diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index c37092b83..6bdaadd36 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -14,13 +14,12 @@ use fabro_agent::config::{ToolAccess, ToolAccessPolicy, ToolExposureMode}; use fabro_agent::profiles::assemble_system_prompt; use fabro_agent::tool_registry::ToolRegistry; use fabro_agent::{ - AgentEvent, AgentProfile, AnthropicProfile, Error as AgentError, GeminiProfile, OpenAiProfile, - Session, SessionEvent, SessionOptions, ToolSecrets, WebFetchSummarizer, + AgentEvent, AgentProfile, AgentProfileBuilder, Error as AgentError, Session, SessionEvent, + SessionOptions, ToolSecrets, WebFetchSummarizer, }; use fabro_api::types::{ CreateRunSessionRequest, PaginatedEventList, PaginationMeta, SubmitTurnRequest, }; -use fabro_llm::client::Client as LlmClient; use fabro_llm::types::ToolDefinition; use fabro_model::{ AgentProfileKind, Catalog, ModelHandle, ModelSelectionError, ProviderId, catalog, @@ -732,13 +731,21 @@ async fn build_agent_session( .await .map_err(AskFabroBuildError::SandboxUnavailable)?; let sandbox: Arc = Arc::from(sandbox); - let mut profile = build_profile( - provider_id, - profile_kind, - &model, - &llm_result.client, - Arc::clone(&catalog), - ); + let brave_search_api_key = state + .vault_secret(EnvVars::BRAVE_SEARCH_API_KEY) + .await + .map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err)))?; + let summarizer = WebFetchSummarizer { + client: llm_result.client.clone(), + model_id: summarizer_model_id(&provider_id, profile_kind, &catalog, &model), + }; + let mut profile = + AgentProfileBuilder::new(profile_kind, provider_id, &model, Arc::clone(&catalog)) + .with_web_fetch_summarizer(Some(summarizer)) + .with_tool_secrets(ToolSecrets { + brave_search_api_key, + }) + .build(); // Give the Ask Fabro agent access to read-only run-inspection tools scoped // to its owning run. The session reaches the local HTTP API via a same-run @@ -771,16 +778,9 @@ async fn build_agent_session( let profile: Arc = Arc::new(AskFabroProfile::new(profile, Arc::clone(&ask_fabro_policy))); - let brave_search_api_key = state - .vault_secret(EnvVars::BRAVE_SEARCH_API_KEY) - .await - .map_err(|err| AskFabroBuildError::Agent(anyhow::Error::new(err)))?; let config = SessionOptions { tool_access_policy: Some(ask_fabro_policy), tool_exposure_mode: ToolExposureMode::AutoApprovedOnly, - tool_secrets: ToolSecrets { - brave_search_api_key, - }, ..SessionOptions::default() }; @@ -917,36 +917,6 @@ fn session_selection_error(error: &ModelSelectionError) -> ApiError { ApiError::bad_request(error.to_string()) } -fn build_profile( - provider_id: ProviderId, - profile_kind: AgentProfileKind, - model: &str, - llm_client: &LlmClient, - catalog: Arc, -) -> Box { - let summarizer = Some(WebFetchSummarizer { - client: llm_client.clone(), - model_id: summarizer_model_id(&provider_id, profile_kind, &catalog, model), - }); - match profile_kind { - AgentProfileKind::OpenAi => Box::new( - OpenAiProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Gemini => Box::new( - GeminiProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Anthropic => Box::new( - AnthropicProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - } -} - fn summarizer_model_id( provider_id: &ProviderId, profile_kind: AgentProfileKind, diff --git a/lib/components/fabro-agent/README.md b/lib/components/fabro-agent/README.md index 69764459e..147605ea6 100644 --- a/lib/components/fabro-agent/README.md +++ b/lib/components/fabro-agent/README.md @@ -73,9 +73,13 @@ pub trait AgentProfile: Send + Sync { ``` Built-in profiles: -- **`AnthropicProfile`** -- 200K context, extended thinking beta headers, tools: `read_file`, `write_file`, `edit_file`, `shell`, `grep`, `glob` -- **`OpenAiProfile`** -- 128K context, reasoning effort support, tools: `read_file`, `write_file`, `shell`, `grep`, `glob`, `apply_patch` (Codex apply_patch format) -- **`GeminiProfile`** -- 1M context, safety settings, tools: all Anthropic tools plus `read_many_files`, `list_dir`, `web_search`, `web_fetch` +- **`AnthropicProfile`** -- 200K context, extended thinking beta headers, and Anthropic task tools +- **`OpenAiProfile`** -- 128K context, reasoning effort support, and `apply_patch` (Codex apply_patch format) +- **`GeminiProfile`** -- 1M context, safety settings, plus `read_many_files` and `list_dir` + +All profiles include the common file, shell, search, and `web_fetch` tools. +`web_search` is included only when a Brave Search API key is supplied while +building the profile. ### `Sandbox` diff --git a/lib/components/fabro-agent/src/cli.rs b/lib/components/fabro-agent/src/cli.rs index d51c8336e..c2f3423ce 100644 --- a/lib/components/fabro-agent/src/cli.rs +++ b/lib/components/fabro-agent/src/cli.rs @@ -31,10 +31,10 @@ use crate::config::{ToolApprovalAdapter, ToolApprovalFn, ToolHookCallback, ToolS use crate::error::InterruptReason; use crate::subagent::{SessionFactory, SubAgentSupervisor}; use crate::tool_permissions::{is_auto_approved, tool_category}; -use crate::tools::{self, WebFetchSummarizer}; +use crate::tools::WebFetchSummarizer; use crate::{ - AgentEvent, AgentProfile, AnthropicProfile, GeminiProfile, LocalSandbox, Message, - OpenAiProfile, Sandbox, Session, SessionOptions, SessionShutdownReason, + AgentEvent, AgentProfile, AgentProfileBuilder, LocalSandbox, Message, Sandbox, Session, + SessionOptions, SessionShutdownReason, }; #[expect( @@ -223,32 +223,6 @@ fn build_summarizer( } } -fn build_profile( - profile_kind: AgentProfileKind, - provider_id: ProviderId, - model: &str, - summarizer: Option, - catalog: Arc, -) -> Box { - match profile_kind { - AgentProfileKind::OpenAi => Box::new( - OpenAiProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Gemini => Box::new( - GeminiProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Anthropic => Box::new( - AnthropicProfile::with_summarizer(model, summarizer) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - } -} - fn parse_provider(args: &AgentArgs) -> anyhow::Result { let provider_str = args.provider.as_deref().unwrap_or("anthropic"); Ok(provider_str.parse()?) @@ -559,18 +533,21 @@ pub async fn run_with_args_and_client_and_catalog( }; let profile_kind = profile_kind_for_provider(&catalog, &provider_id, Some(&model))?; eprintln!("{}", styles.dim.apply_to(format!("Using model: {model}"))); - let mut profile = build_profile( + let tool_secrets = cli_tool_secrets(); + let profile_builder = AgentProfileBuilder::new( profile_kind, provider_id.clone(), &model, - Some(build_summarizer( - &provider_id, - &model, - &catalog, - client.clone(), - )), Arc::clone(&catalog), - ); + ) + .with_web_fetch_summarizer(Some(build_summarizer( + &provider_id, + &model, + &catalog, + client.clone(), + ))) + .with_tool_secrets(tool_secrets); + let mut profile = profile_builder.clone().build(); // Build sandbox let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); @@ -594,41 +571,19 @@ pub async fn run_with_args_and_client_and_catalog( permission_level: Some(permissions), skill_dirs: args.skills_dir.map(|d| vec![d]), mcp_servers, - tool_secrets: cli_tool_secrets(), ..SessionOptions::default() }; - tools::register_secret_backed_tools(profile.tool_registry_mut(), &config.tool_secrets); // Register subagent tools let supervisor = SubAgentSupervisor::new(config.max_subagent_depth); let supervisor_for_session = supervisor.clone(); let factory_client = client.clone(); - let factory_model = model.clone(); - let factory_catalog = Arc::clone(&catalog); - let factory_provider_id = provider_id.clone(); - let factory_profile_kind = profile_kind; + let factory_profile_builder = profile_builder; let factory_env = Arc::clone(&env); let factory_hooks = config.tool_hooks.clone(); let factory_permission_level = config.permission_level; - let factory_tool_secrets = config.tool_secrets.clone(); let factory: SessionFactory = Arc::new(move || { - let child_summarizer = Some(build_summarizer( - &factory_provider_id, - &factory_model, - &factory_catalog, - factory_client.clone(), - )); - let mut child_profile = build_profile( - factory_profile_kind, - factory_provider_id.clone(), - &factory_model, - child_summarizer, - Arc::clone(&factory_catalog), - ); - tools::register_secret_backed_tools( - child_profile.tool_registry_mut(), - &factory_tool_secrets, - ); + let child_profile = factory_profile_builder.clone().build(); let child_profile: Arc = Arc::from(child_profile); Session::new( factory_client.clone(), @@ -637,7 +592,6 @@ pub async fn run_with_args_and_client_and_catalog( SessionOptions { tool_hooks: factory_hooks.clone(), permission_level: factory_permission_level, - tool_secrets: factory_tool_secrets.clone(), ..SessionOptions::default() }, None, @@ -965,38 +919,10 @@ mod tests { assert!(approval_fn("shell", &json!({})).is_ok()); } - // build_profile tests - fn test_catalog() -> Arc { Arc::new(Catalog::from_builtin().unwrap()) } - #[test] - fn build_profile_anthropic() { - let profile = build_profile( - AgentProfileKind::Anthropic, - ProviderId::anthropic(), - "model", - None, - test_catalog(), - ); - assert_eq!(profile.profile_kind(), AgentProfileKind::Anthropic); - assert_eq!(profile.provider_id(), ProviderId::anthropic()); - } - - #[test] - fn build_profile_openai() { - let profile = build_profile( - AgentProfileKind::OpenAi, - ProviderId::openai(), - "model", - None, - test_catalog(), - ); - assert_eq!(profile.profile_kind(), AgentProfileKind::OpenAi); - assert_eq!(profile.provider_id(), ProviderId::openai()); - } - #[test] fn ensure_provider_registered_reports_missing_credentials() { let client = Client::new(HashMap::new(), None, vec![]); @@ -1007,19 +933,6 @@ mod tests { ); } - #[test] - fn build_profile_gemini() { - let profile = build_profile( - AgentProfileKind::Gemini, - ProviderId::gemini(), - "model", - None, - test_catalog(), - ); - assert_eq!(profile.profile_kind(), AgentProfileKind::Gemini); - assert_eq!(profile.provider_id(), ProviderId::gemini()); - } - #[test] fn profile_kind_accepts_custom_catalog_provider() { let mut settings = LlmCatalogSettings::default(); @@ -1232,13 +1145,13 @@ mod tests { #[test] fn build_profile_can_register_subagent_tools() { - let mut profile = build_profile( + let mut profile = AgentProfileBuilder::new( AgentProfileKind::Anthropic, ProviderId::anthropic(), "model", - None, test_catalog(), - ); + ) + .build(); let supervisor = SubAgentSupervisor::new(1); let factory: SessionFactory = Arc::new(|| { panic!("factory should not be called in this test"); diff --git a/lib/components/fabro-agent/src/config.rs b/lib/components/fabro-agent/src/config.rs index bc22fd75c..cb68ada66 100644 --- a/lib/components/fabro-agent/src/config.rs +++ b/lib/components/fabro-agent/src/config.rs @@ -4,6 +4,7 @@ use std::time::Duration; use fabro_llm::types::{ReasoningEffort, Speed}; use fabro_mcp::config::McpServerSettings; +use fabro_model::AgentProfileKind; use fabro_types::PermissionLevel; /// Callback invoked before each tool execution. Return `Ok(())` to allow, @@ -99,15 +100,52 @@ impl ToolHookCallback for ToolApprovalAdapter { async fn post_tool_use_failure(&self, _tool_name: &str, _tool_call_id: &str, _error: &str) {} } -#[derive(Clone, Debug, Default, PartialEq, Eq)] +#[derive(Clone, Default, PartialEq, Eq)] pub struct ToolSecrets { pub brave_search_api_key: Option, } +impl std::fmt::Debug for ToolSecrets { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ToolSecrets") + .field( + "brave_search_configured", + &self.brave_search_api_key.is_some(), + ) + .finish() + } +} + +/// Options captured by native tool executors when a profile is constructed. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct NativeToolOptions { + pub default_command_timeout_ms: u64, + pub max_command_timeout_ms: u64, + pub secrets: ToolSecrets, +} + +impl NativeToolOptions { + pub(crate) fn for_profile(profile_kind: AgentProfileKind) -> Self { + let mut options = Self::default(); + if profile_kind == AgentProfileKind::Anthropic { + options.default_command_timeout_ms = 120_000; + } + options + } +} + +impl Default for NativeToolOptions { + fn default() -> Self { + Self { + default_command_timeout_ms: 10_000, + max_command_timeout_ms: 600_000, + secrets: ToolSecrets::default(), + } + } +} + #[derive(Clone)] pub struct SessionOptions { - pub default_command_timeout_ms: u64, - pub max_command_timeout_ms: u64, pub reasoning_effort: Option, pub speed: Option, pub tool_output_limits: HashMap, @@ -137,8 +175,6 @@ pub struct SessionOptions { pub skill_dirs: Option>, /// MCP server configurations to connect to on session startup. pub mcp_servers: Vec, - /// Secret values supplied by the runtime boundary for native tools. - pub tool_secrets: ToolSecrets, /// Wall-clock timeout for the entire `process_input` call. /// When set, the session's cancel token is triggered after this duration. pub wall_clock_timeout: Option, @@ -147,11 +183,6 @@ pub struct SessionOptions { impl std::fmt::Debug for SessionOptions { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SessionOptions") - .field( - "default_command_timeout_ms", - &self.default_command_timeout_ms, - ) - .field("max_command_timeout_ms", &self.max_command_timeout_ms) .field("max_tokens", &self.max_tokens) .field("reasoning_effort", &self.reasoning_effort) .field("speed", &self.speed) @@ -180,10 +211,6 @@ impl std::fmt::Debug for SessionOptions { .field("compaction_preserve_turns", &self.compaction_preserve_turns) .field("skill_dirs", &self.skill_dirs) .field("mcp_servers", &self.mcp_servers.len()) - .field( - "brave_search_configured", - &self.tool_secrets.brave_search_api_key.is_some(), - ) .field("wall_clock_timeout", &self.wall_clock_timeout) .finish() } @@ -192,8 +219,6 @@ impl std::fmt::Debug for SessionOptions { impl Default for SessionOptions { fn default() -> Self { Self { - default_command_timeout_ms: 10_000, - max_command_timeout_ms: 600_000, max_tokens: None, reasoning_effort: None, speed: None, @@ -213,7 +238,6 @@ impl Default for SessionOptions { compaction_preserve_turns: 6, skill_dirs: None, mcp_servers: Vec::new(), - tool_secrets: ToolSecrets::default(), wall_clock_timeout: None, } } @@ -274,8 +298,6 @@ mod tests { #[test] fn default_config_values() { let config = SessionOptions::default(); - assert_eq!(config.default_command_timeout_ms, 10_000); - assert_eq!(config.max_command_timeout_ms, 600_000); assert!(config.reasoning_effort.is_none()); assert!(config.tool_output_limits.is_empty()); assert!(config.tool_line_limits.is_empty()); @@ -290,20 +312,27 @@ mod tests { ToolExposureMode::AutoApprovedOnly ); assert!(config.mcp_servers.is_empty()); - assert_eq!(config.tool_secrets, ToolSecrets::default()); assert!(config.wall_clock_timeout.is_none()); } #[test] - fn session_options_debug_redacts_tool_secret_values() { - let config = SessionOptions { - tool_secrets: ToolSecrets { - brave_search_api_key: Some("brave-secret-value".to_string()), - }, - ..SessionOptions::default() + fn native_tool_options_have_expected_profile_defaults() { + let openai = NativeToolOptions::for_profile(AgentProfileKind::OpenAi); + let anthropic = NativeToolOptions::for_profile(AgentProfileKind::Anthropic); + + 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); + } + + #[test] + fn tool_secrets_debug_redacts_values() { + let secrets = ToolSecrets { + brave_search_api_key: Some("brave-secret-value".to_string()), }; - let debug = format!("{config:?}"); + let debug = format!("{secrets:?}"); assert!(debug.contains("brave_search_configured: true")); assert!(!debug.contains("brave-secret-value")); diff --git a/lib/components/fabro-agent/src/lib.rs b/lib/components/fabro-agent/src/lib.rs index f422fdb80..ebc732dc2 100644 --- a/lib/components/fabro-agent/src/lib.rs +++ b/lib/components/fabro-agent/src/lib.rs @@ -34,8 +34,8 @@ pub mod types; pub use agent_profile::AgentProfile; pub use config::{ - SessionOptions, ToolAccess, ToolAccessPolicy, ToolApprovalAdapter, ToolExposureMode, - ToolHookCallback, ToolHookDecision, ToolSecrets, + NativeToolOptions, SessionOptions, ToolAccess, ToolAccessPolicy, ToolApprovalAdapter, + ToolExposureMode, ToolHookCallback, ToolHookDecision, ToolSecrets, }; #[cfg(feature = "docker")] pub use docker_sandbox::{DockerSandbox, DockerSandboxOptions}; @@ -47,7 +47,9 @@ pub use history::History; pub use local_sandbox::LocalSandbox; pub use loop_detection::detect_loop; pub use memory::{MemoryDocument, discover_memory}; -pub use profiles::{AnthropicProfile, EnvContext, GeminiProfile, OpenAiProfile}; +pub use profiles::{ + AgentProfileBuilder, AnthropicProfile, EnvContext, GeminiProfile, OpenAiProfile, +}; pub use question_tools::{ ANTHROPIC_ASK_USER_QUESTION_TOOL, AgentQuestion, AgentQuestionAnswer, AgentQuestionAnswerStatus, AgentQuestionRuntime, AgentToolRuntime, @@ -73,8 +75,7 @@ pub use todo_tools::{ pub use tool_registry::{AgentEventEmitter, ToolRegistry}; pub use tools::{ WebFetchSummarizer, make_edit_file_tool, make_glob_tool, make_grep_tool, make_read_file_tool, - make_shell_tool, make_shell_tool_with_config, make_write_file_tool, register_core_tools, - register_secret_backed_tools, + make_shell_tool, make_shell_tool_with_options, make_write_file_tool, register_core_tools, }; pub use truncation::{TruncationMode, truncate_lines, truncate_output, truncate_tool_output}; pub use types::{ diff --git a/lib/components/fabro-agent/src/profiles/anthropic.rs b/lib/components/fabro-agent/src/profiles/anthropic.rs index d26561121..ec7fb07e8 100644 --- a/lib/components/fabro-agent/src/profiles/anthropic.rs +++ b/lib/components/fabro-agent/src/profiles/anthropic.rs @@ -4,7 +4,7 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; -use crate::config::SessionOptions; +use crate::config::NativeToolOptions; use crate::profiles::{BaseProfile, assemble_system_prompt}; use crate::sandbox::Sandbox; use crate::skills::Skill; @@ -19,18 +19,18 @@ pub struct AnthropicProfile { base: BaseProfile, } -fn anthropic_core_prompt(has_spawn_agent: bool) -> String { +fn anthropic_core_prompt(has_spawn_agent: bool, has_web_search: bool) -> String { let mut sections = vec![ - intro_section(), - system_section(), - "{env_block}", - doing_tasks_section(), - executing_actions_section(), - using_tools_section(), - session_specific_guidance_section(has_spawn_agent), - communicating_with_user_section(), - tone_and_style_section(), - coding_best_practices_section(), + intro_section().to_string(), + system_section().to_string(), + "{env_block}".to_string(), + doing_tasks_section().to_string(), + executing_actions_section().to_string(), + using_tools_section(has_web_search), + session_specific_guidance_section(has_spawn_agent).to_string(), + communicating_with_user_section().to_string(), + tone_and_style_section().to_string(), + coding_best_practices_section().to_string(), ]; sections.retain(|section| !section.is_empty()); sections.join("\n\n") @@ -102,8 +102,14 @@ unexpected files, branches, locks, and configuration before deleting or overwrit deleting, replacing, or overwriting anything, read or inspect it first." } -fn using_tools_section() -> &'static str { - "\ +fn using_tools_section(has_web_search: bool) -> String { + let web_guidance = if has_web_search { + " - To search the internet use web_search, and to inspect a specific URL use web_fetch.\n" + } else { + " - To inspect a specific URL use web_fetch.\n" + }; + format!( + "\ # Using your tools - Do NOT use the shell tool to run commands when a relevant dedicated tool is provided. Using \ @@ -113,7 +119,7 @@ dedicated tools helps the user understand and review your work. - To create files use write_file instead of cat with heredoc or echo redirection. - To search for files use glob instead of find or ls. - To search file contents use grep instead of shell grep or rg. - - To search the internet use web_search, and to inspect a specific URL use web_fetch. +{web_guidance}\ - Reserve shell for system commands, tests, builds, and terminal operations that require \ shell execution. - Break down and manage your work with the TaskCreate tool. These tools are helpful for \ @@ -124,6 +130,7 @@ batch up multiple tasks before marking them as completed. - You can call multiple tools in a single response. If there are no dependencies between the \ calls, make independent tool calls in parallel. If one call depends on another call's result, \ run them sequentially." + ) } fn session_specific_guidance_section(has_spawn_agent: bool) -> &'static str { @@ -181,13 +188,18 @@ impl AnthropicProfile { model: impl Into, summarizer: Option, ) -> Self { - let config = SessionOptions { - default_command_timeout_ms: 120_000, - ..SessionOptions::default() - }; + let options = NativeToolOptions::for_profile(AgentProfileKind::Anthropic); + Self::with_native_tools(model, &options, summarizer) + } + + pub(crate) fn with_native_tools( + model: impl Into, + options: &NativeToolOptions, + summarizer: Option, + ) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, &config, summarizer); + register_core_tools(&mut registry, options, summarizer); registry.register(make_edit_file_tool()); // Anthropic task tools share one runtime per profile instance. let todo_runtime = Arc::new(TodoRuntime::new()); @@ -255,7 +267,8 @@ impl AgentProfile for AnthropicProfile { skills: &[Skill], ) -> String { let has_spawn_agent = self.base.registry.get("spawn_agent").is_some(); - let core_prompt = anthropic_core_prompt(has_spawn_agent); + let has_web_search = self.base.registry.get("web_search").is_some(); + let core_prompt = anthropic_core_prompt(has_spawn_agent, has_web_search); assemble_system_prompt( &core_prompt, @@ -330,8 +343,8 @@ mod tests { "prompt should contain coding best practices" ); assert!( - prompt.contains("web_search"), - "prompt should contain web_search guidance" + !prompt.contains("web_search"), + "prompt should omit guidance for unavailable tools" ); assert!( prompt.contains("web_fetch"), @@ -442,14 +455,14 @@ mod tests { fn anthropic_tools_registered() { let profile = AnthropicProfile::new("claude-sonnet-4-20250514"); let names = profile.tool_registry().names(); - assert_eq!(names.len(), 12); + assert_eq!(names.len(), 11); assert!(names.contains(&"read_file".to_string())); assert!(names.contains(&"write_file".to_string())); assert!(names.contains(&"edit_file".to_string())); assert!(names.contains(&"shell".to_string())); assert!(names.contains(&"grep".to_string())); assert!(names.contains(&"glob".to_string())); - assert!(names.contains(&"web_search".to_string())); + assert!(!names.contains(&"web_search".to_string())); assert!(names.contains(&"web_fetch".to_string())); assert!(names.contains(&"TaskCreate".to_string())); assert!(names.contains(&"TaskUpdate".to_string())); @@ -467,7 +480,7 @@ mod tests { #[test] fn anthropic_register_subagent_tools() { let mut profile = AnthropicProfile::new("claude-sonnet-4-20250514"); - assert_eq!(profile.tool_registry().names().len(), 12); + assert_eq!(profile.tool_registry().names().len(), 11); let supervisor = SubAgentSupervisor::new(3); let factory: SessionFactory = Arc::new(|| { @@ -477,7 +490,7 @@ mod tests { profile.register_subagent_tools(supervisor, factory, 0); let names = profile.tool_registry().names(); - assert_eq!(names.len(), 16, "should have 12 base + 4 subagent tools"); + assert_eq!(names.len(), 15, "should have 11 base + 4 subagent tools"); assert!(names.contains(&"spawn_agent".to_string())); assert!(names.contains(&"send_input".to_string())); assert!(names.contains(&"wait".to_string())); diff --git a/lib/components/fabro-agent/src/profiles/gemini.rs b/lib/components/fabro-agent/src/profiles/gemini.rs index b8e3b2f76..31f9e48ce 100644 --- a/lib/components/fabro-agent/src/profiles/gemini.rs +++ b/lib/components/fabro-agent/src/profiles/gemini.rs @@ -4,7 +4,7 @@ use fabro_model::{AgentProfileKind, Catalog, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; -use crate::config::SessionOptions; +use crate::config::NativeToolOptions; use crate::profiles::{BaseProfile, assemble_system_prompt}; use crate::sandbox::Sandbox; use crate::skills::Skill; @@ -29,10 +29,18 @@ impl GeminiProfile { model: impl Into, summarizer: Option, ) -> Self { - let config = SessionOptions::default(); + let options = NativeToolOptions::for_profile(AgentProfileKind::Gemini); + Self::with_native_tools(model, &options, summarizer) + } + + pub(crate) fn with_native_tools( + model: impl Into, + options: &NativeToolOptions, + summarizer: Option, + ) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, &config, summarizer); + register_core_tools(&mut registry, options, summarizer); registry.register(make_edit_file_tool()); registry.register(make_read_many_files_tool()); registry.register(make_list_dir_tool()); @@ -95,7 +103,16 @@ impl AgentProfile for GeminiProfile { user_instructions: Option<&str>, skills: &[Skill], ) -> String { - let core_prompt = "\ + let web_search_guidance = if self.base.registry.get("web_search").is_some() { + "## web_search +Search the web for information. + +" + } else { + "" + }; + let core_prompt = format!( + "\ You are Gemini CLI, an interactive CLI agent specializing in software engineering tasks \ including solving bugs, adding new functionality, refactoring code, and explaining code. \ Your primary goal is to help users safely and effectively. @@ -127,7 +144,7 @@ still providing the best answer you can. files individually. - If you need to read multiple ranges in a file, do so in parallel. -{env_block} +{{env_block}} # Development Lifecycle @@ -184,10 +201,7 @@ Find files by name pattern. Results sorted by modification time. ## list_dir List directory contents with depth control. -## web_search -Search the web for information. - -## web_fetch +{web_search_guidance}## web_fetch Fetch content from a URL and optionally summarize it. Pass a prompt to extract specific \ information instead of returning the full page. @@ -210,10 +224,11 @@ These are foundational mandates that take precedence over defaults in this promp # Coding Best Practices Write clean, maintainable code. Handle errors appropriately. Follow existing code conventions \ -in the project."; +in the project." + ); assemble_system_prompt( - core_prompt, + &core_prompt, env, env_context, memory, @@ -274,7 +289,7 @@ mod tests { assert!(prompt.contains("grep")); assert!(prompt.contains("glob")); assert!(prompt.contains("list_dir")); - assert!(prompt.contains("web_search")); + assert!(!prompt.contains("web_search")); assert!(prompt.contains("web_fetch")); assert!(prompt.contains("Default timeout is 10 seconds")); } @@ -311,7 +326,7 @@ mod tests { fn gemini_tools_registered() { let profile = GeminiProfile::new("gemini-2.0-flash"); let names = profile.tool_registry().names(); - assert_eq!(names.len(), 10); + assert_eq!(names.len(), 9); assert!(names.contains(&"read_file".to_string())); assert!(names.contains(&"read_many_files".to_string())); assert!(names.contains(&"write_file".to_string())); @@ -320,7 +335,7 @@ mod tests { assert!(names.contains(&"grep".to_string())); assert!(names.contains(&"glob".to_string())); assert!(names.contains(&"list_dir".to_string())); - assert!(names.contains(&"web_search".to_string())); + assert!(!names.contains(&"web_search".to_string())); assert!(names.contains(&"web_fetch".to_string())); } @@ -333,7 +348,7 @@ mod tests { }); profile.register_subagent_tools(supervisor, factory, 0); let names = profile.tool_registry().names(); - assert_eq!(names.len(), 14); + assert_eq!(names.len(), 13); assert!(names.contains(&"spawn_agent".to_string())); assert!(names.contains(&"send_input".to_string())); assert!(names.contains(&"wait".to_string())); diff --git a/lib/components/fabro-agent/src/profiles/mod.rs b/lib/components/fabro-agent/src/profiles/mod.rs index fbc8de5e9..5e407d972 100644 --- a/lib/components/fabro-agent/src/profiles/mod.rs +++ b/lib/components/fabro-agent/src/profiles/mod.rs @@ -10,9 +10,102 @@ pub use anthropic::AnthropicProfile; pub use gemini::GeminiProfile; pub use openai::OpenAiProfile; +use crate::agent_profile::AgentProfile; +use crate::config::{NativeToolOptions, ToolSecrets}; use crate::sandbox::Sandbox; use crate::skills::{Skill, format_skills_prompt_section}; use crate::tool_registry::ToolRegistry; +use crate::tools::WebFetchSummarizer; + +/// Builds a provider profile and its native tools from one configuration. +/// +/// Native tool options must be supplied before [`Self::build`] because their +/// values are captured by tool executors during profile construction. Clone a +/// configured builder when root and child sessions must expose the same tools. +#[derive(Clone)] +pub struct AgentProfileBuilder { + profile_kind: AgentProfileKind, + provider_id: ProviderId, + model: String, + catalog: Arc, + native_tool_options: NativeToolOptions, + summarizer: Option, +} + +impl AgentProfileBuilder { + #[must_use] + pub fn new( + profile_kind: AgentProfileKind, + provider_id: ProviderId, + model: impl Into, + catalog: Arc, + ) -> Self { + Self { + profile_kind, + provider_id, + model: model.into(), + catalog, + native_tool_options: NativeToolOptions::for_profile(profile_kind), + summarizer: None, + } + } + + #[must_use] + pub fn with_tool_secrets(mut self, secrets: ToolSecrets) -> Self { + self.native_tool_options.secrets = secrets; + self + } + + #[must_use] + pub fn with_command_timeouts( + mut self, + default_command_timeout_ms: u64, + max_command_timeout_ms: u64, + ) -> Self { + self.native_tool_options.default_command_timeout_ms = default_command_timeout_ms; + self.native_tool_options.max_command_timeout_ms = max_command_timeout_ms; + self + } + + #[must_use] + pub fn with_web_fetch_summarizer(mut self, summarizer: Option) -> Self { + self.summarizer = summarizer; + self + } + + #[must_use] + pub fn build(self) -> Box { + match self.profile_kind { + AgentProfileKind::OpenAi => Box::new( + OpenAiProfile::with_native_tools( + self.model, + &self.native_tool_options, + self.summarizer, + ) + .with_provider_id(self.provider_id) + .with_catalog(self.catalog), + ), + AgentProfileKind::Gemini => Box::new( + GeminiProfile::with_native_tools( + self.model, + &self.native_tool_options, + self.summarizer, + ) + .with_provider_id(self.provider_id) + .with_catalog(self.catalog), + ), + AgentProfileKind::Anthropic => Box::new( + AnthropicProfile::with_native_tools( + self.model, + &self.native_tool_options, + self.summarizer, + ) + .with_provider_id(self.provider_id) + .with_catalog(self.catalog), + ), + } + } +} /// Common fields shared by all provider profiles. /// @@ -153,4 +246,68 @@ mod tests { assert!(block.contains("Model: claude-opus-4-6")); assert!(block.contains("Knowledge cutoff: May 2025")); } + + #[test] + fn profile_builder_keeps_tool_availability_and_prompt_guidance_in_sync() { + let catalog = Arc::new(Catalog::from_builtin().unwrap()); + let env = MockSandbox::linux(); + let cases = [ + ( + AgentProfileKind::OpenAi, + ProviderId::openai(), + "gpt-5.4-mini", + ), + ( + AgentProfileKind::Anthropic, + ProviderId::anthropic(), + "claude-haiku-4-5", + ), + ( + AgentProfileKind::Gemini, + ProviderId::gemini(), + "gemini-3-flash-preview", + ), + ]; + + for (profile_kind, provider_id, model) in cases { + let profile = AgentProfileBuilder::new( + profile_kind, + provider_id.clone(), + model, + Arc::clone(&catalog), + ) + .build(); + assert_eq!(profile.profile_kind(), profile_kind); + assert_eq!(profile.provider_id(), provider_id); + assert!(profile.tool_registry().get("web_search").is_none()); + let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); + assert!( + !prompt.contains("web_search"), + "{profile_kind:?} prompt advertised an unavailable tool" + ); + + let configured_builder = AgentProfileBuilder::new( + profile_kind, + profile.provider_id(), + model, + Arc::clone(&catalog), + ) + .with_tool_secrets(ToolSecrets { + brave_search_api_key: Some("configured-key".to_string()), + }) + .with_command_timeouts(20_000, 600_000); + for configured in [ + configured_builder.clone().build(), + configured_builder.build(), + ] { + assert!(configured.tool_registry().get("web_search").is_some()); + let prompt = + configured.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]); + assert!( + prompt.contains("web_search"), + "{profile_kind:?} prompt omitted guidance for an available tool" + ); + } + } + } } diff --git a/lib/components/fabro-agent/src/profiles/openai.rs b/lib/components/fabro-agent/src/profiles/openai.rs index 868d2e2fe..64898fc65 100644 --- a/lib/components/fabro-agent/src/profiles/openai.rs +++ b/lib/components/fabro-agent/src/profiles/openai.rs @@ -5,7 +5,7 @@ use fabro_model::{AgentProfileKind, Catalog, CodecKind, ProviderId}; use super::EnvContext; use crate::agent_profile::AgentProfile; use crate::apply_patch; -use crate::config::SessionOptions; +use crate::config::NativeToolOptions; use crate::profiles::{BaseProfile, assemble_system_prompt}; use crate::sandbox::Sandbox; use crate::skills::Skill; @@ -46,10 +46,18 @@ impl OpenAiProfile { model: impl Into, summarizer: Option, ) -> Self { - let config = SessionOptions::default(); + let options = NativeToolOptions::for_profile(AgentProfileKind::OpenAi); + Self::with_native_tools(model, &options, summarizer) + } + + pub(crate) fn with_native_tools( + model: impl Into, + options: &NativeToolOptions, + summarizer: Option, + ) -> Self { let mut registry = ToolRegistry::new(); - register_core_tools(&mut registry, &config, summarizer); + register_core_tools(&mut registry, options, summarizer); registry.register(apply_patch::make_apply_patch_tool()); // Codex-compatible `update_plan` is OpenAI-only. let todo_runtime = Arc::new(TodoRuntime::new()); @@ -188,6 +196,14 @@ The `old_string` must match exactly and be unique unless `replace_all` is true; surrounding context to make the match unique and preserve the existing indentation.", ), }; + let web_search_guidance = if self.base.registry.get("web_search").is_some() { + "## web_search +Search the web using Brave Search. Returns titles, URLs, and descriptions. + +" + } else { + "" + }; let core_prompt = format!("\ You are a coding agent powered by {provider_name}, running in a terminal-based agentic coding assistant. \ You are expected to be precise, safe, and helpful. @@ -263,10 +279,7 @@ Search file contents with regex. Use glob_filter to narrow results. ## glob Find files by name pattern. -## web_search -Search the web using Brave Search. Returns titles, URLs, and descriptions. - -## web_fetch +{web_search_guidance}## web_fetch Fetch content from a URL and optionally summarize it. Pass a prompt to extract specific \ information instead of returning the full page. URLs must start with http:// or https://. @@ -330,6 +343,7 @@ mod tests { assert!(prompt.contains("grep")); assert!(prompt.contains("glob")); assert!(prompt.contains("timeout_ms")); + assert!(!prompt.contains("## web_search")); } #[test] @@ -380,26 +394,26 @@ mod tests { #[test] fn openai_subagent_tools_registered() { let mut profile = OpenAiProfile::new("o3-mini"); - assert_eq!(profile.tool_registry().names().len(), 9); + assert_eq!(profile.tool_registry().names().len(), 8); let supervisor = SubAgentSupervisor::new(3); let factory: SessionFactory = Arc::new(|| panic!("should not be called in test")); profile.register_subagent_tools(supervisor, factory, 0); - assert_eq!(profile.tool_registry().names().len(), 13); + assert_eq!(profile.tool_registry().names().len(), 12); } #[test] fn openai_tools_registered() { let profile = OpenAiProfile::new("o3-mini"); let names = profile.tool_registry().names(); - assert_eq!(names.len(), 9); + assert_eq!(names.len(), 8); assert!(names.contains(&"read_file".to_string())); assert!(names.contains(&"write_file".to_string())); assert!(names.contains(&"shell".to_string())); assert!(names.contains(&"grep".to_string())); assert!(names.contains(&"glob".to_string())); assert!(names.contains(&"apply_patch".to_string())); - assert!(names.contains(&"web_search".to_string())); + assert!(!names.contains(&"web_search".to_string())); assert!(names.contains(&"web_fetch".to_string())); assert!(names.contains(&"update_plan".to_string())); diff --git a/lib/components/fabro-agent/src/tools.rs b/lib/components/fabro-agent/src/tools.rs index 97e188a2c..4c4b98c5e 100644 --- a/lib/components/fabro-agent/src/tools.rs +++ b/lib/components/fabro-agent/src/tools.rs @@ -5,10 +5,11 @@ use std::sync::Arc; use fabro_llm::client::Client; use fabro_llm::types::{Message, Request, ToolDefinition}; use fabro_model::ModelHandle; +#[cfg(test)] use fabro_static::EnvVars; use futures::{StreamExt, stream}; -use crate::config::{SessionOptions, ToolSecrets}; +use crate::config::NativeToolOptions; use crate::sandbox::GrepOptions; use crate::tool_registry::{RegisteredTool, ToolRegistry, ToolSource}; @@ -46,35 +47,26 @@ fn html_to_markdown(text: &str) -> String { } /// Registers the core tools shared by all provider profiles: `read_file`, -/// `write_file`, `shell`, `grep`, `glob`, `web_search`, and `web_fetch`. +/// `write_file`, `shell`, `grep`, `glob`, and `web_fetch`. `web_search` is +/// included when a Brave Search API key is configured. /// -/// The shell tool uses `config` to set its default and max timeouts. Pass a -/// custom `SessionOptions` (e.g. with a longer `default_command_timeout_ms`) -/// for providers that need non-default shell behavior. +/// The shell tool captures its default and max timeouts from `options`. pub fn register_core_tools( registry: &mut ToolRegistry, - config: &SessionOptions, + options: &NativeToolOptions, summarizer: Option, ) { registry.register(make_read_file_tool()); registry.register(make_write_file_tool()); - registry.register(make_shell_tool_with_config(config)); + registry.register(make_shell_tool_with_options(options)); registry.register(make_grep_tool()); registry.register(make_glob_tool()); - register_secret_backed_tools(registry, &config.tool_secrets); + if let Some(api_key) = &options.secrets.brave_search_api_key { + registry.register(make_web_search_tool_with_api_key(api_key.clone())); + } registry.register(make_web_fetch_tool(summarizer)); } -/// Registers core tools whose executors capture runtime-supplied secrets. -/// -/// Calling this after profile construction replaces the unconfigured tool -/// registrations created by the default profile constructors. -pub fn register_secret_backed_tools(registry: &mut ToolRegistry, secrets: &ToolSecrets) { - registry.register(make_web_search_tool_with_api_key( - secrets.brave_search_api_key.clone(), - )); -} - pub(crate) fn required_str<'a>(args: &'a serde_json::Value, key: &str) -> Result<&'a str, String> { args.get(key) .and_then(|v| v.as_str()) @@ -218,13 +210,13 @@ pub fn make_edit_file_tool() -> RegisteredTool { #[must_use] pub fn make_shell_tool() -> RegisteredTool { - make_shell_tool_with_config(&SessionOptions::default()) + make_shell_tool_with_options(&NativeToolOptions::default()) } #[must_use] -pub fn make_shell_tool_with_config(config: &SessionOptions) -> RegisteredTool { - let default_timeout = config.default_command_timeout_ms; - let max_timeout = config.max_command_timeout_ms; +pub fn make_shell_tool_with_options(options: &NativeToolOptions) -> RegisteredTool { + let default_timeout = options.default_command_timeout_ms; + let max_timeout = options.max_command_timeout_ms; RegisteredTool { definition: ToolDefinition { name: "shell".into(), @@ -524,7 +516,7 @@ fn format_brave_results(body: &serde_json::Value) -> String { output } -fn make_web_search_tool_with_api_key(api_key: Option) -> RegisteredTool { +fn make_web_search_tool_with_api_key(api_key: String) -> RegisteredTool { use std::sync::OnceLock; static CLIENT: OnceLock = OnceLock::new(); @@ -544,10 +536,6 @@ fn make_web_search_tool_with_api_key(api_key: Option) -> RegisteredTool executor: Arc::new(move |args, _ctx| { let api_key = api_key.clone(); Box::pin(async move { - let api_key = api_key.ok_or_else(|| { - format!("{} is not configured", EnvVars::BRAVE_SEARCH_API_KEY) - })?; - let query = required_str(&args, "query")?; let client = CLIENT .get_or_init(|| { @@ -700,19 +688,19 @@ mod tests { use tokio_util::sync::CancellationToken; use super::*; - use crate::config::ToolSecrets; + use crate::config::{NativeToolOptions, ToolSecrets}; use crate::sandbox::*; use crate::test_support::MockSandbox; use crate::tool_registry::ToolContext; #[test] fn core_tool_descriptions_include_actionable_guidance() { - let config = SessionOptions::default(); + let options = NativeToolOptions::default(); let tools = [ make_read_file_tool(), make_write_file_tool(), make_edit_file_tool(), - make_shell_tool_with_config(&config), + make_shell_tool_with_options(&options), make_grep_tool(), make_glob_tool(), make_web_fetch_tool(None), @@ -1339,27 +1327,18 @@ mod tests { assert!(output.contains("src/lib.rs")); } - #[tokio::test] - async fn web_search_missing_api_key_returns_error() { - let tool = make_web_search_tool_with_api_key(None); - let env: Arc = Arc::new(MockSandbox::default()); - let result = (tool.executor)(serde_json::json!({"query": "test"}), ToolContext { - env, - cancel: CancellationToken::new(), - tool_env_provider: None, - session_id: None, - root_session_id: None, - tool_call_id: None, - agent_event_emitter: None, - }) - .await; - let err = result.unwrap_err(); - assert_eq!(err, "BRAVE_SEARCH_API_KEY is not configured"); + #[test] + fn register_core_tools_omits_web_search_without_api_key() { + let mut registry = ToolRegistry::new(); + + register_core_tools(&mut registry, &NativeToolOptions::default(), None); + + assert!(registry.get("web_search").is_none()); } #[tokio::test] async fn web_search_missing_query_returns_error() { - let tool = make_web_search_tool_with_api_key(Some("fake-key".into())); + let tool = make_web_search_tool_with_api_key("fake-key".into()); let env: Arc = Arc::new(MockSandbox::default()); let result = (tool.executor)(serde_json::json!({}), ToolContext { env, @@ -1381,14 +1360,14 @@ mod tests { #[tokio::test] async fn register_core_tools_passes_configured_brave_search_key() { let mut registry = ToolRegistry::new(); - let config = SessionOptions { - tool_secrets: ToolSecrets { + let options = NativeToolOptions { + secrets: ToolSecrets { brave_search_api_key: Some("fake-key".to_string()), }, - ..SessionOptions::default() + ..NativeToolOptions::default() }; - register_core_tools(&mut registry, &config, None); + register_core_tools(&mut registry, &options, None); let tool = registry .get("web_search") @@ -1823,7 +1802,7 @@ mod tests { async fn web_search_returns_results() { let api_key = std::env::var(EnvVars::BRAVE_SEARCH_API_KEY) .expect("BRAVE_SEARCH_API_KEY must be set to run this test"); - let tool = make_web_search_tool_with_api_key(Some(api_key)); + let tool = make_web_search_tool_with_api_key(api_key); let env: Arc = Arc::new(MockSandbox::default()); let result = (tool.executor)( serde_json::json!({"query": "rust programming language"}), diff --git a/lib/components/fabro-agent/tests/it/parity_matrix.rs b/lib/components/fabro-agent/tests/it/parity_matrix.rs index 2bdf14a51..03585fc93 100644 --- a/lib/components/fabro-agent/tests/it/parity_matrix.rs +++ b/lib/components/fabro-agent/tests/it/parity_matrix.rs @@ -10,15 +10,15 @@ use std::sync::Arc; use fabro_agent::subagent::SessionFactory; use fabro_agent::{ - AgentEvent, AgentProfile, AnthropicProfile, GeminiProfile, LocalSandbox, OpenAiProfile, - Session, SessionOptions, SubAgentSupervisor, WebFetchSummarizer, + AgentEvent, AgentProfile, AgentProfileBuilder, LocalSandbox, OpenAiProfile, Session, + SessionOptions, SubAgentSupervisor, ToolSecrets, WebFetchSummarizer, }; use fabro_auth::EnvCredentialSource; use fabro_llm::client::Client; use fabro_llm::provider::ProviderAdapter; use fabro_llm::providers::{OpenAiAdapter, OpenAiCompatibleAdapter}; use fabro_model::catalog::{LlmCatalogSettings, ProviderCatalogSettings}; -use fabro_model::{Catalog, ModelHandle, ProviderId}; +use fabro_model::{AgentProfileKind, Catalog, ModelHandle, ProviderId}; use fabro_test::{TwinScenario, TwinScenarios, TwinToolCall, twin_openai}; type Provider = ProviderId; @@ -54,65 +54,48 @@ fn build_summarizer(provider: &Provider, client: &Client) -> WebFetchSummarizer } } -fn build_profile(provider: &Provider, model: &str, client: &Client) -> Box { - let summarizer = Some(build_summarizer(provider, client)); - let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build")); +fn profile_kind(provider: &Provider) -> AgentProfileKind { match provider.as_str() { - ProviderId::ANTHROPIC => Box::new(AnthropicProfile::with_summarizer(model, summarizer)), - ProviderId::OPENAI => Box::new( - OpenAiProfile::with_summarizer(model, summarizer).with_catalog(Arc::clone(&catalog)), - ), - "kimi" | "zai" | "minimax" | "inception" => Box::new( - OpenAiProfile::with_summarizer(model, summarizer) - .with_provider_id(provider.clone()) - .with_catalog(Arc::clone(&catalog)), - ), - ProviderId::GEMINI => Box::new(GeminiProfile::with_summarizer(model, summarizer)), + ProviderId::ANTHROPIC => AgentProfileKind::Anthropic, + ProviderId::GEMINI => AgentProfileKind::Gemini, + ProviderId::OPENAI | "kimi" | "zai" | "minimax" | "inception" => AgentProfileKind::OpenAi, other => panic!("unexpected provider {other}"), } } +fn profile_builder( + provider: &Provider, + model: &str, + client: &Client, + tool_secrets: ToolSecrets, +) -> AgentProfileBuilder { + let summarizer = Some(build_summarizer(provider, client)); + let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build")); + AgentProfileBuilder::new(profile_kind(provider), provider.clone(), model, catalog) + .with_web_fetch_summarizer(summarizer) + .with_tool_secrets(tool_secrets) +} + async fn make_session( provider: Provider, model: &str, cwd: &Path, + tool_secrets: ToolSecrets, twin: Option, ) -> Session { let client = make_client(&provider, twin.as_ref()).await; - let mut profile = build_profile(&provider, model, &client); + let profile_builder = profile_builder(&provider, model, &client, tool_secrets); + let mut profile = profile_builder.clone().build(); let env = Arc::new(LocalSandbox::new(cwd.to_path_buf())); // Register subagent tools so spawn_agent / wait / send_input / close_agent are // available let supervisor = SubAgentSupervisor::new(3); let factory_client = client.clone(); - let factory_model: String = model.to_string(); let factory_cwd = cwd.to_path_buf(); - let factory_provider = provider.clone(); + let factory_profile_builder = profile_builder; let factory: SessionFactory = Arc::new(move || { - let catalog = Arc::new(Catalog::from_builtin().expect("default catalog should build")); - let sub_profile: Arc = { - let summarizer = Some(build_summarizer(&factory_provider, &factory_client)); - match factory_provider.as_str() { - ProviderId::ANTHROPIC => Arc::new(AnthropicProfile::with_summarizer( - &factory_model, - summarizer, - )), - ProviderId::OPENAI => Arc::new( - OpenAiProfile::with_summarizer(&factory_model, summarizer) - .with_catalog(Arc::clone(&catalog)), - ), - "kimi" | "zai" | "minimax" | "inception" => Arc::new( - OpenAiProfile::with_summarizer(&factory_model, summarizer) - .with_provider_id(factory_provider.clone()) - .with_catalog(Arc::clone(&catalog)), - ), - ProviderId::GEMINI => { - Arc::new(GeminiProfile::with_summarizer(&factory_model, summarizer)) - } - other => panic!("unexpected provider {other}"), - } - }; + let sub_profile: Arc = Arc::from(factory_profile_builder.clone().build()); let sub_env = Arc::new(LocalSandbox::new(factory_cwd.clone())); Session::new( factory_client.clone(), @@ -142,7 +125,8 @@ async fn make_session_with_config( twin: Option, ) -> Session { let client = make_client(&provider, twin.as_ref()).await; - let profile: Arc = Arc::from(build_profile(&provider, model, &client)); + let profile: Arc = + Arc::from(profile_builder(&provider, model, &client, ToolSecrets::default()).build()); let env = Arc::new(LocalSandbox::new(cwd.to_path_buf())); Session::new(client, profile, env, config, None) } @@ -215,7 +199,13 @@ macro_rules! provider_test { #[fabro_macros::e2e_test($(live($key)),+)] async fn [<$prefix _ $scenario>]() { let tmp = tempfile::tempdir().expect("failed to create tempdir"); - let mut session = make_session($provider, $model, tmp.path(), None).await; + let mut session = make_session( + $provider, + $model, + tmp.path(), + ToolSecrets::default(), + None, + ).await; session.initialize().await.unwrap(); [](&mut session, tmp.path()).await; } @@ -223,6 +213,27 @@ macro_rules! provider_test { }; } +macro_rules! web_search_provider_test { + ($provider:expr, $model:expr, $prefix:ident, keys = [$($key:expr),+ $(,)?]) => { + paste::paste! { + #[fabro_macros::e2e_test($(live($key)),+)] + async fn [<$prefix _web_search>]() { + let tmp = tempfile::tempdir().expect("failed to create tempdir"); + let tool_secrets = ToolSecrets { + brave_search_api_key: Some( + std::env::var("BRAVE_SEARCH_API_KEY") + .expect("BRAVE_SEARCH_API_KEY must be set for web-search tests"), + ), + }; + let mut session = + make_session($provider, $model, tmp.path(), tool_secrets, None).await; + session.initialize().await.unwrap(); + scenario_web_search(&mut session, tmp.path()).await; + } + } + }; +} + macro_rules! openai_twin_provider_test { ($scenario:ident) => { paste::paste! { @@ -239,6 +250,7 @@ macro_rules! openai_twin_provider_test { ProviderId::openai(), "gpt-5.4-mini", tmp.path(), + ToolSecrets::default(), Some(twin), ).await; session.initialize().await.unwrap(); @@ -429,52 +441,45 @@ provider_test!( keys = ["INCEPTION_API_KEY", "OPENAI_API_KEY"] ); -provider_test!( - web_search, +web_search_provider_test!( ProviderId::anthropic(), "claude-haiku-4-5", anthropic, keys = ["ANTHROPIC_API_KEY", "BRAVE_SEARCH_API_KEY"] ); -provider_test!( - web_search, +web_search_provider_test!( ProviderId::openai(), "gpt-5.4-mini", openai, keys = ["OPENAI_API_KEY", "BRAVE_SEARCH_API_KEY"] ); -provider_test!( - web_search, +web_search_provider_test!( ProviderId::gemini(), "gemini-3-flash-preview", gemini, keys = ["GEMINI_API_KEY", "BRAVE_SEARCH_API_KEY"] ); -provider_test!( - web_search, +web_search_provider_test!( ProviderId::new("kimi"), "kimi-k2.5", kimi, keys = ["KIMI_API_KEY", "BRAVE_SEARCH_API_KEY"] ); #[cfg(feature = "quarantine")] -provider_test!( - web_search, +web_search_provider_test!( ProviderId::new("zai"), "glm-4.7", zai, keys = ["ZAI_API_KEY", "BRAVE_SEARCH_API_KEY"] ); -provider_test!( - web_search, +web_search_provider_test!( ProviderId::new("minimax"), "minimax-m2.5", minimax, keys = ["MINIMAX_API_KEY", "BRAVE_SEARCH_API_KEY"] ); #[cfg(feature = "quarantine")] -provider_test!( - web_search, +web_search_provider_test!( ProviderId::new("inception"), "mercury-2", inception, diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs index 052776af1..08b39b723 100644 --- a/lib/components/fabro-hooks/src/executor.rs +++ b/lib/components/fabro-hooks/src/executor.rs @@ -457,9 +457,9 @@ impl HookExecutorImpl { } }; - let config = fabro_agent::SessionOptions::default(); + let options = fabro_agent::NativeToolOptions::default(); let mut registry = fabro_agent::ToolRegistry::new(); - fabro_agent::register_core_tools(&mut registry, &config, None); + fabro_agent::register_core_tools(&mut registry, &options, None); let tool_defs = registry.definitions(); let mut messages = vec![ diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index 4a77f1354..d89957563 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -6,10 +6,9 @@ use async_trait::async_trait; use fabro_agent::subagent::{SessionFactory, SubAgentSupervisor}; use fabro_agent::tool_registry::{RegisteredTool, ToolContext, ToolRegistry, ToolSource}; use fabro_agent::{ - AgentEvent, AgentProfile, AnthropicProfile, CompletionCoordinator, GeminiProfile, - Message as AgentMessage, OpenAiProfile, Sandbox, Session, SessionOptions, - SessionShutdownReason, StaticEnvProvider, ToolEnvProvider, ToolSecrets, - register_question_tools, + AgentEvent, AgentProfile, AgentProfileBuilder, CompletionCoordinator, Message as AgentMessage, + Sandbox, Session, SessionOptions, SessionShutdownReason, StaticEnvProvider, ToolEnvProvider, + ToolSecrets, register_question_tools, }; use fabro_auth::{CredentialSource, EnvCredentialSource}; use fabro_graphviz::graph::{AttrValue, Node}; @@ -20,8 +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::{AgentProfileKind, Catalog, FallbackTarget, ModelRef, ProviderId, UsdMicros}; +use fabro_model::{Catalog, FallbackTarget, ModelRef, ProviderId, UsdMicros}; use fabro_types::settings::run::RunModelControls; use fabro_types::{PermissionLevel, RunId, SessionCapability, StageId, StageTiming}; use serde::de::DeserializeOwned; @@ -182,31 +183,6 @@ async fn discard_session( }); } -fn build_profile( - model: &str, - provider_id: ProviderId, - profile_kind: AgentProfileKind, - catalog: Arc, -) -> Box { - match profile_kind { - AgentProfileKind::OpenAi => Box::new( - OpenAiProfile::new(model) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Gemini => Box::new( - GeminiProfile::new(model) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - AgentProfileKind::Anthropic => Box::new( - AnthropicProfile::new(model) - .with_provider_id(provider_id) - .with_catalog(catalog), - ), - } -} - pub fn register_fabro_run_tools(registry: &mut ToolRegistry, services: &FabroRunToolServices) { for definition in fabro_tool::tool_definitions() { registry.register(fabro_run_tool(definition, services.clone())); @@ -835,13 +811,14 @@ impl AgentApiBackend { .await .map_err(|e| Error::handler_with_source("Failed to create LLM client", e))?; - let mut profile = build_profile( - model, - provider.provider_id.clone(), + let profile_builder = AgentProfileBuilder::new( provider.profile_kind, + provider.provider_id.clone(), + model, Arc::clone(&catalog), - ); - fabro_agent::register_secret_backed_tools(profile.tool_registry_mut(), &tool_secrets); + ) + .with_tool_secrets(tool_secrets); + let mut profile = profile_builder.clone().build(); let config = SessionOptions { max_tokens: node.max_tokens(), @@ -849,7 +826,6 @@ impl AgentApiBackend { speed: controls.speed, tool_hooks, mcp_servers, - tool_secrets, // Workflow agents run with no `tool_access_policy`, which exposes // the entire tool registry (read, write, shell, subagent, MCP) and // skips approval gating. Report that truthfully so the UI doesn't @@ -864,25 +840,13 @@ impl AgentApiBackend { // Build factory that creates child sessions WITHOUT subagent tools let factory_client = client.clone(); - let factory_model = model.to_string(); - let factory_provider = provider.clone(); - let factory_catalog = Arc::clone(&catalog); + let factory_profile_builder = profile_builder; let factory_env = Arc::clone(sandbox); let factory_tool_env = tool_env.cloned(); let factory_fabro_run_tools = fabro_run_tools.clone(); let factory_permission_level = config.permission_level; - let factory_tool_secrets = config.tool_secrets.clone(); let factory: SessionFactory = Arc::new(move || { - let mut child_profile = build_profile( - &factory_model, - factory_provider.provider_id.clone(), - factory_provider.profile_kind, - Arc::clone(&factory_catalog), - ); - fabro_agent::register_secret_backed_tools( - child_profile.tool_registry_mut(), - &factory_tool_secrets, - ); + let mut child_profile = factory_profile_builder.clone().build(); if let Some(services) = factory_fabro_run_tools.clone() { register_fabro_run_tools(child_profile.tool_registry_mut(), &services); } @@ -895,7 +859,6 @@ impl AgentApiBackend { reasoning_effort: controls.reasoning_effort, speed: controls.speed, permission_level: factory_permission_level, - tool_secrets: factory_tool_secrets.clone(), ..SessionOptions::default() }, None, @@ -2693,12 +2656,13 @@ reasoning = false #[test] fn build_profile_can_register_subagent_tools() { - let mut profile = build_profile( - "claude-opus-4-6", - ProviderId::anthropic(), + let mut profile = AgentProfileBuilder::new( AgentProfileKind::Anthropic, + ProviderId::anthropic(), + "claude-opus-4-6", Arc::new(Catalog::from_builtin().unwrap()), - ); + ) + .build(); let supervisor = SubAgentSupervisor::new(1); let factory: SessionFactory = Arc::new(|| { panic!("factory should not be called in this test");