fix(remote): harden unified attachment review paths
- commit
- a068955d8c23859315db65c21030519bf1f2f88e
- author
- brice · · 2026-08-07 08:38 UTC
- parent
- 543bf31
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
31 files · +457 -172
M internal/ports/wire.go +3 -0
| @@ -1258,6 +1258,9 @@ func UnmarshalAck(b []byte) (Ack, error) { | |||
| 1258 | 1258 | ||
| 1259 | 1259 | // MarshalWelcome encodes m into a Welcome message payload. | |
| 1260 | 1260 | func MarshalWelcome(m Welcome) []byte { | |
| 1261 | + | if !validRenderMode(m.RenderMode) { | |
| 1262 | + | return nil | |
| 1263 | + | } | |
| 1261 | 1264 | w := payloadWriter{} | |
| 1262 | 1265 | w.putString(m.SessionID) | |
| 1263 | 1266 | w.putString(m.SessionName) | |
| @@ -1258,6 +1258,9 @@ func UnmarshalAck(b []byte) (Ack, error) { | |||||
| 1258 | 1258 | ||||
| 1259 | // MarshalWelcome encodes m into a Welcome message payload. | 1259 | // MarshalWelcome encodes m into a Welcome message payload. | ||
| 1260 | func MarshalWelcome(m Welcome) []byte { | 1260 | func MarshalWelcome(m Welcome) []byte { | ||
| 1261 | + | if !validRenderMode(m.RenderMode) { | |||
| 1262 | + | return nil | |||
| 1263 | + | } | |||
| 1261 | w := payloadWriter{} | 1264 | w := payloadWriter{} | ||
| 1262 | w.putString(m.SessionID) | 1265 | w.putString(m.SessionID) | ||
| 1263 | w.putString(m.SessionName) | 1266 | w.putString(m.SessionName) | ||
M internal/ports/wire_test.go +7 -0
| @@ -452,6 +452,13 @@ func TestWelcomeGoldenAndRoundTrip(t *testing.T) { | |||
| 452 | 452 | assertTrailingGarbageFails(t, full, UnmarshalWelcome) | |
| 453 | 453 | } | |
| 454 | 454 | ||
| 455 | + | func TestMarshalWelcomeRejectsInvalidRenderMode(t *testing.T) { | |
| 456 | + | got := MarshalWelcome(Welcome{SessionID: "sess-1", RenderMode: RenderMode(99)}) | |
| 457 | + | if got != nil { | |
| 458 | + | t.Fatalf("MarshalWelcome() = %#v, want nil for invalid render mode", got) | |
| 459 | + | } | |
| 460 | + | } | |
| 461 | + | ||
| 455 | 462 | func TestCommandErrorCodes(t *testing.T) { | |
| 456 | 463 | require.Equal(t, uint16(6), ErrUnknownCommand) | |
| 457 | 464 | require.Equal(t, uint16(7), ErrNotScriptable) | |
| @@ -452,6 +452,13 @@ func TestWelcomeGoldenAndRoundTrip(t *testing.T) { | |||||
| 452 | assertTrailingGarbageFails(t, full, UnmarshalWelcome) | 452 | assertTrailingGarbageFails(t, full, UnmarshalWelcome) | ||
| 453 | } | 453 | } | ||
| 454 | 454 | ||||
| 455 | + | func TestMarshalWelcomeRejectsInvalidRenderMode(t *testing.T) { | |||
| 456 | + | got := MarshalWelcome(Welcome{SessionID: "sess-1", RenderMode: RenderMode(99)}) | |||
| 457 | + | if got != nil { | |||
| 458 | + | t.Fatalf("MarshalWelcome() = %#v, want nil for invalid render mode", got) | |||
| 459 | + | } | |||
| 460 | + | } | |||
| 461 | + | ||||
| 455 | func TestCommandErrorCodes(t *testing.T) { | 462 | func TestCommandErrorCodes(t *testing.T) { | ||
| 456 | require.Equal(t, uint16(6), ErrUnknownCommand) | 463 | require.Equal(t, uint16(6), ErrUnknownCommand) | ||
| 457 | require.Equal(t, uint16(7), ErrNotScriptable) | 464 | require.Equal(t, uint16(7), ErrNotScriptable) | ||
M internal/usecase/client/client.go +7 -3
| @@ -40,6 +40,10 @@ func (e *ProtocolError) Error() string { | |||
| 40 | 40 | return fmt.Sprintf("vev: %s", e.Text) | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | + | // errRenderModeMismatch is a permanent attach failure: retrying the same | |
| 44 | + | // request cannot make a daemon accept a different render contract. | |
| 45 | + | var errRenderModeMismatch = errors.New("vev: daemon accepted render mode") | |
| 46 | + | ||
| 43 | 47 | // DetachedError reports that the daemon detached this client for a reason | |
| 44 | 48 | // other than an ordinary user-initiated detach. | |
| 45 | 49 | type DetachedError struct { | |
| @@ -231,7 +235,7 @@ func validateAttachRequest(request AttachRequest) error { | |||
| 231 | 235 | switch request.RenderMode { | |
| 232 | 236 | case ports.RenderModeFullTerminal, ports.RenderModeProxiedContent: | |
| 233 | 237 | default: | |
| 234 | - | return fmt.Errorf("invalid render mode %d", request.RenderMode) | |
| 238 | + | return fmt.Errorf("vev: invalid render mode %d", request.RenderMode) | |
| 235 | 239 | } | |
| 236 | 240 | switch request.Intent { | |
| 237 | 241 | case ports.IntentEphemeral: | |
| @@ -240,7 +244,7 @@ func validateAttachRequest(request AttachRequest) error { | |||
| 240 | 244 | return domain.ErrInvalidSessionName | |
| 241 | 245 | } | |
| 242 | 246 | default: | |
| 243 | - | return fmt.Errorf("invalid session intent %d", request.Intent) | |
| 247 | + | return fmt.Errorf("vev: invalid session intent %d", request.Intent) | |
| 244 | 248 | } | |
| 245 | 249 | if request.SessionName != "" { | |
| 246 | 250 | if err := domain.ValidateSessionName(request.SessionName); err != nil { | |
| @@ -801,7 +805,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||
| 801 | 805 | return result(false, fmt.Errorf("vev: decoding welcome: %w", derr)) | |
| 802 | 806 | } | |
| 803 | 807 | if welcome.RenderMode != request.RenderMode { | |
| 804 | - | return result(false, fmt.Errorf("vev: daemon accepted render mode %d, want %d", welcome.RenderMode, request.RenderMode)) | |
| 808 | + | return result(false, fmt.Errorf("%w %d, want %d", errRenderModeMismatch, welcome.RenderMode, request.RenderMode)) | |
| 805 | 809 | } | |
| 806 | 810 | resumeToken = welcome.ResumeToken | |
| 807 | 811 | name = welcome.SessionName | |
| @@ -40,6 +40,10 @@ func (e *ProtocolError) Error() string { | |||||
| 40 | return fmt.Sprintf("vev: %s", e.Text) | 40 | return fmt.Sprintf("vev: %s", e.Text) | ||
| 41 | } | 41 | } | ||
| 42 | 42 | ||||
| 43 | + | // errRenderModeMismatch is a permanent attach failure: retrying the same | |||
| 44 | + | // request cannot make a daemon accept a different render contract. | |||
| 45 | + | var errRenderModeMismatch = errors.New("vev: daemon accepted render mode") | |||
| 46 | + | ||||
| 43 | // DetachedError reports that the daemon detached this client for a reason | 47 | // DetachedError reports that the daemon detached this client for a reason | ||
| 44 | // other than an ordinary user-initiated detach. | 48 | // other than an ordinary user-initiated detach. | ||
| 45 | type DetachedError struct { | 49 | type DetachedError struct { | ||
| @@ -231,7 +235,7 @@ func validateAttachRequest(request AttachRequest) error { | |||||
| 231 | switch request.RenderMode { | 235 | switch request.RenderMode { | ||
| 232 | case ports.RenderModeFullTerminal, ports.RenderModeProxiedContent: | 236 | case ports.RenderModeFullTerminal, ports.RenderModeProxiedContent: | ||
| 233 | default: | 237 | default: | ||
| 234 | - | return fmt.Errorf("invalid render mode %d", request.RenderMode) | 238 | + | return fmt.Errorf("vev: invalid render mode %d", request.RenderMode) |
| 235 | } | 239 | } | ||
| 236 | switch request.Intent { | 240 | switch request.Intent { | ||
| 237 | case ports.IntentEphemeral: | 241 | case ports.IntentEphemeral: | ||
| @@ -240,7 +244,7 @@ func validateAttachRequest(request AttachRequest) error { | |||||
| 240 | return domain.ErrInvalidSessionName | 244 | return domain.ErrInvalidSessionName | ||
| 241 | } | 245 | } | ||
| 242 | default: | 246 | default: | ||
| 243 | - | return fmt.Errorf("invalid session intent %d", request.Intent) | 247 | + | return fmt.Errorf("vev: invalid session intent %d", request.Intent) |
| 244 | } | 248 | } | ||
| 245 | if request.SessionName != "" { | 249 | if request.SessionName != "" { | ||
| 246 | if err := domain.ValidateSessionName(request.SessionName); err != nil { | 250 | if err := domain.ValidateSessionName(request.SessionName); err != nil { | ||
| @@ -801,7 +805,7 @@ func (a *attachAttempt) run(ctx context.Context) attachResult { | |||||
| 801 | return result(false, fmt.Errorf("vev: decoding welcome: %w", derr)) | 805 | return result(false, fmt.Errorf("vev: decoding welcome: %w", derr)) | ||
| 802 | } | 806 | } | ||
| 803 | if welcome.RenderMode != request.RenderMode { | 807 | if welcome.RenderMode != request.RenderMode { | ||
| 804 | - | return result(false, fmt.Errorf("vev: daemon accepted render mode %d, want %d", welcome.RenderMode, request.RenderMode)) | 808 | + | return result(false, fmt.Errorf("%w %d, want %d", errRenderModeMismatch, welcome.RenderMode, request.RenderMode)) |
| 805 | } | 809 | } | ||
| 806 | resumeToken = welcome.ResumeToken | 810 | resumeToken = welcome.ResumeToken | ||
| 807 | name = welcome.SessionName | 811 | name = welcome.SessionName | ||
M internal/usecase/client/client_test.go +66 -2
| @@ -41,6 +41,44 @@ 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 | + | const runnerInitialBackoff = 100 * time.Millisecond | |
| 45 | + | ||
| 46 | + | type runnerTestClock struct { | |
| 47 | + | handshake chan *runnerTestTimer | |
| 48 | + | palette chan *runnerTestTimer | |
| 49 | + | reconnect chan *runnerTestTimer | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | type runnerTestTimer struct { | |
| 53 | + | ch chan time.Time | |
| 54 | + | duration time.Duration | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | func newRunnerTestClock() *runnerTestClock { | |
| 58 | + | return &runnerTestClock{ | |
| 59 | + | handshake: make(chan *runnerTestTimer, 4), | |
| 60 | + | palette: make(chan *runnerTestTimer, 8), | |
| 61 | + | reconnect: make(chan *runnerTestTimer, 16), | |
| 62 | + | } | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func (*runnerTestClock) Now() time.Time { return time.Time{} } | |
| 66 | + | func (c *runnerTestClock) NewTimer(duration time.Duration) ports.Timer { | |
| 67 | + | timer := &runnerTestTimer{ch: make(chan time.Time, 1), duration: duration} | |
| 68 | + | if duration == ports.HandshakeTimeout { | |
| 69 | + | c.handshake <- timer | |
| 70 | + | } else if duration == 200*time.Millisecond { | |
| 71 | + | c.palette <- timer | |
| 72 | + | } else { | |
| 73 | + | c.reconnect <- timer | |
| 74 | + | } | |
| 75 | + | return timer | |
| 76 | + | } | |
| 77 | + | func (t *runnerTestTimer) C() <-chan time.Time { return t.ch } | |
| 78 | + | func (*runnerTestTimer) Reset(time.Duration) bool { return false } | |
| 79 | + | func (*runnerTestTimer) Stop() bool { return true } | |
| 80 | + | func (t *runnerTestTimer) fire() { t.ch <- time.Time{} } | |
| 81 | + | ||
| 44 | 82 | // recvItem is one scripted Recv result. | |
| 45 | 83 | type recvItem struct { | |
| 46 | 84 | f ports.Frame | |
| @@ -901,8 +939,24 @@ func TestRunReconnectsPreservesRichRemoteTarget(t *testing.T) { | |||
| 901 | 939 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 902 | 940 | } | |
| 903 | 941 | ||
| 904 | - | err := runTestClient(context.Background(), testDependencies(d, term, realClock{}, nil, nil), request) | |
| 905 | - | require.NoError(t, err) | |
| 942 | + | clock := newRunnerTestClock() | |
| 943 | + | done := make(chan error, 1) | |
| 944 | + | go func() { | |
| 945 | + | done <- runTestClient(context.Background(), testDependencies(d, term, clock, nil, nil), request) | |
| 946 | + | }() | |
| 947 | + | handshakeTimer := <-clock.handshake | |
| 948 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |
| 949 | + | firstReconnect := <-clock.reconnect | |
| 950 | + | require.Equal(t, runnerInitialBackoff, firstReconnect.duration) | |
| 951 | + | firstReconnect.fire() | |
| 952 | + | handshakeTimer = <-clock.handshake | |
| 953 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |
| 954 | + | secondReconnect := <-clock.reconnect | |
| 955 | + | require.Equal(t, runnerInitialBackoff, secondReconnect.duration) | |
| 956 | + | secondReconnect.fire() | |
| 957 | + | handshakeTimer = <-clock.handshake | |
| 958 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |
| 959 | + | require.NoError(t, <-done) | |
| 906 | 960 | require.Equal(t, int32(3), d.calls.Load()) | |
| 907 | 961 | require.Equal(t, int32(1), term.rawCount.Load()) | |
| 908 | 962 | require.Equal(t, int32(1), term.restoreCount.Load()) | |
| @@ -982,6 +1036,16 @@ func TestRunnerRejectsInvalidAttachRequestBeforeHello(t *testing.T) { | |||
| 982 | 1036 | request client.AttachRequest | |
| 983 | 1037 | wantErr string | |
| 984 | 1038 | }{ | |
| 1039 | + | { | |
| 1040 | + | name: "invalid render mode", | |
| 1041 | + | request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", RenderMode: ports.RenderMode(99)}, | |
| 1042 | + | wantErr: "vev: invalid render mode", | |
| 1043 | + | }, | |
| 1044 | + | { | |
| 1045 | + | name: "invalid session intent", | |
| 1046 | + | request: client.AttachRequest{Intent: 99, SessionName: "main"}, | |
| 1047 | + | wantErr: "vev: invalid session intent", | |
| 1048 | + | }, | |
| 985 | 1049 | { | |
| 986 | 1050 | name: "missing daemon-owned policy", | |
| 987 | 1051 | request: client.AttachRequest{ | |
| @@ -41,6 +41,44 @@ 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 | + | const runnerInitialBackoff = 100 * time.Millisecond | |||
| 45 | + | ||||
| 46 | + | type runnerTestClock struct { | |||
| 47 | + | handshake chan *runnerTestTimer | |||
| 48 | + | palette chan *runnerTestTimer | |||
| 49 | + | reconnect chan *runnerTestTimer | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | type runnerTestTimer struct { | |||
| 53 | + | ch chan time.Time | |||
| 54 | + | duration time.Duration | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | func newRunnerTestClock() *runnerTestClock { | |||
| 58 | + | return &runnerTestClock{ | |||
| 59 | + | handshake: make(chan *runnerTestTimer, 4), | |||
| 60 | + | palette: make(chan *runnerTestTimer, 8), | |||
| 61 | + | reconnect: make(chan *runnerTestTimer, 16), | |||
| 62 | + | } | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | func (*runnerTestClock) Now() time.Time { return time.Time{} } | |||
| 66 | + | func (c *runnerTestClock) NewTimer(duration time.Duration) ports.Timer { | |||
| 67 | + | timer := &runnerTestTimer{ch: make(chan time.Time, 1), duration: duration} | |||
| 68 | + | if duration == ports.HandshakeTimeout { | |||
| 69 | + | c.handshake <- timer | |||
| 70 | + | } else if duration == 200*time.Millisecond { | |||
| 71 | + | c.palette <- timer | |||
| 72 | + | } else { | |||
| 73 | + | c.reconnect <- timer | |||
| 74 | + | } | |||
| 75 | + | return timer | |||
| 76 | + | } | |||
| 77 | + | func (t *runnerTestTimer) C() <-chan time.Time { return t.ch } | |||
| 78 | + | func (*runnerTestTimer) Reset(time.Duration) bool { return false } | |||
| 79 | + | func (*runnerTestTimer) Stop() bool { return true } | |||
| 80 | + | func (t *runnerTestTimer) fire() { t.ch <- time.Time{} } | |||
| 81 | + | ||||
| 44 | // recvItem is one scripted Recv result. | 82 | // recvItem is one scripted Recv result. | ||
| 45 | type recvItem struct { | 83 | type recvItem struct { | ||
| 46 | f ports.Frame | 84 | f ports.Frame | ||
| @@ -901,8 +939,24 @@ func TestRunReconnectsPreservesRichRemoteTarget(t *testing.T) { | |||||
| 901 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | 939 | EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | ||
| 902 | } | 940 | } | ||
| 903 | 941 | ||||
| 904 | - | err := runTestClient(context.Background(), testDependencies(d, term, realClock{}, nil, nil), request) | 942 | + | clock := newRunnerTestClock() |
| 905 | - | require.NoError(t, err) | 943 | + | done := make(chan error, 1) |
| 944 | + | go func() { | |||
| 945 | + | done <- runTestClient(context.Background(), testDependencies(d, term, clock, nil, nil), request) | |||
| 946 | + | }() | |||
| 947 | + | handshakeTimer := <-clock.handshake | |||
| 948 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |||
| 949 | + | firstReconnect := <-clock.reconnect | |||
| 950 | + | require.Equal(t, runnerInitialBackoff, firstReconnect.duration) | |||
| 951 | + | firstReconnect.fire() | |||
| 952 | + | handshakeTimer = <-clock.handshake | |||
| 953 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |||
| 954 | + | secondReconnect := <-clock.reconnect | |||
| 955 | + | require.Equal(t, runnerInitialBackoff, secondReconnect.duration) | |||
| 956 | + | secondReconnect.fire() | |||
| 957 | + | handshakeTimer = <-clock.handshake | |||
| 958 | + | require.Equal(t, testPreWelcomeTimeout, handshakeTimer.duration) | |||
| 959 | + | require.NoError(t, <-done) | |||
| 906 | require.Equal(t, int32(3), d.calls.Load()) | 960 | require.Equal(t, int32(3), d.calls.Load()) | ||
| 907 | require.Equal(t, int32(1), term.rawCount.Load()) | 961 | require.Equal(t, int32(1), term.rawCount.Load()) | ||
| 908 | require.Equal(t, int32(1), term.restoreCount.Load()) | 962 | require.Equal(t, int32(1), term.restoreCount.Load()) | ||
| @@ -982,6 +1036,16 @@ func TestRunnerRejectsInvalidAttachRequestBeforeHello(t *testing.T) { | |||||
| 982 | request client.AttachRequest | 1036 | request client.AttachRequest | ||
| 983 | wantErr string | 1037 | wantErr string | ||
| 984 | }{ | 1038 | }{ | ||
| 1039 | + | { | |||
| 1040 | + | name: "invalid render mode", | |||
| 1041 | + | request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "main", RenderMode: ports.RenderMode(99)}, | |||
| 1042 | + | wantErr: "vev: invalid render mode", | |||
| 1043 | + | }, | |||
| 1044 | + | { | |||
| 1045 | + | name: "invalid session intent", | |||
| 1046 | + | request: client.AttachRequest{Intent: 99, SessionName: "main"}, | |||
| 1047 | + | wantErr: "vev: invalid session intent", | |||
| 1048 | + | }, | |||
| 985 | { | 1049 | { | ||
| 986 | name: "missing daemon-owned policy", | 1050 | name: "missing daemon-owned policy", | ||
| 987 | request: client.AttachRequest{ | 1051 | request: client.AttachRequest{ | ||
M internal/usecase/client/palette_attach_generation_test.go +1 -1
| @@ -35,7 +35,7 @@ type attachPaletteTimer struct { | |||
| 35 | 35 | func newAttachPaletteClock() *attachPaletteClock { | |
| 36 | 36 | return &attachPaletteClock{ | |
| 37 | 37 | timers: make(chan *attachPaletteTimer, 16), | |
| 38 | - | handshakeTimers: make(chan *attachPaletteTimer, 1), | |
| 38 | + | handshakeTimers: make(chan *attachPaletteTimer, 4), | |
| 39 | 39 | } | |
| 40 | 40 | } | |
| 41 | 41 | ||
| @@ -35,7 +35,7 @@ type attachPaletteTimer struct { | |||||
| 35 | func newAttachPaletteClock() *attachPaletteClock { | 35 | func newAttachPaletteClock() *attachPaletteClock { | ||
| 36 | return &attachPaletteClock{ | 36 | return &attachPaletteClock{ | ||
| 37 | timers: make(chan *attachPaletteTimer, 16), | 37 | timers: make(chan *attachPaletteTimer, 16), | ||
| 38 | - | handshakeTimers: make(chan *attachPaletteTimer, 1), | 38 | + | handshakeTimers: make(chan *attachPaletteTimer, 4), |
| 39 | } | 39 | } | ||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
M internal/usecase/client/reconnect.go +3 -0
| @@ -56,6 +56,9 @@ func shouldReconnect(err error) bool { | |||
| 56 | 56 | if _, ok := errors.AsType[*ProtocolError](err); ok { | |
| 57 | 57 | return false | |
| 58 | 58 | } | |
| 59 | + | if errors.Is(err, errRenderModeMismatch) { | |
| 60 | + | return false | |
| 61 | + | } | |
| 59 | 62 | _, ok := errors.AsType[*DetachedError](err) | |
| 60 | 63 | return !ok | |
| 61 | 64 | } | |
| @@ -56,6 +56,9 @@ func shouldReconnect(err error) bool { | |||||
| 56 | if _, ok := errors.AsType[*ProtocolError](err); ok { | 56 | if _, ok := errors.AsType[*ProtocolError](err); ok { | ||
| 57 | return false | 57 | return false | ||
| 58 | } | 58 | } | ||
| 59 | + | if errors.Is(err, errRenderModeMismatch) { | |||
| 60 | + | return false | |||
| 61 | + | } | |||
| 59 | _, ok := errors.AsType[*DetachedError](err) | 62 | _, ok := errors.AsType[*DetachedError](err) | ||
| 60 | return !ok | 63 | return !ok | ||
| 61 | } | 64 | } | ||
M internal/usecase/daemon/attachment_handshake.go +1 -1
| @@ -10,7 +10,7 @@ import ( | |||
| 10 | 10 | // owner is deliberately generic so a resumed remote view receives the same | |
| 11 | 11 | // local-client Welcome/output lifecycle without being manufactured as a local | |
| 12 | 12 | // session. | |
| 13 | - | func (d *Daemon) completeAttachmentHandshake(handshakeCtx context.Context, timedOut <-chan struct{}, stopHandshakeTransport, finishHandshake func(), tr ports.Transport, owner attachmentOwner, ac *attachedClient) { | |
| 13 | + | func (d *Daemon) completeAttachmentHandshake(handshakeCtx context.Context, stopHandshakeTransport, finishHandshake func(), tr ports.Transport, owner attachmentOwner, ac *attachedClient) { | |
| 14 | 14 | failAttachment := func() { | |
| 15 | 15 | d.failHandshakeAttachmentOwner(owner, ac, tr) | |
| 16 | 16 | } | |
| @@ -10,7 +10,7 @@ import ( | |||||
| 10 | // owner is deliberately generic so a resumed remote view receives the same | 10 | // owner is deliberately generic so a resumed remote view receives the same | ||
| 11 | // local-client Welcome/output lifecycle without being manufactured as a local | 11 | // local-client Welcome/output lifecycle without being manufactured as a local | ||
| 12 | // session. | 12 | // session. | ||
| 13 | - | func (d *Daemon) completeAttachmentHandshake(handshakeCtx context.Context, timedOut <-chan struct{}, stopHandshakeTransport, finishHandshake func(), tr ports.Transport, owner attachmentOwner, ac *attachedClient) { | 13 | + | func (d *Daemon) completeAttachmentHandshake(handshakeCtx context.Context, stopHandshakeTransport, finishHandshake func(), tr ports.Transport, owner attachmentOwner, ac *attachedClient) { |
| 14 | failAttachment := func() { | 14 | failAttachment := func() { | ||
| 15 | d.failHandshakeAttachmentOwner(owner, ac, tr) | 15 | d.failHandshakeAttachmentOwner(owner, ac, tr) | ||
| 16 | } | 16 | } | ||
M internal/usecase/daemon/attachment_owner.go +4 -3
| @@ -162,10 +162,11 @@ func (d *Daemon) retireShutdownRemoteAttachment(view *remoteView, ac *attachedCl | |||
| 162 | 162 | if d == nil || view == nil || ac == nil { | |
| 163 | 163 | return | |
| 164 | 164 | } | |
| 165 | - | if sameAttachmentOwner(ac.currentAttachmentOwner(), view) { | |
| 166 | - | ac.connectionGeneration.Add(1) | |
| 167 | - | ac.setAttachmentOwner(nil) | |
| 165 | + | if !sameAttachmentOwner(ac.currentAttachmentOwner(), view) { | |
| 166 | + | return | |
| 168 | 167 | } | |
| 168 | + | ac.connectionGeneration.Add(1) | |
| 169 | + | ac.setAttachmentOwner(nil) | |
| 169 | 170 | ac.clearPreviousSession() | |
| 170 | 171 | transport := ac.transport() | |
| 171 | 172 | d.boundedSend(ac, frameDetached(reason)) | |
| @@ -162,10 +162,11 @@ func (d *Daemon) retireShutdownRemoteAttachment(view *remoteView, ac *attachedCl | |||||
| 162 | if d == nil || view == nil || ac == nil { | 162 | if d == nil || view == nil || ac == nil { | ||
| 163 | return | 163 | return | ||
| 164 | } | 164 | } | ||
| 165 | - | if sameAttachmentOwner(ac.currentAttachmentOwner(), view) { | 165 | + | if !sameAttachmentOwner(ac.currentAttachmentOwner(), view) { |
| 166 | - | ac.connectionGeneration.Add(1) | 166 | + | return |
| 167 | - | ac.setAttachmentOwner(nil) | |||
| 168 | } | 167 | } | ||
| 168 | + | ac.connectionGeneration.Add(1) | |||
| 169 | + | ac.setAttachmentOwner(nil) | |||
| 169 | ac.clearPreviousSession() | 170 | ac.clearPreviousSession() | ||
| 170 | transport := ac.transport() | 171 | transport := ac.transport() | ||
| 171 | d.boundedSend(ac, frameDetached(reason)) | 172 | d.boundedSend(ac, frameDetached(reason)) | ||
M internal/usecase/daemon/attachment_owner_test.go +2 -5
| @@ -306,11 +306,8 @@ func TestRemoteClientResumeWaitsForInFlightRemoteParking(t *testing.T) { | |||
| 306 | 306 | view.mu.Lock() | |
| 307 | 307 | require.True(t, view.registerAttachmentLocked(ac)) | |
| 308 | 308 | view.mu.Unlock() | |
| 309 | - | resumeToken := ac.resumeToken | |
| 310 | - | if resumeToken == 0 { | |
| 311 | - | resumeToken = d.markParkingInFlightOwner(view, ac) | |
| 312 | - | d.clearParkingInFlight(resumeToken, ac) | |
| 313 | - | } | |
| 309 | + | resumeToken := d.markParkingInFlightOwner(view, ac) | |
| 310 | + | d.clearParkingInFlight(resumeToken, ac) | |
| 314 | 311 | ||
| 315 | 312 | detachStarted := make(chan struct{}) | |
| 316 | 313 | releaseDetach := make(chan struct{}) | |
| @@ -306,11 +306,8 @@ func TestRemoteClientResumeWaitsForInFlightRemoteParking(t *testing.T) { | |||||
| 306 | view.mu.Lock() | 306 | view.mu.Lock() | ||
| 307 | require.True(t, view.registerAttachmentLocked(ac)) | 307 | require.True(t, view.registerAttachmentLocked(ac)) | ||
| 308 | view.mu.Unlock() | 308 | view.mu.Unlock() | ||
| 309 | - | resumeToken := ac.resumeToken | 309 | + | resumeToken := d.markParkingInFlightOwner(view, ac) |
| 310 | - | if resumeToken == 0 { | 310 | + | d.clearParkingInFlight(resumeToken, ac) |
| 311 | - | resumeToken = d.markParkingInFlightOwner(view, ac) | |||
| 312 | - | d.clearParkingInFlight(resumeToken, ac) | |||
| 313 | - | } | |||
| 314 | 311 | ||||
| 315 | detachStarted := make(chan struct{}) | 312 | detachStarted := make(chan struct{}) | ||
| 316 | releaseDetach := make(chan struct{}) | 313 | releaseDetach := make(chan struct{}) | ||
M internal/usecase/daemon/client.go +37 -33
| @@ -109,6 +109,13 @@ type attachedClient struct { | |||
| 109 | 109 | remoteRecent Guarded[[]remoteRecentSession] | |
| 110 | 110 | linkMu sync.Mutex | |
| 111 | 111 | sendMu sync.Mutex | |
| 112 | + | // remotePaintMu coalesces asynchronous remote-view repaints to one worker | |
| 113 | + | // per attachment. The worker always consumes the latest reset request. | |
| 114 | + | remotePaintMu sync.Mutex | |
| 115 | + | remotePaintView *remoteView | |
| 116 | + | remotePaintPending bool | |
| 117 | + | remotePaintRunning bool | |
| 118 | + | remotePaintReset bool | |
| 112 | 119 | // routeCreatedSession marks a session created by this attachment's route. | |
| 113 | 120 | // A handshake that never commits Welcome must tear down that exact empty | |
| 114 | 121 | // session, while an attachment routed to an existing session must not. | |
| @@ -245,6 +252,9 @@ func (ac *attachedClient) currentSession() *session { | |||
| 245 | 252 | // A remote view has no local tabs or PTYs, but the attachment retains its | |
| 246 | 253 | // immediate local predecessor so picker and palette shell state stay local. | |
| 247 | 254 | func (ac *attachedClient) navigationSession() *session { | |
| 255 | + | if ac == nil { | |
| 256 | + | return nil | |
| 257 | + | } | |
| 248 | 258 | if current := ac.currentAttachmentSession(); current != nil { | |
| 249 | 259 | return current | |
| 250 | 260 | } | |
| @@ -905,7 +915,7 @@ func (d *Daemon) applyThemeForAttachment(token attachmentConnectionToken, msg po | |||
| 905 | 915 | if !token.attachmentEffectCurrent() { | |
| 906 | 916 | return false | |
| 907 | 917 | } | |
| 908 | - | if rc := sess.core().coordinator.Load(); rc != nil { | |
| 918 | + | if rc := sess.renderCoordinator(); rc != nil { | |
| 909 | 919 | return rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | |
| 910 | 920 | } | |
| 911 | 921 | return false | |
| @@ -1036,7 +1046,7 @@ func (d *Daemon) handleClientNoticeForAttachment(token attachmentConnectionToken | |||
| 1036 | 1046 | repaint := d.mutateRemoteClientNotice(token.ac, notice) | |
| 1037 | 1047 | d.notices.routingMu.Unlock() | |
| 1038 | 1048 | if repaint && token.attachmentEffectCurrent() { | |
| 1039 | - | d.paintRemoteView(view, token.ac, false, token) | |
| 1049 | + | d.scheduleRemoteViewPaint(view, token.ac, false) | |
| 1040 | 1050 | } | |
| 1041 | 1051 | return | |
| 1042 | 1052 | } | |
| @@ -1067,18 +1077,15 @@ func (d *Daemon) handleClientNoticeDirect(sess *session, ac *attachedClient, not | |||
| 1067 | 1077 | } | |
| 1068 | 1078 | ||
| 1069 | 1079 | func (d *Daemon) mutateClientNotice(sess *session, ac *attachedClient, notice ports.ClientNotice) bool { | |
| 1070 | - | switch notice.Action { | |
| 1071 | - | case ports.ClientNoticeClipboardFallback: | |
| 1072 | - | return d.recordClientNotice(sess, ac, domain.NoticeError, domain.NoticeClipboard, "image paste failed; sent Ctrl+V") | |
| 1073 | - | case ports.ClientNoticeClipboardTooLarge: | |
| 1074 | - | return d.recordClientNotice(sess, ac, domain.NoticeWarn, domain.NoticeClipboardTooLarge, "image too large to paste") | |
| 1075 | - | case ports.ClientNoticeLinkDegraded: | |
| 1076 | - | return d.recordClientNotice(sess, ac, domain.NoticeWarn, domain.NoticeConnection, "connection degraded") | |
| 1077 | - | case ports.ClientNoticeLinkConnected: | |
| 1080 | + | if notice.Action == ports.ClientNoticeLinkConnected { | |
| 1078 | 1081 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, sess.id) | |
| 1079 | - | default: | |
| 1082 | + | } | |
| 1083 | + | n, ok := d.clientNoticeNotification(notice) | |
| 1084 | + | if !ok { | |
| 1080 | 1085 | return false | |
| 1081 | 1086 | } | |
| 1087 | + | n.SessionID = sess.id | |
| 1088 | + | return d.publishToast(ac, d.notices.record(n)) | |
| 1082 | 1089 | } | |
| 1083 | 1090 | ||
| 1084 | 1091 | // mutateRemoteClientNotice keeps client-originated feedback local to the | |
| @@ -1088,35 +1095,32 @@ func (d *Daemon) mutateRemoteClientNotice(ac *attachedClient, notice ports.Clien | |||
| 1088 | 1095 | if ac == nil { | |
| 1089 | 1096 | return false | |
| 1090 | 1097 | } | |
| 1091 | - | var n domain.Notification | |
| 1092 | - | switch notice.Action { | |
| 1093 | - | case ports.ClientNoticeClipboardFallback: | |
| 1094 | - | n = domain.Notification{Code: domain.NoticeClipboard, Severity: domain.NoticeError, Message: "image paste failed; sent Ctrl+V", Time: d.clock.Now()} | |
| 1095 | - | case ports.ClientNoticeClipboardTooLarge: | |
| 1096 | - | n = domain.Notification{Code: domain.NoticeClipboardTooLarge, Severity: domain.NoticeWarn, Message: "image too large to paste", Time: d.clock.Now()} | |
| 1097 | - | case ports.ClientNoticeLinkDegraded: | |
| 1098 | - | n = domain.Notification{Code: domain.NoticeConnection, Severity: domain.NoticeWarn, Message: "connection degraded", Time: d.clock.Now()} | |
| 1099 | - | case ports.ClientNoticeLinkConnected: | |
| 1098 | + | if notice.Action == ports.ClientNoticeLinkConnected { | |
| 1100 | 1099 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, "") | |
| 1101 | - | default: | |
| 1100 | + | } | |
| 1101 | + | n, ok := d.clientNoticeNotification(notice) | |
| 1102 | + | if !ok { | |
| 1102 | 1103 | return false | |
| 1103 | 1104 | } | |
| 1104 | 1105 | n.Count = 1 | |
| 1105 | 1106 | return d.publishToast(ac, n) | |
| 1106 | 1107 | } | |
| 1107 | 1108 | ||
| 1108 | - | // recordClientNotice mutates notice history and the selected attachment while | |
| 1109 | - | // routingMu keeps another connection from changing that selection. Rendering is left | |
| 1110 | - | // to the caller after it releases routingMu. | |
| 1111 | - | func (d *Daemon) recordClientNotice(sess *session, ac *attachedClient, sev domain.NoticeSeverity, code domain.NoticeCode, message string) bool { | |
| 1112 | - | n := d.notices.record(domain.Notification{ | |
| 1113 | - | Code: code, | |
| 1114 | - | Severity: sev, | |
| 1115 | - | Message: message, | |
| 1116 | - | Time: d.clock.Now(), | |
| 1117 | - | SessionID: sess.id, | |
| 1118 | - | }) | |
| 1119 | - | return d.publishToast(ac, n) | |
| 1109 | + | // clientNoticeNotification maps the closed client notice vocabulary to daemon | |
| 1110 | + | // notification values. Caller-specific session scope and LinkConnected handling | |
| 1111 | + | // remain at the two mutation sites; rendering is left to the caller after it | |
| 1112 | + | // releases routingMu. | |
| 1113 | + | func (d *Daemon) clientNoticeNotification(notice ports.ClientNotice) (domain.Notification, bool) { | |
| 1114 | + | switch notice.Action { | |
| 1115 | + | case ports.ClientNoticeClipboardFallback: | |
| 1116 | + | return domain.Notification{Code: domain.NoticeClipboard, Severity: domain.NoticeError, Message: "image paste failed; sent Ctrl+V", Time: d.clock.Now()}, true | |
| 1117 | + | case ports.ClientNoticeClipboardTooLarge: | |
| 1118 | + | return domain.Notification{Code: domain.NoticeClipboardTooLarge, Severity: domain.NoticeWarn, Message: "image too large to paste", Time: d.clock.Now()}, true | |
| 1119 | + | case ports.ClientNoticeLinkDegraded: | |
| 1120 | + | return domain.Notification{Code: domain.NoticeConnection, Severity: domain.NoticeWarn, Message: "connection degraded", Time: d.clock.Now()}, true | |
| 1121 | + | default: | |
| 1122 | + | return domain.Notification{}, false | |
| 1123 | + | } | |
| 1120 | 1124 | } | |
| 1121 | 1125 | ||
| 1122 | 1126 | // resizeForFirstPaint retains attach's synchronous geometry guarantee. The | |
| @@ -109,6 +109,13 @@ type attachedClient struct { | |||||
| 109 | remoteRecent Guarded[[]remoteRecentSession] | 109 | remoteRecent Guarded[[]remoteRecentSession] | ||
| 110 | linkMu sync.Mutex | 110 | linkMu sync.Mutex | ||
| 111 | sendMu sync.Mutex | 111 | sendMu sync.Mutex | ||
| 112 | + | // remotePaintMu coalesces asynchronous remote-view repaints to one worker | |||
| 113 | + | // per attachment. The worker always consumes the latest reset request. | |||
| 114 | + | remotePaintMu sync.Mutex | |||
| 115 | + | remotePaintView *remoteView | |||
| 116 | + | remotePaintPending bool | |||
| 117 | + | remotePaintRunning bool | |||
| 118 | + | remotePaintReset bool | |||
| 112 | // routeCreatedSession marks a session created by this attachment's route. | 119 | // routeCreatedSession marks a session created by this attachment's route. | ||
| 113 | // A handshake that never commits Welcome must tear down that exact empty | 120 | // A handshake that never commits Welcome must tear down that exact empty | ||
| 114 | // session, while an attachment routed to an existing session must not. | 121 | // session, while an attachment routed to an existing session must not. | ||
| @@ -245,6 +252,9 @@ func (ac *attachedClient) currentSession() *session { | |||||
| 245 | // A remote view has no local tabs or PTYs, but the attachment retains its | 252 | // A remote view has no local tabs or PTYs, but the attachment retains its | ||
| 246 | // immediate local predecessor so picker and palette shell state stay local. | 253 | // immediate local predecessor so picker and palette shell state stay local. | ||
| 247 | func (ac *attachedClient) navigationSession() *session { | 254 | func (ac *attachedClient) navigationSession() *session { | ||
| 255 | + | if ac == nil { | |||
| 256 | + | return nil | |||
| 257 | + | } | |||
| 248 | if current := ac.currentAttachmentSession(); current != nil { | 258 | if current := ac.currentAttachmentSession(); current != nil { | ||
| 249 | return current | 259 | return current | ||
| 250 | } | 260 | } | ||
| @@ -905,7 +915,7 @@ func (d *Daemon) applyThemeForAttachment(token attachmentConnectionToken, msg po | |||||
| 905 | if !token.attachmentEffectCurrent() { | 915 | if !token.attachmentEffectCurrent() { | ||
| 906 | return false | 916 | return false | ||
| 907 | } | 917 | } | ||
| 908 | - | if rc := sess.core().coordinator.Load(); rc != nil { | 918 | + | if rc := sess.renderCoordinator(); rc != nil { |
| 909 | return rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | 919 | return rc.invalidateForLease(token.ac, token.lease, renderInvalidation{class: invalidateUrgent, reset: true, producer: "client.go"}) | ||
| 910 | } | 920 | } | ||
| 911 | return false | 921 | return false | ||
| @@ -1036,7 +1046,7 @@ func (d *Daemon) handleClientNoticeForAttachment(token attachmentConnectionToken | |||||
| 1036 | repaint := d.mutateRemoteClientNotice(token.ac, notice) | 1046 | repaint := d.mutateRemoteClientNotice(token.ac, notice) | ||
| 1037 | d.notices.routingMu.Unlock() | 1047 | d.notices.routingMu.Unlock() | ||
| 1038 | if repaint && token.attachmentEffectCurrent() { | 1048 | if repaint && token.attachmentEffectCurrent() { | ||
| 1039 | - | d.paintRemoteView(view, token.ac, false, token) | 1049 | + | d.scheduleRemoteViewPaint(view, token.ac, false) |
| 1040 | } | 1050 | } | ||
| 1041 | return | 1051 | return | ||
| 1042 | } | 1052 | } | ||
| @@ -1067,18 +1077,15 @@ func (d *Daemon) handleClientNoticeDirect(sess *session, ac *attachedClient, not | |||||
| 1067 | } | 1077 | } | ||
| 1068 | 1078 | ||||
| 1069 | func (d *Daemon) mutateClientNotice(sess *session, ac *attachedClient, notice ports.ClientNotice) bool { | 1079 | func (d *Daemon) mutateClientNotice(sess *session, ac *attachedClient, notice ports.ClientNotice) bool { | ||
| 1070 | - | switch notice.Action { | 1080 | + | if notice.Action == ports.ClientNoticeLinkConnected { |
| 1071 | - | case ports.ClientNoticeClipboardFallback: | |||
| 1072 | - | return d.recordClientNotice(sess, ac, domain.NoticeError, domain.NoticeClipboard, "image paste failed; sent Ctrl+V") | |||
| 1073 | - | case ports.ClientNoticeClipboardTooLarge: | |||
| 1074 | - | return d.recordClientNotice(sess, ac, domain.NoticeWarn, domain.NoticeClipboardTooLarge, "image too large to paste") | |||
| 1075 | - | case ports.ClientNoticeLinkDegraded: | |||
| 1076 | - | return d.recordClientNotice(sess, ac, domain.NoticeWarn, domain.NoticeConnection, "connection degraded") | |||
| 1077 | - | case ports.ClientNoticeLinkConnected: | |||
| 1078 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, sess.id) | 1081 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, sess.id) | ||
| 1079 | - | default: | 1082 | + | } |
| 1083 | + | n, ok := d.clientNoticeNotification(notice) | |||
| 1084 | + | if !ok { | |||
| 1080 | return false | 1085 | return false | ||
| 1081 | } | 1086 | } | ||
| 1087 | + | n.SessionID = sess.id | |||
| 1088 | + | return d.publishToast(ac, d.notices.record(n)) | |||
| 1082 | } | 1089 | } | ||
| 1083 | 1090 | ||||
| 1084 | // mutateRemoteClientNotice keeps client-originated feedback local to the | 1091 | // mutateRemoteClientNotice keeps client-originated feedback local to the | ||
| @@ -1088,35 +1095,32 @@ func (d *Daemon) mutateRemoteClientNotice(ac *attachedClient, notice ports.Clien | |||||
| 1088 | if ac == nil { | 1095 | if ac == nil { | ||
| 1089 | return false | 1096 | return false | ||
| 1090 | } | 1097 | } | ||
| 1091 | - | var n domain.Notification | 1098 | + | if notice.Action == ports.ClientNoticeLinkConnected { |
| 1092 | - | switch notice.Action { | |||
| 1093 | - | case ports.ClientNoticeClipboardFallback: | |||
| 1094 | - | n = domain.Notification{Code: domain.NoticeClipboard, Severity: domain.NoticeError, Message: "image paste failed; sent Ctrl+V", Time: d.clock.Now()} | |||
| 1095 | - | case ports.ClientNoticeClipboardTooLarge: | |||
| 1096 | - | n = domain.Notification{Code: domain.NoticeClipboardTooLarge, Severity: domain.NoticeWarn, Message: "image too large to paste", Time: d.clock.Now()} | |||
| 1097 | - | case ports.ClientNoticeLinkDegraded: | |||
| 1098 | - | n = domain.Notification{Code: domain.NoticeConnection, Severity: domain.NoticeWarn, Message: "connection degraded", Time: d.clock.Now()} | |||
| 1099 | - | case ports.ClientNoticeLinkConnected: | |||
| 1100 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, "") | 1099 | return d.dismissToastWithoutRepaint(ac, domain.NoticeConnection, "") | ||
| 1101 | - | default: | 1100 | + | } |
| 1101 | + | n, ok := d.clientNoticeNotification(notice) | |||
| 1102 | + | if !ok { | |||
| 1102 | return false | 1103 | return false | ||
| 1103 | } | 1104 | } | ||
| 1104 | n.Count = 1 | 1105 | n.Count = 1 | ||
| 1105 | return d.publishToast(ac, n) | 1106 | return d.publishToast(ac, n) | ||
| 1106 | } | 1107 | } | ||
| 1107 | 1108 | ||||
| 1108 | - | // recordClientNotice mutates notice history and the selected attachment while | 1109 | + | // clientNoticeNotification maps the closed client notice vocabulary to daemon |
| 1109 | - | // routingMu keeps another connection from changing that selection. Rendering is left | 1110 | + | // notification values. Caller-specific session scope and LinkConnected handling |
| 1110 | - | // to the caller after it releases routingMu. | 1111 | + | // remain at the two mutation sites; rendering is left to the caller after it |
| 1111 | - | func (d *Daemon) recordClientNotice(sess *session, ac *attachedClient, sev domain.NoticeSeverity, code domain.NoticeCode, message string) bool { | 1112 | + | // releases routingMu. |
| 1112 | - | n := d.notices.record(domain.Notification{ | 1113 | + | func (d *Daemon) clientNoticeNotification(notice ports.ClientNotice) (domain.Notification, bool) { |
| 1113 | - | Code: code, | 1114 | + | switch notice.Action { |
| 1114 | - | Severity: sev, | 1115 | + | case ports.ClientNoticeClipboardFallback: |
| 1115 | - | Message: message, | 1116 | + | return domain.Notification{Code: domain.NoticeClipboard, Severity: domain.NoticeError, Message: "image paste failed; sent Ctrl+V", Time: d.clock.Now()}, true |
| 1116 | - | Time: d.clock.Now(), | 1117 | + | case ports.ClientNoticeClipboardTooLarge: |
| 1117 | - | SessionID: sess.id, | 1118 | + | return domain.Notification{Code: domain.NoticeClipboardTooLarge, Severity: domain.NoticeWarn, Message: "image too large to paste", Time: d.clock.Now()}, true |
| 1118 | - | }) | 1119 | + | case ports.ClientNoticeLinkDegraded: |
| 1119 | - | return d.publishToast(ac, n) | 1120 | + | return domain.Notification{Code: domain.NoticeConnection, Severity: domain.NoticeWarn, Message: "connection degraded", Time: d.clock.Now()}, true |
| 1121 | + | default: | |||
| 1122 | + | return domain.Notification{}, false | |||
| 1123 | + | } | |||
| 1120 | } | 1124 | } | ||
| 1121 | 1125 | ||||
| 1122 | // resizeForFirstPaint retains attach's synchronous geometry guarantee. The | 1126 | // resizeForFirstPaint retains attach's synchronous geometry guarantee. The | ||
M internal/usecase/daemon/client_frame_routing.go +3 -3
| @@ -244,7 +244,7 @@ func (d *Daemon) resetOutput(token attachmentConnectionToken) bool { | |||
| 244 | 244 | ac.pipelineScratch = composeCacheInput{} | |
| 245 | 245 | ac.sendMu.Unlock() | |
| 246 | 246 | if view, remote := token.owner.(*remoteView); remote { | |
| 247 | - | go d.paintRemoteView(view, ac, true, token) | |
| 247 | + | d.scheduleRemoteViewPaint(view, ac, true) | |
| 248 | 248 | } else if sess := token.localSession(); sess != nil { | |
| 249 | 249 | go d.paint(sess, ac, true, token.lease) | |
| 250 | 250 | } | |
| @@ -267,7 +267,7 @@ func (d *Daemon) ackOutput(token attachmentConnectionToken, epoch, state uint64) | |||
| 267 | 267 | return true | |
| 268 | 268 | } | |
| 269 | 269 | if sess := token.localSession(); sess != nil { | |
| 270 | - | if rc := sess.core().coordinator.Load(); rc != nil { | |
| 270 | + | if rc := sess.renderCoordinator(); rc != nil { | |
| 271 | 271 | rc.notifyAckForLease(token.lease) | |
| 272 | 272 | } | |
| 273 | 273 | } else if view, remote := token.owner.(*remoteView); remote { | |
| @@ -275,7 +275,7 @@ func (d *Daemon) ackOutput(token attachmentConnectionToken, epoch, state uint64) | |||
| 275 | 275 | // newly available output-window capacity. Recompose from the retained | |
| 276 | 276 | // private VT so an update previously blocked by the local client window | |
| 277 | 277 | // is not stranded until another remote frame arrives. | |
| 278 | - | go d.paintRemoteView(view, ac, false, token) | |
| 278 | + | d.scheduleRemoteViewPaint(view, ac, false) | |
| 279 | 279 | } | |
| 280 | 280 | return true | |
| 281 | 281 | } | |
| @@ -244,7 +244,7 @@ func (d *Daemon) resetOutput(token attachmentConnectionToken) bool { | |||||
| 244 | ac.pipelineScratch = composeCacheInput{} | 244 | ac.pipelineScratch = composeCacheInput{} | ||
| 245 | ac.sendMu.Unlock() | 245 | ac.sendMu.Unlock() | ||
| 246 | if view, remote := token.owner.(*remoteView); remote { | 246 | if view, remote := token.owner.(*remoteView); remote { | ||
| 247 | - | go d.paintRemoteView(view, ac, true, token) | 247 | + | d.scheduleRemoteViewPaint(view, ac, true) |
| 248 | } else if sess := token.localSession(); sess != nil { | 248 | } else if sess := token.localSession(); sess != nil { | ||
| 249 | go d.paint(sess, ac, true, token.lease) | 249 | go d.paint(sess, ac, true, token.lease) | ||
| 250 | } | 250 | } | ||
| @@ -267,7 +267,7 @@ func (d *Daemon) ackOutput(token attachmentConnectionToken, epoch, state uint64) | |||||
| 267 | return true | 267 | return true | ||
| 268 | } | 268 | } | ||
| 269 | if sess := token.localSession(); sess != nil { | 269 | if sess := token.localSession(); sess != nil { | ||
| 270 | - | if rc := sess.core().coordinator.Load(); rc != nil { | 270 | + | if rc := sess.renderCoordinator(); rc != nil { |
| 271 | rc.notifyAckForLease(token.lease) | 271 | rc.notifyAckForLease(token.lease) | ||
| 272 | } | 272 | } | ||
| 273 | } else if view, remote := token.owner.(*remoteView); remote { | 273 | } else if view, remote := token.owner.(*remoteView); remote { | ||
| @@ -275,7 +275,7 @@ func (d *Daemon) ackOutput(token attachmentConnectionToken, epoch, state uint64) | |||||
| 275 | // newly available output-window capacity. Recompose from the retained | 275 | // newly available output-window capacity. Recompose from the retained | ||
| 276 | // private VT so an update previously blocked by the local client window | 276 | // private VT so an update previously blocked by the local client window | ||
| 277 | // is not stranded until another remote frame arrives. | 277 | // is not stranded until another remote frame arrives. | ||
| 278 | - | go d.paintRemoteView(view, ac, false, token) | 278 | + | d.scheduleRemoteViewPaint(view, ac, false) |
| 279 | } | 279 | } | ||
| 280 | return true | 280 | return true | ||
| 281 | } | 281 | } | ||
M internal/usecase/daemon/daemon.go +1 -1
| @@ -1222,7 +1222,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||
| 1222 | 1222 | _ = tr.Close() | |
| 1223 | 1223 | return | |
| 1224 | 1224 | } | |
| 1225 | - | d.completeAttachmentHandshake(handshakeCtx, timedOut, stopHandshakeTransport, finishHandshake, tr, owner, ac) | |
| 1225 | + | d.completeAttachmentHandshake(handshakeCtx, stopHandshakeTransport, finishHandshake, tr, owner, ac) | |
| 1226 | 1226 | } | |
| 1227 | 1227 | ||
| 1228 | 1228 | // protoErr is a session-level rejection carrying a wire ErrorMsg code. | |
| @@ -1222,7 +1222,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||||
| 1222 | _ = tr.Close() | 1222 | _ = tr.Close() | ||
| 1223 | return | 1223 | return | ||
| 1224 | } | 1224 | } | ||
| 1225 | - | d.completeAttachmentHandshake(handshakeCtx, timedOut, stopHandshakeTransport, finishHandshake, tr, owner, ac) | 1225 | + | d.completeAttachmentHandshake(handshakeCtx, stopHandshakeTransport, finishHandshake, tr, owner, ac) |
| 1226 | } | 1226 | } | ||
| 1227 | 1227 | ||||
| 1228 | // protoErr is a session-level rejection carrying a wire ErrorMsg code. | 1228 | // protoErr is a session-level rejection carrying a wire ErrorMsg code. | ||
M internal/usecase/daemon/palette.go +6 -6
| @@ -290,11 +290,11 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 290 | 290 | } | |
| 291 | 291 | if err != nil { | |
| 292 | 292 | ac.paletteFailure(generation, rawQuery, "requested session is unavailable") | |
| 293 | - | d.invalidateRender(entry, ac, true, "palette.go") | |
| 293 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 294 | 294 | return | |
| 295 | 295 | } | |
| 296 | 296 | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 297 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | |
| 297 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 298 | 298 | } | |
| 299 | 299 | return | |
| 300 | 300 | } | |
| @@ -303,7 +303,7 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 303 | 303 | rank, err := command.ParsePositiveDecimal(args) | |
| 304 | 304 | if err != nil { | |
| 305 | 305 | ac.paletteFailure(generation, rawQuery, "rank must be one positive decimal") | |
| 306 | - | d.invalidateRender(entry, ac, true, "palette.go") | |
| 306 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 307 | 307 | return | |
| 308 | 308 | } | |
| 309 | 309 | exec := paletteExec{d: d, sess: sess, attachment: entry, ac: ac, recent: recent, effect: effect} | |
| @@ -312,12 +312,12 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 312 | 312 | return | |
| 313 | 313 | } | |
| 314 | 314 | ac.paletteFailure(generation, rawQuery, "requested recent session is unavailable") | |
| 315 | - | d.invalidateRender(entry, ac, true, "palette.go") | |
| 315 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 316 | 316 | return | |
| 317 | 317 | } | |
| 318 | 318 | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 319 | 319 | d.recordPaletteUse(cmd.Code) | |
| 320 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | |
| 320 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 321 | 321 | } | |
| 322 | 322 | return | |
| 323 | 323 | } | |
| @@ -333,7 +333,7 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||
| 333 | 333 | currentToken := current.attachmentToken(ac, ac.transport()) | |
| 334 | 334 | fresh, admitted := ac.beginAttachmentEffect(currentToken) | |
| 335 | 335 | if ac.closeExecutedPalette(generation, rawQuery) { | |
| 336 | - | d.invalidateRender(current, ac, true, "palette.go") | |
| 336 | + | d.invalidateAttachedOwner(ac, true, "palette.go") | |
| 337 | 337 | } | |
| 338 | 338 | if admitted { | |
| 339 | 339 | fresh.End() | |
| @@ -290,11 +290,11 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 290 | } | 290 | } | ||
| 291 | if err != nil { | 291 | if err != nil { | ||
| 292 | ac.paletteFailure(generation, rawQuery, "requested session is unavailable") | 292 | ac.paletteFailure(generation, rawQuery, "requested session is unavailable") | ||
| 293 | - | d.invalidateRender(entry, ac, true, "palette.go") | 293 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 294 | return | 294 | return | ||
| 295 | } | 295 | } | ||
| 296 | if ac.closeExecutedPalette(generation, rawQuery) { | 296 | if ac.closeExecutedPalette(generation, rawQuery) { | ||
| 297 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | 297 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 298 | } | 298 | } | ||
| 299 | return | 299 | return | ||
| 300 | } | 300 | } | ||
| @@ -303,7 +303,7 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 303 | rank, err := command.ParsePositiveDecimal(args) | 303 | rank, err := command.ParsePositiveDecimal(args) | ||
| 304 | if err != nil { | 304 | if err != nil { | ||
| 305 | ac.paletteFailure(generation, rawQuery, "rank must be one positive decimal") | 305 | ac.paletteFailure(generation, rawQuery, "rank must be one positive decimal") | ||
| 306 | - | d.invalidateRender(entry, ac, true, "palette.go") | 306 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 307 | return | 307 | return | ||
| 308 | } | 308 | } | ||
| 309 | exec := paletteExec{d: d, sess: sess, attachment: entry, ac: ac, recent: recent, effect: effect} | 309 | exec := paletteExec{d: d, sess: sess, attachment: entry, ac: ac, recent: recent, effect: effect} | ||
| @@ -312,12 +312,12 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 312 | return | 312 | return | ||
| 313 | } | 313 | } | ||
| 314 | ac.paletteFailure(generation, rawQuery, "requested recent session is unavailable") | 314 | ac.paletteFailure(generation, rawQuery, "requested recent session is unavailable") | ||
| 315 | - | d.invalidateRender(entry, ac, true, "palette.go") | 315 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 316 | return | 316 | return | ||
| 317 | } | 317 | } | ||
| 318 | if ac.closeExecutedPalette(generation, rawQuery) { | 318 | if ac.closeExecutedPalette(generation, rawQuery) { | ||
| 319 | d.recordPaletteUse(cmd.Code) | 319 | d.recordPaletteUse(cmd.Code) | ||
| 320 | - | d.invalidateRender(ac.currentAttachmentSession(), ac, true, "palette.go") | 320 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 321 | } | 321 | } | ||
| 322 | return | 322 | return | ||
| 323 | } | 323 | } | ||
| @@ -333,7 +333,7 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte, effects ... | |||||
| 333 | currentToken := current.attachmentToken(ac, ac.transport()) | 333 | currentToken := current.attachmentToken(ac, ac.transport()) | ||
| 334 | fresh, admitted := ac.beginAttachmentEffect(currentToken) | 334 | fresh, admitted := ac.beginAttachmentEffect(currentToken) | ||
| 335 | if ac.closeExecutedPalette(generation, rawQuery) { | 335 | if ac.closeExecutedPalette(generation, rawQuery) { | ||
| 336 | - | d.invalidateRender(current, ac, true, "palette.go") | 336 | + | d.invalidateAttachedOwner(ac, true, "palette.go") |
| 337 | } | 337 | } | ||
| 338 | if admitted { | 338 | if admitted { | ||
| 339 | fresh.End() | 339 | fresh.End() | ||
M internal/usecase/daemon/picker.go +5 -2
| @@ -269,6 +269,7 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||
| 269 | 269 | rt := ac.overlays | |
| 270 | 270 | var previewGeneration uint64 | |
| 271 | 271 | var instance remotePickerInstance | |
| 272 | + | var selection *remotePickerSelection | |
| 272 | 273 | var closed bool | |
| 273 | 274 | return listInputState{ | |
| 274 | 275 | pending: &rt.pickerPending, | |
| @@ -279,10 +280,9 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||
| 279 | 280 | unlock: rt.pickerMu.Unlock, | |
| 280 | 281 | active: func() bool { return rt.picker != nil }, | |
| 281 | 282 | closeLocked: func() { | |
| 282 | - | if selection := rt.pickerRemoteSelection; selection != nil { | |
| 283 | + | if selection = rt.pickerRemoteSelection; selection != nil { | |
| 283 | 284 | rt.pickerRemoteSelection = nil | |
| 284 | 285 | rt.pickerTitle = pickerTitle(pickerSortMode(d.pickerSort.Load())) | |
| 285 | - | selection.cancel() | |
| 286 | 286 | return | |
| 287 | 287 | } | |
| 288 | 288 | closed = true | |
| @@ -293,6 +293,9 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||
| 293 | 293 | previewGeneration = rt.pickerPreviewGeneration | |
| 294 | 294 | }, | |
| 295 | 295 | afterClose: func() { | |
| 296 | + | if selection != nil { | |
| 297 | + | selection.cancel() | |
| 298 | + | } | |
| 296 | 299 | if closed { | |
| 297 | 300 | d.clearPreviewGeneration(ac, previewGeneration) | |
| 298 | 301 | d.remotePickerClosed(instance) | |
| @@ -269,6 +269,7 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||||
| 269 | rt := ac.overlays | 269 | rt := ac.overlays | ||
| 270 | var previewGeneration uint64 | 270 | var previewGeneration uint64 | ||
| 271 | var instance remotePickerInstance | 271 | var instance remotePickerInstance | ||
| 272 | + | var selection *remotePickerSelection | |||
| 272 | var closed bool | 273 | var closed bool | ||
| 273 | return listInputState{ | 274 | return listInputState{ | ||
| 274 | pending: &rt.pickerPending, | 275 | pending: &rt.pickerPending, | ||
| @@ -279,10 +280,9 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||||
| 279 | unlock: rt.pickerMu.Unlock, | 280 | unlock: rt.pickerMu.Unlock, | ||
| 280 | active: func() bool { return rt.picker != nil }, | 281 | active: func() bool { return rt.picker != nil }, | ||
| 281 | closeLocked: func() { | 282 | closeLocked: func() { | ||
| 282 | - | if selection := rt.pickerRemoteSelection; selection != nil { | 283 | + | if selection = rt.pickerRemoteSelection; selection != nil { |
| 283 | rt.pickerRemoteSelection = nil | 284 | rt.pickerRemoteSelection = nil | ||
| 284 | rt.pickerTitle = pickerTitle(pickerSortMode(d.pickerSort.Load())) | 285 | rt.pickerTitle = pickerTitle(pickerSortMode(d.pickerSort.Load())) | ||
| 285 | - | selection.cancel() | |||
| 286 | return | 286 | return | ||
| 287 | } | 287 | } | ||
| 288 | closed = true | 288 | closed = true | ||
| @@ -293,6 +293,9 @@ func (d *Daemon) pickerListInputState(ac *attachedClient) listInputState { | |||||
| 293 | previewGeneration = rt.pickerPreviewGeneration | 293 | previewGeneration = rt.pickerPreviewGeneration | ||
| 294 | }, | 294 | }, | ||
| 295 | afterClose: func() { | 295 | afterClose: func() { | ||
| 296 | + | if selection != nil { | |||
| 297 | + | selection.cancel() | |||
| 298 | + | } | |||
| 296 | if closed { | 299 | if closed { | ||
| 297 | d.clearPreviewGeneration(ac, previewGeneration) | 300 | d.clearPreviewGeneration(ac, previewGeneration) | ||
| 298 | d.remotePickerClosed(instance) | 301 | d.remotePickerClosed(instance) | ||
M internal/usecase/daemon/proxied_render.go +3 -1
| @@ -71,7 +71,9 @@ func (d *Daemon) paintProxiedContent(entry *session, ac *attachedClient, reset b | |||
| 71 | 71 | } | |
| 72 | 72 | composed := composeProxiedContent(*state) | |
| 73 | 73 | ac.proxiedOutputStarted = true | |
| 74 | - | d.emitFrame(entry, ac, state, composed, &marks) | |
| 74 | + | if !d.emitFrame(entry, ac, state, composed, &marks) { | |
| 75 | + | return paintRejected | |
| 76 | + | } | |
| 75 | 77 | return paintEmitted | |
| 76 | 78 | } | |
| 77 | 79 | ||
| @@ -71,7 +71,9 @@ func (d *Daemon) paintProxiedContent(entry *session, ac *attachedClient, reset b | |||||
| 71 | } | 71 | } | ||
| 72 | composed := composeProxiedContent(*state) | 72 | composed := composeProxiedContent(*state) | ||
| 73 | ac.proxiedOutputStarted = true | 73 | ac.proxiedOutputStarted = true | ||
| 74 | - | d.emitFrame(entry, ac, state, composed, &marks) | 74 | + | if !d.emitFrame(entry, ac, state, composed, &marks) { |
| 75 | + | return paintRejected | |||
| 76 | + | } | |||
| 75 | return paintEmitted | 77 | return paintEmitted | ||
| 76 | } | 78 | } | ||
| 77 | 79 | ||||
M internal/usecase/daemon/remote_attach_test.go +77 -32
| @@ -18,38 +18,83 @@ func remoteLifecycleForTest() domain.SessionLifecycleID { | |||
| 18 | 18 | } | |
| 19 | 19 | ||
| 20 | 20 | func TestRemotePickerSelectionRejectsReplacedLifecycle(t *testing.T) { | |
| 21 | - | d := newRemotePickerDaemon(nil) | |
| 22 | - | lifecycle := remoteLifecycleForTest() | |
| 23 | - | cachedLifecycle := lifecycle | |
| 24 | - | cachedLifecycle[0]++ | |
| 25 | - | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 26 | - | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |
| 27 | - | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 28 | - | }}, | |
| 29 | - | }}) | |
| 30 | - | d.remoteCatalog.mu.Lock() | |
| 31 | - | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 32 | - | d.remoteCatalog.mu.Unlock() | |
| 33 | - | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | |
| 34 | - | ac.overlays.pickerMu.Lock() | |
| 35 | - | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |
| 36 | - | ac.overlays.pickerGeneration++ | |
| 37 | - | ac.overlays.pickerMu.Unlock() | |
| 38 | - | token := sess.attachmentToken(ac, ac.transport()) | |
| 39 | - | effect, admitted := ac.beginAttachmentEffect(token) | |
| 40 | - | require.True(t, admitted) | |
| 41 | - | token.effect = effect | |
| 42 | - | defer effect.End() | |
| 43 | - | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | |
| 44 | - | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 45 | - | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | |
| 21 | + | tests := []struct { | |
| 22 | + | name string | |
| 23 | + | run func(*testing.T) | |
| 24 | + | }{ | |
| 25 | + | { | |
| 26 | + | name: "replaced lifecycle", | |
| 27 | + | run: func(t *testing.T) { | |
| 28 | + | d := newRemotePickerDaemon(nil) | |
| 29 | + | lifecycle := remoteLifecycleForTest() | |
| 30 | + | cachedLifecycle := lifecycle | |
| 31 | + | cachedLifecycle[0]++ | |
| 32 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | |
| 33 | + | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | |
| 34 | + | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 35 | + | }}, | |
| 36 | + | }}) | |
| 37 | + | d.remoteCatalog.mu.Lock() | |
| 38 | + | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 39 | + | d.remoteCatalog.mu.Unlock() | |
| 40 | + | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | |
| 41 | + | ac.overlays.pickerMu.Lock() | |
| 42 | + | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |
| 43 | + | ac.overlays.pickerGeneration++ | |
| 44 | + | ac.overlays.pickerMu.Unlock() | |
| 45 | + | token := sess.attachmentToken(ac, ac.transport()) | |
| 46 | + | effect, admitted := ac.beginAttachmentEffect(token) | |
| 47 | + | require.True(t, admitted) | |
| 48 | + | token.effect = effect | |
| 49 | + | defer effect.End() | |
| 50 | + | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | |
| 51 | + | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |
| 52 | + | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | |
| 46 | 53 | ||
| 47 | - | require.ErrorIs(t, d.switchToTargetForAttachment(token, target, sessionHandoffGuard{closePicker: true}, "picker-select"), errAttachmentTransition) | |
| 48 | - | require.Same(t, sess, ac.currentAttachmentSession()) | |
| 49 | - | require.True(t, ac.overlays.pickerActive()) | |
| 50 | - | select { | |
| 51 | - | case frame := <-sends: | |
| 52 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | |
| 53 | - | default: | |
| 54 | + | require.ErrorIs(t, d.switchToTargetForAttachment(token, target, sessionHandoffGuard{closePicker: true}, "picker-select"), errAttachmentTransition) | |
| 55 | + | require.Same(t, sess, ac.currentAttachmentSession()) | |
| 56 | + | require.True(t, ac.overlays.pickerActive()) | |
| 57 | + | select { | |
| 58 | + | case frame := <-sends: | |
| 59 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | |
| 60 | + | default: | |
| 61 | + | } | |
| 62 | + | }, | |
| 63 | + | }, | |
| 64 | + | { | |
| 65 | + | name: "replaced picker instance", | |
| 66 | + | run: func(t *testing.T) { | |
| 67 | + | d, source, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 68 | + | view := &remoteView{key: remoteViewKey{endpoint: "remote.example", lifecycleID: domain.SessionLifecycleID{1}, sessionName: "remote"}} | |
| 69 | + | transport := newRemoteLinkTestTransport() | |
| 70 | + | link := &remoteLink{view: view, generation: 1, transport: transport, active: true, commands: NewCommandRequestTracker()} | |
| 71 | + | view.link = link | |
| 72 | + | view.linkGeneration = 1 | |
| 73 | + | d.mu.Lock() | |
| 74 | + | require.NoError(t, d.registerRemoteViewLocked(view)) | |
| 75 | + | d.mu.Unlock() | |
| 76 | + | model := picker.New(nil, picker.SelectionConfig{}) | |
| 77 | + | ac.overlays.pickerMu.Lock() | |
| 78 | + | ac.overlays.picker = model | |
| 79 | + | ac.overlays.pickerGeneration++ | |
| 80 | + | generation := ac.overlays.pickerGeneration | |
| 81 | + | ac.overlays.pickerMu.Unlock() | |
| 82 | + | selection := &remotePickerSelection{model: model, generation: generation, token: source.attachmentToken(ac, ac.transport())} | |
| 83 | + | ac.overlays.pickerMu.Lock() | |
| 84 | + | ac.overlays.pickerRemoteSelection = selection | |
| 85 | + | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |
| 86 | + | ac.overlays.pickerMu.Unlock() | |
| 87 | + | ||
| 88 | + | _, err := d.transitionToRemoteViewForPicker(selection.token, view, selection) | |
| 89 | + | require.ErrorIs(t, err, errAttachmentTransition) | |
| 90 | + | require.Same(t, source, ac.currentAttachmentSession()) | |
| 91 | + | require.True(t, source.attachmentRegistered(ac)) | |
| 92 | + | require.False(t, view.attachmentRegistered(ac)) | |
| 93 | + | require.NoError(t, transport.Close()) | |
| 94 | + | }, | |
| 95 | + | }, | |
| 96 | + | } | |
| 97 | + | for _, tt := range tests { | |
| 98 | + | t.Run(tt.name, tt.run) | |
| 54 | 99 | } | |
| 55 | 100 | } | |
| @@ -18,38 +18,83 @@ func remoteLifecycleForTest() domain.SessionLifecycleID { | |||||
| 18 | } | 18 | } | ||
| 19 | 19 | ||||
| 20 | func TestRemotePickerSelectionRejectsReplacedLifecycle(t *testing.T) { | 20 | func TestRemotePickerSelectionRejectsReplacedLifecycle(t *testing.T) { | ||
| 21 | - | d := newRemotePickerDaemon(nil) | 21 | + | tests := []struct { |
| 22 | - | lifecycle := remoteLifecycleForTest() | 22 | + | name string |
| 23 | - | cachedLifecycle := lifecycle | 23 | + | run func(*testing.T) |
| 24 | - | cachedLifecycle[0]++ | 24 | + | }{ |
| 25 | - | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ | 25 | + | { |
| 26 | - | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ | 26 | + | name: "replaced lifecycle", |
| 27 | - | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | 27 | + | run: func(t *testing.T) { |
| 28 | - | }}, | 28 | + | d := newRemotePickerDaemon(nil) |
| 29 | - | }}) | 29 | + | lifecycle := remoteLifecycleForTest() |
| 30 | - | d.remoteCatalog.mu.Lock() | 30 | + | cachedLifecycle := lifecycle |
| 31 | - | d.remoteCatalog.status["arch"] = remoteHostFresh | 31 | + | cachedLifecycle[0]++ |
| 32 | - | d.remoteCatalog.mu.Unlock() | 32 | + | d.remoteCatalog.replaceCache([]ports.RemoteCatalogCacheEntry{{ |
| 33 | - | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") | 33 | + | Host: "arch", FetchedAt: time.Unix(10, 0), Sessions: []ports.RemoteCatalogSession{{ |
| 34 | - | ac.overlays.pickerMu.Lock() | 34 | + | LifecycleID: cachedLifecycle, Name: "work", State: "running", Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, |
| 35 | - | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | 35 | + | }}, |
| 36 | - | ac.overlays.pickerGeneration++ | 36 | + | }}) |
| 37 | - | ac.overlays.pickerMu.Unlock() | 37 | + | d.remoteCatalog.mu.Lock() |
| 38 | - | token := sess.attachmentToken(ac, ac.transport()) | 38 | + | d.remoteCatalog.status["arch"] = remoteHostFresh |
| 39 | - | effect, admitted := ac.beginAttachmentEffect(token) | 39 | + | d.remoteCatalog.mu.Unlock() |
| 40 | - | require.True(t, admitted) | 40 | + | sess, ac, sends := addRemoteRefreshPickerOwner(t, d, "local") |
| 41 | - | token.effect = effect | 41 | + | ac.overlays.pickerMu.Lock() |
| 42 | - | defer effect.End() | 42 | + | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) |
| 43 | - | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | 43 | + | ac.overlays.pickerGeneration++ |
| 44 | - | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | 44 | + | ac.overlays.pickerMu.Unlock() |
| 45 | - | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | 45 | + | token := sess.attachmentToken(ac, ac.transport()) |
| 46 | + | effect, admitted := ac.beginAttachmentEffect(token) | |||
| 47 | + | require.True(t, admitted) | |||
| 48 | + | token.effect = effect | |||
| 49 | + | defer effect.End() | |||
| 50 | + | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle, DisplayOrigin: "arch"} | |||
| 51 | + | remoteTarget := domain.RemoteSessionTarget{Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, SessionName: "work", LiveTabID: "tab-1"} | |||
| 52 | + | target := picker.Target{Session: key.ID(), RemoteKey: &key, RemoteTarget: &remoteTarget, TabID: "tab-1"} | |||
| 46 | 53 | ||||
| 47 | - | require.ErrorIs(t, d.switchToTargetForAttachment(token, target, sessionHandoffGuard{closePicker: true}, "picker-select"), errAttachmentTransition) | 54 | + | require.ErrorIs(t, d.switchToTargetForAttachment(token, target, sessionHandoffGuard{closePicker: true}, "picker-select"), errAttachmentTransition) |
| 48 | - | require.Same(t, sess, ac.currentAttachmentSession()) | 55 | + | require.Same(t, sess, ac.currentAttachmentSession()) |
| 49 | - | require.True(t, ac.overlays.pickerActive()) | 56 | + | require.True(t, ac.overlays.pickerActive()) |
| 50 | - | select { | 57 | + | select { |
| 51 | - | case frame := <-sends: | 58 | + | case frame := <-sends: |
| 52 | - | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) | 59 | + | require.NotEqual(t, ports.MsgAttachTarget, frame.Type) |
| 53 | - | default: | 60 | + | default: |
| 61 | + | } | |||
| 62 | + | }, | |||
| 63 | + | }, | |||
| 64 | + | { | |||
| 65 | + | name: "replaced picker instance", | |||
| 66 | + | run: func(t *testing.T) { | |||
| 67 | + | d, source, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |||
| 68 | + | view := &remoteView{key: remoteViewKey{endpoint: "remote.example", lifecycleID: domain.SessionLifecycleID{1}, sessionName: "remote"}} | |||
| 69 | + | transport := newRemoteLinkTestTransport() | |||
| 70 | + | link := &remoteLink{view: view, generation: 1, transport: transport, active: true, commands: NewCommandRequestTracker()} | |||
| 71 | + | view.link = link | |||
| 72 | + | view.linkGeneration = 1 | |||
| 73 | + | d.mu.Lock() | |||
| 74 | + | require.NoError(t, d.registerRemoteViewLocked(view)) | |||
| 75 | + | d.mu.Unlock() | |||
| 76 | + | model := picker.New(nil, picker.SelectionConfig{}) | |||
| 77 | + | ac.overlays.pickerMu.Lock() | |||
| 78 | + | ac.overlays.picker = model | |||
| 79 | + | ac.overlays.pickerGeneration++ | |||
| 80 | + | generation := ac.overlays.pickerGeneration | |||
| 81 | + | ac.overlays.pickerMu.Unlock() | |||
| 82 | + | selection := &remotePickerSelection{model: model, generation: generation, token: source.attachmentToken(ac, ac.transport())} | |||
| 83 | + | ac.overlays.pickerMu.Lock() | |||
| 84 | + | ac.overlays.pickerRemoteSelection = selection | |||
| 85 | + | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |||
| 86 | + | ac.overlays.pickerMu.Unlock() | |||
| 87 | + | ||||
| 88 | + | _, err := d.transitionToRemoteViewForPicker(selection.token, view, selection) | |||
| 89 | + | require.ErrorIs(t, err, errAttachmentTransition) | |||
| 90 | + | require.Same(t, source, ac.currentAttachmentSession()) | |||
| 91 | + | require.True(t, source.attachmentRegistered(ac)) | |||
| 92 | + | require.False(t, view.attachmentRegistered(ac)) | |||
| 93 | + | require.NoError(t, transport.Close()) | |||
| 94 | + | }, | |||
| 95 | + | }, | |||
| 96 | + | } | |||
| 97 | + | for _, tt := range tests { | |||
| 98 | + | t.Run(tt.name, tt.run) | |||
| 54 | } | 99 | } | ||
| 55 | } | 100 | } | ||
M internal/usecase/daemon/remote_attachment_geometry_test.go +1 -1
| @@ -24,7 +24,7 @@ func attachRemoteGeometryClient(t *testing.T, view *remoteView, size domain.Size | |||
| 24 | 24 | func resizeRemoteGeometryAttachment(t *testing.T, d *Daemon, view *remoteView, ac *attachedClient, size domain.Size, remoteSends chan ports.Frame) domain.Size { | |
| 25 | 25 | t.Helper() | |
| 26 | 26 | token := attachmentOwnerToken(view, ac, ac.transport()) | |
| 27 | - | require.True(t, token.current()) | |
| 27 | + | require.True(t, token.attachmentCurrent()) | |
| 28 | 28 | require.True(t, d.resizeAttachmentForLease(token, size)) | |
| 29 | 29 | ||
| 30 | 30 | frame := awaitFrame(t, remoteSends, ports.MsgResize) | |
| @@ -24,7 +24,7 @@ func attachRemoteGeometryClient(t *testing.T, view *remoteView, size domain.Size | |||||
| 24 | func resizeRemoteGeometryAttachment(t *testing.T, d *Daemon, view *remoteView, ac *attachedClient, size domain.Size, remoteSends chan ports.Frame) domain.Size { | 24 | func resizeRemoteGeometryAttachment(t *testing.T, d *Daemon, view *remoteView, ac *attachedClient, size domain.Size, remoteSends chan ports.Frame) domain.Size { | ||
| 25 | t.Helper() | 25 | t.Helper() | ||
| 26 | token := attachmentOwnerToken(view, ac, ac.transport()) | 26 | token := attachmentOwnerToken(view, ac, ac.transport()) | ||
| 27 | - | require.True(t, token.current()) | 27 | + | require.True(t, token.attachmentCurrent()) |
| 28 | require.True(t, d.resizeAttachmentForLease(token, size)) | 28 | require.True(t, d.resizeAttachmentForLease(token, size)) | ||
| 29 | 29 | ||||
| 30 | frame := awaitFrame(t, remoteSends, ports.MsgResize) | 30 | frame := awaitFrame(t, remoteSends, ports.MsgResize) | ||
M internal/usecase/daemon/remote_attachment_transition.go +2 -2
| @@ -28,7 +28,7 @@ func (d *Daemon) transitionToRemoteViewForPicker(token attachmentConnectionToken | |||
| 28 | 28 | ||
| 29 | 29 | func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, target *remoteView, selection *remotePickerSelection, expectedLink *remoteLink, expectedLinkGeneration uint64) (attachmentConnectionToken, error) { | |
| 30 | 30 | source := token.localSession() | |
| 31 | - | if d == nil || source == nil || token.ac == nil || target == nil { | |
| 31 | + | if d == nil || source == nil || token.ac == nil || target == nil || token.transport.transport == nil { | |
| 32 | 32 | return attachmentConnectionToken{}, errAttachmentTransition | |
| 33 | 33 | } | |
| 34 | 34 | if token.effect != nil { | |
| @@ -86,6 +86,7 @@ func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, | |||
| 86 | 86 | rebase: true, | |
| 87 | 87 | } | |
| 88 | 88 | token.ac.publishFrozenAttachmentCapability(published) | |
| 89 | + | token.ac.recordPreviousOwner(source) | |
| 89 | 90 | target.mu.Unlock() | |
| 90 | 91 | source.mu.Unlock() | |
| 91 | 92 | d.notices.routingMu.Unlock() | |
| @@ -98,7 +99,6 @@ func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, | |||
| 98 | 99 | coordinator.noteDetach(token.ac) | |
| 99 | 100 | } | |
| 100 | 101 | d.recalculateSessionGeometryAndInvalidateAsync(source, "remote_attachment_transition.go") | |
| 101 | - | token.ac.recordPreviousOwner(source) | |
| 102 | 102 | d.recordRemoteRecent(token.ac, target) | |
| 103 | 103 | return published, nil | |
| 104 | 104 | } | |
| @@ -28,7 +28,7 @@ func (d *Daemon) transitionToRemoteViewForPicker(token attachmentConnectionToken | |||||
| 28 | 28 | ||||
| 29 | func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, target *remoteView, selection *remotePickerSelection, expectedLink *remoteLink, expectedLinkGeneration uint64) (attachmentConnectionToken, error) { | 29 | func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, target *remoteView, selection *remotePickerSelection, expectedLink *remoteLink, expectedLinkGeneration uint64) (attachmentConnectionToken, error) { | ||
| 30 | source := token.localSession() | 30 | source := token.localSession() | ||
| 31 | - | if d == nil || source == nil || token.ac == nil || target == nil { | 31 | + | if d == nil || source == nil || token.ac == nil || target == nil || token.transport.transport == nil { |
| 32 | return attachmentConnectionToken{}, errAttachmentTransition | 32 | return attachmentConnectionToken{}, errAttachmentTransition | ||
| 33 | } | 33 | } | ||
| 34 | if token.effect != nil { | 34 | if token.effect != nil { | ||
| @@ -86,6 +86,7 @@ func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, | |||||
| 86 | rebase: true, | 86 | rebase: true, | ||
| 87 | } | 87 | } | ||
| 88 | token.ac.publishFrozenAttachmentCapability(published) | 88 | token.ac.publishFrozenAttachmentCapability(published) | ||
| 89 | + | token.ac.recordPreviousOwner(source) | |||
| 89 | target.mu.Unlock() | 90 | target.mu.Unlock() | ||
| 90 | source.mu.Unlock() | 91 | source.mu.Unlock() | ||
| 91 | d.notices.routingMu.Unlock() | 92 | d.notices.routingMu.Unlock() | ||
| @@ -98,7 +99,6 @@ func (d *Daemon) transitionToRemoteViewGuarded(token attachmentConnectionToken, | |||||
| 98 | coordinator.noteDetach(token.ac) | 99 | coordinator.noteDetach(token.ac) | ||
| 99 | } | 100 | } | ||
| 100 | d.recalculateSessionGeometryAndInvalidateAsync(source, "remote_attachment_transition.go") | 101 | d.recalculateSessionGeometryAndInvalidateAsync(source, "remote_attachment_transition.go") | ||
| 101 | - | token.ac.recordPreviousOwner(source) | |||
| 102 | d.recordRemoteRecent(token.ac, target) | 102 | d.recordRemoteRecent(token.ac, target) | ||
| 103 | return published, nil | 103 | return published, nil | ||
| 104 | } | 104 | } | ||
M internal/usecase/daemon/remote_attachment_transition_back_test.go +3 -2
| @@ -50,6 +50,7 @@ func TestRemoteRecentAppearsInLocalStatusMRUAfterReturn(t *testing.T) { | |||
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | 52 | func TestBackSessionForAttachmentFailsClosedForStaleOrNonLocalPredecessor(t *testing.T) { | |
| 53 | + | sentinel := &remoteView{id: 99} | |
| 53 | 54 | tests := []struct { | |
| 54 | 55 | name string | |
| 55 | 56 | prep func(*Daemon, *session, *attachedClient, *remoteView) | |
| @@ -72,13 +73,13 @@ func TestBackSessionForAttachmentFailsClosedForStaleOrNonLocalPredecessor(t *tes | |||
| 72 | 73 | { | |
| 73 | 74 | name: "non-local predecessor", | |
| 74 | 75 | prep: func(_ *Daemon, _ *session, ac *attachedClient, _ *remoteView) { | |
| 75 | - | ac.previousOwner.Set(&remoteView{id: 99}) | |
| 76 | + | ac.previousOwner.Set(sentinel) | |
| 76 | 77 | }, | |
| 77 | 78 | check: func(t *testing.T, _ *session, ac *attachedClient, view *remoteView) { | |
| 78 | 79 | other := ac.previousOwner.Get() | |
| 79 | 80 | require.Same(t, view, ac.currentAttachmentOwner()) | |
| 80 | 81 | require.True(t, view.attachmentRegistered(ac)) | |
| 81 | - | require.IsType(t, &remoteView{}, other) | |
| 82 | + | require.Same(t, sentinel, other) | |
| 82 | 83 | }, | |
| 83 | 84 | }, | |
| 84 | 85 | } | |
| @@ -50,6 +50,7 @@ func TestRemoteRecentAppearsInLocalStatusMRUAfterReturn(t *testing.T) { | |||||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | func TestBackSessionForAttachmentFailsClosedForStaleOrNonLocalPredecessor(t *testing.T) { | 52 | func TestBackSessionForAttachmentFailsClosedForStaleOrNonLocalPredecessor(t *testing.T) { | ||
| 53 | + | sentinel := &remoteView{id: 99} | |||
| 53 | tests := []struct { | 54 | tests := []struct { | ||
| 54 | name string | 55 | name string | ||
| 55 | prep func(*Daemon, *session, *attachedClient, *remoteView) | 56 | prep func(*Daemon, *session, *attachedClient, *remoteView) | ||
| @@ -72,13 +73,13 @@ func TestBackSessionForAttachmentFailsClosedForStaleOrNonLocalPredecessor(t *tes | |||||
| 72 | { | 73 | { | ||
| 73 | name: "non-local predecessor", | 74 | name: "non-local predecessor", | ||
| 74 | prep: func(_ *Daemon, _ *session, ac *attachedClient, _ *remoteView) { | 75 | prep: func(_ *Daemon, _ *session, ac *attachedClient, _ *remoteView) { | ||
| 75 | - | ac.previousOwner.Set(&remoteView{id: 99}) | 76 | + | ac.previousOwner.Set(sentinel) |
| 76 | }, | 77 | }, | ||
| 77 | check: func(t *testing.T, _ *session, ac *attachedClient, view *remoteView) { | 78 | check: func(t *testing.T, _ *session, ac *attachedClient, view *remoteView) { | ||
| 78 | other := ac.previousOwner.Get() | 79 | other := ac.previousOwner.Get() | ||
| 79 | require.Same(t, view, ac.currentAttachmentOwner()) | 80 | require.Same(t, view, ac.currentAttachmentOwner()) | ||
| 80 | require.True(t, view.attachmentRegistered(ac)) | 81 | require.True(t, view.attachmentRegistered(ac)) | ||
| 81 | - | require.IsType(t, &remoteView{}, other) | 82 | + | require.Same(t, sentinel, other) |
| 82 | }, | 83 | }, | ||
| 83 | }, | 84 | }, | ||
| 84 | } | 85 | } | ||
M internal/usecase/daemon/remote_attachment_transition_picker_test.go +3 -2
| @@ -15,8 +15,8 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | |||
| 15 | 15 | endpoint: "remote.example", lifecycleID: domain.SessionLifecycleID{1}, sessionName: "remote", | |
| 16 | 16 | }} | |
| 17 | 17 | link := &remoteLink{view: view, generation: 1, transport: newRemoteLinkTestTransport()} | |
| 18 | - | view.link, view.linkGeneration = link, 1 | |
| 19 | 18 | view.mu.Lock() | |
| 19 | + | view.link, view.linkGeneration = link, 1 | |
| 20 | 20 | require.False(t, remoteViewLinkReusableLocked(view), "the fixture must start with an unusable link") | |
| 21 | 21 | view.mu.Unlock() | |
| 22 | 22 | d.mu.Lock() | |
| @@ -24,10 +24,11 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | |||
| 24 | 24 | d.mu.Unlock() | |
| 25 | 25 | ||
| 26 | 26 | model := picker.New(nil, picker.SelectionConfig{}) | |
| 27 | + | token := source.attachmentToken(ac, ac.transport()) | |
| 27 | 28 | ac.overlays.pickerMu.Lock() | |
| 28 | 29 | ac.overlays.picker = model | |
| 29 | 30 | ac.overlays.pickerGeneration++ | |
| 30 | - | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: source.attachmentToken(ac, ac.transport())} | |
| 31 | + | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: token} | |
| 31 | 32 | ac.overlays.pickerRemoteSelection = selection | |
| 32 | 33 | ac.overlays.pickerMu.Unlock() | |
| 33 | 34 | ||
| @@ -15,8 +15,8 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | |||||
| 15 | endpoint: "remote.example", lifecycleID: domain.SessionLifecycleID{1}, sessionName: "remote", | 15 | endpoint: "remote.example", lifecycleID: domain.SessionLifecycleID{1}, sessionName: "remote", | ||
| 16 | }} | 16 | }} | ||
| 17 | link := &remoteLink{view: view, generation: 1, transport: newRemoteLinkTestTransport()} | 17 | link := &remoteLink{view: view, generation: 1, transport: newRemoteLinkTestTransport()} | ||
| 18 | - | view.link, view.linkGeneration = link, 1 | |||
| 19 | view.mu.Lock() | 18 | view.mu.Lock() | ||
| 19 | + | view.link, view.linkGeneration = link, 1 | |||
| 20 | require.False(t, remoteViewLinkReusableLocked(view), "the fixture must start with an unusable link") | 20 | require.False(t, remoteViewLinkReusableLocked(view), "the fixture must start with an unusable link") | ||
| 21 | view.mu.Unlock() | 21 | view.mu.Unlock() | ||
| 22 | d.mu.Lock() | 22 | d.mu.Lock() | ||
| @@ -24,10 +24,11 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) { | |||||
| 24 | d.mu.Unlock() | 24 | d.mu.Unlock() | ||
| 25 | 25 | ||||
| 26 | model := picker.New(nil, picker.SelectionConfig{}) | 26 | model := picker.New(nil, picker.SelectionConfig{}) | ||
| 27 | + | token := source.attachmentToken(ac, ac.transport()) | |||
| 27 | ac.overlays.pickerMu.Lock() | 28 | ac.overlays.pickerMu.Lock() | ||
| 28 | ac.overlays.picker = model | 29 | ac.overlays.picker = model | ||
| 29 | ac.overlays.pickerGeneration++ | 30 | ac.overlays.pickerGeneration++ | ||
| 30 | - | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: source.attachmentToken(ac, ac.transport())} | 31 | + | selection := &remotePickerSelection{model: model, generation: ac.overlays.pickerGeneration, token: token} |
| 31 | ac.overlays.pickerRemoteSelection = selection | 32 | ac.overlays.pickerRemoteSelection = selection | ||
| 32 | ac.overlays.pickerMu.Unlock() | 33 | ac.overlays.pickerMu.Unlock() | ||
| 33 | 34 | ||||
M internal/usecase/daemon/remote_link.go +78 -22
| @@ -42,6 +42,7 @@ type remoteLink struct { | |||
| 42 | 42 | target domain.RemoteSessionTarget | |
| 43 | 43 | clientID [16]byte | |
| 44 | 44 | transport ports.Transport | |
| 45 | + | clock ports.Clock | |
| 45 | 46 | cancel context.CancelFunc | |
| 46 | 47 | ||
| 47 | 48 | sendMu sync.Mutex | |
| @@ -244,10 +245,7 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||
| 244 | 245 | stopAndJoinRemoteLink(replaced) | |
| 245 | 246 | } | |
| 246 | 247 | for _, attachment := range repaint { | |
| 247 | - | token := attachmentOwnerToken(winner, attachment, attachment.transport()) | |
| 248 | - | if token.ac != nil { | |
| 249 | - | go d.paintRemoteView(winner, attachment, false, token) | |
| 250 | - | } | |
| 248 | + | d.scheduleRemoteViewPaint(winner, attachment, false) | |
| 251 | 249 | } | |
| 252 | 250 | if winner != nil { | |
| 253 | 251 | d.mu.Lock() | |
| @@ -256,6 +254,12 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||
| 256 | 254 | if healthy && current == winner { | |
| 257 | 255 | return winner, nil | |
| 258 | 256 | } | |
| 257 | + | // A winner can be invalidated by its receiver immediately after | |
| 258 | + | // publication. Do not spin constructors in a tight loop while the | |
| 259 | + | // remote endpoint is unavailable. | |
| 260 | + | if !sleepRemoteViewRetry(ctx, d.clock) { | |
| 261 | + | return nil, ctx.Err() | |
| 262 | + | } | |
| 259 | 263 | continue | |
| 260 | 264 | } | |
| 261 | 265 | return nil, publishErr | |
| @@ -315,17 +319,25 @@ func installRemoteViewCandidateLocked(view, candidate *remoteView) (*remoteLink, | |||
| 315 | 319 | return replaced, true | |
| 316 | 320 | } | |
| 317 | 321 | ||
| 318 | - | func stopAndJoinRemoteLink(link *remoteLink) { | |
| 322 | + | // interruptRemoteLink invalidates an exact link without waiting for its | |
| 323 | + | // receiver. Callers may join afterward, but a receiver must never join itself. | |
| 324 | + | func interruptRemoteLink(link *remoteLink) { | |
| 319 | 325 | if link == nil { | |
| 320 | 326 | return | |
| 321 | 327 | } | |
| 322 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 328 | + | if link.commands != nil { | |
| 329 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 330 | + | } | |
| 323 | 331 | if link.cancel != nil { | |
| 324 | 332 | link.cancel() | |
| 325 | 333 | } | |
| 326 | 334 | if link.transport != nil { | |
| 327 | 335 | _ = link.transport.Close() | |
| 328 | 336 | } | |
| 337 | + | } | |
| 338 | + | ||
| 339 | + | func stopAndJoinRemoteLink(link *remoteLink) { | |
| 340 | + | interruptRemoteLink(link) | |
| 329 | 341 | joinRemoteLink(link) | |
| 330 | 342 | } | |
| 331 | 343 | ||
| @@ -362,6 +374,7 @@ func (d *Daemon) constructRemoteView(ctx context.Context, target domain.RemoteSe | |||
| 362 | 374 | generation: view.linkGeneration, | |
| 363 | 375 | target: target, | |
| 364 | 376 | clientID: clientID, | |
| 377 | + | clock: d.clock, | |
| 365 | 378 | cancel: cancelLink, | |
| 366 | 379 | commands: NewCommandRequestTracker(), | |
| 367 | 380 | done: make(chan struct{}), | |
| @@ -639,7 +652,14 @@ func (v *remoteView) applyRemoteOutput(link *remoteLink, output ports.Output) (a | |||
| 639 | 652 | return 0, true, nil, false | |
| 640 | 653 | } | |
| 641 | 654 | if output.New == 0 { | |
| 642 | - | return 0, false, nil, true | |
| 655 | + | if len(output.Data) != 0 && v.screen != nil { | |
| 656 | + | v.screen.Write(output.Data) | |
| 657 | + | } | |
| 658 | + | attachments = make([]*attachedClient, 0, len(v.attachments)) | |
| 659 | + | for attachment := range v.attachments { | |
| 660 | + | attachments = append(attachments, attachment) | |
| 661 | + | } | |
| 662 | + | return 0, false, attachments, true | |
| 643 | 663 | } | |
| 644 | 664 | v.output = next | |
| 645 | 665 | v.resetRequested = false | |
| @@ -772,10 +792,7 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||
| 772 | 792 | // I/O happen after that lock has been released. | |
| 773 | 793 | func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*attachedClient) { | |
| 774 | 794 | for _, attachment := range attachments { | |
| 775 | - | token := attachmentOwnerToken(view, attachment, attachment.transport()) | |
| 776 | - | if token.ac != nil { | |
| 777 | - | go d.paintRemoteView(view, attachment, false, token) | |
| 778 | - | } | |
| 795 | + | d.scheduleRemoteViewPaint(view, attachment, false) | |
| 779 | 796 | } | |
| 780 | 797 | } | |
| 781 | 798 | ||
| @@ -796,20 +813,59 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||
| 796 | 813 | link.sendMu.Unlock() | |
| 797 | 814 | return errRemoteViewStale | |
| 798 | 815 | } | |
| 799 | - | err := transport.Send(frame) | |
| 800 | - | link.sendMu.Unlock() | |
| 801 | - | if err != nil { | |
| 802 | - | view.mu.Lock() | |
| 803 | - | if view.link == link && view.linkGeneration == link.generation { | |
| 804 | - | link.active = false | |
| 805 | - | signalRemoteViewMetadataChangedLocked(view) | |
| 816 | + | if link.clock == nil { | |
| 817 | + | err := transport.Send(frame) | |
| 818 | + | link.sendMu.Unlock() | |
| 819 | + | if err != nil { | |
| 820 | + | view.mu.Lock() | |
| 821 | + | if view.link == link && view.linkGeneration == link.generation { | |
| 822 | + | link.active = false | |
| 823 | + | signalRemoteViewMetadataChangedLocked(view) | |
| 824 | + | } | |
| 825 | + | view.mu.Unlock() | |
| 826 | + | if link.commands != nil { | |
| 827 | + | link.commands.FailGeneration(link.generation, err) | |
| 828 | + | } | |
| 829 | + | if link.cancel != nil { | |
| 830 | + | link.cancel() | |
| 831 | + | } | |
| 832 | + | _ = transport.Close() | |
| 806 | 833 | } | |
| 807 | - | view.mu.Unlock() | |
| 808 | - | link.commands.FailGeneration(link.generation, err) | |
| 809 | - | link.cancel() | |
| 834 | + | return err | |
| 835 | + | } | |
| 836 | + | ||
| 837 | + | result := make(chan error, 1) | |
| 838 | + | go func() { | |
| 839 | + | err := transport.Send(frame) | |
| 840 | + | link.sendMu.Unlock() | |
| 841 | + | result <- err | |
| 842 | + | }() | |
| 843 | + | timer := link.clock.NewTimer(detachNotifyTimeout) | |
| 844 | + | select { | |
| 845 | + | case err := <-result: | |
| 846 | + | timer.Stop() | |
| 847 | + | if err != nil { | |
| 848 | + | view.mu.Lock() | |
| 849 | + | if view.link == link && view.linkGeneration == link.generation { | |
| 850 | + | link.active = false | |
| 851 | + | signalRemoteViewMetadataChangedLocked(view) | |
| 852 | + | } | |
| 853 | + | view.mu.Unlock() | |
| 854 | + | if link.commands != nil { | |
| 855 | + | link.commands.FailGeneration(link.generation, err) | |
| 856 | + | } | |
| 857 | + | if link.cancel != nil { | |
| 858 | + | link.cancel() | |
| 859 | + | } | |
| 860 | + | _ = transport.Close() | |
| 861 | + | } | |
| 862 | + | return err | |
| 863 | + | case <-timer.C(): | |
| 864 | + | // Transport.Close is required to interrupt Send. The sending goroutine | |
| 865 | + | // owns sendMu and releases it once the adapter observes that close. | |
| 810 | 866 | _ = transport.Close() | |
| 867 | + | return errSendTimedOut | |
| 811 | 868 | } | |
| 812 | - | return err | |
| 813 | 869 | } | |
| 814 | 870 | ||
| 815 | 871 | // handleRemoteViewInput forwards the raw local-terminal bytes and their | |
| @@ -42,6 +42,7 @@ type remoteLink struct { | |||||
| 42 | target domain.RemoteSessionTarget | 42 | target domain.RemoteSessionTarget | ||
| 43 | clientID [16]byte | 43 | clientID [16]byte | ||
| 44 | transport ports.Transport | 44 | transport ports.Transport | ||
| 45 | + | clock ports.Clock | |||
| 45 | cancel context.CancelFunc | 46 | cancel context.CancelFunc | ||
| 46 | 47 | ||||
| 47 | sendMu sync.Mutex | 48 | sendMu sync.Mutex | ||
| @@ -244,10 +245,7 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||||
| 244 | stopAndJoinRemoteLink(replaced) | 245 | stopAndJoinRemoteLink(replaced) | ||
| 245 | } | 246 | } | ||
| 246 | for _, attachment := range repaint { | 247 | for _, attachment := range repaint { | ||
| 247 | - | token := attachmentOwnerToken(winner, attachment, attachment.transport()) | 248 | + | d.scheduleRemoteViewPaint(winner, attachment, false) |
| 248 | - | if token.ac != nil { | |||
| 249 | - | go d.paintRemoteView(winner, attachment, false, token) | |||
| 250 | - | } | |||
| 251 | } | 249 | } | ||
| 252 | if winner != nil { | 250 | if winner != nil { | ||
| 253 | d.mu.Lock() | 251 | d.mu.Lock() | ||
| @@ -256,6 +254,12 @@ func (d *Daemon) openRemoteView(ctx context.Context, target domain.RemoteSession | |||||
| 256 | if healthy && current == winner { | 254 | if healthy && current == winner { | ||
| 257 | return winner, nil | 255 | return winner, nil | ||
| 258 | } | 256 | } | ||
| 257 | + | // A winner can be invalidated by its receiver immediately after | |||
| 258 | + | // publication. Do not spin constructors in a tight loop while the | |||
| 259 | + | // remote endpoint is unavailable. | |||
| 260 | + | if !sleepRemoteViewRetry(ctx, d.clock) { | |||
| 261 | + | return nil, ctx.Err() | |||
| 262 | + | } | |||
| 259 | continue | 263 | continue | ||
| 260 | } | 264 | } | ||
| 261 | return nil, publishErr | 265 | return nil, publishErr | ||
| @@ -315,17 +319,25 @@ func installRemoteViewCandidateLocked(view, candidate *remoteView) (*remoteLink, | |||||
| 315 | return replaced, true | 319 | return replaced, true | ||
| 316 | } | 320 | } | ||
| 317 | 321 | ||||
| 318 | - | func stopAndJoinRemoteLink(link *remoteLink) { | 322 | + | // interruptRemoteLink invalidates an exact link without waiting for its |
| 323 | + | // receiver. Callers may join afterward, but a receiver must never join itself. | |||
| 324 | + | func interruptRemoteLink(link *remoteLink) { | |||
| 319 | if link == nil { | 325 | if link == nil { | ||
| 320 | return | 326 | return | ||
| 321 | } | 327 | } | ||
| 322 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 328 | + | if link.commands != nil { |
| 329 | + | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |||
| 330 | + | } | |||
| 323 | if link.cancel != nil { | 331 | if link.cancel != nil { | ||
| 324 | link.cancel() | 332 | link.cancel() | ||
| 325 | } | 333 | } | ||
| 326 | if link.transport != nil { | 334 | if link.transport != nil { | ||
| 327 | _ = link.transport.Close() | 335 | _ = link.transport.Close() | ||
| 328 | } | 336 | } | ||
| 337 | + | } | |||
| 338 | + | ||||
| 339 | + | func stopAndJoinRemoteLink(link *remoteLink) { | |||
| 340 | + | interruptRemoteLink(link) | |||
| 329 | joinRemoteLink(link) | 341 | joinRemoteLink(link) | ||
| 330 | } | 342 | } | ||
| 331 | 343 | ||||
| @@ -362,6 +374,7 @@ func (d *Daemon) constructRemoteView(ctx context.Context, target domain.RemoteSe | |||||
| 362 | generation: view.linkGeneration, | 374 | generation: view.linkGeneration, | ||
| 363 | target: target, | 375 | target: target, | ||
| 364 | clientID: clientID, | 376 | clientID: clientID, | ||
| 377 | + | clock: d.clock, | |||
| 365 | cancel: cancelLink, | 378 | cancel: cancelLink, | ||
| 366 | commands: NewCommandRequestTracker(), | 379 | commands: NewCommandRequestTracker(), | ||
| 367 | done: make(chan struct{}), | 380 | done: make(chan struct{}), | ||
| @@ -639,7 +652,14 @@ func (v *remoteView) applyRemoteOutput(link *remoteLink, output ports.Output) (a | |||||
| 639 | return 0, true, nil, false | 652 | return 0, true, nil, false | ||
| 640 | } | 653 | } | ||
| 641 | if output.New == 0 { | 654 | if output.New == 0 { | ||
| 642 | - | return 0, false, nil, true | 655 | + | if len(output.Data) != 0 && v.screen != nil { |
| 656 | + | v.screen.Write(output.Data) | |||
| 657 | + | } | |||
| 658 | + | attachments = make([]*attachedClient, 0, len(v.attachments)) | |||
| 659 | + | for attachment := range v.attachments { | |||
| 660 | + | attachments = append(attachments, attachment) | |||
| 661 | + | } | |||
| 662 | + | return 0, false, attachments, true | |||
| 643 | } | 663 | } | ||
| 644 | v.output = next | 664 | v.output = next | ||
| 645 | v.resetRequested = false | 665 | v.resetRequested = false | ||
| @@ -772,10 +792,7 @@ func (d *Daemon) handleRemoteLinkFrame(link *remoteLink, frame ports.Frame) erro | |||||
| 772 | // I/O happen after that lock has been released. | 792 | // I/O happen after that lock has been released. | ||
| 773 | func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*attachedClient) { | 793 | func (d *Daemon) repaintRemoteViewAttachments(view *remoteView, attachments []*attachedClient) { | ||
| 774 | for _, attachment := range attachments { | 794 | for _, attachment := range attachments { | ||
| 775 | - | token := attachmentOwnerToken(view, attachment, attachment.transport()) | 795 | + | d.scheduleRemoteViewPaint(view, attachment, false) |
| 776 | - | if token.ac != nil { | |||
| 777 | - | go d.paintRemoteView(view, attachment, false, token) | |||
| 778 | - | } | |||
| 779 | } | 796 | } | ||
| 780 | } | 797 | } | ||
| 781 | 798 | ||||
| @@ -796,20 +813,59 @@ func (link *remoteLink) send(frame ports.Frame) error { | |||||
| 796 | link.sendMu.Unlock() | 813 | link.sendMu.Unlock() | ||
| 797 | return errRemoteViewStale | 814 | return errRemoteViewStale | ||
| 798 | } | 815 | } | ||
| 799 | - | err := transport.Send(frame) | 816 | + | if link.clock == nil { |
| 800 | - | link.sendMu.Unlock() | 817 | + | err := transport.Send(frame) |
| 801 | - | if err != nil { | 818 | + | link.sendMu.Unlock() |
| 802 | - | view.mu.Lock() | 819 | + | if err != nil { |
| 803 | - | if view.link == link && view.linkGeneration == link.generation { | 820 | + | view.mu.Lock() |
| 804 | - | link.active = false | 821 | + | if view.link == link && view.linkGeneration == link.generation { |
| 805 | - | signalRemoteViewMetadataChangedLocked(view) | 822 | + | link.active = false |
| 823 | + | signalRemoteViewMetadataChangedLocked(view) | |||
| 824 | + | } | |||
| 825 | + | view.mu.Unlock() | |||
| 826 | + | if link.commands != nil { | |||
| 827 | + | link.commands.FailGeneration(link.generation, err) | |||
| 828 | + | } | |||
| 829 | + | if link.cancel != nil { | |||
| 830 | + | link.cancel() | |||
| 831 | + | } | |||
| 832 | + | _ = transport.Close() | |||
| 806 | } | 833 | } | ||
| 807 | - | view.mu.Unlock() | 834 | + | return err |
| 808 | - | link.commands.FailGeneration(link.generation, err) | 835 | + | } |
| 809 | - | link.cancel() | 836 | + | |
| 837 | + | result := make(chan error, 1) | |||
| 838 | + | go func() { | |||
| 839 | + | err := transport.Send(frame) | |||
| 840 | + | link.sendMu.Unlock() | |||
| 841 | + | result <- err | |||
| 842 | + | }() | |||
| 843 | + | timer := link.clock.NewTimer(detachNotifyTimeout) | |||
| 844 | + | select { | |||
| 845 | + | case err := <-result: | |||
| 846 | + | timer.Stop() | |||
| 847 | + | if err != nil { | |||
| 848 | + | view.mu.Lock() | |||
| 849 | + | if view.link == link && view.linkGeneration == link.generation { | |||
| 850 | + | link.active = false | |||
| 851 | + | signalRemoteViewMetadataChangedLocked(view) | |||
| 852 | + | } | |||
| 853 | + | view.mu.Unlock() | |||
| 854 | + | if link.commands != nil { | |||
| 855 | + | link.commands.FailGeneration(link.generation, err) | |||
| 856 | + | } | |||
| 857 | + | if link.cancel != nil { | |||
| 858 | + | link.cancel() | |||
| 859 | + | } | |||
| 860 | + | _ = transport.Close() | |||
| 861 | + | } | |||
| 862 | + | return err | |||
| 863 | + | case <-timer.C(): | |||
| 864 | + | // Transport.Close is required to interrupt Send. The sending goroutine | |||
| 865 | + | // owns sendMu and releases it once the adapter observes that close. | |||
| 810 | _ = transport.Close() | 866 | _ = transport.Close() | ||
| 867 | + | return errSendTimedOut | |||
| 811 | } | 868 | } | ||
| 812 | - | return err | |||
| 813 | } | 869 | } | ||
| 814 | 870 | ||||
| 815 | // handleRemoteViewInput forwards the raw local-terminal bytes and their | 871 | // handleRemoteViewInput forwards the raw local-terminal bytes and their | ||
M internal/usecase/daemon/remote_link_test.go +24 -11
| @@ -186,14 +186,7 @@ func newRemoteMetadataLinkFixture(t *testing.T) (*Daemon, *remoteView, *remoteLi | |||
| 186 | 186 | ||
| 187 | 187 | func attachRemoteMetadataClient(t *testing.T, view *remoteView) (*attachedClient, chan ports.Frame) { | |
| 188 | 188 | t.Helper() | |
| 189 | - | transport, sends := newCapturingTransport(t) | |
| 190 | - | ac := &attachedClient{tr: transport, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 191 | - | ac.initOverlays() | |
| 192 | - | ac.setAttachmentOwner(view) | |
| 193 | - | view.mu.Lock() | |
| 194 | - | require.True(t, view.registerAttachmentLocked(ac)) | |
| 195 | - | view.mu.Unlock() | |
| 196 | - | return ac, sends | |
| 189 | + | return attachRemoteGeometryClient(t, view, domain.Size{Cols: 80, Rows: 24}) | |
| 197 | 190 | } | |
| 198 | 191 | ||
| 199 | 192 | func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame { | |
| @@ -241,7 +234,10 @@ func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t | |||
| 241 | 234 | callbacks := make(chan struct{}, 2) | |
| 242 | 235 | for _, ac := range []*attachedClient{first, second} { | |
| 243 | 236 | ac.beforeAttachmentTokenValidation = func() { | |
| 237 | + | // This intentionally empty critical section is a callback-outside-lock | |
| 238 | + | // probe: the callback must be able to acquire view.mu. | |
| 244 | 239 | view.mu.Lock() | |
| 240 | + | //lint:ignore SA2001 the empty section is the lock-ordering probe | |
| 245 | 241 | view.mu.Unlock() | |
| 246 | 242 | callbacks <- struct{}{} | |
| 247 | 243 | } | |
| @@ -271,6 +267,23 @@ func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t | |||
| 271 | 267 | view.mu.Unlock() | |
| 272 | 268 | } | |
| 273 | 269 | ||
| 270 | + | func TestRemoteLinkAppliesSideEffectOutputWithoutAdvancingStateOrAcking(t *testing.T) { | |
| 271 | + | _, view, link, _ := newRemoteMetadataLinkFixture(t) | |
| 272 | + | attachment, _ := attachRemoteMetadataClient(t, view) | |
| 273 | + | before := view.output | |
| 274 | + | ack, reset, attachments, accepted := view.applyRemoteOutput(link, ports.Output{ | |
| 275 | + | Epoch: 1, Size: domain.Size{Cols: 80, Rows: 22}, Data: []byte("side effect"), | |
| 276 | + | }) | |
| 277 | + | require.True(t, accepted) | |
| 278 | + | require.Zero(t, ack) | |
| 279 | + | require.False(t, reset) | |
| 280 | + | require.Equal(t, []*attachedClient{attachment}, attachments) | |
| 281 | + | view.mu.Lock() | |
| 282 | + | require.Equal(t, before, view.output) | |
| 283 | + | require.Contains(t, screenLineText(view.screen, 0), "side effect") | |
| 284 | + | view.mu.Unlock() | |
| 285 | + | } | |
| 286 | + | ||
| 274 | 287 | func TestRemoteLinkIgnoresDuplicateAndOlderMetadataWithoutStopping(t *testing.T) { | |
| 275 | 288 | d, view, link, transport := newRemoteMetadataLinkFixture(t) | |
| 276 | 289 | metadata := ports.SessionMeta{ | |
| @@ -899,7 +912,7 @@ func TestOpenRemoteViewCancellationClosesUnpublishedTransport(t *testing.T) { | |||
| 899 | 912 | awaitFrame(t, transport.sent, ports.MsgHello) | |
| 900 | 913 | cancel() | |
| 901 | 914 | ||
| 902 | - | got := <-resultCh | |
| 915 | + | got := awaitTestValue(t, resultCh, "canceled remote view construction did not finish") | |
| 903 | 916 | require.Nil(t, got.view) | |
| 904 | 917 | require.ErrorIs(t, got.err, context.Canceled) | |
| 905 | 918 | select { | |
| @@ -1077,7 +1090,7 @@ func TestOpenRemoteViewElectsOneConstructorAndCanceledWaiterDoesNotPoisonWinner( | |||
| 1077 | 1090 | view, err := d.openRemoteView(context.Background(), target, size) | |
| 1078 | 1091 | ownerResult <- result{view: view, err: err} | |
| 1079 | 1092 | }() | |
| 1080 | - | <-started | |
| 1093 | + | awaitTestCompletion(t, started, "remote view constructor did not start") | |
| 1081 | 1094 | ||
| 1082 | 1095 | waiterCtx, cancelWaiter := context.WithCancel(context.Background()) | |
| 1083 | 1096 | cancelWaiter() | |
| @@ -1087,7 +1100,7 @@ func TestOpenRemoteViewElectsOneConstructorAndCanceledWaiterDoesNotPoisonWinner( | |||
| 1087 | 1100 | require.Equal(t, int32(1), factory.calls.Load()) | |
| 1088 | 1101 | ||
| 1089 | 1102 | close(release) | |
| 1090 | - | owner := <-ownerResult | |
| 1103 | + | owner := awaitTestValue(t, ownerResult, "remote view constructor result did not arrive") | |
| 1091 | 1104 | require.NoError(t, owner.err) | |
| 1092 | 1105 | require.NotNil(t, owner.view) | |
| 1093 | 1106 | require.Equal(t, int32(1), factory.calls.Load()) | |
| @@ -186,14 +186,7 @@ func newRemoteMetadataLinkFixture(t *testing.T) (*Daemon, *remoteView, *remoteLi | |||||
| 186 | 186 | ||||
| 187 | func attachRemoteMetadataClient(t *testing.T, view *remoteView) (*attachedClient, chan ports.Frame) { | 187 | func attachRemoteMetadataClient(t *testing.T, view *remoteView) (*attachedClient, chan ports.Frame) { | ||
| 188 | t.Helper() | 188 | t.Helper() | ||
| 189 | - | transport, sends := newCapturingTransport(t) | 189 | + | return attachRemoteGeometryClient(t, view, domain.Size{Cols: 80, Rows: 24}) |
| 190 | - | ac := &attachedClient{tr: transport, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |||
| 191 | - | ac.initOverlays() | |||
| 192 | - | ac.setAttachmentOwner(view) | |||
| 193 | - | view.mu.Lock() | |||
| 194 | - | require.True(t, view.registerAttachmentLocked(ac)) | |||
| 195 | - | view.mu.Unlock() | |||
| 196 | - | return ac, sends | |||
| 197 | } | 190 | } | ||
| 198 | 191 | ||||
| 199 | func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame { | 192 | func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame { | ||
| @@ -241,7 +234,10 @@ func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t | |||||
| 241 | callbacks := make(chan struct{}, 2) | 234 | callbacks := make(chan struct{}, 2) | ||
| 242 | for _, ac := range []*attachedClient{first, second} { | 235 | for _, ac := range []*attachedClient{first, second} { | ||
| 243 | ac.beforeAttachmentTokenValidation = func() { | 236 | ac.beforeAttachmentTokenValidation = func() { | ||
| 237 | + | // This intentionally empty critical section is a callback-outside-lock | |||
| 238 | + | // probe: the callback must be able to acquire view.mu. | |||
| 244 | view.mu.Lock() | 239 | view.mu.Lock() | ||
| 240 | + | //lint:ignore SA2001 the empty section is the lock-ordering probe | |||
| 245 | view.mu.Unlock() | 241 | view.mu.Unlock() | ||
| 246 | callbacks <- struct{}{} | 242 | callbacks <- struct{}{} | ||
| 247 | } | 243 | } | ||
| @@ -271,6 +267,23 @@ func TestRemoteLinkAcceptedMetadataRepaintsEveryAttachedClientUnderLocalChrome(t | |||||
| 271 | view.mu.Unlock() | 267 | view.mu.Unlock() | ||
| 272 | } | 268 | } | ||
| 273 | 269 | ||||
| 270 | + | func TestRemoteLinkAppliesSideEffectOutputWithoutAdvancingStateOrAcking(t *testing.T) { | |||
| 271 | + | _, view, link, _ := newRemoteMetadataLinkFixture(t) | |||
| 272 | + | attachment, _ := attachRemoteMetadataClient(t, view) | |||
| 273 | + | before := view.output | |||
| 274 | + | ack, reset, attachments, accepted := view.applyRemoteOutput(link, ports.Output{ | |||
| 275 | + | Epoch: 1, Size: domain.Size{Cols: 80, Rows: 22}, Data: []byte("side effect"), | |||
| 276 | + | }) | |||
| 277 | + | require.True(t, accepted) | |||
| 278 | + | require.Zero(t, ack) | |||
| 279 | + | require.False(t, reset) | |||
| 280 | + | require.Equal(t, []*attachedClient{attachment}, attachments) | |||
| 281 | + | view.mu.Lock() | |||
| 282 | + | require.Equal(t, before, view.output) | |||
| 283 | + | require.Contains(t, screenLineText(view.screen, 0), "side effect") | |||
| 284 | + | view.mu.Unlock() | |||
| 285 | + | } | |||
| 286 | + | ||||
| 274 | func TestRemoteLinkIgnoresDuplicateAndOlderMetadataWithoutStopping(t *testing.T) { | 287 | func TestRemoteLinkIgnoresDuplicateAndOlderMetadataWithoutStopping(t *testing.T) { | ||
| 275 | d, view, link, transport := newRemoteMetadataLinkFixture(t) | 288 | d, view, link, transport := newRemoteMetadataLinkFixture(t) | ||
| 276 | metadata := ports.SessionMeta{ | 289 | metadata := ports.SessionMeta{ | ||
| @@ -899,7 +912,7 @@ func TestOpenRemoteViewCancellationClosesUnpublishedTransport(t *testing.T) { | |||||
| 899 | awaitFrame(t, transport.sent, ports.MsgHello) | 912 | awaitFrame(t, transport.sent, ports.MsgHello) | ||
| 900 | cancel() | 913 | cancel() | ||
| 901 | 914 | ||||
| 902 | - | got := <-resultCh | 915 | + | got := awaitTestValue(t, resultCh, "canceled remote view construction did not finish") |
| 903 | require.Nil(t, got.view) | 916 | require.Nil(t, got.view) | ||
| 904 | require.ErrorIs(t, got.err, context.Canceled) | 917 | require.ErrorIs(t, got.err, context.Canceled) | ||
| 905 | select { | 918 | select { | ||
| @@ -1077,7 +1090,7 @@ func TestOpenRemoteViewElectsOneConstructorAndCanceledWaiterDoesNotPoisonWinner( | |||||
| 1077 | view, err := d.openRemoteView(context.Background(), target, size) | 1090 | view, err := d.openRemoteView(context.Background(), target, size) | ||
| 1078 | ownerResult <- result{view: view, err: err} | 1091 | ownerResult <- result{view: view, err: err} | ||
| 1079 | }() | 1092 | }() | ||
| 1080 | - | <-started | 1093 | + | awaitTestCompletion(t, started, "remote view constructor did not start") |
| 1081 | 1094 | ||||
| 1082 | waiterCtx, cancelWaiter := context.WithCancel(context.Background()) | 1095 | waiterCtx, cancelWaiter := context.WithCancel(context.Background()) | ||
| 1083 | cancelWaiter() | 1096 | cancelWaiter() | ||
| @@ -1087,7 +1100,7 @@ func TestOpenRemoteViewElectsOneConstructorAndCanceledWaiterDoesNotPoisonWinner( | |||||
| 1087 | require.Equal(t, int32(1), factory.calls.Load()) | 1100 | require.Equal(t, int32(1), factory.calls.Load()) | ||
| 1088 | 1101 | ||||
| 1089 | close(release) | 1102 | close(release) | ||
| 1090 | - | owner := <-ownerResult | 1103 | + | owner := awaitTestValue(t, ownerResult, "remote view constructor result did not arrive") |
| 1091 | require.NoError(t, owner.err) | 1104 | require.NoError(t, owner.err) | ||
| 1092 | require.NotNil(t, owner.view) | 1105 | require.NotNil(t, owner.view) | ||
| 1093 | require.Equal(t, int32(1), factory.calls.Load()) | 1106 | require.Equal(t, int32(1), factory.calls.Load()) | ||
M internal/usecase/daemon/remote_picker_selection.go +9 -2
| @@ -49,7 +49,11 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar | |||
| 49 | 49 | return errAttachmentTransition | |
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | - | ctx, cancel := context.WithCancel(context.Background()) | |
| 52 | + | root := d.serveCtx | |
| 53 | + | if root == nil { | |
| 54 | + | root = context.Background() | |
| 55 | + | } | |
| 56 | + | ctx, cancel := context.WithCancel(root) | |
| 53 | 57 | selection := &remotePickerSelection{ | |
| 54 | 58 | token: token, | |
| 55 | 59 | target: remoteTarget, | |
| @@ -80,10 +84,13 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar | |||
| 80 | 84 | } | |
| 81 | 85 | ||
| 82 | 86 | func (d *Daemon) completeRemotePickerSelection(selection *remotePickerSelection) { | |
| 83 | - | if selection == nil || selection.token.ac == nil { | |
| 87 | + | if selection == nil { | |
| 84 | 88 | return | |
| 85 | 89 | } | |
| 86 | 90 | defer selection.cancel() | |
| 91 | + | if selection.token.ac == nil { | |
| 92 | + | return | |
| 93 | + | } | |
| 87 | 94 | view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot()) | |
| 88 | 95 | if err != nil { | |
| 89 | 96 | d.failRemotePickerSelection(selection, err) | |
| @@ -49,7 +49,11 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar | |||||
| 49 | return errAttachmentTransition | 49 | return errAttachmentTransition | ||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | - | ctx, cancel := context.WithCancel(context.Background()) | 52 | + | root := d.serveCtx |
| 53 | + | if root == nil { | |||
| 54 | + | root = context.Background() | |||
| 55 | + | } | |||
| 56 | + | ctx, cancel := context.WithCancel(root) | |||
| 53 | selection := &remotePickerSelection{ | 57 | selection := &remotePickerSelection{ | ||
| 54 | token: token, | 58 | token: token, | ||
| 55 | target: remoteTarget, | 59 | target: remoteTarget, | ||
| @@ -80,10 +84,13 @@ func (d *Daemon) startRemotePickerSelection(token attachmentConnectionToken, tar | |||||
| 80 | } | 84 | } | ||
| 81 | 85 | ||||
| 82 | func (d *Daemon) completeRemotePickerSelection(selection *remotePickerSelection) { | 86 | func (d *Daemon) completeRemotePickerSelection(selection *remotePickerSelection) { | ||
| 83 | - | if selection == nil || selection.token.ac == nil { | 87 | + | if selection == nil { |
| 84 | return | 88 | return | ||
| 85 | } | 89 | } | ||
| 86 | defer selection.cancel() | 90 | defer selection.cancel() | ||
| 91 | + | if selection.token.ac == nil { | |||
| 92 | + | return | |||
| 93 | + | } | |||
| 87 | view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot()) | 94 | view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot()) | ||
| 88 | if err != nil { | 95 | if err != nil { | ||
| 89 | d.failRemotePickerSelection(selection, err) | 96 | d.failRemotePickerSelection(selection, err) | ||
M internal/usecase/daemon/remote_reconnect.go +21 -0
| @@ -2,10 +2,31 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "time" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/internal/domain" | |
| 8 | + | "github.com/bnema/vev/internal/ports" | |
| 7 | 9 | ) | |
| 8 | 10 | ||
| 11 | + | const remoteViewRetryDelay = 100 * time.Millisecond | |
| 12 | + | ||
| 13 | + | func sleepRemoteViewRetry(ctx context.Context, clock ports.Clock) bool { | |
| 14 | + | if ctx == nil { | |
| 15 | + | ctx = context.Background() | |
| 16 | + | } | |
| 17 | + | if clock == nil { | |
| 18 | + | return true | |
| 19 | + | } | |
| 20 | + | timer := clock.NewTimer(remoteViewRetryDelay) | |
| 21 | + | defer timer.Stop() | |
| 22 | + | select { | |
| 23 | + | case <-timer.C(): | |
| 24 | + | return true | |
| 25 | + | case <-ctx.Done(): | |
| 26 | + | return false | |
| 27 | + | } | |
| 28 | + | } | |
| 29 | + | ||
| 9 | 30 | // reconnectRemoteView preserves the registered view and its local attachments | |
| 10 | 31 | // while openRemoteView verifies a replacement link. It changes presentation | |
| 11 | 32 | // state only when the failed exact link and reconnect generation are still | |
| @@ -2,10 +2,31 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "time" | |||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| 8 | + | "github.com/bnema/vev/internal/ports" | |||
| 7 | ) | 9 | ) | ||
| 8 | 10 | ||||
| 11 | + | const remoteViewRetryDelay = 100 * time.Millisecond | |||
| 12 | + | ||||
| 13 | + | func sleepRemoteViewRetry(ctx context.Context, clock ports.Clock) bool { | |||
| 14 | + | if ctx == nil { | |||
| 15 | + | ctx = context.Background() | |||
| 16 | + | } | |||
| 17 | + | if clock == nil { | |||
| 18 | + | return true | |||
| 19 | + | } | |||
| 20 | + | timer := clock.NewTimer(remoteViewRetryDelay) | |||
| 21 | + | defer timer.Stop() | |||
| 22 | + | select { | |||
| 23 | + | case <-timer.C(): | |||
| 24 | + | return true | |||
| 25 | + | case <-ctx.Done(): | |||
| 26 | + | return false | |||
| 27 | + | } | |||
| 28 | + | } | |||
| 29 | + | ||||
| 9 | // reconnectRemoteView preserves the registered view and its local attachments | 30 | // reconnectRemoteView preserves the registered view and its local attachments | ||
| 10 | // while openRemoteView verifies a replacement link. It changes presentation | 31 | // while openRemoteView verifies a replacement link. It changes presentation | ||
| 11 | // state only when the failed exact link and reconnect generation are still | 32 | // state only when the failed exact link and reconnect generation are still | ||
M internal/usecase/daemon/remote_render.go +41 -4
| @@ -76,6 +76,46 @@ func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, | |||
| 76 | 76 | }, true | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | + | // scheduleRemoteViewPaint coalesces asynchronous remote-view invalidations. A | |
| 80 | + | // single worker per attachment preserves sendMu serialization without allowing | |
| 81 | + | // ACK, metadata, and overlay callbacks to accumulate blocked goroutines. | |
| 82 | + | func (d *Daemon) scheduleRemoteViewPaint(view *remoteView, ac *attachedClient, reset bool) { | |
| 83 | + | if d == nil || view == nil || ac == nil { | |
| 84 | + | return | |
| 85 | + | } | |
| 86 | + | ac.remotePaintMu.Lock() | |
| 87 | + | ac.remotePaintView = view | |
| 88 | + | ac.remotePaintPending = true | |
| 89 | + | ac.remotePaintReset = ac.remotePaintReset || reset | |
| 90 | + | start := !ac.remotePaintRunning | |
| 91 | + | if start { | |
| 92 | + | ac.remotePaintRunning = true | |
| 93 | + | } | |
| 94 | + | ac.remotePaintMu.Unlock() | |
| 95 | + | if !start { | |
| 96 | + | return | |
| 97 | + | } | |
| 98 | + | go func() { | |
| 99 | + | for { | |
| 100 | + | ac.remotePaintMu.Lock() | |
| 101 | + | if !ac.remotePaintPending { | |
| 102 | + | ac.remotePaintRunning = false | |
| 103 | + | ac.remotePaintMu.Unlock() | |
| 104 | + | return | |
| 105 | + | } | |
| 106 | + | view := ac.remotePaintView | |
| 107 | + | reset := ac.remotePaintReset | |
| 108 | + | ac.remotePaintPending = false | |
| 109 | + | ac.remotePaintReset = false | |
| 110 | + | ac.remotePaintMu.Unlock() | |
| 111 | + | token := attachmentOwnerToken(view, ac, ac.transport()) | |
| 112 | + | if token.ac != nil { | |
| 113 | + | d.paintRemoteView(view, ac, reset, token) | |
| 114 | + | } | |
| 115 | + | } | |
| 116 | + | }() | |
| 117 | + | } | |
| 118 | + | ||
| 79 | 119 | // invalidateAttachedOwner repaints an attachment through its actual owner. | |
| 80 | 120 | // Local UI overlays remain attachment-local while remote content is active, | |
| 81 | 121 | // so they must never enqueue work on the source session's coordinator after | |
| @@ -86,10 +126,7 @@ func (d *Daemon) invalidateAttachedOwner(ac *attachedClient, reset bool, produce | |||
| 86 | 126 | } | |
| 87 | 127 | owner := ac.currentAttachmentOwner() | |
| 88 | 128 | if view, remote := owner.(*remoteView); remote { | |
| 89 | - | token := attachmentOwnerToken(view, ac, ac.transport()) | |
| 90 | - | if token.ac != nil { | |
| 91 | - | go d.paintRemoteView(view, ac, reset, token) | |
| 92 | - | } | |
| 129 | + | d.scheduleRemoteViewPaint(view, ac, reset) | |
| 93 | 130 | return | |
| 94 | 131 | } | |
| 95 | 132 | if sess := localSession(owner); sess != nil { | |
| @@ -76,6 +76,46 @@ func (v *remoteView) renderSnapshot(size domain.Size) (remoteViewRenderSnapshot, | |||||
| 76 | }, true | 76 | }, true | ||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | + | // scheduleRemoteViewPaint coalesces asynchronous remote-view invalidations. A | |||
| 80 | + | // single worker per attachment preserves sendMu serialization without allowing | |||
| 81 | + | // ACK, metadata, and overlay callbacks to accumulate blocked goroutines. | |||
| 82 | + | func (d *Daemon) scheduleRemoteViewPaint(view *remoteView, ac *attachedClient, reset bool) { | |||
| 83 | + | if d == nil || view == nil || ac == nil { | |||
| 84 | + | return | |||
| 85 | + | } | |||
| 86 | + | ac.remotePaintMu.Lock() | |||
| 87 | + | ac.remotePaintView = view | |||
| 88 | + | ac.remotePaintPending = true | |||
| 89 | + | ac.remotePaintReset = ac.remotePaintReset || reset | |||
| 90 | + | start := !ac.remotePaintRunning | |||
| 91 | + | if start { | |||
| 92 | + | ac.remotePaintRunning = true | |||
| 93 | + | } | |||
| 94 | + | ac.remotePaintMu.Unlock() | |||
| 95 | + | if !start { | |||
| 96 | + | return | |||
| 97 | + | } | |||
| 98 | + | go func() { | |||
| 99 | + | for { | |||
| 100 | + | ac.remotePaintMu.Lock() | |||
| 101 | + | if !ac.remotePaintPending { | |||
| 102 | + | ac.remotePaintRunning = false | |||
| 103 | + | ac.remotePaintMu.Unlock() | |||
| 104 | + | return | |||
| 105 | + | } | |||
| 106 | + | view := ac.remotePaintView | |||
| 107 | + | reset := ac.remotePaintReset | |||
| 108 | + | ac.remotePaintPending = false | |||
| 109 | + | ac.remotePaintReset = false | |||
| 110 | + | ac.remotePaintMu.Unlock() | |||
| 111 | + | token := attachmentOwnerToken(view, ac, ac.transport()) | |||
| 112 | + | if token.ac != nil { | |||
| 113 | + | d.paintRemoteView(view, ac, reset, token) | |||
| 114 | + | } | |||
| 115 | + | } | |||
| 116 | + | }() | |||
| 117 | + | } | |||
| 118 | + | ||||
| 79 | // invalidateAttachedOwner repaints an attachment through its actual owner. | 119 | // invalidateAttachedOwner repaints an attachment through its actual owner. | ||
| 80 | // Local UI overlays remain attachment-local while remote content is active, | 120 | // Local UI overlays remain attachment-local while remote content is active, | ||
| 81 | // so they must never enqueue work on the source session's coordinator after | 121 | // so they must never enqueue work on the source session's coordinator after | ||
| @@ -86,10 +126,7 @@ func (d *Daemon) invalidateAttachedOwner(ac *attachedClient, reset bool, produce | |||||
| 86 | } | 126 | } | ||
| 87 | owner := ac.currentAttachmentOwner() | 127 | owner := ac.currentAttachmentOwner() | ||
| 88 | if view, remote := owner.(*remoteView); remote { | 128 | if view, remote := owner.(*remoteView); remote { | ||
| 89 | - | token := attachmentOwnerToken(view, ac, ac.transport()) | 129 | + | d.scheduleRemoteViewPaint(view, ac, reset) |
| 90 | - | if token.ac != nil { | |||
| 91 | - | go d.paintRemoteView(view, ac, reset, token) | |||
| 92 | - | } | |||
| 93 | return | 130 | return | ||
| 94 | } | 131 | } | ||
| 95 | if sess := localSession(owner); sess != nil { | 132 | if sess := localSession(owner); sess != nil { | ||
M internal/usecase/daemon/remote_terminal.go +4 -6
| @@ -3,7 +3,9 @@ package daemon | |||
| 3 | 3 | // retireTerminalRemoteView invalidates one exact remote-link generation after | |
| 4 | 4 | // the remote daemon reports that its session detached. It removes the local | |
| 5 | 5 | // view before interrupting transports, so stale receive callbacks, warm timers, | |
| 6 | - | // and resume credentials cannot resurrect it. | |
| 6 | + | // and resume credentials cannot resurrect it. This is called synchronously | |
| 7 | + | // only from MsgDetached in runRemoteLink; it must not join link here because | |
| 8 | + | // joining the current receiver would deadlock. | |
| 7 | 9 | func (d *Daemon) retireTerminalRemoteView(link *remoteLink, reason uint8) { | |
| 8 | 10 | if d == nil || link == nil || link.view == nil { | |
| 9 | 11 | return | |
| @@ -44,9 +46,5 @@ func (d *Daemon) retireTerminalRemoteView(link *remoteLink, reason uint8) { | |||
| 44 | 46 | for _, attachment := range attachments { | |
| 45 | 47 | d.retireShutdownRemoteAttachment(view, attachment, reason) | |
| 46 | 48 | } | |
| 47 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 48 | - | link.cancel() | |
| 49 | - | if link.transport != nil { | |
| 50 | - | _ = link.transport.Close() | |
| 51 | - | } | |
| 49 | + | interruptRemoteLink(link) | |
| 52 | 50 | } | |
| @@ -3,7 +3,9 @@ package daemon | |||||
| 3 | // retireTerminalRemoteView invalidates one exact remote-link generation after | 3 | // retireTerminalRemoteView invalidates one exact remote-link generation after | ||
| 4 | // the remote daemon reports that its session detached. It removes the local | 4 | // the remote daemon reports that its session detached. It removes the local | ||
| 5 | // view before interrupting transports, so stale receive callbacks, warm timers, | 5 | // view before interrupting transports, so stale receive callbacks, warm timers, | ||
| 6 | - | // and resume credentials cannot resurrect it. | 6 | + | // and resume credentials cannot resurrect it. This is called synchronously |
| 7 | + | // only from MsgDetached in runRemoteLink; it must not join link here because | |||
| 8 | + | // joining the current receiver would deadlock. | |||
| 7 | func (d *Daemon) retireTerminalRemoteView(link *remoteLink, reason uint8) { | 9 | func (d *Daemon) retireTerminalRemoteView(link *remoteLink, reason uint8) { | ||
| 8 | if d == nil || link == nil || link.view == nil { | 10 | if d == nil || link == nil || link.view == nil { | ||
| 9 | return | 11 | return | ||
| @@ -44,9 +46,5 @@ func (d *Daemon) retireTerminalRemoteView(link *remoteLink, reason uint8) { | |||||
| 44 | for _, attachment := range attachments { | 46 | for _, attachment := range attachments { | ||
| 45 | d.retireShutdownRemoteAttachment(view, attachment, reason) | 47 | d.retireShutdownRemoteAttachment(view, attachment, reason) | ||
| 46 | } | 48 | } | ||
| 47 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 49 | + | interruptRemoteLink(link) |
| 48 | - | link.cancel() | |||
| 49 | - | if link.transport != nil { | |||
| 50 | - | _ = link.transport.Close() | |||
| 51 | - | } | |||
| 52 | } | 50 | } | ||
M internal/usecase/daemon/remote_warm.go +1 -5
| @@ -142,11 +142,7 @@ func (d *Daemon) expireRemoteViewWarm(view *remoteView, warm *remoteViewWarm) { | |||
| 142 | 142 | if link == nil { | |
| 143 | 143 | return | |
| 144 | 144 | } | |
| 145 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | |
| 146 | - | link.cancel() | |
| 147 | - | if link.transport != nil { | |
| 148 | - | _ = link.transport.Close() | |
| 149 | - | } | |
| 145 | + | interruptRemoteLink(link) | |
| 150 | 146 | joinRemoteLink(link) | |
| 151 | 147 | } | |
| 152 | 148 | ||
| @@ -142,11 +142,7 @@ func (d *Daemon) expireRemoteViewWarm(view *remoteView, warm *remoteViewWarm) { | |||||
| 142 | if link == nil { | 142 | if link == nil { | ||
| 143 | return | 143 | return | ||
| 144 | } | 144 | } | ||
| 145 | - | link.commands.FailGeneration(link.generation, errRemoteViewUnavailable) | 145 | + | interruptRemoteLink(link) |
| 146 | - | link.cancel() | |||
| 147 | - | if link.transport != nil { | |||
| 148 | - | _ = link.transport.Close() | |||
| 149 | - | } | |||
| 150 | joinRemoteLink(link) | 146 | joinRemoteLink(link) | ||
| 151 | } | 147 | } | ||
| 152 | 148 | ||||
M internal/usecase/daemon/render_pipeline.go +6 -5
| @@ -439,6 +439,9 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||
| 439 | 439 | // lease remains a local-session capability and is therefore rejected for | |
| 440 | 440 | // remote owners. | |
| 441 | 441 | func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, state *capturedRenderState, composed composedRenderFrame, batches ...*runtimeMarkBatch) bool { | |
| 442 | + | owner = normalizeAttachmentOwner(owner) | |
| 443 | + | entry := localSession(owner) | |
| 444 | + | view, remote := owner.(*remoteView) | |
| 442 | 445 | var ownedMarks runtimeMarkBatch | |
| 443 | 446 | var marks *runtimeMarkBatch | |
| 444 | 447 | if len(batches) != 0 { | |
| @@ -450,7 +453,7 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||
| 450 | 453 | // follows emitFrame's release of attachment ownership. | |
| 451 | 454 | defer marks.flush() | |
| 452 | 455 | } | |
| 453 | - | if normalizeAttachmentOwner(owner) == nil || ac == nil || state == nil { | |
| 456 | + | if owner == nil || ac == nil || state == nil { | |
| 454 | 457 | if ac != nil { | |
| 455 | 458 | ac.sendMu.Unlock() | |
| 456 | 459 | } | |
| @@ -461,7 +464,6 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||
| 461 | 464 | return false | |
| 462 | 465 | } | |
| 463 | 466 | if state.lease != nil { | |
| 464 | - | entry := localSession(owner) | |
| 465 | 467 | rc := attachmentRenderCoordinator(entry) | |
| 466 | 468 | if entry == nil || rc == nil || state.lease.attachment != ac || !rc.leaseCurrent(state.lease, true) { | |
| 467 | 469 | ac.sendMu.Unlock() | |
| @@ -485,7 +487,6 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||
| 485 | 487 | if err != nil { | |
| 486 | 488 | ac.sendMu.Unlock() | |
| 487 | 489 | d.log.Error("render draw failed", "err", err, "session", attachmentOwnerName(owner)) | |
| 488 | - | entry := localSession(owner) | |
| 489 | 490 | if entry == nil { | |
| 490 | 491 | // A remote view has no session-scoped notice history or coordinator. | |
| 491 | 492 | // Its next remote publication will retry from the retained private VT. | |
| @@ -575,9 +576,9 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||
| 575 | 576 | // admission first. Detachment freezes the gate and therefore cannot mutate | |
| 576 | 577 | // ownership until any enclosing admitted operation has also ended. | |
| 577 | 578 | if marks.attachmentEffect == nil { | |
| 578 | - | if entry := localSession(owner); entry != nil { | |
| 579 | + | if entry != nil { | |
| 579 | 580 | d.detachOnSendError(entry, ac, sendTr) | |
| 580 | - | } else if view, remote := owner.(*remoteView); remote { | |
| 581 | + | } else if remote { | |
| 581 | 582 | d.clientGoneRemote(view, attachmentOwnerToken(view, ac, sendTr), false) | |
| 582 | 583 | } | |
| 583 | 584 | } else { | |
| @@ -439,6 +439,9 @@ func (d *Daemon) emitFrame(entry *session, ac *attachedClient, state *capturedRe | |||||
| 439 | // lease remains a local-session capability and is therefore rejected for | 439 | // lease remains a local-session capability and is therefore rejected for | ||
| 440 | // remote owners. | 440 | // remote owners. | ||
| 441 | func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, state *capturedRenderState, composed composedRenderFrame, batches ...*runtimeMarkBatch) bool { | 441 | func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, state *capturedRenderState, composed composedRenderFrame, batches ...*runtimeMarkBatch) bool { | ||
| 442 | + | owner = normalizeAttachmentOwner(owner) | |||
| 443 | + | entry := localSession(owner) | |||
| 444 | + | view, remote := owner.(*remoteView) | |||
| 442 | var ownedMarks runtimeMarkBatch | 445 | var ownedMarks runtimeMarkBatch | ||
| 443 | var marks *runtimeMarkBatch | 446 | var marks *runtimeMarkBatch | ||
| 444 | if len(batches) != 0 { | 447 | if len(batches) != 0 { | ||
| @@ -450,7 +453,7 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||||
| 450 | // follows emitFrame's release of attachment ownership. | 453 | // follows emitFrame's release of attachment ownership. | ||
| 451 | defer marks.flush() | 454 | defer marks.flush() | ||
| 452 | } | 455 | } | ||
| 453 | - | if normalizeAttachmentOwner(owner) == nil || ac == nil || state == nil { | 456 | + | if owner == nil || ac == nil || state == nil { |
| 454 | if ac != nil { | 457 | if ac != nil { | ||
| 455 | ac.sendMu.Unlock() | 458 | ac.sendMu.Unlock() | ||
| 456 | } | 459 | } | ||
| @@ -461,7 +464,6 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||||
| 461 | return false | 464 | return false | ||
| 462 | } | 465 | } | ||
| 463 | if state.lease != nil { | 466 | if state.lease != nil { | ||
| 464 | - | entry := localSession(owner) | |||
| 465 | rc := attachmentRenderCoordinator(entry) | 467 | rc := attachmentRenderCoordinator(entry) | ||
| 466 | if entry == nil || rc == nil || state.lease.attachment != ac || !rc.leaseCurrent(state.lease, true) { | 468 | if entry == nil || rc == nil || state.lease.attachment != ac || !rc.leaseCurrent(state.lease, true) { | ||
| 467 | ac.sendMu.Unlock() | 469 | ac.sendMu.Unlock() | ||
| @@ -485,7 +487,6 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||||
| 485 | if err != nil { | 487 | if err != nil { | ||
| 486 | ac.sendMu.Unlock() | 488 | ac.sendMu.Unlock() | ||
| 487 | d.log.Error("render draw failed", "err", err, "session", attachmentOwnerName(owner)) | 489 | d.log.Error("render draw failed", "err", err, "session", attachmentOwnerName(owner)) | ||
| 488 | - | entry := localSession(owner) | |||
| 489 | if entry == nil { | 490 | if entry == nil { | ||
| 490 | // A remote view has no session-scoped notice history or coordinator. | 491 | // A remote view has no session-scoped notice history or coordinator. | ||
| 491 | // Its next remote publication will retry from the retained private VT. | 492 | // Its next remote publication will retry from the retained private VT. | ||
| @@ -575,9 +576,9 @@ func (d *Daemon) emitAttachmentFrame(owner attachmentOwner, ac *attachedClient, | |||||
| 575 | // admission first. Detachment freezes the gate and therefore cannot mutate | 576 | // admission first. Detachment freezes the gate and therefore cannot mutate | ||
| 576 | // ownership until any enclosing admitted operation has also ended. | 577 | // ownership until any enclosing admitted operation has also ended. | ||
| 577 | if marks.attachmentEffect == nil { | 578 | if marks.attachmentEffect == nil { | ||
| 578 | - | if entry := localSession(owner); entry != nil { | 579 | + | if entry != nil { |
| 579 | d.detachOnSendError(entry, ac, sendTr) | 580 | d.detachOnSendError(entry, ac, sendTr) | ||
| 580 | - | } else if view, remote := owner.(*remoteView); remote { | 581 | + | } else if remote { |
| 581 | d.clientGoneRemote(view, attachmentOwnerToken(view, ac, sendTr), false) | 582 | d.clientGoneRemote(view, attachmentOwnerToken(view, ac, sendTr), false) | ||
| 582 | } | 583 | } | ||
| 583 | } else { | 584 | } else { | ||
M internal/usecase/daemon/session.go +3 -8
| @@ -481,9 +481,6 @@ func (d *Daemon) createSessionAndSwitchForAttachment(token attachmentConnectionT | |||
| 481 | 481 | expectedTransport: token.transport, sourceToken: &token, action: "create-session", | |
| 482 | 482 | copySourceEnvironment: true, ready: true, | |
| 483 | 483 | createTargetLocked: func() (*session, error) { | |
| 484 | - | if source == nil { | |
| 485 | - | return nil, errAttachmentTransition | |
| 486 | - | } | |
| 487 | 484 | if d.closing { | |
| 488 | 485 | return nil, errors.New("daemon is shutting down") | |
| 489 | 486 | } | |
| @@ -1169,13 +1166,11 @@ func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *ses | |||
| 1169 | 1166 | snapshot := sessionKillParticipants{target: target} | |
| 1170 | 1167 | if admission != nil { | |
| 1171 | 1168 | token := admission.token | |
| 1172 | - | if token.localSession() == nil || token.ac == nil || token.transport.transport == nil { | |
| 1173 | - | return sessionKillParticipants{}, false | |
| 1174 | - | } | |
| 1175 | - | snapshot.source = token.localSession() | |
| 1176 | - | if snapshot.source == nil { | |
| 1169 | + | source := token.localSession() | |
| 1170 | + | if source == nil || token.ac == nil || token.transport.transport == nil { | |
| 1177 | 1171 | return sessionKillParticipants{}, false | |
| 1178 | 1172 | } | |
| 1173 | + | snapshot.source = source | |
| 1179 | 1174 | snapshot.sourceToken = &admission.token | |
| 1180 | 1175 | } | |
| 1181 | 1176 | ||
| @@ -481,9 +481,6 @@ func (d *Daemon) createSessionAndSwitchForAttachment(token attachmentConnectionT | |||||
| 481 | expectedTransport: token.transport, sourceToken: &token, action: "create-session", | 481 | expectedTransport: token.transport, sourceToken: &token, action: "create-session", | ||
| 482 | copySourceEnvironment: true, ready: true, | 482 | copySourceEnvironment: true, ready: true, | ||
| 483 | createTargetLocked: func() (*session, error) { | 483 | createTargetLocked: func() (*session, error) { | ||
| 484 | - | if source == nil { | |||
| 485 | - | return nil, errAttachmentTransition | |||
| 486 | - | } | |||
| 487 | if d.closing { | 484 | if d.closing { | ||
| 488 | return nil, errors.New("daemon is shutting down") | 485 | return nil, errors.New("daemon is shutting down") | ||
| 489 | } | 486 | } | ||
| @@ -1169,13 +1166,11 @@ func (d *Daemon) snapshotSessionKillParticipants(target *session, admission *ses | |||||
| 1169 | snapshot := sessionKillParticipants{target: target} | 1166 | snapshot := sessionKillParticipants{target: target} | ||
| 1170 | if admission != nil { | 1167 | if admission != nil { | ||
| 1171 | token := admission.token | 1168 | token := admission.token | ||
| 1172 | - | if token.localSession() == nil || token.ac == nil || token.transport.transport == nil { | 1169 | + | source := token.localSession() |
| 1173 | - | return sessionKillParticipants{}, false | 1170 | + | if source == nil || token.ac == nil || token.transport.transport == nil { |
| 1174 | - | } | |||
| 1175 | - | snapshot.source = token.localSession() | |||
| 1176 | - | if snapshot.source == nil { | |||
| 1177 | return sessionKillParticipants{}, false | 1171 | return sessionKillParticipants{}, false | ||
| 1178 | } | 1172 | } | ||
| 1173 | + | snapshot.source = source | |||
| 1179 | snapshot.sourceToken = &admission.token | 1174 | snapshot.sourceToken = &admission.token | ||
| 1180 | } | 1175 | } | ||
| 1181 | 1176 | ||||
M scripts/remote-picker-harness/main.go +31 -6
| @@ -463,6 +463,7 @@ type harnessTransport struct { | |||
| 463 | 463 | ports.Transport | |
| 464 | 464 | probe *visualProbe | |
| 465 | 465 | frames chan harnessFrame | |
| 466 | + | queued []harnessFrame | |
| 466 | 467 | stop chan struct{} | |
| 467 | 468 | closeOnce sync.Once | |
| 468 | 469 | closeErr error | |
| @@ -702,6 +703,9 @@ func awaitOutputContains(ctx context.Context, tr *harnessTransport, want string) | |||
| 702 | 703 | deadline, cancel := context.WithTimeout(ctx, 5*time.Second) | |
| 703 | 704 | defer cancel() | |
| 704 | 705 | visible := tr.probe.contains(want) | |
| 706 | + | if visible { | |
| 707 | + | return nil | |
| 708 | + | } | |
| 705 | 709 | for { | |
| 706 | 710 | frame, err := receiveWithTimeout(deadline, tr) | |
| 707 | 711 | if err != nil { | |
| @@ -739,14 +743,14 @@ func awaitRemotePickerPreview(ctx context.Context, tr *harnessTransport, marker | |||
| 739 | 743 | } | |
| 740 | 744 | text := tr.probe.text() | |
| 741 | 745 | sawLoading = sawLoading || strings.Contains(text, "loading remote preview") | |
| 746 | + | if strings.Contains(text, "remote preview unavailable") { | |
| 747 | + | return errors.New("remote preview unavailable") | |
| 748 | + | } | |
| 742 | 749 | nowMarkerVisible := strings.Contains(text, marker) | |
| 743 | 750 | if !markerVisible && nowMarkerVisible { | |
| 744 | 751 | return nil | |
| 745 | 752 | } | |
| 746 | 753 | markerVisible = nowMarkerVisible | |
| 747 | - | if strings.Contains(text, "remote preview unavailable") { | |
| 748 | - | return errors.New("remote preview unavailable") | |
| 749 | - | } | |
| 750 | 754 | } | |
| 751 | 755 | } | |
| 752 | 756 | ||
| @@ -990,13 +994,34 @@ func assertNoDirectHandoff(tr *harnessTransport) error { | |||
| 990 | 994 | if tr == nil || tr.probe == nil { | |
| 991 | 995 | return errors.New("local picker transport has no visual probe") | |
| 992 | 996 | } | |
| 993 | - | if tr.probe.unexpectedHandoffs != 0 { | |
| 994 | - | return fmt.Errorf("picker emitted %d direct MsgAttachTarget handoff frame(s)", tr.probe.unexpectedHandoffs) | |
| 997 | + | // Inspect frames already queued by the receiver, retaining non-handoff | |
| 998 | + | // frames for the next receiveWithTimeout call. | |
| 999 | + | for { | |
| 1000 | + | select { | |
| 1001 | + | case outcome := <-tr.frames: | |
| 1002 | + | if outcome.frame.Type == ports.MsgAttachTarget { | |
| 1003 | + | tr.probe.recordIncoming(outcome.frame) | |
| 1004 | + | } else { | |
| 1005 | + | tr.queued = append(tr.queued, outcome) | |
| 1006 | + | } | |
| 1007 | + | default: | |
| 1008 | + | if tr.probe.unexpectedHandoffs != 0 { | |
| 1009 | + | return fmt.Errorf("picker emitted %d direct MsgAttachTarget handoff frame(s)", tr.probe.unexpectedHandoffs) | |
| 1010 | + | } | |
| 1011 | + | return nil | |
| 1012 | + | } | |
| 995 | 1013 | } | |
| 996 | - | return nil | |
| 997 | 1014 | } | |
| 998 | 1015 | ||
| 999 | 1016 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | |
| 1017 | + | if len(tr.queued) != 0 { | |
| 1018 | + | outcome := tr.queued[0] | |
| 1019 | + | tr.queued = tr.queued[1:] | |
| 1020 | + | if outcome.err == nil { | |
| 1021 | + | tr.probe.recordIncoming(outcome.frame) | |
| 1022 | + | } | |
| 1023 | + | return outcome.frame, outcome.err | |
| 1024 | + | } | |
| 1000 | 1025 | select { | |
| 1001 | 1026 | case <-ctx.Done(): | |
| 1002 | 1027 | return ports.Frame{}, ctx.Err() | |
| @@ -463,6 +463,7 @@ type harnessTransport struct { | |||||
| 463 | ports.Transport | 463 | ports.Transport | ||
| 464 | probe *visualProbe | 464 | probe *visualProbe | ||
| 465 | frames chan harnessFrame | 465 | frames chan harnessFrame | ||
| 466 | + | queued []harnessFrame | |||
| 466 | stop chan struct{} | 467 | stop chan struct{} | ||
| 467 | closeOnce sync.Once | 468 | closeOnce sync.Once | ||
| 468 | closeErr error | 469 | closeErr error | ||
| @@ -702,6 +703,9 @@ func awaitOutputContains(ctx context.Context, tr *harnessTransport, want string) | |||||
| 702 | deadline, cancel := context.WithTimeout(ctx, 5*time.Second) | 703 | deadline, cancel := context.WithTimeout(ctx, 5*time.Second) | ||
| 703 | defer cancel() | 704 | defer cancel() | ||
| 704 | visible := tr.probe.contains(want) | 705 | visible := tr.probe.contains(want) | ||
| 706 | + | if visible { | |||
| 707 | + | return nil | |||
| 708 | + | } | |||
| 705 | for { | 709 | for { | ||
| 706 | frame, err := receiveWithTimeout(deadline, tr) | 710 | frame, err := receiveWithTimeout(deadline, tr) | ||
| 707 | if err != nil { | 711 | if err != nil { | ||
| @@ -739,14 +743,14 @@ func awaitRemotePickerPreview(ctx context.Context, tr *harnessTransport, marker | |||||
| 739 | } | 743 | } | ||
| 740 | text := tr.probe.text() | 744 | text := tr.probe.text() | ||
| 741 | sawLoading = sawLoading || strings.Contains(text, "loading remote preview") | 745 | sawLoading = sawLoading || strings.Contains(text, "loading remote preview") | ||
| 746 | + | if strings.Contains(text, "remote preview unavailable") { | |||
| 747 | + | return errors.New("remote preview unavailable") | |||
| 748 | + | } | |||
| 742 | nowMarkerVisible := strings.Contains(text, marker) | 749 | nowMarkerVisible := strings.Contains(text, marker) | ||
| 743 | if !markerVisible && nowMarkerVisible { | 750 | if !markerVisible && nowMarkerVisible { | ||
| 744 | return nil | 751 | return nil | ||
| 745 | } | 752 | } | ||
| 746 | markerVisible = nowMarkerVisible | 753 | markerVisible = nowMarkerVisible | ||
| 747 | - | if strings.Contains(text, "remote preview unavailable") { | |||
| 748 | - | return errors.New("remote preview unavailable") | |||
| 749 | - | } | |||
| 750 | } | 754 | } | ||
| 751 | } | 755 | } | ||
| 752 | 756 | ||||
| @@ -990,13 +994,34 @@ func assertNoDirectHandoff(tr *harnessTransport) error { | |||||
| 990 | if tr == nil || tr.probe == nil { | 994 | if tr == nil || tr.probe == nil { | ||
| 991 | return errors.New("local picker transport has no visual probe") | 995 | return errors.New("local picker transport has no visual probe") | ||
| 992 | } | 996 | } | ||
| 993 | - | if tr.probe.unexpectedHandoffs != 0 { | 997 | + | // Inspect frames already queued by the receiver, retaining non-handoff |
| 994 | - | return fmt.Errorf("picker emitted %d direct MsgAttachTarget handoff frame(s)", tr.probe.unexpectedHandoffs) | 998 | + | // frames for the next receiveWithTimeout call. |
| 999 | + | for { | |||
| 1000 | + | select { | |||
| 1001 | + | case outcome := <-tr.frames: | |||
| 1002 | + | if outcome.frame.Type == ports.MsgAttachTarget { | |||
| 1003 | + | tr.probe.recordIncoming(outcome.frame) | |||
| 1004 | + | } else { | |||
| 1005 | + | tr.queued = append(tr.queued, outcome) | |||
| 1006 | + | } | |||
| 1007 | + | default: | |||
| 1008 | + | if tr.probe.unexpectedHandoffs != 0 { | |||
| 1009 | + | return fmt.Errorf("picker emitted %d direct MsgAttachTarget handoff frame(s)", tr.probe.unexpectedHandoffs) | |||
| 1010 | + | } | |||
| 1011 | + | return nil | |||
| 1012 | + | } | |||
| 995 | } | 1013 | } | ||
| 996 | - | return nil | |||
| 997 | } | 1014 | } | ||
| 998 | 1015 | ||||
| 999 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | 1016 | func receiveWithTimeout(ctx context.Context, tr *harnessTransport) (ports.Frame, error) { | ||
| 1017 | + | if len(tr.queued) != 0 { | |||
| 1018 | + | outcome := tr.queued[0] | |||
| 1019 | + | tr.queued = tr.queued[1:] | |||
| 1020 | + | if outcome.err == nil { | |||
| 1021 | + | tr.probe.recordIncoming(outcome.frame) | |||
| 1022 | + | } | |||
| 1023 | + | return outcome.frame, outcome.err | |||
| 1024 | + | } | |||
| 1000 | select { | 1025 | select { | ||
| 1001 | case <-ctx.Done(): | 1026 | case <-ctx.Done(): | ||
| 1002 | return ports.Frame{}, ctx.Err() | 1027 | return ports.Frame{}, ctx.Err() | ||
M scripts/remote-picker-harness/visual_probe_test.go +4 -3
| @@ -188,6 +188,7 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||
| 188 | 188 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | |
| 189 | 189 | for _, tt := range []struct { | |
| 190 | 190 | name string | |
| 191 | + | seed *ports.Output | |
| 191 | 192 | output ports.Output | |
| 192 | 193 | wantACKs int | |
| 193 | 194 | wantResets int | |
| @@ -203,6 +204,7 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||
| 203 | 204 | }, | |
| 204 | 205 | { | |
| 205 | 206 | name: "rejected state", | |
| 207 | + | seed: &ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("seed")}, | |
| 206 | 208 | output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")}, | |
| 207 | 209 | wantResets: 1, | |
| 208 | 210 | }, | |
| @@ -210,9 +212,8 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||
| 210 | 212 | t.Run(tt.name, func(t *testing.T) { | |
| 211 | 213 | transport := &probeTestTransport{} | |
| 212 | 214 | tr := &harnessTransport{Transport: transport, probe: newVisualProbe(size)} | |
| 213 | - | if tt.name == "rejected state" { | |
| 214 | - | seed := probeTestOutput(t, ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("seed")}) | |
| 215 | - | if err := processOutputFrame(tr, seed); err != nil { | |
| 215 | + | if tt.seed != nil { | |
| 216 | + | if err := processOutputFrame(tr, probeTestOutput(t, *tt.seed)); err != nil { | |
| 216 | 217 | t.Fatal(err) | |
| 217 | 218 | } | |
| 218 | 219 | transport.sent = nil | |
| @@ -188,6 +188,7 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||||
| 188 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | 188 | size := domain.Size{Cols: probeTestCols, Rows: probeTestRows} | ||
| 189 | for _, tt := range []struct { | 189 | for _, tt := range []struct { | ||
| 190 | name string | 190 | name string | ||
| 191 | + | seed *ports.Output | |||
| 191 | output ports.Output | 192 | output ports.Output | ||
| 192 | wantACKs int | 193 | wantACKs int | ||
| 193 | wantResets int | 194 | wantResets int | ||
| @@ -203,6 +204,7 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||||
| 203 | }, | 204 | }, | ||
| 204 | { | 205 | { | ||
| 205 | name: "rejected state", | 206 | name: "rejected state", | ||
| 207 | + | seed: &ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("seed")}, | |||
| 206 | output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")}, | 208 | output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")}, | ||
| 207 | wantResets: 1, | 209 | wantResets: 1, | ||
| 208 | }, | 210 | }, | ||
| @@ -210,9 +212,8 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) { | |||||
| 210 | t.Run(tt.name, func(t *testing.T) { | 212 | t.Run(tt.name, func(t *testing.T) { | ||
| 211 | transport := &probeTestTransport{} | 213 | transport := &probeTestTransport{} | ||
| 212 | tr := &harnessTransport{Transport: transport, probe: newVisualProbe(size)} | 214 | tr := &harnessTransport{Transport: transport, probe: newVisualProbe(size)} | ||
| 213 | - | if tt.name == "rejected state" { | 215 | + | if tt.seed != nil { |
| 214 | - | seed := probeTestOutput(t, ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("seed")}) | 216 | + | if err := processOutputFrame(tr, probeTestOutput(t, *tt.seed)); err != nil { |
| 215 | - | if err := processOutputFrame(tr, seed); err != nil { | |||
| 216 | t.Fatal(err) | 217 | t.Fatal(err) | ||
| 217 | } | 218 | } | ||
| 218 | transport.sent = nil | 219 | transport.sent = nil | ||