Skip to content

feat(board): attach an externally opened PR to the card it belongs to (#402) - #437

Merged
mabry1985 merged 5 commits into
mainfrom
feat/attach-pr-402
Sep 10, 2026
Merged

mabry1985 merged 5 commits into
mainfrom
feat/attach-pr-402

Conversation

@mabry1985

@mabry1985 mabry1985 commented Sep 10, 2026

Copy link
Copy Markdown
Member

What was missing

The loop adopts a PR it didn't open in exactly one place: crash recovery (_reconcile_orphan) finds the PR whose head is the card's canonical branch and moves the card to in_review. A PR opened by a human had no edge.

The incident, on protoEngineer, 2026-09-07:

  • bd-ezs7's coder left a finished implementation uncommitted in its candidate worktree (A coder can complete with real edits and publish nothing — no commit, no branch, no PR, no failure recorded #400).
  • The operator promoted the work onto the card's own branch, feat/bd-ezs7-fix-a2a-make-poll-timeout-s-a-no-progres, and opened protoAgent#3369.
  • The card stayed terminal-blocked while #3369 went through CI and review, invisible to the board. Re-dispatching the card would have force-removed the worktree the work still lived in.
  • After the merge, the PM hand-closed the card with board_mark_done.

What this adds

board_attach_pr(feature_id, pr_url, reason) / POST /features/{fid}/attach-pr {pr_url, reason?, by?}. It is deliberately no wider than recovery's adoption.

Refused unless Why
The PR is OPEN and in the card's project repo A merged PR has nothing left to review, so the refusal points at board_mark_done.
Its head is the card's canonical branch feat/<id>-<slug> Every later edge keys on it: fix rounds resume origin/<branch>, and so do recovery and the reap. The refusal gives the exact branch to push to.
It isn't from a fork, and it targets the project's base Fix rounds push to this repo, and the rebase and merge edges work against that base.
The card is a coding feature that passed the Ready gate Judged on its lane underneath any block (ready or in_progress), so a blocked backlog card has not passed. Tasks, epics and milestones are refused.
The card has no other PR, unless that PR is CLOSED A closed PR can be replaced. An open one would be orphaned, a merged one means the card is done, and a PR under live review is never swapped.
The card already tracks this PR → no-op in review, refused anywhere else Re-attaching a blocked card's own PR would lift a block that may be the review gate asking for a human. Unblocking stays a deliberate board_unblock_feature.
There is no open dependency, and the loop isn't working the card Refused, not overridden, if there is a live drive, a claimed build or a running review gate.

Every refusal changes nothing (the tests assert this for each shape) and names what to do instead.

The write. It is one br update (store.attach_pr), run under the loop's claim lock.

  • It leaves the card where open_review does: in_review with the PR on external_ref.
  • It drops ready, the block and its class, and every verdict pinned to an earlier head: the review verdict and its sha pin, the reviewed-head stamp, and merged-verified.
  • It arms review-pending when the review gate is on.
  • The health sweep's moves take the same lock and re-read the card first, so a sweep can't requeue an attach it raced. If the sweep adopts the same PR first, the attach still arms the gate and records itself (already_attached: true).
  • The attached PR: audit comment is written last. If that write fails, the attach still stands and the result carries a warning.

The attached code has not been through the board's pre-PR checks (fixups, the local gate, acceptance tests). CI, the review gate, rebase and merged-state verification still apply. A draft attaches, and auto-merge holds it until it is marked ready.

Seams, CI and guards

  • worktree.pr_identity: a new gh seam, classified REAL. Its tests are in tests/test_attach_pr_gh_402.py, which needs no br. The test (real gh) CI job now runs that file against the pinned fixture ⚠️ CI FIXTURE — DO NOT MERGE, DO NOT CLOSE #370 (vars.PB_GH_FIXTURE_PR). The head check comes before the fork check, so the refusal test doesn't depend on where the fixture comes from.
  • store.attach_pr is REAL: every shape goes through real br.
  • Every gh read the attach makes converts WorktreeError/OSError into an actionable BoardError, so the tool and the route never raise.
  • test_every_tool_returns_boarderror_as_result_never_raises now covers async tools. It used to skip them, which is how feat(loop): merge-poll Done-edge fallback + parallel drives (max_concurrent>1) #6 slipped through.
  • The tools-doc guard only matched def board_*, so board_dispatch had shipped undocumented. It now covers async defs, and both tools are documented.
  • Pre-existing bug fixed: the loop read review_gate with bool(), so "false" read as on. It now uses knob_bool, like every other surface.

Tests

  • tests/test_attach_pr_402.py (real br, gh faked at the seam):
    • the incident end to end: a terminal-blocked card takes the operator's PR, and the real _reconcile_prs closes it as done when the PR merges;
    • ready and orphaned in-progress cards;
    • 12 refusal shapes;
    • replacing an earlier PR, which is only allowed once it's closed;
    • a live drive refuses; the same PR again is a no-op;
    • the attach waits for the claim lock;
    • the route (200/400) and the tool.
  • tests/test_attach_pr_review_402.py (real br), one test per review finding: a blocked backlog card and a blocked epic are refused; a blocked card's own PR is refused naming the block; a fix round is refused; a replacement drops merged-verified and the merged state is gated; the sweep's orphan pass and blocked self-heal don't undo an attach (both race tests); D2 (sweep adopts first → already_attached, review-pending armed, audit written); a gh timeout or missing checkout returns Error:; a failed audit comment doesn't report an error; review_gate: "false" reads as off.
  • tests/test_attach_pr_gh_402.py: the real-GitHub tier. I ran it locally against ⚠️ CI FIXTURE — DO NOT MERGE, DO NOT CLOSE #370: it passes.

Red-check:

  • With origin/main's sources, the attach module doesn't exist, and the widened docs guard names board_dispatch.
  • The 12 review tests fail on this PR's previous head 8c45cd7, each on its own assertion: the backlog card and the epic attach; the blocked card is unblocked; the stamp survives; the sweep undoes the attach; the D2 attach isn't armed; the gh failures raise; the audit failure errors; the loop reads "false" as on.
  • All pass with the fix.

The _TaskStore/_BlockedStore fakes in tests/test_loop.py now model the re-read the sweep does. Their get_feature reflects a claim, and _BlockedStore gained one.

Rebased onto main (#430, #432, #435, #428 and #429 merged).

Gates: ruff format --check ., ruff check . and pytest -q all pass after the rebase: 2041 passed, 16 skipped. The real-GitHub tier also passes against #370 (13 passed; the 2 write seams skip read-only).

Fixes #402

🤖 Generated with Claude Code

https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av

mabry1985 added a commit that referenced this pull request Sep 10, 2026
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
Review of the first cut found five defects. The hand-off was the only
edge that respected a move, and a move it did respect could still lose
work:

1. A FIRST build requeued mid-round published a PR the card never
   recorded. The next round ran resume=False: create_worktree wiped the
   tree and open_pr's --force-with-lease overwrote the pushed branch.
2. Every other build-end path still blocked over a move. A human hold
   placed mid-round plus a transient error re-dispatched the coder three
   times on the held card, re-blocked it `transient`, and the sweep then
   auto-unblocked it, so the hold was lost.
3. Only `cancelled` was checked before publishing. A held or done card
   still got a PR, and the done case left an open PR nothing tracked.
4. The trail comment was not best-effort. If it threw, the outer handler
   re-blocked the card with that error and overwrote a human's hold.
5. A failed read after the refusal meant "not moved": the original
   incident again.

A drive owns its card only while it is in_progress. `_moved_under_drive`
now fails toward not overwriting: an unreadable card is never assumed to
be the drive's own. A refused hand-off names the state it found, and that
is used. Otherwise the answer is `unknown`, and a card left in_progress
with no drive is the sweep's. Before retrying, publishing, handing off or
blocking (all block sites go through `_block_or_stand_aside`), the drive
re-reads the card. A drive that lost its card calls `_stand_aside`:

- it does not block, retry or send the card to review;
- it records a PR it opened on the card (new store.record_pr_url, state
  untouched), so the next round resumes that branch;
- it keeps the worktree, and a held, done or elsewhere-reviewed card
  gets no PR at all;
- it resets the drive's fix budgets, so the next round starts fresh;
- it leaves one best-effort comment. A cancel still takes the #211 edge.

The trigger is also closed: board_requeue_feature, board_requeue_ci_fix,
POST /ci and POST /review refuse a card the loop is still working. That
is a live drive, a claimed build or a review gate, the same three signals
#437's attach uses. The message says to wait for the round or cancel.

Task drives (sister-agent and self tasks) still block through their own
path: their test fake does not model the claim's state change, and they
have no retries.

Red-check: all 15 tests run against the previous head; 14 fail for the
behavioural reasons (the requeue accepted, the PR unrecorded, a PR opened
for a held/done card, a terminal block after the move, 3 dispatches on a
held card, the hold overwritten by the comment failure, a block on an
unreadable card, budgets not reset). The in-progress refusal pin passes
on both. The reviewer's reproducers R1, R2, R3a, R3b, R4, R5, R7 and R9
now fail, which is to say the defects they demonstrated are gone.

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
Adversarial review of #437 found seven defects; each has a test that fails on the previous
head (8c45cd7) and passes here.

1. Ready-gate bypass: eligibility used the board STATE, and `blocked` is a flag on a lane, so
   a blocked backlog card (never Ready) or a blocked epic went straight into review. It is
   now the lane underneath the block (status without the flag) -- ready/in_progress, or
   in_review under a block -- and only an issue_type `feature`.
2. Re-attaching a blocked card's OWN PR lifted the block (e.g. "review findings persist --
   needs human review") and re-armed the gate without a human. A card already tracking the
   PR is a no-op in review and refused anywhere else; the refusal names the block and that
   unblocking is a deliberate board_unblock_feature.
3. The health sweep moved cards on reads taken before a gh round-trip, outside the claim lock
   the attach writes under: the orphan pass and the blocked self-heal requeued an attach that
   had just landed. Both now re-read the card under the same lock and act only if it is
   still what they read (positive evidence of a move skips). When the sweep adopts the PR
   first, the attach reports already_attached, arms review-pending if the gate is on and no
   review state exists, and still writes its audit line.
4. pr_identity is ratcheted REAL but its real-gh tests never ran in CI. They move to
   tests/test_attach_pr_gh_402.py (no br needed) and the `test (real gh)` job runs it against
   the pinned fixture (#370, vars.PB_GH_FIXTURE_PR). The head check now precedes the fork
   check, so the refusal test doesn't depend on the fixture's origin.
5. A replacement PR kept the old PR's merged-verified stamp, letting the merge edge treat new
   code as gated; it is dropped with the other head pins.
6. A gh timeout / missing checkout on the replacement read escaped the tool and route; every
   attach gh read now converts WorktreeError/OSError into an actionable BoardError. The
   tools no-raise contract test now covers async tools (skipping them is how this slipped).
7. The loop parsed review_gate with bool() ("false" -> ON) while attach and the listings used
   knob_bool; the loop now uses knob_bool.

Also: the audit comment is written last and cannot un-say a landed attach (a failure is
reported as a warning); the docs say drafts attach (auto-merge holds them) and that attached
code has not run the board's pre-PR checks (CI + the review gate still apply). The
`_TaskStore`/`_BlockedStore` fakes in tests/test_loop.py now model the re-read the sweep does.

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 27df8d9509c9. No verdict was posted.
A new push will re-trigger the review.

mabry1985 and others added 5 commits September 10, 2026 16:44
…#402)

The loop adopts a PR it did not see opened in exactly one place: crash recovery finds the
PR whose head is the card's canonical branch and moves the card to in_review. A PR a human
opened had no edge. On 2026-09-07 bd-ezs7's recovered implementation went up as PR #3369 on
the card's own branch; the card sat terminal-blocked while the PR went through CI and review
invisible to the board (re-dispatching would have force-removed the candidate worktree the
work still lived in), and the PM hand-closed it with board_mark_done after the merge.

board_attach_pr / POST /features/{fid}/attach-pr is that edge, no wider than recovery:

- the PR must be the one the board would find for the card itself: OPEN, in the card's
  project repo and not a fork, head on the card's canonical branch (every later edge --
  fix-round resume, recovery, the reap -- keys on it; a PR on another branch would be
  abandoned by the first fix round, which opens a second PR), base on the project's base;
- the card must be an in-flight coding card (ready / in_progress / blocked) with no PR of
  its own in review and no open dependency; a card whose earlier PR is CLOSED may take the
  new one in its place (an open earlier PR would be orphaned, a merged one means it's done);
- the loop must not be working it (live drive, claimed build, running review gate) --
  refused, not overridden -- and the write runs under the loop's claim lock, so a ready
  card cannot be claimed halfway through.

The write is one `br update` (store.attach_pr): in_review + external_ref, dropping
ready/blocked/blocked-class and any stale review verdict or head pin, and arming
review-pending when the review gate is on (else the merge edge waits forever for a verdict
nothing produces). The ordinary reconcile then drives it: CI, review, rebase, merge -> done.
An `attached PR:` comment records who, from what state, and why. Every refusal changes
nothing and names what to do instead.

New gh seam worktree.pr_identity (one `gh pr view`), classified REAL and read against the
pinned PR in the GitHub tier; store.attach_pr classified REAL (every shape through real br).

Also: the tools-doc guard matched only `def board_*`, so the async board_dispatch shipped
undocumented and board_attach_pr would have too; it now matches async defs, and both are
documented.

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
… push to (#402)

The wrong-branch refusal named the card's canonical branch but phrased the fix as "push
the work there". It now leads with the instruction itself -- push the work to branch
'feat/<id>-<slug>', open the PR from it, and attach that PR -- and the test pins that
instruction, not merely that the branch name appears somewhere in the message.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
Adversarial review of #437 found seven defects; each has a test that fails on the previous
head (8c45cd7) and passes here.

1. Ready-gate bypass: eligibility used the board STATE, and `blocked` is a flag on a lane, so
   a blocked backlog card (never Ready) or a blocked epic went straight into review. It is
   now the lane underneath the block (status without the flag) -- ready/in_progress, or
   in_review under a block -- and only an issue_type `feature`.
2. Re-attaching a blocked card's OWN PR lifted the block (e.g. "review findings persist --
   needs human review") and re-armed the gate without a human. A card already tracking the
   PR is a no-op in review and refused anywhere else; the refusal names the block and that
   unblocking is a deliberate board_unblock_feature.
3. The health sweep moved cards on reads taken before a gh round-trip, outside the claim lock
   the attach writes under: the orphan pass and the blocked self-heal requeued an attach that
   had just landed. Both now re-read the card under the same lock and act only if it is
   still what they read (positive evidence of a move skips). When the sweep adopts the PR
   first, the attach reports already_attached, arms review-pending if the gate is on and no
   review state exists, and still writes its audit line.
4. pr_identity is ratcheted REAL but its real-gh tests never ran in CI. They move to
   tests/test_attach_pr_gh_402.py (no br needed) and the `test (real gh)` job runs it against
   the pinned fixture (#370, vars.PB_GH_FIXTURE_PR). The head check now precedes the fork
   check, so the refusal test doesn't depend on the fixture's origin.
5. A replacement PR kept the old PR's merged-verified stamp, letting the merge edge treat new
   code as gated; it is dropped with the other head pins.
6. A gh timeout / missing checkout on the replacement read escaped the tool and route; every
   attach gh read now converts WorktreeError/OSError into an actionable BoardError. The
   tools no-raise contract test now covers async tools (skipping them is how this slipped).
7. The loop parsed review_gate with bool() ("false" -> ON) while attach and the listings used
   knob_bool; the loop now uses knob_bool.

Also: the audit comment is written last and cannot un-say a landed attach (a failure is
reported as a warning); the docs say drafts attach (auto-merge holds them) and that attached
code has not run the board's pre-PR checks (CI + the review gate still apply). The
`_TaskStore`/`_BlockedStore` fakes in tests/test_loop.py now model the re-read the sweep does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
Rebased onto main, where #432 put the multi-call task edges (record_delivery, requeue,
record_verification) under a per-card re-entrant lock: `_br_lock` single-flights each `br`
CALL, not a read-then-write SEQUENCE. store.attach_pr is exactly such a sequence -- read the
card, validate it, write the transition, write the audit line -- so it takes the same lock,
and a concurrent requeue of the same card from another thread (a PM tool) can't land between
its read and its write.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F2V6GRejF7mNukAoYjj2Av
@mabry1985
mabry1985 merged commit 7db1ca1 into main Sep 10, 2026
4 of 5 checks passed
@mabry1985
mabry1985 deleted the feat/attach-pr-402 branch September 10, 2026 23:50
mabry1985 added a commit that referenced this pull request Sep 10, 2026
Resolves store.py against #437: record_pr_url sits beside attach_pr.
The requeue guard's three liveness signals are the same ones #437's
attach checks (loop/attach.py _worked_by_the_loop); the two stay
separate because each words its refusal for its own verb.

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 against #437: the store import gains knob_bool beside
MANUAL_BLOCK_CLASS; docs/tools.md keeps board_attach_pr and this
branch's board_block_feature and board_dispatch rows (the latter with
#437's lifecycle marker); tests/test_docs_reference.py takes main's
async-tool guard, which is the same fix this branch had made.

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 11, 2026
Brings in #430 and #437. The one conflict was the worktree seam contract: main's #402
made `pr_identity` REAL (24) while this branch added #405's four stranded-work seams
(27), so the contract is now 28 REAL / 3 EXEMPT / 0 UNCOVERED, and the README says so.

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 11, 2026
…salvage-427

Resolutions, taking #437's text where both branches did the same job:
- tests/test_docs_reference.py: #437's async-aware `_tools()`, verbatim.
- docs/tools.md: #437's `board_dispatch` row (with its ⚠️), in its place after
  `board_deliver`; the salvage PR's own row for it is dropped.
- loop/__init__.py `__all__`: `attach_external_pr` and `request_salvage` both exported.
- api.py / docs/api.md: the attach-pr and salvage routes side by side.
- Worktree seam contract: 30 REAL (main's 24 with `pr_identity`, #405's four, and #427's
  `commits_ahead` and `own_worktree`), in the test and the README.
- tests/test_board_tools.py: #437 extended the never-raise sweep to async tools, so
  `board_salvage_feature` joins `_NO_STORE_ARGS` (its store reads run inside the loop).

#437 added claim-guarded re-reads before the orphan reconcile and the blocked sweep move
a card, because an attach can land during their gh round-trip. A salvage can start in
the same window. Both re-reads now treat a reserved card as owned, and the orphan
reconcile stops after its gh read if the card was reserved meanwhile, so a crash salvage
never runs on a tree an operator salvage is publishing.

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 11, 2026
#398) (#436)

* fix(loop): a card moved under its drive is not blocked at the hand-off (#398)

bd-p8ft went terminal on a live board with an open PR and no new cause. The
loop's own CI-fix round was building it (in_progress, PR attached) when the
PM called board_requeue_ci_fix for another round. The tool accepts that,
because a card mid-round has the same in_progress + open-PR shape as a card
parked by a CI bounce. The requeue moved the card to ready. When the round
finished, open_review refused ("expects in_progress, got 'ready'") and the
drive's catch-all blocked the card terminally on that message. The bead
records it: `blocked: open_review expects in_progress, got 'ready'`, seven
minutes after the requeue.

The hand-off now checks why it was refused. If the card left in_progress
while the coder worked (requeued, blocked by a human, merged), the move is
the newer decision and it stands. The drive leaves a trail comment, frees
its slot and keeps the worktree. Its commits are already pushed, and a
requeued card's next round resumes that branch, carrying the PM's queued
feedback. A refusal while the card is still in_progress still blocks as
before. A cancelled card still takes the cancel edge, which closes the PR.

This also stops the catch-all from overwriting a human's block. An
operator who blocked a card mid-round used to see their reason replaced by
the hand-off's refusal, because the drive's later `blocked:` comment is the
one the board shows.

Tests run on a real `br` board. The coder, git and GitHub are faked. The
PM's requeue goes through the real tool while the round is live. Against
origin/main the requeue test ends `blocked`, class terminal, reason
"open_review expects in_progress, got 'ready'", which is the incident
exactly. The human-block test sees its reason overwritten. A third test,
where the refusal comes while the card is still in_progress, passes on
both, because that path still blocks.

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

* docs(changelog): fragment for #436

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

* fix(loop): a drive stands aside from a card it no longer owns (#398)

Review of the first cut found five defects. The hand-off was the only
edge that respected a move, and a move it did respect could still lose
work:

1. A FIRST build requeued mid-round published a PR the card never
   recorded. The next round ran resume=False: create_worktree wiped the
   tree and open_pr's --force-with-lease overwrote the pushed branch.
2. Every other build-end path still blocked over a move. A human hold
   placed mid-round plus a transient error re-dispatched the coder three
   times on the held card, re-blocked it `transient`, and the sweep then
   auto-unblocked it, so the hold was lost.
3. Only `cancelled` was checked before publishing. A held or done card
   still got a PR, and the done case left an open PR nothing tracked.
4. The trail comment was not best-effort. If it threw, the outer handler
   re-blocked the card with that error and overwrote a human's hold.
5. A failed read after the refusal meant "not moved": the original
   incident again.

A drive owns its card only while it is in_progress. `_moved_under_drive`
now fails toward not overwriting: an unreadable card is never assumed to
be the drive's own. A refused hand-off names the state it found, and that
is used. Otherwise the answer is `unknown`, and a card left in_progress
with no drive is the sweep's. Before retrying, publishing, handing off or
blocking (all block sites go through `_block_or_stand_aside`), the drive
re-reads the card. A drive that lost its card calls `_stand_aside`:

- it does not block, retry or send the card to review;
- it records a PR it opened on the card (new store.record_pr_url, state
  untouched), so the next round resumes that branch;
- it keeps the worktree, and a held, done or elsewhere-reviewed card
  gets no PR at all;
- it resets the drive's fix budgets, so the next round starts fresh;
- it leaves one best-effort comment. A cancel still takes the #211 edge.

The trigger is also closed: board_requeue_feature, board_requeue_ci_fix,
POST /ci and POST /review refuse a card the loop is still working. That
is a live drive, a claimed build or a review gate, the same three signals
#437's attach uses. The message says to wait for the round or cancel.

Task drives (sister-agent and self tasks) still block through their own
path: their test fake does not model the claim's state change, and they
have no retries.

Red-check: all 15 tests run against the previous head; 14 fail for the
behavioural reasons (the requeue accepted, the PR unrecorded, a PR opened
for a held/done card, a terminal block after the move, 3 dispatches on a
held card, the hold overwritten by the comment failure, a block on an
unreadable card, budgets not reset). The in-progress refusal pin passes
on both. The reviewer's reproducers R1, R2, R3a, R3b, R4, R5, R7 and R9
now fail, which is to say the defects they demonstrated are gone.

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

* fix(loop): a task drive blocks its card only while it is still its own (#398)

Main's #432 task-drive failure path re-read the card before blocking it,
but only exempted a delivered card (in_review / done). A re-read that
FAILED still blocked "as it always did", and a card a human held while
the delegate worked was blocked over, its reason and class replaced.
That is the opposite of the coding drive's rule.

_task_dispatch_failed now uses the same ownership read as the coding
drive (_moved_under_drive): it blocks only a card still in_progress.
Delivered stands (#432's case, unchanged). Any other move stands. An
unreadable card is never assumed to be the drive's own: that is logged,
and a card left in_progress with no drive is the sweep's to reconcile.

Tests on real br (tests/test_task_drive_ownership_398.py): the re-read
fault-injected gives no block; a human hold placed mid-dispatch keeps
its reason and class; a card still in flight is blocked as before.
Red-check against origin/main (81a9787): the first two fail (the card
`blocked` after the failed read, the hold's reason replaced by the
dispatch error), and the third passes on both.

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

* docs(changelog): #436 covers task drives too

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>
@protoreview

protoreview Bot commented Sep 11, 2026

Copy link
Copy Markdown

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

mabry1985 added a commit that referenced this pull request Sep 11, 2026
… feat/salvage-427

No textual conflicts. The main changes meet the salvage in these places:

- #436's requeue refusal reads `_inflight_files`, so a card under a salvage cannot be
  requeued out from under it, and #437's attach refuses it the same way.
- A build that stands aside from a held card (#398) leaves exactly what the salvage
  publishes: the card blocked, and the finished work unpushed in its tree. A new test in
  tests/test_moved_build_tree_405.py runs that end to end on real git and a real `br`
  board. A human holds the card mid-build, and the build stands aside with no PR. The
  salvage then publishes the tree as it stands, on the card's own branch, and puts the
  card in review. Nothing is set aside.
- docs/lifecycle.md says so in the salvage section.

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.

feat(board): attach an externally opened PR to an existing feature

1 participant