Skip to content

Commit 299cd2f

Browse files
lidge-junWibias
andcommitted
fix(anthropic): keep tool argument deltas incremental and fail malformed ones
The previous commit fixed the stream/non-stream inconsistency by buffering `input_json_delta` fragments until block close. That was the wrong repair and the review caught it: `src/bridge.ts:628` maps every adapter delta to a client-visible `response.function_call_arguments.delta` frame, and `tests/responses-stream-tool-events.test.ts:30` pins that split fragments stay split. Withholding them left a started function call showing empty arguments until the block ended -- a protocol regression traded for a consistency fix. Fragments flow again as they arrive. The assembled copy is still kept, but only to check at `content_block_stop` whether the payload parses. When it does not, the turn errors instead of emitting `tool_call_end`: the fragments are already downstream and cannot be repaired the way `toolUseArguments` repairs the non-stream path, and a client that received `not json` must not then be told the call completed normally. `streamedToolArgumentsParse` treats an empty buffer as valid, because a no-argument tool call sends no `input_json_delta` at all -- checking for `"{}"` instead would have failed those and also swallowed a legitimate empty-object argument. Four tests, each checked by ablation rather than assumed: - malformed fragments error the turn and emit no `tool_call_end` or `done` (disabling the guard: 16 pass / 1 fail) - valid fragments arrive as two separate deltas, not one joined string -- this is the contract the previous commit broke (removing incremental forwarding: 15 pass / 2 fail) - two `tool_use` blocks in one message keep separate buffers, so the second cannot inherit the first's JSON - a block with no fragments is not treated as malformed Suites: anthropic-stream-hardening, responses-stream-tool-events, anthropic-adapter, anthropic-compatible-stream, anthropic-hardening, anthropic-tail-guard, claude-messages-endpoint -- 64 pass, 0 fail. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
1 parent 05d9c48 commit 299cd2f

2 files changed

Lines changed: 93 additions & 18 deletions

File tree

src/adapters/anthropic.ts

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,23 @@ function toolUseArguments(input: unknown): string {
284284
return JSON.stringify(input ?? {});
285285
}
286286

287+
/**
288+
* Whether the arguments assembled from a stream's `input_json_delta` fragments are usable.
289+
* A tool block that sent no fragments at all is fine -- that is a no-argument call. Anything
290+
* else has to parse, because unlike the non-stream path the fragments have already been
291+
* forwarded to the client and cannot be repaired after the fact.
292+
*/
293+
function streamedToolArgumentsParse(assembled: string): boolean {
294+
const trimmed = assembled.trim();
295+
if (!trimmed) return true;
296+
try {
297+
JSON.parse(trimmed);
298+
return true;
299+
} catch {
300+
return false;
301+
}
302+
}
303+
287304
function anthropicKeyUsesBearer(provider: OcxProviderConfig): boolean {
288305
return provider.apiKeyTransport === "bearer";
289306
}
@@ -835,18 +852,28 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
835852
// so a stray signature on a non-thinking block can never be captured.
836853
yield { type: "thinking_signature", signature: delta.signature };
837854
} else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") {
838-
// Buffered rather than forwarded per-delta so the assembled arguments can be
839-
// validated at content_block_stop, the same bar the non-stream path applies in
840-
// toolUseArguments(). A tool call cannot run before its arguments are complete,
841-
// so holding the fragments costs nothing and keeps the two paths from
842-
// disagreeing about what a malformed payload becomes.
855+
// Forwarded immediately: the bridge maps each delta to a client-visible
856+
// response.function_call_arguments.delta frame, so withholding fragments until
857+
// block close would leave a started call showing empty arguments. A copy is kept
858+
// to validate the assembled payload at content_block_stop.
843859
currentToolCallJson += delta.partial_json;
860+
yield { type: "tool_call_delta", arguments: delta.partial_json };
844861
}
845862
break;
846863
}
847864
case "content_block_stop": {
848865
if (currentBlockType === "tool_use") {
849-
yield { type: "tool_call_delta", arguments: toolUseArguments(currentToolCallJson) };
866+
// The non-stream path repairs an unparseable payload in toolUseArguments(); the
867+
// stream cannot, because the fragments are already downstream. Fail the turn
868+
// instead of ending a tool call whose arguments will not parse -- a client that
869+
// received `not json` must not be told the call completed normally.
870+
if (!streamedToolArgumentsParse(currentToolCallJson)) {
871+
yield {
872+
type: "error",
873+
message: "Anthropic stream sent malformed tool_use arguments (invalid JSON)",
874+
};
875+
return;
876+
}
850877
yield { type: "tool_call_end" };
851878
currentToolCallId = "";
852879
currentToolCallJson = "";

tests/anthropic-stream-hardening.test.ts

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,11 @@ describe("anthropic non-stream tool_use input", () => {
104104
});
105105
});
106106

107-
test("streamed tool arguments are validated at block close, matching the non-stream path", async () => {
108-
// The streaming path used to forward each partial_json fragment verbatim, so a relay emitting
109-
// malformed JSON produced arguments:"not json" while the SAME payload through parseResponse
110-
// produced "{}". Two paths, two answers for one defect. Fragments are buffered and validated
111-
// once the block closes, so both now degrade identically.
107+
test("malformed streamed tool arguments fail the turn instead of completing the call", async () => {
108+
// The stream forwards fragments as they arrive (the bridge turns each into a client-visible
109+
// frame), so a malformed payload cannot be repaired the way the non-stream path repairs it.
110+
// It must not end the call normally either: a client that already received `not json` would
111+
// be told the tool call completed. The turn errors instead.
112112
const response = new Response([
113113
"event: content_block_start\n",
114114
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_9","name":"get_weather"}}\n\n',
@@ -122,14 +122,15 @@ describe("anthropic non-stream tool_use input", () => {
122122
'data: {"type":"message_stop"}\n\n',
123123
].join(""));
124124
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
125-
expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([
126-
{ type: "tool_call_delta", arguments: "{}" },
127-
]);
125+
expect(events.at(-1)?.type).toBe("error");
126+
expect(events.some(e => e.type === "tool_call_end")).toBe(false);
127+
expect(events.some(e => e.type === "done")).toBe(false);
128128
});
129129

130-
test("streamed tool arguments are reassembled intact when the JSON is valid", async () => {
131-
// The buffering must not corrupt the normal case: fragments split mid-token still arrive as
132-
// one well-formed argument string.
130+
test("valid streamed tool arguments still arrive as incremental deltas", async () => {
131+
// The bridge maps each adapter delta to response.function_call_arguments.delta, so the
132+
// fragments must keep flowing as they arrive rather than being withheld until block close.
133+
// Both fragments, in order, and the call ends normally.
133134
const response = new Response([
134135
"event: content_block_start\n",
135136
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_10","name":"get_weather"}}\n\n',
@@ -144,8 +145,55 @@ describe("anthropic non-stream tool_use input", () => {
144145
].join(""));
145146
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
146147
expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([
147-
{ type: "tool_call_delta", arguments: '{"city":"Paris"}' },
148+
{ type: "tool_call_delta", arguments: '{"city":' },
149+
{ type: "tool_call_delta", arguments: '"Paris"}' },
148150
]);
151+
expect(events.some(e => e.type === "tool_call_end")).toBe(true);
152+
expect(events.at(-1)?.type).toBe("done");
153+
});
154+
155+
test("two tool_use blocks in one message keep separate argument buffers", async () => {
156+
// The validation buffer is per block; a leak would make the second block inherit the first
157+
// block's JSON and could fail a well-formed call.
158+
const response = new Response([
159+
"event: content_block_start\n",
160+
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_a","name":"one"}}\n\n',
161+
"event: content_block_delta\n",
162+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"one\\":1}"}}\n\n',
163+
"event: content_block_stop\n",
164+
'data: {"type":"content_block_stop"}\n\n',
165+
"event: content_block_start\n",
166+
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_b","name":"two"}}\n\n',
167+
"event: content_block_delta\n",
168+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"two\\":2}"}}\n\n',
169+
"event: content_block_stop\n",
170+
'data: {"type":"content_block_stop"}\n\n',
171+
"event: message_stop\n",
172+
'data: {"type":"message_stop"}\n\n',
173+
].join(""));
174+
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
175+
expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([
176+
{ type: "tool_call_delta", arguments: '{"one":1}' },
177+
{ type: "tool_call_delta", arguments: '{"two":2}' },
178+
]);
179+
expect(events.filter(e => e.type === "tool_call_end")).toHaveLength(2);
180+
expect(events.at(-1)?.type).toBe("done");
181+
});
182+
183+
test("a tool_use block with no argument fragments is not treated as malformed", async () => {
184+
// A no-argument tool call sends no input_json_delta at all. An empty buffer must stay valid.
185+
const response = new Response([
186+
"event: content_block_start\n",
187+
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_c","name":"now"}}\n\n',
188+
"event: content_block_stop\n",
189+
'data: {"type":"content_block_stop"}\n\n',
190+
"event: message_stop\n",
191+
'data: {"type":"message_stop"}\n\n',
192+
].join(""));
193+
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
194+
expect(events.some(e => e.type === "error")).toBe(false);
195+
expect(events.some(e => e.type === "tool_call_end")).toBe(true);
196+
expect(events.at(-1)?.type).toBe("done");
149197
});
150198

151199
test("EOF after message_delta.stop_reason settles instead of erroring", async () => {

0 commit comments

Comments
 (0)