fix(kanban): review tasks bypass dup-PR respawn guards (active_pr/recent_success)

[demi-hl]

Symptom (reproduced live on main, 2026-06-15)

Review-required cards never get reviewed. A card that shipped a PR sits in status='review' but the dispatcher refuses to spawn the reviewer for up to 24h — the review lane is wedged out by guards that only make sense for the build lane.

Root cause

hermes_cli/kanban_db.py :: check_respawn_guard(conn, task_id) runs in dispatch_once for every ready+assigned task, build and review alike. Two of its four guards exist solely to stop a builder re-running and opening a duplicate PR:

• "recent_success" — a completed run exists within _RESPAWN_GUARD_SUCCESS_WINDOW (1h).
• "active_pr" — a GitHub PR URL appears in a comment within _RESPAWN_GUARD_PR_WINDOW (24h).

But the build run that ships a PR is itself a recent completed run and leaves a PR-URL comment. When that same card transitions to status='review', both guards fire and defer the reviewer spawn — for up to the 24h PR window. A reviewer never opens a PR, so neither rationale applies to a review pass.

Fix (surgical — only check_respawn_guard + tests)

1. Extend the existing task-row SELECT to also read tasks.status.
2. is_review = (status == "review").
3. Skip the recent_success and active_pr checks when is_review is true (return None path).
4. Keep rate_limit_cooldown and blocker_auth active for review spawns — a rate-limited / auth-blocked reviewer should still defer.

No new config keys, no new env vars, no refactor. One extra column in one existing query, two if not is_review: guards.

Tests

Added 4 invariant tests in tests/hermes_cli/test_kanban_db.py (asserting the invariant, not a snapshot):

• test_respawn_guard_review_bypasses_active_pr_build_does_not — same PR-comment task returns "active_pr" on status='ready' but None on status='review'.
• test_respawn_guard_review_bypasses_recent_success_build_does_not — same recent-completed-run task returns "recent_success" on ready but None on review.
• test_respawn_guard_review_with_pr_and_success_returns_none — acceptance shape: review task with both a PR comment and a recent completed run returns None; the build-lane equivalent stays guarded.
• test_respawn_guard_review_still_honors_blocker_auth — review status still defers on an auth/quota error.

Verification

.venv/bin/python -m pytest tests/hermes_cli/test_kanban_db.py -q
218 passed

(The dup-PR protection for the build lane is fully intact — every existing check_respawn_guard test still passes.)

Acceptance criteria (from the task)

• check_respawn_guard returns None for a review-status task that has a PR comment + recent completed run.
• check_respawn_guard still returns "active_pr" for a ready-status task with a PR comment (build dup-PR protection intact).
• New/extended tests pass.
• PR opened, not merged.

[liuhao1024]

Code Review — LGTM ✅

Scope: Kanban check_respawn_guard — review-status tasks bypass recent_success and active_pr guards.

What was checked:

• is_review derived from row["status"] read in the same query — no TOCTOU gap
• Rate-limit cooldown (step 1) and auth blocker (step 2) still apply to review tasks — only dup-PR guards (steps 3-4) are bypassed
• SQL change: SELECT last_failure_error → SELECT last_failure_error, status — minimal, no schema migration needed
• 4 new tests covering: review bypasses active_pr (build doesn't), review bypasses recent_success (build doesn't), combined PR+success returns None for review, review still honors blocker_auth
• Docstring updated to document the review-status exception for both guards

No issues found. The bypass is well-scoped: only the two guards that would wedge a reviewer are disabled, while rate-limit and auth-blocker protections remain intact.

[Chadtao206]

Independent confirmation — we hit this exact bug in production, and the root cause is just as described: a reviewer never opens a PR, so active_pr/recent_success shouldn't gate a review pass.

One gap worth covering before merge: keying the bypass on status == "review" misses dedicated review tasks that sit in status='ready'. When a re-review is delegated as its own card (assignee='reviewer', status='ready', not 'review'), the card posts its own review comment containing the PR URL it is reviewing (e.g. … blocker posted: https://github.com/<org>/<repo>/pull/<N>#issuecomment-…). Once it returns to ready after the remediation completes, active_pr matches that self-authored comment and the dispatcher skips it every tick (respawn_guarded {"reason":"active_pr"}) until the 24h _RESPAWN_GUARD_PR_WINDOW elapses — we observed ~10.5h. status is 'ready' throughout, so a status=='review' check wouldn't catch it.

Suggestion: gate on either status=='review' or the assignee being a review lane — cf. #32241, which exempts {tester,qa} by assignee. Combining the two covers both cards-awaiting-review and delegated review tasks.

Tangent: the same misfire also surfaces a misleading kanban dispatcher stuck … Check profile health (venv, PATH, credentials) warning, because has_spawnable_ready() counts the guarded task as spawnable — #44899 addresses that half. Refs: #29458 (umbrella), #42053 / #49870 (sibling recent_success).

[teknium1]

Thanks for the focused reproduction and regression tests. This is an automated hermes-sweeper review; current main already provides the requested review-card behavior through a later dispatcher structure.

• hermes_cli/kanban_db.py:7465-7485 calls check_respawn_guard() only while iterating ready tasks.
• hermes_cli/kanban_db.py:7568-7639 dispatches status='review' cards in a separate loop via claim_review_task() without invoking that guard, so recent_success and active_pr cannot defer a review-card spawn.
• tests/hermes_cli/test_kanban_db.py:3962-3977 covers dispatching a review task when no ready tasks exist.

The ready-state dedicated-reviewer case noted in the follow-up discussion is distinct: this PR's status == "review" condition would not address it. Closing as implemented on main.