diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index e32415aa9..af7756e45 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -6,6 +6,7 @@ import { mkdtempSync, mkdirSync, rmSync, existsSync, readFileSync, writeFileSync import { DaemonPtyAdapter } from './daemon-pty-adapter' import { DaemonServer } from './daemon-server' import { getHistorySessionDirName } from './history-paths' +import type { HistoryReader } from './history-reader' import type { SubprocessHandle } from './session' import type * as DaemonHealthModule from './daemon-health' import { getDaemonSocketPath } from './daemon-spawner' @@ -1239,6 +1240,159 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(internals.lastFullCheckpointAt.has(sessionId)).toBe(false) }) + it('skips the cold-restore replay when the daemon session is still alive', async () => { + const sessionId = 'warm-reattach-skip-replay' + const first = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + await first.spawn({ cols: 80, rows: 24, cwd: '/home/user', sessionId }) + // Why disconnectOnly: the production app-quit path leaves meta.endedAt + // null so the session stays crash-recoverable — the state every app + // relaunch with a live daemon sees. + await first.disconnectOnly() + + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const reader = (historyAdapter as unknown as { historyReader: HistoryReader }).historyReader + const detectSpy = vi.spyOn(reader, 'detectColdRestore') + const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId }) + + expect(result.isReattach).toBe(true) + expect(result.coldRestore).toBeUndefined() + expect(detectSpy).not.toHaveBeenCalled() + // The unmanaged-reattach re-anchor must survive the skipped detect. + const internals = historyAdapter as unknown as { + sessionsNeedingFullCheckpoint: Set + lastFullCheckpointAt: Map + } + expect(internals.sessionsNeedingFullCheckpoint.has(sessionId)).toBe(true) + expect(internals.lastFullCheckpointAt.has(sessionId)).toBe(false) + }) + + it('does not probe session aliveness when there is no restorable history', async () => { + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const client = ( + historyAdapter as unknown as { + client: { request: (type: string, payload?: unknown) => Promise } + } + ).client + const requestSpy = vi.spyOn(client, 'request') + + await historyAdapter.spawn({ cols: 80, rows: 24, sessionId: 'fresh-no-history' }) + + expect(requestSpy.mock.calls.map((call) => call[0])).not.toContain('getSize') + }) + + it('recovers cold restore when the probed session dies before createOrAttach', async () => { + const sessionId = 'probe-race-cold-restore' + const sessionDir = join(historyDir, getHistorySessionDirName(sessionId)) + mkdirSync(sessionDir, { recursive: true }) + writeFileSync( + join(sessionDir, 'meta.json'), + JSON.stringify({ + cwd: '/projects/raced', + cols: 100, + rows: 30, + startedAt: '2026-04-15T10:00:00Z', + endedAt: null, + exitCode: null + }) + ) + writeFileSync(join(sessionDir, 'scrollback.bin'), 'raced output\r\n') + + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const client = ( + historyAdapter as unknown as { + client: { request: (type: string, payload?: unknown) => Promise } + } + ).client + const originalRequest = client.request.bind(client) + // Why: simulates the probe→createOrAttach race — the probe sees the + // session alive, but it is gone by the time createOrAttach runs. The + // meta rewrite mimics the dying session's exit event beating the + // createOrAttach reply and writing endedAt via closeSession; the + // fallback detect must still restore instead of falling through to + // openSession (which would delete the checkpoint). + vi.spyOn(client, 'request').mockImplementation(async (type: string, payload?: unknown) => { + if (type === 'getSize') { + return { size: { cols: 100, rows: 30 } } + } + const response = await originalRequest(type, payload) + if (type === 'createOrAttach') { + writeFileSync( + join(sessionDir, 'meta.json'), + JSON.stringify({ + cwd: '/projects/raced', + cols: 100, + rows: 30, + startedAt: '2026-04-15T10:00:00Z', + endedAt: '2026-04-15T10:05:00Z', + exitCode: 0 + }) + ) + } + return response + }) + + const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId }) + + expect(result.coldRestore).toBeDefined() + expect(result.coldRestore!.scrollback).toContain('raced output') + // Documented race delta: the fresh shell spawns with the renderer's + // requested params, not the recovered ones. + expect(lastSpawnOpts).toMatchObject({ sessionId, cols: 80, rows: 24 }) + // The recovery data must survive — openSession would have deleted it. + expect(existsSync(join(sessionDir, 'scrollback.bin'))).toBe(true) + const internals = historyAdapter as unknown as { + sessionsNeedingFullCheckpoint: Set + lastFullCheckpointAt: Map + } + expect(internals.sessionsNeedingFullCheckpoint.has(sessionId)).toBe(true) + expect(internals.lastFullCheckpointAt.has(sessionId)).toBe(false) + }) + + it('falls back to the full cold-restore detect when the aliveness probe fails', async () => { + const sessionId = 'probe-error-cold-restore' + const sessionDir = join(historyDir, getHistorySessionDirName(sessionId)) + mkdirSync(sessionDir, { recursive: true }) + writeFileSync( + join(sessionDir, 'meta.json'), + JSON.stringify({ + cwd: '/projects/probeless', + cols: 132, + rows: 43, + startedAt: '2026-04-15T10:00:00Z', + endedAt: null, + exitCode: null + }) + ) + writeFileSync(join(sessionDir, 'scrollback.bin'), 'probeless output\r\n') + + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const client = ( + historyAdapter as unknown as { + client: { request: (type: string, payload?: unknown) => Promise } + } + ).client + const originalRequest = client.request.bind(client) + // Why: an old daemon rejects the unknown getSize method; the spawn must + // behave exactly like the unprobed path. + vi.spyOn(client, 'request').mockImplementation((type: string, payload?: unknown) => { + if (type === 'getSize') { + return Promise.reject(new Error('Unknown request type')) + } + return originalRequest(type, payload) + }) + + const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId }) + + expect(result.coldRestore).toBeDefined() + expect(result.coldRestore!.scrollback).toContain('probeless output') + expect(lastSpawnOpts).toMatchObject({ + sessionId, + cwd: '/projects/probeless', + cols: 132, + rows: 43 + }) + }) + it('returns same cold restore on StrictMode double-mount (sticky cache)', async () => { const sessionId = 'sticky-cache-test' const sessionDir = join(historyDir, getHistorySessionDirName(sessionId)) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index f4d717f19..a649abebc 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -6,7 +6,7 @@ import { existsSync } from 'node:fs' import { DaemonClient } from './client' import { getMacDaemonSystemResolverHealth } from './daemon-health' import { HistoryManager } from './history-manager' -import { HistoryReader } from './history-reader' +import { HistoryReader, type ColdRestoreInfo } from './history-reader' import { mintPtySessionId, parsePtySessionId } from './pty-session-id' import { supportsPtyStartupBarrier } from './shell-ready' import { CODEX_SHELL_READY_TIMEOUT_MS } from './session' @@ -141,16 +141,30 @@ export class DaemonPtyAdapter implements IPtyProvider { await this.replaceUnhealthyMacResolverDaemonBeforeNewPty() } + await this.ensureConnected() + // Why: detect crash-recovery history before spawning a replacement PTY so // the revived shell inherits the recovered cwd and dimensions instead of // whatever the current renderer happened to request on mount. - const restoreInfo = this.historyReader?.detectColdRestore(sessionId) ?? null + // Why probe aliveness first: detectColdRestore synchronously replays the + // full checkpoint + log (up to ~5MB) through a scratch emulator on the + // main process, but a live daemon session ignores spawn params and its + // own snapshot supersedes disk — the replay result would be discarded. + // getSize is a read-only probe; on error/unsupported it degrades to the + // full detect. + let restoreInfo: ColdRestoreInfo | null = null + let restoreSkippedForLiveSession = false + if (this.historyReader?.hasRestorableHistory(sessionId)) { + if ((await this.getAppliedSize(sessionId)) !== null) { + restoreSkippedForLiveSession = true + } else { + restoreInfo = this.historyReader.detectColdRestore(sessionId) + } + } const effectiveCwd = restoreInfo?.cwd ?? opts.cwd const effectiveCols = restoreInfo?.cols ?? opts.cols const effectiveRows = restoreInfo?.rows ?? opts.rows - await this.ensureConnected() - const shellReadySupported = opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false const isCodexStartupCommand = recognizeAgentProcessFromCommandLine(opts.command)?.agent === 'codex' @@ -208,6 +222,19 @@ export class DaemonPtyAdapter implements IPtyProvider { } } + // Why: the probe→createOrAttach gap is racy — the session can exit (or + // enter termination) in between, so the daemon spawned a fresh shell. + // Detect now so scrollback restore matches the unprobed path; only the + // new shell's cwd/dims came from the renderer request in this rare case. + // Why ignoreCleanEnd: the raced session's exit event (stream socket) can + // beat the createOrAttach reply and write endedAt via closeSession; that + // must not null the restore here, or the openSession branch below would + // delete the checkpoint instead of restoring it. + if (result.isNew && restoreSkippedForLiveSession) { + restoreInfo = + this.historyReader?.detectColdRestore(sessionId, { ignoreCleanEnd: true }) ?? null + } + const wasAlreadyManaged = this.activeSessionIds.has(sessionId) this.activeSessionIds.add(sessionId) diff --git a/src/main/daemon/history-reader.ts b/src/main/daemon/history-reader.ts index 2914ed21a..f6661b500 100644 --- a/src/main/daemon/history-reader.ts +++ b/src/main/daemon/history-reader.ts @@ -28,12 +28,28 @@ export class HistoryReader { this.basePath = basePath } - detectColdRestore(sessionId: string): ColdRestoreInfo | null { + // Why: spawn needs a cheap "could this cold-restore?" predicate before + // deciding to pay detectColdRestore's full checkpoint+log replay. Reads only + // the small meta.json, using the same unclean-shutdown test detectColdRestore + // starts with. + hasRestorableHistory(sessionId: string): boolean { + const meta = this.readMeta(sessionId) + return meta !== null && meta.endedAt === null + } + + detectColdRestore( + sessionId: string, + opts?: { ignoreCleanEnd?: boolean } + ): ColdRestoreInfo | null { const meta = this.readMeta(sessionId) if (!meta) { return null } - if (meta.endedAt !== null) { + // Why ignoreCleanEnd: in the spawn probe race, the dying session's exit + // event can write endedAt between the aliveness probe and the post-spawn + // fallback detect. The caller established restore eligibility before the + // probe, so the just-written clean end must not downgrade the restore. + if (meta.endedAt !== null && !opts?.ignoreCleanEnd) { return null }