fix(remote): close remote transition races
- commit
- 76c6a00538b2a16d24d0e2c4020349ec8cb2d50a
- author
- brice · · 2026-08-07 11:01 UTC
- parent
- 1626715
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +137 -17
M internal/usecase/daemon/coderabbit_regression_test.go +17 -0
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "sync" | |
| 9 | 9 | "syscall" | |
| 10 | 10 | "testing" | |
| 11 | + | "time" | |
| 11 | 12 | ||
| 12 | 13 | "github.com/stretchr/testify/require" | |
| 13 | 14 | ||
| @@ -257,6 +258,22 @@ func TestShutdownDeadlineDoesNotWaitForUncooperativeRemoteConstruction(t *testin | |||
| 257 | 258 | require.True(t, awaitTestValue(t, result, "shutdown blocked on canceled remote construction")) | |
| 258 | 259 | } | |
| 259 | 260 | ||
| 261 | + | func TestShutdownWithoutDeadlineBoundsUncooperativeRemoteConstruction(t *testing.T) { | |
| 262 | + | clock := &signalClock{timers: make(chan *signalTimer, 1)} | |
| 263 | + | d := newTestDaemon(t, nil, clock) | |
| 264 | + | construction := &remoteViewConstruction{done: make(chan struct{})} | |
| 265 | + | d.mu.Lock() | |
| 266 | + | d.remoteViewConstructions[remoteViewKey{}] = construction | |
| 267 | + | d.mu.Unlock() | |
| 268 | + | ||
| 269 | + | result := make(chan bool, 1) | |
| 270 | + | go func() { result <- d.shutdownAll(ports.ReasonServerShutdown) }() | |
| 271 | + | timer := awaitTestValue(t, clock.timers, "remote construction shutdown timer") | |
| 272 | + | require.Equal(t, remoteConstructionShutdownGrace, timer.duration) | |
| 273 | + | timer.ch <- time.Now() | |
| 274 | + | require.True(t, awaitTestValue(t, result, "shutdown blocked on uncooperative remote construction")) | |
| 275 | + | } | |
| 276 | + | ||
| 260 | 277 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | |
| 261 | 278 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | |
| 262 | 279 | _, cancel := snapshotStopContext(deadline) | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "sync" | 8 | "sync" | ||
| 9 | "syscall" | 9 | "syscall" | ||
| 10 | "testing" | 10 | "testing" | ||
| 11 | + | "time" | |||
| 11 | 12 | ||||
| 12 | "github.com/stretchr/testify/require" | 13 | "github.com/stretchr/testify/require" | ||
| 13 | 14 | ||||
| @@ -257,6 +258,22 @@ func TestShutdownDeadlineDoesNotWaitForUncooperativeRemoteConstruction(t *testin | |||||
| 257 | require.True(t, awaitTestValue(t, result, "shutdown blocked on canceled remote construction")) | 258 | require.True(t, awaitTestValue(t, result, "shutdown blocked on canceled remote construction")) | ||
| 258 | } | 259 | } | ||
| 259 | 260 | ||||
| 261 | + | func TestShutdownWithoutDeadlineBoundsUncooperativeRemoteConstruction(t *testing.T) { | |||
| 262 | + | clock := &signalClock{timers: make(chan *signalTimer, 1)} | |||
| 263 | + | d := newTestDaemon(t, nil, clock) | |||
| 264 | + | construction := &remoteViewConstruction{done: make(chan struct{})} | |||
| 265 | + | d.mu.Lock() | |||
| 266 | + | d.remoteViewConstructions[remoteViewKey{}] = construction | |||
| 267 | + | d.mu.Unlock() | |||
| 268 | + | ||||
| 269 | + | result := make(chan bool, 1) | |||
| 270 | + | go func() { result <- d.shutdownAll(ports.ReasonServerShutdown) }() | |||
| 271 | + | timer := awaitTestValue(t, clock.timers, "remote construction shutdown timer") | |||
| 272 | + | require.Equal(t, remoteConstructionShutdownGrace, timer.duration) | |||
| 273 | + | timer.ch <- time.Now() | |||
| 274 | + | require.True(t, awaitTestValue(t, result, "shutdown blocked on uncooperative remote construction")) | |||
| 275 | + | } | |||
| 276 | + | ||||
| 260 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | 277 | func TestSnapshotStopContextCancelIsIdempotent(t *testing.T) { | ||
| 261 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | 278 | deadline := &snapshotShutdownDeadline{done: make(chan struct{})} | ||
| 262 | _, cancel := snapshotStopContext(deadline) | 279 | _, cancel := snapshotStopContext(deadline) | ||
M internal/usecase/daemon/daemon.go +23 -1
| @@ -45,6 +45,10 @@ const ( | |||
| 45 | 45 | // the in-flight send. Teardown is never gated on a client draining its socket. | |
| 46 | 46 | const detachNotifyTimeout = time.Second | |
| 47 | 47 | ||
| 48 | + | // remoteConstructionShutdownGrace bounds a non-cooperative remote dialer when | |
| 49 | + | // durable snapshots have not installed a shared shutdown deadline. | |
| 50 | + | const remoteConstructionShutdownGrace = ports.HandshakeTimeout | |
| 51 | + | ||
| 48 | 52 | // maxUnackedOutputStates caps how many output states may be in flight (sent but | |
| 49 | 53 | // not yet acked by the client) before paint defers rather than composing | |
| 50 | 54 | // another diff. It bounds the daemon's paint rate to the client's ack rate, so | |
| @@ -835,12 +839,30 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||
| 835 | 839 | // Candidate construction owns dial and handshake I/O outside daemon locks. | |
| 836 | 840 | // A shutdown deadline bounds a non-cooperative dialer; a late constructor | |
| 837 | 841 | // observes d.closing and retires its unpublished candidate independently. | |
| 842 | + | var constructionTimer ports.Timer | |
| 843 | + | var constructionTimeout <-chan time.Time | |
| 844 | + | if deadline == nil && len(constructions) != 0 { | |
| 845 | + | clock := d.clock | |
| 846 | + | if clock == nil { | |
| 847 | + | clock = systemClock{} | |
| 848 | + | } | |
| 849 | + | constructionTimer = clock.NewTimer(remoteConstructionShutdownGrace) | |
| 850 | + | if constructionTimer == nil || constructionTimer.C() == nil { | |
| 851 | + | constructionTimer = systemClock{}.NewTimer(remoteConstructionShutdownGrace) | |
| 852 | + | } | |
| 853 | + | constructionTimeout = constructionTimer.C() | |
| 854 | + | defer constructionTimer.Stop() | |
| 855 | + | } | |
| 838 | 856 | for _, construction := range constructions { | |
| 839 | 857 | if construction == nil || construction.done == nil { | |
| 840 | 858 | continue | |
| 841 | 859 | } | |
| 842 | 860 | if deadline == nil { | |
| 843 | - | <-construction.done | |
| 861 | + | select { | |
| 862 | + | case <-construction.done: | |
| 863 | + | case <-constructionTimeout: | |
| 864 | + | checkpointIncomplete = true | |
| 865 | + | } | |
| 844 | 866 | continue | |
| 845 | 867 | } | |
| 846 | 868 | select { | |
| @@ -45,6 +45,10 @@ const ( | |||||
| 45 | // the in-flight send. Teardown is never gated on a client draining its socket. | 45 | // the in-flight send. Teardown is never gated on a client draining its socket. | ||
| 46 | const detachNotifyTimeout = time.Second | 46 | const detachNotifyTimeout = time.Second | ||
| 47 | 47 | ||||
| 48 | + | // remoteConstructionShutdownGrace bounds a non-cooperative remote dialer when | |||
| 49 | + | // durable snapshots have not installed a shared shutdown deadline. | |||
| 50 | + | const remoteConstructionShutdownGrace = ports.HandshakeTimeout | |||
| 51 | + | ||||
| 48 | // maxUnackedOutputStates caps how many output states may be in flight (sent but | 52 | // maxUnackedOutputStates caps how many output states may be in flight (sent but | ||
| 49 | // not yet acked by the client) before paint defers rather than composing | 53 | // not yet acked by the client) before paint defers rather than composing | ||
| 50 | // another diff. It bounds the daemon's paint rate to the client's ack rate, so | 54 | // another diff. It bounds the daemon's paint rate to the client's ack rate, so | ||
| @@ -835,12 +839,30 @@ func (d *Daemon) shutdownAllWithSnapshotDeadline(reason uint8, deadline *snapsho | |||||
| 835 | // Candidate construction owns dial and handshake I/O outside daemon locks. | 839 | // Candidate construction owns dial and handshake I/O outside daemon locks. | ||
| 836 | // A shutdown deadline bounds a non-cooperative dialer; a late constructor | 840 | // A shutdown deadline bounds a non-cooperative dialer; a late constructor | ||
| 837 | // observes d.closing and retires its unpublished candidate independently. | 841 | // observes d.closing and retires its unpublished candidate independently. | ||
| 842 | + | var constructionTimer ports.Timer | |||
| 843 | + | var constructionTimeout <-chan time.Time | |||
| 844 | + | if deadline == nil && len(constructions) != 0 { | |||
| 845 | + | clock := d.clock | |||
| 846 | + | if clock == nil { | |||
| 847 | + | clock = systemClock{} | |||
| 848 | + | } | |||
| 849 | + | constructionTimer = clock.NewTimer(remoteConstructionShutdownGrace) | |||
| 850 | + | if constructionTimer == nil || constructionTimer.C() == nil { | |||
| 851 | + | constructionTimer = systemClock{}.NewTimer(remoteConstructionShutdownGrace) | |||
| 852 | + | } | |||
| 853 | + | constructionTimeout = constructionTimer.C() | |||
| 854 | + | defer constructionTimer.Stop() | |||
| 855 | + | } | |||
| 838 | for _, construction := range constructions { | 856 | for _, construction := range constructions { | ||
| 839 | if construction == nil || construction.done == nil { | 857 | if construction == nil || construction.done == nil { | ||
| 840 | continue | 858 | continue | ||
| 841 | } | 859 | } | ||
| 842 | if deadline == nil { | 860 | if deadline == nil { | ||
| 843 | - | <-construction.done | 861 | + | select { |
| 862 | + | case <-construction.done: | |||
| 863 | + | case <-constructionTimeout: | |||
| 864 | + | checkpointIncomplete = true | |||
| 865 | + | } | |||
| 844 | continue | 866 | continue | ||
| 845 | } | 867 | } | ||
| 846 | select { | 868 | select { | ||
M internal/usecase/daemon/input.go +6 -2
| @@ -66,8 +66,12 @@ func (h remoteViewKeyHandler) Forward(data []byte) { | |||
| 66 | 66 | h.d.handleRemoteViewInput(h.view, h.token, h.inputSeq, data) | |
| 67 | 67 | } | |
| 68 | 68 | ||
| 69 | - | func (h remoteViewKeyHandler) Action(action keys.Action, _ []byte) { | |
| 70 | - | if action != keys.ActionOpenPalette || !h.token.attachmentEffectCurrent() { | |
| 69 | + | func (h remoteViewKeyHandler) Action(action keys.Action, raw []byte) { | |
| 70 | + | if !h.token.attachmentEffectCurrent() { | |
| 71 | + | return | |
| 72 | + | } | |
| 73 | + | if action != keys.ActionOpenPalette { | |
| 74 | + | h.Forward(raw) | |
| 71 | 75 | return | |
| 72 | 76 | } | |
| 73 | 77 | if source := h.ac.navigationSession(); source != nil { | |
| @@ -66,8 +66,12 @@ func (h remoteViewKeyHandler) Forward(data []byte) { | |||||
| 66 | h.d.handleRemoteViewInput(h.view, h.token, h.inputSeq, data) | 66 | h.d.handleRemoteViewInput(h.view, h.token, h.inputSeq, data) | ||
| 67 | } | 67 | } | ||
| 68 | 68 | ||||
| 69 | - | func (h remoteViewKeyHandler) Action(action keys.Action, _ []byte) { | 69 | + | func (h remoteViewKeyHandler) Action(action keys.Action, raw []byte) { |
| 70 | - | if action != keys.ActionOpenPalette || !h.token.attachmentEffectCurrent() { | 70 | + | if !h.token.attachmentEffectCurrent() { |
| 71 | + | return | |||
| 72 | + | } | |||
| 73 | + | if action != keys.ActionOpenPalette { | |||
| 74 | + | h.Forward(raw) | |||
| 71 | return | 75 | return | ||
| 72 | } | 76 | } | ||
| 73 | if source := h.ac.navigationSession(); source != nil { | 77 | if source := h.ac.navigationSession(); source != nil { | ||
M internal/usecase/daemon/remote_attachment_transition.go +2 -1
| @@ -137,11 +137,12 @@ func (d *Daemon) transitionRemoteViewToRemoteViewGuarded(token attachmentConnect | |||
| 137 | 137 | ||
| 138 | 138 | if source == target { | |
| 139 | 139 | source.mu.Lock() | |
| 140 | + | _, attached := source.attachments[token.ac] | |
| 140 | 141 | current := sameAttachmentOwner(token.owner, source) && | |
| 141 | 142 | token.generation == token.ac.connectionGeneration.Load() && | |
| 142 | 143 | sameAttachmentOwner(token.ac.currentAttachmentOwner(), source) && | |
| 143 | 144 | token.ac.transportSnapshotCurrent(token.transport) && | |
| 144 | - | attachmentOwnerRegistered(source, token.ac) && !source.closed && | |
| 145 | + | attached && !source.closed && | |
| 145 | 146 | expectedLink != nil && source.link == expectedLink && | |
| 146 | 147 | source.linkGeneration == expectedLinkGeneration && remoteViewLinkReusableLocked(source) | |
| 147 | 148 | source.mu.Unlock() | |
| @@ -137,11 +137,12 @@ func (d *Daemon) transitionRemoteViewToRemoteViewGuarded(token attachmentConnect | |||||
| 137 | 137 | ||||
| 138 | if source == target { | 138 | if source == target { | ||
| 139 | source.mu.Lock() | 139 | source.mu.Lock() | ||
| 140 | + | _, attached := source.attachments[token.ac] | |||
| 140 | current := sameAttachmentOwner(token.owner, source) && | 141 | current := sameAttachmentOwner(token.owner, source) && | ||
| 141 | token.generation == token.ac.connectionGeneration.Load() && | 142 | token.generation == token.ac.connectionGeneration.Load() && | ||
| 142 | sameAttachmentOwner(token.ac.currentAttachmentOwner(), source) && | 143 | sameAttachmentOwner(token.ac.currentAttachmentOwner(), source) && | ||
| 143 | token.ac.transportSnapshotCurrent(token.transport) && | 144 | token.ac.transportSnapshotCurrent(token.transport) && | ||
| 144 | - | attachmentOwnerRegistered(source, token.ac) && !source.closed && | 145 | + | attached && !source.closed && |
| 145 | expectedLink != nil && source.link == expectedLink && | 146 | expectedLink != nil && source.link == expectedLink && | ||
| 146 | source.linkGeneration == expectedLinkGeneration && remoteViewLinkReusableLocked(source) | 147 | source.linkGeneration == expectedLinkGeneration && remoteViewLinkReusableLocked(source) | ||
| 147 | source.mu.Unlock() | 148 | source.mu.Unlock() | ||
M internal/usecase/daemon/remote_attachment_transition_picker_test.go +23 -0
| @@ -9,6 +9,29 @@ import ( | |||
| 9 | 9 | "github.com/bnema/vev/internal/usecase/picker" | |
| 10 | 10 | ) | |
| 11 | 11 | ||
| 12 | + | func TestPickerTransitionReusesCurrentRemoteView(t *testing.T) { | |
| 13 | + | d, view, _, remoteTransport := newRemoteMetadataLinkFixture(t) | |
| 14 | + | t.Cleanup(func() { require.NoError(t, remoteTransport.Close()) }) | |
| 15 | + | ac, _ := attachRemoteMetadataClient(t, view) | |
| 16 | + | model := picker.New(nil, picker.SelectionConfig{}) | |
| 17 | + | token := attachmentOwnerToken(view, ac, ac.transport()) | |
| 18 | + | ac.overlays.pickerMu.Lock() | |
| 19 | + | ac.overlays.picker = model | |
| 20 | + | ac.overlays.pickerGeneration++ | |
| 21 | + | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: token} | |
| 22 | + | ac.overlays.pickerRemoteSelection = selection | |
| 23 | + | ac.overlays.pickerMu.Unlock() | |
| 24 | + | ||
| 25 | + | result := make(chan error, 1) | |
| 26 | + | go func() { | |
| 27 | + | _, err := d.transitionRemoteViewToRemoteViewForPicker(token, view, selection) | |
| 28 | + | result <- err | |
| 29 | + | }() | |
| 30 | + | require.NoError(t, awaitTestValue(t, result, "reselecting the current remote view deadlocked")) | |
| 31 | + | require.Same(t, view, ac.currentAttachmentOwner()) | |
| 32 | + | require.True(t, view.attachmentRegistered(ac)) | |
| 33 | + | } | |
| 34 | + | ||
| 12 | 35 | func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | |
| 13 | 36 | d, source, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 14 | 37 | view := &remoteView{key: remoteViewKey{ | |
| @@ -9,6 +9,29 @@ import ( | |||||
| 9 | "github.com/bnema/vev/internal/usecase/picker" | 9 | "github.com/bnema/vev/internal/usecase/picker" | ||
| 10 | ) | 10 | ) | ||
| 11 | 11 | ||||
| 12 | + | func TestPickerTransitionReusesCurrentRemoteView(t *testing.T) { | |||
| 13 | + | d, view, _, remoteTransport := newRemoteMetadataLinkFixture(t) | |||
| 14 | + | t.Cleanup(func() { require.NoError(t, remoteTransport.Close()) }) | |||
| 15 | + | ac, _ := attachRemoteMetadataClient(t, view) | |||
| 16 | + | model := picker.New(nil, picker.SelectionConfig{}) | |||
| 17 | + | token := attachmentOwnerToken(view, ac, ac.transport()) | |||
| 18 | + | ac.overlays.pickerMu.Lock() | |||
| 19 | + | ac.overlays.picker = model | |||
| 20 | + | ac.overlays.pickerGeneration++ | |||
| 21 | + | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: token} | |||
| 22 | + | ac.overlays.pickerRemoteSelection = selection | |||
| 23 | + | ac.overlays.pickerMu.Unlock() | |||
| 24 | + | ||||
| 25 | + | result := make(chan error, 1) | |||
| 26 | + | go func() { | |||
| 27 | + | _, err := d.transitionRemoteViewToRemoteViewForPicker(token, view, selection) | |||
| 28 | + | result <- err | |||
| 29 | + | }() | |||
| 30 | + | require.NoError(t, awaitTestValue(t, result, "reselecting the current remote view deadlocked")) | |||
| 31 | + | require.Same(t, view, ac.currentAttachmentOwner()) | |||
| 32 | + | require.True(t, view.attachmentRegistered(ac)) | |||
| 33 | + | } | |||
| 34 | + | ||||
| 12 | func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | 35 | func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | ||
| 13 | d, source, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | 36 | d, source, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | ||
| 14 | view := &remoteView{key: remoteViewKey{ | 37 | view := &remoteView{key: remoteViewKey{ | ||
M internal/usecase/daemon/remote_frame_routing_test.go +16 -0
| @@ -114,6 +114,22 @@ func TestRemoteFrameRoutingForwardsOrdinaryTerminalInput(t *testing.T) { | |||
| 114 | 114 | require.Equal(t, []byte("x"), input.Data) | |
| 115 | 115 | } | |
| 116 | 116 | ||
| 117 | + | func TestRemoteFrameRoutingForwardsNonPaletteActions(t *testing.T) { | |
| 118 | + | d, _, _, view, token := remoteBackFixture(t) | |
| 119 | + | remoteTransport := newRemoteLinkTestTransport() | |
| 120 | + | view.mu.Lock() | |
| 121 | + | view.linkGeneration = 1 | |
| 122 | + | view.link = &remoteLink{view: view, generation: 1, transport: remoteTransport, active: true} | |
| 123 | + | view.mu.Unlock() | |
| 124 | + | ||
| 125 | + | d.handleAttachmentClientFrame(token, frameInput([]byte{keys.ESC, '1'})) | |
| 126 | + | ||
| 127 | + | frame := awaitFrame(t, remoteTransport.sent, ports.MsgInput) | |
| 128 | + | input, err := ports.UnmarshalInput(frame.Payload) | |
| 129 | + | require.NoError(t, err) | |
| 130 | + | require.Equal(t, []byte{keys.ESC, '1'}, input.Data) | |
| 131 | + | } | |
| 132 | + | ||
| 117 | 133 | func TestRemoteFrameRoutingOpensPaletteLocally(t *testing.T) { | |
| 118 | 134 | d, _, ac, view, token := remoteBackFixture(t) | |
| 119 | 135 | remoteTransport := newRemoteLinkTestTransport() | |
| @@ -114,6 +114,22 @@ func TestRemoteFrameRoutingForwardsOrdinaryTerminalInput(t *testing.T) { | |||||
| 114 | require.Equal(t, []byte("x"), input.Data) | 114 | require.Equal(t, []byte("x"), input.Data) | ||
| 115 | } | 115 | } | ||
| 116 | 116 | ||||
| 117 | + | func TestRemoteFrameRoutingForwardsNonPaletteActions(t *testing.T) { | |||
| 118 | + | d, _, _, view, token := remoteBackFixture(t) | |||
| 119 | + | remoteTransport := newRemoteLinkTestTransport() | |||
| 120 | + | view.mu.Lock() | |||
| 121 | + | view.linkGeneration = 1 | |||
| 122 | + | view.link = &remoteLink{view: view, generation: 1, transport: remoteTransport, active: true} | |||
| 123 | + | view.mu.Unlock() | |||
| 124 | + | ||||
| 125 | + | d.handleAttachmentClientFrame(token, frameInput([]byte{keys.ESC, '1'})) | |||
| 126 | + | ||||
| 127 | + | frame := awaitFrame(t, remoteTransport.sent, ports.MsgInput) | |||
| 128 | + | input, err := ports.UnmarshalInput(frame.Payload) | |||
| 129 | + | require.NoError(t, err) | |||
| 130 | + | require.Equal(t, []byte{keys.ESC, '1'}, input.Data) | |||
| 131 | + | } | |||
| 132 | + | ||||
| 117 | func TestRemoteFrameRoutingOpensPaletteLocally(t *testing.T) { | 133 | func TestRemoteFrameRoutingOpensPaletteLocally(t *testing.T) { | ||
| 118 | d, _, ac, view, token := remoteBackFixture(t) | 134 | d, _, ac, view, token := remoteBackFixture(t) | ||
| 119 | remoteTransport := newRemoteLinkTestTransport() | 135 | remoteTransport := newRemoteLinkTestTransport() | ||
M internal/usecase/daemon/remote_link.go +19 -7
| @@ -862,7 +862,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||
| 862 | 862 | case <-timer.C(): | |
| 863 | 863 | // Transport.Close is required to interrupt Send. The sending goroutine | |
| 864 | 864 | // owns sendMu and releases it once the adapter observes that close. | |
| 865 | - | _ = transport.Close() | |
| 865 | + | link.failSend(view, transport, errSendTimedOut) | |
| 866 | 866 | return errSendTimedOut | |
| 867 | 867 | } | |
| 868 | 868 | } | |
| @@ -936,8 +936,12 @@ func (d *Daemon) markRemoteLinkUnavailable(link *remoteLink) { | |||
| 936 | 936 | if !current { | |
| 937 | 937 | return | |
| 938 | 938 | } | |
| 939 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 940 | - | link.cancel() | |
| 939 | + | if link.commands != nil { | |
| 940 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 941 | + | } | |
| 942 | + | if link.cancel != nil { | |
| 943 | + | link.cancel() | |
| 944 | + | } | |
| 941 | 945 | if link.transport != nil { | |
| 942 | 946 | _ = link.transport.Close() | |
| 943 | 947 | } | |
| @@ -962,8 +966,12 @@ func (d *Daemon) stopUnpublishedRemoteView(view *remoteView) { | |||
| 962 | 966 | signalRemoteViewMetadataChangedLocked(view) | |
| 963 | 967 | view.mu.Unlock() | |
| 964 | 968 | if link != nil { | |
| 965 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 966 | - | link.cancel() | |
| 969 | + | if link.commands != nil { | |
| 970 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 971 | + | } | |
| 972 | + | if link.cancel != nil { | |
| 973 | + | link.cancel() | |
| 974 | + | } | |
| 967 | 975 | if link.transport != nil { | |
| 968 | 976 | _ = link.transport.Close() | |
| 969 | 977 | } | |
| @@ -988,8 +996,12 @@ func (d *Daemon) stopRemoteViewLink(view *remoteView) *remoteLink { | |||
| 988 | 996 | } | |
| 989 | 997 | view.mu.Unlock() | |
| 990 | 998 | if link != nil { | |
| 991 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 992 | - | link.cancel() | |
| 999 | + | if link.commands != nil { | |
| 1000 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 1001 | + | } | |
| 1002 | + | if link.cancel != nil { | |
| 1003 | + | link.cancel() | |
| 1004 | + | } | |
| 993 | 1005 | if link.transport != nil { | |
| 994 | 1006 | _ = link.transport.Close() | |
| 995 | 1007 | } | |
| @@ -862,7 +862,7 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||||
| 862 | case <-timer.C(): | 862 | case <-timer.C(): | ||
| 863 | // Transport.Close is required to interrupt Send. The sending goroutine | 863 | // Transport.Close is required to interrupt Send. The sending goroutine | ||
| 864 | // owns sendMu and releases it once the adapter observes that close. | 864 | // owns sendMu and releases it once the adapter observes that close. | ||
| 865 | - | _ = transport.Close() | 865 | + | link.failSend(view, transport, errSendTimedOut) |
| 866 | return errSendTimedOut | 866 | return errSendTimedOut | ||
| 867 | } | 867 | } | ||
| 868 | } | 868 | } | ||
| @@ -936,8 +936,12 @@ func (d *Daemon) markRemoteLinkUnavailable(link *remoteLink) { | |||||
| 936 | if !current { | 936 | if !current { | ||
| 937 | return | 937 | return | ||
| 938 | } | 938 | } | ||
| 939 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 939 | + | if link.commands != nil { |
| 940 | - | link.cancel() | 940 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) |
| 941 | + | } | |||
| 942 | + | if link.cancel != nil { | |||
| 943 | + | link.cancel() | |||
| 944 | + | } | |||
| 941 | if link.transport != nil { | 945 | if link.transport != nil { | ||
| 942 | _ = link.transport.Close() | 946 | _ = link.transport.Close() | ||
| 943 | } | 947 | } | ||
| @@ -962,8 +966,12 @@ func (d *Daemon) stopUnpublishedRemoteView(view *remoteView) { | |||||
| 962 | signalRemoteViewMetadataChangedLocked(view) | 966 | signalRemoteViewMetadataChangedLocked(view) | ||
| 963 | view.mu.Unlock() | 967 | view.mu.Unlock() | ||
| 964 | if link != nil { | 968 | if link != nil { | ||
| 965 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 969 | + | if link.commands != nil { |
| 966 | - | link.cancel() | 970 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) |
| 971 | + | } | |||
| 972 | + | if link.cancel != nil { | |||
| 973 | + | link.cancel() | |||
| 974 | + | } | |||
| 967 | if link.transport != nil { | 975 | if link.transport != nil { | ||
| 968 | _ = link.transport.Close() | 976 | _ = link.transport.Close() | ||
| 969 | } | 977 | } | ||
| @@ -988,8 +996,12 @@ func (d *Daemon) stopRemoteViewLink(view *remoteView) *remoteLink { | |||||
| 988 | } | 996 | } | ||
| 989 | view.mu.Unlock() | 997 | view.mu.Unlock() | ||
| 990 | if link != nil { | 998 | if link != nil { | ||
| 991 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 999 | + | if link.commands != nil { |
| 992 | - | link.cancel() | 1000 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) |
| 1001 | + | } | |||
| 1002 | + | if link.cancel != nil { | |||
| 1003 | + | link.cancel() | |||
| 1004 | + | } | |||
| 993 | if link.transport != nil { | 1005 | if link.transport != nil { | ||
| 994 | _ = link.transport.Close() | 1006 | _ = link.transport.Close() | ||
| 995 | } | 1007 | } | ||
M internal/usecase/daemon/remote_view_tabs.go +4 -1
| @@ -59,6 +59,9 @@ func (d *Daemon) activateRemoteViewTab(ctx context.Context, view *remoteView, ta | |||
| 59 | 59 | if err != nil { | |
| 60 | 60 | return err | |
| 61 | 61 | } | |
| 62 | + | if slug == "" { | |
| 63 | + | return nil | |
| 64 | + | } | |
| 62 | 65 | if err := d.sendRemoteViewCommand(ctx, link, generation, slug); err != nil { | |
| 63 | 66 | return err | |
| 64 | 67 | } | |
| @@ -127,7 +130,7 @@ func remoteViewRelativeTabCommand(view *remoteView, link *remoteLink, generation | |||
| 127 | 130 | forward := (selectedIndex - activeIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | |
| 128 | 131 | backward := (activeIndex - selectedIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | |
| 129 | 132 | if forward == 0 || backward == 0 { | |
| 130 | - | return "", errRemoteViewTabUnavailable | |
| 133 | + | return "", nil | |
| 131 | 134 | } | |
| 132 | 135 | if forward <= backward { | |
| 133 | 136 | return "next-tab", nil | |
| @@ -59,6 +59,9 @@ func (d *Daemon) activateRemoteViewTab(ctx context.Context, view *remoteView, ta | |||||
| 59 | if err != nil { | 59 | if err != nil { | ||
| 60 | return err | 60 | return err | ||
| 61 | } | 61 | } | ||
| 62 | + | if slug == "" { | |||
| 63 | + | return nil | |||
| 64 | + | } | |||
| 62 | if err := d.sendRemoteViewCommand(ctx, link, generation, slug); err != nil { | 65 | if err := d.sendRemoteViewCommand(ctx, link, generation, slug); err != nil { | ||
| 63 | return err | 66 | return err | ||
| 64 | } | 67 | } | ||
| @@ -127,7 +130,7 @@ func remoteViewRelativeTabCommand(view *remoteView, link *remoteLink, generation | |||||
| 127 | forward := (selectedIndex - activeIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | 130 | forward := (selectedIndex - activeIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | ||
| 128 | backward := (activeIndex - selectedIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | 131 | backward := (activeIndex - selectedIndex + len(view.metadata.Tabs)) % len(view.metadata.Tabs) | ||
| 129 | if forward == 0 || backward == 0 { | 132 | if forward == 0 || backward == 0 { | ||
| 130 | - | return "", errRemoteViewTabUnavailable | 133 | + | return "", nil |
| 131 | } | 134 | } | ||
| 132 | if forward <= backward { | 135 | if forward <= backward { | ||
| 133 | return "next-tab", nil | 136 | return "next-tab", nil | ||
M internal/usecase/daemon/remote_warm.go +8 -3
| @@ -36,10 +36,15 @@ func (d *Daemon) parkRemoteViewWarm(view *remoteView) { | |||
| 36 | 36 | if d == nil || view == nil { | |
| 37 | 37 | return | |
| 38 | 38 | } | |
| 39 | - | warm := &remoteViewWarm{ | |
| 40 | - | timer: d.clock.NewTimer(remoteViewWarmTTL), | |
| 41 | - | done: make(chan struct{}), | |
| 39 | + | clock := d.clock | |
| 40 | + | if clock == nil { | |
| 41 | + | clock = systemClock{} | |
| 42 | 42 | } | |
| 43 | + | timer := clock.NewTimer(remoteViewWarmTTL) | |
| 44 | + | if timer == nil { | |
| 45 | + | return | |
| 46 | + | } | |
| 47 | + | warm := &remoteViewWarm{timer: timer, done: make(chan struct{})} | |
| 43 | 48 | ||
| 44 | 49 | d.mu.Lock() | |
| 45 | 50 | if d.closing || !d.attachmentOwnerRegisteredByDaemonLocked(view) { | |
| @@ -36,10 +36,15 @@ func (d *Daemon) parkRemoteViewWarm(view *remoteView) { | |||||
| 36 | if d == nil || view == nil { | 36 | if d == nil || view == nil { | ||
| 37 | return | 37 | return | ||
| 38 | } | 38 | } | ||
| 39 | - | warm := &remoteViewWarm{ | 39 | + | clock := d.clock |
| 40 | - | timer: d.clock.NewTimer(remoteViewWarmTTL), | 40 | + | if clock == nil { |
| 41 | - | done: make(chan struct{}), | 41 | + | clock = systemClock{} |
| 42 | } | 42 | } | ||
| 43 | + | timer := clock.NewTimer(remoteViewWarmTTL) | |||
| 44 | + | if timer == nil { | |||
| 45 | + | return | |||
| 46 | + | } | |||
| 47 | + | warm := &remoteViewWarm{timer: timer, done: make(chan struct{})} | |||
| 43 | 48 | ||||
| 44 | d.mu.Lock() | 49 | d.mu.Lock() | ||
| 45 | if d.closing || !d.attachmentOwnerRegisteredByDaemonLocked(view) { | 50 | if d.closing || !d.attachmentOwnerRegisteredByDaemonLocked(view) { | ||
M internal/usecase/picker/model.go +8 -2
| @@ -629,7 +629,11 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||
| 629 | 629 | if !r.kind.rendersAsHeader() { | |
| 630 | 630 | name = " " + name | |
| 631 | 631 | } | |
| 632 | - | name = ui.TruncateText(name, rect.Width) | |
| 632 | + | suffix := "" | |
| 633 | + | if r.kind.rendersAsHeader() && r.stopped { | |
| 634 | + | suffix = "(down)" | |
| 635 | + | } | |
| 636 | + | name = ui.TruncateText(name, max(rect.Width-len(suffix), 0)) | |
| 633 | 637 | x := ui.DrawText(frame, rect.X, rect.Y+y, clipX, name, nameStyle) | |
| 634 | 638 | ||
| 635 | 639 | if r.kind.rendersAsHeader() { | |
| @@ -640,7 +644,9 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||
| 640 | 644 | } else { | |
| 641 | 645 | detail += " " | |
| 642 | 646 | } | |
| 643 | - | ui.DrawText(frame, x, rect.Y+y, clipX, detail+"(down)", base) | |
| 647 | + | detail = ui.TruncateText(detail, max(clipX-x-len(suffix), 0)) | |
| 648 | + | x = ui.DrawText(frame, x, rect.Y+y, clipX, detail, base) | |
| 649 | + | ui.DrawText(frame, x, rect.Y+y, clipX, suffix, base) | |
| 644 | 650 | } else { | |
| 645 | 651 | ui.DrawText(frame, x, rect.Y+y, clipX, ui.TruncateText(r.detail, clipX-x), detailStyle) | |
| 646 | 652 | } | |
| @@ -629,7 +629,11 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||||
| 629 | if !r.kind.rendersAsHeader() { | 629 | if !r.kind.rendersAsHeader() { | ||
| 630 | name = " " + name | 630 | name = " " + name | ||
| 631 | } | 631 | } | ||
| 632 | - | name = ui.TruncateText(name, rect.Width) | 632 | + | suffix := "" |
| 633 | + | if r.kind.rendersAsHeader() && r.stopped { | |||
| 634 | + | suffix = "(down)" | |||
| 635 | + | } | |||
| 636 | + | name = ui.TruncateText(name, max(rect.Width-len(suffix), 0)) | |||
| 633 | x := ui.DrawText(frame, rect.X, rect.Y+y, clipX, name, nameStyle) | 637 | x := ui.DrawText(frame, rect.X, rect.Y+y, clipX, name, nameStyle) | ||
| 634 | 638 | ||||
| 635 | if r.kind.rendersAsHeader() { | 639 | if r.kind.rendersAsHeader() { | ||
| @@ -640,7 +644,9 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||||
| 640 | } else { | 644 | } else { | ||
| 641 | detail += " " | 645 | detail += " " | ||
| 642 | } | 646 | } | ||
| 643 | - | ui.DrawText(frame, x, rect.Y+y, clipX, detail+"(down)", base) | 647 | + | detail = ui.TruncateText(detail, max(clipX-x-len(suffix), 0)) |
| 648 | + | x = ui.DrawText(frame, x, rect.Y+y, clipX, detail, base) | |||
| 649 | + | ui.DrawText(frame, x, rect.Y+y, clipX, suffix, base) | |||
| 644 | } else { | 650 | } else { | ||
| 645 | ui.DrawText(frame, x, rect.Y+y, clipX, ui.TruncateText(r.detail, clipX-x), detailStyle) | 651 | ui.DrawText(frame, x, rect.Y+y, clipX, ui.TruncateText(r.detail, clipX-x), detailStyle) | ||
| 646 | } | 652 | } | ||
M internal/usecase/picker/model_test.go +11 -0
| @@ -506,6 +506,17 @@ func TestStoppedSessionSelectableAndRendered(t *testing.T) { | |||
| 506 | 506 | require.Contains(t, label.String(), "(down)") | |
| 507 | 507 | } | |
| 508 | 508 | ||
| 509 | + | func TestStoppedSessionReservesDownSuffix(t *testing.T) { | |
| 510 | + | m := New([]SessionView{{ID: "stopped:long", Name: "long-session", Tabs: []TabEntry{{}}, Stopped: true}}, SelectionConfig{Mode: SelectNavigationTab}) | |
| 511 | + | ||
| 512 | + | frame := m.Render(domain.Size{Cols: 8, Rows: 2}, Preview{}) | |
| 513 | + | var label strings.Builder | |
| 514 | + | for _, cell := range frame.Row(0) { | |
| 515 | + | label.WriteRune(cell.Rune) | |
| 516 | + | } | |
| 517 | + | require.Contains(t, label.String(), "(down)") | |
| 518 | + | } | |
| 519 | + | ||
| 509 | 520 | func TestRenderStopsStoppedRowsDimItalic(t *testing.T) { | |
| 510 | 521 | live := SessionView{ID: "live", Name: "work", Tabs: []TabEntry{{TabID: "t1", Name: "tab"}}} | |
| 511 | 522 | halted := SessionView{ID: "stopped:old", Name: "old", TargetName: "old", Stopped: true, Tabs: []TabEntry{{}}} | |
| @@ -506,6 +506,17 @@ func TestStoppedSessionSelectableAndRendered(t *testing.T) { | |||||
| 506 | require.Contains(t, label.String(), "(down)") | 506 | require.Contains(t, label.String(), "(down)") | ||
| 507 | } | 507 | } | ||
| 508 | 508 | ||||
| 509 | + | func TestStoppedSessionReservesDownSuffix(t *testing.T) { | |||
| 510 | + | m := New([]SessionView{{ID: "stopped:long", Name: "long-session", Tabs: []TabEntry{{}}, Stopped: true}}, SelectionConfig{Mode: SelectNavigationTab}) | |||
| 511 | + | ||||
| 512 | + | frame := m.Render(domain.Size{Cols: 8, Rows: 2}, Preview{}) | |||
| 513 | + | var label strings.Builder | |||
| 514 | + | for _, cell := range frame.Row(0) { | |||
| 515 | + | label.WriteRune(cell.Rune) | |||
| 516 | + | } | |||
| 517 | + | require.Contains(t, label.String(), "(down)") | |||
| 518 | + | } | |||
| 519 | + | ||||
| 509 | func TestRenderStopsStoppedRowsDimItalic(t *testing.T) { | 520 | func TestRenderStopsStoppedRowsDimItalic(t *testing.T) { | ||
| 510 | live := SessionView{ID: "live", Name: "work", Tabs: []TabEntry{{TabID: "t1", Name: "tab"}}} | 521 | live := SessionView{ID: "live", Name: "work", Tabs: []TabEntry{{TabID: "t1", Name: "tab"}}} | ||
| 511 | halted := SessionView{ID: "stopped:old", Name: "old", TargetName: "old", Stopped: true, Tabs: []TabEntry{{}}} | 522 | halted := SessionView{ID: "stopped:old", Name: "old", TargetName: "old", Stopped: true, Tabs: []TabEntry{{}}} | ||