From e7c1d58125054d602c7286847536db5a886249ea Mon Sep 17 00:00:00 2001 From: Roomote Date: Wed, 9 Sep 2026 21:24:16 +0000 Subject: [PATCH 1/4] [Fix] Billed requests produce no response from silent mid-stream retries, unhandled max_tokens stops, and dropped thinking signatures - Bound mid-stream API failure retries (3 automatic attempts), announce every retry through the visible backoff countdown, and ask the user once the budget is exhausted instead of looping silently. - Propagate the response stop_reason through the usage stream and stop retrying when an empty response ended with max_tokens, surfacing remediation guidance instead of re-billing the full context. - Capture Anthropic thinking-block signatures (signature_delta) and replay each signed thinking block unchanged on tool-use continuations. --- src/api/providers/__tests__/anthropic.spec.ts | 265 ++++++++++++++++++ src/api/providers/anthropic.ts | 77 ++++- src/api/transform/stream.ts | 6 + src/core/task/Task.ts | 146 ++++++++-- src/core/task/__tests__/Task.spec.ts | 212 ++++++++++++++ .../__tests__/apiConversationHistory.spec.ts | 26 ++ src/core/task/apiConversationHistory.ts | 15 +- 7 files changed, 726 insertions(+), 21 deletions(-) diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 7d54116a38..1a9c0eafbf 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -859,6 +859,271 @@ 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("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..a31b43dfb8 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,16 @@ 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 + let pendingThinkingSignature = "" + let pendingThinkingText = "" + for await (const chunk of stream) { switch (chunk.type) { case "message_start": { @@ -294,6 +318,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 +328,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 +368,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 +396,23 @@ 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 } + } + pendingThinkingSignature = "" + pendingThinkingText = "" + } break } } @@ -382,6 +435,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 fae796db6b..dd8b313a56 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -171,6 +171,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 @@ -3192,6 +3196,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 @@ -3258,6 +3267,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 @@ -3645,16 +3655,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) { @@ -3666,17 +3677,77 @@ 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. + let removedMidStreamUserMessage = false + if (currentUserContent.length > 0 && this.apiConversationHistory.length > 0) { + const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] + if (lastMessage.role === "user") { + this.apiConversationHistory.pop() + this.messageCounts.user-- + removedMidStreamUserMessage = true + } + } + + 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 + // re-added exactly once on the next iteration. + stack.push({ + userContent: currentUserContent, + includeFileDetails: false, + retryAttempt: 0, + userMessageWasRemoved: removedMidStreamUserMessage, + }) + + continue + } + + // User declined to retry: restore the user message, surface + // the error, record the failure, and stop the loop. + if (removedMidStreamUserMessage) { + await this.addToApiConversationHistory({ + role: "user", + content: currentUserContent, + }) + this.messageCounts.user++ + } + + 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 { @@ -4054,9 +4125,48 @@ export class Task extends EventEmitter implements TaskLike { } } - // 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.addToApiConversationHistory({ + role: "user", + content: currentUserContent, + }) + this.messageCounts.user++ + } + + 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, @@ -4089,7 +4199,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") { diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 7418920cb1..1ad5b6eb7b 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -465,6 +465,218 @@ describe("Cline", () => { ]) expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) }) + + 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 + } + + 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") + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => + 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).toMatchObject([ + { role: "user", content: [{ type: "text", text: "original user request" }] }, + { + role: "assistant", + content: [ + { + type: "text", + text: "Failure: response hit the max output token limit before producing any visible content.", + }, + ], + }, + ]) + 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(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.messageCounts).toEqual({ user: 1, assistant: 1 }) + }) + }) + + describe("mid-stream retries", () => { + function stream(chunks: ApiStreamChunk[]): AsyncGenerator { + return (async function* () { + yield* chunks + })() + } + + 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 + })() + } + + 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 + } + + it("announces each automatic retry and asks the user after the cap is exhausted", 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(() => 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) + // 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") + // Declined retry surfaces the error and records the failure without + // losing or duplicating the user message. + expect(saySpy.mock.calls.some(([type]) => type === "error")).toBe(true) + expect(task.apiConversationHistory).toMatchObject([ + { role: "user", content: [{ type: "text", text: "original user request" }] }, + { + role: "assistant", + content: [{ type: "text", text: "Failure: the API stream failed mid-response." }], + }, + ]) + 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) + 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 historyAtSuccess: ApiMessage[] | undefined + vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + attempt++ + 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 re-added the user message exactly once. + expect(historyAtSuccess).toHaveLength(1) + expect(historyAtSuccess?.[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), + }) + // 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..1c0d4a10da 100644 --- a/src/core/task/__tests__/apiConversationHistory.spec.ts +++ b/src/core/task/__tests__/apiConversationHistory.spec.ts @@ -51,6 +51,32 @@ 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("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, From 9dd0ec683cec62e90cafdeee1df17afa0773cf46 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 00:19:49 +0000 Subject: [PATCH 2/4] [Fix] Retry restore duplicates the user turn in persisted API history - Restore the exact removed user-message record (messageId/ts) instead of rebuilding it, so merge-on-save never duplicates the user turn on disk. - Add negative guard tests: non-Anthropic protocols never receive thinking blocks, and reasoning_details takes precedence over getThinkingBlocks. - Kill surviving mutation-diff mutants: stray thinking-delta index guard, wrong-index content_block_stop, unsigned thinking block completion, and includeFileDetails staying false on retries; document unobservable initializers with Stryker disable rationales. - Deduplicate the retry-suite test helpers into one shared scope. --- src/api/providers/__tests__/anthropic.spec.ts | 119 ++++++++++ src/api/providers/anthropic.ts | 4 + src/core/task/Task.ts | 73 +++--- src/core/task/__tests__/Task.spec.ts | 210 +++++++++--------- .../__tests__/apiConversationHistory.spec.ts | 45 ++++ 5 files changed, 321 insertions(+), 130 deletions(-) diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 1a9c0eafbf..ce757d425a 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -1067,6 +1067,125 @@ describe("AnthropicHandler", () => { ]) }) + 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([ diff --git a/src/api/providers/anthropic.ts b/src/api/providers/anthropic.ts index a31b43dfb8..c8831d54ef 100644 --- a/src/api/providers/anthropic.ts +++ b/src/api/providers/anthropic.ts @@ -282,7 +282,9 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa 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) { @@ -410,7 +412,9 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa }) 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 diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index dd8b313a56..85ae258dc1 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1102,6 +1102,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) @@ -2918,6 +2931,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 }] @@ -3060,8 +3074,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 @@ -3692,14 +3714,14 @@ export class Task extends EventEmitter implements TaskLike { // 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. - let removedMidStreamUserMessage = false + // message) does not duplicate it in history. Keep the exact + // record so a restore preserves its persisted identity. + let removedMidStreamUserMessage: ApiMessage | undefined if (currentUserContent.length > 0 && this.apiConversationHistory.length > 0) { const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] if (lastMessage.role === "user") { - this.apiConversationHistory.pop() + removedMidStreamUserMessage = this.apiConversationHistory.pop() this.messageCounts.user-- - removedMidStreamUserMessage = true } } @@ -3712,12 +3734,13 @@ export class Task extends EventEmitter implements TaskLike { await this.say("api_req_retried") // Reset the automatic retry budget; the user message is - // re-added exactly once on the next iteration. + // restored exactly once on the next iteration. stack.push({ userContent: currentUserContent, includeFileDetails: false, retryAttempt: 0, - userMessageWasRemoved: removedMidStreamUserMessage, + userMessageWasRemoved: removedMidStreamUserMessage !== undefined, + removedUserMessage: removedMidStreamUserMessage, }) continue @@ -3726,11 +3749,7 @@ export class Task extends EventEmitter implements TaskLike { // User declined to retry: restore the user message, surface // the error, record the failure, and stop the loop. if (removedMidStreamUserMessage) { - await this.addToApiConversationHistory({ - role: "user", - content: currentUserContent, - }) - this.messageCounts.user++ + await this.restoreApiHistoryUserMessage(removedMidStreamUserMessage) } await this.say( @@ -4114,14 +4133,14 @@ 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 } } @@ -4132,11 +4151,7 @@ export class Task extends EventEmitter implements TaskLike { // surface it and stop instead of retrying. if (lastStopReason === "max_tokens") { if (removedCurrentUserMessage) { - await this.addToApiConversationHistory({ - role: "user", - content: currentUserContent, - }) - this.messageCounts.user++ + await this.restoreApiHistoryUserMessage(removedCurrentUserMessage) } await this.say( @@ -4190,7 +4205,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 @@ -4215,20 +4231,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 1ad5b6eb7b..e9a8b7d5f4 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -25,6 +25,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" @@ -399,32 +400,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 @@ -451,37 +453,35 @@ 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.messageCounts).toEqual({ user: 1, assistant: 1 }) - }) - - async function createTaskWithAutoApproval(autoApprovalEnabled: boolean) { - const task = new Task({ - provider: mockProvider, - apiConfiguration: mockApiConfig, - task: "test task", - startTask: false, + expect(task.apiConversationHistory).toHaveLength(2) + expect(task.apiConversationHistory[0]).toMatchObject({ + role: "user", + content: expect.arrayContaining([expect.objectContaining({ text: "original user request" })]), }) - const state = await mockProvider.getState() - vi.spyOn(mockProvider, "getState").mockResolvedValue({ - ...state, - apiConfiguration: mockApiConfig, - autoApprovalEnabled, + expect(task.apiConversationHistory[1]).toMatchObject({ + role: "assistant", + content: [{ type: "text", text: "Failure: I did not provide a response." }], }) - vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) - return task - } + // 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 @@ -489,12 +489,14 @@ describe("Cline", () => { const task = await createTaskWithAutoApproval(true) const saySpy = vi.spyOn(task, "say") const askSpy = vi.spyOn(task, "ask") - const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => - stream([ + 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" }]) @@ -506,18 +508,24 @@ describe("Cline", () => { ([type, text]) => type === "error" && typeof text === "string" && text.includes("max_tokens"), ), ).toBe(true) - expect(task.apiConversationHistory).toMatchObject([ - { role: "user", content: [{ type: "text", text: "original user request" }] }, - { - role: "assistant", - content: [ - { - type: "text", - text: "Failure: response hit the max output token limit before producing any visible content.", - }, - ], - }, - ]) + 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 }) }) @@ -542,21 +550,20 @@ describe("Cline", () => { expect(retryAnnouncements).toHaveLength(3) expect(askSpy).toHaveBeenCalledTimes(1) expect(askSpy.mock.calls[0]?.[0]).toBe("api_req_failed") - 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." }], + }) expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) }) }) describe("mid-stream retries", () => { - function stream(chunks: ApiStreamChunk[]): AsyncGenerator { - return (async function* () { - yield* chunks - })() - } - function failingStream(error: Error): AsyncGenerator { return (async function* () { // Yield one chunk first so the failure is genuinely mid-stream. @@ -565,38 +572,29 @@ describe("Cline", () => { })() } - 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 - } - 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) - const attemptSpy = vi - .spyOn(task, "attemptApiRequest") - .mockImplementation(() => failingStream(new Error("overloaded_error"))) + 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( @@ -608,13 +606,19 @@ describe("Cline", () => { // Declined retry surfaces the error and records the failure without // losing or duplicating the user message. expect(saySpy.mock.calls.some(([type]) => type === "error")).toBe(true) - expect(task.apiConversationHistory).toMatchObject([ - { role: "user", content: [{ type: "text", text: "original user request" }] }, - { - role: "assistant", - content: [{ type: "text", text: "Failure: the API stream failed mid-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: 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 }) }) @@ -649,9 +653,11 @@ describe("Cline", () => { }) 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" }]) @@ -667,12 +673,16 @@ describe("Cline", () => { // 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 re-added the user message exactly once. + // 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) // 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 }) diff --git a/src/core/task/__tests__/apiConversationHistory.spec.ts b/src/core/task/__tests__/apiConversationHistory.spec.ts index 1c0d4a10da..a4e33fee71 100644 --- a/src/core/task/__tests__/apiConversationHistory.spec.ts +++ b/src/core/task/__tests__/apiConversationHistory.spec.ts @@ -77,6 +77,51 @@ describe("prepareApiConversationMessage", () => { ]) }) + 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("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" }, From 8153b0598a2eb79ecdb4d5b9a6a484d607002c89 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 00:38:00 +0000 Subject: [PATCH 3/4] test: cover retry and thinking guard boundaries --- src/core/task/Task.ts | 18 +++++++++-------- src/core/task/__tests__/Task.spec.ts | 10 +++++++++- .../__tests__/apiConversationHistory.spec.ts | 20 +++++++++++++++++++ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 85ae258dc1..5b78b8d220 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -3717,12 +3717,13 @@ export class Task extends EventEmitter implements TaskLike { // message) does not duplicate it in history. Keep the exact // record so a restore preserves its persisted identity. let removedMidStreamUserMessage: ApiMessage | undefined - if (currentUserContent.length > 0 && this.apiConversationHistory.length > 0) { - const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] - if (lastMessage.role === "user") { - removedMidStreamUserMessage = this.apiConversationHistory.pop() - this.messageCounts.user-- - } + 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( @@ -3734,12 +3735,13 @@ export class Task extends EventEmitter implements TaskLike { await this.say("api_req_retried") // Reset the automatic retry budget; the user message is - // restored exactly once on the next iteration. + // 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, - userMessageWasRemoved: removedMidStreamUserMessage !== undefined, removedUserMessage: removedMidStreamUserMessage, }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index e9a8b7d5f4..17ecb5a446 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -550,6 +550,7 @@ describe("Cline", () => { 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", @@ -603,9 +604,14 @@ describe("Cline", () => { 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]) => type === "error")).toBe(true) + 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", @@ -644,6 +650,7 @@ describe("Cline", () => { 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++ @@ -683,6 +690,7 @@ describe("Cline", () => { }) 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 }) diff --git a/src/core/task/__tests__/apiConversationHistory.spec.ts b/src/core/task/__tests__/apiConversationHistory.spec.ts index a4e33fee71..292db40806 100644 --- a/src/core/task/__tests__/apiConversationHistory.spec.ts +++ b/src/core/task/__tests__/apiConversationHistory.spec.ts @@ -100,6 +100,26 @@ describe("prepareApiConversationMessage", () => { ]) }) + 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. From 29258f1d58e45ce4fd11fecf7eb50ad9aac43022 Mon Sep 17 00:00:00 2001 From: Roomote Date: Fri, 11 Sep 2026 01:16:21 +0000 Subject: [PATCH 4/4] test: model API retry persistence and replay signed thinking --- docs/architecture/task-lifecycle-model.md | 23 +++-- package.json | 2 +- scripts/check-api-retry-persistence.ts | 93 +++++++++++++++++++ src/api/providers/__tests__/anthropic.spec.ts | 54 +++++++++++ 4 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 scripts/check-api-retry-persistence.ts diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index d39c0fd9e2..414a9b5e53 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -12,7 +12,8 @@ The command runs five independent bounded submodels in sequence: 2. shared-store concurrency across task-history hosts; 3. the task cleanup protocol; 4. request-stream parser scoping; and -5. completion persistence. +5. completion persistence; and +6. 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. @@ -115,22 +116,24 @@ 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 The following map separates issue observations from the architectural interpretation encoded here. Open issues can change after this document is written; follow each link for current status. -| Issue and directly observed evidence | Derived protocol rule | Production transition and current check | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [#1469](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1469): the issue report states that a barrier-controlled two-host run reproduced an old child completion clearing a newer handoff 25/25 times. | Completion is conditional on the parent still awaiting that exact child; a live-linked child must remain owned by its parent. | `completeDelegatedChild` rejects stale authoritative input. The lifecycle explorer checks that reducer rule, while the shared-store explorer reproduces the cross-host stale-cache counterexample with an exact causal witness. | -| [#1021](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1021): an in-flight `saveClineMessages` can restore parent/root IDs after abandonment cleared them. | Detachment should be monotonic: later lifecycle work must not reattach an abandoned child. | `abandonDelegatedChild` clears both sides. The shared-store explorer proves the detach commit occurs, then reproduces a refreshed-cache delta that preserves interrupted status while restoring stale live-task lineage. | -| [#1453](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1453), under user report [#1279](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1279): CI observed `TaskCompleted` before restart-visible API history once; 120 local repetitions did not reproduce it. | Completion implies restart-visible assistant history. Delayed or failed writes keep completion pending, and cancellation settles readiness without starting stale retries or emitting completion. | The completion persistence explorer checks the bounded event-ordering and cancellation contract for standalone and delegated tasks. Focused `Task` and `AttemptCompletionTool` tests cover the production adapter; `restart-persistence.test.ts` verifies visibility through a fresh extension host. | -| [#921](https://github.com/Zoo-Code-Org/Zoo-Code/issues/921): delegation across parallel tabs lacks coverage for different view-local mode/profile state. | Delegation must bind an explicit immutable execution-context snapshot rather than read whichever view is focused later. | The persisted ownership transition is covered; mode/profile snapshot isolation is outside this state model and belongs in a production adapter/model-based test. | -| [#920](https://github.com/Zoo-Code-Org/Zoo-Code/issues/920): issue analysis identifies a missing cross-instance history-update test and potential lost writes. | Distinct task writes must not overwrite one another, and same-task conflicts need an explicit merge/ownership rule. | The shared-store explorer checks distinct-task writes and same-record independent deltas. Cross-instance store tests retain production API coverage, and the synchronized real-filesystem smoke test exercises the actual lock/write path without claiming exhaustive filesystem proof. | -| [#369](https://github.com/Zoo-Code-Org/Zoo-Code/issues/369) and [#372](https://github.com/Zoo-Code-Org/Zoo-Code/issues/372): planned fan-out keeps a parent live while a child runs and requires completion routing by explicit parent ID, single-writer result readiness, permit release, and orphan cleanup. | Persisted `delegated` status is ownership, not proof that the parent instance is suspended. Completion must route by IDs; scheduler resources and live-instance state need separate invariants. | Nested and sibling lifecycle ownership are covered. Scheduler permits, live/suspended parent selection, orphan cancellation, and single-writer message readiness must be added when fan-out lands; they should not be folded into `HistoryItem` fields prematurely. | +| Issue and directly observed evidence | Derived protocol rule | Production transition and current check | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [#1469](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1469): the issue report states that a barrier-controlled two-host run reproduced an old child completion clearing a newer handoff 25/25 times. | Completion is conditional on the parent still awaiting that exact child; a live-linked child must remain owned by its parent. | `completeDelegatedChild` rejects stale authoritative input. The lifecycle explorer checks that reducer rule, while the shared-store explorer reproduces the cross-host stale-cache counterexample with an exact causal witness. | +| [#1021](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1021): an in-flight `saveClineMessages` can restore parent/root IDs after abandonment cleared them. | Detachment should be monotonic: later lifecycle work must not reattach an abandoned child. | `abandonDelegatedChild` clears both sides. The shared-store explorer proves the detach commit occurs, then reproduces a refreshed-cache delta that preserves interrupted status while restoring stale live-task lineage. | +| [#1453](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1453), under user report [#1279](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1279): CI observed `TaskCompleted` before restart-visible API history once; 120 local repetitions did not reproduce it. | Completion implies restart-visible assistant history. Delayed or failed writes keep completion pending, and cancellation settles readiness without starting stale retries or emitting completion. | The completion persistence explorer checks the bounded event-ordering and cancellation contract for standalone and delegated tasks. Focused `Task` and `AttemptCompletionTool` tests cover the production adapter; `restart-persistence.test.ts` verifies visibility through a fresh extension host. | +| [#921](https://github.com/Zoo-Code-Org/Zoo-Code/issues/921): delegation across parallel tabs lacks coverage for different view-local mode/profile state. | Delegation must bind an explicit immutable execution-context snapshot rather than read whichever view is focused later. | The persisted ownership transition is covered; mode/profile snapshot isolation is outside this state model and belongs in a production adapter/model-based test. | +| [#920](https://github.com/Zoo-Code-Org/Zoo-Code/issues/920): issue analysis identifies a missing cross-instance history-update test and potential lost writes. | Distinct task writes must not overwrite one another, and same-task conflicts need an explicit merge/ownership rule. | The shared-store explorer checks distinct-task writes and same-record independent deltas. Cross-instance store tests retain production API coverage, and the synchronized real-filesystem smoke test exercises the actual lock/write path without claiming exhaustive filesystem proof. | +| [#369](https://github.com/Zoo-Code-Org/Zoo-Code/issues/369) and [#372](https://github.com/Zoo-Code-Org/Zoo-Code/issues/372): planned fan-out keeps a parent live while a child runs and requires completion routing by explicit parent ID, single-writer result readiness, permit release, and orphan cleanup. | Persisted `delegated` status is ownership, not proof that the parent instance is suspended. Completion must route by IDs; scheduler resources and live-instance state need separate invariants. | Nested and sibling lifecycle ownership are covered. Scheduler permits, live/suspended parent selection, orphan cancellation, and single-writer message readiness must be added when fan-out lands; they should not be folded into `HistoryItem` fields prematurely. | | [#1468](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1468): a late chunk from one request combined tool identity with arguments from another request; rerun passed. | Every stream accumulator needs a request/task generation key, and late events cannot mutate another scope. | Separate protocol. The [native tool-call parser request-scope model](./native-tool-call-parser-scoping-model.md), whose source of truth is `scripts/check-native-tool-call-parser-scoping.ts`, exhaustively replays bounded production-parser interleavings without adding fields to this lifecycle model. | -| [#612](https://github.com/Zoo-Code-Org/Zoo-Code/issues/612): the CLI copied a status union and omitted `interrupted`. | Lifecycle vocabulary should have one type owner. | `HistoryItemStatus` is derived from `HistoryItem`, and production/checker transitions share `taskLifecycle.ts`; consumers should import rather than copy the union. | +| [#612](https://github.com/Zoo-Code-Org/Zoo-Code/issues/612): the CLI copied a status union and omitted `interrupted`. | Lifecycle vocabulary should have one type owner. | `HistoryItemStatus` is derived from `HistoryItem`, and production/checker transitions share `taskLifecycle.ts`; consumers should import rather than copy the union. | The issue-derived cases intentionally map to bug classes rather than issue-specific flags. In particular, stale event ownership, monotonic terminal/detached state, explicit scope, and single-writer boundaries generalize to future concurrent task work. diff --git a/package.json b/package.json index 94f2d52e27..25bb165664 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 && 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 && 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", "test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only", 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 ce757d425a..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", () => ({ @@ -1067,6 +1069,58 @@ describe("AnthropicHandler", () => { ]) }) + 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([