PR Review Skill (agents library)
Required
ghCLIgit(PR branch must be checked out locally for authoritative diff)
Working directory
- repo root (parent of this
.claude)
Commands
Run all three together:
gh pr view <PR_NUMBER> --json title,number,body,files,commits,additions,deletions,changedFiles,baseRefName,headRefName,author,labelsgh pr diff <PR_NUMBER>— GitHub API diff (may be stale)git diff $(gh pr view <PR_NUMBER> --json baseRefName --jq '.baseRefName') -- $(gh pr view <PR_NUMBER> --json files --jq '[.files[].path] | join(" ")')— authoritative local diff
Diff precedence
gh pr diffcan be stale after a recent push- cross-check with local
git diff <base>andReadtool on changed files - local git diff + file contents win any disagreement
Review checklist
Commit message
- Follows
type: messageformat (enforced by commitizen) - Valid types:
feat, fix, improvement, docs, style, refactor, perf, test, build, ci, chore, revert - PR will be squash-merged — the PR title becomes the commit message; flag if it doesn't follow the convention
- Semver: new behavior must use
feat:prefix; breaking changes require a major version bump; flag if the commit type doesn't match the impact
Work item reference
- PR title and body must include a GUS work item number
- In the body, the work item must be prepended with
@and surrounded with@on both sides with no spaces (e.g.@W-12345678@) - Flag if the work item is missing or formatted incorrectly
License header
- Every new
.tsfile must include the Apache 2.0 license header - Check any added files; missing header will fail the lint check
TypeScript quality
- No
anytypes — this repo usestsconfig-strict-esm - All public APIs should be typed explicitly
- New exports must be added to
src/index.ts
Code correctness
- Correctness, regressions, edge cases
- Error handling: errors must not be swallowed; rethrown errors must set the original as
cause - Error messages must include a suggested fix or a clear reason for the failure — not just what went wrong
- Input validation at API boundaries
Security
- No credentials, tokens, secrets, or PII hardcoded or leaked in source, tests, or fixtures
Messages / i18n
- User-facing strings belong in
messages/— not hardcoded in source - New messages must have entries in the appropriate
.mdfile undermessages/
Tests
- New code should have corresponding tests in
test/(mirrorssrc/structure) - Unit tests use
.test.tssuffix; NUTs use.nut.ts - Coverage target is 95%+ on new code
- Tests should use
TestContextandMockTestOrgDatafrom@salesforce/core/testSetup - HTTP calls must be mocked using fixtures in
test/mocks/— no live org calls in unit tests - Assertions use Chai (
expect, notassert)
Dependencies
- New packages must use approved Salesforce packages (
@salesforce/core,@salesforce/kit, etc.) - Flag any new
npmdependencies — they require deliberate justification in a library
Agent-specific patterns
AgentBase,ScriptAgent,ProductionAgentfollow a class hierarchy — new agent types should extendAgentBase- Static methods on the
Agentclass (create,list,init) are the public entry points — keep their signatures stable - Template files live in
src/templates/— new templates belong there
Output format
- Findings first, severity order (Critical → High → Medium → Low)
- Cite file paths and line numbers for each finding
- Questions/Assumptions section if needed
- Brief summary last