diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 4be087394e..8c42440744 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -141,6 +141,7 @@ 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 +const MAX_AUTO_APPROVAL_RETRIES = 3 // Bounds the auto-approval retry loop (persistent API errors, e.g. HTTP 429) export interface TaskOptions extends CreateTaskOptions { provider: ClineProvider @@ -4425,6 +4426,18 @@ export class Task extends EventEmitter implements TaskLike { // note that this api_req_failed ask is unique in that we only present this option if the api hasn't streamed any content yet (ie it fails on the first chunk due), as it would allow them to hit a retry button. However if the api failed mid-stream, it could be in any arbitrary state where some tools may have executed, so that error is handled differently and requires cancelling the task entirely. if (autoApprovalEnabled) { + // Bound the retry loop before backoff: a persistent API error (e.g. HTTP 429 fair usage, + // a rate limit on the whole account) is not going to resolve by retrying harder — each + // attempt is charged against the account and postpones recovery. Stop loudly instead of + // recursing until abort. + if (retryAttempt >= MAX_AUTO_APPROVAL_RETRIES) { + throw new Error( + `[Task#attemptApiRequest] task ${this.taskId}.${this.instanceId} aborted after ` + + `${MAX_AUTO_APPROVAL_RETRIES} auto-approval retries — persistent API error ` + + `(last: ${error.message ?? JSON.stringify(serializeError(error))}). Retry loop capped (roo-extensions#3195).`, + ) + } + // Apply shared exponential backoff and countdown UX await this.backoffAndAnnounce(retryAttempt, error) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 37e228f887..7298df67b8 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -30,6 +30,7 @@ import type { ApiMessage } from "../../task-persistence" type TaskTestAccess = { getSystemPrompt: () => Promise + backoffAndAnnounce: (retryAttempt: number, error: unknown) => Promise getEnabledMcpToolsCount: () => Promise<{ enabledToolCount: number; enabledServerCount: number }> initiateTaskLoop: (userContent: Anthropic.Messages.ContentBlockParam[]) => Promise startTask: (task?: string, images?: string[]) => Promise @@ -943,6 +944,80 @@ describe("Cline", () => { expect(mockDelay).toHaveBeenCalledWith(1000) }) + it("should cap the auto-approval retry loop on a persistent API error", async () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + vi.spyOn(getTaskTestAccess(cline), "getSystemPrompt").mockResolvedValue("mock system prompt") + + // Mock delay to keep the backoff instant + const mockDelay = vi.fn().mockResolvedValue(undefined) + vi.spyOn(await import("delay"), "default").mockImplementation(mockDelay) + + const saySpy = vi.spyOn(cline, "say") + + // A stream that errors on every access — the API never succeeds. + const mockError = new Error("API Error") + const mockFailedStream = { + // eslint-disable-next-line require-yield + async *[Symbol.asyncIterator]() { + throw mockError + }, + async next() { + throw mockError + }, + async return() { + return { done: true, value: undefined } + }, + async throw(error: unknown) { + throw error + }, + async [Symbol.asyncDispose]() { + // Cleanup + }, + } as AsyncGenerator + + const providerState = await mockProvider.getState() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + ...providerState, + apiConfiguration: mockApiConfig, + autoApprovalEnabled: true, + requestDelaySeconds: 3, + }) + + let attemptCount = 0 + const createMessageSpy = vi.spyOn(cline.api, "createMessage").mockImplementation(() => { + attemptCount++ + // Fail fast if the retry loop is unbounded — guards against a hang if the cap is removed. + expect(attemptCount).toBeLessThanOrEqual(4) + return mockFailedStream + }) + + // One backoff per retry, and the cap must refuse to back off again once hit. + const backoffSpy = vi.spyOn(getTaskTestAccess(cline), "backoffAndAnnounce").mockResolvedValue(undefined) + + // 1 initial attempt + MAX_AUTO_APPROVAL_RETRIES(3) retries, then the loop must throw. + const iterator = cline.attemptApiRequest(0) + let thrown: unknown + try { + await iterator.next() + } catch (e) { + thrown = e + } + + // The stop is loud and names the last underlying error and the cap. + expect(thrown).toBeInstanceOf(Error) + expect((thrown as Error).message).toMatch(/capped.*roo-extensions#3195/) + expect((thrown as Error).message).toContain("API Error") + expect(attemptCount).toBe(4) + expect(createMessageSpy).toHaveBeenCalledTimes(4) + // Exactly as many backoffs as retries — the request that finally threw never slept. + expect(backoffSpy).toHaveBeenCalledTimes(3) + }) + it("uses the task rate limit in retry backoff when focused provider state differs", async () => { const clock = createRateLimitClock() const rateLimitConfig = {