docs: reconcile merge-gate + front-gate to standing auto-merge

[yihanzhu]

Closes #43

What

Reconcile the merge-gate and front-gate docs with the actual operating policy: the user granted standing auto-merge (Faber merges PRs that are CI-green + Codex-clean and low-risk, without per-PR confirmation), but several docs still said the opposite. This removes every "no auto-merge" / "you merge" / "never merges" contradiction across the repo while preserving every other rail.

Why

The policy had outrun the docs. README.md design-decisions and manager/CLAUDE.md "Never" still asserted manual-merge-only, contradicting how the team actually runs.

Changed per file

• README.md — design-decision "No auto-merge in Phase 1. Faber pings; you merge." → Faber auto-merges clean, low-risk PRs (CI green + Codex clean), human only for needs-human/round-cap escalations, safety-rail changes, and north-star milestones / goal drift. Loop diagram and Rollout phases made consistent (this is the earned auto-merge).
• manager/CLAUDE.md — "Never merge. Merging is mine." → Merge clean PRs per standing authorization; do NOT merge when human review is required (safety-rail changes, ambiguous specs, escalations, north-star/goal drift). Kept "never write code / open PRs" and "never self-approve" intact. Updated the loop pass-step and the Tracking bullet.
• templates/faber-command.md — same reconciliation for the live /faber prompt.
• Consistency sweep (acceptance criteria Add Claude Code GitHub Workflow #3/docs: add RESTORE.md disaster-recovery runbook #4): QUICKSTART.md, RESTORE.md (intro, end-to-end smoke test, safety-rails list, branch-protection note), reviewer/codex-review.md (flow diagram), routines/brief.md, templates/repo-setup.md — removed leftover contradictions. The branch-protection notes now distinguish GitHub's native auto-merge button (kept off; merges run through Faber/human, both gated on green CI) from Faber's deliberate gh pr merge.
• CLAUDE.md self-modification-safety rail list — the "no auto-merge" rail reworded to its surviving form (merge stays gated + human-review carve-outs). See self-mod note below.

Rails preserved (only auto-merge-of-clean-PRs loosens)

• Reviewer stays read-only / comments-only; Codex never approves or merges.
• CI is the hard gate.
• Rounds cap (~3) + needs-human escalation intact.
• Never self-approve; never write code / open PRs.
• One coder-launch per issue.
• Front-gate wording (Faber records ready after the user's explicit approval; never self-approves) confirmed consistent across manager/CLAUDE.md, README.md, templates/faber-command.md (and QUICKSTART.md / RESTORE.md).

⚠️ Self-modification call-out (required)

This is a deliberate, human-authorized change to the "no auto-merge" safety rail — the user granted standing auto-merge, so this PR loosens exactly that one rail (and reworded it in CLAUDE.md's self-mod rail list accordingly). Nothing else loosens.

Prompt sync: this changes prompt/doc sources, which do not alter live behavior until synced. The live /faber command updates only when the user re-runs scripts/install.sh; manager/CLAUDE.md takes effect on Faber's next read.

Tests

Docs/prompt-only — no .sh touched. CI structure check passes locally (structure ok); shellcheck unaffected.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The prompt changes introduce unsafe/ambiguous merge behavior: the final manager instructions can auto-merge high-risk work and the tracking path can strand low-risk PRs after CI turns green.

Full review comments:

• [P1] Keep high-risk PRs out of auto-merge — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xK07kIFvuc/manager/CLAUDE.md:66-70
  For high-risk PRs such as auth, migrations, or shared repos, this final merge rule says to merge any CI-green/Codex-clean PR and then lists human-review carve-outs without high-risk. Because this is the manager prompt's last merge instruction, it can override the earlier high-risk carve-out and lead Faber to auto-merge changes the safety rails say should be brought to the user.

• [P2] Surface low-risk merge-ready PRs after CI completes — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xK07kIFvuc/manager/CLAUDE.md:56-57
  When Codex passes before CI finishes, Faber will have applied merge-ready but cannot merge until checks turn green. This tracking rule now filters merge-ready + green PRs down to ones already held for the human, so a low-risk PR whose CI turns green after the loop ends is neither reported nor auto-merged on the next status scan.

[yihanzhu]

Round-1: both Codex findings addressed in 4803e2a (docs/prompt-only).

[P1] High-risk out of auto-merge. Moved the high-risk exclusion inside the FINAL merge instruction in each file so the last word on merging always carves it out (can't be overridden by an earlier note):

• manager/CLAUDE.md — "Merge clean PRs": merge yourself "unless it is high-risk" → human merge gate even when CI-green + Codex-clean (auth, DB/schema migrations, shared/production repos, security-sensitive / operator-judgment).
• templates/faber-command.md — the closing "you do merge" sentence now states high-risk always goes to the user's merge gate; "last word on merging."
• README.md — the "Faber auto-merges" design decision (and the loop diagram) now route high-risk to YOU explicitly.

[P2] Don't strand merge-ready PRs after CI turns green. Tracking in manager/CLAUDE.md now auto-merges the low-risk merge-ready PRs that are now CI-green on the next status scan (CI may go green after the loop ends) and lists the high-risk / human-held ones for the user. The read-only routines/brief.md query mirrors this — flags low-risk ones as ready-to-merge, lists held ones.

Everything else from the PR (auto-merge reconciliation, front-gate consistency) and all rails intact. Structure check + shellcheck pass locally. Label bumped round-0 → round-1. Not merging / not approving.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The patch changes the workflow to auto-merge low-risk clean PRs, but one of the surfaced routines still defers those PRs instead of merging them, leaving the documented Phase 2 path inconsistent.

Review comment:

• [P2] Make the brief actually merge eligible PRs — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.vRo0p39B7r/routines/brief.md:11-12
  When the Phase 2 brief is used after CI turns green, these instructions tell Faber only to flag low-risk merge-ready PRs for a future status scan instead of merging them. That conflicts with the new manager/README contract that the status/brief scan auto-merges CI-green, Codex-clean low-risk PRs, so eligible PRs can be left pending indefinitely unless the user asks for a separate status pass. Update the brief to perform the same auto-merge action, or stop documenting the brief as the auto-merge path.

[yihanzhu]

Round-2: Codex [P2] addressed in 6b0c237 (docs/prompt-only).

[P2] brief read-only vs. Tracking auto-merge — reconciled. The brief surfaces state; Faber's status/Tracking pass is the actor. No file both calls the brief read-only and says it merges.

• routines/brief.md — now explicit: this report is read-only — it surfaces state, it does not merge. Low-risk merge-ready + CI-green PRs are reported as auto-merge-eligible (Faber merges them on a status/Tracking pass — picked up there, not in the brief); high-risk / human-held ones are listed for you.
• manager/CLAUDE.md — Tracking section now names the status/Tracking pass as the auto-merge actor, contrasted with the read-only routines/brief.md that only surfaces the same state. (Unchanged: it auto-merges low-risk merge-ready + green PRs on the scan so they don't strand.)
• README.md (Phase 2) — decoupled: the brief resurfaces (read-only, never merges); on a status / Tracking pass Faber auto-merges clean low-risk PRs.
• templates/faber-command.md — already attributes merge to standing authorization in the round loop and never references the brief as a merge path; left as-is (no brief-merge wording to remove).

High-risk carve-out intact (high-risk always to your merge gate, even CI-green + Codex-clean — the last word on merging). All rails unchanged: reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-self-approve, never-write-code/open-PRs.

Structure check + shellcheck pass locally. Label bumped round-1 → round-2. Not merging / not approving.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The auto-merge instructions can merge a changed PR head that has not been reviewed, and the documentation now contains contradictory merge-authority instructions. These issues should be fixed before relying on the new behavior.

Full review comments:

• [P1] Require fresh review for the merged head — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.vyFzbOK3oT/manager/CLAUDE.md:58-59
  When this status pass auto-merges any merge-ready PR whose CI later turns green, it relies on a persistent label that GitHub leaves in place if the PR head changes after Codex passed it. In that case a coder or contributor can push another commit, CI can go green, and Faber will merge code that was never reviewed by Codex; the auto-merge path should verify the last Codex review/comment was for the current head SHA, or rerun codex-review.sh/clear merge-ready, before merging.

• [P2] Reconcile the remaining never-merge source text — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.vyFzbOK3oT/README.md:68-68
  This new README rule conflicts with the agent table earlier in the same source-of-truth file, which still says Faber's writes are issues only, never code/merge. Because /faber instructs the manager to read the README before operating, a fresh install now gives Faber opposing instructions about whether it may merge clean PRs, so the remaining never merge wording should be updated or scoped to code/PR authoring.

[yihanzhu]

Round-3: both Codex findings addressed in f405905 (docs/prompt-only).

[P1] Require fresh review for the merged head. merge-ready now means "the CURRENT head SHA passed Codex review," and a new push voids it (GitHub keeps the label across head changes):

• manager/CLAUDE.md — the pass path now defines merge-ready as current-head-bound and adds: "merge-ready is void the moment new commits land — clear it on any head change; only (re)applied after a passing Codex review of the new current head; never merge a label whose review predates the current head." The Tracking auto-merge rule now requires confirming the latest Codex review covered the current head SHA before auto-merging — if the head changed, clear merge-ready, re-run codex-review.sh on the new head, and only re-apply on a passing review of that head. The Merge & never rule states the merge authorization holds only for a review of the current head.
• templates/faber-command.md — round-loop pass line + closing merge sentence now carry the same current-head condition and void-on-new-commits rule.
• routines/brief.md — a merge-ready PR whose head changed is surfaced as stale / needs a fresh current-head review, not auto-merge-eligible (keeps the read-only brief consistent with the actor).

[P2] Reconcile the remaining never-merge text. README.md agent table cell scoped to authoring: "issues only; never authors code/PRs (merges clean low-risk PRs)." Swept the repo for other "never merge" wording — the only remaining hits are correct rails (reviewer never merges; brief never merges; Faber never writes code/opens PRs).

High-risk carve-out intact (high-risk always to your merge gate, even CI-green + Codex-clean — last word on merging). All rails unchanged: reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-self-approve, never-write-code/open-PRs.

Structure check + shellcheck pass locally. Label bumped round-2 → round-3. Not merging / not approving.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The new auto-merge instructions need an atomic current-head guard to uphold the stated safety invariant, and the canonical label state remains out of sync with the changed semantics.

Full review comments:

• [P2] Guard auto-merges with the reviewed head SHA — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.qZh8htKtvh/manager/CLAUDE.md:80-84
  When Faber automates the merge, there is a race between checking that CI/Codex covered the current head and actually running the merge: a contributor can push a new commit in that window. In repos without branch protection enforcement (which the docs allow for free private repos), that can merge a head Codex never reviewed despite the new invariant. Please require an atomic head match for the merge, e.g. instruct Faber to use gh pr merge --match-head-commit <reviewed-sha> or an equivalent guard.

• [P3] Align the canonical merge-ready label description — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.qZh8htKtvh/templates/repo-setup.md:34-36
  This new merge path says Faber may merge low-risk PRs, but the canonical label bootstrap still creates merge-ready as Codex review passed; awaiting your merge, and the checklist above still documents that old meaning. New target repos and doctor --check will preserve/reconcile the stale description, which contradicts the label's new state semantics for operators inspecting labels. Update the script/checklist label definition alongside this new Faber merge guidance.

[yihanzhu]

Faber — round cap + safety-rail change → human checkpoint

CI green throughout. This PR loosens the no-auto-merge rail, and Codex has adversarially hardened it over 4 review cycles — each round a new, more-subtle safety nuance, all valid:

• round-0: auto-merge omitted the high-risk carve-out (auth/migrations/shared) → fixed (high-risk always to the human; last word on merging). + tracking stranding → fixed.
• round-1: the brief both "read-only" and "merges" → fixed (brief read-only; Faber's status pass merges).
• round-2: sticky merge-ready could auto-merge an unreviewed head after a new commit → fixed (merge-ready bound to current head; void on new commits; re-review first). + README table "never merge" → fixed.
• round-3 (now): [P2] TOCTOU race — a push between the head-check and the merge could still slip an unreviewed head in an unprotected repo. Clean definitive fix: atomic gh pr merge --match-head-commit <reviewed-sha>. + [P3] canonical merge-ready label description still says "awaiting your merge" (stale vs the new auto-merge semantics).

Both remaining fixes are concrete and bounded (the atomic --match-head-commit guard closes the race class definitively). Escalating per the cap + because this is a safety-rail change — not because it's stuck.

[yihanzhu]

Round-4: both Codex findings addressed in 76a57d5 (docs/prompt + label-text only).

[P2] Atomic head-match guard for auto-merge. The auto-merge rule now pins the merge to the reviewed SHA — gh pr merge <PR#> --squash --match-head-commit <reviewed-sha> (the head when merge-ready was applied / the head Codex actually reviewed). If a new commit lands between the head-check and the merge, the merge fails atomically instead of merging an unreviewed head — closing the TOCTOU race even in repos without branch-protection enforcement. Encoded in:

• manager/CLAUDE.md — the Tracking auto-merge rule and the Merge & never rule both now require the --match-head-commit <reviewed-sha> pin.
• templates/faber-command.md — the round-loop pass line and the closing "you do merge" sentence both carry the atomic pin.
• README.md — the "Faber auto-merges" design decision states the merge is pinned to the reviewed head and fails atomically on a racing commit.

[P3] Sync the canonical merge-ready label description. Updated from the stale "Codex review passed; awaiting your merge" to "Current head passed Codex review; auto-merged if low-risk, else awaiting your merge" in:

• scripts/setup-target-repo.sh (the canonical label bootstrap — doctor/--check reconcile to this now).
• templates/repo-setup.md (the documented label list).

RESTORE.md references merge-ready only as part of the label-set name (no inline description), so no change needed there.

All rails intact: high-risk always goes to the human merge gate even when CI-green + Codex-clean (last word on merging); reviewer stays read-only / comments-only; current-head invariant (merge-ready void on new commits) preserved and now reinforced by the atomic pin. No earlier-round regressions.

Structure check + shellcheck (-S style, all scripts) pass locally; scripts/setup-target-repo.sh stays +x. Not merging / not approving / not touching labels.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The new auto-merge workflow lacks a durable way to identify the exact reviewed head and uses an unscoped merge command in a cross-repo workflow. These are safety/correctness issues that should be fixed before relying on the new behavior.

Full review comments:

• [P1] Capture the actual reviewed head before merging — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.BJUbVwybI5/manager/CLAUDE.md:71-72
  When a commit lands while codex-review.sh is running, the head SHA at the time merge-ready is applied can differ from the SHA Codex actually fetched and reviewed. Since the review comment/script does not durably record pr_head, a later status pass can end up passing the current unreviewed head to --match-head-commit, which defeats the safety rail this line is trying to add. Record the reviewed SHA in the review output/label workflow and compare it to the current PR head before labeling or merging.

• [P2] Qualify auto-merge commands with the repo — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.BJUbVwybI5/manager/CLAUDE.md:71-71
  During the Tracking pass this prompt scans PRs across multiple repos, but gh pr merge <PR#> is resolved from the current checkout or GH_REPO, and PR numbers are repo-local. If Faber is running from the control-plane repo or another target repo, this will fail to merge the intended PR or could target the wrong same-number PR; include --repo <owner>/<repo> or require changing into the target clone before every auto-merge.

[yihanzhu]

Faber — stopping; recommend scoping down (5th cycle, safety rail)

Round-4's atomic --match-head-commit guard surfaced two more (both valid): [P1] the guard needs the actual reviewed SHA, which codex-review.sh doesn't durably record → a later status pass can't trust it; [P2] cross-repo gh pr merge <PR#> needs --repo.

Root insight: every one of these (stale-head, TOCTOU, durable-SHA, cross-repo) lives in the unattended "auto-merge on a later status scan across repos" path — a mechanism we don't actually have yet. The in-session auto-merge is safe and simple: Faber reviews a PR and merges it back-to-back, in the target repo, pinned to the head it just reviewed (no concurrent pusher, no cross-repo ambiguity).

Recommendation: scope this PR's auto-merge to the in-session case only (Faber auto-merges a clean, low-risk PR it just reviewed, in the current target repo, --match-head-commit = the head it reviewed). Defer the autonomous status-scan / cross-repo / durable-reviewed-SHA auto-merge to a dedicated follow-up issue (it's a real mechanism: have codex-review.sh record the reviewed SHA, guard the merge on it, --repo-qualify). That lands #45 clean now and tracks the hard part honestly, instead of shipping doc promises the mechanism can't keep.

[yihanzhu]

Scoped auto-merge to the in-session case only per the #43 decision; deferred the unattended path to #46. Pushed 473123d.

Auto-merge is now in-session-only — Faber merges a PR only when it just reviewed that exact head back-to-back (review→merge), pinned with gh pr merge <PR#> --squash --match-head-commit <reviewed-sha> (fails atomically if a commit slips in). The status/Tracking pass and the brief are read-only — they only surface merge-ready PRs, never merge on a later scan.

Per file:

• manager/CLAUDE.md — pass-step merge made in-session/back-to-back with the reviewed-SHA pin moved into it; the Tracking pass changed from "the auto-merge actor" to read-only (reports only); "Merge clean PRs" → "Merge clean PRs in-session" with the Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 deferral note.
• templates/faber-command.md — pass-step and the "you do merge" summary scoped to in-session-only; status scan never auto-merges; Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 referenced.
• README.md — design-decision reworded to "in-session only" + Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 note; Phase 2 status-pass/brief now read-only; Phase 3 names the deferred Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 work; loop diagram annotated in-session.
• routines/brief.md — low-risk PRs reported as "awaiting merge" (in-session review or your merge); dropped the "auto-merge-eligible on a status pass" handoff.
• setup-target-repo.sh + templates/repo-setup.md — merge-ready description → "auto-merged in-session if low-risk, else awaiting your merge".

Rails intact: reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-self-approve, never-write-code/open-PRs, high-risk carve-out (human last word), current-head invariant, read-only brief.

Verify: shellcheck clean (incl. -S style as CI runs it); structure check passes (structure ok). Re-read confirms the only auto-merge path is in-session (review→merge, --match-head-commit-pinned), the status pass + brief only report, #46 is referenced for the deferred unattended path, and no contradictions remain.

Self-mod note: prompt/doc-only — no live behavior change until you re-run scripts/install.sh (/faber) and Faber re-reads manager/CLAUDE.md.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The patch introduces an auto-merge workflow, but the documented merge command is not safely bound to the reviewed repository or to a reliably captured reviewed commit SHA. These gaps can cause the new safety checks to merge the wrong PR or an unreviewed head.

Full review comments:

• [P1] Qualify the auto-merge repository — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ktNfo0saJ9/manager/CLAUDE.md:38-38
  When GH_REPO is set in the operator's environment, this bare gh pr merge <PR#> can target that repository instead of the target repo that codex-review.sh just reviewed; the review harness explicitly unsets GH_REPO and passes --repo to avoid the same footgun, but the new merge path does not. In that scenario Faber could merge PR number N in a different repo after reviewing the current repo, so the merge instruction should derive the repo from cwd and pass --repo (and/or unset GH_REPO) for the merge command too.

• [P1] Record the exact reviewed head SHA — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ktNfo0saJ9/manager/CLAUDE.md:39-39
  The safety of --match-head-commit depends on <reviewed-sha> being the commit fetched and reviewed by codex-review.sh, but the harness currently only posts the review and does not surface that pr_head value. If a contributor pushes after Codex finishes and Faber fills this placeholder from the current PR state, the atomic check would be pinned to the new unreviewed head and would no longer prevent merging it; the review step should emit/record the fetched SHA, or the prompt should require a safe capture/verification sequence.

[yihanzhu]

Round-5: both Codex [P1]s addressed in 10581df (docs/prompt-only).

[P1] Trustworthy reviewed SHA via a safe capture sequence. The in-session auto-merge is now an ordered, safe sequence instead of a fillable <reviewed-sha> placeholder:

1. Capture the head BEFORE review — head="$(gh pr view <PR#> --repo <repo> --json headRefOid -q .headRefOid)" (so the pin can't be back-filled from post-review/current state).
2. Review that head — scripts/codex-review.sh <PR#>.
3. On a clean review + low-risk + CI green, merge pinned to the captured head — gh pr merge <PR#> --repo <repo> --squash --match-head-commit "$head".
4. If the merge refuses (head moved before/during/after review), do not force it: merge-ready is void → re-capture + re-review the new head. Never fill --match-head-commit from post-review/current state to "make it pass."

This guarantees Faber only merges the exact head it reviewed; if anything changed, the merge safely refuses.

[P1] Repo-qualify the merge. Mirrors codex-review.sh: GH_REPO unset + explicit --repo <repo> (derived from the cwd's remote) on the merge, so a stray GH_REPO can't redirect the merge to the wrong repo.

Encoded identically in manager/CLAUDE.md (pass step + Merge & never), templates/faber-command.md (round-loop pass + "you do merge" summary), and the README.md "Faber auto-merges" design decision.

Kept intact: in-session-only scope (status pass + brief stay read-only; unattended path deferred to #46), high-risk carve-out (human, last word), merge-ready semantics (current-head-bound, void on new commits), all rails.

Structure check passes; shellcheck clean (no scripts changed). Not merging / not approving / not touching labels.

Self-mod note: prompt/doc-only — no live behavior change until the manager prompt is re-synced (scripts/install.sh / /faber re-reading manager/CLAUDE.md).

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The new auto-merge instructions do not cleanly bind the reviewed SHA to the merge pin because the main loop still runs the reviewer before the capture sequence, leaving either duplicate reviews or a path to merging an unreviewed head.

Review comment:

• [P1] Capture the head before the reviewer runs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.3xrtkfWVus/manager/CLAUDE.md:39-45
  With the surrounding loop, step 2 has already run codex-review.sh before this new safe sequence is reached. If Faber acts on that already-posted clean review, a commit pushed after the review but before this capture will be pinned by --match-head-commit "$head" and can be merged even though Codex did not inspect it; if Faber follows the nested sequence literally, it posts a second review for every passing PR. Move the head capture ahead of the single review that determines pass, and use that same SHA for the merge check.

[yihanzhu]

Scope-down applied (6ed1335).

Per the agreed decision on this PR, I removed the fragile step-by-step merge command sequence from the prompts (the gh pr merge ... --match-head-commit "$head" capture→review→merge steps) and replaced it with the auto-merge policy at intent altitude:

Faber MAY auto-merge a PR it reviewed in-session when it is CI-green, Codex-clean, and low-risk — scoped to the target repo (never another repo) and bound to the exact head it reviewed (if the head moved, re-review rather than merge). High-risk PRs (auth, migrations, shared/production repos, security-sensitive) always go to the human merge gate.

Applied to manager/CLAUDE.md, templates/faber-command.md, and README.md. Each now ends the merge section with: the precise, race-safe merge command sequence — and the unattended status-scan / cross-repo auto-merge — are specified in issue #46 (pending); until it lands, follow this intent for in-session merges.

Note: the exact safe-merge command sequence is intentionally deferred to #46.

Kept intact: the doc reconciliation (no "no auto-merge"/"never merge" absolutes remain — Faber MAY merge clean PRs), front-gate consistency (ready recorded after approval; never self-approves), the high-risk carve-out, read-only status/brief, the current-head principle, and all rails (reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-write-code/open-PRs). The merge-ready label descriptions in setup-target-repo.sh / templates/repo-setup.md were already consistent (no command sequence), so they're unchanged.

Structure check + shellcheck pass locally. Not touching labels/merge/approve.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The patch enables an auto-merge behavior in the manager prompt before the documented race-safe merge mechanism exists, which can allow merging an unreviewed head in a normal PR update race.

Review comment:

• [P1] Gate auto-merge on a SHA-checked merge command — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.oRBG99fYto/manager/CLAUDE.md:39-41
  For the new auto-merge path, this is the installed manager prompt, and it now tells Faber to merge even though the race-safe merge sequence is still pending in Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46. When a contributor pushes between the Codex review/CI check and the eventual merge, there is no atomic reviewed-head guard here (for example a --match-head-commit-style check), so Faber can merge a head Codex did not review. Please keep this as hand-to-human until Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 lands, or include the concrete SHA-checked command sequence before enabling auto-merge.

[yihanzhu]

Faber — root cause found; human decision on the auto-merge rail

After 7 rounds, the reviewer pinned it: we're documenting auto-merge as enabled before its race-safe mechanism (#46) exists. Codex's accepted fixes are either "hand-to-human until #46 lands" or "ship the concrete SHA-checked command" (which is exactly what churned 7×).

The doc-reconciliation core of this PR is done and good (no more "no auto-merge"/"never merge" absolutes; front-gate consistent; high-risk→human; status/brief read-only; rails intact). The only open question is whether to enable auto-merge in the prompt now.

Fork (human's call — auto-merge rail):

• B (recommended): build Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 (the safe mechanism — incl. codex-review.sh emitting the reviewed SHA) FIRST, then this PR's auto-merge wording lands clean on top. Right order (mechanism before enablement); safe auto-merge with no nudging gap.
• C (pragmatic): enable auto-merge now, overriding the P1 — the race it warns about needs a concurrent contributor, which the personal in-session flow doesn't have; multi-contributor hardening stays tracked in Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46.
• A: gate on Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 in the docs (hand-to-human until then) — safe, but re-introduces nudging short-term.

Note: the operator's standing in-session auto-merge (verbal grant) continues regardless — this is about the documented/prompt default.

[yihanzhu]

Round 4 (pushed 663fffb): wired scripts/merge-pr.sh into Faber's in-session auto-merge now that #46/#47 landed.

• manager/CLAUDE.md (pass step, Tracking, Merge & never), templates/faber-command.md (round-loop + never bullets), README.md (loop diagram, design-decisions, Rollout Phase 3): on a clean in-session Codex review + low-risk PR, Faber auto-merges by running scripts/merge-pr.sh <PR#> from the target repo's clone (absolute path, same convention as codex-review.sh). The script owns the mechanical safety — authenticated reviewed head+base SHA marker, head+base unchanged since review, ≥1 real CI pass, --match-head-commit pin — and refuses otherwise, so Faber never hand-crafts a merge command.
• High-risk PRs (auth, migrations, shared/production, security-sensitive) → human merge gate even when clean; Faber does not run merge-pr.sh for those.
• Status pass + brief stay read-only (surface merge-ready only). The unattended status-scan / cross-repo auto-merge remains a future extension of merge-pr.sh, deferred to Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 (per the script's header).
• Front-gate wording, merge-ready semantics, and all rails (reviewer read-only, CI hard gate, rounds cap, needs-human, never-self-approve, never-write-code/open-PRs) unchanged. No leftover "deferred to Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46 / hand-to-human until Build the safe in-session merge mechanism: scripts/merge-pr.sh + codex-review.sh records reviewed-SHA #46" for the in-session path.

Verify: structure check passes (docs/prompt-only; no .sh touched, so shellcheck unaffected).

Note (self-mod safety): this is a prompt change to manager/CLAUDE.md and templates/faber-command.md — live behavior does not change until the human re-pastes the routine.

Label: round-4 doesn't exist (only round-0..3), so per the fallback I removed needs-human and left round-3. Not merging/approving — that's the human/Faber call.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 663fffb
Reviewed-base: 0dd74fb

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The new auto-merge flow depends on a helper with an undocumented prerequisite, so the documented setup can pass while the new merge step fails. There is also a stale helper header contradicting the newly wired Faber behavior.

Full review comments:

• [P2] Add jq to the auto-merge setup path — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ndgVz9w4Tt/QUICKSTART.md:85-86
  When this new merge path runs on a fresh machine without jq, scripts/merge-pr.sh exits before checking/merging because it requires both gh and jq, but the updated quickstart/doctor preflight still only documents/checks gh, Claude, Codex, required files, and labels. That means a user can pass the documented setup and hit a failure only after a clean review; add jq to the prerequisites and/or doctor.sh now that Faber is expected to invoke the merge helper.

• [P3] Update the merge helper header with the new wiring — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ndgVz9w4Tt/manager/CLAUDE.md:33-35
  This switches Faber to run merge-pr.sh automatically, but the helper's own header still says it is a manual/standalone helper and that manager/CLAUDE.md and templates/faber-command.md still hand clean PRs to the human gate until issue docs: reconcile merge-gate + front-gate to standing auto-merge #45 updates them. That makes two source-of-truth files contradict each other in a safety-sensitive flow; update the script header as part of this wiring change.

[yihanzhu]

Round-5: both Codex findings addressed in 8a76792 (docs + script-comment + doctor-check; no behavior change).

[P2] jq prereq documented + checked. jq is required by `scripts/merge-pr.sh` (it parses `gh pr checks --json`) but was previously undocumented and unchecked — a fresh machine passed setup, then the merge step failed. Fixed:

• `QUICKSTART.md` — added a jq bullet to Prerequisites (notes it is needed by the merge helper), and added jq to §5's enumeration of doctor's checks.
• `scripts/doctor.sh` — new `(e) jq on PATH` check placed next to the gh/codex checks, with an actionable fail (jq not on PATH — install jq; required by scripts/merge-pr.sh). Updated the header-comment checks list and `usage()` to include jq; renumbered the file/label checks to (f)/(g).

[P3] merge-pr.sh header aligned with the wired reality. Removed the stale "manual / standalone helper" + "the Faber sources hand clean PRs to the human until #45 updates them" wording. The WIRING note now states the script is invoked by Faber's in-session auto-merge flow (for clean, low-risk, in-session-reviewed PRs); high-risk still → human (Faber never calls it for them); the unattended status-scan / cross-repo path remains the documented future extension.

Kept intact: all of this PR's auto-merge wiring and merge-pr.sh's safety logic (SHA-pin / repo-scope / base-pin / CI-green; `--match-head-commit`) — only the header comment changed. Rails unchanged: reviewer read-only/comments-only, high-risk → human, status/brief read-only, no self-approve/merge.

shellcheck `-S style` (all scripts) clean; structure check passes; both scripts stay `+x`. `./scripts/doctor.sh` shows the new `pass: (e) jq on PATH` on this machine. Label left at `round-3` (unchanged). Not merging / not approving.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 8a76792
Reviewed-base: 0dd74fb

Posted verbatim by codex-review.sh (codex exec review --base origin/main in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The changes consistently update the documentation and prompts to describe the new in-session low-risk merge flow, and the only functional script change adds a jq preflight that matches merge-pr.sh's dependency. I did not find a discrete regression introduced by this patch.