From d6c6a31d6f412908189ec2497d9923c0e47f37be Mon Sep 17 00:00:00 2001 From: olddonkey Date: Thu, 20 Aug 2026 11:14:11 -0700 Subject: [PATCH 1/4] fix(xai): restore Grok Responses tool compatibility --- src/adapters/base.ts | 2 + src/adapters/openai-responses.ts | 60 ++++ src/responses/namespace-tool-compat.ts | 277 +++++++++++++++++++ src/server/responses/core.ts | 16 +- structure/04_transports-and-sidecars.md | 21 +- tests/namespace-tool-compat.test.ts | 150 ++++++++++ tests/openai-responses-passthrough.test.ts | 86 ++++-- tests/server-xai-responses-streaming.test.ts | 122 ++++++++ 8 files changed, 714 insertions(+), 20 deletions(-) create mode 100644 src/responses/namespace-tool-compat.ts create mode 100644 tests/namespace-tool-compat.test.ts diff --git a/src/adapters/base.ts b/src/adapters/base.ts index faeea0e959..45660e478c 100644 --- a/src/adapters/base.ts +++ b/src/adapters/base.ts @@ -72,6 +72,8 @@ export interface AdapterRequest { convertedRoutedCustomToolNames?: ReadonlySet; /** Client tool-search names actually lowered to upstream function calls for this request. */ convertedRoutedToolSearchNames?: ReadonlySet; + /** Upstream-only aliases for namespace tools flattened in this request. */ + convertedRoutedNamespaceToolAliases?: ReadonlyMap; /** Releases observation of a serialized request body after its final fetch attempt settles. */ releaseBodyObservation?: () => void; /** Exact reasoning parameter emitted by the adapter, for request-log diagnostics only. */ diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index b85252b142..60f14bb224 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -12,6 +12,7 @@ import { modelRecordValue } from "../reasoning-effort"; import type { TranslatorBudget } from "../lib/translator-budget"; import { rewriteRoutedCustomToolsForUpstream } from "../responses/custom-tool-compat"; import { rewriteRoutedToolSearchForUpstream } from "../responses/tool-search-compat"; +import { rewriteRoutedNamespaceToolsForUpstream } from "../responses/namespace-tool-compat"; import { openaiResponsesUrl } from "./openai-responses-url"; import { createAdapterTierMetadata, @@ -120,6 +121,52 @@ function stripInvalidItemIds(body: unknown): unknown { return changed ? { ...body, input } : body; } +/** + * Codex attaches ChatGPT's private `external_web_access` policy bit to the public + * `web_search` tool. Third-party Responses APIs enable browsing by the presence of the tool and + * commonly reject the extra argument (xAI returns `Argument not supported: + * external_web_access`). Keep the hosted tool and every public option, but remove only that + * canonical-only hint before a routed request reaches a non-OpenAI gateway. + */ +function stripCanonicalWebSearchAccessHint(body: unknown): unknown { + if (!isPlainObject(body)) return body; + + const rewriteTools = (tools: unknown[]): unknown[] => { + let changed = false; + const rewritten = tools.map(tool => { + if ( + !isPlainObject(tool) + || tool.type !== "web_search" + || !Object.hasOwn(tool, "external_web_access") + ) { + return tool; + } + changed = true; + const { external_web_access: _externalWebAccess, ...rest } = tool; + return rest; + }); + return changed ? rewritten : tools; + }; + + let rewrittenBody = body; + if (Array.isArray(body.tools)) { + const tools = rewriteTools(body.tools); + if (tools !== body.tools) rewrittenBody = { ...rewrittenBody, tools }; + } + if (!Array.isArray(body.input)) return rewrittenBody; + + let input: unknown[] | undefined; + for (let index = 0; index < body.input.length; index += 1) { + const item = body.input[index]; + if (!isPlainObject(item) || item.type !== "additional_tools" || !Array.isArray(item.tools)) continue; + const tools = rewriteTools(item.tools); + if (tools === item.tools) continue; + input ??= [...body.input]; + input[index] = { ...item, tools }; + } + return input ? { ...rewrittenBody, input } : rewrittenBody; +} + /** * When `store` is false, the upstream API does not persist response items. Any item ID * forwarded in `input` is then interpreted as a reference to a stored item that does not @@ -1506,6 +1553,7 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): const forward = provider.authMode === "forward"; let convertedRoutedCustomToolNames: Set | undefined; let convertedRoutedToolSearchNames: Set | undefined; + let convertedRoutedNamespaceToolAliases: Map | undefined; const unexpandedMiss = !!parsed.previousResponseId && parsed._previousResponseInputExpanded !== true; let outBody = stripPreviousResponseId( parsed._rawBody, @@ -1572,6 +1620,17 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): outBody = rewritten.body; convertedRoutedToolSearchNames = rewritten.names; } + if (!isCanonicalOpenAiForwardProvider(provider)) { + // Codex 0.147 emits private namespace tool groups, while public/third-party Responses + // gateways accept only flat tool variants. Run after custom/tool-search lowering so + // namespace children already carry their final public kind before they are promoted. + const rewritten = rewriteRoutedNamespaceToolsForUpstream(outBody); + outBody = rewritten.body; + convertedRoutedNamespaceToolAliases = rewritten.aliases; + } + if (!isCanonicalOpenAiForwardProvider(provider)) { + outBody = stripCanonicalWebSearchAccessHint(outBody); + } const sanitizedBody = normalizeToolSchemas(stripSparkCompatibility(stripUnsupportedReasoningParams(stripItemIdsWhenUnstored(stripInvalidItemIds(stripUnsupportedHostedTools(sanitizeReasoningInputContent(scrubOcxCompactionItems(outBody), { preserveRawReasoningContent: provider.preserveResponsesReasoningContent === true }))))))); const finalBody = stripDisabledReasoningSummaries( normalizeConfiguredReasoningSummaryDelivery(sanitizedBody, provider, parsed.modelId), @@ -1600,6 +1659,7 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): releaseBodyObservation, ...(convertedRoutedCustomToolNames ? { convertedRoutedCustomToolNames } : {}), ...(convertedRoutedToolSearchNames ? { convertedRoutedToolSearchNames } : {}), + ...(convertedRoutedNamespaceToolAliases ? { convertedRoutedNamespaceToolAliases } : {}), ...(tierLog ? { tierLog } : {}), }; }, diff --git a/src/responses/namespace-tool-compat.ts b/src/responses/namespace-tool-compat.ts new file mode 100644 index 0000000000..3ab954cbb7 --- /dev/null +++ b/src/responses/namespace-tool-compat.ts @@ -0,0 +1,277 @@ +import { namespacedToolName } from "../types"; +import { collectResponsesToolGroups } from "./tool-groups"; + +export interface RoutedNamespaceToolIdentity { + namespace: string; + name: string; +} + +export type RoutedNamespaceToolAliases = ReadonlyMap; + +const BUILTIN_FUNCTIONS_NAMESPACE = "functions"; + +function isPlainObject(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +function namespaceIdentity(namespace: string, name: string): string { + return `${namespace}\u0000${name}`; +} + +function namespaceToolIdentity(tool: unknown): RoutedNamespaceToolIdentity | undefined { + if ( + !isPlainObject(tool) + || tool.type !== "namespace" + || typeof tool.name !== "string" + || tool.name.length === 0 + || !Array.isArray(tool.tools) + || tool.tools.length === 0 + ) return undefined; + return { namespace: tool.name, name: "" }; +} + +function namespaceChildren(tool: unknown): Record[] | undefined { + const identity = namespaceToolIdentity(tool); + if (!identity || !isPlainObject(tool) || !Array.isArray(tool.tools)) return undefined; + const children: Record[] = []; + for (const child of tool.tools) { + if ( + !isPlainObject(child) + || child.type === "namespace" + || typeof child.name !== "string" + || child.name.length === 0 + ) return undefined; + children.push(child); + } + return children; +} + +function addSelector( + selectors: Map, + selector: string, + wireName: string, +): void { + const current = selectors.get(selector); + if (current === undefined) selectors.set(selector, wireName); + else if (current !== wireName) selectors.set(selector, null); +} + +type NamespaceRewritePlan = { + aliases: Map; + identities: Map; + selectors: Map; +}; + +function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { + const aliases = new Map(); + const identities = new Map(); + const selectors = new Map(); + const directNames = new Set(); + + for (const group of groups) { + for (const tool of group) { + if ( + isPlainObject(tool) + && tool.type !== "namespace" + && typeof tool.name === "string" + && tool.name.length > 0 + ) directNames.add(tool.name); + } + } + + const wireOwners = new Map(); + for (const name of directNames) wireOwners.set(name, `direct:${name}`); + + for (const group of groups) { + for (const tool of group) { + const parent = namespaceToolIdentity(tool); + const children = namespaceChildren(tool); + if (!parent || !children) continue; + for (const child of children) { + const childName = child.name as string; + const identity = namespaceIdentity(parent.namespace, childName); + const wireName = parent.namespace === BUILTIN_FUNCTIONS_NAMESPACE + ? childName + : namespacedToolName(parent.namespace, childName); + const owner = wireOwners.get(wireName); + if (owner !== undefined && owner !== identity) { + throw new Error( + `namespace tool wire-name collision for "${wireName}"; rename one of the colliding tools`, + ); + } + wireOwners.set(wireName, identity); + identities.set(identity, wireName); + addSelector(selectors, wireName, wireName); + addSelector(selectors, `${parent.namespace}.${childName}`, wireName); + addSelector(selectors, childName, wireName); + if (parent.namespace !== BUILTIN_FUNCTIONS_NAMESPACE) { + aliases.set(wireName, { namespace: parent.namespace, name: childName }); + } + } + } + } + + return { aliases, identities, selectors }; +} + +function rewriteToolList( + tools: unknown[], + plan: NamespaceRewritePlan, +): { tools: unknown[]; changed: boolean } { + let changed = false; + const rewritten: unknown[] = []; + for (const tool of tools) { + const parent = namespaceToolIdentity(tool); + const children = namespaceChildren(tool); + if (!parent || !children) { + rewritten.push(tool); + continue; + } + changed = true; + for (const child of children) { + const identity = namespaceIdentity(parent.namespace, child.name as string); + const wireName = plan.identities.get(identity); + rewritten.push(wireName && wireName !== child.name ? { ...child, name: wireName } : child); + } + } + return changed ? { tools: rewritten, changed: true } : { tools, changed: false }; +} + +function rewriteNamedSelector(value: unknown, plan: NamespaceRewritePlan): unknown { + if (!isPlainObject(value) || typeof value.name !== "string") return value; + const explicitNamespace = typeof value.namespace === "string" ? value.namespace : undefined; + const wireName = explicitNamespace + ? plan.identities.get(namespaceIdentity(explicitNamespace, value.name)) + : plan.selectors.get(value.name) ?? undefined; + if (!wireName) return value; + const { namespace: _namespace, ...rest } = value; + return wireName === value.name && explicitNamespace === undefined + ? value + : { ...rest, name: wireName }; +} + +function rewriteToolChoice(value: unknown, plan: NamespaceRewritePlan): unknown { + if (!isPlainObject(value)) return value; + if ((value.type === "function" || value.type === "custom") && typeof value.name === "string") { + return rewriteNamedSelector(value, plan); + } + if (value.type !== "allowed_tools" || !Array.isArray(value.tools)) return value; + let changed = false; + const tools = value.tools.map(tool => { + if ( + !isPlainObject(tool) + || (tool.type !== "function" && tool.type !== "custom") + || typeof tool.name !== "string" + ) return tool; + const rewritten = rewriteNamedSelector(tool, plan); + changed ||= rewritten !== tool; + return rewritten; + }); + return changed ? { ...value, tools } : value; +} + +function rewriteInputItem(item: unknown, plan: NamespaceRewritePlan): unknown { + if (!isPlainObject(item)) return item; + if (item.type === "additional_tools" && Array.isArray(item.tools)) { + const rewritten = rewriteToolList(item.tools, plan); + return rewritten.changed ? { ...item, tools: rewritten.tools } : item; + } + if ( + (item.type === "function_call" || item.type === "custom_tool_call") + && typeof item.name === "string" + ) return rewriteNamedSelector(item, plan); + return item; +} + +/** + * Lower Codex's private Responses namespace declarations for public/third-party gateways. + * + * Codex 0.147 groups ordinary tools under the reserved `functions` namespace; those children + * become bare top-level declarations. Other namespaces use the same collision-checked + * `__` wire identity as the chat adapters. The returned request-local aliases + * are the only names response restoration is allowed to expand. + */ +export function rewriteRoutedNamespaceToolsForUpstream(body: unknown): { + body: unknown; + aliases: Map; +} { + if (!isPlainObject(body)) return { body, aliases: new Map() }; + const groups = collectResponsesToolGroups(body); + const plan = buildRewritePlan(groups); + if (plan.identities.size === 0) return { body, aliases: plan.aliases }; + + let tools = body.tools; + if (Array.isArray(body.tools)) tools = rewriteToolList(body.tools, plan).tools; + + let input = body.input; + if (Array.isArray(body.input)) input = body.input.map(item => rewriteInputItem(item, plan)); + + const toolChoice = rewriteToolChoice(body.tool_choice, plan); + return { + body: { + ...body, + ...(tools !== body.tools ? { tools } : {}), + ...(input !== body.input ? { input } : {}), + ...(toolChoice !== body.tool_choice ? { tool_choice: toolChoice } : {}), + }, + aliases: plan.aliases, + }; +} + +export function restoreRoutedNamespaceCalls( + value: unknown, + aliases: RoutedNamespaceToolAliases, +): { value: unknown; changed: boolean } { + if (Array.isArray(value)) { + let changed = false; + const restored = value.map(entry => { + const result = restoreRoutedNamespaceCalls(entry, aliases); + changed ||= result.changed; + return result.value; + }); + return changed ? { value: restored, changed: true } : { value, changed: false }; + } + if (!isPlainObject(value)) return { value, changed: false }; + + let changed = false; + const restored: Record = {}; + for (const [key, entry] of Object.entries(value)) { + const result = restoreRoutedNamespaceCalls(entry, aliases); + restored[key] = result.value; + changed ||= result.changed; + } + + if ( + (value.type === "function_call" || value.type === "custom_tool_call") + && typeof value.name === "string" + ) { + const identity = aliases.get(value.name); + if (identity) { + restored.name = identity.name; + restored.namespace = identity.namespace; + changed = true; + } + } + return changed ? { value: restored, changed: true } : { value, changed: false }; +} + +export function restoreRoutedNamespaceCallsInJson( + text: string, + aliases: RoutedNamespaceToolAliases, +): string { + if (aliases.size === 0) return text; + let payload: unknown; + try { + payload = JSON.parse(text); + } catch { + return text; + } + const restored = restoreRoutedNamespaceCalls(payload, aliases); + return restored.changed ? JSON.stringify(restored.value) : text; +} + +export function createRoutedNamespaceCallRestoreRewrite( + aliases: RoutedNamespaceToolAliases, +): (payload: string) => string { + return payload => restoreRoutedNamespaceCallsInJson(payload, aliases); +} diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 3c7773e4e4..830ed32704 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -275,6 +275,11 @@ import { restoreRoutedCustomCallsInJson } from "../../responses/custom-tool-comp import { createRoutedCustomToolRestoreBlockRewrite } from "../responses-custom-tool-repair"; import { restoreRoutedToolSearchCallsInJson } from "../../responses/tool-search-compat"; import { createRoutedToolSearchRestoreBlockRewrite } from "../responses-tool-search-repair"; +import { + createRoutedNamespaceCallRestoreRewrite, + restoreRoutedNamespaceCallsInJson, + type RoutedNamespaceToolAliases, +} from "../../responses/namespace-tool-compat"; import { collectDeclaredWireToolNames, createUndeclaredToolCallGuardBlockRewrite, @@ -2501,6 +2506,7 @@ async function handleResponsesInner( : imageGenToolCallAliases(toolBridgeMaps.toolNsMap, parsed._rawBody, translatorBudget); const routedCustomToolNames = new Set(); const routedToolSearchNames = new Set(); + let routedNamespaceToolAliases: RoutedNamespaceToolAliases = new Map(); // Local continuation cache for the ChatGPT passthrough. Codex WS turns chain with // previous_response_id, ocx converts them to internal HTTP requests, and the ChatGPT Codex // REST backend rejects the parameter — the adapter strips it in forward mode, so the ONLY @@ -2540,6 +2546,7 @@ async function handleResponsesInner( // would incorrectly disable restoration for the exact ambiguous-name case the alias fixes. routedToolSearchNames.add(name); } + routedNamespaceToolAliases = request.convertedRoutedNamespaceToolAliases ?? routedNamespaceToolAliases; // #1700: the bridged paths refuse a call to a tool the request never declared // (`declaredToolNames`, src/bridge.ts). The passthrough had no equivalent, so a routed // provider's top-level `apply_patch` — which under Codex code mode exists only as a nested @@ -3074,6 +3081,9 @@ async function handleResponsesInner( // Compose opt-in payload rewrites into one parse/stringify pass (image-gen restore first). const payloadRewrites = [ createImageGenCallRestoreRewrite(imageGenCallAliases), + routedNamespaceToolAliases.size > 0 + ? createRoutedNamespaceCallRestoreRewrite(routedNamespaceToolAliases) + : undefined, hasResponsesItemIdRepair(repairConfig) ? createResponsesItemIdPayloadRewrite(repairConfig!, translatorBudget) : undefined, @@ -3290,8 +3300,12 @@ async function handleResponsesInner( const text = bounded.text; inspectResponseLogJson(logCtx, text); const clientJson = (() => { - const restored = restoreRoutedCustomCallsInJson( + const restoredNamespace = restoreRoutedNamespaceCallsInJson( restoreImageGenCallsInJson(text, imageGenCallAliases), + routedNamespaceToolAliases, + ); + const restored = restoreRoutedCustomCallsInJson( + restoredNamespace, routedCustomToolNames, ); const restoredToolSearch = restoreRoutedToolSearchCallsInJson( diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index efd1c87dbb..7404311224 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -39,6 +39,20 @@ Responses-compatible streaming output. - 다른 대안 대신 이 방식을 선택한 이유: Provider-specific workarounds fragment the contract, while unconditional restoration could turn an untrusted ordinary function call into a privileged client discovery action. - 장점, 단점 및 영향: Strict third-party Responses gateways can start and continue deferred discovery without changing native ChatGPT behavior; ordinary same-named functions remain distinct, and the proxy performs a capped SSE lifecycle rewrite only when the request actually required compatibility translation. +[Decision Log] +- 목적과 의도: Keep Codex 0.147 namespace tool catalogs usable after a routed provider adopts native Responses but implements only the public flat tool variants. +- 기존 구현 및 제약 조건: Chat translation already flattened namespace children, while native Responses passthrough forwarded the private `namespace` variant unchanged. xAI therefore rejected Grok requests before inference after its OAuth Grok 4.5/4.6 route moved to Responses. +- 검토한 주요 대안: Move Grok back to Chat; special-case only xAI or the reserved `functions` group; flatten every complete namespace on noncanonical Responses and restore request-authorized aliases on return. +- 선택한 방식: Noncanonical Responses lowers `functions` children to their bare top-level names and every other complete namespace to collision-checked `__` aliases after custom/tool-search conversion. It rewrites matching replay calls and tool selectors, records the aliases on the built request, and restores only those aliases in JSON/SSE call items before custom/tool-search lifecycle repair. Canonical OpenAI forward preserves native namespace shapes. +- 다른 대안 대신 이 방식을 선택한 이유: A transport regression should not discard Responses streaming or create a provider-specific fork, and restoration without request-local authorization could reinterpret an unrelated upstream function as a client namespace call. +- 장점, 단점 및 영향: Grok and other public-schema Responses gateways accept current Codex catalogs while Codex still receives explicit namespace routing. Ambiguous wire names fail closed; empty, malformed, and future nested namespace shapes remain untouched rather than losing tools silently. + +The same noncanonical boundary strips ChatGPT's private `external_web_access` bit from routed +`web_search` declarations. The public tool remains enabled and all other options remain intact; +canonical OpenAI forwarding preserves the bit. xAI's public Responses schema enables browsing by +the presence of `web_search` and rejects the private argument, so forwarding it made the first +post-namespace request fail with HTTP 400. + The option-aware `openai` provider uses `openai-responses` with `authMode: "forward"`. Pool mode resolves main plus added accounts through affinity/quota/cooldown ownership; Direct forwards only the allowed Codex/OpenAI auth/session headers from the current request and short-circuits pool @@ -222,9 +236,10 @@ items restore `{ namespace: "image_gen", name: "" }` so Codex can di extension. When item-id repair is also enabled, both transforms compose in one SSE parse/stringify pass (`src/server/sse-payload-rewrite.ts`) rather than chaining separate JS pull wrappers. Inspection and continuation-cache branches keep the raw upstream alias, allowing stored -replays to return upstream without leaking a client-only namespace shape. Malformed, empty, and -unrelated namespaces remain untouched. ChatGPT forward mode preserves the private namespace and -hosted tool because that backend understands their native semantics. +replays to return upstream without leaking a client-only namespace shape. Malformed and empty +image-gen namespaces remain untouched; complete unrelated namespaces follow the general +noncanonical namespace compatibility rule above. ChatGPT forward mode preserves the private +namespace and hosted tool because that backend understands their native semantics. Per-model `modelReasoningSummaryDelivery` is a narrow compatibility layer for `openai-responses` gateways whose summary capability is real but whose accepted delivery enum diff --git a/tests/namespace-tool-compat.test.ts b/tests/namespace-tool-compat.test.ts new file mode 100644 index 0000000000..59e535f1c7 --- /dev/null +++ b/tests/namespace-tool-compat.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, test } from "bun:test"; +import { + createRoutedNamespaceCallRestoreRewrite, + restoreRoutedNamespaceCalls, + restoreRoutedNamespaceCallsInJson, + rewriteRoutedNamespaceToolsForUpstream, +} from "../src/responses/namespace-tool-compat"; + +describe("Responses namespace tool compatibility", () => { + test("flattens builtin and routed namespaces across declarations, selectors, and replay", () => { + const rewritten = rewriteRoutedNamespaceToolsForUpstream({ + model: "routed-model", + tools: [ + { + type: "namespace", + name: "functions", + tools: [{ type: "custom", name: "exec", description: "run" }], + }, + { + type: "namespace", + name: "collaboration", + tools: [{ type: "function", name: "spawn_agent", parameters: {} }], + }, + ], + input: [ + { + type: "function_call", + namespace: "collaboration", + name: "spawn_agent", + call_id: "call_spawn", + arguments: "{}", + }, + { + type: "custom_tool_call", + namespace: "functions", + name: "exec", + call_id: "call_exec", + input: "text(true)", + }, + ], + tool_choice: { + type: "allowed_tools", + mode: "required", + tools: [ + { type: "function", namespace: "collaboration", name: "spawn_agent" }, + { type: "custom", namespace: "functions", name: "exec" }, + ], + }, + }); + const body = rewritten.body as { + tools: Array<{ type: string; name: string }>; + input: Array<{ namespace?: string; name: string }>; + tool_choice: { tools: Array<{ namespace?: string; name: string }> }; + }; + + expect(body.tools).toEqual([ + { type: "custom", name: "exec", description: "run" }, + { type: "function", name: "collaboration__spawn_agent", parameters: {} }, + ]); + expect(body.input[0]).toMatchObject({ name: "collaboration__spawn_agent", call_id: "call_spawn" }); + expect(body.input[0]).not.toHaveProperty("namespace"); + expect(body.input[1]).toMatchObject({ name: "exec", call_id: "call_exec" }); + expect(body.input[1]).not.toHaveProperty("namespace"); + expect(body.tool_choice.tools).toEqual([ + { type: "function", name: "collaboration__spawn_agent" }, + { type: "custom", name: "exec" }, + ]); + expect([...rewritten.aliases]).toEqual([ + ["collaboration__spawn_agent", { namespace: "collaboration", name: "spawn_agent" }], + ]); + }); + + test("rewrites a unique bare selector but leaves an ambiguous one unchanged", () => { + const unique = rewriteRoutedNamespaceToolsForUpstream({ + tools: [{ + type: "namespace", + name: "one", + tools: [{ type: "function", name: "read" }], + }], + tool_choice: { type: "function", name: "read" }, + }).body as { tool_choice: { name: string } }; + expect(unique.tool_choice.name).toBe("one__read"); + + const ambiguous = rewriteRoutedNamespaceToolsForUpstream({ + tools: [ + { type: "namespace", name: "one", tools: [{ type: "function", name: "read" }] }, + { type: "namespace", name: "two", tools: [{ type: "function", name: "read" }] }, + ], + tool_choice: { type: "function", name: "read" }, + }).body as { tool_choice: { name: string } }; + expect(ambiguous.tool_choice.name).toBe("read"); + }); + + test("fails closed when flattening would collide with a declared wire name", () => { + expect(() => rewriteRoutedNamespaceToolsForUpstream({ + tools: [ + { type: "function", name: "workspace__read" }, + { type: "namespace", name: "workspace", tools: [{ type: "function", name: "read" }] }, + ], + })).toThrow('namespace tool wire-name collision for "workspace__read"'); + }); + + test("preserves empty and malformed namespaces instead of dropping capabilities", () => { + const body = { + tools: [ + { type: "namespace", name: "empty", tools: [] }, + { type: "namespace", name: "nested", tools: [{ type: "namespace", name: "child", tools: [] }] }, + ], + }; + expect(rewriteRoutedNamespaceToolsForUpstream(body).body).toEqual(body); + }); + + test("restores only aliases authorized by this request in JSON and SSE payloads", () => { + const aliases = new Map([ + ["collaboration__spawn_agent", { namespace: "collaboration", name: "spawn_agent" }], + ]); + const payload = { + type: "response.completed", + response: { + output: [ + { type: "function_call", name: "collaboration__spawn_agent", call_id: "call_1" }, + { type: "function_call", name: "untrusted__tool", call_id: "call_2" }, + ], + }, + }; + + expect(restoreRoutedNamespaceCalls(payload, aliases).value).toMatchObject({ + response: { + output: [ + { type: "function_call", namespace: "collaboration", name: "spawn_agent" }, + { type: "function_call", name: "untrusted__tool" }, + ], + }, + }); + const text = JSON.stringify(payload); + expect(JSON.parse(restoreRoutedNamespaceCallsInJson(text, aliases))).toMatchObject({ + response: { output: [ + { namespace: "collaboration", name: "spawn_agent" }, + { name: "untrusted__tool" }, + ] }, + }); + expect(JSON.parse(createRoutedNamespaceCallRestoreRewrite(aliases)(text))).toMatchObject({ + response: { output: [ + { namespace: "collaboration", name: "spawn_agent" }, + { name: "untrusted__tool" }, + ] }, + }); + expect(restoreRoutedNamespaceCallsInJson("not-json", aliases)).toBe("not-json"); + }); +}); diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index 52ed4cf3d1..39ba0faec5 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -336,15 +336,15 @@ describe("OpenAI Responses passthrough sanitization", () => { }>; }; - const namespace = body.tools.find(tool => tool.type === "namespace" && tool.name === "workspace"); - expect(namespace?.tools?.map(tool => tool.name)).toEqual([ - "upfront_read", - "declared_deferred_read", - "deferred_read", + expect(body.tools.some(tool => tool.type === "namespace")).toBe(false); + expect(body.tools.filter(tool => tool.name?.startsWith("workspace__")).map(tool => tool.name)).toEqual([ + "workspace__upfront_read", + "workspace__declared_deferred_read", + "workspace__deferred_read", ]); - expect(namespace?.tools?.find(tool => tool.name === "declared_deferred_read")) + expect(body.tools.find(tool => tool.name === "workspace__declared_deferred_read")) .not.toHaveProperty("defer_loading"); - expect(namespace?.tools?.find(tool => tool.name === "deferred_read")) + expect(body.tools.find(tool => tool.name === "workspace__deferred_read")) .not.toHaveProperty("defer_loading"); expect(body.tools.find(tool => tool.name === "tool_search")).toMatchObject({ type: "function", @@ -419,11 +419,11 @@ describe("OpenAI Responses passthrough sanitization", () => { expect(body.tools).toBeUndefined(); const additionalTools = body.input.find(item => item.type === "additional_tools")?.tools; - const namespace = additionalTools?.find(tool => tool.type === "namespace" && tool.name === "workspace"); - expect(namespace?.tools?.map(tool => tool.name)).toEqual([ - "upfront_read", - "declared_deferred_read", - "deferred_read", + expect(additionalTools?.some(tool => tool.type === "namespace")).toBe(false); + expect(additionalTools?.filter(tool => tool.name?.startsWith("workspace__")).map(tool => tool.name)).toEqual([ + "workspace__upfront_read", + "workspace__declared_deferred_read", + "workspace__deferred_read", ]); expect(additionalTools?.find(tool => tool.name === "tool_search")) .toMatchObject({ type: "function", name: "tool_search" }); @@ -816,6 +816,60 @@ describe("OpenAI Responses passthrough sanitization", () => { expect(body.tools[0]).toMatchObject({ type: "image_generation" }); }); + test("drops ChatGPT's external_web_access hint but keeps routed web search", () => { + const adapter = createResponsesPassthroughAdapter({ + adapter: "openai-responses", + baseUrl: "https://api.x.ai/v1", + authMode: "key" as const, + apiKey: "xai-test", + }); + const request = adapter.buildRequest({ + modelId: "grok-4.6", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "grok-4.6", + input: [{ + type: "additional_tools", + tools: [{ type: "web_search", external_web_access: true, search_context_size: "medium" }], + }], + tools: [{ type: "web_search", external_web_access: false, filters: { allowed_domains: ["example.com"] } }], + }, + }, { headers: new Headers() }); + const body = JSON.parse(request.body) as { + tools: Record[]; + input: Array<{ type: string; tools: Record[] }>; + }; + + expect(body.tools).toEqual([{ + type: "web_search", + filters: { allowed_domains: ["example.com"] }, + }]); + expect(body.input[0]?.tools).toEqual([{ + type: "web_search", + search_context_size: "medium", + }]); + }); + + test("preserves external_web_access on the canonical OpenAI forward route", () => { + const adapter = createResponsesPassthroughAdapter(provider); + const request = adapter.buildRequest({ + modelId: "gpt-5.5", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "gpt-5.5", + input: [], + tools: [{ type: "web_search", external_web_access: true }], + }, + }, { headers: new Headers({ authorization: "Bearer token" }) }); + const body = JSON.parse(request.body) as { tools: Record[] }; + + expect(body.tools).toEqual([{ type: "web_search", external_web_access: true }]); + }); + test("preserves prompt_cache_key in the raw Responses passthrough body", () => { const adapter = createResponsesPassthroughAdapter(provider); const request = adapter.buildRequest({ @@ -1488,7 +1542,7 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(JSON.parse(secondRequest.body)).toEqual(firstBody); }); - test("keyed platform preserves unrelated and malformed namespaces", () => { + test("keyed platform flattens complete namespaces and preserves malformed ones", () => { const adapter = createResponsesPassthroughAdapter(keyedProvider); const request = adapter.buildRequest({ modelId: "gpt-5.6-sol", @@ -1518,9 +1572,9 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(body.tools).toEqual([ { type: "namespace", name: "image_gen", tools: [] }, { - type: "namespace", - name: "web", - tools: [{ type: "function", name: "run", parameters: {} }], + type: "function", + name: "web__run", + parameters: { type: "object" }, }, { type: "image_generation" }, ]); diff --git a/tests/server-xai-responses-streaming.test.ts b/tests/server-xai-responses-streaming.test.ts index 7195436a60..0adba13149 100644 --- a/tests/server-xai-responses-streaming.test.ts +++ b/tests/server-xai-responses-streaming.test.ts @@ -220,4 +220,126 @@ describe("xAI OAuth Responses streaming", () => { await server.stop(true); } }, 10_000); + + test("lowers Codex namespaces for xAI and restores routed calls on the client stream", async () => { + let outboundBody: Record | undefined; + + globalThis.fetch = (async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url !== RESPONSES_ENDPOINT) return originalFetch(input, init); + outboundBody = JSON.parse(String(init?.body)) as Record; + const call = { + id: "fc_spawn", + type: "function_call", + status: "completed", + name: "collaboration__spawn_agent", + call_id: "call_spawn", + arguments: "{}", + }; + const body = new ReadableStream({ + start(controller) { + controller.enqueue(sse({ + type: "response.created", + sequence_number: 0, + response: { id: "resp_namespace", object: "response", status: "in_progress", model: "grok-4.6", output: [] }, + })); + controller.enqueue(sse({ + type: "response.output_item.added", + sequence_number: 1, + output_index: 0, + item: call, + })); + controller.enqueue(sse({ + type: "response.output_item.done", + sequence_number: 2, + output_index: 0, + item: call, + })); + controller.enqueue(sse({ + type: "response.completed", + sequence_number: 3, + response: { + id: "resp_namespace", + object: "response", + status: "completed", + model: "grok-4.6", + output: [call], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + }, + })); + controller.close(); + }, + }); + return new Response(body, { headers: { "content-type": "text/event-stream" } }); + }) as typeof fetch; + + saveConfig(config()); + const server = startServer(0); + try { + const response = await originalFetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + model: "xai/grok-4.6", + stream: true, + store: false, + tools: [{ type: "web_search", external_web_access: true }], + input: [ + { + type: "additional_tools", + role: "developer", + tools: [ + { + type: "namespace", + name: "functions", + tools: [{ type: "custom", name: "exec", description: "run code", format: { type: "text" } }], + }, + { + type: "namespace", + name: "collaboration", + tools: [{ type: "function", name: "spawn_agent", description: "spawn", parameters: {} }], + }, + ], + }, + { type: "message", role: "user", content: [{ type: "input_text", text: "delegate" }] }, + ], + }), + }); + expect(response.status).toBe(200); + const clientText = await response.text(); + + const outboundInput = outboundBody?.input as Array<{ + type: string; + tools?: Array<{ type: string; name?: string }>; + }> | undefined; + const outboundTools = outboundInput?.find(item => item.type === "additional_tools")?.tools; + expect(outboundTools?.some(tool => tool.type === "namespace")).toBe(false); + expect(outboundTools?.find(tool => tool.name === "exec")?.type).toBe("function"); + expect(outboundTools?.find(tool => tool.name === "collaboration__spawn_agent")?.type).toBe("function"); + expect(outboundBody?.tools).toEqual([{ type: "web_search" }]); + + const payloads = clientText + .split(/\r?\n/) + .filter(line => line.startsWith("data: ") && line !== "data: [DONE]") + .map(line => JSON.parse(line.slice(6)) as Record); + const added = payloads.find(payload => payload.type === "response.output_item.added") as { + item?: Record; + } | undefined; + expect(added?.item).toMatchObject({ + type: "function_call", + namespace: "collaboration", + name: "spawn_agent", + call_id: "call_spawn", + }); + const completed = payloads.find(payload => payload.type === "response.completed") as { + response?: { output?: Array> }; + } | undefined; + expect(completed?.response?.output?.[0]).toMatchObject({ + namespace: "collaboration", + name: "spawn_agent", + }); + } finally { + await server.stop(true); + } + }, 10_000); }); From e4508d045efd8c5642e8ebfc2f2a422c3de759a2 Mon Sep 17 00:00:00 2001 From: olddonkey Date: Thu, 20 Aug 2026 13:04:45 -0700 Subject: [PATCH 2/4] fix(responses): address namespace review findings --- src/responses/namespace-tool-compat.ts | 5 +- tests/namespace-tool-compat.test.ts | 13 ++++ tests/server-xai-responses-streaming.test.ts | 62 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/responses/namespace-tool-compat.ts b/src/responses/namespace-tool-compat.ts index 3ab954cbb7..98d9f6c54e 100644 --- a/src/responses/namespace-tool-compat.ts +++ b/src/responses/namespace-tool-compat.ts @@ -75,7 +75,10 @@ function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { && tool.type !== "namespace" && typeof tool.name === "string" && tool.name.length > 0 - ) directNames.add(tool.name); + ) { + directNames.add(tool.name); + addSelector(selectors, tool.name, tool.name); + } } } diff --git a/tests/namespace-tool-compat.test.ts b/tests/namespace-tool-compat.test.ts index 59e535f1c7..1801fb3071 100644 --- a/tests/namespace-tool-compat.test.ts +++ b/tests/namespace-tool-compat.test.ts @@ -89,6 +89,19 @@ describe("Responses namespace tool compatibility", () => { tool_choice: { type: "function", name: "read" }, }).body as { tool_choice: { name: string } }; expect(ambiguous.tool_choice.name).toBe("read"); + + const directCollision = rewriteRoutedNamespaceToolsForUpstream({ + tools: [ + { type: "function", name: "read" }, + { type: "namespace", name: "workspace", tools: [{ type: "function", name: "read" }] }, + ], + tool_choice: { type: "function", name: "read" }, + }).body as { + tools: Array<{ name: string }>; + tool_choice: { name: string }; + }; + expect(directCollision.tools.map(tool => tool.name)).toEqual(["read", "workspace__read"]); + expect(directCollision.tool_choice.name).toBe("read"); }); test("fails closed when flattening would collide with a declared wire name", () => { diff --git a/tests/server-xai-responses-streaming.test.ts b/tests/server-xai-responses-streaming.test.ts index 0adba13149..19800234a7 100644 --- a/tests/server-xai-responses-streaming.test.ts +++ b/tests/server-xai-responses-streaming.test.ts @@ -342,4 +342,66 @@ describe("xAI OAuth Responses streaming", () => { await server.stop(true); } }, 10_000); + + test("restores routed namespace calls in a non-streaming xAI JSON response", async () => { + let outboundBody: Record | undefined; + const call = { + id: "fc_spawn_json", + type: "function_call", + status: "completed", + name: "collaboration__spawn_agent", + call_id: "call_spawn_json", + arguments: "{}", + }; + + globalThis.fetch = (async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url !== RESPONSES_ENDPOINT) return originalFetch(input, init); + outboundBody = JSON.parse(String(init?.body)) as Record; + return Response.json({ + id: "resp_namespace_json", + object: "response", + status: "completed", + model: "grok-4.6", + output: [call], + usage: { input_tokens: 1, output_tokens: 1, total_tokens: 2 }, + }); + }) as typeof fetch; + + saveConfig(config()); + const server = startServer(0); + try { + const response = await originalFetch(new URL("/v1/responses", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + model: "xai/grok-4.6", + stream: false, + store: false, + tools: [{ + type: "namespace", + name: "collaboration", + tools: [{ type: "function", name: "spawn_agent", description: "spawn", parameters: {} }], + }], + input: "delegate", + }), + }); + expect(response.status).toBe(200); + + const outboundTools = outboundBody?.tools as Array<{ type: string; name?: string }> | undefined; + expect(outboundTools).toEqual([expect.objectContaining({ + type: "function", + name: "collaboration__spawn_agent", + })]); + const clientBody = await response.json() as { output?: Array> }; + expect(clientBody.output?.[0]).toMatchObject({ + type: "function_call", + namespace: "collaboration", + name: "spawn_agent", + call_id: "call_spawn_json", + }); + } finally { + await server.stop(true); + } + }, 10_000); }); From bf84d174654ab5b212dabe0492faf636dd578e5b Mon Sep 17 00:00:00 2001 From: olddonkey Date: Thu, 20 Aug 2026 15:46:50 -0700 Subject: [PATCH 3/4] fix(responses): close the remaining private-shape leaks on the routed boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The namespace boundary lowered complete groups but still let several Codex-private shapes reach a strict gateway, each reproducing the pre-inference rejection the boundary exists to prevent. No `type: "namespace"` value survives now. A group the layer cannot express — empty, nested, or with an unusable child name — is dropped along with the children it cannot represent. Relaying the private shape costs the whole request rather than one tool, so "preserve rather than lose a tool" was losing strictly more. Replayed call items are lowered whether or not this turn declares the group they name. The routed compaction turn strips the entire tool surface before the boundary runs, so every compaction after a namespaced tool call shipped the private `namespace` key this layer's own restoration had stamped on the item. Only tool_choice resolves a bare name through the catalog: a history item records which tool actually ran, so re-pointing it at a same-named namespace child would rewrite that record on a coincidence rather than translate it. Codex-private tool fields now come from one table instead of one bespoke pass each, and it gains `defer_loading` — `activateDeferredTool` clears that only for tools a `tool_search_output` already loaded, so the first turn of a deferred catalog carried it to the wire — and the `web_search_preview` variant. A bare declaration and a `functions` child of the same name are one logical tool: `buildTools` flattens the reserved group without a namespace, the parser tolerates the duplicate, and `promoteClientLoadedTools` produces it. That shape raised a wire-name collision that escaped every catch up to the Bun handler, so an ordinary catalog became an unstructured 500 with no request log — while the rotation-rebuild path answered 400 for the identical throw. It is now deduped, and a genuine collision is a typed error the passthrough maps to 400. Co-Authored-By: Claude Fable 5 --- src/adapters/openai-responses.ts | 47 +++-- src/responses/namespace-tool-compat.ts | 215 +++++++++++++-------- src/server/responses/core.ts | 8 + structure/04_transports-and-sidecars.md | 28 ++- tests/namespace-tool-compat.test.ts | 92 ++++++++- tests/openai-responses-passthrough.test.ts | 59 +++++- 6 files changed, 335 insertions(+), 114 deletions(-) diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index 60f14bb224..0c5b8f0e21 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -122,28 +122,42 @@ function stripInvalidItemIds(body: unknown): unknown { } /** - * Codex attaches ChatGPT's private `external_web_access` policy bit to the public - * `web_search` tool. Third-party Responses APIs enable browsing by the presence of the tool and - * commonly reject the extra argument (xAI returns `Argument not supported: - * external_web_access`). Keep the hosted tool and every public option, but remove only that - * canonical-only hint before a routed request reaches a non-OpenAI gateway. + * Codex-private tool fields that only the ChatGPT backend understands. + * + * A third-party Responses gateway validates its schema and rejects the whole request before + * inference — xAI answers `Argument not supported: external_web_access` — so these are removed at + * the noncanonical boundary while the tool and every public option stay. + * + * Keep this a table. Each private bit Codex attaches has so far arrived as its own bespoke strip + * with its own traversal, and the traversals disagreed about which containers they covered; a new + * one should be a row here instead. `toolTypes` omitted means the field is private on any tool. */ -function stripCanonicalWebSearchAccessHint(body: unknown): unknown { +const CANONICAL_ONLY_TOOL_FIELDS: readonly { field: string; toolTypes?: ReadonlySet }[] = [ + // ChatGPT's browsing policy bit. The public hosted tool is enabled by its presence alone. + { field: "external_web_access", toolTypes: new Set(["web_search", "web_search_preview"]) }, + // Deferred-discovery marker. `activateDeferredTool` clears it only for tools a `tool_search_output` + // already loaded, so a still-deferred declaration — including one promoted out of a namespace + // group — otherwise reaches the wire carrying it. + { field: "defer_loading" }, +]; + +function stripCanonicalOnlyToolFields(body: unknown): unknown { if (!isPlainObject(body)) return body; const rewriteTools = (tools: unknown[]): unknown[] => { let changed = false; const rewritten = tools.map(tool => { - if ( - !isPlainObject(tool) - || tool.type !== "web_search" - || !Object.hasOwn(tool, "external_web_access") - ) { - return tool; + if (!isPlainObject(tool)) return tool; + let next = tool; + for (const { field, toolTypes } of CANONICAL_ONLY_TOOL_FIELDS) { + if (!Object.hasOwn(next, field)) continue; + if (toolTypes && (typeof next.type !== "string" || !toolTypes.has(next.type))) continue; + const { [field]: _private, ...rest } = next; + next = rest; } + if (next === tool) return tool; changed = true; - const { external_web_access: _externalWebAccess, ...rest } = tool; - return rest; + return next; }); return changed ? rewritten : tools; }; @@ -1627,9 +1641,8 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): const rewritten = rewriteRoutedNamespaceToolsForUpstream(outBody); outBody = rewritten.body; convertedRoutedNamespaceToolAliases = rewritten.aliases; - } - if (!isCanonicalOpenAiForwardProvider(provider)) { - outBody = stripCanonicalWebSearchAccessHint(outBody); + // Last, so promoted namespace children are also cleared of Codex-private fields. + outBody = stripCanonicalOnlyToolFields(outBody); } const sanitizedBody = normalizeToolSchemas(stripSparkCompatibility(stripUnsupportedReasoningParams(stripItemIdsWhenUnstored(stripInvalidItemIds(stripUnsupportedHostedTools(sanitizeReasoningInputContent(scrubOcxCompactionItems(outBody), { preserveRawReasoningContent: provider.preserveResponsesReasoningContent === true }))))))); const finalBody = stripDisabledReasoningSummaries( diff --git a/src/responses/namespace-tool-compat.ts b/src/responses/namespace-tool-compat.ts index 98d9f6c54e..8a52251da5 100644 --- a/src/responses/namespace-tool-compat.ts +++ b/src/responses/namespace-tool-compat.ts @@ -18,32 +18,64 @@ function namespaceIdentity(namespace: string, name: string): string { return `${namespace}\u0000${name}`; } -function namespaceToolIdentity(tool: unknown): RoutedNamespaceToolIdentity | undefined { +/** + * A name that can become a wire tool name. Control characters are rejected because the identity + * key below joins namespace and name with NUL: a name carrying one could otherwise forge another + * tool's identity and silently take over its wire name. + */ +function isRepresentableName(name: unknown): name is string { + if (typeof name !== "string" || name.length === 0) return false; + for (let index = 0; index < name.length; index += 1) { + const code = name.charCodeAt(index); + // C0 controls and DEL, written as code points so this source never carries one itself. + if (code <= 0x1f || code === 0x7f) return false; + } + return true; +} + +type NamespaceGroup = { + namespace: string; + /** Children that can be lowered to a flat declaration; unrepresentable ones are omitted. */ + children: Record[]; +}; + +/** + * Read a private namespace group, or return undefined when the value is not one. + * + * Children that cannot be expressed as a flat declaration — a nested group, a missing name, a + * control character in the name — are dropped, and a group left with no children is dropped whole + * by the rewrite. Preserving the private `namespace` shape instead would lose every tool in the + * request rather than one: the strict gateways this layer exists for reject that tool type before + * inference, which is the failure the layer was written to prevent. + */ +function parseNamespaceGroup(tool: unknown): NamespaceGroup | undefined { if ( !isPlainObject(tool) || tool.type !== "namespace" - || typeof tool.name !== "string" - || tool.name.length === 0 + || !isRepresentableName(tool.name) || !Array.isArray(tool.tools) - || tool.tools.length === 0 ) return undefined; - return { namespace: tool.name, name: "" }; -} - -function namespaceChildren(tool: unknown): Record[] | undefined { - const identity = namespaceToolIdentity(tool); - if (!identity || !isPlainObject(tool) || !Array.isArray(tool.tools)) return undefined; const children: Record[] = []; for (const child of tool.tools) { - if ( - !isPlainObject(child) - || child.type === "namespace" - || typeof child.name !== "string" - || child.name.length === 0 - ) return undefined; + if (!isPlainObject(child) || child.type === "namespace" || !isRepresentableName(child.name)) continue; children.push(child); } - return children; + return { namespace: tool.name, children }; +} + +/** + * Wire identity of a lowered tool. A `functions` child and an identical top-level declaration share + * one identity because they denote the same logical tool: `buildTools` flattens the reserved group + * without a namespace, so the parser already treats them as one and tolerates the duplicate. + */ +function loweredIdentity(namespace: string, name: string): string { + return namespace === BUILTIN_FUNCTIONS_NAMESPACE + ? namespaceIdentity(BUILTIN_FUNCTIONS_NAMESPACE, name) + : namespaceIdentity(namespace, name); +} + +function loweredWireName(namespace: string, name: string): string { + return namespace === BUILTIN_FUNCTIONS_NAMESPACE ? name : namespacedToolName(namespace, name); } function addSelector( @@ -62,53 +94,48 @@ type NamespaceRewritePlan = { selectors: Map; }; +/** Two distinct logical tools would occupy one wire name; the caller maps this to a 400. */ +export class NamespaceToolCollisionError extends Error {} + function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { const aliases = new Map(); const identities = new Map(); const selectors = new Map(); - const directNames = new Set(); + const wireOwners = new Map(); for (const group of groups) { for (const tool of group) { - if ( - isPlainObject(tool) - && tool.type !== "namespace" - && typeof tool.name === "string" - && tool.name.length > 0 - ) { - directNames.add(tool.name); + if (isPlainObject(tool) && tool.type !== "namespace" && isRepresentableName(tool.name)) { + // A bare declaration is the reserved group's flattened form, so it claims that identity: + // declaring the same tool both ways is the duplicate the parser already tolerates, not a + // collision, and `promoteClientLoadedTools` produces exactly that shape. + wireOwners.set(tool.name, loweredIdentity(BUILTIN_FUNCTIONS_NAMESPACE, tool.name)); addSelector(selectors, tool.name, tool.name); } } } - const wireOwners = new Map(); - for (const name of directNames) wireOwners.set(name, `direct:${name}`); - for (const group of groups) { for (const tool of group) { - const parent = namespaceToolIdentity(tool); - const children = namespaceChildren(tool); - if (!parent || !children) continue; - for (const child of children) { + const parsed = parseNamespaceGroup(tool); + if (!parsed) continue; + for (const child of parsed.children) { const childName = child.name as string; - const identity = namespaceIdentity(parent.namespace, childName); - const wireName = parent.namespace === BUILTIN_FUNCTIONS_NAMESPACE - ? childName - : namespacedToolName(parent.namespace, childName); + const identity = loweredIdentity(parsed.namespace, childName); + const wireName = loweredWireName(parsed.namespace, childName); const owner = wireOwners.get(wireName); if (owner !== undefined && owner !== identity) { - throw new Error( + throw new NamespaceToolCollisionError( `namespace tool wire-name collision for "${wireName}"; rename one of the colliding tools`, ); } wireOwners.set(wireName, identity); identities.set(identity, wireName); addSelector(selectors, wireName, wireName); - addSelector(selectors, `${parent.namespace}.${childName}`, wireName); + addSelector(selectors, `${parsed.namespace}.${childName}`, wireName); addSelector(selectors, childName, wireName); - if (parent.namespace !== BUILTIN_FUNCTIONS_NAMESPACE) { - aliases.set(wireName, { namespace: parent.namespace, name: childName }); + if (parsed.namespace !== BUILTIN_FUNCTIONS_NAMESPACE) { + aliases.set(wireName, { namespace: parsed.namespace, name: childName }); } } } @@ -117,72 +144,96 @@ function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { return { aliases, identities, selectors }; } +/** + * Lower every namespace group in one tool container. `emitted` is shared across the whole body so + * a tool declared both bare and under `functions` is written once rather than twice. + * + * No `type: "namespace"` value survives this pass, including a group this layer cannot read: + * relaying the private shape is what the strict gateway rejects. + */ function rewriteToolList( tools: unknown[], plan: NamespaceRewritePlan, -): { tools: unknown[]; changed: boolean } { + emitted: Set, +): unknown[] { let changed = false; const rewritten: unknown[] = []; for (const tool of tools) { - const parent = namespaceToolIdentity(tool); - const children = namespaceChildren(tool); - if (!parent || !children) { - rewritten.push(tool); + if (isPlainObject(tool) && tool.type === "namespace") { + changed = true; + const parsed = parseNamespaceGroup(tool); + if (!parsed) continue; + for (const child of parsed.children) { + const wireName = plan.identities.get(loweredIdentity(parsed.namespace, child.name as string)); + if (wireName === undefined || emitted.has(wireName)) continue; + emitted.add(wireName); + rewritten.push(wireName === child.name ? child : { ...child, name: wireName }); + } continue; } - changed = true; - for (const child of children) { - const identity = namespaceIdentity(parent.namespace, child.name as string); - const wireName = plan.identities.get(identity); - rewritten.push(wireName && wireName !== child.name ? { ...child, name: wireName } : child); - } + if (isPlainObject(tool) && isRepresentableName(tool.name)) emitted.add(tool.name); + rewritten.push(tool); } - return changed ? { tools: rewritten, changed: true } : { tools, changed: false }; + return changed ? rewritten : tools; } -function rewriteNamedSelector(value: unknown, plan: NamespaceRewritePlan): unknown { +/** + * Resolve one `{namespace?, name}` reference to its wire name and drop the private `namespace` key. + * + * `bareFallback` is for tool_choice, where a bare name is a selector the caller expects resolved + * against the catalog. Replayed call items pass `false`: a history item records which tool actually + * ran, so resolving a bare name through a same-named namespace child would rewrite history on a + * coincidence rather than translate it. + * + * An explicit namespace is always lowered, even when this turn's catalog no longer declares that + * group — a compaction turn drops the whole catalog, and a catalog can change mid-session. Leaving + * the key in place ships a Codex-private field to a gateway that rejects unknown fields, which is + * the failure this layer exists to prevent, and this layer's own response restoration is what put + * the key on the item. + */ +function rewriteNamedSelector( + value: unknown, + plan: NamespaceRewritePlan, + bareFallback: boolean, +): unknown { if (!isPlainObject(value) || typeof value.name !== "string") return value; - const explicitNamespace = typeof value.namespace === "string" ? value.namespace : undefined; - const wireName = explicitNamespace - ? plan.identities.get(namespaceIdentity(explicitNamespace, value.name)) - : plan.selectors.get(value.name) ?? undefined; - if (!wireName) return value; - const { namespace: _namespace, ...rest } = value; - return wireName === value.name && explicitNamespace === undefined - ? value - : { ...rest, name: wireName }; + if (typeof value.namespace !== "string") { + if (!bareFallback) return value; + const wireName = plan.selectors.get(value.name) ?? undefined; + return wireName === undefined || wireName === value.name ? value : { ...value, name: wireName }; + } + const { namespace, ...rest } = value; + const wireName = plan.identities.get(loweredIdentity(namespace, value.name)) + ?? loweredWireName(namespace, value.name); + return { ...rest, name: wireName }; } function rewriteToolChoice(value: unknown, plan: NamespaceRewritePlan): unknown { if (!isPlainObject(value)) return value; if ((value.type === "function" || value.type === "custom") && typeof value.name === "string") { - return rewriteNamedSelector(value, plan); + return rewriteNamedSelector(value, plan, true); } if (value.type !== "allowed_tools" || !Array.isArray(value.tools)) return value; let changed = false; const tools = value.tools.map(tool => { - if ( - !isPlainObject(tool) - || (tool.type !== "function" && tool.type !== "custom") - || typeof tool.name !== "string" - ) return tool; - const rewritten = rewriteNamedSelector(tool, plan); + if (!isPlainObject(tool) || typeof tool.name !== "string") return tool; + const rewritten = rewriteNamedSelector(tool, plan, true); changed ||= rewritten !== tool; return rewritten; }); return changed ? { ...value, tools } : value; } -function rewriteInputItem(item: unknown, plan: NamespaceRewritePlan): unknown { +function rewriteInputItem(item: unknown, plan: NamespaceRewritePlan, emitted: Set): unknown { if (!isPlainObject(item)) return item; if (item.type === "additional_tools" && Array.isArray(item.tools)) { - const rewritten = rewriteToolList(item.tools, plan); - return rewritten.changed ? { ...item, tools: rewritten.tools } : item; + const tools = rewriteToolList(item.tools, plan, emitted); + return tools === item.tools ? item : { ...item, tools }; } if ( (item.type === "function_call" || item.type === "custom_tool_call") && typeof item.name === "string" - ) return rewriteNamedSelector(item, plan); + ) return rewriteNamedSelector(item, plan, false); return item; } @@ -201,13 +252,23 @@ export function rewriteRoutedNamespaceToolsForUpstream(body: unknown): { if (!isPlainObject(body)) return { body, aliases: new Map() }; const groups = collectResponsesToolGroups(body); const plan = buildRewritePlan(groups); - if (plan.identities.size === 0) return { body, aliases: plan.aliases }; - let tools = body.tools; - if (Array.isArray(body.tools)) tools = rewriteToolList(body.tools, plan).tools; + // Deliberately not gated on the plan being non-empty: a turn whose catalog is gone still replays + // call items carrying a private `namespace`, and the routed compaction turn strips the whole tool + // surface before this runs. + const emitted = new Set(); + const tools = Array.isArray(body.tools) ? rewriteToolList(body.tools, plan, emitted) : body.tools; let input = body.input; - if (Array.isArray(body.input)) input = body.input.map(item => rewriteInputItem(item, plan)); + if (Array.isArray(body.input)) { + let inputChanged = false; + const rewrittenInput = body.input.map(item => { + const next = rewriteInputItem(item, plan, emitted); + if (next !== item) inputChanged = true; + return next; + }); + if (inputChanged) input = rewrittenInput; + } const toolChoice = rewriteToolChoice(body.tool_choice, plan); return { diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 830ed32704..a0d3e7da74 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -277,6 +277,7 @@ import { restoreRoutedToolSearchCallsInJson } from "../../responses/tool-search- import { createRoutedToolSearchRestoreBlockRewrite } from "../responses-tool-search-repair"; import { createRoutedNamespaceCallRestoreRewrite, + NamespaceToolCollisionError, restoreRoutedNamespaceCallsInJson, type RoutedNamespaceToolAliases, } from "../../responses/namespace-tool-compat"; @@ -2533,6 +2534,13 @@ async function handleResponsesInner( request = await adapter.buildRequest(parsed, { headers: selectedForwardHeaders, translatorBudget }); } catch (error) { releaseCodexAuthContextProbeLease(authCtx); + // A tool catalog this proxy cannot lower onto one wire namespace is a client input error, and + // the rotation-rebuild and bridged paths already answer 400 for the identical throw. Rethrowing + // it here escaped every catch up to the Bun handler, so the same request produced an + // unstructured 500 — and no request log — depending only on whether a rotation ran first. + if (error instanceof NamespaceToolCollisionError) { + return formatErrorResponse(400, "invalid_request_error", redactSecretString(error.message)); + } throw error; } if (route.provider.authMode !== "forward") { diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index 7404311224..7a4b36830d 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -45,7 +45,24 @@ Responses-compatible streaming output. - 검토한 주요 대안: Move Grok back to Chat; special-case only xAI or the reserved `functions` group; flatten every complete namespace on noncanonical Responses and restore request-authorized aliases on return. - 선택한 방식: Noncanonical Responses lowers `functions` children to their bare top-level names and every other complete namespace to collision-checked `__` aliases after custom/tool-search conversion. It rewrites matching replay calls and tool selectors, records the aliases on the built request, and restores only those aliases in JSON/SSE call items before custom/tool-search lifecycle repair. Canonical OpenAI forward preserves native namespace shapes. - 다른 대안 대신 이 방식을 선택한 이유: A transport regression should not discard Responses streaming or create a provider-specific fork, and restoration without request-local authorization could reinterpret an unrelated upstream function as a client namespace call. -- 장점, 단점 및 영향: Grok and other public-schema Responses gateways accept current Codex catalogs while Codex still receives explicit namespace routing. Ambiguous wire names fail closed; empty, malformed, and future nested namespace shapes remain untouched rather than losing tools silently. +- 장점, 단점 및 영향: Grok and other public-schema Responses gateways accept current Codex catalogs while Codex still receives explicit namespace routing. No `type: "namespace"` value survives the boundary: a group the layer cannot express — empty, nested, or with an unusable child name — is dropped along with the children it cannot represent, because relaying the private shape costs the whole request rather than one tool. Genuinely ambiguous wire names still fail closed, now as a 400 rather than an unstructured 500. + +Two coordinates that lower to the same wire name are treated as one tool when they denote one: +`buildTools` flattens the reserved `functions` group without a namespace, so a bare declaration and +a `functions` child of the same name are the duplicate the parser already tolerates — and the one +`promoteClientLoadedTools` produces. The declaration is emitted once instead of failing the request. + +Replayed call items are lowered whether or not this turn declares the group they name. A routed +compaction turn strips the whole tool surface before the boundary runs, and a catalog can change +mid-session, but the client is still replaying items this layer's own response restoration stamped +with a private `namespace`. Only `tool_choice` resolves a bare name through the catalog: a history +item records which tool actually ran, so re-pointing it at a same-named namespace child would +rewrite that record on a coincidence rather than translate it. + +Codex-private tool fields are removed at the same boundary from one table +(`CANONICAL_ONLY_TOOL_FIELDS`) rather than one bespoke pass each: `external_web_access` on either +web-search variant, and `defer_loading` on any declaration, which `activateDeferredTool` clears only +for tools a `tool_search_output` already loaded. A new private bit is a row there. The same noncanonical boundary strips ChatGPT's private `external_web_access` bit from routed `web_search` declarations. The public tool remains enabled and all other options remain intact; @@ -236,10 +253,11 @@ items restore `{ namespace: "image_gen", name: "" }` so Codex can di extension. When item-id repair is also enabled, both transforms compose in one SSE parse/stringify pass (`src/server/sse-payload-rewrite.ts`) rather than chaining separate JS pull wrappers. Inspection and continuation-cache branches keep the raw upstream alias, allowing stored -replays to return upstream without leaking a client-only namespace shape. Malformed and empty -image-gen namespaces remain untouched; complete unrelated namespaces follow the general -noncanonical namespace compatibility rule above. ChatGPT forward mode preserves the private -namespace and hosted tool because that backend understands their native semantics. +replays to return upstream without leaking a client-only namespace shape. The image-gen layer itself +leaves malformed and empty image-gen namespaces untouched, but on a noncanonical route the general +namespace boundary above runs after it and lowers whatever remains, so no private group reaches the +wire. ChatGPT forward mode preserves the private namespace and hosted tool because that backend +understands their native semantics. Per-model `modelReasoningSummaryDelivery` is a narrow compatibility layer for `openai-responses` gateways whose summary capability is real but whose accepted delivery enum diff --git a/tests/namespace-tool-compat.test.ts b/tests/namespace-tool-compat.test.ts index 1801fb3071..81ffbef5ff 100644 --- a/tests/namespace-tool-compat.test.ts +++ b/tests/namespace-tool-compat.test.ts @@ -113,14 +113,96 @@ describe("Responses namespace tool compatibility", () => { })).toThrow('namespace tool wire-name collision for "workspace__read"'); }); - test("preserves empty and malformed namespaces instead of dropping capabilities", () => { - const body = { + // Relaying `type: "namespace"` is what the strict gateway rejects, and it rejects the request + // rather than the tool — so a group this layer cannot represent costs every tool in the turn. + // Dropping what cannot be expressed costs only that. + test("lowers every namespace group rather than relaying the private shape", () => { + const body = rewriteRoutedNamespaceToolsForUpstream({ tools: [ { type: "namespace", name: "empty", tools: [] }, - { type: "namespace", name: "nested", tools: [{ type: "namespace", name: "child", tools: [] }] }, + { + type: "namespace", + name: "partial", + tools: [ + { type: "namespace", name: "nested", tools: [] }, + { type: "function", name: "", parameters: {} }, + { type: "function", name: "ok", parameters: {} }, + ], + }, ], - }; - expect(rewriteRoutedNamespaceToolsForUpstream(body).body).toEqual(body); + }).body as { tools: Array> }; + + expect(body.tools).toEqual([{ type: "function", name: "partial__ok", parameters: {} }]); + expect(body.tools.some(tool => tool.type === "namespace")).toBe(false); + }); + + // The identity key joins namespace and name with NUL, so a name carrying one could otherwise + // forge another tool's identity and silently take over its wire name. + test("drops children whose names cannot become a wire name", () => { + const NUL = String.fromCharCode(0); + const body = rewriteRoutedNamespaceToolsForUpstream({ + tools: [ + { type: "namespace", name: "a", tools: [{ type: "function", name: `b${NUL}c` }] }, + { type: "namespace", name: `a${NUL}b`, tools: [{ type: "function", name: "c" }] }, + { type: "namespace", name: "ok", tools: [{ type: "function", name: "run" }] }, + ], + }).body as { tools: Array> }; + + expect(body.tools).toEqual([{ type: "function", name: "ok__run" }]); + }); + + // `buildTools` flattens the reserved group without a namespace, so the parser treats these as one + // logical tool and tolerates the duplicate; `promoteClientLoadedTools` produces exactly this shape. + test("treats a bare declaration and a functions child of the same name as one tool", () => { + const rewritten = rewriteRoutedNamespaceToolsForUpstream({ + tools: [ + { type: "function", name: "exec", parameters: {} }, + { type: "namespace", name: "functions", tools: [{ type: "function", name: "exec", parameters: {} }] }, + ], + }); + const body = rewritten.body as { tools: Array> }; + + expect(body.tools).toEqual([{ type: "function", name: "exec", parameters: {} }]); + expect([...rewritten.aliases]).toEqual([]); + }); + + // The routed compaction turn strips the whole tool surface before this runs, and a catalog can + // change mid-session — but the client is still replaying items this layer's own restoration + // stamped with a private `namespace`. + test("lowers replayed calls even when this turn declares no namespace", () => { + const body = rewriteRoutedNamespaceToolsForUpstream({ + input: [ + { type: "function_call", namespace: "collaboration", name: "spawn_agent", call_id: "c1", arguments: "{}" }, + { type: "custom_tool_call", namespace: "functions", name: "exec", call_id: "c2", input: "run" }, + ], + }).body as { input: Array> }; + + expect(body.input[0]).toEqual({ + type: "function_call", + name: "collaboration__spawn_agent", + call_id: "c1", + arguments: "{}", + }); + expect(body.input[1]).toEqual({ + type: "custom_tool_call", + name: "exec", + call_id: "c2", + input: "run", + }); + expect(JSON.stringify(body)).not.toContain("namespace"); + }); + + // A history item records which tool actually ran. Resolving its bare name through a same-named + // namespace child would rewrite that record on a coincidence rather than translate it. + test("does not re-point a replayed bare-named call at a namespace child", () => { + const body = rewriteRoutedNamespaceToolsForUpstream({ + tools: [{ type: "namespace", name: "workspace", tools: [{ type: "function", name: "read" }] }], + input: [{ type: "function_call", name: "read", call_id: "c1", arguments: "{}" }], + tool_choice: { type: "function", name: "read" }, + }).body as { input: Array>; tool_choice: { name: string } }; + + expect(body.input[0].name).toBe("read"); + expect(body.tool_choice.name).toBe("workspace__read"); }); test("restores only aliases authorized by this request in JSON and SSE payloads", () => { diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index 39ba0faec5..afbc692017 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -870,6 +870,48 @@ describe("OpenAI Responses passthrough sanitization", () => { expect(body.tools).toEqual([{ type: "web_search", external_web_access: true }]); }); + // `activateDeferredTool` clears `defer_loading` only for tools a `tool_search_output` already + // loaded, so the first turn of a deferred catalog — and any child promoted out of a namespace + // group — otherwise carries the private field to a gateway that rejects unknown arguments. + test("drops Codex-private tool fields from routed declarations", () => { + const adapter = createResponsesPassthroughAdapter({ + adapter: "openai-responses", + baseUrl: "https://api.x.ai/v1", + authMode: "key" as const, + apiKey: "xai-test", + }); + const request = adapter.buildRequest({ + modelId: "grok-4.6", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "grok-4.6", + tools: [ + { type: "web_search_preview", external_web_access: true }, + { + type: "namespace", + name: "workspace", + tools: [{ type: "function", name: "read", defer_loading: true, parameters: {} }], + }, + ], + input: [{ + type: "additional_tools", + tools: [{ type: "function", name: "loose", defer_loading: true, parameters: {} }], + }], + }, + }, { headers: new Headers() }); + const body = JSON.parse(request.body) as { + tools: Record[]; + input: Array<{ tools: Record[] }>; + }; + + expect(body.tools[0]).toEqual({ type: "web_search_preview" }); + expect(body.tools[1]).toMatchObject({ type: "function", name: "workspace__read" }); + expect(body.tools[1]).not.toHaveProperty("defer_loading"); + expect(body.input[0].tools[0]).not.toHaveProperty("defer_loading"); + }); + test("preserves prompt_cache_key in the raw Responses passthrough body", () => { const adapter = createResponsesPassthroughAdapter(provider); const request = adapter.buildRequest({ @@ -1542,7 +1584,7 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(JSON.parse(secondRequest.body)).toEqual(firstBody); }); - test("keyed platform flattens complete namespaces and preserves malformed ones", () => { + test("keyed platform flattens complete namespaces and drops ones it cannot express", () => { const adapter = createResponsesPassthroughAdapter(keyedProvider); const request = adapter.buildRequest({ modelId: "gpt-5.6-sol", @@ -1569,8 +1611,9 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { tool_choice: { type: string; name: string }; }; + // The empty `image_gen` group declares nothing, and relaying `type: "namespace"` is the shape a + // strict gateway rejects for the whole request. expect(body.tools).toEqual([ - { type: "namespace", name: "image_gen", tools: [] }, { type: "function", name: "web__run", @@ -1948,10 +1991,8 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { }, meta); const body = JSON.parse(request.body) as { tools: Array> }; - expect(body.tools).toEqual([ - { type: "namespace", name: "image_gen", tools: [] }, - { type: "image_generation" }, - ]); + // The empty namespace group is lowered away; only the hosted tool reaches the wire. + expect(body.tools).toEqual([{ type: "image_generation" }]); }); test("hosted-tool preference uses the exact model id", () => { @@ -1975,10 +2016,8 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { }, meta); const body = JSON.parse(request.body) as { tools: Array> }; - expect(body.tools).toEqual([ - { type: "namespace", name: "image_gen", tools: [] }, - { type: "image_generation" }, - ]); + // The empty namespace group is lowered away; only the hosted tool reaches the wire. + expect(body.tools).toEqual([{ type: "image_generation" }]); }); test("hosted-tool preference honors an OpenAI virtual model's selected id", () => { From 6bede374013b6bab8ea6022d7cc1a6999e35c8fa Mon Sep 17 00:00:00 2001 From: olddonkey Date: Thu, 20 Aug 2026 18:49:15 -0700 Subject: [PATCH 4/4] fix(responses): make namespace dedup order-independent and restore custom calls by wire identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found two defects in the flattening layer; both are fixed here. Deduplication depended on declaration order. A bare declaration and a `functions` child of the same name are one logical tool, but which one owned the wire name — and therefore which one was emitted — followed whichever container the rewrite reached first. The plan now records the bare wire names from the complete catalog and the bare declaration always wins, so the same catalog flattens identically whichever container declares it. Custom-call restoration used the wrong coordinate. A custom tool inside a non-`functions` namespace is lowered twice on the way out (custom to function, then renamed to `__`), while on the way back namespace restore runs first and replaces the wire name with the bare one. Custom restore then matched that bare name and could convert an unrelated same-named function call, sending Codex a `custom_tool_call` with the wrong payload shape. Converted custom tools are now tracked by their final upstream wire name, and restoration reconstructs that identity from the `{namespace, name}` an earlier rewrite restored. A namespaced custom and a namespaced function sharing a child name now round-trip to their own item types, on both the JSON and SSE paths. Co-Authored-By: Claude Fable 5 --- src/adapters/base.ts | 2 +- src/responses/custom-tool-compat.ts | 75 +++++++- src/responses/namespace-tool-compat.ts | 21 ++- src/server/responses-custom-tool-repair.ts | 4 +- src/server/responses/core.ts | 5 +- tests/namespace-tool-compat.test.ts | 32 ++++ tests/openai-responses-passthrough.test.ts | 193 ++++++++++++++++++++- 7 files changed, 320 insertions(+), 12 deletions(-) diff --git a/src/adapters/base.ts b/src/adapters/base.ts index 45660e478c..3f3f3d06b6 100644 --- a/src/adapters/base.ts +++ b/src/adapters/base.ts @@ -68,7 +68,7 @@ export interface AdapterRequest { method: string; headers: Record; body: string; - /** Custom-tool names actually lowered to upstream function calls while building this request. */ + /** Final upstream wire names of custom tools lowered to functions while building this request. */ convertedRoutedCustomToolNames?: ReadonlySet; /** Client tool-search names actually lowered to upstream function calls for this request. */ convertedRoutedToolSearchNames?: ReadonlySet; diff --git a/src/responses/custom-tool-compat.ts b/src/responses/custom-tool-compat.ts index 83ddc1be19..e7db3c32a6 100644 --- a/src/responses/custom-tool-compat.ts +++ b/src/responses/custom-tool-compat.ts @@ -1,4 +1,8 @@ +import { namespacedToolName } from "../types"; +import { collectResponsesToolGroups } from "./tool-groups"; + const ROUTED_CUSTOM_TOOL_PASSTHROUGH = new Set(["apply_patch"]); +const BUILTIN_FUNCTIONS_NAMESPACE = "functions"; function isPlainObject(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); @@ -13,6 +17,65 @@ function customToolInput(argumentsText: unknown): string { return argumentsText; } +function customToolWireName(namespace: string | undefined, name: string): string { + return namespace === BUILTIN_FUNCTIONS_NAMESPACE ? name : namespacedToolName(namespace, name); +} + +/** Final upstream identity of a call, including a namespace restored by an earlier rewrite. */ +export function routedCustomToolWireName(value: unknown): string | undefined { + if (!isPlainObject(value) || typeof value.name !== "string") return undefined; + return customToolWireName( + typeof value.namespace === "string" ? value.namespace : undefined, + value.name, + ); +} + +/** + * Names of converted custom declarations after namespace lowering. Restoration uses these exact + * wire identities so same-named function and custom children in different namespaces stay distinct. + */ +function collectRoutedCustomToolWireNames(body: unknown): Set { + const names = new Set(); + const groups = collectResponsesToolGroups(body); + const bareWireNames = new Set(); + for (const group of groups) { + for (const tool of group) { + if ( + isPlainObject(tool) + && tool.type !== "namespace" + && typeof tool.name === "string" + ) bareWireNames.add(tool.name); + } + } + + for (const group of groups) { + for (const tool of group) { + if (!isPlainObject(tool)) continue; + if ( + tool.type === "custom" + && typeof tool.name === "string" + && !ROUTED_CUSTOM_TOOL_PASSTHROUGH.has(tool.name) + ) { + names.add(tool.name); + continue; + } + if (tool.type !== "namespace" || typeof tool.name !== "string" || !Array.isArray(tool.tools)) { + continue; + } + for (const child of tool.tools) { + if ( + isPlainObject(child) + && child.type === "custom" + && typeof child.name === "string" + && !ROUTED_CUSTOM_TOOL_PASSTHROUGH.has(child.name) + && !(tool.name === BUILTIN_FUNCTIONS_NAMESPACE && bareWireNames.has(child.name)) + ) names.add(customToolWireName(tool.name, child.name)); + } + } + } + return names; +} + export function customToolItemId(id: unknown): unknown { if (typeof id !== "string") return id; return id.startsWith("fc_") ? `ctc_${id.slice(3)}` : id; @@ -125,11 +188,12 @@ export function rewriteRoutedCustomToolsForUpstream(body: unknown): { body: unknown; names: Set; } { - const names = collectRoutedCustomToolNames(body); - if (names.size === 0) return { body, names }; + const conversionNames = collectRoutedCustomToolNames(body); + const names = collectRoutedCustomToolWireNames(body); + if (conversionNames.size === 0) return { body, names }; const callIds = new Set(); - collectConvertedCallIds(body, names, callIds); - return { body: rewriteForUpstream(body, names, callIds), names }; + collectConvertedCallIds(body, conversionNames, callIds); + return { body: rewriteForUpstream(body, conversionNames, callIds), names }; } export function restoreRoutedCustomCalls( @@ -155,7 +219,8 @@ export function restoreRoutedCustomCalls( changed ||= result.changed; } - if (value.type === "function_call" && typeof value.name === "string" && names.has(value.name)) { + const wireName = routedCustomToolWireName(value); + if (value.type === "function_call" && wireName !== undefined && names.has(wireName)) { restored.type = "custom_tool_call"; restored.id = customToolItemId(value.id); restored.input = customToolInput(value.arguments); diff --git a/src/responses/namespace-tool-compat.ts b/src/responses/namespace-tool-compat.ts index 8a52251da5..3f6cd42ea2 100644 --- a/src/responses/namespace-tool-compat.ts +++ b/src/responses/namespace-tool-compat.ts @@ -90,6 +90,7 @@ function addSelector( type NamespaceRewritePlan = { aliases: Map; + bareWireNames: Set; identities: Map; selectors: Map; }; @@ -99,6 +100,7 @@ export class NamespaceToolCollisionError extends Error {} function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { const aliases = new Map(); + const bareWireNames = new Set(); const identities = new Map(); const selectors = new Map(); const wireOwners = new Map(); @@ -110,6 +112,7 @@ function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { // declaring the same tool both ways is the duplicate the parser already tolerates, not a // collision, and `promoteClientLoadedTools` produces exactly that shape. wireOwners.set(tool.name, loweredIdentity(BUILTIN_FUNCTIONS_NAMESPACE, tool.name)); + bareWireNames.add(tool.name); addSelector(selectors, tool.name, tool.name); } } @@ -141,7 +144,7 @@ function buildRewritePlan(groups: readonly unknown[][]): NamespaceRewritePlan { } } - return { aliases, identities, selectors }; + return { aliases, bareWireNames, identities, selectors }; } /** @@ -165,13 +168,25 @@ function rewriteToolList( if (!parsed) continue; for (const child of parsed.children) { const wireName = plan.identities.get(loweredIdentity(parsed.namespace, child.name as string)); - if (wireName === undefined || emitted.has(wireName)) continue; + // A bare declaration is the canonical representation of a `functions` child. Decide that + // from the complete catalog rather than whichever container happens to be rewritten first. + if ( + wireName === undefined + || (parsed.namespace === BUILTIN_FUNCTIONS_NAMESPACE && plan.bareWireNames.has(wireName)) + || emitted.has(wireName) + ) continue; emitted.add(wireName); rewritten.push(wireName === child.name ? child : { ...child, name: wireName }); } continue; } - if (isPlainObject(tool) && isRepresentableName(tool.name)) emitted.add(tool.name); + if (isPlainObject(tool) && isRepresentableName(tool.name)) { + if (emitted.has(tool.name)) { + changed = true; + continue; + } + emitted.add(tool.name); + } rewritten.push(tool); } return changed ? rewritten : tools; diff --git a/src/server/responses-custom-tool-repair.ts b/src/server/responses-custom-tool-repair.ts index c3c40b134d..1aaa16c73e 100644 --- a/src/server/responses-custom-tool-repair.ts +++ b/src/server/responses-custom-tool-repair.ts @@ -2,6 +2,7 @@ import type { TranslatorBudget } from "../lib/translator-budget"; import { customToolItemId, restoreRoutedCustomCalls, + routedCustomToolWireName, unwrapRoutedCustomToolArguments, } from "../responses/custom-tool-compat"; import { @@ -181,7 +182,8 @@ export function createRoutedCustomToolRestoreBlockRewrite( && typeof parsed.item.name === "string" ) { const upstreamItemId = typeof parsed.item.id === "string" ? parsed.item.id : undefined; - const routed = names.has(parsed.item.name); + const wireName = routedCustomToolWireName(parsed.item); + const routed = wireName !== undefined && names.has(wireName); if (upstreamItemId) { if (routed) { itemNames.set(upstreamItemId, parsed.item.name); diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index a0d3e7da74..f8f881a4e4 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -2545,7 +2545,10 @@ async function handleResponsesInner( } if (route.provider.authMode !== "forward") { for (const name of request.convertedRoutedCustomToolNames ?? []) { - if (toolBridgeMaps.freeformToolNames.has(name)) routedCustomToolNames.add(name); + if ( + toolBridgeMaps.freeformToolNames.has(name) + || toolBridgeMaps.toolNsMap.get(name)?.freeform === true + ) routedCustomToolNames.add(name); } } for (const name of request.convertedRoutedToolSearchNames ?? []) { diff --git a/tests/namespace-tool-compat.test.ts b/tests/namespace-tool-compat.test.ts index 81ffbef5ff..45a4157808 100644 --- a/tests/namespace-tool-compat.test.ts +++ b/tests/namespace-tool-compat.test.ts @@ -166,6 +166,38 @@ describe("Responses namespace tool compatibility", () => { expect([...rewritten.aliases]).toEqual([]); }); + test("chooses the bare declaration regardless of which tool container comes first", () => { + const bare = { + type: "function", + name: "exec", + description: "canonical bare declaration", + parameters: { type: "object", properties: { input: { type: "string" } } }, + }; + const functionsGroup = { + type: "namespace", + name: "functions", + tools: [{ + type: "function", + name: "exec", + description: "namespace duplicate", + parameters: { type: "object", properties: {} }, + }], + }; + const flatten = (bodyTools: unknown[], additionalTools: unknown[]) => { + const rewritten = rewriteRoutedNamespaceToolsForUpstream({ + tools: bodyTools, + input: [{ type: "additional_tools", role: "developer", tools: additionalTools }], + }).body as { + tools: Array>; + input: Array<{ tools: Array> }>; + }; + return [...rewritten.tools, ...rewritten.input[0]!.tools]; + }; + + expect(flatten([bare], [functionsGroup])).toEqual([bare]); + expect(flatten([functionsGroup], [bare])).toEqual([bare]); + }); + // The routed compaction turn strips the whole tool surface before this runs, and a catalog can // change mid-session — but the client is still replaying items this layer's own restoration // stamped with a private `namespace`. diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index afbc692017..a44d7353a1 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -3,8 +3,9 @@ import { createResponsesPassthroughAdapter as createResponsesPassthroughAdapterP import { openaiResponsesUrl } from "../src/adapters/openai-responses-url"; import { enrichProviderFromRegistry, providerConfigSeed } from "../src/providers/derive"; import { getProviderRegistryEntry } from "../src/providers/registry"; -import { sanitizeEncryptedContentInPlace } from "../src/server/responses"; +import { handleResponses, sanitizeEncryptedContentInPlace } from "../src/server/responses"; import { createTranslatorBudget } from "../src/lib/translator-budget"; +import type { OcxConfig } from "../src/types"; import { withTestTranslatorBudget } from "./helpers/translator-budget"; const createResponsesPassthroughAdapter = (...args: Parameters) => @@ -2162,6 +2163,196 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { }); }); +describe("routed namespace and custom-tool identity", () => { + const customNamespace = "custom_catalog"; + const functionNamespace = "function_catalog"; + const rawTools = [ + { + type: "namespace", + name: customNamespace, + tools: [{ + type: "custom", + name: "read", + description: "Read freeform input", + format: { type: "text" }, + }], + }, + { + type: "namespace", + name: functionNamespace, + tools: [{ + type: "function", + name: "read", + description: "Read structured input", + parameters: { type: "object", properties: {} }, + }], + }, + ]; + const customUpstreamItem = { + type: "function_call", + id: "fc_custom_read", + call_id: "call_custom_read", + name: `${customNamespace}__read`, + arguments: JSON.stringify({ input: "freeform payload" }), + status: "completed", + }; + const functionUpstreamItem = { + type: "function_call", + id: "fc_function_read", + call_id: "call_function_read", + name: `${functionNamespace}__read`, + arguments: "{}", + status: "completed", + }; + const config = { + port: 0, + defaultProvider: "fixture", + providers: { + fixture: { + adapter: "openai-responses", + baseUrl: "https://fixture.test/v1", + authMode: "key", + apiKey: "fixture-key", + }, + }, + } as OcxConfig; + + const frame = (event: string, payload: Record): string => + `event: ${event}\ndata: ${JSON.stringify({ type: event, ...payload })}`; + + test("round-trips same-named namespaced custom and function calls through JSON and SSE", async () => { + const adapter = createResponsesPassthroughAdapter(config.providers.fixture!); + const built = adapter.buildRequest({ + modelId: "routed-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { model: "routed-model", input: "read", tools: rawTools }, + }, { headers: new Headers() }); + const builtBody = JSON.parse(built.body) as { tools: Array> }; + + expect(builtBody.tools.map(tool => ({ type: tool.type, name: tool.name }))).toEqual([ + { type: "function", name: `${customNamespace}__read` }, + { type: "function", name: `${functionNamespace}__read` }, + ]); + expect([...(built.convertedRoutedCustomToolNames ?? [])]).toEqual([ + `${customNamespace}__read`, + ]); + + const savedFetch = globalThis.fetch; + const outboundBodies: Array> = []; + globalThis.fetch = (async (_input, init) => { + const outbound = JSON.parse(String(init?.body)) as Record; + outboundBodies.push(outbound); + if (outbound.stream === true) { + const upstream = [ + frame("response.output_item.added", { + output_index: 0, + item: { ...customUpstreamItem, arguments: "", status: "in_progress" }, + }), + frame("response.function_call_arguments.done", { + output_index: 0, + item_id: customUpstreamItem.id, + arguments: customUpstreamItem.arguments, + }), + frame("response.output_item.done", { output_index: 0, item: customUpstreamItem }), + frame("response.output_item.added", { + output_index: 1, + item: { ...functionUpstreamItem, arguments: "", status: "in_progress" }, + }), + frame("response.function_call_arguments.done", { + output_index: 1, + item_id: functionUpstreamItem.id, + arguments: functionUpstreamItem.arguments, + }), + frame("response.output_item.done", { output_index: 1, item: functionUpstreamItem }), + frame("response.completed", { + response: { + id: "resp_stream", + status: "completed", + output: [customUpstreamItem, functionUpstreamItem], + }, + }), + "data: [DONE]", + ].join("\n\n") + "\n\n"; + return new Response(upstream, { headers: { "content-type": "text/event-stream" } }); + } + return new Response(JSON.stringify({ + id: "resp_json", + status: "completed", + output: [customUpstreamItem, functionUpstreamItem], + }), { headers: { "content-type": "application/json" } }); + }) as typeof fetch; + + const requestBody = (stream: boolean) => ({ + model: "fixture/routed-model", + stream, + input: [{ role: "user", content: [{ type: "input_text", text: "read both" }] }], + tools: rawTools, + }); + + try { + const jsonResponse = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(requestBody(false)), + }), config, { model: "", provider: "" }); + const json = await jsonResponse.json() as { output: Array> }; + expect(json.output[0]).toMatchObject({ + type: "custom_tool_call", + namespace: customNamespace, + name: "read", + input: "freeform payload", + }); + expect(json.output[0]).not.toHaveProperty("arguments"); + expect(json.output[1]).toMatchObject({ + type: "function_call", + namespace: functionNamespace, + name: "read", + arguments: "{}", + }); + + const sseResponse = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(requestBody(true)), + }), config, { model: "", provider: "" }); + const clientSse = await sseResponse.text(); + const payloads = clientSse + .split(/\r?\n/) + .filter(line => line.startsWith("data:") && line.slice(5).trim() !== "[DONE]") + .map(line => JSON.parse(line.slice(5).trim()) as Record); + const completed = payloads.find(payload => payload.type === "response.completed") as { + response: { output: Array> }; + } | undefined; + expect(completed?.response.output[0]).toMatchObject({ + type: "custom_tool_call", + namespace: customNamespace, + name: "read", + input: "freeform payload", + }); + expect(completed?.response.output[1]).toMatchObject({ + type: "function_call", + namespace: functionNamespace, + name: "read", + arguments: "{}", + }); + expect(payloads.some(payload => payload.type === "response.custom_tool_call_input.done")).toBe(true); + expect(payloads.some(payload => payload.type === "response.function_call_arguments.done")).toBe(true); + + for (const outbound of outboundBodies) { + const tools = outbound.tools as Array>; + expect(tools.map(tool => ({ type: tool.type, name: tool.name }))).toEqual([ + { type: "function", name: `${customNamespace}__read` }, + { type: "function", name: `${functionNamespace}__read` }, + ]); + } + } finally { + globalThis.fetch = savedFetch; + } + }); +}); + describe("OpenAI Responses forward-mode unsupported param stripping", () => { const meta = { headers: new Headers({ authorization: "Bearer token" }) }; const rawBody = {