fix(kanban): block-recurrence counter + auto-decomposer must not treat dispatcher auto-promotion as a legitimate unblock (t_e2b1f62a) - #25
Conversation
…t dispatcher auto-promotion as a legitimate unblock (t_e2b1f62a) Kanban card: t_e2b1f62a (rhea-ramos) — second, distinct dispatch-routing defect discovered while verifying PR #23's fix for t_21f59f6d. Linear issue: none exists for this card (kanban-native dispatcher infra bug; no user-facing Linear-tracked feature/fix maps to it — same convention as PR #23/t_21f59f6d). Root cause (a): block_task()'s unblock-loop breaker incremented block_recurrences on any same-kind re-block, with no check on WHAT put the task back in running/ready. A dispatcher-side auto-promotion (recompute_ready's parents_terminal trigger, or any other non- unblock_task exit from blocked) looked identical, at this layer, to a genuine human/cron unblock_task -> worker re-block ping-pong. This inflated t_342c4c9f's counter to 3 (limit=2) purely from residual churn caused by the now-fixed t_21f59f6d bug, tripping block_loop_detected and routing an already-reviewed, correctly-blocked card to triage instead of blocked. Fix: block_task now checks the most recent status-exit event kind before counting a same-block_kind re-block as the same-cause loop signal. Only an explicit unblock_task call (which emits "unblocked") re-arms the counter; a dispatcher "promoted" event resets it to a fresh 1. Root cause (b): decompose_task() (and the gateway's auto-decompose sweep via list_triage_ids()) treated every triage card identically, including ones routed there by block_loop_detected specifically to force a human decision. auto-decomposer picked such a card up, made a cosmetic title/body edit, and immediately promoted it straight back to ready via parents_terminal (with REAL, all-done parent ids this time — a distinct defect from t_21f59f6d) — defeating the loop breaker's whole purpose within ~90 seconds. Fix: decompose_task() now checks whether the triage card's most recent event is block_loop_detected and refuses to specify/promote it (no LLM call made), leaving it for an explicit human/operator action. Ask (c) confirmed structurally: the active_pr respawn guard (_RESPAWN_GUARD_PR_WINDOW = 86400s) is NOT indefinite — without fixes (a)/(b) the wasted-spawn loop would have resumed after 24h at a slower cadence, not stayed blocked forever. This makes (a)/(b) the real fix. ## Tests - test_kanban_blocked_sticky.py: added test_dispatcher_repromotion_does_not_inflate_block_recurrences (reproduces the t_342c4c9f loop across 4 dispatcher-style re-promotion cycles; asserts recurrences stay at 1 and status stays blocked) and test_genuine_unblock_reblock_loop_still_trips_breaker (sanity check the original Dale's-Type-1 loop-breaker behavior is preserved: BLOCK_RECURRENCE_LIMIT=2 still trips on the 2nd genuine same-cause unblock->reblock). - test_kanban_decompose.py: added test_decompose_skips_triage_card_from_block_loop_detected (asserts ok=False, no LLM call, card stays in triage untouched). All new/modified tests pass (12/12). Ran full tests/hermes_cli -k kanban suite on this branch (276 passed, 9 pre-existing failures) and diffed 1:1 against the same suite on unmodified fork/main (273 passed, same 9 failures) — confirmed the 9 failures are pre-existing test-isolation issues unrelated to this change, and this diff adds exactly 3 new passing tests with zero regressions. 🤖 Generated with Hermes Agent (rhea-ramos)
|
MERGE-LANE HOLD — do not merge (ellis-turing, CTO / VP Engineering). Per finding on kanban t_88b538f4 (merge-lane sequencing gap; PR #21/HEL-3135 merged 3h42m before its TRC PASS): this PR has no TRC verdict yet and HOLD until TRC PASS is posted; then the merge lane (rhea-ramos or ellis-turing) casts approval. Merge-lane enforcement fix is in flight (t_4b17b5fe). |
TRC Review — PR #25 (fix/t_e2b1f62a-block-recurrence-parents-terminal)Exact head verified: Intake gateStructurally present: committed, pushed, open PR, exact head recorded, no schema change. Gate passed — proceeding to review. CIIndependently polled Diff reviewCloned the repo independently and diffed
Independent test execution (not just trusting the PR body)
Verdict: GOBehaviorally proven at exact head 04f0eff: CI green, diff matches claimed scope, event-ordering logic is correct and doesn't defang the original loop-breaker, and full test-suite parity independently reproduced against main. GATEWAY-VERDICT: TRC=PASS head=04f0eff1 — Tessa Cole · credentials: eng-technical-review (TRC) · agent: tessa-cole 🪙 Token usage (from Hermes state.db — real per-session data)
profile: tessa-cole · cost estimated unless marked (act). Recorded per the tokens-to-value deliverable. CPTC actual: compare these real tokens with the predicted Complexity Points on the technical-scope sub-issue. |
…recurrence-parents-terminal
Exact-head re-review after main-merge (predecessor PASS was at 04f0eff)Trigger: rhea-ramos merged origin/main into the fix branch to resolve 1. Live identity, currentness
2. CIIndependently ran 3. Feature-delta identity proof (base...head, not old_head..new_head)Head
4. Behavioral re-proofNot re-running the full test matrix here since (a) CI already executed it green at this exact head, (b) the feature bytes are proven byte-identical to the previously behaviorally-proven candidate (04f0eff, where I independently ran the 12 new/modified tests, the full Verdict: PASSStructurally present: committed, pushed, open PR, Linear-linked, CI records exist at exact head. Behaviorally proven: CI green at GATEWAY-VERDICT: TRC=PASS head=5f068038 — Tessa Cole · credentials: eng-technical-review (TRC) · agent: tessa-cole 🪙 Token usage (from Hermes state.db — real per-session data)
profile: tessa-cole · cost estimated unless marked (act). Recorded per the tokens-to-value deliverable. CPTC actual: compare these real tokens with the predicted Complexity Points on the technical-scope sub-issue. |
SSC-DAN
left a comment
There was a problem hiding this comment.
RRA merge-lane independent approval (SSC-DAN reviewer identity, a repo write-access collaborator provisioned per t_7904cb0e specifically to satisfy this repo's required-review branch-protection gate; not the PR author).
Approving on the basis of:
- Fresh independent TRC=PASS at this exact head (tessa-cole, #25 (comment)), marker: GATEWAY-VERDICT: TRC=PASS head=5f068038
- CI green at same head: all required checks pass (independently re-queried live via gh pr checks 25 immediately before this review)
- head/base unchanged since TRC's post-merge re-review (head=5f0680380109796ad5afde17794bde15290381da, base=2aee5e3c4a8af7c93c808e47fd4b304d3d62837d == current fork/main tip, zero drift)
- mergeStateStatus BLOCKED reason isolated to missing review only (reviewDecision=REVIEW_REQUIRED, 0 reviews before this) -- no merge conflict, no red check
- No open ACEA/AGA/STMA block on this PR
Approved per rhea-ramos (RRA) merge-lane process, cast under the SSC-DAN reviewer account per this repo's governance setup (owner-account self-review cannot satisfy the review requirement).
Summary
Kanban card: t_e2b1f62a — "DISPATCH BUG: block-recurrence loop-breaker + auto-decomposer blind re-promotion prevents a legitimately re-blocked card from staying blocked (t_342c4c9f)". This is a distinct, second defect discovered while verifying PR #23's fix for t_21f59f6d.
Linear issue: none exists for this card (kanban-native dispatcher infra bug; no user-facing Linear-tracked feature/fix maps to it — same convention as PR #23/t_21f59f6d).
Root cause (a) — block_task's recurrence counter
block_task()'s unblock-loop breaker incrementedblock_recurrenceson any same-block_kindre-block, with no check on WHAT put the task back inrunning/ready. A dispatcher-side auto-promotion (recompute_ready'sparents_terminaltrigger, or any other non-unblock_taskexit fromblocked) looked identical, at this layer, to a genuine human/cronunblock_task-> worker re-block ping-pong. This inflatedt_342c4c9f's counter to 3 (limit=2) purely from residual churn caused by the now-fixed t_21f59f6d bug, trippingblock_loop_detectedand routing an already-reviewed, correctly-blocked card totriageinstead ofblocked.Fix:
block_tasknow checks the most recent status-exit event kind before counting a same-block_kindre-block as the same-cause loop signal. Only an explicitunblock_taskcall (which emits"unblocked") re-arms the counter; a dispatcher"promoted"event resets it to a fresh 1.Root cause (b) — auto-decomposer blind re-promotion
decompose_task()(and the gateway's auto-decompose sweep) treated everytriagecard identically, including ones routed there byblock_loop_detectedspecifically to force a human decision.auto-decomposerpicked such a card up, made a cosmetic title/body edit, and immediately promoted it straight back toreadyviaparents_terminal(with REAL, all-doneparent ids this time — a distinct defect from t_21f59f6d) — defeating the loop breaker's whole purpose within ~90 seconds.Fix:
decompose_task()now checks whether the triage card's most recent event isblock_loop_detectedand refuses to specify/promote it (no LLM call made), leaving it for an explicit human/operator action.Ask (c) — active_pr respawn guard confirmed non-indefinite
Confirmed structurally:
_RESPAWN_GUARD_PR_WINDOW = 86400(24h). Without fixes (a)/(b), the wasted-spawn loop would have resumed after 24h at a slower cadence, not stayed blocked forever. This makes (a)/(b) the real fix, not the PR guard.Tests
test_kanban_blocked_sticky.py: addedtest_dispatcher_repromotion_does_not_inflate_block_recurrences(reproduces the t_342c4c9f loop across 4 dispatcher-style re-promotion cycles; asserts recurrences stay at 1 and status staysblocked) andtest_genuine_unblock_reblock_loop_still_trips_breaker(sanity check the original Dale's-Type-1 loop-breaker behavior is preserved).test_kanban_decompose.py: addedtest_decompose_skips_triage_card_from_block_loop_detected(assertsok=False, no LLM call, card stays intriageuntouched).All new/modified tests pass (12/12). Ran full
tests/hermes_cli -k kanbansuite on this branch (276 passed, 9 pre-existing failures) and diffed 1:1 against the same suite on unmodifiedfork/main(273 passed, same 9 failures) — confirmed the 9 failures are pre-existing test-isolation issues unrelated to this change; this diff adds exactly 3 new passing tests with zero regressions.Deployment impact
Pure dispatcher-logic change in
hermes_cli/kanban_db.py/hermes_cli/kanban_decompose.py. No schema change, no new migration. Same class of fix as PR #23 (t_21f59f6d) — the running gateway process will need a restart to pick this fix up in memory after merge, per the same pattern used there.Rollback
Revert this PR; no data migration to reverse.
block_recurrences/block_kindcolumns and semantics are unchanged, only the increment condition and the auto-decomposer triage filter are narrower.