From 72f0c63fff3560e5f419b537eed756a3786d3105 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 23:33:53 -0700 Subject: [PATCH 1/3] docs: one home for the review rules; pin gh/main; audit prose fixes improvise-28ri: -d skip only silences "Is a directory" on the skills' symlinks, which git ls-files lists as files. grep -r never follows them, so nothing was ever read twice. The "spells no id" sentence now says what the recipe actually cites. improvise-oga4: the review-reply and head-SHA rules live only in agent-rules "GitHub" (plus the after-merge clause moved there); AGENTS.md points to it, and the code-review skill names it. An ADR's "Approved" is an extra gate on top of CI, every review answered and the adversarial review. improvise-diho: agent-rules rule 7, pin gh/main to a SHA at task start. Also: build a test-name list in a fresh CARGO_TARGET_DIR, because git archive's commit mtimes make a reused one return a stale list. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .agents/skills/code-review/SKILL.md | 1 + .agents/skills/wave/agent-rules.md | 22 +++++++++++++--- AGENTS.md | 41 +++++++++++++++++++---------- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/.agents/skills/code-review/SKILL.md b/.agents/skills/code-review/SKILL.md index 618d30c2..6512d580 100644 --- a/.agents/skills/code-review/SKILL.md +++ b/.agents/skills/code-review/SKILL.md @@ -28,6 +28,7 @@ them here would let the copy drift: | `context/repo-map.md` | The "where", plus **numbered gotchas** — read the ones your diff touches | | `prolog-sessions/kb.pl` | Executable knowledge: `invariant/2`, `doctrine/2`, `forbidden/2`, `decision/3`, and prior `work_done/3` | | `AGENTS.md` | Working rules: bug-first tests, `Option`/`Result`/`Box` at the edges, merges over rebases | +| `.agents/skills/wave/agent-rules.md`, "GitHub" | The reviewer's rule: what to read before your first comment, how to answer each existing review, and what every review you post must state. Read it before reviewing a PR | `kb.pl` is the highest-value and least-obvious of these. Grep it before reviewing: `grep -n "invariant(\|doctrine(\|decision(" prolog-sessions/kb.pl`. diff --git a/.agents/skills/wave/agent-rules.md b/.agents/skills/wave/agent-rules.md index 7a4d3f52..7358f704 100644 --- a/.agents/skills/wave/agent-rules.md +++ b/.agents/skills/wave/agent-rules.md @@ -32,9 +32,17 @@ not in a second copy. 6. **The Beads "Session Completion" block near the end of `AGENTS.md` does not set your git policy.** This repository does not opt in to the block's "team-maintainer" profile. On a wave branch that profile's pull and bare - `git push` would be refused (rule 4), or worse. Rules 1–5 are your git - policy. The block is bd's and bd regenerates it whole, so it is not edited - by hand: `AGENTS.md`, "Repository policy over the Beads blocks" (improvise-x1uk). + `git push` would be refused (rule 4), or worse. Rules 1–5 and 7 are your + git policy. The block is bd's and bd regenerates it whole, so it is not + edited by hand: `AGENTS.md`, "Repository policy over the Beads blocks" (improvise-x1uk). +7. **Pin `gh/main` to a literal SHA when the task starts.** Run + `git rev-parse gh/main` once. Use that SHA, not the ref name, for every + reset, merge-base and diff. Remote refs are shared by every worktree of the + repository, so `gh/main` moves whenever another session fetches. An agent + ran `git reset --soft gh/main` mid-task and staged a reverse diff of #196. + It noticed only because it read `git status` (improvise-diho). Re-resolve + only on purpose, for example when you mean to merge a newer main. Rule 3's + check before a PR is such a time. ## Commit early: a worktree is not durable storage @@ -113,6 +121,12 @@ out on purpose: check that no test *disappeared*. Diff test **names**, not counts: a textually clean merge can delete a test module with no conflict marker, and the other side's new tests mask the loss in a count. +- **Build each side's test list in a fresh `CARGO_TARGET_DIR`, every time.** If + a parent's tree comes from `git archive`, a reused target dir gives a wrong + answer, not just a slow one. Archive stamps every file with its commit time, + so cargo judges the sources older than the last build and lists the previous + tree's tests. On #188 that made three of the branch's own tests look + merge-only. - **The shell guard refusing a compound command means NEITHER part ran.** Split the command. After a refusal, re-check the state it was meant to produce. - **Re-run the full suite after reverting an injected fault.** A revert applied @@ -200,6 +214,8 @@ text for any of these in your hand-back. `./scripts/pr-reviews `, unpiped. Reply on its thread to every Codex review: fixed (name the commit), declined (say why), or already one of our tickets (name it). Every review you post states the head SHA it reviewed. +- **Answer a review even if the PR has already merged.** Reviews arrive late, + and the thread is still where the next reader looks. - **Do not merge, and do not close beads.** Your part ends at an open PR plus your hand-back. What "done" means is in `AGENTS.md`, "Repository policy over the Beads blocks". Avoid stacked PRs. If you must stack, never let the base diff --git a/AGENTS.md b/AGENTS.md index 8a11780c..db781f6c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -184,8 +184,8 @@ earlier head, and the finding may already be fixed — verify rather than assume in either direction. Reviews arrive **as the owner's quota allows**, so absence is not evidence. -Wait where a review is plausible, and when one is found, answer it on the -thread even if the PR has already merged: that is where the next reader looks. +Wait where a review is plausible. How a review is answered once found, by the +author and by a reviewer, is `.agents/skills/wave/agent-rules.md`, "GitHub". **Copilot is not reviewing this repository, and the request is a silent no-op.** Measured 2026-09-20: PRs #12, #21 and #27 carry 2, 8 and 4 Copilot reviews; #61, @@ -231,8 +231,8 @@ normally.) Across four PRs Copilot has found genuine bugs and no false positives, so its findings are worth the loop. **But judge each one against the repo's doctrine -rather than applying it reflexively**, and when you decline, reply on the thread -with the reason. The distinction that has mattered: a finding can be *correct* +rather than applying it reflexively**. How a declined finding is answered is +agent-rules, "GitHub". The distinction that has mattered: a finding can be *correct* and still be wrong to fix in that PR. On #12, a non-ASCII `truncate` panic was applied (byte-identical on ASCII, so it cost no evidence) while a hint-line padding bug in the same file was declined — fixing it would have rewritten the @@ -251,21 +251,27 @@ is to branch from `gh/main`, cherry-pick, open a follow-up PR, and delete the stray re-creation. Check `gh pr view --json state` *before* pushing a fix to a PR you have not looked at in a few minutes, rather than after. -Merge only once a review has passed — an adversarial agent's while Copilot is -absent, and it must name the head SHA it reviewed. Prefer a **merge commit** — +Merge only once a review of the PR's current head has passed — an adversarial +agent's while Copilot is absent. What a review must state is agent-rules, +"GitHub". Prefer a **merge commit** — `AGENTS.md` says merges over rebases, and a PR's commits are individually meaningful. ### ADR pull requests -An ADR (`context/adr/`) merges on the user's approval, not on a review. The +An ADR (`context/adr/`) merges on the user's approval, **in addition to** green +CI, every review answered, and a passing adversarial review ("Shipping a +wave"). The approval is an extra gate. It replaces none of the others. The steps, modelled on PR #122 (ADR 004): 1. The PR proposes the ADR with its **open questions numbered**. 2. The user answers them on the PR. 3. The answers are folded into the ADR, in a section headed **"What the user decided"**. -4. The PR merges only after the user comments **"Approved"**. +4. The PR merges only after the user comments **"Approved"**, and only while + the other gates still hold. "Read the PR's reviews before merging it" still + applies after the approval. On #122 the user's own review landed as the PR + merged. ### Never hand-edit `.beads/issues.jsonl` @@ -296,7 +302,7 @@ bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \ `git ls-files` picks the files, and grep only reads them. The file list is the tracked files plus the untracked ones, minus anything git ignores. That leaves -out three things a `grep -r` over the same roots reads: +out two things a `grep -r` over the same roots reads: - **Build output.** After `./harness/build.sh`, `harness/serve/` holds rendered copies of `context/`, and `grep -r` reported three dead ids from them @@ -304,9 +310,14 @@ out three things a `grep -r` over the same roots reads: - **Other agents' worktrees.** In the orchestrator's checkout `.claude/worktrees/` holds every agent's worktree, dozens of copies of the repository with their unpushed work. -- **The skills twice.** `.claude/skills/*` are symlinks to directories under - `.agents/skills/`. `-d skip` passes over them, so the skills are read once, - through `.agents`. + +`-d skip` is there for the skills' symlinks, and it changes no answer. The +`.claude/skills/*` entries are symlinks to directories under `.agents/skills/`. +`git ls-files` lists each one as a file, and grep, handed a symlink to a +directory, prints `grep: : Is a directory` on stderr. `-d skip` silences +those lines. The skills are read once, through `.agents`, with or without the +flag: neither BSD nor GNU `grep -r` follows a symlink it meets while recursing, +so no id is ever counted twice. Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b` and finds a fraction of the ids. @@ -346,8 +357,10 @@ real defect through: added to the exclusion list in the same commit**, or it arrives as a dead id that no amount of `bd` archaeology explains. -Note the recipe's own prose deliberately spells no live or dead id: a document -that lists ids as examples makes itself a source of audit noise. +The recipe's own prose spells no id as an *example*, because a document that +lists example ids makes itself a source of audit noise. The ids it does cite, +improvise-8haj and improvise-oujb, are the tickets behind two of the rules +above. They resolve, and the audit checks them like any other. One environment note: in a worktree-isolated agent the shell guard refuses a loop that passes a runtime-computed argument to `bd`, so the recipe above From 6ccad4b80d3f68ff3a01fc2469db850d06b4b40a Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 07:40:13 -0700 Subject: [PATCH 2/3] docs: only the user's explicit approval accepts or merges an ADR The user's ruling: "An ADR isn't approved until I say it is." CI, the reviews and the adversarial review stay required as gates, but none of them is approval. SKILL.md's "not on a review" pointer is reworded to match. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .agents/skills/wave/SKILL.md | 5 +++-- AGENTS.md | 13 ++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/.agents/skills/wave/SKILL.md b/.agents/skills/wave/SKILL.md index f52ce28a..43152160 100644 --- a/.agents/skills/wave/SKILL.md +++ b/.agents/skills/wave/SKILL.md @@ -80,8 +80,9 @@ defect is a test that passes. "pending". Dispatch an adversarial reviewer and give it the head SHA, the base, and a test-merge, so it reviews the tree that will exist. Point it at `agent-rules.md`, "GitHub", for what a reviewer reads and answers first. -- An ADR PR merges on the user's approval, not on a review: - `AGENTS.md`, "ADR pull requests". +- Only the user's explicit approval accepts or merges an ADR PR. The review + gates still apply, and none of them is approval: `AGENTS.md`, "ADR pull + requests". - A re-review is of the **delta**, at a named SHA. A review of a superseded commit reads as a pass. - Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`. Close diff --git a/AGENTS.md b/AGENTS.md index db781f6c..89b29d2f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -259,17 +259,20 @@ meaningful. ### ADR pull requests -An ADR (`context/adr/`) merges on the user's approval, **in addition to** green -CI, every review answered, and a passing adversarial review ("Shipping a -wave"). The approval is an extra gate. It replaces none of the others. The -steps, modelled on PR #122 (ADR 004): +**Only the user's explicit approval accepts an ADR (`context/adr/`), and only +it lets an ADR PR merge.** "An ADR isn't approved until I say it is" (the +user). Green CI, every review answered and a passing adversarial review +("Shipping a wave") are still required. They are gates, never approval: an ADR +PR that passes all of them is still unapproved and does not merge. The steps, +modelled on PR #122 (ADR 004): 1. The PR proposes the ADR with its **open questions numbered**. 2. The user answers them on the PR. 3. The answers are folded into the ADR, in a section headed **"What the user decided"**. 4. The PR merges only after the user comments **"Approved"**, and only while - the other gates still hold. "Read the PR's reviews before merging it" still + the gates above still hold. Nobody else's comment or review stands in for + it. "Read the PR's reviews before merging it" still applies after the approval. On #122 the user's own review landed as the PR merged. From 41aa4073f1abc7312a703d559d11c2aafb1e6d3a Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 07:53:01 -0700 Subject: [PATCH 3/3] docs: -d skip reason measured; the ADR exception at both merge rules improvise-28ri, from the #211 review at 6ccad4b: the recipe's grep has no -r, so the skills' symlinks arrive as directory operands and are never read. -d skip only silences "Is a directory", and sort -u would absorb a duplicate anyway. Measured with BSD grep 2.6.0 and GNU grep 3.12: 3092 matches either way, 462 distinct ids. GNU grep -r follows those symlinks (3107), so -r must not be added. improvise-oga4: "Shipping a wave"'s opening and "Repository policy over the Beads blocks" now point at the ADR exception. The wave SKILL.md pointer is unwrapped, so a grep for "ADR pull requests" finds it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .agents/skills/wave/SKILL.md | 6 +++--- AGENTS.md | 28 ++++++++++++++++------------ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/.agents/skills/wave/SKILL.md b/.agents/skills/wave/SKILL.md index 43152160..43787a3b 100644 --- a/.agents/skills/wave/SKILL.md +++ b/.agents/skills/wave/SKILL.md @@ -80,9 +80,9 @@ defect is a test that passes. "pending". Dispatch an adversarial reviewer and give it the head SHA, the base, and a test-merge, so it reviews the tree that will exist. Point it at `agent-rules.md`, "GitHub", for what a reviewer reads and answers first. -- Only the user's explicit approval accepts or merges an ADR PR. The review - gates still apply, and none of them is approval: `AGENTS.md`, "ADR pull - requests". +- Only the user's explicit approval accepts an ADR, and lets its PR merge. The + review gates still apply, and none of them is approval: + `AGENTS.md`, "ADR pull requests". - A re-review is of the **delta**, at a named SHA. A review of a superseded commit reads as a pass. - Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`. Close diff --git a/AGENTS.md b/AGENTS.md index 89b29d2f..6e66e82c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -131,9 +131,10 @@ and watch the check fail. **Do not push `main` to `gh/main`.** Each agent's work reaches GitHub as its own branch and its own pull request, and lands on `main` by being merged there, once -CI is green and an adversarial review agent has passed the PR's head SHA. The -orchestrator does not integrate agent branches locally: its `main` only -fast-forwards to `gh/main` after a merge. The procedure is +CI is green and an adversarial review agent has passed the PR's head SHA. An ADR +PR also needs the user's explicit approval, and nothing else stands in for it: +"ADR pull requests" below. The orchestrator does not integrate agent branches +locally: its `main` only fast-forwards to `gh/main` after a merge. The procedure is `.agents/skills/wave/SKILL.md`, "Shepherding". `origin` and `gitea` are mirrors and take `main` directly. @@ -315,12 +316,14 @@ out two things a `grep -r` over the same roots reads: repository with their unpushed work. `-d skip` is there for the skills' symlinks, and it changes no answer. The -`.claude/skills/*` entries are symlinks to directories under `.agents/skills/`. -`git ls-files` lists each one as a file, and grep, handed a symlink to a -directory, prints `grep: : Is a directory` on stderr. `-d skip` silences -those lines. The skills are read once, through `.agents`, with or without the -flag: neither BSD nor GNU `grep -r` follows a symlink it meets while recursing, -so no id is ever counted twice. +`.claude/skills/*` entries are symlinks to directories under `.agents/skills/`, +and `git ls-files` lists each one, so each reaches grep as a command-line +operand. The grep has no `-r`, so it never reads a directory operand. It prints +`grep: : Is a directory` on stderr instead, and `-d skip` only silences +those lines. The output goes through `sort -u`, so a duplicate could not change +the answer anyway. Measured with BSD grep 2.6.0 and GNU grep 3.12: 3092 matches +with or without the flag, 462 distinct ids. Do not add `-r`: GNU grep then +follows those symlinks and reads the skills a second time (3107 matches). Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b` and finds a fraction of the ids. @@ -392,9 +395,10 @@ regeneration. Repository policy lives here, outside them: - **Work is not done until it is in a pull request and that pull request has merged.** A pushed branch is not done, and neither is a green one. In a wave, the merge is the orchestrator's, after an adversarial review passes the head - SHA ("Shipping a wave"). So a wave agent's part ends at an open PR plus its - hand-back, and the ticket is done, and its bead closed, only when the merge - lands. + SHA ("Shipping a wave"). An ADR PR merges only on the user's explicit + approval as well ("ADR pull requests"). So a wave agent's part ends at an + open PR plus its hand-back, and the ticket is done, and its bead closed, only + when the merge lands. - **This repository does not opt in to the Beads "team-maintainer" profile.** So the pull (`git pull`, or `git pull --rebase` in bd's "full" profile) and the bare `git push` that the block lists for that profile are not