fix(kanban): worker-initiated block must not be auto-promoted (#28712) by xxxigm · Pull Request #28726 · NousResearch/hermes-agent

xxxigm · 2026-05-19T12:47:34Z

What does this PR do?

#28712 describes a kanban infinite loop:

Worker calls kanban_block(reason="review-required: ...") to hand off to a human.
Dispatcher's recompute_ready() flips the task back to ready on the next tick.
Fresh worker spawns, finds no actionable instructions (work already applied, review-required comment already posted), exits cleanly.
detect_crashed_workers records protocol_violation → _record_task_failure(failure_limit=1) → gave_up → blocked.
Next tick: recompute_ready promotes again → goto 3.

Result: burned API calls, phantom "crashed" runs polluting task history, misleading repeated_crashes flag from kanban diag, and amplified pressure on rate-limited providers (the reporter hit 429s during the loop on Kiro/Anthropic).

Root cause — recompute_ready was treating every blocked task with satisfied parents as eligible for promotion, with no way to distinguish:

Worker / operator-initiated blocks (kanban_block) — deliberate human-in-the-loop handoff; must stay blocked until explicit kanban_unblock, per the documented kanban-orchestrator skill contract.
Circuit-breaker blocks (_record_task_failure tripping on repeated crashes) — should auto-recover when conditions change (parents complete, transient infra clears). This is the original intent of #40c1decb3 ("promote blocked tasks when parent dependencies complete").

Fix — distinguish the two using the cheapest available signal: the most recent "blocked"/"unblocked" event in task_events.

kanban_block already emits a "blocked" event row (with the review-required: … reason).
kanban_unblock already emits an "unblocked" event row.
Circuit-breaker _record_task_failure emits "gave_up", not "blocked".

New helper _has_sticky_block(conn, task_id) returns True iff the most recent of the two block-related events is "blocked". recompute_ready consults it and skips sticky-blocked tasks. The only legitimate exit is unblock_task(), which emits "unblocked" and flips the predicate back — exactly the documented human-in-the-loop pattern.

Also fixes the tangentially related schema-init crash the reporter flagged at the bottom of #28712 (init_db failed with no such column: session_id on a kanban.db that pre-dated the session_id migration). Three CREATE INDEX statements on tasks(<late-added-column>) were sitting at the top of SCHEMA_SQL, where they run before the additive-column migrations. On a legacy DB the table's CREATE TABLE IF NOT EXISTS is a no-op, the column doesn't exist yet, and the index DDL crashes the whole init script — including the migration that would have fixed it. The reporter had to ALTER TABLE + CREATE INDEX by hand to unstick their install. Moved all three (idx_tasks_tenant, idx_tasks_idempotency, idx_tasks_session_id) into _migrate_add_optional_columns, after the ALTER calls that guarantee the columns exist.

Related Issue

Fixes #28712

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue)
✅ Tests (adding or improving test coverage)

Changes Made

hermes_cli/kanban_db.py —
- New _has_sticky_block(conn, task_id) -> bool helper that reads the most recent "blocked"/"unblocked" event for a task.
- recompute_ready() now continues past blocked tasks whose latest block-related event is "blocked"; circuit-breaker blocks (with no event, or "gave_up" event) continue to auto-recover when parents complete.
- Moved idx_tasks_tenant, idx_tasks_idempotency and idx_tasks_session_id out of SCHEMA_SQL and into _migrate_add_optional_columns, asserted unconditionally with IF NOT EXISTS so new DBs get them on first init and legacy DBs get them after the additive ALTER TABLE calls.
- Combined: 92 added / 15 removed lines.
tests/hermes_cli/test_kanban_blocked_sticky.py — 344 lines, 7 new tests:
- test_worker_block_is_not_auto_promoted_by_recompute_ready — five back-to-back ticks leave the task blocked.
- test_worker_block_on_child_with_done_parents_is_still_sticky — the worst false-positive (parent-completion path) is closed.
- test_circuit_breaker_block_still_auto_promotes — preserves the pre-kanban: dispatcher auto-promotes blocked task → respawn worker → protocol_violation loop #28712 recovery semantics for the original 40c1decb3 intent.
- test_gave_up_event_alone_does_not_make_block_sticky — explicit guard so the protocol_violation loop's second leg can't regress.
- test_unblock_clears_sticky_state_and_lets_block_recover — the only legitimate exit, and subsequent circuit-breaker blocks still auto-recover.
- test_protocol_violation_loop_is_broken — full bug reproduction: block → tick → (would-be) crash + gave_up → next tick still blocked. Would loop indefinitely without the fix.
- test_init_db_recovers_from_legacy_tasks_table_without_session_id — hand-crafted pre-tenant / pre-idempotency_key / pre-session_id tasks table, calls init_db, asserts all three columns + indexes end up present and legacy rows survive.

How to Test

# New regression suite (7 tests).
scripts/run_tests.sh tests/hermes_cli/test_kanban_blocked_sticky.py -q

# Existing dispatcher tests must still pass, including
# test_recompute_ready_promotes_blocked_with_done_parents which
# pins the circuit-breaker recovery contract.
scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py -q
# Expected: 156 passed, 1 failed (pre-existing
# `test_max_runtime_uses_current_run_start_after_retry` — unrelated
# `os.kill(999999, 0)` live-system guard, verified on upstream/main).

# Broader sweep.
scripts/run_tests.sh \
  tests/hermes_cli/test_kanban_cli.py \
  tests/hermes_cli/test_kanban_core_functionality.py \
  tests/hermes_cli/test_kanban_notify.py \
  tests/hermes_cli/test_kanban_diagnostics.py \
  tests/tools/test_kanban_tools.py \
  -q
# Expected: only the 3 pre-existing `os.kill` flakes fail
# (verified identical on upstream/main).

Manual reproduction of the loop fix:

From a worker session, call kanban_block with reason="review-required: please verify".
Without running hermes kanban unblock, wait through one or more dispatcher ticks (or call hermes kanban dispatch directly).
Before this PR: task flips back to ready, fresh worker spawns, exits cleanly with protocol_violation, repeats.
After this PR: task stays blocked indefinitely until you run hermes kanban unblock <id>. Once unblocked, normal promotion + claim semantics resume.

Manual reproduction of the schema-init fix:

# Take any older kanban.db that pre-dates the session_id migration.
sqlite3 ~/.hermes/kanban.db 'PRAGMA table_info(tasks)' | grep -q session_id || echo "DB is pre-session_id"
# Run hermes kanban init — before this PR, crashed with
# `OperationalError: no such column: session_id`.  After this PR,
# completes silently and adds the missing columns + indexes.
hermes kanban init

Checklist

Code

My commit messages follow Conventional Commits (fix(kanban):, test(kanban):)
I searched for existing PRs — no open duplicate (the related feat(kanban): auto-unblock blocked tasks when non-self comment lands (kanban t_0abf738d) #27796 was a different auto-unblock-on-comment design, closed)
My PR contains only changes related to this fix
I've run scripts/run_tests.sh tests/hermes_cli/test_kanban_blocked_sticky.py -q (7 passed)
I've added regression tests covering every leg of the loop and the migration path
I've tested on my platform: macOS 15.x (darwin 24.6.0)

Documentation & Housekeeping

N/A — no user-facing docs change. The fix restores the documented behaviour from the kanban-orchestrator skill ("Any task can kanban_block() to wait for input. Dispatcher respawns after /unblock."), which the bug was violating.
N/A — no config keys changed
N/A — no architecture / workflow change
I've considered cross-platform impact — pure SQLite-via-sqlite3, no platform-specific code
N/A — no tool description/schema changes (the kanban_block / kanban_unblock tool surfaces are unchanged; only the dispatcher's interpretation of their outputs changes)

…search#28712) 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.

…ousResearch#28712) ``init_db`` on a kanban.db that pre-dates the ``session_id``, ``tenant`` or ``idempotency_key`` migrations crashed with ``no such column: <name>`` because ``SCHEMA_SQL`` asserted the index on those columns before ``_migrate_add_optional_columns`` had a chance to ADD the columns. On legacy DBs the ``CREATE TABLE IF NOT EXISTS`` at the top of the script was a no-op, so the table never grew the new columns, and the next ``CREATE INDEX`` blew up the entire init sequence — including the migration that would have fixed it. Reporter had to manually ``ALTER TABLE`` + ``CREATE INDEX`` to unstick their install. Move ``idx_tasks_tenant``, ``idx_tasks_idempotency`` and ``idx_tasks_session_id`` out of ``SCHEMA_SQL`` and into ``_migrate_add_optional_columns``, unconditionally and with ``IF NOT EXISTS`` so they're a cheap no-op on fresh DBs where the columns and indexes were created together. The additive ``ALTER TABLE`` for each column happens first; the index assertion runs after, by which point the column is guaranteed to exist on both new and legacy schemas.

…arch#28712) Seven regression tests pinning the contract that was broken in NousResearch#28712: Dispatcher (``recompute_ready``): * ``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-NousResearch#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. Schema init: * ``test_init_db_recovers_from_legacy_tasks_table_without_session_id`` — hand-crafts a pre-``tenant`` / pre-``idempotency_key`` / pre- ``session_id`` ``tasks`` table, calls ``init_db``, and asserts all three columns + indexes end up present and legacy rows survive.

teknium1 · 2026-05-20T00:26:42Z

Salvaged via #28994 (merged d35f893 + 4dac6e7-ish — both commits preserve your authorship per rebase-merge). Thanks for the clean RCA + thorough test coverage — the task_events history signal was exactly the right approach.

The schema-init half of your PR (the idx_tasks_session_id ordering fix) wasn't cherry-picked because it had already landed on main via #28754 and #28781 between your PR opening and the salvage. The redundant 7th test was also dropped — that contract is now covered by test_kanban_db.py::test_connect_migrates_legacy_db_before_optional_column_indexes.

Closing this PR; your sticky-block fix + 6 regression tests are now on main.

xxxigm added 3 commits May 19, 2026 19:45

alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 19, 2026

This was referenced May 19, 2026

fix(kanban): prevent recompute_ready from unblocking parent-free blocked tasks #28724

Closed

[Bug]: Kanban auto-unblock is too eager #28903

Closed

Kanban: blocked tasks auto-promoted by daemon without explicit human unblock #28944

Closed

teknium1 mentioned this pull request May 20, 2026

fix(kanban): worker-initiated block must not be auto-promoted (#28712) #28994

Merged

teknium1 closed this May 20, 2026

Navigation Menu

Uh oh!

fix(kanban): worker-initiated block must not be auto-promoted (#28712) - #28726

fix(kanban): worker-initiated block must not be auto-promoted (#28712)#28726
xxxigm wants to merge 3 commits into
NousResearch:mainfrom
xxxigm:fix/28712-blocked-sticky-dispatcher

xxxigm commented May 19, 2026

Uh oh!

teknium1 commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

xxxigm commented May 19, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Uh oh!

teknium1 commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants