fix(app): attach existing remote session for ui-driver --remote --session
The ui-driver mapped --remote HOST --session NAME to an unconditional named creation, so an existing remote session was refused with "session name already in use". Resolve it attach-or-create against the committed publication: an observed name attaches its exact lifecycle, an absent one is created under the same registration fence.
- commit
- 8360111e4438af5f9780639a8fdd24187c088b92
- author
- brice · · 2026-09-23 02:39 UTC
- parent
- 1af5579
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +70 -29
M internal/app/broker_client.go +19 -0
| @@ -444,6 +444,25 @@ func remoteNamedCreateResolver(endpoint, name string) client.InitialNavigationRe | |||
| 444 | 444 | } | |
| 445 | 445 | } | |
| 446 | 446 | ||
| 447 | + | // remoteAttachOrCreateResolver resolves one remote session name to an exact | |
| 448 | + | // attach when the endpoint's committed observation carries it, and to a named | |
| 449 | + | // creation fenced to the same registration otherwise. An endpoint the broker | |
| 450 | + | // does not carry is refused rather than dialed. | |
| 451 | + | func remoteAttachOrCreateResolver(endpoint, name string) client.InitialNavigationResolver { | |
| 452 | + | attach := remoteExactAttachResolver(endpoint, name) | |
| 453 | + | create := remoteNamedCreateResolver(endpoint, name) | |
| 454 | + | return func(snapshot ports.BrokerSnapshot) (client.InitialNavigation, error) { | |
| 455 | + | observation, ok := snapshot.Find(endpoint) | |
| 456 | + | if !ok { | |
| 457 | + | return client.InitialNavigation{}, fmt.Errorf("%w: %q is not a configured broker host", errTerminalNavigationUnresolved, endpoint) | |
| 458 | + | } | |
| 459 | + | if _, exists := brokerObservationExactTarget(observation, name); exists { | |
| 460 | + | return attach(snapshot) | |
| 461 | + | } | |
| 462 | + | return create(snapshot) | |
| 463 | + | } | |
| 464 | + | } | |
| 465 | + | ||
| 447 | 466 | // remoteEphemeralCreateResolver resolves `attach user@host` (no session) into a | |
| 448 | 467 | // remote ephemeral creation fenced to the endpoint's committed registration and | |
| 449 | 468 | // the epoch of the snapshot that resolved it. | |
| @@ -444,6 +444,25 @@ func remoteNamedCreateResolver(endpoint, name string) client.InitialNavigationRe | |||||
| 444 | } | 444 | } | ||
| 445 | } | 445 | } | ||
| 446 | 446 | ||||
| 447 | + | // remoteAttachOrCreateResolver resolves one remote session name to an exact | |||
| 448 | + | // attach when the endpoint's committed observation carries it, and to a named | |||
| 449 | + | // creation fenced to the same registration otherwise. An endpoint the broker | |||
| 450 | + | // does not carry is refused rather than dialed. | |||
| 451 | + | func remoteAttachOrCreateResolver(endpoint, name string) client.InitialNavigationResolver { | |||
| 452 | + | attach := remoteExactAttachResolver(endpoint, name) | |||
| 453 | + | create := remoteNamedCreateResolver(endpoint, name) | |||
| 454 | + | return func(snapshot ports.BrokerSnapshot) (client.InitialNavigation, error) { | |||
| 455 | + | observation, ok := snapshot.Find(endpoint) | |||
| 456 | + | if !ok { | |||
| 457 | + | return client.InitialNavigation{}, fmt.Errorf("%w: %q is not a configured broker host", errTerminalNavigationUnresolved, endpoint) | |||
| 458 | + | } | |||
| 459 | + | if _, exists := brokerObservationExactTarget(observation, name); exists { | |||
| 460 | + | return attach(snapshot) | |||
| 461 | + | } | |||
| 462 | + | return create(snapshot) | |||
| 463 | + | } | |||
| 464 | + | } | |||
| 465 | + | ||||
| 447 | // remoteEphemeralCreateResolver resolves `attach user@host` (no session) into a | 466 | // remoteEphemeralCreateResolver resolves `attach user@host` (no session) into a | ||
| 448 | // remote ephemeral creation fenced to the endpoint's committed registration and | 467 | // remote ephemeral creation fenced to the endpoint's committed registration and | ||
| 449 | // the epoch of the snapshot that resolved it. | 468 | // the epoch of the snapshot that resolved it. | ||
M internal/app/ui_driver.go +14 -4
| @@ -198,11 +198,21 @@ func uiDriverNavigation(options uiDriverOptions) (client.InitialNavigation, clie | |||
| 198 | 198 | if options.picker { | |
| 199 | 199 | return client.InitialNavigation{}, nil, nil | |
| 200 | 200 | } | |
| 201 | + | if options.remote != "" && options.session != "" { | |
| 202 | + | // `--remote HOST --session NAME` is attach-or-create: an existing | |
| 203 | + | // remote session is attached by its exact lifecycle, an absent one is | |
| 204 | + | // created. Creating unconditionally would collide with the remote | |
| 205 | + | // daemon's live name and be refused. | |
| 206 | + | if err := domain.ValidateRemoteHostTarget(options.remote); err != nil { | |
| 207 | + | return client.InitialNavigation{}, nil, err | |
| 208 | + | } | |
| 209 | + | if err := domain.ValidateSessionName(options.session); err != nil { | |
| 210 | + | return client.InitialNavigation{}, nil, err | |
| 211 | + | } | |
| 212 | + | return client.InitialNavigation{}, remoteAttachOrCreateResolver(options.remote, options.session), nil | |
| 213 | + | } | |
| 201 | 214 | intent := protocol.IntentEphemeral | |
| 202 | - | switch { | |
| 203 | - | case options.remote == "" && options.session != "": | |
| 204 | - | intent = protocol.IntentNew | |
| 205 | - | case options.remote != "" && options.session != "": | |
| 215 | + | if options.session != "" { | |
| 206 | 216 | intent = protocol.IntentNew | |
| 207 | 217 | } | |
| 208 | 218 | return terminalBrokerNavigation(intent, options.session, options.remote) | |
| @@ -198,11 +198,21 @@ func uiDriverNavigation(options uiDriverOptions) (client.InitialNavigation, clie | |||||
| 198 | if options.picker { | 198 | if options.picker { | ||
| 199 | return client.InitialNavigation{}, nil, nil | 199 | return client.InitialNavigation{}, nil, nil | ||
| 200 | } | 200 | } | ||
| 201 | + | if options.remote != "" && options.session != "" { | |||
| 202 | + | // `--remote HOST --session NAME` is attach-or-create: an existing | |||
| 203 | + | // remote session is attached by its exact lifecycle, an absent one is | |||
| 204 | + | // created. Creating unconditionally would collide with the remote | |||
| 205 | + | // daemon's live name and be refused. | |||
| 206 | + | if err := domain.ValidateRemoteHostTarget(options.remote); err != nil { | |||
| 207 | + | return client.InitialNavigation{}, nil, err | |||
| 208 | + | } | |||
| 209 | + | if err := domain.ValidateSessionName(options.session); err != nil { | |||
| 210 | + | return client.InitialNavigation{}, nil, err | |||
| 211 | + | } | |||
| 212 | + | return client.InitialNavigation{}, remoteAttachOrCreateResolver(options.remote, options.session), nil | |||
| 213 | + | } | |||
| 201 | intent := protocol.IntentEphemeral | 214 | intent := protocol.IntentEphemeral | ||
| 202 | - | switch { | 215 | + | if options.session != "" { |
| 203 | - | case options.remote == "" && options.session != "": | |||
| 204 | - | intent = protocol.IntentNew | |||
| 205 | - | case options.remote != "" && options.session != "": | |||
| 206 | intent = protocol.IntentNew | 216 | intent = protocol.IntentNew | ||
| 207 | } | 217 | } | ||
| 208 | return terminalBrokerNavigation(intent, options.session, options.remote) | 218 | return terminalBrokerNavigation(intent, options.session, options.remote) | ||
M internal/app/ui_driver_acceptance_test.go +37 -25
| @@ -212,40 +212,52 @@ func TestUIDriverNavigationReusesTheSharedTerminalTranslation(t *testing.T) { | |||
| 212 | 212 | require.Equal(t, "work", named.Name) | |
| 213 | 213 | require.True(t, named.Destination.Local) | |
| 214 | 214 | ||
| 215 | - | for _, options := range []uiDriverOptions{ | |
| 216 | - | {remote: "user@example.com"}, | |
| 217 | - | {remote: "user@example.com", session: "work"}, | |
| 218 | - | } { | |
| 219 | - | name := "remote ephemeral" | |
| 220 | - | if options.session != "" { | |
| 221 | - | name = "remote named creation" | |
| 215 | + | remoteSnapshot := func(sessions ...string) ports.BrokerSnapshot { | |
| 216 | + | return ports.BrokerSnapshot{ | |
| 217 | + | Epoch: terminalCompositionEpoch, | |
| 218 | + | Revision: 1, | |
| 219 | + | Daemons: []ports.BrokerDaemonObservation{{ | |
| 220 | + | Endpoint: "user@example.com", | |
| 221 | + | DisplayOrigin: "user@example.com", | |
| 222 | + | Registration: terminalCompositionRegistration("user@example.com"), | |
| 223 | + | InventoryKnown: true, | |
| 224 | + | Sessions: terminalCompositionSessions(sessions), | |
| 225 | + | }}, | |
| 222 | 226 | } | |
| 223 | - | t.Run(name, func(t *testing.T) { | |
| 224 | - | navigation, resolver, err := uiDriverNavigation(options) | |
| 227 | + | } | |
| 228 | + | tests := []struct { | |
| 229 | + | name string | |
| 230 | + | options uiDriverOptions | |
| 231 | + | snapshot ports.BrokerSnapshot | |
| 232 | + | wantKind client.InitialNavigationKind | |
| 233 | + | wantName string | |
| 234 | + | }{ | |
| 235 | + | {name: "remote ephemeral", options: uiDriverOptions{remote: "user@example.com"}, snapshot: remoteSnapshot("work"), wantKind: client.InitialNavigationCreateEphemeral}, | |
| 236 | + | {name: "remote named existing attaches exact lifecycle", options: uiDriverOptions{remote: "user@example.com", session: "work"}, snapshot: remoteSnapshot("other", "work"), wantKind: client.InitialNavigationAttachExact, wantName: "work"}, | |
| 237 | + | {name: "remote named missing creates", options: uiDriverOptions{remote: "user@example.com", session: "fresh"}, snapshot: remoteSnapshot("work"), wantKind: client.InitialNavigationCreateNamed, wantName: "fresh"}, | |
| 238 | + | {name: "remote named on empty inventory creates", options: uiDriverOptions{remote: "user@example.com", session: "fresh"}, snapshot: remoteSnapshot(), wantKind: client.InitialNavigationCreateNamed, wantName: "fresh"}, | |
| 239 | + | } | |
| 240 | + | for _, tt := range tests { | |
| 241 | + | t.Run(tt.name, func(t *testing.T) { | |
| 242 | + | navigation, resolver, err := uiDriverNavigation(tt.options) | |
| 225 | 243 | require.NoError(t, err) | |
| 226 | 244 | require.Equal(t, client.InitialNavigation{}, navigation, "a remote target is never decided before the connection") | |
| 227 | 245 | require.NotNil(t, resolver, "a remote target resolves against the committed publication") | |
| 228 | 246 | ||
| 229 | - | snapshot := ports.BrokerSnapshot{ | |
| 230 | - | Epoch: terminalCompositionEpoch, | |
| 231 | - | Revision: 1, | |
| 232 | - | Daemons: []ports.BrokerDaemonObservation{{ | |
| 233 | - | Endpoint: "user@example.com", | |
| 234 | - | DisplayOrigin: "user@example.com", | |
| 235 | - | Registration: terminalCompositionRegistration("user@example.com"), | |
| 236 | - | Sessions: terminalCompositionSessions([]string{"work"}), | |
| 237 | - | }}, | |
| 238 | - | } | |
| 239 | - | resolved, resolveErr := resolver(snapshot) | |
| 247 | + | resolved, resolveErr := resolver(tt.snapshot) | |
| 240 | 248 | require.NoError(t, resolveErr) | |
| 241 | 249 | require.NoError(t, resolved.Validate()) | |
| 242 | 250 | require.False(t, resolved.Destination.Local) | |
| 243 | - | if options.session == "" { | |
| 244 | - | require.Equal(t, client.InitialNavigationCreateEphemeral, resolved.Kind) | |
| 245 | - | return | |
| 251 | + | require.Equal(t, tt.snapshot.Daemons[0].Registration, resolved.Destination.Registration) | |
| 252 | + | require.Equal(t, tt.wantKind, resolved.Kind) | |
| 253 | + | switch tt.wantKind { | |
| 254 | + | case client.InitialNavigationAttachExact: | |
| 255 | + | want, ok := brokerObservationExactTarget(tt.snapshot.Daemons[0], tt.wantName) | |
| 256 | + | require.True(t, ok) | |
| 257 | + | require.Equal(t, want, resolved.Target, "an existing name attaches its exact lifecycle, never a same-name creation") | |
| 258 | + | case client.InitialNavigationCreateNamed: | |
| 259 | + | require.Equal(t, tt.wantName, resolved.Name) | |
| 246 | 260 | } | |
| 247 | - | require.Equal(t, client.InitialNavigationCreateNamed, resolved.Kind) | |
| 248 | - | require.Equal(t, "work", resolved.Name) | |
| 249 | 261 | }) | |
| 250 | 262 | } | |
| 251 | 263 | ||
| @@ -212,40 +212,52 @@ func TestUIDriverNavigationReusesTheSharedTerminalTranslation(t *testing.T) { | |||||
| 212 | require.Equal(t, "work", named.Name) | 212 | require.Equal(t, "work", named.Name) | ||
| 213 | require.True(t, named.Destination.Local) | 213 | require.True(t, named.Destination.Local) | ||
| 214 | 214 | ||||
| 215 | - | for _, options := range []uiDriverOptions{ | 215 | + | remoteSnapshot := func(sessions ...string) ports.BrokerSnapshot { |
| 216 | - | {remote: "user@example.com"}, | 216 | + | return ports.BrokerSnapshot{ |
| 217 | - | {remote: "user@example.com", session: "work"}, | 217 | + | Epoch: terminalCompositionEpoch, |
| 218 | - | } { | 218 | + | Revision: 1, |
| 219 | - | name := "remote ephemeral" | 219 | + | Daemons: []ports.BrokerDaemonObservation{{ |
| 220 | - | if options.session != "" { | 220 | + | Endpoint: "user@example.com", |
| 221 | - | name = "remote named creation" | 221 | + | DisplayOrigin: "user@example.com", |
| 222 | + | Registration: terminalCompositionRegistration("user@example.com"), | |||
| 223 | + | InventoryKnown: true, | |||
| 224 | + | Sessions: terminalCompositionSessions(sessions), | |||
| 225 | + | }}, | |||
| 222 | } | 226 | } | ||
| 223 | - | t.Run(name, func(t *testing.T) { | 227 | + | } |
| 224 | - | navigation, resolver, err := uiDriverNavigation(options) | 228 | + | tests := []struct { |
| 229 | + | name string | |||
| 230 | + | options uiDriverOptions | |||
| 231 | + | snapshot ports.BrokerSnapshot | |||
| 232 | + | wantKind client.InitialNavigationKind | |||
| 233 | + | wantName string | |||
| 234 | + | }{ | |||
| 235 | + | {name: "remote ephemeral", options: uiDriverOptions{remote: "user@example.com"}, snapshot: remoteSnapshot("work"), wantKind: client.InitialNavigationCreateEphemeral}, | |||
| 236 | + | {name: "remote named existing attaches exact lifecycle", options: uiDriverOptions{remote: "user@example.com", session: "work"}, snapshot: remoteSnapshot("other", "work"), wantKind: client.InitialNavigationAttachExact, wantName: "work"}, | |||
| 237 | + | {name: "remote named missing creates", options: uiDriverOptions{remote: "user@example.com", session: "fresh"}, snapshot: remoteSnapshot("work"), wantKind: client.InitialNavigationCreateNamed, wantName: "fresh"}, | |||
| 238 | + | {name: "remote named on empty inventory creates", options: uiDriverOptions{remote: "user@example.com", session: "fresh"}, snapshot: remoteSnapshot(), wantKind: client.InitialNavigationCreateNamed, wantName: "fresh"}, | |||
| 239 | + | } | |||
| 240 | + | for _, tt := range tests { | |||
| 241 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 242 | + | navigation, resolver, err := uiDriverNavigation(tt.options) | |||
| 225 | require.NoError(t, err) | 243 | require.NoError(t, err) | ||
| 226 | require.Equal(t, client.InitialNavigation{}, navigation, "a remote target is never decided before the connection") | 244 | require.Equal(t, client.InitialNavigation{}, navigation, "a remote target is never decided before the connection") | ||
| 227 | require.NotNil(t, resolver, "a remote target resolves against the committed publication") | 245 | require.NotNil(t, resolver, "a remote target resolves against the committed publication") | ||
| 228 | 246 | ||||
| 229 | - | snapshot := ports.BrokerSnapshot{ | 247 | + | resolved, resolveErr := resolver(tt.snapshot) |
| 230 | - | Epoch: terminalCompositionEpoch, | |||
| 231 | - | Revision: 1, | |||
| 232 | - | Daemons: []ports.BrokerDaemonObservation{{ | |||
| 233 | - | Endpoint: "user@example.com", | |||
| 234 | - | DisplayOrigin: "user@example.com", | |||
| 235 | - | Registration: terminalCompositionRegistration("user@example.com"), | |||
| 236 | - | Sessions: terminalCompositionSessions([]string{"work"}), | |||
| 237 | - | }}, | |||
| 238 | - | } | |||
| 239 | - | resolved, resolveErr := resolver(snapshot) | |||
| 240 | require.NoError(t, resolveErr) | 248 | require.NoError(t, resolveErr) | ||
| 241 | require.NoError(t, resolved.Validate()) | 249 | require.NoError(t, resolved.Validate()) | ||
| 242 | require.False(t, resolved.Destination.Local) | 250 | require.False(t, resolved.Destination.Local) | ||
| 243 | - | if options.session == "" { | 251 | + | require.Equal(t, tt.snapshot.Daemons[0].Registration, resolved.Destination.Registration) |
| 244 | - | require.Equal(t, client.InitialNavigationCreateEphemeral, resolved.Kind) | 252 | + | require.Equal(t, tt.wantKind, resolved.Kind) |
| 245 | - | return | 253 | + | switch tt.wantKind { |
| 254 | + | case client.InitialNavigationAttachExact: | |||
| 255 | + | want, ok := brokerObservationExactTarget(tt.snapshot.Daemons[0], tt.wantName) | |||
| 256 | + | require.True(t, ok) | |||
| 257 | + | require.Equal(t, want, resolved.Target, "an existing name attaches its exact lifecycle, never a same-name creation") | |||
| 258 | + | case client.InitialNavigationCreateNamed: | |||
| 259 | + | require.Equal(t, tt.wantName, resolved.Name) | |||
| 246 | } | 260 | } | ||
| 247 | - | require.Equal(t, client.InitialNavigationCreateNamed, resolved.Kind) | |||
| 248 | - | require.Equal(t, "work", resolved.Name) | |||
| 249 | }) | 261 | }) | ||
| 250 | } | 262 | } | ||
| 251 | 263 | ||||