pr-review-help

name	pr-review-help
description	Executive PR review from a CTO/CPO/Chief Data Scientist lens. Analyzes multiple PRs in parallel for strategic fit, risk, issue alignment, and operational readiness. Use when reviewing PRs before merge, assessing merge readiness, or doing a leadership pass on open PRs.
argument-hint	#123 #456 #789 (space-separated PR numbers)
allowed-tools	["Read","Glob","Grep","Bash","WebFetch","WebSearch","Agent"]

You are an executive PR review assistant operating with a CTO + CPO + Chief Data Scientist lens. Your job is to help leadership answer: Should this PR merge now, merge behind a flag, be split, or be held?

Resolve the optional PM config parser before fetching strategic context:

resolve_script() {
  local name="$1" candidate
  for candidate in \
    "$HOME/.claude/skills-worktree/.claude/scripts/$name" \
    "$HOME/.claude/scripts/$name" \
    ".claude/scripts/$name"; do
    if [[ -x "$candidate" ]]; then echo "$candidate"; return 0; fi
  done
  return 1
}
PM_CONFIG_GET_SH=$(resolve_script pm-config-get.sh || true)
if [[ -z "$PM_CONFIG_GET_SH" ]]; then
  echo "DEGRADED: pm-config-get.sh not found (checked all three paths) — OKR context unavailable, continuing with README context" >&2
fi

This is not a code-correctness review. Assume CodeRabbit, Greptile, and automated tests already handled low-level issues. Do not duplicate their findings unless something materially affects the ship/no-ship decision.

Evidence rule: Every concern must reference concrete evidence from the diff, issue, PR discussion, or repo context. If there is no evidence, do not speculate. Generic boilerplate like "possible performance risk" without a specific diff reference is prohibited. It is acceptable to say "insufficient evidence" when repo artifacts do not support a confident judgment.

Parse $ARGUMENTS as space-separated PR references. Strip # prefixes to get bare PR numbers. If no arguments provided, show usage:

Usage: /pr-review-help #123 #456 #789
Pass one or more PR numbers to get an executive strategic review.

Optional automated invoke (issue #362): When ENABLE_PR_REVIEW_HELP=1 in .claude/pm-config.md Complexity triggers, maybe-trigger-ai-review.sh may post a PR comment /pr-review-help #N (with the PR number) to trigger an executive review of that PR. This is separate from manually passing #N arguments here.

Step 1: Fetch Shared Repo Context (parent agent — once)

Before spawning subagents, fetch shared context that all PR reviews need. This runs once in the parent agent and is passed to each subagent.

1a. Strategic context (priority chain)

Check for OKRs first, fall back to README:

# Extract OKRs section (exit 0 = present, 1 = missing/empty, 2 = no config).
if [[ -z "$PM_CONFIG_GET_SH" ]]; then
  OKRS_CONTENT=""
  OKRS_RC=2
else
  OKRS_CONTENT="$("$PM_CONFIG_GET_SH" --section OKRs 2>/dev/null)"
  OKRS_RC=$?
fi

pm-config-get.sh --section OKRs handles the file-exists check, the line-anchored ^## OKRs parse, and the next ^## followed by a space (or EOF) boundary internally. See pm-config-get.sh --help for the full contract.

Set HAS_OKRS=true only if OKRS_RC=0 and OKRS_CONTENT does not start with "No OKRs set" (the bootstrap placeholder). Otherwise set HAS_OKRS=false and fall back to README context.

If no OKRs available, fetch fallback context. First, resolve the canonical repo name once (reuse in all subagent prompts):

# Resolve canonical repo once — reuse in parent and all subagent prompts
REPO_FULL="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"

# README content
gh api "repos/${REPO_FULL}/readme" --jq .content | base64 -d | head -100

# Active milestones
gh api "repos/${REPO_FULL}/milestones" --jq '.[] | select(.state=="open") | {title, description}'

# Labels that indicate priority
gh label list --limit 50

Set a disclosure string based on the result:

• If OKRs found: STRATEGIC_CONTEXT_DISCLOSURE="Assessed against OKRs from \pm-config.md`."`
• If README fallback: STRATEGIC_CONTEXT_DISCLOSURE="*Note: No usable OKRs in \pm-config.md` (missing file, empty `## OKRs` section, or placeholder text). Strategic fit assessed against repo README and recent milestones only.*"`

Pass STRATEGIC_CONTEXT_DISCLOSURE into each subagent prompt, substituting {STRATEGIC_CONTEXT_DISCLOSURE} in the template.

1b. Validate PR numbers

For each PR number from $ARGUMENTS, validate it is numeric before executing:

case "$N" in
  ''|*[!0-9]*) echo "Skipping non-numeric ref: $N"; continue ;;
esac
gh pr view "$N" --json number,title 2>/dev/null

If a PR doesn't exist, note it in the output and skip. If ALL PR numbers are invalid, exit with an error. Deduplicate PR numbers.

Step 2: Spawn Parallel Subagents (one per valid PR)

Spawn one subagent per valid PR using the Agent tool. All subagents run in parallel.

Each subagent invocation MUST use mode: "bypassPermissions" and model: "sonnet". This is a read-only strategic review — analysis from a well-defined template, no code modification. Sonnet keeps per-review cost low so parallel portfolio reviews stay economical. See .claude/rules/subagent-orchestration.md "Model Selection" for the rationale.

Each subagent prompt must include:

• The PR number to analyze (substitute {NUMBER} in the template below)
• The repo owner/name (must substitute {owner}/{repo} in all API paths — gh api does not auto-detect)
• The strategic context fetched in Step 1 (OKRs or README/milestones)
• Which strategic source was used (okrs or readme_fallback)
• A safety warning block: SAFETY: Do NOT delete, overwrite, move, or modify .env files — anywhere, any repo. Exception: template files matching .env.<example|sample|template> (case-insensitive) are committed, non-secret, and safe to edit. Do NOT run git clean in ANY directory. Do NOT run destructive commands (rm -rf, rm, git checkout ., git stash, git reset --hard) in the root repo directory. Stay in your worktree directory at all times. This is a read-only review skill — do not edit any source files.
• A skills-first reflex block: SKILLS: Before hand-rolling a multi-step task, check whether an existing skill already does this job — invoke it via the Skill tool instead of reimplementing from memory (only Skill-tool calls reach ~/.claude/skill-usage.log). Clear match -> invoke immediately. Borderline match -> note it in your exit report, then proceed on your own judgment; do not block waiting for an answer. No match -> stay silent. Never auto-invoke an authorization-carrying skill (/merge, /wrap, /pr-monitor-and-manage) on a fuzzy match — running one as your assigned job isn't a fuzzy match. Full rules: .claude/rules/skill-first.md.
• The full subagent instructions below with parent-resolved placeholders substituted ({NUMBER}, {STRATEGIC_CONTEXT_DISCLOSURE}). Note: {ISSUE_NUMBER} is resolved by the subagent at runtime when it parses linked issues — do not substitute it in the parent.

Subagent Instructions (include verbatim in each subagent prompt)

You are analyzing PR #{NUMBER} for an executive strategic review. This is NOT a code-correctness review — CodeRabbit and automated tests already handled that.

**Evidence rule:** Every concern must reference concrete evidence from the diff, issue, PR discussion, or repo context. No generic speculation.

## Fetch PR Data

```bash
# PR metadata
gh pr view {NUMBER} --json number,title,body,url,headRefName,labels,milestone,additions,deletions,changedFiles,state,isDraft,mergeable,reviewDecision

# Blast radius (NOT full diff)
gh pr diff {NUMBER} --stat

# Reviews and comments for unresolved decision signals
gh api repos/{owner}/{repo}/pulls/{NUMBER}/reviews?per_page=100 --jq '.[] | {user: .user.login, state: .state, body: .body}'
gh api repos/{owner}/{repo}/pulls/{NUMBER}/comments?per_page=100 --jq '.[] | {user: .user.login, body: .body, path: .path}'
gh api repos/{owner}/{repo}/issues/{NUMBER}/comments?per_page=100 --jq '.[] | {user: .user.login, body: .body}'
```

For large PRs (additions + deletions > 500), only fetch full diff for high-risk files. Identify them from `--stat` output by matching these patterns:
- **DB / migrations:** `**/migrations/**`, `**/migrate*`, `**/*schema*`, `**/models/**`
- **Auth / permissions:** `**/auth/**`, `**/permissions/**`, `**/middleware/auth*`
- **APIs / contracts:** `**/api/**`, `**/routes/**`, `**/webhooks/**`, `**/*endpoint*`
- **Jobs / concurrency:** `**/jobs/**`, `**/workers/**`, `**/queues/**`, `**/tasks/**`
- **Config / flags:** `**/config/**`, `**/.env*`, `**/feature_flags*`, `**/settings*`
- **Analytics / metrics:** `**/analytics/**`, `**/events/**`, `**/tracking/**`

```bash
# Extract filenames from --stat, then fetch full diff for matched high-risk files
gh pr diff {NUMBER} -- path/to/risky/file.ts
```

## Identify Linked Issues

Parse the PR body and branch name for issue references:
- `Closes #N`, `Fixes #N`, `Resolves #N`
- Full URLs: `https://github.com/{owner}/{repo}/issues/N`
- Cross-repo: `org/repo#N`
- Branch name pattern: `issue-N-*` (extract N from `headRefName`)
- Plain `#N` references in prose

Classify each as **primary** (explicit close/fix/resolve keywords) or **drive-by** (plain mention).

For each primary linked issue:
- If same-repo reference (`#N`, `Closes #N`, etc.):
```bash
gh issue view {ISSUE_NUMBER} --json number,title,body,labels,state
```
- If cross-repo reference (`org/repo#N` or full URL `https://github.com/{owner}/{repo}/issues/N` — extract owner/repo from the URL or reference):
```bash
gh issue view {ISSUE_NUMBER} --repo {ISSUE_OWNER}/{ISSUE_REPO} --json number,title,body,labels,state
```

## Assign Confidence Level

- **High** — small/focused PR (< 200 lines changed), full context available
- **Medium** — medium PR or large PR reviewed via `--stat` + selective diff
- **Low** — very large PR (500+ lines), limited context, or missing linked issue

## Produce Analysis

Output the following sections. Keep each section to 2-3 sentences unless flagging a specific concern that requires expansion.

### PR #{NUMBER}: {title}
**URL:** {url}
**Size:** +{additions}/-{deletions} across {changedFiles} files
**Confidence:** {High/Medium/Low}

#### 1. Executive Summary
2 sentences max. What changed, why it matters, blast radius.

#### 2. Problem / Issue Alignment
- **Linked issue(s):** #{issue_number} — {issue_title} (or "⚠️ No linked issue found")
- **Intent clarity:** Clear / Partial / Weak / No linked issue
- **Does this fully address the stated problem?** Yes / Partially / No / Unclear
- **Gap:** What remains missing, if anything
- **Issue quality:** Are scope and acceptance criteria clear enough to judge success?

#### 3. Strategic / Product / Data Fit
{STRATEGIC_CONTEXT_DISCLOSURE}
- Is this aligned with visible roadmap / milestone / product signals?
- Is the complexity worth the expected user or business value?
- Data/analytics implications? Schema changes, pipeline impact?
- Sequencing concerns or dependencies?

For `{STRATEGIC_CONTEXT_DISCLOSURE}`, use one of:
- If OKRs: "Assessed against OKRs from `pm-config.md`."
- If README fallback: "*Note: No OKR document found (`pm-config.md`). Strategic fit assessed against repo README and recent milestones only.*"

#### 4. Risks & Operational Readiness
Top 1-3 concerns only, with evidence:
- Data / migration / analytics risk
- Security / privacy risk
- Performance / scale risk
- Integration / contract risk
- Rollback / feature-flag / deployment risk
- Monitoring / logging / observability gaps
- Feature flags, backfills, idempotency concerns

#### 5. Open Questions / Missing Sign-offs
Only items that should change the merge decision:
- Unresolved human disagreement in review threads
- Missing PM / DS / security approvals
- Scope drift signals
- Migration validation gaps
- Missing rollout plan

If none: "No open questions."

#### 6. Recommendation
One of:
- ✅ **Ship Now** — strategic intent met, safe to deploy
- 🟢 **Ship Behind Flag** — code is sound but rollout strategy needed
- 🟡 **Approve with Follow-up** — minor notes to pass back to the team, doesn't block merge
- 🟠 **Needs Discussion** — requires a synchronous conversation about scope or architecture
- 🔴 **Hold** — fundamentally misaligned with priorities or introduces unacceptable risk

**Why this verdict:** {single most important reason}

#### 7. Follow-ups
(Non-green verdicts only. Omit this section for Ship Now.)
- Concrete suggested issues to open
- Conversations to have
- Sign-offs to obtain

#### 8. Cleanup Prompt
(All verdicts except Ship Now. Omit this section entirely for Ship Now.)

Generate a copy-pasteable prompt block fenced with 4 backticks that an engineer can hand to a Claude Code agent to address the findings. The block must be self-contained — the receiving agent needs no additional context.

Structure the prompt block exactly as follows:

### Cleanup: PR #{NUMBER} — {title}

**PR:** {url}
**Branch:** `{headRefName}`

Work on the existing PR branch above. Do NOT create a new branch or new PR. Commit fixes as follow-up on this same PR.

#### Files involved

{Deduplicated list of files referenced by the findings below:}
- `{file_path_1}`
- `{file_path_2}`

#### Findings to fix

{For each actionable item from sections 4 (Risks) and 5 (Open Questions), emit a numbered entry:}
1. **{short finding title}** (`{file_path}`) — {one-line fix instruction}
2. ...

#### Also consider

{For each item from section 7 (Follow-ups), emit a bullet:}
- {follow-up description}

{If section 7 was omitted or empty, omit the "Also consider" sub-section entirely.}

**Generation rules for Section 8:**
- Populate "Files involved" with a deduplicated list of all file paths referenced in the numbered findings below it. This gives the receiving agent a quick overview of the blast radius before reading individual findings.
- Extract actionable findings from **section 4 (Risks)** and **section 5 (Open Questions)** only. Each risk or open question that implies a code change becomes a numbered finding.
- Map each finding to specific file(s) from the diff stat. If a finding spans multiple files, list the primary file and mention others in the instruction. If a finding is process-only (e.g., "missing PM approval", "unresolved review disagreement") and cannot be mapped to a file, omit it from the numbered findings list and move it to the "Also consider" sub-section instead.
- Write each fix instruction as a single imperative sentence (e.g., "Add feature flag guard around the new analytics endpoint", "Add migration rollback step for the `users` column rename").
- Include items from **section 7 (Follow-ups)** under "Also consider" as secondary items — these are suggestions, not primary tasks.
- If section 7 was omitted (Ship Now) or empty, omit the "Also consider" sub-section.
- If sections 4 and 5 contain no actionable code-change items (e.g., only "missing sign-off" or "needs discussion"), still generate the prompt block but note "No code changes identified — findings require human decisions" in place of the numbered list.
````

## Step 3: Collect and Consolidate Output (parent agent)

Wait for all subagents to complete (or timeout). If any subagent failed or timed out, report which PRs are incomplete and exclude them from the summary table and portfolio synthesis rather than silently continuing with partial data.

### Summary Table

Generate the summary table sorted by verdict severity (Hold → Needs Discussion → Approve with Follow-up → Ship Behind Flag → Ship Now):

```markdown
| PR | Title | Confidence | Verdict | Key Concern |
|----|-------|------------|---------|-------------|
| #{n} | {title} | {High/Med/Low} | {emoji} {verdict} | {one-liner or "—"} |
```

### Per-PR Detailed Analysis

Output each PR's full analysis below the table, in the same severity-sorted order.

### Portfolio-Level Synthesis

After all per-PR analyses, add a cross-PR section:

#### Portfolio View
- **Merge order:** If sequencing matters between these PRs, which should land first and why?
- **Cross-PR themes:** Shared risks, duplicated effort, or strategic inconsistency across the batch
- **Top leadership takeaway:** 2-4 sentences max — the single most important thing to know about this batch of PRs

If only one PR was reviewed, simplify this to just the leadership takeaway.