Merge df9885ef9e into a38e2f0048

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<NodeJS.Signals, number>;
+
+      const mockChild = createMockChildProcess(0, false);
+      const listenerCountsAfterSpawn: Record<string, number> = {};
+
+      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<NodeJS.Signals, number>;
+
+      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<typeof relaunchAppInChildProcess>[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<NodeJS.Signals, number>;
+
+      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]);
+      }
+    });
   });
 });
 

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 <parent_pid>`,
+    // 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<NodeJS.Signals, () => 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<number>((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);