fix(search): stop re-offering search a session cannot use

crates/openhuman-core/src/search/tools.rs

@@ -6,7 +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::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;
@@ -17,12 +19,35 @@ 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 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,
     /// Spawn-time config. `None` for a deferred instance rebuilt from a
     /// recorded transcript, which resolves the live config per call.
     config: Option<Arc<Config>>,
     exposure: ToolExposure,
+    /// 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 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<Option<u64>>,
 }
 
 impl TinySearchTool {
@@ -31,6 +56,7 @@ impl TinySearchTool {
             spec,
             config: Some(config),
             exposure: ToolExposure::Direct,
+            exhausted_for: Mutex::new(None),
         }
     }
 
@@ -41,7 +67,46 @@ impl TinySearchTool {
             spec,
             config: None,
             exposure: ToolExposure::Direct,
+            exhausted_for: Mutex::new(None),
+        }
+    }
+
+    /// 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)
+    }
+
+    /// 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);
+        }
+    }
+
+    /// 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 {
@@ -71,10 +136,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 +154,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 {
@@ -133,6 +205,16 @@ impl Tool for TinySearchTool {
         options: ToolCallOptions,
     ) -> anyhow::Result<ToolResult> {
         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 under this configuration"
+            );
+            return Ok(ToolResult::failed(SEARCH_EXHAUSTED_MESSAGE.to_string()));
+        }
         let subject = super::render::subject(&args);
         let max_results = args
             .get("max_results")
@@ -169,12 +251,20 @@ impl Tool for TinySearchTool {
                 ))
             }
             Err(error) => {
+                if exhausts_providers(&error) {
+                    self.mark_exhausted_for(signature);
+                }
                 tracing::warn!(
                     tool = %self.spec.name,
                     code = error_code(&error).unwrap_or("unclassified"),
+                    exhausted = self.is_exhausted_for(signature),
                     "[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))
+                })
             }
         }
     }

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,135 @@ 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 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()
+        .collect();
+    config
+}
+
+#[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 the model can do 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_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 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"
+    );
+}
+
+#[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"));
+
+    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");
+
+    assert!(result.is_error);
+    assert_eq!(
+        result.error_kind,
+        Some(tinytools::ToolErrorKind::Failed),
+        "the failure is permanent for this configuration, so it must not be tagged retryable"
+    );
+    assert!(
+        result.text().contains("Do not call it again"),
+        "{:?}",
+        result.text()
+    );
+}