diff --git a/src/anthropic.mjs b/src/anthropic.mjs index 9c17811..33b0b9c 100644 --- a/src/anthropic.mjs +++ b/src/anthropic.mjs @@ -93,16 +93,31 @@ export function normalizeTools(tools) { * Dig tool results out of this batch: Anthropic nests `tool_result` blocks in * user messages (no standalone `role:"tool"`), one level deeper than OpenAI. * - * Returns null when the batch carries no tool results at all, `{turn, + * Returns null when the trailing turn carries no tool results, `{turn, * results, orphan: null}` when at least one id matches a live turn, and - * `{turn: null, results, orphan: [ids]}` when results exist but no id - * matches anything. The caller must reject the orphan case — resuming - * nothing would silently bill a second run for the same turn. + * `{turn: null, results, orphan: [ids]}` when trailing results exist but + * no id is live. The caller starts a new run for the orphan case: Claude + * Code compact/follow-up resends finished tool_result blocks, and a 400 + * aborts autocompact. */ +function messagesAfterLastAssistant(messages) { + let start = 0; + for (let i = (messages ?? []).length - 1; i >= 0; i--) { + if (messages[i]?.role === "assistant") { + start = i + 1; + break; + } + } + return (messages ?? []).slice(start); +} + function digToolResults(messages) { const results = []; let turn = null; - for (const m of messages ?? []) { + // Only the trailing user turn after the last assistant reply can resume a + // live RelayTurn. Historical tool_result blocks in earlier user messages + // belong to finished turns; treating them as resume 400s the next prompt. + for (const m of messagesAfterLastAssistant(messages)) { if (m?.role !== "user" || !Array.isArray(m.content)) continue; for (const block of m.content) { if (block?.type !== "tool_result" || !block.tool_use_id) continue; @@ -323,14 +338,11 @@ const adapter = { const resume = digToolResults(messages); if (resume && !resume.turn) { - // Tool results with no live turn behind them: the original turn expired - // (result timeout, server restart, client resend). Resuming would feed - // nothing; treating the batch as fresh would bill a second run for the - // same turn. Say so and let the client retry. - return { - error: `no matching pending tool call (${resume.orphan.join(", ")}); the turn may have expired`, - status: 400, - }; + // Stale tool_result after the last assistant (compaction, follow-up, or + // an already-finished turn). 400 here aborts Claude Code auto-compact + // and surfaces as "Prompt is too long · automatic compaction failed". + // Start a new run with the folded prompt instead of resume. + log.warn(`stale tool_result id(s) ${resume.orphan.join(", ")}; starting a new run`); } return { diff --git a/src/openai.mjs b/src/openai.mjs index 9ab995e..d68501b 100644 --- a/src/openai.mjs +++ b/src/openai.mjs @@ -37,15 +37,26 @@ export function normalizeTools(tools) { /** * Dig tool results out of this batch: `role:"tool"` messages keyed by - * tool_call_id. Returns null with no tool results, `{turn, results}` when an - * id matches a live turn, `{orphan: [ids]}` when results exist but nothing - * matches — the caller must reject the orphan case, or the batch would - * silently bill a second run for a turn that no longer exists. + * tool_call_id. Returns null with no trailing tool results, `{turn, results}` + * when an id matches a live turn, `{orphan: [ids]}` when trailing results + * exist but nothing is live. The caller starts a new run for the orphan + * case so Claude Code compact/follow-up is not aborted with 400. */ +function messagesAfterLastAssistant(messages) { + let start = 0; + for (let i = (messages ?? []).length - 1; i >= 0; i--) { + if (messages[i]?.role === "assistant") { + start = i + 1; + break; + } + } + return (messages ?? []).slice(start); +} + function digToolResults(messages) { const results = []; let turn = null; - for (const m of messages) { + for (const m of messagesAfterLastAssistant(messages)) { if (m?.role !== "tool" || !m.tool_call_id) continue; results.push({ id: m.tool_call_id, content: m.content, isError: m.is_error === true }); turn = turn ?? lookupTurn(m.tool_call_id); @@ -63,13 +74,7 @@ const adapter = { const resume = digToolResults(messages); if (resume?.orphan) { - // Tool results with no live turn behind them (expired timer, server - // restart, client resend): resuming would feed nothing, and treating - // the batch as fresh would bill a second run for the same turn. - return { - error: `no matching pending tool call (${resume.orphan.join(", ")}); the turn may have expired`, - status: 400, - }; + log.warn(`stale tool_result id(s) ${resume.orphan.join(", ")}; starting a new run`); } return { diff --git a/src/tool-relay.mjs b/src/tool-relay.mjs index 20fbc3b..4df59ef 100644 --- a/src/tool-relay.mjs +++ b/src/tool-relay.mjs @@ -177,6 +177,8 @@ export class RelayTurn { this.callIdPrefix = "call_"; /** Calls arriving while the sink is closed wait here for the next attach to replay them. */ this.parked = []; + /** True once any assistant text has been handed to the sink or pendingText. */ + this.sawText = false; } #touch() { @@ -251,6 +253,7 @@ export class RelayTurn { #emitText(t) { if (!t) return; this.#touch(); + this.sawText = true; if (this.sink) this.sink.text(t); else this.pendingText += t; } @@ -390,7 +393,13 @@ export class RelayTurn { } get #hasText() { - return this.pendingText.length > 0 || (this.sink?.parts?.length ?? 0) > 0; + // AnthropicSseWriter stores text in `accum`, not `parts`. After a streamed + // assistant event, pendingText is empty and sink.parts is undefined, so + // the old check treated the turn as empty and appended result.result — + // the client then saw the same paragraph twice. + return this.sawText || this.pendingText.length > 0 + || (this.sink?.parts?.length ?? 0) > 0 + || (this.sink?.accum?.length ?? 0) > 0; } } diff --git a/test-protocol.mjs b/test-protocol.mjs index 6abc45b..a8907ea 100644 --- a/test-protocol.mjs +++ b/test-protocol.mjs @@ -348,10 +348,11 @@ test("collect sink: converts usage, keeps content", () => { assert.equal(c.content, "answer"); }); -// ── entry guard: orphan tool results must 400, never bill a new run ──── -// A tool_result whose id matches no live turn used to be treated as a fresh -// request: new account, new run, second billing for the same turn. It must -// come back as 400 invalid_request_error so the client retries instead. +// ── stale tool_result: new run, not 400 ──── +// Claude Code resends finished tool_result blocks on compact/follow-up. +// A 400 aborts autocompact ("Prompt is too long · compaction failed"). +// Empty test pool still answers 502 before any run; anything but 400 +// proves the orphan guard no longer rejects the request. function fakeHttpRes() { return { status: 0, @@ -362,7 +363,7 @@ function fakeHttpRes() { }; } -test("entry: anthropic tool results with no matching turn -> 400 invalid_request_error", async () => { +test("entry: anthropic tool results with no matching turn start a new run, not 400", async () => { const res = fakeHttpRes(); await A.handleMessages({ model: "claude-opus-5", @@ -370,22 +371,38 @@ test("entry: anthropic tool results with no matching turn -> 400 invalid_request { role: "user", content: [{ type: "tool_result", tool_use_id: "toolu_deadbeef", content: "x" }] }, ], }, res); - assert.equal(res.status, 400); - const body = JSON.parse(res.body); - assert.equal(body.error.type, "invalid_request_error"); - assert.match(body.error.message, /no matching pending tool call/); + assert.notEqual(res.status, 400, "stale tool_result must not abort compact/follow-up"); + assert.equal(res.status, 502, "empty pool answers 502 before any run"); + assert.ok( + recentLogs(50).some((e) => e.level === "warn" && /stale tool_result/.test(e.msg)), + "the fallback must be logged", + ); }); -test("entry: openai tool results with no matching turn -> 400 invalid_request_error", async () => { +test("entry: openai tool results with no matching turn start a new run, not 400", async () => { const res = fakeHttpRes(); await handleChat({ model: "x", messages: [{ role: "tool", tool_call_id: "call_deadbeef", content: "x" }], }, res); - assert.equal(res.status, 400); - const body = JSON.parse(res.body); - assert.equal(body.error.type, "invalid_request_error"); - assert.match(body.error.message, /no matching pending tool call/); + assert.notEqual(res.status, 400, "stale tool_result must not abort compact/follow-up"); + assert.equal(res.status, 502, "empty pool answers 502 before any run"); +}); + +test("entry: historical tool_result before the last assistant does not look like a resume", async () => { + const res = fakeHttpRes(); + await A.handleMessages({ + model: "claude-opus-5", + messages: [ + { role: "user", content: "search this" }, + { role: "assistant", content: [{ type: "tool_use", id: "toolu_old", name: "WebSearch", input: {} }] }, + { role: "user", content: [{ type: "tool_result", tool_use_id: "toolu_old", content: "done" }] }, + { role: "assistant", content: "here is the answer" }, + { role: "user", content: "follow-up question" }, + ], + }, res); + assert.notEqual(res.status, 400, "finished tool rounds in history must not 400"); + assert.equal(res.status, 502, "empty pool answers 502 before any run"); }); test("entry: a fresh request without tool results never trips the orphan guard", async () => {