fix(daemon): preserve environment during snapshot restore
- commit
- 0812bb21eadd775e6ad83f1e346d3203ba3cc449
- author
- brice · · 2026-07-15 05:48 UTC
- parent
- 394dce9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +63 -13
M internal/usecase/daemon/snapshot.go +4 -6
| @@ -114,13 +114,11 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err | |||
| 114 | 114 | ||
| 115 | 115 | sctx, cancel := context.WithCancel(d.serveCtx) | |
| 116 | 116 | opened := make([]*tab, 0, len(snap.Tabs)) | |
| 117 | - | // Snapshot restore runs before any client Hello is available, so restored | |
| 118 | - | // panes start with the conservative terminal environment. The next attach or | |
| 119 | - | // resume updates sess.terminal for future panes from the client's capability. | |
| 117 | + | // Snapshot restore runs before any client Hello is available. Start panes | |
| 118 | + | // from the daemon environment captured at startup; the next attach replaces | |
| 119 | + | // this snapshot for future panes from the client's capability. | |
| 120 | 120 | restoreTerm := terminalEnv{} | |
| 121 | - | // No attach exists during restore, so no client environment is available. | |
| 122 | - | // Future attaches replace this empty snapshot before publishing a client. | |
| 123 | - | restoreEnv := []string(nil) | |
| 121 | + | restoreEnv := copyEnvironment(d.baseEnv) | |
| 124 | 122 | closeOpened := func() { | |
| 125 | 123 | for _, tb := range opened { | |
| 126 | 124 | tb.closeAllPanes() | |
| @@ -114,13 +114,11 @@ func (d *Daemon) restoreSession(ctx context.Context, snap snapcodec.Session) err | |||||
| 114 | 114 | ||||
| 115 | sctx, cancel := context.WithCancel(d.serveCtx) | 115 | sctx, cancel := context.WithCancel(d.serveCtx) | ||
| 116 | opened := make([]*tab, 0, len(snap.Tabs)) | 116 | opened := make([]*tab, 0, len(snap.Tabs)) | ||
| 117 | - | // Snapshot restore runs before any client Hello is available, so restored | 117 | + | // Snapshot restore runs before any client Hello is available. Start panes |
| 118 | - | // panes start with the conservative terminal environment. The next attach or | 118 | + | // from the daemon environment captured at startup; the next attach replaces |
| 119 | - | // resume updates sess.terminal for future panes from the client's capability. | 119 | + | // this snapshot for future panes from the client's capability. |
| 120 | restoreTerm := terminalEnv{} | 120 | restoreTerm := terminalEnv{} | ||
| 121 | - | // No attach exists during restore, so no client environment is available. | 121 | + | restoreEnv := copyEnvironment(d.baseEnv) |
| 122 | - | // Future attaches replace this empty snapshot before publishing a client. | |||
| 123 | - | restoreEnv := []string(nil) | |||
| 124 | closeOpened := func() { | 122 | closeOpened := func() { | ||
| 125 | for _, tb := range opened { | 123 | for _, tb := range opened { | ||
| 126 | tb.closeAllPanes() | 124 | tb.closeAllPanes() | ||
M internal/usecase/daemon/snapshot_restore_test.go +59 -7
| @@ -86,6 +86,57 @@ func TestRestoreSessionClosesOpenedPTYWhenTerminalRestoreFails(t *testing.T) { | |||
| 86 | 86 | require.Equal(t, 1, factory.opens[0].pty.closeCount()) | |
| 87 | 87 | } | |
| 88 | 88 | ||
| 89 | + | func TestRestoreSessionUsesDaemonFallbackEnvironmentAndShellBeforeAttach(t *testing.T) { | |
| 90 | + | factory := &restorePTYFactory{} | |
| 91 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 92 | + | d.baseEnv = []string{"ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "TERM=stale", "RAW"} | |
| 93 | + | ||
| 94 | + | require.NoError(t, d.restoreSession(context.Background(), snapcodec.Session{Name: "work", Tabs: []snapcodec.Tab{{ | |
| 95 | + | StableID: "tab", Cols: 80, Rows: 24, Tree: layout.NewTree("pane-1"), Panes: []snapcodec.Pane{terminalPane(t, "pane-1", "pane", "/tmp", nil, nil)}, | |
| 96 | + | }}})) | |
| 97 | + | require.Len(t, factory.opens, 1) | |
| 98 | + | require.Equal(t, "/usr/local/bin/daemon-shell", factory.opens[0].command) | |
| 99 | + | require.Equal(t, []string{ | |
| 100 | + | "ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "RAW", | |
| 101 | + | "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=work,tab=tab,pane=pane", | |
| 102 | + | }, factory.opens[0].env) | |
| 103 | + | ||
| 104 | + | d.baseEnv[0] = "ORDINARY=changed" | |
| 105 | + | d.mu.Lock() | |
| 106 | + | restored := d.findByNameLocked("work") | |
| 107 | + | d.mu.Unlock() | |
| 108 | + | require.NotNil(t, restored) | |
| 109 | + | restored.mu.Lock() | |
| 110 | + | require.Equal(t, []string{"ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "TERM=stale", "RAW"}, restored.env) | |
| 111 | + | restored.mu.Unlock() | |
| 112 | + | ||
| 113 | + | tr, _ := newCapturingTransport(t) | |
| 114 | + | _, ac, err := d.route(ports.Hello{ | |
| 115 | + | Version: ports.ProtocolVersion, | |
| 116 | + | Intent: ports.IntentAttach, | |
| 117 | + | Name: "work", | |
| 118 | + | Size: domain.Size{Cols: 80, Rows: 24}, | |
| 119 | + | Env: []string{"ORDINARY=attached", "SHELL=/bin/attached-shell"}, | |
| 120 | + | }, tr) | |
| 121 | + | require.NoError(t, err) | |
| 122 | + | require.NoError(t, d.createTab(restored, ac.size)) | |
| 123 | + | require.Len(t, factory.opens, 2) | |
| 124 | + | require.Equal(t, "/bin/attached-shell", factory.opens[1].command) | |
| 125 | + | ||
| 126 | + | restored.mu.Lock() | |
| 127 | + | attachedTab := restored.tabs[1] | |
| 128 | + | restored.mu.Unlock() | |
| 129 | + | attachedTab.mu.Lock() | |
| 130 | + | attachedPane := attachedTab.panes["pane-1"] | |
| 131 | + | attachedTab.mu.Unlock() | |
| 132 | + | require.Equal(t, []string{ | |
| 133 | + | "ORDINARY=attached", "SHELL=/bin/attached-shell", | |
| 134 | + | "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + attachedTab.stableID + ",pane=" + attachedPane.stableID, | |
| 135 | + | }, factory.opens[1].env) | |
| 136 | + | require.NoError(t, d.killSession(restored, ports.ReasonSessionKilled, false)) | |
| 137 | + | d.sessWg.Wait() | |
| 138 | + | } | |
| 139 | + | ||
| 89 | 140 | func TestRestoreSnapshotsRestoresLayoutCwdAndRows(t *testing.T) { | |
| 90 | 141 | store := &restoreSnapshotStore{} | |
| 91 | 142 | store.blobs = []ports.SnapshotBlob{{Name: "work", Data: mustSnapshotBytes(t, snapcodec.Session{ | |
| @@ -541,19 +592,20 @@ type restorePTYFactory struct { | |||
| 541 | 592 | } | |
| 542 | 593 | ||
| 543 | 594 | type restorePTYOpen struct { | |
| 544 | - | ctx context.Context | |
| 545 | - | dir string | |
| 546 | - | size domain.Size | |
| 547 | - | env []string | |
| 548 | - | pty *restorePTY | |
| 595 | + | ctx context.Context | |
| 596 | + | command string | |
| 597 | + | dir string | |
| 598 | + | size domain.Size | |
| 599 | + | env []string | |
| 600 | + | pty *restorePTY | |
| 549 | 601 | } | |
| 550 | 602 | ||
| 551 | - | func (f *restorePTYFactory) Open(ctx context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | |
| 603 | + | func (f *restorePTYFactory) Open(ctx context.Context, command string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | |
| 552 | 604 | f.mu.Lock() | |
| 553 | 605 | defer f.mu.Unlock() | |
| 554 | 606 | pty := newRestorePTY() | |
| 555 | 607 | pty.writeErr = f.writeErr | |
| 556 | - | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | |
| 608 | + | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, command: command, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | |
| 557 | 609 | return pty, nil | |
| 558 | 610 | } | |
| 559 | 611 | ||
| @@ -86,6 +86,57 @@ func TestRestoreSessionClosesOpenedPTYWhenTerminalRestoreFails(t *testing.T) { | |||||
| 86 | require.Equal(t, 1, factory.opens[0].pty.closeCount()) | 86 | require.Equal(t, 1, factory.opens[0].pty.closeCount()) | ||
| 87 | } | 87 | } | ||
| 88 | 88 | ||||
| 89 | + | func TestRestoreSessionUsesDaemonFallbackEnvironmentAndShellBeforeAttach(t *testing.T) { | |||
| 90 | + | factory := &restorePTYFactory{} | |||
| 91 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 92 | + | d.baseEnv = []string{"ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "TERM=stale", "RAW"} | |||
| 93 | + | ||||
| 94 | + | require.NoError(t, d.restoreSession(context.Background(), snapcodec.Session{Name: "work", Tabs: []snapcodec.Tab{{ | |||
| 95 | + | StableID: "tab", Cols: 80, Rows: 24, Tree: layout.NewTree("pane-1"), Panes: []snapcodec.Pane{terminalPane(t, "pane-1", "pane", "/tmp", nil, nil)}, | |||
| 96 | + | }}})) | |||
| 97 | + | require.Len(t, factory.opens, 1) | |||
| 98 | + | require.Equal(t, "/usr/local/bin/daemon-shell", factory.opens[0].command) | |||
| 99 | + | require.Equal(t, []string{ | |||
| 100 | + | "ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "RAW", | |||
| 101 | + | "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=work,tab=tab,pane=pane", | |||
| 102 | + | }, factory.opens[0].env) | |||
| 103 | + | ||||
| 104 | + | d.baseEnv[0] = "ORDINARY=changed" | |||
| 105 | + | d.mu.Lock() | |||
| 106 | + | restored := d.findByNameLocked("work") | |||
| 107 | + | d.mu.Unlock() | |||
| 108 | + | require.NotNil(t, restored) | |||
| 109 | + | restored.mu.Lock() | |||
| 110 | + | require.Equal(t, []string{"ORDINARY=preserved", "SHELL=/usr/local/bin/daemon-shell", "TERM=stale", "RAW"}, restored.env) | |||
| 111 | + | restored.mu.Unlock() | |||
| 112 | + | ||||
| 113 | + | tr, _ := newCapturingTransport(t) | |||
| 114 | + | _, ac, err := d.route(ports.Hello{ | |||
| 115 | + | Version: ports.ProtocolVersion, | |||
| 116 | + | Intent: ports.IntentAttach, | |||
| 117 | + | Name: "work", | |||
| 118 | + | Size: domain.Size{Cols: 80, Rows: 24}, | |||
| 119 | + | Env: []string{"ORDINARY=attached", "SHELL=/bin/attached-shell"}, | |||
| 120 | + | }, tr) | |||
| 121 | + | require.NoError(t, err) | |||
| 122 | + | require.NoError(t, d.createTab(restored, ac.size)) | |||
| 123 | + | require.Len(t, factory.opens, 2) | |||
| 124 | + | require.Equal(t, "/bin/attached-shell", factory.opens[1].command) | |||
| 125 | + | ||||
| 126 | + | restored.mu.Lock() | |||
| 127 | + | attachedTab := restored.tabs[1] | |||
| 128 | + | restored.mu.Unlock() | |||
| 129 | + | attachedTab.mu.Lock() | |||
| 130 | + | attachedPane := attachedTab.panes["pane-1"] | |||
| 131 | + | attachedTab.mu.Unlock() | |||
| 132 | + | require.Equal(t, []string{ | |||
| 133 | + | "ORDINARY=attached", "SHELL=/bin/attached-shell", | |||
| 134 | + | "TERM=xterm-256color", "TERM_PROGRAM=vev", "VEV=session=work,tab=" + attachedTab.stableID + ",pane=" + attachedPane.stableID, | |||
| 135 | + | }, factory.opens[1].env) | |||
| 136 | + | require.NoError(t, d.killSession(restored, ports.ReasonSessionKilled, false)) | |||
| 137 | + | d.sessWg.Wait() | |||
| 138 | + | } | |||
| 139 | + | ||||
| 89 | func TestRestoreSnapshotsRestoresLayoutCwdAndRows(t *testing.T) { | 140 | func TestRestoreSnapshotsRestoresLayoutCwdAndRows(t *testing.T) { | ||
| 90 | store := &restoreSnapshotStore{} | 141 | store := &restoreSnapshotStore{} | ||
| 91 | store.blobs = []ports.SnapshotBlob{{Name: "work", Data: mustSnapshotBytes(t, snapcodec.Session{ | 142 | store.blobs = []ports.SnapshotBlob{{Name: "work", Data: mustSnapshotBytes(t, snapcodec.Session{ | ||
| @@ -541,19 +592,20 @@ type restorePTYFactory struct { | |||||
| 541 | } | 592 | } | ||
| 542 | 593 | ||||
| 543 | type restorePTYOpen struct { | 594 | type restorePTYOpen struct { | ||
| 544 | - | ctx context.Context | 595 | + | ctx context.Context |
| 545 | - | dir string | 596 | + | command string |
| 546 | - | size domain.Size | 597 | + | dir string |
| 547 | - | env []string | 598 | + | size domain.Size |
| 548 | - | pty *restorePTY | 599 | + | env []string |
| 600 | + | pty *restorePTY | |||
| 549 | } | 601 | } | ||
| 550 | 602 | ||||
| 551 | - | func (f *restorePTYFactory) Open(ctx context.Context, _ string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | 603 | + | func (f *restorePTYFactory) Open(ctx context.Context, command string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { |
| 552 | f.mu.Lock() | 604 | f.mu.Lock() | ||
| 553 | defer f.mu.Unlock() | 605 | defer f.mu.Unlock() | ||
| 554 | pty := newRestorePTY() | 606 | pty := newRestorePTY() | ||
| 555 | pty.writeErr = f.writeErr | 607 | pty.writeErr = f.writeErr | ||
| 556 | - | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | 608 | + | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, command: command, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) |
| 557 | return pty, nil | 609 | return pty, nil | ||
| 558 | } | 610 | } | ||
| 559 | 611 | ||||