From 26a3038a887276c9203e08c14f735ba606e89be9 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:25:34 +0900 Subject: [PATCH 1/4] fix(server): keep a reconstructed Grok terminal inside the request's tool selection The sparse-terminal repair rebuilds a terminal output from the output_item.done events it collected, and it received a budget but nothing about what the request had actually selected. The undeclared-tool guard answers a different question -- whether a name was declared -- so a request sent with tool_choice: "none", with a forced selector naming another tool, or with an allowed_tools list that excludes the call still received that call back through the repair. Read the boundary from the final outbound body, after every removal, rename and translation, and apply it to what the repair publishes. A catalog that ends up empty there authorizes no client call whatever the selector still says; an absent catalog states no boundary, exactly as it states none for the declaration guard. Keep the failure narrow and visible. Only the offending item is withheld, so the assistant text that arrived in the same turn still reaches the client instead of being discarded with it, and the withheld position is kept so the contiguity proof still covers the whole output. Because the turn no longer ended the way the upstream said it did, the reconstructed terminal is published as response.incomplete with incomplete_details.reason forbidden_tool_call rather than as a clean response.completed with a quietly shorter output. The raw stream is still forwarded untouched; policing it remains the declaration guard's job. Co-authored-by: luvs01 --- scripts/test-layout/layout.json | 1 + src/server/grok-responses-snapshot-repair.ts | 115 ++++++++- src/server/responses-request-tool-scope.ts | 121 +++++++++ src/server/responses-undeclared-tool-guard.ts | 5 +- src/server/responses/passthrough-delivery.ts | 2 +- tests/fixtures/test-layout-expected.json | 1 + ...sponses-sparse-terminal-tool-scope.test.ts | 235 ++++++++++++++++++ 7 files changed, 468 insertions(+), 12 deletions(-) create mode 100644 src/server/responses-request-tool-scope.ts create mode 100644 tests/responses/responses-sparse-terminal-tool-scope.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index b0a1b3b81ea..33383b78959 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1295,6 +1295,7 @@ "responses-show-thinking-summary.test.ts": "responses", "responses-snapshot-repair-server.test.ts": "responses", "responses-snapshot-repair.test.ts": "responses", + "responses-sparse-terminal-tool-scope.test.ts": "responses", "responses-spill-shutdown-clock.test.ts": "responses", "responses-state-write-amplification.test.ts": "responses", "responses-state.test.ts": "responses", diff --git a/src/server/grok-responses-snapshot-repair.ts b/src/server/grok-responses-snapshot-repair.ts index a667f44e551..5cbb32fd9c8 100644 --- a/src/server/grok-responses-snapshot-repair.ts +++ b/src/server/grok-responses-snapshot-repair.ts @@ -1,8 +1,9 @@ /** Strict terminal reconstruction selected by the Grok compatibility marker. */ import type { TranslatorBudget } from "../lib/translator-budget"; import { MAX_COMPLETED_OUTPUT_ITEMS, MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES } from "./relay"; -import { sseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite"; +import { replaceSseDataPayload, sseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite"; import { isPlainObject, jsonBlock, type RetainedOutputItem } from "./responses-snapshot-codec"; +import { requestToolScope, type RequestToolScope } from "./responses-request-tool-scope"; type SparseTerminalOpenItem = { type: string; @@ -16,6 +17,20 @@ type SparseTerminalCompletedItem = RetainedOutputItem & { const MAX_GROK_OPEN_ITEM_IDENTITY_BYTES = MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES; +/** Terminal event this repair publishes when it refused to reconstruct a call faithfully. */ +export const GROK_REFUSED_TERMINAL_EVENT_TYPE = "response.incomplete"; + +/** `incomplete_details.reason` carried by that terminal. */ +export const GROK_FORBIDDEN_TOOL_CALL_REASON = "forbidden_tool_call"; + +/** An upstream-supplied name reaches the terminal message; keep it bounded. */ +const MAX_REPORTED_TOOL_NAME_CHARS = 100; + +export function forbiddenToolCallMessage(name: string): string { + return `routed provider called "${name.slice(0, MAX_REPORTED_TOOL_NAME_CHARS)}", ` + + "which this request's tool selection excludes; the reconstructed output omits that call"; +} + const GROK_TERMINAL_OUTPUT_ITEM_TYPES = new Set([ "message", "reasoning", @@ -167,6 +182,45 @@ function plausibleGrokOpenItem( }; } +/** + * Publish the refusal on the terminal itself rather than as a silent omission. + * + * The ordinary reconstruction replaces only the data payload's `output`, so its event name still + * describes the payload. A refusal does not: the turn no longer completed the way the upstream + * said it did, so the event line moves with the status instead of leaving a client to read a + * clean finish off an unchanged `event: response.completed`. + */ +function refusedTerminalBlock( + block: string, + parsed: Record, + response: Record, + output: readonly Record[], + refusedName: string | undefined, +): string { + const payload = JSON.stringify({ + ...parsed, + type: GROK_REFUSED_TERMINAL_EVENT_TYPE, + response: { + ...response, + status: "incomplete", + output, + incomplete_details: { + reason: GROK_FORBIDDEN_TOOL_CALL_REASON, + ...(refusedName === undefined ? {} : { message: forbiddenToolCallMessage(refusedName) }), + }, + }, + }); + const rewritten = replaceSseDataPayload(block, payload); + const newline = block.includes("\r\n") ? "\r\n" : "\n"; + let eventRewritten = false; + const lines = rewritten.split(/\r?\n/).map(line => { + if (eventRewritten || !line.startsWith("event:")) return line; + eventRewritten = true; + return `event: ${GROK_REFUSED_TERMINAL_EVENT_TYPE}`; + }); + return lines.join(newline); +} + /** * Narrow client repair for grok-build's Responses consumer. * @@ -176,12 +230,22 @@ function plausibleGrokOpenItem( * empty output. Reconstruct only from real, unique, contiguous, bounded done * events whose raw semantics are already valid. Any ambiguity stays byte-level * fail-closed; the provider-opt-in snapshot repair above is unchanged. + * + * The terminal this publishes is one the upstream never sent, so it carries only what the final + * outbound request still authorized. A client call outside that request's tool selection is left + * out and the terminal says so explicitly. The declaration guard downstream answers the other + * half of the question — whether a name was declared at all — and keeps policing the raw stream, + * which this rewrite never edits. */ export function createGrokResponsesSparseTerminalBlockRewrite( budget?: TranslatorBudget, + outboundRequestBody?: unknown, ): SseBlockRewrite { + const toolScope: RequestToolScope | undefined = requestToolScope(outboundRequestBody); const openItems = new Map(); const completedItems = new Map(); + const withheldIndices = new Set(); + let withheldToolName: string | undefined; let aggregateItemBytes = 0; let aggregateOpenItemBytes = 0; let tainted = false; @@ -194,6 +258,8 @@ export function createGrokResponsesSparseTerminalBlockRewrite( } openItems.clear(); completedItems.clear(); + withheldIndices.clear(); + withheldToolName = undefined; aggregateItemBytes = 0; aggregateOpenItemBytes = 0; hasVisibleOutput = false; @@ -217,7 +283,7 @@ export function createGrokResponsesSparseTerminalBlockRewrite( if (tainted) return; const sourceBytes = Buffer.byteLength(JSON.stringify(item), "utf8"); if (sourceBytes > MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES - || completedItems.size >= MAX_COMPLETED_OUTPUT_ITEMS + || completedItems.size + withheldIndices.size >= MAX_COMPLETED_OUTPUT_ITEMS || aggregateItemBytes + sourceBytes > MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES) { taintAndRelease(); return; @@ -228,6 +294,24 @@ export function createGrokResponsesSparseTerminalBlockRewrite( hasVisibleOutput = hasVisibleOutput || visibleToGrok; }; + /** + * Record the position of a call this request forbade without retaining the item. + * + * Only the offending item is dropped. Tainting here instead would discard the assistant text + * that arrived in the same turn and leave the client the empty terminal this repair exists to + * fix, which punishes the caller for the provider's overreach. The index is kept so the + * contiguity proof below still covers the whole output. + */ + const withholdForbiddenCall = (index: number, name: string): void => { + if (tainted) return; + if (completedItems.size + withheldIndices.size >= MAX_COMPLETED_OUTPUT_ITEMS) { + taintAndRelease(); + return; + } + withheldIndices.add(index); + withheldToolName ??= name.slice(0, MAX_REPORTED_TOOL_NAME_CHARS); + }; + const closeOpenItem = (index: number): void => { const open = openItems.get(index); if (!open) return; @@ -265,6 +349,7 @@ export function createGrokResponsesSparseTerminalBlockRewrite( const open = isPlainObject(parsed.item) ? plausibleGrokOpenItem(parsed.item) : null; if (outputIndex === undefined || !open || openItems.has(outputIndex) || completedItems.has(outputIndex) + || withheldIndices.has(outputIndex) || openItems.size >= MAX_COMPLETED_OUTPUT_ITEMS) { taintAndRelease(); } else if (!tainted) { @@ -284,7 +369,8 @@ export function createGrokResponsesSparseTerminalBlockRewrite( if (type === "response.output_item.done") { const item = isPlainObject(parsed.item) ? parsed.item : null; const proof = item ? trustedGrokCompletedItem(item) : null; - if (outputIndex === undefined || !proof || completedItems.has(outputIndex)) { + if (outputIndex === undefined || !proof + || completedItems.has(outputIndex) || withheldIndices.has(outputIndex)) { taintAndRelease(); return [block]; } @@ -295,7 +381,12 @@ export function createGrokResponsesSparseTerminalBlockRewrite( return [block]; } closeOpenItem(outputIndex); - retainCompletedItem(outputIndex, item!, proof.visibleToGrok); + const forbidden = toolScope?.forbiddenClientToolCallName(item!); + if (forbidden === undefined) { + retainCompletedItem(outputIndex, item!, proof.visibleToGrok); + } else { + withholdForbiddenCall(outputIndex, forbidden); + } return [block]; } @@ -312,14 +403,18 @@ export function createGrokResponsesSparseTerminalBlockRewrite( const outputIsAuthoritative = Array.isArray(output) && output.length > 0; const outputIsSparse = !("output" in response) || (Array.isArray(output) && output.length === 0); + // A withheld call is a reason to publish on its own: the refusal has to reach the client + // even when nothing visible survived it, or the turn ends as an ordinary empty finish. + const refused = withheldIndices.size > 0; if (!outputIsAuthoritative && outputIsSparse && terminalStatusConsistent - && completedItems.size > 0 && openItems.size === 0 && hasVisibleOutput) { + && openItems.size === 0 && (hasVisibleOutput || refused)) { const ordered = [...completedItems.entries()].sort(([left], [right]) => left - right); - if (ordered.every(([index], position) => index === position)) { - out = jsonBlock({ - ...parsed, - response: { ...response, output: ordered.map(([, retained]) => retained.item) }, - }); + const positions = [...completedItems.keys(), ...withheldIndices].sort((left, right) => left - right); + if (positions.length > 0 && positions.every((index, position) => index === position)) { + const rebuilt = ordered.map(([, retained]) => retained.item); + out = refused + ? refusedTerminalBlock(block, parsed, response, rebuilt, withheldToolName) + : jsonBlock({ ...parsed, response: { ...response, output: rebuilt } }); } } } diff --git a/src/server/responses-request-tool-scope.ts b/src/server/responses-request-tool-scope.ts new file mode 100644 index 00000000000..8531ff9b2e7 --- /dev/null +++ b/src/server/responses-request-tool-scope.ts @@ -0,0 +1,121 @@ +/** + * The tool selection a Responses request actually authorized, read from the final outbound body. + * + * The undeclared-tool guard answers whether a NAME was declared. This answers a different + * question: whether this request still permits a client tool call at all, and which names it + * permits. `tool_choice: "none"`, a forced selector and an `allowed_tools` allow-list each narrow + * the catalog without removing a declaration, so a name can be declared and forbidden at the same + * time — and a repair that rebuilds a terminal from collected items would otherwise hand the + * client a call the caller ruled out. + * + * The scope is read from the OUTBOUND body, after every removal, rename and translation, because + * that is the request the destination answered. A catalog that ends up empty there authorizes no + * client call whatever the selector still says. + */ +import { dottedToolName, namespacedToolName } from "../types"; +import { + CLIENT_EXECUTED_CALL_TYPES, + collectDeclaredWireToolNames, + hasExplicitWireToolCatalog, +} from "./responses-undeclared-tool-guard"; +import { isPlainObject } from "./responses-snapshot-codec"; + +/** Every spelling one call item can be named by, so a selector match is not defeated by flattening. */ +function callNameSpellings(item: Record): readonly string[] { + const name = typeof item.name === "string" ? item.name : ""; + if (name.length === 0) return []; + const namespace = typeof item.namespace === "string" && item.namespace.length > 0 + ? item.namespace + : undefined; + if (!namespace) return [name]; + return [name, namespacedToolName(namespace, name), dottedToolName(namespace, name)]; +} + +/** The names one `tool_choice` entry selects; empty when the entry names no client tool. */ +function selectorNameSpellings(selector: unknown): readonly string[] { + if (!isPlainObject(selector)) return []; + const name = typeof selector.name === "string" ? selector.name : ""; + if (name.length === 0) return []; + const namespace = typeof selector.namespace === "string" && selector.namespace.length > 0 + ? selector.namespace + : undefined; + if (!namespace) return [name]; + return [name, namespacedToolName(namespace, name), dottedToolName(namespace, name)]; +} + +type ToolSelection = + | { readonly kind: "unrestricted" } + | { readonly kind: "deny_all" } + | { readonly kind: "allow"; readonly names: ReadonlySet }; + +const UNRESTRICTED: ToolSelection = { kind: "unrestricted" }; + +/** + * Read the selector only where it states a client-call boundary. + * + * `auto`, `required` and an absent selector restrict nothing. A hosted selector + * (`{ type: "web_search" }`) forces a tool the PROVIDER runs and does not describe the client + * calls this turn may contain, so it is left alone rather than read as a deny-all: a false + * refusal would drop a call the caller could have executed. + */ +function toolSelection(body: Record): ToolSelection { + const choice = body.tool_choice; + if (choice === "none") return { kind: "deny_all" }; + if (!isPlainObject(choice)) return UNRESTRICTED; + if (choice.type === "allowed_tools") { + if (!Array.isArray(choice.tools)) return UNRESTRICTED; + const names = new Set(); + for (const entry of choice.tools) { + for (const spelling of selectorNameSpellings(entry)) names.add(spelling); + } + // An allow-list carrying no client tool — emptied by normalization, or hosted entries only — + // still bounds this turn: it allows no client call. + return { kind: "allow", names }; + } + if (choice.type === "function" || choice.type === "custom") { + const names = new Set(selectorNameSpellings(choice)); + return names.size > 0 ? { kind: "allow", names } : UNRESTRICTED; + } + return UNRESTRICTED; +} + +export type RequestToolScope = { + /** + * The name a client call is refused under, or undefined when this request permits it. + * A nameless call type is not answered here: only the declaration guard knows those. + */ + forbiddenClientToolCallName(item: Record): string | undefined; +}; + +/** + * The client-call boundary this request states, or undefined when it states none. + * + * Returning undefined for an unrestricted request keeps every ordinary turn on the path it + * already had: a caller that selected nothing gets no new refusal. + */ +export function requestToolScope(body: unknown): RequestToolScope | undefined { + if (!isPlainObject(body)) return undefined; + const selection = toolSelection(body); + // A readable catalog that declares no client-executable name is authoritative, exactly as it is + // for the declaration guard: an explicit empty list denies every client call. An absent catalog + // says nothing — a passthrough request may omit `tools` and still receive a call the client + // understands. + const catalogDeniesClientCalls = hasExplicitWireToolCatalog(body) + && collectDeclaredWireToolNames(body).size === 0; + if (selection.kind === "unrestricted" && !catalogDeniesClientCalls) return undefined; + return { + forbiddenClientToolCallName(item: Record): string | undefined { + if (typeof item.type !== "string" || !CLIENT_EXECUTED_CALL_TYPES.has(item.type)) { + return undefined; + } + const spellings = callNameSpellings(item); + const reported = spellings[0]; + if (reported === undefined) return undefined; + if (catalogDeniesClientCalls || selection.kind === "deny_all") return reported; + if (selection.kind === "allow") { + return spellings.some(spelling => selection.names.has(spelling)) ? undefined : reported; + } + return undefined; + }, + }; +} diff --git a/src/server/responses-undeclared-tool-guard.ts b/src/server/responses-undeclared-tool-guard.ts index 7b2a2a22c1c..943f19e7d6d 100644 --- a/src/server/responses-undeclared-tool-guard.ts +++ b/src/server/responses-undeclared-tool-guard.ts @@ -13,7 +13,10 @@ import { import { replaceSseDataPayload, sseDataPayload, type SseBlockRewrite } from "./sse-payload-rewrite"; /** Item types the client executes through a request-declared wire name. */ -const CLIENT_EXECUTED_CALL_TYPES = new Set(["function_call", "custom_tool_call"]); +export const CLIENT_EXECUTED_CALL_TYPES: ReadonlySet = new Set([ + "function_call", + "custom_tool_call", +]); /** Codex groups ordinary top-level tools here; unlike an MCP namespace, it has no wire prefix. */ const BUILTIN_FUNCTIONS_NAMESPACE = "functions"; diff --git a/src/server/responses/passthrough-delivery.ts b/src/server/responses/passthrough-delivery.ts index f6cd0009987..a8ee2f3f4b0 100644 --- a/src/server/responses/passthrough-delivery.ts +++ b/src/server/responses/passthrough-delivery.ts @@ -515,7 +515,7 @@ export async function deliverPassthroughResponse( ? createGrokResponsesTimestampBlockRewrite() : undefined, grokClientCompatibilityEnabled - ? createGrokResponsesSparseTerminalBlockRewrite(translatorBudget) + ? createGrokResponsesSparseTerminalBlockRewrite(translatorBudget, nativeExchange.outboundRequestBody) : undefined, snapshotRepairEnabled ? createResponsesSnapshotBlockRewrite(nativeExchange.outboundRequestBody, translatorBudget) diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index d5413609fb8..2b3ac510ea2 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1124,6 +1124,7 @@ "responses-show-thinking-summary.test.ts": "responses", "responses-snapshot-repair-server.test.ts": "responses", "responses-snapshot-repair.test.ts": "responses", + "responses-sparse-terminal-tool-scope.test.ts": "responses", "responses-spill-shutdown-clock.test.ts": "responses", "responses-state-write-amplification.test.ts": "responses", "responses-state.test.ts": "responses", diff --git a/tests/responses/responses-sparse-terminal-tool-scope.test.ts b/tests/responses/responses-sparse-terminal-tool-scope.test.ts new file mode 100644 index 00000000000..4b8cd37becd --- /dev/null +++ b/tests/responses/responses-sparse-terminal-tool-scope.test.ts @@ -0,0 +1,235 @@ +import { describe, expect, test } from "bun:test"; +import { + createGrokResponsesSparseTerminalBlockRewrite, + forbiddenToolCallMessage, + GROK_FORBIDDEN_TOOL_CALL_REASON, + GROK_REFUSED_TERMINAL_EVENT_TYPE, +} from "../../src/server/grok-responses-snapshot-repair"; +import { createTestTranslatorBudget } from "../helpers/translator-budget"; + +function dataBlock(payload: unknown): string { + return `data: ${JSON.stringify(payload)}`; +} + +function terminalBlock(payload: unknown): string { + return `event: response.completed\ndata: ${JSON.stringify(payload)}`; +} + +function payloadOf(block: string): Record { + const data = block.split(/\r?\n/) + .filter(line => line.startsWith("data: ")) + .map(line => line.slice("data: ".length)) + .join(""); + return JSON.parse(data) as Record; +} + +function eventNameOf(block: string): string | undefined { + const line = block.split(/\r?\n/).find(candidate => candidate.startsWith("event: ")); + return line === undefined ? undefined : line.slice("event: ".length); +} + +function responseOf(block: string): Record { + return payloadOf(block).response as Record; +} + +const MESSAGE_ITEM: Record = { + type: "message", + id: "msg_1", + role: "assistant", + status: "completed", + content: [{ type: "output_text", text: "here is the answer", annotations: [] }], +}; + +const CALL_ITEM: Record = { + type: "function_call", + id: "fc_1", + call_id: "call_1", + name: "apply_patch", + arguments: "{}", +}; + +const SPARSE_TERMINAL = { + type: "response.completed", + response: { id: "resp_1", status: "completed", output: [] }, +}; + +function relay( + outboundBody: unknown, + items: readonly { index: number; item: Record }[], +): { terminal: string; forwarded: string[] } { + const rewrite = createGrokResponsesSparseTerminalBlockRewrite( + createTestTranslatorBudget(), + outboundBody, + ); + const forwarded: string[] = []; + for (const { index, item } of items) { + forwarded.push( + ...rewrite(dataBlock({ type: "response.output_item.done", output_index: index, item })), + ); + } + const out = rewrite(terminalBlock(SPARSE_TERMINAL)); + expect(out).toHaveLength(1); + return { terminal: out[0]!, forwarded }; +} + +function ordered(...items: Record[]): { index: number; item: Record }[] { + return items.map((item, index) => ({ index, item })); +} + +describe("Grok sparse terminal reconstruction honours the request's tool selection", () => { + test("a request that selected no tools keeps the text and refuses the call explicitly", () => { + const { terminal } = relay( + { model: "grok-4.6", tools: [{ type: "function", name: "apply_patch" }], tool_choice: "none" }, + ordered(CALL_ITEM, MESSAGE_ITEM), + ); + + expect(payloadOf(terminal).type).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + expect(eventNameOf(terminal)).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + const response = responseOf(terminal); + expect(response.status).toBe("incomplete"); + // The forbidden call is the only casualty; the assistant text that arrived with it survives. + expect(response.output).toEqual([MESSAGE_ITEM]); + expect(response.incomplete_details).toEqual({ + reason: GROK_FORBIDDEN_TOOL_CALL_REASON, + message: forbiddenToolCallMessage(CALL_ITEM.name as string), + }); + }); + + test("a refusal with nothing else to publish is still explicit, not an empty clean finish", () => { + const { terminal } = relay( + { model: "grok-4.6", tools: [{ type: "function", name: "apply_patch" }], tool_choice: "none" }, + ordered(CALL_ITEM), + ); + + expect(payloadOf(terminal).type).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + expect(responseOf(terminal).output).toEqual([]); + }); + + test("the raw stream is forwarded untouched; only the reconstructed terminal is bounded", () => { + const { forwarded } = relay( + { model: "grok-4.6", tools: [{ type: "function", name: "apply_patch" }], tool_choice: "none" }, + ordered(CALL_ITEM), + ); + + expect(forwarded).toEqual([ + dataBlock({ type: "response.output_item.done", output_index: 0, item: CALL_ITEM }), + ]); + }); + + test("a selection that permits the call reconstructs it unchanged", () => { + const { terminal } = relay( + { + model: "grok-4.6", + tools: [{ type: "function", name: "apply_patch" }], + tool_choice: { type: "function", name: "apply_patch" }, + }, + ordered(CALL_ITEM, MESSAGE_ITEM), + ); + + expect(payloadOf(terminal).type).toBe("response.completed"); + expect(responseOf(terminal).output).toEqual([CALL_ITEM, MESSAGE_ITEM]); + }); + + test("a forced selector for a different tool refuses the call it did not select", () => { + const { terminal } = relay( + { + model: "grok-4.6", + tools: [{ type: "function", name: "apply_patch" }, { type: "function", name: "read_file" }], + tool_choice: { type: "function", name: "read_file" }, + }, + ordered(CALL_ITEM, MESSAGE_ITEM), + ); + + expect(payloadOf(terminal).type).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + expect(responseOf(terminal).output).toEqual([MESSAGE_ITEM]); + }); + + test("an allow-list admits a listed tool and refuses an unlisted one", () => { + const body = { + model: "grok-4.6", + tools: [{ type: "function", name: "apply_patch" }, { type: "function", name: "read_file" }], + tool_choice: { + type: "allowed_tools", + mode: "auto", + tools: [{ type: "function", name: "read_file" }], + }, + }; + const listed: Record = { ...CALL_ITEM, name: "read_file" }; + + expect(responseOf(relay(body, ordered(listed)).terminal).output).toEqual([listed]); + expect(payloadOf(relay(body, ordered(CALL_ITEM, MESSAGE_ITEM)).terminal).type) + .toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + }); + + test("a namespaced call matches the selector under either flattened spelling", () => { + const namespaced: Record = { ...CALL_ITEM, name: "search", namespace: "docs" }; + for (const selected of ["docs__search", "docs.search"]) { + const { terminal } = relay( + { + model: "grok-4.6", + tools: [{ type: "namespace", name: "docs", tools: [{ type: "function", name: "search" }] }], + tool_choice: { type: "function", name: selected }, + }, + ordered(namespaced), + ); + expect(responseOf(terminal).output).toEqual([namespaced]); + } + }); + + test("a catalog emptied by normalization admits no client call, with or without a selector", () => { + for (const body of [ + { model: "grok-4.6", tools: [] }, + { model: "grok-4.6", tools: [], tool_choice: "auto" }, + { model: "grok-4.6", tools: [{ type: "web_search" }], tool_choice: "auto" }, + ]) { + const { terminal } = relay(body, ordered(CALL_ITEM, MESSAGE_ITEM)); + expect(payloadOf(terminal).type).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + expect(responseOf(terminal).output).toEqual([MESSAGE_ITEM]); + } + }); + + test("a request that declares no catalog at all keeps its existing reconstruction", () => { + // A passthrough request may legitimately omit `tools` and still receive a call the client + // understands, so an absent catalog states no boundary for this repair to enforce. + const { terminal } = relay({ model: "grok-4.6", input: [] }, ordered(CALL_ITEM, MESSAGE_ITEM)); + + expect(payloadOf(terminal).type).toBe("response.completed"); + expect(responseOf(terminal).output).toEqual([CALL_ITEM, MESSAGE_ITEM]); + }); + + test("a selection with nothing to refuse reconstructs the ordinary terminal", () => { + const { terminal } = relay( + { model: "grok-4.6", tools: [{ type: "function", name: "apply_patch" }], tool_choice: "none" }, + ordered(MESSAGE_ITEM), + ); + + expect(payloadOf(terminal).type).toBe("response.completed"); + expect(responseOf(terminal).output).toEqual([MESSAGE_ITEM]); + }); + + test("a withheld index never fills a gap the stream actually left", () => { + const { terminal } = relay( + { model: "grok-4.6", tools: [{ type: "function", name: "apply_patch" }], tool_choice: "none" }, + [{ index: 0, item: CALL_ITEM }, { index: 2, item: MESSAGE_ITEM }], + ); + + expect(payloadOf(terminal).type).toBe("response.completed"); + expect(responseOf(terminal).output).toEqual([]); + }); + + test("refusing a call releases every retained byte", () => { + const budget = createTestTranslatorBudget(); + const rewrite = createGrokResponsesSparseTerminalBlockRewrite(budget, { + model: "grok-4.6", + tools: [{ type: "function", name: "apply_patch" }], + tool_choice: "none", + }); + rewrite(dataBlock({ type: "response.output_item.done", output_index: 0, item: CALL_ITEM })); + rewrite(dataBlock({ type: "response.output_item.done", output_index: 1, item: MESSAGE_ITEM })); + rewrite(terminalBlock(SPARSE_TERMINAL)); + + expect(budget.snapshot().currentBytes).toBe(0); + rewrite.dispose?.(); + expect(budget.snapshot().currentBytes).toBe(0); + }); +}); From a3bbc0eaaa9cc80840385ce7ce2b3fd81f4af06c Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:25:43 +0900 Subject: [PATCH 2/4] fix(xai): drop a selector that normalization left with nothing to select xAI rejects a Responses request whose tool_choice survives a catalog the adapter had to empty, which is what happens to a cached-only web-search declaration: it is omitted rather than widened to live search, and the request then selects from a catalog it no longer has. Omit an auto or none selector once no tool remains in either the top-level catalog or additional_tools. A forced function selector is preserved: a selector this proxy cannot honor is a client input error, and the request-build path already answers it with a 400 rather than silently turning "call this tool" into "answer however you like". This is the outbound half of the same rule the response-side repair applies -- compatibility is judged on the final request and the final response, after every removal, rename and translation. The change is carried unmodified from #5350. Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> --- scripts/test-layout/layout.json | 1 + src/adapters/xai-web-search.ts | 9 ++- tests/fixtures/test-layout-expected.json | 1 + .../xai/xai-empty-catalog-tool-choice.test.ts | 55 +++++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 tests/providers/xai/xai-empty-catalog-tool-choice.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 33383b78959..364ffcbf73d 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1556,6 +1556,7 @@ "ws-upstream.test.ts": "responses", "ws-upstream-socks5.test.ts": "responses", "xai-client.test.ts": "images", + "xai-empty-catalog-tool-choice.test.ts": "providers/xai", "xai-oauth-retry.test.ts": "providers/xai", "xai-refresh-lock.test.ts": "providers/xai", "xai-responses-adjacency.test.ts": "providers/xai", diff --git a/src/adapters/xai-web-search.ts b/src/adapters/xai-web-search.ts index e558e39f2ed..f5f4bbf7843 100644 --- a/src/adapters/xai-web-search.ts +++ b/src/adapters/xai-web-search.ts @@ -1,4 +1,5 @@ import type { OcxProviderConfig } from "../types"; +import { debugProviderDiagnostic } from "../lib/debug"; import { isXaiResponsesDestination } from "../providers/xai-transport"; const CODEX_WEB_SEARCH_TOOL = "web_search"; @@ -192,7 +193,13 @@ export function normalizeXaiResponsesWebSearch( if (inputChanged) next = { ...next, input }; } - return normalizeToolChoice(next); + const normalized = normalizeToolChoice(next); + if ((normalized.tool_choice === "auto" || normalized.tool_choice === "none") && !hasAnyDeclaredTool(normalized)) { + debugProviderDiagnostic("xai", "tool-choice-omitted", { choice: normalized.tool_choice }); + const { tool_choice: _toolChoice, ...rest } = normalized; + return rest; + } + return normalized; } function isLiveWebSearchTool(tool: unknown): boolean { diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 2b3ac510ea2..29da3774f72 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1385,6 +1385,7 @@ "ws-upstream.test.ts": "responses", "ws-upstream-socks5.test.ts": "responses", "xai-client.test.ts": "images", + "xai-empty-catalog-tool-choice.test.ts": "providers/xai", "xai-oauth-retry.test.ts": "providers/xai", "xai-refresh-lock.test.ts": "providers/xai", "xai-responses-adjacency.test.ts": "providers/xai", diff --git a/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts b/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts new file mode 100644 index 00000000000..60f58785885 --- /dev/null +++ b/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test } from "bun:test"; +import { normalizeXaiResponsesWebSearch } from "../../../src/adapters/xai-web-search"; + +const XAI_PROVIDER = { baseUrl: "https://api.x.ai/v1" }; + +function normalize(body: Record): Record { + return normalizeXaiResponsesWebSearch(body, XAI_PROVIDER) as Record; +} + +describe("xAI Responses selectors after tool normalization", () => { + test("a catalog emptied by normalization drops the selector that has nothing left to select", () => { + // The cached-only declaration is omitted above rather than widened to live search, which + // leaves the request selecting from a catalog it no longer has; xAI answers 400. + const body = normalize({ + model: "grok-4.6", + input: "latest xAI news", + tools: [{ type: "web_search", external_web_access: false }], + tool_choice: "auto", + }); + + expect(Object.hasOwn(body, "tools")).toBe(false); + expect(Object.hasOwn(body, "tool_choice")).toBe(false); + }); + + test("an explicitly empty catalog carries no selector either", () => { + for (const choice of ["auto", "none"]) { + const body = normalize({ model: "grok-4.6", input: "hi", tools: [], tool_choice: choice }); + expect(Object.hasOwn(body, "tool_choice")).toBe(false); + } + }); + + test("a selector that still has a tool to select is left alone", () => { + const body = normalize({ + model: "grok-4.6", + input: "hi", + tools: [{ type: "function", name: "read_file" }], + tool_choice: "auto", + }); + + expect(body.tool_choice).toBe("auto"); + }); + + test("a forced function selector survives an empty catalog as a client input error", () => { + // Dropping it would silently turn "call this tool" into "answer however you like"; the + // request-build path answers a selector this proxy cannot honor with a 400 instead. + const body = normalize({ + model: "grok-4.6", + input: "hi", + tools: [], + tool_choice: { type: "function", name: "read_file" }, + }); + + expect(body.tool_choice).toEqual({ type: "function", name: "read_file" }); + }); +}); From 377269aa23651c30774cf794b8807377a92949be Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:25:49 +0900 Subject: [PATCH 3/4] docs(structure): record how a tool selection survives a response repair The Responses transport document described declaration enforcement and said nothing about selection, so the next person to touch this area would have read the undeclared-tool guard as the whole contract. Write down the boundary between the two questions, where the scope is read from, and why the refusal keeps the text and marks the terminal instead of finishing quietly. --- structure/transports/responses.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 3b40dd43eda..c1f02393757 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -667,6 +667,36 @@ and continuation-state suppression as well as the refusal, and it stands down on `src/server/responses/run-turn-execution.ts` and `src/server/responses/adapter-delivery.ts` set the flag from `inboundWire` on the streaming, buffered, and JSON paths alike, so the three cannot drift. +### Selection outlives the declaration check + +Declaration and selection are different questions, and the guard above answers only the first. +`tool_choice: "none"`, a forced selector and an `allowed_tools` allow-list each narrow a catalog +without removing a declaration, so a name can be declared and forbidden at the same time — and a +guard that compares names against the catalog passes it. + +The gap is reachable because a repair can put such a call back. +`createGrokResponsesSparseTerminalBlockRewrite` in `src/server/grok-responses-snapshot-repair.ts` +rebuilds a terminal `output` the upstream never sent from the items it collected during the turn. +`src/server/responses-request-tool-scope.ts` reads the boundary the request states, and the repair +applies it to what it publishes: a client call outside the selection is left out of the +reconstruction. The scope comes from the final outbound body, after every removal, rename and +translation, so a catalog that ends up empty there authorizes no client call whatever the selector +still says. An absent catalog states no boundary, exactly as it states none for the declaration +guard. + +The refusal is narrow and it is visible. Only the offending item is dropped, so the assistant text +that arrived in the same turn still reaches the client rather than being discarded with it. Because +the turn no longer ended the way the upstream said it did, the reconstructed terminal is published +as `response.incomplete` carrying `incomplete_details.reason: forbidden_tool_call`, not as a clean +`response.completed` with a quietly shorter output. The repair edits nothing but the terminal it +synthesizes; the raw stream remains the declaration guard's to police. + +The selection is kept honest on the way out as well. `src/adapters/xai-web-search.ts` omits an +`auto`/`none` selector once normalization has left nothing for it to select, because xAI answers +that request with a 400. A forced function selector is preserved: a selector this proxy cannot +honor is a client input error, and `src/server/responses/passthrough-dispatch.ts` already answers +it with one. + ### Passthrough SSE stream shapes (#314) Native passthrough SSE has TWO shapes, selected per request in From e1d799b2fb6a25694fbe8fcae87e9a8d28222763 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 21 Sep 2026 07:47:27 +0900 Subject: [PATCH 4/4] fix(xai): restate the prohibition the empty-catalog selector omission removes Omitting an auto/none selector that normalization left with nothing to select keeps xAI from answering 400, but the two words are not interchangeable. auto selects from the catalog, so removing it from a request with an empty one states nothing new. none is a prohibition, and on a request whose catalog this normalizer just emptied it is the only place the turn's client-call boundary is written down. The sparse-terminal repair reads that boundary from the final outbound body, so dropping the word alone handed the reconstruction a request that authorized more than the caller did -- and nothing behind it catches that: the repair runs on the grok client surface while the declaration guard stands down whenever the provider's authMode is forward, which is what the xAI OAuth lane is. A caller who forbade every client tool could get one back inside a terminal the upstream never sent. Restate the prohibition as the explicit empty catalog. It carries the same deny-all, the request scope and the declaration guard both already read it that way, and this destination receives it unchanged whenever a caller sends one itself. auto is still dropped without inventing a catalog, because an absent catalog states no boundary. Co-authored-by: Yeonwoo Choi <32544727+twoimo@users.noreply.github.com> --- src/adapters/xai-web-search.ts | 14 ++- structure/transports/responses.md | 11 ++ .../xai/xai-empty-catalog-tool-choice.test.ts | 100 ++++++++++++++++++ .../xai/xai-web-search-compat.test.ts | 8 +- 4 files changed, 128 insertions(+), 5 deletions(-) diff --git a/src/adapters/xai-web-search.ts b/src/adapters/xai-web-search.ts index f5f4bbf7843..4dbb77d7364 100644 --- a/src/adapters/xai-web-search.ts +++ b/src/adapters/xai-web-search.ts @@ -194,10 +194,18 @@ export function normalizeXaiResponsesWebSearch( } const normalized = normalizeToolChoice(next); - if ((normalized.tool_choice === "auto" || normalized.tool_choice === "none") && !hasAnyDeclaredTool(normalized)) { - debugProviderDiagnostic("xai", "tool-choice-omitted", { choice: normalized.tool_choice }); + const choice = normalized.tool_choice; + if ((choice === "auto" || choice === "none") && !hasAnyDeclaredTool(normalized)) { + debugProviderDiagnostic("xai", "tool-choice-omitted", { choice }); const { tool_choice: _toolChoice, ...rest } = normalized; - return rest; + // `auto` selects from the catalog, so a catalog with nothing in it makes it meaningless and + // the omission says nothing the request did not already say. `none` is the opposite: it is a + // prohibition, and on a request whose catalog this normalizer just emptied it is the only + // place the turn's client-call boundary is written down. Downstream repair reads that + // boundary off the final outbound body, so omitting the word alone would hand back a call the + // caller ruled out. Restate it as the explicit empty catalog, which carries the same deny-all + // and which this destination already receives whenever a caller sends one itself. + return choice === "none" && !Array.isArray(rest.tools) ? { ...rest, tools: [] } : rest; } return normalized; } diff --git a/structure/transports/responses.md b/structure/transports/responses.md index c1f02393757..11b9fed453f 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -697,6 +697,17 @@ that request with a 400. A forced function selector is preserved: a selector thi honor is a client input error, and `src/server/responses/passthrough-dispatch.ts` already answers it with one. +Those two omissions are not the same edit, because the scope above is read from the body this +normalization produces. `auto` selects from the catalog, so removing it from a request with an +empty one states nothing new. `none` is a prohibition, and on a request whose catalog this +normalizer emptied it is the only place the turn's client-call boundary is written down. Dropping +the word alone would let the reconstruction hand back a call the caller ruled out, and nothing +behind it would catch that: the repair runs on the grok client surface, while the declaration +guard stands down whenever the provider's `authMode` is `forward` — which is what the xAI OAuth +lane is. So the prohibition is restated as the explicit empty catalog, which carries the same +deny-all, which the scope and the declaration guard both already read that way, and which this +destination receives unchanged whenever a caller sends one itself. + ### Passthrough SSE stream shapes (#314) Native passthrough SSE has TWO shapes, selected per request in diff --git a/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts b/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts index 60f58785885..28b2549397c 100644 --- a/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts +++ b/tests/providers/xai/xai-empty-catalog-tool-choice.test.ts @@ -1,5 +1,12 @@ import { describe, expect, test } from "bun:test"; import { normalizeXaiResponsesWebSearch } from "../../../src/adapters/xai-web-search"; +import { + createGrokResponsesSparseTerminalBlockRewrite, + GROK_FORBIDDEN_TOOL_CALL_REASON, + GROK_REFUSED_TERMINAL_EVENT_TYPE, + forbiddenToolCallMessage, +} from "../../../src/server/grok-responses-snapshot-repair"; +import { createTestTranslatorBudget } from "../../helpers/translator-budget"; const XAI_PROVIDER = { baseUrl: "https://api.x.ai/v1" }; @@ -7,6 +14,48 @@ function normalize(body: Record): Record { return normalizeXaiResponsesWebSearch(body, XAI_PROVIDER) as Record; } +const CALL_ITEM: Record = { + type: "function_call", + id: "fc_1", + call_id: "call_1", + name: "apply_patch", + arguments: "{}", +}; + +const MESSAGE_ITEM: Record = { + type: "message", + id: "msg_1", + role: "assistant", + status: "completed", + content: [{ type: "output_text", text: "there is nothing to call here", annotations: [] }], +}; + +function dataBlock(payload: unknown): string { + return `data: ${JSON.stringify(payload)}`; +} + +/** Replay a sparse Grok stream against one outbound body and return the terminal it publishes. */ +function reconstructTerminal(outboundBody: unknown): Record { + const rewrite = createGrokResponsesSparseTerminalBlockRewrite( + createTestTranslatorBudget(), + outboundBody, + ); + rewrite(dataBlock({ type: "response.output_item.done", output_index: 0, item: CALL_ITEM })); + rewrite(dataBlock({ type: "response.output_item.done", output_index: 1, item: MESSAGE_ITEM })); + const out = rewrite( + `event: response.completed\n${dataBlock({ + type: "response.completed", + response: { id: "resp_1", status: "completed", output: [] }, + })}`, + ); + expect(out).toHaveLength(1); + const data = out[0]!.split(/\r?\n/) + .filter(line => line.startsWith("data: ")) + .map(line => line.slice("data: ".length)) + .join(""); + return JSON.parse(data) as Record; +} + describe("xAI Responses selectors after tool normalization", () => { test("a catalog emptied by normalization drops the selector that has nothing left to select", () => { // The cached-only declaration is omitted above rather than widened to live search, which @@ -22,6 +71,57 @@ describe("xAI Responses selectors after tool normalization", () => { expect(Object.hasOwn(body, "tool_choice")).toBe(false); }); + test("a prohibition the omission would erase is restated as the explicit empty catalog", () => { + // Normalization turns a selector for the removed search into "none", and the request then + // says nothing else about what this turn may contain. Omitting that word for the wire without + // restating it would leave a body that authorizes more than the caller did. + const body = normalize({ + model: "grok-4.6", + input: "latest xAI news", + tools: [{ type: "web_search", external_web_access: false }], + tool_choice: { type: "web_search" }, + }); + + expect(Object.hasOwn(body, "tool_choice")).toBe(false); + expect(body.tools).toEqual([]); + }); + + test("a caller's own prohibition survives an omission it never asked for", () => { + const body = normalize({ model: "grok-4.6", input: "hi", tool_choice: "none" }); + + expect(Object.hasOwn(body, "tool_choice")).toBe(false); + expect(body.tools).toEqual([]); + }); + + test("omitting auto invents no catalog the caller never declared", () => { + // An absent catalog states no boundary, and a passthrough request may legitimately omit + // tools and still receive a call its client understands. + const body = normalize({ model: "grok-4.6", input: "hi", tool_choice: "auto" }); + + expect(Object.hasOwn(body, "tool_choice")).toBe(false); + expect(Object.hasOwn(body, "tools")).toBe(false); + }); + + test("the restated catalog still refuses a forbidden call in a reconstructed terminal", () => { + // The wire-compatibility omission and the reconstruction boundary meet here: the repair reads + // the final outbound body, so a request that forbade every client call must still refuse one + // that arrives in a sparse stream, and must keep the assistant text that arrived beside it. + const terminal = reconstructTerminal(normalize({ + model: "grok-4.6", + input: "latest xAI news", + tools: [{ type: "web_search", external_web_access: false }], + tool_choice: { type: "web_search" }, + })); + const response = terminal.response as Record; + + expect(terminal.type).toBe(GROK_REFUSED_TERMINAL_EVENT_TYPE); + expect(response.output).toEqual([MESSAGE_ITEM]); + expect(response.incomplete_details).toEqual({ + reason: GROK_FORBIDDEN_TOOL_CALL_REASON, + message: forbiddenToolCallMessage(CALL_ITEM.name as string), + }); + }); + test("an explicitly empty catalog carries no selector either", () => { for (const choice of ["auto", "none"]) { const body = normalize({ model: "grok-4.6", input: "hi", tools: [], tool_choice: choice }); diff --git a/tests/providers/xai/xai-web-search-compat.test.ts b/tests/providers/xai/xai-web-search-compat.test.ts index 67faa1c3f9e..6dd69540f91 100644 --- a/tests/providers/xai/xai-web-search-compat.test.ts +++ b/tests/providers/xai/xai-web-search-compat.test.ts @@ -75,11 +75,15 @@ describe("xAI Responses web-search compatibility", () => { }, }); - expect(body.tools).toBeUndefined(); expect(body.input).toEqual([ { type: "message", role: "user", content: [{ type: "input_text", text: "hello" }] }, ]); - expect(body.tool_choice).toBe("none"); + // The selector is omitted because xAI rejects one that selects from a catalog this + // normalization emptied, and the deny-all it stated is restated as the explicit empty + // catalog. Both spellings forbid every client call this turn; only the second one survives + // the wire, and the reconstruction path reads the request's boundary from exactly this body. + expect(body).not.toHaveProperty("tool_choice"); + expect(body.tools).toEqual([]); }); test("keeps public xAI search declarations live when the private access flag is absent", () => {