diff --git a/packages/cli/src/utils/relaunch.test.ts b/packages/cli/src/utils/relaunch.test.ts index 255671e27f..2022e882fb 100644 --- a/packages/cli/src/utils/relaunch.test.ts +++ b/packages/cli/src/utils/relaunch.test.ts @@ -314,6 +314,120 @@ describe('relaunchAppInChildProcess', () => { // Should default to exit code 1 expect(processExitSpy).toHaveBeenCalledWith(1); }); + + it('should forward termination signals to the child and clean up listeners on close', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const FORWARDED: NodeJS.Signals[] = [ + 'SIGTERM', + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGUSR1', + 'SIGUSR2', + ]; + const baseline = Object.fromEntries( + FORWARDED.map((s) => [s, process.listenerCount(s)]), + ) as Record; + + const mockChild = createMockChildProcess(0, false); + const listenerCountsAfterSpawn: Record = {}; + + mockedSpawn.mockImplementation(() => { + // Defer so the caller has wired up its listeners first. + setImmediate(() => { + for (const sig of FORWARDED) { + listenerCountsAfterSpawn[sig] = process.listenerCount(sig); + } + // Trigger SIGTERM forwarder. + process.emit('SIGTERM'); + // Then close the child so the promise resolves. + setImmediate(() => mockChild.emit('close', 0)); + }); + return mockChild; + }); + + await expect(relaunchAppInChildProcess([], [])).rejects.toThrow( + 'PROCESS_EXIT_CALLED', + ); + + // Each forwarded signal gained exactly one listener while the child was alive. + for (const sig of FORWARDED) { + expect(listenerCountsAfterSpawn[sig]).toBe(baseline[sig] + 1); + } + // SIGTERM was forwarded to the child. + expect(mockChild.kill).toHaveBeenCalledWith('SIGTERM'); + // After child close, listener counts returned to baseline. + for (const sig of FORWARDED) { + expect(process.listenerCount(sig)).toBe(baseline[sig]); + } + }); + + it('should not leak signal listeners when child.send throws synchronously', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const FORWARDED: NodeJS.Signals[] = [ + 'SIGTERM', + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGUSR1', + 'SIGUSR2', + ]; + const baseline = Object.fromEntries( + FORWARDED.map((s) => [s, process.listenerCount(s)]), + ) as Record; + + const mockChild = createMockChildProcess(0, false); + (mockChild.send as unknown as MockInstance).mockImplementation(() => { + throw new Error('IPC send failed'); + }); + mockedSpawn.mockImplementation(() => mockChild); + + await expect( + relaunchAppInChildProcess([], [], { + isReadonly: false, + } as unknown as Parameters[2]), + ).rejects.toThrow('PROCESS_EXIT_CALLED'); + + // Listeners are attached AFTER child.send(); a synchronous throw from + // child.send must not leave forwarders registered on the parent. + for (const sig of FORWARDED) { + expect(process.listenerCount(sig)).toBe(baseline[sig]); + } + }); + + it('should clean up signal listeners on child process error', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const FORWARDED: NodeJS.Signals[] = [ + 'SIGTERM', + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGUSR1', + 'SIGUSR2', + ]; + const baseline = Object.fromEntries( + FORWARDED.map((s) => [s, process.listenerCount(s)]), + ) as Record; + + const mockChild = createMockChildProcess(0, false); + mockedSpawn.mockImplementation(() => { + setImmediate(() => { + mockChild.emit('error', new Error('spawn failed')); + }); + return mockChild; + }); + + await expect(relaunchAppInChildProcess([], [])).rejects.toThrow( + 'PROCESS_EXIT_CALLED', + ); + + for (const sig of FORWARDED) { + expect(process.listenerCount(sig)).toBe(baseline[sig]); + } + }); }); }); diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 7e287e4565..48638f676e 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -65,10 +65,51 @@ export async function relaunchAppInChildProcess( env: newEnv, }); + // Forward termination signals so the whole process tree exits cleanly + // rather than orphaning the child under PID 1 / the user's systemd + // manager. Programmatic signals (e.g. `kill -INT `, + // systemd/container runtimes, supervising ACP clients) target only the + // parent and would otherwise leave the child orphaned. Terminal-generated + // signals (Ctrl+C, Ctrl+\) are already delivered to the whole foreground + // process group by the terminal; the resulting double-delivery is + // harmless for typical signal handlers and strictly preferable to + // orphaning. Waiting for the child to exit via the installed listeners + // also keeps the terminal state clean by preventing the shell prompt + // from returning mid-cleanup. + const FORWARDED_SIGNALS: NodeJS.Signals[] = [ + 'SIGTERM', + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGUSR1', + 'SIGUSR2', + ]; + const forwarders = new Map void>(); + const removeForwarders = () => { + for (const [sig, handler] of forwarders) { + process.off(sig, handler); + } + forwarders.clear(); + }; + if (latestAdminSettings) { child.send({ type: 'admin-settings', settings: latestAdminSettings }); } + // Attach listeners only after any synchronous IPC setup that could throw, + // so a thrown child.send() doesn't leak listeners on the parent process. + for (const sig of FORWARDED_SIGNALS) { + const handler = () => { + try { + child.kill(sig); + } catch { + // Child may already be gone; ignore. + } + }; + forwarders.set(sig, handler); + process.on(sig, handler); + } + child.on('message', (msg: { type?: string; settings?: unknown }) => { if (msg.type === 'admin-settings-update' && msg.settings) { latestAdminSettings = msg.settings as AdminControlsSettings; @@ -76,8 +117,12 @@ export async function relaunchAppInChildProcess( }); return new Promise((resolve, reject) => { - child.on('error', reject); + child.on('error', (err) => { + removeForwarders(); + reject(err); + }); child.on('close', (code) => { + removeForwarders(); // Resume stdin before the parent process exits. process.stdin.resume(); resolve(code ?? 1);