docs: the panel's isolation must be of the repo under review#43
Conversation
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 <base>...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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe fallback review panel documentation strengthens isolation and diff-verification requirements, clarifies degraded-mode execution, preserves triage and rerun steps, and updates the manifest checksum for the changed documentation. ChangesFallback review safeguards
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review recordCodeRabbit reviewed this and found nothing — plan Pro Plus, profile CHILL,
Its walkthrough confirms the range covers the current head exactly: Why the receipt says
|
… filed (#52) Session block for the adopter-upgrade session. Five PRs merged across two repos: OpenKitchen #256 (pre-v2 install to schema v2), #257 (review.bots emptied), #258 (doc sync); kit #43 (panel isolation doctrine) and #49 (test suite detached from ambient config, closing #48). The upgrade worked. What it mostly produced was evidence about the kit: 17 issues, and the recurring shape is that the kit's quality mechanisms each cover a different subset of the kit and nothing checks they agree — KIT_OWNED tracks 24 of 37 shipped files, the manifest tracks no test files, the suite covers no lines of pre-push. That gap is the root cause of #36, #40, #41 and #51, and #47 is the check that closes the class. Also recorded because it casts doubt backwards: #50 — a mutation preserving source LENGTH leaves stale bytecode Python treats as valid, so a git-clean restore does not restore. Mutation evidence gathered earlier in the session is suspect, including on the already-merged #256. Ran archive_plan_sessions.py --keep 4 (447 -> 377 lines); the 2026-07-25 block moved to kit-handoff-history.md verbatim. No friction-log entry: the log is AT its 150-line budget with an explicit "sweep required before the next entry" tripwire, and this session's friction was routed straight to Issues — the Principle #2 routing, not a neglected inbox. CodeRabbit found one real issue in review: the archive pointer references a section that exists nowhere and claims a folding step the script never performs. Filed as #53 rather than fixed here — the text is engine-generated, so a hand-edit would be overwritten by the next sweep and reproduces in every adopter. Next session: cs-toolkit Phase 1 only (additive install, zero behaviour change). Deferring the pr_watch swap (nightly fixer in active development) and the pre-push swap (blocked on #46). Claude-Session: https://claude.ai/code/session_01V27wAgECMEApqAvmJMFyqm
1 participant
A five-line hole in
fallback-review-panel.mdthat the first real adopter upgrade walked straight into.The gap
Contract item 7 said:
It never says of what repo. That is fine while the cockpit is reviewing its own repo, and wrong the moment it isn't.
On the OpenKitchen upgrade (topij/OpenKitchen#256) the cockpit was running in
agentic-dev-kitand reviewingOpenKitchen. The runtime'sisolation: worktreedid exactly what it says — it made a worktree of the session's repo. Both lenses were therefore placed in a worktree of the kit while being asked to review the adopter, andgit diff <base>...HEADwas empty in both.Why it is worth a doctrine change rather than a shrug
Both lenses caught it and cloned the real target themselves, so that review was sound. But consider the failure mode if one hadn't:
fallback:panelreceipt gets recorded for a review that never looked at the codeThat is the worst outcome a review mechanism has available, because it manufactures evidence. And
--lensesis self-reported and unverified by design (#32), so there is no second line of defence here — the receipt would simply be false.The launch prompt on that run also asserted "You are in an isolated git worktree of that repo." That was untrue, and the doctrine invited it by leaving the question unaddressed.
Changes
Contract item 7 — isolation must be of the repo under review; name the target explicitly in the launch prompt; and never state that isolation has already been arranged unless you have confirmed it.
"Running it" — a new step: confirm each lens reviewed the right code before reading its findings. This is contract item 8 ("state what was verified clean, and how") turned around to point at the operator instead of the lens. A zero-finding report is evidence only once you know what was in front of it.
Subsequent steps renumbered. No cross-reference anywhere in the repo depends on the step numbers (checked — every reference is to the file, not to a step).
Verification
scripts/tests+scripts/lib/state_paths/tests: 320 passedkit-manifest.jsonregenerated for the changed doc hash (kit_doctor.py --generate-manifest)Docs-only; no engine touched.
Related
Filed from the same upgrade: #35–#42. Closest relative is #32 — verifying lens coverage rather than self-reporting it. This PR does not fix that; it closes the specific case where the operator can be confident the coverage claim is wrong and currently has nothing telling them.
https://claude.ai/code/session_01V27wAgECMEApqAvmJMFyqm
Summary by CodeRabbit