fix(kanban): preserve reviewer across re-review

hermes_cli/kanban_db.py

@@ -6116,10 +6116,11 @@ def request_review(
     """Transition implementation work into the first-class review phase.
 
     Unlike :func:`block_task`, this transition never touches block recurrence
-    accounting.  The current implementer and optional reviewer are recorded on
+    accounting.  The current implementer and resolved reviewer are recorded on
     the event so an autonomous reviewer can route requested changes back to the
-    right profile.  Supplying ``reviewer`` also reassigns the task before it is
-    exposed to the review dispatcher.
+    right profile.  Supplying ``reviewer`` reassigns the task before it is
+    exposed to the review dispatcher.  On re-review, omitting it reuses the
+    reviewer provenance persisted by the latest ``changes_requested`` event.
     """
     summary = redact_review_value(summary)
     metadata = redact_review_value(metadata)
@@ -6132,6 +6133,30 @@ def request_review(
         if trow is None:
             return False
         implementer = trow["assignee"]
+        if reviewer is None:
+            changes_event = conn.execute(
+                "SELECT payload FROM task_events "
+                "WHERE task_id = ? AND kind = 'changes_requested' "
+                "ORDER BY id DESC LIMIT 1",
+                (task_id,),
+            ).fetchone()
+            try:
+                changes_payload = (
+                    json.loads(changes_event["payload"])
+                    if changes_event and changes_event["payload"]
+                    else {}
+                )
+            except (json.JSONDecodeError, TypeError):
+                changes_payload = {}
+            prior_reviewer = (
+                changes_payload.get("reviewer")
+                if isinstance(changes_payload, dict)
+                else None
+            )
+            if changes_event is not None:
+                if not isinstance(prior_reviewer, str) or not prior_reviewer.strip():
+                    return False
+                reviewer = prior_reviewer
         reviewer = _canonical_assignee(reviewer) if reviewer is not None else None
         assignee_sql = ", assignee = ?" if reviewer is not None else ""
         params: tuple[Any, ...]
@@ -6264,6 +6289,11 @@ def request_changes(
         implementer = requested_payload.get("implementer")
         if not isinstance(implementer, str) or not implementer.strip():
             return False, "review handoff has no valid implementer provenance"
+        reviewer = task_row["assignee"]
+        if isinstance(reviewer, str) and reviewer.strip():
+            reviewer = _canonical_assignee(reviewer)
+        else:
+            reviewer = None
 
         new_status = _landing_status_after_parents(conn, task_id)
         cur = conn.execute(
@@ -6296,6 +6326,7 @@ def request_changes(
             {
                 "reason": reason,
                 "implementer": implementer,
+                "reviewer": reviewer,
                 "status": new_status,
             },
             run_id=run_id,

tests/hermes_cli/test_kanban_review_lifecycle_complete.py

@@ -112,8 +112,10 @@ def test_same_card_review_supports_changes_and_approval_without_block_loop(conn)
     assert rework.assignee == "builder"
     assert rework.current_run_id is None
     changes = _event(kb.list_events(conn, task_id), "changes_requested")
+    assert changes.payload is not None
     assert changes.payload["reason"] == "Add a regression for the fallback branch."
     assert changes.payload["implementer"] == "builder"
+    assert changes.payload["reviewer"] == "reviewer"
     _run(kb.list_runs(conn, task_id), "changes_requested")
 
     implementation_2 = kb.claim_task(conn, task_id, claimer="builder:2")
@@ -121,12 +123,19 @@ def test_same_card_review_supports_changes_and_approval_without_block_loop(conn)
     assert kb.request_review(
         conn,
         task_id,
-        reviewer="reviewer",
         summary="Fallback regression added.",
         expected_run_id=implementation_2.current_run_id,
     )
+    awaiting_rereview = kb.get_task(conn, task_id)
+    assert awaiting_rereview is not None
+    assert awaiting_rereview.status == "review"
+    assert awaiting_rereview.assignee == "reviewer"
     review_2 = kb.claim_review_task(conn, task_id, claimer="reviewer:2")
     assert review_2 is not None
+    assert review_2.assignee == "reviewer"
+    review_run = kb.latest_run(conn, task_id)
+    assert review_run is not None
+    assert review_run.profile == "reviewer"
     assert kb.complete_task(
         conn,
         task_id,
@@ -140,6 +149,53 @@ def test_same_card_review_supports_changes_and_approval_without_block_loop(conn)
     assert completed.block_recurrences == 0
 
 
+@pytest.mark.parametrize("bad_payload", ["{not-json", "{}"])
+def test_rereview_requires_explicit_reviewer_when_provenance_is_invalid(
+    conn,
+    bad_payload: str,
+) -> None:
+    task_id, review = _claimed_review(conn, "Malformed reviewer provenance")
+    assert kb.request_changes(
+        conn,
+        task_id,
+        reason="Correct the implementation.",
+        expected_run_id=review.current_run_id,
+    ) == (True, "builder")
+    with kb.write_txn(conn):
+        conn.execute(
+            "UPDATE task_events SET payload = ? "
+            "WHERE id = (SELECT id FROM task_events "
+            "WHERE task_id = ? AND kind = 'changes_requested' "
+            "ORDER BY id DESC LIMIT 1)",
+            (bad_payload, task_id),
+        )
+
+    implementation = kb.claim_task(conn, task_id, claimer="builder:retry")
+    assert implementation is not None
+    assert not kb.request_review(
+        conn,
+        task_id,
+        summary="Corrected implementation.",
+        expected_run_id=implementation.current_run_id,
+    )
+    unchanged = kb.get_task(conn, task_id)
+    assert unchanged is not None
+    assert unchanged.status == "running"
+    assert unchanged.assignee == "builder"
+
+    assert kb.request_review(
+        conn,
+        task_id,
+        reviewer="reviewer",
+        summary="Corrected implementation.",
+        expected_run_id=implementation.current_run_id,
+    )
+    restored = kb.get_task(conn, task_id)
+    assert restored is not None
+    assert restored.status == "review"
+    assert restored.assignee == "reviewer"
+
+
 def test_review_changes_reapply_parent_gate(conn):
     parent_id = kb.create_task(conn, title="Upstream prerequisite", assignee="planner")
     task_id = kb.create_task(