Summary
The /review skill diffs against the wrong base, so when origin/<base> has moved ahead of the feature branch, commits that landed on base (but aren't yet merged into the feature branch) show up in the review as deletions. The review then flags them as "regressions" or "removed code."
Repro
- Branch off
origin/master at commit A.
- Someone lands commit B on
origin/master (not merged into your branch yet).
- Run
/review on your branch.
- The diff output includes B's changes as
- lines, as if your branch is removing them.
Workaround today: cancel, git merge origin/master, re-run /review.
Root cause
review/SKILL.md.tmpl:75 runs:
That's two-dot (git diff A B = git diff A..B), which is the symmetric diff between the two tips. It includes any commits on origin/<base> that aren't on HEAD as deletions.
Fix
The straightforward-looking fix is three-dot:
git diff origin/<base>...HEAD
That diffs from the merge-base of origin/<base> and HEAD, which excludes the false deletions. But it also drops uncommitted working-tree changes from the review scope — /review mid-work would show "Nothing to review" if HEAD is at the merge-base, even when there are uncommitted changes. Verified empirically on a synthetic repo with one committed change + one uncommitted line + a base that moved ahead.
The correct fix is the merge-base form:
git diff $(git merge-base origin/<base> HEAD)
Equivalent to "working tree vs the merge-base." This:
- excludes commits on
origin/<base> not yet in HEAD (no false deletions — the bug from this issue)
- includes uncommitted working-tree edits (so
/review keeps working on in-progress code)
| Form |
False base-only deletions |
Uncommitted edits |
git diff origin/<base> (current) |
shown as deletions ❌ |
included ✓ |
git diff origin/<base>...HEAD (three-dot) |
excluded ✓ |
dropped ❌ |
git diff $(git merge-base origin/<base> HEAD) |
excluded ✓ |
included ✓ |
PR: https://github.com/dgrant/gstack/pull/1200 (against my fork; will rebase against garrytan/gstack if accepted).
The PR also fixes the same pattern in several call sites that were already three-dot before this issue and had the silent uncommitted-drop regression: scripts/resolvers/{review,review-army,testing,utility,design,preamble/generate-test-failure-triage}.ts, scripts/slop-diff.ts, and several places in ship/SKILL.md.tmpl.
Summary
The
/reviewskill diffs against the wrong base, so whenorigin/<base>has moved ahead of the feature branch, commits that landed on base (but aren't yet merged into the feature branch) show up in the review as deletions. The review then flags them as "regressions" or "removed code."Repro
origin/masterat commit A.origin/master(not merged into your branch yet)./reviewon your branch.-lines, as if your branch is removing them.Workaround today: cancel,
git merge origin/master, re-run/review.Root cause
review/SKILL.md.tmpl:75runs:That's two-dot (
git diff A B=git diff A..B), which is the symmetric diff between the two tips. It includes any commits onorigin/<base>that aren't onHEADas deletions.Fix
The straightforward-looking fix is three-dot:
That diffs from the merge-base of
origin/<base>andHEAD, which excludes the false deletions. But it also drops uncommitted working-tree changes from the review scope —/reviewmid-work would show "Nothing to review" ifHEADis at the merge-base, even when there are uncommitted changes. Verified empirically on a synthetic repo with one committed change + one uncommitted line + a base that moved ahead.The correct fix is the merge-base form:
git diff $(git merge-base origin/<base> HEAD)Equivalent to "working tree vs the merge-base." This:
origin/<base>not yet inHEAD(no false deletions — the bug from this issue)/reviewkeeps working on in-progress code)git diff origin/<base>(current)git diff origin/<base>...HEAD(three-dot)git diff $(git merge-base origin/<base> HEAD)PR: https://github.com/dgrant/gstack/pull/1200 (against my fork; will rebase against
garrytan/gstackif accepted).The PR also fixes the same pattern in several call sites that were already three-dot before this issue and had the silent uncommitted-drop regression:
scripts/resolvers/{review,review-army,testing,utility,design,preamble/generate-test-failure-triage}.ts,scripts/slop-diff.ts, and several places inship/SKILL.md.tmpl.