fix(daemon): surface failed session restores at attach
Each of restoreSnapshots' three failure sites (Load, decode, restoreSession) now also raises a NoticeSnapshotRestore global notice alongside its existing log line, so a broken saved session is no longer silently dropped at startup. These run pre-attach, so NotifyGlobal queues into the pending queue: N failures dedup into one toast with Count N at the first attach, while each failure still lands in history individually.
- commit
- 2175cce059a1ebce79a1c73e398b2cec70a8295e
- author
- brice · · 2026-07-21 04:25 UTC
- parent
- c32a9ec
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +55 -0
M internal/usecase/daemon/snapshot.go +6 -0
| @@ -69,6 +69,8 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||
| 69 | 69 | blobs, err := d.snaps.Load() | |
| 70 | 70 | if err != nil { | |
| 71 | 71 | d.log.Warn("loading session snapshots failed", "err", err) | |
| 72 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |
| 73 | + | "couldn't load saved sessions after restart", err) | |
| 72 | 74 | return | |
| 73 | 75 | } | |
| 74 | 76 | for _, blob := range blobs { | |
| @@ -80,10 +82,14 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||
| 80 | 82 | snap, err := snapcodec.Unmarshal(blob.Data) | |
| 81 | 83 | if err != nil { | |
| 82 | 84 | d.log.Warn("decoding session snapshot failed", "err", err, "session", blob.Name) | |
| 85 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |
| 86 | + | "couldn't restore session "+blob.Name+" after restart", err) | |
| 83 | 87 | continue | |
| 84 | 88 | } | |
| 85 | 89 | if err := d.restoreSession(ctx, snap); err != nil { | |
| 86 | 90 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | |
| 91 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |
| 92 | + | "couldn't restore session "+snap.Name+" after restart", err) | |
| 87 | 93 | } | |
| 88 | 94 | } | |
| 89 | 95 | } | |
| @@ -69,6 +69,8 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||||
| 69 | blobs, err := d.snaps.Load() | 69 | blobs, err := d.snaps.Load() | ||
| 70 | if err != nil { | 70 | if err != nil { | ||
| 71 | d.log.Warn("loading session snapshots failed", "err", err) | 71 | d.log.Warn("loading session snapshots failed", "err", err) | ||
| 72 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |||
| 73 | + | "couldn't load saved sessions after restart", err) | |||
| 72 | return | 74 | return | ||
| 73 | } | 75 | } | ||
| 74 | for _, blob := range blobs { | 76 | for _, blob := range blobs { | ||
| @@ -80,10 +82,14 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||||
| 80 | snap, err := snapcodec.Unmarshal(blob.Data) | 82 | snap, err := snapcodec.Unmarshal(blob.Data) | ||
| 81 | if err != nil { | 83 | if err != nil { | ||
| 82 | d.log.Warn("decoding session snapshot failed", "err", err, "session", blob.Name) | 84 | d.log.Warn("decoding session snapshot failed", "err", err, "session", blob.Name) | ||
| 85 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |||
| 86 | + | "couldn't restore session "+blob.Name+" after restart", err) | |||
| 83 | continue | 87 | continue | ||
| 84 | } | 88 | } | ||
| 85 | if err := d.restoreSession(ctx, snap); err != nil { | 89 | if err := d.restoreSession(ctx, snap); err != nil { | ||
| 86 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | 90 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | ||
| 91 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |||
| 92 | + | "couldn't restore session "+snap.Name+" after restart", err) | |||
| 87 | } | 93 | } | ||
| 88 | } | 94 | } | ||
| 89 | } | 95 | } | ||
M internal/usecase/daemon/snapshot_restore_test.go +49 -0
| @@ -497,6 +497,55 @@ func TestRestoreSnapshotsSkipsCreatedAtOverflow(t *testing.T) { | |||
| 497 | 497 | require.Nil(t, restored) | |
| 498 | 498 | } | |
| 499 | 499 | ||
| 500 | + | // TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots proves each | |
| 501 | + | // unrestorable snapshot is recorded to history individually, while the | |
| 502 | + | // pending-global queue dedups them by code into a single toast with the | |
| 503 | + | // combined Count. Both blobs decode fine (valid snapcodec bytes, Marshal | |
| 504 | + | // itself validates Active/tab range) but fail in restoreSession itself: an | |
| 505 | + | // empty embedded session name is rejected deterministically, before any lock | |
| 506 | + | // is taken or PTY is opened. | |
| 507 | + | func TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots(t *testing.T) { | |
| 508 | + | badSnap := mustSnapshotBytes(t, snapcodec.Session{Name: ""}) | |
| 509 | + | store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{ | |
| 510 | + | {Name: "bad-1", Data: badSnap}, | |
| 511 | + | {Name: "bad-2", Data: badSnap}, | |
| 512 | + | }} | |
| 513 | + | factory := &restorePTYFactory{} | |
| 514 | + | d := newTestDaemon(t, factory, newNoticeClock()) | |
| 515 | + | WithSnapshotStore(store)(d) | |
| 516 | + | ||
| 517 | + | // Mirrors real startup (daemon.go's Serve): restoreSnapshots runs off the | |
| 518 | + | // caller goroutine and signals completion by closing d.restoreDone. | |
| 519 | + | d.sessWg.Go(func() { d.restoreSnapshots(d.serveCtx) }) | |
| 520 | + | ||
| 521 | + | select { | |
| 522 | + | case <-d.restoreDone: | |
| 523 | + | case <-time.After(2 * time.Second): | |
| 524 | + | t.Fatal("restoreSnapshots did not close restoreDone in time") | |
| 525 | + | } | |
| 526 | + | ||
| 527 | + | require.Empty(t, factory.opens, "both snapshots must fail before any PTY is opened") | |
| 528 | + | require.Len(t, d.notices.history(), 2, "each failure is recorded individually") | |
| 529 | + | for _, n := range d.notices.history() { | |
| 530 | + | require.Equal(t, domain.NoticeSnapshotRestore, n.Code) | |
| 531 | + | require.Equal(t, domain.NoticeError, n.Severity) | |
| 532 | + | } | |
| 533 | + | ||
| 534 | + | tr, _ := newCapturingTransport(t) | |
| 535 | + | sess, ac, err := d.route(ports.Hello{ | |
| 536 | + | Version: ports.ProtocolVersion, | |
| 537 | + | Intent: ports.IntentEphemeral, | |
| 538 | + | Size: domain.Size{Cols: 80, Rows: 24}, | |
| 539 | + | }, tr) | |
| 540 | + | require.NoError(t, err) | |
| 541 | + | d.firstPaint(sess, ac, ac.size) | |
| 542 | + | ||
| 543 | + | toasts := awaitToastCount(t, ac, 1) | |
| 544 | + | require.Equal(t, domain.NoticeSnapshotRestore, toasts[0].Code) | |
| 545 | + | require.Equal(t, 2, toasts[0].Count, "two same-code failures dedup into one toast counted twice") | |
| 546 | + | require.Empty(t, d.notices.drainPending(), "firstPaint must consume the queue") | |
| 547 | + | } | |
| 548 | + | ||
| 500 | 549 | func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) { | |
| 501 | 550 | releaseLoad := make(chan struct{}) | |
| 502 | 551 | loaded := make(chan struct{}) | |
| @@ -497,6 +497,55 @@ func TestRestoreSnapshotsSkipsCreatedAtOverflow(t *testing.T) { | |||||
| 497 | require.Nil(t, restored) | 497 | require.Nil(t, restored) | ||
| 498 | } | 498 | } | ||
| 499 | 499 | ||||
| 500 | + | // TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots proves each | |||
| 501 | + | // unrestorable snapshot is recorded to history individually, while the | |||
| 502 | + | // pending-global queue dedups them by code into a single toast with the | |||
| 503 | + | // combined Count. Both blobs decode fine (valid snapcodec bytes, Marshal | |||
| 504 | + | // itself validates Active/tab range) but fail in restoreSession itself: an | |||
| 505 | + | // empty embedded session name is rejected deterministically, before any lock | |||
| 506 | + | // is taken or PTY is opened. | |||
| 507 | + | func TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots(t *testing.T) { | |||
| 508 | + | badSnap := mustSnapshotBytes(t, snapcodec.Session{Name: ""}) | |||
| 509 | + | store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{ | |||
| 510 | + | {Name: "bad-1", Data: badSnap}, | |||
| 511 | + | {Name: "bad-2", Data: badSnap}, | |||
| 512 | + | }} | |||
| 513 | + | factory := &restorePTYFactory{} | |||
| 514 | + | d := newTestDaemon(t, factory, newNoticeClock()) | |||
| 515 | + | WithSnapshotStore(store)(d) | |||
| 516 | + | ||||
| 517 | + | // Mirrors real startup (daemon.go's Serve): restoreSnapshots runs off the | |||
| 518 | + | // caller goroutine and signals completion by closing d.restoreDone. | |||
| 519 | + | d.sessWg.Go(func() { d.restoreSnapshots(d.serveCtx) }) | |||
| 520 | + | ||||
| 521 | + | select { | |||
| 522 | + | case <-d.restoreDone: | |||
| 523 | + | case <-time.After(2 * time.Second): | |||
| 524 | + | t.Fatal("restoreSnapshots did not close restoreDone in time") | |||
| 525 | + | } | |||
| 526 | + | ||||
| 527 | + | require.Empty(t, factory.opens, "both snapshots must fail before any PTY is opened") | |||
| 528 | + | require.Len(t, d.notices.history(), 2, "each failure is recorded individually") | |||
| 529 | + | for _, n := range d.notices.history() { | |||
| 530 | + | require.Equal(t, domain.NoticeSnapshotRestore, n.Code) | |||
| 531 | + | require.Equal(t, domain.NoticeError, n.Severity) | |||
| 532 | + | } | |||
| 533 | + | ||||
| 534 | + | tr, _ := newCapturingTransport(t) | |||
| 535 | + | sess, ac, err := d.route(ports.Hello{ | |||
| 536 | + | Version: ports.ProtocolVersion, | |||
| 537 | + | Intent: ports.IntentEphemeral, | |||
| 538 | + | Size: domain.Size{Cols: 80, Rows: 24}, | |||
| 539 | + | }, tr) | |||
| 540 | + | require.NoError(t, err) | |||
| 541 | + | d.firstPaint(sess, ac, ac.size) | |||
| 542 | + | ||||
| 543 | + | toasts := awaitToastCount(t, ac, 1) | |||
| 544 | + | require.Equal(t, domain.NoticeSnapshotRestore, toasts[0].Code) | |||
| 545 | + | require.Equal(t, 2, toasts[0].Count, "two same-code failures dedup into one toast counted twice") | |||
| 546 | + | require.Empty(t, d.notices.drainPending(), "firstPaint must consume the queue") | |||
| 547 | + | } | |||
| 548 | + | ||||
| 500 | func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) { | 549 | func TestNamedRouteWaitsForRestoreBarrier(t *testing.T) { | ||
| 501 | releaseLoad := make(chan struct{}) | 550 | releaseLoad := make(chan struct{}) | ||
| 502 | loaded := make(chan struct{}) | 551 | loaded := make(chan struct{}) | ||