Keep the controller and node-engine at the build a machine was last sent over an older record (hq issue 201) #273
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-controller#273
Reference in New Issue
Block a user
What
Every send but a person's push (every send through sendToEach: plans, releases, healers, rotations, secrets given) composes mesh-controller and mesh-host at the build the machine was last sent when the mesh's record of it is older by the builds' registration order (issue 219), and carries everything else. Nothing is refused and nothing is retried. A plan whose own build is the older one finds its gate superseded by the kept build (issue 352) and ends.
Exceptions: a gate's own put-back carries its modules back (sendScope.puttingBack, set in gateFailed and sendRollbacks). A sent build that failed its gate is never kept, so when a put-back's own send was refused, the next automatic send completes the put-back.
A warning condition
core.<component>.<machine>.core-downgrade-keptsays it. It names the two ways out:push <node>takes the older build, and a newer merge orrebuild <module>moves the record forward. It is cleared after any send that carries the record or does not carry the module, a person's push included (overTheBus.declare).If the kept build's manifest is no longer in the build records, the send to that machine is refused and says why, as for a recorded module (ADR 0245). The other machines are still sent.
Why
On 2026-10-02 a plan sent the control node a controller older than the one running there, and the mesh went quiet for ten minutes. ADR 0185 took the outage out of it, and the race stayed open. The decision of 2026-10-11 on issue 201 closes the class without pinning the trigger. Review 90: the first version refused the whole send, and that wedged a machine after a gate put-back whose own send was refused.
Design limits chosen
How it was tested
sh merge-check.shagainst a throwaway postgres 17: exit 0.Fixes issue 201
Delivery
novox/mesh-controller@b3787e4d8eb2— delivered since 2026-10-12T18:57:32ZDelivery plan — builds mesh-controller → novox; no bus step
Machines
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
9038d4b0Judged: head
9038d4b0d317merged into trunkf10aedac5880; read beside it: mesh-catalog23ea0f431faf, mesh-controller-mainf10aedac5880, mesh-hosta1f9856b3957, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on ace as
build-1791822107369334674(builds --log build-1791822107369334674).Verdict: request changes at
9038d4b0d3Independent review for hq issue 201 (a push recreated the controller at a digest older than its seat row). The guard is well placed and its tests are good, but it wedges a machine in one state that the code itself produces, and the condition does not clear when a person acts. Details below.
What I ran (head
9038d4b, local clone; also merges cleanly onto mainf10aeda)go vet ./...: clean.go test -race -count=1 -run 'TestReplay201|TestACoreModule|TestARefusedCoreDowngrade' -v ./cmd/mesh-controller/against a throwaway Postgres 18 (MESH_TEST_POSTGRES): all 3 PASS, and TestReplay201 ran against the store (0.18s).refuseCoreDowngradesreturned nil, nil at once), and TestReplay201 FAILED with "a plan's send of the older controller to the machine running the newer is refused: ". So the test proves the guard.sh merge-check.shwith MESH_TEST_POSTGRES: exit 0, with no FAIL and no NOT RUN lines.9038d4b0d3merged into trunkf10aedac58— pass … every machine composes"), mesh/repo-check success, mesh/delivery success ("ready"). The forge says it is mergeable. There are no reviews before this one.Checklist
1. Legitimate rollbacks.
gateFailed,sendRollbacks, and through themfailCarried/putBackBroken) carriesputtingBacktogether with its modules. It is exempt only for the modules it puts back. That is correct.pushCommand) does not go throughsendToEach/ungatedIn. The bus step returns early onscope.person. Neither is touched.buildAskedreturns the newest recorded build of the commit, so the revert is judged newer. Issue 280's "standing build" registers the old artifacts at the new commit, so it is still judged newer.gatedSendwithjudgedset, so the guard applies to it. See finding A.2. Wedging (finding A, blocking).
errCoreDowngradeis an ordinary step error, andadvanceHeldkeeps it as the plan's note with "— tried again" on every tick, without a bound. Only three things end it: a newer registered build, a person's push, or the machine's sent record changing.The trouble is that the guard cannot tell the 201 race apart from a gate's put-back that was registered and then not sent. In
gateFailed,RestoreModule(previous)runs first and the send follows. If that send is refused, the record is at the older build while the machine was sent the failed, newer-asked one.notBacksays this already. The send can be refused because oferrUngatedfrom another build registered during the gate window (two merges a minute apart is exactly the 201 setting),errBusWaits, or a send error. TestReplay201's fixture is exactly this state: the record is restored to the build before, and the machine was sent the newer one.Before this PR, the next automated send to that machine finished the put-back, because the older build is gated. After it, every non-person send to that machine is refused whole: the plans' first and rest sends, release plans, healers, rotations, secrets given, and the hub send. Each walk retries forever. Because only one walk runs at a time (ADR 0276), the open walk holds every other walk too. Nothing but a person or a new merge ends it.
Suggested fix: a build that failed its gate is never "ahead". Do not count a move as a downgrade when its From build has a recorded failed gate (
inv.GateFailedon that build's id, or the gate record's rollback state NotRolledBack/RollingBack withprevious == To). Add a test for that: gate failed, put-back send refused, then a healer send goes through.3. Order (no blocking finding). The comparison uses asked time (or recorded time), which is the same order
RegisterModuleuses (built_asked <=). The two orders differ only for builds that are recorded but not registered. Off-trunk builds have distinct commits. Failed-gate builds lead to finding A. Builds outside the 50-newest window, or with no record, leave the move unproved and let it pass (fail open), which is the right direction.I found no case where a genuinely newer trunk build is judged older. A plan retry or a
rebuildof an old commit is asked anew, so it becomes "newer" by this order. That matches registration and the decision's wording ("by the build's registration order"), but it means the guard does not stop that kind of downgrade. A comment should say so.Minor: "the build its machine runs" is implemented as the build it was last sent (
SentBuilds), not the one it reports running. That is fine as a choice, but the doc comment and the condition should say "was sent".4. Condition (finding B, should fix, plus wording).
ungatedIn, andungatedInis never reached by a person's act:pushCommandbypassessendToEach, andscope.personreturns before the check. So after the person does what the condition asks, the warning stays until some later automated send reaches that machine, which could be hours on a quiet machine. Clear it on the person's path as well, for example by runningrefuseCoreDowngrades' clear for the pushed machine(s) when the push carries no downgrade, or by clearing on any successful send to the node.push <node>accepts the older build, because it sends the record. A newer merge orrebuild mesh-controllermoves the record forward. Also consider saying in the explanation that every automated send to that machine is held until then (that is what the operator will notice), and whether Warning is the right severity for that.5. Tests and checks: they pass, as listed above.
6. Statuses: all three are green and read.
Other notes (not blocking)
ungatedIntomachineMoves(..., true)and then skippingf.gated(mv.Module, mv.To)gives the samewaitinglist as before. I checked this againstmoves().Refuse a send that puts the controller or node-engine behind its machine's build (hq issue 201)to Keep the controller and node-engine at the build a machine was last sent over an older record (hq issue 201)Merge check at
d1704912Judged: head
d17049125655merged into trunkf8daec01d3f0; read beside it: mesh-catalog23ea0f431faf, mesh-controller-mainf8daec01d3f0, mesh-hosta1f9856b3957, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on g14 as
build-1791823881989317567(builds --log build-1791823881989317567).Verdict: request changes at
d170491256Re-review against review 90. Keeping the core module instead of refusing the send fixes what review 90 raised: a send is no longer blocked, and a build that failed its gate is no longer kept. But a send that keeps the core module while carrying a gate now lets that gate be superseded at once, so the machine's other new builds go unjudged. Details below.
What I ran (head
d170491, throwaway Postgres 18)go vet ./...: clean.go test -raceon TestReplay201, TestARollbackWhoseSendWasRefusedIsCompletedByTheNextSend and TestACoreModuleIsKeptOverAnOlderRecord: all PASS against the store.from.failedGateexemption, and TestARollbackWhoseSendWasRefusedIsCompletedByTheNextSend FAILED ("the failed controller was kept over the build put back"). I madecoreKeptreturn nothing, and TestReplay201 FAILED. So both tests prove their guard.sh merge-check.shwith MESH_TEST_POSTGRES: exit 0, with no FAIL and no NOT RUN lines.f8daec0), mesh/repo-check success, mesh/delivery success ("ready"). The forge says it is mergeable.Review 90's findings
settleCoreDowngradesruns inoverTheBus.declare, which both a person's push (push.go:664) and every other send go through. A send that carries the mesh's record clears the condition.What ends a keep
I confirmed what moves a kept machine forward. A newer merge or
rebuildregisters a build asked later than the one the machine was last sent, socoreDowngradesstops finding it and the next send carries the record. A person'spush <node>takes the older build. Nothing keeps a machine at the old build silently: the warning stays raised for the whole time.New finding C (blocking): a send that keeps the core module supersedes its own gate
gatedSendtakes its carried moves frommachineMoves(..., true), which is computed from the record. UnlikeungatedIn, it does not leave out the moves the send keeps. So on a kept machine, the gate carriesmesh-controller c2→c1while the declaration keeps c2. After the send,SentBuildssays c2, and injudgeGatesupersededOn(mesh-controller, node)is true, which settles the whole gate asGateSupersededat once ("nothing judged, nothing put back").I proved it on the store with the
core201fixture and a throwaway test, which I did not commit:The consequence is the 201 scenario itself. The node-engine's plan sends its first machine (the control node, which keeps the newer controller) the new node-engine. The gate is superseded without judging anything, the new node-engine stays there unjudged, and the plan ends
supersededinstead of walking the rest. The same happens to every release plan's gated send and every plan whose first machine is the kept one, for as long as the record stays older. The comment "A plan whose own build is the older one finds its gate superseded … and ends" covers only the plan whose own build is the older controller. Every other module's gate on that machine is skipped as well, which goes against ADR 0236 (no build moves on a machine without a gate).Fix: in
gatedSend, leave out ofmoveseach move the send keeps, with the same filterungatedInuses (recordedKept(keepingRecorded(ctx), …), matched on module andFrom). Then the gate judges only what the declaration changes. Test: oncore201, add a rolling-out module move on anchor, run a gated send, and check that the carried moves leave out mesh-controller and that the gate is not superseded.Minor
testdata/store-tests. Without a store they skip unseen, which is what issue 459 guards against. Please add both lines.sendKeepsapplies to it too. That is fine, but the doc comment says "No send but a person's push".Merge check at
f0fd7fe8Judged: head
f0fd7fe8868emerged into trunkba685b17f5e1; read beside it: mesh-catalog23ea0f431faf, mesh-controller-mainba685b17f5e1, mesh-hosta1f9856b3957, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on shanks as
build-1791825418229347732(builds --log build-1791825418229347732).Verdict: approve at
f0fd7fe886Re-review against review 91. Finding C (a send that keeps the core module superseded its own gate) is fixed, and the minor points are fixed too.
What I checked
withoutKeptis used by bothungatedInandgatedSend. It leaves out a move when the send keeps it at the build the machine was last sent (matched on module andFrom). Its keep context is the same one the send composes under (keepingRecorded, unless the context already keeps). A gate on a kept machine now judges only the other moves.ownsadds that move back, so its gate is superseded and the plan ends, which is the documented outcome.ungatedIn. It now leaves out every kept move, not only those the grants step keeps.moves()lists only modules that roll out, andrecordedKeptskips those unless every build is kept, so the only extra moves left out are the kept core modules. Nothing else in the send changes.withoutKeptcall fromgatedSend, and TestAGateJudgesTheOtherMovesBesideAKeptCore FAILED: the gate's moves includedmesh-controller c2→c1beside the app's move. With the call in place it passes.testdata/store-tests. The doc comment now says that the bus step keeps the core modules too.Runs (head
f0fd7fe, throwaway Postgres 18)go vet ./...: clean.go test -raceon TestReplay201, TestARollbackWhoseSendWasRefusedIsCompletedByTheNextSend, TestAGateJudgesTheOtherMovesBesideAKeptCore and TestACoreModuleIsKeptOverAnOlderRecord: all PASS.sh merge-check.shwith MESH_TEST_POSTGRES: exit 0, with no FAIL and no NOT RUN lines. Its list of store tests shows all three issue 201 tests as PASS.ba685b1, every machine composes), mesh/repo-check success, mesh/delivery success ("ready"). The forge says it is mergeable.The findings from reviews 90 and 91 are all addressed: no machine is blocked, a rollback whose send was refused is finished by the next send, the warning clears on a person's push, and the gate's judging is intact.
Merge check at
f0fd7fe8Judged: head
f0fd7fe8868emerged into trunka681227af373mesh/merge-gate: FAIL — headf0fd7fe886does not merge cleanly into trunka681227af3: in conflict testdata/store-tests (modules: mesh-controller)mesh/repo-check: FAIL — headf0fd7fe886does not merge cleanly into trunka681227af3: in conflict testdata/store-testsDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on shanks as
build-1791829341724108644(builds --log build-1791829341724108644).Re-check at
b3787e4d8e(merge of maina681227ainto the approvedf0fd7fe8): the PR's own diff against main is line-for-line the approved diff (diff ofgit diff merge-base..f0fd7fe8andgit diff a681227a..b3787e4dshows no differing +/- lines); only testdata/store-tests context moved. Review 94's approval carries to this head.Merge check at
b3787e4dJudged: head
b3787e4d8eb2merged into trunka681227af373; read beside it: mesh-catalog22071dabfc7b, mesh-controller-maina681227af373, mesh-host303d4037e84e, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on ace as
build-1791830393099375212(builds --log build-1791830393099375212).