diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 9266d49987..6a3d56d299 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -6,14 +6,15 @@ Zoo Code checks task lifecycle protocols through one compositional verification pnpm lifecycle:model-check ``` -The command runs six independent bounded submodels in sequence: +The command runs seven independent bounded submodels in sequence: 1. the persisted task delegation lifecycle; 2. shared-store concurrency across task-history hosts; 3. production-backed provider handoff and scheduler ordering; 4. the task cleanup protocol; -5. request-stream parser scoping; and -6. completion persistence. +5. request-stream parser scoping; +6. completion persistence; and +7. API retry and logical-user-turn persistence. This umbrella command is the single model-check entry point in the `compile` CI job after type checking. Command-level composition does not merge the submodels' state spaces: each checker retains its own bounds, transitions, invariant ownership, reachability requirements, and counterexample format. In particular, parser state is not part of the persisted lifecycle graph. The focused parser checker remains directly runnable with `pnpm parser-scope:model-check` for debugging. @@ -124,6 +125,8 @@ The completion persistence checker additionally enforces: 4. Delegated completion crosses the same durability boundary as standalone completion and requires successful parent reopen. 5. A failed delegated parent reopen cannot emit the delegated completion event. +The API retry/persistence checker additionally enforces that automatic retries are bounded and visible, terminal `max_tokens` empty responses cannot re-enter automatic retry, and the logical user turn keeps the same `messageId` and timestamp across retry/restoration. + These are safety claims within the documented bounds. The checks do not claim liveness, fairness, power-loss durability, filesystem-lock correctness, or exhaustive coverage of arbitrary task counts or retry counts. The completion explorer specifies the event contract rather than importing `Task` or `AttemptCompletionTool`; focused unit tests and the restart E2E verify that concrete production paths implement the modeled guards. Delegated reopen is abstracted as one success-or-failure event after durable child history; fallback from a failed reopen into the normal standalone completion flow remains production-test coverage rather than part of this model. The lifecycle checker also does not distinguish a delayed pre-interruption completion from a legitimate post-resume completion for the same child ID; that requires a persisted attempt/generation token before it can become a sound invariant. ## Open-issue traceability diff --git a/package.json b/package.json index 1fd9ddc8fe..055f7c31d7 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", - "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts", + "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts && tsx scripts/check-api-retry-persistence.ts", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", "parser-scope:model-check": "node scripts/run-native-tool-call-parser-scoping.mjs", "mcp:integration-check": "tsx scripts/check-mcp-oauth-integration.ts", diff --git a/scripts/check-api-retry-persistence.ts b/scripts/check-api-retry-persistence.ts new file mode 100644 index 0000000000..56bb7025a4 --- /dev/null +++ b/scripts/check-api-retry-persistence.ts @@ -0,0 +1,93 @@ +type StopReason = "none" | "max_tokens" +type Phase = "requesting" | "waiting" | "confirming" | "terminal" + +interface State { + attempt: number + phase: Phase + visibleRetries: number + messageId: string + timestamp: number + stopReason: StopReason +} + +interface Transition { + name: string + next: State +} + +const MAX_RETRIES = 3 +const initial: State = { + attempt: 0, + phase: "requesting", + visibleRetries: 0, + messageId: "logical-user-turn", + timestamp: 1, + stopReason: "none", +} + +function transitions(state: State): Transition[] { + if (state.phase === "terminal") return [] + if (state.phase === "waiting") { + return [{ name: "finish-visible-delay", next: { ...state, phase: "requesting" } }] + } + if (state.phase === "confirming") { + return [ + { name: "decline-retry", next: { ...state, phase: "terminal" } }, + { name: "confirm-retry", next: { ...state, attempt: 0, phase: "requesting" } }, + ] + } + if (state.stopReason === "max_tokens") { + return [{ name: "surface-terminal-stop", next: { ...state, phase: "terminal" } }] + } + if (state.attempt >= MAX_RETRIES) { + return [{ name: "exhaust-automatic-retries", next: { ...state, phase: "confirming" } }] + } + return [ + { + name: "retry-visible", + next: { + ...state, + attempt: state.attempt + 1, + visibleRetries: state.visibleRetries + 1, + phase: "waiting", + }, + }, + { + name: "receive-max-tokens-empty", + next: { ...state, stopReason: "max_tokens" }, + }, + ] +} + +const queue: Array<{ state: State; depth: number }> = [{ state: initial, depth: 0 }] +const seen = new Set() +const landmarks = new Set() + +while (queue.length > 0) { + const current = queue.shift()! + const key = JSON.stringify(current.state) + if (seen.has(key)) continue + seen.add(key) + + const state = current.state + if (state.attempt > MAX_RETRIES) throw new Error("automatic retry bound exceeded") + if (state.visibleRetries < state.attempt) throw new Error("retry occurred without a visible announcement") + if (state.messageId !== initial.messageId || state.timestamp !== initial.timestamp) { + throw new Error("logical user-turn identity changed across retry/restoration") + } + if (state.stopReason === "max_tokens" && state.phase === "waiting") { + throw new Error("terminal max_tokens response silently re-entered retry") + } + + if (state.phase === "confirming" && state.attempt === MAX_RETRIES) landmarks.add("bounded-exhaustion") + if (state.stopReason === "max_tokens" && state.phase === "terminal") landmarks.add("terminal-max-tokens") + if (state.visibleRetries === MAX_RETRIES) landmarks.add("all-retries-visible") + if (current.depth >= 10) continue + for (const transition of transitions(state)) queue.push({ state: transition.next, depth: current.depth + 1 }) +} + +for (const landmark of ["bounded-exhaustion", "terminal-max-tokens", "all-retries-visible"]) { + if (!landmarks.has(landmark)) throw new Error(`semantic landmark unreachable: ${landmark}`) +} + +console.log(`API retry/persistence model check passed (${seen.size} states)`) diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 7d54116a38..5fd5af91bc 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -1,9 +1,11 @@ // npx vitest run src/api/providers/__tests__/anthropic.spec.ts import { AnthropicHandler } from "../anthropic" +import { providerIdentifiers } from "@roo-code/types/provider-identifiers" import { ApiHandlerOptions } from "../../../shared/api" import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" import { clearAllMocks } from "../../../test-utils/reset" +import { prepareApiConversationMessage } from "../../../core/task/apiConversationHistory" // Mock TelemetryService vitest.mock("@roo-code/telemetry", () => ({ @@ -859,6 +861,442 @@ describe("AnthropicHandler", () => { expect(calledMessages.length).toBe(2) // Only the two user messages expect(calledMessages.every((m: any) => m.role === "user")).toBe(true) }) + + it("should preserve signed thinking and redacted_thinking blocks unchanged", async () => { + handler = new AnthropicHandler({ + apiKey: "test-api-key", + apiModelId: "claude-3-5-sonnet-20241022", + }) + + // Signed thinking blocks must round-trip unmodified so tool-use + // continuations pass Anthropic's signature verification. + const signedThinkingBlock = { + type: "thinking" as const, + thinking: "previous reasoning", + signature: "abc123", + } + const redactedThinkingBlock = { + type: "redacted_thinking" as const, + data: "encrypted-blob", + } + const messagesWithThinking: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: "Hello", + }, + { + role: "assistant", + content: [signedThinkingBlock, redactedThinkingBlock, { type: "text", text: "The response" }], + }, + { + role: "user", + content: "Continue", + }, + ] + + const stream = handler.createMessage(systemPrompt, messagesWithThinking) + await collectStream(stream) + + const calledMessages = mockCreate.mock.calls[mockCreate.mock.calls.length - 1][0] + .messages as Anthropic.Messages.MessageParam[] + const assistantMessage = calledMessages.find((m) => m.role === "assistant") + expect(assistantMessage).toBeDefined() + expect(assistantMessage?.content).toEqual([ + signedThinkingBlock, + redactedThinkingBlock, + expect.objectContaining({ type: "text", text: "The response" }), + ]) + }) + }) + + describe("stop reason and thinking signatures", () => { + const systemPrompt = "You are a helpful assistant." + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text" as const, text: "Hi" }], + }, + ] + + it("propagates stop_reason from message_delta on the usage chunk", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "burning the budget" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "message_delta", + delta: { stop_reason: "max_tokens", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + const usageChunks = chunks.filter((chunk) => chunk.type === "usage") + expect(usageChunks.some((chunk) => chunk.stopReason === "max_tokens")).toBe(true) + }) + + it("captures signature_delta events and exposes the completed thinking signature", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "deep thought" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "sig-part-1" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "-part-2" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "content_block_start", + index: 1, + content_block: { type: "text", text: "answer" }, + }, + { type: "content_block_stop", index: 1 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(chunks.filter((chunk) => chunk.type === "thinking_complete")).toEqual([ + { type: "thinking_complete", signature: "sig-part-1-part-2" }, + ]) + expect(handler.getThoughtSignature()).toBe("sig-part-1-part-2") + }) + + it("keeps each thinking block paired with its own signature across multiple blocks", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "first thought" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "sig-one" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "content_block_start", + index: 1, + content_block: { type: "tool_use", id: "toolu_1", name: "read_file", input: {} }, + }, + { type: "content_block_stop", index: 1 }, + { + type: "content_block_start", + index: 2, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 2, + delta: { type: "thinking_delta", thinking: "second thought" }, + }, + { + type: "content_block_delta", + index: 2, + delta: { type: "signature_delta", signature: "sig-two" }, + }, + { type: "content_block_stop", index: 2 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(chunks.filter((chunk) => chunk.type === "thinking_complete")).toEqual([ + { type: "thinking_complete", signature: "sig-one" }, + { type: "thinking_complete", signature: "sig-two" }, + ]) + expect(handler.getThoughtSignature()).toBe("sig-two") + expect(handler.getThinkingBlocks()).toEqual([ + { thinking: "first thought", signature: "sig-one" }, + { thinking: "second thought", signature: "sig-two" }, + ]) + }) + + it("round-trips multiple signed thinking blocks into a tool-result continuation", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { type: "message_start", message: { usage: { input_tokens: 10, output_tokens: 1 } } }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "one", signature: "" }, + }, + { type: "content_block_delta", index: 0, delta: { type: "signature_delta", signature: "sig-one" } }, + { type: "content_block_stop", index: 0 }, + { + type: "content_block_start", + index: 1, + content_block: { type: "thinking", thinking: "two", signature: "" }, + }, + { type: "content_block_delta", index: 1, delta: { type: "signature_delta", signature: "sig-two" } }, + { type: "content_block_stop", index: 1 }, + { type: "message_stop" }, + ]), + ) + + await collectStream(handler.createMessage(systemPrompt, messages)) + const assistant = prepareApiConversationMessage({ + message: { + role: "assistant", + content: [{ type: "tool_use", id: "toolu_1", name: "read_file", input: {} }], + }, + reasoning: "one\ntwo", + api: handler, + apiConfiguration: { + apiProvider: providerIdentifiers.anthropic, + apiModelId: "claude-3-5-sonnet-20241022", + }, + apiConversationHistory: [], + }) + + await collectStream( + handler.createMessage(systemPrompt, [ + assistant, + { role: "user", content: [{ type: "tool_result", tool_use_id: "toolu_1", content: "done" }] }, + ]), + ) + + const continuation = mockCreate.mock.calls.at(-1)?.[0].messages as Anthropic.Messages.MessageParam[] + expect(continuation[0]?.content).toEqual([ + { type: "thinking", thinking: "one", signature: "sig-one" }, + { type: "thinking", thinking: "two", signature: "sig-two" }, + { type: "tool_use", id: "toolu_1", name: "read_file", input: {} }, + ]) + }) + + it("ignores thinking deltas that arrive for a different block index", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "real thought" }, + }, + // Malformed stream: a thinking delta for a block that is not the + // open thinking block must not pollute the signed block text. + { + type: "content_block_delta", + index: 1, + delta: { type: "thinking_delta", thinking: "stray" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "sig" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(handler.getThinkingBlocks()).toEqual([{ thinking: "real thought", signature: "sig" }]) + }) + + it("does not complete a thinking block when content_block_stop arrives for a different index", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "unclosed" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "sig" }, + }, + // Malformed stream: a stop for another block must not finalize + // the open thinking block. + { type: "content_block_stop", index: 1 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(chunks.filter((chunk) => chunk.type === "thinking_complete")).toEqual([]) + expect(handler.getThoughtSignature()).toBeUndefined() + expect(handler.getThinkingBlocks()).toBeUndefined() + }) + + it("does not emit a thinking block completed without a signature", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "thinking_delta", thinking: "unsigned thought" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(chunks.filter((chunk) => chunk.type === "thinking_complete")).toEqual([]) + expect(handler.getThoughtSignature()).toBeUndefined() + expect(handler.getThinkingBlocks()).toBeUndefined() + }) + + it("clears a previously captured signature when the next response has no signed thinking block", async () => { + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "thinking", thinking: "", signature: "" }, + }, + { + type: "content_block_delta", + index: 0, + delta: { type: "signature_delta", signature: "stale-signature" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + await collectStream(handler.createMessage(systemPrompt, messages)) + expect(handler.getThoughtSignature()).toBe("stale-signature") + + mockCreate.mockImplementationOnce(async () => + asyncStreamFrom([ + { + type: "message_start", + message: { usage: { input_tokens: 10, output_tokens: 1 } }, + }, + { + type: "content_block_start", + index: 0, + content_block: { type: "text", text: "plain answer" }, + }, + { type: "content_block_stop", index: 0 }, + { + type: "message_delta", + delta: { stop_reason: "end_turn", stop_sequence: null }, + usage: { output_tokens: 5 }, + }, + { type: "message_stop" }, + ]), + ) + + const chunks = await collectStream(handler.createMessage(systemPrompt, messages)) + + expect(chunks.filter((chunk) => chunk.type === "thinking_complete")).toEqual([]) + expect(handler.getThoughtSignature()).toBeUndefined() + expect(handler.getThinkingBlocks()).toBeUndefined() + }) }) describe("native tool calling", () => { diff --git a/src/api/providers/anthropic.ts b/src/api/providers/anthropic.ts index 2ef70b78ea..c8831d54ef 100644 --- a/src/api/providers/anthropic.ts +++ b/src/api/providers/anthropic.ts @@ -43,6 +43,20 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa private options: ApiHandlerOptions private client: Anthropic private readonly providerName = "Anthropic" + /** + * Signature of the most recently completed thinking block, captured from + * `signature_delta` stream events. Round-tripped into API history via + * `getThoughtSignature()` so signed thinking blocks survive tool-use + * continuations (Anthropic rejects unsigned replays of thinking blocks). + */ + private lastThinkingSignature: string | undefined + /** + * Completed thinking blocks from the current/last response, each with its + * own text and verification signature. Signatures only validate against + * their exact block text, so blocks must be replayed individually rather + * than combined under one signature. + */ + private completedThinkingBlocks: { thinking: string; signature: string }[] = [] constructor(options: ApiHandlerOptions) { super() @@ -261,6 +275,18 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa let cacheWriteTokens = 0 let cacheReadTokens = 0 + // Thinking-block signature capture state. Anthropic streams the + // verification signature as `signature_delta` deltas on the thinking + // block; it must be replayed unchanged when the conversation continues + // after tool use. + this.lastThinkingSignature = undefined + this.completedThinkingBlocks = [] + let thinkingBlockIndex: number | undefined + // Stryker disable next-line StringLiteral: initial value is reset at every thinking-block start and only read after one, so it is never observable. + let pendingThinkingSignature = "" + // Stryker disable next-line StringLiteral: initial value is reset at every thinking-block start and only read after one, so it is never observable. + let pendingThinkingText = "" + for await (const chunk of stream) { switch (chunk.type) { case "message_start": { @@ -294,6 +320,7 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa type: "usage", inputTokens: 0, outputTokens: chunk.usage.output_tokens || 0, + stopReason: chunk.delta.stop_reason, } break @@ -303,6 +330,13 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa case "content_block_start": switch (chunk.content_block.type) { case "thinking": + // Start tracking this block's text and signature, streamed + // via thinking_delta/signature_delta events until + // content_block_stop. + thinkingBlockIndex = chunk.index + pendingThinkingSignature = chunk.content_block.signature + pendingThinkingText = chunk.content_block.thinking + // We may receive multiple text blocks, in which // case just insert a line break between them. if (chunk.index > 0) { @@ -336,8 +370,16 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa case "content_block_delta": switch (chunk.delta.type) { case "thinking_delta": + if (chunk.index === thinkingBlockIndex) { + pendingThinkingText += chunk.delta.thinking + } yield { type: "reasoning", text: chunk.delta.thinking } break + case "signature_delta": + // Accumulate the verification signature for the open + // thinking block (see content_block_start/content_block_stop). + pendingThinkingSignature += chunk.delta.signature + break case "text_delta": yield { type: "text", text: chunk.delta.text } break @@ -356,10 +398,25 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa break case "content_block_stop": - // Block complete - no action needed for now. - // NativeToolCallParser handles tool call completion - // Note: Signature for multi-turn thinking would require using stream.finalMessage() - // after iteration completes, which requires restructuring the streaming approach. + // Block complete - no action needed for tool calls; + // NativeToolCallParser handles tool call completion. + // A completed thinking block with a signature is recorded so the + // signed thinking block can be replayed on tool-use continuations. + if (chunk.index === thinkingBlockIndex) { + thinkingBlockIndex = undefined + if (pendingThinkingSignature) { + this.lastThinkingSignature = pendingThinkingSignature + this.completedThinkingBlocks.push({ + thinking: pendingThinkingText, + signature: pendingThinkingSignature, + }) + yield { type: "thinking_complete", signature: pendingThinkingSignature } + } + // Stryker disable next-line StringLiteral: reset value is never observed - the next thinking-block start overwrites it before any read. + pendingThinkingSignature = "" + // Stryker disable next-line StringLiteral: reset value is never observed - the next thinking-block start overwrites it before any read. + pendingThinkingText = "" + } break } } @@ -382,6 +439,22 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa } } + /** + * Returns the signature of the last completed thinking block so it can be + * persisted into API history and replayed on tool-use continuations. + */ + public getThoughtSignature(): string | undefined { + return this.lastThinkingSignature + } + + /** + * Returns every completed thinking block (text + signature, in order) so + * each signed block can be replayed unchanged on tool-use continuations. + */ + public getThinkingBlocks(): { thinking: string; signature: string }[] | undefined { + return this.completedThinkingBlocks.length > 0 ? [...this.completedThinkingBlocks] : undefined + } + // Guesses capabilities for an unrecognized model ID via known-family substring match. private guessModelInfoFromId(modelId: string): ModelInfo { const lowerModelId = modelId.toLowerCase() diff --git a/src/api/transform/stream.ts b/src/api/transform/stream.ts index 960ebbe770..ba31427527 100644 --- a/src/api/transform/stream.ts +++ b/src/api/transform/stream.ts @@ -63,6 +63,12 @@ export interface ApiStreamUsageChunk { cacheReadTokens?: number reasoningTokens?: number totalCost?: number + /** + * The model's stop reason once known (e.g. Anthropic's message_delta). + * Lets callers distinguish terminal ends like "max_tokens" (which must + * not be silently retried) from genuinely empty responses. + */ + stopReason?: string | null } export interface ApiStreamGroundingChunk { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 92ee8184d6..6d8930634b 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -172,6 +172,10 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors +// Maximum automatic retries for mid-stream failures and empty responses before +// asking the user. Every retry re-bills the full input context, so retries must +// be bounded and user-visible. +const MAX_AUTOMATIC_API_RETRIES = 3 export interface TaskOptions extends CreateTaskOptions { provider: ClineProvider @@ -1108,6 +1112,19 @@ export class Task extends EventEmitter implements TaskLike { // For API requests, consecutive same-role messages are merged via mergeConsecutiveApiMessages() // so rewind/edit behavior can still reference original message boundaries. + /** + * Restores a user message previously removed from the API conversation + * history, keeping the original record (including messageId and ts). + * Rebuilding the message would assign a new identity, and the merge-on-save + * would then keep both the on-disk original and the rebuilt copy, + * duplicating the user turn after a restart. + */ + private async restoreApiHistoryUserMessage(message: ApiMessage) { + this.apiConversationHistory.push(message) + this.messageCounts.user++ + await this.saveApiConversationHistory() + } + /** Replaces the entire API conversation history and persists the new state. */ async overwriteApiConversationHistory(newHistory: ApiMessage[], persist = true) { this.hydrateApiConversationHistory(newHistory) @@ -2924,6 +2941,7 @@ export class Task extends EventEmitter implements TaskLike { includeFileDetails: boolean retryAttempt?: number userMessageWasRemoved?: boolean // Track if user message was removed due to empty response + removedUserMessage?: ApiMessage // The exact removed record, so a retry can restore it with its persisted identity } const stack: StackItem[] = [{ userContent, includeFileDetails, retryAttempt: 0 }] @@ -3066,8 +3084,16 @@ export class Task extends EventEmitter implements TaskLike { userMessageWasRemoved: currentItem.userMessageWasRemoved, }) if (shouldAddUserMessage) { - await this.addToApiConversationHistory({ role: "user", content: finalUserContent }) - this.messageCounts.user++ + if (currentItem.removedUserMessage) { + // Restore the exact record removed before the retry. Rebuilding it + // would assign a new messageId/ts, and the merge-on-save would keep + // both the on-disk original and the rebuilt copy, duplicating the + // user turn after a restart. + await this.restoreApiHistoryUserMessage(currentItem.removedUserMessage) + } else { + await this.addToApiConversationHistory({ role: "user", content: finalUserContent }) + this.messageCounts.user++ + } } // Since we sent off a placeholder api_req_started message to update the @@ -3202,6 +3228,11 @@ export class Task extends EventEmitter implements TaskLike { const stream = this.attemptApiRequest(currentItem.retryAttempt ?? 0, { skipProviderRateLimit: true }) let assistantMessage = "" let reasoningMessage = "" + // Stop reason reported by the provider for this request (if any). + // Used to distinguish terminal ends like "max_tokens" (no retry - + // the same request would fail again while re-billing the full + // context) from genuinely empty responses. + let lastStopReason: string | undefined const pendingGroundingSources: GroundingSource[] = [] this.isStreaming = true @@ -3268,6 +3299,7 @@ export class Task extends EventEmitter implements TaskLike { cacheWriteTokens += chunk.cacheWriteTokens ?? 0 cacheReadTokens += chunk.cacheReadTokens ?? 0 totalCost = chunk.totalCost + lastStopReason = chunk.stopReason ?? lastStopReason break case "grounding": // Handle grounding sources separately from regular content @@ -3655,16 +3687,17 @@ export class Task extends EventEmitter implements TaskLike { this.abortReason = cancelReason await this.abortTask() } else { - // Stream failed - log the error and retry with the same content - // The existing rate limiting will prevent rapid retries + // Stream failed mid-flight. Every automatic retry re-bills + // the full input context, so retries are bounded and always + // announced via the shared backoff countdown. console.error( `[Task#${this.taskId}.${this.instanceId}] Stream failed, will retry: ${streamingFailedMessage}`, ) - // Apply exponential backoff similar to first-chunk errors when auto-resubmit is enabled - const stateForBackoff = await this.providerRef.deref()?.getState() - if (stateForBackoff?.autoApprovalEnabled) { - await this.backoffAndAnnounce(currentItem.retryAttempt ?? 0, error) + const midStreamRetryAttempt = currentItem.retryAttempt ?? 0 + + if (midStreamRetryAttempt < MAX_AUTOMATIC_API_RETRIES) { + await this.backoffAndAnnounce(midStreamRetryAttempt, error) // Check if task was aborted during the backoff if (this.abort) { @@ -3676,17 +3709,76 @@ export class Task extends EventEmitter implements TaskLike { await this.abortTask() break } + + // Push the same content back onto the stack to retry, incrementing the retry attempt counter + stack.push({ + userContent: currentUserContent, + includeFileDetails: false, + retryAttempt: midStreamRetryAttempt + 1, + }) + + // Continue to retry the request + continue } - // Push the same content back onto the stack to retry, incrementing the retry attempt counter - stack.push({ - userContent: currentUserContent, - includeFileDetails: false, - retryAttempt: (currentItem.retryAttempt ?? 0) + 1, + // Automatic retry budget exhausted - surface the failure. + // Remove this turn's user message so a user-approved retry + // (which resets retryAttempt to 0 and therefore re-adds the + // message) does not duplicate it in history. Keep the exact + // record so a restore preserves its persisted identity. + let removedMidStreamUserMessage: ApiMessage | undefined + const hasUserContent = currentUserContent.length > 0 + const lastHistoryMessage = + this.apiConversationHistory[this.apiConversationHistory.length - 1] + // Stryker disable next-line ConditionalExpression,OptionalChaining: whenever content is non-empty here, the last record is this turn's user message; the role check is defensive against corrupted history and has no reachable false branch. + if (hasUserContent && lastHistoryMessage?.role === "user") { + removedMidStreamUserMessage = this.apiConversationHistory.pop() + this.messageCounts.user-- + } + + const { response } = await this.ask( + "api_req_failed", + `The API stream failed ${MAX_AUTOMATIC_API_RETRIES + 1} times mid-response. ${streamingFailedMessage}`, + ) + + if (response === "yesButtonClicked") { + await this.say("api_req_retried") + + // Reset the automatic retry budget; the user message is + // restored exactly once on the next iteration. The + // userMessageWasRemoved flag is redundant here because + // retryAttempt 0 with non-empty content always re-adds. + stack.push({ + userContent: currentUserContent, + includeFileDetails: false, + retryAttempt: 0, + removedUserMessage: removedMidStreamUserMessage, + }) + + continue + } + + // User declined to retry: restore the user message, surface + // the error, record the failure, and stop the loop. + if (removedMidStreamUserMessage) { + await this.restoreApiHistoryUserMessage(removedMidStreamUserMessage) + } + + await this.say( + "error", + `The API stream failed mid-response and was not retried. ${streamingFailedMessage}`, + ) + + // Synthetic assistant message recording the failure -- increment + // messageCounts.assistant to match, same as the normal + // assistant-message-saved path. + await this.addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: "Failure: the API stream failed mid-response." }], }) + this.messageCounts.assistant++ - // Continue to retry the request - continue + return false } } } finally { @@ -4053,20 +4145,55 @@ export class Task extends EventEmitter implements TaskLike { // Only pop the user message that this iteration added. When // shouldAddUserMessage is false (empty continuation, resumed history, // or flushPendingToolResultsToHistory message) there is nothing to - // remove, and popping would corrupt history. - let removedCurrentUserMessage = false + // remove, and popping would corrupt history. Keep the exact record + // so a restore preserves its persisted identity. + let removedCurrentUserMessage: ApiMessage | undefined if (shouldAddUserMessage && this.apiConversationHistory.length > 0) { const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] if (lastMessage.role === "user") { - this.apiConversationHistory.pop() + removedCurrentUserMessage = this.apiConversationHistory.pop() this.messageCounts.user-- - removedCurrentUserMessage = true } } - // Check if we should auto-retry or prompt the user + // A max_tokens stop reason with no usable content means the model + // burned its whole output budget (typically on reasoning) before + // producing anything. Retrying the identical request would fail + // the same way while re-billing the full context each time, so + // surface it and stop instead of retrying. + if (lastStopReason === "max_tokens") { + if (removedCurrentUserMessage) { + await this.restoreApiHistoryUserMessage(removedCurrentUserMessage) + } + + await this.say( + "error", + "The model hit its maximum output token limit (stop_reason: max_tokens) without producing any visible output - it likely spent the entire budget on reasoning. Increase the max output tokens (or lower the thinking budget) for this API profile, then retry.", + ) + + // Synthetic assistant message recording the failure -- increment + // messageCounts.assistant to match, same as the normal + // assistant-message-saved path. + await this.addToApiConversationHistory({ + role: "assistant", + content: [ + { + type: "text", + text: "Failure: response hit the max output token limit before producing any visible content.", + }, + ], + }) + this.messageCounts.assistant++ + + return false + } + + // Check if we should auto-retry or prompt the user. + // Automatic retries are bounded: once the budget is exhausted the + // user is asked, so a persistently empty response cannot loop + // (and bill) forever without visibility. // Reuse the state variable from above - if (state?.autoApprovalEnabled) { + if (state?.autoApprovalEnabled && (currentItem.retryAttempt ?? 0) < MAX_AUTOMATIC_API_RETRIES) { // Auto-retry with backoff - don't persist failure message when retrying await this.backoffAndAnnounce( currentItem.retryAttempt ?? 0, @@ -4090,7 +4217,8 @@ export class Task extends EventEmitter implements TaskLike { userContent: currentUserContent, includeFileDetails: false, retryAttempt: (currentItem.retryAttempt ?? 0) + 1, - userMessageWasRemoved: removedCurrentUserMessage, + userMessageWasRemoved: removedCurrentUserMessage !== undefined, + removedUserMessage: removedCurrentUserMessage, }) // Continue to retry the request @@ -4099,7 +4227,11 @@ export class Task extends EventEmitter implements TaskLike { // Prompt the user for retry decision const { response } = await this.ask( "api_req_failed", - "The model returned no assistant messages. This may indicate an issue with the API or the model's output.", + `The model returned no assistant messages. This may indicate an issue with the API or the model's output.${ + state?.autoApprovalEnabled + ? ` Automatic retries were attempted ${MAX_AUTOMATIC_API_RETRIES} times without success.` + : "" + }`, ) if (response === "yesButtonClicked") { @@ -4111,20 +4243,17 @@ export class Task extends EventEmitter implements TaskLike { userContent: currentUserContent, includeFileDetails: false, retryAttempt: (currentItem.retryAttempt ?? 0) + 1, - userMessageWasRemoved: removedCurrentUserMessage, + userMessageWasRemoved: removedCurrentUserMessage !== undefined, + removedUserMessage: removedCurrentUserMessage, }) // Continue to retry the request continue } else { - // User declined to retry. Re-add the user message only if this + // User declined to retry. Restore the user message only if this // iteration removed one, so the history and counter stay consistent. if (removedCurrentUserMessage) { - await this.addToApiConversationHistory({ - role: "user", - content: currentUserContent, - }) - this.messageCounts.user++ + await this.restoreApiHistoryUserMessage(removedCurrentUserMessage) } await this.say( diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 1bcacd459c..827ec6b421 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -26,6 +26,7 @@ import { ClineProvider } from "../../webview/ClineProvider" import { ApiStreamChunk } from "../../../api/transform/stream" import { ContextProxy } from "../../config/ContextProxy" import { processUserContentMentions } from "../../mentions/processUserContentMentions" +import { getEnvironmentDetails } from "../../environment/getEnvironmentDetails" import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import type { ApiMessage } from "../../task-persistence" import { asyncStreamFrom } from "../../../test-utils/stream" @@ -400,32 +401,33 @@ describe("Cline", () => { })) }) - describe("empty-response retries", () => { - function stream(chunks: ApiStreamChunk[]): AsyncGenerator { - return (async function* () { - yield* chunks - })() - } + // Shared helpers for the retry suites below. + function stream(chunks: ApiStreamChunk[]): AsyncGenerator { + return (async function* () { + yield* chunks + })() + } - async function createTaskWithManualRetries() { - const task = new Task({ - provider: mockProvider, - apiConfiguration: mockApiConfig, - task: "test task", - startTask: false, - }) - const state = await mockProvider.getState() - vi.spyOn(mockProvider, "getState").mockResolvedValue({ - ...state, - apiConfiguration: mockApiConfig, - autoApprovalEnabled: false, - }) - vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) - return task - } + async function createTaskWithAutoApproval(autoApprovalEnabled: boolean) { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + const state = await mockProvider.getState() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + ...state, + apiConfiguration: mockApiConfig, + autoApprovalEnabled, + }) + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + return task + } + describe("empty-response retries", () => { it("restores the user message before a confirmed empty-response retry", async () => { - const task = await createTaskWithManualRetries() + const task = await createTaskWithAutoApproval(false) let retryHistory: ApiMessage[] | undefined let retryUserMessageCount: number | undefined @@ -452,20 +454,248 @@ describe("Cline", () => { }) it("restores the user message and records the failure when retry is declined", async () => { - const task = await createTaskWithManualRetries() + const task = await createTaskWithAutoApproval(false) + let originalUserMessage: ApiMessage | undefined vi.spyOn(task, "ask").mockResolvedValue({ response: "noButtonClicked" } satisfies TaskAskResult) - vi.spyOn(task, "attemptApiRequest").mockImplementation(() => stream([])) + vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + // Capture the persisted identity of the user message before the + // empty-response path removes and later restores it. + originalUserMessage ??= structuredClone(task.apiConversationHistory[0]) + return stream([]) + }) const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) expect(result).toBe(false) - expect(task.apiConversationHistory).toMatchObject([ - { role: "user", content: [{ type: "text", text: "original user request" }] }, - { role: "assistant", content: [{ type: "text", text: "Failure: I did not provide a response." }] }, - ]) + expect(task.apiConversationHistory).toHaveLength(2) + expect(task.apiConversationHistory[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + expect(task.apiConversationHistory[1]).toMatchObject({ + role: "assistant", + content: [{ type: "text", text: "Failure: I did not provide a response." }], + }) + // The restore must keep the original record identity so the + // merge-on-save does not duplicate the user turn on disk. + expect(task.apiConversationHistory[0]?.messageId).toBe(originalUserMessage?.messageId) + expect(task.apiConversationHistory[0]?.ts).toBe(originalUserMessage?.ts) + expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) + }) + + it("does not retry when the response ends with stop_reason max_tokens and no usable content", async () => { + // Auto-approval is on to prove the max_tokens branch stops instead of + // silently auto-retrying (and re-billing the full context). + const task = await createTaskWithAutoApproval(true) + const saySpy = vi.spyOn(task, "say") + const askSpy = vi.spyOn(task, "ask") + let originalUserMessage: ApiMessage | undefined + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + originalUserMessage ??= structuredClone(task.apiConversationHistory[0]) + return stream([ + { type: "reasoning", text: "reasoning that consumed the whole output budget" }, + { type: "usage", inputTokens: 1000, outputTokens: 8192, stopReason: "max_tokens" }, + ]) + }) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + expect(result).toBe(false) + expect(attemptSpy).toHaveBeenCalledTimes(1) + expect(askSpy).not.toHaveBeenCalled() + expect( + saySpy.mock.calls.some( + ([type, text]) => type === "error" && typeof text === "string" && text.includes("max_tokens"), + ), + ).toBe(true) + expect(task.apiConversationHistory).toHaveLength(2) + expect(task.apiConversationHistory[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + expect(task.apiConversationHistory[1]).toMatchObject({ + role: "assistant", + content: [ + { + type: "text", + text: "Failure: response hit the max output token limit before producing any visible content.", + }, + ], + }) + // The restore must keep the original record identity so the + // merge-on-save does not duplicate the user turn on disk. + expect(task.apiConversationHistory[0]?.messageId).toBe(originalUserMessage?.messageId) + expect(task.apiConversationHistory[0]?.ts).toBe(originalUserMessage?.ts) + expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) + }) + + it("bounds automatic empty-response retries and asks the user after the cap", async () => { + const task = await createTaskWithAutoApproval(true) + const saySpy = vi.spyOn(task, "say") + const askSpy = vi + .spyOn(task, "ask") + .mockResolvedValue({ response: "noButtonClicked" } satisfies TaskAskResult) + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => stream([])) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + expect(result).toBe(false) + // Initial attempt + MAX_AUTOMATIC_API_RETRIES (3) automatic retries. + expect(attemptSpy).toHaveBeenCalledTimes(4) + // Every automatic retry was announced via the visible countdown: + // one final (non-partial) announcement per retry. + const retryAnnouncements = saySpy.mock.calls.filter( + ([type, , , partial]) => type === "api_req_retry_delayed" && partial === false, + ) + expect(retryAnnouncements).toHaveLength(3) + expect(askSpy).toHaveBeenCalledTimes(1) + expect(askSpy.mock.calls[0]?.[0]).toBe("api_req_failed") + expect(askSpy.mock.calls[0]?.[1]).toContain("Automatic retries were attempted 3 times without success.") + expect(task.apiConversationHistory).toHaveLength(2) + expect(task.apiConversationHistory[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + expect(task.apiConversationHistory[1]).toMatchObject({ + role: "assistant", + content: [{ type: "text", text: "Failure: I did not provide a response." }], + }) + expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) + }) + }) + + describe("mid-stream retries", () => { + function failingStream(error: Error): AsyncGenerator { + return (async function* () { + // Yield one chunk first so the failure is genuinely mid-stream. + yield { type: "text", text: "partial output" } + throw error + })() + } + + it("announces each automatic retry and asks the user after the cap is exhausted", async () => { + const task = await createTaskWithAutoApproval(true) + vi.mocked(getEnvironmentDetails).mockClear() + const saySpy = vi.spyOn(task, "say") + const askSpy = vi + .spyOn(task, "ask") + .mockResolvedValue({ response: "noButtonClicked" } satisfies TaskAskResult) + let originalUserMessage: ApiMessage | undefined + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + originalUserMessage ??= structuredClone(task.apiConversationHistory[0]) + return failingStream(new Error("overloaded_error")) + }) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + expect(result).toBe(false) + // Initial attempt + MAX_AUTOMATIC_API_RETRIES (3) automatic retries. + expect(attemptSpy).toHaveBeenCalledTimes(4) + // Retries must not resend file details: no request in the retry + // loop includes them. + const envDetailCalls = vi.mocked(getEnvironmentDetails).mock.calls + expect(envDetailCalls).toHaveLength(4) + expect(envDetailCalls.every((call) => call[1] === false)).toBe(true) + // Every automatic retry ran through the visible backoff countdown: + // one final (non-partial) announcement per retry. + const retryAnnouncements = saySpy.mock.calls.filter( + ([type, , , partial]) => type === "api_req_retry_delayed" && partial === false, + ) + expect(retryAnnouncements).toHaveLength(3) + expect(askSpy).toHaveBeenCalledTimes(1) + expect(askSpy.mock.calls[0]?.[0]).toBe("api_req_failed") + expect(askSpy.mock.calls[0]?.[1]).toContain("4 times mid-response") + // Declined retry surfaces the error and records the failure without + // losing or duplicating the user message. + expect( + saySpy.mock.calls.some( + ([type, text]) => type === "error" && typeof text === "string" && text.includes("was not retried"), + ), + ).toBe(true) + expect(task.apiConversationHistory).toHaveLength(2) + expect(task.apiConversationHistory[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + expect(task.apiConversationHistory[1]).toMatchObject({ + role: "assistant", + content: [{ type: "text", text: "Failure: the API stream failed mid-response." }], + }) + // The restore must keep the original record identity so the + // merge-on-save does not duplicate the user turn on disk. + expect(task.apiConversationHistory[0]?.messageId).toBe(originalUserMessage?.messageId) + expect(task.apiConversationHistory[0]?.ts).toBe(originalUserMessage?.ts) expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) }) + + it("makes retries visible even when auto-approval is disabled", async () => { + const task = await createTaskWithAutoApproval(false) + const saySpy = vi.spyOn(task, "say") + const askSpy = vi + .spyOn(task, "ask") + .mockResolvedValue({ response: "noButtonClicked" } satisfies TaskAskResult) + vi.spyOn(task, "attemptApiRequest").mockImplementation(() => failingStream(new Error("overloaded_error"))) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + expect(result).toBe(false) + // Retries stay visible even without auto-approval: one final + // (non-partial) countdown announcement per automatic retry. + const retryAnnouncements = saySpy.mock.calls.filter( + ([type, , , partial]) => type === "api_req_retry_delayed" && partial === false, + ) + expect(retryAnnouncements).toHaveLength(3) + expect(askSpy).toHaveBeenCalledTimes(1) + }) + + it("resets the retry budget without duplicating the user message when the user approves retry", async () => { + const task = await createTaskWithAutoApproval(true) + vi.mocked(getEnvironmentDetails).mockClear() + let askCount = 0 + vi.spyOn(task, "ask").mockImplementation(async () => { + askCount++ + // Approve the first capped-retry prompt; decline the one that + // follows after the recovered turn fails again. + return { response: askCount === 1 ? "yesButtonClicked" : "noButtonClicked" } as TaskAskResult + }) + + let attempt = 0 + let originalUserMessage: ApiMessage | undefined + let historyAtSuccess: ApiMessage[] | undefined + vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + attempt++ + originalUserMessage ??= structuredClone(task.apiConversationHistory[0]) + if (attempt === 5) { + historyAtSuccess = structuredClone(task.apiConversationHistory) + return stream([{ type: "text", text: "recovered" }]) + } + return failingStream(new Error("overloaded_error")) + }) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + expect(result).toBe(false) + // Attempts 1-4: first turn fails to the cap and is approved. + // Attempt 5: recovered text response. Attempts 6-9: the recovered + // turn's no-tool follow-up fails to the cap again and is declined. + expect(attempt).toBe(9) + expect(askCount).toBe(2) + // The retried request restored the user message exactly once, keeping + // its original persisted identity so the merge-on-save does not + // duplicate the turn on disk. + expect(historyAtSuccess).toHaveLength(1) + expect(historyAtSuccess?.[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + expect(historyAtSuccess?.[0]?.messageId).toBe(originalUserMessage?.messageId) + expect(historyAtSuccess?.[0]?.ts).toBe(originalUserMessage?.ts) + expect(vi.mocked(getEnvironmentDetails).mock.calls.every((call) => call[1] === false)).toBe(true) + // Final history: original user turn, recovered assistant turn, the + // follow-up user turn, and the recorded failure. + expect(task.messageCounts).toEqual({ user: 2, assistant: 2 }) + }) }) describe("native tool-call request isolation", () => { diff --git a/src/core/task/__tests__/apiConversationHistory.spec.ts b/src/core/task/__tests__/apiConversationHistory.spec.ts index 7313e4fa1c..292db40806 100644 --- a/src/core/task/__tests__/apiConversationHistory.spec.ts +++ b/src/core/task/__tests__/apiConversationHistory.spec.ts @@ -51,6 +51,97 @@ describe("prepareApiConversationMessage", () => { ]) }) + it("replays each Anthropic thinking block with its own signature", () => { + // Double assertion: the stub only implements the optional history hooks + // this path reads, not the full ApiHandler surface. + const api = { + getThoughtSignature: () => "signature-2", + getThinkingBlocks: () => [ + { thinking: "first thought", signature: "signature-1" }, + { thinking: "second thought", signature: "signature-2" }, + ], + } as unknown as Parameters[0]["api"] + + const result = prepareApiConversationMessage({ + message: { role: "assistant", content: "answer" }, + reasoning: "first thought\nsecond thought", + api, + apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiModelId: "claude-3-5-sonnet" }, + apiConversationHistory: [], + }) + + expect(result.content).toEqual([ + { type: "thinking", thinking: "first thought", signature: "signature-1" }, + { type: "thinking", thinking: "second thought", signature: "signature-2" }, + { type: "text", text: "answer" }, + ]) + }) + + it("does not add thinking blocks for non-Anthropic protocols even when getThinkingBlocks exists", () => { + // Double assertion: the stub only implements the optional history hooks + // this path reads, not the full ApiHandler surface. + const api = { + getThoughtSignature: () => "signature-1", + getThinkingBlocks: () => [{ thinking: "first thought", signature: "signature-1" }], + } as unknown as Parameters[0]["api"] + + const result = prepareApiConversationMessage({ + message: { role: "assistant", content: "answer" }, + reasoning: "first thought", + api, + apiConfiguration: { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "openai/gpt-4" }, + apiConversationHistory: [], + }) + + expect(result.content).toEqual([ + { type: "reasoning", text: "first thought", summary: [] }, + { type: "text", text: "answer" }, + { type: "thoughtSignature", thoughtSignature: "signature-1" }, + ]) + }) + + it("falls back to the single signed block when getThinkingBlocks returns an empty array", () => { + const api = { + getThoughtSignature: () => "signature-1", + getThinkingBlocks: () => [], + } as unknown as Parameters[0]["api"] + + const result = prepareApiConversationMessage({ + message: { role: "assistant", content: "answer" }, + reasoning: "private reasoning", + api, + apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiModelId: "claude-3-5-sonnet" }, + apiConversationHistory: [], + }) + + expect(result.content).toEqual([ + { type: "thinking", thinking: "private reasoning", signature: "signature-1" }, + { type: "text", text: "answer" }, + ]) + }) + + it("prefers reasoning_details over getThinkingBlocks for Anthropic messages", () => { + // Double assertion: the stub only implements the optional history hooks + // this path reads, not the full ApiHandler surface. + const api = { + getThoughtSignature: () => "signature-1", + getThinkingBlocks: () => [{ thinking: "first thought", signature: "signature-1" }], + getReasoningDetails: () => [{ type: "reasoning", text: "detail" }], + } as unknown as Parameters[0]["api"] + + const result = prepareApiConversationMessage({ + message: { role: "assistant", content: "answer" }, + reasoning: "first thought", + api, + apiConfiguration: { apiProvider: providerIdentifiers.anthropic, apiModelId: "claude-3-5-sonnet" }, + apiConversationHistory: [], + }) + + expect(result.reasoning_details).toEqual([{ type: "reasoning", text: "detail" }]) + // No thinking or reasoning block is prepended when reasoning_details wins. + expect(result.content).toBe("answer") + }) + it("falls back to generic reasoning blocks for Anthropic messages without thought signatures", () => { const result = prepareApiConversationMessage({ message: { role: "assistant", content: "answer" }, diff --git a/src/core/task/apiConversationHistory.ts b/src/core/task/apiConversationHistory.ts index d1e609c8dc..95d36aab36 100644 --- a/src/core/task/apiConversationHistory.ts +++ b/src/core/task/apiConversationHistory.ts @@ -12,6 +12,7 @@ type ApiHistoryHandler = ApiHandler & { getResponseId?: () => string | undefined getEncryptedContent?: () => { encrypted_content: string; id?: string } | undefined getThoughtSignature?: () => string | undefined + getThinkingBlocks?: () => { thinking: string; signature: string }[] | undefined getReasoningDetails?: () => any[] | undefined } @@ -46,6 +47,7 @@ function prepareAssistantMessage( const responseId = handler.getResponseId?.() const reasoningData = handler.getEncryptedContent?.() const thoughtSignature = handler.getThoughtSignature?.() + const thinkingBlocks = handler.getThinkingBlocks?.() const reasoningDetails = handler.getReasoningDetails?.() const modelId = getModelId(apiConfiguration) @@ -67,7 +69,18 @@ function prepareAssistantMessage( messageWithTs.reasoning_details = reasoningDetails } - if (isAnthropicProtocol && reasoning && thoughtSignature && !reasoningDetails) { + if (isAnthropicProtocol && thinkingBlocks && thinkingBlocks.length > 0 && !reasoningDetails) { + // Replay each completed thinking block with its own signature - + // signatures only validate against their exact block text, so blocks + // must not be combined under a single signature. + for (let i = thinkingBlocks.length - 1; i >= 0; i--) { + prependContentBlock(messageWithTs, { + type: "thinking", + thinking: thinkingBlocks[i].thinking, + signature: thinkingBlocks[i].signature, + }) + } + } else if (isAnthropicProtocol && reasoning && thoughtSignature && !reasoningDetails) { const thinkingBlock = { type: "thinking", thinking: reasoning,