From 933d00974fc86619e50b5fb72a77077bfb90fa78 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Wed, 12 Aug 2026 09:36:35 +0000 Subject: [PATCH 1/3] fix(openai-chat): log redacted invalid tool-call shape --- src/adapters/openai-chat.ts | 67 ++++++++++++++++++++++++++++- tests/openai-chat-hardening.test.ts | 67 ++++++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 99b62af5e5..63d010e041 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -205,6 +205,64 @@ function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } +type InvalidToolCallReason = + | "tool_calls_not_array" + | "tool_call_not_object" + | "tool_call_id_invalid" + | "tool_call_function_not_object" + | "tool_call_function_name_invalid" + | "tool_call_function_arguments_invalid"; + +/** + * 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. + */ +function diagnoseInvalidToolCalls( + rawToolCalls: unknown, + mode: "stream" | "response", +): { reason: InvalidToolCallReason; callIndex?: number; valueType: string } | undefined { + if (!Array.isArray(rawToolCalls)) { + return { reason: "tool_calls_not_array", valueType: rawToolCalls === null ? "null" : typeof rawToolCalls }; + } + for (let callIndex = 0; callIndex < rawToolCalls.length; callIndex++) { + const rawToolCall = rawToolCalls[callIndex]; + if (!isRecord(rawToolCall)) { + return { + reason: "tool_call_not_object", + callIndex, + valueType: rawToolCall === null ? "null" : Array.isArray(rawToolCall) ? "array" : typeof rawToolCall, + }; + } + if (mode === "stream") { + // Streaming currently rejects only the container/member shapes. Describe exactly that + // existing boundary instead of silently tightening compatibility in a diagnostic change. + continue; + } + if (typeof rawToolCall.id !== "string") { + return { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawToolCall.id }; + } + if (!isRecord(rawToolCall.function)) { + return { + reason: "tool_call_function_not_object", + callIndex, + valueType: rawToolCall.function === null ? "null" : Array.isArray(rawToolCall.function) ? "array" : typeof rawToolCall.function, + }; + } + if (typeof rawToolCall.function.name !== "string") { + return { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof rawToolCall.function.name }; + } + if (typeof rawToolCall.function.arguments !== "string") { + return { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof rawToolCall.function.arguments }; + } + } + return undefined; +} + +function logInvalidToolCalls(mode: "stream" | "response", rawToolCalls: unknown): void { + const diagnostic = diagnoseInvalidToolCalls(rawToolCalls, mode); + if (diagnostic) debugProviderDiagnostic("openai-chat", "invalid-tool-calls", { mode, ...diagnostic }); +} + function developerSystemText(message: OcxMessage): string | undefined { if (message.role !== "developer") return undefined; if (typeof message.content === "string") return message.content; @@ -1018,10 +1076,12 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd // through the adapter error channel instead of escaping as TypeError (#1325). Null is // tolerated as absent because OpenAI-compatible providers may emit it as stream padding. if (!Array.isArray(rawToolCalls)) { + logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } for (const rawToolCall of rawToolCalls) { if (!isRecord(rawToolCall)) { + logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } const tc = rawToolCall as { @@ -1206,9 +1266,13 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd if (typeof msg.content === "string") events.push({ type: "text_delta", text: msg.content }); const rawToolCalls = msg.tool_calls; if (rawToolCalls !== undefined && rawToolCalls !== null) { - if (!Array.isArray(rawToolCalls)) return [invalidToolCallsEvent(usage)]; + if (!Array.isArray(rawToolCalls)) { + logInvalidToolCalls("response", rawToolCalls); + return [invalidToolCallsEvent(usage)]; + } for (const rawToolCall of rawToolCalls) { if (!isRecord(rawToolCall) || !isRecord(rawToolCall.function)) { + logInvalidToolCalls("response", rawToolCalls); return [invalidToolCallsEvent(usage)]; } const id = rawToolCall.id; @@ -1219,6 +1283,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd // a whitespace-only function name is not a legitimate tool-call shape either. if (typeof id !== "string" || typeof name !== "string" || typeof args !== "string" || name.trim().length === 0) { + logInvalidToolCalls("response", rawToolCalls); return [invalidToolCallsEvent(usage)]; } events.push({ type: "tool_call_start", id, name }); diff --git a/tests/openai-chat-hardening.test.ts b/tests/openai-chat-hardening.test.ts index b78c3102b8..a87987ca5e 100644 --- a/tests/openai-chat-hardening.test.ts +++ b/tests/openai-chat-hardening.test.ts @@ -1,5 +1,7 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, test } from "bun:test"; import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../src/adapters/openai-chat"; +import { getDebugLogEntries, resetDebugLogBufferForTests } from "../src/lib/debug-log-buffer"; +import { resetDebugSettingsForTests } from "../src/lib/debug-settings"; import { routeModel } from "../src/router"; import type { AdapterEvent, OcxConfig, OcxParsedRequest, OcxProviderConfig } from "../src/types"; import { withTestTranslatorBudget } from "./helpers/translator-budget"; @@ -7,6 +9,15 @@ import { withTestTranslatorBudget } from "./helpers/translator-budget"; const createOpenAIChatAdapter = (...args: Parameters) => withTestTranslatorBudget(createOpenAIChatAdapterProduction(...args)); +const previousDebug = process.env.OCX_DEBUG; + +afterEach(() => { + resetDebugSettingsForTests(); + resetDebugLogBufferForTests(); + if (previousDebug === undefined) delete process.env.OCX_DEBUG; + else process.env.OCX_DEBUG = previousDebug; +}); + function parsed(): OcxParsedRequest { return { modelId: "test-model", @@ -148,6 +159,37 @@ describe("openai-chat non-stream response hardening", () => { }]); } }); + + test("debug mode records only the non-stream tool-call shape failure", async () => { + process.env.OCX_DEBUG = "1"; + const secretArguments = "private-tool-arguments"; + const adapter = createOpenAIChatAdapter(provider()); + const events = await adapter.parseResponse!(new Response(JSON.stringify({ + choices: [{ message: { role: "assistant", tool_calls: [{ + id: "call_1", + function: { name: "tool", arguments: { secretArguments } }, + }] } }], + }))); + + expect(events).toEqual([{ type: "error", message: "upstream response contained invalid tool calls" }]); + const lines = getDebugLogEntries().map(entry => entry.line).join("\n"); + expect(lines).toContain("[ocx:openai-chat:invalid-tool-calls]"); + expect(lines).toContain('"mode":"response"'); + expect(lines).toContain('"reason":"tool_call_function_arguments_invalid"'); + expect(lines).toContain('"valueType":"object"'); + expect(lines).not.toContain(secretArguments); + expect(lines).not.toContain("call_1"); + }); + + test("tool-call structural diagnostics stay disabled by default", async () => { + delete process.env.OCX_DEBUG; + const adapter = createOpenAIChatAdapter(provider()); + await adapter.parseResponse!(new Response(JSON.stringify({ + choices: [{ message: { role: "assistant", tool_calls: { privateArguments: "secret" } } }], + }))); + + expect(getDebugLogEntries()).toHaveLength(0); + }); }); describe("openai-chat stream response hardening", () => { @@ -227,6 +269,29 @@ describe("openai-chat stream response hardening", () => { }]); } }); + + test("debug mode classifies streaming tool-call structure without retaining values", async () => { + process.env.OCX_DEBUG = "1"; + const privateName = "private-tool-name"; + const adapter = createOpenAIChatAdapter(provider()); + const response = new Response([ + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [{ + privateName, + privateArguments: "private arguments", + }, 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"); + expect(lines).toContain('"mode":"stream"'); + expect(lines).toContain('"reason":"tool_call_not_object"'); + expect(lines).toContain('"callIndex":1'); + expect(lines).toContain('"valueType":"null"'); + expect(lines).not.toContain(privateName); + expect(lines).not.toContain("private arguments"); + }); }); describe("openai-chat credential hardening", () => { From d8fe1229810944f4b951276a7f703a61c89ec1b7 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 12 Aug 2026 23:20:49 +0900 Subject: [PATCH 2/3] fix(openai-chat): align the tool-call diagnostic with the current validation Rebase follow-up on top of @Ingwannu's diagnostic commit. #1531 landed between this PR being written and being rebased, and it moved the validation the diagnostic describes. The streamed path now validates function/name/arguments/id at ingest rather than only checking container and member shapes, so the diagnostic no longer skips per-member inspection in stream mode, and the three new ingest rejections log through the same helper. Blank and whitespace-only names are now rejected on the buffered path; reporting that as name_invalid would claim a type problem for a correctly-typed value, so it gets its own reason code. A diagnostic that describes a boundary the code no longer has is worse than none: it would send provider-compatibility work after the wrong shape. --- src/adapters/openai-chat.ts | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 63d010e041..bfa940c256 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -211,6 +211,7 @@ type InvalidToolCallReason = | "tool_call_id_invalid" | "tool_call_function_not_object" | "tool_call_function_name_invalid" + | "tool_call_function_name_blank" | "tool_call_function_arguments_invalid"; /** @@ -234,8 +235,30 @@ function diagnoseInvalidToolCalls( }; } if (mode === "stream") { - // Streaming currently rejects only the container/member shapes. Describe exactly that - // existing boundary instead of silently tightening compatibility in a diagnostic change. + // The streamed path validates the pieces it is about to store (#1531): a present + // `function` must be a record, and a present `name`/`arguments`/`id` must be a string. + // 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. + const streamFunction = (rawToolCall as { function?: unknown }).function; + if (streamFunction !== undefined && streamFunction !== null) { + if (!isRecord(streamFunction)) { + return { + reason: "tool_call_function_not_object", + callIndex, + valueType: Array.isArray(streamFunction) ? "array" : typeof streamFunction, + }; + } + if (streamFunction.name !== undefined && typeof streamFunction.name !== "string") { + return { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof streamFunction.name }; + } + if (streamFunction.arguments !== undefined && typeof streamFunction.arguments !== "string") { + return { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof streamFunction.arguments }; + } + } + if (rawToolCall.id !== undefined && typeof rawToolCall.id !== "string") { + return { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawToolCall.id }; + } continue; } if (typeof rawToolCall.id !== "string") { @@ -251,6 +274,12 @@ function diagnoseInvalidToolCalls( if (typeof rawToolCall.function.name !== "string") { return { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof rawToolCall.function.name }; } + // #1531 also rejects a blank or whitespace-only name on this path, because such a call + // cannot select a dispatch target. Diagnosing it as `name_invalid` would report a type + // problem for a value that is correctly typed, so it gets its own reason code. + if (rawToolCall.function.name.trim().length === 0) { + return { reason: "tool_call_function_name_blank", callIndex, valueType: "string" }; + } if (typeof rawToolCall.function.arguments !== "string") { return { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof rawToolCall.function.arguments }; } @@ -1096,16 +1125,19 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const rawFunction = (rawToolCall as { function?: unknown }).function; if (rawFunction !== undefined && rawFunction !== null) { if (!isRecord(rawFunction)) { + logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } const rawName = rawFunction.name; const rawArguments = rawFunction.arguments; if ((rawName !== undefined && typeof rawName !== "string") || (rawArguments !== undefined && typeof rawArguments !== "string")) { + logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } } if (tc.id !== undefined && typeof tc.id !== "string") { + logInvalidToolCalls("stream", rawToolCalls); return yield* terminateWithError(invalidToolCallsEvent(pendingUsage)); } const key = typeof tc.index === "number" From be101c32b732a160c08df97da76b188c5dd0d030 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 12 Aug 2026 23:37:25 +0900 Subject: [PATCH 3/3] fix(openai-chat): match diagnostic precedence to the validator Review follow-up. The buffered validator checks the function container first, then id/name/arguments types together, and only then the blank name. The diagnostic checked id before the container and blank name before arguments, so a payload carrying two defects at once was reported under the wrong reason. That matters precisely because of what this diagnostic is for: it exists to point provider-compatibility work at the shape that was rejected. A wrong reason code sends that work after the wrong shape. Four tests each carry two defects at once, so only the matching order produces the expected reason. --- src/adapters/openai-chat.ts | 23 ++++++++++------- tests/openai-chat-hardening.test.ts | 40 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index bfa940c256..b79d73fa9c 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -261,9 +261,11 @@ function diagnoseInvalidToolCalls( } continue; } - if (typeof rawToolCall.id !== "string") { - return { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawToolCall.id }; - } + // Precedence must mirror the buffered validator below, or a payload with more than one + // problem is reported under the wrong reason and sends compatibility work after the wrong + // shape. That validator checks the `function` container first (`!isRecord(rawToolCall) || + // !isRecord(rawToolCall.function)`), then id/name/arguments types together, and only then + // the blank name. if (!isRecord(rawToolCall.function)) { return { reason: "tool_call_function_not_object", @@ -271,18 +273,21 @@ function diagnoseInvalidToolCalls( valueType: rawToolCall.function === null ? "null" : Array.isArray(rawToolCall.function) ? "array" : typeof rawToolCall.function, }; } + if (typeof rawToolCall.id !== "string") { + return { reason: "tool_call_id_invalid", callIndex, valueType: typeof rawToolCall.id }; + } if (typeof rawToolCall.function.name !== "string") { return { reason: "tool_call_function_name_invalid", callIndex, valueType: typeof rawToolCall.function.name }; } - // #1531 also rejects a blank or whitespace-only name on this path, because such a call - // cannot select a dispatch target. Diagnosing it as `name_invalid` would report a type - // problem for a value that is correctly typed, so it gets its own reason code. - if (rawToolCall.function.name.trim().length === 0) { - return { reason: "tool_call_function_name_blank", callIndex, valueType: "string" }; - } if (typeof rawToolCall.function.arguments !== "string") { return { reason: "tool_call_function_arguments_invalid", callIndex, valueType: typeof rawToolCall.function.arguments }; } + // Last, matching the validator: #1531 also rejects a blank or whitespace-only name here, + // because such a call cannot select a dispatch target. Reporting it as `name_invalid` + // would claim a type problem for a correctly-typed value, so it gets its own code. + if (rawToolCall.function.name.trim().length === 0) { + return { reason: "tool_call_function_name_blank", callIndex, valueType: "string" }; + } } return undefined; } diff --git a/tests/openai-chat-hardening.test.ts b/tests/openai-chat-hardening.test.ts index a87987ca5e..9a2117f4a4 100644 --- a/tests/openai-chat-hardening.test.ts +++ b/tests/openai-chat-hardening.test.ts @@ -190,6 +190,46 @@ describe("openai-chat non-stream response hardening", () => { expect(getDebugLogEntries()).toHaveLength(0); }); + + // The diagnostic's job is to say WHICH check rejected the payload. If its precedence drifts + // from the validator's, a payload with more than one problem is reported under the wrong + // reason and sends provider-compatibility work after the wrong shape. These cases each carry + // two defects at once, so only the matching order produces the expected reason. + describe("diagnostic precedence matches the buffered validator", () => { + async function reasonFor(toolCall: unknown): Promise { + process.env.OCX_DEBUG = "1"; + const adapter = createOpenAIChatAdapter(provider()); + await adapter.parseResponse!(new Response(JSON.stringify({ + choices: [{ message: { role: "assistant", tool_calls: [toolCall] } }], + }))); + const lines = getDebugLogEntries().map(entry => entry.line).join("\n"); + const match = /"reason":"([a-z_]+)"/.exec(lines); + return match?.[1] ?? ""; + } + + test("a bad function container outranks a bad id", async () => { + // Validator checks `!isRecord(rawToolCall.function)` before it reads `id`. + expect(await reasonFor({ id: 7, function: "not-an-object" })) + .toBe("tool_call_function_not_object"); + }); + + test("a bad id outranks a bad name", async () => { + expect(await reasonFor({ id: 7, function: { name: 9, arguments: "{}" } })) + .toBe("tool_call_id_invalid"); + }); + + test("a bad arguments type outranks a blank name", async () => { + // Both are rejected by the same validator condition; arguments is checked first there, + // so a blank name must not shadow it. + expect(await reasonFor({ id: "call_1", function: { name: " ", arguments: 5 } })) + .toBe("tool_call_function_arguments_invalid"); + }); + + test("a blank name is reported as blank, not as a type problem", async () => { + expect(await reasonFor({ id: "call_1", function: { name: " ", arguments: "{}" } })) + .toBe("tool_call_function_name_blank"); + }); + }); }); describe("openai-chat stream response hardening", () => {