fix(picker): preserve active remote context in the home picker (#256)
* fix(picker): preserve active route context in the home picker * test(picker): address review feedback
- commit
- bc06929cfab8e8349c4226eac6284f17289f6736
- author
- bnema · · 2026-09-05 22:05 UTC
- committer
- GitHub · · 2026-09-05 22:05 UTC
- parent
- f1fbf32
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
5 files · +241 -8
M docs/remote-resilience.md +7 -0
| @@ -77,6 +77,13 @@ and dial normally. While any switch takes longer than the short display | |||
| 77 | 77 | threshold, the client overlays an animated switching or starting toast until | |
| 78 | 78 | the destination's authoritative output is flushed. | |
| 79 | 79 | ||
| 80 | + | Opening the home picker does not change the client's active route or recent | |
| 81 | + | session order, on either UDP or SSH stdio. Its status bar keeps the source | |
| 82 | + | session label (for example, `misc@igor`), not the local session hosting the | |
| 83 | + | picker. Local backing-session tabs and bar-script output are hidden while | |
| 84 | + | this temporary attachment renders the picker. Selecting a destination commits | |
| 85 | + | that route; Back returns to the source. | |
| 86 | + | ||
| 80 | 87 | ## Durable record compatibility | |
| 81 | 88 | ||
| 82 | 89 | Catalogue record format version 6 stores durable session metadata independently | |
| @@ -77,6 +77,13 @@ and dial normally. While any switch takes longer than the short display | |||||
| 77 | threshold, the client overlays an animated switching or starting toast until | 77 | threshold, the client overlays an animated switching or starting toast until | ||
| 78 | the destination's authoritative output is flushed. | 78 | the destination's authoritative output is flushed. | ||
| 79 | 79 | ||||
| 80 | + | Opening the home picker does not change the client's active route or recent | |||
| 81 | + | session order, on either UDP or SSH stdio. Its status bar keeps the source | |||
| 82 | + | session label (for example, `misc@igor`), not the local session hosting the | |||
| 83 | + | picker. Local backing-session tabs and bar-script output are hidden while | |||
| 84 | + | this temporary attachment renders the picker. Selecting a destination commits | |||
| 85 | + | that route; Back returns to the source. | |||
| 86 | + | ||||
| 80 | ## Durable record compatibility | 87 | ## Durable record compatibility | ||
| 81 | 88 | ||||
| 82 | Catalogue record format version 6 stores durable session metadata independently | 89 | Catalogue record format version 6 stores durable session metadata independently | ||
M internal/usecase/client/client.go +11 -6
| @@ -607,8 +607,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 607 | 607 | finishHandshake: finishHandshake, stopHandshakeTransport: stopHandshakeTransport, | |
| 608 | 608 | request: request, clientID: processClientID, milestones: &ms, | |
| 609 | 609 | themeState: themeState, enterRaw: enterRaw, reconnect: localReconnect, | |
| 610 | - | terminalInput: func() *terminalInputPump { return input }, | |
| 611 | - | transientPicker: true, returnAttachTargets: true, | |
| 610 | + | terminalInput: func() *terminalInputPump { return input }, | |
| 612 | 611 | }).run(attemptCtx) | |
| 613 | 612 | stopHandshakeTransport() | |
| 614 | 613 | finishHandshake() | |
| @@ -952,6 +951,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 952 | 951 | if attemptRequest.StartupOverlay == protocol.StartupOverlaySessionPicker { | |
| 953 | 952 | returnRoute = nil | |
| 954 | 953 | returnResumeFallback = false | |
| 954 | + | nextRequest.StartupOverlay = protocol.StartupOverlayNone | |
| 955 | + | nextRequest.NavigationCapabilities &^= protocol.NavigationCapabilityBack | |
| 955 | 956 | } | |
| 956 | 957 | nextRequest = cloneAttachRequest(nextRequest) | |
| 957 | 958 | if err := validateAttachRequest(nextRequest); err != nil { | |
| @@ -1274,8 +1275,6 @@ type attachAttempt struct { | |||
| 1274 | 1275 | rememberRemoteHost func() | |
| 1275 | 1276 | terminalInput func() *terminalInputPump | |
| 1276 | 1277 | openHomePicker func(context.Context) attachResult | |
| 1277 | - | transientPicker bool | |
| 1278 | - | returnAttachTargets bool | |
| 1279 | 1278 | } | |
| 1280 | 1279 | ||
| 1281 | 1280 | type attachResult struct { | |
| @@ -1335,6 +1334,9 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 1335 | 1334 | transport = a.connection.Connection() | |
| 1336 | 1335 | } | |
| 1337 | 1336 | request := a.request | |
| 1337 | + | // Both parked UDP and close-and-dial home pickers only borrow a local | |
| 1338 | + | // attachment for rendering. They do not own the client's active route. | |
| 1339 | + | transientPicker := request.StartupOverlay == protocol.StartupOverlaySessionPicker | |
| 1338 | 1340 | term := a.runner.term | |
| 1339 | 1341 | clk := a.runner.clock | |
| 1340 | 1342 | intent := request.Intent | |
| @@ -1500,7 +1502,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 1500 | 1502 | if request.RemoteTarget != nil && (request.RemoteTarget.LifecycleID != committedIdentity.Target.LifecycleID || request.RemoteTarget.SessionName != committedIdentity.Target.SessionName) { | |
| 1501 | 1503 | return welcomedResult(errRouteTargetChanged) | |
| 1502 | 1504 | } | |
| 1503 | - | if !a.transientPicker { | |
| 1505 | + | if !transientPicker { | |
| 1504 | 1506 | candidate := routeCandidateForAttach(request, *committedIdentity, a.dialer, resumeToken) | |
| 1505 | 1507 | var commitErr error | |
| 1506 | 1508 | if a.killedSelection != nil { | |
| @@ -2138,7 +2140,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 2138 | 2140 | } | |
| 2139 | 2141 | case protocol.AttachTarget: | |
| 2140 | 2142 | target := message | |
| 2141 | - | if a.returnAttachTargets { | |
| 2143 | + | if transientPicker { | |
| 2142 | 2144 | handoff := welcomedResult(nil) | |
| 2143 | 2145 | handoff.target = &target | |
| 2144 | 2146 | return handoff | |
| @@ -2280,6 +2282,9 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 2280 | 2282 | } | |
| 2281 | 2283 | name = identity.Target.SessionName | |
| 2282 | 2284 | committedIdentity = cloneCommittedIdentity(&identity) | |
| 2285 | + | if transientPicker { | |
| 2286 | + | continue | |
| 2287 | + | } | |
| 2283 | 2288 | if a.runner.ledger == nil { | |
| 2284 | 2289 | return welcomedResult(errors.New("vev: route ledger unavailable")) | |
| 2285 | 2290 | } | |
| @@ -607,8 +607,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 607 | finishHandshake: finishHandshake, stopHandshakeTransport: stopHandshakeTransport, | 607 | finishHandshake: finishHandshake, stopHandshakeTransport: stopHandshakeTransport, | ||
| 608 | request: request, clientID: processClientID, milestones: &ms, | 608 | request: request, clientID: processClientID, milestones: &ms, | ||
| 609 | themeState: themeState, enterRaw: enterRaw, reconnect: localReconnect, | 609 | themeState: themeState, enterRaw: enterRaw, reconnect: localReconnect, | ||
| 610 | - | terminalInput: func() *terminalInputPump { return input }, | 610 | + | terminalInput: func() *terminalInputPump { return input }, |
| 611 | - | transientPicker: true, returnAttachTargets: true, | |||
| 612 | }).run(attemptCtx) | 611 | }).run(attemptCtx) | ||
| 613 | stopHandshakeTransport() | 612 | stopHandshakeTransport() | ||
| 614 | finishHandshake() | 613 | finishHandshake() | ||
| @@ -952,6 +951,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 952 | if attemptRequest.StartupOverlay == protocol.StartupOverlaySessionPicker { | 951 | if attemptRequest.StartupOverlay == protocol.StartupOverlaySessionPicker { | ||
| 953 | returnRoute = nil | 952 | returnRoute = nil | ||
| 954 | returnResumeFallback = false | 953 | returnResumeFallback = false | ||
| 954 | + | nextRequest.StartupOverlay = protocol.StartupOverlayNone | |||
| 955 | + | nextRequest.NavigationCapabilities &^= protocol.NavigationCapabilityBack | |||
| 955 | } | 956 | } | ||
| 956 | nextRequest = cloneAttachRequest(nextRequest) | 957 | nextRequest = cloneAttachRequest(nextRequest) | ||
| 957 | if err := validateAttachRequest(nextRequest); err != nil { | 958 | if err := validateAttachRequest(nextRequest); err != nil { | ||
| @@ -1274,8 +1275,6 @@ type attachAttempt struct { | |||||
| 1274 | rememberRemoteHost func() | 1275 | rememberRemoteHost func() | ||
| 1275 | terminalInput func() *terminalInputPump | 1276 | terminalInput func() *terminalInputPump | ||
| 1276 | openHomePicker func(context.Context) attachResult | 1277 | openHomePicker func(context.Context) attachResult | ||
| 1277 | - | transientPicker bool | |||
| 1278 | - | returnAttachTargets bool | |||
| 1279 | } | 1278 | } | ||
| 1280 | 1279 | ||||
| 1281 | type attachResult struct { | 1280 | type attachResult struct { | ||
| @@ -1335,6 +1334,9 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 1335 | transport = a.connection.Connection() | 1334 | transport = a.connection.Connection() | ||
| 1336 | } | 1335 | } | ||
| 1337 | request := a.request | 1336 | request := a.request | ||
| 1337 | + | // Both parked UDP and close-and-dial home pickers only borrow a local | |||
| 1338 | + | // attachment for rendering. They do not own the client's active route. | |||
| 1339 | + | transientPicker := request.StartupOverlay == protocol.StartupOverlaySessionPicker | |||
| 1338 | term := a.runner.term | 1340 | term := a.runner.term | ||
| 1339 | clk := a.runner.clock | 1341 | clk := a.runner.clock | ||
| 1340 | intent := request.Intent | 1342 | intent := request.Intent | ||
| @@ -1500,7 +1502,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 1500 | if request.RemoteTarget != nil && (request.RemoteTarget.LifecycleID != committedIdentity.Target.LifecycleID || request.RemoteTarget.SessionName != committedIdentity.Target.SessionName) { | 1502 | if request.RemoteTarget != nil && (request.RemoteTarget.LifecycleID != committedIdentity.Target.LifecycleID || request.RemoteTarget.SessionName != committedIdentity.Target.SessionName) { | ||
| 1501 | return welcomedResult(errRouteTargetChanged) | 1503 | return welcomedResult(errRouteTargetChanged) | ||
| 1502 | } | 1504 | } | ||
| 1503 | - | if !a.transientPicker { | 1505 | + | if !transientPicker { |
| 1504 | candidate := routeCandidateForAttach(request, *committedIdentity, a.dialer, resumeToken) | 1506 | candidate := routeCandidateForAttach(request, *committedIdentity, a.dialer, resumeToken) | ||
| 1505 | var commitErr error | 1507 | var commitErr error | ||
| 1506 | if a.killedSelection != nil { | 1508 | if a.killedSelection != nil { | ||
| @@ -2138,7 +2140,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 2138 | } | 2140 | } | ||
| 2139 | case protocol.AttachTarget: | 2141 | case protocol.AttachTarget: | ||
| 2140 | target := message | 2142 | target := message | ||
| 2141 | - | if a.returnAttachTargets { | 2143 | + | if transientPicker { |
| 2142 | handoff := welcomedResult(nil) | 2144 | handoff := welcomedResult(nil) | ||
| 2143 | handoff.target = &target | 2145 | handoff.target = &target | ||
| 2144 | return handoff | 2146 | return handoff | ||
| @@ -2280,6 +2282,9 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 2280 | } | 2282 | } | ||
| 2281 | name = identity.Target.SessionName | 2283 | name = identity.Target.SessionName | ||
| 2282 | committedIdentity = cloneCommittedIdentity(&identity) | 2284 | committedIdentity = cloneCommittedIdentity(&identity) | ||
| 2285 | + | if transientPicker { | |||
| 2286 | + | continue | |||
| 2287 | + | } | |||
| 2283 | if a.runner.ledger == nil { | 2288 | if a.runner.ledger == nil { | ||
| 2284 | return welcomedResult(errors.New("vev: route ledger unavailable")) | 2289 | return welcomedResult(errors.New("vev: route ledger unavailable")) | ||
| 2285 | } | 2290 | } | ||
A internal/usecase/client/home_picker_context_test.go +132 -0
| @@ -0,0 +1,132 @@ | |||
| 1 | + | package client_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/ports" | |
| 10 | + | "github.com/bnema/vev/internal/protocol" | |
| 11 | + | "github.com/bnema/vev/internal/protocol/wire" | |
| 12 | + | "github.com/bnema/vev/internal/usecase/client" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestHomePickerPreservesActiveRouteSnapshot(t *testing.T) { | |
| 16 | + | for _, tc := range []struct { | |
| 17 | + | name string | |
| 18 | + | udp bool | |
| 19 | + | rename bool | |
| 20 | + | selectLocal bool | |
| 21 | + | }{ | |
| 22 | + | {name: "UDP", udp: true}, | |
| 23 | + | {name: "stdio"}, | |
| 24 | + | {name: "UDP backing session renamed", udp: true, rename: true}, | |
| 25 | + | {name: "stdio backing session renamed", rename: true}, | |
| 26 | + | {name: "stdio select new local destination", selectLocal: true}, | |
| 27 | + | } { | |
| 28 | + | t.Run(tc.name, func(t *testing.T) { | |
| 29 | + | term := newRunTerminal() | |
| 30 | + | defer term.in.unblock() | |
| 31 | + | localLifecycle := domain.SessionLifecycleID{1} | |
| 32 | + | remoteLifecycle := domain.SessionLifecycleID{2} | |
| 33 | + | target := domain.RemoteSessionTarget{ | |
| 34 | + | Endpoint: "igor", DisplayOrigin: "igor", LifecycleID: remoteLifecycle, | |
| 35 | + | SessionName: "misc", LiveTabID: "remote-tab", | |
| 36 | + | } | |
| 37 | + | initial := hybridLocalBootstrap(localLifecycle, target) | |
| 38 | + | remote := &recordingTransport{recvs: []recvItem{ | |
| 39 | + | {f: hybridWelcomeFrame("misc", remoteLifecycle)}, | |
| 40 | + | {f: navigationDirectiveFrame(protocol.NavigationOpenHomePicker)}, | |
| 41 | + | }} | |
| 42 | + | remoteReturn := &recordingTransport{recvs: []recvItem{ | |
| 43 | + | {f: hybridWelcomeFrame("misc", remoteLifecycle)}, | |
| 44 | + | {f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |
| 45 | + | }} | |
| 46 | + | remoteDialer := &sequenceDialer{trs: []wire.Transport{remote, remoteReturn}} | |
| 47 | + | if tc.udp { | |
| 48 | + | prepareSent, resumeSent := make(chan struct{}), make(chan struct{}) | |
| 49 | + | remote.onSend = hybridParkedRequestHandler(nil, nil, map[protocol.ParkedRouteAction]chan struct{}{ | |
| 50 | + | protocol.ParkedRoutePrepare: prepareSent, | |
| 51 | + | protocol.ParkedRouteResume: resumeSent, | |
| 52 | + | }) | |
| 53 | + | remote.recvs = append(remote.recvs, | |
| 54 | + | recvItem{f: frameOf(wire.MsgParkedRouteResponse, wire.MarshalParkedRouteResponse(protocol.ParkedRouteResponse{RequestID: 1, Status: protocol.ParkedRouteReady})), wait: prepareSent}, | |
| 55 | + | recvItem{f: frameOf(wire.MsgParkedRouteResponse, wire.MarshalParkedRouteResponse(protocol.ParkedRouteResponse{RequestID: 2, Status: protocol.ParkedRouteResumed})), wait: resumeSent}, | |
| 56 | + | recvItem{f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |
| 57 | + | ) | |
| 58 | + | remoteDialer.trs = []wire.Transport{markedDatagramTransport{Transport: remote}} | |
| 59 | + | } | |
| 60 | + | localPicker := &recordingTransport{recvs: []recvItem{{f: hybridWelcomeFrame("local", localLifecycle)}}} | |
| 61 | + | if tc.rename { | |
| 62 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: frameOf(wire.MsgCommittedRouteIdentity, mustMarshalCommittedIdentity(protocol.CommittedRouteIdentity{ | |
| 63 | + | Target: protocol.ExactSessionTarget{LifecycleID: localLifecycle, SessionName: "renamed-local"}, | |
| 64 | + | }))}) | |
| 65 | + | } | |
| 66 | + | localDialer := &sequenceDialer{trs: []wire.Transport{initial, localPicker}} | |
| 67 | + | var localDestination *recordingTransport | |
| 68 | + | if tc.selectLocal { | |
| 69 | + | selected := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{3}, SessionName: "destination"} | |
| 70 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: frameOf(wire.MsgAttachTarget, wire.MarshalAttachTarget(protocol.AttachTarget{ | |
| 71 | + | Session: selected.SessionName, Intent: protocol.IntentAttach, ExactTarget: &selected, | |
| 72 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, SamePeer: true, | |
| 73 | + | }))}) | |
| 74 | + | localDestination = &recordingTransport{recvs: []recvItem{ | |
| 75 | + | {f: hybridWelcomeFrame(selected.SessionName, selected.LifecycleID)}, | |
| 76 | + | {f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |
| 77 | + | }} | |
| 78 | + | localDialer.trs = append(localDialer.trs, localDestination) | |
| 79 | + | } else { | |
| 80 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: navigationDirectiveFrame(protocol.NavigationBack)}) | |
| 81 | + | } | |
| 82 | + | clock := &reconnectTestClock{} | |
| 83 | + | deps := hybridPickerDependencies(localDialer, term, clock, map[string]ports.ClientDialer{"igor": remoteDialer}) | |
| 84 | + | require.NoError(t, runTestClient(t.Context(), deps, client.AttachRequest{ | |
| 85 | + | Intent: protocol.IntentAttach, SessionName: "local", Origin: protocol.RouteOriginLocal, OriginKey: "local", | |
| 86 | + | })) | |
| 87 | + | ||
| 88 | + | var source protocol.RecentRouteSnapshot | |
| 89 | + | for _, sent := range remote.Sends() { | |
| 90 | + | if sent.Type == wire.MsgRecentRouteSnapshot { | |
| 91 | + | var err error | |
| 92 | + | source, err = wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |
| 93 | + | require.NoError(t, err) | |
| 94 | + | break | |
| 95 | + | } | |
| 96 | + | } | |
| 97 | + | require.Equal(t, "misc", source.ActiveEntry.Name) | |
| 98 | + | publications := 0 | |
| 99 | + | for _, sent := range localPicker.Sends() { | |
| 100 | + | if sent.Type != wire.MsgRecentRouteSnapshot { | |
| 101 | + | continue | |
| 102 | + | } | |
| 103 | + | publications++ | |
| 104 | + | snapshot, err := wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |
| 105 | + | require.NoError(t, err) | |
| 106 | + | require.Equal(t, source, snapshot, "rendering the home picker must not commit its backing session as the active route") | |
| 107 | + | } | |
| 108 | + | require.Positive(t, publications) | |
| 109 | + | wantDials := int32(2) | |
| 110 | + | if tc.udp || tc.selectLocal { | |
| 111 | + | wantDials = 1 | |
| 112 | + | } | |
| 113 | + | require.Equal(t, wantDials, remoteDialer.calls.Load()) | |
| 114 | + | if localDestination != nil { | |
| 115 | + | hello := helloFromSend(t, localDestination) | |
| 116 | + | require.Equal(t, protocol.StartupOverlayNone, hello.StartupOverlay) | |
| 117 | + | require.Zero(t, hello.NavigationCapabilities&protocol.NavigationCapabilityBack) | |
| 118 | + | var selected protocol.RecentRouteSnapshot | |
| 119 | + | for _, sent := range localDestination.Sends() { | |
| 120 | + | if sent.Type == wire.MsgRecentRouteSnapshot { | |
| 121 | + | var err error | |
| 122 | + | selected, err = wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |
| 123 | + | require.NoError(t, err) | |
| 124 | + | } | |
| 125 | + | } | |
| 126 | + | require.Equal(t, "destination", selected.ActiveEntry.Name) | |
| 127 | + | require.Equal(t, protocol.RouteKindLocal, selected.ActiveEntry.Kind) | |
| 128 | + | require.Equal(t, source.Active, selected.Previous) | |
| 129 | + | } | |
| 130 | + | }) | |
| 131 | + | } | |
| 132 | + | } | |
| @@ -0,0 +1,132 @@ | |||||
| 1 | + | package client_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/ports" | |||
| 10 | + | "github.com/bnema/vev/internal/protocol" | |||
| 11 | + | "github.com/bnema/vev/internal/protocol/wire" | |||
| 12 | + | "github.com/bnema/vev/internal/usecase/client" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestHomePickerPreservesActiveRouteSnapshot(t *testing.T) { | |||
| 16 | + | for _, tc := range []struct { | |||
| 17 | + | name string | |||
| 18 | + | udp bool | |||
| 19 | + | rename bool | |||
| 20 | + | selectLocal bool | |||
| 21 | + | }{ | |||
| 22 | + | {name: "UDP", udp: true}, | |||
| 23 | + | {name: "stdio"}, | |||
| 24 | + | {name: "UDP backing session renamed", udp: true, rename: true}, | |||
| 25 | + | {name: "stdio backing session renamed", rename: true}, | |||
| 26 | + | {name: "stdio select new local destination", selectLocal: true}, | |||
| 27 | + | } { | |||
| 28 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 29 | + | term := newRunTerminal() | |||
| 30 | + | defer term.in.unblock() | |||
| 31 | + | localLifecycle := domain.SessionLifecycleID{1} | |||
| 32 | + | remoteLifecycle := domain.SessionLifecycleID{2} | |||
| 33 | + | target := domain.RemoteSessionTarget{ | |||
| 34 | + | Endpoint: "igor", DisplayOrigin: "igor", LifecycleID: remoteLifecycle, | |||
| 35 | + | SessionName: "misc", LiveTabID: "remote-tab", | |||
| 36 | + | } | |||
| 37 | + | initial := hybridLocalBootstrap(localLifecycle, target) | |||
| 38 | + | remote := &recordingTransport{recvs: []recvItem{ | |||
| 39 | + | {f: hybridWelcomeFrame("misc", remoteLifecycle)}, | |||
| 40 | + | {f: navigationDirectiveFrame(protocol.NavigationOpenHomePicker)}, | |||
| 41 | + | }} | |||
| 42 | + | remoteReturn := &recordingTransport{recvs: []recvItem{ | |||
| 43 | + | {f: hybridWelcomeFrame("misc", remoteLifecycle)}, | |||
| 44 | + | {f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |||
| 45 | + | }} | |||
| 46 | + | remoteDialer := &sequenceDialer{trs: []wire.Transport{remote, remoteReturn}} | |||
| 47 | + | if tc.udp { | |||
| 48 | + | prepareSent, resumeSent := make(chan struct{}), make(chan struct{}) | |||
| 49 | + | remote.onSend = hybridParkedRequestHandler(nil, nil, map[protocol.ParkedRouteAction]chan struct{}{ | |||
| 50 | + | protocol.ParkedRoutePrepare: prepareSent, | |||
| 51 | + | protocol.ParkedRouteResume: resumeSent, | |||
| 52 | + | }) | |||
| 53 | + | remote.recvs = append(remote.recvs, | |||
| 54 | + | recvItem{f: frameOf(wire.MsgParkedRouteResponse, wire.MarshalParkedRouteResponse(protocol.ParkedRouteResponse{RequestID: 1, Status: protocol.ParkedRouteReady})), wait: prepareSent}, | |||
| 55 | + | recvItem{f: frameOf(wire.MsgParkedRouteResponse, wire.MarshalParkedRouteResponse(protocol.ParkedRouteResponse{RequestID: 2, Status: protocol.ParkedRouteResumed})), wait: resumeSent}, | |||
| 56 | + | recvItem{f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |||
| 57 | + | ) | |||
| 58 | + | remoteDialer.trs = []wire.Transport{markedDatagramTransport{Transport: remote}} | |||
| 59 | + | } | |||
| 60 | + | localPicker := &recordingTransport{recvs: []recvItem{{f: hybridWelcomeFrame("local", localLifecycle)}}} | |||
| 61 | + | if tc.rename { | |||
| 62 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: frameOf(wire.MsgCommittedRouteIdentity, mustMarshalCommittedIdentity(protocol.CommittedRouteIdentity{ | |||
| 63 | + | Target: protocol.ExactSessionTarget{LifecycleID: localLifecycle, SessionName: "renamed-local"}, | |||
| 64 | + | }))}) | |||
| 65 | + | } | |||
| 66 | + | localDialer := &sequenceDialer{trs: []wire.Transport{initial, localPicker}} | |||
| 67 | + | var localDestination *recordingTransport | |||
| 68 | + | if tc.selectLocal { | |||
| 69 | + | selected := protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{3}, SessionName: "destination"} | |||
| 70 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: frameOf(wire.MsgAttachTarget, wire.MarshalAttachTarget(protocol.AttachTarget{ | |||
| 71 | + | Session: selected.SessionName, Intent: protocol.IntentAttach, ExactTarget: &selected, | |||
| 72 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, SamePeer: true, | |||
| 73 | + | }))}) | |||
| 74 | + | localDestination = &recordingTransport{recvs: []recvItem{ | |||
| 75 | + | {f: hybridWelcomeFrame(selected.SessionName, selected.LifecycleID)}, | |||
| 76 | + | {f: frameOf(wire.MsgDetached, wire.MarshalDetached(protocol.Detached{Reason: protocol.ReasonDetach}))}, | |||
| 77 | + | }} | |||
| 78 | + | localDialer.trs = append(localDialer.trs, localDestination) | |||
| 79 | + | } else { | |||
| 80 | + | localPicker.recvs = append(localPicker.recvs, recvItem{f: navigationDirectiveFrame(protocol.NavigationBack)}) | |||
| 81 | + | } | |||
| 82 | + | clock := &reconnectTestClock{} | |||
| 83 | + | deps := hybridPickerDependencies(localDialer, term, clock, map[string]ports.ClientDialer{"igor": remoteDialer}) | |||
| 84 | + | require.NoError(t, runTestClient(t.Context(), deps, client.AttachRequest{ | |||
| 85 | + | Intent: protocol.IntentAttach, SessionName: "local", Origin: protocol.RouteOriginLocal, OriginKey: "local", | |||
| 86 | + | })) | |||
| 87 | + | ||||
| 88 | + | var source protocol.RecentRouteSnapshot | |||
| 89 | + | for _, sent := range remote.Sends() { | |||
| 90 | + | if sent.Type == wire.MsgRecentRouteSnapshot { | |||
| 91 | + | var err error | |||
| 92 | + | source, err = wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |||
| 93 | + | require.NoError(t, err) | |||
| 94 | + | break | |||
| 95 | + | } | |||
| 96 | + | } | |||
| 97 | + | require.Equal(t, "misc", source.ActiveEntry.Name) | |||
| 98 | + | publications := 0 | |||
| 99 | + | for _, sent := range localPicker.Sends() { | |||
| 100 | + | if sent.Type != wire.MsgRecentRouteSnapshot { | |||
| 101 | + | continue | |||
| 102 | + | } | |||
| 103 | + | publications++ | |||
| 104 | + | snapshot, err := wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |||
| 105 | + | require.NoError(t, err) | |||
| 106 | + | require.Equal(t, source, snapshot, "rendering the home picker must not commit its backing session as the active route") | |||
| 107 | + | } | |||
| 108 | + | require.Positive(t, publications) | |||
| 109 | + | wantDials := int32(2) | |||
| 110 | + | if tc.udp || tc.selectLocal { | |||
| 111 | + | wantDials = 1 | |||
| 112 | + | } | |||
| 113 | + | require.Equal(t, wantDials, remoteDialer.calls.Load()) | |||
| 114 | + | if localDestination != nil { | |||
| 115 | + | hello := helloFromSend(t, localDestination) | |||
| 116 | + | require.Equal(t, protocol.StartupOverlayNone, hello.StartupOverlay) | |||
| 117 | + | require.Zero(t, hello.NavigationCapabilities&protocol.NavigationCapabilityBack) | |||
| 118 | + | var selected protocol.RecentRouteSnapshot | |||
| 119 | + | for _, sent := range localDestination.Sends() { | |||
| 120 | + | if sent.Type == wire.MsgRecentRouteSnapshot { | |||
| 121 | + | var err error | |||
| 122 | + | selected, err = wire.UnmarshalRecentRouteSnapshot(sent.Payload) | |||
| 123 | + | require.NoError(t, err) | |||
| 124 | + | } | |||
| 125 | + | } | |||
| 126 | + | require.Equal(t, "destination", selected.ActiveEntry.Name) | |||
| 127 | + | require.Equal(t, protocol.RouteKindLocal, selected.ActiveEntry.Kind) | |||
| 128 | + | require.Equal(t, source.Active, selected.Previous) | |||
| 129 | + | } | |||
| 130 | + | }) | |||
| 131 | + | } | |||
| 132 | + | } | |||
A internal/usecase/daemon/home_picker_status_test.go +75 -0
| @@ -0,0 +1,75 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/protocol" | |
| 10 | + | ) | |
| 11 | + | ||
| 12 | + | func TestHomePickerStatusKeepsActiveRemoteRoute(t *testing.T) { | |
| 13 | + | for _, tc := range []struct { | |
| 14 | + | name string | |
| 15 | + | homePicker bool | |
| 16 | + | noSnapshot bool | |
| 17 | + | staleRef bool | |
| 18 | + | ephemeral bool | |
| 19 | + | wantSession string | |
| 20 | + | }{ | |
| 21 | + | {name: "ordinary local picker ignores unrelated route", wantSession: "vev"}, | |
| 22 | + | {name: "remote home picker", homePicker: true, wantSession: "misc@igor"}, | |
| 23 | + | {name: "ephemeral remote route", homePicker: true, ephemeral: true, wantSession: "misc*@igor"}, | |
| 24 | + | {name: "before route publication", homePicker: true, noSnapshot: true}, | |
| 25 | + | {name: "inconsistent active reference", homePicker: true, staleRef: true}, | |
| 26 | + | } { | |
| 27 | + | t.Run(tc.name, func(t *testing.T) { | |
| 28 | + | d, sess, ac, _, releases := newManualTabSession(t, 1) | |
| 29 | + | defer releases[0]() | |
| 30 | + | sess.name = "vev" | |
| 31 | + | sess.incarnation = domain.SessionLifecycleID{1} | |
| 32 | + | d.barScripts = &barScriptState{outputs: map[domain.SessionID]barScriptOutputs{ | |
| 33 | + | sess.id: {topRight: "local top script", bottomRight: "local bottom script"}, | |
| 34 | + | }} | |
| 35 | + | if tc.homePicker { | |
| 36 | + | ac.startupOverlay = protocol.StartupOverlaySessionPicker | |
| 37 | + | ac.navigationCapabilities = protocol.NavigationCapabilityBack | |
| 38 | + | } | |
| 39 | + | ref := protocol.RouteRef{Key: 2, Generation: 1} | |
| 40 | + | snapshot := protocol.RecentRouteSnapshot{ | |
| 41 | + | Generation: 2, Active: ref, | |
| 42 | + | ActiveEntry: protocol.RecentRouteEntry{ | |
| 43 | + | Key: ref.Key, Generation: ref.Generation, | |
| 44 | + | Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "misc"}, | |
| 45 | + | Name: "misc", HostLabel: "dev@igor", Kind: protocol.RouteKindRemote, Ephemeral: tc.ephemeral, | |
| 46 | + | }, | |
| 47 | + | } | |
| 48 | + | if tc.staleRef { | |
| 49 | + | snapshot.Active.Generation++ | |
| 50 | + | } | |
| 51 | + | if !tc.noSnapshot { | |
| 52 | + | ac.setRouteSnapshot(snapshot) | |
| 53 | + | } | |
| 54 | + | d.enterPicker(sess, ac) | |
| 55 | + | require.True(t, ac.overlays.pickerActive()) | |
| 56 | + | bars := d.barStateForAttachmentPaletteHintsFor(sess, ac, "", nil, protocol.RecentRouteSnapshot{}) | |
| 57 | + | frame := composeFrame(capturedRenderState{window: domain.Size{Cols: 80, Rows: 24}, bars: bars}, composeCacheInput{}).frame | |
| 58 | + | require.Equal(t, tc.wantSession, bars.status.session) | |
| 59 | + | if tc.wantSession != "" { | |
| 60 | + | require.Contains(t, rowText(frame.Row(frame.Height-1)), tc.wantSession) | |
| 61 | + | } | |
| 62 | + | if tc.homePicker { | |
| 63 | + | require.Empty(t, bars.status.tabs, "local tabs must not masquerade as remote tabs") | |
| 64 | + | require.Empty(t, bars.topRight) | |
| 65 | + | require.Empty(t, bars.bottomRight) | |
| 66 | + | require.NotContains(t, rowText(frame.Row(frame.Height-1)), "vev") | |
| 67 | + | } else { | |
| 68 | + | require.Len(t, bars.status.tabs, 1) | |
| 69 | + | require.Equal(t, "local top script", bars.topRight) | |
| 70 | + | require.Equal(t, "local bottom script", bars.bottomRight) | |
| 71 | + | } | |
| 72 | + | require.Equal(t, "vev", sess.statusSegments(true).session, "picker presentation must not rename its local backing session") | |
| 73 | + | }) | |
| 74 | + | } | |
| 75 | + | } | |
| @@ -0,0 +1,75 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/protocol" | |||
| 10 | + | ) | |||
| 11 | + | ||||
| 12 | + | func TestHomePickerStatusKeepsActiveRemoteRoute(t *testing.T) { | |||
| 13 | + | for _, tc := range []struct { | |||
| 14 | + | name string | |||
| 15 | + | homePicker bool | |||
| 16 | + | noSnapshot bool | |||
| 17 | + | staleRef bool | |||
| 18 | + | ephemeral bool | |||
| 19 | + | wantSession string | |||
| 20 | + | }{ | |||
| 21 | + | {name: "ordinary local picker ignores unrelated route", wantSession: "vev"}, | |||
| 22 | + | {name: "remote home picker", homePicker: true, wantSession: "misc@igor"}, | |||
| 23 | + | {name: "ephemeral remote route", homePicker: true, ephemeral: true, wantSession: "misc*@igor"}, | |||
| 24 | + | {name: "before route publication", homePicker: true, noSnapshot: true}, | |||
| 25 | + | {name: "inconsistent active reference", homePicker: true, staleRef: true}, | |||
| 26 | + | } { | |||
| 27 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 28 | + | d, sess, ac, _, releases := newManualTabSession(t, 1) | |||
| 29 | + | defer releases[0]() | |||
| 30 | + | sess.name = "vev" | |||
| 31 | + | sess.incarnation = domain.SessionLifecycleID{1} | |||
| 32 | + | d.barScripts = &barScriptState{outputs: map[domain.SessionID]barScriptOutputs{ | |||
| 33 | + | sess.id: {topRight: "local top script", bottomRight: "local bottom script"}, | |||
| 34 | + | }} | |||
| 35 | + | if tc.homePicker { | |||
| 36 | + | ac.startupOverlay = protocol.StartupOverlaySessionPicker | |||
| 37 | + | ac.navigationCapabilities = protocol.NavigationCapabilityBack | |||
| 38 | + | } | |||
| 39 | + | ref := protocol.RouteRef{Key: 2, Generation: 1} | |||
| 40 | + | snapshot := protocol.RecentRouteSnapshot{ | |||
| 41 | + | Generation: 2, Active: ref, | |||
| 42 | + | ActiveEntry: protocol.RecentRouteEntry{ | |||
| 43 | + | Key: ref.Key, Generation: ref.Generation, | |||
| 44 | + | Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{2}, SessionName: "misc"}, | |||
| 45 | + | Name: "misc", HostLabel: "dev@igor", Kind: protocol.RouteKindRemote, Ephemeral: tc.ephemeral, | |||
| 46 | + | }, | |||
| 47 | + | } | |||
| 48 | + | if tc.staleRef { | |||
| 49 | + | snapshot.Active.Generation++ | |||
| 50 | + | } | |||
| 51 | + | if !tc.noSnapshot { | |||
| 52 | + | ac.setRouteSnapshot(snapshot) | |||
| 53 | + | } | |||
| 54 | + | d.enterPicker(sess, ac) | |||
| 55 | + | require.True(t, ac.overlays.pickerActive()) | |||
| 56 | + | bars := d.barStateForAttachmentPaletteHintsFor(sess, ac, "", nil, protocol.RecentRouteSnapshot{}) | |||
| 57 | + | frame := composeFrame(capturedRenderState{window: domain.Size{Cols: 80, Rows: 24}, bars: bars}, composeCacheInput{}).frame | |||
| 58 | + | require.Equal(t, tc.wantSession, bars.status.session) | |||
| 59 | + | if tc.wantSession != "" { | |||
| 60 | + | require.Contains(t, rowText(frame.Row(frame.Height-1)), tc.wantSession) | |||
| 61 | + | } | |||
| 62 | + | if tc.homePicker { | |||
| 63 | + | require.Empty(t, bars.status.tabs, "local tabs must not masquerade as remote tabs") | |||
| 64 | + | require.Empty(t, bars.topRight) | |||
| 65 | + | require.Empty(t, bars.bottomRight) | |||
| 66 | + | require.NotContains(t, rowText(frame.Row(frame.Height-1)), "vev") | |||
| 67 | + | } else { | |||
| 68 | + | require.Len(t, bars.status.tabs, 1) | |||
| 69 | + | require.Equal(t, "local top script", bars.topRight) | |||
| 70 | + | require.Equal(t, "local bottom script", bars.bottomRight) | |||
| 71 | + | } | |||
| 72 | + | require.Equal(t, "vev", sess.statusSegments(true).session, "picker presentation must not rename its local backing session") | |||
| 73 | + | }) | |||
| 74 | + | } | |||
| 75 | + | } | |||
M internal/usecase/daemon/status.go +16 -2
| @@ -283,14 +283,28 @@ func (d *Daemon) barStateForAttachmentPaletteHintsFor(cur *session, ac *attached | |||
| 283 | 283 | if d != nil { | |
| 284 | 284 | state.attentionFrame = d.attentionFrame() | |
| 285 | 285 | } | |
| 286 | - | if cur != nil { | |
| 286 | + | homePicker := ac != nil && ac.startupOverlay == protocol.StartupOverlaySessionPicker && ac.navigationCapabilities&protocol.NavigationCapabilityBack != 0 | |
| 287 | + | if homePicker { | |
| 288 | + | // The local session only hosts the picker. Its name, tabs and bar | |
| 289 | + | // scripts do not describe the remote route the user is still on. | |
| 290 | + | // Leave that chrome empty until the client publishes its snapshot. | |
| 291 | + | if active, ok := activeRouteEntryForLifecycle(routeSnapshot, routeSnapshot.ActiveEntry.Target.LifecycleID); ok { | |
| 292 | + | kind := recentRouteLocal | |
| 293 | + | if active.Kind == protocol.RouteKindRemote { | |
| 294 | + | kind = recentRouteRemote | |
| 295 | + | } | |
| 296 | + | state.status.session = formatRecentRouteName(recentRoutePresentation{ | |
| 297 | + | name: active.Name, hostLabel: domain.RemoteDisplayOrigin(active.HostLabel), kind: kind, ephemeral: active.Ephemeral, | |
| 298 | + | }) | |
| 299 | + | } | |
| 300 | + | } else if cur != nil { | |
| 287 | 301 | includeTerminalTitle := true | |
| 288 | 302 | if d != nil { | |
| 289 | 303 | includeTerminalTitle = d.currentTabsConfig().TerminalTitle | |
| 290 | 304 | } | |
| 291 | 305 | state.status = cur.statusSegmentsFor(ac, includeTerminalTitle) | |
| 292 | 306 | } | |
| 293 | - | if cur != nil && d != nil { | |
| 307 | + | if !homePicker && cur != nil && d != nil { | |
| 294 | 308 | state.topRight, state.bottomRight = d.barScriptSnapshot(cur) | |
| 295 | 309 | } | |
| 296 | 310 | if ranked != nil { | |
| @@ -283,14 +283,28 @@ func (d *Daemon) barStateForAttachmentPaletteHintsFor(cur *session, ac *attached | |||||
| 283 | if d != nil { | 283 | if d != nil { | ||
| 284 | state.attentionFrame = d.attentionFrame() | 284 | state.attentionFrame = d.attentionFrame() | ||
| 285 | } | 285 | } | ||
| 286 | - | if cur != nil { | 286 | + | homePicker := ac != nil && ac.startupOverlay == protocol.StartupOverlaySessionPicker && ac.navigationCapabilities&protocol.NavigationCapabilityBack != 0 |
| 287 | + | if homePicker { | |||
| 288 | + | // The local session only hosts the picker. Its name, tabs and bar | |||
| 289 | + | // scripts do not describe the remote route the user is still on. | |||
| 290 | + | // Leave that chrome empty until the client publishes its snapshot. | |||
| 291 | + | if active, ok := activeRouteEntryForLifecycle(routeSnapshot, routeSnapshot.ActiveEntry.Target.LifecycleID); ok { | |||
| 292 | + | kind := recentRouteLocal | |||
| 293 | + | if active.Kind == protocol.RouteKindRemote { | |||
| 294 | + | kind = recentRouteRemote | |||
| 295 | + | } | |||
| 296 | + | state.status.session = formatRecentRouteName(recentRoutePresentation{ | |||
| 297 | + | name: active.Name, hostLabel: domain.RemoteDisplayOrigin(active.HostLabel), kind: kind, ephemeral: active.Ephemeral, | |||
| 298 | + | }) | |||
| 299 | + | } | |||
| 300 | + | } else if cur != nil { | |||
| 287 | includeTerminalTitle := true | 301 | includeTerminalTitle := true | ||
| 288 | if d != nil { | 302 | if d != nil { | ||
| 289 | includeTerminalTitle = d.currentTabsConfig().TerminalTitle | 303 | includeTerminalTitle = d.currentTabsConfig().TerminalTitle | ||
| 290 | } | 304 | } | ||
| 291 | state.status = cur.statusSegmentsFor(ac, includeTerminalTitle) | 305 | state.status = cur.statusSegmentsFor(ac, includeTerminalTitle) | ||
| 292 | } | 306 | } | ||
| 293 | - | if cur != nil && d != nil { | 307 | + | if !homePicker && cur != nil && d != nil { |
| 294 | state.topRight, state.bottomRight = d.barScriptSnapshot(cur) | 308 | state.topRight, state.bottomRight = d.barScriptSnapshot(cur) | ||
| 295 | } | 309 | } | ||
| 296 | if ranked != nil { | 310 | if ranked != nil { | ||