fix(search): always use native Anthropic web search when available (#374)
Three bugs prevented native web_search from being used with Anthropic: 1. model_select never fires on session restore (SDK's modelsAreEqual guard suppresses it when the default model matches the restored one), so isAnthropicProvider stayed false and native search was never injected. Fix: fall back to detecting "claude-" in the payload model name when model_select hasn't fired. 2. Custom search tools (search-the-web, search_and_read) were only removed from the payload when BRAVE_API_KEY was missing. With the key present, the model saw both native and Brave tools and often picked the Brave ones, which failed with network errors. Fix: always remove custom search tools from Anthropic requests. 3. google_search (Gemini-based) was not included in the disable list, so the model could call it even without GEMINI_API_KEY set. Fix: new CUSTOM_SEARCH_TOOL_NAMES list covers all three custom search tools.
This commit is contained in:
Juan Francisco Lebrero
GitHub
parent
c69be2078f
commit
f6f1f9aa27
2 changed files with 87 additions and 31 deletions
|
|
@ -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<string, unknown>;
|
||||
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",
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> = {
|
||||
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;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue