From b71c505c4d76d652a9436400733354b980b90623 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 22 Apr 2026 11:07:51 +0200 Subject: [PATCH] =?UTF-8?q?chore(skills):=20pr-review=20=E2=80=94=20point?= =?UTF-8?q?=20reviewers=20at=20team=20worktree,=20not=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sprint 37 PR #138 review produced 6 false-positive findings because the reviewer defaulted to Read/Grep on the main repo path instead of the sprint-37/copy branch worktree. Every finding was a verbatim match against main's state but irrelevant to the branch — the branch had already cleaned the residue being flagged as "still present." False negatives are the same drift running the other direction: bugs introduced on the branch never get seen because the reviewer never reads the branch. Fix: resolve the team worktree path before spawning reviewers ($(dirname )/.sprint/sprint-N//) and pass it into the reviewer prompt with non-negotiable language. The worktree IS the branch; Read/Grep on paths rooted there cannot resolve to main. Fallback (no worktree — e.g. non-sprint branch or post-teardown): git show origin/:. Explicitly flagged in the prompt so the reviewer knows Read/Grep on any local path is wrong. --- .claude/skills/pr-review/SKILL.md | 57 ++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md index ed28b8c08..c347afea5 100644 --- a/.claude/skills/pr-review/SKILL.md +++ b/.claude/skills/pr-review/SKILL.md @@ -138,24 +138,57 @@ Three-dot diff with pathspec exclusions is unreliable. Instead, either: For large diffs (>1000 lines of source), provide **source files** rather than raw diff to reviewers — cleaner context, better reviews. -**Reviewer agents read source files via `git show`.** Sprint branches -use the naming pattern `sprint-{N}/{team}`. To read a file from the -branch being reviewed: +**Reviewer agents read source files from the team worktree.** + +Sprint branches follow `sprint-{N}/{team}`. Mid-sprint, a worktree +of each branch exists at `$(dirname )/.sprint/sprint-{N}/{team}/` +— a *sibling* of the repo root, not a child. This worktree IS the +branch: Read/Grep on paths rooted there resolve against the branch's +checkout, not main's. + +**Why this matters:** Sprint 37 PR #138 review produced 6 false- +positive findings because the reviewer defaulted to Read/Grep on the +main repo path (`/var/mnt/data/projects/settled-reach/main/`) instead +of the branch worktree. Every finding was a verbatim match against +main's state but irrelevant to the branch — the branch had already +cleaned the residue the reviewer flagged as "still present." Sending +those findings to the team would have caused busywork on already-clean +code, and more dangerously, the same drift hides *false negatives* +(branch-introduced bugs the reviewer never saw because it never read +the branch). + +Fix: before spawning reviewers, resolve the worktree path and pass it +into every reviewer prompt with prominent language. The reviewer +reads from the worktree, not from main. ```bash -git show origin/: +# Determine worktree path +SPRINT_NUM=$(echo "" | sed -E 's|sprint-([0-9]+)/.*|\1|') +TEAM=$(echo "" | sed -E 's|sprint-[0-9]+/||') +REPO_ROOT=$(git rev-parse --show-toplevel) +WORKTREE="$(dirname "$REPO_ROOT")/.sprint/sprint-${SPRINT_NUM}/${TEAM}" + +# Verify it exists and matches the branch tip +git -C "$WORKTREE" rev-parse HEAD # should equal `git rev-parse origin/` ``` -For example: -```bash -git show origin/sprint-31/server:server/src/bin/atlas.rs -``` +If the worktree exists and its HEAD matches `origin/`, use it +as the reviewer's source of truth. If it doesn't exist (e.g. the +sprint has been torn down or you're reviewing a non-sprint branch), +fall back to `git show origin/:` — explicitly flag this +fallback in the reviewer prompt so the reviewer knows Read/Grep on +any local path would be wrong. -If the sprint branch has an active worktree (under `.sprint/`), reviewers -can also use the Read tool with the worktree path. But `git show` is -the reliable default — it works whether or not a worktree exists. +In the reviewer prompt, state the rule non-negotiably: -Also tell agents to read relevant `decisions/*.md` files for context. +> **Read source from `` only.** Do NOT Read or Grep +> paths under the main repo root (`/var/mnt/data/projects/settled-reach/main/`). +> Those resolve to main, not the branch. The worktree at `` +> IS the branch — point all file tools there. + +Also tell agents to read relevant `decisions/*.md` files for context +(these can be read from either path — they're usually identical — +but for consistency, use the worktree path). ### 4. Spawn reviewers in parallel