From 1594bce5d824c0822b1ca508aa1c0cf98000916d Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Fri, 24 Jul 2026 12:23:40 -0700 Subject: [PATCH 1/7] Add a merge skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lands one PR: rebase onto latest main, review it adversarially with two fable subagents (defects, and AGENTS.md invariant conformance), triage and fix the findings, gate on `ci`, merge. Rebase rather than merge-from-main, so the tree fable reviews is the tree that lands. The driver writes the fixes and the reviewers run on fable — a model reviewing its own fixes is not an adversarial review. Findings are confirmed against real control flow before acting, since adversarial reviewers over-report. Three fix rounds, then it stops and reports. Co-Authored-By: Claude Opus 5 --- .claude/skills/merge/SKILL.md | 106 ++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 .claude/skills/merge/SKILL.md diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md new file mode 100644 index 0000000..5220d63 --- /dev/null +++ b/.claude/skills/merge/SKILL.md @@ -0,0 +1,106 @@ +--- +name: merge +description: Land a PR — rebase onto latest main, review it adversarially with fable, fix every finding, then merge. Use when asked to merge a PR, land a branch, or ship a change. +--- + +# Merge + +Land one PR on `main`: rebase, review, fix, merge. Takes a PR number +(`/merge 28`); with no argument, use the PR for the current branch +(`gh pr view --json number`). + +## Ground rules + +- **Rebase, never merge-from-main.** The premise is that reviewed code is the code + that lands. A merge commit from `main` means fable reviewed a tree that never + existed. +- **`bun run ci` green is necessary and never sufficient.** Fable's verdict is the + other half of the gate. +- **Never weaken the gate to get a merge.** No skipped tests, loosened assertions, + new ignore rules, or edits to `.github/workflows/ci.yml`. If landing the PR + requires any of that, stop and say so. + +## Procedure + +### 1. Preflight and confirm + +Check `gh auth status`, and that the PR is open and not a draft. Show the user the +PR title, the diff stat, and what you're about to do, then get approval for the +**whole run** — after that, rebase, review, fix, and merge without asking again. + +Re-confirm only if something changes the deal: a conflict resolution you aren't +confident in, or the diff touching release/deploy machinery you didn't flag up front. + +### 2. Rebase onto latest `main` + +Work in a worktree, not the main checkout. Reuse the branch's existing worktree if +`git worktree list` shows one; otherwise +`git worktree add /Users/koomen/git/scratch/scratchwork-worktrees/ `. + +```bash +git fetch origin main +git rebase origin/main +git push --force-with-lease +``` + +Resolve only conflicts whose resolution is mechanical and whose intent on both sides +is unambiguous. Anything where you'd be choosing behavior rather than reconciling +text: `git rebase --abort` and hand it back to the user. + +This is the one failure the rest of the pipeline cannot catch — fable will review +whatever tree you produce and find a misresolution perfectly coherent. + +### 3. Adversarial review with fable + +Spawn **two** subagents with the Task tool, in parallel, both on `claude-fable-5`. +Give each the PR number and title, and `git diff origin/main...HEAD` as the target. + +You are the author of the fixes; they are the reviewers. Keep that split — a model +reviewing its own fixes is not an adversarial review. + +Instruct each to hunt for reasons this should **not** land — a reviewer that returns +"looks good to me" has not done its job — and to report findings as +`{file, line, severity: blocking|nit, claim, failure_scenario}`, empty only if it +genuinely finds nothing after a real search. + +- **Lens 1 — defects.** Correctness, security, regressions, error handling, + concurrency, data loss. Where does this break in production? What input makes it + wrong? +- **Lens 2 — invariant conformance.** Read `AGENTS.md` in full, then check the diff + against the *agent-pass* bullets of all six invariants (same job as the + `check-invariants` skill). CI already mechanizes the greppable cores; the + reviewer's value is the judgment residue. + +### 4. Triage, then fix + +Adversarial reviewers over-report — that's the cost of telling them to attack. +Confirm each finding against the actual code before acting: trace the failure +scenario through the real control flow, and check the premise still holds after the +rebase. Discard what doesn't survive and say what you discarded and why. Don't fix a +phantom to make a reviewer happy. + +Fix every surviving finding, `blocking` and `nit` alike. Then `bun run ci` (needs +Docker for the LocalStack e2e lane), commit, push, and re-run step 3 on the new diff. + +**At most 3 rounds.** If findings still survive, stop and report — a PR that can't +converge in three rounds has a problem this skill is the wrong tool for. + +### 5. Merge + +Wait for the real check on the pushed head — a local `bun run ci` is a prediction, +the workflow run is the fact. `ci` is a required check on `main`, so a red run can't +merge anyway. + +```bash +gh pr checks --watch +gh pr merge --merge --delete-branch +git worktree remove +``` + +Merge commits match this repo's history. + +## Report + +The PR number and title, rebase result, review rounds, findings fixed and findings +discarded (with why), and the final `main` SHA. If you stopped short of merging, say +exactly why and what the user needs to decide. From b484664fd57b2bd6c73a1087c1cd82e042a0864a Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Fri, 24 Jul 2026 13:46:04 -0700 Subject: [PATCH 2/7] Fix procedural defects in the merge skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 adversarial review found the runbook would fail in practice. Fixed, in order of severity: - Pin the reviewed SHA and merge with `--match-head-commit`, so a commit pushed after the review cannot land unreviewed. Re-check that main has not moved before merging. - Reorder cleanup: a branch checked out in a worktree cannot be deleted, so `--delete-branch` failed on every run. Remove the worktree first, then delete the branch explicitly. - Fetch the PR branch, not just main; `git worktree add` needs the ref. - `bun install` in the worktree before `bun run ci` — a fresh worktree has no node_modules. - Give the reviewers the checkout directory. A subagent starts in the session cwd, so a reviewer that is not told where to look diffs the wrong tree and reports nothing. - Handle the branch already being checked out, including in the primary checkout, where `git worktree add` refuses outright. - Require the PR base to be main; rebasing a stacked PR onto main would rewrite it and split the review target from the merge destination. - Only remove a worktree this skill created; a reused one may be a long-lived checkout the user is sitting in. - Derive the worktree path from the repo root instead of hardcoding a home directory into a shared repo file. - Use the `fable` model alias; the Agent tool takes sonnet|opus|haiku|fable, not a full model ID. - Defer to the check-invariants skill instead of restating it, per the standing rule against duplication that can drift. - Note that `gh pr checks` errors rather than waits before a run registers. Co-Authored-By: Claude Opus 5 --- .claude/skills/merge/SKILL.md | 100 ++++++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 23 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index 5220d63..f8317a8 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -24,23 +24,54 @@ Land one PR on `main`: rebase, review, fix, merge. Takes a PR number ### 1. Preflight and confirm -Check `gh auth status`, and that the PR is open and not a draft. Show the user the -PR title, the diff stat, and what you're about to do, then get approval for the -**whole run** — after that, rebase, review, fix, and merge without asking again. +```bash +gh auth status +gh pr view --json title,isDraft,state,baseRefName,headRefName +``` + +The PR must be open and not a draft. **`baseRefName` must be `main`** — this skill +rebases onto `main` and assumes the merge lands there. On a stacked PR based on +another branch, stop: rebasing it onto `main` would rewrite it to include or drop the +base branch's commits, and the review target, the rebase, and the merge destination +would all disagree. -Re-confirm only if something changes the deal: a conflict resolution you aren't -confident in, or the diff touching release/deploy machinery you didn't flag up front. +Show the user the title, the diff stat, and what you're about to do, then get +approval for the **whole run** — after that, rebase, review, fix, and merge without +asking again. Re-confirm only if something changes the deal: a conflict resolution +you aren't confident in, or the diff touching release/deploy machinery you didn't +flag up front. -### 2. Rebase onto latest `main` +### 2. Get a checkout, then rebase -Work in a worktree, not the main checkout. Reuse the branch's existing worktree if -`git worktree list` shows one; otherwise -`git worktree add /Users/koomen/git/scratch/scratchwork-worktrees/ `. +Find where the branch lives — **do not assume you can create a worktree for it.** +`git worktree add` fails if the branch is already checked out anywhere, including the +primary checkout. + +```bash +git fetch origin main # the branch too: worktree add needs the ref +git worktree list # is already checked out? +``` + +- **Already checked out** (any worktree, including the primary one): use that + directory, and **remember not to remove it in step 5** — it may be a long-lived + checkout, possibly the one the user is sitting in. It must be clean + (`git status --porcelain` empty) or the rebase won't start; if it's dirty, stop and + hand it back rather than stashing the user's work. +- **Not checked out:** create a scratch worktree beside the repo, and remember that + you created it: + + ```bash + ROOT=$(git rev-parse --show-toplevel) + WT="$(dirname "$ROOT")/scratchwork-worktrees/" + git worktree add "$WT" + ``` + +Everything below runs in that directory. Then: ```bash -git fetch origin main git rebase origin/main git push --force-with-lease +bun install --frozen-lockfile # a fresh worktree has no node_modules ``` Resolve only conflicts whose resolution is mechanical and whose intent on both sides @@ -52,8 +83,14 @@ whatever tree you produce and find a misresolution perfectly coherent. ### 3. Adversarial review with fable -Spawn **two** subagents with the Task tool, in parallel, both on `claude-fable-5`. -Give each the PR number and title, and `git diff origin/main...HEAD` as the target. +Spawn **two** subagents with the Agent tool, in parallel, both with `model: "fable"` +(the alias — the model parameter takes `sonnet|opus|haiku|fable`, not a full model +ID). + +Give each one **the checkout directory from step 2** — a subagent starts in the +session's cwd, not your working directory, so a reviewer that isn't told where to +look will diff the wrong tree and cheerfully report nothing. Also give it the PR +number and title, and `git diff origin/main...HEAD` as the target. You are the author of the fixes; they are the reviewers. Keep that split — a model reviewing its own fixes is not an adversarial review. @@ -66,10 +103,9 @@ genuinely finds nothing after a real search. - **Lens 1 — defects.** Correctness, security, regressions, error handling, concurrency, data loss. Where does this break in production? What input makes it wrong? -- **Lens 2 — invariant conformance.** Read `AGENTS.md` in full, then check the diff - against the *agent-pass* bullets of all six invariants (same job as the - `check-invariants` skill). CI already mechanizes the greppable cores; the - reviewer's value is the judgment residue. +- **Lens 2 — invariant conformance.** Run the `check-invariants` skill's procedure + against the diff — that skill is normative for this lens; follow it rather than a + restatement here, and if it and this file ever disagree, it wins. ### 4. Triage, then fix @@ -85,19 +121,37 @@ Docker for the LocalStack e2e lane), commit, push, and re-run step 3 on the new **At most 3 rounds.** If findings still survive, stop and report — a PR that can't converge in three rounds has a problem this skill is the wrong tool for. -### 5. Merge +### 5. Merge the tree you reviewed + +Pin the reviewed commit, and confirm `main` hasn't moved out from under it: + +```bash +REVIEWED=$(git rev-parse HEAD) +git fetch origin main +git rev-list --count HEAD..origin/main # non-zero → main moved; restart at step 2 +``` -Wait for the real check on the pushed head — a local `bun run ci` is a prediction, -the workflow run is the fact. `ci` is a required check on `main`, so a red run can't -merge anyway. +Wait for the real check on that head — a local `bun run ci` is a prediction, the +workflow run is the fact. `gh pr checks` errors rather than waits if GitHub hasn't +registered the run yet, so retry a few times before believing it. ```bash gh pr checks --watch -gh pr merge --merge --delete-branch -git worktree remove +gh pr merge --merge --match-head-commit "$REVIEWED" ``` -Merge commits match this repo's history. +`--match-head-commit` is what makes this honest: if anything landed on the branch +after the review, the merge is refused instead of silently shipping an unreviewed +tree. Merge commits match this repo's history. + +Then clean up, in this order — a branch checked out in a worktree cannot be deleted, +which is why `--delete-branch` is not used above: + +```bash +git worktree remove "$WT" # ONLY if you created it in step 2 +git branch -d +git push origin --delete +``` ## Report From f0a91d8bd0c225bbe9659a9bda7cd914958055b4 Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Fri, 24 Jul 2026 13:51:41 -0700 Subject: [PATCH 3/7] Fix defects the round-1 fixes introduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review found the worktree and cleanup machinery was generating most of the failure modes, several of them added by round 1. Cut the machinery rather than patching each bug. - Stop creating worktrees. `git worktree add` fails when the branch is checked out, `--show-toplevel` returns the linked worktree's own root rather than the primary one (so the path nested wrongly), and removing a worktree you are standing in breaks every later command. Use the checkout the branch already lives in. - Stop deleting local branches and worktrees. Deleting a checked-out branch fails, and the checkout may be one the user lives in. Delete the remote branch and report what local state remains. - Never carry shell variables across steps. Each Bash call is a fresh shell, so `$WT` and `$REVIEWED` were empty by the time they were used — which silently neutered `--match-head-commit`. Print and substitute literals instead. - Reconcile the local branch against origin/ before rebasing. Round 1 added a fetch of the branch, which makes `--force-with-lease` compare against the ref it just updated — so a stale local branch would force-push away remote-only commits and the lease would pass. - Resolve the two contradictions the check-invariants deference created: give lens 2 an explicit diff base and findings format that override that skill, so both lenses review the same tree and report the same shape. - Reject fork PRs at preflight rather than failing at the fetch. - Say one lens per reviewer, not both lenses to both. Co-Authored-By: Claude Opus 5 --- .claude/skills/merge/SKILL.md | 76 ++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index f8317a8..e53af11 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -26,10 +26,12 @@ Land one PR on `main`: rebase, review, fix, merge. Takes a PR number ```bash gh auth status -gh pr view --json title,isDraft,state,baseRefName,headRefName +gh pr view --json title,isDraft,state,baseRefName,headRefName,isCrossRepository ``` -The PR must be open and not a draft. **`baseRefName` must be `main`** — this skill +The PR must be open and not a draft. **`isCrossRepository` must be false** — a fork's +branch can't be fetched from `origin` or force-pushed back, so step 2 would die +mid-procedure; reject it here instead. **`baseRefName` must be `main`** — this skill rebases onto `main` and assumes the merge lands there. On a stacked PR based on another branch, stop: rebasing it onto `main` would rewrite it to include or drop the base branch's commits, and the review target, the rebase, and the merge destination @@ -43,35 +45,44 @@ flag up front. ### 2. Get a checkout, then rebase -Find where the branch lives — **do not assume you can create a worktree for it.** -`git worktree add` fails if the branch is already checked out anywhere, including the -primary checkout. +**Never write shell variables you intend to use later.** Each Bash call is a fresh +shell — `$WT` and `$REVIEWED` set in one block are empty in the next. Print the value, +then substitute the literal into later commands. + +Find where the branch lives. Do **not** create a worktree: `git worktree add` fails +when the branch is checked out anywhere, `--show-toplevel` returns the wrong root when +you are already inside a linked worktree, and removing a worktree you are standing in +breaks every command after it. ```bash -git fetch origin main # the branch too: worktree add needs the ref +git fetch origin main git worktree list # is already checked out? ``` - **Already checked out** (any worktree, including the primary one): use that - directory, and **remember not to remove it in step 5** — it may be a long-lived - checkout, possibly the one the user is sitting in. It must be clean - (`git status --porcelain` empty) or the rebase won't start; if it's dirty, stop and - hand it back rather than stashing the user's work. -- **Not checked out:** create a scratch worktree beside the repo, and remember that - you created it: + directory. +- **Not checked out:** check it out in the primary checkout — the first line of + `git worktree list` — and note that you must restore it to `main` in step 5. + +Either way the directory must be clean (`git status --porcelain` empty). If it's +dirty, stop and hand it back; never stash the user's work. + +Now reconcile with the remote **before** rebasing: - ```bash - ROOT=$(git rev-parse --show-toplevel) - WT="$(dirname "$ROOT")/scratchwork-worktrees/" - git worktree add "$WT" - ``` +```bash +git rev-list --count ..origin/ # non-zero → local is stale +``` -Everything below runs in that directory. Then: +This check is not optional. Step 2 just fetched `origin/`, which means +`--force-with-lease` will compare against the ref it *just updated* and pass happily +— so a stale local branch would force-push away commits that exist only on the +remote, and nothing downstream would notice. If the count is non-zero, +`git reset --hard origin/` before continuing. ```bash git rebase origin/main git push --force-with-lease -bun install --frozen-lockfile # a fresh worktree has no node_modules +bun install --frozen-lockfile # node_modules may be absent or stale ``` Resolve only conflicts whose resolution is mechanical and whose intent on both sides @@ -85,7 +96,7 @@ whatever tree you produce and find a misresolution perfectly coherent. Spawn **two** subagents with the Agent tool, in parallel, both with `model: "fable"` (the alias — the model parameter takes `sonnet|opus|haiku|fable`, not a full model -ID). +ID). **One lens each**, not both lenses to both agents. Give each one **the checkout directory from step 2** — a subagent starts in the session's cwd, not your working directory, so a reviewer that isn't told where to @@ -104,8 +115,12 @@ genuinely finds nothing after a real search. concurrency, data loss. Where does this break in production? What input makes it wrong? - **Lens 2 — invariant conformance.** Run the `check-invariants` skill's procedure - against the diff — that skill is normative for this lens; follow it rather than a - restatement here, and if it and this file ever disagree, it wins. + against the diff. That skill owns *what to check* — follow its per-invariant + agent-pass bullets rather than a restatement here, so this file can't drift from it. + Two deliberate overrides, which you must state in the reviewer's prompt: diff against + `origin/main...HEAD`, not that skill's `main...HEAD` (local `main` may be stale, and + both lenses must review the same tree); and return the findings JSON above, not that + skill's per-invariant pass/fail report, so triage can consume both lenses the same way. ### 4. Triage, then fix @@ -123,10 +138,10 @@ converge in three rounds has a problem this skill is the wrong tool for. ### 5. Merge the tree you reviewed -Pin the reviewed commit, and confirm `main` hasn't moved out from under it: +Print the reviewed commit and confirm `main` hasn't moved out from under it: ```bash -REVIEWED=$(git rev-parse HEAD) +git rev-parse HEAD # copy this SHA; do not store it in a variable git fetch origin main git rev-list --count HEAD..origin/main # non-zero → main moved; restart at step 2 ``` @@ -137,22 +152,25 @@ registered the run yet, so retry a few times before believing it. ```bash gh pr checks --watch -gh pr merge --merge --match-head-commit "$REVIEWED" +gh pr merge --merge --match-head-commit ``` `--match-head-commit` is what makes this honest: if anything landed on the branch after the review, the merge is refused instead of silently shipping an unreviewed tree. Merge commits match this repo's history. -Then clean up, in this order — a branch checked out in a worktree cannot be deleted, -which is why `--delete-branch` is not used above: +Then clean up the remote only: ```bash -git worktree remove "$WT" # ONLY if you created it in step 2 -git branch -d git push origin --delete ``` +**Leave local state alone.** Deleting the local branch fails while it is checked out, +and the checkout may be one the user lives in — so `--delete-branch` is not used above. +If you checked the branch out in the primary checkout in step 2, restore it +(`git checkout main && git pull`). Then tell the user which local branch and which +directory are still there, so they can clean up if they want to. + ## Report The PR number and title, rebase result, review rounds, findings fixed and findings From 429e053c8065203f6301b18446335e0c80f85e8b Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Sun, 16 Aug 2026 13:45:04 -0700 Subject: [PATCH 4/7] Simplify the merge skill for a single-contributor repo Rebase in a detached scratch worktree off origin/ instead of hunting for and mutating existing checkouts, drop the fork/stacked-PR preflight and approval ceremony, and collapse the review to one fable agent carrying both lenses. Keep the agent-facing guards: gate integrity, mechanical-conflicts-only, author/reviewer split, the 3-round cap, and --match-head-commit. Co-Authored-By: Claude Fable 5 --- .claude/skills/merge/SKILL.md | 197 +++++++++------------------------- 1 file changed, 50 insertions(+), 147 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index e53af11..ca08453 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -5,174 +5,77 @@ description: Land a PR — rebase onto latest main, review it adversarially with # Merge -Land one PR on `main`: rebase, review, fix, merge. Takes a PR number -(`/merge 28`); with no argument, use the PR for the current branch -(`gh pr view --json number`). +Land one PR on `main`: rebase, review, fix, merge. Takes a PR number (`/merge 28`); +with no argument, use the current branch's PR (`gh pr view --json number`). The PR +must be open, not a draft, and based on `main` — otherwise stop and say why. -## Ground rules +Invoking the skill is approval for the whole run — don't re-confirm along the way. +Check back only if a conflict resolution would mean choosing behavior, or the diff +touches release/deploy machinery you didn't flag up front. -- **Rebase, never merge-from-main.** The premise is that reviewed code is the code - that lands. A merge commit from `main` means fable reviewed a tree that never - existed. -- **`bun run ci` green is necessary and never sufficient.** Fable's verdict is the - other half of the gate. -- **Never weaken the gate to get a merge.** No skipped tests, loosened assertions, - new ignore rules, or edits to `.github/workflows/ci.yml`. If landing the PR - requires any of that, stop and say so. +**Never weaken the gate to get a merge.** No skipped tests, loosened assertions, new +ignore rules, or edits to `.github/workflows/ci.yml`. If landing requires any of +that, stop and say so. -## Procedure +Each Bash call is a fresh shell — never set a shell variable for later use. Print +the value, then substitute the literal into later commands. -### 1. Preflight and confirm +## 1. Rebase in a scratch worktree -```bash -gh auth status -gh pr view --json title,isDraft,state,baseRefName,headRefName,isCrossRepository -``` - -The PR must be open and not a draft. **`isCrossRepository` must be false** — a fork's -branch can't be fetched from `origin` or force-pushed back, so step 2 would die -mid-procedure; reject it here instead. **`baseRefName` must be `main`** — this skill -rebases onto `main` and assumes the merge lands there. On a stacked PR based on -another branch, stop: rebasing it onto `main` would rewrite it to include or drop the -base branch's commits, and the review target, the rebase, and the merge destination -would all disagree. - -Show the user the title, the diff stat, and what you're about to do, then get -approval for the **whole run** — after that, rebase, review, fix, and merge without -asking again. Re-confirm only if something changes the deal: a conflict resolution -you aren't confident in, or the diff touching release/deploy machinery you didn't -flag up front. - -### 2. Get a checkout, then rebase - -**Never write shell variables you intend to use later.** Each Bash call is a fresh -shell — `$WT` and `$REVIEWED` set in one block are empty in the next. Print the value, -then substitute the literal into later commands. - -Find where the branch lives. Do **not** create a worktree: `git worktree add` fails -when the branch is checked out anywhere, `--show-toplevel` returns the wrong root when -you are already inside a linked worktree, and removing a worktree you are standing in -breaks every command after it. +Work on `origin/` in a detached throwaway worktree — never in a checkout the +user lives in, and never on a possibly-stale local branch: ```bash git fetch origin main -git worktree list # is already checked out? -``` - -- **Already checked out** (any worktree, including the primary one): use that - directory. -- **Not checked out:** check it out in the primary checkout — the first line of - `git worktree list` — and note that you must restore it to `main` in step 5. - -Either way the directory must be clean (`git status --porcelain` empty). If it's -dirty, stop and hand it back; never stash the user's work. - -Now reconcile with the remote **before** rebasing: - -```bash -git rev-list --count ..origin/ # non-zero → local is stale -``` - -This check is not optional. Step 2 just fetched `origin/`, which means -`--force-with-lease` will compare against the ref it *just updated* and pass happily -— so a stale local branch would force-push away commits that exist only on the -remote, and nothing downstream would notice. If the count is non-zero, -`git reset --hard origin/` before continuing. - -```bash +git worktree add /merge- origin/ --detach +cd /merge- git rebase origin/main -git push --force-with-lease -bun install --frozen-lockfile # node_modules may be absent or stale +git push --force-with-lease= origin HEAD: +bun install --frozen-lockfile ``` -Resolve only conflicts whose resolution is mechanical and whose intent on both sides -is unambiguous. Anything where you'd be choosing behavior rather than reconciling -text: `git rebase --abort` and hand it back to the user. +Resolve only conflicts that are mechanical, with unambiguous intent on both sides. +Anything where you'd be choosing behavior rather than reconciling text: +`git rebase --abort` and hand it back. This is the one failure the rest of the +pipeline cannot catch — the reviewer will find a misresolution perfectly coherent. -This is the one failure the rest of the pipeline cannot catch — fable will review -whatever tree you produce and find a misresolution perfectly coherent. +## 2. Adversarial review with fable -### 3. Adversarial review with fable +Spawn one subagent with the Agent tool, `model: "fable"`. Tell it the worktree +directory from step 1 (subagents start in the session cwd, not yours — an untold +reviewer diffs the wrong tree and reports nothing), the PR number and title, and the +target: `git diff origin/main...HEAD`. You are the author of the fixes; it is the +reviewer — a model reviewing its own fixes is not an adversarial review. -Spawn **two** subagents with the Agent tool, in parallel, both with `model: "fable"` -(the alias — the model parameter takes `sonnet|opus|haiku|fable`, not a full model -ID). **One lens each**, not both lenses to both agents. +Instruct it to hunt for reasons this should **not** land ("looks good to me" is a +failed review), covering both defects — correctness, security, regressions, error +handling, concurrency, data loss — and the `check-invariants` skill's agent-pass +procedure, and to report findings as +`{file, line, severity: blocking|nit, claim, failure_scenario}`. -Give each one **the checkout directory from step 2** — a subagent starts in the -session's cwd, not your working directory, so a reviewer that isn't told where to -look will diff the wrong tree and cheerfully report nothing. Also give it the PR -number and title, and `git diff origin/main...HEAD` as the target. - -You are the author of the fixes; they are the reviewers. Keep that split — a model -reviewing its own fixes is not an adversarial review. - -Instruct each to hunt for reasons this should **not** land — a reviewer that returns -"looks good to me" has not done its job — and to report findings as -`{file, line, severity: blocking|nit, claim, failure_scenario}`, empty only if it -genuinely finds nothing after a real search. - -- **Lens 1 — defects.** Correctness, security, regressions, error handling, - concurrency, data loss. Where does this break in production? What input makes it - wrong? -- **Lens 2 — invariant conformance.** Run the `check-invariants` skill's procedure - against the diff. That skill owns *what to check* — follow its per-invariant - agent-pass bullets rather than a restatement here, so this file can't drift from it. - Two deliberate overrides, which you must state in the reviewer's prompt: diff against - `origin/main...HEAD`, not that skill's `main...HEAD` (local `main` may be stale, and - both lenses must review the same tree); and return the findings JSON above, not that - skill's per-invariant pass/fail report, so triage can consume both lenses the same way. - -### 4. Triage, then fix +## 3. Triage, fix, repeat Adversarial reviewers over-report — that's the cost of telling them to attack. -Confirm each finding against the actual code before acting: trace the failure -scenario through the real control flow, and check the premise still holds after the -rebase. Discard what doesn't survive and say what you discarded and why. Don't fix a -phantom to make a reviewer happy. - -Fix every surviving finding, `blocking` and `nit` alike. Then `bun run ci` (needs -Docker for the LocalStack e2e lane), commit, push, and re-run step 3 on the new diff. +Confirm each finding against the actual code before acting, and say what you +discarded and why. Fix every surviving finding, `blocking` and `nit` alike, then +`bun run ci` (needs Docker for the LocalStack e2e lane), commit, push, and re-run +step 2 on the new diff. **At most 3 rounds** — if findings still survive, stop and +report. -**At most 3 rounds.** If findings still survive, stop and report — a PR that can't -converge in three rounds has a problem this skill is the wrong tool for. - -### 5. Merge the tree you reviewed - -Print the reviewed commit and confirm `main` hasn't moved out from under it: - -```bash -git rev-parse HEAD # copy this SHA; do not store it in a variable -git fetch origin main -git rev-list --count HEAD..origin/main # non-zero → main moved; restart at step 2 -``` - -Wait for the real check on that head — a local `bun run ci` is a prediction, the -workflow run is the fact. `gh pr checks` errors rather than waits if GitHub hasn't -registered the run yet, so retry a few times before believing it. - -```bash -gh pr checks --watch -gh pr merge --merge --match-head-commit -``` - -`--match-head-commit` is what makes this honest: if anything landed on the branch -after the review, the merge is refused instead of silently shipping an unreviewed -tree. Merge commits match this repo's history. - -Then clean up the remote only: +## 4. Merge the reviewed tree ```bash +git rev-parse HEAD # the reviewed SHA — print it, don't store it +gh pr checks --watch # errors if GitHub hasn't registered the run yet; retry a few times +gh pr merge --merge --match-head-commit git push origin --delete +git worktree remove /merge- ``` -**Leave local state alone.** Deleting the local branch fails while it is checked out, -and the checkout may be one the user lives in — so `--delete-branch` is not used above. -If you checked the branch out in the primary checkout in step 2, restore it -(`git checkout main && git pull`). Then tell the user which local branch and which -directory are still there, so they can clean up if they want to. - -## Report +`--match-head-commit` refuses the merge if anything — say, another agent session — +touched the branch after the review, instead of silently shipping an unreviewed +tree. Afterward, tell the user if a local checkout of `` exists anywhere +(`git worktree list`, `git branch`): it is now behind the rebased remote. -The PR number and title, rebase result, review rounds, findings fixed and findings -discarded (with why), and the final `main` SHA. If you stopped short of merging, say -exactly why and what the user needs to decide. +Report the PR, rebase result, review rounds, findings fixed and discarded (with +why), and the final `main` SHA — or exactly where and why you stopped. From c902ee6a2bd56fd9bd27df38bf6c806382327ea8 Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Sun, 16 Aug 2026 13:53:27 -0700 Subject: [PATCH 5/7] Fix procedural hazards found in round-1 review Guard the rebase->push and checks->merge fences with &&, pin the push lease to the fetched SHA (shared-ref worktrees defeat the expect-less form), preflight local branches that are ahead of origin and leftover worktree paths, pin the check-invariants diff base to origin/main, and cd out before removing the scratch worktree. Co-Authored-By: Claude Fable 5 --- .claude/skills/merge/SKILL.md | 58 +++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index ca08453..4f87ae2 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -17,23 +17,39 @@ touches release/deploy machinery you didn't flag up front. ignore rules, or edits to `.github/workflows/ci.yml`. If landing requires any of that, stop and say so. -Each Bash call is a fresh shell — never set a shell variable for later use. Print -the value, then substitute the literal into later commands. +Two Bash-harness rules. Never set a shell variable for later use — each call is a +fresh shell; print the value, then substitute the literal into later commands. And +start every Bash call after step 1 by `cd`-ing into the scratch worktree by literal +path — cwd doesn't reliably persist either, and a command that lands in the user's +checkout instead would build, commit, or push the wrong tree. ## 1. Rebase in a scratch worktree Work on `origin/` in a detached throwaway worktree — never in a checkout the -user lives in, and never on a possibly-stale local branch: +user lives in, and never on a possibly-stale local branch. Two preflights: + +- If a local `` exists (`git branch --list `), it must not be + **ahead** of origin — `git rev-list --count origin/..` must be 0 + after the fetch below. If it isn't, stop: the PR is missing unpushed work, and + only the user can decide to push it. +- If `/merge-` is left over from an earlier run, clear it with + `git worktree remove --force` before adding it again. ```bash git fetch origin main +git rev-parse origin/ # print this SHA — the push lease below pins it git worktree add /merge- origin/ --detach -cd /merge- -git rebase origin/main -git push --force-with-lease= origin HEAD: -bun install --frozen-lockfile +cd /merge- && git rebase origin/main && + git push --force-with-lease=: origin HEAD: +cd /merge- && bun install --frozen-lockfile ``` +Both guards in the push line are load-bearing. The `&&` chain: a conflicted rebase +exits nonzero mid-rebase, and an unguarded push would overwrite the PR with that +partial history. The explicit `:` lease: worktrees share refs, so any +concurrent fetch refreshes `origin/` and an expect-less lease would wave +through the loss of another session's push. + Resolve only conflicts that are mechanical, with unambiguous intent on both sides. Anything where you'd be choosing behavior rather than reconciling text: `git rebase --abort` and hand it back. This is the one failure the rest of the @@ -50,8 +66,9 @@ reviewer — a model reviewing its own fixes is not an adversarial review. Instruct it to hunt for reasons this should **not** land ("looks good to me" is a failed review), covering both defects — correctness, security, regressions, error handling, concurrency, data loss — and the `check-invariants` skill's agent-pass -procedure, and to report findings as -`{file, line, severity: blocking|nit, claim, failure_scenario}`. +procedure. State explicitly that the diff base is `origin/main`, overriding that +skill's own `main...HEAD` — local `main` in this shared-ref repo is routinely stale. +Findings come back as `{file, line, severity: blocking|nit, claim, failure_scenario}`. ## 3. Triage, fix, repeat @@ -65,17 +82,24 @@ report. ## 4. Merge the reviewed tree ```bash -git rev-parse HEAD # the reviewed SHA — print it, don't store it -gh pr checks --watch # errors if GitHub hasn't registered the run yet; retry a few times -gh pr merge --merge --match-head-commit +cd /merge- && git rev-parse HEAD # the reviewed SHA — print it +gh pr checks --watch && + gh pr merge --merge --match-head-commit git push origin --delete -git worktree remove /merge- +cd && git worktree remove /merge- ``` -`--match-head-commit` refuses the merge if anything — say, another agent session — -touched the branch after the review, instead of silently shipping an unreviewed -tree. Afterward, tell the user if a local checkout of `` exists anywhere -(`git worktree list`, `git branch`): it is now behind the rebased remote. +If `gh pr checks` fails, stop and report — never merge red. The `&&` is the only +thing enforcing that: an admin token can merge over a failed required check here. +(`gh pr checks` also errors if GitHub hasn't registered the run yet — retry a few +times before believing it.) `--match-head-commit` refuses the merge if anything — +say, another agent session — touched the branch after the review; it pins the +branch head, not `main`. The final `cd` out matters too: removing the worktree +you're standing in strands the shell in a deleted directory. + +Afterward, tell the user if a local checkout of `` exists anywhere +(`git worktree list`, `git branch`) — the remote it tracked is gone and its +history was rewritten. Report the PR, rebase result, review rounds, findings fixed and discarded (with why), and the final `main` SHA — or exactly where and why you stopped. From 6d8698a696fa9d62313ffee740ae5605fdeb9533 Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Sun, 16 Aug 2026 13:59:08 -0700 Subject: [PATCH 6/7] Fix procedural hazards found in round-2 review Chain the remote-branch delete onto merge success so a --match-head-commit refusal never deletes an unmerged PR's branch, prescribe the step-3 fast-forward push (stop on rejection, never re-lease), make the reviewer explicitly read-only, run the fetch before the preflights that depend on it, and carve the final cleanup out of the cd-into-worktree rule. Co-Authored-By: Claude Fable 5 --- .claude/skills/merge/SKILL.md | 52 +++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index 4f87ae2..1a05265 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -21,22 +21,28 @@ Two Bash-harness rules. Never set a shell variable for later use — each call i fresh shell; print the value, then substitute the literal into later commands. And start every Bash call after step 1 by `cd`-ing into the scratch worktree by literal path — cwd doesn't reliably persist either, and a command that lands in the user's -checkout instead would build, commit, or push the wrong tree. +checkout instead would build, commit, or push the wrong tree. (One exception: the +final cleanup in step 4 must run from outside the worktree.) ## 1. Rebase in a scratch worktree Work on `origin/` in a detached throwaway worktree — never in a checkout the -user lives in, and never on a possibly-stale local branch. Two preflights: +user lives in, and never on a possibly-stale local branch. -- If a local `` exists (`git branch --list `), it must not be - **ahead** of origin — `git rev-list --count origin/..` must be 0 - after the fetch below. If it isn't, stop: the PR is missing unpushed work, and - only the user can decide to push it. +```bash +git fetch origin main +``` + +Two preflights, now that the tracking ref is fresh: + +- A local ``, if one exists (`git branch --list `), must not be + **ahead** of origin: `git rev-list --count origin/..` must be 0. + If it isn't, stop — the PR is missing unpushed work, and only the user can decide + to push it. - If `/merge-` is left over from an earlier run, clear it with `git worktree remove --force` before adding it again. ```bash -git fetch origin main git rev-parse origin/ # print this SHA — the push lease below pins it git worktree add /merge- origin/ --detach cd /merge- && git rebase origin/main && @@ -61,7 +67,8 @@ Spawn one subagent with the Agent tool, `model: "fable"`. Tell it the worktree directory from step 1 (subagents start in the session cwd, not yours — an untold reviewer diffs the wrong tree and reports nothing), the PR number and title, and the target: `git diff origin/main...HEAD`. You are the author of the fixes; it is the -reviewer — a model reviewing its own fixes is not an adversarial review. +reviewer — a model reviewing its own fixes is not an adversarial review, and the +reviewer reports only: instruct it to modify nothing in the worktree. Instruct it to hunt for reasons this should **not** land ("looks good to me" is a failed review), covering both defects — correctness, security, regressions, error @@ -75,27 +82,32 @@ Findings come back as `{file, line, severity: blocking|nit, claim, failure_scena Adversarial reviewers over-report — that's the cost of telling them to attack. Confirm each finding against the actual code before acting, and say what you discarded and why. Fix every surviving finding, `blocking` and `nit` alike, then -`bun run ci` (needs Docker for the LocalStack e2e lane), commit, push, and re-run -step 2 on the new diff. **At most 3 rounds** — if findings still survive, stop and -report. +`bun run ci` (needs Docker for the LocalStack e2e lane), commit, and push with a +plain fast-forward — `git push origin HEAD:` — which succeeds because the +remote sits at the head you just rebased or fixed. If that push is rejected, +something else touched the branch: stop and report; never re-lease or force here. +Then re-run step 2 on the new diff. **At most 3 rounds** — if findings still +survive, stop and report. ## 4. Merge the reviewed tree ```bash cd /merge- && git rev-parse HEAD # the reviewed SHA — print it gh pr checks --watch && - gh pr merge --merge --match-head-commit -git push origin --delete + gh pr merge --merge --match-head-commit && + git push origin --delete cd && git worktree remove /merge- ``` -If `gh pr checks` fails, stop and report — never merge red. The `&&` is the only -thing enforcing that: an admin token can merge over a failed required check here. -(`gh pr checks` also errors if GitHub hasn't registered the run yet — retry a few -times before believing it.) `--match-head-commit` refuses the merge if anything — -say, another agent session — touched the branch after the review; it pins the -branch head, not `main`. The final `cd` out matters too: removing the worktree -you're standing in strands the shell in a deleted directory. +If the checks or the merge itself fail, stop and report — never merge red, and +never delete the branch of an unmerged PR. The `&&` chain is the only thing +enforcing either: an admin token can merge over a failed required check here, and +an unchained delete would discard the very commits a `--match-head-commit` refusal +just protected. (`gh pr checks` also errors if GitHub hasn't registered the run +yet — retry a few times before believing it.) `--match-head-commit` refuses the +merge if anything — say, another agent session — touched the branch after the +review; it pins the branch head, not `main`. The final `cd` out matters too: +removing the worktree you're standing in strands the shell in a deleted directory. Afterward, tell the user if a local checkout of `` exists anywhere (`git worktree list`, `git branch`) — the remote it tracked is gone and its From b9e86be3f60a4c00aeb37a5d9d64a0be8dbc7113 Mon Sep 17 00:00:00 2001 From: Peter Koomen Date: Sun, 16 Aug 2026 14:04:18 -0700 Subject: [PATCH 7/7] Polish two round-3 nits Guard the leftover-worktree removal against discarding a crashed run's unpushed commits, and widen the cd-rule carve-out to everything after the step-4 cleanup. Co-Authored-By: Claude Fable 5 --- .claude/skills/merge/SKILL.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.claude/skills/merge/SKILL.md b/.claude/skills/merge/SKILL.md index 1a05265..738f128 100644 --- a/.claude/skills/merge/SKILL.md +++ b/.claude/skills/merge/SKILL.md @@ -22,7 +22,8 @@ fresh shell; print the value, then substitute the literal into later commands. A start every Bash call after step 1 by `cd`-ing into the scratch worktree by literal path — cwd doesn't reliably persist either, and a command that lands in the user's checkout instead would build, commit, or push the wrong tree. (One exception: the -final cleanup in step 4 must run from outside the worktree.) +final cleanup in step 4, and everything after it, runs from the primary checkout — +the worktree is gone by then.) ## 1. Rebase in a scratch worktree @@ -39,8 +40,11 @@ Two preflights, now that the tracking ref is fresh: **ahead** of origin: `git rev-list --count origin/..` must be 0. If it isn't, stop — the PR is missing unpushed work, and only the user can decide to push it. -- If `/merge-` is left over from an earlier run, clear it with - `git worktree remove --force` before adding it again. +- If `/merge-` is left over from an earlier run, confirm it holds + nothing unpushed — `git -C status --porcelain` empty and its HEAD reachable + from `origin/` — then `git worktree remove --force` it (or, if only the + registration survives, `git worktree prune`). If it does hold unpushed work, stop + and report rather than discard it. ```bash git rev-parse origin/ # print this SHA — the push lease below pins it