From bc75c6f55f4b6aaaac5488f7e776e951ef993c9e Mon Sep 17 00:00:00 2001 From: obchain Date: Mon, 5 Oct 2026 14:14:20 +0530 Subject: [PATCH 1/2] fix(search): stop re-offering search a session cannot use A resolvable credential made the managed route look reachable, so a core that is offline, firewalled, out of balance or holding a dead key answered every call with "unavailable right now" and the agent kept trying. The verdict is now final for the session: the message tells the model to stop, the result is tagged as a permanent failure, and the session's three web tools share the latch so later calls answer without a module round-trip. Closes #6991 --- .../src/agent/session_host/recorded_tools.rs | 33 +++--- crates/openhuman-core/src/search/tools.rs | 88 ++++++++++++++- .../openhuman-core/src/search/tools_tests.rs | 103 +++++++++++++++++- 3 files changed, 202 insertions(+), 22 deletions(-) diff --git a/crates/openhuman-core/src/agent/session_host/recorded_tools.rs b/crates/openhuman-core/src/agent/session_host/recorded_tools.rs index c23aee4106d..5343fef64f9 100644 --- a/crates/openhuman-core/src/agent/session_host/recorded_tools.rs +++ b/crates/openhuman-core/src/agent/session_host/recorded_tools.rs @@ -135,21 +135,24 @@ pub(super) fn rehydrate_search_tools( .flat_map(|tools| tools.iter().map(|tool| tool.name())) .collect(); let mut seen = HashSet::new(); - let rebuilt: Vec> = recorded - .iter() - .filter(|spec| is_search_role_tool(&spec.name)) - .filter(|spec| !live_names.contains(spec.name.as_str())) - .filter(|spec| seen.insert(spec.name.clone())) - .map(|spec| { - Box::new(crate::search::TinySearchTool::recorded( - tinysearch_bus::ToolSpec { - name: spec.name.clone(), - description: spec.description.clone(), - parameters: spec.parameters.clone(), - }, - )) as Box - }) - .collect(); + // One batch, so the rebuilt roles share the "no provider answered" latch + // the live surface gives its own search tools (#6991). + let rebuilt: Vec> = crate::search::TinySearchTool::recorded_batch( + recorded + .iter() + .filter(|spec| is_search_role_tool(&spec.name)) + .filter(|spec| !live_names.contains(spec.name.as_str())) + .filter(|spec| seen.insert(spec.name.clone())) + .map(|spec| tinysearch_bus::ToolSpec { + name: spec.name.clone(), + description: spec.description.clone(), + parameters: spec.parameters.clone(), + }) + .collect::>(), + ) + .into_iter() + .map(|tool| Box::new(tool) as Box) + .collect(); if !rebuilt.is_empty() { log::info!( "[session] rebuilt {} recorded search tool(s) the live surface did not supply agent={agent_id}", diff --git a/crates/openhuman-core/src/search/tools.rs b/crates/openhuman-core/src/search/tools.rs index c237144fb50..82d4e9d0b9b 100644 --- a/crates/openhuman-core/src/search/tools.rs +++ b/crates/openhuman-core/src/search/tools.rs @@ -6,6 +6,7 @@ //! through `modules::search::execute_tool`, so a provider or login change is //! honoured on the next call without rebuilding the session. +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use async_trait::async_trait; @@ -17,12 +18,29 @@ use crate::config::Config; /// One TinySearch tool (a role tool such as `web_search_tool`, or a provider /// tool in `all_tools` presentation). +/// What a call reports once every provider has already answered +/// "unavailable" in this session. It tells the model to stop rather than to +/// wait, because nothing about the session will change the answer (#6991). +pub const SEARCH_EXHAUSTED_MESSAGE: &str = + "Web search is not available in this session. Do not call it again \u{2014} \ + answer from the material you already have."; + pub struct TinySearchTool { spec: ToolSpec, /// Spawn-time config. `None` for a deferred instance rebuilt from a /// recorded transcript, which resolves the live config per call. config: Option>, exposure: ToolExposure, + /// Latched when a call reports that no provider could answer. Shared by + /// the search tools built for one session instance, so the model is told + /// once and the later calls cost no module round-trip. + /// + /// A credential alone makes the managed route look reachable + /// (`providers::backend_credential_available`), so a deployment that is + /// offline, firewalled, out of balance or holding a dead key offers these + /// tools on every turn and fails every call. The agent then spends turns + /// on a tool that cannot work, at the moment it is least sure what to do. + exhausted: Arc, } impl TinySearchTool { @@ -31,6 +49,7 @@ impl TinySearchTool { spec, config: Some(config), exposure: ToolExposure::Direct, + exhausted: Arc::new(AtomicBool::new(false)), } } @@ -41,9 +60,36 @@ impl TinySearchTool { spec, config: None, exposure: ToolExposure::Direct, + exhausted: Arc::new(AtomicBool::new(false)), } } + /// Recorded tools that share one exhaustion latch, the way the tools of a + /// live session do: the roles are three declarations over the same + /// providers, so one of them finding nothing settles it for all three. + pub fn recorded_batch(specs: impl IntoIterator) -> Vec { + let exhausted = Arc::new(AtomicBool::new(false)); + specs + .into_iter() + .map(|spec| Self { + spec, + config: None, + exposure: ToolExposure::Direct, + exhausted: exhausted.clone(), + }) + .collect() + } + + /// Whether this session already learned that no provider can answer. + pub(crate) fn is_exhausted(&self) -> bool { + self.exhausted.load(Ordering::Relaxed) + } + + /// Record that no provider could answer, for the tools sharing this latch. + pub(crate) fn mark_exhausted(&self) { + self.exhausted.store(true, Ordering::Relaxed); + } + pub fn spec(&self) -> &ToolSpec { &self.spec } @@ -71,10 +117,7 @@ pub fn user_facing_error(error: &str) -> String { Some(code) if code == errors::RATE_LIMITED => { "Web search is rate limited right now. Wait a moment and try again.".to_string() } - Some(code) if code == errors::UNAVAILABLE => { - "Every configured search provider for this request is unavailable right now." - .to_string() - } + Some(code) if code == errors::UNAVAILABLE => SEARCH_EXHAUSTED_MESSAGE.to_string(), Some(code) if code == errors::INVALID_ARGUMENTS => { let marker = format!("{}{code}: ", errors::PREFIX); let detail = error @@ -92,6 +135,16 @@ pub fn error_code(error: &str) -> Option<&'static str> { errors::code_of(error) } +/// Whether this failure means no provider can answer for the rest of the +/// session, rather than something a later call could get past. +/// +/// Only the "every provider is unavailable" verdict qualifies. A rate limit +/// clears on its own, and a low balance or a rejected argument has its own +/// message telling the caller what to change, so neither latches. +pub(crate) fn exhausts_providers(error: &str) -> bool { + error_code(error) == Some(errors::UNAVAILABLE) +} + #[async_trait] impl Tool for TinySearchTool { fn name(&self) -> &str { @@ -132,6 +185,13 @@ impl Tool for TinySearchTool { args: Value, options: ToolCallOptions, ) -> anyhow::Result { + if self.is_exhausted() { + tracing::debug!( + tool = %self.spec.name, + "[search][tool] refused: no provider answered earlier in this session" + ); + return Ok(ToolResult::failed(SEARCH_EXHAUSTED_MESSAGE.to_string())); + } let config = self.live_config().await?; let subject = super::render::subject(&args); let max_results = args @@ -169,12 +229,20 @@ impl Tool for TinySearchTool { )) } Err(error) => { + if exhausts_providers(&error) { + self.mark_exhausted(); + } tracing::warn!( tool = %self.spec.name, code = error_code(&error).unwrap_or("unclassified"), + exhausted = self.is_exhausted(), "[search][tool] failed" ); - Ok(ToolResult::error(user_facing_error(&error))) + Ok(if exhausts_providers(&error) { + ToolResult::failed(user_facing_error(&error)) + } else { + ToolResult::error(user_facing_error(&error)) + }) } } } @@ -199,9 +267,17 @@ pub fn build_search_tools(config: &Config) -> Vec> { "[search][tool] registered search tools" ); let shared = Arc::new(config.clone()); + let exhausted = Arc::new(AtomicBool::new(false)); specs .into_iter() - .map(|spec| Box::new(TinySearchTool::new(shared.clone(), spec)) as Box) + .map(|spec| { + Box::new(TinySearchTool { + spec, + config: Some(shared.clone()), + exposure: ToolExposure::Direct, + exhausted: exhausted.clone(), + }) as Box + }) .collect() } diff --git a/crates/openhuman-core/src/search/tools_tests.rs b/crates/openhuman-core/src/search/tools_tests.rs index 8cdc4e17dfc..417f71f71ae 100644 --- a/crates/openhuman-core/src/search/tools_tests.rs +++ b/crates/openhuman-core/src/search/tools_tests.rs @@ -19,7 +19,10 @@ fn classified_errors_become_actionable_messages() { let balance = user_facing_error("tinysearch.insufficient_balance: 402 from backend"); assert!(balance.contains("balance is too low")); assert!(user_facing_error("tinysearch.rate_limited: slow down").contains("rate limited")); - assert!(user_facing_error("tinysearch.provider_unavailable: all down").contains("unavailable")); + assert_eq!( + user_facing_error("tinysearch.provider_unavailable: all down"), + SEARCH_EXHAUSTED_MESSAGE + ); assert_eq!( user_facing_error( "search ExecuteTool failed: tinysearch.invalid_arguments: urls must not be empty" @@ -100,3 +103,101 @@ fn standard_privacy_mode_allows_search_tool_dispatch() { assert!(local_only_search_block("web_search_tool").is_none()); } + +// --------------------------------------------------------------------------- +// #6991: a credential alone makes the managed route look reachable, so a +// deployment that is offline, firewalled, out of balance or holding a dead key +// offers these tools on every turn and fails every call. The old message ended +// "unavailable right now", which reads as "retry later": in the DeepSWE-10 run +// the agent called search three times on one task, then guessed, and the guess +// was what the hidden test rejected. +// --------------------------------------------------------------------------- + +fn role_specs() -> Vec { + ["web_search_tool", "web_answer_tool", "web_contents_tool"] + .into_iter() + .map(|name| ToolSpec { + name: name.to_string(), + description: "search".into(), + parameters: serde_json::json!({"type": "object"}), + }) + .collect() +} + +#[test] +fn the_unavailable_verdict_tells_the_model_to_stop_rather_than_wait() { + let message = user_facing_error("tinysearch.provider_unavailable: all down"); + + assert!( + message.contains("Do not call it again"), + "the model must be told to stop, got: {message}" + ); + assert!( + !message.contains("right now"), + "nothing in the session will change the answer, so it must not read as a retry hint: {message}" + ); +} + +#[test] +fn only_the_exhaustion_verdict_is_treated_as_final() { + // A rate limit clears on its own; a low balance and a rejected argument + // each have a message naming what to change. None of them may latch. + assert!(exhausts_providers( + "tinysearch.provider_unavailable: all down" + )); + assert!(!exhausts_providers("tinysearch.rate_limited: slow down")); + assert!(!exhausts_providers("tinysearch.insufficient_balance: 402")); + assert!(!exhausts_providers( + "tinysearch.invalid_arguments: urls must not be empty" + )); + assert!(!exhausts_providers("boom")); +} + +#[test] +fn a_session_starts_unlatched() { + for tool in TinySearchTool::recorded_batch(role_specs()) { + assert!(!tool.is_exhausted(), "{} started latched", tool.name()); + } +} + +#[test] +fn one_role_finding_nothing_settles_it_for_the_others() { + // The three roles are declarations over the same providers, so dropping + // only the role that failed would still leave two tools that cannot work. + let tools = TinySearchTool::recorded_batch(role_specs()); + + tools[0].mark_exhausted(); + + for tool in &tools { + assert!( + tool.is_exhausted(), + "{} did not share the latch", + tool.name() + ); + } +} + +#[tokio::test] +async fn a_latched_tool_refuses_before_reaching_the_module() { + // The refusal is returned without loading config or calling the module, so + // a dead search costs the session one round trip rather than one per call. + let tools = TinySearchTool::recorded_batch(role_specs()); + tools[1].mark_exhausted(); + + let result = tools[0] + .execute(serde_json::json!({"query": "anything"})) + .await + .expect("a refusal is a reported failure, not a tool error"); + + assert!(result.is_error); + assert_eq!( + result.error_kind, + Some(tinytools::ToolErrorKind::Failed), + "the failure is permanent, so it must not be tagged retryable" + ); + assert!( + result.text().contains("Do not call it again"), + "{:?}", + result.text() + ); +} From 4726dd8f7b7c175bd6c15e33aa140cb0a80eb081 Mon Sep 17 00:00:00 2001 From: obchain Date: Mon, 5 Oct 2026 15:46:24 +0530 Subject: [PATCH 2/2] fix(search): key the refusal to the configuration that failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The latch was session-wide and permanent, which broke this module's contract that every call re-reads the live config so a provider or login change is honoured without rebuilding the session: a key added after the failure stayed refused. It now records the provider signature the call failed under — role order plus every provider's resolved reachability — and refuses only a repeat under that same signature. Per tool, so one tool's dead providers cannot answer for another's. --- .../src/agent/session_host/recorded_tools.rs | 33 +++--- crates/openhuman-core/src/search/tools.rs | 108 ++++++++++-------- .../openhuman-core/src/search/tools_tests.rs | 104 +++++++++++------ 3 files changed, 145 insertions(+), 100 deletions(-) diff --git a/crates/openhuman-core/src/agent/session_host/recorded_tools.rs b/crates/openhuman-core/src/agent/session_host/recorded_tools.rs index 5343fef64f9..c23aee4106d 100644 --- a/crates/openhuman-core/src/agent/session_host/recorded_tools.rs +++ b/crates/openhuman-core/src/agent/session_host/recorded_tools.rs @@ -135,24 +135,21 @@ pub(super) fn rehydrate_search_tools( .flat_map(|tools| tools.iter().map(|tool| tool.name())) .collect(); let mut seen = HashSet::new(); - // One batch, so the rebuilt roles share the "no provider answered" latch - // the live surface gives its own search tools (#6991). - let rebuilt: Vec> = crate::search::TinySearchTool::recorded_batch( - recorded - .iter() - .filter(|spec| is_search_role_tool(&spec.name)) - .filter(|spec| !live_names.contains(spec.name.as_str())) - .filter(|spec| seen.insert(spec.name.clone())) - .map(|spec| tinysearch_bus::ToolSpec { - name: spec.name.clone(), - description: spec.description.clone(), - parameters: spec.parameters.clone(), - }) - .collect::>(), - ) - .into_iter() - .map(|tool| Box::new(tool) as Box) - .collect(); + let rebuilt: Vec> = recorded + .iter() + .filter(|spec| is_search_role_tool(&spec.name)) + .filter(|spec| !live_names.contains(spec.name.as_str())) + .filter(|spec| seen.insert(spec.name.clone())) + .map(|spec| { + Box::new(crate::search::TinySearchTool::recorded( + tinysearch_bus::ToolSpec { + name: spec.name.clone(), + description: spec.description.clone(), + parameters: spec.parameters.clone(), + }, + )) as Box + }) + .collect(); if !rebuilt.is_empty() { log::info!( "[session] rebuilt {} recorded search tool(s) the live surface did not supply agent={agent_id}", diff --git a/crates/openhuman-core/src/search/tools.rs b/crates/openhuman-core/src/search/tools.rs index 82d4e9d0b9b..38cd3d940c4 100644 --- a/crates/openhuman-core/src/search/tools.rs +++ b/crates/openhuman-core/src/search/tools.rs @@ -6,8 +6,9 @@ //! through `modules::search::execute_tool`, so a provider or login change is //! honoured on the next call without rebuilding the session. -use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; +use std::sync::{Arc, Mutex}; use async_trait::async_trait; use serde_json::Value; @@ -22,8 +23,8 @@ use crate::config::Config; /// "unavailable" in this session. It tells the model to stop rather than to /// wait, because nothing about the session will change the answer (#6991). pub const SEARCH_EXHAUSTED_MESSAGE: &str = - "Web search is not available in this session. Do not call it again \u{2014} \ - answer from the material you already have."; + "Web search is not available with this setup: no provider could answer. Do not call \ + it again \u{2014} answer from the material you already have."; pub struct TinySearchTool { spec: ToolSpec, @@ -31,16 +32,22 @@ pub struct TinySearchTool { /// recorded transcript, which resolves the live config per call. config: Option>, exposure: ToolExposure, - /// Latched when a call reports that no provider could answer. Shared by - /// the search tools built for one session instance, so the model is told - /// once and the later calls cost no module round-trip. + /// The provider configuration a call last found nothing usable under. /// /// A credential alone makes the managed route look reachable /// (`providers::backend_credential_available`), so a deployment that is - /// offline, firewalled, out of balance or holding a dead key offers these - /// tools on every turn and fails every call. The agent then spends turns - /// on a tool that cannot work, at the moment it is least sure what to do. - exhausted: Arc, + /// offline, firewalled, out of balance or holding a dead key offers this + /// tool on every turn and fails every call. The agent then spends turns on + /// a tool that cannot work, at the moment it is least sure what to do. + /// + /// Keyed by configuration rather than latched outright, because this + /// module's contract is that every call re-reads the live config so a + /// provider or login change is honoured without rebuilding the session. A + /// call whose signature differs from the recorded one tries again; only a + /// repeat under the same configuration is refused. The signature is this + /// tool's own view — its role order and the resolved providers — so one + /// tool's dead providers never answer for another's. + exhausted_for: Mutex>, } impl TinySearchTool { @@ -49,7 +56,7 @@ impl TinySearchTool { spec, config: Some(config), exposure: ToolExposure::Direct, - exhausted: Arc::new(AtomicBool::new(false)), + exhausted_for: Mutex::new(None), } } @@ -60,34 +67,46 @@ impl TinySearchTool { spec, config: None, exposure: ToolExposure::Direct, - exhausted: Arc::new(AtomicBool::new(false)), + exhausted_for: Mutex::new(None), } } - /// Recorded tools that share one exhaustion latch, the way the tools of a - /// live session do: the roles are three declarations over the same - /// providers, so one of them finding nothing settles it for all three. - pub fn recorded_batch(specs: impl IntoIterator) -> Vec { - let exhausted = Arc::new(AtomicBool::new(false)); - specs - .into_iter() - .map(|spec| Self { - spec, - config: None, - exposure: ToolExposure::Direct, - exhausted: exhausted.clone(), - }) - .collect() + /// Whether this tool already found nothing usable under `signature`. + pub(crate) fn is_exhausted_for(&self, signature: u64) -> bool { + self.exhausted_for + .lock() + .map(|recorded| *recorded == Some(signature)) + .unwrap_or(false) } - /// Whether this session already learned that no provider can answer. - pub(crate) fn is_exhausted(&self) -> bool { - self.exhausted.load(Ordering::Relaxed) + /// Record that no provider could answer under `signature`. Replaces any + /// earlier one, so the refusal always describes the current configuration. + pub(crate) fn mark_exhausted_for(&self, signature: u64) { + if let Ok(mut recorded) = self.exhausted_for.lock() { + *recorded = Some(signature); + } } - /// Record that no provider could answer, for the tools sharing this latch. - pub(crate) fn mark_exhausted(&self) { - self.exhausted.store(true, Ordering::Relaxed); + /// What this tool's providers look like right now: the order its role + /// draws from, and every provider's resolved reachability. Two calls agree + /// only while nothing a user could change — a key, a route, a provider + /// selection, a login — has moved. + pub(crate) fn provider_signature(&self, config: &Config) -> u64 { + let mut hasher = DefaultHasher::new(); + if let Some(role) = tinysearch_bus::role_for_tool(&self.spec.name) { + for provider in super::providers::role_order(config, role) { + provider.hash(&mut hasher); + } + } + for provider in super::providers::resolve(config) { + provider.id.hash(&mut hasher); + provider.enabled.hash(&mut hasher); + provider.usable.hash(&mut hasher); + provider.key_configured.hash(&mut hasher); + provider.managed_available.hash(&mut hasher); + matches!(provider.route, crate::config::SearchRoute::Managed).hash(&mut hasher); + } + hasher.finish() } pub fn spec(&self) -> &ToolSpec { @@ -185,14 +204,17 @@ impl Tool for TinySearchTool { args: Value, options: ToolCallOptions, ) -> anyhow::Result { - if self.is_exhausted() { + let config = self.live_config().await?; + // Re-read per call, so a key added or a provider switched mid-session + // clears an earlier refusal instead of outliving it. + let signature = self.provider_signature(&config); + if self.is_exhausted_for(signature) { tracing::debug!( tool = %self.spec.name, - "[search][tool] refused: no provider answered earlier in this session" + "[search][tool] refused: no provider answered under this configuration" ); return Ok(ToolResult::failed(SEARCH_EXHAUSTED_MESSAGE.to_string())); } - let config = self.live_config().await?; let subject = super::render::subject(&args); let max_results = args .get("max_results") @@ -230,12 +252,12 @@ impl Tool for TinySearchTool { } Err(error) => { if exhausts_providers(&error) { - self.mark_exhausted(); + self.mark_exhausted_for(signature); } tracing::warn!( tool = %self.spec.name, code = error_code(&error).unwrap_or("unclassified"), - exhausted = self.is_exhausted(), + exhausted = self.is_exhausted_for(signature), "[search][tool] failed" ); Ok(if exhausts_providers(&error) { @@ -267,17 +289,9 @@ pub fn build_search_tools(config: &Config) -> Vec> { "[search][tool] registered search tools" ); let shared = Arc::new(config.clone()); - let exhausted = Arc::new(AtomicBool::new(false)); specs .into_iter() - .map(|spec| { - Box::new(TinySearchTool { - spec, - config: Some(shared.clone()), - exposure: ToolExposure::Direct, - exhausted: exhausted.clone(), - }) as Box - }) + .map(|spec| Box::new(TinySearchTool::new(shared.clone(), spec)) as Box) .collect() } diff --git a/crates/openhuman-core/src/search/tools_tests.rs b/crates/openhuman-core/src/search/tools_tests.rs index 417f71f71ae..0bd99870516 100644 --- a/crates/openhuman-core/src/search/tools_tests.rs +++ b/crates/openhuman-core/src/search/tools_tests.rs @@ -113,15 +113,22 @@ fn standard_privacy_mode_allows_search_tool_dispatch() { // was what the hidden test rejected. // --------------------------------------------------------------------------- -fn role_specs() -> Vec { - ["web_search_tool", "web_answer_tool", "web_contents_tool"] +fn spec(name: &str) -> ToolSpec { + ToolSpec { + name: name.to_string(), + description: "search".into(), + parameters: serde_json::json!({"type": "object"}), + } +} + +/// A config whose managed providers are selected but whose direct keys are +/// absent, which is the shape that offers the tools and cannot serve them. +fn nothing_usable_config() -> Config { + let mut config = Config::default(); + config.search.providers = [("brave".to_string(), SearchProviderSettings::direct())] .into_iter() - .map(|name| ToolSpec { - name: name.to_string(), - description: "search".into(), - parameters: serde_json::json!({"type": "object"}), - }) - .collect() + .collect(); + config } #[test] @@ -134,7 +141,7 @@ fn the_unavailable_verdict_tells_the_model_to_stop_rather_than_wait() { ); assert!( !message.contains("right now"), - "nothing in the session will change the answer, so it must not read as a retry hint: {message}" + "nothing the model can do will change the answer, so it must not read as a retry hint: {message}" ); } @@ -154,37 +161,64 @@ fn only_the_exhaustion_verdict_is_treated_as_final() { } #[test] -fn a_session_starts_unlatched() { - for tool in TinySearchTool::recorded_batch(role_specs()) { - assert!(!tool.is_exhausted(), "{} started latched", tool.name()); - } +fn a_tool_starts_with_nothing_recorded() { + let config = nothing_usable_config(); + let tool = TinySearchTool::recorded(spec("web_search_tool")); + + assert!(!tool.is_exhausted_for(tool.provider_signature(&config))); } #[test] -fn one_role_finding_nothing_settles_it_for_the_others() { - // The three roles are declarations over the same providers, so dropping - // only the role that failed would still leave two tools that cannot work. - let tools = TinySearchTool::recorded_batch(role_specs()); - - tools[0].mark_exhausted(); - - for tool in &tools { - assert!( - tool.is_exhausted(), - "{} did not share the latch", - tool.name() - ); - } +fn the_refusal_answers_only_for_the_configuration_that_failed() { + // The contract this module documents is that every call re-reads the live + // config, so a provider or login change is honoured without rebuilding the + // session. A refusal that outlived a config change would break it: the user + // adds a key and search stays dead until the thread is abandoned. + let dead = nothing_usable_config(); + let tool = TinySearchTool::recorded(spec("web_search_tool")); + let dead_signature = tool.provider_signature(&dead); + + tool.mark_exhausted_for(dead_signature); + assert!(tool.is_exhausted_for(dead_signature)); + + let mut fixed = dead.clone(); + fixed.search.brave.api_key = Some("added-after-the-failure".into()); + + assert_ne!( + tool.provider_signature(&fixed), + dead_signature, + "adding a provider key must change what the tool sees" + ); + assert!( + !tool.is_exhausted_for(tool.provider_signature(&fixed)), + "adding a key must clear the refusal" + ); } -#[tokio::test] -async fn a_latched_tool_refuses_before_reaching_the_module() { - // The refusal is returned without loading config or calling the module, so - // a dead search costs the session one round trip rather than one per call. - let tools = TinySearchTool::recorded_batch(role_specs()); - tools[1].mark_exhausted(); +#[test] +fn one_tools_dead_providers_do_not_answer_for_another() { + // `build_search_tools` can offer provider-specific tools beside the role + // tools, and the roles draw on different provider orders, so a shared + // verdict would let one provider's outage disable unrelated ones. + let config = nothing_usable_config(); + let failed = TinySearchTool::recorded(spec("web_search_tool")); + let other = TinySearchTool::recorded(spec("web_answer_tool")); - let result = tools[0] + failed.mark_exhausted_for(failed.provider_signature(&config)); + + assert!(!other.is_exhausted_for(other.provider_signature(&config))); +} + +#[tokio::test] +async fn a_refused_call_does_not_reach_the_module() { + // The second call under the same configuration is answered from the + // recorded verdict, so a dead search costs one module round trip per + // configuration rather than one per call. + let config = nothing_usable_config(); + let tool = TinySearchTool::new(std::sync::Arc::new(config.clone()), spec("web_search_tool")); + tool.mark_exhausted_for(tool.provider_signature(&config)); + + let result = tool .execute(serde_json::json!({"query": "anything"})) .await .expect("a refusal is a reported failure, not a tool error"); @@ -193,7 +227,7 @@ async fn a_latched_tool_refuses_before_reaching_the_module() { assert_eq!( result.error_kind, Some(tinytools::ToolErrorKind::Failed), - "the failure is permanent, so it must not be tagged retryable" + "the failure is permanent for this configuration, so it must not be tagged retryable" ); assert!( result.text().contains("Do not call it again"),