fix(loop): max-mode hands the drive the failure that killed every candidate (#425)

changelog.d/429.md

@@ -0,0 +1,12 @@
+Max-mode now tells the loop why its candidates failed, instead of reporting "no diff". It used to
+swallow each candidate's error, so when every candidate failed the loop saw a capability failure
+and climbed a tier. That happened on a spent quota or a model the provider refuses, while a working
+sibling at the same rung sat idle. It also hid timeouts from the decompose ask, and seam failures
+from the pre-model block. When every candidate raises, the loop now takes the edge of the most
+specific failure among them, as it would for a single dispatch. In order, that is a refused model
+(even beside a quota), a quota, a timeout, a dispatch failure the loop does not retry, a retryable
+one, then any other board error. A raw non-board error keeps the old verdict, so shutdown and
+cancel still win. Only a fan-out where at least one candidate ran and came back empty is certain
+to climb as a capability failure. A climb after a timeout now fans out again at the new rung. Its
+"previous attempt timed out" note describes the candidate that actually timed out, and no longer
+follows the card into later drives.

coder_seam.py

@@ -495,6 +495,16 @@ def progress_stop_reason(fid: str | None, gen: int, reason) -> None:
         b.stop_reason = str(reason)[:200]
 
 
+# The stop reason the dispatch stamps on a gen its watchdog killed. The loop's timeout
+# note (#146) is mined from THAT gen — in a max-mode fan-out the last gen may be a
+# sibling that failed fast on something else (#425 review).
+TIMED_OUT_REASON = "timed out"
+
+
+def _stamp_timeout(fid: str | None, gen: int, timeout) -> None:
+    progress_stop_reason(fid, gen, f"{TIMED_OUT_REASON} after {timeout}s")
+
+
 # ── Persist finished gens to the bead (#226) ────────────────────────────────────
 # The WRITE side of the coder-monitor history (#226): when a gen finishes, its live
 # snapshot is serialized to a `coder-monitor: {…}` JSON bead comment so the drawer
@@ -751,6 +761,7 @@ async def _tool_cb(event):
         # client + SIGKILLed the tree on its way out — nothing for the board to clean up.
         raise
     except asyncio.TimeoutError:
+        _stamp_timeout(fid, gen, timeout)
         raise worktree.CoderTimeout(f"coder timed out after {timeout}s")
     except Exception as exc:  # noqa: BLE001 — normalise every below-seam failure to the
         # adapter path's contract: nothing propagates raw; a dispatch failure surfaces as
@@ -843,6 +854,9 @@ async def _dispatch_coder_tapped_legacy(
             return await worktree.dispatch_coder(
                 coder, worktree_path, prompt, timeout=timeout, env_passthrough=env_passthrough
             )
+        except worktree.CoderTimeout:
+            _stamp_timeout(fid, gen, timeout)
+            raise
         finally:
             progress_end(fid, gen)  # the gen must close on EVERY exit path (panel: orphaned gens)
 
@@ -881,6 +895,7 @@ async def _tool_cb(event):
     # client requires a number (it logs int(timeout)), so "unbounded" rides a 24h
     # sentinel instead of the 600s floor the first tap draft imposed (panel round 2).
     prompt_timeout = timeout or getattr(scoped, "timeout_s", None) or 86400.0
+    timed_out = False
     try:
         coro = client.prompt(
             prompt,
@@ -902,6 +917,7 @@ async def _tool_cb(event):
             pass
         raise
     except asyncio.TimeoutError:
+        timed_out = True
         raise worktree.CoderTimeout(f"coder timed out after {timeout}s")
     except (AcpError, DelegateError) as exc:
         raise worktree.WorktreeError(f"coder dispatch failed: {exc}")
@@ -913,9 +929,12 @@ async def _tool_cb(event):
         # Stash whatever stop-reason / dead-end signal the ACP client reports (#198)
         # — sampled on EVERY exit so an empty reply still records WHY the coder
         # stopped. Best-effort getattr: a host without the attribute yields None.
-        progress_stop_reason(
-            fid, gen, getattr(client, "last_stop_reason", None) or getattr(client, "last_dead_end", None)
-        )
+        if timed_out:
+            _stamp_timeout(fid, gen, timeout)  # the pooled client's last reason is not THIS turn's
+        else:
+            progress_stop_reason(
+                fid, gen, getattr(client, "last_stop_reason", None) or getattr(client, "last_dead_end", None)
+            )
         progress_end(fid, gen)
         try:
             await adapter.teardown(scoped)  # #1 lifecycle rule: reap the worktree-scoped subprocess

docs/configuration.md

@@ -238,8 +238,22 @@ the quota backoff is spent — a marked provider still gets its one real attempt
 card blocks (the operator may have repointed it), and a dispatch it serves clears the mark. If every provider on the rung refused its model on this
 card, the card blocks under `dispatch-infra` naming them, and does not climb: that is a
 config problem, and a stronger rung would only hide it. If some were only rate-limited, it
-blocks as `rate_limit`, which the sweep heals on its own. Max-mode swallows each candidate's
-error, so neither rotation applies there.
+blocks as `rate_limit`, which the sweep heals on its own.
+
+Max-mode (`max_mode_n > 1`) follows the same rules. If every candidate raised, the loop takes
+the edge of the most specific failure among them, exactly as it would for a single dispatch.
+The precedence is:
+
+1. a refused model, even beside a quota;
+2. a quota;
+3. a timeout;
+4. a dispatch failure the loop does not retry;
+5. a retryable one;
+6. any other board error, as raised.
+
+A raw error that is not a board error keeps the old "no diff" verdict. Only a fan-out where
+at least one candidate ran and came back with nothing is guaranteed to be a capability failure
+that climbs. A climb after a timeout is a fresh build, so the stronger rung fans out again.
 
 A card's STARTING rung comes from its difficulty, so a `medium` card never touches rung 1.
 

loop/_common.py

@@ -835,6 +835,56 @@ def _next_rung_cursor() -> int:
 _ROTATABLE_CATEGORIES = frozenset({"rate_limit", "provider_unavailable"})
 
 
+def provider_failure_category(exc: BaseException) -> str | None:
+    """The provider class (``rate_limit`` / ``provider_unavailable``) of a coder DISPATCH
+    failure, or ``None`` when ``exc`` is anything else — the one definition of "the
+    provider failed, not the model". The drive rotates on it; max-mode asks it of every
+    candidate before it decides what the drive is told (#425)."""
+    text = str(exc)
+    if not text.startswith("coder dispatch failed"):
+        return None
+    category = classify(text).category
+    return category if category in _ROTATABLE_CATEGORIES else None
+
+
+def representative_failure(errors: list[Exception]) -> Exception | None:
+    """The ONE error that speaks for a max-mode fan-out in which EVERY candidate raised
+    (#425). No candidate returned anything to judge, so the drive is handed this instead
+    of a "no diff" and its own handling applies, as for a single dispatch. The most
+    specific edge wins:
+
+    0. a provider that refused its model — beside a quota too: the provider is at least
+       partly broken, and a capability climb is wrong either way;
+    1. a spent quota — rotate within the rung, or back off;
+    2. a timeout — so #378's timeout counter, and its decompose ask, see it;
+    3. a dispatch failure the drive does not retry — #339 blocks it as pre-model unless a
+       candidate reached the model;
+    4. any other dispatch failure (a retryable one: back off and re-run);
+    5. any other ``WorktreeError``, as it was raised.
+
+    Ties go to the earliest candidate. Only a ``WorktreeError`` can speak: anything else
+    (a raw ``BrokenPipeError`` from an untapped host, say) returns None, and the caller
+    keeps its old "no diff" verdict — which the drive's shutdown and cancel checks see
+    first, where a raw error would skip them and block the card as `unexpected`. Pure,
+    like ``rotation_target``, so the order is testable without driving a card."""
+
+    def rank(exc: Exception) -> int:
+        category = provider_failure_category(exc)
+        if category == "provider_unavailable":
+            return 0
+        if category == "rate_limit":
+            return 1
+        if isinstance(exc, worktree.CoderTimeout):
+            return 2
+        text = str(exc)
+        if text.startswith("coder dispatch failed"):
+            return 4 if classify(text).retryable else 3
+        return 5
+
+    speakers = [e for e in errors if isinstance(e, worktree.WorktreeError)]
+    return min(speakers, key=rank) if speakers else None
+
+
 # ── #420: remember a provider that can't serve its model ────────────────────────────
 # Rotation alone rediscovers a dead provider card by card: the rung cursor spreads cards
 # across a rung's siblings, so every card that happens to open on it pays one failed
@@ -1263,6 +1313,8 @@ def _inbox_db_path():
     "rotation_target",
     "prefer_live_sibling",
     "_ROTATABLE_CATEGORIES",
+    "provider_failure_category",
+    "representative_failure",
     "_next_rung_cursor",
     "_PROVIDER_DOWN_TTL_S",
     "_PROVIDER_DOWN",

loop/drive.py

@@ -1116,14 +1116,20 @@ async def _drive(self, feature: dict):
         wt = branch = None
         pr_url = None  # set once open_pr returns — the cancel paths below close it (#211)
         keep_wt = False  # reuse the worktree on a goal-fix retry (keep the impl; add tests)
+        # "A prior attempt timed out" (#146) for the NEXT dispatch after a timeout climb.
+        # Drive-local on purpose (#425 review): it rode `_ci_feedback`, which persists
+        # across drives and marks a carried-forward FIX — so one all-timeout fan-out
+        # switched max-mode off for that card for good, and every later drive still
+        # opened with "a PREVIOUS attempt TIMED OUT". A timeout climb is a fresh build.
+        timeout_note = ""
         try:
             while True:
                 # Rebuild the prompt each attempt so a re-dispatch (CI bounce,
                 # goal-verify gap, or tier escalation) picks up the latest
                 # _ci_feedback + _ci_prior_diff. Fetch this area's distilled lessons
                 # from the KG (best-effort, async) and inject them — the flywheel READ.
                 lessons = await self._fetch_kg_lessons(feature)
-                prompt = self._build_prompt(feature, lessons=lessons)
+                prompt = self._build_prompt(feature, lessons=lessons, timeout_note=timeout_note)
                 # Which PROVIDER at this rung. `siblings` are interchangeable delegates for
                 # the same capability tier (#362); `sib` advances only on a provider failure
                 # (below) or round-robin across dispatches, never on a capability failure —
@@ -1278,6 +1284,7 @@ async def _drive(self, feature: dict):
                     # dispatch scheduled on worker threads land before the drive
                     # proceeds toward open_pr (the pre-offload ordering).
                     await self._await_bg_records(fid)
+                    timeout_note = ""  # the dispatch that carried it has run — consumed
                     # The provider served this dispatch, so a down mark on it (#420) — set
                     # by another card, or one that was stale when every sibling was marked
                     # and this one got the rung's last attempt — no longer holds.
@@ -1567,7 +1574,7 @@ async def _drive(self, feature: dict):
                     dispatch_error = str(exc).startswith("coder dispatch failed")
                     if policy.category == "provider_unavailable" and not dispatch_error:
                         policy = classify(str(exc), provider_rules=False)
-                    provider_failure = dispatch_error and policy.category in _ROTATABLE_CATEGORIES
+                    provider_failure = provider_failure_category(exc) is not None
                     # A dispatch that failed on a fix round's KEPT worktree left it untouched:
                     # its files hold the implementation and the feedback says so. Whatever
                     # re-dispatches next — a sibling, or the same provider after a backoff —
@@ -1641,6 +1648,7 @@ async def _drive(self, feature: dict):
                                     )
                                     tier = nxt
                                     retries = 0
+                                    timeout_note = ""  # a new rung: an older timeout note is stale
                                     spent_here.clear()  # a NEW rung has its own providers (#362)
                                     refused_here.clear()
                                     # Fresh per-tier budgets on the climb — mirrors the
@@ -1835,15 +1843,18 @@ async def _drive(self, feature: dict):
                             # about a worktree that no longer exists, so clear it (+ the prior diff)
                             # to keep prompt and worktree consistent.
                             keep_wt_class = str(exc).startswith(("goal verification failed", "requirements unresolved"))
+                            timeout_note = ""  # a new rung: an older timeout note is stale
                             if keep_wt_class and wt is not None:
                                 keep_wt = True  # reuse the verified worktree; _ci_feedback is truthful
                             elif isinstance(exc, worktree.CoderTimeout):
                                 # A timeout carries NO diff and NO CI output, so the stronger tier
                                 # would otherwise get a BYTE-IDENTICAL prompt — blind to the fact a
                                 # prior attempt ran out of time and what it was doing when killed
-                                # (#146). Seed the CI/review-bounce feedback lever with the ring
-                                # buffer's timeout context so the escalated dispatch leads with it.
-                                self._ci_feedback[fid] = self._timeout_escalation_context(fid)
+                                # (#146). Lead the escalated dispatch with the ring buffer's timeout
+                                # context — as a drive-local note, NOT `_ci_feedback`: this is a fresh
+                                # build, so a max-mode/solve board fans out again at the new rung.
+                                timeout_note = self._timeout_escalation_context(fid)
+                                self._ci_feedback.pop(fid, None)  # fresh worktree ahead: no fix to carry
                                 self._ci_prior_diff.pop(fid, None)  # a timeout produced no diff to echo back
                             else:
                                 # Fresh worktree ahead — drop any gate-fix feedback describing the
@@ -2241,7 +2252,10 @@ async def _dispatch_max_mode(
         pass; ADR 0064), else the best-of-N LLM judge; the winner is PROMOTED into the canonical
         ``feat-<id>`` worktree / ``feat/<id>`` branch (so the rest of the lifecycle is
         unchanged) and the losers are reaped. All-empty → ``NoChangesError``, which
-        ``_drive`` escalates/blocks exactly like a single coder that produced nothing.
+        ``_drive`` escalates/blocks exactly like a single coder that produced nothing —
+        unless EVERY candidate raised, when the one error that speaks for them
+        (``representative_failure``) is re-raised instead, so the drive handles it as it
+        would a single dispatch's (#425).
 
         Returns (canonical_wt, canonical_branch, winner_reply). The fan-out is bounded by
         ``max_concurrent`` × ``max_mode_n`` coders; size those to the host."""
@@ -2271,6 +2285,26 @@ async def _dispatch_max_mode(
         if idx is None:
             for cid in cand_ids:
                 await worktree.reap_feature_worktree(repo, self.root, cid)
+            # #425: "no diff" is a CAPABILITY verdict, and the drive climbs a rung on it. That is
+            # only true when a candidate RETURNED — ran, and came back with nothing. When EVERY
+            # candidate raised there is nothing to judge, and swallowing the errors hid what
+            # actually happened: a quota (#362) or a refused model (#420) climbed instead of
+            # rotating, a timeout never reached #378's counter, a pre-model seam failure never
+            # reached #339's block. So hand the drive ONE of those errors — the most specific
+            # edge, `representative_failure` — and its own handling applies exactly as for a
+            # single dispatch. A CancelledError child neither returned nor raised, and a raw
+            # (non-WorktreeError) error can't speak for the fan-out: both keep the old verdict.
+            raised = [r for r in results if isinstance(r, Exception)]
+            rep = representative_failure(raised) if len(raised) == len(results) else None
+            if rep is not None:
+                log.info(
+                    "[project_board] %s max-mode: all %d candidates raised — handing the drive the "
+                    "failure that speaks for them, not a no-diff: %s",
+                    fid,
+                    n,
+                    str(rep)[:160],
+                )
+                raise rep
             raise worktree.NoChangesError(f"max-mode: all {n} candidates produced no diff")
         log.info("[project_board] %s max-mode: candidate %d/%d wins → promoting", fid, idx, n)
         win_wt, win_branch = cands[idx]
@@ -2361,16 +2395,22 @@ def _timeout_escalation_context(self, fid: str) -> str:
         attempt ran out of time, how long it ran, or what it was doing when killed.
         Mine the progress ring buffer (``coder_seam.progress_snapshot``) for the
         timed-out gen's elapsed time, the last tool in flight, and the thought tail,
-        and lead the re-dispatch with them. Returned for injection into
-        ``_ci_feedback`` so it rides the exact same prompt path a CI/review bounce
-        uses — no new plumbing. Best-effort: a missing/empty snapshot still yields a
-        usable "prior attempt timed out, produced no diff" note — a monitor read must
-        never break escalation."""
+        and lead the re-dispatch with them. Returned as the drive's timeout note, which
+        rides the same rejected-attempt block a CI/review bounce uses. Best-effort: a
+        missing/empty snapshot still yields a usable "prior attempt timed out, produced
+        no diff" note — a monitor read must never break escalation.
+
+        Mined from the gen that TIMED OUT — the seam stamps its stop reason — not simply
+        the last one: in a max-mode fan-out that is just the last candidate, which may
+        have failed fast on something else, and the note then described the wrong
+        attempt ("ran ~0.0s", no tool; #425 review). The last gen is only the fallback
+        for a buffer that carries no stamp (an older gen, an untapped host)."""
         try:
             gens = coder_seam.progress_snapshot(fid).get("gens") or []
         except Exception:  # noqa: BLE001 — a monitor read must never break escalation
             gens = []
-        gen = gens[-1] if gens else {}
+        timed_out = [g for g in gens if str(g.get("stop_reason") or "").startswith(coder_seam.TIMED_OUT_REASON)]
+        gen = (timed_out or gens or [{}])[-1]
         elapsed = gen.get("elapsed_s")
         ran_for = f"ran ~{elapsed}s and " if elapsed is not None else ""
         lines = [