review-pr

Review a Pull Request

Review PR $ARGUMENTS and produce a confidence score for merge readiness.

Phase 1: Gather PR Context

# PR metadata
gh pr view $ARGUMENTS --json title,body,author,labels,files,additions,deletions,baseRefName,headRefName,state,isDraft,number

# Changed files list
gh pr diff $ARGUMENTS --name-only

# Full diff
gh pr diff $ARGUMENTS

# CI status
gh pr checks $ARGUMENTS

Phase 2: Classify PR Type

Determine the PR type from changed files and metadata:

For v8 packages (packages/react/): skip V9 pattern checks — those are maintenance-only with different patterns. For web-components (packages/web-components/): skip React-specific checks.

Phase 3: Run Checks

Run each check category. For each finding, assign a severity:

• BLOCKER — must fix before merge
• WARNING — should address
• INFO — consider

A. Beachball Change File

Required if any published package source code changed (not just tests/stories/docs).

• Check change/ directory in the diff for new .json files
• Verify change type: patch for fixes, minor for features, never major without explicit approval
• Not required for changes that only affect tests, stories, docs, or snapshots

BLOCKER if missing for published source changes.

B. V9 Component Pattern Compliance

Only for files in packages/react-components/react-*/library/src/:

Reference: docs/architecture/component-patterns.md

C. Dependency Layer Violations

For changes to package.json files or new imports in Tier 3 component packages:

• BLOCKER if a Tier 3 package (react-button, react-menu, etc.) adds a dependency on another Tier 3 package
• Allowed Tier 2 deps: react-utilities, react-theme, react-shared-contexts, react-tabster, react-positioning, react-portal
• Allowed Tier 1 deps: @griffel/react, @fluentui/tokens, @fluentui/react-jsx-runtime

Reference: docs/architecture/layers.md

D. SSR Safety

Grep added lines for unguarded browser API access:

Check 3 lines above each match for a guard (canUseDOM, typeof window !== 'undefined').

E. Testing

F. API Surface

G. Accessibility

H. Security and Quality

I. Documentation coverage

A code change frequently lands something that someone downstream needs to learn about. The "someone" splits into two audiences with very different reading habits, so this check actually walks two passes:

• Pass 1 — user-facing docs. Component consumers (storybook stories, migration guides, docsite, MDX, change-file comments).
• Pass 2 — harness / agent-facing docs. Future agent sessions reading the skills, AGENTS.md, docs/workflows/*, and docs/architecture/*. When a PR changes a build target, a script, a CI step, a label taxonomy, a project-board field, or an assumption that a skill makes, the skill or harness doc has to be updated in the same PR — otherwise every fresh /triage-issues, /visual-test, /review-pr, etc. invocation pays the discovery cost over again.

Both passes ask the same two questions: should this PR have updated docs, and if so, did it? Bug fixes that restore documented behavior, internal refactors, and test-only PRs default to PASS in both passes — the interesting cases are below.

Pass 1 — user-facing docs

Pass 2 — harness engineering / agent-facing docs

This pass exists because skills are part of the contract, not just convenience. When the skills are stale, the agents are wrong — quietly. Symptoms: a /visual-test invocation that points at a nonexistent project name, a /triage-issues flow that recommends a label the repo no longer has, a /triage-board skill that misses a new view filter the maintainers added.

Walk the diff for any of these and check whether the matching agent-facing doc was updated:

When docs are NOT expected (default to PASS)

• Pure bug fix that restores documented behavior
• Internal refactor with no exported-symbol or behavior change
• Build / CI changes that have no contributor-facing or skill-facing impact
• Test-only PRs
• Style-only PRs (typo, formatting) on internal code

Severity

How to actually run this check

The model should walk the diff, not guess.

1. Group changed files into surfaces. Code surfaces (library/src/**, index.ts barrels, *.types.ts); user-doc surfaces (stories/**, library/docs/**, apps/public-docsite-v9/**); harness surfaces (.agents/skills/**, .claude/**, AGENTS.md, CLAUDE.md, docs/workflows/**, docs/architecture/**, docs/team-routing.md, project.json, nx.json, root package.json scripts, .github/CODEOWNERS, .github/labeler.yml).
2. If the PR is purely a doc/harness surface, the check is PASS — it's documentation already.
3. For each row in the user-facing table that matches a code change in the diff, verify the corresponding doc surface was also touched. If not, raise the matching severity.
4. For each row in the harness-engineering table that matches, verify the corresponding skill or harness doc was also touched. Be specific: name the skill that needs updating, not just "a skill."
5. Cross-search: when a PR renames a path, target, or label, run grep -rn "<old-name>" .agents/skills/ docs/ AGENTS.md to find references that will break. Anything that turns up is a BLOCKER unless this PR also updates it.
6. Read the PR body before reporting. Authors often pre-empt with "docs follow-up tracked in #NNNN" or "harness skills updated in commit X" — those move findings from WARNING to INFO when the deferral is explicit and reasonable.

Phase 4: Calculate Confidence Score

Start at 100

For each BLOCKER:  -25 points
For each WARNING:  -5 points
For each INFO:     -1 point

Bonuses:
  +5 if tests added/updated alongside source changes
  +3 if change file present and well-described
  +2 if PR description is thorough

Floor at 0, cap at 100.

Score interpretation:

• 90–100: High confidence — safe to merge
• 70–89: Moderate confidence — minor concerns
• 50–69: Low confidence — needs attention
• 0–49: Not safe to merge — blockers present

Phase 5: Produce Output

Use this exact format (also used verbatim as the PR-comment body in Phase 6 — don't duplicate work).

Start directly with the score. Skip a header title, author, type, packages-affected, and CI-status preamble — when this is posted as a PR comment, all of that is already visible in the GitHub UI immediately above the comment, so repeating it just pushes the actually-useful content (score + findings) below the fold. The classification work from Phase 2 still happens; it just isn't echoed back at the reader.

### Confidence Score: <score>/100

<one-sentence summary>

### Findings

#### Blockers (must fix before merge)
- [ ] <finding with file:line reference>

#### Warnings (should address)
- [ ] <finding with file:line reference>

#### Info (consider)
- <finding>

### Category Breakdown

| Category | Status | Notes |
|----------|--------|-------|
| Change file | PASS/FAIL | ... |
| V9 patterns | PASS/WARN | ... |
| Dep layers | PASS/FAIL | ... |
| SSR safety | PASS/WARN | ... |
| Testing | PASS/WARN | ... |
| API surface | PASS/WARN | ... |
| Accessibility | PASS/WARN | ... |
| Security/Quality | PASS/WARN | ... |
| Docs coverage | PASS/WARN/FAIL | ... |

### Recommendation

APPROVE / REQUEST_CHANGES / COMMENT

<brief rationale>

Phase 6: Post the review back to the PR

After presenting the output in the chat, post the same text as a comment on the PR so the review is visible to maintainers and to Copilot (when the PR author is copilot-swe-agent, the comment becomes actionable feedback).

Save the output from Phase 5 to a temp file so the markdown isn't mangled by shell quoting, then:

gh pr comment $ARGUMENTS --repo microsoft/fluentui --body-file /tmp/pr-review-$ARGUMENTS.md

Append a single trailer line to the body so the post is identifiable:

---
*Posted via the `/review-pr` skill.*

The posted comment should be identical to what you rendered in chat — don't paraphrase or summarize. The chat output and the PR comment must match so the user can trust that what they saw is what the maintainers see.

Pre-checks before posting:

1. Confirm the active gh account has write access to the PR's repo (EMU accounts read fine but silently fail on writes):

   gh api graphql -f query='{ viewer { login } repository(owner:"microsoft", name:"fluentui") { viewerPermission } }'
   

   If viewerPermission is NONE, stop and ask the user to gh auth switch --user <non-emu-account>.
2. Don't post on PRs from your own branches unless explicitly asked — self-review comments are noise.
3. If REQUEST_CHANGES is the recommendation, still post the comment but note to the user that only a formal review (gh pr review --request-changes) actually blocks merge; a comment is advisory.

When to skip posting:

• The user explicitly asks for a review without posting ("review but don't post").
• The PR has an existing comment from this skill within the last day on the same head SHA — avoid duplicate noise. Look for the *Posted via the \/review-pr` skill.*` trailer.
• Draft PRs where the user is clearly still iterating (state=OPEN, isDraft=true, recent force-push) — offer to post but don't do it by default.

Notes

• For large PRs (50+ files), prioritize: published source files > test files > config. Note reduced confidence due to review scope.
• Draft PRs: still review but note WIP status.
• Merge conflicts: flag as BLOCKER if detected.
• The ### Confidence Score: NN/100 line must always appear on its own line for machine parsing.