Skip to content

fix(loop): a timed-out card is parked for its split, and the ask is dispatched (#378) - #435

Merged
mabry1985 merged 4 commits into
mainfrom
fix/timeout-decompose-378
Sep 10, 2026
Merged

mabry1985 merged 4 commits into
mainfrom
fix/timeout-decompose-378

Conversation

@mabry1985

@mabry1985 mabry1985 commented Sep 10, 2026

Copy link
Copy Markdown
Member

Fixes #378. #379 and #380 did most of it; this closes the three gaps they left, plus five defects an adversarial review found in this PR's first cut.

What was still wrong on main

#379 capped large in the breadth gate. #380 made a repeated CoderTimeout ask the board's own agent to split the card. I reproduced each of the following on origin/main:

  1. The ask was never picked up. request_decomposition files its task through create_feature, which puts every bead in backlog. The puller's queue is br ready --label ready, so the A task assigned to the agent itself should be dispatched to it, not parked forever #311 self-dispatch path never saw it. On real br 0.2.16 the task sat backlog and was absent from ready_queue().
  2. The ask waited for the ladder to run out. On smart → reasoning → opus a card timed out three times (dispatched a, b, c, asked timeouts=3) before anything asked for a split.
  3. The card was then rebuilt anyway. The block beside the ask was the self-healing transient. The sweep requeued the card it had just asked to split and rebuilt it whole, up to 2 more full timeouts.

Defects the review found in the first cut (each reproduced, R1–R5)

  • R1: an infra (pre-first-token) timeout was counted. After the operator fixed the infra, the card's first genuine timeout parked it. (The fix(loop): adapter chatter is not evidence the model ran (#422) #428 review independently confirmed this one.)
  • R2: a fix-round timeout on a card that had already built was parked. The split's cancel would have closed its open PR, and the count never reset after a successful build.
  • R5: the re-park reason lied, and unblocking gave no real retry. A re-park that filed nothing still said "parked for a split", and after an unblock the count was still at the threshold.
  • R3/R4: the task's steps were unexecutable. Slices marked ready while the parked card still claimed their files were refused by the shared-file gate (R3). Cancelling the card released its dependents before any slice had landed (R4).
  • Plausible, also fixed: a lost decompose-asked label write could leave an orphan task and file a duplicate at the next park.

The fix

What counts (drive, new step 1.7, after the pre-model block and before the climb):

The park, in the one safe order (_park_for_split):

  1. File the decompose task. request_decomposition leaves it in backlog, invisible to the puller.
  2. Park the card under its own class too-wide. The sweep never re-runs it and the operator is told once. The reason is built from what step 1 returned: it names the task, or says plainly that none was filed and what to do.
  3. Release the task to ready. The agent that picks it up ends by cancelling this card, so the card must already be parked.

Operator reset:

  • clear_blocked on a too-wide park also drops the budget:timeout label.
  • The unblock tool and the /unblock route drop the running loop's cached count (loop.forget_timeout_count), since that cache wins over the label (F5: every fix budget lives in memory, so a restart refunds them all #259).
  • So a retry after raising coder_timeout_s is a real attempt. Other blocks keep their count, so the sweep's self-heal can't reset it.

The split's own steps. The spec lists the card's dependents and orders the work so each step passes the gates the next one relies on:

  1. Create the slices and leave them in backlog.
  2. Re-point every dependent onto the slice(s) it needs.
  3. Cancel the card.
  4. Mark the slices ready.

Idempotent on the task. A repeat ask finds the task already filed for the card (by its title), heals a lost label, and returns the open task so the re-park can name and release it. It returns None once that split has closed.

A pre-first-token timeout still blocks as dispatch-infra (#339). 0 still switches all of this off. The docs and manifest are updated, and "never for a pre-first-token timeout" now holds for the count as well as the ask.

Tests (tests/test_timeout_decompose_378.py, plus two adjusted existing tests)

  • drive: 3-rung ladder → a, b, one climb, then park too-wide naming the task. The order is ask < park < release.
  • drive → sweep: a parked card is not requeued, and the operator is told once with the task id.
  • drive: a re-park that files nothing says NO split task was filed, what to do, and releases nothing.
  • drive: an infra timeout is never counted; the next genuine one counts 1 and is not parked.
  • drive: a fix round on a card with an open PR is never parked and never counted, and climbs as usual.
  • drive: a keep-worktree fix round that times out is not counted.
  • drive: a build that reaches review clears the count.
  • drive: after an operator unblock (store reset + forget_timeout_count), the retry's first timeout is not re-parked.
  • store: clear_blocked resets the count only for a too-wide park. The ask files in backlog (never releases itself), lists dependents, and gives the steps in the safe order. It is idempotent on the task: it heals a lost label, returns an open task, and returns None once that split has closed.
  • real br:
    • the drive parks a real card naming its split; the split is ready, in the queue, and self-dispatched to the board's own agent with its assignee intact (R6's check, folded in);
    • the split's steps executed in order pass every gate, the dependent is not released early, and it is released once the slices land. The wrong order is shown to be refused by the shared-file gate.
    • a lost label write cannot file a duplicate;
    • an unblock resets a real parked card's count.

Two existing tests change:

Red-checks: all 15 new tests fail against this PR's previous head. The reviewer's reproducers pass on that head and, on this one, R1, R2 and R5 now fail as intended. R3 and R6 fail by premise, because the store's ask now comes out in backlog and the drive releases it; the real-br tests above cover both properties end to end. R4 pins a raw br fact (a cancel releases dependents), which the new step order handles.

Mutation checks (13, all caught):

  • counting infra timeouts;
  • parking a fix round with an open PR;
  • parking a keep-worktree round;
  • no count reset at review;
  • a reason that always claims a split;
  • releasing before parking;
  • an unblock that keeps the count;
  • a no-op forget_timeout_count;
  • an ask that promotes itself;
  • label-only idempotency;
  • a spec that cancels before re-pointing;
  • plus the earlier threshold and park-class mutations.

Gates: ruff format --check, ruff check, full suite 1918 passed / 13 skipped.

Intended behaviour (not gaps)

🤖 Generated with Claude Code

https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

mabry1985 and others added 2 commits September 10, 2026 15:07
…ispatched (#378)

#379 capped `large` in the breadth gate and #380 made a repeated timeout
ask the board's own agent to split the card. Three gaps were left, each
reproduced on origin/main.

1. The ask was never picked up. request_decomposition filed its task in
   backlog, like every new bead, and the puller only pulls `ready`, so the
   "self-dispatch picks it up" path never ran. Real br 0.2.16: the task is
   `backlog` and absent from ready_queue(). The ask now promotes it through
   the ordinary mark_ready gate, after the once-per-card label so a
   promotion failure can't re-arm the ask. A refusal leaves it filed.
2. The ask waited for the ladder to run out. It sat on the ladder-
   exhausted block, so a card on smart->reasoning->opus timed out three
   times (dispatched a, b, c; asked with timeouts=3), the last on the
   priciest rung, before anything asked for a split.
3. The card was rebuilt anyway. The block beside the ask was the self-
   healing `transient`, so the sweep requeued the card it had just asked
   to split and rebuilt it whole, up to _UNBLOCK_RETRY_MAX more full
   timeouts, racing its own decomposition.

The timeout that reaches decompose_after_timeouts now parks the card
before any climb, under `terminal`, which the sweep never re-runs. The
operator is told once, and the task's agent cancels the card when the
slices exist. The park comes before the ask, because the ask now files
its task `ready` and the agent that picks it up must never find the card
in flight. It parks whatever the ask returns: that conclusion is about
the card, not the filing. A pre-first-token timeout still blocks as
infra (#339) and never reaches this step, and 0 still turns the whole
thing off.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head 92eec5483efb · formal

⚠️ PR advanced 1 commit(s) during this round (92eec5483efba0a344bc2cf2); 0 finding(s) in the delta were demoted to possibly addressed.

This review is a total panel failure, not a clean pass. All finders timed out or hit a gateway error, and the verifier confirmed there were no findings to verify because the panel never produced any. The empty array below reflects infrastructure failure, not an absence of defects. Do not rely on this result for merge decisions — re-run the review workflow (or at minimum the structural pass, which was also skipped) before treating PR #435 as reviewed. No prior requests exist, so no dispositions are owed.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

4 panel step(s) hit their time budget and were skipped this round: find_correctness, find_removed_behavior, find_crossfile, find_conventions. The verdict stands on the remaining angles; a finding only that step would have caught could be missed — the next push re-runs the full panel.

mabry1985 and others added 2 commits September 10, 2026 16:08
…its steps safe (#378)

Review of the first cut found five defects, each reproduced.

1. A fix-round timeout on a card that had already BUILT was parked and
   asked to split, and the split's cancel would close its open PR. The
   count also never reset after a successful build. Now only a FRESH
   build's timeout counts (no kept worktree, no open PR); a fix round
   takes the ordinary timeout path. The count clears when a build
   reaches review: a card that built in one dispatch is not too wide.
2. A pre-first-token (infra) timeout was counted, so after the operator
   fixed the infra, the card's first genuine timeout parked it. The count
   now sits behind the pre-model block, so an infra timeout never counts.
3. A re-park that filed nothing still claimed "parked for a split", and
   an unblock gave no real retry (the count was still at the threshold).
   The ask now files its task in backlog, the park's reason is built from
   what the ask returned (the task, or plainly none plus what to do), and
   only then is the task released to `ready`. The agent that picks it up
   cancels the card, so the card is always parked first. The park has
   its own class, `too-wide`: clear_blocked on it resets the timeout
   count, and the unblock tool and route also drop the running loop's
   cached count (it wins over the label, #259).
4.-5. The task's own steps were unexecutable. Slices marked ready while
   the parked card still claimed their files were refused by the
   shared-file gate, and cancelling the card first released its
   dependents before any slice landed. The spec now lists the card's
   dependents and orders the steps: slices in backlog, dependents
   re-pointed onto them, cancel, slices ready. A real-br test executes
   that sequence as the agent would.

The ask is also idempotent on the task itself, not only the label
written after the create, so a lost label write can't leave an orphan
and file a duplicate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
mabry1985 added a commit that referenced this pull request Sep 10, 2026
…r a fan-out (#425)

Review of the generalised fan-out edge found five issues, each reproduced.

1. One all-timeout fan-out switched max-mode off for the card for good.
   The climb put its "prior attempt timed out" note (#146) in
   _ci_feedback. That slot marks a carried-forward FIX, which disables
   fan-out, and it outlives the drive. So the climbed rung ran ONE
   dispatch, and every later drive of the card also ran single, opening
   with "a PREVIOUS attempt TIMED OUT". The note is now drive-local: a
   timeout climb is a fresh build, so the new rung fans out again
   (codex x2 -> opus x2). The note is spent once the dispatch that
   carried it has run, and dropped on any other climb.
2. The note was mined from the LAST gen. In a fan-out, that is just the
   last candidate, which may have failed fast on something else ("ran
   ~0.0s", no tool). The tap now stamps the stop reason of a gen its
   watchdog killed, on every dispatch path, and the note is mined from
   that gen.
3. A raw (non-WorktreeError) error re-raised from the fan-out skipped the
   drive's shutdown and cancel checks. A BrokenPipeError mid-shutdown
   blocked the card as `unexpected`. Only a WorktreeError can now speak
   for a fan-out; anything else keeps the old no-diff verdict, which
   those checks see first.
4. Untested behaviour, now pinned: every candidate is reaped before the
   drive handles the representative; a CancelledError child is neither
   returned nor raised; and the "a candidate RETURNED -> capability
   climb" fallback is exercised through the real dispatch_coder_tapped,
   not a stand-in.
5. The docs now state the actual rule and the full precedence. A
   drive-level test of the merged #429 x #435 behaviour (a fan-out that
   keeps timing out is parked) lives here; it skips until #435's park is
   on the branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
@mabry1985
mabry1985 merged commit eb904df into main Sep 10, 2026
4 of 5 checks passed
@mabry1985
mabry1985 deleted the fix/timeout-decompose-378 branch September 10, 2026 23:29
mabry1985 added a commit that referenced this pull request Sep 10, 2026
…didate (#425) (#429)

* fix(loop): max-mode hands the drive a shared provider failure (#425)

Max-mode builds a card N ways at once and swallows each candidate's
error. With every candidate dead the drive saw only NoChangesError("max-
mode: all N candidates produced no diff"), a capability verdict, and
climbed a rung. When the candidates died because their PROVIDER did (a
spent quota, #362, or a model it can't serve, #420), that is the wrong
edge: neither says anything about the model, and the single-dispatch and
coder.solve paths already rotate within the rung on both. The review of
#421 reproduced it: rung [codex, ...] with max_mode_n: 2 and codex
refusing its model dispatched codex, codex, escalated, then opus, opus,
and escalated again. No mark, no rotation.

When every candidate failed on its provider, and in the same way, max-
mode now re-raises one of those dispatch errors, so the drive rotates,
marks, backs off and blocks exactly as it does for a single dispatch. One
error can only stand for all of them when they agree. A candidate that
came back with nothing reached the model and failed it. A mix of provider
classes has no single truthful edge (a mark versus a backoff). Both stay
the capability failure they always were.

"Provider failure" now has one definition, provider_failure_category(),
which the drive's rotation and max-mode both ask.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

* docs: changelog fragment for #429

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

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

The first cut re-raised only when every candidate failed on its provider
the same way. But the swallow hid more than provider failures. A timeout
on every candidate never reached #378's timeout counter or its decompose
ask, and a pre-model seam failure never reached #339's block. Both became
a "no diff" capability verdict and climbed a rung, and so did a mix of a
refusal and a quota.

Now, whenever EVERY candidate raised, nothing returned for the selector to
judge, so max-mode re-raises the one error that speaks for them and the
drive's own handling applies as for a single dispatch. The choice is
representative_failure(), most specific edge first:
- a refused model (beside a quota too: the provider is at least partly
  broken, and a climb is wrong either way)
- a spent quota
- a timeout (so #378 sees it)
- a dispatch failure the drive does not retry (#339 blocks it as pre-model
  unless a candidate reached the model)
- any other dispatch failure (retryable: back off and re-run)
- anything else as raised

Ties go to the earliest candidate. Only when a candidate RETURNED, ran and
came back with nothing, is it the capability failure it always was.
provider_failure_category() stays the shared predicate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

* docs: changelog fragment for #429 covers the generalised edge

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

* fix(loop): a timeout climb fans out again; only board errors speak for a fan-out (#425)

Review of the generalised fan-out edge found five issues, each reproduced.

1. One all-timeout fan-out switched max-mode off for the card for good.
   The climb put its "prior attempt timed out" note (#146) in
   _ci_feedback. That slot marks a carried-forward FIX, which disables
   fan-out, and it outlives the drive. So the climbed rung ran ONE
   dispatch, and every later drive of the card also ran single, opening
   with "a PREVIOUS attempt TIMED OUT". The note is now drive-local: a
   timeout climb is a fresh build, so the new rung fans out again
   (codex x2 -> opus x2). The note is spent once the dispatch that
   carried it has run, and dropped on any other climb.
2. The note was mined from the LAST gen. In a fan-out, that is just the
   last candidate, which may have failed fast on something else ("ran
   ~0.0s", no tool). The tap now stamps the stop reason of a gen its
   watchdog killed, on every dispatch path, and the note is mined from
   that gen.
3. A raw (non-WorktreeError) error re-raised from the fan-out skipped the
   drive's shutdown and cancel checks. A BrokenPipeError mid-shutdown
   blocked the card as `unexpected`. Only a WorktreeError can now speak
   for a fan-out; anything else keeps the old no-diff verdict, which
   those checks see first.
4. Untested behaviour, now pinned: every candidate is reaped before the
   drive handles the representative; a CancelledError child is neither
   returned nor raised; and the "a candidate RETURNED -> capability
   climb" fallback is exercised through the real dispatch_coder_tapped,
   not a stand-in.
5. The docs now state the actual rule and the full precedence. A
   drive-level test of the merged #429 x #435 behaviour (a fan-out that
   keeps timing out is parked) lives here; it skips until #435's park is
   on the branch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

* docs: changelog fragment for #429 states the actual rule

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mabry1985 added a commit that referenced this pull request Sep 10, 2026
Resolves the overlaps with #425/#432/#435/#428: failures.py keeps main's seam
shapes beside STRANDED_WORK_CLASS; the kernel exports both TOO_WIDE_CLASS and
STRANDED_WORK_CLASS; the terminal-block path takes main's structure (the #378
ask moved to the park path) with this branch's save-first discard; max-mode's
all-raised fan-out keeps #425's representative failure and discards its own
candidates by path. The new too-wide park removal saves first too, and the
#425 test now expects the candidates discarded by path, not reaped by id.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
mabry1985 added a commit that referenced this pull request Sep 10, 2026
Resolves store.py against #432/#435: keeps main's AlreadyDelivered and
BR_FAILURES beside BoardTimeout (BR_FAILURES stays correct, and now
never sees a raw TimeoutExpired from _run), and main's task-signal pass
in list_features beside the pre-filtered, batched reads (the archived
post-filter is redundant once rows are filtered before the show).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
@protoreview

protoreview Bot commented Sep 10, 2026

Copy link
Copy Markdown

⚠️ QA panel exhausted — this PR has not been reviewed.
The review panel failed after 2 attempt(s) on head ad07ce6312b9. No verdict was posted.
A new push will re-trigger the review.

mabry1985 added a commit that referenced this pull request Sep 10, 2026
Resolves against #435/#432: keeps main's drive-local timeout_note beside
the attempt counter, and takes main's move of the #378 decomposition
request into _park_for_split (the terminal block site keeps only the
stand-aside helper). _park_for_split blocks the card as too-wide, so it
now asks first like every other drive-side block: a held card is not
re-blocked as too wide and gets no split task (new test; without the
guard the hold is replaced by too-wide). Main's task drives already
re-read before blocking (#432).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A large card can still time out forever: the design gate is not a breadth cap, and a repeated timeout should decompose rather than park

1 participant