Review Prs

Review PRs Skill

A single, repeatable pass over everything that needs PR-review attention right now — the Perri daemon queue, which now includes a dedicated dependabot bucket for bot-authored PRs alongside the human-review buckets.

Designed to run from a fresh context: each cycle is mostly self-contained and does not need prior conversation history. Start it after a /clear (or in a fresh session) for maximum room.

The Approval Principle (never violated)

Every action posted to GitHub — approve, comment, request-changes, merge — must be covered by an explicit approval from the user, given in the same session. One approval MAY cover a batch, provided the approval request clearly enumerates exactly which PRs and what action it authorizes.

Corollaries:

• A batch approval is valid only when the request lists the exact PR numbers and the exact action ("Approve these 5: #x, #y, #z, #a, #b?"). Vague asks ("approve everything?") do not satisfy it.
• Comments are almost always 1:1 (the body differs per PR), so they go individually — but the same rule holds: clear ask, explicit yes.
• Merges are actions too. If a group will be approved and merged (typical for green dependabot PRs), the approval request must say so.
• Submissions that carry a comment or a request-changes go through the submit-review skill, which provides the per-PR confirmation UI. The batch shortcut (one confirmation → loop the approvals) is reserved for trivial, clean, and green-dependabot groups, where the action is a bare approve (± merge) with no comment.

Step 1 — Gather the queue

Single source — the Perri daemon queue is now the unified source of truth for all PRs, including dependabot. Fetch it the same way perri.md's Startup does:

In Nostromo (MCP available): perri.list_pr_queue(). Otherwise (standalone): ~/.claude/bin/perri-queue-pane --json.

The field list is identical either way — perri.list_pr_queue()'s items match perri-queue-pane --json's .items[] field-for-field, so Steps 2–4 below don't care which source Step 1 used:

Parse the items: each has repo, number, title, author, bucket, new_activity, url, ci_state, is_bot.

Bucket values:

• "requested", "needs_review", "changes_req" — human-review PRs (unchanged)
• "dependabot" — bot-authored PRs; is_bot == true (daemon-sourced, no separate fetch needed)

Do NOT run a separate gh search prs --author app/dependabot query. The daemon (is_bot field in perri_queue_native.rs) is the single source of truth for what counts as a bot PR. Running a parallel search would re-introduce the two-source divergence this integration was designed to close.

Closed/merged PRs are automatically excluded by the daemon (it checks the PR detail state / merged_at fields independently of the GitHub search index). Do not filter on those fields yourself.

Report a one-line summary by bucket before proceeding.

Step 2 — CI gate + classify every PR

Check each PR's status (gh pr view <n> --repo <r> --json statusCheckRollup,mergeable). Classify into exactly one class:

Note: the daemon already filters out dependabot PRs with hard Actions CI failures (dependabot-blocked). If a bucket == "dependabot" PR appears in the queue, it passed the daemon's CI gate — you still check for flaky iac-plan failures specifically, but hard failures are pre-filtered.

The trivial gate

trivial is the one class approved without being read for logic, so it is gated rather than guessed. perri.md → ## Trivial PR Batch is the authority and this skill must not diverge from it; the gate in short, tested in this order:

1. Homogeneity — disqualifies alone. Every changed file belongs to the one claimed trivial category (machine-generated / hand-authored trivial / bulk mechanical). Not the dominant category, and never on the strength of the title. One out-of-category file demotes the PR — name the file.
2. Sensitive surfaces — absolute. Authentication, authorization, permission or policy gates, payments or billing, DB schema or migrations, CI/CD or deploy configuration, secrets or credentials, rate limiting or throttling, crypto or signature verification. No category and no line count overrides this. A one-line config-only flip of a permission gate default is not trivial.
3. Size — only after 1 and 2 pass, and only ever subtractively. Machine-generated diffs (lockfiles, resolved-dependency files) are size-exempt at any length, but more than 30 changed lines outside the generated files demotes. Hand-authored trivial diffs (docs, prose, comments, config values) have a ceiling of 200 changed lines. Bulk mechanical diffs (formatter sweeps, codemods, mechanical renames) are size-exempt when every sampled hunk shows the same transformation — state the transformation and the number of hunks sampled, and demote the whole PR on any non-conforming hunk.

Size never promotes. There is no line count small enough to make a PR trivial. Smallness is not a category: if you cannot name the trivial category a PR matches, it is not trivial no matter how few lines it changes. Never justify a trivial classification with "only N lines" or "small change".

Every demotion here is a read, never a rejection. Homogeneity, sensitivity and size demotions all route to clean — never to discuss, never to skip. The PR gets read and can still be batch-approved with the clean group, so state it as "reading it as a clean candidate", not as blocked or deferred. Where the classification is genuinely ambiguous, treat the PR as non-trivial and say the classification was uncertain rather than giving an unqualified verdict.

These ceilings are unrelated to the >1k lines → delegate the deep read guidance under "Per-PR analysis" below. That one decides how a read is performed once a read is happening; these decide whether a read happens at all. Never state either in terms of the other.

CI gate rule: never review on top of red CI except when the only red is the approval check itself. List everything skipped at the end so nothing is silently dropped.

Step 3 — Work the groups in order

Order: dependabot-green → dependabot-flake → trivial → amber (changes_req + new_activity) → requested → needs_review (clean) → comment → discuss.

For each group:

In Nostromo, before reading any PR individually — i.e. every clean, comment, and discuss PR; not the unread trivial batch, which is deliberately never opened — pick it up the same way perri.md's Per-PR Review Workflow step 1 does:

perri.load_pr({ number, repo })
nostromo.show({ type: "pr_conversation", target: { repo, number } })
nostromo.show({ type: "pr_diff", target: { repo, number } })

This puts the PR's conversation and diff on screen while you form the verdict, and tells the daemon which PR is under review (drives pinning and the reset rule). Never pass highlights to load_pr on this path — see perri.md for why. Standalone (no Nostromo), skip straight to reading the PR the existing way.

dependabot-green

• Enumerate the PRs (number, repo, title).
• One batch approval request: "Approve and merge these N dependabot PRs: …?"
• On yes: gh pr review --approve then merge per repo convention:
  • admin-portal uses a merge queue → gh pr merge <n> --repo <r> --auto
  • family-portal / payments → gh pr merge <n> --repo <r> --squash --auto
• Refresh: ~/.claude/lib/perri-refresh.sh --clear

dependabot-flake

• Rerun failed iac-plan jobs (gh run rerun <run-id> --failed --repo <r>).
• Recheck after; promote to dependabot-green if it clears. Do not block the rest waiting.

trivial / clean

• For trivial, present each PR with a one-line basis line: matched category, file count, changed-line count, sensitive-path clearance — plus the transformation and hunks sampled for bulk mechanical. E.g. • operations #211 — bump axios 1.7.2 → 1.7.9 / [dep bump · 2 files · manifest +2/-2, lock +2,104/-1,880 · no sensitive paths]. This is required, not a nicety: the user is authorizing an unread approval, and the basis line is the only thing that makes a misclassification visible to him before he confirms. Never drop it to save space.
• List anything demoted out of trivial alongside the batch, each with its reason and the words "reading it as a clean candidate". A demotion moves the PR into the clean group below — it is read, and it can still be batch-approved. It is never a block, a deferral, or a skip.
• For clean, show the per-PR verdict first (one or two lines each) so the user sees what they're approving.
• One batch approval request enumerating the exact PRs + "Approve (no comment)?".
• On yes: loop gh pr review <n> --repo <r> --approve, then perri-refresh.sh --clear.

comment

• In Nostromo, when raising a specific finding, show it rather than narrating it — nostromo.show({ type: "file", target: { path }, anchor: { kind: "line", line }, emphasis: [...], reason: "<short phrase>" }), same as perri.md's Per-PR Review Workflow step 3. reason is required in practice: it becomes the tab's caption. Say what's wrong; let the shown file carry the code.
• Draft the full comment body and show it before asking.
• Invoke the submit-review skill per PR (it provides the confirmation UI, and inside Nostromo poses the decision via nostromo.ask_decision). Never call gh pr review directly for a comment/request-changes.

discuss

• In Nostromo, same as comment above: show the file at the line (or the ticket, for an acceptance-criteria question) instead of quoting it back, with a reason.
• Summarize the concern, your recommendation, and the options. Take no action until the user decides. If the verdict becomes request-changes, route through submit-review.

Step 4 — Report what's left

End every run with a short status:

• Approved / merged this run (with numbers)
• Reruns kicked off (dependabot-flake)
• Skipped — CI red (with the failing check names)
• Deferred — dependabot-blocked (with the real root cause, one line each)
• Discuss — awaiting your decision

Per-PR analysis — Carefeed conventions to check

When reviewing a non-trivial PR, check against the project rules (see admin-portal CLAUDE.md / docs/claude/project-rules.md). High-signal ones:

• Migrations have #[PreDeploy] / #[PostDeploy], real timestamps, reversible or honestly-noted-irreversible down(), literal values (no enum refs).
• No Session:: in Services / Jobs / Actions (Controllers + Middleware only).
• Controllers delegate to services — no static model calls, no instantiating controllers.
• Carbon only for dates (no time(), date(), new DateTime(), strtotime()).
• Log facade only (no error_log() / logger()).
• Vue Composition API, <script setup lang="ts">, no any without justification.
• Tests: DatabaseTransactions (not RefreshDatabase); behavioral assertions; bug-fix PRs include a red→green test.
• Security: hash_equals() for secret comparison; PHI never in logs; fail-closed permission defaults (?? false, not ?? true).

For very large PRs (roughly >1k lines or many files), delegate the deep read to a subagent (feature-dev:code-explorer, pr-review-toolkit:review-pr, or a general agent) so the orchestrator context stays lean — then synthesize its findings yourself before forming a verdict. Never delegate the verdict itself.

Hard rules

• The Approval Principle above is absolute.
• Comments and request-changes always go through submit-review.
• Never review on top of failing CI (except an approval-only check).
• Trust-but-verify: confirm what was actually posted (gh pr view <n> --json reviews) rather than assuming the action landed.