fix(mcp): shut down when the stdio parent exits (#3737)

* fix(mcp): shut down when the stdio parent exits

* test(mcp): simplify parent-exit stream tracking

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>

gitnexus/src/mcp/server.ts

@@ -433,10 +433,37 @@ export function installSignalShutdown(
   on('SIGTERM', () => void shutdown(SHUTDOWN_EXIT_CODES.SIGTERM));
 }
 
+/** Stdio can outlive its launcher when another process holds the pipe open. */
+export function installParentShutdown(
+  shutdown: (exitCode?: number) => unknown,
+  parentPid = process.ppid,
+): () => void {
+  if (parentPid <= 1) return () => {};
+  const timer = setInterval(() => {
+    // Reparenting also detects death when the original PID has been reused.
+    let parentGone = process.ppid !== parentPid;
+    if (!parentGone) {
+      try {
+        process.kill(parentPid, 0);
+      } catch (error) {
+        // EPERM (or any unrecognized failure) is not proof of parent death.
+        parentGone = (error as NodeJS.ErrnoException).code === 'ESRCH';
+      }
+    }
+    if (parentGone) {
+      clearInterval(timer);
+      void shutdown(0);
+    }
+  }, 1_000);
+  timer.unref();
+  return () => clearInterval(timer);
+}
+
 export async function startMCPServer(
   backend: LocalBackend,
   repositoryPolicy?: McpRepositoryPolicy,
 ): Promise<void> {
+  const parentPid = process.ppid;
   const validatedRepositoryPolicy = repositoryPolicy ?? (await createMcpRepositoryPolicy(backend));
   const server = createMCPServer(backend, { repositoryPolicy: validatedRepositoryPolicy });
 
@@ -473,6 +500,7 @@ export async function startMCPServer(
   const shutdown = async (exitCode = 0) => {
     if (shuttingDown) return;
     shuttingDown = true;
+    stopParentWatch();
     // Safety net: if backend.disconnect()/server.close() hangs, still exit so a
     // SIGINT/SIGTERM reliably terminates the process. Unref'd so the timer alone
     // never keeps the event loop alive.
@@ -489,6 +517,7 @@ export async function startMCPServer(
     clearTimeout(forceExit);
     process.exit(exitCode);
   };
+  const stopParentWatch = installParentShutdown(shutdown, parentPid);
 
   // Handle graceful shutdown. Node invokes signal listeners with the signal
   // NAME (e.g. 'SIGTERM') as the first argument; registering `shutdown`

gitnexus/test/integration/mcp/server-startup.test.ts

@@ -21,7 +21,9 @@
  */
 
 import { describe, it, expect } from 'vitest';
-import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
+import { execFileSync, spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
+import { once } from 'node:events';
+import { tmpdir } from 'node:os';
 import path from 'node:path';
 import fs from 'node:fs';
 import { fileURLToPath } from 'node:url';
@@ -172,6 +174,7 @@ function spawnMcpServer(): SpawnedServer {
       proc.stdin.write(frame);
     },
     close: async () => {
+      if (proc.exitCode !== null || proc.signalCode !== null) return;
       proc.stdin.end();
       // Give the server a moment to clean up; force-kill if it hangs.
       await new Promise<void>((resolve) => {
@@ -315,3 +318,147 @@ describe('MCP server end-to-end startup', () => {
     }
   }, 60_000);
 });
+
+async function within<T>(promise: Promise<T>, milliseconds: number): Promise<T> {
+  let timer: ReturnType<typeof setTimeout>;
+  try {
+    return await Promise.race([
+      promise,
+      new Promise<never>((_, reject) => {
+        timer = setTimeout(
+          () => reject(new Error('MCP lifecycle operation timed out')),
+          milliseconds,
+        );
+      }),
+    ]);
+  } finally {
+    clearTimeout(timer!);
+  }
+}
+
+const initializeMessage = {
+  jsonrpc: '2.0',
+  id: 1,
+  method: 'initialize',
+  params: {
+    protocolVersion: '2025-06-18',
+    capabilities: {},
+    clientInfo: { name: 'gitnexus-lifecycle-test', version: '0.0.0' },
+  },
+};
+
+describe('MCP stdio process lifecycle', () => {
+  it.each(['before', 'after'] as const)('exits cleanly on EOF %s the handshake', async (when) => {
+    const server = spawnMcpServer();
+    try {
+      if (when === 'after') {
+        server.send(initializeMessage);
+        expect(await server.nextMessage()).toMatchObject({ id: 1, result: {} });
+      }
+      const exited = once(server.proc, 'exit');
+      server.proc.stdin.end();
+      expect(await within(exited, TOTAL_BUDGET_MS)).toEqual([0, null]);
+    } finally {
+      await server.close();
+    }
+  });
+
+  it('keeps a second MCP client usable when the first client disconnects', async () => {
+    const first = spawnMcpServer();
+    const second = spawnMcpServer();
+    try {
+      first.send(initializeMessage);
+      second.send(initializeMessage);
+      expect(await first.nextMessage()).toMatchObject({ id: 1, result: {} });
+      expect(await second.nextMessage()).toMatchObject({ id: 1, result: {} });
+      const firstExited = once(first.proc, 'exit');
+      first.proc.stdin.end();
+      expect(await within(firstExited, TOTAL_BUDGET_MS)).toEqual([0, null]);
+      second.send({ jsonrpc: '2.0', id: 2, method: 'ping' });
+      expect(await second.nextMessage()).toMatchObject({ id: 2, result: {} });
+    } finally {
+      await first.close();
+      await second.close();
+    }
+  });
+
+  // A FIFO lets the client keep stdin open independently of the launching
+  // parent. ChildProcess.stdin would close automatically when that parent dies,
+  // accidentally testing EOF again. Windows has no mkfifo; unit tests cover its
+  // unchanged-PPID / missing-parent liveness probe instead.
+  it.skipIf(process.platform === 'win32')(
+    'exits when its parent dies even if stdin stays open',
+    async () => {
+      const home = fs.mkdtempSync(path.join(tmpdir(), 'gitnexus-parent-exit-'));
+      const fifo = path.join(home, 'stdin');
+      execFileSync('mkfifo', [fifo]);
+      const fd = fs.openSync(fifo, fs.constants.O_RDWR);
+      const parent = spawn(
+        process.execPath,
+        [
+          '-e',
+          `
+      const { spawn } = require('node:child_process');
+      const child = spawn(process.execPath, [process.argv[1], 'mcp'], { stdio: [0, 1, 2] });
+      process.send({ pid: child.pid });
+      setInterval(() => {}, 1000);
+    `,
+          DIST_CLI,
+        ],
+        {
+          cwd: REPO_ROOT,
+          env: { ...process.env, GITNEXUS_HOME: home, NODE_OPTIONS: '' },
+          stdio: [fd, 'pipe', 'pipe', 'ipc'],
+        },
+      );
+      let serverPid: number | undefined;
+      let output = '';
+      let errors = '';
+      parent.stdout!.setEncoding('utf8');
+      parent.stdout!.on('data', (chunk: string) => {
+        output += chunk;
+      });
+      parent.stderr!.on('data', (chunk: Buffer) => {
+        errors += chunk.toString();
+      });
+      try {
+        const [message] = await within(once(parent, 'message'), TOTAL_BUDGET_MS);
+        serverPid = (message as { pid: number }).pid;
+        fs.writeSync(fd, JSON.stringify(initializeMessage) + '\n');
+        await expect.poll(() => output, { timeout: FIRST_FRAME_BUDGET_MS }).toContain('"id":1');
+        expect(JSON.parse(output.trim())).toMatchObject({ id: 1, result: {} });
+
+        // A live, idle parent must not be mistaken for a disconnected client.
+        await new Promise((resolve) => setTimeout(resolve, 1_200));
+        expect(parent.stdout!.readableEnded).toBe(false);
+        output = '';
+        fs.writeSync(fd, JSON.stringify({ jsonrpc: '2.0', id: 2, method: 'ping' }) + '\n');
+        await expect.poll(() => output, { timeout: FIRST_FRAME_BUDGET_MS }).toContain('"id":2');
+
+        const parentExited = once(parent, 'exit');
+        parent.kill('SIGKILL');
+        await within(parentExited, TOTAL_BUDGET_MS);
+        await expect
+          .poll(() => parent.stdout!.readableEnded, { timeout: TOTAL_BUDGET_MS, message: errors })
+          .toBe(true);
+        expect(errors).not.toMatch(/uncaughtException|unhandledRejection/);
+      } finally {
+        if (!parent.stdout!.readableEnded && serverPid !== undefined) {
+          try {
+            process.kill(serverPid, 'SIGKILL');
+          } catch {
+            /* already exited */
+          }
+        }
+        if (parent.exitCode === null && parent.signalCode === null) {
+          const exited = once(parent, 'exit');
+          parent.kill('SIGKILL');
+          await within(exited, TOTAL_BUDGET_MS);
+        }
+        fs.closeSync(fd);
+        fs.rmSync(home, { recursive: true, force: true });
+      }
+    },
+    60_000,
+  );
+});

gitnexus/test/unit/server.test.ts

@@ -12,7 +12,7 @@
  * NOTE: We test the server handler logic by calling the request handlers
  * directly through the MCP Server's handler dispatch.
  */
-import { describe, it, expect, vi } from 'vitest';
+import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest';
 import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { Server } from '@modelcontextprotocol/sdk/server/index.js';
 import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
@@ -21,6 +21,7 @@ import {
   createMCPServer,
   getNextStepHint,
   installSignalShutdown,
+  installParentShutdown,
   startMCPServer,
   SHUTDOWN_EXIT_CODES,
 } from '../../src/mcp/server.js';
@@ -682,6 +683,7 @@ describe('prompt registration', () => {
 
 describe('startMCPServer', () => {
   it('registers stdin shutdown listeners before awaiting Server.prototype.connect', async () => {
+    vi.useFakeTimers();
     // P1 finding: startMCPServer must register process.stdin end/close/error
     // shutdown listeners before calling server.connect(), so that a stdin
     // closure during transport setup is safely handled.
@@ -718,6 +720,8 @@ describe('startMCPServer', () => {
     try {
       await startMCPServer(backend);
     } finally {
+      vi.clearAllTimers();
+      vi.useRealTimers();
       connectSpy.mockRestore();
       exitStub.mockRestore();
       // Remove only listeners added by startMCPServer so no side effects leak.
@@ -762,6 +766,81 @@ describe('startMCPServer', () => {
   });
 });
 
+describe('installParentShutdown', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.spyOn(process, 'ppid', 'get').mockReturnValue(1234);
+    vi.spyOn(process, 'kill').mockReturnValue(true);
+  });
+
+  afterEach(() => {
+    vi.clearAllTimers();
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it('leaves a live parent alone and stops probing when disposed', () => {
+    const shutdown = vi.fn();
+    const stop = installParentShutdown(shutdown);
+    vi.advanceTimersByTime(1_000);
+    expect(process.kill).toHaveBeenCalledWith(1234, 0);
+    expect(shutdown).not.toHaveBeenCalled();
+    stop();
+    vi.advanceTimersByTime(2_000);
+    expect(process.kill).toHaveBeenCalledTimes(1);
+  });
+
+  it('shuts down once after reparenting, including adoption by a subreaper', () => {
+    const shutdown = vi.fn();
+    installParentShutdown(shutdown);
+    vi.spyOn(process, 'ppid', 'get').mockReturnValue(4321);
+    vi.advanceTimersByTime(3_000);
+    expect(shutdown).toHaveBeenCalledExactlyOnceWith(0);
+    expect(process.kill).not.toHaveBeenCalled();
+  });
+
+  it('detects a missing parent even when the OS retains the original PPID', () => {
+    const shutdown = vi.fn();
+    installParentShutdown(shutdown);
+    vi.mocked(process.kill).mockImplementation(() => {
+      throw Object.assign(new Error('No such process'), { code: 'ESRCH' });
+    });
+    vi.advanceTimersByTime(3_000);
+    expect(shutdown).toHaveBeenCalledExactlyOnceWith(0);
+  });
+
+  it.each(['EPERM', 'EACCES', undefined])(
+    'keeps serving after an inconclusive probe (%s)',
+    (code) => {
+      const shutdown = vi.fn();
+      installParentShutdown(shutdown);
+      vi.mocked(process.kill).mockImplementation(() => {
+        throw Object.assign(new Error('Cannot inspect parent'), { code });
+      });
+      vi.advanceTimersByTime(2_000);
+      expect(shutdown).not.toHaveBeenCalled();
+      expect(process.kill).toHaveBeenCalledTimes(2);
+    },
+  );
+
+  it('does not keep the event loop alive', () => {
+    const interval = vi.spyOn(globalThis, 'setInterval');
+    const stop = installParentShutdown(vi.fn());
+    expect(interval.mock.results[0].value.hasRef()).toBe(false);
+    stop();
+  });
+
+  it.each([0, 1])(
+    'does not probe a process group or init when started under PPID %s',
+    (parentPid) => {
+      const shutdown = vi.fn();
+      installParentShutdown(shutdown, parentPid)();
+      expect(vi.getTimerCount()).toBe(0);
+      expect(shutdown).not.toHaveBeenCalled();
+    },
+  );
+});
+
 // ─── Graceful shutdown signal handling (#1132) ────────────────────────
 
 describe('installSignalShutdown (#1132)', () => {