fix(daemon): reserve shutdown preservation before cancelling pane processes

Daemon stop (KillDaemon) could purge a named session's durable record
instead of preserving it. closeMoveLifecycles cancelled the daemon-wide
pane process context as its last step, but terminateAllForShutdown only
called reserveShutdownTeardown for each session afterwards. With the PTYs
already closed, a session's reader could win teardown ownership on EOF
before the reservation was published and treat the cancellation as a
natural final-shell exit (purge=true, reason=session-killed), deleting
the record. The restarted daemon then listed that session as missing, or
restored from a partially written state.

Split the cancellation out of closeMoveLifecycles into
cancelPaneProcesses and call it after every live session has reserved
its shutdown teardown, so PTY EOF during shutdown always resolves to
preserve.

The integration tests asserted that a freshly restarted daemon lists
named sessions as SessionDown. Restoration runs in the background and
publishes each session atomically as SessionDown or SessionUp, so either
is a valid snapshot of a healthy session; assert that instead of an
exact state. Add a deterministic unit test for the shutdown ordering.

commit: 291cc757159bab4255dcf76afd3f9f7211d7ec43
author: brice · 19d ago · 2026-10-03 15:49 UTC
parent: 0db7657
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +86 -8

M internal/app/daemon_integration_test.go +20 -6


@@ -412,6 +412,24 @@ func awaitText(t testing.T, p typedPump, sz domain.Size, want string) {
412	412		_ = awaitScreenText(t, p, sz, want)
413	413		}
414	414
	415	+	// requireListedAfterRestart asserts that every wanted named session is listed,
	416	+	// in name order, as a healthy durable session. A restarted daemon restores
	417	+	// named sessions in the background, publishing each one atomically from the
	418	+	// stopped registry (SessionDown) to the live one (SessionUp), so a listing taken
	419	+	// right after startup legitimately observes either state per session. Only
	420	+	// SessionBroken, a missing name, or an unexpected extra name is a failure.
	421	+	func requireListedAfterRestart(t *testing.T, sessions []protocol.SessionInfo, names ...string) {
	422	+	t.Helper()
	423	+	got := make([]string, 0, len(sessions))
	424	+	for _, info := range sessions {
	425	+	got = append(got, info.Name)
	426	+	require.Contains(t, []protocol.SessionState{protocol.SessionDown, protocol.SessionUp}, info.State,
	427	+	"session %q must be stopped or restored, not broken", info.Name)
	428	+	require.False(t, info.Ephemeral, "session %q must be a durable named session", info.Name)
	429	+	}
	430	+	require.Equal(t, names, got)
	431	+	}
	432	+
415	433		// awaitDetached consumes typed messages until the daemon signals the session's
416	434		// end with the expected Detached reason, ignoring interleaved Output frames.
417	435		func awaitDetached(t testing.T, p typedPump, reason uint8) {

@@ -789,11 +807,7 @@ func TestIntegration_KillDaemonPreservesMultipleNamedSessions(t *testing.T) {
789	807		}
790	808
791	809		served = start()
792		-	sessions := listRemoteSessions(t, dir)
793		-	require.Equal(t, []protocol.SessionInfo{
794		-	{Name: "alpha", State: protocol.SessionDown},
795		-	{Name: "beta", State: protocol.SessionDown},
796		-	}, sessions.Sessions)
	810	+	requireListedAfterRestart(t, listRemoteSessions(t, dir).Sessions, "alpha", "beta")
797	811
798	812		// Kill-all purges every durable record but leaves this daemon serving.
799	813		require.NoError(t, killAll(dir))

@@ -943,7 +957,7 @@ func TestIntegration_NamedSessionRestoresPersistedIdentity(t *testing.T) {
943	957		daemon.WithSnapshotRepository(repository),
944	958		daemon.WithRecoveryCoordinator(coordinator),
945	959		)
946		-	require.Equal(t, []protocol.SessionInfo{{Name: name, State: protocol.SessionDown}}, listRemoteSessions(t, dir).Sessions)
	960	+	requireListedAfterRestart(t, listRemoteSessions(t, dir).Sessions, name)
947	961		select {
948	962		case err := <-served:
949	963		t.Fatalf("Serve stopped before the restore: %v", err)

 	_ = awaitScreenText(t, p, sz, want)
 }
+// requireListedAfterRestart asserts that every wanted named session is listed,
+// in name order, as a healthy durable session. A restarted daemon restores
+// named sessions in the background, publishing each one atomically from the
+// stopped registry (SessionDown) to the live one (SessionUp), so a listing taken
+// right after startup legitimately observes either state per session. Only
+// SessionBroken, a missing name, or an unexpected extra name is a failure.
+func requireListedAfterRestart(t *testing.T, sessions []protocol.SessionInfo, names ...string) {
+	t.Helper()
+	got := make([]string, 0, len(sessions))
+	for _, info := range sessions {
+		got = append(got, info.Name)
+		require.Contains(t, []protocol.SessionState{protocol.SessionDown, protocol.SessionUp}, info.State,
+			"session %q must be stopped or restored, not broken", info.Name)
+		require.False(t, info.Ephemeral, "session %q must be a durable named session", info.Name)
+	}
+	require.Equal(t, names, got)
+}
 // awaitDetached consumes typed messages until the daemon signals the session's
 // end with the expected Detached reason, ignoring interleaved Output frames.
 func awaitDetached(t *testing.T, p *typedPump, reason uint8) {
 	}
 	served = start()
-	sessions := listRemoteSessions(t, dir)
+	requireListedAfterRestart(t, listRemoteSessions(t, dir).Sessions, "alpha", "beta")
-	require.Equal(t, []protocol.SessionInfo{
-		{Name: "alpha", State: protocol.SessionDown},
-		{Name: "beta", State: protocol.SessionDown},
-	}, sessions.Sessions)
 	// Kill-all purges every durable record but leaves this daemon serving.
 	require.NoError(t, killAll(dir))
 		daemon.WithSnapshotRepository(repository),
 		daemon.WithRecoveryCoordinator(coordinator),
 	)
-	require.Equal(t, []protocol.SessionInfo{{Name: name, State: protocol.SessionDown}}, listRemoteSessions(t, dir).Sessions)
+	requireListedAfterRestart(t, listRemoteSessions(t, dir).Sessions, name)
 	select {
 	case err := <-served:
 		t.Fatalf("Serve stopped before the restore: %v", err)

M internal/usecase/daemon/daemon.go +3 -0


@@ -941,9 +941,12 @@ func (d Daemon) terminateAllForShutdown(reason uint8, deadline snapshotShutdow
941	941		// Publish preservation policy for the complete registry snapshot before
942	942		// cancelling any PTY. Cancellation-driven EOF is allowed to own teardown,
943	943		// but its disposition must remain daemon shutdown rather than session purge.
	944	+	// closeMoveLifecycles therefore leaves pane processes running; only this
	945	+	// ordering, reserve then cancel, keeps a named session durable.
944	946		for _, s := range snapshot {
945	947		s.reserveShutdownTeardown()
946	948		}
	949	+	d.cancelPaneProcesses()
947	950		d.log.Info("session termination begin", "reason", reason, "live_sessions", len(snapshot))
948	951		for _, s := range snapshot {
949	952		// Cancellation and PTY closure must not wait behind a teardown owner that


@@ -941,9 +941,12 @@ func (d Daemon) terminateAllForShutdown(reason uint8, deadline snapshotShutdow
941	// Publish preservation policy for the complete registry snapshot before	941		// Publish preservation policy for the complete registry snapshot before
942	// cancelling any PTY. Cancellation-driven EOF is allowed to own teardown,	942		// cancelling any PTY. Cancellation-driven EOF is allowed to own teardown,
943	// but its disposition must remain daemon shutdown rather than session purge.	943		// but its disposition must remain daemon shutdown rather than session purge.
		944	+	// closeMoveLifecycles therefore leaves pane processes running; only this
		945	+	// ordering, reserve then cancel, keeps a named session durable.
944	for _, s := range snapshot {	946		for _, s := range snapshot {
945	s.reserveShutdownTeardown()	947		s.reserveShutdownTeardown()
946	}	948		}
		949	+	d.cancelPaneProcesses()
947	d.log.Info("session termination begin", "reason", reason, "live_sessions", len(snapshot))	950		d.log.Info("session termination begin", "reason", reason, "live_sessions", len(snapshot))
948	for _, s := range snapshot {	951		for _, s := range snapshot {
949	// Cancellation and PTY closure must not wait behind a teardown owner that	952		// Cancellation and PTY closure must not wait behind a teardown owner that

M internal/usecase/daemon/move_lifecycle.go +17 -2


@@ -120,8 +120,11 @@ func (r *moveLifecycleReservation) Release() {
120	120		}
121	121
122	122		// closeMoveLifecycles rejects new moves, publishes daemon shutdown to ordinary
123		-	// routing, then drains reservations without holding either daemon mutex. Pane
124		-	// process lifetime is cancelled only after the final reservation releases.
	123	+	// routing, then drains reservations without holding either daemon mutex. It
	124	+	// deliberately does not cancel pane processes: the caller must first publish the
	125	+	// shutdown-preservation policy for every live session, then call
	126	+	// cancelPaneProcesses, so a PTY EOF caused by the cancellation can never be
	127	+	// mistaken for a natural final-shell exit that purges a named session.
125	128		func (d *Daemon) closeMoveLifecycles() {
126	129		if d == nil {
127	130		return

@@ -146,6 +149,18 @@ func (d *Daemon) closeMoveLifecycles() {
146	149		<-changed
147	150		d.moveLifecycleMu.Lock()
148	151		}
	152	+	d.moveLifecycleMu.Unlock()
	153	+	}
	154	+
	155	+	// cancelPaneProcesses cancels the daemon-wide pane process lifetime, closing
	156	+	// every PTY. It must run only after closeMoveLifecycles has drained the final
	157	+	// move reservation and after every live session has reserved its shutdown
	158	+	// teardown (see terminateAllForShutdown).
	159	+	func (d *Daemon) cancelPaneProcesses() {
	160	+	if d == nil {
	161	+	return
	162	+	}
	163	+	d.moveLifecycleMu.Lock()
149	164		cancel := d.paneProcessCancel
150	165		d.moveLifecycleMu.Unlock()
151	166		if cancel != nil {


@@ -120,8 +120,11 @@ func (r *moveLifecycleReservation) Release() {
120		}	120		}
121			121
122		// closeMoveLifecycles rejects new moves, publishes daemon shutdown to ordinary	122		// closeMoveLifecycles rejects new moves, publishes daemon shutdown to ordinary
123	-	// routing, then drains reservations without holding either daemon mutex. Pane	123	+	// routing, then drains reservations without holding either daemon mutex. It
124	-	// process lifetime is cancelled only after the final reservation releases.	124	+	// deliberately does not cancel pane processes: the caller must first publish the
			125	+	// shutdown-preservation policy for every live session, then call
			126	+	// cancelPaneProcesses, so a PTY EOF caused by the cancellation can never be
			127	+	// mistaken for a natural final-shell exit that purges a named session.
125		func (d *Daemon) closeMoveLifecycles() {	128		func (d *Daemon) closeMoveLifecycles() {
126		if d == nil {	129		if d == nil {
127		return	130		return

@@ -146,6 +149,18 @@ func (d *Daemon) closeMoveLifecycles() {
146		<-changed	149		<-changed
147		d.moveLifecycleMu.Lock()	150		d.moveLifecycleMu.Lock()
148		}	151		}
			152	+	d.moveLifecycleMu.Unlock()
			153	+	}
			154	+
			155	+	// cancelPaneProcesses cancels the daemon-wide pane process lifetime, closing
			156	+	// every PTY. It must run only after closeMoveLifecycles has drained the final
			157	+	// move reservation and after every live session has reserved its shutdown
			158	+	// teardown (see terminateAllForShutdown).
			159	+	func (d *Daemon) cancelPaneProcesses() {
			160	+	if d == nil {
			161	+	return
			162	+	}
			163	+	d.moveLifecycleMu.Lock()
149		cancel := d.paneProcessCancel	164		cancel := d.paneProcessCancel
150		d.moveLifecycleMu.Unlock()	165		d.moveLifecycleMu.Unlock()
151		if cancel != nil {	166		if cancel != nil {

M internal/usecase/daemon/move_lifecycle_test.go +46 -0


@@ -272,3 +272,49 @@ func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) {
272	272		require.Nil(t, reservation)
273	273		awaitDaemonMoveActive(t, d, 0)
274	274		}
	275	+
	276	+	// TestShutdownReservesPreservationBeforeCancellingPaneProcesses guards the
	277	+	// daemon-stop ordering that keeps named sessions durable. Cancelling the
	278	+	// daemon-wide pane lifetime closes every PTY; a session whose reader sees that
	279	+	// EOF before its shutdown-preservation policy is reserved takes the natural
	280	+	// final-shell-exit path and purges its durable record instead of preserving it.
	281	+	// The drain step must therefore leave pane processes running; only the full
	282	+	// shutdown cancels them, after reserving preservation for every live session.
	283	+	func TestShutdownReservesPreservationBeforeCancellingPaneProcesses(t *testing.T) {
	284	+	tests := []struct {
	285	+	name string
	286	+	run func(d *Daemon)
	287	+	wantCancelled bool
	288	+	wantReserved bool
	289	+	}{
	290	+	{
	291	+	name: "draining move admission leaves pane processes running",
	292	+	run: func(d *Daemon) { d.closeMoveLifecycles() },
	293	+	},
	294	+	{
	295	+	name: "full shutdown reserves preservation and cancels pane processes",
	296	+	run: func(d *Daemon) { d.shutdownAll(protocol.ReasonServerShutdown) },
	297	+	wantCancelled: true,
	298	+	wantReserved: true,
	299	+	},
	300	+	}
	301	+
	302	+	for _, tt := range tests {
	303	+	t.Run(tt.name, func(t *testing.T) {
	304	+	d := newTestDaemon(t, nil, stubClock{})
	305	+	sess := &session{sessionCore: sessionCore{id: "sess-0", name: "work"}}
	306	+	d.mu.Lock()
	307	+	require.True(t, d.registerSessionLocked(sess))
	308	+	d.mu.Unlock()
	309	+
	310	+	tt.run(d)
	311	+
	312	+	cancelled := d.paneProcessCtx.Err() != nil
	313	+	sess.teardownMu.Lock()
	314	+	reserved := sess.shutdownTeardownRequested
	315	+	sess.teardownMu.Unlock()
	316	+	require.Equal(t, tt.wantCancelled, cancelled, "pane process cancellation")
	317	+	require.Equal(t, tt.wantReserved, reserved, "shutdown preservation reservation")
	318	+	})
	319	+	}
	320	+	}


@@ -272,3 +272,49 @@ func TestDaemonShutdownDrainsMoveBeforePaneLifetimeCancellation(t *testing.T) {
272	require.Nil(t, reservation)	272		require.Nil(t, reservation)
273	awaitDaemonMoveActive(t, d, 0)	273		awaitDaemonMoveActive(t, d, 0)
274	}	274		}
		275	+
		276	+	// TestShutdownReservesPreservationBeforeCancellingPaneProcesses guards the
		277	+	// daemon-stop ordering that keeps named sessions durable. Cancelling the
		278	+	// daemon-wide pane lifetime closes every PTY; a session whose reader sees that
		279	+	// EOF before its shutdown-preservation policy is reserved takes the natural
		280	+	// final-shell-exit path and purges its durable record instead of preserving it.
		281	+	// The drain step must therefore leave pane processes running; only the full
		282	+	// shutdown cancels them, after reserving preservation for every live session.
		283	+	func TestShutdownReservesPreservationBeforeCancellingPaneProcesses(t *testing.T) {
		284	+	tests := []struct {
		285	+	name string
		286	+	run func(d *Daemon)
		287	+	wantCancelled bool
		288	+	wantReserved bool
		289	+	}{
		290	+	{
		291	+	name: "draining move admission leaves pane processes running",
		292	+	run: func(d *Daemon) { d.closeMoveLifecycles() },
		293	+	},
		294	+	{
		295	+	name: "full shutdown reserves preservation and cancels pane processes",
		296	+	run: func(d *Daemon) { d.shutdownAll(protocol.ReasonServerShutdown) },
		297	+	wantCancelled: true,
		298	+	wantReserved: true,
		299	+	},
		300	+	}
		301	+
		302	+	for _, tt := range tests {
		303	+	t.Run(tt.name, func(t *testing.T) {
		304	+	d := newTestDaemon(t, nil, stubClock{})
		305	+	sess := &session{sessionCore: sessionCore{id: "sess-0", name: "work"}}
		306	+	d.mu.Lock()
		307	+	require.True(t, d.registerSessionLocked(sess))
		308	+	d.mu.Unlock()
		309	+
		310	+	tt.run(d)
		311	+
		312	+	cancelled := d.paneProcessCtx.Err() != nil
		313	+	sess.teardownMu.Lock()
		314	+	reserved := sess.shutdownTeardownRequested
		315	+	sess.teardownMu.Unlock()
		316	+	require.Equal(t, tt.wantCancelled, cancelled, "pane process cancellation")
		317	+	require.Equal(t, tt.wantReserved, reserved, "shutdown preservation reservation")
		318	+	})
		319	+	}
		320	+	}