From fbbfc928d839d4544c2518e4cffeecec6ebf629b Mon Sep 17 00:00:00 2001 From: doorgan Date: Thu, 23 Oct 2025 15:09:58 -0300 Subject: [PATCH 1/2] fix: disable shell sessions when fetching the PATH --- apps/expert/lib/expert.ex | 5 + apps/expert/lib/expert/engine_node.ex | 111 +++++++++++-------- apps/expert/lib/expert/port.ex | 34 +++--- apps/expert/test/expert/engine_node_test.exs | 23 ++++ 4 files changed, 112 insertions(+), 61 deletions(-) diff --git a/apps/expert/lib/expert.ex b/apps/expert/lib/expert.ex index 264e8fc4..53e9d838 100644 --- a/apps/expert/lib/expert.ex +++ b/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)) diff --git a/apps/expert/lib/expert/engine_node.ex b/apps/expert/lib/expert/engine_node.ex index 0519b924..c948d21e 100644 --- a/apps/expert/lib/expert/engine_node.ex +++ b/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 diff --git a/apps/expert/lib/expert/port.ex b/apps/expert/lib/expert/port.ex index 877339a7..c4e3bde3 100644 --- a/apps/expert/lib/expert/port.ex +++ b/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()} @@ -21,14 +23,14 @@ defmodule Expert.Port do """ @spec open_elixir(Project.t(), open_opts()) :: port() 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 diff --git a/apps/expert/test/expert/engine_node_test.exs b/apps/expert/test/expert/engine_node_test.exs index 2b645243..c1fb7c28 100644 --- a/apps/expert/test/expert/engine_node_test.exs +++ b/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 From 309049c793168662d63bf6d469efdf6a1fb8f59b Mon Sep 17 00:00:00 2001 From: doorgan Date: Thu, 23 Oct 2025 15:49:05 -0300 Subject: [PATCH 2/2] fix: update open_elixir types --- apps/expert/lib/expert/port.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/expert/lib/expert/port.ex b/apps/expert/lib/expert/port.ex index c4e3bde3..99c8a650 100644 --- a/apps/expert/lib/expert/port.ex +++ b/apps/expert/lib/expert/port.ex @@ -21,7 +21,7 @@ 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 with {:ok, elixir_executable, environment_variables} <- elixir_executable(project) do opts =