From 29e8eae70e01b59a44dff3362afc3897f8df21c5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 13 May 2026 16:34:44 -0700 Subject: [PATCH] perf(session): prune local terminal scrollback from persisted sessions (#1753) Co-authored-by: Orca --- src/main/daemon/daemon-init.test.ts | 53 ++++- src/main/daemon/daemon-init.ts | 4 + src/main/ipc/notifications.ts | 3 +- src/main/ipc/worktree-logic.ts | 10 +- src/main/memory/collector.ts | 7 +- src/main/persistence.test.ts | 126 ++++++++++- src/main/persistence.ts | 12 +- src/main/runtime/orca-runtime.ts | 15 +- src/main/usage-worktree-metadata.ts | 14 +- .../FloatingTerminalPanel.tsx | 2 +- .../settings/ManageSessionsSection.tsx | 8 +- .../status-bar/mergeSnapshotAndSessions.ts | 12 +- src/renderer/src/lib/floating-terminal.ts | 4 - .../src/lib/workspace-session.test.ts | 97 ++++++++- src/renderer/src/lib/workspace-session.ts | 8 +- .../store/slices/terminals-hydration.test.ts | 2 +- src/renderer/src/store/slices/terminals.ts | 12 +- .../src/store/slices/worktree-helpers.ts | 6 +- src/shared/constants.ts | 4 + ...workspace-session-terminal-buffers.test.ts | 204 ++++++++++++++++++ .../workspace-session-terminal-buffers.ts | 68 ++++++ src/shared/worktree-id.test.ts | 71 ++++++ src/shared/worktree-id.ts | 24 +++ tests/e2e/helpers/orca-restart.ts | 37 +++- tests/e2e/helpers/terminal.ts | 9 +- .../e2e/terminal-restart-persistence.spec.ts | 30 +-- 26 files changed, 749 insertions(+), 93 deletions(-) create mode 100644 src/shared/workspace-session-terminal-buffers.test.ts create mode 100644 src/shared/workspace-session-terminal-buffers.ts create mode 100644 src/shared/worktree-id.test.ts create mode 100644 src/shared/worktree-id.ts diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index 82914e487..f750369cc 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -14,6 +14,7 @@ import { PROTOCOL_VERSION } from './types' // non-daemon-init dependency is replaced by a minimal stub that records calls. const { getPathMock, + getAppPathMock, isPackagedMock, probeSocketExistsMock, netConnectMock, @@ -30,9 +31,10 @@ const { rebindLocalProviderListenersMock } = vi.hoisted(() => { const getPathMock = vi.fn(() => '/fake/userData') + const getAppPathMock = vi.fn(() => '/fake/app') const isPackagedMock = vi.fn(() => false) - const probeSocketExistsMock = vi.fn(() => false) + const probeSocketExistsMock = vi.fn((_path?: string) => false) const forkMock = vi.fn() const netConnectMock = vi.fn(() => { // Why: the real probeSocket() in daemon-init connects to the socket and @@ -79,6 +81,7 @@ const { return { getPathMock, + getAppPathMock, isPackagedMock, probeSocketExistsMock, netConnectMock, @@ -134,13 +137,13 @@ vi.mock('electron', () => ({ return isPackagedMock() }, getPath: getPathMock, - getAppPath: () => '/fake/app' + getAppPath: getAppPathMock } })) vi.mock('fs', () => ({ mkdirSync: vi.fn(), - existsSync: (p: string) => probeSocketExistsMock() || p.includes('.pid'), + existsSync: (p: string) => probeSocketExistsMock(p) || p.includes('.pid'), unlinkSync: vi.fn(), writeFileSync: vi.fn() })) @@ -242,6 +245,8 @@ async function importFresh() { healthCheckDaemonMock.mockClear() getDaemonLaunchIdentityMock.mockClear() killStaleDaemonMock.mockClear() + getAppPathMock.mockReset() + getAppPathMock.mockReturnValue('/fake/app') forkMock.mockReset() isPackagedMock.mockReset() isPackagedMock.mockReturnValue(false) @@ -670,6 +675,48 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ) }) + it('uses the direct daemon entry when Electron app path is already out/main', async () => { + probeSocketExistsMock.mockImplementation( + (p?: string) => p === '/fake/app/out/main/daemon-entry.js' + ) + healthCheckDaemonMock.mockResolvedValueOnce(false) + const mod = await importFresh() + getAppPathMock.mockReturnValue('/fake/app/out/main') + await mod.initDaemonPtyProvider() + + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + forkMock.mockImplementationOnce(() => { + const handlers: Record void)[]> = { + message: [], + error: [], + exit: [] + } + return { + pid: 12345, + on(event: string, cb: (arg?: unknown) => void) { + handlers[event]?.push(cb) + if (event === 'message') { + queueMicrotask(() => cb({ type: 'ready' })) + } + return this + }, + disconnect: vi.fn(), + unref: vi.fn() + } + }) + + await launcher('/fake/socket', '/fake/token') + + expect(forkMock).toHaveBeenCalledWith( + '/fake/app/out/main/daemon-entry.js', + ['--socket', '/fake/socket', '--token', '/fake/token'], + expect.objectContaining({ detached: true }) + ) + }) + it('keeps packaged healthy-daemon reuse independent of dev app-path identity', async () => { const mod = await importFresh() await mod.initDaemonPtyProvider() diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 715701b60..2e1833078 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -65,6 +65,10 @@ function getDaemonEntryPath(): string { // execute it from disk. In packaged apps app.getAppPath() points at // app.asar, so redirect to the unpacked sibling before joining the script. const basePath = app.isPackaged ? appPath.replace('app.asar', 'app.asar.unpacked') : appPath + const directEntryPath = join(basePath, 'daemon-entry.js') + if (existsSync(directEntryPath)) { + return directEntryPath + } return join(basePath, 'out', 'main', 'daemon-entry.js') } diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index e67b3fa08..775130af9 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -8,6 +8,7 @@ import type { NotificationPermissionStatusResult, NotificationSoundDataResult } from '../../shared/types' +import { getRepoIdFromWorktreeId } from '../../shared/worktree-id' import type { OrcaRuntimeService } from '../runtime/orca-runtime' const NOTIFICATION_COOLDOWN_MS = 5000 @@ -154,7 +155,7 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime // the click-to-navigate binding — the notification still fires but // clicking it will not attempt to switch to an unknown worktree. if (args.worktreeId && args.worktreeId.includes('::')) { - const repoId = args.worktreeId.slice(0, args.worktreeId.indexOf('::')) + const repoId = getRepoIdFromWorktreeId(args.worktreeId) notification.on('click', () => { release() const win = BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()) diff --git a/src/main/ipc/worktree-logic.ts b/src/main/ipc/worktree-logic.ts index b950ec289..382ba20f6 100644 --- a/src/main/ipc/worktree-logic.ts +++ b/src/main/ipc/worktree-logic.ts @@ -1,5 +1,6 @@ import { basename, join, resolve, relative, isAbsolute, posix, win32 } from 'path' import type { GitWorktreeInfo, Worktree, WorktreeMeta } from '../../shared/types' +import { splitWorktreeId } from '../../shared/worktree-id' import { getWslHome, parseWslPath } from '../wsl' /** @@ -212,14 +213,11 @@ export function mergeWorktree( * Parse a composite worktreeId ("repoId::worktreePath") into its parts. */ export function parseWorktreeId(worktreeId: string): { repoId: string; worktreePath: string } { - const sepIdx = worktreeId.indexOf('::') - if (sepIdx === -1) { + const parsed = splitWorktreeId(worktreeId) + if (!parsed) { throw new Error(`Invalid worktreeId: ${worktreeId}`) } - return { - repoId: worktreeId.slice(0, sepIdx), - worktreePath: worktreeId.slice(sepIdx + 2) - } + return parsed } /** diff --git a/src/main/memory/collector.ts b/src/main/memory/collector.ts index b00b24f47..bd5d93c39 100644 --- a/src/main/memory/collector.ts +++ b/src/main/memory/collector.ts @@ -23,6 +23,7 @@ import { basename } from 'node:path' import { exec } from 'node:child_process' import { promisify } from 'node:util' import os from 'node:os' +import { splitWorktreeId } from '../../shared/worktree-id' import { app } from 'electron' import type { AppMemory, @@ -374,9 +375,9 @@ function resolveWorktreeNames( repoName: string } { // Orca worktree ids look like `${repoId}::${absolutePath}`. - const sep = worktreeId.indexOf('::') - const repoId = sep > 0 ? worktreeId.slice(0, sep) : worktreeId - const worktreePath = sep > 0 ? worktreeId.slice(sep + 2) : '' + const parsed = splitWorktreeId(worktreeId) + const repoId = parsed?.repoId ?? worktreeId + const worktreePath = parsed?.worktreePath ?? '' const fallbackName = worktreePath ? basename(worktreePath) : worktreeId const meta = store.getWorktreeMeta(worktreeId) diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 694618151..b00b6cdce 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -5,7 +5,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { writeFileSync, readFileSync, rmSync, mkdtempSync, mkdirSync } from 'fs' import { join } from 'path' import { tmpdir } from 'os' -import type { Repo } from '../shared/types' +import type { Repo, TerminalTab, WorkspaceSessionState } from '../shared/types' // Shared mutable state so the electron mock can reference a per-test directory const testState = { dir: '' } @@ -61,6 +61,58 @@ const makeRepo = (overrides: Partial = {}): Repo => ({ ...overrides }) +const makeTerminalTab = (overrides: Partial = {}): TerminalTab => ({ + id: 'tab1', + ptyId: 'pty1', + worktreeId: 'repo1::/worktree', + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ...overrides +}) + +function makeSessionWithTerminalBuffers(): WorkspaceSessionState { + return { + activeRepoId: 'local-repo', + activeWorktreeId: 'local-repo::/local', + activeTabId: 'local-tab', + tabsByWorktree: { + 'local-repo::/local': [ + makeTerminalTab({ + id: 'local-tab', + ptyId: 'local-pty', + worktreeId: 'local-repo::/local' + }) + ], + 'remote-repo::/remote': [ + makeTerminalTab({ + id: 'remote-tab', + ptyId: 'remote-pty', + worktreeId: 'remote-repo::/remote' + }) + ] + }, + terminalLayoutsByTabId: { + 'local-tab': { + root: { type: 'leaf', leafId: 'leaf-local' }, + activeLeafId: 'leaf-local', + expandedLeafId: null, + buffersByLeafId: { 'leaf-local': 'local-scrollback' }, + ptyIdsByLeafId: { 'leaf-local': 'local-pty' } + }, + 'remote-tab': { + root: { type: 'leaf', leafId: 'leaf-remote' }, + activeLeafId: 'leaf-remote', + expandedLeafId: null, + buffersByLeafId: { 'leaf-remote': 'remote-scrollback' }, + ptyIdsByLeafId: { 'leaf-remote': 'remote-pty' } + } + } + } +} + describe('Store', () => { beforeEach(() => { testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-')) @@ -917,6 +969,78 @@ describe('Store', () => { expect(store.getWorkspaceSession()).toEqual(session) }) + it('strips local terminal scrollback buffers when setting workspace session', async () => { + const store = await createStore() + store.addRepo(makeRepo({ id: 'local-repo', connectionId: null })) + store.addRepo(makeRepo({ id: 'remote-repo', connectionId: 'ssh-target-1' })) + + store.setWorkspaceSession(makeSessionWithTerminalBuffers()) + + const session = store.getWorkspaceSession() + expect(session.terminalLayoutsByTabId['local-tab'].buffersByLeafId).toBeUndefined() + expect(session.terminalLayoutsByTabId['local-tab'].ptyIdsByLeafId).toEqual({ + 'leaf-local': 'local-pty' + }) + expect(session.terminalLayoutsByTabId['remote-tab'].buffersByLeafId).toEqual({ + 'leaf-remote': 'remote-scrollback' + }) + }) + + it('keeps terminal scrollback buffers when the repo catalog is not hydrated yet', async () => { + const store = await createStore() + + store.setWorkspaceSession({ + activeRepoId: 'remote-repo', + activeWorktreeId: 'remote-repo::/remote', + activeTabId: 'remote-tab', + tabsByWorktree: { + 'remote-repo::/remote': [ + makeTerminalTab({ + id: 'remote-tab', + ptyId: 'remote-pty', + worktreeId: 'remote-repo::/remote' + }) + ] + }, + terminalLayoutsByTabId: { + 'remote-tab': { + root: { type: 'leaf', leafId: 'leaf-remote' }, + activeLeafId: 'leaf-remote', + expandedLeafId: null, + buffersByLeafId: { 'leaf-remote': 'maybe-remote-scrollback' } + } + } + }) + + expect( + store.getWorkspaceSession().terminalLayoutsByTabId['remote-tab'].buffersByLeafId + ).toEqual({ + 'leaf-remote': 'maybe-remote-scrollback' + }) + }) + + it('strips legacy local terminal scrollback buffers when loading workspace session', async () => { + writeDataFile({ + schemaVersion: 1, + repos: [ + makeRepo({ id: 'local-repo', connectionId: null }), + makeRepo({ id: 'remote-repo', connectionId: 'ssh-target-1' }) + ], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + workspaceSession: makeSessionWithTerminalBuffers() + }) + + const store = await createStore() + const session = store.getWorkspaceSession() + expect(session.terminalLayoutsByTabId['local-tab'].buffersByLeafId).toBeUndefined() + expect(session.terminalLayoutsByTabId['remote-tab'].buffersByLeafId).toEqual({ + 'leaf-remote': 'remote-scrollback' + }) + }) + it('does not restore cleared SSH bindings after a lease expired', async () => { const store = await createStore() store.upsertSshRemotePtyLease({ diff --git a/src/main/persistence.ts b/src/main/persistence.ts index c1fd20d94..7aea40c20 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -31,6 +31,8 @@ import { ONBOARDING_FINAL_STEP } from '../shared/constants' import { parseWorkspaceSession } from '../shared/workspace-session-schema' +import { pruneLocalTerminalScrollbackBuffers } from '../shared/workspace-session-terminal-buffers' +import { getRepoIdFromWorktreeId } from '../shared/worktree-id' function encrypt(plaintext: string): string { if (!plaintext || !safeStorage.isEncryptionAvailable()) { @@ -446,6 +448,11 @@ export class Store { result = getDefaultPersistedState(homedir()) } + result = { + ...result, + workspaceSession: pruneLocalTerminalScrollbackBuffers(result.workspaceSession, result.repos) + } + return this.migrateTelemetry(result, fileExistedOnLoad) } @@ -899,6 +906,8 @@ export class Store { } setWorkspaceSession(session: PersistedState['workspaceSession']): void { + session = pruneLocalTerminalScrollbackBuffers(session, this.state.repos) + // Why: closes the second half of the SIGKILL race (Issue #217). The // renderer's debounced session writer captures its state BEFORE pty:spawn // returns, so the snapshot it later flushes via session:set has no @@ -1080,8 +1089,7 @@ export class Store { } private getConnectionIdForWorktree(worktreeId: string): string | null { - const separatorIdx = worktreeId.indexOf('::') - const repoId = separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx) + const repoId = getRepoIdFromWorktreeId(worktreeId) return this.state.repos.find((repo) => repo.id === repoId)?.connectionId ?? null } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 02e36514b..4a9e5a66f 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -23,6 +23,7 @@ import type { WorktreeRemoteBranchConflictEvent, WorktreeStartupLaunch } from '../../shared/types' +import { getRepoIdFromWorktreeId, splitWorktreeId } from '../../shared/worktree-id' import { isFolderRepo } from '../../shared/repo-kind' import { buildSetupRunnerCommand } from '../../shared/setup-runner-command' import { FIRST_PANE_ID } from '../../shared/pane-key' @@ -6031,7 +6032,7 @@ export class OrcaRuntimeService { // browser commands fail with "CDP connection refused". private async ensureBrowserWorktreeActive(worktreeId: string): Promise { const win = this.getAuthoritativeWindow() - const repoId = worktreeId.split('::')[0] + const repoId = getRepoIdFromWorktreeId(worktreeId) if (!repoId) { return } @@ -7474,18 +7475,14 @@ function inferWorktreeIdFromPtyId(ptyId: string): string | null { function parseRuntimeWorktreeId( worktreeId: string ): { repoId: string; worktreePath: string } | null { - const separatorIndex = worktreeId.indexOf('::') - if (separatorIndex <= 0) { + const parsed = splitWorktreeId(worktreeId) + if (!parsed?.repoId) { return null } - const worktreePath = worktreeId.slice(separatorIndex + 2) - if (!worktreePath) { + if (!parsed.worktreePath) { return null } - return { - repoId: worktreeId.slice(0, separatorIndex), - worktreePath - } + return parsed } function findResolvedWorktreeIdForPath( diff --git a/src/main/usage-worktree-metadata.ts b/src/main/usage-worktree-metadata.ts index f08bc98d6..442638d9f 100644 --- a/src/main/usage-worktree-metadata.ts +++ b/src/main/usage-worktree-metadata.ts @@ -1,5 +1,6 @@ import { basename } from 'path' import type { Repo } from '../shared/types' +import { splitWorktreeId } from '../shared/worktree-id' import type { Store } from './persistence' export type UsageWorktreeRef = { @@ -12,17 +13,6 @@ function getDefaultUsageWorktreeLabel(pathValue: string): string { return basename(pathValue) } -function parseKnownWorktreeId(worktreeId: string): { repoId: string; worktreePath: string } | null { - const sepIdx = worktreeId.indexOf('::') - if (sepIdx === -1) { - return null - } - return { - repoId: worktreeId.slice(0, sepIdx), - worktreePath: worktreeId.slice(sepIdx + 2) - } -} - export function loadKnownUsageWorktreesByRepo( store: Pick, repos: Repo[] @@ -46,7 +36,7 @@ export function loadKnownUsageWorktreesByRepo( // Why: usage scans are background/opt-in analytics. Do not spawn // `git worktree list` here; it can re-touch macOS protected folders. for (const [worktreeId, meta] of Object.entries(store.getAllWorktreeMeta())) { - const parsed = parseKnownWorktreeId(worktreeId) + const parsed = splitWorktreeId(worktreeId) if (!parsed || !repoIds.has(parsed.repoId)) { continue } diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index cf8d5ec7c..a2f211c8f 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -4,9 +4,9 @@ import TabBar from '@/components/tab-bar/TabBar' import TerminalPane from '@/components/terminal-pane/TerminalPane' import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { FLOATING_TERMINAL_WORKTREE_ID } from '@/lib/floating-terminal' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' import { useAppStore } from '@/store' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import type { TerminalTab } from '../../../../shared/types' import { FloatingTerminalResizeHandles } from './FloatingTerminalResizeHandles' import { diff --git a/src/renderer/src/components/settings/ManageSessionsSection.tsx b/src/renderer/src/components/settings/ManageSessionsSection.tsx index 79e8de123..921488216 100644 --- a/src/renderer/src/components/settings/ManageSessionsSection.tsx +++ b/src/renderer/src/components/settings/ManageSessionsSection.tsx @@ -18,6 +18,7 @@ import { import { Tooltip, TooltipContent, TooltipTrigger } from '../ui/tooltip' import { SearchableSetting } from './SearchableSetting' import { MANAGE_SESSIONS_SEARCH_ENTRIES } from './terminal-search' +import { splitWorktreeId } from '../../../../shared/worktree-id' import { useAppStore } from '../../store' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' @@ -52,10 +53,9 @@ function formatWorkspace(session: { cwd: string | null; sessionId: string }): st const sep = session.sessionId.lastIndexOf('@@') if (sep !== -1) { const worktreeId = session.sessionId.slice(0, sep) - // Strip a leading `::` prefix if present (newer protocol). - const afterColons = worktreeId.split('::') - const path = afterColons.at(-1) - return shortCwd(path ?? worktreeId) + // Why: take everything after the first `::` to recover the worktree path + // from the canonical `${repoId}::${path}` worktreeId encoding. + return shortCwd(splitWorktreeId(worktreeId)?.worktreePath ?? worktreeId) } return 'unknown' } diff --git a/src/renderer/src/components/status-bar/mergeSnapshotAndSessions.ts b/src/renderer/src/components/status-bar/mergeSnapshotAndSessions.ts index 2cd2b473a..665b3b620 100644 --- a/src/renderer/src/components/status-bar/mergeSnapshotAndSessions.ts +++ b/src/renderer/src/components/status-bar/mergeSnapshotAndSessions.ts @@ -28,7 +28,8 @@ import type { TerminalTab, WorktreeMemory } from '../../../../shared/types' -import { parsePtySessionId, WORKTREE_ID_SEPARATOR } from '../../../../shared/pty-session-id-format' +import { getRepoIdFromWorktreeId, splitWorktreeId } from '../../../../shared/worktree-id' +import { parsePtySessionId } from '../../../../shared/pty-session-id-format' // ─── View-model types (renderer-local) ────────────────────────────── @@ -105,16 +106,15 @@ export type MergeContext = { // ─── Helpers ──────────────────────────────────────────────────────── function deriveRepoIdFromWorktreeId(worktreeId: string): string { - const sep = worktreeId.indexOf(WORKTREE_ID_SEPARATOR) - return sep > 0 ? worktreeId.slice(0, sep) : worktreeId + return getRepoIdFromWorktreeId(worktreeId) } function deriveWorktreeNameFromWorktreeId(worktreeId: string): string { - const sep = worktreeId.indexOf(WORKTREE_ID_SEPARATOR) - if (sep <= 0) { + const parsed = splitWorktreeId(worktreeId) + if (!parsed) { return worktreeId } - const path = worktreeId.slice(sep + WORKTREE_ID_SEPARATOR.length) + const path = parsed.worktreePath if (!path) { return worktreeId } diff --git a/src/renderer/src/lib/floating-terminal.ts b/src/renderer/src/lib/floating-terminal.ts index 5df9520d4..85a53f5d7 100644 --- a/src/renderer/src/lib/floating-terminal.ts +++ b/src/renderer/src/lib/floating-terminal.ts @@ -1,5 +1 @@ -// Why: a stable synthetic worktree id lets the floating terminal reuse the normal -// tab/store/TerminalPane lifecycle without attaching it to any repo worktree. -export const FLOATING_TERMINAL_WORKTREE_ID = 'global-floating-terminal' - export const TOGGLE_FLOATING_TERMINAL_EVENT = 'orca-toggle-floating-terminal' diff --git a/src/renderer/src/lib/workspace-session.test.ts b/src/renderer/src/lib/workspace-session.test.ts index 93e590a78..a7416d770 100644 --- a/src/renderer/src/lib/workspace-session.test.ts +++ b/src/renderer/src/lib/workspace-session.test.ts @@ -5,7 +5,7 @@ import { type WorkspaceSessionSnapshot } from './workspace-session' import type { AppState } from '../store' -import { FLOATING_TERMINAL_WORKTREE_ID } from './floating-terminal' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' function createSnapshot(overrides: Partial = {}): AppState { return { @@ -86,6 +86,17 @@ function createSnapshot(overrides: Partial = {}): AppState { } as AppState } +function createRepo(id: string, connectionId: string | null): AppState['repos'][number] { + return { + id, + path: `/${id}`, + displayName: id, + badgeColor: '#fff', + addedAt: 1, + connectionId + } +} + describe('buildWorkspaceSessionPayload', () => { it('preserves activeWorktreeIdsOnShutdown for full replacement writes', () => { const payload = buildWorkspaceSessionPayload(createSnapshot()) @@ -107,7 +118,13 @@ describe('buildWorkspaceSessionPayload', () => { ] }, terminalLayoutsByTabId: { - 'floating-tab-1': { root: null, activeLeafId: null, expandedLeafId: null } + 'floating-tab-1': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'floating-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'floating-pty-1' } + } }, activeTabIdByWorktree: { [FLOATING_TERMINAL_WORKTREE_ID]: 'floating-tab-1' @@ -117,7 +134,10 @@ describe('buildWorkspaceSessionPayload', () => { expect(payload.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID]).toHaveLength(1) expect(payload.activeTabIdByWorktree?.[FLOATING_TERMINAL_WORKTREE_ID]).toBe('floating-tab-1') - expect(payload.terminalLayoutsByTabId['floating-tab-1']).toBeDefined() + expect(payload.terminalLayoutsByTabId['floating-tab-1'].buffersByLeafId).toBeUndefined() + expect(payload.terminalLayoutsByTabId['floating-tab-1'].ptyIdsByLeafId).toEqual({ + 'pane:1': 'floating-pty-1' + }) expect(payload.activeWorktreeIdsOnShutdown).toEqual([FLOATING_TERMINAL_WORKTREE_ID]) }) @@ -138,6 +158,75 @@ describe('buildWorkspaceSessionPayload', () => { expect(payload.browserTabsByWorktree?.['wt-1'][0].loading).toBe(false) }) + it('drops local terminal scrollback buffers from session payloads', () => { + const localWorktreeId = 'repo-1::/local/worktree' + const payload = buildWorkspaceSessionPayload( + createSnapshot({ + tabsByWorktree: { + [localWorktreeId]: [ + { + id: 'tab-local', + title: 'shell', + ptyId: 'pty-1', + worktreeId: localWorktreeId + } as never + ] + }, + terminalLayoutsByTabId: { + 'tab-local': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'serialized-local-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'pty-1' }, + titlesByLeafId: { 'pane:1': 'build' } + } + }, + repos: [createRepo('repo-1', null)] + }) + ) + + expect(payload.terminalLayoutsByTabId['tab-local']).toEqual({ + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { 'pane:1': 'pty-1' }, + titlesByLeafId: { 'pane:1': 'build' } + }) + }) + + it('preserves SSH terminal scrollback buffers because relay teardown has no local history', () => { + const sshWorktreeId = 'repo-ssh::/remote/worktree' + const payload = buildWorkspaceSessionPayload( + createSnapshot({ + tabsByWorktree: { + [sshWorktreeId]: [ + { + id: 'tab-ssh', + title: 'remote', + ptyId: 'relay-pty-1', + worktreeId: sshWorktreeId + } as never + ] + }, + terminalLayoutsByTabId: { + 'tab-ssh': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'serialized-remote-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'relay-pty-1' } + } + }, + repos: [createRepo('repo-ssh', 'conn-1')] + }) + ) + + expect(payload.terminalLayoutsByTabId['tab-ssh'].buffersByLeafId).toEqual({ + 'pane:1': 'serialized-remote-scrollback' + }) + }) + it('uses lastKnownRelayPtyIdByTabId fallback for SSH worktrees with null ptyIds', () => { const payload = buildWorkspaceSessionPayload( createSnapshot({ @@ -146,7 +235,7 @@ describe('buildWorkspaceSessionPayload', () => { 'wt-ssh': [{ id: 'tab-ssh', title: 'remote', ptyId: null, worktreeId: 'wt-ssh' } as never] }, lastKnownRelayPtyIdByTabId: { 'tab-ssh': 'relay-sess-42' }, - repos: [{ id: 'repo-ssh', connectionId: 'conn-1' } as never], + repos: [createRepo('repo-ssh', 'conn-1')], worktreesByRepo: { 'repo-ssh': [{ id: 'wt-ssh', repoId: 'repo-ssh' } as never] }, diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index 75f7243b9..13a45d971 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -5,6 +5,7 @@ import type { WorkspaceSessionState, WorkspaceVisibleTabType } from '../../../shared/types' +import { pruneLocalTerminalScrollbackBuffers } from '../../../shared/workspace-session-terminal-buffers' import type { AppState } from '../store' import type { OpenFile } from '../store/slices/editor' @@ -176,7 +177,6 @@ export function buildWorkspaceSessionPayload( const groupsByWorktree = snapshot.groupsByWorktree const layoutByWorktree = snapshot.layoutByWorktree const activeGroupIdByWorktree = snapshot.activeGroupIdByWorktree - const terminalLayoutsByTabId = snapshot.terminalLayoutsByTabId // Why: lastKnownRelayPtyIdByTabId preserves session IDs across relay // disconnect/reconnect cycles. tab.ptyId is cleared on disconnect, but @@ -235,12 +235,12 @@ export function buildWorkspaceSessionPayload( ]) ) - return { + const payload = { activeRepoId: snapshot.activeRepoId, activeWorktreeId: snapshot.activeWorktreeId, activeTabId: snapshot.activeTabId, tabsByWorktree: sanitizedTabsByWorktree, - terminalLayoutsByTabId, + terminalLayoutsByTabId: snapshot.terminalLayoutsByTabId, // Why: session:set fully replaces the persisted object, so every write path // must carry forward which worktrees still had live PTYs. Dropping this // field silently disables eager terminal reconnect on the next restart. @@ -274,4 +274,6 @@ export function buildWorkspaceSessionPayload( ? snapshot.lastVisitedAtByWorktreeId : undefined } + + return pruneLocalTerminalScrollbackBuffers(payload, snapshot.repos) } diff --git a/src/renderer/src/store/slices/terminals-hydration.test.ts b/src/renderer/src/store/slices/terminals-hydration.test.ts index 16255ad4c..cae64760f 100644 --- a/src/renderer/src/store/slices/terminals-hydration.test.ts +++ b/src/renderer/src/store/slices/terminals-hydration.test.ts @@ -76,9 +76,9 @@ const mockApi = { globalThis.window = { api: mockApi } import type { WorkspaceSessionState } from '../../../../shared/types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers' import { canGoBackWorktreeHistory } from './worktree-nav-history' -import { FLOATING_TERMINAL_WORKTREE_ID } from '@/lib/floating-terminal' describe('hydrateWorkspaceSession', () => { beforeEach(() => { diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index e60b66595..93946a0fb 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -8,6 +8,8 @@ import type { Worktree, WorkspaceSessionState } from '../../../../shared/types' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { getRepoIdFromWorktreeId, splitWorktreeId } from '../../../../shared/worktree-id' import type { AgentStartedTelemetry } from '../../lib/worktree-activation' import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph' import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers' @@ -26,7 +28,6 @@ import { unregisterPtyDataHandlers } from '@/components/terminal-pane/pty-transport' import { shutdownBufferCaptures } from '@/components/terminal-pane/shutdown-buffer-captures' -import { FLOATING_TERMINAL_WORKTREE_ID } from '@/lib/floating-terminal' function getNextTerminalOrdinal(tabs: TerminalTab[]): number { const usedOrdinals = new Set() @@ -1437,7 +1438,7 @@ export const createTerminalSlice: StateCreator validWorktreeIds.add(FLOATING_TERMINAL_WORKTREE_ID) for (const worktreeId of Object.keys(session.tabsByWorktree)) { if (!validWorktreeIds.has(worktreeId)) { - const repoId = worktreeId.split('::')[0] + const repoId = getRepoIdFromWorktreeId(worktreeId) if (sshRepoIds.has(repoId)) { validWorktreeIds.add(worktreeId) } @@ -1585,7 +1586,7 @@ export const createTerminalSlice: StateCreator // data once SSH reconnects and fetchWorktrees runs. const worktreesByRepo = { ...s.worktreesByRepo } for (const worktreeId of Object.keys(tabsByWorktree)) { - const repoId = worktreeId.split('::')[0] + const repoId = getRepoIdFromWorktreeId(worktreeId) if (!sshRepoIds.has(repoId)) { continue } @@ -1593,10 +1594,7 @@ export const createTerminalSlice: StateCreator if (existing) { continue } - // Why: worktreeId is `${repoId}::${path}` and POSIX paths can legally - // contain `::`. Split only on the first separator to preserve the full path. - const separatorIdx = worktreeId.indexOf('::') - const path = separatorIdx >= 0 ? worktreeId.slice(separatorIdx + 2) : '' + const path = splitWorktreeId(worktreeId)?.worktreePath ?? '' // Why: SSH worktree paths may use backslash separators on Windows remotes. const displayName = path.split(/[/\\]/).pop() || path const placeholder: Worktree = { diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 64072125e..66324ff6c 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -9,6 +9,7 @@ import type { WorktreeRemoteBranchConflictEvent, WorktreeMeta } from '../../../../shared/types' +export { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id' export type WorktreeDeleteState = { isDeleting: boolean @@ -165,8 +166,3 @@ export function applyWorktreeUpdates( return changed ? next : worktreesByRepo } - -export function getRepoIdFromWorktreeId(worktreeId: string): string { - const sepIdx = worktreeId.indexOf('::') - return sepIdx === -1 ? worktreeId : worktreeId.slice(0, sepIdx) -} diff --git a/src/shared/constants.ts b/src/shared/constants.ts index f0a0b4eb2..d9d104794 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -100,6 +100,10 @@ export const DEFAULT_STATUS_BAR_ITEMS: StatusBarItem[] = [ * the collector and the status-bar popover agree on the sentinel. */ export const ORPHAN_WORKTREE_ID = '__orphan__' +// Why: the floating terminal is a local synthetic workspace, so persistence +// pruning must classify it without consulting the repo catalog. +export const FLOATING_TERMINAL_WORKTREE_ID = 'global-floating-terminal' + export const REPO_COLORS = [ '#737373', // neutral '#ef4444', // red diff --git a/src/shared/workspace-session-terminal-buffers.test.ts b/src/shared/workspace-session-terminal-buffers.test.ts new file mode 100644 index 000000000..01d54528a --- /dev/null +++ b/src/shared/workspace-session-terminal-buffers.test.ts @@ -0,0 +1,204 @@ +import { describe, expect, it } from 'vitest' +import { FLOATING_TERMINAL_WORKTREE_ID } from './constants' +import type { WorkspaceSessionState } from './types' +import { pruneLocalTerminalScrollbackBuffers } from './workspace-session-terminal-buffers' + +function makeSession(overrides: Partial = {}): WorkspaceSessionState { + return { + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: { + 'local-repo::/local/worktree': [ + { + id: 'local-tab', + title: 'local', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ptyId: 'local-pty', + worktreeId: 'local-repo::/local/worktree' + } + ], + 'remote-repo::/remote/worktree': [ + { + id: 'remote-tab', + title: 'remote', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ptyId: 'remote-pty', + worktreeId: 'remote-repo::/remote/worktree' + } + ] + }, + terminalLayoutsByTabId: { + 'local-tab': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'local-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'local-pty' } + }, + 'remote-tab': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'remote-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'remote-pty' } + } + }, + ...overrides + } +} + +describe('pruneLocalTerminalScrollbackBuffers', () => { + it('drops local buffers while preserving SSH buffers and PTY bindings', () => { + const result = pruneLocalTerminalScrollbackBuffers(makeSession(), [ + { id: 'local-repo', connectionId: null }, + { id: 'remote-repo', connectionId: 'ssh-target-1' } + ]) + + expect(result.terminalLayoutsByTabId['local-tab']).toEqual({ + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { 'pane:1': 'local-pty' } + }) + expect(result.terminalLayoutsByTabId['remote-tab'].buffersByLeafId).toEqual({ + 'pane:1': 'remote-scrollback' + }) + }) + + it('drops floating terminal buffers even though the synthetic worktree has no repo', () => { + const result = pruneLocalTerminalScrollbackBuffers( + makeSession({ + tabsByWorktree: { + [FLOATING_TERMINAL_WORKTREE_ID]: [ + { + id: 'floating-tab', + title: 'floating', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ptyId: 'floating-pty', + worktreeId: FLOATING_TERMINAL_WORKTREE_ID + } + ] + }, + terminalLayoutsByTabId: { + 'floating-tab': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'floating-scrollback' }, + ptyIdsByLeafId: { 'pane:1': 'floating-pty' } + } + } + }), + [] + ) + + expect(result.terminalLayoutsByTabId['floating-tab']).toEqual({ + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { 'pane:1': 'floating-pty' } + }) + }) + + it('treats orphaned layouts as local and prunes their buffers', () => { + const result = pruneLocalTerminalScrollbackBuffers( + makeSession({ + terminalLayoutsByTabId: { + 'orphan-tab': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'orphan-scrollback' } + } + } + }), + [{ id: 'remote-repo', connectionId: 'ssh-target-1' }] + ) + + expect(result.terminalLayoutsByTabId['orphan-tab'].buffersByLeafId).toBeUndefined() + }) + + it('preserves buffers for unresolved repo catalogs until worktrees can be classified', () => { + const result = pruneLocalTerminalScrollbackBuffers( + makeSession({ + tabsByWorktree: { + 'remote-repo::/remote/worktree': [ + { + id: 'remote-tab', + title: 'remote', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1, + ptyId: 'remote-pty', + worktreeId: 'remote-repo::/remote/worktree' + } + ] + }, + terminalLayoutsByTabId: { + 'remote-tab': { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': 'maybe-remote-scrollback' } + } + } + }), + [] + ) + + expect(result.terminalLayoutsByTabId['remote-tab'].buffersByLeafId).toEqual({ + 'pane:1': 'maybe-remote-scrollback' + }) + }) + + it('keeps persisted session size from scaling with local scrollback buffers', () => { + const largeScrollback = 'x'.repeat(8 * 1024) + const tabs = Array.from({ length: 8 }, (_, index) => ({ + id: `local-tab-${index}`, + title: `local ${index}`, + customTitle: null, + color: null, + sortOrder: index, + createdAt: index, + ptyId: `local-pty-${index}`, + worktreeId: 'local-repo::/local/worktree' + })) + const session = makeSession({ + tabsByWorktree: { + 'local-repo::/local/worktree': tabs + }, + terminalLayoutsByTabId: Object.fromEntries( + tabs.map((tab, index) => [ + tab.id, + { + root: null, + activeLeafId: null, + expandedLeafId: null, + buffersByLeafId: { 'pane:1': `${largeScrollback}-${index}` }, + ptyIdsByLeafId: { 'pane:1': tab.ptyId ?? '' } + } + ]) + ) + }) + + const originalBytes = Buffer.byteLength(JSON.stringify(session)) + const result = pruneLocalTerminalScrollbackBuffers(session, [ + { id: 'local-repo', connectionId: null } + ]) + const prunedBytes = Buffer.byteLength(JSON.stringify(result)) + + expect(JSON.stringify(result)).not.toContain(largeScrollback) + expect(prunedBytes).toBeLessThan(originalBytes / 5) + }) +}) diff --git a/src/shared/workspace-session-terminal-buffers.ts b/src/shared/workspace-session-terminal-buffers.ts new file mode 100644 index 000000000..77bc0c48f --- /dev/null +++ b/src/shared/workspace-session-terminal-buffers.ts @@ -0,0 +1,68 @@ +import type { Repo, WorkspaceSessionState } from './types' +import { FLOATING_TERMINAL_WORKTREE_ID } from './constants' +import { getRepoIdFromWorktreeId } from './worktree-id' + +export type RepoConnection = Pick + +export function pruneLocalTerminalScrollbackBuffers( + session: WorkspaceSessionState, + repos: readonly RepoConnection[] +): WorkspaceSessionState { + const connectionIdByRepoId = new Map(repos.map((repo) => [repo.id, repo.connectionId] as const)) + const worktreeIdByTabId = new Map() + for (const [worktreeId, tabs] of Object.entries(session.tabsByWorktree)) { + for (const tab of tabs) { + worktreeIdByTabId.set(tab.id, worktreeId) + } + } + + let terminalLayoutsByTabId: WorkspaceSessionState['terminalLayoutsByTabId'] | null = null + for (const [tabId, layout] of Object.entries(session.terminalLayoutsByTabId)) { + if (!layout.buffersByLeafId) { + continue + } + const worktreeId = worktreeIdByTabId.get(tabId) + if (worktreeId !== undefined) { + if (worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + terminalLayoutsByTabId ??= { ...session.terminalLayoutsByTabId } + const layoutWithoutBuffers = { ...layout } + delete layoutWithoutBuffers.buffersByLeafId + terminalLayoutsByTabId[tabId] = layoutWithoutBuffers + continue + } + const repoId = getRepoIdFromWorktreeId(worktreeId) + const connectionId = connectionIdByRepoId.get(repoId) + if (connectionId) { + continue + } + if (!connectionIdByRepoId.has(repoId)) { + // Why: when the repo catalog does not know this repoId — either because + // it is not yet hydrated, or because the repo has been removed — we + // cannot classify the worktree as local vs SSH. Preserve the buffer + // until a later call with a hydrated catalog can decide. SSH buffers + // are the only authoritative scrollback source, so the cost of a wrong + // prune (lost remote scrollback) is higher than the cost of a wrong + // preserve (extra bytes persisted). + continue + } + } + + terminalLayoutsByTabId ??= { ...session.terminalLayoutsByTabId } + const layoutWithoutBuffers = { ...layout } + delete layoutWithoutBuffers.buffersByLeafId + terminalLayoutsByTabId[tabId] = layoutWithoutBuffers + } + + if (!terminalLayoutsByTabId) { + return session + } + + return { + ...session, + // Why: local daemon history/checkpoints are authoritative for restart + // scrollback. Keeping renderer-captured buffers for local tabs makes every + // persisted state write scale with old terminal output; SSH keeps them + // because relay teardown may leave no local history to cold-restore. + terminalLayoutsByTabId + } +} diff --git a/src/shared/worktree-id.test.ts b/src/shared/worktree-id.test.ts new file mode 100644 index 000000000..58e43322d --- /dev/null +++ b/src/shared/worktree-id.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest' +import { WORKTREE_ID_SEPARATOR, getRepoIdFromWorktreeId, splitWorktreeId } from './worktree-id' + +describe('WORKTREE_ID_SEPARATOR', () => { + it('is the literal "::" separator', () => { + expect(WORKTREE_ID_SEPARATOR).toBe('::') + }) +}) + +describe('getRepoIdFromWorktreeId', () => { + it('returns the repo id for a canonical worktree id', () => { + expect(getRepoIdFromWorktreeId('repo-123::/abs/path')).toBe('repo-123') + }) + + it('returns the whole input when there is no separator', () => { + expect(getRepoIdFromWorktreeId('just-a-repo-id')).toBe('just-a-repo-id') + }) + + it('returns the empty string for an empty input', () => { + expect(getRepoIdFromWorktreeId('')).toBe('') + }) + + it('returns an empty repo id for a bare separator', () => { + expect(getRepoIdFromWorktreeId('::')).toBe('') + }) + + it('returns an empty repo id for a leading separator', () => { + expect(getRepoIdFromWorktreeId('::path')).toBe('') + }) + + it('returns the repo id when only a trailing separator is present', () => { + expect(getRepoIdFromWorktreeId('repo::')).toBe('repo') + }) + + it('splits on the first separator when the path itself contains "::"', () => { + expect(getRepoIdFromWorktreeId('repo::a::b')).toBe('repo') + }) +}) + +describe('splitWorktreeId', () => { + it('splits a canonical worktree id into repo id and path', () => { + expect(splitWorktreeId('repo-123::/abs/path')).toEqual({ + repoId: 'repo-123', + worktreePath: '/abs/path' + }) + }) + + it('returns null when there is no separator', () => { + expect(splitWorktreeId('just-a-repo-id')).toBeNull() + }) + + it('returns null for an empty input', () => { + expect(splitWorktreeId('')).toBeNull() + }) + + it('returns empty repo id and empty path for a bare separator', () => { + expect(splitWorktreeId('::')).toEqual({ repoId: '', worktreePath: '' }) + }) + + it('returns an empty repo id when the separator is leading', () => { + expect(splitWorktreeId('::path')).toEqual({ repoId: '', worktreePath: 'path' }) + }) + + it('returns an empty path when the separator is trailing', () => { + expect(splitWorktreeId('repo::')).toEqual({ repoId: 'repo', worktreePath: '' }) + }) + + it('splits on the first separator when the path itself contains "::"', () => { + expect(splitWorktreeId('repo::a::b')).toEqual({ repoId: 'repo', worktreePath: 'a::b' }) + }) +}) diff --git a/src/shared/worktree-id.ts b/src/shared/worktree-id.ts new file mode 100644 index 000000000..1a5d68558 --- /dev/null +++ b/src/shared/worktree-id.ts @@ -0,0 +1,24 @@ +import { WORKTREE_ID_SEPARATOR } from './pty-session-id-format' + +export { WORKTREE_ID_SEPARATOR } from './pty-session-id-format' + +export type ParsedWorktreeId = { + repoId: string + worktreePath: string +} + +export function getRepoIdFromWorktreeId(worktreeId: string): string { + const separatorIdx = worktreeId.indexOf(WORKTREE_ID_SEPARATOR) + return separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx) +} + +export function splitWorktreeId(worktreeId: string): ParsedWorktreeId | null { + const separatorIdx = worktreeId.indexOf(WORKTREE_ID_SEPARATOR) + if (separatorIdx === -1) { + return null + } + return { + repoId: worktreeId.slice(0, separatorIdx), + worktreePath: worktreeId.slice(separatorIdx + WORKTREE_ID_SEPARATOR.length) + } +} diff --git a/tests/e2e/helpers/orca-restart.ts b/tests/e2e/helpers/orca-restart.ts index 6a7dc3ff8..7e8d40306 100644 --- a/tests/e2e/helpers/orca-restart.ts +++ b/tests/e2e/helpers/orca-restart.ts @@ -16,7 +16,7 @@ import { type TestInfo } from '@stablyai/playwright-test' import { execSync } from 'child_process' -import { existsSync, mkdtempSync, rmSync } from 'fs' +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'fs' import os from 'os' import path from 'path' @@ -61,6 +61,31 @@ export function createRestartSession(testInfo: TestInfo): RestartSession { const userDataDir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-restart-')) const headful = shouldLaunchHeadful(testInfo) + // Why: this helper bypasses the shared `electronApp` fixture, so it must + // seed the same dismissed onboarding state or the full-screen overlay covers + // both launches and obscures restart failures. + writeFileSync( + path.join(userDataDir, 'orca-data.json'), + `${JSON.stringify( + { + settings: { + telemetry: { + optedIn: true, + installId: '00000000-0000-4000-8000-000000000000', + existedBeforeTelemetryRelease: false + } + }, + onboarding: { + closedAt: 1, + outcome: 'completed', + lastCompletedStep: 4 + } + }, + null, + 2 + )}\n` + ) + const launch = async (): Promise => { const app = await electron.launch({ args: [mainPath], @@ -167,7 +192,7 @@ export async function attachRepoAndOpenTerminal(page: Page, repoPath: string): P ) .toBe(true) - const repoBasename = repoPath.split('/').filter(Boolean).pop() ?? '' + const repoBasename = path.basename(repoPath) const worktreeId = await page.evaluate((repoBasename: string) => { const store = window.__store if (!store) { @@ -180,7 +205,13 @@ export async function attachRepoAndOpenTerminal(page: Page, repoPath: string): P // and will not match this suffix. This gives us the primary deterministically // without depending on boolean fields on the worktree record. const primary = - allWorktrees.find((worktree) => worktree.path.endsWith(`/${repoBasename}`)) ?? allWorktrees[0] + allWorktrees.find( + (worktree) => + worktree.path + .split(/[\\/]+/) + .filter(Boolean) + .pop() === repoBasename + ) ?? allWorktrees[0] if (!primary) { return null } diff --git a/tests/e2e/helpers/terminal.ts b/tests/e2e/helpers/terminal.ts index bd6136ad4..70c3576b2 100644 --- a/tests/e2e/helpers/terminal.ts +++ b/tests/e2e/helpers/terminal.ts @@ -141,8 +141,10 @@ export async function discoverActivePtyId(page: Page): Promise { await page.evaluate( ({ marker, candidateIds }) => { - for (const id of candidateIds) { - window.api.pty.write(String(id), `\x03\x15echo ${marker}_${id}\r`) + // Why: daemon PTY IDs can contain path separators and shell metacharacters. + // Echo a numeric probe index, then map it back to the opaque ID in Node. + for (const [index, id] of candidateIds.entries()) { + window.api.pty.write(String(id), `\x03\x15echo ${marker}_${index}\r`) } }, { marker, candidateIds } @@ -156,7 +158,8 @@ export async function discoverActivePtyId(page: Page): Promise { const markerRe = new RegExp(`${marker}_(\\d+)`, 'g') const matches = [...content.matchAll(markerRe)] if (matches.length > 0) { - foundPtyId = matches.at(-1)?.[1] ?? null + const index = Number(matches.at(-1)?.[1] ?? Number.NaN) + foundPtyId = Number.isInteger(index) ? (candidateIds[index] ?? null) : null return true } return false diff --git a/tests/e2e/terminal-restart-persistence.spec.ts b/tests/e2e/terminal-restart-persistence.spec.ts index 87da9ab28..0e812d97d 100644 --- a/tests/e2e/terminal-restart-persistence.spec.ts +++ b/tests/e2e/terminal-restart-persistence.spec.ts @@ -7,11 +7,10 @@ * wouldn't lose in-session output. With many panes of accumulated output, * each tick blocked the renderer main thread for seconds, causing visible * input lag across the whole app. The periodic save was removed in favor - * of the out-of-process terminal daemon (PR #729). For users who don't - * opt into the daemon, the `beforeunload` save in App.tsx is now the - * *only* thing that preserves scrollback across a restart — this suite - * locks that behavior down so a future regression can't silently return - * us to "quit → empty terminal on relaunch." + * of the out-of-process terminal daemon (PR #729), and local renderer + * scrollback buffers are pruned from persisted workspace sessions. This + * suite locks down daemon-backed clean quit → relaunch so we don't silently + * return to "quit → empty terminal on relaunch." * * What it covers: * - Scrollback survives clean quit → relaunch (primary regression test). @@ -22,9 +21,7 @@ * * What it does NOT try to cover: * - Main-thread input-lag improvement — machine-dependent and flaky. - * - Crash/SIGKILL recovery — non-daemon users now intentionally lose - * in-session scrollback on unclean exit; that's the tradeoff the - * removed periodic save represented. + * - Crash/SIGKILL recovery — that is covered by daemon history checkpoints. */ import { readFileSync, existsSync } from 'fs' @@ -47,6 +44,7 @@ import { ensureTerminalVisible } from './helpers/store' import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart' +import { PTY_SESSION_ID_SEPARATOR } from '../../src/shared/pty-session-id-format' // Why: each test in this file does a full quit→relaunch cycle, which spawns // two Electron instances back-to-back. Running in serial keeps the isolated @@ -121,6 +119,10 @@ test.describe('Terminal restart persistence', () => { const firstLaunch = await session.launch() firstApp = firstLaunch.app const { worktreeId, ptyId } = await bootstrapFirstLaunch(firstLaunch.page, repoPath) + // Why: this spec validates the daemon-backed persistence path. If the + // daemon falls back to LocalPtyProvider, local buffers are intentionally + // pruned and the scrollback assertion would fail with the wrong signal. + expect(ptyId).toContain(PTY_SESSION_ID_SEPARATOR) // Why: the marker must be distinctive enough that it can't appear in the // restored prompt banner or a stray OSC sequence. The timestamp suffix @@ -130,9 +132,8 @@ test.describe('Terminal restart persistence', () => { await execInTerminal(firstLaunch.page, ptyId, `echo ${marker}`) await waitForTerminalOutput(firstLaunch.page, marker) - // Why: closing the app triggers beforeunload → session.setSync, which is - // the one remaining codepath that flushes serialized scrollback to disk - // for non-daemon users. This is the behavior the suite is guarding. + // Why: closing the app triggers the session save plus daemon disconnect. + // The session keeps the PTY binding while the daemon keeps the scrollback. await session.close(firstApp) firstApp = null @@ -141,10 +142,9 @@ test.describe('Terminal restart persistence', () => { secondApp = secondLaunch.app await bootstrapRestoredLaunch(secondLaunch.page, worktreeId) - // Why: buffer restore replays the serialized output through xterm.write - // during pane mount. Poll the live terminal content rather than hitting - // the store because the store only sees the raw saved buffer, whereas - // restoreScrollbackBuffers can strip alt-screen sequences before write. + // Why: daemon reattach replays its snapshot through xterm.write during + // pane mount. Poll the live terminal content, not the store, because the + // store intentionally no longer carries local scrollback buffers. await expect .poll(async () => (await getTerminalContent(secondLaunch.page)).includes(marker), { timeout: 15_000,