fix(kanban): recover blocked implementations into review

[manjaroblack]

What does this PR do?

When a Kanban implementation worker finishes a green, open, non-draft PR after the card has already entered blocked, kanban_complete is rejected as a terminal state. Manually unblocking the card leaves it in the ready implementation lane, where the active_pr respawn guard correctly refuses to spawn another implementer. The work then sits blocked with no path into independent review.

This change recovers that one state automatically: structured immutable PR evidence plus a live provider check (open, non-draft, exact head, required checks green) routes the card to the configured review lane. Free-form comments cannot promote. Missing, draft, closed, diverged, red, pending, or unknown evidence stays blocked with an actionable diagnostic. Ordinary unblock of a card that already has a PR still does not respawn the implementer. Same-card review and a pre-created downstream review child remain mutually exclusive. changes_requested rework on the existing PR is unchanged.

Related Issue

No existing issue covered this lifecycle hole. Opened as a focused bug fix rather than expanding #42896 (first-class request-review transition).

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_cli/kanban_review_recovery.py — immutable evidence parsing and live GitHub PR/check validation
• hermes_cli/kanban_db.py — blocked-completion recovery, graph-exclusive review routing, CAS/event audit, structured active_pr guard
• hermes_cli/kanban.py, tools/kanban_tools.py, agent/prompt_builder.py, hermes_cli/config_defaults.py — CLI, tool, prompt, and config surfaces
• tests/hermes_cli/test_kanban_blocked_review_recovery.py — positive recovery, negative evidence, lane exclusivity, rework, concurrency, and persistence
• website/docs/user-guide/features/kanban-worker-lanes.md — operator note for the recovery handoff

How to Test

1. Reproduce the prior hole: an implementation card is blocked, then later has structured review_evidence for an open non-draft PR whose exact head is green. Confirm the dispatcher now emits review_recovery_routed and does not spawn the implementer.
2. Negative cases: omit evidence, point at a draft/closed/diverged PR, or leave checks red/pending/unknown. Confirm the card stays blocked with review_recovery_blocked and an actionable reason.
3. Confirm a ready implementation card that already has a PR still hits active_pr after ordinary unblock.
4. Confirm same-card review and a pre-created downstream review child cannot both be selected.
5. Confirm changes_requested still returns the existing PR to the implementer without opening a second PR.

scripts/run_tests.sh \
  tests/hermes_cli/test_kanban_*.py \
  tests/tools/test_kanban_tools.py \
  tests/plugins/test_kanban_dashboard_plugin.py \
  tests/gateway/test_kanban_watchers_mixin.py \
  tests/gateway/test_kanban_notifier.py -q

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run the repository test wrapper (scripts/run_tests.sh) for the Kanban/CLI/gateway slice above
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Linux

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(kanban): recover blocked implementations into review

This is a substantial, well-structured change: the provider verification runs outside the write transaction (no board lock held during provider latency), the final status change is a compare-and-swap with an append-only review_recovery_routed event for idempotency, graph state is re-read under the writer lock, and the fail-closed timestamp parsing (_parse_finite_int_timestamp) is a good hardening. A few observations:

1. _REVIEW_CHILD_RE (\b(review|reviewer|quality|qa|release)\b) is an aggressive heuristic for lane selection. A child task whose title merely contains "release" (e.g. "release notes for task X") or an assignee whose name contains "qa" as a standalone word is classified as a review-shaped child, which flips the lane from same_card to downstream and changes whether the parent is completed or moved to review. Since a wrong lane here routes real work, consider preferring exact assignee/profile matches (e.g. hermes-review, reviewer) and treating the regex as a fallback, or narrowing the word list.

2. check_respawn_guard now scans the full comment history. The PR-comment query dropped the created_at >= ? (window) filter in favor of scanning all comments ORDER BY id ASC and filtering in Python (kanban_db.py). Correctness-wise this is needed to find the latest PR handoff timestamp, but for tasks with long comment histories every guard evaluation (per tick, per task) walks the entire thread. Consider bounding the SQL scan (e.g. only comments newer than pr_cutoff, with a fallback for the _review_rework_follows_pr_handoff case) to keep the hot path cheap.

3. Rate-limit cooldown: a NULL ended_at now blocks instead of allowing respawn. Previously the latest-run query filtered ended_at IS NOT NULL and skipped the cooldown branch for None; now _parse_finite_int_timestamp(None) returns None and the branch ended_at is None -> return "rate_limit_cooldown". If a rate_limit-outcome run can ever persist with ended_at unset, the task is now held in cooldown indefinitely (fail-closed, but a behavioral shift). Worth confirming rate_limit runs always receive ended_at, or treating None as elapsed for this specific case.

4. Minor: _latest_blocked_review_metadata treats the newest block event as authoritative and returns (None, "blocked_event") without falling through to older events or task_runs metadata — intentional per the comment, but a block transition that drops pre-existing evidence will orphan the older handoff until the card is re-blocked with evidence.

No blocking issues found.

[manjaroblack]

Addressed the non-blocking notes in 0f7298c:

1. Review-child detection now prefers exact assignees (hermes-review, reviewer) and the sdlc-review skill. The title/workflow fallback is only \b(review|reviewer)\b — release / qa / quality no longer flip the lane.
2. The active-PR comment scan is bounded to the 24h window in SQL. NULL / non-integer created_at rows are still included so malformed provenance fail-closes.
3. A rate_limited run with missing ended_at now allows a probe instead of parking forever. Future/numeric-malformed timestamps stay fail-closed.

Item 4 is unchanged: the newest block event remains authoritative so an unrelated later block cannot revive an older handoff.