feat(pty): post-release monitoring + Issue #217 e2e regression test

Why (design doc §9): surface the signal that matters after shipping
the deterministic-sessionId fix. Two log sites added:

- daemon-pty-adapter.ts: warn when a spawn with an explicit sessionId
  produced a new daemon session AND no disk history was found — that
  means either the reaper ate the dir, or the persisted mapping
  pointed at a session that never wrote a checkpoint. Shouldn't happen
  under the defensive keep-set derivation; worth watching.

- main/index.ts: log the reaper's ReapStats at each startup pass so
  we can tell whether it's firing at all and whether the three gates
  (keep set, mtime grace, quarantine) are in expected ratios.

E2E test added (§7.2): Issue #217's load-bearing scenario — scrollback
survives a quit where the persisted orca-data.json lacks `ptyId`. We
simulate the SIGKILL-before-persist race by mutating orca-data.json
between launches (delete ptyId, leave terminal-history/ dir intact).
Pre-fix code couldn't reach the dir; post-fix code re-derives the id
via mintPtySessionId(worktreeId, tabId, leafId) and cold-restores.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-05-04 16:24:23 -04:00
co-authored by Orca
parent 67df244a0d
commit a088f481ba
3 changed files with 101 additions and 2 deletions
+15
View File
@@ -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 ?? '',
+7 -1
View File
@@ -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)
}
+79 -1
View File
@@ -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<string, { ptyId: string | null }[]>
}
}
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()