tui3: make the switch and plan test fakes safe for off-loop readers

docs/changes/unreleased/1836-held-send-race-fakes.md

@@ -0,0 +1,16 @@
+---
+kind: internal
+title: the held-send and plan tests pass under -race; their fakes now lock what off-loop readers touch
+pr: 1836
+invalidates:
+  - >-
+    The tui3 held-send and run-stop-card tests were believed to fail under -race
+    because the keeper goroutine or the room touched state the update loop owns;
+    the races were in the switchAgent and planFake test fakes, which now lock
+    what the off-loop readers touch, and production code is unchanged.
+---
+
+The real agents already lock Attach, and the real plan reads build fresh values
+on every call. Three more tests with the same fake races, also red under -race
+on dev, are fixed by the same change: TestAProgramRoomReadsTheLandingTheStoreEndedAhead,
+TestTheCancelKeyOnAPlanRowEndsItThroughTheStore and TestPOnAPlanRowPausesThenResumes.

internal/tui3/c295_rail_task_page_test.go

@@ -3,6 +3,7 @@ package tui3
 import (
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 
 	tea "charm.land/bubbletea/v2"
@@ -11,19 +12,20 @@ import (
 )
 
 // railPlanCounter proves that opening the store is a gesture, never paint.
+// The counts are bumped by reads the surface makes off the loop.
 type railPlanCounter struct {
 	*planFake
-	rows  int
-	pages int
+	rows  atomic.Int64
+	pages atomic.Int64
 }
 
 func (f *railPlanCounter) PlanTasks() []session.PlanTaskRow {
-	f.rows++
+	f.rows.Add(1)
 	return f.planFake.PlanTasks()
 }
 
 func (f *railPlanCounter) PlanTaskPage(id string) (session.PlanTaskPage, bool) {
-	f.pages++
+	f.pages.Add(1)
 	return f.planFake.PlanTaskPage(id)
 }
 
@@ -55,12 +57,12 @@ func railTaskPageApp(t *testing.T, held bool) (*app, *railPlanCounter) {
 
 func TestRailClickReadsTheStoreAtTheGestureAndOpensTheTaskPage(t *testing.T) {
 	a, fake := railTaskPageApp(t, true)
-	if fake.pages != 0 {
-		t.Fatalf("building and drawing the conversation read %d task pages", fake.pages)
+	if fake.pages.Load() != 0 {
+		t.Fatalf("building and drawing the conversation read %d task pages", fake.pages.Load())
 	}
 	clickRail(t, a, 0)
-	if fake.pages != 1 {
-		t.Fatalf("rail click read task pages %d times, want once at the gesture", fake.pages)
+	if fake.pages.Load() != 1 {
+		t.Fatalf("rail click read task pages %d times, want once at the gesture", fake.pages.Load())
 	}
 	if plan := a.roomPlan(); plan == nil || plan.id != "2" {
 		t.Fatalf("rail click did not open the task room over store task 2: room=%v", a.roomOpen())
@@ -74,16 +76,16 @@ func TestRailEnterReadsTheStoreAtTheGestureAndOpensTheTaskPage(t *testing.T) {
 	a, fake := railTaskPageApp(t, true)
 	a.railWhere, a.railHold = railSpot{id: 2}, true
 	drive(t, a, tea.KeyPressMsg{Code: tea.KeyEnter})
-	if plan := a.roomPlan(); fake.pages != 1 || plan == nil || plan.id != "2" {
-		t.Fatalf("rail enter reads=%d room=%v", fake.pages, a.roomOpen())
+	if plan := a.roomPlan(); fake.pages.Load() != 1 || plan == nil || plan.id != "2" {
+		t.Fatalf("rail enter reads=%d room=%v", fake.pages.Load(), a.roomOpen())
 	}
 }
 
 func TestRailRowWithoutAStoredPageStillOpensItsRoom(t *testing.T) {
 	a, fake := railTaskPageApp(t, false)
 	clickRail(t, a, 0)
-	if fake.pages != 1 {
-		t.Fatalf("room fallback did %d page reads, want one gesture read", fake.pages)
+	if fake.pages.Load() != 1 {
+		t.Fatalf("room fallback did %d page reads, want one gesture read", fake.pages.Load())
 	}
 	if a.roomPlan() != nil || !a.roomOpen() || a.room.id != 2 {
 		t.Fatalf("absent page opened room=%v id=%d", a.roomOpen(), roomID(a))
@@ -93,16 +95,18 @@ func TestRailRowWithoutAStoredPageStillOpensItsRoom(t *testing.T) {
 func TestRailTaskPageSurvivesSettledFramesAndFramesDoNotReadTheAgent(t *testing.T) {
 	a, fake := railTaskPageApp(t, true)
 	clickRail(t, a, 0)
-	for i := range fake.plan {
-		fake.plan[i].Status = "done"
-	}
+	fake.editRows(func(rows []session.PlanTaskRow) {
+		for i := range rows {
+			rows[i].Status = "done"
+		}
+	})
 	for i := range a.taskSheet.mine.plan {
 		a.taskSheet.mine.plan[i].Status = "done"
 	}
 	settled := fake.planFake.pages["2"]
 	settled.Row.Status = "done"
-	fake.planFake.pages["2"] = settled
-	reads, pages := fake.rows, fake.pages
+	fake.planFake.setPage("2", settled)
+	reads, pages := fake.rows.Load(), fake.pages.Load()
 	for range 10 {
 		drive(t, a, frameMsg{})
 		frame, _, _ := a.frame()
@@ -112,8 +116,8 @@ func TestRailTaskPageSurvivesSettledFramesAndFramesDoNotReadTheAgent(t *testing.
 	}
 	// A ROOM ON A RUNNING TASK FOLLOWS IT ON ITS OWN BEAT, never on the paint
 	// clock: ten frames cost no read of the page and no read of the rows.
-	if fake.rows != reads || fake.pages != pages {
-		t.Fatalf("frames called the agent: row reads %d→%d, page reads %d→%d", reads, fake.rows, pages, fake.pages)
+	if fake.rows.Load() != reads || fake.pages.Load() != pages {
+		t.Fatalf("frames called the agent: row reads %d→%d, page reads %d→%d", reads, fake.rows.Load(), pages, fake.pages.Load())
 	}
 	if a.roomPlan() == nil {
 		t.Fatal("ten settled frames closed the task room")
@@ -140,8 +144,8 @@ func TestAChildRemainsOpenableFromARailTaskRoom(t *testing.T) {
 		t.Fatalf("the run's room does not draw its part:\n%s", got)
 	}
 	openPlanRoomNow(t, a, "3")
-	if plan := a.roomPlan(); fake.pages != 2 || plan == nil || plan.id != "3" {
-		t.Fatalf("child open reads=%d room=%v", fake.pages, a.roomOpen())
+	if plan := a.roomPlan(); fake.pages.Load() != 2 || plan == nil || plan.id != "3" {
+		t.Fatalf("child open reads=%d room=%v", fake.pages.Load(), a.roomOpen())
 	}
 	if got := planRoomText(t, a); !strings.Contains(got, "the child page") {
 		t.Fatalf("the child's room was not drawn:\n%s", got)

internal/tui3/parkkeep_test.go

@@ -58,7 +58,7 @@ func parkedKeeperLab(t *testing.T, parks []parked) (*app, *switchAgent, *fakeAge
 // Attach is the authority behind follow-up priority.
 func landHeld(t *testing.T, a *app, agent *switchAgent, key string, held *kept, running bool) {
 	t.Helper()
-	agent.running = running
+	agent.setRunning(running)
 	held.watch.landed.Store(true)
 	held.watch.armed.Store(true)
 	drive(t, a, runCmd(a.behindStir(behindStirMsg{key: key}))...)
@@ -185,7 +185,7 @@ func TestHeldWaitingMessagesGoOnePerLandingAfterSessionFollowUpsAndNeverTwice(t
 
 func TestComingBackDuringAHeldSendNeverSendsItTwice(t *testing.T) {
 	a, agent, _, key, held := parkedKeeperLab(t, []parked{{text: "only once"}})
-	agent.running = false
+	agent.setRunning(false)
 	cmd := a.sendBehindParked(key, held)
 	if cmd == nil || !held.side.parkSending || !held.side.parks[0].sending {
 		t.Fatal("the held send did not mark its queue head before crossing")

internal/tui3/planroom_test.go

@@ -351,7 +351,7 @@ func TestTheRunsTaskRoomFollowsAStepTheStoreRecordsLater(t *testing.T) {
 		t.Fatalf("the room opened with calls %q", got)
 	}
 	page.Steps = append(page.Steps, session.PlanStep{Kind: "step", Step: 2, Command: "go test ./..."})
-	fake.pages[row.ID] = page
+	fake.setPage(row.ID, page)
 	drive(t, a, planRoomTickMsg{gen: a.room.gen})
 	if got := roomCommands(a); len(got) != 2 || got[1] != "go test ./..." {
 		t.Fatalf("the beat did not bring the new step in once: %q", got)
@@ -372,7 +372,7 @@ func TestTheLiveStepLeavesTheRoomWhenTheTaskEnds(t *testing.T) {
 	row.Status, row.Live = "done", plandb.LiveStep{}
 	page.Row, page.Live = row, row.Live
 	page.Steps = []session.PlanStep{{Kind: "step", Step: 2, Command: "go test ./...", Observation: "ok"}}
-	fake.pages[row.ID] = page
+	fake.setPage(row.ID, page)
 	drive(t, a, planRoomTickMsg{gen: a.room.gen})
 	if !a.room.done {
 		t.Fatal("the room of a task that ended is not done")

internal/tui3/planrows_offframe_test.go

@@ -96,7 +96,7 @@ func TestWhileTheRowsAreOnTheirWayTheFrameDrawsWhatItHeld(t *testing.T) {
 	a.agent = slow
 	a.taskSheet.regroup(a)
 
-	fake.plan = append(fake.plan, session.PlanTaskRow{ID: "p1", Parent: "1", Title: "a part the run added", Status: "running"})
+	fake.setPlan(append(fake.plan, session.PlanTaskRow{ID: "p1", Parent: "1", Title: "a part the run added", Status: "running"}))
 	a.railStamp++
 	cmd := a.refreshPlanRows()
 	if cmd == nil {

internal/tui3/programbadge_test.go

@@ -188,14 +188,14 @@ func TestDrawingAProgramsBadgeReadsNothingFromTheAgent(t *testing.T) {
 	// AN OLDER ENGINE'S ROW, naming no program: the badge is read off the held
 	// plan row.
 	drive(t, a, streamEventMsg{gen: a.gen, ev: update(7, row.Title, session.TaskRunning, session.TaskNotice{StartedAt: programRunBegan})})
-	rows, pages := counted.rows, counted.pages
+	rows, pages := counted.rows.Load(), counted.pages.Load()
 	if drawn := plain(strings.Join(a.railRows(a.viewHeight()), "\n")); !strings.Contains(drawn, "[sd]") {
 		t.Fatalf("a node named only by its held plan row wears no badge:\n%s", drawn)
 	}
 	a.stripRow(90)
 	a.taskSheetOwnRows()
-	if counted.rows != rows || counted.pages != pages {
-		t.Fatalf("drawing the badge read the agent: rows %d→%d, pages %d→%d", rows, counted.rows, pages, counted.pages)
+	if counted.rows.Load() != rows || counted.pages.Load() != pages {
+		t.Fatalf("drawing the badge read the agent: rows %d→%d, pages %d→%d", rows, counted.rows.Load(), pages, counted.pages.Load())
 	}
 }
 

internal/tui3/programroom_test.go

@@ -151,7 +151,7 @@ func TestAProgramRoomNamesItsTaskOnceAndHidesItsBriefBehindADropdown(t *testing.
 	a, agent := programRoomApp(t, 120, 40)
 	page := agent.planFake.pages["7"]
 	page.Description = strings.Repeat("the store interface changes and every caller moves with it. ", 12) + "THE LAST WORDS"
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	openProgramRoomNow(t, a)
 	width, _ := a.size()
 	head := func() []string {
@@ -198,7 +198,7 @@ func TestAProgramsOpenBriefIsAWholeDocumentTheRoomScrolls(t *testing.T) {
 		items = append(items, "- item "+strconv.Itoa(i)+" keeps __init__.py as it is")
 	}
 	page.Description = "THE WORK\n\nFIRST ACTION: read the spec.\n" + strings.Join(items, "\n") + "\n\nWHAT TO PRODUCE\n\nTHE LAST WORDS"
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	openProgramRoomNow(t, a)
 	a.roomScroll(-3)
 	before := a.room.offset
@@ -266,7 +266,7 @@ func TestAProgramsStepOpensToItsWholeStepAndShutsAgain(t *testing.T) {
 	program.Actions = append(program.Actions, delegate.Shown{At: at, Step: "explore", Text: "ran go test ./...",
 		Outcome: "fails · exit 1", Detail: "bash: go test ./...\n\n--- FAIL: TestTheWholeOutput"})
 	page.Program = &program
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	openProgramRoomNow(t, a)
 	var target row
 	for _, r := range a.roomRows(a.bodyWidth()) {
@@ -300,30 +300,30 @@ func TestAProgramRoomFollowsWhileRunningAndStopsAfterItSettles(t *testing.T) {
 	if !a.programRoomFollows() {
 		t.Fatal("a room on a running program is not on the paint clock")
 	}
-	reads := agent.railPlanCounter.pages
+	reads := agent.railPlanCounter.pages.Load()
 	for range 5 {
 		drive(t, a, frameMsg{})
 	}
-	if agent.railPlanCounter.pages != reads {
-		t.Fatalf("frames inside one beat read the page %d times", agent.railPlanCounter.pages-reads)
+	if agent.railPlanCounter.pages.Load() != reads {
+		t.Fatalf("frames inside one beat read the page %d times", agent.railPlanCounter.pages.Load()-reads)
 	}
 	for beat := 1; beat <= 3; beat++ {
 		planBeat(t, a)
-		if got := agent.railPlanCounter.pages - reads; got != beat {
+		if got := agent.railPlanCounter.pages.Load() - reads; got != int64(beat) {
 			t.Fatalf("after %d beats the page was read %d times, want once a beat", beat, got)
 		}
 	}
 	// THE RUN LANDS: the store ends its root and the conversation's row settles.
 	page := agent.planFake.pages["7"]
 	page.Row.Status, page.Row.Ended = "done", a.now()
 	page.Notes = []session.PlanTaskNote{{Body: "the work landed on branch senior-dev/auth"}}
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	drive(t, a, streamEventMsg{gen: a.gen, ev: update(7, page.Row.Title, session.TaskDone,
 		session.TaskNotice{StartedAt: programRunBegan, EndedAt: a.now()})})
-	before := agent.railPlanCounter.pages
+	before := agent.railPlanCounter.pages.Load()
 	drive(t, a, frameMsg{})
-	if agent.railPlanCounter.pages != before+1 {
-		t.Fatalf("the landing was read %d times, want once", agent.railPlanCounter.pages-before)
+	if agent.railPlanCounter.pages.Load() != before+1 {
+		t.Fatalf("the landing was read %d times, want once", agent.railPlanCounter.pages.Load()-before)
 	}
 	if !strings.Contains(roomText(a), "the work landed on branch senior-dev/auth") {
 		t.Fatalf("the room did not end on the page the store ended on:\n%s", roomText(a))
@@ -334,12 +334,12 @@ func TestAProgramRoomFollowsWhileRunningAndStopsAfterItSettles(t *testing.T) {
 	if a.programRoomFollows() {
 		t.Fatal("a settled program's room keeps the paint clock turning")
 	}
-	settled := agent.railPlanCounter.pages
+	settled := agent.railPlanCounter.pages.Load()
 	for range 3 {
 		planBeat(t, a)
 	}
-	if agent.railPlanCounter.pages != settled {
-		t.Fatalf("a settled program's room was read %d more times", agent.railPlanCounter.pages-settled)
+	if agent.railPlanCounter.pages.Load() != settled {
+		t.Fatalf("a settled program's room was read %d more times", agent.railPlanCounter.pages.Load()-settled)
 	}
 }
 
@@ -363,7 +363,7 @@ func TestAProgramRoomReadsTheLandingTheStoreEndedAhead(t *testing.T) {
 			openProgramRoomNow(t, a)
 			page := agent.planFake.pages["7"]
 			page.Row.Status, page.Row.Ended = "done", a.now()
-			agent.planFake.pages["7"] = page
+			agent.planFake.setPage("7", page)
 			planBeat(t, a)
 			if a.room.done || !a.programRoomFollows() {
 				t.Fatalf("a store that ended ahead of the row took the room off the clock: done=%v", a.room.done)
@@ -385,10 +385,10 @@ func TestAProgramRoomReadsTheLandingTheStoreEndedAhead(t *testing.T) {
 				// The read that was out answers with the page before the landing.
 				stale := page
 				stale.Notes = nil
-				agent.planFake.pages["7"] = stale
+				agent.planFake.setPage("7", stale)
 				drain(t, a, out)
 			}
-			agent.planFake.pages["7"] = page
+			agent.planFake.setPage("7", page)
 			for range 2 {
 				drive(t, a, frameMsg{})
 			}
@@ -485,7 +485,7 @@ func TestAProgramRunReadsOneFigureOnTheRoomTheRailAndTheCard(t *testing.T) {
 	// The store was seeded sixteen seconds before the run's hand-off.
 	page := agent.planFake.pages["7"]
 	page.Row.Started = programRunBegan.Add(-16 * time.Second)
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	rail := strings.Split(plain(a.railTelemetry(a.tasks[7], 40)), railSep)[0]
 	openProgramRoomNow(t, a)
 	facts, _ := a.programFactsWord(120)
@@ -498,7 +498,7 @@ func TestAProgramRunReadsOneFigureOnTheRoomTheRailAndTheCard(t *testing.T) {
 	now := ended.Add(3 * time.Second)
 	a.clock = func() time.Time { return now }
 	page.Row.Status, page.Row.Ended = "done", ended.Add(2*time.Second)
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	drive(t, a, streamEventMsg{gen: a.gen, ev: update(7, page.Row.Title, session.TaskDone,
 		session.TaskNotice{StartedAt: programRunBegan, EndedAt: ended, Elapsed: ended.Sub(programRunBegan)})})
 	drive(t, a, frameMsg{})
@@ -530,7 +530,7 @@ func TestAProgramRoomsClockStopsAtTheProgramsExit(t *testing.T) {
 	exit := programRunBegan.Add(20 * time.Minute)
 	page := agent.planFake.pages["7"]
 	page.Row.Ended = exit
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	now := exit.Add(65 * time.Second)
 	a.clock = func() time.Time { return now }
 	drain(t, a, a.programRoomRead())
@@ -590,7 +590,7 @@ func TestAStepThatChangedAFileWearsItsLinesInTheDiffsColours(t *testing.T) {
 		delegate.Shown{At: at, Step: "implement", Text: "edited internal/auth/middleware.go", Lines: true, Added: 123, Removed: 21},
 		delegate.Shown{At: at.Add(time.Second), Step: "implement", Text: "read internal/auth/store.go"})
 	page.Program = &program
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	openProgramRoomNow(t, a)
 	var edited, read string
 	for _, r := range a.roomRows(a.bodyWidth()) {
@@ -640,7 +640,7 @@ func TestAProgramRoomsSeamNamesTheModelsItsRunWasLaunchedOn(t *testing.T) {
 	program.Models = []string{"deepseek/deepseek-v4-pro", "moonshotai/kimi-k2.6", "z-ai/glm-5.1"}
 	program.Effort = "high"
 	page.Program = &program
-	agent.planFake.pages["7"] = page
+	agent.planFake.setPage("7", page)
 	a.closeRoom()
 	openProgramRoomNow(t, a)
 	frame, _, _ := a.frame()
@@ -718,7 +718,7 @@ func TestProgramRoomStartingRefusalKeepsThePersonsWords(t *testing.T) {
 			a, agent := programRoomApp(t, 120, 28)
 			page := agent.planFake.pages["7"]
 			page.Program.Listens = true
-			agent.planFake.pages["7"] = page
+			agent.planFake.setPage("7", page)
 			openProgramRoomNow(t, a)
 			// The held node is what owns the room's ending, so queued remains live.
 			a.programOf().page.Row.Status = state

internal/tui3/stoprun_page_test.go

@@ -68,7 +68,7 @@ func TestStopOnAPartsRoomIsTheCardAndThenTheStoresOwnCancel(t *testing.T) {
 	counted.setStatus("3", "running")
 	part := counted.planFake.pages["3"]
 	part.Row.Status = "running"
-	counted.planFake.pages["3"] = part
+	counted.planFake.setPage("3", part)
 	openPlanRoomNow(t, a, "3")
 	if plan := a.roomPlan(); plan == nil || plan.id != "3" {
 		t.Fatal("the part's room did not open")