fix(cli): forward termination signals to relaunched child process

[C0d3N1nja97342]

Summary

relaunchAppInChildProcess now forwards termination signals (SIGTERM, SIGHUP, SIGINT, SIGQUIT, SIGUSR1, SIGUSR2) from the bootstrap parent to the spawned child, so a supervised kill -TERM <bootstrap-pid> takes the child down instead of orphaning it.

Details

The bootstrap parent spawned the child with stdio: inherit + ipc but installed no signal handlers. A programmatic SIGTERM/SIGHUP from a supervising process (ACP client, systemd, container runtime) killed the parent 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 this because SIGINT is delivered to the whole foreground process group via the controlling terminal; the bug only manifests with kill(pid, signal).

Forwarders are installed after spawn and removed on both close and error via a Map<signal, handler> so cleanup is precise (removeAllListeners would disturb unrelated subscribers) and does not leak a handler per relaunch iteration (which would trip MaxListenersExceededWarning after ~10 relaunches). child.kill(sig) is guarded with try/catch for the race where the signal arrives just after the child exits.

stdio: 'inherit' and detached are left unchanged - signal forwarding is orthogonal to the stdio wiring.

Related Issues

Fixes #25590

How to Validate

npm run test -- packages/cli/src/utils/relaunch.test.ts

The new test forwards termination signals to the child and removes them on close (#25590) asserts:

1. After spawn, a SIGTERM listener is registered on the parent (listenerCount increases by 1).
2. Invoking that handler calls child.kill('SIGTERM').
3. After the child closes, the listener is removed (listenerCount returns to baseline - no leak across relaunch iterations).

All 11 tests in relaunch.test.ts pass (10 pre-existing + 1 new).

To reproduce the original orphan manually (before this fix):

gemini -y --acp &
BOOTSTRAP_PID=$!
kill -TERM $BOOTSTRAP_PID
ps -eo pid,ppid,cmd | grep gemini   # child still alive, PPID 1

After this fix the child exits with the parent.

[github-actions]

📊 PR Size: size/M

• Lines changed: 191
• Additions: +189
• Deletions: -2
• Files changed: 2

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where child processes spawned by the bootstrap parent were becoming orphaned when the parent received termination signals. By forwarding these signals to the child, the system ensures that supervised processes are terminated gracefully rather than remaining active as orphans. The implementation includes robust cleanup mechanisms to maintain process stability and prevent listener accumulation over multiple relaunch cycles.

Highlights

• Signal Forwarding: Implemented signal forwarding for termination signals (SIGTERM, SIGHUP, SIGINT, SIGQUIT, SIGUSR1, SIGUSR2) from the bootstrap parent process to the spawned child process.
• Memory and Listener Management: Added precise cleanup logic using a Map of signal handlers to ensure listeners are removed upon child process exit or error, preventing memory leaks and MaxListenersExceededWarnings.
• Testing: Added a new test case to verify that termination signals are correctly forwarded to the child and that signal listeners are properly cleaned up after the child closes.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request implements signal forwarding from the parent bootstrap process to the spawned child process in relaunchAppInChildProcess to prevent orphaned child processes upon termination, along with proper cleanup of listeners to avoid memory leaks. A corresponding unit test has also been added. The review feedback recommends using process.once instead of process.on to register the signal handlers, ensuring that a second termination signal (such as a second Ctrl+C) can still force-terminate the CLI if the child process hangs during a graceful shutdown.

[gemini-code-assist]

Using process.on to register signal forwarders can lead to an unresponsive CLI if the child process hangs or takes too long to clean up. For example, if a user presses Ctrl+C (SIGINT) and the child process is deadlocked, pressing Ctrl+C again will continue to be intercepted by the parent and forwarded, preventing the user from force-terminating the CLI from the terminal.

By changing process.on to process.once, the first signal is forwarded to the child to allow graceful shutdown. If the child hangs and the user sends the signal a second time, the default signal disposition (immediate termination) will take over, allowing the user to force-quit the process.

Suggested change

	const forwarders = new Map<NodeJS.Signals, () => 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 forwarders = new Map<NodeJS.Signals, () => 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.once(sig, handler);
	}

[C0d3N1nja97342]

Thanks for the review. Switched to process.once as suggested (commit 03f24c1) so the first signal is forwarded for graceful shutdown and a second one takes the default disposition to force-quit if the child hangs.

I tried to pin the once semantics in a unit test but process.once only auto-removes on process.emit(sig), and emitting real signals (SIGINT/SIGTERM) in the vitest process also fires the runner's own listeners - not safe to test that way. The existing test still covers forwarder registration, kill forwarding, and cleanup-on-close (no leak).

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding from the parent process to the child process in relaunchAppInChildProcess to prevent orphaned child processes upon parent termination, and adds corresponding unit tests. The feedback suggests removing SIGINT and SIGQUIT from the forwarded signals list because the TTY already delivers these to the entire process group, and forwarding them again can cause double delivery and disrupt graceful shutdown.

[gemini-code-assist]

Forwarding SIGINT and SIGQUIT can break interactive graceful shutdown (e.g., via Ctrl+C or Ctrl+\).

When running interactively in a terminal, the TTY automatically delivers these signals to the entire foreground process group (both the parent and the child process). If the parent also forwards these signals, the child process will receive them twice.

If the child process registers a graceful shutdown handler using process.once('SIGINT'), the first signal (from the TTY) will trigger the handler and remove the listener. The second signal (forwarded from the parent) will then immediately trigger the default OS disposition (abrupt termination), preventing the child from completing its graceful shutdown.

Since supervisors primarily use SIGTERM for programmatic termination, we should exclude SIGINT and SIGQUIT from the forwarded signals list.

const FORWARDED_SIGNALS: readonly NodeJS.Signals[] = [
  'SIGTERM',
  'SIGHUP',
  'SIGUSR1',
  'SIGUSR2',
];

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding from the parent bootstrap process to the spawned child process in relaunchAppInChildProcess for termination signals (SIGTERM, SIGHUP, SIGUSR1, SIGUSR2). This prevents the child process from being orphaned when the parent is programmatically terminated. It also handles cleanup of these listeners on child process close or error to prevent listener leaks. A unit test has been added to verify this behavior. I have no further feedback to provide as the implementation is clean and well-tested.

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements termination signal forwarding from the parent bootstrap process to the spawned child process to prevent orphaned processes, along with corresponding unit tests. The review feedback identifies two key areas for improvement: changing process.once to process.on to ensure multiple signals are forwarded during slow shutdowns, and properly propagating the child's signal termination status to the parent process rather than exiting with code 1.

[gemini-code-assist]

Using process.once to forward signals can lead to orphaned child processes if a second signal is received during a slow graceful shutdown.

While SIGINT and SIGQUIT are excluded from forwarding (and thus handled by the TTY), termination signals like SIGTERM or SIGHUP are programmatic. If a supervisor sends SIGTERM and the child process takes a few seconds to clean up and exit, a second SIGTERM from an impatient supervisor will immediately terminate the parent process (since the once listener has been removed), leaving the child process running as an orphan.

Using process.on ensures that all termination signals continue to be forwarded to the child until it exits, at which point the parent's listeners are cleaned up anyway.

Suggested change

	// 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() instead of once() so that if the child process 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.
	process.on(sig, handler);

[gemini-code-assist]

When the child process is terminated by a signal, the parent process currently exits with code 1 (via resolve(code ?? 1) because code is null when killed by a signal).

This misrepresents the termination status to supervisors (like systemd or Kubernetes), which may interpret exit code 1 as an unexpected application crash rather than a clean signal termination.

To propagate the signal termination status correctly, the parent process should kill itself with the same signal after removing the forwarders.

Since we cannot directly modify the outer child.on('close', ...) signature via a targeted suggestion here without touching context lines, please consider refactoring the handler to capture the signal parameter and propagate it:

      child.on('close', (code, signal) => {
        removeForwarders();
        // Resume stdin before the parent process exits.
        process.stdin.resume();
        if (signal) {
          try {
            process.kill(process.pid, signal);
            return;
          } catch {
            // Fallback if process.kill is not supported
          }
        }
        resolve(code ?? 1);
      });

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces signal forwarding from the parent process to the spawned child process in relaunch.ts to prevent orphaned child processes when the parent is programmatically terminated, along with corresponding unit tests. The reviewer identified a critical issue where propagating non-fatal signals (such as SIGUSR1) via process.kill could cause the parent process to hang indefinitely due to an early return; statement, and suggested removing the return; to safely allow the promise to resolve if the process is not terminated.

[gemini-code-assist]

If process.kill(process.pid, signal) is successful but does not terminate the parent process (for example, with non-fatal signals like SIGUSR1 which Node.js uses to start the debugger, or if the signal is ignored/handled), the return; statement will prevent the promise from resolving, causing the bootstrap parent process to hang indefinitely.

Since process.kill is synchronous, if it successfully terminates the process, the execution stops immediately. If it does not terminate the process, we should allow the execution to continue and resolve the promise normally. Removing the return; statement resolves this issue safely.

        if (signal) {
          try {
            process.kill(process.pid, signal);
          } catch {
            // Fall back to exit code 1 if the signal cannot be re-raised.
          }
        }

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding and propagation between the parent and child processes during application relaunch to prevent orphaned child processes and ensure clean signal exits. It registers listeners for termination signals (SIGTERM, SIGHUP, SIGUSR1, SIGUSR2) on the parent process to forward them to the child, ensures proper cleanup of these listeners, and propagates the child's exit signal back to the parent. The review feedback recommends adding a dedicated test case to verify signal propagation by mocking process.kill to avoid terminating the Vitest test runner during tests.

[gemini-code-assist]

The signal propagation logic (process.kill) is currently untested because calling it directly would terminate the Vitest test runner. To ensure the robustness of the signal propagation and prevent accidental test runner termination in future tests, we should mock process.kill and add a dedicated test case to verify that the signal is correctly propagated to the parent process.

    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<typeof vi.fn>;

      // 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);
    });

    it('propagates the child signal termination to the parent process', async () => {
      process.argv = ['/usr/bin/node', '/app/cli.js'];

      const mockChild = createMockChildProcess(null, false);
      mockedSpawn.mockImplementation(() => mockChild);

      const processKillSpy = vi.spyOn(process, 'kill').mockImplementation(() => true);

      const promise = relaunchAppInChildProcess([], []);
      await new Promise((r) => setImmediate(r));

      mockChild.emit('close', null, 'SIGTERM');
      await expect(promise).rejects.toThrow('PROCESS_EXIT_CALLED');
      expect(processKillSpy).toHaveBeenCalledWith(process.pid, 'SIGTERM');

      processKillSpy.mockRestore();
    });

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding and propagation in relaunchAppInChildProcess to prevent orphaned child processes and ensure clean exit codes when termination signals are received, along with corresponding unit tests. The reviewer suggests registering no-op handlers for SIGINT and SIGQUIT on the parent process during the child's lifetime to prevent the parent from terminating prematurely on Ctrl+C before the child finishes its graceful shutdown.

[gemini-code-assist]

While SIGINT and SIGQUIT are intentionally not forwarded to prevent duplicate signal delivery in interactive TTY sessions, the parent process itself currently has no listeners for these signals. As a result, when a user presses Ctrl+C (sending SIGINT to the entire process group), the parent process will terminate immediately on its default disposition. This causes the shell prompt to return prematurely while the child process is still running its graceful shutdown handlers in the background, leading to mixed terminal output and orphaned child processes during shutdown.

To prevent this, we should register no-op handlers for SIGINT and SIGQUIT on the parent process during the child's lifetime. This keeps the parent process alive, allowing it to wait for the child process to exit cleanly before exiting itself.

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces signal forwarding and keep-alive handling in the application relaunch mechanism to prevent orphaned child processes and ensure clean termination propagation. A critical issue was identified where propagating the child's termination signal to the parent via process.kill will not take effect because the synchronous execution path immediately proceeds to call process.exit(), terminating the process before the event loop can deliver the signal. It is recommended to defer the promise resolution using setTimeout to allow the event loop to process the signal.

[gemini-code-assist]

Because JavaScript is single-threaded, calling process.kill(process.pid, signal) schedules the signal delivery asynchronously on the event loop. However, the current synchronous execution context (resolving the promise, returning to relaunchOnExitCode, and immediately calling process.exit(exitCode)) runs to completion first. Since process.exit() terminates the process synchronously, the event loop never gets a chance to process the pending signal, and the parent process exits with the exit code instead of being terminated by the signal.

To fix this, we should defer the promise resolution using setTimeout when a signal is propagated. This allows the event loop to run and process the fatal signal (terminating the process). If the signal is non-fatal, the timeout will fire and resolve the promise normally.

        if (signal) {
          try {
            process.kill(process.pid, signal);
            // Defer resolution to allow the event loop to process the signal.
            // If the signal is fatal, the process will terminate before the timeout fires.
            // If it is non-fatal, we resolve with code ?? 1 after the timeout.
            setTimeout(() => {
              resolve(code ?? 1);
            }, 20);
            return;
          } catch {
            // Fall back to exit code 1 if the signal cannot be re-raised.
          }
        }
        resolve(code ?? 1);

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding and keep-alive handling in the CLI relaunch utility to prevent orphaned child processes when the parent process is terminated. It forwards termination signals to the child and registers no-op handlers for interactive signals to ensure graceful shutdown. The review feedback correctly identifies a critical cross-platform issue where registering unsupported POSIX signals (such as SIGUSR1, SIGUSR2, and SIGQUIT) will throw a runtime error and crash the CLI on Windows. Wrapping these registrations in try/catch blocks is recommended to ensure compatibility.

[gemini-code-assist]

On Windows, Node.js does not support several POSIX signals such as SIGUSR1, SIGUSR2, and SIGQUIT. Attempting to register a listener for these unsupported signals using process.on() will throw a runtime error (e.g., Error: EINVAL: invalid argument, uv_signal_start or ERR_UNKNOWN_SIGNAL), which will crash the CLI during relaunch on Windows.

To ensure cross-platform compatibility and prevent crashes on Windows, wrap the process.on() registration in a try/catch block, and only add the signal to the forwarders map if it was successfully registered.

    const forwarders = new Map<NodeJS.Signals, () => void>();
    for (const sig of FORWARDED_SIGNALS) {
      const handler = () => {
        try {
          child.kill(sig);
        } catch {
          // The child may have already exited; ignore the race.
        }
      };
      try {
        process.on(sig, handler);
        forwarders.set(sig, handler);
      } catch {
        // Signal not supported on this platform (e.g., SIGUSR1/2 on Windows)
      }
    }
    // 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 = () => {};
      try {
        process.on(sig, handler);
        forwarders.set(sig, handler);
      } catch {
        // Signal not supported on this platform (e.g., SIGQUIT on Windows)
      }
    }

[C0d3N1nja97342]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements signal forwarding and propagation in the CLI's relaunch utility to prevent orphaned child processes and ensure clean exits under supervisors. Specifically, it forwards termination signals (like SIGTERM) to the child process, registers no-op handlers for interactive signals (like SIGINT) to keep the parent alive during graceful shutdown, and propagates the child's termination signal back to the parent. It also cleans up registered listeners on close or error to avoid memory leaks, and adds corresponding unit tests to verify these behaviors. There are no review comments, so I have no feedback to provide.