From 4167fcd39b1f1894d3d0d30b623f1de4c3333f09 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 13:15:57 -0400 Subject: [PATCH 01/19] feat(agent): add Claude 5 profile --- lib/components/fabro-agent/src/config.rs | 5 +- lib/components/fabro-agent/src/lib.rs | 3 +- lib/components/fabro-agent/src/memory.rs | 21 +- lib/components/fabro-agent/src/native_tool.rs | 55 +- .../fabro-agent/src/profiles/claude5.rs | 261 +++++++ .../fabro-agent/src/profiles/claude5_tools.rs | 670 ++++++++++++++++++ .../fabro-agent/src/profiles/mod.rs | 160 ++++- .../src/profiles/prompts/claude5.md.j2 | 80 +++ ...ude5_all_conditionals_prompt_snapshot.snap | 89 +++ ...ests__claude5_default_prompt_snapshot.snap | 71 ++ ...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 ++ .../fabro-agent/src/question_tools.rs | 280 ++++++++ lib/components/fabro-agent/src/session.rs | 101 ++- lib/components/fabro-agent/src/skills.rs | 60 ++ lib/components/fabro-agent/src/subagent.rs | 281 +++++++- .../fabro-agent/src/todo_runtime.rs | 20 +- lib/components/fabro-agent/src/todo_tools.rs | 29 +- lib/components/fabro-agent/src/tools.rs | 2 +- .../fabro-llm/src/adapter_registry.rs | 7 +- .../fabro-workflow/src/handler/llm/api.rs | 38 +- .../fabro-workflow/src/operations/create.rs | 4 +- .../fabro-workflow/src/pipeline/transform.rs | 4 +- .../fabro-workflow/tests/materialize_run.rs | 2 +- lib/foundation/fabro-model/src/adapter.rs | 6 + lib/foundation/fabro-model/src/catalog.rs | 54 +- .../src/catalog/providers/anthropic.toml | 31 +- .../src/catalog/providers/bedrock.toml | 38 +- .../src/catalog/providers/openrouter.toml | 35 +- 33 files changed, 2795 insertions(+), 92 deletions(-) create mode 100644 lib/components/fabro-agent/src/profiles/claude5.rs create mode 100644 lib/components/fabro-agent/src/profiles/claude5_tools.rs create mode 100644 lib/components/fabro-agent/src/profiles/prompts/claude5.md.j2 create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_all_conditionals_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_default_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap create mode 100644 lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap diff --git a/lib/components/fabro-agent/src/config.rs b/lib/components/fabro-agent/src/config.rs index 51165fd3e..a6c8b5c39 100644 --- a/lib/components/fabro-agent/src/config.rs +++ b/lib/components/fabro-agent/src/config.rs @@ -130,7 +130,7 @@ impl NativeToolOptions { // Matched exhaustively so a new profile kind has to state its answer // rather than silently inheriting the default timeout. let default_command_timeout_ms = match profile_kind { - AgentProfileKind::Anthropic => 120_000, + AgentProfileKind::Anthropic | AgentProfileKind::Claude5 => 120_000, // Matches the 60s foreground default Kimi Code's Bash tool // documents, which is what these models are used to budgeting // against. @@ -333,12 +333,15 @@ mod tests { fn native_tool_options_have_expected_profile_defaults() { let openai = NativeToolOptions::for_profile(AgentProfileKind::OpenAi); let anthropic = NativeToolOptions::for_profile(AgentProfileKind::Anthropic); + let claude5 = NativeToolOptions::for_profile(AgentProfileKind::Claude5); let kimi = NativeToolOptions::for_profile(AgentProfileKind::Kimi); assert_eq!(openai.default_command_timeout_ms, 10_000); assert_eq!(openai.max_command_timeout_ms, 600_000); assert_eq!(anthropic.default_command_timeout_ms, 120_000); assert_eq!(anthropic.max_command_timeout_ms, 600_000); + assert_eq!(claude5.default_command_timeout_ms, 120_000); + assert_eq!(claude5.max_command_timeout_ms, 600_000); assert_eq!(kimi.default_command_timeout_ms, 60_000); assert_eq!(kimi.max_command_timeout_ms, 600_000); } diff --git a/lib/components/fabro-agent/src/lib.rs b/lib/components/fabro-agent/src/lib.rs index e8c8b01dc..738faaf23 100644 --- a/lib/components/fabro-agent/src/lib.rs +++ b/lib/components/fabro-agent/src/lib.rs @@ -50,7 +50,8 @@ pub use loop_detection::detect_loop; pub use memory::{MemoryDocument, discover_memory}; pub use native_tool::{NativeTool, ToolVocabulary}; pub use profiles::{ - AgentProfileBuilder, AnthropicProfile, EnvContext, GeminiProfile, KimiProfile, OpenAiProfile, + AgentProfileBuilder, AnthropicProfile, Claude5Profile, EnvContext, GeminiProfile, KimiProfile, + OpenAiProfile, }; pub use question_tools::{ ANTHROPIC_ASK_USER_QUESTION_TOOL, AgentQuestion, AgentQuestionAnswer, diff --git a/lib/components/fabro-agent/src/memory.rs b/lib/components/fabro-agent/src/memory.rs index 41d607ecc..94a88eb40 100644 --- a/lib/components/fabro-agent/src/memory.rs +++ b/lib/components/fabro-agent/src/memory.rs @@ -31,7 +31,9 @@ pub async fn discover_memory( let directories = build_directory_walk(git_root, working_dir); let candidate_filenames: Vec<&str> = match profile_kind { - AgentProfileKind::Anthropic => vec!["AGENTS.md", "CLAUDE.md"], + AgentProfileKind::Anthropic | AgentProfileKind::Claude5 => { + vec!["AGENTS.md", "CLAUDE.md"] + } AgentProfileKind::OpenAi | AgentProfileKind::Gpt56 => { vec!["AGENTS.md", ".codex/instructions.md"] } @@ -207,6 +209,23 @@ mod tests { assert_eq!(anthropic_docs[0].content, "agents"); assert_eq!(anthropic_docs[1].content, "claude"); + let env: Arc = Arc::new(MockSandbox { + files: files.clone(), + ..Default::default() + }); + let claude5_docs = discover_memory( + env.as_ref(), + "/repo", + "/repo", + AgentProfileKind::Claude5, + &CancellationToken::new(), + ) + .await + .unwrap(); + assert_eq!(claude5_docs.len(), 2); + assert_eq!(claude5_docs[0].content, "agents"); + assert_eq!(claude5_docs[1].content, "claude"); + let env: Arc = Arc::new(MockSandbox { files: files.clone(), ..Default::default() diff --git a/lib/components/fabro-agent/src/native_tool.rs b/lib/components/fabro-agent/src/native_tool.rs index bcedb9c53..83b1890ff 100644 --- a/lib/components/fabro-agent/src/native_tool.rs +++ b/lib/components/fabro-agent/src/native_tool.rs @@ -9,8 +9,8 @@ //! //! A [`NativeTool`] is an identity, not a name. The same tool is expressed //! under different names depending on the [`ToolVocabulary`] a profile speaks: -//! fabro's own names by default, Kimi Code's names for the Kimi profile, and -//! Codex's names for the GPT-5.6 profile. +//! fabro's own names by default, Anthropic's names for Claude 5, Kimi Code's +//! names for the Kimi profile, and Codex's names for the GPT-5.6 profile. //! Permissions, categories, and telemetry resolve any name back to the //! identity, so behavior never depends on which vocabulary is in play. //! @@ -26,6 +26,8 @@ pub enum ToolVocabulary { /// Fabro's own names, and the canonical identity used internally. #[default] Fabro, + /// The names Anthropic's Claude 5 coding harness exposes. + Claude5, /// The names Kimi Code exposes, for models trained against that harness. KimiCode, /// The names Codex exposes, for the GPT-5.6 models trained against it. @@ -57,7 +59,11 @@ pub enum NativeTool { Shell, #[strum(to_string = "web_search", serialize = "WebSearch")] WebSearch, - #[strum(to_string = "web_fetch", serialize = "FetchURL")] + #[strum( + to_string = "web_fetch", + serialize = "FetchURL", + serialize = "WebFetch" + )] WebFetch, #[strum(to_string = "spawn_agent")] SpawnAgent, @@ -67,6 +73,14 @@ pub enum NativeTool { Wait, #[strum(to_string = "close_agent")] CloseAgent, + #[strum(to_string = "Agent")] + ClaudeAgent, + #[strum(to_string = "TaskOutput")] + TaskOutput, + #[strum(to_string = "TaskStop")] + TaskStop, + #[strum(to_string = "SendMessage")] + SendMessage, #[strum(to_string = "use_skill", serialize = "Skill")] UseSkill, #[strum(to_string = "update_plan")] @@ -116,6 +130,16 @@ impl NativeTool { pub fn name(self, vocabulary: ToolVocabulary) -> &'static str { match vocabulary { ToolVocabulary::Fabro => self.canonical_name(), + ToolVocabulary::Claude5 => match self { + Self::ReadFile => "Read", + Self::WriteFile => "Write", + Self::EditFile => "Edit", + Self::Shell => "Bash", + Self::WebSearch => "WebSearch", + Self::WebFetch => "WebFetch", + Self::UseSkill => "Skill", + other => other.canonical_name(), + }, ToolVocabulary::KimiCode => match self { Self::ReadFile => "Read", Self::WriteFile => "Write", @@ -177,9 +201,14 @@ impl NativeTool { } Self::WriteFile | Self::EditFile | Self::ApplyPatch => Some(AgentToolCategory::Write), Self::Shell => Some(AgentToolCategory::Shell), - Self::SpawnAgent | Self::SendInput | Self::Wait | Self::CloseAgent => { - Some(AgentToolCategory::Subagent) - } + Self::SpawnAgent + | Self::SendInput + | Self::Wait + | Self::CloseAgent + | Self::ClaudeAgent + | Self::TaskOutput + | Self::TaskStop + | Self::SendMessage => Some(AgentToolCategory::Subagent), // Uncategorized today. Giving these a category would change the CLI // permission gate, which is a behavior change rather than a // classification cleanup, so they keep their existing answer. @@ -261,6 +290,20 @@ mod tests { ); } + #[test] + fn claude5_vocabulary_uses_anthropic_harness_names() { + assert_eq!(NativeTool::ReadFile.name(ToolVocabulary::Claude5), "Read"); + assert_eq!(NativeTool::Shell.name(ToolVocabulary::Claude5), "Bash"); + assert_eq!( + NativeTool::WebFetch.name(ToolVocabulary::Claude5), + "WebFetch" + ); + assert_eq!( + NativeTool::ClaudeAgent.name(ToolVocabulary::Claude5), + "Agent" + ); + } + #[test] fn codex_vocabulary_renames_only_the_shell() { assert_eq!( diff --git a/lib/components/fabro-agent/src/profiles/claude5.rs b/lib/components/fabro-agent/src/profiles/claude5.rs new file mode 100644 index 000000000..6b7826584 --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/claude5.rs @@ -0,0 +1,261 @@ +//! Profile for Claude Fable 5, Opus 5, and Sonnet 5. + +use std::sync::Arc; + +use fabro_model::{AgentProfileKind, Catalog, ProviderId}; + +use super::EnvContext; +use crate::agent_profile::AgentProfile; +use crate::config::NativeToolOptions; +use crate::native_tool::{NativeTool, ToolVocabulary}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, claude5_tools}; +use crate::sandbox::Sandbox; +use crate::skills::Skill; +use crate::subagent::{SessionFactory, SubAgentSupervisor}; +use crate::todo_runtime::TodoRuntime; +use crate::todo_tools::{ + make_task_create_tool, make_task_get_tool, make_task_list_tool, make_task_update_tool, +}; +use crate::tool_registry::ToolRegistry; +use crate::tools::WebFetchSummarizer; + +const CORE_PROMPT: &str = include_str!("prompts/claude5.md.j2"); + +pub struct Claude5Profile { + base: BaseProfile, +} + +impl Claude5Profile { + #[must_use] + pub fn new(model: impl Into) -> Self { + let options = NativeToolOptions::for_profile(AgentProfileKind::Claude5); + Self::with_native_tools(model, &options, None) + } + + pub(crate) fn with_native_tools( + model: impl Into, + options: &NativeToolOptions, + summarizer: Option, + ) -> Self { + Self::with_native_tools_and_todo_runtime( + model, + options, + summarizer, + Arc::new(TodoRuntime::new()), + ) + } + + pub(crate) fn with_native_tools_and_todo_runtime( + model: impl Into, + options: &NativeToolOptions, + summarizer: Option, + todo_runtime: Arc, + ) -> Self { + let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::Claude5); + registry.register(claude5_tools::make_read_tool()); + registry.register(claude5_tools::make_write_tool()); + registry.register(claude5_tools::make_edit_tool()); + registry.register(claude5_tools::make_bash_tool(options)); + registry.register(claude5_tools::make_web_fetch_tool(summarizer)); + if let Some(api_key) = &options.secrets.brave_search_api_key { + registry.register(claude5_tools::make_web_search_tool(api_key.clone())); + } + + registry.register(claude5_tools::strict_object_tool(make_task_create_tool( + todo_runtime.clone(), + ))); + registry.register(claude5_tools::strict_object_tool(make_task_update_tool( + todo_runtime.clone(), + ))); + registry.register(claude5_tools::strict_object_tool(make_task_get_tool( + todo_runtime.clone(), + ))); + registry.register(claude5_tools::strict_object_tool(make_task_list_tool( + todo_runtime, + ))); + + Self { + base: BaseProfile { + profile_kind: AgentProfileKind::Claude5, + provider_id: ProviderId::anthropic(), + model: model.into(), + catalog: None, + registry, + }, + } + } + + /// Override the transport provider while retaining Claude 5 harness + /// behavior. + #[must_use] + pub fn with_provider_id(mut self, provider_id: ProviderId) -> Self { + self.base.provider_id = provider_id; + self + } + + #[must_use] + pub fn with_catalog(mut self, catalog: Arc) -> Self { + self.base.catalog = Some(catalog); + self + } +} + +impl AgentProfile for Claude5Profile { + fn profile_kind(&self) -> AgentProfileKind { + self.base.profile_kind + } + + fn provider_id(&self) -> ProviderId { + self.base.provider_id.clone() + } + + fn model(&self) -> &str { + &self.base.model + } + + fn catalog(&self) -> Option<&Catalog> { + self.base.catalog.as_deref() + } + + fn tool_registry(&self) -> &ToolRegistry { + &self.base.registry + } + + fn tool_registry_mut(&mut self) -> &mut ToolRegistry { + &mut self.base.registry + } + + fn build_system_prompt( + &self, + env: &dyn Sandbox, + env_context: &EnvContext, + memory: &[String], + user_instructions: Option<&str>, + skills: &[Skill], + ) -> String { + let template = EmbeddedPrompt::new("claude5.md.j2", CORE_PROMPT) + .with_vocabulary(ToolVocabulary::Claude5) + .with_bool( + "has_agent", + self.base + .registry + .get_native(NativeTool::ClaudeAgent) + .is_some(), + ) + .with_bool( + "has_ask_user_question", + self.base + .registry + .get_native(NativeTool::AskUserQuestion) + .is_some(), + ) + .with_bool( + "has_web_search", + self.base + .registry + .get_native(NativeTool::WebSearch) + .is_some(), + ); + + profiles::assemble_system_prompt( + template, + env, + env_context, + memory, + user_instructions, + skills, + ) + } + + fn register_subagent_tools( + &mut self, + supervisor: SubAgentSupervisor, + session_factory: SessionFactory, + current_depth: usize, + ) { + self.base.registry.register(claude5_tools::make_agent_tool( + supervisor.clone(), + session_factory, + current_depth, + )); + self.base + .registry + .register(claude5_tools::make_task_output_tool(supervisor.clone())); + self.base + .registry + .register(claude5_tools::make_task_stop_tool(supervisor.clone())); + self.base + .registry + .register(claude5_tools::make_send_message_tool(supervisor)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::subagent::SessionFactory; + use crate::test_support::MockSandbox; + + #[test] + fn profile_identity() { + let profile = Claude5Profile::new("claude-fable-5"); + assert_eq!(profile.profile_kind(), AgentProfileKind::Claude5); + assert_eq!(profile.provider_id(), ProviderId::anthropic()); + assert_eq!(profile.model(), "claude-fable-5"); + } + + #[test] + fn core_tools_match_the_accepted_claude5_surface() { + let profile = Claude5Profile::new("claude-sonnet-5"); + let mut names = profile.tool_registry().names(); + names.sort(); + assert_eq!(names, vec![ + "Bash", + "Edit", + "Read", + "TaskCreate", + "TaskGet", + "TaskList", + "TaskUpdate", + "WebFetch", + "Write", + ]); + assert!(!names.iter().any(|name| name == "Grep" || name == "Glob")); + } + + #[test] + fn root_agent_tools_use_claude_names() { + let mut profile = Claude5Profile::new("claude-opus-5"); + let factory: SessionFactory = Arc::new(|| panic!("unused")); + profile.register_subagent_tools(SubAgentSupervisor::new(3), factory, 0); + + for expected in ["Agent", "TaskOutput", "TaskStop", "SendMessage"] { + assert!( + profile.tool_registry().get(expected).is_some(), + "missing {expected}" + ); + } + for absent in ["spawn_agent", "wait", "close_agent", "send_input"] { + assert!( + profile.tool_registry().get(absent).is_none(), + "found {absent}" + ); + } + } + + #[test] + fn prompt_conditionals_follow_registered_tools() { + let env = MockSandbox::linux(); + let profile = Claude5Profile::new("claude-fable-5"); + let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); + assert!(!prompt.contains("# Background agents")); + assert!(!prompt.contains("# Asking the user")); + assert!(!prompt.contains("Use `WebSearch`")); + + let mut profile = profile; + let factory: SessionFactory = Arc::new(|| panic!("unused")); + profile.register_subagent_tools(SubAgentSupervisor::new(3), factory, 0); + let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); + assert!(prompt.contains("# Background agents")); + } +} diff --git a/lib/components/fabro-agent/src/profiles/claude5_tools.rs b/lib/components/fabro-agent/src/profiles/claude5_tools.rs new file mode 100644 index 000000000..20f284c2b --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/claude5_tools.rs @@ -0,0 +1,670 @@ +//! Claude 5 harness adapters. +//! +//! Execution stays shared with Fabro wherever the behavior agrees. This module +//! narrows the model-facing schemas and supplies the few lifecycle semantics +//! that differ from Fabro's native tools. + +use std::sync::Arc; +use std::time::Duration; + +use fabro_llm::types::ToolDefinition; +use fabro_util::error as util_error; +use serde_json::Value; +use tokio::time; + +use crate::config::NativeToolOptions; +use crate::error::{Error, InterruptReason}; +use crate::native_tool::NativeTool; +use crate::session::Session; +use crate::subagent::{SessionFactory, SubAgentResult, SubAgentStatus, SubAgentSupervisor}; +use crate::tool_registry::{RegisteredTool, ToolContext, ToolSource}; +use crate::tools::{self, WebFetchSummarizer}; + +fn definition( + tool: NativeTool, + description: impl Into, + parameters: Value, +) -> ToolDefinition { + ToolDefinition { + name: tool.canonical_name().to_string(), + description: description.into(), + parameters, + } +} + +/// Reject unknown top-level fields while retaining a shared executor. +#[must_use] +pub(crate) fn strict_object_tool(mut tool: RegisteredTool) -> RegisteredTool { + let object = tool + .definition + .parameters + .as_object_mut() + .expect("native JSON-schema tools should use an object schema"); + object.insert("additionalProperties".to_string(), Value::Bool(false)); + tool +} + +#[must_use] +pub(crate) fn make_read_tool() -> RegisteredTool { + strict_object_tool(tools::make_read_file_tool()) +} + +#[must_use] +pub(crate) fn make_write_tool() -> RegisteredTool { + strict_object_tool(tools::make_write_file_tool()) +} + +#[must_use] +pub(crate) fn make_edit_tool() -> RegisteredTool { + strict_object_tool(tools::make_edit_file_tool()) +} + +#[must_use] +pub(crate) fn make_bash_tool(options: &NativeToolOptions) -> RegisteredTool { + let default_timeout_ms = options.default_command_timeout_ms; + let max_timeout_ms = options.max_command_timeout_ms; + RegisteredTool { + definition: definition( + NativeTool::Shell, + format!( + "Execute a Bash command in a fresh foreground non-login shell. Use this for \ + searches, git inspection, builds, tests, package managers, and terminal \ + operations. Prefer `rg` for content search and `rg --files` for file discovery. \ + Working-directory and environment changes do not persist between calls. \ + `timeout` is in milliseconds, defaults to {default_timeout_ms}, and is capped at \ + {max_timeout_ms}." + ), + serde_json::json!({ + "type": "object", + "properties": { + "command": { + "type": "string", + "description": "Bash source to evaluate." + }, + "timeout": { + "type": "integer", + "minimum": 0, + "maximum": max_timeout_ms, + "description": format!( + "Maximum runtime in milliseconds (default {default_timeout_ms})." + ) + }, + "description": { + "type": "string", + "description": "Short description of what the command does." + } + }, + "required": ["command"], + "additionalProperties": false + }), + ), + executor: Arc::new(move |args, ctx| { + Box::pin(async move { + let command = tools::required_str(&args, "command")?; + let timeout_ms = args + .get("timeout") + .and_then(Value::as_u64) + .unwrap_or(default_timeout_ms) + .min(max_timeout_ms); + tools::run_shell_command(&ctx, command, timeout_ms, None).await + }) + }), + source: ToolSource::Native, + } +} + +#[must_use] +pub(crate) fn make_web_search_tool(api_key: String) -> RegisteredTool { + let mut tool = tools::make_web_search_tool_with_api_key(api_key); + tool.definition = definition( + NativeTool::WebSearch, + "Search the web when current external information is needed. Returns result titles, URLs, \ + and descriptions; use WebFetch to inspect a specific URL.", + serde_json::json!({ + "type": "object", + "properties": { + "query": { + "type": "string", + "description": "The web search query." + } + }, + "required": ["query"], + "additionalProperties": false + }), + ); + tool +} + +#[must_use] +pub(crate) fn make_web_fetch_tool(summarizer: Option) -> RegisteredTool { + let mut tool = tools::make_web_fetch_tool(summarizer); + tool.definition = definition( + NativeTool::WebFetch, + "Fetch an HTTP or HTTPS URL and answer the supplied prompt from its contents.", + serde_json::json!({ + "type": "object", + "properties": { + "url": { + "type": "string", + "description": "The HTTP or HTTPS URL to fetch." + }, + "prompt": { + "type": "string", + "description": "The question or extraction instruction to apply to the page." + } + }, + "required": ["url", "prompt"], + "additionalProperties": false + }), + ); + tool +} + +fn child_session(session_factory: &SessionFactory, ctx: &ToolContext) -> Session { + let mut session = session_factory(); + if let Some(root) = ctx.root_session_id.as_ref().or(ctx.session_id.as_ref()) { + session.set_root_session_id(root.clone()); + } + session +} + +fn format_agent_result(result: &SubAgentResult) -> String { + format!( + "Agent completed (success: {}, turns: {})\n\n{}", + result.success, result.turns_used, result.output + ) +} + +fn format_error(error: &Error) -> String { + util_error::collect_chain(error).join(": ") +} + +#[must_use] +pub(crate) fn make_agent_tool( + supervisor: SubAgentSupervisor, + session_factory: SessionFactory, + current_depth: usize, +) -> RegisteredTool { + RegisteredTool { + definition: definition( + NativeTool::ClaudeAgent, + "Launch a child agent for an independent task. Agents run in the background by \ + default and notify the parent when they finish. Set run_in_background to false to \ + wait for the result synchronously.", + serde_json::json!({ + "type": "object", + "properties": { + "description": { + "type": "string", + "description": "A short 3-5 word description of the task." + }, + "prompt": { + "type": "string", + "description": "The task for the agent to perform." + }, + "run_in_background": { + "type": "boolean", + "description": "Whether to return immediately (default true)." + } + }, + "required": ["description", "prompt"], + "additionalProperties": false + }), + ), + executor: Arc::new(move |args, ctx| { + let supervisor = supervisor.clone(); + let session_factory = session_factory.clone(); + Box::pin(async move { + let description = tools::required_str(&args, "description")?; + let prompt = tools::required_str(&args, "prompt")?; + let run_in_background = args + .get("run_in_background") + .and_then(Value::as_bool) + .unwrap_or(true); + let session = child_session(&session_factory, &ctx); + + if run_in_background { + let task_id = supervisor + .spawn_with_parent_notification( + session, + prompt.to_string(), + description.to_string(), + current_depth, + ) + .map_err(|error| format_error(&error))?; + Ok(format!( + "Agent started in the background.\n\nTask ID: {task_id}" + )) + } else { + let task_id = supervisor + .spawn(session, prompt.to_string(), current_depth) + .map_err(|error| format_error(&error))?; + match supervisor.wait_with_cancel(&task_id, &ctx.cancel).await { + Ok(result) => Ok(format_agent_result(&result)), + Err(Error::Interrupted(InterruptReason::Cancelled)) => { + Err("Cancelled".to_string()) + } + Err(error) => Err(format_error(&error)), + } + } + }) + }), + source: ToolSource::Native, + } +} + +fn required_bool(args: &Value, key: &str) -> Result { + args.get(key) + .and_then(Value::as_bool) + .ok_or_else(|| format!("Missing required boolean parameter: {key}")) +} + +fn required_u64(args: &Value, key: &str) -> Result { + args.get(key) + .and_then(Value::as_u64) + .ok_or_else(|| format!("Missing required non-negative integer parameter: {key}")) +} + +fn finished_output( + supervisor: &SubAgentSupervisor, + task_id: &str, + result: Result, +) -> Result { + supervisor.suppress_parent_notification(task_id); + match result { + Ok(result) => Ok(format_agent_result(&result)), + Err(error) => Err(format_error(&error)), + } +} + +#[must_use] +pub(crate) fn make_task_output_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { + RegisteredTool { + definition: definition( + NativeTool::TaskOutput, + "Get a background agent's current status or wait for its final output. Automatic \ + completion notifications make ordinary polling unnecessary.", + serde_json::json!({ + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": "The background agent task ID." + }, + "block": { + "type": "boolean", + "default": true, + "description": "Whether to wait for completion." + }, + "timeout": { + "type": "number", + "minimum": 0, + "maximum": 600_000, + "default": 30000, + "description": "Maximum wait time in milliseconds." + } + }, + "required": ["task_id", "block", "timeout"], + "additionalProperties": false + }), + ), + executor: Arc::new(move |args, ctx| { + let supervisor = supervisor.clone(); + Box::pin(async move { + let task_id = tools::required_str(&args, "task_id")?; + let block = required_bool(&args, "block")?; + let timeout_ms = required_u64(&args, "timeout")?; + if timeout_ms > 600_000 { + return Err("timeout must be between 0 and 600000 milliseconds".to_string()); + } + + match supervisor.status(task_id) { + Some(SubAgentStatus::Finished(result)) => { + return finished_output(&supervisor, task_id, result); + } + Some(SubAgentStatus::Running) if !block => { + return Ok(format!("Agent {task_id} is still running.")); + } + Some(SubAgentStatus::Closing | SubAgentStatus::Closed) => { + return Ok(format!("Agent {task_id} has been stopped.")); + } + None => { + return Err(format!( + "No agent found with id: {task_id} (it was never spawned)" + )); + } + Some(SubAgentStatus::Running) => {} + } + + match time::timeout( + Duration::from_millis(timeout_ms), + supervisor.wait_with_cancel(task_id, &ctx.cancel), + ) + .await + { + Ok(Ok(result)) => { + supervisor.suppress_parent_notification(task_id); + Ok(format_agent_result(&result)) + } + Ok(Err(Error::Interrupted(InterruptReason::Cancelled))) => { + supervisor.suppress_parent_notification(task_id); + Err("Cancelled".to_string()) + } + Ok(Err(error)) => { + supervisor.suppress_parent_notification(task_id); + Err(format_error(&error)) + } + Err(_) => Ok(format!( + "Agent {task_id} is still running after waiting {timeout_ms} ms." + )), + } + }) + }), + source: ToolSource::Native, + } +} + +#[must_use] +pub(crate) fn make_task_stop_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { + RegisteredTool { + definition: definition( + NativeTool::TaskStop, + "Stop a running background agent by task ID.", + serde_json::json!({ + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": "The background agent task ID to stop." + } + }, + "required": ["task_id"], + "additionalProperties": false + }), + ), + executor: Arc::new(move |args, _ctx| { + let supervisor = supervisor.clone(); + Box::pin(async move { + let task_id = tools::required_str(&args, "task_id")?; + supervisor + .close_agent(task_id) + .await + .map_err(|error| format_error(&error))?; + Ok(format!("Agent {task_id} stopped.")) + }) + }), + source: ToolSource::Native, + } +} + +#[must_use] +pub(crate) fn make_send_message_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { + RegisteredTool { + definition: definition( + NativeTool::SendMessage, + "Send additional instructions to a running child agent by its Fabro agent ID.", + serde_json::json!({ + "type": "object", + "properties": { + "to": { + "type": "string", + "description": "The running Fabro agent ID." + }, + "message": { + "type": "string", + "description": "The follow-up message." + }, + "summary": { + "type": "string", + "maxLength": 200, + "description": "Optional short preview of the message." + } + }, + "required": ["to", "message"], + "additionalProperties": false + }), + ), + executor: Arc::new(move |args, _ctx| { + let supervisor = supervisor.clone(); + Box::pin(async move { + let recipient = tools::required_str(&args, "to")?; + let message = tools::required_str(&args, "message")?; + supervisor + .send_input(recipient, message) + .map_err(|error| format_error(&error))?; + Ok(format!("Message sent to agent {recipient}.")) + }) + }), + source: ToolSource::Native, + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeSet; + use std::sync::Mutex; + + use serde_json::json; + use tokio_util::sync::CancellationToken; + + use super::*; + use crate::sandbox::Sandbox; + use crate::test_support::{MockSandbox, make_session, text_response}; + use crate::todo_runtime::TodoRuntime; + use crate::todo_tools::{ + make_task_create_tool, make_task_get_tool, make_task_list_tool, make_task_update_tool, + }; + + fn property_names(tool: &RegisteredTool) -> BTreeSet<&str> { + tool.definition.parameters["properties"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect() + } + + fn required_names(tool: &RegisteredTool) -> BTreeSet<&str> { + tool.definition.parameters["required"] + .as_array() + .map(|required| { + required + .iter() + .map(|value| value.as_str().unwrap()) + .collect() + }) + .unwrap_or_default() + } + + fn assert_schema(tool: &RegisteredTool, properties: &[&str], required: &[&str]) { + assert_eq!(tool.definition.parameters["type"], "object"); + assert_eq!( + tool.definition.parameters["additionalProperties"], + Value::Bool(false) + ); + assert_eq!(property_names(tool), properties.iter().copied().collect()); + assert_eq!(required_names(tool), required.iter().copied().collect()); + } + + fn context() -> ToolContext { + ToolContext { + env: Arc::new(MockSandbox::default()) as Arc, + cancel: CancellationToken::new(), + tool_env_provider: None, + session_id: Some("root".to_string()), + root_session_id: Some("root".to_string()), + tool_call_id: Some("call".to_string()), + agent_event_emitter: None, + } + } + + #[test] + fn core_adapter_schemas_match_the_claude5_contract() { + let options = NativeToolOptions::for_profile(fabro_model::AgentProfileKind::Claude5); + assert_schema(&make_read_tool(), &["file_path", "limit", "offset"], &[ + "file_path", + ]); + assert_schema(&make_write_tool(), &["content", "file_path"], &[ + "content", + "file_path", + ]); + assert_schema( + &make_edit_tool(), + &["file_path", "new_string", "old_string", "replace_all"], + &["file_path", "new_string", "old_string"], + ); + let bash = make_bash_tool(&options); + assert_schema(&bash, &["command", "description", "timeout"], &["command"]); + assert_eq!( + bash.definition.parameters["properties"]["timeout"]["maximum"], + 600_000 + ); + assert_schema(&make_web_fetch_tool(None), &["prompt", "url"], &[ + "prompt", "url", + ]); + assert_schema(&make_web_search_tool("key".to_string()), &["query"], &[ + "query", + ]); + + let todo_runtime = Arc::new(TodoRuntime::new()); + assert_schema( + &strict_object_tool(make_task_create_tool(todo_runtime.clone())), + &["activeForm", "description", "metadata", "subject"], + &["description", "subject"], + ); + assert_schema( + &strict_object_tool(make_task_update_tool(todo_runtime.clone())), + &[ + "activeForm", + "addBlockedBy", + "addBlocks", + "description", + "metadata", + "owner", + "status", + "subject", + "taskId", + ], + &["taskId"], + ); + assert_schema( + &strict_object_tool(make_task_get_tool(todo_runtime.clone())), + &["taskId"], + &["taskId"], + ); + assert_schema( + &strict_object_tool(make_task_list_tool(todo_runtime)), + &[], + &[], + ); + } + + #[test] + fn lifecycle_adapter_schemas_match_the_claude5_contract() { + let supervisor = SubAgentSupervisor::new(3); + let factory: SessionFactory = Arc::new(|| panic!("unused")); + assert_schema( + &make_agent_tool(supervisor.clone(), factory, 0), + &["description", "prompt", "run_in_background"], + &["description", "prompt"], + ); + assert_schema( + &make_task_output_tool(supervisor.clone()), + &["block", "task_id", "timeout"], + &["block", "task_id", "timeout"], + ); + assert_schema(&make_task_stop_tool(supervisor.clone()), &["task_id"], &[ + "task_id", + ]); + assert_schema( + &make_send_message_tool(supervisor), + &["message", "summary", "to"], + &["message", "to"], + ); + } + + #[tokio::test] + async fn agent_defaults_to_background_and_produces_parent_notification() { + let supervisor = SubAgentSupervisor::new(3); + let session = make_session(vec![text_response("child report")]).await; + let session_slot = Arc::new(Mutex::new(Some(session))); + let factory_slot = Arc::clone(&session_slot); + let factory: SessionFactory = Arc::new(move || { + factory_slot + .lock() + .unwrap() + .take() + .expect("factory should be called once") + }); + let tool = make_agent_tool(supervisor.clone(), factory, 0); + + let output = (tool.executor)( + json!({ + "description": "Inspect child", + "prompt": "Inspect the child task" + }), + context(), + ) + .await + .unwrap(); + + let task_id = output + .strip_prefix("Agent started in the background.\n\nTask ID: ") + .expect("Agent should return a background task ID"); + let notifications = supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .unwrap(); + assert_eq!(notifications.len(), 1); + assert_eq!(notifications[0].agent_id, task_id); + assert_eq!(notifications[0].description, "Inspect child"); + assert_eq!( + notifications[0].result.as_ref().unwrap().output, + "child report" + ); + + supervisor.shutdown_all().await; + } + + #[tokio::test] + async fn task_output_suppresses_a_racing_automatic_notification() { + let supervisor = SubAgentSupervisor::new(3); + let session = make_session(vec![text_response("explicit report")]).await; + let task_id = supervisor + .spawn_with_parent_notification( + session, + "Inspect".to_string(), + "Inspect explicitly".to_string(), + 0, + ) + .unwrap(); + supervisor + .wait_with_cancel(&task_id, &CancellationToken::new()) + .await + .unwrap(); + + let tool = make_task_output_tool(supervisor.clone()); + let output = (tool.executor)( + json!({ + "task_id": task_id, + "block": false, + "timeout": 0 + }), + context(), + ) + .await + .unwrap(); + + assert!(output.contains("explicit report")); + assert!( + supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .is_none() + ); + + supervisor.shutdown_all().await; + } +} diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index 9defbf54f..f3ff26aa1 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -4,6 +4,8 @@ use std::sync::Arc; use fabro_model::{AgentProfileKind, Catalog, CodecKind, ProviderId}; pub mod anthropic; +pub mod claude5; +pub(crate) mod claude5_tools; pub mod gemini; pub mod gpt56; pub mod kimi; @@ -11,6 +13,7 @@ pub mod kimi_tools; pub mod openai; pub use anthropic::AnthropicProfile; +pub use claude5::Claude5Profile; pub use gemini::GeminiProfile; pub use gpt56::Gpt56Profile; pub use kimi::KimiProfile; @@ -22,6 +25,7 @@ use crate::config::{NativeToolOptions, ToolSecrets}; use crate::native_tool::{NativeTool, ToolVocabulary}; use crate::sandbox::Sandbox; use crate::skills::{Skill, format_skills_prompt_section}; +use crate::todo_runtime::TodoRuntime; use crate::tool_registry::ToolRegistry; use crate::tools::{self, WebFetchSummarizer}; @@ -39,6 +43,7 @@ pub struct AgentProfileBuilder { catalog: Arc, native_tool_options: NativeToolOptions, summarizer: Option, + todo_runtime: Arc, } impl AgentProfileBuilder { @@ -56,6 +61,7 @@ impl AgentProfileBuilder { catalog, native_tool_options: NativeToolOptions::for_profile(profile_kind), summarizer: None, + todo_runtime: Arc::new(TodoRuntime::new()), } } @@ -99,6 +105,16 @@ impl AgentProfileBuilder { .with_provider_id(self.provider_id.clone()) .with_catalog(Arc::clone(&self.catalog)), ), + AgentProfileKind::Claude5 => Box::new( + Claude5Profile::with_native_tools_and_todo_runtime( + model, + options, + summarizer, + Arc::clone(&self.todo_runtime), + ) + .with_provider_id(self.provider_id.clone()) + .with_catalog(Arc::clone(&self.catalog)), + ), AgentProfileKind::Kimi => Box::new( KimiProfile::with_native_tools(model, options, summarizer) .with_provider_id(self.provider_id.clone()) @@ -374,11 +390,13 @@ pub fn build_env_context_block_with(env: &dyn Sandbox, ctx: &EnvContext) -> Stri mod tests { use fabro_llm::types::ToolDefinition; use fabro_model::catalog::LlmCatalogSettings; + use tokio_util::sync::CancellationToken; use super::*; + use crate::question_tools; use crate::subagent::{SessionFactory, SubAgentSupervisor}; use crate::test_support::MockSandbox; - use crate::tools::WEB_SEARCH_TOOL_NAME; + use crate::tool_registry::ToolContext; fn native_tool_options( profile_kind: AgentProfileKind, @@ -411,6 +429,25 @@ mod tests { profile } + fn claude5_profile( + has_web_search: bool, + has_subagents: bool, + has_question: bool, + ) -> Claude5Profile { + let options = native_tool_options(AgentProfileKind::Claude5, has_web_search); + let mut profile = Claude5Profile::with_native_tools("claude-sonnet-5", &options, None); + if has_subagents { + register_test_subagent_tools(&mut profile); + } + if has_question { + question_tools::register_question_tools( + AgentProfileKind::Claude5, + profile.tool_registry_mut(), + ); + } + profile + } + fn gemini_profile(has_web_search: bool) -> GeminiProfile { let options = native_tool_options(AgentProfileKind::Gemini, has_web_search); GeminiProfile::with_native_tools("gemini-3-flash-preview", &options, None) @@ -595,6 +632,11 @@ mod tests { ProviderId::gemini(), "gemini-3-flash-preview", ), + ( + AgentProfileKind::Claude5, + ProviderId::anthropic(), + "claude-sonnet-5", + ), (AgentProfileKind::Gpt56, ProviderId::openai(), "gpt-5.6-sol"), ]; @@ -606,12 +648,13 @@ mod tests { Arc::clone(&catalog), ) .build(); + let web_search_name = NativeTool::WebSearch.name(profile.tool_registry().vocabulary()); assert_eq!(profile.profile_kind(), profile_kind); assert_eq!(profile.provider_id(), provider_id); - assert!(profile.tool_registry().get(WEB_SEARCH_TOOL_NAME).is_none()); + assert!(profile.tool_registry().get(web_search_name).is_none()); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); assert!( - !prompt.contains("web_search"), + !prompt.contains(web_search_name), "{profile_kind:?} prompt advertised an unavailable tool" ); @@ -627,22 +670,79 @@ mod tests { // Built twice: one configured builder must outfit both a root // session and the child sessions it spawns. for configured in [configured_builder.build(), configured_builder.build()] { - assert!( - configured - .tool_registry() - .get(WEB_SEARCH_TOOL_NAME) - .is_some() - ); + assert!(configured.tool_registry().get(web_search_name).is_some()); let prompt = configured.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); assert!( - prompt.contains("web_search"), + prompt.contains(web_search_name), "{profile_kind:?} prompt omitted guidance for an available tool" ); } } } + #[tokio::test] + async fn claude5_builder_shares_tasks_across_root_and_child_profiles() { + let builder = AgentProfileBuilder::new( + AgentProfileKind::Claude5, + ProviderId::anthropic(), + "claude-sonnet-5", + Arc::new(Catalog::from_builtin().unwrap()), + ); + let root = builder.build(); + let child = builder.build(); + let root_create = Arc::clone( + &root + .tool_registry() + .get("TaskCreate") + .expect("root should expose TaskCreate") + .executor, + ); + let child_create = Arc::clone( + &child + .tool_registry() + .get("TaskCreate") + .expect("child should expose TaskCreate") + .executor, + ); + let child_list = Arc::clone( + &child + .tool_registry() + .get("TaskList") + .expect("child should expose TaskList") + .executor, + ); + let env: Arc = Arc::new(MockSandbox::default()); + let context = |session_id: &str| ToolContext { + env: Arc::clone(&env), + cancel: CancellationToken::new(), + tool_env_provider: None, + session_id: Some(session_id.to_string()), + root_session_id: Some("root-session".to_string()), + tool_call_id: None, + agent_event_emitter: None, + }; + + root_create( + serde_json::json!({"subject": "Parent task", "description": "Root work"}), + context("root-session"), + ) + .await + .unwrap(); + child_create( + serde_json::json!({"subject": "Child task", "description": "Child work"}), + context("child-session"), + ) + .await + .unwrap(); + let tasks = child_list(serde_json::json!({}), context("child-session")) + .await + .unwrap(); + + assert!(tasks.contains("#1 [pending] Parent task"), "{tasks}"); + assert!(tasks.contains("#2 [pending] Child task"), "{tasks}"); + } + #[test] fn profile_builder_selects_a_codec_compatible_gpt56_editor() { let overrides: LlmCatalogSettings = @@ -680,6 +780,46 @@ mod tests { insta::assert_snapshot!(system_prompt(&anthropic_profile(true, true))); } + #[test] + fn claude5_default_prompt_snapshot() { + 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))); + } + #[test] fn gemini_default_prompt_snapshot() { insta::assert_snapshot!(system_prompt(&gemini_profile(false))); diff --git a/lib/components/fabro-agent/src/profiles/prompts/claude5.md.j2 b/lib/components/fabro-agent/src/profiles/prompts/claude5.md.j2 new file mode 100644 index 000000000..143be4f55 --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/prompts/claude5.md.j2 @@ -0,0 +1,80 @@ +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. + +{{ inputs.env_block }} + +# 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. +{% if inputs.has_web_search %} +Use `WebSearch` when current external information is needed. Its input is a search query; use `WebFetch` to inspect a specific result. +{% endif %} + +Other registered tools may be supplied by MCP servers or the surrounding Fabro workflow. Follow their definitions. + +{% if inputs.has_agent %} +# 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. +{% endif %} + +{% if inputs.has_ask_user_question %} +# 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. +{% endif %} + +# 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_all_conditionals_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_all_conditionals_prompt_snapshot.snap new file mode 100644 index 000000000..8d4a27c3f --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_all_conditionals_prompt_snapshot.snap @@ -0,0 +1,89 @@ +--- +source: lib/components/fabro-agent/src/profiles/mod.rs +expression: "system_prompt(&claude5_profile(true, 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. + +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. + + + +# 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_default_prompt_snapshot.snap b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_default_prompt_snapshot.snap new file mode 100644 index 000000000..2152f374d --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_default_prompt_snapshot.snap @@ -0,0 +1,71 @@ +--- +source: lib/components/fabro-agent/src/profiles/mod.rs +expression: "system_prompt(&claude5_profile(false, 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. + + +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/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 new file mode 100644 index 000000000..c1628ad2a --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_question_prompt_snapshot.snap @@ -0,0 +1,77 @@ +--- +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 new file mode 100644 index 000000000..95d827e7a --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_and_question_prompt_snapshot.snap @@ -0,0 +1,87 @@ +--- +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 new file mode 100644 index 000000000..eaaaf1cbb --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_subagents_prompt_snapshot.snap @@ -0,0 +1,81 @@ +--- +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 new file mode 100644 index 000000000..041ac14ff --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_question_prompt_snapshot.snap @@ -0,0 +1,79 @@ +--- +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 new file mode 100644 index 000000000..d67ec831e --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_and_subagents_prompt_snapshot.snap @@ -0,0 +1,83 @@ +--- +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 new file mode 100644 index 000000000..b92b72e73 --- /dev/null +++ b/lib/components/fabro-agent/src/profiles/snapshots/fabro_agent__profiles__tests__claude5_web_search_prompt_snapshot.snap @@ -0,0 +1,73 @@ +--- +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/question_tools.rs b/lib/components/fabro-agent/src/question_tools.rs index d4ff2f274..3fcc2980f 100644 --- a/lib/components/fabro-agent/src/question_tools.rs +++ b/lib/components/fabro-agent/src/question_tools.rs @@ -149,6 +149,28 @@ struct AnthropicOption { preview: Option, } +#[derive(Debug, Deserialize)] +struct Claude5QuestionToolArgs { + questions: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct Claude5Question { + question: String, + header: String, + options: Vec, + multi_select: bool, +} + +#[derive(Debug, Deserialize)] +struct Claude5Option { + label: String, + description: String, + #[serde(default)] + preview: Option, +} + #[must_use] pub fn is_question_tool(name: &str) -> bool { matches!( @@ -168,6 +190,9 @@ pub fn register_question_tools(profile_kind: AgentProfileKind, registry: &mut To AgentProfileKind::Anthropic | AgentProfileKind::Kimi => { registry.register(make_anthropic_question_tool()); } + AgentProfileKind::Claude5 => { + registry.register(make_claude5_question_tool()); + } AgentProfileKind::Gemini => {} } } @@ -269,6 +294,82 @@ fn make_anthropic_question_tool() -> RegisteredTool { } } +fn make_claude5_question_tool() -> RegisteredTool { + RegisteredTool { + definition: ToolDefinition { + name: ANTHROPIC_ASK_USER_QUESTION_TOOL.to_string(), + description: "Ask the human up to four questions when a decision is genuinely theirs to make. The UI automatically provides an Other option for custom text.".to_string(), + parameters: json!({ + "type": "object", + "properties": { + "questions": { + "description": "Questions to ask the user (1-4 questions)", + "type": "array", + "minItems": 1, + "maxItems": 4, + "items": { + "type": "object", + "properties": { + "question": { + "description": "The complete, clear, and specific question to ask.", + "type": "string" + }, + "header": { + "description": "Very short label displayed as a chip/tag (max 12 chars).", + "type": "string" + }, + "options": { + "description": "Two to four choices. Do not include Other; the UI adds it automatically.", + "type": "array", + "minItems": 2, + "maxItems": 4, + "items": { + "type": "object", + "properties": { + "label": { + "description": "Concise display text for the option.", + "type": "string" + }, + "description": { + "description": "What the option means and its relevant trade-offs.", + "type": "string" + }, + "preview": { + "description": "Optional Markdown preview for single-select visual comparisons.", + "type": "string" + } + }, + "required": ["label", "description"], + "additionalProperties": false + } + }, + "multiSelect": { + "description": "Whether the user may select multiple options.", + "default": false, + "type": "boolean" + } + }, + "required": ["question", "header", "options", "multiSelect"], + "additionalProperties": false + } + } + }, + "required": ["questions"], + "additionalProperties": false + }), + }, + executor: Arc::new(|args, ctx| { + Box::pin(async move { + let parsed: Claude5QuestionToolArgs = parse_tool_args(args)?; + let questions = normalize_claude5_questions(parsed)?; + let answers = execute_question_tool(ctx, questions).await?; + format_anthropic_answers(&answers) + }) + }), + source: ToolSource::Native, + } +} + fn parse_tool_args Deserialize<'de>>(args: serde_json::Value) -> Result { serde_json::from_value(args).map_err(|err| format!("invalid question tool arguments: {err}")) } @@ -350,6 +451,71 @@ fn normalize_anthropic_questions( .collect() } +fn normalize_claude5_questions( + args: Claude5QuestionToolArgs, +) -> Result, String> { + if !(1..=4).contains(&args.questions.len()) { + return Err("questions must contain between one and four questions".to_string()); + } + + args.questions + .into_iter() + .map(|question| { + let original_question = non_empty(&question.question, "question")?; + let header = non_empty(&question.header, "question header")?; + if header.chars().count() > 12 { + return Err("question header must contain at most 12 characters".to_string()); + } + if !(2..=4).contains(&question.options.len()) { + return Err("each question must contain between two and four options".to_string()); + } + if question.multi_select + && question + .options + .iter() + .any(|option| option.preview.is_some()) + { + return Err( + "option previews are not supported for multi-select questions".to_string(), + ); + } + + let options = question + .options + .into_iter() + .enumerate() + .map(|(idx, option)| { + Ok(InterviewOption { + key: option_key(idx), + label: non_empty(&option.label, "option label")?, + description: Some(bounded_display_field( + &non_empty(&option.description, "option description")?, + OPTION_DESCRIPTION_MAX_CHARS, + )), + preview: option + .preview + .map(|value| bounded_display_field(&value, OPTION_PREVIEW_MAX_CHARS)), + }) + }) + .collect::, String>>()?; + + Ok(AgentQuestion { + original_id: None, + text: display_text(Some(&header), &original_question), + header: Some(header), + original_question, + question_type: if question.multi_select { + QuestionType::MultiSelect + } else { + QuestionType::MultipleChoice + }, + options, + allow_freeform: true, + }) + }) + .collect() +} + fn options_from_openai(options: Vec) -> Vec { options .into_iter() @@ -472,6 +638,7 @@ fn format_anthropic_answers(answers: &[AgentQuestionAnswer]) -> Result, @@ -601,8 +768,121 @@ mod tests { assert!(kimi.get(ANTHROPIC_ASK_USER_QUESTION_TOOL).is_some()); assert!(kimi.get(OPENAI_REQUEST_USER_INPUT_TOOL).is_none()); + let mut claude5 = ToolRegistry::with_vocabulary(ToolVocabulary::Claude5); + register_question_tools(AgentProfileKind::Claude5, &mut claude5); + let tool = claude5.get(ANTHROPIC_ASK_USER_QUESTION_TOOL).unwrap(); + assert_eq!(tool.definition.parameters["additionalProperties"], false); + assert_eq!( + tool.definition.parameters["properties"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect::>(), + vec!["questions"] + ); + assert_eq!( + tool.definition.parameters["properties"]["questions"]["maxItems"], + 4 + ); + assert!(claude5.get(OPENAI_REQUEST_USER_INPUT_TOOL).is_none()); + let mut gemini = ToolRegistry::new(); register_question_tools(AgentProfileKind::Gemini, &mut gemini); assert!(gemini.names().is_empty()); } + + #[test] + fn claude5_question_contract_is_strict_and_preserves_preview() { + let args: Claude5QuestionToolArgs = serde_json::from_value(json!({ + "questions": [{ + "header": "Approach", + "question": "Which approach should we use?", + "multiSelect": false, + "options": [ + { + "label": "Simple", + "description": "Use the smallest implementation.", + "preview": "fn simple() {}" + }, + { + "label": "Flexible", + "description": "Allow future extension." + } + ] + }] + })) + .unwrap(); + + let questions = normalize_claude5_questions(args).unwrap(); + + assert_eq!(questions[0].header.as_deref(), Some("Approach")); + assert_eq!( + questions[0].options[0].preview.as_deref(), + Some("fn simple() {}") + ); + assert!(questions[0].allow_freeform); + } + + #[test] + fn claude5_rejects_previews_for_multi_select_questions() { + let args: Claude5QuestionToolArgs = serde_json::from_value(json!({ + "questions": [{ + "header": "Features", + "question": "Which features should we enable?", + "multiSelect": true, + "options": [ + { + "label": "Auth", + "description": "Enable authentication.", + "preview": "auth = true" + }, + { + "label": "Metrics", + "description": "Enable metrics." + } + ] + }] + })) + .unwrap(); + + assert!(normalize_claude5_questions(args).is_err()); + } + + #[tokio::test] + async fn claude5_question_tool_rejects_subagent_sessions() { + let tool = make_claude5_question_tool(); + let error = (tool.executor)( + json!({ + "questions": [{ + "header": "Approach", + "question": "Which approach?", + "multiSelect": false, + "options": [ + { + "label": "Simple", + "description": "Use the simple approach." + }, + { + "label": "Flexible", + "description": "Use the flexible approach." + } + ] + }] + }), + ToolContext { + env: Arc::new(MockSandbox::default()), + cancel: CancellationToken::new(), + tool_env_provider: None, + session_id: Some("child".to_string()), + root_session_id: Some("root".to_string()), + tool_call_id: Some("call".to_string()), + agent_event_emitter: None, + }, + ) + .await + .unwrap_err(); + + assert!(error.contains("only available to the root agent")); + } } diff --git a/lib/components/fabro-agent/src/session.rs b/lib/components/fabro-agent/src/session.rs index 4914b5638..f04e99257 100644 --- a/lib/components/fabro-agent/src/session.rs +++ b/lib/components/fabro-agent/src/session.rs @@ -47,7 +47,10 @@ use crate::skills::{ ExpandedInput, Skill, default_skill_dirs, discover_skills, expand_skill, make_use_skill_tool_for_vocabulary, }; -use crate::subagent::{SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor}; +use crate::subagent::{ + SubAgentCallbackEvent, SubAgentEventCallback, SubAgentSupervisor, + format_parent_notification_batch, +}; use crate::tool_execution::execute_tool_calls; use crate::tool_permissions::canonical_tool_name; use crate::tool_registry::ToolDefinitionWithSource; @@ -1318,7 +1321,10 @@ impl Session { }) }); - // Process the initial input, then drain any followups + // Process the initial input, then drain followups. Claude-compatible + // background-agent results join this same boundary queue: they never + // interrupt inference or a tool call, and all results already ready at + // a boundary are delivered in one additional parent turn. let mut result = self .run_single_input( input, @@ -1336,10 +1342,33 @@ impl Session { .lock() .expect("followup queue lock poisoned") .pop_front(); - let Some(followup) = followup else { break }; + let next_input = if let Some(followup) = followup { + Some(followup) + } else if let Some(supervisor) = self.subagent_supervisor.clone() { + match supervisor + .next_parent_notification_batch(&self.cancel_token) + .await + { + Ok(Some(notifications)) => { + Some(format_parent_notification_batch(¬ifications)) + } + Ok(None) => None, + Err(Error::Interrupted(InterruptReason::Cancelled)) => { + result = Err(self.interrupted_error()); + None + } + Err(error) => { + result = Err(error); + None + } + } + } else { + None + }; + let Some(next_input) = next_input else { break }; result = self .run_single_input( - &followup, + &next_input, &agent_tool_runtime, &mut timing, &mut usage, @@ -3040,6 +3069,70 @@ mod tests { ); } + #[tokio::test] + async fn background_agent_notifications_are_batched_into_one_parent_turn() { + let supervisor = SubAgentSupervisor::new(3); + let first = make_session(vec![text_response("first result")]).await; + let second = make_session(vec![text_response("second result")]).await; + let first_id = supervisor + .spawn_with_parent_notification( + first, + "first task".to_string(), + "Inspect first".to_string(), + 0, + ) + .unwrap(); + let second_id = supervisor + .spawn_with_parent_notification( + second, + "second task".to_string(), + "Inspect second".to_string(), + 0, + ) + .unwrap(); + + // Make both results ready before the parent reaches its safe turn + // boundary so batching is deterministic. + supervisor + .wait_with_cancel(&first_id, &CancellationToken::new()) + .await + .unwrap(); + supervisor + .wait_with_cancel(&second_id, &CancellationToken::new()) + .await + .unwrap(); + + let provider = Arc::new(ScriptedStreamProvider::new(vec![ + ScriptedStreamCall::Response(Box::new(text_response("Parent is waiting"))), + ScriptedStreamCall::Response(Box::new(text_response("Synthesized both results"))), + ])); + let mut parent = + make_session_with_provider_and_manager(provider, Some(supervisor.clone())).await; + + let output = parent + .process_input_with_output("Delegate both tasks") + .await + .unwrap(); + + assert_eq!(output.as_deref(), Some("Synthesized both results")); + let turns = parent.history().turns(); + assert_eq!(turns.len(), 4); + let Message::User { + content: notification, + .. + } = &turns[2] + else { + panic!("third turn should deliver the background results"); + }; + assert_eq!(notification.matches("").count(), 2); + assert!(notification.contains(&first_id)); + assert!(notification.contains(&second_id)); + assert!(notification.contains("first result")); + assert!(notification.contains("second result")); + + 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/skills.rs b/lib/components/fabro-agent/src/skills.rs index ecd08e9cb..f1aff7031 100644 --- a/lib/components/fabro-agent/src/skills.rs +++ b/lib/components/fabro-agent/src/skills.rs @@ -189,6 +189,24 @@ pub fn make_use_skill_tool_for_vocabulary( "required": ["skill_name"] }), ), + ToolVocabulary::Claude5 => ( + "skill", + serde_json::json!({ + "type": "object", + "properties": { + "skill": { + "type": "string", + "description": "Exact name of the skill to invoke" + }, + "args": { + "type": "string", + "description": "Optional argument string to pass to the skill" + } + }, + "required": ["skill"], + "additionalProperties": false + }), + ), ToolVocabulary::KimiCode => ( "skill", serde_json::json!({ @@ -730,4 +748,46 @@ name: trimmed .is_none() ); } + + #[tokio::test] + async fn claude5_skill_schema_uses_skill_and_optional_args() { + let skills = Arc::new(test_skills()); + let tool = make_use_skill_tool_for_vocabulary(skills, ToolVocabulary::Claude5); + let result = (tool.executor)( + serde_json::json!({"skill": "commit", "args": "only staged files"}), + ToolContext { + env: Arc::new(MockSandbox::default()), + cancel: CancellationToken::new(), + tool_env_provider: None, + session_id: None, + root_session_id: None, + tool_call_id: None, + agent_event_emitter: None, + }, + ) + .await + .unwrap(); + + assert!(result.contains("only staged files"), "{result}"); + assert_eq!( + tool.definition.parameters["required"], + serde_json::json!(["skill"]) + ); + assert_eq!(tool.definition.parameters["additionalProperties"], false); + assert!( + tool.definition.parameters["properties"] + .get("skill") + .is_some() + ); + assert!( + tool.definition.parameters["properties"] + .get("args") + .is_some() + ); + assert!( + tool.definition.parameters["properties"] + .get("skill_name") + .is_none() + ); + } } diff --git a/lib/components/fabro-agent/src/subagent.rs b/lib/components/fabro-agent/src/subagent.rs index 3cb2af0da..0d91d58e8 100644 --- a/lib/components/fabro-agent/src/subagent.rs +++ b/lib/components/fabro-agent/src/subagent.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex, RwLock}; use std::time::Duration; use fabro_llm::types::ToolDefinition; +use fabro_util::error as util_error; use futures::future; use tokio::sync::{oneshot, watch}; use tokio::task::{AbortHandle, JoinHandle}; @@ -32,6 +33,53 @@ pub struct SubAgentResult { pub turns_used: usize, } +/// A terminal background-agent result waiting to be delivered to its parent at +/// a safe turn boundary. +#[derive(Debug, Clone)] +pub(crate) struct SubAgentParentNotification { + pub agent_id: String, + pub description: String, + pub result: Result, +} + +pub(crate) fn format_parent_notification_batch( + notifications: &[SubAgentParentNotification], +) -> String { + notifications + .iter() + .map(|notification| { + let (status, result) = match ¬ification.result { + Ok(result) if result.success => ("completed", result.output.clone()), + Ok(result) => ("failed", result.output.clone()), + Err(error) => ("failed", util_error::collect_chain(error).join(": ")), + }; + format!( + "\n {}\n {status}\n \ + {}\n {}\n", + escape_notification_xml(¬ification.agent_id), + escape_notification_xml(¬ification.description), + escape_notification_xml(&result), + ) + }) + .collect::>() + .join("\n\n") +} + +fn escape_notification_xml(value: &str) -> String { + let mut escaped = String::with_capacity(value.len()); + for character in value.chars() { + match character { + '&' => escaped.push_str("&"), + '<' => escaped.push_str("<"), + '>' => escaped.push_str(">"), + '"' => escaped.push_str("""), + '\'' => escaped.push_str("'"), + _ => escaped.push(character), + } + } + escaped +} + #[derive(Debug, Clone)] pub enum SubAgentStatus { Running, @@ -76,6 +124,113 @@ struct SupervisorState { agents: HashMap, } +#[derive(Default)] +struct ParentNotificationState { + pending: HashMap, + ready: VecDeque, +} + +struct ParentNotificationHub { + state: Mutex, + changed: watch::Sender, +} + +impl ParentNotificationHub { + fn new() -> Self { + let (changed, _) = watch::channel(0); + Self { + state: Mutex::new(ParentNotificationState::default()), + changed, + } + } + + fn register(&self, agent_id: String, description: String) { + self.state + .lock() + .expect("parent notification lock poisoned") + .pending + .insert(agent_id, description); + self.signal(); + } + + fn complete(&self, agent_id: &str, result: Result) { + { + let mut state = self + .state + .lock() + .expect("parent notification lock poisoned"); + let Some(description) = state.pending.remove(agent_id) else { + return; + }; + state.ready.push_back(SubAgentParentNotification { + agent_id: agent_id.to_string(), + description, + result, + }); + } + self.signal(); + } + + fn suppress(&self, agent_id: &str) { + let changed = { + let mut state = self + .state + .lock() + .expect("parent notification lock poisoned"); + let removed_pending = state.pending.remove(agent_id).is_some(); + let ready_len = state.ready.len(); + state + .ready + .retain(|notification| notification.agent_id != agent_id); + removed_pending || state.ready.len() != ready_len + }; + if changed { + self.signal(); + } + } + + async fn next_batch( + &self, + cancel: &CancellationToken, + ) -> Result>, Error> { + let mut changed = self.changed.subscribe(); + loop { + { + let mut state = self + .state + .lock() + .expect("parent notification lock poisoned"); + if !state.ready.is_empty() { + return Ok(Some(state.ready.drain(..).collect())); + } + if state.pending.is_empty() { + return Ok(None); + } + } + + tokio::select! { + biased; + () = cancel.cancelled() => { + return Err(Error::Interrupted(InterruptReason::Cancelled)); + } + observed = changed.changed() => { + observed.map_err(|_| { + Error::InvalidState( + "Background-agent notification observer closed unexpectedly".to_string(), + ) + })?; + } + } + } + } + + fn signal(&self) { + self.changed.send_modify(|generation| { + *generation = generation.wrapping_add(1); + }); + } +} + struct ShutdownWork { agent_id: String, depth: usize, @@ -119,6 +274,7 @@ fn spawn_result_monitor( child_task: JoinHandle>, status: watch::Sender, event_callback: Arc>>, + parent_notifications: Arc, agent_id: String, depth: usize, ) -> JoinHandle<()> { @@ -141,17 +297,17 @@ fn spawn_result_monitor( return; } - let event = match task_result { + let event = match &task_result { Ok(result) => AgentEvent::SubAgentCompleted { - agent_id, + agent_id: agent_id.clone(), depth, success: result.success, turns_used: result.turns_used, }, Err(error) => AgentEvent::SubAgentFailed { - agent_id, + agent_id: agent_id.clone(), depth, - error, + error: error.clone(), }, }; let callback = event_callback @@ -161,6 +317,7 @@ fn spawn_result_monitor( if let Some(callback) = callback { callback(SubAgentCallbackEvent::Lifecycle(event)); } + parent_notifications.complete(&agent_id, task_result); }) } @@ -171,9 +328,10 @@ fn spawn_result_monitor( /// happen after the guard has been released. #[derive(Clone)] pub struct SubAgentSupervisor { - state: Arc>, - max_depth: usize, - event_callback: Arc>>, + state: Arc>, + max_depth: usize, + event_callback: Arc>>, + parent_notifications: Arc, } impl SubAgentSupervisor { @@ -183,6 +341,7 @@ impl SubAgentSupervisor { state: Arc::new(Mutex::new(SupervisorState::default())), max_depth, event_callback: Arc::new(RwLock::new(None)), + parent_notifications: Arc::new(ParentNotificationHub::new()), } } @@ -205,10 +364,32 @@ impl SubAgentSupervisor { } pub fn spawn( + &self, + session: Session, + task_prompt: String, + depth: usize, + ) -> Result { + self.spawn_inner(session, task_prompt, depth, None) + } + + /// Spawn a child whose terminal result should automatically be delivered + /// to the parent session. + pub(crate) fn spawn_with_parent_notification( + &self, + session: Session, + task_prompt: String, + description: String, + depth: usize, + ) -> Result { + self.spawn_inner(session, task_prompt, depth, Some(description)) + } + + fn spawn_inner( &self, mut session: Session, task_prompt: String, depth: usize, + parent_notification_description: Option, ) -> Result { if depth >= self.max_depth { return Err(Error::InvalidState(format!( @@ -295,6 +476,7 @@ impl SubAgentSupervisor { child_task, status.clone(), Arc::clone(&self.event_callback), + Arc::clone(&self.parent_notifications), agent_id.clone(), child_depth, ); @@ -314,6 +496,10 @@ 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(), @@ -397,6 +583,22 @@ impl SubAgentSupervisor { } } + /// Stop automatic delivery for an agent whose result the parent explicitly + /// retrieved. Removes a result that may already have raced into the ready + /// queue. + pub(crate) fn suppress_parent_notification(&self, agent_id: &str) { + self.parent_notifications.suppress(agent_id); + } + + /// Wait until all currently-ready background results can be delivered in + /// one parent turn, or return `None` once no notifiable agents remain. + pub(crate) async fn next_parent_notification_batch( + &self, + cancel: &CancellationToken, + ) -> Result>, Error> { + self.parent_notifications.next_batch(cancel).await + } + #[cfg(test)] async fn wait(&self, agent_id: &str) -> Result { self.wait_with_cancel(agent_id, &CancellationToken::new()) @@ -404,6 +606,7 @@ 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!( @@ -628,6 +831,7 @@ impl SubAgentSupervisor { child_task, status.clone(), Arc::clone(&self.event_callback), + Arc::clone(&self.parent_notifications), agent_id.clone(), depth, ); @@ -842,6 +1046,69 @@ mod tests { assert!(manager.is_empty()); } + #[tokio::test] + async fn parent_notifications_are_exactly_once_and_xml_escaped() { + let hub = ParentNotificationHub::new(); + hub.register("agent<&".to_string(), "Review & tests".to_string()); + let result = Ok(SubAgentResult { + output: "done & \"verified\"".to_string(), + success: true, + turns_used: 2, + }); + hub.complete("agent<&", result.clone()); + hub.complete("agent<&", result); + + let notifications = hub + .next_batch(&CancellationToken::new()) + .await + .unwrap() + .unwrap(); + assert_eq!(notifications.len(), 1); + let envelope = format_parent_notification_batch(¬ifications); + assert!(envelope.contains("completed")); + assert!(envelope.contains("agent<&")); + assert!(envelope.contains("Review <core> & tests")); + assert!( + envelope.contains("done <safely> & "verified"") + ); + assert!( + hub.next_batch(&CancellationToken::new()) + .await + .unwrap() + .is_none() + ); + } + + #[tokio::test] + async fn suppress_removes_pending_and_ready_parent_notifications() { + let hub = ParentNotificationHub::new(); + hub.register("pending".to_string(), "Pending".to_string()); + hub.suppress("pending"); + assert!( + hub.next_batch(&CancellationToken::new()) + .await + .unwrap() + .is_none() + ); + + hub.register("ready".to_string(), "Ready".to_string()); + hub.complete( + "ready", + Ok(SubAgentResult { + output: "done".to_string(), + success: true, + turns_used: 1, + }), + ); + hub.suppress("ready"); + assert!( + hub.next_batch(&CancellationToken::new()) + .await + .unwrap() + .is_none() + ); + } + #[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 760f2eb20..0d1d79e11 100644 --- a/lib/components/fabro-agent/src/todo_runtime.rs +++ b/lib/components/fabro-agent/src/todo_runtime.rs @@ -21,17 +21,33 @@ use crate::types::AgentEvent; /// `Arc` into each tool closure that needs it. #[derive(Debug, Default)] pub struct TodoRuntime { - lists: Mutex>, + lists: Mutex>, + task_counters: Mutex>, } impl TodoRuntime { #[must_use] pub fn new() -> Self { Self { - lists: Mutex::new(BTreeMap::new()), + lists: Mutex::new(BTreeMap::new()), + task_counters: Mutex::new(BTreeMap::new()), } } + /// Allocate the next monotonically increasing Claude task ID for a list. + /// + /// 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(); + *counter = counter.saturating_add(1); + *counter + } + /// Snapshot the projection for `list_id`. Used by tests and by the /// list-style tools that need a stable view. #[must_use] diff --git a/lib/components/fabro-agent/src/todo_tools.rs b/lib/components/fabro-agent/src/todo_tools.rs index 764bf8bbc..2eb136ed8 100644 --- a/lib/components/fabro-agent/src/todo_tools.rs +++ b/lib/components/fabro-agent/src/todo_tools.rs @@ -10,8 +10,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::Write; use std::str::FromStr; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use fabro_llm::types::ToolDefinition; use fabro_types::{TodoListKind, TodoProjection, TodoStatus, TodoUpdatedProps}; @@ -395,28 +394,6 @@ pub fn make_todo_list_tool(runtime: Arc) -> RegisteredTool { } } -/// Per-list monotonically-increasing task counter for Anthropic -/// `TaskCreate`. Shared state lives inside the tool closure so two parallel -/// `TaskCreate` calls inside one session can never receive the same ID. -#[derive(Debug, Default)] -struct AnthropicTaskCounters { - counters: Mutex>>, -} - -impl AnthropicTaskCounters { - fn next(&self, list_id: &str) -> u64 { - let counter = { - let mut guard = self.counters.lock().expect("task counter lock poisoned"); - Arc::clone( - guard - .entry(list_id.to_string()) - .or_insert_with(|| Arc::new(AtomicU64::new(0))), - ) - }; - counter.fetch_add(1, Ordering::Relaxed) + 1 - } -} - fn optional_string(args: &Value, key: &str) -> Option { args.get(key) .and_then(Value::as_str) @@ -471,7 +448,6 @@ fn format_task_details(todo: &TodoProjection) -> String { #[must_use] pub fn make_task_create_tool(runtime: Arc) -> RegisteredTool { - let counters = Arc::new(AnthropicTaskCounters::default()); RegisteredTool { definition: ToolDefinition { name: "TaskCreate".into(), @@ -489,7 +465,6 @@ pub fn make_task_create_tool(runtime: Arc) -> RegisteredTool { }, executor: Arc::new(move |args, ctx| { let runtime = runtime.clone(); - let counters = counters.clone(); Box::pin(async move { let list_id = anthropic_task_scope(&ctx)?; let subject = args @@ -502,7 +477,7 @@ pub fn make_task_create_tool(runtime: Arc) -> RegisteredTool { .and_then(Value::as_str) .ok_or_else(|| "Missing required parameter: description".to_string())? .to_string(); - let task_id = counters.next(&list_id); + let task_id = runtime.next_task_id(&list_id); let id_string = task_id.to_string(); let order = u32::try_from(task_id.saturating_sub(1)).unwrap_or(u32::MAX); diff --git a/lib/components/fabro-agent/src/tools.rs b/lib/components/fabro-agent/src/tools.rs index 82deb68e7..a36410b33 100644 --- a/lib/components/fabro-agent/src/tools.rs +++ b/lib/components/fabro-agent/src/tools.rs @@ -656,7 +656,7 @@ fn format_brave_results(body: &serde_json::Value) -> String { output } -fn make_web_search_tool_with_api_key(api_key: String) -> RegisteredTool { +pub(crate) fn make_web_search_tool_with_api_key(api_key: String) -> RegisteredTool { use std::sync::OnceLock; static CLIENT: OnceLock = OnceLock::new(); diff --git a/lib/components/fabro-llm/src/adapter_registry.rs b/lib/components/fabro-llm/src/adapter_registry.rs index 822626f34..26ff6d70b 100644 --- a/lib/components/fabro-llm/src/adapter_registry.rs +++ b/lib/components/fabro-llm/src/adapter_registry.rs @@ -294,14 +294,15 @@ mod tests { #[rustfmt::skip] let expected: &[RouteRow] = &[ // model id deployment_id transport codec billing profile - ("claude-fable-5", "claude-fable-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), + ("claude-fable-5", "claude-fable-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Claude5), ("claude-haiku-4-5", "claude-haiku-4-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-opus-4-6", "claude-opus-4-6", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-opus-4-7", "claude-opus-4-7", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-opus-4-8", "claude-opus-4-8", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), - ("claude-opus-5", "claude-opus-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), + ("claude-opus-5", "claude-opus-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Claude5), ("claude-sonnet-4-5", "claude-sonnet-4-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-sonnet-4-6", "claude-sonnet-4-6", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), + ("claude-sonnet-5", "claude-sonnet-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Claude5), ("gemini-3-flash-preview", "gemini-3-flash-preview", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), ("gemini-3.1-flash-lite", "gemini-3.1-flash-lite", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), ("gemini-3.1-pro-preview", "gemini-3.1-pro-preview", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), @@ -360,7 +361,7 @@ mod tests { let by_alias = resolve_route(catalog, select_from_all(catalog, "sonnet")) .expect("alias should resolve"); - let by_id = resolve_route(catalog, select_from_all(catalog, "claude-sonnet-4-6")) + let by_id = resolve_route(catalog, select_from_all(catalog, "claude-sonnet-5")) .expect("id should resolve"); assert_eq!(by_alias, by_id); diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index c4ce3737e..3441aa89b 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -8,7 +8,7 @@ use fabro_agent::tool_registry::{RegisteredTool, ToolContext, ToolRegistry, Tool use fabro_agent::{ AgentEvent, AgentProfile, AgentProfileBuilder, CompletionCoordinator, Message as AgentMessage, Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider, - ToolSecrets, canonical_tool_name, register_question_tools, + ToolSecrets, WebFetchSummarizer, canonical_tool_name, register_question_tools, }; use fabro_auth::{CredentialSource, EnvCredentialSource}; use fabro_graphviz::graph::{AttrValue, Node}; @@ -19,10 +19,10 @@ use fabro_llm::types::{ }; use fabro_mcp::config::McpServerSettings; #[cfg(test)] -use fabro_model::AgentProfileKind; -#[cfg(test)] use fabro_model::catalog::LlmCatalogSettings; -use fabro_model::{Catalog, FallbackTarget, ModelRef, ProviderId, UsdMicros}; +use fabro_model::{ + AgentProfileKind, Catalog, FallbackTarget, ModelHandle, ModelRef, ProviderId, UsdMicros, +}; use fabro_types::settings::run::RunModelControls; use fabro_types::{PermissionLevel, RunId, SessionCapability, StageId, StageTiming}; use serde::de::DeserializeOwned; @@ -823,6 +823,17 @@ impl AgentApiBackend { Arc::clone(&catalog), ) .with_tool_secrets(tool_secrets); + let profile_builder = if provider.profile_kind == AgentProfileKind::Claude5 { + profile_builder.with_web_fetch_summarizer(Some(WebFetchSummarizer { + client: client.clone(), + model_id: ModelHandle::ByName { + provider: provider.provider_id.clone(), + model: model.to_string(), + }, + })) + } else { + profile_builder + }; let mut profile = profile_builder.build(); let config = SessionOptions { @@ -2843,6 +2854,25 @@ reasoning = false assert_eq!(provider.profile_kind, AgentProfileKind::Anthropic); } + #[test] + fn api_backend_selects_claude5_profile_for_sonnet5() { + let backend = AgentApiBackend::new_with_catalog( + "claude-sonnet-5".to_string(), + ProviderId::anthropic(), + Vec::new(), + Arc::new(EnvCredentialSource::new()), + SteeringHub::for_tests(), + Arc::new(Catalog::from_builtin().unwrap()), + ); + + let provider = backend + .resolve_provider_context("claude-sonnet-5", None) + .unwrap(); + + assert_eq!(provider.provider_id, ProviderId::anthropic()); + assert_eq!(provider.profile_kind, AgentProfileKind::Claude5); + } + #[test] fn api_backend_preserves_default_provider_for_legacy_model_identifier() { let settings: LlmCatalogSettings = toml::from_str( diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 2ec40d443..3f2b739af 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -1008,7 +1008,7 @@ reasoning = false assert_eq!( validated.graph().nodes["work"].attrs.get("model"), - Some(&AttrValue::String("claude-sonnet-4-6".into())) + Some(&AttrValue::String("claude-sonnet-5".into())) ); } @@ -1416,7 +1416,7 @@ reasoning = false .model .name .as_deref(), - Some("claude-sonnet-4-6") + Some("claude-sonnet-5") ); assert_eq!( created diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index b399d6637..45b70792d 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -157,7 +157,7 @@ mod tests { let transformed = transform(parsed, &transform_options()).unwrap(); assert_eq!( transformed.graph.nodes["work"].attrs.get("model"), - Some(&AttrValue::String("claude-sonnet-4-6".into())) + Some(&AttrValue::String("claude-sonnet-5".into())) ); } @@ -252,7 +252,7 @@ mod tests { ); assert_eq!( lint.attrs.get("model"), - Some(&AttrValue::String("claude-sonnet-4-6".into())) + Some(&AttrValue::String("claude-sonnet-5".into())) ); } diff --git a/lib/components/fabro-workflow/tests/materialize_run.rs b/lib/components/fabro-workflow/tests/materialize_run.rs index 4fec97585..830a73a3b 100644 --- a/lib/components/fabro-workflow/tests/materialize_run.rs +++ b/lib/components/fabro-workflow/tests/materialize_run.rs @@ -40,7 +40,7 @@ fn materialize_run_applies_graph_and_catalog_defaults() { .unwrap(); let resolved = &materialized.run; - assert_eq!(resolved.model.name.as_deref(), Some("claude-sonnet-4-6")); + assert_eq!(resolved.model.name.as_deref(), Some("claude-sonnet-5")); assert_eq!(resolved.model.provider.as_deref(), Some("anthropic")); assert_eq!( materialized.run.goal.as_ref(), diff --git a/lib/foundation/fabro-model/src/adapter.rs b/lib/foundation/fabro-model/src/adapter.rs index 5ff264b4c..c9eb25b97 100644 --- a/lib/foundation/fabro-model/src/adapter.rs +++ b/lib/foundation/fabro-model/src/adapter.rs @@ -67,6 +67,12 @@ impl AsRef for AdapterKind { #[strum(serialize_all = "snake_case")] pub enum AgentProfileKind { Anthropic, + /// Claude 5 models trained against Anthropic's current coding-agent + /// harness. This remains model-scoped so older Claude models keep the + /// established Anthropic profile. + #[serde(rename = "claude-5")] + #[strum(to_string = "claude-5")] + Claude5, #[serde(rename = "openai")] #[strum(to_string = "openai")] OpenAi, diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index f012a62ff..d020bfdba 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -2846,7 +2846,7 @@ enabled = true catalog .default_for_provider(&bedrock) .map(|model| model.id.as_str()), - Some("claude-sonnet-4-6") + Some("claude-sonnet-5") ); // Fable 5 ships with sampling params pinned off (the Converse // encoder drops temperature/top_p for it). @@ -2854,12 +2854,11 @@ enabled = true .get_on_provider(&bedrock, "claude-fable-5") .expect("fable row should be present"); assert!(!fable.features.sampling_params); - assert!( - catalog - .settings_for(fable) - .expect("fable settings should be present") - .reasoning_by_default - ); + let fable_settings = catalog + .settings_for(fable) + .expect("fable settings should be present"); + assert!(fable_settings.reasoning_by_default); + assert_eq!(fable_settings.agent_profile, AgentProfileKind::Claude5); assert_eq!( catalog .model_settings_on_provider(&bedrock, "claude-fable-5") @@ -2867,6 +2866,16 @@ enabled = true .billing_policy, BillingPolicy::Anthropic ); + let sonnet = catalog + .get_on_provider(&bedrock, "claude-sonnet-5") + .expect("Sonnet 5 row should be present"); + assert_eq!(sonnet.limits.context_window, 1_000_000); + assert_eq!(sonnet.limits.max_output, Some(128_000)); + assert!(!sonnet.features.sampling_params); + assert_eq!( + catalog.settings_for(sonnet).unwrap().agent_profile, + AgentProfileKind::Claude5 + ); } #[test] @@ -3013,7 +3022,7 @@ enabled = true // open-weights rows inherit it. assert_eq!( catalog - .model_settings_on_provider(&openrouter, "claude-sonnet-4-6") + .model_settings_on_provider(&openrouter, "claude-sonnet-5") .unwrap() .billing_policy, BillingPolicy::Anthropic @@ -3029,7 +3038,7 @@ enabled = true catalog .default_for_provider(&openrouter) .map(|model| model.id.as_str()), - Some("claude-sonnet-4-6") + Some("claude-sonnet-5") ); } @@ -3122,6 +3131,19 @@ enabled = true true, BillingPolicy::Anthropic, ), + ( + "claude-sonnet-5", + "anthropic/claude-sonnet-5", + "claude-5", + 1_000_000, + 2.0, + 10.0, + 0.2, + ReasoningEffortFeature::Levels, + false, + true, + BillingPolicy::Anthropic, + ), ]; for ( @@ -3173,13 +3195,21 @@ enabled = true ReasoningEffort::VARIANTS, "{id}" ); + if family == "claude-5" { + assert_eq!(settings.agent_profile, AgentProfileKind::Claude5, "{id}"); + } } - for alias in ["opus", "claude-opus"] { + for (alias, expected) in [ + ("opus", "claude-opus-5"), + ("claude-opus", "claude-opus-5"), + ("sonnet", "claude-sonnet-5"), + ("claude-sonnet", "claude-sonnet-5"), + ] { let model = catalog .resolve_on_provider(&ProviderId::new("openrouter"), alias) .unwrap_or_else(|error| panic!("{alias} should resolve on OpenRouter: {error}")); - assert_eq!(model.id, "claude-opus-5", "{alias}"); + assert_eq!(model.id, expected, "{alias}"); } } @@ -3868,7 +3898,7 @@ enabled = true let m = Catalog::builtin() .default_for_provider(&ProviderId::anthropic()) .unwrap(); - assert_eq!(m.id, "claude-sonnet-4-6"); + assert_eq!(m.id, "claude-sonnet-5"); assert!(m.default); let m = Catalog::builtin() diff --git a/lib/foundation/fabro-model/src/catalog/providers/anthropic.toml b/lib/foundation/fabro-model/src/catalog/providers/anthropic.toml index 25e9774ae..6d0adb16c 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/anthropic.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/anthropic.toml @@ -13,6 +13,7 @@ header = { custom = "x-api-key" } display_name = "Claude Fable 5" family = "claude-5" aliases = ["fable", "claude-fable"] +agent_profile = "claude-5" [providers.anthropic.models."claude-fable-5".limits] context_window = 1000000 @@ -37,6 +38,7 @@ family = "claude-5" training = "2026-05-01" knowledge_cutoff = "May 2026" aliases = ["opus", "claude-opus"] +agent_profile = "claude-5" [providers.anthropic.models."claude-opus-5".limits] context_window = 1000000 @@ -63,6 +65,33 @@ input_cost_per_mtok = 10.0 output_cost_per_mtok = 50.0 cache_input_cost_per_mtok = 1.0 +[providers.anthropic.models."claude-sonnet-5"] +display_name = "Claude Sonnet 5" +family = "claude-5" +training = "2026-01-01" +knowledge_cutoff = "Jan 2026" +default = true +aliases = ["sonnet", "claude-sonnet"] +agent_profile = "claude-5" + +[providers.anthropic.models."claude-sonnet-5".limits] +context_window = 1000000 +max_output = 128000 + +[providers.anthropic.models."claude-sonnet-5".features] +tools = true +vision = true +reasoning = true +reasoning_effort = "levels" +prompt_cache = true +sampling_params = false + +# Introductory pricing through August 31, 2026. +[providers.anthropic.models."claude-sonnet-5".costs] +input_cost_per_mtok = 2.0 +output_cost_per_mtok = 10.0 +cache_input_cost_per_mtok = 0.2 + [providers.anthropic.models."claude-opus-4-8"] display_name = "Claude Opus 4.8" family = "claude-4" @@ -188,9 +217,7 @@ display_name = "Claude Sonnet 4.6" family = "claude-4" training = "2025-08-01" knowledge_cutoff = "May 2025" -default = true estimated_output_tps = 50 -aliases = ["sonnet", "claude-sonnet"] [providers.anthropic.models."claude-sonnet-4-6".limits] context_window = 200000 diff --git a/lib/foundation/fabro-model/src/catalog/providers/bedrock.toml b/lib/foundation/fabro-model/src/catalog/providers/bedrock.toml index 71c74d686..401037c42 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/bedrock.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/bedrock.toml @@ -41,16 +41,15 @@ credentials = [ # ---------- Anthropic Claude ---------- # # Claude bills Anthropic-style cache reads/writes, so these rows override -# the provider's billing default. Claude Fable 5 appears at the end of this -# file because its Bedrock deployment pins sampling parameters and requires an -# extra data-sharing opt-in. +# the provider's billing default. Claude 5 models appear at the end of this +# file because their Bedrock deployments pin sampling parameters and require +# extra endpoint-specific handling. [providers.bedrock.models."claude-sonnet-4-6"] api_id = "us.anthropic.claude-sonnet-4-6" display_name = "Claude Sonnet 4.6 (Bedrock)" family = "claude-4" billing_policy = "anthropic" -default = true [providers.bedrock.models."claude-sonnet-4-6".limits] context_window = 1000000 @@ -360,6 +359,7 @@ api_id = "us.anthropic.claude-fable-5" display_name = "Claude Fable 5 (Bedrock)" family = "claude-5" billing_policy = "anthropic" +agent_profile = "claude-5" [providers.bedrock.models."claude-fable-5".limits] context_window = 1000000 @@ -377,3 +377,33 @@ sampling_params = false input_cost_per_mtok = 10.0 output_cost_per_mtok = 50.0 cache_input_cost_per_mtok = 1.0 + +# Claude Sonnet 5 uses adaptive thinking by default and rejects non-default +# sampling parameters. Effort-level mapping through +# additionalModelRequestFields is a named follow-up, as for Fable 5. + +[providers.bedrock.models."claude-sonnet-5"] +api_id = "us.anthropic.claude-sonnet-5" +display_name = "Claude Sonnet 5 (Bedrock)" +family = "claude-5" +billing_policy = "anthropic" +default = true +agent_profile = "claude-5" + +[providers.bedrock.models."claude-sonnet-5".limits] +context_window = 1000000 +max_output = 128000 + +[providers.bedrock.models."claude-sonnet-5".features] +tools = true +vision = true +reasoning = true +reasoning_by_default = true +prompt_cache = true +sampling_params = false + +# Introductory pricing through August 31, 2026. +[providers.bedrock.models."claude-sonnet-5".costs] +input_cost_per_mtok = 2.0 +output_cost_per_mtok = 10.0 +cache_input_cost_per_mtok = 0.2 diff --git a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml index 294dea6eb..f57361b0f 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml @@ -39,6 +39,7 @@ display_name = "Claude Fable 5 (via OpenRouter)" family = "claude-5" billing_policy = "anthropic" aliases = ["fable", "claude-fable"] +agent_profile = "claude-5" [providers.openrouter.models."claude-fable-5".limits] context_window = 1000000 @@ -66,6 +67,7 @@ billing_policy = "anthropic" training = "2026-05-01" knowledge_cutoff = "May 2026" aliases = ["opus", "claude-opus"] +agent_profile = "claude-5" [providers.openrouter.models."claude-opus-5".limits] context_window = 1000000 @@ -85,6 +87,37 @@ input_cost_per_mtok = 5.0 output_cost_per_mtok = 25.0 cache_input_cost_per_mtok = 0.5 +[providers.openrouter.models."claude-sonnet-5"] +api_id = "anthropic/claude-sonnet-5" +display_name = "Claude Sonnet 5 (via OpenRouter)" +family = "claude-5" +billing_policy = "anthropic" +training = "2026-01-01" +knowledge_cutoff = "Jan 2026" +default = true +aliases = ["sonnet", "claude-sonnet"] +agent_profile = "claude-5" + +[providers.openrouter.models."claude-sonnet-5".limits] +context_window = 1000000 +max_output = 128000 + +[providers.openrouter.models."claude-sonnet-5".features] +tools = true +vision = true +reasoning = true +reasoning_effort = "levels" +prompt_cache = true +cache_control_breakpoints = true +sampling_params = false + +# Current introductory rate. OpenRouter's authoritative in-band usage.cost +# supersedes this estimate on completed responses. +[providers.openrouter.models."claude-sonnet-5".costs] +input_cost_per_mtok = 2.0 +output_cost_per_mtok = 10.0 +cache_input_cost_per_mtok = 0.2 + [providers.openrouter.models."claude-opus-4-8"] api_id = "anthropic/claude-opus-4.8" display_name = "Claude Opus 4.8 (via OpenRouter)" @@ -138,8 +171,6 @@ api_id = "anthropic/claude-sonnet-4.6" display_name = "Claude Sonnet 4.6 (via OpenRouter)" family = "claude-4" billing_policy = "anthropic" -default = true -aliases = ["sonnet", "claude-sonnet"] [providers.openrouter.models."claude-sonnet-4-6".limits] context_window = 1000000 From d669a2d55c0b808e2005624dfef05e583b563487 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 15:15:39 -0400 Subject: [PATCH 02/19] 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) From f293e3de18af896440d2d810699bb840728ac2c4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 23:44:20 -0400 Subject: [PATCH 03/19] refactor(agent): fold parent notifications into subagent state `ParentNotificationHub` kept a second `Mutex` and `watch` channel holding a copy of each child's terminal result -- data `SubAgent.status` already owns as `SubAgentStatus::Finished`, and which is never evicted, since nothing removes entries from `SupervisorState.agents`. Two of the three bugs fixed in the previous commit were ordering bugs in the coupling between those two structures: suppress-vs-commit in `begin_shutdown`, and register-vs-publish in `spawn_inner`. Both were fixed by ordering the steps correctly. Keeping the registration beside the status it is delivered with makes that whole class unrepresentable instead: - Registration is now a field on the `SubAgent` literal `spawn_inner` already builds, under the lock that publishes it. There is no window between publishing an agent and registering its notification. - Suppression on shutdown happens inside the critical section that decides the shutdown, after the status transition commits, so a rejected shutdown cannot discard a result the parent is owed. - `next_parent_notification_batch` scans agents for a live registration whose status is `Finished`, and ignores `Closing`/`Closed` outright -- so a shutdown racing delivery can no longer park the parent on a result that will never arrive, even if suppression were missed. `spawn_result_monitor` no longer takes the hub; it bumps a single `watch` counter after committing the status it already commits. Batch order was the queue's insertion order, so `SubAgent` carries a `spawn_seq` to keep delivery oldest-first. Tests move from exercising the hub directly to the supervisor API, and cover spawn-order batching and the shutdown-races-delivery case that the old shape could not express. Co-Authored-By: Claude Opus 5 (1M context) --- lib/components/fabro-agent/src/subagent.rs | 446 ++++++++++++--------- 1 file changed, 259 insertions(+), 187 deletions(-) diff --git a/lib/components/fabro-agent/src/subagent.rs b/lib/components/fabro-agent/src/subagent.rs index f4d6c1b84..476094a94 100644 --- a/lib/components/fabro-agent/src/subagent.rs +++ b/lib/components/fabro-agent/src/subagent.rs @@ -89,16 +89,27 @@ pub enum SubAgentStatus { const SUBAGENT_SHUTDOWN_GRACE: Duration = Duration::from_secs(5); struct SubAgent { - status: watch::Sender, - cleanup_done: watch::Sender, - cleanup_started: bool, - monitor_task: Option>, - event_forwarder: Option>, - cleanup_task: Option>, - child_abort_handle: AbortHandle, - followup_queue: Arc>>, - cancel_token: CancellationToken, - depth: usize, + status: watch::Sender, + cleanup_done: watch::Sender, + cleanup_started: bool, + monitor_task: Option>, + event_forwarder: Option>, + cleanup_task: Option>, + child_abort_handle: AbortHandle, + followup_queue: Arc>>, + cancel_token: CancellationToken, + depth: usize, + /// Task description, set when the parent should receive this child's + /// terminal result automatically. Cleared once the result is delivered, + /// the parent retrieves it explicitly, or the agent is shut down. + /// + /// Keeping this beside the status it is delivered with means a + /// notification cannot be registered before -- or suppressed after -- the + /// state it describes: there is only one lock and one ordering. + parent_notification: Option, + /// Spawn order, so a batch is delivered oldest-first rather than in + /// whatever order the map happens to iterate. + spawn_seq: u64, } impl Drop for SubAgent { @@ -119,114 +130,8 @@ impl Drop for SubAgent { #[derive(Default)] struct SupervisorState { - agents: HashMap, -} - -#[derive(Default)] -struct ParentNotificationState { - pending: HashMap, - ready: VecDeque, -} - -struct ParentNotificationHub { - state: Mutex, - changed: watch::Sender, -} - -impl ParentNotificationHub { - fn new() -> Self { - let (changed, _) = watch::channel(0); - Self { - state: Mutex::new(ParentNotificationState::default()), - changed, - } - } - - fn register(&self, agent_id: String, description: String) { - self.state - .lock() - .expect("parent notification lock poisoned") - .pending - .insert(agent_id, description); - self.signal(); - } - - fn complete(&self, agent_id: &str, result: Result) { - { - let mut state = self - .state - .lock() - .expect("parent notification lock poisoned"); - let Some(description) = state.pending.remove(agent_id) else { - return; - }; - state.ready.push_back(SubAgentParentNotification { - agent_id: agent_id.to_string(), - description, - result, - }); - } - self.signal(); - } - - fn suppress(&self, agent_id: &str) { - let changed = { - let mut state = self - .state - .lock() - .expect("parent notification lock poisoned"); - let removed_pending = state.pending.remove(agent_id).is_some(); - let ready_len = state.ready.len(); - state - .ready - .retain(|notification| notification.agent_id != agent_id); - removed_pending || state.ready.len() != ready_len - }; - if changed { - self.signal(); - } - } - - async fn next_batch( - &self, - cancel: &CancellationToken, - ) -> Result>, Error> { - let mut changed = self.changed.subscribe(); - loop { - { - let mut state = self - .state - .lock() - .expect("parent notification lock poisoned"); - if !state.ready.is_empty() { - return Ok(Some(state.ready.drain(..).collect())); - } - if state.pending.is_empty() { - return Ok(None); - } - } - - tokio::select! { - biased; - () = cancel.cancelled() => { - return Err(Error::Interrupted(InterruptReason::Cancelled)); - } - observed = changed.changed() => { - observed.map_err(|_| { - Error::InvalidState( - "Background-agent notification observer closed unexpectedly".to_string(), - ) - })?; - } - } - } - } - - fn signal(&self) { - self.changed.send_modify(|generation| { - *generation = generation.wrapping_add(1); - }); - } + agents: HashMap, + next_spawn_seq: u64, } struct ShutdownWork { @@ -268,11 +173,20 @@ impl Drop for CleanupDoneGuard { } } +/// Wake anything parked in +/// [`SubAgentSupervisor::next_parent_notification_batch`] so it can re-evaluate +/// which children are deliverable. +fn signal_notifications(changed: &watch::Sender) { + changed.send_modify(|generation| { + *generation = generation.wrapping_add(1); + }); +} + fn spawn_result_monitor( child_task: JoinHandle>, status: watch::Sender, event_callback: Arc>>, - parent_notifications: Arc, + notifications_changed: Arc>, agent_id: String, depth: usize, ) -> JoinHandle<()> { @@ -294,6 +208,8 @@ fn spawn_result_monitor( if !committed { return; } + // The status this agent will be delivered with is now committed. + signal_notifications(¬ifications_changed); let event = match &task_result { Ok(result) => AgentEvent::SubAgentCompleted { @@ -315,7 +231,6 @@ fn spawn_result_monitor( if let Some(callback) = callback { callback(SubAgentCallbackEvent::Lifecycle(event)); } - parent_notifications.complete(&agent_id, task_result); }) } @@ -326,10 +241,10 @@ fn spawn_result_monitor( /// happen after the guard has been released. #[derive(Clone)] pub struct SubAgentSupervisor { - state: Arc>, - max_depth: usize, - event_callback: Arc>>, - parent_notifications: Arc, + state: Arc>, + max_depth: usize, + event_callback: Arc>>, + notifications_changed: Arc>, } impl SubAgentSupervisor { @@ -339,7 +254,7 @@ impl SubAgentSupervisor { state: Arc::new(Mutex::new(SupervisorState::default())), max_depth, event_callback: Arc::new(RwLock::new(None)), - parent_notifications: Arc::new(ParentNotificationHub::new()), + notifications_changed: Arc::new(watch::channel(0).0), } } @@ -474,23 +389,15 @@ impl SubAgentSupervisor { child_task, status.clone(), Arc::clone(&self.event_callback), - Arc::clone(&self.parent_notifications), + Arc::clone(&self.notifications_changed), agent_id.clone(), 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"); + let spawn_seq = state.next_spawn_seq; + state.next_spawn_seq = state.next_spawn_seq.saturating_add(1); state.agents.insert(agent_id.clone(), SubAgent { status, cleanup_done, @@ -502,8 +409,11 @@ impl SubAgentSupervisor { followup_queue, cancel_token, depth: child_depth, + parent_notification: parent_notification_description, + spawn_seq, }); } + signal_notifications(&self.notifications_changed); self.emit_event(AgentEvent::SubAgentSpawned { agent_id: agent_id.clone(), @@ -587,11 +497,20 @@ impl SubAgentSupervisor { } } - /// Stop automatic delivery for an agent whose result the parent explicitly - /// retrieved. Removes a result that may already have raced into the ready - /// queue. + /// Stop automatic delivery for an agent whose result the parent retrieved + /// explicitly. pub(crate) fn suppress_parent_notification(&self, agent_id: &str) { - self.parent_notifications.suppress(agent_id); + let cleared = { + let mut state = self.state.lock().expect("subagent state lock poisoned"); + state + .agents + .get_mut(agent_id) + .and_then(|agent| agent.parent_notification.take()) + .is_some() + }; + if cleared { + signal_notifications(&self.notifications_changed); + } } /// Wait until all currently-ready background results can be delivered in @@ -616,7 +535,68 @@ impl SubAgentSupervisor { &self, cancel: &CancellationToken, ) -> Result>, Error> { - self.parent_notifications.next_batch(cancel).await + let mut changed = self.notifications_changed.subscribe(); + loop { + { + let mut state = self.state.lock().expect("subagent state lock poisoned"); + let mut ready = Vec::new(); + let mut awaiting_result = false; + for (agent_id, agent) in &state.agents { + let Some(description) = agent.parent_notification.as_ref() else { + continue; + }; + let finished = match &*agent.status.borrow() { + SubAgentStatus::Finished(result) => Some(result.clone()), + SubAgentStatus::Running => { + awaiting_result = true; + None + } + // Being torn down, so no result is coming. Ignoring + // these is what keeps a shutdown that races delivery + // from parking the parent forever. + SubAgentStatus::Closing | SubAgentStatus::Closed => None, + }; + if let Some(result) = finished { + ready.push((agent.spawn_seq, SubAgentParentNotification { + agent_id: agent_id.clone(), + description: description.clone(), + result, + })); + } + } + + if !ready.is_empty() { + ready.sort_by_key(|(spawn_seq, _)| *spawn_seq); + let batch: Vec<_> = ready + .into_iter() + .map(|(_, notification)| notification) + .collect(); + for notification in &batch { + if let Some(agent) = state.agents.get_mut(¬ification.agent_id) { + agent.parent_notification = None; + } + } + return Ok(Some(batch)); + } + if !awaiting_result { + return Ok(None); + } + } + + tokio::select! { + biased; + () = cancel.cancelled() => { + return Err(Error::Interrupted(InterruptReason::Cancelled)); + } + observed = changed.changed() => { + observed.map_err(|_| { + Error::InvalidState( + "Background-agent notification observer closed unexpectedly".to_string(), + ) + })?; + } + } + } } #[cfg(test)] @@ -666,6 +646,11 @@ impl SubAgentSupervisor { } }; + // Shutdown is committed, so this child's result will never reach the + // parent. The early returns above leave the notification intact, so a + // rejected shutdown cannot discard a result the parent is owed. + agent.parent_notification = None; + if agent.cleanup_started { return Ok(ShutdownDisposition::Follow(agent.cleanup_done.subscribe())); } @@ -772,10 +757,7 @@ impl SubAgentSupervisor { async fn ensure_closed(&self, agent_id: &str) -> Result<(), Error> { 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); + signal_notifications(&self.notifications_changed); let cleanup_done = match disposition { ShutdownDisposition::Lead(work) => self.spawn_shutdown(work), ShutdownDisposition::Follow(cleanup_done) => cleanup_done, @@ -788,7 +770,7 @@ 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 disposition = self.begin_shutdown(agent_id, true)?; - self.parent_notifications.suppress(agent_id); + signal_notifications(&self.notifications_changed); let cleanup_done = match disposition { ShutdownDisposition::Lead(work) => self.spawn_shutdown(work), ShutdownDisposition::Follow(_) | ShutdownDisposition::Done => { @@ -857,7 +839,7 @@ impl SubAgentSupervisor { child_task, status.clone(), Arc::clone(&self.event_callback), - Arc::clone(&self.parent_notifications), + Arc::clone(&self.notifications_changed), agent_id.clone(), depth, ); @@ -873,6 +855,8 @@ impl SubAgentSupervisor { event_forwarder, cleanup_task: None, child_abort_handle, + parent_notification: None, + spawn_seq: 0, followup_queue: Arc::new(Mutex::new(VecDeque::new())), cancel_token, depth, @@ -1072,67 +1056,155 @@ mod tests { assert!(manager.is_empty()); } - #[tokio::test] - async fn parent_notifications_are_exactly_once_and_xml_escaped() { - let hub = ParentNotificationHub::new(); - hub.register("agent<&".to_string(), "Review & tests".to_string()); - let result = Ok(SubAgentResult { - output: "done & \"verified\"".to_string(), - success: true, - turns_used: 2, - }); - hub.complete("agent<&", result.clone()); - hub.complete("agent<&", result); + #[test] + fn parent_notification_envelope_escapes_xml() { + let envelope = format_parent_notification_batch(&[SubAgentParentNotification { + agent_id: "agent<&".to_string(), + description: "Review & tests".to_string(), + result: Ok(SubAgentResult { + output: "done & \"verified\"".to_string(), + success: true, + turns_used: 2, + }), + }]); - let notifications = hub - .next_batch(&CancellationToken::new()) - .await - .unwrap() - .unwrap(); - assert_eq!(notifications.len(), 1); - let envelope = format_parent_notification_batch(¬ifications); assert!(envelope.contains("completed")); assert!(envelope.contains("agent<&")); assert!(envelope.contains("Review <core> & tests")); assert!( envelope.contains("done <safely> & "verified"") ); - assert!( - hub.next_batch(&CancellationToken::new()) - .await - .unwrap() - .is_none() - ); } #[tokio::test] - async fn suppress_removes_pending_and_ready_parent_notifications() { - let hub = ParentNotificationHub::new(); - hub.register("pending".to_string(), "Pending".to_string()); - hub.suppress("pending"); + async fn a_finished_agent_is_delivered_to_the_parent_exactly_once() { + 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(); + supervisor + .wait_with_cancel(&agent_id, &CancellationToken::new()) + .await + .unwrap(); + + let batch = supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .expect("the finished child must be delivered"); + assert_eq!(batch.len(), 1); + assert_eq!(batch[0].agent_id, agent_id); + assert_eq!(batch[0].description, "Inspect the module"); + + // The status stays `Finished`, so re-delivery is prevented by clearing + // the registration rather than by consuming the result. assert!( - hub.next_batch(&CancellationToken::new()) + supervisor + .next_parent_notification_batch(&CancellationToken::new()) .await .unwrap() .is_none() ); - hub.register("ready".to_string(), "Ready".to_string()); - hub.complete( - "ready", - Ok(SubAgentResult { - output: "done".to_string(), - success: true, - turns_used: 1, - }), - ); - hub.suppress("ready"); + supervisor.shutdown_all().await; + } + + #[tokio::test] + async fn batches_are_delivered_in_spawn_order() { + let supervisor = SubAgentSupervisor::new(3); + let mut ids = Vec::new(); + for index in 0..3 { + let child = make_session(vec![text_response("done")]).await; + ids.push( + supervisor + .spawn_with_parent_notification( + child, + format!("task {index}"), + format!("Task {index}"), + 0, + ) + .unwrap(), + ); + } + for id in &ids { + supervisor + .wait_with_cancel(id, &CancellationToken::new()) + .await + .unwrap(); + } + + let batch = supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .expect("all three children must be delivered together"); + let delivered: Vec<_> = batch.iter().map(|n| n.agent_id.clone()).collect(); + assert_eq!(delivered, ids); + + supervisor.shutdown_all().await; + } + + #[tokio::test] + async fn suppressing_before_completion_stops_delivery() { + 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(); + + supervisor.suppress_parent_notification(&agent_id); + supervisor + .wait_with_cancel(&agent_id, &CancellationToken::new()) + .await + .unwrap(); + assert!( - hub.next_batch(&CancellationToken::new()) + supervisor + .next_parent_notification_batch(&CancellationToken::new()) .await .unwrap() .is_none() ); + + supervisor.shutdown_all().await; + } + + #[tokio::test] + async fn closing_a_running_agent_stops_delivery_without_parking_the_parent() { + 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(); + + supervisor.close_agent(&agent_id).await.unwrap(); + + // Must resolve rather than wait for a result that will never arrive. + assert!( + supervisor + .next_parent_notification_batch(&CancellationToken::new()) + .await + .unwrap() + .is_none() + ); + + supervisor.shutdown_all().await; } #[tokio::test] From 27549c2358edc12a20f25d732d80d3934b2955f2 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 26 Jul 2026 07:37:48 -0400 Subject: [PATCH 04/19] fix(agent): share the task runtime with every profile's children Task tools scope their list by `root_session_id` -- `Session` documents this as "a subagent session inherits its parent's `root_session_id` so todo tools that scope by root (Anthropic tasks) share one list across all subagents" -- so a root and its children address one logical list. `build()` runs once per session, though, and `AnthropicProfile` constructed its own `TodoRuntime` inside that call. Root and child therefore resolved the same `list_id` through different runtimes: both ID counters started at zero, so both emitted `todo.created` with id `1` for the same list, and `TodoListProjection::upsert` matches on id -- the child's task replaced the parent's in the persisted projection. `TaskGet` and `TaskList` read the local runtime, so neither session could see the other's tasks either. The previous commit's shared runtime fixed this for Claude 5 only, because `build()` passed dependencies positionally and adding a fourth argument would have meant touching all six call sites. It grew a second constructor for Claude 5 instead, leaving the other five on a signature that could not carry the runtime. Bundle them into `ProfileDeps` so every profile takes the same `(model, &deps)`. The duplicate constructor is gone, Anthropic shares the runtime by construction rather than by opting in, and a future dependency reaches all six profiles or none. The existing Claude 5 sharing test is generalized and now also runs for Anthropic; it fails against a per-profile runtime. Co-Authored-By: Claude Opus 5 (1M context) --- .../fabro-agent/src/profiles/anthropic.rs | 26 ++- .../fabro-agent/src/profiles/claude5.rs | 32 +--- .../fabro-agent/src/profiles/gemini.rs | 19 +-- .../fabro-agent/src/profiles/gpt56.rs | 13 +- .../fabro-agent/src/profiles/kimi.rs | 17 +- .../fabro-agent/src/profiles/mod.rs | 149 ++++++++++++------ .../fabro-agent/src/profiles/openai.rs | 17 +- 7 files changed, 148 insertions(+), 125 deletions(-) diff --git a/lib/components/fabro-agent/src/profiles/anthropic.rs b/lib/components/fabro-agent/src/profiles/anthropic.rs index 7cf84a80b..a34358d81 100644 --- a/lib/components/fabro-agent/src/profiles/anthropic.rs +++ b/lib/components/fabro-agent/src/profiles/anthropic.rs @@ -5,17 +5,14 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; use crate::sandbox::Sandbox; use crate::skills::Skill; -use crate::todo_runtime::TodoRuntime; use crate::todo_tools::{ make_task_create_tool, make_task_get_tool, make_task_list_tool, make_task_update_tool, }; use crate::tool_registry::ToolRegistry; -use crate::tools::{ - WEB_SEARCH_TOOL_NAME, WebFetchSummarizer, make_edit_file_tool, register_core_tools, -}; +use crate::tools::{WEB_SEARCH_TOOL_NAME, make_edit_file_tool, register_core_tools}; pub struct AnthropicProfile { base: BaseProfile, @@ -26,21 +23,20 @@ const CORE_PROMPT: &str = include_str!("prompts/anthropic.md.j2"); impl AnthropicProfile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::Anthropic); - Self::with_native_tools(model, &options, None) + let deps = + ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::Anthropic)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - ) -> Self { + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, options, summarizer); + register_core_tools(&mut registry, &deps.options, deps.summarizer.clone()); registry.register(make_edit_file_tool()); - // Anthropic task tools share one runtime per profile instance. - let todo_runtime = Arc::new(TodoRuntime::new()); + // Task tools scope their list by `root_session_id`, so a root session + // and its children address one logical list. They must therefore + // resolve it through the one runtime the builder shares between them. + let todo_runtime = Arc::clone(&deps.todo_runtime); registry.register(make_task_create_tool(todo_runtime.clone())); registry.register(make_task_update_tool(todo_runtime.clone())); registry.register(make_task_get_tool(todo_runtime.clone())); diff --git a/lib/components/fabro-agent/src/profiles/claude5.rs b/lib/components/fabro-agent/src/profiles/claude5.rs index d6d2faa6c..43a2db179 100644 --- a/lib/components/fabro-agent/src/profiles/claude5.rs +++ b/lib/components/fabro-agent/src/profiles/claude5.rs @@ -8,16 +8,14 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, claude5_tools}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps, claude5_tools}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::subagent::{SessionFactory, SubAgentSupervisor}; -use crate::todo_runtime::TodoRuntime; use crate::todo_tools::{ make_task_create_tool, make_task_get_tool, make_task_list_tool, make_task_update_tool, }; use crate::tool_registry::ToolRegistry; -use crate::tools::WebFetchSummarizer; const CORE_PROMPT: &str = include_str!("prompts/claude5.md.j2"); @@ -28,29 +26,15 @@ pub struct Claude5Profile { impl Claude5Profile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::Claude5); - Self::with_native_tools(model, &options, None) + let deps = + ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::Claude5)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - ) -> Self { - Self::with_native_tools_and_todo_runtime( - model, - options, - summarizer, - Arc::new(TodoRuntime::new()), - ) - } - - pub(crate) fn with_native_tools_and_todo_runtime( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - todo_runtime: Arc, - ) -> Self { + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { + let options = &deps.options; + let summarizer = deps.summarizer.clone(); + let todo_runtime = Arc::clone(&deps.todo_runtime); let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::Claude5); registry.register(claude5_tools::make_read_tool()); registry.register(claude5_tools::make_write_tool()); diff --git a/lib/components/fabro-agent/src/profiles/gemini.rs b/lib/components/fabro-agent/src/profiles/gemini.rs index a3a9fdf5b..c533a85e3 100644 --- a/lib/components/fabro-agent/src/profiles/gemini.rs +++ b/lib/components/fabro-agent/src/profiles/gemini.rs @@ -5,13 +5,13 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::tool_registry::ToolRegistry; use crate::tools::{ - WEB_SEARCH_TOOL_NAME, WebFetchSummarizer, make_edit_file_tool, make_list_dir_tool, - make_read_many_files_tool, register_core_tools, + WEB_SEARCH_TOOL_NAME, make_edit_file_tool, make_list_dir_tool, make_read_many_files_tool, + register_core_tools, }; const CORE_PROMPT: &str = include_str!("prompts/gemini.md.j2"); @@ -23,18 +23,15 @@ pub struct GeminiProfile { impl GeminiProfile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::Gemini); - Self::with_native_tools(model, &options, None) + let deps = + ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::Gemini)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - ) -> Self { + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, options, summarizer); + register_core_tools(&mut registry, &deps.options, deps.summarizer.clone()); registry.register(make_edit_file_tool()); registry.register(make_read_many_files_tool()); registry.register(make_list_dir_tool()); diff --git a/lib/components/fabro-agent/src/profiles/gpt56.rs b/lib/components/fabro-agent/src/profiles/gpt56.rs index b5a436466..66fa5ddfd 100644 --- a/lib/components/fabro-agent/src/profiles/gpt56.rs +++ b/lib/components/fabro-agent/src/profiles/gpt56.rs @@ -24,7 +24,7 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, FileEditToolKind}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, FileEditToolKind, ProfileDeps}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; @@ -45,11 +45,13 @@ pub struct Gpt56Profile { impl Gpt56Profile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::Gpt56); - Self::with_native_tools(model, &options) + let deps = ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::Gpt56)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools(model: impl Into, options: &NativeToolOptions) -> Self { + /// `deps.summarizer` is ignored: this profile exposes no `web_fetch`. + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { + let options = &deps.options; // The registry carries the vocabulary, so tools registered later -- // subagent tools, skills -- are named consistently too. let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::Codex); @@ -386,7 +388,8 @@ mod tests { let mut options = NativeToolOptions::for_profile(AgentProfileKind::Gpt56); options.secrets.brave_search_api_key = Some("configured-key".to_string()); - let searching = Gpt56Profile::with_native_tools("gpt-5.6-sol", &options); + let deps = ProfileDeps::standalone(options); + let searching = Gpt56Profile::with_native_tools("gpt-5.6-sol", &deps); assert!(searching.tool_registry().get("web_search").is_some()); assert!(prompt(&searching).contains("web_search")); } diff --git a/lib/components/fabro-agent/src/profiles/kimi.rs b/lib/components/fabro-agent/src/profiles/kimi.rs index f5f8f05fa..1e5bb01de 100644 --- a/lib/components/fabro-agent/src/profiles/kimi.rs +++ b/lib/components/fabro-agent/src/profiles/kimi.rs @@ -6,13 +6,13 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, kimi_tools}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps, kimi_tools}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; use crate::todo_tools::make_todo_list_tool; use crate::tool_registry::ToolRegistry; -use crate::tools::{WebFetchSummarizer, register_discovery_and_web_tools}; +use crate::tools::register_discovery_and_web_tools; const CORE_PROMPT: &str = include_str!("prompts/kimi.md.j2"); @@ -56,15 +56,12 @@ pub struct KimiProfile { impl KimiProfile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::Kimi); - Self::with_native_tools(model, &options, None) + let deps = ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::Kimi)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - ) -> Self { + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { + let options = &deps.options; // The registry carries the vocabulary, so tools registered later // (subagent tools, skills) are renamed too. let mut registry = ToolRegistry::with_vocabulary(ToolVocabulary::KimiCode); @@ -72,7 +69,7 @@ impl KimiProfile { // Glob and the web tools have the same contract in both vocabularies. // The remaining Kimi tools use adapters for their different schemas, // while reusing shared execution helpers where their behavior agrees. - register_discovery_and_web_tools(&mut registry, options, summarizer); + register_discovery_and_web_tools(&mut registry, options, deps.summarizer.clone()); registry.register(kimi_tools::make_kimi_read_tool()); registry.register(kimi_tools::make_kimi_write_tool()); registry.register(kimi_tools::make_kimi_edit_tool(EDIT_FILE_DESCRIPTION)); diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index 798e39661..23168f13c 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -46,6 +46,32 @@ pub struct AgentProfileBuilder { todo_runtime: Arc, } +/// Everything a profile constructor needs from the builder. +/// +/// Bundled rather than passed positionally so that adding a dependency does +/// not mean editing every profile's signature -- and, more importantly, so a +/// dependency cannot reach some profiles and silently miss others. The shared +/// `todo_runtime` is exactly that case: task tools scope their list by +/// `root_session_id`, so a root and its children address one logical list and +/// must resolve it through one runtime. +pub(crate) struct ProfileDeps { + pub options: NativeToolOptions, + pub summarizer: Option, + pub todo_runtime: Arc, +} + +impl ProfileDeps { + /// Standalone defaults, for `Profile::new` and tests. A profile built this + /// way owns its runtime because it has no children to share one with. + pub(crate) fn standalone(options: NativeToolOptions) -> Self { + Self { + options, + summarizer: None, + todo_runtime: Arc::new(TodoRuntime::new()), + } + } +} + impl AgentProfileBuilder { #[must_use] pub fn new( @@ -84,44 +110,42 @@ impl AgentProfileBuilder { #[must_use] pub fn build(&self) -> Box { let model = self.model.as_str(); - let options = &self.native_tool_options; - let summarizer = if self.profile_kind == AgentProfileKind::Gpt56 { - None - } else { - self.summarizer.clone() + let deps = ProfileDeps { + options: self.native_tool_options.clone(), + summarizer: if self.profile_kind == AgentProfileKind::Gpt56 { + None + } else { + self.summarizer.clone() + }, + todo_runtime: Arc::clone(&self.todo_runtime), }; match self.profile_kind { AgentProfileKind::OpenAi => Box::new( - OpenAiProfile::with_native_tools(model, options, summarizer) + OpenAiProfile::with_native_tools(model, &deps) .with_route(self.provider_id.clone(), Arc::clone(&self.catalog)), ), AgentProfileKind::Gemini => Box::new( - GeminiProfile::with_native_tools(model, options, summarizer) + GeminiProfile::with_native_tools(model, &deps) .with_provider_id(self.provider_id.clone()) .with_catalog(Arc::clone(&self.catalog)), ), AgentProfileKind::Anthropic => Box::new( - AnthropicProfile::with_native_tools(model, options, summarizer) + AnthropicProfile::with_native_tools(model, &deps) .with_provider_id(self.provider_id.clone()) .with_catalog(Arc::clone(&self.catalog)), ), AgentProfileKind::Claude5 => Box::new( - Claude5Profile::with_native_tools_and_todo_runtime( - model, - options, - summarizer, - Arc::clone(&self.todo_runtime), - ) - .with_provider_id(self.provider_id.clone()) - .with_catalog(Arc::clone(&self.catalog)), + Claude5Profile::with_native_tools(model, &deps) + .with_provider_id(self.provider_id.clone()) + .with_catalog(Arc::clone(&self.catalog)), ), AgentProfileKind::Kimi => Box::new( - KimiProfile::with_native_tools(model, options, summarizer) + KimiProfile::with_native_tools(model, &deps) .with_provider_id(self.provider_id.clone()) .with_catalog(Arc::clone(&self.catalog)), ), AgentProfileKind::Gpt56 => Box::new( - Gpt56Profile::with_native_tools(model, options) + Gpt56Profile::with_native_tools(model, &deps) .with_route(self.provider_id.clone(), Arc::clone(&self.catalog)), ), } @@ -422,7 +446,8 @@ mod tests { fn anthropic_profile(has_web_search: bool, has_subagents: bool) -> AnthropicProfile { let options = native_tool_options(AgentProfileKind::Anthropic, has_web_search); - let mut profile = AnthropicProfile::with_native_tools("claude-haiku-4-5", &options, None); + let deps = ProfileDeps::standalone(options); + let mut profile = AnthropicProfile::with_native_tools("claude-haiku-4-5", &deps); if has_subagents { register_test_subagent_tools(&mut profile); } @@ -435,7 +460,8 @@ mod tests { has_question: bool, ) -> Claude5Profile { let options = native_tool_options(AgentProfileKind::Claude5, has_web_search); - let mut profile = Claude5Profile::with_native_tools("claude-sonnet-5", &options, None); + let deps = ProfileDeps::standalone(options); + let mut profile = Claude5Profile::with_native_tools("claude-sonnet-5", &deps); if has_subagents { register_test_subagent_tools(&mut profile); } @@ -450,26 +476,30 @@ mod tests { fn gemini_profile(has_web_search: bool) -> GeminiProfile { let options = native_tool_options(AgentProfileKind::Gemini, has_web_search); - GeminiProfile::with_native_tools("gemini-3-flash-preview", &options, None) + let deps = ProfileDeps::standalone(options); + GeminiProfile::with_native_tools("gemini-3-flash-preview", &deps) } fn openai_apply_patch_profile(has_web_search: bool) -> OpenAiProfile { let options = native_tool_options(AgentProfileKind::OpenAi, has_web_search); - OpenAiProfile::with_native_tools("gpt-5.4-mini", &options, None) + let deps = ProfileDeps::standalone(options); + OpenAiProfile::with_native_tools("gpt-5.4-mini", &deps) } fn gpt56_profile(has_web_search: bool) -> Gpt56Profile { let options = native_tool_options(AgentProfileKind::Gpt56, has_web_search); - Gpt56Profile::with_native_tools("gpt-5.6-sol", &options) + let deps = ProfileDeps::standalone(options); + Gpt56Profile::with_native_tools("gpt-5.6-sol", &deps) } /// GPT-5.6 through an OpenAI-compatible gateway, where `apply_patch` /// cannot be carried and `edit_file` takes its place. fn gpt56_edit_file_profile(has_web_search: bool) -> Gpt56Profile { let options = native_tool_options(AgentProfileKind::Gpt56, has_web_search); + let deps = ProfileDeps::standalone(options); let overrides: LlmCatalogSettings = toml::from_str("[providers.openrouter]\nenabled = true\n").unwrap(); - Gpt56Profile::with_native_tools("gpt-5.6-sol", &options).with_route( + Gpt56Profile::with_native_tools("gpt-5.6-sol", &deps).with_route( ProviderId::new("openrouter"), Arc::new(Catalog::from_builtin_with_overrides(&overrides).unwrap()), ) @@ -477,7 +507,8 @@ mod tests { fn openai_edit_file_profile(has_web_search: bool) -> OpenAiProfile { let options = native_tool_options(AgentProfileKind::OpenAi, has_web_search); - OpenAiProfile::with_native_tools("kimi-k2.5", &options, None).with_route( + let deps = ProfileDeps::standalone(options); + OpenAiProfile::with_native_tools("kimi-k2.5", &deps).with_route( ProviderId::new("kimi"), Arc::new(Catalog::from_builtin().unwrap()), ) @@ -681,37 +712,37 @@ mod tests { } } - #[tokio::test] - async fn claude5_builder_shares_tasks_across_root_and_child_profiles() { + /// Task tools scope their list by `root_session_id`, so a root session and + /// every child it spawns address one logical list. `build()` runs once per + /// session, so the runtime behind that list has to come from the builder -- + /// a per-profile runtime gives each session its own projection and its own + /// ID counter, and the two sessions then collide on `#1` in the merged + /// projection while neither can see the other's tasks. + async fn assert_builder_shares_tasks_across_root_and_child( + profile_kind: AgentProfileKind, + model: &str, + ) { let builder = AgentProfileBuilder::new( - AgentProfileKind::Claude5, + profile_kind, ProviderId::anthropic(), - "claude-sonnet-5", + model, Arc::new(Catalog::from_builtin().unwrap()), ); let root = builder.build(); let child = builder.build(); - let root_create = Arc::clone( - &root - .tool_registry() - .get("TaskCreate") - .expect("root should expose TaskCreate") - .executor, - ); - let child_create = Arc::clone( - &child - .tool_registry() - .get("TaskCreate") - .expect("child should expose TaskCreate") - .executor, - ); - let child_list = Arc::clone( - &child - .tool_registry() - .get("TaskList") - .expect("child should expose TaskList") - .executor, - ); + let executor = |profile: &dyn AgentProfile, name: &str| { + Arc::clone( + &profile + .tool_registry() + .get(name) + .unwrap_or_else(|| panic!("{profile_kind} should expose {name}")) + .executor, + ) + }; + let root_create = executor(root.as_ref(), "TaskCreate"); + let child_create = executor(child.as_ref(), "TaskCreate"); + let child_list = executor(child.as_ref(), "TaskList"); + let env: Arc = Arc::new(MockSandbox::default()); let context = |session_id: &str| ToolContext { env: Arc::clone(&env), @@ -743,6 +774,24 @@ mod tests { assert!(tasks.contains("#2 [pending] Child task"), "{tasks}"); } + #[tokio::test] + async fn claude5_builder_shares_tasks_across_root_and_child_profiles() { + assert_builder_shares_tasks_across_root_and_child( + AgentProfileKind::Claude5, + "claude-sonnet-5", + ) + .await; + } + + #[tokio::test] + async fn anthropic_builder_shares_tasks_across_root_and_child_profiles() { + assert_builder_shares_tasks_across_root_and_child( + AgentProfileKind::Anthropic, + "claude-haiku-4-5", + ) + .await; + } + #[test] fn profile_builder_selects_a_codec_compatible_gpt56_editor() { let overrides: LlmCatalogSettings = diff --git a/lib/components/fabro-agent/src/profiles/openai.rs b/lib/components/fabro-agent/src/profiles/openai.rs index 8eeafdfce..2f5bbf4df 100644 --- a/lib/components/fabro-agent/src/profiles/openai.rs +++ b/lib/components/fabro-agent/src/profiles/openai.rs @@ -6,13 +6,13 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::apply_patch; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt}; +use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; use crate::todo_tools::make_update_plan_tool; use crate::tool_registry::ToolRegistry; -use crate::tools::{self, WebFetchSummarizer, register_core_tools}; +use crate::tools::{self, register_core_tools}; const CORE_PROMPT: &str = include_str!("prompts/openai.md.j2"); @@ -23,18 +23,15 @@ pub struct OpenAiProfile { impl OpenAiProfile { #[must_use] pub fn new(model: impl Into) -> Self { - let options = NativeToolOptions::for_profile(AgentProfileKind::OpenAi); - Self::with_native_tools(model, &options, None) + let deps = + ProfileDeps::standalone(NativeToolOptions::for_profile(AgentProfileKind::OpenAi)); + Self::with_native_tools(model, &deps) } - pub(crate) fn with_native_tools( - model: impl Into, - options: &NativeToolOptions, - summarizer: Option, - ) -> Self { + pub(crate) fn with_native_tools(model: impl Into, deps: &ProfileDeps) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, options, summarizer); + register_core_tools(&mut registry, &deps.options, deps.summarizer.clone()); registry.register(apply_patch::make_apply_patch_tool()); // Codex-compatible `update_plan` is OpenAI-only. let todo_runtime = Arc::new(TodoRuntime::new()); From 3c755a7d4e7244ea61ecc6bc6221543698f4424c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 26 Jul 2026 07:57:42 -0400 Subject: [PATCH 05/19] refactor(agent): give Claude 5 subagent tools fabro canonical names `NativeTool` documents itself as "an identity, not a name" whose canonical form is fabro's own vocabulary, with harness names layered on as aliases: `to_string = "read_file", serialize = "Read"`. The four Claude 5 subagent tools inverted that. `ClaudeAgent` declared `to_string = "Agent"`, making the Anthropic wire name the identity and leaving `name(ToolVocabulary::Fabro)` returning `"Agent"` -- and pairing a provider-specific variant name with a generic wire name. It also meant the `Claude5` arm listed none of them: they fell through to `canonical_name()` and were correct only by accident. Rename to `BackgroundAgent` / `AgentOutput` / `StopAgent` / `MessageAgent` with fabro canonical names, keep the harness names as `serialize` aliases so `from_any_name` still resolves them, and name them explicitly in the `Claude5` vocabulary arm. Also map `Grep`/`Glob` there: that arm describes the vocabulary rather than the profile's registry, and if either were ever registered it would otherwise reach the harness lowercased. Records why these are separate identities from `spawn_agent`/`wait`/`close_agent`/`send_input` rather than aliases of them, since the capabilities genuinely differ. Co-Authored-By: Claude Opus 5 (1M context) --- lib/components/fabro-agent/src/native_tool.rs | 72 +++++++++++++++---- .../fabro-agent/src/profiles/claude5.rs | 2 +- .../fabro-agent/src/profiles/claude5_tools.rs | 8 +-- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/lib/components/fabro-agent/src/native_tool.rs b/lib/components/fabro-agent/src/native_tool.rs index 83b1890ff..84061fbc8 100644 --- a/lib/components/fabro-agent/src/native_tool.rs +++ b/lib/components/fabro-agent/src/native_tool.rs @@ -73,14 +73,21 @@ pub enum NativeTool { Wait, #[strum(to_string = "close_agent")] CloseAgent, - #[strum(to_string = "Agent")] - ClaudeAgent, - #[strum(to_string = "TaskOutput")] - TaskOutput, - #[strum(to_string = "TaskStop")] - TaskStop, - #[strum(to_string = "SendMessage")] - SendMessage, + // Claude 5 drives one background agent through four tools, where fabro's + // own vocabulary uses `spawn_agent`/`wait`/`close_agent`/`send_input`. + // They are separate identities rather than aliases of those because the + // capabilities differ: `Agent` runs in the background or inline depending + // on `run_in_background`, and `TaskOutput` both polls and waits. Mapping + // them onto the fabro four would promise semantics those tools do not + // have -- the same reason Kimi Code's `Agent` is deliberately unmapped. + #[strum(to_string = "background_agent", serialize = "Agent")] + BackgroundAgent, + #[strum(to_string = "agent_output", serialize = "TaskOutput")] + AgentOutput, + #[strum(to_string = "stop_agent", serialize = "TaskStop")] + StopAgent, + #[strum(to_string = "message_agent", serialize = "SendMessage")] + MessageAgent, #[strum(to_string = "use_skill", serialize = "Skill")] UseSkill, #[strum(to_string = "update_plan")] @@ -135,9 +142,18 @@ impl NativeTool { Self::WriteFile => "Write", Self::EditFile => "Edit", Self::Shell => "Bash", + // Named for completeness: this arm describes the vocabulary, + // not the profile's registry, and the Claude 5 profile + // deliberately registers neither. + Self::Grep => "Grep", + Self::Glob => "Glob", Self::WebSearch => "WebSearch", Self::WebFetch => "WebFetch", Self::UseSkill => "Skill", + Self::BackgroundAgent => "Agent", + Self::AgentOutput => "TaskOutput", + Self::StopAgent => "TaskStop", + Self::MessageAgent => "SendMessage", other => other.canonical_name(), }, ToolVocabulary::KimiCode => match self { @@ -205,10 +221,10 @@ impl NativeTool { | Self::SendInput | Self::Wait | Self::CloseAgent - | Self::ClaudeAgent - | Self::TaskOutput - | Self::TaskStop - | Self::SendMessage => Some(AgentToolCategory::Subagent), + | Self::BackgroundAgent + | Self::AgentOutput + | Self::StopAgent + | Self::MessageAgent => Some(AgentToolCategory::Subagent), // Uncategorized today. Giving these a category would change the CLI // permission gate, which is a behavior change rather than a // classification cleanup, so they keep their existing answer. @@ -299,9 +315,39 @@ mod tests { "WebFetch" ); assert_eq!( - NativeTool::ClaudeAgent.name(ToolVocabulary::Claude5), + NativeTool::BackgroundAgent.name(ToolVocabulary::Claude5), "Agent" ); + assert_eq!( + NativeTool::AgentOutput.name(ToolVocabulary::Claude5), + "TaskOutput" + ); + assert_eq!( + NativeTool::StopAgent.name(ToolVocabulary::Claude5), + "TaskStop" + ); + assert_eq!( + NativeTool::MessageAgent.name(ToolVocabulary::Claude5), + "SendMessage" + ); + } + + /// The harness name is how a tool is expressed, not what it is: the + /// identity keeps a fabro name, and the harness name resolves back to it. + #[test] + fn claude5_subagent_tools_keep_fabro_canonical_names() { + for (tool, canonical, claude5) in [ + (NativeTool::BackgroundAgent, "background_agent", "Agent"), + (NativeTool::AgentOutput, "agent_output", "TaskOutput"), + (NativeTool::StopAgent, "stop_agent", "TaskStop"), + (NativeTool::MessageAgent, "message_agent", "SendMessage"), + ] { + assert_eq!(tool.canonical_name(), canonical); + assert_eq!(tool.name(ToolVocabulary::Fabro), canonical); + assert_eq!(tool.name(ToolVocabulary::Claude5), claude5); + assert_eq!(NativeTool::from_any_name(canonical), Some(tool)); + assert_eq!(NativeTool::from_any_name(claude5), Some(tool)); + } } #[test] diff --git a/lib/components/fabro-agent/src/profiles/claude5.rs b/lib/components/fabro-agent/src/profiles/claude5.rs index 43a2db179..875001c17 100644 --- a/lib/components/fabro-agent/src/profiles/claude5.rs +++ b/lib/components/fabro-agent/src/profiles/claude5.rs @@ -123,7 +123,7 @@ impl AgentProfile for Claude5Profile { "has_agent", self.base .registry - .get_native(NativeTool::ClaudeAgent) + .get_native(NativeTool::BackgroundAgent) .is_some(), ) .with_bool( diff --git a/lib/components/fabro-agent/src/profiles/claude5_tools.rs b/lib/components/fabro-agent/src/profiles/claude5_tools.rs index 663161bab..58278fa1c 100644 --- a/lib/components/fabro-agent/src/profiles/claude5_tools.rs +++ b/lib/components/fabro-agent/src/profiles/claude5_tools.rs @@ -187,7 +187,7 @@ pub(crate) fn make_agent_tool( ) -> RegisteredTool { RegisteredTool { definition: definition( - NativeTool::ClaudeAgent, + NativeTool::BackgroundAgent, "Launch a child agent for an independent task. Agents run in the background by \ default and notify the parent when they finish. Set run_in_background to false to \ wait for the result synchronously.", @@ -281,7 +281,7 @@ fn finished_output( pub(crate) fn make_task_output_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { RegisteredTool { definition: definition( - NativeTool::TaskOutput, + NativeTool::AgentOutput, "Get a background agent's current status or wait for its final output. Automatic \ completion notifications make ordinary polling unnecessary.", serde_json::json!({ @@ -368,7 +368,7 @@ pub(crate) fn make_task_output_tool(supervisor: SubAgentSupervisor) -> Registere pub(crate) fn make_task_stop_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { RegisteredTool { definition: definition( - NativeTool::TaskStop, + NativeTool::StopAgent, "Stop a running background agent by task ID.", serde_json::json!({ "type": "object", @@ -401,7 +401,7 @@ pub(crate) fn make_task_stop_tool(supervisor: SubAgentSupervisor) -> RegisteredT pub(crate) fn make_send_message_tool(supervisor: SubAgentSupervisor) -> RegisteredTool { RegisteredTool { definition: definition( - NativeTool::SendMessage, + NativeTool::MessageAgent, "Send additional instructions to a running background agent by its task ID.", serde_json::json!({ "type": "object", From 8b7d07b84be0c040ad1af5913b10c4a2b31dcc59 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 26 Jul 2026 08:00:21 -0400 Subject: [PATCH 06/19] refactor(agent): deduplicate the BaseProfile accessor delegation All six provider profiles embed a `BaseProfile` and hand-wrote the same six delegating accessors -- 24 identical lines each. What actually distinguishes them is `build_system_prompt`, and for Claude 5, `register_subagent_tools`. Replace the copies with one `impl_base_profile_accessors!()` invocation. A macro rather than trait defaults because three implementors have no `BaseProfile` to delegate to -- `TestProfile`, the workflow crate's `ShutdownTestProfile`, and the server's `AskFabroProfile` -- so a default would need a runtime fallback for a case the compiler can already rule out. Those three keep their hand-written accessors and are untouched. Co-Authored-By: Claude Opus 5 (1M context) --- .../fabro-agent/src/profiles/anthropic.rs | 28 ++----------- .../fabro-agent/src/profiles/claude5.rs | 28 ++----------- .../fabro-agent/src/profiles/gemini.rs | 28 ++----------- .../fabro-agent/src/profiles/gpt56.rs | 28 ++----------- .../fabro-agent/src/profiles/kimi.rs | 28 ++----------- .../fabro-agent/src/profiles/mod.rs | 39 +++++++++++++++++++ .../fabro-agent/src/profiles/openai.rs | 28 ++----------- 7 files changed, 63 insertions(+), 144 deletions(-) diff --git a/lib/components/fabro-agent/src/profiles/anthropic.rs b/lib/components/fabro-agent/src/profiles/anthropic.rs index a34358d81..338c141fa 100644 --- a/lib/components/fabro-agent/src/profiles/anthropic.rs +++ b/lib/components/fabro-agent/src/profiles/anthropic.rs @@ -5,7 +5,9 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, ProfileDeps, impl_base_profile_accessors, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_tools::{ @@ -68,29 +70,7 @@ impl AnthropicProfile { } impl AgentProfile for AnthropicProfile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, diff --git a/lib/components/fabro-agent/src/profiles/claude5.rs b/lib/components/fabro-agent/src/profiles/claude5.rs index 875001c17..ffa4b6ea1 100644 --- a/lib/components/fabro-agent/src/profiles/claude5.rs +++ b/lib/components/fabro-agent/src/profiles/claude5.rs @@ -8,7 +8,9 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps, claude5_tools}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, ProfileDeps, claude5_tools, impl_base_profile_accessors, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::subagent::{SessionFactory, SubAgentSupervisor}; @@ -85,29 +87,7 @@ impl Claude5Profile { } impl AgentProfile for Claude5Profile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, diff --git a/lib/components/fabro-agent/src/profiles/gemini.rs b/lib/components/fabro-agent/src/profiles/gemini.rs index c533a85e3..f6b3d494d 100644 --- a/lib/components/fabro-agent/src/profiles/gemini.rs +++ b/lib/components/fabro-agent/src/profiles/gemini.rs @@ -5,7 +5,9 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, ProfileDeps, impl_base_profile_accessors, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::tool_registry::ToolRegistry; @@ -62,29 +64,7 @@ impl GeminiProfile { } impl AgentProfile for GeminiProfile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, diff --git a/lib/components/fabro-agent/src/profiles/gpt56.rs b/lib/components/fabro-agent/src/profiles/gpt56.rs index 66fa5ddfd..d0fb93ed1 100644 --- a/lib/components/fabro-agent/src/profiles/gpt56.rs +++ b/lib/components/fabro-agent/src/profiles/gpt56.rs @@ -24,7 +24,9 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, FileEditToolKind, ProfileDeps}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, FileEditToolKind, ProfileDeps, impl_base_profile_accessors, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; @@ -179,29 +181,7 @@ fn make_shell_command_tool(options: &NativeToolOptions) -> RegisteredTool { } impl AgentProfile for Gpt56Profile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, diff --git a/lib/components/fabro-agent/src/profiles/kimi.rs b/lib/components/fabro-agent/src/profiles/kimi.rs index 1e5bb01de..b4010deb3 100644 --- a/lib/components/fabro-agent/src/profiles/kimi.rs +++ b/lib/components/fabro-agent/src/profiles/kimi.rs @@ -6,7 +6,9 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::config::NativeToolOptions; use crate::native_tool::{NativeTool, ToolVocabulary}; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps, kimi_tools}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, ProfileDeps, impl_base_profile_accessors, kimi_tools, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; @@ -116,29 +118,7 @@ impl KimiProfile { } impl AgentProfile for KimiProfile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index 23168f13c..01e851302 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -199,6 +199,45 @@ impl FileEditToolKind { } } +/// Implement the [`AgentProfile`](crate::agent_profile::AgentProfile) +/// accessors that just delegate to an embedded [`BaseProfile`] named `base`. +/// +/// Every profile that owns a `BaseProfile` writes the same six methods; what +/// actually distinguishes them is `build_system_prompt` and, for some, +/// `register_subagent_tools`. Types that implement the trait without a +/// `BaseProfile` -- test doubles, and the server's ask-fabro profile -- write +/// the accessors themselves, which is why this is a macro rather than a set of +/// trait defaults: there is no sensible default for a profile that has no base. +macro_rules! impl_base_profile_accessors { + () => { + fn profile_kind(&self) -> ::fabro_model::AgentProfileKind { + self.base.profile_kind + } + + fn provider_id(&self) -> ::fabro_model::ProviderId { + self.base.provider_id.clone() + } + + fn model(&self) -> &str { + &self.base.model + } + + fn catalog(&self) -> Option<&::fabro_model::Catalog> { + self.base.catalog.as_deref() + } + + fn tool_registry(&self) -> &$crate::tool_registry::ToolRegistry { + &self.base.registry + } + + fn tool_registry_mut(&mut self) -> &mut $crate::tool_registry::ToolRegistry { + &mut self.base.registry + } + }; +} + +pub(crate) use impl_base_profile_accessors; + /// Common fields shared by all provider profiles. /// /// Each concrete profile embeds this struct and delegates `profile_kind()`, diff --git a/lib/components/fabro-agent/src/profiles/openai.rs b/lib/components/fabro-agent/src/profiles/openai.rs index 2f5bbf4df..5f1029e87 100644 --- a/lib/components/fabro-agent/src/profiles/openai.rs +++ b/lib/components/fabro-agent/src/profiles/openai.rs @@ -6,7 +6,9 @@ use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::apply_patch; use crate::config::NativeToolOptions; -use crate::profiles::{self, BaseProfile, EmbeddedPrompt, ProfileDeps}; +use crate::profiles::{ + self, BaseProfile, EmbeddedPrompt, ProfileDeps, impl_base_profile_accessors, +}; use crate::sandbox::Sandbox; use crate::skills::Skill; use crate::todo_runtime::TodoRuntime; @@ -59,29 +61,7 @@ impl OpenAiProfile { } impl AgentProfile for OpenAiProfile { - fn profile_kind(&self) -> AgentProfileKind { - self.base.profile_kind - } - - fn provider_id(&self) -> ProviderId { - self.base.provider_id.clone() - } - - fn model(&self) -> &str { - &self.base.model - } - - fn catalog(&self) -> Option<&Catalog> { - self.base.catalog.as_deref() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.base.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.base.registry - } + impl_base_profile_accessors!(); fn build_system_prompt( &self, From 73051c9a8adad0385bc37dd3069828091298fdbf Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 26 Jul 2026 08:04:24 -0400 Subject: [PATCH 07/19] refactor(agent): share one normalizer between the question tools `Claude5QuestionToolArgs`/`Claude5Question`/`Claude5Option` differed from the Anthropic trio only in required-ness -- `header: String` rather than `Option`, same for each option's `description`. The JSON Schema already enforces that at the model boundary, so the lenient structs deserialize the strict payload unchanged. `normalize_claude5_questions` then reproduced `normalize_anthropic_questions` plus an inlined copy of `options_from_anthropic`, so `option_key`, `display_text`, and `bounded_display_field` were each applied in two places and could drift. Replace both with one normalizer taking a `QuestionLimits`. The genuine Claude 5 deltas -- at most four questions, two to four options, a twelve-character header cap, required header and option descriptions, and no previews on multi-select -- become data rather than a second code path. Two rules serde used to enforce are now the normalizer's: a missing header and a missing option description. Both are still rejected, with a clearer message than serde's "missing field". `multiSelect` now defaults to false instead of being a deserialization error; the schema still marks it required, which is where that contract belongs. Adds tests pinning the strict rules against the shared normalizer, and one asserting the lenient contract still accepts optional headers and descriptions. Co-Authored-By: Claude Opus 5 (1M context) --- .../fabro-agent/src/question_tools.rs | 276 ++++++++++++------ 1 file changed, 183 insertions(+), 93 deletions(-) diff --git a/lib/components/fabro-agent/src/question_tools.rs b/lib/components/fabro-agent/src/question_tools.rs index 3fcc2980f..63eb58522 100644 --- a/lib/components/fabro-agent/src/question_tools.rs +++ b/lib/components/fabro-agent/src/question_tools.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::future::Future; +use std::ops::RangeInclusive; use std::sync::Arc; use async_trait::async_trait; @@ -149,27 +150,41 @@ struct AnthropicOption { preview: Option, } -#[derive(Debug, Deserialize)] -struct Claude5QuestionToolArgs { - questions: Vec, +/// Contract rules the JSON Schema cannot express, and which differ between +/// the two harnesses sharing one normalizer. +struct QuestionLimits { + questions: RangeInclusive, + questions_error: &'static str, + /// `None` leaves the option count unbounded. + options: Option>, + options_error: &'static str, + max_header_chars: Option, + /// Claude 5's schema marks `header` and every option `description` + /// required, so both are validated rather than passed through as given. + require_header_and_descriptions: bool, + /// Claude 5 renders multi-select without a preview pane. + allow_preview_with_multi_select: bool, } -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -struct Claude5Question { - question: String, - header: String, - options: Vec, - multi_select: bool, -} +const ANTHROPIC_QUESTION_LIMITS: QuestionLimits = QuestionLimits { + questions: 1..=usize::MAX, + questions_error: "questions must contain at least one question", + options: None, + options_error: "", + max_header_chars: None, + require_header_and_descriptions: false, + allow_preview_with_multi_select: true, +}; -#[derive(Debug, Deserialize)] -struct Claude5Option { - label: String, - description: String, - #[serde(default)] - preview: Option, -} +const CLAUDE5_QUESTION_LIMITS: QuestionLimits = QuestionLimits { + questions: 1..=4, + questions_error: "questions must contain between one and four questions", + options: Some(2..=4), + options_error: "each question must contain between two and four options", + max_header_chars: Some(12), + require_header_and_descriptions: true, + allow_preview_with_multi_select: false, +}; #[must_use] pub fn is_question_tool(name: &str) -> bool { @@ -285,7 +300,8 @@ fn make_anthropic_question_tool() -> RegisteredTool { executor: Arc::new(|args, ctx| { Box::pin(async move { let parsed: AnthropicQuestionToolArgs = parse_tool_args(args)?; - let questions = normalize_anthropic_questions(parsed)?; + let questions = + normalize_anthropic_questions(parsed, &ANTHROPIC_QUESTION_LIMITS)?; let answers = execute_question_tool(ctx, questions).await?; format_anthropic_answers(&answers) }) @@ -360,8 +376,9 @@ fn make_claude5_question_tool() -> RegisteredTool { }, executor: Arc::new(|args, ctx| { Box::pin(async move { - let parsed: Claude5QuestionToolArgs = parse_tool_args(args)?; - let questions = normalize_claude5_questions(parsed)?; + let parsed: AnthropicQuestionToolArgs = parse_tool_args(args)?; + let questions = + normalize_anthropic_questions(parsed, &CLAUDE5_QUESTION_LIMITS)?; let answers = execute_question_tool(ctx, questions).await?; format_anthropic_answers(&answers) }) @@ -426,50 +443,42 @@ fn normalize_openai_questions(args: OpenAiQuestionToolArgs) -> Result Result, String> { - if args.questions.is_empty() { - return Err("questions must contain at least one question".to_string()); - } - args.questions - .into_iter() - .map(|question| { - let original_question = non_empty(&question.question, "question")?; - Ok(AgentQuestion { - original_id: None, - text: display_text(question.header.as_deref(), &question.question), - header: question.header, - original_question, - question_type: if question.multi_select { - QuestionType::MultiSelect - } else { - QuestionType::MultipleChoice - }, - options: options_from_anthropic(question.options), - allow_freeform: true, - }) - }) - .collect() -} - -fn normalize_claude5_questions( - args: Claude5QuestionToolArgs, -) -> Result, String> { - if !(1..=4).contains(&args.questions.len()) { - return Err("questions must contain between one and four questions".to_string()); + if !limits.questions.contains(&args.questions.len()) { + return Err(limits.questions_error.to_string()); } args.questions .into_iter() .map(|question| { let original_question = non_empty(&question.question, "question")?; - let header = non_empty(&question.header, "question header")?; - if header.chars().count() > 12 { - return Err("question header must contain at most 12 characters".to_string()); + let header = if limits.require_header_and_descriptions { + let header = non_empty( + question.header.as_deref().unwrap_or_default(), + "question header", + )?; + if limits + .max_header_chars + .is_some_and(|max| header.chars().count() > max) + { + return Err(format!( + "question header must contain at most {} characters", + limits.max_header_chars.unwrap_or_default() + )); + } + Some(header) + } else { + question.header + }; + + if let Some(bounds) = &limits.options { + if !bounds.contains(&question.options.len()) { + return Err(limits.options_error.to_string()); + } } - if !(2..=4).contains(&question.options.len()) { - return Err("each question must contain between two and four options".to_string()); - } - if question.multi_select + if !limits.allow_preview_with_multi_select + && question.multi_select && question .options .iter() @@ -480,36 +489,25 @@ fn normalize_claude5_questions( ); } - let options = question - .options - .into_iter() - .enumerate() - .map(|(idx, option)| { - Ok(InterviewOption { - key: option_key(idx), - label: non_empty(&option.label, "option label")?, - description: Some(bounded_display_field( - &non_empty(&option.description, "option description")?, - OPTION_DESCRIPTION_MAX_CHARS, - )), - preview: option - .preview - .map(|value| bounded_display_field(&value, OPTION_PREVIEW_MAX_CHARS)), - }) - }) - .collect::, String>>()?; + // The lenient contract renders the question and header exactly as + // supplied; the strict one has already trimmed them. + let text = if limits.require_header_and_descriptions { + display_text(header.as_deref(), &original_question) + } else { + display_text(header.as_deref(), &question.question) + }; Ok(AgentQuestion { original_id: None, - text: display_text(Some(&header), &original_question), - header: Some(header), + text, + header, original_question, question_type: if question.multi_select { QuestionType::MultiSelect } else { QuestionType::MultipleChoice }, - options, + options: options_from_anthropic(question.options, limits)?, allow_freeform: true, }) }) @@ -531,19 +529,34 @@ fn options_from_openai(options: Vec) -> Vec { .collect() } -fn options_from_anthropic(options: Vec) -> Vec { +fn options_from_anthropic( + options: Vec, + limits: &QuestionLimits, +) -> Result, String> { options .into_iter() .enumerate() - .map(|(idx, option)| InterviewOption { - key: option_key(idx), - label: option.label, - description: option - .description - .map(|value| bounded_display_field(&value, OPTION_DESCRIPTION_MAX_CHARS)), - preview: option - .preview - .map(|value| bounded_display_field(&value, OPTION_PREVIEW_MAX_CHARS)), + .map(|(idx, option)| { + let (label, description) = if limits.require_header_and_descriptions { + ( + non_empty(&option.label, "option label")?, + Some(non_empty( + option.description.as_deref().unwrap_or_default(), + "option description", + )?), + ) + } else { + (option.label, option.description) + }; + Ok(InterviewOption { + key: option_key(idx), + label, + description: description + .map(|value| bounded_display_field(&value, OPTION_DESCRIPTION_MAX_CHARS)), + preview: option + .preview + .map(|value| bounded_display_field(&value, OPTION_PREVIEW_MAX_CHARS)), + }) }) .collect() } @@ -696,7 +709,7 @@ mod tests { })) .unwrap(); - let questions = normalize_anthropic_questions(args).unwrap(); + let questions = normalize_anthropic_questions(args, &ANTHROPIC_QUESTION_LIMITS).unwrap(); assert_eq!(questions[0].question_type, QuestionType::MultiSelect); assert_eq!( @@ -794,7 +807,7 @@ mod tests { #[test] fn claude5_question_contract_is_strict_and_preserves_preview() { - let args: Claude5QuestionToolArgs = serde_json::from_value(json!({ + let args: AnthropicQuestionToolArgs = serde_json::from_value(json!({ "questions": [{ "header": "Approach", "question": "Which approach should we use?", @@ -814,7 +827,7 @@ mod tests { })) .unwrap(); - let questions = normalize_claude5_questions(args).unwrap(); + let questions = normalize_anthropic_questions(args, &CLAUDE5_QUESTION_LIMITS).unwrap(); assert_eq!(questions[0].header.as_deref(), Some("Approach")); assert_eq!( @@ -824,9 +837,86 @@ mod tests { assert!(questions[0].allow_freeform); } + /// The Claude 5 payload is deserialized through the lenient struct now, so + /// the rules its own struct used to enforce are the normalizer's job. + #[test] + fn claude5_limits_reject_what_the_lenient_contract_allows() { + let question = |patch: serde_json::Value| { + let mut base = json!({ + "question": "Which approach?", + "header": "Approach", + "multiSelect": false, + "options": [ + {"label": "First", "description": "One"}, + {"label": "Second", "description": "Two"} + ] + }); + let object = base.as_object_mut().unwrap(); + for (key, value) in patch.as_object().unwrap() { + if value.is_null() { + object.remove(key); + } else { + object.insert(key.clone(), value.clone()); + } + } + base + }; + let normalize = |questions: serde_json::Value| { + let args: AnthropicQuestionToolArgs = + serde_json::from_value(json!({"questions": questions})).unwrap(); + normalize_anthropic_questions(args, &CLAUDE5_QUESTION_LIMITS) + }; + + // A missing header and a missing option description used to be caught + // by serde; the normalizer has to reject them now. + assert!(normalize(json!([question(json!({"header": null}))])).is_err()); + assert!( + normalize(json!([question(json!({ + "options": [{"label": "First"}, {"label": "Second"}] + }))])) + .is_err() + ); + + assert!( + normalize(json!([question(json!({"header": "ThirteenChars"}))])).is_err(), + "header longer than 12 characters" + ); + assert!( + normalize(json!([question(json!({ + "options": [{"label": "Only", "description": "One"}] + }))])) + .is_err(), + "fewer than two options" + ); + assert!( + normalize(json!(vec![question(json!({})); 5])).is_err(), + "more than four questions" + ); + + assert!(normalize(json!([question(json!({}))])).is_ok()); + } + + /// The same payloads stay acceptable under the lenient contract, so the + /// shared normalizer has not tightened the Anthropic tool. + #[test] + fn anthropic_limits_still_accept_optional_headers_and_descriptions() { + let args: AnthropicQuestionToolArgs = serde_json::from_value(json!({ + "questions": [{ + "question": "Which approach?", + "options": [{"label": "First"}] + }] + })) + .unwrap(); + + let questions = normalize_anthropic_questions(args, &ANTHROPIC_QUESTION_LIMITS).unwrap(); + assert_eq!(questions.len(), 1); + assert_eq!(questions[0].header, None); + assert_eq!(questions[0].options[0].description, None); + } + #[test] fn claude5_rejects_previews_for_multi_select_questions() { - let args: Claude5QuestionToolArgs = serde_json::from_value(json!({ + let args: AnthropicQuestionToolArgs = serde_json::from_value(json!({ "questions": [{ "header": "Features", "question": "Which features should we enable?", @@ -846,7 +936,7 @@ mod tests { })) .unwrap(); - assert!(normalize_claude5_questions(args).is_err()); + assert!(normalize_anthropic_questions(args, &CLAUDE5_QUESTION_LIMITS).is_err()); } #[tokio::test] From c812274db8f88313421e99118e0cf476ea114f17 Mon Sep 17 00:00:00 2001 From: Release Repro Date: Mon, 27 Jul 2026 17:08:08 -0400 Subject: [PATCH 08/19] Show live status for parallel branches --- .../parallel-children.test.tsx | 247 ++++++++++++++---- .../stage-renderers/parallel-children.tsx | 115 +++++--- apps/fabro-web/app/lib/stage-sidebar.test.ts | 24 ++ apps/fabro-web/app/lib/stage-sidebar.ts | 6 + docs/public/api-reference/fabro-api.yaml | 22 ++ lib/apps/fabro-server/src/demo/mod.rs | 2 + .../src/server/handler/billing.rs | 7 + lib/apps/fabro-server/src/server/tests.rs | 79 +++++- lib/components/fabro-store/src/run_state.rs | 59 ++++- .../tests/stage_projection_round_trip.rs | 11 +- .../fabro-types/src/run_projection.rs | 10 +- .../fabro-api-client/src/models/run-stage.ts | 8 + .../src/models/stage-projection.ts | 4 + 13 files changed, 497 insertions(+), 97 deletions(-) diff --git a/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx b/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx index f1313b780..beee51a41 100644 --- a/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx +++ b/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { StageState } from "@qltysh/fabro-api-client"; import type { EventEnvelope } from "@qltysh/fabro-api-client"; import TestRenderer, { act } from "react-test-renderer"; import { MemoryRouter } from "react-router"; @@ -13,35 +14,98 @@ beforeEach(() => { }); afterEach(() => teardown()); -const parallelStage: Stage = { +function makeStage(overrides: Partial = {}): Stage { + return { + id: "stage@1", + name: "stage", + handler: "agent", + status: StageState.RUNNING, + duration: "--", + nodeId: "stage", + visit: 1, + graphVisit: 1, + resumedFromStageId: null, + parallelGroupId: null, + parallelBranchIndex: null, + startedAt: "2026-04-09T12:00:00Z", + providerUsed: null, + ...overrides, + }; +} + +const parallelStage = makeStage({ id: "fork@1", name: "fork", handler: "parallel", - status: "succeeded", + status: StageState.RUNNING, duration: "12s", nodeId: "fork", - visit: 1, - startedAt: "2026-04-09T12:00:00Z", - providerUsed: null, -}; +}); -function event(partial: Partial): EventEnvelope { - return makeEventEnvelope(partial.seq ?? 1, { event: "parallel.completed", ...partial }); +function branchStage( + name: string, + index: number, + status: StageState, + groupId = "fork@1", + visit = 1, +): Stage { + return makeStage({ + id: `${name}@${visit}`, + name, + nodeId: name, + visit, + status, + parallelGroupId: groupId, + parallelBranchIndex: index, + }); } -function renderParallel(events: EventEnvelope[]): TestRenderer.ReactTestRenderer { +function event(partial: Partial): EventEnvelope { + return makeEventEnvelope(partial.seq ?? 1, { + event: "parallel.completed", + stage_id: "fork@1", + ...partial, + }); +} + +function startedEvent(branchCount: number): EventEnvelope { + return event({ + event: "parallel.started", + properties: { branch_count: branchCount }, + }); +} + +function completedEvent( + results: Array<{ id: string; status: string }>, + successCount: number, + failureCount: number, +): EventEnvelope { + return event({ + seq: 2, + event: "parallel.completed", + properties: { + duration_ms: 12000, + success_count: successCount, + failure_count: failureCount, + results: results.map((result) => ({ ...result, context_updates: {} })), + }, + }); +} + +function renderParallel( + events: EventEnvelope[], + allStages: Stage[], + stage = parallelStage, +): TestRenderer.ReactTestRenderer { let renderer!: TestRenderer.ReactTestRenderer; act(() => { renderer = TestRenderer.create( , ); @@ -49,37 +113,132 @@ function renderParallel(events: EventEnvelope[]): TestRenderer.ReactTestRenderer return renderer; } -describe("ParallelChildren", () => { - test("renders branch status and stage links without checkout metadata", () => { - const renderer = renderParallel([ - event({ - event: "parallel.started", - properties: { branch_count: 2 }, - }), - event({ - seq: 2, - event: "parallel.completed", - properties: { - duration_ms: 12000, - success_count: 1, - failure_count: 1, - results: [ - { id: "branch-a", status: "succeeded", context_updates: {} }, - { id: "branch-b", status: "failed", context_updates: {} }, - ], - }, - }), - ]); +function textContent(node: TestRenderer.ReactTestInstance): string { + return node.children + .map((child) => typeof child === "string" ? child : textContent(child)) + .join(""); +} - const rendered = JSON.stringify(renderer.toJSON()); - expect(rendered).toContain("branch-a"); - expect(rendered).toContain("Succeeded"); - expect(rendered).toContain("branch-b"); - expect(rendered).toContain("Failed"); - const hrefs = renderer.root.findAllByType("a").map((link) => link.props.href); - expect(hrefs).toEqual([ - "/runs/run-1/stages/branch-a@1", - "/runs/run-1/stages/branch-b@1", +function branchRowText(renderer: TestRenderer.ReactTestRenderer): string[] { + return renderer.root.findAllByType("li").map(textContent); +} + +function hrefs(renderer: TestRenderer.ReactTestRenderer): string[] { + return renderer.root.findAllByType("a").map((link) => link.props.href); +} + +function statValue(renderer: TestRenderer.ReactTestRenderer, label: string): string { + const stat = renderer.root + .findAllByProps({ className: "flex flex-col gap-0.5" }) + .find((item) => textContent(item).startsWith(label)); + if (!stat) throw new Error(`stat ${label} not found`); + return textContent(stat.findAllByType("span")[1]); +} + +describe("ParallelChildren", () => { + test("renders live branch names, statuses, counts, and stage links", () => { + const renderer = renderParallel( + [startedEvent(2)], + [ + branchStage("review_glm", 0, StageState.SUCCEEDED), + branchStage("review_opus", 1, StageState.RUNNING), + ], + ); + + const rows = branchRowText(renderer); + expect(rows).toHaveLength(2); + expect(rows[0]).toContain("Succeeded"); + expect(rows[0]).toContain("review_glm"); + expect(rows[1]).toContain("Running"); + expect(rows[1]).toContain("review_opus"); + expect(hrefs(renderer)).toEqual([ + "/runs/run-1/stages/review_glm@1", + "/runs/run-1/stages/review_opus@1", ]); + expect(statValue(renderer, "Succeeded")).toBe("1"); + expect(statValue(renderer, "Failed")).toBe("0"); + }); + + test("keeps looped fork links scoped to the selected fork visit", () => { + const renderer = renderParallel( + [startedEvent(1)], + [ + branchStage("review_glm", 0, StageState.SUCCEEDED, "fork@1", 1), + branchStage("review_glm", 0, StageState.RUNNING, "fork@2", 2), + ], + ); + + expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_glm@1"]); + }); + + test("keeps duplicate branch targets in index order and only links recorded stages", () => { + const renderer = renderParallel( + [ + startedEvent(2), + completedEvent( + [ + { id: "review", status: "failed" }, + { id: "review", status: "failed" }, + ], + 1, + 1, + ), + ], + [branchStage("review", 0, StageState.SUCCEEDED)], + ); + + const rows = branchRowText(renderer); + expect(rows).toHaveLength(2); + expect(rows[0]).toContain("Succeeded"); + expect(rows[1]).toContain("Failed"); + expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review@1"]); + }); + + test("renders a completed result without a matching stage as an unlinked row", () => { + const renderer = renderParallel( + [ + startedEvent(1), + completedEvent( + [{ id: "legacy_branch", status: "succeeded" }], + 1, + 0, + ), + ], + [], + ); + + expect(branchRowText(renderer)).toEqual(["Succeededlegacy_branch"]); + expect(hrefs(renderer)).toEqual([]); + }); + + test("counts partial and skipped branches as neither succeeded nor failed", () => { + const allStages = [ + branchStage("partial", 0, StageState.PARTIALLY_SUCCEEDED), + branchStage("skipped", 1, StageState.SKIPPED), + ]; + const running = renderParallel([startedEvent(2)], allStages); + const completed = renderParallel( + [ + startedEvent(2), + completedEvent( + [ + { id: "partial", status: "partially_succeeded" }, + { id: "skipped", status: "skipped" }, + ], + 0, + 0, + ), + ], + allStages, + ); + + expect([ + statValue(running, "Succeeded"), + statValue(running, "Failed"), + ]).toEqual(["0", "0"]); + expect([ + statValue(completed, "Succeeded"), + statValue(completed, "Failed"), + ]).toEqual(["0", "0"]); }); }); diff --git a/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx b/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx index 30cb6169c..d9b142046 100644 --- a/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx +++ b/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx @@ -10,10 +10,12 @@ import { formatDurationMs } from "../../lib/format"; import { StageMetaBar } from "./meta-bar"; import { parseParallelOverview } from "./helpers"; -/** Branch row view state: completed outcomes plus a synthesized in-flight row. */ +/** Branch row view state sourced from a live branch stage or completed result. */ interface BranchRow { + branchIndex: number; id: string; status: StageState; + stageHref: string | null; } function StatItem({ @@ -38,25 +40,23 @@ function StatItem({ } function ChildRow({ - result, - stageHref, + row, }: { - result: BranchRow; - stageHref: string | null; + row: BranchRow; }) { - const tone = stageStatusTone(result.status); + const tone = stageStatusTone(row.status); const inner = ( <> - {stageStatusLabel(result.status)} + {stageStatusLabel(row.status)} - {result.id} + {row.id} - {stageHref && ( + {row.stageHref && (