fix(daemon): surface remaining session action errors
- commit
- 41eb8b04b8b7322574bd12772b420249058cd92b
- author
- brice · · 2026-07-21 11:43 UTC
- parent
- eb9f4eb
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +58 -2
M internal/usecase/daemon/input_test.go +21 -0
| @@ -648,6 +648,27 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||
| 648 | 648 | } | |
| 649 | 649 | } | |
| 650 | 650 | ||
| 651 | + | func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |
| 652 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 653 | + | defer releaseAll(releases) | |
| 654 | + | target := d.sessions[domain.SessionID("recent")] | |
| 655 | + | ac.previousSession.Set(target) | |
| 656 | + | ||
| 657 | + | // A displaced attachment makes the previously valid handoff stale by the | |
| 658 | + | // time switchToTarget commits it. | |
| 659 | + | current.mu.Lock() | |
| 660 | + | current.client = nil | |
| 661 | + | current.mu.Unlock() | |
| 662 | + | ||
| 663 | + | d.backSession(current, ac) | |
| 664 | + | ||
| 665 | + | require.Same(t, current, ac.currentSession(), "a stale handoff must leave the attachment on its origin") | |
| 666 | + | require.Same(t, target, ac.previousSession.Get(), "a failed handoff remains retryable") | |
| 667 | + | history := d.notices.history() | |
| 668 | + | require.Len(t, history, 1, "the switch failure must be reported exactly once") | |
| 669 | + | require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code) | |
| 670 | + | } | |
| 671 | + | ||
| 651 | 672 | func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | |
| 652 | 673 | ac := &attachedClient{} | |
| 653 | 674 | stale := &session{id: "stale"} | |
| @@ -648,6 +648,27 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||||
| 648 | } | 648 | } | ||
| 649 | } | 649 | } | ||
| 650 | 650 | ||||
| 651 | + | func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |||
| 652 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 653 | + | defer releaseAll(releases) | |||
| 654 | + | target := d.sessions[domain.SessionID("recent")] | |||
| 655 | + | ac.previousSession.Set(target) | |||
| 656 | + | ||||
| 657 | + | // A displaced attachment makes the previously valid handoff stale by the | |||
| 658 | + | // time switchToTarget commits it. | |||
| 659 | + | current.mu.Lock() | |||
| 660 | + | current.client = nil | |||
| 661 | + | current.mu.Unlock() | |||
| 662 | + | ||||
| 663 | + | d.backSession(current, ac) | |||
| 664 | + | ||||
| 665 | + | require.Same(t, current, ac.currentSession(), "a stale handoff must leave the attachment on its origin") | |||
| 666 | + | require.Same(t, target, ac.previousSession.Get(), "a failed handoff remains retryable") | |||
| 667 | + | history := d.notices.history() | |||
| 668 | + | require.Len(t, history, 1, "the switch failure must be reported exactly once") | |||
| 669 | + | require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code) | |||
| 670 | + | } | |||
| 671 | + | ||||
| 651 | func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | 672 | func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | ||
| 652 | ac := &attachedClient{} | 673 | ac := &attachedClient{} | ||
| 653 | stale := &session{id: "stale"} | 674 | stale := &session{id: "stale"} | ||
M internal/usecase/daemon/picker.go +1 -1
| @@ -564,7 +564,7 @@ func (d *Daemon) killPickerTarget(target picker.Target) error { | |||
| 564 | 564 | targetSess := d.sessions[target.Session] | |
| 565 | 565 | d.mu.Unlock() | |
| 566 | 566 | if targetSess != nil { | |
| 567 | - | _ = d.killSession(targetSess, ports.ReasonSessionKilled, true) | |
| 567 | + | return d.killSession(targetSess, ports.ReasonSessionKilled, true) | |
| 568 | 568 | } | |
| 569 | 569 | return nil | |
| 570 | 570 | } | |
| @@ -564,7 +564,7 @@ func (d *Daemon) killPickerTarget(target picker.Target) error { | |||||
| 564 | targetSess := d.sessions[target.Session] | 564 | targetSess := d.sessions[target.Session] | ||
| 565 | d.mu.Unlock() | 565 | d.mu.Unlock() | ||
| 566 | if targetSess != nil { | 566 | if targetSess != nil { | ||
| 567 | - | _ = d.killSession(targetSess, ports.ReasonSessionKilled, true) | 567 | + | return d.killSession(targetSess, ports.ReasonSessionKilled, true) |
| 568 | } | 568 | } | ||
| 569 | return nil | 569 | return nil | ||
| 570 | } | 570 | } | ||
M internal/usecase/daemon/picker_test.go +33 -0
| @@ -35,6 +35,12 @@ func (failingDeleteStore) Range(func(k, v []byte) bool) {} | |||
| 35 | 35 | func (failingDeleteStore) Sync() error { return nil } | |
| 36 | 36 | func (failingDeleteStore) Close() error { return nil } | |
| 37 | 37 | ||
| 38 | + | type refusingSnapshotDeleteStore struct{ err error } | |
| 39 | + | ||
| 40 | + | func (refusingSnapshotDeleteStore) Write(string, []byte) error { return nil } | |
| 41 | + | func (refusingSnapshotDeleteStore) Load() ([]ports.SnapshotBlob, error) { return nil, nil } | |
| 42 | + | func (s refusingSnapshotDeleteStore) Delete(string) error { return s.err } | |
| 43 | + | ||
| 38 | 44 | func newTestTabWithContext(p ports.PTY, ctx context.Context, cancel context.CancelFunc) *tab { | |
| 39 | 45 | tb := newTab(p, domain.Size{Cols: 80, Rows: 23}) | |
| 40 | 46 | tb.ctx, tb.cancel = ctx, cancel | |
| @@ -992,6 +998,33 @@ func TestPickerEnterOnStoppedSessionRestoreFailureSurfacesNoticeAndStaysPut(t *t | |||
| 992 | 998 | require.Same(t, from, ac.currentSession(), "a failed restore must leave the client on its origin session") | |
| 993 | 999 | } | |
| 994 | 1000 | ||
| 1001 | + | func TestPickerKillActiveSessionSnapshotDeleteRefusalReportsOnceAndKeepsPicker(t *testing.T) { | |
| 1002 | + | d, from, ac, sends, releases := newRecentNavigationTestSessions(t) | |
| 1003 | + | defer releaseAll(releases) | |
| 1004 | + | cause := errors.New("snapshot delete refused") | |
| 1005 | + | WithSnapshotStore(refusingSnapshotDeleteStore{err: cause})(d) | |
| 1006 | + | target := d.sessions[domain.SessionID("recent")] | |
| 1007 | + | ||
| 1008 | + | d.enterPicker(from, ac) | |
| 1009 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 1010 | + | d.handleInput(from, ac, []byte("k")) | |
| 1011 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 1012 | + | d.handleInput(from, ac, []byte("x")) | |
| 1013 | + | ||
| 1014 | + | history := d.notices.history() | |
| 1015 | + | require.Len(t, history, 1, "the refused purge must be reported exactly once") | |
| 1016 | + | require.Equal(t, domain.NoticeInternal, history[0].Code) | |
| 1017 | + | require.Contains(t, history[0].Details, cause.Error()) | |
| 1018 | + | require.Len(t, awaitToastCount(t, ac, 1), 1, "the refusal must produce one visible notice") | |
| 1019 | + | ||
| 1020 | + | require.Same(t, from, ac.currentSession(), "purging another session must preserve the attachment") | |
| 1021 | + | require.True(t, ac.overlays.pickerActive(), "x refreshes the picker instead of closing it") | |
| 1022 | + | d.mu.Lock() | |
| 1023 | + | _, stillActive := d.sessions[target.id] | |
| 1024 | + | d.mu.Unlock() | |
| 1025 | + | require.False(t, stillActive, "the purge keeps its existing removal semantics despite cleanup failure") | |
| 1026 | + | } | |
| 1027 | + | ||
| 995 | 1028 | // TestPickerKillStoppedSessionPersistDeleteFailureSurfacesNoticeAndKeepsEntry | |
| 996 | 1029 | // drives 'x' on a stopped picker entry whose persisted-record delete fails | |
| 997 | 1030 | // (mock store error). The failure must reach the user as a | |
| @@ -35,6 +35,12 @@ func (failingDeleteStore) Range(func(k, v []byte) bool) {} | |||||
| 35 | func (failingDeleteStore) Sync() error { return nil } | 35 | func (failingDeleteStore) Sync() error { return nil } | ||
| 36 | func (failingDeleteStore) Close() error { return nil } | 36 | func (failingDeleteStore) Close() error { return nil } | ||
| 37 | 37 | ||||
| 38 | + | type refusingSnapshotDeleteStore struct{ err error } | |||
| 39 | + | ||||
| 40 | + | func (refusingSnapshotDeleteStore) Write(string, []byte) error { return nil } | |||
| 41 | + | func (refusingSnapshotDeleteStore) Load() ([]ports.SnapshotBlob, error) { return nil, nil } | |||
| 42 | + | func (s refusingSnapshotDeleteStore) Delete(string) error { return s.err } | |||
| 43 | + | ||||
| 38 | func newTestTabWithContext(p ports.PTY, ctx context.Context, cancel context.CancelFunc) *tab { | 44 | func newTestTabWithContext(p ports.PTY, ctx context.Context, cancel context.CancelFunc) *tab { | ||
| 39 | tb := newTab(p, domain.Size{Cols: 80, Rows: 23}) | 45 | tb := newTab(p, domain.Size{Cols: 80, Rows: 23}) | ||
| 40 | tb.ctx, tb.cancel = ctx, cancel | 46 | tb.ctx, tb.cancel = ctx, cancel | ||
| @@ -992,6 +998,33 @@ func TestPickerEnterOnStoppedSessionRestoreFailureSurfacesNoticeAndStaysPut(t *t | |||||
| 992 | require.Same(t, from, ac.currentSession(), "a failed restore must leave the client on its origin session") | 998 | require.Same(t, from, ac.currentSession(), "a failed restore must leave the client on its origin session") | ||
| 993 | } | 999 | } | ||
| 994 | 1000 | ||||
| 1001 | + | func TestPickerKillActiveSessionSnapshotDeleteRefusalReportsOnceAndKeepsPicker(t *testing.T) { | |||
| 1002 | + | d, from, ac, sends, releases := newRecentNavigationTestSessions(t) | |||
| 1003 | + | defer releaseAll(releases) | |||
| 1004 | + | cause := errors.New("snapshot delete refused") | |||
| 1005 | + | WithSnapshotStore(refusingSnapshotDeleteStore{err: cause})(d) | |||
| 1006 | + | target := d.sessions[domain.SessionID("recent")] | |||
| 1007 | + | ||||
| 1008 | + | d.enterPicker(from, ac) | |||
| 1009 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 1010 | + | d.handleInput(from, ac, []byte("k")) | |||
| 1011 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 1012 | + | d.handleInput(from, ac, []byte("x")) | |||
| 1013 | + | ||||
| 1014 | + | history := d.notices.history() | |||
| 1015 | + | require.Len(t, history, 1, "the refused purge must be reported exactly once") | |||
| 1016 | + | require.Equal(t, domain.NoticeInternal, history[0].Code) | |||
| 1017 | + | require.Contains(t, history[0].Details, cause.Error()) | |||
| 1018 | + | require.Len(t, awaitToastCount(t, ac, 1), 1, "the refusal must produce one visible notice") | |||
| 1019 | + | ||||
| 1020 | + | require.Same(t, from, ac.currentSession(), "purging another session must preserve the attachment") | |||
| 1021 | + | require.True(t, ac.overlays.pickerActive(), "x refreshes the picker instead of closing it") | |||
| 1022 | + | d.mu.Lock() | |||
| 1023 | + | _, stillActive := d.sessions[target.id] | |||
| 1024 | + | d.mu.Unlock() | |||
| 1025 | + | require.False(t, stillActive, "the purge keeps its existing removal semantics despite cleanup failure") | |||
| 1026 | + | } | |||
| 1027 | + | ||||
| 995 | // TestPickerKillStoppedSessionPersistDeleteFailureSurfacesNoticeAndKeepsEntry | 1028 | // TestPickerKillStoppedSessionPersistDeleteFailureSurfacesNoticeAndKeepsEntry | ||
| 996 | // drives 'x' on a stopped picker entry whose persisted-record delete fails | 1029 | // drives 'x' on a stopped picker entry whose persisted-record delete fails | ||
| 997 | // (mock store error). The failure must reach the user as a | 1030 | // (mock store error). The failure must reach the user as a | ||
M internal/usecase/daemon/session_back.go +3 -1
| @@ -14,7 +14,9 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||
| 14 | 14 | d.invalidateRender(current, ac, true, "session_back.go") | |
| 15 | 15 | return | |
| 16 | 16 | } | |
| 17 | - | _ = d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}) | |
| 17 | + | if err := d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}); err != nil { | |
| 18 | + | d.reportError(current, err) | |
| 19 | + | } | |
| 18 | 20 | } | |
| 19 | 21 | ||
| 20 | 22 | // clearPreviousSessionIf clears target only if it has not been replaced since | |
| @@ -14,7 +14,9 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||||
| 14 | d.invalidateRender(current, ac, true, "session_back.go") | 14 | d.invalidateRender(current, ac, true, "session_back.go") | ||
| 15 | return | 15 | return | ||
| 16 | } | 16 | } | ||
| 17 | - | _ = d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}) | 17 | + | if err := d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}); err != nil { |
| 18 | + | d.reportError(current, err) | |||
| 19 | + | } | |||
| 18 | } | 20 | } | ||
| 19 | 21 | ||||
| 20 | // clearPreviousSessionIf clears target only if it has not been replaced since | 22 | // clearPreviousSessionIf clears target only if it has not been replaced since | ||