fix(kanban): worker-initiated block must not be auto-promoted (#28712) - #28994
Merged
Conversation
When a worker calls ``kanban_block(reason="review-required: ...")`` to hand a task off for human review, the dispatcher's ``recompute_ready`` was treating the resulting ``blocked`` status as eligible for auto-promotion — exactly the same as a circuit-breaker block. On the next tick the task flipped back to ``ready``, a fresh worker spawned, found nothing to do (work already applied, review-required comment already posted), exited cleanly, got recorded as ``protocol_violation`` → ``gave_up`` → ``blocked``, and the dispatcher promoted again. Infinite loop until manual ``hermes kanban reclaim`` + ``kanban block``. Add ``_has_sticky_block`` which distinguishes the two block sources using the cheapest available signal: the most recent ``"blocked"``/``"unblocked"`` event in ``task_events``. * Worker / operator ``kanban_block`` emits ``"blocked"`` → ``_has_sticky_block`` returns True → ``recompute_ready`` skips the task entirely. ``unblock_task`` emits ``"unblocked"`` which flips the predicate back, so the only legitimate exit is the documented human-in-the-loop path. * Circuit-breaker ``_record_task_failure`` emits ``"gave_up"`` (not ``"blocked"``) → predicate stays False → original parent-completion-recovery semantics from #40c1decb3 are preserved. * Tasks blocked purely by direct DB manipulation also recover, since they have no ``"blocked"`` event row at all — matches the existing ``test_recompute_ready_promotes_blocked_with_done_parents`` fixture behaviour.
…28712) Six regression tests pinning the dispatcher contract that was broken in #28712: * test_worker_block_is_not_auto_promoted_by_recompute_ready — kanban_block survives five back-to-back ticks (compressed dispatcher loop). * test_worker_block_on_child_with_done_parents_is_still_sticky — the parent-completion code path was the worst false-positive; even when every parent is done, an explicit worker block stays blocked. * test_circuit_breaker_block_still_auto_promotes — preserves the pre-#28712 recovery semantics for circuit-breaker blocks (direct UPDATE + no "blocked" event). * test_gave_up_event_alone_does_not_make_block_sticky — explicit guard so the gave_up event is never accidentally treated as sticky; covers the second leg of the protocol_violation loop. * test_unblock_clears_sticky_state_and_lets_block_recover — only unblock_task resolves the sticky state; subsequent circuit-breaker blocks recover normally. * test_protocol_violation_loop_is_broken — full bug-shaped reproduction: block → tick → (would-be) crash + gave_up → next tick still blocked. Without the fix this would loop indefinitely. The seventh test from the original PR (legacy-DB init recovery) was dropped during salvage — the schema-init half of #28712 is already fixed on main by #28754 and #28781, and the contract is covered by test_kanban_db.py::test_connect_migrates_legacy_db_before_optional_column_indexes.
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
4 |
unresolved-import |
1 |
First entries
tests/hermes_cli/test_kanban_blocked_sticky.py:229: [unresolved-attribute] unresolved-attribute: Attribute `current_run_id` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_blocked_sticky.py:132: [unresolved-attribute] unresolved-attribute: Attribute `last_failure_error` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_blocked_sticky.py:131: [unresolved-attribute] unresolved-attribute: Attribute `consecutive_failures` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_blocked_sticky.py:35: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/hermes_cli/test_kanban_blocked_sticky.py:260: [unresolved-attribute] unresolved-attribute: Attribute `status` is not defined on `None` in union `Task | None`
✅ Fixed issues: none
Unchanged: 4736 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
alt-glitch
added
type/bug
13 tasks
This was referenced May 20, 2026
This was referenced Jun 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of #28726 — worker / operator-initiated
kanban_block()is now sticky.recompute_readyskips tasks whose latest block-related event intask_eventsis"blocked"while continuing to auto-promote circuit-breaker blocks (which emit"gave_up", not"blocked"). Fixes #28712.Why salvage instead of merge #28726
#28726 also included a schema-init ordering fix for
idx_tasks_session_idon legacy DBs. That half is already onmainvia #28754 (Michael Nguyen) and #28781 (kshitijk4poor), which is why the original PR was showingCONFLICTING. This salvage cherry-picks only the sticky-block half —xxxigm's authorship is preserved per-commit.Changes
hermes_cli/kanban_db.py— new_has_sticky_block(conn, task_id)helper;recompute_ready()skips tasks whose latestblocked/unblockedevent isblocked. +55 / −1 lines.tests/hermes_cli/test_kanban_blocked_sticky.py— 6 regression tests covering both legs of the loop, the circuit-breaker preservation, the unblock exit, and the full protocol-violation reproduction. The original PR's 7th test (legacy-DB init recovery) was dropped during salvage — that contract is already covered bytest_kanban_db.py::test_connect_migrates_legacy_db_before_optional_column_indexeson main.Validation
tests/hermes_cli/test_kanban_blocked_sticky.pytests/hermes_cli/test_kanban_db.pytests/hermes_cli/test_kanban_{cli,core_functionality,notify,diagnostics}.py + tests/tools/test_kanban_tools.pykanban_block/recompute_ready/unblock_task)unblock_taskclears it; circuit-breaker blocks (gave_up event only) still auto-recover; full protocol-violation loop confirmed brokenCredit