From af57e18dd1bfaa8ed17f76012d1a5a5a66154d63 Mon Sep 17 00:00:00 2001 From: Topi Jarvinen Date: Sun, 26 Jul 2026 11:13:12 +0300 Subject: [PATCH] docs: the panel's isolation must be of the repo under review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract item 7 said "give each lens a scratch copy or its own git worktree". It did not say *of what*, and the omission bit on the first real use. On the OpenKitchen upgrade both lenses were handed a worktree of the KIT while reviewing the ADOPTER — a runtime's built-in isolation clones the session's repo, which is not the target when the cockpit is working on a different one. `git diff ...HEAD` was empty in both worktrees. Both lenses noticed and cloned the real target themselves, so that review was sound. But a lens that did not would have reviewed an empty diff and reported all-clear, and nothing downstream distinguishes that from a genuine clean pass — the worst failure available to a review mechanism, because it produces a receipt for a review that never happened. Two changes: - item 7 now says isolation must be of the repo UNDER REVIEW, tells you to name the target explicitly, and warns against asserting in the launch prompt that isolation is already arranged. That assertion was made on the run that found this, and it was false. - "Running it" gains a step: confirm each lens reviewed the right code before reading its findings. A zero-finding report is evidence only once you know what was in front of the lens — which is contract item 8 ("state what was verified clean, and how") applied to the operator rather than to the lens. Regenerated kit-manifest.json for the changed doc hash. Claude-Session: https://claude.ai/code/session_01V27wAgECMEApqAvmJMFyqm --- docs/agentic-dev-kit/fallback-review-panel.md | 44 ++++++++++++++----- kit-manifest.json | 2 +- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/docs/agentic-dev-kit/fallback-review-panel.md b/docs/agentic-dev-kit/fallback-review-panel.md index d84672c..acfb410 100644 --- a/docs/agentic-dev-kit/fallback-review-panel.md +++ b/docs/agentic-dev-kit/fallback-review-panel.md @@ -68,13 +68,28 @@ author re-reading their own diff. before believing a kill. 6. **Report, do not fix.** A lens that edits loses the disjointness: it starts defending its own changes on the next round. -7. **Mutate in an isolated copy, never the shared tree.** Mutation testing needs - temporary writes, and lenses run concurrently. Discovered the hard way on the - PR that added this file: one lens's mutations appeared to the other as an - external process corrupting the repo, and it "restored" them mid-run — so one - lens's results were unreliable and the other nearly destroyed live work. Give - each lens a scratch copy or its own git worktree, and require it to leave the - shared tree byte-identical. +7. **Mutate in an isolated copy of the repo under review, never the shared + tree.** Mutation testing needs temporary writes, and lenses run concurrently. + Discovered the hard way on the PR that added this file: one lens's mutations + appeared to the other as an external process corrupting the repo, and it + "restored" them mid-run — so one lens's results were unreliable and the other + nearly destroyed live work. Give each lens a scratch copy or its own git + worktree, and require it to leave the shared tree byte-identical. + + **Of the repo under review — check, don't assume.** A runtime's built-in + isolation usually clones *the session's* repo, which is not the target when + the cockpit is reviewing a different one (an adopter repo, a sibling + checkout). On the OpenKitchen upgrade both lenses were handed a worktree of + the *kit* while reviewing the *adopter*: `git diff ...HEAD` was empty in + both. Both noticed and cloned the real target themselves — but a lens that + did not would have reviewed an empty diff and reported all-clear, which is + the worst failure available to a review mechanism. + + So: name the target repo explicitly in the launch prompt, tell the lens to + **verify it is looking at the right thing before reviewing** (a non-empty diff + with the expected head), and never state in the prompt that isolation has + already been arranged unless you have confirmed it. The launch prompt on that + run asserted "you are in an isolated worktree of that repo" and was wrong. 8. **State what was verified clean, and how.** Absence of findings is only evidence if you know what was actually checked. @@ -84,12 +99,17 @@ author re-reading their own diff. 2. Launch **one isolated reviewer per lens**, concurrently, each with the contract above and its lens focus. Use whatever isolation your runtime has (a subagent, a separate session, a second person). If it has none, see - *Degraded mode*. -3. Triage every finding against the *current* code — some go stale across + *Degraded mode*. State the **repo and branch under review** explicitly — see + contract item 7 on why the runtime's own isolation may not point at it. +3. **Confirm each lens reviewed the right code** before you read its findings: a + lens reporting a clean pass over an empty or wrong diff looks exactly like a + lens reporting a clean pass. A finding count of zero is a result only once you + know what was in front of it. +4. Triage every finding against the *current* code — some go stale across rounds. -4. Fix real findings, reply-with-reason to the rest. -5. **Re-run the panel after the fix round.** Not optional: see below. -6. Record the receipt with the lenses that actually ran: +5. Fix real findings, reply-with-reason to the rest. +6. **Re-run the panel after the fix round.** Not optional: see below. +7. Record the receipt with the lenses that actually ran: ```sh uv run /pr_watch.py \ diff --git a/kit-manifest.json b/kit-manifest.json index bee30d7..bee44ea 100644 --- a/kit-manifest.json +++ b/kit-manifest.json @@ -15,7 +15,7 @@ "files": { "docs/agentic-dev-kit/fallback-review-panel.md": { "role": "doctrine", - "sha256": "cf460b82d9c0e4f55691ca23a77f0a46911af3282d9f317da46b51e81b9227d2" + "sha256": "6d7972f70679beadde81648d25063d9a15f522a3f41fc09da3ee30d55d8616ed" }, "docs/agentic-dev-kit/safety-critical-changes.md": { "role": "doctrine",