Playbooks 07 and 08: review asks only what makes a change wrong, try before opening, mark ready on approval, prove a module whole first, hand back stopped issues
From the delivery-time audit of 2026-10-16: seven review rounds on issue 704, a feature branch's first check failing on 29 problems, mesh-catalog #300 waiting 12 h unmarked, the mail modules failing live, 98 issues held in fixing.
This commit is contained in:
@@ -21,6 +21,16 @@ A small fix (one repository, one issue, about an hour of work) follows
|
||||
undelivered for a day: an issue done at 21:09 was still not on the machines at 15:00 the next day.
|
||||
- **Work that waits and nobody notices.** Most lost time was not work at all, but a check, a delivery plan
|
||||
or an agent waiting on something that had stopped, unseen.
|
||||
- **Review that keeps hardening.** One fix went through seven review rounds in five and a half hours, each
|
||||
asking for more hardening than its issue asked (issue 704, 2026-10-14). A round is for what makes the
|
||||
change wrong; everything else is an issue of its own.
|
||||
- **The first check after the whole branch is written.** A feature's catalogue branch, pushed once after
|
||||
hours of local work, met its first check on 2026-10-16 and failed on 29 problems, TypeScript errors among
|
||||
them, that its own check finds in minutes.
|
||||
- **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).
|
||||
- **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.
|
||||
|
||||
## The shape
|
||||
|
||||
@@ -107,19 +117,33 @@ never into the running one, unless it blocks the running one.
|
||||
4. **The reviewer reads the answers and the new commits**, and approves or reports again. From round 2 it
|
||||
asks changes only for a finding that makes the feature wrong; smaller points become issues of their own.
|
||||
After round 3 the lead stops and brings the operator what is left, with a recommendation.
|
||||
5. **A finding outside the issues' done-when is never a change asked, in any round.** More hardening, a
|
||||
further case, a nicer form: the reviewer opens it with `issue-tracker.open` and names the number in its
|
||||
report, and the round does not wait for it. What a round asks is only what makes the change wrong against
|
||||
its issues and its decisions.
|
||||
|
||||
## Phase 3 — Check (no review)
|
||||
|
||||
1. **Push every feature branch and open one pull request per repository, together.** Pull requests in two or
|
||||
1. **Push every feature branch and try it before any pull request.** `mesh-controller.try` with the
|
||||
repository and the branch runs the merge check in the check's own toolchain; a branch that fails goes back
|
||||
to its fixing agent, and nothing is opened until every branch passes. A local run never stands in for it:
|
||||
the check's containers carry other tools than the nodes.
|
||||
2. **Open one pull request per repository, together.** Pull requests in two or
|
||||
more repositories that share the branch name are one delivery group (ADR 0239 §2).
|
||||
Where a session pushes through the forge tool `gitea_commit` (one commit on the forge's current base), the
|
||||
reviewer first confirms that commit's tree equals the reviewed one.
|
||||
2. **The forge's merge checks run the integration tests once** (`mesh/merge-gate`, `mesh/repo-check`). No
|
||||
3. **The forge's merge checks run the integration tests once** (`mesh/merge-gate`, `mesh/repo-check`). No
|
||||
second review: phase 2 reviewed this tree.
|
||||
3. **A check that fails** goes back to the reviewer with the failure, for **one more cycle** of phase 2 with
|
||||
4. **A check that fails** goes back to the reviewer with the failure, for **one more cycle** of phase 2 with
|
||||
the same fixing agents; then push once again.
|
||||
4. **Mark each pull request ready** (`mesh/ready` and a comment `ready at <full sha>`) once the operator's word
|
||||
for the feature is given; mesh-delivery merges it when its checks pass (ADR 0364).
|
||||
5. **Prove the step's acceptance before it is marked ready.** A step that adds or changes a module runs that
|
||||
module whole (its process against a real bus and store) on the cheapest bed that can show the step's
|
||||
acceptance sentence, as playbook [01](01-research.md) says for research; the lab when only machines can.
|
||||
Unit tests alone are not that proof.
|
||||
6. **The reviewer marks each pull request ready as soon as the acceptance is shown** (`mesh/ready` and a
|
||||
comment `ready at <full sha>`, the head it approved); mesh-delivery merges it when its checks pass
|
||||
(ADR 0364). The operator's word is waited for only when the step changes what the operator sees, removes
|
||||
data, or needs a step only the operator's terminal can take; the lead says which before the step starts.
|
||||
|
||||
**One exception: a catalogue change that uses a new manifest word.** The catalogue's merge check reads
|
||||
manifests with the controller the mesh runs, so the controller's pull request is merged and delivered first;
|
||||
@@ -127,7 +151,8 @@ then `mesh-delivery.recheck` the catalogue's and merge it on the new verdict. Pl
|
||||
|
||||
## Phase 4 — Deliver
|
||||
|
||||
The merge starts the delivery plan: build, publish, send. The lead follows it to done
|
||||
The merge starts the delivery plan: build, publish, send. A merge into a branch other than `main` is not a
|
||||
delivery: no machine receives it (issue 889 while it still starts one). The lead follows it to done
|
||||
(`mesh-controller.plans` with `wait` and `until: done`), then resolves the feature's issues with `fixed-by`
|
||||
naming its pull requests, and removes what is left: no `feat/<slug>` branch and no `.work/<slug>` worktree
|
||||
survive.
|
||||
@@ -200,6 +225,13 @@ lead prepares a `/clear` as the mesh's instruction "prepare before clear" says:
|
||||
feature's phase, its branches and heads, the context files' paths, the agents' ids, the wait list and the
|
||||
next action; then the prompt the operator pastes. The context files make the feature resumable after a clear.
|
||||
|
||||
## Issues that stop
|
||||
|
||||
An issue stays in `fixing` only while an agent works on it. A step that stops, for a wait past its bound, a
|
||||
hand-over, or the session's end, moves each of its issues not delivered back to `open` with
|
||||
`issue-tracker.move` and a note saying how far it got (its branch, its last commit, what is left). A list of
|
||||
issues in `fixing` that nobody works on reads to the operator as stuck work.
|
||||
|
||||
## How it is checked
|
||||
|
||||
- A feature's issues show their commits on the feature branch (`git log --grep 'issue <n>'`), and no pull
|
||||
@@ -209,6 +241,10 @@ next action; then the prompt the operator pastes. The context files make the fea
|
||||
- After a feature's delivery, `git branch -r | grep feat/<slug>` and `git worktree list` return nothing for it.
|
||||
- More than one open pull request in a repository that share no feature name, or a feature step whose issues
|
||||
stay in `fixing` for more than a working day, is the smell: stop and bring it to the operator.
|
||||
- A review report that asks a change outside the issues' done-when, or a fourth round, is the failure of the
|
||||
review rules; `issue-tracker.list` with `flag: abandoned` lists issues left in `fixing` with nobody on them.
|
||||
- A pull request reviewed and green but not marked ready for more than 15 minutes is a lead that did not
|
||||
mark it.
|
||||
- A bed run from `.work/<slug>/mesh-lab` that fails naming a `.work/<slug>/<repo>/…` path it cannot find is a
|
||||
missing sibling worktree, not a mesh fault.
|
||||
|
||||
|
||||
@@ -31,7 +31,8 @@ one delivery**. Everything before the push is local and fast.
|
||||
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
|
||||
model; at most 3 rounds, as playbook 07; from round 2 only what is wrong). The reviewer's findings go back to the same agent that wrote the fix, continued by
|
||||
model; at most 3 rounds, as playbook 07; from round 2 only what is wrong; a finding outside the issue's
|
||||
done-when 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 word on the pull request is its review.
|
||||
5. **Push the branch, try it, then open the pull request.** `mesh-controller.try` with the repository and the
|
||||
@@ -40,17 +41,24 @@ one delivery**. Everything before the push is local and fast.
|
||||
nodes (a test that called util-linux `flock -E` passed on a node and failed on BusyBox's in the check). The
|
||||
forge's merge checks run the integration tests. A failed check
|
||||
goes back to the same agent with the failure, and the reviewer reads only the change it makes.
|
||||
6. **Mark it ready** (`mesh/ready` and `ready at <full sha>`) once the operator's word is given; mesh-delivery
|
||||
merges it when its checks pass.
|
||||
6. **The reviewer marks it ready at once** (`mesh/ready` and `ready at <full sha>`, the head it approved);
|
||||
mesh-delivery merges it when its checks pass. A fix to a module proves the issue's done-when with the
|
||||
module running whole (its process against a real bus and store) before it is marked, not with unit tests
|
||||
alone. The operator's word is waited for only when the fix changes what the operator sees, removes data,
|
||||
or needs the operator's terminal.
|
||||
7. **Follow its delivery plan to done** (`mesh-controller.plans` with `wait` and `until: done`), prove the fix on
|
||||
the mesh (the symptom gone, measured), and resolve the issue with `located-in` and `fixed-by` the same hour.
|
||||
|
||||
A fix that stops before its delivery moves its issue back to `open` with a note saying how far it got
|
||||
(branch, last commit, what is left), as playbook [07](07-feature-branches.md) says.
|
||||
|
||||
## Bounds
|
||||
|
||||
| Waiting on | Bound | Past the bound |
|
||||
|---|---|---|
|
||||
| building the fix | 1 hour | it is a feature: stop and plan it under playbook 07 |
|
||||
| the review | 15 minutes | ask the reviewer where it stands |
|
||||
| the ready mark | 15 minutes after the review approved | the reviewer marks it, or the lead does |
|
||||
| the merge check | 15 minutes | read `mesh-controller.queue` and the check's log |
|
||||
| the merge after ready | 5 minutes after the checks pass | read `mesh-delivery.show` for the delivery |
|
||||
| the delivery plan | its class's budget (`mesh-delivery.times`), at most 30 minutes | read `mesh-controller.plans`, then `mesh-delivery.stalled` |
|
||||
|
||||
Reference in New Issue
Block a user