🐛 fix(kanban): clear active_pr respawn guard on unblock from triage - #61
Conversation
The outer feedback loop (feedback -> author on a card carrying an open
PR) had no automatic active_pr respawn-guard-clearing path — only the
inner review-bounce loop did. unblock_task matched only
`status IN ('blocked','scheduled')`, so a card the block-loop breaker
escalated to `triage` matched zero rows, returned False, and emitted no
`unblocked` event. Because check_respawn_guard uses the latest
`unblocked` event as the cutoff that clears the active_pr guard, a
triaged card carrying an open-PR comment stayed respawn-guarded forever:
the dispatcher refused to spawn the author every tick, and the standard
block->unblock recovery silently no-oped from triage.
Add `'triage'` to both `WHERE status IN (...)` clauses in unblock_task
(the stale-run-pointer SELECT and the status UPDATE), so a triaged card
transitions back to ready/todo (parent gate re-checked) and emits the
`unblocked` cutoff, clearing active_pr exactly like a normal
block->unblock. block_recurrences is deliberately still not reset, so
the loop breaker is preserved — a genuine same-finding loop still
escalates and the counter reset remains an explicit operator action.
Reproduces the wedge RED-first (a triage card with an inflated counter
and an open-PR comment stays active_pr-guarded), then GREEN. Adds
behavior-contract tests: unblock from triage emits the cutoff and clears
the guard; the parent gate is re-checked (undone parent -> todo); the
loop counter survives the unblock. Syncs the now-stale unblock CLI
comments/messages. Carried as a PATCHES.md row composing with the
block-loop-breaker + reset-recovery rows.
cwest
left a comment
There was a problem hiding this comment.
No changes needed.
The fix is exactly the two-clause change it claims: 'triage' added to both the stale-run-pointer SELECT and the status UPDATE in unblock_task, so a card the loop breaker escalated to triage can emit the unblocked cutoff event that check_respawn_guard reads to clear active_pr. Everything downstream of those clauses already generalizes — the parent-gate recheck picks todo vs ready correctly from a triage source, and block_recurrences is left untouched so the loop breaker still escalates a genuine same-finding loop. The auto_route_review_bounce caller is unaffected: it filters on status = 'blocked' before it ever reaches unblock_task, so the only newly-reachable entry is the explicit hermes kanban unblock operator path, which is the intended recovery surface.
One thing I checked because the two triage kinds share a status column with no discriminator: an intake/specifier triage card can now also be moved out by unblock rather than only by specify_task. That's a deliberate consequence of the minimal option, it needs an explicit operator action to trigger, and specification stays the normal path — the specify tests still pass, so it doesn't regress that flow.
Verified against head d9a32b4: 14/14 in the new test file, 369/0 across the nine kanban files that touch the changed code (including the specify and loop-breaker suites). The one signal-handler timing failure I saw locally reproduces identically on the pristine base tip (08e4e5d) and is green in CI here, so it's a pre-existing flake, not this change. mergeable MERGEABLE, mergeStateStatus CLEAN, zero unresolved threads, all required checks green. PATCHES.md row is present and correctly scoped as permanent-local composing with the block-loop rows.
) The outer feedback loop (feedback -> author on a card carrying an open PR) had no automatic active_pr respawn-guard-clearing path — only the inner review-bounce loop did. unblock_task matched only `status IN ('blocked','scheduled')`, so a card the block-loop breaker escalated to `triage` matched zero rows, returned False, and emitted no `unblocked` event. Because check_respawn_guard uses the latest `unblocked` event as the cutoff that clears the active_pr guard, a triaged card carrying an open-PR comment stayed respawn-guarded forever: the dispatcher refused to spawn the author every tick, and the standard block->unblock recovery silently no-oped from triage. Add `'triage'` to both `WHERE status IN (...)` clauses in unblock_task (the stale-run-pointer SELECT and the status UPDATE), so a triaged card transitions back to ready/todo (parent gate re-checked) and emits the `unblocked` cutoff, clearing active_pr exactly like a normal block->unblock. block_recurrences is deliberately still not reset, so the loop breaker is preserved — a genuine same-finding loop still escalates and the counter reset remains an explicit operator action. Reproduces the wedge RED-first (a triage card with an inflated counter and an open-PR comment stays active_pr-guarded), then GREEN. Adds behavior-contract tests: unblock from triage emits the cutoff and clears the guard; the parent gate is re-checked (undone parent -> todo); the loop counter survives the unblock. Syncs the now-stale unblock CLI comments/messages. Carried as a PATCHES.md row composing with the block-loop-breaker + reset-recovery rows. (cherry picked from commit 789e667)
Why
The outer feedback loop — a card bounced back to its author for a revision while it carries an open PR — had no automatic
active_prrespawn-guard-clearing path. Only the inner review-bounce loop self-heals (auto_route_review_bouncereassigns + unblocks, emitting theunblockedcutoff event thatcheck_respawn_guardhonors to clearactive_pr).unblock_taskmatched onlyWHERE status IN ('blocked', 'scheduled'). When repeated churn on the outer loop inflatedblock_recurrencespastBLOCK_RECURRENCE_LIMIT, the block-loop breaker escalated the card totriage— whereunblock_taskmatched zero rows, returnedFalse, and emitted nounblockedevent. Sincecheck_respawn_guarduses the latestunblockedevent as the cutoff that clears theactive_prguard, a triaged card carrying an open-PR comment stayed respawn-guarded forever: the dispatcher refused to spawn the author every tick (respawn_guarded {active_pr}), and the standardblock→unblockrecovery silently no-oped from triage. Only a handreset_block_recurrences+status=blocked+unblock_taskrescued it.What
Add
'triage'to bothWHERE status IN (...)clauses inunblock_task(hermes_cli/kanban_db.py) — the stale-run-pointer SELECT and the status UPDATE. A card escalated totriagenow transitions back toready/todo(parent gate re-checked) and emits theunblockedcutoff, clearingactive_prexactly like a normalblock→unblock.block_recurrencesis deliberately still not reset on unblock, so the loop breaker is fully preserved: a genuine same-finding loop still escalates, and the counter reset remains an explicit operator action (reset_block_recurrences/--reset-loop) or a completion.Tests (RED → GREEN)
test_triage_card_with_open_pr_is_wedged_before_unblockdocuments the exact wedge (a triage card with an inflated counter + open-PR comment isactive_pr-guarded).test_unblock_from_triage_emits_cutoff_and_clears_active_pr— unblock from triage flips toready, emits theunblockedevent, andcheck_respawn_guardreturnsNone.test_unblock_from_triage_rechecks_parent_gate— an undone parent sends the card totodo, notready(parent-completion invariant preserved).test_unblock_from_triage_preserves_block_recurrences— the loop counter survives the unblock (breaker preserved).test_cli_unblock_reset_loop_recovers_card_stuck_in_triagenow asserts the card returns toready(behavior strengthened); stale CLI comments/messages synced.Verification: 451 kanban tests pass, 0 regressions. The 5 new behavior-contract tests reproduce today's exact wedge. Unrelated pre-existing failures (WSL / systemd / qwen-oauth / signal-timing) were confirmed identical on the pristine base tip — this change introduces none.
Carried as a
PATCHES.mdrow (permanent-local) composing with the block-loop-breaker + reset-recovery rows. Base commit SHA unchanged (no rebase onto new upstreammain).