Remove startup PTY spawn health probe and guard daemon replacement with live-session check (#5230)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo Hong
GitHub
parent
6762a10f3f
commit
0b19195fd9
|
|
@ -42,12 +42,7 @@ describe('daemon health socket listener cleanup', () => {
|
|||
|
||||
const result = healthCheckDaemon(socketPath, tokenPath)
|
||||
socket.emit('connect')
|
||||
socket.emit(
|
||||
'data',
|
||||
Buffer.from(
|
||||
'{"type":"hello","ok":true}\n{"id":"health-1","ok":true}\n{"id":"health-2","ok":true}\n'
|
||||
)
|
||||
)
|
||||
socket.emit('data', Buffer.from('{"type":"hello","ok":true}\n{"id":"health-1","ok":true}\n'))
|
||||
|
||||
await expect(result).resolves.toBe(true)
|
||||
expect(socket.listenerCount('connect')).toBe(0)
|
||||
|
|
|
|||
|
|
@ -74,36 +74,15 @@ describe('daemon health', () => {
|
|||
})
|
||||
|
||||
it('passes when a daemon answers ping', async () => {
|
||||
const ptySpawnHealthCheck = vi.fn(async () => {})
|
||||
const server = new DaemonServer({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
ptySpawnHealthCheck,
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(true)
|
||||
expect(ptySpawnHealthCheck).toHaveBeenCalledOnce()
|
||||
} finally {
|
||||
await server.shutdown()
|
||||
}
|
||||
})
|
||||
|
||||
it('fails when a protocol-healthy daemon cannot spawn PTYs', async () => {
|
||||
const server = new DaemonServer({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
ptySpawnHealthCheck: vi.fn(async () => {
|
||||
throw new Error('stale node-pty helper')
|
||||
}),
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
|
||||
try {
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(false)
|
||||
} finally {
|
||||
await server.shutdown()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,8 +21,6 @@ const KILL_WAIT_MS = 3_000
|
|||
const KILL_POLL_MS = 100
|
||||
const START_TIME_TOLERANCE_MS = 1_500
|
||||
|
||||
export type DaemonHealthCheckResult = 'healthy' | 'unhealthy' | 'pty-spawn-unhealthy'
|
||||
|
||||
type ParsedDaemonPid = {
|
||||
pid: number
|
||||
startedAtMs: number | null
|
||||
|
|
@ -67,13 +65,10 @@ function canConnectSocket(socketPath: string): Promise<boolean> {
|
|||
})
|
||||
}
|
||||
|
||||
export function checkDaemonHealth(
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
): Promise<DaemonHealthCheckResult> {
|
||||
export function healthCheckDaemon(socketPath: string, tokenPath: string): Promise<boolean> {
|
||||
return new Promise((resolve) => {
|
||||
if (process.platform !== 'win32' && !existsSync(socketPath)) {
|
||||
resolve('unhealthy')
|
||||
resolve(false)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -81,13 +76,13 @@ export function checkDaemonHealth(
|
|||
try {
|
||||
token = readFileSync(tokenPath, 'utf8').trim()
|
||||
} catch {
|
||||
resolve('unhealthy')
|
||||
resolve(false)
|
||||
return
|
||||
}
|
||||
|
||||
let settled = false
|
||||
let sock: Socket | null = null
|
||||
const settle = (result: DaemonHealthCheckResult): void => {
|
||||
const settle = (result: boolean): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
|
|
@ -102,7 +97,7 @@ export function checkDaemonHealth(
|
|||
sock?.off('connect', onConnect)
|
||||
sock?.off('data', onData)
|
||||
}
|
||||
const onError = (): void => settle('unhealthy')
|
||||
const onError = (): void => settle(false)
|
||||
const onConnect = (): void => {
|
||||
const hello: HelloMessage = {
|
||||
type: 'hello',
|
||||
|
|
@ -133,13 +128,13 @@ export function checkDaemonHealth(
|
|||
try {
|
||||
message = JSON.parse(line) as Record<string, unknown>
|
||||
} catch {
|
||||
settle('unhealthy')
|
||||
settle(false)
|
||||
return
|
||||
}
|
||||
|
||||
if (message.type === 'hello') {
|
||||
if (!(message as HelloResponse).ok) {
|
||||
settle('unhealthy')
|
||||
settle(false)
|
||||
return
|
||||
}
|
||||
sock?.write(encodeNdjson({ id: 'health-1', type: 'ping' }))
|
||||
|
|
@ -147,24 +142,12 @@ export function checkDaemonHealth(
|
|||
}
|
||||
|
||||
if (message.id === 'health-1') {
|
||||
if (!message.ok) {
|
||||
settle('unhealthy')
|
||||
return
|
||||
}
|
||||
// Why: protocol ping only proves the socket loop is alive. New
|
||||
// terminals also depend on node-pty's native helper state inside
|
||||
// the daemon process, which can go stale after dev rebuilds.
|
||||
sock?.write(encodeNdjson({ id: 'health-2', type: 'ptySpawnHealth' }))
|
||||
continue
|
||||
}
|
||||
|
||||
if (message.id === 'health-2') {
|
||||
settle(message.ok ? 'healthy' : 'pty-spawn-unhealthy')
|
||||
settle(message.ok === true)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
const timer = setTimeout(() => settle('unhealthy'), HEALTH_CHECK_TIMEOUT_MS)
|
||||
const timer = setTimeout(() => settle(false), HEALTH_CHECK_TIMEOUT_MS)
|
||||
|
||||
sock = connect({ path: socketPath })
|
||||
sock.on('error', onError)
|
||||
|
|
@ -175,10 +158,6 @@ export function checkDaemonHealth(
|
|||
})
|
||||
}
|
||||
|
||||
export async function healthCheckDaemon(socketPath: string, tokenPath: string): Promise<boolean> {
|
||||
return (await checkDaemonHealth(socketPath, tokenPath)) === 'healthy'
|
||||
}
|
||||
|
||||
function isSystemResolverHealth(value: unknown): value is SystemResolverHealth {
|
||||
return value === 'healthy' || value === 'unhealthy' || value === 'unknown'
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,7 +20,6 @@ const {
|
|||
writeFileSyncMock,
|
||||
netConnectMock,
|
||||
forkMock,
|
||||
checkDaemonHealthMock,
|
||||
healthCheckDaemonMock,
|
||||
getMacDaemonSystemResolverHealthMock,
|
||||
getDaemonLaunchIdentityMock,
|
||||
|
|
@ -67,7 +66,6 @@ const {
|
|||
}
|
||||
})
|
||||
|
||||
const checkDaemonHealthMock = vi.fn(async () => 'healthy')
|
||||
const healthCheckDaemonMock = vi.fn(async () => true)
|
||||
const getMacDaemonSystemResolverHealthMock = vi.fn(() => 'healthy')
|
||||
const getDaemonLaunchIdentityMock = vi.fn(() => 'match')
|
||||
|
|
@ -103,7 +101,6 @@ const {
|
|||
writeFileSyncMock,
|
||||
netConnectMock,
|
||||
forkMock,
|
||||
checkDaemonHealthMock,
|
||||
healthCheckDaemonMock,
|
||||
getMacDaemonSystemResolverHealthMock,
|
||||
getDaemonLaunchIdentityMock,
|
||||
|
|
@ -175,7 +172,6 @@ vi.mock('child_process', () => ({ fork: forkMock }))
|
|||
vi.mock('net', () => ({ connect: netConnectMock }))
|
||||
|
||||
vi.mock('./daemon-health', () => ({
|
||||
checkDaemonHealth: checkDaemonHealthMock,
|
||||
getDaemonLaunchIdentity: getDaemonLaunchIdentityMock,
|
||||
getMacDaemonSystemResolverHealth: getMacDaemonSystemResolverHealthMock,
|
||||
healthCheckDaemon: healthCheckDaemonMock,
|
||||
|
|
@ -272,9 +268,8 @@ async function importFresh() {
|
|||
setLocalPtyProviderMock.mockClear()
|
||||
unbindLocalProviderListenersMock.mockClear()
|
||||
rebindLocalProviderListenersMock.mockClear()
|
||||
checkDaemonHealthMock.mockClear()
|
||||
checkDaemonHealthMock.mockResolvedValue('healthy')
|
||||
healthCheckDaemonMock.mockClear()
|
||||
healthCheckDaemonMock.mockResolvedValue(true)
|
||||
getMacDaemonSystemResolverHealthMock.mockReset()
|
||||
getMacDaemonSystemResolverHealthMock.mockReturnValue('healthy')
|
||||
getDaemonLaunchIdentityMock.mockClear()
|
||||
|
|
@ -1031,7 +1026,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
probeSocketExistsMock.mockImplementation(
|
||||
(p?: string) => p === '/fake/app/out/main/daemon-entry.js'
|
||||
)
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unhealthy')
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
const mod = await importFresh()
|
||||
getAppPathMock.mockReturnValue('/fake/app/out/main')
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
|
@ -1074,7 +1069,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
})
|
||||
|
||||
it('removes detached daemon startup listeners after readiness', async () => {
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unhealthy')
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
|
|
@ -1129,7 +1124,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
})
|
||||
|
||||
it('removes detached daemon startup listeners after startup error', async () => {
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unhealthy')
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
|
|
@ -1173,7 +1168,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
expect(child.unref).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves a spawn-unhealthy daemon when it owns live sessions', async () => {
|
||||
it('preserves a health-check-failing daemon when it owns live sessions', async () => {
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
|
|
@ -1198,7 +1193,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('pty-spawn-unhealthy')
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
|
|
@ -1208,7 +1203,51 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
expect(forkMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces a spawn-unhealthy daemon when no live sessions would be lost', async () => {
|
||||
it('replaces a health-check-failing daemon when live sessions cannot be verified', async () => {
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
daemonClientMock.mockImplementationOnce(function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('daemon is wedged')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
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/userData/daemon',
|
||||
'/fake/socket',
|
||||
'/fake/token'
|
||||
)
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('replaces a health-check-failing daemon when no live sessions would be lost', async () => {
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
|
|
@ -1216,7 +1255,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
|||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('pty-spawn-unhealthy')
|
||||
healthCheckDaemonMock.mockResolvedValueOnce(false)
|
||||
forkMock.mockImplementationOnce(() => {
|
||||
const handlers: Record<string, ((arg?: unknown) => void)[]> = {
|
||||
message: [],
|
||||
|
|
|
|||
|
|
@ -28,10 +28,10 @@ import {
|
|||
type ListSessionsResult
|
||||
} from './types'
|
||||
import {
|
||||
checkDaemonHealth,
|
||||
getMacDaemonSystemResolverHealth,
|
||||
getDaemonLaunchIdentity,
|
||||
getProcessStartedAtMs,
|
||||
healthCheckDaemon,
|
||||
isDaemonStaleForCurrentBundle,
|
||||
killStaleDaemon
|
||||
} from './daemon-health'
|
||||
|
|
@ -174,77 +174,65 @@ async function shouldPreserveDaemonWithLiveSessions(
|
|||
function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
|
||||
return async (socketPath, tokenPath) => {
|
||||
const entryPath = getDaemonEntryPath()
|
||||
const health = await checkDaemonHealth(socketPath, tokenPath)
|
||||
if (health !== 'unhealthy') {
|
||||
if (health === 'pty-spawn-unhealthy') {
|
||||
if (
|
||||
await shouldPreserveDaemonWithLiveSessions(
|
||||
socketPath,
|
||||
tokenPath,
|
||||
'that cannot spawn new PTYs'
|
||||
const healthy = await healthCheckDaemon(socketPath, tokenPath)
|
||||
if (healthy) {
|
||||
const resolverHealth = await getMacDaemonSystemResolverHealth(socketPath, tokenPath)
|
||||
if (resolverHealth === 'unhealthy') {
|
||||
const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
if (liveSessionCount !== 0) {
|
||||
console.warn(
|
||||
liveSessionCount === null
|
||||
? '[daemon] Preserving daemon with unavailable macOS system resolver because live session state could not be verified'
|
||||
: `[daemon] Preserving daemon with unavailable macOS system resolver because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}`
|
||||
)
|
||||
) {
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
console.warn('[daemon] Replacing daemon that cannot spawn new PTYs')
|
||||
console.warn('[daemon] Replacing daemon with unavailable macOS system resolver')
|
||||
await cleanupDaemonForProtocol(runtimeDir, PROTOCOL_VERSION)
|
||||
} else {
|
||||
const resolverHealth = await getMacDaemonSystemResolverHealth(socketPath, tokenPath)
|
||||
if (resolverHealth === 'unhealthy') {
|
||||
const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
if (liveSessionCount !== 0) {
|
||||
console.warn(
|
||||
liveSessionCount === null
|
||||
? '[daemon] Preserving daemon with unavailable macOS system resolver because live session state could not be verified'
|
||||
: `[daemon] Preserving daemon with unavailable macOS system resolver because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}`
|
||||
)
|
||||
// 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 = await getDaemonLaunchIdentity(runtimeDir, socketPath, tokenPath, entryPath)
|
||||
const stalePackagedBundle =
|
||||
app.isPackaged &&
|
||||
(await isDaemonStaleForCurrentBundle(runtimeDir, socketPath, tokenPath, app.getVersion()))
|
||||
if (identity === 'mismatch' || stalePackagedBundle) {
|
||||
// Why: replacing a healthy daemon kills its child PTYs; defer code
|
||||
// freshness until no live terminal sessions would be lost.
|
||||
const replacementLabel = stalePackagedBundle
|
||||
? 'launched before the current app bundle was installed'
|
||||
: 'launched from a different app path'
|
||||
if (await shouldPreserveDaemonWithLiveSessions(socketPath, tokenPath, replacementLabel)) {
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
console.warn('[daemon] Replacing daemon with unavailable macOS system resolver')
|
||||
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: 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 = await getDaemonLaunchIdentity(
|
||||
runtimeDir,
|
||||
socketPath,
|
||||
tokenPath,
|
||||
entryPath
|
||||
)
|
||||
const stalePackagedBundle =
|
||||
app.isPackaged &&
|
||||
(await isDaemonStaleForCurrentBundle(
|
||||
runtimeDir,
|
||||
socketPath,
|
||||
tokenPath,
|
||||
app.getVersion()
|
||||
))
|
||||
if (identity === 'mismatch' || stalePackagedBundle) {
|
||||
// Why: replacing a healthy daemon kills its child PTYs; defer code
|
||||
// freshness until no live terminal sessions would be lost.
|
||||
const replacementLabel = stalePackagedBundle
|
||||
? 'launched before the current app bundle was installed'
|
||||
: 'launched from a different app path'
|
||||
if (
|
||||
await shouldPreserveDaemonWithLiveSessions(socketPath, tokenPath, replacementLabel)
|
||||
) {
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
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
|
||||
// responded to a protocol-level ping. Safe to reuse.
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
// Why: daemon is already running from a previous app session and
|
||||
// responded to a protocol-level ping. Safe to reuse.
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// 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.
|
||||
const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
if (liveSessionCount !== null && liveSessionCount > 0) {
|
||||
console.warn(
|
||||
`[daemon] Preserving daemon that failed the health check because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}`
|
||||
)
|
||||
return createPreservedDaemonHandle(runtimeDir)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a raw socket can outlive a broken or wedged daemon. Kill by PID
|
||||
|
|
|
|||
|
|
@ -198,19 +198,19 @@ describe('DaemonServer', () => {
|
|||
expect(result).toEqual({ pong: true })
|
||||
})
|
||||
|
||||
it('handles ptySpawnHealth through the daemon process', async () => {
|
||||
const ptySpawnHealthCheck = vi.fn(async () => {})
|
||||
server = new DaemonServer({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
ptySpawnHealthCheck,
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
it('replies with an error to unknown request types and keeps serving', async () => {
|
||||
await startServer()
|
||||
const c = await connectClient()
|
||||
|
||||
await expect(c.request('ptySpawnHealth', undefined)).resolves.toEqual({ healthy: true })
|
||||
expect(ptySpawnHealthCheck).toHaveBeenCalledOnce()
|
||||
// Why: older app builds still send the removed ptySpawnHealth probe.
|
||||
// The daemon must reject it gracefully so a downgraded client lands on
|
||||
// its session-preserving branch instead of crashing the daemon.
|
||||
await expect(c.request('ptySpawnHealth', undefined)).rejects.toThrow(
|
||||
'Unknown request type: ptySpawnHealth'
|
||||
)
|
||||
await expect(c.request<{ pong: boolean }>('ping', undefined)).resolves.toEqual({
|
||||
pong: true
|
||||
})
|
||||
})
|
||||
|
||||
it('handles systemResolverHealth', async () => {
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ import { TerminalHost } from './terminal-host'
|
|||
import { DaemonStreamDataBatcher } from './daemon-stream-data-batcher'
|
||||
import { readCurrentProcessMacSystemResolverHealth } from '../network/macos-system-resolver-health'
|
||||
import type { SubprocessHandle } from './session'
|
||||
import { checkPtySpawnHealth } from './pty-subprocess'
|
||||
import {
|
||||
PROTOCOL_VERSION,
|
||||
NOTIFY_PREFIX,
|
||||
|
|
@ -23,7 +22,6 @@ import {
|
|||
export type DaemonServerOptions = {
|
||||
socketPath: string
|
||||
tokenPath: string
|
||||
ptySpawnHealthCheck?: () => Promise<void>
|
||||
spawnSubprocess: (opts: {
|
||||
sessionId: string
|
||||
cols: number
|
||||
|
|
@ -47,7 +45,6 @@ export class DaemonServer {
|
|||
private host: TerminalHost
|
||||
private socketPath: string
|
||||
private tokenPath: string
|
||||
private ptySpawnHealthCheck: () => Promise<void>
|
||||
|
||||
private clients = new Map<string, ConnectedClient>()
|
||||
private streamDataBatcher = new DaemonStreamDataBatcher((clientId) => this.clients.get(clientId))
|
||||
|
|
@ -65,7 +62,6 @@ export class DaemonServer {
|
|||
this.tokenPath = opts.tokenPath
|
||||
this.token = randomUUID()
|
||||
this.host = new TerminalHost({ spawnSubprocess: opts.spawnSubprocess })
|
||||
this.ptySpawnHealthCheck = opts.ptySpawnHealthCheck ?? checkPtySpawnHealth
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
|
|
@ -379,10 +375,6 @@ export class DaemonServer {
|
|||
case 'systemResolverHealth':
|
||||
return { health: await readCurrentProcessMacSystemResolverHealth() }
|
||||
|
||||
case 'ptySpawnHealth':
|
||||
await this.ptySpawnHealthCheck()
|
||||
return { healthy: true }
|
||||
|
||||
case 'shutdown':
|
||||
if (request.payload.killSessions) {
|
||||
this.host.dispose()
|
||||
|
|
|
|||
|
|
@ -1,19 +1,13 @@
|
|||
/* oxlint-disable max-lines -- Why: exercises full PTY subprocess surface (spawn setup, signal routing, data events, platform-specific shell configs, and Windows PowerShell implementations) with co-located test scenarios to prevent fixture drift. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, realpathSync, rmSync, writeFileSync } from 'fs'
|
||||
import { mkdtempSync, realpathSync, rmSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import type * as LocalPtyUtils from '../providers/local-pty-utils'
|
||||
|
||||
const {
|
||||
spawnMock,
|
||||
isPwshAvailableMock,
|
||||
validateWorkingDirectoryMock,
|
||||
getNodePtySpawnHelperCandidatesMock
|
||||
} = vi.hoisted(() => ({
|
||||
const { spawnMock, isPwshAvailableMock, validateWorkingDirectoryMock } = vi.hoisted(() => ({
|
||||
spawnMock: vi.fn(),
|
||||
isPwshAvailableMock: vi.fn(),
|
||||
getNodePtySpawnHelperCandidatesMock: vi.fn(),
|
||||
validateWorkingDirectoryMock: vi.fn((cwd: string) => {
|
||||
if (cwd.includes('definitely-missing')) {
|
||||
throw new Error(
|
||||
|
|
@ -35,12 +29,11 @@ vi.mock('../providers/local-pty-utils', async (importOriginal) => {
|
|||
const actual = await importOriginal<typeof LocalPtyUtils>()
|
||||
return {
|
||||
...actual,
|
||||
getNodePtySpawnHelperCandidates: getNodePtySpawnHelperCandidatesMock,
|
||||
validateWorkingDirectory: validateWorkingDirectoryMock
|
||||
}
|
||||
})
|
||||
|
||||
import { checkPtySpawnHealth, createPtySubprocess } from './pty-subprocess'
|
||||
import { createPtySubprocess } from './pty-subprocess'
|
||||
|
||||
const ORCA_SHELL_WRAPPER_ENV = [
|
||||
'ORCA_ATTRIBUTION_SHIM_DIR',
|
||||
|
|
@ -86,10 +79,6 @@ describe('createPtySubprocess', () => {
|
|||
isPwshAvailableMock.mockReturnValue(false)
|
||||
previousUserDataPath = process.env.ORCA_USER_DATA_PATH
|
||||
userDataPath = mkdtempSync(join(tmpdir(), 'daemon-pty-subprocess-test-'))
|
||||
const spawnHelperPath = join(userDataPath, 'spawn-helper')
|
||||
writeFileSync(spawnHelperPath, '')
|
||||
getNodePtySpawnHelperCandidatesMock.mockReset()
|
||||
getNodePtySpawnHelperCandidatesMock.mockReturnValue([spawnHelperPath])
|
||||
process.env.ORCA_USER_DATA_PATH = userDataPath
|
||||
for (const key of ORCA_SHELL_WRAPPER_ENV) {
|
||||
savedWrapperEnv[key] = process.env[key]
|
||||
|
|
@ -180,54 +169,6 @@ describe('createPtySubprocess', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('checks macOS PTY spawn health with a short-lived shell', async () => {
|
||||
const proc = mockPtyProcess()
|
||||
spawnMock.mockReturnValue(proc)
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'darwin' })
|
||||
|
||||
try {
|
||||
const result = checkPtySpawnHealth()
|
||||
proc._simulateExit(0)
|
||||
await expect(result).resolves.toBeUndefined()
|
||||
expect(proc.kill).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'/bin/sh',
|
||||
['-c', 'exit 0'],
|
||||
expect.objectContaining({
|
||||
cols: 2,
|
||||
rows: 1,
|
||||
cwd: userDataPath,
|
||||
name: 'xterm-256color'
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('surfaces stale node-pty helper failures during macOS PTY spawn health', async () => {
|
||||
spawnMock.mockImplementation(() => {
|
||||
throw new Error(
|
||||
"node-pty: posix_spawn failed: ENOENT (errno 2, No such file or directory) - helper='/tmp/deleted/spawn-helper'"
|
||||
)
|
||||
})
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'darwin' })
|
||||
|
||||
try {
|
||||
await expect(checkPtySpawnHealth()).rejects.toThrow('Daemon failed to spawn shell "/bin/sh"')
|
||||
await expect(checkPtySpawnHealth()).rejects.toThrow('posix_spawn failed: ENOENT')
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('returns a SubprocessHandle with correct pid', () => {
|
||||
const proc = mockPtyProcess(42)
|
||||
spawnMock.mockReturnValue(proc)
|
||||
|
|
|
|||
|
|
@ -29,7 +29,6 @@ import { isWindowsGitBashShellPath, resolveWindowsGitBashShellPath } from '../gi
|
|||
import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell'
|
||||
|
||||
const PANE_IDENTITY_ENV_KEYS = ['ORCA_PANE_KEY', 'ORCA_TAB_ID', 'ORCA_WORKTREE_ID'] as const
|
||||
const PTY_SPAWN_HEALTH_TIMEOUT_MS = 2_000
|
||||
|
||||
export type PtySubprocessOptions = {
|
||||
sessionId: string
|
||||
|
|
@ -232,75 +231,6 @@ function formatPtySpawnError(err: unknown, shellPath: string, spawnCwd: string):
|
|||
return formatted
|
||||
}
|
||||
|
||||
export async function checkPtySpawnHealth(): Promise<void> {
|
||||
if (process.platform !== 'darwin') {
|
||||
return
|
||||
}
|
||||
|
||||
ensureNodePtySpawnHelperExecutable()
|
||||
preflightMacNodePtySpawnEnvironment()
|
||||
|
||||
const cwd = isExistingDirectory(process.env.ORCA_USER_DATA_PATH)
|
||||
? process.env.ORCA_USER_DATA_PATH
|
||||
: getDefaultCwd()
|
||||
|
||||
let proc: pty.IPty
|
||||
try {
|
||||
proc = pty.spawn('/bin/sh', ['-c', 'exit 0'], {
|
||||
name: 'xterm-256color',
|
||||
cols: 2,
|
||||
rows: 1,
|
||||
cwd,
|
||||
env: {
|
||||
...process.env,
|
||||
TERM: 'xterm-256color'
|
||||
}
|
||||
})
|
||||
} catch (err) {
|
||||
throw formatPtySpawnError(err, '/bin/sh', cwd)
|
||||
}
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
let settled = false
|
||||
let exitDisposable: { dispose(): void } | undefined
|
||||
const finish = (error?: Error, opts?: { kill?: boolean }): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
clearTimeout(timer)
|
||||
exitDisposable?.dispose()
|
||||
if (opts?.kill) {
|
||||
try {
|
||||
proc.kill()
|
||||
} catch {
|
||||
// Best-effort cleanup for a short-lived health probe.
|
||||
}
|
||||
}
|
||||
if (error) {
|
||||
reject(error)
|
||||
return
|
||||
}
|
||||
resolve()
|
||||
}
|
||||
const timer = setTimeout(() => {
|
||||
finish(new Error(`PTY spawn health check timed out after ${PTY_SPAWN_HEALTH_TIMEOUT_MS}ms`), {
|
||||
kill: true
|
||||
})
|
||||
}, PTY_SPAWN_HEALTH_TIMEOUT_MS)
|
||||
|
||||
// Why: ping only proves the daemon protocol is alive. A real short-lived
|
||||
// PTY spawn catches stale node-pty helper paths captured by this process.
|
||||
exitDisposable = proc.onExit(({ exitCode }) => {
|
||||
if (exitCode === 0) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
finish(new Error(`PTY spawn health check exited with code ${exitCode}`))
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
function normalizeForegroundProcessName(processName: string | null | undefined): string | null {
|
||||
const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? ''
|
||||
if (!trimmed || trimmed === 'xterm-256color') {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,126 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { connect, createServer, type Server, type Socket } from 'net'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { mkdtempSync, rmSync } from 'fs'
|
||||
import { DaemonServer } from './daemon-server'
|
||||
import { DaemonClient } from './client'
|
||||
import { healthCheckDaemon } from './daemon-health'
|
||||
import type { ListSessionsResult } from './types'
|
||||
import type { SubprocessHandle } from './session'
|
||||
|
||||
// Why: terminals were lost after app updates because a busy machine could
|
||||
// time out the 3s startup health check against a daemon that was alive and
|
||||
// owning sessions, and the unhealthy path killed it. The fix re-verifies
|
||||
// with listSessions, which has far larger budgets (5s hello, 30s request).
|
||||
// This test reproduces the production asymmetry against a REAL daemon by
|
||||
// inserting a response delay that exceeds the health-check budget but fits
|
||||
// the verification budgets, and asserts the guard's two inputs disagree the
|
||||
// way the fix depends on.
|
||||
const RESPONSE_DELAY_MS = 3_500
|
||||
|
||||
function createMockSubprocess(): SubprocessHandle {
|
||||
return {
|
||||
pid: 55555,
|
||||
getForegroundProcess: vi.fn(() => null),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
kill: vi.fn(),
|
||||
forceKill: vi.fn(),
|
||||
signal: vi.fn(),
|
||||
onData: vi.fn(),
|
||||
onExit: vi.fn(),
|
||||
dispose: vi.fn()
|
||||
}
|
||||
}
|
||||
|
||||
/** Forwards client bytes to the daemon immediately, but delays every daemon
|
||||
* response so each round-trip looks like a daemon under heavy load. */
|
||||
function startDelayProxy(listenPath: string, upstreamPath: string): Server {
|
||||
const proxy = createServer((clientSocket: Socket) => {
|
||||
const upstream = connect(upstreamPath)
|
||||
clientSocket.on('data', (chunk) => upstream.write(chunk))
|
||||
upstream.on('data', (chunk) => {
|
||||
setTimeout(() => {
|
||||
if (!clientSocket.destroyed) {
|
||||
clientSocket.write(chunk)
|
||||
}
|
||||
}, RESPONSE_DELAY_MS)
|
||||
})
|
||||
const teardown = (): void => {
|
||||
clientSocket.destroy()
|
||||
upstream.destroy()
|
||||
}
|
||||
clientSocket.on('close', teardown)
|
||||
clientSocket.on('error', teardown)
|
||||
upstream.on('close', () => {
|
||||
setTimeout(teardown, RESPONSE_DELAY_MS)
|
||||
})
|
||||
upstream.on('error', teardown)
|
||||
})
|
||||
proxy.listen(listenPath)
|
||||
return proxy
|
||||
}
|
||||
|
||||
describe('slow daemon session verification', () => {
|
||||
let dir: string
|
||||
let daemonSocketPath: string
|
||||
let proxySocketPath: string
|
||||
let tokenPath: string
|
||||
let server: DaemonServer
|
||||
let proxy: Server
|
||||
const clients: DaemonClient[] = []
|
||||
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), 'daemon-slow-verification-test-'))
|
||||
daemonSocketPath = join(dir, 'daemon.sock')
|
||||
proxySocketPath = join(dir, 'proxy.sock')
|
||||
tokenPath = join(dir, 'daemon.token')
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
for (const client of clients.splice(0)) {
|
||||
client.disconnect()
|
||||
}
|
||||
await new Promise<void>((resolve) => proxy?.close(() => resolve()))
|
||||
await server?.shutdown()
|
||||
rmSync(dir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it(
|
||||
'fails the 3s health check against a slow daemon while listSessions still verifies its live session',
|
||||
{ timeout: 60_000 },
|
||||
async () => {
|
||||
server = new DaemonServer({
|
||||
socketPath: daemonSocketPath,
|
||||
tokenPath,
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
|
||||
const directClient = new DaemonClient({ socketPath: daemonSocketPath, tokenPath })
|
||||
clients.push(directClient)
|
||||
await directClient.ensureConnected()
|
||||
await directClient.request('createOrAttach', {
|
||||
sessionId: 'wt-1@@live-session',
|
||||
cols: 80,
|
||||
rows: 24
|
||||
})
|
||||
|
||||
proxy = startDelayProxy(proxySocketPath, daemonSocketPath)
|
||||
|
||||
// The exact pre-fix kill trigger: the daemon is alive but too slow for
|
||||
// the health-check budget.
|
||||
await expect(healthCheckDaemon(proxySocketPath, tokenPath)).resolves.toBe(false)
|
||||
|
||||
// The fix's re-verification against the SAME slow daemon: the larger
|
||||
// client budgets absorb the latency and prove the session is alive.
|
||||
const verificationClient = new DaemonClient({ socketPath: proxySocketPath, tokenPath })
|
||||
clients.push(verificationClient)
|
||||
await verificationClient.ensureConnected()
|
||||
const result = await verificationClient.request<ListSessionsResult>('listSessions', undefined)
|
||||
const liveSessionCount = result.sessions.filter((session) => session.isAlive).length
|
||||
expect(liveSessionCount).toBe(1)
|
||||
}
|
||||
)
|
||||
})
|
||||
|
|
@ -182,11 +182,6 @@ export type SystemResolverHealthRequest = {
|
|||
type: 'systemResolverHealth'
|
||||
}
|
||||
|
||||
export type PtySpawnHealthRequest = {
|
||||
id: string
|
||||
type: 'ptySpawnHealth'
|
||||
}
|
||||
|
||||
export type GetSnapshotRequest = {
|
||||
id: string
|
||||
type: 'getSnapshot'
|
||||
|
|
@ -210,7 +205,6 @@ export type DaemonRequest =
|
|||
| ShutdownRequest
|
||||
| PingRequest
|
||||
| SystemResolverHealthRequest
|
||||
| PtySpawnHealthRequest
|
||||
| GetSnapshotRequest
|
||||
|
||||
// ─── RPC Responses (Daemon → Client, on control socket) ────────────
|
||||
|
|
|
|||
|
|
@ -0,0 +1,135 @@
|
|||
import { existsSync, readFileSync } from 'fs'
|
||||
import path from 'path'
|
||||
import type { ElectronApplication } from '@stablyai/playwright-test'
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import { TEST_REPO_PATH_FILE } from './global-setup'
|
||||
import {
|
||||
discoverActivePtyId,
|
||||
execInTerminal,
|
||||
getTerminalContent,
|
||||
waitForActiveTerminalManager,
|
||||
waitForPaneCount,
|
||||
waitForTerminalOutput
|
||||
} from './helpers/terminal'
|
||||
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart'
|
||||
import { PROTOCOL_VERSION } from '../../src/main/daemon/types'
|
||||
import { PTY_SESSION_ID_SEPARATOR } from '../../src/shared/pty-session-id-format'
|
||||
|
||||
// Why: must land after the relaunched app's 3s daemon health check has timed
|
||||
// out (so the unhealthy guard runs) but before the guard's 5s client hello
|
||||
// budget expires. Daemon init starts within the first ~2s of main startup.
|
||||
const RESUME_DAEMON_AFTER_MS = 6_500
|
||||
|
||||
function readDaemonPid(userDataDir: string): number {
|
||||
const raw = readFileSync(
|
||||
path.join(userDataDir, 'daemon', `daemon-v${PROTOCOL_VERSION}.pid`),
|
||||
'utf8'
|
||||
)
|
||||
const parsed = JSON.parse(raw) as { pid?: unknown }
|
||||
if (typeof parsed.pid !== 'number') {
|
||||
throw new Error(`Daemon pid file did not contain a numeric pid: ${raw}`)
|
||||
}
|
||||
return parsed.pid
|
||||
}
|
||||
|
||||
test.describe.configure({ mode: 'serial' })
|
||||
|
||||
test('preserves a live daemon PTY when the daemon is too slow for the startup health check', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set.
|
||||
{}, testInfo) => {
|
||||
const repoPath = readFileSync(TEST_REPO_PATH_FILE, 'utf-8').trim()
|
||||
if (!repoPath || !existsSync(repoPath)) {
|
||||
test.skip(true, 'Global setup did not produce a seeded test repo')
|
||||
return
|
||||
}
|
||||
test.skip(process.platform === 'win32', 'SIGSTOP/SIGCONT are POSIX-only')
|
||||
|
||||
const session = createRestartSession(testInfo)
|
||||
let firstApp: ElectronApplication | null = null
|
||||
let secondApp: ElectronApplication | null = null
|
||||
let daemonPid: number | null = null
|
||||
|
||||
try {
|
||||
const firstLaunch = await session.launch()
|
||||
firstApp = firstLaunch.app
|
||||
const page = await firstApp.firstWindow()
|
||||
const worktreeId = await attachRepoAndOpenTerminal(page, repoPath)
|
||||
await waitForSessionReady(page)
|
||||
await waitForActiveWorktree(page)
|
||||
await ensureTerminalVisible(page)
|
||||
await waitForActiveTerminalManager(page, 30_000)
|
||||
await waitForPaneCount(page, 1, 30_000)
|
||||
const ptyId = await discoverActivePtyId(page)
|
||||
expect(ptyId).toContain(PTY_SESSION_ID_SEPARATOR)
|
||||
|
||||
const marker = `DAEMON_SLOW_HEALTH_PRESERVE_${Date.now()}`
|
||||
await execInTerminal(firstLaunch.page, ptyId, `echo ${marker}`)
|
||||
await waitForTerminalOutput(firstLaunch.page, marker)
|
||||
|
||||
daemonPid = readDaemonPid(session.userDataDir)
|
||||
|
||||
await session.close(firstApp)
|
||||
firstApp = null
|
||||
|
||||
// Why: a stopped daemon still accepts socket connections at the kernel
|
||||
// level but answers nothing — the same observable behavior as a daemon
|
||||
// that is too busy to respond within the health-check budget.
|
||||
process.kill(daemonPid, 'SIGSTOP')
|
||||
|
||||
const stderrLines: string[] = []
|
||||
const resumeTimer = setTimeout(() => {
|
||||
if (daemonPid !== null) {
|
||||
process.kill(daemonPid, 'SIGCONT')
|
||||
}
|
||||
}, RESUME_DAEMON_AFTER_MS)
|
||||
try {
|
||||
const secondLaunch = await session.launch()
|
||||
secondApp = secondLaunch.app
|
||||
secondApp.process().stderr?.on('data', (chunk: Buffer) => {
|
||||
stderrLines.push(chunk.toString())
|
||||
})
|
||||
|
||||
await waitForSessionReady(secondLaunch.page)
|
||||
await expect
|
||||
.poll(
|
||||
async () => secondLaunch.page.evaluate(() => window.__store?.getState().activeWorktreeId),
|
||||
{ timeout: 15_000 }
|
||||
)
|
||||
.toBe(worktreeId)
|
||||
await ensureTerminalVisible(secondLaunch.page)
|
||||
await waitForActiveTerminalManager(secondLaunch.page, 30_000)
|
||||
await waitForPaneCount(secondLaunch.page, 1, 30_000)
|
||||
await waitForTerminalOutput(secondLaunch.page, marker, 20_000)
|
||||
|
||||
// The guard path must actually have run: the daemon failed the health
|
||||
// check and was preserved because its live session was verified.
|
||||
await expect
|
||||
.poll(() => stderrLines.join(''), { timeout: 10_000 })
|
||||
.toContain('Preserving daemon that failed the health check')
|
||||
expect(readDaemonPid(session.userDataDir)).toBe(daemonPid)
|
||||
// Why: a killed daemon cold-restores scrollback from history, so the
|
||||
// marker text alone cannot distinguish a live session from a dead one.
|
||||
// The restore banner only appears for cold-restored (dead) sessions.
|
||||
expect(await getTerminalContent(secondLaunch.page)).not.toContain('--- session restored ---')
|
||||
} finally {
|
||||
clearTimeout(resumeTimer)
|
||||
}
|
||||
} finally {
|
||||
if (daemonPid !== null) {
|
||||
try {
|
||||
// Idempotent: ensures the daemon is resumable for harness cleanup even
|
||||
// if the test failed before the resume timer fired.
|
||||
process.kill(daemonPid, 'SIGCONT')
|
||||
} catch {
|
||||
// Daemon already gone
|
||||
}
|
||||
}
|
||||
if (secondApp) {
|
||||
await session.close(secondApp)
|
||||
}
|
||||
if (firstApp) {
|
||||
await session.close(firstApp)
|
||||
}
|
||||
await session.dispose()
|
||||
}
|
||||
})
|
||||
Loading…
Reference in New Issue