diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index c68ea022..c240d478 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -701,7 +701,6 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, } } now := time.Now().UTC() - step := nextRollout(*state, running, policy.Together, reports, now, planWaitBound) // **Sent with others, judged with them** (issue 281): the gate of the send that carried it is its // verdict on its first machine. A failure there stopped the plan already. if state.GatedBy != "" { @@ -715,6 +714,9 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, } passedWith(m, state, state.GatedBy, lead.Gate) } + // Read after its pass is taken over from the send that carried it, so a passed gate is never judged + // again from the first machine's later reports (novox/hq issue 335). + step := nextRollout(*state, running, policy.Together, reports, now, planWaitBound) switch { case step.failed != "": // The first machine refused or failed what it was sent, or never said: the gate failed, and @@ -974,6 +976,19 @@ func failFirstSend(ctx context.Context, open *stores, p *inventory.Plan, module fmt.Printf("%s: %s\n", p.ID, p.Note) }() g := state.Gate + if g != nil && g.Verdict == inventory.GatePassed { + // **A guard: a build that passed its gate is never put back for what came after** (novox/hq issue 335). + // Not reached while nextRollout answers a passed gate with the rest to send, and advanceOnce reads it + // after a carried module takes over its lead's pass; it is here so that a path added later cannot + // overturn a verdict. If it is reached, the plan stops and says why, and the build is not marked failed. + // The module is left a stopped rollout (its Why said, sent first and not to the rest), which + // `plans retry` takes: it sends the first machine again, and the passed gate then sends the rest. + state.Why = "passed its gate; its walk then stopped: " + why + p.State = inventory.PlanFailed + p.Note = fmt.Sprintf("%s passed its gate on %s in tier %d (%s) and is kept; its walk stopped after: %s", + module, strings.Join(g.Machines, ", "), p.Tier, g.Why, why) + return + } if g != nil && slices.Contains(g.Returned, module) { // Put back at once when it broke: its rollback was made then, and is not made again. state.Why = "put back when it broke; its send failed: " + g.Why @@ -1061,6 +1076,15 @@ func nextRollout(s inventory.PlanModule, running []string, together bool, report rest = append(rest, n) } } + // **A passed gate is the first machine's verdict, and its later reports are not** (novox/hq issue 335). + // Once the build passed there, what that machine reports next is about whatever it was sent after — + // another walk's send, a push — and says nothing of this build. On 2026-10-08 a build passed on the + // laptop, its send to the rest waited on another walk, that walk sent the laptop a new declaration it did + // not report for half an hour, and the plan read the silence as the first machine never applying the + // passed build: it marked it failed at its gate and put it back. The rest are sent, as the pass said. + if s.Gate != nil && s.Gate.Verdict == inventory.GatePassed { + return rolloutStep{send: rest} + } var waiting, failed []string for _, n := range s.First { r, said := byNode[n] diff --git a/cmd/mesh-controller/replay335_test.go b/cmd/mesh-controller/replay335_test.go new file mode 100644 index 00000000..1b0c4d33 --- /dev/null +++ b/cmd/mesh-controller/replay335_test.go @@ -0,0 +1,132 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// TestReplay335 replays novox/hq issue 335 (2026-10-08): dunst's new build passed its gate on the laptop +// (healthy 3 times over 2m5s); its send to the rest was refused while another walk's build waited on the +// workstation; that walk then sent the laptop a declaration the laptop did not report on; and thirty minutes +// after the first send the plan read the laptop's silence as the passed build never applied, marked it failed +// at its gate with the pass's own words, and put it back. Written with only what the controller had before +// its fix, so it is laid over the commit before. +func TestReplay335(t *testing.T) { + t.Run("the first machine's later reports", testAPassedGateIsNotJudgedAgainFromTheFirstMachinesLaterReports) + t.Run("a walk stopped after the pass", testAWalkStoppedAfterItsGatePassedKeepsThePass) +} + +// novox/hq issue 335: a build that passed its gate on its first machine is not judged again from that +// machine's later reports. On 2026-10-08 the send to the rest waited on another walk, that walk sent the +// first machine a declaration it did not report for half an hour, and the plan failed the passed build at +// the wait's bound and put it back. +func testAPassedGateIsNotJudgedAgainFromTheFirstMachinesLaterReports(t *testing.T) { + sentAt := time.Date(2026, 10, 8, 16, 44, 45, 0, time.UTC) + judged := sentAt.Add(2 * time.Minute) + later := sentAt.Add(5 * time.Minute) + state := inventory.PlanModule{First: []string{"laptop"}, FirstAt: &sentAt, + Gate: &inventory.PlanGate{Machines: []string{"laptop"}, Verdict: inventory.GatePassed, + Why: "healthy 3 times over 2m5s", JudgedAt: &judged, Kept: true}} + running := []string{"laptop", "workstation"} + now := sentAt.Add(30*time.Minute + 9*time.Second) + + for what, reports := range map[string][]inventory.Reported{ + "no report about what it was sent since": {{Node: "laptop", At: &later, Outcome: inventory.OutcomeApplied, Current: false}}, + "another send failed there since": {{Node: "laptop", At: &later, Outcome: inventory.OutcomeFailed, Current: true}}, + "no report at all": nil, + } { + step := nextRollout(state, running, false, reports, now, 30*time.Minute) + if step.failed != "" || step.waiting != "" || !reflect.DeepEqual(step.send, []string{"workstation"}) { + t.Errorf("%s: %+v, want the rest sent as the pass said", what, step) + } + } +} + +// novox/hq issue 335: whatever stops a walk after its gate passed, the passed build is not marked failed at +// its gate, nor put back. +func testAWalkStoppedAfterItsGatePassedKeepsThePass(t *testing.T) { + sentAt := time.Date(2026, 10, 8, 16, 44, 45, 0, time.UTC) + g := &inventory.PlanGate{Machines: []string{"laptop"}, Verdict: inventory.GatePassed, Why: "healthy 3 times over 2m5s"} + state := &inventory.PlanModule{Build: "build-1", First: []string{"laptop"}, FirstAt: &sentAt, Gate: g} + p := &inventory.Plan{ID: "plan-1", Modules: map[string]*inventory.PlanModule{"dunst": state}} + // No stores: a walk that keeps the pass touches none, and one that reaches for them is putting it back. + defer func() { + if r := recover(); r != nil { + t.Fatalf("the passed build was taken to be failed and put back: %v", r) + } + }() + failFirstSend(t.Context(), nil, p, "dunst", state, []string{"laptop"}, "laptop did not report it applied within 30m0s", + []string{"workstation"}) + if g.Verdict != inventory.GatePassed || g.Rollback != "" { + t.Fatalf("the passed gate became %q, rollback %q", g.Verdict, g.Rollback) + } + if strings.Contains(p.Note, "failed its gate") || strings.Contains(p.Note, "put back") || + !strings.Contains(p.Note, "did not report it applied") { + t.Fatalf("the note reads %q", p.Note) + } +} + +// novox/hq issue 335 review: **a module carried by its lead's send takes over the lead's pass before its own +// step is read.** The state is the one a step leaves when the lead's gate passed and the step ended before the +// carried module's turn (an error read before it, kept with the plan): the lead passed, the carried module has +// no gate of its own yet. Since then another send reached the first machine and it has not reported on it, and +// the first send is older than the wait for a first machine's report. Read before the pass is taken over, the +// carried module's step said the first machine never applied it, and the passed build was put back. +func TestACarriedModuleTakesItsLeadsPassBeforeItsStepIsRead(t *testing.T) { + tm := aTierMesh(t, "m01", "m02") + ctx := t.Context() + inv := tm.open.inventory + + advancePlans(ctx, tm.open) + if !reflect.DeepEqual(tm.sent, [][]string{{"anchor"}}) { + t.Fatalf("sent %v: the first machine once, for the tier", tm.sent) + } + p := tm.plan(t) + lead, carried := p.Modules["m01"], p.Modules["m02"] + if lead.Gate == nil || carried.GatedBy != "m01" { + t.Fatalf("m02 is not carried by m01's send: lead %+v, carried %+v", lead.Gate, carried) + } + // The lead passed; the carried module's turn did not come. The first send is past the wait's bound. + sent := time.Now().UTC().Add(-planWaitBound - time.Minute) + judged := sent.Add(2 * time.Minute) + lead.FirstAt, carried.FirstAt, lead.Gate.Since = &sent, &sent, &sent + lead.Gate.Verdict, lead.Gate.Why, lead.Gate.JudgedAt, lead.Gate.Kept = inventory.GatePassed, + "healthy 3 times over 2m5s", &judged, true + carried.Gate = nil + if err := inv.SavePlan(ctx, &p); err != nil { + t.Fatal(err) + } + // Another walk's send reached anchor, which has not reported on it. + if err := inv.RecordSent(ctx, nodeID(t, tm.open, "anchor"), "d-anchor-elsewhere", + map[string]string{"m01": "c2", "m02": "c2"}); err != nil { + t.Fatal(err) + } + + advancePlans(ctx, tm.open) + p = tm.plan(t) + if p.State == inventory.PlanFailed { + t.Fatalf("the plan failed after its gate passed: %s", p.Note) + } + if !reflect.DeepEqual(tm.sent, [][]string{{"anchor"}, {"laptop"}}) { + t.Fatalf("sent %v: the rest once, as the pass said", tm.sent) + } + current, err := inv.CurrentBuilds(ctx) + if err != nil { + t.Fatal(err) + } + for _, m := range []string{"m01", "m02"} { + if current[m].Commit != "c2" { + t.Errorf("%s was put back to %s after its gate passed", m, current[m].Commit) + } + if failed, _ := inv.GateFailed(ctx, "build-"+m+"-2"); failed { + t.Errorf("%s's build was marked failed at its gate after the gate passed", m) + } + } + if g := p.Modules["m02"].Gate; g == nil || g.Verdict != inventory.GatePassed { + t.Errorf("m02 did not take over m01's pass: %+v", g) + } +}