diff --git a/README.md b/README.md index f67987f780..7a70a045a5 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,10 @@ GitHub Projects deployments can optionally auto-promote validated PRs from `Huma [elixir/README.md](elixir/README.md#auto-promote-from-human-review) for the opt-in config and rollout controls. +The Elixir implementation can optionally keep repository-local lessons from failed runs in +`.symphony/lessons.md`. When enabled in `WORKFLOW.md`, Symphony recalls recent entries into future +agent prompts, but it never auto-commits the file. + --- ## License diff --git a/SPEC.md b/SPEC.md index 02664fa954..9155782f98 100644 --- a/SPEC.md +++ b/SPEC.md @@ -612,6 +612,11 @@ not require recognizing or validating extension fields unless that extension is - `agent.auto_promote.quiet_seconds`: integer, default `600` - `agent.auto_promote.optout_label`: string, default `requires-human-review` - `agent.auto_promote.allowed_issue_labels`: list of strings, default `[]` +- `agent.lessons.enabled`: boolean, default `false` +- `agent.lessons.path`: repo-relative markdown path, default `.symphony/lessons.md` +- `agent.lessons.max_entries`: positive integer, default `50` +- `agent.lessons.recall_n`: non-negative integer, default `10` +- `agent.lessons.postmortem_max_tokens`: positive integer, default `1024` - `codex.command`: shell command string, default `codex app-server` - `codex.approval_policy`: Codex `AskForApproval` value, default implementation-defined - `codex.thread_sandbox`: Codex `SandboxMode` value, default implementation-defined @@ -694,6 +699,8 @@ Distinct terminal reasons are important because retry logic and logs differ. before accepting the successful exit. - If the workspace HEAD is unchanged or cannot be read, keep the issue in an active review state, write an operator-visible comment, and do not schedule the normal success continuation. + - If lessons are enabled and the workspace HEAD is unchanged, write a repository-local lesson + entry before ending the worker session. - If no claim-time workspace HEAD SHA was captured, fail open and continue the normal success path. - Schedule continuation retry (attempt `1`) after the worker exhausts or finishes its in-process turn loop. @@ -1171,10 +1178,14 @@ The `Agent Runner` wraps workspace + prompt + app-server client. Behavior: 1. Create/reuse workspace for issue. -2. Build prompt from workflow template. +2. Build prompt from workflow template, appending recent lessons from `agent.lessons.path` when + lessons are enabled. 3. Start app-server session. 4. Forward app-server events to orchestrator. -5. On any error, fail the worker attempt (the orchestrator will retry). +5. On a HEAD-unchanged failure with lessons enabled, ask the coding agent for a bounded + post-mortem and append a markdown entry under the configured lessons path. If the post-mortem + fails, append the entry with unavailable hypothesis and hint fields. +6. On any error, fail the worker attempt (the orchestrator will retry). Note: diff --git a/elixir/README.md b/elixir/README.md index 6af5dffa77..904a9f4500 100644 --- a/elixir/README.md +++ b/elixir/README.md @@ -463,6 +463,12 @@ hooks: agent: max_concurrent_agents: 10 max_turns: 20 + lessons: + enabled: false + path: ".symphony/lessons.md" + max_entries: 50 + recall_n: 10 + postmortem_max_tokens: 1024 codex: command: codex app-server --- @@ -536,6 +542,12 @@ agent: - `agent.auto_promote` controls the optional GitHub-only `Human Review` promotion rule. It is disabled by default; when enabled, use `allowed_issue_labels` for a conservative rollout and `optout_label` for per-issue human-review overrides. +- `agent.lessons.enabled` defaults to `false`. When enabled, a run whose workspace HEAD does not + advance writes a short markdown entry to `agent.lessons.path` inside the issue workspace, capped + to the newest `agent.lessons.max_entries` entries. Future dispatches append the newest + `agent.lessons.recall_n` entries to the Codex prompt under `## Lessons from prior runs`. + Symphony does not auto-commit this file; maintainers decide whether to keep, edit, prune, or + commit `.symphony/lessons.md`. - If the Markdown body is blank, Symphony uses a default prompt template that includes the issue identifier, title, and body. - Use `hooks.after_create` to bootstrap a fresh workspace. For a Git-backed repo, you can run diff --git a/elixir/WORKFLOW.github.example.md b/elixir/WORKFLOW.github.example.md index 5710a78bb8..e14a5b96be 100644 --- a/elixir/WORKFLOW.github.example.md +++ b/elixir/WORKFLOW.github.example.md @@ -118,6 +118,12 @@ agent: quiet_seconds: 600 optout_label: "requires-human-review" allowed_issue_labels: [] + lessons: + enabled: false + path: ".symphony/lessons.md" + max_entries: 50 + recall_n: 10 + postmortem_max_tokens: 1024 codex: command: codex --config shell_environment_policy.inherit=all --config 'model="gpt-5.5"' --config model_reasoning_effort=xhigh app-server diff --git a/elixir/WORKFLOW.md b/elixir/WORKFLOW.md index 912f65f85e..4edfcf495b 100644 --- a/elixir/WORKFLOW.md +++ b/elixir/WORKFLOW.md @@ -38,6 +38,12 @@ agent: quiet_seconds: 600 optout_label: "requires-human-review" allowed_issue_labels: [] + lessons: + enabled: false + path: ".symphony/lessons.md" + max_entries: 50 + recall_n: 10 + postmortem_max_tokens: 1024 codex: command: codex --config shell_environment_policy.inherit=all --config 'model="gpt-5.5"' --config model_reasoning_effort=xhigh app-server approval_policy: never diff --git a/elixir/lib/symphony_elixir/agent_runner.ex b/elixir/lib/symphony_elixir/agent_runner.ex index 3d227d8c86..611ab53f88 100644 --- a/elixir/lib/symphony_elixir/agent_runner.ex +++ b/elixir/lib/symphony_elixir/agent_runner.ex @@ -5,9 +5,11 @@ defmodule SymphonyElixir.AgentRunner do require Logger alias SymphonyElixir.Codex.AppServer - alias SymphonyElixir.{Config, Linear.Issue, ProjectConfig, PromptBuilder, Tracker, Workspace} + alias SymphonyElixir.{Config, Lessons, Linear.Issue, ProjectConfig, PromptBuilder, Tracker, Workspace} @type worker_host :: String.t() | nil + @postmortem_timeout_ms 30_000 + @transcript_tail_chars 8_000 @spec run(map(), pid() | nil, keyword()) :: :ok | no_return() def run(issue, codex_update_recipient \\ nil, opts \\ []) do @@ -46,7 +48,7 @@ defmodule SymphonyElixir.AgentRunner do claim_head_sha ) - run_codex_turns(workspace, issue, codex_update_recipient, opts, worker_host, runtime_config) + run_codex_turns(workspace, issue, codex_update_recipient, opts, worker_host, runtime_config, claim_head_sha) end after Workspace.run_after_run_hook(workspace, issue, worker_host) @@ -57,8 +59,9 @@ defmodule SymphonyElixir.AgentRunner do end end - defp codex_message_handler(recipient, issue) do + defp codex_message_handler(recipient, issue, transcript_ref) do fn message -> + capture_transcript(transcript_ref, message) send_codex_update(recipient, issue, message) end end @@ -131,57 +134,57 @@ defmodule SymphonyElixir.AgentRunner do Map.put(runtime_info, :claim_head_sha_error, reason) end - defp run_codex_turns(workspace, issue, codex_update_recipient, opts, worker_host, runtime_config) do + defp run_codex_turns(workspace, issue, codex_update_recipient, opts, worker_host, runtime_config, claim_head_sha) do 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) + transcript_ref = make_ref() session_opts = [worker_host: worker_host, runtime_config: runtime_config] - prompt_opts = Keyword.put(opts, :auto_branch, runtime_config.auto_branch) + + prompt_opts = + opts + |> Keyword.put(:auto_branch, runtime_config.auto_branch) + |> Keyword.put(:workspace, workspace) with {:ok, session} <- AppServer.start_session(workspace, session_opts) do try do - do_run_codex_turns( - session, - workspace, - issue, - codex_update_recipient, - prompt_opts, - issue_state_fetcher, - 1, - max_turns - ) + turn_context = %{ + codex_update_recipient: codex_update_recipient, + issue_state_fetcher: issue_state_fetcher, + max_turns: max_turns, + opts: prompt_opts, + transcript_ref: transcript_ref, + workspace: workspace + } + + result = + do_run_codex_turns(session, issue, turn_context, 1) + + record_head_unchanged_lesson(result, session, workspace, issue, worker_host, claim_head_sha, transcript_ref) + result after AppServer.stop_session(session) end end end - defp do_run_codex_turns(app_session, workspace, issue, codex_update_recipient, opts, issue_state_fetcher, turn_number, max_turns) do - prompt = build_turn_prompt(issue, opts, turn_number, max_turns) + defp do_run_codex_turns(app_session, issue, context, turn_number) do + prompt = build_turn_prompt(issue, context.opts, turn_number, context.max_turns) with {:ok, turn_session} <- AppServer.run_turn( app_session, prompt, issue, - on_message: codex_message_handler(codex_update_recipient, issue) + on_message: codex_message_handler(context.codex_update_recipient, issue, context.transcript_ref) ) do - Logger.info("Completed agent run for #{issue_context(issue)} session_id=#{turn_session[:session_id]} workspace=#{workspace} turn=#{turn_number}/#{max_turns}") - - case continue_with_issue?(issue, issue_state_fetcher) do - {:continue, refreshed_issue} when turn_number < max_turns -> - Logger.info("Continuing agent run for #{issue_context(refreshed_issue)} after normal turn completion turn=#{turn_number}/#{max_turns}") - - do_run_codex_turns( - app_session, - workspace, - refreshed_issue, - codex_update_recipient, - opts, - issue_state_fetcher, - turn_number + 1, - max_turns - ) + Logger.info("Completed agent run for #{issue_context(issue)} session_id=#{turn_session[:session_id]} workspace=#{context.workspace} turn=#{turn_number}/#{context.max_turns}") + + case continue_with_issue?(issue, context.issue_state_fetcher) do + {:continue, refreshed_issue} when turn_number < context.max_turns -> + Logger.info("Continuing agent run for #{issue_context(refreshed_issue)} after normal turn completion turn=#{turn_number}/#{context.max_turns}") + + do_run_codex_turns(app_session, refreshed_issue, context, turn_number + 1) {:continue, refreshed_issue} -> Logger.info("Reached agent.max_turns for #{issue_context(refreshed_issue)} with issue still active; returning control to orchestrator") @@ -197,6 +200,205 @@ defmodule SymphonyElixir.AgentRunner do end end + defp record_head_unchanged_lesson(:ok, app_session, workspace, issue, worker_host, {:ok, claim_head_sha}, transcript_ref) + when is_binary(claim_head_sha) do + lessons_config = Config.settings!().agent.lessons + + with true <- lessons_config.enabled, + {:ok, ^claim_head_sha} <- Workspace.workspace_head_sha(workspace, worker_host) do + write_head_unchanged_lesson(app_session, workspace, issue, transcript_ref, lessons_config) + else + false -> + :ok + + {:ok, _advanced_head_sha} -> + :ok + + {:error, reason} -> + Logger.warning("Skipped lesson capture because workspace HEAD could not be read for #{issue_context(issue)}: #{inspect(reason)}") + :ok + end + end + + defp record_head_unchanged_lesson(_result, _app_session, _workspace, _issue, _worker_host, _claim_head_sha, _transcript_ref) do + :ok + end + + defp write_head_unchanged_lesson(app_session, workspace, issue, transcript_ref, lessons_config) do + transcript_tail = transcript_tail(transcript_ref) + postmortem_result = request_postmortem(app_session, issue, transcript_tail, lessons_config.postmortem_max_tokens) + path = Path.expand(lessons_config.path, workspace) + + case Lessons.append(path, lesson_entry(issue, transcript_tail, postmortem_result), max_entries: lessons_config.max_entries) do + :ok -> + :ok + + {:error, reason} -> + Logger.warning("Failed to write lesson for #{issue_context(issue)} path=#{path}: #{inspect(reason)}") + :ok + end + end + + defp request_postmortem(app_session, issue, transcript_tail, max_tokens) do + postmortem_ref = make_ref() + + case AppServer.run_turn( + app_session, + postmortem_prompt(transcript_tail, max_tokens), + issue, + on_message: fn message -> capture_transcript(postmortem_ref, message) end, + turn_timeout_ms: @postmortem_timeout_ms + ) do + {:ok, _turn_session} -> + postmortem_text(postmortem_ref) + + {:error, reason} -> + {:error, reason} + end + end + + defp postmortem_prompt(transcript_tail, max_tokens) do + """ + Post-mortem request: + + Given this transcript from a Symphony agent run whose workspace HEAD did not advance, answer in one paragraph under #{max_tokens} tokens: what went wrong, and what should the next agent know before retrying similar work? + + Do not run commands or edit files. Answer only with the post-mortem. + + Transcript tail: + + ```text + #{transcript_tail} + ``` + """ + end + + defp lesson_entry(issue, transcript_tail, {:ok, postmortem}) do + issue + |> base_lesson_entry(transcript_tail) + |> Map.put(:hypothesis, postmortem) + |> Map.put(:hint, postmortem) + end + + defp lesson_entry(issue, transcript_tail, {:error, _reason}) do + issue + |> base_lesson_entry(transcript_tail) + |> Map.put(:hypothesis, "") + |> Map.put(:hint, "") + end + + defp base_lesson_entry(issue, transcript_tail) do + %{ + failure_kind: "workspace HEAD did not advance", + symptom: transcript_tail, + title: issue_title(issue) + } + |> put_lesson_issue_ref(issue) + end + + defp put_lesson_issue_ref(entry, issue) do + case issue_number(issue) do + nil -> Map.put(entry, :issue_ref, "issue #{issue_identifier(issue)}") + number -> Map.put(entry, :issue_number, number) + end + end + + defp issue_number(issue) do + case Regex.run(~r/#(\d+)\z/, issue_identifier(issue), capture: :all_but_first) do + [number] -> String.to_integer(number) + _no_match -> nil + end + end + + defp issue_title(%{title: title}) when is_binary(title) and title != "", do: title + defp issue_title(_issue), do: "Untitled" + + defp issue_identifier(%{identifier: identifier}) when is_binary(identifier) and identifier != "", do: identifier + defp issue_identifier(%{id: id}) when is_binary(id) and id != "", do: id + defp issue_identifier(_issue), do: "" + + defp capture_transcript(transcript_ref, message) do + Process.put(transcript_ref, [message | Process.get(transcript_ref, [])]) + :ok + end + + defp transcript_tail(transcript_ref) do + transcript_ref + |> transcript_messages() + |> Enum.map_join("\n", &transcript_line/1) + |> String.trim() + |> case do + "" -> "" + transcript -> take_tail(transcript, @transcript_tail_chars) + end + end + + defp transcript_messages(transcript_ref) do + transcript_ref + |> Process.get([]) + |> Enum.reverse() + end + + defp transcript_line(message) when is_map(message) do + event = Map.get(message, :event, "message") + payload = Map.get(message, :raw) || Map.get(message, :payload) || Map.get(message, :details) || Map.delete(message, :timestamp) + + "#{event}: #{format_transcript_payload(payload)}" + end + + defp transcript_line(message), do: format_transcript_payload(message) + + defp format_transcript_payload(payload) when is_binary(payload), do: payload + + defp format_transcript_payload(payload) do + inspect(payload, limit: 20, printable_limit: 1_000) + end + + defp take_tail(text, max_chars) do + length = String.length(text) + + if length > max_chars do + String.slice(text, length - max_chars, max_chars) + else + text + end + end + + defp postmortem_text(transcript_ref) do + postmortem = + transcript_ref + |> transcript_messages() + |> Enum.flat_map(&extract_texts/1) + |> Enum.map(&String.trim/1) + |> Enum.reject(&(&1 == "")) + |> Enum.join("\n") + |> String.trim() + + case postmortem do + "" -> {:error, :empty_postmortem} + text -> {:ok, take_tail(text, @transcript_tail_chars)} + end + end + + defp extract_texts(%_struct{}), do: [] + + defp extract_texts(value) when is_map(value) do + Enum.flat_map(value, fn {key, nested} -> + if text_key?(key) and is_binary(nested) do + [nested] + else + extract_texts(nested) + end + end) + end + + defp extract_texts(value) when is_list(value), do: Enum.flat_map(value, &extract_texts/1) + defp extract_texts(_value), do: [] + + defp text_key?(key) when key in [:content, :message, :output, :text], do: true + defp text_key?(key) when key in ["content", "message", "output", "text"], do: true + defp text_key?(_key), do: false + defp build_turn_prompt(issue, opts, 1, _max_turns), do: PromptBuilder.build_prompt(issue, opts) defp build_turn_prompt(_issue, _opts, turn_number, max_turns) do diff --git a/elixir/lib/symphony_elixir/codex/app_server.ex b/elixir/lib/symphony_elixir/codex/app_server.ex index 493cbba547..6394da2077 100644 --- a/elixir/lib/symphony_elixir/codex/app_server.ex +++ b/elixir/lib/symphony_elixir/codex/app_server.ex @@ -89,6 +89,7 @@ defmodule SymphonyElixir.Codex.AppServer do opts \\ [] ) do on_message = Keyword.get(opts, :on_message, &default_on_message/1) + turn_timeout_ms = Keyword.get(opts, :turn_timeout_ms, turn_timeout_ms) tool_executor = Keyword.get(opts, :tool_executor, fn tool, arguments -> diff --git a/elixir/lib/symphony_elixir/config/schema.ex b/elixir/lib/symphony_elixir/config/schema.ex index 8e589da8df..875d6817b6 100644 --- a/elixir/lib/symphony_elixir/config/schema.ex +++ b/elixir/lib/symphony_elixir/config/schema.ex @@ -257,6 +257,56 @@ defmodule SymphonyElixir.Config.Schema do alias SymphonyElixir.Config.Schema.AutoPromote @primary_key false + + defmodule Lessons do + @moduledoc false + use Ecto.Schema + import Ecto.Changeset + + @primary_key false + embedded_schema do + field(:enabled, :boolean, default: false) + field(:path, :string, default: ".symphony/lessons.md") + field(:max_entries, :integer, default: 50) + field(:recall_n, :integer, default: 10) + field(:postmortem_max_tokens, :integer, default: 1024) + end + + @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() + def changeset(schema, attrs) do + schema + |> cast(attrs, [:enabled, :path, :max_entries, :recall_n, :postmortem_max_tokens], empty_values: []) + |> validate_required([:path]) + |> validate_change(:path, &validate_path/2) + |> validate_number(:max_entries, greater_than: 0) + |> validate_number(:recall_n, greater_than_or_equal_to: 0) + |> validate_number(:postmortem_max_tokens, greater_than: 0) + end + + defp validate_path(field, path) do + trimmed_path = path |> to_string() |> String.trim() + + if Path.type(trimmed_path) != :relative or home_relative_path?(trimmed_path) or + windows_absolute_path?(trimmed_path) or escapes_workspace?(trimmed_path) do + [{field, "must be a relative path inside the workspace"}] + else + [] + end + end + + defp home_relative_path?(path), do: String.starts_with?(path, "~") + + defp windows_absolute_path?(path) do + String.starts_with?(path, "\\") or Regex.match?(~r/^[A-Za-z]:[\\\/]/, path) + end + + defp escapes_workspace?(path) do + path + |> Path.split() + |> Enum.member?("..") + end + end + embedded_schema do field(:max_concurrent_agents, :integer, default: 10) field(:max_turns, :integer, default: 20) @@ -264,6 +314,7 @@ defmodule SymphonyElixir.Config.Schema do field(:max_concurrent_agents_by_state, :map, default: %{}) field(:dispatch_priority_by_state, {:array, :string}, default: []) embeds_one(:auto_promote, AutoPromote, on_replace: :update, defaults_to_struct: true) + embeds_one(:lessons, Lessons, on_replace: :update, defaults_to_struct: true) end @spec changeset(%__MODULE__{}, map()) :: Ecto.Changeset.t() @@ -288,6 +339,7 @@ defmodule SymphonyElixir.Config.Schema do |> update_change(:dispatch_priority_by_state, &Schema.normalize_state_list/1) |> Schema.validate_state_limits(:max_concurrent_agents_by_state) |> Schema.validate_state_list(:dispatch_priority_by_state) + |> cast_embed(:lessons, with: &Lessons.changeset/2) end end diff --git a/elixir/lib/symphony_elixir/lessons.ex b/elixir/lib/symphony_elixir/lessons.ex new file mode 100644 index 0000000000..3fd0954bf7 --- /dev/null +++ b/elixir/lib/symphony_elixir/lessons.ex @@ -0,0 +1,98 @@ +defmodule SymphonyElixir.Lessons do + @moduledoc """ + Stores and recalls repository-local lessons from failed agent runs. + """ + + @default_max_entries 50 + + @type entry :: %{ + optional(:date) => Date.t(), + optional(:failure_kind) => String.t(), + optional(:hint) => String.t(), + optional(:hypothesis) => String.t(), + optional(:issue_number) => integer() | String.t(), + optional(:issue_ref) => String.t(), + optional(:symptom) => String.t(), + optional(:title) => String.t() + } + + @spec append(Path.t(), entry(), keyword()) :: :ok | {:error, term()} + def append(path, entry, opts \\ []) when is_binary(path) and is_map(entry) do + max_entries = opts |> Keyword.get(:max_entries, @default_max_entries) |> max(1) + rendered_entry = render_entry(entry, Keyword.get(opts, :date, Date.utc_today())) + + case read_all(path) do + {:ok, entries} -> + entries = entries |> Kernel.++([rendered_entry]) |> Enum.take(-max_entries) + + write_entries(path, entries) + + {:error, reason} -> + {:error, reason} + end + end + + @spec recent(Path.t(), non_neg_integer()) :: {:ok, [String.t()]} | {:error, term()} + def recent(path, count) when is_binary(path) and is_integer(count) do + case read_all(path) do + {:ok, entries} -> {:ok, Enum.take(entries, -max(count, 0))} + {:error, reason} -> {:error, reason} + end + end + + @spec read_all(Path.t()) :: {:ok, [String.t()]} | {:error, term()} + def read_all(path) when is_binary(path) do + case File.read(path) do + {:ok, content} -> {:ok, parse_entries(content)} + {:error, :enoent} -> {:ok, []} + {:error, reason} -> {:error, reason} + end + end + + defp render_entry(entry, date) do + [ + "## #{Date.to_iso8601(date)} - #{issue_ref(entry)} - \"#{heading_title(entry)}\"", + "- **Failure kind:** #{field(entry, :failure_kind, "")}", + "- **Symptom:** #{field(entry, :symptom, "")}", + "- **Hypothesis (Symphony):** #{field(entry, :hypothesis, "")}", + "- **Hint for next time:** #{field(entry, :hint, "")}" + ] + |> Enum.join("\n") + end + + defp issue_ref(%{issue_number: number}) when is_integer(number), do: "issue ##{number}" + defp issue_ref(%{issue_number: number}) when is_binary(number), do: "issue ##{String.trim_leading(number, "#")}" + defp issue_ref(%{issue_ref: issue_ref}) when is_binary(issue_ref), do: issue_ref + defp issue_ref(_entry), do: "issue " + + defp heading_title(entry) do + entry + |> field(:title, "Untitled") + |> String.replace("\"", "\\\"") + end + + defp field(entry, key, fallback) do + entry + |> Map.get(key, fallback) + |> to_string() + |> String.replace(~r/\s+/, " ") + |> String.trim() + |> case do + "" -> fallback + value -> value + end + end + + defp parse_entries(content) do + content + |> String.split(~r/(?=^## )/m, trim: true) + |> Enum.map(&String.trim/1) + |> Enum.filter(&String.starts_with?(&1, "## ")) + end + + defp write_entries(path, entries) do + with :ok <- path |> Path.dirname() |> File.mkdir_p() do + File.write(path, Enum.join(entries, "\n\n") <> "\n") + end + end +end diff --git a/elixir/lib/symphony_elixir/prompt_builder.ex b/elixir/lib/symphony_elixir/prompt_builder.ex index 03f1c74075..d612268115 100644 --- a/elixir/lib/symphony_elixir/prompt_builder.ex +++ b/elixir/lib/symphony_elixir/prompt_builder.ex @@ -3,7 +3,7 @@ defmodule SymphonyElixir.PromptBuilder do Builds agent prompts from Linear issue data. """ - alias SymphonyElixir.{Config, Workflow} + alias SymphonyElixir.{Config, Lessons, Workflow} alias SymphonyElixir.Config.Schema @render_opts [strict_variables: true, strict_filters: true] @@ -23,17 +23,20 @@ defmodule SymphonyElixir.PromptBuilder do template = prompt_template |> default_prompt() |> parse_template!() settings = settings!(config) - template - |> Solid.render!( - %{ - "attempt" => Keyword.get(opts, :attempt), - "issue" => issue |> Map.from_struct() |> to_solid_map(), - "tracker" => tracker_assigns(settings), - "workspace" => workspace_assigns(settings, opts) - }, - @render_opts - ) - |> IO.iodata_to_binary() + rendered = + template + |> Solid.render!( + %{ + "attempt" => Keyword.get(opts, :attempt), + "issue" => issue |> Map.from_struct() |> to_solid_map(), + "tracker" => tracker_assigns(settings), + "workspace" => workspace_assigns(settings, opts) + }, + @render_opts + ) + |> IO.iodata_to_binary() + + append_lessons_block(rendered, settings, opts) end defp settings!(config) when is_map(config) do @@ -60,6 +63,24 @@ defmodule SymphonyElixir.PromptBuilder do } end + defp append_lessons_block(rendered, settings, opts) do + lessons = settings.agent.lessons + workspace = Keyword.get(opts, :workspace) + + if lessons.enabled and is_binary(workspace) do + lessons.path + |> Path.expand(workspace) + |> Lessons.recent(lessons.recall_n) + |> case do + {:ok, []} -> rendered + {:ok, entries} -> rendered <> "\n\n## Lessons from prior runs\n\n" <> Enum.join(entries, "\n\n") + {:error, _reason} -> rendered + end + else + rendered + end + end + defp raise_workflow_unavailable(reason) do raise RuntimeError, "workflow_unavailable: #{inspect(reason)}" end diff --git a/elixir/test/support/test_support.exs b/elixir/test/support/test_support.exs index 66fe8001a2..a3d1edfc00 100644 --- a/elixir/test/support/test_support.exs +++ b/elixir/test/support/test_support.exs @@ -129,6 +129,7 @@ defmodule SymphonyElixir.TestSupport do auto_promote_quiet_seconds: 600, auto_promote_optout_label: "requires-human-review", auto_promote_allowed_issue_labels: [], + agent_lessons: nil, codex_command: "codex app-server", codex_approval_policy: %{reject: %{sandbox_approval: true, rules: true, mcp_elicitations: true}}, codex_thread_sandbox: "workspace-write", @@ -174,6 +175,7 @@ defmodule SymphonyElixir.TestSupport do auto_promote_quiet_seconds = Keyword.get(config, :auto_promote_quiet_seconds) auto_promote_optout_label = Keyword.get(config, :auto_promote_optout_label) auto_promote_allowed_issue_labels = Keyword.get(config, :auto_promote_allowed_issue_labels) + agent_lessons = Keyword.get(config, :agent_lessons) codex_command = Keyword.get(config, :codex_command) codex_approval_policy = Keyword.get(config, :codex_approval_policy) codex_thread_sandbox = Keyword.get(config, :codex_thread_sandbox) @@ -223,6 +225,7 @@ defmodule SymphonyElixir.TestSupport do " quiet_seconds: #{yaml_value(auto_promote_quiet_seconds)}", " optout_label: #{yaml_value(auto_promote_optout_label)}", " allowed_issue_labels: #{yaml_value(auto_promote_allowed_issue_labels)}", + lessons_yaml(agent_lessons), "codex:", " command: #{yaml_value(codex_command)}", " approval_policy: #{yaml_value(codex_approval_policy)}", @@ -294,6 +297,20 @@ defmodule SymphonyElixir.TestSupport do |> Enum.join("\n") end + defp lessons_yaml(nil), do: nil + + defp lessons_yaml(lessons) when is_map(lessons) do + [ + " lessons:", + " enabled: #{yaml_value(Map.get(lessons, :enabled, false))}", + " path: #{yaml_value(Map.get(lessons, :path, ".symphony/lessons.md"))}", + " max_entries: #{yaml_value(Map.get(lessons, :max_entries, 50))}", + " recall_n: #{yaml_value(Map.get(lessons, :recall_n, 10))}", + " postmortem_max_tokens: #{yaml_value(Map.get(lessons, :postmortem_max_tokens, 1024))}" + ] + |> Enum.join("\n") + end + defp observability_yaml(enabled, refresh_ms, render_interval_ms) do [ "observability:", diff --git a/elixir/test/symphony_elixir/agent_runner_lessons_test.exs b/elixir/test/symphony_elixir/agent_runner_lessons_test.exs new file mode 100644 index 0000000000..6e7ec4db9a --- /dev/null +++ b/elixir/test/symphony_elixir/agent_runner_lessons_test.exs @@ -0,0 +1,203 @@ +defmodule SymphonyElixir.AgentRunnerLessonsTest do + use SymphonyElixir.TestSupport + + test "HEAD-did-not-advance run writes a lesson and the next dispatch prompt recalls it" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-agent-runner-lessons-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + codex_binary = Path.join(test_root, "fake-codex") + trace_file = Path.join(test_root, "codex.trace") + + File.mkdir_p!(test_root) + write_codex_script!(codex_binary, postmortem: :ok) + System.put_env("SYMP_TEST_CODEX_TRACE", trace_file) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + hook_after_create: git_repo_hook(), + codex_command: "#{codex_binary} app-server", + agent_lessons: %{ + enabled: true, + max_entries: 5, + recall_n: 1, + postmortem_max_tokens: 512 + } + ) + + issue = lessons_issue() + state_fetcher = fn [_issue_id] -> {:ok, [%{issue | state: "Done"}]} end + + assert :ok = AgentRunner.run(issue, nil, issue_state_fetcher: state_fetcher) + + lessons_path = Path.join([workspace_root, "digitaldrywood_symphony_89", ".symphony", "lessons.md"]) + lesson = File.read!(lessons_path) + + assert lesson =~ "issue #89" + assert lesson =~ "- **Failure kind:** workspace HEAD did not advance" + assert lesson =~ "mix ecto.gen.migration failed because the alias was missing" + assert lesson =~ "next time alias Ecto.Migration before touching migrations" + + assert :ok = AgentRunner.run(issue, nil, issue_state_fetcher: state_fetcher) + + main_prompts = + trace_file + |> turn_prompts() + |> Enum.filter(&String.contains?(&1, "You are an agent for this repository.")) + + assert length(main_prompts) == 2 + refute Enum.at(main_prompts, 0) =~ "## Lessons from prior runs" + assert Enum.at(main_prompts, 1) =~ "## Lessons from prior runs" + assert Enum.at(main_prompts, 1) =~ "next time alias Ecto.Migration before touching migrations" + after + System.delete_env("SYMP_TEST_CODEX_TRACE") + File.rm_rf(test_root) + end + end + + test "postmortem failure still writes a fallback lesson" do + test_root = + Path.join( + System.tmp_dir!(), + "symphony-elixir-agent-runner-lesson-fallback-#{System.unique_integer([:positive])}" + ) + + try do + workspace_root = Path.join(test_root, "workspaces") + codex_binary = Path.join(test_root, "fake-codex") + trace_file = Path.join(test_root, "codex.trace") + + File.mkdir_p!(test_root) + write_codex_script!(codex_binary, postmortem: :failed) + System.put_env("SYMP_TEST_CODEX_TRACE", trace_file) + + write_workflow_file!(Workflow.workflow_file_path(), + workspace_root: workspace_root, + hook_after_create: git_repo_hook(), + codex_command: "#{codex_binary} app-server", + agent_lessons: %{enabled: true} + ) + + issue = lessons_issue() + state_fetcher = fn [_issue_id] -> {:ok, [%{issue | state: "Done"}]} end + + assert :ok = AgentRunner.run(issue, nil, issue_state_fetcher: state_fetcher) + + lessons_path = Path.join([workspace_root, "digitaldrywood_symphony_89", ".symphony", "lessons.md"]) + lesson = File.read!(lessons_path) + + assert lesson =~ "mix ecto.gen.migration failed because the alias was missing" + assert lesson =~ "- **Hypothesis (Symphony):** " + assert lesson =~ "- **Hint for next time:** " + after + System.delete_env("SYMP_TEST_CODEX_TRACE") + File.rm_rf(test_root) + end + end + + test "prompt builder ignores unreadable lessons files" do + workspace = Path.join(System.tmp_dir!(), "symphony-elixir-unreadable-lessons-#{System.unique_integer([:positive])}") + lessons_path = Path.join([workspace, ".symphony", "lessons.md"]) + + try do + File.mkdir_p!(lessons_path) + + write_workflow_file!(Workflow.workflow_file_path(), + agent_lessons: %{ + enabled: true, + path: ".symphony/lessons.md" + } + ) + + prompt = PromptBuilder.build_prompt(lessons_issue(), workspace: workspace) + + refute prompt =~ "## Lessons from prior runs" + assert prompt =~ "You are an agent for this repository." + after + File.rm_rf(workspace) + end + end + + defp write_codex_script!(path, opts) do + postmortem = Keyword.fetch!(opts, :postmortem) + + File.write!(path, """ + #!/bin/sh + trace_file="${SYMP_TEST_CODEX_TRACE:-/tmp/symphony-lessons-codex.trace}" + count=0 + + while IFS= read -r line; do + count=$((count + 1)) + printf 'JSON:%s\\n' "$line" >> "$trace_file" + + case "$count" in + 1) + printf '%s\\n' '{"id":1,"result":{}}' + ;; + 3) + printf '%s\\n' '{"id":2,"result":{"thread":{"id":"thread-lessons"}}}' + ;; + 4) + printf '%s\\n' '{"id":3,"result":{"turn":{"id":"turn-main"}}}' + printf '%s\\n' '{"method":"agent/message","params":{"text":"mix ecto.gen.migration failed because the alias was missing"}}' + printf '%s\\n' '{"method":"turn/completed"}' + ;; + 5) + printf '%s\\n' '{"id":3,"result":{"turn":{"id":"turn-postmortem"}}}' + #{postmortem_output(postmortem)} + ;; + esac + done + """) + + File.chmod!(path, 0o755) + end + + defp postmortem_output(:ok) do + """ + printf '%s\\n' '{"method":"agent/message","params":{"text":"The run stopped after the migration generator failed; next time alias Ecto.Migration before touching migrations."}}' + printf '%s\\n' '{"method":"turn/completed"}' + """ + end + + defp postmortem_output(:failed) do + """ + printf '%s\\n' '{"method":"turn/failed","params":{"reason":"postmortem failed"}}' + """ + end + + defp turn_prompts(trace_file) do + trace_file + |> File.read!() + |> String.split("\n", trim: true) + |> Enum.filter(&String.starts_with?(&1, "JSON:")) + |> Enum.map(&String.trim_leading(&1, "JSON:")) + |> Enum.map(&Jason.decode!/1) + |> Enum.filter(&(&1["method"] == "turn/start")) + |> Enum.map(fn payload -> + payload + |> get_in(["params", "input"]) + |> Enum.map_join("\n", &Map.get(&1, "text", "")) + end) + end + + defp git_repo_hook do + "git init -b main && git config user.name Test && git config user.email test@example.com && printf initial > README.md && git add README.md && git commit -m initial" + end + + defp lessons_issue do + %Issue{ + id: "issue-lessons", + identifier: "digitaldrywood/symphony#89", + title: "Record lessons", + description: "Record failed-run lessons", + state: "In Progress", + url: "https://github.com/digitaldrywood/symphony/issues/89", + labels: ["enhancement"] + } + end +end diff --git a/elixir/test/symphony_elixir/lessons_test.exs b/elixir/test/symphony_elixir/lessons_test.exs new file mode 100644 index 0000000000..b3a452bc3f --- /dev/null +++ b/elixir/test/symphony_elixir/lessons_test.exs @@ -0,0 +1,160 @@ +defmodule SymphonyElixir.LessonsTest do + use ExUnit.Case, async: true + + alias SymphonyElixir.Lessons + + test "read_all and recent treat a missing lessons file as empty" do + path = Path.join(tmp_dir(), ".symphony/lessons.md") + + assert Lessons.read_all(path) == {:ok, []} + assert Lessons.recent(path, 3) == {:ok, []} + end + + test "append writes a markdown entry and read_all ignores non-entry preamble" do + path = Path.join(tmp_dir(), ".symphony/lessons.md") + File.mkdir_p!(Path.dirname(path)) + File.write!(path, "preamble\n\n") + + assert :ok = + Lessons.append( + path, + %{ + failure_kind: "workspace HEAD did not advance", + symptom: "Codex produced no diff", + hypothesis: "The generator failed before creating a file.", + hint: "Check aliases before running the generator.", + issue_number: 142, + title: "Migrate users table to UUIDs" + }, + date: ~D[2026-05-22] + ) + + assert {:ok, [entry]} = Lessons.read_all(path) + assert entry =~ "## 2026-05-22 - issue #142 - \"Migrate users table to UUIDs\"" + assert entry =~ "- **Failure kind:** workspace HEAD did not advance" + assert entry =~ "- **Symptom:** Codex produced no diff" + assert entry =~ "- **Hypothesis (Symphony):** The generator failed before creating a file." + assert entry =~ "- **Hint for next time:** Check aliases before running the generator." + end + + test "append caps entries by dropping the oldest records" do + path = Path.join(tmp_dir(), ".symphony/lessons.md") + + for index <- 1..6 do + assert :ok = + Lessons.append( + path, + %{ + failure_kind: "kind #{index}", + symptom: "symptom #{index}", + hypothesis: "hypothesis #{index}", + hint: "hint #{index}", + issue_number: index, + title: "Issue #{index}" + }, + date: ~D[2026-05-22], + max_entries: 5 + ) + end + + assert {:ok, entries} = Lessons.read_all(path) + assert length(entries) == 5 + refute Enum.any?(entries, &String.contains?(&1, "issue #1")) + assert List.first(entries) =~ "issue #2" + assert List.last(entries) =~ "issue #6" + end + + test "recent returns the newest entries in file order" do + path = Path.join(tmp_dir(), ".symphony/lessons.md") + + for index <- 1..3 do + assert :ok = + Lessons.append( + path, + %{ + failure_kind: "kind #{index}", + symptom: "symptom #{index}", + hypothesis: "hypothesis #{index}", + hint: "hint #{index}", + issue_number: "#{index}", + title: "Issue #{index}" + }, + date: ~D[2026-05-22] + ) + end + + assert {:ok, entries} = Lessons.recent(path, 2) + assert Enum.map(entries, &Regex.run(~r/issue #(\d+)/, &1, capture: :all_but_first)) == [["2"], ["3"]] + assert Lessons.recent(path, 0) == {:ok, []} + end + + test "entry rendering falls back for missing issue metadata and blank fields" do + path = Path.join(tmp_dir(), ".symphony/lessons.md") + + assert :ok = + Lessons.append( + path, + %{ + failure_kind: "", + symptom: "", + hypothesis: "", + hint: "", + title: "Needs \"quotes\" escaped" + }, + date: ~D[2026-05-22] + ) + + assert {:ok, [entry]} = Lessons.read_all(path) + assert entry =~ "issue " + assert entry =~ "Needs \\\"quotes\\\" escaped" + assert entry =~ "- **Failure kind:** " + assert entry =~ "- **Symptom:** " + assert entry =~ "- **Hypothesis (Symphony):** " + assert entry =~ "- **Hint for next time:** " + end + + test "explicit issue refs and read errors are surfaced" do + root = tmp_dir() + path = Path.join(root, ".symphony/lessons.md") + + assert :ok = + Lessons.append( + path, + %{ + failure_kind: "workspace HEAD did not advance", + symptom: "No commit", + hypothesis: "No files changed", + hint: "Inspect the diff before finishing", + issue_ref: "issue MT-9", + title: "Linear issue" + }, + date: ~D[2026-05-22] + ) + + assert {:ok, [entry]} = Lessons.read_all(path) + assert entry =~ "issue MT-9" + + assert {:error, _reason} = Lessons.read_all(root) + assert {:error, _reason} = Lessons.recent(root, 1) + assert {:error, _reason} = Lessons.append(root, %{title: "cannot append"}) + end + + test "append returns mkdir errors without raising" do + root = tmp_dir() + protected_dir = Path.join(root, "protected") + path = Path.join([protected_dir, "nested", "lessons.md"]) + File.mkdir_p!(protected_dir) + File.chmod!(protected_dir, 0o500) + + on_exit(fn -> + File.chmod(protected_dir, 0o700) + File.rm_rf(root) + end) + + assert {:error, _reason} = Lessons.append(path, %{title: "cannot write"}) + end + + defp tmp_dir do + Path.join(System.tmp_dir!(), "symphony-elixir-lessons-#{System.unique_integer([:positive])}") + end +end diff --git a/elixir/test/symphony_elixir/workspace_and_config_test.exs b/elixir/test/symphony_elixir/workspace_and_config_test.exs index 8653863e8d..4c12e1f822 100644 --- a/elixir/test/symphony_elixir/workspace_and_config_test.exs +++ b/elixir/test/symphony_elixir/workspace_and_config_test.exs @@ -1218,6 +1218,11 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert config.workspace.auto_branch assert config.worker.max_concurrent_agents_per_host == nil assert config.agent.max_concurrent_agents == 10 + refute config.agent.lessons.enabled + assert config.agent.lessons.path == ".symphony/lessons.md" + assert config.agent.lessons.max_entries == 50 + assert config.agent.lessons.recall_n == 10 + assert config.agent.lessons.postmortem_max_tokens == 1024 assert config.codex.command == "codex app-server" assert config.codex.approval_policy == %{ @@ -1299,6 +1304,62 @@ defmodule SymphonyElixir.WorkspaceAndConfigTest do assert {:error, {:invalid_workflow_config, message}} = Config.validate!() assert message =~ "agent.max_concurrent_agents" + write_workflow_file!(Workflow.workflow_file_path(), + agent_lessons: %{ + enabled: true, + path: ".symphony/custom-lessons.md", + max_entries: 5, + recall_n: 2, + postmortem_max_tokens: 256 + } + ) + + config = Config.settings!() + assert config.agent.lessons.enabled + assert config.agent.lessons.path == ".symphony/custom-lessons.md" + assert config.agent.lessons.max_entries == 5 + assert config.agent.lessons.recall_n == 2 + assert config.agent.lessons.postmortem_max_tokens == 256 + + write_workflow_file!(Workflow.workflow_file_path(), + agent_lessons: %{ + enabled: true, + path: "", + max_entries: 0, + recall_n: -1, + postmortem_max_tokens: 0 + } + ) + + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "agent.lessons.path" + assert message =~ "agent.lessons.max_entries" + assert message =~ "agent.lessons.recall_n" + assert message =~ "agent.lessons.postmortem_max_tokens" + + for invalid_path <- [ + "/tmp/lessons.md", + "~/.symphony/lessons.md", + "../lessons.md", + ".symphony/../lessons.md", + "C:/lessons.md", + "\\\\server\\share\\lessons.md" + ] do + write_workflow_file!(Workflow.workflow_file_path(), + agent_lessons: %{ + enabled: true, + path: invalid_path, + max_entries: 5, + recall_n: 2, + postmortem_max_tokens: 256 + } + ) + + assert {:error, {:invalid_workflow_config, message}} = Config.validate!() + assert message =~ "agent.lessons.path" + assert message =~ "must be a relative path inside the workspace" + end + write_workflow_file!(Workflow.workflow_file_path(), worker_max_concurrent_agents_per_host: 0) assert {:error, {:invalid_workflow_config, message}} = Config.validate!() assert message =~ "worker.max_concurrent_agents_per_host"