From d669a2d55c0b808e2005624dfef05e583b563487 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 15:15:39 -0400 Subject: [PATCH] fix(agent): correct background-agent notification delivery Three correctness fixes in the Claude 5 background-agent path, plus cleanups from a reuse/quality/efficiency review pass. Fixes: - Background-agent output was run through skill expansion. A child that wrote a bare path ("cleaned up /tmp") failed the whole parent turn with `Unknown skill: /tmp`, and a child whose output happened to name a real skill had its report replaced by that skill's template. Synthesized harness turns now skip expansion; only text the user typed can invoke a skill. - `begin_shutdown` suppressed the pending notification before deciding whether a shutdown would happen. Stopping an agent that had just finished rejected the stop *and* discarded the result the parent was owed. Suppression now happens only once shutdown is committed. - `spawn_inner` registered the notification after publishing the agent in `state.agents`, so a concurrent `shutdown_all` in that window left a pending entry the monitor never completes, and the parent's drain loop would never see the queue as drained. Registration now precedes publication. - `TaskOutput.timeout` was declared `number` but parsed with `as_u64`, so a schema-valid `30000.0` failed at runtime. - Update the fabro-server alias test for the `sonnet` alias moving to Claude Sonnet 5. Cleanups: - The supervisor renders the notification turn; `Session` no longer knows the envelope format. - Replace six near-identical prompt snapshots with a property test over all eight conditional combinations, keeping the default and all-conditionals snapshots for wording. - Collapse `TodoRuntime`'s two mutexes into one. - Read the prompt vocabulary from the registry instead of hardcoding it. - Drop internal vocabulary from the `SendMessage` tool description. Co-Authored-By: Claude Opus 5 (1M context) --- lib/apps/fabro-server/src/server/tests.rs | 2 +- .../fabro-agent/src/profiles/claude5.rs | 2 +- .../fabro-agent/src/profiles/claude5_tools.rs | 6 +- .../fabro-agent/src/profiles/mod.rs | 61 ++++++------- ...sts__claude5_question_prompt_snapshot.snap | 77 ---------------- ...ubagents_and_question_prompt_snapshot.snap | 87 ------------------- ...ts__claude5_subagents_prompt_snapshot.snap | 81 ----------------- ...b_search_and_question_prompt_snapshot.snap | 79 ----------------- ..._search_and_subagents_prompt_snapshot.snap | 83 ------------------ ...s__claude5_web_search_prompt_snapshot.snap | 73 ---------------- lib/components/fabro-agent/src/session.rs | 85 +++++++++++++++--- lib/components/fabro-agent/src/subagent.rs | 83 +++++++++++++++--- .../fabro-agent/src/todo_runtime.rs | 36 ++++---- 13 files changed, 202 insertions(+), 553 deletions(-) delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap delete mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 71b4dfebf..958b2e876 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -6583,7 +6583,7 @@ async fn test_model_explicit_provider_alias_returns_canonical_model_id_when_unav let response = app.oneshot(req).await.unwrap(); let body = response_json!(response, StatusCode::OK).await; - assert_eq!(body["model_id"], "claude-sonnet-4-6"); + assert_eq!(body["model_id"], "claude-sonnet-5"); assert_eq!(body["provider"], "anthropic"); assert_eq!(body["status"], "skip"); } diff --git a/lib/components/fabro-agent/src/profiles/claude5.rs b/lib/components/fabro-agent/src/profiles/claude5.rs index 6b7826584..d6d2faa6c 100644 --- a/lib/components/fabro-agent/src/profiles/claude5.rs +++ b/lib/components/fabro-agent/src/profiles/claude5.rs @@ -134,7 +134,7 @@ impl AgentProfile for Claude5Profile { skills: &[Skill], ) -> String { let template = EmbeddedPrompt::new("claude5.md.j2", CORE_PROMPT) - .with_vocabulary(ToolVocabulary::Claude5) + .with_vocabulary(self.base.registry.vocabulary()) .with_bool( "has_agent", self.base diff --git a/lib/components/fabro-agent/src/profiles/claude5_tools.rs b/lib/components/fabro-agent/src/profiles/claude5_tools.rs index 20f284c2b..663161bab 100644 --- a/lib/components/fabro-agent/src/profiles/claude5_tools.rs +++ b/lib/components/fabro-agent/src/profiles/claude5_tools.rs @@ -297,7 +297,7 @@ pub(crate) fn make_task_output_tool(supervisor: SubAgentSupervisor) -> Registere "description": "Whether to wait for completion." }, "timeout": { - "type": "number", + "type": "integer", "minimum": 0, "maximum": 600_000, "default": 30000, @@ -402,13 +402,13 @@ pub(crate) fn make_send_message_tool(supervisor: SubAgentSupervisor) -> Register RegisteredTool { definition: definition( NativeTool::SendMessage, - "Send additional instructions to a running child agent by its Fabro agent ID.", + "Send additional instructions to a running background agent by its task ID.", serde_json::json!({ "type": "object", "properties": { "to": { "type": "string", - "description": "The running Fabro agent ID." + "description": "The background agent task ID." }, "message": { "type": "string", diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index f3ff26aa1..798e39661 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -785,41 +785,42 @@ mod tests { insta::assert_snapshot!(system_prompt(&claude5_profile(false, false, false))); } - #[test] - fn claude5_web_search_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(true, false, false))); - } - - #[test] - fn claude5_subagents_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(false, true, false))); - } - - #[test] - fn claude5_question_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(false, false, true))); - } - - #[test] - fn claude5_web_search_and_subagents_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(true, true, false))); - } - - #[test] - fn claude5_web_search_and_question_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(true, false, true))); - } - - #[test] - fn claude5_subagents_and_question_prompt_snapshot() { - insta::assert_snapshot!(system_prompt(&claude5_profile(false, true, true))); - } - #[test] fn claude5_all_conditionals_prompt_snapshot() { insta::assert_snapshot!(system_prompt(&claude5_profile(true, true, true))); } + /// The two snapshots above pin the wording of every conditional section. + /// This covers the six intermediate combinations, which only need to show + /// that each section appears exactly when its tool is registered -- as + /// snapshots they were six near-identical copies of the same prose, and any + /// edit to the template invalidated all eight at once. + #[test] + fn claude5_prompt_sections_track_registered_tools() { + for web_search in [false, true] { + for subagents in [false, true] { + for question in [false, true] { + let prompt = system_prompt(&claude5_profile(web_search, subagents, question)); + assert_eq!( + prompt.contains("Use `WebSearch`"), + web_search, + "web_search={web_search} subagents={subagents} question={question}" + ); + assert_eq!( + prompt.contains("# Background agents"), + subagents, + "web_search={web_search} subagents={subagents} question={question}" + ); + assert_eq!( + prompt.contains("# Asking the user"), + question, + "web_search={web_search} subagents={subagents} question={question}" + ); + } + } + } + } + #[test] fn gemini_default_prompt_snapshot() { insta::assert_snapshot!(system_prompt(&gemini_profile(false))); diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap deleted file mode 100644 index c1628ad2a..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap +++ /dev/null @@ -1,77 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(false, false, true))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - - - -# Asking the user - -Use `AskUserQuestion` only when blocked on a decision that genuinely belongs to the user and cannot be resolved from the request, code, project instructions, or a sensible default. Do not use it for routine permission to continue. - -When recommending an option, put it first and mark it as recommended. The interface supplies a free-form alternative automatically. - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap deleted file mode 100644 index 95d827e7a..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap +++ /dev/null @@ -1,87 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(false, true, true))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - -# Background agents - -Use `Agent` for independent, substantial work or to keep broad exploration out of the parent context. Do not delegate a task and duplicate the same work yourself. - -Agents run in the background by default. Launch independent agents together in one response so they run concurrently. Set `run_in_background` to false when their result is an immediate prerequisite and there is no useful parent work to do meanwhile. - -Background completion and failure notifications arrive automatically. Do not poll `TaskOutput` for ordinary progress. Use it only when you deliberately need to block or inspect status. Use `SendMessage` to add instructions to a running agent and `TaskStop` when a running agent is no longer needed. - -An agent's report is evidence, not automatic proof. Inspect relevant changes or run appropriate verification before reporting delegated implementation as complete. Synthesize the useful result for the user; raw agent reports are not a substitute for the final response. - - - -# Asking the user - -Use `AskUserQuestion` only when blocked on a decision that genuinely belongs to the user and cannot be resolved from the request, code, project instructions, or a sensible default. Do not use it for routine permission to continue. - -When recommending an option, put it first and mark it as recommended. The interface supplies a free-form alternative automatically. - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap deleted file mode 100644 index eaaaf1cbb..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap +++ /dev/null @@ -1,81 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(false, true, false))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - -# Background agents - -Use `Agent` for independent, substantial work or to keep broad exploration out of the parent context. Do not delegate a task and duplicate the same work yourself. - -Agents run in the background by default. Launch independent agents together in one response so they run concurrently. Set `run_in_background` to false when their result is an immediate prerequisite and there is no useful parent work to do meanwhile. - -Background completion and failure notifications arrive automatically. Do not poll `TaskOutput` for ordinary progress. Use it only when you deliberately need to block or inspect status. Use `SendMessage` to add instructions to a running agent and `TaskStop` when a running agent is no longer needed. - -An agent's report is evidence, not automatic proof. Inspect relevant changes or run appropriate verification before reporting delegated implementation as complete. Synthesize the useful result for the user; raw agent reports are not a substitute for the final response. - - - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap deleted file mode 100644 index 041ac14ff..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap +++ /dev/null @@ -1,79 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(true, false, true))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - -Use `WebSearch` when current external information is needed. Its input is a search query; use `WebFetch` to inspect a specific result. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - - - -# Asking the user - -Use `AskUserQuestion` only when blocked on a decision that genuinely belongs to the user and cannot be resolved from the request, code, project instructions, or a sensible default. Do not use it for routine permission to continue. - -When recommending an option, put it first and mark it as recommended. The interface supplies a free-form alternative automatically. - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap deleted file mode 100644 index d67ec831e..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap +++ /dev/null @@ -1,83 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(true, true, false))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - -Use `WebSearch` when current external information is needed. Its input is a search query; use `WebFetch` to inspect a specific result. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - -# Background agents - -Use `Agent` for independent, substantial work or to keep broad exploration out of the parent context. Do not delegate a task and duplicate the same work yourself. - -Agents run in the background by default. Launch independent agents together in one response so they run concurrently. Set `run_in_background` to false when their result is an immediate prerequisite and there is no useful parent work to do meanwhile. - -Background completion and failure notifications arrive automatically. Do not poll `TaskOutput` for ordinary progress. Use it only when you deliberately need to block or inspect status. Use `SendMessage` to add instructions to a running agent and `TaskStop` when a running agent is no longer needed. - -An agent's report is evidence, not automatic proof. Inspect relevant changes or run appropriate verification before reporting delegated implementation as complete. Synthesize the useful result for the user; raw agent reports are not a substitute for the final response. - - - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap deleted file mode 100644 index b92b72e73..000000000 --- a/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap +++ /dev/null @@ -1,73 +0,0 @@ ---- -source: lib/components/fabro-agent/src/profiles/mod.rs -expression: "system_prompt(&claude5_profile(true, false, false))" ---- -You are Claude, a software engineering agent running in Fabro. Use the tools available in this session to complete software engineering work and to answer questions about the codebase. - -When the request is to explain, diagnose, review, or report status, inspect the relevant evidence and return an assessment. Do not modify files or external state unless the request asks for a change. When the request asks you to build or change something, implement the complete change and verify it. - - -Working directory: /home/test -Is git repository: false -Platform: linux -OS version: Linux 6.1.0 - - -# Harness - -- Text outside tool calls is shown to the user as GitHub-flavored Markdown. -- The user may not see your reasoning or raw tool output. Make the final response self-contained. -- Independent tool calls can run in parallel in one response. Run dependent operations sequentially. -- Follow all project and user instructions included in this prompt. -- Reference code with `file_path:line_number` when a precise location helps. - -# Delivering work - -Work on the request actually given. Do not quietly narrow, widen, or transform its scope. Make routine judgment calls yourself. If different interpretations would materially change the result, finish everything independent of that decision and use `AskUserQuestion` when it is available. - -Keep going until the requested outcome is complete. Do not stop at a plan, a list of next steps, or a promise to do work that can be performed with the available tools. If one part is blocked, complete the remaining independent work and report the blocker precisely. - -Use evidence rather than guesses. When an attempt fails, inspect the error and assumptions before making a focused adjustment. Report outcomes faithfully: state which verification ran, what passed or failed, and what was not checked. - -# Working in the codebase - -- Read relevant code before proposing or making changes. -- This workspace tracks reads before writes. Before every `Edit` or `Write` to an existing file, call `Read` on its current contents. Re-read after the file changes before making another edit to it. -- Prefer `Edit` for targeted changes. `Write` creates a file or deliberately replaces its entire contents. -- Make the smallest complete change that satisfies the request. Avoid unrelated refactors, speculative abstractions, and compatibility machinery without a real requirement. -- Match the surrounding code's structure, naming, formatting, and comment density. Add comments only for constraints or reasoning the code cannot make evident. -- Validate at real boundaries such as external input and APIs; do not add defensive branches for states excluded by established invariants. -- Run the most targeted useful verification first, then broaden it when the risk warrants it. Exercise user-facing behavior when the environment supports doing so. - -# Tool use - -Use `Read`, `Edit`, and `Write` instead of shell commands for file inspection and mutation. `Read` handles UTF-8 text and returns numbered lines; it does not render images, PDFs, or notebooks. - -Use `Bash` for searches, git inspection, builds, tests, package managers, and other terminal operations. Prefer `rg` for content search and `rg --files` with `-g` filters for file discovery. Narrow commands so their output remains useful. - -Each `Bash` call is a fresh foreground, non-login shell. Working-directory and environment changes do not persist between calls; keep dependent `cd` or environment setup in the same command. `timeout` is measured in milliseconds. - -Use `TaskCreate`, `TaskUpdate`, `TaskGet`, and `TaskList` when meaningful multi-step work benefits from visible tracking. Keep statuses current as work progresses. Skip task tracking when it adds no value. - -Use `WebFetch` with both a URL and a prompt describing the information to extract. - -Use `WebSearch` when current external information is needed. Its input is a search query; use `WebFetch` to inspect a specific result. - - -Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. - - - - - -# Communicating with the user - -Before the first tool call, state what you are about to do in one concise sentence. While working, give brief updates only at meaningful milestones, such as finding the cause, changing direction, or completing a major phase. - -Do not expose internal deliberation. Write for a teammate catching up: use complete sentences, explain only details that affect conclusions or next actions, and avoid private shorthand. - -Lead the final response with the outcome. A simple question deserves a direct answer rather than unnecessary sections. Be concise by omitting low-value detail, not by compressing useful explanation into fragments. - -# Context management - -Long sessions may be summarized and continued in a new context window. Treat the supplied summary as the continuation of the same work. Do not wrap up early merely because the session is long. diff --git a/lib/components/fabro-agent/src/session.rs b/lib/components/fabro-agent/src/session.rs index f04e99257..c72d33683 100644 --- a/lib/components/fabro-agent/src/session.rs +++ b/lib/components/fabro-agent/src/session.rs @@ -47,10 +47,7 @@ use crate::skills::{ ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill, make_use_skill_tool_for_vocabulary, }; -use crate::subagent::{ - SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor, - format_parent_notification_batch, -}; +use crate::subagent::{SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor}; use crate::tool_execution::execute_tool_calls; use crate::tool_permissions::canonical_tool_name; use crate::tool_registry::ToolDefinitionWithSource; @@ -371,6 +368,18 @@ struct BuiltRequest { context_window: StageContextWindowProjection, } +/// Whether an input's `/name` tokens should be treated as skill references. +/// +/// Only text the user actually typed can invoke a skill. Harness-synthesized +/// input carries whatever a child agent wrote, where `/tmp` is a path rather +/// than an invocation: expanding it would either fail the parent turn on an +/// unknown name or splice a skill template in place of the envelope. +#[derive(Clone, Copy, PartialEq, Eq)] +enum SkillExpansion { + Apply, + Skip, +} + pub struct Session { id: String, /// Root agent session ID for this session's agent tree. A root session @@ -1328,6 +1337,7 @@ impl Session { let mut result = self .run_single_input( input, + SkillExpansion::Apply, &agent_tool_runtime, &mut timing, &mut usage, @@ -1343,15 +1353,13 @@ impl Session { .expect("followup queue lock poisoned") .pop_front(); let next_input = if let Some(followup) = followup { - Some(followup) + Some((followup, SkillExpansion::Apply)) } else if let Some(supervisor) = self.subagent_supervisor.clone() { match supervisor - .next_parent_notification_batch(&self.cancel_token) + .next_parent_notification_turn(&self.cancel_token) .await { - Ok(Some(notifications)) => { - Some(format_parent_notification_batch(¬ifications)) - } + Ok(Some(turn)) => Some((turn, SkillExpansion::Skip)), Ok(None) => None, Err(Error::Interrupted(InterruptReason::Cancelled)) => { result = Err(self.interrupted_error()); @@ -1365,10 +1373,13 @@ impl Session { } else { None }; - let Some(next_input) = next_input else { break }; + let Some((next_input, skill_expansion)) = next_input else { + break; + }; result = self .run_single_input( &next_input, + skill_expansion, &agent_tool_runtime, &mut timing, &mut usage, @@ -1406,6 +1417,7 @@ impl Session { async fn run_single_input( &mut self, input: &str, + skill_expansion: SkillExpansion, agent_tool_runtime: &AgentToolRuntime, timing: &mut SessionInputTiming, usage_accumulator: &mut TokenCounts, @@ -1420,7 +1432,7 @@ impl Session { self.transition(SessionState::Thinking); // Expand skill references in input - let expanded = if self.skills.is_empty() { + let expanded = if self.skills.is_empty() || skill_expansion == SkillExpansion::Skip { ExpandedInput { text: input.to_string(), skill_name: None, @@ -3133,6 +3145,57 @@ mod tests { supervisor.shutdown_all().await; } + #[tokio::test] + async fn background_agent_output_is_not_parsed_for_skill_references() { + let supervisor = SubAgentSupervisor::new(3); + let child = make_session(vec![text_response("Cleaned up /tmp and exited")]).await; + let child_id = supervisor + .spawn_with_parent_notification( + child, + "clean up".to_string(), + "Clean scratch files".to_string(), + 0, + ) + .unwrap(); + supervisor + .wait_with_cancel(&child_id, &CancellationToken::new()) + .await + .unwrap(); + + let provider = Arc::new(ScriptedStreamProvider::new(vec![ + ScriptedStreamCall::Response(Box::new(text_response("Delegated"))), + ScriptedStreamCall::Response(Box::new(text_response("Acknowledged"))), + ])); + let mut parent = + make_session_with_provider_and_manager(provider, Some(supervisor.clone())).await; + parent.skills = vec![Skill { + name: "commit".to_string(), + description: "Make a commit".to_string(), + template: "Review changes and commit.".to_string(), + }]; + + // A child that mentions a bare path must not fail the parent turn on + // `Unknown skill: /tmp`, nor have its report replaced by a skill body. + let output = parent + .process_input_with_output("Delegate the cleanup") + .await + .unwrap(); + + assert_eq!(output.as_deref(), Some("Acknowledged")); + let turns = parent.history().turns(); + let Message::User { + content: notification, + .. + } = &turns[2] + else { + panic!("third turn should deliver the background result"); + }; + assert!(notification.contains("Cleaned up /tmp and exited")); + assert!(!notification.contains("Review changes and commit.")); + + supervisor.shutdown_all().await; + } + #[tokio::test] async fn events_emitted() { let mut session = make_session(vec![text_response("Hello")]).await; diff --git a/lib/components/fabro-agent/src/subagent.rs b/lib/components/fabro-agent/src/subagent.rs index 0d91d58e8..f4d6c1b84 100644 --- a/lib/components/fabro-agent/src/subagent.rs +++ b/lib/components/fabro-agent/src/subagent.rs @@ -42,9 +42,7 @@ pub(crate) struct SubAgentParentNotification { pub result: Result, } -pub(crate) fn format_parent_notification_batch( - notifications: &[SubAgentParentNotification], -) -> String { +fn format_parent_notification_batch(notifications: &[SubAgentParentNotification]) -> String { notifications .iter() .map(|notification| { @@ -481,6 +479,16 @@ impl SubAgentSupervisor { child_depth, ); + // Register before the agent becomes discoverable in `state.agents`. + // Once it is, a concurrent `shutdown_all` can suppress and close it; + // registering afterwards would leave a pending entry that the monitor + // never completes (it early-returns for a non-Running agent), and + // `next_batch` would then never report the queue as drained. Nothing + // can complete this registration before `start_tx.send(())` below. + if let Some(description) = parent_notification_description { + self.parent_notifications + .register(agent_id.clone(), description); + } { let mut state = self.state.lock().expect("subagent state lock poisoned"); state.agents.insert(agent_id.clone(), SubAgent { @@ -496,10 +504,6 @@ impl SubAgentSupervisor { depth: child_depth, }); } - if let Some(description) = parent_notification_description { - self.parent_notifications - .register(agent_id.clone(), description); - } self.emit_event(AgentEvent::SubAgentSpawned { agent_id: agent_id.clone(), @@ -591,7 +595,23 @@ impl SubAgentSupervisor { } /// Wait until all currently-ready background results can be delivered in - /// one parent turn, or return `None` once no notifiable agents remain. + /// one parent turn, rendered as the text of that turn. Returns `None` once + /// no notifiable agents remain. + /// + /// The envelope format is the supervisor's concern, so callers receive a + /// finished turn rather than the notifications behind it. + pub(crate) async fn next_parent_notification_turn( + &self, + cancel: &CancellationToken, + ) -> Result, Error> { + Ok(self + .next_parent_notification_batch(cancel) + .await? + .map(|notifications| format_parent_notification_batch(¬ifications))) + } + + /// The notifications behind [`Self::next_parent_notification_turn`], for + /// tests that assert on delivery semantics rather than on the rendering. pub(crate) async fn next_parent_notification_batch( &self, cancel: &CancellationToken, @@ -606,7 +626,6 @@ impl SubAgentSupervisor { } fn begin_shutdown(&self, agent_id: &str, strict: bool) -> Result { - self.parent_notifications.suppress(agent_id); let mut state = self.state.lock().expect("subagent state lock poisoned"); let agent = state.agents.get_mut(agent_id).ok_or_else(|| { Error::InvalidState(format!( @@ -752,7 +771,12 @@ impl SubAgentSupervisor { } async fn ensure_closed(&self, agent_id: &str) -> Result<(), Error> { - let cleanup_done = match self.begin_shutdown(agent_id, false)? { + let disposition = self.begin_shutdown(agent_id, false)?; + // Only once shutdown is committed. Suppressing before `begin_shutdown` + // would also discard the result of an agent that had already finished, + // which rejects the shutdown but had a delivery pending. + self.parent_notifications.suppress(agent_id); + let cleanup_done = match disposition { ShutdownDisposition::Lead(work) => self.spawn_shutdown(work), ShutdownDisposition::Follow(cleanup_done) => cleanup_done, ShutdownDisposition::Done => return Ok(()), @@ -763,7 +787,9 @@ impl SubAgentSupervisor { /// Strict user-facing close: only a currently running child may be closed. pub async fn close_agent(&self, agent_id: &str) -> Result<(), Error> { - let cleanup_done = match self.begin_shutdown(agent_id, true)? { + let disposition = self.begin_shutdown(agent_id, true)?; + self.parent_notifications.suppress(agent_id); + let cleanup_done = match disposition { ShutdownDisposition::Lead(work) => self.spawn_shutdown(work), ShutdownDisposition::Follow(_) | ShutdownDisposition::Done => { return Err(Error::InvalidState(format!( @@ -1109,6 +1135,41 @@ mod tests { ); } + #[tokio::test] + async fn rejected_stop_of_a_finished_agent_keeps_its_notification() { + let supervisor = SubAgentSupervisor::new(3); + let child = make_session(vec![text_response("child result")]).await; + let agent_id = supervisor + .spawn_with_parent_notification( + child, + "task".to_string(), + "Inspect the module".to_string(), + 0, + ) + .unwrap(); + + // Finish the child so its result is queued for automatic delivery. + supervisor + .wait_with_cancel(&agent_id, &CancellationToken::new()) + .await + .unwrap(); + + // Stopping a finished agent is rejected... + let error = supervisor.close_agent(&agent_id).await.unwrap_err(); + assert!(matches!(error, Error::InvalidState(_)), "{error:?}"); + + // ...so it must not have discarded the result the parent is owed. + let batch = supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .expect("a rejected stop must leave the pending result deliverable"); + assert_eq!(batch.len(), 1); + assert_eq!(batch[0].agent_id, agent_id); + + supervisor.shutdown_all().await; + } + #[tokio::test] async fn spawn_creates_agent_and_returns_id() { let manager = SubAgentSupervisor::new(3); diff --git a/lib/components/fabro-agent/src/todo_runtime.rs b/lib/components/fabro-agent/src/todo_runtime.rs index 0d1d79e11..dbf2791f5 100644 --- a/lib/components/fabro-agent/src/todo_runtime.rs +++ b/lib/components/fabro-agent/src/todo_runtime.rs @@ -17,20 +17,26 @@ use fabro_types::{ use crate::tool_registry::ToolContext; use crate::types::AgentEvent; +/// Projections and their ID counters, behind one lock so a list and its +/// counter can never be observed out of step. +#[derive(Debug, Default)] +struct TodoRuntimeState { + lists: BTreeMap, + task_counters: BTreeMap, +} + /// Shared, thread-safe todo projection. Wrap it in `Arc` and clone the /// `Arc` into each tool closure that needs it. #[derive(Debug, Default)] pub struct TodoRuntime { - lists: Mutex>, - task_counters: Mutex>, + state: Mutex, } impl TodoRuntime { #[must_use] pub fn new() -> Self { Self { - lists: Mutex::new(BTreeMap::new()), - task_counters: Mutex::new(BTreeMap::new()), + state: Mutex::new(TodoRuntimeState::default()), } } @@ -39,11 +45,8 @@ impl TodoRuntime { /// Keeping the counter beside the projection lets root and child profiles /// safely create tasks in the same shared list. pub(crate) fn next_task_id(&self, list_id: &str) -> u64 { - let mut counters = self - .task_counters - .lock() - .expect("task counter lock poisoned"); - let counter = counters.entry(list_id.to_string()).or_default(); + let mut guard = self.state.lock().expect("todo runtime lock poisoned"); + let counter = guard.task_counters.entry(list_id.to_string()).or_default(); *counter = counter.saturating_add(1); *counter } @@ -52,8 +55,8 @@ impl TodoRuntime { /// list-style tools that need a stable view. #[must_use] pub fn snapshot(&self, list_id: &str) -> Option { - let guard = self.lists.lock().expect("todo runtime lock poisoned"); - guard.get(list_id).cloned() + let guard = self.state.lock().expect("todo runtime lock poisoned"); + guard.lists.get(list_id).cloned() } /// Insert (or replace) a todo and emit `todo.created`. @@ -79,8 +82,9 @@ impl TodoRuntime { metadata: todo.metadata.clone(), }; { - let mut guard = self.lists.lock().expect("todo runtime lock poisoned"); + let mut guard = self.state.lock().expect("todo runtime lock poisoned"); guard + .lists .entry(list_id) .or_insert_with(|| TodoListProjection::new(kind, props.list_id.clone())) .upsert(todo); @@ -97,8 +101,8 @@ impl TodoRuntime { } let applied = { - let mut guard = self.lists.lock().expect("todo runtime lock poisoned"); - let Some(list) = guard.get_mut(&props.list_id) else { + let mut guard = self.state.lock().expect("todo runtime lock poisoned"); + let Some(list) = guard.lists.get_mut(&props.list_id) else { return false; }; list.apply_patch(&props.todo_id, &TodoPatch::from_props(&props)) @@ -119,8 +123,8 @@ impl TodoRuntime { todo_id: String, ) -> bool { let removed = { - let mut guard = self.lists.lock().expect("todo runtime lock poisoned"); - let Some(list) = guard.get_mut(&list_id) else { + let mut guard = self.state.lock().expect("todo runtime lock poisoned"); + let Some(list) = guard.lists.get_mut(&list_id) else { return false; }; list.remove(&todo_id)