diff --git a/src/bridge.ts b/src/bridge.ts index 82c73a0bee..b5fecc2893 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -6,7 +6,7 @@ import type { OcxReasoningReplayScopeRef, OcxUsage, } from "./types"; -import { coerceIntegerToolArguments } from "./lib/tool-argument-integers"; +import { coerceIntegerToolArguments, lookupToolParameterSchema } from "./lib/tool-argument-integers"; import { adapterFailureFromMessage, classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode, type OcxErrorPayload } from "./lib/errors"; import { encodeCompactionSummary } from "./responses/compaction"; import { isTruncatedStopReason, truncationReasonFor } from "./responses/truncated-stop-reason"; @@ -625,7 +625,7 @@ export function bridgeToResponsesSSE( // against the declared schema; a non-integral value stays an error. const argsStr = coerceIntegerToolArguments( currentToolCall.args || "{}", - options?.toolParameterSchemas?.get(currentToolCall.name), + lookupToolParameterSchema(options?.toolParameterSchemas, currentToolCall.name), ); // Finalize streamed function-call arguments so Codex commits the call (incl. MCP / computer_use). if (!currentToolCall.freeform && !currentToolCall.toolSearch) { @@ -1655,7 +1655,7 @@ function buildResponseJSONWithBudget( // the request declared, which is the pre-namespace-mapping `currentToolCallName`. const coercedArgs = coerceIntegerToolArguments( currentToolCallArgs, - options?.toolParameterSchemas?.get(currentToolCallName), + lookupToolParameterSchema(options?.toolParameterSchemas, currentToolCallName), ); // Freeform tools serialize as custom_tool_call without extra_content; remember the // signature server-side regardless so the replayed call can be re-signed (#1735). diff --git a/src/lib/tool-argument-integers.ts b/src/lib/tool-argument-integers.ts index 19457c535f..6745757e7f 100644 --- a/src/lib/tool-argument-integers.ts +++ b/src/lib/tool-argument-integers.ts @@ -36,6 +36,44 @@ // // Anything without a declared `integer`/`string` type is never touched. +/** + * Known Codex native tool-call property names that Rust deserializes as integers (e.g. u64, usize), + * even when the tool parameter schema declares `type: "number"`, `type: ["number", "null"]`, or is unresolvable. + * (Issue #2316: Grok serializes wait_agent timeout_ms as 120000.0, which Codex u64 rejects). + */ +export const CODEX_NATIVE_INTEGER_PROPERTY_NAMES = new Set([ + "timeout_ms", + "yield_time_ms", + "max_tokens", + "max_output_tokens", + "session_id", + "line", + "start", + "end", + "priority", + "port", +]); + +export function lookupToolParameterSchema( + toolParameterSchemas: ReadonlyMap> | undefined, + toolName: string | undefined, +): Record | undefined { + if (!toolParameterSchemas || !toolName) return undefined; + const direct = toolParameterSchemas.get(toolName); + if (direct) return direct; + const idx = toolName.indexOf("__"); + if (idx !== -1) { + const bare = toolName.slice(idx + 2); + const bareMatch = toolParameterSchemas.get(bare); + if (bareMatch) return bareMatch; + } else { + for (const [key, schema] of toolParameterSchemas.entries()) { + if (key.endsWith(`__${toolName}`)) return schema; + } + } + return undefined; +} + /** JSON Schema subset we need; provider tool schemas are untrusted input. */ type SchemaNode = Record; @@ -118,15 +156,27 @@ interface CoerceResult { changed: boolean; } -function coerceValue(value: unknown, schema: SchemaNode | undefined, root: SchemaNode, depth: number): CoerceResult { +function coerceValue( + value: unknown, + schema: SchemaNode | undefined, + root: SchemaNode, + depth: number, + propertyName?: string, +): CoerceResult { // A hostile or deeply nested schema must not blow the stack. if (depth > 64) return { value, changed: false }; const resolved = schema ? resolveRef(schema, root, new Set()) : undefined; if (typeof value === "number") { - if (!resolved) return { value, changed: false }; + if (!resolved) { + if (propertyName && CODEX_NATIVE_INTEGER_PROPERTY_NAMES.has(propertyName) && safelyIntegral(value)) { + return { value, changed: true }; + } + return { value, changed: false }; + } const branches = compositionBranches(resolved); - const integerDeclared = declaresInteger(resolved) || branches.some(declaresInteger); + const integerDeclared = declaresInteger(resolved) || branches.some(declaresInteger) || + (propertyName !== undefined && CODEX_NATIVE_INTEGER_PROPERTY_NAMES.has(propertyName) && (declaresNumeric(resolved) || branches.some(declaresNumeric))); if (!integerDeclared && safelyIntegral(value)) { // Issue #1938: a bare integer in a string-only field has exactly one faithful // string reading. A field that also accepts a numeric type keeps the number. @@ -148,7 +198,7 @@ function coerceValue(value: unknown, schema: SchemaNode | undefined, root: Schem const itemSchema = resolved ? asSchema(resolved.items) : undefined; let changed = false; const next = value.map(entry => { - const result = coerceValue(entry, itemSchema, root, depth + 1); + const result = coerceValue(entry, itemSchema, root, depth + 1, propertyName); if (result.changed) changed = true; return result.value; }); @@ -164,7 +214,7 @@ function coerceValue(value: unknown, schema: SchemaNode | undefined, root: Schem const next: Record = {}; for (const [key, entry] of Object.entries(object)) { const childSchema = asSchema(properties?.[key]) ?? additional; - const result = coerceValue(entry, childSchema, root, depth + 1); + const result = coerceValue(entry, childSchema, root, depth + 1, key); if (result.changed) changed = true; next[key] = result.value; } @@ -183,7 +233,7 @@ export function coerceIntegerToolArguments( args: string, parameters: Record | undefined, ): string { - if (!parameters || !args) return args; + if (!args) return args; // Cheap reject: a payload with no digit cannot need either repair (integral-float // -> integer, or bare-integer -> string). if (!/\d/.test(args)) return args; @@ -195,8 +245,8 @@ export function coerceIntegerToolArguments( // existing paths already handle them. return args; } - const root = parameters as SchemaNode; - const result = coerceValue(parsed, root, root, 0); + const root = (parameters ?? {}) as SchemaNode; + const result = coerceValue(parsed, parameters ? root : undefined, root, 0); if (!result.changed) return args; return JSON.stringify(result.value); } diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index 7c671a7de4..1a73fd7e7a 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -26,7 +26,7 @@ import { } from "../../combos"; import { isInjectionDebugEnabled } from "../../lib/debug-settings"; import { injectionDebugLog } from "../../lib/injection-debug-log"; -import { modelInList, namespacedToolName, toolChoiceToolPredicate } from "../../types"; +import { modelInList, namespacedToolName, toolChoiceAliases, toolChoiceToolPredicate } from "../../types"; import type { AdapterEvent, OcxConfig, OcxParsedRequest, OcxProviderConfig, OcxProviderContinuationState, OcxUsage } from "../../types"; import { forceRefreshOAuthAccessSnapshot, @@ -124,6 +124,12 @@ export function buildToolBridgeMaps(parsed: OcxParsedRequest, budget?: Translato // Retained by reference (the schema is already resident in parsed.context.tools), // so this adds a map entry rather than a copy of every tool's parameters. if (t.parameters && typeof t.parameters === "object") toolParameterSchemas.set(wireName, t.parameters); + if (t.parameters && typeof t.parameters === "object") { + for (const alias of toolChoiceAliases(t)) { + toolParameterSchemas.set(alias, t.parameters); + } + if (!toolParameterSchemas.has(t.name)) toolParameterSchemas.set(t.name, t.parameters); + } if (t.namespace) { budget?.chargeRetained(new TextEncoder().encode(JSON.stringify([wireName, t.namespace, t.name])).byteLength, { kind: "retained_collectors" }); toolNsMap.set(wireName, { namespace: t.namespace, name: t.name, ...(t.freeform ? { freeform: true } : {}) }); diff --git a/tests/tool-argument-integers.test.ts b/tests/tool-argument-integers.test.ts index 196cd74428..4d4fa055c1 100644 --- a/tests/tool-argument-integers.test.ts +++ b/tests/tool-argument-integers.test.ts @@ -259,3 +259,72 @@ describe("bare-integer-for-string tool argument repair (#1938)", () => { }); }); +/** The multi-agent wait shape from the #2316 report: timeout_ms declared number, rejected as u64. */ +const MULTI_AGENT_WAIT_SCHEMA = { + type: "object", + properties: { + timeout_ms: { type: "number" }, + targets: { type: "array", items: { type: "string" } }, + temperature: { type: "number" }, + }, +}; + +describe("Codex native integer field repair for number-declared schemas (#2316)", () => { + test("repairs timeout_ms float emitted by Grok when schema declares type number", () => { + expect(coerceIntegerToolArguments('{"timeout_ms":120000.0}', MULTI_AGENT_WAIT_SCHEMA)) + .toBe('{"timeout_ms":120000}'); + expect(coerceIntegerToolArguments('{"timeout_ms":60000.0}', MULTI_AGENT_WAIT_SCHEMA)) + .toBe('{"timeout_ms":60000}'); + }); + + test("leaves non-integral float in timeout_ms untouched so it fails honestly", () => { + const raw = '{"timeout_ms":1.5}'; + expect(coerceIntegerToolArguments(raw, MULTI_AGENT_WAIT_SCHEMA)).toBe(raw); + }); + + test("still never touches non-integer fields like temperature even when integral float", () => { + const raw = '{"temperature":1.0}'; + expect(coerceIntegerToolArguments(raw, MULTI_AGENT_WAIT_SCHEMA)).toBe(raw); + }); + + test("repairs known native fields even if tool parameter schema is omitted or unresolvable", () => { + expect(coerceIntegerToolArguments('{"timeout_ms":120000.0}', undefined)) + .toBe('{"timeout_ms":120000}'); + expect(coerceIntegerToolArguments('{"yield_time_ms":60000.0}', undefined)) + .toBe('{"yield_time_ms":60000}'); + }); + + test("streaming bridge resolves schema across wire name and bare name aliases (#2316)", async () => { + const toolSchemas = new Map>([ + ["multi_agent_v1__wait_agent", MULTI_AGENT_WAIT_SCHEMA], + ]); + + // Model emits bare name wait_agent while schema was registered under multi_agent_v1__wait_agent + const frames = await collectSse(bridgeToResponsesSSE(replay([ + { type: "tool_call_start", id: "call_wait", name: "wait_agent" }, + { type: "tool_call_delta", arguments: '{"timeout_ms":120000.0,"temperature":1.0}' }, + { type: "tool_call_end", id: "call_wait" }, + { type: "done" }, + ]), "grok-4.6", undefined, undefined, undefined, undefined, 2_000, { toolParameterSchemas: toolSchemas })); + + const done = frames.find(f => f.event === "response.function_call_arguments.done"); + expect(done?.data.arguments).toBe('{"timeout_ms":120000,"temperature":1}'); + }); + + test("non-streaming bridge resolves schema for namespaced call when registered under bare name", () => { + const toolSchemas = new Map>([ + ["wait_agent", MULTI_AGENT_WAIT_SCHEMA], + ]); + + const body = buildResponseJSON([ + { type: "tool_call_start", id: "call_wait", name: "multi_agent_v1__wait_agent" }, + { type: "tool_call_delta", arguments: '{"timeout_ms":60000.0}' }, + { type: "tool_call_end", id: "call_wait" }, + { type: "done" }, + ], "grok-4.6", { toolParameterSchemas: toolSchemas }) as Record; + + const output = body.output as Record[]; + const call = output.find(item => item.type === "function_call"); + expect(call?.arguments).toBe('{"timeout_ms":60000}'); + }); +});