diff --git a/src/adapters/anthropic.ts b/src/adapters/anthropic.ts index de86753b7c..25e6467ad0 100644 --- a/src/adapters/anthropic.ts +++ b/src/adapters/anthropic.ts @@ -265,6 +265,39 @@ export function formatAnthropicErrorBody(status: number, _headers: Headers, payl return redactSecretString(detail).slice(0, 400); } +function isAnthropicRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function anthropicStructuralValueType(value: unknown): string { + if (value === null) return "null"; + return Array.isArray(value) ? "array" : typeof value; +} + +interface InvalidAnthropicShapeDiagnostic { + reason: "content_not_array" | "content_block_not_object"; + blockIndex?: number; + valueType: string; +} + +/** + * Structured refusal for a malformed buffered response body, shaped like the google adapter's + * `invalidGoogleShapeEvent` so an operator reading a log can tell which rung failed: the content + * container, or one block inside an otherwise well-formed container. + */ +function invalidAnthropicShapeEvent( + diagnostic: InvalidAnthropicShapeDiagnostic, +): Extract { + const at = diagnostic.blockIndex !== undefined ? `; blockIndex=${diagnostic.blockIndex}` : ""; + const subject = diagnostic.reason === "content_not_array" ? "content" : "content block"; + return { + type: "error", + message: `anthropic response contained invalid ${subject} (${diagnostic.reason}${at}; valueType=${diagnostic.valueType})`, + status: 502, + errorType: "upstream_error", + }; +} + function extractAnthropicErrorDetail(parsed: unknown): string | undefined { if (typeof parsed === "string") return parsed.trim() || undefined; if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) return undefined; @@ -1279,12 +1312,55 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti }, async parseResponse(response: Response, budget: TranslatorBudget): Promise { - const json = await response.json() as Record; + const parsed: unknown = await response.json(); + // `response.json()` resolves a body of `null` to `null` without throwing, so the cast below + // used to reach `json.content` on it — the #1219 defect at the buffered body root. The + // streaming parser has skipped a non-record frame since #1240, but a buffered body has no + // next frame to recover into, so this fails closed instead. + if (!isAnthropicRecord(parsed)) { + return [{ + type: "error", + message: `anthropic response was not a JSON object (${anthropicStructuralValueType(parsed)})`, + status: 502, + errorType: "upstream_error", + }]; + } + const json = parsed; const responseBytes = new TextEncoder().encode(JSON.stringify(json)).byteLength; budget.chargeRetained(responseBytes, { kind: "retained_collectors" }); try { const events: AdapterEvent[] = []; - const content = json.content as { type: string; text?: string; id?: string; name?: string; input?: unknown; thinking?: string; reasoning?: string; signature?: string; data?: string }[] | undefined; + // Same retain-then-return tail every other terminal branch in this function uses; naming it + // keeps the two shape guards below from drifting out of step with the accounting. + const finishWithEvents = (batch: AdapterEvent[]): AdapterEvent[] => { + retainTranslatedEventBatch(batch, budget); + return batch; + }; + const rawContent: unknown = json.content; + // `content` is claimed model output inside a well-formed response, so it is governed by the + // #1332 nested-shape rule (fail closed) rather than #1240's root-frame padding rule (skip). + // Absence stays legal in both encodings. A present non-array was silently accepted and is + // the dangerous case: a string is iterable, so `for (const block of "text")` walked it one + // CHARACTER at a time, read `undefined` from every `block.type`, and completed the turn as a + // successful empty response — the #2231 failure mode on a different adapter. + if (rawContent !== undefined && rawContent !== null && !Array.isArray(rawContent)) { + return finishWithEvents([invalidAnthropicShapeEvent({ + reason: "content_not_array", + valueType: anthropicStructuralValueType(rawContent), + })]); + } + if (Array.isArray(rawContent)) { + for (let blockIndex = 0; blockIndex < rawContent.length; blockIndex++) { + if (!isAnthropicRecord(rawContent[blockIndex])) { + return finishWithEvents([invalidAnthropicShapeEvent({ + reason: "content_block_not_object", + blockIndex, + valueType: anthropicStructuralValueType(rawContent[blockIndex]), + })]); + } + } + } + const content = rawContent as { type: string; text?: string; id?: string; name?: string; input?: unknown; thinking?: string; reasoning?: string; signature?: string; data?: string }[] | undefined; if (content) { for (const block of content) { if (block.type === "text" && block.text) { diff --git a/src/adapters/command-code.ts b/src/adapters/command-code.ts index 1cd7975d83..49f8bcf2a3 100644 --- a/src/adapters/command-code.ts +++ b/src/adapters/command-code.ts @@ -7,6 +7,7 @@ import { isAllowedToolChoice, namespacedToolName, resolveToolChoiceWireName, too import type { AdapterFetchContext, AdapterRequest, ProviderAdapter } from "./base"; import type { TranslatorBudget } from "../lib/translator-budget"; import { readBoundedResponseBody } from "../lib/bounded-body"; +import { debugDroppedFrame } from "../lib/debug"; import { configuredReasoningEfforts } from "../reasoning-effort"; import { commandCodeReasoningEfforts, refreshCommandCodeReasoningEfforts } from "../providers/command-code-efforts"; import { identifyRoutedModel } from "./identity"; @@ -365,7 +366,7 @@ async function*ndjson(response: Response, budget: TranslatorBudget): AsyncGenera let newline = buffer.indexOf("\n"); while (newline >= 0) { const line = buffer.slice(0, newline).trim(); buffer = buffer.slice(newline + 1); - if (line) { try { yield JSON.parse(stripEventFrame(line)) as Record; } catch { /* ignore non-events */ } } + if (line) yield* decodeEventLine(line); newline = buffer.indexOf("\n"); } const residualBytes = encoder.encode(buffer).byteLength; @@ -376,7 +377,7 @@ async function*ndjson(response: Response, budget: TranslatorBudget): AsyncGenera if (done) break; } const final = buffer.trim(); - if (final) { try { yield JSON.parse(stripEventFrame(final)) as Record; } catch { /* ignore */ } } + if (final) yield* decodeEventLine(final); } finally { budget.releaseRetained(bufferBytes, { kind: "live_transient" }); try { await reader.cancel(); } catch { /* already closed */ } @@ -384,6 +385,41 @@ async function*ndjson(response: Response, budget: TranslatorBudget): AsyncGenera } } +/** + * Yield one NDJSON line as an event record, or nothing. + * + * `JSON.parse("null")` returns `null` instead of throwing, so the `try/catch` around the parse + * cannot see it and the `event.type` read in parseStream crashed the turn — the #1219 defect, on + * the one streaming transport the #1240 audit did not cover because it is NDJSON rather than SSE. + * + * A frame that does not parse to a record is padding, not an event: drop it and continue exactly + * as an unparseable line is already dropped, so a stream whose only frames are junk ends in the + * same single terminal `done` as an empty body. Skipping is what preserves an answer whose deltas + * have already arrived — the observed #1219 case is `null` padding BETWEEN content deltas, where + * terminating would discard a complete response (#1240). + * + * Note this deliberately makes a junk-only stream a quiet `[done]` where it previously threw. That + * throw was an unguarded type assumption, not a designed failure signal, and `[done]` is already + * what an empty body, a blank-line-only body and an unparseable-only body all produce here. The + * broader question — whether this adapter should report *any* no-valid-event stream as a failure + * rather than an empty success — is pre-existing, applies to all four of those inputs equally, and + * is deliberately not decided by this change. + */ +function* decodeEventLine(line: string): Generator> { + let parsed: unknown; + try { + parsed = JSON.parse(stripEventFrame(line)); + } catch { + debugDroppedFrame("command-code", line); + return; + } + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + debugDroppedFrame("command-code", line); + return; + } + yield parsed as Record; +} + /** The endpoint is newline-delimited JSON; defensively strip an SSE `data:` frame if the gateway ever switches shapes. */ function stripEventFrame(line: string): string { return line.startsWith("data:") ? line.slice("data:".length).trim() : line; diff --git a/src/adapters/google.ts b/src/adapters/google.ts index 92fa2a81fe..19c142a994 100644 --- a/src/adapters/google.ts +++ b/src/adapters/google.ts @@ -1158,7 +1158,18 @@ export function createGoogleAdapter(provider: OcxProviderConfig): ProviderAdapte let raw: Record; let rawBytes = 0; try { - raw = JSON.parse(rawText) as Record; + const parsedRaw: unknown = JSON.parse(rawText); + // `JSON.parse("null")` returns null instead of throwing, so the catch below cannot see it + // and the `raw.error` read crashed the turn — #1219 at the buffered body root, which #1240 + // never reached because that audit swept SSE frame parsers only. There is no next frame to + // recover into here, so unlike a stream frame this fails closed, matching the + // unparseable-body branch just below and the buffered candidate guards added in #2232. + if (!isGoogleRecord(parsedRaw)) { + budget.releaseRetained(rawTextBytes, { kind: "retained_collectors" }); + const valueType = googleStructuralValueType(parsedRaw); + return [{ type: "error", message: `google response was not a JSON object (${valueType})` }]; + } + raw = parsedRaw; rawBytes = new TextEncoder().encode(JSON.stringify(raw)).byteLength; const rawReservation = budget.reserveTransient(rawBytes, { kind: "retained_collectors" }); rawReservation.commitRetained(); diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 7ad61f1b4d..c877a1b357 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1932,8 +1932,28 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const choice = rawChoice; if (choice.finish_reason === "error") return [upstreamErrorEvent(choice.error, usage)]; if (!choice.message) return [{ type: "error", message: "upstream response contained no choices", ...(usage ? { usage } : {}) }]; + // `!choice.message` splits this input class on TRUTHINESS, not on shape: `null` and `0` fail + // closed here, while `"text"`, `true` and `[{...}]` pass and every property read below yields + // `undefined` — so a choice claiming an assistant message completed as a SUCCESSFUL EMPTY + // turn, stranding any tool call it claimed. The one line above already validates the choice + // container this way; its message was left on a truthiness test. + // + // Every non-record is rejected, arrays included. The google adapter does carve out `[]` for + // `content`, but that carve-out is specific to a protobuf-derived wire where a repeated + // field can spell an empty message — and `content` is genuinely an ARRAY of blocks there. + // `message` is a record on a plain-JSON wire that already has `{}`, so importing the + // exception would be an analogy rather than evidence. `[{"content":"…"}]` is the case that + // matters: it discards a complete answer, #2232's `content: [{ parts: [...] }]` one adapter over. + // + // Read through `unknown` rather than the declared type: `choices` is a cast over wire data, + // so its `message?: Record` is an assertion the upstream never made, and + // narrowing against it is what let the missing check look type-safe. + const rawMessage: unknown = choice.message; + if (!isRecord(rawMessage)) { + return [invalidChoicesEvent(usage)]; + } - const msg = choice.message; + const msg = rawMessage as Record; const reasoningText = reasoningTextFrom(msg); if (reasoningText !== undefined) events.push({ type: "reasoning_raw_delta", text: reasoningText }); if (typeof msg.content === "string") events.push({ type: "text_delta", text: msg.content }); diff --git a/tests/buffered-response-shape-guards.test.ts b/tests/buffered-response-shape-guards.test.ts new file mode 100644 index 0000000000..ba0f309451 --- /dev/null +++ b/tests/buffered-response-shape-guards.test.ts @@ -0,0 +1,340 @@ +import { describe, expect, test } from "bun:test"; +import { createAnthropicAdapter } from "../src/adapters/anthropic"; +import { createCommandCodeAdapter } from "../src/adapters/command-code"; +import { createGoogleAdapter } from "../src/adapters/google"; +import { createOpenAIChatAdapter } from "../src/adapters/openai-chat"; +import { createTestTranslatorBudget } from "./helpers/translator-budget"; +import { createTranslatorBudget, translatorObservedBufferSnapshot } from "../src/lib/translator-budget"; +import type { AdapterEvent, OcxProviderConfig } from "../src/types"; + +/** + * `JSON.parse("null")` returns `null` without throwing, so a `try/catch` around a body parse + * cannot see it. #1240 closed that at the SSE *frame* root for the four SSE parsers. Two rungs + * were never swept, because neither is an SSE frame parser: + * + * - the BUFFERED body root, on the `parseResponse` path reached from + * `src/server/responses/core.ts` for non-streaming turns; + * - the NDJSON frame root in the Command Code transport. + * + * And inside a well-formed anthropic body, `content` was consumed unchecked, so the #1332/#2232 + * nested-shape ladder was open there too. + * + * Every assertion below is written as PARITY against a control that was already handled correctly + * — an unparseable body, or the same field on a sibling adapter — so the test states the actual + * requirement rather than re-encoding each adapter's exact wording and drifting when it changes. + */ + +// Valid JSON that does not deserialize to a record. Only `null` ever threw; property access on a +// number, string, boolean or array is legal JS, so those were silently accepted as empty bodies. +// Both are wrong for the same reason, so they are asserted together. +const NON_RECORD_BODIES = ["null", "42", '"text"', "true", "[]", "[null]"] as const; + +// The syntactically-invalid control, correct before this change on every adapter here. +const INVALID_JSON_BODY = "{not json}"; + +const googleProvider = { adapter: "google", baseUrl: "https://example.test/v1", apiKey: "k", authMode: "key" } as OcxProviderConfig; +const anthropicProvider = { adapter: "anthropic", baseUrl: "https://example.test/v1", apiKey: "k", authMode: "key" } as OcxProviderConfig; +const commandCodeProvider: OcxProviderConfig = { adapter: "command-code", baseUrl: "https://api.command.example", apiKey: "k" }; +const openAIChatProvider = { adapter: "openai-chat", baseUrl: "https://example.test/v1", apiKey: "k", authMode: "key" } as OcxProviderConfig; + +function jsonResponse(body: string): Response { + return new Response(body, { status: 200, headers: { "content-type": "application/json" } }); +} + +function ndjsonResponse(lines: string[]): Response { + return new Response(lines.map(l => `${l}\n`).join(""), { + status: 200, + headers: { "content-type": "application/x-ndjson" }, + }); +} + +function errorEvent(events: AdapterEvent[]): Extract | undefined { + return events.find(e => e.type === "error") as Extract | undefined; +} + +function text(events: AdapterEvent[]): string { + return events.flatMap(e => (e.type === "text_delta" ? [e.text] : [])).join(""); +} + +describe("google parseResponse: buffered body root", () => { + const parse = (body: string) => + createGoogleAdapter(googleProvider).parseResponse!(jsonResponse(body), createTestTranslatorBudget()); + + test("an unparseable body reports a structured error (control)", async () => { + const events = await parse(INVALID_JSON_BODY); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test.each(NON_RECORD_BODIES)("a valid-JSON non-record body (%s) is treated like the control", async body => { + // Before the guard, `null` threw `null is not an object (evaluating 'raw.error')` out of + // parseResponse; the others reached `json.candidates` as `undefined` and reported "no + // candidates". Both now fail closed the same way an unparseable body does. + const events = await parse(body); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("the value type is named, so a log distinguishes the shapes", async () => { + expect(errorEvent(await parse("null"))?.message).toContain("null"); + expect(errorEvent(await parse("[]"))?.message).toContain("array"); + }); + + test("a healthy body is unaffected", async () => { + const events = await parse(JSON.stringify({ + candidates: [{ content: { parts: [{ text: "PONG" }] }, finishReason: "STOP" }], + })); + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "done")).toBe(true); + }); +}); + +describe("anthropic parseResponse: buffered body root", () => { + const parse = (body: string) => + createAnthropicAdapter(anthropicProvider).parseResponse!(jsonResponse(body), createTestTranslatorBudget()); + + test.each(NON_RECORD_BODIES)("a valid-JSON non-record body (%s) fails closed", async body => { + // `response.json()` resolves a body of `null` to `null`, and the cast reached `json.content` + // on it. The rest were accepted as an empty successful turn. + const events = await parse(body); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("a healthy body is unaffected", async () => { + const events = await parse(JSON.stringify({ + type: "message", + role: "assistant", + content: [{ type: "text", text: "PONG" }], + stop_reason: "end_turn", + usage: { input_tokens: 2, output_tokens: 3 }, + })); + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "done")).toBe(true); + }); +}); + +describe("anthropic parseResponse: content container and blocks", () => { + const parseContent = (contentLiteral: string) => + createAnthropicAdapter(anthropicProvider).parseResponse!( + jsonResponse(`{"type":"message","role":"assistant","content":${contentLiteral},"stop_reason":"end_turn"}`), + createTestTranslatorBudget(), + ); + + // Absence is a legitimate contract and both encodings of it must keep working. Tightening the + // container must not delete this, exactly as the google guards left `parts: null` alone. + test.each(["undefined-omitted", "null", "[]"])("absence encoding %s still completes the turn", async encoding => { + const events = encoding === "undefined-omitted" + ? await createAnthropicAdapter(anthropicProvider).parseResponse!( + jsonResponse('{"type":"message","role":"assistant","stop_reason":"end_turn"}'), + createTestTranslatorBudget(), + ) + : await parseContent(encoding); + expect(errorEvent(events)).toBeUndefined(); + expect(events.some(e => e.type === "done")).toBe(true); + }); + + test.each(['"a claimed answer"', "42", "true", '{"type":"text","text":"x"}'])( + "a present non-array content (%s) fails closed", + async literal => { + // The string case is the dangerous one and the reason this is not merely tidiness: a string + // is iterable, so `for (const block of "a claimed answer")` walked it one CHARACTER at a + // time, read `undefined` from every `block.type`, emitted nothing, and reported a clean + // `done` — a successful EMPTY turn for a response that claimed content. Same failure mode as + // #2231's `parts: "txt"`. + const events = await parseContent(literal); + const error = errorEvent(events); + expect(error).toBeDefined(); + expect(error?.message).toContain("content_not_array"); + expect(events.some(e => e.type === "done")).toBe(false); + }, + ); + + test("a non-record block inside a well-formed array fails closed and names its index", async () => { + const events = await parseContent('[{"type":"text","text":"first"},null]'); + const error = errorEvent(events); + expect(error).toBeDefined(); + expect(error?.message).toContain("content_block_not_object"); + expect(error?.message).toContain("blockIndex=1"); + // Fail closed BEFORE emitting: half a claimed answer plus a silent stop is worse than a + // reported failure, and matches how #1332 handles a malformed nested payload. + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("a well-formed content array is unaffected", async () => { + const events = await parseContent('[{"type":"text","text":"PONG"},{"type":"tool_use","id":"toolu_1","name":"get","input":{}}]'); + expect(errorEvent(events)).toBeUndefined(); + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "tool_call_start")).toBe(true); + }); +}); + +describe("command-code ndjson: non-record frame", () => { + async function collect(lines: string[]): Promise { + const events: AdapterEvent[] = []; + const adapter = createCommandCodeAdapter(commandCodeProvider); + for await (const e of adapter.parseStream(ndjsonResponse(lines), createTestTranslatorBudget())) events.push(e); + return events; + } + + const HEALTHY_HEAD = ['{"type":"text-delta","text":"P"}', '{"type":"text-delta","text":"ONG"}']; + const HEALTHY_TAIL = ['{"type":"finish","finishReason":"stop","totalUsage":{"inputTokens":1,"outputTokens":2}}']; + + test.each(["null", "42", '"text"', "true", "[]"])( + "a mid-stream non-record line (%s) is skipped, not fatal", + async line => { + // Unlike a buffered body, a stream frame has a next frame to recover into. #1240 established + // that a non-record frame is padding: terminating on it throws away an answer that has + // already fully arrived. `null` used to throw `null is not an object ('event.type')`. + const events = await collect([...HEALTHY_HEAD, line, ...HEALTHY_TAIL]); + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "done")).toBe(true); + expect(errorEvent(events)).toBeUndefined(); + }, + ); + + test("an unparseable line is skipped the same way (control)", async () => { + const events = await collect([...HEALTHY_HEAD, INVALID_JSON_BODY, ...HEALTHY_TAIL]); + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "done")).toBe(true); + }); + + test("a non-record line in the RESIDUAL buffer is skipped", async () => { + // The trailing-buffer branch (`const final = buffer.trim()`) parses separately from the newline + // loop and shared the same defect, so it needs its own case. + // + // `ndjsonResponse` cannot reach it: it appends a newline to EVERY line, so the newline loop + // drains the buffer and `final` is always empty. An earlier version of this test used that + // helper and asserted the same outcome — it passed for the wrong reason, because the residual + // branch never ran. The body below deliberately omits the trailing newline, and places no + // finish event before the residual frame, so the malformed line is reachable only via `final`. + const body = new Response(`${HEALTHY_HEAD.join("\n")}\nnull`, { + status: 200, + headers: { "content-type": "application/x-ndjson" }, + }); + const events: AdapterEvent[] = []; + const adapter = createCommandCodeAdapter(commandCodeProvider); + for await (const e of adapter.parseStream(body, createTestTranslatorBudget())) events.push(e); + + expect(text(events)).toBe("PONG"); + expect(events.some(e => e.type === "done")).toBe(true); + }); + + test("a junk-only stream matches EVERY other junk-only control", async () => { + // NOT the #1240 rule, deliberately. The SSE adapters fail closed on an all-padding stream; + // this adapter ends every finish-less stream with a terminal `done` on purpose ("so the server + // does not wait on an adapter that silently stopped emitting"), and an EMPTY body, a + // blank-line-only body and an unparseable-only body all already do exactly that on unmodified + // `dev`. So the requirement here is parity with those controls, not termination — asserting + // otherwise would be demanding a behavior change this diff has no mandate to make. + // The claim being pinned is that non-record-only joins an EXISTING class rather than forming a + // new one, so all four members are compared, not just the nearest. + const nonRecordOnly = await collect(["null", "42", "[]"]); + for (const control of [[INVALID_JSON_BODY], [""], []]) { + expect(nonRecordOnly.map(e => e.type)).toEqual((await collect(control)).map(e => e.type)); + } + expect(errorEvent(nonRecordOnly)).toBeUndefined(); + }); +}); + +describe("openai-chat parseResponse: the claimed assistant message", () => { + const parseMessage = (messageLiteral: string) => + createOpenAIChatAdapter(openAIChatProvider).parseResponse!( + jsonResponse(`{"choices":[{"message":${messageLiteral},"finish_reason":"stop","index":0}],"usage":{"prompt_tokens":1,"completion_tokens":2}}`), + createTestTranslatorBudget(), + ); + + // The pre-existing `if (!choice.message)` split this input class on TRUTHINESS rather than shape. + // These two were already refused and must stay refused — they are the control the rest is parity + // against, and they are why the gap was invisible: the guard looked present because it fired on + // the two shapes anyone would try first. + test.each(["null", "0"])("a falsy non-record message (%s) still fails closed", async literal => { + const events = await parseMessage(literal); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test.each(['"ANSWER"', "true", "[null]", "[]"])( + "a truthy non-record message (%s) now fails closed too", + async literal => { + // Before: every property read yielded `undefined`, so the turn completed as a SUCCESSFUL + // EMPTY response for a choice that claimed an assistant message — and any tool call it + // claimed was silently stranded. + const events = await parseMessage(literal); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }, + ); + + test("a non-empty array message does not discard the answer inside it", async () => { + // #2232's `content: [{ parts: [...] }]` shape, one adapter over: a complete answer sits in the + // payload, `message.content` reads `undefined` off the array, and the turn reported success. + const events = await parseMessage('[{"role":"assistant","content":"ANSWER"}]'); + expect(errorEvent(events)).toBeDefined(); + expect(text(events)).toBe(""); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("an empty array message fails closed too - the google carve-out does not transfer", async () => { + // google DOES accept `content: []`, but that carve-out is specific to a protobuf-derived wire + // where a repeated field can spell an empty message, and `content` is genuinely an array of + // blocks there. `message` is a record on a plain-JSON wire that already has `{}`. Importing the + // exception here would be reasoning from analogy rather than from this wire's contract. + const events = await parseMessage("[]"); + expect(errorEvent(events)).toBeDefined(); + expect(events.some(e => e.type === "done")).toBe(false); + }); + + test("an empty record message stays legal", async () => { + const events = await parseMessage("{}"); + expect(errorEvent(events)).toBeUndefined(); + expect(events.some(e => e.type === "done")).toBe(true); + }); + + test("a well-formed message still delivers its text and tool call", async () => { + const events = await parseMessage('{"role":"assistant","content":"ANSWER","tool_calls":[{"id":"c1","type":"function","function":{"name":"get","arguments":"{}"}}]}'); + expect(errorEvent(events)).toBeUndefined(); + expect(text(events)).toBe("ANSWER"); + expect(events.some(e => e.type === "tool_call_start")).toBe(true); + expect(events.some(e => e.type === "done")).toBe(true); + }); +}); + + +describe("a guard that fails closed still releases the response-body reservation", () => { + // These guards return early from inside the region between `chargeRetained(responseBytes)` and + // the `finally` that releases it, so a return placed on the wrong side of that boundary would + // strand the whole body in the translator budget on every malformed response — a slow leak that + // no events-only assertion can see. + // + // Asserting "bytes return to baseline after dispose" would NOT catch it: `dispose()` force-clears + // whatever the budget still holds (`aggregateCurrentBytes -= this.currentBytes`), so that check is + // vacuously true. The measurement has to happen BEFORE dispose, and the body has to be large + // enough that a leaked reservation is unmistakable next to the tiny error event legitimately + // retained on the way out. + const PADDING = "x".repeat(50_000); + + const cases: [string, OcxProviderConfig, (p: OcxProviderConfig) => { parseResponse?: unknown }, string][] = [ + ["anthropic content non-array", anthropicProvider, createAnthropicAdapter as never, + `{"content":"${PADDING}","stop_reason":"end_turn"}`], + ["anthropic block non-record", anthropicProvider, createAnthropicAdapter as never, + `{"content":[{"type":"text","text":"${PADDING}"},null],"stop_reason":"end_turn"}`], + ["openai-chat message non-record", openAIChatProvider, createOpenAIChatAdapter as never, + `{"choices":[{"message":"${PADDING}","finish_reason":"stop"}]}`], + ]; + + test.each(cases)("%s does not strand the body", async (_label, provider, make, body) => { + const budget = createTranslatorBudget(); + const before = translatorObservedBufferSnapshot().currentBytes; + const adapter = (make as (p: OcxProviderConfig) => { + parseResponse: (r: Response, b: unknown) => Promise; + })(provider); + const events = await adapter.parseResponse(jsonResponse(body), budget); + const heldBeforeDispose = translatorObservedBufferSnapshot().currentBytes - before; + budget.dispose(); + + expect(errorEvent(events)).toBeDefined(); + // Only the returned error event may still be held. The 50KB body must already be released. + expect(heldBeforeDispose).toBeLessThan(2_000); + }); +});