🐛 fix(kanban): make reviewer PASS->acceptance atomic via accept_task - #70
Conversation
A reviewer PASS parked the card for sign-off in two separate steps: a status flip to blocked and an assign to the acceptance owner. When only one half landed, the card stranded in review/owner — the reviewer's lane with the acceptance owner's name on it — and, because the acceptance notification rides the blocked event, no ping fired. Hand-reconciled via a housekeeping safety net after the fact. Add accept_task: a single atomic primitive that, in ONE write_txn, flips status to blocked, sets assignee to the acceptance owner, ends the reviewer's run, and emits the blocked event carrying the awaiting-casey-signoff reason. The reviewer PASS path (kanban_block with that reason) resolves the acceptance owner from the card's own owner map — profile-agnostic, no name baked into core — and routes to accept_task, so the transition can never land half-applied. A block whose reason is not the acceptance sign-off, or a card with no stamped acceptance owner, falls through to the normal block path unchanged. Behavior-contract tests cover the atomic landing, the single sign-off event, run/claim release, reason normalization, refusal off a non-review card, the unclaimed-review-lane path, and the reviewer toolset routing.
cwest
left a comment
cwest
left a comment
There was a problem hiding this comment.
No changes needed.
The split PASS->acceptance (a status flip plus a separate assign) is what stranded cards in review/owner with no blocked event, and this collapses both halves into one write_txn. accept_task mirrors block_task's proven shape line for line: the same status-guarded UPDATE (with and without the run fence), the _end_run then _synthesize_ended_run fallback for a never-claimed review card, and the lifecycle hook fired outside the transaction. Either every field moves together or none does.
The part that had to be right is how the emitted event interacts with the two housekeeping sweepers, and it holds. accept_task emits a blocked event whose reason carries the awaiting-casey-signoff prefix. auto_route_review_bounce gates on the review-changes-requested prefix, so it skips this reason and never bounces a PASS'd card back to an author. reconcile_pass_acceptance only scans status='review' rows, and the card is already blocked once accept_task runs, so the safety net can't double-fire. The two coexist as claimed.
Owner resolution is read from the card's own submit-stage owner map, not a name in core; it resolves to the acceptance owner against the real card comment. The toolset routing keys strictly on the signoff prefix, so a needs_input or any other block still takes the normal path with the reviewer keeping the card.
Verified against the head SHA in a throwaway clone: the new file is 8/8, and the surrounding kanban suite (accept_task, block_kinds, reconcile_pass_acceptance, complete_acceptance_guard, auto_route_review_bounce, sticky-block, reset-loop, core kanban_db, and the tools tests) is 635 passed, 0 failed. The two toolset tests exercise _handle_block directly rather than accept_task, so the actual reviewer entry point is covered end to end. SQL is parameterized throughout; no injection, secrets, or unsafe calls.
…70) A reviewer PASS parked the card for sign-off in two separate steps: a status flip to blocked and an assign to the acceptance owner. When only one half landed, the card stranded in review/owner — the reviewer's lane with the acceptance owner's name on it — and, because the acceptance notification rides the blocked event, no ping fired. Hand-reconciled via a housekeeping safety net after the fact. Add accept_task: a single atomic primitive that, in ONE write_txn, flips status to blocked, sets assignee to the acceptance owner, ends the reviewer's run, and emits the blocked event carrying the awaiting-casey-signoff reason. The reviewer PASS path (kanban_block with that reason) resolves the acceptance owner from the card's own owner map — profile-agnostic, no name baked into core — and routes to accept_task, so the transition can never land half-applied. A block whose reason is not the acceptance sign-off, or a card with no stamped acceptance owner, falls through to the normal block path unchanged. Behavior-contract tests cover the atomic landing, the single sign-off event, run/claim release, reason normalization, refusal off a non-review card, the unclaimed-review-lane path, and the reviewer toolset routing. (cherry picked from commit b3aa55f)
1 participant
Why
A reviewer PASS parks the card for sign-off in two separate steps — a status flip to
blockedand anassignto the acceptance owner. When only one half lands, the card strands inreview/owner (the reviewer's lane with the acceptance owner's name on it), and because the acceptance notification rides theblockedevent, no ping fires. This was hit on three cards in one session and hand-reconciled after the fact by a housekeeping safety net (reconcile_pass_acceptance).What
Add
accept_task: a single atomic primitive that, in ONEwrite_txn:status→blocked(the acceptance lane),assignee= the acceptance owner (resolved by the caller, never a literal name in core),blockedevent carrying theawaiting-casey-signoffreason (the event the acceptance notifier pings on, that makes the state stick, and that the sticky-block / auto-route detectors key on).The reviewer PASS path (a
kanban_blockwhose reason is the acceptance sign-off) now resolves the acceptance owner from the card's own materialized owner map — profile-agnostic, no name baked into core — and routes toaccept_task, so the assign and the block can never land half-applied. A block whose reason is not the acceptance sign-off, or a card with no stamped acceptance owner, falls through to the normal block path unchanged.The housekeeping
reconcile_pass_acceptancesafety net stays as defense-in-depth for any legacy/out-of-band stranding; this change removes the need for it on the reviewer PASS path itself.Done when
blocked/owner with theawaiting-casey-signoffevent in a single transition — noreview/owner stranding.blockedevent).Verification
tests/hermes_cli/test_kanban_accept_task.py: 8 tests, all green.hermes_cli/tools/gatewaytransition-emit + notify): 1004 tests passed, 0 failed.test_kanban_accept_task,test_kanban_block_kinds,test_kanban_reconcile_pass_acceptance,test_kanban_complete_acceptance_guard— 42 passed, 0 failed.