From aae8480a22d36ead302c64add47d70d59413633f Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Mon, 9 Mar 2026 15:58:42 -0700 Subject: [PATCH 1/2] refactor(config): replace nimble options with ecto schema Summary: - replace the WORKFLOW.md config parser with nested Ecto embedded schemas and field-level defaults - centralize env, path, and sandbox post-processing in a single schema pipeline - update config validation tests and orchestrator logging for the new strict invalid_workflow_config path Rationale: - NimbleOptions was carrying a large amount of custom extraction and fallback code that duplicated schema behavior - Ecto gives nested typed validation directly and makes the remaining config behavior easier to reason about - consolidating the config flow removes silent compatibility logic and makes failures explicit Tests: - cd elixir && mix test --warnings-as-errors - cd elixir && mix credo --strict Co-authored-by: Codex --- elixir/lib/symphony_elixir/config.ex | 861 ++---------------- elixir/lib/symphony_elixir/config/schema.ex | 490 ++++++++++ elixir/lib/symphony_elixir/orchestrator.ex | 12 +- elixir/mix.exs | 2 +- elixir/mix.lock | 1 + elixir/test/symphony_elixir/core_test.exs | 23 +- .../workspace_and_config_test.exs | 61 +- 7 files changed, 620 insertions(+), 830 deletions(-) create mode 100644 elixir/lib/symphony_elixir/config/schema.ex diff --git a/elixir/lib/symphony_elixir/config.ex b/elixir/lib/symphony_elixir/config.ex index 3a9f0d99..ba30fb4d 100644 --- a/elixir/lib/symphony_elixir/config.ex +++ b/elixir/lib/symphony_elixir/config.ex @@ -3,12 +3,9 @@ defmodule SymphonyElixir.Config do Runtime configuration loaded from `WORKFLOW.md`. """ - alias NimbleOptions + alias SymphonyElixir.Config.Schema alias SymphonyElixir.Workflow - @default_active_states ["Todo", "In Progress"] - @default_terminal_states ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] - @default_linear_endpoint "https://api.linear.app/graphql" @default_prompt_template """ You are working on a Linear issue. @@ -22,141 +19,6 @@ defmodule SymphonyElixir.Config do No description provided. {% endif %} """ - @default_poll_interval_ms 30_000 - @default_workspace_root Path.join(System.tmp_dir!(), "symphony_workspaces") - @default_hook_timeout_ms 60_000 - @default_max_concurrent_agents 10 - @default_agent_max_turns 20 - @default_max_retry_backoff_ms 300_000 - @default_codex_command "codex app-server" - @default_codex_turn_timeout_ms 3_600_000 - @default_codex_read_timeout_ms 5_000 - @default_codex_stall_timeout_ms 300_000 - @default_codex_approval_policy %{ - "reject" => %{ - "sandbox_approval" => true, - "rules" => true, - "mcp_elicitations" => true - } - } - @default_codex_thread_sandbox "workspace-write" - @default_observability_enabled true - @default_observability_refresh_ms 1_000 - @default_observability_render_interval_ms 16 - @default_server_host "127.0.0.1" - @workflow_options_schema NimbleOptions.new!( - tracker: [ - type: :map, - default: %{}, - keys: [ - kind: [type: {:or, [:string, nil]}, default: nil], - endpoint: [type: :string, default: @default_linear_endpoint], - api_key: [type: {:or, [:string, nil]}, default: nil], - project_slug: [type: {:or, [:string, nil]}, default: nil], - assignee: [type: {:or, [:string, nil]}, default: nil], - active_states: [ - type: {:list, :string}, - default: @default_active_states - ], - terminal_states: [ - type: {:list, :string}, - default: @default_terminal_states - ] - ] - ], - polling: [ - type: :map, - default: %{}, - keys: [ - interval_ms: [type: :integer, default: @default_poll_interval_ms] - ] - ], - workspace: [ - type: :map, - default: %{}, - keys: [ - root: [type: {:or, [:string, nil]}, default: @default_workspace_root] - ] - ], - agent: [ - type: :map, - default: %{}, - keys: [ - max_concurrent_agents: [ - type: :integer, - default: @default_max_concurrent_agents - ], - max_turns: [ - type: :pos_integer, - default: @default_agent_max_turns - ], - max_retry_backoff_ms: [ - type: :pos_integer, - default: @default_max_retry_backoff_ms - ], - max_concurrent_agents_by_state: [ - type: {:map, :string, :pos_integer}, - default: %{} - ] - ] - ], - codex: [ - type: :map, - default: %{}, - keys: [ - command: [type: :string, default: @default_codex_command], - turn_timeout_ms: [ - type: :integer, - default: @default_codex_turn_timeout_ms - ], - read_timeout_ms: [ - type: :integer, - default: @default_codex_read_timeout_ms - ], - stall_timeout_ms: [ - type: :integer, - default: @default_codex_stall_timeout_ms - ] - ] - ], - hooks: [ - type: :map, - default: %{}, - keys: [ - after_create: [type: {:or, [:string, nil]}, default: nil], - before_run: [type: {:or, [:string, nil]}, default: nil], - after_run: [type: {:or, [:string, nil]}, default: nil], - before_remove: [type: {:or, [:string, nil]}, default: nil], - timeout_ms: [type: :pos_integer, default: @default_hook_timeout_ms] - ] - ], - observability: [ - type: :map, - default: %{}, - keys: [ - dashboard_enabled: [ - type: :boolean, - default: @default_observability_enabled - ], - refresh_ms: [ - type: :integer, - default: @default_observability_refresh_ms - ], - render_interval_ms: [ - type: :integer, - default: @default_observability_render_interval_ms - ] - ] - ], - server: [ - type: :map, - default: %{}, - keys: [ - port: [type: {:or, [:non_neg_integer, nil]}, default: nil], - host: [type: :string, default: @default_server_host] - ] - ] - ) @type workflow_payload :: Workflow.loaded_workflow() @type tracker_kind :: String.t() | nil @@ -174,150 +36,81 @@ defmodule SymphonyElixir.Config do } @spec current_workflow() :: {:ok, workflow_payload()} | {:error, term()} - def current_workflow do - Workflow.current() - end + def current_workflow, do: Workflow.current() @spec tracker_kind() :: tracker_kind() - def tracker_kind do - get_in(validated_workflow_options(), [:tracker, :kind]) - end + def tracker_kind, do: get_setting!([:tracker, :kind]) @spec linear_endpoint() :: String.t() - def linear_endpoint do - get_in(validated_workflow_options(), [:tracker, :endpoint]) - end + def linear_endpoint, do: get_setting!([:tracker, :endpoint]) @spec linear_api_token() :: String.t() | nil - def linear_api_token do - validated_workflow_options() - |> get_in([:tracker, :api_key]) - |> resolve_env_value(System.get_env("LINEAR_API_KEY")) - |> normalize_secret_value() - end + def linear_api_token, do: get_setting!([:tracker, :api_key]) @spec linear_project_slug() :: String.t() | nil - def linear_project_slug do - get_in(validated_workflow_options(), [:tracker, :project_slug]) - end + def linear_project_slug, do: get_setting!([:tracker, :project_slug]) @spec linear_assignee() :: String.t() | nil - def linear_assignee do - validated_workflow_options() - |> get_in([:tracker, :assignee]) - |> resolve_env_value(System.get_env("LINEAR_ASSIGNEE")) - |> normalize_secret_value() - end + def linear_assignee, do: get_setting!([:tracker, :assignee]) @spec linear_active_states() :: [String.t()] - def linear_active_states do - get_in(validated_workflow_options(), [:tracker, :active_states]) - end + def linear_active_states, do: get_setting!([:tracker, :active_states]) @spec linear_terminal_states() :: [String.t()] - def linear_terminal_states do - get_in(validated_workflow_options(), [:tracker, :terminal_states]) - end + def linear_terminal_states, do: get_setting!([:tracker, :terminal_states]) @spec poll_interval_ms() :: pos_integer() - def poll_interval_ms do - get_in(validated_workflow_options(), [:polling, :interval_ms]) - end + def poll_interval_ms, do: get_setting!([:polling, :interval_ms]) @spec workspace_root() :: Path.t() - def workspace_root do - validated_workflow_options() - |> get_in([:workspace, :root]) - |> resolve_path_value(@default_workspace_root) - end + def workspace_root, do: get_setting!([:workspace, :root]) @spec workspace_hooks() :: workspace_hooks() - def workspace_hooks do - hooks = get_in(validated_workflow_options(), [:hooks]) - - %{ - after_create: Map.get(hooks, :after_create), - before_run: Map.get(hooks, :before_run), - after_run: Map.get(hooks, :after_run), - before_remove: Map.get(hooks, :before_remove), - timeout_ms: Map.get(hooks, :timeout_ms) - } - end + def workspace_hooks, do: get_setting!([:hooks]) @spec hook_timeout_ms() :: pos_integer() - def hook_timeout_ms do - get_in(validated_workflow_options(), [:hooks, :timeout_ms]) - end + def hook_timeout_ms, do: get_setting!([:hooks, :timeout_ms]) @spec max_concurrent_agents() :: pos_integer() - def max_concurrent_agents do - get_in(validated_workflow_options(), [:agent, :max_concurrent_agents]) - end + def max_concurrent_agents, do: get_setting!([:agent, :max_concurrent_agents]) @spec max_retry_backoff_ms() :: pos_integer() - def max_retry_backoff_ms do - get_in(validated_workflow_options(), [:agent, :max_retry_backoff_ms]) - end + def max_retry_backoff_ms, do: get_setting!([:agent, :max_retry_backoff_ms]) @spec agent_max_turns() :: pos_integer() - def agent_max_turns do - get_in(validated_workflow_options(), [:agent, :max_turns]) - end + def agent_max_turns, do: get_setting!([:agent, :max_turns]) @spec max_concurrent_agents_for_state(term()) :: pos_integer() def max_concurrent_agents_for_state(state_name) when is_binary(state_name) do - state_limits = get_in(validated_workflow_options(), [:agent, :max_concurrent_agents_by_state]) - global_limit = max_concurrent_agents() - Map.get(state_limits, normalize_issue_state(state_name), global_limit) + state_limits = get_setting!([:agent, :max_concurrent_agents_by_state]) + Map.get(state_limits, Schema.normalize_issue_state(state_name), max_concurrent_agents()) end def max_concurrent_agents_for_state(_state_name), do: max_concurrent_agents() @spec codex_command() :: String.t() - def codex_command do - get_in(validated_workflow_options(), [:codex, :command]) - end + def codex_command, do: get_setting!([:codex, :command]) @spec codex_turn_timeout_ms() :: pos_integer() - def codex_turn_timeout_ms do - get_in(validated_workflow_options(), [:codex, :turn_timeout_ms]) - end + def codex_turn_timeout_ms, do: get_setting!([:codex, :turn_timeout_ms]) @spec codex_approval_policy() :: String.t() | map() - def codex_approval_policy do - case resolve_codex_approval_policy() do - {:ok, approval_policy} -> approval_policy - {:error, _reason} -> @default_codex_approval_policy - end - end + def codex_approval_policy, do: get_setting!([:codex, :approval_policy]) @spec codex_thread_sandbox() :: String.t() - def codex_thread_sandbox do - case resolve_codex_thread_sandbox() do - {:ok, thread_sandbox} -> thread_sandbox - {:error, _reason} -> @default_codex_thread_sandbox - end - end + def codex_thread_sandbox, do: get_setting!([:codex, :thread_sandbox]) @spec codex_turn_sandbox_policy(Path.t() | nil) :: map() def codex_turn_sandbox_policy(workspace \\ nil) do - case resolve_codex_turn_sandbox_policy(workspace) do - {:ok, turn_sandbox_policy} -> turn_sandbox_policy - {:error, _reason} -> default_codex_turn_sandbox_policy(workspace) - end + validated_settings!() + |> Schema.resolve_turn_sandbox_policy(workspace) end @spec codex_read_timeout_ms() :: pos_integer() - def codex_read_timeout_ms do - get_in(validated_workflow_options(), [:codex, :read_timeout_ms]) - end + def codex_read_timeout_ms, do: get_setting!([:codex, :read_timeout_ms]) @spec codex_stall_timeout_ms() :: non_neg_integer() - def codex_stall_timeout_ms do - validated_workflow_options() - |> get_in([:codex, :stall_timeout_ms]) - |> max(0) - end + def codex_stall_timeout_ms, do: get_setting!([:codex, :stall_timeout_ms]) @spec workflow_prompt() :: String.t() def workflow_prompt do @@ -331,63 +124,77 @@ defmodule SymphonyElixir.Config do end @spec observability_enabled?() :: boolean() - def observability_enabled? do - get_in(validated_workflow_options(), [:observability, :dashboard_enabled]) - end + def observability_enabled?, do: get_setting!([:observability, :dashboard_enabled]) @spec observability_refresh_ms() :: pos_integer() - def observability_refresh_ms do - get_in(validated_workflow_options(), [:observability, :refresh_ms]) - end + def observability_refresh_ms, do: get_setting!([:observability, :refresh_ms]) @spec observability_render_interval_ms() :: pos_integer() - def observability_render_interval_ms do - get_in(validated_workflow_options(), [:observability, :render_interval_ms]) - end + def observability_render_interval_ms, do: get_setting!([:observability, :render_interval_ms]) @spec server_port() :: non_neg_integer() | nil def server_port do case Application.get_env(:symphony_elixir, :server_port_override) do - port when is_integer(port) and port >= 0 -> - port - - _ -> - get_in(validated_workflow_options(), [:server, :port]) + port when is_integer(port) and port >= 0 -> port + _ -> get_setting!([:server, :port]) end end @spec server_host() :: String.t() - def server_host do - get_in(validated_workflow_options(), [:server, :host]) - end + def server_host, do: get_setting!([:server, :host]) @spec validate!() :: :ok | {:error, term()} def validate! do - with {:ok, _workflow} <- current_workflow(), - :ok <- require_tracker_kind(), - :ok <- require_linear_token(), - :ok <- require_linear_project(), - :ok <- require_valid_codex_runtime_settings() do - require_codex_command() + with {:ok, settings} <- validated_settings(), + :ok <- require_tracker_kind(settings), + :ok <- require_linear_token(settings), + :ok <- require_linear_project(settings) do + require_codex_command(settings) end end @spec codex_runtime_settings(Path.t() | nil) :: {:ok, codex_runtime_settings()} | {:error, term()} def codex_runtime_settings(workspace \\ nil) do - with {:ok, approval_policy} <- resolve_codex_approval_policy(), - {:ok, thread_sandbox} <- resolve_codex_thread_sandbox(), - {:ok, turn_sandbox_policy} <- resolve_codex_turn_sandbox_policy(workspace) do + with {:ok, settings} <- validated_settings() do {:ok, %{ - approval_policy: approval_policy, - thread_sandbox: thread_sandbox, - turn_sandbox_policy: turn_sandbox_policy + approval_policy: get_in(settings, [:codex, :approval_policy]), + thread_sandbox: get_in(settings, [:codex, :thread_sandbox]), + turn_sandbox_policy: Schema.resolve_turn_sandbox_policy(settings, workspace) }} end end - defp require_tracker_kind do - case tracker_kind() do + defp get_setting!(path) do + validated_settings!() + |> get_in(path) + end + + defp validated_settings! do + case validated_settings() do + {:ok, settings} -> + settings + + {:error, reason} -> + raise ArgumentError, message: format_config_error(reason) + end + end + + defp validated_settings do + case current_workflow() do + {:ok, %{config: config}} when is_map(config) -> + Schema.parse(config) + + {:ok, _workflow} -> + Schema.parse(%{}) + + {:error, reason} -> + {:error, reason} + end + end + + defp require_tracker_kind(settings) do + case get_in(settings, [:tracker, :kind]) do "linear" -> :ok "memory" -> :ok nil -> {:error, :missing_tracker_kind} @@ -395,10 +202,10 @@ defmodule SymphonyElixir.Config do end end - defp require_linear_token do - case tracker_kind() do + defp require_linear_token(settings) do + case get_in(settings, [:tracker, :kind]) do "linear" -> - if is_binary(linear_api_token()) do + if is_binary(get_in(settings, [:tracker, :api_key])) do :ok else {:error, :missing_linear_api_token} @@ -409,10 +216,10 @@ defmodule SymphonyElixir.Config do end end - defp require_linear_project do - case tracker_kind() do + defp require_linear_project(settings) do + case get_in(settings, [:tracker, :kind]) do "linear" -> - if is_binary(linear_project_slug()) do + if is_binary(get_in(settings, [:tracker, :project_slug])) do :ok else {:error, :missing_linear_project_slug} @@ -423,516 +230,30 @@ defmodule SymphonyElixir.Config do end end - defp require_codex_command do - if byte_size(String.trim(codex_command())) > 0 do + defp require_codex_command(settings) do + if is_binary(get_in(settings, [:codex, :command])) do :ok else {:error, :missing_codex_command} end end - defp require_valid_codex_runtime_settings do - case codex_runtime_settings() do - {:ok, _settings} -> :ok - {:error, reason} -> {:error, reason} - end - end - - defp validated_workflow_options do - workflow_config() - |> extract_workflow_options() - |> NimbleOptions.validate!(@workflow_options_schema) - end - - defp extract_workflow_options(config) do - %{ - tracker: extract_tracker_options(section_map(config, "tracker")), - polling: extract_polling_options(section_map(config, "polling")), - workspace: extract_workspace_options(section_map(config, "workspace")), - agent: extract_agent_options(section_map(config, "agent")), - codex: extract_codex_options(section_map(config, "codex")), - hooks: extract_hooks_options(section_map(config, "hooks")), - observability: extract_observability_options(section_map(config, "observability")), - server: extract_server_options(section_map(config, "server")) - } - end - - defp extract_tracker_options(section) do - %{} - |> put_if_present(:kind, normalize_tracker_kind(scalar_string_value(Map.get(section, "kind")))) - |> put_if_present(:endpoint, scalar_string_value(Map.get(section, "endpoint"))) - |> put_if_present(:api_key, binary_value(Map.get(section, "api_key"), allow_empty: true)) - |> put_if_present(:project_slug, scalar_string_value(Map.get(section, "project_slug"))) - |> put_if_present(:active_states, csv_value(Map.get(section, "active_states"))) - |> put_if_present(:terminal_states, csv_value(Map.get(section, "terminal_states"))) - end - - defp extract_polling_options(section) do - %{} - |> put_if_present(:interval_ms, integer_value(Map.get(section, "interval_ms"))) - end - - defp extract_workspace_options(section) do - %{} - |> put_if_present(:root, binary_value(Map.get(section, "root"))) - end - - defp extract_agent_options(section) do - %{} - |> put_if_present(:max_concurrent_agents, integer_value(Map.get(section, "max_concurrent_agents"))) - |> put_if_present(:max_turns, positive_integer_value(Map.get(section, "max_turns"))) - |> put_if_present(:max_retry_backoff_ms, positive_integer_value(Map.get(section, "max_retry_backoff_ms"))) - |> put_if_present( - :max_concurrent_agents_by_state, - state_limits_value(Map.get(section, "max_concurrent_agents_by_state")) - ) - end - - defp extract_codex_options(section) do - %{} - |> put_if_present(:command, command_value(Map.get(section, "command"))) - |> put_if_present(:turn_timeout_ms, integer_value(Map.get(section, "turn_timeout_ms"))) - |> put_if_present(:read_timeout_ms, integer_value(Map.get(section, "read_timeout_ms"))) - |> put_if_present(:stall_timeout_ms, integer_value(Map.get(section, "stall_timeout_ms"))) - end - - defp extract_hooks_options(section) do - %{} - |> put_if_present(:after_create, hook_command_value(Map.get(section, "after_create"))) - |> put_if_present(:before_run, hook_command_value(Map.get(section, "before_run"))) - |> put_if_present(:after_run, hook_command_value(Map.get(section, "after_run"))) - |> put_if_present(:before_remove, hook_command_value(Map.get(section, "before_remove"))) - |> put_if_present(:timeout_ms, positive_integer_value(Map.get(section, "timeout_ms"))) - end - - defp extract_observability_options(section) do - %{} - |> put_if_present(:dashboard_enabled, boolean_value(Map.get(section, "dashboard_enabled"))) - |> put_if_present(:refresh_ms, integer_value(Map.get(section, "refresh_ms"))) - |> put_if_present(:render_interval_ms, integer_value(Map.get(section, "render_interval_ms"))) - end - - defp extract_server_options(section) do - %{} - |> put_if_present(:port, non_negative_integer_value(Map.get(section, "port"))) - |> put_if_present(:host, scalar_string_value(Map.get(section, "host"))) - end - - defp section_map(config, key) do - case Map.get(config, key) do - section when is_map(section) -> section - _ -> %{} - end - end - - defp put_if_present(map, _key, :omit), do: map - defp put_if_present(map, key, value), do: Map.put(map, key, value) - - defp scalar_string_value(nil), do: :omit - defp scalar_string_value(value) when is_binary(value), do: String.trim(value) - defp scalar_string_value(value) when is_boolean(value), do: to_string(value) - defp scalar_string_value(value) when is_integer(value), do: to_string(value) - defp scalar_string_value(value) when is_float(value), do: to_string(value) - defp scalar_string_value(value) when is_atom(value), do: Atom.to_string(value) - defp scalar_string_value(_value), do: :omit - - defp binary_value(value, opts \\ []) - - defp binary_value(value, opts) when is_binary(value) do - allow_empty = Keyword.get(opts, :allow_empty, false) - - if value == "" and not allow_empty do - :omit - else - value - end - end - - defp binary_value(_value, _opts), do: :omit - - defp command_value(value) when is_binary(value) do - case String.trim(value) do - "" -> :omit - trimmed -> trimmed - end - end - - defp command_value(_value), do: :omit - - defp hook_command_value(value) when is_binary(value) do - case String.trim(value) do - "" -> :omit - _ -> String.trim_trailing(value) - end - end - - defp hook_command_value(_value), do: :omit - - defp csv_value(values) when is_list(values) do - values - |> Enum.reduce([], fn value, acc -> maybe_append_csv_value(acc, value) end) - |> Enum.reverse() - |> case do - [] -> :omit - normalized_values -> normalized_values - end - end - - defp csv_value(value) when is_binary(value) do - value - |> String.split(",", trim: true) - |> Enum.map(&String.trim/1) - |> Enum.reject(&(&1 == "")) - |> case do - [] -> :omit - normalized_values -> normalized_values - end - end - - defp csv_value(_value), do: :omit - - defp maybe_append_csv_value(acc, value) do - case scalar_string_value(value) do - :omit -> - acc - - normalized -> - append_csv_value_if_present(acc, normalized) - end - end - - defp append_csv_value_if_present(acc, value) do - trimmed = String.trim(value) - - if trimmed == "" do - acc - else - [trimmed | acc] - end - end - - defp integer_value(value) do - case parse_integer(value) do - {:ok, parsed} -> parsed - :error -> :omit - end - end - - defp positive_integer_value(value) do - case parse_positive_integer(value) do - {:ok, parsed} -> parsed - :error -> :omit - end - end - - defp non_negative_integer_value(value) do - case parse_non_negative_integer(value) do - {:ok, parsed} -> parsed - :error -> :omit - end - end - - defp boolean_value(value) when is_boolean(value), do: value - - defp boolean_value(value) when is_binary(value) do - case String.downcase(String.trim(value)) do - "true" -> true - "false" -> false - _ -> :omit - end - end - - defp boolean_value(_value), do: :omit - - defp state_limits_value(value) when is_map(value) do - value - |> Enum.reduce(%{}, fn {state_name, limit}, acc -> - case parse_positive_integer(limit) do - {:ok, parsed} -> - Map.put(acc, normalize_issue_state(to_string(state_name)), parsed) - - :error -> - acc - end - end) - end - - defp state_limits_value(_value), do: :omit - - defp parse_integer(value) when is_integer(value), do: {:ok, value} - - defp parse_integer(value) when is_binary(value) do - case Integer.parse(String.trim(value)) do - {parsed, _} -> {:ok, parsed} - :error -> :error - end - end - - defp parse_integer(_value), do: :error + defp format_config_error(reason) do + case reason do + {:invalid_workflow_config, message} -> + "Invalid WORKFLOW.md config: #{message}" - defp parse_positive_integer(value) do - case parse_integer(value) do - {:ok, parsed} when parsed > 0 -> {:ok, parsed} - _ -> :error - end - end + {:missing_workflow_file, path, raw_reason} -> + "Missing WORKFLOW.md at #{path}: #{inspect(raw_reason)}" - defp parse_non_negative_integer(value) do - case parse_integer(value) do - {:ok, parsed} when parsed >= 0 -> {:ok, parsed} - _ -> :error - end - end + {:workflow_parse_error, raw_reason} -> + "Failed to parse WORKFLOW.md: #{inspect(raw_reason)}" - defp fetch_value(paths, default) do - config = workflow_config() + :workflow_front_matter_not_a_map -> + "Failed to parse WORKFLOW.md: workflow front matter must decode to a map" - case resolve_config_value(config, paths) do - :missing -> default - value -> value + other -> + "Invalid WORKFLOW.md config: #{inspect(other)}" end end - - defp resolve_codex_approval_policy do - case fetch_value([["codex", "approval_policy"]], :missing) do - :missing -> - {:ok, @default_codex_approval_policy} - - nil -> - {:ok, @default_codex_approval_policy} - - value when is_binary(value) -> - approval_policy = String.trim(value) - - if approval_policy == "" do - {:error, {:invalid_codex_approval_policy, value}} - else - {:ok, approval_policy} - end - - value when is_map(value) -> - {:ok, value} - - value -> - {:error, {:invalid_codex_approval_policy, value}} - end - end - - defp resolve_codex_thread_sandbox do - case fetch_value([["codex", "thread_sandbox"]], :missing) do - :missing -> - {:ok, @default_codex_thread_sandbox} - - nil -> - {:ok, @default_codex_thread_sandbox} - - value when is_binary(value) -> - thread_sandbox = String.trim(value) - - if thread_sandbox == "" do - {:error, {:invalid_codex_thread_sandbox, value}} - else - {:ok, thread_sandbox} - end - - value -> - {:error, {:invalid_codex_thread_sandbox, value}} - end - end - - defp resolve_codex_turn_sandbox_policy(workspace) do - case fetch_value([["codex", "turn_sandbox_policy"]], :missing) do - :missing -> - {:ok, default_codex_turn_sandbox_policy(workspace)} - - nil -> - {:ok, default_codex_turn_sandbox_policy(workspace)} - - value when is_map(value) -> - {:ok, value} - - value -> - {:error, {:invalid_codex_turn_sandbox_policy, {:unsupported_value, value}}} - end - end - - defp default_codex_turn_sandbox_policy(workspace) do - writable_root = - if is_binary(workspace) and String.trim(workspace) != "" do - Path.expand(workspace) - else - Path.expand(workspace_root()) - end - - %{ - "type" => "workspaceWrite", - "writableRoots" => [writable_root], - "readOnlyAccess" => %{"type" => "fullAccess"}, - "networkAccess" => false, - "excludeTmpdirEnvVar" => false, - "excludeSlashTmp" => false - } - end - - defp normalize_issue_state(state_name) when is_binary(state_name) do - state_name - |> String.trim() - |> String.downcase() - end - - defp normalize_tracker_kind(kind) when is_binary(kind) do - kind - |> String.trim() - |> String.downcase() - |> case do - "" -> nil - normalized -> normalized - end - end - - defp normalize_tracker_kind(_kind), do: nil - - defp workflow_config do - case current_workflow() do - {:ok, %{config: config}} when is_map(config) -> - normalize_keys(config) - - _ -> - %{} - end - end - - defp resolve_config_value(%{} = config, paths) do - Enum.reduce_while(paths, :missing, fn path, _acc -> - case get_in_path(config, path) do - :missing -> {:cont, :missing} - value -> {:halt, value} - end - end) - end - - defp get_in_path(config, path) when is_list(path) and is_map(config) do - get_in_path(config, path, 0) - end - - defp get_in_path(_, _), do: :missing - - defp get_in_path(config, [], _depth), do: config - - defp get_in_path(%{} = current, [segment | rest], _depth) do - case Map.fetch(current, normalize_key(segment)) do - {:ok, value} -> get_in_path(value, rest, 0) - :error -> :missing - end - end - - defp get_in_path(_, _, _depth), do: :missing - - defp normalize_keys(value) when is_map(value) do - Enum.reduce(value, %{}, fn {key, raw_value}, normalized -> - Map.put(normalized, normalize_key(key), normalize_keys(raw_value)) - end) - end - - defp normalize_keys(value) when is_list(value), do: Enum.map(value, &normalize_keys/1) - defp normalize_keys(value), do: value - - defp normalize_key(value) when is_atom(value), do: Atom.to_string(value) - defp normalize_key(value), do: to_string(value) - - defp resolve_path_value(:missing, default), do: default - defp resolve_path_value(nil, default), do: default - - defp resolve_path_value(value, default) when is_binary(value) do - case normalize_path_token(value) do - :missing -> - default - - path -> - path - |> String.trim() - |> preserve_command_name() - |> then(fn - "" -> default - resolved -> resolved - end) - end - end - - defp resolve_path_value(_value, default), do: default - - defp preserve_command_name(path) do - cond do - uri_path?(path) -> - path - - String.contains?(path, "/") or String.contains?(path, "\\") -> - Path.expand(path) - - true -> - path - end - end - - defp uri_path?(path) do - String.match?(to_string(path), ~r/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//) - end - - defp resolve_env_value(:missing, fallback), do: fallback - defp resolve_env_value(nil, fallback), do: fallback - - defp resolve_env_value(value, fallback) when is_binary(value) do - trimmed = String.trim(value) - - case env_reference_name(trimmed) do - {:ok, env_name} -> - env_name - |> System.get_env() - |> then(fn - nil -> fallback - "" -> nil - env_value -> env_value - end) - - :error -> - trimmed - end - end - - defp resolve_env_value(_value, fallback), do: fallback - - defp normalize_path_token(value) when is_binary(value) do - trimmed = String.trim(value) - - case env_reference_name(trimmed) do - {:ok, env_name} -> resolve_env_token(env_name) - :error -> trimmed - end - end - - defp env_reference_name("$" <> env_name) do - if String.match?(env_name, ~r/^[A-Za-z_][A-Za-z0-9_]*$/) do - {:ok, env_name} - else - :error - end - end - - defp env_reference_name(_value), do: :error - - defp resolve_env_token(value) do - case System.get_env(value) do - nil -> :missing - env_value -> env_value - end - end - - defp normalize_secret_value(value) when is_binary(value) do - case String.trim(value) do - "" -> nil - trimmed -> trimmed - end - end - - defp normalize_secret_value(_value), do: nil end diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex new file mode 100644 index 00000000..52af553a --- /dev/null +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -0,0 +1,490 @@ +defmodule SymphonyElixir.Config.Schema do + @moduledoc false + + use Ecto.Schema + + import Ecto.Changeset + + @primary_key false + + defmodule StringOrMap do + @moduledoc false + @behaviour Ecto.Type + + def type, do: :map + def embed_as(_format), do: :self + def equal?(left, right), do: left == right + + def cast(value) when is_binary(value) or is_map(value), do: {:ok, value} + def cast(_value), do: :error + + def load(value) when is_binary(value) or is_map(value), do: {:ok, value} + def load(_value), do: :error + + def dump(value) when is_binary(value) or is_map(value), do: {:ok, value} + def dump(_value), do: :error + end + + defmodule Tracker do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + + embedded_schema do + field :kind, :string + field :endpoint, :string, default: "https://api.linear.app/graphql" + field :api_key, :string + field :project_slug, :string + field :assignee, :string + field :active_states, {:array, :string}, default: ["Todo", "In Progress"] + field :terminal_states, {:array, :string}, default: ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:kind, :endpoint, :api_key, :project_slug, :assignee, :active_states, :terminal_states], + empty_values: [] + ) + end + end + + defmodule Polling do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :interval_ms, :integer, default: 30_000 + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:interval_ms], empty_values: []) + |> validate_number(:interval_ms, greater_than: 0) + end + end + + defmodule Workspace do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :root, :string, default: Path.join(System.tmp_dir!(), "symphony_workspaces") + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:root], empty_values: []) + end + end + + defmodule Agent do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + alias SymphonyElixir.Config.Schema + + @primary_key false + embedded_schema do + field :max_concurrent_agents, :integer, default: 10 + field :max_turns, :integer, default: 20 + field :max_retry_backoff_ms, :integer, default: 300_000 + field :max_concurrent_agents_by_state, :map, default: %{} + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:max_concurrent_agents, :max_turns, :max_retry_backoff_ms, :max_concurrent_agents_by_state], + empty_values: [] + ) + |> validate_number(:max_concurrent_agents, greater_than: 0) + |> validate_number(:max_turns, greater_than: 0) + |> validate_number(:max_retry_backoff_ms, greater_than: 0) + |> update_change(:max_concurrent_agents_by_state, &Schema.normalize_state_limits/1) + |> Schema.validate_state_limits(:max_concurrent_agents_by_state) + end + end + + defmodule Codex do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :command, :string, default: "codex app-server" + field :approval_policy, StringOrMap, + default: %{ + "reject" => %{ + "sandbox_approval" => true, + "rules" => true, + "mcp_elicitations" => true + } + } + + field :thread_sandbox, :string, default: "workspace-write" + field :turn_sandbox_policy, :map + field :turn_timeout_ms, :integer, default: 3_600_000 + field :read_timeout_ms, :integer, default: 5_000 + field :stall_timeout_ms, :integer, default: 300_000 + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [ + :command, + :approval_policy, + :thread_sandbox, + :turn_sandbox_policy, + :turn_timeout_ms, + :read_timeout_ms, + :stall_timeout_ms + ], empty_values: []) + |> validate_number(:turn_timeout_ms, greater_than: 0) + |> validate_number(:read_timeout_ms, greater_than: 0) + |> validate_number(:stall_timeout_ms, greater_than_or_equal_to: 0) + end + end + + defmodule Hooks do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :after_create, :string + field :before_run, :string + field :after_run, :string + field :before_remove, :string + field :timeout_ms, :integer, default: 60_000 + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:after_create, :before_run, :after_run, :before_remove, :timeout_ms], + empty_values: [] + ) + |> validate_number(:timeout_ms, greater_than: 0) + end + end + + defmodule Observability do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :dashboard_enabled, :boolean, default: true + field :refresh_ms, :integer, default: 1_000 + field :render_interval_ms, :integer, default: 16 + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:dashboard_enabled, :refresh_ms, :render_interval_ms], empty_values: []) + |> validate_number(:refresh_ms, greater_than: 0) + |> validate_number(:render_interval_ms, greater_than: 0) + end + end + + defmodule Server do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field :port, :integer + field :host, :string, default: "127.0.0.1" + end + + def changeset(schema, attrs) do + schema + |> cast(attrs, [:port, :host], empty_values: []) + |> validate_number(:port, greater_than_or_equal_to: 0) + end + end + + embedded_schema do + embeds_one :tracker, Tracker, on_replace: :update, defaults_to_struct: true + embeds_one :polling, Polling, on_replace: :update, defaults_to_struct: true + embeds_one :workspace, Workspace, on_replace: :update, defaults_to_struct: true + embeds_one :agent, Agent, on_replace: :update, defaults_to_struct: true + embeds_one :codex, Codex, on_replace: :update, defaults_to_struct: true + embeds_one :hooks, Hooks, on_replace: :update, defaults_to_struct: true + embeds_one :observability, Observability, on_replace: :update, defaults_to_struct: true + embeds_one :server, Server, on_replace: :update, defaults_to_struct: true + end + + @spec parse(map()) :: {:ok, map()} | {:error, {:invalid_workflow_config, String.t()}} + def parse(config) when is_map(config) do + config + |> normalize_keys() + |> drop_nil_values() + |> changeset() + |> apply_action(:validate) + |> case do + {:ok, settings} -> + {:ok, finalize_settings(settings)} + + {:error, changeset} -> + {:error, {:invalid_workflow_config, format_errors(changeset)}} + end + end + + @spec resolve_turn_sandbox_policy(map(), Path.t() | nil) :: map() + def resolve_turn_sandbox_policy(settings, workspace \\ nil) do + case get_in(settings, [:codex, :turn_sandbox_policy]) do + %{} = policy -> + policy + + _ -> + default_turn_sandbox_policy(workspace || get_in(settings, [:workspace, :root])) + end + 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) + end + + @doc false + def normalize_state_limits(nil), do: %{} + + def normalize_state_limits(limits) when is_map(limits) do + Enum.reduce(limits, %{}, fn {state_name, limit}, acc -> + Map.put(acc, normalize_issue_state(to_string(state_name)), limit) + end) + end + + @doc false + def validate_state_limits(changeset, field) do + validate_change(changeset, field, fn ^field, limits -> + Enum.flat_map(limits, fn {state_name, limit} -> + cond do + to_string(state_name) == "" -> + [{field, "state names must not be blank"}] + + not is_integer(limit) or limit <= 0 -> + [{field, "limits must be positive integers"}] + + true -> + [] + end + end) + end) + end + + defp changeset(attrs) do + %__MODULE__{} + |> cast(attrs, []) + |> cast_embed(:tracker, with: &Tracker.changeset/2) + |> cast_embed(:polling, with: &Polling.changeset/2) + |> cast_embed(:workspace, with: &Workspace.changeset/2) + |> cast_embed(:agent, with: &Agent.changeset/2) + |> cast_embed(:codex, with: &Codex.changeset/2) + |> cast_embed(:hooks, with: &Hooks.changeset/2) + |> cast_embed(:observability, with: &Observability.changeset/2) + |> cast_embed(:server, with: &Server.changeset/2) + end + + defp finalize_settings(settings) do + settings + |> deep_to_map() + |> update_in([:tracker, :api_key], &resolve_secret_setting(&1, System.get_env("LINEAR_API_KEY"))) + |> update_in([:tracker, :assignee], &resolve_secret_setting(&1, System.get_env("LINEAR_ASSIGNEE"))) + |> update_in([:workspace, :root], &resolve_path_value(&1, Path.join(System.tmp_dir!(), "symphony_workspaces"))) + |> update_in([:codex, :approval_policy], &normalize_keys/1) + |> update_in([:codex, :turn_sandbox_policy], &normalize_optional_map/1) + end + + defp deep_to_map(%_{} = struct) do + struct + |> Map.from_struct() + |> Enum.reduce(%{}, fn {key, value}, acc -> + Map.put(acc, key, deep_to_map(value)) + end) + end + + defp deep_to_map(value) when is_map(value) do + Enum.reduce(value, %{}, fn {key, nested}, acc -> + Map.put(acc, key, deep_to_map(nested)) + end) + end + + defp deep_to_map(value) when is_list(value), do: Enum.map(value, &deep_to_map/1) + defp deep_to_map(value), do: value + + defp normalize_keys(value) when is_map(value) do + Enum.reduce(value, %{}, fn {key, raw_value}, normalized -> + Map.put(normalized, normalize_key(key), normalize_keys(raw_value)) + end) + end + + defp normalize_keys(value) when is_list(value), do: Enum.map(value, &normalize_keys/1) + defp normalize_keys(value), do: value + + defp normalize_optional_map(nil), do: nil + defp normalize_optional_map(value) when is_map(value), do: normalize_keys(value) + defp normalize_optional_map(value), do: value + + defp normalize_key(value) when is_atom(value), do: Atom.to_string(value) + defp normalize_key(value), do: to_string(value) + + defp drop_nil_values(value) when is_map(value) do + Enum.reduce(value, %{}, fn {key, nested}, acc -> + case drop_nil_values(nested) do + nil -> acc + normalized -> Map.put(acc, key, normalized) + end + end) + end + + defp drop_nil_values(value) when is_list(value), do: Enum.map(value, &drop_nil_values/1) + defp drop_nil_values(value), do: value + + defp resolve_secret_setting(nil, fallback), do: normalize_secret_value(fallback) + + defp resolve_secret_setting(value, fallback) when is_binary(value) do + case resolve_env_value(value, fallback) do + resolved when is_binary(resolved) -> normalize_secret_value(resolved) + resolved -> resolved + end + end + + defp resolve_secret_setting(_value, fallback), do: normalize_secret_value(fallback) + + defp resolve_path_value(nil, default), do: Path.expand(default) + + defp resolve_path_value(value, default) when is_binary(value) do + case normalize_path_token(value) do + :missing -> + Path.expand(default) + + "" -> + Path.expand(default) + + path -> + Path.expand(path) + end + end + + defp resolve_path_value(_value, default), do: Path.expand(default) + + defp resolve_env_value(nil, fallback), do: fallback + + defp resolve_env_value(value, fallback) when is_binary(value) do + case env_reference_name(value) do + {:ok, env_name} -> + case System.get_env(env_name) do + nil -> fallback + "" -> nil + env_value -> env_value + end + + :error -> + value + end + end + + defp resolve_env_value(_value, fallback), do: fallback + + defp normalize_path_token(value) when is_binary(value) do + case env_reference_name(value) do + {:ok, env_name} -> resolve_env_token(env_name) + :error -> value + end + end + + defp env_reference_name("$" <> env_name) do + if String.match?(env_name, ~r/^[A-Za-z_][A-Za-z0-9_]*$/) do + {:ok, env_name} + else + :error + end + end + + defp env_reference_name(_value), do: :error + + defp resolve_env_token(env_name) do + case System.get_env(env_name) do + nil -> :missing + env_value -> env_value + end + end + + defp normalize_secret_value(value) when is_binary(value) do + if value == "", do: nil, else: value + end + + defp normalize_secret_value(_value), do: nil + + defp default_turn_sandbox_policy(workspace) do + writable_root = + if is_binary(workspace) and workspace != "" do + Path.expand(workspace) + else + Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces")) + end + + %{ + "type" => "workspaceWrite", + "writableRoots" => [writable_root], + "readOnlyAccess" => %{"type" => "fullAccess"}, + "networkAccess" => false, + "excludeTmpdirEnvVar" => false, + "excludeSlashTmp" => false + } + end + + defp format_errors(changeset) do + changeset + |> traverse_errors(&translate_error/1) + |> flatten_errors() + |> Enum.join(", ") + end + + defp flatten_errors(errors, prefix \\ nil) + + defp flatten_errors(errors, prefix) when is_map(errors) do + Enum.flat_map(errors, fn {key, value} -> + next_prefix = + case prefix do + nil -> to_string(key) + current -> current <> "." <> to_string(key) + end + + flatten_errors(value, next_prefix) + end) + end + + defp flatten_errors(errors, prefix) when is_list(errors) do + Enum.flat_map(errors, fn + value when is_binary(value) -> + [prefix <> " " <> value] + + value -> + flatten_errors(value, prefix) + end) + end + + defp translate_error({message, options}) do + Enum.reduce(options, message, fn {key, value}, acc -> + String.replace(acc, "%{#{key}}", error_value_to_string(value)) + end) + end + + defp error_value_to_string(value) when is_binary(value), do: value + defp error_value_to_string(value) when is_atom(value), do: Atom.to_string(value) + defp error_value_to_string(value) when is_integer(value), do: Integer.to_string(value) + defp error_value_to_string(value) when is_float(value), do: Float.to_string(value) + defp error_value_to_string(value), do: inspect(value) +end diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index a4dead12..828440b3 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -200,16 +200,8 @@ defmodule SymphonyElixir.Orchestrator do Logger.error("Codex command missing in WORKFLOW.md") state - {:error, {:invalid_codex_approval_policy, value}} -> - Logger.error("Invalid codex.approval_policy in WORKFLOW.md: #{inspect(value)}") - state - - {:error, {:invalid_codex_thread_sandbox, value}} -> - Logger.error("Invalid codex.thread_sandbox in WORKFLOW.md: #{inspect(value)}") - state - - {:error, {:invalid_codex_turn_sandbox_policy, reason}} -> - Logger.error("Invalid codex.turn_sandbox_policy in WORKFLOW.md: #{inspect(reason)}") + {:error, {:invalid_workflow_config, message}} -> + Logger.error("Invalid WORKFLOW.md config: #{message}") state {:error, {:missing_workflow_file, path, reason}} -> diff --git a/elixir/mix.exs b/elixir/mix.exs index 062706aa..aff9e4d9 100644 --- a/elixir/mix.exs +++ b/elixir/mix.exs @@ -73,7 +73,7 @@ defmodule SymphonyElixir.MixProject do {:jason, "~> 1.4"}, {:yaml_elixir, "~> 2.12"}, {:solid, "~> 1.2"}, - {:nimble_options, "~> 1.1"}, + {:ecto, "~> 3.13"}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.4", only: [:dev], runtime: false} ] diff --git a/elixir/mix.lock b/elixir/mix.lock index 4f52fd70..f2f7c58d 100644 --- a/elixir/mix.lock +++ b/elixir/mix.lock @@ -6,6 +6,7 @@ "date_time_parser": {:hex, :date_time_parser, "1.3.0", "6ba16850b5ab83dd126576451023ab65349e29af2336ca5084aa1e37025b476e", [:mix], [{:kday, "~> 1.0", [hex: :kday, repo: "hexpm", optional: false]}], "hexpm", "93c8203a8ddc66b1f1531fc0e046329bf0b250c75ffa09567ef03d2c09218e8c"}, "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"}, + "ecto": {:hex, :ecto, "3.13.5", "9d4a69700183f33bf97208294768e561f5c7f1ecf417e0fa1006e4a91713a834", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "df9efebf70cf94142739ba357499661ef5dbb559ef902b68ea1f3c1fabce36de"}, "elixir_make": {:hex, :elixir_make, "0.9.0", "6484b3cd8c0cee58f09f05ecaf1a140a8c97670671a6a0e7ab4dc326c3109726", [:mix], [], "hexpm", "db23d4fd8b757462ad02f8aa73431a426fe6671c80b200d9710caf3d1dd0ffdb"}, "erlex": {:hex, :erlex, "0.2.8", "cd8116f20f3c0afe376d1e8d1f0ae2452337729f68be016ea544a72f767d9c12", [:mix], [], "hexpm", "9d66ff9fedf69e49dc3fd12831e12a8a37b76f8651dd21cd45fcf5561a8a7590"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index 400c006e..0f3681bd 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -18,19 +18,27 @@ defmodule SymphonyElixir.CoreTest do assert Config.agent_max_turns() == 20 write_workflow_file!(Workflow.workflow_file_path(), poll_interval_ms: "invalid") - assert Config.poll_interval_ms() == 30_000 + + assert_raise ArgumentError, ~r/interval_ms/, fn -> + Config.poll_interval_ms() + end + + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "polling.interval_ms" write_workflow_file!(Workflow.workflow_file_path(), poll_interval_ms: 45_000) assert Config.poll_interval_ms() == 45_000 write_workflow_file!(Workflow.workflow_file_path(), max_turns: 0) - assert Config.agent_max_turns() == 20 + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "agent.max_turns" write_workflow_file!(Workflow.workflow_file_path(), max_turns: 5) assert Config.agent_max_turns() == 5 write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: "Todo, Review,") - assert Config.linear_active_states() == ["Todo", "Review"] + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "tracker.active_states" write_workflow_file!(Workflow.workflow_file_path(), tracker_api_token: "token", @@ -45,6 +53,7 @@ defmodule SymphonyElixir.CoreTest do ) assert :ok = Config.validate!() + assert Config.codex_command() == "" write_workflow_file!(Workflow.workflow_file_path(), codex_command: "/bin/sh app-server") assert :ok = Config.validate!() @@ -62,12 +71,14 @@ defmodule SymphonyElixir.CoreTest do assert :ok = Config.validate!() write_workflow_file!(Workflow.workflow_file_path(), codex_approval_policy: 123) - assert {:error, {:invalid_codex_approval_policy, 123}} = Config.validate!() + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.approval_policy" write_workflow_file!(Workflow.workflow_file_path(), codex_thread_sandbox: 123) - assert {:error, {:invalid_codex_thread_sandbox, 123}} = Config.validate!() + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.thread_sandbox" - write_workflow_file!(Workflow.workflow_file_path(), tracker_kind: 123) + write_workflow_file!(Workflow.workflow_file_path(), tracker_kind: "123") assert {:error, {:unsupported_tracker_kind, "123"}} = Config.validate!() end diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 10f9f524..1e0b33fd 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -703,19 +703,24 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: ",") - assert Config.linear_active_states() == ["Todo", "In Progress"] + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "tracker.active_states" write_workflow_file!(Workflow.workflow_file_path(), max_concurrent_agents: "bad") - assert Config.max_concurrent_agents() == 10 + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "agent.max_concurrent_agents" write_workflow_file!(Workflow.workflow_file_path(), codex_turn_timeout_ms: "bad") - assert Config.codex_turn_timeout_ms() == 3_600_000 + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.turn_timeout_ms" write_workflow_file!(Workflow.workflow_file_path(), codex_read_timeout_ms: "bad") - assert Config.codex_read_timeout_ms() == 5_000 + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.read_timeout_ms" write_workflow_file!(Workflow.workflow_file_path(), codex_stall_timeout_ms: "bad") - assert Config.codex_stall_timeout_ms() == 300_000 + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.stall_timeout_ms" write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: %{todo: true}, @@ -732,49 +737,19 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do server_host: 123 ) - assert Config.linear_active_states() == ["Todo", "In Progress"] - assert Config.linear_terminal_states() == ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] - assert Config.poll_interval_ms() == 30_000 - assert Config.workspace_root() == Path.join(System.tmp_dir!(), "symphony_workspaces") - assert Config.max_retry_backoff_ms() == 300_000 - assert Config.max_concurrent_agents_for_state("Todo") == 1 - assert Config.max_concurrent_agents_for_state("Review") == 10 - assert Config.hook_timeout_ms() == 60_000 - assert Config.observability_enabled?() - assert Config.observability_refresh_ms() == 1_000 - assert Config.observability_render_interval_ms() == 16 - assert Config.server_port() == nil - assert Config.server_host() == "123" + assert {:error, {:invalid_workflow_config, _message}} = Config.validate!() write_workflow_file!(Workflow.workflow_file_path(), codex_approval_policy: "") - - assert Config.codex_approval_policy() == %{ - "reject" => %{ - "sandbox_approval" => true, - "rules" => true, - "mcp_elicitations" => true - } - } - - assert {:error, {:invalid_codex_approval_policy, ""}} = Config.validate!() + assert :ok = Config.validate!() + assert Config.codex_approval_policy() == "" write_workflow_file!(Workflow.workflow_file_path(), codex_thread_sandbox: "") - assert Config.codex_thread_sandbox() == "workspace-write" - assert {:error, {:invalid_codex_thread_sandbox, ""}} = Config.validate!() + assert :ok = Config.validate!() + assert Config.codex_thread_sandbox() == "" write_workflow_file!(Workflow.workflow_file_path(), codex_turn_sandbox_policy: "bad") - - assert Config.codex_turn_sandbox_policy() == %{ - "type" => "workspaceWrite", - "writableRoots" => [Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces"))], - "readOnlyAccess" => %{"type" => "fullAccess"}, - "networkAccess" => false, - "excludeTmpdirEnvVar" => false, - "excludeSlashTmp" => false - } - - assert {:error, {:invalid_codex_turn_sandbox_policy, {:unsupported_value, "bad"}}} = - Config.validate!() + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.turn_sandbox_policy" write_workflow_file!(Workflow.workflow_file_path(), codex_approval_policy: "future-policy", @@ -851,7 +826,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do ) assert Config.linear_api_token() == "env:#{api_key_env_var}" - assert Config.workspace_root() == "env:#{workspace_env_var}" + assert Config.workspace_root() == Path.expand("env:#{workspace_env_var}") end test "config supports per-state max concurrent agent overrides" do From d2e32becc92e220908ac004e950c576cb2e733ac Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Mon, 9 Mar 2026 16:28:04 -0700 Subject: [PATCH 2/2] refactor(config): simplify typed settings access Summary: - collapse Config down to settings/settings! plus the remaining runtime helpers - switch callers and tests to use nested schema structs instead of the field-by-field getter layer - tighten the schema around codex.command and add the specs and tests needed for the full make all pipeline Rationale: - make the Ecto schema the source of truth for config shape and defaults - remove the extra access hop so config reads are direct and typed - keep the repo green under formatting, lint, coverage, and dialyzer Tests: - cd elixir && make all Co-authored-by: Codex --- elixir/lib/symphony_elixir/agent_runner.ex | 4 +- .../lib/symphony_elixir/codex/app_server.ex | 15 +- elixir/lib/symphony_elixir/config.ex | 204 ++++-------------- elixir/lib/symphony_elixir/config/schema.ex | 201 ++++++++--------- elixir/lib/symphony_elixir/http_server.ex | 2 +- elixir/lib/symphony_elixir/linear/client.ex | 18 +- elixir/lib/symphony_elixir/orchestrator.ex | 28 +-- .../lib/symphony_elixir/status_dashboard.ex | 21 +- elixir/lib/symphony_elixir/tracker.ex | 2 +- elixir/lib/symphony_elixir/workspace.ex | 22 +- elixir/lib/symphony_elixir_web/presenter.ex | 2 +- elixir/test/symphony_elixir/core_test.exs | 30 +-- .../test/symphony_elixir/extensions_test.exs | 4 +- .../workspace_and_config_test.exs | 179 ++++++++++++--- 14 files changed, 385 insertions(+), 347 deletions(-) diff --git a/elixir/lib/symphony_elixir/agent_runner.ex b/elixir/lib/symphony_elixir/agent_runner.ex index 7292a4bc..14635060 100644 --- a/elixir/lib/symphony_elixir/agent_runner.ex +++ b/elixir/lib/symphony_elixir/agent_runner.ex @@ -47,7 +47,7 @@ defmodule SymphonyElixir.AgentRunner do defp send_codex_update(_recipient, _issue, _message), do: :ok defp run_codex_turns(workspace, issue, codex_update_recipient, opts) do - max_turns = Keyword.get(opts, :max_turns, Config.agent_max_turns()) + max_turns = Keyword.get(opts, :max_turns, Config.settings!().agent.max_turns) issue_state_fetcher = Keyword.get(opts, :issue_state_fetcher, &Tracker.fetch_issue_states_by_ids/1) with {:ok, session} <- AppServer.start_session(workspace) do @@ -136,7 +136,7 @@ defmodule SymphonyElixir.AgentRunner do defp active_issue_state?(state_name) when is_binary(state_name) do normalized_state = normalize_issue_state(state_name) - Config.linear_active_states() + Config.settings!().tracker.active_states |> Enum.any?(fn active_state -> normalize_issue_state(active_state) == normalized_state end) end diff --git a/elixir/lib/symphony_elixir/codex/app_server.ex b/elixir/lib/symphony_elixir/codex/app_server.ex index e824c63b..1f9252e2 100644 --- a/elixir/lib/symphony_elixir/codex/app_server.ex +++ b/elixir/lib/symphony_elixir/codex/app_server.ex @@ -143,7 +143,7 @@ defmodule SymphonyElixir.Codex.AppServer do defp validate_workspace_cwd(workspace) when is_binary(workspace) do workspace_path = Path.expand(workspace) - workspace_root = Path.expand(Config.workspace_root()) + workspace_root = Path.expand(Config.settings!().workspace.root) root_prefix = workspace_root <> "/" @@ -172,7 +172,7 @@ defmodule SymphonyElixir.Codex.AppServer do :binary, :exit_status, :stderr_to_stdout, - args: [~c"-lc", String.to_charlist(Config.codex_command())], + args: [~c"-lc", String.to_charlist(Config.settings!().codex.command)], cd: String.to_charlist(workspace), line: @port_line_bytes ] @@ -277,7 +277,14 @@ defmodule SymphonyElixir.Codex.AppServer do end defp await_turn_completion(port, on_message, tool_executor, auto_approve_requests) do - receive_loop(port, on_message, Config.codex_turn_timeout_ms(), "", tool_executor, auto_approve_requests) + receive_loop( + port, + on_message, + Config.settings!().codex.turn_timeout_ms, + "", + tool_executor, + auto_approve_requests + ) end defp receive_loop(port, on_message, timeout_ms, pending_line, tool_executor, auto_approve_requests) do @@ -820,7 +827,7 @@ defmodule SymphonyElixir.Codex.AppServer do end defp await_response(port, request_id) do - with_timeout_response(port, request_id, Config.codex_read_timeout_ms(), "") + with_timeout_response(port, request_id, Config.settings!().codex.read_timeout_ms, "") end defp with_timeout_response(port, request_id, timeout_ms, pending_line) do diff --git a/elixir/lib/symphony_elixir/config.ex b/elixir/lib/symphony_elixir/config.ex index ba30fb4d..84623ca2 100644 --- a/elixir/lib/symphony_elixir/config.ex +++ b/elixir/lib/symphony_elixir/config.ex @@ -20,101 +20,56 @@ defmodule SymphonyElixir.Config do {% endif %} """ - @type workflow_payload :: Workflow.loaded_workflow() - @type tracker_kind :: String.t() | nil @type codex_runtime_settings :: %{ approval_policy: String.t() | map(), thread_sandbox: String.t(), turn_sandbox_policy: map() } - @type workspace_hooks :: %{ - after_create: String.t() | nil, - before_run: String.t() | nil, - after_run: String.t() | nil, - before_remove: String.t() | nil, - timeout_ms: pos_integer() - } - - @spec current_workflow() :: {:ok, workflow_payload()} | {:error, term()} - def current_workflow, do: Workflow.current() - - @spec tracker_kind() :: tracker_kind() - def tracker_kind, do: get_setting!([:tracker, :kind]) - - @spec linear_endpoint() :: String.t() - def linear_endpoint, do: get_setting!([:tracker, :endpoint]) - - @spec linear_api_token() :: String.t() | nil - def linear_api_token, do: get_setting!([:tracker, :api_key]) - - @spec linear_project_slug() :: String.t() | nil - def linear_project_slug, do: get_setting!([:tracker, :project_slug]) - - @spec linear_assignee() :: String.t() | nil - def linear_assignee, do: get_setting!([:tracker, :assignee]) - - @spec linear_active_states() :: [String.t()] - def linear_active_states, do: get_setting!([:tracker, :active_states]) - @spec linear_terminal_states() :: [String.t()] - def linear_terminal_states, do: get_setting!([:tracker, :terminal_states]) - - @spec poll_interval_ms() :: pos_integer() - def poll_interval_ms, do: get_setting!([:polling, :interval_ms]) - - @spec workspace_root() :: Path.t() - def workspace_root, do: get_setting!([:workspace, :root]) - - @spec workspace_hooks() :: workspace_hooks() - def workspace_hooks, do: get_setting!([:hooks]) - - @spec hook_timeout_ms() :: pos_integer() - def hook_timeout_ms, do: get_setting!([:hooks, :timeout_ms]) + @spec settings() :: {:ok, Schema.t()} | {:error, term()} + def settings do + case Workflow.current() do + {:ok, %{config: config}} when is_map(config) -> + Schema.parse(config) - @spec max_concurrent_agents() :: pos_integer() - def max_concurrent_agents, do: get_setting!([:agent, :max_concurrent_agents]) + {:error, reason} -> + {:error, reason} + end + end - @spec max_retry_backoff_ms() :: pos_integer() - def max_retry_backoff_ms, do: get_setting!([:agent, :max_retry_backoff_ms]) + @spec settings!() :: Schema.t() + def settings! do + case settings() do + {:ok, settings} -> + settings - @spec agent_max_turns() :: pos_integer() - def agent_max_turns, do: get_setting!([:agent, :max_turns]) + {:error, reason} -> + raise ArgumentError, message: format_config_error(reason) + end + end @spec max_concurrent_agents_for_state(term()) :: pos_integer() def max_concurrent_agents_for_state(state_name) when is_binary(state_name) do - state_limits = get_setting!([:agent, :max_concurrent_agents_by_state]) - Map.get(state_limits, Schema.normalize_issue_state(state_name), max_concurrent_agents()) - end - - def max_concurrent_agents_for_state(_state_name), do: max_concurrent_agents() - - @spec codex_command() :: String.t() - def codex_command, do: get_setting!([:codex, :command]) - - @spec codex_turn_timeout_ms() :: pos_integer() - def codex_turn_timeout_ms, do: get_setting!([:codex, :turn_timeout_ms]) + config = settings!() - @spec codex_approval_policy() :: String.t() | map() - def codex_approval_policy, do: get_setting!([:codex, :approval_policy]) + Map.get( + config.agent.max_concurrent_agents_by_state, + Schema.normalize_issue_state(state_name), + config.agent.max_concurrent_agents + ) + end - @spec codex_thread_sandbox() :: String.t() - def codex_thread_sandbox, do: get_setting!([:codex, :thread_sandbox]) + def max_concurrent_agents_for_state(_state_name), do: settings!().agent.max_concurrent_agents @spec codex_turn_sandbox_policy(Path.t() | nil) :: map() def codex_turn_sandbox_policy(workspace \\ nil) do - validated_settings!() + settings!() |> Schema.resolve_turn_sandbox_policy(workspace) end - @spec codex_read_timeout_ms() :: pos_integer() - def codex_read_timeout_ms, do: get_setting!([:codex, :read_timeout_ms]) - - @spec codex_stall_timeout_ms() :: non_neg_integer() - def codex_stall_timeout_ms, do: get_setting!([:codex, :stall_timeout_ms]) - @spec workflow_prompt() :: String.t() def workflow_prompt do - case current_workflow() do + case Workflow.current() do {:ok, %{prompt_template: prompt}} -> if String.trim(prompt) == "", do: @default_prompt_template, else: prompt @@ -123,121 +78,52 @@ defmodule SymphonyElixir.Config do end end - @spec observability_enabled?() :: boolean() - def observability_enabled?, do: get_setting!([:observability, :dashboard_enabled]) - - @spec observability_refresh_ms() :: pos_integer() - def observability_refresh_ms, do: get_setting!([:observability, :refresh_ms]) - - @spec observability_render_interval_ms() :: pos_integer() - def observability_render_interval_ms, do: get_setting!([:observability, :render_interval_ms]) - @spec server_port() :: non_neg_integer() | nil def server_port do case Application.get_env(:symphony_elixir, :server_port_override) do port when is_integer(port) and port >= 0 -> port - _ -> get_setting!([:server, :port]) + _ -> settings!().server.port end end - @spec server_host() :: String.t() - def server_host, do: get_setting!([:server, :host]) - @spec validate!() :: :ok | {:error, term()} def validate! do - with {:ok, settings} <- validated_settings(), - :ok <- require_tracker_kind(settings), - :ok <- require_linear_token(settings), - :ok <- require_linear_project(settings) do - require_codex_command(settings) + with {:ok, settings} <- settings() do + validate_semantics(settings) end end @spec codex_runtime_settings(Path.t() | nil) :: {:ok, codex_runtime_settings()} | {:error, term()} def codex_runtime_settings(workspace \\ nil) do - with {:ok, settings} <- validated_settings() do + with {:ok, settings} <- settings() do {:ok, %{ - approval_policy: get_in(settings, [:codex, :approval_policy]), - thread_sandbox: get_in(settings, [:codex, :thread_sandbox]), + approval_policy: settings.codex.approval_policy, + thread_sandbox: settings.codex.thread_sandbox, turn_sandbox_policy: Schema.resolve_turn_sandbox_policy(settings, workspace) }} end end - defp get_setting!(path) do - validated_settings!() - |> get_in(path) - end - - defp validated_settings! do - case validated_settings() do - {:ok, settings} -> - settings - - {:error, reason} -> - raise ArgumentError, message: format_config_error(reason) - end - end - - defp validated_settings do - case current_workflow() do - {:ok, %{config: config}} when is_map(config) -> - Schema.parse(config) - - {:ok, _workflow} -> - Schema.parse(%{}) - - {:error, reason} -> - {:error, reason} - end - end - - defp require_tracker_kind(settings) do - case get_in(settings, [:tracker, :kind]) do - "linear" -> :ok - "memory" -> :ok - nil -> {:error, :missing_tracker_kind} - other -> {:error, {:unsupported_tracker_kind, other}} - end - end + defp validate_semantics(settings) do + cond do + is_nil(settings.tracker.kind) -> + {:error, :missing_tracker_kind} - defp require_linear_token(settings) do - case get_in(settings, [:tracker, :kind]) do - "linear" -> - if is_binary(get_in(settings, [:tracker, :api_key])) do - :ok - else - {:error, :missing_linear_api_token} - end + settings.tracker.kind not in ["linear", "memory"] -> + {:error, {:unsupported_tracker_kind, settings.tracker.kind}} - _ -> - :ok - end - end + settings.tracker.kind == "linear" and not is_binary(settings.tracker.api_key) -> + {:error, :missing_linear_api_token} - defp require_linear_project(settings) do - case get_in(settings, [:tracker, :kind]) do - "linear" -> - if is_binary(get_in(settings, [:tracker, :project_slug])) do - :ok - else - {:error, :missing_linear_project_slug} - end + settings.tracker.kind == "linear" and not is_binary(settings.tracker.project_slug) -> + {:error, :missing_linear_project_slug} - _ -> + true -> :ok end end - defp require_codex_command(settings) do - if is_binary(get_in(settings, [:codex, :command])) do - :ok - else - {:error, :missing_codex_command} - end - end - defp format_config_error(reason) do case reason do {:invalid_workflow_config, message} -> diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 52af553a..dcb67f98 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -7,20 +7,30 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false + @type t :: %__MODULE__{} + defmodule StringOrMap do @moduledoc false @behaviour Ecto.Type + @spec type() :: :map def type, do: :map + + @spec embed_as(term()) :: :self def embed_as(_format), do: :self + + @spec equal?(term(), term()) :: boolean() def equal?(left, right), do: left == right + @spec cast(term()) :: {:ok, String.t() | map()} | :error def cast(value) when is_binary(value) or is_map(value), do: {:ok, value} def cast(_value), do: :error + @spec load(term()) :: {:ok, String.t() | map()} | :error def load(value) when is_binary(value) or is_map(value), do: {:ok, value} def load(_value), do: :error + @spec dump(term()) :: {:ok, String.t() | map()} | :error def dump(value) when is_binary(value) or is_map(value), do: {:ok, value} def dump(_value), do: :error end @@ -33,18 +43,21 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :kind, :string - field :endpoint, :string, default: "https://api.linear.app/graphql" - field :api_key, :string - field :project_slug, :string - field :assignee, :string - field :active_states, {:array, :string}, default: ["Todo", "In Progress"] - field :terminal_states, {:array, :string}, default: ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] + field(:kind, :string) + field(:endpoint, :string, default: "https://api.linear.app/graphql") + field(:api_key, :string) + field(:project_slug, :string) + field(:assignee, :string) + field(:active_states, {:array, :string}, default: ["Todo", "In Progress"]) + field(:terminal_states, {:array, :string}, default: ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"]) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema - |> cast(attrs, [:kind, :endpoint, :api_key, :project_slug, :assignee, :active_states, :terminal_states], + |> cast( + attrs, + [:kind, :endpoint, :api_key, :project_slug, :assignee, :active_states, :terminal_states], empty_values: [] ) end @@ -57,9 +70,10 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :interval_ms, :integer, default: 30_000 + field(:interval_ms, :integer, default: 30_000) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema |> cast(attrs, [:interval_ms], empty_values: []) @@ -74,9 +88,10 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :root, :string, default: Path.join(System.tmp_dir!(), "symphony_workspaces") + field(:root, :string, default: Path.join(System.tmp_dir!(), "symphony_workspaces")) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema |> cast(attrs, [:root], empty_values: []) @@ -92,15 +107,18 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :max_concurrent_agents, :integer, default: 10 - field :max_turns, :integer, default: 20 - field :max_retry_backoff_ms, :integer, default: 300_000 - field :max_concurrent_agents_by_state, :map, default: %{} + field(:max_concurrent_agents, :integer, default: 10) + field(:max_turns, :integer, default: 20) + field(:max_retry_backoff_ms, :integer, default: 300_000) + field(:max_concurrent_agents_by_state, :map, default: %{}) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema - |> cast(attrs, [:max_concurrent_agents, :max_turns, :max_retry_backoff_ms, :max_concurrent_agents_by_state], + |> cast( + attrs, + [:max_concurrent_agents, :max_turns, :max_retry_backoff_ms, :max_concurrent_agents_by_state], empty_values: [] ) |> validate_number(:max_concurrent_agents, greater_than: 0) @@ -118,8 +136,9 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :command, :string, default: "codex app-server" - field :approval_policy, StringOrMap, + field(:command, :string, default: "codex app-server") + + field(:approval_policy, StringOrMap, default: %{ "reject" => %{ "sandbox_approval" => true, @@ -127,25 +146,32 @@ defmodule SymphonyElixir.Config.Schema do "mcp_elicitations" => true } } + ) - field :thread_sandbox, :string, default: "workspace-write" - field :turn_sandbox_policy, :map - field :turn_timeout_ms, :integer, default: 3_600_000 - field :read_timeout_ms, :integer, default: 5_000 - field :stall_timeout_ms, :integer, default: 300_000 + field(:thread_sandbox, :string, default: "workspace-write") + field(:turn_sandbox_policy, :map) + field(:turn_timeout_ms, :integer, default: 3_600_000) + field(:read_timeout_ms, :integer, default: 5_000) + field(:stall_timeout_ms, :integer, default: 300_000) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema - |> cast(attrs, [ - :command, - :approval_policy, - :thread_sandbox, - :turn_sandbox_policy, - :turn_timeout_ms, - :read_timeout_ms, - :stall_timeout_ms - ], empty_values: []) + |> cast( + attrs, + [ + :command, + :approval_policy, + :thread_sandbox, + :turn_sandbox_policy, + :turn_timeout_ms, + :read_timeout_ms, + :stall_timeout_ms + ], + empty_values: [] + ) + |> validate_required([:command]) |> validate_number(:turn_timeout_ms, greater_than: 0) |> validate_number(:read_timeout_ms, greater_than: 0) |> validate_number(:stall_timeout_ms, greater_than_or_equal_to: 0) @@ -159,18 +185,17 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :after_create, :string - field :before_run, :string - field :after_run, :string - field :before_remove, :string - field :timeout_ms, :integer, default: 60_000 + field(:after_create, :string) + field(:before_run, :string) + field(:after_run, :string) + field(:before_remove, :string) + field(:timeout_ms, :integer, default: 60_000) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema - |> cast(attrs, [:after_create, :before_run, :after_run, :before_remove, :timeout_ms], - empty_values: [] - ) + |> cast(attrs, [:after_create, :before_run, :after_run, :before_remove, :timeout_ms], empty_values: []) |> validate_number(:timeout_ms, greater_than: 0) end end @@ -182,11 +207,12 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :dashboard_enabled, :boolean, default: true - field :refresh_ms, :integer, default: 1_000 - field :render_interval_ms, :integer, default: 16 + field(:dashboard_enabled, :boolean, default: true) + field(:refresh_ms, :integer, default: 1_000) + field(:render_interval_ms, :integer, default: 16) end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema |> cast(attrs, [:dashboard_enabled, :refresh_ms, :render_interval_ms], empty_values: []) @@ -202,10 +228,11 @@ defmodule SymphonyElixir.Config.Schema do @primary_key false embedded_schema do - field :port, :integer - field :host, :string, default: "127.0.0.1" + field(:port, :integer) + field(:host, :string, default: "127.0.0.1") end + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() def changeset(schema, attrs) do schema |> cast(attrs, [:port, :host], empty_values: []) @@ -214,17 +241,17 @@ defmodule SymphonyElixir.Config.Schema do end embedded_schema do - embeds_one :tracker, Tracker, on_replace: :update, defaults_to_struct: true - embeds_one :polling, Polling, on_replace: :update, defaults_to_struct: true - embeds_one :workspace, Workspace, on_replace: :update, defaults_to_struct: true - embeds_one :agent, Agent, on_replace: :update, defaults_to_struct: true - embeds_one :codex, Codex, on_replace: :update, defaults_to_struct: true - embeds_one :hooks, Hooks, on_replace: :update, defaults_to_struct: true - embeds_one :observability, Observability, on_replace: :update, defaults_to_struct: true - embeds_one :server, Server, on_replace: :update, defaults_to_struct: true + embeds_one(:tracker, Tracker, on_replace: :update, defaults_to_struct: true) + embeds_one(:polling, Polling, on_replace: :update, defaults_to_struct: true) + embeds_one(:workspace, Workspace, on_replace: :update, defaults_to_struct: true) + embeds_one(:agent, Agent, on_replace: :update, defaults_to_struct: true) + embeds_one(:codex, Codex, on_replace: :update, defaults_to_struct: true) + embeds_one(:hooks, Hooks, on_replace: :update, defaults_to_struct: true) + embeds_one(:observability, Observability, on_replace: :update, defaults_to_struct: true) + embeds_one(:server, Server, on_replace: :update, defaults_to_struct: true) end - @spec parse(map()) :: {:ok, map()} | {:error, {:invalid_workflow_config, String.t()}} + @spec parse(map()) :: {:ok, %__MODULE__{}} | {:error, {:invalid_workflow_config, String.t()}} def parse(config) when is_map(config) do config |> normalize_keys() @@ -240,14 +267,14 @@ defmodule SymphonyElixir.Config.Schema do end end - @spec resolve_turn_sandbox_policy(map(), Path.t() | nil) :: map() + @spec resolve_turn_sandbox_policy(%__MODULE__{}, Path.t() | nil) :: map() def resolve_turn_sandbox_policy(settings, workspace \\ nil) do - case get_in(settings, [:codex, :turn_sandbox_policy]) do + case settings.codex.turn_sandbox_policy do %{} = policy -> policy _ -> - default_turn_sandbox_policy(workspace || get_in(settings, [:workspace, :root])) + default_turn_sandbox_policy(workspace || settings.workspace.root) end end @@ -257,6 +284,7 @@ defmodule SymphonyElixir.Config.Schema do end @doc false + @spec normalize_state_limits(nil | map()) :: map() def normalize_state_limits(nil), do: %{} def normalize_state_limits(limits) when is_map(limits) do @@ -266,6 +294,7 @@ defmodule SymphonyElixir.Config.Schema do end @doc false + @spec validate_state_limits(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t() def validate_state_limits(changeset, field) do validate_change(changeset, field, fn ^field, limits -> Enum.flat_map(limits, fn {state_name, limit} -> @@ -297,31 +326,25 @@ defmodule SymphonyElixir.Config.Schema do end defp finalize_settings(settings) do - settings - |> deep_to_map() - |> update_in([:tracker, :api_key], &resolve_secret_setting(&1, System.get_env("LINEAR_API_KEY"))) - |> update_in([:tracker, :assignee], &resolve_secret_setting(&1, System.get_env("LINEAR_ASSIGNEE"))) - |> update_in([:workspace, :root], &resolve_path_value(&1, Path.join(System.tmp_dir!(), "symphony_workspaces"))) - |> update_in([:codex, :approval_policy], &normalize_keys/1) - |> update_in([:codex, :turn_sandbox_policy], &normalize_optional_map/1) - end + tracker = %{ + settings.tracker + | api_key: resolve_secret_setting(settings.tracker.api_key, System.get_env("LINEAR_API_KEY")), + assignee: resolve_secret_setting(settings.tracker.assignee, System.get_env("LINEAR_ASSIGNEE")) + } - defp deep_to_map(%_{} = struct) do - struct - |> Map.from_struct() - |> Enum.reduce(%{}, fn {key, value}, acc -> - Map.put(acc, key, deep_to_map(value)) - end) - end + workspace = %{ + settings.workspace + | root: resolve_path_value(settings.workspace.root, Path.join(System.tmp_dir!(), "symphony_workspaces")) + } - defp deep_to_map(value) when is_map(value) do - Enum.reduce(value, %{}, fn {key, nested}, acc -> - Map.put(acc, key, deep_to_map(nested)) - end) - end + codex = %{ + settings.codex + | approval_policy: normalize_keys(settings.codex.approval_policy), + turn_sandbox_policy: normalize_optional_map(settings.codex.turn_sandbox_policy) + } - defp deep_to_map(value) when is_list(value), do: Enum.map(value, &deep_to_map/1) - defp deep_to_map(value), do: value + %{settings | tracker: tracker, workspace: workspace, codex: codex} + end defp normalize_keys(value) when is_map(value) do Enum.reduce(value, %{}, fn {key, raw_value}, normalized -> @@ -334,7 +357,6 @@ defmodule SymphonyElixir.Config.Schema do defp normalize_optional_map(nil), do: nil defp normalize_optional_map(value) when is_map(value), do: normalize_keys(value) - defp normalize_optional_map(value), do: value defp normalize_key(value) when is_atom(value), do: Atom.to_string(value) defp normalize_key(value), do: to_string(value) @@ -360,10 +382,6 @@ defmodule SymphonyElixir.Config.Schema do end end - defp resolve_secret_setting(_value, fallback), do: normalize_secret_value(fallback) - - defp resolve_path_value(nil, default), do: Path.expand(default) - defp resolve_path_value(value, default) when is_binary(value) do case normalize_path_token(value) do :missing -> @@ -377,10 +395,6 @@ defmodule SymphonyElixir.Config.Schema do end end - defp resolve_path_value(_value, default), do: Path.expand(default) - - defp resolve_env_value(nil, fallback), do: fallback - defp resolve_env_value(value, fallback) when is_binary(value) do case env_reference_name(value) do {:ok, env_name} -> @@ -395,8 +409,6 @@ defmodule SymphonyElixir.Config.Schema do end end - defp resolve_env_value(_value, fallback), do: fallback - defp normalize_path_token(value) when is_binary(value) do case env_reference_name(value) do {:ok, env_name} -> resolve_env_token(env_name) @@ -467,13 +479,7 @@ defmodule SymphonyElixir.Config.Schema do end defp flatten_errors(errors, prefix) when is_list(errors) do - Enum.flat_map(errors, fn - value when is_binary(value) -> - [prefix <> " " <> value] - - value -> - flatten_errors(value, prefix) - end) + Enum.map(errors, &(prefix <> " " <> &1)) end defp translate_error({message, options}) do @@ -482,9 +488,6 @@ defmodule SymphonyElixir.Config.Schema do end) end - defp error_value_to_string(value) when is_binary(value), do: value defp error_value_to_string(value) when is_atom(value), do: Atom.to_string(value) - defp error_value_to_string(value) when is_integer(value), do: Integer.to_string(value) - defp error_value_to_string(value) when is_float(value), do: Float.to_string(value) defp error_value_to_string(value), do: inspect(value) end diff --git a/elixir/lib/symphony_elixir/http_server.ex b/elixir/lib/symphony_elixir/http_server.ex index 47686e93..5f947b88 100644 --- a/elixir/lib/symphony_elixir/http_server.ex +++ b/elixir/lib/symphony_elixir/http_server.ex @@ -20,7 +20,7 @@ defmodule SymphonyElixir.HttpServer do def start_link(opts \\ []) do case Keyword.get(opts, :port, Config.server_port()) do port when is_integer(port) and port >= 0 -> - host = Keyword.get(opts, :host, Config.server_host()) + host = Keyword.get(opts, :host, Config.settings!().server.host) orchestrator = Keyword.get(opts, :orchestrator, Orchestrator) snapshot_timeout_ms = Keyword.get(opts, :snapshot_timeout_ms, 15_000) diff --git a/elixir/lib/symphony_elixir/linear/client.ex b/elixir/lib/symphony_elixir/linear/client.ex index ad8eee55..0ff290fe 100644 --- a/elixir/lib/symphony_elixir/linear/client.ex +++ b/elixir/lib/symphony_elixir/linear/client.ex @@ -105,10 +105,11 @@ defmodule SymphonyElixir.Linear.Client do @spec fetch_candidate_issues() :: {:ok, [Issue.t()]} | {:error, term()} def fetch_candidate_issues do - project_slug = Config.linear_project_slug() + tracker = Config.settings!().tracker + project_slug = tracker.project_slug cond do - is_nil(Config.linear_api_token()) -> + is_nil(tracker.api_key) -> {:error, :missing_linear_api_token} is_nil(project_slug) -> @@ -116,7 +117,7 @@ defmodule SymphonyElixir.Linear.Client do true -> with {:ok, assignee_filter} <- routing_assignee_filter() do - do_fetch_by_states(project_slug, Config.linear_active_states(), assignee_filter) + do_fetch_by_states(project_slug, tracker.active_states, assignee_filter) end end end @@ -128,10 +129,11 @@ defmodule SymphonyElixir.Linear.Client do if normalized_states == [] do {:ok, []} else - project_slug = Config.linear_project_slug() + tracker = Config.settings!().tracker + project_slug = tracker.project_slug cond do - is_nil(Config.linear_api_token()) -> + is_nil(tracker.api_key) -> {:error, :missing_linear_api_token} is_nil(project_slug) -> @@ -325,7 +327,7 @@ defmodule SymphonyElixir.Linear.Client do end defp graphql_headers do - case Config.linear_api_token() do + case Config.settings!().tracker.api_key do nil -> {:error, :missing_linear_api_token} @@ -339,7 +341,7 @@ defmodule SymphonyElixir.Linear.Client do end defp post_graphql_request(payload, headers) do - Req.post(Config.linear_endpoint(), + Req.post(Config.settings!().tracker.endpoint, headers: headers, json: payload, connect_options: [timeout: 30_000] @@ -432,7 +434,7 @@ defmodule SymphonyElixir.Linear.Client do defp assignee_id(%{} = assignee), do: normalize_assignee_match_value(assignee["id"]) defp routing_assignee_filter do - case Config.linear_assignee() do + case Config.settings!().tracker.assignee do nil -> {:ok, nil} diff --git a/elixir/lib/symphony_elixir/orchestrator.ex b/elixir/lib/symphony_elixir/orchestrator.ex index 828440b3..4286ce1f 100644 --- a/elixir/lib/symphony_elixir/orchestrator.ex +++ b/elixir/lib/symphony_elixir/orchestrator.ex @@ -49,10 +49,11 @@ defmodule SymphonyElixir.Orchestrator do @impl true def init(_opts) do now_ms = System.monotonic_time(:millisecond) + config = Config.settings!() state = %State{ - poll_interval_ms: Config.poll_interval_ms(), - max_concurrent_agents: Config.max_concurrent_agents(), + poll_interval_ms: config.polling.interval_ms, + max_concurrent_agents: config.agent.max_concurrent_agents, next_poll_due_at_ms: now_ms, poll_check_in_progress: false, codex_totals: @empty_codex_totals, @@ -196,10 +197,6 @@ defmodule SymphonyElixir.Orchestrator do state - {:error, :missing_codex_command} -> - Logger.error("Codex command missing in WORKFLOW.md") - state - {:error, {:invalid_workflow_config, message}} -> Logger.error("Invalid WORKFLOW.md config: #{message}") state @@ -357,7 +354,7 @@ defmodule SymphonyElixir.Orchestrator do end defp reconcile_stalled_running_issues(%State{} = state) do - timeout_ms = Config.codex_stall_timeout_ms() + timeout_ms = Config.settings!().codex.stall_timeout_ms cond do timeout_ms <= 0 -> @@ -554,14 +551,14 @@ defmodule SymphonyElixir.Orchestrator do end defp terminal_state_set do - Config.linear_terminal_states() + Config.settings!().tracker.terminal_states |> Enum.map(&normalize_issue_state/1) |> Enum.filter(&(&1 != "")) |> MapSet.new() end defp active_state_set do - Config.linear_active_states() + Config.settings!().tracker.active_states |> Enum.map(&normalize_issue_state/1) |> Enum.filter(&(&1 != "")) |> MapSet.new() @@ -766,7 +763,7 @@ defmodule SymphonyElixir.Orchestrator do defp cleanup_issue_workspace(_identifier), do: :ok defp run_terminal_workspace_cleanup do - case Tracker.fetch_issues_by_states(Config.linear_terminal_states()) do + case Tracker.fetch_issues_by_states(Config.settings!().tracker.terminal_states) do {:ok, issues} -> issues |> Enum.each(fn @@ -820,7 +817,7 @@ defmodule SymphonyElixir.Orchestrator do defp failure_retry_delay(attempt) do max_delay_power = min(attempt - 1, 10) - min(@failure_retry_base_ms * (1 <<< max_delay_power), Config.max_retry_backoff_ms()) + min(@failure_retry_base_ms * (1 <<< max_delay_power), Config.settings!().agent.max_retry_backoff_ms) end defp normalize_retry_attempt(attempt) when is_integer(attempt) and attempt > 0, do: attempt @@ -869,7 +866,8 @@ defmodule SymphonyElixir.Orchestrator do defp available_slots(%State{} = state) do max( - (state.max_concurrent_agents || Config.max_concurrent_agents()) - map_size(state.running), + (state.max_concurrent_agents || Config.settings!().agent.max_concurrent_agents) - + map_size(state.running), 0 ) end @@ -1090,10 +1088,12 @@ defmodule SymphonyElixir.Orchestrator do defp record_session_completion_totals(state, _running_entry), do: state defp refresh_runtime_config(%State{} = state) do + config = Config.settings!() + %{ state - | poll_interval_ms: Config.poll_interval_ms(), - max_concurrent_agents: Config.max_concurrent_agents() + | poll_interval_ms: config.polling.interval_ms, + max_concurrent_agents: config.agent.max_concurrent_agents } end diff --git a/elixir/lib/symphony_elixir/status_dashboard.ex b/elixir/lib/symphony_elixir/status_dashboard.ex index 19b628bf..8cd4b17f 100644 --- a/elixir/lib/symphony_elixir/status_dashboard.ex +++ b/elixir/lib/symphony_elixir/status_dashboard.ex @@ -99,10 +99,11 @@ defmodule SymphonyElixir.StatusDashboard do refresh_ms_override = keyword_override(opts, :refresh_ms) enabled_override = keyword_override(opts, :enabled) render_interval_ms_override = keyword_override(opts, :render_interval_ms) - refresh_ms = refresh_ms_override || Config.observability_refresh_ms() - render_interval_ms = render_interval_ms_override || Config.observability_render_interval_ms() + observability = Config.settings!().observability + refresh_ms = refresh_ms_override || observability.refresh_ms + render_interval_ms = render_interval_ms_override || observability.render_interval_ms render_fun = Keyword.get(opts, :render_fun, &render_to_terminal/1) - enabled = resolve_override(enabled_override, Config.observability_enabled?() and dashboard_enabled?()) + enabled = resolve_override(enabled_override, observability.dashboard_enabled and dashboard_enabled?()) schedule_tick(refresh_ms, enabled) {:ok, @@ -176,11 +177,13 @@ defmodule SymphonyElixir.StatusDashboard do def handle_info(:tick, state), do: {:noreply, state} defp refresh_runtime_config(%__MODULE__{} = state) do + observability = Config.settings!().observability + %{ state - | enabled: resolve_override(state.enabled_override, Config.observability_enabled?() and dashboard_enabled?()), - refresh_ms: state.refresh_ms_override || Config.observability_refresh_ms(), - render_interval_ms: state.render_interval_ms_override || Config.observability_render_interval_ms() + | enabled: resolve_override(state.enabled_override, observability.dashboard_enabled and dashboard_enabled?()), + refresh_ms: state.refresh_ms_override || observability.refresh_ms, + render_interval_ms: state.render_interval_ms_override || observability.render_interval_ms } end @@ -338,7 +341,7 @@ defmodule SymphonyElixir.StatusDashboard do codex_total_tokens = Map.get(codex_totals, :total_tokens, 0) codex_seconds_running = Map.get(codex_totals, :seconds_running, 0) agent_count = length(running) - max_agents = Config.max_concurrent_agents() + max_agents = Config.settings!().agent.max_concurrent_agents running_event_width = running_event_width(terminal_columns_override) running_rows = format_running_rows(running, running_event_width) running_to_backoff_spacer = if(running == [], do: [], else: ["│"]) @@ -391,7 +394,7 @@ defmodule SymphonyElixir.StatusDashboard do defp format_project_link_lines do project_part = - case Config.linear_project_slug() do + case Config.settings!().tracker.project_slug do project_slug when is_binary(project_slug) and project_slug != "" -> colorize(linear_project_url(project_slug), @ansi_cyan) @@ -427,7 +430,7 @@ defmodule SymphonyElixir.StatusDashboard do defp linear_project_url(project_slug), do: "https://linear.app/project/#{project_slug}/issues" defp dashboard_url do - dashboard_url(Config.server_host(), Config.server_port(), HttpServer.bound_port()) + dashboard_url(Config.settings!().server.host, Config.server_port(), HttpServer.bound_port()) end defp dashboard_url(_host, nil, _bound_port), do: nil diff --git a/elixir/lib/symphony_elixir/tracker.ex b/elixir/lib/symphony_elixir/tracker.ex index 504b54af..000b6edf 100644 --- a/elixir/lib/symphony_elixir/tracker.ex +++ b/elixir/lib/symphony_elixir/tracker.ex @@ -38,7 +38,7 @@ defmodule SymphonyElixir.Tracker do @spec adapter() :: module() def adapter do - case Config.tracker_kind() do + case Config.settings!().tracker.kind do "memory" -> SymphonyElixir.Tracker.Memory _ -> SymphonyElixir.Linear.Adapter end diff --git a/elixir/lib/symphony_elixir/workspace.ex b/elixir/lib/symphony_elixir/workspace.ex index e8c085ea..2401573f 100644 --- a/elixir/lib/symphony_elixir/workspace.ex +++ b/elixir/lib/symphony_elixir/workspace.ex @@ -71,7 +71,7 @@ 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.workspace_root(), safe_id) + workspace = Path.join(Config.settings!().workspace.root, safe_id) remove(workspace) :ok @@ -84,8 +84,9 @@ defmodule SymphonyElixir.Workspace do @spec run_before_run_hook(Path.t(), map() | String.t() | nil) :: :ok | {:error, term()} def run_before_run_hook(workspace, issue_or_identifier) when is_binary(workspace) do issue_context = issue_context(issue_or_identifier) + hooks = Config.settings!().hooks - case Config.workspace_hooks()[:before_run] do + case hooks.before_run do nil -> :ok @@ -97,8 +98,9 @@ defmodule SymphonyElixir.Workspace do @spec run_after_run_hook(Path.t(), map() | String.t() | nil) :: :ok def run_after_run_hook(workspace, issue_or_identifier) when is_binary(workspace) do issue_context = issue_context(issue_or_identifier) + hooks = Config.settings!().hooks - case Config.workspace_hooks()[:after_run] do + case hooks.after_run do nil -> :ok @@ -109,7 +111,7 @@ defmodule SymphonyElixir.Workspace do end defp workspace_path_for_issue(safe_id) when is_binary(safe_id) do - Path.join(Config.workspace_root(), safe_id) + Path.join(Config.settings!().workspace.root, safe_id) end defp safe_identifier(identifier) do @@ -123,9 +125,11 @@ defmodule SymphonyElixir.Workspace do end defp maybe_run_after_create_hook(workspace, issue_context, created?) do + hooks = Config.settings!().hooks + case created? do true -> - case Config.workspace_hooks()[:after_create] do + case hooks.after_create do nil -> :ok @@ -139,9 +143,11 @@ defmodule SymphonyElixir.Workspace do end defp maybe_run_before_remove_hook(workspace) do + hooks = Config.settings!().hooks + case File.dir?(workspace) do true -> - case Config.workspace_hooks()[:before_remove] do + case hooks.before_remove do nil -> :ok @@ -164,7 +170,7 @@ defmodule SymphonyElixir.Workspace do defp ignore_hook_failure({:error, _reason}), do: :ok defp run_hook(command, workspace, issue_context, hook_name) do - timeout_ms = Config.workspace_hooks()[:timeout_ms] + timeout_ms = Config.settings!().hooks.timeout_ms Logger.info("Running workspace hook hook=#{hook_name} #{issue_log_context(issue_context)} workspace=#{workspace}") @@ -212,7 +218,7 @@ defmodule SymphonyElixir.Workspace do defp validate_workspace_path(workspace) when is_binary(workspace) do expanded_workspace = Path.expand(workspace) - root = Path.expand(Config.workspace_root()) + root = Path.expand(Config.settings!().workspace.root) root_prefix = root <> "/" cond do diff --git a/elixir/lib/symphony_elixir_web/presenter.ex b/elixir/lib/symphony_elixir_web/presenter.ex index 34eb1e66..dc78ab32 100644 --- a/elixir/lib/symphony_elixir_web/presenter.ex +++ b/elixir/lib/symphony_elixir_web/presenter.ex @@ -66,7 +66,7 @@ defmodule SymphonyElixirWeb.Presenter do issue_id: issue_id_from_entries(running, retry), status: issue_status(running, retry), workspace: %{ - path: Path.join(Config.workspace_root(), issue_identifier) + path: Path.join(Config.settings!().workspace.root, issue_identifier) }, attempts: %{ restart_count: restart_count(retry), diff --git a/elixir/test/symphony_elixir/core_test.exs b/elixir/test/symphony_elixir/core_test.exs index 0f3681bd..023305eb 100644 --- a/elixir/test/symphony_elixir/core_test.exs +++ b/elixir/test/symphony_elixir/core_test.exs @@ -11,30 +11,31 @@ defmodule SymphonyElixir.CoreTest do codex_command: nil ) - assert Config.poll_interval_ms() == 30_000 - assert Config.linear_active_states() == ["Todo", "In Progress"] - assert Config.linear_terminal_states() == ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] - assert Config.linear_assignee() == nil - assert Config.agent_max_turns() == 20 + config = Config.settings!() + assert config.polling.interval_ms == 30_000 + assert config.tracker.active_states == ["Todo", "In Progress"] + assert config.tracker.terminal_states == ["Closed", "Cancelled", "Canceled", "Duplicate", "Done"] + assert config.tracker.assignee == nil + assert config.agent.max_turns == 20 write_workflow_file!(Workflow.workflow_file_path(), poll_interval_ms: "invalid") assert_raise ArgumentError, ~r/interval_ms/, fn -> - Config.poll_interval_ms() + Config.settings!().polling.interval_ms end assert {:error, {:invalid_workflow_config, message}} = Config.validate!() assert message =~ "polling.interval_ms" write_workflow_file!(Workflow.workflow_file_path(), poll_interval_ms: 45_000) - assert Config.poll_interval_ms() == 45_000 + assert Config.settings!().polling.interval_ms == 45_000 write_workflow_file!(Workflow.workflow_file_path(), max_turns: 0) assert {:error, {:invalid_workflow_config, message}} = Config.validate!() assert message =~ "agent.max_turns" write_workflow_file!(Workflow.workflow_file_path(), max_turns: 5) - assert Config.agent_max_turns() == 5 + assert Config.settings!().agent.max_turns == 5 write_workflow_file!(Workflow.workflow_file_path(), tracker_active_states: "Todo, Review,") assert {:error, {:invalid_workflow_config, message}} = Config.validate!() @@ -52,8 +53,13 @@ defmodule SymphonyElixir.CoreTest do codex_command: "" ) + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "codex.command" + assert message =~ "can't be blank" + + write_workflow_file!(Workflow.workflow_file_path(), codex_command: " ") assert :ok = Config.validate!() - assert Config.codex_command() == "" + assert Config.settings!().codex.command == " " write_workflow_file!(Workflow.workflow_file_path(), codex_command: "/bin/sh app-server") assert :ok = Config.validate!() @@ -122,8 +128,8 @@ defmodule SymphonyElixir.CoreTest do codex_command: "/bin/sh app-server" ) - assert Config.linear_api_token() == env_api_key - assert Config.linear_project_slug() == "project" + assert Config.settings!().tracker.api_key == env_api_key + assert Config.settings!().tracker.project_slug == "project" assert :ok = Config.validate!() end @@ -140,7 +146,7 @@ defmodule SymphonyElixir.CoreTest do codex_command: "/bin/sh app-server" ) - assert Config.linear_assignee() == env_assignee + assert Config.settings!().tracker.assignee == env_assignee end test "workflow file path defaults to WORKFLOW.md in the current working directory when app env is unset" do diff --git a/elixir/test/symphony_elixir/extensions_test.exs b/elixir/test/symphony_elixir/extensions_test.exs index 59c8d058..27b2416b 100644 --- a/elixir/test/symphony_elixir/extensions_test.exs +++ b/elixir/test/symphony_elixir/extensions_test.exs @@ -187,7 +187,7 @@ defmodule SymphonyElixir.ExtensionsTest do Application.put_env(:symphony_elixir, :memory_tracker_recipient, self()) write_workflow_file!(Workflow.workflow_file_path(), tracker_kind: "memory") - assert Config.tracker_kind() == "memory" + assert Config.settings!().tracker.kind == "memory" assert SymphonyElixir.Tracker.adapter() == Memory assert {:ok, [^issue]} = SymphonyElixir.Tracker.fetch_candidate_issues() assert {:ok, [^issue]} = SymphonyElixir.Tracker.fetch_issues_by_states([" in progress ", 42]) @@ -382,7 +382,7 @@ defmodule SymphonyElixir.ExtensionsTest do "issue_identifier" => "MT-HTTP", "issue_id" => "issue-http", "status" => "running", - "workspace" => %{"path" => Path.join(Config.workspace_root(), "MT-HTTP")}, + "workspace" => %{"path" => Path.join(Config.settings!().workspace.root, "MT-HTTP")}, "attempts" => %{"restart_count" => 0, "current_retry_attempt" => 0}, "running" => %{ "session_id" => "thread-http", diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 1e0b33fd..7b681d54 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -1,5 +1,8 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do use SymphonyElixir.TestSupport + alias Ecto.Changeset + alias SymphonyElixir.Config.Schema + alias SymphonyElixir.Config.Schema.{Codex, StringOrMap} alias SymphonyElixir.Linear.Client test "workspace bootstrap can be implemented in after_create hook" do @@ -533,8 +536,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do hook_before_remove: "echo before_remove > \"#{before_remove_marker}\"" ) - assert Config.workspace_hooks().after_create =~ "echo after_create > after_create.log" - assert Config.workspace_hooks().before_remove =~ "echo before_remove >" + config = Config.settings!() + assert config.hooks.after_create =~ "echo after_create > after_create.log" + assert config.hooks.before_remove =~ "echo before_remove >" assert {:ok, workspace} = Workspace.create_for_issue("MT-HOOKS") assert File.read!(Path.join(workspace, "after_create.log")) == "after_create\n" @@ -655,14 +659,15 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do tracker_project_slug: nil ) - assert Config.linear_endpoint() == "https://api.linear.app/graphql" - assert Config.linear_api_token() == nil - assert Config.linear_project_slug() == nil - assert Config.workspace_root() == Path.join(System.tmp_dir!(), "symphony_workspaces") - assert Config.max_concurrent_agents() == 10 - assert Config.codex_command() == "codex app-server" + config = Config.settings!() + assert config.tracker.endpoint == "https://api.linear.app/graphql" + assert config.tracker.api_key == nil + assert config.tracker.project_slug == nil + assert config.workspace.root == Path.join(System.tmp_dir!(), "symphony_workspaces") + assert config.agent.max_concurrent_agents == 10 + assert config.codex.command == "codex app-server" - assert Config.codex_approval_policy() == %{ + assert config.codex.approval_policy == %{ "reject" => %{ "sandbox_approval" => true, "rules" => true, @@ -670,7 +675,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } } - assert Config.codex_thread_sandbox() == "workspace-write" + assert config.codex.thread_sandbox == "workspace-write" assert Config.codex_turn_sandbox_policy() == %{ "type" => "workspaceWrite", @@ -681,12 +686,12 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do "excludeSlashTmp" => false } - assert Config.codex_turn_timeout_ms() == 3_600_000 - assert Config.codex_read_timeout_ms() == 5_000 - assert Config.codex_stall_timeout_ms() == 300_000 + assert config.codex.turn_timeout_ms == 3_600_000 + assert config.codex.read_timeout_ms == 5_000 + assert config.codex.stall_timeout_ms == 300_000 write_workflow_file!(Workflow.workflow_file_path(), codex_command: "codex app-server --model gpt-5.3-codex") - assert Config.codex_command() == "codex app-server --model gpt-5.3-codex" + assert Config.settings!().codex.command == "codex app-server --model gpt-5.3-codex" write_workflow_file!(Workflow.workflow_file_path(), codex_approval_policy: "on-request", @@ -694,8 +699,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do codex_turn_sandbox_policy: %{type: "workspaceWrite", writableRoots: ["/tmp/workspace", "/tmp/cache"]} ) - assert Config.codex_approval_policy() == "on-request" - assert Config.codex_thread_sandbox() == "workspace-write" + config = Config.settings!() + assert config.codex.approval_policy == "on-request" + assert config.codex.thread_sandbox == "workspace-write" assert Config.codex_turn_sandbox_policy() == %{ "type" => "workspaceWrite", @@ -741,11 +747,11 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do write_workflow_file!(Workflow.workflow_file_path(), codex_approval_policy: "") assert :ok = Config.validate!() - assert Config.codex_approval_policy() == "" + assert Config.settings!().codex.approval_policy == "" write_workflow_file!(Workflow.workflow_file_path(), codex_thread_sandbox: "") assert :ok = Config.validate!() - assert Config.codex_thread_sandbox() == "" + assert Config.settings!().codex.thread_sandbox == "" write_workflow_file!(Workflow.workflow_file_path(), codex_turn_sandbox_policy: "bad") assert {:error, {:invalid_workflow_config, message}} = Config.validate!() @@ -760,8 +766,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do } ) - assert Config.codex_approval_policy() == "future-policy" - assert Config.codex_thread_sandbox() == "future-sandbox" + config = Config.settings!() + assert config.codex.approval_policy == "future-policy" + assert config.codex.thread_sandbox == "future-sandbox" assert Config.codex_turn_sandbox_policy() == %{ "type" => "futureSandbox", @@ -771,7 +778,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert :ok = Config.validate!() write_workflow_file!(Workflow.workflow_file_path(), codex_command: "codex app-server") - assert Config.codex_command() == "codex app-server" + assert Config.settings!().codex.command == "codex app-server" end test "config resolves $VAR references for env-backed secret and path values" do @@ -798,9 +805,10 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do codex_command: "#{codex_bin} app-server" ) - assert Config.linear_api_token() == api_key - assert Config.workspace_root() == Path.expand(workspace_root) - assert Config.codex_command() == "#{codex_bin} app-server" + config = Config.settings!() + assert config.tracker.api_key == api_key + assert config.workspace.root == Path.expand(workspace_root) + assert config.codex.command == "#{codex_bin} app-server" end test "config no longer resolves legacy env: references" do @@ -825,8 +833,9 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do workspace_root: "env:#{workspace_env_var}" ) - assert Config.linear_api_token() == "env:#{api_key_env_var}" - assert Config.workspace_root() == Path.expand("env:#{workspace_env_var}") + config = Config.settings!() + assert config.tracker.api_key == "env:#{api_key_env_var}" + assert config.workspace.root == Path.expand("env:#{workspace_env_var}") end test "config supports per-state max concurrent agent overrides" do @@ -843,7 +852,7 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do File.write!(Workflow.workflow_file_path(), workflow) - assert Config.max_concurrent_agents() == 10 + assert Config.settings!().agent.max_concurrent_agents == 10 assert Config.max_concurrent_agents_for_state("Todo") == 1 assert Config.max_concurrent_agents_for_state("In Progress") == 4 assert Config.max_concurrent_agents_for_state("In Review") == 2 @@ -851,6 +860,122 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert Config.max_concurrent_agents_for_state(:not_a_string) == 10 end + test "schema helpers cover custom type and state limit validation" do + assert StringOrMap.type() == :map + assert StringOrMap.embed_as(:json) == :self + assert StringOrMap.equal?(%{"a" => 1}, %{"a" => 1}) + refute StringOrMap.equal?(%{"a" => 1}, %{"a" => 2}) + + assert {:ok, "value"} = StringOrMap.cast("value") + assert {:ok, %{"a" => 1}} = StringOrMap.cast(%{"a" => 1}) + assert :error = StringOrMap.cast(123) + + assert {:ok, "value"} = StringOrMap.load("value") + assert :error = StringOrMap.load(123) + + assert {:ok, %{"a" => 1}} = StringOrMap.dump(%{"a" => 1}) + assert :error = StringOrMap.dump(123) + + assert Schema.normalize_state_limits(nil) == %{} + + assert Schema.normalize_state_limits(%{"In Progress" => 2, todo: 1}) == %{ + "todo" => 1, + "in progress" => 2 + } + + changeset = + {%{}, %{limits: :map}} + |> Changeset.cast(%{limits: %{"" => 1, "todo" => 0}}, [:limits]) + |> Schema.validate_state_limits(:limits) + + assert changeset.errors == [ + limits: {"state names must not be blank", []}, + limits: {"limits must be positive integers", []} + ] + end + + test "schema parse normalizes policy keys and env-backed fallbacks" do + missing_workspace_env = "SYMP_MISSING_WORKSPACE_#{System.unique_integer([:positive])}" + empty_secret_env = "SYMP_EMPTY_SECRET_#{System.unique_integer([:positive])}" + missing_secret_env = "SYMP_MISSING_SECRET_#{System.unique_integer([:positive])}" + + previous_missing_workspace_env = System.get_env(missing_workspace_env) + previous_empty_secret_env = System.get_env(empty_secret_env) + previous_missing_secret_env = System.get_env(missing_secret_env) + previous_linear_api_key = System.get_env("LINEAR_API_KEY") + + System.delete_env(missing_workspace_env) + System.put_env(empty_secret_env, "") + System.delete_env(missing_secret_env) + System.put_env("LINEAR_API_KEY", "fallback-linear-token") + + on_exit(fn -> + restore_env(missing_workspace_env, previous_missing_workspace_env) + restore_env(empty_secret_env, previous_empty_secret_env) + restore_env(missing_secret_env, previous_missing_secret_env) + restore_env("LINEAR_API_KEY", previous_linear_api_key) + end) + + assert {:ok, settings} = + Schema.parse(%{ + tracker: %{api_key: "$#{empty_secret_env}"}, + workspace: %{root: "$#{missing_workspace_env}"}, + codex: %{approval_policy: %{reject: %{sandbox_approval: true}}} + }) + + assert settings.tracker.api_key == nil + assert settings.workspace.root == Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces")) + + assert settings.codex.approval_policy == %{ + "reject" => %{"sandbox_approval" => true} + } + + assert {:ok, settings} = + Schema.parse(%{ + tracker: %{api_key: "$#{missing_secret_env}"}, + workspace: %{root: ""} + }) + + assert settings.tracker.api_key == "fallback-linear-token" + assert settings.workspace.root == Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces")) + end + + test "schema resolves sandbox policies from explicit and default workspaces" do + explicit_policy = %{"type" => "workspaceWrite", "writableRoots" => ["/tmp/explicit"]} + + assert Schema.resolve_turn_sandbox_policy(%Schema{ + codex: %Codex{turn_sandbox_policy: explicit_policy}, + workspace: %Schema.Workspace{root: "/tmp/ignored"} + }) == explicit_policy + + assert Schema.resolve_turn_sandbox_policy(%Schema{ + codex: %Codex{turn_sandbox_policy: nil}, + workspace: %Schema.Workspace{root: ""} + }) == %{ + "type" => "workspaceWrite", + "writableRoots" => [Path.expand(Path.join(System.tmp_dir!(), "symphony_workspaces"))], + "readOnlyAccess" => %{"type" => "fullAccess"}, + "networkAccess" => false, + "excludeTmpdirEnvVar" => false, + "excludeSlashTmp" => false + } + + assert Schema.resolve_turn_sandbox_policy( + %Schema{ + codex: %Codex{turn_sandbox_policy: nil}, + workspace: %Schema.Workspace{root: "/tmp/ignored"} + }, + "/tmp/workspace" + ) == %{ + "type" => "workspaceWrite", + "writableRoots" => [Path.expand("/tmp/workspace")], + "readOnlyAccess" => %{"type" => "fullAccess"}, + "networkAccess" => false, + "excludeTmpdirEnvVar" => false, + "excludeSlashTmp" => false + } + end + test "workflow prompt is used when building base prompt" do workflow_prompt = "Workflow prompt body used as codex instruction."