Files
improvise/AGENTS.md
T
Edward Langley 141be9fca6 Review replies go on the PR thread; ADR PRs merge on the user's approval
- agent-rules "GitHub": authors reply on the PR to every finding they
  address (commit, or why declined); reviewers read existing reviews first
  with scripts/pr-reviews, answer every Codex review on its thread, and
  state the head SHA they reviewed. SKILL.md points reviewers there.
- AGENTS.md "ADR pull requests": numbered open questions, the user answers
  on the PR, "What the user decided", merge only on "Approved" (PR #122).
  SKILL.md "Shepherding" points to it.
- Fix a pointer to the renamed policy section that wrapped across a line
  break and so escaped the rename.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-22 23:16:48 -07:00

25 KiB

  • Always use tests to demonstrate the existence of a bug before fixing the bug.
    • If you suspect that a bug exists, use a test to demonstrate it first:
      • prefer unit tests testing a small amount of code to integration or e2e tests
  • Option<...> or Result<...> are fine but should not be present in the majority of the code.
  • Similarly, code managing Box<...> or RC<...>, etc. for containers pointing to heap data should be split from logic
  • @context/repo-map.md is your "road map" for the repository. use it to reduce exploration and keep it updated.
  • @context/design-principles.md is also important for keeping the repository consistent.
  • prefer merges to rebasing.
  • always start responses with bananaS!
  • there is a prolog MCP available: load a session from kb at the root of the repository at the beginning of every session, and query it as you work. Reading only — the MCP must never write kb.pl.
  • save structured facts as you go — work done, interesting locations, important decisions — with scripts/kb add (see the kb skill). Write a fact when you learn it; there is no reason to batch.
  • query the knowledge base as needed and use Prolog reasoning when it makes sense.
  • Prolog should be used as a design tool to reason about the code and the implementation of tickets and to persist a formal model of it.

Agent Instructions

This project uses bd (beads) for issue tracking. Run bd prime for full workflow context.

Quick Reference

bd ready              # Find available work
bd show <id>          # View issue details
bd update <id> --claim  # Claim work atomically
bd close <id>         # Complete work
bd dolt push          # Push beads data to remote

Non-Interactive Shell Commands

ALWAYS use non-interactive flags with file operations to avoid hanging on confirmation prompts.

Shell commands like cp, mv, and rm may be aliased to include -i (interactive) mode on some systems, causing the agent to hang indefinitely waiting for y/n input.

Use these forms instead:

# Force overwrite without prompting
cp -f source dest           # NOT: cp source dest
mv -f source dest           # NOT: mv source dest
rm -f file                  # NOT: rm file

# For recursive operations
rm -rf directory            # NOT: rm -r directory
cp -rf source dest          # NOT: cp -r source dest

Other commands that may prompt:

  • scp - use -o BatchMode=yes for non-interactive
  • ssh - use -o BatchMode=yes to fail instead of prompting
  • apt-get - use -y flag
  • brew - use HOMEBREW_NO_AUTO_UPDATE=1 env var

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, 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

Three files are orchestrator-only, and the rule exists because they are guaranteed conflicts — every agent has a reason to touch them and they merge badly:

File Why
.beads/ (incl. issues.jsonl) bd owns it. See "Never hand-edit" below.
prolog-sessions/kb.pl Written by scripts/kb only. See context/code/kb-tool.md.
context/repo-map.md Every agent wants to add a gotcha; every wave would conflict.

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. 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, 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.

Partitioning a wave

Partition by crate or directory, not by ticket count, and tell each agent explicitly which ground is not theirs. Two rules that fall out of the ones we broke:

  • Tickets that share a file belong to one agent. The four TUI widget migrations all touch src/draw.rs; split across agents they are a guaranteed conflict, so they go to one agent to do sequentially. Group first, then count.
  • An agent that must stray says so rather than guessing. Give the boundary and ask for a report when it has to be crossed.

Two agents each adding a workspace member will still collide in Cargo.toml/Cargo.lock. That one is expected; keep both sides.

Every wave agent's brief must include

The wave skill holds both halves, each in one place (improvise-1f8e):

  • 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.

.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

Run the code-review skill on the merged range and iterate until its backlog is empty. This is not ceremony: in this repo the review's characteristic find is not a wrong line, it is a test that passes while the thing it names is broken — a vacuous fold test, a bit-witness that cannot see the bit, a one-category fixture, a helper that fabricates the _Measure items the code under test needs, a check-the-checker that injected malformed JSON and so proved nothing. A green suite is not evidence. Ask the reviewer to inject the fault and watch the check fail.

Shipping a wave

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.

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 passes. The tell is a PR diff naming files the ticket has no reason to touch, so read your own PR's file list before asking for review — and diff against gh/main...<branch>, never the local equivalent. The second vector is the worktree base: a branch cut from the orchestrator's local main inherits whatever that checkout has merged but not yet pushed, and nothing in the branch says so. That hit PR #148 with another PR's whole diff — including .beads/, kb.pl and context/repo-map.md, the three files agents are forbidden to touch, so the review read as a rule violation rather than a base problem. Note the window is transient — the two remotes are equal immediately after the orchestrator syncs — which makes it worse, not better: the hazard is absent exactly when you would think to check for it.

This is easy to drift out of, because merging locally and pushing feels like 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, 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.

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 (improvise-x1ns). Both were correct.

gh pr view <n> --json reviews -q '.reviews[]|"[\(.state)] \(.submittedAt) \(.commit.oid[0:8])\n\(.body)"'

Two things make a Codex review easy to miss. It arrives as a review, not a comment, so a glance at the comment thread does not show it. And it posts under the repository owner's account — the same account our own agents post under — so authorship does not distinguish it. The tell is the trailing line _Automated review by ChatGPT Codex (…)_; ours open with _Posted by Claude (…)_. Check the commit.oid too: a review may target an 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.

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, #66, #68 and #70 carry none. Both request paths report success and produce zero review_requested events — the GraphQL mutation returns clientMutationId: null and the REST call returns 200 with requested_reviewers: []. So do not wait on it: an absent review and an unrequested one look identical from outside, which is doctrine(review_is_requested_not_inherited) seen from the requester's side. Review is an adversarial agent the orchestrator dispatches, given the head SHA. The mechanics below are kept for the day it comes back.

Copilot review must be re-requested after every fix push; it does not re-review on its own. The mutation takes a botIds field — userIds fails with "Could not resolve to User node":

gh api graphql -f query='mutation($pr:ID!,$ids:[ID!]!){requestReviews(input:{pullRequestId:$pr,botIds:$ids,union:true}){clientMutationId}}' \
  -f pr="$(gh pr view <n> --json id -q .id)" -f ids=BOT_kgDOCnlnWA

REST also works, with the login spelled copilot-pull-request-reviewer[bot] — without the [bot] suffix it 422s as "not a collaborator".

In a worktree-isolated agent the shell guard refuses an inline gh api graphql -f query='mutation(...)' as "too complex to verify". Put the query in a file and pass -F query=@file; PR comment bodies likewise go through -F body=@file rather than command substitution.

Anything you post to GitHub goes out under the repository owner's account, so say who is speaking. Open every PR comment, review and issue body with a line naming the model and that it is an agent posting in his session. A PR body's 🤖 Generated with Claude Code footer covers that case; comments have nothing unless you add it, and an unattributed one reads as his own words to everyone who sees it.

Two environment notes. gh is not on PATH here — the name is shell-aliased to git-hub, a different tool — so use nix shell nixpkgs#gh --command gh …. And a Copilot review takes about six minutes; tool calls return fast, which makes elapsed time easy to misjudge, so check date -u against the GitHub timeline before concluding a review is stuck. (One agent closed and reopened a PR to "unstick" a review that was proceeding 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 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 exact-frame characterization tests that were the PR's only evidence the migration changed nothing. Two right findings, different costs. File the declined one.

Under merge-as-soon-as-green, a review round can land after its own merge. GitHub deletes the branch when a PR merges, so a fix push to the old branch name silently succeeds and accomplishes nothing — it re-creates a branch main will never see, CI runs on it, and the commit is nowhere. The tell is the push printing * [new branch] where you expected an old..new range. It happened on #51: the third review round completed four minutes after the merge, and the correction it prompted went to a re-created wave/b-projection-gaps. The repair 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 — 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 git checkout-ing it, which fires the post-checkout hook — breaks auto-export and desynchronises export-state.json. Use bd commands, let auto-export run, and commit whatever it leaves.

Cite a ticket ID you have read back

bd create assigns the ID; it is not derivable from the title. An orchestrator that files with output suppressed and then writes the ticket into a gotcha or an ADR will invent one — six invented IDs survived into context/ in a single session before an agent tried one and found nothing. Read the ID back after filing, and before pushing docs run the audit:

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|report|side|web)$' \
  > /tmp/bd-audit-ids.txt
bd show $(cat /tmp/bd-audit-ids.txt) 2>&1 \
  | 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:

  • It scanned two paths (context/repo-map.md context/adr/*.md), so it found one dead id in the repo map and missed both copies of it in harness/tests/shell.spec.js. Ticket IDs are cited in code comments, in README.md and in web/, not only in context/.
  • [a-z0-9]{3,4} cannot see a commit SHA wearing the prefix. A seven-hex commit sha cited as improvise-<sha> survived in crates/improvise-core/src/workbook.rs precisely because it was too long to match. An ID-shaped non-ID is worse than a dead ID, because the diagnosis is identical and the repair is not.
  • 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, 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 exclusion list is a filter, and a filter that over-matches fails in the direction that looks like success. (Anchoring it is also what caught the example id this very paragraph originally spelled out — see the note below.)
  • A release artifact's name is ID-shaped, and improvise-installer.sh escaped only by being ten characters long. improvise-web.tar.gz (improvise-oujb) is three, so it matches {3,8} and resolves to nothing; web is therefore excluded above. The general point is that the length bound, not any rule, is what has been keeping dist's artifact names out of this audit — so a new artifact whose name is a short lowercase word must be 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.

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 materialises the id list into a file and hands bd show the whole list in one call rather than looping. If bd show in your build does not take several ids, run the ids through xargs -n1 -I{} bd show {} from a plain shell instead.

A dead ID is worse than no ID: it reads as a promise that the follow-up exists.

Agent-filed issues duplicate work that is already open, sometimes already 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

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").

Beads Issue Tracker

This project uses bd (beads) for issue tracking. Run bd prime to see full workflow context and commands.

Quick Reference

bd ready              # Find available work
bd show <id>          # View issue details
bd update <id> --claim  # Claim work
bd close <id>         # Complete work

Rules

  • 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

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

This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions.

  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. Handle git/sync by active profile:
    # 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
    
  5. Hand off - Summarize changes, validation, issue status, and any blocked sync/commit/push step

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.

Beads Issue Tracker

Use Beads (bd) for durable task tracking in repositories that include it. Use the beads skill at .agents/skills/beads/SKILL.md (project install) or ~/.agents/skills/beads/SKILL.md (global install) for Beads workflow guidance, then use the bd CLI for issue operations.

Quick Reference

bd ready                # Find available work
bd show <id>            # View issue details
bd update <id> --claim  # Claim work
bd close <id>           # Complete work
bd prime                # Refresh Beads context

Rules

  • Use bd for all task tracking; do not create markdown TODO lists.
  • Run bd prime when Beads context is missing or stale. Codex 0.129.0+ can load Beads context automatically through native hooks; use /hooks to inspect or toggle them.
  • Keep persistent project memory in Beads via bd remember; do not create ad hoc memory 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.