Test that a carried module takes its lead's pass before its step is read (hq issue 335 review)

Without the reorder the passed build's walk stopped on the first machine's
later silence; the guard that caught it is now said to be one.

cmd/mesh-controller/release_plan.go

@@ -977,8 +977,12 @@ func failFirstSend(ctx context.Context, open *stores, p *inventory.Plan, module
 	}()
 	g := state.Gate
 	if g != nil && g.Verdict == inventory.GatePassed {
-		// **A build that passed its gate is never put back for what came after** (novox/hq issue 335): its
-		// verdict stands; the plan stops, and says why, but the build is not marked failed.
+		// **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",

cmd/mesh-controller/replay335_test.go

@@ -69,3 +69,64 @@ func testAWalkStoppedAfterItsGatePassedKeepsThePass(t *testing.T) {
 		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)
+	}
+}