From a4e55ba4eb3222c06780c02c5e1dba1537eabb16 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Tue, 4 Aug 2026 11:01:21 +0800 Subject: [PATCH 1/9] fix(cli): forward termination signals to relaunched child process relaunchAppInChildProcess spawned the child via spawn() with stdio inherit + ipc but installed no signal handlers on the bootstrap parent. When a supervising process (ACP client, systemd, container runtime) sent SIGTERM/SIGHUP to the parent, the parent died on its default disposition while the child was reparented to PID 1 and kept running - holding the OAuth session and allocated heap until killed manually. Interactive Ctrl+C did not surface the bug because SIGINT is delivered to the whole foreground process group via the controlling terminal; the orphan only manifests with programmatic kill(pid). Install forwarders for SIGTERM, SIGHUP, SIGINT, SIGQUIT, SIGUSR1, and SIGUSR2 that proxy the signal to child.kill(sig) before awaiting the child. Remove them on both close and error via a Map of {signal -> handler} so cleanup is precise (removeAllListeners would disturb unrelated subscribers) and does not leak a handler per relaunch iteration (which trips MaxListenersExceededWarning after ~10 relaunches). child.kill is guarded for the race where the signal arrives just after the child exits. Fixes #25590 --- packages/cli/src/utils/relaunch.test.ts | 33 ++++++++++++++++++ packages/cli/src/utils/relaunch.ts | 45 ++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/relaunch.test.ts b/packages/cli/src/utils/relaunch.test.ts index 3a6de661229..3ba856393ab 100644 --- a/packages/cli/src/utils/relaunch.test.ts +++ b/packages/cli/src/utils/relaunch.test.ts @@ -345,6 +345,39 @@ describe('relaunchAppInChildProcess', () => { // Should default to exit code 1 expect(processExitSpy).toHaveBeenCalledWith(1); }); + + it('forwards termination signals to the child and removes them on close (#25590)', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const mockChild = createMockChildProcess(0, false); + mockedSpawn.mockImplementation(() => mockChild); + + const before = process.listenerCount('SIGTERM'); + const killSpy = mockChild.kill as ReturnType; + + // Start the relaunch process (does not auto-close, so it awaits). + const promise = relaunchAppInChildProcess([], []); + + // Drain microtasks so the spawn + forwarder registration runs. + await new Promise((r) => setImmediate(r)); + + // A forwarder for SIGTERM was installed on the parent process. + expect(process.listenerCount('SIGTERM')).toBe(before + 1); + + // Emulate the OS delivering SIGTERM: invoke the registered handler + // directly (process.emit would also fire unrelated listeners). + const handler = process.listeners('SIGTERM').at(-1) as + | (() => void) + | undefined; + expect(handler).toBeDefined(); + handler!(); + expect(killSpy).toHaveBeenCalledWith('SIGTERM'); + + // Closing the child must remove the forwarder (no listener leak). + mockChild.emit('close', 0); + await expect(promise).rejects.toThrow('PROCESS_EXIT_CALLED'); + expect(process.listenerCount('SIGTERM')).toBe(before); + }); }); }); diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 752918539bc..a25f2645557 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -15,6 +15,20 @@ import { type AdminControlsSettings, } from '@google/gemini-cli-core'; +// Signals a supervising process (ACP client, systemd, container runtime) +// may send to the bootstrap parent. Without forwarding, the parent dies on +// its default disposition while the spawned child is reparented to PID 1 +// and keeps running - holding the OAuth session and allocated heap until +// killed manually. See #25590. +const FORWARDED_SIGNALS: readonly NodeJS.Signals[] = [ + 'SIGTERM', + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGUSR1', + 'SIGUSR2', +]; + export async function relaunchOnExitCode(runner: () => Promise) { while (true) { try { @@ -71,9 +85,38 @@ export async function relaunchAppInChildProcess( } }); + // Forward termination signals to the child so a supervised parent + // (kill -TERM ) takes the child down with it instead of + // orphaning it. Use a Map of {signal -> handler} for precise cleanup on + // close/error; removeAllListeners would disturb unrelated subscribers, + // and leaking a handler per relaunch iteration trips + // MaxListenersExceededWarning after ~10 relaunches. #25590. + const forwarders = new Map void>(); + for (const sig of FORWARDED_SIGNALS) { + const handler = () => { + try { + child.kill(sig); + } catch { + // The child may have already exited; ignore the race. + } + }; + forwarders.set(sig, handler); + process.on(sig, handler); + } + const removeForwarders = () => { + for (const [sig, handler] of forwarders) { + process.off(sig, handler); + } + forwarders.clear(); + }; + 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); From 03f24c177a18362ffc873a4d4b68b425bd58523a Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Tue, 4 Aug 2026 11:21:16 +0800 Subject: [PATCH 2/9] fix(cli): use process.once for signal forwarders Per code review: process.on keeps forwarding every signal, so if the child hangs during graceful shutdown a second Ctrl+C is still intercepted and the user cannot force-quit. process.once forwards the first signal for graceful shutdown; a second signal takes the default disposition and force-terminates the parent. --- packages/cli/src/utils/relaunch.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index a25f2645557..1f12c4d70a2 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -101,7 +101,11 @@ export async function relaunchAppInChildProcess( } }; forwarders.set(sig, handler); - process.on(sig, handler); + // Use once() so the first signal is forwarded to the child for a + // graceful shutdown, and a second signal (e.g. a second Ctrl+C while + // the child is hung) takes the default disposition and force-quits the + // parent instead of being silently forwarded again. + process.once(sig, handler); } const removeForwarders = () => { for (const [sig, handler] of forwarders) { From 27586d9bf7086d4cfff0cb26fda851cb151a1809 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Wed, 5 Aug 2026 23:54:32 +0800 Subject: [PATCH 3/9] fix(cli): don't forward SIGINT/SIGQUIT to avoid double delivery Per gemini review: when running interactively, the TTY delivers SIGINT/SIGQUIT to the whole foreground process group (parent and child), so forwarding them from the parent delivers them twice and can interrupt the child's graceful-shutdown handler. Supervisors use SIGTERM for programmatic termination, which remains forwarded. --- packages/cli/src/utils/relaunch.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 1f12c4d70a2..7cb444a8bab 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -20,11 +20,14 @@ import { // its default disposition while the spawned child is reparented to PID 1 // and keeps running - holding the OAuth session and allocated heap until // killed manually. See #25590. +// SIGINT/SIGQUIT are intentionally NOT forwarded: when running +// interactively, the TTY delivers them to the whole foreground process +// group (parent and child), so forwarding would deliver them twice and +// could interrupt the child's graceful-shutdown handler. Supervisors use +// SIGTERM for programmatic termination, which is forwarded. const FORWARDED_SIGNALS: readonly NodeJS.Signals[] = [ 'SIGTERM', 'SIGHUP', - 'SIGINT', - 'SIGQUIT', 'SIGUSR1', 'SIGUSR2', ]; From 38c31a1c6caee74a48eb565ec03fe0f09c0bcad4 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 00:35:36 +0800 Subject: [PATCH 4/9] fix(cli): forward all signals until child exits and propagate signal status Per gemini review: process.once would orphan the child if a second SIGTERM/SIGHUP arrived during a slow graceful shutdown (the listener is removed and the parent dies immediately). Use process.on - listeners are cleaned up on child close anyway. Also propagate the child's signal termination: when close has a signal, re-raise it on the parent so supervisors see a clean signal exit instead of exit code 1. --- packages/cli/src/utils/relaunch.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 7cb444a8bab..90df745b261 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -104,11 +104,11 @@ export async function relaunchAppInChildProcess( } }; forwarders.set(sig, handler); - // Use once() so the first signal is forwarded to the child for a - // graceful shutdown, and a second signal (e.g. a second Ctrl+C while - // the child is hung) takes the default disposition and force-quits the - // parent instead of being silently forwarded again. - process.once(sig, handler); + // Use on() so that if the child is slow to shut down and a second + // signal is received, it is still forwarded rather than killing the + // parent immediately and orphaning the child. The listeners are + // removed on child close/error anyway. #25590. + process.on(sig, handler); } const removeForwarders = () => { for (const [sig, handler] of forwarders) { @@ -122,10 +122,21 @@ export async function relaunchAppInChildProcess( removeForwarders(); reject(err); }); - child.on('close', (code) => { + child.on('close', (code, signal) => { removeForwarders(); // Resume stdin before the parent process exits. process.stdin.resume(); + // Propagate the child's signal termination so supervisors (systemd, + // Kubernetes) see a clean signal exit rather than an unexpected code + // 1 crash. #25590. + if (signal) { + try { + process.kill(process.pid, signal); + return; + } catch { + // Fall back to exit code 1 if the signal cannot be re-raised. + } + } resolve(code ?? 1); }); }); From 76ebc7781f4edc18e6d8772fc0e48ff4b09c87e1 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 00:46:42 +0800 Subject: [PATCH 5/9] fix(cli): don't hang on non-fatal signal propagation Per gemini review: process.kill(pid, signal) with a non-fatal signal (e.g. SIGUSR1) does not terminate the process, so the early return after kill skipped resolve() and the promise never settled. Remove the return - if the signal is fatal the process exits before resolve runs (desired); if non-fatal, resolve proceeds normally. #25590 --- packages/cli/src/utils/relaunch.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 90df745b261..83c6b9cb96b 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -132,7 +132,10 @@ export async function relaunchAppInChildProcess( if (signal) { try { process.kill(process.pid, signal); - return; + // Do NOT return here: a non-fatal signal (e.g. SIGUSR1) does not + // terminate this process, so we must still resolve the promise + // below. If the signal IS fatal, this process exits before + // resolve runs - which is the desired outcome. } catch { // Fall back to exit code 1 if the signal cannot be re-raised. } From e8fafee0611559b522d2fcda6645519256bfd804 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 10:09:20 +0800 Subject: [PATCH 6/9] test(cli): cover child-to-parent signal propagation on relaunch Gemini code assist noted the signal re-raise path (process.kill on the parent when the child exits with a signal) was untested because calling it directly would terminate the Vitest runner. Mock process.kill and assert the parent re-raises the child's exit signal. #25590 --- packages/cli/src/utils/relaunch.test.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/cli/src/utils/relaunch.test.ts b/packages/cli/src/utils/relaunch.test.ts index 3ba856393ab..5b04fb037ec 100644 --- a/packages/cli/src/utils/relaunch.test.ts +++ b/packages/cli/src/utils/relaunch.test.ts @@ -378,6 +378,29 @@ describe('relaunchAppInChildProcess', () => { await expect(promise).rejects.toThrow('PROCESS_EXIT_CALLED'); expect(process.listenerCount('SIGTERM')).toBe(before); }); + + it('propagates the child signal termination to the parent process (#25590)', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const mockChild = createMockChildProcess(0, false); + mockedSpawn.mockImplementation(() => mockChild); + + // Mock process.kill so the re-raised signal does not terminate the + // Vitest runner; assert the parent re-raises the child's exit signal. + const processKillSpy = vi + .spyOn(process, 'kill') + .mockImplementation(() => true); + + const promise = relaunchAppInChildProcess([], []); + await new Promise((r) => setImmediate(r)); + + // Child exits with a signal: the parent must re-raise it. + mockChild.emit('close', null, 'SIGTERM'); + await expect(promise).rejects.toThrow('PROCESS_EXIT_CALLED'); + expect(processKillSpy).toHaveBeenCalledWith(process.pid, 'SIGTERM'); + + processKillSpy.mockRestore(); + }); }); }); From f0cda82792e2cf168ca89f41fd5e551051c00841 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 10:39:47 +0800 Subject: [PATCH 7/9] fix(cli): keep bootstrap parent alive on interactive SIGINT/SIGQUIT When the user presses Ctrl+C, the TTY delivers SIGINT to the whole foreground process group. The parent had no SIGINT listener, so it died on its default disposition before the child finished its graceful shutdown - orphaning the child and returning the shell prompt early with mixed output. Register no-op keepalive handlers for SIGINT/SIGQUIT during the child's lifetime (the child already receives them from the TTY). They are removed in the close handler before the child's exit signal is re-raised, so the re-raise still terminates the parent. #25590 --- packages/cli/src/utils/relaunch.test.ts | 30 +++++++++++++++++++++++++ packages/cli/src/utils/relaunch.ts | 30 ++++++++++++++++++++----- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/utils/relaunch.test.ts b/packages/cli/src/utils/relaunch.test.ts index 5b04fb037ec..6829dd050dc 100644 --- a/packages/cli/src/utils/relaunch.test.ts +++ b/packages/cli/src/utils/relaunch.test.ts @@ -379,6 +379,36 @@ describe('relaunchAppInChildProcess', () => { expect(process.listenerCount('SIGTERM')).toBe(before); }); + it('keeps the parent alive on SIGINT (no-op) and removes it on close (#25590)', async () => { + process.argv = ['/usr/bin/node', '/app/cli.js']; + + const mockChild = createMockChildProcess(0, false); + mockedSpawn.mockImplementation(() => mockChild); + + const before = process.listenerCount('SIGINT'); + const killSpy = mockChild.kill as ReturnType; + + const promise = relaunchAppInChildProcess([], []); + await new Promise((r) => setImmediate(r)); + + // A no-op keepalive handler for SIGINT was installed on the parent. + expect(process.listenerCount('SIGINT')).toBe(before + 1); + + // Emulate the TTY delivering SIGINT to the whole process group: the + // handler must NOT forward it to the child (it already received it). + const handler = process.listeners('SIGINT').at(-1) as + | (() => void) + | undefined; + expect(handler).toBeDefined(); + handler!(); + expect(killSpy).not.toHaveBeenCalledWith('SIGINT'); + + // Closing the child must remove the keepalive handler (no listener leak). + mockChild.emit('close', 0); + await expect(promise).rejects.toThrow('PROCESS_EXIT_CALLED'); + expect(process.listenerCount('SIGINT')).toBe(before); + }); + it('propagates the child signal termination to the parent process (#25590)', async () => { process.argv = ['/usr/bin/node', '/app/cli.js']; diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 83c6b9cb96b..e358ac3a0c0 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -20,11 +20,6 @@ import { // its default disposition while the spawned child is reparented to PID 1 // and keeps running - holding the OAuth session and allocated heap until // killed manually. See #25590. -// SIGINT/SIGQUIT are intentionally NOT forwarded: when running -// interactively, the TTY delivers them to the whole foreground process -// group (parent and child), so forwarding would deliver them twice and -// could interrupt the child's graceful-shutdown handler. Supervisors use -// SIGTERM for programmatic termination, which is forwarded. const FORWARDED_SIGNALS: readonly NodeJS.Signals[] = [ 'SIGTERM', 'SIGHUP', @@ -32,6 +27,18 @@ const FORWARDED_SIGNALS: readonly NodeJS.Signals[] = [ 'SIGUSR2', ]; +// SIGINT/SIGQUIT are intentionally NOT forwarded: when running +// interactively, the TTY delivers them to the whole foreground process +// group (parent and child), so forwarding would deliver them twice and +// could interrupt the child's graceful-shutdown handler. Instead the parent +// registers a no-op handler (KEEPALIVE_SIGNALS) so it does NOT die on its +// default disposition before the child finishes shutting down - which would +// orphan the child and return the shell prompt early with mixed output. The +// child receives the signal directly from the TTY; when it exits, the no-op +// handler is removed and the child's exit signal is re-raised on the parent. +// Supervisors use SIGTERM for programmatic termination, which is forwarded. +const KEEPALIVE_SIGNALS: readonly NodeJS.Signals[] = ['SIGINT', 'SIGQUIT']; + export async function relaunchOnExitCode(runner: () => Promise) { while (true) { try { @@ -110,6 +117,19 @@ export async function relaunchAppInChildProcess( // removed on child close/error anyway. #25590. process.on(sig, handler); } + // No-op handlers for the interactive interrupt signals the TTY delivers to + // the whole foreground process group. The parent must survive long enough + // for the child to finish its graceful shutdown; otherwise it dies on the + // default disposition and the child is orphaned. The no-op is removed in + // the close handler before the child's exit signal is re-raised, so the + // re-raise still terminates the parent. #25590. + for (const sig of KEEPALIVE_SIGNALS) { + // Reuse the same handler reference for both the Map and the listener so + // removeForwarders can process.off() it on close. + const handler = () => {}; + forwarders.set(sig, handler); + process.on(sig, handler); + } const removeForwarders = () => { for (const [sig, handler] of forwarders) { process.off(sig, handler); From 76ca4f9a8cc5c631c98ba5be44a4c3463f82cd45 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 10:51:11 +0800 Subject: [PATCH 8/9] fix(cli): defer relaunch exit so re-raised signal is delivered process.kill(process.pid, signal) schedules the signal on the event loop rather than terminating synchronously when a JS handler is registered. The previous code called resolve() immediately after, and relaunchOnExitCode's process.exit() ran before the loop could deliver the signal - so the parent exited with the fallback code instead of the signal, defeating the clean-signal-exit propagation for supervisors. Defer resolve() one tick via setTimeout so the event loop delivers the re-raised signal first. A fatal signal terminates the process in that tick; a non-fatal one (e.g. SIGUSR1) resolves normally. #25590 --- packages/cli/src/utils/relaunch.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index e358ac3a0c0..4a6b1a64e55 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -152,10 +152,15 @@ export async function relaunchAppInChildProcess( if (signal) { try { process.kill(process.pid, signal); - // Do NOT return here: a non-fatal signal (e.g. SIGUSR1) does not - // terminate this process, so we must still resolve the promise - // below. If the signal IS fatal, this process exits before - // resolve runs - which is the desired outcome. + // process.kill schedules the signal on the event loop; it does NOT + // terminate synchronously when a JS handler is registered (e.g. by + // the app or a supervisor integration), so deferring resolve gives + // the loop a tick to deliver the re-raised signal before + // relaunchOnExitCode calls process.exit(). A fatal signal + // terminates the process here; a non-fatal one (e.g. SIGUSR1) falls + // through to resolve below. #25590. + setTimeout(() => resolve(code ?? 1), 0); + return; } catch { // Fall back to exit code 1 if the signal cannot be re-raised. } From e8a15af894e702d1e0af9709e3fc960adf37e814 Mon Sep 17 00:00:00 2001 From: keaixiaozhu Date: Thu, 6 Aug 2026 11:02:03 +0800 Subject: [PATCH 9/9] fix(cli): guard signal registration against unsupported signals Some POSIX-only signals (SIGUSR1, SIGUSR2 on Windows; SIGQUIT) throw in process.on() on platforms that do not implement them, which would crash the CLI at relaunch time. Register each signal inside try/catch and only track successfully-attached listeners in the forwarder Map, so removeForwarders never calls process.off on a signal that was never registered. #25590 --- packages/cli/src/utils/relaunch.ts | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/utils/relaunch.ts b/packages/cli/src/utils/relaunch.ts index 4a6b1a64e55..13b5193e625 100644 --- a/packages/cli/src/utils/relaunch.ts +++ b/packages/cli/src/utils/relaunch.ts @@ -110,12 +110,20 @@ export async function relaunchAppInChildProcess( // The child may have already exited; ignore the race. } }; - forwarders.set(sig, handler); - // Use on() so that if the child is slow to shut down and a second - // signal is received, it is still forwarded rather than killing the - // parent immediately and orphaning the child. The listeners are - // removed on child close/error anyway. #25590. - process.on(sig, handler); + // Register only signals the runtime supports - some POSIX-only signals + // (e.g. SIGUSR1/SIGUSR2 on Windows) throw in process.on(). Register + // first and only track successful listeners in the Map, since + // removeForwarders calls process.off on every entry. #25590. + try { + // Use on() so that if the child is slow to shut down and a second + // signal is received, it is still forwarded rather than killing the + // parent immediately and orphaning the child. The listeners are + // removed on child close/error anyway. #25590. + process.on(sig, handler); + forwarders.set(sig, handler); + } catch { + // Signal unsupported on this platform; skip forwarding for it. + } } // No-op handlers for the interactive interrupt signals the TTY delivers to // the whole foreground process group. The parent must survive long enough @@ -127,8 +135,12 @@ export async function relaunchAppInChildProcess( // Reuse the same handler reference for both the Map and the listener so // removeForwarders can process.off() it on close. const handler = () => {}; - forwarders.set(sig, handler); - process.on(sig, handler); + try { + process.on(sig, handler); + forwarders.set(sig, handler); + } catch { + // Signal unsupported on this platform (e.g. SIGQUIT on Windows). + } } const removeForwarders = () => { for (const [sig, handler] of forwarders) {