Merge pull request 'Issues 301, 302: a push of recorded builds is no repair; the lab's Go replays are checked in Go' (#184) from issues/301-302-a-push-of-recorded-builds-and-the-labs-go into main
This commit was merged in pull request #184.
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
layer: to-be
|
||||
status: in-progress
|
||||
code: [mesh-controller, mesh-host, mesh-tools, mesh-catalog, mesh-sdk, mesh-lab]
|
||||
updated: 2026-10-07
|
||||
updated: 2026-10-08
|
||||
decisions:
|
||||
- 02-DECISIONS/0240-a-module-says-how-it-is-healthy-and-the-node-engine-judges-it.md
|
||||
- 02-DECISIONS/0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md
|
||||
@@ -424,6 +424,16 @@ row no longer sees.
|
||||
`hand-act record` refuses that cause, so a repair cannot pass for a drill by the word it gives (issue
|
||||
292; checked by the controller's S15 tests).
|
||||
|
||||
**A push of recorded builds is no repair.** A build of a module whose upgrade policy records moves only
|
||||
by a person's push ([ADR 0242](../../02-DECISIONS/0242-a-recorded-build-moves-only-by-a-persons-push-and-a-send-says-what-it-recreates.md)),
|
||||
and that push is the word the policy asks for. So a push says what it was, read by the controller from
|
||||
what it carries and never from a word the person gives. It is a push of recorded builds when it names one
|
||||
machine, that machine's last send is known and its last report is no failure, and every build it moves
|
||||
records rather than rolls out: a held new build, or a recorded module's first build on a machine newly
|
||||
assigned it. The act is written with its kind and what it carried; the table lists a push of that kind as
|
||||
a person's decision, and S15 never counts it. Any other push by hand still counts (issue 301; checked by
|
||||
the controller's replay R301 and its tests of the rule).
|
||||
|
||||
> **Progressive insight — 2026-10-06.** This paragraph said before: "except `retire-waiting` and
|
||||
> `cleanup-waiting`: approving a retirement and deleting what was retired are a person's decision by
|
||||
> design (ADR 0230), and no healer may take them over." The exception was keyed on two causes, and the
|
||||
@@ -536,7 +546,10 @@ A changed file touches exactly the modules whose build reads it; a file no build
|
||||
change reaching a module, or adding one, runs **the gate** on the build seat (`mesh/merge-gate`): the
|
||||
touched manifests through `module check`, failing only what the change brings; every machine composed with
|
||||
the definitions of the modules the plan moves or adds; the replays. Every repository of the mesh runs **its
|
||||
own** `merge-check.sh` (`mesh/repo-check`) in the toolchain it declares; one with none has "no repository
|
||||
own** `merge-check.sh` (`mesh/repo-check`) in the toolchain it declares. A repository in two languages
|
||||
declares each further part of its check and the toolchain that part runs in, and the build seat runs each
|
||||
part in that toolchain's own container: the layer passes only when every part does (issue 302; checked by
|
||||
the builder's `TestARepositoryInTwoLanguagesIsCheckedInBoth`). A repository with none has "no repository
|
||||
check defined", a success where the branch does not require `mesh/repo-check` and a failure for a person
|
||||
where it does. **A note is never a warning on a required status**: the forge combines `warning` as a
|
||||
failure, so a note — a wide rebuild, a problem already so on the base — is a success that says it, and only
|
||||
|
||||
+106
@@ -0,0 +1,106 @@
|
||||
---
|
||||
status: resolved
|
||||
opened: 2026-10-07
|
||||
located-in: [mesh-controller cmd/mesh-controller (handacts.go, recorded_push.go, signals.go S15)]
|
||||
fixed-by: novox/mesh-controller PR #126
|
||||
replay: R301
|
||||
amended-design: 03-DESIGN/01-to-be/45-a-core-that-cannot-fail-silently.md
|
||||
---
|
||||
|
||||
# 301. A person's push of a recorded build was counted as a repair
|
||||
|
||||
## Symptom
|
||||
|
||||
On the evening of 2026-10-07 the controller's `conditions` held three warnings from S15:
|
||||
|
||||
- `mesh.hand-acts.split-dns.healer-wanted`
|
||||
- `mesh.hand-acts.words.healer-wanted`
|
||||
- `mesh.hand-acts.uplink-verbs.healer-wanted`
|
||||
|
||||
The first one read:
|
||||
|
||||
> "split-dns" was repaired by hand 4 times in 14 days … a healer is wanted for it
|
||||
|
||||
None of the ten acts behind them was a repair. Each was `mesh-controller.push {node, why, cause}`
|
||||
through the seat, with the operator's yes as its why. Machine by machine, they rolled out new builds of
|
||||
the resolver (`systemd-resolved`, its first build on the laptop and the workstation), the FortiClient
|
||||
adapter, the two network managers, the packet filter and mail. Every one of those modules declares the
|
||||
upgrade policy `record`. [ADR 0242](../../02-DECISIONS/0242-a-recorded-build-moves-only-by-a-persons-push-and-a-send-says-what-it-recreates.md)
|
||||
says a recorded build moves only by a person's push. So each push was the person's word that the policy
|
||||
asks for: the mesh working as decided.
|
||||
|
||||
## Why it is a design issue
|
||||
|
||||
To-be 45 §7 says a push by hand counts toward `healer-wanted`, because roll-out by default
|
||||
([ADR 0236](../../02-DECISIONS/0236-a-build-is-judged-on-its-first-machine-and-put-back-by-something-other-than-itself-and-so-it-rolls-out-unattended.md))
|
||||
exists to end pushes by hand. ADR 0242 then made one kind of push the only way a recorded build moves.
|
||||
The two rules were never put side by side. A module the mesh may never roll out by itself is exactly one
|
||||
no healer may move, so a healer for it is never wanted. Yet every time a person rolled out such a module
|
||||
the condition was raised. And the more carefully the person did it, one machine at a time, the sooner it
|
||||
was raised.
|
||||
|
||||
The table of verbs that write the log ([issue 292](../292-a-drill-was-counted-as-a-repair/00-report.md),
|
||||
to-be 45 §7) reads whether an act is a repair from its verb. Here that is not enough: the push the policy
|
||||
asks for and a push that repairs are the same verb. What tells them apart is what the push carried, and
|
||||
the log did not keep that.
|
||||
|
||||
## Fix
|
||||
|
||||
**A push says what it was, read from what it carried.** It is never read from a word the person gives,
|
||||
which issue 292 ruled out. Before a push that names one machine is recorded, the controller reads what
|
||||
that machine was last sent against what the mesh holds. The push is a **push of recorded builds** when
|
||||
all of these hold:
|
||||
|
||||
- what the machine was last sent is known;
|
||||
- its last report is no failure or refusal, so the push is not re-sending to mend one;
|
||||
- it moves at least one module's build; and
|
||||
- every module it moves declares `record`: either a held new build of a module the machine runs, or the
|
||||
first build of one newly assigned there.
|
||||
|
||||
Anything else counts as a push by hand, as before: a build that rolls out, a module taken off, or a push
|
||||
that moves nothing.
|
||||
|
||||
The act is written with `kind: recorded-builds` and, in `carried`, each module's move from one build to
|
||||
the next. The table of verbs reads a push of that kind as a person's decision, and S15 never counts it.
|
||||
`hand-acts` prints it as such. The push prints which kind it is, and when it is not a push of recorded
|
||||
builds, why.
|
||||
|
||||
**The three open warnings clear by observation.** The ten acts behind them were recorded before a push
|
||||
said its kind, and the log kept no record of what they carried. So the controller names them by their
|
||||
log ids, each with what it carried (`recordedBefore`). That list is checked in the pull request and
|
||||
covers no act recorded after the fix. S15 then reads those ten as pushes of recorded builds. On its next
|
||||
tick after the build goes out it no longer sees the three causes repeated, so the conditions close.
|
||||
Nobody closes them by hand.
|
||||
|
||||
A fourth warning, `mesh.hand-acts.push.healer-wanted`, stays open. It comes from four pushes that set a
|
||||
setting on each machine and one that sent a new holder's assignment. Those pushes carried a settings
|
||||
change and a module that rolls out, not a recorded build. Whether either is a repair is a separate
|
||||
question.
|
||||
|
||||
## How it is checked
|
||||
|
||||
- **The replay, R301**, in the controller (`TestReplay301APersonsPushOfAHeldRecordedBuildIsNoRepair`).
|
||||
It sets up two machines running a recorded module, then registers the module's new build. Each
|
||||
machine gets a push, recorded the way the seat's push records it, with the same cause. S15 then reads
|
||||
the log as the watchdog does. The test fails on the commit before the fix with "split-dns was repaired
|
||||
by hand 2 times in 14 days", and passes on the fix. It is registered in mesh-lab's replays register
|
||||
(novox/mesh-lab PR #59).
|
||||
- `TestAPushIsOfRecordedBuildsOnlyWhenEveryBuildItMovesWasHeldForAPersonsWord` covers the rule case by
|
||||
case. One held build, two, and a recorded module's first build are pushes of recorded builds. A build
|
||||
that rolls out, a push that moves nothing, a module taken off, an unknown last send and a failed last
|
||||
report are not.
|
||||
- `TestThePushesOfRecordedBuildsOf20261007WantNoHealer` takes the live log's ten acts as recorded and
|
||||
checks that the three warnings do not open. It also checks that the settings pushes still count, a push
|
||||
with no kind still counts, and another verb under a listed id is not read as such a push.
|
||||
|
||||
Live: once the build goes out, the three `healer-wanted` warnings close within a watchdog tick, and
|
||||
`hand-acts` marks the ten pushes as pushes of recorded builds.
|
||||
|
||||
The trail is in [01-diagnosis.md](01-diagnosis.md).
|
||||
|
||||
## Resolved — 2026-10-08
|
||||
|
||||
Fixed by novox/mesh-controller PR #126. The replay is R301 in mesh-lab's replays register: run against
|
||||
the fix's branch, it fails on the commit before the fix and passes on the fix. Once the pull requests
|
||||
merge, the build goes out like any controller build. Watching the three warnings close then is the live
|
||||
check.
|
||||
+42
@@ -0,0 +1,42 @@
|
||||
# 301 — diagnosis
|
||||
|
||||
## 2026-10-07: where a push becomes a repair
|
||||
|
||||
`mesh-controller.push` writes its entry in the hand-act log (`handActFlags.record`) before anything
|
||||
else, before it reads the store. The entry holds the verb `push`, the machine, the why and the cause.
|
||||
S15 (`watchHandActs`) counts every entry that the table of verbs (`handActVerbs`) does not mark as a
|
||||
person's decision. `push` was listed there with no decision, and the comment beside it said what the
|
||||
design says: "a push by hand is exactly what roll-out by default exists to end".
|
||||
|
||||
So the controller could not have told these pushes apart from repairs. It recorded each one before
|
||||
knowing what it would send, and kept nothing of what it sent.
|
||||
|
||||
`hand-acts` showed all ten acts with the causes the person gave (`split-dns`, `words`, `uplink-verbs`).
|
||||
Each why names the modules moved. All of them (`systemd-resolved`, `forticlient`, `networkmanager`,
|
||||
`systemd-networkd`, `nftables`, `mailu`) declare `upgrade.policy: record` in the catalogue, each with
|
||||
the reason ADR 0236 §4 or ADR 0242 gives.
|
||||
|
||||
## Ruled out
|
||||
|
||||
- **Keying the exemption on the cause.** Issue 292 ruled this out: any repair could then avoid S15 by
|
||||
using that word.
|
||||
- **A separate verb for such a push**, as issue 292 gave drills `hand-act drill`. For a drill, the person
|
||||
is the only one who knows that something was broken on purpose. For a push of recorded builds, the controller can
|
||||
read the answer from its own store: what the machine was last sent, what the mesh holds, and each
|
||||
module's policy. A second verb would make the person pick the right one, and a recorded build pushed with
|
||||
the wrong verb would still raise the condition. It would also leave the push itself unable to say what it
|
||||
carried. Reading the kind from the send is what the mesh observes, so it is the answer that needs no
|
||||
one's word.
|
||||
- **Reading the kind at S15's tick instead of at the push.** By the time the watchdog reads the log, the
|
||||
machine's last send is the push itself, and what it moved from is gone. The kind has to be read before
|
||||
the push sends, and kept in the act.
|
||||
- **Reconstructing the ten acts from the store.** The store keeps only each machine's last send
|
||||
(`sent_builds`), not a history of sends. Nothing in it says what those pushes moved. Their whys and the
|
||||
catalogue's policies do say it, so the controller names them, and the pull request is where that list
|
||||
is checked.
|
||||
|
||||
## What is not compared
|
||||
|
||||
A push carries the machine's whole declaration (ADR 0221). A push of a recorded build also carries any
|
||||
settings or grants that changed with it. Those are not compared: such a push is judged by the builds it
|
||||
moves. A push that moves no build is never one, so a settings change pushed alone still counts.
|
||||
@@ -0,0 +1,77 @@
|
||||
---
|
||||
status: resolved
|
||||
opened: 2026-10-07
|
||||
located-in: [mesh-controller internal/builder (check.go, the build agent's merge check), mesh-lab merge-check.sh]
|
||||
fixed-by: novox/mesh-controller PR #125, novox/mesh-lab PR #59
|
||||
replay: R302
|
||||
amended-design: 03-DESIGN/01-to-be/45-a-core-that-cannot-fail-silently.md
|
||||
---
|
||||
|
||||
# 302. The lab's Go replays were never checked before merge
|
||||
|
||||
## Symptom
|
||||
|
||||
The verdict report of novox/mesh-lab PR #58, which registered the replays of issues 296, 299 and 300,
|
||||
read via `mesh-delivery.checks`:
|
||||
|
||||
> NOT CHECKED HERE: replays/ (Go) — the TypeScript toolchain holds no Go compiler
|
||||
|
||||
`mesh/repo-check` passed. The pull request changed only `replays/register.go`, Go code that every core
|
||||
change's gate runs, and nothing on the build seat had compiled it, vetted it, or run its own test,
|
||||
`TestEveryReplayIsWhole`. A register that did not compile, or that registered a replay with no fix, would
|
||||
have merged green. The first gate of a core change after it would then have failed for a reason that
|
||||
had nothing to do with that change.
|
||||
|
||||
## Why it is a design issue
|
||||
|
||||
To-be 45 §9 and ADR 0237 (as amended) say every repository of the mesh runs its own `merge-check.sh` in
|
||||
the toolchain it declares. The build agent read one toolchain per repository from the script's header
|
||||
and ran the script in that toolchain's container. The lab is in two languages: a TypeScript lab, and the
|
||||
Go replays register that the gate itself runs. Its script could declare only one toolchain. The script
|
||||
did say the gap, and the rule that nothing is passed silently was kept. But no step of any check could
|
||||
close it, so the replays were reviewed only by eye and proven by hand.
|
||||
|
||||
## Fix
|
||||
|
||||
**A repository's own check has parts, each in its own toolchain.** Among its first twenty lines,
|
||||
`merge-check.sh` declares its toolchain as before (`# mesh-check-toolchain: <toolchain>`). It may now also
|
||||
declare further parts, one line each: `# mesh-check-also: <toolchain> <script>`. The build seat runs the
|
||||
script first, then each part in the order declared, from the repository's root and in the named
|
||||
toolchain's own container, with the same environment and none of the container runtime's socket.
|
||||
`mesh/repo-check` passes only when every part passes. A part fails the layer when it fails, when the
|
||||
repository does not hold its script, or when its path leads outside the repository. A part whose
|
||||
toolchain the mesh does not hold is an error, never a pass. The summary names each part that ran and its
|
||||
toolchain.
|
||||
|
||||
**The lab declares its Go part.** `replays/merge-check.sh` runs in the Go toolchain. It checks that the
|
||||
register, the replays and the prover are gofmt'd, runs `go vet` over them (which compiles every test
|
||||
file), and runs the tests that need only Go and git: `TestEveryReplayIsWhole` and the bed's
|
||||
`TestTheBedProvesWhatTheChangeTouches`. The replays themselves still do not run there, because they raise
|
||||
containers and the check's container holds no container runtime. The script says so on every run, and
|
||||
the gate of every core change still runs the replays from the lab's main. By hand with Go at hand, the
|
||||
lab's `merge-check.sh` runs the Go part itself.
|
||||
|
||||
## How it is checked
|
||||
|
||||
**The check:** `mesh/repo-check` on every mesh-lab pull request. Its report shows a
|
||||
`--- its replays/merge-check.sh (go toolchain)` section, and its summary lists
|
||||
`replays/merge-check.sh (go)` among the parts that passed. A register that does not compile or is not
|
||||
whole fails the status by name.
|
||||
|
||||
- `TestARepositoryInTwoLanguagesIsCheckedInBoth` (mesh-controller `internal/builder`) reads the parts from
|
||||
the script's header. It checks that each part runs in its own toolchain's image, in order. A failing
|
||||
second part fails the layer, naming the part and what failed. A failing first part stops the check
|
||||
before the second runs. A declared script that is missing, or outside the repository, fails. A part in
|
||||
a toolchain the mesh does not hold is an error.
|
||||
- **The replay, R302**, in mesh-lab's register, is a `gate` replay of that test: on the commit before the
|
||||
fix the check it replays did not exist, and on the fix it passes.
|
||||
|
||||
The trail is in [01-diagnosis.md](01-diagnosis.md).
|
||||
|
||||
## Resolved — 2026-10-08
|
||||
|
||||
Fixed by novox/mesh-controller PR #125, which runs a declared part in its own toolchain, and
|
||||
novox/mesh-lab PR #59, which declares the replays as the lab's Go part. Until the controller's build
|
||||
reaches the build seat, the lab's new header line is read as a comment, and its pull requests are checked
|
||||
as before. The live check is the first lab pull request checked after the controller's build goes out, whose report must show
|
||||
the Go part.
|
||||
@@ -0,0 +1,35 @@
|
||||
# 302 — diagnosis
|
||||
|
||||
## 2026-10-07: how a repository's check picks its toolchain
|
||||
|
||||
The build agent's merge check (mesh-controller `internal/builder`, `Check`) runs two layers. The second,
|
||||
`mesh/repo-check`, is the repository's own `merge-check.sh`. The agent reads the script's toolchain from
|
||||
a `# mesh-check-toolchain: <toolchain>` line among its first twenty lines, defaulting to go
|
||||
(`ScriptToolchain`). It takes that toolchain's image from the toolchains the mesh holds (`ToolchainsOf`:
|
||||
the Go toolchain, and the TypeScript build image of the mesh's tools), and runs `sh merge-check.sh` in
|
||||
one container of it. The controller's script declares `go`, which is the "go toolchain" its reports name.
|
||||
The lab's declares `typescript`, because the lab is a TypeScript program.
|
||||
|
||||
The TypeScript toolchain holds Node and the compiler, and no Go. The lab's script knew this. It ran the
|
||||
Go part only `if command -v go`, and otherwise printed "NOT CHECKED HERE". On the build seat it always
|
||||
took the second branch.
|
||||
|
||||
## Ruled out
|
||||
|
||||
- **Go in the TypeScript toolchain's image.** That image is what every TypeScript module of the mesh is
|
||||
built on, and it is pinned and fingerprinted as their base. Adding a compiler to it for one
|
||||
repository's check would rebuild every module that stands on it, and it would hide the fact that the
|
||||
check needs two toolchains.
|
||||
- **Declaring the lab `go`.** Then its TypeScript sources and unit suite would go unchecked instead.
|
||||
- **Running the replays' Go code inside the gate only.** The gate does run them, from the lab's main, for
|
||||
core changes, and nowhere for a change to the lab itself, which is where a broken register comes in.
|
||||
Checking a lab pull request is the repository check's job, not the gate's.
|
||||
- **A second status per toolchain.** The forge's branch protection names one `mesh/repo-check`. One layer
|
||||
whose parts must all pass keeps the protection, the merge gate's reading and the delivery's checks as
|
||||
they are.
|
||||
|
||||
## The fix
|
||||
|
||||
Covered in the report. The order is script first, then parts as declared. A script that fails stops the
|
||||
layer before the parts run, as `set -e` would. A part's path must be inside the repository and must
|
||||
exist, so a mistyped header line fails the check instead of being skipped.
|
||||
Reference in New Issue
Block a user