From 7735cfd107338ed8e89faa1a3e326dca8192eeea Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 13:03:28 +0200 Subject: [PATCH] Try before the review, fail first on behaviour, survey every reader of a changed value (playbooks 07 and 08) --- 00-META/process/00-overview.md | 2 +- 00-META/process/07-feature-branches.md | 42 ++++++++++++++++++-------- 00-META/process/08-small-fixes.md | 16 +++++----- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/00-META/process/00-overview.md b/00-META/process/00-overview.md index d6901ac7..96b95b0a 100644 --- a/00-META/process/00-overview.md +++ b/00-META/process/00-overview.md @@ -57,7 +57,7 @@ the expensive half. | [05](05-constitution-sync.md) | Constitution sync | `how-we-build.md` changed a rule the mesh enforces | | [06](06-writing-a-module.md) | Writing a module | Something that runs today must run on the mesh | | [07](07-feature-branches.md) | Features | Work that changes code across issues or repositories: prepared context, local development, one review, one check, one delivery | -| [08](08-small-fixes.md) | Small fixes | One issue in one repository, about an hour of work: one review, one push, one delivery | +| [08](08-small-fixes.md) | Small fixes | One issue in one repository, about an hour of work: one try, one review, one delivery | | [09](09-cleanup.md) | Cleanup | Merged branches, stale pull requests, finished worktrees and folders: read first, delete only what the operator approves, report the rest | ## The cycle is checked diff --git a/00-META/process/07-feature-branches.md b/00-META/process/07-feature-branches.md index 821f579b..dc1019e1 100644 --- a/00-META/process/07-feature-branches.md +++ b/00-META/process/07-feature-branches.md @@ -30,6 +30,11 @@ A small fix (one repository, one issue, about an hour of work) follows - **A green pull request nobody marks.** An approved, green pull request that was not marked ready waited 12 h 17 min for someone to merge it (mesh-catalog #300, 2026-10-14). There is no ready mark since ADR 0421: the operator's Approve is asked as soon as the checks pass. +- **Review rounds spent on what a run or a search would have shown.** On 2026-10-19 milestone R3b of design 47 + took two review rounds and one more cycle after a failed try. A test that "failed first" only by not compiling + proved nothing, and dropping the field it was for passed every test. A reader of the changed record that the + survey never listed ended a module's conditions at each sync. The reviewer judged the bus tests by reasoning, and + wrongly, because they ran only after the review. - **A module proven only after it reached the machines.** The mail modules were delivered with unit tests only, then failed live three times (issues 859, 865, 868) and took five more hours of fixes. @@ -42,13 +47,13 @@ opened **once**, after the review. It runs in five steps, and its time is bounde | Step | What happens | Bound | |---|---|---| | 0. Prepare | the lead settles the open questions and writes one context file per repository | 15 minutes | -| 1. Develop | one fixing agent per repository builds every issue locally, the whole unit suite before every commit | the milestone's issues, 30 minutes each | -| 2. Review | one reviewer reads all of it; reports go both ways; at most 3 rounds | 15 minutes | -| 3. Check | push; `mesh-controller.try` runs the integration tests on each pushed branch, then the pull requests are opened and the forge's merge checks run them again; no further review | one try and one check run | +| 1. Develop | one fixing agent per repository builds every issue locally, the whole unit suite before every commit; then each branch is pushed and tried | the milestone's issues, 30 minutes each, and one try | +| 2. Review | one reviewer reads all of it with the tries' results; reports go both ways; at most 3 rounds | 15 minutes | +| 3. Check | push the reviewed heads; a head the review changed is tried again; then the pull requests are opened and the forge's merge checks run the integration tests again; no further review | at most one more try and one check run | | 4. Deliver | merge; the delivery plan builds, publishes and sends the whole feature | its delivery plan | -**One milestone is one release** (the operator, 2026-10-17). A milestone is developed whole, reviewed once, -pushed once and delivered once. It is never cut into steps for its size or for time: every cut adds a review, a +**One milestone is one release** (the operator, 2026-10-17). A milestone is developed whole, tried before its review, reviewed once, +opened once and delivered once. It is never cut into steps for its size or for time: every cut adds a review, a merge check, an Approve and a delivery plan of waiting, so the fewest cuts is the fastest. Milestones are cut by release order when the design is broken up (playbook [04](04-build-handoff.md)); a release-order dependency found inside a milestone is a fault of that cut, said to the operator, never a silent extra step. @@ -93,7 +98,10 @@ never into the running one, unless it blocks the running one. 5. **Write the findings into the issues** (the operator, 2026-10-17: the better the content of the issues, the better work the agents do, and the less review is needed). Before any agent starts, each issue carries, with `issue-tracker.note`: every file and test that touches the change, found by search and not from memory - (file:line); how the change is checked and delivered end to end (which test proves its done-when, which + (file:line); **every other reader and writer of each value the change adds or changes** (a field, a key, a kind + of record), each with what it does with the new value, and a criterion for each that could treat it wrongly + (2026-10-19: the router's sync with the controller ended every condition the controller did not list, and the + survey, which followed only the new condition's own path, never named it); how the change is checked and delivered end to end (which test proves its done-when, which modules its delivery plan moves, to which machines); and for a group, its merge order. The context file points to these notes; an issue reread by another session or a later agent then holds what was found. When the reading is large, it starts with a read-only survey agent on the smaller model. @@ -133,13 +141,20 @@ never into the running one, unless it blocks the running one. the next instruction, so its context is paid for once. - **Unit tests only, each well under a second** (how-we-build §5, ADR 0391). **The whole unit suite runs before every commit** (the operator, 2026-10-17), not only the tests of what changed: it takes under a second, and a - change breaks tests in packages it never touched. Each issue's first test fails before its change and passes - after; `gofmt`, `go vet` and the whole unit suite stay green. **The tests are written from the issue's + change breaks tests in packages it never touched. Each issue's first test fails before its change, on an assertion about what the + code does, and passes after. **A test that fails only because it does not compile has not failed first**: where + the test calls what the change adds, the agent takes out the change's effect (the line that does it, not the new + function), sees the test fail, and puts it back (2026-10-19: a check request's new field was proven by a test of + its helper alone, and dropping the field from the request passed every test); `gofmt`, `go vet` and the whole unit suite stay green. **The tests are written from the issue's acceptance criteria**: one test or more per criterion at the level it names, each test's comment citing the criterion it proves by its label (`// 1118/AC2`), and no criterion left without its test; an integration criterion is a test that compiles and waits for the check step. The integration tests (those that start a store, a bus or a process) wait for the check step, where `mesh-controller.try` runs them first. -- **Nothing is pushed** and no pull request is opened in this step. Never land on a red check: run the +- **No pull request is opened in this step. When every issue is committed, the lead pushes each feature branch + and tries it** (`mesh-controller.try`, as step 3 item 1 says), so the review reads the integration tests' real + results and judges none by reasoning (2026-10-19: a reviewer reasoned that a warning goes out at once, the + router holds it 30 s, and the try after the review found it). A failed try goes back to the same fixing agent + with its whole log, read as step 3 item 3 says, before the review starts. Never land on a red check: run the checks, read the result, then land in a separate step. - **The fixing agent's brief** (the lead gives it with the context file): name your issue in every step and message ("issue 805: landing in mesh-catalog"); reach the mesh only through the mesh MCP server, never ssh; @@ -172,7 +187,7 @@ never into the running one, unless it blocks the running one. It says which round it is in ("round 1 of at most 3"). 3. **The report goes to the same fixing agents** that wrote the code, continued by their ids, so they fix with the context they already hold. Each answers the reviewer: what it changed, per finding, or why not. -4. **Every new integration test is read against the code it drives before the first push.** It has never run: fixing agents only compile it, and its first run is the merge check, so each fault it hides costs a whole check. The reviewer traces each step of each new integration test through the code (fixtures against the manifest rules, what the store keeps and reads back, who holds a seat), and says which steps it could not trace. On 2026-10-17 a new test failed on its first two runs, on a fixture and then on a fault of the change; reading it against the code after the first failure found another fault, which would have failed a run too. The reviewer also checks that each new integration test enters through the same path the live trigger takes, not through a state it writes by hand: on 2026-10-17 milestone 4's test saved its delivery plan directly, so it skipped the cut from a batch that every live plan takes, and it passed while the backup step never ran (issue 1086). +4. **Every new integration test is read against the code it drives and against its try.** Fixing agents only compile it, and its one run before the review is the try (step 1), which shows whether it passed but not whether it proves what it says. The reviewer traces each step of each new integration test through the code (fixtures against the manifest rules, what the store keeps and reads back, who holds a seat), and says which steps it could not trace. On 2026-10-17 a new test failed on its first two runs, on a fixture and then on a fault of the change; reading it against the code after the first failure found another fault, which would have failed a run too. The reviewer also checks that each new integration test enters through the same path the live trigger takes, not through a state it writes by hand: on 2026-10-17 milestone 4's test saved its delivery plan directly, so it skipped the cut from a batch that every live plan takes, and it passed while the backup step never ran (issue 1086). 5. **The reviewer reads the answers and the new commits**, and approves or reports again (step 7 says what a round may ask). After round 3 the lead stops and brings the operator what is left, with a recommendation. 6. **The reviewer reads every acceptance criterion against its test**: each criterion has a test at its level @@ -186,12 +201,13 @@ never into the running one, unless it blocks the running one. ## Step 3 — Check (no review) -1. **Push every feature branch, try it, then open one pull request per repository, together.** - **The integration tests run once before the pull requests are opened**: `mesh-controller.try` with the +1. **Push the reviewed heads, try again a head the review changed, then open one pull request per repository, + together.** **The integration tests run before the pull requests are opened**, first before the review (step 1) + and again for a head the review changed since its try: `mesh-controller.try` with the repository and the pushed branch runs the repository's `merge-check.sh` (main's version) on the merge, in the toolchain containers and against the throwaway store and bus a merge check gets, and answers with an id to read back. A try that fails goes to step 3 item 3 as a failed check does; the pull requests are opened only - when every try passed (the operator, 2026-10-18: three pushes in a row failed their first merge check on + when every head's last try passed (the operator, 2026-10-18: three pushes in a row failed their first merge check on faults a try finds in minutes). Never `merge-check.sh`, `go test -tags integration` or a store or bus started by hand on the machine (the operator, 2026-10-18). Pull requests in two or more repositories that share the branch name are one delivery group (ADR 0239 §2). diff --git a/00-META/process/08-small-fixes.md b/00-META/process/08-small-fixes.md index a3df1a1c..35022704 100644 --- a/00-META/process/08-small-fixes.md +++ b/00-META/process/08-small-fixes.md @@ -18,8 +18,8 @@ hour is a feature. ## The aim -A small fix spends its time on the fix, and runs the long part once: **one review, one push, one check run, -one delivery**. Everything before the push is local and fast. +A small fix spends its time on the fix, and runs the long part once: **one try before the review, one review, one check run, +one delivery**. Everything before the try is local and fast. ## Steps @@ -34,20 +34,22 @@ one delivery**. Everything before the push is local and fast. 2. **Move the issue to `fixing`** with `by` as who@node. 3. **Build on a local branch** `fix/-`, in a worktree of its own branched from `main`, never in the repository's own clone, which stays on `main` (playbook [07](07-feature-branches.md), step 0, item 2). Write the - test that fails today first; make the change; keep `gofmt`, `go vet` and the repository's whole unit suite green + test that fails today first, on an assertion about what the code does: a test that fails only because it + does not compile has not failed first (playbook [07](07-feature-branches.md) step 1); make the change; keep `gofmt`, `go vet` and the repository's whole unit suite green before every commit (each test well under a second; never a merge check or a database locally: how-we-build §5, ADR 0391). The tests are written from the acceptance criteria, at least one per criterion at its level, each citing the criterion it proves by its label (`/AC`, playbook 07 step 0 item 5). Commit, naming the issue. For a playbook or design edit, run the repository's own checks (in hq, - `sh merge-check.sh`). -4. **One review of the local commit, before anything is pushed** (a reviewer that did not write it, larger + `sh merge-check.sh`). Then push the branch and run `mesh-controller.try` on it, so the review reads the + integration tests' real results; a failed try goes back to the same agent before the review. +4. **One review of the commit and its try** (a reviewer that did not write it, larger model; at most 3 rounds, as playbook 07; from round 2 only what is wrong; it reads each acceptance criterion against its test, as playbook 07 step 2 item 6; a finding outside the issue's done-when and its acceptance criteria is opened as an issue of its own, never asked as a change). The reviewer's findings go back to the same agent that wrote the fix, continued by its id, so the fix keeps its context. A playbook or design edit of a few lines may skip the reviewer: the operator's Approve of its merge is its review, and the review record says so. -5. **Push once, try it, and open the pull request.** The trunk is what runs and nothing holds merged work back +5. **Push the reviewed head, try it again if the review changed it, and open the pull request.** The trunk is what runs and nothing holds merged work back (ADR 0401), so a fix waits for no other change's delivery. `mesh-controller.try` on the pushed branch runs - the integration tests once, and the pull request is opened only on a pass; the forge's merge check runs + the integration tests, and the pull request is opened only when the head's last try passed; the forge's merge check runs them again as the gate (playbook [07](07-feature-branches.md) step 3). They are never run on the machine. A failed try or check goes back to the same agent with the failure, and the reviewer reads only the change it makes. 6. **The pull request's description carries the review record** (`Reviewed-by: at `,