From 43dd17c34baa5e5861c67bb1a9f9ad4dffb5e55d Mon Sep 17 00:00:00 2001 From: rubenssoto <36298331+rubenssoto@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:50:44 -0300 Subject: [PATCH 1/3] fix(tools): validate Brave search parameters Brave accepts fixed country and language values, but web_search exposed unconstrained strings and forwarded them verbatim. That allowed model-generated localization values to fail with HTTP 422. Scope the localization schema to the Brave provider, normalize optional values before sending them, and retry once without optional filters when Brave still rejects the request. --- crates/tools/src/web_search.rs | 119 +++++++----- crates/tools/src/web_search/brave.rs | 270 +++++++++++++++++++++++++++ 2 files changed, 338 insertions(+), 51 deletions(-) create mode 100644 crates/tools/src/web_search/brave.rs diff --git a/crates/tools/src/web_search.rs b/crates/tools/src/web_search.rs index a9c62c3c94..e83ae9cdb1 100644 --- a/crates/tools/src/web_search.rs +++ b/crates/tools/src/web_search.rs @@ -20,6 +20,8 @@ use { use crate::exec::EnvVarProvider; +mod brave; + /// Cached search result with expiry. struct CacheEntry { value: serde_json::Value, @@ -364,35 +366,23 @@ impl WebSearchTool { })); } - let mut url = format!( - "https://api.search.brave.com/res/v1/web/search?q={}&count={count}", - urlencoding::encode(query) - ); - - if let Some(country) = params.get("country").and_then(|v| v.as_str()) { - url.push_str(&format!("&country={country}")); - } - if let Some(lang) = params.get("search_lang").and_then(|v| v.as_str()) { - url.push_str(&format!("&search_lang={lang}")); - } - if let Some(lang) = params.get("ui_lang").and_then(|v| v.as_str()) { - url.push_str(&format!("&ui_lang={lang}")); - } - if let Some(freshness) = params.get("freshness").and_then(|v| v.as_str()) { - url.push_str(&format!("&freshness={freshness}")); - } - - let client = crate::shared_http_client(); + let brave_params = brave::Params::from_json(params); + let url = brave_params.request_url(query, count); + let mut resp = self + .send_brave_request(&url, accept_language, api_key) + .await?; - let mut req = client - .get(&url) - .timeout(self.timeout) - .header("Accept", "application/json") - .header("X-Subscription-Token", api_key); - if let Some(lang) = accept_language { - req = req.header("Accept-Language", lang); + if resp.status() == reqwest::StatusCode::UNPROCESSABLE_ENTITY + && brave_params.has_optional_filters() + { + debug!( + "Brave rejected optional search parameters; retrying without localization or freshness" + ); + let retry_url = brave::Params::default().request_url(query, count); + resp = self + .send_brave_request(&retry_url, accept_language, api_key) + .await?; } - let resp = req.send().await?; if !resp.status().is_success() { let status = resp.status(); @@ -420,6 +410,24 @@ impl WebSearchTool { })) } + async fn send_brave_request( + &self, + url: &str, + accept_language: Option<&str>, + api_key: &str, + ) -> crate::Result { + let client = crate::shared_http_client(); + let mut request = client + .get(url) + .timeout(self.timeout) + .header("Accept", "application/json") + .header("X-Subscription-Token", api_key); + if let Some(lang) = accept_language { + request = request.header("Accept-Language", lang); + } + Ok(request.send().await?) + } + async fn search_perplexity( &self, query: &str, @@ -732,36 +740,32 @@ impl AgentTool for WebSearchTool { } fn parameters_schema(&self) -> serde_json::Value { - serde_json::json!({ - "type": "object", - "properties": { - "query": { + let mut properties = serde_json::Map::from_iter([ + ( + "query".to_string(), + serde_json::json!({ "type": "string", "description": "The search query" - }, - "count": { + }), + ), + ( + "count".to_string(), + serde_json::json!({ "type": "integer", "description": "Number of results (1-10, default 5)", "minimum": 1, "maximum": 10 - }, - "country": { - "type": "string", - "description": "Country code for search results (e.g. 'US', 'GB')" - }, - "search_lang": { - "type": "string", - "description": "Search language (e.g. 'en')" - }, - "ui_lang": { - "type": "string", - "description": "UI language (e.g. 'en-US')" - }, - "freshness": { - "type": "string", - "description": "Freshness filter (Brave only): 'pd' (past day), 'pw' (past week), 'pm' (past month), 'py' (past year)" - } - }, + }), + ), + ]); + + if matches!(&self.provider, SearchProvider::Brave) { + properties.extend(brave::parameter_properties()); + } + + serde_json::json!({ + "type": "object", + "properties": properties, "required": ["query"] }) } @@ -899,6 +903,19 @@ mod tests { assert_eq!(tool.name(), "web_search"); let schema = tool.parameters_schema(); assert_eq!(schema["required"][0], "query"); + assert!(schema["properties"]["country"]["enum"].is_array()); + assert!(schema["properties"]["search_lang"]["enum"].is_array()); + assert!(schema["properties"]["ui_lang"]["enum"].is_array()); + } + + #[test] + fn test_non_brave_schema_omits_brave_parameters() { + let schema = perplexity_tool().parameters_schema(); + let properties = schema["properties"].as_object().expect("properties"); + assert!(!properties.contains_key("country")); + assert!(!properties.contains_key("search_lang")); + assert!(!properties.contains_key("ui_lang")); + assert!(!properties.contains_key("freshness")); } #[tokio::test] diff --git a/crates/tools/src/web_search/brave.rs b/crates/tools/src/web_search/brave.rs new file mode 100644 index 0000000000..109bccce82 --- /dev/null +++ b/crates/tools/src/web_search/brave.rs @@ -0,0 +1,270 @@ +use serde_json::{Map, Value, json}; + +const ENDPOINT: &str = "https://api.search.brave.com/res/v1/web/search"; + +// Keep these values aligned with Brave's Web Search API enums. They are intentionally +// provider-local: Perplexity, Firecrawl, and DuckDuckGo do not share this contract. +const COUNTRIES: &[&str] = &[ + "AR", "AU", "AT", "BE", "BR", "CA", "CL", "DK", "FI", "FR", "DE", "GR", "HK", "IN", "ID", "IT", + "JP", "KR", "MY", "MX", "NL", "NZ", "NO", "CN", "PL", "PT", "PH", "RU", "SA", "ZA", "ES", "SE", + "CH", "TW", "TR", "GB", "US", "ALL", +]; + +const SEARCH_LANGUAGES: &[&str] = &[ + "ar", "eu", "bn", "bg", "ca", "zh-hans", "zh-hant", "hr", "cs", "da", "nl", "en", "en-gb", + "et", "fi", "fr", "gl", "de", "el", "gu", "he", "hi", "hu", "is", "it", "ja", "jp", "kn", "ko", + "lv", "lt", "ms", "ml", "mr", "nb", "pl", "pt-br", "pt-pt", "pa", "ro", "ru", "sr", "sk", "sl", + "es", "sv", "ta", "te", "th", "tr", "uk", "vi", +]; + +const UI_LANGUAGES: &[&str] = &[ + "es-AR", "en-AU", "de-AT", "nl-BE", "fr-BE", "pt-BR", "en-CA", "fr-CA", "es-CL", "da-DK", + "fi-FI", "fr-FR", "de-DE", "el-GR", "zh-HK", "en-IN", "en-ID", "it-IT", "ja-JP", "ko-KR", + "en-MY", "es-MX", "nl-NL", "en-NZ", "no-NO", "zh-CN", "pl-PL", "en-PH", "ru-RU", "en-ZA", + "es-ES", "sv-SE", "fr-CH", "de-CH", "zh-TW", "tr-TR", "en-GB", "en-US", "es-US", +]; + +#[derive(Debug, Default, Eq, PartialEq)] +pub(super) struct Params { + country: Option<&'static str>, + search_lang: Option<&'static str>, + ui_lang: Option<&'static str>, + freshness: Option, +} + +impl Params { + pub(super) fn from_json(params: &Value) -> Self { + let requested_country = string_param(params, "country"); + let country = requested_country.map(|value| canonical(value, COUNTRIES).unwrap_or("ALL")); + + let requested_ui_lang = string_param(params, "ui_lang"); + let ui_lang = requested_ui_lang.and_then(|value| canonical(value, UI_LANGUAGES)); + + let search_lang = string_param(params, "search_lang") + .and_then(|value| normalize_search_language(value, country, ui_lang)); + let freshness = string_param(params, "freshness").and_then(normalize_freshness); + + Self { + country, + search_lang, + ui_lang, + freshness, + } + } + + pub(super) fn has_optional_filters(&self) -> bool { + self.country.is_some() + || self.search_lang.is_some() + || self.ui_lang.is_some() + || self.freshness.is_some() + } + + pub(super) fn request_url(&self, query: &str, count: u8) -> String { + let mut url = format!( + "{ENDPOINT}?q={}&count={count}", + super::urlencoding::encode(query) + ); + append_param(&mut url, "country", self.country); + append_param(&mut url, "search_lang", self.search_lang); + append_param(&mut url, "ui_lang", self.ui_lang); + append_param(&mut url, "freshness", self.freshness.as_deref()); + url + } +} + +pub(super) fn parameter_properties() -> Map { + Map::from_iter([ + ( + "country".to_string(), + json!({ + "type": "string", + "description": "Brave Search country market. Use ALL when the target country is not listed.", + "enum": COUNTRIES, + }), + ), + ( + "search_lang".to_string(), + json!({ + "type": "string", + "description": "Brave Search result language.", + "enum": SEARCH_LANGUAGES, + }), + ), + ( + "ui_lang".to_string(), + json!({ + "type": "string", + "description": "Brave Search response UI language.", + "enum": UI_LANGUAGES, + }), + ), + ( + "freshness".to_string(), + json!({ + "type": "string", + "description": "Brave freshness filter: pd (past day), pw (past week), pm (past month), py (past year), or YYYY-MM-DDtoYYYY-MM-DD." + }), + ), + ]) +} + +fn string_param<'a>(params: &'a Value, name: &str) -> Option<&'a str> { + params + .get(name) + .and_then(Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) +} + +fn canonical(value: &str, allowed: &'static [&'static str]) -> Option<&'static str> { + let normalized = value.replace('_', "-"); + allowed + .iter() + .copied() + .find(|candidate| candidate.eq_ignore_ascii_case(&normalized)) +} + +fn normalize_search_language( + value: &str, + country: Option<&str>, + ui_lang: Option<&str>, +) -> Option<&'static str> { + if let Some(language) = canonical(value, SEARCH_LANGUAGES) { + return Some(language); + } + + let normalized = value.replace('_', "-").to_ascii_lowercase(); + let base = normalized.split('-').next()?; + if let Some(language) = canonical(base, SEARCH_LANGUAGES) { + return Some(language); + } + + let mut candidates = SEARCH_LANGUAGES + .iter() + .copied() + .filter(|candidate| candidate.starts_with(&format!("{base}-"))); + let first = candidates.next()?; + if candidates.next().is_none() { + return Some(first); + } + + let preferred_region = ui_lang + .and_then(|locale| locale.split_once('-').map(|(_, region)| region)) + .or_else(|| country.filter(|country| *country != "ALL")); + preferred_region.and_then(|region| { + SEARCH_LANGUAGES.iter().copied().find(|candidate| { + candidate + .rsplit_once('-') + .is_some_and(|(_, suffix)| suffix.eq_ignore_ascii_case(region)) + && candidate.starts_with(&format!("{base}-")) + }) + }) +} + +fn normalize_freshness(value: &str) -> Option { + let normalized = match value.to_ascii_lowercase().as_str() { + "pd" | "day" => "pd", + "pw" | "week" => "pw", + "pm" | "month" => "pm", + "py" | "year" => "py", + _ if looks_like_date_range(value) => value, + _ => return None, + }; + Some(normalized.to_string()) +} + +fn looks_like_date_range(value: &str) -> bool { + let Some((start, end)) = value.split_once("to") else { + return false; + }; + [start, end].into_iter().all(|date| { + date.len() == 10 + && date.chars().enumerate().all(|(index, ch)| { + matches!(index, 4 | 7) && ch == '-' + || !matches!(index, 4 | 7) && ch.is_ascii_digit() + }) + }) +} + +fn append_param(url: &mut String, name: &str, value: Option<&str>) { + if let Some(value) = value { + url.push('&'); + url.push_str(name); + url.push('='); + url.push_str(&super::urlencoding::encode(value)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn normalizes_unsupported_market_without_region_specific_assumptions() { + let params = Params::from_json(&json!({ + "country": "PY", + "search_lang": "es", + "ui_lang": "es-PY", + })); + assert_eq!(params.country, Some("ALL")); + assert_eq!(params.search_lang, Some("es")); + assert_eq!(params.ui_lang, None); + } + + #[test] + fn infers_split_search_language_from_supported_region() { + let params = Params::from_json(&json!({ + "country": "br", + "search_lang": "pt", + "ui_lang": "pt_BR", + })); + assert_eq!(params.country, Some("BR")); + assert_eq!(params.search_lang, Some("pt-br")); + assert_eq!(params.ui_lang, Some("pt-BR")); + } + + #[test] + fn drops_invalid_optional_values_and_normalizes_freshness_aliases() { + let params = Params::from_json(&json!({ + "search_lang": "not-a-language", + "ui_lang": "not-a-locale", + "freshness": "month", + })); + assert_eq!(params.search_lang, None); + assert_eq!(params.ui_lang, None); + assert_eq!(params.freshness.as_deref(), Some("pm")); + } + + #[test] + fn request_url_only_contains_sanitized_brave_parameters() { + let params = Params::from_json(&json!({ + "country": "py", + "search_lang": "es-AR", + "ui_lang": "es-PY", + "freshness": "week", + })); + assert_eq!( + params.request_url("ração senior", 5), + concat!( + "https://api.search.brave.com/res/v1/web/search?", + "q=ra%C3%A7%C3%A3o%20senior&count=5&country=ALL&search_lang=es&freshness=pw" + ) + ); + } + + #[test] + fn accepts_documented_custom_date_range() { + let params = Params::from_json(&json!({ + "freshness": "2026-08-01to2026-08-25", + })); + assert_eq!(params.freshness.as_deref(), Some("2026-08-01to2026-08-25")); + } + + #[test] + fn schema_lists_only_supported_localization_values() { + let properties = parameter_properties(); + assert!(properties["country"]["enum"].as_array().is_some()); + assert!(properties["search_lang"]["enum"].as_array().is_some()); + assert!(properties["ui_lang"]["enum"].as_array().is_some()); + } +} From a035d594e200cdc4c2923cf026cd6df0b5bf0658 Mon Sep 17 00:00:00 2001 From: rubenssoto <36298331+rubenssoto@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:11:46 -0300 Subject: [PATCH 2/3] test(tools): cover Brave search retry flow Exercise the Brave HTTP path with a mock server so the 422 fallback cannot regress silently. Verify sanitized first-request parameters, a filter-free retry, preserved headers, successful retry parsing, no retry without filters, and propagation of the retry response error. --- crates/tools/src/web_search.rs | 139 ++++++++++++++++++++++++++- crates/tools/src/web_search/brave.rs | 8 +- 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/crates/tools/src/web_search.rs b/crates/tools/src/web_search.rs index e83ae9cdb1..8f45f851c2 100644 --- a/crates/tools/src/web_search.rs +++ b/crates/tools/src/web_search.rs @@ -48,6 +48,9 @@ pub struct WebSearchTool { ddg_blocked_until: Mutex>, /// Optional runtime env provider (credential store) for hot key updates. env_provider: Option>, + /// Test-only endpoint override used to exercise the complete Brave HTTP flow. + #[cfg(test)] + brave_endpoint_override: Option, } #[derive(Debug, Clone)] @@ -228,9 +231,26 @@ impl WebSearchTool { fallback_enabled, ddg_blocked_until: Mutex::new(None), env_provider: None, + #[cfg(test)] + brave_endpoint_override: None, } } + #[cfg(test)] + fn with_brave_endpoint(mut self, endpoint: String) -> Self { + self.brave_endpoint_override = Some(endpoint); + self + } + + fn brave_endpoint(&self) -> &str { + #[cfg(test)] + if let Some(endpoint) = self.brave_endpoint_override.as_deref() { + return endpoint; + } + + brave::ENDPOINT + } + /// Attach a runtime environment provider (credential store). pub fn with_env_provider(mut self, provider: Arc) -> Self { self.env_provider = Some(provider); @@ -367,7 +387,7 @@ impl WebSearchTool { } let brave_params = brave::Params::from_json(params); - let url = brave_params.request_url(query, count); + let url = brave_params.request_url(self.brave_endpoint(), query, count); let mut resp = self .send_brave_request(&url, accept_language, api_key) .await?; @@ -378,7 +398,8 @@ impl WebSearchTool { debug!( "Brave rejected optional search parameters; retrying without localization or freshness" ); - let retry_url = brave::Params::default().request_url(query, count); + let retry_url = + brave::Params::default().request_url(self.brave_endpoint(), query, count); resp = self .send_brave_request(&retry_url, accept_language, api_key) .await?; @@ -937,6 +958,120 @@ mod tests { assert!(result["hint"].as_str().unwrap().contains("BRAVE_API_KEY")); } + #[tokio::test] + async fn test_brave_retries_422_without_optional_filters() { + let mut server = mockito::Server::new_async().await; + let first_request = server + .mock("GET", "/res/v1/web/search") + .match_query(mockito::Matcher::Exact( + "q=edge%20query&count=5&country=ALL&search_lang=es&freshness=pw".into(), + )) + .match_header("x-subscription-token", "test-key") + .match_header("accept-language", "pt-BR") + .with_status(422) + .expect(1) + .create_async() + .await; + let retry_request = server + .mock("GET", "/res/v1/web/search") + .match_query(mockito::Matcher::Exact("q=edge%20query&count=5".into())) + .match_header("x-subscription-token", "test-key") + .match_header("accept-language", "pt-BR") + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"web":{"results":[{"title":"Recovered","url":"https://example.com","description":"Retry result"}]}}"#, + ) + .expect(1) + .create_async() + .await; + let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + + let result = tool + .search_brave( + "edge query", + 5, + &serde_json::json!({ + "country": "PY", + "search_lang": "es-AR", + "ui_lang": "es-PY", + "freshness": "week", + }), + Some("pt-BR"), + "test-key", + ) + .await + .unwrap(); + + assert_eq!(result["provider"], "brave"); + assert_eq!(result["query"], "edge query"); + assert_eq!(result["results"][0]["title"], "Recovered"); + first_request.assert_async().await; + retry_request.assert_async().await; + } + + #[tokio::test] + async fn test_brave_does_not_retry_422_without_optional_filters() { + let mut server = mockito::Server::new_async().await; + let request = server + .mock("GET", "/res/v1/web/search") + .match_query(mockito::Matcher::Exact("q=plain&count=5".into())) + .with_status(422) + .with_body("invalid query") + .expect(1) + .create_async() + .await; + let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + + let error = tool + .search_brave("plain", 5, &serde_json::json!({}), None, "test-key") + .await + .unwrap_err(); + + assert!(error.to_string().contains("422 Unprocessable Entity")); + assert!(error.to_string().contains("invalid query")); + request.assert_async().await; + } + + #[tokio::test] + async fn test_brave_surfaces_retry_response_error() { + let mut server = mockito::Server::new_async().await; + let first_request = server + .mock("GET", "/res/v1/web/search") + .match_query(mockito::Matcher::Exact( + "q=unavailable&count=5&country=US".into(), + )) + .with_status(422) + .expect(1) + .create_async() + .await; + let retry_request = server + .mock("GET", "/res/v1/web/search") + .match_query(mockito::Matcher::Exact("q=unavailable&count=5".into())) + .with_status(503) + .with_body("temporary outage") + .expect(1) + .create_async() + .await; + let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + + let error = tool + .search_brave( + "unavailable", + 5, + &serde_json::json!({"country": "US"}), + None, + "test-key", + ) + .await + .unwrap_err(); + + assert!(error.to_string().contains("503 Service Unavailable")); + assert!(error.to_string().contains("temporary outage")); + first_request.assert_async().await; + retry_request.assert_async().await; + } + #[tokio::test] async fn test_perplexity_missing_api_key_returns_hint() { let tool = perplexity_tool(); diff --git a/crates/tools/src/web_search/brave.rs b/crates/tools/src/web_search/brave.rs index 109bccce82..6cb47a7b17 100644 --- a/crates/tools/src/web_search/brave.rs +++ b/crates/tools/src/web_search/brave.rs @@ -1,6 +1,6 @@ use serde_json::{Map, Value, json}; -const ENDPOINT: &str = "https://api.search.brave.com/res/v1/web/search"; +pub(super) const ENDPOINT: &str = "https://api.search.brave.com/res/v1/web/search"; // Keep these values aligned with Brave's Web Search API enums. They are intentionally // provider-local: Perplexity, Firecrawl, and DuckDuckGo do not share this contract. @@ -59,9 +59,9 @@ impl Params { || self.freshness.is_some() } - pub(super) fn request_url(&self, query: &str, count: u8) -> String { + pub(super) fn request_url(&self, endpoint: &str, query: &str, count: u8) -> String { let mut url = format!( - "{ENDPOINT}?q={}&count={count}", + "{endpoint}?q={}&count={count}", super::urlencoding::encode(query) ); append_param(&mut url, "country", self.country); @@ -244,7 +244,7 @@ mod tests { "freshness": "week", })); assert_eq!( - params.request_url("ração senior", 5), + params.request_url(ENDPOINT, "ração senior", 5), concat!( "https://api.search.brave.com/res/v1/web/search?", "q=ra%C3%A7%C3%A3o%20senior&count=5&country=ALL&search_lang=es&freshness=pw" From 7ad57b78c2da289d25f1390a88a1fbf5a07ec449 Mon Sep 17 00:00:00 2001 From: rubenssoto <36298331+rubenssoto@users.noreply.github.com> Date: Tue, 25 Aug 2026 20:29:15 -0300 Subject: [PATCH 3/3] fix(tools): preserve Brave search context in cache Include normalized localization, freshness, and accepted language in Brave cache keys so distinct searches cannot reuse mismatched results. Also infer simplified or traditional Chinese from supported Brave regions. --- crates/tools/src/web_search.rs | 106 +++++++++++++++++++-------- crates/tools/src/web_search/brave.rs | 31 +++++++- 2 files changed, 105 insertions(+), 32 deletions(-) diff --git a/crates/tools/src/web_search.rs b/crates/tools/src/web_search.rs index 8f45f851c2..a945895957 100644 --- a/crates/tools/src/web_search.rs +++ b/crates/tools/src/web_search.rs @@ -371,11 +371,29 @@ impl WebSearchTool { } } + fn cache_key( + &self, + key_state: &str, + query: &str, + count: u8, + brave_params: &brave::Params, + accept_language: Option<&str>, + ) -> String { + let provider_context = match &self.provider { + SearchProvider::Brave => format!(":{brave_params:?}:{accept_language:?}"), + _ => String::new(), + }; + format!( + "{:?}:{key_state}:{query:?}:{count}{provider_context}", + self.provider + ) + } + async fn search_brave( &self, query: &str, count: u8, - params: &serde_json::Value, + params: &brave::Params, accept_language: Option<&str>, api_key: &str, ) -> crate::Result { @@ -386,14 +404,13 @@ impl WebSearchTool { })); } - let brave_params = brave::Params::from_json(params); - let url = brave_params.request_url(self.brave_endpoint(), query, count); + let url = params.request_url(self.brave_endpoint(), query, count); let mut resp = self .send_brave_request(&url, accept_language, api_key) .await?; if resp.status() == reqwest::StatusCode::UNPROCESSABLE_ENTITY - && brave_params.has_optional_filters() + && params.has_optional_filters() { debug!( "Brave rejected optional search parameters; retrying without localization or freshness" @@ -812,14 +829,14 @@ impl AgentTool for WebSearchTool { } else { "has-key" }; - let cache_key = format!("{:?}:{key_state}:{query}:{count}", self.provider); + let accept_language = params.get("_accept_language").and_then(|v| v.as_str()); + let brave_params = brave::Params::from_json(¶ms); + let cache_key = self.cache_key(key_state, query, count, &brave_params, accept_language); if let Some(cached) = self.cache_get(&cache_key) { debug!("web_search cache hit for: {query}"); return Ok(cached); } - let accept_language = params.get("_accept_language").and_then(|v| v.as_str()); - debug!("web_search: {query} (count={count})"); // When no API key is configured, skip the provider entirely and go @@ -835,7 +852,7 @@ impl AgentTool for WebSearchTool { } else { match &self.provider { SearchProvider::Brave => { - self.search_brave(query, count, ¶ms, accept_language, &api_key) + self.search_brave(query, count, &brave_params, accept_language, &api_key) .await? }, SearchProvider::Perplexity { @@ -939,6 +956,43 @@ mod tests { assert!(!properties.contains_key("freshness")); } + #[test] + fn test_brave_cache_key_uses_normalized_localization() { + let tool = brave_tool(); + let normalized = brave::Params::from_json(&serde_json::json!({ + "country": "BR", + "search_lang": "pt-br", + "ui_lang": "pt-BR", + "freshness": "pw", + })); + let aliases = brave::Params::from_json(&serde_json::json!({ + "country": "br", + "search_lang": "pt", + "ui_lang": "pt_BR", + "freshness": "week", + })); + let other_market = brave::Params::from_json(&serde_json::json!({ + "country": "US", + "search_lang": "en", + "ui_lang": "en-US", + "freshness": "pw", + })); + + let key = tool.cache_key("has-key", "query", 5, &normalized, Some("pt-BR")); + assert_eq!( + key, + tool.cache_key("has-key", "query", 5, &aliases, Some("pt-BR")) + ); + assert_ne!( + key, + tool.cache_key("has-key", "query", 5, &other_market, Some("pt-BR")) + ); + assert_ne!( + key, + tool.cache_key("has-key", "query", 5, &normalized, Some("en-US")) + ); + } + #[tokio::test] async fn test_missing_query_param() { let tool = brave_tool(); @@ -950,8 +1004,9 @@ mod tests { #[tokio::test] async fn test_brave_missing_api_key_returns_hint() { let tool = brave_tool(); + let params = brave::Params::default(); let result = tool - .search_brave("test", 5, &serde_json::json!({}), None, "") + .search_brave("test", 5, ¶ms, None, "") .await .unwrap(); assert!(result["error"].as_str().unwrap().contains("not configured")); @@ -987,19 +1042,15 @@ mod tests { .await; let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + let params = brave::Params::from_json(&serde_json::json!({ + "country": "PY", + "search_lang": "es-AR", + "ui_lang": "es-PY", + "freshness": "week", + })); + let result = tool - .search_brave( - "edge query", - 5, - &serde_json::json!({ - "country": "PY", - "search_lang": "es-AR", - "ui_lang": "es-PY", - "freshness": "week", - }), - Some("pt-BR"), - "test-key", - ) + .search_brave("edge query", 5, ¶ms, Some("pt-BR"), "test-key") .await .unwrap(); @@ -1023,8 +1074,9 @@ mod tests { .await; let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + let params = brave::Params::default(); let error = tool - .search_brave("plain", 5, &serde_json::json!({}), None, "test-key") + .search_brave("plain", 5, ¶ms, None, "test-key") .await .unwrap_err(); @@ -1055,14 +1107,10 @@ mod tests { .await; let tool = brave_tool().with_brave_endpoint(format!("{}/res/v1/web/search", server.url())); + let params = brave::Params::from_json(&serde_json::json!({"country": "US"})); + let error = tool - .search_brave( - "unavailable", - 5, - &serde_json::json!({"country": "US"}), - None, - "test-key", - ) + .search_brave("unavailable", 5, ¶ms, None, "test-key") .await .unwrap_err(); diff --git a/crates/tools/src/web_search/brave.rs b/crates/tools/src/web_search/brave.rs index 6cb47a7b17..d78df9458c 100644 --- a/crates/tools/src/web_search/brave.rs +++ b/crates/tools/src/web_search/brave.rs @@ -139,6 +139,17 @@ fn normalize_search_language( return Some(language); } + let preferred_region = ui_lang + .and_then(|locale| locale.split_once('-').map(|(_, region)| region)) + .or_else(|| country.filter(|country| *country != "ALL")); + if base == "zh" { + return match preferred_region { + Some("CN") => Some("zh-hans"), + Some("HK" | "TW") => Some("zh-hant"), + _ => None, + }; + } + let mut candidates = SEARCH_LANGUAGES .iter() .copied() @@ -148,9 +159,6 @@ fn normalize_search_language( return Some(first); } - let preferred_region = ui_lang - .and_then(|locale| locale.split_once('-').map(|(_, region)| region)) - .or_else(|| country.filter(|country| *country != "ALL")); preferred_region.and_then(|region| { SEARCH_LANGUAGES.iter().copied().find(|candidate| { candidate @@ -223,6 +231,23 @@ mod tests { assert_eq!(params.ui_lang, Some("pt-BR")); } + #[test] + fn infers_chinese_script_from_supported_region() { + let simplified = Params::from_json(&json!({ + "country": "CN", + "search_lang": "zh", + "ui_lang": "zh-CN", + })); + assert_eq!(simplified.search_lang, Some("zh-hans")); + + let traditional = Params::from_json(&json!({ + "country": "TW", + "search_lang": "zh", + "ui_lang": "zh-TW", + })); + assert_eq!(traditional.search_lang, Some("zh-hant")); + } + #[test] fn drops_invalid_optional_values_and_normalizes_freshness_aliases() { let params = Params::from_json(&json!({