Playbook: AI PR Auto-Review System

See also: git-identity-and-roles.md — the canonical identity × role × action matrix (who reviews / approves / merges). This file remains the source of truth for the review-pipeline mechanics and require_last_push_approval semantics.

Overview

A review-pr job reaches the queue by three complementary paths:

Event-driven — .github/workflows/dispatch-to-queue.yml fires on the pull_request actions opened, ready_for_review, labeled, and synchronize (a push to an open PR, added #6387). This is the immediate path and the only one p24-infra relies on in practice (see the GH_TOKEN scanner blocker below).
Periodic scanner — every dispatcher cycle (~2 min) queue-dispatcher-loop.py’s scan_prs_for_review() sweeps open PRs in the repos listed in its REVIEW_REPOS (scripts/dispatcher-review-config.json: p24-infra, et-op, Art-Agency, radekkonarski-personal-brand, gmail-tools).
Meta-dispatcher orphan backstop (#7707, guardrails #7715) — the p24-meta-dispatcher CF Worker’s 4-hourly runOrphanBackstop scans open non-draft PRs for repos in DEFAULT_BACKSTOP_REPOS that ship no first-party dispatch-to-queue.yml. Single owner per repo: the backstop PR scan excludes any repo already in the scanner’s REVIEW_REPOS (DISPATCHER_OWNED_PR_REVIEW_REPOS in src/index.ts; env override BACKSTOP_PR_SCAN_EXCLUDE_REPOS) — so in practice the backstop only reviews PRs for whatsup-android-chat-puller (the one backstop repo the scanner does not cover), and path 2 owns et-op etc. The backstop path is bounded by head-sha dedup (metadata.head_sha), a per-cycle cap (BACKSTOP_REVIEW_PR_CAP, default 3/repo/tick, oldest-updated first), and skips for ai-review-queued and mergeable_state=dirty. Kill switch: wrangler var BACKSTOP_PR_SCAN=0 disables the PR scan only (the #6831 issue backstop stays live). See queue-dispatch-cross-project.md §0 mechanism 2.

Both enqueue a review-pr job for each PR that is:

Not labeled human-action (human must decide)
Not labeled ai-review-queued (already in queue / being reviewed)
Not a draft

`synchronize` self-heals a stale-approval-after-push PR (#6387)

This repo’s branch protection sets require_last_push_approval: true: a commit pushed after the AI review approved a PR invalidates that approval. Before #6387 there was no synchronize trigger, so such a push produced no event to re-queue a review — and because the PR still carried ai-review-queued, the periodic scanner skipped it too. The PR then sat in REVIEW_REQUIRED/BLOCKED indefinitely (live: #6184, stuck 6 days until manually re-triggered).

With synchronize wired in, a post-approval push re-queues review automatically; the worker re-reviews, re-approves via the distinct p24-infra-admin App identity (Phase 2 Step 7), and merges — no human click. Bot-authored pushes (worker fixup commits, rebases, auto-format bots) are the common trigger for this, so synchronize is allowlisted in dispatch-to-queue.yml’s bot-author gate exactly like opened is — it is not a self-queue-loop vector because the review-pr worker only comments and merges, it never pushes commits back to the PR branch.

Runaway guard. A burst of pushes during active development each fire synchronize. The meta-dispatcher’s in-flight dedup (BLOCKING_STATES, scoped to job_type='review-pr' in infra-src/meta-dispatcher/src/dedup.ts) skips a re-queue while a prior review-pr row for the same PR is still queued/claimed/running, collapsing the burst to one review. Once that row is terminal the next push (or scanner cycle) queues a fresh review. Locked by test/dedup.test.ts (synchronize re-fire is SKIPPED while a review-pr row is in flight).

Review-pr burn guard — head-sha dedup, throttle, conflict skip (#7663)

The synchronize self-heal above fixed PRs never being re-reviewed; this guard fixes the opposite failure — a PR being re-reviewed every cycle with no change. Because the review-pr worker removes ai-review-queued on every terminal outcome (so the scanner can re-review after a real push), an unchanged open PR was otherwise re-enqueued on each ~2-min scanner cycle: 26 and 25 review-pr rows on 7586 in 24h, and conflict-blocked PRs (#7632) re-reviewed APPROVE every cycle only to fail the merge. Three complementary suppressions, all keyed on the PR head sha so a genuine push (new sha) always re-triggers a review and the #6387 self-heal is preserved:

Head-sha dedup. Skip enqueuing a review-pr when a review-pr row already finished done at the PR’s current head sha — the diff is unchanged, so re-reviewing adds nothing. Implemented in both enqueue paths: the periodic scanner scan_prs_for_review() (scripts/queue-dispatcher-loop.py, which stamps metadata.head_sha on every row it creates) and the event-driven dedup guard reviewPrHeadShaSignal() (infra-src/meta-dispatcher/src/dedup.ts, wired into shouldSkipDispatchPure right after the in-flight check; singleDispatch in index.ts stamps metadata.head_sha for review-pr jobs).
Throttle. At most REVIEW_PR_THROTTLE_MAX (default 3) review-pr rows per PR at the same head sha within REVIEW_PR_THROTTLE_WINDOW_HOURS (default 6). Beyond the cap the scanner posts one idempotent comment (hidden marker dispatcher:review-pr-throttled) and stops enqueueing — it never silently drops. The count is scoped to the current head sha, so pushing a new commit clears the throttle immediately (new sha → count 0); otherwise it lifts when the 6h window rolls off.
Conflict skip. A re-review of a PR whose mergeable_state is dirty (GraphQL mergeStateStatus: DIRTY) can never merge and only burns a slot, so the scanner skips it and posts one “needs rebase” comment (hidden marker dispatcher:review-pr-conflict). A rebase push changes the head sha and re-triggers review via synchronize. The first-ever review of a PR still runs even if it is already dirty.

CI-skip exclusion — a pre-flight skip is not a completed review (#7736). The review-pr worker ends its queue row done for two different outcomes: a genuine council+judge review, and a pre-flight skip (draft, or required CI failing — Phase 0.1; already merged/closed — Phase 0.0). Rule 1 above naïvely treated any same-sha done row as “already reviewed”, so a CI-skip done row dedup’d the head sha exactly like a real review. If the PR’s CI was then fixed without a new commit (e.g. a PR-body edit to satisfy the full-close-keyword check — the head sha is unchanged), the guard returned skip_same_sha every cycle and the review was never re-enqueued — the PR wedged in REVIEW_REQUIRED with green CI and no human-action label (observed on PR #7730, which needed a manual review-pr row to unblock). The fix lives entirely in the dispatcher (scripts/queue-dispatcher-loop.py) because the worker prompt is a protected: true file:

_recent_review_rows_by_pr() selects each row’s result_summary and marks it ci_skipped when the summary contains the pre-flight skipped token (_review_summary_is_skip()) — genuine verdicts (APPROVE/CHANGES_REQUESTED/COMMENT) never do, so no worker-side metadata flag is needed.
_review_pr_burn_decision() dedups the head sha only on a genuine done review. When the only same-sha completion is a CI-skip, it re-enqueues a real review once CI is green, and returns the new silent action skip_ci_failing while CI is still red (so a still-broken PR is not re-reviewed — and comment-spammed — every cycle). CI state is fetched lazily by _gh_pr_ci_failing() (mirrors the worker’s statusCheckRollup conclusion == FAILURE check) only for this narrow CI-skip-wedge case; None (unknown) fails open to enqueue.
CI-skip rows spend ~zero Claude tokens, so they are excluded from the Rule 2 throttle count (only genuine reviews count toward the per-sha quota cap) — otherwise 3 cheap CI-skips at a sha would wedge the real review once CI finally passed.

Fail-open everywhere: an unknown head sha, a Supabase lookup error, an unknown CI state, or REVIEW_PR_BURN_GUARD_ENABLED set false all degrade to the pre-#7663 behaviour (always enqueue) — the guard can never wedge reviews. Each cycle the scanner logs a one-line summary of how many re-reviews it suppressed (burn guard suppressed N unchanged-sha, M throttled, K conflict, J ci-still-failing…) so a re-review storm is visible in journald before it exhausts Claude quota. Config keys follow the same .env-over-dispatcher-review-config.json precedence as REVIEW_REPOS (#6216): REVIEW_PR_BURN_GUARD_ENABLED, REVIEW_PR_THROTTLE_MAX, REVIEW_PR_THROTTLE_WINDOW_HOURS. Locked by scripts/tests/test_review_pr_burn_guard.py and test/dedup.test.ts.

Council cost (separate, protected-file change). Reducing the per-review cost (a 3-lens “lite” council for docs-only PRs instead of 9) requires editing the protected: true infra/agent-prompts/review-pr-worker.md, so it ships as a human-applied diff: docs/plans/plan-7663-review-council-lite.md (+ .patch). It is not auto-applied.

Auto-rebase of conflicted worker PRs (#7822)

The burn guard above only pauses re-review of a DIRTY (merge-conflicted) PR and posts a “needs rebase” comment — it never resolves the conflict. On radieu/et-operational-platform, worker PRs target dev; when two PRs touch the same files the second goes DIRTY and nothing rebases it (the dev-issue job ends done, the review-pr job can’t merge a conflicted PR). Those PRs stranded indefinitely (live: et-op 2380). The auto-rebase loop closes that gap.

Detection — scan_prs_for_rebase(budget_zone) runs at the end of every dispatcher cycle, right after scan_prs_for_review(). For each repo in AUTO_REBASE_REPOS (default radieu/et-operational-platform; base branch from REVIEW_REPO_BASE_BRANCHES, default main) it lists open PRs and runs the pure _rebase_pr_decision() per PR. It enqueues a rebase-pr job (weight heavy, priority 12 — above review-pr’s 15, below impl’s 10) only when the PR is DIRTY, authored by a worker account, not human-action, not draft, has no active review-pr/rebase-pr job, is under the attempt cap, and has idled past the floor.

Repair — the rebase-pr worker (infra/agent-prompts/rebase-pr-worker.md, a dedicated worker because review-pr-worker.md is protected: true and can’t be extended autonomously; routed by spawn-worker.sh, persona dev-coder). It re-verifies every guardrail against the live PR, deepens the shallow clone, checks out the PR head, rebases it onto the base branch, then:

clean replay (the common case — DIRTY only because the base moved) → run the repo’s type-check + tests, then git push --force-with-lease;
trivial conflicts only (import blocks, i18n locale JSON disjoint key sets, lockfiles) → resolve them, validate with tests, push;
non-trivial conflict (logic/content in a .ts/.tsx/.py body, JSX, config, or a locale key with two different values) → git rebase --abort, comment the file list, add human-action, mark the queue row done. Never force-push a half-resolved or test-failing tree.

The --force-with-lease push changes the head sha, which clears the burn guard and fires the pull_request synchronize event — so review-pr re-triggers automatically; the worker also removes any stale ai-review-queued label so the review scanner re-enqueues promptly.

Guardrails (defense in depth — enforced in both the scanner and the worker):

Guardrail	Mechanism
Worker PRs only	`AUTO_REBASE_PR_AUTHORS` allowlist (default `AI-Dev-BMS4-1,ai-dev-bms4-2`) — a human/other PR is never touched
No `human-action` PRs	checked before the attempt cap, so an already-escalated PR is a silent skip, not a re-escalation
Attempt cap	`AUTO_REBASE_MAX_ATTEMPTS` (3) rebase-pr rows per PR within `AUTO_REBASE_WINDOW_HOURS` (24h); on exhaustion → one idempotent comment (`AUTO_REBASE_ESCALATE_MARKER`) + `human-action`
Dedup (BLOCKING_STATES)	skip if an active (`queued`/`claimed`/`running`) review-pr OR rebase-pr row already covers the PR
Min idle	`AUTO_REBASE_MIN_IDLE_HOURS` (2h) on the PR’s `updated_at`, so a worker still iterating on its branch is never fought
Non-destructive push	`--force-with-lease=<ref>:<old-sha>` only — a concurrent human push is never clobbered
Budget guard	skipped in the `hard` zone (DIRTY PRs picked up once fleet headroom recovers)

Fail-open / off-switch: empty AUTO_REBASE_REPOS or AUTO_REBASE_ENABLED=false disables the scan entirely; any GitHub/Supabase error degrades to “do nothing this cycle” and logs a WARN (wrapped in main()’s try/except, exactly like scan_prs_for_review). A rebase-pr row carries a PR number in github_issue_number/metadata.pr_number, so it is held out of the per-issue claim mutex and issue-escalation paths via _PR_SCOPED_JOB_TYPES (the _REVIEW_JOB_TYPES superset, #2454). Locked by scripts/tests/test_scan_prs_for_rebase.py.

Enforcement model — approval required before merge, no worker self-merge (#7612)

Decided 2026-10-08 (#7612, parent #7611). An APPROVE is required before any repo’s PR merges, including radieu/et-operational-platform. et-op is a GitHub Free private repo, so it has no branch protection and no require_last_push_approval — approval cannot be enforced by the platform there and must be enforced by process. Evidence that drove the decision: et-op worker PRs were merged by the same account that opened them with zero reviews (e.g. #2210 landed ~14 s after opening).

Two coordinated controls implement this:

E1 — implementing workers never self-merge. worker-issue.md, worker-nextjs-dev.md, and worker-python-dev.md open the PR, set milestone Review, and STOP. They contain no autonomous gh pr merge on their own PR. (worker-python-dev.md already behaved this way.) Merging is owned exclusively by the review-pr worker below. On p24-infra this was already enforced by main branch protection; dropping the worker’s self-merge attempt makes it explicit for every repo and stops stranding the queue row as failed on the expected block. As a follow-up (#7629), worker-secret-manager.md Step -1a(ii) no longer gh pr merges a still-open generic CI-rotator PR (chore: auto-rotate credentials) either — a SOPS-touching PR is left open for the review-pr path to merge after an APPROVE, closing the last gh pr merge in infra/agent-prompts/ outside review-pr-worker.md.
E2 — review-pr worker verifies the APPROVE is on the current head sha before gh pr merge (gh api repos/{repo}/pulls/{n}/reviews, commit_id == head.sha). This file is protected: true — see the Enforcement site section below; the change lands via a human-confirmed edit.
E3 — detective backstop in et-op. The post-merge audit-merged-to-main guard is extended from main to dev/staging/rc: a PR merged without a head-sha APPROVE gets a red run + sticky comment. Filed as a cross-repo request in radieu/et-operational-platform (this repo does not edit et-op directly). Shipped as et-op’s guard-pr-base-branch.yml job “Audit merge that landed on an integration branch” (#2352, closed 2026-10-08).
E3a — the et-op APPROVE must be posted as p24-reviewer (#7823). The E3 audit’s isValidApprover whitelists only p24-reviewer (the dedicated review App) or a human — not App 2109526 (p24-infra-admin). So the fleet-wide “approve as App 2109526” rule (correct for p24-infra, where a contents:write approver is needed to clear require_last_push_approval) produces a no-approval false-positive on et-op: App 2109526’s approve is either not posted (et-op has no counting gate and the worker can’t approve its own PR → the chain falls back to a plain comment) or, if posted, the audit does not recognize it. et-op is a GitHub Free private repo → no branch protection, no counting required-review gate, so the approve identity does not need contents:write. Therefore the review-pr worker approves et-op PRs as p24-reviewer: a distinct non-author identity (no self-approve block) that satisfies the approval-before-merge guarantee, is recognized by the et-op audit as-is, and keeps the high-blast-radius App 2109526 out of et-op entirely. No et-op-side change is required — p24-reviewer is already whitelisted by the audit and installed fleet-wide (repository_selection: all, see reviewer-github-app.md §6/§7). This is a per-repo refinement of the APPROVE step in the protected: true review-pr-worker.md, so it ships as a human-confirmed edit (exact diff on #7823). Evidence that drove the decision: every recent et-op automation PR (e.g. #2375, #2380, #2394) fails the audit with reviews: [] even though review-pr ran and judged APPROVE. Options (b) “teach the audit to accept an App-2109526 approve” and (c) “silence the audit” were rejected: (b) keeps the privileged App as the et-op approver (worse blast radius) and still needs the worker to post it; (c) suppresses a check that is correctly detecting a genuine missing-approval, re-opening the #7612 self-merge gap.

Merge method per repo (E6). The merge is performed by the review-pr worker, so the method is the review worker’s: --squash --delete-branch for every repo (see the APPROVE row below). The implementing-worker --merge calls were the self-merge that E1 removed, so there is no longer a --merge vs --squash split to reconcile — squash is the single documented method.

The review worker (infra/agent-prompts/review-pr-worker.md) runs two-pass analysis (Haiku scout → Sonnet analyst) and then takes one of three actions:

Verdict	Action
APPROVE or COMMENT (only MINOR issues)	Squash-merge PR, delete branch, remove `ai-review-queued` label
REQUEST_CHANGES (CRITICAL or MAJOR issues)	Close PR with explanation, comment on linked issue, remove `ai-review-queued` + add `ai-review-rejected`, re-queue issue for reimplementation
SKIP (draft / CI failing)	Post a comment explaining skip, mark queue row done

Labels

Label	Set by	Meaning
`human-action`	Worker or human	Do NOT auto-review — human decision required
`ai-review-queued`	Dispatcher scanner	PR is in the review queue (prevents double-dispatch)
`ai-review-rejected`	Review worker	PR was closed by AI review; issue re-queued

Add these to a repo’s standard labels via new-repo-setup.md §Step 4 or:

$repo = "radieu/<repo>"
gh label create "ai-review-queued"  --repo $repo --color "0075ca" --description "PR queued for AI auto-review"
gh label create "ai-review-rejected" --repo $repo --color "d93f0b" --description "PR rejected by AI review — issue re-queued"

Configuration

The dispatcher reads two env vars. They must be set in the EnvironmentFile of whichever host is the current dispatch leader (p24-queue-dispatcher.service is HA leader-elected — only the leader runs scan_prs_for_review()). Each host reads a different EnvironmentFile (see the unit header):

Dispatch host	EnvironmentFile the dispatcher reads
vps-i1 (nominal “primary” per the lease note)	`/opt/p24-infra/monitoring/.env`
bms-4 (nominal “backup”; actual lease holder since 2026-07-30)	`/opt/p24-infra/bms-4/.env`

Because the leader can be either host, set both vars in BOTH files so the config survives an HA failover. The #5607 roll-out set them in both (2026-08-05).

Env var	Meaning	Default
`REVIEW_REPOS`	Comma-separated repos to scan for PRs to auto-review. Empty = disable.	`radieu/p24-infra`
`REVIEW_REPO_BASE_BRANCHES`	Comma-separated `repo=branch` overrides for the PR-list base filter (#5607). Any repo not listed defaults to `main`.	(empty)
`REVIEW_PR_BURN_GUARD_ENABLED`	Master switch for the #7663 burn guard (head-sha dedup + throttle + conflict skip). Anything other than a truthy string disables it (full fail-open).	`true`
`REVIEW_PR_THROTTLE_MAX`	Max review-pr rows per PR at the same head sha within the window before the scanner throttles and posts one comment.	`3`
`REVIEW_PR_THROTTLE_WINDOW_HOURS`	Rolling window (hours) for the throttle count and the head-sha dedup lookup.	`6`
`AUTO_REBASE_ENABLED`	Master switch for the #7822 auto-rebase scanner. Falsey = disabled (no scan).	`true`
`AUTO_REBASE_REPOS`	Comma-separated repos whose DIRTY worker PRs are auto-rebased. Empty = disable. Base branch from `REVIEW_REPO_BASE_BRANCHES` (default `main`).	`radieu/et-operational-platform`
`AUTO_REBASE_PR_AUTHORS`	Comma-separated GitHub logins (case-insensitive) whose PRs may be auto-rebased.	`AI-Dev-BMS4-1,ai-dev-bms4-2`
`AUTO_REBASE_MAX_ATTEMPTS`	Max rebase-pr rows per PR within the window before escalating `human-action`.	`3`
`AUTO_REBASE_WINDOW_HOURS`	Rolling window (hours) for the rebase attempt count.	`24`
`AUTO_REBASE_MIN_IDLE_HOURS`	A DIRTY PR must be idle (no `updated_at` change) this long before a rebase is attempted.	`2`

AUTO_REBASE_ENABLED / AUTO_REBASE_REPOS live in the same tracked scripts/dispatcher-review-config.json (same .env-over-file precedence); the remaining AUTO_REBASE_* tuning keys default in-code and are overridable via the dispatcher .env. See §Auto-rebase above.

Current fleet-wide value (#5607) — source of truth is the tracked file scripts/dispatcher-review-config.json (#6216), deployed to every host by deploy-vps-scripts.yml and read by the dispatcher when the env var is unset (so it survives any .env regeneration):

{
  "REVIEW_REPOS": "radieu/p24-infra,radieu/et-operational-platform,radieu/Art-Agency,radieu/radekkonarski-personal-brand,radieu/gmail-tools",
  "REVIEW_REPO_BASE_BRANCHES": "radieu/et-operational-platform=dev",
  "AUTO_REBASE_ENABLED": "true",
  "AUTO_REBASE_REPOS": "radieu/et-operational-platform"
}

To change the fleet config, edit that file in a PR — do not hand-append to the on-server .env (that append is dropped on the next SOPS-driven .env regeneration; it was the #6216 outage). The equivalent .env override still exists for host-specific tweaks / emergency disable:

# et-operational-platform's PRs target `dev`, not `main` — override it (all others default to main):
REVIEW_REPOS=radieu/p24-infra,radieu/et-operational-platform,radieu/Art-Agency,radieu/radekkonarski-personal-brand,radieu/gmail-tools
REVIEW_REPO_BASE_BRANCHES=radieu/et-operational-platform=dev

After a deploy-vps-scripts.yml run (or editing .env), the next dispatcher cycle (a Type=oneshot timer fire that re-reads both the checkout and the EnvironmentFile) picks the values up automatically — no restart needed.

Diagnostic (#6216): scan_prs_for_review() no longer early-returns silently. It logs a distinct line for each skip cause — skipped — GH_TOKEN not set … or skipped — REVIEW_REPOS empty … — to the dispatcher journal every cycle, so a broken-config state (the class below) is visible within one ~2-min cycle instead of going unnoticed. Grep the dispatcher journal / /var/log/p24-infra-dispatcher.log for scan_prs_for_review: skipped.

⚠️ Known blocker — scanner needs GH_TOKEN on the active leader (found 2026-08-05, #5607). scan_prs_for_review() starts with if not GH_TOKEN: … return / if not REVIEW_REPOS: … return (each now logs its cause, #6216). GH_TOKEN is present in vps-i1’s /opt/p24-infra/monitoring/.env but absent from bms-4’s /opt/p24-infra/bms-4/.env (the isolated secrets/n8n-bms4-gh.env.sops GH_TOKEN is deployed by secrets-sync.yml into worker/role env files, not the dispatcher’s EnvironmentFile). Since bms-4 has held the dispatch lease since 2026-07-30 (vps-i1 healthy but logs “Not leader” every cycle — no preemption toward the nominal primary), the scanner has been inactive fleet-wide — zero review-pr rows enqueued since 2026-07-30. p24-infra’s own reviews were unaffected only because they also arrive via the event-driven dispatch-to-queue.yml (pull_request trigger), a path the other repos do not have. So the REVIEW_REPOS config above is correct but inert until a GH_TOKEN-bearing host is the scan leader. Fix options (a decision-stop, escalated on #5607): (A) deliver GH_TOKEN to the bms-4 dispatcher EnvironmentFile (secret-manager: add to the SOPS source feeding bms-4/.env, or add the role/worker-keys env as a second unit EnvironmentFile=); (B) restore vps-i1 (which has GH_TOKEN) as the dispatch leader; or (C) a code fallback in queue-dispatcher.sh to export GH_TOKEN from an existing bms-4 token when unset. Until one lands, verify via the event path on p24-infra only.

Prerequisites when adding a repo (do not silently add to REVIEW_REPOS):

Confirm the repo’s current PR-target branch from its own CLAUDE.md §Branching — do not assume. Verified 2026-08-05: radieu/et-operational-platform targets dev (its CLAUDE.md: “Każdy nowy branch feature/fix tworzymy od dev. PR celuje w dev”), so it must be listed in REVIEW_REPO_BASE_BRANCHES. Art-Agency, radekkonarski-personal-brand and gmail-tools default to main.

Confirm the repo is wired for job_type=review-pr execution (correct role/job_profile per docs/playbooks/cross-repo-worker-isolation.md) before adding it — otherwise the enqueued review job fails at spawn.

radieu/brandpilot is intentionally excluded (2026-08-05): it is not resolvable by the dispatcher’s gh/GH_TOKEN (both radieu/brandpilot and p24-infra/brandpilot return “Could not resolve to a Repository”). Resolve the repo location/access first, then add it.

Add the standard labels (see §Labels) to the repo before or when adding it — scan_prs_for_review() sets ai-review-queued, and the review worker sets ai-review-rejected.

Base-branch filter (per-repo since #5607). scan_prs_for_review() lists PRs with a base branch resolved per repo: REVIEW_REPO_BASE_BRANCHES.get(repo, "main"). Repos not in the map use main (this repo’s policy — CLAUDE.md §Branching). Before #5607 the filter was hardcoded base=main for every repo (and base=dev before #4579), so it silently no-oped — returning zero PRs with no error — for any repo whose PRs target another branch. That is why et-operational-platform=dev is required. The event-driven pull_request handler in .github/workflows/dispatch-to-queue.yml has no branches: filter and is unaffected. scripts/tests/test_scan_prs_base_branch.py locks the default, the per-repo lookup, and the no-hardcoded-branch rule.

Durability — resolved (#6216). These two vars are non-secret config. They used to live only in the hand-edited on-server .env, so any redeploy that regenerated .env from SOPS silently dropped them (a ~week-long fleet-wide PR-review outage). The source of truth is now the tracked, non-secret file scripts/dispatcher-review-config.json, deployed to /opt/p24-infra by deploy-vps-scripts.yml (git reset --hard origin/main) — so it survives any .env regeneration. A matching env var in the dispatcher .env still overrides the file (host-specific tweaks + emergency disable). This was chosen over moving the vars into secrets/monitoring.env.sops (a secret-manager op) precisely because they are non-secret: keeping them as tracked repo config keeps them reviewable in a normal PR and configurable without a secret-manager round-trip.

Precedence (per key): dispatcher .env (os.environ, present-even-if-empty → wins) → scripts/dispatcher-review-config.json → "". Emergency-disable still works: set REVIEW_REPOS= (empty) in .env and the present-but-empty env value beats the file (see Disabling globally below).

Priority of review-pr jobs: 15. Both insertion paths must stay aligned to this value — scan_prs_for_review() here and the pull_request handler in .github/workflows/dispatch-to-queue.yml. Before #3526 they had drifted apart (15 vs 20); since the GH Actions path fires immediately on PR events (ahead of the ~2min scanner cycle), the drift meant review-pr jobs effectively ran at 20 in practice.

This sits between the default (unlabeled) dev-issue priority (12) and the P2 dev-issue tier (50): P1 (10) < default dev-issue (12) < review-pr (15) < P2 (50). Reviews no longer crowd out unlabeled implementation work, but still outrank the lowest-priority dev-issue backlog.

Flow diagram

Dispatcher cycle (every 2 min)
  └─ scan_prs_for_review()
       ├─ gh pr list (open, base=REVIEW_REPO_BASE_BRANCHES.get(repo, "main"), repo=REVIEW_REPOS)
       ├─ skip: human-action | ai-review-queued | draft | already in queue
       ├─ INSERT dev_r_worker_queue (job_type=review-pr, priority=15)
       └─ add label ai-review-queued to PR

Worker (bms-4 or vps-i1)
  └─ review-pr-worker.md
       ├─ Phase 0: Haiku scout (pre-scan, CI check, diff)
       ├─ Phase 1: Sonnet deep analysis
       ├─ Phase 2: verdict logic
       ├─ Phase 3: post GitHub review comment
       ├─ Phase 3.5: ACTION
       │    ├─ APPROVE / COMMENT → gh pr merge --squash --delete-branch
       │    └─ REQUEST_CHANGES  → gh pr close
       │                          → gh issue comment (linked issue)
       │                          → INSERT dev_r_worker_queue (issue re-queue)
       │                          → label ai-review-rejected
       └─ Phase 4: cleanup + mark queue row done

Review dismissal — never dismiss a review on timing alone (#6815)

A review that raises specific, falsifiable findings must never be dismissed on the grounds that a later automated pass reached a different verdict on the same, unchanged commit. “Predates this run’s approval” is a timing argument, not a correctness one — and timing alone is never grounds to override a review. This governs the review-pr worker’s APPROVE → merge action (Phase 3 Case A of infra/agent-prompts/review-pr-worker.md).

Background (the incident this rule exists for). On PR #6810 a CHANGES_REQUESTED review was posted with two code-verified findings. ~6 minutes later — with no new commit (HEAD unchanged at d8ae07ff) — a fresh council+judge pass concluded APPROVE and dismissed the prior review with “Superseded by the current review-pr run … The earlier CHANGES_REQUESTED is stale (predates this run’s approval). Dismissing to honor the current verdict.” The PR merged with both findings still present in the shipped code. This manual dismissal was improvised — it is not (and must not be) written into any worker step; the fix is to forbid it explicitly.

The rule

Same-commit dismissal is prohibited. The review-pr worker MUST NOT call GitHub’s review-dismissal API (PUT /repos/{repo}/pulls/{pr}/reviews/{id}/dismissals, or any equivalent) to clear another reviewer’s review — human or bot — while HEAD is unchanged from what that review examined. A blocking CHANGES_REQUESTED on the current HEAD is not “stale” merely because a newer pass ran after it.
Staleness comes only from a real new commit. A prior review becomes legitimately stale only when an actual new commit changes HEAD — at which point GitHub’s own native auto-dismiss-on-push (this repo sets dismiss_stale_reviews: true + require_last_push_approval: true) handles it, and a fresh review is re-queued via the synchronize event (see the self-heal section above). The worker relies on that native behavior; it never hand-rolls a same-commit dismissal.
If the worker’s own pass would APPROVE but a blocking review exists on the current HEAD, it has exactly two allowed outcomes — dismissal is not one of them:
- (a) Engage point-by-point. Address each specific finding of the prior review in its own review body/comment, stating for each why it does not block on the current code (e.g. “the unbounded glob is acceptable at current data volumes because …”). A blanket “newer verdict wins” is never sufficient. Only after genuine point-by-point engagement may it proceed, and only if branch protection then permits the merge without a manual dismissal.
- (b) Leave it for a human. Do not auto-merge. Treat it exactly like the “merge fails after APPROVE” path: log the block, keep the PR open, and remove ai-review-queued (per Monitoring / #6387) so the PR is not a dead-end for the next scanner cycle.
A human-identity review is never bot-dismissable at all. A radieu-authored CHANGES_REQUESTED (whether posted by a person or by Claude acting under that identity) may be cleared only by an actual new commit (GitHub-native dismissal) or by that human’s own re-review — never by a bot pass on the same commit, not even with point-by-point engagement.

Enforcement site (protected file — human edit required)

The binding guard belongs in infra/agent-prompts/review-pr-worker.md Phase 3 Case A (the APPROVE → gh pr merge block), which is protected: true (change-policy: PRODUCTION — human confirmation required before ANY edit). A dev-issue worker cannot edit it autonomously (#6173). The exact clause to paste into Case A, immediately before the gh pr merge call, is:

Do NOT dismiss a blocking review to unblock this merge (#6815). If a CHANGES_REQUESTED review exists on the current (unchanged) HEAD, never call the review-dismissal API to clear it. Either engage every one of its specific findings point-by-point in your own review body, or leave the PR open for a human (log the block, set ACTION="merge-failed", remove ai-review-queued, continue to Phase 4 — the existing merge-failure fallback). A radieu-identity review is never bot-dismissable on the same commit at all. See docs/playbooks/ai-pr-auto-review.md §Review dismissal.

Enforcement model — approve-before-merge (#7612, E1–E6)

#7612 defines a set of controls (E1–E6) that make “an APPROVE must exist before a worker merges” an actual gate rather than a convention. The need differs by repo: on p24-infra, main branch protection (required_approving_review_count: 1, require_last_push_approval: true) already enforces it structurally and these controls are a defensive backup; on et-operational-platform (a GitHub Free private repo) nothing structural enforces approve-before-merge, so the worker-side gate is the control.

Control	What it does	Where	Status
E1 / E4	Workers stop self-merging; approval is enforced before merge	review-pr-worker.md / et-op	Pending — PR #7620 (open)
E2	Head-sha APPROVE gate: Phase 3 Case A merges only when a formal `APPROVED` review exists whose `commit_id` equals the current PR head sha (a stale approve from an earlier push does not count). A merge-eligible `COMMENT` verdict is also posted as a formal APPROVE (option a) so it passes the gate.	`infra/agent-prompts/review-pr-worker.md` Phase 2 Step 7 + Phase 3 Case A	Ready — #7628. `review-pr-worker.md` is `protected: true`; the exact change is staged as a `git apply`-able diff at `docs/plans/7628-e2-head-sha-approve-gate.review-pr-worker.diff` and applied by a human (the `protected`-frontmatter guard blocks an autonomous worker).
E3 / E5	et-op side of the gate	radieu/et-operational-platform#2352	Tracked in that repo

E2 behaviour, by case (the four cases the #7628 proof covers):

Case	Head-sha APPROVE present?	Outcome
APPROVE on current head sha	yes	merges as before
APPROVE only on an earlier (stale) sha	no (commit_id ≠ head)	merge held — “Merge held (E2)” comment, PR stays open
No APPROVE at all	no	merge held — same
Merge-eligible `COMMENT` verdict	posted as a formal APPROVE on head	merges

A merge held by the E2 gate is not an error: the PR stays open and Phase 4 removes ai-review-queued on every outcome (idempotent, see §Monitoring), so a new push or the next scan_prs_for_review() cycle re-queues the review. This is the same “leave it open, clear the label” discipline as the §Review dismissal rule and the merge-failure fallback.

Monitoring

Grafana: worker-queue dashboard — filter job_type=review-pr to see review jobs.

Supabase: dev_r_worker_queue where job_type = 'review-pr' — result_summary field shows outcome (merged / rejected+requeued / skipped).

GitHub PR labels: ai-review-queued (in progress) → removed on any terminal outcome that leaves the PR open, not only a successful merge (#6387). Specifically the worker (Phase 4) must remove it on: merged, closed (REQUEST_CHANGES), skipped (draft/CI), and approved-but-merge-failed (branch-protection / stale-approval block). Leaving the label on an open PR is the #6184 dead-end: scan_prs_for_review() skips any PR that still carries ai-review-queued, so a stuck-open PR would never be re-picked-up by the scanner. Clearing it means the next scanner cycle (or a synchronize event) re-queues a fresh review. ai-review-rejected (blocked PRs that need rework).

What triggers human-action (escapes auto-review)

The review worker sets human-action on a PR’s linked issue (not the PR itself) when:

OOM / spawn failure exceeds max retries (set by dispatcher)
The re-queued issue fails 3+ times

The dispatcher sets human-action on the linked issue when:

spawn_failures > MAX_SPAWN_FAILURES

A PR itself is never labeled human-action by the review worker — it is either merged or closed. If neither worked, the queue row is marked failed and Discord alerts fire.

Disabling for a PR

To opt a specific PR out of auto-review, add the human-action label to it. The scanner skips any PR with this label.

Disabling globally

Set REVIEW_REPOS= (empty string) in the dispatcher .env on the active leader (bms-4: /opt/p24-infra/bms-4/.env; vps-i1: /opt/p24-infra/monitoring/.env). A present-but-empty env value beats the tracked scripts/dispatcher-review-config.json default (#6216 precedence), so the scanner no-ops even though the file still lists repos. The skip is logged (scan_prs_for_review: skipped — REVIEW_REPOS empty …), so an intentional disable is distinguishable in the journal from an accidental one. To disable permanently in git, empty the REVIEW_REPOS value in the tracked file instead.

Escalation

Review job stuck in queued > 10 min: check dev_r_server_capacity — all slots may be full with heavier jobs. review-pr jobs are light and should run quickly once a slot opens.

Merge fails after APPROVE: likely a branch protection rule (required reviews, CI gate). Check gh pr view ${PR_NUMBER} for merge errors. The review worker will log the error and mark the queue row done (not failed) — the PR stays open for human merge. On this path the worker must still remove ai-review-queued (see Monitoring, #6387): otherwise the PR is invisible to the next scan_prs_for_review() cycle and self-heal (fresh review on the next synchronize or scanner sweep) never fires.

Observed (2026-08-09, PRs 6037): the self-approval collision is probabilistic, not structural (#6039). The queue picks a worker OS account per job by Claude-subscription headroom, not by GitHub identity — so claude-runner (GitHub AI-Dev-BMS4-1) and claude-runner-2 (ai-dev-bms4-2) are genuinely distinct GitHub accounts, but nothing guarantees the reviewer account differs from the branch’s pusher account. When they happen to coincide, the approval (even when the gh pr review --approve command itself succeeds) does not satisfy require_last_push_approval, and gh pr merge then fails with New changes require approval from someone other than the last pusher — even with --admin. When they differ, it just merges (e.g. #6037 reviewed+approved by ai-dev-bms4-2 ≠ pusher, merged cleanly). A genuinely non-pushing reviewing identity is what’s required.

Status (2026-10-08, #7478 / #7574): the dedicated p24-reviewer App is live and posts reviews, but its approval does NOT count toward the required review on main. The App (App ID 5208530, installed on all repos of the radieu account; real creds in secrets/github-reviewer-app.env.sops, #7516), --app reviewer (#7483) and the worker switch (review-pr-worker.md v3.2.0, #7595) are all deployed. Worker logs (review-pr-7604, -7606, -7607, -7608) show the APPROVE posted as p24-reviewer (exit 0) while reviewDecision stayed REVIEW_REQUIRED: the App’s reviews have authorAssociation: NONE (likely because it holds contents:read only — inferred, not proven). Owner decision 2026-10-08 (“option 2”, #7625): keep p24-reviewer review-only (no contents:write). So the counting approve = App 2109526 (p24-infra-admin) — the mechanism described below remains the one that actually unblocks merges — and p24-reviewer = comments / CHANGES_REQUESTED only. See reviewer-github-app.md §6 for the full finding. Merge stays on the existing path regardless — p24-reviewer has no contents:write, so it never merges.

Fix (#6039): approve as the App-2109526 bot, unconditionally, for every APPROVE. The App bot (scripts/gh-app-token --permissions pull_requests:write → the distinct p24-infra-admin identity) never pushes PR branches — the App token is only used for spawn-worker.sh’s clone, never inside a worker session — so an APPROVE posted under it is always from a non-pusher identity and satisfies require_last_push_approval on every PR, regardless of which OS account ran the review. infra/agent-prompts/review-pr-worker.md’s judge/review step (Phase 2 Step 7) posts the APPROVE under the App token as the primary path (not a failure-gated retry — a same-account approve can return exit 0 yet still fail to satisfy require_last_push_approval, so the old “retry only on approve-command-failure” logic missed exactly that case). It falls back to a worker-identity approve only if the App token can’t be minted. gh pr merge then succeeds with no human click. The App-token approve mechanism itself was confirmed live 2026-08-12 (PR #6211, manual SSH-executed call); #6039 makes it the unconditional approve path. Residual case: if the App token can’t be minted (mint failure, missing pull_requests:write grant), the fallback worker-identity approve may self-collide and the PR then needs a human merge click — don’t loop retrying gh pr merge past that genuine failure.

Re-queue loop: if an issue keeps getting rejected, check spawn_failures in dev_r_worker_queue — after MAX_SPAWN_FAILURES (5) the dispatcher escalates to human-action and stops re-dispatching.

p24-infra Docs

Explorer

ai-pr-auto-review

Playbook: AI PR Auto-Review System

Overview

`synchronize` self-heals a stale-approval-after-push PR (#6387)

Review-pr burn guard — head-sha dedup, throttle, conflict skip (#7663)

Auto-rebase of conflicted worker PRs (#7822)

Enforcement model — approval required before merge, no worker self-merge (#7612)

Labels

Configuration

Flow diagram

Review dismissal — never dismiss a review on timing alone (#6815)

The rule

Enforcement site (protected file — human edit required)

Enforcement model — approve-before-merge (#7612, E1–E6)

Monitoring

What triggers human-action (escapes auto-review)

Disabling for a PR

Disabling globally

Escalation

Graph View

Table of Contents

Backlinks

p24-infra Docs

Explorer

ai-pr-auto-review

Playbook: AI PR Auto-Review System

Overview

synchronize self-heals a stale-approval-after-push PR (#6387)

Review-pr burn guard — head-sha dedup, throttle, conflict skip (#7663)

Auto-rebase of conflicted worker PRs (#7822)

Enforcement model — approval required before merge, no worker self-merge (#7612)

Labels

Configuration

Flow diagram

Review dismissal — never dismiss a review on timing alone (#6815)

The rule

Enforcement site (protected file — human edit required)

Enforcement model — approve-before-merge (#7612, E1–E6)

Monitoring

What triggers human-action (escapes auto-review)

Disabling for a PR

Disabling globally

Escalation

Graph View

Table of Contents

Backlinks

`synchronize` self-heals a stale-approval-after-push PR (#6387)