From 7de71a3e22748e05cdd7b82fe08e14733c1ece9d Mon Sep 17 00:00:00 2001 From: orangeboyChen Date: Thu, 17 Sep 2026 16:32:56 +0800 Subject: [PATCH 1/2] fix(server-tools): leave a client-declared web_fetch to the client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Responses API has no `web_fetch` server tool — only `web_search`. A client that declares one as a plain function owns it and resolves it itself. But the fetch branch of `replaceServerTools` took the call over whenever a backend was configured, before it ever asked whether the declaration was client-owned: if (isWebFetchTool(tool)) { if (fetchEnabled && fetchProvider) { ...take over... } if (!isServerDeclaredFetchTool(tool)) { return [tool]; } So choosing `codebuddy` or `codebuddy2api` silently disabled a tool the client had declared and depended on — the client never saw the call come back. Only the `passthrough` default kept it working, which is why this went unnoticed. Reorder so the client-owned check comes first, matching what the search branch already does. The backend setting chooses who runs the *proxy's* tool; it is not a licence to take the client's. Verified end to end on the Responses route: a Codex-declared `web_fetch` is now handed back unresolved, with the proxy neither fetching the page nor re-asking upstream. Two existing tests encoded the old behaviour and are updated: - `takes over a client-declared web_fetch function when a backend is set` asserted the takeover directly. It now asserts the loop declines to touch the request, which is the behaviour it was named for. - Two `runOnce` cases built the proxy's own definition by hand instead of going through the Responses translator, so it arrived unmarked and looked client-owned. They now use `translateResponsesToolsToChat`, which is what actually marks an Anthropic-typed declaration as server-side. --- lib/server/proxy/web-search-loop.ts | 13 ++-- tests/server/server-tools.test.ts | 106 +++++++++++++++++++++++----- tests/server/web-search.test.ts | 57 +++++++++++++++ 3 files changed, 156 insertions(+), 20 deletions(-) diff --git a/lib/server/proxy/web-search-loop.ts b/lib/server/proxy/web-search-loop.ts index 11c75cc..e4d53f6 100644 --- a/lib/server/proxy/web-search-loop.ts +++ b/lib/server/proxy/web-search-loop.ts @@ -338,6 +338,15 @@ const replaceServerTools = ({ } if (isWebFetchTool(tool)) { + // A client-owned function of the same name wins over the backend, exactly + // as it does for search. The backend setting chooses who runs the *proxy's* + // tool; it is not a licence to take over a tool the client declared and + // resolves itself. Without this, a client that ships its own `web_fetch` + // loses it the moment a deployment picks a backend. + if (!isServerDeclaredFetchTool(tool)) { + return [tool]; + } + if (fetchEnabled && fetchProvider) { matched = true; executes = true; @@ -345,10 +354,6 @@ const replaceServerTools = ({ return [{ type: 'function', function: buildWebFetchToolDefinition() }]; } - if (!isServerDeclaredFetchTool(tool)) { - return [tool]; - } - matched = true; return [stripServerToolMarker(tool)]; } diff --git a/tests/server/server-tools.test.ts b/tests/server/server-tools.test.ts index 95d74cc..6f4a801 100644 --- a/tests/server/server-tools.test.ts +++ b/tests/server/server-tools.test.ts @@ -1373,10 +1373,14 @@ describe('server tool backends', () => { expect(isMarkedServerTool(result?.body.tools?.[0])).toBe(false); }); - it('takes over a client-declared web_fetch function when a backend is set', async () => { - // Regression guard: with an executable backend the proxy must run the - // tool itself. Leaving it to the client here silently disabled the - // setting for clients that happen to declare `web_fetch` themselves. + it('leaves a client-declared web_fetch function alone when a backend is set', async () => { + // The backend setting chooses who runs the *proxy's* tool, not whether + // the proxy may take over one the client declared. A client that ships + // its own `web_fetch` keeps resolving it — otherwise picking a backend + // would silently disable a capability the client asked for. + // + // The client's own parameters are the proof: the proxy's definition has + // `properties.url`, the client's here is `{ type: 'object' }`. await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy2api', CODEBUDDY_WEB_SEARCH_BACKEND: 'passthrough', @@ -1403,16 +1407,9 @@ describe('server tool backends', () => { }), }); - const tools = (result?.body.tools ?? []) as Array<{ - function: { name: string; parameters: Record }; - }>; - - // Replaced with the proxy's definition, so the loop — not the client — - // resolves the call. - expect(tools).toHaveLength(1); - expect(tools[0]?.function.name).toBe('web_fetch'); - expect(tools[0]?.function.parameters).toHaveProperty('properties.url'); - expect(tools[0]?.function.parameters).toHaveProperty('properties.prompt'); + // Nothing matched a server-tool declaration, so the loop declines to + // touch the request at all. + expect(result).toBeNull(); }); it('keeps a client function of the same name when fetch cannot run', async () => { @@ -1632,6 +1629,18 @@ describe('server tool backends', () => { }); describe('proxy integration', () => { + /** + * A `web_fetch` declaration as it arrives from a client: an Anthropic + * server-tool type run through the Responses translator, which is what + * marks it as provider-executed. Declaring the proxy's own definition + * directly would skip that step and arrive unmarked — indistinguishable + * from a tool the client owns. + */ + const translatedFetchTools = (): unknown[] => + translateResponsesToolsToChat([ + { type: 'web_fetch_20250910', name: 'web_fetch' }, + ]) ?? []; + const runOnce = async ({ fetchImpl, tools, @@ -1706,7 +1715,7 @@ describe('server tool backends', () => { status: 200, }); }, - tools: [{ type: 'function', function: buildWebFetchToolDefinition() }], + tools: translatedFetchTools(), }); const payload = (await response.json()) as { @@ -1765,7 +1774,7 @@ describe('server tool backends', () => { status: 200, }); }, - tools: [{ type: 'function', function: buildWebFetchToolDefinition() }], + tools: translatedFetchTools(), }); const payload = (await response.json()) as { @@ -1983,6 +1992,71 @@ describe('server tool backends', () => { resetWebSearchProviders(); }); + it('hands a client-declared web_fetch call back even with a backend set', async () => { + // End-to-end companion to the declaration-level guard: the backend is + // executable, so the only thing keeping this the client's call is the + // declaration being client-owned. The model calls the tool, the proxy + // must not answer it — the unresolved call goes back to the client. + await updateSettings({ + CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy2api', + CODEBUDDY_WEB_SEARCH_BACKEND: 'passthrough', + }); + + let upstreamCalls = 0; + let pageFetches = 0; + const response = await runOnce({ + fetchImpl: async (...args: unknown[]) => { + const url = String(args[0]); + + if (url.includes('a.test/page')) { + pageFetches += 1; + + return new Response('

Page body

', { + headers: { 'Content-Type': 'text/html' }, + status: 200, + }); + } + + upstreamCalls += 1; + + return makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + tool_calls: [ + { + id: 'c1', + function: { + arguments: '{"url":"https://a.test/page"}', + name: 'web_fetch', + }, + }, + ], + }, + }, + ], + }); + }, + tools: [ + { + type: 'function', + function: { name: 'web_fetch', parameters: { type: 'object' } }, + }, + ], + }); + + const payload = (await response.json()) as { + choices: Array<{ message: { tool_calls?: unknown[] } }>; + }; + + // The proxy never fetched the page and never re-asked upstream; the call + // comes back for the client to resolve. + expect(pageFetches).toBe(0); + expect(upstreamCalls).toBe(1); + expect(payload.choices[0]?.message.tool_calls).toHaveLength(1); + }); + it('leaves a client-declared web_fetch function alone when disabled', async () => { await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'passthrough', diff --git a/tests/server/web-search.test.ts b/tests/server/web-search.test.ts index c2e7e35..e2c46e1 100644 --- a/tests/server/web-search.test.ts +++ b/tests/server/web-search.test.ts @@ -3565,6 +3565,63 @@ describe('chat proxy web search integration', () => { ); }); + it('leaves a client-declared web_fetch to the client on the Responses route', async () => { + // The Responses API has no `web_fetch` server tool — only `web_search`. A + // client that declares one as a plain function owns it and resolves it + // itself, and no backend setting changes that: the setting chooses who runs + // the *proxy's* tool, not whether the proxy may take the client's. + await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy' }); + let upstreamCalls = 0; + let pageFetches = 0; + + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = String(input); + + if (url.includes('/agenttool/v1/webfetch') || url.includes('page.test')) { + pageFetches += 1; + + return makeJsonResponse({ content: 'Fetched body.' }); + } + + upstreamCalls += 1; + + return makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + role: 'assistant', + tool_calls: [ + { + id: 'call_fetch', + function: { + arguments: '{"url":"https://page.test/a"}', + name: 'web_fetch', + }, + }, + ], + }, + }, + ], + }); + }); + + const response = await handleResponsesRequest( + makeNextRequest('http://localhost/v1/responses', { method: 'POST' }), + { + input: 'Fetch the page', + tools: [{ type: 'function', name: 'web_fetch' }], + }, + ); + const body = await response.text(); + + // The proxy neither fetched nor re-asked upstream: the call is handed back + // for the client to resolve. + expect(pageFetches).toBe(0); + expect(upstreamCalls).toBe(1); + expect(body).not.toContain('open_page'); + }); + it('streams a Responses open_page lifecycle for local fetch', async () => { await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy' }); let upstreamCalls = 0; From 5649a449fd68883c7336f66e0d79e2173972e458 Mon Sep 17 00:00:00 2001 From: orangeboyChen Date: Thu, 17 Sep 2026 16:56:36 +0800 Subject: [PATCH 2/2] fix(server-tools): consult ownership when classifying fetched calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The declaration-level guard kept a client's `web_fetch` out of the proxy's hands, but the synchronous loop still classified calls by name and backend alone: (Boolean(fetchProvider) && isWebFetchToolCall(toolCall)) So a request carrying both a server-declared `web_search` and a client-owned `web_fetch` — search makes the loop run, which activates the fetch backend — had the client's fetch executed instead of handed back. Ownership is decided when the declarations are rewritten, and the streaming paths already consulted it via `ownedNames`; only this path did not. Route it through `isLocalServerToolCall` like the streaming paths, and thread `ownedNames` into the loop scope. Verified by inspection at the decision point — `ownedNames` is `["websearch"]`, so `web_search` is executed and `web_fetch` is returned — and by a test that fails (two upstream rounds instead of one) when the classification is reverted. --- lib/server/proxy/web-search-loop.ts | 82 +++++++++++--- tests/server/server-tools.test.ts | 164 ++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+), 13 deletions(-) diff --git a/lib/server/proxy/web-search-loop.ts b/lib/server/proxy/web-search-loop.ts index e4d53f6..5844ba0 100644 --- a/lib/server/proxy/web-search-loop.ts +++ b/lib/server/proxy/web-search-loop.ts @@ -312,19 +312,29 @@ const replaceServerTools = ({ searchPassthrough: boolean; searchProvider: WebSearchProvider | null; tools: unknown; -}): { executes: boolean; tools: unknown[] } | null => { +}): { + executes: boolean; + /** Canonical names the proxy took over, so call classification can tell its own calls from a client's. */ + ownedNames: Set; + tools: unknown[]; +} | null => { if (!Array.isArray(tools) || !tools.length) { return null; } let matched = false; let executes = false; + // Names the proxy is executing itself. A client may declare its own tool + // under the same name, and the loop must not answer those calls: matching + // the name is not enough to own it. + const ownedNames = new Set(); const rewritten = tools.flatMap((tool): unknown[] => { if (isWebSearchTool(tool)) { if (searchEnabled && searchProvider) { matched = true; executes = true; + ownedNames.add(normalizeToolName(WEB_SEARCH_TOOL_NAME)); return [{ type: 'function', function: buildWebSearchToolDefinition() }]; } @@ -350,6 +360,7 @@ const replaceServerTools = ({ if (fetchEnabled && fetchProvider) { matched = true; executes = true; + ownedNames.add(normalizeToolName(WEB_FETCH_TOOL_NAME)); return [{ type: 'function', function: buildWebFetchToolDefinition() }]; } @@ -362,7 +373,7 @@ const replaceServerTools = ({ return [stripServerToolMarker(tool)]; }); - return matched ? { executes, tools: rewritten } : null; + return matched ? { executes, ownedNames, tools: rewritten } : null; }; /** @@ -840,15 +851,36 @@ interface ServerToolProbe { */ const isLocalServerToolCall = ({ fetchProvider, + ownedNames, toolCall, searchProvider, }: { fetchProvider: WebFetchProvider | null; + /** + * Canonical names the proxy took over. Without it a client's own tool that + * happens to share a name — `web_fetch`, which is not a server tool in the + * Responses API — gets executed by the loop instead of handed back. + */ + ownedNames?: Set; toolCall: ChatCompletionToolCall; searchProvider: WebSearchProvider | null; }): boolean => - (Boolean(searchProvider) && isWebSearchToolCall(toolCall)) || - (Boolean(fetchProvider) && isWebFetchToolCall(toolCall)); + (Boolean(searchProvider) && + isWebSearchToolCall(toolCall) && + isOwned(ownedNames, WEB_SEARCH_TOOL_NAME)) || + (Boolean(fetchProvider) && + isWebFetchToolCall(toolCall) && + isOwned(ownedNames, WEB_FETCH_TOOL_NAME)); + +/** + * Whether the proxy owns calls to `name`. + * + * `undefined` means the caller predates ownership tracking; those callers only + * ever run the proxy's own declarations, so they are unaffected by client tools + * of the same name. + */ +const isOwned = (ownedNames: Set | undefined, name: string): boolean => + !ownedNames || ownedNames.has(normalizeToolName(name)); const probeServerToolStream = async ({ canContinue, @@ -856,6 +888,7 @@ const probeServerToolStream = async ({ emitRaw, fetchProvider, onReader, + ownedNames, response, searchProvider, }: { @@ -870,6 +903,7 @@ const probeServerToolStream = async ({ }; emitRaw: (frame: string) => void; fetchProvider: WebFetchProvider | null; + ownedNames?: Set; /** * Hands the active reader to the caller's cancellation path. Without it a * disconnect cannot interrupt a read that is already parked: the loop only @@ -983,7 +1017,12 @@ const probeServerToolStream = async ({ const toolCalls = aggregateStreamingToolCalls(toolCallDeltas); const isLocalCall = (toolCall: ChatCompletionToolCall): boolean => - isLocalServerToolCall({ fetchProvider, searchProvider, toolCall }); + isLocalServerToolCall({ + fetchProvider, + ownedNames, + searchProvider, + toolCall, + }); return { content, @@ -1002,6 +1041,7 @@ const createInlineServerToolStream = async ({ callbacks, callUpstream, fetchProvider, + ownedNames, searchProvider, }: { body: ChatRequestBody; @@ -1011,6 +1051,7 @@ const createInlineServerToolStream = async ({ mode: ServerToolUpstreamMode, ) => Promise; fetchProvider: WebFetchProvider | null; + ownedNames?: Set; searchProvider: WebSearchProvider | null; }): Promise => { const firstResponse = await callUpstream(body, 'stream'); @@ -1028,7 +1069,12 @@ const createInlineServerToolStream = async ({ // A call is locally executable only when its backend is available; anything // else stays the client's to answer. const isLocalCall = (toolCall: ChatCompletionToolCall): boolean => - isLocalServerToolCall({ fetchProvider, searchProvider, toolCall }); + isLocalServerToolCall({ + fetchProvider, + ownedNames, + searchProvider, + toolCall, + }); const emitJson = ( controller: ReadableStreamDefaultController, @@ -1083,6 +1129,7 @@ const createInlineServerToolStream = async ({ onReader: (reader) => { activeReader = reader; }, + ownedNames, response: firstResponse, searchProvider, }); @@ -1256,6 +1303,7 @@ const createInlineServerToolStream = async ({ onReader: (reader) => { activeReader = reader; }, + ownedNames, response, searchProvider, }); @@ -1405,6 +1453,7 @@ const createInlineServerToolStream = async ({ onReader: (reader) => { activeReader = reader; }, + ownedNames, response, searchProvider, }); @@ -1529,7 +1578,7 @@ export const executeWebSearchLoop = async ({ return null; } - const { executes, tools } = replacement; + const { executes, ownedNames, tools } = replacement; // Nothing can be executed, so there is nothing to loop for. The rewritten // `tools` still have to reach the caller: it forwards them upstream, and the @@ -1562,6 +1611,7 @@ export const executeWebSearchLoop = async ({ callbacks, callUpstream, fetchProvider, + ownedNames, searchProvider, }); } @@ -1599,13 +1649,19 @@ export const executeWebSearchLoop = async ({ const message = payload.choices?.[0]?.message; const toolCalls = message?.tool_calls ?? []; - const localCalls = toolCalls.filter( - (toolCall) => - (Boolean(searchProvider) && isWebSearchToolCall(toolCall)) || - (Boolean(fetchProvider) && isWebFetchToolCall(toolCall)), - ); + // The same ownership test the streaming paths use. Matching the name alone + // would execute a client's own `web_fetch` whenever a backend is + // configured, instead of handing the call back. + const isLocalCall = (toolCall: ChatCompletionToolCall): boolean => + isLocalServerToolCall({ + fetchProvider, + ownedNames, + searchProvider, + toolCall, + }); + const localCalls = toolCalls.filter(isLocalCall); const remainingCalls = toolCalls.filter( - (toolCall) => !localCalls.includes(toolCall), + (toolCall) => !isLocalCall(toolCall), ); if (!localCalls.length) { diff --git a/tests/server/server-tools.test.ts b/tests/server/server-tools.test.ts index 6f4a801..4af81ee 100644 --- a/tests/server/server-tools.test.ts +++ b/tests/server/server-tools.test.ts @@ -1992,6 +1992,94 @@ describe('server tool backends', () => { resetWebSearchProviders(); }); + it('runs server search while still handing a client web_fetch back', async () => { + // The case the declaration-level guard alone does not cover: a + // server-declared search makes the loop run, so the proxy's fetch + // backend is active and the model calls both tools in one turn. Call + // classification has to consult ownership, not just the name — + // otherwise the client's `web_fetch` is executed here even though its + // declaration was left untouched. + process.env.SEARXNG_URL = 'https://searx.test'; + resetWebSearchProviders(); + await updateSettings({ + CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy2api', + CODEBUDDY_WEB_SEARCH_BACKEND: 'searxng', + }); + + let upstreamCalls = 0; + + const result = await executeWebSearchLoop({ + body: { + messages: [{ content: 'hi', role: 'user' }], + tools: [ + ...(translateResponsesToolsToChat([ + { type: 'web_search_preview' }, + ]) ?? []), + { + type: 'function', + function: { name: 'web_fetch', parameters: { type: 'object' } }, + }, + ], + } as ChatRequestBody, + callUpstream: async () => { + upstreamCalls += 1; + + return upstreamCalls === 1 + ? makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + role: 'assistant', + tool_calls: [ + { + id: 'c1', + function: { + arguments: '{"url":"https://a.test/page"}', + name: 'web_fetch', + }, + }, + { + id: 'c2', + function: { + arguments: '{"query":"q"}', + name: 'web_search', + }, + }, + ], + }, + }, + ], + }) + : makeJsonResponse({ + choices: [ + { finish_reason: 'stop', message: { content: 'ANSWER' } }, + ], + }); + }, + }); + + // The loop stopped after one turn: the search ran, and the fetch was + // returned to the client rather than answered by another round. + expect(upstreamCalls).toBe(1); + // Only the client's fetch is still outstanding — the search was executed + // locally, so it is gone from the call list. + const payload = await readPayload(result); + const toolCalls = ( + payload.choices as Array<{ + message?: { + tool_calls?: Array<{ function?: { name?: string } }>; + }; + }> + )?.[0]?.message?.tool_calls; + expect(toolCalls?.map((call) => call.function?.name)).toEqual([ + 'web_fetch', + ]); + + delete process.env.SEARXNG_URL; + resetWebSearchProviders(); + }); + it('hands a client-declared web_fetch call back even with a backend set', async () => { // End-to-end companion to the declaration-level guard: the backend is // executable, so the only thing keeping this the client's call is the @@ -2057,6 +2145,82 @@ describe('server tool backends', () => { expect(payload.choices[0]?.message.tool_calls).toHaveLength(1); }); + it('keeps a client fetch out of the loop when a server search runs', async () => { + // The declaration-level guard alone is not enough: a server-declared + // search starts the loop, and once it is running the call classifier + // used to match on name alone — so the client's `web_fetch` was executed + // alongside the search it had nothing to do with. + await updateSettings({ + CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy2api', + CODEBUDDY_WEB_SEARCH_BACKEND: 'searxng', + }); + process.env.SEARXNG_URL = 'https://searx.test'; + resetWebSearchProviders(); + + let upstreamCalls = 0; + const result = await executeWebSearchLoop({ + body: { + messages: [{ content: 'hi', role: 'user' }], + tools: [ + // A server tool the proxy owns... + { + type: 'web_search_20260209', + name: 'web_search', + input_schema: {}, + }, + // ...and a client-owned tool the proxy must not touch. + { + type: 'function', + function: { name: 'web_fetch', parameters: { type: 'object' } }, + }, + ], + } as ChatRequestBody, + callUpstream: async () => { + upstreamCalls += 1; + + return upstreamCalls === 1 + ? makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + tool_calls: [ + { + id: 'c1', + function: { + arguments: '{"url":"https://a.test/page"}', + name: 'web_fetch', + }, + }, + { + id: 'c2', + function: { + arguments: '{"query":"q"}', + name: 'web_search', + }, + }, + ], + }, + }, + ], + }) + : makeJsonResponse({ + choices: [ + { finish_reason: 'stop', message: { content: 'Done.' } }, + ], + }); + }, + }); + + // Only the search ran. The client's fetch is left for the client. + expect(result?.executions.map((execution) => execution.type)).toEqual([ + 'web_search', + ]); + + delete process.env.SEARXNG_URL; + resetWebSearchProviders(); + }); + it('leaves a client-declared web_fetch function alone when disabled', async () => { await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'passthrough',