fix: clean daemon init startup listeners (#2884)

* fix: clean daemon init startup listeners

* test: cover daemon startup listener cleanup
This commit is contained in:
Neil 2026-05-26 20:27:47 -07:00 committed by GitHub
parent efcc0c315f
commit dafc35bbc8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 139 additions and 13 deletions

View File

@ -656,6 +656,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
}
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()
}
@ -705,6 +709,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
}
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()
}
@ -754,6 +762,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
}
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()
}
@ -768,6 +780,96 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
)
})
it('removes detached daemon startup listeners after readiness', async () => {
healthCheckDaemonMock.mockResolvedValueOnce(false)
const mod = await importFresh()
await mod.initDaemonPtyProvider()
const launcher = spawnerInstances[0].launcher as (
socketPath: string,
tokenPath: string
) => Promise<{ shutdown(): Promise<void> }>
const handlers: Record<string, ((arg?: unknown) => void)[]> = {
message: [],
error: [],
exit: []
}
const offMock = vi.fn((event: string, cb: (arg?: unknown) => void) => {
handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? []
return child
})
const child = {
pid: 12345,
on(event: string, cb: (arg?: unknown) => void) {
handlers[event]?.push(cb)
if (event === 'message') {
queueMicrotask(() => cb({ type: 'ready' }))
}
return this
},
off: offMock,
disconnect: vi.fn(),
unref: vi.fn()
}
forkMock.mockReturnValueOnce(child)
await launcher('/fake/socket', '/fake/token')
expect(offMock).toHaveBeenCalledWith('message', expect.any(Function))
expect(offMock).toHaveBeenCalledWith('error', expect.any(Function))
expect(offMock).toHaveBeenCalledWith('exit', expect.any(Function))
expect(handlers.message).toHaveLength(0)
expect(handlers.error).toHaveLength(0)
expect(handlers.exit).toHaveLength(0)
expect(child.disconnect).toHaveBeenCalledOnce()
expect(child.unref).toHaveBeenCalledOnce()
})
it('removes detached daemon startup listeners after startup error', async () => {
healthCheckDaemonMock.mockResolvedValueOnce(false)
const mod = await importFresh()
await mod.initDaemonPtyProvider()
const launcher = spawnerInstances[0].launcher as (
socketPath: string,
tokenPath: string
) => Promise<{ shutdown(): Promise<void> }>
const handlers: Record<string, ((arg?: unknown) => void)[]> = {
message: [],
error: [],
exit: []
}
const offMock = vi.fn((event: string, cb: (arg?: unknown) => void) => {
handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? []
return child
})
const child = {
pid: undefined,
on(event: string, cb: (arg?: unknown) => void) {
handlers[event]?.push(cb)
if (event === 'error') {
queueMicrotask(() => cb(new Error('startup failed')))
}
return this
},
off: offMock,
disconnect: vi.fn(),
unref: vi.fn()
}
forkMock.mockReturnValueOnce(child)
await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow('startup failed')
expect(offMock).toHaveBeenCalledWith('message', expect.any(Function))
expect(offMock).toHaveBeenCalledWith('error', expect.any(Function))
expect(offMock).toHaveBeenCalledWith('exit', expect.any(Function))
expect(handlers.message).toHaveLength(0)
expect(handlers.error).toHaveLength(0)
expect(handlers.exit).toHaveLength(0)
expect(child.disconnect).not.toHaveBeenCalled()
expect(child.unref).not.toHaveBeenCalled()
})
it('keeps packaged healthy-daemon reuse independent of dev app-path identity', async () => {
const mod = await importFresh()
await mod.initDaemonPtyProvider()

View File

@ -157,8 +157,22 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
// Wait for the daemon to signal readiness via IPC
await new Promise<void>((resolve, reject) => {
const fail = (error: Error): void => {
clearTimeout(timer)
let timer: ReturnType<typeof setTimeout> | undefined
let settled = false
function cleanupStartupListeners(): void {
if (timer) {
clearTimeout(timer)
}
child.off('message', onReadyMessage)
child.off('error', onStartupError)
child.off('exit', onStartupExit)
}
function fail(error: Error): void {
if (settled) {
return
}
settled = true
cleanupStartupListeners()
if (child.pid) {
try {
process.kill(child.pid, 'SIGTERM')
@ -168,13 +182,15 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
}
reject(error)
}
const timer = setTimeout(() => {
fail(new Error('Daemon startup timed out'))
}, 10000)
child.on('message', (msg: unknown) => {
function onReadyMessage(msg: unknown): void {
if (msg && typeof msg === 'object' && (msg as { type?: string }).type === 'ready') {
clearTimeout(timer)
if (settled) {
return
}
settled = true
// Why: the daemon process is detached after readiness; leaving
// startup listeners attached retains this launch promise closure.
cleanupStartupListeners()
if (child.pid) {
// Why: JSON pid file carries pid + process start time so later
// killStaleDaemon() can verify the pid still belongs to the daemon
@ -196,15 +212,23 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
child.unref()
resolve()
}
})
}
child.on('error', (err) => {
function onStartupError(err: Error): void {
fail(err)
})
}
child.on('exit', (code) => {
function onStartupExit(code: number | null): void {
fail(new Error(`Daemon exited during startup with code ${code}`))
})
}
timer = setTimeout(() => {
fail(new Error('Daemon startup timed out'))
}, 10000)
child.on('message', onReadyMessage)
child.on('error', onStartupError)
child.on('exit', onStartupExit)
})
return {