Skip to content

Commit 05d9c48

Browse files
lidge-junWibias
andcommitted
fix(anthropic): validate streamed tool arguments the same way as non-stream
The review of the batch caught a real inconsistency I introduced. Making `toolUseArguments` degrade an unparseable string to `{}` fixed the non-stream path only: the streaming path forwarded every `partial_json` fragment verbatim, so one malformed relay payload became `arguments:"not json"` through `parseStream` and `"{}"` through `parseResponse`. Two answers to the same defect, and the streaming one is what a live client actually hits. Fragments are now buffered per tool block and validated once at `content_block_stop`, through the same helper. A tool call cannot execute before its arguments are complete, so buffering costs nothing. Three tests, each verified by ablation rather than assumed: - malformed streamed fragments degrade to `{}` (reverting the buffering fails it) - valid fragments split mid-token reassemble intact, so buffering does not corrupt the normal case (also fails when reverted) - EOF carrying `message_delta.stop_reason` but no `message_stop` settles with that stop reason instead of erroring That third one closes the second review finding: the two existing EOF tests assert the pre-existing error behavior and never send a stop reason, so they stayed green with the new fallback reverted -- they were not testing the change they shipped with. Disabling the fallback now fails this test (14 pass / 1 fail) and restoring it returns 15/15. Suites: anthropic-stream-hardening, anthropic-adapter, anthropic-hardening, anthropic-compatible-stream, anthropic-tail-guard, anthropic-empty-content, anthropic-thinking-signature, anthropic-reasoning, anthropic-tool-schema, claude-messages-endpoint, anthropic-tool-reorder -- 115 pass, 0 fail. Co-authored-by: Wibias <37517432+Wibias@users.noreply.github.com>
1 parent decb08b commit 05d9c48

2 files changed

Lines changed: 71 additions & 1 deletion

File tree

src/adapters/anthropic.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
765765
let currentBlockType = "";
766766
let currentToolCallId = "";
767767
let currentToolCallName = "";
768+
let currentToolCallJson = "";
768769
let pendingUsage: Record<string, number> | undefined;
769770
let pendingStopReason: string | undefined;
770771
let emittedDone = false;
@@ -808,6 +809,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
808809
if (block.type === "tool_use") {
809810
currentToolCallId = block.id ?? synthesizeToolUseId();
810811
currentToolCallName = toolNames.fromWire(block.name ?? "");
812+
currentToolCallJson = "";
811813
yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName };
812814
}
813815
if (block.type === "redacted_thinking" && typeof block.data === "string") {
@@ -833,14 +835,21 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
833835
// so a stray signature on a non-thinking block can never be captured.
834836
yield { type: "thinking_signature", signature: delta.signature };
835837
} else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") {
836-
yield { type: "tool_call_delta", arguments: delta.partial_json };
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.
843+
currentToolCallJson += delta.partial_json;
837844
}
838845
break;
839846
}
840847
case "content_block_stop": {
841848
if (currentBlockType === "tool_use") {
849+
yield { type: "tool_call_delta", arguments: toolUseArguments(currentToolCallJson) };
842850
yield { type: "tool_call_end" };
843851
currentToolCallId = "";
852+
currentToolCallJson = "";
844853
}
845854
currentBlockType = "";
846855
break;

tests/anthropic-stream-hardening.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,65 @@ describe("anthropic non-stream tool_use input", () => {
103103
arguments: "{}",
104104
});
105105
});
106+
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.
112+
const response = new Response([
113+
"event: content_block_start\n",
114+
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_9","name":"get_weather"}}\n\n',
115+
"event: content_block_delta\n",
116+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"not "}}\n\n',
117+
"event: content_block_delta\n",
118+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"json"}}\n\n',
119+
"event: content_block_stop\n",
120+
'data: {"type":"content_block_stop"}\n\n',
121+
"event: message_stop\n",
122+
'data: {"type":"message_stop"}\n\n',
123+
].join(""));
124+
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+
]);
128+
});
129+
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.
133+
const response = new Response([
134+
"event: content_block_start\n",
135+
'data: {"type":"content_block_start","content_block":{"type":"tool_use","id":"toolu_10","name":"get_weather"}}\n\n',
136+
"event: content_block_delta\n",
137+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"{\\"city\\":"}}\n\n',
138+
"event: content_block_delta\n",
139+
'data: {"type":"content_block_delta","delta":{"type":"input_json_delta","partial_json":"\\"Paris\\"}"}}\n\n',
140+
"event: content_block_stop\n",
141+
'data: {"type":"content_block_stop"}\n\n',
142+
"event: message_stop\n",
143+
'data: {"type":"message_stop"}\n\n',
144+
].join(""));
145+
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
146+
expect(events.filter(e => e.type === "tool_call_delta")).toMatchObject([
147+
{ type: "tool_call_delta", arguments: '{"city":"Paris"}' },
148+
]);
149+
});
150+
151+
test("EOF after message_delta.stop_reason settles instead of erroring", async () => {
152+
// The blocker the other EOF tests could not see: they assert the pre-existing error path, so
153+
// reverting the stop-reason fallback leaves them green. This drives the fallback itself -- a
154+
// stream that reported why it stopped but never sent message_stop.
155+
const response = new Response([
156+
"event: content_block_start\n",
157+
'data: {"type":"content_block_start","content_block":{"type":"text","text":""}}\n\n',
158+
"event: content_block_delta\n",
159+
'data: {"type":"content_block_delta","delta":{"type":"text_delta","text":"hi"}}\n\n',
160+
"event: message_delta\n",
161+
'data: {"type":"message_delta","delta":{"stop_reason":"end_turn"}}\n\n',
162+
].join(""));
163+
const events = await collect(createAnthropicAdapter(provider).parseStream(response));
164+
expect(events.at(-1)).toMatchObject({ type: "done", stopReason: "end_turn" });
165+
expect(events.some(e => e.type === "error")).toBe(false);
166+
});
106167
});

0 commit comments

Comments
 (0)