From 7da74083be1d61e679c5cdf780af07ebc61ec888 Mon Sep 17 00:00:00 2001 From: jsboige Date: Fri, 21 Aug 2026 12:18:47 +0200 Subject: [PATCH 1/3] fix(task): bound the auto-approval retry loop in attemptApiRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autoApprovalEnabled path of attemptApiRequest recursed with no cap — only abort stopped it. On a persistent API error (e.g. HTTP 429 fair usage, a whole-account rate limit), each retry is charged against the account and worsens the condition; observed 17 retries (~2h50) and 48 (~8h) in production. Add MAX_AUTO_APPROVAL_RETRIES = 3 (same convention as MAX_CONTEXT_WINDOW_RETRIES) checked before backoffAndAnnounce so the refused request never sleeps on a backoff that cannot succeed, and stop loudly with an Error naming the cap and the last underlying error. The context-window and interactive retry paths are untouched. Add a mutation-checked spec: always-failing stream + autoApprovalEnabled must throw after MAX+1 total attempts; the in-mock guard fails fast if the cap is removed. Co-Authored-By: Claude-Code --- src/core/task/Task.ts | 13 +++++ src/core/task/__tests__/Task.spec.ts | 75 ++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) 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 = { From d6d4f6a873dec2af3031e0e5a6c2ef623e9cee9c Mon Sep 17 00:00:00 2001 From: jsboige Date: Tue, 8 Sep 2026 22:46:41 +0200 Subject: [PATCH 2/3] fix(task): bound the retry loop at the consumer boundary, not just attemptApiRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The internal cap did not bound production: the streaming_failed handler in recursivelyMakeClineRequests caught the capped error and re-pushed the stack item, and since attemptApiRequest only checks the cap after an error occurs, every re-push issued another full API request — the loop never ended. - ApiRetryCapExceededError (exported): distinct terminal signal from the first-chunk cap; the consumer honors it before the generic retry path — loud say("error"), abortReason=streaming_failed, abortTask, re-throw into the outer catch so the parent sees didEndLoop=true (a bare break reported "completed normally"). - Consumer-side cap for mid-stream failures: retryAttempt at MAX_AUTO_APPROVAL_RETRIES with auto-approval on stops instead of re-pushing. - Context-window exhaustion falling through the cap now names its own cause instead of blaming the auto-approval cap. - Constant comment scoped to what it actually bounds (first-chunk + mid-stream re-push; not mid-stream without auto-approval). - Tests: cap asserted through the recursivelyMakeClineRequests boundary (4 requests / 3 backoffs, mutation-hardened), marker terminal (1 attempt / 0 backoffs), context-window message, instanceof on the existing test. Mutation-verified: disabling the consumer guard reddens both consumer tests; disabling the context-window cause reddens the message test. Refs roo-extensions#3195 --- src/core/task/Task.ts | 56 ++++++++- src/core/task/__tests__/Task.spec.ts | 171 ++++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 6 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 8c42440744..9ad587a0bf 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -141,7 +141,22 @@ 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) +// Bounds the auto-approval retry loop for persistent API errors (e.g. HTTP 429 fair usage). +// Applied at both boundaries: first-chunk failures inside attemptApiRequest, and the +// streaming_failed re-push loop in recursivelyMakeClineRequests. Does not govern mid-stream +// retries when auto-approval is disabled (those re-push without backoff, upstream behavior). +const MAX_AUTO_APPROVAL_RETRIES = 3 + +// Terminal signal that the auto-approval retry cap is spent. Thrown by attemptApiRequest on +// first-chunk failures; the streaming_failed handler in recursivelyMakeClineRequests must +// honor it instead of re-pushing — attemptApiRequest only checks the cap after an error +// occurs, so a re-push would issue another full API request and loop forever. +export class ApiRetryCapExceededError extends Error { + constructor(message: string) { + super(message) + this.name = "ApiRetryCapExceededError" + } +} export interface TaskOptions extends CreateTaskOptions { provider: ClineProvider @@ -3297,6 +3312,35 @@ export class Task extends EventEmitter implements TaskLike { // Apply exponential backoff similar to first-chunk errors when auto-resubmit is enabled const stateForBackoff = await this.providerRef.deref()?.getState() + + // Terminal — auto-approval retry cap spent (roo-extensions#3195). Stop loudly + // instead of re-pushing: attemptApiRequest checks the cap only after an error, + // so every re-push would issue another full API request and the loop would + // never end. ApiRetryCapExceededError comes from the first-chunk path (only + // thrown with auto-approval on, so it stays terminal even if the toggle was + // flipped mid-flight); the retryAttempt check bounds mid-stream failures, + // which re-enter here directly with the counter already at the cap. + if ( + error instanceof ApiRetryCapExceededError || + (stateForBackoff?.autoApprovalEnabled && + (currentItem.retryAttempt ?? 0) >= MAX_AUTO_APPROVAL_RETRIES) + ) { + const capMessage = + error instanceof ApiRetryCapExceededError + ? error.message + : `[Task#recursivelyMakeClineRequests] task ${this.taskId}.${this.instanceId} aborted after ` + + `${MAX_AUTO_APPROVAL_RETRIES} mid-stream auto-approval retries — persistent API error ` + + `(last: ${rawErrorMessage}). Retry loop capped (roo-extensions#3195).` + await this.say("error", capMessage) + this.abortReason = "streaming_failed" + await this.abortTask() + // Re-throw into the loop's outer catch so the parent sees + // didEndLoop=true — a bare `break` exits the stack loop and + // reports "completed normally" (return false) for what is a + // task-level terminal stop. + throw error instanceof ApiRetryCapExceededError ? error : new Error(capMessage) + } + if (stateForBackoff?.autoApprovalEnabled) { await this.backoffAndAnnounce(currentItem.retryAttempt ?? 0, error) @@ -4431,9 +4475,13 @@ export class Task extends EventEmitter implements TaskLike { // 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 ` + + // Context-window errors fall through to this branch once their own retries are + // spent — name that cause instead of blaming the auto-approval cap. + const cause = isContextWindowExceededError + ? `context window retries exhausted (${MAX_CONTEXT_WINDOW_RETRIES}) — truncation did not make the request fit` + : `persistent API error after ${MAX_AUTO_APPROVAL_RETRIES} auto-approval retries` + throw new ApiRetryCapExceededError( + `[Task#attemptApiRequest] task ${this.taskId}.${this.instanceId} aborted — ${cause} ` + `(last: ${error.message ?? JSON.stringify(serializeError(error))}). Retry loop capped (roo-extensions#3195).`, ) } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 7298df67b8..f302c402c1 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -17,7 +17,7 @@ import { } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" -import { Task } from "../Task" +import { Task, ApiRetryCapExceededError } from "../Task" import { SYSTEM_PROMPT } from "../../prompts/system" import { createRateLimitClock } from "../RateLimitClock" import { summarizeConversation } from "../../condense" @@ -1009,7 +1009,7 @@ describe("Cline", () => { } // The stop is loud and names the last underlying error and the cap. - expect(thrown).toBeInstanceOf(Error) + expect(thrown).toBeInstanceOf(ApiRetryCapExceededError) expect((thrown as Error).message).toMatch(/capped.*roo-extensions#3195/) expect((thrown as Error).message).toContain("API Error") expect(attemptCount).toBe(4) @@ -1018,6 +1018,173 @@ describe("Cline", () => { expect(backoffSpy).toHaveBeenCalledTimes(3) }) + it("names context-window exhaustion instead of the auto-approval cap when truncation retries are spent", async () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + vi.spyOn(getTaskTestAccess(cline), "getSystemPrompt").mockResolvedValue("mock system prompt") + + const providerState = await mockProvider.getState() + vi.spyOn(mockProvider, "getState").mockResolvedValue({ + ...providerState, + apiConfiguration: mockApiConfig, + autoApprovalEnabled: true, + requestDelaySeconds: 3, + }) + + // A first-chunk error whose message matches the context-window patterns. + const mockError = Object.assign(new Error("This model has a maximum context window of 200000 tokens"), { + status: 400, + }) + 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 createMessageSpy = vi.spyOn(cline.api, "createMessage").mockImplementation(() => mockFailedStream) + + // Enter with the context-window budget already spent — the fall-through must + // blame the context window, not the auto-approval cap. + const iterator = cline.attemptApiRequest(3) + let thrown: unknown + try { + await iterator.next() + } catch (e) { + thrown = e + } + + expect(thrown).toBeInstanceOf(ApiRetryCapExceededError) + const message = (thrown as Error).message + expect(message).toMatch(/context window retries exhausted/) + expect(message).not.toMatch(/auto-approval retries/) + // Terminal on the first attempt — no backoff, no retry. + expect(createMessageSpy).toHaveBeenCalledTimes(1) + }) + + it("caps the mid-stream re-push loop in recursivelyMakeClineRequests (roo-extensions#3195)", async () => { + 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: true, + requestDelaySeconds: 1, + }) + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + + let attempts = 0 + const midStreamFailure = () => + (async function* () { + yield { type: "text", text: "partial" } + throw new Error("mid-stream API Error") + })() as AsyncGenerator + const success = () => + (async function* () { + yield { type: "text", text: "done" } + })() as AsyncGenerator + vi.spyOn(task.api, "createMessage").mockImplementation(() => { + attempts++ + // Safety valve: if the cap were removed, the loop would re-push forever — + // succeeding on the 5th attempt ends the loop and reddens the counts below. + return attempts >= 5 ? success() : midStreamFailure() + }) + + const backoffSpy = vi.spyOn(getTaskTestAccess(task), "backoffAndAnnounce").mockResolvedValue(undefined) + const saySpy = vi.spyOn(task, "say") + const abortTaskSpy = vi.spyOn(task, "abortTask").mockImplementation(async () => { + task.abort = true + }) + + const result = await task.recursivelyMakeClineRequests([ + { type: "text", text: "original user request" }, + ]) + + // 1 initial attempt + 3 mid-stream retries — the 4th failure is terminal, + // never re-queued, and never backed off again. + expect(attempts).toBe(4) + expect(backoffSpy).toHaveBeenCalledTimes(3) + expect(abortTaskSpy).toHaveBeenCalledTimes(1) + expect(task.abortReason).toBe("streaming_failed") + // The stop is loud — surfaced in the transcript, naming the cap and the cause. + const errorCall = saySpy.mock.calls.find((call) => call[0] === "error") + expect(errorCall?.[1]).toMatch(/mid-stream auto-approval retries/) + expect(errorCall?.[1]).toMatch(/capped.*roo-extensions#3195/) + expect(errorCall?.[1]).toContain("mid-stream API Error") + expect(result).toBe(true) + }) + + it("treats ApiRetryCapExceededError as terminal in the streaming_failed handler — no re-push", async () => { + 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: true, + requestDelaySeconds: 1, + }) + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + + const capError = new ApiRetryCapExceededError( + "[Task#attemptApiRequest] task aborted — persistent API error after 3 auto-approval retries " + + "(last: API Error). Retry loop capped (roo-extensions#3195).", + ) + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation( + () => + // eslint-disable-next-line require-yield + (async function* () { + throw capError + })() as AsyncGenerator, + ) + + const backoffSpy = vi.spyOn(getTaskTestAccess(task), "backoffAndAnnounce").mockResolvedValue(undefined) + const saySpy = vi.spyOn(task, "say") + const abortTaskSpy = vi.spyOn(task, "abortTask").mockImplementation(async () => { + task.abort = true + }) + + const result = await task.recursivelyMakeClineRequests([ + { type: "text", text: "original user request" }, + ]) + + // The capped error must end the loop on the spot — one attempt, zero backoffs, + // no re-push into a fresh API request. + expect(attemptSpy).toHaveBeenCalledTimes(1) + expect(backoffSpy).not.toHaveBeenCalled() + expect(abortTaskSpy).toHaveBeenCalledTimes(1) + const errorCall = saySpy.mock.calls.find((call) => call[0] === "error") + expect(errorCall?.[1]).toContain("Retry loop capped") + expect(result).toBe(true) + }) + it("uses the task rate limit in retry backoff when focused provider state differs", async () => { const clock = createRateLimitClock() const rateLimitConfig = { From 23124149a2bde5eaa1f9a0cbbae1bddaffc39b84 Mon Sep 17 00:00:00 2001 From: jsboige Date: Wed, 9 Sep 2026 01:05:13 +0200 Subject: [PATCH 3/3] test(task): harden #3195 retry-cap tests against mutation timeouts and survivors The mutation-diff gate surfaced 6 timeouts and 1 survivor on the changed Task.ts lines. Add fail-fast abort valves to the consumer tests so a cap-disabling mutation reddens the attempt count instead of hanging, keep a re-push assertion for the stateForBackoff-undefined path, and assert the auto-approval cause text so a mutant emptying it reddens. Stryker (scoped to the changed lines): 23/23 killed, 0 survived, 0 timeout. 111/111 vitest, tsc clean. Refs roo-extensions#3195 --- src/core/task/__tests__/Task.spec.ts | 150 ++++++++++++++++++++++++--- 1 file changed, 134 insertions(+), 16 deletions(-) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index a42d3b4329..818e400f6f 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1170,10 +1170,32 @@ describe("Cline", () => { }) let attemptCount = 0 + const mockSuccessStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "Success" } + }, + async next() { + return { done: true, value: { type: "text", text: "Success" } } + }, + async return() { + return { done: true, value: undefined } + }, + async throw(error: unknown) { + throw error + }, + async [Symbol.asyncDispose]() { + // Cleanup + }, + } as AsyncGenerator 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) + // Fail fast if the retry loop is unbounded — succeeding on a 5th attempt + // bounds the run if the cap is mutated away, so the count assertions + // below redden instead of the suite hanging. + if (attemptCount > 4) { + cline.abort = true + return mockSuccessStream + } return mockFailedStream }) @@ -1191,8 +1213,13 @@ describe("Cline", () => { // The stop is loud and names the last underlying error and the cap. expect(thrown).toBeInstanceOf(ApiRetryCapExceededError) + expect((thrown as Error).name).toBe("ApiRetryCapExceededError") + expect((thrown as Error).message).toMatch(/^\[Task#attemptApiRequest\] task [\w-]+\.[\w-]+ aborted/) expect((thrown as Error).message).toMatch(/capped.*roo-extensions#3195/) - expect((thrown as Error).message).toContain("API Error") + expect((thrown as Error).message).toContain("(last: API Error)") + // The cause names the auto-approval cap (not the context-window path) so a + // mutant emptying the cause string reddens here. + expect((thrown as Error).message).toContain("persistent API error after") expect(attemptCount).toBe(4) expect(createMessageSpy).toHaveBeenCalledTimes(4) // Exactly as many backoffs as retries — the request that finally threw never slept. @@ -1239,7 +1266,21 @@ describe("Cline", () => { }, } as AsyncGenerator - const createMessageSpy = vi.spyOn(cline.api, "createMessage").mockImplementation(() => mockFailedStream) + let attempts = 0 + const createMessageSpy = vi.spyOn(cline.api, "createMessage").mockImplementation(() => { + attempts++ + // Valve: if the cap is mutated away, succeed instead of recursing forever — + // the assertions below redden rather than the suite hanging. + if (attempts > 1) { + cline.abort = true + return { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "Success" } + }, + } as AsyncGenerator + } + return mockFailedStream + }) // Enter with the context-window budget already spent — the fall-through must // blame the context window, not the auto-approval cap. @@ -1282,15 +1323,16 @@ describe("Cline", () => { yield { type: "text", text: "partial" } throw new Error("mid-stream API Error") })() as AsyncGenerator - const success = () => - (async function* () { - yield { type: "text", text: "done" } - })() as AsyncGenerator vi.spyOn(task.api, "createMessage").mockImplementation(() => { attempts++ - // Safety valve: if the cap were removed, the loop would re-push forever — - // succeeding on the 5th attempt ends the loop and reddens the counts below. - return attempts >= 5 ? success() : midStreamFailure() + // Fail-fast bound: if the cap is mutated away the loop re-pushes forever. + // Aborting bounds the run so the attempt-count assertions below redden + // instead of the suite hanging (a success stream would route to normal + // completion and hang). + if (attempts > 6) { + task.abort = true + } + return midStreamFailure() }) const backoffSpy = vi.spyOn(getTaskTestAccess(task), "backoffAndAnnounce").mockResolvedValue(undefined) @@ -1311,9 +1353,12 @@ describe("Cline", () => { expect(task.abortReason).toBe("streaming_failed") // The stop is loud — surfaced in the transcript, naming the cap and the cause. const errorCall = saySpy.mock.calls.find((call) => call[0] === "error") + expect(errorCall?.[1]).toMatch( + /^\[Task#recursivelyMakeClineRequests\] task [\w-]+\.[\w-]+ aborted after/, + ) expect(errorCall?.[1]).toMatch(/mid-stream auto-approval retries/) expect(errorCall?.[1]).toMatch(/capped.*roo-extensions#3195/) - expect(errorCall?.[1]).toContain("mid-stream API Error") + expect(errorCall?.[1]).toContain("(last: mid-stream API Error)") expect(result).toBe(true) }) @@ -1338,13 +1383,22 @@ describe("Cline", () => { "[Task#attemptApiRequest] task aborted — persistent API error after 3 auto-approval retries " + "(last: API Error). Retry loop capped (roo-extensions#3195).", ) - const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation( - () => + let markerCalls = 0 + const attemptSpy = vi.spyOn(task, "attemptApiRequest").mockImplementation(() => { + markerCalls++ + // Valve: if the marker handling is mutated away, the generic path re-pushes + // forever with an always-throwing stream — aborting on the 2nd call bounds + // the run so the count assertion below reddens instead of hanging. + if (markerCalls > 1) { + task.abort = true + } + return ( // eslint-disable-next-line require-yield (async function* () { throw capError - })() as AsyncGenerator, - ) + })() as AsyncGenerator + ) + }) const backoffSpy = vi.spyOn(getTaskTestAccess(task), "backoffAndAnnounce").mockResolvedValue(undefined) const saySpy = vi.spyOn(task, "say") @@ -1366,6 +1420,70 @@ describe("Cline", () => { expect(result).toBe(true) }) + it("re-pushes a mid-stream failure without crashing when the provider reference is gone (stateForBackoff undefined)", async () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + // stateForBackoff resolves undefined (no getState payload) — a GC'd/cleared + // provider's deref() yields the same result, without breaking unrelated + // providerRef.deref() uses elsewhere in the streaming path. + vi.spyOn(mockProvider, "getState").mockResolvedValue( + undefined as unknown as Awaited>, + ) + vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined) + vi.spyOn(getTaskTestAccess(task), "getSystemPrompt").mockResolvedValue("mock system prompt") + + let attempts = 0 + const midStreamFailure = () => + (async function* () { + yield { type: "text", text: "partial" } + throw new Error("mid-stream API Error") + })() as AsyncGenerator + const terminalFailure = () => + (async function* () { + // Yield a chunk first so the throw lands mid-stream (routed to the + // consumer's streaming-failed catch, not attemptApiRequest's first-chunk + // `ask` path, which would hang unmocked). + yield { type: "text", text: "partial2" } + throw new ApiRetryCapExceededError("terminal after re-push") + })() as AsyncGenerator + vi.spyOn(task.api, "createMessage").mockImplementation(() => { + attempts++ + // Fail-fast bound: a terminal-disabling mutation (e.g. instanceof → false) + // makes the re-push run forever — aborting bounds the run so the attempt + // count below reddens instead of the suite timing out. + if (attempts > 4) { + task.abort = true + } + // First failure is generic (re-pushed); the retry raises the terminal cap + // error so the loop ends without needing a successful stream (which would + // require the full presentAssistantMessage/state path). + return attempts === 1 ? midStreamFailure() : terminalFailure() + }) + + const backoffSpy = vi.spyOn(getTaskTestAccess(task), "backoffAndAnnounce").mockResolvedValue(undefined) + vi.spyOn(task, "say") + vi.spyOn(task, "abortTask").mockImplementation(async () => { + task.abort = true + }) + + const result = await task.recursivelyMakeClineRequests([ + { type: "text", text: "original user request" }, + ]) + + // stateForBackoff is undefined: the auto-approval arm short-circuits safely and the + // failure is re-pushed without backoff. Mutating `stateForBackoff?.` to + // `stateForBackoff.` throws a TypeError inside the catch on the first failure + // (swallowed by the outer catch, so it still returns true) but NEVER re-pushes — + // which reddens the attempt count below. + expect(attempts).toBe(2) + expect(backoffSpy).not.toHaveBeenCalled() + expect(result).toBe(true) + }) + it("uses the task rate limit in retry backoff when focused provider state differs", async () => { const clock = createRateLimitClock() const rateLimitConfig = {