Skip to content

Commit 22660e0

Browse files
committed
refactor(mcp): Simplify shutdown and lifecycle code
Remove dead code paths, redundant wrappers, and duplicated type definitions across the MCP shutdown pipeline for clarity.
1 parent 0cf824b commit 22660e0

File tree

6 files changed

+53
-70
lines changed

6 files changed

+53
-70
lines changed

src/mcp/tools/swift-package/active-processes.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/**
2-
* Shared process state management for Swift Package tools
3-
* This module provides a centralized way to manage active processes
4-
* between swift_package_run and swift_package_stop tools
2+
* Shared process state management for Swift Package tools.
3+
*
4+
* Tracks active child processes so they can be individually stopped
5+
* or bulk-terminated during server shutdown.
56
*/
67

78
interface TrackedProcess {
@@ -25,28 +26,28 @@ export interface ProcessInfo {
2526

2627
export const activeProcesses = new Map<number, ProcessInfo>();
2728

28-
export const getProcess = (pid: number): ProcessInfo | undefined => {
29+
export function getProcess(pid: number): ProcessInfo | undefined {
2930
return activeProcesses.get(pid);
30-
};
31+
}
3132

32-
export const addProcess = (pid: number, processInfo: ProcessInfo): void => {
33+
export function addProcess(pid: number, processInfo: ProcessInfo): void {
3334
const existing = activeProcesses.get(pid);
3435
existing?.releaseActivity?.();
3536
activeProcesses.set(pid, processInfo);
36-
};
37+
}
3738

38-
export const removeProcess = (pid: number): boolean => {
39+
export function removeProcess(pid: number): boolean {
3940
const existing = activeProcesses.get(pid);
4041
existing?.releaseActivity?.();
4142
return activeProcesses.delete(pid);
42-
};
43+
}
4344

44-
export const clearAllProcesses = (): void => {
45+
export function clearAllProcesses(): void {
4546
for (const processInfo of activeProcesses.values()) {
4647
processInfo.releaseActivity?.();
4748
}
4849
activeProcesses.clear();
49-
};
50+
}
5051

5152
function createTimeoutPromise(timeoutMs: number): Promise<'timed_out'> {
5253
return new Promise((resolve) => {
@@ -79,7 +80,7 @@ async function terminateProcess(
7980
return;
8081
}
8182
info.process.on('exit', onExit);
82-
}).catch(() => 'exited' as const);
83+
});
8384

8485
const outcome = await Promise.race([exitPromise, createTimeoutPromise(timeoutMs)]);
8586
if (outcome === 'timed_out') {

src/server/mcp-lifecycle.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,15 @@ interface PeerProcessSample {
7474
peers: McpPeerProcessSummary[];
7575
}
7676

77-
interface LifecycleStdinLike {
78-
once(event: string, listener: (...args: unknown[]) => void): this;
79-
removeListener(event: string, listener: (...args: unknown[]) => void): this;
80-
}
81-
82-
interface LifecycleStdoutLike {
77+
interface LifecycleStreamLike {
8378
once(event: string, listener: (...args: unknown[]) => void): this;
8479
removeListener(event: string, listener: (...args: unknown[]) => void): this;
8580
}
8681

8782
interface LifecycleProcessLike {
88-
stdin: LifecycleStdinLike;
89-
stdout?: LifecycleStdoutLike;
90-
stderr?: LifecycleStdoutLike;
83+
stdin: LifecycleStreamLike;
84+
stdout?: LifecycleStreamLike;
85+
stderr?: LifecycleStreamLike;
9186
once(event: string, listener: (...args: unknown[]) => void): this;
9287
removeListener(event: string, listener: (...args: unknown[]) => void): this;
9388
}
@@ -178,7 +173,7 @@ export function classifyMcpLifecycleAnomalies(
178173
anomalies.add('long-lived-high-rss');
179174
}
180175

181-
return Array.from(anomalies.values()).sort();
176+
return [...anomalies].sort();
182177
}
183178

184179
function isLikelyMcpProcessCommand(command: string): boolean {

src/server/mcp-shutdown.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,8 @@ export async function runMcpShutdown(input: {
166166
});
167167
pushStep('server.close', serverCloseOutcome);
168168

169-
const perItemTimeoutMs = STEP_TIMEOUT_MS;
170169
const bulkStepTimeoutMs = (itemCount: number): number => {
171-
const boundedCount = Math.max(1, itemCount);
172-
return Math.max(
173-
STEP_TIMEOUT_MS + STEP_TIMEOUT_HEADROOM_MS,
174-
boundedCount * perItemTimeoutMs + STEP_TIMEOUT_HEADROOM_MS,
175-
);
170+
return Math.max(1, itemCount) * STEP_TIMEOUT_MS + STEP_TIMEOUT_HEADROOM_MS;
176171
};
177172

178173
const debuggerStepTimeoutMs = (debuggerSessionCount: number): number => {
@@ -202,22 +197,22 @@ export async function runMcpShutdown(input: {
202197
{
203198
name: 'simulator-logs.stop-all',
204199
timeoutMs: bulkStepTimeoutMs(input.snapshot.simulatorLogSessionCount),
205-
operation: () => stopAllLogCaptures(perItemTimeoutMs),
200+
operation: () => stopAllLogCaptures(STEP_TIMEOUT_MS),
206201
},
207202
{
208203
name: 'device-logs.stop-all',
209204
timeoutMs: bulkStepTimeoutMs(input.snapshot.deviceLogSessionCount),
210-
operation: () => stopAllDeviceLogCaptures(perItemTimeoutMs),
205+
operation: () => stopAllDeviceLogCaptures(STEP_TIMEOUT_MS),
211206
},
212207
{
213208
name: 'video-capture.stop-all',
214209
timeoutMs: bulkStepTimeoutMs(input.snapshot.videoCaptureSessionCount),
215-
operation: () => stopAllVideoCaptureSessions(perItemTimeoutMs),
210+
operation: () => stopAllVideoCaptureSessions(STEP_TIMEOUT_MS),
216211
},
217212
{
218213
name: 'swift-processes.stop-all',
219214
timeoutMs: bulkStepTimeoutMs(input.snapshot.swiftPackageProcessCount),
220-
operation: () => stopAllTrackedProcesses(perItemTimeoutMs),
215+
operation: () => stopAllTrackedProcesses(STEP_TIMEOUT_MS),
221216
},
222217
];
223218

src/server/start-mcp-server.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
} from '../utils/sentry.ts';
1919
import { version } from '../version.ts';
2020
import process from 'node:process';
21-
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
2221
import { bootstrapServer } from './bootstrap.ts';
2322
import { createStartupProfiler, getStartupProfileNowMs } from './startup-profiler.ts';
2423
import { getConfig } from '../utils/config-store.ts';
@@ -38,17 +37,13 @@ export async function startMcpServer(): Promise<void> {
3837
const isCrash = reason === 'uncaught-exception' || reason === 'unhandled-rejection';
3938
const event = isCrash ? 'crash' : 'shutdown';
4039

41-
if (reason === 'stdin-end') {
42-
log('info', 'MCP stdin ended; shutting down MCP server');
43-
} else if (reason === 'stdin-close') {
44-
log('info', 'MCP stdin closed; shutting down MCP server');
45-
} else if (reason === 'stdout-error') {
46-
log('info', 'MCP stdout pipe broke; shutting down MCP server');
47-
} else if (reason === 'stderr-error') {
48-
log('info', 'MCP stderr pipe broke; shutting down MCP server');
49-
} else {
50-
log('info', `MCP shutdown requested: ${reason}`);
51-
}
40+
const transportMessages: Record<string, string> = {
41+
'stdin-end': 'MCP stdin ended; shutting down MCP server',
42+
'stdin-close': 'MCP stdin closed; shutting down MCP server',
43+
'stdout-error': 'MCP stdout pipe broke; shutting down MCP server',
44+
'stderr-error': 'MCP stderr pipe broke; shutting down MCP server',
45+
};
46+
log('info', transportMessages[reason] ?? `MCP shutdown requested: ${reason}`);
5247

5348
if (!isTransportDisconnectReason(reason)) {
5449
recordMcpLifecycleMetric({
@@ -75,7 +70,7 @@ export async function startMcpServer(): Promise<void> {
7570
reason,
7671
error,
7772
snapshot,
78-
server: server ? ({ close: () => server.close() } as Pick<McpServer, 'close'>) : null,
73+
server,
7974
});
8075

8176
lifecycle.detachProcessHandlers();

src/utils/sentry.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import * as Sentry from '@sentry/node';
88
import { version } from '../version.ts';
99
import { isSentryCaptureSealed } from './shutdown-state.ts';
10+
1011
const USER_HOME_PATH_PATTERN = /\/Users\/[^/\s]+/g;
1112
const XCODE_VERSION_PATTERN = /^Xcode\s+(.+)$/m;
1213
const XCODE_BUILD_PATTERN = /^Build version\s+(.+)$/m;
@@ -400,20 +401,22 @@ export function captureMcpShutdownSummary(summary: McpShutdownSummaryEvent): voi
400401
}
401402

402403
try {
403-
const anomalies =
404-
(summary.snapshot as { anomalies?: unknown }).anomalies &&
405-
Array.isArray((summary.snapshot as { anomalies?: unknown }).anomalies)
406-
? (summary.snapshot as { anomalies: unknown[] }).anomalies.length
407-
: 0;
404+
const snapshotAnomalies = (summary.snapshot as { anomalies?: unknown }).anomalies;
405+
const anomalyCount = Array.isArray(snapshotAnomalies) ? snapshotAnomalies.length : 0;
408406

409-
const level =
407+
const isCrashReason =
410408
summary.reason === 'startup-failure' ||
411409
summary.reason === 'uncaught-exception' ||
412-
summary.reason === 'unhandled-rejection'
413-
? 'error'
414-
: summary.cleanupFailureCount > 0 || anomalies > 0
415-
? 'warning'
416-
: 'info';
410+
summary.reason === 'unhandled-rejection';
411+
412+
let level: 'error' | 'warning' | 'info';
413+
if (isCrashReason) {
414+
level = 'error';
415+
} else if (summary.cleanupFailureCount > 0 || anomalyCount > 0) {
416+
level = 'warning';
417+
} else {
418+
level = 'info';
419+
}
417420

418421
Sentry.captureEvent({
419422
level,

src/utils/shutdown-state.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import process from 'node:process';
2+
13
type WritableMethod = (chunk: unknown, encoding?: unknown, callback?: unknown) => boolean;
24

35
interface StdioWriteTarget {
@@ -10,20 +12,12 @@ let originalStdoutWrite: WritableMethod | null = null;
1012
let originalStderrWrite: WritableMethod | null = null;
1113

1214
function createSuppressedWrite(): WritableMethod {
13-
return (chunk: unknown, encoding?: unknown, callback?: unknown): boolean => {
14-
const maybeEncoding = typeof encoding === 'function' ? undefined : encoding;
15-
const maybeCallback =
16-
typeof encoding === 'function'
17-
? encoding
18-
: typeof callback === 'function'
19-
? callback
20-
: undefined;
21-
22-
void chunk;
23-
void maybeEncoding;
24-
25-
if (typeof maybeCallback === 'function') {
26-
queueMicrotask(() => maybeCallback(null));
15+
return (_chunk: unknown, encoding?: unknown, callback?: unknown): boolean => {
16+
const resolvedCallback =
17+
typeof encoding === 'function' ? encoding : typeof callback === 'function' ? callback : null;
18+
19+
if (typeof resolvedCallback === 'function') {
20+
queueMicrotask(() => resolvedCallback(null));
2721
}
2822

2923
return true;

0 commit comments

Comments
 (0)