From 31e407e71e0c6339a854414547c1a801380c0906 Mon Sep 17 00:00:00 2001 From: Luis Carmona Date: Sat, 8 Aug 2026 14:47:33 -0400 Subject: [PATCH] fix: cap the empty-assistant-message retry loop and surface diagnostics --- src/api/providers/anthropic.ts | 3 + src/api/transform/stream.ts | 8 ++ src/core/task/Task.ts | 107 +++++++++++++++++- src/core/task/__tests__/Task.spec.ts | 49 ++++++++ src/core/webview/ClineProvider.ts | 18 +++ .../ClineProvider.flicker-free-cancel.spec.ts | 33 ++++++ 6 files changed, 215 insertions(+), 3 deletions(-) diff --git a/src/api/providers/anthropic.ts b/src/api/providers/anthropic.ts index b55c8b3089..a482c6da43 100644 --- a/src/api/providers/anthropic.ts +++ b/src/api/providers/anthropic.ts @@ -277,6 +277,9 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa type: "usage", inputTokens: 0, outputTokens: chunk.usage.output_tokens || 0, + // thread stop_reason through so Task.ts can surface it in + // empty-response diagnostics. + finishReason: chunk.delta.stop_reason || undefined, } break diff --git a/src/api/transform/stream.ts b/src/api/transform/stream.ts index 960ebbe770..bf766af176 100644 --- a/src/api/transform/stream.ts +++ b/src/api/transform/stream.ts @@ -63,6 +63,14 @@ export interface ApiStreamUsageChunk { cacheReadTokens?: number reasoningTokens?: number totalCost?: number + /** + * finish/stop reason from the provider stream (e.g. "end_turn", "max_tokens") + * when available. Used to diagnose empty-response retries. + * NOTE: only the Anthropic provider populates this field today; the other providers + * leave it unset. The empty-response diagnostic therefore omits the finish_reason + * field when it is unknown rather than printing "unknown". + */ + finishReason?: string } export interface ApiStreamGroundingChunk { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index f55078b6ff..e94c9f8759 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -141,6 +141,12 @@ const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds 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 +// Hard cap on consecutive empty-assistant-message retries. Before this, the empty-response +// retry loop had NO upper bound and could resend the same unchanged oversized request forever +// at the maximum backoff delay with no give-up condition. We observed this live requiring +// ~4 retries (~7 min) before the provider recovered on its own; 5 gives one margin round +// without letting the loop run indefinitely. +const MAX_EMPTY_RESPONSE_RETRIES = 5 // Maximum consecutive empty-response retries before giving up export interface TaskOptions extends CreateTaskOptions { provider: ClineProvider @@ -321,6 +327,9 @@ export class Task extends EventEmitter implements TaskLike { consecutiveMistakeCountForEditFile: Map = new Map() consecutiveNoToolUseCount: number = 0 consecutiveNoAssistantMessagesCount: number = 0 + // timestamp (performance.now()) when the empty-assistant-message retry loop started, + // used to surface total elapsed time in the terminal error toast. + emptyResponseRetryLoopStartTimeMs: number = 0 toolUsage: ToolUsage = {} // Conversation message counts, summarized once per Task Completed @@ -2246,6 +2255,8 @@ export class Task extends EventEmitter implements TaskLike { // Reset consecutive error counters on abort (manual intervention) this.consecutiveNoToolUseCount = 0 this.consecutiveNoAssistantMessagesCount = 0 + // Also reset the empty-response retry loop timer so a later run starts fresh + this.emptyResponseRetryLoopStartTimeMs = 0 // Force final token usage update before abort event this.emitFinalTokenUsageUpdate() @@ -2772,6 +2783,10 @@ export class Task extends EventEmitter implements TaskLike { const stream = this.attemptApiRequest(currentItem.retryAttempt ?? 0, { skipProviderRateLimit: true }) let assistantMessage = "" let reasoningMessage = "" + // finish/stop reason surfaced by the provider stream (when available), included + // in empty-response diagnostics. Only the Anthropic provider populates this + // today, so the diagnostic below omits finish_reason when it is unknown. + let finishReason: string | undefined const pendingGroundingSources: GroundingSource[] = [] this.isStreaming = true @@ -2838,6 +2853,8 @@ export class Task extends EventEmitter implements TaskLike { cacheWriteTokens += chunk.cacheWriteTokens ?? 0 cacheReadTokens += chunk.cacheReadTokens ?? 0 totalCost = chunk.totalCost + // capture finish_reason if the provider surfaces it + finishReason = chunk.finishReason ?? finishReason break case "grounding": // Handle grounding sources separately from regular content @@ -3423,6 +3440,8 @@ export class Task extends EventEmitter implements TaskLike { if (hasTextContent || hasToolUses) { // Reset counter when we get a successful response with content this.consecutiveNoAssistantMessagesCount = 0 + // Reset the empty-response retry loop timer on success + this.emptyResponseRetryLoopStartTimeMs = 0 // Display grounding sources to the user if they exist if (pendingGroundingSources.length > 0) { const citationLinks = pendingGroundingSources.map((source, i) => `[${i + 1}](${source.url})`) @@ -3670,6 +3689,63 @@ export class Task extends EventEmitter implements TaskLike { } } + // Enforce a hard cap on consecutive empty responses. Previously this loop + // had NO upper bound — it could retry the same unchanged oversized request + // forever at the maximum backoff delay. Now, after + // MAX_EMPTY_RESPONSE_RETRIES consecutive empty responses we stop retrying + // and surface a terminal error instead of looping indefinitely. + if (this.emptyResponseRetryLoopStartTimeMs === 0) { + this.emptyResponseRetryLoopStartTimeMs = performance.now() + } + const emptyResponseElapsedSec = Math.round( + (performance.now() - this.emptyResponseRetryLoopStartTimeMs) / 1000, + ) + // Diagnostic detail surfaced in the live toast so the failure is diagnosable + // without grepping sidecar logs afterward. Only the Anthropic provider + // surfaces finish_reason today, so omit the field when it is unknown instead + // of printing "unknown" for every other provider. + const finishReasonDetail = finishReason ? `, finish_reason: ${finishReason}` : "" + const emptyResponseDetail = + `(consecutive empty responses: ${this.consecutiveNoAssistantMessagesCount}, ` + + `retryAttempt: ${currentItem.retryAttempt ?? 0}, elapsed: ${emptyResponseElapsedSec}s` + + `${finishReasonDetail})` + + if (this.consecutiveNoAssistantMessagesCount >= MAX_EMPTY_RESPONSE_RETRIES) { + // Give up: the user message was already removed above, so re-add it, surface + // a clear terminal error, and end the turn cleanly (mirrors the "user declined + // to retry" terminal path below so the task does not hang). + await this.addToApiConversationHistory({ + role: "user", + content: currentUserContent, + }) + // Append the Failure marker immediately after the user message so the + // persisted history never ends with a trailing user message, even if an + // abort lands while the user message is being persisted. + await this.addToApiConversationHistory({ + role: "assistant", + content: [{ type: "text", text: "Failure: I repeatedly did not provide a response." }], + }) + // Abort check after the history is consistent: if the user hit Stop mid + // give-up, bail before announcing the terminal error. + if (this.abort) { + console.log( + `[Task#${this.taskId}.${this.instanceId}] Task aborted during empty-response give-up; history finalized`, + ) + return false + } + await this.say( + "error", + `Unexpected API Response: The language model repeatedly returned no response after ` + + `${MAX_EMPTY_RESPONSE_RETRIES} consecutive attempts. ${emptyResponseDetail}`, + ) + // Reset the retry counters on the terminal give-up path so a later Stop on + // the same task takes the normal graceful path in cancelTask rather than the + // hard-abort gate. + this.consecutiveNoAssistantMessagesCount = 0 + this.emptyResponseRetryLoopStartTimeMs = 0 + return false + } + // Check if we should auto-retry or prompt the user // Reuse the state variable from above if (state?.autoApprovalEnabled) { @@ -3677,7 +3753,8 @@ export class Task extends EventEmitter implements TaskLike { await this.backoffAndAnnounce( currentItem.retryAttempt ?? 0, new Error( - "Unexpected API Response: The language model did not provide any assistant messages. This may indicate an issue with the API or the model's output.", + `Unexpected API Response: The language model did not provide any assistant messages. ` + + `This may indicate an issue with the API or the model's output. ${emptyResponseDetail}`, ), ) @@ -3705,7 +3782,7 @@ 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. ${emptyResponseDetail}`, ) if (response === "yesButtonClicked") { @@ -3735,7 +3812,8 @@ export class Task extends EventEmitter implements TaskLike { await this.say( "error", - "Unexpected API Response: The language model did not provide any assistant messages. This may indicate an issue with the API or the model's output.", + `Unexpected API Response: The language model did not provide any assistant messages. ` + + `This may indicate an issue with the API or the model's output. ${emptyResponseDetail}`, ) // Synthetic assistant message recording the failure -- increment @@ -3746,6 +3824,11 @@ export class Task extends EventEmitter implements TaskLike { content: [{ type: "text", text: "Failure: I did not provide a response." }], }) this.messageCounts.assistant++ + // Reset the retry counters on the user-declined terminal path so a later + // Stop on the same task takes the normal graceful path in cancelTask + // rather than the hard-abort gate. + this.consecutiveNoAssistantMessagesCount = 0 + this.emptyResponseRetryLoopStartTimeMs = 0 } } } @@ -4498,6 +4581,24 @@ export class Task extends EventEmitter implements TaskLike { headerText = "Unknown error" } + // Surface a curated errorDetails summary so a live "Provider Error" toast is + // diagnosable without grepping sidecar logs afterward. The raw array can include + // provider-internal metadata (e.g. google.rpc.RetryInfo with its retryDelay, already + // parsed above for the backoff) that is opaque in a user-facing toast, so filter that + // entry out before serializing the remainder. + const backoffDetailLines: string[] = [] + if (Array.isArray(error?.errorDetails) && error.errorDetails.length > 0) { + const sanitizedDetails = error.errorDetails.filter( + (d: { "@type"?: string }) => d?.["@type"] !== "type.googleapis.com/google.rpc.RetryInfo", + ) + if (sanitizedDetails.length > 0) { + backoffDetailLines.push(`errorDetails: ${JSON.stringify(sanitizedDetails)}`) + } + } + if (backoffDetailLines.length > 0) { + headerText = `${headerText}\n${backoffDetailLines.join("\n")}` + } + headerText = headerText ? `${headerText}\n` : "" // Show countdown timer with exponential backoff diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 60bc2f3192..f6eae2854b 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -457,6 +457,55 @@ describe("Cline", () => { ]) expect(task.messageCounts).toEqual({ user: 1, assistant: 1 }) }) + + it("gives up after MAX_EMPTY_RESPONSE_RETRIES consecutive empty responses (real retry loop)", async () => { + const task = await createTaskWithManualRetries() + + // Drive the REAL request loop: auto-approval is off, so answer the retry prompt + // affirmatively until the hard cap trips. Each attempt surfaces a finish_reason + // (e.g. a model that hit max_tokens) but yields no assistant content. + vi.spyOn(task, "ask").mockResolvedValue({ response: "yesButtonClicked" } satisfies TaskAskResult) + // Call-through spy: records error announcements while still letting the real `say` + // run so the `api_req_started` placeholder is added to clineMessages + // (recursivelyMakeClineRequests updates that placeholder and would otherwise index + // an empty list). + const saySpy = vi.spyOn(task, "say") + vi.spyOn(task, "attemptApiRequest").mockImplementation(() => + stream([{ type: "usage", inputTokens: 0, outputTokens: 0, finishReason: "max_tokens" }]), + ) + + const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "original user request" }]) + + // Loop exits with a terminal failure (does not hang). + expect(result).toBe(false) + + // Retry counters/timer are reset on the terminal give-up path so a later Stop on + // the same task would not hit cancelTask's hard-abort gate. + expect(task.consecutiveNoAssistantMessagesCount).toBe(0) + expect(task.emptyResponseRetryLoopStartTimeMs).toBe(0) + + // Repeated empty responses announce the MODEL_NO_ASSISTANT_MESSAGES marker... + const errorCalls = saySpy.mock.calls.filter(([type]) => type === "error") + expect(errorCalls.some(([, text]) => text === "MODEL_NO_ASSISTANT_MESSAGES")).toBe(true) + + // ...and the hard cap trips with the terminal give-up error carrying the diagnostic + // detail, including the finish_reason propagated from the (mocked) provider stream. + const giveUpCall = errorCalls.find(([, text]) => + String(text).startsWith("Unexpected API Response: The language model repeatedly"), + ) + expect(giveUpCall).toBeDefined() + expect(String(giveUpCall?.[1])).toContain("consecutive empty responses: 5") + expect(String(giveUpCall?.[1])).toContain("finish_reason: max_tokens") + + // History ends with the synthetic Failure message, never a trailing user message. + expect(task.apiConversationHistory).toMatchObject([ + { role: "user", content: [{ type: "text", text: "original user request" }] }, + { + role: "assistant", + content: [{ type: "text", text: "Failure: I repeatedly did not provide a response." }], + }, + ]) + }) }) describe("constructor", () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 206d6ca611..69c5370175 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -3342,6 +3342,24 @@ export class ClineProvider return } + // Hard-abort instead of graceful rehydrate when the task is confirmed mid + // empty-response retry loop. The graceful path below cancels the request, marks the + // task "interrupted", and REHYDRATES the same task with its still-oversized history — + // so an unbounded empty-response retry loop would resume immediately after Stop. + // For a task already stuck resending an unchanged retry + // (consecutiveNoAssistantMessagesCount > 0), evict it from the stack entirely + // (abortTask(true) via removeClineFromStack, no rehydrate) so a fresh task/context is + // required to continue. Normal Stops for tasks NOT in this retry loop (counter === 0) + // are completely unaffected. + if (task.consecutiveNoAssistantMessagesCount > 0) { + this.log( + `[cancelTask] Task ${task.taskId}.${task.instanceId} is mid empty-response retry loop ` + + `(${task.consecutiveNoAssistantMessagesCount} consecutive empty responses); using hard abort instead of rehydrate`, + ) + await this.evictCurrentTask() + return + } + console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`) await this.cancelTaskInternal(task) } diff --git a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts index 3513bd3bd5..62234e511d 100644 --- a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts @@ -16,6 +16,7 @@ type MockTask = Partial & isStreaming?: boolean didFinishAbortingStream?: boolean isWaitingForFirstChunk?: boolean + consecutiveNoAssistantMessagesCount?: number } type CreatedHistoryTask = Awaited> @@ -446,6 +447,38 @@ describe("ClineProvider flicker-free cancel", () => { expect(mockTask2.emit).toHaveBeenCalledWith("taskFocused") }) + it("hard-aborts (evicts) the current task when it is mid empty-response retry loop", async () => { + seedRegistry(provider, mockTask1) + mockTask1.consecutiveNoAssistantMessagesCount = 3 + + const evictSpy = vi.spyOn(provider, "evictCurrentTask").mockResolvedValue(undefined) + const cancelInternalSpy = vi + .spyOn(provider as unknown as { cancelTaskInternal: () => Promise }, "cancelTaskInternal") + .mockResolvedValue(undefined) + + await provider.cancelTask() + + // The hard-abort gate must evict the task and never reach the graceful path. + expect(evictSpy).toHaveBeenCalledTimes(1) + expect(cancelInternalSpy).not.toHaveBeenCalled() + }) + + it("takes the normal graceful cancel path when not in an empty-response retry loop", async () => { + seedRegistry(provider, mockTask1) + mockTask1.consecutiveNoAssistantMessagesCount = 0 + + const evictSpy = vi.spyOn(provider, "evictCurrentTask").mockResolvedValue(undefined) + const cancelInternalSpy = vi + .spyOn(provider as unknown as { cancelTaskInternal: () => Promise }, "cancelTaskInternal") + .mockResolvedValue(undefined) + + await provider.cancelTask() + + // With the counter at 0 the gate must not fire; the graceful path runs. + expect(evictSpy).not.toHaveBeenCalled() + expect(cancelInternalSpy).toHaveBeenCalledTimes(1) + }) + it("should remove task from stack when creating different task", async () => { // Setup: Add a task to the registry first seedRegistry(provider, mockTask1)