pr-council-review

Files (1)

• SKILL.md

SKILL.md (first lines)

# `pr-council-review` — PR Council Review skill

Council-driven PR review. Dispatches **6 baseline + N adaptive** parallel reviewers, each scoped to one dimension, each invoking the relevant Claude Code specialized skill (e.g. `security-review`, `code-review`, `verify`), then synthesizes a single consensus verdict.

This skill is the canonical body. The Claude Code slash command at `.claude/commands/gflow/pr-council-review.md` is a thin wrapper that invokes this skill. Non-Claude tools (Antigravity / Codex / Cursor / Aider) can consume this SKILL.md directly via their own skill loaders.

**Three modes:**
1. **No argument** → list open PRs ranked by review priority; user picks. (See § 1.)
2. **`PR#` argument** → run the full council on that PR. (See § 2 onward.)
3. **Branch mode** → run the full council on the current local feature branch (no PR yet). Invoked via the `/gflow:branch-review` wrapper. See § 8 for the PR→branch translation table and pre-flight.

Treat **YELLOW as soft block** — it is advisory in name only; clear it or dismiss it
with a logged justification (§ 5 step 8).

---

## 0 · Pre-flight

**All six checks are mandatory. Any failure (except step 6, which records a finding) halts before Phase 1/2.**

1. **`gh` authenticated** — run `gh auth status`. Non-zero exit → stop with: *"`gh` is not authenticated. Run `gh auth login` and re-invoke."*
2. **Inside the repo** — assert `AGENTS.md` AND `CLAUDE.md` exist in the working directory.
3. **Resolve the argument:**
   - **Empty** → jump to **Phase 1 (Prioritize)**.
   - **PR number** → validate with `gh pr view <N> --json number`. If error → stop with the error verbatim.
4. **Draft check** (PR# mode only) — if `gh pr view <N> --json isDraft` returns `true`, surface a banner citing memory `[[draft-pr-merge-trap]]`: *"PR #N is DRAFT. Reviewing is fine, but do NOT merge a draft (the merge API can close it + delete the head ref). Run `gh pr ready N` first if you intend to merge. Continue review? (yes/no)"*. Ask the user before dispatching.
5. **Capture PR head ref + SHA (pin the review)** — `head_branch=$(gh pr view <N> --json headRefName --jq '.headRefName')` and `head_sha=$(gh pr view <N> --json headRefOid --jq '.headRefOid')`. **Pin both to a `REVIEWED_SHA` variable** and pass to every dispatched agent so the council's verdict is anchored to one commit. The local working tree is NOT on the PR head; all file reads must go through `git show $REVIEWED_SHA:<path>` (or `git show origin/$head_branch:<path>` if you fetched first). If the author pushes new commits during the review, the council still reports against `REVIEWED_SHA`; the synthesizer notes any divergence in Phase 5 step 5.
6. **Mechanical CI gate (D0 — non-LLM, runs BEFORE dispatch).** The LLM dimensions reason about the diff; none of them run the repo's actual lint/format/link gates, so a whole-tree failure sails past the council (this happened on PR #269 — a latent `ruff format` failure in a file the diff only *touched* went green through 8 agents, then reddened CI and dragged SonarCloud `new_coverage` to 0). Run the **exact CI gate commands** (`.github/workflows/ci.yml` → Lint / Format check / Documentation links / Repo hygiene) against the reviewed tree:
   ```bash
   # Prefer running at REVIEWED_SHA. If HEAD is already there (reviewing your own
   # just-pushed PR, or branch-review mode), run in place:
   if [ "$(git rev-parse HEAD)" = "$REVIEWED_SHA" ]; then dir=.; else \
     dir=$(mktemp -d); git worktree add --detach "$dir" "$REVIEWED_SHA"; fi
   ( cd "$dir" && uv run ruff check src tests \
       && uv run ruff format --check src tests \
       && uv run python scripts/ci/check_doc_links.py \
       && uv run python scripts/ci/check_repo_hygiene.py )
   # if a worktree was created: git worktree remove --force "$dir" (Windows: prune later if locked)
   ```
   - **Any non-zero → record a `D0 — CI-mechanical` RED.** This is a hard blocker regardless of the LLM dimensions' verdicts; surface the failing command + output verbatim in the report and do NOT call the PR merge-ready. (Mirrors the SonarCloud-gate rule in the wrapper: the council must not bless a tree CI will reject.)
   - If running the gate is impractical (no `uv`, worktree add fails), fall back to `gh pr checks <N>` and inspect the `test` job's Lint/Format steps; if they are **pending or failing**, flag D0 as `UNVERIFIED — must be confirmed green before merge`, never as GREEN.
   - **On a fork PR, `gh pr checks` is not sufficient and the fallback above is blind.**
     GitHub holds `pull_request` workflows from forks at `conclusion=action_required`
     until a maintainer clicks *Approve and run*, and such a run **does not appear in
     `statusCheckRollup` at all** — so the PR reports every check green while nothing
     ran. Measured on 2026-09-15: #781 (approved) showed 16 checks; #793 (held) and
     #787 (no run) each showed 2, all green. Run:
     ```bash
     uv run python scripts/ci/check_fork_pr_ci.py --pr <N>
     ```
     Non-zero → D0 is `RED — CI never ran`. Approve the workflow run, or gate the head
     locally, before trusting any green on that PR.
   - Unlike steps 1–5, a D0 failure does **not** halt — dispatch the LLM council anyway so its findings are gathered in one pass, then fold D0 into the Phase 5 verdict.

---

## 1 · Prioritize (no-argument mode)

```bash
gh pr list --state open --json number,title,author,isDraft,headRefName,updatedAt,additions,deletions,labels,reviewDecision,statusCheckRollup
```

**Empty-list short-circuit:** if the result is `[]`, print *"No open PRs to review."* and exit.

Rank with these heuristics (highest priority first):

| Signal | Weight | Why |
|---|---|---|
| `isDraft == false` AND CI all-green | +3 | Ready to merge once approved — highest ROI |
| Touched path includes `src/gflow_cli/api/transports/` | +2 | UI-automation is the highest-risk surface (memory `[[pr-must-verify-on-affected-surface]]`) |
| Touched path includes `src/gflow_cli/auth/` or `recaptcha` | +2 | Auth changes need security-deep-dive |
| Touched path includes `src/gflow_cli/api/client.py` or `src/gflow_cli/api/_sapisidhash.py` | +2 | Auth-token plumbing (Bearer / access-token / SAPISID) — lives outside `auth/` but is security-material; backtest found 3 historical fixes here |
| Touched path includes `src/gflow_cli/data/` | +2 | Migration safety + #86 hygiene history |
| Older than 7 days (stale risk) | +1 | Conflict risk grows with age |
| `additions + deletions <= 300` | +1 | Small PRs ship faster |
| Label contains `release-blocker`, `security`, `hotfix` | +5 | Anything labelled urgent jumps the queue |
| `isDraft == true` AND CI red | −2 | Author still iterating; review wastes their time |

Present a numbered table, then **stop and ask** the user to pick a PR number. Do NOT auto-start on Rank 1 — *recommend*, do not *pre-select*.

---

## 2 · Gather context (PR# mode)

Pull in parallel via `ctx_batch_execute`:
- `PR_META` → `gh pr view <N> --json title,body,author,baseRefName,headRefName,headRefOid,state,isDraft,additions,deletions,changedFiles,labels,files,statusCheckRollup`
- `PR_DIFF` → `gh pr diff <N>`
- `PR_CHECKS` → `gh pr checks <N>`
- `TOUCHED_PATHS` → `gh pr view <N> --json files --jq '.files[].path' | sort -u`
- `RECENT_COMMITS` → `gh pr view <N> --json commits --jq '.commits[-5:] | .[] | "\(.oid[:7]) \(.messageHeadline)"'`
- `PR_COMMENTS` → `gh pr view <N> --json comments --jq '.comments[] | "\(.createdAt) \(.author.login): \(.body)"'`

> **Read the thread before you flag "no evidence".** `PR_COMMENTS` is not optional colour:
> a prior council verdict, a maintainer's counter-capture, and the contributor's reply all
> live there and none of them appear in the diff. On PR #650 the autonomous run posted
> *"reverses a confirmed-live finding with no live evidence attached"* as its headline
> must-fix — 50 minutes after the contributor had posted a machine-generated capability
> matrix with a SHA-256 and a screenshot in that same thread. Truncate long bodies if you
> must, but never review a contested PR without reading what has already been said on it.

**Reference files** (read via `git show origin/$head_branch:<path>` — NOT local `Read`, because the working tree is on `develop`):
- `CLAUDE.md`, `AGENTS.md`, `docs/INDEX.md`

**Spike traversal (NEW v2.2) — do this BEFORE dispatch, and hand the result to the
reviewers.** The council reasons about the *diff*. A measurement that refutes the diff's
premise does not appear in the diff, so no dimension can find it by reading well. Sweep
`docs/superpowers/spikes/` for the surface the PR touches and pass every match into the
prompts of the dimensions that own that surface, as **required reading**:

```bash
total=$(ls docs/superpowers/spikes/*.md | wc -l)
gh pr diff <N> | grep -ohiE '/about|batchexecute|recaptcha|networkidle|SNlM0e|aisandbox[a-z-]*|agent-mode|referenceEntit[a-z]*|SignOutOptions|flow-[a-z-]+|ya29|SAPISID' \
| tr 'A-Z' 'a-z' | sort -u | while read -r t; do
    hits=$(grep -rli -- "$t" docs/superpowers/spikes/ 2>/dev/null)
    n=$(printf '%s\n' "$hits" | grep -c . )
    # A term matching most of the corpus is a topic, not a lead. Skip it.
    [ "$n" -gt 0 ] && [ "$n" -le $(( total / 3 )) ] && printf '## %s (%s)\n%s\n' "$t" "$n" "$hits"
  done
```

From ffroliva/gflow-cli, MIT.

More from ffroliva/gflow-cli