From f1a4105962138c2067bce4e3b0676990b7f2ac57 Mon Sep 17 00:00:00 2001 From: orangeboyChen Date: Thu, 17 Sep 2026 18:03:55 +0800 Subject: [PATCH] fix(anthropic): carry server-tool hop metadata beside the response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #152 laid non-streaming server-tool blocks out per hop, but put the grouping it needed on the chat-completion payload itself. That payload is what an OpenAI-protocol client receives, so three things went wrong. ## 1. Hop metadata leaked into chat-completions responses `withIntermediateTurns` attached `turns` to the payload, and `Response.json` serialized it. `app/v1/chat/completions/route.ts` returns `proxyChatCompletions` unchanged, so `/v1/chat/completions` exposed a non-protocol `turns` field holding internal tool inputs and results — strict validators can reject it, and every client received the tool data twice. Carry the grouping out of band through a WeakMap, the way `serverToolExecutions` already is: `attachServerToolTurns` / `getServerToolTurns`. The Anthropic adapter reads it off the response instead of the payload. ## 2. Mixed server/client-tool turns lost the grouping When a later hop mixed a local server tool with a client-owned tool, `withIntermediateTurns` built the turns but the mixed branch took only `.message` off its result and never grouped the current hop. With no hop metadata the mapper flattened all reasoning and text ahead of all server-tool blocks — the exact ordering bug #152 set out to fix. Preserve the prior turns and add the current hop's calls to its entry. ## 3. A first hop that mixed tools dropped its own prose The mixed branch built the current hop with empty text and reasoning. When that hop was also the first, `withIntermediateTurns` had no earlier hops to fold and so returned no turns, and a block renderer renders purely from `turns` once it is non-empty — so everything the model said in that hop disappeared from the Anthropic response. `withIntermediateTurns` already builds this hop as its closing entry, so the calls are added to that entry rather than appended as a new one, which would repeat the prose. Only when there are no earlier hops is a fresh entry built, seeded with this iteration's text and reasoning. ## Verification - Three regression tests, one per issue. Each was confirmed to fail with its fix reverted and pass with it applied, so they guard the behaviour rather than the implementation. - `withIntermediateTurns` and `buildMixedTurnPayload` now return `{ payload, turns }`, and `ServerToolLoopResult.turns` is required rather than optional, so every return path states its grouping explicitly instead of relying on a `?? []` fallback. - 733 tests pass; typecheck, eslint, prettier, and build are clean. Coverage 94.67% statements. Rebased onto #156, which exports `withIntermediateTurns` to the image-generation loop; those call sites now take `.payload`. --- lib/server/proxy/anthropic.ts | 18 +- lib/server/proxy/codebuddy.ts | 23 +-- lib/server/proxy/image-generation.ts | 4 +- lib/server/proxy/web-search-loop.ts | 197 +++++++++++++++------- tests/server/web-search.test.ts | 241 +++++++++++++++++++++++++++ 5 files changed, 398 insertions(+), 85 deletions(-) diff --git a/lib/server/proxy/anthropic.ts b/lib/server/proxy/anthropic.ts index 38f1900..49c3ad6 100644 --- a/lib/server/proxy/anthropic.ts +++ b/lib/server/proxy/anthropic.ts @@ -11,6 +11,7 @@ import { proxyChatCompletions, type ChatRequestBody } from './codebuddy'; import { getServerToolStreamEvent, getServerToolExecutions, + getServerToolTurns, type ServerToolExecution, type ServerToolTurn, } from './web-search-loop'; @@ -138,12 +139,6 @@ interface OpenAIChatResponse { id?: string; model?: string; choices?: OpenAIChatChoice[]; - /** - * Per-hop grouping emitted by the local server-tool loop. Not part of the - * OpenAI protocol — it survives only as far as this file, which turns it into - * Anthropic content blocks. - */ - turns?: ServerToolTurn[]; usage?: OpenAIUsage; } @@ -1662,6 +1657,10 @@ export const handleMessagesRequest = async ( const model = String(chatBody.model ?? 'unknown'); const serverToolExecutions = getServerToolExecutions(upstreamResponse); + // Carried beside the response rather than inside it: the OpenAI-shaped + // payload the loop emits must stay protocol-clean for chat-completions + // clients, so this file reads the grouping off the response itself. + const turns = getServerToolTurns(upstreamResponse); if (body.stream) { return mapOpenAIStreamToAnthropicSSE(upstreamResponse, model); @@ -1670,12 +1669,7 @@ export const handleMessagesRequest = async ( const payload = (await upstreamResponse.json()) as OpenAIChatResponse; return Response.json( - mapOpenAIResponseToAnthropic( - payload, - model, - serverToolExecutions, - payload.turns, - ), + mapOpenAIResponseToAnthropic(payload, model, serverToolExecutions, turns), ); } catch (error) { return createAnthropicError( diff --git a/lib/server/proxy/codebuddy.ts b/lib/server/proxy/codebuddy.ts index 6992e2b..cb48022 100644 --- a/lib/server/proxy/codebuddy.ts +++ b/lib/server/proxy/codebuddy.ts @@ -36,6 +36,7 @@ import { } from '../search/tool'; import { attachServerToolExecutions, + attachServerToolTurns, type ChatCompletionPayload, executeWebSearchLoop, type ServerToolCallbacks, @@ -3116,11 +3117,17 @@ export const proxyChatCompletions = async ( } if (webSearch?.response) { - if (!webSearch.response.ok) { - return attachServerToolExecutions( - webSearch.response, - webSearch.executions, + // The hop grouping rides beside the response rather than inside its + // body, so the OpenAI-shaped payload a chat client receives stays + // protocol-clean. See `attachServerToolTurns`. + const attachServerToolResult = (response: Response): Response => + attachServerToolTurns( + attachServerToolExecutions(response, webSearch.executions), + webSearch.turns, ); + + if (!webSearch.response.ok) { + return attachServerToolResult(webSearch.response); } if ( @@ -3130,19 +3137,15 @@ export const proxyChatCompletions = async ( ?.toLowerCase() .includes('text/event-stream') ) { - return attachServerToolExecutions( + return attachServerToolResult( synthesizeChatCompletionStream( (await webSearch.response.json()) as ChatCompletionPayload, String(upstreamBody.model ?? 'unknown'), ), - webSearch.executions, ); } - return attachServerToolExecutions( - webSearch.response, - webSearch.executions, - ); + return attachServerToolResult(webSearch.response); } } diff --git a/lib/server/proxy/image-generation.ts b/lib/server/proxy/image-generation.ts index afe1c23..fabcf2f 100644 --- a/lib/server/proxy/image-generation.ts +++ b/lib/server/proxy/image-generation.ts @@ -419,7 +419,7 @@ export const executeImageGenerationLoop = async ({ payload, reasonings: [], texts: intermediateTexts, - }), + }).payload, ), }; } @@ -482,7 +482,7 @@ export const executeImageGenerationLoop = async ({ payload: lastPayload ?? {}, reasonings: [], texts: intermediateTexts, - }), + }).payload, ), }; }; diff --git a/lib/server/proxy/web-search-loop.ts b/lib/server/proxy/web-search-loop.ts index d70b08a..55d4518 100644 --- a/lib/server/proxy/web-search-loop.ts +++ b/lib/server/proxy/web-search-loop.ts @@ -88,15 +88,6 @@ export interface ChatCompletionPayload { id?: string; model?: string; object?: string; - /** - * One entry per server-tool hop, in the order the model produced them. - * - * Carries the grouping `message.content` / `reasoning_content` cannot: a - * multi-hop turn joins every hop into one string per kind, which loses where - * one hop's reasoning ends and the next begins. Absent when no hop ran, so - * callers fall back to the OpenAI-shaped fields. - */ - turns?: ServerToolTurn[]; usage?: unknown; } @@ -541,7 +532,7 @@ const buildMixedTurnPayload = ({ payload: ChatCompletionPayload; remainingCalls: ChatCompletionToolCall[]; searchResults: string[]; - usage: unknown; + usage?: unknown; }): ChatCompletionPayload => { const existingText = typeof message?.content === 'string' && message.content.trim() @@ -624,8 +615,11 @@ const buildIntermediateTurns = ({ * dropping it hides the model's reasoning from the user and leaves the * client's transcript out of step with what the model actually said. * - * The same hops are also re-grouped into `turns`, because the folded strings - * cannot express where one hop ends and the next begins. + * The same hops are also re-grouped into the `turns` half of the result, + * because the folded strings cannot express where one hop ends and the next + * begins. That half travels beside the response rather than inside it: the + * payload is what an OpenAI-protocol client receives, and `turns` is not part + * of that protocol, so it is handed over out of band like `executions`. * * Shared with the image-generation loop, which has the same shape: a local * tool call is replayed with its result appended, so only the final hop's @@ -641,13 +635,13 @@ export const withIntermediateTurns = ({ payload: ChatCompletionPayload; reasonings: string[]; texts: string[]; -}): ChatCompletionPayload => { +}): { payload: ChatCompletionPayload; turns: ServerToolTurn[] } => { const extraText = texts.filter(Boolean).join('\n\n'); const extraReasoning = reasonings.filter(Boolean).join('\n\n'); const [first, ...rest] = payload.choices ?? []; if (!first) { - return payload; + return { payload, turns: [] }; } const message = first.message ?? {}; @@ -658,7 +652,7 @@ export const withIntermediateTurns = ({ // still have run tools, which is exactly the case a caller consuming `turns` // needs: a hop that called a tool without speaking first is still a hop. if (!extraText && !extraReasoning && !executions.length) { - return payload; + return { payload, turns: [] }; } const content = [extraText, existingText].filter(Boolean).join('\n\n'); @@ -667,7 +661,20 @@ export const withIntermediateTurns = ({ .join('\n\n'); return { - ...payload, + payload: { + ...payload, + choices: [ + { + ...first, + message: { + ...message, + content, + ...(reasoning ? { reasoning_content: reasoning } : {}), + }, + }, + ...rest, + ], + }, // Per-hop grouping for renderers that can express it. The joined strings // above stay as the OpenAI-shaped view; a client that builds Anthropic // content blocks needs to know where one hop's reasoning ends and the next @@ -678,17 +685,6 @@ export const withIntermediateTurns = ({ reasonings: [...reasonings, readReasoning(message)], texts: [...texts, existingText], }), - choices: [ - { - ...first, - message: { - ...message, - content, - ...(reasoning ? { reasoning_content: reasoning } : {}), - }, - }, - ...rest, - ], }; }; @@ -703,6 +699,17 @@ export interface ServerToolLoopResult { body: ChatRequestBody; executions: ServerToolExecution[]; response: Response | null; + /** + * One entry per server-tool hop, in the order the model produced them. + * + * Carries the grouping `message.content` / `reasoning_content` cannot: a + * multi-hop turn joins every hop into one string per kind, which loses where + * one hop's reasoning ends and the next begins. Travels beside the response + * rather than inside it, because it is not part of the OpenAI protocol — a + * block renderer reads it off the response through `getServerToolTurns`. + * Empty when no hop ran. + */ + turns: ServerToolTurn[]; } export type ServerToolInvocation = @@ -799,6 +806,29 @@ export const getServerToolExecutions = ( response: Response, ): ServerToolExecution[] => serverToolExecutions.get(response) ?? []; +/** + * Per-hop grouping, kept off the wire for the same reason `executions` is: it + * is not part of the OpenAI protocol, so a `/v1/chat/completions` client must + * not see it — a strict validator can reject the extra field, and the tool + * data would otherwise be sent twice. + */ +const serverToolTurns = new WeakMap(); + +export const attachServerToolTurns = ( + response: Response, + turns: ServerToolTurn[], +): Response => { + if (turns.length) { + serverToolTurns.set(response, turns); + } + + return response; +}; + +export const getServerToolTurns = ( + response: Response, +): ServerToolTurn[] | undefined => serverToolTurns.get(response); + export type ServerToolUpstreamMode = 'buffer' | 'detect-both' | 'detect-fetch' | 'detect-search' | 'stream'; @@ -1157,7 +1187,7 @@ const createInlineServerToolStream = async ({ const contentType = firstResponse.headers.get('content-type') ?? ''; if (!contentType.toLowerCase().includes('text/event-stream')) { - return { body, executions: [], response: firstResponse }; + return { body, executions: [], response: firstResponse, turns: [] }; } const executions: ServerToolExecution[] = []; @@ -1635,6 +1665,9 @@ const createInlineServerToolStream = async ({ status: firstResponse.status, statusText: firstResponse.statusText, }), + // The streamed path already emits each hop in order, so no grouping has + // to be reconstructed downstream. + turns: [], }; }; @@ -1690,7 +1723,12 @@ export const executeWebSearchLoop = async ({ // `tools` still have to reach the caller: it forwards them upstream, and the // stripped declarations have to stay stripped on that path too. if (!executes) { - return { body: { ...body, tools }, executions: [], response: null }; + return { + body: { ...body, tools }, + executions: [], + response: null, + turns: [], + }; } const messages: JsonRecord[] = body.messages as JsonRecord[]; @@ -1738,7 +1776,7 @@ export const executeWebSearchLoop = async ({ ?.toLowerCase() .includes('text/event-stream') ) { - return { body: loopBody, executions, response }; + return { body: loopBody, executions, response, turns: [] }; } // The payload is only needed to detect a tool call or a failure, so read @@ -1752,6 +1790,7 @@ export const executeWebSearchLoop = async ({ body: loopBody, executions, response: await buildServerToolFailureResponse(response), + turns: [], }; } @@ -1841,28 +1880,57 @@ export const executeWebSearchLoop = async ({ // turn. The findings ride along in the message text only for routes that // cannot render them structurally; see `buildMixedTurnPayload`. if (remainingCalls.length) { + const { payload: folded, turns: priorTurns } = withIntermediateTurns({ + executions: intermediateExecutions, + payload, + reasonings: intermediateReasonings, + texts: intermediateTexts, + }); + // The hops already run, plus this one's own: it called server tools + // before handing the client's calls back, so it is a hop like any other + // and has to stay grouped with them. Dropping it would leave the block + // renderer with no grouping at all, flattening every hop's prose ahead + // of the tool blocks. + // + // `withIntermediateTurns` already built this hop as its closing entry — + // the one that carries the current message's prose — but without the + // calls, because it runs before they exist. So the calls are added to + // that entry rather than appended as a new hop, which would repeat the + // prose. Only when there are no earlier hops does it return nothing and + // a fresh entry has to be built here. + const currentTurn: ServerToolTurn = { + // With no earlier hops there is no closing entry to carry the prose, + // so this hop's own text and reasoning are used directly. They are + // what `withIntermediateTurns` folded into `folded` above, and a block + // renderer renders purely from `turns` once it is non-empty, so + // leaving them out would drop everything the model said here. + ...(priorTurns.at(-1) ?? { + reasoning: iterationReasoning, + text: iterationText, + }), + executions: results.map((result) => result.execution), + }; + const mixedTurns: ServerToolTurn[] = [ + ...priorTurns.slice(0, -1), + currentTurn, + ]; + const mixed = buildMixedTurnPayload({ + findingsAsStructuredBlocks: callbacks?.findingsAsStructuredBlocks, + // `buildMixedTurnPayload` reads this iteration's text and reasoning + // off `message`, so only the earlier iterations go on top; the + // current one is folded in by the helper itself. + message: folded.choices?.[0]?.message, + payload, + remainingCalls, + searchResults: results.map((result) => result.content), + usage, + }); + return { body: loopBody, executions, - response: Response.json( - buildMixedTurnPayload({ - findingsAsStructuredBlocks: callbacks?.findingsAsStructuredBlocks, - // `buildMixedTurnPayload` reads this iteration's text and reasoning - // off `message`, so only the earlier iterations go on top; the - // current one is folded in by the helper itself. - message: withIntermediateTurns({ - payload, - executions: intermediateExecutions, - reasonings: intermediateReasonings, - texts: intermediateTexts, - }).choices?.[0]?.message, - payload, - remainingCalls, - searchResults: results.map((result) => result.content), - usage, - }), - { status: response.status }, - ), + response: Response.json(mixed, { status: response.status }), + turns: mixedTurns, }; } @@ -1922,42 +1990,49 @@ export const executeWebSearchLoop = async ({ body: loopBody, executions, response: await buildServerToolFailureResponse(finalResponse), + turns: [], }; } + const final = withIntermediateTurns({ + executions: intermediateExecutions, + payload, + reasonings: intermediateReasonings, + texts: intermediateTexts, + }); + return { body: loopBody, executions, response: Response.json( { - ...withIntermediateTurns({ - payload, - executions: intermediateExecutions, - reasonings: intermediateReasonings, - texts: intermediateTexts, - }), + ...final.payload, ...(usage ? { usage } : {}), }, { status: finalResponse.status }, ), + turns: final.turns, }; } + const final = withIntermediateTurns({ + executions: intermediateExecutions, + payload, + reasonings: intermediateReasonings, + texts: intermediateTexts, + }); + return { body: loopBody, executions, response: Response.json( { - ...withIntermediateTurns({ - payload, - executions: intermediateExecutions, - reasonings: intermediateReasonings, - texts: intermediateTexts, - }), + ...final.payload, ...(usage ? { usage } : {}), }, { status: response!.status }, ), + turns: final.turns, }; }; diff --git a/tests/server/web-search.test.ts b/tests/server/web-search.test.ts index 621393e..02341f4 100644 --- a/tests/server/web-search.test.ts +++ b/tests/server/web-search.test.ts @@ -6075,6 +6075,247 @@ describe('proxy integration', () => { ).toEqual(['server_tool_use', 'web_fetch_tool_result', 'text:Done.']); }); + it('keeps earlier hops grouped when a later hop mixes in a client tool', async () => { + await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy' }); + let upstreamCalls = 0; + + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = String(input); + + if (url.includes('/agenttool/v1/webfetch')) { + return makeJsonResponse({ content: 'Fetched body.' }); + } + + upstreamCalls += 1; + + // The first hop searches on its own; the second runs a server tool and + // also asks the client for one of its own. The loop has to hand the + // client's call back, and the hop metadata still has to reach the + // block renderer — losing it flattens every hop's prose ahead of the + // tool blocks, which is the bug this guards. + return upstreamCalls === 1 + ? makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + content: 'Checking first.', + role: 'assistant', + tool_calls: [ + { + id: 'call_first', + function: { + arguments: '{"url":"https://page.test/a"}', + name: 'web_fetch', + }, + }, + ], + }, + }, + ], + }) + : makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + content: 'Now yours.', + role: 'assistant', + tool_calls: [ + { + id: 'call_second', + function: { + arguments: '{"url":"https://page.test/b"}', + name: 'web_fetch', + }, + }, + { + id: 'call_client', + function: { + arguments: '{"city":"Berlin"}', + name: 'weather', + }, + }, + ], + }, + }, + ], + }); + }); + + const response = await handleMessagesRequest( + new NextRequest('http://localhost/v1/messages', { method: 'POST' }), + { + max_tokens: 1024, + messages: [{ role: 'user', content: 'Read both' }], + tools: [ + { type: 'web_fetch_20260209', name: 'web_fetch', input_schema: {} }, + { name: 'weather', input_schema: {}, type: 'custom' }, + ], + }, + ); + + const payload = (await response.json()) as { + content: Array<{ text?: string; type: string }>; + }; + + // The first hop stays ahead of the second hop's fetch instead of both + // fetches collapsing to the end, and the client's own call survives as a + // tool_use the client has to resolve. + expect( + payload.content.map((block) => + block.type === 'text' ? `text:${block.text}` : block.type, + ), + ).toEqual([ + 'text:Checking first.', + 'server_tool_use', + 'web_fetch_tool_result', + 'text:Now yours.', + 'server_tool_use', + 'web_fetch_tool_result', + 'tool_use', + ]); + }); + + it('keeps hop metadata off the OpenAI chat-completions response', async () => { + await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy' }); + let upstreamCalls = 0; + + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = String(input); + + if (url.includes('/agenttool/v1/webfetch')) { + return makeJsonResponse({ content: 'Fetched body.' }); + } + + upstreamCalls += 1; + + return upstreamCalls === 1 + ? makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + content: 'Looking it up.', + role: 'assistant', + tool_calls: [ + { + id: 'call_fetch', + function: { + arguments: '{"url":"https://page.test/a"}', + name: 'web_fetch', + }, + }, + ], + }, + }, + ], + }) + : makeJsonResponse({ + choices: [{ finish_reason: 'stop', message: { content: 'Done.' } }], + }); + }); + + const response = await proxyChatCompletions(makeProxyRequest(), { + messages: [{ role: 'user', content: 'Read https://page.test/a' }], + tools: [ + { name: 'web_fetch', type: 'function' }, + { type: 'web_fetch_20260209', name: 'web_fetch' }, + ], + }); + + const payload = (await response.json()) as Record; + + // The per-hop grouping is not part of the OpenAI protocol. Serializing it + // here would hand chat clients a field that names internal tool inputs and + // results, which strict validators reject and every other client receives + // as duplicated tool data. + expect(Object.keys(payload)).not.toContain('turns'); + expect(JSON.stringify(payload)).not.toContain('web_fetch_tool_result'); + // Guards against the assertion passing because the loop never ran: a + // two-hop turn is what would have carried the grouping in the first place. + expect(upstreamCalls).toBe(2); + }); + + it('keeps the prose of a first hop that mixes in a client tool', async () => { + await updateSettings({ CODEBUDDY_WEB_FETCH_BACKEND: 'codebuddy' }); + + vi.spyOn(globalThis, 'fetch').mockImplementation(async (input) => { + const url = String(input); + + if (url.includes('/agenttool/v1/webfetch')) { + return makeJsonResponse({ content: 'Fetched body.' }); + } + + // The very first response already carries both a locally executed tool + // and a client-owned one. There is no earlier hop to fold, so the hop + // metadata has to be built from this iteration alone. + return makeJsonResponse({ + choices: [ + { + finish_reason: 'tool_calls', + message: { + content: 'Let me check, then you decide.', + reasoning_content: 'I need the page first.', + role: 'assistant', + tool_calls: [ + { + id: 'call_fetch', + function: { + arguments: '{"url":"https://page.test/a"}', + name: 'web_fetch', + }, + }, + { + id: 'call_client', + function: { + arguments: '{"city":"Berlin"}', + name: 'weather', + }, + }, + ], + }, + }, + ], + }); + }); + + const response = await handleMessagesRequest( + new NextRequest('http://localhost/v1/messages', { method: 'POST' }), + { + max_tokens: 1024, + messages: [{ role: 'user', content: 'Read it' }], + tools: [ + { type: 'web_fetch_20260209', name: 'web_fetch', input_schema: {} }, + { name: 'weather', input_schema: {}, type: 'custom' }, + ], + }, + ); + + const payload = (await response.json()) as { + content: Array<{ text?: string; thinking?: string; type: string }>; + }; + + // A block renderer renders purely from the hop metadata once it is + // non-empty, so this hop's prose has to be on the turn: leaving it off + // drops everything the model said here, not just reorders it. + expect( + payload.content.map((block) => + block.type === 'thinking' + ? `thinking:${block.thinking}` + : block.type === 'text' + ? `text:${block.text}` + : block.type, + ), + ).toEqual([ + 'thinking:I need the page first.', + 'text:Let me check, then you decide.', + 'server_tool_use', + 'web_fetch_tool_result', + 'tool_use', + ]); + }); + it('passes through untouched when no search tool is declared', async () => { process.env.SEARXNG_URL = 'https://searx.test'; resetWebSearchProviders();