detached e7d0da2

fix(remote): harden ownership recovery

commit
e7d0da21f95a8aaf97fe7eb0abc1c61d56521423
author
brice · · 2026-08-07 06:39 UTC
parent
e6d3912
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
21 files · +212 -84
M internal/usecase/client/client.go +1 -1
@@ -266,7 +266,7 @@ func validateAttachRequest(request AttachRequest) error {
266266 return errors.New("vev: remote attach target requires daemon-owned environment")
267267 }
268268 if request.Intent != ports.IntentAttach && request.Intent != ports.IntentResume {
269- return fmt.Errorf("vev: remote attach target requires attach or resume intent")
269+ return errors.New("vev: remote attach target requires attach or resume intent")
270270 }
271271 return nil
272272 }
@@ -266,7 +266,7 @@ func validateAttachRequest(request AttachRequest) error {
266 return errors.New("vev: remote attach target requires daemon-owned environment")266 return errors.New("vev: remote attach target requires daemon-owned environment")
267 }267 }
268 if request.Intent != ports.IntentAttach && request.Intent != ports.IntentResume {268 if request.Intent != ports.IntentAttach && request.Intent != ports.IntentResume {
269- return fmt.Errorf("vev: remote attach target requires attach or resume intent")269+ return errors.New("vev: remote attach target requires attach or resume intent")
270 }270 }
271 return nil271 return nil
272 }272 }
M internal/usecase/client/client_test.go +20 -0
@@ -950,6 +950,26 @@ func TestRunSendsRequestedProxiedRenderMode(t *testing.T) {
950950 require.Equal(t, ports.RenderModeProxiedContent, helloFromSend(t, tr).RenderMode)
951951 }
952952
953+func TestRunRejectsMismatchedProxiedWelcomeBeforeEnteringRawMode(t *testing.T) {
954+ lifecycle := domain.SessionLifecycleID{1}
955+ target := domain.RemoteSessionTarget{
956+ Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle,
957+ SessionName: "main", LiveTabID: "tab-live",
958+ }
959+ term := newRunTerminal()
960+ defer term.in.unblock()
961+ tr := &recordingTransport{recvs: []recvItem{{f: frameOf(ports.MsgWelcome, ports.MarshalWelcome(ports.Welcome{
962+ SessionID: "s1", SessionName: "main", RenderMode: ports.RenderModeFullTerminal,
963+ }))}}}
964+
965+ err := runTestClient(context.Background(), testDependencies(&sequenceDialer{trs: []ports.Transport{tr}}, term, realClock{}, nil, nil), client.AttachRequest{
966+ Intent: ports.IntentAttach, SessionName: "main", RenderMode: ports.RenderModeProxiedContent,
967+ RemoteTarget: &target, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned,
968+ })
969+ require.ErrorContains(t, err, "daemon accepted render mode")
970+ require.Zero(t, term.rawCount.Load())
971+}
972+
953973 func TestRunnerRejectsInvalidAttachRequestBeforeHello(t *testing.T) {
954974 lifecycle := domain.SessionLifecycleID{}
955975 lifecycle[0] = 1
@@ -950,6 +950,26 @@ func TestRunSendsRequestedProxiedRenderMode(t *testing.T) {
950 require.Equal(t, ports.RenderModeProxiedContent, helloFromSend(t, tr).RenderMode)950 require.Equal(t, ports.RenderModeProxiedContent, helloFromSend(t, tr).RenderMode)
951 }951 }
952 952
953+func TestRunRejectsMismatchedProxiedWelcomeBeforeEnteringRawMode(t *testing.T) {
954+ lifecycle := domain.SessionLifecycleID{1}
955+ target := domain.RemoteSessionTarget{
956+ Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle,
957+ SessionName: "main", LiveTabID: "tab-live",
958+ }
959+ term := newRunTerminal()
960+ defer term.in.unblock()
961+ tr := &recordingTransport{recvs: []recvItem{{f: frameOf(ports.MsgWelcome, ports.MarshalWelcome(ports.Welcome{
962+ SessionID: "s1", SessionName: "main", RenderMode: ports.RenderModeFullTerminal,
963+ }))}}}
964+
965+ err := runTestClient(context.Background(), testDependencies(&sequenceDialer{trs: []ports.Transport{tr}}, term, realClock{}, nil, nil), client.AttachRequest{
966+ Intent: ports.IntentAttach, SessionName: "main", RenderMode: ports.RenderModeProxiedContent,
967+ RemoteTarget: &target, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned,
968+ })
969+ require.ErrorContains(t, err, "daemon accepted render mode")
970+ require.Zero(t, term.rawCount.Load())
971+}
972+
953 func TestRunnerRejectsInvalidAttachRequestBeforeHello(t *testing.T) {973 func TestRunnerRejectsInvalidAttachRequestBeforeHello(t *testing.T) {
954 lifecycle := domain.SessionLifecycleID{}974 lifecycle := domain.SessionLifecycleID{}
955 lifecycle[0] = 1975 lifecycle[0] = 1
M internal/usecase/client/session_connection_test.go +7 -0
@@ -48,6 +48,11 @@ func TestSessionConnectionOwnsCompleteAttachRequestForLocalAndRemoteTransports(t
4848 }
4949
5050 func TestSessionConnectionAttachRequestValidation(t *testing.T) {
51+ lifecycle := domain.SessionLifecycleID{1}
52+ remoteTarget := &domain.RemoteSessionTarget{
53+ Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle,
54+ SessionName: "work", LiveTabID: "tab-1",
55+ }
5156 tests := []struct {
5257 name string
5358 request client.AttachRequest
@@ -62,6 +67,8 @@ func TestSessionConnectionAttachRequestValidation(t *testing.T) {
6267 {name: "unknown intent", request: client.AttachRequest{Intent: 99, SessionName: "work"}, wantErr: true},
6368 {name: "invalid render mode", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderMode(99)}, wantErr: true},
6469 {name: "proxied without remote target", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderModeProxiedContent}, wantErr: true},
70+ {name: "remote target with new intent", request: client.AttachRequest{Intent: ports.IntentNew, SessionName: "work", RemoteTarget: remoteTarget, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, wantErr: true},
71+ {name: "daemon-owned without remote target", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, wantErr: true},
6572 }
6673 for _, tt := range tests {
6774 t.Run(tt.name, func(t *testing.T) {
@@ -48,6 +48,11 @@ func TestSessionConnectionOwnsCompleteAttachRequestForLocalAndRemoteTransports(t
48 }48 }
49 49
50 func TestSessionConnectionAttachRequestValidation(t *testing.T) {50 func TestSessionConnectionAttachRequestValidation(t *testing.T) {
51+ lifecycle := domain.SessionLifecycleID{1}
52+ remoteTarget := &domain.RemoteSessionTarget{
53+ Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle,
54+ SessionName: "work", LiveTabID: "tab-1",
55+ }
51 tests := []struct {56 tests := []struct {
52 name string57 name string
53 request client.AttachRequest58 request client.AttachRequest
@@ -62,6 +67,8 @@ func TestSessionConnectionAttachRequestValidation(t *testing.T) {
62 {name: "unknown intent", request: client.AttachRequest{Intent: 99, SessionName: "work"}, wantErr: true},67 {name: "unknown intent", request: client.AttachRequest{Intent: 99, SessionName: "work"}, wantErr: true},
63 {name: "invalid render mode", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderMode(99)}, wantErr: true},68 {name: "invalid render mode", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderMode(99)}, wantErr: true},
64 {name: "proxied without remote target", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderModeProxiedContent}, wantErr: true},69 {name: "proxied without remote target", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RenderMode: ports.RenderModeProxiedContent}, wantErr: true},
70+ {name: "remote target with new intent", request: client.AttachRequest{Intent: ports.IntentNew, SessionName: "work", RemoteTarget: remoteTarget, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, wantErr: true},
71+ {name: "daemon-owned without remote target", request: client.AttachRequest{Intent: ports.IntentAttach, SessionName: "work", EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned}, wantErr: true},
65 }72 }
66 for _, tt := range tests {73 for _, tt := range tests {
67 t.Run(tt.name, func(t *testing.T) {74 t.Run(tt.name, func(t *testing.T) {
M internal/usecase/daemon/ansi_content_replay_phase0_test.go +5 -1
@@ -261,6 +261,8 @@ func TestPhase0PrivateReplaySuppressesHostSideEffects(t *testing.T) {
261261 data = append(data, "\x1b[5n\x1b[c\x1b[6n\x1b]10;?\x07"...)
262262 data = append(data, "\x1b]9;remote notification\x07"...)
263263 data = append(data, "\x1b]777;notify;title;body\x07"...)
264+ data = append(data, "\x1b]0;remote-title\x07"...)
265+ data = append(data, "\x1b]2;remote-title-2\x1b\\"...)
264266 data = append(data, "\x1b]9;4;2;0\x07\x1b]9;4;0\x07\a"...)
265267 data = append(data, "\x1bP1;2|dcs-payload\x1b\\"...)
266268 data = append(data, "\x1b_Ga=T,f=100;image-payload\x1b\\"...)
@@ -277,7 +279,9 @@ func TestPhase0PrivateReplaySuppressesHostSideEffects(t *testing.T) {
277279 requirePhase0ComposedContent(t, composition, expected)
278280 require.Equal(t, 0, capture.cursor.Row)
279281 require.Equal(t, 4, capture.cursor.Col)
280- require.Empty(t, screen.TerminalTitle())
282+ // The private remote VT retains its title as terminal state, but the
283+ // composed local output below contains no title-setting control sequence.
284+ require.Equal(t, "remote-title-2", screen.TerminalTitle())
281285
282286 // A control sequence is consumed before composition and cannot appear in
283287 // the locally rendered result.
@@ -261,6 +261,8 @@ func TestPhase0PrivateReplaySuppressesHostSideEffects(t *testing.T) {
261 data = append(data, "\x1b[5n\x1b[c\x1b[6n\x1b]10;?\x07"...)261 data = append(data, "\x1b[5n\x1b[c\x1b[6n\x1b]10;?\x07"...)
262 data = append(data, "\x1b]9;remote notification\x07"...)262 data = append(data, "\x1b]9;remote notification\x07"...)
263 data = append(data, "\x1b]777;notify;title;body\x07"...)263 data = append(data, "\x1b]777;notify;title;body\x07"...)
264+ data = append(data, "\x1b]0;remote-title\x07"...)
265+ data = append(data, "\x1b]2;remote-title-2\x1b\\"...)
264 data = append(data, "\x1b]9;4;2;0\x07\x1b]9;4;0\x07\a"...)266 data = append(data, "\x1b]9;4;2;0\x07\x1b]9;4;0\x07\a"...)
265 data = append(data, "\x1bP1;2|dcs-payload\x1b\\"...)267 data = append(data, "\x1bP1;2|dcs-payload\x1b\\"...)
266 data = append(data, "\x1b_Ga=T,f=100;image-payload\x1b\\"...)268 data = append(data, "\x1b_Ga=T,f=100;image-payload\x1b\\"...)
@@ -277,7 +279,9 @@ func TestPhase0PrivateReplaySuppressesHostSideEffects(t *testing.T) {
277 requirePhase0ComposedContent(t, composition, expected)279 requirePhase0ComposedContent(t, composition, expected)
278 require.Equal(t, 0, capture.cursor.Row)280 require.Equal(t, 0, capture.cursor.Row)
279 require.Equal(t, 4, capture.cursor.Col)281 require.Equal(t, 4, capture.cursor.Col)
280- require.Empty(t, screen.TerminalTitle())282+ // The private remote VT retains its title as terminal state, but the
283+ // composed local output below contains no title-setting control sequence.
284+ require.Equal(t, "remote-title-2", screen.TerminalTitle())
281 285
282 // A control sequence is consumed before composition and cannot appear in286 // A control sequence is consumed before composition and cannot appear in
283 // the locally rendered result.287 // the locally rendered result.
M internal/usecase/daemon/attachment_connection.go +1 -0
@@ -26,6 +26,7 @@ func attachmentToken(entry *session, ac *attachedClient, tr ports.Transport) att
2626 }
2727
2828 func attachmentOwnerToken(owner attachmentOwner, ac *attachedClient, tr ports.Transport) attachmentConnectionToken {
29+ owner = normalizeAttachmentOwner(owner)
2930 if owner == nil || ac == nil || tr == nil {
3031 return attachmentConnectionToken{}
3132 }
@@ -26,6 +26,7 @@ func attachmentToken(entry *session, ac *attachedClient, tr ports.Transport) att
26 }26 }
27 27
28 func attachmentOwnerToken(owner attachmentOwner, ac *attachedClient, tr ports.Transport) attachmentConnectionToken {28 func attachmentOwnerToken(owner attachmentOwner, ac *attachedClient, tr ports.Transport) attachmentConnectionToken {
29+ owner = normalizeAttachmentOwner(owner)
29 if owner == nil || ac == nil || tr == nil {30 if owner == nil || ac == nil || tr == nil {
30 return attachmentConnectionToken{}31 return attachmentConnectionToken{}
31 }32 }
A internal/usecase/daemon/attachment_connection_test.go +35 -0
@@ -0,0 +1,35 @@
1+package daemon
2+
3+import (
4+ "testing"
5+
6+ "github.com/stretchr/testify/require"
7+)
8+
9+func TestAttachmentOwnerTokenRejectsTypedNilRemoteView(t *testing.T) {
10+ var view *remoteView
11+ ac := &attachedClient{tr: &closeTrackingTransport{}}
12+
13+ token := attachmentOwnerToken(view, ac, ac.transport())
14+
15+ require.Nil(t, token.owner)
16+ require.Nil(t, token.ac)
17+}
18+
19+func TestTypedNilRemoteOwnerCannotEnterBackNavigation(t *testing.T) {
20+ var view *remoteView
21+ ac := &attachedClient{}
22+
23+ require.NotPanics(t, func() {
24+ require.NoError(t, (&Daemon{}).backSessionForAttachment(attachmentConnectionToken{owner: view, ac: ac}))
25+ })
26+}
27+
28+func TestRecordPreviousOwnerIgnoresTypedNilRemoteView(t *testing.T) {
29+ var view *remoteView
30+ ac := &attachedClient{}
31+
32+ ac.recordPreviousOwner(view)
33+
34+ require.Nil(t, ac.previousOwner.Get())
35+}
@@ -0,0 +1,35 @@
1+package daemon
2+
3+import (
4+ "testing"
5+
6+ "github.com/stretchr/testify/require"
7+)
8+
9+func TestAttachmentOwnerTokenRejectsTypedNilRemoteView(t *testing.T) {
10+ var view *remoteView
11+ ac := &attachedClient{tr: &closeTrackingTransport{}}
12+
13+ token := attachmentOwnerToken(view, ac, ac.transport())
14+
15+ require.Nil(t, token.owner)
16+ require.Nil(t, token.ac)
17+}
18+
19+func TestTypedNilRemoteOwnerCannotEnterBackNavigation(t *testing.T) {
20+ var view *remoteView
21+ ac := &attachedClient{}
22+
23+ require.NotPanics(t, func() {
24+ require.NoError(t, (&Daemon{}).backSessionForAttachment(attachmentConnectionToken{owner: view, ac: ac}))
25+ })
26+}
27+
28+func TestRecordPreviousOwnerIgnoresTypedNilRemoteView(t *testing.T) {
29+ var view *remoteView
30+ ac := &attachedClient{}
31+
32+ ac.recordPreviousOwner(view)
33+
34+ require.Nil(t, ac.previousOwner.Get())
35+}
M internal/usecase/daemon/attachment_owner_test.go +3 -0
@@ -558,6 +558,9 @@ func TestRemoteViewRegistryUsesDaemonLocalIDAndExactRemoteLifecycleKey(t *testin
558558 require.NotZero(t, first.id)
559559 require.Same(t, first, d.remoteViewByKeyLocked(key))
560560 require.Error(t, d.registerRemoteViewLocked(second), "a lifecycle key cannot alias another local view")
561+ require.Zero(t, second.id, "a rejected registration must not consume a local view ID")
562+ require.Len(t, d.remoteViews, 1)
563+ require.Len(t, d.remoteViewsByKey, 1)
561564 require.True(t, d.unregisterRemoteViewLocked(first))
562565 require.Nil(t, d.remoteViewByKeyLocked(key))
563566 d.mu.Unlock()
@@ -558,6 +558,9 @@ func TestRemoteViewRegistryUsesDaemonLocalIDAndExactRemoteLifecycleKey(t *testin
558 require.NotZero(t, first.id)558 require.NotZero(t, first.id)
559 require.Same(t, first, d.remoteViewByKeyLocked(key))559 require.Same(t, first, d.remoteViewByKeyLocked(key))
560 require.Error(t, d.registerRemoteViewLocked(second), "a lifecycle key cannot alias another local view")560 require.Error(t, d.registerRemoteViewLocked(second), "a lifecycle key cannot alias another local view")
561+ require.Zero(t, second.id, "a rejected registration must not consume a local view ID")
562+ require.Len(t, d.remoteViews, 1)
563+ require.Len(t, d.remoteViewsByKey, 1)
561 require.True(t, d.unregisterRemoteViewLocked(first))564 require.True(t, d.unregisterRemoteViewLocked(first))
562 require.Nil(t, d.remoteViewByKeyLocked(key))565 require.Nil(t, d.remoteViewByKeyLocked(key))
563 d.mu.Unlock()566 d.mu.Unlock()
M internal/usecase/daemon/notify.go +2 -7
@@ -494,13 +494,8 @@ func (d *Daemon) repaintForNotice(ac *attachedClient) {
494494 if ac == nil {
495495 return
496496 }
497- if view, remote := ac.currentAttachmentOwner().(*remoteView); remote {
498- tr := ac.transport()
499- token := attachmentOwnerToken(view, ac, tr)
500- if token.ac == nil {
501- return
502- }
503- d.paintRemoteView(view, ac, false, token)
497+ if _, remote := normalizeAttachmentOwner(ac.currentAttachmentOwner()).(*remoteView); remote {
498+ d.invalidateAttachedOwner(ac, false, "notify.go")
504499 return
505500 }
506501 sess := ac.currentSession()
@@ -494,13 +494,8 @@ func (d *Daemon) repaintForNotice(ac *attachedClient) {
494 if ac == nil {494 if ac == nil {
495 return495 return
496 }496 }
497- if view, remote := ac.currentAttachmentOwner().(*remoteView); remote {497+ if _, remote := normalizeAttachmentOwner(ac.currentAttachmentOwner()).(*remoteView); remote {
498- tr := ac.transport()498+ d.invalidateAttachedOwner(ac, false, "notify.go")
499- token := attachmentOwnerToken(view, ac, tr)
500- if token.ac == nil {
501- return
502- }
503- d.paintRemoteView(view, ac, false, token)
504 return499 return
505 }500 }
506 sess := ac.currentSession()501 sess := ac.currentSession()
M internal/usecase/daemon/proxied_handshake_test.go +10 -0
@@ -98,6 +98,16 @@ func TestProxiedCompositionUsesAttachmentContentWindowAndFloatingSurface(t *test
9898 require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, composed.damage)
9999 }
100100
101+func TestProxiedPaintFlushesRuntimeMarks(t *testing.T) {
102+ observer := &daemonRuntimeObserver{}
103+ d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY())
104+ d.runtimeObserver = observer
105+ ac.renderMode = ports.RenderModeProxiedContent
106+
107+ require.Equal(t, paintEmitted, d.paintProxiedContent(sess, ac, true, nil))
108+ require.NotEmpty(t, observer.marks, "proxied rendering must flush observer marks after releasing sendMu")
109+}
110+
101111 func TestProxiedMetadataFallsBackFromStaleAttachmentTab(t *testing.T) {
102112 _, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY())
103113 sess.mu.Lock()
@@ -98,6 +98,16 @@ func TestProxiedCompositionUsesAttachmentContentWindowAndFloatingSurface(t *test
98 require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, composed.damage)98 require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, composed.damage)
99 }99 }
100 100
101+func TestProxiedPaintFlushesRuntimeMarks(t *testing.T) {
102+ observer := &daemonRuntimeObserver{}
103+ d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY())
104+ d.runtimeObserver = observer
105+ ac.renderMode = ports.RenderModeProxiedContent
106+
107+ require.Equal(t, paintEmitted, d.paintProxiedContent(sess, ac, true, nil))
108+ require.NotEmpty(t, observer.marks, "proxied rendering must flush observer marks after releasing sendMu")
109+}
110+
101 func TestProxiedMetadataFallsBackFromStaleAttachmentTab(t *testing.T) {111 func TestProxiedMetadataFallsBackFromStaleAttachmentTab(t *testing.T) {
102 _, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY())112 _, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY())
103 sess.mu.Lock()113 sess.mu.Lock()
M internal/usecase/daemon/proxied_render.go +1 -0
@@ -14,6 +14,7 @@ func (d *Daemon) paintProxiedContent(entry *session, ac *attachedClient, reset b
1414 return paintRejected
1515 }
1616 marks := d.newRuntimeMarkBatch()
17+ defer marks.flush()
1718 if lease != nil {
1819 token := attachmentToken(entry, ac, ac.transport())
1920 token.lease = lease
@@ -14,6 +14,7 @@ func (d *Daemon) paintProxiedContent(entry *session, ac *attachedClient, reset b
14 return paintRejected14 return paintRejected
15 }15 }
16 marks := d.newRuntimeMarkBatch()16 marks := d.newRuntimeMarkBatch()
17+ defer marks.flush()
17 if lease != nil {18 if lease != nil {
18 token := attachmentToken(entry, ac, ac.transport())19 token := attachmentToken(entry, ac, ac.transport())
19 token.lease = lease20 token.lease = lease
M internal/usecase/daemon/remote_attachment_transition_picker_test.go +3 -0
@@ -16,6 +16,9 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) {
1616 }}
1717 link := &remoteLink{view: view, generation: 1, transport: newRemoteLinkTestTransport()}
1818 view.link, view.linkGeneration = link, 1
19+ view.mu.Lock()
20+ require.False(t, remoteViewLinkReusableLocked(view), "the fixture must start with an unusable link")
21+ view.mu.Unlock()
1922 d.mu.Lock()
2023 require.NoError(t, d.registerRemoteViewLocked(view))
2124 d.mu.Unlock()
@@ -16,6 +16,9 @@ func TestPickerTransitionRejectsInactiveRemoteLink(t *testing.T) {
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, 118 view.link, view.linkGeneration = link, 1
19+ view.mu.Lock()
20+ require.False(t, remoteViewLinkReusableLocked(view), "the fixture must start with an unusable link")
21+ view.mu.Unlock()
19 d.mu.Lock()22 d.mu.Lock()
20 require.NoError(t, d.registerRemoteViewLocked(view))23 require.NoError(t, d.registerRemoteViewLocked(view))
21 d.mu.Unlock()24 d.mu.Unlock()
M internal/usecase/daemon/remote_link.go +1 -3
@@ -326,9 +326,7 @@ func stopAndJoinRemoteLink(link *remoteLink) {
326326 if link.transport != nil {
327327 _ = link.transport.Close()
328328 }
329- if link.done != nil {
330- <-link.done
331- }
329+ joinRemoteLink(link)
332330 }
333331
334332 // constructRemoteView builds an unexposed candidate. Its caller owns a
@@ -326,9 +326,7 @@ func stopAndJoinRemoteLink(link *remoteLink) {
326 if link.transport != nil {326 if link.transport != nil {
327 _ = link.transport.Close()327 _ = link.transport.Close()
328 }328 }
329- if link.done != nil {329+ joinRemoteLink(link)
330- <-link.done
331- }
332 }330 }
333 331
334 // constructRemoteView builds an unexposed candidate. Its caller owns a332 // constructRemoteView builds an unexposed candidate. Its caller owns a
M internal/usecase/daemon/remote_link_test.go +16 -0
@@ -203,6 +203,22 @@ func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame {
203203 return ports.Frame{Type: ports.MsgSessionMeta, Payload: payload}
204204 }
205205
206+func TestStopAndJoinRemoteLinkDoesNotWaitForUnstartedWorker(t *testing.T) {
207+ transport := newRemoteLinkTestTransport()
208+ link := &remoteLink{generation: 1, transport: transport, done: make(chan struct{})}
209+ done := make(chan struct{})
210+ go func() {
211+ stopAndJoinRemoteLink(link)
212+ close(done)
213+ }()
214+ awaitTestCompletion(t, done, "stopping an unstarted remote link blocked")
215+ select {
216+ case <-transport.closed:
217+ default:
218+ t.Fatal("stopping an unstarted remote link did not close its transport")
219+ }
220+}
221+
206222 func TestRemoteLinkDetachedPropagatesItsExactReason(t *testing.T) {
207223 d, view, link, _ := newRemoteMetadataLinkFixture(t)
208224 attachment, sends := attachRemoteMetadataClient(t, view)
@@ -203,6 +203,22 @@ func remoteMetadataFrame(t *testing.T, metadata ports.SessionMeta) ports.Frame {
203 return ports.Frame{Type: ports.MsgSessionMeta, Payload: payload}203 return ports.Frame{Type: ports.MsgSessionMeta, Payload: payload}
204 }204 }
205 205
206+func TestStopAndJoinRemoteLinkDoesNotWaitForUnstartedWorker(t *testing.T) {
207+ transport := newRemoteLinkTestTransport()
208+ link := &remoteLink{generation: 1, transport: transport, done: make(chan struct{})}
209+ done := make(chan struct{})
210+ go func() {
211+ stopAndJoinRemoteLink(link)
212+ close(done)
213+ }()
214+ awaitTestCompletion(t, done, "stopping an unstarted remote link blocked")
215+ select {
216+ case <-transport.closed:
217+ default:
218+ t.Fatal("stopping an unstarted remote link did not close its transport")
219+ }
220+}
221+
206 func TestRemoteLinkDetachedPropagatesItsExactReason(t *testing.T) {222 func TestRemoteLinkDetachedPropagatesItsExactReason(t *testing.T) {
207 d, view, link, _ := newRemoteMetadataLinkFixture(t)223 d, view, link, _ := newRemoteMetadataLinkFixture(t)
208 attachment, sends := attachRemoteMetadataClient(t, view)224 attachment, sends := attachRemoteMetadataClient(t, view)
M internal/usecase/daemon/remote_picker_selection.go +1 -0
@@ -83,6 +83,7 @@ func (d *Daemon) completeRemotePickerSelection(selection *remotePickerSelection)
8383 if selection == nil || selection.token.ac == nil {
8484 return
8585 }
86+ defer selection.cancel()
8687 view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot())
8788 if err != nil {
8889 d.failRemotePickerSelection(selection, err)
@@ -83,6 +83,7 @@ func (d *Daemon) completeRemotePickerSelection(selection *remotePickerSelection)
83 if selection == nil || selection.token.ac == nil {83 if selection == nil || selection.token.ac == nil {
84 return84 return
85 }85 }
86+ defer selection.cancel()
86 view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot())87 view, err := d.openRemoteView(selection.ctx, selection.target, selection.token.ac.sizeSnapshot())
87 if err != nil {88 if err != nil {
88 d.failRemotePickerSelection(selection, err)89 d.failRemotePickerSelection(selection, err)
M internal/usecase/daemon/remote_picker_test.go +2 -0
@@ -1276,7 +1276,9 @@ func TestRemoteRowsRequireFreshRichIdentityForPickerActivation(t *testing.T) {
12761276 {status: remoteHostCached},
12771277 {status: remoteHostRefreshing},
12781278 {status: remoteHostFresh, want: true},
1279+ {status: remoteHostStale},
12791280 {status: remoteHostUnreachable},
1281+ {status: remoteHostMalformed},
12801282 {status: remoteHostVersionMismatch},
12811283 } {
12821284 view := remotePickerView(key, session, test.status, time.Unix(1, 0))
@@ -1276,7 +1276,9 @@ func TestRemoteRowsRequireFreshRichIdentityForPickerActivation(t *testing.T) {
1276 {status: remoteHostCached},1276 {status: remoteHostCached},
1277 {status: remoteHostRefreshing},1277 {status: remoteHostRefreshing},
1278 {status: remoteHostFresh, want: true},1278 {status: remoteHostFresh, want: true},
1279+ {status: remoteHostStale},
1279 {status: remoteHostUnreachable},1280 {status: remoteHostUnreachable},
1281+ {status: remoteHostMalformed},
1280 {status: remoteHostVersionMismatch},1282 {status: remoteHostVersionMismatch},
1281 } {1283 } {
1282 view := remotePickerView(key, session, test.status, time.Unix(1, 0))1284 view := remotePickerView(key, session, test.status, time.Unix(1, 0))
M internal/usecase/daemon/remote_view_tabs_test.go +32 -0
@@ -40,6 +40,38 @@ func TestSendRemoteViewCommandAllowsOnlyRelativeTabCommands(t *testing.T) {
4040 }
4141 }
4242
43+func TestSendRemoteViewCommandReportsRemoteFailureAndCancellation(t *testing.T) {
44+ t.Run("remote failure", func(t *testing.T) {
45+ d, _, link, transport := newRemoteMetadataLinkFixture(t)
46+ link.commands = NewCommandRequestTracker()
47+ done := make(chan error, 1)
48+ go func() {
49+ done <- d.sendRemoteViewCommand(context.Background(), link, link.generation, "next-tab")
50+ }()
51+ frame := awaitFrame(t, transport.sent, ports.MsgCommand)
52+ request, err := ports.UnmarshalCommandRequest(frame.Payload)
53+ require.NoError(t, err)
54+ resultPayload := ports.MarshalCommandResult(ports.CommandResult{RequestID: request.RequestID, Code: 12, Text: "rejected"})
55+ require.NoError(t, d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgCommandResult, Payload: resultPayload}))
56+ require.ErrorContains(t, awaitTestValue(t, done, "failed remote tab command did not complete"), "command \"next-tab\" failed (12): rejected")
57+ require.Zero(t, link.commands.PendingCount())
58+ })
59+
60+ t.Run("canceled context", func(t *testing.T) {
61+ d, _, link, transport := newRemoteMetadataLinkFixture(t)
62+ link.commands = NewCommandRequestTracker()
63+ ctx, cancel := context.WithCancel(context.Background())
64+ done := make(chan error, 1)
65+ go func() {
66+ done <- d.sendRemoteViewCommand(ctx, link, link.generation, "previous-tab")
67+ }()
68+ awaitFrame(t, transport.sent, ports.MsgCommand)
69+ cancel()
70+ require.ErrorIs(t, awaitTestValue(t, done, "canceled remote tab command did not complete"), context.Canceled)
71+ require.Zero(t, link.commands.PendingCount())
72+ })
73+}
74+
4375 func TestSendRemoteViewCommandRejectsOperationsOutsideRelativeTabAllowlist(t *testing.T) {
4476 for _, slug := range []string{
4577 "new-session",
@@ -40,6 +40,38 @@ func TestSendRemoteViewCommandAllowsOnlyRelativeTabCommands(t *testing.T) {
40 }40 }
41 }41 }
42 42
43+func TestSendRemoteViewCommandReportsRemoteFailureAndCancellation(t *testing.T) {
44+ t.Run("remote failure", func(t *testing.T) {
45+ d, _, link, transport := newRemoteMetadataLinkFixture(t)
46+ link.commands = NewCommandRequestTracker()
47+ done := make(chan error, 1)
48+ go func() {
49+ done <- d.sendRemoteViewCommand(context.Background(), link, link.generation, "next-tab")
50+ }()
51+ frame := awaitFrame(t, transport.sent, ports.MsgCommand)
52+ request, err := ports.UnmarshalCommandRequest(frame.Payload)
53+ require.NoError(t, err)
54+ resultPayload := ports.MarshalCommandResult(ports.CommandResult{RequestID: request.RequestID, Code: 12, Text: "rejected"})
55+ require.NoError(t, d.handleRemoteLinkFrame(link, ports.Frame{Type: ports.MsgCommandResult, Payload: resultPayload}))
56+ require.ErrorContains(t, awaitTestValue(t, done, "failed remote tab command did not complete"), "command \"next-tab\" failed (12): rejected")
57+ require.Zero(t, link.commands.PendingCount())
58+ })
59+
60+ t.Run("canceled context", func(t *testing.T) {
61+ d, _, link, transport := newRemoteMetadataLinkFixture(t)
62+ link.commands = NewCommandRequestTracker()
63+ ctx, cancel := context.WithCancel(context.Background())
64+ done := make(chan error, 1)
65+ go func() {
66+ done <- d.sendRemoteViewCommand(ctx, link, link.generation, "previous-tab")
67+ }()
68+ awaitFrame(t, transport.sent, ports.MsgCommand)
69+ cancel()
70+ require.ErrorIs(t, awaitTestValue(t, done, "canceled remote tab command did not complete"), context.Canceled)
71+ require.Zero(t, link.commands.PendingCount())
72+ })
73+}
74+
43 func TestSendRemoteViewCommandRejectsOperationsOutsideRelativeTabAllowlist(t *testing.T) {75 func TestSendRemoteViewCommandRejectsOperationsOutsideRelativeTabAllowlist(t *testing.T) {
44 for _, slug := range []string{76 for _, slug := range []string{
45 "new-session",77 "new-session",
M internal/usecase/daemon/session_back.go +2 -1
@@ -13,7 +13,7 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error
1313 if d == nil || token.ac == nil {
1414 return nil
1515 }
16- if view, remote := token.owner.(*remoteView); remote {
16+ if view, remote := normalizeAttachmentOwner(token.owner).(*remoteView); remote {
1717 previous := token.ac.previousOwner.Get()
1818 target := localSession(previous)
1919 if target == nil {
@@ -103,6 +103,7 @@ func (ac *attachedClient) clearPreviousOwnerIf(target attachmentOwner) {
103103 // committed. Keeping this after the commit makes failed/displaced switches
104104 // leave the toggle pair intact.
105105 func (ac *attachedClient) recordPreviousOwner(origin attachmentOwner) {
106+ origin = normalizeAttachmentOwner(origin)
106107 if ac != nil && origin != nil {
107108 ac.previousOwner.Set(origin)
108109 }
@@ -13,7 +13,7 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error
13 if d == nil || token.ac == nil {13 if d == nil || token.ac == nil {
14 return nil14 return nil
15 }15 }
16- if view, remote := token.owner.(*remoteView); remote {16+ if view, remote := normalizeAttachmentOwner(token.owner).(*remoteView); remote {
17 previous := token.ac.previousOwner.Get()17 previous := token.ac.previousOwner.Get()
18 target := localSession(previous)18 target := localSession(previous)
19 if target == nil {19 if target == nil {
@@ -103,6 +103,7 @@ func (ac *attachedClient) clearPreviousOwnerIf(target attachmentOwner) {
103 // committed. Keeping this after the commit makes failed/displaced switches103 // committed. Keeping this after the commit makes failed/displaced switches
104 // leave the toggle pair intact.104 // leave the toggle pair intact.
105 func (ac *attachedClient) recordPreviousOwner(origin attachmentOwner) {105 func (ac *attachedClient) recordPreviousOwner(origin attachmentOwner) {
106+ origin = normalizeAttachmentOwner(origin)
106 if ac != nil && origin != nil {107 if ac != nil && origin != nil {
107 ac.previousOwner.Set(origin)108 ac.previousOwner.Set(origin)
108 }109 }
M internal/usecase/daemon/wire.go +3 -1
@@ -71,7 +71,9 @@ func frameSessionMeta(s *session, ac *attachedClient, revision uint64) (ports.Fr
7171 meta.Tabs = append(meta.Tabs, ports.SessionTabMeta{ID: id, Name: tabDisplayName(tab, i), Attention: tab.attention})
7272 activePresent = activePresent || active == id
7373 }
74- if !activePresent && len(meta.Tabs) != 0 {
74+ if len(meta.Tabs) == 0 {
75+ active = ""
76+ } else if !activePresent {
7577 active = meta.Tabs[0].ID
7678 }
7779 meta.ActiveTabID = active
@@ -71,7 +71,9 @@ func frameSessionMeta(s *session, ac *attachedClient, revision uint64) (ports.Fr
71 meta.Tabs = append(meta.Tabs, ports.SessionTabMeta{ID: id, Name: tabDisplayName(tab, i), Attention: tab.attention})71 meta.Tabs = append(meta.Tabs, ports.SessionTabMeta{ID: id, Name: tabDisplayName(tab, i), Attention: tab.attention})
72 activePresent = activePresent || active == id72 activePresent = activePresent || active == id
73 }73 }
74- if !activePresent && len(meta.Tabs) != 0 {74+ if len(meta.Tabs) == 0 {
75+ active = ""
76+ } else if !activePresent {
75 active = meta.Tabs[0].ID77 active = meta.Tabs[0].ID
76 }78 }
77 meta.ActiveTabID = active79 meta.ActiveTabID = active
M scripts/remote-picker-harness/main.go +4 -4
@@ -163,7 +163,7 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports
163163 if err != nil {
164164 return fmt.Errorf("open remote picker source: %w", err)
165165 }
166- remote.probe.configure("remote-picker-source", nil)
166+ remote.probe.configure("remote-picker-source")
167167 ownsRemoteSession := false
168168 defer func() {
169169 _ = remote.Close()
@@ -244,7 +244,7 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports
244244 // This is the one client-equivalent connection for the proof. Remote
245245 // selection must replace its daemon-side owner without sending an endpoint
246246 // or MsgAttachTarget to this transport.
247- local.probe.configure("local-picker", nil)
247+ local.probe.configure("local-picker")
248248 defer func() { _ = local.Close() }()
249249 if err := attachAndCheck(ctx, local, ports.Hello{
250250 Intent: ports.IntentNew, Name: localSession, Env: localEnvironment(), Cwd: "/tmp/local-cwd",
@@ -519,7 +519,7 @@ func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target s
519519 return nil, err
520520 }
521521 h := newHarnessTransport(transport)
522- h.probe.configure(string(mode), nil)
522+ h.probe.configure(string(mode))
523523 if artifact := harnessArtifactFromContext(ctx); artifact != nil {
524524 artifact.registerProbe(h.probe)
525525 }
@@ -625,7 +625,7 @@ func openLocalTransport(ctx context.Context) (*harnessTransport, error) {
625625 return nil, err
626626 }
627627 h := newHarnessTransport(transport)
628- h.probe.configure("local", nil)
628+ h.probe.configure("local")
629629 if artifact := harnessArtifactFromContext(ctx); artifact != nil {
630630 artifact.registerProbe(h.probe)
631631 }
@@ -163,7 +163,7 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports
163 if err != nil {163 if err != nil {
164 return fmt.Errorf("open remote picker source: %w", err)164 return fmt.Errorf("open remote picker source: %w", err)
165 }165 }
166- remote.probe.configure("remote-picker-source", nil)166+ remote.probe.configure("remote-picker-source")
167 ownsRemoteSession := false167 ownsRemoteSession := false
168 defer func() {168 defer func() {
169 _ = remote.Close()169 _ = remote.Close()
@@ -244,7 +244,7 @@ func runLocalPickerUnitedPhase(ctx context.Context, target string, catalog ports
244 // This is the one client-equivalent connection for the proof. Remote244 // This is the one client-equivalent connection for the proof. Remote
245 // selection must replace its daemon-side owner without sending an endpoint245 // selection must replace its daemon-side owner without sending an endpoint
246 // or MsgAttachTarget to this transport.246 // or MsgAttachTarget to this transport.
247- local.probe.configure("local-picker", nil)247+ local.probe.configure("local-picker")
248 defer func() { _ = local.Close() }()248 defer func() { _ = local.Close() }()
249 if err := attachAndCheck(ctx, local, ports.Hello{249 if err := attachAndCheck(ctx, local, ports.Hello{
250 Intent: ports.IntentNew, Name: localSession, Env: localEnvironment(), Cwd: "/tmp/local-cwd",250 Intent: ports.IntentNew, Name: localSession, Env: localEnvironment(), Cwd: "/tmp/local-cwd",
@@ -519,7 +519,7 @@ func openTransport(ctx context.Context, mode ports.RemoteTransportMode, target s
519 return nil, err519 return nil, err
520 }520 }
521 h := newHarnessTransport(transport)521 h := newHarnessTransport(transport)
522- h.probe.configure(string(mode), nil)522+ h.probe.configure(string(mode))
523 if artifact := harnessArtifactFromContext(ctx); artifact != nil {523 if artifact := harnessArtifactFromContext(ctx); artifact != nil {
524 artifact.registerProbe(h.probe)524 artifact.registerProbe(h.probe)
525 }525 }
@@ -625,7 +625,7 @@ func openLocalTransport(ctx context.Context) (*harnessTransport, error) {
625 return nil, err625 return nil, err
626 }626 }
627 h := newHarnessTransport(transport)627 h := newHarnessTransport(transport)
628- h.probe.configure("local", nil)628+ h.probe.configure("local")
629 if artifact := harnessArtifactFromContext(ctx); artifact != nil {629 if artifact := harnessArtifactFromContext(ctx); artifact != nil {
630 artifact.registerProbe(h.probe)630 artifact.registerProbe(h.probe)
631 }631 }
M scripts/remote-picker-harness/visual_probe.go +11 -46
@@ -20,7 +20,6 @@ const (
2020
2121 maxProbeEvents = 128
2222 maxProbeCheckpoints = 16
23- maxProbeTraces = 128
2423 maxArtifactBytes = 64 << 10
2524
2625 probeEventOutput = "output"
@@ -45,6 +44,9 @@ func (s visualOutputState) next(output ports.Output) (visualOutputState, bool) {
4544 }
4645 if !s.initialized {
4746 if output.New == 0 {
47+ if output.Base != 0 || output.Full {
48+ return visualOutputState{}, false
49+ }
4850 return s, true
4951 }
5052 if output.Base != 0 || !output.Full {
@@ -91,20 +93,6 @@ type probeEvent struct {
9193 DataBytes int
9294 }
9395
94-type probeTrace struct {
95- events []*probeEvent
96-}
97-
98-func (t *probeTrace) record(event *probeEvent) {
99- if t == nil || event == nil {
100- return
101- }
102- if len(t.events) == maxProbeTraces {
103- t.events = t.events[1:]
104- }
105- t.events = append(t.events, event)
106-}
107-
10896 type visualCheckpoint struct {
10997 event *probeEvent
11098 state visualOutputState
@@ -129,7 +117,6 @@ type visualProbe struct {
129117
130118 events []*probeEvent
131119 checkpoints []visualCheckpoint
132- trace *probeTrace
133120 nextEvent uint64
134121
135122 outputFrames int
@@ -164,12 +151,11 @@ func newVisualProbe(size domain.Size) *visualProbe {
164151 return &visualProbe{screen: vt.NewScreen(width, height)}
165152 }
166153
167-func (p *visualProbe) configure(label string, trace *probeTrace) {
154+func (p *visualProbe) configure(label string) {
168155 if p == nil {
169156 return
170157 }
171158 p.label = label
172- p.trace = trace
173159 }
174160
175161 func (p *visualProbe) record(event probeEvent) *probeEvent {
@@ -184,9 +170,6 @@ func (p *visualProbe) record(event probeEvent) *probeEvent {
184170 p.events = p.events[1:]
185171 }
186172 p.events = append(p.events, owned)
187- if p.trace != nil {
188- p.trace.record(owned)
189- }
190173 return owned
191174 }
192175
@@ -327,6 +310,9 @@ func assertUnifiedPickerRows(rows []string, localSession, remoteSession string)
327310 }
328311
329312 func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error {
313+ if probe == nil {
314+ return errors.New("remote view probe was not created")
315+ }
330316 lines := visualScreenLines(probe.screen)
331317 if len(lines) < 3 {
332318 return errors.New("remote view did not expose a content region")
@@ -344,7 +330,6 @@ func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error {
344330 type harnessArtifact struct {
345331 dir string
346332 probes []*visualProbe
347- traces []*probeTrace
348333 }
349334
350335 func newHarnessArtifact(dir string) *harnessArtifact {
@@ -361,13 +346,6 @@ func (a *harnessArtifact) registerProbe(probe *visualProbe) {
361346 a.probes = append(a.probes, probe)
362347 }
363348
364-func (a *harnessArtifact) registerTrace(trace *probeTrace) {
365- if a == nil || trace == nil {
366- return
367- }
368- a.traces = append(a.traces, trace)
369-}
370-
371349 type artifactEvent struct {
372350 Sequence uint64 `json:"sequence"`
373351 Transport string `json:"transport"`
@@ -404,10 +382,9 @@ type artifactProbe struct {
404382 }
405383
406384 type artifactDocument struct {
407- Version int `json:"version"`
408- Passed bool `json:"passed"`
409- Probes []artifactProbe `json:"probes"`
410- Traces [][]artifactEvent `json:"traces,omitempty"`
385+ Version int `json:"version"`
386+ Passed bool `json:"passed"`
387+ Probes []artifactProbe `json:"probes"`
411388 }
412389
413390 func (a *harnessArtifact) document(passed bool) artifactDocument {
@@ -443,18 +420,6 @@ func (a *harnessArtifact) document(passed bool) artifactDocument {
443420 }
444421 document.Probes = append(document.Probes, item)
445422 }
446- for _, trace := range a.traces {
447- if trace == nil {
448- continue
449- }
450- var events []artifactEvent
451- for _, event := range trace.events {
452- if event != nil {
453- events = append(events, artifactEventFrom(event))
454- }
455- }
456- document.Traces = append(document.Traces, events)
457- }
458423 return document
459424 }
460425
@@ -494,7 +459,7 @@ func (a *harnessArtifact) write(passed bool) error {
494459 return fmt.Errorf("create harness artifact: %w", err)
495460 }
496461 tmpName := tmp.Name()
497- defer os.Remove(tmpName)
462+ defer func() { _ = os.Remove(tmpName) }()
498463 if err := tmp.Chmod(0o600); err != nil {
499464 _ = tmp.Close()
500465 return fmt.Errorf("protect harness artifact: %w", err)
@@ -20,7 +20,6 @@ const (
20 20
21 maxProbeEvents = 12821 maxProbeEvents = 128
22 maxProbeCheckpoints = 1622 maxProbeCheckpoints = 16
23- maxProbeTraces = 128
24 maxArtifactBytes = 64 << 1023 maxArtifactBytes = 64 << 10
25 24
26 probeEventOutput = "output"25 probeEventOutput = "output"
@@ -45,6 +44,9 @@ func (s visualOutputState) next(output ports.Output) (visualOutputState, bool) {
45 }44 }
46 if !s.initialized {45 if !s.initialized {
47 if output.New == 0 {46 if output.New == 0 {
47+ if output.Base != 0 || output.Full {
48+ return visualOutputState{}, false
49+ }
48 return s, true50 return s, true
49 }51 }
50 if output.Base != 0 || !output.Full {52 if output.Base != 0 || !output.Full {
@@ -91,20 +93,6 @@ type probeEvent struct {
91 DataBytes int93 DataBytes int
92 }94 }
93 95
94-type probeTrace struct {
95- events []*probeEvent
96-}
97-
98-func (t *probeTrace) record(event *probeEvent) {
99- if t == nil || event == nil {
100- return
101- }
102- if len(t.events) == maxProbeTraces {
103- t.events = t.events[1:]
104- }
105- t.events = append(t.events, event)
106-}
107-
108 type visualCheckpoint struct {96 type visualCheckpoint struct {
109 event *probeEvent97 event *probeEvent
110 state visualOutputState98 state visualOutputState
@@ -129,7 +117,6 @@ type visualProbe struct {
129 117
130 events []*probeEvent118 events []*probeEvent
131 checkpoints []visualCheckpoint119 checkpoints []visualCheckpoint
132- trace *probeTrace
133 nextEvent uint64120 nextEvent uint64
134 121
135 outputFrames int122 outputFrames int
@@ -164,12 +151,11 @@ func newVisualProbe(size domain.Size) *visualProbe {
164 return &visualProbe{screen: vt.NewScreen(width, height)}151 return &visualProbe{screen: vt.NewScreen(width, height)}
165 }152 }
166 153
167-func (p *visualProbe) configure(label string, trace *probeTrace) {154+func (p *visualProbe) configure(label string) {
168 if p == nil {155 if p == nil {
169 return156 return
170 }157 }
171 p.label = label158 p.label = label
172- p.trace = trace
173 }159 }
174 160
175 func (p *visualProbe) record(event probeEvent) *probeEvent {161 func (p *visualProbe) record(event probeEvent) *probeEvent {
@@ -184,9 +170,6 @@ func (p *visualProbe) record(event probeEvent) *probeEvent {
184 p.events = p.events[1:]170 p.events = p.events[1:]
185 }171 }
186 p.events = append(p.events, owned)172 p.events = append(p.events, owned)
187- if p.trace != nil {
188- p.trace.record(owned)
189- }
190 return owned173 return owned
191 }174 }
192 175
@@ -327,6 +310,9 @@ func assertUnifiedPickerRows(rows []string, localSession, remoteSession string)
327 }310 }
328 311
329 func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error {312 func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error {
313+ if probe == nil {
314+ return errors.New("remote view probe was not created")
315+ }
330 lines := visualScreenLines(probe.screen)316 lines := visualScreenLines(probe.screen)
331 if len(lines) < 3 {317 if len(lines) < 3 {
332 return errors.New("remote view did not expose a content region")318 return errors.New("remote view did not expose a content region")
@@ -344,7 +330,6 @@ func assertNoRemoteChrome(probe *visualProbe, remoteSession string) error {
344 type harnessArtifact struct {330 type harnessArtifact struct {
345 dir string331 dir string
346 probes []*visualProbe332 probes []*visualProbe
347- traces []*probeTrace
348 }333 }
349 334
350 func newHarnessArtifact(dir string) *harnessArtifact {335 func newHarnessArtifact(dir string) *harnessArtifact {
@@ -361,13 +346,6 @@ func (a *harnessArtifact) registerProbe(probe *visualProbe) {
361 a.probes = append(a.probes, probe)346 a.probes = append(a.probes, probe)
362 }347 }
363 348
364-func (a *harnessArtifact) registerTrace(trace *probeTrace) {
365- if a == nil || trace == nil {
366- return
367- }
368- a.traces = append(a.traces, trace)
369-}
370-
371 type artifactEvent struct {349 type artifactEvent struct {
372 Sequence uint64 `json:"sequence"`350 Sequence uint64 `json:"sequence"`
373 Transport string `json:"transport"`351 Transport string `json:"transport"`
@@ -404,10 +382,9 @@ type artifactProbe struct {
404 }382 }
405 383
406 type artifactDocument struct {384 type artifactDocument struct {
407- Version int `json:"version"`385+ Version int `json:"version"`
408- Passed bool `json:"passed"`386+ Passed bool `json:"passed"`
409- Probes []artifactProbe `json:"probes"`387+ Probes []artifactProbe `json:"probes"`
410- Traces [][]artifactEvent `json:"traces,omitempty"`
411 }388 }
412 389
413 func (a *harnessArtifact) document(passed bool) artifactDocument {390 func (a *harnessArtifact) document(passed bool) artifactDocument {
@@ -443,18 +420,6 @@ func (a *harnessArtifact) document(passed bool) artifactDocument {
443 }420 }
444 document.Probes = append(document.Probes, item)421 document.Probes = append(document.Probes, item)
445 }422 }
446- for _, trace := range a.traces {
447- if trace == nil {
448- continue
449- }
450- var events []artifactEvent
451- for _, event := range trace.events {
452- if event != nil {
453- events = append(events, artifactEventFrom(event))
454- }
455- }
456- document.Traces = append(document.Traces, events)
457- }
458 return document423 return document
459 }424 }
460 425
@@ -494,7 +459,7 @@ func (a *harnessArtifact) write(passed bool) error {
494 return fmt.Errorf("create harness artifact: %w", err)459 return fmt.Errorf("create harness artifact: %w", err)
495 }460 }
496 tmpName := tmp.Name()461 tmpName := tmp.Name()
497- defer os.Remove(tmpName)462+ defer func() { _ = os.Remove(tmpName) }()
498 if err := tmp.Chmod(0o600); err != nil {463 if err := tmp.Chmod(0o600); err != nil {
499 _ = tmp.Close()464 _ = tmp.Close()
500 return fmt.Errorf("protect harness artifact: %w", err)465 return fmt.Errorf("protect harness artifact: %w", err)
M scripts/remote-picker-harness/visual_probe_test.go +52 -20
@@ -9,6 +9,7 @@ import (
99
1010 "github.com/bnema/vev/internal/domain"
1111 "github.com/bnema/vev/internal/ports"
12+ "github.com/stretchr/testify/require"
1213 )
1314
1415 const (
@@ -126,6 +127,23 @@ func TestVisualProbeRejectsOutputWithoutMutationOrAck(t *testing.T) {
126127 }
127128 }
128129
130+func TestVisualProbeRejectsInvalidUninitializedSideEffects(t *testing.T) {
131+ size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
132+ for _, output := range []ports.Output{
133+ {Epoch: 1, Base: 1, Size: size, Data: []byte("invalid base")},
134+ {Epoch: 1, Full: true, Size: size, Data: []byte("invalid full")},
135+ } {
136+ probe := newVisualProbe(size)
137+ result := probe.apply(output)
138+ if result.Accepted || result.StateBearing || result.Ack != (ports.Ack{}) {
139+ t.Fatalf("result = %+v, want rejected uninitialized side effect", result)
140+ }
141+ require.Len(t, probe.events, 1)
142+ require.False(t, probe.events[0].Accepted)
143+ require.Empty(t, probe.checkpoints)
144+ }
145+}
146+
129147 func TestVisualProbeSideEffectsApplyWithoutAck(t *testing.T) {
130148 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
131149 for _, tt := range []struct {
@@ -169,24 +187,24 @@ func TestVisualProbeSideEffectsApplyWithoutAck(t *testing.T) {
169187 func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) {
170188 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
171189 for _, tt := range []struct {
172- name string
173- output ports.Output
174- wantAck bool
175- wantReset bool
190+ name string
191+ output ports.Output
192+ wantACKs int
193+ wantResets int
176194 }{
177195 {
178- name: "accepted full",
179- output: ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("full")},
180- wantAck: true,
196+ name: "accepted full",
197+ output: ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("full")},
198+ wantACKs: 1,
181199 },
182200 {
183201 name: "accepted side effect",
184202 output: ports.Output{Epoch: 1, Size: size, Data: []byte("side effect")},
185203 },
186204 {
187- name: "rejected state",
188- output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")},
189- wantReset: true,
205+ name: "rejected state",
206+ output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")},
207+ wantResets: 1,
190208 },
191209 } {
192210 t.Run(tt.name, func(t *testing.T) {
@@ -211,11 +229,11 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) {
211229 resetCount++
212230 }
213231 }
214- if (ackCount > 0) != tt.wantAck {
215- t.Fatalf("ACK count = %d, want ACK = %t", ackCount, tt.wantAck)
232+ if ackCount != tt.wantACKs {
233+ t.Fatalf("ACK count = %d, want %d", ackCount, tt.wantACKs)
216234 }
217- if (resetCount > 0) != tt.wantReset {
218- t.Fatalf("reset count = %d, want reset = %t", resetCount, tt.wantReset)
235+ if resetCount != tt.wantResets {
236+ t.Fatalf("reset count = %d, want %d", resetCount, tt.wantResets)
219237 }
220238 })
221239 }
@@ -244,6 +262,7 @@ func TestVisualProbeRecordsUnexpectedAttachTarget(t *testing.T) {
244262 }
245263
246264 func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T) {
265+ require.Error(t, assertNoRemoteChrome(nil, "picker"))
247266 for _, test := range []struct {
248267 name string
249268 content string
@@ -266,12 +285,19 @@ func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T)
266285 func TestHarnessArtifactIsBoundedAndContainsOnlyMetadata(t *testing.T) {
267286 dir := t.TempDir()
268287 artifact := newHarnessArtifact(dir)
269- probe := newVisualProbe(domain.Size{Cols: probeTestCols, Rows: probeTestRows})
270- probe.configure("local-picker", nil)
288+ size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
289+ probe := newVisualProbe(size)
290+ probe.configure("local-picker")
271291 artifact.registerProbe(probe)
272292 secret := "not-for-artifact"
273- if result := probe.apply(ports.Output{Epoch: 1, New: 1, Full: true, Size: domain.Size{Cols: probeTestCols, Rows: probeTestRows}, Data: []byte(secret)}); !result.Accepted {
274- t.Fatal("probe output was rejected")
293+ for state := uint64(1); state <= maxProbeEvents+1; state++ {
294+ output := ports.Output{Epoch: 1, Base: state - 1, New: state, Size: size, Data: []byte(secret)}
295+ if state == 1 {
296+ output.Full = true
297+ }
298+ if result := probe.apply(output); !result.Accepted {
299+ t.Fatalf("probe output %d was rejected", state)
300+ }
275301 }
276302 if err := artifact.write(true); err != nil {
277303 t.Fatal(err)
@@ -291,7 +317,13 @@ func TestHarnessArtifactIsBoundedAndContainsOnlyMetadata(t *testing.T) {
291317 if err := json.Unmarshal(data, &document); err != nil {
292318 t.Fatalf("artifact JSON: %v", err)
293319 }
294- if !document.Passed || len(document.Probes) != 1 || len(document.Probes[0].Checkpoints) != 1 {
295- t.Fatalf("artifact document = %+v, want one passed probe checkpoint", document)
320+ if !document.Passed || len(document.Probes) != 1 {
321+ t.Fatalf("artifact document = %+v, want one passed probe", document)
322+ }
323+ if len(document.Probes[0].Events) != maxProbeEvents {
324+ t.Fatalf("artifact events = %d, want %d", len(document.Probes[0].Events), maxProbeEvents)
325+ }
326+ if len(document.Probes[0].Checkpoints) != maxProbeCheckpoints {
327+ t.Fatalf("artifact checkpoints = %d, want %d", len(document.Probes[0].Checkpoints), maxProbeCheckpoints)
296328 }
297329 }
@@ -9,6 +9,7 @@ import (
9 9
10 "github.com/bnema/vev/internal/domain"10 "github.com/bnema/vev/internal/domain"
11 "github.com/bnema/vev/internal/ports"11 "github.com/bnema/vev/internal/ports"
12+ "github.com/stretchr/testify/require"
12 )13 )
13 14
14 const (15 const (
@@ -126,6 +127,23 @@ func TestVisualProbeRejectsOutputWithoutMutationOrAck(t *testing.T) {
126 }127 }
127 }128 }
128 129
130+func TestVisualProbeRejectsInvalidUninitializedSideEffects(t *testing.T) {
131+ size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
132+ for _, output := range []ports.Output{
133+ {Epoch: 1, Base: 1, Size: size, Data: []byte("invalid base")},
134+ {Epoch: 1, Full: true, Size: size, Data: []byte("invalid full")},
135+ } {
136+ probe := newVisualProbe(size)
137+ result := probe.apply(output)
138+ if result.Accepted || result.StateBearing || result.Ack != (ports.Ack{}) {
139+ t.Fatalf("result = %+v, want rejected uninitialized side effect", result)
140+ }
141+ require.Len(t, probe.events, 1)
142+ require.False(t, probe.events[0].Accepted)
143+ require.Empty(t, probe.checkpoints)
144+ }
145+}
146+
129 func TestVisualProbeSideEffectsApplyWithoutAck(t *testing.T) {147 func TestVisualProbeSideEffectsApplyWithoutAck(t *testing.T) {
130 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}148 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
131 for _, tt := range []struct {149 for _, tt := range []struct {
@@ -169,24 +187,24 @@ func TestVisualProbeSideEffectsApplyWithoutAck(t *testing.T) {
169 func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) {187 func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) {
170 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}188 size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
171 for _, tt := range []struct {189 for _, tt := range []struct {
172- name string190+ name string
173- output ports.Output191+ output ports.Output
174- wantAck bool192+ wantACKs int
175- wantReset bool193+ wantResets int
176 }{194 }{
177 {195 {
178- name: "accepted full",196+ name: "accepted full",
179- output: ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("full")},197+ output: ports.Output{Epoch: 1, New: 1, Full: true, ViewRevision: 4, Size: size, Data: []byte("full")},
180- wantAck: true,198+ wantACKs: 1,
181 },199 },
182 {200 {
183 name: "accepted side effect",201 name: "accepted side effect",
184 output: ports.Output{Epoch: 1, Size: size, Data: []byte("side effect")},202 output: ports.Output{Epoch: 1, Size: size, Data: []byte("side effect")},
185 },203 },
186 {204 {
187- name: "rejected state",205+ name: "rejected state",
188- output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")},206+ output: ports.Output{Epoch: 1, New: 3, Full: true, ViewRevision: 4, Size: size, Data: []byte("gap")},
189- wantReset: true,207+ wantResets: 1,
190 },208 },
191 } {209 } {
192 t.Run(tt.name, func(t *testing.T) {210 t.Run(tt.name, func(t *testing.T) {
@@ -211,11 +229,11 @@ func TestProcessOutputFrameACKsOnlyAcceptedStateBearingFrames(t *testing.T) {
211 resetCount++229 resetCount++
212 }230 }
213 }231 }
214- if (ackCount > 0) != tt.wantAck {232+ if ackCount != tt.wantACKs {
215- t.Fatalf("ACK count = %d, want ACK = %t", ackCount, tt.wantAck)233+ t.Fatalf("ACK count = %d, want %d", ackCount, tt.wantACKs)
216 }234 }
217- if (resetCount > 0) != tt.wantReset {235+ if resetCount != tt.wantResets {
218- t.Fatalf("reset count = %d, want reset = %t", resetCount, tt.wantReset)236+ t.Fatalf("reset count = %d, want %d", resetCount, tt.wantResets)
219 }237 }
220 })238 })
221 }239 }
@@ -244,6 +262,7 @@ func TestVisualProbeRecordsUnexpectedAttachTarget(t *testing.T) {
244 }262 }
245 263
246 func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T) {264 func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T) {
265+ require.Error(t, assertNoRemoteChrome(nil, "picker"))
247 for _, test := range []struct {266 for _, test := range []struct {
248 name string267 name string
249 content string268 content string
@@ -266,12 +285,19 @@ func TestAssertNoRemoteChromeIgnoresLocalBarsButRejectsContentLeak(t *testing.T)
266 func TestHarnessArtifactIsBoundedAndContainsOnlyMetadata(t *testing.T) {285 func TestHarnessArtifactIsBoundedAndContainsOnlyMetadata(t *testing.T) {
267 dir := t.TempDir()286 dir := t.TempDir()
268 artifact := newHarnessArtifact(dir)287 artifact := newHarnessArtifact(dir)
269- probe := newVisualProbe(domain.Size{Cols: probeTestCols, Rows: probeTestRows})288+ size := domain.Size{Cols: probeTestCols, Rows: probeTestRows}
270- probe.configure("local-picker", nil)289+ probe := newVisualProbe(size)
290+ probe.configure("local-picker")
271 artifact.registerProbe(probe)291 artifact.registerProbe(probe)
272 secret := "not-for-artifact"292 secret := "not-for-artifact"
273- if result := probe.apply(ports.Output{Epoch: 1, New: 1, Full: true, Size: domain.Size{Cols: probeTestCols, Rows: probeTestRows}, Data: []byte(secret)}); !result.Accepted {293+ for state := uint64(1); state <= maxProbeEvents+1; state++ {
274- t.Fatal("probe output was rejected")294+ output := ports.Output{Epoch: 1, Base: state - 1, New: state, Size: size, Data: []byte(secret)}
295+ if state == 1 {
296+ output.Full = true
297+ }
298+ if result := probe.apply(output); !result.Accepted {
299+ t.Fatalf("probe output %d was rejected", state)
300+ }
275 }301 }
276 if err := artifact.write(true); err != nil {302 if err := artifact.write(true); err != nil {
277 t.Fatal(err)303 t.Fatal(err)
@@ -291,7 +317,13 @@ func TestHarnessArtifactIsBoundedAndContainsOnlyMetadata(t *testing.T) {
291 if err := json.Unmarshal(data, &document); err != nil {317 if err := json.Unmarshal(data, &document); err != nil {
292 t.Fatalf("artifact JSON: %v", err)318 t.Fatalf("artifact JSON: %v", err)
293 }319 }
294- if !document.Passed || len(document.Probes) != 1 || len(document.Probes[0].Checkpoints) != 1 {320+ if !document.Passed || len(document.Probes) != 1 {
295- t.Fatalf("artifact document = %+v, want one passed probe checkpoint", document)321+ t.Fatalf("artifact document = %+v, want one passed probe", document)
322+ }
323+ if len(document.Probes[0].Events) != maxProbeEvents {
324+ t.Fatalf("artifact events = %d, want %d", len(document.Probes[0].Events), maxProbeEvents)
325+ }
326+ if len(document.Probes[0].Checkpoints) != maxProbeCheckpoints {
327+ t.Fatalf("artifact checkpoints = %d, want %d", len(document.Probes[0].Checkpoints), maxProbeCheckpoints)
296 }328 }
297 }329 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close