Merge remote-tracking branch 'gh/main' into fix/yf7p-cursor-is-the-only-context

This commit is contained in:
Edward Langley
2026-09-22 21:23:54 -07:00
10 changed files with 474 additions and 209 deletions
+39 -27
View File
@@ -7,6 +7,8 @@ description: Dispatch parallel agents to work through open beads and shepherd ea
An orchestrator chunks the open backlog, fans agents out, reviews what comes
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
This file is the brief checklist's one home, and `agent-rules.md` beside it
holds the rules each agent follows.
## Every agent gets its own worktree. No exceptions.
@@ -17,9 +19,8 @@ here. If you discover it mid-wave, stop every agent, have each create a worktree
and copy across **only files it can prove it touched**, then `git checkout --`
those files in the primary tree.
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
allowed — it reads like the opposite.
In the worktree `.envrc` must be allowed, or `bd` fails confusingly:
`agent-rules.md`, "Discipline per ticket".
## Partitioning
@@ -33,32 +34,41 @@ it must cross. Two agents adding a workspace member will still collide in
## What every brief must carry
**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
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:
1. **The expected base commit, by name.** Worktrees do not reliably branch from
HEAD — one was created three merges behind and only noticed because it was
told to check.
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
count, harness counts, node counts.
3. **Its ground, and the ground that is not.**
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
Agents read them freely and report proposed text; the orchestrator applies it.
5. **The commit trailers and PR footer**, spelled out.
6. **Do not push to `main`, do not merge.** The orchestrator merges.
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
*what could not be verified and why*, proposed repo-map/kb text, and **what it
learned that the ticket did not predict** — consistently the most valuable
part, and omitted unless asked for explicitly.
HEAD. One was created three merges behind and was only noticed because it
was told to check. The agent checks against it: `agent-rules.md` rule 1.
2. **A reference baseline**, so "N passing" is checkable and a drop is
visible: Rust test count, harness counts, node counts, **and the machine**.
The same commit once measured `improvise-io`'s binary at 1.4s on one machine
and 3.7–5.2s on another, so a figure with no machine is a claim about the
suite, which it is not. The agent still measures its own.
3. **Its ground, and the ground that is not.** Name any orchestrator-only file
it is exceptionally allowed to edit.
4. **Its `BEADS_ACTOR` and its tickets.**
5. **The attribution line, commit trailers and PR footer**, spelled out.
They name the agent (Claude's line opens `_Posted by Claude (<model>)…_`)
and this session's URL, which is why the shared `agent-rules.md` cannot
hold them.
6. **An explicit ask for what the ticket did not predict.** `agent-rules.md`
lists it in the hand-back. Say it in the brief anyway: it is consistently
the most valuable part of a hand-back, and agents omit it unless asked.
## Method to require, because this repo's defect is a test that passes
When a hand-back teaches a new rule, add it to `agent-rules.md` in the same
wave, not to the next brief.
- Failing test first, then the fix.
- Every guard proved by **injecting the fault it names** and watching it fail
**by name**. An injection that does not compile reads as a clean run, so
require a positive signal the build ran.
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
blast radius.
- A textually clean merge can **delete tests** with no conflict marker, and
`cargo check` will not notice. After any merge that moves a test module, diff
test **names** — a count is masked by the other side's additions.
## Method to require
`agent-rules.md`, "Discipline per ticket" and "Gates": failing test first, a
rival injected and watched failing by name, `--no-fail-fast`, and test *names*
diffed after any merge. Hold the hand-back to it. This repo's characteristic
defect is a test that passes.
## Shepherding
@@ -72,7 +82,9 @@ it must cross. Two agents adding a workspace member will still collide in
commit reads as a pass.
- Merge on GitHub, then `git fetch gh && git merge --ff-only gh/main`.
- Anything posted to GitHub goes out under the owner's account: open every
comment with a line saying a Codex agent wrote it.
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
(`.claude/skills/wave` is a symlink to `.agents/skills/wave`).
## Between waves
+198
View File
@@ -0,0 +1,198 @@
# Standing rules for a wave agent
Every work-unit agent reads this **in full before its first command**. A brief
points here and adds only what is specific to its wave: the expected base
commit, the agent's ground, its `BEADS_ACTOR`, and its tickets. Every rule below
was bought by a failure.
This file holds each agent-side rule **once**. The brief checklist is
`SKILL.md` beside it. Where `AGENTS.md` ("The Wave Workflow") carries a longer
history, the rule here points there. Change a rule here, not in a brief, and
not in a second copy.
## Base, branch, push
1. **Verify your base against the commit your brief names, first.** Run
`git log --oneline -1` and compare. Worktrees do not reliably branch from
the orchestrator's HEAD: one was created three merges behind, and it was
only noticed because the agent was told to look.
2. **Branch from `gh/main`, never `main` or `origin/main`.** `git fetch gh &&
git checkout -b <branch> gh/main`. A worktree handed to you may already sit
on unpushed commits. Why: `AGENTS.md`, "Shipping a wave" (the two remotes
both spelled `main`, and the worktree-base vector).
3. **Before opening a PR, check `git diff --numstat gh/main...<branch>`.** Do
not use `gh pr view --json files`: that is the union of your commits' files,
not the effective diff.
4. **Push with an explicit refspec:** `git push gh HEAD:refs/heads/<branch>`. A
branch cut from `gh/main` tracks `gh/main`, so a bare push targets `main` and
is refused.
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).
## Commit early: a worktree is not durable storage
Commit a WIP as soon as your first test run is green, and again after every
meaningful step. Push your branch (rule 4) once it is worth keeping. An agent
finished a whole ticket with every gate green and lost all of it, uncommitted,
when its worktree was deleted in a disk cleanup. A local commit survives a
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 . ./scripts/node-test.sh 'web/tests/*.test.js'
```
- **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
the repo root exits **1**, *"not a member of the workspace"*, which looks
exactly like a real formatting failure (improvise-b4ks). Both spellings fail
in the direction that looks like success.
- **Run every gate under `direnv exec .`.** `./scripts/checkers.sh` scrubs the
environment to an allowlist, so a bare run drops `tpl` off `PATH` and reports
spurious failures.
- **Never pipe a gate.** `$?` must be the gate's own. And
`check-comment-runs.py --check-evidence` prints two lines with the comment
cap **first**, so `| tail -1` silently drops half of it.
- **`--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
`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/`.
- **Measure your own baseline** before changing anything, and report it with
the machine it was measured on. A figure from the brief is a reference, not
your baseline.
- **Merge `gh/main` and re-run every gate before calling a PR green.** Then
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.
- **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
to the wrong occurrence once shipped two faults past every other gate.
## Discipline per ticket
- **Claim first:** `export BEADS_ACTOR=<your actor>` in every shell, then
`bd update <id> --claim`. Exit 13 means another actor holds it. Pick another;
do not retry.
- **`bd` needs `.envrc` allowed in your worktree.** Without it you get a
**version mismatch**, not a clean failure. Run `direnv allow` in the worktree,
or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
`direnv status` reports `Found RC allowed 0`, and `0` means **allowed** (the
enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
- **Reproduce at HEAD before fixing.** A dozen tickets were stale-open in one
campaign, one fixed two days before it was filed. If it does not reproduce,
find the fixing commit, confirm with `git merge-base --is-ancestor <sha> HEAD`,
report the SHA, and close nothing.
- **Write a failing test first, then inject a rival and watch it fail.** Run
it; do not reason about it. Report *where* it failed, by case name. An
injection that does not compile reads as a clean run, so get a positive
signal that the build ran.
- **Check your test against every vacuity shape seen here:**
- an assertion the default state already satisfies;
- `f(x) == f(x)`: calling a pure `Cmd` twice against one immutable context;
- a disjunctive assertion whose other branch is the bug's own symptom;
- a rival that fails the *setup*, which proves the neighbouring question
(e.g. setup and subject share an `EffectKind`);
- a fixture that is right by accident (expected value 1, group at row 0,
bystander already on the relayed value);
- a fixture or stub that lacks the structure the hazard needs, or fails in a
wider shape than the real fault;
- hand-picked fixture cases where the set could be **enumerated**: four
vacuities in `fmt-workspaces-test.sh` were each a case nobody thought of
(improvise-90ea);
- a rendered-output test at a width where the thing asserted is never drawn;
- a fold test that batches differently from the real producer (a
`Binding::Sequence` is ONE effect vec);
- **the cursor at index 0**: "the item under the cursor" and "the first item"
are indistinguishable there, and that was four of five findings in one
review. Seat the cursor at a nonzero index of a list with at least two
elements;
- a determinism check against ONE context instance: every `HashMap` iterates
the same way twice. Build two equal contexts independently
(improvise-hzjo);
- two derivations of one fact where the fixture makes them agree (drawn vs
committed, edit text vs value, `1200` vs `1.23456789`). Use a value where
they disagree.
- **Record each bug in `context/test-evidence/<area>.md`**, keyed by test name
(module-qualify the name when the leaf is not unique). It is append-at-EOF, so
**expect a conflict**. Keep both sides and verify line by line against both
parents: `--check-evidence` cannot see a dropped entry.
- **Comment runs are at most two lines.** Reasoning goes in `context/`, not in
comments (`context/design-principles.md` §6).
- **If the acceptance criteria look wrong, argue it** rather than following
them silently. Three agents inverted theirs in one campaign, and all three
were right.
## Files you must not write
`.beads/**` (never commit `issues.jsonl`, never `git checkout` it),
`prolog-sessions/kb.pl`, `context/repo-map.md`: `AGENTS.md`, "Who owns what".
Also `AGENTS.md`, unless your brief grants you a named section. Report proposed
text for any of these in your hand-back.
## GitHub
- `gh` is shell-aliased to a different tool. Use
`nix shell nixpkgs#gh --command gh …`, and pass bodies with `-F body=@file`.
- **Everything posted goes out under the repository owner's account.** Open
every PR body and comment with the attribution line your brief gives you,
verbatim. It names your agent and model, which is how our posts are told
apart from an automated Codex review (`AGENTS.md`, "Shipping a wave").
- The attribution line, commit trailers and PR footer all come from your brief.
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.
## Hand-back, per ticket
Report:
- your branch, head SHA and PR number. Review is dispatched against the SHA;
- what changed, per ticket;
- the claim result;
- your measured baseline;
- whether the ticket reproduced (the SHA if it did not);
- the rival and where it failed;
- what you could not verify, and why;
- proposed repo-map and `kb.pl` text;
- **what the ticket did not predict**, consistently the most valuable part.
## Disk
Each worktree's `target/` grows to 3–6G, and a full disk (ENOSPC) once stopped
every agent. **When you hand back, delete your own build output, and only
yours.**
Run `pwd` and `git rev-parse --show-toplevel` first, and read the answer. Then
delete by **literal absolute path**, typed out:
`rm -rf /Users/.../.claude/worktrees/agent-<you>/target`.
**Never interpolate a computed path into `rm -rf`.** One agent's worktree had
been removed, so `$(git rev-parse --show-toplevel)` printed nothing and the
command became `rm -rf "/target"`. A guarded one-liner was tried, but the
worktree shell guard refuses it as too complex, which pushed agents back to an
unguarded form. A literal path is the version that is both allowed and safe.
Keep your branch and any uncommitted work. Delete only `target/` (and
`harness/opfs-contract/target` if you built it). If you see ENOSPC, stop and
report rather than deleting anything outside your worktree.
+1
View File
@@ -0,0 +1 @@
../../.agents/skills/wave
-83
View File
@@ -1,83 +0,0 @@
---
name: wave
description: Dispatch parallel agents to work through open beads and shepherd each to a merged PR. Use when an orchestrator is running a wave, partitioning the backlog, or writing a brief for a work-unit agent.
---
# Running a wave
An orchestrator chunks the open backlog, fans agents out, reviews what comes
back, and merges. `AGENTS.md` has the history; this is the operating procedure.
## Every agent gets its own worktree. No exceptions.
Dispatch with `isolation: "worktree"`. Without it every agent edits the **same**
working tree: one `git status` shows four agents' half-finished files, any commit
sweeps up the others' work, and any `git checkout` destroys it. This has happened
here. If you discover it mid-wave, stop every agent, have each create a worktree
and copy across **only files it can prove it touched**, then `git checkout --`
those files in the primary tree.
In the worktree: `direnv allow`, or `bd` reports a version *mismatch* rather than
failing cleanly. `direnv status` prints `Found RC allowed 0`, and `0` means
allowed — it reads like the opposite.
## Partitioning
By **crate or directory**, never by ticket count. **Tickets that share a file go
to one agent**, to be done sequentially — four widget migrations all touching
`src/draw.rs` are a guaranteed conflict split four ways. Group first, then count.
Tell each agent what ground is *not* theirs, and to report rather than guess when
it must cross. Two agents adding a workspace member will still collide in
`Cargo.toml`/`Cargo.lock`; keep both sides.
## What every brief must carry
1. **The expected base commit, by name.** Worktrees do not reliably branch from
HEAD — one was created three merges behind and only noticed because it was
told to check.
2. **The baseline**, so "N passing" is checkable rather than asserted: Rust test
count, harness counts, node counts.
3. **Its ground, and the ground that is not.**
4. **Never write** `.beads/**`, `prolog-sessions/kb.pl`, `context/repo-map.md`.
Agents read them freely and report proposed text; the orchestrator applies it.
5. **The commit trailers and PR footer**, spelled out.
6. **Do not push to `main`, do not merge.** The orchestrator merges.
7. **What to report**: branch and head SHA, PR number, what changed per ticket,
*what could not be verified and why*, proposed repo-map/kb text, and **what it
learned that the ticket did not predict** — consistently the most valuable
part, and omitted unless asked for explicitly.
## Method to require, because this repo's defect is a test that passes
- Failing test first, then the fix.
- Every guard proved by **injecting the fault it names** and watching it fail
**by name**. An injection that does not compile reads as a clean run, so
require a positive signal the build ran.
- `cargo test --workspace` is **fail-fast**; `--no-fail-fast` for any claim about
blast radius.
- A textually clean merge can **delete tests** with no conflict marker, and
`cargo check` will not notice. After any merge that moves a test module, diff
test **names** — a count is masked by the other side's additions.
## Shepherding
- Wait on checks with `scripts/pr-watch <pr> --expect <sha>`. Never hand-roll a
poll; see the `pr-watch` skill for the three ways they report green while wrong.
- 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.
- 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`.
- Anything posted to GitHub goes out under the owner's account: open every
comment with a line saying a Claude agent wrote it.
## Between waves
Run the code-review skill on the merged range. The characteristic find here is
not a wrong line, it is **a test that passes while the thing it names is
broken** — a vacuous fold test, a one-category fixture, a helper that fabricates
what the code under test needs. Ask the reviewer to inject the fault and watch
the check fail.
+11 -44
View File
@@ -100,52 +100,19 @@ Two agents each adding a workspace member will still collide in
### Every wave agent's brief must include
1. **Verify your base commit first.** Worktrees do **not** reliably branch from
the orchestrator's HEAD — one was created three merges behind and only
noticed because it was told to look. `git log --oneline -1` against a named
commit, reset onto `main` if behind. Name the expected commit in the brief.
2. **The baseline test count**, so "1090 passing" is checkable rather than
asserted, and a drop is visible. Tell the agent to **measure its own** and
cite the machine: the same commit measured `improvise-io`'s binary at 1.4s on
one machine and 3.7–5.2s on another, so a figure with no machine is a claim
about the suite, which it is not.
3. **The gate list, and it must not say `cargo fmt --all`.** That reaches only
**one** of this repo's two workspace roots — `harness/opfs-contract` declares
its own `[workspace]` — and the obvious repair is worse: `cargo fmt
-p opfs-contract` from the repo root exits **1**, *"not a member of the
workspace"*, which is indistinguishable from a real formatting failure
(improvise-b4ks). Both spellings fail in the direction that looks like
success. The gate is:
The wave skill holds both halves, each in one place (improvise-1f8e):
```
direnv exec . ./scripts/fmt-workspaces.sh # exit 2 = the gate did not run
```
- **The brief checklist** — what an orchestrator must put in a brief — is
`.agents/skills/wave/SKILL.md`, "What every brief must carry".
- **The agent's standing rules** — base and push, gates, claim, vacuity shapes,
evidence, forbidden files, GitHub, hand-back, disk, commit early — are
`.agents/skills/wave/agent-rules.md`. A brief points there rather than
restating them.
Every gate goes through `direnv exec .` — `./scripts/checkers.sh` scrubs the
environment to an allowlist, so a bare invocation drops `tpl` off `PATH` and
reports spurious failures. And **never pipe a gate**: `$?` must be the gate's
own, and `check-comment-runs.py --check-evidence` prints two lines with the
comment cap **first**, so `| tail -1` silently drops half of it.
4. **Commit frequently, so the work is safe in git.** A WIP commit after the
first green test, and again after every meaningful step. A worktree is not
durable storage: an agent finished a whole ticket with every gate green and
lost all of it, uncommitted, when its worktree was deleted during a disk
cleanup. Tidying the history before the PR is fine; losing it is not.
5. **Do not merge to `main`.** The orchestrator merges. Pushing the agent's own
branch (`git push gh HEAD:refs/heads/<branch>`) is encouraged once it is
worth keeping.
6. The commit-message trailers, spelled out.
7. **What to report**: branch and commits, what changed per ticket, *what could
not be verified and why*, and proposed repo-map/`kb.pl` text. Ask explicitly
for the thing the agent learned that the ticket did not predict — that is
consistently the most valuable part of a hand-back, and agents omit it unless
asked.
`bd` works in any worktree, but `.envrc` must be allowed there or you get a
**version mismatch** rather than a clean failure — `direnv allow` in the
worktree, or rely on the `[whitelist] prefix` in `~/.config/direnv/direnv.toml`.
Note `direnv status` reports `Found RC allowed 0`, and `0` means **allowed**
(the enum is 0=Allowed, 1=NotAllowed). It reads like the opposite.
`.claude/skills/wave` is a symlink to `.agents/skills/wave`, so Claude and Codex
read the same files. A new rule goes into one of those two files, not here and
not into the next brief: a restated rule is the copy nobody corrects, and this
list was once a third copy that drifted from the skill's.
### Between waves
+33 -3
View File
@@ -2382,6 +2382,31 @@ keystroke to land in.
Injected fault: dropping `self.view_state = snapshot.view_state` from
`App::restore` fails this test and only this test.
**Addendum (improvise-gho5).** Improvise-gho5 found that this
test and `a_rolled_back_load_tells_nobody_and_puts_every_mirror_back` passed
against a host that **never ran the load**. The review's fault, reproduced
here, was placed at the top of `Host::commands`: refuse any multi-command frame
that contains a `LoadModel` with `Rejected(DocumentReplaced)`, before
`CheckedBatch` runs. Both tests stayed green. A frame that never got that far
leaves the generation, the view, the status line and every mirror untouched,
which is exactly what the tests asserted.
Both tests now assert that the asker's whole reply is
`the_load_ran_then_the_write_broke()`, which is `Rejected(ChainBroken(North))`.
They also assert, before the frame, that `North` resolves. Together these make
the rejection a witness that the load ran. `ChainBroken` comes only from
`reduce::broken_chain`, which runs inside `mutating_frame` just before each
mutation. It names `North` only once the document no longer holds that
record, and only an applied `LoadModel` in this frame can have removed it.
With the same fault re-injected, both tests fail. The left side is
`[Rejected(DocumentReplaced)]` and the right is
`[Rejected(ChainBroken(<North>))]`. The fault was reverted.
The residual: a rogue up-front check that forged `ChainBroken` specifically
would still pass. That is the same forgery a hand-rolled client could commit
against any basis check, and a test cannot guard against it.
### a_refused_mutation_rolls_back_its_predecessor_and_the_abort_flag
The `Rejection::MutationRefused` arm — the one the first hand-back wrongly called
@@ -4900,9 +4925,14 @@ Three things make it non-vacuous:
`LoadModel` silently failed, `Applied::Rebased` would never run and every
assertion here would pass over nothing. The control asserts the generation
moves and that *both* clients are owed a `Snapshot`.
- **The rollback is asserted before anything is concluded from it** — the
`document_generation` equality is checked first, so "the frame was rejected"
cannot be confused with "the frame never got that far".
- **The load is witnessed to have RUN before anything is concluded from the
rollback** (improvise-gho5). The asker's whole reply must be
`Rejected(ChainBroken(North))`. `North` is asserted to resolve before the
frame, so only `broken_chain` can give that answer, and only after the
`LoadModel` has replaced the document. `CheckedBatch` never produces
`ChainBroken`. This bullet used to say the `document_generation` equality
did the job, and that was false: generation equality is exactly what holds
when the frame never got that far.
- **A bystander is seated where a wrong relay could move it.** A single-client
fixture cannot tell "the mirrors were restored" from "there was one mirror and
it happened to be the asker's".
+68 -3
View File
@@ -391,6 +391,8 @@ green suite over a vacuous assertion.
**What is not covered.** The script asks `cargo metadata` for members, so a root
whose metadata succeeds but whose member list is wrong is not detectable here.
*Narrowed by improvise-90ea, below:* a list that **omits** a package in the tree
is now refused; one that names a package it should not is still not detected.
And `harness/opfs-contract`'s own `cargo fmt -p opfs-contract` step in the
`harness-lint` job is left as it is: it resolves correctly because that step
sets `working-directory`, and the ticket's own note confirms CI as written was
@@ -415,9 +417,11 @@ getting wrong. The line the split actually needs is **"did the gate run"**:
- exit 1 — rustfmt did not certify a root clean. A diff or a refusal to parse;
either way **a file is named** and the tree is what is wrong.
- exit 2 — the gate did not run. Usage, no cargo, no rustfmt, `cargo metadata`
failed, no `[workspace]` root. **No file is named.** Every such cause is
checked *above* the loop, which is what makes the narrowing true rather than
hopeful.
failed, no `[workspace]` root. **No source file is named.** Every such cause
is checked *above* the loop, which is what makes the narrowing true rather
than hopeful. (Since improvise-90ea, a package no root reaches is also exit 2,
and it names that package's `Cargo.toml`, which is the reason no verdict
exists. The line is "no verdict", not "no output"; improvise-ximf.)
Both directions are now pinned, and two further rivals were injected:
@@ -439,6 +443,67 @@ stub must fail in the **shape** of the real fault, not merely fail — a stub th
breaks more than the real thing breaks satisfies the test through the wrong arm,
and reads exactly like coverage.
#### The root package, and why the fourth fix is structural — `scripts/fmt-workspaces-test.sh`
A post-merge review of PR #159 found a **fourth** fixture vacuity
(improvise-90ea). The fixture's outer root is a package (`root-pkg`) as well as
a workspace, which is right, but no case ever dirtied **the root package's own
`src/lib.rs`**. Only the member `a/` and `nested/` were dirtied. So a script
that formats every member *except* the root package passed 17/17. In this repo
the root package is `improvise`, the whole TUI. Reproduced at `40b8913` with
the review's rival in `members_of` (drop the package whose manifest sits at
`workspace_root`): the suite passed 17/17 and `--list` left `improvise` out.
Adding one more hand-picked case would have fixed the fourth and not the fifth.
Four vacuities in one checker all have the same cause: **the cases were chosen
by hand, one per hazard somebody had thought of.** So the fix is structural,
and it has two halves because it has to cover two sets of packages:
- **Every package the fixture has.** The two hand-picked fault cases are
replaced by a loop over every `[package]` manifest in the fixture. Each one
gets its own misformatted `src/lib.rs`, must exit 1, and must be named
**exactly once**, so the `--all` boundary check now covers every package
rather than one. The enumeration is a `grep` for `[package]`, deliberately
*not* the script's `[workspace]` + `cargo metadata` discovery, since that
discovery is what is under test. A count assertion (three packages) stops
the loop going vacuous if the fixture loses a package. A package added to
the fixture gets its case automatically.
- **Every package the tree has.** The loop proves only that the script handles
the fixture's *shapes*. A real tree can hold a package no root lists (an
`exclude`d crate, say), and nothing in the fixture would say so. So the gate
itself now checks an invariant **before it formats anything**: every
`[package]` manifest the find reaches must be a member of some root, or it
exits **2** and names the package. That is exit 2 and not 1 because no
verdict was produced for that package, which is what exit 2 means here.
Paths are compared through `realpath`, because on macOS `mktemp` lives under
`/var` while cargo reports `/private/var`.
**Rivals injected, and where each failed** (the suite went from 17 to 21 cases):
- **The review's skip-the-root rival.** It now fails the fixture at `a fault in
src/lib.rs is reported` (exit 0). With the invariant in place it fails
**six** cases (exit 2, "no workspace root reaches"). It also fails the
**real repo** with no fixture involved: `fmt-workspaces.sh` exits 2 naming
`Cargo.toml`. That last one is the property worth having. The gate refuses
its own blindness in CI, not only in its test suite.
- **Disabling the invariant** (`if false`). It fails `a package no workspace
root reaches is exit 2, not a pass` with **exit 0**, even though that
package's `src/lib.rs` is misformatted. That exit 0 *is* the hazard: an
unreached package is silently clean.
- **The two earlier rivals, re-run against the loop.** `--all` for `-p` fails
both `a/src/lib.rs` and `src/lib.rs` "named exactly once". The second of
those was not predicted: the nested root's `--all` also reaches the root
package. The bare manifest-path form fails `a fault in a/src/lib.rs is
reported`.
The real-repo control from the ticket still holds: a misformatted fn appended
to `src/ui/text.rs` is reported (`Diff in …/src/ui/text.rs:72`).
**What is still not covered.** A root whose member list *includes* something it
should not is not detected, and a package in a directory the find excludes
(`target/`, `.claude/`, `node_modules/`) is outside the invariant by
construction.
### The kb tool's completeness guard and atomic write — `scripts/kb-test.sh` cases 18–22
improvise-usus: `scripts/kb add` wrote onto a working-tree `kb.pl` that had lost
+27 -5
View File
@@ -1049,6 +1049,12 @@ mod tests {
let was_view = host.app().view_state.active_view.clone();
let was_status = host.app().view_state.status_msg.clone();
let basis = frame_basis(&host, &["North"]);
assert!(
host.app()
.resolve_record(addressed("North").record)
.is_some(),
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
);
// The load lands, then the write names a record the incoming document
// has never had — so the frame fails at its second command.
@@ -1069,11 +1075,10 @@ mod tests {
]),
);
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Rejected(_))),
"the fixture must actually refuse the frame: {answer:?}"
assert_eq!(
to(&answer, asker),
vec![the_load_ran_then_the_write_broke()],
"only a load that RAN can refuse the write this way: {answer:?}"
);
assert_eq!(
host.app().view_state.active_view,
@@ -1097,6 +1102,12 @@ mod tests {
);
}
/// `North` resolves in the outgoing document, so only `broken_chain` AFTER the
/// load applied can answer this. Evidence: `context/test-evidence/command.md`.
fn the_load_ran_then_the_write_broke() -> ServerFrame {
ServerFrame::Rejected(Rejection::ChainBroken(addressed("North").record))
}
/// Everything one client was told, flattened to the projections it carries.
fn projections(frames: &[Addressed], id: SessionId) -> Vec<Projection> {
to(frames, id)
@@ -1147,6 +1158,12 @@ mod tests {
let (bystander, _) = host.connect(10, 10);
let was_generation = host.app().document_generation();
let basis = frame_basis(&host, &["North"]);
assert!(
host.app()
.resolve_record(addressed("North").record)
.is_some(),
"North must resolve BEFORE the load, or a refusal naming it proves nothing"
);
// The load lands and rebases every mirror; the write then names a
// record the incoming document has never had, so the frame fails.
@@ -1167,6 +1184,11 @@ mod tests {
]),
);
assert_eq!(
to(&answer, asker),
vec![the_load_ran_then_the_write_broke()],
"only a load that RAN can refuse the write this way: {answer:?}"
);
assert_eq!(
host.app().document_generation(),
was_generation,
+54 -27
View File
@@ -76,36 +76,37 @@ run_case() {
scaffold
run_case "both workspaces clean is a pass" 0 "$work/repo"
# The load-bearing case. `cargo fmt --all` at the outer root cannot reach this
# file, and `cargo fmt -p inner-pkg` from the outer root exits 1 saying `not a
# member of the workspace` -- the usage error this whole script exists to split
# from a real verdict.
# One fault case PER PACKAGE, enumerated from the fixture rather than
# hand-picked: three hand-picked shapes each missed a rival, and the fourth
# missed the root package itself (improvise-90ea). Reasoning:
# context/test-evidence/tooling.md. The enumeration is a find over
# `[package]` manifests, deliberately NOT the script's own `[workspace]` +
# `cargo metadata` discovery, which is the thing under test.
scaffold
printf '%s' "$DIRTY" > "$work/repo/nested/src/lib.rs"
if run_case "a fault in the NESTED workspace is reported" 1 "$work/repo"; then
case "$OUT" in
*nested/src/lib.rs*) ok " and the nested file is named" ;;
*) bad " and the nested file is named" "output did not name it: ${OUT:0:300}" ;;
esac
pkg_manifests=()
while IFS= read -r m; do pkg_manifests+=("$m"); done < <(
cd "$work/repo" && grep -rl --include=Cargo.toml '^\[package\]' . | sort)
# An empty or shrunken enumeration would make the loop below vacuous.
if [ "${#pkg_manifests[@]}" = 3 ]; then
ok "the fixture enumerates all three packages (root, member, nested)"
else
bad "the fixture enumerates all three packages" "found: ${pkg_manifests[*]:-none}"
fi
scaffold
printf '%s' "$DIRTY" > "$work/repo/a/src/lib.rs"
if run_case "a fault in the OUTER workspace is reported" 1 "$work/repo"; then
case "$OUT" in
*a/src/lib.rs*) ok " and the outer file is named" ;;
*) bad " and the outer file is named" "output did not name it: ${OUT:0:300}" ;;
esac
# The rival: spelling the per-root check `--all` instead of `-p <members>`
# walks PAST the nested workspace boundary (measured), so the OUTER file is
# reported a second time under the inner root. Counting is what sees that.
n="$(printf '%s\n' "$OUT" | grep -c 'a/src/lib.rs')"
if [ "$n" = 1 ]; then
ok " and it is named ONCE -- no root reaches past its own boundary"
else
bad " and it is named ONCE" "named $n times; a root is walking past its boundary"
for m in "${pkg_manifests[@]}"; do
rel="$(dirname "${m#./}")/src/lib.rs"; rel="${rel#./}"
scaffold
printf '%s' "$DIRTY" > "$work/repo/$rel"
if run_case "a fault in $rel is reported" 1 "$work/repo"; then
# Counting, not matching: `--all` for `-p` walks PAST the nested boundary
# (measured) and names an outer file a second time under the inner root.
n="$(printf '%s\n' "$OUT" | grep -cF "$work/repo/$rel")"
if [ "$n" = 1 ]; then
ok " and $rel is named exactly once"
else
bad " and $rel is named exactly once" "named $n times; output: ${OUT:0:300}"
fi
fi
fi
done
# A usage error must NOT wear a formatting failure's exit code. This is the
# whole ticket: `cargo fmt -p opfs-contract` from the repo root exits 1.
@@ -121,6 +122,32 @@ run_case "a tree with no workspace root is a usage error, not a pass" 2 "$work/e
run_case "--help is a usage error, so a typo cannot pass" 2 "$work/repo" --help
# The loop above covers every package the FIXTURE has; this covers every
# package the TREE has. A package no root lists is checked by nothing, so the
# gate must refuse a verdict -- and name the package -- rather than pass.
scaffold
mkdir -p "$work/repo/orphan/src"
printf '[package]\nname = "orphan-pkg"\nversion = "0.1.0"\nedition = "2021"\n' \
> "$work/repo/orphan/Cargo.toml"
printf '%s' "$DIRTY" > "$work/repo/orphan/src/lib.rs"
cat > "$work/repo/Cargo.toml" <<'EOF'
[workspace]
members = ["a"]
exclude = ["orphan"]
resolver = "2"
[package]
name = "root-pkg"
version = "0.1.0"
edition = "2021"
EOF
if run_case "a package no workspace root reaches is exit 2, not a pass" 2 "$work/repo"; then
case "$OUT" in
*"no workspace root reaches"*orphan/Cargo.toml*) ok " and the unreached package is named" ;;
*) bad " and the unreached package is named" "output: ${OUT:0:300}" ;;
esac
fi
# The narrowed contract, pinned so it is not merely prose (ChatGPT Codex on
# PR #159). rustfmt exits 1 for a source it cannot parse, exactly as for a
# diff -- so exit 1 is "rustfmt did not certify this root clean", and what
+43 -17
View File
@@ -1,7 +1,7 @@
#!/usr/bin/env bash
# Format-checks every Cargo workspace root in the repo, from any directory.
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/code/tooling.md
# section 4 -- including why `-p` scoped by `--manifest-path` is the only
# `cargo fmt --all` reaches only ONE of them. Reasoning: context/test-evidence/
# tooling.md, "The two-workspace fmt gate" -- including why `-p` scoped by `--manifest-path` is the only
# spelling that is neither blind nor boundary-crossing (improvise-b4ks).
set -uo pipefail
@@ -18,8 +18,9 @@ exit 1 rustfmt did not certify a root clean: a file differs from rustfmt's
output, or rustfmt refused to parse one. Either way a FILE IS NAMED
and the tree is what is wrong.
exit 2 the gate did not run: usage error, no cargo, no rustfmt, cargo
metadata failed, or no [workspace] root found. NO verdict was
produced and NO file is named.
metadata failed, no [workspace] root found, or a [package] that no
root has as a member. NO verdict was produced. No SOURCE file is
named; the unreached-package case names its Cargo.toml.
The split is "did the gate run", not "is every exit-1 cause a diff". That is
narrower than it sounds and it is deliberate -- see the note on rc=1 below.
@@ -38,8 +39,8 @@ EOF
# The find is RELATIVE to the tree being scanned: an absolute `-path` filter
# matches the ancestors too, and this repo is itself checked out under
# `.claude/worktrees/` in every agent worktree, where that excluded everything.
workspace_roots() {
local root="$1"
manifests_with() {
local root="$1" section="$2"
( cd "$root" && find . -name Cargo.toml \
-not -path './target/*' -not -path '*/target/*' \
-not -path './.git/*' \
@@ -47,11 +48,12 @@ workspace_roots() {
-not -path './.claude/*' ) \
| sort \
| while IFS= read -r rel; do
if grep -q '^\[workspace\]' "$root/${rel#./}"; then
if grep -q "^\\[$section\\]" "$root/${rel#./}"; then
printf '%s\n' "$root/${rel#./}"
fi
done
}
workspace_roots() { manifests_with "$1" workspace; }
# The members of ONE workspace, named by cargo rather than parsed out of the
# manifest: a `members = [...]` list may carry globs and path dependencies.
@@ -61,10 +63,10 @@ members_of() {
printf '%s' "$meta" | python3 -c '
import json, sys
doc = json.load(sys.stdin)
names = [p["name"] for p in doc["packages"]]
if not names:
rows = [p["name"] + "\t" + p["manifest_path"] for p in doc["packages"]]
if not rows:
sys.exit(1)
print("\n".join(names))
print("\n".join(rows))
' || return 1
}
@@ -106,15 +108,39 @@ main() {
return 2
fi
local rc=0 manifest member
for manifest in "${roots[@]}"; do
local -a pkgs=()
while IFS= read -r member; do [ -n "$member" ] && pkgs+=("$member"); done \
< <(members_of "$manifest")
if [ ${#pkgs[@]} -eq 0 ]; then
printf 'fmt-workspaces: cargo metadata failed for %s\n' "$manifest" >&2
# Pass 1: every root's members, before anything is formatted, so that a
# package NO root reaches is exit 2 rather than silently never checked.
local rc=0 manifest member i m
local -a members=() covered=()
for i in "${!roots[@]}"; do
local rows
rows="$(members_of "${roots[$i]}")" || rows=""
if [ -z "$rows" ]; then
printf 'fmt-workspaces: cargo metadata failed for %s\n' "${roots[$i]}" >&2
return 2
fi
members[$i]="$(printf '%s\n' "$rows" | cut -f1 | tr '\n' ' ')"
while IFS= read -r m; do covered+=("$m"); done < <(printf '%s\n' "$rows" | cut -f2)
done
local orphans
orphans="$(manifests_with "$root" package | python3 -c '
import os, sys
covered = {os.path.realpath(p) for p in sys.argv[1:]}
for line in sys.stdin:
m = line.rstrip("\n")
if m and os.path.realpath(m) not in covered:
print(m)
' "${covered[@]}")"
if [ -n "$orphans" ]; then
printf 'fmt-workspaces: no workspace root reaches these packages, so nothing checks them:\n%s\n' \
"$orphans" >&2
return 2
fi
for i in "${!roots[@]}"; do
manifest="${roots[$i]}"
local -a pkgs=()
read -r -a pkgs <<< "${members[$i]}"
if [ "$list" = 1 ]; then
printf '%s: %s\n' "$manifest" "${pkgs[*]}"