diff --git a/open-sse/services/reasoningInputPolicy.ts b/open-sse/services/reasoningInputPolicy.ts index 71a283a706..94f27ddcdc 100644 --- a/open-sse/services/reasoningInputPolicy.ts +++ b/open-sse/services/reasoningInputPolicy.ts @@ -1,5 +1,6 @@ import { REGISTRY } from "../config/providerRegistry.ts"; import type { ReasoningTransport } from "../config/providerRegistry.ts"; +import { isValidResponsesItemId } from "./responsesItemId.ts"; type JsonRecord = Record; @@ -279,13 +280,27 @@ function sanitizeResponsesInput( if (!hasPlaintext && !hasOpaque && (!hasDisplaySummary(next) || stripOrphanedSummaries)) { continue; } - if (!hasOpaque && typeof next.id === "string") delete next.id; + // `id` is only worth keeping on an opaque item with a valid string value — + // non-opaque items don't replay their id, and a malformed value (e.g. `null`, + // observed on opencode/zen) must not survive either way (#11108). + if (!hasOpaque || !isValidResponsesItemId(next.id)) delete next.id; + // Some upstreams (e.g. opencode/zen) omit `summary` entirely on opaque + // reasoning items instead of sending an empty array. Replaying that shape + // verbatim trips strict Responses-API validators that require the field + // to be present on every `input[]` item of type `reasoning` (#11108). + // Plaintext-only items intentionally have no `summary` key and must stay + // untouched. + if (hasOpaque && next.summary === undefined) next.summary = []; filtered.push(next); continue; } const cloned = { ...record }; - if (typeof cloned.id === "string") delete cloned.id; + // Strip `id` whenever present, valid or not: these items don't need a + // replayed server id, and a malformed one (e.g. `null`, same opencode/zen + // omission pattern as the reasoning branch above) must not survive either + // (#11108). + if (cloned.id !== undefined) delete cloned.id; filtered.push(cloned); } return filtered; diff --git a/open-sse/services/responsesInputSanitizer.ts b/open-sse/services/responsesInputSanitizer.ts index 5d81b787a1..94cd99f934 100644 --- a/open-sse/services/responsesInputSanitizer.ts +++ b/open-sse/services/responsesInputSanitizer.ts @@ -1,3 +1,5 @@ +import { isValidResponsesItemId } from "./responsesItemId.ts"; + type JsonRecord = Record; type SanitizeResponsesInputOptions = { dropInternalAssistantMessages?: boolean; @@ -40,7 +42,12 @@ function sanitizeFunctionName(name: string): string { } function sanitizeInputItemId(record: JsonRecord): JsonRecord { - if (typeof record.id !== "string") return record; + if (record.id === undefined) return record; + if (!isValidResponsesItemId(record.id)) { + const next = { ...record }; + delete next.id; + return next; + } const type = typeof record.type === "string" ? record.type : ""; const expectedPrefix = SERVER_ITEM_ID_PREFIX_BY_TYPE[type]; diff --git a/open-sse/services/responsesItemId.ts b/open-sse/services/responsesItemId.ts new file mode 100644 index 0000000000..a57ac92e42 --- /dev/null +++ b/open-sse/services/responsesItemId.ts @@ -0,0 +1,7 @@ +// Shared by reasoningInputPolicy.ts and responsesInputSanitizer.ts: both strip a +// Responses-API `input[]` item's `id` field when it isn't a valid string before +// replay, so a malformed value (e.g. `null`, observed on opencode/zen) never +// survives to trip a strict upstream with "Expected 'id' to be a string." (#11108). +export function isValidResponsesItemId(id: unknown): id is string { + return typeof id === "string"; +} diff --git a/tests/unit/reasoning-input-policy-summary-11108.test.ts b/tests/unit/reasoning-input-policy-summary-11108.test.ts new file mode 100644 index 0000000000..4317225dfb --- /dev/null +++ b/tests/unit/reasoning-input-policy-summary-11108.test.ts @@ -0,0 +1,145 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +const { applyReasoningInputPolicy } = + await import("../../open-sse/services/reasoningInputPolicy.ts"); + +test("#11108 applyReasoningInputPolicy defaults summary on a kept opaque reasoning item", () => { + const body: Record = { + input: [ + { + type: "reasoning", + id: "rs_example", + encrypted_content: "opaque-blob", + }, + ], + }; + + applyReasoningInputPolicy(body, "responses", { + provider: "opencode", + preserveEncryptedReasoning: true, + }); + + const input = body.input as Record[]; + assert.equal(input.length, 1); + assert.deepEqual(input[0].summary, []); +}); + +test("#11108 applyReasoningInputPolicy preserves an existing summary on a kept reasoning item", () => { + const body: Record = { + input: [ + { + type: "reasoning", + id: "rs_example", + encrypted_content: "opaque-blob", + summary: [{ type: "summary_text", text: "Planning." }], + }, + ], + }; + + applyReasoningInputPolicy(body, "responses", { + provider: "opencode", + preserveEncryptedReasoning: true, + }); + + const input = body.input as Record[]; + assert.deepEqual(input[0].summary, [{ type: "summary_text", text: "Planning." }]); +}); + +test("#11108 applyReasoningInputPolicy defaults summary on an opaque item surviving incompatible-drop", () => { + // Mixed item (plaintext + opaque) on an opaque-only transport is incompatible; + // dropIncompatibleResponsesReasoning() strips the plaintext content but keeps + // the opaque item alive — it must still get a default `summary`. + const body: Record = { + input: [ + { + type: "reasoning", + id: "rs_mixed", + content: [{ type: "reasoning_text", text: "inspect first" }], + encrypted_content: "opaque-blob", + }, + ], + }; + + const result = applyReasoningInputPolicy(body, "responses", { + provider: "codex", + onIncompatibleReasoning: "drop", + }); + + assert.equal(result.incompatibleReasoning, false); + const input = body.input as Record[]; + assert.equal(input.length, 1); + assert.equal(input[0].content, undefined); + assert.equal(input[0].encrypted_content, "opaque-blob"); + assert.deepEqual(input[0].summary, []); +}); + +test("#11108 applyReasoningInputPolicy strips a non-string id on a kept opaque reasoning item", () => { + // Same gap class as the summary fix above: opencode/zen also omits `id` + // entirely (surfaced by the client as `id: null`) on opaque-only reasoning + // items instead of a `rs_...` string. Replaying that shape verbatim trips + // strict Responses-API validators with "Expected 'id' to be a string." + const body: Record = { + input: [ + { + type: "reasoning", + id: null, + encrypted_content: "opaque-blob", + }, + ], + }; + + applyReasoningInputPolicy(body, "responses", { + provider: "opencode", + preserveEncryptedReasoning: true, + }); + + const input = body.input as Record[]; + assert.equal(input.length, 1); + assert.equal("id" in input[0], false); +}); + +test("#11108 applyReasoningInputPolicy strips a non-string id on a non-reasoning item (function_call)", () => { + // Same gap class, generic branch: any non-"reasoning" input item (function_call, + // message, ...) only stripped `id` when it was already a valid string, so a + // malformed `id` (e.g. `null`, mirroring the opencode/zen omission pattern) + // on a function_call item survived replay untouched. + const body: Record = { + input: [ + { + type: "function_call", + id: null, + call_id: "call_abc", + name: "bash", + arguments: "{}", + }, + ], + }; + + applyReasoningInputPolicy(body, "responses", { provider: "opencode" }); + + const input = body.input as Record[]; + assert.equal(input.length, 1); + assert.equal("id" in input[0], false); + assert.equal(input[0].call_id, "call_abc"); +}); + +test("#11108 applyReasoningInputPolicy preserves a valid string id on a kept opaque reasoning item", () => { + const body: Record = { + input: [ + { + type: "reasoning", + id: "rs_example", + encrypted_content: "opaque-blob", + }, + ], + }; + + applyReasoningInputPolicy(body, "responses", { + provider: "opencode", + preserveEncryptedReasoning: true, + }); + + const input = body.input as Record[]; + assert.equal(input[0].id, "rs_example"); +}); diff --git a/tests/unit/responses-input-sanitizer-name.test.ts b/tests/unit/responses-input-sanitizer-name.test.ts index 7ac09604c7..4eec2d6c10 100644 --- a/tests/unit/responses-input-sanitizer-name.test.ts +++ b/tests/unit/responses-input-sanitizer-name.test.ts @@ -73,6 +73,23 @@ test("keeps valid server reasoning item ids", () => { assert.equal(result[0].id, "rs_123"); }); +test("strips a non-string reasoning item id instead of passing it through (#11108)", () => { + // Same gap class fixed in reasoningInputPolicy.ts: some upstreams (e.g. + // opencode/zen) send `id: null` instead of omitting it. The previous + // `typeof record.id !== "string"` guard returned the record unchanged in + // that case, letting a malformed id reach a strict Responses-API upstream. + const items = [ + { + id: null, + type: "reasoning", + summary: [{ type: "summary_text", text: "cached reasoning" }], + }, + ]; + const result = sanitizeResponsesInputItems(items) as Array>; + assert.equal("id" in result[0], false); + assert.equal(result[0].type, "reasoning"); +}); + test("normalizes user image_url content parts to input_image", () => { const items = [ { diff --git a/tests/unit/strip-reasoning-blobs-agentic-context-1599.test.ts b/tests/unit/strip-reasoning-blobs-agentic-context-1599.test.ts index cab5e9c37f..25eae7a839 100644 --- a/tests/unit/strip-reasoning-blobs-agentic-context-1599.test.ts +++ b/tests/unit/strip-reasoning-blobs-agentic-context-1599.test.ts @@ -274,7 +274,11 @@ test("explicit custom target opt-in remains an opaque transport override", () => const result = applyReasoningInputPolicy(body, "responses", { preserveEncryptedReasoning: true }); assert.equal(result.incompatibleReasoning, false); - assert.deepEqual(body.input, [{ type: "reasoning", encrypted_content: "encrypted-blob" }]); + // #11108: a kept opaque item defaults `summary` when the source omitted it — + // some upstreams reject `input[]` reasoning items missing the field entirely. + assert.deepEqual(body.input, [ + { type: "reasoning", encrypted_content: "encrypted-blob", summary: [] }, + ]); }); test("preserved opaque reasoning remains redacted from log copies", () => {