fix(errors): max_turns stays "Exceeded max turns" — observe the repeated failure, don't diagnose it

Reviewer concern on the ABCA-662 work: re-titling a max_turns failure as "Ran out
of turns retrying a failing step" over-claims. The stuck-guard's trailing window is
only the last ~6 tool calls, which genuinely CANNOT distinguish a hard blocker (bad
creds, no permission — more turns won't help) from a long task that made real
progress and hit a recoverable snag only at the tail (more turns / a retry WOULD
help — 662 itself: siblings pushed fine with the same token). Framing the whole run
around its last 6 calls misrepresents the latter.

- error-classifier: drop the separate "retrying a failing step" bucket. A max_turns
  failure stays the plain "Exceeded max turns"; the description/remedy point the
  reader at the observed detail and still surface the environment-blocker path,
  but assert NO cause.
- stuck_guard.recent_failure_summary: emit a NEUTRAL observation ("last tool calls
  repeated: `<cmd>` → <err>") instead of "spinning on failing tool calls". The
  mid-run steer TO the agent (an advisory nudge, not a user surface) still says
  "spinning" — that's fine, it's coaching the agent, not classifying the outcome.
- tests updated to pin the neutral wording + assert the title is NOT re-framed.

Author: isadeks

agent/src/pipeline.py

@@ -714,8 +714,8 @@ def run_task(
         from hooks import reset_blocker_reason, reset_stuck_summary
 
         reset_blocker_reason()
-        # ABCA-662: same per-task reset for the stuck-guard "why it spun" latch,
-        # so a prior task's spin summary can't leak into this task's max_turns copy.
+        # ABCA-662: same per-task reset for the stuck-guard recent-failure latch,
+        # so a prior task's observation can't leak into this task's max_turns copy.
         reset_stuck_summary()
         # --trace accumulator (design §10.1): when the task opted into
         # trace, ``_TrajectoryWriter`` keeps an in-memory copy of each

agent/src/stuck_guard.py

@@ -331,14 +331,20 @@ def _dominant_window_failure(self) -> tuple[str, str] | None:
         return None
 
     def recent_failure_summary(self) -> str | None:
-        """A one-line "why it spun" summary for a max_turns terminal reason.
+        """A one-line NEUTRAL observation of the recent repeated failure, for a
+        max_turns terminal reason.
 
         Returns None unless the trailing window is failure-dominated (the same
         bar the window-steer uses) — so a task that genuinely used its turns
         making progress yields no summary and its max_turns reason is unchanged.
-        Names the dominant recent failing command + a short slice of its output,
-        so the platform can say "hit max turns while stuck on: <cmd> → <err>"
-        instead of a bare "Exceeded max turns".
+        Names the dominant recent failing command + a short slice of its output.
+
+        Deliberately states only WHAT was observed, not WHY it capped: the window
+        is the last few tool calls, which can't tell a hard blocker from a long
+        task that hit a recoverable snag near the end. So the platform can say
+        "hit max turns; last tool calls repeated: <cmd> → <err>" and let the
+        reader judge — it must NOT assert the task was "spinning" or that more
+        turns wouldn't have helped.
         """
         dominant = self._dominant_window_failure()
         if dominant is None:
@@ -351,5 +357,5 @@ def recent_failure_summary(self) -> str | None:
             if failed and p == prev:
                 detail = re.sub(r"\s+", " ", fp).strip()[:120]
                 break
-        base = f"spinning on failing tool calls (last: `{prev}`"
-        return f"{base} — {detail})" if detail else f"{base})"
+        base = f"last tool calls repeated: `{prev}`"
+        return f"{base} — {detail}" if detail else base

agent/tests/test_stuck_guard.py

@@ -198,13 +198,15 @@ def test_recent_failure_summary_names_the_last_failure(self):
             g.record_tool_result("Bash", cmd, out)
         summary = g.recent_failure_summary()
         assert summary is not None
-        assert "spinning on failing tool calls" in summary
+        # Neutral observation only — names WHAT repeated, makes no causal claim.
+        assert "last tool calls repeated" in summary
+        assert "spinning" not in summary  # must not editorialize
         assert "git push --force-with-lease" in summary  # most recent failing command
         assert "invalid credentials" in summary  # the recurring error detail
 
     def test_no_summary_when_window_is_mostly_successful(self):
         # A productive agent (varied commands, mostly succeeding) must yield no
-        # spin summary — so its max_turns reason stays unchanged.
+        # summary — so its max_turns reason stays unchanged.
         g = StuckGuard()
         for i in range(6):
             g.record_tool_result("Bash", {"command": f"step {i}"}, OK)

cdk/src/handlers/shared/error-classifier.ts

@@ -306,35 +306,21 @@ const PATTERNS: readonly ErrorPattern[] = [
   // (live-caught on ABCA-483: a task hit the 100-turn cap but the reply
   // said "Unexpected error"). Match either ``agent_status=``/``subtype=``.
   {
-    // ABCA-662: max_turns hit while the last several tool calls were the SAME
-    // repeated failure (the agent's stuck-guard latched a failure-dominated
-    // trailing window; the pipeline appended "spinning on failing tool calls
-    // (last: …)"). This SURFACES what it was stuck on — but deliberately does NOT
-    // assert "more turns won't help", because the window alone can't distinguish
-    // (a) a HARD blocker no turn count fixes (bad creds, no permission) from
-    // (b) a LONG task that hit a transient/recoverable snag late and just ran out
-    // (662 itself: siblings 661/663 pushed fine with the same token → its
-    // 'invalid credentials' was a transient race that MORE turns / a retry would
-    // have cleared). So we name the failing operation and offer BOTH remedies;
-    // the failure KIND (in the detail) tells the reader which applies. Ordered
-    // before the generic error_max_turns bucket so this specific case wins.
-    pattern: /error_max_turns.*spinning on failing tool calls/i,
-    classification: {
-      category: ErrorCategory.TIMEOUT,
-      title: 'Ran out of turns retrying a failing step',
-      description: 'The agent hit its max-turns limit while the last several tool calls were the same repeated failure (see the detail below). That means one of two things — the failing step is a hard blocker no extra turns can fix, OR the task was long and hit a recoverable snag near the end and simply ran out of turns.',
-      remedy: 'Check the failing operation in the detail below. If it is an environment/tooling blocker (auth, credentials, permission, network, disk), retrying more won\'t help — fix the environment (an admin), then reply here to retry. If it looks transient or recoverable (a flaky/temporary error, or a sibling task with a shorter to-do got past the same thing), just reply to retry and/or raise --max-turns.',
-      retryable: true,
-      errorClass: ErrorClass.USER,
-    },
-  },
-  {
+    // A max-turns cap is a correct, self-explanatory classification. When the
+    // stuck-guard observed the last several tool calls repeating the SAME failure
+    // it is appended to the reason as a neutral OBSERVATION ("last tool calls
+    // repeated: `<cmd>` → <err>") — we surface WHAT was on screen but deliberately
+    // make NO causal claim about whether more turns would have helped: the
+    // trailing window (last 6 calls) can't distinguish a hard blocker from a long
+    // task that hit a recoverable snag only at the tail, so re-framing the whole
+    // run as "retrying a failing step" would misrepresent the latter. The reader
+    // sees the observed detail and the neutral remedy and decides.
     pattern: /(?:agent_status|subtype)=['"]?error_max_turns['"]?/i,
     classification: {
       category: ErrorCategory.TIMEOUT,
       title: 'Exceeded max turns',
-      description: 'The agent reached the configured ``max_turns`` limit before completing.',
-      remedy: 'Raise ``--max-turns`` on the submit call, simplify the task, or break it into smaller sub-tasks.',
+      description: 'The agent reached the configured ``max_turns`` limit before completing. If a repeated tool failure was observed near the end, it is shown in the detail below.',
+      remedy: 'Look at the detail below to see what the agent was doing when it ran out. Raise ``--max-turns`` on the submit call, simplify the task, or break it into smaller sub-tasks — and if the detail shows an environment/tooling blocker (auth, credentials, permission, network, disk), fix that first, then reply here to retry.',
       retryable: true,
       errorClass: ErrorClass.USER,
     },

cdk/test/handlers/shared/error-classifier.test.ts

@@ -258,24 +258,26 @@ describe('classifyError', () => {
       expect(result!.remedy).toMatch(/--max-turns/);
     });
 
-    test('ABCA-662: max_turns hit while retrying a failing step surfaces WHAT it was stuck on, and offers BOTH remedies (does not over-claim)', () => {
+    test('ABCA-662: max_turns with an observed repeated failure stays "Exceeded max turns" and makes NO causal claim', () => {
       // When the agent capped out with the last several calls being the same
-      // repeated failure (pipeline appended "spinning on failing tool calls"), we
-      // name the failing op — but must NOT assert "more turns won't help", because
-      // the window can't tell a hard blocker from a long task that hit a
-      // recoverable snag late (662: siblings pushed fine → transient). So the
-      // remedy presents BOTH paths. Ordered before the generic max_turns bucket.
+      // repeated failure, the pipeline appends a NEUTRAL observation ("last tool
+      // calls repeated: …"). The classification must NOT re-title the failure as
+      // "retrying a failing step" or assert more turns wouldn't help — the window
+      // (last few calls) can't tell a hard blocker from a long task that hit a
+      // recoverable snag late (662: siblings pushed fine → transient). It stays the
+      // plain max_turns bucket; the observed detail rides along in the message.
       const result = classifyError(
-        "Agent session error (subtype='error_max_turns') — spinning on failing tool calls "
-        + '(last: `git push --force-with-lease` — remote: invalid credentials fatal: exit 128)',
+        "Agent session error (subtype='error_max_turns') — last tool calls repeated: "
+        + '`git push --force-with-lease` — remote: invalid credentials fatal: exit 128',
       );
       expect(result!.category).toBe(ErrorCategory.TIMEOUT);
-      expect(result!.title).toBe('Ran out of turns retrying a failing step');
+      expect(result!.title).toBe('Exceeded max turns');
       expect(result!.retryable).toBe(true);
-      // Surfaces the environment-blocker path AND the transient/recoverable path —
-      // it does NOT flatly claim raising turns is useless.
-      expect(result!.remedy).toMatch(/environment/i);
-      expect(result!.remedy).toMatch(/transient|recoverable|raise --max-turns/i);
+      // Does not editorialize: no "spinning" / "won't help" claim. It points the
+      // reader at the detail and still offers the environment-blocker path.
+      expect(result!.title).not.toMatch(/retrying a failing step/i);
+      expect(result!.remedy).toMatch(/detail/i);
+      expect(result!.remedy).toMatch(/environment|auth|credentials/i);
     });
 
     test('classifies error_max_budget_usd as TIMEOUT with specific title', () => {