fix(session): bound the retry loop, fix the doom-loop guard, treat abort as a clean stop (#124)

- Cap transient-error retries at MAX_RETRY_ATTEMPTS (10) per message
  generation. The while(true) retry loop was unbounded, and retry.ts marks
  any JSON body with an 'error' field as retryable, so a persistently-failing
  provider (or a permanent error arriving as JSON) retried forever. (#7)
- Doom-loop detection now scans the last N *tool* parts (isDoomLoop helper)
  instead of the last N raw parts. Reasoning models emit a reasoning part
  before each tool call, so the last 3 raw parts were never 3 identical tool
  parts and the guard never fired against a genuinely looping tool. (#10)
- A user-initiated abort no longer fires the session Error event: it's a clean
  cancellation, so record it on the message (turn still stops) but don't
  surface it as an error state in the UI. (#51)

Author: aayambansal

backend/cli/src/session/processor.ts

@@ -20,8 +20,32 @@ import { requiresWalletBalance, shouldReportUsage, resolveCredentialSource, llmB
 
 export namespace SessionProcessor {
   const DOOM_LOOP_THRESHOLD = 3
+  // Hard ceiling on transient-error retries within a single message generation.
+  // The retry loop is otherwise unbounded, and retry.ts classifies any JSON
+  // body carrying an `error` field as retryable — so a persistently-failing
+  // provider (or a permanent error arriving as JSON) looped forever.
+  const MAX_RETRY_ATTEMPTS = 10
   const log = Log.create({ service: "session.processor" })
 
+  /** True when the last `threshold` TOOL calls are the same tool with the same
+   *  input, ignoring reasoning/text/step parts interleaved between them. A naive
+   *  "last N raw parts" check was defeated by reasoning models, which emit a
+   *  reasoning part before each tool call, so the doom-loop guard never fired. */
+  export function isDoomLoop(
+    parts: MessageV2.Part[],
+    toolName: string,
+    input: unknown,
+    threshold = DOOM_LOOP_THRESHOLD,
+  ): boolean {
+    const tools = parts.filter((p): p is MessageV2.ToolPart => p.type === "tool")
+    const last = tools.slice(-threshold)
+    if (last.length < threshold) return false
+    return last.every(
+      (p) =>
+        p.tool === toolName && p.state.status !== "pending" && JSON.stringify(p.state.input) === JSON.stringify(input),
+    )
+  }
+
   export type Info = Awaited<ReturnType<typeof create>>
   export type Result = Awaited<ReturnType<Info["process"]>>
 
@@ -182,18 +206,8 @@ export namespace SessionProcessor {
                     toolcalls[value.toolCallId] = part as MessageV2.ToolPart
 
                     const parts = await MessageV2.parts(input.assistantMessage.id)
-                    const lastThree = parts.slice(-DOOM_LOOP_THRESHOLD)
 
-                    if (
-                      lastThree.length === DOOM_LOOP_THRESHOLD &&
-                      lastThree.every(
-                        (p) =>
-                          p.type === "tool" &&
-                          p.tool === value.toolName &&
-                          p.state.status !== "pending" &&
-                          JSON.stringify(p.state.input) === JSON.stringify(value.input),
-                      )
-                    ) {
+                    if (isDoomLoop(parts, value.toolName, value.input)) {
                       const agent = await Agent.get(input.assistantMessage.agent)
                       await PermissionNext.ask({
                         permission: "doom_loop",
@@ -419,7 +433,7 @@ export namespace SessionProcessor {
             })
             const error = MessageV2.fromError(e, { providerID: input.model.providerID })
             const retry = SessionRetry.retryable(error)
-            if (retry !== undefined) {
+            if (retry !== undefined && attempt < MAX_RETRY_ATTEMPTS) {
               attempt++
               const delay = SessionRetry.delay(attempt, error.name === "APIError" ? error : undefined)
               SessionStatus.set(input.sessionID, {
@@ -432,10 +446,15 @@ export namespace SessionProcessor {
               continue
             }
             input.assistantMessage.error = error
-            Bus.publish(Session.Event.Error, {
-              sessionID: input.assistantMessage.sessionID,
-              error: input.assistantMessage.error,
-            })
+            // A user-initiated abort is a clean cancellation, not a failure —
+            // record it on the message (so the turn stops) but don't fire the
+            // session Error event the UI renders as an error state.
+            if (!MessageV2.AbortedError.isInstance(error)) {
+              Bus.publish(Session.Event.Error, {
+                sessionID: input.assistantMessage.sessionID,
+                error: input.assistantMessage.error,
+              })
+            }
           }
           if (snapshot) {
             const patch = await Snapshot.patch(snapshot)

backend/cli/test/session/doom-loop.test.ts

@@ -0,0 +1,48 @@
+import { describe, expect, test } from "bun:test"
+import { SessionProcessor } from "../../src/session/processor"
+
+const tool = (name: string, input: unknown, status = "completed"): any => ({
+  type: "tool",
+  tool: name,
+  callID: "c",
+  state: { status, input },
+})
+const reasoning = (): any => ({ type: "reasoning", text: "thinking..." })
+const text = (): any => ({ type: "text", text: "hi" })
+
+describe("SessionProcessor.isDoomLoop", () => {
+  test("fires when the last 3 TOOL calls are identical, even with reasoning/text between them", () => {
+    // A reasoning model interleaves a reasoning part before every tool call —
+    // the old last-3-raw-parts check never saw 3 consecutive tool parts.
+    const parts = [
+      reasoning(),
+      tool("bash", { cmd: "ls" }),
+      reasoning(),
+      tool("bash", { cmd: "ls" }),
+      text(),
+      reasoning(),
+      tool("bash", { cmd: "ls" }),
+    ]
+    expect(SessionProcessor.isDoomLoop(parts, "bash", { cmd: "ls" })).toBe(true)
+  })
+
+  test("does not fire when the inputs differ", () => {
+    const parts = [tool("bash", { cmd: "a" }), tool("bash", { cmd: "b" }), tool("bash", { cmd: "c" })]
+    expect(SessionProcessor.isDoomLoop(parts, "bash", { cmd: "c" })).toBe(false)
+  })
+
+  test("does not fire below the threshold of 3 tool calls", () => {
+    const parts = [reasoning(), tool("bash", { cmd: "ls" }), reasoning(), tool("bash", { cmd: "ls" })]
+    expect(SessionProcessor.isDoomLoop(parts, "bash", { cmd: "ls" })).toBe(false)
+  })
+
+  test("does not fire when a different tool breaks the streak", () => {
+    const parts = [tool("bash", { cmd: "ls" }), tool("read", { path: "x" }), tool("bash", { cmd: "ls" })]
+    expect(SessionProcessor.isDoomLoop(parts, "bash", { cmd: "ls" })).toBe(false)
+  })
+
+  test("ignores a pending tool call (not yet a confirmed repeat)", () => {
+    const parts = [tool("bash", { cmd: "ls" }), tool("bash", { cmd: "ls" }), tool("bash", { cmd: "ls" }, "pending")]
+    expect(SessionProcessor.isDoomLoop(parts, "bash", { cmd: "ls" })).toBe(false)
+  })
+})