feat: safe in-session merge harness + reviewed-SHA marker by yihanzhu · Pull Request #47 · yihanzhu/ystack

yihanzhu · 2026-06-26T10:54:47Z

Closes #46.

What

Builds the safe in-session merge mechanism that #45 needs, and gives it a trustworthy reviewed-SHA to bind to.

scripts/codex-review.sh records the reviewed head SHA. It already fetches the PR head it reviews; now it stamps that SHA into the posted review comment header as a parseable marker line Reviewed-head: <full-sha>. The marker is part of Faber's header prefix, clearly separate from Codex's verbatim body — no other behavior change; stays read-only / comments-only / verbatim.
New scripts/merge-pr.sh <PR#> — the safe in-session merge harness. Read-only until the final merge:
- Preflight: command -v gh (+ jq); numeric <PR#> else usage; unset GH_REPO; derive repo from cwd (gh repo view --json nameWithOwner); explicit --repo "$repo" on every gh call.
- Reads the reviewed SHA from the latest codex-review.sh comment (Reviewed-head: marker, most recent). None → refuse (actionable: run codex-review.sh first).
- Confirms current PR head == reviewed SHA (gh pr view --json headRefOid). Mismatch → refuse (re-review). Race guard.
- Confirms CI green on the current head via gh pr checks --json bucket (every check pass/skipping; refuses on fail/pending/cancel or no checks).
- Merges: gh pr merge <PR#> --repo "$repo" --squash --match-head-commit "<reviewed-sha>" (server-side belt-and-suspenders on the head check).
- Does not judge review pass/fail or risk — Faber's call. Header notes the unattended/cross-repo path is a future extension, not supported yet.
Registered scripts/merge-pr.sh in ci/required-files.txt + README Layout; noted the Reviewed-head: marker in reviewer/codex-review.md.

Why

PR #45 churned 7 rounds trying to specify a race-safe merge sequence in prompt prose, which can't be made bulletproof. This puts the mechanical safety (SHA-pin, repo-scope, CI-green) in a validated script bound to a durable reviewed-SHA. #45 then enables auto-merge by calling it.

Self-modification note

This is a script change, not a live prompt change — no re-paste into the Claude UI needed. reviewer/codex-review.md is the reviewer doc (mechanism), not a pasted routine. Wiring this into Faber's prompt is #45, out of scope here.

How tested

shellcheck -S style scripts/*.sh — clean (CI-equivalent find | xargs).
Structure check (bash) — passes, incl. merge-pr.sh present + +x.
Refuse paths demonstrated (no real PR merged, no test artifacts left on PRs):
- No review — ran merge-pr.sh 45 against real PR docs: reconcile merge-gate + front-gate to standing auto-merge #45 (no marker): exit 1, "no Codex review found … run scripts/codex-review.sh 45 first". (PR docs: reconcile merge-gate + front-gate to standing auto-merge #45 comment count unchanged — read-only.)
- Head mismatch — stubbed gh: exit 1, "head changed since review (d…→e…); re-review before merging".
- CI not green — stubbed gh: exit 1 for fail, pending (gh exit 8), and no checks ("CI is the hard gate").
- Usage / non-numeric arg — exit 1 with usage.
- Happy path (head matches + all pass/skipping) — emits exactly gh pr merge <PR#> --repo … --squash --match-head-commit <reviewed-sha> (stub; not executed against a real PR).
Verified gh's embedded gojq parses the Reviewed-head: marker query (picks most recent; ignores inline-prose mentions via the ^…$ anchor).

🤖 Generated with Claude Code

Closes #46. - scripts/codex-review.sh now stamps `Reviewed-head: <full-sha>` into its posted review comment header, so a later actor knows the exact commit Codex reviewed. Marker is part of Faber's header prefix, clearly separate from Codex's verbatim body — read-only/comments-only/verbatim unchanged. - New scripts/merge-pr.sh <PR#>: read-only until the final merge. Reads the latest Reviewed-head marker, refuses if none; confirms current head == reviewed SHA (race guard), refuses on mismatch; confirms CI green (gh pr checks buckets), refuses otherwise; then squash-merges pinned with --match-head-commit. Repo-scoped (GH_REPO unset, cwd-derived --repo). Does not judge review pass/fail or risk (Faber's call). Header notes the unattended/cross-repo path is a future extension, not supported yet. - Registered merge-pr.sh in ci/required-files.txt + README Layout; noted the Reviewed-head marker in reviewer/codex-review.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

yihanzhu · 2026-06-26T10:59:08Z

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 merge harness has safety guard gaps: a review marker can be spoofed by ordinary PR comments, and an all-skipped CI result is treated as green. These issues can lead to merging changes without the intended Codex review or actual passing CI.

Full review comments:

[P2] Authenticate review markers before trusting them — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JVJInQxq4m/scripts/merge-pr.sh:91-94
When a PR author or collaborator comments a line exactly Reviewed-head: <current SHA> after the last Codex run, this query accepts that comment because it scans all comments and does not validate the expected harness header or trusted author. In the environments this script is meant to guard, that lets merge-pr.sh satisfy the review marker and squash-merge a commit that Codex never reviewed; filter to comments posted by the harness before accepting the SHA.
[P2] Require a real passing check before merging — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JVJInQxq4m/scripts/merge-pr.sh:125-125
When every reported check is skipped, for example because jobs are gated by if: conditions, length is non-zero and not_green becomes 0, so the script proceeds even though no CI command actually passed. If CI is the hard gate, require at least one bucket == "pass" in addition to allowing optional skipped checks.

…ng check Round-1 review fixes for the two Codex [P2]s on PR #47 (issue #46): 1. Authenticate the review marker. The Reviewed-head: scan trusted any comment, so a PR author/collaborator could spoof a review marker and get an unreviewed head merged. Now the marker is accepted only from a genuine codex-review.sh comment, requiring BOTH: (a) the comment is authored by the gh-authenticated operator (gh api user) and (b) it carries the harness header signature ("## Codex reviewer (cross-vendor, read-only)") together with the Reviewed-head: <40-hex> marker. A spoof fails (a) and/or (b). 2. Require a real passing check. An all-skipped CI result had not_green==0 and slipped through. Now green = (>=1 bucket "pass") AND (no fail/pending/cancel); skipped is tolerated only alongside a real pass. Zero-checks still refused. shellcheck -S style clean; +x preserved; structure check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

yihanzhu · 2026-06-26T11:03:54Z

Round-1: addressed both Codex [P2]s in scripts/merge-pr.sh (commit a4c918d).

[P2] Authenticate the review marker. The Reviewed-head: scan trusted any comment, so a PR author/collaborator could spoof a marker and get an unreviewed head merged. The marker is now accepted only from a genuine codex-review.sh comment, requiring BOTH:

(a) author — the comment is authored by the gh-authenticated operator (gh api user -q .login), compared to .author.login; a collaborator's comment has a different login; and
(b) header signature — the comment carries the harness header ## Codex reviewer (cross-vendor, read-only) (exact text from codex-review.sh) together with the anchored Reviewed-head: <40-hex> marker line.

A spoof fails (a) and/or (b). Most-recent authenticated marker still wins (filter then sort_by(.createdAt) | reverse). (Note: gh -q doesn't support --arg, so the comments JSON is piped into a real jq --arg operator.)

[P2] Require a real passing check. An all-skipping CI result had not_green == 0 and slipped through. Green is now (>=1 bucket "pass") AND (no fail/pending/cancel); skipping is tolerated only alongside a real pass. Zero-checks still refused.

Kept intact: preflight, unset GH_REPO + explicit --repo, head==reviewed-SHA race guard, --match-head-commit pinned squash-merge, the future-extension note. set -euo pipefail, +x, shellcheck -S style clean, structure check passes.

Verification (no real merge):

Spoof marker authored by a non-operator (or lacking the harness header) → refuses at the authenticated-marker gate ("no authenticated Codex review found").
All-skipping CI → refuses ("no passing CI check … CI must actually run and pass").
Genuine operator+header+marker comment with >=1 passing check → proceeds to the pinned --match-head-commit squash-merge (merge stubbed; not executed).
Real-PR run of the live script against this PR refuses cleanly at the marker gate (the existing Codex comment's Reviewed-head: is inside a code path, not a standalone marker line), never reaching the merge call.

Label bumped round-0 → round-1. Not merging / not approving — over to Codex for re-review.

yihanzhu · 2026-06-26T11:08:14Z

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 merge harness has a safety gap around base-branch movement and a smaller diagnostics issue in its CI failure path. The head pinning is useful, but it does not fully guarantee the reviewed/tested integration is what gets merged.

Full review comments:

[P2] Guard against base changes after review — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Z4TzWiXMwQ/scripts/merge-pr.sh:128-129
In repos without branch protection or a merge queue requiring up-to-date branches, if the base branch advances after codex-review.sh runs, this guard still passes because only the PR head SHA is compared; --match-head-commit also only pins that head. That can merge a PR whose integration with the current base was not the diff Codex reviewed and whose prior checks may still be green, so the review marker/merge guard should also bind the reviewed base SHA or require an up-to-date/queued merge.
[P3] Include check names in CI diagnostics — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Z4TzWiXMwQ/scripts/merge-pr.sh:145-145
When CI is not green, the diagnostic below prints \(.name), but this gh pr checks call only requests the bucket field. Since gh --json returns only requested fields, the failing-check list will show null: fail/pending instead of actionable check names; request name here as well or stop printing it.

Round-2 Codex feedback on PR #47: [P2] Bind the reviewed BASE, not just the head. --match-head-commit and the head==reviewed-SHA check only pin the PR head; if the base branch advances after codex-review.sh runs, the merge integrates a base Codex never reviewed (a different effective diff) yet the guard passes. - codex-review.sh: rev-parse origin/<base> right after the fetch and stamp a `Reviewed-base: <full-sha>` marker alongside `Reviewed-head:` in the harness comment. - merge-pr.sh: parse Reviewed-head AND Reviewed-base from the SAME authenticated harness comment, then after the head check also confirm the PR's current baseRefOid == the reviewed base SHA; refuse ("base advanced since review (<old>-><new>)") if it moved. [P3] Show real check names in the CI diagnostic. The not-green diagnostic printed `.name` but the gh pr checks --json call only requested `bucket`, so it printed null. Request `name,bucket` so failing checks list real names. Kept intact: marker authentication (operator author + harness header signature), the >=1-real-pass CI rule, preflight, unset GH_REPO + explicit --repo, the --match-head-commit pinned merge, refuse-paths. shellcheck-clean, +x, structure check passes. Verified (mocked gh, no real merge): base-changed -> refuse; head-changed -> refuse; all-skipped/no-real-pass -> refuse; spoofed/non-operator marker -> refuse; missing Reviewed-base -> refuse; genuine marker (head+base match) + >=1 pass -> proceeds to the pinned merge (stubbed). CI diagnostic now prints the real failing check name. Refs #46 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

yihanzhu · 2026-06-26T11:14:56Z

Round-2: addressed both Codex findings (commit eef1555).

[P2] Bind the reviewed BASE, not just the head. --match-head-commit + the head check only pinned the PR head, so a base that advanced after review could be merged as an integration Codex never saw.

scripts/codex-review.sh: after the fork-safe fetch, git rev-parse origin/<base> and stamp a Reviewed-base: <full-sha> marker alongside Reviewed-head: in the harness comment header.
scripts/merge-pr.sh: parse Reviewed-head and Reviewed-base from the same authenticated harness comment (one consistent review, never mixed across comments), then after the head check also confirm the PR's current baseRefOid == the reviewed base SHA. If it moved → refuse (base advanced since review (<old>→<new>); re-review before merging).

[P3] Show real check names in the CI diagnostic. The not-green diagnostic printed .name but the gh pr checks --json call only requested bucket (so it printed null). Now requests name,bucket → failing checks list real names.

Kept intact: round-1 marker authentication (operator author + harness header signature), the ≥1-real-pass CI rule, preflight, unset GH_REPO + explicit --repo, --match-head-commit pinned squash-merge, refuse-paths, Reviewed-head: marker. set -euo pipefail, +x, shellcheck -S style clean, structure check passes.

Verification (mocked gh, no real merge):

base changed → refuse (base advanced since review …).
head changed → refuse (existing race guard).
all-skipped / no real pass → refuse (no passing CI check …).
spoofed marker by a non-operator → refuse; operator comment lacking the harness header (bare markers) → refuse; genuine header but missing Reviewed-base → refuse.
genuine operator+header comment with head+base match and ≥1 pass → proceeds to the pinned --match-head-commit squash-merge (merge stubbed; not executed).
Confirmed the CI not-green diagnostic now prints real check names (e.g. - e2e-tests: fail), not null.

Label bumped round-1 → round-2. Not merging / not approving — over to Codex for re-review.

yihanzhu · 2026-06-26T11:21:49Z

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 merge path has a remaining base-branch race despite claiming to pin the reviewed base, and it is not wired into the Faber instructions that drive the workflow. These issues mean the patch does not yet correctly deliver the advertised safe in-session merge behavior.

Full review comments:

[P2] Move the base check next to the merge — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.kcGIoNZYVG/scripts/merge-pr.sh:149-149
Because this baseRefOid check runs before the CI query and the final gh pr merge only pins the head, a repo where another PR lands on the base branch after this check but before the merge will still merge into an unreviewed base. That is exactly the case the new Reviewed-base guard is meant to prevent in repos without an up-to-date-branch/merge-queue requirement, so recheck the base immediately before the merge or require a server-side up-to-date gate rather than relying on this earlier read.
[P2] Wire the merge harness into Faber’s instructions — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.kcGIoNZYVG/scripts/merge-pr.sh:6-7
This new script says Faber invokes it after a clean review, but the actual /faber sources still tell Faber to apply merge-ready and “Never merge” (manager/CLAUDE.md and templates/faber-command.md). In that current workflow, clean low-risk PRs will still stop at the human merge gate and this SHA-pinned merge path is never used; update those source-of-truth instructions to call this script, or describe it as a manual helper instead.

…ase-race doc (round-3) [P2] Minimize the base-race window: move the `baseRefOid == Reviewed-base` re-check from before the CI query to immediately before `gh pr merge` (after the CI check), so the base is verified as late as possible. Also document the residual honestly in the header: `--match-head-commit` pins only the head (gh has no `--match-base-commit`), so the base-race is fully closed only by server-side branch protection / required-up-to-date — a per-repo setup step; the script minimizes but cannot eliminate it alone. [P2] Don't over-claim wiring: soften the header to describe this as the safe in-session merge mechanism / manual-standalone helper. Wiring it into Faber's flow lands in #45; until then the Faber sources still hand clean PRs to the human gate. (No Faber prompt files touched — that's #45.) Round-1/2 safety kept intact: authenticated marker (operator author + harness header), Reviewed-head + Reviewed-base binding from one comment, >=1-real-pass CI, head pin via --match-head-commit, preflight, unset GH_REPO + explicit --repo, refuse-paths. set -euo pipefail, +x, shellcheck -S style clean, structure check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

yihanzhu · 2026-06-26T11:26:57Z

Round-3: addressed both Codex [P2]s in scripts/merge-pr.sh (commit 0418858).

[P2] Minimize the base-race window + document the residual honestly. The baseRefOid == Reviewed-base re-check used to run before the CI query. It now runs immediately before gh pr merge (after the CI check), so the base is verified as late as possible — the window between the re-check and the merge is now minimal. The header documents the residual honestly: --match-head-commit pins only the head (gh has no --match-base-commit), so the merge cannot atomically pin the base server-side; a PR landing on the base in the gap between the last-moment re-check and the merge would still merge into an unreviewed base. That race is fully closed only by a server-side gate — branch protection with "require branches to be up to date before merging" (or a merge queue) on the base branch (a per-repo setting, a documented setup step). The script minimizes but cannot eliminate it alone.

[P2] Don't over-claim wiring. The header no longer says "Faber invokes it after a clean review." It now describes this as the safe in-session merge mechanism and a manual / standalone helper, and states that wiring it into Faber's flow lands in #45 (in progress) — until then the Faber sources (manager/CLAUDE.md, templates/faber-command.md) still hand clean PRs to the human gate. No Faber prompt files were touched here (that's #45).

Kept intact: round-1/2 marker authentication (operator author + harness header signature), Reviewed-head + Reviewed-base binding read from one consistent comment, the >=1-real-pass CI rule, head pin via --match-head-commit, preflight, unset GH_REPO + explicit --repo, refuse-paths. set -euo pipefail, +x, shellcheck -S style clean, structure check passes.

Verification (mocked gh, no real merge):

spoofed marker (non-operator author) -> refuse at marker gate; operator comment lacking the harness header -> refuse.
head changed -> refuse (race guard).
CI all-skipped / no real pass -> refuse; CI fail -> refuse.
base changed with head OK and CI green -> refuse (base advanced since review …) — this fires after the CI green check, proving the base re-check now sits immediately before the merge.
genuine operator+header comment with head+base match and >=1 pass -> reaches the pinned --match-head-commit squash-merge (merge stubbed; not executed). Merge log confirms only the happy path ever reached gh pr merge.

Label bumped round-2 -> round-3. Not merging / not approving — over to Codex for re-review.

yihanzhu · 2026-06-26T11:33:08Z

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 changes add the reviewed head/base markers and a guarded merge helper without an obvious correctness or safety regression in the reviewed diff. No actionable blocking issues were identified.

Now that scripts/merge-pr.sh has landed on main (via #46/#47), enable the in-session auto-merge by pointing Faber at it instead of stopping at the human merge gate / deferring the mechanism to #46. - manager/CLAUDE.md, templates/faber-command.md, README.md: 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) still go to the 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 #46 (per the script's header). - Front-gate, merge-ready semantics, and all rails unchanged. Prompt change: this updates manager/CLAUDE.md and templates/faber-command.md; live behavior does not change until the human re-pastes the routine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs: reconcile merge-gate + front-gate to standing auto-merge (Closes #43) The operating policy granted Faber standing auto-merge (CI-green + Codex-clean, low-risk) but the docs still said the opposite. Reconcile every doc to actual practice without weakening the remaining rails. - README.md: replace "No auto-merge in Phase 1. Faber pings; you merge." with the real policy (Faber auto-merges clean low-risk PRs; human only for needs-human/round-cap, safety-rail changes, north-star/goal drift). Loop diagram + Rollout phases made consistent. - manager/CLAUDE.md: "Never merge. Merging is mine." -> "Merge clean PRs" per standing authorization, with the human-review carve-outs; kept never-write-code/open-PRs and never-self-approve intact. Loop pass-step and Tracking updated. - templates/faber-command.md: same reconciliation for the live /faber prompt. - Consistency sweep: QUICKSTART.md, RESTORE.md (intro, smoke test, safety rails, branch-protection note), reviewer/codex-review.md flow diagram, routines/brief.md, templates/repo-setup.md — removed every leftover "no auto-merge" / "you merge" / "never merges" contradiction. - CLAUDE.md self-mod rail list: the "no auto-merge" rail reworded to its surviving form (merge stays gated + human-review carve-outs) — this is the deliberate, human-authorized loosening of that rail. Rails preserved: reviewer read-only/comments-only, CI hard gate, rounds cap (~3), needs-human escalation, never-self-approve, never-write-code, one coder-launch per issue. Only auto-merge-of-clean-PRs loosens. Docs/prompt-only; no .sh touched. Structure check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: exclude high-risk from final merge rule; un-strand merge-ready PRs Round-1 review fixes for PR #45 (issue #43): - [P1] Move the high-risk exclusion *inside* the FINAL merge instruction in manager/CLAUDE.md, templates/faber-command.md, and README.md so the last word on merging always carves out auth / DB-migrations / shared-or-production repos / security-sensitive / operator-judgment changes — these go to the human merge gate even when CI-green + Codex-clean. - [P2] Tracking now auto-merges low-risk merge-ready PRs that turned CI-green after the loop ended (manager/CLAUDE.md), and the read-only brief flags them as ready-to-merge — so they're no longer stranded. High-risk / human-held ones are still listed for the user. Prompt/docs-only: live behavior changes only when re-pasted into the routines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: brief = read-only surfacing; auto-merge = Faber's status/Tracking pass Resolves the round-2 [P2]: routines/brief.md only FLAGGED low-risk merge-ready PRs while manager/CLAUDE.md Tracking AUTO-MERGES them — inconsistent, and stranding-prone if the brief is the only scan. - routines/brief.md: state plainly it is read-only — it surfaces state, does not merge. Report low-risk merge-ready + CI-green PRs as auto-merge-eligible (Faber merges them on a status/Tracking pass, picked up there, not in the brief); list high-risk / human-held ones. - manager/CLAUDE.md: name the status/Tracking pass the auto-merge actor, contrasted with the read-only brief that only surfaces the same state. - README.md (Phase 2): decouple — the brief resurfaces (read-only, never merges); Faber auto-merges on a status/Tracking pass. No file both calls the brief read-only AND says it merges. High-risk carve-out (always to the human merge gate) and all rails unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: bind merge-ready to current head; scope README table to authoring Round-3 Codex feedback (issue #43, docs/prompt-only). [P1] merge-ready now means "the CURRENT head passed Codex review." A new push voids the label (GitHub keeps it across head changes); before auto-merging, confirm the latest Codex review covered the current head SHA, else clear merge-ready and re-run codex-review.sh on the new head first — only a passing review of the current head re-applies merge-ready. Stated in the pass path, the Tracking auto-merge rule, and the Merge & never rule of manager/CLAUDE.md; mirrored in templates/faber-command.md (round loop + the closing merge sentence) and routines/brief.md (stale heads flagged as needs-fresh-review, not auto-merge-eligible). [P2] README agent table no longer contradicts the auto-merge rule: Faber's writes scoped to authoring — "issues only; never authors code/PRs (merges clean low-risk PRs)." High-risk carve-out and all rails (reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-self-approve, never-write-code/open-PRs) unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: pin auto-merge to reviewed head; sync merge-ready label description Round-4 of issue #43. Address both Codex findings (docs/prompt + label text only): [P2] Atomic head-match guard. State the auto-merge action as pinned to the reviewed SHA via `gh pr merge <PR#> --squash --match-head-commit <reviewed-sha>` so a push between the head-check and the merge fails atomically instead of merging an unreviewed head (closes the TOCTOU race in unprotected repos). Encoded in manager/CLAUDE.md (Tracking auto-merge rule + Merge & never rule), templates/faber-command.md (round-loop pass line + closing merge sentence), and README.md (Faber auto-merges design decision). [P3] Sync canonical merge-ready label description to the auto-merge semantics in scripts/setup-target-repo.sh and templates/repo-setup.md: "Current head passed Codex review; auto-merged if low-risk, else awaiting your merge". All rails intact: high-risk always to the human (last word on merging), reviewer read-only/comments-only, current-head invariant (merge-ready void on new commits). Structure check + shellcheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: scope auto-merge to in-session-only; status pass + brief report-only; defer unattended path to #46 The only auto-merge path is now in-session: Faber merges a PR only when it just reviewed that exact head back-to-back (review->merge), pinned with `gh pr merge --squash --match-head-commit <reviewed-sha>`. A later status/ Tracking scan and the brief only SURFACE `merge-ready` PRs (read-only) and never merge. The unattended status-scan / cross-repo auto-merge path (which needs a durable reviewed-SHA mechanism and `--repo` qualification) is deferred to #46 and intentionally not enabled. - manager/CLAUDE.md: pass-step merge is in-session/back-to-back with the reviewed-SHA pin; Tracking pass is read-only (was the "auto-merge actor"); "Merge clean PRs" -> "Merge clean PRs in-session" + #46 deferral note. - templates/faber-command.md: pass-step + summary scoped to in-session-only; status scan never auto-merges; #46 referenced. - README.md: design-decision -> "in-session only" + #46 note; Phase 2 status pass/brief now read-only; Phase 3 names the deferred #46 work; loop diagram annotated in-session. - routines/brief.md: low-risk PRs reported as "awaiting merge" via in-session review (no more "auto-merge-eligible on a status pass" handoff). - setup-target-repo.sh + templates/repo-setup.md: merge-ready label -> "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, current-head invariant, read-only brief. Self-mod note: prompt/doc-only; no live behavior change until the human re-runs scripts/install.sh (/faber) and Faber re-reads manager/CLAUDE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: encode safe capture→review→pinned-merge sequence; repo-qualify the merge Addresses both Codex [P1]s on PR #45 (docs/prompt-only): - Trustworthy reviewed SHA: the in-session auto-merge now captures the head BEFORE review (gh pr view --json headRefOid) and pins the merge to it, so the pin can never be back-filled from post-review/current state. If the head moved at any point the merge refuses; treat merge-ready as void and re-review. - Repo-qualify the merge: mirror codex-review.sh — unset GH_REPO + explicit --repo <repo> derived from the cwd's remote, 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"), and the README.md "Faber auto-merges" design decision. In-session-only scope, #46 deferral, high-risk carve-out, and merge-ready semantics all intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: defer race-safe merge sequence to #46; keep auto-merge policy at intent altitude Per the scope-down on #43/PR #45: remove the step-by-step gh pr merge --match-head-commit capture→review→merge command sequence from the prompts (manager/CLAUDE.md, templates/faber-command.md, README.md) and replace it with the auto-merge POLICY at intent altitude — Faber MAY auto-merge an in-session reviewed PR when CI-green, Codex-clean, low-risk; repo-scoped; bound to the reviewed head (head moved → re-review). High-risk PRs (auth, migrations, shared/production repos, security-sensitive) always go to the human merge gate. The precise race-safe command sequence and unattended status-scan / cross-repo auto-merge are deferred to issue #46 (pending). Rails unchanged: reviewer read-only/comments-only, CI hard gate, rounds cap, needs-human, never-write-code/open-PRs, never-self-approve, read-only status/brief, current-head principle. merge-ready label descriptions already consistent (no command sequence) — left as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: wire scripts/merge-pr.sh into Faber's in-session auto-merge (#43) Now that scripts/merge-pr.sh has landed on main (via #46/#47), enable the in-session auto-merge by pointing Faber at it instead of stopping at the human merge gate / deferring the mechanism to #46. - manager/CLAUDE.md, templates/faber-command.md, README.md: 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) still go to the 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 #46 (per the script's header). - Front-gate, merge-ready semantics, and all rails unchanged. Prompt change: this updates manager/CLAUDE.md and templates/faber-command.md; live behavior does not change until the human re-pastes the routine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: document+check jq prereq; align merge-pr.sh header with wired flow [P2] jq is required by scripts/merge-pr.sh (parses gh pr checks --json) but was undocumented and unchecked, so a fresh machine passes setup then fails at merge. Add jq to QUICKSTART Prerequisites and to its §5 list of doctor's checks; add a (command -v jq) check to scripts/doctor.sh (check (e), next to gh/codex) with an actionable fail message, and update doctor's header-comment checks list and usage() to include jq. File/label checks renumbered (f)/(g). [P3] scripts/merge-pr.sh header no longer calls itself a manual/standalone helper or says the Faber sources defer to the human 'until #45 updates them'. It now states it is invoked by Faber's in-session auto-merge flow for clean, low-risk, in-session-reviewed PRs; high-risk still goes to the human gate; the unattended/cross-repo path remains the future extension. Behavior and safety logic unchanged — comment only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

yihanzhu added the round-0 Review-loop counter: initial PR label Jun 26, 2026

yihanzhu added round-1 Review-loop counter: revision 1 and removed round-0 Review-loop counter: initial PR labels Jun 26, 2026

yihanzhu added round-2 Review-loop counter: revision 2 and removed round-1 Review-loop counter: revision 1 labels Jun 26, 2026

yihanzhu added round-3 Review-loop counter: revision 3 (cap) and removed round-2 Review-loop counter: revision 2 labels Jun 26, 2026

yihanzhu merged commit 0dd74fb into main Jun 26, 2026
1 check passed

yihanzhu deleted the issue-46-merge-pr branch June 26, 2026 11:33

yihanzhu mentioned this pull request Jun 26, 2026

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

Merged

Navigation Menu

feat: safe in-session merge harness + reviewed-SHA marker - #47

feat: safe in-session merge harness + reviewed-SHA marker#47
yihanzhu merged 4 commits into
mainfrom
issue-46-merge-pr

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

yihanzhu commented Jun 26, 2026

What

Why

Self-modification note

How tested

Uh oh!

yihanzhu commented Jun 26, 2026

Codex reviewer (cross-vendor, read-only)

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Codex reviewer (cross-vendor, read-only)

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Codex reviewer (cross-vendor, read-only)

Uh oh!

yihanzhu commented Jun 26, 2026

Uh oh!

yihanzhu commented Jun 26, 2026

Codex reviewer (cross-vendor, read-only)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant