From fef024a69cfbe735d3ce0a6d33e65e911461bd2d Mon Sep 17 00:00:00 2001 From: Hako <25837994+devswha@users.noreply.github.com> Date: Sat, 5 Sep 2026 23:42:15 +0900 Subject: [PATCH 1/4] fix(adapters): retain late chat tool-call index aliases (cherry picked from commit c8240c51d664f7cfb790b6d60679adfe0490b5c9) --- .../src/content/docs/reference/adapters.md | 5 + src/adapters/openai-chat.ts | 15 ++- .../openai-chat-parallel-stream.test.ts | 126 +++++++++++++++++- 3 files changed, 135 insertions(+), 11 deletions(-) diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 1db98357d3..930c4d8d15 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -49,6 +49,11 @@ provider — xAI, Kimi, DeepSeek, GLM, Groq, OpenRouter, Ollama (local), and mor tiers, accepts reasoning deltas from either `delta.reasoning_content` or `delta.reasoning`, requests streamed usage with `stream_options.include_usage`, and reads usage from non-stream response envelopes. +Streaming tool calls retain their identity when a provider first sends an ID, +then associates that ID with an index, and later sends index-only argument +fragments. Those fragments assemble into one call with the original name and +complete arguments; parallel calls retain separate identities. + ## `ollama-native` **Targets:** Ollama's own **Chat API** (`POST /api/chat`) rather than its OpenAI-compatible diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 8b7d9c8614..2620ce36f6 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1660,6 +1660,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd let bufferBytes = 0; interface PendingToolCall { key: string; + indexKey?: string; id: string; name: string; args: string; @@ -1853,12 +1854,12 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd // streamers repeat an already-sent field as a non-string placeholder on a // continuation delta; judging first meant the whole stream died with a 502 even // though the value being repeated was already held in canonical form. - const key = typeof rawIndex === "number" - ? `i:${rawIndex}` - : idDelta - ? `id:${idDelta}` - : pendingToolCalls[pendingToolCalls.length - 1]?.key; + const indexKey = typeof rawIndex === "number" ? `i:${rawIndex}` : undefined; + const key = indexKey ?? (idDelta + ? `id:${idDelta}` + : pendingToolCalls[pendingToolCalls.length - 1]?.key); let call = key !== undefined ? pendingToolCalls.find(c => c.key === key) : undefined; + if (!call && indexKey !== undefined) call = pendingToolCalls.find(c => c.indexKey === indexKey); if (!call && idDelta) call = pendingToolCalls.find(c => c.id === idDelta); if (!call) { call = { @@ -1872,6 +1873,10 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd pendingToolCalls.push(call); budget.openCall(call.key); } + // An ID-only call may learn its index from a later ID+index fragment. Retain that + // alias without changing the key that owns its argument budget. Only the first + // observed index binds: a repeated ID on a different index must not alias both. + if (indexKey !== undefined && call.indexKey === undefined) call.indexKey = indexKey; // Tolerance is per FIELD, keyed on that field's own provenance. A canonical name // says nothing about whether `arguments` was ever sent as a string, so it cannot diff --git a/tests/adapters/openai/openai-chat-parallel-stream.test.ts b/tests/adapters/openai/openai-chat-parallel-stream.test.ts index 238faa9082..ec1e3d55cd 100644 --- a/tests/adapters/openai/openai-chat-parallel-stream.test.ts +++ b/tests/adapters/openai/openai-chat-parallel-stream.test.ts @@ -1,16 +1,17 @@ import { describe, expect, test } from "bun:test"; import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../../../src/adapters/openai-chat"; +import type { TranslatorBudget } from "../../../src/lib/translator-budget"; import type { AdapterEvent } from "../../../src/types"; -import { withTestTranslatorBudget } from "../../helpers/translator-budget"; +import { createTestTranslatorBudget, withTestTranslatorBudget } from "../../helpers/translator-budget"; const createOpenAIChatAdapter = (...args: Parameters) => withTestTranslatorBudget(createOpenAIChatAdapterProduction(...args)); const provider = { adapter: "openai-chat", baseUrl: "https://example.test/v1", apiKey: "key" }; -async function collect(body: string): Promise { +async function collect(body: string, budget?: TranslatorBudget): Promise { const out: AdapterEvent[] = []; - for await (const e of createOpenAIChatAdapter(provider).parseStream(new Response(body))) out.push(e); + for await (const e of createOpenAIChatAdapter(provider).parseStream(new Response(body), budget)) out.push(e); return out; } @@ -211,12 +212,125 @@ describe("openai-chat parallel tool call stream assembly", () => { expect(assembled(events)).toEqual([{ id: "call_a", name: "shell", args: "{\"cmd\":\"ls\"}" }]); }); - test("T9b: id-only first chunk followed by index+id continuation stays ONE call", async () => { + test("T9b: id-only call retains a later index for index-only continuation", async () => { + const budget = createTestTranslatorBudget(); const events = await collect(sse([ chunkOf([{ id: "call_b", function: { name: "read", arguments: "{\"p\"" } }]), - chunkOf([{ index: 0, id: "call_b", function: { arguments: ":\"x\"}" } }]), + chunkOf([{ index: 0, id: "call_b", function: { arguments: ":\"x\"" } }]), + chunkOf([{ index: 0, function: { arguments: "}" } }]), chunkOf([], "tool_calls"), - ])); + ]), budget); expect(assembled(events)).toEqual([{ id: "call_b", name: "read", args: "{\"p\":\"x\"}" }]); + expect(events.at(-1)?.type).toBe("done"); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + + test("late indexes keep interleaved calls separate without adding budget owners", async () => { + const budget = createTestTranslatorBudget(); + const response = new Response(sse([ + chunkOf([ + { id: "call_a", function: { name: "read", arguments: "{\"p\":" } }, + { id: "call_b", function: { name: "write", arguments: "{\"p\":" } }, + ]), + chunkOf([{ index: 9, id: "call_b", function: { arguments: "\"b\"" } }]), + chunkOf([{ index: 4, id: "call_a", function: { arguments: "\"a\"" } }]), + chunkOf([{ index: 9, function: { arguments: "}" } }]), + chunkOf([{ index: 4, function: { arguments: "}" } }]), + chunkOf([{ id: "call_a", function: { arguments: " " } }]), + chunkOf([], "tool_calls"), + ])); + const events: AdapterEvent[] = []; + let maxActiveCalls = 0; + for await (const event of createOpenAIChatAdapter(provider).parseStream(response, budget)) { + events.push(event); + maxActiveCalls = Math.max(maxActiveCalls, budget.snapshot().activeCalls); + } + expect(assembled(events)).toEqual([ + { id: "call_a", name: "read", args: "{\"p\":\"a\"} " }, + { id: "call_b", name: "write", args: "{\"p\":\"b\"}" }, + ]); + expect(events.at(-1)?.type).toBe("done"); + expect(maxActiveCalls).toBe(2); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + + test("index-only fragments do not guess an association between unindexed calls", async () => { + const events = await collect(sse([ + chunkOf([ + { id: "call_a", function: { name: "read", arguments: "{\"p\":" } }, + { id: "call_b", function: { name: "write", arguments: "{\"p\":" } }, + ]), + chunkOf([{ index: 0, function: { arguments: "\"a\"}" } }]), + chunkOf([{ index: 1, function: { arguments: "\"b\"}" } }]), + chunkOf([], "tool_calls"), + ])); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(event => event.type === "done")).toBe(false); + }); + + test("an observed index wins over a conflicting ID without rebinding either call", async () => { + const events = await collect(sse([ + chunkOf([{ id: "call_a", function: { name: "read", arguments: "{\"p\":" } }]), + chunkOf([{ index: 1, id: "call_b", function: { name: "write", arguments: "{\"p\":" } }]), + chunkOf([{ index: 0, id: "call_a", function: { arguments: "\"a\"" } }]), + chunkOf([{ index: 1, id: "call_a", function: { arguments: "\"b\"}" } }]), + chunkOf([{ index: 0, id: "call_b", function: { arguments: "}" } }]), + chunkOf([{ index: 0, function: { arguments: " " } }]), + chunkOf([], "tool_calls"), + ])); + expect(assembled(events)).toEqual([ + { id: "call_a", name: "read", args: "{\"p\":\"a\"} " }, + { id: "call_b", name: "write", args: "{\"p\":\"b\"}" }, + ]); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("duplicate IDs on established indexed calls keep first-match ID fallback", async () => { + const events = await collect(sse([ + chunkOf([ + { index: 0, function: { name: "read", arguments: "{\"p\":" } }, + { index: 1, function: { name: "write", arguments: "{\"p\":" } }, + ]), + chunkOf([{ index: 0, id: "shared", function: { arguments: "\"a\"" } }]), + chunkOf([{ index: 1, id: "shared", function: { arguments: "\"b\"" } }]), + chunkOf([{ id: "shared", function: { arguments: "}" } }]), + chunkOf([{ index: 1, function: { arguments: "}" } }]), + chunkOf([], "tool_calls"), + ])); + expect(assembled(events)).toEqual([ + { id: "shared", name: "read", args: "{\"p\":\"a\"}" }, + { id: "shared", name: "write", args: "{\"p\":\"b\"}" }, + ]); + expect(events.at(-1)?.type).toBe("done"); + }); + + test("a repeated ID on a different index does not replace the first observed alias", async () => { + const events = await collect(sse([ + chunkOf([{ id: "call_a", function: { name: "read", arguments: "{\"p\":" } }]), + chunkOf([{ index: 0, id: "call_a", function: { arguments: "\"a\"" } }]), + chunkOf([{ index: 1, id: "call_a", function: { arguments: "" } }]), + chunkOf([{ index: 0, function: { arguments: "}" } }]), + chunkOf([], "tool_calls"), + ])); + expect(assembled(events)).toEqual([{ id: "call_a", name: "read", args: "{\"p\":\"a\"}" }]); + expect(events.at(-1)?.type).toBe("done"); + }); + + test.each([9, 10])("late index preserves a %i-byte argument limit across all fragments", async limit => { + const budget = createTestTranslatorBudget({ maxCallArgumentBytes: limit }); + const events = await collect(sse([ + chunkOf([{ id: "call_a", function: { name: "read", arguments: "{\"p\":" } }]), + chunkOf([{ index: 0, id: "call_a", function: { arguments: "\"é\"" } }]), + chunkOf([{ index: 0, function: { arguments: "}" } }]), + chunkOf([], "tool_calls"), + ]), budget); + if (limit === 9) { + expect(events.at(-1)).toMatchObject({ type: "error", code: "translation_buffer_limit" }); + expect(events.some(event => event.type === "tool_call_start")).toBe(false); + } else { + expect(assembled(events)).toEqual([{ id: "call_a", name: "read", args: "{\"p\":\"é\"}" }]); + expect(events.at(-1)?.type).toBe("done"); + } + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: limit === 9 ? 1 : 0 }); }); }); From c79db69d4f384bc4bcdaec24946da287fcac0b0f Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 02:57:45 +0900 Subject: [PATCH 2/4] fix(adapters): reject invalid numeric indexes before tool matching Co-authored-by: Hako <25837994+devswha@users.noreply.github.com> --- .../021_tool_alias_refresh.md | 7 +++ .../src/content/docs/reference/adapters.md | 2 + src/adapters/openai-chat.ts | 11 ++++- structure/04_transports-and-sidecars.md | 16 +++++++ .../openai-chat-parallel-stream.test.ts | 45 +++++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md diff --git a/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md b/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md new file mode 100644 index 0000000000..667aae674e --- /dev/null +++ b/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md @@ -0,0 +1,7 @@ +# Tool-call alias cycle P refresh + +Current parent: 22da7a4bc80040f66b819239c5028e578f9a1ede, after TOML delivery. Original source c8240c51d664f7cfb790b6d60679adfe0490b5c9 remains open and authored by Hako. Relevant baseline comparison is retained in scratch; implementation uses the current tree and preserves adjacent changes. + +Apply the original commit, then the independently reviewed 020 numeric-index amendment. Missing/non-numeric placeholders keep existing tolerance; negative/fractional numeric indexes terminate before matching. Preserve the immutable reservation key and first observed valid index alias. Add direct malformed-index activation coverage alongside all original positive/collision/UTF-8 budget cases. Update the transport structure contract as planned. + +Main owns cherry-pick/commits/PR/CI/merge. An inherited worker may edit only src/adapters/openai-chat.ts, tests/adapters/openai/openai-chat-parallel-stream.test.ts, and structure/04_transports-and-sidecars.md after A passes. Main owns this document and all other files. Independent reviewer checks resulting code; all tests/typechecks execute remotely or in GitHub Actions. Full-suite readiness remains remote; no local application checks. macmini shared test lock is respected. diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 930c4d8d15..62440679b2 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -53,6 +53,8 @@ Streaming tool calls retain their identity when a provider first sends an ID, then associates that ID with an index, and later sends index-only argument fragments. Those fragments assemble into one call with the original name and complete arguments; parallel calls retain separate identities. +Numeric streamed tool-call indexes must be non-negative integers; malformed numeric +indexes terminate the stream with an upstream error before identity matching. ## `ollama-native` diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 2620ce36f6..76c9d7e931 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1849,8 +1849,17 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const rawId = rawToolCall.id; const idDelta = typeof rawId === "string" ? rawId : ""; const rawIndex = rawToolCall.index; + // Invalid numeric indexes must not fall through to ID or last-call matching. + // Reject before an alias can bind or any pending call can consume the fragment. + if (typeof rawIndex === "number" + && (!Number.isInteger(rawIndex) || rawIndex < 0)) { + return yield* terminateWithError({ + ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), + message: "upstream response contained invalid tool calls (invalid numeric index)", + }); + } - // Resolve the pending call BEFORE judging the fields. Some OpenAI-compatible + // Resolve the pending call BEFORE judging repeated string fields. Some OpenAI-compatible // streamers repeat an already-sent field as a non-string placeholder on a // continuation delta; judging first meant the whole stream died with a 502 even // though the value being repeated was already held in canonical form. diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index 4ee22c114c..fa4258a800 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -1502,6 +1502,22 @@ shares the 12-image active cap. Bounded source labels are emitted in active user root pruning cannot erase attachment provenance; the same text participates in token estimation. Native Composer/MCP behavior and text-only historical replay remain unchanged. +## Chat streamed tool-call identity + +`src/adapters/openai-chat.ts` retains a call's first observed non-negative integer +index as an alias when the call started by ID. Numeric indexes that are negative +or non-integer terminate the stream before any key, alias, ID or last-call matching. +The invalid-index error releases all pending call reservations without emitting +those calls or a successful completion; invalid indexes are never treated as absent. +Missing and non-numeric index placeholders retain their existing tolerance. + +For valid indexes, lookup preserves direct-key precedence, then index alias, then +ID fallback. The initial key continues to own all translator budget reservations +and release; learning an alias creates no additional owner. Unassociated index-only +fragments are not guessed onto pending ID-only calls. +`tests/adapters/openai/openai-chat-parallel-stream.test.ts` covers late aliases, +parallel/colliding identities, invalid numeric indexes and UTF-8 byte-limit boundaries. + ## Sidecars Web search and vision sidecars run only when the main request needs that capability and a usable diff --git a/tests/adapters/openai/openai-chat-parallel-stream.test.ts b/tests/adapters/openai/openai-chat-parallel-stream.test.ts index ec1e3d55cd..3e65b8534e 100644 --- a/tests/adapters/openai/openai-chat-parallel-stream.test.ts +++ b/tests/adapters/openai/openai-chat-parallel-stream.test.ts @@ -268,6 +268,51 @@ describe("openai-chat parallel tool call stream assembly", () => { expect(events.some(event => event.type === "done")).toBe(false); }); + test.each([ + [-1, undefined], + [-1, "call_a"], + [0.5, undefined], + [0.5, "call_a"], + ] as const)("invalid numeric index %s with ID %s aborts without reassigning pending calls", async (index, id) => { + const budget = createTestTranslatorBudget(); + const response = new Response(sse([ + chunkOf([ + { id: "call_a", function: { name: "read", arguments: '{"p":"a"}' } }, + { id: "call_b", function: { name: "write", arguments: '{"p":"b"}' } }, + ]), + chunkOf([{ index: 0, id: "call_a", function: { arguments: "" } }]), + // Whitespace keeps either complete JSON argument valid if the invalid index is + // mistakenly ignored and this fragment falls back to its ID or the last call. + chunkOf([{ index, id, function: { arguments: " " } }]), + chunkOf([{ index: 0, function: { arguments: " " } }]), + chunkOf([], "tool_calls"), + ])); + const events: AdapterEvent[] = []; + let sawBothPendingReservations = false; + for await (const event of createOpenAIChatAdapter(provider).parseStream(response, budget)) { + events.push(event); + const snapshot = budget.snapshot(); + // Each ASCII JSON argument is nine bytes; the valid alias heartbeat observes + // both retained reservations before the malformed continuation arrives. + sawBothPendingReservations ||= snapshot.activeCalls === 2 && snapshot.currentBytes === 18; + if (event.type === "error") { + expect(snapshot).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + } + } + expect(sawBothPendingReservations).toBe(true); + expect(events.filter(event => event.type === "error")).toEqual([expect.objectContaining({ + type: "error", + status: 502, + errorType: "upstream_error", + message: "upstream response contained invalid tool calls (invalid numeric index)", + })]); + expect(events.at(-1)?.type).toBe("error"); + expect(events.some(event => event.type === "done")).toBe(false); + expect(events.some(event => event.type === "tool_call_start" + || event.type === "tool_call_delta" || event.type === "tool_call_end")).toBe(false); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + test("an observed index wins over a conflicting ID without rebinding either call", async () => { const events = await collect(sse([ chunkOf([{ id: "call_a", function: { name: "read", arguments: "{\"p\":" } }]), From d6bfb044a5dc6494cba57c1238ded7c23faf5586 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 03:25:21 +0900 Subject: [PATCH 3/4] fix(adapters): reject unsafe integer tool-call indexes Co-authored-by: Hako <25837994+devswha@users.noreply.github.com> --- .../020_tool_aliases.md | 8 ++- .../src/content/docs/reference/adapters.md | 2 +- src/adapters/openai-chat.ts | 3 +- structure/04_transports-and-sidecars.md | 11 ++-- .../openai-chat-parallel-stream.test.ts | 55 +++++++++++++++++++ 5 files changed, 71 insertions(+), 8 deletions(-) diff --git a/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md b/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md index 54ecb53073..845fd6fec7 100644 --- a/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md +++ b/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md @@ -44,7 +44,7 @@ Current anchors: `src/adapters/openai-chat.ts:1661` pending interface, `:1856` i ```ts if (typeof rawIndex === "number" - && (!Number.isInteger(rawIndex) || rawIndex < 0)) { + && (!Number.isSafeInteger(rawIndex) || rawIndex < 0)) { return yield* terminateWithError({ ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), message: "upstream response contained invalid tool calls (invalid numeric index)", @@ -121,4 +121,8 @@ The implementation cycle certifies its published current-head candidate. Every d ## External review amendment: numeric index contract -Only non-negative integer indexes may become an alias. Immediately after reading rawIndex, if it is numeric but not an integer or is negative, terminate through the existing invalidToolCallsEvent/terminateWithError path; do not treat an invalid numeric index as absent and append its data to the last pending call. Other tolerated placeholder fields retain their existing rules. Add reachable negative/fractional numeric-index regressions with two distinct pending calls: one error, no done, no fragment reassignment, and all budget reservations released. Preserve all original positive and collision cases. This is an explicit source-patch amendment, not a claim the original commit already implements validation. +Only non-negative safe-integer indexes may become an alias. Immediately after reading rawIndex, if it is numeric but not an integer or is negative, terminate through the existing invalidToolCallsEvent/terminateWithError path; do not treat an invalid numeric index as absent and append its data to the last pending call. Other tolerated placeholder fields retain their existing rules. Add reachable negative/fractional numeric-index regressions with two distinct pending calls: one error, no done, no fragment reassignment, and all budget reservations released. Preserve all original positive and collision cases. This is an explicit source-patch amendment, not a claim the original commit already implements validation. + +## Safe-integer review repair + +The numeric guard uses Number.isSafeInteger: parsed indices beyond the safe range can already have lost identity precision. Add a raw-wire regression containing distinct large integer literals (not JS values rounded before serialization), and retain a positive MAX_SAFE_INTEGER boundary. Capture error/no tool success plus existing reservation-release coverage. The correction must be verified in this same unit; no original source tests are removed. diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 62440679b2..64d5278ec6 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -53,7 +53,7 @@ Streaming tool calls retain their identity when a provider first sends an ID, then associates that ID with an index, and later sends index-only argument fragments. Those fragments assemble into one call with the original name and complete arguments; parallel calls retain separate identities. -Numeric streamed tool-call indexes must be non-negative integers; malformed numeric +Numeric streamed tool-call indexes must be non-negative safe integers; malformed numeric indexes terminate the stream with an upstream error before identity matching. ## `ollama-native` diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 76c9d7e931..66db05167b 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1850,9 +1850,10 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const idDelta = typeof rawId === "string" ? rawId : ""; const rawIndex = rawToolCall.index; // Invalid numeric indexes must not fall through to ID or last-call matching. + // Unsafe integers can collapse distinct wire indexes onto the same JS number. // Reject before an alias can bind or any pending call can consume the fragment. if (typeof rawIndex === "number" - && (!Number.isInteger(rawIndex) || rawIndex < 0)) { + && (!Number.isSafeInteger(rawIndex) || rawIndex < 0)) { return yield* terminateWithError({ ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), message: "upstream response contained invalid tool calls (invalid numeric index)", diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index fa4258a800..be179a2682 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -1504,9 +1504,11 @@ Native Composer/MCP behavior and text-only historical replay remain unchanged. ## Chat streamed tool-call identity -`src/adapters/openai-chat.ts` retains a call's first observed non-negative integer -index as an alias when the call started by ID. Numeric indexes that are negative -or non-integer terminate the stream before any key, alias, ID or last-call matching. +`src/adapters/openai-chat.ts` retains a call's first observed non-negative safe integer +index as an alias when the call started by ID. Numeric indexes that are negative, +non-integer or outside JavaScript's safe-integer range terminate the stream before +any key, alias, ID or last-call matching. `Number.MAX_SAFE_INTEGER` is accepted; +larger integers are rejected because distinct wire literals can parse to the same number. The invalid-index error releases all pending call reservations without emitting those calls or a successful completion; invalid indexes are never treated as absent. Missing and non-numeric index placeholders retain their existing tolerance. @@ -1516,7 +1518,8 @@ ID fallback. The initial key continues to own all translator budget reservations and release; learning an alias creates no additional owner. Unassociated index-only fragments are not guessed onto pending ID-only calls. `tests/adapters/openai/openai-chat-parallel-stream.test.ts` covers late aliases, -parallel/colliding identities, invalid numeric indexes and UTF-8 byte-limit boundaries. +parallel/colliding identities, distinct unsafe raw JSON index literals, the maximum +safe-integer boundary, invalid numeric indexes and UTF-8 byte-limit boundaries. ## Sidecars diff --git a/tests/adapters/openai/openai-chat-parallel-stream.test.ts b/tests/adapters/openai/openai-chat-parallel-stream.test.ts index 3e65b8534e..85f49e8f21 100644 --- a/tests/adapters/openai/openai-chat-parallel-stream.test.ts +++ b/tests/adapters/openai/openai-chat-parallel-stream.test.ts @@ -313,6 +313,61 @@ describe("openai-chat parallel tool call stream assembly", () => { expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); }); + test("rejects distinct unsafe raw JSON indexes before they collapse into one call", async () => { + const budget = createTestTranslatorBudget(); + // Keep both index literals on the wire: constructing JS numbers before JSON.stringify + // would already round 9007199254740993 to 9007199254740992. Without rejection, + // both whitespace fragments would silently join call_a's valid JSON despite call_b's ID/name. + const response = new Response(String.raw`data: {"choices":[{"delta":{"tool_calls":[{"id":"call_a","function":{"name":"read","arguments":"{}"}}]}}]} + +data: {"choices":[{"delta":{"tool_calls":[{"id":"call_a","function":{"arguments":""}}]}}]} + +data: {"choices":[{"delta":{"tool_calls":[{"index":9007199254740992,"id":"call_a","function":{"name":"read","arguments":" "}}]}}]} + +data: {"choices":[{"delta":{"tool_calls":[{"index":9007199254740993,"id":"call_b","function":{"name":"write","arguments":" "}}]}}]} + +data: {"choices":[{"delta":{"tool_calls":[]},"finish_reason":"tool_calls"}]} + +data: [DONE] + +`); + const events: AdapterEvent[] = []; + let sawPendingReservation = false; + for await (const event of createOpenAIChatAdapter(provider).parseStream(response, budget)) { + events.push(event); + const snapshot = budget.snapshot(); + sawPendingReservation ||= snapshot.activeCalls === 1 && snapshot.currentBytes === 2; + if (event.type === "error") { + expect(snapshot).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + } + } + expect(sawPendingReservation).toBe(true); + // The first unsafe index terminates before either unsafe fragment emits a heartbeat, + // a tool call, or done; the buffered reservation is released at the error itself. + expect(events.map(event => event.type)).toEqual(["heartbeat", "heartbeat", "error"]); + expect(events.at(-1)).toMatchObject({ + type: "error", + status: 502, + errorType: "upstream_error", + message: "upstream response contained invalid tool calls (invalid numeric index)", + }); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + + test("retains a late MAX_SAFE_INTEGER alias for index-only continuation", async () => { + const budget = createTestTranslatorBudget(); + const events = await collect(sse([ + chunkOf([{ id: "call_boundary", function: { name: "read", arguments: '{"p":' } }]), + chunkOf([{ index: Number.MAX_SAFE_INTEGER, id: "call_boundary", function: { arguments: '"x"' } }]), + chunkOf([{ index: Number.MAX_SAFE_INTEGER, function: { arguments: "}" } }]), + chunkOf([], "tool_calls"), + ]), budget); + expect(assembled(events)).toEqual([{ id: "call_boundary", name: "read", args: '{"p":"x"}' }]); + expect(events.some(event => event.type === "error")).toBe(false); + expect(events.at(-1)?.type).toBe("done"); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + test("an observed index wins over a conflicting ID without rebinding either call", async () => { const events = await collect(sse([ chunkOf([{ id: "call_a", function: { name: "read", arguments: "{\"p\":" } }]), From baf8303bbda32335424afc8fe2065950f04f6d9b Mon Sep 17 00:00:00 2001 From: t Date: Sun, 6 Sep 2026 04:29:08 +0900 Subject: [PATCH 4/4] fix(adapters): reject every invalid claimed tool-call index type Co-authored-by: Hako <25837994+devswha@users.noreply.github.com> --- .../020_tool_aliases.md | 12 ++++-- .../021_tool_alias_refresh.md | 2 + .../022_index_type_repair.md | 41 +++++++++++++++++++ .../src/content/docs/reference/adapters.md | 6 ++- src/adapters/openai-chat.ts | 10 +++-- structure/04_transports-and-sidecars.md | 13 +++--- .../openai-chat-parallel-stream.test.ts | 40 ++++++++++++++---- 7 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 devlog/_plan/260906_d_integrations_delivery/022_index_type_repair.md diff --git a/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md b/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md index 845fd6fec7..9d1b646314 100644 --- a/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md +++ b/devlog/_plan/260906_d_integrations_delivery/020_tool_aliases.md @@ -43,11 +43,13 @@ The exact original patch is the complete diff `git show c8240c51d664f7cfb790b6d6 Current anchors: `src/adapters/openai-chat.ts:1661` pending interface, `:1856` identity lookup, `:1873` budget opening, `:1912` argument-byte accounting, `:1679` budget closing. Replace the lookup block with: ```ts -if (typeof rawIndex === "number" - && (!Number.isSafeInteger(rawIndex) || rawIndex < 0)) { +if (rawIndex !== undefined && rawIndex !== null + && (typeof rawIndex !== "number" + || !Number.isSafeInteger(rawIndex) + || rawIndex < 0)) { return yield* terminateWithError({ ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), - message: "upstream response contained invalid tool calls (invalid numeric index)", + message: "upstream response contained invalid tool calls (invalid index)", }); } const indexKey = typeof rawIndex === "number" ? `i:${rawIndex}` : undefined; @@ -126,3 +128,7 @@ Only non-negative safe-integer indexes may become an alias. Immediately after re ## Safe-integer review repair The numeric guard uses Number.isSafeInteger: parsed indices beyond the safe range can already have lost identity precision. Add a raw-wire regression containing distinct large integer literals (not JS values rounded before serialization), and retain a positive MAX_SAFE_INTEGER boundary. Capture error/no tool success plus existing reservation-release coverage. The correction must be verified in this same unit; no original source tests are removed. + +## Claimed-type boundary update + +022 supersedes the earlier non-numeric-index tolerance assumption: only missing/null indexes are absent. Every other claimed value must be a non-negative safe integer; no coercion of strings/objects/bools/arrays. Repeated ID/name/argument-field tolerance is unchanged. diff --git a/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md b/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md index 667aae674e..c8d03ce3f5 100644 --- a/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md +++ b/devlog/_plan/260906_d_integrations_delivery/021_tool_alias_refresh.md @@ -1,5 +1,7 @@ # Tool-call alias cycle P refresh +Historical refresh: its non-numeric-index policy is superseded by the explicit null/missing boundary in 022_index_type_repair.md. + Current parent: 22da7a4bc80040f66b819239c5028e578f9a1ede, after TOML delivery. Original source c8240c51d664f7cfb790b6d60679adfe0490b5c9 remains open and authored by Hako. Relevant baseline comparison is retained in scratch; implementation uses the current tree and preserves adjacent changes. Apply the original commit, then the independently reviewed 020 numeric-index amendment. Missing/non-numeric placeholders keep existing tolerance; negative/fractional numeric indexes terminate before matching. Preserve the immutable reservation key and first observed valid index alias. Add direct malformed-index activation coverage alongside all original positive/collision/UTF-8 budget cases. Update the transport structure contract as planned. diff --git a/devlog/_plan/260906_d_integrations_delivery/022_index_type_repair.md b/devlog/_plan/260906_d_integrations_delivery/022_index_type_repair.md new file mode 100644 index 0000000000..704278ae34 --- /dev/null +++ b/devlog/_plan/260906_d_integrations_delivery/022_index_type_repair.md @@ -0,0 +1,41 @@ +# 022 — Reject claimed invalid index types + +## Loop specification + +Class C2/C3 bounded parent repair. Source: current #3702 at d6bfb044a; late reviews PRRT_kwDOS-0Gi86fl4vM and fl4vC. Goal: a present invalid index cannot be mistaken for an absent index and routed to the last pending call. Non-goals: changing repeated ID/name/argument placeholder tolerance, parsing numeric strings, new adapters or unrelated Logs work. Remote/CI verification only; no local tests/typecheck. Same session resource bounds apply. Main owns Git/FSM/integration; one worker may edit only the adapter, its parallel-stream test and structure04. Main reclaims after two failed delegates. + +This additive repair preempts unfinished Logs planning. No previous work-phase completion marks or final criteria were removed. Detailed review synthesis is in scratch. Resume Logs after this full cycle and cascade. + +## Exact change map + +MODIFY src/adapters/openai-chat.ts, before all key matching: + +```ts +if (rawIndex !== undefined && rawIndex !== null + && (typeof rawIndex !== "number" + || !Number.isSafeInteger(rawIndex) + || rawIndex < 0)) { + return yield* terminateWithError({ + ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), + message: "upstream response contained invalid tool calls (invalid index)", + }); +} +``` + +Before: only invalid numbers reject; present strings/objects/bools become no indexKey and may select the last pending call. After: only missing/null is absent; every other claimed index must be a non-negative safe integer. The existing terminateWithError closes all budget reservations before the error is yielded. Keep the alias/key precedence and immutable reservation keys unchanged. No new fields/enums/dependencies. + +MODIFY tests/adapters/openai/openai-chat-parallel-stream.test.ts: retain all Hako and safe-integer cases. Update expected diagnostic wording. Add labeled table cases for numeric string, empty string, true/false, object and array, with pending complete JSON calls so a silent fallback could otherwise produce success; assert one terminal502, no tool/done event and released reservations. Include explicit missing/null positive continuation through a later valid numeric alias. Use tuple wrappers for array-valued cases so test.each cannot mistake an index array for argument tuples. + +MODIFY docs-site/src/content/docs/reference/adapters.md: specify non-negative safe integers; explicitly reject non-numeric values and negative/fractional/unsafe numbers; missing/null remain absent-index placeholders. Do not call valid JSON numbers malformed JSON. + +MODIFY structure/04_transports-and-sidecars.md: align the same index contract and source/test ownership. + +MODIFY 020_tool_aliases.md: carry the corrected guard and compatibility boundary. Annotate 021's former non-numeric-placeholder policy as superseded by this repair; retain its historical source snapshot. + +## Verification and exit + +- Independent plan and implementation review; original source authorship retained. +- Exact-head pinned remote typecheck/full suite/docs build, hosted CI registration and no unresolved findings. Full final integrated CI remains mandatory under c-2; build readiness is not merge permission. +- Existing numeric/unsafe/UTF-8/collision cases remain green; new claimed-type cases actually observe pending allocations before early failure, and null/missing positive cases still assemble one correct tool. +- Cascade new parent into Cursor with a merge preserving both authors' commits and both structure sections; fast-forward the still-unpublished Logs branch to updated Cursor. Verify both ancestry edges. Do not mark the updated Cursor head verified until its own new evidence exists. +- Main returns to parent for the repair receipt/D, then resumes original Logs planning. Shipping #3702 still requires strict merge verification and actual dev ancestry before source #3673 closes. diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 64d5278ec6..5705113e9e 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -53,8 +53,10 @@ Streaming tool calls retain their identity when a provider first sends an ID, then associates that ID with an index, and later sends index-only argument fragments. Those fragments assemble into one call with the original name and complete arguments; parallel calls retain separate identities. -Numeric streamed tool-call indexes must be non-negative safe integers; malformed numeric -indexes terminate the stream with an upstream error before identity matching. +When present, streamed tool-call indexes must be non-negative safe integers. Non-numeric +values and negative, fractional, or unsafe numbers terminate the stream with an upstream +error before identity matching. Missing and null indexes remain absent-index placeholders; +numeric strings are not coerced. ## `ollama-native` diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 66db05167b..0abb3277ae 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -1849,14 +1849,16 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd const rawId = rawToolCall.id; const idDelta = typeof rawId === "string" ? rawId : ""; const rawIndex = rawToolCall.index; - // Invalid numeric indexes must not fall through to ID or last-call matching. + // Only missing/null indexes are absent; every claimed index must be valid. // Unsafe integers can collapse distinct wire indexes onto the same JS number. // Reject before an alias can bind or any pending call can consume the fragment. - if (typeof rawIndex === "number" - && (!Number.isSafeInteger(rawIndex) || rawIndex < 0)) { + if (rawIndex !== undefined && rawIndex !== null + && (typeof rawIndex !== "number" + || !Number.isSafeInteger(rawIndex) + || rawIndex < 0)) { return yield* terminateWithError({ ...invalidToolCallsEvent(rawToolCalls, "stream", pendingUsage), - message: "upstream response contained invalid tool calls (invalid numeric index)", + message: "upstream response contained invalid tool calls (invalid index)", }); } diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index be179a2682..a7b563378a 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -1505,13 +1505,15 @@ Native Composer/MCP behavior and text-only historical replay remain unchanged. ## Chat streamed tool-call identity `src/adapters/openai-chat.ts` retains a call's first observed non-negative safe integer -index as an alias when the call started by ID. Numeric indexes that are negative, -non-integer or outside JavaScript's safe-integer range terminate the stream before -any key, alias, ID or last-call matching. `Number.MAX_SAFE_INTEGER` is accepted; +index as an alias when the call started by ID. Every present, non-null index must +be a number in that range: strings (including numeric and empty strings), booleans, +objects, arrays, negative numbers, fractions and unsafe integers terminate the stream +before any key, alias, ID or last-call matching. `Number.MAX_SAFE_INTEGER` is accepted; larger integers are rejected because distinct wire literals can parse to the same number. The invalid-index error releases all pending call reservations without emitting those calls or a successful completion; invalid indexes are never treated as absent. -Missing and non-numeric index placeholders retain their existing tolerance. +Only missing and null indexes are absent-index placeholders. Repeated ID, name and +argument string-field tolerance retains its existing rules. For valid indexes, lookup preserves direct-key precedence, then index alias, then ID fallback. The initial key continues to own all translator budget reservations @@ -1519,7 +1521,8 @@ and release; learning an alias creates no additional owner. Unassociated index-o fragments are not guessed onto pending ID-only calls. `tests/adapters/openai/openai-chat-parallel-stream.test.ts` covers late aliases, parallel/colliding identities, distinct unsafe raw JSON index literals, the maximum -safe-integer boundary, invalid numeric indexes and UTF-8 byte-limit boundaries. +safe-integer boundary, invalid index types, missing/null continuations and UTF-8 +byte-limit boundaries. ## Sidecars diff --git a/tests/adapters/openai/openai-chat-parallel-stream.test.ts b/tests/adapters/openai/openai-chat-parallel-stream.test.ts index 85f49e8f21..9e47b3a742 100644 --- a/tests/adapters/openai/openai-chat-parallel-stream.test.ts +++ b/tests/adapters/openai/openai-chat-parallel-stream.test.ts @@ -269,11 +269,18 @@ describe("openai-chat parallel tool call stream assembly", () => { }); test.each([ - [-1, undefined], - [-1, "call_a"], - [0.5, undefined], - [0.5, "call_a"], - ] as const)("invalid numeric index %s with ID %s aborts without reassigning pending calls", async (index, id) => { + ["negative, no ID", -1, undefined], + ["negative, matching ID", -1, "call_a"], + ["fractional, no ID", 0.5, undefined], + ["fractional, matching ID", 0.5, "call_a"], + ["numeric string, no ID", "0", undefined], + ["numeric string, matching ID", "0", "call_a"], + ["empty string", "", undefined], + ["true", true, undefined], + ["false", false, undefined], + ["object", {}, undefined], + ["array", [], undefined], + ] as const)("invalid index (%s) aborts without reassigning pending calls", async (_label, index, id) => { const budget = createTestTranslatorBudget(); const response = new Response(sse([ chunkOf([ @@ -304,7 +311,7 @@ describe("openai-chat parallel tool call stream assembly", () => { type: "error", status: 502, errorType: "upstream_error", - message: "upstream response contained invalid tool calls (invalid numeric index)", + message: "upstream response contained invalid tool calls (invalid index)", })]); expect(events.at(-1)?.type).toBe("error"); expect(events.some(event => event.type === "done")).toBe(false); @@ -313,6 +320,25 @@ describe("openai-chat parallel tool call stream assembly", () => { expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); }); + test.each([ + ["missing", undefined], + ["null", null], + ] as const)("%s index placeholders preserve continuation through a later valid alias", async (_label, index) => { + const budget = createTestTranslatorBudget(); + const events = await collect(sse([ + chunkOf([{ index, id: "call_a", function: { name: "read", arguments: '{"p":' } }]), + chunkOf([{ index, id: "call_a", function: { arguments: '"x"' } }]), + chunkOf([{ index: 7, id: "call_a", function: { arguments: "}" } }]), + chunkOf([{ index, function: { arguments: " " } }]), + chunkOf([{ index: 7, function: { arguments: " " } }]), + chunkOf([], "tool_calls"), + ]), budget); + expect(assembled(events)).toEqual([{ id: "call_a", name: "read", args: '{"p":"x"} ' }]); + expect(events.some(event => event.type === "error")).toBe(false); + expect(events.at(-1)?.type).toBe("done"); + expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); + }); + test("rejects distinct unsafe raw JSON indexes before they collapse into one call", async () => { const budget = createTestTranslatorBudget(); // Keep both index literals on the wire: constructing JS numbers before JSON.stringify @@ -349,7 +375,7 @@ data: [DONE] type: "error", status: 502, errorType: "upstream_error", - message: "upstream response contained invalid tool calls (invalid numeric index)", + message: "upstream response contained invalid tool calls (invalid index)", }); expect(budget.snapshot()).toMatchObject({ activeCalls: 0, currentBytes: 0, overflows: 0 }); });