diff --git a/.github/workflows/computer-e2e.yml b/.github/workflows/computer-e2e.yml index 218ff6a6c..58cdbb36d 100644 --- a/.github/workflows/computer-e2e.yml +++ b/.github/workflows/computer-e2e.yml @@ -11,6 +11,7 @@ on: - 'config/scripts/computer-use-smoke.mjs' - 'config/scripts/computer-use-smoke.test.mjs' - 'config/scripts/daemon-boot-smoke.mjs' + - 'config/scripts/windows-daemon-workspace-close-repro.mjs' - 'config/scripts/verify-computer-native.mjs' # Why: the native-smoke job boots the built terminal daemon under plain # Node, so any change to the daemon bundle graph or the main build must @@ -111,6 +112,11 @@ jobs: # the PR when the built daemon cannot start on ubuntu-22.04 / windows. - name: Daemon boot smoke run: node config/scripts/daemon-boot-smoke.mjs + # Why: workspace removal overlaps graceful renderer teardown with the + # forced main-process sweep; exercise that exact pair against real ConPTY. + - name: Windows daemon workspace-close repro + if: runner.os == 'Windows' + run: node config/scripts/windows-daemon-workspace-close-repro.mjs - if: runner.os == 'Linux' env: ORCA_COMPUTER_E2E: '1' diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 3b8f53d04..309404761 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -969,7 +969,7 @@ "id": "terminal-platform.windows-conpty-liveness", "title": "Windows ConPTY terminals stay input-live, render-live, and geometry-live", "maturity": "experimental", - "protection": "none", + "protection": "partial", "owner": "terminal-platform", "layer": "windows-electron-conpty", "surfaces": [ @@ -987,34 +987,77 @@ "daemon", "wsl" ], - "coveredPlatforms": [], - "coveredProviders": [], - "coverageNotes": "Registered gap only; no executable coverage is wired yet.", + "coveredPlatforms": [ + "windows" + ], + "coveredProviders": [ + "daemon" + ], + "coverageNotes": "Issue #8048 now has deterministic wrapper and cold-restore re-anchor tests plus a Windows PR-CI harness that drives the built daemon through 25 real ConPTY workspace-close races while an unrelated witness PTY stays alive. Keyboard reset, CJK repaint, WSL, and full visible Electron coverage remain gaps.", "motivatingLinks": [ "https://github.com/stablyai/orca/pull/6541", "https://github.com/stablyai/orca/pull/6858", "https://github.com/stablyai/orca/pull/6876", "https://github.com/stablyai/orca/pull/6968", "https://github.com/stablyai/orca/pull/6970", - "https://github.com/stablyai/orca/pull/6999" + "https://github.com/stablyai/orca/pull/6999", + "https://github.com/stablyai/orca/issues/8048" + ], + "invariant": "Windows local and daemon terminals must spawn with the intended shell, survive overlapping graceful/forced workspace teardown without affecting unrelated PTYs, retain recovered scrollback across the fresh daemon's first checkpoint, accept normal Enter/Backspace/Arrow input after agent or TUI exit, render cursor/CJK/wide-glyph redraws without stale cells, and converge to nonzero applied size.", + "oracle": "The issue #8048 slice asserts one node-pty ConPTY close for a graceful-then-force sequence, atomically seeds recovered history before fresh shell output and re-anchoring, preserves recovery after seed failure plus adapter restart, and runs 25 built-daemon close races while checking victim session/PID reaping, a stable daemon PID, and a live witness PTY. A broader Windows live gate still needs shell input, resize, cursor, and CJK/wide-glyph pixel evidence.", + "commands": [ + "pnpm vitest run src/main/daemon/pty-subprocess.test.ts src/main/daemon/daemon-pty-adapter.test.ts", + "pnpm build:electron-vite && node config/scripts/windows-daemon-workspace-close-repro.mjs", + "node config/scripts/windows-daemon-workspace-close-repro.mjs" + ], + "testFiles": [ + "src/main/daemon/pty-subprocess.test.ts", + "src/main/daemon/daemon-pty-adapter.test.ts", + "config/scripts/windows-daemon-workspace-close-repro.mjs" + ], + "assertionRefs": [ + { + "file": "src/main/daemon/pty-subprocess.test.ts", + "assertions": [ + "graceful kill followed by force and dispose invokes Windows node-pty kill exactly once and never retries the dead child PID" + ] + }, + { + "file": "src/main/daemon/daemon-pty-adapter.test.ts", + "assertions": [ + "the first checkpoint orders recovered scrollback before synchronously emitted fresh-shell startup output", + "a failed atomic history seed remains non-authoritative across adapter restart and cannot overwrite the recovery files" + ] + }, + { + "file": "config/scripts/windows-daemon-workspace-close-repro.mjs", + "assertions": [ + "all 25 victim sessions and OS PIDs are reaped while the built daemon PID and an unrelated witness PowerShell remain alive" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-07-10", + "runner": "local", + "platform": "windows", + "command": "node config/scripts/windows-daemon-workspace-close-repro.mjs", + "result": "passed", + "durationSeconds": 7.6, + "summary": "All 25 victim sessions and OS PIDs were reaped while the built daemon and witness PTY survived the real ConPTY workspace-close races. The double-close, history ordering, and seed-failure restart regressions produced intentional red failures before their fixes and passed afterward." + } ], - "invariant": "Windows local and daemon terminals must spawn with the intended shell, accept normal Enter/Backspace/Arrow input after agent or TUI exit, render cursor/CJK/wide-glyph redraws without stale cells, and converge to nonzero applied size.", - "oracle": "A Windows live gate proves shell echo, command submission, keyboard reset, resize readback, cursor visibility, and CJK/wide-glyph redraw using PTY input logs plus visible buffer/pixel evidence.", - "commands": [], - "testFiles": [], - "assertionRefs": [], - "evidenceRuns": [], "runtimeBudget": { "p95Seconds": 90, "scope": "Windows focused Electron ConPTY gate" }, "flakeHistory": { - "status": "not-started", - "evidence": "Windows live E2E remains a known weak area; gate must soak before blocking." + "status": "unknown", + "evidence": "The built-daemon issue #8048 harness passed locally once and is wired into Windows PR CI; it needs repeated CI history before promotion." }, "redGreenEvidence": { - "status": "missing", - "evidence": "Needs proof against stale keyboard-protocol mode, shell resolution failure, dropped resize, and CJK repaint regression." + "status": "partial", + "evidence": "The ConPTY double-close and cold-restore re-anchor assertions were each observed failing before the fix and passing afterward. Keyboard protocol, shell resolution, resize, and CJK repaint still need red/green proof." }, "performanceBudget": { "required": true, @@ -1026,8 +1069,8 @@ "Split shell parity, keyboard reset, and CJK repaint into smaller gates if a combined gate is flaky." ], "knownGaps": [ - "No manifest command yet.", - "Real IME composition may require a separate lower-layer/native-text-forwarding gate." + "Real IME composition may require a separate lower-layer/native-text-forwarding gate.", + "The built-daemon harness proves process/session liveness but not renderer pixels; visible shell input, resize, cursor, and CJK repaint remain uncovered." ], "demotionRule": "Cannot promote while Windows E2E is flaky, silently skipped, or screenshot-only." }, diff --git a/config/scripts/computer-e2e-workflow.test.mjs b/config/scripts/computer-e2e-workflow.test.mjs index 3d4fbaecd..3ef415832 100644 --- a/config/scripts/computer-e2e-workflow.test.mjs +++ b/config/scripts/computer-e2e-workflow.test.mjs @@ -127,6 +127,23 @@ describe('computer-use e2e workflow', () => { expect(daemonSmokeIndex).toBeGreaterThan(buildIndex) }) + it('runs the Windows workspace-close daemon repro after the main build', () => { + const workflow = parse( + readFileSync(join(projectDir, '.github/workflows/computer-e2e.yml'), 'utf8') + ) + const steps = workflow.jobs['native-smoke'].steps + const buildIndex = steps.findIndex((step) => step.run === 'pnpm build:electron-vite') + const reproIndex = steps.findIndex( + (step) => step.run === 'node config/scripts/windows-daemon-workspace-close-repro.mjs' + ) + + expect(reproIndex).toBeGreaterThan(buildIndex) + expect(steps[reproIndex].if).toBe("runner.os == 'Windows'") + expect(workflow.on.pull_request.paths).toContain( + 'config/scripts/windows-daemon-workspace-close-repro.mjs' + ) + }) + it('re-runs the native-smoke job when the daemon bundle graph changes', () => { const workflow = parse( readFileSync(join(projectDir, '.github/workflows/computer-e2e.yml'), 'utf8') @@ -136,6 +153,7 @@ describe('computer-use e2e workflow', () => { expect(triggerPaths).toEqual( expect.arrayContaining([ 'config/scripts/daemon-boot-smoke.mjs', + 'config/scripts/windows-daemon-workspace-close-repro.mjs', 'electron.vite.config.ts', 'build-plugins/**', 'src/main/daemon/**' diff --git a/config/scripts/windows-daemon-workspace-close-repro.mjs b/config/scripts/windows-daemon-workspace-close-repro.mjs new file mode 100644 index 000000000..be920ca71 --- /dev/null +++ b/config/scripts/windows-daemon-workspace-close-repro.mjs @@ -0,0 +1,271 @@ +/** + * Reproduces issue #8048 against the built daemon on Windows. + * + * A witness PowerShell stays alive while victim sessions receive the same + * graceful-then-immediate kill pair emitted when Orca closes a workspace. + * The daemon PID and witness session must survive every iteration. + */ +import { fork } from 'node:child_process' +import { randomUUID } from 'node:crypto' +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { connect } from 'node:net' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' + +const projectDir = resolve(import.meta.dirname, '../..') +const entryPath = join(projectDir, 'out', 'main', 'daemon-entry.js') +const iterations = Number(process.env.ORCA_WINDOWS_DAEMON_CLOSE_ITERATIONS ?? 25) +const requestTimeoutMs = 15_000 + +function log(message) { + process.stdout.write(`[windows-daemon-workspace-close] ${message}\n`) +} + +function readProtocolVersion() { + const source = readFileSync(join(projectDir, 'src/main/daemon/types.ts'), 'utf8') + const match = source.match(/PROTOCOL_VERSION\s*=\s*(\d+)/) + if (!match) { + throw new Error('Could not read the daemon protocol version') + } + return Number(match[1]) +} + +function createRpcClient(socketPath, tokenPath) { + const socket = connect(socketPath) + const pending = new Map() + let buffer = '' + let requestId = 0 + let helloResolve + let helloReject + const hello = new Promise((resolveHello, rejectHello) => { + helloResolve = resolveHello + helloReject = rejectHello + }) + + const rejectPending = (error) => { + helloReject(error) + for (const { reject, timer } of pending.values()) { + clearTimeout(timer) + reject(error) + } + pending.clear() + } + + socket.on('error', rejectPending) + socket.on('close', () => rejectPending(new Error('Daemon control socket closed'))) + socket.on('data', (chunk) => { + buffer += chunk.toString('utf8') + let newline = buffer.indexOf('\n') + while (newline !== -1) { + const line = buffer.slice(0, newline) + buffer = buffer.slice(newline + 1) + const message = JSON.parse(line) + if (message.type === 'hello') { + if (message.ok) { + helloResolve() + } else { + helloReject(new Error(message.error ?? 'Daemon rejected hello')) + } + } else if (message.id) { + const request = pending.get(message.id) + if (request) { + pending.delete(message.id) + clearTimeout(request.timer) + if (message.ok) { + request.resolve(message.payload) + } else { + request.reject(new Error(message.error ?? 'Daemon request failed')) + } + } + } + newline = buffer.indexOf('\n') + } + }) + + const connected = new Promise((resolveConnected, rejectConnected) => { + socket.once('connect', resolveConnected) + socket.once('error', rejectConnected) + }).then(() => { + socket.write( + `${JSON.stringify({ + type: 'hello', + version: readProtocolVersion(), + token: readFileSync(tokenPath, 'utf8').trim(), + clientId: randomUUID(), + role: 'control' + })}\n` + ) + return hello + }) + + return { + async request(type, payload) { + await connected + const id = `repro-${++requestId}` + return new Promise((resolveRequest, rejectRequest) => { + const timer = setTimeout(() => { + pending.delete(id) + rejectRequest(new Error(`Daemon request ${type} timed out`)) + }, requestTimeoutMs) + pending.set(id, { resolve: resolveRequest, reject: rejectRequest, timer }) + socket.write(`${JSON.stringify({ id, type, ...(payload ? { payload } : {}) })}\n`) + }) + }, + close() { + socket.destroy() + } + } +} + +function waitForReady(child, stderr) { + return new Promise((resolveReady, rejectReady) => { + const timer = setTimeout( + () => rejectReady(new Error(`Daemon readiness timed out.\n${stderr()}`)), + requestTimeoutMs + ) + child.on('message', (message) => { + if (message?.type === 'ready') { + clearTimeout(timer) + resolveReady() + } + }) + child.once('exit', (code, signal) => { + clearTimeout(timer) + rejectReady( + new Error(`Daemon exited before readiness (code=${code}, signal=${signal}).\n${stderr()}`) + ) + }) + }) +} + +async function stopChild(child) { + if (child.exitCode !== null || child.signalCode !== null) { + return + } + const exited = new Promise((resolveExit) => child.once('exit', resolveExit)) + child.kill('SIGTERM') + await Promise.race([ + exited, + new Promise((resolveTimeout) => + setTimeout(() => { + if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + } + resolveTimeout() + }, 5_000) + ) + ]) +} + +function isProcessAlive(pid) { + try { + process.kill(pid, 0) + return true + } catch (error) { + if (error?.code === 'ESRCH') { + return false + } + throw error + } +} + +async function waitForVictimExit(rpc, sessionId, pid) { + const deadline = Date.now() + requestTimeoutMs + while (Date.now() < deadline) { + const { sessions } = await rpc.request('listSessions') + const sessionAlive = sessions.some((session) => session.sessionId === sessionId) + if (!sessionAlive && !isProcessAlive(pid)) { + return sessions + } + await new Promise((resolveDelay) => setTimeout(resolveDelay, 25)) + } + throw new Error(`Victim ${sessionId} or OS pid ${pid} was not reaped`) +} + +async function main() { + if (process.platform !== 'win32') { + log('SKIP: Windows ConPTY is required') + return + } + if (!existsSync(entryPath)) { + throw new Error(`Missing ${entryPath}; run pnpm build:electron-vite first`) + } + + const scratch = mkdtempSync(join(tmpdir(), 'orca-windows-daemon-close-')) + const socketPath = `\\\\.\\pipe\\orca-daemon-close-${process.pid}-${randomUUID()}` + const tokenPath = join(scratch, 'daemon.token') + const daemonLogPath = join(scratch, 'daemon.log') + const child = fork( + entryPath, + ['--socket', socketPath, '--token', tokenPath, '--log-file', daemonLogPath], + { + stdio: ['ignore', 'ignore', 'pipe', 'ipc'], + windowsHide: true, + env: { ...process.env, ORCA_USER_DATA_PATH: scratch } + } + ) + const daemonPid = child.pid + let stderr = '' + child.stderr?.on('data', (chunk) => { + stderr += chunk.toString('utf8') + }) + let rpc + + try { + await waitForReady(child, () => stderr) + rpc = createRpcClient(socketPath, tokenPath) + const witnessId = `repro-witness@@${randomUUID().slice(0, 8)}` + await rpc.request('createOrAttach', { + sessionId: witnessId, + cols: 80, + rows: 24, + cwd: projectDir, + shellOverride: 'powershell.exe' + }) + + for (let index = 0; index < iterations; index += 1) { + const victimId = `repro-victim-${index}@@${randomUUID().slice(0, 8)}` + const victim = await rpc.request('createOrAttach', { + sessionId: victimId, + cols: 80, + rows: 24, + cwd: projectDir, + shellOverride: 'powershell.exe' + }) + if (!Number.isInteger(victim.pid) || victim.pid <= 0) { + throw new Error(`Victim ${victimId} did not return a valid OS pid`) + } + + // Why: sending both RPCs before awaiting either preserves the renderer + // unmount/worktree-sweep overlap that produced issue #8048. + const graceful = rpc.request('kill', { sessionId: victimId, immediate: false }) + const forced = rpc.request('kill', { sessionId: victimId, immediate: true }) + await Promise.all([graceful, forced]) + + const sessions = await waitForVictimExit(rpc, victimId, victim.pid) + if (child.pid !== daemonPid || child.exitCode !== null) { + throw new Error(`Daemon PID ${daemonPid} exited while closing victim ${index}`) + } + if (!sessions.some((session) => session.sessionId === witnessId && session.isAlive)) { + throw new Error(`Witness PTY disappeared while closing victim ${index}`) + } + } + + await rpc.request('kill', { sessionId: witnessId, immediate: true }) + log( + `PASS: ${iterations} victim sessions/PIDs were reaped while daemon ${daemonPid} and the witness PTY survived` + ) + } catch (error) { + const daemonLog = existsSync(daemonLogPath) ? readFileSync(daemonLogPath, 'utf8') : '' + throw new Error(`${error.message}\nstderr:\n${stderr}\ndaemon.log:\n${daemonLog}`) + } finally { + rpc?.close() + await stopChild(child) + rmSync(scratch, { recursive: true, force: true }) + } +} + +main().catch((error) => { + process.stderr.write(`[windows-daemon-workspace-close] FAIL: ${error.message}\n`) + process.exitCode = 1 +}) diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index af7756e45..f369980bc 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -5,6 +5,7 @@ import { join } from 'node:path' import { mkdtempSync, mkdirSync, rmSync, existsSync, readFileSync, writeFileSync } from 'node:fs' import { DaemonPtyAdapter } from './daemon-pty-adapter' import { DaemonServer } from './daemon-server' +import { HeadlessEmulator } from './headless-emulator' import { getHistorySessionDirName } from './history-paths' import type { HistoryReader } from './history-reader' import type { SubprocessHandle } from './session' @@ -29,7 +30,7 @@ function createTestDir(): string { return mkdtempSync(join(tmpdir(), 'daemon-adapter-test-')) } -function createMockSubprocess(): SubprocessHandle & { +function createMockSubprocess(dataOnSubscribe?: string): SubprocessHandle & { _simulateData: (data: string) => void _simulateExit: (code: number) => void } { @@ -47,6 +48,9 @@ function createMockSubprocess(): SubprocessHandle & { signal: vi.fn(), onData(cb) { onDataCb = cb + if (dataOnSubscribe) { + cb(dataOnSubscribe) + } }, onExit(cb) { onExitCb = cb @@ -86,8 +90,10 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { env?: Record command?: string } | null + let subprocessDataOnSubscribe: string | undefined beforeEach(async () => { + subprocessDataOnSubscribe = undefined dir = createTestDir() socketPath = getDaemonSocketPath(dir) tokenPath = join(dir, 'test.token') @@ -97,7 +103,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { tokenPath, spawnSubprocess: (opts) => { lastSpawnOpts = opts - lastSubprocess = createMockSubprocess() + lastSubprocess = createMockSubprocess(subprocessDataOnSubscribe) return lastSubprocess } }) @@ -938,7 +944,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(existsSync(join(historyDir, getHistorySessionDirName(id)))).toBe(true) }) - it('persists final take records that are not represented in the snapshot', async () => { + itOnPosix('persists final take records that are not represented in the snapshot', async () => { historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) const { id } = await historyAdapter.spawn({ @@ -1335,9 +1341,9 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { 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 unseeded race winner is replaced before exposure, so the retained + // shell uses the recovered dimensions as well as the recovered history. + expect(lastSpawnOpts).toMatchObject({ sessionId, cols: 100, rows: 30 }) // The recovery data must survive — openSession would have deleted it. expect(existsSync(join(sessionDir, 'scrollback.bin'))).toBe(true) const internals = historyAdapter as unknown as { @@ -1520,6 +1526,87 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(meta.rows).toBe(24) }) + it('keeps recovered scrollback when the fresh daemon session re-anchors history', async () => { + const sessionId = 'cold-restore-reanchor' + const sessionDir = join(historyDir, getHistorySessionDirName(sessionId)) + mkdirSync(sessionDir, { recursive: true }) + writeFileSync( + join(sessionDir, 'meta.json'), + JSON.stringify({ + cwd: '/tmp', + cols: 80, + rows: 24, + startedAt: '2026-07-10T08:00:00Z', + endedAt: null, + exitCode: null + }) + ) + writeFileSync(join(sessionDir, 'scrollback.bin'), 'recovered marker\r\n') + subprocessDataOnSubscribe = 'fresh shell output\r\n' + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId }) + expect(result.coldRestore?.scrollback).toContain('recovered marker') + + const internals = historyAdapter as unknown as { + checkpointSessions(sessionIds: Iterable): Promise> + } + await internals.checkpointSessions([sessionId]) + const checkpointPath = join(sessionDir, 'checkpoint.json') + const checkpoint = JSON.parse(readFileSync(checkpointPath, 'utf8')) + + expect(checkpoint.snapshotAnsi).toContain('recovered marker') + expect(checkpoint.snapshotAnsi).toContain('fresh shell output') + expect(checkpoint.snapshotAnsi.indexOf('recovered marker')).toBeLessThan( + checkpoint.snapshotAnsi.indexOf('fresh shell output') + ) + }) + + it('keeps recovery persistence suspended across an adapter restart after seed failure', async () => { + const sessionId = 'cold-restore-seed-failure' + const sessionDir = join(historyDir, getHistorySessionDirName(sessionId)) + mkdirSync(sessionDir, { recursive: true }) + writeFileSync( + join(sessionDir, 'meta.json'), + JSON.stringify({ + cwd: '/tmp', + cols: 80, + rows: 24, + startedAt: '2026-07-10T08:00:00Z', + endedAt: null, + exitCode: null + }) + ) + writeFileSync(join(sessionDir, 'scrollback.bin'), 'recovered marker\r\n') + + const first = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const originalWriteSync = HeadlessEmulator.prototype.writeSync + const writeSyncSpy = vi + .spyOn(HeadlessEmulator.prototype, 'writeSync') + .mockImplementation(function (this: HeadlessEmulator, data) { + return data.includes('recovered marker') ? false : originalWriteSync.call(this, data) + }) + try { + await first.spawn({ cols: 80, rows: 24, sessionId }) + lastSubprocess._simulateData('fresh-only output\r\n') + await first.disconnectOnly() + + historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir }) + const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId }) + expect(result.coldRestore?.scrollback).toContain('recovered marker') + const internals = historyAdapter as unknown as { + checkpointSessions(sessionIds: Iterable): Promise> + } + await internals.checkpointSessions([sessionId]) + + expect(existsSync(join(sessionDir, 'checkpoint.json'))).toBe(false) + expect(readFileSync(join(sessionDir, 'scrollback.bin'), 'utf8')).toContain( + 'recovered marker' + ) + } finally { + writeSyncSpy.mockRestore() + } + }) + it('does not cold-restore for clean shutdown (endedAt set)', async () => { const sessionId = 'clean-exit' 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 648870abe..cac7f14e4 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -36,6 +36,14 @@ type ColdRestorePayload = { oscLinks?: TerminalOscLinkRange[] } +function getRecoveredHistorySeed(restoreInfo: ColdRestoreInfo): string | null { + // Why: alt-screen snapshots represent the TUI buffer; prefer its normal + // scrollback so a dead TUI is not revived as the fresh shell's active screen. + return restoreInfo.modes.alternateScreen + ? restoreInfo.scrollbackAnsi || restoreInfo.snapshotAnsi || null + : restoreInfo.rehydrateSequences + restoreInfo.snapshotAnsi +} + export type DaemonPtyAdapterOptions = { socketPath: string tokenPath: string @@ -166,9 +174,9 @@ export class DaemonPtyAdapter implements IPtyProvider { restoreInfo = this.historyReader.detectColdRestore(sessionId) } } - const effectiveCwd = restoreInfo?.cwd ?? opts.cwd - const effectiveCols = restoreInfo?.cols ?? opts.cols - const effectiveRows = restoreInfo?.rows ?? opts.rows + let effectiveCwd = restoreInfo?.cwd ?? opts.cwd + let effectiveCols = restoreInfo?.cols ?? opts.cols + let effectiveRows = restoreInfo?.rows ?? opts.rows const shellReadySupported = opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false const isCodexStartupCommand = @@ -184,26 +192,31 @@ export class DaemonPtyAdapter implements IPtyProvider { ? CODEX_SHELL_READY_TIMEOUT_MS : undefined - const result = await this.client.request('createOrAttach', { - sessionId, - cols: effectiveCols, - rows: effectiveRows, - cwd: effectiveCwd, - env: opts.env, - envToDelete: opts.envToDelete, - command: opts.command, - startupCommandDelivery: opts.startupCommandDelivery, - // Why: without this, the daemon always spawns cmd.exe (COMSPEC) or - // PowerShell as a fallback — regardless of which shell the renderer - // asked for in the "+" menu or persisted as the default. Forwarding - // the override makes the daemon path behave the same as the in-process - // LocalPtyProvider. - shellOverride: opts.shellOverride, - terminalWindowsWslDistro: opts.terminalWindowsWslDistro, - terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation, - shellReadySupported, - ...(shellReadyTimeoutMs !== undefined ? { shellReadyTimeoutMs } : {}) - }) + const createOrAttach = (historySeed: string | null) => + this.client.request('createOrAttach', { + sessionId, + cols: effectiveCols, + rows: effectiveRows, + cwd: effectiveCwd, + env: opts.env, + envToDelete: opts.envToDelete, + command: opts.command, + startupCommandDelivery: opts.startupCommandDelivery, + // Why: without this, the daemon always spawns cmd.exe (COMSPEC) or + // PowerShell as a fallback — regardless of which shell the renderer + // asked for in the "+" menu or persisted as the default. Forwarding + // the override makes the daemon path behave the same as the in-process + // LocalPtyProvider. + shellOverride: opts.shellOverride, + terminalWindowsWslDistro: opts.terminalWindowsWslDistro, + terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation, + shellReadySupported, + ...(shellReadyTimeoutMs !== undefined ? { shellReadyTimeoutMs } : {}), + ...(historySeed ? { historySeed } : {}) + }) + + let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null + let result = await createOrAttach(scrollback) if (effectiveCwd) { this.initialCwds.set(sessionId, effectiveCwd) @@ -212,7 +225,7 @@ export class DaemonPtyAdapter implements IPtyProvider { // Why: the daemon RPC returns the shell pid of the backing subprocess. // Surfacing it through PtySpawnResult lets ipc/pty register with the // memory collector without a provider-specific accessor. - const pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null + let pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null // Why: check sticky cache first — StrictMode double-mounts call spawn // twice. The second call finds an existing daemon session (isNew=false) @@ -238,6 +251,21 @@ export class DaemonPtyAdapter implements IPtyProvider { if (result.isNew && restoreSkippedForLiveSession) { restoreInfo = this.historyReader?.detectColdRestore(sessionId, { ignoreCleanEnd: true }) ?? null + scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null + if (restoreInfo && scrollback) { + // Why: the aliveness probe raced with session death, so the first + // create lacked recovery bytes. Replace it before exposing the PTY. + await this.client.request('kill', { sessionId, immediate: true }) + effectiveCwd = restoreInfo.cwd + effectiveCols = restoreInfo.cols + effectiveRows = restoreInfo.rows + result = await createOrAttach(scrollback) + pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null + this.initialCwds.set(sessionId, effectiveCwd) + } + } else if (!result.isNew && result.historySeeded === false) { + restoreInfo = this.historyReader?.detectColdRestore(sessionId) ?? null + scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null } const wasAlreadyManaged = this.activeSessionIds.has(sessionId) @@ -246,7 +274,7 @@ export class DaemonPtyAdapter implements IPtyProvider { // Cold restore: daemon created a new session but disk history shows // an unclean shutdown → return saved scrollback so the renderer can // display the previous terminal content. - if (result.isNew && restoreInfo) { + if (restoreInfo && (result.isNew || result.historySeeded === false)) { // Why prefer scrollbackAnsi for alt-screen: snapshotAnsi is the alt buffer // (vim/less/htop); normal sessions use the full snapshot + rehydrate. // Why the snapshotAnsi fallback: a hibernated TUI agent (empty scrollback) @@ -254,25 +282,33 @@ export class DaemonPtyAdapter implements IPtyProvider { // (no rehydrateSequences — they start with \x1b[?1049h, which the // renderer's POST_REPLAY_MODE_RESET does NOT undo) lands the last frame as // normal scrollback. An empty snapshot still yields null → no-op. - const isAltScreen = restoreInfo.modes.alternateScreen - const scrollback = isAltScreen - ? restoreInfo.scrollbackAnsi || restoreInfo.snapshotAnsi || null - : restoreInfo.rehydrateSequences + restoreInfo.snapshotAnsi + const canReanchorHistory = !scrollback || result.historySeeded === true // Why: use registerWriter (not openSession) to avoid deleting the // existing checkpoint.json. If the revived daemon crashes again before // the next 5s tick, the checkpoint is the only recovery data available. if (this.historyManager) { - this.historyManager.registerWriter(sessionId) - this.sessionsNeedingFullCheckpoint.add(sessionId) - // Why: the revived generation has no valid checkpoint of its own; a - // cooldown inherited from the pre-crash generation (daemon respawn - // within one adapter) must not defer this re-anchor. - this.lastFullCheckpointAt.delete(sessionId) + if (canReanchorHistory) { + this.historyManager.registerWriter(sessionId) + this.sessionsNeedingFullCheckpoint.add(sessionId) + // Why: the revived generation has no valid checkpoint of its own; a + // cooldown inherited from the pre-crash generation (daemon respawn + // within one adapter) must not defer this re-anchor. + this.lastFullCheckpointAt.delete(sessionId) + } else { + // Preserve the old recovery files when the new daemon cannot include + // them; a fresh-only checkpoint would make the data loss permanent. + this.historyManager.suspendSession(sessionId) + } } if (scrollback) { const coldRestore = { scrollback, cwd: restoreInfo.cwd, oscLinks: restoreInfo.oscLinks } this.coldRestoreCache.set(sessionId, coldRestore) - return { id: sessionId, pid, coldRestore } + return { + id: sessionId, + pid, + coldRestore, + ...(!result.isNew ? { isReattach: true } : {}) + } } return { id: sessionId, pid } } @@ -285,6 +321,10 @@ export class DaemonPtyAdapter implements IPtyProvider { rows: effectiveRows }) .catch((err) => console.warn('[history] openSession failed:', sessionId, err)) + } else if (this.historyManager && result.historySeeded === false) { + // Why: the daemon keeps this failure bit with the live session, so a new + // adapter cannot promote its fresh-only snapshot after an app restart. + this.historyManager.suspendSession(sessionId) } else if (this.historyManager) { // Why: on warm reattach after app relaunch, the HistoryManager is a // fresh instance with no writers. registerWriter adds the writer diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index 3a585a089..7d98babfd 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -290,6 +290,7 @@ export class DaemonServer { terminalWindowsWslDistro: p.terminalWindowsWslDistro, terminalWindowsPowerShellImplementation: p.terminalWindowsPowerShellImplementation, shellReadySupported: p.shellReadySupported, + historySeed: p.historySeed, ...(p.shellReadyTimeoutMs !== undefined ? { shellReadyTimeoutMs: p.shellReadyTimeoutMs } : {}), @@ -332,7 +333,8 @@ export class DaemonServer { isNew: result.isNew, snapshot: result.snapshot, pid: result.pid, - shellState: result.shellState + shellState: result.shellState, + ...(result.historySeeded !== undefined ? { historySeeded: result.historySeeded } : {}) } } diff --git a/src/main/daemon/history-manager.ts b/src/main/daemon/history-manager.ts index 0edcdb91b..9453488c2 100644 --- a/src/main/daemon/history-manager.ts +++ b/src/main/daemon/history-manager.ts @@ -132,6 +132,13 @@ export class HistoryManager { }) } + suspendSession(sessionId: string): void { + // Why: if a fresh daemon cannot accept recovered scrollback, leaving its + // writer active would let the next checkpoint overwrite the only good copy. + this.writers.delete(sessionId) + this.disabledSessions.delete(sessionId) + } + /** Appends one take batch to the incremental log. Returns 'needs-checkpoint' * when the log is at capacity — the caller must take a full snapshot, which * subsumes the un-appended records (they were already applied to the live diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index ad8f14cb0..9ff69f89a 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -2372,6 +2372,32 @@ describe('createPtySubprocess', () => { } }) + it('does not issue a second Windows ConPTY kill when force follows graceful kill', () => { + const proc = mockPtyProcess(123456) as ReturnType & { + destroy: ReturnType + } + proc.destroy = vi.fn(() => proc.kill()) + spawnMock.mockReturnValue(proc) + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => { + throw new Error('already gone') + }) + const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + try { + const handle = createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 }) + handle.kill() + handle.forceKill() + handle.dispose() + + expect(proc.kill).toHaveBeenCalledOnce() + expect(killSpy).not.toHaveBeenCalled() + expect(proc.destroy).not.toHaveBeenCalled() + } finally { + killSpy.mockRestore() + restorePlatform(origPlatform) + } + }) + it('dispose() on Windows skips destroy after forceKill falls back to node-pty kill()', () => { const proc = mockPtyProcess(123456) as ReturnType & { destroy: ReturnType diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 30eb7b400..c732a16e1 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -1094,7 +1094,9 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl // has run, proc.pid refers to a recycled pid. Sending SIGKILL would // terminate an unrelated process. The fd release is handled by // dispose()/destroy(); forceKill is strictly for signalling a live child. - if (dead) { + // Why: Windows node-pty kill already closes ConPTY; retrying it through + // forceKill can double-close the native handle during workspace teardown. + if (dead || (process.platform === 'win32' && nodePtyKillIssued)) { return } try { diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index d156aeb48..0d377c7f3 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -65,6 +65,7 @@ export type SessionOptions = { subprocess: SubprocessHandle shellReadySupported: boolean shellReadyTimeoutMs?: number + historySeed?: string scrollback?: number // Why: fired once the session reaches a terminal state (natural exit or // kill-timeout force-dispose) so the owner (TerminalHost) can reap it — @@ -101,6 +102,7 @@ export class Session { private pendingOutputBytes = 0 private pendingOutputOverflowed = false private pendingOutputSeq = 0 + private readonly _historySeeded: boolean | undefined constructor(opts: SessionOptions) { this.sessionId = opts.sessionId @@ -117,6 +119,10 @@ export class Session { // responder; any daemon reply races ahead via in-process parsing and // clobbers the renderer's answer. See the comment in HeadlessEmulator. }) + // Why: recovery must precede listener registration; shells can emit their + // prompt synchronously as soon as onData subscribes. + this._historySeeded = + opts.historySeed === undefined ? undefined : this.emulator.writeSync(opts.historySeed) if (opts.shellReadySupported) { this._shellState = 'pending' @@ -141,6 +147,10 @@ export class Session { return this._shellState } + get historySeeded(): boolean | undefined { + return this._historySeeded + } + get exitCode(): number | null { return this._exitCode } diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 2a54ce2c1..8b8968931 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -31,6 +31,7 @@ export type CreateOrAttachOptions = { terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' shellReadySupported?: boolean shellReadyTimeoutMs?: number + historySeed?: string streamClient: { onData: (data: string) => void; onExit: (code: number) => void } } @@ -39,6 +40,7 @@ export type CreateOrAttachResult = { snapshot: TerminalSnapshot | null pid: number | null shellState: ShellReadyState + historySeeded?: boolean attachToken: symbol } @@ -105,6 +107,7 @@ export class TerminalHost { snapshot, pid: existing.pid, shellState: existing.shellState, + ...(existing.historySeeded !== undefined ? { historySeeded: existing.historySeeded } : {}), attachToken: token } } @@ -140,6 +143,7 @@ export class TerminalHost { terminalHandle: opts.env?.ORCA_TERMINAL_HANDLE, subprocess, shellReadySupported: opts.shellReadySupported ?? false, + historySeed: opts.historySeed, // Why: reap the dead session (dispose emulator + drop from the map) the // moment its subprocess exits, instead of retaining it for the daemon's // lifetime. Nothing reads a dead session's emulator (getSnapshot/ @@ -180,6 +184,7 @@ export class TerminalHost { snapshot: null, pid: subprocess.pid, shellState: session.shellState, + ...(session.historySeeded !== undefined ? { historySeeded: session.historySeeded } : {}), attachToken: token } } diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index 6c1c7c281..faf0df309 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -3,13 +3,11 @@ import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges // ─── Protocol Version ──────────────────────────────────────────────── import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' -// Why: daemons can survive app updates. Bump for IPC wire-shape changes, or -// when daemon-baked behavior cannot be delivered by on-disk wrapper refresh. -// Why: bump when adding daemon wire behavior so same-version old daemons do -// not silently accept the handshake and then reject new RPCs. -export const PROTOCOL_VERSION = 18 +// Why: daemons survive app updates; bump for IPC shape or baked behavior that +// wrapper refresh cannot deliver, so old daemons reject unsupported RPCs. +export const PROTOCOL_VERSION = 19 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18 ] as const // ─── Session State Machine ────────────────────────────────────────── @@ -104,6 +102,8 @@ export type CreateOrAttachRequest = { terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' shellReadySupported?: boolean shellReadyTimeoutMs?: number + /** Recovered ANSI applied before the new subprocess can emit startup output. */ + historySeed?: string } } @@ -313,6 +313,7 @@ export type CreateOrAttachResult = { snapshot: TerminalSnapshot | null pid: number | null shellState: ShellReadyState + historySeeded?: boolean } export type GetSnapshotResult = {