From 4607e703f7a030f2e4a8439432658d6de1de80d6 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:29:56 -0700 Subject: [PATCH 1/8] fix(orchestrator): ignore stale retry timer deliveries Summary: - Tokenized scheduled retry messages so retries are applied only when the delivered timeout matches the current retry entry. - Ignore legacy or stale retry timeout deliveries instead of letting them consume newer retry metadata. - Added a regression test covering stale retry messages. Rationale: - Retry cancellation alone does not prevent already-delivered timeout messages from racing with newer retry state. - Matching on a per-retry token keeps retry backoff sequencing deterministic under mailbox races. Tests: - mix test test/symphony_elixir/core_test.exs test/symphony_elixir/orchestrator_status_test.exs Co-authored-by: Codex --- elixir/lib/symphony_elixir/orchestrator.ex | 14 +++++--- elixir/test/symphony_elixir/core_test.exs | 40 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index 4286ce1f..f86e7a3d 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -155,9 +155,9 @@ defmodule SymphonyElixir.Orchestrator do def handle_info({:codex_worker_update, _issue_id, _update}, state), do: {:noreply, state} - def handle_info({:retry_issue, issue_id}, state) do + def handle_info({:retry_issue, issue_id, retry_token}, state) do result = - case pop_retry_attempt_state(state, issue_id) do + case pop_retry_attempt_state(state, issue_id, retry_token) do {:ok, attempt, metadata, state} -> handle_retry_issue(state, issue_id, attempt, metadata) :missing -> {:noreply, state} end @@ -166,6 +166,8 @@ defmodule SymphonyElixir.Orchestrator do result end + def handle_info({:retry_issue, _issue_id}, state), do: {:noreply, state} + def handle_info(msg, state) do Logger.debug("Orchestrator ignored message: #{inspect(msg)}") {:noreply, state} @@ -669,6 +671,7 @@ defmodule SymphonyElixir.Orchestrator do next_attempt = if is_integer(attempt), do: attempt, else: previous_retry.attempt + 1 delay_ms = retry_delay(next_attempt, metadata) old_timer = Map.get(previous_retry, :timer_ref) + retry_token = make_ref() due_at_ms = System.monotonic_time(:millisecond) + delay_ms identifier = pick_retry_identifier(issue_id, previous_retry, metadata) error = pick_retry_error(previous_retry, metadata) @@ -677,7 +680,7 @@ defmodule SymphonyElixir.Orchestrator do Process.cancel_timer(old_timer) end - timer_ref = Process.send_after(self(), {:retry_issue, issue_id}, delay_ms) + timer_ref = Process.send_after(self(), {:retry_issue, issue_id, retry_token}, delay_ms) error_suffix = if is_binary(error), do: " error=#{error}", else: "" @@ -689,6 +692,7 @@ defmodule SymphonyElixir.Orchestrator do Map.put(state.retry_attempts, issue_id, %{ attempt: next_attempt, timer_ref: timer_ref, + retry_token: retry_token, due_at_ms: due_at_ms, identifier: identifier, error: error @@ -696,9 +700,9 @@ defmodule SymphonyElixir.Orchestrator do } end - defp pop_retry_attempt_state(%State{} = state, issue_id) do + defp pop_retry_attempt_state(%State{} = state, issue_id, retry_token) when is_reference(retry_token) do case Map.get(state.retry_attempts, issue_id) do - %{attempt: attempt} = retry_entry -> + %{attempt: attempt, retry_token: ^retry_token} = retry_entry -> metadata = %{ identifier: Map.get(retry_entry, :identifier), error: Map.get(retry_entry, :error) diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index 023305eb..ffc6dd55 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -555,6 +555,46 @@ defmodule SymphonyElixir.CoreTest do assert_due_in_range(due_at_ms, 9_000, 10_500) end + test "stale retry timer messages do not consume newer retry entries" do + issue_id = "issue-stale-retry" + orchestrator_name = Module.concat(__MODULE__, :StaleRetryOrchestrator) + {:ok, pid} = Orchestrator.start_link(name: orchestrator_name) + + on_exit(fn -> + if Process.alive?(pid) do + Process.exit(pid, :normal) + end + end) + + initial_state = :sys.get_state(pid) + current_retry_token = make_ref() + stale_retry_token = make_ref() + + :sys.replace_state(pid, fn _ -> + initial_state + |> Map.put(:retry_attempts, %{ + issue_id => %{ + attempt: 2, + timer_ref: nil, + retry_token: current_retry_token, + due_at_ms: System.monotonic_time(:millisecond) + 30_000, + identifier: "MT-561", + error: "agent exited: :boom" + } + }) + end) + + send(pid, {:retry_issue, issue_id, stale_retry_token}) + Process.sleep(50) + + assert %{ + attempt: 2, + retry_token: ^current_retry_token, + identifier: "MT-561", + error: "agent exited: :boom" + } = :sys.get_state(pid).retry_attempts[issue_id] + end + defp assert_due_in_range(due_at_ms, min_remaining_ms, max_remaining_ms) do remaining_ms = due_at_ms - System.monotonic_time(:millisecond) From b53f2b5a3a0200f135635780b56f00cea2d5c917 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:31:54 -0700 Subject: [PATCH 2/8] fix(orchestrator): coalesce superseded poll ticks Summary: - Tracked the active poll timer and delivery token in orchestrator state. - Replaced fire-and-forget poll scheduling with cancellable, tokenized tick scheduling for startup, periodic polls, and manual refreshes. - Added a regression test covering repeated refresh requests and ignored stale tick deliveries. Rationale: - Manual refreshes should replace a pending poll instead of racing it and enqueueing duplicate poll cycles. - Tokenized tick messages keep the scheduler deterministic even when a cancelled timer still delivers later. Tests: - mix test test/symphony_elixir/core_test.exs test/symphony_elixir/orchestrator_status_test.exs Co-authored-by: Codex --- elixir/lib/symphony_elixir/orchestrator.ex | 61 +++++++++++++++++----- elixir/test/symphony_elixir/core_test.exs | 30 +++++++++++ 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index f86e7a3d..25e82ed6 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -31,6 +31,8 @@ defmodule SymphonyElixir.Orchestrator do :max_concurrent_agents, :next_poll_due_at_ms, :poll_check_in_progress, + :tick_timer_ref, + :tick_token, running: %{}, completed: MapSet.new(), claimed: MapSet.new(), @@ -56,20 +58,46 @@ defmodule SymphonyElixir.Orchestrator do max_concurrent_agents: config.agent.max_concurrent_agents, next_poll_due_at_ms: now_ms, poll_check_in_progress: false, + tick_timer_ref: nil, + tick_token: nil, codex_totals: @empty_codex_totals, codex_rate_limits: nil } run_terminal_workspace_cleanup() - :ok = schedule_tick(0) + state = schedule_tick(state, 0) {:ok, state} end @impl true + def handle_info({:tick, tick_token}, %{tick_token: tick_token} = state) + when is_reference(tick_token) do + state = refresh_runtime_config(state) + state = %{ + state + | poll_check_in_progress: true, + next_poll_due_at_ms: nil, + tick_timer_ref: nil, + tick_token: nil + } + + notify_dashboard() + :ok = schedule_poll_cycle_start() + {:noreply, state} + end + + def handle_info({:tick, _tick_token}, state), do: {:noreply, state} + def handle_info(:tick, state) do state = refresh_runtime_config(state) - state = %{state | poll_check_in_progress: true, next_poll_due_at_ms: nil} + state = %{ + state + | poll_check_in_progress: true, + next_poll_due_at_ms: nil, + tick_timer_ref: nil, + tick_token: nil + } notify_dashboard() :ok = schedule_poll_cycle_start() @@ -79,11 +107,8 @@ defmodule SymphonyElixir.Orchestrator do def handle_info(:run_poll_cycle, state) do state = refresh_runtime_config(state) state = maybe_dispatch(state) - now_ms = System.monotonic_time(:millisecond) - next_poll_due_at_ms = now_ms + state.poll_interval_ms - :ok = schedule_tick(state.poll_interval_ms) - - state = %{state | poll_check_in_progress: false, next_poll_due_at_ms: next_poll_due_at_ms} + state = schedule_tick(state, state.poll_interval_ms) + state = %{state | poll_check_in_progress: false} notify_dashboard() {:noreply, state} @@ -964,10 +989,7 @@ defmodule SymphonyElixir.Orchestrator do now_ms = System.monotonic_time(:millisecond) already_due? = is_integer(state.next_poll_due_at_ms) and state.next_poll_due_at_ms <= now_ms coalesced = state.poll_check_in_progress == true or already_due? - - unless coalesced do - :ok = schedule_tick(0) - end + state = if coalesced, do: state, else: schedule_tick(state, 0) {:reply, %{ @@ -1052,9 +1074,20 @@ defmodule SymphonyElixir.Orchestrator do } end - defp schedule_tick(delay_ms) do - :timer.send_after(delay_ms, self(), :tick) - :ok + defp schedule_tick(%State{} = state, delay_ms) when is_integer(delay_ms) and delay_ms >= 0 do + if is_reference(state.tick_timer_ref) do + Process.cancel_timer(state.tick_timer_ref) + end + + tick_token = make_ref() + timer_ref = Process.send_after(self(), {:tick, tick_token}, delay_ms) + + %{ + state + | tick_timer_ref: timer_ref, + tick_token: tick_token, + next_poll_due_at_ms: System.monotonic_time(:millisecond) + delay_ms + } end defp schedule_poll_cycle_start do diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index ffc6dd55..fb9311d2 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -595,6 +595,36 @@ defmodule SymphonyElixir.CoreTest do } = :sys.get_state(pid).retry_attempts[issue_id] end + test "manual refresh coalesces repeated requests and ignores superseded ticks" do + now_ms = System.monotonic_time(:millisecond) + stale_tick_token = make_ref() + + state = %Orchestrator.State{ + poll_interval_ms: 30_000, + max_concurrent_agents: 1, + next_poll_due_at_ms: now_ms + 30_000, + poll_check_in_progress: false, + tick_timer_ref: nil, + tick_token: stale_tick_token, + codex_totals: %{input_tokens: 0, output_tokens: 0, total_tokens: 0, seconds_running: 0}, + codex_rate_limits: nil + } + + assert {:reply, %{queued: true, coalesced: false}, refreshed_state} = + Orchestrator.handle_call(:request_refresh, {self(), make_ref()}, state) + + assert is_reference(refreshed_state.tick_timer_ref) + assert is_reference(refreshed_state.tick_token) + refute refreshed_state.tick_token == stale_tick_token + assert refreshed_state.next_poll_due_at_ms <= System.monotonic_time(:millisecond) + + assert {:reply, %{queued: true, coalesced: true}, coalesced_state} = + Orchestrator.handle_call(:request_refresh, {self(), make_ref()}, refreshed_state) + + assert coalesced_state.tick_token == refreshed_state.tick_token + assert {:noreply, ^coalesced_state} = Orchestrator.handle_info({:tick, stale_tick_token}, coalesced_state) + end + defp assert_due_in_range(due_at_ms, min_remaining_ms, max_remaining_ms) do remaining_ms = due_at_ms - System.monotonic_time(:millisecond) From e0ec20a5972a24a9acc7deab945b4be492384a0c Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:38:50 -0700 Subject: [PATCH 3/8] fix(safety): canonicalize workspaces and sandbox roots Summary: - Added canonical path resolution for workspace and Codex cwd checks so symlinked roots and child paths resolve to their real locations. - Hardened runtime turn sandbox policy resolution to reject unsafe types, external writable roots, relative writable roots, and network-enabled policies. - Added regression coverage for symlinked workspace roots, symlink cwd escapes, and unsafe custom sandbox policies. Rationale: - Lexical prefix checks are not sufficient on macOS and other systems where workspace paths can traverse symlinks. - Codex turn policies must stay bounded to the active issue workspace instead of accepting broader writable roots at runtime. Tests: - mix test test/symphony_elixir/workspace_and_config_test.exs test/symphony_elixir/app_server_test.exs Co-authored-by: Codex --- .../lib/symphony_elixir/codex/app_server.ex | 38 +++-- elixir/lib/symphony_elixir/config.ex | 24 ++- elixir/lib/symphony_elixir/config/schema.ex | 107 +++++++++++++ elixir/lib/symphony_elixir/path_safety.ex | 52 +++++++ elixir/lib/symphony_elixir/workspace.ex | 69 ++++----- .../test/symphony_elixir/app_server_test.exs | 37 +++++ .../workspace_and_config_test.exs | 141 ++++++++++++++++-- 7 files changed, 392 insertions(+), 76 deletions(-) create mode 100644 elixir/lib/symphony_elixir/path_safety.ex diff --git a/elixir/lib/symphony_elixir/codex/app_server.ex b/elixir/lib/symphony_elixir/codex/app_server.ex index 1f9252e2..86bfff5a 100644 --- a/elixir/lib/symphony_elixir/codex/app_server.ex +++ b/elixir/lib/symphony_elixir/codex/app_server.ex @@ -4,7 +4,7 @@ defmodule SymphonyElixir.Codex.AppServer do """ require Logger - alias SymphonyElixir.{Codex.DynamicTool, Config} + alias SymphonyElixir.{Codex.DynamicTool, Config, PathSafety} @initialize_id 1 @thread_start_id 2 @@ -37,10 +37,9 @@ defmodule SymphonyElixir.Codex.AppServer do @spec start_session(Path.t()) :: {:ok, session()} | {:error, term()} def start_session(workspace) do - with :ok <- validate_workspace_cwd(workspace), - {:ok, port} <- start_port(workspace) do + with {:ok, expanded_workspace} <- validate_workspace_cwd(workspace), + {:ok, port} <- start_port(expanded_workspace) do metadata = port_metadata(port) - expanded_workspace = Path.expand(workspace) with {:ok, session_policies} <- session_policies(expanded_workspace), {:ok, thread_id} <- do_start_session(port, expanded_workspace, session_policies) do @@ -142,20 +141,31 @@ defmodule SymphonyElixir.Codex.AppServer do end defp validate_workspace_cwd(workspace) when is_binary(workspace) do - workspace_path = Path.expand(workspace) - workspace_root = Path.expand(Config.settings!().workspace.root) + expanded_workspace = Path.expand(workspace) + expanded_root = Path.expand(Config.settings!().workspace.root) + expanded_root_prefix = expanded_root <> "/" - root_prefix = workspace_root <> "/" + with {:ok, canonical_workspace} <- PathSafety.canonicalize(expanded_workspace), + {:ok, canonical_root} <- PathSafety.canonicalize(expanded_root) do + canonical_root_prefix = canonical_root <> "/" - cond do - workspace_path == workspace_root -> - {:error, {:invalid_workspace_cwd, :workspace_root, workspace_path}} + cond do + canonical_workspace == canonical_root -> + {:error, {:invalid_workspace_cwd, :workspace_root, canonical_workspace}} - not String.starts_with?(workspace_path <> "/", root_prefix) -> - {:error, {:invalid_workspace_cwd, :outside_workspace_root, workspace_path, workspace_root}} + String.starts_with?(canonical_workspace <> "/", canonical_root_prefix) -> + {:ok, canonical_workspace} - true -> - :ok + String.starts_with?(expanded_workspace <> "/", expanded_root_prefix) -> + {:error, {:invalid_workspace_cwd, :symlink_escape, expanded_workspace, canonical_root}} + + true -> + {:error, + {:invalid_workspace_cwd, :outside_workspace_root, canonical_workspace, canonical_root}} + end + else + {:error, {:path_canonicalize_failed, path, reason}} -> + {:error, {:invalid_workspace_cwd, :path_unreadable, path, reason}} end end diff --git a/elixir/lib/symphony_elixir/config.ex b/elixir/lib/symphony_elixir/config.ex index 84623ca2..87011ac1 100644 --- a/elixir/lib/symphony_elixir/config.ex +++ b/elixir/lib/symphony_elixir/config.ex @@ -63,8 +63,13 @@ defmodule SymphonyElixir.Config do @spec codex_turn_sandbox_policy(Path.t() | nil) :: map() def codex_turn_sandbox_policy(workspace \\ nil) do - settings!() - |> Schema.resolve_turn_sandbox_policy(workspace) + case Schema.resolve_runtime_turn_sandbox_policy(settings!(), workspace) do + {:ok, policy} -> + policy + + {:error, reason} -> + raise ArgumentError, message: "Invalid codex turn sandbox policy: #{inspect(reason)}" + end end @spec workflow_prompt() :: String.t() @@ -96,12 +101,15 @@ defmodule SymphonyElixir.Config do @spec codex_runtime_settings(Path.t() | nil) :: {:ok, codex_runtime_settings()} | {:error, term()} def codex_runtime_settings(workspace \\ nil) do with {:ok, settings} <- settings() do - {:ok, - %{ - approval_policy: settings.codex.approval_policy, - thread_sandbox: settings.codex.thread_sandbox, - turn_sandbox_policy: Schema.resolve_turn_sandbox_policy(settings, workspace) - }} + with {:ok, turn_sandbox_policy} <- + Schema.resolve_runtime_turn_sandbox_policy(settings, workspace) do + {:ok, + %{ + approval_policy: settings.codex.approval_policy, + thread_sandbox: settings.codex.thread_sandbox, + turn_sandbox_policy: turn_sandbox_policy + }} + end end end diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index dcb67f98..87dd481f 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -5,6 +5,8 @@ defmodule SymphonyElixir.Config.Schema do import Ecto.Changeset + alias SymphonyElixir.PathSafety + @primary_key false @type t :: %__MODULE__{} @@ -278,6 +280,16 @@ defmodule SymphonyElixir.Config.Schema do end end + @spec resolve_runtime_turn_sandbox_policy(%__MODULE__{}, Path.t() | nil) :: + {:ok, map()} | {:error, term()} + def resolve_runtime_turn_sandbox_policy(settings, workspace \\ nil) do + workspace_root = workspace || settings.workspace.root + + settings + |> resolve_turn_sandbox_policy(workspace_root) + |> validate_runtime_turn_sandbox_policy(workspace_root) + end + @spec normalize_issue_state(String.t()) :: String.t() def normalize_issue_state(state_name) when is_binary(state_name) do String.downcase(state_name) @@ -457,6 +469,101 @@ defmodule SymphonyElixir.Config.Schema do } end + defp validate_runtime_turn_sandbox_policy(policy, workspace_root) + when is_map(policy) and is_binary(workspace_root) do + with {:ok, canonical_workspace_root} <- PathSafety.canonicalize(workspace_root) do + case Map.get(policy, "type") do + "workspaceWrite" -> + validate_workspace_write_policy(policy, canonical_workspace_root) + + "readOnly" -> + validate_read_only_policy(policy) + + other -> + {:error, {:unsafe_turn_sandbox_policy, {:unsupported_type, other}}} + end + end + end + + defp validate_runtime_turn_sandbox_policy(_policy, workspace_root) do + {:error, {:unsafe_turn_sandbox_policy, {:invalid_workspace_root, workspace_root}}} + end + + defp validate_workspace_write_policy(policy, canonical_workspace_root) do + with :ok <- validate_network_access_disabled(policy), + {:ok, writable_roots} <- + validate_writable_roots(Map.get(policy, "writableRoots"), canonical_workspace_root) do + {:ok, + policy + |> Map.put("type", "workspaceWrite") + |> Map.put("writableRoots", writable_roots) + |> Map.put_new("readOnlyAccess", %{"type" => "fullAccess"}) + |> Map.put_new("networkAccess", false) + |> Map.put_new("excludeTmpdirEnvVar", false) + |> Map.put_new("excludeSlashTmp", false)} + end + end + + defp validate_read_only_policy(policy) do + with :ok <- validate_network_access_disabled(policy) do + {:ok, Map.put(policy, "type", "readOnly")} + end + end + + defp validate_network_access_disabled(policy) do + if Map.get(policy, "networkAccess") == true do + {:error, {:unsafe_turn_sandbox_policy, :network_access_enabled}} + else + :ok + end + end + + defp validate_writable_roots(writable_roots, canonical_workspace_root) + when is_list(writable_roots) and writable_roots != [] do + writable_roots + |> Enum.reduce_while({:ok, []}, fn root, {:ok, acc} -> + case validate_writable_root(root, canonical_workspace_root) do + {:ok, canonical_root} -> {:cont, {:ok, [canonical_root | acc]}} + {:error, reason} -> {:halt, {:error, reason}} + end + end) + |> case do + {:ok, canonical_roots} -> {:ok, Enum.reverse(canonical_roots)} + {:error, reason} -> {:error, reason} + end + end + + defp validate_writable_roots(_writable_roots, _canonical_workspace_root) do + {:error, {:unsafe_turn_sandbox_policy, :missing_writable_roots}} + end + + defp validate_writable_root(root, canonical_workspace_root) when is_binary(root) do + if Path.type(root) != :absolute do + {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, root}}} + else + with {:ok, canonical_root} <- PathSafety.canonicalize(root) do + workspace_prefix = canonical_workspace_root <> "/" + + cond do + canonical_root == canonical_workspace_root -> + {:ok, canonical_root} + + String.starts_with?(canonical_root <> "/", workspace_prefix) -> + {:ok, canonical_root} + + true -> + {:error, + {:unsafe_turn_sandbox_policy, + {:writable_root_outside_workspace, canonical_root, canonical_workspace_root}}} + end + end + end + end + + defp validate_writable_root(root, _canonical_workspace_root) do + {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, root}}} + end + defp format_errors(changeset) do changeset |> traverse_errors(&translate_error/1) diff --git a/elixir/lib/symphony_elixir/path_safety.ex b/elixir/lib/symphony_elixir/path_safety.ex new file mode 100644 index 00000000..c12ce00f --- /dev/null +++ b/elixir/lib/symphony_elixir/path_safety.ex @@ -0,0 +1,52 @@ +defmodule SymphonyElixir.PathSafety do + @moduledoc false + + @spec canonicalize(Path.t()) :: {:ok, Path.t()} | {:error, term()} + def canonicalize(path) when is_binary(path) do + expanded_path = Path.expand(path) + {root, segments} = split_absolute_path(expanded_path) + + case resolve_segments(root, [], segments) do + {:ok, canonical_path} -> + {:ok, canonical_path} + + {:error, reason} -> + {:error, {:path_canonicalize_failed, expanded_path, reason}} + end + end + + defp split_absolute_path(path) when is_binary(path) do + case Path.split(path) do + [root | segments] -> {root, segments} + [] -> {"/", []} + end + end + + defp resolve_segments(root, resolved_segments, []), do: {:ok, join_path(root, resolved_segments)} + + defp resolve_segments(root, resolved_segments, [segment | rest]) do + candidate_path = join_path(root, resolved_segments ++ [segment]) + + case File.lstat(candidate_path) do + {:ok, %File.Stat{type: :symlink}} -> + with {:ok, target} <- :file.read_link_all(String.to_charlist(candidate_path)) do + resolved_target = Path.expand(IO.chardata_to_string(target), join_path(root, resolved_segments)) + {target_root, target_segments} = split_absolute_path(resolved_target) + resolve_segments(target_root, [], target_segments ++ rest) + end + + {:ok, _stat} -> + resolve_segments(root, resolved_segments ++ [segment], rest) + + {:error, :enoent} -> + {:ok, join_path(root, resolved_segments ++ [segment | rest])} + + {:error, reason} -> + {:error, reason} + end + end + + defp join_path(root, segments) when is_list(segments) do + Enum.reduce(segments, root, fn segment, acc -> Path.join(acc, segment) end) + end +end diff --git a/elixir/lib/symphony_elixir/workspace.ex b/elixir/lib/symphony_elixir/workspace.ex index 2401573f..539976e9 100644 --- a/elixir/lib/symphony_elixir/workspace.ex +++ b/elixir/lib/symphony_elixir/workspace.ex @@ -4,7 +4,7 @@ defmodule SymphonyElixir.Workspace do """ require Logger - alias SymphonyElixir.Config + alias SymphonyElixir.{Config, PathSafety} @excluded_entries MapSet.new([".elixir_ls", "tmp"]) @@ -15,9 +15,8 @@ defmodule SymphonyElixir.Workspace do try do safe_id = safe_identifier(issue_context.issue_identifier) - workspace = workspace_path_for_issue(safe_id) - - with :ok <- validate_workspace_path(workspace), + with {:ok, workspace} <- workspace_path_for_issue(safe_id), + :ok <- validate_workspace_path(workspace), {:ok, created?} <- ensure_workspace(workspace), :ok <- maybe_run_after_create_hook(workspace, issue_context, created?) do {:ok, workspace} @@ -71,9 +70,12 @@ defmodule SymphonyElixir.Workspace do @spec remove_issue_workspaces(term()) :: :ok def remove_issue_workspaces(identifier) when is_binary(identifier) do safe_id = safe_identifier(identifier) - workspace = Path.join(Config.settings!().workspace.root, safe_id) - remove(workspace) + case workspace_path_for_issue(safe_id) do + {:ok, workspace} -> remove(workspace) + {:error, _reason} -> :ok + end + :ok end @@ -111,7 +113,9 @@ defmodule SymphonyElixir.Workspace do end defp workspace_path_for_issue(safe_id) when is_binary(safe_id) do - Path.join(Config.settings!().workspace.root, safe_id) + Config.settings!().workspace.root + |> Path.join(safe_id) + |> PathSafety.canonicalize() end defp safe_identifier(identifier) do @@ -218,46 +222,29 @@ defmodule SymphonyElixir.Workspace do defp validate_workspace_path(workspace) when is_binary(workspace) do expanded_workspace = Path.expand(workspace) - root = Path.expand(Config.settings!().workspace.root) - root_prefix = root <> "/" + expanded_root = Path.expand(Config.settings!().workspace.root) + expanded_root_prefix = expanded_root <> "/" - cond do - expanded_workspace == root -> - {:error, {:workspace_equals_root, expanded_workspace, root}} + with {:ok, canonical_workspace} <- PathSafety.canonicalize(expanded_workspace), + {:ok, canonical_root} <- PathSafety.canonicalize(expanded_root) do + canonical_root_prefix = canonical_root <> "/" - String.starts_with?(expanded_workspace <> "/", root_prefix) -> - ensure_no_symlink_components(expanded_workspace, root) + cond do + canonical_workspace == canonical_root -> + {:error, {:workspace_equals_root, canonical_workspace, canonical_root}} - true -> - {:error, {:workspace_outside_root, expanded_workspace, root}} - end - end - - defp ensure_no_symlink_components(workspace, root) do - workspace - |> Path.relative_to(root) - |> Path.split() - |> Enum.reduce_while(root, fn segment, current_path -> - next_path = Path.join(current_path, segment) - - case File.lstat(next_path) do - {:ok, %File.Stat{type: :symlink}} -> - {:halt, {:error, {:workspace_symlink_escape, next_path, root}}} + String.starts_with?(canonical_workspace <> "/", canonical_root_prefix) -> + :ok - {:ok, _stat} -> - {:cont, next_path} + String.starts_with?(expanded_workspace <> "/", expanded_root_prefix) -> + {:error, {:workspace_symlink_escape, expanded_workspace, canonical_root}} - {:error, :enoent} -> - {:halt, :ok} - - {:error, reason} -> - {:halt, {:error, {:workspace_path_unreadable, next_path, reason}}} + true -> + {:error, {:workspace_outside_root, canonical_workspace, canonical_root}} end - end) - |> case do - :ok -> :ok - {:error, _reason} = error -> error - _final_path -> :ok + else + {:error, {:path_canonicalize_failed, path, reason}} -> + {:error, {:workspace_path_unreadable, path, reason}} end end diff --git a/elixir/test/symphony_elixir/app_server_test.exs b/elixir/test/symphony_elixir/app_server_test.exs index 20ab61e9..1a9ec88a 100644 --- a/elixir/test/symphony_elixir/app_server_test.exs +++ b/elixir/test/symphony_elixir/app_server_test.exs @@ -39,6 +39,43 @@ defmodule SymphonyElixir.AppServerTest do end end + test "app server rejects symlink escape cwd paths under the workspace root" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-app-server-symlink-cwd-guard-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + outside_workspace = Path.join(test_root, "outside") + symlink_workspace = Path.join(workspace_root, "MT-1000") + + File.mkdir_p!(workspace_root) + File.mkdir_p!(outside_workspace) + File.ln_s!(outside_workspace, symlink_workspace) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root + ) + + issue = %Issue{ + id: "issue-workspace-symlink-guard", + identifier: "MT-1000", + title: "Validate symlink workspace guard", + description: "Ensure app-server refuses symlink escape cwd targets", + state: "In Progress", + url: "https://example.org/issues/MT-1000", + labels: ["backend"] + } + + assert {:error, {:invalid_workspace_cwd, :symlink_escape, ^symlink_workspace, _root}} = + AppServer.run(symlink_workspace, "guard", issue) + after + File.rm_rf(test_root) + end + end + test "app server marks request-for-input events as a hard failure" do test_root = Path.join( diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 7b681d54..55a71425 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -106,8 +106,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root) + assert {:ok, canonical_workspace} = SymphonyElixir.PathSafety.canonicalize(stale_workspace) assert {:ok, workspace} = Workspace.create_for_issue("MT-STALE") - assert workspace == stale_workspace + assert workspace == canonical_workspace assert File.dir?(workspace) after File.rm_rf(workspace_root) @@ -132,13 +133,43 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root) - assert {:error, {:workspace_symlink_escape, ^symlink_path, ^workspace_root}} = + assert {:ok, canonical_outside_root} = SymphonyElixir.PathSafety.canonicalize(outside_root) + assert {:ok, canonical_workspace_root} = SymphonyElixir.PathSafety.canonicalize(workspace_root) + + assert {:error, {:workspace_outside_root, ^canonical_outside_root, ^canonical_workspace_root}} = Workspace.create_for_issue("MT-SYM") after File.rm_rf(test_root) end end + test "workspace canonicalizes symlinked workspace roots before creating issue directories" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-workspace-root-symlink-#{System.unique_integer([:positive])}" + ) + + try do + actual_root = Path.join(test_root, "actual-workspaces") + linked_root = Path.join(test_root, "linked-workspaces") + + File.mkdir_p!(actual_root) + File.ln_s!(actual_root, linked_root) + + write_workflow_file!(Workflow.workflow_file_path(), workspace_root: linked_root) + + assert {:ok, canonical_workspace} = + SymphonyElixir.PathSafety.canonicalize(Path.join(actual_root, "MT-LINK")) + + assert {:ok, workspace} = Workspace.create_for_issue("MT-LINK") + assert workspace == canonical_workspace + assert File.dir?(workspace) + after + File.rm_rf(test_root) + end + end + test "workspace remove rejects the workspace root itself with a distinct error" do workspace_root = Path.join( @@ -150,7 +181,10 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do File.mkdir_p!(workspace_root) write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root) - assert {:error, {:workspace_equals_root, ^workspace_root, ^workspace_root}, ""} = + assert {:ok, canonical_workspace_root} = + SymphonyElixir.PathSafety.canonicalize(workspace_root) + + assert {:error, {:workspace_equals_root, ^canonical_workspace_root, ^canonical_workspace_root}, ""} = Workspace.remove(workspace_root) after File.rm_rf(workspace_root) @@ -209,8 +243,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root) workspace = Path.join(workspace_root, "MT-608") + assert {:ok, canonical_workspace} = SymphonyElixir.PathSafety.canonicalize(workspace) - assert {:ok, ^workspace} = Workspace.create_for_issue("MT-608") + assert {:ok, ^canonical_workspace} = Workspace.create_for_issue("MT-608") assert File.dir?(workspace) assert {:ok, []} = File.ls(workspace) after @@ -677,9 +712,12 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert config.codex.thread_sandbox == "workspace-write" + assert {:ok, canonical_default_workspace_root} = + SymphonyElixir.PathSafety.canonicalize(Path.join(System.tmp_dir!(), "symphony_workspaces")) + assert Config.codex_turn_sandbox_policy() == %{ "type" => "workspaceWrite", - "writableRoots" => [Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces"))], + "writableRoots" => [canonical_default_workspace_root], "readOnlyAccess" => %{"type" => "fullAccess"}, "networkAccess" => false, "excludeTmpdirEnvVar" => false, @@ -693,19 +731,45 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do write_workflow_file!(Workflow.workflow_file_path(), codex_command: "codex app-server --model gpt-5.3-codex") assert Config.settings!().codex.command == "codex app-server --model gpt-5.3-codex" + explicit_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-explicit-sandbox-root-#{System.unique_integer([:positive])}" + ) + + explicit_workspace = Path.join(explicit_root, "MT-EXPLICIT") + explicit_cache = Path.join(explicit_workspace, "cache") + File.mkdir_p!(explicit_cache) + + on_exit(fn -> File.rm_rf(explicit_root) end) + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: explicit_root, codex_approval_policy: "on-request", codex_thread_sandbox: "workspace-write", - codex_turn_sandbox_policy: %{type: "workspaceWrite", writableRoots: ["/tmp/workspace", "/tmp/cache"]} + codex_turn_sandbox_policy: %{ + type: "workspaceWrite", + writableRoots: [explicit_workspace, explicit_cache] + } ) config = Config.settings!() assert config.codex.approval_policy == "on-request" assert config.codex.thread_sandbox == "workspace-write" - assert Config.codex_turn_sandbox_policy() == %{ + assert {:ok, canonical_explicit_workspace} = + SymphonyElixir.PathSafety.canonicalize(explicit_workspace) + + assert {:ok, canonical_explicit_cache} = + SymphonyElixir.PathSafety.canonicalize(explicit_cache) + + assert Config.codex_turn_sandbox_policy(explicit_workspace) == %{ "type" => "workspaceWrite", - "writableRoots" => ["/tmp/workspace", "/tmp/cache"] + "writableRoots" => [canonical_explicit_workspace, canonical_explicit_cache], + "readOnlyAccess" => %{"type" => "fullAccess"}, + "networkAccess" => false, + "excludeTmpdirEnvVar" => false, + "excludeSlashTmp" => false } write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: ",") @@ -770,13 +834,12 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert config.codex.approval_policy == "future-policy" assert config.codex.thread_sandbox == "future-sandbox" - assert Config.codex_turn_sandbox_policy() == %{ - "type" => "futureSandbox", - "nested" => %{"flag" => true} - } - assert :ok = Config.validate!() + assert_raise ArgumentError, ~r/Invalid codex turn sandbox policy/, fn -> + Config.codex_turn_sandbox_policy() + end + write_workflow_file!(Workflow.workflow_file_path(), codex_command: "codex app-server") assert Config.settings!().codex.command == "codex app-server" end @@ -976,6 +1039,58 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } end + test "runtime sandbox policy resolution rejects unsafe custom policies" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-runtime-sandbox-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + issue_workspace = Path.join(workspace_root, "MT-100") + cache_root = Path.join(issue_workspace, "cache") + + File.mkdir_p!(cache_root) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + codex_turn_sandbox_policy: %{ + type: "workspaceWrite", + writableRoots: [workspace_root] + } + ) + + assert {:error, {:unsafe_turn_sandbox_policy, {:writable_root_outside_workspace, _, _}}} = + Config.codex_runtime_settings(issue_workspace) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + codex_turn_sandbox_policy: %{ + type: "workspaceWrite", + writableRoots: ["relative/path"] + } + ) + + assert {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, "relative/path"}}} = + Config.codex_runtime_settings(issue_workspace) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + codex_turn_sandbox_policy: %{ + type: "workspaceWrite", + writableRoots: [issue_workspace, cache_root], + networkAccess: true + } + ) + + assert {:error, {:unsafe_turn_sandbox_policy, :network_access_enabled}} = + Config.codex_runtime_settings(issue_workspace) + after + File.rm_rf(test_root) + end + end + test "workflow prompt is used when building base prompt" do workflow_prompt = "Workflow prompt body used as codex instruction." From f27a69083f31c54b85ef425b4cc37aa7f01c8874 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:41:23 -0700 Subject: [PATCH 4/8] fix(orchestrator): stop workers for missing tracker issues Summary: - Reconciled running workers against the IDs actually returned by the tracker refresh and stop workers whose issues disappear. - Logged the missing-issue shutdown path and released claims without deleting the workspace. - Added regression coverage for the missing-issue case and updated app-server startup expectations for canonical workspace paths. Rationale: - A running worker should not continue until stall timeout when the tracker no longer returns its issue. - The runtime now canonicalizes workspace paths, so startup payload assertions must match the real bounded workspace and sandbox roots. Tests: - mix test test/symphony_elixir/core_test.exs test/symphony_elixir/orchestrator_status_test.exs Co-authored-by: Codex --- elixir/lib/symphony_elixir/orchestrator.ex | 43 ++++++++- elixir/test/symphony_elixir/core_test.exs | 101 ++++++++++++++++++++- 2 files changed, 137 insertions(+), 7 deletions(-) diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index 25e82ed6..49e40349 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -258,12 +258,13 @@ defmodule SymphonyElixir.Orchestrator do else case Tracker.fetch_issue_states_by_ids(running_ids) do {:ok, issues} -> - reconcile_running_issue_states( - issues, + issues + |> reconcile_running_issue_states( state, active_state_set(), terminal_state_set() ) + |> reconcile_missing_running_issue_ids(running_ids, issues) {:error, reason} -> Logger.debug("Failed to refresh running issue states: #{inspect(reason)}; keeping active workers") @@ -338,6 +339,44 @@ defmodule SymphonyElixir.Orchestrator do defp reconcile_issue_state(_issue, state, _active_states, _terminal_states), do: state + defp reconcile_missing_running_issue_ids(%State{} = state, requested_issue_ids, issues) + when is_list(requested_issue_ids) and is_list(issues) do + visible_issue_ids = + issues + |> Enum.flat_map(fn + %Issue{id: issue_id} when is_binary(issue_id) -> [issue_id] + _ -> [] + end) + |> MapSet.new() + + Enum.reduce(requested_issue_ids, state, fn issue_id, state_acc -> + if MapSet.member?(visible_issue_ids, issue_id) do + state_acc + else + log_missing_running_issue(state_acc, issue_id) + terminate_running_issue(state_acc, issue_id, false) + end + end) + end + + defp reconcile_missing_running_issue_ids(state, _requested_issue_ids, _issues), do: state + + defp log_missing_running_issue(%State{} = state, issue_id) when is_binary(issue_id) do + case Map.get(state.running, issue_id) do + %{identifier: identifier} -> + Logger.info( + "Issue no longer visible during running-state refresh: issue_id=#{issue_id} issue_identifier=#{identifier}; stopping active agent" + ) + + _ -> + Logger.info( + "Issue no longer visible during running-state refresh: issue_id=#{issue_id}; stopping active agent" + ) + end + end + + defp log_missing_running_issue(_state, _issue_id), do: :ok + defp refresh_running_issue_state(%State{} = state, %Issue{} = issue) do case Map.get(state.running, issue.id) do %{issue: _} = running_entry -> diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index fb9311d2..791471f9 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -350,6 +350,84 @@ defmodule SymphonyElixir.CoreTest do end end + test "missing running issues stop active agents without cleaning the workspace" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-missing-running-reconcile-#{System.unique_integer([:positive])}" + ) + + previous_memory_issues = Application.get_env(:symphony_elixir, :memory_tracker_issues) + issue_id = "issue-missing" + issue_identifier = "MT-557" + + try do + write_workflow_file!(Workflow.workflow_file_path(), + tracker_kind: "memory", + workspace_root: test_root, + tracker_active_states: ["Todo", "In Progress", "In Review"], + tracker_terminal_states: ["Closed", "Cancelled", "Canceled", "Duplicate"], + poll_interval_ms: 30_000 + ) + + Application.put_env(:symphony_elixir, :memory_tracker_issues, []) + + orchestrator_name = Module.concat(__MODULE__, :MissingRunningIssueOrchestrator) + {:ok, pid} = Orchestrator.start_link(name: orchestrator_name) + + on_exit(fn -> + restore_app_env(:memory_tracker_issues, previous_memory_issues) + + if Process.alive?(pid) do + Process.exit(pid, :normal) + end + end) + + Process.sleep(50) + + assert {:ok, workspace} = + SymphonyElixir.PathSafety.canonicalize(Path.join(test_root, issue_identifier)) + + File.mkdir_p!(workspace) + + agent_pid = + spawn(fn -> + receive do + :stop -> :ok + end + end) + + initial_state = :sys.get_state(pid) + + running_entry = %{ + pid: agent_pid, + ref: nil, + identifier: issue_identifier, + issue: %Issue{id: issue_id, state: "In Progress", identifier: issue_identifier}, + started_at: DateTime.utc_now() + } + + :sys.replace_state(pid, fn _ -> + initial_state + |> Map.put(:running, %{issue_id => running_entry}) + |> Map.put(:claimed, MapSet.new([issue_id])) + |> Map.put(:retry_attempts, %{}) + end) + + send(pid, :tick) + Process.sleep(100) + state = :sys.get_state(pid) + + refute Map.has_key?(state.running, issue_id) + refute MapSet.member?(state.claimed, issue_id) + refute Process.alive?(agent_pid) + assert File.exists?(workspace) + after + restore_app_env(:memory_tracker_issues, previous_memory_issues) + File.rm_rf(test_root) + end + end + test "reconcile updates running issue state for active issues" do issue_id = "issue-3" @@ -632,6 +710,9 @@ defmodule SymphonyElixir.CoreTest do assert remaining_ms <= max_remaining_ms end + defp restore_app_env(key, nil), do: Application.delete_env(:symphony_elixir, key) + defp restore_app_env(key, value), do: Application.put_env(:symphony_elixir, key, value) + test "fetch issues by states with empty state set is a no-op" do assert {:ok, []} = Client.fetch_issues_by_states([]) end @@ -1338,6 +1419,7 @@ defmodule SymphonyElixir.CoreTest do } assert {:ok, _result} = AppServer.run(workspace, "Fix workspace start args", issue) + assert {:ok, canonical_workspace} = SymphonyElixir.PathSafety.canonicalize(workspace) trace = File.read!(trace_file) lines = String.split(trace, "\n", trim: true) @@ -1365,7 +1447,7 @@ defmodule SymphonyElixir.CoreTest do payload["method"] == "thread/start" && get_in(payload, ["params", "approvalPolicy"]) == expected_approval_policy && get_in(payload, ["params", "sandbox"]) == "workspace-write" && - get_in(payload, ["params", "cwd"]) == Path.expand(workspace) + get_in(payload, ["params", "cwd"]) == canonical_workspace end) else false @@ -1374,7 +1456,7 @@ defmodule SymphonyElixir.CoreTest do expected_turn_sandbox_policy = %{ "type" => "workspaceWrite", - "writableRoots" => [Path.expand(workspace)], + "writableRoots" => [canonical_workspace], "readOnlyAccess" => %{"type" => "fullAccess"}, "networkAccess" => false, "excludeTmpdirEnvVar" => false, @@ -1396,7 +1478,7 @@ defmodule SymphonyElixir.CoreTest do } payload["method"] == "turn/start" && - get_in(payload, ["params", "cwd"]) == Path.expand(workspace) && + get_in(payload, ["params", "cwd"]) == canonical_workspace && get_in(payload, ["params", "approvalPolicy"]) == expected_approval_policy && get_in(payload, ["params", "sandboxPolicy"]) == expected_turn_sandbox_policy end) @@ -1551,6 +1633,9 @@ defmodule SymphonyElixir.CoreTest do File.chmod!(codex_binary, 0o755) + workspace_cache = Path.join(Path.expand(workspace), ".cache") + File.mkdir_p!(workspace_cache) + write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root, codex_command: "#{codex_binary} app-server", @@ -1558,7 +1643,7 @@ defmodule SymphonyElixir.CoreTest do codex_thread_sandbox: "workspace-write", codex_turn_sandbox_policy: %{ type: "workspaceWrite", - writableRoots: [Path.expand(workspace), Path.join(Path.expand(workspace_root), ".cache")] + writableRoots: [Path.expand(workspace), workspace_cache] } ) @@ -1573,6 +1658,8 @@ defmodule SymphonyElixir.CoreTest do } assert {:ok, _result} = AppServer.run(workspace, "Fix workspace start args", issue) + assert {:ok, canonical_workspace} = SymphonyElixir.PathSafety.canonicalize(workspace) + assert {:ok, canonical_workspace_cache} = SymphonyElixir.PathSafety.canonicalize(workspace_cache) lines = File.read!(trace_file) |> String.split("\n", trim: true) @@ -1592,8 +1679,12 @@ defmodule SymphonyElixir.CoreTest do end) expected_turn_policy = %{ + "excludeSlashTmp" => false, + "excludeTmpdirEnvVar" => false, + "networkAccess" => false, + "readOnlyAccess" => %{"type" => "fullAccess"}, "type" => "workspaceWrite", - "writableRoots" => [Path.expand(workspace), Path.join(Path.expand(workspace_root), ".cache")] + "writableRoots" => [canonical_workspace, canonical_workspace_cache] } assert Enum.any?(lines, fn line -> From 6513a7eac410e8fb5b4aaf88e1d4ca997bfb97ca Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:42:48 -0700 Subject: [PATCH 5/8] docs(elixir): clarify last-known-good workflow reloads Summary: - Updated the Elixir README to distinguish startup failures from runtime workflow reload failures. - Documented that reload errors keep the last known good workflow in service until the file is fixed. - Included the formatter-only cleanup required by the final validation pass. Rationale: - The runtime intentionally keeps operating on the last known good workflow after a bad reload. - The docs should match the implemented and tested behavior rather than describing a full scheduling halt on reload errors. Tests: - mix test - mix format --check-formatted - mix specs.check Co-authored-by: Codex --- elixir/README.md | 4 +++- elixir/lib/symphony_elixir/codex/app_server.ex | 3 +-- elixir/lib/symphony_elixir/config/schema.ex | 4 +--- elixir/lib/symphony_elixir/orchestrator.ex | 10 ++++------ 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/elixir/README.md b/elixir/README.md index 3f711587..c96047b1 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -144,7 +144,9 @@ codex: command: "$CODEX_BIN app-server --model gpt-5.3-codex" ``` -- If `WORKFLOW.md` is missing or has invalid YAML, startup and scheduling are halted until fixed. +- If `WORKFLOW.md` is missing or has invalid YAML at startup, Symphony does not boot. +- If a later reload fails, Symphony keeps running with the last known good workflow and logs the + reload error until the file is fixed. - `server.port` or CLI `--port` enables the optional Phoenix LiveView dashboard and JSON API at `/`, `/api/v1/state`, `/api/v1/`, and `/api/v1/refresh`. diff --git a/elixir/lib/symphony_elixir/codex/app_server.ex b/elixir/lib/symphony_elixir/codex/app_server.ex index 86bfff5a..ef790224 100644 --- a/elixir/lib/symphony_elixir/codex/app_server.ex +++ b/elixir/lib/symphony_elixir/codex/app_server.ex @@ -160,8 +160,7 @@ defmodule SymphonyElixir.Codex.AppServer do {:error, {:invalid_workspace_cwd, :symlink_escape, expanded_workspace, canonical_root}} true -> - {:error, - {:invalid_workspace_cwd, :outside_workspace_root, canonical_workspace, canonical_root}} + {:error, {:invalid_workspace_cwd, :outside_workspace_root, canonical_workspace, canonical_root}} end else {:error, {:path_canonicalize_failed, path, reason}} -> diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 87dd481f..3bf19100 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -552,9 +552,7 @@ defmodule SymphonyElixir.Config.Schema do {:ok, canonical_root} true -> - {:error, - {:unsafe_turn_sandbox_policy, - {:writable_root_outside_workspace, canonical_root, canonical_workspace_root}}} + {:error, {:unsafe_turn_sandbox_policy, {:writable_root_outside_workspace, canonical_root, canonical_workspace_root}}} end end end diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index 49e40349..36ada1dc 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -74,6 +74,7 @@ defmodule SymphonyElixir.Orchestrator do def handle_info({:tick, tick_token}, %{tick_token: tick_token} = state) when is_reference(tick_token) do state = refresh_runtime_config(state) + state = %{ state | poll_check_in_progress: true, @@ -91,6 +92,7 @@ defmodule SymphonyElixir.Orchestrator do def handle_info(:tick, state) do state = refresh_runtime_config(state) + state = %{ state | poll_check_in_progress: true, @@ -364,14 +366,10 @@ defmodule SymphonyElixir.Orchestrator do defp log_missing_running_issue(%State{} = state, issue_id) when is_binary(issue_id) do case Map.get(state.running, issue_id) do %{identifier: identifier} -> - Logger.info( - "Issue no longer visible during running-state refresh: issue_id=#{issue_id} issue_identifier=#{identifier}; stopping active agent" - ) + Logger.info("Issue no longer visible during running-state refresh: issue_id=#{issue_id} issue_identifier=#{identifier}; stopping active agent") _ -> - Logger.info( - "Issue no longer visible during running-state refresh: issue_id=#{issue_id}; stopping active agent" - ) + Logger.info("Issue no longer visible during running-state refresh: issue_id=#{issue_id}; stopping active agent") end end From 10b55944308bf549433c6611e9a87a1f8073d767 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:48:53 -0700 Subject: [PATCH 6/8] test(elixir): restore make all coverage gate Summary: - refactor writable root validation into smaller helpers that satisfy Credo without changing runtime behavior - remove the unreachable empty-path branch from PathSafety - add runtime sandbox and path safety tests to cover the remaining validation and error branches Rationale: - make all enforces formatting, lint, and 100 percent coverage - the sandbox hardening changes introduced new branches that were not exercised by the existing suite Tests: - make all Co-authored-by: Codex --- elixir/lib/symphony_elixir/config/schema.ex | 48 ++++++++---- elixir/lib/symphony_elixir/path_safety.ex | 6 +- .../workspace_and_config_test.exs | 73 +++++++++++++++++++ 3 files changed, 108 insertions(+), 19 deletions(-) diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 3bf19100..7253def1 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -538,28 +538,46 @@ defmodule SymphonyElixir.Config.Schema do end defp validate_writable_root(root, canonical_workspace_root) when is_binary(root) do - if Path.type(root) != :absolute do - {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, root}}} + with :ok <- validate_absolute_writable_root(root), + {:ok, canonical_root} <- PathSafety.canonicalize(root), + :ok <- validate_writable_root_boundary(canonical_root, canonical_workspace_root) do + {:ok, canonical_root} + end + end + + defp validate_writable_root(root, _canonical_workspace_root) do + {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, root}}} + end + + defp validate_absolute_writable_root(root) when is_binary(root) do + if Path.type(root) == :absolute do + :ok else - with {:ok, canonical_root} <- PathSafety.canonicalize(root) do - workspace_prefix = canonical_workspace_root <> "/" + {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, root}}} + end + end - cond do - canonical_root == canonical_workspace_root -> - {:ok, canonical_root} + defp validate_writable_root_boundary(canonical_root, canonical_workspace_root) + when is_binary(canonical_root) and is_binary(canonical_workspace_root) do + workspace_prefix = canonical_workspace_root <> "/" - String.starts_with?(canonical_root <> "/", workspace_prefix) -> - {:ok, canonical_root} + cond do + canonical_root == canonical_workspace_root -> + :ok - true -> - {:error, {:unsafe_turn_sandbox_policy, {:writable_root_outside_workspace, canonical_root, canonical_workspace_root}}} - end - end + String.starts_with?(canonical_root <> "/", workspace_prefix) -> + :ok + + true -> + writable_root_outside_workspace_error(canonical_root, canonical_workspace_root) end end - defp validate_writable_root(root, _canonical_workspace_root) do - {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, root}}} + defp writable_root_outside_workspace_error(canonical_root, canonical_workspace_root) + when is_binary(canonical_root) and is_binary(canonical_workspace_root) do + reason = {:writable_root_outside_workspace, canonical_root, canonical_workspace_root} + + {:error, {:unsafe_turn_sandbox_policy, reason}} end defp format_errors(changeset) do diff --git a/elixir/lib/symphony_elixir/path_safety.ex b/elixir/lib/symphony_elixir/path_safety.ex index c12ce00f..fca59887 100644 --- a/elixir/lib/symphony_elixir/path_safety.ex +++ b/elixir/lib/symphony_elixir/path_safety.ex @@ -16,10 +16,8 @@ defmodule SymphonyElixir.PathSafety do end defp split_absolute_path(path) when is_binary(path) do - case Path.split(path) do - [root | segments] -> {root, segments} - [] -> {"/", []} - end + [root | segments] = Path.split(path) + {root, segments} end defp resolve_segments(root, resolved_segments, []), do: {:ok, join_path(root, resolved_segments)} diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 55a71425..fb548dac 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -1091,6 +1091,79 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do end end + test "path safety returns errors for invalid path segments" do + invalid_segment = String.duplicate("a", 300) + path = Path.join(System.tmp_dir!(), invalid_segment) + expanded_path = Path.expand(path) + + assert {:error, {:path_canonicalize_failed, ^expanded_path, :enametoolong}} = + SymphonyElixir.PathSafety.canonicalize(path) + end + + test "runtime sandbox policy resolution covers valid and malformed branches" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-runtime-sandbox-branches-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + issue_workspace = Path.join(workspace_root, "MT-101") + + File.mkdir_p!(issue_workspace) + + write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root) + + settings = Config.settings!() + + assert {:ok, canonical_workspace_root} = + SymphonyElixir.PathSafety.canonicalize(workspace_root) + + assert {:ok, default_policy} = Schema.resolve_runtime_turn_sandbox_policy(settings) + assert default_policy["type"] == "workspaceWrite" + assert default_policy["writableRoots"] == [canonical_workspace_root] + + read_only_settings = %{ + settings + | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "readOnly"}} + } + + assert {:ok, %{"type" => "readOnly"}} = + Schema.resolve_runtime_turn_sandbox_policy(read_only_settings, issue_workspace) + + missing_writable_roots_settings = %{ + settings + | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "workspaceWrite"}} + } + + assert {:error, {:unsafe_turn_sandbox_policy, :missing_writable_roots}} = + Schema.resolve_runtime_turn_sandbox_policy( + missing_writable_roots_settings, + issue_workspace + ) + + invalid_writable_root_settings = %{ + settings + | codex: %{ + settings.codex + | turn_sandbox_policy: %{"type" => "workspaceWrite", "writableRoots" => [123]} + } + } + + assert {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, 123}}} = + Schema.resolve_runtime_turn_sandbox_policy( + invalid_writable_root_settings, + issue_workspace + ) + + assert {:error, {:unsafe_turn_sandbox_policy, {:invalid_workspace_root, 123}}} = + Schema.resolve_runtime_turn_sandbox_policy(settings, 123) + after + File.rm_rf(test_root) + end + end + test "workflow prompt is used when building base prompt" do workflow_prompt = "Workflow prompt body used as codex instruction." From cca0ea450eac3aafce48a9f290dcff5f0e4b7851 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 11:54:04 -0700 Subject: [PATCH 7/8] fix(elixir): trust explicit writable roots Summary: - allow custom workspaceWrite writableRoots outside the issue workspace while still requiring absolute canonicalized paths - keep runtime rejection for relative writable roots and networkAccess-enabled policies - update tests and docs to reflect the Model B contract for trusted operator policy Rationale: - Symphony is operating in a trusted local automation model rather than a strict isolation harness - the old boundary check added complexity and rejected legitimate shared caches or other explicit host paths Tests: - make all Co-authored-by: Codex --- elixir/README.md | 5 +- elixir/lib/symphony_elixir/agent_runner.ex | 2 +- elixir/lib/symphony_elixir/config/schema.ex | 50 +++++-------------- .../workspace_and_config_test.exs | 21 ++++++-- 4 files changed, 34 insertions(+), 44 deletions(-) diff --git a/elixir/README.md b/elixir/README.md index c96047b1..0cf80365 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -14,7 +14,7 @@ This directory contains the current Elixir/OTP implementation of Symphony, based ## How it works 1. Polls Linear for candidate work -2. Creates an isolated workspace per issue +2. Creates a workspace per issue 3. Launches Codex in [App Server mode](https://developers.openai.com/codex/app-server/) inside the workspace 4. Sends a workflow prompt to Codex @@ -114,6 +114,9 @@ Notes: - `codex.approval_policy` defaults to `{"reject":{"sandbox_approval":true,"rules":true,"mcp_elicitations":true}}` - `codex.thread_sandbox` defaults to `workspace-write` - `codex.turn_sandbox_policy` defaults to a `workspaceWrite` policy rooted at the current issue workspace +- If you set `codex.turn_sandbox_policy.writableRoots` explicitly, Symphony treats those paths as + trusted operator policy. They must still be absolute paths, but they may point outside the issue + workspace and will be canonicalized before use. - Supported `codex.approval_policy` values depend on the targeted Codex app-server version. In the current local Codex schema, string values include `untrusted`, `on-failure`, `on-request`, and `never`, and object-form `reject` is also supported. - Supported `codex.thread_sandbox` values: `read-only`, `workspace-write`, `danger-full-access`. - Supported `codex.turn_sandbox_policy.type` values: `dangerFullAccess`, `readOnly`, diff --git a/elixir/lib/symphony_elixir/agent_runner.ex b/elixir/lib/symphony_elixir/agent_runner.ex index 14635060..482d5476 100644 --- a/elixir/lib/symphony_elixir/agent_runner.ex +++ b/elixir/lib/symphony_elixir/agent_runner.ex @@ -1,6 +1,6 @@ defmodule SymphonyElixir.AgentRunner do @moduledoc """ - Executes a single Linear issue in an isolated workspace with Codex. + Executes a single Linear issue in its workspace with Codex. """ require Logger diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 7253def1..57ce8d56 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -471,10 +471,10 @@ defmodule SymphonyElixir.Config.Schema do defp validate_runtime_turn_sandbox_policy(policy, workspace_root) when is_map(policy) and is_binary(workspace_root) do - with {:ok, canonical_workspace_root} <- PathSafety.canonicalize(workspace_root) do + with {:ok, _canonical_workspace_root} <- PathSafety.canonicalize(workspace_root) do case Map.get(policy, "type") do "workspaceWrite" -> - validate_workspace_write_policy(policy, canonical_workspace_root) + validate_workspace_write_policy(policy) "readOnly" -> validate_read_only_policy(policy) @@ -489,10 +489,9 @@ defmodule SymphonyElixir.Config.Schema do {:error, {:unsafe_turn_sandbox_policy, {:invalid_workspace_root, workspace_root}}} end - defp validate_workspace_write_policy(policy, canonical_workspace_root) do + defp validate_workspace_write_policy(policy) do with :ok <- validate_network_access_disabled(policy), - {:ok, writable_roots} <- - validate_writable_roots(Map.get(policy, "writableRoots"), canonical_workspace_root) do + {:ok, writable_roots} <- validate_writable_roots(Map.get(policy, "writableRoots")) do {:ok, policy |> Map.put("type", "workspaceWrite") @@ -518,11 +517,10 @@ defmodule SymphonyElixir.Config.Schema do end end - defp validate_writable_roots(writable_roots, canonical_workspace_root) - when is_list(writable_roots) and writable_roots != [] do + defp validate_writable_roots(writable_roots) when is_list(writable_roots) and writable_roots != [] do writable_roots |> Enum.reduce_while({:ok, []}, fn root, {:ok, acc} -> - case validate_writable_root(root, canonical_workspace_root) do + case validate_writable_root(root) do {:ok, canonical_root} -> {:cont, {:ok, [canonical_root | acc]}} {:error, reason} -> {:halt, {:error, reason}} end @@ -533,19 +531,18 @@ defmodule SymphonyElixir.Config.Schema do end end - defp validate_writable_roots(_writable_roots, _canonical_workspace_root) do + defp validate_writable_roots(_writable_roots) do {:error, {:unsafe_turn_sandbox_policy, :missing_writable_roots}} end - defp validate_writable_root(root, canonical_workspace_root) when is_binary(root) do - with :ok <- validate_absolute_writable_root(root), - {:ok, canonical_root} <- PathSafety.canonicalize(root), - :ok <- validate_writable_root_boundary(canonical_root, canonical_workspace_root) do - {:ok, canonical_root} + defp validate_writable_root(root) when is_binary(root) do + case validate_absolute_writable_root(root) do + :ok -> PathSafety.canonicalize(root) + {:error, reason} -> {:error, reason} end end - defp validate_writable_root(root, _canonical_workspace_root) do + defp validate_writable_root(root) do {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, root}}} end @@ -557,29 +554,6 @@ defmodule SymphonyElixir.Config.Schema do end end - defp validate_writable_root_boundary(canonical_root, canonical_workspace_root) - when is_binary(canonical_root) and is_binary(canonical_workspace_root) do - workspace_prefix = canonical_workspace_root <> "/" - - cond do - canonical_root == canonical_workspace_root -> - :ok - - String.starts_with?(canonical_root <> "/", workspace_prefix) -> - :ok - - true -> - writable_root_outside_workspace_error(canonical_root, canonical_workspace_root) - end - end - - defp writable_root_outside_workspace_error(canonical_root, canonical_workspace_root) - when is_binary(canonical_root) and is_binary(canonical_workspace_root) do - reason = {:writable_root_outside_workspace, canonical_root, canonical_workspace_root} - - {:error, {:unsafe_turn_sandbox_policy, reason}} - end - defp format_errors(changeset) do changeset |> traverse_errors(&translate_error/1) diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index fb548dac..0fd8697f 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -1039,7 +1039,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } end - test "runtime sandbox policy resolution rejects unsafe custom policies" do + test "runtime sandbox policy resolution accepts trusted writable roots and rejects malformed policies" do test_root = Path.join( System.tmp_dir!(), @@ -1049,20 +1049,33 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do try do workspace_root = Path.join(test_root, "workspaces") issue_workspace = Path.join(workspace_root, "MT-100") + shared_root = Path.join(test_root, "shared-cache") cache_root = Path.join(issue_workspace, "cache") + File.mkdir_p!(shared_root) File.mkdir_p!(cache_root) write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root, codex_turn_sandbox_policy: %{ type: "workspaceWrite", - writableRoots: [workspace_root] + writableRoots: [shared_root] } ) - assert {:error, {:unsafe_turn_sandbox_policy, {:writable_root_outside_workspace, _, _}}} = - Config.codex_runtime_settings(issue_workspace) + assert {:ok, canonical_shared_root} = + SymphonyElixir.PathSafety.canonicalize(shared_root) + + assert {:ok, runtime_settings} = Config.codex_runtime_settings(issue_workspace) + + assert runtime_settings.turn_sandbox_policy == %{ + "type" => "workspaceWrite", + "writableRoots" => [canonical_shared_root], + "readOnlyAccess" => %{"type" => "fullAccess"}, + "networkAccess" => false, + "excludeTmpdirEnvVar" => false, + "excludeSlashTmp" => false + } write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root, From 807eec8b700ea60286c7d1cd8cdf0527d96278ff Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Tue, 10 Mar 2026 12:41:44 -0700 Subject: [PATCH 8/8] fix(elixir): pass through turn policies and paginate refreshes Summary: - pass explicit codex turn sandbox policies through unchanged and keep local policy synthesis only for the default workspaceWrite case - add integration coverage asserting AppServer forwards explicit sandbox policies without local rewriting - paginate Linear issue-state refreshes by id in 50-item batches and preserve requested issue ordering across merged results Rationale: - Symphony should not impose local semantics on explicit Codex sandbox policy maps beyond generating its own default when omitted - issue-state reconciliation depends on seeing the full running-id set, so single-page by-id fetches could wrongly stop healthy workers Tests: - make all Co-authored-by: Codex --- elixir/README.md | 8 +- elixir/lib/symphony_elixir/config/schema.ex | 93 ++---------- elixir/lib/symphony_elixir/linear/client.ex | 62 +++++++- .../test/symphony_elixir/app_server_test.exs | 107 ++++++++++++++ elixir/test/symphony_elixir/core_test.exs | 8 +- .../workspace_and_config_test.exs | 138 +++++++++--------- 6 files changed, 248 insertions(+), 168 deletions(-) diff --git a/elixir/README.md b/elixir/README.md index 0cf80365..004b5943 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -114,13 +114,11 @@ Notes: - `codex.approval_policy` defaults to `{"reject":{"sandbox_approval":true,"rules":true,"mcp_elicitations":true}}` - `codex.thread_sandbox` defaults to `workspace-write` - `codex.turn_sandbox_policy` defaults to a `workspaceWrite` policy rooted at the current issue workspace -- If you set `codex.turn_sandbox_policy.writableRoots` explicitly, Symphony treats those paths as - trusted operator policy. They must still be absolute paths, but they may point outside the issue - workspace and will be canonicalized before use. - Supported `codex.approval_policy` values depend on the targeted Codex app-server version. In the current local Codex schema, string values include `untrusted`, `on-failure`, `on-request`, and `never`, and object-form `reject` is also supported. - Supported `codex.thread_sandbox` values: `read-only`, `workspace-write`, `danger-full-access`. -- Supported `codex.turn_sandbox_policy.type` values: `dangerFullAccess`, `readOnly`, - `externalSandbox`, `workspaceWrite`. +- When `codex.turn_sandbox_policy` is set explicitly, Symphony passes the map through to Codex + unchanged. Compatibility then depends on the targeted Codex app-server version rather than local + Symphony validation. - `agent.max_turns` caps how many back-to-back Codex turns Symphony will run in a single agent invocation when a turn completes normally but the issue is still in an active state. Default: `20`. - If the Markdown body is blank, Symphony uses a default prompt template that includes the issue diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 57ce8d56..c9ad948f 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -283,11 +283,13 @@ defmodule SymphonyElixir.Config.Schema do @spec resolve_runtime_turn_sandbox_policy(%__MODULE__{}, Path.t() | nil) :: {:ok, map()} | {:error, term()} def resolve_runtime_turn_sandbox_policy(settings, workspace \\ nil) do - workspace_root = workspace || settings.workspace.root + case settings.codex.turn_sandbox_policy do + %{} = policy -> + {:ok, policy} - settings - |> resolve_turn_sandbox_policy(workspace_root) - |> validate_runtime_turn_sandbox_policy(workspace_root) + _ -> + default_runtime_turn_sandbox_policy(workspace || settings.workspace.root) + end end @spec normalize_issue_state(String.t()) :: String.t() @@ -469,91 +471,16 @@ defmodule SymphonyElixir.Config.Schema do } end - defp validate_runtime_turn_sandbox_policy(policy, workspace_root) - when is_map(policy) and is_binary(workspace_root) do - with {:ok, _canonical_workspace_root} <- PathSafety.canonicalize(workspace_root) do - case Map.get(policy, "type") do - "workspaceWrite" -> - validate_workspace_write_policy(policy) - - "readOnly" -> - validate_read_only_policy(policy) - - other -> - {:error, {:unsafe_turn_sandbox_policy, {:unsupported_type, other}}} - end + defp default_runtime_turn_sandbox_policy(workspace_root) when is_binary(workspace_root) do + with {:ok, canonical_workspace_root} <- PathSafety.canonicalize(workspace_root) do + {:ok, default_turn_sandbox_policy(canonical_workspace_root)} end end - defp validate_runtime_turn_sandbox_policy(_policy, workspace_root) do + defp default_runtime_turn_sandbox_policy(workspace_root) do {:error, {:unsafe_turn_sandbox_policy, {:invalid_workspace_root, workspace_root}}} end - defp validate_workspace_write_policy(policy) do - with :ok <- validate_network_access_disabled(policy), - {:ok, writable_roots} <- validate_writable_roots(Map.get(policy, "writableRoots")) do - {:ok, - policy - |> Map.put("type", "workspaceWrite") - |> Map.put("writableRoots", writable_roots) - |> Map.put_new("readOnlyAccess", %{"type" => "fullAccess"}) - |> Map.put_new("networkAccess", false) - |> Map.put_new("excludeTmpdirEnvVar", false) - |> Map.put_new("excludeSlashTmp", false)} - end - end - - defp validate_read_only_policy(policy) do - with :ok <- validate_network_access_disabled(policy) do - {:ok, Map.put(policy, "type", "readOnly")} - end - end - - defp validate_network_access_disabled(policy) do - if Map.get(policy, "networkAccess") == true do - {:error, {:unsafe_turn_sandbox_policy, :network_access_enabled}} - else - :ok - end - end - - defp validate_writable_roots(writable_roots) when is_list(writable_roots) and writable_roots != [] do - writable_roots - |> Enum.reduce_while({:ok, []}, fn root, {:ok, acc} -> - case validate_writable_root(root) do - {:ok, canonical_root} -> {:cont, {:ok, [canonical_root | acc]}} - {:error, reason} -> {:halt, {:error, reason}} - end - end) - |> case do - {:ok, canonical_roots} -> {:ok, Enum.reverse(canonical_roots)} - {:error, reason} -> {:error, reason} - end - end - - defp validate_writable_roots(_writable_roots) do - {:error, {:unsafe_turn_sandbox_policy, :missing_writable_roots}} - end - - defp validate_writable_root(root) when is_binary(root) do - case validate_absolute_writable_root(root) do - :ok -> PathSafety.canonicalize(root) - {:error, reason} -> {:error, reason} - end - end - - defp validate_writable_root(root) do - {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, root}}} - end - - defp validate_absolute_writable_root(root) when is_binary(root) do - if Path.type(root) == :absolute do - :ok - else - {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, root}}} - end - end - defp format_errors(changeset) do changeset |> traverse_errors(&translate_error/1) diff --git a/elixir/lib/symphony_elixir/linear/client.ex b/elixir/lib/symphony_elixir/linear/client.ex index 0ff290fe..1a196496 100644 --- a/elixir/lib/symphony_elixir/linear/client.ex +++ b/elixir/lib/symphony_elixir/linear/client.ex @@ -220,6 +220,22 @@ defmodule SymphonyElixir.Linear.Client do |> finalize_paginated_issues() end + @doc false + @spec fetch_issue_states_by_ids_for_test([String.t()], (String.t(), map() -> {:ok, map()} | {:error, term()})) :: + {:ok, [Issue.t()]} | {:error, term()} + def fetch_issue_states_by_ids_for_test(issue_ids, graphql_fun) + when is_list(issue_ids) and is_function(graphql_fun, 2) do + ids = Enum.uniq(issue_ids) + + case ids do + [] -> + {:ok, []} + + ids -> + do_fetch_issue_states(ids, nil, graphql_fun) + end + end + defp do_fetch_by_states(project_slug, state_names, assignee_filter) do do_fetch_by_states_page(project_slug, state_names, assignee_filter, nil, []) end @@ -256,19 +272,57 @@ defmodule SymphonyElixir.Linear.Client do defp finalize_paginated_issues(acc_issues) when is_list(acc_issues), do: Enum.reverse(acc_issues) defp do_fetch_issue_states(ids, assignee_filter) do - case graphql(@query_by_ids, %{ - ids: ids, - first: Enum.min([length(ids), @issue_page_size]), + do_fetch_issue_states(ids, assignee_filter, &graphql/2) + end + + defp do_fetch_issue_states(ids, assignee_filter, graphql_fun) + when is_list(ids) and is_function(graphql_fun, 2) do + issue_order_index = issue_order_index(ids) + do_fetch_issue_states_page(ids, assignee_filter, graphql_fun, [], issue_order_index) + end + + defp do_fetch_issue_states_page([], _assignee_filter, _graphql_fun, acc_issues, issue_order_index) do + acc_issues + |> finalize_paginated_issues() + |> sort_issues_by_requested_ids(issue_order_index) + |> then(&{:ok, &1}) + end + + defp do_fetch_issue_states_page(ids, assignee_filter, graphql_fun, acc_issues, issue_order_index) do + {batch_ids, rest_ids} = Enum.split(ids, @issue_page_size) + + case graphql_fun.(@query_by_ids, %{ + ids: batch_ids, + first: length(batch_ids), relationFirst: @issue_page_size }) do {:ok, body} -> - decode_linear_response(body, assignee_filter) + with {:ok, issues} <- decode_linear_response(body, assignee_filter) do + updated_acc = prepend_page_issues(issues, acc_issues) + do_fetch_issue_states_page(rest_ids, assignee_filter, graphql_fun, updated_acc, issue_order_index) + end {:error, reason} -> {:error, reason} end end + defp issue_order_index(ids) when is_list(ids) do + ids + |> Enum.with_index() + |> Map.new() + end + + defp sort_issues_by_requested_ids(issues, issue_order_index) + when is_list(issues) and is_map(issue_order_index) do + fallback_index = map_size(issue_order_index) + + Enum.sort_by(issues, fn + %Issue{id: issue_id} -> Map.get(issue_order_index, issue_id, fallback_index) + _ -> fallback_index + end) + end + defp build_graphql_payload(query, variables, operation_name) do %{ "query" => query, diff --git a/elixir/test/symphony_elixir/app_server_test.exs b/elixir/test/symphony_elixir/app_server_test.exs index 1a9ec88a..3b98c443 100644 --- a/elixir/test/symphony_elixir/app_server_test.exs +++ b/elixir/test/symphony_elixir/app_server_test.exs @@ -76,6 +76,113 @@ defmodule SymphonyElixir.AppServerTest do end end + test "app server passes explicit turn sandbox policies through unchanged" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-app-server-supported-turn-policies-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + workspace = Path.join(workspace_root, "MT-1001") + codex_binary = Path.join(test_root, "fake-codex") + trace_file = Path.join(test_root, "codex-supported-turn-policies.trace") + previous_trace = System.get_env("SYMP_TEST_CODEx_TRACE") + + on_exit(fn -> + if is_binary(previous_trace) do + System.put_env("SYMP_TEST_CODEx_TRACE", previous_trace) + else + System.delete_env("SYMP_TEST_CODEx_TRACE") + end + end) + + System.put_env("SYMP_TEST_CODEx_TRACE", trace_file) + File.mkdir_p!(workspace) + + File.write!(codex_binary, """ + #!/bin/sh + trace_file="${SYMP_TEST_CODEx_TRACE:-/tmp/codex-supported-turn-policies.trace}" + count=0 + + while IFS= read -r line; do + count=$((count + 1)) + printf 'JSON:%s\\n' "$line" >> "$trace_file" + + case "$count" in + 1) + printf '%s\\n' '{"id":1,"result":{}}' + ;; + 2) + printf '%s\\n' '{"id":2,"result":{"thread":{"id":"thread-1001"}}}' + ;; + 3) + printf '%s\\n' '{"id":3,"result":{"turn":{"id":"turn-1001"}}}' + ;; + 4) + printf '%s\\n' '{"method":"turn/completed"}' + exit 0 + ;; + *) + exit 0 + ;; + esac + done + """) + + File.chmod!(codex_binary, 0o755) + + issue = %Issue{ + id: "issue-supported-turn-policies", + identifier: "MT-1001", + title: "Validate explicit turn sandbox policy passthrough", + description: "Ensure runtime startup forwards configured turn sandbox policies unchanged", + state: "In Progress", + url: "https://example.org/issues/MT-1001", + labels: ["backend"] + } + + policy_cases = [ + %{"type" => "dangerFullAccess"}, + %{"type" => "externalSandbox", "profile" => "remote-ci"}, + %{"type" => "workspaceWrite", "writableRoots" => ["relative/path"], "networkAccess" => true}, + %{"type" => "futureSandbox", "nested" => %{"flag" => true}} + ] + + Enum.each(policy_cases, fn configured_policy -> + File.rm(trace_file) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + codex_command: "#{codex_binary} app-server", + codex_turn_sandbox_policy: configured_policy + ) + + assert {:ok, _result} = AppServer.run(workspace, "Validate supported turn policy", issue) + + trace = File.read!(trace_file) + lines = String.split(trace, "\n", trim: true) + + assert Enum.any?(lines, fn line -> + if String.starts_with?(line, "JSON:") do + line + |> String.trim_leading("JSON:") + |> Jason.decode!() + |> then(fn payload -> + payload["method"] == "turn/start" && + get_in(payload, ["params", "sandboxPolicy"]) == configured_policy + end) + else + false + end + end) + end) + after + File.rm_rf(test_root) + end + end + test "app server marks request-for-input events as a hard failure" do test_root = Path.join( diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index 791471f9..fa96b7b0 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -1658,8 +1658,6 @@ defmodule SymphonyElixir.CoreTest do } assert {:ok, _result} = AppServer.run(workspace, "Fix workspace start args", issue) - assert {:ok, canonical_workspace} = SymphonyElixir.PathSafety.canonicalize(workspace) - assert {:ok, canonical_workspace_cache} = SymphonyElixir.PathSafety.canonicalize(workspace_cache) lines = File.read!(trace_file) |> String.split("\n", trim: true) @@ -1679,12 +1677,8 @@ defmodule SymphonyElixir.CoreTest do end) expected_turn_policy = %{ - "excludeSlashTmp" => false, - "excludeTmpdirEnvVar" => false, - "networkAccess" => false, - "readOnlyAccess" => %{"type" => "fullAccess"}, "type" => "workspaceWrite", - "writableRoots" => [canonical_workspace, canonical_workspace_cache] + "writableRoots" => [Path.expand(workspace), workspace_cache] } assert Enum.any?(lines, fn line -> diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 0fd8697f..03a3c59e 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -386,6 +386,49 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert Enum.map(merged, & &1.identifier) == ["MT-1", "MT-2", "MT-3"] end + test "linear client paginates issue state fetches by id beyond one page" do + issue_ids = Enum.map(1..55, &"issue-#{&1}") + first_batch_ids = Enum.take(issue_ids, 50) + second_batch_ids = Enum.drop(issue_ids, 50) + + raw_issue = fn issue_id -> + suffix = String.replace_prefix(issue_id, "issue-", "") + + %{ + "id" => issue_id, + "identifier" => "MT-#{suffix}", + "title" => "Issue #{suffix}", + "description" => "Description #{suffix}", + "state" => %{"name" => "In Progress"}, + "labels" => %{"nodes" => []}, + "inverseRelations" => %{"nodes" => []} + } + end + + graphql_fun = fn query, variables -> + send(self(), {:fetch_issue_states_page, query, variables}) + + body = %{ + "data" => %{ + "issues" => %{ + "nodes" => Enum.map(variables.ids, raw_issue) + } + } + } + + {:ok, body} + end + + assert {:ok, issues} = Client.fetch_issue_states_by_ids_for_test(issue_ids, graphql_fun) + + assert Enum.map(issues, & &1.id) == issue_ids + + assert_receive {:fetch_issue_states_page, query, %{ids: ^first_batch_ids, first: 50, relationFirst: 50}} + assert query =~ "SymphonyLinearIssuesById" + + assert_receive {:fetch_issue_states_page, ^query, %{ids: ^second_batch_ids, first: 5, relationFirst: 50}} + end + test "linear client logs response bodies for non-200 graphql responses" do log = ExUnit.CaptureLog.capture_log(fn -> @@ -757,19 +800,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert config.codex.approval_policy == "on-request" assert config.codex.thread_sandbox == "workspace-write" - assert {:ok, canonical_explicit_workspace} = - SymphonyElixir.PathSafety.canonicalize(explicit_workspace) - - assert {:ok, canonical_explicit_cache} = - SymphonyElixir.PathSafety.canonicalize(explicit_cache) - assert Config.codex_turn_sandbox_policy(explicit_workspace) == %{ "type" => "workspaceWrite", - "writableRoots" => [canonical_explicit_workspace, canonical_explicit_cache], - "readOnlyAccess" => %{"type" => "fullAccess"}, - "networkAccess" => false, - "excludeTmpdirEnvVar" => false, - "excludeSlashTmp" => false + "writableRoots" => [explicit_workspace, explicit_cache] } write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: ",") @@ -836,9 +869,10 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert :ok = Config.validate!() - assert_raise ArgumentError, ~r/Invalid codex turn sandbox policy/, fn -> - Config.codex_turn_sandbox_policy() - end + assert Config.codex_turn_sandbox_policy() == %{ + "type" => "futureSandbox", + "nested" => %{"flag" => true} + } write_workflow_file!(Workflow.workflow_file_path(), codex_command: "codex app-server") assert Config.settings!().codex.command == "codex app-server" @@ -1039,7 +1073,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } end - test "runtime sandbox policy resolution accepts trusted writable roots and rejects malformed policies" do + test "runtime sandbox policy resolution passes explicit policies through unchanged" do test_root = Path.join( System.tmp_dir!(), @@ -1049,56 +1083,39 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do try do workspace_root = Path.join(test_root, "workspaces") issue_workspace = Path.join(workspace_root, "MT-100") - shared_root = Path.join(test_root, "shared-cache") - cache_root = Path.join(issue_workspace, "cache") - - File.mkdir_p!(shared_root) - File.mkdir_p!(cache_root) + File.mkdir_p!(issue_workspace) write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root, codex_turn_sandbox_policy: %{ type: "workspaceWrite", - writableRoots: [shared_root] + writableRoots: ["relative/path"], + networkAccess: true } ) - assert {:ok, canonical_shared_root} = - SymphonyElixir.PathSafety.canonicalize(shared_root) - assert {:ok, runtime_settings} = Config.codex_runtime_settings(issue_workspace) assert runtime_settings.turn_sandbox_policy == %{ "type" => "workspaceWrite", - "writableRoots" => [canonical_shared_root], - "readOnlyAccess" => %{"type" => "fullAccess"}, - "networkAccess" => false, - "excludeTmpdirEnvVar" => false, - "excludeSlashTmp" => false + "writableRoots" => ["relative/path"], + "networkAccess" => true } write_workflow_file!(Workflow.workflow_file_path(), workspace_root: workspace_root, codex_turn_sandbox_policy: %{ - type: "workspaceWrite", - writableRoots: ["relative/path"] + type: "futureSandbox", + nested: %{flag: true} } ) - assert {:error, {:unsafe_turn_sandbox_policy, {:relative_writable_root, "relative/path"}}} = - Config.codex_runtime_settings(issue_workspace) - - write_workflow_file!(Workflow.workflow_file_path(), - workspace_root: workspace_root, - codex_turn_sandbox_policy: %{ - type: "workspaceWrite", - writableRoots: [issue_workspace, cache_root], - networkAccess: true - } - ) + assert {:ok, runtime_settings} = Config.codex_runtime_settings(issue_workspace) - assert {:error, {:unsafe_turn_sandbox_policy, :network_access_enabled}} = - Config.codex_runtime_settings(issue_workspace) + assert runtime_settings.turn_sandbox_policy == %{ + "type" => "futureSandbox", + "nested" => %{"flag" => true} + } after File.rm_rf(test_root) end @@ -1113,7 +1130,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do SymphonyElixir.PathSafety.canonicalize(path) end - test "runtime sandbox policy resolution covers valid and malformed branches" do + test "runtime sandbox policy resolution defaults when omitted and ignores workspace for explicit policies" do test_root = Path.join( System.tmp_dir!(), @@ -1139,36 +1156,19 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do read_only_settings = %{ settings - | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "readOnly"}} + | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "readOnly", "networkAccess" => true}} } - assert {:ok, %{"type" => "readOnly"}} = - Schema.resolve_runtime_turn_sandbox_policy(read_only_settings, issue_workspace) + assert {:ok, %{"type" => "readOnly", "networkAccess" => true}} = + Schema.resolve_runtime_turn_sandbox_policy(read_only_settings, 123) - missing_writable_roots_settings = %{ + future_settings = %{ settings - | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "workspaceWrite"}} - } - - assert {:error, {:unsafe_turn_sandbox_policy, :missing_writable_roots}} = - Schema.resolve_runtime_turn_sandbox_policy( - missing_writable_roots_settings, - issue_workspace - ) - - invalid_writable_root_settings = %{ - settings - | codex: %{ - settings.codex - | turn_sandbox_policy: %{"type" => "workspaceWrite", "writableRoots" => [123]} - } + | codex: %{settings.codex | turn_sandbox_policy: %{"type" => "futureSandbox", "nested" => %{"flag" => true}}} } - assert {:error, {:unsafe_turn_sandbox_policy, {:invalid_writable_root, 123}}} = - Schema.resolve_runtime_turn_sandbox_policy( - invalid_writable_root_settings, - issue_workspace - ) + assert {:ok, %{"type" => "futureSandbox", "nested" => %{"flag" => true}}} = + Schema.resolve_runtime_turn_sandbox_policy(future_settings, 123) assert {:error, {:unsafe_turn_sandbox_policy, {:invalid_workspace_root, 123}}} = Schema.resolve_runtime_turn_sandbox_policy(settings, 123)