-
Notifications
You must be signed in to change notification settings - Fork 3
docs: add /review skill for automated PR review sweeps #474
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.
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
Changes from all commits
5d29cd9
deac23e
34820e6
83efde5
ccb8bd5
73c51a4
e3c1d4a
5b4a5cd
20013d4
7c877d7
7e69570
0d0fdd9
2fa9dfc
651c164
5dc707a
5a9dd2b
b87a12b
1301b9f
25e940b
5a77db1
de87d18
620128b
e9e4de5
b3e46b5
a3ee610
cc6dafd
af0a536
e0dabd2
7ef9fae
7c483b4
99af3d3
645455c
cf95629
5cb944c
6ab8371
77e98a0
9393ad0
e23e9a0
3f9be49
c20321d
4509b9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,188 @@ | ||||||||||||||||||||||
| --- | ||||||||||||||||||||||
| name: review | ||||||||||||||||||||||
| description: Check all open PRs, resolve conflicts, update branches, address Claude and Greptile review concerns, fix CI failures, and retrigger reviewers until clean | ||||||||||||||||||||||
| allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Agent | ||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # PR Review Sweep | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| You are performing a full review sweep across all open PRs in this repository. Your goal is to bring every PR to a clean, mergeable state: no conflicts, CI passing, all reviewer comments addressed, and reviewers re-triggered until satisfied. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Step 0: Worktree Isolation | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Before doing anything else, run `/worktree` to get an isolated copy of the repo. CLAUDE.md mandates that every session starts with `/worktree` to prevent cross-session interference. All subsequent steps run inside the worktree. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Step 1: Discover Open PRs | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh pr list --repo optave/codegraph --state open --json number,title,headRefName,baseRefName,mergeable,statusCheckRollup,reviewDecision --limit 50 | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Record each PR's number, branch, base, merge status, and CI state. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Step 2: Process Each PR | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| For **each** open PR, perform the following steps in order. Process PRs one at a time to avoid cross-contamination. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2a. Switch to the PR branch | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Ensure the working tree is clean before switching to avoid cross-PR contamination: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| if [ -n "$(git status --porcelain)" ]; then | ||||||||||||||||||||||
| git stash push -m "pre-checkout stash" | ||||||||||||||||||||||
| fi | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Then check out the PR branch: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh pr checkout <number> | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2b. Resolve merge conflicts | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Check if the PR has conflicts with its base branch: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
|
Comment on lines
+49
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No clean-tree check before each PR checkout Step 2a runs A
Suggested change
If any output is shown, stash or reset: git stash push -m "pre-checkout stash for PR <previous-number>" || git reset --hard HEADThen check out the PR branch: gh pr checkout <number>
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — added a git status --porcelain guard before each gh pr checkout, with a stash fallback to prevent cross-PR contamination. |
||||||||||||||||||||||
| gh pr view <number> --json mergeable --jq '.mergeable' | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| If `CONFLICTING`: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Merge the base branch into the head branch (never rebase): | ||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| git merge origin/<base-branch> | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| 2. Resolve all conflicts by reading the conflicting files, understanding both sides, and making the correct resolution. | ||||||||||||||||||||||
| 3. After resolving, stage the resolved files by name (not `git add .`), commit with: `fix: resolve merge conflicts with <base-branch>` | ||||||||||||||||||||||
| 4. Push the updated branch. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2c. Check CI status | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh pr checks <number> | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| If any checks are failing: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Read the failing check logs: | ||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh run view <run-id> --log-failed | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| 2. Diagnose the failure — read the relevant source files, understand the error. | ||||||||||||||||||||||
| 3. Fix the issue in code. | ||||||||||||||||||||||
| 4. Run tests locally to verify: `npm test` | ||||||||||||||||||||||
| 5. Run lint locally: `npm run lint` | ||||||||||||||||||||||
| 6. Commit the fix with a descriptive message: `fix: <what was broken and why>` | ||||||||||||||||||||||
| 7. Push and wait for CI to re-run. Check again: | ||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh pr checks <number> | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| 8. Repeat until CI is green. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2d. Gather all review comments | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Fetch **all** review comments from both Claude and Greptile: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| # PR review comments (inline code comments) | ||||||||||||||||||||||
| gh api repos/optave/codegraph/pulls/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, path: .path, line: .line, created_at: .created_at}' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # PR reviews (top-level review bodies) | ||||||||||||||||||||||
| gh api repos/optave/codegraph/pulls/<number>/reviews --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, state: .state}' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Issue-style comments (includes @greptileai trigger responses) | ||||||||||||||||||||||
| gh api repos/optave/codegraph/issues/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, created_at: .created_at}' | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2e. Address every comment | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| For **each** review comment — including minor suggestions, nits, style feedback, and optional improvements: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. **Read the comment carefully.** Understand what the reviewer is asking for. | ||||||||||||||||||||||
| 2. **Read the relevant code** at the file and line referenced. | ||||||||||||||||||||||
| 3. **Make the change.** Even if the comment is marked as "nit" or "suggestion" or "minor" — address it. The goal is zero outstanding comments. | ||||||||||||||||||||||
| 4. **If you disagree** with a suggestion (e.g., it would introduce a bug or contradicts project conventions), do NOT silently ignore it. Reply to the comment explaining why you chose a different approach. | ||||||||||||||||||||||
| 5. **Reply to each comment** explaining what you did: | ||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \ | ||||||||||||||||||||||
| -f body="Fixed — <brief description of what was changed>" | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| For issue-style comments, reply on the issue: | ||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh api repos/optave/codegraph/issues/<number>/comments \ | ||||||||||||||||||||||
| -f body="Addressed: <summary of changes made>" | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2f. Commit and push fixes | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| After addressing all comments for a PR: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Stage only the files you changed. | ||||||||||||||||||||||
| 2. Group changes by concern — each logically distinct fix gets its own commit (e.g., one commit for a missing validation, another for a naming change). Do not lump all feedback into a single commit. | ||||||||||||||||||||||
| 3. Use descriptive messages per commit: `fix: <what this specific change does> (#<number>)` | ||||||||||||||||||||||
| 4. Push to the PR branch. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2g. Re-trigger reviewers | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **Greptile:** Always re-trigger after pushing fixes. Post a comment: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh api repos/optave/codegraph/issues/<number>/comments \ | ||||||||||||||||||||||
| -f body="@greptileai" | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **Claude (claude-code-review / claude bot):** Only re-trigger if you addressed something Claude specifically suggested. If you did: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||
| gh api repos/optave/codegraph/issues/<number>/comments \ | ||||||||||||||||||||||
| -f body="@claude" | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| If all changes were only in response to Greptile feedback, do NOT re-trigger Claude. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 2h. Wait and re-check | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| After re-triggering: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Wait for the new reviews to come in (check after a reasonable interval). | ||||||||||||||||||||||
| 2. Fetch new comments again (repeat Step 2d). | ||||||||||||||||||||||
| 3. If there are **new** comments from Greptile or Claude, go back to Step 2e and address them. | ||||||||||||||||||||||
| 4. **Repeat this loop for a maximum of 3 rounds.** If after 3 rounds there are still actionable comments, mark the PR as "needs human review" in the summary table and move to the next PR. | ||||||||||||||||||||||
| 5. Verify CI is still green after all changes. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Step 3: Summary | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| After processing all PRs, output a summary table: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| | PR | Branch | Conflicts | CI | Comments Addressed | Reviewers Re-triggered | Status | | ||||||||||||||||||||||
| |----|--------|-----------|----|--------------------|----------------------|--------| | ||||||||||||||||||||||
| | #N | branch | resolved/none | green/red | N comments | greptile, claude | ready/needs-work | | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Rules | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - **Never rebase.** Always `git merge <base>` to resolve conflicts. | ||||||||||||||||||||||
| - **Never force-push** unless fixing a commit message that fails commitlint. Amend + force-push is the only way to fix a pushed commit title (messages are part of the SHA). This is safe on feature branches. For all other problems, fix with a new commit. | ||||||||||||||||||||||
| - **Address ALL comments**, even minor/nit/optional ones. Leave zero unaddressed. | ||||||||||||||||||||||
| - **Always reply to comments** explaining what was done. Don't just fix silently. | ||||||||||||||||||||||
| - **Always re-trigger Greptile** after pushing fixes — it must confirm satisfaction. | ||||||||||||||||||||||
| - **Only re-trigger Claude** if you addressed Claude's feedback specifically. | ||||||||||||||||||||||
| - **No co-author lines** in commit messages. | ||||||||||||||||||||||
| - **No Claude Code references** in commit messages or comments. | ||||||||||||||||||||||
| - **Run tests and lint locally** before pushing any fix. | ||||||||||||||||||||||
| - **One concern per commit** — don't lump conflict resolution with code fixes. | ||||||||||||||||||||||
| - **Flag scope creep.** If a PR's diff contains files unrelated to its stated purpose (e.g., a docs PR carrying `src/` or test changes from a merged feature branch), flag it immediately. Split the unrelated changes into a separate branch and PR. Do not proceed with review until the PR is scoped correctly — scope creep is not acceptable. | ||||||||||||||||||||||
| - If a PR is fundamentally broken beyond what review feedback can fix, note it in the summary and skip to the next PR. | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git checkoutbreaks for fork PRsStep 2a fetches from
originand then doesgit checkout <head-branch>. For PRs opened from a forked repository, the head branch lives on the fork's remote — not onorigin— so the checkout will fail with "pathspec did not match any file(s) known to git".gh pr checkout <number>handles this transparently: it fetches from the correct remote (fork or origin) and sets up a local tracking branch. Replace the three-line block with:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — replaced git fetch/checkout with gh pr checkout which handles fork PRs transparently.