fix(kanban): resume rework after retained PR

[MaximCrabbe]

Summary

• record the latest task-comment ID when changes_requested or review_reopened explicitly returns a reviewed task to implementation
• let ready-lane rework ignore only PR comments the reviewer had already seen, while any later PR comment still activates the duplicate-worker guard
• retain a conservative timestamp fallback for legacy events and fail closed for malformed, out-of-range, negative, future, or ambiguous data

Why

A same-card review can retain the original PR URL after request_changes. The task correctly returns to ready, but the existing active_pr respawn guard then treats that historical URL as evidence of duplicate work and prevents the original implementer from resuming.

Timestamp-only requeue exceptions are not sufficient because comments and lifecycle events have second-resolution timestamps. This change uses the monotonic, task-scoped comment ID as the causal boundary for new events, without changing the schema. Legacy events fall back to strict timestamp ordering.

Reproduction / behavior

Before this change:

1. claim an implementation task
2. add its PR URL as a task comment
3. request and claim same-card review
4. call request_changes
5. check_respawn_guard(..., lane="ready") returns active_pr

After this change, the ready retry is dispatchable to the original implementer. A PR comment added after the rework boundary returns active_pr again.

Verification

• scripts/run_tests.sh tests/hermes_cli/test_kanban_review_lifecycle_complete.py tests/hermes_cli/test_kanban_review_lifecycle.py -j 2 — 60 passed
• scripts/run_tests.sh tests/hermes_cli/test_kanban*.py tests/plugins/test_kanban_dashboard_plugin.py -j 8 — 262 passed, 1 Windows-only skip
• .venv/bin/ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_review_lifecycle_complete.py — passed
• .venv/bin/python -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_review_lifecycle_complete.py — passed
• git diff --check and added-lines security scan — passed
• ty check ... — no new diagnostics versus the exact base (same 7 pre-existing findings)
• clean git merge-tree --write-tree origin/main HEAD against current upstream main

Tested on Linux with Python 3.11.

Compatibility and scope

• no database migration or schema change
• modern lifecycle metadata is additive
• legacy rows remain supported conservatively
• existing parent gating, claim locking, stale-worker handling, review-lane behavior, and duplicate-worker protection remain covered

Related work

Related to #29458. This is narrower than the generic requeue/timestamp approaches in #76640, #76227, #72555, and #62393: it keys the exception specifically to review-to-rework lifecycle events, establishes same-second causal ordering with a comment-ID watermark, and validates malformed/legacy data fail closed.

[enzo-adami]

Independent revalidation on de315a9: 60/60 lifecycle tests pass through the repository runner, Ruff and the Windows-footgun checker pass, git diff --check is clean, and a virtual merge with current main 165c889 succeeds.

I reviewed the causal boundary specifically: the modern event stores the task-scoped max comment ID inside the same connection, same-second comments are ordered by ID, malformed/foreign/future/boolean markers fail closed, and any later PR comment restores active_pr. This remains distinct from worker lifecycle/artifact confinement in #86609/#86610. I did not find a counterexample in this scope; no extension PR is needed.

[Enough1122]

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

fix(kanban): resume rework after retained PR

1. hermes_cli/kanban_db.py (check_respawn_guard): the marker == 0 sub-check depends on the invariant that add_comment always emits a 'commented' task_event with id < the rework event. If that invariant ever changes (e.g., a comment inserted directly into task_comments), a 0 watermark becomes indistinguishable from "no comments" and the guard silently falls back to the legacy timestamp path. Worth asserting that invariant in the test suite.
2. hermes_cli/kanban_db.py: the watermark validation is intricate (bool/int discrimination, int64 bounds, cross-table existence checks) and the same "resolve reviewed_through from rework event" logic is now shared conceptually by request_changes and reopen_review_task. Extracting a small helper would make the fail-closed behavior easier to audit.
3. check_respawn_guard now issues up to 3 additional queries per call on what is likely a dispatch hot path — acceptable for SQLite, but they could be batched into a single query if this ever shows up in profiling. The behavior and the malformed/legacy/clock-regression tests look well-considered.