| name | review-pr |
| description | Run two parallel adversarial PR reviewers and combine their verdicts with
external PR reviews (GH Copilot bot, humans, other bots) and CI state into a
merge decision. Agent reviewers post structured comment verdicts (since the
agent is the PR author and cannot self-approve via GH native review).
External CHANGES_REQUESTED reviews block merge identically to agent
CHANGES_REQUESTED. Operates on issues in `in-review`. Auto-merges with
--admin on all-green (after filing follow-up issues for unaddressed inline
review comments, so non-critical feedback is preserved as backlog instead of
dropped); bounces to `ready-for-doing` on any request-changes or CI red;
blocks after 3 bounce-backs on the same code state. At the lifetime cap
(6 bounces since last `## Review: Reset`) the PR enters **force-converge
mode**: if CI is green, the PR auto-merges and unresolved reviewer concerns
become follow-up issues; only red/pending CI at the cap still blocks. Use
when invoked by /project-tick with an issue in in-review, or manually as
/review-pr <issue>.
|
| argument-hint | <issue-number> |
/review-pr — adversarial PR review
You are the PR-level gate. Three independent signals decide the merge: reviewer A, reviewer B, and CI. All three must be green to auto-merge. Any red bounces.
You do not review the code as one undifferentiated pass — you run two independent reviewer lenses that use the existing /review and /spec-adhere skills (or /review-fix in review mode for risk-focused review), then combine their PR reviews with CI state. Under the pipeline's current single-level dispatch model (the orchestrator spawns one specialist per issue; that specialist cannot itself spawn nested sub-agents), you run both reviewer lenses yourself, as clearly-delimited in-agent passes rather than as nested sub-agents — see the multi-lens discipline in Step 4.
Inputs
$ISSUE — issue number.
- The linked open PR (fetched from the issue).
- The PR's diff, CI run state, and any prior reviews.
Step 0 — Find the PR
Source the shared merge helper for linked-PR state classification and merge execution:
REPO_ROOT="$(git rev-parse --show-toplevel)"
source "$REPO_ROOT/scripts/lib/review-pr-merge.sh"
Classify the linked PR state (distinguishes OPEN, MERGED, and CLOSED-without-merge):
if ! PR_STATE=$(classify_linked_pr_state $ISSUE); then
sleep 5
if ! PR_STATE=$(classify_linked_pr_state $ISSUE); then
echo "## Review: API Failure" && exit 1
fi
fi
Extract PR_NUM and route based on state:
case "$PR_STATE" in
open:*)
PR_NUM="${PR_STATE#open:}"
;;
merged:*)
PR_NUM="${PR_STATE#merged:}"
MERGE_SHA=$(gh pr view "$PR_NUM" --repo stellar-experimental/henyey \
--json mergeCommit --jq '.mergeCommit.oid')
FOLLOWUP_ISSUES=$(gh pr view "$PR_NUM" --repo stellar-experimental/henyey \
--json comments --jq '
[.comments[].body
| select(contains("## Review: Auto-merge armed"))
| capture("Follow-up issues filed:\\*\\* (?<nums>[^\n]+)"; "g") | .nums]
| last // "none"')
;;
closed:*)
;;
missing)
;;
esac
Handle each state:
open:<number> → PR_NUM extracted above; proceed to Step 0.5.
merged:<number> → the PR was already merged (e.g. by a previous /review-pr tick that armed auto-merge and GH completed it). Recover the merge SHA via gh pr view <number> --json mergeCommit --jq '.mergeCommit.oid' and follow-up issue numbers from the ## Review: Auto-merge armed comment body. Run the Step 7.4 cleanup/done path and exit.
closed:<number> → PR was closed without merge. That's a bug in /do. Post ## Review: No PR Linked and move the issue back to ready-for-doing. Unassign. Exit.
missing → no linked PRs at all. Same handling as closed.
error:<message> → API failure. Retry once after 5s; if still failing, leave assigned and exit non-zero.
Step 0.5 — Check if auto-merge is already armed
Before running the reviewer passes or filing follow-up issues, check whether this PR already has auto-merge enabled from a previous tick:
if ! AUTO_MERGE_STATE=$(is_auto_merge_armed $PR_NUM); then
echo "Warning: could not check auto-merge state (API failure)" >&2
elif [[ "$AUTO_MERGE_STATE" == "true" ]]; then
ARMED_HEALTH=$(check_armed_pr_health $PR_NUM) || ARMED_HEALTH="error"
case "$ARMED_HEALTH" in
healthy)
ALREADY_COMMENTED=$(has_armed_waiting_comment $PR_NUM)
if [[ "$ALREADY_COMMENTED" != "true" ]]; then
gh pr comment $PR_NUM --repo stellar-experimental/henyey \
--body "## Review: Auto-merge armed — waiting
Auto-merge was previously enabled on this PR. GitHub will merge automatically once all branch protection checks pass. Re-picking on next tick to verify completion.
CI state: healthy. Next check on next tick."
fi
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
exit 0
;;
ci-red)
gh pr merge $PR_NUM --repo stellar-experimental/henyey --disable-auto
gh pr comment $PR_NUM --repo stellar-experimental/henyey \
--body "## Review: Auto-merge cancelled — CI red
Auto-merge was armed but CI has failures. Disabling auto-merge and bouncing back to \`/do\` for investigation."
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE ready-for-doing
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
exit 0
;;
ci-stuck)
gh pr merge $PR_NUM --repo stellar-experimental/henyey --disable-auto
gh pr comment $PR_NUM --repo stellar-experimental/henyey \
--body "## Review: Auto-merge cancelled — CI stuck
Auto-merge was armed but CI has been running past the stuck threshold. Blocking for operator investigation."
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE blocked
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
exit 0
;;
not-mergeable)
gh pr merge $PR_NUM --repo stellar-experimental/henyey --disable-auto
gh pr comment $PR_NUM --repo stellar-experimental/henyey \
--body "## Review: Auto-merge cancelled — merge conflicts
Auto-merge was armed but the PR now has merge conflicts. Bouncing back to \`/do\` for rebase."
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE ready-for-doing
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
exit 0
;;
no-ci)
gh pr merge $PR_NUM --repo stellar-experimental/henyey --disable-auto
gh pr comment $PR_NUM --repo stellar-experimental/henyey \
--body "## Review: Auto-merge cancelled — no CI detected
Auto-merge was armed but no CI checks are present on this PR. This may indicate a workflow misconfiguration or disabled Actions. Blocking for operator investigation."
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE blocked
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
exit 0
;;
error)
echo "Warning: could not check armed PR health (API failure), proceeding with full review" >&2
;;
esac
fi
This short-circuit prevents duplicate reviewer runs and duplicate follow-up issue filing on subsequent ticks while waiting for GitHub to complete the deferred merge. The has_armed_waiting_comment check ensures idempotency: if a waiting comment was already posted on a previous tick, no duplicate is created. Critically, it still checks CI state on every tick — if CI goes red or gets stuck while auto-merge is armed, the skill cancels auto-merge and routes the issue appropriately rather than waiting indefinitely.
Step 1 — Read the PR + CI state
gh pr view $PR_NUM --repo stellar-experimental/henyey \
--json title,body,baseRefName,headRefName,reviews,files,statusCheckRollup,mergeable
gh run list --repo stellar-experimental/henyey --branch <head-ref> --limit 5 \
--json conclusion,status,name,databaseId
Capture:
- Files changed — for parity-vs-risk reviewer auto-detection.
- Current CI status:
green | failing | running | not_started.
- Existing reviews: for bounce-back counting.
Step 2 — Count prior bounce-backs (head-scoped + lifetime cap, both with Reset escape hatch)
Two caps run together:
- Head-scoped cap (3 bounces per code state): old bounces against earlier commits don't carry forward — they represent failed merge attempts on code that has since been rebased or replaced. Fresh push naturally resets this counter.
- Lifetime cap (6 bounces since last Reset): catches the runaway pattern where
/do keeps pushing fresh commits and the head-scoped counter resets every cycle, allowing indefinite churn. Only a ## Review: Reset resets this counter.
Compute the head-scoped baseline timestamp:
HEAD_PUSHED_ISO=$(gh pr view $PR_NUM --repo stellar-experimental/henyey \
--json commits --jq '.commits | sort_by(.committedDate) | last | .committedDate')
RESET_AT_ISO=$(gh api repos/stellar-experimental/henyey/issues/$ISSUE/comments --paginate \
--jq '[.[] | select(.body | startswith("## Review: Reset"))] | sort_by(.created_at) | last.created_at // ""')
HEAD_PUSHED_EPOCH=$(date -u -d "$HEAD_PUSHED_ISO" +%s)
if [ -n "$RESET_AT_ISO" ]; then
RESET_AT_EPOCH=$(date -u -d "$RESET_AT_ISO" +%s)
else
RESET_AT_EPOCH=0
fi
if [ "$RESET_AT_EPOCH" -gt "$HEAD_PUSHED_EPOCH" ]; then
HEAD_BASELINE_EPOCH=$RESET_AT_EPOCH
else
HEAD_BASELINE_EPOCH=$HEAD_PUSHED_EPOCH
fi
LIFETIME_BASELINE_EPOCH=$RESET_AT_EPOCH
Then count bounce comments for each cap (jq's fromdate parses ISO-8601 into epoch seconds, so the comparison is numeric, not lexical):
ALL_BOUNCES_JSON=$(gh api repos/stellar-experimental/henyey/issues/$ISSUE/comments --paginate \
--jq "[.[] | select(.body | startswith(\"## Review: Bounce-Back Cycle\")) | .created_at]")
HEAD_COUNT=$(echo "$ALL_BOUNCES_JSON" | jq --argjson base "$HEAD_BASELINE_EPOCH" \
'[.[] | select((. | fromdate) > $base)] | length')
LIFETIME_COUNT=$(echo "$ALL_BOUNCES_JSON" | jq --argjson base "$LIFETIME_BASELINE_EPOCH" \
'[.[] | select((. | fromdate) > $base)] | length')
Cap checks (apply in order):
- If
LIFETIME_COUNT >= 6, the PR has cycled too many times across multiple code states. Enter force-converge mode: export FORCE_CONVERGE=1 and continue with the normal review flow. The matrix in Step 6 will, if CI is green, route to a force-merge that files follow-up issues for any remaining CHANGES_REQUESTED concerns. If CI is still red/pending at the lifetime cap, the matrix routes to blocked (force-converge cannot merge unsafe code). This replaces the older "always block at lifetime cap" rule, which created a graveyard of PRs nobody could land.
- Otherwise, if
HEAD_COUNT >= 3, this is the 4th cycle on the current code. Post ## Review: Cycle Cap Reached, move to blocked, unassign, and exit. (Head-scoped cap still blocks; force-converge applies only to the lifetime cap.)
- Otherwise (both under cap) → proceed with the review.
Recovery semantics:
Step 3 — Auto-detect the second reviewer's lens
Inspect the PR's changed files:
gh pr diff $PR_NUM --repo stellar-experimental/henyey --name-only
If any path matches one of these prefixes, the PR is parity-critical — Reviewer B uses parity lens. These crates own the observable/interop surface defined in docs/PARITY.md:
crates/scp/
crates/herder/
crates/ledger/
crates/tx/
crates/overlay/
Otherwise, the PR is non-parity — Reviewer B uses risk lens.
Step 4 — Run 2 reviewer lenses (in-agent multi-lens)
Run both reviewer lenses (correctness + parity-or-risk) as two clearly-delimited passes within this agent. Under the current dispatch model you cannot spawn nested reviewer sub-agents, so the downstream contract is the structured comments + verdict markers, not the number of OS-level agents. A disciplined in-agent multi-lens pass satisfies every Step 6 parser as long as you hold to these four properties:
- Distinct framing per lens. Before each pass, adopt ONLY that lens's brief (Reviewer A / B below is the per-pass framing). Do not carry the previous lens's conclusions into the next.
- No cross-lens peeking. Form and post each lens's verdict as its own separate comment — two distinct
gh pr comment calls with the two distinct headers (## 🔍 Reviewer: Correctness and ## 🔍 Reviewer: <Parity|Risk>), never one merged block — before starting the next lens. The Step 6.1 parser keys on the header line and keeps the latest comment per reviewer name, so each lens MUST be its own comment.
- Explicit reset between lenses. When you move to the second lens, evaluate it as if you had not authored the first — re-read the diff fresh through the new lens.
- Refute as a genuine adversary (Step 6.1c). The refute pass argues the opposing case as if it must prove a different reviewer wrong — see Step 6.1c for the stance-switch wording.
The two distinct lenses are the primary missed-bug defense; the adversarial refute pass in Step 6.1c is what trims the false-positive bounces. (If a future nested-spawn capability lands, these same two briefs can be dispatched as parallel sub-agents instead — the per-lens framing and comment shapes are unchanged.)
Self-modifying detection (do the Step 5.5 path-match now, before the lens passes). Compute IS_SELF_MOD per Step 5.5 first. If the PR is self-modifying, apply the extra-scrutiny checklist from Step 5.5 in BOTH reviewer passes below (skill frontmatter/markdown parses, scripts bash -n / source cleanly, no dispatch-table/step-numbering breakage). A self-modifying PR is NOT gated — it still auto-merges on triple-green — but the reviewer passes must confirm it cannot break the orchestrator's or specialists' next run.
Review against current origin/main (do not review stale state). Each reviewer pass must git fetch origin and check out the PR head rebased onto / merged with the latest origin/main before reviewing or running anything — e.g. git fetch origin && git checkout pr-$PR_NUM && git merge --no-edit origin/main inside $REVIEWER_WORKTREE. Reviewing on a stale base is what caused historical merge-helper failures (the PR looked clean against an old main but conflicted or regressed against current main); always evaluate the PR as it will actually land.
Workspace binding (issue #2843 — applies to each reviewer pass). Before the lens passes, run the Workspace-contract bootstrap to derive $REVIEWER_WORKTREE (under ~/data/$SESSION_ID/review-pr-$ISSUE/reviewer). Use that exact path in each reviewer pass as the ONLY scratch location:
Your working directory and any checkout/build scratch MUST be $REVIEWER_WORKTREE (<resolved-path>). If you need a git worktree, run git worktree add "$REVIEWER_WORKTREE/<sub>"; if you build, set CARGO_TARGET_DIR="$REVIEWER_WORKTREE/cargo-target". You are FORBIDDEN from creating .review-data/, .review-worktrees/, .worktrees/, .copilot-tmp/, .opencode/worktrees/, any <repo>-pr<N> sibling, or any path under /tmp. These are the disk-leak patterns from #2843 and have repeatedly filled the root FS.
Why structured comments, not gh pr review --approve: the authenticated GH user is the PR author (the same user opened the PR via /do and now reviews it). GitHub disallows author self-approval, so gh pr review --approve is silently downgraded to a comment by gh. Instead, each reviewer posts a structured comment with a verdict marker that /review-pr parses in Step 6.
Verdict comment format — each reviewer MUST post exactly one comment with this exact shape:
## 🔍 Reviewer: <Correctness|Parity|Risk>
**Verdict:** APPROVE | CHANGES_REQUESTED
**Summary:** <one or two lines>
<details>
<summary>Full review</summary>
<bulleted list of concerns, inline references, alternate approaches.
For CHANGES_REQUESTED, list every specific change `/do` Mode B should make.
Keep under 400 lines.>
</details>
Post via gh pr comment $PR_NUM --repo stellar-experimental/henyey --body-file <tmpfile> so multi-line bodies survive intact. Reviewers MAY also post inline line comments via gh api repos/.../pulls/$PR_NUM/comments for specific concerns — those don't count toward the verdict; only the top-level structured comment does.
Inline-comment convention: if a reviewer's concern is non-blocking (would be a MINOR note), they should APPROVE at the top level AND leave the concern as an inline comment. /review-pr will auto-file a follow-up issue for every unaddressed inline comment at merge time (see Step 7.2), so non-critical feedback is preserved as actionable backlog without blocking the merge. If a concern is blocking, use **Verdict:** CHANGES_REQUESTED at the top level; /do Mode B will address every inline in that case.
Concern-class discipline (kills whack-a-mole bouncing): Every concern in a CHANGES_REQUESTED verdict MUST be labeled with a concern class — a 1–3 word category like test-coverage, workspace-contract, parity-gap, regression-risk, doc-drift, error-handling, api-shape, ci-failure. The reviewer prompts below enforce two rules:
- Cycle 1 (no prior
## 🔍 Reviewer: <YourName> comment on this PR): produce a COMPLETE change-list. Don't hold concerns back for later cycles. Every concern is labeled with its class. The implicit contract: a class not raised in cycle 1 should not be raised in cycle 2+.
- Cycle N≥2 (prior reviewer comments exist): you read your latest prior verdict, enumerate the classes you previously raised, and stick to them. Verify each prior concern is addressed; you may add new specific bullets within an existing class. If you genuinely identify a concern in a class you did NOT previously raise, you MUST flag the section with
**NEW CLASS DISCOVERED:** <class-name> and explain why cycle 1 missed it. This becomes audit-trail evidence that cycle 1 was incomplete and signals the orchestrator/operator that the PR may be larger in scope than was understood. (It still counts as a normal bounce — the goal is transparency, not blocking.)
Reviewer A — Correctness (always) / Reviewer B — Parity OR Risk (auto-detected)
Both reviewer briefs — the full verbatim prompt text for Reviewer A (Correctness, always) and Reviewer B (Parity if parity-critical, else Risk) — live in reference-reviewer-briefs.md. Consult that file when composing each lens's verdict comment; it is a relocation of the detailed briefs with zero information loss.
The load-bearing contract stays here and is unchanged:
- Reviewer A (Correctness, always): correctness of the diff, test coverage, readability, error handling; the cycle-awareness step (fetch your own prior
## 🔍 Reviewer: Correctness comments first — empty ⇒ cycle 1 COMPLETE class-labeled change-list, non-empty ⇒ cycle N≥2 stick to prior classes, flag any **NEW CLASS DISCOVERED:**); the test-verification gate by linked-issue kind: (bug-fix ⇒ regression test that provably fails at the pre-fix commit; feature ⇒ every new pub surface has a test; refactor/docs/test-only ⇒ existing tests still green in CI); and the visibility-narrowing / dead-code build-verify gate (green CI proves it compiles, but confirm the per-symbol (a) in-crate caller vs (b) dead classification was actually reasoned through — CHANGES_REQUESTED if the justification is a bare caller grep).
- Reviewer B (Parity if parity-critical, else Risk): parity lens focuses only on the observable/interop surface per
docs/PARITY.md (hashes, tx result/meta XDR, SCP/overlay wire bytes, history archive format, HTTP/RPC/CLI contracts, crypto outputs) — internal architecture/metrics/logging/perf are explicitly allowed and must not be flagged; the risk lens covers regressions, perf, API/data-format breaks, security, operational concerns. Same cycle-awareness discipline as Reviewer A, keyed on the reviewer's own header (Parity/Risk).
Each lens posts its verdict as its own structured gh pr comment using the exact header (## 🔍 Reviewer: Correctness and ## 🔍 Reviewer: <Parity|Risk>) and **Verdict:** APPROVE | CHANGES_REQUESTED shape defined above — never a single merged block (the Step 6.1 parser keys on the per-reviewer header).
Run both lens passes and post both verdict comments (each its own separate comment) before proceeding.
Step 5 — Recheck CI
The reviewer passes overlap with CI (CI runs on the PR head while you review). By the time both verdict comments are posted, CI may have finished. Re-query:
ROLLUP=$(gh pr view $PR_NUM --repo stellar-experimental/henyey \
--json statusCheckRollup --jq '.statusCheckRollup')
CI_TOTAL=$(echo "$ROLLUP" | jq 'length')
CI state buckets — apply in this order:
- Empty rollup (
CI_TOTAL == 0): the PR has NO CI runs at all. Could be a misconfigured workflow file, a fork PR with workflows gated, or a workflow_dispatch-only repo. NEVER classify this as green. Block with ## Review: No CI Detected and a note that the operator needs to investigate why CI didn't trigger.
- Red — at least one entry has
conclusion: FAILURE | CANCELLED | TIMED_OUT (or state: FAILURE | ERROR for StatusContext).
- Running — entries exist, none failed, but at least one is
status != COMPLETED (or for StatusContext: state == PENDING).
- Green —
CI_TOTAL > 0 AND every entry has conclusion: SUCCESS | SKIPPED | NEUTRAL (or state: SUCCESS for StatusContext). Requires positive evidence of completion, never vacuous.
if [ "$CI_TOTAL" -eq 0 ]; then
CI_STATE="empty"
elif [ "$(echo "$ROLLUP" | jq '[.[] | select(
((.conclusion // "") | ascii_upcase) as $c |
$c == "FAILURE" or $c == "CANCELLED" or $c == "TIMED_OUT"
or ((.state // "") | ascii_upcase) as $s | $s == "FAILURE" or $s == "ERROR"
)] | length')" -gt 0 ]; then
CI_STATE="red"
elif [ "$(echo "$ROLLUP" | jq '[.[] | select(
(.status != null and (.status | ascii_upcase) != "COMPLETED")
or (.status == null and (.state | ascii_upcase) == "PENDING")
)] | length')" -gt 0 ]; then
CI_STATE="running"
elif [ "$(echo "$ROLLUP" | jq '[.[] | select(
((.conclusion // "") | ascii_upcase) as $c |
$c == "SUCCESS" or $c == "SKIPPED" or $c == "NEUTRAL"
or ((.state // "") | ascii_upcase) == "SUCCESS"
)] | length')" -eq "$CI_TOTAL" ]; then
CI_STATE="green"
else
CI_STATE="red"
fi
Step 5.5 — Self-modifying gate (BEFORE the merge decision)
A PR that edits the project-management pipeline's own skills or scripts is higher-risk — a bad self-edit could break the next orchestrator run (the very machinery that would otherwise catch and revert it). Such a PR still merges autonomously on triple-green like any other PR, but the reviewers apply extra scrutiny to confirm the change cannot break the next run. Detect this before deciding the merge so the extra checks and flagging are applied.
Fetch the PR's changed file paths:
SELF_MOD_FILES=$(gh pr view $PR_NUM --repo stellar-experimental/henyey \
--json files --jq '.files[].path')
The PR is self-modifying if any changed path matches one of these patterns:
.claude/skills/{project-loop,project-tick,plan,do,review-pr,triage}/... (any file under those skill dirs)
scripts/lib/{review-pr-merge,review-pr-verdicts,agent-worktree-contract,pipeline-anomaly-log}.sh
.github/skills/shared/scripts/...
IS_SELF_MOD=$(echo "$SELF_MOD_FILES" | grep -Eq \
'^\.claude/skills/(project-loop|project-tick|plan|do|review-pr|triage)/|^scripts/lib/(review-pr-merge|review-pr-verdicts|agent-worktree-contract|pipeline-anomaly-log)\.sh$|^\.github/skills/shared/scripts/' \
&& echo "true" || echo "false")
If IS_SELF_MOD == true, the PR is flagged as self-modifying and the reviewers apply extra scrutiny — but it is not gated or escalated. It proceeds through the normal merge decision (Step 6) and auto-merges autonomously on triple-green, exactly like any other PR. Adversarial review + CI are the gate; there is no separate operator-approval hold.
When the PR is self-modifying, both reviewer passes (Step 4) must explicitly verify the change cannot break the orchestrator's or specialists' next run, in addition to their normal lens. Apply this checklist in each pass:
- Skill files still parse: every changed
.claude/skills/*/SKILL.md has valid YAML frontmatter (name / description / argument-hint) and is well-formed markdown — no truncated frontmatter, no broken fences.
- Scripts still source cleanly: every changed
*.sh passes bash -n (syntax check) and, for sourced libraries, bash -c 'source <file>' without error.
- No dispatch-table / step-numbering breakage: dispatch tables, state→skill routing, and step references (e.g. "see Step 5.5") are still internally consistent — the edit didn't orphan a step, renumber a referenced step, or break a routing entry the orchestrator relies on.
If this extra scrutiny passes and the PR is triple-green (both agent reviewers APPROVE post-refute + CI green), merge autonomously via attempt_merge in Step 7, the same as any PR.
Safety note: because a self-modifying change is higher-risk, if a reviewer has genuine doubt the change could break the live pipeline, that reviewer returns CHANGES_REQUESTED (a normal bounce back to ready-for-doing) — not an operator escalation. The bounce / refute / lifetime-cap machinery handles it like any other concern.
Record that the PR is self-modifying in the eventual verdict comment (so the flag is visible), then fall through to Step 6. Do not stop here.
**Self-modifying paths touched (extra reviewer scrutiny applied):**
- <each matched path>
Step 6 — Decide
6.1 Parse the reviewer verdicts from PR comments
Fetch all PR-level comments and find the latest one matching each reviewer header. Use the most recent comment per reviewer (in case a reviewer posted, then re-posted):
gh api repos/stellar-experimental/henyey/issues/$PR_NUM/comments \
--paginate --jq '.[] | select(.body | startswith("## 🔍 Reviewer:")) |
{created_at, body}' | jq -s 'sort_by(.created_at)'
For each comment, extract the reviewer name from the first line (## 🔍 Reviewer: Correctness etc.) and the verdict from the **Verdict:** line. Keep only the LATEST comment per reviewer name. The two reviewers we expect:
Correctness (always)
Parity or Risk (depending on parity-critical detection from Step 3)
If only one verdict is found, treat the missing one as CHANGES_REQUESTED and bounce. A missing verdict means a reviewer pass failed to produce its verdict comment — that's the same failure mode as the "Reviewer pass fails to produce a verdict comment" entry in the failure-handling table below. Do NOT wait indefinitely on a missing verdict; bounce so /do Mode B can retry, and the next /review-pr cycle will re-run fresh reviewer passes. If both are present, use them. If a reviewer posted twice (e.g. revised verdict), the latest comment wins.
6.1b Parse external reviewer verdicts (GH Copilot bot, humans, other bots)
The agent reviewers post structured PR comments (Step 4); but GitHub also tracks native PR reviews from anyone else — GH's Copilot auto-reviewer, human reviewers, third-party bots. Fetch them too and include in the matrix:
ME=$(gh api user --jq .login)
gh api repos/stellar-experimental/henyey/pulls/$PR_NUM/reviews \
--paginate --jq --arg me "$ME" '
[.[] | select(.user.login != $me)] |
group_by(.user.login) |
map(max_by(.submitted_at) | {user: .user.login, state, body_head: ((.body // "") | split("\n")[0])})
'
Each external reviewer's verdict is the state of their latest review:
APPROVED → external approve (nice-to-have, doesn't gate by itself)
CHANGES_REQUESTED → blocker, treated identically to an agent reviewer's CHANGES_REQUESTED
COMMENTED → neutral (notes; doesn't gate). The body still gets captured at merge time via the inline-comment follow-up logic in Step 7.
6.1c Adversarial refute pass (agent CHANGES_REQUESTED findings only)
This replaces the cross-model diversity the pipeline used to get from a second model, and directly cuts the false-positive bounces. Run it after the reviewers post and before applying the matrix.
Enumerate every distinct blocking finding from the agent reviewers' latest CHANGES_REQUESTED verdicts (one finding = one concern bullet from a reviewer's <details> change-list). For each, run an independent refute pass in-agent — one delimited pass per finding, per the Step 4 multi-lens discipline (distinct framing, explicit reset from the reviewer pass that raised it, each refute posted as its own separate comment). Switch stance and argue the opposing case as if you must prove a different reviewer wrong. Each refute pass first checks out the PR against current origin/main (same git fetch origin + merge instruction as Step 4) so it refutes on fresh state. Use this framing for each:
Reviewer <Correctness|Parity|Risk> raised this blocking finding (CHANGES_REQUESTED)
on PR #$PR_NUM in stellar-experimental/henyey:
Switch stance and REFUTE this finding as a genuine adversary — argue the
opposing case as if you must prove a different reviewer wrong. First
git fetch origin and check out the PR head merged with the latest
origin/main inside $REVIEWER_WORKTREE (do NOT review stale state). Then
argue, with evidence, that the finding is one of:
(a) wrong — the diff already handles this, or the reviewer misread the
code; (b) already-fixed on current origin/main — the change the reviewer
wants already exists once the PR is rebased (confirm by reading the merged
tree); or (c) non-blocking — even if true, it does not warrant bouncing
the PR and is at most an inline / follow-up-issue item. Be honest: if you
cannot refute it on any of the three grounds, say so. The finding only
stands if it cannot be refuted.
Post your finding as a comment headed ## 🥊 Refute — <reviewer lens>: <short finding summary> with an **Outcome:** REFUTED | STANDS line and a 2–4
bullet justification (cite files/lines for ground (b)).
After every refute pass has posted, read the outcomes. Then:
- A finding is dropped if its refute pass returned
REFUTED — it does NOT bounce the PR. Record it as refuted: <reason> in the eventual verdict comment.
- A finding stands if its refute pass returned
STANDS (or you could not produce an outcome after one retry — un-refuted findings stand, fail-safe toward the reviewer).
For the matrix below, an agent reviewer's verdict is treated as CHANGES_REQUESTED only if at least one of its findings survived refutation. If every finding from that reviewer was refuted, treat that reviewer's verdict as APPROVE for matrix purposes (the dropped findings are still recorded in the verdict comment).
External CHANGES_REQUESTED reviews (GH Copilot bot, humans, other bots) are NOT subject to the refute pass — they gate exactly as before (Step 6.1b). Only the agent reviewers' own findings are refutable.
6.2 Combine all signals
Required signals — both must be APPROVE (post-refute, per Step 6.1c) or the matrix bounces / waits:
- Reviewer A verdict (agent, Correctness): APPROVE / CHANGES_REQUESTED / pending.
- Reviewer B verdict (agent, Parity-or-Risk): APPROVE / CHANGES_REQUESTED / pending.
Additional gate — any external CHANGES_REQUESTED is a blocker:
- External reviewer states: union over all non-agent reviewers' latest states. If ANY is
CHANGES_REQUESTED → bounce as if an agent had CHANGES_REQUESTED. External APPROVED and COMMENTED are non-blocking.
CI state — empty / green / red / running (per the bucket rules in Step 5).
Apply the outcome matrix (top-to-bottom, first match wins):
Force-converge override (lifetime cap reached, CI green)
If Step 2 set FORCE_CONVERGE=1 (lifetime bounce cap was reached), the matrix is short-circuited:
| FORCE_CONVERGE | CI state | Action |
|---|
| 1 | green | Force-merge via Step 7-bis. File one follow-up issue per CHANGES_REQUESTED concern bullet from the latest reviewer comments; merge with --admin; post ## ⚠️ Force-Converged (Lifetime Cap). |
| 1 | running | Wait (## Review: Waiting on CI — force-converge still needs green CI). Re-pick next tick. |
| 1 | red / empty | Block. Post ## Review: Lifetime Cap + Unsafe CI (force-converge cannot merge red/missing CI; this requires operator). |
The reviewer-verdict outputs are still used (to produce the follow-up list), but they no longer gate the merge. This is intentional: at the lifetime cap, the reviewer↔doer loop has demonstrated it cannot converge, so we drain the PR with CI as the sole final safety net and preserve the unresolved concerns as backlog.
If FORCE_CONVERGE is unset, fall through to the normal matrix below.
Block immediately on suspicious CI state
| CI state | Action |
|---|
empty (zero rollup entries — no CI ever started) | Block. Post ## Review: No CI Detected explaining the situation; move to blocked. Operator investigates whether the workflow file is broken or whether this is a fork-PR / dispatch-only scenario. Never auto-merge without positive CI evidence. |
Auto-merge (triple-green)
| A | B | CI | Action |
|---|
| APPROVE | APPROVE | green | Auto-merge (see Step 7) |
Wait (re-pick next tick)
| A | B | CI | Action |
|---|
| APPROVE | APPROVE | running | Wait. Comment ## Review: Waiting on CI. Unassign so next tick re-picks. |
| (other waiting cases below) | | | |
Bounce (PR has issues to address)
| A | B | External | CI | Action |
|---|
| CHANGES_REQUESTED | (any) | (any) | (any) | Bounce — A has concerns. |
| (any) | CHANGES_REQUESTED | (any) | (any) | Bounce — B has concerns. |
| (any) | (any) | any CHANGES_REQUESTED | (any) | Bounce — external reviewer (bot or human) has concerns. |
| APPROVE | APPROVE | none CHANGES_REQUESTED | red (diff-attributable) | Bounce — CI is a reviewer too. |
For diff-attributable vs. unrelated CI red, inspect the failing check's logs:
gh run view <run-id> --log-failed
If failures reference code in the PR's diff → diff-attributable. If failures look upstream (e.g. a shared dependency, an unrelated test on main) → unrelated.
Unrelated CI red:
| A | B | CI | Action |
|---|
| APPROVE | APPROVE | red (unrelated) | Bounce with note. /do will rebase on origin/main and retry. If still red after rebase, the next /review-pr will mark blocked. |
Block (cycle cap or CI genuinely stuck)
| A | B | CI | Action |
|---|
| APPROVE | APPROVE | running > CI_STUCK_AFTER_MINUTES (default 60) wall-clock since the oldest in-progress check started | blocked — CI is genuinely stuck (see Step 6.0 for the wall-clock check). |
| (any) | (any) | (any) | If this would be the 4th bounce → blocked (handled at Step 2). |
Why wall-clock, not tick count: henyey integration tests routinely take 25+ minutes. A tick-count threshold (e.g. "3 ticks") is too aggressive when the tick rate is faster than CI completion — it produces false-positive blocks on perfectly healthy slow CI. Use the actual start time of the oldest still-in-progress CI check; only block if the run has exceeded a generous wall-clock budget.
Step 6.0 — Compute CI age before applying the matrix
If CI is in the running bucket (Step 5), determine its wall-clock age via the oldest in-progress check's startedAt:
HEAD_REF=$(gh pr view $PR_NUM --repo stellar-experimental/henyey --json headRefName --jq '.headRefName')
CI_OLDEST_START=$(gh run list --repo stellar-experimental/henyey \
--branch "$HEAD_REF" --limit 20 \
--json startedAt,conclusion,status \
--jq '[.[] | select(.status != "completed")] | min_by(.startedAt) | .startedAt // ""')
CI_AGE_MIN=0
if [ -n "$CI_OLDEST_START" ]; then
NOW_EPOCH=$(date -u +%s)
CI_START_EPOCH=$(date -u -d "$CI_OLDEST_START" +%s)
CI_AGE_MIN=$(( (NOW_EPOCH - CI_START_EPOCH) / 60 ))
fi
CI_STUCK_AFTER_MINUTES="${CI_STUCK_AFTER_MINUTES:-60}"
Apply to the matrix: if CI_AGE_MIN > CI_STUCK_AFTER_MINUTES AND both reviewers APPROVE AND CI is still running → block. Otherwise → wait (re-pick next tick).
Step 7 — Execute the decision
Auto-merge path
The pipeline gates merges on its OWN signals (parsed verdicts + CI state), not GitHub's review-approval count. The reviewer comments are advisory metadata only — GitHub does not "know" they're approvals, because GitHub disallows author self-approval. So we merge with --admin to bypass GitHub's review-required gate (CI gates still apply via branch protection if configured).
Before merging, file follow-up issues for any UNADDRESSED inline (line-level) review comments. Reviewers can flag non-critical concerns inline without blocking the merge with CHANGES_REQUESTED — those concerns must be preserved as actionable backlog, not lost.
7.1 Identify unaddressed inline comments
Fetch all inline-review threads via GraphQL (REST doesn't expose isResolved):
gh api graphql -f query='
query($owner: String!, $repo: String!, $pr: Int!) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
reviewThreads(first: 100) {
nodes {
isResolved
comments(first: 50) {
nodes {
databaseId
author { login }
body
path
line
url
}
}
}
}
}
}
}' -f owner=stellar-experimental -f repo=henyey -F pr=$PR_NUM \
--jq '.data.repository.pullRequest.reviewThreads.nodes'
For each thread, classify it as addressed if any of:
isResolved == true (GitHub-side thread resolution).
- The thread has at least one reply after the original comment AND the reply body contains any of:
Addressed in , Fixed in , Done in , or a <commit-sha> reference.
Otherwise, classify as unaddressed.
7.2 File one follow-up issue per unaddressed thread
For each unaddressed thread, create an issue:
gh issue create --repo stellar-experimental/henyey \
--title "<short summary derived from first line of comment body, ≤80 chars>" \
--body "$(cat <<EOF
Follow-up from PR #$PR_NUM (issue #$ISSUE). Non-critical inline review comment that was not addressed before merge.
## Concern
\`<path>:<line>\` — <full original comment body, indented or as-is>
## Source
[Original review comment](<thread.comments.nodes[0].url>) on PR #$PR_NUM.
## Severity
Low / non-blocking — reviewer chose to APPROVE rather than request changes. Filing as backlog so the concern isn't lost.
EOF
)" \
--label "enhancement,low,follow-up"
If the comment's path matches a known crate prefix (crates/<name>/...), also add the crate:<name> label.
Collect the list of newly-filed issue numbers — they'll be referenced in the merge comment.
7.3 Merge
Use the shared merge helper which handles the deferred-merge path. The helper
requires REVIEW_PR_SCRATCH_DIR to be set to a session-scoped directory under
the workspace contract (all scratch state under ~/data/$SESSION_ID/review-pr-$ISSUE/):
export REVIEW_PR_SCRATCH_DIR="${WORKTREE_BASE:-$(eval echo "~$(id -un)")/data/${SESSION_ID:-$(date +%Y%m%d-%H%M%S)}/review-pr-$ISSUE}"
mkdir -p "$REVIEW_PR_SCRATCH_DIR"
if ! MERGE_RESULT=$(attempt_merge $PR_NUM); then
MERGE_RESULT="hard-failure:attempt_merge returned nonzero"
fi
Handle the result:
-
merged → immediate merge succeeded. Proceed to Step 7.4 cleanup.
-
auto-merge-armed → GitHub cannot create the merge commit cleanly right now but will merge automatically once conditions are met. Post a waiting comment recording the follow-up issue numbers filed in Step 7.2:
## Review: Auto-merge armed
Enabled deferred auto-merge (squash). GitHub will merge once branch protection checks pass.
**Follow-up issues filed:** #N1, #N2 (or "none")
Re-picking on next tick to verify merge completion.
Unassign and exit. The next /review-pr tick will detect the MERGED state via classify_linked_pr_state and run cleanup, or detect OPEN + armed via is_auto_merge_armed and short-circuit to waiting.
-
hard-failure:<stderr> → merge failed for a reason other than the auto-merge hint. Leave the issue in in-review; file a follow-up issue documenting the merge-permission gap; do NOT downgrade to non-admin merge that might silently bypass CI. Post ## Review: Merge Failed with the stderr content.
7.4 Clean up
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE done
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
REPO_ROOT="$(git rev-parse --show-toplevel)"
PW_HOME="$(getent passwd "$(id -un)" 2>/dev/null | cut -d: -f6)"
[ -z "$PW_HOME" ] && PW_HOME="$(eval echo "~$(id -un)")"
reap_workspace() {
local ws="$1"
[ -z "$ws" ] && return 0
local canon home_data rp
for rp in /usr/bin/realpath /bin/realpath /usr/local/bin/realpath; do
[ -x "$rp" ] || continue
canon="$("$rp" -m "$ws" 2>/dev/null)" || canon=""
home_data="$("$rp" -m "$PW_HOME/data" 2>/dev/null)" || home_data=""
[ -n "$canon" ] && [ -n "$home_data" ] && break
done
if [ -z "$canon" ] || [ -z "$home_data" ]; then
local py
for py in /usr/bin/python3 /usr/local/bin/python3 /bin/python3; do
[ -x "$py" ] || continue
canon="$("$py" -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$ws" 2>/dev/null)"
home_data="$("$py" -c 'import os,sys; print(os.path.realpath(sys.argv[1]))' "$PW_HOME/data" 2>/dev/null)"
break
done
fi
if [ -z "$canon" ] || [ -z "$home_data" ]; then
echo "WARN: refusing to reap '$ws' — could not canonicalize for boundary check" >&2
return 0
fi
case "$canon" in
"$home_data"/*) [ -d "$canon" ] && rm -rf -- "$canon" ;;
*) echo "WARN: refusing to reap '$ws' — resolves to '$canon', not strictly under '$home_data/' (the bare data root and any path outside it are both rejected)" >&2 ;;
esac
}
WORKSPACE_MARKER="$PW_HOME/data/do-$ISSUE.workspace"
if [ -f "$WORKSPACE_MARKER" ]; then
reap_workspace "$(cat "$WORKSPACE_MARKER")"
rm -f "$WORKSPACE_MARKER"
elif [ -f "$REPO_ROOT/data/do-$ISSUE/.session-id" ]; then
SESSION_ID=$(cat "$REPO_ROOT/data/do-$ISSUE/.session-id")
[ -n "$SESSION_ID" ] && reap_workspace "$PW_HOME/data/$SESSION_ID/do-$ISSUE"
fi
rm -rf "$REPO_ROOT/data/do-$ISSUE"
git worktree prune
Post a ## ✅ Merged comment with the merge commit SHA AND the list of follow-up issues filed:
## ✅ Merged
**Commit:** <merge-commit-sha>
**Follow-up issues filed for unaddressed inline review comments:** #N1, #N2
(none if all inline comments were addressed)
Exit.
Step 7-bis — Force-converge merge (lifetime cap, CI green)
Only entered if Step 6's force-converge override fired (lifetime bounce cap reached AND CI green). The flow is parallel to Step 7 but takes its follow-up source from the top-level reviewer CHANGES_REQUESTED concern bullets (in addition to unaddressed inline comments). In brief: (7-bis.1) collect the unresolved CHANGES_REQUESTED concern bullets from the latest reviewer comments plus unaddressed inline comments; (7-bis.2) file one follow-up,force-converge-labeled issue per concern bullet; (7-bis.3) merge with gh pr merge --squash --admin (on --admin failure, file the follow-ups but leave the PR open and post ## Review: Force-Converge Permission Gap — never degrade to a non-admin merge); (7-bis.4) run the Step 7.4 cleanup and post the ## ⚠️ Force-Converged (Lifetime Cap) comment listing the follow-up issues by class, then exit.
See reference-force-converge.md for the full worked procedure — the verbatim follow-up issue-body template, the merge command, and the complete ## ⚠️ Force-Converged (Lifetime Cap) comment template. Nothing was dropped; the reference file holds the exact text.
Wait path
Post:
## Review: Waiting on CI
CI is still running (checks: <names>). Re-picking this PR on the next tick.
CI age: <CI_AGE_MIN> min / budget <CI_STUCK_AFTER_MINUTES> min.
Bounce-back count: <N>/3.
Unassign yourself so the next tick re-picks this issue. Exit.
Bounce path
Post:
## Review: Bounce-Back Cycle <N+1>
**Reason:** <"Reviewer A requested changes" | "Reviewer B requested changes" | "<external-user> requested changes" | "CI failed (diff-attributable)" | "CI failed (unrelated, will rebase)">
**Reviewer A:** APPROVE | CHANGES_REQUESTED — <one-line summary>
**Reviewer B:** APPROVE | CHANGES_REQUESTED — <one-line summary>
**External reviewers:** <list "user: STATE" for each non-agent reviewer with a recent review; omit the section if none>
**CI:** green | red | running
<If an external reviewer's CHANGES_REQUESTED triggered the bounce, paste the relevant body excerpt and a link to the review so /do Mode B can address it the same way it addresses agent feedback.>
<If CI red, paste the relevant failed-check excerpt via gh run view --log-failed>
Routing back to `ready-for-doing` for `/do` Mode B.
Move state and unassign:
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE ready-for-doing
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
Exit.
Block path
Post one of these (whichever cap tripped):
## Review: Cycle Cap Reached / CI Stuck
**Bounce count (head-scoped):** 3
**Status:** blocked
**Pattern:** <summary of the disagreement: which reviewer kept failing, what
they kept asking for, what /do kept doing. This is what humans need to break
the tie.>
This PR has cycled 3 times on the current code state without converging. Human review required.
## Review: Lifetime Cap + Unsafe CI
**Lifetime bounce count:** <LIFETIME_COUNT>
**CI state at cap:** red | empty | running-too-long
**Status:** blocked
The lifetime bounce cap was reached, but CI is not currently green so force-converge cannot proceed. Force-converge requires green CI as its sole final safety net; without it the pipeline cannot guarantee the merge is safe.
**Pattern:** <summary across the lifetime of this PR: what concerns kept recurring
across multiple /do pushes. This is the runaway-loop pattern — /do produces a
fix, /review-pr finds different (or same) issues, /do tries again, indefinitely.
This is what humans need to break the underlying disagreement.>
This PR has cycled 6+ times across multiple code states without converging. The head-scoped cap kept resetting on each fresh push but the underlying issue never resolved. Human review required to either:
- Approve manually (if reviewer concerns are over-strict),
- Reframe the issue / plan (if /do is missing context),
- Or close the PR and re-plan.
To retry after addressing root cause, post `## Review: Reset` with a one-line reason.
Move state:
bash .github/skills/shared/scripts/move-issue-status.sh $ISSUE blocked
gh issue edit $ISSUE --repo stellar-experimental/henyey --remove-assignee @me
Exit.
What you do NOT do
- Do not post a single combined review. Run two reviewer passes that each post their own structured comment (each its own
gh pr comment with its own header), then combine and parse those comments — do not merge the two lenses into one block (the Step 6.1 parser keys on the per-reviewer header).
- Do not override or summarize the reviewers' verdicts. Their
**Verdict:** line is the verdict. You read it; you don't rewrite it — except that an agent reviewer's CHANGES_REQUESTED finding is dropped if the Step 6.1c refute pass returns REFUTED (recorded as refuted: <reason>).
- Do not run the refute pass on external CHANGES_REQUESTED reviews (GH Copilot bot, humans, other bots). Only the agent reviewers' own findings are refutable; external reviews gate as-is.
- Do flag a self-modifying PR (touches the pipeline's own skills/scripts — see Step 5.5) and apply extra reviewer scrutiny, but do not hold it for operator approval — it auto-merges autonomously on triple-green like any other PR. If a reviewer genuinely doubts the change is safe for the live pipeline, that's a CHANGES_REQUESTED bounce, not an operator escalation.
- Do not merge if any of the three signals is not green. The matrix is the rule. (Exception: the force-converge override at the lifetime cap merges on green CI alone — see Step 7-bis. Reviewer verdicts become follow-up issues rather than merge gates.)
- Do not wait synchronously on long-running CI. If CI is
running, unassign and exit — the next tick re-picks the issue.
- Do not use
gh pr review --approve — GH silently downgrades it to a comment because the agent is the PR author. Use structured PR comments via gh pr comment instead.
Failure handling
| Failure | Action |
|---|
| Reviewer pass fails to produce a verdict comment | Retry that pass once. If still failing, treat as CHANGES_REQUESTED and bounce. |
| Refute pass fails to produce an outcome comment (Step 6.1c) | Retry that pass once. If still failing, the finding stands (fail-safe toward the reviewer) — it gates the matrix as a surviving CHANGES_REQUESTED. |
| Reviewer's comment doesn't match the expected header/verdict shape | Treat as pending; if it stays malformed after Step 4 completes, bounce with a ## Review: Malformed Verdict note. |
| No PR linked | Bounce to ready-for-doing with ## Review: No PR Linked. |
gh pr merge --admin fails with auto-merge hint | Retry with --auto (deferred merge). If --auto also fails, hard failure. See Step 7.3. |
gh pr merge --admin fails (other error, e.g. token lacks admin) | Leave the issue in in-review; file a follow-up issue documenting the gap; do NOT degrade to a non-admin merge that might bypass CI gates. |
| GH API failure | Retry once after 5s; if still failing, leave assigned and exit non-zero. |
Workspace contract
The /review-pr skill must never create worktrees outside ~/data (the real home
directory's data/ subdirectory, derived from the passwd entry — immune to $HOME
poisoning). All scratch state lives under ~/data/$SESSION_ID/review-pr-$ISSUE/.
Reviewers derive their workspace by sourcing the shared contract helper:
REPO_ROOT="$(git rev-parse --show-toplevel)"
source "$REPO_ROOT/scripts/lib/agent-worktree-contract.sh"
review_pr_bootstrap "$ISSUE" || exit 1
mkdir -p "$REVIEWER_WORKTREE"
This ensures build artifacts are isolated per-session and automatically discoverable
for cleanup. The skill itself is read-only (no code changes), so it rarely needs a
build cache — but if a reviewer pass builds to verify, the target dir must
resolve under ~/data and within the session prefix.
Never create worktrees at the repo root or outside ~/data. All worktrees
must be under ~/data/$SESSION_ID/ to avoid polluting the shared checkout.
Forbidden scratch patterns (issue #2843 — hard requirement). Every reviewer
pass (and any /review, /spec-adhere, /review-fix it invokes) is bound
to $REVIEWER_WORKTREE and must NEVER create any of these observed disk-leak
patterns — not in the repo tree, not as a <repo>-pr<N> sibling, not under /tmp:
.review-data/
.review-worktrees/
.worktrees/
.copilot-tmp/
.opencode/worktrees/
- any path under
/tmp (e.g. /tmp/pr<N>-review)
These are the exact paths prior review cycles leaked (an 11 GB /tmp/pr2797-review
near-OOM'd the root FS). On every exit path — including the Step 7 cleanup of
WORKTREE_BASE — assert the repo tree is clean:
assert_no_repo_tree_scratch "$REPO_ROOT" || {
echo "Reviewer scratch leaked into the repo tree or a sibling — clean it now." >&2
}
Branch protection
Because the pipeline gates merges on its own parsed verdicts (not GH-recognized approvals), main branch protection should NOT require pull-request approvals — that gate is impossible to satisfy when the agent is the PR author. Recommended branch protection:
- Required: all CI checks green (the names of every check in
.github/workflows/ci.yml).
- Required: branch up-to-date with base.
- Required approvals: 0 — the pipeline's own reviewer verdicts gate merge.
- The agent's token must have
Pull requests: Read and write and the user must be a repo admin for --admin merges to succeed.
The pdr-managed label on every pipeline-created PR distinguishes them from human PRs if you want to scope additional automation to managed ones only.