feat(reviewer): ground on the per-cycle worktree, not a stale sibling checkout (#120)

The reviewer grounded citations on $PDCA_TARGET = the human's sibling working checkout,
which can lag origin/<base> (→ a false-blocking "patch cannot apply" C4, although the
patch applied cleanly to the base the gates actually used) or be sandbox-unreadable
(→ no target grounding). The gates and builder already run off a fresh per-cycle
worktree pinned to origin/<base>; the reviewer didn't.

Make _reviewer_target prefer the per-cycle worktree (#94) — it is fetched + pinned to
<base_remote>/<base> and carries the patch, so the reviewer grounds on the SAME base the
gates ran against. When worktree isolation is off / the target isn't a git checkout, fall
back to the sibling checkout but `git fetch` it first so a lagging sibling can't drift the
grounding — refs only, NEVER reset/checkout the human's working tree. Both the main
reviewer and the advisory leaves (#64) route through _reviewer_target, so both are fixed.

Tests (ReviewerTargetAccess): prefers the worktree when present; the sibling fallback
fetches without mutating the working tree (real git — uncommitted + untracked work
preserved). Full offline suite: 197 OK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Eduard Ralph <15236434+eduralph@users.noreply.github.com>

Author: eduralph

template/src/pdca_harness/leaves.py

@@ -386,21 +386,34 @@ def reviewer_input_paths(d: Path) -> list[Path]:
 
 
 def _reviewer_target(d: Path, cfg: Config) -> Path | None:
-    """The local target checkout the reviewer grounds its citations on, or None (#75).
+    """The local target checkout the reviewer grounds its citations on, or None (#75/#120).
 
-    Single-sourced from the brief's "Repo + branch target" via the same resolution
-    publish uses (``_checkout_path`` — configured ``[publisher.checkouts]`` or the
-    sibling convention). Returned only if it exists on disk; the reviewer is told to
-    ground against ``$PDCA_TARGET`` and not to wander into other checkouts. Best-effort:
-    any failure (no target, unresolved) yields None and the reviewer falls back to the diff.
+    Prefer the per-cycle **worktree** (#94): it is fetched + pinned to
+    ``<base_remote>/<base>`` and carries the patch, so the reviewer grounds on the *same*
+    base the gates ran against — not the human's sibling working checkout, which can lag
+    ``origin/<base>`` (a false "patch cannot apply" C4) or be sandbox-unreadable (#120).
+
+    When no worktree exists (isolation off / non-git target), fall back to the resolved
+    sibling checkout — but first ``git fetch`` it so grounding sees the current base. The
+    fetch is **non-destructive** (refs only): never ``reset``/``checkout`` the human's
+    working tree. Best-effort: any failure yields None and the reviewer grounds on the diff.
     """
+    wt = worktree.path(d, cfg)
+    if wt is not None:
+        return wt
     from . import publish  # lazy: publish imports leaves, avoid an import cycle
     try:
         repo_spec, _base, _slug = publish._resolve_target(d)
         if not repo_spec:
             return None
         p = publish._checkout_path(cfg, repo_spec)
-        return p if p.exists() else None
+        if not p.exists():
+            return None
+        # Refresh refs so a lagging sibling doesn't drift the reviewer's grounding; do NOT
+        # touch the working tree (it is the human's checkout). Best-effort.
+        subprocess.run(["git", "-C", str(p), "fetch", cfg.base_remote],
+                       capture_output=True, text=True)
+        return p
     except Exception:  # noqa: BLE001 — grounding access is best-effort, never fatal
         return None
 

template/tests/test_driver_slice.py

@@ -15,6 +15,7 @@
 import tempfile
 import unittest
 from pathlib import Path
+from unittest import mock
 
 from pdca_harness import act, assemble, driver, gates, publish, queue, leaves, signoff, state
 from pdca_harness.config import DEFAULT_CLOSE_DISPOSITIONS, Config, LeafConfig
@@ -256,6 +257,37 @@ def test_review_prompt_grounds_on_pdca_target(self) -> None:
         self.assertIn("$PDCA_TARGET", leaves._REVIEW_PROMPT)
         self.assertIn("do NOT search other checkouts", leaves._REVIEW_PROMPT)
 
+    def test_prefers_worktree_over_sibling(self) -> None:
+        # When a per-cycle worktree exists it is the grounding target — pinned to the gate
+        # base + patch applied — never the human's (possibly stale) sibling checkout (#120).
+        wt = self.tmp / "checkout.pdca-wt"
+        wt.mkdir()
+        with mock.patch.object(leaves.worktree, "path", return_value=wt):
+            self.assertEqual(leaves._reviewer_target(self.d, self.cfg), wt)
+
+    def test_sibling_fallback_fetches_without_touching_working_tree(self) -> None:
+        # Worktree off: ground on the sibling, but only `git fetch` it — NEVER reset or
+        # checkout the human's working tree (#120). Real git, no network.
+        self.cfg.worktree = False
+        self.cfg.base_remote = "origin"
+        origin = self.tmp / "origin.git"
+        repo = self.tmp / "myrepo"
+        subprocess.run(["git", "init", "-q", "--bare", str(origin)], check=True)
+        subprocess.run(["git", "clone", "-q", str(origin), str(repo)], check=True)
+        run = lambda *a: subprocess.run(["git", "-C", str(repo), *a], check=True,
+                                        capture_output=True)
+        run("config", "user.email", "t@e.com"); run("config", "user.name", "T")
+        (repo / "f.txt").write_text("base\n", encoding="utf-8")
+        run("add", "-A"); run("commit", "-q", "-m", "base"); run("branch", "-M", "main")
+        run("push", "-q", "-u", "origin", "main")
+        (repo / "f.txt").write_text("human edit\n", encoding="utf-8")   # uncommitted work
+        (repo / "untracked.txt").write_text("x\n", encoding="utf-8")
+        self.cfg.repo_checkouts = {"org/myrepo": str(repo)}
+        got = leaves._reviewer_target(self.d, self.cfg)
+        self.assertEqual(got, repo)
+        self.assertEqual((repo / "f.txt").read_text(encoding="utf-8"), "human edit\n")
+        self.assertTrue((repo / "untracked.txt").exists())   # fetch is refs-only
+
 
 class AdvisoryReviewResilience(unittest.TestCase):
     """A failed/interrupted reviewer must degrade to a §6 NEEDS-HUMAN, never crash