diff --git a/src/resources/extensions/search-the-web/native-search.ts b/src/resources/extensions/search-the-web/native-search.ts index 7aac943a7..e3e2cbafc 100644 --- a/src/resources/extensions/search-the-web/native-search.ts +++ b/src/resources/extensions/search-the-web/native-search.ts @@ -8,6 +8,9 @@ /** Tool names for the Brave-backed custom search tools */ export const BRAVE_TOOL_NAMES = ["search-the-web", "search_and_read"]; +/** All custom search tool names that should be disabled when native search is active */ +export const CUSTOM_SEARCH_TOOL_NAMES = ["search-the-web", "search_and_read", "google_search"]; + /** Thinking block types that require signature validation by the API */ const THINKING_TYPES = new Set(["thinking", "redacted_thinking"]); @@ -57,27 +60,30 @@ export function stripThinkingFromHistory( */ export function registerNativeSearchHooks(pi: NativeSearchPI): { getIsAnthropic: () => boolean } { let isAnthropicProvider = false; + let modelSelectFired = false; // Track provider changes via model selection — also handles diagnostics // since model_select fires AFTER session_start and knows the provider. pi.on("model_select", async (event: any, ctx: any) => { + modelSelectFired = true; const wasAnthropic = isAnthropicProvider; isAnthropicProvider = event.model.provider === "anthropic"; const hasBrave = !!process.env.BRAVE_API_KEY; - // When Anthropic + no Brave key: disable custom search tools (they'd fail) - if (isAnthropicProvider && !hasBrave) { + // When Anthropic: disable all custom search tools — native web_search is + // server-side and more reliable. Custom tools cause confusion and failures. + if (isAnthropicProvider) { const active = pi.getActiveTools(); pi.setActiveTools( - active.filter((t: string) => !BRAVE_TOOL_NAMES.includes(t)) + active.filter((t: string) => !CUSTOM_SEARCH_TOOL_NAMES.includes(t)) ); - } else if (!isAnthropicProvider && wasAnthropic && !hasBrave) { - // Switching away from Anthropic without Brave — re-enable so the user - // sees the "missing key" error rather than tools silently vanishing. - // Only add tools not already active to avoid duplicates on repeated toggles. + } else if (!isAnthropicProvider && wasAnthropic) { + // Switching away from Anthropic — re-enable custom search tools (they + // were disabled while native search was active). If keys are missing, + // user sees the error rather than tools silently vanishing. const active = pi.getActiveTools(); - const toAdd = BRAVE_TOOL_NAMES.filter((t) => !active.includes(t)); + const toAdd = CUSTOM_SEARCH_TOOL_NAMES.filter((t) => !active.includes(t)); if (toAdd.length > 0) { pi.setActiveTools([...active, ...toAdd]); } @@ -99,11 +105,17 @@ export function registerNativeSearchHooks(pi: NativeSearchPI): { getIsAnthropic: const payload = event.payload as Record; if (!payload) return; - // Only inject native web search for confirmed Anthropic provider. - // model_select sets isAnthropicProvider via the provider field. - // Model name prefix is NOT sufficient — other providers (GitHub Copilot, - // AWS Bedrock, etc.) serve claude-* models through non-Anthropic APIs. - if (!isAnthropicProvider) return; + // Detect Anthropic provider. Prefer the model_select flag when available, + // but fall back to checking the model name in the payload. model_select + // may not fire when the session restores with the same model already set + // (modelsAreEqual guard in the SDK suppresses the event). When model_select + // HAS fired and said "not Anthropic" (e.g. Copilot serving claude-*), + // respect that — don't override with model name heuristic. + const modelName = typeof payload.model === "string" ? payload.model : ""; + const isAnthropic = modelSelectFired + ? isAnthropicProvider + : modelName.startsWith("claude-"); + if (!isAnthropic) return; // Strip thinking blocks from history to avoid signature validation errors // caused by the SDK dropping server_tool_use/web_search_tool_result blocks. @@ -119,16 +131,13 @@ export function registerNativeSearchHooks(pi: NativeSearchPI): { getIsAnthropic: // Don't double-inject if already present if (tools.some((t) => t.type === "web_search_20250305")) return; - // When no Brave key, remove Brave-based search tool definitions from the - // payload so Claude doesn't see (and try to call) broken tools. - // This is more reliable than setActiveTools since model_select may not fire. - const hasBrave = !!process.env.BRAVE_API_KEY; - if (!hasBrave) { - tools = tools.filter( - (t) => !BRAVE_TOOL_NAMES.includes(t.name as string) - ); - payload.tools = tools; - } + // Always remove custom search tool definitions from Anthropic requests. + // Native web_search is server-side and more reliable — keeping both confuses + // the model and causes it to pick custom tools which can fail with network errors. + tools = tools.filter( + (t) => !CUSTOM_SEARCH_TOOL_NAMES.includes(t.name as string) + ); + payload.tools = tools; tools.push({ type: "web_search_20250305", diff --git a/src/tests/native-search.test.ts b/src/tests/native-search.test.ts index 2f2474b23..7e818260c 100644 --- a/src/tests/native-search.test.ts +++ b/src/tests/native-search.test.ts @@ -4,6 +4,7 @@ import { registerNativeSearchHooks, stripThinkingFromHistory, BRAVE_TOOL_NAMES, + CUSTOM_SEARCH_TOOL_NAMES, type NativeSearchPI, } from "../resources/extensions/search-the-web/native-search.ts"; @@ -22,7 +23,7 @@ interface MockHandler { function createMockPI() { const handlers: MockHandler[] = []; - let activeTools = ["search-the-web", "search_and_read", "fetch_page", "bash", "read"]; + let activeTools = ["search-the-web", "search_and_read", "google_search", "fetch_page", "bash", "read"]; const notifications: Array<{ message: string; level: string }> = []; const mockCtx = { @@ -98,6 +99,34 @@ test("before_provider_request injects web_search for claude models", async () => assert.equal((tools as any[]).length, 2, "Should have original + injected tool"); }); +test("before_provider_request injects web_search for claude models even without model_select", async () => { + const pi = createMockPI(); + registerNativeSearchHooks(pi); + + // NO model_select fired — simulates session restore where modelsAreEqual suppresses the event + const payload: Record = { + model: "claude-opus-4-6", + tools: [ + { name: "bash", type: "custom" }, + { name: "search-the-web", type: "function" }, + { name: "google_search", type: "function" }, + ], + }; + + const result = await pi.fire("before_provider_request", { + type: "before_provider_request", + payload, + }); + + const tools = ((result as any)?.tools ?? payload.tools) as any[]; + const names = tools.map((t: any) => t.name ?? t.type); + + assert.ok(names.includes("web_search"), "Should inject native web_search based on model name"); + assert.ok(!names.includes("search-the-web"), "Should remove search-the-web"); + assert.ok(!names.includes("google_search"), "Should remove google_search"); + assert.ok(names.includes("bash"), "Should keep non-search tools"); +}); + test("before_provider_request does NOT inject for non-claude models", async () => { const pi = createMockPI(); registerNativeSearchHooks(pi); @@ -230,6 +259,7 @@ test("model_select disables Brave tools when Anthropic + no BRAVE_API_KEY", asyn const active = pi.getActiveTools(); assert.ok(!active.includes("search-the-web"), "search-the-web should be disabled"); assert.ok(!active.includes("search_and_read"), "search_and_read should be disabled"); + assert.ok(!active.includes("google_search"), "google_search should be disabled"); assert.ok(active.includes("fetch_page"), "fetch_page should remain active"); assert.ok(active.includes("bash"), "Other tools should remain active"); } finally { @@ -238,7 +268,7 @@ test("model_select disables Brave tools when Anthropic + no BRAVE_API_KEY", asyn } }); -test("model_select keeps Brave tools when BRAVE_API_KEY is set", async () => { +test("model_select disables all custom search tools when Anthropic even with BRAVE_API_KEY", async () => { const originalKey = process.env.BRAVE_API_KEY; process.env.BRAVE_API_KEY = "test-key"; @@ -254,8 +284,10 @@ test("model_select keeps Brave tools when BRAVE_API_KEY is set", async () => { }); const active = pi.getActiveTools(); - assert.ok(active.includes("search-the-web"), "search-the-web should stay active"); - assert.ok(active.includes("search_and_read"), "search_and_read should stay active"); + assert.ok(!active.includes("search-the-web"), "search-the-web should be disabled for Anthropic"); + assert.ok(!active.includes("search_and_read"), "search_and_read should be disabled for Anthropic"); + assert.ok(!active.includes("google_search"), "google_search should be disabled for Anthropic"); + assert.ok(active.includes("fetch_page"), "fetch_page should remain active"); } finally { if (originalKey) process.env.BRAVE_API_KEY = originalKey; else delete process.env.BRAVE_API_KEY; @@ -292,6 +324,7 @@ test("model_select re-enables Brave tools when switching away from Anthropic", a active = pi.getActiveTools(); assert.ok(active.includes("search-the-web"), "search-the-web should be re-enabled"); assert.ok(active.includes("search_and_read"), "search_and_read should be re-enabled"); + assert.ok(active.includes("google_search"), "google_search should be re-enabled"); } finally { if (originalKey) process.env.BRAVE_API_KEY = originalKey; else delete process.env.BRAVE_API_KEY; @@ -387,6 +420,10 @@ test("BRAVE_TOOL_NAMES contains expected tool names", () => { assert.deepEqual(BRAVE_TOOL_NAMES, ["search-the-web", "search_and_read"]); }); +test("CUSTOM_SEARCH_TOOL_NAMES contains all custom search tools", () => { + assert.deepEqual(CUSTOM_SEARCH_TOOL_NAMES, ["search-the-web", "search_and_read", "google_search"]); +}); + test("before_provider_request removes Brave tools from payload when no BRAVE_API_KEY", async () => { const originalKey = process.env.BRAVE_API_KEY; delete process.env.BRAVE_API_KEY; @@ -408,6 +445,7 @@ test("before_provider_request removes Brave tools from payload when no BRAVE_API { name: "bash", type: "function" }, { name: "search-the-web", type: "function" }, { name: "search_and_read", type: "function" }, + { name: "google_search", type: "function" }, { name: "fetch_page", type: "function" }, ], }; @@ -422,6 +460,7 @@ test("before_provider_request removes Brave tools from payload when no BRAVE_API assert.ok(!names.includes("search-the-web"), "search-the-web should be removed from payload"); assert.ok(!names.includes("search_and_read"), "search_and_read should be removed from payload"); + assert.ok(!names.includes("google_search"), "google_search should be removed from payload"); assert.ok(names.includes("bash"), "bash should remain"); assert.ok(names.includes("fetch_page"), "fetch_page should remain"); assert.ok(names.includes("web_search"), "native web_search should be injected"); @@ -431,7 +470,7 @@ test("before_provider_request removes Brave tools from payload when no BRAVE_API } }); -test("before_provider_request keeps Brave tools in payload when BRAVE_API_KEY set", async () => { +test("before_provider_request removes all custom search tools from payload even with BRAVE_API_KEY", async () => { const originalKey = process.env.BRAVE_API_KEY; process.env.BRAVE_API_KEY = "test-key"; @@ -451,6 +490,8 @@ test("before_provider_request keeps Brave tools in payload when BRAVE_API_KEY se tools: [ { name: "search-the-web", type: "function" }, { name: "search_and_read", type: "function" }, + { name: "google_search", type: "function" }, + { name: "fetch_page", type: "function" }, ], }; @@ -462,9 +503,11 @@ test("before_provider_request keeps Brave tools in payload when BRAVE_API_KEY se const tools = ((result as any)?.tools ?? payload.tools) as any[]; const names = tools.map((t: any) => t.name); - assert.ok(names.includes("search-the-web"), "search-the-web should remain with Brave key"); - assert.ok(names.includes("search_and_read"), "search_and_read should remain with Brave key"); - assert.ok(names.includes("web_search"), "native web_search should also be injected"); + assert.ok(!names.includes("search-the-web"), "search-the-web should be removed for Anthropic"); + assert.ok(!names.includes("search_and_read"), "search_and_read should be removed for Anthropic"); + assert.ok(!names.includes("google_search"), "google_search should be removed for Anthropic"); + assert.ok(names.includes("fetch_page"), "fetch_page should remain"); + assert.ok(names.includes("web_search"), "native web_search should be injected"); } finally { if (originalKey) process.env.BRAVE_API_KEY = originalKey; else delete process.env.BRAVE_API_KEY; @@ -527,6 +570,10 @@ test("model_select re-enable does not duplicate Brave tools across toggle cycles active.filter((t) => t === "search_and_read").length, 1, "search_and_read exactly once (no duplicates)" ); + assert.equal( + active.filter((t) => t === "google_search").length, 1, + "google_search exactly once (no duplicates)" + ); } finally { if (originalKey) process.env.BRAVE_API_KEY = originalKey; else delete process.env.BRAVE_API_KEY;