fix(remote): address ownership recovery review findings
- commit
- 92a7932227293392c828956dd82c3c55287e7103
- author
- brice · · 2026-08-08 17:06 UTC
- parent
- f019e0f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +143 -46
M internal/ports/frame.go +2 -2
| @@ -22,7 +22,7 @@ const MaxFrameLen = 16 << 20 | |||
| 22 | 22 | type MsgType uint8 | |
| 23 | 23 | ||
| 24 | 24 | // Frame message types. Client-originated messages occupy 1–13 and 15; | |
| 25 | - | // server-originated messages occupy 16–22 and 25–27. Values 14, 23–24 remain | |
| 25 | + | // server-originated messages occupy 16–23 and 25. Values 14 and 24 remain | |
| 26 | 26 | // reserved for future extensions. | |
| 27 | 27 | const ( | |
| 28 | 28 | MsgHello MsgType = 1 | |
| @@ -49,7 +49,7 @@ const ( | |||
| 49 | 49 | MsgCommandResult MsgType = 22 | |
| 50 | 50 | MsgAttachTarget MsgType = 25 | |
| 51 | 51 | MsgRemotePreviewResponse MsgType = 26 | |
| 52 | - | MsgNavigationAction MsgType = 27 | |
| 52 | + | MsgNavigationAction MsgType = 23 | |
| 53 | 53 | ) | |
| 54 | 54 | ||
| 55 | 55 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | |
| @@ -22,7 +22,7 @@ const MaxFrameLen = 16 << 20 | |||||
| 22 | type MsgType uint8 | 22 | type MsgType uint8 | ||
| 23 | 23 | ||||
| 24 | // Frame message types. Client-originated messages occupy 1–13 and 15; | 24 | // Frame message types. Client-originated messages occupy 1–13 and 15; | ||
| 25 | - | // server-originated messages occupy 16–22 and 25–27. Values 14, 23–24 remain | 25 | + | // server-originated messages occupy 16–23 and 25. Values 14 and 24 remain |
| 26 | // reserved for future extensions. | 26 | // reserved for future extensions. | ||
| 27 | const ( | 27 | const ( | ||
| 28 | MsgHello MsgType = 1 | 28 | MsgHello MsgType = 1 | ||
| @@ -49,7 +49,7 @@ const ( | |||||
| 49 | MsgCommandResult MsgType = 22 | 49 | MsgCommandResult MsgType = 22 | ||
| 50 | MsgAttachTarget MsgType = 25 | 50 | MsgAttachTarget MsgType = 25 | ||
| 51 | MsgRemotePreviewResponse MsgType = 26 | 51 | MsgRemotePreviewResponse MsgType = 26 | ||
| 52 | - | MsgNavigationAction MsgType = 27 | 52 | + | MsgNavigationAction MsgType = 23 |
| 53 | ) | 53 | ) | ||
| 54 | 54 | ||||
| 55 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | 55 | // Frame is the unit of exchange over a Transport: a typed, length-delimited | ||
M internal/ports/navigation_test.go +10 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package ports | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "encoding/hex" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/internal/domain" | |
| @@ -49,7 +50,9 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||
| 49 | 50 | overlay StartupOverlay | |
| 50 | 51 | }{ | |
| 51 | 52 | {name: "ordinary", hello: base, valid: true}, | |
| 53 | + | {name: "resume route", hello: Hello{Version: ProtocolVersion, Intent: IntentResume, Size: domain.Size{Cols: 80, Rows: 24}}, valid: true}, | |
| 52 | 54 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker}, | |
| 55 | + | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | |
| 53 | 56 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | |
| 54 | 57 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | |
| 55 | 58 | {name: "picker without back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | |
| @@ -62,6 +65,13 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||
| 62 | 65 | require.NoError(t, err) | |
| 63 | 66 | payload := MarshalHello(tt.hello) | |
| 64 | 67 | require.NotNil(t, payload) | |
| 68 | + | if tt.name == "startup picker requires back" { | |
| 69 | + | expected, decodeErr := hex.DecodeString("001a02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "0201") | |
| 70 | + | require.NoError(t, decodeErr) | |
| 71 | + | require.Equal(t, expected, payload) | |
| 72 | + | assertAllPrefixesFail(t, expected, UnmarshalHello) | |
| 73 | + | assertTrailingGarbageFails(t, expected, UnmarshalHello) | |
| 74 | + | } | |
| 65 | 75 | decoded, decodeErr := UnmarshalHello(payload) | |
| 66 | 76 | require.NoError(t, decodeErr) | |
| 67 | 77 | require.Equal(t, tt.capability, decoded.NavigationCapabilities) | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package ports | 1 | package ports | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "encoding/hex" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| @@ -49,7 +50,9 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||||
| 49 | overlay StartupOverlay | 50 | overlay StartupOverlay | ||
| 50 | }{ | 51 | }{ | ||
| 51 | {name: "ordinary", hello: base, valid: true}, | 52 | {name: "ordinary", hello: base, valid: true}, | ||
| 53 | + | {name: "resume route", hello: Hello{Version: ProtocolVersion, Intent: IntentResume, Size: domain.Size{Cols: 80, Rows: 24}}, valid: true}, | |||
| 52 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker}, | 54 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker}, | ||
| 55 | + | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | |||
| 53 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | 56 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | ||
| 54 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | 57 | {name: "back without picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack}, valid: false}, | ||
| 55 | {name: "picker without back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | 58 | {name: "picker without back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | ||
| @@ -62,6 +65,13 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||||
| 62 | require.NoError(t, err) | 65 | require.NoError(t, err) | ||
| 63 | payload := MarshalHello(tt.hello) | 66 | payload := MarshalHello(tt.hello) | ||
| 64 | require.NotNil(t, payload) | 67 | require.NotNil(t, payload) | ||
| 68 | + | if tt.name == "startup picker requires back" { | |||
| 69 | + | expected, decodeErr := hex.DecodeString("001a02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "0201") | |||
| 70 | + | require.NoError(t, decodeErr) | |||
| 71 | + | require.Equal(t, expected, payload) | |||
| 72 | + | assertAllPrefixesFail(t, expected, UnmarshalHello) | |||
| 73 | + | assertTrailingGarbageFails(t, expected, UnmarshalHello) | |||
| 74 | + | } | |||
| 65 | decoded, decodeErr := UnmarshalHello(payload) | 75 | decoded, decodeErr := UnmarshalHello(payload) | ||
| 66 | require.NoError(t, decodeErr) | 76 | require.NoError(t, decodeErr) | ||
| 67 | require.Equal(t, tt.capability, decoded.NavigationCapabilities) | 77 | require.Equal(t, tt.capability, decoded.NavigationCapabilities) | ||
M internal/ports/wire.go +6 -0
| @@ -101,6 +101,12 @@ func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOver | |||
| 101 | 101 | } | |
| 102 | 102 | ||
| 103 | 103 | func validateHelloNavigation(h Hello) error { | |
| 104 | + | if h.Intent != IntentAttach && h.Intent != IntentResume { | |
| 105 | + | if h.NavigationCapabilities != 0 || h.StartupOverlay != StartupOverlayNone { | |
| 106 | + | return ErrInvalidNavigation | |
| 107 | + | } | |
| 108 | + | return nil | |
| 109 | + | } | |
| 104 | 110 | return ValidateNavigation(h.NavigationCapabilities, h.StartupOverlay, h.RemoteTarget != nil || h.EnvironmentPolicy == EnvironmentPolicyDaemonOwned) | |
| 105 | 111 | } | |
| 106 | 112 | ||
| @@ -101,6 +101,12 @@ func ValidateNavigation(capabilities NavigationCapabilities, overlay StartupOver | |||||
| 101 | } | 101 | } | ||
| 102 | 102 | ||||
| 103 | func validateHelloNavigation(h Hello) error { | 103 | func validateHelloNavigation(h Hello) error { | ||
| 104 | + | if h.Intent != IntentAttach && h.Intent != IntentResume { | |||
| 105 | + | if h.NavigationCapabilities != 0 || h.StartupOverlay != StartupOverlayNone { | |||
| 106 | + | return ErrInvalidNavigation | |||
| 107 | + | } | |||
| 108 | + | return nil | |||
| 109 | + | } | |||
| 104 | return ValidateNavigation(h.NavigationCapabilities, h.StartupOverlay, h.RemoteTarget != nil || h.EnvironmentPolicy == EnvironmentPolicyDaemonOwned) | 110 | return ValidateNavigation(h.NavigationCapabilities, h.StartupOverlay, h.RemoteTarget != nil || h.EnvironmentPolicy == EnvironmentPolicyDaemonOwned) | ||
| 105 | } | 111 | } | ||
| 106 | 112 | ||||
M internal/usecase/client/client.go +10 -10
| @@ -178,7 +178,8 @@ type AttachRequest struct { | |||
| 178 | 178 | StartupOverlay ports.StartupOverlay | |
| 179 | 179 | } | |
| 180 | 180 | ||
| 181 | - | // Runner owns the client lifecycle across one or more attachment attempts. | |
| 181 | + | // attachRoute captures the dialer, request, and resume token needed to | |
| 182 | + | // restore a previous attachment route after a navigation handoff. | |
| 182 | 183 | type attachRoute struct { | |
| 183 | 184 | dialer ports.Dialer | |
| 184 | 185 | request AttachRequest | |
| @@ -490,6 +491,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 490 | 491 | return errors.New("vev: stale return navigation action") | |
| 491 | 492 | } | |
| 492 | 493 | route := *returnRoute | |
| 494 | + | returnRoute = nil | |
| 495 | + | homeNavigationPending = false | |
| 493 | 496 | returnNavigationPending = true | |
| 494 | 497 | dialer = route.dialer | |
| 495 | 498 | attemptRequest = route.request | |
| @@ -549,13 +552,10 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 549 | 552 | attemptRequest.SessionName = result.sessionName | |
| 550 | 553 | } | |
| 551 | 554 | if result.err == nil { | |
| 552 | - | if result.welcomed { | |
| 553 | - | homeNavigationPending = false | |
| 554 | - | if returnNavigationPending { | |
| 555 | - | returnRoute = nil | |
| 556 | - | returnNavigationPending = false | |
| 557 | - | returnResumeFallback = false | |
| 558 | - | } | |
| 555 | + | if result.welcomed && returnNavigationPending { | |
| 556 | + | returnRoute = nil | |
| 557 | + | returnNavigationPending = false | |
| 558 | + | returnResumeFallback = false | |
| 559 | 559 | } | |
| 560 | 560 | if clearErr := reconnect.clear(); clearErr != nil { | |
| 561 | 561 | return clearErr | |
| @@ -566,8 +566,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 566 | 566 | route := *returnRoute | |
| 567 | 567 | returnRoute = nil | |
| 568 | 568 | homeNavigationPending = false | |
| 569 | - | returnNavigationPending = false | |
| 570 | - | returnResumeFallback = false | |
| 569 | + | returnNavigationPending = true | |
| 570 | + | returnResumeFallback = true | |
| 571 | 571 | dialer = route.dialer | |
| 572 | 572 | attemptRequest = route.request | |
| 573 | 573 | attemptRequest.Intent = ports.IntentResume | |
| @@ -178,7 +178,8 @@ type AttachRequest struct { | |||||
| 178 | StartupOverlay ports.StartupOverlay | 178 | StartupOverlay ports.StartupOverlay | ||
| 179 | } | 179 | } | ||
| 180 | 180 | ||||
| 181 | - | // Runner owns the client lifecycle across one or more attachment attempts. | 181 | + | // attachRoute captures the dialer, request, and resume token needed to |
| 182 | + | // restore a previous attachment route after a navigation handoff. | |||
| 182 | type attachRoute struct { | 183 | type attachRoute struct { | ||
| 183 | dialer ports.Dialer | 184 | dialer ports.Dialer | ||
| 184 | request AttachRequest | 185 | request AttachRequest | ||
| @@ -490,6 +491,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 490 | return errors.New("vev: stale return navigation action") | 491 | return errors.New("vev: stale return navigation action") | ||
| 491 | } | 492 | } | ||
| 492 | route := *returnRoute | 493 | route := *returnRoute | ||
| 494 | + | returnRoute = nil | |||
| 495 | + | homeNavigationPending = false | |||
| 493 | returnNavigationPending = true | 496 | returnNavigationPending = true | ||
| 494 | dialer = route.dialer | 497 | dialer = route.dialer | ||
| 495 | attemptRequest = route.request | 498 | attemptRequest = route.request | ||
| @@ -549,13 +552,10 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 549 | attemptRequest.SessionName = result.sessionName | 552 | attemptRequest.SessionName = result.sessionName | ||
| 550 | } | 553 | } | ||
| 551 | if result.err == nil { | 554 | if result.err == nil { | ||
| 552 | - | if result.welcomed { | 555 | + | if result.welcomed && returnNavigationPending { |
| 553 | - | homeNavigationPending = false | 556 | + | returnRoute = nil |
| 554 | - | if returnNavigationPending { | 557 | + | returnNavigationPending = false |
| 555 | - | returnRoute = nil | 558 | + | returnResumeFallback = false |
| 556 | - | returnNavigationPending = false | |||
| 557 | - | returnResumeFallback = false | |||
| 558 | - | } | |||
| 559 | } | 559 | } | ||
| 560 | if clearErr := reconnect.clear(); clearErr != nil { | 560 | if clearErr := reconnect.clear(); clearErr != nil { | ||
| 561 | return clearErr | 561 | return clearErr | ||
| @@ -566,8 +566,8 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 566 | route := *returnRoute | 566 | route := *returnRoute | ||
| 567 | returnRoute = nil | 567 | returnRoute = nil | ||
| 568 | homeNavigationPending = false | 568 | homeNavigationPending = false | ||
| 569 | - | returnNavigationPending = false | 569 | + | returnNavigationPending = true |
| 570 | - | returnResumeFallback = false | 570 | + | returnResumeFallback = true |
| 571 | dialer = route.dialer | 571 | dialer = route.dialer | ||
| 572 | attemptRequest = route.request | 572 | attemptRequest = route.request | ||
| 573 | attemptRequest.Intent = ports.IntentResume | 573 | attemptRequest.Intent = ports.IntentResume | ||
M internal/usecase/client/client_test.go +68 -3
| @@ -41,6 +41,66 @@ func (r *realTimer) C() <-chan time.Time { return r.t.C } | |||
| 41 | 41 | func (r *realTimer) Reset(d time.Duration) bool { return r.t.Reset(d) } | |
| 42 | 42 | func (r *realTimer) Stop() bool { return r.t.Stop() } | |
| 43 | 43 | ||
| 44 | + | type reconnectTestClock struct { | |
| 45 | + | mu sync.Mutex | |
| 46 | + | timers []*reconnectTestTimer | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | func (c *reconnectTestClock) Now() time.Time { return time.Time{} } | |
| 50 | + | func (c *reconnectTestClock) NewTimer(d time.Duration) ports.Timer { | |
| 51 | + | t := &reconnectTestTimer{ch: make(chan time.Time, 1), duration: d} | |
| 52 | + | c.mu.Lock() | |
| 53 | + | c.timers = append(c.timers, t) | |
| 54 | + | c.mu.Unlock() | |
| 55 | + | return t | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func (c *reconnectTestClock) fireReconnect(t *testing.T) { | |
| 59 | + | t.Helper() | |
| 60 | + | require.Eventually(t, func() bool { | |
| 61 | + | c.mu.Lock() | |
| 62 | + | defer c.mu.Unlock() | |
| 63 | + | for _, timer := range c.timers { | |
| 64 | + | if timer.duration != 100*time.Millisecond { | |
| 65 | + | continue | |
| 66 | + | } | |
| 67 | + | timer.mu.Lock() | |
| 68 | + | if !timer.stopped && !timer.fired { | |
| 69 | + | timer.fired = true | |
| 70 | + | timer.ch <- time.Time{} | |
| 71 | + | timer.mu.Unlock() | |
| 72 | + | return true | |
| 73 | + | } | |
| 74 | + | timer.mu.Unlock() | |
| 75 | + | } | |
| 76 | + | return false | |
| 77 | + | }, time.Second, time.Millisecond) | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | type reconnectTestTimer struct { | |
| 81 | + | mu sync.Mutex | |
| 82 | + | ch chan time.Time | |
| 83 | + | duration time.Duration | |
| 84 | + | stopped bool | |
| 85 | + | fired bool | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | func (t *reconnectTestTimer) C() <-chan time.Time { return t.ch } | |
| 89 | + | func (t *reconnectTestTimer) Reset(d time.Duration) bool { | |
| 90 | + | t.mu.Lock() | |
| 91 | + | t.duration = d | |
| 92 | + | t.fired = false | |
| 93 | + | t.mu.Unlock() | |
| 94 | + | return true | |
| 95 | + | } | |
| 96 | + | func (t *reconnectTestTimer) Stop() bool { | |
| 97 | + | t.mu.Lock() | |
| 98 | + | defer t.mu.Unlock() | |
| 99 | + | wasStopped := t.stopped | |
| 100 | + | t.stopped = true | |
| 101 | + | return !wasStopped | |
| 102 | + | } | |
| 103 | + | ||
| 44 | 104 | // recvItem is one scripted Recv result. | |
| 45 | 105 | type recvItem struct { | |
| 46 | 106 | f ports.Frame | |
| @@ -882,9 +942,14 @@ func TestRunReconnectsWithRotatedTokenAndSameClientID(t *testing.T) { | |||
| 882 | 942 | tr2 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(22)}, {err: io.EOF}}} | |
| 883 | 943 | tr3 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(33)}, {f: frameOf(ports.MsgDetached, ports.MarshalDetached(ports.Detached{Reason: ports.ReasonDetach}))}}} | |
| 884 | 944 | d := &sequenceDialer{trs: []ports.Transport{tr1, tr2, tr3}} | |
| 885 | - | ||
| 886 | - | err := runTestClient(context.Background(), testDependencies(d, term, realClock{}, nil, nil), client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", Remote: false}) | |
| 887 | - | require.NoError(t, err) | |
| 945 | + | clock := &reconnectTestClock{} | |
| 946 | + | result := make(chan error, 1) | |
| 947 | + | go func() { | |
| 948 | + | result <- runTestClient(context.Background(), testDependencies(d, term, clock, nil, nil), client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", Remote: false}) | |
| 949 | + | }() | |
| 950 | + | clock.fireReconnect(t) | |
| 951 | + | clock.fireReconnect(t) | |
| 952 | + | require.NoError(t, <-result) | |
| 888 | 953 | require.Equal(t, int32(3), d.calls.Load()) | |
| 889 | 954 | require.Equal(t, int32(1), term.rawCount.Load()) | |
| 890 | 955 | require.Equal(t, int32(1), term.restoreCount.Load()) | |
| @@ -41,6 +41,66 @@ func (r *realTimer) C() <-chan time.Time { return r.t.C } | |||||
| 41 | func (r *realTimer) Reset(d time.Duration) bool { return r.t.Reset(d) } | 41 | func (r *realTimer) Reset(d time.Duration) bool { return r.t.Reset(d) } | ||
| 42 | func (r *realTimer) Stop() bool { return r.t.Stop() } | 42 | func (r *realTimer) Stop() bool { return r.t.Stop() } | ||
| 43 | 43 | ||||
| 44 | + | type reconnectTestClock struct { | |||
| 45 | + | mu sync.Mutex | |||
| 46 | + | timers []*reconnectTestTimer | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | func (c *reconnectTestClock) Now() time.Time { return time.Time{} } | |||
| 50 | + | func (c *reconnectTestClock) NewTimer(d time.Duration) ports.Timer { | |||
| 51 | + | t := &reconnectTestTimer{ch: make(chan time.Time, 1), duration: d} | |||
| 52 | + | c.mu.Lock() | |||
| 53 | + | c.timers = append(c.timers, t) | |||
| 54 | + | c.mu.Unlock() | |||
| 55 | + | return t | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | func (c *reconnectTestClock) fireReconnect(t *testing.T) { | |||
| 59 | + | t.Helper() | |||
| 60 | + | require.Eventually(t, func() bool { | |||
| 61 | + | c.mu.Lock() | |||
| 62 | + | defer c.mu.Unlock() | |||
| 63 | + | for _, timer := range c.timers { | |||
| 64 | + | if timer.duration != 100*time.Millisecond { | |||
| 65 | + | continue | |||
| 66 | + | } | |||
| 67 | + | timer.mu.Lock() | |||
| 68 | + | if !timer.stopped && !timer.fired { | |||
| 69 | + | timer.fired = true | |||
| 70 | + | timer.ch <- time.Time{} | |||
| 71 | + | timer.mu.Unlock() | |||
| 72 | + | return true | |||
| 73 | + | } | |||
| 74 | + | timer.mu.Unlock() | |||
| 75 | + | } | |||
| 76 | + | return false | |||
| 77 | + | }, time.Second, time.Millisecond) | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | type reconnectTestTimer struct { | |||
| 81 | + | mu sync.Mutex | |||
| 82 | + | ch chan time.Time | |||
| 83 | + | duration time.Duration | |||
| 84 | + | stopped bool | |||
| 85 | + | fired bool | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | func (t *reconnectTestTimer) C() <-chan time.Time { return t.ch } | |||
| 89 | + | func (t *reconnectTestTimer) Reset(d time.Duration) bool { | |||
| 90 | + | t.mu.Lock() | |||
| 91 | + | t.duration = d | |||
| 92 | + | t.fired = false | |||
| 93 | + | t.mu.Unlock() | |||
| 94 | + | return true | |||
| 95 | + | } | |||
| 96 | + | func (t *reconnectTestTimer) Stop() bool { | |||
| 97 | + | t.mu.Lock() | |||
| 98 | + | defer t.mu.Unlock() | |||
| 99 | + | wasStopped := t.stopped | |||
| 100 | + | t.stopped = true | |||
| 101 | + | return !wasStopped | |||
| 102 | + | } | |||
| 103 | + | ||||
| 44 | // recvItem is one scripted Recv result. | 104 | // recvItem is one scripted Recv result. | ||
| 45 | type recvItem struct { | 105 | type recvItem struct { | ||
| 46 | f ports.Frame | 106 | f ports.Frame | ||
| @@ -882,9 +942,14 @@ func TestRunReconnectsWithRotatedTokenAndSameClientID(t *testing.T) { | |||||
| 882 | tr2 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(22)}, {err: io.EOF}}} | 942 | tr2 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(22)}, {err: io.EOF}}} | ||
| 883 | tr3 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(33)}, {f: frameOf(ports.MsgDetached, ports.MarshalDetached(ports.Detached{Reason: ports.ReasonDetach}))}}} | 943 | tr3 := &recordingTransport{recvs: []recvItem{{f: welcomeFrame(33)}, {f: frameOf(ports.MsgDetached, ports.MarshalDetached(ports.Detached{Reason: ports.ReasonDetach}))}}} | ||
| 884 | d := &sequenceDialer{trs: []ports.Transport{tr1, tr2, tr3}} | 944 | d := &sequenceDialer{trs: []ports.Transport{tr1, tr2, tr3}} | ||
| 885 | - | 945 | + | clock := &reconnectTestClock{} | |
| 886 | - | err := runTestClient(context.Background(), testDependencies(d, term, realClock{}, nil, nil), client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", Remote: false}) | 946 | + | result := make(chan error, 1) |
| 887 | - | require.NoError(t, err) | 947 | + | go func() { |
| 948 | + | result <- runTestClient(context.Background(), testDependencies(d, term, clock, nil, nil), client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", Remote: false}) | |||
| 949 | + | }() | |||
| 950 | + | clock.fireReconnect(t) | |||
| 951 | + | clock.fireReconnect(t) | |||
| 952 | + | require.NoError(t, <-result) | |||
| 888 | require.Equal(t, int32(3), d.calls.Load()) | 953 | require.Equal(t, int32(3), d.calls.Load()) | ||
| 889 | require.Equal(t, int32(1), term.rawCount.Load()) | 954 | require.Equal(t, int32(1), term.rawCount.Load()) | ||
| 890 | require.Equal(t, int32(1), term.restoreCount.Load()) | 955 | require.Equal(t, int32(1), term.restoreCount.Load()) | ||
M internal/usecase/client/remote_attach_validation_test.go +14 -18
| @@ -2,35 +2,31 @@ package client | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "fmt" | |
| 6 | 5 | "testing" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/stretchr/testify/require" | |
| 9 | 8 | ||
| 10 | 9 | "github.com/bnema/vev/internal/domain" | |
| 11 | 10 | "github.com/bnema/vev/internal/ports" | |
| 11 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | type validationCountingDialer struct{ calls int } | |
| 15 | - | ||
| 16 | - | func (d *validationCountingDialer) Dial(context.Context) (ports.Transport, error) { | |
| 17 | - | d.calls++ | |
| 18 | - | return nil, nil | |
| 19 | - | } | |
| 20 | - | ||
| 21 | 14 | func TestMalformedAttachRequestsNeverDial(t *testing.T) { | |
| 22 | - | tests := []AttachRequest{ | |
| 23 | - | {Intent: ports.IntentAttach}, | |
| 24 | - | {Intent: ports.IntentAttach, SessionName: "bad name"}, | |
| 25 | - | {Intent: ports.IntentAttach, SessionName: "main", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, | |
| 26 | - | {Intent: ports.IntentEphemeral, NavigationCapabilities: ports.NavigationCapabilityHomePicker}, | |
| 15 | + | tests := []struct { | |
| 16 | + | name string | |
| 17 | + | request AttachRequest | |
| 18 | + | }{ | |
| 19 | + | {name: "missing session name", request: AttachRequest{Intent: ports.IntentAttach}}, | |
| 20 | + | {name: "unsafe session name", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "bad name"}}, | |
| 21 | + | {name: "daemon-owned without remote", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "main", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}}, | |
| 22 | + | {name: "ephemeral with navigation capability", request: AttachRequest{Intent: ports.IntentEphemeral, NavigationCapabilities: ports.NavigationCapabilityHomePicker}}, | |
| 27 | 23 | } | |
| 28 | - | for i, request := range tests { | |
| 29 | - | t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { | |
| 30 | - | dialer := &validationCountingDialer{} | |
| 31 | - | err := NewRunner(Dependencies{Dialer: dialer}).Run(context.Background(), request) | |
| 24 | + | for _, tt := range tests { | |
| 25 | + | t.Run(tt.name, func(t *testing.T) { | |
| 26 | + | // No EXPECT: any Dial call fails the test. | |
| 27 | + | dialer := portsmocks.NewMockDialer(t) | |
| 28 | + | err := NewRunner(Dependencies{Dialer: dialer}).Run(context.Background(), tt.request) | |
| 32 | 29 | require.Error(t, err) | |
| 33 | - | require.Zero(t, dialer.calls) | |
| 34 | 30 | }) | |
| 35 | 31 | } | |
| 36 | 32 | } | |
| @@ -2,35 +2,31 @@ package client | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "fmt" | |||
| 6 | "testing" | 5 | "testing" | ||
| 7 | 6 | ||||
| 8 | "github.com/stretchr/testify/require" | 7 | "github.com/stretchr/testify/require" | ||
| 9 | 8 | ||||
| 10 | "github.com/bnema/vev/internal/domain" | 9 | "github.com/bnema/vev/internal/domain" | ||
| 11 | "github.com/bnema/vev/internal/ports" | 10 | "github.com/bnema/vev/internal/ports" | ||
| 11 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | type validationCountingDialer struct{ calls int } | |||
| 15 | - | ||||
| 16 | - | func (d *validationCountingDialer) Dial(context.Context) (ports.Transport, error) { | |||
| 17 | - | d.calls++ | |||
| 18 | - | return nil, nil | |||
| 19 | - | } | |||
| 20 | - | ||||
| 21 | func TestMalformedAttachRequestsNeverDial(t *testing.T) { | 14 | func TestMalformedAttachRequestsNeverDial(t *testing.T) { | ||
| 22 | - | tests := []AttachRequest{ | 15 | + | tests := []struct { |
| 23 | - | {Intent: ports.IntentAttach}, | 16 | + | name string |
| 24 | - | {Intent: ports.IntentAttach, SessionName: "bad name"}, | 17 | + | request AttachRequest |
| 25 | - | {Intent: ports.IntentAttach, SessionName: "main", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, | 18 | + | }{ |
| 26 | - | {Intent: ports.IntentEphemeral, NavigationCapabilities: ports.NavigationCapabilityHomePicker}, | 19 | + | {name: "missing session name", request: AttachRequest{Intent: ports.IntentAttach}}, |
| 20 | + | {name: "unsafe session name", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "bad name"}}, | |||
| 21 | + | {name: "daemon-owned without remote", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "main", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}}, | |||
| 22 | + | {name: "ephemeral with navigation capability", request: AttachRequest{Intent: ports.IntentEphemeral, NavigationCapabilities: ports.NavigationCapabilityHomePicker}}, | |||
| 27 | } | 23 | } | ||
| 28 | - | for i, request := range tests { | 24 | + | for _, tt := range tests { |
| 29 | - | t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { | 25 | + | t.Run(tt.name, func(t *testing.T) { |
| 30 | - | dialer := &validationCountingDialer{} | 26 | + | // No EXPECT: any Dial call fails the test. |
| 31 | - | err := NewRunner(Dependencies{Dialer: dialer}).Run(context.Background(), request) | 27 | + | dialer := portsmocks.NewMockDialer(t) |
| 28 | + | err := NewRunner(Dependencies{Dialer: dialer}).Run(context.Background(), tt.request) | |||
| 32 | require.Error(t, err) | 29 | require.Error(t, err) | ||
| 33 | - | require.Zero(t, dialer.calls) | |||
| 34 | }) | 30 | }) | ||
| 35 | } | 31 | } | ||
| 36 | } | 32 | } | ||
M internal/usecase/client/session_connection.go +3 -2
| @@ -20,9 +20,10 @@ type SessionTarget struct { | |||
| 20 | 20 | func (t SessionTarget) validate() error { | |
| 21 | 21 | switch t.Intent { | |
| 22 | 22 | case ports.IntentEphemeral: | |
| 23 | - | if t.SessionName == "" { | |
| 24 | - | return nil | |
| 23 | + | if t.SessionName != "" { | |
| 24 | + | return domain.ErrInvalidSessionName | |
| 25 | 25 | } | |
| 26 | + | return nil | |
| 26 | 27 | case ports.IntentNew, ports.IntentAttach, ports.IntentResume: | |
| 27 | 28 | if t.SessionName == "" { | |
| 28 | 29 | return domain.ErrInvalidSessionName | |
| @@ -20,9 +20,10 @@ type SessionTarget struct { | |||||
| 20 | func (t SessionTarget) validate() error { | 20 | func (t SessionTarget) validate() error { | ||
| 21 | switch t.Intent { | 21 | switch t.Intent { | ||
| 22 | case ports.IntentEphemeral: | 22 | case ports.IntentEphemeral: | ||
| 23 | - | if t.SessionName == "" { | 23 | + | if t.SessionName != "" { |
| 24 | - | return nil | 24 | + | return domain.ErrInvalidSessionName |
| 25 | } | 25 | } | ||
| 26 | + | return nil | |||
| 26 | case ports.IntentNew, ports.IntentAttach, ports.IntentResume: | 27 | case ports.IntentNew, ports.IntentAttach, ports.IntentResume: | ||
| 27 | if t.SessionName == "" { | 28 | if t.SessionName == "" { | ||
| 28 | return domain.ErrInvalidSessionName | 29 | return domain.ErrInvalidSessionName | ||
M internal/usecase/client/session_connection_test.go +1 -0
| @@ -32,6 +32,7 @@ func TestSessionTargetValidation(t *testing.T) { | |||
| 32 | 32 | wantErr bool | |
| 33 | 33 | }{ | |
| 34 | 34 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | |
| 35 | + | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true}, | |
| 35 | 36 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | |
| 36 | 37 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | |
| 37 | 38 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | |
| @@ -32,6 +32,7 @@ func TestSessionTargetValidation(t *testing.T) { | |||||
| 32 | wantErr bool | 32 | wantErr bool | ||
| 33 | }{ | 33 | }{ | ||
| 34 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | 34 | {name: "ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral}}, | ||
| 35 | + | {name: "named ephemeral", target: client.SessionTarget{Intent: ports.IntentEphemeral, SessionName: "work"}, wantErr: true}, | |||
| 35 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | 36 | {name: "new", target: client.SessionTarget{Intent: ports.IntentNew, SessionName: "work"}}, | ||
| 36 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | 37 | {name: "attach", target: client.SessionTarget{Intent: ports.IntentAttach, SessionName: "work"}}, | ||
| 37 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | 38 | {name: "resume", target: client.SessionTarget{Intent: ports.IntentResume, SessionName: "work"}}, | ||
M internal/usecase/daemon/daemon.go +1 -0
| @@ -1170,6 +1170,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||
| 1170 | 1170 | if !d.abortResumeClaim(ac) { | |
| 1171 | 1171 | d.clientGone(sess, ac, tr, false) | |
| 1172 | 1172 | } | |
| 1173 | + | _ = tr.Close() | |
| 1173 | 1174 | return | |
| 1174 | 1175 | } | |
| 1175 | 1176 | if err := handshakeCtx.Err(); err != nil { | |
| @@ -1170,6 +1170,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||||
| 1170 | if !d.abortResumeClaim(ac) { | 1170 | if !d.abortResumeClaim(ac) { | ||
| 1171 | d.clientGone(sess, ac, tr, false) | 1171 | d.clientGone(sess, ac, tr, false) | ||
| 1172 | } | 1172 | } | ||
| 1173 | + | _ = tr.Close() | |||
| 1173 | return | 1174 | return | ||
| 1174 | } | 1175 | } | ||
| 1175 | if err := handshakeCtx.Err(); err != nil { | 1176 | if err := handshakeCtx.Err(); err != nil { | ||
M internal/usecase/daemon/palette.go +6 -2
| @@ -289,7 +289,9 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 289 | 289 | return | |
| 290 | 290 | } | |
| 291 | 291 | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 292 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | |
| 292 | + | if current := ac.currentAttachmentSession(); current != nil { | |
| 293 | + | d.invalidateRender(current, ac, true, "palette.go") | |
| 294 | + | } | |
| 293 | 295 | } | |
| 294 | 296 | return | |
| 295 | 297 | } | |
| @@ -312,7 +314,9 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 312 | 314 | } | |
| 313 | 315 | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 314 | 316 | d.recordPaletteUse(cmd.Code) | |
| 315 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | |
| 317 | + | if current := ac.currentAttachmentSession(); current != nil { | |
| 318 | + | d.invalidateRender(current, ac, true, "palette.go") | |
| 319 | + | } | |
| 316 | 320 | } | |
| 317 | 321 | return | |
| 318 | 322 | } | |
| @@ -289,7 +289,9 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 289 | return | 289 | return | ||
| 290 | } | 290 | } | ||
| 291 | if ac.closeExecutedPalette(generation, rawQuery) { | 291 | if ac.closeExecutedPalette(generation, rawQuery) { | ||
| 292 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | 292 | + | if current := ac.currentAttachmentSession(); current != nil { |
| 293 | + | d.invalidateRender(current, ac, true, "palette.go") | |||
| 294 | + | } | |||
| 293 | } | 295 | } | ||
| 294 | return | 296 | return | ||
| 295 | } | 297 | } | ||
| @@ -312,7 +314,9 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 312 | } | 314 | } | ||
| 313 | if ac.closeExecutedPalette(generation, rawQuery) { | 315 | if ac.closeExecutedPalette(generation, rawQuery) { | ||
| 314 | d.recordPaletteUse(cmd.Code) | 316 | d.recordPaletteUse(cmd.Code) | ||
| 315 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | 317 | + | if current := ac.currentAttachmentSession(); current != nil { |
| 318 | + | d.invalidateRender(current, ac, true, "palette.go") | |||
| 319 | + | } | |||
| 316 | } | 320 | } | ||
| 317 | return | 321 | return | ||
| 318 | } | 322 | } | ||
M internal/usecase/daemon/picker.go +11 -8
| @@ -930,14 +930,17 @@ func (d *Daemon) remotePickerTargetReady(key domain.RemoteSessionKey) bool { | |||
| 930 | 930 | } | |
| 931 | 931 | d.remoteCatalog.mu.Lock() | |
| 932 | 932 | defer d.remoteCatalog.mu.Unlock() | |
| 933 | - | for _, entry := range d.remoteCatalog.cache { | |
| 934 | - | if entry.Host != key.Host { | |
| 935 | - | continue | |
| 936 | - | } | |
| 937 | - | for _, session := range entry.Sessions { | |
| 938 | - | if session.Name == key.Name { | |
| 939 | - | return d.remoteCatalog.status[key.Host] != remoteHostVersionMismatch | |
| 940 | - | } | |
| 933 | + | if d.remoteCatalog.status[key.Host] != remoteHostFresh { | |
| 934 | + | return false | |
| 935 | + | } | |
| 936 | + | entry, ok := d.remoteCatalog.cache[key.Host] | |
| 937 | + | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { | |
| 938 | + | d.remoteCatalog.status[key.Host] = remoteHostStale | |
| 939 | + | return false | |
| 940 | + | } | |
| 941 | + | for _, session := range entry.Sessions { | |
| 942 | + | if session.Name == key.Name { | |
| 943 | + | return true | |
| 941 | 944 | } | |
| 942 | 945 | } | |
| 943 | 946 | return false | |
| @@ -930,14 +930,17 @@ func (d *Daemon) remotePickerTargetReady(key domain.RemoteSessionKey) bool { | |||||
| 930 | } | 930 | } | ||
| 931 | d.remoteCatalog.mu.Lock() | 931 | d.remoteCatalog.mu.Lock() | ||
| 932 | defer d.remoteCatalog.mu.Unlock() | 932 | defer d.remoteCatalog.mu.Unlock() | ||
| 933 | - | for _, entry := range d.remoteCatalog.cache { | 933 | + | if d.remoteCatalog.status[key.Host] != remoteHostFresh { |
| 934 | - | if entry.Host != key.Host { | 934 | + | return false |
| 935 | - | continue | 935 | + | } |
| 936 | - | } | 936 | + | entry, ok := d.remoteCatalog.cache[key.Host] |
| 937 | - | for _, session := range entry.Sessions { | 937 | + | if !ok || remoteCatalogExpired(entry.FetchedAt, d.clock.Now()) { |
| 938 | - | if session.Name == key.Name { | 938 | + | d.remoteCatalog.status[key.Host] = remoteHostStale |
| 939 | - | return d.remoteCatalog.status[key.Host] != remoteHostVersionMismatch | 939 | + | return false |
| 940 | - | } | 940 | + | } |
| 941 | + | for _, session := range entry.Sessions { | |||
| 942 | + | if session.Name == key.Name { | |||
| 943 | + | return true | |||
| 941 | } | 944 | } | ||
| 942 | } | 945 | } | ||
| 943 | return false | 946 | return false | ||
M internal/usecase/daemon/remote_picker_test.go +6 -0
| @@ -604,6 +604,9 @@ func TestRemotePickerHandoffSendsTargetAndLeavesNoShadowSession(t *testing.T) { | |||
| 604 | 604 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 605 | 605 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | |
| 606 | 606 | }}) | |
| 607 | + | d.remoteCatalog.mu.Lock() | |
| 608 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 609 | + | d.remoteCatalog.mu.Unlock() | |
| 607 | 610 | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | |
| 608 | 611 | token := sess.attachmentToken(ac, ac.transport()) | |
| 609 | 612 | effect, admitted := ac.beginAttachmentEffect(token) | |
| @@ -656,6 +659,9 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 656 | 659 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 657 | 660 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | |
| 658 | 661 | }}) | |
| 662 | + | d.remoteCatalog.mu.Lock() | |
| 663 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 664 | + | d.remoteCatalog.mu.Unlock() | |
| 659 | 665 | const sendErr = "remote attach send failed" | |
| 660 | 666 | tr := &remotePickerSendErrorTransport{err: errors.New(sendErr)} | |
| 661 | 667 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| @@ -604,6 +604,9 @@ func TestRemotePickerHandoffSendsTargetAndLeavesNoShadowSession(t *testing.T) { | |||||
| 604 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | 604 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | ||
| 605 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | 605 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | ||
| 606 | }}) | 606 | }}) | ||
| 607 | + | d.remoteCatalog.mu.Lock() | |||
| 608 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |||
| 609 | + | d.remoteCatalog.mu.Unlock() | |||
| 607 | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | 610 | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | ||
| 608 | token := sess.attachmentToken(ac, ac.transport()) | 611 | token := sess.attachmentToken(ac, ac.transport()) | ||
| 609 | effect, admitted := ac.beginAttachmentEffect(token) | 612 | effect, admitted := ac.beginAttachmentEffect(token) | ||
| @@ -656,6 +659,9 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 656 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | 659 | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | ||
| 657 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | 660 | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{Name: "work", State: "up"}}, | ||
| 658 | }}) | 661 | }}) | ||
| 662 | + | d.remoteCatalog.mu.Lock() | |||
| 663 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |||
| 664 | + | d.remoteCatalog.mu.Unlock() | |||
| 659 | const sendErr = "remote attach send failed" | 665 | const sendErr = "remote attach send failed" | ||
| 660 | tr := &remotePickerSendErrorTransport{err: errors.New(sendErr)} | 666 | tr := &remotePickerSendErrorTransport{err: errors.New(sendErr)} | ||
| 661 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | 667 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | ||
M internal/usecase/daemon/render_pipeline.go +5 -1
| @@ -476,7 +476,11 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||
| 476 | 476 | endDiff(0, err == nil) | |
| 477 | 477 | if err != nil { | |
| 478 | 478 | ac.sendMu.Unlock() | |
| 479 | - | d.log.Error("render draw failed", "err", err, "session", entry.core().name) | |
| 479 | + | core := entry.core() | |
| 480 | + | core.mu.Lock() | |
| 481 | + | sessionName := core.name | |
| 482 | + | core.mu.Unlock() | |
| 483 | + | d.log.Error("render draw failed", "err", err, "session", sessionName) | |
| 480 | 484 | // Without a coordinator reportError repaints synchronously. Suppress only | |
| 481 | 485 | // that nested notice repaint; leave the guard before returning so a later, | |
| 482 | 486 | // independent failed transaction can still notify the user. | |
| @@ -476,7 +476,11 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||||
| 476 | endDiff(0, err == nil) | 476 | endDiff(0, err == nil) | ||
| 477 | if err != nil { | 477 | if err != nil { | ||
| 478 | ac.sendMu.Unlock() | 478 | ac.sendMu.Unlock() | ||
| 479 | - | d.log.Error("render draw failed", "err", err, "session", entry.core().name) | 479 | + | core := entry.core() |
| 480 | + | core.mu.Lock() | |||
| 481 | + | sessionName := core.name | |||
| 482 | + | core.mu.Unlock() | |||
| 483 | + | d.log.Error("render draw failed", "err", err, "session", sessionName) | |||
| 480 | // Without a coordinator reportError repaints synchronously. Suppress only | 484 | // Without a coordinator reportError repaints synchronously. Suppress only | ||
| 481 | // that nested notice repaint; leave the guard before returning so a later, | 485 | // that nested notice repaint; leave the guard before returning so a later, | ||
| 482 | // independent failed transaction can still notify the user. | 486 | // independent failed transaction can still notify the user. | ||