diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index cd73232acf7..7a962caff9c 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -186,6 +186,21 @@ export class DaemonPtyAdapter implements IPtyProvider { } if (this.historyManager && result.isNew) { + // Why (design doc §9 post-release monitoring): a spawn with an explicit + // sessionId that produced a brand-new daemon session AND has no disk + // history is the signal that either (a) the reaper ate a dir the user + // still wanted, or (b) the persisted mapping pointed at a session that + // never wrote a checkpoint. This shouldn't happen under the defensive + // keep-set derivation — if it starts showing up in logs, investigate + // before the next release. + if (opts.sessionId !== undefined && !restoreInfo) { + console.warn('[pty] cold-restore miss for explicit sessionId — no disk history found', { + sessionId, + worktreeId: opts.worktreeId, + tabId: opts.tabId, + leafId: opts.leafId + }) + } void this.historyManager .openSession(sessionId, { cwd: effectiveCwd ?? '', diff --git a/src/main/index.ts b/src/main/index.ts index d351fa2395e..fa8d9d824cb 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -550,12 +550,18 @@ app.whenReady().then(async () => { liveDaemonSessionIds.add(id) } } - await runOrphanHistoryReaper({ + const reapStats = await runOrphanHistoryReaper({ historyBasePath: getHistoryDir(), sidecarDir: app.getPath('userData'), workspaceSession: store.getWorkspaceSession(), liveDaemonSessionIds }) + // Why (design doc §9): log counts so post-release we can tell whether + // the reaper is firing at all and whether the three gates (keep set, + // mtime grace, quarantine) are in their expected ratios. Persistent + // non-zero `reaped` without corresponding shrinkage of history dirs in + // the wild would indicate a logic bug worth investigating. + console.log('[history-reaper] startup pass stats:', reapStats) } catch (error) { console.warn('[history-reaper] startup pass failed:', error) } diff --git a/tests/e2e/terminal-restart-persistence.spec.ts b/tests/e2e/terminal-restart-persistence.spec.ts index 87da9ab28d2..d85d9b0ca12 100644 --- a/tests/e2e/terminal-restart-persistence.spec.ts +++ b/tests/e2e/terminal-restart-persistence.spec.ts @@ -27,7 +27,8 @@ * removed periodic save represented. */ -import { readFileSync, existsSync } from 'fs' +import { readFileSync, writeFileSync, existsSync } from 'fs' +import { join } from 'path' import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { TEST_REPO_PATH_FILE } from './global-setup' @@ -227,6 +228,83 @@ test.describe('Terminal restart persistence', () => { } }) + test('scrollback survives quit with missing ptyId (Issue #217 regression)', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set. + {}, testInfo) => { + // Why (design doc §7.2): this is the load-bearing test for the + // deterministic-sessionId fix. Issue #217 failed specifically when + // orca-data.json's `ptyId` field was empty at restore time (the + // SIGKILL-between-spawn-and-persist window). Before the fix, the + // renderer had no way to reach the on-disk history dir without that + // field. After the fix, mintPtySessionId(worktreeId, tabId, leafId) + // reproduces the same id, so the cold-restore reader finds the dir + // anyway. We simulate the race here by mutating the persisted + // orca-data.json between launches — deleting the ptyId field gives + // us the exact state the old code couldn't recover from, without + // the flakiness of racing a real SIGKILL. + const repoPath = readFileSync(TEST_REPO_PATH_FILE, 'utf-8').trim() + if (!repoPath || !existsSync(repoPath)) { + test.skip(true, 'Global setup did not produce a seeded test repo') + return + } + + const session = createRestartSession(testInfo) + let firstApp: ElectronApplication | null = null + let secondApp: ElectronApplication | null = null + + try { + // ── First launch: produce scrollback, let clean quit flush state ── + const firstLaunch = await session.launch() + firstApp = firstLaunch.app + const { worktreeId, ptyId } = await bootstrapFirstLaunch(firstLaunch.page, repoPath) + + const marker = `DETERMINISTIC_RESTORE_${Date.now()}` + await execInTerminal(firstLaunch.page, ptyId, `echo ${marker}`) + await waitForTerminalOutput(firstLaunch.page, marker) + + await session.close(firstApp) + firstApp = null + + // Simulate the SIGKILL-before-persist race: strip ptyId from every + // tab in the persisted session. The terminal-history/ dir stays on + // disk (it was written by the daemon's checkpoint tick, not by + // orca-data.json). Pre-fix code would fail to find it; post-fix + // code re-derives the id from (worktreeId, tabId, leafId) and + // hits the same directory. + const dataFile = join(session.userDataDir, 'orca-data.json') + const data = JSON.parse(readFileSync(dataFile, 'utf-8')) as { + workspaceSession?: { + tabsByWorktree?: Record + } + } + const tabs = data.workspaceSession?.tabsByWorktree?.[worktreeId] ?? [] + for (const tab of tabs) { + tab.ptyId = null + } + writeFileSync(dataFile, JSON.stringify(data)) + + // ── Second launch: deterministic path must recover scrollback ──── + const secondLaunch = await session.launch() + secondApp = secondLaunch.app + await bootstrapRestoredLaunch(secondLaunch.page, worktreeId) + + await expect + .poll(async () => (await getTerminalContent(secondLaunch.page)).includes(marker), { + timeout: 15_000, + message: + 'Deterministic-sessionId restore did not recover scrollback when ptyId was missing' + }) + .toBe(true) + } finally { + if (secondApp) { + await session.close(secondApp) + } + if (firstApp) { + await session.close(firstApp) + } + session.dispose() + } + }) + test('idle session does not spam session.set writes', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set. {}, testInfo) => { const repoPath = readFileSync(TEST_REPO_PATH_FILE, 'utf-8').trim()