WIP: feat(client): switch same-daemon sessions in place and quiet the connecting notice (#318)
* feat(client): switch same-daemon sessions in place and delay the connecting notice The client picker and recent-route navigation now move a live attachment to another session on the same daemon with SamePeerSwitchRequest instead of detaching and sending a new Hello. A refused switch falls back to the reconnect path. The daemon accepts a client-initiated request when no offer is pending and still restricts a pending offer to its exact target. The Connecting to session notice now appears only after 200 ms, so fast switches no longer flash it. * fix(client): harden in-place session switches and the connecting notice Number each in-place switch and report arrived, refused, or superseded, so a newer daemon offer is never overridden by a reconnect to an older choice. Try in place only for sessions the catalogue shows running, skip the source repaint before the switch, and let a pending switch be replaced by picking the original session again. Share the request path between picker and daemon-offered switches. Remote attaches and resumes show the connecting notice at once, because nothing can paint it once the attachment owns the terminal. The delayed local notice now ticks exactly when it becomes due. * fix(client): repaint a kept source and settle repeated in-place picks When a daemon offer replaces a picker in-place switch and the daemon then keeps the source, ask for the repaint the closing picker skipped. Validate a daemon offer before following its UI action. Picking the destination of an in-flight switch again closes the picker instead of starting a second switch. SlowConnecting is now reset outside the Connecting an attachment entered, and restores of stopped local sessions show the notice at once.
- commit
- c7eae950faebccb30bccbc8c587e6011b7aefdb3
- author
- bnema · · 2026-10-07 18:34 UTC
- committer
- GitHub · · 2026-10-07 18:34 UTC
- parent
- 29fb826
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
16 files · +780 -82
M docs/architecture.md +1 -1
| @@ -75,7 +75,7 @@ Interactive observation is a separate opt-in composition. `term.Terminal` remain | |||
| 75 | 75 | ||
| 76 | 76 | A client publishes exactly one of three presentations at a time. `picker` is the local picker: the client is usable and describable without a broker and without a daemon. `connecting` is an attachment attempt that has not committed its first frame. `attached` is a committed attachment, and only it carries the validated session identity, the committed output boundary, and the real actionable generation; `picker` and `connecting` carry the run's stable service handle and their status with every session field zeroed, so a capture can never present session metadata as an attachment and no caller can fence an input action against a generation no attachment committed. The former `detached`, `reconnecting`, and `transitioning` values are removed without alias: a reconnect is `connecting`, a detach returns to `picker`, and terminating closes the UI service instead of becoming a fourth persistent presentation. The presentation is published inside the terminal's UI output transaction by its owner: the supervisor publishes `connecting` before the attachment foreground is granted and `picker` after the run drained and revoked, while the admitted foreground publishes `attached` only after its first frame was written, flushed, and committed. | |
| 77 | 77 | ||
| 78 | - | Session navigation is client-owned. While attached, Alt+Space opens the daemon command palette; `SSP` / `session-picker` opens the client picker as an overlay over the live attachment. Cancel returns to the same attachment without reconnecting. Committing a different destination switches attachments through `connecting`; attachment loss or explicit detach leaves the picker as the standalone presentation. Search, sorting, cursor movement, selection, and broker-backed attachment opening are client-owned. Daemon move-destination offers still use the attached worker's separate move picker. | |
| 78 | + | Session navigation is client-owned. While attached, Alt+Space opens the daemon command palette; `SSP` / `session-picker` opens the client picker as an overlay over the live attachment. Cancel returns to the same attachment without reconnecting. Committing another live session on the daemon already serving the attachment moves that attachment in place with `SamePeerSwitchRequest`, without a new stream or `Hello`; any other destination, or a refused in-place switch, switches attachments through `connecting`; attachment loss or explicit detach leaves the picker as the standalone presentation. Search, sorting, cursor movement, selection, and broker-backed attachment opening are client-owned. Daemon move-destination offers still use the attached worker's separate move picker. | |
| 79 | 79 | ||
| 80 | 80 | The selected row owns one client-scoped live preview subscription through `ports.BrokerNavigator`. The broker opens one long-lived observation stream over its local or remote physical pool; the daemon pushes changed `RemotePreview` frames at a broker-selected interval (33 ms local, 125 ms remote). Cursor changes are debounced for 80 ms, and the client retains eight recent previews so a selection can remain visible while refreshing. Broker loss, commit, picker close, and client exit cancel the watch. The client checks epoch, connection, generation, target, and dimensions before painting; previews remain bounded, memory-only, and are never persisted. | |
| 81 | 81 | ||
| @@ -75,7 +75,7 @@ Interactive observation is a separate opt-in composition. `term.Terminal` remain | |||||
| 75 | 75 | ||||
| 76 | A client publishes exactly one of three presentations at a time. `picker` is the local picker: the client is usable and describable without a broker and without a daemon. `connecting` is an attachment attempt that has not committed its first frame. `attached` is a committed attachment, and only it carries the validated session identity, the committed output boundary, and the real actionable generation; `picker` and `connecting` carry the run's stable service handle and their status with every session field zeroed, so a capture can never present session metadata as an attachment and no caller can fence an input action against a generation no attachment committed. The former `detached`, `reconnecting`, and `transitioning` values are removed without alias: a reconnect is `connecting`, a detach returns to `picker`, and terminating closes the UI service instead of becoming a fourth persistent presentation. The presentation is published inside the terminal's UI output transaction by its owner: the supervisor publishes `connecting` before the attachment foreground is granted and `picker` after the run drained and revoked, while the admitted foreground publishes `attached` only after its first frame was written, flushed, and committed. | 76 | A client publishes exactly one of three presentations at a time. `picker` is the local picker: the client is usable and describable without a broker and without a daemon. `connecting` is an attachment attempt that has not committed its first frame. `attached` is a committed attachment, and only it carries the validated session identity, the committed output boundary, and the real actionable generation; `picker` and `connecting` carry the run's stable service handle and their status with every session field zeroed, so a capture can never present session metadata as an attachment and no caller can fence an input action against a generation no attachment committed. The former `detached`, `reconnecting`, and `transitioning` values are removed without alias: a reconnect is `connecting`, a detach returns to `picker`, and terminating closes the UI service instead of becoming a fourth persistent presentation. The presentation is published inside the terminal's UI output transaction by its owner: the supervisor publishes `connecting` before the attachment foreground is granted and `picker` after the run drained and revoked, while the admitted foreground publishes `attached` only after its first frame was written, flushed, and committed. | ||
| 77 | 77 | ||||
| 78 | - | Session navigation is client-owned. While attached, Alt+Space opens the daemon command palette; `SSP` / `session-picker` opens the client picker as an overlay over the live attachment. Cancel returns to the same attachment without reconnecting. Committing a different destination switches attachments through `connecting`; attachment loss or explicit detach leaves the picker as the standalone presentation. Search, sorting, cursor movement, selection, and broker-backed attachment opening are client-owned. Daemon move-destination offers still use the attached worker's separate move picker. | 78 | + | Session navigation is client-owned. While attached, Alt+Space opens the daemon command palette; `SSP` / `session-picker` opens the client picker as an overlay over the live attachment. Cancel returns to the same attachment without reconnecting. Committing another live session on the daemon already serving the attachment moves that attachment in place with `SamePeerSwitchRequest`, without a new stream or `Hello`; any other destination, or a refused in-place switch, switches attachments through `connecting`; attachment loss or explicit detach leaves the picker as the standalone presentation. Search, sorting, cursor movement, selection, and broker-backed attachment opening are client-owned. Daemon move-destination offers still use the attached worker's separate move picker. |
| 79 | 79 | ||||
| 80 | The selected row owns one client-scoped live preview subscription through `ports.BrokerNavigator`. The broker opens one long-lived observation stream over its local or remote physical pool; the daemon pushes changed `RemotePreview` frames at a broker-selected interval (33 ms local, 125 ms remote). Cursor changes are debounced for 80 ms, and the client retains eight recent previews so a selection can remain visible while refreshing. Broker loss, commit, picker close, and client exit cancel the watch. The client checks epoch, connection, generation, target, and dimensions before painting; previews remain bounded, memory-only, and are never persisted. | 80 | The selected row owns one client-scoped live preview subscription through `ports.BrokerNavigator`. The broker opens one long-lived observation stream over its local or remote physical pool; the daemon pushes changed `RemotePreview` frames at a broker-selected interval (33 ms local, 125 ms remote). Cursor changes are debounced for 80 ms, and the client retains eight recent previews so a selection can remain visible while refreshing. Broker loss, commit, picker close, and client exit cancel the watch. The client checks epoch, connection, generation, target, and dimensions before painting; previews remain bounded, memory-only, and are never persisted. | ||
| 81 | 81 | ||||
M docs/remote-resilience.md +2 -1
| @@ -41,7 +41,7 @@ On remote sessions vev shows what you type before the server answers, like mosh. | |||
| 41 | 41 | | What you see | Meaning | | |
| 42 | 42 | |---|---| | |
| 43 | 43 | | The session | The link works. | | |
| 44 | - | | `Connecting to session…` | The link dropped; vev is reconnecting and keeps your keys. | | |
| 44 | + | | `Connecting to session…` | vev is opening a session or reconnecting a dropped link, and keeps your keys. Switching to a running local session shows it only after 200 ms. | | |
| 45 | 45 | | The picker with an error | The session could not be reached again, or you cancelled. | | |
| 46 | 46 | ||
| 47 | 47 | vev does not yet report a slow (`degraded`) link separately. Connection notices never appear inside your shell output. | |
| @@ -57,6 +57,7 @@ vev host add user@host | |||
| 57 | 57 | Then the session picker (`SSP`) shows local and remote sessions in one list. Remote sessions appear as `session@host`, with tags like `down` or `broken` when they can't be opened. | |
| 58 | 58 | ||
| 59 | 59 | - Selecting a session switches to it, local or remote. `BCK` goes back to the previous one; `JRS` jumps to a recent one. | |
| 60 | + | - Switching between sessions on the same daemon (two local sessions, or two sessions on the same host) reuses the live attachment, so there is no reconnect. | |
| 60 | 61 | - A remote session opened from the picker uses the remote host's environment and saved working directory. | |
| 61 | 62 | - `CNS` asks where to create the new session when several daemons are available. | |
| 62 | 63 | - Session history and the last selected tab in each session are per client and kept only while that client runs. Switching back restores that tab; choosing a specific tab overrides it. If the remembered tab was closed, vev falls back to the first tab. | |
| @@ -41,7 +41,7 @@ On remote sessions vev shows what you type before the server answers, like mosh. | |||||
| 41 | | What you see | Meaning | | 41 | | What you see | Meaning | | ||
| 42 | |---|---| | 42 | |---|---| | ||
| 43 | | The session | The link works. | | 43 | | The session | The link works. | | ||
| 44 | - | | `Connecting to session…` | The link dropped; vev is reconnecting and keeps your keys. | | 44 | + | | `Connecting to session…` | vev is opening a session or reconnecting a dropped link, and keeps your keys. Switching to a running local session shows it only after 200 ms. | |
| 45 | | The picker with an error | The session could not be reached again, or you cancelled. | | 45 | | The picker with an error | The session could not be reached again, or you cancelled. | | ||
| 46 | 46 | ||||
| 47 | vev does not yet report a slow (`degraded`) link separately. Connection notices never appear inside your shell output. | 47 | vev does not yet report a slow (`degraded`) link separately. Connection notices never appear inside your shell output. | ||
| @@ -57,6 +57,7 @@ vev host add user@host | |||||
| 57 | Then the session picker (`SSP`) shows local and remote sessions in one list. Remote sessions appear as `session@host`, with tags like `down` or `broken` when they can't be opened. | 57 | Then the session picker (`SSP`) shows local and remote sessions in one list. Remote sessions appear as `session@host`, with tags like `down` or `broken` when they can't be opened. | ||
| 58 | 58 | ||||
| 59 | - Selecting a session switches to it, local or remote. `BCK` goes back to the previous one; `JRS` jumps to a recent one. | 59 | - Selecting a session switches to it, local or remote. `BCK` goes back to the previous one; `JRS` jumps to a recent one. | ||
| 60 | + | - Switching between sessions on the same daemon (two local sessions, or two sessions on the same host) reuses the live attachment, so there is no reconnect. | |||
| 60 | - A remote session opened from the picker uses the remote host's environment and saved working directory. | 61 | - A remote session opened from the picker uses the remote host's environment and saved working directory. | ||
| 61 | - `CNS` asks where to create the new session when several daemons are available. | 62 | - `CNS` asks where to create the new session when several daemons are available. | ||
| 62 | - Session history and the last selected tab in each session are per client and kept only while that client runs. Switching back restores that tab; choosing a specific tab overrides it. If the remembered tab was closed, vev falls back to the first tab. | 63 | - Session history and the last selected tab in each session are per client and kept only while that client runs. Switching back restores that tab; choosing a specific tab overrides it. If the remembered tab was closed, vev falls back to the first tab. | ||
M internal/app/broker_client.go +44 -3
| @@ -96,10 +96,21 @@ type brokerClientPresentation struct { | |||
| 96 | 96 | clock ports.Clock | |
| 97 | 97 | spinner ports.Timer | |
| 98 | 98 | frame int | |
| 99 | + | // spinning is set while a transition notice is due or shown, so the | |
| 100 | + | // spinner timer exists only then and never repaints the plain picker. | |
| 101 | + | spinning bool | |
| 102 | + | // connectingSince is when the current Connecting presentation began, zero | |
| 103 | + | // outside it. The notice stays hidden for connectingNoticeDelay so a fast | |
| 104 | + | // session switch never flashes it. | |
| 105 | + | connectingSince time.Time | |
| 99 | 106 | ||
| 100 | 107 | mu sync.Mutex | |
| 101 | 108 | } | |
| 102 | 109 | ||
| 110 | + | // connectingNoticeDelay is how long Connecting stays silent before its notice | |
| 111 | + | // is painted. A spinner tick repaints after it elapses. | |
| 112 | + | const connectingNoticeDelay = 200 * time.Millisecond | |
| 113 | + | ||
| 103 | 114 | // Render observes the state and paints the picker frame and notice while the | |
| 104 | 115 | // shared client.PickerPresentation fence admits the presentation. That is | |
| 105 | 116 | // exactly PresentPicker: an admitted attachment foreground owns the same | |
| @@ -124,11 +135,26 @@ func (p *brokerClientPresentation) Render(state client.State) { | |||
| 124 | 135 | if p.onState != nil { | |
| 125 | 136 | p.onState(state) | |
| 126 | 137 | } | |
| 138 | + | p.mu.Lock() | |
| 139 | + | defer p.mu.Unlock() | |
| 140 | + | if state.Presentation != client.PresentConnecting { | |
| 141 | + | p.connectingSince = time.Time{} | |
| 142 | + | } | |
| 127 | 143 | if !client.PickerPresentation(state) && (state.Presentation != client.PresentConnecting || p.clock == nil) { | |
| 128 | 144 | return | |
| 129 | 145 | } | |
| 130 | - | p.mu.Lock() | |
| 131 | - | defer p.mu.Unlock() | |
| 146 | + | if state.Presentation == client.PresentConnecting || state.Connectivity == client.ConnectivityRetryWait { | |
| 147 | + | p.spinning = true | |
| 148 | + | } | |
| 149 | + | if state.Presentation == client.PresentConnecting && state.Connectivity != client.ConnectivityRetryWait && !state.SlowConnecting { | |
| 150 | + | now := p.clock.Now() | |
| 151 | + | if p.connectingSince.IsZero() { | |
| 152 | + | p.connectingSince = now | |
| 153 | + | } | |
| 154 | + | if now.Sub(p.connectingSince) < connectingNoticeDelay { | |
| 155 | + | return | |
| 156 | + | } | |
| 157 | + | } | |
| 132 | 158 | geometry, err := p.terminal.Geometry() | |
| 133 | 159 | if err != nil { | |
| 134 | 160 | return | |
| @@ -191,8 +217,19 @@ func (p *brokerClientPresentation) Render(state client.State) { | |||
| 191 | 217 | func (p *brokerClientPresentation) Spinner() <-chan time.Time { | |
| 192 | 218 | p.mu.Lock() | |
| 193 | 219 | defer p.mu.Unlock() | |
| 220 | + | if !p.spinning || p.clock == nil { | |
| 221 | + | return nil | |
| 222 | + | } | |
| 194 | 223 | if p.spinner == nil { | |
| 195 | - | p.spinner = p.clock.NewTimer(120 * time.Millisecond) | |
| 224 | + | delay := 120 * time.Millisecond | |
| 225 | + | if !p.connectingSince.IsZero() { | |
| 226 | + | // While the notice is held back, the next tick lands exactly when | |
| 227 | + | // it becomes due. | |
| 228 | + | if remaining := connectingNoticeDelay - p.clock.Now().Sub(p.connectingSince); remaining > 0 { | |
| 229 | + | delay = remaining | |
| 230 | + | } | |
| 231 | + | } | |
| 232 | + | p.spinner = p.clock.NewTimer(delay) | |
| 196 | 233 | } | |
| 197 | 234 | return p.spinner.C() | |
| 198 | 235 | } | |
| @@ -209,8 +246,12 @@ func (p *brokerClientPresentation) AdvanceSpinner(state client.State) { | |||
| 209 | 246 | ||
| 210 | 247 | func (p *brokerClientPresentation) StopSpinner() { | |
| 211 | 248 | p.mu.Lock() | |
| 249 | + | p.spinning = false | |
| 212 | 250 | if p.spinner != nil { | |
| 213 | 251 | p.spinner.Stop() | |
| 252 | + | // A stopped timer never fires again: the next transition needs a | |
| 253 | + | // fresh one or its delayed notice would never appear. | |
| 254 | + | p.spinner = nil | |
| 214 | 255 | } | |
| 215 | 256 | p.mu.Unlock() | |
| 216 | 257 | } | |
| @@ -96,10 +96,21 @@ type brokerClientPresentation struct { | |||||
| 96 | clock ports.Clock | 96 | clock ports.Clock | ||
| 97 | spinner ports.Timer | 97 | spinner ports.Timer | ||
| 98 | frame int | 98 | frame int | ||
| 99 | + | // spinning is set while a transition notice is due or shown, so the | |||
| 100 | + | // spinner timer exists only then and never repaints the plain picker. | |||
| 101 | + | spinning bool | |||
| 102 | + | // connectingSince is when the current Connecting presentation began, zero | |||
| 103 | + | // outside it. The notice stays hidden for connectingNoticeDelay so a fast | |||
| 104 | + | // session switch never flashes it. | |||
| 105 | + | connectingSince time.Time | |||
| 99 | 106 | ||||
| 100 | mu sync.Mutex | 107 | mu sync.Mutex | ||
| 101 | } | 108 | } | ||
| 102 | 109 | ||||
| 110 | + | // connectingNoticeDelay is how long Connecting stays silent before its notice | |||
| 111 | + | // is painted. A spinner tick repaints after it elapses. | |||
| 112 | + | const connectingNoticeDelay = 200 * time.Millisecond | |||
| 113 | + | ||||
| 103 | // Render observes the state and paints the picker frame and notice while the | 114 | // Render observes the state and paints the picker frame and notice while the | ||
| 104 | // shared client.PickerPresentation fence admits the presentation. That is | 115 | // shared client.PickerPresentation fence admits the presentation. That is | ||
| 105 | // exactly PresentPicker: an admitted attachment foreground owns the same | 116 | // exactly PresentPicker: an admitted attachment foreground owns the same | ||
| @@ -124,11 +135,26 @@ func (p *brokerClientPresentation) Render(state client.State) { | |||||
| 124 | if p.onState != nil { | 135 | if p.onState != nil { | ||
| 125 | p.onState(state) | 136 | p.onState(state) | ||
| 126 | } | 137 | } | ||
| 138 | + | p.mu.Lock() | |||
| 139 | + | defer p.mu.Unlock() | |||
| 140 | + | if state.Presentation != client.PresentConnecting { | |||
| 141 | + | p.connectingSince = time.Time{} | |||
| 142 | + | } | |||
| 127 | if !client.PickerPresentation(state) && (state.Presentation != client.PresentConnecting || p.clock == nil) { | 143 | if !client.PickerPresentation(state) && (state.Presentation != client.PresentConnecting || p.clock == nil) { | ||
| 128 | return | 144 | return | ||
| 129 | } | 145 | } | ||
| 130 | - | p.mu.Lock() | 146 | + | if state.Presentation == client.PresentConnecting || state.Connectivity == client.ConnectivityRetryWait { |
| 131 | - | defer p.mu.Unlock() | 147 | + | p.spinning = true |
| 148 | + | } | |||
| 149 | + | if state.Presentation == client.PresentConnecting && state.Connectivity != client.ConnectivityRetryWait && !state.SlowConnecting { | |||
| 150 | + | now := p.clock.Now() | |||
| 151 | + | if p.connectingSince.IsZero() { | |||
| 152 | + | p.connectingSince = now | |||
| 153 | + | } | |||
| 154 | + | if now.Sub(p.connectingSince) < connectingNoticeDelay { | |||
| 155 | + | return | |||
| 156 | + | } | |||
| 157 | + | } | |||
| 132 | geometry, err := p.terminal.Geometry() | 158 | geometry, err := p.terminal.Geometry() | ||
| 133 | if err != nil { | 159 | if err != nil { | ||
| 134 | return | 160 | return | ||
| @@ -191,8 +217,19 @@ func (p *brokerClientPresentation) Render(state client.State) { | |||||
| 191 | func (p *brokerClientPresentation) Spinner() <-chan time.Time { | 217 | func (p *brokerClientPresentation) Spinner() <-chan time.Time { | ||
| 192 | p.mu.Lock() | 218 | p.mu.Lock() | ||
| 193 | defer p.mu.Unlock() | 219 | defer p.mu.Unlock() | ||
| 220 | + | if !p.spinning || p.clock == nil { | |||
| 221 | + | return nil | |||
| 222 | + | } | |||
| 194 | if p.spinner == nil { | 223 | if p.spinner == nil { | ||
| 195 | - | p.spinner = p.clock.NewTimer(120 * time.Millisecond) | 224 | + | delay := 120 * time.Millisecond |
| 225 | + | if !p.connectingSince.IsZero() { | |||
| 226 | + | // While the notice is held back, the next tick lands exactly when | |||
| 227 | + | // it becomes due. | |||
| 228 | + | if remaining := connectingNoticeDelay - p.clock.Now().Sub(p.connectingSince); remaining > 0 { | |||
| 229 | + | delay = remaining | |||
| 230 | + | } | |||
| 231 | + | } | |||
| 232 | + | p.spinner = p.clock.NewTimer(delay) | |||
| 196 | } | 233 | } | ||
| 197 | return p.spinner.C() | 234 | return p.spinner.C() | ||
| 198 | } | 235 | } | ||
| @@ -209,8 +246,12 @@ func (p *brokerClientPresentation) AdvanceSpinner(state client.State) { | |||||
| 209 | 246 | ||||
| 210 | func (p *brokerClientPresentation) StopSpinner() { | 247 | func (p *brokerClientPresentation) StopSpinner() { | ||
| 211 | p.mu.Lock() | 248 | p.mu.Lock() | ||
| 249 | + | p.spinning = false | |||
| 212 | if p.spinner != nil { | 250 | if p.spinner != nil { | ||
| 213 | p.spinner.Stop() | 251 | p.spinner.Stop() | ||
| 252 | + | // A stopped timer never fires again: the next transition needs a | |||
| 253 | + | // fresh one or its delayed notice would never appear. | |||
| 254 | + | p.spinner = nil | |||
| 214 | } | 255 | } | ||
| 215 | p.mu.Unlock() | 256 | p.mu.Unlock() | ||
| 216 | } | 257 | } | ||
M internal/app/broker_offline_client_render_test.go +74 -0
| @@ -335,3 +335,77 @@ func TestBrokerClientFailureWritesOnlyNonRetryableErrors(t *testing.T) { | |||
| 335 | 335 | }) | |
| 336 | 336 | } | |
| 337 | 337 | } | |
| 338 | + | ||
| 339 | + | // TestBrokerClientConnectingNoticeIsDelayed pins that a fast switch never | |
| 340 | + | // flashes the Connecting notice: it paints only once Connecting has lasted | |
| 341 | + | // connectingNoticeDelay, and leaving Connecting restarts the delay. | |
| 342 | + | func TestBrokerClientConnectingNoticeIsDelayed(t *testing.T) { | |
| 343 | + | attached := client.State{Presentation: client.PresentAttached, Connectivity: client.ConnectivityReady, Generation: 1} | |
| 344 | + | tests := []struct { | |
| 345 | + | name string | |
| 346 | + | slow bool // a remote attach or a resume | |
| 347 | + | steps []time.Duration // clock advance before each Connecting render | |
| 348 | + | reset bool // leave Connecting before the last render | |
| 349 | + | painted bool | |
| 350 | + | }{ | |
| 351 | + | {name: "first render stays silent", steps: []time.Duration{0}}, | |
| 352 | + | {name: "a fast switch stays silent", steps: []time.Duration{0, connectingNoticeDelay - time.Millisecond}}, | |
| 353 | + | {name: "a slow switch shows the notice", steps: []time.Duration{0, connectingNoticeDelay}, painted: true}, | |
| 354 | + | {name: "leaving connecting restarts the delay", steps: []time.Duration{0, connectingNoticeDelay}, reset: true}, | |
| 355 | + | {name: "a remote attach or resume shows it at once", slow: true, steps: []time.Duration{0}, painted: true}, | |
| 356 | + | } | |
| 357 | + | for _, tt := range tests { | |
| 358 | + | t.Run(tt.name, func(t *testing.T) { | |
| 359 | + | connecting := client.State{Presentation: client.PresentConnecting, Connectivity: client.ConnectivityReady, Generation: 1, SlowConnecting: tt.slow} | |
| 360 | + | terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}}) | |
| 361 | + | clock := newSandboxClock() | |
| 362 | + | presentation := &brokerClientPresentation{terminal: terminal, picker: offlineRenderPicker(t), clock: clock} | |
| 363 | + | for i, step := range tt.steps { | |
| 364 | + | clock.Advance(step) | |
| 365 | + | if tt.reset && i == len(tt.steps)-1 { | |
| 366 | + | presentation.Render(attached) | |
| 367 | + | } | |
| 368 | + | presentation.Render(connecting) | |
| 369 | + | } | |
| 370 | + | written, _ := terminal.written() | |
| 371 | + | require.Equal(t, tt.painted, strings.Contains(written, "Connecting to session")) | |
| 372 | + | }) | |
| 373 | + | } | |
| 374 | + | } | |
| 375 | + | ||
| 376 | + | // TestBrokerClientSpinnerTimer pins the spinner timer lifecycle: none while | |
| 377 | + | // the plain picker shows, its first tick exactly when a held-back Connecting | |
| 378 | + | // notice becomes due, and a fresh timer after a transition ends. | |
| 379 | + | func TestBrokerClientSpinnerTimer(t *testing.T) { | |
| 380 | + | terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}}) | |
| 381 | + | clock := newSandboxClock() | |
| 382 | + | presentation := &brokerClientPresentation{terminal: terminal, picker: offlineRenderPicker(t), clock: clock} | |
| 383 | + | connecting := client.State{Presentation: client.PresentConnecting, Connectivity: client.ConnectivityReady, Generation: 1} | |
| 384 | + | ||
| 385 | + | presentation.Render(client.State{Presentation: client.PresentPicker, Connectivity: client.ConnectivityReady, Generation: 1}) | |
| 386 | + | require.Nil(t, presentation.Spinner(), "the plain picker has no spinner") | |
| 387 | + | ||
| 388 | + | for range 2 { | |
| 389 | + | presentation.Render(connecting) | |
| 390 | + | tick := presentation.Spinner() | |
| 391 | + | require.NotNil(t, tick) | |
| 392 | + | clock.Advance(connectingNoticeDelay - time.Millisecond) | |
| 393 | + | select { | |
| 394 | + | case <-tick: | |
| 395 | + | t.Fatal("the spinner ticked before the notice was due") | |
| 396 | + | default: | |
| 397 | + | } | |
| 398 | + | clock.Advance(time.Millisecond) | |
| 399 | + | select { | |
| 400 | + | case <-tick: | |
| 401 | + | default: | |
| 402 | + | t.Fatal("the spinner never ticked when the notice became due") | |
| 403 | + | } | |
| 404 | + | presentation.AdvanceSpinner(connecting) | |
| 405 | + | written, _ := terminal.written() | |
| 406 | + | require.Contains(t, written, "Connecting to session") | |
| 407 | + | presentation.StopSpinner() | |
| 408 | + | presentation.Render(client.State{Presentation: client.PresentAttached, Connectivity: client.ConnectivityReady, Generation: 1}) | |
| 409 | + | require.Nil(t, presentation.Spinner(), "a finished transition leaves no spinner") | |
| 410 | + | } | |
| 411 | + | } | |
| @@ -335,3 +335,77 @@ func TestBrokerClientFailureWritesOnlyNonRetryableErrors(t *testing.T) { | |||||
| 335 | }) | 335 | }) | ||
| 336 | } | 336 | } | ||
| 337 | } | 337 | } | ||
| 338 | + | ||||
| 339 | + | // TestBrokerClientConnectingNoticeIsDelayed pins that a fast switch never | |||
| 340 | + | // flashes the Connecting notice: it paints only once Connecting has lasted | |||
| 341 | + | // connectingNoticeDelay, and leaving Connecting restarts the delay. | |||
| 342 | + | func TestBrokerClientConnectingNoticeIsDelayed(t *testing.T) { | |||
| 343 | + | attached := client.State{Presentation: client.PresentAttached, Connectivity: client.ConnectivityReady, Generation: 1} | |||
| 344 | + | tests := []struct { | |||
| 345 | + | name string | |||
| 346 | + | slow bool // a remote attach or a resume | |||
| 347 | + | steps []time.Duration // clock advance before each Connecting render | |||
| 348 | + | reset bool // leave Connecting before the last render | |||
| 349 | + | painted bool | |||
| 350 | + | }{ | |||
| 351 | + | {name: "first render stays silent", steps: []time.Duration{0}}, | |||
| 352 | + | {name: "a fast switch stays silent", steps: []time.Duration{0, connectingNoticeDelay - time.Millisecond}}, | |||
| 353 | + | {name: "a slow switch shows the notice", steps: []time.Duration{0, connectingNoticeDelay}, painted: true}, | |||
| 354 | + | {name: "leaving connecting restarts the delay", steps: []time.Duration{0, connectingNoticeDelay}, reset: true}, | |||
| 355 | + | {name: "a remote attach or resume shows it at once", slow: true, steps: []time.Duration{0}, painted: true}, | |||
| 356 | + | } | |||
| 357 | + | for _, tt := range tests { | |||
| 358 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 359 | + | connecting := client.State{Presentation: client.PresentConnecting, Connectivity: client.ConnectivityReady, Generation: 1, SlowConnecting: tt.slow} | |||
| 360 | + | terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}}) | |||
| 361 | + | clock := newSandboxClock() | |||
| 362 | + | presentation := &brokerClientPresentation{terminal: terminal, picker: offlineRenderPicker(t), clock: clock} | |||
| 363 | + | for i, step := range tt.steps { | |||
| 364 | + | clock.Advance(step) | |||
| 365 | + | if tt.reset && i == len(tt.steps)-1 { | |||
| 366 | + | presentation.Render(attached) | |||
| 367 | + | } | |||
| 368 | + | presentation.Render(connecting) | |||
| 369 | + | } | |||
| 370 | + | written, _ := terminal.written() | |||
| 371 | + | require.Equal(t, tt.painted, strings.Contains(written, "Connecting to session")) | |||
| 372 | + | }) | |||
| 373 | + | } | |||
| 374 | + | } | |||
| 375 | + | ||||
| 376 | + | // TestBrokerClientSpinnerTimer pins the spinner timer lifecycle: none while | |||
| 377 | + | // the plain picker shows, its first tick exactly when a held-back Connecting | |||
| 378 | + | // notice becomes due, and a fresh timer after a transition ends. | |||
| 379 | + | func TestBrokerClientSpinnerTimer(t *testing.T) { | |||
| 380 | + | terminal := newOfflineRenderTerminal(domain.Geometry{Size: domain.Size{Cols: 80, Rows: 24}}) | |||
| 381 | + | clock := newSandboxClock() | |||
| 382 | + | presentation := &brokerClientPresentation{terminal: terminal, picker: offlineRenderPicker(t), clock: clock} | |||
| 383 | + | connecting := client.State{Presentation: client.PresentConnecting, Connectivity: client.ConnectivityReady, Generation: 1} | |||
| 384 | + | ||||
| 385 | + | presentation.Render(client.State{Presentation: client.PresentPicker, Connectivity: client.ConnectivityReady, Generation: 1}) | |||
| 386 | + | require.Nil(t, presentation.Spinner(), "the plain picker has no spinner") | |||
| 387 | + | ||||
| 388 | + | for range 2 { | |||
| 389 | + | presentation.Render(connecting) | |||
| 390 | + | tick := presentation.Spinner() | |||
| 391 | + | require.NotNil(t, tick) | |||
| 392 | + | clock.Advance(connectingNoticeDelay - time.Millisecond) | |||
| 393 | + | select { | |||
| 394 | + | case <-tick: | |||
| 395 | + | t.Fatal("the spinner ticked before the notice was due") | |||
| 396 | + | default: | |||
| 397 | + | } | |||
| 398 | + | clock.Advance(time.Millisecond) | |||
| 399 | + | select { | |||
| 400 | + | case <-tick: | |||
| 401 | + | default: | |||
| 402 | + | t.Fatal("the spinner never ticked when the notice became due") | |||
| 403 | + | } | |||
| 404 | + | presentation.AdvanceSpinner(connecting) | |||
| 405 | + | written, _ := terminal.written() | |||
| 406 | + | require.Contains(t, written, "Connecting to session") | |||
| 407 | + | presentation.StopSpinner() | |||
| 408 | + | presentation.Render(client.State{Presentation: client.PresentAttached, Connectivity: client.ConnectivityReady, Generation: 1}) | |||
| 409 | + | require.Nil(t, presentation.Spinner(), "a finished transition leaves no spinner") | |||
| 410 | + | } | |||
| 411 | + | } | |||
M internal/usecase/client/attachment_routes.go +90 -1
| @@ -1,6 +1,9 @@ | |||
| 1 | 1 | package client | |
| 2 | 2 | ||
| 3 | - | import "github.com/bnema/vev/internal/protocol" | |
| 3 | + | import ( | |
| 4 | + | "github.com/bnema/vev/internal/domain" | |
| 5 | + | "github.com/bnema/vev/internal/protocol" | |
| 6 | + | ) | |
| 4 | 7 | ||
| 5 | 8 | // Route publication and daemon navigation seams of the attached foreground. | |
| 6 | 9 | // The worker owns the stream, so the supervisor reaches the | |
| @@ -121,6 +124,92 @@ func (h *attachmentHost) replyNavigation(token AttachmentToken, message protocol | |||
| 121 | 124 | } | |
| 122 | 125 | } | |
| 123 | 126 | ||
| 127 | + | // inPlaceSwitch is one supervisor-committed session on the daemon already | |
| 128 | + | // serving the attachment. The worker asks that daemon to move the live | |
| 129 | + | // attachment there instead of reconnecting. seq identifies this choice, so a | |
| 130 | + | // result for an older one is never mistaken for it. | |
| 131 | + | type inPlaceSwitch struct { | |
| 132 | + | seq uint64 | |
| 133 | + | target protocol.ExactSessionTarget | |
| 134 | + | tab domain.TabStableID | |
| 135 | + | } | |
| 136 | + | ||
| 137 | + | // inPlaceOutcome is how one in-place switch ended. | |
| 138 | + | type inPlaceOutcome uint8 | |
| 139 | + | ||
| 140 | + | const ( | |
| 141 | + | // inPlaceArrived: the daemon moved the attachment to the target. | |
| 142 | + | inPlaceArrived inPlaceOutcome = iota + 1 | |
| 143 | + | // inPlaceRefused: the daemon kept the source; reconnect to the target. | |
| 144 | + | inPlaceRefused | |
| 145 | + | // inPlaceSuperseded: a newer navigation replaced this one; drop it. | |
| 146 | + | inPlaceSuperseded | |
| 147 | + | ) | |
| 148 | + | ||
| 149 | + | // inPlaceResult reports the outcome of the in-place switch seq. | |
| 150 | + | type inPlaceResult struct { | |
| 151 | + | seq uint64 | |
| 152 | + | outcome inPlaceOutcome | |
| 153 | + | } | |
| 154 | + | ||
| 155 | + | // requestInPlace hands one in-place switch to the current foreground's worker. | |
| 156 | + | // The newest request replaces an unsent one. It reports false when token no | |
| 157 | + | // longer owns the foreground. | |
| 158 | + | func (h *attachmentHost) requestInPlace(token AttachmentToken, request inPlaceSwitch) bool { | |
| 159 | + | fg := h.currentForeground(token) | |
| 160 | + | if fg == nil { | |
| 161 | + | return false | |
| 162 | + | } | |
| 163 | + | for { | |
| 164 | + | select { | |
| 165 | + | case fg.inPlace <- request: | |
| 166 | + | return true | |
| 167 | + | default: | |
| 168 | + | } | |
| 169 | + | select { | |
| 170 | + | case <-fg.inPlace: | |
| 171 | + | default: | |
| 172 | + | } | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | ||
| 176 | + | // InPlaceResults carries the outcome of each in-place switch to the | |
| 177 | + | // supervisor. | |
| 178 | + | func (h *attachmentHost) InPlaceResults() <-chan inPlaceResult { | |
| 179 | + | if h == nil { | |
| 180 | + | return nil | |
| 181 | + | } | |
| 182 | + | return h.inPlaceResults | |
| 183 | + | } | |
| 184 | + | ||
| 185 | + | func (f *attachmentForeground) inPlaceSwitches() <-chan inPlaceSwitch { | |
| 186 | + | if f == nil { | |
| 187 | + | return nil | |
| 188 | + | } | |
| 189 | + | return f.inPlace | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | // reportInPlace hands the outcome of one in-place switch to the supervisor. | |
| 193 | + | // The newest outcome replaces one the supervisor has not taken yet; it is | |
| 194 | + | // always the latest choice, the only one the supervisor still waits for. | |
| 195 | + | func (f *attachmentForeground) reportInPlace(seq uint64, outcome inPlaceOutcome) { | |
| 196 | + | if f == nil || f.host == nil || seq == 0 { | |
| 197 | + | return | |
| 198 | + | } | |
| 199 | + | result := inPlaceResult{seq: seq, outcome: outcome} | |
| 200 | + | for { | |
| 201 | + | select { | |
| 202 | + | case f.host.inPlaceResults <- result: | |
| 203 | + | return | |
| 204 | + | default: | |
| 205 | + | } | |
| 206 | + | select { | |
| 207 | + | case <-f.host.inPlaceResults: | |
| 208 | + | default: | |
| 209 | + | } | |
| 210 | + | } | |
| 211 | + | } | |
| 212 | + | ||
| 124 | 213 | func (h *attachmentHost) currentForeground(token AttachmentToken) *attachmentForeground { | |
| 125 | 214 | if h == nil { | |
| 126 | 215 | return nil | |
| @@ -1,6 +1,9 @@ | |||||
| 1 | package client | 1 | package client | ||
| 2 | 2 | ||||
| 3 | - | import "github.com/bnema/vev/internal/protocol" | 3 | + | import ( |
| 4 | + | "github.com/bnema/vev/internal/domain" | |||
| 5 | + | "github.com/bnema/vev/internal/protocol" | |||
| 6 | + | ) | |||
| 4 | 7 | ||||
| 5 | // Route publication and daemon navigation seams of the attached foreground. | 8 | // Route publication and daemon navigation seams of the attached foreground. | ||
| 6 | // The worker owns the stream, so the supervisor reaches the | 9 | // The worker owns the stream, so the supervisor reaches the | ||
| @@ -121,6 +124,92 @@ func (h *attachmentHost) replyNavigation(token AttachmentToken, message protocol | |||||
| 121 | } | 124 | } | ||
| 122 | } | 125 | } | ||
| 123 | 126 | ||||
| 127 | + | // inPlaceSwitch is one supervisor-committed session on the daemon already | |||
| 128 | + | // serving the attachment. The worker asks that daemon to move the live | |||
| 129 | + | // attachment there instead of reconnecting. seq identifies this choice, so a | |||
| 130 | + | // result for an older one is never mistaken for it. | |||
| 131 | + | type inPlaceSwitch struct { | |||
| 132 | + | seq uint64 | |||
| 133 | + | target protocol.ExactSessionTarget | |||
| 134 | + | tab domain.TabStableID | |||
| 135 | + | } | |||
| 136 | + | ||||
| 137 | + | // inPlaceOutcome is how one in-place switch ended. | |||
| 138 | + | type inPlaceOutcome uint8 | |||
| 139 | + | ||||
| 140 | + | const ( | |||
| 141 | + | // inPlaceArrived: the daemon moved the attachment to the target. | |||
| 142 | + | inPlaceArrived inPlaceOutcome = iota + 1 | |||
| 143 | + | // inPlaceRefused: the daemon kept the source; reconnect to the target. | |||
| 144 | + | inPlaceRefused | |||
| 145 | + | // inPlaceSuperseded: a newer navigation replaced this one; drop it. | |||
| 146 | + | inPlaceSuperseded | |||
| 147 | + | ) | |||
| 148 | + | ||||
| 149 | + | // inPlaceResult reports the outcome of the in-place switch seq. | |||
| 150 | + | type inPlaceResult struct { | |||
| 151 | + | seq uint64 | |||
| 152 | + | outcome inPlaceOutcome | |||
| 153 | + | } | |||
| 154 | + | ||||
| 155 | + | // requestInPlace hands one in-place switch to the current foreground's worker. | |||
| 156 | + | // The newest request replaces an unsent one. It reports false when token no | |||
| 157 | + | // longer owns the foreground. | |||
| 158 | + | func (h *attachmentHost) requestInPlace(token AttachmentToken, request inPlaceSwitch) bool { | |||
| 159 | + | fg := h.currentForeground(token) | |||
| 160 | + | if fg == nil { | |||
| 161 | + | return false | |||
| 162 | + | } | |||
| 163 | + | for { | |||
| 164 | + | select { | |||
| 165 | + | case fg.inPlace <- request: | |||
| 166 | + | return true | |||
| 167 | + | default: | |||
| 168 | + | } | |||
| 169 | + | select { | |||
| 170 | + | case <-fg.inPlace: | |||
| 171 | + | default: | |||
| 172 | + | } | |||
| 173 | + | } | |||
| 174 | + | } | |||
| 175 | + | ||||
| 176 | + | // InPlaceResults carries the outcome of each in-place switch to the | |||
| 177 | + | // supervisor. | |||
| 178 | + | func (h *attachmentHost) InPlaceResults() <-chan inPlaceResult { | |||
| 179 | + | if h == nil { | |||
| 180 | + | return nil | |||
| 181 | + | } | |||
| 182 | + | return h.inPlaceResults | |||
| 183 | + | } | |||
| 184 | + | ||||
| 185 | + | func (f *attachmentForeground) inPlaceSwitches() <-chan inPlaceSwitch { | |||
| 186 | + | if f == nil { | |||
| 187 | + | return nil | |||
| 188 | + | } | |||
| 189 | + | return f.inPlace | |||
| 190 | + | } | |||
| 191 | + | ||||
| 192 | + | // reportInPlace hands the outcome of one in-place switch to the supervisor. | |||
| 193 | + | // The newest outcome replaces one the supervisor has not taken yet; it is | |||
| 194 | + | // always the latest choice, the only one the supervisor still waits for. | |||
| 195 | + | func (f *attachmentForeground) reportInPlace(seq uint64, outcome inPlaceOutcome) { | |||
| 196 | + | if f == nil || f.host == nil || seq == 0 { | |||
| 197 | + | return | |||
| 198 | + | } | |||
| 199 | + | result := inPlaceResult{seq: seq, outcome: outcome} | |||
| 200 | + | for { | |||
| 201 | + | select { | |||
| 202 | + | case f.host.inPlaceResults <- result: | |||
| 203 | + | return | |||
| 204 | + | default: | |||
| 205 | + | } | |||
| 206 | + | select { | |||
| 207 | + | case <-f.host.inPlaceResults: | |||
| 208 | + | default: | |||
| 209 | + | } | |||
| 210 | + | } | |||
| 211 | + | } | |||
| 212 | + | ||||
| 124 | func (h *attachmentHost) currentForeground(token AttachmentToken) *attachmentForeground { | 213 | func (h *attachmentHost) currentForeground(token AttachmentToken) *attachmentForeground { | ||
| 125 | if h == nil { | 214 | if h == nil { | ||
| 126 | return nil | 215 | return nil | ||
M internal/usecase/client/attachment_session.go +69 -11
| @@ -320,14 +320,46 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 320 | 320 | var tabSelections <-chan domain.TabStableID | |
| 321 | 321 | var routeUpdates <-chan protocol.RecentRouteSnapshot | |
| 322 | 322 | var navigationReplies <-chan protocol.ClientMessage | |
| 323 | + | var inPlaceSwitches <-chan inPlaceSwitch | |
| 323 | 324 | if overlay != nil { | |
| 324 | 325 | repaint = overlay.overlayRepaint() | |
| 325 | 326 | tabSelections = overlay.tabSelections() | |
| 326 | 327 | routeUpdates = overlay.routeSnapshots() | |
| 327 | 328 | navigationReplies = overlay.navigationReplies() | |
| 329 | + | inPlaceSwitches = overlay.inPlaceSwitches() | |
| 328 | 330 | } | |
| 329 | 331 | var samePeerRequests uint64 | |
| 330 | 332 | var samePeerSwitch *samePeerSwitchPending | |
| 333 | + | // repaintOwed is set while a picker in-place switch replaced the closing | |
| 334 | + | // picker's repaint: if the source is kept after all, the worker still owes | |
| 335 | + | // the daemon that full repaint over the picker box. | |
| 336 | + | repaintOwed := false | |
| 337 | + | reportInPlace := func(seq uint64, outcome inPlaceOutcome) { | |
| 338 | + | if overlay != nil { | |
| 339 | + | overlay.reportInPlace(seq, outcome) | |
| 340 | + | } | |
| 341 | + | } | |
| 342 | + | // startSamePeer asks the daemon to move this attachment to target. seq is | |
| 343 | + | // the supervisor's choice, or zero for a daemon offer. A switch still in | |
| 344 | + | // flight is superseded: the newest navigation wins. | |
| 345 | + | newSamePeerRequest := func(target protocol.ExactSessionTarget, tab domain.TabStableID) (protocol.SamePeerSwitchRequest, bool) { | |
| 346 | + | request := protocol.SamePeerSwitchRequest{ | |
| 347 | + | RequestID: samePeerRequests + 1, Target: target, | |
| 348 | + | PreferredTabID: w.cfg.Tabs.preferred(requestAuthority(w.cfg.Request), target, tab), | |
| 349 | + | } | |
| 350 | + | return request, request.Validate() == nil | |
| 351 | + | } | |
| 352 | + | startSamePeer := func(request protocol.SamePeerSwitchRequest, seq uint64) error { | |
| 353 | + | samePeerRequests = request.RequestID | |
| 354 | + | if seq != 0 { | |
| 355 | + | repaintOwed = true | |
| 356 | + | } | |
| 357 | + | if samePeerSwitch != nil { | |
| 358 | + | reportInPlace(samePeerSwitch.seq, inPlaceSuperseded) | |
| 359 | + | } | |
| 360 | + | samePeerSwitch = &samePeerSwitchPending{requestID: request.RequestID, target: request.Target, seq: seq} | |
| 361 | + | return w.send(ctx, fg, stream, request) | |
| 362 | + | } | |
| 331 | 363 | samePeerUI := attachmentSamePeerUI(fg) | |
| 332 | 364 | picker := &attachmentMovePicker{worker: w, fg: fg, overlay: overlay, stream: stream, size: w.cfg.Geometry.Size, move: newMovePickerOverlay(w.cfg.Color)} | |
| 333 | 365 | defer picker.stopEscape() | |
| @@ -470,6 +502,17 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 470 | 502 | if err := w.send(ctx, fg, stream, reply); err != nil { | |
| 471 | 503 | return w.settle(ctx, fg, stream, token, err) | |
| 472 | 504 | } | |
| 505 | + | case next := <-inPlaceSwitches: | |
| 506 | + | // The supervisor committed another session on this daemon: ask it | |
| 507 | + | // to move this attachment there instead of reconnecting. | |
| 508 | + | request, ok := newSamePeerRequest(next.target, next.tab) | |
| 509 | + | if !ok { | |
| 510 | + | reportInPlace(next.seq, inPlaceRefused) | |
| 511 | + | continue | |
| 512 | + | } | |
| 513 | + | if err := startSamePeer(request, next.seq); err != nil { | |
| 514 | + | return w.settle(ctx, fg, stream, token, err) | |
| 515 | + | } | |
| 473 | 516 | case <-picker.escape(): | |
| 474 | 517 | picker.escapeFired() | |
| 475 | 518 | op, changed := picker.move.flush() | |
| @@ -491,7 +534,9 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 491 | 534 | if samePeerSwitch != nil && state.context.Route.Target == samePeerSwitch.target { | |
| 492 | 535 | // The in-place destination committed: its publication | |
| 493 | 536 | // settles the action that caused the switch. | |
| 537 | + | reportInPlace(samePeerSwitch.seq, inPlaceArrived) | |
| 494 | 538 | samePeerSwitch = nil | |
| 539 | + | repaintOwed = false | |
| 495 | 540 | if samePeerUI != nil { | |
| 496 | 541 | samePeerUI.uiSamePeerArrived() | |
| 497 | 542 | } | |
| @@ -539,19 +584,14 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 539 | 584 | if typed.SamePeer && typed.ExactTarget != nil { | |
| 540 | 585 | // An endpoint-empty offer on this very connection: confirm | |
| 541 | 586 | // it and the daemon moves this attachment in place. | |
| 542 | - | samePeerRequests++ | |
| 543 | - | request := protocol.SamePeerSwitchRequest{ | |
| 544 | - | RequestID: samePeerRequests, Target: *typed.ExactTarget, | |
| 545 | - | PreferredTabID: w.cfg.Tabs.preferred(requestAuthority(w.cfg.Request), *typed.ExactTarget, typed.PreferredTabID), | |
| 546 | - | } | |
| 547 | - | if request.Validate() != nil { | |
| 587 | + | request, ok := newSamePeerRequest(*typed.ExactTarget, typed.PreferredTabID) | |
| 588 | + | if !ok { | |
| 548 | 589 | continue | |
| 549 | 590 | } | |
| 550 | 591 | if samePeerUI != nil { | |
| 551 | 592 | samePeerUI.uiFollowSamePeer(typed.CauseActionID) | |
| 552 | 593 | } | |
| 553 | - | samePeerSwitch = &samePeerSwitchPending{requestID: request.RequestID, target: request.Target} | |
| 554 | - | if err := w.send(ctx, fg, stream, request); err != nil { | |
| 594 | + | if err := startSamePeer(request, 0); err != nil { | |
| 555 | 595 | return w.settle(ctx, fg, stream, token, err) | |
| 556 | 596 | } | |
| 557 | 597 | continue | |
| @@ -562,10 +602,25 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||
| 562 | 602 | case protocol.SamePeerSwitchFailure: | |
| 563 | 603 | // The daemon refused the confirmed switch and kept the source. | |
| 564 | 604 | if samePeerSwitch != nil && typed.RequestID == samePeerSwitch.requestID { | |
| 565 | - | samePeerSwitch = nil | |
| 566 | - | if samePeerUI != nil { | |
| 567 | - | samePeerUI.uiSamePeerFailed() | |
| 605 | + | if samePeerSwitch.seq != 0 { | |
| 606 | + | // The supervisor falls back to a fresh attachment; the | |
| 607 | + | // action it follows settles there. | |
| 608 | + | reportInPlace(samePeerSwitch.seq, inPlaceRefused) | |
| 609 | + | } else { | |
| 610 | + | if samePeerUI != nil { | |
| 611 | + | samePeerUI.uiSamePeerFailed() | |
| 612 | + | } | |
| 613 | + | if repaintOwed { | |
| 614 | + | // No swap follows: the source stays, under what is | |
| 615 | + | // left of the picker box. | |
| 616 | + | repaintOwed = false | |
| 617 | + | if err := w.send(ctx, fg, stream, protocol.OutputResetRequest{}); err != nil { | |
| 618 | + | return w.settle(ctx, fg, stream, token, err) | |
| 619 | + | } | |
| 620 | + | outputResetRequested = true | |
| 621 | + | } | |
| 568 | 622 | } | |
| 623 | + | samePeerSwitch = nil | |
| 569 | 624 | } | |
| 570 | 625 | case protocol.RouteNavigationAction, protocol.RouteCreateSessionAction: | |
| 571 | 626 | // The daemon asks the client to navigate: the supervisor | |
| @@ -744,6 +799,9 @@ func attachmentSamePeerUI(fg AttachmentForeground) attachmentSamePeerForeground | |||
| 744 | 799 | type samePeerSwitchPending struct { | |
| 745 | 800 | requestID uint64 | |
| 746 | 801 | target protocol.ExactSessionTarget | |
| 802 | + | // seq is the supervisor's in-place choice this switch carries out, zero | |
| 803 | + | // for a daemon offer. A nonzero seq reports its outcome to the supervisor. | |
| 804 | + | seq uint64 | |
| 747 | 805 | } | |
| 748 | 806 | ||
| 749 | 807 | // pumpAttachmentInput forwards each authorized delivery to the attached loop. | |
| @@ -320,14 +320,46 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 320 | var tabSelections <-chan domain.TabStableID | 320 | var tabSelections <-chan domain.TabStableID | ||
| 321 | var routeUpdates <-chan protocol.RecentRouteSnapshot | 321 | var routeUpdates <-chan protocol.RecentRouteSnapshot | ||
| 322 | var navigationReplies <-chan protocol.ClientMessage | 322 | var navigationReplies <-chan protocol.ClientMessage | ||
| 323 | + | var inPlaceSwitches <-chan inPlaceSwitch | |||
| 323 | if overlay != nil { | 324 | if overlay != nil { | ||
| 324 | repaint = overlay.overlayRepaint() | 325 | repaint = overlay.overlayRepaint() | ||
| 325 | tabSelections = overlay.tabSelections() | 326 | tabSelections = overlay.tabSelections() | ||
| 326 | routeUpdates = overlay.routeSnapshots() | 327 | routeUpdates = overlay.routeSnapshots() | ||
| 327 | navigationReplies = overlay.navigationReplies() | 328 | navigationReplies = overlay.navigationReplies() | ||
| 329 | + | inPlaceSwitches = overlay.inPlaceSwitches() | |||
| 328 | } | 330 | } | ||
| 329 | var samePeerRequests uint64 | 331 | var samePeerRequests uint64 | ||
| 330 | var samePeerSwitch *samePeerSwitchPending | 332 | var samePeerSwitch *samePeerSwitchPending | ||
| 333 | + | // repaintOwed is set while a picker in-place switch replaced the closing | |||
| 334 | + | // picker's repaint: if the source is kept after all, the worker still owes | |||
| 335 | + | // the daemon that full repaint over the picker box. | |||
| 336 | + | repaintOwed := false | |||
| 337 | + | reportInPlace := func(seq uint64, outcome inPlaceOutcome) { | |||
| 338 | + | if overlay != nil { | |||
| 339 | + | overlay.reportInPlace(seq, outcome) | |||
| 340 | + | } | |||
| 341 | + | } | |||
| 342 | + | // startSamePeer asks the daemon to move this attachment to target. seq is | |||
| 343 | + | // the supervisor's choice, or zero for a daemon offer. A switch still in | |||
| 344 | + | // flight is superseded: the newest navigation wins. | |||
| 345 | + | newSamePeerRequest := func(target protocol.ExactSessionTarget, tab domain.TabStableID) (protocol.SamePeerSwitchRequest, bool) { | |||
| 346 | + | request := protocol.SamePeerSwitchRequest{ | |||
| 347 | + | RequestID: samePeerRequests + 1, Target: target, | |||
| 348 | + | PreferredTabID: w.cfg.Tabs.preferred(requestAuthority(w.cfg.Request), target, tab), | |||
| 349 | + | } | |||
| 350 | + | return request, request.Validate() == nil | |||
| 351 | + | } | |||
| 352 | + | startSamePeer := func(request protocol.SamePeerSwitchRequest, seq uint64) error { | |||
| 353 | + | samePeerRequests = request.RequestID | |||
| 354 | + | if seq != 0 { | |||
| 355 | + | repaintOwed = true | |||
| 356 | + | } | |||
| 357 | + | if samePeerSwitch != nil { | |||
| 358 | + | reportInPlace(samePeerSwitch.seq, inPlaceSuperseded) | |||
| 359 | + | } | |||
| 360 | + | samePeerSwitch = &samePeerSwitchPending{requestID: request.RequestID, target: request.Target, seq: seq} | |||
| 361 | + | return w.send(ctx, fg, stream, request) | |||
| 362 | + | } | |||
| 331 | samePeerUI := attachmentSamePeerUI(fg) | 363 | samePeerUI := attachmentSamePeerUI(fg) | ||
| 332 | picker := &attachmentMovePicker{worker: w, fg: fg, overlay: overlay, stream: stream, size: w.cfg.Geometry.Size, move: newMovePickerOverlay(w.cfg.Color)} | 364 | picker := &attachmentMovePicker{worker: w, fg: fg, overlay: overlay, stream: stream, size: w.cfg.Geometry.Size, move: newMovePickerOverlay(w.cfg.Color)} | ||
| 333 | defer picker.stopEscape() | 365 | defer picker.stopEscape() | ||
| @@ -470,6 +502,17 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 470 | if err := w.send(ctx, fg, stream, reply); err != nil { | 502 | if err := w.send(ctx, fg, stream, reply); err != nil { | ||
| 471 | return w.settle(ctx, fg, stream, token, err) | 503 | return w.settle(ctx, fg, stream, token, err) | ||
| 472 | } | 504 | } | ||
| 505 | + | case next := <-inPlaceSwitches: | |||
| 506 | + | // The supervisor committed another session on this daemon: ask it | |||
| 507 | + | // to move this attachment there instead of reconnecting. | |||
| 508 | + | request, ok := newSamePeerRequest(next.target, next.tab) | |||
| 509 | + | if !ok { | |||
| 510 | + | reportInPlace(next.seq, inPlaceRefused) | |||
| 511 | + | continue | |||
| 512 | + | } | |||
| 513 | + | if err := startSamePeer(request, next.seq); err != nil { | |||
| 514 | + | return w.settle(ctx, fg, stream, token, err) | |||
| 515 | + | } | |||
| 473 | case <-picker.escape(): | 516 | case <-picker.escape(): | ||
| 474 | picker.escapeFired() | 517 | picker.escapeFired() | ||
| 475 | op, changed := picker.move.flush() | 518 | op, changed := picker.move.flush() | ||
| @@ -491,7 +534,9 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 491 | if samePeerSwitch != nil && state.context.Route.Target == samePeerSwitch.target { | 534 | if samePeerSwitch != nil && state.context.Route.Target == samePeerSwitch.target { | ||
| 492 | // The in-place destination committed: its publication | 535 | // The in-place destination committed: its publication | ||
| 493 | // settles the action that caused the switch. | 536 | // settles the action that caused the switch. | ||
| 537 | + | reportInPlace(samePeerSwitch.seq, inPlaceArrived) | |||
| 494 | samePeerSwitch = nil | 538 | samePeerSwitch = nil | ||
| 539 | + | repaintOwed = false | |||
| 495 | if samePeerUI != nil { | 540 | if samePeerUI != nil { | ||
| 496 | samePeerUI.uiSamePeerArrived() | 541 | samePeerUI.uiSamePeerArrived() | ||
| 497 | } | 542 | } | ||
| @@ -539,19 +584,14 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 539 | if typed.SamePeer && typed.ExactTarget != nil { | 584 | if typed.SamePeer && typed.ExactTarget != nil { | ||
| 540 | // An endpoint-empty offer on this very connection: confirm | 585 | // An endpoint-empty offer on this very connection: confirm | ||
| 541 | // it and the daemon moves this attachment in place. | 586 | // it and the daemon moves this attachment in place. | ||
| 542 | - | samePeerRequests++ | 587 | + | request, ok := newSamePeerRequest(*typed.ExactTarget, typed.PreferredTabID) |
| 543 | - | request := protocol.SamePeerSwitchRequest{ | 588 | + | if !ok { |
| 544 | - | RequestID: samePeerRequests, Target: *typed.ExactTarget, | |||
| 545 | - | PreferredTabID: w.cfg.Tabs.preferred(requestAuthority(w.cfg.Request), *typed.ExactTarget, typed.PreferredTabID), | |||
| 546 | - | } | |||
| 547 | - | if request.Validate() != nil { | |||
| 548 | continue | 589 | continue | ||
| 549 | } | 590 | } | ||
| 550 | if samePeerUI != nil { | 591 | if samePeerUI != nil { | ||
| 551 | samePeerUI.uiFollowSamePeer(typed.CauseActionID) | 592 | samePeerUI.uiFollowSamePeer(typed.CauseActionID) | ||
| 552 | } | 593 | } | ||
| 553 | - | samePeerSwitch = &samePeerSwitchPending{requestID: request.RequestID, target: request.Target} | 594 | + | if err := startSamePeer(request, 0); err != nil { |
| 554 | - | if err := w.send(ctx, fg, stream, request); err != nil { | |||
| 555 | return w.settle(ctx, fg, stream, token, err) | 595 | return w.settle(ctx, fg, stream, token, err) | ||
| 556 | } | 596 | } | ||
| 557 | continue | 597 | continue | ||
| @@ -562,10 +602,25 @@ func (w *sessionAttachmentWorker) pumpAttached(ctx context.Context, fg Attachmen | |||||
| 562 | case protocol.SamePeerSwitchFailure: | 602 | case protocol.SamePeerSwitchFailure: | ||
| 563 | // The daemon refused the confirmed switch and kept the source. | 603 | // The daemon refused the confirmed switch and kept the source. | ||
| 564 | if samePeerSwitch != nil && typed.RequestID == samePeerSwitch.requestID { | 604 | if samePeerSwitch != nil && typed.RequestID == samePeerSwitch.requestID { | ||
| 565 | - | samePeerSwitch = nil | 605 | + | if samePeerSwitch.seq != 0 { |
| 566 | - | if samePeerUI != nil { | 606 | + | // The supervisor falls back to a fresh attachment; the |
| 567 | - | samePeerUI.uiSamePeerFailed() | 607 | + | // action it follows settles there. |
| 608 | + | reportInPlace(samePeerSwitch.seq, inPlaceRefused) | |||
| 609 | + | } else { | |||
| 610 | + | if samePeerUI != nil { | |||
| 611 | + | samePeerUI.uiSamePeerFailed() | |||
| 612 | + | } | |||
| 613 | + | if repaintOwed { | |||
| 614 | + | // No swap follows: the source stays, under what is | |||
| 615 | + | // left of the picker box. | |||
| 616 | + | repaintOwed = false | |||
| 617 | + | if err := w.send(ctx, fg, stream, protocol.OutputResetRequest{}); err != nil { | |||
| 618 | + | return w.settle(ctx, fg, stream, token, err) | |||
| 619 | + | } | |||
| 620 | + | outputResetRequested = true | |||
| 621 | + | } | |||
| 568 | } | 622 | } | ||
| 623 | + | samePeerSwitch = nil | |||
| 569 | } | 624 | } | ||
| 570 | case protocol.RouteNavigationAction, protocol.RouteCreateSessionAction: | 625 | case protocol.RouteNavigationAction, protocol.RouteCreateSessionAction: | ||
| 571 | // The daemon asks the client to navigate: the supervisor | 626 | // The daemon asks the client to navigate: the supervisor | ||
| @@ -744,6 +799,9 @@ func attachmentSamePeerUI(fg AttachmentForeground) attachmentSamePeerForeground | |||||
| 744 | type samePeerSwitchPending struct { | 799 | type samePeerSwitchPending struct { | ||
| 745 | requestID uint64 | 800 | requestID uint64 | ||
| 746 | target protocol.ExactSessionTarget | 801 | target protocol.ExactSessionTarget | ||
| 802 | + | // seq is the supervisor's in-place choice this switch carries out, zero | |||
| 803 | + | // for a daemon offer. A nonzero seq reports its outcome to the supervisor. | |||
| 804 | + | seq uint64 | |||
| 747 | } | 805 | } | ||
| 748 | 806 | ||||
| 749 | // pumpAttachmentInput forwards each authorized delivery to the attached loop. | 807 | // pumpAttachmentInput forwards each authorized delivery to the attached loop. | ||
M internal/usecase/client/attachment_worker.go +22 -4
| @@ -396,6 +396,9 @@ type attachmentHost struct { | |||
| 396 | 396 | // overlayActions carries driver actions the client picker overlay | |
| 397 | 397 | // consumed to the supervisor, which settles them. | |
| 398 | 398 | overlayActions chan overlayActionConsumed | |
| 399 | + | // inPlaceResults carries the outcome of each in-place switch the | |
| 400 | + | // supervisor requested on the current foreground. | |
| 401 | + | inPlaceResults chan inPlaceResult | |
| 399 | 402 | } | |
| 400 | 403 | ||
| 401 | 404 | // newAttachmentHost builds the reusable foreground host. A nil clock falls back | |
| @@ -438,6 +441,7 @@ func newAttachmentHost(cfg attachmentHostConfig) *attachmentHost { | |||
| 438 | 441 | routeDemand: make(chan struct{}, 1), | |
| 439 | 442 | navigations: make(chan protocol.ServerMessage, 1), | |
| 440 | 443 | overlayActions: make(chan overlayActionConsumed, 1), | |
| 444 | + | inPlaceResults: make(chan inPlaceResult, 1), | |
| 441 | 445 | } | |
| 442 | 446 | } | |
| 443 | 447 | ||
| @@ -611,6 +615,7 @@ func (h *attachmentHost) newForeground(token AttachmentToken, stream ports.Broke | |||
| 611 | 615 | tabSelect: make(chan domain.TabStableID, 1), | |
| 612 | 616 | routes: make(chan protocol.RecentRouteSnapshot, 1), | |
| 613 | 617 | replies: make(chan protocol.ClientMessage, attachmentReplyCapacity), | |
| 618 | + | inPlace: make(chan inPlaceSwitch, 1), | |
| 614 | 619 | } | |
| 615 | 620 | } | |
| 616 | 621 | ||
| @@ -624,6 +629,7 @@ func (h *attachmentHost) drainForegroundSignals() { | |||
| 624 | 629 | case <-h.routeDemand: | |
| 625 | 630 | case <-h.navigations: | |
| 626 | 631 | case <-h.overlayActions: | |
| 632 | + | case <-h.inPlaceResults: | |
| 627 | 633 | default: | |
| 628 | 634 | return | |
| 629 | 635 | } | |
| @@ -654,14 +660,14 @@ func (h *attachmentHost) beginNavigationOverlay(sink pickerInputConsumer) bool { | |||
| 654 | 660 | return fg.setOverlay(attachmentOverlayNavigation, sink) | |
| 655 | 661 | } | |
| 656 | 662 | ||
| 657 | - | // endNavigationOverlay returns the terminal to the current foreground and asks | |
| 658 | - | // its worker for an authoritative repaint over the picker box. | |
| 659 | - | func (h *attachmentHost) endNavigationOverlay() { | |
| 663 | + | // endNavigationOverlay returns the terminal to the current foreground. With | |
| 664 | + | // repaint it asks the worker for an authoritative repaint over the picker box. | |
| 665 | + | func (h *attachmentHost) endNavigationOverlay(repaint bool) { | |
| 660 | 666 | if h == nil { | |
| 661 | 667 | return | |
| 662 | 668 | } | |
| 663 | 669 | if fg := h.authority.foreground(); fg != nil { | |
| 664 | - | fg.clearOverlay(attachmentOverlayNavigation) | |
| 670 | + | fg.releaseOverlay(attachmentOverlayNavigation, repaint) | |
| 665 | 671 | } | |
| 666 | 672 | } | |
| 667 | 673 | ||
| @@ -761,6 +767,8 @@ type attachmentOverlayForeground interface { | |||
| 761 | 767 | routeSnapshots() <-chan protocol.RecentRouteSnapshot | |
| 762 | 768 | navigationReplies() <-chan protocol.ClientMessage | |
| 763 | 769 | requestNavigation(message protocol.ServerMessage) | |
| 770 | + | inPlaceSwitches() <-chan inPlaceSwitch | |
| 771 | + | reportInPlace(seq uint64, outcome inPlaceOutcome) | |
| 764 | 772 | } | |
| 765 | 773 | ||
| 766 | 774 | var _ attachmentOverlayForeground = (*attachmentForeground)(nil) | |
| @@ -807,6 +815,11 @@ func (f *attachmentForeground) setOverlay(kind attachmentOverlayKind, sink picke | |||
| 807 | 815 | // authoritative repaint: the terminal still shows the picker box and any | |
| 808 | 816 | // attachment frames it suppressed were never written. | |
| 809 | 817 | func (f *attachmentForeground) clearOverlay(kind attachmentOverlayKind) { | |
| 818 | + | f.releaseOverlay(kind, true) | |
| 819 | + | } | |
| 820 | + | ||
| 821 | + | // releaseOverlay is clearOverlay with the repaint request optional. | |
| 822 | + | func (f *attachmentForeground) releaseOverlay(kind attachmentOverlayKind, repaint bool) { | |
| 810 | 823 | if f == nil { | |
| 811 | 824 | return | |
| 812 | 825 | } | |
| @@ -832,6 +845,9 @@ func (f *attachmentForeground) clearOverlay(kind attachmentOverlayKind) { | |||
| 832 | 845 | f.overlayMu.Unlock() | |
| 833 | 846 | return | |
| 834 | 847 | } | |
| 848 | + | if !repaint { | |
| 849 | + | return | |
| 850 | + | } | |
| 835 | 851 | select { | |
| 836 | 852 | case f.repaint <- struct{}{}: | |
| 837 | 853 | default: | |
| @@ -1191,6 +1207,8 @@ type attachmentForeground struct { | |||
| 1191 | 1207 | // carries the supervisor's navigation failures. | |
| 1192 | 1208 | routes chan protocol.RecentRouteSnapshot | |
| 1193 | 1209 | replies chan protocol.ClientMessage | |
| 1210 | + | // inPlace carries one pending supervisor in-place switch to the worker. | |
| 1211 | + | inPlace chan inPlaceSwitch | |
| 1194 | 1212 | } | |
| 1195 | 1213 | ||
| 1196 | 1214 | // Token is the generation/attempt identity of this grant. | |
| @@ -396,6 +396,9 @@ type attachmentHost struct { | |||||
| 396 | // overlayActions carries driver actions the client picker overlay | 396 | // overlayActions carries driver actions the client picker overlay | ||
| 397 | // consumed to the supervisor, which settles them. | 397 | // consumed to the supervisor, which settles them. | ||
| 398 | overlayActions chan overlayActionConsumed | 398 | overlayActions chan overlayActionConsumed | ||
| 399 | + | // inPlaceResults carries the outcome of each in-place switch the | |||
| 400 | + | // supervisor requested on the current foreground. | |||
| 401 | + | inPlaceResults chan inPlaceResult | |||
| 399 | } | 402 | } | ||
| 400 | 403 | ||||
| 401 | // newAttachmentHost builds the reusable foreground host. A nil clock falls back | 404 | // newAttachmentHost builds the reusable foreground host. A nil clock falls back | ||
| @@ -438,6 +441,7 @@ func newAttachmentHost(cfg attachmentHostConfig) *attachmentHost { | |||||
| 438 | routeDemand: make(chan struct{}, 1), | 441 | routeDemand: make(chan struct{}, 1), | ||
| 439 | navigations: make(chan protocol.ServerMessage, 1), | 442 | navigations: make(chan protocol.ServerMessage, 1), | ||
| 440 | overlayActions: make(chan overlayActionConsumed, 1), | 443 | overlayActions: make(chan overlayActionConsumed, 1), | ||
| 444 | + | inPlaceResults: make(chan inPlaceResult, 1), | |||
| 441 | } | 445 | } | ||
| 442 | } | 446 | } | ||
| 443 | 447 | ||||
| @@ -611,6 +615,7 @@ func (h *attachmentHost) newForeground(token AttachmentToken, stream ports.Broke | |||||
| 611 | tabSelect: make(chan domain.TabStableID, 1), | 615 | tabSelect: make(chan domain.TabStableID, 1), | ||
| 612 | routes: make(chan protocol.RecentRouteSnapshot, 1), | 616 | routes: make(chan protocol.RecentRouteSnapshot, 1), | ||
| 613 | replies: make(chan protocol.ClientMessage, attachmentReplyCapacity), | 617 | replies: make(chan protocol.ClientMessage, attachmentReplyCapacity), | ||
| 618 | + | inPlace: make(chan inPlaceSwitch, 1), | |||
| 614 | } | 619 | } | ||
| 615 | } | 620 | } | ||
| 616 | 621 | ||||
| @@ -624,6 +629,7 @@ func (h *attachmentHost) drainForegroundSignals() { | |||||
| 624 | case <-h.routeDemand: | 629 | case <-h.routeDemand: | ||
| 625 | case <-h.navigations: | 630 | case <-h.navigations: | ||
| 626 | case <-h.overlayActions: | 631 | case <-h.overlayActions: | ||
| 632 | + | case <-h.inPlaceResults: | |||
| 627 | default: | 633 | default: | ||
| 628 | return | 634 | return | ||
| 629 | } | 635 | } | ||
| @@ -654,14 +660,14 @@ func (h *attachmentHost) beginNavigationOverlay(sink pickerInputConsumer) bool { | |||||
| 654 | return fg.setOverlay(attachmentOverlayNavigation, sink) | 660 | return fg.setOverlay(attachmentOverlayNavigation, sink) | ||
| 655 | } | 661 | } | ||
| 656 | 662 | ||||
| 657 | - | // endNavigationOverlay returns the terminal to the current foreground and asks | 663 | + | // endNavigationOverlay returns the terminal to the current foreground. With |
| 658 | - | // its worker for an authoritative repaint over the picker box. | 664 | + | // repaint it asks the worker for an authoritative repaint over the picker box. |
| 659 | - | func (h *attachmentHost) endNavigationOverlay() { | 665 | + | func (h *attachmentHost) endNavigationOverlay(repaint bool) { |
| 660 | if h == nil { | 666 | if h == nil { | ||
| 661 | return | 667 | return | ||
| 662 | } | 668 | } | ||
| 663 | if fg := h.authority.foreground(); fg != nil { | 669 | if fg := h.authority.foreground(); fg != nil { | ||
| 664 | - | fg.clearOverlay(attachmentOverlayNavigation) | 670 | + | fg.releaseOverlay(attachmentOverlayNavigation, repaint) |
| 665 | } | 671 | } | ||
| 666 | } | 672 | } | ||
| 667 | 673 | ||||
| @@ -761,6 +767,8 @@ type attachmentOverlayForeground interface { | |||||
| 761 | routeSnapshots() <-chan protocol.RecentRouteSnapshot | 767 | routeSnapshots() <-chan protocol.RecentRouteSnapshot | ||
| 762 | navigationReplies() <-chan protocol.ClientMessage | 768 | navigationReplies() <-chan protocol.ClientMessage | ||
| 763 | requestNavigation(message protocol.ServerMessage) | 769 | requestNavigation(message protocol.ServerMessage) | ||
| 770 | + | inPlaceSwitches() <-chan inPlaceSwitch | |||
| 771 | + | reportInPlace(seq uint64, outcome inPlaceOutcome) | |||
| 764 | } | 772 | } | ||
| 765 | 773 | ||||
| 766 | var _ attachmentOverlayForeground = (*attachmentForeground)(nil) | 774 | var _ attachmentOverlayForeground = (*attachmentForeground)(nil) | ||
| @@ -807,6 +815,11 @@ func (f *attachmentForeground) setOverlay(kind attachmentOverlayKind, sink picke | |||||
| 807 | // authoritative repaint: the terminal still shows the picker box and any | 815 | // authoritative repaint: the terminal still shows the picker box and any | ||
| 808 | // attachment frames it suppressed were never written. | 816 | // attachment frames it suppressed were never written. | ||
| 809 | func (f *attachmentForeground) clearOverlay(kind attachmentOverlayKind) { | 817 | func (f *attachmentForeground) clearOverlay(kind attachmentOverlayKind) { | ||
| 818 | + | f.releaseOverlay(kind, true) | |||
| 819 | + | } | |||
| 820 | + | ||||
| 821 | + | // releaseOverlay is clearOverlay with the repaint request optional. | |||
| 822 | + | func (f *attachmentForeground) releaseOverlay(kind attachmentOverlayKind, repaint bool) { | |||
| 810 | if f == nil { | 823 | if f == nil { | ||
| 811 | return | 824 | return | ||
| 812 | } | 825 | } | ||
| @@ -832,6 +845,9 @@ func (f *attachmentForeground) clearOverlay(kind attachmentOverlayKind) { | |||||
| 832 | f.overlayMu.Unlock() | 845 | f.overlayMu.Unlock() | ||
| 833 | return | 846 | return | ||
| 834 | } | 847 | } | ||
| 848 | + | if !repaint { | |||
| 849 | + | return | |||
| 850 | + | } | |||
| 835 | select { | 851 | select { | ||
| 836 | case f.repaint <- struct{}{}: | 852 | case f.repaint <- struct{}{}: | ||
| 837 | default: | 853 | default: | ||
| @@ -1191,6 +1207,8 @@ type attachmentForeground struct { | |||||
| 1191 | // carries the supervisor's navigation failures. | 1207 | // carries the supervisor's navigation failures. | ||
| 1192 | routes chan protocol.RecentRouteSnapshot | 1208 | routes chan protocol.RecentRouteSnapshot | ||
| 1193 | replies chan protocol.ClientMessage | 1209 | replies chan protocol.ClientMessage | ||
| 1210 | + | // inPlace carries one pending supervisor in-place switch to the worker. | |||
| 1211 | + | inPlace chan inPlaceSwitch | |||
| 1194 | } | 1212 | } | ||
| 1195 | 1213 | ||||
| 1196 | // Token is the generation/attempt identity of this grant. | 1214 | // Token is the generation/attempt identity of this grant. | ||
M internal/usecase/client/picker_overlay_test.go +190 -40
| @@ -12,6 +12,7 @@ import ( | |||
| 12 | 12 | "github.com/bnema/vev/internal/domain" | |
| 13 | 13 | "github.com/bnema/vev/internal/ports" | |
| 14 | 14 | "github.com/bnema/vev/internal/protocol" | |
| 15 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |
| 15 | 16 | ) | |
| 16 | 17 | ||
| 17 | 18 | // Picker overlay and move picker tests (Plan 003 A1/A2). They drive the real | |
| @@ -54,6 +55,12 @@ func attachLiveSession(t *testing.T, harness *attachTestHarness, picker *attachT | |||
| 54 | 55 | return stream | |
| 55 | 56 | } | |
| 56 | 57 | ||
| 58 | + | // publishLiveLocal publishes a broker catalogue whose local daemon runs | |
| 59 | + | // target, so an overlay commit to it may switch in place. | |
| 60 | + | func publishLiveLocal(harness *attachTestHarness, target protocol.ExactSessionTarget) { | |
| 61 | + | harness.service.publishSnapshot(routeTestSnapshot(routeTestLocal(pickerTestSessionWithLifecycle(target.SessionName, target.LifecycleID)))) | |
| 62 | + | } | |
| 63 | + | ||
| 57 | 64 | func awaitPresentation(t *testing.T, sup *Supervisor, want Presentation) { | |
| 58 | 65 | t.Helper() | |
| 59 | 66 | require.Eventually(t, func() bool { return sup.State().Presentation == want }, 5*time.Second, time.Millisecond, "presentation never became %s", want) | |
| @@ -165,45 +172,90 @@ func TestPickerOverlayReturnsToLiveAttachment(t *testing.T) { | |||
| 165 | 172 | } | |
| 166 | 173 | } | |
| 167 | 174 | ||
| 168 | - | // TestPickerOverlayCommitElsewhereSwapsAttachment pins the swap: a commit to | |
| 169 | - | // another session detaches the live attachment cleanly and attaches the | |
| 170 | - | // committed target through Connecting, without passing through the plain | |
| 175 | + | // TestPickerOverlayCommitElsewhereSwitchesAttachment pins how a commit to | |
| 176 | + | // another session leaves the live attachment. A session on the same daemon | |
| 177 | + | // moves the attachment in place with no new stream or Hello; if the daemon | |
| 178 | + | // refuses, or the session is on another daemon, the attachment detaches | |
| 179 | + | // cleanly and the target attaches through Connecting, never through the plain | |
| 171 | 180 | // picker. | |
| 172 | - | func TestPickerOverlayCommitElsewhereSwapsAttachment(t *testing.T) { | |
| 173 | - | picker := newAttachTestPicker() | |
| 174 | - | notices := make(chan LifecycleNotice, 8) | |
| 175 | - | harness := startAttachHarnessConfig(t, picker, func(cfg *SupervisorConfig) { | |
| 176 | - | cfg.NotifyLifecycle = func(notice LifecycleNotice) { notices <- notice } | |
| 177 | - | }) | |
| 178 | - | first := attachLiveSession(t, harness, picker) | |
| 179 | - | first.deliver(navigationOffer(1)) | |
| 180 | - | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |
| 181 | + | func TestPickerOverlayCommitElsewhereSwitchesAttachment(t *testing.T) { | |
| 182 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 183 | + | tests := []struct { | |
| 184 | + | name string | |
| 185 | + | local bool | |
| 186 | + | stopped bool // the catalogue shows the target stopped | |
| 187 | + | inPlace bool // the daemon accepts the in-place switch; false refuses it | |
| 188 | + | }{ | |
| 189 | + | {name: "same daemon switches in place", local: true, inPlace: true}, | |
| 190 | + | {name: "a refused in-place switch reconnects", local: true}, | |
| 191 | + | {name: "a stopped session reconnects at once", local: true, stopped: true}, | |
| 192 | + | {name: "another daemon reconnects"}, | |
| 193 | + | } | |
| 194 | + | for _, tt := range tests { | |
| 195 | + | t.Run(tt.name, func(t *testing.T) { | |
| 196 | + | picker := newAttachTestPicker() | |
| 197 | + | notices := make(chan LifecycleNotice, 8) | |
| 198 | + | harness := startAttachHarnessConfig(t, picker, func(cfg *SupervisorConfig) { | |
| 199 | + | cfg.NotifyLifecycle = func(notice LifecycleNotice) { notices <- notice } | |
| 200 | + | }) | |
| 201 | + | if tt.stopped { | |
| 202 | + | harness.service.publishSnapshot(routeTestSnapshot(routeTestLocal(catalogue.RemoteCatalogSession{LifecycleID: beta.LifecycleID, Name: beta.SessionName, State: catalogue.RemoteCatalogSessionDown}))) | |
| 203 | + | } else { | |
| 204 | + | publishLiveLocal(harness, beta) | |
| 205 | + | } | |
| 206 | + | first := attachLiveSession(t, harness, picker) | |
| 207 | + | foreground := harness.sup.attachments.authority.foreground() | |
| 208 | + | first.deliver(navigationOffer(1)) | |
| 209 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |
| 181 | 210 | ||
| 182 | - | var mu sync.Mutex | |
| 183 | - | second := newSessionTestStream() | |
| 184 | - | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 185 | - | return second, nil | |
| 186 | - | }) | |
| 187 | - | other := sessionTestRequest(true) | |
| 188 | - | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 189 | - | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 190 | - | ||
| 191 | - | awaitSent(t, first, "Detach", isSent[protocol.Detach]) | |
| 192 | - | awaitStreamHello(t, &mu, &second) | |
| 193 | - | second.deliver(protocol.Welcome{SessionName: "beta"}) | |
| 194 | - | output := sessionTestOutput(1, "\x1b[Hbeta") | |
| 195 | - | output.Context.Route.Target = other.Target | |
| 196 | - | second.deliver(output) | |
| 197 | - | awaitAttachedState(t, harness.sup) | |
| 211 | + | var mu sync.Mutex | |
| 212 | + | second := newSessionTestStream() | |
| 213 | + | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 214 | + | return second, nil | |
| 215 | + | }) | |
| 216 | + | other := sessionTestRequest(tt.local) | |
| 217 | + | other.Target = beta | |
| 218 | + | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 219 | + | ||
| 220 | + | if tt.local && !tt.stopped { | |
| 221 | + | request := awaitSent(t, first, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |
| 222 | + | require.Equal(t, beta, request.Target) | |
| 223 | + | awaitPresentation(t, harness.sup, PresentAttached) | |
| 224 | + | if tt.inPlace { | |
| 225 | + | destination := laterOutput(2, 2, "\x1b[Hbeta") | |
| 226 | + | destination.Context.Route.Target = beta | |
| 227 | + | first.deliver(destination) | |
| 228 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "beta") }, 5*time.Second, time.Millisecond) | |
| 229 | + | require.Same(t, foreground, harness.sup.attachments.authority.foreground(), "an in-place switch keeps the attachment") | |
| 230 | + | require.Len(t, harness.service.openedRequests(), 1, "an in-place switch never reconnects") | |
| 231 | + | require.Zero(t, countSent[protocol.Detach](first)) | |
| 232 | + | require.Zero(t, countSent[protocol.OutputResetRequest](first), "an in-place switch never repaints the source") | |
| 233 | + | return | |
| 234 | + | } | |
| 235 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: request.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |
| 236 | + | } | |
| 198 | 237 | ||
| 199 | - | opened := harness.service.openedRequests() | |
| 200 | - | require.Len(t, opened, 2) | |
| 201 | - | require.Equal(t, other.Target, opened[1].Target) | |
| 202 | - | require.True(t, first.closedNow(), "the previous attachment is released") | |
| 203 | - | select { | |
| 204 | - | case notice := <-notices: | |
| 205 | - | t.Fatalf("a swap must not report a lifecycle transition to the picker: %+v", notice) | |
| 206 | - | default: | |
| 238 | + | awaitSent(t, first, "Detach", isSent[protocol.Detach]) | |
| 239 | + | awaitStreamHello(t, &mu, &second) | |
| 240 | + | second.deliver(protocol.Welcome{SessionName: "beta"}) | |
| 241 | + | output := sessionTestOutput(1, "\x1b[Hbeta") | |
| 242 | + | output.Context.Route.Target = other.Target | |
| 243 | + | second.deliver(output) | |
| 244 | + | awaitAttachedState(t, harness.sup) | |
| 245 | + | ||
| 246 | + | opened := harness.service.openedRequests() | |
| 247 | + | require.Len(t, opened, 2) | |
| 248 | + | require.Equal(t, other.Target, opened[1].Target) | |
| 249 | + | require.True(t, first.closedNow(), "the previous attachment is released") | |
| 250 | + | if tt.stopped || !tt.local { | |
| 251 | + | require.Zero(t, countSent[protocol.SamePeerSwitchRequest](first), "only a live session on the serving daemon is tried in place") | |
| 252 | + | } | |
| 253 | + | select { | |
| 254 | + | case notice := <-notices: | |
| 255 | + | t.Fatalf("a swap must not report a lifecycle transition to the picker: %+v", notice) | |
| 256 | + | default: | |
| 257 | + | } | |
| 258 | + | }) | |
| 207 | 259 | } | |
| 208 | 260 | } | |
| 209 | 261 | ||
| @@ -226,13 +278,17 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||
| 226 | 278 | overlayFirst bool | |
| 227 | 279 | keys []string | |
| 228 | 280 | // commit scripts a picker commit elsewhere for the consumed read. | |
| 229 | - | commit bool | |
| 281 | + | commit bool | |
| 282 | + | // inPlace commits a session on the serving daemon, which switches the | |
| 283 | + | // live attachment instead of reconnecting. | |
| 284 | + | inPlace bool | |
| 230 | 285 | wantTarget protocol.ExactSessionTarget | |
| 231 | 286 | wantFence bool | |
| 232 | 287 | }{ | |
| 233 | 288 | {name: "the opening action settles on its suppressed receipt", keys: []string{"Enter"}, wantTarget: alpha, wantFence: true}, | |
| 234 | 289 | {name: "a picker-consumed action settles without a daemon fence", overlayFirst: true, keys: []string{"j"}, wantTarget: alpha}, | |
| 235 | 290 | {name: "a picker commit elsewhere settles on the destination", overlayFirst: true, keys: []string{"Enter"}, commit: true, wantTarget: beta}, | |
| 291 | + | {name: "an in-place picker commit settles on the destination", overlayFirst: true, keys: []string{"Enter"}, commit: true, inPlace: true, wantTarget: beta}, | |
| 236 | 292 | } | |
| 237 | 293 | for _, tt := range tests { | |
| 238 | 294 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -242,6 +298,7 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||
| 242 | 298 | ui = NewUI(cfg.Terminal.(ports.UIState), cfg.Clock) | |
| 243 | 299 | cfg.UI = ui | |
| 244 | 300 | }) | |
| 301 | + | publishLiveLocal(harness, beta) | |
| 245 | 302 | stream := attachLiveSession(t, harness, picker) | |
| 246 | 303 | attached := sessionTestOutput(2, "\x1b[Hattached") | |
| 247 | 304 | attached.Full, attached.Base, attached.New = false, 1, 2 | |
| @@ -269,7 +326,8 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||
| 269 | 326 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 270 | 327 | return second, nil | |
| 271 | 328 | }) | |
| 272 | - | other := sessionTestRequest(true) | |
| 329 | + | // A remote target is another daemon, so it swaps. | |
| 330 | + | other := sessionTestRequest(tt.inPlace) | |
| 273 | 331 | other.Target = beta | |
| 274 | 332 | picker.decideOnConsume(pickerOp{commit: true}, other) | |
| 275 | 333 | } | |
| @@ -298,7 +356,12 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||
| 298 | 356 | }, 5*time.Second, time.Millisecond, "the suppressed output was never applied") | |
| 299 | 357 | stream.deliver(protocol.UIReceipt{ActionID: fence.ActionID, Epoch: 1, State: 3, ViewPublication: 3, Outcome: protocol.UIReceiptProcessed}) | |
| 300 | 358 | } | |
| 301 | - | if tt.commit { | |
| 359 | + | if tt.inPlace { | |
| 360 | + | awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]) | |
| 361 | + | destination := laterOutput(2, 3, "\x1b[Hbeta") | |
| 362 | + | destination.Context.Route.Target = beta | |
| 363 | + | stream.deliver(destination) | |
| 364 | + | } else if tt.commit { | |
| 302 | 365 | awaitSent(t, stream, "Detach", isSent[protocol.Detach]) | |
| 303 | 366 | awaitStreamHello(t, &mu, &second) | |
| 304 | 367 | second.deliver(protocol.Welcome{SessionName: "beta"}) | |
| @@ -325,7 +388,10 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||
| 325 | 388 | require.Equal(t, fences, countSent[protocol.UIFence](stream), "a picker-consumed action never fences through the daemon") | |
| 326 | 389 | require.Zero(t, countSent[protocol.Input](stream), "picker input never reaches the session") | |
| 327 | 390 | } | |
| 328 | - | if tt.commit { | |
| 391 | + | if tt.inPlace { | |
| 392 | + | require.Equal(t, snapshot.Context.Generation, outcome.result.Context.Generation, "an in-place switch keeps the generation") | |
| 393 | + | require.Len(t, harness.service.openedRequests(), 1, "an in-place switch never reconnects") | |
| 394 | + | } else if tt.commit { | |
| 329 | 395 | require.Greater(t, outcome.result.Context.Generation, snapshot.Context.Generation, "the swap commits a new generation") | |
| 330 | 396 | opened := harness.service.openedRequests() | |
| 331 | 397 | require.Len(t, opened, 2) | |
| @@ -601,3 +667,87 @@ func TestPickerControllerLoneEscapeResolvesOnItsWindow(t *testing.T) { | |||
| 601 | 667 | }) | |
| 602 | 668 | } | |
| 603 | 669 | } | |
| 670 | + | ||
| 671 | + | // TestPickerOverlayInPlaceSupersededByDaemonOffer pins that a daemon same-peer | |
| 672 | + | // offer arriving while a picker in-place switch is pending wins: the picker's | |
| 673 | + | // switch is dropped, never turned into a reconnect. If the daemon then keeps | |
| 674 | + | // the source, the source is repainted over the closed picker box. | |
| 675 | + | func TestPickerOverlayInPlaceSupersededByDaemonOffer(t *testing.T) { | |
| 676 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 677 | + | gamma := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{3}, SessionName: "gamma"} | |
| 678 | + | tests := []struct { | |
| 679 | + | name string | |
| 680 | + | refuseOffer bool | |
| 681 | + | wantRepaints int | |
| 682 | + | }{ | |
| 683 | + | {name: "the daemon offer arrives"}, | |
| 684 | + | {name: "the daemon offer is refused", refuseOffer: true, wantRepaints: 1}, | |
| 685 | + | } | |
| 686 | + | for _, tt := range tests { | |
| 687 | + | t.Run(tt.name, func(t *testing.T) { | |
| 688 | + | picker := newAttachTestPicker() | |
| 689 | + | harness := startAttachHarness(t, picker) | |
| 690 | + | publishLiveLocal(harness, beta) | |
| 691 | + | first := attachLiveSession(t, harness, picker) | |
| 692 | + | first.deliver(navigationOffer(1)) | |
| 693 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |
| 694 | + | ||
| 695 | + | other := sessionTestRequest(true) | |
| 696 | + | other.Target = beta | |
| 697 | + | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 698 | + | toBeta := awaitSent(t, first, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |
| 699 | + | ||
| 700 | + | first.deliver(protocol.AttachTarget{Session: gamma.SessionName, Intent: protocol.IntentAttach, ExactTarget: &gamma, EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, SamePeer: true}) | |
| 701 | + | var toGamma protocol.SamePeerSwitchRequest | |
| 702 | + | require.Eventually(t, func() bool { | |
| 703 | + | for _, message := range first.messages() { | |
| 704 | + | if request, ok := message.(protocol.SamePeerSwitchRequest); ok && request.Target == gamma { | |
| 705 | + | toGamma = request | |
| 706 | + | return true | |
| 707 | + | } | |
| 708 | + | } | |
| 709 | + | return false | |
| 710 | + | }, 5*time.Second, time.Millisecond) | |
| 711 | + | // The daemon refuses the older choice because its own offer is pending. | |
| 712 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: toBeta.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |
| 713 | + | if tt.refuseOffer { | |
| 714 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: toGamma.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |
| 715 | + | awaitSent(t, first, "OutputResetRequest", isSent[protocol.OutputResetRequest]) | |
| 716 | + | } else { | |
| 717 | + | destination := laterOutput(2, 2, "\x1b[Hgamma") | |
| 718 | + | destination.Context.Route.Target = gamma | |
| 719 | + | first.deliver(destination) | |
| 720 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "gamma") }, 5*time.Second, time.Millisecond) | |
| 721 | + | } | |
| 722 | + | ||
| 723 | + | require.Len(t, harness.service.openedRequests(), 1, "a superseded in-place switch never reconnects") | |
| 724 | + | require.Zero(t, countSent[protocol.Detach](first)) | |
| 725 | + | require.Equal(t, tt.wantRepaints, countSent[protocol.OutputResetRequest](first)) | |
| 726 | + | }) | |
| 727 | + | } | |
| 728 | + | } | |
| 729 | + | ||
| 730 | + | // TestPickerOverlayRepeatInPlaceChoice pins that choosing the destination of | |
| 731 | + | // an in-flight in-place switch again is a close, not a second switch. | |
| 732 | + | func TestPickerOverlayRepeatInPlaceChoice(t *testing.T) { | |
| 733 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 734 | + | picker := newAttachTestPicker() | |
| 735 | + | harness := startAttachHarness(t, picker) | |
| 736 | + | publishLiveLocal(harness, beta) | |
| 737 | + | first := attachLiveSession(t, harness, picker) | |
| 738 | + | other := sessionTestRequest(true) | |
| 739 | + | other.Target = beta | |
| 740 | + | for id := range uint64(2) { | |
| 741 | + | first.deliver(navigationOffer(id + 1)) | |
| 742 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |
| 743 | + | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 744 | + | awaitPresentation(t, harness.sup, PresentAttached) | |
| 745 | + | } | |
| 746 | + | destination := laterOutput(2, 2, "\x1b[Hbeta") | |
| 747 | + | destination.Context.Route.Target = beta | |
| 748 | + | first.deliver(destination) | |
| 749 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "beta") }, 5*time.Second, time.Millisecond) | |
| 750 | + | ||
| 751 | + | require.Equal(t, 1, countSent[protocol.SamePeerSwitchRequest](first)) | |
| 752 | + | require.Len(t, harness.service.openedRequests(), 1) | |
| 753 | + | } | |
| @@ -12,6 +12,7 @@ import ( | |||||
| 12 | "github.com/bnema/vev/internal/domain" | 12 | "github.com/bnema/vev/internal/domain" | ||
| 13 | "github.com/bnema/vev/internal/ports" | 13 | "github.com/bnema/vev/internal/ports" | ||
| 14 | "github.com/bnema/vev/internal/protocol" | 14 | "github.com/bnema/vev/internal/protocol" | ||
| 15 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |||
| 15 | ) | 16 | ) | ||
| 16 | 17 | ||||
| 17 | // Picker overlay and move picker tests (Plan 003 A1/A2). They drive the real | 18 | // Picker overlay and move picker tests (Plan 003 A1/A2). They drive the real | ||
| @@ -54,6 +55,12 @@ func attachLiveSession(t *testing.T, harness *attachTestHarness, picker *attachT | |||||
| 54 | return stream | 55 | return stream | ||
| 55 | } | 56 | } | ||
| 56 | 57 | ||||
| 58 | + | // publishLiveLocal publishes a broker catalogue whose local daemon runs | |||
| 59 | + | // target, so an overlay commit to it may switch in place. | |||
| 60 | + | func publishLiveLocal(harness *attachTestHarness, target protocol.ExactSessionTarget) { | |||
| 61 | + | harness.service.publishSnapshot(routeTestSnapshot(routeTestLocal(pickerTestSessionWithLifecycle(target.SessionName, target.LifecycleID)))) | |||
| 62 | + | } | |||
| 63 | + | ||||
| 57 | func awaitPresentation(t *testing.T, sup *Supervisor, want Presentation) { | 64 | func awaitPresentation(t *testing.T, sup *Supervisor, want Presentation) { | ||
| 58 | t.Helper() | 65 | t.Helper() | ||
| 59 | require.Eventually(t, func() bool { return sup.State().Presentation == want }, 5*time.Second, time.Millisecond, "presentation never became %s", want) | 66 | require.Eventually(t, func() bool { return sup.State().Presentation == want }, 5*time.Second, time.Millisecond, "presentation never became %s", want) | ||
| @@ -165,45 +172,90 @@ func TestPickerOverlayReturnsToLiveAttachment(t *testing.T) { | |||||
| 165 | } | 172 | } | ||
| 166 | } | 173 | } | ||
| 167 | 174 | ||||
| 168 | - | // TestPickerOverlayCommitElsewhereSwapsAttachment pins the swap: a commit to | 175 | + | // TestPickerOverlayCommitElsewhereSwitchesAttachment pins how a commit to |
| 169 | - | // another session detaches the live attachment cleanly and attaches the | 176 | + | // another session leaves the live attachment. A session on the same daemon |
| 170 | - | // committed target through Connecting, without passing through the plain | 177 | + | // moves the attachment in place with no new stream or Hello; if the daemon |
| 178 | + | // refuses, or the session is on another daemon, the attachment detaches | |||
| 179 | + | // cleanly and the target attaches through Connecting, never through the plain | |||
| 171 | // picker. | 180 | // picker. | ||
| 172 | - | func TestPickerOverlayCommitElsewhereSwapsAttachment(t *testing.T) { | 181 | + | func TestPickerOverlayCommitElsewhereSwitchesAttachment(t *testing.T) { |
| 173 | - | picker := newAttachTestPicker() | 182 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} |
| 174 | - | notices := make(chan LifecycleNotice, 8) | 183 | + | tests := []struct { |
| 175 | - | harness := startAttachHarnessConfig(t, picker, func(cfg *SupervisorConfig) { | 184 | + | name string |
| 176 | - | cfg.NotifyLifecycle = func(notice LifecycleNotice) { notices <- notice } | 185 | + | local bool |
| 177 | - | }) | 186 | + | stopped bool // the catalogue shows the target stopped |
| 178 | - | first := attachLiveSession(t, harness, picker) | 187 | + | inPlace bool // the daemon accepts the in-place switch; false refuses it |
| 179 | - | first.deliver(navigationOffer(1)) | 188 | + | }{ |
| 180 | - | awaitPresentation(t, harness.sup, PresentAttachedPicker) | 189 | + | {name: "same daemon switches in place", local: true, inPlace: true}, |
| 190 | + | {name: "a refused in-place switch reconnects", local: true}, | |||
| 191 | + | {name: "a stopped session reconnects at once", local: true, stopped: true}, | |||
| 192 | + | {name: "another daemon reconnects"}, | |||
| 193 | + | } | |||
| 194 | + | for _, tt := range tests { | |||
| 195 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 196 | + | picker := newAttachTestPicker() | |||
| 197 | + | notices := make(chan LifecycleNotice, 8) | |||
| 198 | + | harness := startAttachHarnessConfig(t, picker, func(cfg *SupervisorConfig) { | |||
| 199 | + | cfg.NotifyLifecycle = func(notice LifecycleNotice) { notices <- notice } | |||
| 200 | + | }) | |||
| 201 | + | if tt.stopped { | |||
| 202 | + | harness.service.publishSnapshot(routeTestSnapshot(routeTestLocal(catalogue.RemoteCatalogSession{LifecycleID: beta.LifecycleID, Name: beta.SessionName, State: catalogue.RemoteCatalogSessionDown}))) | |||
| 203 | + | } else { | |||
| 204 | + | publishLiveLocal(harness, beta) | |||
| 205 | + | } | |||
| 206 | + | first := attachLiveSession(t, harness, picker) | |||
| 207 | + | foreground := harness.sup.attachments.authority.foreground() | |||
| 208 | + | first.deliver(navigationOffer(1)) | |||
| 209 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |||
| 181 | 210 | ||||
| 182 | - | var mu sync.Mutex | 211 | + | var mu sync.Mutex |
| 183 | - | second := newSessionTestStream() | 212 | + | second := newSessionTestStream() |
| 184 | - | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | 213 | + | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { |
| 185 | - | return second, nil | 214 | + | return second, nil |
| 186 | - | }) | 215 | + | }) |
| 187 | - | other := sessionTestRequest(true) | 216 | + | other := sessionTestRequest(tt.local) |
| 188 | - | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | 217 | + | other.Target = beta |
| 189 | - | picker.recordOp(pickerOp{commit: true}, "row", other) | 218 | + | picker.recordOp(pickerOp{commit: true}, "row", other) |
| 190 | - | 219 | + | ||
| 191 | - | awaitSent(t, first, "Detach", isSent[protocol.Detach]) | 220 | + | if tt.local && !tt.stopped { |
| 192 | - | awaitStreamHello(t, &mu, &second) | 221 | + | request := awaitSent(t, first, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) |
| 193 | - | second.deliver(protocol.Welcome{SessionName: "beta"}) | 222 | + | require.Equal(t, beta, request.Target) |
| 194 | - | output := sessionTestOutput(1, "\x1b[Hbeta") | 223 | + | awaitPresentation(t, harness.sup, PresentAttached) |
| 195 | - | output.Context.Route.Target = other.Target | 224 | + | if tt.inPlace { |
| 196 | - | second.deliver(output) | 225 | + | destination := laterOutput(2, 2, "\x1b[Hbeta") |
| 197 | - | awaitAttachedState(t, harness.sup) | 226 | + | destination.Context.Route.Target = beta |
| 227 | + | first.deliver(destination) | |||
| 228 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "beta") }, 5*time.Second, time.Millisecond) | |||
| 229 | + | require.Same(t, foreground, harness.sup.attachments.authority.foreground(), "an in-place switch keeps the attachment") | |||
| 230 | + | require.Len(t, harness.service.openedRequests(), 1, "an in-place switch never reconnects") | |||
| 231 | + | require.Zero(t, countSent[protocol.Detach](first)) | |||
| 232 | + | require.Zero(t, countSent[protocol.OutputResetRequest](first), "an in-place switch never repaints the source") | |||
| 233 | + | return | |||
| 234 | + | } | |||
| 235 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: request.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |||
| 236 | + | } | |||
| 198 | 237 | ||||
| 199 | - | opened := harness.service.openedRequests() | 238 | + | awaitSent(t, first, "Detach", isSent[protocol.Detach]) |
| 200 | - | require.Len(t, opened, 2) | 239 | + | awaitStreamHello(t, &mu, &second) |
| 201 | - | require.Equal(t, other.Target, opened[1].Target) | 240 | + | second.deliver(protocol.Welcome{SessionName: "beta"}) |
| 202 | - | require.True(t, first.closedNow(), "the previous attachment is released") | 241 | + | output := sessionTestOutput(1, "\x1b[Hbeta") |
| 203 | - | select { | 242 | + | output.Context.Route.Target = other.Target |
| 204 | - | case notice := <-notices: | 243 | + | second.deliver(output) |
| 205 | - | t.Fatalf("a swap must not report a lifecycle transition to the picker: %+v", notice) | 244 | + | awaitAttachedState(t, harness.sup) |
| 206 | - | default: | 245 | + | |
| 246 | + | opened := harness.service.openedRequests() | |||
| 247 | + | require.Len(t, opened, 2) | |||
| 248 | + | require.Equal(t, other.Target, opened[1].Target) | |||
| 249 | + | require.True(t, first.closedNow(), "the previous attachment is released") | |||
| 250 | + | if tt.stopped || !tt.local { | |||
| 251 | + | require.Zero(t, countSent[protocol.SamePeerSwitchRequest](first), "only a live session on the serving daemon is tried in place") | |||
| 252 | + | } | |||
| 253 | + | select { | |||
| 254 | + | case notice := <-notices: | |||
| 255 | + | t.Fatalf("a swap must not report a lifecycle transition to the picker: %+v", notice) | |||
| 256 | + | default: | |||
| 257 | + | } | |||
| 258 | + | }) | |||
| 207 | } | 259 | } | ||
| 208 | } | 260 | } | ||
| 209 | 261 | ||||
| @@ -226,13 +278,17 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||||
| 226 | overlayFirst bool | 278 | overlayFirst bool | ||
| 227 | keys []string | 279 | keys []string | ||
| 228 | // commit scripts a picker commit elsewhere for the consumed read. | 280 | // commit scripts a picker commit elsewhere for the consumed read. | ||
| 229 | - | commit bool | 281 | + | commit bool |
| 282 | + | // inPlace commits a session on the serving daemon, which switches the | |||
| 283 | + | // live attachment instead of reconnecting. | |||
| 284 | + | inPlace bool | |||
| 230 | wantTarget protocol.ExactSessionTarget | 285 | wantTarget protocol.ExactSessionTarget | ||
| 231 | wantFence bool | 286 | wantFence bool | ||
| 232 | }{ | 287 | }{ | ||
| 233 | {name: "the opening action settles on its suppressed receipt", keys: []string{"Enter"}, wantTarget: alpha, wantFence: true}, | 288 | {name: "the opening action settles on its suppressed receipt", keys: []string{"Enter"}, wantTarget: alpha, wantFence: true}, | ||
| 234 | {name: "a picker-consumed action settles without a daemon fence", overlayFirst: true, keys: []string{"j"}, wantTarget: alpha}, | 289 | {name: "a picker-consumed action settles without a daemon fence", overlayFirst: true, keys: []string{"j"}, wantTarget: alpha}, | ||
| 235 | {name: "a picker commit elsewhere settles on the destination", overlayFirst: true, keys: []string{"Enter"}, commit: true, wantTarget: beta}, | 290 | {name: "a picker commit elsewhere settles on the destination", overlayFirst: true, keys: []string{"Enter"}, commit: true, wantTarget: beta}, | ||
| 291 | + | {name: "an in-place picker commit settles on the destination", overlayFirst: true, keys: []string{"Enter"}, commit: true, inPlace: true, wantTarget: beta}, | |||
| 236 | } | 292 | } | ||
| 237 | for _, tt := range tests { | 293 | for _, tt := range tests { | ||
| 238 | t.Run(tt.name, func(t *testing.T) { | 294 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -242,6 +298,7 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||||
| 242 | ui = NewUI(cfg.Terminal.(ports.UIState), cfg.Clock) | 298 | ui = NewUI(cfg.Terminal.(ports.UIState), cfg.Clock) | ||
| 243 | cfg.UI = ui | 299 | cfg.UI = ui | ||
| 244 | }) | 300 | }) | ||
| 301 | + | publishLiveLocal(harness, beta) | |||
| 245 | stream := attachLiveSession(t, harness, picker) | 302 | stream := attachLiveSession(t, harness, picker) | ||
| 246 | attached := sessionTestOutput(2, "\x1b[Hattached") | 303 | attached := sessionTestOutput(2, "\x1b[Hattached") | ||
| 247 | attached.Full, attached.Base, attached.New = false, 1, 2 | 304 | attached.Full, attached.Base, attached.New = false, 1, 2 | ||
| @@ -269,7 +326,8 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||||
| 269 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | 326 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | ||
| 270 | return second, nil | 327 | return second, nil | ||
| 271 | }) | 328 | }) | ||
| 272 | - | other := sessionTestRequest(true) | 329 | + | // A remote target is another daemon, so it swaps. |
| 330 | + | other := sessionTestRequest(tt.inPlace) | |||
| 273 | other.Target = beta | 331 | other.Target = beta | ||
| 274 | picker.decideOnConsume(pickerOp{commit: true}, other) | 332 | picker.decideOnConsume(pickerOp{commit: true}, other) | ||
| 275 | } | 333 | } | ||
| @@ -298,7 +356,12 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||||
| 298 | }, 5*time.Second, time.Millisecond, "the suppressed output was never applied") | 356 | }, 5*time.Second, time.Millisecond, "the suppressed output was never applied") | ||
| 299 | stream.deliver(protocol.UIReceipt{ActionID: fence.ActionID, Epoch: 1, State: 3, ViewPublication: 3, Outcome: protocol.UIReceiptProcessed}) | 357 | stream.deliver(protocol.UIReceipt{ActionID: fence.ActionID, Epoch: 1, State: 3, ViewPublication: 3, Outcome: protocol.UIReceiptProcessed}) | ||
| 300 | } | 358 | } | ||
| 301 | - | if tt.commit { | 359 | + | if tt.inPlace { |
| 360 | + | awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]) | |||
| 361 | + | destination := laterOutput(2, 3, "\x1b[Hbeta") | |||
| 362 | + | destination.Context.Route.Target = beta | |||
| 363 | + | stream.deliver(destination) | |||
| 364 | + | } else if tt.commit { | |||
| 302 | awaitSent(t, stream, "Detach", isSent[protocol.Detach]) | 365 | awaitSent(t, stream, "Detach", isSent[protocol.Detach]) | ||
| 303 | awaitStreamHello(t, &mu, &second) | 366 | awaitStreamHello(t, &mu, &second) | ||
| 304 | second.deliver(protocol.Welcome{SessionName: "beta"}) | 367 | second.deliver(protocol.Welcome{SessionName: "beta"}) | ||
| @@ -325,7 +388,10 @@ func TestPickerOverlaySettlesUIActions(t *testing.T) { | |||||
| 325 | require.Equal(t, fences, countSent[protocol.UIFence](stream), "a picker-consumed action never fences through the daemon") | 388 | require.Equal(t, fences, countSent[protocol.UIFence](stream), "a picker-consumed action never fences through the daemon") | ||
| 326 | require.Zero(t, countSent[protocol.Input](stream), "picker input never reaches the session") | 389 | require.Zero(t, countSent[protocol.Input](stream), "picker input never reaches the session") | ||
| 327 | } | 390 | } | ||
| 328 | - | if tt.commit { | 391 | + | if tt.inPlace { |
| 392 | + | require.Equal(t, snapshot.Context.Generation, outcome.result.Context.Generation, "an in-place switch keeps the generation") | |||
| 393 | + | require.Len(t, harness.service.openedRequests(), 1, "an in-place switch never reconnects") | |||
| 394 | + | } else if tt.commit { | |||
| 329 | require.Greater(t, outcome.result.Context.Generation, snapshot.Context.Generation, "the swap commits a new generation") | 395 | require.Greater(t, outcome.result.Context.Generation, snapshot.Context.Generation, "the swap commits a new generation") | ||
| 330 | opened := harness.service.openedRequests() | 396 | opened := harness.service.openedRequests() | ||
| 331 | require.Len(t, opened, 2) | 397 | require.Len(t, opened, 2) | ||
| @@ -601,3 +667,87 @@ func TestPickerControllerLoneEscapeResolvesOnItsWindow(t *testing.T) { | |||||
| 601 | }) | 667 | }) | ||
| 602 | } | 668 | } | ||
| 603 | } | 669 | } | ||
| 670 | + | ||||
| 671 | + | // TestPickerOverlayInPlaceSupersededByDaemonOffer pins that a daemon same-peer | |||
| 672 | + | // offer arriving while a picker in-place switch is pending wins: the picker's | |||
| 673 | + | // switch is dropped, never turned into a reconnect. If the daemon then keeps | |||
| 674 | + | // the source, the source is repainted over the closed picker box. | |||
| 675 | + | func TestPickerOverlayInPlaceSupersededByDaemonOffer(t *testing.T) { | |||
| 676 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |||
| 677 | + | gamma := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{3}, SessionName: "gamma"} | |||
| 678 | + | tests := []struct { | |||
| 679 | + | name string | |||
| 680 | + | refuseOffer bool | |||
| 681 | + | wantRepaints int | |||
| 682 | + | }{ | |||
| 683 | + | {name: "the daemon offer arrives"}, | |||
| 684 | + | {name: "the daemon offer is refused", refuseOffer: true, wantRepaints: 1}, | |||
| 685 | + | } | |||
| 686 | + | for _, tt := range tests { | |||
| 687 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 688 | + | picker := newAttachTestPicker() | |||
| 689 | + | harness := startAttachHarness(t, picker) | |||
| 690 | + | publishLiveLocal(harness, beta) | |||
| 691 | + | first := attachLiveSession(t, harness, picker) | |||
| 692 | + | first.deliver(navigationOffer(1)) | |||
| 693 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |||
| 694 | + | ||||
| 695 | + | other := sessionTestRequest(true) | |||
| 696 | + | other.Target = beta | |||
| 697 | + | picker.recordOp(pickerOp{commit: true}, "row", other) | |||
| 698 | + | toBeta := awaitSent(t, first, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |||
| 699 | + | ||||
| 700 | + | first.deliver(protocol.AttachTarget{Session: gamma.SessionName, Intent: protocol.IntentAttach, ExactTarget: &gamma, EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, SamePeer: true}) | |||
| 701 | + | var toGamma protocol.SamePeerSwitchRequest | |||
| 702 | + | require.Eventually(t, func() bool { | |||
| 703 | + | for _, message := range first.messages() { | |||
| 704 | + | if request, ok := message.(protocol.SamePeerSwitchRequest); ok && request.Target == gamma { | |||
| 705 | + | toGamma = request | |||
| 706 | + | return true | |||
| 707 | + | } | |||
| 708 | + | } | |||
| 709 | + | return false | |||
| 710 | + | }, 5*time.Second, time.Millisecond) | |||
| 711 | + | // The daemon refuses the older choice because its own offer is pending. | |||
| 712 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: toBeta.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |||
| 713 | + | if tt.refuseOffer { | |||
| 714 | + | first.deliver(protocol.SamePeerSwitchFailure{RequestID: toGamma.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |||
| 715 | + | awaitSent(t, first, "OutputResetRequest", isSent[protocol.OutputResetRequest]) | |||
| 716 | + | } else { | |||
| 717 | + | destination := laterOutput(2, 2, "\x1b[Hgamma") | |||
| 718 | + | destination.Context.Route.Target = gamma | |||
| 719 | + | first.deliver(destination) | |||
| 720 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "gamma") }, 5*time.Second, time.Millisecond) | |||
| 721 | + | } | |||
| 722 | + | ||||
| 723 | + | require.Len(t, harness.service.openedRequests(), 1, "a superseded in-place switch never reconnects") | |||
| 724 | + | require.Zero(t, countSent[protocol.Detach](first)) | |||
| 725 | + | require.Equal(t, tt.wantRepaints, countSent[protocol.OutputResetRequest](first)) | |||
| 726 | + | }) | |||
| 727 | + | } | |||
| 728 | + | } | |||
| 729 | + | ||||
| 730 | + | // TestPickerOverlayRepeatInPlaceChoice pins that choosing the destination of | |||
| 731 | + | // an in-flight in-place switch again is a close, not a second switch. | |||
| 732 | + | func TestPickerOverlayRepeatInPlaceChoice(t *testing.T) { | |||
| 733 | + | beta := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |||
| 734 | + | picker := newAttachTestPicker() | |||
| 735 | + | harness := startAttachHarness(t, picker) | |||
| 736 | + | publishLiveLocal(harness, beta) | |||
| 737 | + | first := attachLiveSession(t, harness, picker) | |||
| 738 | + | other := sessionTestRequest(true) | |||
| 739 | + | other.Target = beta | |||
| 740 | + | for id := range uint64(2) { | |||
| 741 | + | first.deliver(navigationOffer(id + 1)) | |||
| 742 | + | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |||
| 743 | + | picker.recordOp(pickerOp{commit: true}, "row", other) | |||
| 744 | + | awaitPresentation(t, harness.sup, PresentAttached) | |||
| 745 | + | } | |||
| 746 | + | destination := laterOutput(2, 2, "\x1b[Hbeta") | |||
| 747 | + | destination.Context.Route.Target = beta | |||
| 748 | + | first.deliver(destination) | |||
| 749 | + | require.Eventually(t, func() bool { return strings.Contains(harness.terminal.written(), "beta") }, 5*time.Second, time.Millisecond) | |||
| 750 | + | ||||
| 751 | + | require.Equal(t, 1, countSent[protocol.SamePeerSwitchRequest](first)) | |||
| 752 | + | require.Len(t, harness.service.openedRequests(), 1) | |||
| 753 | + | } | |||
M internal/usecase/client/supervisor.go +21 -0
| @@ -149,6 +149,11 @@ type State struct { | |||
| 149 | 149 | // ReadyLost is set only when an established ready generation was lost and | |
| 150 | 150 | // cleared by the replacement's committed publication. | |
| 151 | 151 | ReadyLost bool | |
| 152 | + | // SlowConnecting marks a Connecting presentation that is expected to be | |
| 153 | + | // slow (a remote attachment or a resume), so its notice is shown at once. | |
| 154 | + | // The notice cannot be painted later: the attachment foreground owns the | |
| 155 | + | // terminal during the handshake. | |
| 156 | + | SlowConnecting bool | |
| 152 | 157 | } | |
| 153 | 158 | ||
| 154 | 159 | // supervisorEventKind is one input to the pure supervisor reducer. | |
| @@ -200,6 +205,9 @@ type supervisorEvent struct { | |||
| 200 | 205 | // picker, so the attempt presents Connecting instead of flashing the | |
| 201 | 206 | // picker until the navigation settles. | |
| 202 | 207 | navigating bool | |
| 208 | + | // slow marks a supervisorAttachBegin whose Connecting notice must not be | |
| 209 | + | // delayed; see State.SlowConnecting. | |
| 210 | + | slow bool | |
| 203 | 211 | } | |
| 204 | 212 | ||
| 205 | 213 | // reduceSupervisor is the pure connectivity/presentation reducer. It is | |
| @@ -209,6 +217,7 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||
| 209 | 217 | switch event.kind { | |
| 210 | 218 | case supervisorBeginAttempt: | |
| 211 | 219 | state.Presentation = PresentPicker | |
| 220 | + | state.SlowConnecting = false | |
| 212 | 221 | if event.navigating { | |
| 213 | 222 | state.Presentation = PresentConnecting | |
| 214 | 223 | } | |
| @@ -233,6 +242,7 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||
| 233 | 242 | // publication: the broker connection is still ready and the attempt | |
| 234 | 243 | // cadence is untouched. | |
| 235 | 244 | state.Presentation = PresentConnecting | |
| 245 | + | state.SlowConnecting = event.slow | |
| 236 | 246 | state.Err = nil | |
| 237 | 247 | case supervisorAttached: | |
| 238 | 248 | state.Presentation = PresentAttached | |
| @@ -271,6 +281,10 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||
| 271 | 281 | state.Connectivity = ConnectivityDisconnected | |
| 272 | 282 | state.Err = event.err | |
| 273 | 283 | } | |
| 284 | + | // SlowConnecting describes only the Connecting that AttachBegin entered. | |
| 285 | + | if state.Presentation != PresentConnecting { | |
| 286 | + | state.SlowConnecting = false | |
| 287 | + | } | |
| 274 | 288 | return state | |
| 275 | 289 | } | |
| 276 | 290 | ||
| @@ -462,6 +476,13 @@ type Supervisor struct { | |||
| 462 | 476 | // committed to another target while an attachment was live. It is only | |
| 463 | 477 | // touched from the run goroutine and consumed by settleAttachment. | |
| 464 | 478 | pendingSwap *pickerAttachmentTarget | |
| 479 | + | // pendingInPlace is the choice the live attachment is switching to in | |
| 480 | + | // place on its own daemon. A refusal turns it into pendingSwap. Only | |
| 481 | + | // touched from the run goroutine; cleared when the attachment settles. | |
| 482 | + | pendingInPlace *pendingInPlace | |
| 483 | + | // inPlaceSeq numbers in-place choices so a late outcome for an older one | |
| 484 | + | // is never applied to a newer one. | |
| 485 | + | inPlaceSeq uint64 | |
| 465 | 486 | // kills runs the picker's `x` operations off the run goroutine. | |
| 466 | 487 | kills pickerKills | |
| 467 | 488 | // routes is the client route ledger published to the serving daemon; | |
| @@ -149,6 +149,11 @@ type State struct { | |||||
| 149 | // ReadyLost is set only when an established ready generation was lost and | 149 | // ReadyLost is set only when an established ready generation was lost and | ||
| 150 | // cleared by the replacement's committed publication. | 150 | // cleared by the replacement's committed publication. | ||
| 151 | ReadyLost bool | 151 | ReadyLost bool | ||
| 152 | + | // SlowConnecting marks a Connecting presentation that is expected to be | |||
| 153 | + | // slow (a remote attachment or a resume), so its notice is shown at once. | |||
| 154 | + | // The notice cannot be painted later: the attachment foreground owns the | |||
| 155 | + | // terminal during the handshake. | |||
| 156 | + | SlowConnecting bool | |||
| 152 | } | 157 | } | ||
| 153 | 158 | ||||
| 154 | // supervisorEventKind is one input to the pure supervisor reducer. | 159 | // supervisorEventKind is one input to the pure supervisor reducer. | ||
| @@ -200,6 +205,9 @@ type supervisorEvent struct { | |||||
| 200 | // picker, so the attempt presents Connecting instead of flashing the | 205 | // picker, so the attempt presents Connecting instead of flashing the | ||
| 201 | // picker until the navigation settles. | 206 | // picker until the navigation settles. | ||
| 202 | navigating bool | 207 | navigating bool | ||
| 208 | + | // slow marks a supervisorAttachBegin whose Connecting notice must not be | |||
| 209 | + | // delayed; see State.SlowConnecting. | |||
| 210 | + | slow bool | |||
| 203 | } | 211 | } | ||
| 204 | 212 | ||||
| 205 | // reduceSupervisor is the pure connectivity/presentation reducer. It is | 213 | // reduceSupervisor is the pure connectivity/presentation reducer. It is | ||
| @@ -209,6 +217,7 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||||
| 209 | switch event.kind { | 217 | switch event.kind { | ||
| 210 | case supervisorBeginAttempt: | 218 | case supervisorBeginAttempt: | ||
| 211 | state.Presentation = PresentPicker | 219 | state.Presentation = PresentPicker | ||
| 220 | + | state.SlowConnecting = false | |||
| 212 | if event.navigating { | 221 | if event.navigating { | ||
| 213 | state.Presentation = PresentConnecting | 222 | state.Presentation = PresentConnecting | ||
| 214 | } | 223 | } | ||
| @@ -233,6 +242,7 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||||
| 233 | // publication: the broker connection is still ready and the attempt | 242 | // publication: the broker connection is still ready and the attempt | ||
| 234 | // cadence is untouched. | 243 | // cadence is untouched. | ||
| 235 | state.Presentation = PresentConnecting | 244 | state.Presentation = PresentConnecting | ||
| 245 | + | state.SlowConnecting = event.slow | |||
| 236 | state.Err = nil | 246 | state.Err = nil | ||
| 237 | case supervisorAttached: | 247 | case supervisorAttached: | ||
| 238 | state.Presentation = PresentAttached | 248 | state.Presentation = PresentAttached | ||
| @@ -271,6 +281,10 @@ func reduceSupervisor(state State, event supervisorEvent) State { | |||||
| 271 | state.Connectivity = ConnectivityDisconnected | 281 | state.Connectivity = ConnectivityDisconnected | ||
| 272 | state.Err = event.err | 282 | state.Err = event.err | ||
| 273 | } | 283 | } | ||
| 284 | + | // SlowConnecting describes only the Connecting that AttachBegin entered. | |||
| 285 | + | if state.Presentation != PresentConnecting { | |||
| 286 | + | state.SlowConnecting = false | |||
| 287 | + | } | |||
| 274 | return state | 288 | return state | ||
| 275 | } | 289 | } | ||
| 276 | 290 | ||||
| @@ -462,6 +476,13 @@ type Supervisor struct { | |||||
| 462 | // committed to another target while an attachment was live. It is only | 476 | // committed to another target while an attachment was live. It is only | ||
| 463 | // touched from the run goroutine and consumed by settleAttachment. | 477 | // touched from the run goroutine and consumed by settleAttachment. | ||
| 464 | pendingSwap *pickerAttachmentTarget | 478 | pendingSwap *pickerAttachmentTarget | ||
| 479 | + | // pendingInPlace is the choice the live attachment is switching to in | |||
| 480 | + | // place on its own daemon. A refusal turns it into pendingSwap. Only | |||
| 481 | + | // touched from the run goroutine; cleared when the attachment settles. | |||
| 482 | + | pendingInPlace *pendingInPlace | |||
| 483 | + | // inPlaceSeq numbers in-place choices so a late outcome for an older one | |||
| 484 | + | // is never applied to a newer one. | |||
| 485 | + | inPlaceSeq uint64 | |||
| 465 | // kills runs the picker's `x` operations off the run goroutine. | 486 | // kills runs the picker's `x` operations off the run goroutine. | ||
| 466 | kills pickerKills | 487 | kills pickerKills | ||
| 467 | // routes is the client route ledger published to the serving daemon; | 488 | // routes is the client route ledger published to the serving daemon; | ||
M internal/usecase/client/supervisor_attach.go +44 -3
| @@ -328,6 +328,13 @@ type pickerAttachmentTarget struct { | |||
| 328 | 328 | tab attachmentTab | |
| 329 | 329 | } | |
| 330 | 330 | ||
| 331 | + | // pendingInPlace is one in-place switch the supervisor is waiting on: its | |
| 332 | + | // choice number and the target to reconnect to if the daemon refuses. | |
| 333 | + | type pendingInPlace struct { | |
| 334 | + | seq uint64 | |
| 335 | + | target pickerAttachmentTarget | |
| 336 | + | } | |
| 337 | + | ||
| 331 | 338 | // newAttachmentWorker builds the real typed-session worker for one admitted | |
| 332 | 339 | // request. The worker receives no endpoint, route, or raw-mode authority: it | |
| 333 | 340 | // only drives the typed session protocol on the stream the supervisor opened. | |
| @@ -709,7 +716,12 @@ func (s *Supervisor) withFreshStream(service ports.BrokerNavigator, target picke | |||
| 709 | 716 | // attachmentRetry instead of presented. | |
| 710 | 717 | func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, target pickerAttachmentTarget, localProvenance SessionEnvironmentProvenance, resuming bool) attachmentOutcome { | |
| 711 | 718 | request := target.request | |
| 712 | - | s.transition(supervisorEvent{kind: supervisorAttachBegin}) | |
| 719 | + | // A live local session usually commits within the notice delay; a | |
| 720 | + | // remote handshake, a resume, or a restore may not, and nothing can paint | |
| 721 | + | // once the foreground owns the terminal, so their notice shows at once. | |
| 722 | + | slow := resuming || !request.Local || target.tab.stopped != nil || | |
| 723 | + | (request.Admission == ports.BrokerAdmissionExact && !catalogueSessionLive(service.Snapshot(), request)) | |
| 724 | + | s.transition(supervisorEvent{kind: supervisorAttachBegin, slow: slow}) | |
| 713 | 725 | deadline := startAttachmentDeadline(ctx, s.cfg.Clock) | |
| 714 | 726 | defer deadline.finish() | |
| 715 | 727 | ||
| @@ -744,7 +756,7 @@ func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLif | |||
| 744 | 756 | } | |
| 745 | 757 | }() | |
| 746 | 758 | } | |
| 747 | - | stream, err := service.OpenStream(openCtx, request) | |
| 759 | + | stream, err := s.openStreamTicking(openCtx, service, request) | |
| 748 | 760 | if keys != nil { | |
| 749 | 761 | keys.release() | |
| 750 | 762 | cancelOpen() | |
| @@ -805,6 +817,30 @@ func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLif | |||
| 805 | 817 | return s.settleAttachment(ctx, input, service, deadline, run, request, resuming) | |
| 806 | 818 | } | |
| 807 | 819 | ||
| 820 | + | // openStreamTicking opens one stream while advancing the Connecting spinner, | |
| 821 | + | // so a slow open still shows its delayed notice. No foreground owns the | |
| 822 | + | // terminal before Begin, so painting here never overwrites session output. | |
| 823 | + | // OpenStream honors ctx, so the wait always ends. | |
| 824 | + | func (s *Supervisor) openStreamTicking(ctx context.Context, service ports.BrokerNavigator, request ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 825 | + | type opened struct { | |
| 826 | + | stream ports.BrokerLogicalConnection | |
| 827 | + | err error | |
| 828 | + | } | |
| 829 | + | result := make(chan opened, 1) | |
| 830 | + | go func() { | |
| 831 | + | stream, err := service.OpenStream(ctx, request) | |
| 832 | + | result <- opened{stream: stream, err: err} | |
| 833 | + | }() | |
| 834 | + | for { | |
| 835 | + | select { | |
| 836 | + | case r := <-result: | |
| 837 | + | return r.stream, r.err | |
| 838 | + | case <-s.spinnerTick(): | |
| 839 | + | s.cfg.Spinner.AdvanceSpinner(s.State()) | |
| 840 | + | } | |
| 841 | + | } | |
| 842 | + | } | |
| 843 | + | ||
| 808 | 844 | // settleAttachment joins one admitted attachment while watching the parent | |
| 809 | 845 | // context, terminal EOF, and broker-connection loss. The deadline is the only | |
| 810 | 846 | // other stop: when it fires before the worker settled, the attachment ends as | |
| @@ -854,6 +890,8 @@ settlement: | |||
| 854 | 890 | overlay.takeOp() | |
| 855 | 891 | case consumed := <-s.attachments.OverlayActions(): | |
| 856 | 892 | overlay.settleAction(consumed) | |
| 893 | + | case result := <-s.attachments.InPlaceResults(): | |
| 894 | + | overlay.inPlaceSettled(result) | |
| 857 | 895 | case <-overlay.invalidation(): | |
| 858 | 896 | overlay.resize() | |
| 859 | 897 | case <-notices.arm(s.cfg.Picker): | |
| @@ -898,6 +936,9 @@ settlement: | |||
| 898 | 936 | // while this attachment was live; this attempt consumes it either way. | |
| 899 | 937 | swap := s.pendingSwap | |
| 900 | 938 | s.pendingSwap = nil | |
| 939 | + | // An in-place switch still in flight ended with its attachment; the | |
| 940 | + | // resume or picker path below decides what comes next. | |
| 941 | + | s.pendingInPlace = nil | |
| 901 | 942 | if terminated { | |
| 902 | 943 | return attachmentTerminatedOutcome(termErr) | |
| 903 | 944 | } | |
| @@ -998,7 +1039,7 @@ func resumeTarget(request ports.BrokerOpenStreamRequest, fg *attachmentForegroun | |||
| 998 | 1039 | // with Ctrl-C or Esc, which returns to the picker. Other keys typed during | |
| 999 | 1040 | // the outage are kept and replayed into the session once it resumes. | |
| 1000 | 1041 | func (s *Supervisor) waitResume(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, attempt int) (terminated bool, termErr error, stop bool) { | |
| 1001 | - | s.transition(supervisorEvent{kind: supervisorAttachBegin}) | |
| 1042 | + | s.transition(supervisorEvent{kind: supervisorAttachBegin, slow: true}) | |
| 1002 | 1043 | delay := supervisorBackoffDelay(uint64(attempt), s.cfg.Jitter) | |
| 1003 | 1044 | timer := s.cfg.Clock.NewTimer(delay) | |
| 1004 | 1045 | defer stopSupervisorTimer(timer) | |
| @@ -328,6 +328,13 @@ type pickerAttachmentTarget struct { | |||||
| 328 | tab attachmentTab | 328 | tab attachmentTab | ||
| 329 | } | 329 | } | ||
| 330 | 330 | ||||
| 331 | + | // pendingInPlace is one in-place switch the supervisor is waiting on: its | |||
| 332 | + | // choice number and the target to reconnect to if the daemon refuses. | |||
| 333 | + | type pendingInPlace struct { | |||
| 334 | + | seq uint64 | |||
| 335 | + | target pickerAttachmentTarget | |||
| 336 | + | } | |||
| 337 | + | ||||
| 331 | // newAttachmentWorker builds the real typed-session worker for one admitted | 338 | // newAttachmentWorker builds the real typed-session worker for one admitted | ||
| 332 | // request. The worker receives no endpoint, route, or raw-mode authority: it | 339 | // request. The worker receives no endpoint, route, or raw-mode authority: it | ||
| 333 | // only drives the typed session protocol on the stream the supervisor opened. | 340 | // only drives the typed session protocol on the stream the supervisor opened. | ||
| @@ -709,7 +716,12 @@ func (s *Supervisor) withFreshStream(service ports.BrokerNavigator, target picke | |||||
| 709 | // attachmentRetry instead of presented. | 716 | // attachmentRetry instead of presented. | ||
| 710 | func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, target pickerAttachmentTarget, localProvenance SessionEnvironmentProvenance, resuming bool) attachmentOutcome { | 717 | func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, target pickerAttachmentTarget, localProvenance SessionEnvironmentProvenance, resuming bool) attachmentOutcome { | ||
| 711 | request := target.request | 718 | request := target.request | ||
| 712 | - | s.transition(supervisorEvent{kind: supervisorAttachBegin}) | 719 | + | // A live local session usually commits within the notice delay; a |
| 720 | + | // remote handshake, a resume, or a restore may not, and nothing can paint | |||
| 721 | + | // once the foreground owns the terminal, so their notice shows at once. | |||
| 722 | + | slow := resuming || !request.Local || target.tab.stopped != nil || | |||
| 723 | + | (request.Admission == ports.BrokerAdmissionExact && !catalogueSessionLive(service.Snapshot(), request)) | |||
| 724 | + | s.transition(supervisorEvent{kind: supervisorAttachBegin, slow: slow}) | |||
| 713 | deadline := startAttachmentDeadline(ctx, s.cfg.Clock) | 725 | deadline := startAttachmentDeadline(ctx, s.cfg.Clock) | ||
| 714 | defer deadline.finish() | 726 | defer deadline.finish() | ||
| 715 | 727 | ||||
| @@ -744,7 +756,7 @@ func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLif | |||||
| 744 | } | 756 | } | ||
| 745 | }() | 757 | }() | ||
| 746 | } | 758 | } | ||
| 747 | - | stream, err := service.OpenStream(openCtx, request) | 759 | + | stream, err := s.openStreamTicking(openCtx, service, request) |
| 748 | if keys != nil { | 760 | if keys != nil { | ||
| 749 | keys.release() | 761 | keys.release() | ||
| 750 | cancelOpen() | 762 | cancelOpen() | ||
| @@ -805,6 +817,30 @@ func (s *Supervisor) attachResolved(ctx context.Context, input *terminalInputLif | |||||
| 805 | return s.settleAttachment(ctx, input, service, deadline, run, request, resuming) | 817 | return s.settleAttachment(ctx, input, service, deadline, run, request, resuming) | ||
| 806 | } | 818 | } | ||
| 807 | 819 | ||||
| 820 | + | // openStreamTicking opens one stream while advancing the Connecting spinner, | |||
| 821 | + | // so a slow open still shows its delayed notice. No foreground owns the | |||
| 822 | + | // terminal before Begin, so painting here never overwrites session output. | |||
| 823 | + | // OpenStream honors ctx, so the wait always ends. | |||
| 824 | + | func (s *Supervisor) openStreamTicking(ctx context.Context, service ports.BrokerNavigator, request ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |||
| 825 | + | type opened struct { | |||
| 826 | + | stream ports.BrokerLogicalConnection | |||
| 827 | + | err error | |||
| 828 | + | } | |||
| 829 | + | result := make(chan opened, 1) | |||
| 830 | + | go func() { | |||
| 831 | + | stream, err := service.OpenStream(ctx, request) | |||
| 832 | + | result <- opened{stream: stream, err: err} | |||
| 833 | + | }() | |||
| 834 | + | for { | |||
| 835 | + | select { | |||
| 836 | + | case r := <-result: | |||
| 837 | + | return r.stream, r.err | |||
| 838 | + | case <-s.spinnerTick(): | |||
| 839 | + | s.cfg.Spinner.AdvanceSpinner(s.State()) | |||
| 840 | + | } | |||
| 841 | + | } | |||
| 842 | + | } | |||
| 843 | + | ||||
| 808 | // settleAttachment joins one admitted attachment while watching the parent | 844 | // settleAttachment joins one admitted attachment while watching the parent | ||
| 809 | // context, terminal EOF, and broker-connection loss. The deadline is the only | 845 | // context, terminal EOF, and broker-connection loss. The deadline is the only | ||
| 810 | // other stop: when it fires before the worker settled, the attachment ends as | 846 | // other stop: when it fires before the worker settled, the attachment ends as | ||
| @@ -854,6 +890,8 @@ settlement: | |||||
| 854 | overlay.takeOp() | 890 | overlay.takeOp() | ||
| 855 | case consumed := <-s.attachments.OverlayActions(): | 891 | case consumed := <-s.attachments.OverlayActions(): | ||
| 856 | overlay.settleAction(consumed) | 892 | overlay.settleAction(consumed) | ||
| 893 | + | case result := <-s.attachments.InPlaceResults(): | |||
| 894 | + | overlay.inPlaceSettled(result) | |||
| 857 | case <-overlay.invalidation(): | 895 | case <-overlay.invalidation(): | ||
| 858 | overlay.resize() | 896 | overlay.resize() | ||
| 859 | case <-notices.arm(s.cfg.Picker): | 897 | case <-notices.arm(s.cfg.Picker): | ||
| @@ -898,6 +936,9 @@ settlement: | |||||
| 898 | // while this attachment was live; this attempt consumes it either way. | 936 | // while this attachment was live; this attempt consumes it either way. | ||
| 899 | swap := s.pendingSwap | 937 | swap := s.pendingSwap | ||
| 900 | s.pendingSwap = nil | 938 | s.pendingSwap = nil | ||
| 939 | + | // An in-place switch still in flight ended with its attachment; the | |||
| 940 | + | // resume or picker path below decides what comes next. | |||
| 941 | + | s.pendingInPlace = nil | |||
| 901 | if terminated { | 942 | if terminated { | ||
| 902 | return attachmentTerminatedOutcome(termErr) | 943 | return attachmentTerminatedOutcome(termErr) | ||
| 903 | } | 944 | } | ||
| @@ -998,7 +1039,7 @@ func resumeTarget(request ports.BrokerOpenStreamRequest, fg *attachmentForegroun | |||||
| 998 | // with Ctrl-C or Esc, which returns to the picker. Other keys typed during | 1039 | // with Ctrl-C or Esc, which returns to the picker. Other keys typed during | ||
| 999 | // the outage are kept and replayed into the session once it resumes. | 1040 | // the outage are kept and replayed into the session once it resumes. | ||
| 1000 | func (s *Supervisor) waitResume(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, attempt int) (terminated bool, termErr error, stop bool) { | 1041 | func (s *Supervisor) waitResume(ctx context.Context, input *terminalInputLifetime, service ports.BrokerNavigator, attempt int) (terminated bool, termErr error, stop bool) { | ||
| 1001 | - | s.transition(supervisorEvent{kind: supervisorAttachBegin}) | 1042 | + | s.transition(supervisorEvent{kind: supervisorAttachBegin, slow: true}) |
| 1002 | delay := supervisorBackoffDelay(uint64(attempt), s.cfg.Jitter) | 1043 | delay := supervisorBackoffDelay(uint64(attempt), s.cfg.Jitter) | ||
| 1003 | timer := s.cfg.Clock.NewTimer(delay) | 1044 | timer := s.cfg.Clock.NewTimer(delay) | ||
| 1004 | defer stopSupervisorTimer(timer) | 1045 | defer stopSupervisorTimer(timer) | ||
M internal/usecase/client/supervisor_attach_test.go +29 -4
| @@ -560,7 +560,7 @@ func TestSupervisorDetachedReasonDeterminesExitOrPicker(t *testing.T) { | |||
| 560 | 560 | } | |
| 561 | 561 | ||
| 562 | 562 | // TestSupervisorPendingSwapContinuesRatherThanExiting guards the overlay | |
| 563 | - | // commit-elsewhere path (same-peer switch) against being folded into the | |
| 563 | + | // commit-elsewhere path (cross-daemon swap) against being folded into the | |
| 564 | 564 | // explicit-detach-and-exit outcome above: the outgoing attachment ends | |
| 565 | 565 | // through a local lifecycle action, never a protocol.Detached message, and a | |
| 566 | 566 | // pendingSwap in flight must land the supervisor on the new target instead of | |
| @@ -580,7 +580,8 @@ func TestSupervisorPendingSwapContinuesRatherThanExiting(t *testing.T) { | |||
| 580 | 580 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 581 | 581 | return second, nil | |
| 582 | 582 | }) | |
| 583 | - | other := sessionTestRequest(true) | |
| 583 | + | // Another daemon: the commit swaps instead of switching in place. | |
| 584 | + | other := sessionTestRequest(false) | |
| 584 | 585 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 585 | 586 | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 586 | 587 | ||
| @@ -594,7 +595,7 @@ func TestSupervisorPendingSwapContinuesRatherThanExiting(t *testing.T) { | |||
| 594 | 595 | ||
| 595 | 596 | select { | |
| 596 | 597 | case <-harness.runDone: | |
| 597 | - | t.Fatal("a same-peer swap must not end the supervisor run") | |
| 598 | + | t.Fatal("a swap must not end the supervisor run") | |
| 598 | 599 | default: | |
| 599 | 600 | } | |
| 600 | 601 | select { | |
| @@ -630,7 +631,8 @@ func TestSupervisorSwapDropsKeysKeptForPreviousSession(t *testing.T) { | |||
| 630 | 631 | }) | |
| 631 | 632 | base.deliver(navigationOffer(1)) | |
| 632 | 633 | awaitPresentation(t, harness.sup, PresentAttachedPicker) | |
| 633 | - | other := sessionTestRequest(true) | |
| 634 | + | // Another daemon: the commit swaps instead of switching in place. | |
| 635 | + | other := sessionTestRequest(false) | |
| 634 | 636 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | |
| 635 | 637 | picker.recordOp(pickerOp{commit: true}, "row", other) | |
| 636 | 638 | // The Detach queues behind the stalled write; let the bounded drain expire | |
| @@ -692,6 +694,29 @@ func TestSupervisorReducerAttachmentPresentation(t *testing.T) { | |||
| 692 | 694 | require.ErrorIs(t, returned.Err, errAttachmentDeadline) | |
| 693 | 695 | } | |
| 694 | 696 | ||
| 697 | + | // TestSupervisorReducerSlowConnecting pins that SlowConnecting describes only | |
| 698 | + | // the Connecting an attachment entered, never a later one. | |
| 699 | + | func TestSupervisorReducerSlowConnecting(t *testing.T) { | |
| 700 | + | ready := State{Presentation: PresentPicker, Connectivity: ConnectivityReady, Generation: 2} | |
| 701 | + | slow := reduceSupervisor(ready, supervisorEvent{kind: supervisorAttachBegin, slow: true}) | |
| 702 | + | tests := []struct { | |
| 703 | + | name string | |
| 704 | + | event supervisorEvent | |
| 705 | + | want bool | |
| 706 | + | }{ | |
| 707 | + | {name: "a slow attach begins slow", event: supervisorEvent{kind: supervisorAttachBegin, slow: true}, want: true}, | |
| 708 | + | {name: "a fast attach begins fast", event: supervisorEvent{kind: supervisorAttachBegin}}, | |
| 709 | + | {name: "attaching clears it", event: supervisorEvent{kind: supervisorAttached}}, | |
| 710 | + | {name: "a navigating broker attempt clears it", event: supervisorEvent{kind: supervisorBeginAttempt, navigating: true}}, | |
| 711 | + | {name: "a broker loss while connecting keeps it", event: supervisorEvent{kind: supervisorBrokerLoss}, want: true}, | |
| 712 | + | } | |
| 713 | + | for _, tt := range tests { | |
| 714 | + | t.Run(tt.name, func(t *testing.T) { | |
| 715 | + | require.Equal(t, tt.want, reduceSupervisor(slow, tt.event).SlowConnecting) | |
| 716 | + | }) | |
| 717 | + | } | |
| 718 | + | } | |
| 719 | + | ||
| 695 | 720 | // TestSupervisorAttachmentLocalRemoteParity proves the same supervisor path | |
| 696 | 721 | // opens the exact resolved request for a local and a remote selection, with no | |
| 697 | 722 | // dialer choice in the client. | |
| @@ -560,7 +560,7 @@ func TestSupervisorDetachedReasonDeterminesExitOrPicker(t *testing.T) { | |||||
| 560 | } | 560 | } | ||
| 561 | 561 | ||||
| 562 | // TestSupervisorPendingSwapContinuesRatherThanExiting guards the overlay | 562 | // TestSupervisorPendingSwapContinuesRatherThanExiting guards the overlay | ||
| 563 | - | // commit-elsewhere path (same-peer switch) against being folded into the | 563 | + | // commit-elsewhere path (cross-daemon swap) against being folded into the |
| 564 | // explicit-detach-and-exit outcome above: the outgoing attachment ends | 564 | // explicit-detach-and-exit outcome above: the outgoing attachment ends | ||
| 565 | // through a local lifecycle action, never a protocol.Detached message, and a | 565 | // through a local lifecycle action, never a protocol.Detached message, and a | ||
| 566 | // pendingSwap in flight must land the supervisor on the new target instead of | 566 | // pendingSwap in flight must land the supervisor on the new target instead of | ||
| @@ -580,7 +580,8 @@ func TestSupervisorPendingSwapContinuesRatherThanExiting(t *testing.T) { | |||||
| 580 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | 580 | harness.service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | ||
| 581 | return second, nil | 581 | return second, nil | ||
| 582 | }) | 582 | }) | ||
| 583 | - | other := sessionTestRequest(true) | 583 | + | // Another daemon: the commit swaps instead of switching in place. |
| 584 | + | other := sessionTestRequest(false) | |||
| 584 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | 585 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | ||
| 585 | picker.recordOp(pickerOp{commit: true}, "row", other) | 586 | picker.recordOp(pickerOp{commit: true}, "row", other) | ||
| 586 | 587 | ||||
| @@ -594,7 +595,7 @@ func TestSupervisorPendingSwapContinuesRatherThanExiting(t *testing.T) { | |||||
| 594 | 595 | ||||
| 595 | select { | 596 | select { | ||
| 596 | case <-harness.runDone: | 597 | case <-harness.runDone: | ||
| 597 | - | t.Fatal("a same-peer swap must not end the supervisor run") | 598 | + | t.Fatal("a swap must not end the supervisor run") |
| 598 | default: | 599 | default: | ||
| 599 | } | 600 | } | ||
| 600 | select { | 601 | select { | ||
| @@ -630,7 +631,8 @@ func TestSupervisorSwapDropsKeysKeptForPreviousSession(t *testing.T) { | |||||
| 630 | }) | 631 | }) | ||
| 631 | base.deliver(navigationOffer(1)) | 632 | base.deliver(navigationOffer(1)) | ||
| 632 | awaitPresentation(t, harness.sup, PresentAttachedPicker) | 633 | awaitPresentation(t, harness.sup, PresentAttachedPicker) | ||
| 633 | - | other := sessionTestRequest(true) | 634 | + | // Another daemon: the commit swaps instead of switching in place. |
| 635 | + | other := sessionTestRequest(false) | |||
| 634 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | 636 | other.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "beta"} | ||
| 635 | picker.recordOp(pickerOp{commit: true}, "row", other) | 637 | picker.recordOp(pickerOp{commit: true}, "row", other) | ||
| 636 | // The Detach queues behind the stalled write; let the bounded drain expire | 638 | // The Detach queues behind the stalled write; let the bounded drain expire | ||
| @@ -692,6 +694,29 @@ func TestSupervisorReducerAttachmentPresentation(t *testing.T) { | |||||
| 692 | require.ErrorIs(t, returned.Err, errAttachmentDeadline) | 694 | require.ErrorIs(t, returned.Err, errAttachmentDeadline) | ||
| 693 | } | 695 | } | ||
| 694 | 696 | ||||
| 697 | + | // TestSupervisorReducerSlowConnecting pins that SlowConnecting describes only | |||
| 698 | + | // the Connecting an attachment entered, never a later one. | |||
| 699 | + | func TestSupervisorReducerSlowConnecting(t *testing.T) { | |||
| 700 | + | ready := State{Presentation: PresentPicker, Connectivity: ConnectivityReady, Generation: 2} | |||
| 701 | + | slow := reduceSupervisor(ready, supervisorEvent{kind: supervisorAttachBegin, slow: true}) | |||
| 702 | + | tests := []struct { | |||
| 703 | + | name string | |||
| 704 | + | event supervisorEvent | |||
| 705 | + | want bool | |||
| 706 | + | }{ | |||
| 707 | + | {name: "a slow attach begins slow", event: supervisorEvent{kind: supervisorAttachBegin, slow: true}, want: true}, | |||
| 708 | + | {name: "a fast attach begins fast", event: supervisorEvent{kind: supervisorAttachBegin}}, | |||
| 709 | + | {name: "attaching clears it", event: supervisorEvent{kind: supervisorAttached}}, | |||
| 710 | + | {name: "a navigating broker attempt clears it", event: supervisorEvent{kind: supervisorBeginAttempt, navigating: true}}, | |||
| 711 | + | {name: "a broker loss while connecting keeps it", event: supervisorEvent{kind: supervisorBrokerLoss}, want: true}, | |||
| 712 | + | } | |||
| 713 | + | for _, tt := range tests { | |||
| 714 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 715 | + | require.Equal(t, tt.want, reduceSupervisor(slow, tt.event).SlowConnecting) | |||
| 716 | + | }) | |||
| 717 | + | } | |||
| 718 | + | } | |||
| 719 | + | ||||
| 695 | // TestSupervisorAttachmentLocalRemoteParity proves the same supervisor path | 720 | // TestSupervisorAttachmentLocalRemoteParity proves the same supervisor path | ||
| 696 | // opens the exact resolved request for a local and a remote selection, with no | 721 | // opens the exact resolved request for a local and a remote selection, with no | ||
| 697 | // dialer choice in the client. | 722 | // dialer choice in the client. | ||
M internal/usecase/client/supervisor_overlay.go +82 -4
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "github.com/bnema/vev/internal/domain" | |
| 5 | 5 | "github.com/bnema/vev/internal/ports" | |
| 6 | 6 | "github.com/bnema/vev/internal/protocol" | |
| 7 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |
| 7 | 8 | ) | |
| 8 | 9 | ||
| 9 | 10 | // Picker overlay over a live attachment. | |
| @@ -88,14 +89,19 @@ func (o *attachmentPickerOverlay) enter() { | |||
| 88 | 89 | } | |
| 89 | 90 | ||
| 90 | 91 | // exit returns the terminal to the same live attachment. | |
| 91 | - | func (o *attachmentPickerOverlay) exit() { | |
| 92 | + | func (o *attachmentPickerOverlay) exit() { o.release(true) } | |
| 93 | + | ||
| 94 | + | // release returns the terminal to the live attachment. repaint asks the | |
| 95 | + | // daemon to repaint the attached session over the picker box; an in-place | |
| 96 | + | // switch skips it because the destination's first paint is a full frame. | |
| 97 | + | func (o *attachmentPickerOverlay) release(repaint bool) { | |
| 92 | 98 | if !o.active { | |
| 93 | 99 | return | |
| 94 | 100 | } | |
| 95 | 101 | // Release the slot before the picker stops owning input, so a delivery | |
| 96 | 102 | // racing the cancel reaches the session instead of a picker that no | |
| 97 | 103 | // longer consumes it. | |
| 98 | - | o.sup.attachments.endNavigationOverlay() | |
| 104 | + | o.sup.attachments.endNavigationOverlay(repaint) | |
| 99 | 105 | o.drop() | |
| 100 | 106 | o.sup.transition(supervisorEvent{kind: supervisorOverlayClosed}) | |
| 101 | 107 | } | |
| @@ -145,7 +151,15 @@ func (o *attachmentPickerOverlay) takeOp() { | |||
| 145 | 151 | s.renderCurrent() | |
| 146 | 152 | return | |
| 147 | 153 | } | |
| 148 | - | if o.sameTarget(request) && tab.stopped == nil { | |
| 154 | + | // While an in-place switch is in flight the attachment is leaving | |
| 155 | + | // its committed session, so choosing that session again is a new | |
| 156 | + | // navigation, not a close. | |
| 157 | + | if s.pendingInPlace != nil && s.pendingInPlace.target.request.Target == request.Target && tab.stopped == nil { | |
| 158 | + | // The attachment is already switching there. | |
| 159 | + | o.release(false) | |
| 160 | + | return | |
| 161 | + | } | |
| 162 | + | if s.pendingInPlace == nil && o.sameTarget(request) && tab.stopped == nil { | |
| 149 | 163 | // Another tab of the attached session switches in place; the | |
| 150 | 164 | // attachment is never reconnected for it. | |
| 151 | 165 | _, current, _ := s.attachments.committedView() | |
| @@ -160,7 +174,12 @@ func (o *attachmentPickerOverlay) takeOp() { | |||
| 160 | 174 | if s.cfg.UI != nil && o.run != nil && o.run.fg != nil { | |
| 161 | 175 | s.cfg.UI.followOverlay(o.run.fg.uiGeneration) | |
| 162 | 176 | } | |
| 163 | - | s.pendingSwap = &pickerAttachmentTarget{request: request, tab: tab} | |
| 177 | + | target := pickerAttachmentTarget{request: request, tab: tab} | |
| 178 | + | if o.tryInPlace(target) { | |
| 179 | + | o.release(false) | |
| 180 | + | return | |
| 181 | + | } | |
| 182 | + | s.pendingSwap = &target | |
| 164 | 183 | o.swapping = true | |
| 165 | 184 | s.preview.close(o.picker()) | |
| 166 | 185 | s.attachments.requestDetach(o.run.token) | |
| @@ -209,6 +228,65 @@ func (o *attachmentPickerOverlay) sameTarget(request ports.BrokerOpenStreamReque | |||
| 209 | 228 | return sameAttachmentTarget(o.request, o.sup.attachments.committedTargetOrZero(), request) | |
| 210 | 229 | } | |
| 211 | 230 | ||
| 231 | + | // tryInPlace asks the live attachment to move to target on its own daemon, | |
| 232 | + | // without a new stream or Hello. It applies only to a live session on the | |
| 233 | + | // serving daemon (the local daemon, or the same remote registration) and | |
| 234 | + | // reports false otherwise, leaving the caller to swap. A refusal later falls | |
| 235 | + | // back to the swap (see inPlaceSettled). | |
| 236 | + | func (o *attachmentPickerOverlay) tryInPlace(target pickerAttachmentTarget) bool { | |
| 237 | + | s := o.sup | |
| 238 | + | request := target.request | |
| 239 | + | if request.Admission != ports.BrokerAdmissionExact || request.Local != o.request.Local || target.tab.stopped != nil { | |
| 240 | + | return false | |
| 241 | + | } | |
| 242 | + | if !request.Local && !request.Registration.Equal(o.request.Registration) { | |
| 243 | + | return false | |
| 244 | + | } | |
| 245 | + | if !catalogueSessionLive(o.service.Snapshot(), request) { | |
| 246 | + | // A stopped session needs a restore, which only a fresh attach does. | |
| 247 | + | return false | |
| 248 | + | } | |
| 249 | + | s.inPlaceSeq++ | |
| 250 | + | if !s.attachments.requestInPlace(o.run.token, inPlaceSwitch{seq: s.inPlaceSeq, target: request.Target, tab: target.tab.preferred}) { | |
| 251 | + | return false | |
| 252 | + | } | |
| 253 | + | s.pendingInPlace = &pendingInPlace{seq: s.inPlaceSeq, target: target} | |
| 254 | + | return true | |
| 255 | + | } | |
| 256 | + | ||
| 257 | + | // catalogueSessionLive reports whether the broker catalogue shows the exact | |
| 258 | + | // session request names as running on its daemon. | |
| 259 | + | func catalogueSessionLive(snapshot ports.BrokerSnapshot, request ports.BrokerOpenStreamRequest) bool { | |
| 260 | + | authority := pickerAuthorityInSnapshot(snapshot, pickerSelectionRef{local: request.Local, endpoint: request.Endpoint}) | |
| 261 | + | if !authority.found { | |
| 262 | + | return false | |
| 263 | + | } | |
| 264 | + | session, ok := pickerFindSession(authority.observation.Sessions, request.Target.LifecycleID) | |
| 265 | + | return ok && session.Name == request.Target.SessionName && session.State == catalogue.RemoteCatalogSessionUp | |
| 266 | + | } | |
| 267 | + | ||
| 268 | + | // inPlaceSettled applies the worker's outcome for the current in-place | |
| 269 | + | // switch; an outcome for an older choice is ignored. A refusal leaves the | |
| 270 | + | // source attached, so the same choice falls back to a fresh attachment. A | |
| 271 | + | // superseded switch was replaced by a newer navigation and is dropped. | |
| 272 | + | func (o *attachmentPickerOverlay) inPlaceSettled(result inPlaceResult) { | |
| 273 | + | s := o.sup | |
| 274 | + | pending := s.pendingInPlace | |
| 275 | + | if pending == nil || pending.seq != result.seq { | |
| 276 | + | return | |
| 277 | + | } | |
| 278 | + | s.pendingInPlace = nil | |
| 279 | + | if result.outcome != inPlaceRefused || o.swapping { | |
| 280 | + | return | |
| 281 | + | } | |
| 282 | + | if o.active { | |
| 283 | + | o.exit() | |
| 284 | + | } | |
| 285 | + | s.pendingSwap = &pending.target | |
| 286 | + | o.swapping = true | |
| 287 | + | s.attachments.requestDetach(o.run.token) | |
| 288 | + | } | |
| 289 | + | ||
| 212 | 290 | // sameAttachmentTarget compares one committed selection with the live | |
| 213 | 291 | // attachment: same daemon authority and same session lifecycle. | |
| 214 | 292 | func sameAttachmentTarget(current ports.BrokerOpenStreamRequest, committed protocol.ExactSessionTarget, next ports.BrokerOpenStreamRequest) bool { | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "github.com/bnema/vev/internal/domain" | 4 | "github.com/bnema/vev/internal/domain" | ||
| 5 | "github.com/bnema/vev/internal/ports" | 5 | "github.com/bnema/vev/internal/ports" | ||
| 6 | "github.com/bnema/vev/internal/protocol" | 6 | "github.com/bnema/vev/internal/protocol" | ||
| 7 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |||
| 7 | ) | 8 | ) | ||
| 8 | 9 | ||||
| 9 | // Picker overlay over a live attachment. | 10 | // Picker overlay over a live attachment. | ||
| @@ -88,14 +89,19 @@ func (o *attachmentPickerOverlay) enter() { | |||||
| 88 | } | 89 | } | ||
| 89 | 90 | ||||
| 90 | // exit returns the terminal to the same live attachment. | 91 | // exit returns the terminal to the same live attachment. | ||
| 91 | - | func (o *attachmentPickerOverlay) exit() { | 92 | + | func (o *attachmentPickerOverlay) exit() { o.release(true) } |
| 93 | + | ||||
| 94 | + | // release returns the terminal to the live attachment. repaint asks the | |||
| 95 | + | // daemon to repaint the attached session over the picker box; an in-place | |||
| 96 | + | // switch skips it because the destination's first paint is a full frame. | |||
| 97 | + | func (o *attachmentPickerOverlay) release(repaint bool) { | |||
| 92 | if !o.active { | 98 | if !o.active { | ||
| 93 | return | 99 | return | ||
| 94 | } | 100 | } | ||
| 95 | // Release the slot before the picker stops owning input, so a delivery | 101 | // Release the slot before the picker stops owning input, so a delivery | ||
| 96 | // racing the cancel reaches the session instead of a picker that no | 102 | // racing the cancel reaches the session instead of a picker that no | ||
| 97 | // longer consumes it. | 103 | // longer consumes it. | ||
| 98 | - | o.sup.attachments.endNavigationOverlay() | 104 | + | o.sup.attachments.endNavigationOverlay(repaint) |
| 99 | o.drop() | 105 | o.drop() | ||
| 100 | o.sup.transition(supervisorEvent{kind: supervisorOverlayClosed}) | 106 | o.sup.transition(supervisorEvent{kind: supervisorOverlayClosed}) | ||
| 101 | } | 107 | } | ||
| @@ -145,7 +151,15 @@ func (o *attachmentPickerOverlay) takeOp() { | |||||
| 145 | s.renderCurrent() | 151 | s.renderCurrent() | ||
| 146 | return | 152 | return | ||
| 147 | } | 153 | } | ||
| 148 | - | if o.sameTarget(request) && tab.stopped == nil { | 154 | + | // While an in-place switch is in flight the attachment is leaving |
| 155 | + | // its committed session, so choosing that session again is a new | |||
| 156 | + | // navigation, not a close. | |||
| 157 | + | if s.pendingInPlace != nil && s.pendingInPlace.target.request.Target == request.Target && tab.stopped == nil { | |||
| 158 | + | // The attachment is already switching there. | |||
| 159 | + | o.release(false) | |||
| 160 | + | return | |||
| 161 | + | } | |||
| 162 | + | if s.pendingInPlace == nil && o.sameTarget(request) && tab.stopped == nil { | |||
| 149 | // Another tab of the attached session switches in place; the | 163 | // Another tab of the attached session switches in place; the | ||
| 150 | // attachment is never reconnected for it. | 164 | // attachment is never reconnected for it. | ||
| 151 | _, current, _ := s.attachments.committedView() | 165 | _, current, _ := s.attachments.committedView() | ||
| @@ -160,7 +174,12 @@ func (o *attachmentPickerOverlay) takeOp() { | |||||
| 160 | if s.cfg.UI != nil && o.run != nil && o.run.fg != nil { | 174 | if s.cfg.UI != nil && o.run != nil && o.run.fg != nil { | ||
| 161 | s.cfg.UI.followOverlay(o.run.fg.uiGeneration) | 175 | s.cfg.UI.followOverlay(o.run.fg.uiGeneration) | ||
| 162 | } | 176 | } | ||
| 163 | - | s.pendingSwap = &pickerAttachmentTarget{request: request, tab: tab} | 177 | + | target := pickerAttachmentTarget{request: request, tab: tab} |
| 178 | + | if o.tryInPlace(target) { | |||
| 179 | + | o.release(false) | |||
| 180 | + | return | |||
| 181 | + | } | |||
| 182 | + | s.pendingSwap = &target | |||
| 164 | o.swapping = true | 183 | o.swapping = true | ||
| 165 | s.preview.close(o.picker()) | 184 | s.preview.close(o.picker()) | ||
| 166 | s.attachments.requestDetach(o.run.token) | 185 | s.attachments.requestDetach(o.run.token) | ||
| @@ -209,6 +228,65 @@ func (o *attachmentPickerOverlay) sameTarget(request ports.BrokerOpenStreamReque | |||||
| 209 | return sameAttachmentTarget(o.request, o.sup.attachments.committedTargetOrZero(), request) | 228 | return sameAttachmentTarget(o.request, o.sup.attachments.committedTargetOrZero(), request) | ||
| 210 | } | 229 | } | ||
| 211 | 230 | ||||
| 231 | + | // tryInPlace asks the live attachment to move to target on its own daemon, | |||
| 232 | + | // without a new stream or Hello. It applies only to a live session on the | |||
| 233 | + | // serving daemon (the local daemon, or the same remote registration) and | |||
| 234 | + | // reports false otherwise, leaving the caller to swap. A refusal later falls | |||
| 235 | + | // back to the swap (see inPlaceSettled). | |||
| 236 | + | func (o *attachmentPickerOverlay) tryInPlace(target pickerAttachmentTarget) bool { | |||
| 237 | + | s := o.sup | |||
| 238 | + | request := target.request | |||
| 239 | + | if request.Admission != ports.BrokerAdmissionExact || request.Local != o.request.Local || target.tab.stopped != nil { | |||
| 240 | + | return false | |||
| 241 | + | } | |||
| 242 | + | if !request.Local && !request.Registration.Equal(o.request.Registration) { | |||
| 243 | + | return false | |||
| 244 | + | } | |||
| 245 | + | if !catalogueSessionLive(o.service.Snapshot(), request) { | |||
| 246 | + | // A stopped session needs a restore, which only a fresh attach does. | |||
| 247 | + | return false | |||
| 248 | + | } | |||
| 249 | + | s.inPlaceSeq++ | |||
| 250 | + | if !s.attachments.requestInPlace(o.run.token, inPlaceSwitch{seq: s.inPlaceSeq, target: request.Target, tab: target.tab.preferred}) { | |||
| 251 | + | return false | |||
| 252 | + | } | |||
| 253 | + | s.pendingInPlace = &pendingInPlace{seq: s.inPlaceSeq, target: target} | |||
| 254 | + | return true | |||
| 255 | + | } | |||
| 256 | + | ||||
| 257 | + | // catalogueSessionLive reports whether the broker catalogue shows the exact | |||
| 258 | + | // session request names as running on its daemon. | |||
| 259 | + | func catalogueSessionLive(snapshot ports.BrokerSnapshot, request ports.BrokerOpenStreamRequest) bool { | |||
| 260 | + | authority := pickerAuthorityInSnapshot(snapshot, pickerSelectionRef{local: request.Local, endpoint: request.Endpoint}) | |||
| 261 | + | if !authority.found { | |||
| 262 | + | return false | |||
| 263 | + | } | |||
| 264 | + | session, ok := pickerFindSession(authority.observation.Sessions, request.Target.LifecycleID) | |||
| 265 | + | return ok && session.Name == request.Target.SessionName && session.State == catalogue.RemoteCatalogSessionUp | |||
| 266 | + | } | |||
| 267 | + | ||||
| 268 | + | // inPlaceSettled applies the worker's outcome for the current in-place | |||
| 269 | + | // switch; an outcome for an older choice is ignored. A refusal leaves the | |||
| 270 | + | // source attached, so the same choice falls back to a fresh attachment. A | |||
| 271 | + | // superseded switch was replaced by a newer navigation and is dropped. | |||
| 272 | + | func (o *attachmentPickerOverlay) inPlaceSettled(result inPlaceResult) { | |||
| 273 | + | s := o.sup | |||
| 274 | + | pending := s.pendingInPlace | |||
| 275 | + | if pending == nil || pending.seq != result.seq { | |||
| 276 | + | return | |||
| 277 | + | } | |||
| 278 | + | s.pendingInPlace = nil | |||
| 279 | + | if result.outcome != inPlaceRefused || o.swapping { | |||
| 280 | + | return | |||
| 281 | + | } | |||
| 282 | + | if o.active { | |||
| 283 | + | o.exit() | |||
| 284 | + | } | |||
| 285 | + | s.pendingSwap = &pending.target | |||
| 286 | + | o.swapping = true | |||
| 287 | + | s.attachments.requestDetach(o.run.token) | |||
| 288 | + | } | |||
| 289 | + | ||||
| 212 | // sameAttachmentTarget compares one committed selection with the live | 290 | // sameAttachmentTarget compares one committed selection with the live | ||
| 213 | // attachment: same daemon authority and same session lifecycle. | 291 | // attachment: same daemon authority and same session lifecycle. | ||
| 214 | func sameAttachmentTarget(current ports.BrokerOpenStreamRequest, committed protocol.ExactSessionTarget, next ports.BrokerOpenStreamRequest) bool { | 292 | func sameAttachmentTarget(current ports.BrokerOpenStreamRequest, committed protocol.ExactSessionTarget, next ports.BrokerOpenStreamRequest) bool { | ||
M internal/usecase/client/supervisor_routes.go +12 -1
| @@ -75,13 +75,24 @@ func (s *Supervisor) settleDaemonNavigation(service ports.BrokerNavigator, overl | |||
| 75 | 75 | if actionID := navigationCauseActionID(message); actionID != 0 && s.cfg.UI != nil { | |
| 76 | 76 | s.cfg.UI.follow(overlay.run.fg.uiGeneration, actionID) | |
| 77 | 77 | } | |
| 78 | - | if sameAttachmentTarget(overlay.request, s.attachments.committedTargetOrZero(), target.request) && target.tab.stopped == nil { | |
| 78 | + | if s.pendingInPlace != nil && s.pendingInPlace.target.request.Target == target.request.Target && target.tab.stopped == nil { | |
| 79 | + | // The attachment is already switching there. | |
| 80 | + | overlay.release(false) | |
| 81 | + | return | |
| 82 | + | } | |
| 83 | + | if s.pendingInPlace == nil && sameAttachmentTarget(overlay.request, s.attachments.committedTargetOrZero(), target.request) && target.tab.stopped == nil { | |
| 79 | 84 | _, current, _ := s.attachments.committedView() | |
| 80 | 85 | if target.tab.preferred != "" && target.tab.preferred != current { | |
| 81 | 86 | s.attachments.requestTabSelection(overlay.run.token, target.tab.preferred) | |
| 82 | 87 | } | |
| 83 | 88 | return | |
| 84 | 89 | } | |
| 90 | + | // A recent route on the serving daemon moves in place. A daemon handoff | |
| 91 | + | // (AttachTarget) keeps close-and-dial: the daemon chose it on purpose. | |
| 92 | + | if _, route := message.(protocol.RouteNavigationAction); route && overlay.tryInPlace(target) { | |
| 93 | + | overlay.release(false) | |
| 94 | + | return | |
| 95 | + | } | |
| 85 | 96 | if overlay.active { | |
| 86 | 97 | overlay.exit() | |
| 87 | 98 | } | |
| @@ -75,13 +75,24 @@ func (s *Supervisor) settleDaemonNavigation(service ports.BrokerNavigator, overl | |||||
| 75 | if actionID := navigationCauseActionID(message); actionID != 0 && s.cfg.UI != nil { | 75 | if actionID := navigationCauseActionID(message); actionID != 0 && s.cfg.UI != nil { | ||
| 76 | s.cfg.UI.follow(overlay.run.fg.uiGeneration, actionID) | 76 | s.cfg.UI.follow(overlay.run.fg.uiGeneration, actionID) | ||
| 77 | } | 77 | } | ||
| 78 | - | if sameAttachmentTarget(overlay.request, s.attachments.committedTargetOrZero(), target.request) && target.tab.stopped == nil { | 78 | + | if s.pendingInPlace != nil && s.pendingInPlace.target.request.Target == target.request.Target && target.tab.stopped == nil { |
| 79 | + | // The attachment is already switching there. | |||
| 80 | + | overlay.release(false) | |||
| 81 | + | return | |||
| 82 | + | } | |||
| 83 | + | if s.pendingInPlace == nil && sameAttachmentTarget(overlay.request, s.attachments.committedTargetOrZero(), target.request) && target.tab.stopped == nil { | |||
| 79 | _, current, _ := s.attachments.committedView() | 84 | _, current, _ := s.attachments.committedView() | ||
| 80 | if target.tab.preferred != "" && target.tab.preferred != current { | 85 | if target.tab.preferred != "" && target.tab.preferred != current { | ||
| 81 | s.attachments.requestTabSelection(overlay.run.token, target.tab.preferred) | 86 | s.attachments.requestTabSelection(overlay.run.token, target.tab.preferred) | ||
| 82 | } | 87 | } | ||
| 83 | return | 88 | return | ||
| 84 | } | 89 | } | ||
| 90 | + | // A recent route on the serving daemon moves in place. A daemon handoff | |||
| 91 | + | // (AttachTarget) keeps close-and-dial: the daemon chose it on purpose. | |||
| 92 | + | if _, route := message.(protocol.RouteNavigationAction); route && overlay.tryInPlace(target) { | |||
| 93 | + | overlay.release(false) | |||
| 94 | + | return | |||
| 95 | + | } | |||
| 85 | if overlay.active { | 96 | if overlay.active { | ||
| 86 | overlay.exit() | 97 | overlay.exit() | ||
| 87 | } | 98 | } | ||
M internal/usecase/client/supervisor_routes_test.go +36 -5
| @@ -109,10 +109,12 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||
| 109 | 109 | tests := []struct { | |
| 110 | 110 | name string | |
| 111 | 111 | // request builds the daemon message from the first route snapshot. | |
| 112 | - | request func(protocol.RecentRouteSnapshot) protocol.ServerMessage | |
| 113 | - | detached bool | |
| 114 | - | wantOpen func(*testing.T, ports.BrokerOpenStreamRequest) | |
| 115 | - | wantReply func(*testing.T, *sessionTestStream) | |
| 112 | + | request func(protocol.RecentRouteSnapshot) protocol.ServerMessage | |
| 113 | + | detached bool | |
| 114 | + | // refuseInPlace answers the in-place switch with a daemon refusal. | |
| 115 | + | refuseInPlace bool | |
| 116 | + | wantOpen func(*testing.T, ports.BrokerOpenStreamRequest) | |
| 117 | + | wantReply func(*testing.T, *sessionTestStream) | |
| 116 | 118 | }{ | |
| 117 | 119 | { | |
| 118 | 120 | name: "recent route to another daemon swaps", | |
| @@ -127,6 +129,29 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||
| 127 | 129 | require.Equal(t, "rem", request.Target.SessionName) | |
| 128 | 130 | }, | |
| 129 | 131 | }, | |
| 132 | + | { | |
| 133 | + | name: "recent route on the serving daemon switches in place", | |
| 134 | + | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | |
| 135 | + | entry, _ := routeEntryNamed(s, "beta") | |
| 136 | + | return protocol.RouteNavigationAction{SnapshotGeneration: s.Generation, Key: entry.Key, Generation: entry.Generation} | |
| 137 | + | }, | |
| 138 | + | wantReply: func(t *testing.T, stream *sessionTestStream) { | |
| 139 | + | request := awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |
| 140 | + | require.Equal(t, beta, request.Target) | |
| 141 | + | }, | |
| 142 | + | }, | |
| 143 | + | { | |
| 144 | + | name: "a refused in-place route reconnects", | |
| 145 | + | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | |
| 146 | + | entry, _ := routeEntryNamed(s, "beta") | |
| 147 | + | return protocol.RouteNavigationAction{SnapshotGeneration: s.Generation, Key: entry.Key, Generation: entry.Generation} | |
| 148 | + | }, | |
| 149 | + | refuseInPlace: true, | |
| 150 | + | wantOpen: func(t *testing.T, request ports.BrokerOpenStreamRequest) { | |
| 151 | + | require.True(t, request.Local) | |
| 152 | + | require.Equal(t, beta, request.Target) | |
| 153 | + | }, | |
| 154 | + | }, | |
| 130 | 155 | { | |
| 131 | 156 | name: "a stale reference is refused typed", | |
| 132 | 157 | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | |
| @@ -205,6 +230,10 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||
| 205 | 230 | return second, nil | |
| 206 | 231 | }) | |
| 207 | 232 | stream.deliver(tt.request(snapshot)) | |
| 233 | + | if tt.refuseInPlace { | |
| 234 | + | request := awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |
| 235 | + | stream.deliver(protocol.SamePeerSwitchFailure{RequestID: request.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |
| 236 | + | } | |
| 208 | 237 | if tt.detached { | |
| 209 | 238 | // The daemon ends the source right after a close-and-dial | |
| 210 | 239 | // handoff; the supervisor still adopts the handoff. | |
| @@ -227,7 +256,9 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||
| 227 | 256 | return | |
| 228 | 257 | } | |
| 229 | 258 | tt.wantReply(t, stream) | |
| 230 | - | require.Never(t, func() bool { return len(harness.service.openedRequests()) > 1 }, 50*time.Millisecond, time.Millisecond, "a refusal or an in-place switch never reconnects") | |
| 259 | + | // The supervisor chose before the worker sent the reply above, so | |
| 260 | + | // no reconnect can still be pending. | |
| 261 | + | require.Len(t, harness.service.openedRequests(), 1, "a refusal or an in-place switch never reconnects") | |
| 231 | 262 | require.Zero(t, countSent[protocol.Detach](stream)) | |
| 232 | 263 | }) | |
| 233 | 264 | } | |
| @@ -109,10 +109,12 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||||
| 109 | tests := []struct { | 109 | tests := []struct { | ||
| 110 | name string | 110 | name string | ||
| 111 | // request builds the daemon message from the first route snapshot. | 111 | // request builds the daemon message from the first route snapshot. | ||
| 112 | - | request func(protocol.RecentRouteSnapshot) protocol.ServerMessage | 112 | + | request func(protocol.RecentRouteSnapshot) protocol.ServerMessage |
| 113 | - | detached bool | 113 | + | detached bool |
| 114 | - | wantOpen func(*testing.T, ports.BrokerOpenStreamRequest) | 114 | + | // refuseInPlace answers the in-place switch with a daemon refusal. |
| 115 | - | wantReply func(*testing.T, *sessionTestStream) | 115 | + | refuseInPlace bool |
| 116 | + | wantOpen func(*testing.T, ports.BrokerOpenStreamRequest) | |||
| 117 | + | wantReply func(*testing.T, *sessionTestStream) | |||
| 116 | }{ | 118 | }{ | ||
| 117 | { | 119 | { | ||
| 118 | name: "recent route to another daemon swaps", | 120 | name: "recent route to another daemon swaps", | ||
| @@ -127,6 +129,29 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||||
| 127 | require.Equal(t, "rem", request.Target.SessionName) | 129 | require.Equal(t, "rem", request.Target.SessionName) | ||
| 128 | }, | 130 | }, | ||
| 129 | }, | 131 | }, | ||
| 132 | + | { | |||
| 133 | + | name: "recent route on the serving daemon switches in place", | |||
| 134 | + | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | |||
| 135 | + | entry, _ := routeEntryNamed(s, "beta") | |||
| 136 | + | return protocol.RouteNavigationAction{SnapshotGeneration: s.Generation, Key: entry.Key, Generation: entry.Generation} | |||
| 137 | + | }, | |||
| 138 | + | wantReply: func(t *testing.T, stream *sessionTestStream) { | |||
| 139 | + | request := awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |||
| 140 | + | require.Equal(t, beta, request.Target) | |||
| 141 | + | }, | |||
| 142 | + | }, | |||
| 143 | + | { | |||
| 144 | + | name: "a refused in-place route reconnects", | |||
| 145 | + | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | |||
| 146 | + | entry, _ := routeEntryNamed(s, "beta") | |||
| 147 | + | return protocol.RouteNavigationAction{SnapshotGeneration: s.Generation, Key: entry.Key, Generation: entry.Generation} | |||
| 148 | + | }, | |||
| 149 | + | refuseInPlace: true, | |||
| 150 | + | wantOpen: func(t *testing.T, request ports.BrokerOpenStreamRequest) { | |||
| 151 | + | require.True(t, request.Local) | |||
| 152 | + | require.Equal(t, beta, request.Target) | |||
| 153 | + | }, | |||
| 154 | + | }, | |||
| 130 | { | 155 | { | ||
| 131 | name: "a stale reference is refused typed", | 156 | name: "a stale reference is refused typed", | ||
| 132 | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | 157 | request: func(s protocol.RecentRouteSnapshot) protocol.ServerMessage { | ||
| @@ -205,6 +230,10 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||||
| 205 | return second, nil | 230 | return second, nil | ||
| 206 | }) | 231 | }) | ||
| 207 | stream.deliver(tt.request(snapshot)) | 232 | stream.deliver(tt.request(snapshot)) | ||
| 233 | + | if tt.refuseInPlace { | |||
| 234 | + | request := awaitSent(t, stream, "SamePeerSwitchRequest", isSent[protocol.SamePeerSwitchRequest]).(protocol.SamePeerSwitchRequest) | |||
| 235 | + | stream.deliver(protocol.SamePeerSwitchFailure{RequestID: request.RequestID, Code: protocol.SamePeerSwitchStaleTarget}) | |||
| 236 | + | } | |||
| 208 | if tt.detached { | 237 | if tt.detached { | ||
| 209 | // The daemon ends the source right after a close-and-dial | 238 | // The daemon ends the source right after a close-and-dial | ||
| 210 | // handoff; the supervisor still adopts the handoff. | 239 | // handoff; the supervisor still adopts the handoff. | ||
| @@ -227,7 +256,9 @@ func TestSupervisorSettlesDaemonNavigation(t *testing.T) { | |||||
| 227 | return | 256 | return | ||
| 228 | } | 257 | } | ||
| 229 | tt.wantReply(t, stream) | 258 | tt.wantReply(t, stream) | ||
| 230 | - | require.Never(t, func() bool { return len(harness.service.openedRequests()) > 1 }, 50*time.Millisecond, time.Millisecond, "a refusal or an in-place switch never reconnects") | 259 | + | // The supervisor chose before the worker sent the reply above, so |
| 260 | + | // no reconnect can still be pending. | |||
| 261 | + | require.Len(t, harness.service.openedRequests(), 1, "a refusal or an in-place switch never reconnects") | |||
| 231 | require.Zero(t, countSent[protocol.Detach](stream)) | 262 | require.Zero(t, countSent[protocol.Detach](stream)) | ||
| 232 | }) | 263 | }) | ||
| 233 | } | 264 | } | ||
M internal/usecase/daemon/same_peer_switch.go +10 -4
| @@ -41,13 +41,19 @@ func (ac *attachedClient) clearSamePeerOffer() { | |||
| 41 | 41 | ac.samePeerOfferMu.Unlock() | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | - | // consumeSamePeerOffer linearizes the client confirmation with the daemon's | |
| 45 | - | // endpoint-empty offer. A client cannot select an arbitrary local session by | |
| 46 | - | // manufacturing this frame. | |
| 44 | + | // consumeSamePeerOffer linearizes a switch request with the daemon's pending | |
| 45 | + | // endpoint-empty offer: while one is pending, only its exact target is | |
| 46 | + | // accepted. Without a pending offer the request is the client's own picker | |
| 47 | + | // choice. It grants no more than a fresh exact attach would on this | |
| 48 | + | // authenticated connection, and samePeerTarget still requires a live session | |
| 49 | + | // with the exact lifecycle. | |
| 47 | 50 | func (ac *attachedClient) consumeSamePeerOffer(target protocol.ExactSessionTarget) bool { | |
| 48 | 51 | ac.samePeerOfferMu.Lock() | |
| 49 | 52 | defer ac.samePeerOfferMu.Unlock() | |
| 50 | - | if ac.samePeerOffer == nil || *ac.samePeerOffer != target { | |
| 53 | + | if ac.samePeerOffer == nil { | |
| 54 | + | return true | |
| 55 | + | } | |
| 56 | + | if *ac.samePeerOffer != target { | |
| 51 | 57 | return false | |
| 52 | 58 | } | |
| 53 | 59 | ac.samePeerOffer = nil | |
| @@ -41,13 +41,19 @@ func (ac *attachedClient) clearSamePeerOffer() { | |||||
| 41 | ac.samePeerOfferMu.Unlock() | 41 | ac.samePeerOfferMu.Unlock() | ||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | - | // consumeSamePeerOffer linearizes the client confirmation with the daemon's | 44 | + | // consumeSamePeerOffer linearizes a switch request with the daemon's pending |
| 45 | - | // endpoint-empty offer. A client cannot select an arbitrary local session by | 45 | + | // endpoint-empty offer: while one is pending, only its exact target is |
| 46 | - | // manufacturing this frame. | 46 | + | // accepted. Without a pending offer the request is the client's own picker |
| 47 | + | // choice. It grants no more than a fresh exact attach would on this | |||
| 48 | + | // authenticated connection, and samePeerTarget still requires a live session | |||
| 49 | + | // with the exact lifecycle. | |||
| 47 | func (ac *attachedClient) consumeSamePeerOffer(target protocol.ExactSessionTarget) bool { | 50 | func (ac *attachedClient) consumeSamePeerOffer(target protocol.ExactSessionTarget) bool { | ||
| 48 | ac.samePeerOfferMu.Lock() | 51 | ac.samePeerOfferMu.Lock() | ||
| 49 | defer ac.samePeerOfferMu.Unlock() | 52 | defer ac.samePeerOfferMu.Unlock() | ||
| 50 | - | if ac.samePeerOffer == nil || *ac.samePeerOffer != target { | 53 | + | if ac.samePeerOffer == nil { |
| 54 | + | return true | |||
| 55 | + | } | |||
| 56 | + | if *ac.samePeerOffer != target { | |||
| 51 | return false | 57 | return false | ||
| 52 | } | 58 | } | ||
| 53 | ac.samePeerOffer = nil | 59 | ac.samePeerOffer = nil | ||
M internal/usecase/daemon/same_peer_switch_test.go +54 -0
| @@ -121,6 +121,60 @@ func TestSamePeerSwitchRejectsStaleTargetWithoutMutation(t *testing.T) { | |||
| 121 | 121 | require.Equal(t, protocol.SamePeerSwitchFailure{RequestID: 1, Code: protocol.SamePeerSwitchStaleTarget}, failure) | |
| 122 | 122 | } | |
| 123 | 123 | ||
| 124 | + | // TestSamePeerSwitchClientInitiated pins that the client picker can move its | |
| 125 | + | // attachment without a daemon offer, while a pending offer still admits only | |
| 126 | + | // its own exact target. | |
| 127 | + | func TestSamePeerSwitchClientInitiated(t *testing.T) { | |
| 128 | + | lifecycle := domain.SessionLifecycleID{7} | |
| 129 | + | requestTarget := protocol.ExactSessionTarget{LifecycleID: lifecycle, SessionName: "target"} | |
| 130 | + | otherOffer := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{8}, SessionName: "other"} | |
| 131 | + | tests := []struct { | |
| 132 | + | name string | |
| 133 | + | offer *protocol.ExactSessionTarget | |
| 134 | + | switched bool | |
| 135 | + | }{ | |
| 136 | + | {name: "no pending offer switches", switched: true}, | |
| 137 | + | {name: "matching offer switches", offer: &requestTarget, switched: true}, | |
| 138 | + | {name: "another pending offer refuses", offer: &otherOffer}, | |
| 139 | + | } | |
| 140 | + | for _, tt := range tests { | |
| 141 | + | t.Run(tt.name, func(t *testing.T) { | |
| 142 | + | d, source, ac, sends, releases := newManualTabSession(t, 1) | |
| 143 | + | defer releaseAll(releases) | |
| 144 | + | target := &session{ | |
| 145 | + | sessionCore: sessionCore{id: "target", name: "target", incarnation: lifecycle, attachments: make(map[*attachedClient]struct{})}, | |
| 146 | + | ctx: source.ctx, | |
| 147 | + | cancel: func() {}, | |
| 148 | + | tabs: []*tab{newTab(nil, domain.Size{Cols: 80, Rows: 23})}, | |
| 149 | + | } | |
| 150 | + | publishTiledPaneOwners(target, target.tabs[0]) | |
| 151 | + | d.mu.Lock() | |
| 152 | + | d.sessions[target.id] = target | |
| 153 | + | d.mu.Unlock() | |
| 154 | + | ac.setRouteSnapshot(protocol.RecentRouteSnapshot{Generation: 1}) | |
| 155 | + | if tt.offer != nil { | |
| 156 | + | ac.offerSamePeerTarget(*tt.offer) | |
| 157 | + | } | |
| 158 | + | ||
| 159 | + | token := source.captureAttachmentCapability(ac, ac.transport()) | |
| 160 | + | effect, admitted := ac.beginAttachmentEffect(token) | |
| 161 | + | require.True(t, admitted) | |
| 162 | + | defer effect.End() | |
| 163 | + | d.switchSamePeerForAttachment(effect, protocol.SamePeerSwitchRequest{RequestID: 1, Target: requestTarget}) | |
| 164 | + | ||
| 165 | + | if tt.switched { | |
| 166 | + | require.Same(t, target, ac.currentAttachmentSession()) | |
| 167 | + | identity := decodeServerMessage(t, awaitFrame(t, sends, "CommittedRouteIdentity")).(protocol.CommittedRouteIdentity) | |
| 168 | + | require.Equal(t, requestTarget, identity.Target) | |
| 169 | + | return | |
| 170 | + | } | |
| 171 | + | require.Same(t, source, ac.currentAttachmentSession()) | |
| 172 | + | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | |
| 173 | + | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | |
| 174 | + | }) | |
| 175 | + | } | |
| 176 | + | } | |
| 177 | + | ||
| 124 | 178 | func TestSamePeerSwitchRefreshesTargetFromSwitchingAttachment(t *testing.T) { | |
| 125 | 179 | targetEnv := []string{"SHELL=/usr/bin/fish", "WAYLAND_DISPLAY=wayland-old", "SSH_AUTH_SOCK=/run/other-agent"} | |
| 126 | 180 | tests := []struct { | |
| @@ -121,6 +121,60 @@ func TestSamePeerSwitchRejectsStaleTargetWithoutMutation(t *testing.T) { | |||||
| 121 | require.Equal(t, protocol.SamePeerSwitchFailure{RequestID: 1, Code: protocol.SamePeerSwitchStaleTarget}, failure) | 121 | require.Equal(t, protocol.SamePeerSwitchFailure{RequestID: 1, Code: protocol.SamePeerSwitchStaleTarget}, failure) | ||
| 122 | } | 122 | } | ||
| 123 | 123 | ||||
| 124 | + | // TestSamePeerSwitchClientInitiated pins that the client picker can move its | |||
| 125 | + | // attachment without a daemon offer, while a pending offer still admits only | |||
| 126 | + | // its own exact target. | |||
| 127 | + | func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||
| 128 | + | lifecycle := domain.SessionLifecycleID{7} | |||
| 129 | + | requestTarget := protocol.ExactSessionTarget{LifecycleID: lifecycle, SessionName: "target"} | |||
| 130 | + | otherOffer := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{8}, SessionName: "other"} | |||
| 131 | + | tests := []struct { | |||
| 132 | + | name string | |||
| 133 | + | offer *protocol.ExactSessionTarget | |||
| 134 | + | switched bool | |||
| 135 | + | }{ | |||
| 136 | + | {name: "no pending offer switches", switched: true}, | |||
| 137 | + | {name: "matching offer switches", offer: &requestTarget, switched: true}, | |||
| 138 | + | {name: "another pending offer refuses", offer: &otherOffer}, | |||
| 139 | + | } | |||
| 140 | + | for _, tt := range tests { | |||
| 141 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 142 | + | d, source, ac, sends, releases := newManualTabSession(t, 1) | |||
| 143 | + | defer releaseAll(releases) | |||
| 144 | + | target := &session{ | |||
| 145 | + | sessionCore: sessionCore{id: "target", name: "target", incarnation: lifecycle, attachments: make(map[*attachedClient]struct{})}, | |||
| 146 | + | ctx: source.ctx, | |||
| 147 | + | cancel: func() {}, | |||
| 148 | + | tabs: []*tab{newTab(nil, domain.Size{Cols: 80, Rows: 23})}, | |||
| 149 | + | } | |||
| 150 | + | publishTiledPaneOwners(target, target.tabs[0]) | |||
| 151 | + | d.mu.Lock() | |||
| 152 | + | d.sessions[target.id] = target | |||
| 153 | + | d.mu.Unlock() | |||
| 154 | + | ac.setRouteSnapshot(protocol.RecentRouteSnapshot{Generation: 1}) | |||
| 155 | + | if tt.offer != nil { | |||
| 156 | + | ac.offerSamePeerTarget(*tt.offer) | |||
| 157 | + | } | |||
| 158 | + | ||||
| 159 | + | token := source.captureAttachmentCapability(ac, ac.transport()) | |||
| 160 | + | effect, admitted := ac.beginAttachmentEffect(token) | |||
| 161 | + | require.True(t, admitted) | |||
| 162 | + | defer effect.End() | |||
| 163 | + | d.switchSamePeerForAttachment(effect, protocol.SamePeerSwitchRequest{RequestID: 1, Target: requestTarget}) | |||
| 164 | + | ||||
| 165 | + | if tt.switched { | |||
| 166 | + | require.Same(t, target, ac.currentAttachmentSession()) | |||
| 167 | + | identity := decodeServerMessage(t, awaitFrame(t, sends, "CommittedRouteIdentity")).(protocol.CommittedRouteIdentity) | |||
| 168 | + | require.Equal(t, requestTarget, identity.Target) | |||
| 169 | + | return | |||
| 170 | + | } | |||
| 171 | + | require.Same(t, source, ac.currentAttachmentSession()) | |||
| 172 | + | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | |||
| 173 | + | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | |||
| 174 | + | }) | |||
| 175 | + | } | |||
| 176 | + | } | |||
| 177 | + | ||||
| 124 | func TestSamePeerSwitchRefreshesTargetFromSwitchingAttachment(t *testing.T) { | 178 | func TestSamePeerSwitchRefreshesTargetFromSwitchingAttachment(t *testing.T) { | ||
| 125 | targetEnv := []string{"SHELL=/usr/bin/fish", "WAYLAND_DISPLAY=wayland-old", "SSH_AUTH_SOCK=/run/other-agent"} | 179 | targetEnv := []string{"SHELL=/usr/bin/fish", "WAYLAND_DISPLAY=wayland-old", "SSH_AUTH_SOCK=/run/other-agent"} | ||
| 126 | tests := []struct { | 180 | tests := []struct { | ||