5992754 fix(daemon): address snapshot review findings · vev

M internal/adapters/snapshot/store.go +2 -2


@@ -95,7 +95,7 @@ func (s *Store) Load() ([]ports.SnapshotBlob, error) {
95	95		path := filepath.Join(s.dir, base)
96	96		if strings.HasPrefix(base, ".tmp-") {
97	97		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
98		-	s.log.Warn("remove stale snapshot temp", "path", path, "error", err)
	98	+	s.log.Warn("remove stale snapshot temp", "path", path, "err", err)
99	99		}
100	100		continue
101	101		}

@@ -104,7 +104,7 @@ func (s *Store) Load() ([]ports.SnapshotBlob, error) {
104	104		}
105	105		data, err := readSnapshotFileBounded(path)
106	106		if err != nil {
107		-	s.log.Warn("read snapshot", "path", path, "error", err)
	107	+	s.log.Warn("read snapshot", "path", path, "err", err)
108	108		continue
109	109		}
110	110		name := strings.TrimSuffix(base, ".snap")


@@ -95,7 +95,7 @@ func (s *Store) Load() ([]ports.SnapshotBlob, error) {
95		path := filepath.Join(s.dir, base)	95		path := filepath.Join(s.dir, base)
96		if strings.HasPrefix(base, ".tmp-") {	96		if strings.HasPrefix(base, ".tmp-") {
97		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {	97		if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
98	-	s.log.Warn("remove stale snapshot temp", "path", path, "error", err)	98	+	s.log.Warn("remove stale snapshot temp", "path", path, "err", err)
99		}	99		}
100		continue	100		continue
101		}	101		}

@@ -104,7 +104,7 @@ func (s *Store) Load() ([]ports.SnapshotBlob, error) {
104		}	104		}
105		data, err := readSnapshotFileBounded(path)	105		data, err := readSnapshotFileBounded(path)
106		if err != nil {	106		if err != nil {
107	-	s.log.Warn("read snapshot", "path", path, "error", err)	107	+	s.log.Warn("read snapshot", "path", path, "err", err)
108		continue	108		continue
109		}	109		}
110		name := strings.TrimSuffix(base, ".snap")	110		name := strings.TrimSuffix(base, ".snap")

M internal/usecase/daemon/daemon.go +5 -1


@@ -547,7 +547,11 @@ func (d Daemon) route(h ports.Hello, tr ports.Transport) (session, *attachedCl
547	547		sz = defaultSize
548	548		}
549	549		if d.snapsEnabled && (h.Intent == ports.IntentResume \|\| h.Intent == ports.IntentAttach \|\| h.Intent == ports.IntentNew) {
550		-	<-d.restoreDone
	550	+	select {
	551	+	case <-d.restoreDone:
	552	+	case <-d.serveCtx.Done():
	553	+	return nil, nil, &protoErr{ports.ErrServerShutdown, "daemon is shutting down"}
	554	+	}
551	555		}
552	556
553	557		d.mu.Lock()

 		sz = defaultSize
 	}
 	if d.snapsEnabled && (h.Intent == ports.IntentResume || h.Intent == ports.IntentAttach || h.Intent == ports.IntentNew) {
-		<-d.restoreDone
+		select {
+		case <-d.restoreDone:
+		case <-d.serveCtx.Done():
+			return nil, nil, &protoErr{ports.ErrServerShutdown, "daemon is shutting down"}
+		}
 	}
 	d.mu.Lock()

M internal/usecase/daemon/session.go +6 -2


@@ -59,6 +59,7 @@ type session struct {
59	59		createdAt int64
60	60		mruAt atomic.Uint64
61	61		snapDirty atomic.Bool
	62	+	snapEligible atomic.Bool
62	63		// clipFiles records clipboard-image-transfer temp file paths (see
63	64		// clipboard.go) written for this session, removed best-effort in
64	65		// killSession.

@@ -153,6 +154,7 @@ func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz
153	154		if lastUsedSeq > 0 {
154	155		sess.mruAt.Store(lastUsedSeq)
155	156		}
	157	+	sess.snapEligible.Store(!ephemeral && name != "")
156	158		if !ephemeral {
157	159		if err := d.persist.Save(persist.Record{Name: name, Cwd: cwd, CreatedAt: createdAt, UpdatedAt: createdAt, LastUsedSeq: lastUsedSeq}); err != nil {
158	160		_ = pty.Close()

@@ -433,6 +435,7 @@ func (d Daemon) renameSession(sess session, name string) error {
433	435		delete(d.stopped, name)
434	436		sess.name = name
435	437		sess.ephemeral = false
	438	+	sess.snapEligible.Store(name != "")
436	439		sess.mu.Unlock()
437	440		markSnapshotDirty(sess)
438	441		return nil

@@ -495,14 +498,15 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
495	498		if !purge && !isEphemeral && d.persistEnabled {
496	499		d.refreshSessionCwd(sess)
497	500		}
	501	+	var snapshotDeleteErr error
498	502		if d.snapsEnabled && !isEphemeral {
499	503		if purge {
500	504		sess.mu.Lock()
501	505		name := sess.name
502	506		sess.mu.Unlock()
503	507		if err := d.snaps.Delete(name); err != nil {
	508	+	snapshotDeleteErr = err
504	509		d.log.Warn("deleting session snapshot failed", "err", err, "session", name)
505		-	return err
506	510		}
507	511		} else {
508	512		d.captureSession(sess)

@@ -535,7 +539,7 @@ func (d Daemon) killSession(sess session, reason uint8, purge bool) error {
535	539		d.mu.Unlock()
536	540		d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
537	541
538		-	var purgeErr error
	542	+	purgeErr := snapshotDeleteErr
539	543		if !ephemeral && purge {
540	544		if err := d.persist.Delete(stoppedName); err != nil {
541	545		purgeErr = err

 	createdAt              int64
 	mruAt                  atomic.Uint64
 	snapDirty              atomic.Bool
+	snapEligible           atomic.Bool
 	// clipFiles records clipboard-image-transfer temp file paths (see
 	// clipboard.go) written for this session, removed best-effort in
 	// killSession.
 	if lastUsedSeq > 0 {
 		sess.mruAt.Store(lastUsedSeq)
 	}
+	sess.snapEligible.Store(!ephemeral && name != "")
 	if !ephemeral {
 		if err := d.persist.Save(persist.Record{Name: name, Cwd: cwd, CreatedAt: createdAt, UpdatedAt: createdAt, LastUsedSeq: lastUsedSeq}); err != nil {
 			_ = pty.Close()
 	delete(d.stopped, name)
 	sess.name = name
 	sess.ephemeral = false
+	sess.snapEligible.Store(name != "")
 	sess.mu.Unlock()
 	markSnapshotDirty(sess)
 	return nil
 	if !purge && !isEphemeral && d.persistEnabled {
 		d.refreshSessionCwd(sess)
 	}
+	var snapshotDeleteErr error
 	if d.snapsEnabled && !isEphemeral {
 		if purge {
 			sess.mu.Lock()
 			name := sess.name
 			sess.mu.Unlock()
 			if err := d.snaps.Delete(name); err != nil {
+				snapshotDeleteErr = err
 				d.log.Warn("deleting session snapshot failed", "err", err, "session", name)
-				return err
 			}
 		} else {
 			d.captureSession(sess)
 	d.mu.Unlock()
 	d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
-	var purgeErr error
+	purgeErr := snapshotDeleteErr
 	if !ephemeral && purge {
 		if err := d.persist.Delete(stoppedName); err != nil {
 			purgeErr = err

M internal/usecase/daemon/snapshot.go +18 -8


@@ -3,6 +3,7 @@ package daemon
3	3		import (
4	4		"context"
5	5		"fmt"
	6	+	"math"
6	7		"time"
7	8
8	9		"github.com/bnema/vev/internal/domain"

@@ -48,9 +49,15 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) {
48	49		}
49	50
50	51		func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) error {
	52	+	if err := ctx.Err(); err != nil {
	53	+	return err
	54	+	}
51	55		if snap.Name == "" {
52	56		return fmt.Errorf("snapshot: empty session name")
53	57		}
	58	+	if snap.CreatedAt > math.MaxInt64 {
	59	+	return fmt.Errorf("snapshot: created_at overflows int64")
	60	+	}
54	61		d.mu.Lock()
55	62		if d.closing \|\| d.findByNameLocked(snap.Name) != nil {
56	63		d.mu.Unlock()

@@ -67,6 +74,10 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
67	74		cancel()
68	75		}
69	76		for _, tabSnap := range snap.Tabs {
	77	+	if err := ctx.Err(); err != nil {
	78	+	closeOpened()
	79	+	return err
	80	+	}
70	81		tbSize := domain.Size{Cols: int(tabSnap.Cols), Rows: int(tabSnap.Rows)}
71	82		if !tbSize.Valid() {
72	83		closeOpened()

@@ -88,6 +99,10 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
88	99		tb.ctx, tb.cancel = context.WithCancel(sctx)
89	100		opened = append(opened, tb)
90	101		for _, paneSnap := range tabSnap.Panes {
	102	+	if err := ctx.Err(); err != nil {
	103	+	closeOpened()
	104	+	return err
	105	+	}
91	106		contentRect, ok := placementByPane[paneSnap.ID]
92	107		if !ok {
93	108		closeOpened()

@@ -115,6 +130,7 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err
115	130		if sess.active < 0 \|\| sess.active >= len(sess.tabs) {
116	131		sess.active = 0
117	132		}
	133	+	sess.snapEligible.Store(true)
118	134		if len(snap.Tabs) > 0 && len(snap.Tabs[0].Panes) > 0 {
119	135		sess.cwd = snap.Tabs[0].Panes[0].Cwd
120	136		}

@@ -151,10 +167,7 @@ func markSnapshotDirty(sess *session) {
151	167		if sess == nil {
152	168		return
153	169		}
154		-	sess.mu.Lock()
155		-	named := !sess.ephemeral && sess.name != ""
156		-	sess.mu.Unlock()
157		-	if named {
	170	+	if sess.snapEligible.Load() {
158	171		sess.snapDirty.Store(true)
159	172		}
160	173		}

@@ -292,10 +305,7 @@ func (d *Daemon) snapshotSaver(ctx context.Context) {
292	305		sessions := d.sessionsSnapshotLocked()
293	306		d.mu.Unlock()
294	307		for _, sess := range sessions {
295		-	sess.mu.Lock()
296		-	named := !sess.ephemeral && sess.name != ""
297		-	sess.mu.Unlock()
298		-	if named && sess.snapDirty.Swap(false) {
	308	+	if sess.snapEligible.Load() && sess.snapDirty.Swap(false) {
299	309		if !d.captureSession(sess) {
300	310		sess.snapDirty.Store(true)
301	311		}

 import (
 	"context"
 	"fmt"
+	"math"
 	"time"
 	"github.com/bnema/vev/internal/domain"
 }
 func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) error {
+	if err := ctx.Err(); err != nil {
+		return err
+	}
 	if snap.Name == "" {
 		return fmt.Errorf("snapshot: empty session name")
 	}
+	if snap.CreatedAt > math.MaxInt64 {
+		return fmt.Errorf("snapshot: created_at overflows int64")
+	}
 	d.mu.Lock()
 	if d.closing || d.findByNameLocked(snap.Name) != nil {
 		d.mu.Unlock()
 		cancel()
 	}
 	for _, tabSnap := range snap.Tabs {
+		if err := ctx.Err(); err != nil {
+			closeOpened()
+			return err
+		}
 		tbSize := domain.Size{Cols: int(tabSnap.Cols), Rows: int(tabSnap.Rows)}
 		if !tbSize.Valid() {
 			closeOpened()
 		tb.ctx, tb.cancel = context.WithCancel(sctx)
 		opened = append(opened, tb)
 		for _, paneSnap := range tabSnap.Panes {
+			if err := ctx.Err(); err != nil {
+				closeOpened()
+				return err
+			}
 			contentRect, ok := placementByPane[paneSnap.ID]
 			if !ok {
 				closeOpened()
 	if sess.active < 0 || sess.active >= len(sess.tabs) {
 		sess.active = 0
 	}
+	sess.snapEligible.Store(true)
 	if len(snap.Tabs) > 0 && len(snap.Tabs[0].Panes) > 0 {
 		sess.cwd = snap.Tabs[0].Panes[0].Cwd
 	}
 	if sess == nil {
 		return
 	}
-	sess.mu.Lock()
+	if sess.snapEligible.Load() {
-	named := !sess.ephemeral && sess.name != ""
-	sess.mu.Unlock()
-	if named {
 		sess.snapDirty.Store(true)
 	}
 }
 		sessions := d.sessionsSnapshotLocked()
 		d.mu.Unlock()
 		for _, sess := range sessions {
-			sess.mu.Lock()
+			if sess.snapEligible.Load() && sess.snapDirty.Swap(false) {
-			named := !sess.ephemeral && sess.name != ""
-			sess.mu.Unlock()
-			if named && sess.snapDirty.Swap(false) {
 				if !d.captureSession(sess) {
 					sess.snapDirty.Store(true)
 				}

M internal/usecase/daemon/snapshot_restore_test.go +43 -0


@@ -3,6 +3,7 @@ package daemon
3	3		import (
4	4		"context"
5	5		"io"
	6	+	"math"
6	7		"sync"
7	8		"testing"
8	9		"time"

@@ -142,6 +143,33 @@ func TestRestoreSnapshotsSkipsLiveCorruptAndEmpty(t *testing.T) {
142	143		}
143	144		}
144	145
	146	+	func TestRestoreSnapshotsSkipsCreatedAtOverflow(t *testing.T) {
	147	+	store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "overflow", Data: mustSnapshotBytes(t, snapcodec.Session{
	148	+	Name: "overflow",
	149	+	CreatedAt: uint64(math.MaxInt64) + 1,
	150	+	Tabs: []snapcodec.Tab{{
	151	+	Cols: 80,
	152	+	Rows: 24,
	153	+	Tree: layout.NewTree("pane-1"),
	154	+	Panes: []snapcodec.Pane{{ID: "pane-1", Cwd: "/bad"}},
	155	+	}},
	156	+	})}}}
	157	+	factory := &restorePTYFactory{}
	158	+	d := newTestDaemon(t, factory, stubClock{})
	159	+	WithSnapshotStore(store)(d)
	160	+
	161	+	d.restoreSnapshots(context.Background())
	162	+
	163	+	records, err := d.persist.LoadAll()
	164	+	require.NoError(t, err)
	165	+	require.Empty(t, records)
	166	+	require.Len(t, factory.opens, 0)
	167	+	d.mu.Lock()
	168	+	restored := d.findByNameLocked("overflow")
	169	+	d.mu.Unlock()
	170	+	require.Nil(t, restored)
	171	+	}
	172	+
145	173		func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) {
146	174		releaseLoad := make(chan struct{})
147	175		loaded := make(chan struct{})

@@ -183,6 +211,21 @@ func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) {
183	211		}
184	212		}
185	213
	214	+	func TestNamedRouteRestoreBarrierReturnsShutdown(t *testing.T) {
	215	+	d := newTestDaemon(t, &restorePTYFactory{}, stubClock{})
	216	+	WithSnapshotStore(&restoreSnapshotStore{})(d)
	217	+	d.serveCancel()
	218	+
	219	+	tr, _ := newCapturingTransport(t)
	220	+	sess, ac, err := d.route(ports.Hello{Intent: ports.IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}}, tr)
	221	+
	222	+	require.Nil(t, sess)
	223	+	require.Nil(t, ac)
	224	+	var perr *protoErr
	225	+	require.ErrorAs(t, err, &perr)
	226	+	require.Equal(t, ports.ErrServerShutdown, perr.code)
	227	+	}
	228	+
186	229		func mustSnapshotBytes(t *testing.T, s snapcodec.Session) []byte {
187	230		t.Helper()
188	231		b, err := snapcodec.Marshal(s)


@@ -3,6 +3,7 @@ package daemon
3	import (	3		import (
4	"context"	4		"context"
5	"io"	5		"io"
		6	+	"math"
6	"sync"	7		"sync"
7	"testing"	8		"testing"
8	"time"	9		"time"

@@ -142,6 +143,33 @@ func TestRestoreSnapshotsSkipsLiveCorruptAndEmpty(t *testing.T) {
142	}	143		}
143	}	144		}
144		145
		146	+	func TestRestoreSnapshotsSkipsCreatedAtOverflow(t *testing.T) {
		147	+	store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "overflow", Data: mustSnapshotBytes(t, snapcodec.Session{
		148	+	Name: "overflow",
		149	+	CreatedAt: uint64(math.MaxInt64) + 1,
		150	+	Tabs: []snapcodec.Tab{{
		151	+	Cols: 80,
		152	+	Rows: 24,
		153	+	Tree: layout.NewTree("pane-1"),
		154	+	Panes: []snapcodec.Pane{{ID: "pane-1", Cwd: "/bad"}},
		155	+	}},
		156	+	})}}}
		157	+	factory := &restorePTYFactory{}
		158	+	d := newTestDaemon(t, factory, stubClock{})
		159	+	WithSnapshotStore(store)(d)
		160	+
		161	+	d.restoreSnapshots(context.Background())
		162	+
		163	+	records, err := d.persist.LoadAll()
		164	+	require.NoError(t, err)
		165	+	require.Empty(t, records)
		166	+	require.Len(t, factory.opens, 0)
		167	+	d.mu.Lock()
		168	+	restored := d.findByNameLocked("overflow")
		169	+	d.mu.Unlock()
		170	+	require.Nil(t, restored)
		171	+	}
		172	+
145	func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) {	173		func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) {
146	releaseLoad := make(chan struct{})	174		releaseLoad := make(chan struct{})
147	loaded := make(chan struct{})	175		loaded := make(chan struct{})

@@ -183,6 +211,21 @@ func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) {
183	}	211		}
184	}	212		}
185		213
		214	+	func TestNamedRouteRestoreBarrierReturnsShutdown(t *testing.T) {
		215	+	d := newTestDaemon(t, &restorePTYFactory{}, stubClock{})
		216	+	WithSnapshotStore(&restoreSnapshotStore{})(d)
		217	+	d.serveCancel()
		218	+
		219	+	tr, _ := newCapturingTransport(t)
		220	+	sess, ac, err := d.route(ports.Hello{Intent: ports.IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}}, tr)
		221	+
		222	+	require.Nil(t, sess)
		223	+	require.Nil(t, ac)
		224	+	var perr *protoErr
		225	+	require.ErrorAs(t, err, &perr)
		226	+	require.Equal(t, ports.ErrServerShutdown, perr.code)
		227	+	}
		228	+
186	func mustSnapshotBytes(t *testing.T, s snapcodec.Session) []byte {	229		func mustSnapshotBytes(t *testing.T, s snapcodec.Session) []byte {
187	t.Helper()	230		t.Helper()
188	b, err := snapcodec.Marshal(s)	231		b, err := snapcodec.Marshal(s)

M internal/usecase/daemon/snapshot_test.go +10 -2


@@ -110,6 +110,7 @@ func TestPTYReaderSynchronizedUpdateFlushMarksSnapshotDirty(t *testing.T) {
110	110		defer cancel()
111	111		tb := newTestTabWithContext(p, sctx, cancel)
112	112		sess := &session{id: "sync", name: "sync", tabs: []*tab{tb}, ctx: sctx, cancel: cancel}
	113	+	sess.snapEligible.Store(true)
113	114		d.sessions[sess.id] = sess
114	115		d.sessWg.Add(1)
115	116		d.ptyReader(sess, tb, tb.focusedPane())

@@ -173,9 +174,14 @@ func TestKillSessionPurgeReturnsSnapshotDeleteError(t *testing.T) {
173	174		d.sessions[sess.id] = sess
174	175
175	176		store.EXPECT().Delete("work").Return(errors.New("delete failed")).Once()
	177	+	mockPTY, ok := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
	178	+	require.True(t, ok)
	179	+	mockPTY.EXPECT().Close().Return(nil).Once()
	180	+
176	181		err := d.killSession(sess, 0, true)
177	182		require.ErrorContains(t, err, "delete failed")
178		-	require.Contains(t, d.sessions, sess.id)
	183	+	require.NotContains(t, d.sessions, sess.id)
	184	+	require.NotContains(t, d.stopped, "work")
179	185		}
180	186
181	187		func TestRefreshSessionCwdMarksSnapshotDirty(t *testing.T) {

@@ -300,7 +306,7 @@ func newSnapshotTestSession(t *testing.T, name string, ephemeral bool, cwd strin
300	306		p := tb.panes["pane-1"]
301	307		p.screen.Write([]byte("hello"))
302	308		p.scrollback.Append([]renderer.Cell{{Rune: 'h'}, {Rune: 'i'}})
303		-	return &session{
	309	+	sess := &session{
304	310		id: domain.SessionID("sess-" + name),
305	311		name: name,
306	312		ephemeral: ephemeral,

@@ -311,6 +317,8 @@ func newSnapshotTestSession(t *testing.T, name string, ephemeral bool, cwd strin
311	317		cwd: cwd,
312	318		createdAt: 42,
313	319		}
	320	+	sess.snapEligible.Store(!ephemeral && name != "")
	321	+	return sess
314	322		}
315	323
316	324		type manualSnapshotClock struct {

 	defer cancel()
 	tb := newTestTabWithContext(p, sctx, cancel)
 	sess := &session{id: "sync", name: "sync", tabs: []*tab{tb}, ctx: sctx, cancel: cancel}
+	sess.snapEligible.Store(true)
 	d.sessions[sess.id] = sess
 	d.sessWg.Add(1)
 	d.ptyReader(sess, tb, tb.focusedPane())
 	d.sessions[sess.id] = sess
 	store.EXPECT().Delete("work").Return(errors.New("delete failed")).Once()
+	mockPTY, ok := sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY)
+	require.True(t, ok)
+	mockPTY.EXPECT().Close().Return(nil).Once()
 	err := d.killSession(sess, 0, true)
 	require.ErrorContains(t, err, "delete failed")
-	require.Contains(t, d.sessions, sess.id)
+	require.NotContains(t, d.sessions, sess.id)
+	require.NotContains(t, d.stopped, "work")
 }
 func TestRefreshSessionCwdMarksSnapshotDirty(t *testing.T) {
 	p := tb.panes["pane-1"]
 	p.screen.Write([]byte("hello"))
 	p.scrollback.Append([]renderer.Cell{{Rune: 'h'}, {Rune: 'i'}})
-	return &session{
+	sess := &session{
 		id:        domain.SessionID("sess-" + name),
 		name:      name,
 		ephemeral: ephemeral,
 		cwd:       cwd,
 		createdAt: 42,
 	}
+	sess.snapEligible.Store(!ephemeral && name != "")
+	return sess
 }
 type manualSnapshotClock struct {

M internal/usecase/snapshot/codec.go +9 -6


@@ -273,10 +273,11 @@ func writeTab(w *payloadWriter, t Tab) error {
273	273		if err := w.putString(string(t.Focus)); err != nil {
274	274		return err
275	275		}
276		-	if t.Tree == nil {
277		-	t.Tree = &layout.Tree{Focus: t.Focus}
	276	+	var root *layout.Node
	277	+	if t.Tree != nil {
	278	+	root = t.Tree.Root
278	279		}
279		-	if err := writeNode(w, t.Tree.Root); err != nil {
	280	+	if err := writeNode(w, root); err != nil {
280	281		return err
281	282		}
282	283		if len(t.Panes) > math.MaxUint16 {

@@ -329,8 +330,8 @@ func readTab(r *payloadReader) (Tab, error) {
329	330
330	331		func writeNode(w payloadWriter, n layout.Node) error {
331	332		if n == nil {
332		-	w.putUint8(0)
333		-	return w.putString("")
	333	+	w.putUint8(3)
	334	+	return nil
334	335		}
335	336		switch n.Kind {
336	337		case layout.Leaf:

@@ -396,6 +397,8 @@ func readNode(r payloadReader) (layout.Node, error) {
396	397		return nil, err
397	398		}
398	399		return &layout.Node{Kind: layout.Stack, Expanded: layout.PaneID(exp), Children: children}, nil
	400	+	case 3:
	401	+	return nil, nil
399	402		default:
400	403		return nil, fmt.Errorf("%w: unknown node kind", ErrInvalidData)
401	404		}

@@ -713,7 +716,7 @@ func validateTree(t *layout.Tree, ids map[layout.PaneID]struct{}) error {
713	716
714	717		func validateNode(n *layout.Node, ids map[layout.PaneID]struct{}) error {
715	718		if n == nil {
716		-	return nil
	719	+	return fmt.Errorf("%w: nil child node", ErrInvalidData)
717	720		}
718	721		switch n.Kind {
719	722		case layout.Leaf:

 	if err := w.putString(string(t.Focus)); err != nil {
 		return err
 	}
-	if t.Tree == nil {
+	var root *layout.Node
-		t.Tree = &layout.Tree{Focus: t.Focus}
+	if t.Tree != nil {
+		root = t.Tree.Root
 	}
-	if err := writeNode(w, t.Tree.Root); err != nil {
+	if err := writeNode(w, root); err != nil {
 		return err
 	}
 	if len(t.Panes) > math.MaxUint16 {
 func writeNode(w *payloadWriter, n *layout.Node) error {
 	if n == nil {
-		w.putUint8(0)
+		w.putUint8(3)
-		return w.putString("")
+		return nil
 	}
 	switch n.Kind {
 	case layout.Leaf:
 			return nil, err
 		}
 		return &layout.Node{Kind: layout.Stack, Expanded: layout.PaneID(exp), Children: children}, nil
+	case 3:
+		return nil, nil
 	default:
 		return nil, fmt.Errorf("%w: unknown node kind", ErrInvalidData)
 	}
 func validateNode(n *layout.Node, ids map[layout.PaneID]struct{}) error {
 	if n == nil {
-		return nil
+		return fmt.Errorf("%w: nil child node", ErrInvalidData)
 	}
 	switch n.Kind {
 	case layout.Leaf:

M internal/usecase/snapshot/codec_test.go +39 -1


@@ -26,6 +26,37 @@ func TestMarshalMinimalGolden(t *testing.T) {
26	26		}
27	27		}
28	28
	29	+	func TestRoundTripNilTreeAndRootDoNotMaterializeLeaf(t *testing.T) {
	30	+	cases := []struct {
	31	+	name string
	32	+	tab Tab
	33	+	}{
	34	+	{name: "nil tree", tab: Tab{Tree: nil}},
	35	+	{name: "nil root", tab: Tab{Tree: &layout.Tree{Root: nil}}},
	36	+	}
	37	+	for _, tc := range cases {
	38	+	t.Run(tc.name, func(t *testing.T) {
	39	+	b, err := Marshal(Session{Name: "s", Tabs: []Tab{tc.tab}})
	40	+	if err != nil {
	41	+	t.Fatalf("Marshal() error = %v", err)
	42	+	}
	43	+	got, err := Unmarshal(b)
	44	+	if err != nil {
	45	+	t.Fatalf("Unmarshal() error = %v", err)
	46	+	}
	47	+	if len(got.Tabs) != 1 {
	48	+	t.Fatalf("tabs len = %d, want 1", len(got.Tabs))
	49	+	}
	50	+	if got.Tabs[0].Tree == nil {
	51	+	t.Fatalf("Tree = nil, want tree with nil root")
	52	+	}
	53	+	if got.Tabs[0].Tree.Root != nil {
	54	+	t.Fatalf("Root = %#v, want nil", got.Tabs[0].Tree.Root)
	55	+	}
	56	+	})
	57	+	}
	58	+	}
	59	+
29	60		func TestRoundTripSessions(t *testing.T) {
30	61		boldRGB := renderer.DefaultStyle()
31	62		boldRGB.Bold = true

@@ -217,7 +248,7 @@ func equalSession(a, b Session) bool {
217	248		}
218	249
219	250		func equalTab(a, b Tab) bool {
220		-	if a.Cols != b.Cols \|\| a.Rows != b.Rows \|\| a.NextPaneID != b.NextPaneID \|\| a.Focus != b.Focus \|\| len(a.Panes) != len(b.Panes) \|\| !equalNode(a.Tree.Root, b.Tree.Root) {
	251	+	if a.Cols != b.Cols \|\| a.Rows != b.Rows \|\| a.NextPaneID != b.NextPaneID \|\| a.Focus != b.Focus \|\| len(a.Panes) != len(b.Panes) \|\| !equalNode(treeRoot(a.Tree), treeRoot(b.Tree)) {
221	252		return false
222	253		}
223	254		for i := range a.Panes {

@@ -228,6 +259,13 @@ func equalTab(a, b Tab) bool {
228	259		return true
229	260		}
230	261
	262	+	func treeRoot(t layout.Tree) layout.Node {
	263	+	if t == nil {
	264	+	return nil
	265	+	}
	266	+	return t.Root
	267	+	}
	268	+
231	269		func equalNode(a, b *layout.Node) bool {
232	270		if a == nil \|\| b == nil {
233	271		return a == b


@@ -26,6 +26,37 @@ func TestMarshalMinimalGolden(t *testing.T) {
26		}	26		}
27		}	27		}
28			28
			29	+	func TestRoundTripNilTreeAndRootDoNotMaterializeLeaf(t *testing.T) {
			30	+	cases := []struct {
			31	+	name string
			32	+	tab Tab
			33	+	}{
			34	+	{name: "nil tree", tab: Tab{Tree: nil}},
			35	+	{name: "nil root", tab: Tab{Tree: &layout.Tree{Root: nil}}},
			36	+	}
			37	+	for _, tc := range cases {
			38	+	t.Run(tc.name, func(t *testing.T) {
			39	+	b, err := Marshal(Session{Name: "s", Tabs: []Tab{tc.tab}})
			40	+	if err != nil {
			41	+	t.Fatalf("Marshal() error = %v", err)
			42	+	}
			43	+	got, err := Unmarshal(b)
			44	+	if err != nil {
			45	+	t.Fatalf("Unmarshal() error = %v", err)
			46	+	}
			47	+	if len(got.Tabs) != 1 {
			48	+	t.Fatalf("tabs len = %d, want 1", len(got.Tabs))
			49	+	}
			50	+	if got.Tabs[0].Tree == nil {
			51	+	t.Fatalf("Tree = nil, want tree with nil root")
			52	+	}
			53	+	if got.Tabs[0].Tree.Root != nil {
			54	+	t.Fatalf("Root = %#v, want nil", got.Tabs[0].Tree.Root)
			55	+	}
			56	+	})
			57	+	}
			58	+	}
			59	+
29		func TestRoundTripSessions(t *testing.T) {	60		func TestRoundTripSessions(t *testing.T) {
30		boldRGB := renderer.DefaultStyle()	61		boldRGB := renderer.DefaultStyle()
31		boldRGB.Bold = true	62		boldRGB.Bold = true

@@ -217,7 +248,7 @@ func equalSession(a, b Session) bool {
217		}	248		}
218			249
219		func equalTab(a, b Tab) bool {	250		func equalTab(a, b Tab) bool {
220	-	if a.Cols != b.Cols \|\| a.Rows != b.Rows \|\| a.NextPaneID != b.NextPaneID \|\| a.Focus != b.Focus \|\| len(a.Panes) != len(b.Panes) \|\| !equalNode(a.Tree.Root, b.Tree.Root) {	251	+	if a.Cols != b.Cols \|\| a.Rows != b.Rows \|\| a.NextPaneID != b.NextPaneID \|\| a.Focus != b.Focus \|\| len(a.Panes) != len(b.Panes) \|\| !equalNode(treeRoot(a.Tree), treeRoot(b.Tree)) {
221		return false	252		return false
222		}	253		}
223		for i := range a.Panes {	254		for i := range a.Panes {

@@ -228,6 +259,13 @@ func equalTab(a, b Tab) bool {
228		return true	259		return true
229		}	260		}
230			261
			262	+	func treeRoot(t layout.Tree) layout.Node {
			263	+	if t == nil {
			264	+	return nil
			265	+	}
			266	+	return t.Root
			267	+	}
			268	+
231		func equalNode(a, b *layout.Node) bool {	269		func equalNode(a, b *layout.Node) bool {
232		if a == nil \|\| b == nil {	270		if a == nil \|\| b == nil {
233		return a == b	271		return a == b