Skip to content

[Bug] Nested field shapes bypass the #1219 frame guard and crash both adapter paths #1325

Description

@snowyukitty

Client or integration

Not client-specific — reproduced by driving the adapters directly with a synthetic Response.

Area

Adapters · Streaming / SSE parsing · Non-streaming response parsing

Summary

#1219 / #1240 fixed the frame root: JSON.parse(payload) was cast to Record<string, unknown> and dereferenced, so data: null crashed the parser. The same "cast, then dereference" pattern survives one level deeper, on fields inside an otherwise well-formed frame. A frame that is a valid JSON object still crashes the adapter when a nested field has an unexpected shape.

These are found by audit, not from a live capture. Unlike data: null — which agentrouter.org demonstrably emits as padding — I have no evidence any provider sends these. Filing because the defect class and blast radius are identical to #1219 (an escaping TypeError mid-stream, which the surrounding try/catch cannot classify), and because the guards are cheap.

Verified on dev at 79831c90, with the #1240 fix already present.

# File Line Trigger Thrown
A src/adapters/google.ts :583 {"candidates":[null]} null is not an object (evaluating 'candidates[0].finishReason')
B src/adapters/openai-chat.ts :1040 delta.tool_calls not an array {} is not iterable
C src/adapters/openai-chat.ts :1041 delta.tool_calls: [null] null is not an object (evaluating 'tc.index')
D src/adapters/openai-chat.ts :1226 non-stream tool_calls not an array {} is not iterable
E src/adapters/openai-chat.ts :1226 non-stream tool_calls: [null] null is not an object (evaluating 'tc.id')
F src/adapters/openai-chat.ts :1227 non-stream element without function undefined is not an object (evaluating 'tc.function.name')

D–F are in parseResponse, not parseStream — the non-streaming path has the same defect, so a fix confined to the streaming parsers would be incomplete.

In each case the cast asserts a shape that is never checked: root.candidates as {...}[] at google.ts:580 (!candidates?.length guards the array, not its elements), and delta.tool_calls as {...}[] / msg.tool_calls as {...}[] at openai-chat.ts:1038 and :1224.

Checked and clean — not every nested site is affected. google.ts:809 reads candidates?.[0]?.content?.parts with optional chaining, so the non-streaming Google path returns a normal done for {"candidates":[null]} rather than throwing. Google's non-stream path needs no change.

Reproduction

Save as tests/zz-nested-probe.test.ts on dev and run bun scripts/test.ts tests/zz-nested-probe.test.ts, then delete it.

import { test } from "bun:test";
import { createGoogleAdapter } from "../src/adapters/google";
import { createOpenAIChatAdapter } from "../src/adapters/openai-chat";
import { withTestTranslatorBudget } from "./helpers/translator-budget";

const g = { adapter: "google", baseUrl: "https://x.test", apiKey: "k", authMode: "key" } as any;
const o = { adapter: "openai-chat", baseUrl: "https://x.test/v1", apiKey: "k", authMode: "key" } as any;

async function stream(label: string, adapter: any, body: string) {
  try {
    const out: string[] = [];
    for await (const e of adapter.parseStream(new Response(body, { headers: { "content-type": "text/event-stream" } }))) out.push(e.type);
    console.log(`  ${label}: no throw -> [${out.join(",")}]`);
  } catch (e) { console.log(`  ${label}: THREW -> ${(e as Error).message}`); }
}
async function nonStream(label: string, adapter: any, body: unknown) {
  try {
    const ev = await adapter.parseResponse(new Response(JSON.stringify(body)));
    console.log(`  ${label}: no throw -> ${ev.map((e: any) => e.type).join(",")}`);
  } catch (e) { console.log(`  ${label}: THREW -> ${(e as Error).message}`); }
}

test("nested shape probe", async () => {
  await stream("A google candidates:[null]   ", withTestTranslatorBudget(createGoogleAdapter(g)), 'data: {"candidates":[null]}\n\n');
  await stream("B openai tool_calls object   ", withTestTranslatorBudget(createOpenAIChatAdapter(o)), 'data: {"choices":[{"delta":{"tool_calls":{"a":1}}}]}\n\n');
  await stream("C openai tool_calls [null]   ", withTestTranslatorBudget(createOpenAIChatAdapter(o)), 'data: {"choices":[{"delta":{"tool_calls":[null]}}]}\n\n');
  const oa = withTestTranslatorBudget(createOpenAIChatAdapter(o));
  await nonStream("D openai NS tool_calls object", oa, { choices: [{ message: { role: "assistant", tool_calls: { a: 1 } } }] });
  await nonStream("E openai NS tool_calls [null]", oa, { choices: [{ message: { role: "assistant", tool_calls: [null] } }] });
  await nonStream("F openai NS no .function    ", oa, { choices: [{ message: { role: "assistant", tool_calls: [{ id: "x" }] } }] });
  await nonStream("G google NS candidates:[null]", withTestTranslatorBudget(createGoogleAdapter(g)), { candidates: [null] });
});

Logs or error output

A google candidates:[null]   : THREW -> null is not an object (evaluating 'candidates[0].finishReason')
B openai tool_calls object   : THREW -> {} is not iterable
C openai tool_calls [null]   : THREW -> null is not an object (evaluating 'tc.index')
D openai NS tool_calls object: THREW -> {} is not iterable
E openai NS tool_calls [null]: THREW -> null is not an object (evaluating 'tc.id')
F openai NS no .function     : THREW -> undefined is not an object (evaluating 'tc.function.name')
G google NS candidates:[null]: no throw -> done          <- correctly guarded, no change needed

Version

dev at 79831c90 (with #1240 merged as 2f0dc7cb).

Operating system

Windows 11. Not platform-specific — the adapters are driven directly, no network and no config.

Provider and model

None. Reproduced adapter-locally for openai-chat and google.

Redacted configuration

Not applicable — synthetic Response objects, no config and no credentials.

Additional note on how to fix

Deliberately not opening a PR for this yet. The shape of the fix is a real decision, not a mechanical one, and it is yours to make:

  • the #1240 precedent for the frame root was skip, do not terminate, because the frame turned out to be benign padding. For a malformed tool_calls the safer default is probably the opposite, since silently dropping a tool call can strand a turn — but that is a judgement about which failure is worse, and I would rather not guess it into the codebase;
  • google.ts:809 shows the cheapest idiom already in use here (optional chaining), which may be all A needs.

Happy to write it once you say which way you want it, or to leave it entirely. Also happy for it to be closed as won't-fix given there is no field evidence — the audit is on the record either way.

Checks

  • I searched existing issues and documentation.
  • I removed secrets, tokens, account details, request credentials, and personal data.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingtoolstool_calls, MCP, web-search / sidecar tools

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions