fix: disable shell sessions when fetching the PATH (#177)

* fix: disable shell sessions when fetching the PATH

* fix: update open_elixir types

Author: doorgan

apps/expert/lib/expert.ex

@@ -29,6 +29,11 @@ defmodule Expert do
 
   def get_lsp, do: :persistent_term.get(:expert_lsp, nil)
 
+  def terminate(message, status \\ 0) do
+    Logger.error(message)
+    System.stop(status)
+  end
+
   def start_link(args) do
     Logger.debug(inspect(args))
 

apps/expert/lib/expert/engine_node.ex

@@ -45,9 +45,16 @@ defmodule Expert.EngineNode do
           | path_append_arguments(paths)
         ]
 
-      port = Expert.Port.open_elixir(state.project, args: args)
-
-      %{state | port: port, started_by: from}
+      case Expert.Port.open_elixir(state.project, args: args) do
+        {:error, :no_elixir, message} ->
+          GenLSP.error(Expert.get_lsp(), message)
+          Expert.terminate("Failed to find an elixir executable, shutting down", 1)
+          {:error, :no_elixir}
+
+        port ->
+          state = %{state | port: port, started_by: from}
+          {:ok, state}
+      end
     end
 
     def stop(%__MODULE__{} = state, from, stop_timeout) do
@@ -166,48 +173,54 @@ defmodule Expert.EngineNode do
     defp glob_paths(%Project{} = project) do
       lsp = Expert.get_lsp()
       project_name = Project.name(project)
-      {:ok, elixir, env} = Expert.Port.elixir_executable(project)
-
-      GenLSP.info(lsp, "Found elixir for #{project_name} at #{elixir}")
-
-      expert_priv = :code.priv_dir(:expert)
-      packaged_engine_source = Path.join([expert_priv, "engine_source", "apps", "engine"])
-
-      engine_source =
-        "EXPERT_ENGINE_PATH"
-        |> System.get_env(packaged_engine_source)
-        |> Path.expand()
-
-      build_engine_script = Path.join(expert_priv, "build_engine.exs")
-
-      opts =
-        [
-          :stderr_to_stdout,
-          args: [
-            elixir,
-            build_engine_script,
-            "--source-path",
-            engine_source,
-            "--vsn",
-            Expert.vsn()
-          ],
-          env: Expert.Port.ensure_charlists(env),
-          cd: engine_source
-        ]
-
-      launcher = Expert.Port.path()
 
-      GenLSP.info(lsp, "Finding or building engine for project #{project_name}")
-
-      with_progress(project, "Building engine for #{project_name}", fn ->
-        port =
-          Port.open(
-            {:spawn_executable, launcher},
-            opts
-          )
-
-        wait_for_engine(port)
-      end)
+      case Expert.Port.elixir_executable(project) do
+        {:ok, elixir, env} ->
+          GenLSP.info(lsp, "Found elixir for #{project_name} at #{elixir}")
+
+          expert_priv = :code.priv_dir(:expert)
+          packaged_engine_source = Path.join([expert_priv, "engine_source", "apps", "engine"])
+
+          engine_source =
+            "EXPERT_ENGINE_PATH"
+            |> System.get_env(packaged_engine_source)
+            |> Path.expand()
+
+          build_engine_script = Path.join(expert_priv, "build_engine.exs")
+
+          opts =
+            [
+              :stderr_to_stdout,
+              args: [
+                elixir,
+                build_engine_script,
+                "--source-path",
+                engine_source,
+                "--vsn",
+                Expert.vsn()
+              ],
+              env: Expert.Port.ensure_charlists(env),
+              cd: engine_source
+            ]
+
+          launcher = Expert.Port.path()
+
+          GenLSP.info(lsp, "Finding or building engine for project #{project_name}")
+
+          with_progress(project, "Building engine for #{project_name}", fn ->
+            port =
+              Port.open(
+                {:spawn_executable, launcher},
+                opts
+              )
+
+            wait_for_engine(port)
+          end)
+
+        {:error, :no_elixir, message} ->
+          GenLSP.error(Expert.get_lsp(), message)
+          Expert.terminate("Failed to find an elixir executable, shutting down", 1)
+      end
     end
 
     defp wait_for_engine(port) do
@@ -273,8 +286,14 @@ defmodule Expert.EngineNode do
   def handle_call({:start, paths}, from, %State{} = state) do
     :ok = :net_kernel.monitor_nodes(true, node_type: :all)
     Process.send_after(self(), :maybe_start_timeout, @start_timeout)
-    state = State.start(state, paths, from)
-    {:noreply, state}
+
+    case State.start(state, paths, from) do
+      {:ok, state} ->
+        {:noreply, state}
+
+      {:error, :no_elixir} ->
+        {:reply, {:error, :no_elixir}, state}
+    end
   end
 
   @impl true

apps/expert/lib/expert/port.ex

@@ -5,6 +5,8 @@ defmodule Expert.Port do
 
   alias Forge.Project
 
+  require Logger
+
   @type open_opt ::
           {:env, list()}
           | {:cd, String.t() | charlist()}
@@ -19,16 +21,16 @@ defmodule Expert.Port do
   This function takes the project's context into account and looks for the executable via calling
   `elixir_executable(project)`. Environment variables are also retrieved with that call.
   """
-  @spec open_elixir(Project.t(), open_opts()) :: port()
+  @spec open_elixir(Project.t(), open_opts()) :: port() | {:error, :no_elixir, String.t()}
   def open_elixir(%Project{} = project, opts) do
-    {:ok, elixir_executable, environment_variables} = elixir_executable(project)
-
-    opts =
-      opts
-      |> Keyword.put_new_lazy(:cd, fn -> Project.root_path(project) end)
-      |> Keyword.put_new(:env, environment_variables)
+    with {:ok, elixir_executable, environment_variables} <- elixir_executable(project) do
+      opts =
+        opts
+        |> Keyword.put_new_lazy(:cd, fn -> Project.root_path(project) end)
+        |> Keyword.put_new(:env, environment_variables)
 
-    open(project, elixir_executable, opts)
+      open(project, elixir_executable, opts)
+    end
   end
 
   def elixir_executable(%Project{} = project) do
@@ -39,12 +41,8 @@ defmodule Expert.Port do
 
     case :os.find_executable(~c"elixir", to_charlist(path)) do
       false ->
-        GenLSP.error(
-          Expert.get_lsp(),
-          "Couldn't find an elixir executable for project at #{root_path}. Using shell at #{shell} with PATH=#{path}"
-        )
-
-        {:error, :no_elixir}
+        {:error, :no_elixir,
+         "Couldn't find an elixir executable for project at #{root_path}. Using shell at #{shell} with PATH=#{path}"}
 
       elixir ->
         env =
@@ -64,6 +62,8 @@ defmodule Expert.Port do
     # or we get an incomplete PATH not including erl or any other version manager
     # managed programs.
 
+    env = [{"SHELL_SESSIONS_DISABLE", "1"}]
+
     case Path.basename(shell) do
       # Ideally, it should contain the path to shell (e.g. `/usr/bin/fish`),
       # but it might contain only the name of the shell (e.g. `fish`).
@@ -72,12 +72,16 @@ defmodule Expert.Port do
         # to join the entries with colons and have a standard colon-separated PATH output
         # as in bash, which is expected by `:os.find_executable/2`.
         {path, 0} =
-          System.cmd(shell, ["-i", "-l", "-c", "cd #{directory} && string join ':' $PATH"])
+          System.cmd(shell, ["-i", "-l", "-c", "cd #{directory} && string join ':' $PATH"],
+            env: env
+          )
 
         path
 
       _ ->
-        {path, 0} = System.cmd(shell, ["-i", "-l", "-c", "cd #{directory} && echo $PATH"])
+        {path, 0} =
+          System.cmd(shell, ["-i", "-l", "-c", "cd #{directory} && echo $PATH"], env: env)
+
         path
     end
   end

apps/expert/test/expert/engine_node_test.exs

@@ -6,6 +6,7 @@ defmodule Expert.EngineNodeTest do
   import Forge.Test.Fixtures
 
   use ExUnit.Case, async: false
+  use Patch
 
   setup do
     project = project()
@@ -40,4 +41,26 @@ defmodule Expert.EngineNodeTest do
     Process.exit(linked_node_process, :kill)
     assert_eventually Process.whereis(node_process_name) == nil, 50
   end
+
+  test "terminates the server if no elixir is found", %{project: project} do
+    test_pid = self()
+
+    patch(Expert.Port, :path_env_at_directory, nil)
+
+    patch(Expert, :terminate, fn _, status ->
+      send(test_pid, {:stopped, status})
+    end)
+
+    # Note(dorgan): ideally we would use GenLSP.Test here, but
+    # calling `server(Expert)` causes the tests to behave erratically
+    # and either not run or terminate ExUnit early
+    patch(GenLSP, :error, fn _, message ->
+      send(test_pid, {:lsp_log, message})
+    end)
+
+    {:error, :no_elixir} = EngineNode.start(project)
+
+    assert_receive {:stopped, 1}
+    assert_receive {:lsp_log, "Couldn't find an elixir executable for project" <> _}
+  end
 end