Quick Pr Review

---
name: quick-pr-review
description: Rapidly review and approve a GitHub pull request to unblock others. Approves unless there are significant risks or significant public interface changes.
allowed-tools: Bash(gh:*, ghx:*, git:*, say, ~/.agents/scripts/should-post-to-github:*), Read
argument-hint: "<owner/repo> <pr-number>"
---

# Quick PR Review

Rapidly reviews a GitHub pull request and prepares an approval decision to unblock others.
Creates or updates a single review comment per PR, keyed to the latest commit.
Approves automatically unless there are significant risks or significant public interface changes (removals, breaking changes, or substantial new API surface).
Whether the review comment is posted to GitHub (and the PR approved) is decided by `should-post-to-github` (based on `~/.sdlc/config.yaml`); when posting is disabled the comment is composed and saved locally without posting or approving.

## Prerequisites

- `gh` CLI authenticated with write access to the target repository
- `$1`: repository in `owner/repo` format
- `$2`: PR number identifying an open pull request
- Apply the shared communication guidelines in `skills/communication-guidelines/SKILL.md` when composing the review comment.

## Workflow

```
Fetch PR metadata + latest commit SHA
              |
              v
     Load author trust profile
     (neutral if not found)
              |
       always_reject?
        /          \
      Yes            No
       |              |
     Skip           Find existing review comment
     (report        (<!-- quick-pr-review: marker)
      to user)               |
                       Same commit?
                        /         \
                       Yes           No (new or updated)
                       |                    |
                     No-op            Run checks
                                     (cautious = stricter)
                                          |
                               Checks pending? --Yes--> wait + re-check
                                          |              (up to 10x)
                                          No
                                          |
                               Any blocking failures?
                                  /           \
                                Yes             No
                                 |               |
                           Post/update    Manual-approval org
                           comment only   (Shopify/shop)?
                                            /         \
                                          Yes          No
                                           |            |
                                    Post/update      Approve +
                                    comment only,    post/update
                                    alert user       comment
                                           \            /
                                            \          /
                                        Update trust profile
```

## Steps

### 1. Gather PR information

```bash
ghx pr view $2 --repo $1 --json --refresh
gh pr view $2 --repo $1 --json headRefOid,author,statusCheckRollup
gh pr diff $2 --repo $1
```

Extract:
- `REPO`: `$1` (`owner/repo`)
- `OWNER`: the organization/owner portion of `$1` (before the `/`)
- `HEAD_COMMIT`: the `headRefOid` (latest commit SHA, full)
- `SHORT_SHA`: first 7 characters of HEAD_COMMIT
- `PR_AUTHOR`: the `author.login` (GitHub username of the PR author)

**Manual-approval organizations**: if `OWNER` (case-insensitive) is `Shopify` or `shop`, set `MANUAL_APPROVAL_ORG=true`.
When `MANUAL_APPROVAL_ORG=true`, run all checks and post/update the review comment as normal, but **never run `gh pr review --approve` automatically** (see step 8).

Also resolve agents attribution for the review comment footer: read [`github-post-attribution/SKILL.md`](../github-post-attribution/SKILL.md) and compute `SKILL_COMMIT`, `SKILL_SHORT_SHA`, `SKILL_FILE_URL`, and `{BASE}` for `SKILL_DIR` = `quick-pr-review`.
Use the **Reviewed with** line and optional `{BASE}/issues/new` sub-line from that skill.

Extract:
- `SKILL_COMMIT`: full commit SHA of the agents repo
- `SKILL_SHORT_SHA`: short SHA from the same procedure
- `SKILL_FILE_URL`: URL to `skills/quick-pr-review/SKILL.md` at `SKILL_COMMIT`

### 2. Load author trust profile

`TRUST_PROFILE_PATH`: `~/.developer-trust/{PR_AUTHOR}.md`

If the file exists, read it and extract:
- `TRUST_LEVEL`: the value after `**Level**:` (one of `trusted`, `neutral`, `cautious`, `always_reject`)
- `TRUST_REASON`: the value after `**Reason**:`

If the file does not exist, default to `TRUST_LEVEL=neutral` and `TRUST_REASON=` (no prior history).

**If `TRUST_LEVEL == always_reject`**: stop immediately.
Do not fetch the diff, post a comment, or approve.
Report to the user: "Skipped PR #{PR_NUMBER} ({REPO}) — author is flagged for manual review only."

The trust level modifies behavior in steps 2, 4, and 8:
- `trusted`: Standard checks. On borderline cases (e.g., a check whose result is ambiguous), prefer passing.
- `neutral`: Standard behavior (no adjustment).
- `cautious`: Apply stricter interpretation. Flag marginal cases as failing.
- `always_reject`: Skip this PR entirely. Do not post a comment or approve. Report to the user that the PR was skipped.

### 3. Find existing review comment

```bash
gh api repos/{REPO}/issues/$2/comments \
  --jq '.[] | select(.body | test("<!-- quick-pr-review:")) | {id: .id, body: .body}' \
  | head -1
```

If a comment exists, extract the commit SHA from the marker line `<!-- quick-pr-review:COMMIT_SHA -->`.

If `COMMENT_COMMIT == HEAD_COMMIT`: output "Review already up to date for commit `SHORT_SHA`." and stop.

### 4. Run review checks

Evaluate each item below.
Record each as passing (`[x]`) or failing (`[ ]`).
Checks are ordered by impact/risk level (highest first).

When `TRUST_LEVEL == cautious`: apply stricter interpretation.
When a check is borderline (e.g., a change is arguably a public interface addition but minor), treat it as failing.

#### Public interface impact (approval gate)
- Scan the diff for **significant changes to public interfaces**, both removals and additions:
  - Removing or renaming exported functions, classes, or types
  - Removing or changing API endpoints (HTTP routes, RPC methods)
  - Removing public configuration keys or environment variables
  - Breaking changes to serialization formats (JSON fields removed, renamed)
  - Introducing new exported classes, types, protocols, or public method signatures
  - New public interfaces that commit to a non-extensible design and break forward compatibility: closed enums that reject unknown values, schemas that fail on unknown fields, serialization formats or API contracts with no versioning path, or fixed-set assumptions that make a future additive change breaking
  - ADRs, specs, or design docs that define or commit to new public API contracts (even if the diff is markdown, the intent is to establish an interface)
  - Dependency version bumps that are effectively major: for pre-release packages (version < 1.0.0), a minor version bump (e.g. 0.20→0.22) is equivalent to a major version change under semver and may introduce breaking API changes
- If any significant public interface changes are found (removals, breaking changes, substantial new API surface, or new contracts that are not forward compatible): **do not approve**

#### Security-sensitive changes (approval gate)
- Scan the diff for changes to:
  - Authentication or authorization logic (login, token validation, session management, permission checks)
  - Cryptographic code (hashing, encryption, key generation, certificate handling)
  - Secret/credential patterns (API keys, tokens, passwords, `.env` files, credentials in config)
  - Security-critical configuration (CORS, CSP, TLS settings, OAuth scopes, RBAC rules)
  - Input validation or sanitization that guards against injection attacks
- If any security-sensitive changes are found: **do not approve**

#### New dependencies (approval gate)
- Scan the diff for additions to dependency manifests:
  - `package.json`, `package-lock.json`, `yarn.lock`, `pnpm-lock.yaml`
  - `Gemfile`, `Gemfile.lock`
  - `requirements.txt`, `pyproject.toml`, `uv.lock`, `poetry.lock`
  - `go.mod`, `go.sum`
  - `Cargo.toml`, `Cargo.lock`
  - Or any other language-specific dependency file
- If new dependencies are introduced (not just version bumps of existing ones): **do not approve**

#### Change is reversible
- Scan the diff for:
  - Database migrations that DROP columns, tables, or indexes without a corresponding rollback
  - Deletion of data files or configuration that cannot be recovered from source control
  - Permanent data transformations with no undo path
- Infrastructure-level destructive operations (e.g., `terraform destroy` patterns)

#### Tests pass
- Check CI status from `statusCheckRollup` in the PR JSON.
- If any required checks are still **pending** or **in progress** (not yet concluded): wait and re-check rather than treating the PR as ready.
  - Wait ~60 seconds, then re-fetch the PR JSON with `gh pr view $2 --repo $1 --json statusCheckRollup --jq '.statusCheckRollup'` and re-evaluate `statusCheckRollup`.
  - Repeat until all required checks have concluded (passing, failing, or skipped), up to a maximum of 10 attempts (~10 minutes).
  - If checks are still pending after the maximum attempts: do not approve. Post/update the comment with `[ ] Tests pass` noting that checks did not conclude in time, and report to the user that the review should be re-run once CI completes.
- Once all required checks have concluded: all required checks must be passing or skipped (not failing).

#### Change is part of the spec (approval gate)
- Read the PR title and description.
- If no issue is referenced (e.g., `Fixes #N`, `Closes #N`, `Refs #N`, or a plain `#N` link): **do not approve** and ask the author to update the PR description with a reference to the issue that explains why this PR exists.
  - Try to find potentially matching open issues for this PR by searching the repository (e.g., `ghx issue list --repo {REPO} --state open --search "<keywords from PR title/description>"`).
  - Suggest at most 3 candidates as a bullet list, listing only the issue number (no title), e.g.:
    - `#41`
    - `#58`
    - `#72`
  - If no plausible matches are found, omit the list.
- If an issue is referenced: fetch the issue with `ghx issue view {N} --repo {REPO} --json` and extract its acceptance criteria (any checklist, "Acceptance Criteria" section, or equivalent).
- If acceptance criteria are found: verify that the diff satisfies them.
- If the PR is not aligned, **do not approve** and list the unmet criteria, asking the author to provide a justification or update the PR.
- If the issue has no acceptance criteria: treat this sub-check as passing (no way to verify alignment).
- The change should also have no unexplained scope creep beyond what the linked issue describes.

#### Documentation updated
- Does the diff include updates to README, docs/, or relevant user-facing documentation when the change adds or modifies user-visible behavior?
- For purely internal changes (refactors, tests), documentation is not required.

### 5. Compose the review comment body

```
<!-- quick-pr-review:HEAD_COMMIT -->
## Quick PR Review

Automated review to unblock merging.

Reviewed commit: SHORT_SHA
✅ **Approved** _or_ ❌ **Not approved**

- [x/[ ]] No significant public interface changes
- [x/[ ]] No security-sensitive changes
- [x/[ ]] No new dependencies
- [x/[ ]] Change is reversible
- [x/[ ]] Tests pass
- [x/[ ]] Issue referenced
- [x/[ ]] Change aligns with acceptance criteria
- [x/[ ]] Documentation updated

<details>
<summary>Evaluation details</summary>

### No significant public interface changes
<Reasoning>

### No security-sensitive changes
<Reasoning>

### No new dependencies
<Reasoning>

### Change is reversible
<Reasoning>

### Tests pass
<Reasoning>

### Issue referenced
<Reasoning>

### Change aligns with acceptance criteria
<Reasoning>

### Documentation updated
<Reasoning>

</details>

---
Reviewed with [quick-pr-review](SKILL_FILE_URL) (`SKILL_SHORT_SHA`)
<sub>This should not have been approved? [Let me know]({BASE}/issues/new).</sub>
```

Substitute `SKILL_FILE_URL`, `SKILL_SHORT_SHA`, and `{BASE}` per [`github-post-attribution/SKILL.md`](../github-post-attribution/SKILL.md).

For each failing check, append a block after the bullet list:

```
# <Check name>
<One-line explanation of what failed and what needs to be addressed>
```

Example with two failures:

```
<!-- quick-pr-review:abc1234... -->
## Quick PR Review

Automated review to unblock merging.

Reviewed commit: abc1234
❌ **Not approved**

- [ ] No significant public interface changes
- [x] No security-sensitive changes
- [x] No new dependencies
- [x] Change is reversible
- [ ] Tests pass
- [x] Issue referenced
- [x] Change aligns with acceptance criteria
- [x] Documentation updated

# No significant public interface changes
`removeUser()` is removed from the public SDK without a deprecation period or major version bump.

# Tests pass
CI check `unit-tests` is failing - fix the failing tests before merging.

<details>
<summary>Evaluation details</summary>

### No significant public interface changes
`removeUser()` is deleted from `sdk/public.ts` (an exported function) without a deprecation period or major version bump. This is an irreversible breaking change.

### No security-sensitive changes
No authentication, authorization, cryptographic, or security-critical configuration changes detected in the diff.

### No new dependencies
No changes to dependency manifests detected in the diff.

### Change is reversible
No database migrations, data deletions, or infrastructure-level destructive operations found in the diff.

### Tests pass
CI check `unit-tests` is failing with 3 test failures in `test_user.py`. All other checks (lint, build) are passing.

### Issue referenced
PR description references issue #41 (`Fixes #41`).

### Change aligns with acceptance criteria
Fetched issue #41 — acceptance criteria: (1) user module split into separate files, (2) no public API changes.
Both are satisfied by this diff.
No unexplained scope creep.

### Documentation updated
No user-facing behavior changes detected; documentation update not required.

</details>

---
Reviewed with [quick-pr-review](https://github.com/tomzx/agents/blob/abc1234deadbeef.../skills/quick-pr-review/SKILL.md) (`abc1234`)
<sub>This should not have been approved? [Let me know](https://github.com/tomzx/agents/issues/new).</sub>
```

(Example URLs show the pattern; substitute real `SKILL_FILE_URL`, `{BASE}`, and SHAs from your repo.)

### 6. Save review to local repository

Write the comment body to a file in `~/.quick-pr-review` and commit it:

```bash
OWNER=$(echo {REPO} | cut -d/ -f1)
REPO_NAME=$(echo {REPO} | cut -d/ -f2)
REVIEW_DIR=~/.quick-pr-review/${OWNER}/${REPO_NAME}
REVIEW_FILE=${REVIEW_DIR}/$2-{SHORT_SHA}.md

git -C ~/.quick-pr-review rev-parse --git-dir > /dev/null 2>&1 || git init ~/.quick-pr-review
mkdir -p "${REVIEW_DIR}"
printf '%s' "{COMMENT_BODY}" > "${REVIEW_FILE}"
git -C ~/.quick-pr-review add "${REVIEW_FILE}"
git -C ~/.quick-pr-review commit -m "Review {REPO}: PR #$2 @ {SHORT_SHA}"
```

### 7. Create or update the comment

Run `~/.agents/scripts/should-post-to-github --repo "{REPO}" --author "{PR_AUTHOR}"`. If it exits 1, skip this step (the comment body is already saved locally in step 6).

**If no existing comment:**
```bash
ghx pr comment $2 --repo {REPO} --body "{COMMENT_BODY}"
```

**If existing comment (different commit):**
```bash
gh api repos/{REPO}/issues/comments/{COMMENT_ID} \
  -X PATCH \
  -f body="{COMMENT_BODY}"
```

### 8. Approve or not

Only run the approval when step 7's `should-post-to-github` check exited 0 (posting allowed). If it exited 1, report the approval decision to the user without executing it on GitHub.

**If `MANUAL_APPROVAL_ORG == true`** (owner is `Shopify` or `shop`): **never approve automatically**, regardless of check results.
Post/update the comment as normal, then leave the approval to the user.
Speak the audible alert (see **Output**) so the user knows their manual review is needed.

**Approve** when all of the following are true (and `MANUAL_APPROVAL_ORG != true`):
- No significant public interface changes (removals, breaking changes, substantial new API surface, or new contracts that are not forward compatible)
- No security-sensitive changes
- No new dependencies added
- No failing checks that represent significant risk (tests failing, or non-reversible destructive operations)

```bash
gh pr review $2 --repo {REPO} --approve
```

**Do not approve** when:
- The owner is a manual-approval organization (`Shopify` or `shop`)
- Significant public interface changes are detected (removals, breaking changes, substantial new API surface, forward-incompatible new contracts, or specs/ADRs that define new public contracts)
- Security-sensitive changes are detected (auth, crypto, secrets, security config, input validation)
- New dependencies are introduced
- Tests are failing
- Change is not reversible and involves destructive operations

In the do-not-approve case, only post/update the comment.
Do not request changes automatically unless the issue is clearly blocking (e.g., tests failing, data loss risk).

### 9. Update developer trust profile

After posting the comment and applying the approval decision, update the author's trust profile:

```
/developer-trust-profile {PR_AUTHOR} --after-review {REPO} {PR_NUMBER} {approved|not_approved}
```

This records the review outcome and observations in `~/.developer-trust/{PR_AUTHOR}.md`, creating the file if it does not exist.

## Output

Report to the user:
- Whether the comment would be created or updated (or was created/updated if posting was allowed)
- The short commit SHA reviewed
- Whether the PR would be approved or not, and why (or was approved if posting was allowed)
- Whether the trust profile was created or updated (and at which path)

### Notify the user when their review is needed

If the PR was **not approved** (including when it was withheld because the owner is a manual-approval organization such as `Shopify` or `shop`), or posting was disabled so the review is pending the user's action, or you otherwise need the user to personally review the PR (e.g., ambiguous risk, policy judgment), speak a short audible alert on macOS so they notice even if the chat is in the background:

```bash
say "{REPO} #{PR_NUMBER} needs your review"
```

Use exactly that wording (substitute `REPO` and `PR_NUMBER` only).
Run `say` **after** you have posted or updated the GitHub comment (if posting was allowed), or after saving the review locally (if posting was disabled), in addition to the normal text report above.

**Do not** run `say` when the PR was **skipped** because `TRUST_LEVEL == always_reject` (no review, no comment, only the text report to the user).

## Example Usage

**Scenario 1: Clean PR, approve**
```
/quick-pr-review owner/myrepo 42
```
All checks pass, no public interface changes.
Approve and post review comment (unless `should-post-to-github` disables posting, in which case the review is composed and saved locally without posting or approving).

**Scenario 2: Failing CI**
```
/quick-pr-review owner/myrepo 88
```
CI is red.
Post comment with `[ ] Tests pass` and the failing check name.
Do not approve.

**Scenario 3: Significant public interface change**
```
/quick-pr-review owner/myrepo 55
```
Diff removes a public API method or introduces substantial new API surface.
Post comment with `[ ] No significant public interface changes`.
Do not approve.

**Scenario 4: Re-run on same commit**
```
/quick-pr-review owner/myrepo 42
```
Review comment already exists for the current HEAD commit.
Skip and report "already up to date".

**Scenario 5: Author with cautious trust level**
```
/quick-pr-review owner/myrepo 99
```
Author profile exists with `cautious` level.
Applies stricter check interpretation.
A borderline new export that might normally pass is flagged as failing.
Profile is updated with new review entry.

**Scenario 6: Author with always_reject trust level**
```
/quick-pr-review owner/myrepo 77
```
Author profile has `always_reject` level.
Skill stops immediately after loading the profile.
No comment is posted, no approval issued.
Reports: "Skipped PR #77 (owner/myrepo) — author is flagged for manual review only."

**Scenario 7: First review for an unknown author**
```
/quick-pr-review owner/myrepo 101
```
No trust profile found for the author.
Defaults to `neutral`.
After review, creates a new profile at `~/.developer-trust/{author}.md` with the first review history entry and initial observations.

**Scenario 8: Manual-approval organization**
```
/quick-pr-review Shopify/myrepo 200
```
The owner is `Shopify` (a manual-approval organization).
All checks are run and the review comment is posted/updated as normal, but the PR is **never approved automatically**.
The user is alerted via `say` that their manual review is needed.

**Scenario 9: Checks still pending**
```
/quick-pr-review owner/myrepo 120
```
CI checks are still running.
Wait ~60 seconds and re-fetch the PR JSON, repeating until checks conclude (up to 10 attempts).
Once checks pass, proceed to approve.
If checks never conclude, post comment with `[ ] Tests pass` and do not approve.

## Useful Commands Reference

| Command | Description |
|---|---|
| `ghx pr view <pr> --repo <owner/repo> --json --refresh` | Fetch PR metadata including title, description, and author (fresh) |
| `gh pr view <pr> --repo <owner/repo> --json headRefOid,statusCheckRollup` | Fetch the PR head commit SHA and current CI status |
| `gh pr diff <pr> --repo <owner/repo>` | Show the full PR diff |
| `ghx pr comment <pr> --repo <owner/repo> --body "..."` | Post a new comment on the PR |
| `gh api repos/{owner}/{repo}/issues/comments/{id} -X PATCH -f body="..."` | Update an existing comment |
| `gh api repos/{owner}/{repo}/issues/<pr>/comments` | List all comments on a PR |
| `gh pr review <pr> --repo <owner/repo> --approve` | Approve the PR |
| `say "..."` | macOS TTS: alert the user when manual review is needed (see **Output**) |