diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 187912afbd..d22659ef4c 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -332,6 +332,16 @@ type InvalidToolCallReason = | "tool_call_function_name_blank" | "tool_call_function_arguments_invalid"; +/** + * Streamed string fields are absent when null or undefined (#1731): OpenAI-compatible + * streamers repeat already-sent `id`/`name`/`arguments` as null on continuation deltas. + * The accumulator and this diagnostic share this predicate so they cannot disagree about + * which delta was the invalid one. + */ +function isInvalidStreamStringField(value: unknown): boolean { + return value != null && typeof value !== "string"; +} + /** * Explain only the rejected wire shape, never its values. This diagnostic exists so provider * compatibility can be tightened from evidence without retaining tool arguments or credentials. @@ -358,6 +368,10 @@ function diagnoseInvalidToolCalls( // Blank names are caught later at flush, not here, so they are not diagnosed on this // branch. Describe exactly that boundary rather than tightening compatibility in a // diagnostic change. + // #1731: "present" means the same thing here as in the accumulator — null and undefined + // are both absent, because some OpenAI-compatible streamers repeat already-sent fields + // as null on continuation deltas. A separate predicate here would diagnose accepted + // padding as the failure and point compatibility work at the wrong delta. const streamFunction = (rawToolCall as { function?: unknown }).function; if (streamFunction !== undefined && streamFunction !== null) { if (!isRecord(streamFunction)) { @@ -367,14 +381,14 @@ function diagnoseInvalidToolCalls( valueType: Array.isArray(streamFunction) ? "array" : typeof streamFunction, }; } - if (streamFunction.name !== undefined && typeof streamFunction.name !== "string") { + if (isInvalidStreamStringField(streamFunction.name)) { return { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof streamFunction.name }; } - if (streamFunction.arguments !== undefined && typeof streamFunction.arguments !== "string") { + if (isInvalidStreamStringField(streamFunction.arguments)) { return { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof streamFunction.arguments }; } } - if (rawToolCall.id !== undefined && typeof rawToolCall.id !== "string") { + if (isInvalidStreamStringField(rawToolCall.id)) { return { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawToolCall.id }; } continue; @@ -1488,13 +1502,15 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd } const rawName = rawFunction.name; const rawArguments = rawFunction.arguments; - if ((rawName !== undefined && typeof rawName !== "string") - || (rawArguments !== undefined && typeof rawArguments !== "string")) { + // Some OpenAI-compatible streamers repeat already-sent fields as null on + // continuation deltas. Treat only null/undefined as absent; every other + // non-string value still fails closed before entering the accumulator. + if (isInvalidStreamStringField(rawName) || isInvalidStreamStringField(rawArguments)) { logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } } - if (tc.id !== undefined && typeof tc.id !== "string") { + if (isInvalidStreamStringField(tc.id)) { logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } diff --git a/tests/openai-chat-hardening.test.ts b/tests/openai-chat-hardening.test.ts index 0492e9575b..b0e1984205 100644 --- a/tests/openai-chat-hardening.test.ts +++ b/tests/openai-chat-hardening.test.ts @@ -332,6 +332,27 @@ describe("openai-chat stream response hardening", () => { expect(lines).not.toContain(privateName); expect(lines).not.toContain("private arguments"); }); + + test("debug mode skips accepted null padding and blames the real malformed delta (#1731)", async () => { + process.env.OCX_DEBUG = "1"; + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [ + { index: 0, id: null, function: { name: null, arguments: null } }, + null, + ] } }] })}\n\n`, + "data: [DONE]\n\n", + ].join("")); + + const events = await collect(adapter.parseStream(response)); + expect(events).toEqual([{ type: "error", message: "upstream response contained invalid tool calls" }]); + const lines = getDebugLogEntries().map(entry => entry.line).join("\n"); + // The null-padded continuation delta at index 0 is accepted by the accumulator, so the + // diagnostic must point at index 1 rather than claiming the padding was the defect. + expect(lines).toContain('"reason":"tool_call_not_object"'); + expect(lines).toContain('"callIndex":1'); + expect(lines).not.toContain('"tool_call_function_name_invalid"'); + }); }); describe("openai-chat credential hardening", () => { diff --git a/tests/openai-chat-parallel-stream.test.ts b/tests/openai-chat-parallel-stream.test.ts index 7c7c9e01b1..53d74f906f 100644 --- a/tests/openai-chat-parallel-stream.test.ts +++ b/tests/openai-chat-parallel-stream.test.ts @@ -97,6 +97,20 @@ describe("openai-chat parallel tool call stream assembly", () => { ]); }); + test("T2b: null-padded continuation fields preserve the earlier tool call", async () => { + const events = await collect(sse([ + chunkOf([{ index: 0, id: "call_null_padding", function: { name: "shell", arguments: "{\"x\":" } }]), + chunkOf([{ index: 0, id: null, function: { name: null, arguments: "1}" } }]), + chunkOf([{ index: 0, id: null, function: { name: null, arguments: null } }]), + chunkOf([], "tool_calls"), + ])); + expect(assembled(events)).toEqual([{ + id: "call_null_padding", + name: "shell", + args: "{\"x\":1}", + }]); + }); + test("T3: whole-chunk multi-call (xAI style) emits both calls", async () => { const events = await collect(sse([ chunkOf([ @@ -147,7 +161,7 @@ describe("openai-chat parallel tool call stream assembly", () => { // a call the Codex tool-call contract cannot dispatch was never a usable outcome (#1514). test("T7: name never arrives - turn fails closed instead of emitting an undispatchable call", async () => { const events = await collect(sse([ - chunkOf([{ index: 0, id: "anon", function: { arguments: "{\"q\":1}" } }]), + chunkOf([{ index: 0, id: "anon", function: { name: null, arguments: "{\"q\":1}" } }]), chunkOf([], "tool_calls"), ])); expect(assembled(events)).toEqual([]);