Merge pull request 'Keep a passed gate's verdict when the first machine's later reports go quiet (hq issue 335)' (#165) from fix/335-a-passed-gate-is-not-judged-again into main

This commit was merged in pull request #165.
This commit is contained in:
2026-10-08 19:48:03 +00:00
2 changed files with 157 additions and 1 deletions
+25 -1
View File
@@ -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]
+132
View File
@@ -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)
}
}