fix: the session reviewer was shown an empty diff, always

src/agent_harness/session_executor.py

@@ -634,7 +634,7 @@ def _execute(self, record: WorkRecord) -> Outcome:
                         record, "draft_pr_opened", detail=outcome.pr_url, session_id=session.id
                     )
 
-            verdict_text = self._review(record, tree, True, "")
+            verdict_text = self._review(record, tree, True, "", base=base)
             outcome.stages.append("review")
             verdict = APPROVED if verdict_text.strip().upper().startswith("APPROVED") else REJECTED
             outcome.verdict = verdict
@@ -744,13 +744,26 @@ def _on_waiting(self, record: WorkRecord, session: Session) -> None:
             url=url,
         )
 
-    def _review(self, record: WorkRecord, tree: Path, passed: bool, failure: str) -> str:
+    def _review(
+        self, record: WorkRecord, tree: Path, passed: bool, failure: str, base: str = ""
+    ) -> str:
         if self.reviewer is None:
             # Checked before the diff is computed: there is no point building
             # a diff nobody will read. Says so rather than silently treating
             # unreviewed work as approved.
             return "REJECTED\nNo reviewer is configured, so nothing has reviewed this."
-        diff = run_git(tree, "diff", "HEAD")
+        # Against the BASE, not the working tree. `git diff HEAD` answers
+        # "what is uncommitted?", and by the time a reviewer is called the
+        # answer is always "nothing": the checkpoint before the expensive
+        # gate has just committed all of it. So every session-mode reviewer
+        # was shown an empty diff, and said the only correct thing about one:
+        #
+        #   "The supplied diff is empty, so it demonstrates none of that and
+        #    cannot be judged as satisfying the request."
+        #
+        # Measured on a real 48-line change that had already passed its
+        # checks. No session-mode item could ever have been approved.
+        diff = run_git(tree, "diff", f"{base}...HEAD") if base else run_git(tree, "diff", "HEAD")
         prompt = REVIEW_PROMPT.format(
             brief=record.brief,
             diff=diff[:20000],

tests/test_reviewer.py

@@ -98,3 +98,73 @@ def test_the_prompt_demands_evidence_not_just_a_verdict() -> None:
     assert "What I could not verify" in REVIEW_PROMPT
     assert "Assume it is wrong" in REVIEW_PROMPT
     assert "cannot name any, that is a REJECTED" in REVIEW_PROMPT
+
+
+def test_the_reviewer_sees_the_work_after_the_checkpoint_committed_it(tmp_path: Any) -> None:
+    """The measured bug: every session-mode reviewer was shown an empty diff.
+
+    The checkpoint before the expensive gate commits the work — deliberately,
+    so a worker killed during review does not lose what passed the cheap
+    gates. `git diff HEAD` then answers "what is uncommitted?", and the
+    answer is always "nothing". So the reviewer received an empty diff and
+    said the only correct thing about one:
+
+        "The supplied diff is empty, so it demonstrates none of that and
+         cannot be judged as satisfying the request."
+
+    Measured on a real 48-line change that had already passed its checks. No
+    session-mode item could ever have been approved.
+    """
+    import subprocess
+
+    from agent_harness.session_executor import SessionExecutor
+    from agent_harness.work import WorkQueue, WorkRecord
+
+    tree = tmp_path / "repo"
+    tree.mkdir()
+    for argv in (
+        ["init", "-q", "-b", "main"],
+        ["config", "user.email", "t@t"],
+        ["config", "user.name", "t"],
+    ):
+        subprocess.run(["git", "-C", str(tree), *argv], check=True, capture_output=True)
+    (tree / "hello.txt").write_text("hello world\n")
+    subprocess.run(["git", "-C", str(tree), "add", "-A"], check=True, capture_output=True)
+    subprocess.run(
+        ["git", "-C", str(tree), "commit", "-q", "-m", "base"], check=True, capture_output=True
+    )
+    # The item's own branch, as the executor cuts one, then the agent's work
+    # committed onto it by the pre-review checkpoint.
+    subprocess.run(
+        ["git", "-C", str(tree), "checkout", "-q", "-b", "harness/t1"],
+        check=True,
+        capture_output=True,
+    )
+    (tree / "hello.txt").write_text("hello harness\n")
+    subprocess.run(["git", "-C", str(tree), "add", "-A"], check=True, capture_output=True)
+    subprocess.run(
+        ["git", "-C", str(tree), "commit", "-q", "-m", "checkpoint"],
+        check=True,
+        capture_output=True,
+    )
+
+    seen: dict[str, str] = {}
+
+    class Reviewer:
+        def call(self, _role: str, messages: Any, **_: Any) -> Any:
+            seen["prompt"] = messages[-1]["content"]
+            body = {"choices": [{"message": {"content": "APPROVED\nok"}}]}
+            return type("R", (), {"body": body})()
+
+    queue = WorkQueue(str(tmp_path / "w.sqlite"))
+    executor = SessionExecutor(
+        queue,
+        type("Host", (), {})(),
+        tree,
+        reviewer=Reviewer(),  # type: ignore[arg-type]
+    )
+
+    executor._review(WorkRecord(item_id="T1", title="t", brief="b"), tree, True, "", base="main")
+
+    assert "hello harness" in seen["prompt"], "the reviewer was not shown the committed work"
+    assert "-hello world" in seen["prompt"], "nor what it replaced"