Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/adapters/openai-chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)) {
Expand All @@ -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;
Expand Down Expand Up @@ -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));
}
Expand Down
21 changes: 21 additions & 0 deletions tests/openai-chat-hardening.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
16 changes: 15 additions & 1 deletion tests/openai-chat-parallel-stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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([]);
Expand Down
Loading