/review-contribution

name: review-contribution
description: Review a community-contributed PR for safety, correctness, and adherence to Nimbalyst rules

Review a community-contributed PR and answer one question: **is there anything that blocks merging?** Non-blocking improvements get a brief mention so the maintainer can clean them up post-merge -- they do not belong in the response to the contributor.

CRITICAL: Do NOT touch the working directory

This review runs **entirely from the GitHub API** via `gh`. The user's current working directory is off-limits.

**Forbidden** -- never run, even "just to look":

• `gh pr checkout`, `git checkout`, `git switch`, `git restore`
• `git fetch`, `git pull`, `git remote add`, `git remote update`
• `git apply`, `git am`, `git cherry-pick`, `git merge`, `git rebase`
• `git stash` (any form), `git reset`, `git clean`, `git add`
• Any command that creates or moves refs (including `gh pr checkout --detach`, `git fetch origin pull/N/head`)
• Any write to files in the working tree
• `gh pr merge`, `gh pr review --approve|--request-changes|--comment`, `gh pr comment`, `gh pr edit`

**Allowed** -- read-only only:

• `gh pr view`, `gh pr diff`, `gh pr checks`, `gh pr list`, `gh api` (GET only)
• `git log`, `git show`, `git rev-parse`, `git config --get`
• `Read` / `Grep` / `Glob` against the maintainer's already-checked-out `HEAD` (useful as "what does this file look like today" context, not as the PR content itself).

If the maintainer has uncommitted changes or is on a feature branch, **do not care** -- the review does not depend on working-directory state and must not modify it.

Argument

`/review-contribution <PR# | PR URL>`

A PR identifier is **required**. If the user runs `/review-contribution` with no argument, ask for a PR number or URL.

Session phase: `planning`, never `validating`

Call `update_session_meta` once at the start with `phase: "planning"` and leave it there for the whole review:

update_session_meta({ name: "PR #<number> contribution review", add: ["review", "github-pr"], phase: "planning" })

Reading someone else's code and deciding what to do about it is exploration, and this command writes no code of its own. `validating` means **we** made a change and are now testing it -- a session that has edited nothing has nothing to validate. Two specific traps:

• **Reaching the verdict does not advance the phase.** Producing the report is this session's deliverable, exactly like a plan or a design doc; it stays `planning` when it's done, and the board is not wrong for showing it there.
• **The tracker status is not the session phase.** Steps 3 and 7 move the `github-pr` item through `inspecting -> safe | needs-review`. That is the PR's state, not this session's. Never mirror it into `update_session_meta`.

If the review turns into actual work -- the maintainer asks you to fix the PR's problems yourself in a worktree -- that is when the phase moves to `implementing`, and `validating` only once you are testing those edits.

Steps

1. Gather PR context (via gh, no checkout)

gh pr view <pr> --json number,title,author,body,headRefName,baseRefName,headRepositoryOwner,additions,deletions,changedFiles,labels,state,mergeable,mergeStateStatus,reviewDecision,files,commits,isCrossRepository
gh pr diff <pr>
gh pr checks <pr>

For larger PRs you can pull per-file patches via `gh api repos/{owner}/{repo}/pulls/{pr}/files` -- still read-only.

Capture only what feeds the verdict: author type, cross-repo / fork status, CI status, file count, lines changed. If CI is failing, look at the failing job log to decide whether it's caused by this PR or pre-existing main breakage -- this matters for the verdict.

Merge conflicts: figure out *which* files conflict before letting it affect the verdict

`mergeable: CONFLICTING` / `mergeStateStatus: DIRTY` on its own says nothing about severity. A `CHANGELOG.md` conflict is expected on almost every contribution -- every merged PR adds a bullet to `[Unreleased]`, so any PR that also adds one conflicts. The maintainer fixes that in seconds at merge time.

Determine the likely conflict set read-only, without checking anything out:

# merge base of the PR
gh api repos/{owner}/{repo}/compare/<baseRefName>...<headRefName> --jq '.merge_base_commit.sha'
# files main changed since that merge base
gh api repos/{owner}/{repo}/compare/<mergeBaseSha>...<baseRefName> --jq '.files[].filename'

Intersect that list with the PR's changed files. The intersection is the set of files that *can* conflict.

• Intersection is only `CHANGELOG.md` (and/or other pure-append bookkeeping files -- `CHANGELOG.md`, lockfile-free version bumps in `package.json`'s `version` field) -> **treat the PR as conflict-free for verdict purposes.** Do not downgrade to NEEDS REVIEW. Note it in one line under Mergeable: "conflicts (CHANGELOG.md only -- trivial)".
• Intersection includes any source, test, config, or schema file -> before treating it as a real conflict, check whether the hunks actually collide. The intersection is only a *candidate* set; two sides can edit the same file in different places and merge cleanly. Compare `@@` ranges:

  # PR-side hunks for the file
  gh api repos/{owner}/{repo}/compare/<mergeBaseSha>...<headRefName> --jq '.files[] | select(.filename=="<file>") | .patch' | grep '^@@'
  # main-side commits touching it since the merge base, then that commit's hunks
  gh api "repos/{owner}/{repo}/commits?path=<file>&sha=<baseRefName>&since=<mergeBaseDate>" --jq '.[].sha'
  gh api repos/{owner}/{repo}/commits/<sha> --jq '.files[] | select(.filename=="<file>") | .patch' | grep '^@@'

Ranges more than 3 lines apart (git's context size) merge cleanly. If every non-`CHANGELOG.md` file in the intersection clears this check, the conflict is CHANGELOG-only -- apply the rule above. Otherwise it's a real review finding and feeds the verdict as usual.

Caveat: `compare` truncates at 300 files an