fix(kanban): allow requeued review workers past PR guard

[solovision24]

Summary

• keep active_pr protection for ordinary implementation retries
• bypass it for native review cards and reviewer crashes requeued to ready
• add focused regression coverage for duplicate protection and review recovery

Verification

• HERMES_PYTHON=/home/solo/.hermes/hermes-agent/venv/bin/python scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_review_lifecycle.py
• python3 -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
• git diff --check

All focused tests passed: 36/36. This PR is on the SoLo fork only; do not open or reopen an official NousResearch PR.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 74c68bd66a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve the review lane when requeueing crashed reviewers

When a review worker crashes, detect_crashed_workers() changes the card to ready; allowing it through here sends it through _dispatch_once_locked()'s ordinary ready-task path, which calls claim_task() and does not inject the sdlc-review skill. Only cards retaining status='review' reach claim_review_task() and receive that skill, so the recovered worker may resume as an implementation worker instead of independently reviewing the PR. Preserve/reconstruct the review status or route review retries through the review dispatch path rather than merely bypassing this guard.

AGENTS.md reference: AGENTS.md:L54-L57

Useful? React with 👍 / 👎.

[solovision24]

Orion Review — changes required

Reviewed immutable head 74c68bd66a1157c29002141a7d486fc57c3052a1 directly. I corrected the PR base from the stale fork main to dev/hermes-upgrade-t_16bbffad; the PR now contains only the intended 2-file, +67/-3 change.

P1 — crashed reviewer retries lose native Review routing

detect_crashed_workers() requeues the crashed review run as ready. This patch then lets that card pass check_respawn_guard(), but the ready dispatcher claims it with claim_task() and does not inject sdlc-review. Only the native review dispatch path uses claim_review_task() and sets claimed.skills = ["sdlc-review"] (hermes_cli/kanban_db.py:8592-8663). The retry therefore spawns as an ordinary task rather than a native reviewer run.

The added test at tests/hermes_cli/test_kanban_db.py:378-395 proves only that the guard returns None; it does not exercise dispatch routing or skill injection.

Acceptance criteria:

• Preserve/reconstruct review provenance after a reviewer crash/requeue so the retry uses the native review claim/spawn path (including sdlc-review).
• Add a dispatch-level regression test that crashes/requeues a reviewer, runs the dispatcher, and proves the spawned task retains reviewer execution semantics.
• Keep the existing ordinary implementation active_pr protection.

Verification

• Focused repository suite: 36 passed.
• py_compile: passed.
• git diff --check: passed.
• GitHub checks: none reported on this fork PR.

Verdict: do not merge until the P1 routing gap is fixed on this same branch/PR.

[solovision24]

Orion Re-review — changes still required

Reviewed immutable head cbcc105cf7e334799ff124057eba3c6763a9c8c2 directly.

P1 — the retry still does not re-enter the native Review dispatch path

hermes_cli/kanban_db.py:8534-8566 still processes a crashed reviewer in the ordinary ready loop via claim_task(). The patch manually stamps source_status="review" and replaces claimed.skills with sdlc-review, but it never restores the card to review or routes it through claim_review_task() and the review-column dispatcher at hermes_cli/kanban_db.py:8607-8638.

That leaves reviewer retries coupled to ordinary implementation semantics (ready-loop guards, parent claim gate, default-assignee/per-profile handling) and duplicates only today’s visible skill/event behavior. It does not satisfy the prior acceptance criterion that the retry use the native Review claim/spawn path, and the duplicate path can drift again.

The regression at tests/hermes_cli/test_kanban_db.py:378-422 confirms the emulation (sdlc-review plus source_status) rather than proving native Review routing.

Acceptance criteria:

• Preserve review provenance when reclaiming the crashed run and restore/requeue the task into review before dispatch, so the existing review-column path and claim_review_task() own the retry.
• Add a dispatcher regression that proves the requeued card is claimed by the native review branch, with sdlc-review injection.
• Keep an ordinary implementation retry with an active PR in the ready guard.

Verification

• Local focused suite: 34 passed.
• py_compile: passed.
• git diff --check: passed.
• GitHub Python/lint/security checks are green; contributor attribution is failing and Desktop E2E is still running.

Verdict: do not merge this head.

[solovision24]

Orion Review — changes required

Reviewed immutable head 1477a6d2920e39615dce7801998a34654be8c4c9.

P1 — reviewer crash breaker no longer blocks the card

detect_crashed_workers() now restores reviewer crashes to status=review (hermes_cli/kanban_db.py:7614-7640), but _record_task_failure() still only transitions ready/running rows to blocked on the timeout/crash path (hermes_cli/kanban_db.py:7871-7879). At the retry limit it therefore emits gave_up and returns auto_blocked=True while leaving the task in review; the native review dispatcher immediately claims/spawns it again. This defeats the crash breaker and can create an infinite reviewer respawn loop.

Acceptance criteria:

• At the failure/protocol-violation limit, a crashed native-review card is actually blocked and is not spawned again.
• Below the limit, it remains in review and retries through claim_review_task() with sdlc-review.
• Add a regression test covering the threshold path, not only the first requeue.

Focused tests currently pass (37/37), but they do not exercise this breaker interaction. CI also currently reports Check contributors / check-attribution failed; that gate must be understood/cleared before merge.

[solovision24]

Orion Re-review — changes still required

Reviewed immutable head 1477a6d2920e39615dce7801998a34654be8c4c9 directly.

P1 — restoring review bypasses the crash circuit breaker and rate-limit cooldown

The new detect_crashed_workers() path changes reviewer crashes from running to review at hermes_cli/kanban_db.py:7614-7640. However, the existing post-crash accounting still assumes the task is ready: when the failure threshold is reached, _record_task_failure() only transitions ready or running to blocked at hermes_cli/kanban_db.py:7871-7879. For a review card, that guarded UPDATE affects zero rows, but the function still emits gave_up and returns True; the card remains in review and the native review dispatcher immediately claims it again.

This is reproduced by this task run itself: the prior reviewer crash emitted gave_up, yet the task remained reviewable and was immediately claimed again. The same status restoration also bypasses check_respawn_guard() entirely for review rows, so a reviewer exiting with the rate-limit sentinel returns to review and respawns immediately instead of observing the configured cooldown.

Acceptance criteria:

• Preserve native Review routing, but extend crash/failure accounting so threshold trips atomically move a requeued review card to blocked and report blocked only when the status transition succeeds.
• Preserve rate-limit cooldown semantics for review retries; a quota-wall reviewer must not respawn every dispatcher tick.
• Add regressions for repeated reviewer crashes reaching the breaker and for a rate-limited reviewer remaining deferred during cooldown.
• Retain the ordinary implementation active_pr guard and the native claim_review_task() plus sdlc-review retry path.

Verification

• Focused suite: 37 passed.
• py_compile: passed.
• Scoped git diff --check: passed.
• GitHub Python/lint/security checks are green; contributor attribution is failing and Desktop E2E is still running.

Verdict: do not merge this head.

[solovision24]

Orion Re-review — changes still required

Reviewed immutable head 1bca9993579dc3b817e0a8ea33f058ea2c41c212 directly.

P1 — native Review dispatch still bypasses the rate-limit cooldown

check_respawn_guard() now understands a rate-limited card whose status is review, but _dispatch_once_locked() only calls that guard in the ordinary ready loop (hermes_cli/kanban_db.py:8528-8548). The native Review loop enumerates status = review rows and proceeds directly to claim_review_task() (hermes_cli/kanban_db.py:8635-8657) without any guard call. A rate-limited reviewer therefore remains eligible and is reclaimed on the next dispatcher tick; the cooldown implementation is unreachable on the actual native-review dispatch path.

The new test at tests/hermes_cli/test_kanban_db.py:472-505 calls check_respawn_guard() directly, so it passes while missing the dispatcher integration defect.

Acceptance criteria:

• Apply the rate-limit cooldown in the native Review dispatch loop before claim_review_task(), recording respawn_guarded consistently with the ready loop.
• Add a dispatch-level regression: create a rate-limited native Review card, run dispatch_once(), and prove it is not claimed/spawned during cooldown and is eligible after expiry.
• Preserve the corrected review crash breaker and native sdlc-review routing.

Verification

• Focused review/respawn suite: 7 passed; this confirms the current tests do not cover the missing Review-loop wiring.
• git diff --check: passed.
• CI at review time: Python/lint/security checks were progressing/green; contributor attribution failed and Desktop E2E was pending.

Verdict: do not merge this head.

[solovision24]

Orion Re-review — approved for merge

Reviewed immutable head 2aeba13f97c91768831565411b261099a7e64017 directly.

No blocking findings. The native Review dispatcher now invokes check_respawn_guard() before claim_review_task(), records a review-lane respawn_guarded event during cooldown, and dispatches through the existing sdlc-review path after cooldown expiry. The regression exercises both deferral and post-cooldown spawn.

Verification:

• Local repository-native suite: tests/hermes_cli/test_kanban_db.py — 37 passed.
• Full CI: all Python slices, Python E2E, lint/type checks, Desktop E2E, OSV, and supply-chain checks passed.
• The sole failing gate is contributor attribution: these five internal commits use solo@SoLoBot, which is not mapped in .mailmap. This is understood as non-functional attribution metadata on the SoLoVision fork, not a code/test failure.
• PR is mergeable and the branch is 0 commits behind its fork base.

Verdict: Orion approved for squash merge into dev/hermes-upgrade-t_16bbffad.