diff --git a/src/cli/operations/dev/__tests__/dev-server.test.ts b/src/cli/operations/dev/__tests__/dev-server.test.ts index b6ca6085b..3e00e0c14 100644 --- a/src/cli/operations/dev/__tests__/dev-server.test.ts +++ b/src/cli/operations/dev/__tests__/dev-server.test.ts @@ -1,7 +1,7 @@ import type { DevConfig } from '../config.js'; import { DevServer, type DevServerCallbacks, type DevServerOptions, type SpawnConfig } from '../dev-server.js'; import { EventEmitter } from 'events'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { type MockInstance, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockSpawn = vi.fn(); vi.mock('child_process', () => ({ @@ -56,6 +56,8 @@ describe('DevServer', () => { let options: DevServerOptions; let server: TestDevServer; let mockChild: ReturnType; + let onceSpy: MockInstance; + let removeListenerSpy: MockInstance; beforeEach(() => { onLog = vi.fn(); @@ -65,12 +67,20 @@ describe('DevServer', () => { server = new TestDevServer(config, options); mockChild = createMockChildProcess(); mockSpawn.mockReturnValue(mockChild); + onceSpy = vi.spyOn(process, 'once').mockReturnValue(process); + removeListenerSpy = vi.spyOn(process, 'removeListener').mockReturnValue(process); }); afterEach(() => { + vi.restoreAllMocks(); vi.clearAllMocks(); }); + function getRegisteredExitHandler(): (() => void) | undefined { + const call = onceSpy.mock.calls.find(([event]) => event === 'exit'); + return call?.[1] as (() => void) | undefined; + } + describe('start()', () => { it('calls spawn with correct cmd, args, cwd, env, and stdio when prepare succeeds', async () => { await server.start(); @@ -173,6 +183,47 @@ describe('DevServer', () => { }); }); + describe('exit cleanup', () => { + it('reaps the detached process group on process exit', async () => { + mockChild.pid = 4242; + const processKillSpy = vi.spyOn(process, 'kill').mockImplementation(() => true); + + await server.start(); + const exitHandler = getRegisteredExitHandler(); + expect(exitHandler).toBeDefined(); + + exitHandler!(); + expect(processKillSpy).toHaveBeenCalledWith(-4242, 'SIGKILL'); + }); + + it('does not register an exit reaper when the child has no pid', async () => { + await server.start(); + expect(getRegisteredExitHandler()).toBeUndefined(); + }); + + it('removes the exit reaper once the child exits on its own', async () => { + mockChild.pid = 4242; + + await server.start(); + const exitHandler = getRegisteredExitHandler(); + mockChild.emit('exit', 0); + + expect(removeListenerSpy).toHaveBeenCalledWith('exit', exitHandler); + }); + + it('swallows errors when the process group is already gone', async () => { + mockChild.pid = 4242; + vi.spyOn(process, 'kill').mockImplementation(() => { + throw new Error('kill ESRCH'); + }); + + await server.start(); + const exitHandler = getRegisteredExitHandler(); + + expect(() => exitHandler!()).not.toThrow(); + }); + }); + describe('output routing', () => { it('forwards stdout lines to onLog at info level', async () => { await server.start(); diff --git a/src/cli/operations/dev/dev-server.ts b/src/cli/operations/dev/dev-server.ts index d44d4c765..3d208eb51 100644 --- a/src/cli/operations/dev/dev-server.ts +++ b/src/cli/operations/dev/dev-server.ts @@ -48,6 +48,7 @@ function isInternalFrame(line: string): boolean { export abstract class DevServer { protected child: ChildProcess | null = null; + private onProcessExit: (() => void) | null = null; private recentStderr: string[] = []; private inTraceback = false; private tracebackBuffer: string[] = []; @@ -77,9 +78,38 @@ export abstract class DevServer { }); this.attachHandlers(); + this.registerExitCleanup(); return this.child; } + /** + * Reap the detached child's process group when this process exits. + * + * The dev command's SIGINT/SIGTERM handler (which calls kill()) is registered after the + * global handler in setupAltScreenCleanup, which runs first and calls process.exit(0) — so + * kill() never runs and the child (its own process group since it is spawned detached) is + * orphaned, holding the port. The 'exit' event always fires on process.exit(), so use it as + * a last-resort reaper. POSIX only: on Windows the child is not detached and dies with the parent. + */ + private registerExitCleanup(): void { + const pid = this.child?.pid; + if (isWindows || !pid) return; + this.onProcessExit = () => { + try { + process.kill(-pid, 'SIGKILL'); + } catch { + // Group already gone — nothing to reap. + } + }; + process.once('exit', this.onProcessExit); + } + + private clearExitCleanup(): void { + if (!this.onProcessExit) return; + process.removeListener('exit', this.onProcessExit); + this.onProcessExit = null; + } + /** Kill the dev server process tree. Sends SIGTERM to the process group, then SIGKILL after 2 seconds. */ kill(): void { if (!this.child || this.child.killed) return; @@ -203,6 +233,7 @@ export abstract class DevServer { }); this.child?.on('exit', code => { + this.clearExitCleanup(); if (code !== 0 && code !== null && this.recentStderr.length > 0) { for (const line of this.recentStderr) { onLog('error', line);