review-pr

PR Review Workflow

By default, use a local checkout. Only when the user explicitly requests a worktree, parallel review, or concurrent review should an isolated Git worktree be used. If the PR reference is missing or ambiguous, ask before changing Git state.

Accepted PR references:

• GitHub URL：https://github.com/<base-owner>/<base-repo>/pull/<number>
• Shorthand:<base-owner>/<base-repo>#<number>
• PR number only: available when the current checkout belongs to the target repository

Safety constraints

• Start from git status --short --branch . In default local mode, if the checkout has uncommitted changes, stop before switching branches. Explicit worktree mode may continue from a dirty checkout because it must not switch or modify that checkout.
• Do not overwrite, delete, rename, rebase, reset, force-update, stash, or discard local branches, worktrees, or changes.
• Unless the user explicitly requests a push, do not push during preparation, merging, conflict resolution, or review.
• The contributor remote name must exactly match the PR head repository owner login. If that remote name already points elsewhere, stop and ask.
• In local checkout mode, branch selection prefers the PR head branch name. Only when the exact name cannot be safely used should a conflicting fallback branch be created review/pr-<number>-<head-short-sha>。
• Treat the PR metadata headRefOid as the only valid HEAD for a normal review. A local branch with the same name may fast-forward to that SHA, but must not contain extra local commits, and must not have diverged from it.
• When a branch name conflict occurs, automatically fall back to the local review branch review/pr-<number>-<head-short-sha> and continue. Never bypass by checking out a remote-tracking ref, entering detached HEAD, or directly reviewing an already fetched ref. Leave the conflicting branch unchanged.
• Stop without falling back only when the worktree has changes, the contributor remote points elsewhere, the fetched head differs from headRefOid , or an existing review branch is incompatible.
• Unless the user explicitly requests an isolated worktree or latest-base integration review, do not create other local branches with different names.
• If the normal preparation flow rejects an existing branch for other reasons, do not bypass it; keep that branch and ask how to proceed.
• In explicit worktree mode, use review/pr-<number>-<head-short-sha>for a normal review, and use review/pr-<number>-merge-<head-short-sha>for a latest-base review. Place the worktree under ../.review-pr-worktrees/<repo>/pr-<number>[-merge]-<head-short-sha> .
• After review, keep the prepared branch or worktree so the user can run and debug. Never automatically delete the review worktree.
• Treat "select branch" and "merge latest base" as two independent decisions: in local mode, first select the branch with the same name as head or the conflicting fallback branch, then decide whether to merge the latest base according to the user's request. Remote collaborative PRs do not use rebase.
• A local latest-base merge keeps the selected branch name; only an explicit --worktree --merge-latest uses review/pr-<number>-merge-<head-short-sha> this kind of isolated naming.
• Do not treat mergeable: CONFLICTING、mergeStateStatus: DIRTY or the base branch being ahead of the PR as authorization to merge. Unless the user explicitly requests a latest-base integration review or conflict resolution, these states are only review context.
• After reading the conflict code and surrounding context, resolve the merge conflict semantically. Do not mechanically choose ours/theirs.
• Do not review based only on the PR description. Check the linked issue, changed files, actual diff, relevant surrounding code and CI status.
• For accurate inline review context, unresolved comments or discussion_r... id, use gh api / GraphQL, so that isResolved、isOutdated, path and line remain visible. Do not rely only on gh pr view --json。
• Treat each isResolved == false review thread as open review comments that must be enumerated and evaluated one by one, including bot comments, comments containing replies, and comments marked as isOutdated == true . Do not omit any just because the thread seems reasonable, was generated by automation, or was not independently identified as a new finding.
• Treat PR feedback as live state. Opening the PR, marking a draft as ready, bot workflows, and manual review requests may all add new reviews or threads during the local review. When writing the final reply, never assume the initial comment snapshot is still the latest state.
• For each open comment evaluated as reasonable or partially reasonable , provide a separate Suggested Fix, grounded in the actual diff, surrounding code and project patterns. Suggest the minimal specific change that solves the problem, including the affected logic, expected behavior and targeted verification when relevant. This applies even if the comment comes from a bot. Put the full evaluation and fix in Open Review Comments ; do not repeat the same issue in Findings .
• For each independent finding also provide equally specific Suggested Fix. Only issues whose trigger conditions, risks and fix approach are all different from all open comments may appear in Findings。
• Do not use vague suggestions such as "fix this issue". When multiple valid approaches exist, recommend one and explain the important tradeoffs. If the fix depends on a product decision, give conditional options and raise that decision in Open Questions .
• Treat fix suggestions as review guidance. Unless the user explicitly asks, do not modify the PR; include short code examples only when they clearly improve the clarity of the suggestion.

Workflow

1. Collect PR metadata

When manually running the gh command, normalize the GitHub URL and <owner>/<repo>#<number> shorthand to <number> --repo <owner>/<repo>。

git status --short --branch
gh pr view <number> [--repo <base-owner>/<base-repo>] \
  --json number,title,url,body,baseRefName,headRefName,headRefOid,headRepository,headRepositoryOwner,closingIssuesReferences

Record the head owner, fork repository, head branch, head SHA, base branch, PR URL and linked issue. On the normal path, a helper script adds the remote, fetches the branch and sets up upstream tracking.

2. Choose the branch preparation path

Check mergeability before preparing the branch:

gh pr view <number> [--repo <base-owner>/<base-repo>] \
  --json mergeable,mergeStateStatus,isDraft,state,updatedAt,headRefOid,baseRefOid

Unless the user explicitly requests a worktree or parallel review, use local branch preparation. Do not infer worktree mode merely because the current checkout has changes. For a normal local PR, run one of the following commands; both local modes first choose the branch by the same rules:

bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh <pr-ref>
bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh --merge-latest <pr-ref>

When isolation of the source checkout is needed, explicitly use:

bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh --worktree <pr-ref>
bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh --worktree --merge-latest <pr-ref>

If the user did not request latest-base, even if GitHub reports mergeable: CONFLICTING or mergeStateStatus: DIRTY, the review still uses the normal preparation flow. Check out the PR head to a local branch with the same name, inspect the PR as of the commit state, and report the merge status without changing its history.

Only use the latest-base merge action when the user explicitly asks to update to the latest base, resolve conflicts, or review integration results. For local mode, first prepare a branch with the same name as the PR head or a collision fallback, then merge the latest base on that branch; do not automatically switch to review/pr-<number>-merge-<head-short-sha>just because a merge is needed. Only use this isolated merge branch for explicit worktree mode. Before running, explain that a local merge commit will be created; if the request does not yet explicitly include one of these actions, stop and ask before creating the branch.

If the PR head branch name conflicts with the base branch, a protected local branch name, or an existing branch with the same name that is different from upstream, the helper automatically creates the local review branch review/pr-<number>-<head-short-sha> and continues. Never rebind, rename, or delete the conflicting branch. Only when the existing review/pr-<number>-<head-short-sha> is clean, is at headRefOid and tracks <owner>/<branch> will the helper reuse it; otherwise stop and ask to inspect or delete it. Do not choose latest-base mode just to bypass a branch name conflict.

After a normal checkout, fetch the latest base and check whether the PR already includes it:

git merge-base --is-ancestor <base-remote>/<base-branch> HEAD

If the check fails, report that the PR is behind the latest base; do not automatically merge. When GitHub reports conflicts, where useful treat git merge-tree as a read-only conflict signal:

merge_base=$(git merge-base <base-remote>/<base-branch> HEAD)
git merge-tree "$merge_base" <base-remote>/<base-branch> HEAD

Treat baseRefName as the target branch; do not hardcode dev. Run only after latest-base is explicitly requested:

bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh --merge-latest <pr-ref>
bash .agents/skills/review-pr/scripts/prepare-pr-branch.sh --worktree --merge-latest <pr-ref>

The local latest-base helper fetches the PR base on the chosen local branch, runs git merge --no-edit <base-remote>/<base-branch>, and never pushes. Only explicit worktree latest-base creates from the PR head review/pr-<number>-merge-<head-short-sha>。

The normal preparation flow only updates branches that exactly match or can fast-forward to headRefOid . If the existing local PR branch is ahead of the fetched head, has diverged from it, is in use by another worktree, or the upstream does not match, the helper automatically falls back to review/pr-<number>-<head-short-sha>; never modify the conflicting branch, and never temporarily use a detached checkout.

3. Handle Merge Conflicts

If the merge helper stops due to a conflict, inspect the actual conflict before editing:

git status --short
git diff --name-only --diff-filter=U
git diff --cc

After resolving the conflict semantically, stage only the resolved conflict files and complete the merge:

git add <resolved-files>
git commit --no-edit

In worktree mode, run all conflict commands at the reported worktree path, for example git -C <worktree-path> status --short. Do not resolve the merge from the original checkout.

If the conflict requires a product decision, or cannot be safely resolved based on the local code and PR context, stop and report a blocker. Do not submit a complete review based on a partially merged tree.

4. Verify Checkout and Review Context

After preparation, verify the local state:

git branch --show-current
git rev-parse HEAD
git status --short
git branch -vv

Normal local preparation requires the branch to be clean, the branch name to be the PR head branch or collision fallback, the upstream set to <owner>/<branch>, and before merge HEAD to equal the recorded headRefOid. Only an upstream match is not enough, because the local branch may contain commits that do not exist in the PR. If local latest-base is enabled, after merge instead require both the PR head and latest base to be ancestors of HEAD ; if a merge commit is produced, also confirm that its two parents are the PR head and latest base respectively. Explicit worktree latest-base still requires a clean review/pr-<number>-merge-<head-short-sha> branch.

Worktree preparation requires the reported worktree to be clean and on the review branch for the corresponding SHA. A normal worktree branch must track <owner>/<head-branch>; the merged worktree branch remains local-only. Confirm that the source checkout's branch, HEAD, and file status are unchanged, then run the remaining review commands with that worktree as the working directory.

Before reviewing code, record a snapshot of the PR and issue context:

gh pr view <number> [--repo <base-owner>/<base-repo>] \
  --comments \
  --json number,title,url,body,baseRefName,headRefName,headRefOid,updatedAt,files,commits,closingIssuesReferences,comments,reviews
gh issue view <issue-url-or-number> --comments

Record headRefOid、updatedAt, the latest review's submittedAt , and the complete set of inline review threads.comments and reviews fields do not contain the complete inline thread content, so always also use GraphQL reviewThreads. For each thread, keep the id,isResolved、 isOutdated, path, line, and the database id, URL, body, author, timestamp, and commit OID of all comments. Keep paginating until pageInfo.hasNextPage is false; do not assume the first page contains all threads.

Before reviewing the overall diff, build a checklist for each open review thread (isResolved == false). The checklist must include unresolved outdated threads and all replies; explicitly mark isOutdated; do not silently discard threads. Review each checklist item in light of the current PR head, the change diff, the relevant call chains, and the surrounding code. The reviewer must determine whether the comment is reasonable、partially reasonable、unreasonable、 outdated/not applicable, or requires a product decision.

For reasonable and partially reasonable comments, identify the precise behavioral risk and provide a specific Suggested Fix, expected behavior, and targeted verification. For unreasonable or outdated/not applicable comments, explain the code evidence used to refute or supersede the concern. For comments that require a product decision, recommend one option, provide its Suggested Fix, and repeat the still-unresolved choices in Open Questions . Each reviewed issue may be assigned to only one output section. If an issue is raised by an open review comment, its assessment and fix may only go in Open Review Comments; do not repeat it in Findings .Findings Use only for additional issues whose trigger conditions, risks, and fixes are all different. When the checklist is empty, report separately No open review comments. After assignment, when there are no other issues, report No additional findings。

For old PRs or stale PRs, before deciding whether the branch should still be kept, check the linked issue history, subsequent replacement PRs, and the live base tree. When mergeability is central to the review, use git merge-tree <merge-base> <base-remote>/<base-branch> HEAD as a read-only staleness or conflict signal.

Before using origin , confirm the base repository remote. Fetch the real base branch, then inspect the diff and surrounding code:

git fetch <base-remote> <base-branch>
git diff --stat <base-remote>/<base-branch>...HEAD
git diff --name-status <base-remote>/<base-branch>...HEAD
git diff <base-remote>/<base-branch>...HEAD

Use rg to search the surrounding source code, tests, configuration, generated files, and documentation. Unless the user explicitly requests a local build, do not run xcodebuildduring PR review. When verification status matters, check PR checks:

gh pr checks <number> [--repo <base-owner>/<base-repo>]

When relevant, run lightweight local checks such as git diff --check .

5. Refresh the live PR status before the final output

Immediately before writing the final reply, refresh all mutable review status, even if the initial checks all passed:

gh pr view <number> [--repo <base-owner>/<base-repo>] \
  --json headRefOid,updatedAt,state,mergeStateStatus,comments,reviews
gh pr checks <number> [--repo <base-owner>/<base-repo>]

Repeat the same complete paginated GraphQL reviewThreads query used for the initial snapshot, then compare the two snapshots, including the open thread checklist, comment replies,isResolved and isOutdated status.

• If headRefOid has changed, stop the final output, update the prepared checkout to the new head, check the new diff against the real base, and re-verify the previous findings and the new changes.
• If a new review, thread, reply, or resolution/outdated status change appears, read its exact content, verify it against the current head and surrounding code, and in Open Review Comments add or update its standalone entry; only update Findings. Update before final output Open Questions and Verification. Re-evaluate existing entries for reply or resolution/outdated status changes. Automated feedback and human feedback use the same handling.
• If after analyzing new activity there is still enough time for another review to appear, refresh once more. Only complete when there are no unchecked feedback items in the latest snapshot.
• If a final refresh is unavailable, report that limitation and do not claim to have checked all current comments or threads. The final reply is complete only when there are no unchecked items in the latest open comment list.

Review Focus

Check whether the implementation truly addresses the PR description and linked issue. Prioritize bugs, regressions, edge cases, concurrency issues, persistence errors, localization gaps, platform version issues, API contract drift, missing validation, and unrelated changes.

When the user asks whether an old PR is still worth keeping, first give the decision to keep, modify, or close, then explain the code findings supporting that decision.

Output Format

Unless the user requests otherwise, write the final review in the user's system preferred language. The system preferred language is the first language in macOS AppleLanguages ; if it cannot be read, use the current conversation language.

Section titles,PR Context subheadings, priority labels, and Suggested Fix labels must remain as-is. Strictly use the following structure:

For Open Review Comments, prefer a compact summary line, followed by one Markdown card per thread. Put the comment permalink in the card title and do not show the long raw URL. Put path, line, author, and status on the same metadata line; translate raw GraphQL flags such as isResolved=false、isOutdated=false into short natural-language status labels in the output preferred language. Use separate paragraphs for the issue, evidence/impact, assessment, and fix. Include the Thread Replies block only when replies exist. Do not cram all metadata and body text into one list item, and do not use tables or large quote blocks to carry review content.

## PR Context

**Purpose and Scope**

Describe what the PR is trying to achieve, which issue or workflow it targets,
and the boundary of the change.

**Key Changes**

Describe the main implementation changes and the important code paths touched.

**Review Focus**

Describe the expected impact, important risks, compatibility concerns, or areas
reviewers should inspect.

---

## Open Review Comments

Open threads: <count> · Reasonable: <count> · Partially reasonable: <count> · Outdated: <count>

### C1 — [Comment title](comment-permalink)

`path:line` · `author`
Status: unresolved · current

**Issue**

Summarize the comment's concern and the trigger condition.

**Evidence / Impact**

Explain the current-code evidence and the user or system impact.

**Assessment**

`reasonable`

**Suggested Fix:**

Describe the smallest concrete remediation, expected behavior, and targeted
verification.

**Thread Replies**

- [Author](reply-permalink): Summarize this reply. Use one bullet per reply
  and retain the reply permalink.

### C2 — [Another comment title](comment-permalink)

`path:line` · `author`
Status: unresolved · current

**Issue**

...

**Evidence / Impact**

...

**Assessment**

`outdated/not applicable`

- Explain why the current code no longer requires a change. Do not invent a
  `Suggested Fix` for this assessment.

If there are no open review threads, write `No open review comments`.

## Findings

### [P1] `path:line` — Finding title

**Evidence / Impact**

Describe the distinct trigger, risk, and impact. This must not repeat an issue
already represented in `Open Review Comments`.

**Suggested Fix:**

Describe the smallest concrete change, expected behavior, and targeted
verification.

If there are no additional issues, write `No additional findings`. Do not
duplicate an open-comment assessment or invent a second fix for it.

## Open Questions
- List correctness-affecting questions, or say clearly that there are no
  meaningful open questions.

## Verification
- List commands and checks performed, or explain why validation was not run.
- State whether local or worktree preparation was used. For local preparation,
  include the checkout branch and upstream when applicable. For worktree
  preparation, include its absolute path, review branch, upstream or local-only
  status, and confirm the source checkout remained unchanged.
- When a collision fallback was used, name the
  `review/pr-<number>-<head-short-sha>` branch and the collision reason.
- State whether the latest-base merge action was triggered. If local mode was
  used, name the selected head or collision-fallback branch and distinguish
  the pre-merge `headRefOid` check from the post-merge head/base ancestry check.
  If worktree mode was used, list the isolated merge branch, conflict files,
  conflict resolution status, and confirm that no push was performed.
- Report the final live-state refresh: final `headRefOid`, PR `updatedAt`, and
  whether new reviews, threads, replies, or thread-state changes appeared after
  the initial snapshot. Report the final open-comment inventory, whether any
  additional findings remain, and state that every new or changed item was
  individually assessed, or describe the remaining limitation.
- Confirm that no push was performed unless the user explicitly asked for one.
- If merge conflicts could not be resolved safely, report that blocker here and
  do not claim that a full review was completed.

## Summary
Short neutral summary of the overall review result without repeating the PR
context.

Build from the checked PR title and body, linked issue, actual diff, and relevant surrounding code PR Context. Do not merely restate the PR description. Under each subheading, write one natural paragraph containing 2–4 sentences.

Priority meanings:

• P0: data loss, crash, security flaw, or core workflow breakage.
• P1: likely to cause a user-visible regression or incorrect behavior.
• P2: edge-case bug, missing compatibility, or incomplete issue coverage.
• P3: maintainability, clarity, or test/documentation gap worth fixing.