🐛 fix(kanban): restore crashed reviewer to review lane, not build lane

[cwest]

What & why

A card moved to review (a worker opened a PR and parked it) is claimed by claim_review_task, which CAS-transitions review → running and records a claimed event with source_status: "review" in its payload. While the reviewer works, the card's row status is running — indistinguishable at the row level from a build run.

If that reviewer worker crashes (signal / OOM / host hiccup), detect_crashed_workers finds the card in running and takes the running-crash path, which reset the card to ready. That dropped the review lane entirely, with two consequences on the next dispatch tick:

1. Wrong worker respawns. The card is now ready, not review, so the review-column dispatch (WHERE status = 'review') does not pick it. The normal ready dispatch claims it as a fresh build run — re-running the implementer instead of the reviewer. The PR under review silently falls out of the review lane back into the build lane.
2. Respawn is deferred. check_respawn_guard recomputes is_review = (status == "review") becomes False, so the recent_success guard (the original build run is a recent completed run) and active_pr guard (the build handoff left a PR-URL comment) both fire, deferring respawn for the full guard window.

A prior change (the non-running-lane stale-claim clear) only handled a dead worker on a card parked in a non-running lane; it did not cover a reviewer that dies while actively running. This is that gap.

The fix

In hermes_cli/kanban_db.py:

• Durable signal (no new schema). Added _crashed_run_was_review(conn, task_id, current_run_id) which reads the source_status: "review" field off the crashed run's claimed event payload, scoped to current_run_id (falls back to the task's latest claimed event). This reuses the existing signal claim_review_task already writes — no new column.
• Lane restoration. In the running-crash branch, compute was_review_run and set the reset UPDATE's status to 'review' when true (else 'ready'). The UPDATE still clears claim_lock / claim_expires / worker_pid and is still CAS-guarded on status = 'running'. The crash event, run outcome, failure counter, and circuit breaker bookkeeping are unchanged — only the lane a crash returns to changes.
• Breaker still works. The crash-path breaker-trip UPDATE in _record_task_failure widened its WHERE ... status IN (...) from ('ready','running') to ('ready','running','review'). Without this, a card restored to review would never flip to blocked and a flaky reviewer would loop forever. Now a repeatedly-crashing reviewer still trips to blocked via the normal failure-count path.
• Rate-limited (cooldown defer) and protocol-violation (immediate trip) sub-cases are untouched.

Tests — RED to GREEN

Added 4 regression tests in tests/hermes_cli/test_kanban_db.py:

1. test_crashed_reviewer_run_restored_to_review_lane — reviewer run (claimed via claim_review_task) whose PID is dead → card back in review, claim cleared.
2. test_crashed_build_run_restored_to_ready_lane — build run (claimed via claim_task) whose PID is dead → card back in ready (no regression).
3. test_crashed_reviewer_still_trips_breaker_to_blocked — reviewer crashing up to the breaker limit → blocked (no infinite review-lane loop).
4. test_respawn_guard_frees_restored_review_card — end-to-end: restored review card returns None from check_respawn_guard for the recent_success/active_pr cases (reviewer is free to respawn).

RED (4 tests against the pristine kanban_db.py, implementation stashed):

FAILED test_crashed_reviewer_run_restored_to_review_lane     - got 'ready', expected 'review'
FAILED test_crashed_reviewer_still_trips_breaker_to_blocked  - 2nd claim_review_task returns None (lane lost)
FAILED test_respawn_guard_frees_restored_review_card         - got 'ready', expected 'review'
3 failed, 1 passed, 230 deselected in 0.48s

(Test 2 passes on pristine code by design — it asserts the unchanged build-crash behavior.)

GREEN (with the fix):

tests/hermes_cli/test_kanban_db.py::test_crashed_reviewer_run_restored_to_review_lane PASSED
tests/hermes_cli/test_kanban_db.py::test_crashed_build_run_restored_to_ready_lane PASSED
tests/hermes_cli/test_kanban_db.py::test_crashed_reviewer_still_trips_breaker_to_blocked PASSED
tests/hermes_cli/test_kanban_db.py::test_respawn_guard_frees_restored_review_card PASSED
4 passed, 230 deselected in 0.22s

Verification

• pytest tests/hermes_cli/test_kanban_db.py → 234 passed (230 baseline + 4 new).
• Full kanban surface (test_kanban_db.py, test_kanban_core_functionality.py, test_signal_handler_kanban_worker.py, test_kanban_cli.py, test_kanban_diagnostics.py, test_kanban_goal_mode.py, test_kanban_boards.py, test_kanban_db_init.py, test_kanban_tools.py) → 671 passed, 1 skipped, 1 pre-existing failure (test_sigterm_with_kanban_task_env_terminates_quickly, a SIGTERM timing test that fails identically on the pristine base 3a065375d — unrelated to this change, verified by stashing).
• ruff check hermes_cli/kanban_db.py → All checks passed!

PATCHES.md

Added an upstream-pending row (no upstream PR — fork-internal review-lane semantics, not surfaced to NousResearch) documenting the carry and its retire trigger.

Notes

• Base: cwest/integration (3a065375d). Branched off integration, not main.
• Single source file changed: hermes_cli/kanban_db.py.
• This is a draft and stays a draft until reviewed and undrafted; merge is the maintainer's call.

[cwest]

The fix itself holds up. I read the source at the head SHA and confirmed the premise: check_respawn_guard gates both recent_success and active_pr on if not is_review (where is_review keys off status == 'review'), so restoring a crashed reviewer to review instead of ready is exactly what frees those guards. The crash path calls _record_task_failure with release_claim=False, which takes the branch whose WHERE-IN you widened to include 'review', so a repeatedly-crashing reviewer still trips the breaker to blocked rather than looping. Reading source_status off the run's claimed event is the right durable signal; no new column. The CAS guard on status='running' and the crash bookkeeping are untouched.

Verified the tests independently: reverted only the production hunk to the parent commit and the new tests went 3 failed / 1 passed (the build-to-ready case passing as designed); restored, and the full file runs 234 passed. ruff clean. Commit is signed and follows the convention.

One thing to settle before this lands, on the PATCHES.md row.

[cwest]

The PATCHES.md bucket is reclassified to permanent-local and the row now reads consistently with it (no upstream PR, never auto-retires, not surfaced upstream). That was the only open item; the kanban_db.py fix and its four regression tests are unchanged from the prior pass and verify clean.

Re-ran at this head in a throwaway clone: 234 tests pass (the 4 new crash-recovery cases included), ruff clean on hermes_cli/kanban_db.py. The merge-readiness picture is green end to end — mergeable, all required checks succeeded, no unresolved threads.

No changes needed. Ready to merge.