[frahlg]

A real bug that a flaky cmd/ftw test showed. Run 40115146270 (PR #1822), race job:

--- FAIL: TestAPlugInReachesAStreamingPhoneAsAPlan (0.53s)
    app_link_plan_push_test.go:123: pushed rev 1 after 1, car {ID:easee ... PluggedIn:true Pending:false ... Windows:[{StartMs:1792875600000 EndMs:1792882800000 Wh:20000}] TotalWh:20000 ...}

Cause

writePlan builds a plan message from three reads: Plans.Latest() for the slots, Plans.Rev() for the revision, and PlanChargers.PlanChargers() for each charger's windows. In the box, appPlans.Rev() reads the planner's latest plan again, and appPlanChargers takes its own PlanSnapshot(). When a replan publishes between those reads, one message carries two plans:

• the new plan's charger windows under the old plan's rev and slots (the CI failure: the plug-in plan's windows arrived as rev 1); or
• the old plan's slots under the new rev. The next push then repeats that rev with the right slots, and a phone that treats a repeated rev as a redelivery could keep the wrong slots until the next replan.

Fix

readPlan reads the three, then checks that the planner's latest plan is still the one it started from. If it moved, it reads again, at most twice more. A replan moves the pointer once, and once more when the planner swaps in the copy that carries its shadow result, so three reads cover it. Every write to the planner's latest plan stores a fresh allocation, so when the first and last reads see the same pointer, every read in between saw it too. Patch changeset.

Evidence

TestAReplanDuringTheReadSendsOnePlan publishes a new plan on the fake box during the read, as the planner can. On master it fails with plan rev 1 with slots from 1760000000000, want rev 2 from 1760000060000, with the new windows already in the chargers. On this branch it passes.

• go test -race ./internal/appproto/ passes.
• go test -race -count=3 -run 'Plan|AppLink' ./cmd/ftw/ passes, including the test that failed.
• go vet ./... passes.
• The CI test did not fail in 90 local runs on master. The window is a few microseconds and needs CI's load.

Review

An independent review (Sonnet) found no bug. It checked that every write to s.last is a fresh allocation, so the pointer check holds, and that lock order, skip-same, agent ids and truncation are unchanged. Its notes are in the second commit: the shadow swap named in the comment, the test reports a plan without slots instead of panicking, and the changeset no longer says "always".

It also found a separate, older issue, not changed here. appPlans.Rev (cmd/ftw/app_link.go) calls Latest() before it takes revMu. With two sessions on the same appPlans, a slow reader can move lastGen back and bump the rev twice for one replan. The cost is a jump in rev and one extra push, not a mixed plan.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-25T11:56:17.337791Z	87310da	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.