diff --git a/.agents/skills/wave/agent-rules.md b/.agents/skills/wave/agent-rules.md index 4c9a5bf8..c5cd5e3c 100644 --- a/.agents/skills/wave/agent-rules.md +++ b/.agents/skills/wave/agent-rules.md @@ -29,12 +29,12 @@ not in a second copy. 5. **Before pushing a fix to an existing PR, check `gh pr view --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`, "The Beads blocks are bd's" (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 ` **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 diff --git a/AGENTS.md b/AGENTS.md index 4be26a9d..9a6fb364 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 @@ -127,18 +127,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 "The Beads blocks are bd's" 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 +156,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,7 +251,7 @@ 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. ### Never hand-edit `.beads/issues.jsonl` @@ -271,13 +272,28 @@ filing, and before pushing docs run the audit: ```bash grep -rohE 'improvise-[a-z0-9]{3,8}\b' \ context README.md AGENTS.md CLAUDE.md crates src tests examples web harness docs \ + .agents .claude/settings.json .claude/hooks \ | 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" ``` +Two things about the paths that are easy to get wrong: + +- **`.claude` is named file by file, never whole.** In the orchestrator's + checkout `.claude/worktrees/` holds every agent's worktree, so `grep -r + .claude` scans dozens of copies of the repository and reports ids from other + agents' unpushed work. +- **`.claude/skills/*` are symlinks into `.agents/skills/`, and `grep -r` does + not follow a symlink it meets while recursing** (GNU and BSD grep both). So + the skills are scanned once, through `.agents`, with no double count and no + loop. + +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 +309,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 +344,26 @@ 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. +## The Beads blocks are bd's - +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: + +- **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"). + + ## Beads Issue Tracker This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands. @@ -344,35 +379,44 @@ bd close # 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. diff --git a/context/design-principles.md b/context/design-principles.md index 8d3e29b1..23c31b1b 100644 --- a/context/design-principles.md +++ b/context/design-principles.md @@ -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.