fix(mcp): Bound shutdown cleanup budgets and race handling

cameroncooke · codex · cameroncooke · commit 4e527c5ac0bc · 2026-03-16T19:48:21.000Z
Scale bulk cleanup step timeouts to the tracked item counts so sequential\ncleanup work is not prematurely cut off by the outer step timer.\n\nHarden runStep timeout racing by consuming late operation rejections and\nreturning a structured failed outcome instead of risking unhandled\nrejections during shutdown.\n\nTrack the parent hard-exit helper script used by the lifecycle repro so\n--shutdown-mode parent-hard-exit works from a clean checkout.\n\nAdd shutdown regression coverage for expanded bulk timeout budgeting.

Co-Authored-By: OpenAI Codex &lt;noreply@openai.com&gt;
diff --git a/scripts/repro-mcp-parent-exit-helper.mjs b/scripts/repro-mcp-parent-exit-helper.mjs
@@ -0,0 +1,21 @@
+import { spawn } from 'node:child_process';
+import process from 'node:process';
+
+const [nodeExecPath, cliPath, cwd, exitDelayMsRaw] = process.argv.slice(2);
+const exitDelayMs = Number(exitDelayMsRaw ?? 0);
+
+if (!nodeExecPath || !cliPath || !cwd || !Number.isFinite(exitDelayMs) || exitDelayMs < 0) {
+  console.error('Usage: node repro-mcp-parent-exit-helper.mjs <nodeExecPath> <cliPath> <cwd> <exitDelayMs>');
+  process.exit(2);
+}
+
+const child = spawn(nodeExecPath, [cliPath, 'mcp'], {
+  cwd,
+  stdio: ['pipe', 'pipe', 'pipe'],
+});
+
+process.stdout.write(`${child.pid}\n`);
+
+setTimeout(() => {
+  process.exit(0);
+}, exitDelayMs);
diff --git a/src/server/__tests__/mcp-shutdown.test.ts b/src/server/__tests__/mcp-shutdown.test.ts
@@ -56,6 +56,10 @@ vi.mock('../../utils/shutdown-state.ts', () => ({
 
 import { runMcpShutdown } from '../mcp-shutdown.ts';
 
+function wait(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
 describe('runMcpShutdown', () => {
   beforeEach(() => {
     vi.clearAllMocks();
@@ -101,4 +105,41 @@ describe('runMcpShutdown', () => {
     expect(mocks.stopAllVideoCaptureSessions).toHaveBeenCalledTimes(1);
     expect(mocks.stopAllTrackedProcesses).toHaveBeenCalledTimes(1);
   });
+
+  it('uses an expanded timeout budget for sequential bulk cleanup steps', async () => {
+    mocks.stopAllLogCaptures.mockImplementationOnce(async () => {
+      await wait(1100);
+      return { stoppedSessionCount: 2, errorCount: 0, errors: [] };
+    });
+
+    const result = await runMcpShutdown({
+      reason: 'sigterm',
+      snapshot: {
+        pid: 1,
+        ppid: 1,
+        orphaned: false,
+        phase: 'running',
+        shutdownReason: 'sigterm',
+        uptimeMs: 100,
+        rssBytes: 1,
+        heapUsedBytes: 1,
+        watcherRunning: false,
+        watchedPath: null,
+        activeOperationCount: 0,
+        activeOperationByCategory: {},
+        debuggerSessionCount: 0,
+        simulatorLogSessionCount: 2,
+        deviceLogSessionCount: 0,
+        videoCaptureSessionCount: 0,
+        swiftPackageProcessCount: 0,
+        matchingMcpProcessCount: 0,
+        matchingMcpPeerSummary: [],
+        anomalies: [],
+      },
+      server: { close: async () => undefined },
+    });
+
+    const simulatorLogsStep = result.steps.find((step) => step.name === 'simulator-logs.stop-all');
+    expect(simulatorLogsStep?.status).toBe('completed');
+  });
 });
diff --git a/src/server/mcp-shutdown.ts b/src/server/mcp-shutdown.ts
@@ -37,6 +37,11 @@ interface ShutdownStepOutcome<T> {
   error?: string;
 }
 
+type RunStepRaceOutcome<T> =
+  | { kind: 'value'; value: T }
+  | { kind: 'error'; error: string }
+  | { kind: 'timed_out' };
+
 export interface McpShutdownResult {
   exitCode: number;
   transportDisconnected: boolean;
@@ -63,14 +68,19 @@ async function runStep<T>(
   let timeoutHandle: NodeJS.Timeout | null = null;
 
   try {
-    const timeoutPromise = new Promise<'timed_out'>((resolve) => {
-      timeoutHandle = createTimer(timeoutMs, () => resolve('timed_out'));
+    const timeoutPromise = new Promise<RunStepRaceOutcome<T>>((resolve) => {
+      timeoutHandle = createTimer(timeoutMs, () => resolve({ kind: 'timed_out' }));
     });
-    const operationPromise = operation();
-    const outcome = await Promise.race([
-      operationPromise.then((value) => ({ kind: 'value' as const, value })),
-      timeoutPromise.then(() => ({ kind: 'timed_out' as const })),
-    ]);
+
+    const operationOutcome = operation()
+      .then((value): RunStepRaceOutcome<T> => ({ kind: 'value', value }))
+      .catch(
+        (error): RunStepRaceOutcome<T> => ({
+          kind: 'error',
+          error: stringifyError(error),
+        }),
+      );
+    const outcome = await Promise.race([operationOutcome, timeoutPromise]);
 
     if (outcome.kind === 'timed_out') {
       return {
@@ -79,17 +89,19 @@ async function runStep<T>(
       };
     }
 
+    if (outcome.kind === 'error') {
+      return {
+        status: 'failed',
+        durationMs: Date.now() - startedAt,
+        error: outcome.error,
+      };
+    }
+
     return {
       status: 'completed',
       durationMs: Date.now() - startedAt,
       value: outcome.value,
     };
-  } catch (error) {
-    return {
-      status: 'failed',
-      durationMs: Date.now() - startedAt,
-      error: stringifyError(error),
-    };
   } finally {
     if (timeoutHandle) {
       clearTimeout(timeoutHandle);
@@ -152,19 +164,52 @@ export async function runMcpShutdown(input: {
   });
   pushStep('server.close', serverCloseOutcome);
 
-  const cleanupSteps: Array<[string, () => Promise<unknown>]> = [
-    ['watcher.stop', () => stopXcodeStateWatcher()],
-    ['xcode-tools-bridge.shutdown', () => shutdownXcodeToolsBridge()],
-    ['debugger.dispose-all', () => getDefaultDebuggerManager().disposeAll()],
-    ['simulator-logs.stop-all', () => stopAllLogCaptures(STEP_TIMEOUT_MS)],
-    ['device-logs.stop-all', () => stopAllDeviceLogCaptures(STEP_TIMEOUT_MS)],
-    ['video-capture.stop-all', () => stopAllVideoCaptureSessions(STEP_TIMEOUT_MS)],
-    ['swift-processes.stop-all', () => stopAllTrackedProcesses(STEP_TIMEOUT_MS)],
+  const perItemTimeoutMs = STEP_TIMEOUT_MS;
+  const bulkStepTimeoutMs = (itemCount: number): number => {
+    return Math.max(STEP_TIMEOUT_MS, itemCount * perItemTimeoutMs);
+  };
+
+  const cleanupSteps: Array<{
+    name: string;
+    timeoutMs: number;
+    operation: () => Promise<unknown>;
+  }> = [
+    { name: 'watcher.stop', timeoutMs: STEP_TIMEOUT_MS, operation: () => stopXcodeStateWatcher() },
+    {
+      name: 'xcode-tools-bridge.shutdown',
+      timeoutMs: STEP_TIMEOUT_MS,
+      operation: () => shutdownXcodeToolsBridge(),
+    },
+    {
+      name: 'debugger.dispose-all',
+      timeoutMs: STEP_TIMEOUT_MS,
+      operation: () => getDefaultDebuggerManager().disposeAll(),
+    },
+    {
+      name: 'simulator-logs.stop-all',
+      timeoutMs: bulkStepTimeoutMs(input.snapshot.simulatorLogSessionCount),
+      operation: () => stopAllLogCaptures(perItemTimeoutMs),
+    },
+    {
+      name: 'device-logs.stop-all',
+      timeoutMs: bulkStepTimeoutMs(input.snapshot.deviceLogSessionCount),
+      operation: () => stopAllDeviceLogCaptures(perItemTimeoutMs),
+    },
+    {
+      name: 'video-capture.stop-all',
+      timeoutMs: bulkStepTimeoutMs(input.snapshot.videoCaptureSessionCount),
+      operation: () => stopAllVideoCaptureSessions(perItemTimeoutMs),
+    },
+    {
+      name: 'swift-processes.stop-all',
+      timeoutMs: bulkStepTimeoutMs(input.snapshot.swiftPackageProcessCount),
+      operation: () => stopAllTrackedProcesses(perItemTimeoutMs),
+    },
   ];
 
-  for (const [name, operation] of cleanupSteps) {
-    const outcome = await runStep(name, STEP_TIMEOUT_MS, operation);
-    pushStep(name, outcome);
+  for (const cleanupStep of cleanupSteps) {
+    const outcome = await runStep(cleanupStep.name, cleanupStep.timeoutMs, cleanupStep.operation);
+    pushStep(cleanupStep.name, outcome);
   }
 
   const triggerError = input.error === undefined ? undefined : stringifyError(input.error);
