fix(remote): address direct ownership review findings
- commit
- b9f056533f8f73ef4e3af741b6709cbe73242694
- author
- brice · · 2026-08-09 10:53 UTC
- parent
- b0018c5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
25 files · +225 -110
M internal/ports/frame_test.go +29 -8
| @@ -52,15 +52,36 @@ func TestMsgTypeUnique(t *testing.T) { | |||
| 52 | 52 | } | |
| 53 | 53 | ||
| 54 | 54 | func TestNavigationActionFrame(t *testing.T) { | |
| 55 | - | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(NavigationOpenHomePicker)} | |
| 56 | - | if frame.Type != MsgNavigationAction { | |
| 57 | - | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | |
| 55 | + | tests := []struct { | |
| 56 | + | name string | |
| 57 | + | action NavigationAction | |
| 58 | + | bytes []byte | |
| 59 | + | }{ | |
| 60 | + | {name: "open home picker", action: NavigationOpenHomePicker, bytes: []byte{1}}, | |
| 61 | + | {name: "back", action: NavigationBack, bytes: []byte{2}}, | |
| 58 | 62 | } | |
| 59 | - | action, err := UnmarshalNavigationAction(frame.Payload) | |
| 60 | - | if err != nil { | |
| 61 | - | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | |
| 63 | + | for _, tt := range tests { | |
| 64 | + | t.Run(tt.name, func(t *testing.T) { | |
| 65 | + | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(tt.action)} | |
| 66 | + | if frame.Type != MsgNavigationAction { | |
| 67 | + | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | |
| 68 | + | } | |
| 69 | + | if got := frame.Payload; string(got) != string(tt.bytes) { | |
| 70 | + | t.Fatalf("payload = %x, want %x", got, tt.bytes) | |
| 71 | + | } | |
| 72 | + | action, err := UnmarshalNavigationAction(frame.Payload) | |
| 73 | + | if err != nil { | |
| 74 | + | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | |
| 75 | + | } | |
| 76 | + | if action != tt.action { | |
| 77 | + | t.Fatalf("action = %d, want %d", action, tt.action) | |
| 78 | + | } | |
| 79 | + | }) | |
| 62 | 80 | } | |
| 63 | - | if action != NavigationOpenHomePicker { | |
| 64 | - | t.Fatalf("action = %d, want %d", action, NavigationOpenHomePicker) | |
| 81 | + | ||
| 82 | + | for _, payload := range [][]byte{nil, []byte{1, 0, 0}, []byte{2, 0, 0}} { | |
| 83 | + | if _, err := UnmarshalNavigationAction(payload); err == nil { | |
| 84 | + | t.Fatalf("UnmarshalNavigationAction(%x) succeeded, want error", payload) | |
| 85 | + | } | |
| 65 | 86 | } | |
| 66 | 87 | } | |
| @@ -52,15 +52,36 @@ func TestMsgTypeUnique(t *testing.T) { | |||||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
| 54 | func TestNavigationActionFrame(t *testing.T) { | 54 | func TestNavigationActionFrame(t *testing.T) { | ||
| 55 | - | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(NavigationOpenHomePicker)} | 55 | + | tests := []struct { |
| 56 | - | if frame.Type != MsgNavigationAction { | 56 | + | name string |
| 57 | - | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | 57 | + | action NavigationAction |
| 58 | + | bytes []byte | |||
| 59 | + | }{ | |||
| 60 | + | {name: "open home picker", action: NavigationOpenHomePicker, bytes: []byte{1}}, | |||
| 61 | + | {name: "back", action: NavigationBack, bytes: []byte{2}}, | |||
| 58 | } | 62 | } | ||
| 59 | - | action, err := UnmarshalNavigationAction(frame.Payload) | 63 | + | for _, tt := range tests { |
| 60 | - | if err != nil { | 64 | + | t.Run(tt.name, func(t *testing.T) { |
| 61 | - | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | 65 | + | frame := Frame{Type: MsgNavigationAction, Payload: MarshalNavigationAction(tt.action)} |
| 66 | + | if frame.Type != MsgNavigationAction { | |||
| 67 | + | t.Fatalf("frame type = %d, want %d", frame.Type, MsgNavigationAction) | |||
| 68 | + | } | |||
| 69 | + | if got := frame.Payload; string(got) != string(tt.bytes) { | |||
| 70 | + | t.Fatalf("payload = %x, want %x", got, tt.bytes) | |||
| 71 | + | } | |||
| 72 | + | action, err := UnmarshalNavigationAction(frame.Payload) | |||
| 73 | + | if err != nil { | |||
| 74 | + | t.Fatalf("UnmarshalNavigationAction() error = %v", err) | |||
| 75 | + | } | |||
| 76 | + | if action != tt.action { | |||
| 77 | + | t.Fatalf("action = %d, want %d", action, tt.action) | |||
| 78 | + | } | |||
| 79 | + | }) | |||
| 62 | } | 80 | } | ||
| 63 | - | if action != NavigationOpenHomePicker { | 81 | + | |
| 64 | - | t.Fatalf("action = %d, want %d", action, NavigationOpenHomePicker) | 82 | + | for _, payload := range [][]byte{nil, []byte{1, 0, 0}, []byte{2, 0, 0}} { |
| 83 | + | if _, err := UnmarshalNavigationAction(payload); err == nil { | |||
| 84 | + | t.Fatalf("UnmarshalNavigationAction(%x) succeeded, want error", payload) | |||
| 85 | + | } | |||
| 65 | } | 86 | } | ||
| 66 | } | 87 | } | ||
M internal/ports/navigation_test.go +6 -0
| @@ -43,6 +43,10 @@ func TestNavigationActionWireTable(t *testing.T) { | |||
| 43 | 43 | ||
| 44 | 44 | func TestHelloNavigationValidationTable(t *testing.T) { | |
| 45 | 45 | base := Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}} | |
| 46 | + | remoteTarget := &domain.RemoteSessionTarget{ | |
| 47 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: domain.SessionLifecycleID{1}, | |
| 48 | + | SessionName: "work", LiveTabID: "tab-1", | |
| 49 | + | } | |
| 46 | 50 | tests := []struct { | |
| 47 | 51 | name string | |
| 48 | 52 | hello Hello | |
| @@ -54,8 +58,10 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||
| 54 | 58 | {name: "ordinary", hello: base, valid: true}, | |
| 55 | 59 | {name: "resume route", hello: Hello{Version: ProtocolVersion, Intent: IntentResume, Size: domain.Size{Cols: 80, Rows: 24}}, valid: true}, | |
| 56 | 60 | {name: "daemon-owned home picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: true, capability: NavigationCapabilityHomePicker}, | |
| 61 | + | {name: "remote-target home picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityHomePicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: true, capability: NavigationCapabilityHomePicker}, | |
| 57 | 62 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | |
| 58 | 63 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | |
| 64 | + | {name: "remote-target back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | |
| 59 | 65 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001a02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "0201"}, | |
| 60 | 66 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | |
| 61 | 67 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | |
| @@ -43,6 +43,10 @@ func TestNavigationActionWireTable(t *testing.T) { | |||||
| 43 | 43 | ||||
| 44 | func TestHelloNavigationValidationTable(t *testing.T) { | 44 | func TestHelloNavigationValidationTable(t *testing.T) { | ||
| 45 | base := Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}} | 45 | base := Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}} | ||
| 46 | + | remoteTarget := &domain.RemoteSessionTarget{ | |||
| 47 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: domain.SessionLifecycleID{1}, | |||
| 48 | + | SessionName: "work", LiveTabID: "tab-1", | |||
| 49 | + | } | |||
| 46 | tests := []struct { | 50 | tests := []struct { | ||
| 47 | name string | 51 | name string | ||
| 48 | hello Hello | 52 | hello Hello | ||
| @@ -54,8 +58,10 @@ func TestHelloNavigationValidationTable(t *testing.T) { | |||||
| 54 | {name: "ordinary", hello: base, valid: true}, | 58 | {name: "ordinary", hello: base, valid: true}, | ||
| 55 | {name: "resume route", hello: Hello{Version: ProtocolVersion, Intent: IntentResume, Size: domain.Size{Cols: 80, Rows: 24}}, valid: true}, | 59 | {name: "resume route", hello: Hello{Version: ProtocolVersion, Intent: IntentResume, Size: domain.Size{Cols: 80, Rows: 24}}, valid: true}, | ||
| 56 | {name: "daemon-owned home picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: true, capability: NavigationCapabilityHomePicker}, | 60 | {name: "daemon-owned home picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: true, capability: NavigationCapabilityHomePicker}, | ||
| 61 | + | {name: "remote-target home picker", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityHomePicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: true, capability: NavigationCapabilityHomePicker}, | |||
| 57 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | 62 | {name: "client-owned home picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityHomePicker}, valid: false}, | ||
| 58 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | 63 | {name: "daemon-owned back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | ||
| 64 | + | {name: "remote-target back picker rejected", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Name: "work", Size: domain.Size{Cols: 80, Rows: 24}, RemoteTarget: remoteTarget, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker, EnvironmentPolicy: EnvironmentPolicyDaemonOwned}, valid: false}, | |||
| 59 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001a02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "0201"}, | 65 | {name: "startup picker requires back", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: true, capability: NavigationCapabilityBack, overlay: StartupOverlaySessionPicker, wantPayload: "001a02" + "00000000000000000000000000000000" + "0000000000000000" + "0000" + "00500018" + "0000" + "0000" + "00" + "00" + "00000000" + "00" + "00" + "0201"}, | ||
| 60 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | 66 | {name: "new rejects navigation", hello: Hello{Version: ProtocolVersion, Intent: IntentNew, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: NavigationCapabilityBack, StartupOverlay: StartupOverlaySessionPicker}, valid: false}, | ||
| 61 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | 67 | {name: "unknown capability", hello: Hello{Version: ProtocolVersion, Intent: IntentAttach, Size: domain.Size{Cols: 80, Rows: 24}, NavigationCapabilities: 4}, valid: false}, | ||
M internal/ports/remote_target_wire_test.go +7 -4
| @@ -89,6 +89,7 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||
| 89 | 89 | marshalGood func() []byte | |
| 90 | 90 | unmarshal func([]byte) error | |
| 91 | 91 | policyOffset int | |
| 92 | + | wantErr error | |
| 92 | 93 | }{ | |
| 93 | 94 | { | |
| 94 | 95 | name: "hello", | |
| @@ -115,6 +116,7 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||
| 115 | 116 | }, | |
| 116 | 117 | unmarshal: func(payload []byte) error { _, err := UnmarshalHello(payload); return err }, | |
| 117 | 118 | policyOffset: 3, | |
| 119 | + | wantErr: ErrInvalidHello, | |
| 118 | 120 | }, | |
| 119 | 121 | { | |
| 120 | 122 | name: "attach target", | |
| @@ -138,13 +140,14 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||
| 138 | 140 | }, | |
| 139 | 141 | unmarshal: func(payload []byte) error { _, err := UnmarshalAttachTarget(payload); return err }, | |
| 140 | 142 | policyOffset: 1, | |
| 143 | + | wantErr: ErrInvalidAttachTarget, | |
| 141 | 144 | }, | |
| 142 | 145 | } | |
| 143 | 146 | ||
| 144 | 147 | for _, tt := range tests { | |
| 145 | 148 | t.Run(tt.name, func(t *testing.T) { | |
| 146 | - | if err := tt.validate(); err == nil { | |
| 147 | - | t.Fatal("direct validation accepted client-owned policy for remote target") | |
| 149 | + | if err := tt.validate(); !errors.Is(err, tt.wantErr) { | |
| 150 | + | t.Fatalf("direct validation error = %v, want %v", err, tt.wantErr) | |
| 148 | 151 | } | |
| 149 | 152 | if payload := tt.marshalBad(); payload != nil { | |
| 150 | 153 | t.Fatalf("marshal accepted client-owned policy: %x", payload) | |
| @@ -163,8 +166,8 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||
| 163 | 166 | t.Fatal("trailing garbage unexpectedly decoded") | |
| 164 | 167 | } | |
| 165 | 168 | payload[len(payload)-tt.policyOffset] = byte(EnvironmentPolicyClientOwned) | |
| 166 | - | if err := tt.unmarshal(payload); err == nil { | |
| 167 | - | t.Fatal("unmarshal accepted client-owned policy for remote target") | |
| 169 | + | if err := tt.unmarshal(payload); !errors.Is(err, tt.wantErr) { | |
| 170 | + | t.Fatalf("mutated policy error = %v, want %v", err, tt.wantErr) | |
| 168 | 171 | } | |
| 169 | 172 | }) | |
| 170 | 173 | } | |
| @@ -89,6 +89,7 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||||
| 89 | marshalGood func() []byte | 89 | marshalGood func() []byte | ||
| 90 | unmarshal func([]byte) error | 90 | unmarshal func([]byte) error | ||
| 91 | policyOffset int | 91 | policyOffset int | ||
| 92 | + | wantErr error | |||
| 92 | }{ | 93 | }{ | ||
| 93 | { | 94 | { | ||
| 94 | name: "hello", | 95 | name: "hello", | ||
| @@ -115,6 +116,7 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||||
| 115 | }, | 116 | }, | ||
| 116 | unmarshal: func(payload []byte) error { _, err := UnmarshalHello(payload); return err }, | 117 | unmarshal: func(payload []byte) error { _, err := UnmarshalHello(payload); return err }, | ||
| 117 | policyOffset: 3, | 118 | policyOffset: 3, | ||
| 119 | + | wantErr: ErrInvalidHello, | |||
| 118 | }, | 120 | }, | ||
| 119 | { | 121 | { | ||
| 120 | name: "attach target", | 122 | name: "attach target", | ||
| @@ -138,13 +140,14 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||||
| 138 | }, | 140 | }, | ||
| 139 | unmarshal: func(payload []byte) error { _, err := UnmarshalAttachTarget(payload); return err }, | 141 | unmarshal: func(payload []byte) error { _, err := UnmarshalAttachTarget(payload); return err }, | ||
| 140 | policyOffset: 1, | 142 | policyOffset: 1, | ||
| 143 | + | wantErr: ErrInvalidAttachTarget, | |||
| 141 | }, | 144 | }, | ||
| 142 | } | 145 | } | ||
| 143 | 146 | ||||
| 144 | for _, tt := range tests { | 147 | for _, tt := range tests { | ||
| 145 | t.Run(tt.name, func(t *testing.T) { | 148 | t.Run(tt.name, func(t *testing.T) { | ||
| 146 | - | if err := tt.validate(); err == nil { | 149 | + | if err := tt.validate(); !errors.Is(err, tt.wantErr) { |
| 147 | - | t.Fatal("direct validation accepted client-owned policy for remote target") | 150 | + | t.Fatalf("direct validation error = %v, want %v", err, tt.wantErr) |
| 148 | } | 151 | } | ||
| 149 | if payload := tt.marshalBad(); payload != nil { | 152 | if payload := tt.marshalBad(); payload != nil { | ||
| 150 | t.Fatalf("marshal accepted client-owned policy: %x", payload) | 153 | t.Fatalf("marshal accepted client-owned policy: %x", payload) | ||
| @@ -163,8 +166,8 @@ func TestRemoteTargetWireRejectsClientOwnedEnvironmentPolicy(t *testing.T) { | |||||
| 163 | t.Fatal("trailing garbage unexpectedly decoded") | 166 | t.Fatal("trailing garbage unexpectedly decoded") | ||
| 164 | } | 167 | } | ||
| 165 | payload[len(payload)-tt.policyOffset] = byte(EnvironmentPolicyClientOwned) | 168 | payload[len(payload)-tt.policyOffset] = byte(EnvironmentPolicyClientOwned) | ||
| 166 | - | if err := tt.unmarshal(payload); err == nil { | 169 | + | if err := tt.unmarshal(payload); !errors.Is(err, tt.wantErr) { |
| 167 | - | t.Fatal("unmarshal accepted client-owned policy for remote target") | 170 | + | t.Fatalf("mutated policy error = %v, want %v", err, tt.wantErr) |
| 168 | } | 171 | } | ||
| 169 | }) | 172 | }) | ||
| 170 | } | 173 | } | ||
M internal/ports/wire.go +1 -1
| @@ -789,7 +789,7 @@ func ValidateAttachTarget(m AttachTarget) error { | |||
| 789 | 789 | if m.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | |
| 790 | 790 | return ErrInvalidAttachTarget | |
| 791 | 791 | } | |
| 792 | - | if m.Intent != IntentAttach && m.Intent != IntentResume { | |
| 792 | + | if m.Intent != IntentAttach { | |
| 793 | 793 | return ErrInvalidAttachTarget | |
| 794 | 794 | } | |
| 795 | 795 | if err := validateWireRemoteTarget(*m.RemoteTarget); err != nil { | |
| @@ -789,7 +789,7 @@ func ValidateAttachTarget(m AttachTarget) error { | |||||
| 789 | if m.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | 789 | if m.EnvironmentPolicy != EnvironmentPolicyDaemonOwned { | ||
| 790 | return ErrInvalidAttachTarget | 790 | return ErrInvalidAttachTarget | ||
| 791 | } | 791 | } | ||
| 792 | - | if m.Intent != IntentAttach && m.Intent != IntentResume { | 792 | + | if m.Intent != IntentAttach { |
| 793 | return ErrInvalidAttachTarget | 793 | return ErrInvalidAttachTarget | ||
| 794 | } | 794 | } | ||
| 795 | if err := validateWireRemoteTarget(*m.RemoteTarget); err != nil { | 795 | if err := validateWireRemoteTarget(*m.RemoteTarget); err != nil { | ||
M internal/usecase/client/client.go +1 -2
| @@ -531,7 +531,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 531 | 531 | if handoffErr != nil { | |
| 532 | 532 | return handoffErr | |
| 533 | 533 | } | |
| 534 | - | if homeRoute != nil { | |
| 534 | + | if homeRoute != nil && (nextRequest.RemoteTarget != nil || nextRequest.EnvironmentPolicy == ports.EnvironmentPolicyDaemonOwned) { | |
| 535 | 535 | nextRequest.NavigationCapabilities |= ports.NavigationCapabilityHomePicker | |
| 536 | 536 | } | |
| 537 | 537 | if attemptRequest.StartupOverlay == ports.StartupOverlaySessionPicker { | |
| @@ -563,7 +563,6 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||
| 563 | 563 | } | |
| 564 | 564 | if result.err == nil { | |
| 565 | 565 | if result.welcomed && returnNavigationPending { | |
| 566 | - | returnRoute = nil | |
| 567 | 566 | returnNavigationPending = false | |
| 568 | 567 | returnResumeFallback = false | |
| 569 | 568 | } | |
| @@ -531,7 +531,7 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 531 | if handoffErr != nil { | 531 | if handoffErr != nil { | ||
| 532 | return handoffErr | 532 | return handoffErr | ||
| 533 | } | 533 | } | ||
| 534 | - | if homeRoute != nil { | 534 | + | if homeRoute != nil && (nextRequest.RemoteTarget != nil || nextRequest.EnvironmentPolicy == ports.EnvironmentPolicyDaemonOwned) { |
| 535 | nextRequest.NavigationCapabilities |= ports.NavigationCapabilityHomePicker | 535 | nextRequest.NavigationCapabilities |= ports.NavigationCapabilityHomePicker | ||
| 536 | } | 536 | } | ||
| 537 | if attemptRequest.StartupOverlay == ports.StartupOverlaySessionPicker { | 537 | if attemptRequest.StartupOverlay == ports.StartupOverlaySessionPicker { | ||
| @@ -563,7 +563,6 @@ func (r *Runner) Run(ctx context.Context, request AttachRequest) (retErr error) | |||||
| 563 | } | 563 | } | ||
| 564 | if result.err == nil { | 564 | if result.err == nil { | ||
| 565 | if result.welcomed && returnNavigationPending { | 565 | if result.welcomed && returnNavigationPending { | ||
| 566 | - | returnRoute = nil | |||
| 567 | returnNavigationPending = false | 566 | returnNavigationPending = false | ||
| 568 | returnResumeFallback = false | 567 | returnResumeFallback = false | ||
| 569 | } | 568 | } | ||
M internal/usecase/client/navigation_test.go +11 -0
| @@ -3,6 +3,7 @@ package client | |||
| 3 | 3 | import ( | |
| 4 | 4 | "testing" | |
| 5 | 5 | ||
| 6 | + | "github.com/bnema/vev/internal/domain" | |
| 6 | 7 | "github.com/bnema/vev/internal/ports" | |
| 7 | 8 | "github.com/stretchr/testify/require" | |
| 8 | 9 | ) | |
| @@ -24,12 +25,22 @@ func TestSyncReconnectRemoteKeepsRequestAndToastClassificationTogether(t *testin | |||
| 24 | 25 | } | |
| 25 | 26 | ||
| 26 | 27 | func TestValidateAttachRequestNavigationTable(t *testing.T) { | |
| 28 | + | var lifecycle domain.SessionLifecycleID | |
| 29 | + | lifecycle[0] = 1 | |
| 30 | + | remoteRoute := &domain.RemoteSessionTarget{ | |
| 31 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |
| 32 | + | SessionName: "work", LiveTabID: "tab-1", | |
| 33 | + | } | |
| 27 | 34 | tests := []struct { | |
| 28 | 35 | name string | |
| 29 | 36 | request AttachRequest | |
| 30 | 37 | valid bool | |
| 31 | 38 | }{ | |
| 32 | 39 | {name: "ordinary route", request: AttachRequest{}, valid: true}, | |
| 40 | + | {name: "home picker without remote route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", NavigationCapabilities: ports.NavigationCapabilityHomePicker}, valid: false}, | |
| 41 | + | {name: "home picker on daemon-owned remote route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", Remote: true, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, NavigationCapabilities: ports.NavigationCapabilityHomePicker}, valid: true}, | |
| 42 | + | {name: "back on client-owned route", request: AttachRequest{StartupOverlay: ports.StartupOverlaySessionPicker, NavigationCapabilities: ports.NavigationCapabilityBack}, valid: true}, | |
| 43 | + | {name: "back on remote-target route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RemoteTarget: remoteRoute, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, StartupOverlay: ports.StartupOverlaySessionPicker, NavigationCapabilities: ports.NavigationCapabilityBack}, valid: false}, | |
| 33 | 44 | {name: "unknown capability", request: AttachRequest{NavigationCapabilities: 4}, valid: false}, | |
| 34 | 45 | {name: "back without startup picker", request: AttachRequest{NavigationCapabilities: ports.NavigationCapabilityBack}, valid: false}, | |
| 35 | 46 | {name: "startup picker without back", request: AttachRequest{StartupOverlay: ports.StartupOverlaySessionPicker}, valid: false}, | |
| @@ -3,6 +3,7 @@ package client | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "testing" | 4 | "testing" | ||
| 5 | 5 | ||||
| 6 | + | "github.com/bnema/vev/internal/domain" | |||
| 6 | "github.com/bnema/vev/internal/ports" | 7 | "github.com/bnema/vev/internal/ports" | ||
| 7 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 8 | ) | 9 | ) | ||
| @@ -24,12 +25,22 @@ func TestSyncReconnectRemoteKeepsRequestAndToastClassificationTogether(t *testin | |||||
| 24 | } | 25 | } | ||
| 25 | 26 | ||||
| 26 | func TestValidateAttachRequestNavigationTable(t *testing.T) { | 27 | func TestValidateAttachRequestNavigationTable(t *testing.T) { | ||
| 28 | + | var lifecycle domain.SessionLifecycleID | |||
| 29 | + | lifecycle[0] = 1 | |||
| 30 | + | remoteRoute := &domain.RemoteSessionTarget{ | |||
| 31 | + | Endpoint: "arch", DisplayOrigin: "arch", LifecycleID: lifecycle, | |||
| 32 | + | SessionName: "work", LiveTabID: "tab-1", | |||
| 33 | + | } | |||
| 27 | tests := []struct { | 34 | tests := []struct { | ||
| 28 | name string | 35 | name string | ||
| 29 | request AttachRequest | 36 | request AttachRequest | ||
| 30 | valid bool | 37 | valid bool | ||
| 31 | }{ | 38 | }{ | ||
| 32 | {name: "ordinary route", request: AttachRequest{}, valid: true}, | 39 | {name: "ordinary route", request: AttachRequest{}, valid: true}, | ||
| 40 | + | {name: "home picker without remote route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", NavigationCapabilities: ports.NavigationCapabilityHomePicker}, valid: false}, | |||
| 41 | + | {name: "home picker on daemon-owned remote route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", Remote: true, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, NavigationCapabilities: ports.NavigationCapabilityHomePicker}, valid: true}, | |||
| 42 | + | {name: "back on client-owned route", request: AttachRequest{StartupOverlay: ports.StartupOverlaySessionPicker, NavigationCapabilities: ports.NavigationCapabilityBack}, valid: true}, | |||
| 43 | + | {name: "back on remote-target route", request: AttachRequest{Intent: ports.IntentAttach, SessionName: "work", RemoteTarget: remoteRoute, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, StartupOverlay: ports.StartupOverlaySessionPicker, NavigationCapabilities: ports.NavigationCapabilityBack}, valid: false}, | |||
| 33 | {name: "unknown capability", request: AttachRequest{NavigationCapabilities: 4}, valid: false}, | 44 | {name: "unknown capability", request: AttachRequest{NavigationCapabilities: 4}, valid: false}, | ||
| 34 | {name: "back without startup picker", request: AttachRequest{NavigationCapabilities: ports.NavigationCapabilityBack}, valid: false}, | 45 | {name: "back without startup picker", request: AttachRequest{NavigationCapabilities: ports.NavigationCapabilityBack}, valid: false}, | ||
| 35 | {name: "startup picker without back", request: AttachRequest{StartupOverlay: ports.StartupOverlaySessionPicker}, valid: false}, | 46 | {name: "startup picker without back", request: AttachRequest{StartupOverlay: ports.StartupOverlaySessionPicker}, valid: false}, | ||
M internal/usecase/daemon/attachment_connection.go +5 -2
| @@ -108,8 +108,11 @@ func (t *attachmentConnectionToken) endAttachmentEffect() { | |||
| 108 | 108 | t.effect = nil | |
| 109 | 109 | } | |
| 110 | 110 | ||
| 111 | - | // attachmentEffectCurrentSessionLocked is the same check at a session-owned | |
| 112 | - | // mutation boundary where sess.core().mu is already held. | |
| 111 | + | // attachmentEffectCurrentSessionLocked validates authority at a session-owned | |
| 112 | + | // mutation boundary where sess.core().mu is already held. A live effect ticket | |
| 113 | + | // preserves the exact capability it captured for the effect because attachment | |
| 114 | + | // transitions drain the gate before changing identity. This differs from | |
| 115 | + | // attachmentCurrent, which revalidates the attachment's currently published identity. | |
| 113 | 116 | func (t attachmentConnectionToken) attachmentEffectCurrentSessionLocked() bool { | |
| 114 | 117 | if t.effect != nil && !t.effect.ended.Load() { | |
| 115 | 118 | return true | |
| @@ -108,8 +108,11 @@ func (t *attachmentConnectionToken) endAttachmentEffect() { | |||||
| 108 | t.effect = nil | 108 | t.effect = nil | ||
| 109 | } | 109 | } | ||
| 110 | 110 | ||||
| 111 | - | // attachmentEffectCurrentSessionLocked is the same check at a session-owned | 111 | + | // attachmentEffectCurrentSessionLocked validates authority at a session-owned |
| 112 | - | // mutation boundary where sess.core().mu is already held. | 112 | + | // mutation boundary where sess.core().mu is already held. A live effect ticket |
| 113 | + | // preserves the exact capability it captured for the effect because attachment | |||
| 114 | + | // transitions drain the gate before changing identity. This differs from | |||
| 115 | + | // attachmentCurrent, which revalidates the attachment's currently published identity. | |||
| 113 | func (t attachmentConnectionToken) attachmentEffectCurrentSessionLocked() bool { | 116 | func (t attachmentConnectionToken) attachmentEffectCurrentSessionLocked() bool { | ||
| 114 | if t.effect != nil && !t.effect.ended.Load() { | 117 | if t.effect != nil && !t.effect.ended.Load() { | ||
| 115 | return true | 118 | return true | ||
M internal/usecase/daemon/attachment_effect_gate.go +2 -1
| @@ -35,7 +35,7 @@ func capabilityFromToken(token attachmentConnectionToken) attachmentCapability { | |||
| 35 | 35 | } | |
| 36 | 36 | ||
| 37 | 37 | func (c attachmentCapability) matches(token attachmentConnectionToken) bool { | |
| 38 | - | return token.ac != nil && c.sess == token.sess && | |
| 38 | + | return token.ac != nil && c.sess != nil && token.sess != nil && c.sess == token.sess && | |
| 39 | 39 | c.generation == token.generation && c.transport.transport == token.transport.transport && | |
| 40 | 40 | c.transport.incarnation == token.transport.incarnation && c.lease == token.lease | |
| 41 | 41 | } | |
| @@ -220,6 +220,7 @@ func (ac *attachedClient) beginAttachmentEffect(token attachmentConnectionToken) | |||
| 220 | 220 | // after Welcome has completed so a replacement blocked behind that send can | |
| 221 | 221 | // publish before readiness or first paint. | |
| 222 | 222 | func (ac *attachedClient) beginCurrentAttachmentEffect(sess *session, tr ports.Transport) (attachmentConnectionToken, *attachmentEffectTicket, bool) { | |
| 223 | + | //nolint:contextcheck // This compatibility wrapper deliberately delegates with a fresh context; cancellable callers use the context variant below. | |
| 223 | 224 | return ac.beginCurrentAttachmentEffectContext(context.Background(), sess, tr) | |
| 224 | 225 | } | |
| 225 | 226 | ||
| @@ -35,7 +35,7 @@ func capabilityFromToken(token attachmentConnectionToken) attachmentCapability { | |||||
| 35 | } | 35 | } | ||
| 36 | 36 | ||||
| 37 | func (c attachmentCapability) matches(token attachmentConnectionToken) bool { | 37 | func (c attachmentCapability) matches(token attachmentConnectionToken) bool { | ||
| 38 | - | return token.ac != nil && c.sess == token.sess && | 38 | + | return token.ac != nil && c.sess != nil && token.sess != nil && c.sess == token.sess && |
| 39 | c.generation == token.generation && c.transport.transport == token.transport.transport && | 39 | c.generation == token.generation && c.transport.transport == token.transport.transport && | ||
| 40 | c.transport.incarnation == token.transport.incarnation && c.lease == token.lease | 40 | c.transport.incarnation == token.transport.incarnation && c.lease == token.lease | ||
| 41 | } | 41 | } | ||
| @@ -220,6 +220,7 @@ func (ac *attachedClient) beginAttachmentEffect(token attachmentConnectionToken) | |||||
| 220 | // after Welcome has completed so a replacement blocked behind that send can | 220 | // after Welcome has completed so a replacement blocked behind that send can | ||
| 221 | // publish before readiness or first paint. | 221 | // publish before readiness or first paint. | ||
| 222 | func (ac *attachedClient) beginCurrentAttachmentEffect(sess *session, tr ports.Transport) (attachmentConnectionToken, *attachmentEffectTicket, bool) { | 222 | func (ac *attachedClient) beginCurrentAttachmentEffect(sess *session, tr ports.Transport) (attachmentConnectionToken, *attachmentEffectTicket, bool) { | ||
| 223 | + | //nolint:contextcheck // This compatibility wrapper deliberately delegates with a fresh context; cancellable callers use the context variant below. | |||
| 223 | return ac.beginCurrentAttachmentEffectContext(context.Background(), sess, tr) | 224 | return ac.beginCurrentAttachmentEffectContext(context.Background(), sess, tr) | ||
| 224 | } | 225 | } | ||
| 225 | 226 | ||||
M internal/usecase/daemon/attachment_effect_gate_test.go +1 -1
| @@ -675,7 +675,7 @@ func TestAttachedNavigationCommandSendsResultAfterLocalTransition(t *testing.T) | |||
| 675 | 675 | d.mu.Lock() | |
| 676 | 676 | d.sessions[target.id] = target | |
| 677 | 677 | d.mu.Unlock() | |
| 678 | - | ac.previousSession.Set(target) | |
| 678 | + | ac.previousSession.Set(target.id) | |
| 679 | 679 | ||
| 680 | 680 | transport := &closeTrackingTransport{} | |
| 681 | 681 | ac.replaceTransport(transport) | |
| @@ -675,7 +675,7 @@ func TestAttachedNavigationCommandSendsResultAfterLocalTransition(t *testing.T) | |||||
| 675 | d.mu.Lock() | 675 | d.mu.Lock() | ||
| 676 | d.sessions[target.id] = target | 676 | d.sessions[target.id] = target | ||
| 677 | d.mu.Unlock() | 677 | d.mu.Unlock() | ||
| 678 | - | ac.previousSession.Set(target) | 678 | + | ac.previousSession.Set(target.id) |
| 679 | 679 | ||||
| 680 | transport := &closeTrackingTransport{} | 680 | transport := &closeTrackingTransport{} | ||
| 681 | ac.replaceTransport(transport) | 681 | ac.replaceTransport(transport) | ||
M internal/usecase/daemon/attachment_parking_test.go +2 -1
| @@ -167,9 +167,10 @@ func TestFailedResumeHandshakeKeepsParkedCredential(t *testing.T) { | |||
| 167 | 167 | require.Nil(t, ac.transport()) | |
| 168 | 168 | d.mu.Lock() | |
| 169 | 169 | parked := d.parked[token] | |
| 170 | + | claimed := parked != nil && parked.claimed | |
| 170 | 171 | d.mu.Unlock() | |
| 171 | 172 | require.NotNil(t, parked) | |
| 172 | - | require.False(t, parked.claimed) | |
| 173 | + | require.False(t, claimed) | |
| 173 | 174 | require.Equal(t, token, ac.resumeToken) | |
| 174 | 175 | ||
| 175 | 176 | _, resumed, ok, err := d.resumeParked(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}, defaultSize) | |
| @@ -167,9 +167,10 @@ func TestFailedResumeHandshakeKeepsParkedCredential(t *testing.T) { | |||||
| 167 | require.Nil(t, ac.transport()) | 167 | require.Nil(t, ac.transport()) | ||
| 168 | d.mu.Lock() | 168 | d.mu.Lock() | ||
| 169 | parked := d.parked[token] | 169 | parked := d.parked[token] | ||
| 170 | + | claimed := parked != nil && parked.claimed | |||
| 170 | d.mu.Unlock() | 171 | d.mu.Unlock() | ||
| 171 | require.NotNil(t, parked) | 172 | require.NotNil(t, parked) | ||
| 172 | - | require.False(t, parked.claimed) | 173 | + | require.False(t, claimed) |
| 173 | require.Equal(t, token, ac.resumeToken) | 174 | require.Equal(t, token, ac.resumeToken) | ||
| 174 | 175 | ||||
| 175 | _, resumed, ok, err := d.resumeParked(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}, defaultSize) | 176 | _, resumed, ok, err := d.resumeParked(helloResumeCapable(ports.IntentResume, "work", token), &closeTrackingTransport{}, defaultSize) | ||
M internal/usecase/daemon/attachment_transition.go +1 -1
| @@ -45,7 +45,7 @@ type attachmentTransitionRequest struct { | |||
| 45 | 45 | } | |
| 46 | 46 | ||
| 47 | 47 | func transitionSourceTokenMatchesRequest(token attachmentConnectionToken, source *session, req attachmentTransitionRequest) bool { | |
| 48 | - | return token.sess == source && token.ac == req.next && | |
| 48 | + | return source != nil && token.sess != nil && token.sess == source && token.ac == req.next && | |
| 49 | 49 | token.generation == req.next.connectionGeneration.Load() && | |
| 50 | 50 | token.transport.transport == req.expectedTransport.transport && | |
| 51 | 51 | token.transport.incarnation == req.expectedTransport.incarnation | |
| @@ -45,7 +45,7 @@ type attachmentTransitionRequest struct { | |||||
| 45 | } | 45 | } | ||
| 46 | 46 | ||||
| 47 | func transitionSourceTokenMatchesRequest(token attachmentConnectionToken, source *session, req attachmentTransitionRequest) bool { | 47 | func transitionSourceTokenMatchesRequest(token attachmentConnectionToken, source *session, req attachmentTransitionRequest) bool { | ||
| 48 | - | return token.sess == source && token.ac == req.next && | 48 | + | return source != nil && token.sess != nil && token.sess == source && token.ac == req.next && |
| 49 | token.generation == req.next.connectionGeneration.Load() && | 49 | token.generation == req.next.connectionGeneration.Load() && | ||
| 50 | token.transport.transport == req.expectedTransport.transport && | 50 | token.transport.transport == req.expectedTransport.transport && | ||
| 51 | token.transport.incarnation == req.expectedTransport.incarnation | 51 | token.transport.incarnation == req.expectedTransport.incarnation | ||
M internal/usecase/daemon/client.go +2 -2
| @@ -96,7 +96,7 @@ type attachedClient struct { | |||
| 96 | 96 | renderStages renderStageHooks // optional render and handoff observability hooks | |
| 97 | 97 | // previousSession is guarded independently. It is retained through temporary | |
| 98 | 98 | // setSession(nil) hand-offs and cleared only on terminal teardown. | |
| 99 | - | previousSession Guarded[*session] | |
| 99 | + | previousSession Guarded[domain.SessionID] | |
| 100 | 100 | linkMu sync.Mutex | |
| 101 | 101 | sendMu sync.Mutex | |
| 102 | 102 | // routeCreatedSession marks a session created by this attachment's route. | |
| @@ -227,7 +227,7 @@ func (ac *attachedClient) setSession(sess *session) { ac.sess.Set(sess) } | |||
| 227 | 227 | ||
| 228 | 228 | func (ac *attachedClient) clearPreviousSession() { | |
| 229 | 229 | if ac != nil { | |
| 230 | - | ac.previousSession.Set(nil) | |
| 230 | + | ac.previousSession.Set("") | |
| 231 | 231 | } | |
| 232 | 232 | } | |
| 233 | 233 | ||
| @@ -96,7 +96,7 @@ type attachedClient struct { | |||||
| 96 | renderStages renderStageHooks // optional render and handoff observability hooks | 96 | renderStages renderStageHooks // optional render and handoff observability hooks | ||
| 97 | // previousSession is guarded independently. It is retained through temporary | 97 | // previousSession is guarded independently. It is retained through temporary | ||
| 98 | // setSession(nil) hand-offs and cleared only on terminal teardown. | 98 | // setSession(nil) hand-offs and cleared only on terminal teardown. | ||
| 99 | - | previousSession Guarded[*session] | 99 | + | previousSession Guarded[domain.SessionID] |
| 100 | linkMu sync.Mutex | 100 | linkMu sync.Mutex | ||
| 101 | sendMu sync.Mutex | 101 | sendMu sync.Mutex | ||
| 102 | // routeCreatedSession marks a session created by this attachment's route. | 102 | // routeCreatedSession marks a session created by this attachment's route. | ||
| @@ -227,7 +227,7 @@ func (ac *attachedClient) setSession(sess *session) { ac.sess.Set(sess) } | |||||
| 227 | 227 | ||||
| 228 | func (ac *attachedClient) clearPreviousSession() { | 228 | func (ac *attachedClient) clearPreviousSession() { | ||
| 229 | if ac != nil { | 229 | if ac != nil { | ||
| 230 | - | ac.previousSession.Set(nil) | 230 | + | ac.previousSession.Set("") |
| 231 | } | 231 | } | ||
| 232 | } | 232 | } | ||
| 233 | 233 | ||||
M internal/usecase/daemon/client_transport_errors.go +5 -4
| @@ -86,7 +86,7 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||
| 86 | 86 | } | |
| 87 | 87 | if !explicit && d.parkAttachment(sess, ac) { | |
| 88 | 88 | _ = ac.closeCapturedTransport(oldTr) | |
| 89 | - | d.log.Info("client parked", "session", sess.nameSnapshot()) | |
| 89 | + | d.log.Info("client parked", "session", name) | |
| 90 | 90 | return | |
| 91 | 91 | } | |
| 92 | 92 | // Explicit winners (and non-explicit park failures) must drop any same- | |
| @@ -105,7 +105,7 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||
| 105 | 105 | d.boundedSend(ac, frameDetached(ports.ReasonDetach)) | |
| 106 | 106 | } | |
| 107 | 107 | _ = ac.closeCapturedTransport(oldTr) | |
| 108 | - | d.log.Info("client detached", "session", sess.nameSnapshot(), "explicit", explicit) | |
| 108 | + | d.log.Info("client detached", "session", name, "explicit", explicit) | |
| 109 | 109 | } | |
| 110 | 110 | ||
| 111 | 111 | // detachOnSendError drops a client whose transport failed, leaving the session | |
| @@ -170,13 +170,14 @@ func (d *Daemon) reserveAttachmentSendErrorCleanup(token attachmentConnectionTok | |||
| 170 | 170 | } | |
| 171 | 171 | ||
| 172 | 172 | func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed ports.Transport) { | |
| 173 | + | name := sess.nameSnapshot() | |
| 173 | 174 | if rc := sess.renderCoordinator(); rc != nil { | |
| 174 | 175 | rc.noteDetach(ac) | |
| 175 | 176 | } | |
| 176 | 177 | d.recalculateSessionGeometryAndInvalidate(sess, nil, "client_transport_errors.go") | |
| 177 | 178 | if d.parkAttachment(sess, ac) { | |
| 178 | 179 | _ = ac.closeCapturedTransport(failed) | |
| 179 | - | d.log.Warn("parked client after send error", "session", sess.nameSnapshot()) | |
| 180 | + | d.log.Warn("parked client after send error", "session", name) | |
| 180 | 181 | return | |
| 181 | 182 | } | |
| 182 | 183 | d.clearParkingInFlight(d.resumeTokenSnapshot(ac), ac) | |
| @@ -184,5 +185,5 @@ func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed | |||
| 184 | 185 | d.resetScreenDefaultColors(sess) | |
| 185 | 186 | ac.clearPreviousSession() | |
| 186 | 187 | _ = ac.closeCapturedTransport(failed) | |
| 187 | - | d.log.Warn("detached client after send error", "session", sess.nameSnapshot()) | |
| 188 | + | d.log.Warn("detached client after send error", "session", name) | |
| 188 | 189 | } | |
| @@ -86,7 +86,7 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||||
| 86 | } | 86 | } | ||
| 87 | if !explicit && d.parkAttachment(sess, ac) { | 87 | if !explicit && d.parkAttachment(sess, ac) { | ||
| 88 | _ = ac.closeCapturedTransport(oldTr) | 88 | _ = ac.closeCapturedTransport(oldTr) | ||
| 89 | - | d.log.Info("client parked", "session", sess.nameSnapshot()) | 89 | + | d.log.Info("client parked", "session", name) |
| 90 | return | 90 | return | ||
| 91 | } | 91 | } | ||
| 92 | // Explicit winners (and non-explicit park failures) must drop any same- | 92 | // Explicit winners (and non-explicit park failures) must drop any same- | ||
| @@ -105,7 +105,7 @@ func (d *Daemon) finishClientGone(sess *session, ac *attachedClient, failed port | |||||
| 105 | d.boundedSend(ac, frameDetached(ports.ReasonDetach)) | 105 | d.boundedSend(ac, frameDetached(ports.ReasonDetach)) | ||
| 106 | } | 106 | } | ||
| 107 | _ = ac.closeCapturedTransport(oldTr) | 107 | _ = ac.closeCapturedTransport(oldTr) | ||
| 108 | - | d.log.Info("client detached", "session", sess.nameSnapshot(), "explicit", explicit) | 108 | + | d.log.Info("client detached", "session", name, "explicit", explicit) |
| 109 | } | 109 | } | ||
| 110 | 110 | ||||
| 111 | // detachOnSendError drops a client whose transport failed, leaving the session | 111 | // detachOnSendError drops a client whose transport failed, leaving the session | ||
| @@ -170,13 +170,14 @@ func (d *Daemon) reserveAttachmentSendErrorCleanup(token attachmentConnectionTok | |||||
| 170 | } | 170 | } | ||
| 171 | 171 | ||||
| 172 | func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed ports.Transport) { | 172 | func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed ports.Transport) { | ||
| 173 | + | name := sess.nameSnapshot() | |||
| 173 | if rc := sess.renderCoordinator(); rc != nil { | 174 | if rc := sess.renderCoordinator(); rc != nil { | ||
| 174 | rc.noteDetach(ac) | 175 | rc.noteDetach(ac) | ||
| 175 | } | 176 | } | ||
| 176 | d.recalculateSessionGeometryAndInvalidate(sess, nil, "client_transport_errors.go") | 177 | d.recalculateSessionGeometryAndInvalidate(sess, nil, "client_transport_errors.go") | ||
| 177 | if d.parkAttachment(sess, ac) { | 178 | if d.parkAttachment(sess, ac) { | ||
| 178 | _ = ac.closeCapturedTransport(failed) | 179 | _ = ac.closeCapturedTransport(failed) | ||
| 179 | - | d.log.Warn("parked client after send error", "session", sess.nameSnapshot()) | 180 | + | d.log.Warn("parked client after send error", "session", name) |
| 180 | return | 181 | return | ||
| 181 | } | 182 | } | ||
| 182 | d.clearParkingInFlight(d.resumeTokenSnapshot(ac), ac) | 183 | d.clearParkingInFlight(d.resumeTokenSnapshot(ac), ac) | ||
| @@ -184,5 +185,5 @@ func (d *Daemon) finishSendErrorDetach(sess *session, ac *attachedClient, failed | |||||
| 184 | d.resetScreenDefaultColors(sess) | 185 | d.resetScreenDefaultColors(sess) | ||
| 185 | ac.clearPreviousSession() | 186 | ac.clearPreviousSession() | ||
| 186 | _ = ac.closeCapturedTransport(failed) | 187 | _ = ac.closeCapturedTransport(failed) | ||
| 187 | - | d.log.Warn("detached client after send error", "session", sess.nameSnapshot()) | 188 | + | d.log.Warn("detached client after send error", "session", name) |
| 188 | } | 189 | } | ||
M internal/usecase/daemon/daemon.go +1 -0
| @@ -1148,6 +1148,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||
| 1148 | 1148 | } | |
| 1149 | 1149 | ||
| 1150 | 1150 | welcomeSent := false | |
| 1151 | + | //nolint:contextcheck // handshakeCtx is intentionally not propagated: cancellation teardown must complete its geometry and ownership cleanup. | |
| 1151 | 1152 | failAttachment := func() { | |
| 1152 | 1153 | d.failHandshakeAttachment(sess, ac, tr, welcomeSent) | |
| 1153 | 1154 | } | |
| @@ -1148,6 +1148,7 @@ func (d *Daemon) handleHelloWithContext(handshakeCtx context.Context, timedOut < | |||||
| 1148 | } | 1148 | } | ||
| 1149 | 1149 | ||||
| 1150 | welcomeSent := false | 1150 | welcomeSent := false | ||
| 1151 | + | //nolint:contextcheck // handshakeCtx is intentionally not propagated: cancellation teardown must complete its geometry and ownership cleanup. | |||
| 1151 | failAttachment := func() { | 1152 | failAttachment := func() { | ||
| 1152 | d.failHandshakeAttachment(sess, ac, tr, welcomeSent) | 1153 | d.failHandshakeAttachment(sess, ac, tr, welcomeSent) | ||
| 1153 | } | 1154 | } | ||
M internal/usecase/daemon/input.go +1 -0
| @@ -397,6 +397,7 @@ func (h daemonKeyHandler) acquireAttachmentEffect() (*session, *attachmentEffect | |||
| 397 | 397 | token.effect = effect | |
| 398 | 398 | h.d.afterAttachmentEffectAdmitted(token) | |
| 399 | 399 | } | |
| 400 | + | // Keep this defensive check even though token admission normally requires a session. | |
| 400 | 401 | if h.connectionToken.sess == nil { | |
| 401 | 402 | effect.End() | |
| 402 | 403 | return nil, nil, false | |
| @@ -397,6 +397,7 @@ func (h daemonKeyHandler) acquireAttachmentEffect() (*session, *attachmentEffect | |||||
| 397 | token.effect = effect | 397 | token.effect = effect | ||
| 398 | h.d.afterAttachmentEffectAdmitted(token) | 398 | h.d.afterAttachmentEffectAdmitted(token) | ||
| 399 | } | 399 | } | ||
| 400 | + | // Keep this defensive check even though token admission normally requires a session. | |||
| 400 | if h.connectionToken.sess == nil { | 401 | if h.connectionToken.sess == nil { | ||
| 401 | effect.End() | 402 | effect.End() | ||
| 402 | return nil, nil, false | 403 | return nil, nil, false | ||
M internal/usecase/daemon/input_test.go +15 -15
| @@ -799,8 +799,8 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||
| 799 | 799 | prepare func() | |
| 800 | 800 | }{ | |
| 801 | 801 | {name: "no target", prepare: func() {}}, | |
| 802 | - | {name: "stale target", prepare: func() { ac.previousSession.Set(&session{sessionCore: sessionCore{id: "gone"}}) }}, | |
| 803 | - | {name: "same session", prepare: func() { ac.previousSession.Set(current) }}, | |
| 802 | + | {name: "stale target", prepare: func() { ac.previousSession.Set("gone") }}, | |
| 803 | + | {name: "same session", prepare: func() { ac.previousSession.Set(current.id) }}, | |
| 804 | 804 | } { | |
| 805 | 805 | t.Run(tc.name, func(t *testing.T) { | |
| 806 | 806 | ac.clearPreviousSession() | |
| @@ -808,7 +808,7 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||
| 808 | 808 | runPaletteCommand(t, d, current, ac, "BSK") | |
| 809 | 809 | require.Same(t, current, ac.currentSession()) | |
| 810 | 810 | if tc.name != "no target" { | |
| 811 | - | require.Nil(t, ac.previousSession.Get(), "invalid target must not remain toggleable") | |
| 811 | + | require.Empty(t, ac.previousSession.Get(), "invalid target must not remain toggleable") | |
| 812 | 812 | } | |
| 813 | 813 | }) | |
| 814 | 814 | } | |
| @@ -818,7 +818,7 @@ func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |||
| 818 | 818 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 819 | 819 | defer releaseAll(releases) | |
| 820 | 820 | target := d.sessions[domain.SessionID("recent")] | |
| 821 | - | ac.previousSession.Set(target) | |
| 821 | + | ac.previousSession.Set(target.id) | |
| 822 | 822 | ||
| 823 | 823 | // A displaced attachment makes the previously valid handoff stale by the | |
| 824 | 824 | // time switchToTarget commits it. | |
| @@ -829,7 +829,7 @@ func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |||
| 829 | 829 | d.backSession(current, ac) | |
| 830 | 830 | ||
| 831 | 831 | require.Same(t, current, ac.currentSession(), "a stale handoff must leave the attachment on its origin") | |
| 832 | - | require.Same(t, target, ac.previousSession.Get(), "a failed handoff remains retryable") | |
| 832 | + | require.Equal(t, target.id, ac.previousSession.Get(), "a failed handoff remains retryable") | |
| 833 | 833 | history := d.notices.history() | |
| 834 | 834 | require.Len(t, history, 1, "the switch failure must be reported exactly once") | |
| 835 | 835 | require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code) | |
| @@ -839,14 +839,14 @@ func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | |||
| 839 | 839 | ac := &attachedClient{} | |
| 840 | 840 | stale := &session{sessionCore: sessionCore{id: "stale"}} | |
| 841 | 841 | updated := &session{sessionCore: sessionCore{id: "updated"}} | |
| 842 | - | ac.previousSession.Set(stale) | |
| 842 | + | ac.previousSession.Set(stale.id) | |
| 843 | 843 | ||
| 844 | 844 | // Model a completed hand-off between observing a stale target and clearing | |
| 845 | 845 | // it. The conditional clear must not erase the newer toggle destination. | |
| 846 | - | ac.previousSession.Set(updated) | |
| 847 | - | ac.clearPreviousSessionIf(stale) | |
| 846 | + | ac.previousSession.Set(updated.id) | |
| 847 | + | ac.clearPreviousSessionIf(stale.id) | |
| 848 | 848 | ||
| 849 | - | require.Same(t, updated, ac.previousSession.Get()) | |
| 849 | + | require.Equal(t, updated.id, ac.previousSession.Get()) | |
| 850 | 850 | } | |
| 851 | 851 | ||
| 852 | 852 | // backSession's invalid-target fallback is a render producer: it must publish | |
| @@ -861,10 +861,10 @@ func TestBackSessionInvalidTargetsFallBackThroughOneInvalidation(t *testing.T) { | |||
| 861 | 861 | }{ | |
| 862 | 862 | {name: "no previous target", prepare: func(*session, *attachedClient) {}}, | |
| 863 | 863 | {name: "stale previous target", prepare: func(_ *session, ac *attachedClient) { | |
| 864 | - | ac.previousSession.Set(&session{sessionCore: sessionCore{id: "gone"}}) | |
| 864 | + | ac.previousSession.Set("gone") | |
| 865 | 865 | }}, | |
| 866 | 866 | {name: "previous target equals current", prepare: func(current *session, ac *attachedClient) { | |
| 867 | - | ac.previousSession.Set(current) | |
| 867 | + | ac.previousSession.Set(current.id) | |
| 868 | 868 | }}, | |
| 869 | 869 | } { | |
| 870 | 870 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -940,10 +940,10 @@ func TestSwitchSourcePreviousSessionContracts(t *testing.T) { | |||
| 940 | 940 | } | |
| 941 | 941 | require.Equal(t, domain.SessionID(tc.wantCurrent), ac.currentSession().id) | |
| 942 | 942 | if tc.wantPrev == "" { | |
| 943 | - | require.Nil(t, ac.previousSession.Get()) | |
| 943 | + | require.Empty(t, ac.previousSession.Get()) | |
| 944 | 944 | return | |
| 945 | 945 | } | |
| 946 | - | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get().core().id) | |
| 946 | + | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get()) | |
| 947 | 947 | }) | |
| 948 | 948 | } | |
| 949 | 949 | } | |
| @@ -963,10 +963,10 @@ func TestTerminalTeardownClearsPreviousSession(t *testing.T) { | |||
| 963 | 963 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 964 | 964 | defer releaseAll(releases) | |
| 965 | 965 | ||
| 966 | - | ac.previousSession.Set(d.sessions[domain.SessionID("recent")]) | |
| 966 | + | ac.previousSession.Set(domain.SessionID("recent")) | |
| 967 | 967 | tc.teardown(d, current, ac) | |
| 968 | 968 | ||
| 969 | - | require.Nil(t, ac.previousSession.Get()) | |
| 969 | + | require.Empty(t, ac.previousSession.Get()) | |
| 970 | 970 | }) | |
| 971 | 971 | } | |
| 972 | 972 | } | |
| @@ -799,8 +799,8 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||||
| 799 | prepare func() | 799 | prepare func() | ||
| 800 | }{ | 800 | }{ | ||
| 801 | {name: "no target", prepare: func() {}}, | 801 | {name: "no target", prepare: func() {}}, | ||
| 802 | - | {name: "stale target", prepare: func() { ac.previousSession.Set(&session{sessionCore: sessionCore{id: "gone"}}) }}, | 802 | + | {name: "stale target", prepare: func() { ac.previousSession.Set("gone") }}, |
| 803 | - | {name: "same session", prepare: func() { ac.previousSession.Set(current) }}, | 803 | + | {name: "same session", prepare: func() { ac.previousSession.Set(current.id) }}, |
| 804 | } { | 804 | } { | ||
| 805 | t.Run(tc.name, func(t *testing.T) { | 805 | t.Run(tc.name, func(t *testing.T) { | ||
| 806 | ac.clearPreviousSession() | 806 | ac.clearPreviousSession() | ||
| @@ -808,7 +808,7 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||||
| 808 | runPaletteCommand(t, d, current, ac, "BSK") | 808 | runPaletteCommand(t, d, current, ac, "BSK") | ||
| 809 | require.Same(t, current, ac.currentSession()) | 809 | require.Same(t, current, ac.currentSession()) | ||
| 810 | if tc.name != "no target" { | 810 | if tc.name != "no target" { | ||
| 811 | - | require.Nil(t, ac.previousSession.Get(), "invalid target must not remain toggleable") | 811 | + | require.Empty(t, ac.previousSession.Get(), "invalid target must not remain toggleable") |
| 812 | } | 812 | } | ||
| 813 | }) | 813 | }) | ||
| 814 | } | 814 | } | ||
| @@ -818,7 +818,7 @@ func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |||||
| 818 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | 818 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | ||
| 819 | defer releaseAll(releases) | 819 | defer releaseAll(releases) | ||
| 820 | target := d.sessions[domain.SessionID("recent")] | 820 | target := d.sessions[domain.SessionID("recent")] | ||
| 821 | - | ac.previousSession.Set(target) | 821 | + | ac.previousSession.Set(target.id) |
| 822 | 822 | ||||
| 823 | // A displaced attachment makes the previously valid handoff stale by the | 823 | // A displaced attachment makes the previously valid handoff stale by the | ||
| 824 | // time switchToTarget commits it. | 824 | // time switchToTarget commits it. | ||
| @@ -829,7 +829,7 @@ func TestBackSessionReportsStaleHandoffOnce(t *testing.T) { | |||||
| 829 | d.backSession(current, ac) | 829 | d.backSession(current, ac) | ||
| 830 | 830 | ||||
| 831 | require.Same(t, current, ac.currentSession(), "a stale handoff must leave the attachment on its origin") | 831 | require.Same(t, current, ac.currentSession(), "a stale handoff must leave the attachment on its origin") | ||
| 832 | - | require.Same(t, target, ac.previousSession.Get(), "a failed handoff remains retryable") | 832 | + | require.Equal(t, target.id, ac.previousSession.Get(), "a failed handoff remains retryable") |
| 833 | history := d.notices.history() | 833 | history := d.notices.history() | ||
| 834 | require.Len(t, history, 1, "the switch failure must be reported exactly once") | 834 | require.Len(t, history, 1, "the switch failure must be reported exactly once") | ||
| 835 | require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code) | 835 | require.Equal(t, domain.NoticeSessionUnavailable, history[0].Code) | ||
| @@ -839,14 +839,14 @@ func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | |||||
| 839 | ac := &attachedClient{} | 839 | ac := &attachedClient{} | ||
| 840 | stale := &session{sessionCore: sessionCore{id: "stale"}} | 840 | stale := &session{sessionCore: sessionCore{id: "stale"}} | ||
| 841 | updated := &session{sessionCore: sessionCore{id: "updated"}} | 841 | updated := &session{sessionCore: sessionCore{id: "updated"}} | ||
| 842 | - | ac.previousSession.Set(stale) | 842 | + | ac.previousSession.Set(stale.id) |
| 843 | 843 | ||||
| 844 | // Model a completed hand-off between observing a stale target and clearing | 844 | // Model a completed hand-off between observing a stale target and clearing | ||
| 845 | // it. The conditional clear must not erase the newer toggle destination. | 845 | // it. The conditional clear must not erase the newer toggle destination. | ||
| 846 | - | ac.previousSession.Set(updated) | 846 | + | ac.previousSession.Set(updated.id) |
| 847 | - | ac.clearPreviousSessionIf(stale) | 847 | + | ac.clearPreviousSessionIf(stale.id) |
| 848 | 848 | ||||
| 849 | - | require.Same(t, updated, ac.previousSession.Get()) | 849 | + | require.Equal(t, updated.id, ac.previousSession.Get()) |
| 850 | } | 850 | } | ||
| 851 | 851 | ||||
| 852 | // backSession's invalid-target fallback is a render producer: it must publish | 852 | // backSession's invalid-target fallback is a render producer: it must publish | ||
| @@ -861,10 +861,10 @@ func TestBackSessionInvalidTargetsFallBackThroughOneInvalidation(t *testing.T) { | |||||
| 861 | }{ | 861 | }{ | ||
| 862 | {name: "no previous target", prepare: func(*session, *attachedClient) {}}, | 862 | {name: "no previous target", prepare: func(*session, *attachedClient) {}}, | ||
| 863 | {name: "stale previous target", prepare: func(_ *session, ac *attachedClient) { | 863 | {name: "stale previous target", prepare: func(_ *session, ac *attachedClient) { | ||
| 864 | - | ac.previousSession.Set(&session{sessionCore: sessionCore{id: "gone"}}) | 864 | + | ac.previousSession.Set("gone") |
| 865 | }}, | 865 | }}, | ||
| 866 | {name: "previous target equals current", prepare: func(current *session, ac *attachedClient) { | 866 | {name: "previous target equals current", prepare: func(current *session, ac *attachedClient) { | ||
| 867 | - | ac.previousSession.Set(current) | 867 | + | ac.previousSession.Set(current.id) |
| 868 | }}, | 868 | }}, | ||
| 869 | } { | 869 | } { | ||
| 870 | t.Run(tc.name, func(t *testing.T) { | 870 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -940,10 +940,10 @@ func TestSwitchSourcePreviousSessionContracts(t *testing.T) { | |||||
| 940 | } | 940 | } | ||
| 941 | require.Equal(t, domain.SessionID(tc.wantCurrent), ac.currentSession().id) | 941 | require.Equal(t, domain.SessionID(tc.wantCurrent), ac.currentSession().id) | ||
| 942 | if tc.wantPrev == "" { | 942 | if tc.wantPrev == "" { | ||
| 943 | - | require.Nil(t, ac.previousSession.Get()) | 943 | + | require.Empty(t, ac.previousSession.Get()) |
| 944 | return | 944 | return | ||
| 945 | } | 945 | } | ||
| 946 | - | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get().core().id) | 946 | + | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get()) |
| 947 | }) | 947 | }) | ||
| 948 | } | 948 | } | ||
| 949 | } | 949 | } | ||
| @@ -963,10 +963,10 @@ func TestTerminalTeardownClearsPreviousSession(t *testing.T) { | |||||
| 963 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | 963 | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | ||
| 964 | defer releaseAll(releases) | 964 | defer releaseAll(releases) | ||
| 965 | 965 | ||||
| 966 | - | ac.previousSession.Set(d.sessions[domain.SessionID("recent")]) | 966 | + | ac.previousSession.Set(domain.SessionID("recent")) |
| 967 | tc.teardown(d, current, ac) | 967 | tc.teardown(d, current, ac) | ||
| 968 | 968 | ||||
| 969 | - | require.Nil(t, ac.previousSession.Get()) | 969 | + | require.Empty(t, ac.previousSession.Get()) |
| 970 | }) | 970 | }) | ||
| 971 | } | 971 | } | ||
| 972 | } | 972 | } | ||
M internal/usecase/daemon/palette_test.go +1 -1
| @@ -460,7 +460,7 @@ func TestPaletteFailedRoleHandoffClosesExecutedInteraction(t *testing.T) { | |||
| 460 | 460 | // Establish a valid back-session target, then remove that target immediately | |
| 461 | 461 | // after the command releases its role admission. | |
| 462 | 462 | target := d.sessions[domain.SessionID("recent")] | |
| 463 | - | ac.previousSession.Set(target) | |
| 463 | + | ac.previousSession.Set(target.id) | |
| 464 | 464 | invalidations := installPaletteInvalidationObserver(current) | |
| 465 | 465 | d.enterPalette(current, ac) | |
| 466 | 466 | d.handlePaletteInput(ac, []byte("BSK")) | |
| @@ -460,7 +460,7 @@ func TestPaletteFailedRoleHandoffClosesExecutedInteraction(t *testing.T) { | |||||
| 460 | // Establish a valid back-session target, then remove that target immediately | 460 | // Establish a valid back-session target, then remove that target immediately | ||
| 461 | // after the command releases its role admission. | 461 | // after the command releases its role admission. | ||
| 462 | target := d.sessions[domain.SessionID("recent")] | 462 | target := d.sessions[domain.SessionID("recent")] | ||
| 463 | - | ac.previousSession.Set(target) | 463 | + | ac.previousSession.Set(target.id) |
| 464 | invalidations := installPaletteInvalidationObserver(current) | 464 | invalidations := installPaletteInvalidationObserver(current) | ||
| 465 | d.enterPalette(current, ac) | 465 | d.enterPalette(current, ac) | ||
| 466 | d.handlePaletteInput(ac, []byte("BSK")) | 466 | d.handlePaletteInput(ac, []byte("BSK")) | ||
M internal/usecase/daemon/picker.go +16 -3
| @@ -378,8 +378,21 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||
| 378 | 378 | backNavigationSent := false | |
| 379 | 379 | if result.exit && !committing { | |
| 380 | 380 | back := ac.startupOverlay == ports.StartupOverlaySessionPicker && ac.navigationCapabilities&ports.NavigationCapabilityBack != 0 | |
| 381 | - | if back && len(effects) != 0 && effects[0] != nil { | |
| 382 | - | if err := d.sendNavigationActionForAttachment(effects[0].connectionToken(), ports.NavigationBack); err != nil { | |
| 381 | + | if back { | |
| 382 | + | var token attachmentConnectionToken | |
| 383 | + | if len(effects) != 0 && effects[0] != nil { | |
| 384 | + | token = effects[0].connectionToken() | |
| 385 | + | } else { | |
| 386 | + | var effect *attachmentEffectTicket | |
| 387 | + | var admitted bool | |
| 388 | + | token, effect, admitted = ac.beginCurrentAttachmentEffect(sess, ac.transport()) | |
| 389 | + | if !admitted { | |
| 390 | + | d.invalidateRender(sess, ac, true, "picker.go") | |
| 391 | + | return | |
| 392 | + | } | |
| 393 | + | defer effect.End() | |
| 394 | + | } | |
| 395 | + | if err := d.sendNavigationActionForAttachment(token, ports.NavigationBack); err != nil { | |
| 383 | 396 | d.reportAttachmentError(sess, err) | |
| 384 | 397 | d.invalidateRender(sess, ac, true, "picker.go") | |
| 385 | 398 | return | |
| @@ -925,7 +938,7 @@ func (d *Daemon) sendRemoteAttachTargetForAttachment(token attachmentConnectionT | |||
| 925 | 938 | return failUnavailable() | |
| 926 | 939 | } | |
| 927 | 940 | if err := token.sendControl(ports.Frame{Type: ports.MsgAttachTarget, Payload: payload}); err != nil { | |
| 928 | - | return err | |
| 941 | + | return domain.UserErr(domain.NoticeSessionUnavailable, "couldn't attach to remote session", err) | |
| 929 | 942 | } | |
| 930 | 943 | if guard.closePicker { | |
| 931 | 944 | d.closePicker(token.ac) | |
| @@ -378,8 +378,21 @@ func (d *Daemon) handlePickerInput(ac *attachedClient, data []byte, effects ...* | |||||
| 378 | backNavigationSent := false | 378 | backNavigationSent := false | ||
| 379 | if result.exit && !committing { | 379 | if result.exit && !committing { | ||
| 380 | back := ac.startupOverlay == ports.StartupOverlaySessionPicker && ac.navigationCapabilities&ports.NavigationCapabilityBack != 0 | 380 | back := ac.startupOverlay == ports.StartupOverlaySessionPicker && ac.navigationCapabilities&ports.NavigationCapabilityBack != 0 | ||
| 381 | - | if back && len(effects) != 0 && effects[0] != nil { | 381 | + | if back { |
| 382 | - | if err := d.sendNavigationActionForAttachment(effects[0].connectionToken(), ports.NavigationBack); err != nil { | 382 | + | var token attachmentConnectionToken |
| 383 | + | if len(effects) != 0 && effects[0] != nil { | |||
| 384 | + | token = effects[0].connectionToken() | |||
| 385 | + | } else { | |||
| 386 | + | var effect *attachmentEffectTicket | |||
| 387 | + | var admitted bool | |||
| 388 | + | token, effect, admitted = ac.beginCurrentAttachmentEffect(sess, ac.transport()) | |||
| 389 | + | if !admitted { | |||
| 390 | + | d.invalidateRender(sess, ac, true, "picker.go") | |||
| 391 | + | return | |||
| 392 | + | } | |||
| 393 | + | defer effect.End() | |||
| 394 | + | } | |||
| 395 | + | if err := d.sendNavigationActionForAttachment(token, ports.NavigationBack); err != nil { | |||
| 383 | d.reportAttachmentError(sess, err) | 396 | d.reportAttachmentError(sess, err) | ||
| 384 | d.invalidateRender(sess, ac, true, "picker.go") | 397 | d.invalidateRender(sess, ac, true, "picker.go") | ||
| 385 | return | 398 | return | ||
| @@ -925,7 +938,7 @@ func (d *Daemon) sendRemoteAttachTargetForAttachment(token attachmentConnectionT | |||||
| 925 | return failUnavailable() | 938 | return failUnavailable() | ||
| 926 | } | 939 | } | ||
| 927 | if err := token.sendControl(ports.Frame{Type: ports.MsgAttachTarget, Payload: payload}); err != nil { | 940 | if err := token.sendControl(ports.Frame{Type: ports.MsgAttachTarget, Payload: payload}); err != nil { | ||
| 928 | - | return err | 941 | + | return domain.UserErr(domain.NoticeSessionUnavailable, "couldn't attach to remote session", err) |
| 929 | } | 942 | } | ||
| 930 | if guard.closePicker { | 943 | if guard.closePicker { | ||
| 931 | d.closePicker(token.ac) | 944 | d.closePicker(token.ac) | ||
M internal/usecase/daemon/picker_test.go +30 -1
| @@ -908,6 +908,35 @@ func TestPickerNavigationBackSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 908 | 908 | require.Contains(t, history[len(history)-1].Details, sendErr) | |
| 909 | 909 | } | |
| 910 | 910 | ||
| 911 | + | func TestPickerNavigationBackWithoutCallerTicketKeepsPickerOpen(t *testing.T) { | |
| 912 | + | for _, tc := range []struct { | |
| 913 | + | name string | |
| 914 | + | passNil bool | |
| 915 | + | }{ | |
| 916 | + | {name: "empty effects"}, | |
| 917 | + | {name: "nil first effect", passNil: true}, | |
| 918 | + | } { | |
| 919 | + | t.Run(tc.name, func(t *testing.T) { | |
| 920 | + | d, sess, ac, _, releases := newManualTabSession(t, 1) | |
| 921 | + | defer releases[0]() | |
| 922 | + | ac.startupOverlay = ports.StartupOverlaySessionPicker | |
| 923 | + | ac.navigationCapabilities = ports.NavigationCapabilityBack | |
| 924 | + | ac.overlays.pickerMu.Lock() | |
| 925 | + | ac.overlays.picker = picker.New([]picker.SessionView{{ID: sess.id, Name: sess.name, Tabs: []picker.TabEntry{{Name: "tab"}}, Active: 0}}, picker.SelectionConfig{}) | |
| 926 | + | ac.overlays.pickerGeneration++ | |
| 927 | + | ac.overlays.pickerMu.Unlock() | |
| 928 | + | ||
| 929 | + | if tc.passNil { | |
| 930 | + | d.handlePickerInput(ac, []byte{3}, nil) | |
| 931 | + | } else { | |
| 932 | + | d.handlePickerInput(ac, []byte{3}) | |
| 933 | + | } | |
| 934 | + | ||
| 935 | + | require.True(t, ac.overlays.pickerActive(), "back with missing effect must keep picker open when fresh admission is unavailable") | |
| 936 | + | }) | |
| 937 | + | } | |
| 938 | + | } | |
| 939 | + | ||
| 911 | 940 | func TestPickerLoneEscapeExitsAfterDelay(t *testing.T) { | |
| 912 | 941 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | |
| 913 | 942 | defer releases[0]() | |
| @@ -950,7 +979,7 @@ func TestBackSessionFirstResetDoesNotReuseSamePaneIDCapture(t *testing.T) { | |||
| 950 | 979 | targetPane.screen.Write([]byte("TARGET")) | |
| 951 | 980 | targetPane.screen.ClearDamage() // TARGET is already rendered and has no pending VT damage. | |
| 952 | 981 | require.Empty(t, targetPane.screen.Damage(), "target deliberately has no pending VT damage") | |
| 953 | - | ac.previousSession.Set(target) | |
| 982 | + | ac.previousSession.Set(target.id) | |
| 954 | 983 | ||
| 955 | 984 | // Exercise the user-facing previous-session route, which delegates to the | |
| 956 | 985 | // real switchToTarget hand-off and immediately emits its required reset. | |
| @@ -908,6 +908,35 @@ func TestPickerNavigationBackSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 908 | require.Contains(t, history[len(history)-1].Details, sendErr) | 908 | require.Contains(t, history[len(history)-1].Details, sendErr) | ||
| 909 | } | 909 | } | ||
| 910 | 910 | ||||
| 911 | + | func TestPickerNavigationBackWithoutCallerTicketKeepsPickerOpen(t *testing.T) { | |||
| 912 | + | for _, tc := range []struct { | |||
| 913 | + | name string | |||
| 914 | + | passNil bool | |||
| 915 | + | }{ | |||
| 916 | + | {name: "empty effects"}, | |||
| 917 | + | {name: "nil first effect", passNil: true}, | |||
| 918 | + | } { | |||
| 919 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 920 | + | d, sess, ac, _, releases := newManualTabSession(t, 1) | |||
| 921 | + | defer releases[0]() | |||
| 922 | + | ac.startupOverlay = ports.StartupOverlaySessionPicker | |||
| 923 | + | ac.navigationCapabilities = ports.NavigationCapabilityBack | |||
| 924 | + | ac.overlays.pickerMu.Lock() | |||
| 925 | + | ac.overlays.picker = picker.New([]picker.SessionView{{ID: sess.id, Name: sess.name, Tabs: []picker.TabEntry{{Name: "tab"}}, Active: 0}}, picker.SelectionConfig{}) | |||
| 926 | + | ac.overlays.pickerGeneration++ | |||
| 927 | + | ac.overlays.pickerMu.Unlock() | |||
| 928 | + | ||||
| 929 | + | if tc.passNil { | |||
| 930 | + | d.handlePickerInput(ac, []byte{3}, nil) | |||
| 931 | + | } else { | |||
| 932 | + | d.handlePickerInput(ac, []byte{3}) | |||
| 933 | + | } | |||
| 934 | + | ||||
| 935 | + | require.True(t, ac.overlays.pickerActive(), "back with missing effect must keep picker open when fresh admission is unavailable") | |||
| 936 | + | }) | |||
| 937 | + | } | |||
| 938 | + | } | |||
| 939 | + | ||||
| 911 | func TestPickerLoneEscapeExitsAfterDelay(t *testing.T) { | 940 | func TestPickerLoneEscapeExitsAfterDelay(t *testing.T) { | ||
| 912 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | 941 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | ||
| 913 | defer releases[0]() | 942 | defer releases[0]() | ||
| @@ -950,7 +979,7 @@ func TestBackSessionFirstResetDoesNotReuseSamePaneIDCapture(t *testing.T) { | |||||
| 950 | targetPane.screen.Write([]byte("TARGET")) | 979 | targetPane.screen.Write([]byte("TARGET")) | ||
| 951 | targetPane.screen.ClearDamage() // TARGET is already rendered and has no pending VT damage. | 980 | targetPane.screen.ClearDamage() // TARGET is already rendered and has no pending VT damage. | ||
| 952 | require.Empty(t, targetPane.screen.Damage(), "target deliberately has no pending VT damage") | 981 | require.Empty(t, targetPane.screen.Damage(), "target deliberately has no pending VT damage") | ||
| 953 | - | ac.previousSession.Set(target) | 982 | + | ac.previousSession.Set(target.id) |
| 954 | 983 | ||||
| 955 | // Exercise the user-facing previous-session route, which delegates to the | 984 | // Exercise the user-facing previous-session route, which delegates to the | ||
| 956 | // real switchToTarget hand-off and immediately emits its required reset. | 985 | // real switchToTarget hand-off and immediately emits its required reset. | ||
M internal/usecase/daemon/remote_picker.go +1 -1
| @@ -130,7 +130,7 @@ func remoteSessionOrigin(host string) string { | |||
| 130 | 130 | } | |
| 131 | 131 | ||
| 132 | 132 | func remoteSessionStateStopped(state string) bool { | |
| 133 | - | return state == "down" | |
| 133 | + | return state == "down" || state == "stopped" | |
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | 136 | func remotePickerView(key domain.RemoteSessionKey, session ports.RemoteCatalogSession, status remoteHostStatus, fetchedAt time.Time) picker.SessionView { | |
| @@ -130,7 +130,7 @@ func remoteSessionOrigin(host string) string { | |||||
| 130 | } | 130 | } | ||
| 131 | 131 | ||||
| 132 | func remoteSessionStateStopped(state string) bool { | 132 | func remoteSessionStateStopped(state string) bool { | ||
| 133 | - | return state == "down" | 133 | + | return state == "down" || state == "stopped" |
| 134 | } | 134 | } | ||
| 135 | 135 | ||||
| 136 | func remotePickerView(key domain.RemoteSessionKey, session ports.RemoteCatalogSession, status remoteHostStatus, fetchedAt time.Time) picker.SessionView { | 136 | func remotePickerView(key domain.RemoteSessionKey, session ports.RemoteCatalogSession, status remoteHostStatus, fetchedAt time.Time) picker.SessionView { | ||
M internal/usecase/daemon/remote_picker_test.go +36 -29
| @@ -56,24 +56,28 @@ func newRemotePickerDaemon(store ports.RemoteHostStore) *Daemon { | |||
| 56 | 56 | } | |
| 57 | 57 | ||
| 58 | 58 | func TestRemotePickerStoppedRowsUseCanonicalStateAndSafeSelection(t *testing.T) { | |
| 59 | - | lifecycle := remoteLifecycleForTest() | |
| 60 | - | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle} | |
| 61 | - | session := ports.RemoteCatalogSession{ | |
| 62 | - | LifecycleID: lifecycle, Name: "work", State: "down", | |
| 63 | - | Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 59 | + | for _, state := range []string{"down", "stopped"} { | |
| 60 | + | t.Run(state, func(t *testing.T) { | |
| 61 | + | lifecycle := remoteLifecycleForTest() | |
| 62 | + | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle} | |
| 63 | + | session := ports.RemoteCatalogSession{ | |
| 64 | + | LifecycleID: lifecycle, Name: "work", State: state, | |
| 65 | + | Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |
| 66 | + | } | |
| 67 | + | view := remotePickerView(key, session, remoteHostFresh, time.Unix(100, 0)) | |
| 68 | + | require.True(t, view.Stopped) | |
| 69 | + | require.NotNil(t, view.RemoteTarget) | |
| 70 | + | require.True(t, view.RemoteTarget.Stopped) | |
| 71 | + | require.True(t, view.RemoteAttachReady, "structured stopped rows have an explicit safe restore target") | |
| 72 | + | ||
| 73 | + | legacy := session | |
| 74 | + | legacy.LifecycleID = domain.SessionLifecycleID{} | |
| 75 | + | legacyView := remotePickerView(domain.RemoteSessionKey{Host: "arch", Name: "work"}, legacy, remoteHostFresh, time.Unix(100, 0)) | |
| 76 | + | require.True(t, legacyView.Stopped) | |
| 77 | + | require.True(t, legacyView.ConnectOnly) | |
| 78 | + | require.False(t, legacyView.RemoteAttachReady, "legacy stopped rows cannot be selected without lifecycle identity") | |
| 79 | + | }) | |
| 64 | 80 | } | |
| 65 | - | live := remotePickerView(key, session, remoteHostFresh, time.Unix(100, 0)) | |
| 66 | - | require.True(t, live.Stopped) | |
| 67 | - | require.NotNil(t, live.RemoteTarget) | |
| 68 | - | require.True(t, live.RemoteTarget.Stopped) | |
| 69 | - | require.True(t, live.RemoteAttachReady, "structured stopped rows have an explicit safe restore target") | |
| 70 | - | ||
| 71 | - | legacy := session | |
| 72 | - | legacy.LifecycleID = domain.SessionLifecycleID{} | |
| 73 | - | legacyView := remotePickerView(domain.RemoteSessionKey{Host: "arch", Name: "work"}, legacy, remoteHostFresh, time.Unix(100, 0)) | |
| 74 | - | require.True(t, legacyView.Stopped) | |
| 75 | - | require.True(t, legacyView.ConnectOnly) | |
| 76 | - | require.False(t, legacyView.RemoteAttachReady, "legacy stopped rows cannot be selected without lifecycle identity") | |
| 77 | 81 | } | |
| 78 | 82 | ||
| 79 | 83 | func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | |
| @@ -673,15 +677,9 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 673 | 677 | d.remoteCatalog.mu.Lock() | |
| 674 | 678 | d.remoteCatalog.status["arch"] = remoteHostFresh | |
| 675 | 679 | d.remoteCatalog.mu.Unlock() | |
| 676 | - | const sendErr = "remote attach send failed" | |
| 677 | - | tr := &remotePickerSendErrorTransport{err: errors.New(sendErr)} | |
| 678 | - | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 679 | - | ac.initOverlays() | |
| 680 | - | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 681 | - | sess := &session{sessionCore: sessionCore{id: "local", name: "local", attachments: map[*attachedClient]struct{}{ac: {}}}, tabs: []*tab{tb}} | |
| 682 | - | publishTiledPaneOwners(sess, tb) | |
| 683 | - | ac.setSession(sess) | |
| 684 | - | d.sessions[sess.id] = sess | |
| 680 | + | cause := errors.New("remote attach send failed") | |
| 681 | + | tr := &remotePickerSendErrorTransport{err: cause} | |
| 682 | + | sess, ac, _ := addRemoteRefreshPickerOwner(t, d, "local", tr) | |
| 685 | 683 | ac.overlays.pickerMu.Lock() | |
| 686 | 684 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | |
| 687 | 685 | ac.overlays.pickerGeneration++ | |
| @@ -697,7 +695,10 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||
| 697 | 695 | key := domain.RemoteSessionKey{Host: "arch", Name: "work"} | |
| 698 | 696 | target := picker.Target{Session: key.ID(), RemoteKey: &key} | |
| 699 | 697 | err := d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{closePicker: true}, "picker-select") | |
| 700 | - | require.EqualError(t, err, sendErr) | |
| 698 | + | var userErr *domain.UserError | |
| 699 | + | require.ErrorAs(t, err, &userErr) | |
| 700 | + | require.Equal(t, "couldn't attach to remote session", userErr.Msg) | |
| 701 | + | require.ErrorIs(t, err, cause) | |
| 701 | 702 | require.True(t, ac.overlays.pickerActive(), "failed control send must leave the picker open") | |
| 702 | 703 | select { | |
| 703 | 704 | case <-gone: | |
| @@ -715,9 +716,15 @@ func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.er | |||
| 715 | 716 | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | |
| 716 | 717 | func (*remotePickerSendErrorTransport) Close() error { return nil } | |
| 717 | 718 | ||
| 718 | - | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID) (*session, *attachedClient, chan ports.Frame) { | |
| 719 | + | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID, transports ...ports.Transport) (*session, *attachedClient, chan ports.Frame) { | |
| 719 | 720 | t.Helper() | |
| 720 | - | tr, sends := newCapturingTransport(t) | |
| 721 | + | var tr ports.Transport | |
| 722 | + | var sends chan ports.Frame | |
| 723 | + | if len(transports) != 0 { | |
| 724 | + | tr = transports[0] | |
| 725 | + | } else { | |
| 726 | + | tr, sends = newCapturingTransport(t) | |
| 727 | + | } | |
| 721 | 728 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | |
| 722 | 729 | ac.initOverlays() | |
| 723 | 730 | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| @@ -56,24 +56,28 @@ func newRemotePickerDaemon(store ports.RemoteHostStore) *Daemon { | |||||
| 56 | } | 56 | } | ||
| 57 | 57 | ||||
| 58 | func TestRemotePickerStoppedRowsUseCanonicalStateAndSafeSelection(t *testing.T) { | 58 | func TestRemotePickerStoppedRowsUseCanonicalStateAndSafeSelection(t *testing.T) { | ||
| 59 | - | lifecycle := remoteLifecycleForTest() | 59 | + | for _, state := range []string{"down", "stopped"} { |
| 60 | - | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle} | 60 | + | t.Run(state, func(t *testing.T) { |
| 61 | - | session := ports.RemoteCatalogSession{ | 61 | + | lifecycle := remoteLifecycleForTest() |
| 62 | - | LifecycleID: lifecycle, Name: "work", State: "down", | 62 | + | key := domain.RemoteSessionKey{Host: "arch", Name: "work", LifecycleID: lifecycle} |
| 63 | - | Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | 63 | + | session := ports.RemoteCatalogSession{ |
| 64 | + | LifecycleID: lifecycle, Name: "work", State: state, | |||
| 65 | + | Tabs: []ports.RemoteCatalogTab{{ID: "tab-1", Index: 0, Name: "main"}}, | |||
| 66 | + | } | |||
| 67 | + | view := remotePickerView(key, session, remoteHostFresh, time.Unix(100, 0)) | |||
| 68 | + | require.True(t, view.Stopped) | |||
| 69 | + | require.NotNil(t, view.RemoteTarget) | |||
| 70 | + | require.True(t, view.RemoteTarget.Stopped) | |||
| 71 | + | require.True(t, view.RemoteAttachReady, "structured stopped rows have an explicit safe restore target") | |||
| 72 | + | ||||
| 73 | + | legacy := session | |||
| 74 | + | legacy.LifecycleID = domain.SessionLifecycleID{} | |||
| 75 | + | legacyView := remotePickerView(domain.RemoteSessionKey{Host: "arch", Name: "work"}, legacy, remoteHostFresh, time.Unix(100, 0)) | |||
| 76 | + | require.True(t, legacyView.Stopped) | |||
| 77 | + | require.True(t, legacyView.ConnectOnly) | |||
| 78 | + | require.False(t, legacyView.RemoteAttachReady, "legacy stopped rows cannot be selected without lifecycle identity") | |||
| 79 | + | }) | |||
| 64 | } | 80 | } | ||
| 65 | - | live := remotePickerView(key, session, remoteHostFresh, time.Unix(100, 0)) | |||
| 66 | - | require.True(t, live.Stopped) | |||
| 67 | - | require.NotNil(t, live.RemoteTarget) | |||
| 68 | - | require.True(t, live.RemoteTarget.Stopped) | |||
| 69 | - | require.True(t, live.RemoteAttachReady, "structured stopped rows have an explicit safe restore target") | |||
| 70 | - | ||||
| 71 | - | legacy := session | |||
| 72 | - | legacy.LifecycleID = domain.SessionLifecycleID{} | |||
| 73 | - | legacyView := remotePickerView(domain.RemoteSessionKey{Host: "arch", Name: "work"}, legacy, remoteHostFresh, time.Unix(100, 0)) | |||
| 74 | - | require.True(t, legacyView.Stopped) | |||
| 75 | - | require.True(t, legacyView.ConnectOnly) | |||
| 76 | - | require.False(t, legacyView.RemoteAttachReady, "legacy stopped rows cannot be selected without lifecycle identity") | |||
| 77 | } | 81 | } | ||
| 78 | 82 | ||||
| 79 | func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | 83 | func TestRemotePickerCatalogExpiresAtAttachTTL(t *testing.T) { | ||
| @@ -673,15 +677,9 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 673 | d.remoteCatalog.mu.Lock() | 677 | d.remoteCatalog.mu.Lock() | ||
| 674 | d.remoteCatalog.status["arch"] = remoteHostFresh | 678 | d.remoteCatalog.status["arch"] = remoteHostFresh | ||
| 675 | d.remoteCatalog.mu.Unlock() | 679 | d.remoteCatalog.mu.Unlock() | ||
| 676 | - | const sendErr = "remote attach send failed" | 680 | + | cause := errors.New("remote attach send failed") |
| 677 | - | tr := &remotePickerSendErrorTransport{err: errors.New(sendErr)} | 681 | + | tr := &remotePickerSendErrorTransport{err: cause} |
| 678 | - | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | 682 | + | sess, ac, _ := addRemoteRefreshPickerOwner(t, d, "local", tr) |
| 679 | - | ac.initOverlays() | |||
| 680 | - | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 681 | - | sess := &session{sessionCore: sessionCore{id: "local", name: "local", attachments: map[*attachedClient]struct{}{ac: {}}}, tabs: []*tab{tb}} | |||
| 682 | - | publishTiledPaneOwners(sess, tb) | |||
| 683 | - | ac.setSession(sess) | |||
| 684 | - | d.sessions[sess.id] = sess | |||
| 685 | ac.overlays.pickerMu.Lock() | 683 | ac.overlays.pickerMu.Lock() | ||
| 686 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | 684 | ac.overlays.picker = picker.New(nil, picker.SelectionConfig{}) | ||
| 687 | ac.overlays.pickerGeneration++ | 685 | ac.overlays.pickerGeneration++ | ||
| @@ -697,7 +695,10 @@ func TestRemotePickerHandoffSendFailureKeepsPickerOpen(t *testing.T) { | |||||
| 697 | key := domain.RemoteSessionKey{Host: "arch", Name: "work"} | 695 | key := domain.RemoteSessionKey{Host: "arch", Name: "work"} | ||
| 698 | target := picker.Target{Session: key.ID(), RemoteKey: &key} | 696 | target := picker.Target{Session: key.ID(), RemoteKey: &key} | ||
| 699 | err := d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{closePicker: true}, "picker-select") | 697 | err := d.sendRemoteAttachTargetForAttachment(token, target, key, sessionHandoffGuard{closePicker: true}, "picker-select") | ||
| 700 | - | require.EqualError(t, err, sendErr) | 698 | + | var userErr *domain.UserError |
| 699 | + | require.ErrorAs(t, err, &userErr) | |||
| 700 | + | require.Equal(t, "couldn't attach to remote session", userErr.Msg) | |||
| 701 | + | require.ErrorIs(t, err, cause) | |||
| 701 | require.True(t, ac.overlays.pickerActive(), "failed control send must leave the picker open") | 702 | require.True(t, ac.overlays.pickerActive(), "failed control send must leave the picker open") | ||
| 702 | select { | 703 | select { | ||
| 703 | case <-gone: | 704 | case <-gone: | ||
| @@ -715,9 +716,15 @@ func (t *remotePickerSendErrorTransport) Send(ports.Frame) error { return t.er | |||||
| 715 | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | 716 | func (*remotePickerSendErrorTransport) Recv() (ports.Frame, error) { return ports.Frame{}, io.EOF } | ||
| 716 | func (*remotePickerSendErrorTransport) Close() error { return nil } | 717 | func (*remotePickerSendErrorTransport) Close() error { return nil } | ||
| 717 | 718 | ||||
| 718 | - | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID) (*session, *attachedClient, chan ports.Frame) { | 719 | + | func addRemoteRefreshPickerOwner(t *testing.T, d *Daemon, id domain.SessionID, transports ...ports.Transport) (*session, *attachedClient, chan ports.Frame) { |
| 719 | t.Helper() | 720 | t.Helper() | ||
| 720 | - | tr, sends := newCapturingTransport(t) | 721 | + | var tr ports.Transport |
| 722 | + | var sends chan ports.Frame | |||
| 723 | + | if len(transports) != 0 { | |||
| 724 | + | tr = transports[0] | |||
| 725 | + | } else { | |||
| 726 | + | tr, sends = newCapturingTransport(t) | |||
| 727 | + | } | |||
| 721 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | 728 | ac := &attachedClient{tr: tr, output: newOutputStateStream(), size: domain.Size{Cols: 80, Rows: 24}} | ||
| 722 | ac.initOverlays() | 729 | ac.initOverlays() | ||
| 723 | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | 730 | tb := newTab(newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | ||
M internal/usecase/daemon/remote_stopped_route_test.go +23 -12
| @@ -11,19 +11,30 @@ import ( | |||
| 11 | 11 | ) | |
| 12 | 12 | ||
| 13 | 13 | func TestDaemonOwnedNoExactTargetRejectsNewAndEphemeral(t *testing.T) { | |
| 14 | - | d := newTestDaemon(t, nil, stubClock{}) | |
| 15 | - | for _, intent := range []uint8{ports.IntentNew, ports.IntentEphemeral} { | |
| 16 | - | hello := ports.Hello{ | |
| 17 | - | Version: ports.ProtocolVersion, Intent: intent, Name: "work", | |
| 18 | - | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: "/untrusted/cwd", | |
| 19 | - | Env: []string{"UNTRUSTED=client"}, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 20 | - | } | |
| 21 | - | _, _, err := d.route(hello, &closeTrackingTransport{}) | |
| 22 | - | var protocol *protoErr | |
| 23 | - | require.ErrorAs(t, err, &protocol) | |
| 24 | - | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |
| 14 | + | for _, tt := range []struct { | |
| 15 | + | name string | |
| 16 | + | intent uint8 | |
| 17 | + | }{ | |
| 18 | + | {name: "new", intent: ports.IntentNew}, | |
| 19 | + | {name: "ephemeral", intent: ports.IntentEphemeral}, | |
| 20 | + | } { | |
| 21 | + | t.Run(tt.name, func(t *testing.T) { | |
| 22 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 23 | + | hello := ports.Hello{ | |
| 24 | + | Version: ports.ProtocolVersion, Intent: tt.intent, Name: "work", | |
| 25 | + | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: "/untrusted/cwd", | |
| 26 | + | Env: []string{"UNTRUSTED=client"}, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |
| 27 | + | } | |
| 28 | + | _, _, err := d.route(hello, &closeTrackingTransport{}) | |
| 29 | + | var protocol *protoErr | |
| 30 | + | require.ErrorAs(t, err, &protocol) | |
| 31 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |
| 32 | + | d.mu.Lock() | |
| 33 | + | sessions := len(d.sessions) | |
| 34 | + | d.mu.Unlock() | |
| 35 | + | require.Zero(t, sessions) | |
| 36 | + | }) | |
| 25 | 37 | } | |
| 26 | - | require.Empty(t, d.sessions) | |
| 27 | 38 | } | |
| 28 | 39 | ||
| 29 | 40 | func TestLegacyDaemonOwnedStoppedAttachUsesDaemonEnvironment(t *testing.T) { | |
| @@ -11,19 +11,30 @@ import ( | |||||
| 11 | ) | 11 | ) | ||
| 12 | 12 | ||||
| 13 | func TestDaemonOwnedNoExactTargetRejectsNewAndEphemeral(t *testing.T) { | 13 | func TestDaemonOwnedNoExactTargetRejectsNewAndEphemeral(t *testing.T) { | ||
| 14 | - | d := newTestDaemon(t, nil, stubClock{}) | 14 | + | for _, tt := range []struct { |
| 15 | - | for _, intent := range []uint8{ports.IntentNew, ports.IntentEphemeral} { | 15 | + | name string |
| 16 | - | hello := ports.Hello{ | 16 | + | intent uint8 |
| 17 | - | Version: ports.ProtocolVersion, Intent: intent, Name: "work", | 17 | + | }{ |
| 18 | - | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: "/untrusted/cwd", | 18 | + | {name: "new", intent: ports.IntentNew}, |
| 19 | - | Env: []string{"UNTRUSTED=client"}, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | 19 | + | {name: "ephemeral", intent: ports.IntentEphemeral}, |
| 20 | - | } | 20 | + | } { |
| 21 | - | _, _, err := d.route(hello, &closeTrackingTransport{}) | 21 | + | t.Run(tt.name, func(t *testing.T) { |
| 22 | - | var protocol *protoErr | 22 | + | d := newTestDaemon(t, nil, stubClock{}) |
| 23 | - | require.ErrorAs(t, err, &protocol) | 23 | + | hello := ports.Hello{ |
| 24 | - | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | 24 | + | Version: ports.ProtocolVersion, Intent: tt.intent, Name: "work", |
| 25 | + | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: "/untrusted/cwd", | |||
| 26 | + | Env: []string{"UNTRUSTED=client"}, EnvironmentPolicy: ports.EnvironmentPolicyDaemonOwned, | |||
| 27 | + | } | |||
| 28 | + | _, _, err := d.route(hello, &closeTrackingTransport{}) | |||
| 29 | + | var protocol *protoErr | |||
| 30 | + | require.ErrorAs(t, err, &protocol) | |||
| 31 | + | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |||
| 32 | + | d.mu.Lock() | |||
| 33 | + | sessions := len(d.sessions) | |||
| 34 | + | d.mu.Unlock() | |||
| 35 | + | require.Zero(t, sessions) | |||
| 36 | + | }) | |||
| 25 | } | 37 | } | ||
| 26 | - | require.Empty(t, d.sessions) | |||
| 27 | } | 38 | } | ||
| 28 | 39 | ||||
| 29 | func TestLegacyDaemonOwnedStoppedAttachUsesDaemonEnvironment(t *testing.T) { | 40 | func TestLegacyDaemonOwnedStoppedAttachUsesDaemonEnvironment(t *testing.T) { | ||
M internal/usecase/daemon/resume.go +2 -2
| @@ -107,8 +107,8 @@ func (d *Daemon) clearParkingInFlight(token uint64, ac *attachedClient) { | |||
| 107 | 107 | ||
| 108 | 108 | // clearParkingInFlightIfAbandoned drops a pre-detach parking marker when this | |
| 109 | 109 | // teardown lost the seat while still the live owner (stale transport fence). | |
| 110 | - | // If another path already detached/unrouted, that path | |
| 111 | - | // owns park publication. | |
| 110 | + | // The live owner clears its own marker; if another path already detached or | |
| 111 | + | // unrouted, that owning teardown is responsible for park publication/cleanup. | |
| 112 | 112 | func (d *Daemon) clearParkingInFlightIfAbandoned(sess *session, ac *attachedClient, token uint64) { | |
| 113 | 113 | if token == 0 || ac == nil { | |
| 114 | 114 | return | |
| @@ -107,8 +107,8 @@ func (d *Daemon) clearParkingInFlight(token uint64, ac *attachedClient) { | |||||
| 107 | 107 | ||||
| 108 | // clearParkingInFlightIfAbandoned drops a pre-detach parking marker when this | 108 | // clearParkingInFlightIfAbandoned drops a pre-detach parking marker when this | ||
| 109 | // teardown lost the seat while still the live owner (stale transport fence). | 109 | // teardown lost the seat while still the live owner (stale transport fence). | ||
| 110 | - | // If another path already detached/unrouted, that path | 110 | + | // The live owner clears its own marker; if another path already detached or |
| 111 | - | // owns park publication. | 111 | + | // unrouted, that owning teardown is responsible for park publication/cleanup. |
| 112 | func (d *Daemon) clearParkingInFlightIfAbandoned(sess *session, ac *attachedClient, token uint64) { | 112 | func (d *Daemon) clearParkingInFlightIfAbandoned(sess *session, ac *attachedClient, token uint64) { | ||
| 113 | if token == 0 || ac == nil { | 113 | if token == 0 || ac == nil { | ||
| 114 | return | 114 | return | ||
M internal/usecase/daemon/resume_test.go +12 -7
| @@ -160,9 +160,10 @@ func TestParkedResumeRejectsMismatchedRemoteTargetBeforeClaim(t *testing.T) { | |||
| 160 | 160 | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | |
| 161 | 161 | d.mu.Lock() | |
| 162 | 162 | parked := d.parked[token] | |
| 163 | + | claimed := parked != nil && parked.claimed | |
| 163 | 164 | d.mu.Unlock() | |
| 164 | 165 | require.NotNil(t, parked) | |
| 165 | - | require.False(t, parked.claimed, "mismatched target must not claim the parked attachment") | |
| 166 | + | require.False(t, claimed, "mismatched target must not claim the parked attachment") | |
| 166 | 167 | ||
| 167 | 168 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | |
| 168 | 169 | } | |
| @@ -200,9 +201,10 @@ func TestFailedResumeRearmsParkExpiryAfterClaimTimerFires(t *testing.T) { | |||
| 200 | 201 | require.NotSame(t, parkTimer, rearmedTimer) | |
| 201 | 202 | d.mu.Lock() | |
| 202 | 203 | rearmed := d.parked[token] | |
| 204 | + | claimed := rearmed != nil && rearmed.claimed | |
| 203 | 205 | d.mu.Unlock() | |
| 204 | 206 | require.NotNil(t, rearmed) | |
| 205 | - | require.False(t, rearmed.claimed) | |
| 207 | + | require.False(t, claimed) | |
| 206 | 208 | rearmedTimer.ch <- time.Time{} | |
| 207 | 209 | d.expireParked(token, rearmed) | |
| 208 | 210 | d.mu.Lock() | |
| @@ -1338,7 +1340,10 @@ func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |||
| 1338 | 1340 | require.NoError(t, err) | |
| 1339 | 1341 | ||
| 1340 | 1342 | previous := &session{sessionCore: sessionCore{id: "previous"}} | |
| 1341 | - | ac.previousSession.Set(previous) | |
| 1343 | + | d.mu.Lock() | |
| 1344 | + | d.sessions[previous.id] = previous | |
| 1345 | + | d.mu.Unlock() | |
| 1346 | + | ac.previousSession.Set(previous.id) | |
| 1342 | 1347 | d.clientGone(sess, ac, ac.transport(), false) | |
| 1343 | 1348 | ||
| 1344 | 1349 | token := ac.resumeToken | |
| @@ -1346,14 +1351,14 @@ func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |||
| 1346 | 1351 | parked := d.parked[token] | |
| 1347 | 1352 | d.mu.Unlock() | |
| 1348 | 1353 | require.NotNil(t, parked) | |
| 1349 | - | require.Same(t, previous, ac.previousSession.Get(), "a live parked attachment keeps its toggle") | |
| 1354 | + | require.Equal(t, previous.id, ac.previousSession.Get(), "a live parked attachment keeps its toggle") | |
| 1350 | 1355 | ||
| 1351 | 1356 | tr2, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |
| 1352 | 1357 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), tr2) | |
| 1353 | 1358 | require.NoError(t, err) | |
| 1354 | 1359 | require.Same(t, sess, resumedSess) | |
| 1355 | 1360 | require.Same(t, ac, resumedAC) | |
| 1356 | - | require.Same(t, previous, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") | |
| 1361 | + | require.Equal(t, previous.id, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") | |
| 1357 | 1362 | } | |
| 1358 | 1363 | ||
| 1359 | 1364 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |
| @@ -1383,7 +1388,7 @@ func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |||
| 1383 | 1388 | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), tr) | |
| 1384 | 1389 | require.NoError(t, err) | |
| 1385 | 1390 | ||
| 1386 | - | ac.previousSession.Set(&session{sessionCore: sessionCore{id: "previous"}}) | |
| 1391 | + | ac.previousSession.Set("previous") | |
| 1387 | 1392 | d.clientGone(sess, ac, ac.transport(), false) | |
| 1388 | 1393 | ||
| 1389 | 1394 | d.mu.Lock() | |
| @@ -1393,7 +1398,7 @@ func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |||
| 1393 | 1398 | d.mu.Unlock() | |
| 1394 | 1399 | d.finishParkedAttachmentRetirements(retirements) | |
| 1395 | 1400 | ||
| 1396 | - | require.Nil(t, ac.previousSession.Get()) | |
| 1401 | + | require.Empty(t, ac.previousSession.Get()) | |
| 1397 | 1402 | }) | |
| 1398 | 1403 | } | |
| 1399 | 1404 | } | |
| @@ -160,9 +160,10 @@ func TestParkedResumeRejectsMismatchedRemoteTargetBeforeClaim(t *testing.T) { | |||||
| 160 | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | 160 | require.Equal(t, ports.ErrNoSuchTarget, protocol.code) | ||
| 161 | d.mu.Lock() | 161 | d.mu.Lock() | ||
| 162 | parked := d.parked[token] | 162 | parked := d.parked[token] | ||
| 163 | + | claimed := parked != nil && parked.claimed | |||
| 163 | d.mu.Unlock() | 164 | d.mu.Unlock() | ||
| 164 | require.NotNil(t, parked) | 165 | require.NotNil(t, parked) | ||
| 165 | - | require.False(t, parked.claimed, "mismatched target must not claim the parked attachment") | 166 | + | require.False(t, claimed, "mismatched target must not claim the parked attachment") |
| 166 | 167 | ||||
| 167 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | 168 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, true)) | ||
| 168 | } | 169 | } | ||
| @@ -200,9 +201,10 @@ func TestFailedResumeRearmsParkExpiryAfterClaimTimerFires(t *testing.T) { | |||||
| 200 | require.NotSame(t, parkTimer, rearmedTimer) | 201 | require.NotSame(t, parkTimer, rearmedTimer) | ||
| 201 | d.mu.Lock() | 202 | d.mu.Lock() | ||
| 202 | rearmed := d.parked[token] | 203 | rearmed := d.parked[token] | ||
| 204 | + | claimed := rearmed != nil && rearmed.claimed | |||
| 203 | d.mu.Unlock() | 205 | d.mu.Unlock() | ||
| 204 | require.NotNil(t, rearmed) | 206 | require.NotNil(t, rearmed) | ||
| 205 | - | require.False(t, rearmed.claimed) | 207 | + | require.False(t, claimed) |
| 206 | rearmedTimer.ch <- time.Time{} | 208 | rearmedTimer.ch <- time.Time{} | ||
| 207 | d.expireParked(token, rearmed) | 209 | d.expireParked(token, rearmed) | ||
| 208 | d.mu.Lock() | 210 | d.mu.Lock() | ||
| @@ -1338,7 +1340,10 @@ func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |||||
| 1338 | require.NoError(t, err) | 1340 | require.NoError(t, err) | ||
| 1339 | 1341 | ||||
| 1340 | previous := &session{sessionCore: sessionCore{id: "previous"}} | 1342 | previous := &session{sessionCore: sessionCore{id: "previous"}} | ||
| 1341 | - | ac.previousSession.Set(previous) | 1343 | + | d.mu.Lock() |
| 1344 | + | d.sessions[previous.id] = previous | |||
| 1345 | + | d.mu.Unlock() | |||
| 1346 | + | ac.previousSession.Set(previous.id) | |||
| 1342 | d.clientGone(sess, ac, ac.transport(), false) | 1347 | d.clientGone(sess, ac, ac.transport(), false) | ||
| 1343 | 1348 | ||||
| 1344 | token := ac.resumeToken | 1349 | token := ac.resumeToken | ||
| @@ -1346,14 +1351,14 @@ func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |||||
| 1346 | parked := d.parked[token] | 1351 | parked := d.parked[token] | ||
| 1347 | d.mu.Unlock() | 1352 | d.mu.Unlock() | ||
| 1348 | require.NotNil(t, parked) | 1353 | require.NotNil(t, parked) | ||
| 1349 | - | require.Same(t, previous, ac.previousSession.Get(), "a live parked attachment keeps its toggle") | 1354 | + | require.Equal(t, previous.id, ac.previousSession.Get(), "a live parked attachment keeps its toggle") |
| 1350 | 1355 | ||||
| 1351 | tr2, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | 1356 | tr2, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | ||
| 1352 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), tr2) | 1357 | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), tr2) | ||
| 1353 | require.NoError(t, err) | 1358 | require.NoError(t, err) | ||
| 1354 | require.Same(t, sess, resumedSess) | 1359 | require.Same(t, sess, resumedSess) | ||
| 1355 | require.Same(t, ac, resumedAC) | 1360 | require.Same(t, ac, resumedAC) | ||
| 1356 | - | require.Same(t, previous, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") | 1361 | + | require.Equal(t, previous.id, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") |
| 1357 | } | 1362 | } | ||
| 1358 | 1363 | ||||
| 1359 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | 1364 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | ||
| @@ -1383,7 +1388,7 @@ func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |||||
| 1383 | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), tr) | 1388 | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), tr) | ||
| 1384 | require.NoError(t, err) | 1389 | require.NoError(t, err) | ||
| 1385 | 1390 | ||||
| 1386 | - | ac.previousSession.Set(&session{sessionCore: sessionCore{id: "previous"}}) | 1391 | + | ac.previousSession.Set("previous") |
| 1387 | d.clientGone(sess, ac, ac.transport(), false) | 1392 | d.clientGone(sess, ac, ac.transport(), false) | ||
| 1388 | 1393 | ||||
| 1389 | d.mu.Lock() | 1394 | d.mu.Lock() | ||
| @@ -1393,7 +1398,7 @@ func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |||||
| 1393 | d.mu.Unlock() | 1398 | d.mu.Unlock() | ||
| 1394 | d.finishParkedAttachmentRetirements(retirements) | 1399 | d.finishParkedAttachmentRetirements(retirements) | ||
| 1395 | 1400 | ||||
| 1396 | - | require.Nil(t, ac.previousSession.Get()) | 1401 | + | require.Empty(t, ac.previousSession.Get()) |
| 1397 | }) | 1402 | }) | ||
| 1398 | } | 1403 | } | ||
| 1399 | } | 1404 | } | ||
M internal/usecase/daemon/session_back.go +14 -12
| @@ -11,12 +11,13 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||
| 11 | 11 | if d == nil || token.sess == nil || token.ac == nil { | |
| 12 | 12 | return nil | |
| 13 | 13 | } | |
| 14 | - | target := token.ac.previousSession.Get() | |
| 15 | - | if target == nil || target == token.sess || target.core() == nil || d.sessionByID(target.core().id) != target { | |
| 16 | - | token.ac.clearPreviousSessionIf(target) | |
| 14 | + | targetID := token.ac.previousSession.Get() | |
| 15 | + | target := d.sessionByID(targetID) | |
| 16 | + | if target == nil || target == token.sess { | |
| 17 | + | token.ac.clearPreviousSessionIf(targetID) | |
| 17 | 18 | return nil | |
| 18 | 19 | } | |
| 19 | - | pickerTarget := picker.Target{Session: target.core().id, TabIndex: -1} | |
| 20 | + | pickerTarget := picker.Target{Session: targetID, TabIndex: -1} | |
| 20 | 21 | return d.switchToTargetForAttachment(token, pickerTarget, sessionHandoffGuard{}, "back-session") | |
| 21 | 22 | } | |
| 22 | 23 | ||
| @@ -24,9 +25,10 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||
| 24 | 25 | if d == nil || current == nil || ac == nil { | |
| 25 | 26 | return | |
| 26 | 27 | } | |
| 27 | - | target := ac.previousSession.Get() | |
| 28 | - | if target == nil || target == current || target.core() == nil || d.sessionByID(target.core().id) != target { | |
| 29 | - | ac.clearPreviousSessionIf(target) | |
| 28 | + | targetID := ac.previousSession.Get() | |
| 29 | + | target := d.sessionByID(targetID) | |
| 30 | + | if target == nil || target == current { | |
| 31 | + | ac.clearPreviousSessionIf(targetID) | |
| 30 | 32 | d.invalidateRender(current, ac, true, "session_back.go") | |
| 31 | 33 | return | |
| 32 | 34 | } | |
| @@ -45,13 +47,13 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||
| 45 | 47 | ||
| 46 | 48 | // clearPreviousSessionIf clears target only if it has not been replaced since | |
| 47 | 49 | // the caller observed it. This preserves a concurrent successful hand-off. | |
| 48 | - | func (ac *attachedClient) clearPreviousSessionIf(target *session) { | |
| 50 | + | func (ac *attachedClient) clearPreviousSessionIf(targetID domain.SessionID) { | |
| 49 | 51 | if ac == nil { | |
| 50 | 52 | return | |
| 51 | 53 | } | |
| 52 | - | ac.previousSession.With(func(previous **session) { | |
| 53 | - | if *previous == target { | |
| 54 | - | *previous = nil | |
| 54 | + | ac.previousSession.With(func(previous *domain.SessionID) { | |
| 55 | + | if *previous == targetID { | |
| 56 | + | *previous = "" | |
| 55 | 57 | } | |
| 56 | 58 | }) | |
| 57 | 59 | } | |
| @@ -61,6 +63,6 @@ func (ac *attachedClient) clearPreviousSessionIf(target *session) { | |||
| 61 | 63 | // leave the toggle pair intact. | |
| 62 | 64 | func (ac *attachedClient) recordPreviousSession(origin *session) { | |
| 63 | 65 | if ac != nil && origin != nil { | |
| 64 | - | ac.previousSession.Set(origin) | |
| 66 | + | ac.previousSession.Set(origin.id) | |
| 65 | 67 | } | |
| 66 | 68 | } | |
| @@ -11,12 +11,13 @@ func (d *Daemon) backSessionForAttachment(token attachmentConnectionToken) error | |||||
| 11 | if d == nil || token.sess == nil || token.ac == nil { | 11 | if d == nil || token.sess == nil || token.ac == nil { | ||
| 12 | return nil | 12 | return nil | ||
| 13 | } | 13 | } | ||
| 14 | - | target := token.ac.previousSession.Get() | 14 | + | targetID := token.ac.previousSession.Get() |
| 15 | - | if target == nil || target == token.sess || target.core() == nil || d.sessionByID(target.core().id) != target { | 15 | + | target := d.sessionByID(targetID) |
| 16 | - | token.ac.clearPreviousSessionIf(target) | 16 | + | if target == nil || target == token.sess { |
| 17 | + | token.ac.clearPreviousSessionIf(targetID) | |||
| 17 | return nil | 18 | return nil | ||
| 18 | } | 19 | } | ||
| 19 | - | pickerTarget := picker.Target{Session: target.core().id, TabIndex: -1} | 20 | + | pickerTarget := picker.Target{Session: targetID, TabIndex: -1} |
| 20 | return d.switchToTargetForAttachment(token, pickerTarget, sessionHandoffGuard{}, "back-session") | 21 | return d.switchToTargetForAttachment(token, pickerTarget, sessionHandoffGuard{}, "back-session") | ||
| 21 | } | 22 | } | ||
| 22 | 23 | ||||
| @@ -24,9 +25,10 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||||
| 24 | if d == nil || current == nil || ac == nil { | 25 | if d == nil || current == nil || ac == nil { | ||
| 25 | return | 26 | return | ||
| 26 | } | 27 | } | ||
| 27 | - | target := ac.previousSession.Get() | 28 | + | targetID := ac.previousSession.Get() |
| 28 | - | if target == nil || target == current || target.core() == nil || d.sessionByID(target.core().id) != target { | 29 | + | target := d.sessionByID(targetID) |
| 29 | - | ac.clearPreviousSessionIf(target) | 30 | + | if target == nil || target == current { |
| 31 | + | ac.clearPreviousSessionIf(targetID) | |||
| 30 | d.invalidateRender(current, ac, true, "session_back.go") | 32 | d.invalidateRender(current, ac, true, "session_back.go") | ||
| 31 | return | 33 | return | ||
| 32 | } | 34 | } | ||
| @@ -45,13 +47,13 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||||
| 45 | 47 | ||||
| 46 | // clearPreviousSessionIf clears target only if it has not been replaced since | 48 | // clearPreviousSessionIf clears target only if it has not been replaced since | ||
| 47 | // the caller observed it. This preserves a concurrent successful hand-off. | 49 | // the caller observed it. This preserves a concurrent successful hand-off. | ||
| 48 | - | func (ac *attachedClient) clearPreviousSessionIf(target *session) { | 50 | + | func (ac *attachedClient) clearPreviousSessionIf(targetID domain.SessionID) { |
| 49 | if ac == nil { | 51 | if ac == nil { | ||
| 50 | return | 52 | return | ||
| 51 | } | 53 | } | ||
| 52 | - | ac.previousSession.With(func(previous **session) { | 54 | + | ac.previousSession.With(func(previous *domain.SessionID) { |
| 53 | - | if *previous == target { | 55 | + | if *previous == targetID { |
| 54 | - | *previous = nil | 56 | + | *previous = "" |
| 55 | } | 57 | } | ||
| 56 | }) | 58 | }) | ||
| 57 | } | 59 | } | ||
| @@ -61,6 +63,6 @@ func (ac *attachedClient) clearPreviousSessionIf(target *session) { | |||||
| 61 | // leave the toggle pair intact. | 63 | // leave the toggle pair intact. | ||
| 62 | func (ac *attachedClient) recordPreviousSession(origin *session) { | 64 | func (ac *attachedClient) recordPreviousSession(origin *session) { | ||
| 63 | if ac != nil && origin != nil { | 65 | if ac != nil && origin != nil { | ||
| 64 | - | ac.previousSession.Set(origin) | 66 | + | ac.previousSession.Set(origin.id) |
| 65 | } | 67 | } | ||
| 66 | } | 68 | } | ||