fix(appproto): a plan on the wire is read as one plan (#1841)

* fix(appproto): a plan on the wire is read as one plan

writePlan read the plan, its rev and the chargers' windows in three
calls. cmd/ftw's Rev reads the planner again, and PlanChargers takes its
own snapshot, so a replan between them mixed two plans in one message:
the new plan's windows under the old plan's rev (run 40115146270,
TestAPlugInReachesAStreamingPhoneAsAPlan: "pushed rev 1 after 1"), or
the old slots under the new rev. The box now reads the plan again when
it moved during the read, at most twice more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(appproto): name the shadow swap in readPlan; test reports empty slots

Review: the planner also swaps in a copy of the plan that carries its
shadow result, so a read can move twice. The test now reports a plan
without slots instead of panicking on it, and the changeset no longer
says always.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: frahlg

.changeset/plan-push-one-plan.md

@@ -0,0 +1,5 @@
+---
+"ftw": patch
+---
+
+A plan sent to the app no longer mixes a new plan with the one before it. A replan that finished while the box read the plan could send the new plan's charging windows under the old plan's number, or the old slots under the new number.

go/internal/appproto/handler.go

@@ -1316,20 +1316,38 @@ func (h *Handler) onPlanGet(id *uint32) error {
 
 func (h *Handler) sendPlan(id *uint32) error { return h.writePlan(id, false) }
 
+// readPlan reads the plan, its rev and each charger's share as one plan. The
+// three are separate reads, and a replan that lands between them would mix
+// two plans in one message, under either one's rev. A plan that moved while
+// it was read is read again. A replan moves it once, and once more when the
+// planner swaps in the copy that carries its shadow result, so the third
+// read is sent as it is.
+func (h *Handler) readPlan() Plan {
+	for attempt := 1; ; attempt++ {
+		plan := h.cfg.Plans.Latest()
+		body := planFrom(
+			plan,
+			h.cfg.Plans.Rev(),
+			h.cfg.Plans.CeilingW(),
+			h.cfg.Clock.UptimeMs(),
+			h.cfg.Clock.Now(),
+		)
+		if h.cfg.PlanChargers != nil {
+			body.Chargers = h.cfg.PlanChargers.PlanChargers(h.cfg.Clock.Now())
+		}
+		if attempt == 3 || h.cfg.Plans.Latest() == plan {
+			return body
+		}
+	}
+}
+
 // writePlan reads the plan and sends it. With skipSame it sends nothing when
 // this session already has this plan.
 func (h *Handler) writePlan(id *uint32, skipSame bool) error {
 	h.planMu.Lock()
 	defer h.planMu.Unlock()
-	body := planFrom(
-		h.cfg.Plans.Latest(),
-		h.cfg.Plans.Rev(),
-		h.cfg.Plans.CeilingW(),
-		h.cfg.Clock.UptimeMs(),
-		h.cfg.Clock.Now(),
-	)
+	body := h.readPlan()
 	if h.cfg.PlanChargers != nil {
-		body.Chargers = h.cfg.PlanChargers.PlanChargers(h.cfg.Clock.Now())
 		if h.isAgent() {
 			// An agent knows chargers and cars by the ids loadpoints.get
 			// and cars.get gave it, never by the box's names. A copy: the

go/internal/appproto/plan_feed_test.go

@@ -337,3 +337,60 @@ func TestThePlanWithChargersVector(t *testing.T) {
 		t.Fatalf("plan bytes moved:\n got %s\nwant %s", got, planChargersVector)
 	}
 }
+
+// replanningChargers publishes a new plan on the box just before its first
+// read, as a replan can land while the box reads a plan for the wire.
+type replanningChargers struct {
+	box   *fakeBox
+	reads int
+}
+
+func (c *replanningChargers) PlanChargers(time.Time) []PlanCharger {
+	c.reads++
+	if c.reads == 1 {
+		c.box.replan(2, c.box.plan.GeneratedAtMs+60_000)
+	}
+	if c.box.planRev == 2 {
+		return []PlanCharger{{ID: "easee", PluggedIn: true, Windows: []PlanChargerWindow{{StartMs: 1, EndMs: 2, Wh: 7400}}, TotalWh: 7400}}
+	}
+	return []PlanCharger{{ID: "easee", Windows: []PlanChargerWindow{}}}
+}
+
+// A plan on the wire is one plan. Its rev, slots and chargers once came from
+// separate reads, and a replan between them sent the new plan's windows
+// under the old plan's rev (cmd/ftw TestAPlugInReachesAStreamingPhoneAsAPlan,
+// run 40115146270).
+func TestAReplanDuringTheReadSendsOnePlan(t *testing.T) {
+	clock := &fakeClock{uptimeMs: 60_000, now: time.UnixMilli(1_760_000_000_000)}
+	box := &fakeBox{identity: Identity{ID: "ftw-0001", Build: "test", TZ: "Europe/Stockholm"}}
+	box.replan(1, clock.now.UnixMilli())
+	chargers := &replanningChargers{box: box}
+	rec := &recorder{}
+	h, err := New(Config{
+		Clock: clock, Site: box, Info: box, Modes: box, Plans: box,
+		PlanChargers: chargers, Codec: testCodec{}, Sender: rec, Caller: ownerCaller(),
+		Grants: newGrants(), Commands: NewCmdLog(),
+	})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	t.Cleanup(h.Close)
+
+	if err := h.sendPlan(nil); err != nil {
+		t.Fatal(err)
+	}
+	got := plans(rec)
+	if len(got) != 1 {
+		t.Fatalf("want one plan, got %d (%s)", len(got), rec.types())
+	}
+	p := body[Plan](t, got[0])
+	if len(p.Slots) == 0 {
+		t.Fatalf("plan rev %d has no slots", p.Rev)
+	}
+	if p.Rev != 2 || p.Slots[0].StartMs != box.plan.GeneratedAtMs {
+		t.Fatalf("plan rev %d with slots from %d, want rev 2 from %d", p.Rev, p.Slots[0].StartMs, box.plan.GeneratedAtMs)
+	}
+	if len(p.Chargers) != 1 || len(p.Chargers[0].Windows) != 1 {
+		t.Fatalf("chargers %+v, want the new plan's window", p.Chargers)
+	}
+}