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:

   # 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:

     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)

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:

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

Do not pipe the loop through sort -u — it separates the headers from their paths, and the grouping is the readable part.

Selectivity is the whole trick, and it is measurable rather than a matter of taste. Counted against the 30-spike corpus on 2026-09-16: flow.google.com hits 23 — a topic, useless as a lead — while /about hits 9, including all three about-redirect spikes. So filter out any term matching more than a third of the corpus; what remains is short enough to actually read.

Read the verdict section of each hit, not the whole spike. If one contradicts something the PR asserts, that is a blocking finding before a single agent is dispatched — and it is cheaper than every dimension that would have failed to notice.

Written from PR #835. It added a migrated-host auth oracle that read the rendered DOM and claimed the signal was server-attested. Eight dimensions passed it — D1, D3, D6, D10 all read the auth path closely and none objected to the premise. The refutation was five days old and already in this repository: 2026-09-11-about-redirect-is-decided-client-side.md measured that Flow's /about hop is decided client-side with zero requests to Flow, and states outright that "the backend grants access while the frontend declines to open it." That single sentence invalidates the oracle. The diff touched flow.google.com in fourteen places; a grep would have put the spike in front of D6 and D10 as required reading. The e2e caught it instead — by going red on a live account whose state moved mid-session. This is a routing failure, not a reviewer failure, and routing is fixable.

Memory traversal: for each TOUCHED_PATH, look up relevant slugs:

• transports/ → [[migrated-host-driver-wire-lessons]], [[pr-must-verify-on-affected-surface]], [[flow-locale-leak-icon-ligatures]], [[ligature-carrier-differs-by-host]], [[playwright-click-no-downstream-event-signature]], [[rest-transports-drop-ui-fields]], [[image-video-mode-switch-symmetry]], [[ui-selector-drift-error-exit-23]]
• data/ → [[data-layer-overview]], [[data-layer-test-pollution-trap]], [[exit-code-16-data-store]], [[on-started-callback-recorder-safety]]
• auth/ → [[real-browser-auth-mandatory]], [[release-signing]]
• cli → [[release-back-merge-gap-recovery]], [[wheel-build-sanity-gate]]
• tests/ (any) → [[bdd-stubs-mirror-runtime-signatures]], [[background-e2e-pytest-pattern]], [[full-test-suite-ooms]], [[stale-test-discovery]], [[structlog-cache-logger-off-for-tests]]
• tests/features/ (BDD) → also [[bdd-stubs-mirror-runtime-signatures]]
• scripts/ → [[wheel-build-sanity-gate]], [[release-back-merge-gap-recovery]]
• .planning/, docs/superpowers/ → [[release-spec-plan-memory-consolidation]]
• docs/, *.md → [[readme-hybrid-router-pattern]], [[doc-examples-are-untested-fixtures]], [[agents-md-vs-llms-txt]], [[pypi-readme-staleness-fix]]
• pyproject.toml, .github/ → [[release-spec-plan-memory-consolidation]], [[pr-hygiene-revert-and-multi-commit]], [[draft-pr-merge-trap]], [[pypi-rejected-filename-reusable]]

---

3 · Detect adaptive dimensions

Dimension	Always?	Activates when…
D1 — Correctness & completeness	✅ baseline	always
D2 — Code quality & best practices	✅ baseline	always
D3 — Security	✅ baseline	always
D4 — Tests & coverage	✅ baseline	always
D5 — Memory hygiene & consolidation	✅ baseline (NEW v2)	always
D6 — UI / live-verification	adaptive	any path under src/gflow_cli/api/transports/ or tests/e2e/
D7 — Data-migration safety	adaptive	any path under src/gflow_cli/data/ or *.sql
D8 — CLI UX & help-text consistency	adaptive	any path matching src/gflow_cli/cli*.py or src/gflow_cli/commands/
D9 — Docs cross-reference & drift	adaptive	≥2 of: README.md, docs/**, CHANGELOG.md, AGENTS.md, CLAUDE.md, PLAN.md
D10 — Auth / reCAPTCHA / Chrome-profile	adaptive	any path under src/gflow_cli/auth/ or label security
D11 — Release-gate compliance	adaptive	pyproject.toml, src/gflow_cli/__init__.py, .github/workflows/, release/* branch
D12 — BDD step-stub signatures	adaptive	any path under tests/features/
D13 — Dev / release scripts	adaptive	any path under scripts/
D14 — Over-engineering / YAGNI	✅ baseline (NEW v3)	always
D15 — Surface parity (CLI ↔ MCP ↔ docs)	adaptive (NEW v4)	any path matching src/gflow_cli/cli*.py, src/gflow_cli/mcp/**, src/gflow_cli/worker/**, or a changed --help/remediation string

D15 specifics. gflow ships every capability twice, and no automated gate can see the two copies drift: tests/mcp/test_cli_parity.py is command-level (a new leaf needs a mapping), so an unmirrored option, an unread queued-payload key, or a docstring asserting removed behaviour is green everywhere. Walk the six mirror axes in skills/check/SKILL.md step 1b against the diff and report each as satisfied or drifted. Highest-yield check: for every param the PR touches, confirm the key mcp/tools.py writes into the queue payload is the key worker/codec.py reads — they are matched by string, so a mismatch type-checks and silently no-ops. This dimension exists because #626 unlocked a CLI combination while mcp/tools.py and docs/MCP.md went on telling agents it was rejected, through a fully green pipeline.

Baseline floor is non-negotiable. D1–D5 and D14 ALWAYS run. Docs-only PRs (100% paths under *.md, docs/**, CHANGELOG.md, README.md, LICENSE, AUTHORS) → D4 reframes from "test code coverage" to "docs-verification"; D5 still runs unchanged.

---

4 · Dispatch the council

Use the superpowers:dispatching-parallel-agents skill. Send all agents in one message — they must run concurrently.

Per-dimension specialized-skill mapping (NEW v2)

Each agent is a general-purpose agent (only subagent type that supports arbitrary parallel dispatch), but the prompt instructs it to invoke the relevant Claude Code skill inside the agent for specialized capability. Mapping:

Dim	Agent invokes skill (via Skill tool)	Rationale
D1 Correctness	review (single-agent PR review built-in)	Provides PR-review framing for free
D2 Code quality	code-review	Reuse-and-quality lens
D3 Security	security-review	Built-in security-review skill — the most important specialization
D4 Tests	superpowers:test-driven-development (informed)	TDD principles + verification mindset
D5 Memory hygiene	(none — direct memory inspection)	Inspect memory files via git show + filesystem
D6 UI/live-verify	verify (if live-verify approved)	Runs the app to confirm behavior
D7 Data-migration	(none — direct code inspection)
D8 CLI UX	(none — direct help-text inspection)
D9 Docs drift	(none — direct doc-cross-ref)
D10 Auth	security-review (subset of D3 with auth-specific lens)
D11 Release-gate	(none — direct config inspection)
D12 BDD	(none — direct stub-signature inspection)
D13 Scripts	(none — direct script inspection)
D14 Over-engineering	ponytail:ponytail-review (soft dep — invoke if installed; else apply the inline YAGNI rubric in § Per-dimension specifics)	"Should this code exist at all?" — the lens D1–D2 don't cover

On the D14 soft dependency: ponytail:ponytail-review is a user-local plugin, not shipped with this repo, so it is optional (same pattern as the agy extra reviewer in issue-resolve). The over-engineering lens is owned by this skill (the rubric below); the plugin only accelerates it. An agent without the plugin applies the rubric directly and still produces a D14 verdict — never skip D14 because the plugin is absent.

Per-agent prompt skeleton (mandatory v2 changes in bold)

You are one of <N> parallel reviewers on a council reviewing PR #<N> of `gflow-cli` at C:\development\github\gflow-cli.

Your dimension is **<DIMENSION NAME>**. Other agents handle <other dimensions> — do NOT duplicate their work.

**PR head branch:** `<head_branch>` (head SHA: `<head_sha>`).
**Base branch:** `<base_branch>`.

**🚨 CRITICAL — file reading + verification rules (v2 stale-tree-reads fix + v2.1 verify-before-claim):**

The orchestrator's working tree is on `<base_branch>` (typically `develop`), NOT the PR head. If you `Read` a file in `C:\development\github\gflow-cli\`, you get the PRE-PR copy and will produce FALSE POSITIVES like "file X doesn't exist" or "claim Y not applied" when in fact X and Y are present on the PR head.

**Mandatory rules:**

1. **For file inspection** — ALWAYS use `ctx_execute(language="shell", code="git show <REVIEWED_SHA>:<path>")` (or `git show origin/<head_branch>:<path>`). For the diff itself, `gh pr diff <N>`. For metadata, `gh pr view <N> --json ...`. NEVER use `Read` on a repo file unless you have verified `git branch --show-current` returns `<head_branch>`.

2. **Verify-before-claim — applies to BEHAVIOR claims, not just file existence (v2.1 NEW):** any "feature/setting/marker/env-var is NOT present" or "is missing" or "is wrong" claim MUST be backed by an explicit `git show <REVIEWED_SHA>:<path> | grep <expected>` (or equivalent) that you ran. Quote the exact command + its output in your report. Do NOT rely on memory or summary; the v2 council had a real false-negative where D5 claimed "PR doesn't add addopts filter" because the agent assumed-not-verified — the addopts WAS added but the agent never ran `git show <SHA>:pyproject.toml`. Treat your own claims like a code reviewer: would this assertion survive a hostile re-review? If yes, ship it; if uncertain, re-verify.

3. **SHA pinning verification (v2.1 NEW):** before reporting findings, run `git rev-parse $REVIEWED_SHA` (or `git ls-remote origin <head_branch>`) and confirm your reads were against `<REVIEWED_SHA>`. If the author has pushed new commits during your dispatch, your findings still apply to `<REVIEWED_SHA>` — the synthesizer will note any divergence at Phase 5 step 5.

**Specialized skill (if listed for your dimension):** before deep analysis, invoke the Skill tool for `<skill_name>` to load specialized review guidance. Apply that skill's checklists in addition to the dimension-specific questions below.

Assess specifically:
1. <dimension-specific question 1, with code citation hooks>
2. <…>

**Mandatory memory you MUST consult and cite if relevant:** <fixed slug list from the Dimension → Slugs table>.

Output a structured report under 500 words:
- Verdict: GREEN / YELLOW / RED
- Must-fix (numbered, file:line refs)
- Nice-to-have (numbered)
- Confirmed-<good/safe/correct> (1-line bullets)

If you have nothing to flag, say so explicitly and state GREEN with a one-line justification — do NOT manufacture findings.

If you are NOT sure a finding is real because it depends on file content, VERIFY via `git show origin/<head_branch>:<path>` before reporting it. Stale-tree false positives are a documented v1 council bug.

Dimension → mandatory memory slugs table

Slugs resolve directly: [[<slug>]] → docs/superpowers/memory/<slug>.md. Open exactly the files your dimension's row names — no searching, no judgement call about what is relevant. The directory is in the repo, so it is available to every agent that can read the tree, including the sandboxed autonomous runs that have no access to a maintainer's local store.

scripts/ci/check_council_memory.py enforces the round trip both ways: a citation with no file fails CI, and a file no dimension cites fails CI too. So a gap announces itself instead of quietly degrading routing back into a search — which is what this table previously did, when the slugs were "conceptual anchors" that resolved to nothing.

These files are a published subset of the maintainer's working memory, not a mirror of it: review-relevant facts only, with private identifiers stripped. If a slug's file is missing, say so in your report and move on — never fabricate its contents.

Dim	Mandatory memory slugs
D1	[[pr-must-verify-on-affected-surface]], [[video-model-capability-matrix]], [[flow-capabilities-are-cohort-dependent]], [[migrated-refusal-is-a-dom-card-not-a-wire-record]]
D2	[[ruff-format-scope-is-src-tests]], [[git-add-all-sweeps-scratch-files]]
D3	[[real-browser-auth-mandatory]], [[release-signing]]
D4	[[e2e-evidence-is-a-contributor-deliverable]], [[force-color-breaks-cli-tests]], [[pr-must-verify-on-affected-surface]], [[full-test-suite-ooms]], [[stale-test-discovery]], [[structlog-cache-logger-off-for-tests]], [[windows-running-launcher-blocks-uv-upgrade]]
D5	[[memory-is-working-dir-keyed]], [[release-spec-plan-memory-consolidation]], [[pr-council-review-stale-tree-reads]] (this very bug, as the council should self-improve)
D6	[[ui-selector-drift-error-exit-23]], [[credit-free-route-abort-verification]], [[flow-credits-videos-only]], [[flow-recon-must-run-on-denon82-ffroliva-migrated]], [[flow-locale-leak-icon-ligatures]], [[ligature-carrier-differs-by-host]], [[playwright-click-no-downstream-event-signature]], [[rest-transports-drop-ui-fields]], [[image-video-mode-switch-symmetry]], [[verification-ledger-5-layer]], [[migrated-host-driver-wire-lessons]], [[content-policy-text-scan-false-positives-on-page-chrome]]
D7	[[on-started-callback-recorder-safety]], [[data-layer-test-pollution-trap]], [[exit-code-16-data-store]]
D8	(none mandatory)
D9	[[prose-conflicts-hide-in-disjoint-files]], [[doc-examples-are-untested-fixtures]], [[readme-hybrid-router-pattern]], [[agents-md-vs-llms-txt]], [[pypi-readme-staleness-fix]]
D10	[[real-browser-auth-mandatory]]
D11	[[release-back-merge-gap-recovery]], [[wheel-build-sanity-gate]], [[pypi-rejected-filename-reusable]], [[draft-pr-merge-trap]], [[windows-running-launcher-blocks-uv-upgrade]]
D12	[[bdd-stubs-mirror-runtime-signatures]]
D13	[[wheel-build-sanity-gate]]
D14	(none mandatory; apply the YAGNI rubric below)
D15	[[mcp-is-first-class-across-skill-chain]]

Per-dimension specifics