fix(daemon): use remote environment for palette session creation (#258)
* fix(daemon): use remote environment for palette session creation * test(daemon): tighten remote environment coverage
- commit
- d587eb9ebee1d8b4f45b052c5a6c1c02975e2496
- author
- bnema · · 2026-09-06 07:32 UTC
- committer
- GitHub · · 2026-09-06 07:32 UTC
- parent
- 8230561
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
7 files · +153 -31
M docs/terminal.md +12 -3
| @@ -29,9 +29,18 @@ their first frame as a static fallback. | |||
| 29 | 29 | ||
| 30 | 30 | ## Environment | |
| 31 | 31 | ||
| 32 | - | On each attach, the attaching environment becomes the session's source of truth for future PTY children. vev preserves it except that it replaces `TERM`, `COLORTERM`, `TERM_PROGRAM`, and `VEV`; `SHELL` selects the command used to start each PTY. Already-running processes keep their original environment. | |
| 33 | - | ||
| 34 | - | The attaching client's exported environment is sent in the connection handshake and becomes the session's source of truth for future PTY children, including remote attaches. A later attachment updates that shared environment for children created afterward; already-running processes keep their original environment. | |
| 32 | + | Local and direct CLI remote attachments supply the session's environment for | |
| 33 | + | future PTY children. A later client-owned attachment updates that shared | |
| 34 | + | environment; already-running processes keep their original environment. vev | |
| 35 | + | replaces `TERM`, `COLORTERM`, `TERM_PROGRAM`, and `VEV`; `SHELL` selects the | |
| 36 | + | command used to start each PTY. | |
| 37 | + | ||
| 38 | + | Picker-selected remote attachments do not replace the session's environment | |
| 39 | + | with the client's exports. When restarting a stopped remote session, vev uses | |
| 40 | + | the destination daemon's environment and the persisted working directory. A | |
| 41 | + | `CNS` handoff to another remote daemon also uses that daemon's environment, | |
| 42 | + | starting in its home directory rather than the client's working directory. This | |
| 43 | + | keeps `HOME`, `SHELL`, `PATH`, and XDG paths local to the destination host. | |
| 35 | 44 | ||
| 36 | 45 | ## VT compatibility | |
| 37 | 46 | ||
| @@ -29,9 +29,18 @@ their first frame as a static fallback. | |||||
| 29 | 29 | ||||
| 30 | ## Environment | 30 | ## Environment | ||
| 31 | 31 | ||||
| 32 | - | On each attach, the attaching environment becomes the session's source of truth for future PTY children. vev preserves it except that it replaces `TERM`, `COLORTERM`, `TERM_PROGRAM`, and `VEV`; `SHELL` selects the command used to start each PTY. Already-running processes keep their original environment. | 32 | + | Local and direct CLI remote attachments supply the session's environment for |
| 33 | - | 33 | + | future PTY children. A later client-owned attachment updates that shared | |
| 34 | - | The attaching client's exported environment is sent in the connection handshake and becomes the session's source of truth for future PTY children, including remote attaches. A later attachment updates that shared environment for children created afterward; already-running processes keep their original environment. | 34 | + | environment; already-running processes keep their original environment. vev |
| 35 | + | replaces `TERM`, `COLORTERM`, `TERM_PROGRAM`, and `VEV`; `SHELL` selects the | |||
| 36 | + | command used to start each PTY. | |||
| 37 | + | ||||
| 38 | + | Picker-selected remote attachments do not replace the session's environment | |||
| 39 | + | with the client's exports. When restarting a stopped remote session, vev uses | |||
| 40 | + | the destination daemon's environment and the persisted working directory. A | |||
| 41 | + | `CNS` handoff to another remote daemon also uses that daemon's environment, | |||
| 42 | + | starting in its home directory rather than the client's working directory. This | |||
| 43 | + | keeps `HOME`, `SHELL`, `PATH`, and XDG paths local to the destination host. | |||
| 35 | 44 | ||||
| 36 | ## VT compatibility | 45 | ## VT compatibility | ||
| 37 | 46 | ||||
M internal/app/run_test.go +37 -21
| @@ -1039,27 +1039,43 @@ func TestRunAttachWithDepsBoundsRepeatedAttachTargetHandoffs(t *testing.T) { | |||
| 1039 | 1039 | } | |
| 1040 | 1040 | ||
| 1041 | 1041 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | |
| 1042 | - | factory := newRemoteDialerFactoryMock(t) | |
| 1043 | - | factory.EXPECT().DialerForRemote("selected.example", "picked", remoteadapter.TransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once() | |
| 1044 | - | ||
| 1045 | - | localCalls, remoteCalls := 0, 0 | |
| 1046 | - | err := runAttachWithDeps(context.Background(), protocol.IntentAttach, "work", "", "", nil, runAttachDeps{ | |
| 1047 | - | localDialer: func() wire.Dialer { return namedDialer{name: "local"} }, | |
| 1048 | - | remoteDialerFactory: factory.DialerForRemote, | |
| 1049 | - | runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error { | |
| 1050 | - | if deps.Remote { | |
| 1051 | - | remoteCalls++ | |
| 1052 | - | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "picked", Remote: true, Origin: protocol.RouteOriginDiscovery, OriginKey: "selected.example", HostLabel: "selected.example", EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned}, request) | |
| 1053 | - | return nil | |
| 1054 | - | } | |
| 1055 | - | localCalls++ | |
| 1056 | - | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "work", Origin: protocol.RouteOriginLocal, OriginKey: "local"}, request) | |
| 1057 | - | return &client.AttachTargetError{Target: protocol.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: protocol.IntentAttach}} | |
| 1058 | - | }, | |
| 1059 | - | }) | |
| 1060 | - | require.NoError(t, err) | |
| 1061 | - | require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally") | |
| 1062 | - | require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once") | |
| 1042 | + | for _, tt := range []struct { | |
| 1043 | + | name string | |
| 1044 | + | intent uint8 | |
| 1045 | + | requestID uint64 | |
| 1046 | + | policy protocol.EnvironmentPolicy | |
| 1047 | + | }{ | |
| 1048 | + | {name: "attach", intent: protocol.IntentAttach}, | |
| 1049 | + | {name: "create", intent: protocol.IntentNew, requestID: 1, policy: protocol.EnvironmentPolicyDaemonOwned}, | |
| 1050 | + | } { | |
| 1051 | + | t.Run(tt.name, func(t *testing.T) { | |
| 1052 | + | factory := newRemoteDialerFactoryMock(t) | |
| 1053 | + | factory.EXPECT().DialerForRemote("selected.example", "picked", remoteadapter.TransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(2) | |
| 1054 | + | target := protocol.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: tt.intent, RequestID: tt.requestID, EnvironmentPolicy: tt.policy} | |
| 1055 | + | want := client.AttachRequest{Intent: tt.intent, SessionName: "picked", Remote: true, Origin: protocol.RouteOriginDiscovery, OriginKey: "selected.example", HostLabel: "selected.example", EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned} | |
| 1056 | + | localCalls, remoteCalls := 0, 0 | |
| 1057 | + | err := runAttachWithDeps(context.Background(), protocol.IntentAttach, "work", "", "", nil, runAttachDeps{ | |
| 1058 | + | localDialer: func() wire.Dialer { return namedDialer{name: "local"} }, | |
| 1059 | + | remoteDialerFactory: factory.DialerForRemote, | |
| 1060 | + | runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error { | |
| 1061 | + | if deps.Remote { | |
| 1062 | + | remoteCalls++ | |
| 1063 | + | require.Equal(t, want, request) | |
| 1064 | + | return nil | |
| 1065 | + | } | |
| 1066 | + | localCalls++ | |
| 1067 | + | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "work", Origin: protocol.RouteOriginLocal, OriginKey: "local"}, request) | |
| 1068 | + | _, next, err := deps.AttachHandoff(target) | |
| 1069 | + | require.NoError(t, err) | |
| 1070 | + | require.Equal(t, want, next, "in-run handoff must preserve environment ownership") | |
| 1071 | + | return &client.AttachTargetError{Target: target} | |
| 1072 | + | }, | |
| 1073 | + | }) | |
| 1074 | + | require.NoError(t, err) | |
| 1075 | + | require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally") | |
| 1076 | + | require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once") | |
| 1077 | + | }) | |
| 1078 | + | } | |
| 1063 | 1079 | } | |
| 1064 | 1080 | ||
| 1065 | 1081 | func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) { | |
| @@ -1039,27 +1039,43 @@ func TestRunAttachWithDepsBoundsRepeatedAttachTargetHandoffs(t *testing.T) { | |||||
| 1039 | } | 1039 | } | ||
| 1040 | 1040 | ||||
| 1041 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | 1041 | func TestRunAttachWithDepsLocalPickerHandoffAttachesSelectedRemote(t *testing.T) { | ||
| 1042 | - | factory := newRemoteDialerFactoryMock(t) | 1042 | + | for _, tt := range []struct { |
| 1043 | - | factory.EXPECT().DialerForRemote("selected.example", "picked", remoteadapter.TransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Once() | 1043 | + | name string |
| 1044 | - | 1044 | + | intent uint8 | |
| 1045 | - | localCalls, remoteCalls := 0, 0 | 1045 | + | requestID uint64 |
| 1046 | - | err := runAttachWithDeps(context.Background(), protocol.IntentAttach, "work", "", "", nil, runAttachDeps{ | 1046 | + | policy protocol.EnvironmentPolicy |
| 1047 | - | localDialer: func() wire.Dialer { return namedDialer{name: "local"} }, | 1047 | + | }{ |
| 1048 | - | remoteDialerFactory: factory.DialerForRemote, | 1048 | + | {name: "attach", intent: protocol.IntentAttach}, |
| 1049 | - | runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error { | 1049 | + | {name: "create", intent: protocol.IntentNew, requestID: 1, policy: protocol.EnvironmentPolicyDaemonOwned}, |
| 1050 | - | if deps.Remote { | 1050 | + | } { |
| 1051 | - | remoteCalls++ | 1051 | + | t.Run(tt.name, func(t *testing.T) { |
| 1052 | - | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "picked", Remote: true, Origin: protocol.RouteOriginDiscovery, OriginKey: "selected.example", HostLabel: "selected.example", EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned}, request) | 1052 | + | factory := newRemoteDialerFactoryMock(t) |
| 1053 | - | return nil | 1053 | + | factory.EXPECT().DialerForRemote("selected.example", "picked", remoteadapter.TransportUDP, mock.Anything).Return(namedDialer{name: "remote"}, nil).Times(2) |
| 1054 | - | } | 1054 | + | target := protocol.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: tt.intent, RequestID: tt.requestID, EnvironmentPolicy: tt.policy} |
| 1055 | - | localCalls++ | 1055 | + | want := client.AttachRequest{Intent: tt.intent, SessionName: "picked", Remote: true, Origin: protocol.RouteOriginDiscovery, OriginKey: "selected.example", HostLabel: "selected.example", EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned} |
| 1056 | - | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "work", Origin: protocol.RouteOriginLocal, OriginKey: "local"}, request) | 1056 | + | localCalls, remoteCalls := 0, 0 |
| 1057 | - | return &client.AttachTargetError{Target: protocol.AttachTarget{Endpoint: "selected.example", Session: "picked", Intent: protocol.IntentAttach}} | 1057 | + | err := runAttachWithDeps(context.Background(), protocol.IntentAttach, "work", "", "", nil, runAttachDeps{ |
| 1058 | - | }, | 1058 | + | localDialer: func() wire.Dialer { return namedDialer{name: "local"} }, |
| 1059 | - | }) | 1059 | + | remoteDialerFactory: factory.DialerForRemote, |
| 1060 | - | require.NoError(t, err) | 1060 | + | runClient: func(_ context.Context, deps client.Dependencies, request client.AttachRequest) error { |
| 1061 | - | require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally") | 1061 | + | if deps.Remote { |
| 1062 | - | require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once") | 1062 | + | remoteCalls++ |
| 1063 | + | require.Equal(t, want, request) | |||
| 1064 | + | return nil | |||
| 1065 | + | } | |||
| 1066 | + | localCalls++ | |||
| 1067 | + | require.Equal(t, client.AttachRequest{Intent: protocol.IntentAttach, SessionName: "work", Origin: protocol.RouteOriginLocal, OriginKey: "local"}, request) | |||
| 1068 | + | _, next, err := deps.AttachHandoff(target) | |||
| 1069 | + | require.NoError(t, err) | |||
| 1070 | + | require.Equal(t, want, next, "in-run handoff must preserve environment ownership") | |||
| 1071 | + | return &client.AttachTargetError{Target: target} | |||
| 1072 | + | }, | |||
| 1073 | + | }) | |||
| 1074 | + | require.NoError(t, err) | |||
| 1075 | + | require.Equal(t, 1, localCalls, "the failed local picker attach must not retry locally") | |||
| 1076 | + | require.Equal(t, 1, remoteCalls, "the selected target must be dialed and attached exactly once") | |||
| 1077 | + | }) | |||
| 1078 | + | } | |||
| 1063 | } | 1079 | } | ||
| 1064 | 1080 | ||||
| 1065 | func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) { | 1081 | func TestRunAttachWithDepsRejectsInvalidHandoffBeforeDialing(t *testing.T) { | ||
M internal/usecase/client/client_test.go +5 -2
| @@ -1334,7 +1334,7 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||
| 1334 | 1334 | } | |
| 1335 | 1335 | createRemote := protocol.AttachTarget{ | |
| 1336 | 1336 | RequestID: 1, Endpoint: "remote", Session: "created", Intent: protocol.IntentNew, | |
| 1337 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | |
| 1337 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, | |
| 1338 | 1338 | } | |
| 1339 | 1339 | local1 := &recordingTransport{recvs: []recvItem{ | |
| 1340 | 1340 | {f: welcome("misc", localLifecycle)}, | |
| @@ -1364,7 +1364,7 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||
| 1364 | 1364 | return remoteDialer, client.AttachRequest{ | |
| 1365 | 1365 | Intent: protocol.IntentNew, SessionName: target.Session, Remote: true, | |
| 1366 | 1366 | Origin: protocol.RouteOriginDiscovery, OriginKey: target.Endpoint, | |
| 1367 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | |
| 1367 | + | EnvironmentPolicy: target.EnvironmentPolicy, | |
| 1368 | 1368 | }, nil | |
| 1369 | 1369 | } | |
| 1370 | 1370 | ||
| @@ -1373,6 +1373,9 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||
| 1373 | 1373 | }) | |
| 1374 | 1374 | require.NoError(t, err) | |
| 1375 | 1375 | require.Equal(t, int32(2), remoteDialer.calls.Load()) | |
| 1376 | + | createHello := helloFromSend(t, remote1) | |
| 1377 | + | require.Equal(t, protocol.IntentNew, createHello.Intent) | |
| 1378 | + | require.Equal(t, protocol.EnvironmentPolicyDaemonOwned, createHello.EnvironmentPolicy) | |
| 1376 | 1379 | returnHello := helloFromSend(t, remote2) | |
| 1377 | 1380 | require.Equal(t, protocol.IntentResume, returnHello.Intent) | |
| 1378 | 1381 | require.Equal(t, "created", returnHello.Name) | |
| @@ -1334,7 +1334,7 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||||
| 1334 | } | 1334 | } | ||
| 1335 | createRemote := protocol.AttachTarget{ | 1335 | createRemote := protocol.AttachTarget{ | ||
| 1336 | RequestID: 1, Endpoint: "remote", Session: "created", Intent: protocol.IntentNew, | 1336 | RequestID: 1, Endpoint: "remote", Session: "created", Intent: protocol.IntentNew, | ||
| 1337 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | 1337 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, |
| 1338 | } | 1338 | } | ||
| 1339 | local1 := &recordingTransport{recvs: []recvItem{ | 1339 | local1 := &recordingTransport{recvs: []recvItem{ | ||
| 1340 | {f: welcome("misc", localLifecycle)}, | 1340 | {f: welcome("misc", localLifecycle)}, | ||
| @@ -1364,7 +1364,7 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||||
| 1364 | return remoteDialer, client.AttachRequest{ | 1364 | return remoteDialer, client.AttachRequest{ | ||
| 1365 | Intent: protocol.IntentNew, SessionName: target.Session, Remote: true, | 1365 | Intent: protocol.IntentNew, SessionName: target.Session, Remote: true, | ||
| 1366 | Origin: protocol.RouteOriginDiscovery, OriginKey: target.Endpoint, | 1366 | Origin: protocol.RouteOriginDiscovery, OriginKey: target.Endpoint, | ||
| 1367 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | 1367 | + | EnvironmentPolicy: target.EnvironmentPolicy, |
| 1368 | }, nil | 1368 | }, nil | ||
| 1369 | } | 1369 | } | ||
| 1370 | 1370 | ||||
| @@ -1373,6 +1373,9 @@ func TestRouteNavigationReturnsToCreatedRemoteSession(t *testing.T) { | |||||
| 1373 | }) | 1373 | }) | ||
| 1374 | require.NoError(t, err) | 1374 | require.NoError(t, err) | ||
| 1375 | require.Equal(t, int32(2), remoteDialer.calls.Load()) | 1375 | require.Equal(t, int32(2), remoteDialer.calls.Load()) | ||
| 1376 | + | createHello := helloFromSend(t, remote1) | |||
| 1377 | + | require.Equal(t, protocol.IntentNew, createHello.Intent) | |||
| 1378 | + | require.Equal(t, protocol.EnvironmentPolicyDaemonOwned, createHello.EnvironmentPolicy) | |||
| 1376 | returnHello := helloFromSend(t, remote2) | 1379 | returnHello := helloFromSend(t, remote2) | ||
| 1377 | require.Equal(t, protocol.IntentResume, returnHello.Intent) | 1380 | require.Equal(t, protocol.IntentResume, returnHello.Intent) | ||
| 1378 | require.Equal(t, "created", returnHello.Name) | 1381 | require.Equal(t, "created", returnHello.Name) | ||
M internal/usecase/daemon/daemon.go +8 -2
| @@ -1610,7 +1610,7 @@ func (d *Daemon) routeWithContext(ctx context.Context, h protocol.Hello, tr port | |||
| 1610 | 1610 | } | |
| 1611 | 1611 | } | |
| 1612 | 1612 | if h.EnvironmentPolicy == protocol.EnvironmentPolicyDaemonOwned && h.RemoteTarget == nil && | |
| 1613 | - | (h.Intent == protocol.IntentNew || h.Intent == protocol.IntentEphemeral) { | |
| 1613 | + | h.Intent == protocol.IntentEphemeral { | |
| 1614 | 1614 | return nil, nil, &protoErr{protocol.ErrNoSuchTarget, "daemon-owned environment requires an exact remote target"} | |
| 1615 | 1615 | } | |
| 1616 | 1616 | d.mu.Lock() | |
| @@ -1655,7 +1655,13 @@ func (d *Daemon) routeWithContext(ctx context.Context, h protocol.Hello, tr port | |||
| 1655 | 1655 | d.mu.Unlock() | |
| 1656 | 1656 | return nil, nil, &protoErr{protocol.ErrNameTaken, "session name already in use: " + h.Name} | |
| 1657 | 1657 | } | |
| 1658 | - | sess, err := d.createSessionLockedWithMode(h.Name, false, h.Cwd, h.Geometry(), h.Env) | |
| 1658 | + | cwd, env := h.Cwd, h.Env | |
| 1659 | + | if h.EnvironmentPolicy == protocol.EnvironmentPolicyDaemonOwned { | |
| 1660 | + | // A remote CNS destination owns shell startup just like a picker | |
| 1661 | + | // attach. Client paths may exist here but belong to another user. | |
| 1662 | + | cwd, env = d.dirOrHome(""), copyEnvironment(d.baseEnv) | |
| 1663 | + | } | |
| 1664 | + | sess, err := d.createSessionLockedWithMode(h.Name, false, cwd, h.Geometry(), env) | |
| 1659 | 1665 | if err != nil { | |
| 1660 | 1666 | d.mu.Unlock() | |
| 1661 | 1667 | return nil, nil, err | |
| @@ -1610,7 +1610,7 @@ func (d *Daemon) routeWithContext(ctx context.Context, h protocol.Hello, tr port | |||||
| 1610 | } | 1610 | } | ||
| 1611 | } | 1611 | } | ||
| 1612 | if h.EnvironmentPolicy == protocol.EnvironmentPolicyDaemonOwned && h.RemoteTarget == nil && | 1612 | if h.EnvironmentPolicy == protocol.EnvironmentPolicyDaemonOwned && h.RemoteTarget == nil && | ||
| 1613 | - | (h.Intent == protocol.IntentNew || h.Intent == protocol.IntentEphemeral) { | 1613 | + | h.Intent == protocol.IntentEphemeral { |
| 1614 | return nil, nil, &protoErr{protocol.ErrNoSuchTarget, "daemon-owned environment requires an exact remote target"} | 1614 | return nil, nil, &protoErr{protocol.ErrNoSuchTarget, "daemon-owned environment requires an exact remote target"} | ||
| 1615 | } | 1615 | } | ||
| 1616 | d.mu.Lock() | 1616 | d.mu.Lock() | ||
| @@ -1655,7 +1655,13 @@ func (d *Daemon) routeWithContext(ctx context.Context, h protocol.Hello, tr port | |||||
| 1655 | d.mu.Unlock() | 1655 | d.mu.Unlock() | ||
| 1656 | return nil, nil, &protoErr{protocol.ErrNameTaken, "session name already in use: " + h.Name} | 1656 | return nil, nil, &protoErr{protocol.ErrNameTaken, "session name already in use: " + h.Name} | ||
| 1657 | } | 1657 | } | ||
| 1658 | - | sess, err := d.createSessionLockedWithMode(h.Name, false, h.Cwd, h.Geometry(), h.Env) | 1658 | + | cwd, env := h.Cwd, h.Env |
| 1659 | + | if h.EnvironmentPolicy == protocol.EnvironmentPolicyDaemonOwned { | |||
| 1660 | + | // A remote CNS destination owns shell startup just like a picker | |||
| 1661 | + | // attach. Client paths may exist here but belong to another user. | |||
| 1662 | + | cwd, env = d.dirOrHome(""), copyEnvironment(d.baseEnv) | |||
| 1663 | + | } | |||
| 1664 | + | sess, err := d.createSessionLockedWithMode(h.Name, false, cwd, h.Geometry(), env) | |||
| 1659 | if err != nil { | 1665 | if err != nil { | ||
| 1660 | d.mu.Unlock() | 1666 | d.mu.Unlock() | ||
| 1661 | return nil, nil, err | 1667 | return nil, nil, err | ||
M internal/usecase/daemon/palette.go +1 -1
| @@ -908,7 +908,7 @@ func (e paletteExec) createSessionOnValidatedDestination(effect *attachmentEffec | |||
| 908 | 908 | case palette.CreateSessionOnRemoteHost: | |
| 909 | 909 | handoff := protocol.AttachTarget{ | |
| 910 | 910 | RequestID: requestID, Endpoint: endpoint, Session: name, Intent: protocol.IntentNew, | |
| 911 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | |
| 911 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, | |
| 912 | 912 | } | |
| 913 | 913 | if err := protocol.ValidateAttachTarget(handoff); err != nil { | |
| 914 | 914 | return errAttachmentTransition | |
| @@ -908,7 +908,7 @@ func (e paletteExec) createSessionOnValidatedDestination(effect *attachmentEffec | |||||
| 908 | case palette.CreateSessionOnRemoteHost: | 908 | case palette.CreateSessionOnRemoteHost: | ||
| 909 | handoff := protocol.AttachTarget{ | 909 | handoff := protocol.AttachTarget{ | ||
| 910 | RequestID: requestID, Endpoint: endpoint, Session: name, Intent: protocol.IntentNew, | 910 | RequestID: requestID, Endpoint: endpoint, Session: name, Intent: protocol.IntentNew, | ||
| 911 | - | EnvironmentPolicy: protocol.EnvironmentPolicyClientOwned, | 911 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, |
| 912 | } | 912 | } | ||
| 913 | if err := protocol.ValidateAttachTarget(handoff); err != nil { | 913 | if err := protocol.ValidateAttachTarget(handoff); err != nil { | ||
| 914 | return errAttachmentTransition | 914 | return errAttachmentTransition | ||
A internal/usecase/daemon/remote_create_environment_test.go +89 -0
| @@ -0,0 +1,89 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/mock" | |
| 9 | + | "github.com/stretchr/testify/require" | |
| 10 | + | ||
| 11 | + | "github.com/bnema/vev/internal/domain" | |
| 12 | + | "github.com/bnema/vev/internal/ports" | |
| 13 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 14 | + | "github.com/bnema/vev/internal/protocol" | |
| 15 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |
| 16 | + | "github.com/bnema/vev/internal/protocol/wire" | |
| 17 | + | "github.com/bnema/vev/internal/usecase/palette" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | func TestPaletteRemoteCreateUsesDaemonEnvironment(t *testing.T) { | |
| 21 | + | local := newRemotePickerDaemon(&remoteRefreshHostStore{hosts: []string{"remote.example"}}) | |
| 22 | + | local.remoteCatalog.replaceCache([]catalogue.RemoteCatalogCacheEntry{{Host: "remote.example", FetchedAt: time.Unix(10, 0)}}) | |
| 23 | + | local.remoteCatalog.mu.Lock() | |
| 24 | + | local.remoteCatalog.status["remote.example"] = remoteHostFresh | |
| 25 | + | local.remoteCatalog.mu.Unlock() | |
| 26 | + | sess, ac, sends := addRemoteRefreshPickerOwner(t, local, "local") | |
| 27 | + | effect := beginRecentRoutePaletteEffect(t, local, sess, ac) | |
| 28 | + | result := palette.NewCreateSessionDestination(palette.CreateSessionOnRemoteHost, "remote.example", "remote.example", protocol.RouteRef{}, 0) | |
| 29 | + | err := (paletteExec{d: local, sess: sess, ac: ac, effect: effect}).createSessionOnDestination(effect, result, "work") | |
| 30 | + | require.NoError(t, err) | |
| 31 | + | frame := awaitFrame(t, sends, wire.MsgAttachTarget) | |
| 32 | + | handoff, err := wire.UnmarshalAttachTarget(frame.Payload) | |
| 33 | + | require.NoError(t, err) | |
| 34 | + | require.Equal(t, protocol.IntentNew, handoff.Intent) | |
| 35 | + | require.NotZero(t, handoff.RequestID) | |
| 36 | + | require.Equal(t, protocol.EnvironmentPolicyDaemonOwned, handoff.EnvironmentPolicy) | |
| 37 | + | require.Nil(t, ac.currentAttachmentSession()) | |
| 38 | + | } | |
| 39 | + | ||
| 40 | + | func TestNewSessionEnvironmentOwnership(t *testing.T) { | |
| 41 | + | for _, tt := range []struct { | |
| 42 | + | name string | |
| 43 | + | policy protocol.EnvironmentPolicy | |
| 44 | + | }{ | |
| 45 | + | {name: "direct client", policy: protocol.EnvironmentPolicyClientOwned}, | |
| 46 | + | {name: "remote palette", policy: protocol.EnvironmentPolicyDaemonOwned}, | |
| 47 | + | } { | |
| 48 | + | t.Run(tt.name, func(t *testing.T) { | |
| 49 | + | clientCwd, remoteHome := t.TempDir(), t.TempDir() | |
| 50 | + | clientEnv := []string{"HOME=/client/home", "SHELL=/client/shell", "PATH=/client/bin", "XDG_CONFIG_HOME=/client/config"} | |
| 51 | + | remoteEnv := []string{"HOME=" + remoteHome, "SHELL=/remote/shell", "PATH=/remote/bin", "XDG_CONFIG_HOME=/remote/config"} | |
| 52 | + | wantEnv, wantCwd, wantShell := clientEnv, clientCwd, "/client/shell" | |
| 53 | + | if tt.policy == protocol.EnvironmentPolicyDaemonOwned { | |
| 54 | + | wantEnv, wantCwd, wantShell = remoteEnv, remoteHome, "/remote/shell" | |
| 55 | + | } | |
| 56 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 57 | + | factory.EXPECT().Open(mock.Anything, wantShell, mock.Anything, mock.Anything, wantCwd, mock.Anything).RunAndReturn( | |
| 58 | + | func(_ context.Context, _ string, _ []string, env []string, _ string, _ domain.Geometry) (ports.PTY, error) { | |
| 59 | + | for _, value := range wantEnv { | |
| 60 | + | require.Contains(t, env, value) | |
| 61 | + | } | |
| 62 | + | if tt.policy == protocol.EnvironmentPolicyDaemonOwned { | |
| 63 | + | for _, value := range clientEnv { | |
| 64 | + | require.NotContains(t, env, value) | |
| 65 | + | } | |
| 66 | + | } | |
| 67 | + | return newQuietPTY(), nil | |
| 68 | + | }, | |
| 69 | + | ) | |
| 70 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 71 | + | d.baseEnv = remoteEnv | |
| 72 | + | d.dirOrHome = func(cwd string) string { | |
| 73 | + | require.Empty(t, cwd, "remote creation must not resolve the client's working directory") | |
| 74 | + | return remoteHome | |
| 75 | + | } | |
| 76 | + | sess, ac, err := d.route(protocol.Hello{ | |
| 77 | + | Version: protocol.Version, Intent: protocol.IntentNew, Name: "work", | |
| 78 | + | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: clientCwd, Env: clientEnv, | |
| 79 | + | EnvironmentPolicy: tt.policy, | |
| 80 | + | }, &closeTrackingTransport{}) | |
| 81 | + | require.NoError(t, err) | |
| 82 | + | require.NotNil(t, ac) | |
| 83 | + | sess.mu.Lock() | |
| 84 | + | require.Equal(t, wantEnv, sess.env, "future PTYs must retain the selected environment") | |
| 85 | + | sess.mu.Unlock() | |
| 86 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, true)) | |
| 87 | + | }) | |
| 88 | + | } | |
| 89 | + | } | |
| @@ -0,0 +1,89 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/mock" | |||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 10 | + | ||||
| 11 | + | "github.com/bnema/vev/internal/domain" | |||
| 12 | + | "github.com/bnema/vev/internal/ports" | |||
| 13 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 14 | + | "github.com/bnema/vev/internal/protocol" | |||
| 15 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |||
| 16 | + | "github.com/bnema/vev/internal/protocol/wire" | |||
| 17 | + | "github.com/bnema/vev/internal/usecase/palette" | |||
| 18 | + | ) | |||
| 19 | + | ||||
| 20 | + | func TestPaletteRemoteCreateUsesDaemonEnvironment(t *testing.T) { | |||
| 21 | + | local := newRemotePickerDaemon(&remoteRefreshHostStore{hosts: []string{"remote.example"}}) | |||
| 22 | + | local.remoteCatalog.replaceCache([]catalogue.RemoteCatalogCacheEntry{{Host: "remote.example", FetchedAt: time.Unix(10, 0)}}) | |||
| 23 | + | local.remoteCatalog.mu.Lock() | |||
| 24 | + | local.remoteCatalog.status["remote.example"] = remoteHostFresh | |||
| 25 | + | local.remoteCatalog.mu.Unlock() | |||
| 26 | + | sess, ac, sends := addRemoteRefreshPickerOwner(t, local, "local") | |||
| 27 | + | effect := beginRecentRoutePaletteEffect(t, local, sess, ac) | |||
| 28 | + | result := palette.NewCreateSessionDestination(palette.CreateSessionOnRemoteHost, "remote.example", "remote.example", protocol.RouteRef{}, 0) | |||
| 29 | + | err := (paletteExec{d: local, sess: sess, ac: ac, effect: effect}).createSessionOnDestination(effect, result, "work") | |||
| 30 | + | require.NoError(t, err) | |||
| 31 | + | frame := awaitFrame(t, sends, wire.MsgAttachTarget) | |||
| 32 | + | handoff, err := wire.UnmarshalAttachTarget(frame.Payload) | |||
| 33 | + | require.NoError(t, err) | |||
| 34 | + | require.Equal(t, protocol.IntentNew, handoff.Intent) | |||
| 35 | + | require.NotZero(t, handoff.RequestID) | |||
| 36 | + | require.Equal(t, protocol.EnvironmentPolicyDaemonOwned, handoff.EnvironmentPolicy) | |||
| 37 | + | require.Nil(t, ac.currentAttachmentSession()) | |||
| 38 | + | } | |||
| 39 | + | ||||
| 40 | + | func TestNewSessionEnvironmentOwnership(t *testing.T) { | |||
| 41 | + | for _, tt := range []struct { | |||
| 42 | + | name string | |||
| 43 | + | policy protocol.EnvironmentPolicy | |||
| 44 | + | }{ | |||
| 45 | + | {name: "direct client", policy: protocol.EnvironmentPolicyClientOwned}, | |||
| 46 | + | {name: "remote palette", policy: protocol.EnvironmentPolicyDaemonOwned}, | |||
| 47 | + | } { | |||
| 48 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 49 | + | clientCwd, remoteHome := t.TempDir(), t.TempDir() | |||
| 50 | + | clientEnv := []string{"HOME=/client/home", "SHELL=/client/shell", "PATH=/client/bin", "XDG_CONFIG_HOME=/client/config"} | |||
| 51 | + | remoteEnv := []string{"HOME=" + remoteHome, "SHELL=/remote/shell", "PATH=/remote/bin", "XDG_CONFIG_HOME=/remote/config"} | |||
| 52 | + | wantEnv, wantCwd, wantShell := clientEnv, clientCwd, "/client/shell" | |||
| 53 | + | if tt.policy == protocol.EnvironmentPolicyDaemonOwned { | |||
| 54 | + | wantEnv, wantCwd, wantShell = remoteEnv, remoteHome, "/remote/shell" | |||
| 55 | + | } | |||
| 56 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 57 | + | factory.EXPECT().Open(mock.Anything, wantShell, mock.Anything, mock.Anything, wantCwd, mock.Anything).RunAndReturn( | |||
| 58 | + | func(_ context.Context, _ string, _ []string, env []string, _ string, _ domain.Geometry) (ports.PTY, error) { | |||
| 59 | + | for _, value := range wantEnv { | |||
| 60 | + | require.Contains(t, env, value) | |||
| 61 | + | } | |||
| 62 | + | if tt.policy == protocol.EnvironmentPolicyDaemonOwned { | |||
| 63 | + | for _, value := range clientEnv { | |||
| 64 | + | require.NotContains(t, env, value) | |||
| 65 | + | } | |||
| 66 | + | } | |||
| 67 | + | return newQuietPTY(), nil | |||
| 68 | + | }, | |||
| 69 | + | ) | |||
| 70 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 71 | + | d.baseEnv = remoteEnv | |||
| 72 | + | d.dirOrHome = func(cwd string) string { | |||
| 73 | + | require.Empty(t, cwd, "remote creation must not resolve the client's working directory") | |||
| 74 | + | return remoteHome | |||
| 75 | + | } | |||
| 76 | + | sess, ac, err := d.route(protocol.Hello{ | |||
| 77 | + | Version: protocol.Version, Intent: protocol.IntentNew, Name: "work", | |||
| 78 | + | Size: domain.Size{Cols: 80, Rows: 24}, Cwd: clientCwd, Env: clientEnv, | |||
| 79 | + | EnvironmentPolicy: tt.policy, | |||
| 80 | + | }, &closeTrackingTransport{}) | |||
| 81 | + | require.NoError(t, err) | |||
| 82 | + | require.NotNil(t, ac) | |||
| 83 | + | sess.mu.Lock() | |||
| 84 | + | require.Equal(t, wantEnv, sess.env, "future PTYs must retain the selected environment") | |||
| 85 | + | sess.mu.Unlock() | |||
| 86 | + | require.NoError(t, d.killSession(sess, protocol.ReasonSessionKilled, true)) | |||
| 87 | + | }) | |||
| 88 | + | } | |||
| 89 | + | } | |||
M internal/usecase/daemon/remote_stopped_route_test.go +1 -2
| @@ -14,12 +14,11 @@ import ( | |||
| 14 | 14 | recoveryusecase "github.com/bnema/vev/internal/usecase/recovery" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | - | func TestDaemonOwnedNoExactTargetRejectsNewAndEphemeral(t *testing.T) { | |
| 17 | + | func TestDaemonOwnedNoExactTargetRejectsEphemeral(t *testing.T) { | |
| 18 | 18 | for _, tt := range []struct { | |
| 19 | 19 | name string | |
| 20 | 20 | intent uint8 | |
| 21 | 21 | }{ | |
| 22 | - | {name: "new", intent: protocol.IntentNew}, | |
| 23 | 22 | {name: "ephemeral", intent: protocol.IntentEphemeral}, | |
| 24 | 23 | } { | |
| 25 | 24 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -14,12 +14,11 @@ import ( | |||||
| 14 | recoveryusecase "github.com/bnema/vev/internal/usecase/recovery" | 14 | recoveryusecase "github.com/bnema/vev/internal/usecase/recovery" | ||
| 15 | ) | 15 | ) | ||
| 16 | 16 | ||||
| 17 | - | func TestDaemonOwnedNoExactTargetRejectsNewAndEphemeral(t *testing.T) { | 17 | + | func TestDaemonOwnedNoExactTargetRejectsEphemeral(t *testing.T) { |
| 18 | for _, tt := range []struct { | 18 | for _, tt := range []struct { | ||
| 19 | name string | 19 | name string | ||
| 20 | intent uint8 | 20 | intent uint8 | ||
| 21 | }{ | 21 | }{ | ||
| 22 | - | {name: "new", intent: protocol.IntentNew}, | |||
| 23 | {name: "ephemeral", intent: protocol.IntentEphemeral}, | 22 | {name: "ephemeral", intent: protocol.IntentEphemeral}, | ||
| 24 | } { | 23 | } { | ||
| 25 | t.Run(tt.name, func(t *testing.T) { | 24 | t.Run(tt.name, func(t *testing.T) { | ||