pr-review

Files (1)

• SKILL.md

SKILL.md (first lines)

**CRITICAL**: When `--publish` is passed, this skill produces a PUBLISHED GitHub review visible to the PR author and all watchers. Every word matters. Every claim must be proven. User MUST approve the text before publishing.

Default behavior is **draft only**: the skill performs the full analysis, presents the review and inline comments to the user, and stops. Nothing is sent to GitHub unless `--publish` is set.

## Arguments

Parse $ARGUMENTS for:

- PR number (positional, optional). If omitted, detect from current branch: `gh pr view --json number --jq .number 2>/dev/null`
- `--publish` — actually post the review to GitHub via the API. Without this flag the skill stops after presenting the draft; the draft is the deliverable. Required separately from `--approve` so that the LGTM/NOT-LGTM decision and the publish action are independent.
- `--approve` — submit as LGTM (APPROVE event) explicitly even when no blockers found (default verdict policy is LGTM without blockers, so this flag is informational). Has no effect unless `--publish` is also set.
- `--target` — target branch override (passed through to `/branch-review`)
- `--ticket` — external ticket reference (URL or ID) for cross-referencing requirements beyond the PR description; activates the Ticket Compliance section in the output

If no PR number and no PR exists for current branch, report error and stop.

## Step 1: Sync to Remote and Verify PR Scope

Before any analysis, fetch ALL remote refs and verify the PR scope via GitHub API.

**CRITICAL**: Local branches go stale. NEVER use bare `main`/`master`/base-branch in `git diff` — ALWAYS use `origin/<branch>`. A stale local branch produces phantom diffs that include already-merged commits, leading to false findings (e.g., claiming a PR needs rebasing when it doesn't).

```bash
# Fetch everything so origin/* refs are current
git fetch origin

# Get PR metadata from the API — this is the ONLY authoritative source for PR scope
PR_BRANCH=$(gh pr view $PR_NUMBER --json headRefName --jq .headRefName)
PR_BASE=$(gh pr view $PR_NUMBER --json baseRefName --jq .baseRefName)

# Checkout and reset to remote head
git checkout $PR_BRANCH
git reset --hard origin/$PR_BRANCH

# Verify PR scope via API — do NOT trust local git diff for file count
gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER" --jq '{commits: .commits, changed_files: .changed_files}'
gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER/files" --jq '.[].filename'
gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER/commits" --jq '.[] | "\(.sha[:8]) \(.commit.message | split("\n")[0])"'
```

Print the current commit hash, commit count, file count, and file list so the user can confirm this is the right state. If the local `git diff origin/$PR_BASE...$PR_BRANCH --stat` shows different files than the API, **trust the API** and investigate the local discrepancy before proceeding.

## Step 1.5: Readiness Gate (CI + Mergeability)

Before spending any effort on substance or code analysis, do a fast pass on whether the PR is even in a reviewable state. This is a quick check, not the review. Two independent conditions send the review into **gate mode** — an early exit that produces a comment-style note with NO `LGTM` / `NOT LGTM` verdict, because a verdict on a PR that cannot merge or does not build is noise, and it wastes the expensive analysis on code the author still has to change.

```bash
# Mergeability — CONFLICTING / DIRTY means the branch conflicts with its base
gh pr view $PR_NUMBER --json mergeable,mergeStateStatus --jq '{mergeable, mergeStateStatus}'

# CI — every check run/context with its conclusion (handles both CheckRun and StatusContext shapes)
gh pr view $PR_NUMBER --json statusCheckRollup \
  --jq '.statusCheckRollup[] | {name: (.name // .context), status: .status, conclusion: (.conclusion // .state)}'
```

If `mergeStateStatus` comes back `UNKNOWN`, GitHub has not finished computing it — wait a few seconds and re-query once before deciding.

### 1.5a. Merge conflicts (DIRTY)

If `mergeable == "CONFLICTING"` or `mergeStateStatus == "DIRTY"`, the branch conflicts with its base. The diff cannot be trusted — "what changed" is unstable until the author rebases, and any finding computed against a conflicted tree may be an artifact of the conflict rather than the change. **Do not render a verdict.** Enter gate mode: the deliverable is a short note asking the author to resolve the conflicts and re-request review. Skip the dual analysis entirely.

### 1.5b. Red CI plausibly tied to the code

Consider only checks that actually FAILED — `conclusion` in `FAILURE`, `TIMED_OUT`, `ERROR`, `STARTUP_FAILURE`, or a StatusContext `state` of `FAILURE` / `ERROR`. Checks that are still running (`QUEUED`, `IN_PROGRESS`, `PENDING`) or awaiting manual authorization (`ACTION_REQUIRED` — common for fork PRs and protected-environment / deployment gates) are NOT red: the diff has not failed, so do not gate on them, though you may note that CI has not finished. `NEUTRAL`, `SKIPPED`, `CANCELLED` are not failures. Only genuine failures reach the code-related-vs-not test below — do not let the "if you cannot tell" rule pull a pending or manual-approval check into gating.

Split the failing checks into two buckets:

- **Code-related** — build, compile, unit/integration/e2e tests, lint, type-check, vet, schema/manifest validation, generated-code drift, coverage gate. A failure here means the PR in its current state is broken or unverified, so reviewing it wastes the review. Enter gate mode.
- **Not code-related** — missing secrets on a fork PR, a flaky external service, a rate-limited registry, an unrelated required-check misconfiguration, a DCO/label bot. These do NOT gate; note them and continue the normal review.

The test is: "could this red check plausibly be caused by the diff?" If yes — or if you cannot tell — treat it as code-related and gate. Do not spend a full review proving a defect the CI already caught; hand it back.

### 1.5c. Gate-mode output

When 1.5a or 1.5b fires, STOP before Step 2 — do NOT run `/branch-review`, Codex, or Claude's analysis. This also skips Ticket Compliance (the `--ticket` pass): no code was reviewed against requirements, so the note makes no compliance claim. Produce a comment-style note with no verdict word, covering:

- What blocks readiness: the conflict and/or the specific failing checks, named.
- The concrete ask: rebase/resolve conflicts, fix the named checks, then re-request review.
- Nothing about code quality — you have not reviewed it and must not imply you have.

Example note (gate form):

> This PR isn't ready for review yet. CI is red on checks that look code-related — `build`, `test (integration)` — and the branch has merge conflicts with `main`. Please rebase to resolve the conflicts, get those checks green, then re-request review. I haven't looked at the code itself yet.

Then hand off to the normal presentation and publish machinery, in gate form: present the note per Step 8, and if (and only if) `--publish` was passed AND the user approved, post it per Step 9 as a `COMMENT`-event review — never `APPROVE` / `REQUEST_CHANGES`. In default (draft) mode the note is simply reported to the user and the skill stops.

If there are no conflicts and no code-related red CI, proceed to Step 1.6.

## Step 1.6: Check Bot Comments

Read all comments and inline review comments from bots (CodeRabbit, Gemini Code Assist, etc.):

```bash
gh pr view $PR_NUMBER --json comments --jq '.comments[] | select(.author.is_bot == true or (.author.login | test("bot|coderabbit|gemini"; "i"))) | {author: .author.login, body: .body}'
gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER/comments" --jq '.[] | select(.user.login | test("bot|coderabbit|gemini"; "i")) | {author: .user.login, path: .path, line: (.original_line // .line), body: .body}'
```

Note which bot findings are valid and which are noise. Check if the author has responded to bot comments. If valid bot findings remain unaddressed, either incorporate them into the review (with new evidence — never repeat verbatim) or surface a general note asking the author to address them.

## Step 1.7: Provenance and Message Hygiene

Cheap, PR-level, and independent of the code: read every commit message and the PR body before the substance pass, because the defects here are decided by the text alone.

```bash
gh api "repos/{owner}/{repo}/pulls/$PR_NUMBER/commits" --jq '.[] | "--- \(.sha[:8]) <\(.commit.author.email)>\n\(.commit.message)"'
gh pr view $PR_NUMBER --json body --jq .body
```

Four checks. Each hit is a verdict finding (Step 6, Provenance blockers) that quotes the offending line as its Evidence:

- **Trailers that advertise a model.** `Assisted-by:` (any casing) accepts exactly one value, `LLM`. A model or vendor anywhere in a trailer or byline — `Assisted-by: Claude <noreply@anthropic.com>`, `Co-authored-by: <model> <noreply@...>`, `Generated-by:`, a "Generated with <tool>" line — blocks; the fix is `Assisted-by: LLM`.
- **Session links.** A `Claude-Session:` trailer, or a URL to an assistant session or shared transcript (`claude.ai/code/...`, `chatgpt.com/share/...` and the like) in a commit message, the PR body, or the author's comments. A transcript is not a rationale and the link rots; the fix is to drop it and put the WHY into the message.
- **Commit messages.** The subject is imperative and specific, in `type(scope): description` form where the repository uses Conventional Commits (`fix`, `wip`, `update` are not subjects). The body explains WHY and only WHY: a body that walks through the diff, or carries review-iteration or planning vocabulary ("address review comments", "pass 2", "batch 1", "as discussed", a bare ticket ID as the whole explanation), blocks. So does a missing `Signed-off-by` in a repository whose history carries them.
- **Undisclosed machine authorship.** Obviously machine-written code on commits without an `Assisted-by: LLM` trailer. "Obviously" is an evidence bar, not an impression: the diff shows at least two of — comments narrating the next line, section banners, tutorial-flow comments ("First, ... Next, ... Finally"), docstrings restating the signature, `IMPORTANT:` / `NOTE:` on the unremarkable, comment density far above the surrounding file, em-dash clusters or the "not X, but Y" frame in prose. Quote each tell. One tell alone is a non-blocking note. The fix has two halves: the disclosure trailer, and the tells themselves, which the cascaded `/branch-review` flags as WHAT-comments in their own right.

## Step 2: Business Context

Before any code reading, frame WHY this PR exists. Read in this order:

From lexfrei/ccc, BSD-3-Clause.

More from lexfrei/ccc