diff --git a/src/main/daemon/daemon-health.test.ts b/src/main/daemon/daemon-health.test.ts index 3574eaa7a..5ace2af8d 100644 --- a/src/main/daemon/daemon-health.test.ts +++ b/src/main/daemon/daemon-health.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdtempSync, rmSync, writeFileSync } from 'fs' +import { spawn } from 'child_process' +import { mkdtempSync, rmSync, utimesSync, writeFileSync } from 'fs' import { tmpdir } from 'os' import { join } from 'path' import { createServer, connect, type Server } from 'net' @@ -8,6 +9,7 @@ import { getDaemonPidPath, serializeDaemonPidFile } from './daemon-spawner' import { getProcessStartedAtMs, healthCheckDaemon, + isDaemonOlderThanPathMtime, killStaleDaemon, parseDaemonPidFile, startTimeMatches @@ -264,3 +266,60 @@ describe('killStaleDaemon pid identity guards', () => { } }) }) + +describe('isDaemonOlderThanPathMtime', () => { + let dir: string + let socketPath: string + let tokenPath: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'daemon-health-mtime-test-')) + socketPath = join(dir, 'daemon.sock') + tokenPath = join(dir, 'daemon.token') + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + it('detects a daemon that started before the current bundle entry was written', async () => { + if (process.platform === 'win32') { + return + } + + const child = spawn( + process.execPath, + [ + '-e', + 'setTimeout(() => {}, 30000)', + 'daemon-entry', + '--socket', + socketPath, + '--token', + tokenPath + ], + { stdio: 'ignore' } + ) + try { + await new Promise((resolve) => setTimeout(resolve, 100)) + const startedAtMs = getProcessStartedAtMs(child.pid!) + if (startedAtMs === null) { + return + } + + const entryPath = join(dir, 'daemon-entry.js') + writeFileSync(entryPath, '', 'utf8') + const future = new Date(startedAtMs + 10_000) + utimesSync(entryPath, future, future) + writeFileSync( + getDaemonPidPath(dir), + serializeDaemonPidFile({ pid: child.pid!, startedAtMs, entryPath }), + { mode: 0o600 } + ) + + expect(isDaemonOlderThanPathMtime(dir, socketPath, tokenPath, entryPath)).toBe(true) + } finally { + child.kill('SIGKILL') + } + }) +}) diff --git a/src/main/daemon/daemon-health.ts b/src/main/daemon/daemon-health.ts index 81437998c..b1a063823 100644 --- a/src/main/daemon/daemon-health.ts +++ b/src/main/daemon/daemon-health.ts @@ -1,7 +1,7 @@ /* oxlint-disable max-lines -- Why: pid validation shares process-identity helpers with kill escalation so the SIGKILL safety checks stay co-located. */ import { execFileSync } from 'child_process' -import { existsSync, readFileSync, unlinkSync } from 'fs' +import { existsSync, readFileSync, statSync, unlinkSync } from 'fs' import { connect, type Socket } from 'net' import { encodeNdjson } from './ndjson' import { getDaemonPidPath } from './daemon-spawner' @@ -498,6 +498,38 @@ export function getDaemonLaunchIdentity( return commandLine.includes(expectedEntryPath) ? 'match' : 'mismatch' } +export function isDaemonOlderThanPathMtime( + runtimeDir: string, + socketPath: string, + tokenPath: string, + path: string, + protocolVersion = PROTOCOL_VERSION +): boolean { + let parsedPid: ParsedDaemonPid | null + try { + parsedPid = parseDaemonPidFile( + readFileSync(getDaemonPidPath(runtimeDir, protocolVersion), 'utf8') + ) + } catch { + return false + } + + if (!parsedPid || !isDaemonProcess(parsedPid.pid, socketPath, tokenPath, parsedPid.startedAtMs)) { + return false + } + + const startedAtMs = parsedPid.startedAtMs ?? getProcessStartedAtMs(parsedPid.pid) + if (startedAtMs === null) { + return false + } + + try { + return startedAtMs + START_TIME_TOLERANCE_MS < statSync(path).mtimeMs + } catch { + return false + } +} + export async function killStaleDaemon( runtimeDir: string, socketPath: string, diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index de7cbe44c..3aa47ac55 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -22,6 +22,7 @@ const { healthCheckDaemonMock, getMacDaemonSystemResolverHealthMock, getDaemonLaunchIdentityMock, + isDaemonOlderThanPathMtimeMock, killStaleDaemonMock, getProcessStartedAtMsMock, daemonClientMock, @@ -65,6 +66,7 @@ const { const healthCheckDaemonMock = vi.fn(async () => true) const getMacDaemonSystemResolverHealthMock = vi.fn(() => 'healthy') const getDaemonLaunchIdentityMock = vi.fn(() => 'match') + const isDaemonOlderThanPathMtimeMock = vi.fn(() => false) const killStaleDaemonMock = vi.fn(async () => true) const getProcessStartedAtMsMock = vi.fn(() => 1_000_000) @@ -97,6 +99,7 @@ const { healthCheckDaemonMock, getMacDaemonSystemResolverHealthMock, getDaemonLaunchIdentityMock, + isDaemonOlderThanPathMtimeMock, killStaleDaemonMock, getProcessStartedAtMsMock, daemonClientMock, @@ -165,6 +168,7 @@ vi.mock('./daemon-health', () => ({ getDaemonLaunchIdentity: getDaemonLaunchIdentityMock, getMacDaemonSystemResolverHealth: getMacDaemonSystemResolverHealthMock, healthCheckDaemon: healthCheckDaemonMock, + isDaemonOlderThanPathMtime: isDaemonOlderThanPathMtimeMock, killStaleDaemon: killStaleDaemonMock, getProcessStartedAtMs: getProcessStartedAtMsMock })) @@ -256,6 +260,8 @@ async function importFresh() { getMacDaemonSystemResolverHealthMock.mockReset() getMacDaemonSystemResolverHealthMock.mockReturnValue('healthy') getDaemonLaunchIdentityMock.mockClear() + isDaemonOlderThanPathMtimeMock.mockReset() + isDaemonOlderThanPathMtimeMock.mockReturnValue(false) killStaleDaemonMock.mockClear() getAppPathMock.mockReset() getAppPathMock.mockReturnValue('/fake/app') @@ -1023,7 +1029,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { expect(child.unref).not.toHaveBeenCalled() }) - it('keeps packaged healthy-daemon reuse independent of dev app-path identity', async () => { + it('preserves a packaged healthy daemon when its app bundle is current', async () => { const mod = await importFresh() await mod.initDaemonPtyProvider() @@ -1035,12 +1041,76 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { killStaleDaemonMock.mockClear() forkMock.mockClear() isPackagedMock.mockReturnValue(true) - getDaemonLaunchIdentityMock.mockReturnValueOnce('mismatch') await launcher('/fake/socket', '/fake/token') - expect(getDaemonLaunchIdentityMock).not.toHaveBeenCalled() + expect(getDaemonLaunchIdentityMock).toHaveBeenCalledWith( + '/fake/userData/daemon', + '/fake/socket', + '/fake/token', + '/fake/app/out/main/daemon-entry.js' + ) + expect(isDaemonOlderThanPathMtimeMock).toHaveBeenCalledWith( + '/fake/userData/daemon', + '/fake/socket', + '/fake/token', + '/fake/app/out/main/daemon-entry.js' + ) expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() }) + + it('respawns a packaged daemon that predates the current app bundle', async () => { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + isPackagedMock.mockReturnValue(true) + isDaemonOlderThanPathMtimeMock.mockReturnValueOnce(true) + forkMock.mockImplementationOnce(() => { + const handlers: Record void)[]> = { + message: [], + error: [], + exit: [] + } + return { + pid: 12345, + on(event: string, cb: (arg?: unknown) => void) { + handlers[event]?.push(cb) + if (event === 'message') { + queueMicrotask(() => cb({ type: 'ready' })) + } + return this + }, + off(event: string, cb: (arg?: unknown) => void) { + handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? [] + return this + }, + disconnect: vi.fn(), + unref: vi.fn() + } + }) + + await launcher('/fake/socket', '/fake/token') + + expect(isDaemonOlderThanPathMtimeMock).toHaveBeenCalledWith( + '/fake/userData/daemon', + '/fake/socket', + '/fake/token', + '/fake/app/out/main/daemon-entry.js' + ) + expect(killStaleDaemonMock).toHaveBeenCalledWith( + '/fake/userData/daemon', + '/fake/socket', + '/fake/token' + ) + expect(forkMock).toHaveBeenCalledWith( + '/fake/app/out/main/daemon-entry.js', + ['--socket', '/fake/socket', '--token', '/fake/token'], + expect.objectContaining({ detached: true }) + ) + }) }) diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index e35315697..eaa14b329 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -32,6 +32,7 @@ import { getDaemonLaunchIdentity, getProcessStartedAtMs, healthCheckDaemon, + isDaemonOlderThanPathMtime, killStaleDaemon } from './daemon-health' import { @@ -162,15 +163,19 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { console.warn('[daemon] Replacing daemon with unavailable macOS system resolver') await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION) } else { - // Why: dev worktrees share the same orca-dev userData, so a daemon from - // a deleted sibling checkout can pass protocol health checks while still - // pointing at missing native modules. Packaged app paths are stable and - // should preserve existing warm daemon reuse semantics. - const identity = app.isPackaged - ? 'match' - : getDaemonLaunchIdentity(runtimeDir, socketPath, tokenPath, entryPath) - if (identity === 'mismatch') { - console.warn('[daemon] Replacing daemon launched from a different app path') + // Why: a protocol-healthy daemon can outlive the app bundle that + // launched it. In dev this happens after deleting/rebuilding a + // worktree; in packaged apps it happens when the stable + // /Applications/Orca.app path is replaced during update. + const identity = getDaemonLaunchIdentity(runtimeDir, socketPath, tokenPath, entryPath) + const stalePackagedBundle = + app.isPackaged && isDaemonOlderThanPathMtime(runtimeDir, socketPath, tokenPath, entryPath) + if (identity === 'mismatch' || stalePackagedBundle) { + console.warn( + stalePackagedBundle + ? '[daemon] Replacing daemon launched before the current app bundle was installed' + : '[daemon] Replacing daemon launched from a different app path' + ) await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION) } else { // Why: daemon is already running from a previous app session and diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts new file mode 100644 index 000000000..ee1515393 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest' +import { shouldOfferDaemonRestart } from './TerminalErrorToast' + +describe('shouldOfferDaemonRestart', () => { + it('matches stale daemon node-pty install failures', () => { + expect( + shouldOfferDaemonRestart( + "Daemon's node-pty install is gone (worktree deleted?). Restart Orca. node-pty: posix_spawn failed: ENOENT (errno 2, No such file or directory) - helper='/Applications/Orca.app/Contents/Resources/app.asar.unpacked/node_modules/node-pty/build/Release/spawn-helper'" + ) + ).toBe(true) + }) + + it('does not match unrelated terminal spawn errors', () => { + expect(shouldOfferDaemonRestart('SSH connection is not active.')).toBe(false) + expect(shouldOfferDaemonRestart('node-pty: open_slave failed: EMFILE (errno 24)')).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index ab397e728..1728da780 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -1,17 +1,28 @@ const SSH_PREFIX = 'SSH connection is not active' +const STALE_NODE_PTY_DAEMON_MARKERS = [ + "Daemon's node-pty install is gone", + 'node-pty: posix_spawn failed: ENOENT' +] function isSshError(error: string): boolean { return error.startsWith(SSH_PREFIX) } +export function shouldOfferDaemonRestart(error: string): boolean { + return STALE_NODE_PTY_DAEMON_MARKERS.every((marker) => error.includes(marker)) +} + export function TerminalErrorToast({ error, - onDismiss + onDismiss, + onRestartDaemon }: { error: string onDismiss: () => void + onRestartDaemon?: () => void }): React.JSX.Element { const ssh = isSshError(error) + const showDaemonRestart = !ssh && onRestartDaemon && shouldOfferDaemonRestart(error) return (
- + {error} - {!ssh && ( + {showDaemonRestart ? ( + <> + {'\n'} + Restart the terminal daemon from here to clear stale daemon state. + + ) : !ssh ? ( <> {'\n'} If this persists, please{' '} @@ -47,8 +63,27 @@ export function TerminalErrorToast({ . - )} + ) : null} + {showDaemonRestart ? ( + + ) : null}