From 92eec5483efbd922ba4215ee777662560fb2e9ee Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 10 Sep 2026 15:07:43 -0700 Subject: [PATCH 1/4] fix(loop): a timed-out card is parked for its split, and the ask is dispatched (#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) Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av --- docs/configuration.md | 11 +- loop/drive.py | 64 +++++--- protoagent.plugin.yaml | 5 +- store.py | 26 +++- tests/test_timeout_decompose_378.py | 217 ++++++++++++++++++++++++++++ 5 files changed, 296 insertions(+), 27 deletions(-) create mode 100644 tests/test_timeout_decompose_378.py diff --git a/docs/configuration.md b/docs/configuration.md index 8093471..57f2bc8 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -141,10 +141,13 @@ The gates between a green build and main. `decompose_after_timeouts` is the one that changes behaviour rather than tuning it. A coder timeout is a **size** signal, not a capability one: it produces no diff and no CI output, so a retry re-sends a near-identical prompt and climbing the model ladder spends a stronger model on -a card that was never model-limited. After this many timeouts on one card, the loop files a task -asking this agent to split it into buildable slices — once per card, and never for a -pre-first-token timeout (that is an infra fault, and splitting would be the wrong remedy). Set -`0` to switch the ask off and have the card simply block, as it did before. +a card that was never model-limited. On the timeout that reaches this count, the loop files a +`ready` task asking this agent to split the card into buildable slices, and parks the card for +it. The card does not climb another rung, and the blocked sweep does not rebuild it (that would +only time out again, racing the split). The operator is told once, and the agent cancels the +card when the slices exist. The ask is made once per card, and never for a pre-first-token +timeout, which is an infra fault that splitting would not fix. Set `0` to switch the ask off and +have the card simply block, as it did before. ## Concurrency diff --git a/loop/drive.py b/loop/drive.py index 47d8ff3..c43d45f 100644 --- a/loop/drive.py +++ b/loop/drive.py @@ -1669,6 +1669,7 @@ async def _drive(self, feature: dict): # #378: count timeouts durably, BEFORE the retry/escalate/block fork — # bd-sxxf timed out, escalated a tier, timed out again and only then # blocked, so a counter bumped at the block alone would have read 1. + timeouts = 0 # this card's timeouts so far, THIS one included; 0 = not a timeout if isinstance(exc, worktree.CoderTimeout): timeouts = await self._budget_get(store, fid, "timeout") + 1 await self._budget_set(store, fid, "timeout", timeouts) @@ -1820,6 +1821,52 @@ async def _drive(self, feature: dict): await worktree.remove_worktree(repo, wt, branch or "") self._inflight.pop(fid, None) return + # 1.7 The timeout that brings this card to `decompose_after_timeouts` → PARK it, + # and ask the board's own agent to split it (#378). A repeated + # timeout is a SIZE signal: no diff, no CI output, so neither a stronger + # rung nor a rebuild has anything new to work with (on a ladder, the first + # timeout's climb already carried #146's timeout context). The ask used to be + # made only once the ladder ran out, and the block beside it was the + # self-healing `transient` — so a card on a three-rung ladder timed out + # three times before it was asked, then the sweep requeued it and rebuilt + # it whole twice more, racing the very decomposition it had just filed. + # Now: no climb, and `terminal`, which the sweep never re-runs — it tells + # the operator once, and the task's agent retires the card when the slices + # land. Parked whatever the ask returns: that conclusion is about the card, + # not the filing (a card asked once and requeued by hand that times out + # again is just as wide). A pre-first-token timeout never gets here (1.5: + # infra, not size). + threshold = self.decompose_after_timeouts # 0 = never (a timeout blocks as before) + if timeouts and threshold and timeouts >= threshold: + reason = ( + f"too wide to build in one dispatch — timed out {timeouts}x, so it is parked for " + f"a split into slices instead of climbing a tier or being rebuilt: {exc}" + ) + log.warning( + "[project_board] %s timed out %dx — parked for a split (a timeout is a SIZE signal, " + "not a capability one)", + fid, + timeouts, + ) + # Park BEFORE asking: the ask files its task `ready`, and the agent that + # picks it up cancels this card — it must never find the card in flight. + await asyncio.to_thread(store.flag_blocked, fid, reason, category="terminal") + ask = getattr(store, "request_decomposition", None) # older/stub store: skip + asked = await asyncio.to_thread(ask, fid, timeouts=timeouts) if callable(ask) else None + if asked: + log.info( + "[project_board] %s filed %s for this agent to split it", fid, asked.get("id", "?") + ) + else: + log.warning( + "[project_board] %s no new decompose task filed (asked before, or the store " + "refused) — it waits for the operator", + fid, + ) + if wt: + await worktree.remove_worktree(repo, wt, branch or "") + self._inflight.pop(fid, None) + return # 2. Capability failure + a ladder → climb a model tier (fresh budget). if self.escalation_on and capability: nxt = await asyncio.to_thread(store.escalate, fid, str(exc)[:200]) @@ -1869,23 +1916,6 @@ async def _drive(self, feature: dict): await asyncio.to_thread( store.flag_blocked, fid, f"{policy.category}: {exc}", category=policy.category ) - # #378: a card that has now timed out repeatedly is not model-limited, it - # is too wide — and nothing in the block path says so, which is how one - # sat parked while an operator guessed. Ask the board's own agent to split - # it (once; `request_decomposition` no-ops on a repeat or on a task). - if isinstance(exc, worktree.CoderTimeout) and self.decompose_after_timeouts: - spent = await self._budget_get(store, fid, "timeout") - ask = getattr(store, "request_decomposition", None) # older/stub store: skip - if spent >= self.decompose_after_timeouts and callable(ask): - asked = await asyncio.to_thread(ask, fid, timeouts=spent) - if asked: - log.warning( - "[project_board] %s timed out %dx — filed %s to decompose it " - "(a timeout is a SIZE signal, not a capability one)", - fid, - spent, - asked.get("id", "?"), - ) if wt: await worktree.remove_worktree(repo, wt, branch or "") self._inflight.pop(fid, None) diff --git a/protoagent.plugin.yaml b/protoagent.plugin.yaml index 009a1d9..b12b74f 100644 --- a/protoagent.plugin.yaml +++ b/protoagent.plugin.yaml @@ -211,7 +211,8 @@ config: # A repeated coder timeout is a SIZE signal, not a capability one: it carries no diff and # no CI output, so a retry gets a near-identical prompt and a tier climb spends a stronger # model on a card that was never model-limited. After this many timeouts on one card, the - # loop files a task asking the board's own agent to split it (once per card). 0 = off. + # loop files a task asking the board's own agent to split it (once per card) and parks the + # card for it — no further tier climb or rebuild. 0 = off. decompose_after_timeouts: 2 # (in_progress with no live drive → ready) + reap orphaned # feat- worktrees (feature gone/done). 0 disables it. @@ -260,7 +261,7 @@ settings: - { key: loop_enabled, label: "Run orchestration loop", type: bool, group: "Project Board", tab: automation, restart: true, description: "Start the background puller after the member restarts. Off keeps the board and tools available without dispatching work." } - { key: decompose_after_timeouts, label: "Ask to split after N timeouts", type: number, minimum: 0, maximum: 10, group: "Project Board", tab: automation, restart: true, - description: "A repeated coder timeout means the card is too WIDE, not that the model is too weak: it produces no diff and no CI output, so a retry re-sends the same prompt and a tier climb spends a stronger model on the wrong problem. After this many timeouts on one card, file a task asking this agent to split it into buildable slices (once per card). 0 turns the ask off and the card simply blocks." } + description: "A repeated coder timeout means the card is too WIDE, not that the model is too weak: it produces no diff and no CI output, so a retry re-sends the same prompt and a tier climb spends a stronger model on the wrong problem. After this many timeouts on one card, file a task asking this agent to split it into buildable slices (once per card) and park the card for it, with no further tier climb or rebuild. 0 turns the ask off and the card simply blocks." } - { key: coder, label: "Coder delegate", type: string, group: "Project Board", tab: automation, description: "Name of the acp delegate (Settings ▸ Delegates) the loop dispatches builds to. No default. Applies live on save: a paused loop resumes on its next check." } - { key: br_autofetch, label: "Fetch br automatically", type: bool, group: "Project Board", tab: automation, diff --git a/store.py b/store.py index 62fc838..40de347 100644 --- a/store.py +++ b/store.py @@ -1704,10 +1704,16 @@ def request_decomposition(self, fid: str, *, timeouts: int) -> dict | None: Rather than decompose inline (the ``decompose`` subagent is a pure proposer driven by a skill, and an LLM call inside the drive loop can itself time out), file a - TASK assigned to the board's own agent. The existing self-dispatch path (#311) - picks it up, the agent decomposes with the board tools it already has, and the - Ready gate enforces that the slices are actually well-formed — which is the part - an unattended splitter gets wrong. + TASK assigned to the board's own agent, and promote it to ``ready``. The existing + self-dispatch path (#311) picks it up, the agent decomposes with the board tools it + already has, and the Ready gate enforces that the slices are actually well-formed — + which is the part an unattended splitter gets wrong. + + The promotion is not optional. ``create_feature`` files every bead in ``backlog``, + and the puller only pulls ``ready``: the ask as first shipped sat in the backlog + until someone noticed it, so the loop that "asked its own agent" had in fact asked + nobody. It goes through the ordinary ``mark_ready`` gate; a refusal leaves the task + filed in backlog for a human rather than un-asking. Returns the new task, or None when the ask was already made (idempotent) or the card is itself a decomposition task (never recurse). Never raises: failing to ask @@ -1762,6 +1768,18 @@ def request_decomposition(self, fid: str, *, timeouts: int) -> dict | None: ) self._run("update", fid, "--add-label", self.LABEL_DECOMPOSE_ASKED) self.comment(fid, f"decompose requested after {timeouts} timeouts → {(task or {}).get('id', '?')}") + # After the once-per-card label, so a promotion failure can never re-arm the ask. + if (task or {}).get("id"): + try: + task = self.mark_ready(task["id"]) or task + except Exception: # noqa: BLE001 — filed is still asked; a human can promote it + log.warning( + "[project_board] %s decompose task %s filed but not promoted to ready — it waits " + "in backlog until someone marks it ready", + fid, + task["id"], + exc_info=True, + ) return task except Exception: # noqa: BLE001 — the ask is best-effort; the block still happens log.warning("[project_board] %s decompose request failed (ignored)", fid, exc_info=True) diff --git a/tests/test_timeout_decompose_378.py b/tests/test_timeout_decompose_378.py new file mode 100644 index 0000000..ada03ea --- /dev/null +++ b/tests/test_timeout_decompose_378.py @@ -0,0 +1,217 @@ +"""#378: a card that keeps timing out is too wide, and the loop must stop re-running it. + +#379 capped `large` in the breadth gate and #380 made a repeated timeout ask the board's +own agent to split the card. What was left, each one 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 board's own agent" was + asked by nobody (real `br`: the task is `backlog` and absent from `ready_queue`). +2. The ask waited for the ladder to run out. It lived on the ladder-exhausted block, so + a card on a three-rung ladder timed out three times — the last on the priciest model + — before anything asked for a split. +3. The card was then 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 two more full timeouts, racing its own decomposition. +""" + +from __future__ import annotations + +import shutil + +import pytest + +from project_board import coder_seam, worktree +from project_board import store as store_mod +import project_board.loop as loop_mod +from project_board.loop import BoardLoop +from project_board.store import BeadsBoard, BoardError + +from test_loop import _BlockedStore, _EscalatingStore, _blocked, _no_sleep + +_TIMEOUT = "coder timed out after 1800s" + + +class _SplitStore(_EscalatingStore): + """Climbs through `tiers`, keeps budget labels durably, and answers the decompose ask + with `answer` (a task dict, or None for "already asked / could not file").""" + + def __init__(self, tiers=(), *, answer=None): + super().__init__(list(tiers)) + self.asked: list[tuple[str, int]] = [] + self.budgets: dict[str, int] = {} + self._answer = {"id": "bd-split"} if answer is None else answer + + def record_budget(self, fid, kind, n): + self.budgets[f"{fid}:{kind}"] = n + + def clear_budgets(self, fid, kinds=None): + pass + + def get_feature(self, fid): + labels = [f"budget:{k.split(':', 1)[1]}:{v}" for k, v in self.budgets.items() if k.startswith(f"{fid}:")] + return {"id": fid, "labels": labels, "board_state": "in_progress"} + + def request_decomposition(self, fid, *, timeouts): + self.asked.append((fid, timeouts)) + self.calls.append(("request_decomposition", fid, timeouts)) + return self._answer or None + + +def _blocks(store): + return [c for c in store.calls if c[0] == "flag_blocked"] + + +def _timing_out_board(monkeypatch, store, cfg, *, start_tier="smart"): + """A drive whose every dispatch WORKS (a tool call reaches the ring buffer — this is a + size signal, not the pre-first-token infra timeout #339 blocks) and then times out.""" + monkeypatch.setattr("project_board.loop.get_store", lambda **_kw: store) + monkeypatch.setattr("project_board.loop.asyncio.sleep", _no_sleep) + dispatched: list[str] = [] + + async def _create(repo, base, fid, root, title="", **_kw): + return ("/wt/feat-" + fid, "feat/" + fid) + + async def _noop(*_a, **_kw): + return None + + async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): + dispatched.append(coder) + coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {"path": "a.py"}}) + raise worktree.CoderTimeout(_TIMEOUT) + + monkeypatch.setattr(worktree, "create_worktree", _create) + monkeypatch.setattr(worktree, "dispatch_coder", _dispatch) + monkeypatch.setattr(worktree, "remove_worktree", _noop) + monkeypatch.setattr(worktree, "reap_feature_worktree", _noop) + loop = BoardLoop(cfg) + monkeypatch.setattr(store, "current_tier", lambda fid: start_tier, raising=False) + monkeypatch.setattr(loop, "_resolve_delegate", lambda name, expect: name) + return loop, dispatched + + +async def test_the_second_timeout_asks_for_a_split_instead_of_climbing_again(monkeypatch): + """Gap 2. The first timeout climbs, carrying #146's timeout context — one can be an + unlucky gate run. The second is the size signal: ask, and stop. It must not spend the + top rung on a card the loop has just concluded is too wide.""" + store = _SplitStore(tiers=["reasoning", "opus"]) + loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b", "opus": "c"}}) + await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) + + assert dispatched == ["a", "b"], f"the threshold timeout climbed instead of asking: {dispatched}" + assert store.asked == [("bd-1", 2)] + assert len(store.escalated) == 1 # the first timeout's climb, and only that + blocked = _blocks(store) + assert len(blocked) == 1 and "timed out 2x" in blocked[0][2] + # Parked BEFORE the ask: the ask files its task `ready`, and the agent that picks it up + # cancels this card — it must never find the card still in flight. + names = store.names() + assert names.index("flag_blocked") < names.index("request_decomposition") + + +async def test_a_card_parked_for_its_split_is_not_rebuilt_by_the_blocked_sweep(monkeypatch): + """Gap 3, end to end: the block the drive writes, read back by the sweep. The card the + loop just asked to split must stay parked — the operator hears about it once — instead + of being requeued and rebuilt whole while its decomposition is under way.""" + store = _SplitStore() + store.budgets["bd-1:timeout"] = 1 # a one-coder board: the first timeout already blocked + loop, dispatched = _timing_out_board(monkeypatch, store, {"coder": "proto"}) + await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) + assert store.asked == [("bd-1", 2)] + (_, fid, reason, category) = _blocks(store)[-1] + + lane = _BlockedStore([_blocked(fid, category.replace("_", "-"), reason=reason, title="Wide card")]) + sweep = BoardLoop({"coder": "proto"}) + told: list[str] = [] + monkeypatch.setattr(sweep, "_notify_operator", lambda _fid, text, **_kw: told.append(text)) + await sweep._recover_blocked(lane) + + assert lane.requeued == [] and lane.cleared == [], f"the sweep rebuilt a card parked for its split ({category})" + assert len(told) == 1 and "timed out 2x" in told[0] and "Wide card" in told[0] + assert category.replace("_", "-") not in loop_mod._SELF_HEALING_BLOCKS + + +async def test_a_card_past_the_threshold_is_parked_even_when_no_new_ask_is_filed(monkeypatch): + """The park follows from the card, not from the filing. Here the store files nothing — + the card was asked once already and an operator requeued it since, or the store could + not file. It timed out past the threshold again: it is exactly as wide, so it is parked + for the operator, not climbed onto a pricier rung or handed back to the sweep.""" + store = _SplitStore(tiers=["reasoning"], answer={}) + store.budgets["bd-1:timeout"] = 3 # well past the threshold + loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b"}}) + await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) + + assert dispatched == ["a"] and store.escalated == [], dispatched + assert store.asked == [("bd-1", 4)] # still asked — the store decides whether anything is filed + blocked = _blocks(store) + assert len(blocked) == 1 and blocked[0][3] not in ("transient", "rate_limit", "merge_conflict") + + +# ── gap 1: the ask has to reach the puller ──────────────────────────────────────────── + + +@pytest.mark.skipif(shutil.which(store_mod.BR) is None, reason="real `br` (beads) CLI not on PATH") +def test_the_decompose_ask_is_filed_ready_so_the_agent_actually_gets_it(tmp_path): + """Against REAL `br`: the puller's queue is `br ready --label ready`. A task filed in + `backlog` is never dispatched, whatever its assignee — so the ask must come out of + `request_decomposition` already promoted, through the ordinary Ready gate.""" + board = BeadsBoard(repo=str(tmp_path), actor="test") + wide = board.create_feature( + "Wide card that keeps timing out", + spec="build the whole subsystem", + acceptance_criteria="- WHEN done THE SYSTEM SHALL work", + files_to_modify=["a.py (new)", "b.py (new)"], + ) + task = board.request_decomposition(wide["id"], timeouts=2) + + assert task is not None + assert board.get_feature(task["id"])["board_state"] == "ready" + assert task["id"] in [f["id"] for f in board.ready_queue()], "the self-dispatch path never sees the ask" + + +def test_a_refused_promotion_still_counts_as_asked(make_board, monkeypatch): + """If the Ready gate refuses the task, it stays filed in backlog for a human — the card + was still asked (its once-per-card label is on), and the promotion failure is logged, + not raised: the ask never raises into the drive.""" + calls: list[tuple] = [] + + def _br(*args, want_json=False): + calls.append(args) + return {} + + b = make_board(_br) + monkeypatch.setattr( + b, "get_feature", lambda fid: {"id": fid, "title": "Wide", "labels": [], "issue_type": "feature"} + ) + monkeypatch.setattr(b, "create_feature", lambda title, **kw: {"id": "bd-9"}) + monkeypatch.setattr(b, "comment", lambda fid, text: None) + + def _refuse(fid): + raise BoardError(f"Ready gate: {fid} refused") + + monkeypatch.setattr(b, "mark_ready", _refuse) + assert b.request_decomposition("bd-8", timeouts=2) == {"id": "bd-9"} + assert ("update", "bd-8", "--add-label", "decompose-asked") in calls + + +def test_the_ask_is_promoted_after_the_once_per_card_label(make_board, monkeypatch): + """Order matters: the label is what stops a second ask. Promoting first would let a + promotion crash leave a filed task with no label, and the next timeout would file a + duplicate.""" + order: list[str] = [] + + def _br(*args, want_json=False): + if "decompose-asked" in args: + order.append("label") + return {} + + b = make_board(_br) + monkeypatch.setattr( + b, "get_feature", lambda fid: {"id": fid, "title": "Wide", "labels": [], "issue_type": "feature"} + ) + monkeypatch.setattr(b, "create_feature", lambda title, **kw: {"id": "bd-9"}) + monkeypatch.setattr(b, "comment", lambda fid, text: None) + monkeypatch.setattr(b, "mark_ready", lambda fid: order.append("ready") or {"id": fid, "board_state": "ready"}) + out = b.request_decomposition("bd-8", timeouts=2) + + assert order == ["label", "ready"] + assert out == {"id": "bd-9", "board_state": "ready"} From a0a344bc2cf24977aa4f40e0abdac1b22d47c189 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 10 Sep 2026 15:09:36 -0700 Subject: [PATCH 2/4] docs: changelog fragment for #435 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av --- changelog.d/435.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog.d/435.md diff --git a/changelog.d/435.md b/changelog.d/435.md new file mode 100644 index 0000000..ecbc85a --- /dev/null +++ b/changelog.d/435.md @@ -0,0 +1,8 @@ +A card that keeps timing out is now parked for its split instead of being rebuilt. The decompose +task a repeated timeout files used to land in backlog, where the loop never dispatches, so the +board's own agent was never actually asked. It is now filed `ready`. The ask also waited for the +model ladder to run out, so a card on a three-rung ladder timed out three times first. And it was +paired with a self-healing block, so the sweep requeued the oversized card and rebuilt it twice +more, racing its own split. On the timeout that reaches `decompose_after_timeouts`, the loop now +parks the card before climbing again, tells the operator once, and leaves it for the agent to +split and retire. From 0ea4040e83ebd3cd06b475fa6c74b1a446802fe4 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 10 Sep 2026 16:08:23 -0700 Subject: [PATCH 3/4] fix(loop): park only fresh-build size timeouts, name the split, keep 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) Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av --- __init__.py | 8 +- api.py | 8 +- docs/configuration.md | 36 ++- failures.py | 9 + loop/_common.py | 19 +- loop/drive.py | 144 ++++++---- protoagent.plugin.yaml | 9 +- store.py | 118 +++++--- tests/test_integration.py | 7 +- tests/test_loop.py | 3 + tests/test_timeout_decompose_378.py | 427 ++++++++++++++++++++++------ 11 files changed, 589 insertions(+), 199 deletions(-) diff --git a/__init__.py b/__init__.py index 9adaa6e..c185c11 100644 --- a/__init__.py +++ b/__init__.py @@ -949,9 +949,15 @@ def board_block_feature(feature_id: str, reason: str) -> str: def board_unblock_feature(feature_id: str) -> str: """Clear the `blocked` flag so the feature can be re-dispatched — the inverse of board_block_feature. Removes the blocked label; the puller can claim it again once - it's otherwise `ready`.""" + it's otherwise `ready`. A card parked `too-wide` after repeated timeouts also gets + its timeout count reset, so the retry is a real attempt.""" try: f = get_store(**store_kw).clear_blocked(feature_id) + # The running loop's cached timeout count wins over the label the store just + # reset (#259) — drop it too (#378). Lazy import: the loop imports from here. + from .loop import forget_timeout_count + + forget_timeout_count(feature_id) return json.dumps({"id": f["id"], "state": f["board_state"]}) except BoardError as exc: return f"Error: {exc}" diff --git a/api.py b/api.py index 926a9ee..547fe74 100644 --- a/api.py +++ b/api.py @@ -677,7 +677,13 @@ async def _block(fid: str, body: dict = Body(...)): @router.post("/features/{fid}/unblock") async def _unblock(fid: str): - return await _guard(lambda: store().clear_blocked(fid)) + f = await _guard(lambda: store().clear_blocked(fid)) + # The running loop's cached timeout count wins over the label the store just reset + # (#259) — drop it too, so a card parked `too-wide` gets a real retry (#378). + from .loop import forget_timeout_count + + forget_timeout_count(fid) + return f @router.post("/features/{fid}/cancel") async def _cancel(fid: str, body: dict = Body(default={})): diff --git a/docs/configuration.md b/docs/configuration.md index 57f2bc8..55a3ea0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -139,15 +139,33 @@ The gates between a green build and main. | `kg_lessons_domain` | `"loop-lessons"` | reload **· YAML only** | `decompose_after_timeouts` is the one that changes behaviour rather than tuning it. A coder -timeout is a **size** signal, not a capability one: it produces no diff and no CI output, so a -retry re-sends a near-identical prompt and climbing the model ladder spends a stronger model on -a card that was never model-limited. On the timeout that reaches this count, the loop files a -`ready` task asking this agent to split the card into buildable slices, and parks the card for -it. The card does not climb another rung, and the blocked sweep does not rebuild it (that would -only time out again, racing the split). The operator is told once, and the agent cancels the -card when the slices exist. The ask is made once per card, and never for a pre-first-token -timeout, which is an infra fault that splitting would not fix. Set `0` to switch the ask off and -have the card simply block, as it did before. +timeout on a fresh build is a **size** signal, not a capability one. It produces no diff and no +CI output, so a retry re-sends a near-identical prompt, and climbing the model ladder spends a +stronger model on a card that was never model-limited. + +Only those timeouts count: + +- A pre-first-token timeout is an infra fault that splitting would not fix. It never counts, + and never asks. +- A timeout on a fix round doesn't count either: the card already has a PR, or the coder was + fixing a kept worktree. A card that built in one dispatch is not too wide. +- The count clears when a build reaches review. + +On the timeout that reaches this count, the loop parks the card (blocked class `too-wide`) and +files a `ready` task asking this agent to split it. The card does not climb another rung, and +the blocked sweep does not rebuild it, since that would only time out again, racing the split. +The operator is told once, and the block reason names the task, or says that none was filed. + +The task gives the agent a fixed order, each step passing the gates the next one relies on: + +1. Create the slices, left in backlog. +2. Re-point the card's dependents onto the slices they need. +3. Cancel the card. +4. Mark the slices ready. + +The ask is made once per card. Unblocking a parked card resets its count, so a retry after +raising `coder_timeout_s` is a real attempt. Set `0` to switch the ask off and have the card +simply block, as it did before. ## Concurrency diff --git a/failures.py b/failures.py index 1d22195..0d07c52 100644 --- a/failures.py +++ b/failures.py @@ -85,6 +85,15 @@ class it would otherwise have (a `502 … timeout` is still transient).""" return TERMINAL +# The `blocked-class:` of a card the loop PARKED because its fresh builds keep timing out +# (#378): too wide to build in one dispatch, with a split handed to the board's own agent. +# Like `dispatch-infra` below it is the loop's own class, not one of `classify()`'s: the +# blocked sweep never re-runs it (it would only time out again, racing its own split), and +# an operator unblock resets the card's timeout count, so a deliberate retry — after +# raising `coder_timeout_s`, say — gets a real attempt instead of re-parking at once. +TOO_WIDE_CLASS = "too-wide" + + # ── pre-model dispatch / infrastructure failures (#339) ────────────────────────── # The `blocked-class:` a pre-model dispatch/infra failure carries. It is deliberately # NOT one of `classify()`'s categories: it can't be decided from the message alone diff --git a/loop/_common.py b/loop/_common.py index 7d8a3fa..475564a 100644 --- a/loop/_common.py +++ b/loop/_common.py @@ -50,7 +50,7 @@ import types from .. import br_fetch, coder_seam, config, health, setup_check, work_snapshot, worktree -from ..failures import PRE_MODEL_DISPATCH_CLASS, classify, is_pre_model_dispatch_failure +from ..failures import PRE_MODEL_DISPATCH_CLASS, TOO_WIDE_CLASS, classify, is_pre_model_dispatch_failure from ..projects import default_project as resolve_default_project from ..projects import resolve_projects from .. import store as store_mod @@ -1129,6 +1129,21 @@ def reset_merged_verify_budget(fid: str, store) -> bool: return True +def forget_timeout_count(fid: str) -> bool: + """Drop the live loop's cached timeout count for ``fid`` after an operator unblock + (#378), so the next ``_budget_get`` re-reads it from the bead. ``clear_blocked`` resets + the persisted count when it releases a card parked as `too-wide`, but the loop's cache + wins over the labels (#259): without this, a retry after raising `coder_timeout_s` + would re-park on its very first timeout. Forgetting — not pinning 0 — is right for any + unblock: the cache simply re-syncs with whatever the bead now says. No lock needed: a + blocked card has no drive in flight to race. Returns False when no loop is running.""" + loop = live_loop() + if loop is None: + return False + loop._budget_cache("timeout").pop(fid, None) + return True + + def cancel_pr_comment(fid: str) -> str: return f"cancelled by operator — see card {fid}" @@ -1289,6 +1304,7 @@ def _inbox_db_path(): "work_snapshot", "worktree", "PRE_MODEL_DISPATCH_CLASS", + "TOO_WIDE_CLASS", "classify", "is_pre_model_dispatch_failure", "resolve_default_project", @@ -1370,6 +1386,7 @@ def _inbox_db_path(): "_unregister_loop", "live_loop", "reset_merged_verify_budget", + "forget_timeout_count", "cancel_pr_comment", "cancel_side_effects", "_MAX_MODE_JUDGE_SYS", diff --git a/loop/drive.py b/loop/drive.py index c43d45f..0d9969e 100644 --- a/loop/drive.py +++ b/loop/drive.py @@ -1666,13 +1666,6 @@ async def _drive(self, feature: dict): # it burned the whole tier ladder in ten seconds (three attempts, # three tiers, a block; 2026-08-28, bd-cwpv.12/.16) and left `tier:` # labels that misrouted the card when it was requeued after the reset. - # #378: count timeouts durably, BEFORE the retry/escalate/block fork — - # bd-sxxf timed out, escalated a tier, timed out again and only then - # blocked, so a counter bumped at the block alone would have read 1. - timeouts = 0 # this card's timeouts so far, THIS one included; 0 = not a timeout - if isinstance(exc, worktree.CoderTimeout): - timeouts = await self._budget_get(store, fid, "timeout") + 1 - await self._budget_set(store, fid, "timeout", timeouts) dispatch_failed = str(exc).startswith("coder dispatch failed") and not policy.retryable capability = ( isinstance(exc, (worktree.NoChangesError, worktree.CoderTimeout, coder_seam.SolveExhausted)) @@ -1821,52 +1814,34 @@ async def _drive(self, feature: dict): await worktree.remove_worktree(repo, wt, branch or "") self._inflight.pop(fid, None) return - # 1.7 The timeout that brings this card to `decompose_after_timeouts` → PARK it, - # and ask the board's own agent to split it (#378). A repeated - # timeout is a SIZE signal: no diff, no CI output, so neither a stronger - # rung nor a rebuild has anything new to work with (on a ladder, the first - # timeout's climb already carried #146's timeout context). The ask used to be - # made only once the ladder ran out, and the block beside it was the - # self-healing `transient` — so a card on a three-rung ladder timed out - # three times before it was asked, then the sweep requeued it and rebuilt - # it whole twice more, racing the very decomposition it had just filed. - # Now: no climb, and `terminal`, which the sweep never re-runs — it tells - # the operator once, and the task's agent retires the card when the slices - # land. Parked whatever the ask returns: that conclusion is about the card, - # not the filing (a card asked once and requeued by hand that times out - # again is just as wide). A pre-first-token timeout never gets here (1.5: - # infra, not size). - threshold = self.decompose_after_timeouts # 0 = never (a timeout blocks as before) - if timeouts and threshold and timeouts >= threshold: - reason = ( - f"too wide to build in one dispatch — timed out {timeouts}x, so it is parked for " - f"a split into slices instead of climbing a tier or being rebuilt: {exc}" - ) - log.warning( - "[project_board] %s timed out %dx — parked for a split (a timeout is a SIZE signal, " - "not a capability one)", - fid, - timeouts, - ) - # Park BEFORE asking: the ask files its task `ready`, and the agent that - # picks it up cancels this card — it must never find the card in flight. - await asyncio.to_thread(store.flag_blocked, fid, reason, category="terminal") - ask = getattr(store, "request_decomposition", None) # older/stub store: skip - asked = await asyncio.to_thread(ask, fid, timeouts=timeouts) if callable(ask) else None - if asked: - log.info( - "[project_board] %s filed %s for this agent to split it", fid, asked.get("id", "?") - ) - else: - log.warning( - "[project_board] %s no new decompose task filed (asked before, or the store " - "refused) — it waits for the operator", - fid, - ) - if wt: - await worktree.remove_worktree(repo, wt, branch or "") - self._inflight.pop(fid, None) - return + # 1.7 A card whose FRESH builds keep timing out → PARK it, and hand its split to + # the board's own agent (#378). A repeated timeout is a SIZE signal: no diff, + # no CI output, so neither a stronger rung nor a rebuild has anything new to + # work with (on a ladder, the first timeout's climb already carried #146's + # timeout context). Only a timeout that IS that signal counts: + # • one that reached the model — a pre-first-token timeout is infra, and 1.5 + # above has already blocked it, uncounted; + # • on a fresh build — a fix round (a kept worktree, or a card whose PR is + # open) is fixing a card that already BUILT in one dispatch, so it is not + # too wide. It takes the ordinary path below. Parked, it would ask to split + # a card with an open PR, and the split's cancel would close that PR. + # The count is durable (a `budget:` label, so a restart can't reset it) and + # is cleared when a build reaches review or an operator unblocks the park. + # Counting it here, before the climb/block fork, is what lets bd-sxxf's shape + # — timed out, climbed, timed out again — reach the threshold at all. The ask + # used to wait for the ladder to run out, beside a self-healing `transient` + # block: a three-rung ladder timed out three times before it asked, and the + # sweep then rebuilt the card twice more, racing its own split. + if isinstance(exc, worktree.CoderTimeout) and not reusing and not feature.get("pr_url"): + timeouts = await self._budget_get(store, fid, "timeout") + 1 + await self._budget_set(store, fid, "timeout", timeouts) + threshold = self.decompose_after_timeouts # 0 = never park (block as before) + if threshold and timeouts >= threshold: + await self._park_for_split(store, fid, timeouts, exc) + if wt: + await worktree.remove_worktree(repo, wt, branch or "") + self._inflight.pop(fid, None) + return # 2. Capability failure + a ladder → climb a model tier (fresh budget). if self.escalation_on and capability: nxt = await asyncio.to_thread(store.escalate, fid, str(exc)[:200]) @@ -1925,8 +1900,11 @@ async def _drive(self, feature: dict): log.info("[project_board] %s coder done (%d chars) → %s", fid, len(result or ""), pr_url) await asyncio.to_thread(store.open_review, fid, pr_url=pr_url) # Gate passed — reset the pre-PR budgets (goal-fix, local-gate, the - # requirement ledger #113, and the empty-result count #198). - await self._budget_reset(store, fid, "goal-fix", "gate-fix", "req-fix", "empty-result", "ledger-only") + # requirement ledger #113, and the empty-result count #198), and the timeout + # count (#378): a card that built in one dispatch is demonstrably not too wide. + await self._budget_reset( + store, fid, "goal-fix", "gate-fix", "req-fix", "empty-result", "ledger-only", "timeout" + ) if self.review_gate: # Blocking adversarial review (M5). May requeue the feature with # findings injected — the next drive carries them in the prompt. @@ -1964,6 +1942,62 @@ async def _drive(self, feature: dict): await worktree.remove_worktree(repo, wt, branch or "") self._inflight.pop(fid, None) + async def _park_for_split(self, store, fid: str, timeouts: int, exc: Exception) -> None: + """Park a card whose fresh builds keep timing out, and hand its split to the board's + own agent (#378) — in the one order that is safe: + + 1. FILE the decompose task. ``request_decomposition`` leaves it in backlog, where the + puller cannot see it, and returns the task still open for this card (new, or filed + earlier), or None. + 2. PARK the card under ``too-wide`` — never auto-healed; an operator unblock resets the + count — with a reason built from what step 1 returned: it names the task, or says + plainly that none was filed and what to do instead. (Building the reason before the + ask told a card re-parked after an operator requeue that a split was on its way + when none had been filed.) + 3. RELEASE the task, only now: the agent that picks it up ends by cancelling this card, + which must never be found still in flight.""" + ask = getattr(store, "request_decomposition", None) # older/stub store: skip + task = await asyncio.to_thread(ask, fid, timeouts=timeouts) if callable(ask) else None + task_id = str((task or {}).get("id") or "") + retry = "raise coder_timeout_s and unblock it (unblocking resets its timeout count)" + if task_id: + reason = ( + f"too wide to build in one dispatch — timed out {timeouts}x; parked while this agent splits " + f"it into slices ({task_id}). No tier climb and no auto-retry; to retry it whole instead, " + f"{retry}: {exc}" + ) + else: + reason = ( + f"too wide to build in one dispatch — timed out {timeouts}x, and NO split task was filed (one " + f"was requested for this card before, or the store refused): split it by hand, or {retry}: {exc}" + ) + log.warning( + "[project_board] %s timed out %dx on fresh builds — parked as too wide (%s)", + fid, + timeouts, + f"split task {task_id}" if task_id else "no split task filed", + ) + await asyncio.to_thread(store.flag_blocked, fid, reason, category=TOO_WIDE_CLASS) + release = getattr(store, "mark_ready", None) + if task_id and str(task.get("board_state") or "backlog") == "backlog" and callable(release): + try: + await asyncio.to_thread(release, task_id) + except Exception: # noqa: BLE001 — the park stands; a human can promote the task + log.warning( + "[project_board] %s split task %s is filed but the Ready gate refused it — it waits in backlog", + fid, + task_id, + exc_info=True, + ) + try: + await asyncio.to_thread( + store.comment, + fid, + f"split task {task_id} is filed but NOT ready — the Ready gate refused it; promote it by hand", + ) + except Exception: # noqa: BLE001 — the trail is best-effort + log.debug("[project_board] %s split-task comment failed", fid, exc_info=True) + # ── operator cancel during a drive (#211) ──────────────────────────────── @staticmethod def _cancelled(store, fid: str) -> bool: diff --git a/protoagent.plugin.yaml b/protoagent.plugin.yaml index b12b74f..59556be 100644 --- a/protoagent.plugin.yaml +++ b/protoagent.plugin.yaml @@ -210,9 +210,10 @@ config: health_sweep_interval_s: 300 # periodic self-heal: reclaim slots from dead drives # A repeated coder timeout is a SIZE signal, not a capability one: it carries no diff and # no CI output, so a retry gets a near-identical prompt and a tier climb spends a stronger - # model on a card that was never model-limited. After this many timeouts on one card, the - # loop files a task asking the board's own agent to split it (once per card) and parks the - # card for it — no further tier climb or rebuild. 0 = off. + # model on a card that was never model-limited. After this many timeouts on FRESH builds of + # one card (never a pre-first-token or fix-round timeout; the count clears when a build + # reaches review), the loop parks the card and files a task asking the board's own agent to + # split it (once per card) — no further tier climb or rebuild. Unblocking resets it. 0 = off. decompose_after_timeouts: 2 # (in_progress with no live drive → ready) + reap orphaned # feat- worktrees (feature gone/done). 0 disables it. @@ -261,7 +262,7 @@ settings: - { key: loop_enabled, label: "Run orchestration loop", type: bool, group: "Project Board", tab: automation, restart: true, description: "Start the background puller after the member restarts. Off keeps the board and tools available without dispatching work." } - { key: decompose_after_timeouts, label: "Ask to split after N timeouts", type: number, minimum: 0, maximum: 10, group: "Project Board", tab: automation, restart: true, - description: "A repeated coder timeout means the card is too WIDE, not that the model is too weak: it produces no diff and no CI output, so a retry re-sends the same prompt and a tier climb spends a stronger model on the wrong problem. After this many timeouts on one card, file a task asking this agent to split it into buildable slices (once per card) and park the card for it, with no further tier climb or rebuild. 0 turns the ask off and the card simply blocks." } + description: "A repeated coder timeout means the card is too WIDE, not that the model is too weak: it produces no diff and no CI output, so a retry re-sends the same prompt and a tier climb spends a stronger model on the wrong problem. After this many timeouts on fresh builds of one card (a pre-first-token timeout, or one on a fix round, never counts), park the card and file a task asking this agent to split it into buildable slices (once per card), with no further tier climb or rebuild. Unblocking a parked card resets its count. 0 turns the ask off and the card simply blocks." } - { key: coder, label: "Coder delegate", type: string, group: "Project Board", tab: automation, description: "Name of the acp delegate (Settings ▸ Delegates) the loop dispatches builds to. No default. Applies live on save: a paused loop resumes on its next check." } - { key: br_autofetch, label: "Fetch br automatically", type: bool, group: "Project Board", tab: automation, diff --git a/store.py b/store.py index 40de347..24d39d6 100644 --- a/store.py +++ b/store.py @@ -200,7 +200,8 @@ def _warn_blocking_on_event_loop(op: str) -> None: LABEL_BLOCKED = "blocked" # WHY a feature is blocked, as the failure classifier's category (failures.classify): # `blocked-class:transient` / `-rate-limit` / `-merge-conflict` / `-auth` / `-terminal` / -# `-provider-unavailable` (#420), plus the loop's own `-dispatch-infra` (#339). +# `-provider-unavailable` (#420), plus the loop's own `-dispatch-infra` (#339) and +# `-too-wide` (#378: parked for a split after repeated timeouts). # A single REPLACED label (the `gens:` pattern) so the projection can tell a block that # will clear itself from one that needs a human — WITHOUT a `br show` per card to read # the `blocked:` comment. Underscores in a category are hyphenated: beads' label @@ -1704,31 +1705,55 @@ def request_decomposition(self, fid: str, *, timeouts: int) -> dict | None: Rather than decompose inline (the ``decompose`` subagent is a pure proposer driven by a skill, and an LLM call inside the drive loop can itself time out), file a - TASK assigned to the board's own agent, and promote it to ``ready``. The existing - self-dispatch path (#311) picks it up, the agent decomposes with the board tools it + TASK assigned to the board's own agent. The existing self-dispatch path (#311) + picks it up once it is ``ready``, the agent decomposes with the board tools it already has, and the Ready gate enforces that the slices are actually well-formed — which is the part an unattended splitter gets wrong. - The promotion is not optional. ``create_feature`` files every bead in ``backlog``, - and the puller only pulls ``ready``: the ask as first shipped sat in the backlog - until someone noticed it, so the loop that "asked its own agent" had in fact asked - nobody. It goes through the ordinary ``mark_ready`` gate; a refusal leaves the task - filed in backlog for a human rather than un-asking. - - Returns the new task, or None when the ask was already made (idempotent) or the - card is itself a decomposition task (never recurse). Never raises: failing to ask - must not change how the timeout itself is handled.""" + The task is filed in ``backlog``, where the puller cannot see it, and the CALLER + releases it with ``mark_ready``. That split is deliberate: the loop parks the card + between the two, so the park can name the task and the agent — whose job ends in + cancelling this card — can never find it still in flight. + + Its steps are ordered so each one passes the gates the next depends on: slices in + backlog first (the card still claims its files, so a READY slice naming one is + refused by the shared-file gate), dependents re-pointed onto the slices next + (cancelling the card drops its edges, releasing every dependent at once), then the + cancel, and only then the slices marked ready. + + Idempotent on the task itself, not only on the ``decompose-asked`` label: a label + write lost after the create must not file a duplicate at the next park. Returns + the task still open for this card — new, or filed earlier and not yet closed — or + None when its split was already requested and has closed, or the card is itself a + decomposition task (never recurse). Never raises: failing to ask must not change + how the timeout itself is handled.""" try: f = self.get_feature(fid) - if not f: + if not f or f.get("issue_type") == LABEL_TASK: return None labels = list(f.get("labels") or []) - if self.LABEL_DECOMPOSE_ASKED in labels or f.get("issue_type") == LABEL_TASK: - return None + rows = self.list_features() + filed = [ + t + for t in rows + if t.get("issue_type") == LABEL_TASK and str(t.get("title") or "").startswith(f"Decompose {fid} ") + ] + if filed: + if self.LABEL_DECOMPOSE_ASKED not in labels: # heal a label write the create outlived + self._run("update", fid, "--add-label", self.LABEL_DECOMPOSE_ASKED) + return next((t for t in filed if t.get("board_state") not in _TERMINAL_STATES), None) + if self.LABEL_DECOMPOSE_ASKED in labels: + return None # asked before; that task is archived or gone files = list(f.get("files_to_modify") or []) + dependents = sorted( + d["id"] + for d in rows + if fid in (d.get("depends_on") or []) and d.get("board_state") not in _TERMINAL_STATES + ) spec = ( f"Card {fid} ({f.get('title') or 'untitled'}) has timed out {timeouts}x under the " - f"coder and is too wide to build in one dispatch. Split it into slices and retire it.\n\n" + f"coder and is too wide to build in one dispatch. The loop has PARKED it (blocked): " + f"split it into slices and retire it.\n\n" f"A timeout is not a capability failure — it carries no diff and no CI output, so " f"retrying or escalating the tier spends the clock again on a card that was never " f"model-limited. It is a SIZE signal.\n\n" @@ -1736,21 +1761,33 @@ def request_decomposition(self, fid: str, *, timeouts: int) -> dict | None: f"ORIGINAL ACCEPTANCE CRITERIA\n{f.get('acceptance_criteria') or '(none)'}\n\n" f"ORIGINAL files_to_modify ({len(files)})\n" + ("\n".join(f"- {p}" for p in files) or "- (none)") - + "\n\nHOW TO SPLIT (the board's own gates will refuse a sloppy decomposition, so " - "satisfy them up front):\n" - "- Each slice must sit at or under the breadth cap for its difficulty, and must be " + + f"\n\nCARDS THAT DEPEND ON {fid}\n" + + ("\n".join(f"- {d}" for d in dependents) or "- (none)") + + "\n\nDO IT IN THIS ORDER — each step is what lets the next one pass the board's gates:\n\n" + "1. Create the slices with `board_create_feature` and LEAVE THEM IN BACKLOG. " + f"{fid} still claims its files until it is cancelled, so the shared-file gate " + "refuses a slice marked ready while it names one of them.\n" + " - Each slice sits at or under the breadth cap for its difficulty, and is " "independently buildable and gate-passing on its own — not a fragment that only " "compiles once its siblings land.\n" - "- A slice naming a file that does not exist YET must mark it `(new)`, or the Ready " - "gate refuses it as a phantom path.\n" - "- Slices editing the SAME file need a depends_on edge so one waits for the other. " - "The edge counts in either direction, but only between the two cards that actually " - "share the file: in a chain A->B->C, the A/C pair needs its OWN edge.\n" - "- Order them so the first slice is independently useful and the rest gate behind it.\n\n" - f"Then cancel {fid} as superseded, naming the slice ids in the reason." + " - A file that does not exist YET is marked `(new)`, or the Ready gate refuses " + "it as a phantom path.\n" + " - Slices editing the SAME file need a depends_on edge between them. The edge " + "counts in either direction, but only between the two cards that share the file: " + "in a chain A->B->C, the A/C pair needs its OWN edge.\n" + " - Order them so the first slice is independently useful and the rest gate behind it.\n" + f"2. Re-point every card that depends on {fid} (listed above; check again, the list " + "may have grown) onto the slice or slices that deliver what it needs: " + "`board_update_feature(feature_id=, depends_on=[])`. Cancelling " + f"{fid} drops its edges, so a dependent you skip is released before anything it " + "needs has landed.\n" + f"3. Cancel {fid} with `board_cancel_feature`, naming the slice ids in the reason.\n" + "4. Mark every slice ready with `board_mark_ready`. The first becomes buildable; the " + "rest wait behind their depends_on edges." ) criteria = ( f"- {fid} is cancelled with a reason naming the slices that replace it.\n" + f"- Every card that depended on {fid} now depends on the slice(s) it needs.\n" "- Every slice is `ready` (or dag_blocked behind a sibling), so no slice needs a " "second pass to become buildable.\n" "- Together the slices cover the original acceptance criteria, with nothing dropped.\n" @@ -1768,20 +1805,8 @@ def request_decomposition(self, fid: str, *, timeouts: int) -> dict | None: ) self._run("update", fid, "--add-label", self.LABEL_DECOMPOSE_ASKED) self.comment(fid, f"decompose requested after {timeouts} timeouts → {(task or {}).get('id', '?')}") - # After the once-per-card label, so a promotion failure can never re-arm the ask. - if (task or {}).get("id"): - try: - task = self.mark_ready(task["id"]) or task - except Exception: # noqa: BLE001 — filed is still asked; a human can promote it - log.warning( - "[project_board] %s decompose task %s filed but not promoted to ready — it waits " - "in backlog until someone marks it ready", - fid, - task["id"], - exc_info=True, - ) return task - except Exception: # noqa: BLE001 — the ask is best-effort; the block still happens + except Exception: # noqa: BLE001 — the ask is best-effort; the park still happens log.warning("[project_board] %s decompose request failed (ignored)", fid, exc_info=True) return None @@ -2410,8 +2435,14 @@ def clear_blocked(self, fid: str) -> dict: A card that never escalated carries no ``tier:`` label, so it still starts at its difficulty-selected tier on the next build; the ladder stays a model-capability record, and the infra incident adds nothing to it. A - model-reachable block (or an unclassified one) is untouched, exactly as before.""" - from .failures import PRE_MODEL_DISPATCH_CLASS + model-reachable block (or an unclassified one) is untouched, exactly as before. + + A card the loop PARKED as ``too-wide`` (#378) also gets its timeout count reset: + unblocking one is a deliberate retry — after raising ``coder_timeout_s``, or + narrowing the card by hand — and with the count still at the threshold its very + next timeout would park it again at once. (The loop's in-process copy of the count + is dropped separately, by the unblock verbs: ``loop.forget_timeout_count``.)""" + from .failures import PRE_MODEL_DISPATCH_CLASS, TOO_WIDE_CLASS f = self._require(fid) labels = f.get("labels") or [] @@ -2421,6 +2452,11 @@ def clear_blocked(self, fid: str) -> dict: # Drop only the stale infra class — NOT the tier labels, which predate the # incident and record genuine model-capability escalation (#339). args += ["--remove-label", f"{LABEL_BLOCKED_CLASS_PREFIX}{cls}"] + elif cls == TOO_WIDE_CLASS: + args += ["--remove-label", f"{LABEL_BLOCKED_CLASS_PREFIX}{cls}"] + for label in labels: + if label.startswith(f"{LABEL_BUDGET_PREFIX}timeout:"): + args += ["--remove-label", label] self._run(*args) return self.get_feature(fid) diff --git a/tests/test_integration.py b/tests/test_integration.py index 503ce27..a363be8 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -685,9 +685,12 @@ def test_request_decomposition_files_a_real_task_and_marks_the_card(board): assert fid in got["title"] assert "build the whole subsystem" in got["spec"] # the original intent rode along - # The original is marked, so the ask cannot re-fire on the next timeout. + # The original is marked, so the ask cannot re-fire on the next timeout: a repeat hands + # back the SAME open task (#378 — idempotent on the task, so a re-park can name it) and + # files no second one. assert "decompose-asked" in (board.get_feature(fid).get("labels") or []) - assert board.request_decomposition(fid, timeouts=3) is None + assert board.request_decomposition(fid, timeouts=3)["id"] == task["id"] + assert [t["id"] for t in board.list_features() if t.get("issue_type") == "task"] == [task["id"]] # ── structured setup-gap actions: the register() wiring, end to end ─────────────────── diff --git a/tests/test_loop.py b/tests/test_loop.py index 78fae08..3464a89 100644 --- a/tests/test_loop.py +++ b/tests/test_loop.py @@ -2468,6 +2468,9 @@ async def test_the_timeout_counter_survives_a_restart(monkeypatch): that reset to zero on every reload would never reach the threshold.""" async def _dispatch(c, wt, prompt, *, timeout=None, env_passthrough=()): + # the model worked before the clock ran out — a pre-first-token timeout is infra + # and never counts (#378 review) + coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {"path": "a.py"}}) raise worktree.CoderTimeout("coder timed out after 1800s") monkeypatch.setattr("project_board.loop.asyncio.sleep", _no_sleep) diff --git a/tests/test_timeout_decompose_378.py b/tests/test_timeout_decompose_378.py index ada03ea..19ca9db 100644 --- a/tests/test_timeout_decompose_378.py +++ b/tests/test_timeout_decompose_378.py @@ -12,6 +12,11 @@ 3. The card was then 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 two more full timeouts, racing its own decomposition. + +And from the review of the first cut: only a FRESH build's timeout that reached the model +is a size signal (an infra timeout, or a fix round on a card that already built, is not); +the park must say what it actually filed; an operator unblock must give a real retry; and +the split's own steps must pass the gates in the order they are written. """ from __future__ import annotations @@ -29,23 +34,26 @@ from test_loop import _BlockedStore, _EscalatingStore, _blocked, _no_sleep _TIMEOUT = "coder timed out after 1800s" +requires_br = pytest.mark.skipif(shutil.which(store_mod.BR) is None, reason="real `br` (beads) CLI not on PATH") class _SplitStore(_EscalatingStore): - """Climbs through `tiers`, keeps budget labels durably, and answers the decompose ask - with `answer` (a task dict, or None for "already asked / could not file").""" + """Climbs through `tiers`, keeps budget labels durably, answers the decompose ask with + `answer` (a task, or {} for "nothing filed"), and records the task's release.""" def __init__(self, tiers=(), *, answer=None): super().__init__(list(tiers)) self.asked: list[tuple[str, int]] = [] self.budgets: dict[str, int] = {} - self._answer = {"id": "bd-split"} if answer is None else answer + self._answer = {"id": "bd-split", "board_state": "backlog"} if answer is None else answer def record_budget(self, fid, kind, n): self.budgets[f"{fid}:{kind}"] = n def clear_budgets(self, fid, kinds=None): - pass + for kind in kinds or [k.split(":", 1)[1] for k in self.budgets if k.startswith(f"{fid}:")]: + self.budgets.pop(f"{fid}:{kind}", None) + self.calls.append(("clear_budgets", fid, tuple(kinds or ()))) def get_feature(self, fid): labels = [f"budget:{k.split(':', 1)[1]}:{v}" for k, v in self.budgets.items() if k.startswith(f"{fid}:")] @@ -56,14 +64,18 @@ def request_decomposition(self, fid, *, timeouts): self.calls.append(("request_decomposition", fid, timeouts)) return self._answer or None + def mark_ready(self, fid): + self.calls.append(("mark_ready", fid)) + return {"id": fid, "board_state": "ready"} + def _blocks(store): return [c for c in store.calls if c[0] == "flag_blocked"] -def _timing_out_board(monkeypatch, store, cfg, *, start_tier="smart"): - """A drive whose every dispatch WORKS (a tool call reaches the ring buffer — this is a - size signal, not the pre-first-token infra timeout #339 blocks) and then times out.""" +def _timing_out_board(monkeypatch, store, cfg, *, start_tier="smart", model_worked=True): + """A drive whose every dispatch times out — after real model work (a tool call reaches + the ring buffer) unless `model_worked=False`, the pre-first-token infra timeout.""" monkeypatch.setattr("project_board.loop.get_store", lambda **_kw: store) monkeypatch.setattr("project_board.loop.asyncio.sleep", _no_sleep) dispatched: list[str] = [] @@ -76,7 +88,8 @@ async def _noop(*_a, **_kw): async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): dispatched.append(coder) - coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {"path": "a.py"}}) + if model_worked: + coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {"path": "a"}}) raise worktree.CoderTimeout(_TIMEOUT) monkeypatch.setattr(worktree, "create_worktree", _create) @@ -89,34 +102,39 @@ async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): return loop, dispatched +_CARD = {"id": "bd-1", "title": "Wide card", "spec": "s"} + + +# ── the park: when, and in what order ──────────────────────────────────────────────── + + async def test_the_second_timeout_asks_for_a_split_instead_of_climbing_again(monkeypatch): - """Gap 2. The first timeout climbs, carrying #146's timeout context — one can be an - unlucky gate run. The second is the size signal: ask, and stop. It must not spend the - top rung on a card the loop has just concluded is too wide.""" + """The first timeout climbs, carrying #146's timeout context — one can be an unlucky + gate run. The second is the size signal: park and ask. It must not spend the top rung + on a card the loop has just concluded is too wide. The park names the task, and lands + between filing it (backlog, invisible to the puller) and releasing it (ready): the + agent that picks it up cancels this card, which must never be found in flight.""" store = _SplitStore(tiers=["reasoning", "opus"]) loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b", "opus": "c"}}) - await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) + await loop._drive(_CARD) assert dispatched == ["a", "b"], f"the threshold timeout climbed instead of asking: {dispatched}" - assert store.asked == [("bd-1", 2)] - assert len(store.escalated) == 1 # the first timeout's climb, and only that - blocked = _blocks(store) - assert len(blocked) == 1 and "timed out 2x" in blocked[0][2] - # Parked BEFORE the ask: the ask files its task `ready`, and the agent that picks it up - # cancels this card — it must never find the card still in flight. + assert store.asked == [("bd-1", 2)] and len(store.escalated) == 1 + (_, _, reason, category) = _blocks(store)[-1] + assert category == "too-wide" and "timed out 2x" in reason and "bd-split" in reason names = store.names() - assert names.index("flag_blocked") < names.index("request_decomposition") + assert names.index("request_decomposition") < names.index("flag_blocked") < names.index("mark_ready") + assert ("mark_ready", "bd-split") in store.calls async def test_a_card_parked_for_its_split_is_not_rebuilt_by_the_blocked_sweep(monkeypatch): - """Gap 3, end to end: the block the drive writes, read back by the sweep. The card the - loop just asked to split must stay parked — the operator hears about it once — instead - of being requeued and rebuilt whole while its decomposition is under way.""" + """End to end: the block the drive writes, read back by the sweep. The card the loop + just asked to split stays parked — the operator hears about it once — instead of being + requeued and rebuilt whole while its decomposition is under way.""" store = _SplitStore() store.budgets["bd-1:timeout"] = 1 # a one-coder board: the first timeout already blocked - loop, dispatched = _timing_out_board(monkeypatch, store, {"coder": "proto"}) - await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) - assert store.asked == [("bd-1", 2)] + loop, _ = _timing_out_board(monkeypatch, store, {"coder": "proto"}) + await loop._drive(_CARD) (_, fid, reason, category) = _blocks(store)[-1] lane = _BlockedStore([_blocked(fid, category.replace("_", "-"), reason=reason, title="Wide card")]) @@ -126,92 +144,331 @@ async def test_a_card_parked_for_its_split_is_not_rebuilt_by_the_blocked_sweep(m await sweep._recover_blocked(lane) assert lane.requeued == [] and lane.cleared == [], f"the sweep rebuilt a card parked for its split ({category})" - assert len(told) == 1 and "timed out 2x" in told[0] and "Wide card" in told[0] + assert len(told) == 1 and "bd-split" in told[0] and "Wide card" in told[0] assert category.replace("_", "-") not in loop_mod._SELF_HEALING_BLOCKS -async def test_a_card_past_the_threshold_is_parked_even_when_no_new_ask_is_filed(monkeypatch): - """The park follows from the card, not from the filing. Here the store files nothing — - the card was asked once already and an operator requeued it since, or the store could - not file. It timed out past the threshold again: it is exactly as wide, so it is parked - for the operator, not climbed onto a pricier rung or handed back to the sweep.""" +async def test_a_re_park_that_files_nothing_says_so_and_what_to_do(monkeypatch): + """Nothing was filed this time — the card was asked once and requeued by hand since, or + the store refused. It is exactly as wide, so it is still parked, but the reason (and + so the operator's alert) must not claim a split is on its way.""" store = _SplitStore(tiers=["reasoning"], answer={}) - store.budgets["bd-1:timeout"] = 3 # well past the threshold + store.budgets["bd-1:timeout"] = 3 loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b"}}) - await loop._drive({"id": "bd-1", "title": "Wide card", "spec": "s"}) + await loop._drive(_CARD) assert dispatched == ["a"] and store.escalated == [], dispatched - assert store.asked == [("bd-1", 4)] # still asked — the store decides whether anything is filed - blocked = _blocks(store) - assert len(blocked) == 1 and blocked[0][3] not in ("transient", "rate_limit", "merge_conflict") + (_, _, reason, category) = _blocks(store)[-1] + assert category == "too-wide" + assert "NO split task was filed" in reason and "split it by hand" in reason and "coder_timeout_s" in reason + assert "mark_ready" not in store.names() -# ── gap 1: the ask has to reach the puller ──────────────────────────────────────────── +# ── what counts ────────────────────────────────────────────────────────────────────── -@pytest.mark.skipif(shutil.which(store_mod.BR) is None, reason="real `br` (beads) CLI not on PATH") -def test_the_decompose_ask_is_filed_ready_so_the_agent_actually_gets_it(tmp_path): - """Against REAL `br`: the puller's queue is `br ready --label ready`. A task filed in - `backlog` is never dispatched, whatever its assignee — so the ask must come out of - `request_decomposition` already promoted, through the ordinary Ready gate.""" - board = BeadsBoard(repo=str(tmp_path), actor="test") - wide = board.create_feature( - "Wide card that keeps timing out", - spec="build the whole subsystem", - acceptance_criteria="- WHEN done THE SYSTEM SHALL work", - files_to_modify=["a.py (new)", "b.py (new)"], - ) - task = board.request_decomposition(wide["id"], timeouts=2) +async def test_a_pre_first_token_timeout_is_never_counted(monkeypatch): + """A timeout with no model work is a wedged adapter (#339): infra, blocked for triage, + and NOT a size signal. It used to be counted all the same, so after the operator fixed + the infra, the card's first genuine timeout parked it and asked for a split.""" + store = _SplitStore() + loop, _ = _timing_out_board(monkeypatch, store, {"coder": "proto"}, model_worked=False) + await loop._drive(_CARD) + assert _blocks(store)[-1][3] == "dispatch-infra" + assert "bd-1:timeout" not in store.budgets and "bd-1" not in loop._timeout_attempts + + loop2, _ = _timing_out_board(monkeypatch, store, {"coder": "proto"}) # infra fixed; one real timeout + await loop2._drive(_CARD) + assert store.asked == [] and store.budgets["bd-1:timeout"] == 1 + assert _blocks(store)[-1][3] == "transient" # the ordinary first-timeout block + + +async def test_a_fix_round_on_a_card_with_an_open_pr_is_never_parked(monkeypatch): + """A card whose PR is open already BUILT in one dispatch — it is not too wide. A fix + round on it (a CI bounce) that times out takes the ordinary path. Parked, it would be + asked to split and cancel, and the cancel would close its open PR.""" + store = _SplitStore(tiers=["reasoning"]) + store.budgets["bd-1:timeout"] = 5 # well past the threshold + loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b"}}) + loop._ci_feedback["bd-1"] = "CI failed: test_x" + await loop._drive({**_CARD, "pr_url": "https://example/pr/1"}) + + assert store.asked == [] and all(c[3] != "too-wide" for c in _blocks(store)) + assert store.budgets["bd-1:timeout"] == 5, "a fix-round timeout is not a size signal" + assert store.escalated, "the ordinary timeout path climbs" + + +async def test_a_keep_worktree_fix_round_that_times_out_is_not_counted(monkeypatch): + """Same rule inside one drive: the build returned a diff, the goal check found a gap, + and the fix round on the KEPT worktree timed out. The card built in one dispatch.""" + store = _SplitStore() + store.budgets["bd-1:timeout"] = 1 # one short of the threshold + loop, dispatched = _timing_out_board(monkeypatch, store, {"coder": "proto"}) + calls = {"n": 0} - assert task is not None - assert board.get_feature(task["id"])["board_state"] == "ready" - assert task["id"] in [f["id"] for f in board.ready_queue()], "the self-dispatch path never sees the ask" + async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): + calls["n"] += 1 + dispatched.append(coder) + coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {}}) + if calls["n"] == 1: + return "built it" + raise worktree.CoderTimeout(_TIMEOUT) # the fix round on the kept worktree + + gaps = iter(["missing tests"]) + + async def _gap(feature, wt, base, reply=""): + return next(gaps, None) + + monkeypatch.setattr(worktree, "dispatch_coder", _dispatch) + monkeypatch.setattr(loop, "_verify_goal", _gap) + loop.goal_verify = True + await loop._drive(_CARD) + + assert store.asked == [] and store.budgets["bd-1:timeout"] == 1 + assert all(c[3] != "too-wide" for c in _blocks(store)) + + +async def test_a_build_that_reaches_review_resets_the_count(monkeypatch): + """A card that built once is demonstrably not too wide: the timeout count is cleared + when a build reaches review, so an old timeout can never combine with a later one.""" + store = _SplitStore(tiers=["reasoning"]) + loop, dispatched = _timing_out_board(monkeypatch, store, {"coders": {"smart": "a", "reasoning": "b"}}) + calls = {"n": 0} + + async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): + calls["n"] += 1 + coder_seam.progress_tool("bd-1", 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {}}) + if calls["n"] == 1: + raise worktree.CoderTimeout(_TIMEOUT) + return "built it" + + async def _open_pr(wt, branch, *, base, title, body, promote_draft=True): + return "https://example/pr/1" + + monkeypatch.setattr(worktree, "dispatch_coder", _dispatch) + monkeypatch.setattr(worktree, "open_pr", _open_pr) + await loop._drive(_CARD) + + assert ("open_review", "bd-1", "https://example/pr/1") in store.calls + assert "bd-1:timeout" not in store.budgets and loop._timeout_attempts.get("bd-1") == 0 + + +async def test_an_operator_unblock_gives_a_parked_card_a_real_retry(monkeypatch): + """The operator raised `coder_timeout_s` and unblocked the parked card. The store resets + the persisted count; the running loop's cached count must go too, or the retry's first + timeout re-parks the card at once (#259: the cache wins over the labels).""" + store = _SplitStore() + store.budgets["bd-1:timeout"] = 1 + loop, _ = _timing_out_board(monkeypatch, store, {"coder": "proto"}) + await loop._drive(_CARD) + assert _blocks(store)[-1][3] == "too-wide" and loop._timeout_attempts["bd-1"] == 2 + store.budgets.pop("bd-1:timeout") # clear_blocked on a too-wide park (store half, pinned below) + monkeypatch.setattr(loop_mod._common, "live_loop", lambda: loop) + assert loop_mod.forget_timeout_count("bd-1") is True + await loop._drive(_CARD) # the retry times out once more -def test_a_refused_promotion_still_counts_as_asked(make_board, monkeypatch): - """If the Ready gate refuses the task, it stays filed in backlog for a human — the card - was still asked (its once-per-card label is on), and the promotion failure is logged, - not raised: the ask never raises into the drive.""" + assert _blocks(store)[-1][3] == "transient", "the retry re-parked on its first timeout" + assert store.asked == [("bd-1", 2)] + + +def test_clear_blocked_resets_the_count_only_for_a_too_wide_park(make_board, monkeypatch): + """The store half. Only the park's own class resets the count: the blocked sweep also + clears blocks (its self-heal), and resetting there would stop a one-coder board's + timeouts from ever adding up.""" calls: list[tuple] = [] + b = make_board(lambda *args, want_json=False: calls.append(args) or {}) + labels = {"too-wide": ["blocked", "blocked-class:too-wide", "budget:timeout:2", "budget:goal-fix:1"]} + labels["transient"] = ["blocked", "blocked-class:transient", "budget:timeout:1"] + for cls, row in labels.items(): + calls.clear() + monkeypatch.setattr(b, "get_feature", lambda fid, row=row: {"id": fid, "labels": row, "board_state": "blocked"}) + b.clear_blocked("bd-9") + update = next(c for c in calls if c[0] == "update") + if cls == "too-wide": + assert "budget:timeout:2" in update and "blocked-class:too-wide" in update + assert "budget:goal-fix:1" not in update + else: + assert not any(str(a).startswith("budget:") for a in update) + + +# ── the ask itself ─────────────────────────────────────────────────────────────────── - def _br(*args, want_json=False): - calls.append(args) - return {} - b = make_board(_br) +def _asking_board(make_board, monkeypatch, *, rows, card_labels=()): + calls: list[tuple] = [] + b = make_board(lambda *args, want_json=False: calls.append(args) or ([] if want_json else "")) + card = {"id": "bd-8", "title": "Wide", "labels": list(card_labels), "issue_type": "feature", "spec": "s"} + monkeypatch.setattr(b, "get_feature", lambda fid: card) + monkeypatch.setattr(b, "list_features", lambda *a, **k: rows) + monkeypatch.setattr(b, "comment", lambda fid, text: None) + created: list[dict] = [] monkeypatch.setattr( - b, "get_feature", lambda fid: {"id": fid, "title": "Wide", "labels": [], "issue_type": "feature"} + b, "create_feature", lambda title, **kw: created.append({"title": title, **kw}) or {"id": "bd-9"} ) - monkeypatch.setattr(b, "create_feature", lambda title, **kw: {"id": "bd-9"}) - monkeypatch.setattr(b, "comment", lambda fid, text: None) + monkeypatch.setattr(b, "mark_ready", lambda fid: pytest.fail("the ask must not release its own task")) + return b, calls, created - def _refuse(fid): - raise BoardError(f"Ready gate: {fid} refused") - monkeypatch.setattr(b, "mark_ready", _refuse) +def test_the_ask_files_in_backlog_with_the_dependents_and_the_safe_order(make_board, monkeypatch): + """Filed, not released: the caller parks the card first. The spec names the card's + dependents and orders the steps so each passes the gates the next relies on.""" + dependent = {"id": "bd-d", "issue_type": "feature", "depends_on": ["bd-8"], "board_state": "ready", "title": "D"} + b, calls, created = _asking_board(make_board, monkeypatch, rows=[dependent]) assert b.request_decomposition("bd-8", timeouts=2) == {"id": "bd-9"} + + spec = created[0]["spec"] + assert "- bd-d" in spec + steps = ["board_create_feature", "board_update_feature", "board_cancel_feature", "board_mark_ready"] + assert [spec.index(s) for s in steps] == sorted(spec.index(s) for s in steps) + assert "LEAVE THEM IN BACKLOG" in spec assert ("update", "bd-8", "--add-label", "decompose-asked") in calls -def test_the_ask_is_promoted_after_the_once_per_card_label(make_board, monkeypatch): - """Order matters: the label is what stops a second ask. Promoting first would let a - promotion crash leave a filed task with no label, and the next timeout would file a - duplicate.""" - order: list[str] = [] +def test_the_ask_is_idempotent_on_the_task_not_only_the_label(make_board, monkeypatch): + """The label is written AFTER the create. If that write is lost, the next park must find + the task it already filed — not file a duplicate and orphan the first — and heal the + label. An open task is handed back so the re-park can name and release it.""" + filed = {"id": "bd-9", "issue_type": "task", "title": "Decompose bd-8 — timed out 2x, too wide to build"} + b, calls, created = _asking_board(make_board, monkeypatch, rows=[{**filed, "board_state": "backlog"}]) + assert b.request_decomposition("bd-8", timeouts=3)["id"] == "bd-9" + assert created == [] and ("update", "bd-8", "--add-label", "decompose-asked") in calls - def _br(*args, want_json=False): - if "decompose-asked" in args: - order.append("label") - return {} + b, calls, created = _asking_board(make_board, monkeypatch, rows=[{**filed, "board_state": "done"}]) + assert b.request_decomposition("bd-8", timeouts=3) is None # its split already ran + assert created == [] - b = make_board(_br) - monkeypatch.setattr( - b, "get_feature", lambda fid: {"id": fid, "title": "Wide", "labels": [], "issue_type": "feature"} + +# ── against real `br` ──────────────────────────────────────────────────────────────── + + +@requires_br +async def test_a_real_card_is_parked_naming_its_split_and_the_split_is_dispatchable(tmp_path, monkeypatch): + """The drive against a REAL board: the card is parked `too-wide` with the task named, + and the task is `ready` — in the puller's queue — while the card is not.""" + board = BeadsBoard(repo=str(tmp_path), actor="test") + wide = board.create_feature( + "Wide card", spec="s", acceptance_criteria="- WHEN x THE SYSTEM SHALL y", files_to_modify=["a.py (new)"] ) - monkeypatch.setattr(b, "create_feature", lambda title, **kw: {"id": "bd-9"}) - monkeypatch.setattr(b, "comment", lambda fid, text: None) - monkeypatch.setattr(b, "mark_ready", lambda fid: order.append("ready") or {"id": fid, "board_state": "ready"}) - out = b.request_decomposition("bd-8", timeouts=2) + fid = wide["id"] + board.mark_ready(fid) + board.record_budget(fid, "timeout", 1) + claimed = board.claim(fid, assignee="proto") + monkeypatch.setattr("project_board.loop.get_store", lambda **_kw: board) + monkeypatch.setattr("project_board.loop.asyncio.sleep", _no_sleep) - assert order == ["label", "ready"] - assert out == {"id": "bd-9", "board_state": "ready"} + async def _create(repo, base, f, root, title="", **_kw): + return ("/wt/feat-" + f, "feat/" + f) + + async def _noop(*_a, **_kw): + return None + + async def _dispatch(coder, wt, prompt, *, timeout=None, env_passthrough=()): + coder_seam.progress_tool(fid, 1, {"phase": "start", "name": "Edit", "id": "t1", "input": {}}) + raise worktree.CoderTimeout(_TIMEOUT) + + monkeypatch.setattr(worktree, "create_worktree", _create) + monkeypatch.setattr(worktree, "dispatch_coder", _dispatch) + monkeypatch.setattr(worktree, "remove_worktree", _noop) + loop = BoardLoop({"coder": "proto", "kg_lessons": False}) + monkeypatch.setattr(loop, "_resolve_delegate", lambda name, expect: object()) + await loop._drive(claimed) + + card = board.get_feature(fid) + assert card["board_state"] == "blocked" and card["blocked_class"] == "too-wide" + task = next(t for t in board.list_features() if t.get("issue_type") == "task") + assert task["id"] in card["blocked_reason"] + queue = board.ready_queue() + assert task["id"] in [f["id"] for f in queue] and fid not in [f["id"] for f in queue] + + # …and the puller hands it to the board's OWN agent (#311), assignee intact. + async def _invoke(prompt, session_id): + return "split" + + started: list[str] = [] + + async def _drive_self_task(feature, invoke, session_id): + started.append(session_id) + + monkeypatch.setattr(coder_seam, "resolve_self_invoke", lambda: _invoke) + monkeypatch.setattr(coder_seam, "host_invoke_busy", lambda: False) + monkeypatch.setattr(loop, "_drive_self_task", _drive_self_task) + assert await loop._dispatch_task(board, next(f for f in queue if f["id"] == task["id"])) == "self" + for t in list(loop._drives): + await t + assert started == [f"board-self-{task['id']}"] + assert board.get_feature(task["id"])["assignee"] == "agent" + + +@requires_br +def test_the_split_steps_pass_the_gates_end_to_end_in_the_order_written(tmp_path): + """Executes the filed task's steps against a REAL board, as the agent would, in its + order: slices in backlog → dependents re-pointed → original cancelled → slices ready. + Every gate passes; the dependent is never released early; it is released once the + slices land. And the two ways the first cut's order broke are pinned alongside.""" + board = BeadsBoard(repo=str(tmp_path), actor="test") + ac = "- WHEN x THE SYSTEM SHALL y" + wide = board.create_feature("Wide", spec="s", acceptance_criteria=ac, files_to_modify=["a.py (new)", "b.py (new)"]) + w = wide["id"] + board.mark_ready(w) + dep = board.create_feature("Dependent", spec="s", acceptance_criteria=ac, files_to_modify=["c.py (new)"]) + board.add_dependency(dep["id"], w) + board.mark_ready(dep["id"]) + board.flag_blocked(w, "too wide — parked", category="too-wide") + task = board.request_decomposition(w, timeouts=2) + assert f"- {dep['id']}" in task["spec"] + + # 1. slices, left in backlog + s1 = board.create_feature("Slice 1", spec="s", acceptance_criteria=ac, files_to_modify=["a.py (new)"])["id"] + s2 = board.create_feature("Slice 2", spec="s", acceptance_criteria=ac, files_to_modify=["b.py (new)"])["id"] + with pytest.raises(BoardError, match="Shared-file gate"): # why step 4 waits for step 3 + board.mark_ready(s1) + # 2. re-point the dependent onto the slices it needs + board.update_feature(dep["id"], depends_on=[s1, s2]) + # 3. cancel the original + board.cancel_feature(w, f"superseded by {s1}, {s2}") + assert dep["id"] not in [f["id"] for f in board.ready_queue()], "the cancel released the dependent early" + # 4. slices ready + board.mark_ready(s1) + board.mark_ready(s2) + + queue = [f["id"] for f in board.ready_queue()] + assert s1 in queue and s2 in queue and dep["id"] not in queue + for s in (s1, s2): + board.claim(s, assignee="proto") + board.mark_done(s, reason="shipped") + assert dep["id"] in [f["id"] for f in board.ready_queue()] # released once the slices landed + + +@requires_br +def test_a_lost_label_write_cannot_file_a_duplicate_split(tmp_path): + """Real `br`: the task exists but the card's `decompose-asked` label does not — the + write after the create was lost. The next ask returns the SAME task and heals the + label; there is still exactly one decompose task.""" + board = BeadsBoard(repo=str(tmp_path), actor="test") + wide = board.create_feature( + "Wide", spec="s", acceptance_criteria="- WHEN x THE SYSTEM SHALL y", files_to_modify=["a.py (new)"] + ) + first = board.request_decomposition(wide["id"], timeouts=2) + board._run("update", wide["id"], "--remove-label", "decompose-asked") + + again = board.request_decomposition(wide["id"], timeouts=3) + assert again is not None and again["id"] == first["id"] + assert "decompose-asked" in board.get_feature(wide["id"])["labels"] + assert [t["id"] for t in board.list_features() if t.get("issue_type") == "task"] == [first["id"]] + + +@requires_br +def test_an_operator_unblock_resets_a_real_parked_cards_count(tmp_path): + board = BeadsBoard(repo=str(tmp_path), actor="test") + wide = board.create_feature( + "Wide", spec="s", acceptance_criteria="- WHEN x THE SYSTEM SHALL y", files_to_modify=["a.py (new)"] + ) + fid = wide["id"] + board.record_budget(fid, "timeout", 2) + board.flag_blocked(fid, "too wide — parked", category="too-wide") + board.clear_blocked(fid) + + labels = board.get_feature(fid)["labels"] + assert not any(label.startswith("budget:timeout:") for label in labels) + assert "blocked-class:too-wide" not in labels and "blocked" not in labels From ad07ce6312b91d95a00920f4c90935565431ff54 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 10 Sep 2026 16:09:01 -0700 Subject: [PATCH 4/4] docs: changelog fragment for #435 covers the review fixes Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av --- changelog.d/435.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/changelog.d/435.md b/changelog.d/435.md index ecbc85a..566caec 100644 --- a/changelog.d/435.md +++ b/changelog.d/435.md @@ -1,8 +1,11 @@ -A card that keeps timing out is now parked for its split instead of being rebuilt. The decompose -task a repeated timeout files used to land in backlog, where the loop never dispatches, so the -board's own agent was never actually asked. It is now filed `ready`. The ask also waited for the -model ladder to run out, so a card on a three-rung ladder timed out three times first. And it was -paired with a self-healing block, so the sweep requeued the oversized card and rebuilt it twice -more, racing its own split. On the timeout that reaches `decompose_after_timeouts`, the loop now -parks the card before climbing again, tells the operator once, and leaves it for the agent to -split and retire. +A card whose fresh builds keep timing out is now parked for its split instead of being rebuilt. +The decompose task a repeated timeout files used to land in backlog, where the loop never +dispatches, so the board's own agent was never actually asked. It is now released once the card +is parked under its own `too-wide` block class, with a reason that names the task. The ask also +waited for the model ladder to run out, so a card on a three-rung ladder timed out three times +first. And it was paired with a self-healing block, so the sweep requeued the oversized card and +rebuilt it twice more, racing its own split. Only a timeout on a fresh build that reached the +model now counts toward `decompose_after_timeouts`: never an infra timeout, never a fix round on +a card that already built. The count clears when a build reaches review, and unblocking a parked +card resets it. The split's steps now run in an order that passes the board's gates: create the +slices, re-point the card's dependents, cancel the card, then mark the slices ready.