diff --git a/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.test.ts b/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.test.ts new file mode 100644 index 00000000000..228c3f9b01a --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest' +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import { retainLocalScrollbackInRemoteLayout } from './remote-layout-scrollback-retention' + +const LEAF_A = 'leaf-a' +const LEAF_B = 'leaf-b' + +function layout(overrides: Partial = {}): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: LEAF_A }, + activeLeafId: LEAF_A, + expandedLeafId: null, + ...overrides + } +} + +const SPLIT_ROOT: TerminalLayoutSnapshot['root'] = { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_A }, + second: { type: 'leaf', leafId: LEAF_B } +} + +describe('retainLocalScrollbackInRemoteLayout', () => { + it('keeps a parked tab scrollback the host copy never had', () => { + const local = layout({ buffersByLeafId: { [LEAF_A]: 'parked-scrollback' } }) + + const merged = retainLocalScrollbackInRemoteLayout(local, layout()) + + expect(merged.buffersByLeafId).toEqual({ [LEAF_A]: 'parked-scrollback' }) + }) + + it('takes the host structure verbatim when the host added a split while we were away', () => { + const local = layout({ buffersByLeafId: { [LEAF_A]: 'parked-scrollback' } }) + const remote = layout({ root: SPLIT_ROOT, ptyIdsByLeafId: { [LEAF_B]: 'remote:env/pty-b' } }) + + const merged = retainLocalScrollbackInRemoteLayout(local, remote) + + expect(merged.root).toBe(SPLIT_ROOT) + expect(merged.ptyIdsByLeafId).toEqual({ [LEAF_B]: 'remote:env/pty-b' }) + // The shared leaf keeps its bytes; nothing is invented for the leaf the host just added. + expect(merged.buffersByLeafId).toEqual({ [LEAF_A]: 'parked-scrollback' }) + }) + + it('drops scrollback for a leaf the host retired', () => { + const local = layout({ + root: SPLIT_ROOT, + buffersByLeafId: { [LEAF_A]: 'kept', [LEAF_B]: 'retired-pane' } + }) + + const merged = retainLocalScrollbackInRemoteLayout(local, layout()) + + expect(merged.buffersByLeafId).toEqual({ [LEAF_A]: 'kept' }) + }) + + it('prefers the local copy when both sides hold bytes for one leaf', () => { + const local = layout({ buffersByLeafId: { [LEAF_A]: 'captured-since-last-upload' } }) + const remote = layout({ buffersByLeafId: { [LEAF_A]: 'older-upload' } }) + + const merged = retainLocalScrollbackInRemoteLayout(local, remote) + + expect(merged.buffersByLeafId).toEqual({ [LEAF_A]: 'captured-since-last-upload' }) + }) + + it('carries scrollback refs, which name client-local snapshot files only this client can read', () => { + const local = layout({ scrollbackRefsByLeafId: { [LEAF_A]: 'v1-abc' } }) + + const merged = retainLocalScrollbackInRemoteLayout(local, layout()) + + expect(merged.scrollbackRefsByLeafId).toEqual({ [LEAF_A]: 'v1-abc' }) + }) + + it('returns the host layout untouched when this client holds no scrollback', () => { + const remote = layout({ buffersByLeafId: { [LEAF_A]: 'from-another-client' } }) + + expect(retainLocalScrollbackInRemoteLayout(layout(), remote)).toBe(remote) + expect(retainLocalScrollbackInRemoteLayout(undefined, remote)).toBe(remote) + }) + + it('leaves a rootless host layout alone rather than guessing which leaves are live', () => { + const local = layout({ buffersByLeafId: { [LEAF_A]: 'parked-scrollback' } }) + const remote = layout({ root: null }) + + expect(retainLocalScrollbackInRemoteLayout(local, remote)).toBe(remote) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.ts b/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.ts new file mode 100644 index 00000000000..5146cc85be3 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-layout-scrollback-retention.ts @@ -0,0 +1,56 @@ +import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' +import { collectLeafIdsInOrder } from './terminal-layout-leaf-ids' +import { mergeCapturedLeafState } from './merge-captured-leaf-state' + +/** + * Carry this client's captured scrollback into the host's layout for a replaced tab. + * + * Why: the pull replaces a replaced tab's layout wholesale, and a park capture does not bump + * `tab.generation`, so a just-parked tab is not in `locallyPreservedTabIds` and its + * `buffersByLeafId` — the only client-side copy of a remote pane's scrollback — goes with it. + * The host is authoritative for structure and never mints scrollback of its own: its copy is + * only ever some client's earlier upload. So take `root`, `ptyIdsByLeafId`, titles and the + * active/expanded leaves from the host verbatim, and let the client fill in content the host's + * copy lacks. Structure from the host, bytes from whoever still has them. + * + * Why local wins a conflict: neither copy is then the only one, so neither choice destroys + * evidence — but remote-wins would overwrite the tail captured since this client's last upload + * and then propagate that regression back on the next replace-session patch. + * + * Why filtered to the host's leaves: it keeps the client from resurrecting a leaf the host + * retired, and from contributing anything for a split the host added while we were away. + */ +export function retainLocalScrollbackInRemoteLayout( + local: TerminalLayoutSnapshot | undefined, + remote: TerminalLayoutSnapshot +): TerminalLayoutSnapshot { + if (!local?.buffersByLeafId && !local?.scrollbackRefsByLeafId) { + return remote + } + const currentLeafIds = new Set(collectLeafIdsInOrder(remote.root)) + if (currentLeafIds.size === 0) { + return remote + } + const buffersByLeafId = mergeCapturedLeafState({ + prior: remote.buffersByLeafId, + fresh: local.buffersByLeafId ?? {}, + currentLeafIds + }) + const scrollbackRefsByLeafId = mergeCapturedLeafState({ + prior: remote.scrollbackRefsByLeafId, + fresh: local.scrollbackRefsByLeafId ?? {}, + currentLeafIds + }) + const retained = { ...remote } + if (Object.keys(buffersByLeafId).length > 0) { + retained.buffersByLeafId = buffersByLeafId + } else { + delete retained.buffersByLeafId + } + if (Object.keys(scrollbackRefsByLeafId).length > 0) { + retained.scrollbackRefsByLeafId = scrollbackRefsByLeafId + } else { + delete retained.scrollbackRefsByLeafId + } + return retained +} diff --git a/src/renderer/src/hooks/remote-workspace-session-merge-parked-scrollback.test.ts b/src/renderer/src/hooks/remote-workspace-session-merge-parked-scrollback.test.ts new file mode 100644 index 00000000000..c1dd4f7579d --- /dev/null +++ b/src/renderer/src/hooks/remote-workspace-session-merge-parked-scrollback.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from 'vitest' + +import { mergeDirectSshRemoteWorkspaceSession } from './remote-workspace-session-merge' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { TerminalTab, TerminalLayoutSnapshot } from '../../../shared/terminal-tab-types' +import type { AppState } from '../store/types' + +/** + * A reconnect must not delete the only copy of a parked remote pane's scrollback. + * + * A remote-runtime pty's bytes never transit main, so the park capture in + * `terminalLayoutsByTabId[tabId].buffersByLeafId` is the client's only copy. The reconnect replaces + * a replaced tab's layout wholesale, and a park capture does not bump `tab.generation` — so the tab + * is not in `locallyPreservedTabIds` and, before this was fixed, its scrollback went with the + * layout. The host never mints scrollback of its own; its copy is only ever an earlier upload. + */ +const WORKTREE = 'repo-1::/home/user/remote-checkout' +const TAB = 'tab-parked' +const LEAF = 'leaf-1' + +// Why a complete tab rather than a cast partial: the merge reads `generation` to decide local +// preservation, so a fixture missing it would make the control pass for the wrong reason. +function terminalTab(overrides: Partial = {}): TerminalTab { + return { + id: TAB, + ptyId: null, + worktreeId: WORKTREE, + title: TAB, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0, + generation: 1, + ...overrides + } +} + +function layout(overrides: Partial = {}): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId: LEAF }, + activeLeafId: LEAF, + expandedLeafId: null, + ...overrides + } +} + +function sessionState(overrides: Partial = {}): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + activeRepoId: 'repo-1', + activeWorktreeId: WORKTREE, + activeWorkspaceKey: worktreeWorkspaceKey(WORKTREE), + activeTabId: TAB, + ...overrides + } +} + +function merge( + current: WorkspaceSessionState, + remote: WorkspaceSessionState, + liveTabs: AppState['tabsByWorktree'] = {} +): WorkspaceSessionState { + return mergeDirectSshRemoteWorkspaceSession( + current, + remote, + new Set([WORKTREE]), + liveTabs, + new Set() + ) +} + +describe('direct-SSH reconnect merge: parked scrollback', () => { + it('keeps the park capture when the host copy predates it and no generation changed', () => { + const tab = terminalTab() + const current = sessionState({ + tabsByWorktree: { [WORKTREE]: [tab] }, + terminalLayoutsByTabId: { + [TAB]: layout({ buffersByLeafId: { [LEAF]: 'parked-scrollback' } }) + } + }) + // Same generation on both sides is the whole point: nothing marks this tab as locally newer. + const remote = sessionState({ + tabsByWorktree: { [WORKTREE]: [tab] }, + terminalLayoutsByTabId: { [TAB]: layout() } + }) + + const merged = merge(current, remote, { [WORKTREE]: [tab] }) + + expect(merged.terminalLayoutsByTabId[TAB]?.buffersByLeafId).toEqual({ + [LEAF]: 'parked-scrollback' + }) + }) + + it('still lets the host own the layout structure it changed while we were away', () => { + const tab = terminalTab() + const splitRoot: TerminalLayoutSnapshot['root'] = { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF }, + second: { type: 'leaf', leafId: 'leaf-2' } + } + const current = sessionState({ + tabsByWorktree: { [WORKTREE]: [tab] }, + terminalLayoutsByTabId: { + [TAB]: layout({ buffersByLeafId: { [LEAF]: 'parked-scrollback' } }) + } + }) + const remote = sessionState({ + tabsByWorktree: { [WORKTREE]: [tab] }, + terminalLayoutsByTabId: { [TAB]: layout({ root: splitRoot }) } + }) + + const merged = merge(current, remote, { [WORKTREE]: [tab] }) + + expect(merged.terminalLayoutsByTabId[TAB]?.root).toEqual(splitRoot) + expect(merged.terminalLayoutsByTabId[TAB]?.buffersByLeafId).toEqual({ + [LEAF]: 'parked-scrollback' + }) + }) +}) diff --git a/src/renderer/src/hooks/remote-workspace-session-merge.ts b/src/renderer/src/hooks/remote-workspace-session-merge.ts index f8c2c9dabf2..0baabb93031 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge.ts @@ -4,6 +4,7 @@ import { reconcileClosedTerminalTabTombstones } from '../../../shared/closed-ter import type { ExecutionHostId } from '../../../shared/execution-host' import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' import { splitWorktreeId } from '../../../shared/worktree/id' +import { retainLocalScrollbackInRemoteLayout } from '@/components/terminal-pane/remote-layout-scrollback-retention' import { getWorktreeIdFromHostIdentity, isWorktreeHostIdentity @@ -249,9 +250,15 @@ export function mergeDirectSshRemoteWorkspaceSession( ) ), ...Object.fromEntries( - Object.entries(remote.terminalLayoutsByTabId).filter( - ([tabId]) => !locallyPreservedTabIds.has(tabId) && !suppressedTabIds.has(tabId) - ) + Object.entries(remote.terminalLayoutsByTabId) + .filter(([tabId]) => !locallyPreservedTabIds.has(tabId) && !suppressedTabIds.has(tabId)) + // Why: this replace is wholesale, and a park capture does not bump tab.generation, so a + // just-parked tab is not locally preserved and the only client-side copy of its remote + // scrollback would go with its layout. Structure stays the host's. + .map(([tabId, layout]) => [ + tabId, + retainLocalScrollbackInRemoteLayout(current.terminalLayoutsByTabId[tabId], layout) + ]) ) } const activeOutsideTarget =