Merge pull request #78 from eduralph/feat/75-neutral-agents

feat(agents): project-neutral prompts + reviewer target access

Author: eduralph

copier.yml

@@ -126,6 +126,19 @@ default_branch:
   help: Default branch fixes target unless an area rule overrides
   default: main
 
+# How this project contributes (issue #75). "fork": contribute upstream through a
+# fork (upstream/maintenance-line targeting, cross-version cherry-picks) — the agents
+# cite docs/fork-discipline.md. "own-repo": this repo IS the target (branch from the
+# default branch, no upstream/fork) — the agents drop the fork doctrine and defer
+# targeting to docs/INTEGRATION.md §2. Default "fork" preserves prior behavior.
+contribution_model:
+  type: str
+  help: How this project contributes — own repo (branch from default) or a fork (upstream line)
+  choices:
+    Fork (contribute upstream via a fork): fork
+    Own repo (this repo is the target): own-repo
+  default: fork
+
 # ----------------------------------------------------------------------------
 # Integration: bundle + process layout (docs 05 item 7)
 # ----------------------------------------------------------------------------

docs/05-check.md

@@ -97,6 +97,13 @@ re-checks that cited `path:line`s exist on the target branch, and flags scope
 creep. Its output, `check-review.md`, is **advisory** — it annotates, it never
 gates. The blocking path contains no LLM at all.
 
+The reviewer runs in an isolation sandbox (only `{patch.diff, brief.md,
+check-gates.json}` are present), so the driver resolves the brief's target checkout and
+hands it over as **`$PDCA_TARGET`** (read-only; for a `claude` reviewer also via
+`--add-dir`). The reviewer grounds every citation there and is told **not** to wander
+into other checkouts on the machine — without this it can't ground, or hunts the
+filesystem for "the target" (issue #75).
+
 ## 3. Assembly — the SUMMARY the human signs
 
 The driver folds brief + gates + review into `SUMMARY.md`, a 10-section document.

template/.claude/agents/builder.md.jinja

@@ -45,10 +45,13 @@ failing gate. Address it; do **not** re-submit the rejected approach unchanged.
 
 Cite `path:line` on the target branch for every claim and change.
 
-Write the patch against the brief's **target branch** and follow
-`docs/fork-discipline.md`: a cross-version cherry-pick must *remain correct* on the
-target, not just apply cleanly (§3); ship the test in the location the target branch
-uses (§3); make the patch commit-ready for the target's own hooks (§4).
+Write the patch against the brief's **target branch** (targeting resolved at Plan per
+`docs/INTEGRATION.md` §2). Ship the test in the location the target uses, and make the
+patch commit-ready for the target's own commit hooks (formatter / linters).
+{% if contribution_model == "fork" %}
+This is a fork contribution — also follow `docs/fork-discipline.md`: a cross-version
+cherry-pick must *remain correct* on the target, not just apply cleanly (§3).
+{% endif %}
 
 **When you reject an alternative on cost, show the cost** — a diff sketch or a concrete
 line count someone can check, never an adjective ("heavier", "larger", "touches every
@@ -69,13 +72,13 @@ ad-hoc test command, or similar): it has **no timeout**, so a hung test blocks
 the whole Do beat forever.
 
 Do **not** assume the runner gives you a display, GUI, or other rich runtime —
-many are **headless**. If your test imports a heavy module (a GUI toolkit, a
+many are **headless**. If your test pulls in a heavy dependency (a GUI toolkit, a
 display/IO-bound library, …) **at load time**, a headless runner can crash on
-import — and it recurs on every iterate-do until the test stops importing it.
-Keep the unit under test import-light: extract the logic into a module free of
-those heavy imports and test *that*. Check what the
-runner actually provides (`pdca.toml`, `docs/INTEGRATION.md`) rather than assuming
-— an inaccurate belief about the environment is what makes a test crash silently.
+load — and it recurs on every iterate-do until the test stops pulling it in.
+Keep the unit under test load-light: extract the logic into a unit free of those
+heavy dependencies and test *that*. Check what the runner actually provides
+(`pdca.toml`, `docs/INTEGRATION.md`) rather than assuming — an inaccurate belief
+about the environment is what makes a test crash silently.
 This pre-fix/post-fix check is a fast sanity pass (Check's gates re-run the real
 suite), so a single quick run through the wrapper is enough.
 

template/.claude/agents/planner.md.jinja

@@ -33,9 +33,12 @@ Either way the output file is `brief.md`, and you must keep the parsed
 `- **Label:** value` lines (the driver reads the spec from them). The load-bearing
 field is the **success criterion** — the sentence Check later tests "did this work"
 against. Resolve the **repo + branch target** here, and state **scope / out of
-scope** so Do can't sprawl. Follow `docs/fork-discipline.md` for the targeting and
-prior-art rules — fixes ride the current maintenance line (§3); run the prior-art
-check by **affected file path**, across merged history *and* closed/rejected PRs (§5).
+scope** so Do can't sprawl. Resolve targeting per `docs/INTEGRATION.md` §2, and run the
+prior-art check by **affected file path**, across merged history *and* closed/rejected work.
+{% if contribution_model == "fork" %}
+This is a fork contribution — follow `docs/fork-discipline.md` for the fork targeting +
+prior-art rules: fixes ride the current maintenance line (§3); the check spans upstream PRs (§5).
+{% endif %}
 
 **One issue or several (batch).** You run from the project root; your prompt names
 where to write. For a single issue it gives one bundle directory — write the one

template/.claude/agents/publisher.md.jinja

@@ -58,10 +58,14 @@ not push, branch, or open a PR yourself.**
   `cd <checkout> && …` (`git -C` is the safe idiom).
 - Resolve the branch target per INTEGRATION §2. One logical fix per PR; do not invent
   scope the brief didn't accept.
-- Follow `docs/fork-discipline.md` §1–§2: the contribution branches from
-  `upstream/<base>` (not the fork's drifted branch), the PR is **draft-only** and the
-  human marks it ready, and the deterministic `pdca publish` performs the push. Write
-  the commit-msg/PR prose to match the target; do not push or open the PR yourself.
+- The contribution branches from the brief's **target branch** (per INTEGRATION §2),
+  the PR is **draft-only** and the human marks it ready, and the deterministic `pdca
+  publish` performs the push. Write the commit-msg/PR prose to match the target; do not
+  push or open the PR yourself.
+{% if contribution_model == "fork" %}
+- This is a fork contribution — branch from `upstream/<base>` (not the fork's drifted
+  branch); see `docs/fork-discipline.md` §1–§2.
+{% endif %}
 
 ## Boundaries
 

template/.claude/agents/reviewer.md.jinja

@@ -32,8 +32,11 @@ physically cannot patch what you judge.
 
 - Re-run the asserted evidence: stash the fix → confirm red; unstash → confirm
   green. Re-run the validator/scanners yourself. Trust re-runs, not claims.
-- Re-check that every cited `path:line` grounds on the target branch; drop
-  findings that do not ground.
+- Re-check that every cited `path:line` grounds on the **target source at
+  `$PDCA_TARGET`** (read-only; the driver resolves it from the brief's target and adds
+  it for you). Ground only there — do **not** wander into other checkouts on the
+  machine; if `$PDCA_TARGET` is unset, ground against `patch.diff` alone. Drop findings
+  that do not ground.
 - Emit per item `PASS / FAIL / NEEDS-HUMAN` + one-line rationale + path:line.
 
 ## Always emit the complete 5/5/1 verdict table
@@ -69,17 +72,21 @@ upstream-isn't-ahead; scope-creep / Plan re-entry; visual / manual-repro
 outcomes; and the project's enumerated human-only items (INTEGRATION.md §4).
 Each becomes a `- [ ]` row in `SUMMARY.md` §6 the human must clear.
 
-Judge the contribution against `docs/fork-discipline.md`: a cross-version
-cherry-pick that *applies cleanly* is not necessarily *correct* on the target (§3);
-validation must be against the **clean upstream** target, not the fork's branches
-(§4); and confirm the prior-art check ran by file path (§5). Where these can't be
-mechanically settled, raise them NEEDS-HUMAN.
+Confirm the prior-art check ran by **affected file path** (merged history + closed/
+rejected work); where it can't be mechanically settled, raise it NEEDS-HUMAN.
+{% if contribution_model == "fork" %}
+This is a fork contribution — judge it against `docs/fork-discipline.md`: a
+cross-version cherry-pick that *applies cleanly* is not necessarily *correct* on the
+target (§3), and validation must be against the **clean upstream** target, not the
+fork's drifted branches (§4). Raise NEEDS-HUMAN where these can't be mechanically settled.
+{% endif %}
 
 ### C5 symptom-guard smell-test
 
 The "contested symptom-vs-root-cause" trigger above has a concrete detection rule —
 apply it to `patch.diff` every cycle. If the fix adds a **capability probe** (a
-capability check, `hasattr`, `try: import …`) or a **runtime guard** *inside code that
+feature/attribute check, or a try-it-and-fall-back around an optional capability —
+e.g. in Python `hasattr` / `try: import …`) or a **runtime guard** *inside code that
 is meant to run with that capability present* — the guard protects a path that, by
 design, only executes when the capability exists — flag C5 **NEEDS-HUMAN** and ask in
 the basis: can the eager / load-time cause be removed instead (e.g. compute lazily on

template/AGENTS.md.jinja

@@ -17,8 +17,10 @@ driver enforces this by not passing the file; do not ask for it.
 
 - Re-run the asserted evidence: stash the fix → confirm red; unstash → confirm
   green. Re-run the validator / scanners yourself. Trust re-runs, not claims.
-- Re-check that every `path:line` the patch cites resolves on the target branch;
-  drop any finding that does not ground.
+- Re-check that every `path:line` the patch cites resolves on the **target source at
+  `$PDCA_TARGET`** (the driver resolves it from the brief's target). Ground only there —
+  do **not** search other checkouts on the machine; if `$PDCA_TARGET` is unset, ground
+  against `patch.diff` alone. Drop any finding that does not ground.
 - Emit per item `PASS / FAIL / NEEDS-HUMAN` + a one-line rationale + a path:line.
   No free-form prose verdict.
 - You have **execute** access (run tests/validator, git stash/unstash) and **no
@@ -30,7 +32,8 @@ These are structurally undecidable from the artifacts — flag them, don't guess
 
 - Validation fitness-to-purpose ("is this the right thing at all").
 - Symptom-vs-root-cause when the bug's mechanism is contested.
-- Upstream-isn't-ahead *semantic* match (does an open rewrite PR supersede this).
+- Superseded-by: does other open work *semantically* supersede this (e.g. an upstream
+  rewrite PR, or another in-flight change)?
 - Scope-creep / Plan re-entry calls (diff exceeds the brief's scope but looks plausible).
 - Visual sign-off / manual-repro outcomes.
 - The project's enumerated human-only items (INTEGRATION.md §4). **TODO: list them.**

template/docs/fork-discipline.md.jinja

@@ -1,5 +1,12 @@
 # Fork & contribution discipline
 
+{% if contribution_model != "fork" %}
+> **This project renders as `own-repo`** (this repo IS the contribution target, not a
+> fork). The agents do **not** cite this file — they resolve targeting from
+> `INTEGRATION.md` §2. It is kept as a reference for the fork model; ignore it unless
+> you switch `contribution_model` to `fork`.
+
+{% endif %}
 > The generic rules for contributing a fix upstream through a fork — the **Check
 > closing / contribution** discipline (the counterpart to [`principles.md`](principles.md),
 > which governs Plan-time solution design). These rules are project-agnostic; **your

template/src/pdca_harness/leaves.py

@@ -67,6 +67,8 @@ def _invoke(
     label: str = "",
     status=None,
     stream_json: bool = False,
+    env: dict | None = None,
+    extra_argv: list[str] | None = None,
 ) -> None:
     """Run the leaf's configured command in ``workdir``, feeding it ``prompt``.
 
@@ -83,9 +85,10 @@ def _invoke(
     leaf is using right now. Ignored for non-claude families (e.g. a codex reviewer),
     which don't speak that format.
     """
-    argv = list(leaf.argv)
+    argv = list(leaf.argv) + list(extra_argv or [])
+    run_env = {**os.environ, **env} if env else None
     if leaf.interactive:
-        subprocess.run(argv + [prompt], cwd=workdir)
+        subprocess.run(argv + [prompt], cwd=workdir, env=run_env)
         return
     # Headless: feed the prompt on stdin (a trailing positional would be swallowed
     # by a variadic --allowedTools) and tick a heartbeat, since `claude -p` prints
@@ -95,7 +98,7 @@ def _invoke(
         argv += ["--output-format", "stream-json", "--verbose"]
     rc, _ = progress.run_with_heartbeat(
         argv, cwd=workdir, input_text=prompt, label=label, status=status,
-        stream_json=use_stream)
+        stream_json=use_stream, env=run_env)
     if rc != 0:
         raise subprocess.CalledProcessError(rc, argv)
 
@@ -358,10 +361,33 @@ def reviewer_input_paths(d: Path) -> list[Path]:
     "you can). Emit NEEDS-HUMAN for the always-human items (validation "
     "fitness-to-purpose, contested root-cause, ambiguous scope) — each NEEDS-HUMAN "
     "row becomes a §6 item the human must clear. Do not omit a row; use N/A with a "
-    "reason when an element does not apply."
+    "reason when an element does not apply. "
+    "Ground every cited path:line on the target source at $PDCA_TARGET (read-only); "
+    "if $PDCA_TARGET is unset, ground against patch.diff alone — do NOT search other "
+    "checkouts on the machine."
 )
 
 
+def _reviewer_target(d: Path, cfg: Config) -> Path | None:
+    """The local target checkout the reviewer grounds its citations on, or None (#75).
+
+    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.
+    """
+    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
+    except Exception:  # noqa: BLE001 — grounding access is best-effort, never fatal
+        return None
+
+
 def run_review(d: Path, cfg: Config) -> None:
     inputs = reviewer_input_paths(d)
     assert (d / "build-notes.md") not in inputs, "independence contract violated"
@@ -385,12 +411,20 @@ def _run_review_sandboxed(d: Path, cfg: Config) -> None:
             src = d / name
             if src.exists():
                 shutil.copy2(src, sandbox / name)
+        # Ground citations on the brief's target checkout (#75): name it via $PDCA_TARGET
+        # so the reviewer doesn't wander into unrelated checkouts, and grant read access
+        # for the claude family (--add-dir). Independence holds — the target is the
+        # upstream source, not build-notes.md.
+        target = _reviewer_target(d, cfg)
+        env = {"PDCA_TARGET": str(target)} if target else None
+        extra_argv = ["--add-dir", str(target)] if target and cfg.reviewer.family == "claude" else None
         try:
             _invoke(
                 cfg.reviewer, sandbox, _REVIEW_PROMPT,
                 label=f"Check review {d.name}",
                 status=lambda: progress.bundle_activity(sandbox, ("check-review.md",)),
                 stream_json=True,  # Tier 3 (no-op unless the reviewer family is claude)
+                env=env, extra_argv=extra_argv,
             )
         except Exception as exc:  # a failed reviewer (e.g. dropped connection) must
             _review_unavailable(d, f"reviewer leaf failed: {exc}")  # not crash the cycle

template/tests/test_driver_slice.py

@@ -224,6 +224,39 @@ def test_config_close_class(self) -> None:
         self.assertEqual(self.cfg.close_class("likely-close"), "")
 
 
+class ReviewerTargetAccess(unittest.TestCase):
+    """The reviewer grounds citations on the brief's target checkout via $PDCA_TARGET
+    (issue #75) — single-sourced from the brief, so it doesn't wander into other
+    checkouts on the machine."""
+
+    def setUp(self) -> None:
+        self.tmp = Path(tempfile.mkdtemp())
+        self.cfg = _stub_config(self.tmp)
+        self.d = self.cfg.bundle("T")
+        self.d.mkdir(parents=True)
+        (self.d / "brief.md").write_text(
+            "- **Slug:** x\n- **Repo + branch target:** org/myrepo @ main\n", encoding="utf-8")
+
+    def tearDown(self) -> None:
+        shutil.rmtree(self.tmp, ignore_errors=True)
+
+    def test_resolves_target_checkout_when_present(self) -> None:
+        self.cfg.repo_checkouts = {"org/myrepo": "target"}  # → <root>/target
+        self.assertIsNone(leaves._reviewer_target(self.d, self.cfg))  # not on disk yet
+        (self.cfg.root / "target").mkdir()
+        self.assertEqual(leaves._reviewer_target(self.d, self.cfg),
+                         (self.cfg.root / "target").resolve())
+
+    def test_no_target_when_brief_has_no_target(self) -> None:
+        (self.d / "brief.md").write_text("- **Slug:** x\n", encoding="utf-8")
+        self.assertIsNone(leaves._reviewer_target(self.d, self.cfg))
+
+    def test_review_prompt_grounds_on_pdca_target(self) -> None:
+        # The prompt names $PDCA_TARGET and forbids wandering to other checkouts.
+        self.assertIn("$PDCA_TARGET", leaves._REVIEW_PROMPT)
+        self.assertIn("do NOT search other checkouts", leaves._REVIEW_PROMPT)
+
+
 class AdvisoryReviewResilience(unittest.TestCase):
     """A failed/interrupted reviewer must degrade to a §6 NEEDS-HUMAN, never crash
     the deterministic spine (the review is advisory, not a gating artifact)."""