agents-pr-review

PR Review Skill (agents library)

Required

• gh CLI
• git (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,labels
• gh 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 diff can be stale after a recent push
• cross-check with local git diff <base> and Read tool on changed files
• local git diff + file contents win any disagreement

Review checklist

Commit message

• Follows type: message format (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 .ts file must include the Apache 2.0 license header
• Check any added files; missing header will fail the lint check

TypeScript quality

• No any types — this repo uses tsconfig-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 .md file under messages/

Tests

• New code should have corresponding tests in test/ (mirrors src/ structure)
• Unit tests use .test.ts suffix; NUTs use .nut.ts
• Coverage target is 95%+ on new code
• Tests should use TestContext and MockTestOrgData from @salesforce/core/testSetup
• HTTP calls must be mocked using fixtures in test/mocks/ — no live org calls in unit tests
• Assertions use Chai (expect, not assert)

Dependencies

• New packages must use approved Salesforce packages (@salesforce/core, @salesforce/kit, etc.)
• Flag any new npm dependencies — they require deliberate justification in a library

Agent-specific patterns

• AgentBase, ScriptAgent, ProductionAgent follow a class hierarchy — new agent types should extend AgentBase
• Static methods on the Agent class (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