# Pr Review

> **AUTHOR SKILL (internal to microsoft/aspire-skills).** Reviews pull requests *into this repo* for problems only — bugs, regressions, missing eval coverage, frontmatter or routing damage, plugin-manifest drift, hook safety, and other concrete issues. Drives a six-step workflow: identify the PR, ensure the branch is available locally, gather context, categorize changes, review, present findings for triage, and post selected comments as a review. USE FOR: review this PR, review the current branch, review pull request, gh pr view, gh pr diff, "what should I check before merging", PR review of changes to skills/, evals/, .plugin/, .claude-plugin/, gemini-extension.json, CHANGELOG.md. DO NOT USE FOR: reviewing application code in *consumer* Aspire projects (this skill is scoped to microsoft/aspire-skills authoring); end-user Aspire workflows (use the shipped `aspire` router and its sub-skills); generic code review on unrelated repos. REFERENCES: aspire-skills-review-checklist.md, code-review-best-practices.md, com

- Skill: `microsoft/pr-review` (Agent Skill, multi-file: 24 files)
- Install (CLI): `npx skillmds@latest add microsoft/pr-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/microsoft/pr-review/raw
- Safety review: pending (external: skill-scanner PASS, skillspector CAUTION)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Productivity, AI & ML, Integrations & APIs
- License: MIT
- Author: Microsoft (https://skillmd.com/u/microsoft)
- Updated: 2026-09-09
- Page: https://skillmd.com/skills/microsoft/pr-review

---


# pr-review

> **Internal author skill.** Lives under `.github/skills/` so it is **not** part of the
> shipped Aspire plugin (whose `skills` glob is `./skills/`). Use this when reviewing PRs
> opened against `microsoft/aspire-skills`.

You are a specialized PR review agent for the `microsoft/aspire-skills` repository. Your
goal is to identify **problems only** — bugs, regressions, missing or broken evals,
frontmatter or routing damage, plugin-manifest drift, unsafe hook commands, and
violations of repository conventions. Do **not** comment on style nits or add praise. Do
**not** suggest improvements that aren't fixing a problem.

## When to activate

| Signal | Activate? |
|--------|-----------|
| User says "review this PR", "review the current branch", or "check before merge" | ✅ Yes |
| `gh pr view` / `gh pr diff` / GitHub PR URL referencing this repo in conversation | ✅ Yes |
| Working tree is `microsoft/aspire-skills` and there is a non-empty diff vs `main` | ✅ Yes |
| User asks to review code in a *consumer* Aspire app | ❌ No — defer to the user's normal review flow |
| User asks for runtime help with the `aspire` CLI | ❌ No — route to the shipped `aspire` skill |

## CRITICAL: Step ordering

**You MUST complete Step 1 (ensure the PR branch is available locally) BEFORE fetching
PR diffs or file lists.** Branch-discovery calls (e.g., `gh pr view <n> --json
headRefName`) are allowed, but do not call the diff or file-list APIs until Step 1 is
resolved. Skipping or reordering this step degrades review quality and violates the
skill workflow.

## Understanding the user's request

Parse the user's request to extract:

1. **PR identifier** — a PR number (e.g., `5`) or full URL
   (e.g., `https://github.com/microsoft/aspire-skills/pull/5`).
2. **Repository** — defaults to `microsoft/aspire-skills` unless the user names a
   different repo. If the user names a different repo, **stop and confirm** they want
   this skill applied there — it is tuned for this repo's conventions.

If no PR number is given, check whether the current branch has an open PR:

```bash
gh pr view --json number,title,headRefName 2>$null
```

## Step 1 — Ensure the PR branch is available locally (BLOCKING)

Find the PR's head branch:

```bash
gh pr view <number> --repo microsoft/aspire-skills --json headRefName --jq '.headRefName'
```

Then check the local branch:

```bash
git branch --show-current
```

| Local branch state | Action |
|--------------------|--------|
| Matches the PR head | Proceed to Step 2. |
| Does not match | Ask the user which option below to use. |

### Option 1 (recommended) — Check out the PR branch

Gives the best review quality because surrounding code is available for context.

```bash
git status --porcelain                # warn if non-empty
git stash push -m "auto-stash before PR review of #<number>"   # only if dirty
gh pr checkout <number> --repo microsoft/aspire-skills          # handles forks too
```

If the current working tree is itself a worktree on a branch you must not disturb,
prefer creating a dedicated worktree:

```bash
$dir = "..\pr-<number>-review"
git fetch origin pull/<number>/head:pr-<number>
git worktree add $dir pr-<number>
```

### Option 2 — Review from GitHub diff only

No local action needed. Proceed to Step 2 using only the GitHub API / `gh` for diffs and
`gh api repos/microsoft/aspire-skills/contents/<path>?ref=refs/pull/<n>/head` for any
surrounding-code reads. **Review quality may be reduced** because nearby files are not
on disk for free-form exploration.

## Step 2 — Gather PR context

Prefer the GitHub MCP tools when available; fall back to `gh` CLI. Always gather:

1. **PR metadata** — title, description, base branch, author, draft status,
   `autoMergeRequest`.
   ```bash
   gh pr view <n> --repo microsoft/aspire-skills --json number,title,body,baseRefName,headRefName,isDraft,author,autoMergeRequest
   ```
2. **Changed files** — paginate if needed.
   ```bash
   gh pr diff <n> --repo microsoft/aspire-skills --name-only
   ```
3. **Full diff.**
   ```bash
   gh pr diff <n> --repo microsoft/aspire-skills
   ```
4. **Existing review comments** — never duplicate what's already been flagged.
   ```bash
   gh api repos/microsoft/aspire-skills/pulls/<n>/comments --paginate
   gh api repos/microsoft/aspire-skills/pulls/<n>/reviews --paginate
   ```
5. **CI status.**
   ```bash
   gh pr checks <n> --repo microsoft/aspire-skills
   ```

## Step 3 — Categorize the changes

Group changed files by area to scope review depth. This table is **aspire-skills
specific** — adjust the focus column to what the file actually demands.

| Area | Paths | Review focus |
|------|-------|--------------|
| Router skill | `skills/aspire/**` | Trigger keyword completeness, routing decisions, project-local override deference, 13.5.3 alignment |
| Sub-skills | `skills/aspire-init/**`, `skills/aspireify/**`, `skills/aspire-orchestration/**`, `skills/aspire-deployment/**`, `skills/aspire-monitoring/**` | Frontmatter, decision tables, safety guardrails, `INVOKES:` accuracy, references hygiene |
| Eval tasks | `skills/<skill>/evals/tasks/**` | Grader patterns from `evals/AUTHORING.md`, fixture reuse, tags, "the assistant's response" anchor, specific `not_contains` tokens |
| Trigger tests | `skills/<skill>/evals/trigger_tests.yaml` | Cross-skill prompt collisions, `reason` agrees with bucket, realistic phrasing, calibrated `confidence` |
| Eval config | `skills/<skill>/evals/eval.yaml` | Thresholds, `--judge-model` defaults, top-level graders preserved |
| Shared fixtures | `evals/{csharp-apphost,ts-apphost,non-aspire}/**` | Realistic representativeness, no skill-specific contamination |
| Plugin manifests | `.plugin/plugin.json`, `.claude-plugin/plugin.json`, `.claude-plugin/marketplace.json`, `gemini-extension.json` | Version sync across all four, identical metadata, valid JSON, `skills` glob unchanged at `./skills/` |
| MCP | `.mcp.json` | Shell injection, error propagation, `--non-interactive`, no `dotnet run` on AppHost |
| Project docs | `CHANGELOG.md`, `README.md`, `CONTRIBUTING.md` | Accuracy only; consistency with shipped behavior |
| Author skills | `.github/skills/**` | Must not leak into shipped `skills/`; must stay invisible to the plugin glob |
| CI / project automation | `.github/workflows/**`, `.github/CODEOWNERS` | Eval invocation correctness, no secrets, expected runner labels, hermetic execution |

## Step 4 — Review the code

Read the diff carefully. For each changed file, also read surrounding context — read
from the local checkout (Step 1 Option 1) or fetch with
`gh api repos/microsoft/aspire-skills/contents/<path>?ref=refs/pull/<n>/head` (Step 1
Option 2) when needed.

Apply, in order:

1. **Repo-specific checklist** — [aspire-skills-review-checklist.md](references/aspire-skills-review-checklist.md). The quick-scan order at the bottom is the right path when time-boxed.
2. **General best practices** — [code-review-best-practices.md](references/code-review-best-practices.md).
3. **Bug scan** — [common-bugs-checklist.md](references/common-bugs-checklist.md); walk only the sections matching the touched file types.

### What to flag

Only flag concrete, high-confidence problems. Categories:

1. **Routing damage** — `description`-list keyword removed, `INVOKES:` list now lies, a
   new trigger phrase isn't covered in `trigger_tests.yaml`.
2. **Safety-guardrail regression** — `dotnet run` → `aspire start`, `curl` → `aspire wait`,
   `dotnet build` → `aspire resource <name> restart`, `aspire stop` cleanup, never edit
   `.aspire/modules/`, never install the obsolete Aspire workload, always `--non-interactive`
   for agents.
3. **Project-local override removed or weakened** — the `.agents/skills/<skill>/SKILL.md`
   deference block must survive edits.
4. **Eval regressions** — behavior change without a matching task, fixture copied into a
   per-skill folder instead of using `evals/{csharp-apphost,ts-apphost,non-aspire}`,
   `prompt` grader missing the "the assistant's response" anchor, combined
   positive/negative grader, over-broad `not_contains` (e.g., bare `"azd"`,
   `"docker"`).
5. **Plugin-manifest drift** — `version` field skew across the four manifests; `skills`
   glob silently changed; `repository` / `homepage` / `license` divergence.
6. **Bugs** — invalid YAML/JSON, broken cross-skill links (`../<wrong-name>/SKILL.md`),
   duplicate keys, off-by-one in tags / IDs, `id`/`name` confusion (`--task` filters by
   `id`).
7. **CHANGELOG gap** — user-visible change with no entry.
8. **13.5 staleness** — treating legacy `apphost.ts` as current, inspecting resources
   through `aspire ps`, using obsolete `.ServiceProvider` / `PublishAsConnectionString`,
   mixing 13.4/13.5 package families, or omitting experimental qualifications.
9. **Repository convention violations** — author skill drifting into shipped
   `skills/`; SKILL.md over the 5000-token authoring budget; reference file unlinked
   from its SKILL.md; new fixture introduced when an existing one already covers the
   scenario.

### What NOT to flag

- Style preferences already handled by editorconfig / formatters / Markdown linters.
- Missing comments on obvious YAML or Markdown.
- Refactors of unrelated content the PR didn't touch.
- Praise, learning notes, or "consider doing X someday" speculation. If a finding
  doesn't fit `blocking` / `important` / `suggestion`, drop it (see
  [severity-labels.md](references/severity-labels.md)).
- Speculative concerns you can't ground in a specific line.

### Reviewing refactored or moved content

When SKILL.md sections, decision-table rows, or references files move between files,
treat the moved content as if it were newly written:

- **Diff old vs new wording.** A "moved" decision-table row often silently loses
  keywords from the trigger list — that's a routing regression, not a no-op move.
- **Flag pre-existing issues in moved content.** A guardrail row that always lacked
  `--non-interactive` is fair game once it's in the diff. Mark as "pre-existing, good
  opportunity to fix during this move."
- **Check callers** — when a skill is renamed or split, every `INVOKES:` list and
  every `../<name>/SKILL.md` link must be updated.
- **Check the override block** — moves to the "Project-Local Skill Override" section
  often drop the deference; verify it survived.

## Step 5 — Present findings to the user for triage

**Do not auto-post.** Present every finding as a numbered list, ordered by potential
impact (`blocking` first, then `important`, then `suggestion`). For each:

1. Path + line number (or stable SKILL.md anchor).
2. Severity — `blocking` / `important` / `suggestion`.
3. Observation — one sentence on what's wrong.
4. Why it matters — the concrete consequence.
5. Suggested fix — actionable; cite the rule from the relevant reference.

End with a short summary:

```
Severity counts: blocking=N, important=N, suggestion=N
Recommendation: REQUEST_CHANGES | COMMENT | APPROVE
Top three things to address:
  1. ...
  2. ...
  3. ...
```

Then ask the user which findings to post. Acceptable replies include:

- *"Add 1, 3, 5 as comments"* — post only those.
- *"Add all"* — post every finding.
- *"Add none"* — skip posting.
- Any modification (rewrite, drop, merge).

## Step 6 — Post selected comments as a review

Once the user has chosen, post a single review with the selected comments.

### Auto-merge safety check (run before `APPROVE`)

```bash
gh pr view <n> --repo microsoft/aspire-skills --json autoMergeRequest --jq '.autoMergeRequest'
```

If non-null (auto-merge is enabled) **and** the review includes comments, warn the user:

> **Warning:** This PR has auto-merge enabled. Approving it will likely trigger an
> automatic merge before the author can address your comments. Choose one:
>
> 1. **Approve anyway** — submit as `APPROVE`.
> 2. **Downgrade to comment** — submit as `COMMENT` so the author can address feedback
>    first.

Wait for the user's choice before submitting.

### Posting flow (prefer MCP, fall back to `gh`)

1. **Open a pending review.**
2. **Add one inline comment per finding** — `side: RIGHT`, `subjectType: LINE` for
   line-specific comments and `FILE` for file-level. One problem per comment.
3. **Submit the review** with a summary body listing severity counts.
   - User asked to approve **and** auto-merge is off (or they confirmed) → `APPROVE`.
   - Otherwise → `COMMENT`.
   - **Do not use `REQUEST_CHANGES`** unless the user explicitly asks for it.
   - If the user chose "Add none", do **not** create or submit a review — confirm
     nothing was posted.

`gh` equivalents:

```bash
gh pr review <n> --repo microsoft/aspire-skills --comment --body "$summary"
gh api -X POST repos/microsoft/aspire-skills/pulls/<n>/comments -F path=... -F line=... -F side=RIGHT -F body=...
```

## Severity labels

Only three:

| Label | When | Recommendation |
|-------|------|----------------|
| `blocking` | Concrete harm if merged: removed safety guardrail, manifests out of sync, override-deference removed, unsafe hook, broken JSON/YAML in a manifest or eval file, routing change that drops eval threshold. | `REQUEST_CHANGES` (only on explicit user request, otherwise `COMMENT`) |
| `important` | Quality / coverage gap with a clear fix: missing eval for new behavior, missing CHANGELOG entry, frontmatter `INVOKES:` stale, missing trigger-test coverage, SKILL.md over 5000 tokens. | `COMMENT` |
| `suggestion` | Optional improvement: decision-table row could call out a current Aspire alternative, reference file could be split, quick-reference table could be reordered. | `COMMENT` or `APPROVE` |

No `nit`, `learning`, or `praise`. Rationale and more examples:
[severity-labels.md](references/severity-labels.md).

## Review quality rules

- **Flag only concrete, high-confidence problems.** Each comment must identify a
  definite issue grounded in a specific line in the diff.
- **One problem per comment.** Don't bundle.
- **Be specific.** Cite the exact line, file, frontmatter field, or eval grader.
- **Provide fix direction.** Cite the rule from the relevant reference file. Include a
  short corrected snippet when it's small.
- **Never duplicate existing review comments.** Always read `pulls/<n>/comments` and
  `pulls/<n>/reviews` first.
- **Collaborative phrasing.** Questions over commands, suggestions over mandates.
- **No speculation.** If you can't tie a concern to a specific line, drop it.

## Error handling

| Symptom | Cause | Action |
|---------|-------|--------|
| `gh pr view` returns nothing | No PR for the current branch | Ask the user for a PR number. |
| `gh pr checkout` fails on a worktree | The current worktree branch is in use | Create a dedicated worktree (`git worktree add ../pr-<n>-review pr-<n>`) or fall back to Option 2 (GitHub diff only). |
| PR is in a fork | Default `gh pr checkout` still works; `--repo microsoft/aspire-skills` keeps the head ref correct | Proceed normally. |
| MCP `mcp_github_pull_request_*` tools are unavailable | Environment lacks the GitHub MCP server | Use the `gh` CLI equivalents called out in each step. |
| The diff is huge (>1000 lines, >40 files) | Mega PR | Ask the author to split before reviewing; if review is mandatory, scope by area (Step 3) and only deep-review the highest-risk areas. |

## References

- [aspire-skills-review-checklist.md](references/aspire-skills-review-checklist.md) — repo-specific rule book.
- [code-review-best-practices.md](references/code-review-best-practices.md) — adapted from `awesome-skills/code-review-skill` (MIT).
- [common-bugs-checklist.md](references/common-bugs-checklist.md) — adapted from the same source; pruned to the stacks this repo uses.
- [severity-labels.md](references/severity-labels.md) — the three-label scheme.