name: review description: Autonomous PR review — reads diff, cross-references knowledge base, posts inline comments, and leaves an overall verdict. user-invocable: true argument-hint: "[pr-number | pr-url]"
Review Agent — Autonomous PR Review
Reads a PR diff, cross-references the full knowledge base, posts inline comments on specific issues, and leaves an overall verdict. Zero user input required.
Invoke as:
/review <pr-number-or-url>— review a specific PR/review— review the open PR on the current branch
For creating PRs use /create-pr. For CLI version bumps use /pac-cli-update.
Core rule: never ask, always decide
| Condition | Action |
|---|---|
| Line-level issue (wrong pattern, bug risk, breaking change) | Post inline comment on that line |
| File-level concern (missing test, architecture violation) | Post inline comment on first line of file |
| Overall concern (security, missing checklist item) | Add to overall review body |
| No issues in a section | Skip the section — don't pad the review |
Step 1 — Sync knowledge if stale
grep "## Last sync" memory/ado-knowledge.md 2>/dev/null | tail -1
If last sync was > 7 days ago or file missing: run /knowledge-sync inline, then continue.
Step 2 — Resolve PR and classify type
# Resolve PR number, head SHA, base branch, changed files
gh pr view <number-or-url> --json number,title,body,headRefOid,baseRefName,author,url 2>&1
gh pr checks <number-or-url> 2>&1
gh pr view <number-or-url> --json baseRefName,files,author \
--jq '{base:.baseRefName, author:.author.login, files:[.files[].path]}' 2>&1
Capture: PR_NUMBER, HEAD_SHA, BASE_BRANCH.
Classify the PR type — determines which checks run:
| Changed files | PR type | Checks |
|---|---|---|
nuget.json + extension/overview.md only | PAC CLI bump | Step 3a |
package.json + package-lock.json only | npm dep update | Step 3b |
package.json + ALL task.json files | Node target update | Step 3b + Step 5 |
src/tasks/*/index.ts or selective task.json | Feature / new input | Step 5 (full) |
Base = release/stable (any type) | Release branch PR | Step 3c first, then type check |
Step 3 — Type-specific checks (run the section matching the PR type)
3a — PAC CLI bump checks (nuget.json changed)
# Verify both packages have the same version
node -e "
const n = require('./nuget.json');
const vers = n.packages.map(p => p.version);
console.log('versions:', [...new Set(vers)].join(', '));
console.log(vers.every(v => v === vers[0]) ? 'OK: match' : 'MISMATCH');
" 2>&1
# Verify the version exists on nuget.org
curl -s "https://api.nuget.org/v3-flatcontainer/microsoft.powerapps.cli/index.json" \
| node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8')); \
const v=require('./nuget.json').packages[0].version; \
console.log(d.versions.includes(v)?'nuget: found':'nuget: NOT FOUND')" 2>&1
Request changes immediately if:
nuget.jsonpackages have mismatched versions- Version does not exist on nuget.org
overview.mdhas no entry for the new version{{NextReleaseVersion}}placeholder was removed fromoverview.md
3b — npm dependency checks (package.json changed)
Check the diff for:
- No flat
minimatch@^3.xoverride — causes infinite npm resolution loop ajvoverride stays at^6.x— v8 breaks ESLint- New overrides are at minimum required version (not over-pinned)
inBundle: trueentries inpackage-lock.jsonchanges are intentional (overrides can't reach them)- No new direct dependency that should be in
bundleDependencies
3c — Release/stable branch checks (base == release/stable)
Release PRs must be version bumps only. Request changes immediately if:
- Contains
.tslogic changes (not just version string changes) - Adds new dependencies not present in the corresponding
mainPR - No corresponding merged
mainPR exists with the same changes
Verify the paired main PR exists:
gh search prs --repo microsoft/powerplatform-build-tools \
--state merged --base main "<version-or-keyword>" 2>&1
Step 4 — Research context
Run in parallel to inform the review:
# Find similar past PRs (2-3 keywords from PR title)
gh search prs --repo microsoft/powerplatform-build-tools \
--state merged --limit 8 "<keyword1> <keyword2>" 2>&1
# For top matches, read the body:
gh pr view <past-pr-number> --json title,body,mergedAt 2>&1
Also check memory/ado-knowledge.md for related ADO work items.
Skip ADO query for pure PAC CLI or dependency PRs — rarely adds value.
Step 5 — File-level review (feature/bug fix PRs, or any PR touching src/)
Fetch the full diff:
gh pr diff <PR_NUMBER> 2>&1
For each changed file apply:
| File pattern | What to check |
|---|---|
src/tasks/*/index.ts | tl.getInput() used correctly; errors reach tl.setResult(TaskResult.Failed, ...); no pac args assembled here |
src/tasks/*/task.json | Input names unchanged (renaming breaks customer pipelines); type correct; required set correctly |
src/host/BuildToolsHost.ts | ALL 32 tasks affected — flag blast radius |
src/params/auth/getCredentials.ts | All four auth types still work: UsernamePassword, SPN, ManagedIdentity, WorkloadIdentity |
src/params/auth/getEnvironmentUrl.ts | 4-level fallback preserved: task input → pipeline variable → connection → default |
extension/task-metadata.json | LIVE GUIDs must never change |
extension/extension-manifest.json | Publisher and extension ID must not change |
gulp/pack.mjs | tar imported via createRequire, not ESM import |
test/unit-test/** | New behaviour is tested; mocks use rewiremock |
Security (every PR):
- No secrets, tokens, or credentials committed
- No
--no-verifyor commit hook bypasses
Architecture alignment:
- Pipeline logic stays in build-tools; pac CLI arg construction stays in cli-wrapper
- New tasks follow IIFE →
isRunningOnAgent()→main()→ cli-wrapper pattern
Known failure patterns from knowledge base:
- WhoAmI locale failure (ADO #4846644): flag any change near
whoAmIor locale handling - AAD/OAuth authority (ADO #4863652):
resolveCloudInstance()map must be preserved - PVA import failure (IcM 604312672): pac CLI version bumps should reference changelog
Step 6 — Post inline comments
HEAD_SHA=$(gh pr view <PR_NUMBER> --json headRefOid --jq .headRefOid)
gh api repos/microsoft/powerplatform-build-tools/pulls/<PR_NUMBER>/comments \
--method POST \
--field body="<comment>" \
--field commit_id="$HEAD_SHA" \
--field path="<file-path>" \
--field line=<line-number> \
--field side="RIGHT" 2>&1
Format: ⚠️ <risk> / 🚨 Breaking: <impact> / 💡 Suggestion: / ❓ <question>
Max 15 inline comments — consolidate minor points into the overall review body.
Step 7 — Post overall review and verdict
gh pr review <PR_NUMBER> --comment --body "$(cat <<'EOF'
## Review
**Reviewed by:** Claude Code (autonomous review agent)
**PR type:** <PAC CLI bump / npm dep update / feature / release branch>
**Similar PRs:** <titles + numbers, or "none">
**ADO context:** <relevant items, or "none">
### Breaking changes
<task.json, GUID, or interface changes — or "None detected">
### Security
<!-- Omit if no findings -->
### Architecture
<!-- Omit if no findings -->
### Dependencies
<!-- Omit if package files unchanged -->
### Suggestions
<!-- Omit if none -->
### Verdict
**✅ Approve** / **🔄 Request changes** / **💬 Comment** — <one line reason>
EOF
)" 2>&1
# Then execute the verdict:
gh pr review <PR_NUMBER> --approve 2>&1
# or
gh pr review <PR_NUMBER> --request-changes --body "<blocker>" 2>&1
Verdict decision table
| Condition | Verdict |
|---|---|
| LIVE GUID changed | Request changes — critical breaking |
task.json input renamed | Request changes — breaks customer pipelines |
| Secret/credential in diff | Request changes — security |
| CI failing on non-functional-test step | Request changes |
release/stable PR has logic changes | Request changes — version bumps only on release branch |
release/stable PR missing paired main PR | Request changes |
nuget.json package version mismatch | Request changes — CLI update incomplete |
| PAC CLI version not on nuget.org | Request changes — version doesn't exist |
| Only non-blocking suggestions | Approve with inline suggestions |
| PAC CLI bump verified clean | Approve |
| No findings | Approve |
| Intent unclear | Comment |
Never:
- Approve a PR with a LIVE GUID change, renamed input, or committed secret
- Block a PR because functional tests fail locally (expected — require live credentials)
- Re-flag
ellipticLOW vuln (known accepted risk, no patch exists)