mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 08:03:14 +00:00
Fix Windows update-relaunch killing the live terminal daemon
On a Windows update relaunch the daemon can be wedged past every RPC budget (final checkpoint flush + installer/AV disk pressure), so the 3s health check AND the 5s session-list hello both time out while sessions are still alive - and the launcher failed closed, killing the daemon and every terminal session it owned. - Adopt an unresponsive daemon whose pipe still accepts a raw connection; a new rejected health state keeps replacing daemons that answered and refused the handshake (never adoptable). - Give Windows pid files a real startedAtMs (daemon self-reports it in the ready IPC message) and verify it via CIM CreationDate piggybacked on the existing command-line query, so the pid-recycling guard is no longer inert on win32. - Only delete legacy daemon pid/token files when the pid-file process is provably dead; deleting a live daemon''s token made its sessions permanently unadoptable after a protocol bump. - Capture agent resume records every 60s in the renderer (skipping unchanged records) so hard kills still leave a fresh resume record.
This commit is contained in:
@@ -83,7 +83,9 @@ async function main(): Promise<void> {
|
||||
|
||||
// Signal readiness to parent via IPC (if available)
|
||||
if (process.send) {
|
||||
process.send({ type: 'ready' })
|
||||
// Why: Windows has no cheap OS query for a child's start time, so the
|
||||
// daemon self-reports it here for the pid file's pid-recycling guard.
|
||||
process.send({ type: 'ready', startedAtMs: Date.now() - process.uptime() * 1000 })
|
||||
}
|
||||
|
||||
warmWindowsConptyOnce()
|
||||
|
||||
@@ -13,7 +13,9 @@ import {
|
||||
parseLinuxBootTimeSeconds,
|
||||
parseLinuxProcStartTicks,
|
||||
parseDaemonPidFile,
|
||||
startTimeMatches
|
||||
parseWindowsProcessIdentityJson,
|
||||
startTimeMatches,
|
||||
startTimesWithinTolerance
|
||||
} from './daemon-health'
|
||||
import type { SubprocessHandle } from './session'
|
||||
|
||||
@@ -125,6 +127,26 @@ describe('daemon health', () => {
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('classifies a hello-rejected daemon as rejected, not unreachable', async () => {
|
||||
// Why: 'rejected' means the daemon answered and refused adoption — the
|
||||
// launcher may replace it. 'unreachable' also covers a wedged-but-live
|
||||
// daemon, which must never be replaced while its pipe accepts connections.
|
||||
const server = new DaemonServer({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
writeFileSync(tokenPath, 'not-the-daemon-token', { mode: 0o600 })
|
||||
await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('rejected')
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(false)
|
||||
} finally {
|
||||
await server.shutdown()
|
||||
}
|
||||
})
|
||||
|
||||
it('does not unlink a live socket when the pid file does not match this daemon', async () => {
|
||||
if (process.platform === 'win32') {
|
||||
return
|
||||
@@ -286,6 +308,42 @@ describe('startTimeMatches', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseWindowsProcessIdentityJson', () => {
|
||||
it('parses command line and start time from the CIM query output', () => {
|
||||
expect(
|
||||
parseWindowsProcessIdentityJson(
|
||||
'{"cmd":"Orca.exe daemon-entry.js","start":1700000000000}\r\n'
|
||||
)
|
||||
).toEqual({ commandLine: 'Orca.exe daemon-entry.js', startedAtMs: 1_700_000_000_000 })
|
||||
})
|
||||
|
||||
it('returns a null start time when CreationDate was unavailable', () => {
|
||||
expect(
|
||||
parseWindowsProcessIdentityJson('{"cmd":"Orca.exe daemon-entry.js","start":null}')
|
||||
).toEqual({ commandLine: 'Orca.exe daemon-entry.js', startedAtMs: null })
|
||||
})
|
||||
|
||||
it('returns null for a missing process or inaccessible command line', () => {
|
||||
expect(parseWindowsProcessIdentityJson('')).toBeNull()
|
||||
expect(parseWindowsProcessIdentityJson(' \r\n')).toBeNull()
|
||||
expect(parseWindowsProcessIdentityJson('{"cmd":null,"start":123}')).toBeNull()
|
||||
expect(parseWindowsProcessIdentityJson('not-json')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('startTimesWithinTolerance', () => {
|
||||
it('fails open when either side is null', () => {
|
||||
expect(startTimesWithinTolerance(null, 1_700_000_000_000, 1_500)).toBe(true)
|
||||
expect(startTimesWithinTolerance(1_700_000_000_000, null, 1_500)).toBe(true)
|
||||
expect(startTimesWithinTolerance(null, null, 1_500)).toBe(true)
|
||||
})
|
||||
|
||||
it('matches within tolerance and rejects outside it', () => {
|
||||
expect(startTimesWithinTolerance(1_700_000_001_000, 1_700_000_000_000, 1_500)).toBe(true)
|
||||
expect(startTimesWithinTolerance(1_700_000_005_000, 1_700_000_000_000, 1_500)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('killStaleDaemon pid identity guards', () => {
|
||||
let dir: string
|
||||
let socketPath: string
|
||||
|
||||
@@ -24,8 +24,17 @@ const RESOLVER_HEALTH_CHECK_TIMEOUT_MS = 3_000
|
||||
const KILL_WAIT_MS = 3_000
|
||||
const KILL_POLL_MS = 100
|
||||
const START_TIME_TOLERANCE_MS = 1_500
|
||||
// Why: on Windows the pid file's startedAtMs is the daemon's self-reported
|
||||
// Node start time, while verification reads the OS process creation time —
|
||||
// the gap between them is the exe bootstrap, which AV/disk pressure can
|
||||
// stretch to seconds. Pid recycling differs by minutes-to-days, so a wide
|
||||
// tolerance keeps the guard effective without false mismatches.
|
||||
const WIN32_START_TIME_TOLERANCE_MS = 10_000
|
||||
|
||||
export type DaemonHealth = 'healthy' | 'unreachable' | 'pty-spawn-unhealthy'
|
||||
// 'rejected' means the daemon answered and refused the handshake (bad token,
|
||||
// foreign protocol) — it can never be adopted, unlike 'unreachable', which
|
||||
// also covers a live-but-wedged daemon that simply missed the RPC budget.
|
||||
export type DaemonHealth = 'healthy' | 'unreachable' | 'rejected' | 'pty-spawn-unhealthy'
|
||||
|
||||
type ParsedDaemonPid = {
|
||||
pid: number
|
||||
@@ -134,13 +143,13 @@ export function checkDaemonHealth(socketPath: string, tokenPath: string): Promis
|
||||
try {
|
||||
message = JSON.parse(line) as Record<string, unknown>
|
||||
} catch {
|
||||
settle('unreachable')
|
||||
settle('rejected')
|
||||
return
|
||||
}
|
||||
|
||||
if (message.type === 'hello') {
|
||||
if (!(message as HelloResponse).ok) {
|
||||
settle('unreachable')
|
||||
settle('rejected')
|
||||
return
|
||||
}
|
||||
// Why: a protocol-live daemon with a stale cwd or node-pty helper
|
||||
@@ -371,6 +380,10 @@ export function getProcessStartedAtMs(pid: number): number | null {
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
// Why: the only OS source is a CIM query costing a powershell spawn —
|
||||
// too slow for this sync path. Windows pid files instead carry the
|
||||
// daemon's self-reported start time from its ready message, and
|
||||
// isDaemonProcess verifies it against CIM CreationDate asynchronously.
|
||||
return null
|
||||
}
|
||||
|
||||
@@ -387,27 +400,61 @@ export function getProcessStartedAtMs(pid: number): number | null {
|
||||
}
|
||||
|
||||
export function startTimeMatches(pid: number, expectedStartedAtMs: number | null): boolean {
|
||||
if (expectedStartedAtMs === null) {
|
||||
return startTimesWithinTolerance(
|
||||
getProcessStartedAtMs(pid),
|
||||
expectedStartedAtMs,
|
||||
START_TIME_TOLERANCE_MS
|
||||
)
|
||||
}
|
||||
|
||||
// Why: fail open on null — a pid file or OS query without a start time must
|
||||
// not veto an otherwise-matching daemon (adoption safety beats recycle safety).
|
||||
export function startTimesWithinTolerance(
|
||||
actualStartedAtMs: number | null,
|
||||
expectedStartedAtMs: number | null,
|
||||
toleranceMs: number
|
||||
): boolean {
|
||||
if (expectedStartedAtMs === null || actualStartedAtMs === null) {
|
||||
return true
|
||||
}
|
||||
|
||||
const actualStartedAtMs = getProcessStartedAtMs(pid)
|
||||
if (actualStartedAtMs === null) {
|
||||
return true
|
||||
}
|
||||
|
||||
return Math.abs(actualStartedAtMs - expectedStartedAtMs) <= START_TIME_TOLERANCE_MS
|
||||
return Math.abs(actualStartedAtMs - expectedStartedAtMs) <= toleranceMs
|
||||
}
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
|
||||
export type WindowsProcessIdentity = {
|
||||
commandLine: string
|
||||
startedAtMs: number | null
|
||||
}
|
||||
|
||||
export function parseWindowsProcessIdentityJson(stdout: string): WindowsProcessIdentity | null {
|
||||
const trimmed = stdout.trim()
|
||||
if (!trimmed) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
const parsed = JSON.parse(trimmed) as { cmd?: unknown; start?: unknown }
|
||||
if (typeof parsed.cmd !== 'string' || !parsed.cmd) {
|
||||
return null
|
||||
}
|
||||
return {
|
||||
commandLine: parsed.cmd,
|
||||
startedAtMs:
|
||||
typeof parsed.start === 'number' && Number.isFinite(parsed.start) ? parsed.start : null
|
||||
}
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
// Why: the only reliable command-line source on Windows is a CIM query, which
|
||||
// costs a full powershell.exe spawn (300-800ms cold, worse under Defender).
|
||||
// Async because the sync version measurably froze the Electron main thread at
|
||||
// startup for the whole spawn (benchmark: ~0.5s warm, 3s timeout cap cold).
|
||||
// Timed under ORCA_STARTUP_DIAGNOSTICS so the cold-start benchmark can
|
||||
// attribute startup cost to these checks.
|
||||
async function queryWindowsProcessCommandLine(pid: number): Promise<string | null> {
|
||||
// CreationDate rides along in the same spawn so start-time verification adds
|
||||
// zero extra process launches. Timed under ORCA_STARTUP_DIAGNOSTICS so the
|
||||
// cold-start benchmark can attribute startup cost to these checks.
|
||||
async function queryWindowsProcessIdentity(pid: number): Promise<WindowsProcessIdentity | null> {
|
||||
const startedAt = performance.now()
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
@@ -416,14 +463,17 @@ async function queryWindowsProcessCommandLine(pid: number): Promise<string | nul
|
||||
'-NoProfile',
|
||||
'-NonInteractive',
|
||||
'-Command',
|
||||
`(Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}").CommandLine`
|
||||
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; ` +
|
||||
`if ($p) { $start = $null; ` +
|
||||
`if ($p.CreationDate) { $start = [long]([DateTimeOffset]$p.CreationDate).ToUnixTimeMilliseconds() }; ` +
|
||||
`@{ cmd = $p.CommandLine; start = $start } | ConvertTo-Json -Compress }`
|
||||
],
|
||||
{
|
||||
encoding: 'utf8',
|
||||
timeout: 3_000
|
||||
}
|
||||
)
|
||||
return stdout
|
||||
return parseWindowsProcessIdentityJson(stdout)
|
||||
} catch {
|
||||
return null
|
||||
} finally {
|
||||
@@ -450,15 +500,16 @@ async function isDaemonProcess(
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
const output = await queryWindowsProcessCommandLine(pid)
|
||||
if (output === null) {
|
||||
const identity = await queryWindowsProcessIdentity(pid)
|
||||
if (identity === null) {
|
||||
return false
|
||||
}
|
||||
// Why: image names are too broad after PID reuse. Match the daemon entry
|
||||
// plus the exact socket/token args so we only kill the daemon for this
|
||||
// userData protocol endpoint.
|
||||
return (
|
||||
commandLineMatchesDaemon(output, socketPath, tokenPath) && startTimeMatches(pid, startedAtMs)
|
||||
commandLineMatchesDaemon(identity.commandLine, socketPath, tokenPath) &&
|
||||
startTimesWithinTolerance(identity.startedAtMs, startedAtMs, WIN32_START_TIME_TOLERANCE_MS)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -485,7 +536,7 @@ async function isDaemonProcess(
|
||||
|
||||
async function getDaemonCommandLine(pid: number): Promise<string | null> {
|
||||
if (process.platform === 'win32') {
|
||||
return queryWindowsProcessCommandLine(pid)
|
||||
return (await queryWindowsProcessIdentity(pid))?.commandLine ?? null
|
||||
}
|
||||
|
||||
try {
|
||||
|
||||
@@ -25,6 +25,8 @@ const {
|
||||
isPackagedMock,
|
||||
probeSocketExistsMock,
|
||||
writeFileSyncMock,
|
||||
readFileSyncMock,
|
||||
unlinkSyncMock,
|
||||
netConnectMock,
|
||||
forkMock,
|
||||
checkDaemonHealthMock,
|
||||
@@ -34,6 +36,7 @@ const {
|
||||
isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemonMock,
|
||||
getProcessStartedAtMsMock,
|
||||
parseDaemonPidFileMock,
|
||||
daemonClientMock,
|
||||
spawnerInstances,
|
||||
ensureRunningOverrides,
|
||||
@@ -50,6 +53,13 @@ const {
|
||||
|
||||
const probeSocketExistsMock = vi.fn((_path?: string) => false)
|
||||
const writeFileSyncMock = vi.fn()
|
||||
// Why: readFileSync throws by default so legacyDaemonProcessMayBeAlive's
|
||||
// catch treats every legacy pid file as unreadable — matching the pre-fix
|
||||
// cleanup behavior every existing test was written against.
|
||||
const readFileSyncMock = vi.fn((): string => {
|
||||
throw new Error('ENOENT')
|
||||
})
|
||||
const unlinkSyncMock = vi.fn()
|
||||
const forkMock = vi.fn()
|
||||
const netConnectMock = vi.fn(() => {
|
||||
// Why: the real probeSocket() in daemon-init connects to the socket and
|
||||
@@ -82,7 +92,10 @@ const {
|
||||
const getDaemonLaunchIdentityMock = vi.fn(() => 'match')
|
||||
const isDaemonStaleForCurrentBundleMock = vi.fn(() => false)
|
||||
const killStaleDaemonMock = vi.fn(async () => true)
|
||||
const getProcessStartedAtMsMock = vi.fn(() => 1_000_000)
|
||||
const getProcessStartedAtMsMock = vi.fn((): number | null => 1_000_000)
|
||||
const parseDaemonPidFileMock = vi.fn(
|
||||
(): { pid: number; startedAtMs: number | null } | null => null
|
||||
)
|
||||
|
||||
const daemonClientMock = vi.fn().mockImplementation(function MockDaemonClient() {
|
||||
return {
|
||||
@@ -141,6 +154,8 @@ const {
|
||||
isPackagedMock,
|
||||
probeSocketExistsMock,
|
||||
writeFileSyncMock,
|
||||
readFileSyncMock,
|
||||
unlinkSyncMock,
|
||||
netConnectMock,
|
||||
forkMock,
|
||||
checkDaemonHealthMock,
|
||||
@@ -150,6 +165,7 @@ const {
|
||||
isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemonMock,
|
||||
getProcessStartedAtMsMock,
|
||||
parseDaemonPidFileMock,
|
||||
daemonClientMock,
|
||||
spawnerInstances,
|
||||
ensureRunningOverrides,
|
||||
@@ -209,7 +225,8 @@ vi.mock('electron', () => ({
|
||||
vi.mock('fs', () => ({
|
||||
mkdirSync: vi.fn(),
|
||||
existsSync: (p: string) => probeSocketExistsMock(p) || p.includes('.pid'),
|
||||
unlinkSync: vi.fn(),
|
||||
unlinkSync: unlinkSyncMock,
|
||||
readFileSync: readFileSyncMock,
|
||||
writeFileSync: writeFileSyncMock
|
||||
}))
|
||||
|
||||
@@ -224,7 +241,8 @@ vi.mock('./daemon-health', () => ({
|
||||
healthCheckDaemon: healthCheckDaemonMock,
|
||||
isDaemonStaleForCurrentBundle: isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemon: killStaleDaemonMock,
|
||||
getProcessStartedAtMs: getProcessStartedAtMsMock
|
||||
getProcessStartedAtMs: getProcessStartedAtMsMock,
|
||||
parseDaemonPidFile: parseDaemonPidFileMock
|
||||
}))
|
||||
|
||||
vi.mock('./client', () => ({ DaemonClient: daemonClientMock }))
|
||||
@@ -352,6 +370,15 @@ async function importFresh() {
|
||||
daemonClientMock.mockClear()
|
||||
probeSocketExistsMock.mockClear()
|
||||
writeFileSyncMock.mockClear()
|
||||
readFileSyncMock.mockReset()
|
||||
readFileSyncMock.mockImplementation(() => {
|
||||
throw new Error('ENOENT')
|
||||
})
|
||||
unlinkSyncMock.mockClear()
|
||||
parseDaemonPidFileMock.mockReset()
|
||||
parseDaemonPidFileMock.mockReturnValue(null)
|
||||
getProcessStartedAtMsMock.mockReset()
|
||||
getProcessStartedAtMsMock.mockReturnValue(1_000_000)
|
||||
// Why: importing daemon-init *after* resetModules means the module-level
|
||||
// `spawner`/`adapter`/`restartInFlight` start fresh for every test, which is
|
||||
// the only way to reliably exercise the "first-time init" path and the
|
||||
@@ -1385,7 +1412,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces a health-check-failing daemon when live sessions cannot be verified', async () => {
|
||||
it('replaces a health-check-failing daemon when live sessions cannot be verified and the pipe is dead', async () => {
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
@@ -1429,6 +1456,203 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('adopts an unresponsive daemon whose pipe still accepts connections (update-relaunch wedge)', async () => {
|
||||
// Why: the Windows update-relaunch regression — post-install disk/AV load
|
||||
// wedges the daemon past the 3s health budget AND the 5s hello budget of
|
||||
// the session-list re-verification, while its sessions are still alive.
|
||||
// The old fail-closed path killed the daemon here. A pipe that accepts a
|
||||
// raw connection proves the daemon is alive, so the launcher must adopt.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
daemonClientMock.mockImplementationOnce(function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
|
||||
// The raw pipe probe succeeds even though every RPC timed out.
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
netConnectMock.mockImplementationOnce(() => {
|
||||
const handlers: Record<string, (() => void)[]> = { connect: [], error: [] }
|
||||
return {
|
||||
on(event: string, cb: () => void) {
|
||||
handlers[event]?.push(cb)
|
||||
if (event === 'connect') {
|
||||
queueMicrotask(() => cb())
|
||||
}
|
||||
return this
|
||||
},
|
||||
removeListener(event: string, cb: () => void) {
|
||||
handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? []
|
||||
return this
|
||||
},
|
||||
destroy() {}
|
||||
}
|
||||
})
|
||||
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(killStaleDaemonMock).not.toHaveBeenCalled()
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces a hello-rejected daemon even though its pipe accepts connections', async () => {
|
||||
// Why: 'rejected' means the daemon answered and refused the handshake —
|
||||
// it can never be adopted, so keeping it alive would strand the app with
|
||||
// no terminals forever. Replacement stays the only recovery.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
daemonClientMock.mockImplementationOnce(function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('Hello rejected')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('rejected')
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
forkMock.mockImplementationOnce(() => ({
|
||||
pid: 12345,
|
||||
on(event: string, cb: (arg?: unknown) => void) {
|
||||
if (event === 'message') {
|
||||
queueMicrotask(() => cb({ type: 'ready' }))
|
||||
}
|
||||
return this
|
||||
},
|
||||
off() {
|
||||
return this
|
||||
},
|
||||
disconnect: vi.fn(),
|
||||
unref: vi.fn()
|
||||
}))
|
||||
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(killStaleDaemonMock).toHaveBeenCalledWith(
|
||||
FAKE_RUNTIME_DIR,
|
||||
'/fake/socket',
|
||||
'/fake/token'
|
||||
)
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('adopts a healthy daemon whose pid-file identity cannot be verified (null startedAtMs metadata)', async () => {
|
||||
// Why: the regression contract — a pid file with startedAtMs null (all
|
||||
// pre-fix Windows pid files) resolves launch identity to 'unknown'. With
|
||||
// a live daemon answering on the pipe, that must ADOPT, never replace.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
getDaemonLaunchIdentityMock.mockReturnValueOnce('unknown')
|
||||
isPackagedMock.mockReturnValue(true)
|
||||
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(killStaleDaemonMock).not.toHaveBeenCalled()
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('writes the daemon self-reported start time to the pid file when the OS query returns null', async () => {
|
||||
// Why: getProcessStartedAtMs has no cheap Windows implementation, so the
|
||||
// pid file's pid-recycling guard depends on the ready-message fallback.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
|
||||
getProcessStartedAtMsMock.mockReturnValue(null)
|
||||
forkMock.mockImplementationOnce(() => ({
|
||||
pid: 12345,
|
||||
on(event: string, cb: (arg?: unknown) => void) {
|
||||
if (event === 'message') {
|
||||
queueMicrotask(() => cb({ type: 'ready', startedAtMs: 1_700_000_123_456 }))
|
||||
}
|
||||
return this
|
||||
},
|
||||
off() {
|
||||
return this
|
||||
},
|
||||
disconnect: vi.fn(),
|
||||
unref: vi.fn()
|
||||
}))
|
||||
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(writeFileSyncMock).toHaveBeenCalledWith(
|
||||
`/fake/daemon/daemon-v${PROTOCOL_VERSION}.pid`,
|
||||
JSON.stringify({
|
||||
pid: 12345,
|
||||
startedAtMs: 1_700_000_123_456,
|
||||
entryPath: FAKE_DAEMON_ENTRY_PATH,
|
||||
appVersion: '1.2.3'
|
||||
}),
|
||||
{ mode: 0o600 }
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps legacy daemon pid/token files when the probe fails but the pid-file process is alive', async () => {
|
||||
// Why: deleting a live-but-wedged legacy daemon's token file makes its
|
||||
// sessions permanently unadoptable — no future launch could authenticate.
|
||||
const mod = await importFresh()
|
||||
readFileSyncMock.mockReturnValue('{"pid":123}')
|
||||
// process.pid is guaranteed alive, so the liveness probe succeeds.
|
||||
parseDaemonPidFileMock.mockReturnValue({ pid: process.pid, startedAtMs: null })
|
||||
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const legacyUnlinks = unlinkSyncMock.mock.calls.filter(
|
||||
([p]) => typeof p === 'string' && (p.includes('.token') || p.includes('.pid'))
|
||||
)
|
||||
expect(legacyUnlinks).toEqual([])
|
||||
})
|
||||
|
||||
it('cleans up legacy daemon pid/token files when the probe fails and the process is gone', async () => {
|
||||
const mod = await importFresh()
|
||||
readFileSyncMock.mockReturnValue('{"pid":123}')
|
||||
// Why: spy on process.kill so the liveness probe deterministically reports
|
||||
// "no such process" without depending on an unallocated real pid.
|
||||
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => {
|
||||
throw new Error('ESRCH')
|
||||
})
|
||||
parseDaemonPidFileMock.mockReturnValue({ pid: 999_999, startedAtMs: null })
|
||||
|
||||
try {
|
||||
await mod.initDaemonPtyProvider()
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
|
||||
const tokenUnlinks = unlinkSyncMock.mock.calls.filter(
|
||||
([p]) => typeof p === 'string' && p.includes('.token')
|
||||
)
|
||||
expect(tokenUnlinks.length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('replaces a health-check-failing daemon when no live sessions would be lost', async () => {
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
@@ -8,7 +8,7 @@ module-level spawner/adapter singletons must stay co-located so a future
|
||||
change cannot leave them drifting out of sync. */
|
||||
import { join } from 'node:path'
|
||||
import { app } from 'electron'
|
||||
import { mkdirSync, existsSync, unlinkSync, writeFileSync } from 'node:fs'
|
||||
import { mkdirSync, existsSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'
|
||||
import { fork } from 'node:child_process'
|
||||
import { connect } from 'node:net'
|
||||
import {
|
||||
@@ -34,7 +34,8 @@ import {
|
||||
getProcessStartedAtMs,
|
||||
checkDaemonHealth,
|
||||
isDaemonStaleForCurrentBundle,
|
||||
killStaleDaemon
|
||||
killStaleDaemon,
|
||||
parseDaemonPidFile
|
||||
} from './daemon-health'
|
||||
import { DegradedDaemonPtyProvider } from './degraded-daemon-pty-provider'
|
||||
import {
|
||||
@@ -233,9 +234,7 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
|
||||
// Why: a busy machine (e.g. right after an update) can time out the
|
||||
// health check while the daemon is alive and owning terminals. Killing
|
||||
// it would destroy every live session, so re-verify with a session list
|
||||
// first. Only a verified non-empty list preserves: a daemon that cannot
|
||||
// even list sessions cannot serve terminals, and replacing it is the
|
||||
// only recovery.
|
||||
// first.
|
||||
const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
if (liveSessionCount !== null && liveSessionCount > 0) {
|
||||
if (health === 'pty-spawn-unhealthy') {
|
||||
@@ -253,6 +252,20 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
|
||||
)
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
// Why: on a Windows update relaunch the daemon can be wedged past every
|
||||
// RPC budget (final checkpoint flush + installer/AV disk pressure), so
|
||||
// both the health check AND the session list time out while sessions
|
||||
// are still alive — failing closed here is what killed those sessions.
|
||||
// A pipe that still accepts connections proves a live daemon: adopt it
|
||||
// and let the adapter reconnect once the daemon drains. 'rejected'
|
||||
// means the daemon answered and refused the handshake — it can never be
|
||||
// adopted, so replacement stays the only recovery.
|
||||
if (liveSessionCount === null && health !== 'rejected' && (await probeSocket(socketPath))) {
|
||||
console.warn(
|
||||
'[daemon] Preserving unresponsive daemon because its socket still accepts connections'
|
||||
)
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a raw socket can outlive a broken or wedged daemon. Kill by PID
|
||||
@@ -323,11 +336,19 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
|
||||
// killStaleDaemon() can verify the pid still belongs to the daemon
|
||||
// we forked before SIGTERMing it. Prevents pid-recycling hazard
|
||||
// where the OS hands the daemon's old pid to an unrelated process.
|
||||
// Why the ready-message fallback: Windows has no cheap OS query
|
||||
// for start time, so the daemon self-reports it — without this the
|
||||
// recycling guard was permanently inert on win32.
|
||||
const selfReported = (msg as { startedAtMs?: unknown }).startedAtMs
|
||||
writeFileSync(
|
||||
getDaemonPidPath(runtimeDir),
|
||||
serializeDaemonPidFile({
|
||||
pid: child.pid,
|
||||
startedAtMs: getProcessStartedAtMs(child.pid),
|
||||
startedAtMs:
|
||||
getProcessStartedAtMs(child.pid) ??
|
||||
(typeof selfReported === 'number' && Number.isFinite(selfReported)
|
||||
? selfReported
|
||||
: null),
|
||||
entryPath,
|
||||
appVersion: app.getVersion()
|
||||
}),
|
||||
@@ -708,6 +729,21 @@ export async function cleanupDaemonForProtocol(
|
||||
return { cleaned: didRequestShutdown || didKillStaleDaemon, killedCount }
|
||||
}
|
||||
|
||||
function legacyDaemonProcessMayBeAlive(runtimeDir: string, protocolVersion: number): boolean {
|
||||
try {
|
||||
const parsed = parseDaemonPidFile(
|
||||
readFileSync(getDaemonPidPath(runtimeDir, protocolVersion), 'utf8')
|
||||
)
|
||||
if (!parsed) {
|
||||
return false
|
||||
}
|
||||
process.kill(parsed.pid, 0)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
async function createLegacyDaemonAdapters(runtimeDir: string): Promise<DaemonPtyAdapter[]> {
|
||||
const adapters: DaemonPtyAdapter[] = []
|
||||
for (const protocolVersion of PREVIOUS_DAEMON_PROTOCOL_VERSIONS) {
|
||||
@@ -717,23 +753,27 @@ async function createLegacyDaemonAdapters(runtimeDir: string): Promise<DaemonPty
|
||||
// Why: dead legacy daemons leave pid/token files behind forever (one per
|
||||
// protocol bump). A stale pid eventually gets recycled by an unrelated
|
||||
// process, turning any future identity check into a PowerShell spawn.
|
||||
// The socket is provably dead, so remove the leftovers — mirrors what
|
||||
// cleanupDaemonForProtocol already does for the current version.
|
||||
for (const stalePath of [
|
||||
getDaemonPidPath(runtimeDir, protocolVersion),
|
||||
getDaemonTokenPath(runtimeDir, protocolVersion)
|
||||
]) {
|
||||
try {
|
||||
unlinkSync(stalePath)
|
||||
} catch {
|
||||
// Best-effort
|
||||
// Only clean up when the pid-file process is provably gone: a live
|
||||
// legacy daemon can transiently fail the 1s probe right after an update
|
||||
// (wedged event loop, exhausted pipe backlog), and deleting its token
|
||||
// file would make its sessions permanently unadoptable.
|
||||
if (!legacyDaemonProcessMayBeAlive(runtimeDir, protocolVersion)) {
|
||||
for (const stalePath of [
|
||||
getDaemonPidPath(runtimeDir, protocolVersion),
|
||||
getDaemonTokenPath(runtimeDir, protocolVersion)
|
||||
]) {
|
||||
try {
|
||||
unlinkSync(stalePath)
|
||||
} catch {
|
||||
// Best-effort
|
||||
}
|
||||
}
|
||||
}
|
||||
if (process.platform !== 'win32' && existsSync(socketPath)) {
|
||||
try {
|
||||
unlinkSync(socketPath)
|
||||
} catch {
|
||||
// Best-effort
|
||||
if (process.platform !== 'win32' && existsSync(socketPath)) {
|
||||
try {
|
||||
unlinkSync(socketPath)
|
||||
} catch {
|
||||
// Best-effort
|
||||
}
|
||||
}
|
||||
}
|
||||
continue
|
||||
|
||||
@@ -168,6 +168,11 @@ import { resolveMountedLazyModalIds, type LazyModalId } from './lazy-modal-mount
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import PinnedTabCloseDialog from './components/terminal-pane/PinnedTabCloseDialog'
|
||||
|
||||
// Why: agents alive during a hard kill (crash, forced update install) need a
|
||||
// reasonably fresh resume record on disk; one minute bounds the lost window
|
||||
// without measurable per-tick cost (the capture skips unchanged records).
|
||||
const SLEEPING_AGENT_RESUME_CAPTURE_INTERVAL_MS = 60_000
|
||||
|
||||
const isMac = navigator.userAgent.includes('Mac')
|
||||
const isWindows = !isMac && navigator.userAgent.includes('Windows')
|
||||
const shortcutPlatform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux'
|
||||
@@ -1306,6 +1311,22 @@ function App(): React.JSX.Element {
|
||||
return () => window.removeEventListener('beforeunload', captureAndFlush)
|
||||
}, [])
|
||||
|
||||
// Why: beforeunload never fires on a hard kill (crash, forced update
|
||||
// install, TerminateProcess), so agents alive at that moment would leave no
|
||||
// resume record. This periodic capture stores only agent session ids — not
|
||||
// scrollback, see the no-periodic-scrollback note below — and the store
|
||||
// action skips unchanged records, so idle ticks write nothing; real changes
|
||||
// flow through the debounced session-write subscriber.
|
||||
useEffect(() => {
|
||||
const timer = window.setInterval(() => {
|
||||
if (!shouldPersistWorkspaceSession(useAppStore.getState())) {
|
||||
return
|
||||
}
|
||||
useAppStore.getState().captureAllSleepingAgentSessions()
|
||||
}, SLEEPING_AGENT_RESUME_CAPTURE_INTERVAL_MS)
|
||||
return () => window.clearInterval(timer)
|
||||
}, [])
|
||||
|
||||
// Own the single window-close-request subscription at the always-mounted App
|
||||
// root. Why: the rich confirmation flow lives in Terminal, which is not
|
||||
// mounted on the no-workspace landing page (and is lazy-loaded elsewhere), so
|
||||
|
||||
@@ -214,6 +214,52 @@ describe('captureAllSleepingAgentSessions', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('skips rewriting an unchanged resume record on repeated capture', () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
tabsByWorktree: {
|
||||
'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1' })]
|
||||
}
|
||||
} as Partial<AppState>)
|
||||
const providerSession = { key: 'session_id' as const, id: 'codex-session-1' }
|
||||
store
|
||||
.getState()
|
||||
.setAgentStatus(
|
||||
'tab-1:leaf-1',
|
||||
{ state: 'working', prompt: 'first task', agentType: 'codex' },
|
||||
'Codex',
|
||||
{ updatedAt: 10, stateStartedAt: 10 },
|
||||
{ tabId: 'tab-1', worktreeId: 'wt-1' },
|
||||
{ providerSession }
|
||||
)
|
||||
|
||||
store.getState().captureAllSleepingAgentSessions()
|
||||
const first = store.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']
|
||||
expect(first).toMatchObject({ origin: 'quit' })
|
||||
|
||||
// Why: the periodic resume-record capture re-runs this action every
|
||||
// minute; an unchanged agent must not dirty the store (and the debounced
|
||||
// session write) with a record differing only by capturedAt.
|
||||
store.getState().captureAllSleepingAgentSessions()
|
||||
expect(store.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBe(first)
|
||||
|
||||
// A real status change must still refresh the record.
|
||||
store
|
||||
.getState()
|
||||
.setAgentStatus(
|
||||
'tab-1:leaf-1',
|
||||
{ state: 'waiting', prompt: 'first task', agentType: 'codex' },
|
||||
'Codex',
|
||||
{ updatedAt: 20, stateStartedAt: 20 },
|
||||
{ tabId: 'tab-1', worktreeId: 'wt-1' },
|
||||
{ providerSession }
|
||||
)
|
||||
store.getState().captureAllSleepingAgentSessions()
|
||||
const refreshed = store.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']
|
||||
expect(refreshed).not.toBe(first)
|
||||
expect(refreshed).toMatchObject({ state: 'waiting', origin: 'quit' })
|
||||
})
|
||||
|
||||
it('preserves hydrated launch config during live recapture without a registry entry', () => {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
|
||||
@@ -542,6 +542,34 @@ export function collectHibernatedCompletionEvidenceForWorktree(
|
||||
return retained
|
||||
}
|
||||
|
||||
// Why: the periodic resume-record capture re-runs on an interval; comparing
|
||||
// everything except capturedAt lets an unchanged agent skip the store write
|
||||
// entirely, so idle ticks never dirty the session persistence pipeline.
|
||||
function sleepingRecordsEquivalentIgnoringCaptureTime(
|
||||
existing: SleepingAgentSessionRecord | undefined,
|
||||
next: SleepingAgentSessionRecord
|
||||
): boolean {
|
||||
if (!existing) {
|
||||
return false
|
||||
}
|
||||
return (
|
||||
existing.paneKey === next.paneKey &&
|
||||
existing.tabId === next.tabId &&
|
||||
existing.worktreeId === next.worktreeId &&
|
||||
existing.agent === next.agent &&
|
||||
existing.providerSession.key === next.providerSession.key &&
|
||||
existing.providerSession.id === next.providerSession.id &&
|
||||
existing.prompt === next.prompt &&
|
||||
existing.state === next.state &&
|
||||
existing.updatedAt === next.updatedAt &&
|
||||
existing.terminalTitle === next.terminalTitle &&
|
||||
existing.lastAssistantMessage === next.lastAssistantMessage &&
|
||||
existing.interrupted === next.interrupted &&
|
||||
existing.origin === next.origin &&
|
||||
launchConfigsEqual(existing.launchConfig, next.launchConfig)
|
||||
)
|
||||
}
|
||||
|
||||
function recoveryRecordMatches(
|
||||
existing: SleepingAgentSessionRecord | undefined,
|
||||
next: SleepingAgentSessionRecord
|
||||
@@ -2009,7 +2037,10 @@ export const createAgentStatusSlice: StateCreator<AppState, [], [], AgentStatusS
|
||||
launchConfig: getLaunchConfigForEntry(s, entry),
|
||||
origin: 'quit'
|
||||
})
|
||||
if (record && next[record.paneKey] !== record) {
|
||||
if (
|
||||
record &&
|
||||
!sleepingRecordsEquivalentIgnoringCaptureTime(next[record.paneKey], record)
|
||||
) {
|
||||
next[record.paneKey] = record
|
||||
changed = true
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user