Skip to content

pr-review skill: judge at TARGET, stacked bases, leave generated files to CI #7501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrewnester merged 3 commits into from
Nov 12, 2026
Merged

pr-review skill: judge at TARGET, stacked bases, leave generated files to CI #7501

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0356f5c
pr-review skill: judge at TARGET, leave stale generated files to CI
andrewnester Nov 11, 2026
9c69c44
pr-review skill: take BASE from the stacked PR's base branch
andrewnester Nov 11, 2026
00b7b95
pr-review skill: run the merge check against the PR's base branch
andrewnester Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions .agents/skills/pr-review/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ code is designed" never justifies a behavior on its own.
## 1. Collect the change

The review target is `HEAD` unless you were given a commit. Set `TARGET` to it
and `BASE` to its merge base with main:
and `BASE` to its merge base with the PR's base branch:

```bash
TARGET=HEAD
Expand All @@ -27,26 +27,31 @@ git diff --stat "$BASE" "$TARGET"
git diff "$BASE" "$TARGET"
```

For a stacked PR (`gh pr view --json baseRefName` is not `main`), fetch that
branch and take `BASE` from `git merge-base "$TARGET" origin/<baseRefName>`
instead, so the parent PR's hunks are not reviewed again here.

If the change has a PR, read its title and description to learn the intent.
Do not read existing review comments before forming your own view. List the
hunks the description does not explain: a fix riding along with the main
change belongs in its own PR (`AGENTS.md`: one change per PR), so ask for
the split.

Check how far the branch is from main with
`git rev-list --count "$BASE"..origin/main` and
`git merge-tree --write-tree "$TARGET" origin/main`. When the base is far
behind, files the change touches may have moved, split or been deleted on
main, and conventions it follows (changelog location, test matrix variables)
may have changed. Those are judged at merge time, not at `TARGET`, so name
the new location in the review body.
Judge the code at `TARGET`. Separately, check what a merge into the PR's
base branch would hit: `git rev-list --count "$BASE"..origin/main` and
`git merge-tree --write-tree "$TARGET" origin/main`, with
`origin/<baseRefName>` in place of `origin/main` for a stacked PR, so a
conflict the parent PR has with main is reported once, on the parent. GitHub
flags the conflict too, but not where things went: if files the change
touches were moved, split or deleted there, or a convention it follows
changed (changelog location, test matrix variables), say so in the review
body with the new location, so the author rebases before the next round.
That is the only use of this check; do not re-review against main.

Read every changed non-generated file in full at `TARGET`, not just the hunks
(`git show "$TARGET":path`). Skim generated files (schemas, codegen output,
acceptance test output) only to confirm they follow from the source change;
when the generator is cheap to run, regenerate into a temporary directory and
`diff -r` it against the committed tree, since stale generated files outside
the diff never show up in the hunks.
CI already fails on stale generated output, so do not regenerate locally.
Read the repo's agent rules that apply to the changed files, under
`.agents/rules/` (testing, comments, language style guides).

Expand Down
Loading