Merge pull request #201 from fiddlerwoaroof/docs/agents-md-wave

AGENTS.md agrees with the wave skill; bd block superseded; audit scans .agents; gates from ci.yml
This commit is contained in:
Ed Langley
2026-09-22 23:29:37 -07:00
committed by GitHub
4 changed files with 179 additions and 68 deletions
+9 -3
View File
@@ -37,7 +37,8 @@ it must cross. Two agents adding a workspace member will still collide in
**Point the agent at [`agent-rules.md`](agent-rules.md) and tell it to read it in
full first.** That file is the agent's standing rules: base and push mechanics,
the gate list, the claim, the vacuity shapes, evidence, the files it must not
write, GitHub conventions, the hand-back and disk cleanup. Do not restate them
write, GitHub conventions (including how authors and reviewers answer reviews
on the PR thread), the hand-back and disk cleanup. Do not restate them
in the brief. A restated rule drifts from its source, and the brief is the copy
nobody corrects. The brief adds only what varies per wave:
@@ -77,10 +78,15 @@ defect is a test that passes.
- Review before merging. Copilot's quota may be exhausted and its request is a
**silent no-op**; an absent review and an unrequested one both read as
"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.
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".
- 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`.
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`. Close
the bead only once the merge has landed. That is when the work is done:
`AGENTS.md`, "Repository policy over the Beads blocks".
- Anything posted to GitHub goes out under the owner's account: open every
comment with a line naming the model and saying an agent wrote it. That
wording is agent-neutral because this one file serves Claude and Codex
+51 -15
View File
@@ -29,12 +29,12 @@ not in a second copy.
5. **Before pushing a fix to an existing PR, check `gh pr view <n> --json
state`.** GitHub deletes a merged PR's branch. A push to it then prints
`* [new branch]` and lands nowhere. Why: `AGENTS.md`, "Shipping a wave".
6. **The "Session Completion" block near the end of `AGENTS.md` does not apply
to you.** It says `git merge origin/main` and then a bare `git push`. On a
wave branch the first is the #145 trap and the second is refused (rule 4).
Rules 1–5 replace it. The block sits between bd's generated
`BEGIN/END BEADS INTEGRATION` markers, so it is not edited by hand
(improvise-x1uk).
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).
## Commit early: a worktree is not durable storage
@@ -47,14 +47,33 @@ deleted worktree. Tidying history before the PR is fine; losing it is not.
## Gates
```
direnv exec . cargo test --workspace --no-fail-fast
direnv exec . cargo clippy --workspace --all-targets -- -D warnings
direnv exec . ./scripts/fmt-workspaces.sh # NOT cargo fmt --all; exit 2 = the gate did not run
direnv exec . python3 scripts/check-comment-runs.py --check-evidence
direnv exec . ./scripts/checkers.sh
direnv exec . cargo clippy --workspace --all-targets -- -D warnings
direnv exec . cargo test --workspace --no-fail-fast
direnv exec . python3 scripts/check-comment-runs.py --self-test
direnv exec . python3 scripts/check-comment-runs.py --check-evidence # also answers the comment cap
direnv exec . python3 scripts/check-ci-graph.py --self-test
direnv exec . python3 scripts/check-ci-graph.py
direnv exec . ./scripts/checkers.sh # every suite in scripts/, its own self-test included
direnv exec . tpl prolog-sessions/kb.pl -g "(kb_consistent -> halt(0) ; (kb_report, halt(1)))"
direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
```
This list is the `test`, `js` and `kb` jobs of `.github/workflows/ci.yml`,
step for step, and this file is the one place it is written down. Change it in
the same PR that changes CI. The other CI jobs are conditional, below, or left
out on purpose:
- **`wasm`**: when you touch a crate it checks (next bullet).
- **`harness-lint`**: when you touch `harness/opfs-contract`, run
`cargo clippy --target wasm32-unknown-unknown --all-targets -- -D warnings`
from that directory. Its format half is already `fmt-workspaces.sh`.
- **`build` and `browser`**: when you touch `web/`, `harness/`, or the surface
the page calls in `improvise-wasm-client` or `improvise-worker-server`
(third bullet).
- **`pages-deploy`** is left out. It deploys, and it runs only on a push to
`main`. What guards it locally is `check-ci-graph.py`, which is in the list.
- **Never `cargo fmt --all`.** It reaches only **one** of this repo's two
workspace roots, because `harness/opfs-contract` declares its own
`[workspace]`. The obvious repair is worse: `cargo fmt -p opfs-contract` from
@@ -70,10 +89,17 @@ direnv exec . ./scripts/node-test.sh 'web/tests/*.test.js'
- **`--no-fail-fast`**, because plain `cargo test --workspace` stops at the
first failing binary. Use it for any claim about blast radius.
- If you touch a browser-bundle crate (core, formula, io, protocol, command,
wasm-client, worker-server), also run
wasm-client, worker-server) or `improvise-gp`, also run
`cargo check --target wasm32-unknown-unknown -p <crate>` **for each crate**.
CI checks each one separately, so feature unification cannot hide a leak.
- If you touch `web/` or `harness/`, run the Playwright suite in `harness/`.
- If you touch `web/`, `harness/` or the page's wasm surface, run
`./harness/build.sh`, then
`./scripts/node-test.sh 'web/tests/bundle/*.test.js'`, then `npm ci` and
`npx playwright test` in `harness/`. The bundle tests sit outside the
`web/tests/*.test.js` glob and read the built `web/pkg`, so the standard list
never runs them. They failed only in CI once. A fresh worktree has no
`harness/node_modules`, and Playwright then reports a missing
`@playwright/test`, which reads like a broken checkout.
- **If you change the grid's row or column count, also grep
`harness/fixtures/*.improv` and `harness/tests/*.spec.js`.** On PR #176 a
Playwright spec pinned the old collapsed-group row count. `cargo test` cannot
@@ -165,9 +191,19 @@ text for any of these in your hand-back.
They name the agent and the session, so this shared file cannot hold them.
- Read reviews with `./scripts/pr-reviews <n>`, and do not pipe it. How to tell
a ChatGPT Codex review from ours, and why Copilot is not coming: `AGENTS.md`,
"Shipping a wave". Answer each Codex review on its thread.
- **Do not merge, and do not close beads.** Avoid stacked PRs. If you must
stack, never let the base branch be deleted.
"Shipping a wave".
- **As a PR's author, reply on the PR to every review finding you address.**
Name the commit that fixes it, or say why you declined it. Putting it only in
your hand-back to the orchestrator is not enough: the PR thread is where the
next reader looks.
- **As a reviewer, read the existing reviews first**, with
`./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.
- **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
branch be deleted.
## Hand-back, per ticket
+116 -46
View File
@@ -57,9 +57,9 @@ cp -rf source dest # NOT: cp -r source dest
## The Wave Workflow
Most work here is done by **waves**: an orchestrator chunks the open backlog,
fans several agents out into their own git worktrees, merges what comes back,
reviews it, and pushes the wave as one PR. This section is what that costs to
get right. Everything in it was learned by getting it wrong first.
fans several agents out into their own git worktrees, and shepherds each
agent's pull request through review to a merge on GitHub. This section is what
that costs to get right. Everything in it was learned by getting it wrong first.
### Who owns what
@@ -75,11 +75,13 @@ badly:
Wave agents **read** all three freely (`bd show`, `bd ready`, grep `kb.pl`) and
**write** none of them. They report proposed repo-map lines and `kb.pl` facts as
*text* in their hand-back, and the orchestrator applies them — batched, at the
end of the wave, after the review. **`kb.pl` is written by `scripts/kb add|replace|rm` and by nothing else** — not
*text* in their hand-back, and the orchestrator applies them. Repo-map lines
are batched at the end of the wave, after the review, because they conflict.
`kb.pl` facts are not batched: the orchestrator writes each one when it arrives.
**`kb.pl` is written by `scripts/kb add|replace|rm` and by nothing else** — not
the Prolog MCP, not an editor. Every write is gated on `kb_consistent` against
the candidate, so a bad delta leaves the file untouched. Facts do not need
batching: write one when you learn it. There is a `kb` skill; read it before
the candidate, so a bad delta leaves the file untouched, which is why a fact
can be written the moment it is learned. There is a `kb` skill; read it before
your first write. Reading is unaffected — `loadSession` at session start is
still how a session gets the model in hand.
@@ -127,18 +129,21 @@ and watch the check fail.
### Shipping a wave
**Do not push `main` to `gh/main`.** Work reaches GitHub as a branch and a pull
request, and lands on `main` by being merged there. The orchestrator still
merges agent branches into its *local* `main` to integrate and run the gates —
that is how conflicts get found early — but the push is a branch push, not a
`main` push. `origin` and `gitea` are mirrors and take `main` directly.
**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
`.agents/skills/wave/SKILL.md`, "Shepherding". `origin` and `gitea` are mirrors
and take `main` directly.
**So `origin/main` runs ahead of `gh/main`, and a branch destined for a GitHub
PR must merge `gh/main` — never `origin/main`.** This is a trap in this repo's
own topology rather than a slip: the two remotes are both spelled `main`, and
nothing about `git merge origin/main` looks wrong. The session-completion
protocol below literally says `git merge origin/main`, which is right for
`main` itself and wrong for a PR branch. It cost a review round on PR #145,
**Because the mirrors take `main` directly, `origin/main` can run ahead of
`gh/main`, and a branch destined for a GitHub PR must merge `gh/main` — never
`origin/main`.** This is a trap in this repo's own topology rather than a slip:
the two remotes are both spelled `main`, and nothing about
`git merge origin/main` looks wrong. bd's generated Session Completion block
once prescribed exactly that command (see "Repository policy over the Beads blocks" below).
It cost a review round on PR #145,
where four unmerged commits — including another PR's whole diff — were pulled
into a branch based on `gh/main` and had to be rebuilt by cherry-picking.
ChatGPT Codex caught it; nothing in CI could, because the result compiles and
@@ -153,15 +158,13 @@ finishing. It happened in this repo: 27 commits across six agent branches —
including a new crate, a new protocol type, and a type moving between crates —
went straight to `main` unreviewed, while `AGENTS.md` already said not to. If
you notice a batch has bypassed the loop, the cheap repair is a **review-only
PR**: base a branch at the last PR merge, open a PR with `main` as the head, let
Copilot review the diff, and land the findings as an ordinary PR afterwards. It
PR**: base a branch at the last PR merge, open a PR with `main` as the head,
have an adversarial review agent review the diff, and land the findings as an
ordinary PR afterwards. It
cannot be merged — the commits are already in — so close it once its findings
are filed or fixed. Rewinding the default branch to fake a mergeable PR is not
worth it on a repo with other collaborators.
One PR per wave, built by a **dedicated PR agent in its own worktree**, so waves
pipeline against it: the next wave runs while the PR absorbs review fixes.
**Read the PR's reviews before merging it.** This is separate from CI and from
the adversarial agent, and it has been missed: two ChatGPT Codex reviews landed
on merged PRs and were not read until afterwards, one of them a live P2
@@ -250,9 +253,20 @@ 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** —
`AGENTS.md` says merges over rebases, and a wave's commits are individually
`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
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"**.
### Never hand-edit `.beads/issues.jsonl`
bd manipulates that file and auto-exports to it. Editing it — or even
@@ -269,15 +283,37 @@ session before an agent tried one and found nothing. Read the ID back after
filing, and before pushing docs run the audit:
```bash
grep -rohE 'improvise-[a-z0-9]{3,8}\b' \
git ls-files -z --cached --others --exclude-standard -- \
context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \
.agents .claude \
| xargs -0 grep -d skip -ohE 'improvise-[a-z0-9]{3,8}\b' \
| sort -u \
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|side|web)$' \
| grep -vE '^improvise-(core|formula|io|protocol|command|gp|ws|worker|wasm|harness|registry|report|side|web)$' \
> /tmp/bd-audit-ids.txt
bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
| grep -iE 'not found|no such issue' || echo "all resolve"
| grep -E '^(Issue [^ ]+ not found|[Nn]o such issue)' || echo "all resolve"
```
`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:
- **Build output.** After `./harness/build.sh`, `harness/serve/` holds
rendered copies of `context/`, and `grep -r` reported three dead ids from them
that no tracked file contains (improvise-8haj).
- **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`.
Plain `git grep` is not a substitute: on macOS its `-E` silently ignores `\b`
and finds a fraction of the ids.
The last line matches at the start of a line because `bd show` prints each
ticket's body indented, and a body can itself contain the words "not found".
Three things the earlier version of this recipe got wrong, each of which let a
real defect through:
@@ -293,7 +329,8 @@ real defect through:
- **The exclusion list named only `core` and `wasm`**, so every other crate
name would have been reported missing. The exclusions above also cover the
non-ticket tokens this repo really contains: a package name, a temp file
name, and the prose word "improvise-side". **They are anchored `^…$` for the
name, the prose word "improvise-side", and the example crate name in the
`kb` skill's `crate(...)` fact. **They are anchored `^…$` for the
same reason the recipe is read as code**: unanchored, the `gp` alternative is
a *prefix* match, so a future real ticket whose id happens to begin `gp` is
silently excluded and the audit reports nothing while skipping it. An
@@ -327,8 +364,32 @@ in progress. Two duplicates were filed in this repo within minutes of the
hand-backs that prompted them. `bd search` the core concept *and* scan
in-progress issues first.
## Repository policy over the Beads blocks
<!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:ca08a54f -->
Everything between a `BEGIN BEADS …` marker and its `END` is generated by bd,
and bd replaces it whole. `bd init` and `bd setup factory|opencode|mux` rewrite
the integration block, and the last three call it "stale" and invite the
rewrite whenever bd's template has moved on. `bd setup codex` rewrites the
Codex block. So a hand edit inside the markers is silently reverted by the next
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.
- **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
instructions here. Work reaches `main` as "Shipping a wave" describes,
and a wave agent pushes with the explicit refspec in
`.agents/skills/wave/agent-rules.md`, "Base, branch, push".
- **The no-markdown-TODO and no-`MEMORY.md` rules have one exception:**
`context/design-principles.md` and `context/repo-map.md` are kept by hand
(the repo map by the orchestrator only; see "Who owns what").
<!-- BEGIN BEADS INTEGRATION v:1 profile:minimal hash:46cd31e7 -->
## Beads Issue Tracker
This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands.
@@ -344,35 +405,44 @@ bd close <id> # Complete work
### Rules
- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists (the exception being that you must maintain context/design-principles.md and context/repo-map.md)
- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists
- Run `bd prime` for detailed command reference and session close protocol
- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files (the exception being that you must maintain context/design-principles.md and context/repo-map.md)
- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files
**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/core-concepts/sync-concepts.md for details and anti-patterns.
## Agent Context Profiles
The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions.
- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands.
- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise.
- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins.
## Session Completion
**When ending a work session**, you MUST complete ALL steps below. Work is NOT complete until `git push` succeeds.
This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions.
**MANDATORY WORKFLOW:**
1. **File issues for remaining work** - Create issues for anything that needs follow-up
1. **File issues for remaining work** - Create beads for anything that needs follow-up
2. **Run quality gates** (if code changed) - Tests, linters, builds
3. **Update issue status** - Close finished work, update in-progress items
4. **PUSH TO REMOTE** - This is MANDATORY:
4. **Handle git/sync by active profile**:
```bash
git merge origin/main
# Conservative/minimal/default: report status and proposed commands; wait for approval.
git status
# Team-maintainer opt-in only, unless current instructions forbid it:
git pull
bd dolt push
git push
git status # MUST show "up to date with origin"
git status
```
5. **Clean up** - Clear stashes, prune remote branches
6. **Verify** - All changes committed AND pushed
7. **Hand off** - Provide context for next session
5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step
**CRITICAL RULES:**
- Work is NOT complete until `git push` succeeds
- NEVER stop before pushing - that leaves work stranded locally
- NEVER say "ready to push when you are" - YOU must push
- If push fails, resolve and retry until it succeeds
**Critical rules:**
- Explicit user or orchestrator instructions override this Beads block.
- Do not commit or push without clear authority from the active profile or the current user request.
- If a required sync or push is blocked, stop and report the exact command and error.
<!-- END BEADS INTEGRATION -->
<!-- BEGIN BEADS CODEX SETUP: generated by bd setup codex -->
+3 -4
View File
@@ -311,10 +311,9 @@ candidate for the store.
- **And an argument must not live only in a commit message.** A message is
write-once and unauditable: it cannot be corrected without rewriting merged
history, and no check in this repo can read one — the ID audit greps
`context`, `README.md`, `AGENTS.md`, `CLAUDE.md`, `crates`, `src`, `tests`,
`examples`, `web`, `harness` and `docs`, and a commit message is in none of
them. So a reason recorded only there is a reason nothing can ever find wrong.
history, and no check in this repo can read one — the ID audit
(`AGENTS.md`, "Cite a ticket ID you have read back") greps files in the
tree, and a commit message is in none of them. So a reason recorded only there is a reason nothing can ever find wrong.
When one turns out to be wrong, correct `context/` and say in `context/` that
the message is superseded; that is the fix rather than a compromise. PR #51's
`7d25762` is the worked example, corrected by #56.