agentleFS
dougborgdougborg/frontapp-openapi-client/.claude/skills/review-pr/SKILL.md

review-pr

Skill0 starsChanged 7 months agoMITArchived repositoryView on GitHub

Address PR review comments — fetch all unresolved comments, fix the issues, validate with `uv run poe check`, commit, push, and reply to each comment on GitHub. Use when asked to handle PR review feedback.

  • Deletes or force-pushes
  • Commits and pushes

185 other projects use a version of this.

Adopted by

Most recently taken up by

Used with

Projects with it also use diagnose-ci-failures, docs-loop and 10 more.

Other files people use alongside this one.

Files found in the same public projects as a version of this one, more often than they turn up anywhere else. Each project counts once, large collections count for less, and this file's own repository is left out.
  • zerx-labSkill

    diagnose-ci-failures

    Diagnose CI failures for a PR using the GitHub CLI, extract error logs, and generate a plan to fix them. Use when the user asks to check CI status, pull CI issues, triage test failures, or investigate PR build failures.

    Also in 3 projects that use this

  • GeiserXSkill

    docs-loop

    Use when the user asks to audit docs, check documentation for stale claims, or verify docs against the code.

    Also in 3 projects that use this

  • GeiserXSkill

    full-loop

    Drive a repository to done with sergio-loop, then keep improving it with refine-loop at a descending theta. Use only when the user explicitly invokes full-loop.

    Also in 3 projects that use this

  • n24q02mSkill

    review-delta

    Review only changes since last commit using impact analysis. Token-efficient delta review with automatic blast-radius detection.

    Also in 3 projects that use this

  • n24q02mSkill

    build-graph

    Build or update the code review knowledge graph. Run this first to initialize, or let hooks keep it updated automatically.

    Also in 3 projects that use this

  • +7 more from GeiserX

The file

Show its rules and text

We split this skill into the rules it gives an agent

22 rules. 0 are common, 11 are distinctive, and 11 are very specific to this project.

  • 0

    Common

    Most of the 178 edited versions we read keep these.

  • 11

    Distinctive

    This author's own choices. Worth a look.

  • 11

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  • could be automatic: a setting could enforce it
  1. Very specific:

    If $ARGUMENTS is empty, gh pr view auto-detects from the current branch.

    line 41
  2. Distinctive:

    If no PR is found, tell the user and stop.

    line 42
  3. Very specific:

    If git rev-parse --abbrev-ref HEAD doesn't match, run git checkout <headRefName> (stash dirty work first if needed).

    line 47
  4. Distinctive:

    Do not assume main.

    line 50
  5. Very specific:

    If <baseRefName> is not the repo's default branch (e.g.

    line 54
  6. Distinctive:

    If a comment has prior replies, check whether the replies actually resolved the concern.

    line 142
  7. Distinctive:

    If not, treat it as still needing action.

    line 143
  8. Very specific:

    NEVER edit generated files (api//*.py, models//*.py, client.py)

    line 153
  9. Very specific:

    Use unwrap_unset(), unwrap_as(), is_success() helpers per CLAUDE.md patterns

    line 154
  10. Very specific:

    Run uv run poe fix for auto-fixable lint/format issues

    line 165
  11. Distinctive:

    Create a temporary fixup commit:

    line 183
  12. Distinctive:

    If fixes span multiple original commits, create multiple fixup commits — one per target commit.

    line 197
  13. Distinctive:

    If fixes don't clearly map to an existing commit (e.g., they address a cross-cutting concern), create a standalone commit with a …

    line 200
  14. Very specific:

    Use --force-with-lease (not --force) to avoid overwriting changes pushed by others.

    line 228
  15. Very specific:

    If Phase 1 detected <baseRefName> is not the default branch (e.g.

    line 234
  16. Distinctive:

    After your git push --force-with-lease, every child PR's branch needs to be rebased onto the new SHA:

    line 241
  17. Very specific:

    If the parent has been rewritten upstream, your git rebase --autosquash origin/<baseRefName> from Step 2 will re-apply the parent's old …

    line 259
  18. Distinctive:

    Before merging a parent in a stack, flip every child's base to the eventual destination (usually main):

    line 271
  19. Very specific:

    After pushing, reply to every unresolved comment.

    line 285
  20. Very specific:

    When the comment makes a factual claim about the generated API surface (module name, response shape, helper presence), docs/api-facts.yaml …

    line 306
  21. Distinctive:

    Skip this phase entirely: if the review fixes are minor (typos, doc tweaks, clarifications) and the existing description is still accurate.

    line 320
  22. Distinctive:

    Update only when scope materially changed (new files added, behavior changed, test plan no longer applies).

    line 324

“Common” is measured: kept by at least 6 in 10 of the 178 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

PR Review Comment Workflow

Address all unresolved PR review comments: fix the code, validate, commit, push, and reply.

Phase 1: Identify the PR + check its state

Fetch everything needed in one call:

gh pr view ${ARGUMENTS:-} --json \
  number,url,title,headRefName,baseRefName,mergeable,mergeStateStatus,statusCheckRollup
gh repo view --json nameWithOwner --jq '.nameWithOwner'

If $ARGUMENTS is empty, gh pr view auto-detects from the current branch. If no PR is found, tell the user and stop.

Store:

  • headRefName — the PR's branch. You must be on this branch before any commits. If git rev-parse --abbrev-ref HEAD doesn't match, run git checkout <headRefName> (stash dirty work first if needed).
  • baseRefName — use origin/<baseRefName> as the rebase/log target throughout. Do not assume main.

Detect stacked PRs early

If <baseRefName> is not the repo's default branch (e.g. it's another feature branch like chore/harness-phase-a), this PR is stacked on another PR. That changes how rebases and force-pushes propagate — see the "Stacked PR" call-out in Phase 5.

If mergeable: CONFLICTING

  1. git fetch origin <baseRefName>
  2. git merge origin/<baseRefName>
  3. Resolve each conflicted file by reading both sides; keep our branch's intent integrated with the base's changes.
  4. git add <resolved files> then continue the merge.

If mergeStateStatus is not CLEAN / HAS_HOOKS

statusCheckRollup already lists every check (no need for a second gh pr checks call). Triage by failure type:

  • Code issue (lint / type-check / test) → fix as part of Phase 4.
  • Flaky CI (timeout, transient network) → note in the summary, don't block.
  • Repo-config issue (Dependency Graph disabled, missing secret, deprecated action) → note in the summary, link the GitHub-org-or-repo settings page that needs the change, and don't block on it. These aren't fixable from a PR.

Resolve conflicts before addressing review comments — comments may no longer apply after the resolution.

Phase 2: Fetch all review comments

Fetch every review comment on the PR:

gh api repos/{owner}/{repo}/pulls/{number}/comments --paginate

Also fetch review threads to check resolved status:

gh api graphql -f query='
  query($owner: String!, $repo: String!, $number: Int!) {
    repository(owner: $owner, name: $repo) {
      pullRequest(number: $number) {
        reviewThreads(first: 100) {
          nodes {
            isResolved
            comments(first: 100) {
              nodes {
                id
                databaseId
                body
                path
                line
                author { login }
              }
            }
          }
        }
      }
    }
  }
' -F owner="{owner}" -F repo="{repo}" -F number={number}

Present a summary table of all comments:

#StatusFile:LineAuthorComment (truncated)
1unresolvedsrc/foo.py:42reviewer"Consider using..."
2resolvedsrc/bar.py:10reviewer"Typo in variable..."

Skip resolved threads — only work on unresolved comments.

Phase 3: Triage each unresolved comment

For each unresolved comment:

  1. Read the affected file and surrounding context
  2. Check if already fixed — the code may have changed since the comment was posted
  3. Classify the comment:
    • fix needed — code change required
    • already fixed — issue was addressed in a prior commit, just needs a reply
    • acknowledge — valid point but deferring, or explaining why current approach is correct
    • disagree — the comment is wrong (e.g. an automated reviewer hallucinated a missing import, or claims a file doesn't exist when it does). Verify with ls / grep / a small smoke test, then reply with the citation that proves the comment wrong. Don't silently dismiss.

If a comment has prior replies, check whether the replies actually resolved the concern. If not, treat it as still needing action.

Automated reviewers (Copilot, etc.) produce false positives often enough that "disagree" is a routine outcome — not an exception. Don't bend the code to satisfy a hallucinated finding.

Phase 4: Fix all issues

Make all necessary code changes across affected files. Remember this repo's constraints:

  • NEVER edit generated files (api/**/*.py, models/**/*.py, client.py)
  • Use unwrap_unset(), unwrap_as(), is_success() helpers per CLAUDE.md patterns
  • Resilience is at the transport layer — don't wrap API methods with retries

Then validate:

uv run poe check

This runs format + lint + type-check + tests. ALL must pass. If validation fails:

  1. Run uv run poe fix for auto-fixable lint/format issues
  2. Fix remaining issues manually
  3. Re-run uv run poe check until clean

Phase 5: Commit, rebase, and push

The goal is a clean commit history — review fixes should be folded into the commits they logically belong to, not piled on as separate "fix review comments" commits. Don't squash everything into one commit; preserve meaningful commit boundaries.

Step 1: Stage and commit fixes

Stage only the files that were changed (never use git add -A or git add .):

git add <file1> <file2> ...

Create a temporary fixup commit:

git commit -m "$(cat <<'EOF'
fixup! <original commit subject line this fix belongs to>

- Description of fix 1
- Description of fix 2

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EOF
)"

If fixes span multiple original commits, create multiple fixup commits — one per target commit.

If fixes don't clearly map to an existing commit (e.g., they address a cross-cutting concern), create a standalone commit with a descriptive message instead.

Step 2: Rebase to fold fixups into their target commits

git fetch origin <baseRefName>
git rebase --autosquash origin/<baseRefName>

This automatically reorders and squashes fixup! commits into their targets. After the rebase, verify with:

git log origin/<baseRefName>..HEAD --oneline

The result should be a clean set of logical commits — no "fix review comments" or "address feedback" commits.

Step 3: Force-push the rebased branch

Since the branch is already pushed, a force-push is required after rebasing:

git push --force-with-lease

Use --force-with-lease (not --force) to avoid overwriting changes pushed by others.

NEVER reply to comments before pushing. Replies confirm the fix is live.

Stacked PRs (this PR's base is another feature branch)

If Phase 1 detected <baseRefName> is not the default branch (e.g. it's chore/harness-phase-a for a PR that stacks on PR #18), force-pushing this branch is fine — but force-pushing the parent in a separate /review-pr run rewrites this branch's base, breaking the chain. Two scenarios:

This PR is the parent in a stack (other PRs target this branch as their base):

After your git push --force-with-lease, every child PR's branch needs to be rebased onto the new SHA:

# For each child PR's local branch:
git checkout <child-branch>
git rebase --onto <parent-branch> <OLD_PARENT_SHA> <child-branch>
git push --force-with-lease

<OLD_PARENT_SHA> is the SHA the child was previously based on (find with git log <child-branch>~N --oneline | grep <parent-subject>). The plain git rebase <parent-branch> re-applies the parent's old commits and conflicts — use --onto to skip them.

This PR is a child (its base is some other feature branch the user is also reviewing in another /review-pr session):

If the parent has been rewritten upstream, your git rebase --autosquash origin/<baseRefName> from Step 2 will re-apply the parent's old commits and conflict. Detect this by git fetch origin <baseRefName> first; if origin/<baseRefName> moved, use git rebase --onto origin/<baseRefName> <OLD_BASE_SHA> instead of plain --autosquash.

About to merge the parent? gh pr merge --delete-branch on the parent deletes its branch on origin, which auto-closes every child PR whose baseRefName points at that branch. The auto-closed PRs cannot be reopened (GitHub refuses because the base ref is gone) — you'd have to open fresh PRs from the same head branch with content rebased onto a still-existing base.

Before merging a parent in a stack, flip every child's base to the eventual destination (usually main):

gh pr edit <child-pr-number> --base main

This is a metadata-only change (no force-push). The child PR will then show its diff against main until the parent merges; once the parent merges, you can rebase the child's branch onto fresh main to drop the parent's now-redundant commits. Doing this before the parent merge means no PRs auto-close.

Phase 6: Reply to each comment

After pushing, reply to every unresolved comment. Pipe through jq -r '.html_url' so the response is a tidy one-line confirmation, not the ~3 KB JSON blob gh api dumps by default:

gh api repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}/replies \
  -X POST -f body='...' | jq -r '.html_url'

Reply format by classification:

  • Code fix: "Fixed in <short-sha> — [describe what was changed]. [mention tests if added]"
  • Already fixed: "Already addressed in <short-sha> — [brief explanation]"
  • Acknowledged/deferred: "Acknowledged — [reason for deferring]. Tracked in #NNN." (must include a GitHub issue link)
  • Disagree: "Disagree — [evidence]. [What you checked, e.g. ls path/, grep -n …, docs/api-facts.yaml: tags.X.endpoints[…], smoke-test result.] Marking as not-applicable; happy to revisit if there's a repro." Don't be hostile — be specific.

    When the comment makes a factual claim about the generated API surface (module name, response shape, helper presence), docs/api-facts.yaml is the authoritative cite — it's CI-validated, so a single grep against that file refutes most automated-reviewer hallucinations. Example: a reviewer claiming api/tags/list_tags.py doesn't exist is refuted by summary.list_endpoints_with_field_results: tags.list_tags.

Reply to EVERY unresolved comment. None should be left without a response.

Cite the rewritten SHA from Step 3, not the original commit, since --autosquash produced a new SHA.

Phase 7: Update PR description (only if scope changed)

Skip this phase entirely if the review fixes are minor (typos, doc tweaks, clarifications) and the existing description is still accurate. Overwriting a rich PR body with a stub template is destructive.

Update only when scope materially changed (new files added, behavior changed, test plan no longer applies). Fetch the existing body first and amend it:

gh pr view {number} --json body --jq .body > /tmp/pr-body.md
# Edit /tmp/pr-body.md in place to reflect current scope
gh pr edit {number} --body-file /tmp/pr-body.md

Review the commit log (git log origin/<baseRefName>..HEAD --oneline) before deciding whether scope changed.

Phase 8: Summary

Print a final summary for the user:

  • Number of comments addressed (fixed / already fixed / acknowledged)
  • Files changed
  • Validation result (tests, lint, type-check)
  • Any comments that couldn't be addressed, with explanation

Important Rules

  • Fix first, reply after push — never reply before the code is pushed
  • Reply to every comment — even if just acknowledging
  • No shortcuts — uv run poe check must pass; never use --no-verify, noqa, or type: ignore
  • Be specific in replies — describe what changed, don't just say "done"
  • Clean history via rebase — fold review fixes into the commits they belong to using fixup! + --autosquash; don't pile on "fix review comments" commits, but also don't squash everything into one commit — preserve meaningful commit boundaries
  • Evaluate existing replies — if a comment already has replies, check whether the issue was actually resolved; if not, fix it and add a new reply
  • No ignores or suppressions — never use noqa, type: ignore, or skip tests to make validation pass
  • File issues for deferred work — if a review comment identifies a valid issue that is out of scope or too large to fix in this PR, create a GitHub issue with gh issue create before replying. The reply MUST include the issue number. Never defer work without a tracking issue.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.