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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -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`.
|
||||
|
||||
@@ -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 <n>`, 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
|
||||
|
||||
@@ -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 <n> --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: <path>: 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
|
||||
|
||||
Reference in New Issue
Block a user