refactor(daemon): centralize attach environment ownership
- commit
- d1042d051011153e3159054d4686290459a0ea8f
- author
- brice · · 2026-07-15 06:05 UTC
- parent
- 811c2e4
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +109 -103
M internal/usecase/daemon/client.go +3 -25
| @@ -55,8 +55,6 @@ type attachedClient struct { | |||
| 55 | 55 | // previousSession is guarded independently. It is retained through temporary | |
| 56 | 56 | // setSession(nil) hand-offs and cleared only on terminal teardown. | |
| 57 | 57 | previousSession Guarded[*session] | |
| 58 | - | envMu sync.Mutex // guards the immutable attach environment snapshot | |
| 59 | - | env []string | |
| 60 | 58 | linkMu sync.Mutex | |
| 61 | 59 | sendMu sync.Mutex | |
| 62 | 60 | } | |
| @@ -109,18 +107,6 @@ func (ac *attachedClient) currentSession() *session { return ac.sess.Get() } | |||
| 109 | 107 | ||
| 110 | 108 | func (ac *attachedClient) setSession(sess *session) { ac.sess.Set(sess) } | |
| 111 | 109 | ||
| 112 | - | func (ac *attachedClient) environment() []string { | |
| 113 | - | ac.envMu.Lock() | |
| 114 | - | defer ac.envMu.Unlock() | |
| 115 | - | return copyEnvironment(ac.env) | |
| 116 | - | } | |
| 117 | - | ||
| 118 | - | func (ac *attachedClient) setEnvironment(env []string) { | |
| 119 | - | ac.envMu.Lock() | |
| 120 | - | ac.env = copyEnvironment(env) | |
| 121 | - | ac.envMu.Unlock() | |
| 122 | - | } | |
| 123 | - | ||
| 124 | 110 | func (ac *attachedClient) clearPreviousSession() { | |
| 125 | 111 | if ac != nil { | |
| 126 | 112 | ac.previousSession.Set(nil) | |
| @@ -406,12 +392,9 @@ func (d *Daemon) notifiesSnapshot() []chan struct{} { | |||
| 406 | 392 | } | |
| 407 | 393 | ||
| 408 | 394 | type attachClientOptions struct { | |
| 409 | - | clientID [16]byte | |
| 410 | - | resumeCapable bool | |
| 411 | - | maxOutputInFlight uint8 | |
| 412 | - | environment []string | |
| 413 | - | terminal terminalEnv | |
| 414 | - | refreshEnvironment bool | |
| 395 | + | clientID [16]byte | |
| 396 | + | resumeCapable bool | |
| 397 | + | maxOutputInFlight uint8 | |
| 415 | 398 | } | |
| 416 | 399 | ||
| 417 | 400 | func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, opts attachClientOptions) (*attachedClient, *attachedClient) { | |
| @@ -432,7 +415,6 @@ func (d *Daemon) attachClientDeferred(sess *session, tr ports.Transport, sz doma | |||
| 432 | 415 | clientID: opts.clientID, | |
| 433 | 416 | resumeCapable: opts.resumeCapable, | |
| 434 | 417 | resumeToken: resumeToken, | |
| 435 | - | env: copyEnvironment(opts.environment), | |
| 436 | 418 | } | |
| 437 | 419 | ac.initOverlays() | |
| 438 | 420 | ac.setSession(sess) | |
| @@ -443,10 +425,6 @@ func (d *Daemon) attachClientDeferred(sess *session, tr ports.Transport, sz doma | |||
| 443 | 425 | sess.mu.Lock() | |
| 444 | 426 | old := sess.client | |
| 445 | 427 | name := sess.name | |
| 446 | - | if opts.refreshEnvironment { | |
| 447 | - | sess.env = copyEnvironment(opts.environment) | |
| 448 | - | sess.terminal = opts.terminal | |
| 449 | - | } | |
| 450 | 428 | sess.mu.Unlock() | |
| 451 | 429 | _, cleanup := d.attachCoordinatorDeferred(sess, old, ac, false) | |
| 452 | 430 | sess.mu.Lock() | |
| @@ -55,8 +55,6 @@ type attachedClient struct { | |||||
| 55 | // previousSession is guarded independently. It is retained through temporary | 55 | // previousSession is guarded independently. It is retained through temporary | ||
| 56 | // setSession(nil) hand-offs and cleared only on terminal teardown. | 56 | // setSession(nil) hand-offs and cleared only on terminal teardown. | ||
| 57 | previousSession Guarded[*session] | 57 | previousSession Guarded[*session] | ||
| 58 | - | envMu sync.Mutex // guards the immutable attach environment snapshot | |||
| 59 | - | env []string | |||
| 60 | linkMu sync.Mutex | 58 | linkMu sync.Mutex | ||
| 61 | sendMu sync.Mutex | 59 | sendMu sync.Mutex | ||
| 62 | } | 60 | } | ||
| @@ -109,18 +107,6 @@ func (ac *attachedClient) currentSession() *session { return ac.sess.Get() } | |||||
| 109 | 107 | ||||
| 110 | func (ac *attachedClient) setSession(sess *session) { ac.sess.Set(sess) } | 108 | func (ac *attachedClient) setSession(sess *session) { ac.sess.Set(sess) } | ||
| 111 | 109 | ||||
| 112 | - | func (ac *attachedClient) environment() []string { | |||
| 113 | - | ac.envMu.Lock() | |||
| 114 | - | defer ac.envMu.Unlock() | |||
| 115 | - | return copyEnvironment(ac.env) | |||
| 116 | - | } | |||
| 117 | - | ||||
| 118 | - | func (ac *attachedClient) setEnvironment(env []string) { | |||
| 119 | - | ac.envMu.Lock() | |||
| 120 | - | ac.env = copyEnvironment(env) | |||
| 121 | - | ac.envMu.Unlock() | |||
| 122 | - | } | |||
| 123 | - | ||||
| 124 | func (ac *attachedClient) clearPreviousSession() { | 110 | func (ac *attachedClient) clearPreviousSession() { | ||
| 125 | if ac != nil { | 111 | if ac != nil { | ||
| 126 | ac.previousSession.Set(nil) | 112 | ac.previousSession.Set(nil) | ||
| @@ -406,12 +392,9 @@ func (d *Daemon) notifiesSnapshot() []chan struct{} { | |||||
| 406 | } | 392 | } | ||
| 407 | 393 | ||||
| 408 | type attachClientOptions struct { | 394 | type attachClientOptions struct { | ||
| 409 | - | clientID [16]byte | 395 | + | clientID [16]byte |
| 410 | - | resumeCapable bool | 396 | + | resumeCapable bool |
| 411 | - | maxOutputInFlight uint8 | 397 | + | maxOutputInFlight uint8 |
| 412 | - | environment []string | |||
| 413 | - | terminal terminalEnv | |||
| 414 | - | refreshEnvironment bool | |||
| 415 | } | 398 | } | ||
| 416 | 399 | ||||
| 417 | func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, opts attachClientOptions) (*attachedClient, *attachedClient) { | 400 | func (d *Daemon) attachClient(sess *session, tr ports.Transport, sz domain.Size, opts attachClientOptions) (*attachedClient, *attachedClient) { | ||
| @@ -432,7 +415,6 @@ func (d *Daemon) attachClientDeferred(sess *session, tr ports.Transport, sz doma | |||||
| 432 | clientID: opts.clientID, | 415 | clientID: opts.clientID, | ||
| 433 | resumeCapable: opts.resumeCapable, | 416 | resumeCapable: opts.resumeCapable, | ||
| 434 | resumeToken: resumeToken, | 417 | resumeToken: resumeToken, | ||
| 435 | - | env: copyEnvironment(opts.environment), | |||
| 436 | } | 418 | } | ||
| 437 | ac.initOverlays() | 419 | ac.initOverlays() | ||
| 438 | ac.setSession(sess) | 420 | ac.setSession(sess) | ||
| @@ -443,10 +425,6 @@ func (d *Daemon) attachClientDeferred(sess *session, tr ports.Transport, sz doma | |||||
| 443 | sess.mu.Lock() | 425 | sess.mu.Lock() | ||
| 444 | old := sess.client | 426 | old := sess.client | ||
| 445 | name := sess.name | 427 | name := sess.name | ||
| 446 | - | if opts.refreshEnvironment { | |||
| 447 | - | sess.env = copyEnvironment(opts.environment) | |||
| 448 | - | sess.terminal = opts.terminal | |||
| 449 | - | } | |||
| 450 | sess.mu.Unlock() | 428 | sess.mu.Unlock() | ||
| 451 | _, cleanup := d.attachCoordinatorDeferred(sess, old, ac, false) | 429 | _, cleanup := d.attachCoordinatorDeferred(sess, old, ac, false) | ||
| 452 | sess.mu.Lock() | 430 | sess.mu.Lock() | ||
M internal/usecase/daemon/daemon.go +13 -10
| @@ -672,13 +672,16 @@ func (e *protoErr) Error() string { return e.text } | |||
| 672 | 672 | // publishes the terminal state before releasing d.mu, then queues replacement | |
| 673 | 673 | // teardown so obsolete worker joins never delay the new handshake. | |
| 674 | 674 | func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient { | |
| 675 | + | // Session state is the sole source for future PTY children. Update it before | |
| 676 | + | // publishing the attachment; existing PTYs keep their original environment. | |
| 677 | + | sess.mu.Lock() | |
| 678 | + | sess.env = copyEnvironment(h.Env) | |
| 679 | + | sess.terminal = term | |
| 680 | + | sess.mu.Unlock() | |
| 675 | 681 | ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{ | |
| 676 | - | clientID: h.ClientID, | |
| 677 | - | resumeCapable: true, | |
| 678 | - | maxOutputInFlight: normalizeOutputWindow(h.MaxOutputInFlight), | |
| 679 | - | environment: h.Env, | |
| 680 | - | terminal: term, | |
| 681 | - | refreshEnvironment: true, | |
| 682 | + | clientID: h.ClientID, | |
| 683 | + | resumeCapable: true, | |
| 684 | + | maxOutputInFlight: normalizeOutputWindow(h.MaxOutputInFlight), | |
| 682 | 685 | }) | |
| 683 | 686 | d.mu.Unlock() | |
| 684 | 687 | d.retireReplacedClient(old, cleanup) | |
| @@ -728,7 +731,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||
| 728 | 731 | } | |
| 729 | 732 | cwd := d.dirOrHome(stopped.cwd) | |
| 730 | 733 | var err error | |
| 731 | - | sess, err = d.createSessionWithEnvLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | |
| 734 | + | sess, err = d.createSessionLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | |
| 732 | 735 | if err != nil { | |
| 733 | 736 | d.mu.Unlock() | |
| 734 | 737 | return nil, nil, err | |
| @@ -739,7 +742,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||
| 739 | 742 | ||
| 740 | 743 | case ports.IntentEphemeral: | |
| 741 | 744 | name := d.allocEphemeralNameLocked() | |
| 742 | - | sess, err := d.createSessionWithEnvLocked(name, true, h.Cwd, sz, term, h.Env) | |
| 745 | + | sess, err := d.createSessionLocked(name, true, h.Cwd, sz, term, h.Env) | |
| 743 | 746 | if err != nil { | |
| 744 | 747 | d.mu.Unlock() | |
| 745 | 748 | return nil, nil, err | |
| @@ -760,7 +763,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||
| 760 | 763 | d.mu.Unlock() | |
| 761 | 764 | return nil, nil, &protoErr{ports.ErrNameTaken, "session name already in use: " + h.Name} | |
| 762 | 765 | } | |
| 763 | - | sess, err := d.createSessionWithEnvLocked(h.Name, false, h.Cwd, sz, term, h.Env) | |
| 766 | + | sess, err := d.createSessionLocked(h.Name, false, h.Cwd, sz, term, h.Env) | |
| 764 | 767 | if err != nil { | |
| 765 | 768 | d.mu.Unlock() | |
| 766 | 769 | return nil, nil, err | |
| @@ -778,7 +781,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||
| 778 | 781 | } | |
| 779 | 782 | cwd := d.dirOrHome(stopped.cwd) | |
| 780 | 783 | var err error | |
| 781 | - | sess, err = d.createSessionWithEnvLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | |
| 784 | + | sess, err = d.createSessionLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | |
| 782 | 785 | if err != nil { | |
| 783 | 786 | d.mu.Unlock() | |
| 784 | 787 | return nil, nil, err | |
| @@ -672,13 +672,16 @@ func (e *protoErr) Error() string { return e.text } | |||||
| 672 | // publishes the terminal state before releasing d.mu, then queues replacement | 672 | // publishes the terminal state before releasing d.mu, then queues replacement | ||
| 673 | // teardown so obsolete worker joins never delay the new handshake. | 673 | // teardown so obsolete worker joins never delay the new handshake. | ||
| 674 | func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient { | 674 | func (d *Daemon) finishAttach(sess *session, tr ports.Transport, sz domain.Size, term terminalEnv, h ports.Hello) *attachedClient { | ||
| 675 | + | // Session state is the sole source for future PTY children. Update it before | |||
| 676 | + | // publishing the attachment; existing PTYs keep their original environment. | |||
| 677 | + | sess.mu.Lock() | |||
| 678 | + | sess.env = copyEnvironment(h.Env) | |||
| 679 | + | sess.terminal = term | |||
| 680 | + | sess.mu.Unlock() | |||
| 675 | ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{ | 681 | ac, old, cleanup := d.attachClientDeferred(sess, tr, sz, attachClientOptions{ | ||
| 676 | - | clientID: h.ClientID, | 682 | + | clientID: h.ClientID, |
| 677 | - | resumeCapable: true, | 683 | + | resumeCapable: true, |
| 678 | - | maxOutputInFlight: normalizeOutputWindow(h.MaxOutputInFlight), | 684 | + | maxOutputInFlight: normalizeOutputWindow(h.MaxOutputInFlight), |
| 679 | - | environment: h.Env, | |||
| 680 | - | terminal: term, | |||
| 681 | - | refreshEnvironment: true, | |||
| 682 | }) | 685 | }) | ||
| 683 | d.mu.Unlock() | 686 | d.mu.Unlock() | ||
| 684 | d.retireReplacedClient(old, cleanup) | 687 | d.retireReplacedClient(old, cleanup) | ||
| @@ -728,7 +731,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||||
| 728 | } | 731 | } | ||
| 729 | cwd := d.dirOrHome(stopped.cwd) | 732 | cwd := d.dirOrHome(stopped.cwd) | ||
| 730 | var err error | 733 | var err error | ||
| 731 | - | sess, err = d.createSessionWithEnvLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | 734 | + | sess, err = d.createSessionLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) |
| 732 | if err != nil { | 735 | if err != nil { | ||
| 733 | d.mu.Unlock() | 736 | d.mu.Unlock() | ||
| 734 | return nil, nil, err | 737 | return nil, nil, err | ||
| @@ -739,7 +742,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||||
| 739 | 742 | ||||
| 740 | case ports.IntentEphemeral: | 743 | case ports.IntentEphemeral: | ||
| 741 | name := d.allocEphemeralNameLocked() | 744 | name := d.allocEphemeralNameLocked() | ||
| 742 | - | sess, err := d.createSessionWithEnvLocked(name, true, h.Cwd, sz, term, h.Env) | 745 | + | sess, err := d.createSessionLocked(name, true, h.Cwd, sz, term, h.Env) |
| 743 | if err != nil { | 746 | if err != nil { | ||
| 744 | d.mu.Unlock() | 747 | d.mu.Unlock() | ||
| 745 | return nil, nil, err | 748 | return nil, nil, err | ||
| @@ -760,7 +763,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||||
| 760 | d.mu.Unlock() | 763 | d.mu.Unlock() | ||
| 761 | return nil, nil, &protoErr{ports.ErrNameTaken, "session name already in use: " + h.Name} | 764 | return nil, nil, &protoErr{ports.ErrNameTaken, "session name already in use: " + h.Name} | ||
| 762 | } | 765 | } | ||
| 763 | - | sess, err := d.createSessionWithEnvLocked(h.Name, false, h.Cwd, sz, term, h.Env) | 766 | + | sess, err := d.createSessionLocked(h.Name, false, h.Cwd, sz, term, h.Env) |
| 764 | if err != nil { | 767 | if err != nil { | ||
| 765 | d.mu.Unlock() | 768 | d.mu.Unlock() | ||
| 766 | return nil, nil, err | 769 | return nil, nil, err | ||
| @@ -778,7 +781,7 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl | |||||
| 778 | } | 781 | } | ||
| 779 | cwd := d.dirOrHome(stopped.cwd) | 782 | cwd := d.dirOrHome(stopped.cwd) | ||
| 780 | var err error | 783 | var err error | ||
| 781 | - | sess, err = d.createSessionWithEnvLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) | 784 | + | sess, err = d.createSessionLocked(h.Name, false, cwd, sz, term, h.Env, stopped.tabNames) |
| 782 | if err != nil { | 785 | if err != nil { | ||
| 783 | d.mu.Unlock() | 786 | d.mu.Unlock() | ||
| 784 | return nil, nil, err | 787 | return nil, nil, err | ||
M internal/usecase/daemon/floating_test.go +5 -3
| @@ -24,6 +24,7 @@ import ( | |||
| 24 | 24 | type gatedFloatingOpen struct { | |
| 25 | 25 | command string | |
| 26 | 26 | args []string | |
| 27 | + | env []string | |
| 27 | 28 | dir string | |
| 28 | 29 | size domain.Size | |
| 29 | 30 | } | |
| @@ -37,8 +38,8 @@ func newGatedOpenFactory(t *testing.T, result ports.PTY, openErr error) (*portsm | |||
| 37 | 38 | releaseOpen := func() { releaseOnce.Do(func() { close(release) }) } | |
| 38 | 39 | t.Cleanup(releaseOpen) | |
| 39 | 40 | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 40 | - | func(_ context.Context, command string, args, _ []string, dir string, size domain.Size) (ports.PTY, error) { | |
| 41 | - | opened <- gatedFloatingOpen{command: command, args: append([]string(nil), args...), dir: dir, size: size} | |
| 41 | + | func(_ context.Context, command string, args, env []string, dir string, size domain.Size) (ports.PTY, error) { | |
| 42 | + | opened <- gatedFloatingOpen{command: command, args: append([]string(nil), args...), env: append([]string(nil), env...), dir: dir, size: size} | |
| 42 | 43 | <-release | |
| 43 | 44 | return result, openErr | |
| 44 | 45 | }).Once() | |
| @@ -146,7 +147,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||
| 146 | 147 | cwd := t.TempDir() | |
| 147 | 148 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | |
| 148 | 149 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | |
| 149 | - | sess := &session{name: "work", cwd: cwd, env: []string{"SHELL=/bin/custom-shell"}, tabs: []*tab{tb}, ctx: t.Context()} | |
| 150 | + | sess := &session{name: "work", cwd: cwd, terminal: terminalEnv{TrueColor: true}, env: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/bin/custom-shell", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"}, tabs: []*tab{tb}, ctx: t.Context()} | |
| 150 | 151 | d.ApplyConfig(domain.Config{Theme: domain.ThemeDark, Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | |
| 151 | 152 | d.ensureFloatingWarm(sess, tb) | |
| 152 | 153 | // Open has started while this goroutine owns tab.mu: an external factory | |
| @@ -170,6 +171,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||
| 170 | 171 | floating := tb.floating.pane | |
| 171 | 172 | tb.mu.Unlock() | |
| 172 | 173 | require.NotNil(t, floating) | |
| 174 | + | require.Equal(t, []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/bin/custom-shell", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + tb.stableID + ",pane=" + floating.stableID}, openCall.env) | |
| 173 | 175 | assertPaneDefaultColors(t, floating, themeui.BuiltinDark.Foreground, themeui.BuiltinDark.Background) | |
| 174 | 176 | assertPaneColorScheme(t, floating, false) | |
| 175 | 177 | require.Equal(t, "/bin/custom-shell", openCall.command) | |
| @@ -24,6 +24,7 @@ import ( | |||||
| 24 | type gatedFloatingOpen struct { | 24 | type gatedFloatingOpen struct { | ||
| 25 | command string | 25 | command string | ||
| 26 | args []string | 26 | args []string | ||
| 27 | + | env []string | |||
| 27 | dir string | 28 | dir string | ||
| 28 | size domain.Size | 29 | size domain.Size | ||
| 29 | } | 30 | } | ||
| @@ -37,8 +38,8 @@ func newGatedOpenFactory(t *testing.T, result ports.PTY, openErr error) (*portsm | |||||
| 37 | releaseOpen := func() { releaseOnce.Do(func() { close(release) }) } | 38 | releaseOpen := func() { releaseOnce.Do(func() { close(release) }) } | ||
| 38 | t.Cleanup(releaseOpen) | 39 | t.Cleanup(releaseOpen) | ||
| 39 | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | 40 | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | ||
| 40 | - | func(_ context.Context, command string, args, _ []string, dir string, size domain.Size) (ports.PTY, error) { | 41 | + | func(_ context.Context, command string, args, env []string, dir string, size domain.Size) (ports.PTY, error) { |
| 41 | - | opened <- gatedFloatingOpen{command: command, args: append([]string(nil), args...), dir: dir, size: size} | 42 | + | opened <- gatedFloatingOpen{command: command, args: append([]string(nil), args...), env: append([]string(nil), env...), dir: dir, size: size} |
| 42 | <-release | 43 | <-release | ||
| 43 | return result, openErr | 44 | return result, openErr | ||
| 44 | }).Once() | 45 | }).Once() | ||
| @@ -146,7 +147,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||||
| 146 | cwd := t.TempDir() | 147 | cwd := t.TempDir() | ||
| 147 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | 148 | tb := newTabWithStableID("t_stable", "p_normal", newBlockingPanePTY(t), domain.Size{Cols: 100, Rows: 40}) | ||
| 148 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | 149 | tb.ctx, tb.cancel = context.WithCancel(t.Context()) | ||
| 149 | - | sess := &session{name: "work", cwd: cwd, env: []string{"SHELL=/bin/custom-shell"}, tabs: []*tab{tb}, ctx: t.Context()} | 150 | + | sess := &session{name: "work", cwd: cwd, terminal: terminalEnv{TrueColor: true}, env: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/bin/custom-shell", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"}, tabs: []*tab{tb}, ctx: t.Context()} |
| 150 | d.ApplyConfig(domain.Config{Theme: domain.ThemeDark, Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | 151 | d.ApplyConfig(domain.Config{Theme: domain.ThemeDark, Floating: domain.FloatingConfig{Command: "btop --utf", Width: 50, Height: 50}}) | ||
| 151 | d.ensureFloatingWarm(sess, tb) | 152 | d.ensureFloatingWarm(sess, tb) | ||
| 152 | // Open has started while this goroutine owns tab.mu: an external factory | 153 | // Open has started while this goroutine owns tab.mu: an external factory | ||
| @@ -170,6 +171,7 @@ func TestFloatingLifecycleCapturesLaunchBeforeOpenAndDoesNotHoldTabLock(t *testi | |||||
| 170 | floating := tb.floating.pane | 171 | floating := tb.floating.pane | ||
| 171 | tb.mu.Unlock() | 172 | tb.mu.Unlock() | ||
| 172 | require.NotNil(t, floating) | 173 | require.NotNil(t, floating) | ||
| 174 | + | require.Equal(t, []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/bin/custom-shell", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + tb.stableID + ",pane=" + floating.stableID}, openCall.env) | |||
| 173 | assertPaneDefaultColors(t, floating, themeui.BuiltinDark.Foreground, themeui.BuiltinDark.Background) | 175 | assertPaneDefaultColors(t, floating, themeui.BuiltinDark.Foreground, themeui.BuiltinDark.Background) | ||
| 174 | assertPaneColorScheme(t, floating, false) | 176 | assertPaneColorScheme(t, floating, false) | ||
| 175 | require.Equal(t, "/bin/custom-shell", openCall.command) | 177 | require.Equal(t, "/bin/custom-shell", openCall.command) | ||
M internal/usecase/daemon/pane_actions_test.go +24 -24
| @@ -331,48 +331,48 @@ func blockingRead(t *testing.T) func([]byte) (int, error) { | |||
| 331 | 331 | } | |
| 332 | 332 | } | |
| 333 | 333 | ||
| 334 | - | func TestSplitPaneInheritsTerminalEnv(t *testing.T) { | |
| 335 | - | tests := []struct { | |
| 336 | - | name string | |
| 337 | - | term terminalEnv | |
| 338 | - | wantContain []string | |
| 339 | - | wantNotContain []string | |
| 334 | + | func TestSplitPaneUsesExactAuthoritativeEnvironment(t *testing.T) { | |
| 335 | + | for _, tt := range []struct { | |
| 336 | + | name string | |
| 337 | + | term terminalEnv | |
| 338 | + | want []string | |
| 340 | 339 | }{ | |
| 341 | 340 | { | |
| 342 | - | name: "truecolor", | |
| 343 | - | term: terminalEnv{TrueColor: true}, | |
| 344 | - | wantContain: []string{"TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev"}, | |
| 341 | + | name: "truecolor", | |
| 342 | + | term: terminalEnv{TrueColor: true}, | |
| 343 | + | want: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev"}, | |
| 345 | 344 | }, | |
| 346 | 345 | { | |
| 347 | - | name: "no truecolor", | |
| 348 | - | term: terminalEnv{}, | |
| 349 | - | wantContain: []string{"TERM=xterm-256color", "TERM_PROGRAM=vev"}, | |
| 350 | - | wantNotContain: []string{"TERM=xterm-direct", "COLORTERM=truecolor"}, | |
| 346 | + | name: "no truecolor", | |
| 347 | + | want: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=xterm-256color", "TERM_PROGRAM=vev"}, | |
| 351 | 348 | }, | |
| 352 | - | } | |
| 353 | - | ||
| 354 | - | for _, tt := range tests { | |
| 349 | + | } { | |
| 355 | 350 | t.Run(tt.name, func(t *testing.T) { | |
| 356 | 351 | d, sess, oldPTY, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | |
| 352 | + | d.shellOverride = false | |
| 357 | 353 | sess.terminal = tt.term | |
| 354 | + | sess.env = []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | |
| 358 | 355 | newPTY := portsmocks.NewMockPTY(t) | |
| 359 | 356 | oldPTY.EXPECT().Resize(domain.Size{Cols: 20, Rows: 10}).Return(nil).Once() | |
| 360 | 357 | newPTY.EXPECT().Read(mock.Anything).RunAndReturn(blockingRead(t)).Maybe() | |
| 358 | + | var command string | |
| 361 | 359 | var gotEnv []string | |
| 362 | - | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).RunAndReturn( | |
| 363 | - | func(_ context.Context, _ string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 360 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).RunAndReturn( | |
| 361 | + | func(_ context.Context, gotCommand string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) { | |
| 362 | + | command = gotCommand | |
| 364 | 363 | gotEnv = append([]string(nil), env...) | |
| 365 | 364 | return newPTY, nil | |
| 366 | 365 | }, | |
| 367 | 366 | ).Once() | |
| 368 | 367 | ||
| 369 | 368 | require.NoError(t, d.splitPane(sess, nil, layout.Right)) | |
| 370 | - | for _, want := range tt.wantContain { | |
| 371 | - | require.Contains(t, gotEnv, want) | |
| 372 | - | } | |
| 373 | - | for _, notWant := range tt.wantNotContain { | |
| 374 | - | require.NotContains(t, gotEnv, notWant) | |
| 375 | - | } | |
| 369 | + | tb := sess.activeTab() | |
| 370 | + | tb.mu.Lock() | |
| 371 | + | paneID := tb.panes["pane-2"].stableID | |
| 372 | + | tabID := tb.stableID | |
| 373 | + | tb.mu.Unlock() | |
| 374 | + | require.Equal(t, "/usr/bin/fish", command) | |
| 375 | + | require.Equal(t, append(tt.want, "VEV=session=s,tab="+tabID+",pane="+paneID), gotEnv) | |
| 376 | 376 | }) | |
| 377 | 377 | } | |
| 378 | 378 | } | |
| @@ -331,48 +331,48 @@ func blockingRead(t *testing.T) func([]byte) (int, error) { | |||||
| 331 | } | 331 | } | ||
| 332 | } | 332 | } | ||
| 333 | 333 | ||||
| 334 | - | func TestSplitPaneInheritsTerminalEnv(t *testing.T) { | 334 | + | func TestSplitPaneUsesExactAuthoritativeEnvironment(t *testing.T) { |
| 335 | - | tests := []struct { | 335 | + | for _, tt := range []struct { |
| 336 | - | name string | 336 | + | name string |
| 337 | - | term terminalEnv | 337 | + | term terminalEnv |
| 338 | - | wantContain []string | 338 | + | want []string |
| 339 | - | wantNotContain []string | |||
| 340 | }{ | 339 | }{ | ||
| 341 | { | 340 | { | ||
| 342 | - | name: "truecolor", | 341 | + | name: "truecolor", |
| 343 | - | term: terminalEnv{TrueColor: true}, | 342 | + | term: terminalEnv{TrueColor: true}, |
| 344 | - | wantContain: []string{"TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev"}, | 343 | + | want: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev"}, |
| 345 | }, | 344 | }, | ||
| 346 | { | 345 | { | ||
| 347 | - | name: "no truecolor", | 346 | + | name: "no truecolor", |
| 348 | - | term: terminalEnv{}, | 347 | + | want: []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=xterm-256color", "TERM_PROGRAM=vev"}, |
| 349 | - | wantContain: []string{"TERM=xterm-256color", "TERM_PROGRAM=vev"}, | |||
| 350 | - | wantNotContain: []string{"TERM=xterm-direct", "COLORTERM=truecolor"}, | |||
| 351 | }, | 348 | }, | ||
| 352 | - | } | 349 | + | } { |
| 353 | - | ||||
| 354 | - | for _, tt := range tests { | |||
| 355 | t.Run(tt.name, func(t *testing.T) { | 350 | t.Run(tt.name, func(t *testing.T) { | ||
| 356 | d, sess, oldPTY, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | 351 | d, sess, oldPTY, factory := newSplitTestDaemon(t, domain.Size{Cols: 41, Rows: 10}) | ||
| 352 | + | d.shellOverride = false | |||
| 357 | sess.terminal = tt.term | 353 | sess.terminal = tt.term | ||
| 354 | + | sess.env = []string{"ORDINARY=preserved", "DUP=first", "DUP=second", "PAIR=a=b", "SHELL=/bin/first", "SHELL=/usr/bin/fish", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | |||
| 358 | newPTY := portsmocks.NewMockPTY(t) | 355 | newPTY := portsmocks.NewMockPTY(t) | ||
| 359 | oldPTY.EXPECT().Resize(domain.Size{Cols: 20, Rows: 10}).Return(nil).Once() | 356 | oldPTY.EXPECT().Resize(domain.Size{Cols: 20, Rows: 10}).Return(nil).Once() | ||
| 360 | newPTY.EXPECT().Read(mock.Anything).RunAndReturn(blockingRead(t)).Maybe() | 357 | newPTY.EXPECT().Read(mock.Anything).RunAndReturn(blockingRead(t)).Maybe() | ||
| 358 | + | var command string | |||
| 361 | var gotEnv []string | 359 | var gotEnv []string | ||
| 362 | - | factory.EXPECT().Open(mock.Anything, "/bin/sh", []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).RunAndReturn( | 360 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, []string(nil), mock.Anything, "/work", domain.Size{Cols: 20, Rows: 10}).RunAndReturn( |
| 363 | - | func(_ context.Context, _ string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) { | 361 | + | func(_ context.Context, gotCommand string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) { |
| 362 | + | command = gotCommand | |||
| 364 | gotEnv = append([]string(nil), env...) | 363 | gotEnv = append([]string(nil), env...) | ||
| 365 | return newPTY, nil | 364 | return newPTY, nil | ||
| 366 | }, | 365 | }, | ||
| 367 | ).Once() | 366 | ).Once() | ||
| 368 | 367 | ||||
| 369 | require.NoError(t, d.splitPane(sess, nil, layout.Right)) | 368 | require.NoError(t, d.splitPane(sess, nil, layout.Right)) | ||
| 370 | - | for _, want := range tt.wantContain { | 369 | + | tb := sess.activeTab() |
| 371 | - | require.Contains(t, gotEnv, want) | 370 | + | tb.mu.Lock() |
| 372 | - | } | 371 | + | paneID := tb.panes["pane-2"].stableID |
| 373 | - | for _, notWant := range tt.wantNotContain { | 372 | + | tabID := tb.stableID |
| 374 | - | require.NotContains(t, gotEnv, notWant) | 373 | + | tb.mu.Unlock() |
| 375 | - | } | 374 | + | require.Equal(t, "/usr/bin/fish", command) |
| 375 | + | require.Equal(t, append(tt.want, "VEV=session=s,tab="+tabID+",pane="+paneID), gotEnv) | |||
| 376 | }) | 376 | }) | ||
| 377 | } | 377 | } | ||
| 378 | } | 378 | } | ||
M internal/usecase/daemon/picker.go +3 -3
| @@ -415,7 +415,6 @@ func (d *Daemon) switchToTarget(sess *session, ac *attachedClient, target picker | |||
| 415 | 415 | } | |
| 416 | 416 | ||
| 417 | 417 | func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetSess *session, target picker.Target) *attachedClient { | |
| 418 | - | env := ac.environment() | |
| 419 | 418 | d.mu.Lock() | |
| 420 | 419 | if d.sessions[target.Session] != targetSess { | |
| 421 | 420 | d.mu.Unlock() | |
| @@ -428,6 +427,7 @@ func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetS | |||
| 428 | 427 | return nil | |
| 429 | 428 | } | |
| 430 | 429 | term := from.terminal | |
| 430 | + | env := copyEnvironment(from.env) | |
| 431 | 431 | from.client = nil | |
| 432 | 432 | ac.setSession(nil) | |
| 433 | 433 | from.mu.Unlock() | |
| @@ -470,9 +470,9 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe | |||
| 470 | 470 | return false | |
| 471 | 471 | } | |
| 472 | 472 | term := from.terminal | |
| 473 | - | env := ac.environment() | |
| 473 | + | env := copyEnvironment(from.env) | |
| 474 | 474 | cwd := d.dirOrHome(stopped.cwd) | |
| 475 | - | targetSess, err := d.createSessionWithEnvLocked(target.Name, false, cwd, ac.size, term, env, stopped.tabNames) | |
| 475 | + | targetSess, err := d.createSessionLocked(target.Name, false, cwd, ac.size, term, env, stopped.tabNames) | |
| 476 | 476 | if err != nil { | |
| 477 | 477 | from.mu.Unlock() | |
| 478 | 478 | d.mu.Unlock() | |
| @@ -415,7 +415,6 @@ func (d *Daemon) switchToTarget(sess *session, ac *attachedClient, target picker | |||||
| 415 | } | 415 | } | ||
| 416 | 416 | ||||
| 417 | func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetSess *session, target picker.Target) *attachedClient { | 417 | func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetSess *session, target picker.Target) *attachedClient { | ||
| 418 | - | env := ac.environment() | |||
| 419 | d.mu.Lock() | 418 | d.mu.Lock() | ||
| 420 | if d.sessions[target.Session] != targetSess { | 419 | if d.sessions[target.Session] != targetSess { | ||
| 421 | d.mu.Unlock() | 420 | d.mu.Unlock() | ||
| @@ -428,6 +427,7 @@ func (d *Daemon) stealClientForTarget(from *session, ac *attachedClient, targetS | |||||
| 428 | return nil | 427 | return nil | ||
| 429 | } | 428 | } | ||
| 430 | term := from.terminal | 429 | term := from.terminal | ||
| 430 | + | env := copyEnvironment(from.env) | |||
| 431 | from.client = nil | 431 | from.client = nil | ||
| 432 | ac.setSession(nil) | 432 | ac.setSession(nil) | ||
| 433 | from.mu.Unlock() | 433 | from.mu.Unlock() | ||
| @@ -470,9 +470,9 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe | |||||
| 470 | return false | 470 | return false | ||
| 471 | } | 471 | } | ||
| 472 | term := from.terminal | 472 | term := from.terminal | ||
| 473 | - | env := ac.environment() | 473 | + | env := copyEnvironment(from.env) |
| 474 | cwd := d.dirOrHome(stopped.cwd) | 474 | cwd := d.dirOrHome(stopped.cwd) | ||
| 475 | - | targetSess, err := d.createSessionWithEnvLocked(target.Name, false, cwd, ac.size, term, env, stopped.tabNames) | 475 | + | targetSess, err := d.createSessionLocked(target.Name, false, cwd, ac.size, term, env, stopped.tabNames) |
| 476 | if err != nil { | 476 | if err != nil { | ||
| 477 | from.mu.Unlock() | 477 | from.mu.Unlock() | ||
| 478 | d.mu.Unlock() | 478 | d.mu.Unlock() | ||
M internal/usecase/daemon/picker_test.go +8 -7
| @@ -466,9 +466,8 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) { | |||
| 466 | 466 | sctx2, cancel2 := context.WithCancel(d.serveCtx) | |
| 467 | 467 | defer cancel1() | |
| 468 | 468 | defer cancel2() | |
| 469 | - | sess1 := &session{id: "s1", name: "alpha", cwd: t.TempDir(), ctx: sctx1, cancel: cancel1, tabs: []*tab{newTestTabWithContext(p1, sctx1, cancel1)}, client: ac1, terminal: terminalEnv{}} | |
| 470 | - | sess2 := &session{id: "s2", name: "beta", cwd: t.TempDir(), ctx: sctx2, cancel: cancel2, tabs: []*tab{newTestTabWithContext(p2, sctx2, cancel2)}, client: ac2, terminal: terminalEnv{TrueColor: true}} | |
| 471 | - | ac1.setEnvironment([]string{"SECRET=from-active-client", "SHELL=/usr/bin/fish"}) | |
| 469 | + | sess1 := &session{id: "s1", name: "alpha", cwd: t.TempDir(), ctx: sctx1, cancel: cancel1, tabs: []*tab{newTestTabWithContext(p1, sctx1, cancel1)}, client: ac1, terminal: terminalEnv{}, env: []string{"SECRET=from-active-session", "SHELL=/usr/bin/fish", "PAIR=a=b"}} | |
| 470 | + | sess2 := &session{id: "s2", name: "beta", cwd: t.TempDir(), ctx: sctx2, cancel: cancel2, tabs: []*tab{newTestTabWithContext(p2, sctx2, cancel2)}, client: ac2, terminal: terminalEnv{TrueColor: true}, env: []string{"STALE=destination-only", "SHELL=/bin/stale", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=stale", "VEV=stale"}} | |
| 472 | 471 | ac1.setSession(sess1) | |
| 473 | 472 | ac2.setSession(sess2) | |
| 474 | 473 | d.sessions[sess1.id] = sess1 | |
| @@ -479,11 +478,13 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) { | |||
| 479 | 478 | require.Same(t, ac2, old) | |
| 480 | 479 | require.False(t, sess2.terminal.TrueColor) | |
| 481 | 480 | require.NoError(t, d.createTab(sess2, domain.Size{Cols: 80, Rows: 24})) | |
| 481 | + | sess2.mu.Lock() | |
| 482 | + | futureTabID := sess2.tabs[1].stableID | |
| 483 | + | futurePaneID := sess2.tabs[1].focusedPane().stableID | |
| 484 | + | sess2.mu.Unlock() | |
| 482 | 485 | require.Equal(t, "/usr/bin/fish", openedCommand) | |
| 483 | - | require.Contains(t, openedEnv, "SECRET=from-active-client") | |
| 484 | - | require.Contains(t, openedEnv, "TERM=xterm-256color") | |
| 485 | - | require.NotContains(t, openedEnv, "COLORTERM=truecolor") | |
| 486 | - | require.NotContains(t, openedEnv, "TERM=xterm-direct") | |
| 486 | + | require.Equal(t, []string{"SECRET=from-active-session", "SHELL=/usr/bin/fish", "PAIR=a=b", "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=beta,tab=" + futureTabID + ",pane=" + futurePaneID}, openedEnv) | |
| 487 | + | require.NotContains(t, openedEnv, "STALE=destination-only") | |
| 487 | 488 | } | |
| 488 | 489 | ||
| 489 | 490 | func TestPickerPreviewGenerationRejectsStaleSubscriptionReplacement(t *testing.T) { | |
| @@ -466,9 +466,8 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) { | |||||
| 466 | sctx2, cancel2 := context.WithCancel(d.serveCtx) | 466 | sctx2, cancel2 := context.WithCancel(d.serveCtx) | ||
| 467 | defer cancel1() | 467 | defer cancel1() | ||
| 468 | defer cancel2() | 468 | defer cancel2() | ||
| 469 | - | sess1 := &session{id: "s1", name: "alpha", cwd: t.TempDir(), ctx: sctx1, cancel: cancel1, tabs: []*tab{newTestTabWithContext(p1, sctx1, cancel1)}, client: ac1, terminal: terminalEnv{}} | 469 | + | sess1 := &session{id: "s1", name: "alpha", cwd: t.TempDir(), ctx: sctx1, cancel: cancel1, tabs: []*tab{newTestTabWithContext(p1, sctx1, cancel1)}, client: ac1, terminal: terminalEnv{}, env: []string{"SECRET=from-active-session", "SHELL=/usr/bin/fish", "PAIR=a=b"}} |
| 470 | - | sess2 := &session{id: "s2", name: "beta", cwd: t.TempDir(), ctx: sctx2, cancel: cancel2, tabs: []*tab{newTestTabWithContext(p2, sctx2, cancel2)}, client: ac2, terminal: terminalEnv{TrueColor: true}} | 470 | + | sess2 := &session{id: "s2", name: "beta", cwd: t.TempDir(), ctx: sctx2, cancel: cancel2, tabs: []*tab{newTestTabWithContext(p2, sctx2, cancel2)}, client: ac2, terminal: terminalEnv{TrueColor: true}, env: []string{"STALE=destination-only", "SHELL=/bin/stale", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=stale", "VEV=stale"}} |
| 471 | - | ac1.setEnvironment([]string{"SECRET=from-active-client", "SHELL=/usr/bin/fish"}) | |||
| 472 | ac1.setSession(sess1) | 471 | ac1.setSession(sess1) | ||
| 473 | ac2.setSession(sess2) | 472 | ac2.setSession(sess2) | ||
| 474 | d.sessions[sess1.id] = sess1 | 473 | d.sessions[sess1.id] = sess1 | ||
| @@ -479,11 +478,13 @@ func TestPickerCrossSessionSwitchCopiesTerminalEnvForFutureTabs(t *testing.T) { | |||||
| 479 | require.Same(t, ac2, old) | 478 | require.Same(t, ac2, old) | ||
| 480 | require.False(t, sess2.terminal.TrueColor) | 479 | require.False(t, sess2.terminal.TrueColor) | ||
| 481 | require.NoError(t, d.createTab(sess2, domain.Size{Cols: 80, Rows: 24})) | 480 | require.NoError(t, d.createTab(sess2, domain.Size{Cols: 80, Rows: 24})) | ||
| 481 | + | sess2.mu.Lock() | |||
| 482 | + | futureTabID := sess2.tabs[1].stableID | |||
| 483 | + | futurePaneID := sess2.tabs[1].focusedPane().stableID | |||
| 484 | + | sess2.mu.Unlock() | |||
| 482 | require.Equal(t, "/usr/bin/fish", openedCommand) | 485 | require.Equal(t, "/usr/bin/fish", openedCommand) | ||
| 483 | - | require.Contains(t, openedEnv, "SECRET=from-active-client") | 486 | + | require.Equal(t, []string{"SECRET=from-active-session", "SHELL=/usr/bin/fish", "PAIR=a=b", "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=beta,tab=" + futureTabID + ",pane=" + futurePaneID}, openedEnv) |
| 484 | - | require.Contains(t, openedEnv, "TERM=xterm-256color") | 487 | + | require.NotContains(t, openedEnv, "STALE=destination-only") |
| 485 | - | require.NotContains(t, openedEnv, "COLORTERM=truecolor") | |||
| 486 | - | require.NotContains(t, openedEnv, "TERM=xterm-direct") | |||
| 487 | } | 488 | } | ||
| 488 | 489 | ||||
| 489 | func TestPickerPreviewGenerationRejectsStaleSubscriptionReplacement(t *testing.T) { | 490 | func TestPickerPreviewGenerationRejectsStaleSubscriptionReplacement(t *testing.T) { | ||
M internal/usecase/daemon/resume.go +2 -1
| @@ -179,12 +179,13 @@ func (d *Daemon) resumeParkedLocked(h ports.Hello, tr ports.Transport, sz domain | |||
| 179 | 179 | // mandatory first paint cannot be blocked by ACKs that died with the link. | |
| 180 | 180 | ac.output.rebase() | |
| 181 | 181 | ac.output.maxOutstanding = uint64(normalizeOutputWindow(h.MaxOutputInFlight)) | |
| 182 | - | ac.setEnvironment(h.Env) | |
| 183 | 182 | ac.replaceTransport(tr) | |
| 184 | 183 | ac.size = sz | |
| 185 | 184 | ac.resumeToken = d.nextResumeTokenLocked() | |
| 186 | 185 | ac.parked = false | |
| 187 | 186 | ac.setSession(sess) | |
| 187 | + | // The resumed session's snapshot is the sole source for future PTY children. | |
| 188 | + | // Existing PTYs retain the environment they were started with. | |
| 188 | 189 | sess.mu.Lock() | |
| 189 | 190 | sess.env = copyEnvironment(h.Env) | |
| 190 | 191 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | |
| @@ -179,12 +179,13 @@ func (d *Daemon) resumeParkedLocked(h ports.Hello, tr ports.Transport, sz domain | |||||
| 179 | // mandatory first paint cannot be blocked by ACKs that died with the link. | 179 | // mandatory first paint cannot be blocked by ACKs that died with the link. | ||
| 180 | ac.output.rebase() | 180 | ac.output.rebase() | ||
| 181 | ac.output.maxOutstanding = uint64(normalizeOutputWindow(h.MaxOutputInFlight)) | 181 | ac.output.maxOutstanding = uint64(normalizeOutputWindow(h.MaxOutputInFlight)) | ||
| 182 | - | ac.setEnvironment(h.Env) | |||
| 183 | ac.replaceTransport(tr) | 182 | ac.replaceTransport(tr) | ||
| 184 | ac.size = sz | 183 | ac.size = sz | ||
| 185 | ac.resumeToken = d.nextResumeTokenLocked() | 184 | ac.resumeToken = d.nextResumeTokenLocked() | ||
| 186 | ac.parked = false | 185 | ac.parked = false | ||
| 187 | ac.setSession(sess) | 186 | ac.setSession(sess) | ||
| 187 | + | // The resumed session's snapshot is the sole source for future PTY children. | |||
| 188 | + | // Existing PTYs retain the environment they were started with. | |||
| 188 | sess.mu.Lock() | 189 | sess.mu.Lock() | ||
| 189 | sess.env = copyEnvironment(h.Env) | 190 | sess.env = copyEnvironment(h.Env) | ||
| 190 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | 191 | sess.terminal = terminalEnv{TrueColor: h.TrueColor} | ||
M internal/usecase/daemon/resume_test.go +35 -10
| @@ -1,16 +1,19 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "errors" | |
| 5 | 6 | "strings" | |
| 6 | 7 | "sync" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| 11 | + | "github.com/stretchr/testify/mock" | |
| 10 | 12 | "github.com/stretchr/testify/require" | |
| 11 | 13 | ||
| 12 | 14 | "github.com/bnema/vev/internal/domain" | |
| 13 | 15 | "github.com/bnema/vev/internal/ports" | |
| 16 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 14 | 17 | "github.com/bnema/vev/internal/usecase/layout" | |
| 15 | 18 | "github.com/bnema/vev/pkg/vt" | |
| 16 | 19 | ) | |
| @@ -678,14 +681,31 @@ func TestSequencedInputDoesNotPrematurelyEchoAck(t *testing.T) { | |||
| 678 | 681 | require.Zero(t, ac.echoAck.Load()) | |
| 679 | 682 | } | |
| 680 | 683 | ||
| 681 | - | func TestResumeParkedUpdatesTerminalEnv(t *testing.T) { | |
| 682 | - | pty, release := newBlockingPTY(t) | |
| 683 | - | defer release() | |
| 684 | - | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 684 | + | func TestResumeParkedReplacesFuturePTYEnvironment(t *testing.T) { | |
| 685 | + | initialPTY, releaseInitial := newBlockingPTY(t) | |
| 686 | + | futurePTY, releaseFuture := newBlockingPTY(t) | |
| 687 | + | defer releaseInitial() | |
| 688 | + | defer releaseFuture() | |
| 689 | + | var commands []string | |
| 690 | + | var envs [][]string | |
| 691 | + | factory := portsmocks.NewMockPTYFactory(t) | |
| 692 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 693 | + | func(_ context.Context, command string, _ []string, env []string, _ string, size domain.Size) (ports.PTY, error) { | |
| 694 | + | if size != (domain.Size{Cols: 80, Rows: 22}) { | |
| 695 | + | return newQuietPTY(), nil | |
| 696 | + | } | |
| 697 | + | commands = append(commands, command) | |
| 698 | + | envs = append(envs, append([]string(nil), env...)) | |
| 699 | + | if len(commands) == 1 { | |
| 700 | + | return initialPTY, nil | |
| 701 | + | } | |
| 702 | + | return futurePTY, nil | |
| 703 | + | }, | |
| 704 | + | ).Maybe() | |
| 705 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 685 | 706 | tr, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |
| 686 | 707 | hello := helloResumeCapable(ports.IntentNew, "work", 0) | |
| 687 | - | hello.Env = []string{"SECRET=before", "SHELL=/bin/sh"} | |
| 688 | - | hello.TrueColor = false | |
| 708 | + | hello.Env = []string{"SECRET=before", "SHELL=/bin/sh", "TERM=old", "VEV=old"} | |
| 689 | 709 | sess, ac, err := d.route(hello, tr) | |
| 690 | 710 | require.NoError(t, err) | |
| 691 | 711 | token := ac.resumeToken | |
| @@ -693,16 +713,21 @@ func TestResumeParkedUpdatesTerminalEnv(t *testing.T) { | |||
| 693 | 713 | require.True(t, d.parkAttachment(sess, ac)) | |
| 694 | 714 | ||
| 695 | 715 | resumeHello := helloResumeCapable(ports.IntentResume, "work", token) | |
| 696 | - | resumeHello.Env = []string{"SECRET=after", "SHELL=/usr/bin/fish"} | |
| 716 | + | resumeHello.Env = []string{"SECRET=after", "PAIR=a=b", "SHELL=/usr/bin/fish", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | |
| 697 | 717 | resumeHello.TrueColor = true | |
| 698 | - | _, resumed, ok, err := d.resumeParked(resumeHello, &closeTrackingTransport{}, domain.Size{Cols: 80, Rows: 24}) | |
| 718 | + | _, _, ok, err := d.resumeParked(resumeHello, &closeTrackingTransport{}, domain.Size{Cols: 80, Rows: 24}) | |
| 699 | 719 | require.NoError(t, err) | |
| 700 | 720 | require.True(t, ok) | |
| 721 | + | require.NoError(t, d.createTab(sess, domain.Size{Cols: 80, Rows: 24})) | |
| 722 | + | ||
| 701 | 723 | sess.mu.Lock() | |
| 702 | - | defer sess.mu.Unlock() | |
| 724 | + | future := sess.tabs[1].focusedPane() | |
| 725 | + | futureTabID, futurePaneID := sess.tabs[1].stableID, future.stableID | |
| 703 | 726 | require.True(t, sess.terminal.TrueColor) | |
| 704 | 727 | require.Equal(t, resumeHello.Env, sess.env) | |
| 705 | - | require.Equal(t, resumeHello.Env, resumed.environment()) | |
| 728 | + | sess.mu.Unlock() | |
| 729 | + | require.Equal(t, []string{"/bin/sh", "/usr/bin/fish"}, commands) | |
| 730 | + | require.Equal(t, []string{"SECRET=after", "PAIR=a=b", "SHELL=/usr/bin/fish", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + futureTabID + ",pane=" + futurePaneID}, envs[1]) | |
| 706 | 731 | } | |
| 707 | 732 | ||
| 708 | 733 | func TestResumeRenegotiatesOutputWindowOnReusedStream(t *testing.T) { | |
| @@ -1,16 +1,19 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "context" | |||
| 4 | "errors" | 5 | "errors" | ||
| 5 | "strings" | 6 | "strings" | ||
| 6 | "sync" | 7 | "sync" | ||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| 11 | + | "github.com/stretchr/testify/mock" | |||
| 10 | "github.com/stretchr/testify/require" | 12 | "github.com/stretchr/testify/require" | ||
| 11 | 13 | ||||
| 12 | "github.com/bnema/vev/internal/domain" | 14 | "github.com/bnema/vev/internal/domain" | ||
| 13 | "github.com/bnema/vev/internal/ports" | 15 | "github.com/bnema/vev/internal/ports" | ||
| 16 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 14 | "github.com/bnema/vev/internal/usecase/layout" | 17 | "github.com/bnema/vev/internal/usecase/layout" | ||
| 15 | "github.com/bnema/vev/pkg/vt" | 18 | "github.com/bnema/vev/pkg/vt" | ||
| 16 | ) | 19 | ) | ||
| @@ -678,14 +681,31 @@ func TestSequencedInputDoesNotPrematurelyEchoAck(t *testing.T) { | |||||
| 678 | require.Zero(t, ac.echoAck.Load()) | 681 | require.Zero(t, ac.echoAck.Load()) | ||
| 679 | } | 682 | } | ||
| 680 | 683 | ||||
| 681 | - | func TestResumeParkedUpdatesTerminalEnv(t *testing.T) { | 684 | + | func TestResumeParkedReplacesFuturePTYEnvironment(t *testing.T) { |
| 682 | - | pty, release := newBlockingPTY(t) | 685 | + | initialPTY, releaseInitial := newBlockingPTY(t) |
| 683 | - | defer release() | 686 | + | futurePTY, releaseFuture := newBlockingPTY(t) |
| 684 | - | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | 687 | + | defer releaseInitial() |
| 688 | + | defer releaseFuture() | |||
| 689 | + | var commands []string | |||
| 690 | + | var envs [][]string | |||
| 691 | + | factory := portsmocks.NewMockPTYFactory(t) | |||
| 692 | + | factory.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 693 | + | func(_ context.Context, command string, _ []string, env []string, _ string, size domain.Size) (ports.PTY, error) { | |||
| 694 | + | if size != (domain.Size{Cols: 80, Rows: 22}) { | |||
| 695 | + | return newQuietPTY(), nil | |||
| 696 | + | } | |||
| 697 | + | commands = append(commands, command) | |||
| 698 | + | envs = append(envs, append([]string(nil), env...)) | |||
| 699 | + | if len(commands) == 1 { | |||
| 700 | + | return initialPTY, nil | |||
| 701 | + | } | |||
| 702 | + | return futurePTY, nil | |||
| 703 | + | }, | |||
| 704 | + | ).Maybe() | |||
| 705 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 685 | tr, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | 706 | tr, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | ||
| 686 | hello := helloResumeCapable(ports.IntentNew, "work", 0) | 707 | hello := helloResumeCapable(ports.IntentNew, "work", 0) | ||
| 687 | - | hello.Env = []string{"SECRET=before", "SHELL=/bin/sh"} | 708 | + | hello.Env = []string{"SECRET=before", "SHELL=/bin/sh", "TERM=old", "VEV=old"} |
| 688 | - | hello.TrueColor = false | |||
| 689 | sess, ac, err := d.route(hello, tr) | 709 | sess, ac, err := d.route(hello, tr) | ||
| 690 | require.NoError(t, err) | 710 | require.NoError(t, err) | ||
| 691 | token := ac.resumeToken | 711 | token := ac.resumeToken | ||
| @@ -693,16 +713,21 @@ func TestResumeParkedUpdatesTerminalEnv(t *testing.T) { | |||||
| 693 | require.True(t, d.parkAttachment(sess, ac)) | 713 | require.True(t, d.parkAttachment(sess, ac)) | ||
| 694 | 714 | ||||
| 695 | resumeHello := helloResumeCapable(ports.IntentResume, "work", token) | 715 | resumeHello := helloResumeCapable(ports.IntentResume, "work", token) | ||
| 696 | - | resumeHello.Env = []string{"SECRET=after", "SHELL=/usr/bin/fish"} | 716 | + | resumeHello.Env = []string{"SECRET=after", "PAIR=a=b", "SHELL=/usr/bin/fish", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} |
| 697 | resumeHello.TrueColor = true | 717 | resumeHello.TrueColor = true | ||
| 698 | - | _, resumed, ok, err := d.resumeParked(resumeHello, &closeTrackingTransport{}, domain.Size{Cols: 80, Rows: 24}) | 718 | + | _, _, ok, err := d.resumeParked(resumeHello, &closeTrackingTransport{}, domain.Size{Cols: 80, Rows: 24}) |
| 699 | require.NoError(t, err) | 719 | require.NoError(t, err) | ||
| 700 | require.True(t, ok) | 720 | require.True(t, ok) | ||
| 721 | + | require.NoError(t, d.createTab(sess, domain.Size{Cols: 80, Rows: 24})) | |||
| 722 | + | ||||
| 701 | sess.mu.Lock() | 723 | sess.mu.Lock() | ||
| 702 | - | defer sess.mu.Unlock() | 724 | + | future := sess.tabs[1].focusedPane() |
| 725 | + | futureTabID, futurePaneID := sess.tabs[1].stableID, future.stableID | |||
| 703 | require.True(t, sess.terminal.TrueColor) | 726 | require.True(t, sess.terminal.TrueColor) | ||
| 704 | require.Equal(t, resumeHello.Env, sess.env) | 727 | require.Equal(t, resumeHello.Env, sess.env) | ||
| 705 | - | require.Equal(t, resumeHello.Env, resumed.environment()) | 728 | + | sess.mu.Unlock() |
| 729 | + | require.Equal(t, []string{"/bin/sh", "/usr/bin/fish"}, commands) | |||
| 730 | + | require.Equal(t, []string{"SECRET=after", "PAIR=a=b", "SHELL=/usr/bin/fish", "TERM=xterm-direct", "COLORTERM=truecolor", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + futureTabID + ",pane=" + futurePaneID}, envs[1]) | |||
| 706 | } | 731 | } | ||
| 707 | 732 | ||||
| 708 | func TestResumeRenegotiatesOutputWindowOnReusedStream(t *testing.T) { | 733 | func TestResumeRenegotiatesOutputWindowOnReusedStream(t *testing.T) { | ||
M internal/usecase/daemon/session.go +3 -7
| @@ -129,11 +129,7 @@ func (d *Daemon) touchMRU(sess *session) { | |||
| 129 | 129 | } | |
| 130 | 130 | } | |
| 131 | 131 | ||
| 132 | - | func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, restoredTabNames ...[]string) (*session, error) { | |
| 133 | - | return d.createSessionWithEnvLocked(name, ephemeral, cwd, sz, term, d.baseEnv, restoredTabNames...) | |
| 134 | - | } | |
| 135 | - | ||
| 136 | - | func (d *Daemon) createSessionWithEnvLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, env []string, restoredTabNames ...[]string) (*session, error) { | |
| 132 | + | func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, env []string, restoredTabNames ...[]string) (*session, error) { | |
| 137 | 133 | env = copyEnvironment(env) | |
| 138 | 134 | tbSize := tabSize(sz) | |
| 139 | 135 | var names []string | |
| @@ -227,7 +223,6 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||
| 227 | 223 | return err | |
| 228 | 224 | } | |
| 229 | 225 | sz := ac.size | |
| 230 | - | env := ac.environment() | |
| 231 | 226 | d.mu.Lock() | |
| 232 | 227 | if d.closing { | |
| 233 | 228 | d.mu.Unlock() | |
| @@ -240,6 +235,7 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||
| 240 | 235 | from.mu.Lock() | |
| 241 | 236 | cwd := from.cwd | |
| 242 | 237 | term := from.terminal | |
| 238 | + | env := copyEnvironment(from.env) | |
| 243 | 239 | if from.client != ac { | |
| 244 | 240 | from.mu.Unlock() | |
| 245 | 241 | d.mu.Unlock() | |
| @@ -247,7 +243,7 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||
| 247 | 243 | } | |
| 248 | 244 | from.mu.Unlock() | |
| 249 | 245 | ||
| 250 | - | newSess, err := d.createSessionWithEnvLocked(name, false, cwd, sz, term, env) | |
| 246 | + | newSess, err := d.createSessionLocked(name, false, cwd, sz, term, env) | |
| 251 | 247 | if err != nil { | |
| 252 | 248 | d.mu.Unlock() | |
| 253 | 249 | return err | |
| @@ -129,11 +129,7 @@ func (d *Daemon) touchMRU(sess *session) { | |||||
| 129 | } | 129 | } | ||
| 130 | } | 130 | } | ||
| 131 | 131 | ||||
| 132 | - | func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, restoredTabNames ...[]string) (*session, error) { | 132 | + | func (d *Daemon) createSessionLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, env []string, restoredTabNames ...[]string) (*session, error) { |
| 133 | - | return d.createSessionWithEnvLocked(name, ephemeral, cwd, sz, term, d.baseEnv, restoredTabNames...) | |||
| 134 | - | } | |||
| 135 | - | ||||
| 136 | - | func (d *Daemon) createSessionWithEnvLocked(name string, ephemeral bool, cwd string, sz domain.Size, term terminalEnv, env []string, restoredTabNames ...[]string) (*session, error) { | |||
| 137 | env = copyEnvironment(env) | 133 | env = copyEnvironment(env) | ||
| 138 | tbSize := tabSize(sz) | 134 | tbSize := tabSize(sz) | ||
| 139 | var names []string | 135 | var names []string | ||
| @@ -227,7 +223,6 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||||
| 227 | return err | 223 | return err | ||
| 228 | } | 224 | } | ||
| 229 | sz := ac.size | 225 | sz := ac.size | ||
| 230 | - | env := ac.environment() | |||
| 231 | d.mu.Lock() | 226 | d.mu.Lock() | ||
| 232 | if d.closing { | 227 | if d.closing { | ||
| 233 | d.mu.Unlock() | 228 | d.mu.Unlock() | ||
| @@ -240,6 +235,7 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||||
| 240 | from.mu.Lock() | 235 | from.mu.Lock() | ||
| 241 | cwd := from.cwd | 236 | cwd := from.cwd | ||
| 242 | term := from.terminal | 237 | term := from.terminal | ||
| 238 | + | env := copyEnvironment(from.env) | |||
| 243 | if from.client != ac { | 239 | if from.client != ac { | ||
| 244 | from.mu.Unlock() | 240 | from.mu.Unlock() | ||
| 245 | d.mu.Unlock() | 241 | d.mu.Unlock() | ||
| @@ -247,7 +243,7 @@ func (d *Daemon) createSessionAndSwitch(from *session, ac *attachedClient, name | |||||
| 247 | } | 243 | } | ||
| 248 | from.mu.Unlock() | 244 | from.mu.Unlock() | ||
| 249 | 245 | ||||
| 250 | - | newSess, err := d.createSessionWithEnvLocked(name, false, cwd, sz, term, env) | 246 | + | newSess, err := d.createSessionLocked(name, false, cwd, sz, term, env) |
| 251 | if err != nil { | 247 | if err != nil { | ||
| 252 | d.mu.Unlock() | 248 | d.mu.Unlock() | ||
| 253 | return err | 249 | return err | ||
M internal/usecase/daemon/session_test.go +11 -11
| @@ -853,7 +853,7 @@ func TestCreateSessionSeedsMRUFromStopped(t *testing.T) { | |||
| 853 | 853 | d.stopped["work"] = stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 1, lastUsedSeq: 42} | |
| 854 | 854 | d.mruSeq.Store(42) | |
| 855 | 855 | ||
| 856 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 856 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 857 | 857 | require.NoError(t, err) | |
| 858 | 858 | require.Equal(t, uint64(42), sess.mruAt.Load()) | |
| 859 | 859 | require.Equal(t, uint64(42), state.record(t, "work").LastUsedSeq) | |
| @@ -869,7 +869,7 @@ func TestCreateRenameKillPersistenceLifecycle(t *testing.T) { | |||
| 869 | 869 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | |
| 870 | 870 | WithStore(store)(d) | |
| 871 | 871 | ||
| 872 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 872 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 873 | 873 | require.NoError(t, err) | |
| 874 | 874 | require.True(t, state.has("work")) | |
| 875 | 875 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | |
| @@ -893,7 +893,7 @@ func TestRenameTabPersistsForNamedSession(t *testing.T) { | |||
| 893 | 893 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 894 | 894 | WithStore(store)(d) | |
| 895 | 895 | ||
| 896 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 896 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 897 | 897 | require.NoError(t, err) | |
| 898 | 898 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | |
| 899 | 899 | ||
| @@ -930,7 +930,7 @@ func TestTabNamePersistenceTracksTabIndexShifts(t *testing.T) { | |||
| 930 | 930 | WithStore(store)(d) | |
| 931 | 931 | sz := domain.Size{Cols: 80, Rows: 24} | |
| 932 | 932 | ||
| 933 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 933 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 934 | 934 | require.NoError(t, err) | |
| 935 | 935 | require.NoError(t, d.createTab(sess, sz)) | |
| 936 | 936 | require.NoError(t, d.createTab(sess, sz)) | |
| @@ -978,7 +978,7 @@ func TestRenameTabDoesNotPersistForEphemeralSession(t *testing.T) { | |||
| 978 | 978 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 979 | 979 | WithStore(store)(d) | |
| 980 | 980 | ||
| 981 | - | sess, err := d.createSessionLocked("0", true, "/tmp/work", sz, terminalEnv{}) | |
| 981 | + | sess, err := d.createSessionLocked("0", true, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 982 | 982 | require.NoError(t, err) | |
| 983 | 983 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | |
| 984 | 984 | require.Equal(t, "shell", sess.tabs[0].name) | |
| @@ -1021,7 +1021,7 @@ func TestEphemeralRenamePromotesAndStoppedCollisionRejected(t *testing.T) { | |||
| 1021 | 1021 | WithStore(store)(d) | |
| 1022 | 1022 | d.stopped["taken"] = stoppedSession{name: "taken", cwd: "/tmp", createdAt: 1} | |
| 1023 | 1023 | ||
| 1024 | - | sess, err := d.createSessionLocked("0", true, "/tmp/e", sz, terminalEnv{}) | |
| 1024 | + | sess, err := d.createSessionLocked("0", true, "/tmp/e", sz, terminalEnv{}, d.baseEnv) | |
| 1025 | 1025 | require.NoError(t, err) | |
| 1026 | 1026 | require.False(t, state.has("0")) | |
| 1027 | 1027 | require.EqualError(t, d.renameSession(sess, "taken"), "name already in use") | |
| @@ -1039,7 +1039,7 @@ func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | |||
| 1039 | 1039 | cwd := "/tmp/work" | |
| 1040 | 1040 | WithCwdReader(func(int) (string, error) { return cwd, nil })(d) | |
| 1041 | 1041 | ||
| 1042 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 1042 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 1043 | 1043 | require.NoError(t, err) | |
| 1044 | 1044 | state.mu.Lock() | |
| 1045 | 1045 | setsAfterCreate := state.sets | |
| @@ -1136,7 +1136,7 @@ func TestNewSessionAssignsStableIDsAndChildEnv(t *testing.T) { | |||
| 1136 | 1136 | d := newTestDaemon(t, f, stubClock{}) | |
| 1137 | 1137 | d.baseEnv = []string{"KEEP=1", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | |
| 1138 | 1138 | ||
| 1139 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{TrueColor: true}) | |
| 1139 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{TrueColor: true}, d.baseEnv) | |
| 1140 | 1140 | require.NoError(t, err) | |
| 1141 | 1141 | defer func() { | |
| 1142 | 1142 | _ = d.killSession(sess, ports.ReasonServerShutdown, false) | |
| @@ -1192,7 +1192,7 @@ func TestRenameSessionUnsafeNameRejected(t *testing.T) { | |||
| 1192 | 1192 | p, release := newBlockingPTY(t) | |
| 1193 | 1193 | defer release() | |
| 1194 | 1194 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 1195 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | |
| 1195 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) | |
| 1196 | 1196 | require.NoError(t, err) | |
| 1197 | 1197 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | |
| 1198 | 1198 | } | |
| @@ -1208,9 +1208,9 @@ func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | |||
| 1208 | 1208 | WithStore(store)(d) | |
| 1209 | 1209 | WithCwdReader(func(int) (string, error) { return "/tmp/latest", nil })(d) | |
| 1210 | 1210 | ||
| 1211 | - | natural, err := d.createSessionLocked("natural", false, "/tmp/old", sz, terminalEnv{}) | |
| 1211 | + | natural, err := d.createSessionLocked("natural", false, "/tmp/old", sz, terminalEnv{}, d.baseEnv) | |
| 1212 | 1212 | require.NoError(t, err) | |
| 1213 | - | other, err := d.createSessionLocked("other", false, "/tmp/other", sz, terminalEnv{}) | |
| 1213 | + | other, err := d.createSessionLocked("other", false, "/tmp/other", sz, terminalEnv{}, d.baseEnv) | |
| 1214 | 1214 | require.NoError(t, err) | |
| 1215 | 1215 | _ = d.killSession(natural, ports.ReasonSessionKilled, false) | |
| 1216 | 1216 | require.True(t, state.has("natural")) | |
| @@ -853,7 +853,7 @@ func TestCreateSessionSeedsMRUFromStopped(t *testing.T) { | |||||
| 853 | d.stopped["work"] = stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 1, lastUsedSeq: 42} | 853 | d.stopped["work"] = stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 1, lastUsedSeq: 42} | ||
| 854 | d.mruSeq.Store(42) | 854 | d.mruSeq.Store(42) | ||
| 855 | 855 | ||||
| 856 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 856 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 857 | require.NoError(t, err) | 857 | require.NoError(t, err) | ||
| 858 | require.Equal(t, uint64(42), sess.mruAt.Load()) | 858 | require.Equal(t, uint64(42), sess.mruAt.Load()) | ||
| 859 | require.Equal(t, uint64(42), state.record(t, "work").LastUsedSeq) | 859 | require.Equal(t, uint64(42), state.record(t, "work").LastUsedSeq) | ||
| @@ -869,7 +869,7 @@ func TestCreateRenameKillPersistenceLifecycle(t *testing.T) { | |||||
| 869 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | 869 | d := newTestDaemon(t, newFactorySeq(t, p1, p2), stubClock{}) | ||
| 870 | WithStore(store)(d) | 870 | WithStore(store)(d) | ||
| 871 | 871 | ||||
| 872 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 872 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 873 | require.NoError(t, err) | 873 | require.NoError(t, err) | ||
| 874 | require.True(t, state.has("work")) | 874 | require.True(t, state.has("work")) | ||
| 875 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | 875 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | ||
| @@ -893,7 +893,7 @@ func TestRenameTabPersistsForNamedSession(t *testing.T) { | |||||
| 893 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | 893 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | ||
| 894 | WithStore(store)(d) | 894 | WithStore(store)(d) | ||
| 895 | 895 | ||||
| 896 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 896 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 897 | require.NoError(t, err) | 897 | require.NoError(t, err) | ||
| 898 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | 898 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | ||
| 899 | 899 | ||||
| @@ -930,7 +930,7 @@ func TestTabNamePersistenceTracksTabIndexShifts(t *testing.T) { | |||||
| 930 | WithStore(store)(d) | 930 | WithStore(store)(d) | ||
| 931 | sz := domain.Size{Cols: 80, Rows: 24} | 931 | sz := domain.Size{Cols: 80, Rows: 24} | ||
| 932 | 932 | ||||
| 933 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 933 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 934 | require.NoError(t, err) | 934 | require.NoError(t, err) | ||
| 935 | require.NoError(t, d.createTab(sess, sz)) | 935 | require.NoError(t, d.createTab(sess, sz)) | ||
| 936 | require.NoError(t, d.createTab(sess, sz)) | 936 | require.NoError(t, d.createTab(sess, sz)) | ||
| @@ -978,7 +978,7 @@ func TestRenameTabDoesNotPersistForEphemeralSession(t *testing.T) { | |||||
| 978 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | 978 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | ||
| 979 | WithStore(store)(d) | 979 | WithStore(store)(d) | ||
| 980 | 980 | ||||
| 981 | - | sess, err := d.createSessionLocked("0", true, "/tmp/work", sz, terminalEnv{}) | 981 | + | sess, err := d.createSessionLocked("0", true, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 982 | require.NoError(t, err) | 982 | require.NoError(t, err) | ||
| 983 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | 983 | require.NoError(t, d.renameTab(sess, sess.tabs[0], "shell")) | ||
| 984 | require.Equal(t, "shell", sess.tabs[0].name) | 984 | require.Equal(t, "shell", sess.tabs[0].name) | ||
| @@ -1021,7 +1021,7 @@ func TestEphemeralRenamePromotesAndStoppedCollisionRejected(t *testing.T) { | |||||
| 1021 | WithStore(store)(d) | 1021 | WithStore(store)(d) | ||
| 1022 | d.stopped["taken"] = stoppedSession{name: "taken", cwd: "/tmp", createdAt: 1} | 1022 | d.stopped["taken"] = stoppedSession{name: "taken", cwd: "/tmp", createdAt: 1} | ||
| 1023 | 1023 | ||||
| 1024 | - | sess, err := d.createSessionLocked("0", true, "/tmp/e", sz, terminalEnv{}) | 1024 | + | sess, err := d.createSessionLocked("0", true, "/tmp/e", sz, terminalEnv{}, d.baseEnv) |
| 1025 | require.NoError(t, err) | 1025 | require.NoError(t, err) | ||
| 1026 | require.False(t, state.has("0")) | 1026 | require.False(t, state.has("0")) | ||
| 1027 | require.EqualError(t, d.renameSession(sess, "taken"), "name already in use") | 1027 | require.EqualError(t, d.renameSession(sess, "taken"), "name already in use") | ||
| @@ -1039,7 +1039,7 @@ func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | |||||
| 1039 | cwd := "/tmp/work" | 1039 | cwd := "/tmp/work" | ||
| 1040 | WithCwdReader(func(int) (string, error) { return cwd, nil })(d) | 1040 | WithCwdReader(func(int) (string, error) { return cwd, nil })(d) | ||
| 1041 | 1041 | ||||
| 1042 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 1042 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 1043 | require.NoError(t, err) | 1043 | require.NoError(t, err) | ||
| 1044 | state.mu.Lock() | 1044 | state.mu.Lock() | ||
| 1045 | setsAfterCreate := state.sets | 1045 | setsAfterCreate := state.sets | ||
| @@ -1136,7 +1136,7 @@ func TestNewSessionAssignsStableIDsAndChildEnv(t *testing.T) { | |||||
| 1136 | d := newTestDaemon(t, f, stubClock{}) | 1136 | d := newTestDaemon(t, f, stubClock{}) | ||
| 1137 | d.baseEnv = []string{"KEEP=1", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | 1137 | d.baseEnv = []string{"KEEP=1", "TERM=old", "COLORTERM=old", "TERM_PROGRAM=old", "VEV=old"} | ||
| 1138 | 1138 | ||||
| 1139 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{TrueColor: true}) | 1139 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{TrueColor: true}, d.baseEnv) |
| 1140 | require.NoError(t, err) | 1140 | require.NoError(t, err) | ||
| 1141 | defer func() { | 1141 | defer func() { | ||
| 1142 | _ = d.killSession(sess, ports.ReasonServerShutdown, false) | 1142 | _ = d.killSession(sess, ports.ReasonServerShutdown, false) | ||
| @@ -1192,7 +1192,7 @@ func TestRenameSessionUnsafeNameRejected(t *testing.T) { | |||||
| 1192 | p, release := newBlockingPTY(t) | 1192 | p, release := newBlockingPTY(t) | ||
| 1193 | defer release() | 1193 | defer release() | ||
| 1194 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | 1194 | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | ||
| 1195 | - | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}) | 1195 | + | sess, err := d.createSessionLocked("work", false, "/tmp/work", sz, terminalEnv{}, d.baseEnv) |
| 1196 | require.NoError(t, err) | 1196 | require.NoError(t, err) | ||
| 1197 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | 1197 | require.ErrorIs(t, d.renameSession(sess, "my work"), domain.ErrInvalidSessionName) | ||
| 1198 | } | 1198 | } | ||
| @@ -1208,9 +1208,9 @@ func TestNaturalExitStoppedButExplicitKillPurges(t *testing.T) { | |||||
| 1208 | WithStore(store)(d) | 1208 | WithStore(store)(d) | ||
| 1209 | WithCwdReader(func(int) (string, error) { return "/tmp/latest", nil })(d) | 1209 | WithCwdReader(func(int) (string, error) { return "/tmp/latest", nil })(d) | ||
| 1210 | 1210 | ||||
| 1211 | - | natural, err := d.createSessionLocked("natural", false, "/tmp/old", sz, terminalEnv{}) | 1211 | + | natural, err := d.createSessionLocked("natural", false, "/tmp/old", sz, terminalEnv{}, d.baseEnv) |
| 1212 | require.NoError(t, err) | 1212 | require.NoError(t, err) | ||
| 1213 | - | other, err := d.createSessionLocked("other", false, "/tmp/other", sz, terminalEnv{}) | 1213 | + | other, err := d.createSessionLocked("other", false, "/tmp/other", sz, terminalEnv{}, d.baseEnv) |
| 1214 | require.NoError(t, err) | 1214 | require.NoError(t, err) | ||
| 1215 | _ = d.killSession(natural, ports.ReasonSessionKilled, false) | 1215 | _ = d.killSession(natural, ports.ReasonSessionKilled, false) | ||
| 1216 | require.True(t, state.has("natural")) | 1216 | require.True(t, state.has("natural")) | ||
M internal/usecase/daemon/status_test.go +2 -2
| @@ -1047,9 +1047,9 @@ func TestBarStateForMRURestoredStoppedSessionsUsePersistedOrder(t *testing.T) { | |||
| 1047 | 1047 | cur := &session{id: "cur", name: "current", tabs: []*tab{{}}} | |
| 1048 | 1048 | d.sessions[cur.id] = cur | |
| 1049 | 1049 | ||
| 1050 | - | _, err := d.createSessionLocked("zeta", false, "/tmp/zeta", sz, terminalEnv{}) | |
| 1050 | + | _, err := d.createSessionLocked("zeta", false, "/tmp/zeta", sz, terminalEnv{}, d.baseEnv) | |
| 1051 | 1051 | require.NoError(t, err) | |
| 1052 | - | _, err = d.createSessionLocked("alpha", false, "/tmp/alpha", sz, terminalEnv{}) | |
| 1052 | + | _, err = d.createSessionLocked("alpha", false, "/tmp/alpha", sz, terminalEnv{}, d.baseEnv) | |
| 1053 | 1053 | require.NoError(t, err) | |
| 1054 | 1054 | ||
| 1055 | 1055 | state := d.barStateFor(cur, "") | |
| @@ -1047,9 +1047,9 @@ func TestBarStateForMRURestoredStoppedSessionsUsePersistedOrder(t *testing.T) { | |||||
| 1047 | cur := &session{id: "cur", name: "current", tabs: []*tab{{}}} | 1047 | cur := &session{id: "cur", name: "current", tabs: []*tab{{}}} | ||
| 1048 | d.sessions[cur.id] = cur | 1048 | d.sessions[cur.id] = cur | ||
| 1049 | 1049 | ||||
| 1050 | - | _, err := d.createSessionLocked("zeta", false, "/tmp/zeta", sz, terminalEnv{}) | 1050 | + | _, err := d.createSessionLocked("zeta", false, "/tmp/zeta", sz, terminalEnv{}, d.baseEnv) |
| 1051 | require.NoError(t, err) | 1051 | require.NoError(t, err) | ||
| 1052 | - | _, err = d.createSessionLocked("alpha", false, "/tmp/alpha", sz, terminalEnv{}) | 1052 | + | _, err = d.createSessionLocked("alpha", false, "/tmp/alpha", sz, terminalEnv{}, d.baseEnv) |
| 1053 | require.NoError(t, err) | 1053 | require.NoError(t, err) | ||
| 1054 | 1054 | ||||
| 1055 | state := d.barStateFor(cur, "") | 1055 | state := d.barStateFor(cur, "") | ||