Skip to content

Commit c3d78ca

Browse files
cameroncookecodex
andcommitted
fix(mcp): Harden server lifecycle shutdown
Attach MCP shutdown handlers before async startup, add broken-pipe and crash handling, and explicitly stop the Xcode watcher during teardown. This closes lifecycle gaps that could leave MCP processes running after clients disappear and adds concrete shutdown reasons for investigation. Add lifecycle Sentry metrics, richer session diagnostics, and a repro script so process age, peer counts, memory, and active runtime state are visible when this issue recurs in the wild. Fixes #273 Co-Authored-By: Codex <noreply@openai.com>
1 parent 2996e8d commit c3d78ca

10 files changed

Lines changed: 1114 additions & 72 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
- Clarified configuration layering: `session_set_defaults` overrides `config.yaml`, which overrides env-based bootstrap values. See [docs/CONFIGURATION.md](docs/CONFIGURATION.md) ([#268](https://github.com/getsentry/XcodeBuildMCP/pull/268) by [@detailobsessed](https://github.com/detailobsessed)).
1414

15+
### Fixed
16+
17+
- Fixed orphaned MCP server processes by attaching shutdown handlers before async startup, explicitly stopping the Xcode watcher during teardown, and adding lifecycle diagnostics for memory and peer-process anomalies ([#273](https://github.com/getsentry/XcodeBuildMCP/issues/273)).
18+
1519
## [2.2.1]
1620

1721
- Fix AXe bundling issue.
@@ -406,4 +410,3 @@ Please note that the UI automation features are an early preview and currently i
406410
## [v1.0.1] - 2025-04-02
407411
- Initial release of XcodeBuildMCP
408412
- Basic support for building iOS and macOS applications
409-

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"prepare": "node scripts/install-git-hooks.js",
2222
"hooks:install": "node scripts/install-git-hooks.js",
2323
"generate:version": "npx tsx scripts/generate-version.ts",
24+
"repro:mcp-lifecycle-leak": "npm run build && npx tsx scripts/repro-mcp-lifecycle-leak.ts",
2425
"bundle:axe": "scripts/bundle-axe.sh",
2526
"package:macos": "scripts/package-macos-portable.sh",
2627
"package:macos:universal": "scripts/package-macos-portable.sh --universal",
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
import { spawn } from 'node:child_process';
2+
import process from 'node:process';
3+
4+
interface CliOptions {
5+
iterations: number;
6+
closeDelayMs: number;
7+
settleMs: number;
8+
}
9+
10+
interface PeerProcess {
11+
pid: number;
12+
ageSeconds: number;
13+
rssKb: number;
14+
command: string;
15+
}
16+
17+
function parseArgs(argv: string[]): CliOptions {
18+
const options: CliOptions = {
19+
iterations: 20,
20+
closeDelayMs: 0,
21+
settleMs: 2000,
22+
};
23+
24+
for (let index = 0; index < argv.length; index += 1) {
25+
const arg = argv[index];
26+
const value = argv[index + 1];
27+
28+
if (arg === '--iterations' && value) {
29+
options.iterations = Number(value);
30+
index += 1;
31+
} else if (arg === '--close-delay-ms' && value) {
32+
options.closeDelayMs = Number(value);
33+
index += 1;
34+
} else if (arg === '--settle-ms' && value) {
35+
options.settleMs = Number(value);
36+
index += 1;
37+
}
38+
}
39+
40+
if (!Number.isFinite(options.iterations) || options.iterations < 1) {
41+
throw new Error('--iterations must be a positive number');
42+
}
43+
if (!Number.isFinite(options.closeDelayMs) || options.closeDelayMs < 0) {
44+
throw new Error('--close-delay-ms must be a non-negative number');
45+
}
46+
if (!Number.isFinite(options.settleMs) || options.settleMs < 0) {
47+
throw new Error('--settle-ms must be a non-negative number');
48+
}
49+
50+
return options;
51+
}
52+
53+
function delay(ms: number): Promise<void> {
54+
return new Promise((resolve) => setTimeout(resolve, ms));
55+
}
56+
57+
function isLikelyMcpCommand(command: string): boolean {
58+
const normalized = command.toLowerCase();
59+
return (
60+
/(^|\s)mcp(\s|$)/.test(normalized) &&
61+
!/(^|\s)daemon(\s|$)/.test(normalized) &&
62+
(normalized.includes('xcodebuildmcp') ||
63+
normalized.includes('build/cli.js') ||
64+
normalized.includes('/cli.js'))
65+
);
66+
}
67+
68+
function parseElapsedSeconds(value: string): number | null {
69+
const trimmed = value.trim();
70+
if (!trimmed) {
71+
return null;
72+
}
73+
74+
const daySplit = trimmed.split('-');
75+
const timePart = daySplit.length === 2 ? daySplit[1] : daySplit[0];
76+
const dayCount = daySplit.length === 2 ? Number(daySplit[0]) : 0;
77+
const parts = timePart.split(':').map((part) => Number(part));
78+
79+
if (!Number.isFinite(dayCount) || parts.some((part) => !Number.isFinite(part))) {
80+
return null;
81+
}
82+
83+
if (parts.length === 1) {
84+
return dayCount * 86400 + parts[0];
85+
}
86+
if (parts.length === 2) {
87+
return dayCount * 86400 + parts[0] * 60 + parts[1];
88+
}
89+
if (parts.length === 3) {
90+
return dayCount * 86400 + parts[0] * 3600 + parts[1] * 60 + parts[2];
91+
}
92+
93+
return null;
94+
}
95+
96+
async function sampleMcpProcesses(): Promise<PeerProcess[]> {
97+
return new Promise((resolve, reject) => {
98+
const child = spawn('ps', ['-axo', 'pid=,etime=,rss=,command='], {
99+
stdio: ['ignore', 'pipe', 'pipe'],
100+
});
101+
let stdout = '';
102+
let stderr = '';
103+
104+
child.stdout.on('data', (chunk: Buffer) => {
105+
stdout += chunk.toString();
106+
});
107+
child.stderr.on('data', (chunk: Buffer) => {
108+
stderr += chunk.toString();
109+
});
110+
child.on('error', reject);
111+
child.on('close', (code) => {
112+
if (code !== 0) {
113+
reject(new Error(stderr || `ps exited with code ${code}`));
114+
return;
115+
}
116+
117+
const processes = stdout
118+
.split('\n')
119+
.map((line) => line.trim())
120+
.filter(Boolean)
121+
.map((line) => {
122+
const match = line.match(/^(\d+)\s+(\S+)\s+(\d+)\s+(.+)$/);
123+
if (!match) {
124+
return null;
125+
}
126+
const ageSeconds = parseElapsedSeconds(match[2]);
127+
return {
128+
pid: Number(match[1]),
129+
ageSeconds,
130+
rssKb: Number(match[3]),
131+
command: match[4],
132+
};
133+
})
134+
.filter((entry): entry is PeerProcess => {
135+
return (
136+
entry !== null &&
137+
Number.isFinite(entry.pid) &&
138+
Number.isFinite(entry.ageSeconds) &&
139+
Number.isFinite(entry.rssKb) &&
140+
isLikelyMcpCommand(entry.command)
141+
);
142+
});
143+
144+
resolve(processes);
145+
});
146+
});
147+
}
148+
149+
async function runIteration(closeDelayMs: number): Promise<boolean> {
150+
return new Promise((resolve) => {
151+
const child = spawn(process.execPath, ['build/cli.js', 'mcp'], {
152+
cwd: process.cwd(),
153+
stdio: ['pipe', 'ignore', 'ignore'],
154+
});
155+
156+
let exited = false;
157+
child.once('close', () => {
158+
exited = true;
159+
resolve(true);
160+
});
161+
child.once('error', () => {
162+
exited = true;
163+
resolve(false);
164+
});
165+
166+
setTimeout(() => {
167+
child.stdin.end();
168+
}, closeDelayMs);
169+
170+
setTimeout(
171+
() => {
172+
if (!exited) {
173+
resolve(false);
174+
}
175+
},
176+
Math.max(1000, closeDelayMs + 1000),
177+
);
178+
});
179+
}
180+
181+
async function main(): Promise<void> {
182+
const options = parseArgs(process.argv.slice(2));
183+
const before = await sampleMcpProcesses();
184+
const baselinePids = new Set(before.map((entry) => entry.pid));
185+
186+
let exitedCount = 0;
187+
for (let index = 0; index < options.iterations; index += 1) {
188+
const exited = await runIteration(options.closeDelayMs);
189+
if (exited) {
190+
exitedCount += 1;
191+
}
192+
}
193+
194+
await delay(options.settleMs);
195+
196+
const after = await sampleMcpProcesses();
197+
const lingering = after.filter((entry) => !baselinePids.has(entry.pid));
198+
199+
console.log(
200+
JSON.stringify(
201+
{
202+
iterations: options.iterations,
203+
exitedCount,
204+
baselineProcessCount: before.length,
205+
finalProcessCount: after.length,
206+
lingeringProcessCount: lingering.length,
207+
lingering: lingering.map(({ pid, ageSeconds, rssKb, command }) => ({
208+
pid,
209+
ageSeconds,
210+
rssKb,
211+
command,
212+
})),
213+
},
214+
null,
215+
2,
216+
),
217+
);
218+
219+
process.exit(lingering.length === 0 ? 0 : 1);
220+
}
221+
222+
void main().catch((error) => {
223+
console.error(error instanceof Error ? error.message : String(error));
224+
process.exit(1);
225+
});

src/mcp/resources/__tests__/session-status.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
2+
import { clearDaemonActivityRegistry } from '../../../daemon/activity-registry.ts';
23
import { getDefaultDebuggerManager } from '../../../utils/debugger/index.ts';
34
import { activeLogSessions } from '../../../utils/log_capture.ts';
45
import { activeDeviceLogSessions } from '../../../utils/log-capture/device-log-sessions.ts';
6+
import { clearAllProcesses } from '../../tools/swift-package/active-processes.ts';
57
import sessionStatusResource, { sessionStatusResourceLogic } from '../session-status.ts';
68

79
describe('session-status resource', () => {
810
beforeEach(async () => {
911
activeLogSessions.clear();
1012
activeDeviceLogSessions.clear();
13+
clearAllProcesses();
14+
clearDaemonActivityRegistry();
1115
await getDefaultDebuggerManager().disposeAll();
1216
});
1317

1418
afterEach(async () => {
1519
activeLogSessions.clear();
1620
activeDeviceLogSessions.clear();
21+
clearAllProcesses();
22+
clearDaemonActivityRegistry();
1723
await getDefaultDebuggerManager().disposeAll();
1824
});
1925

@@ -48,6 +54,14 @@ describe('session-status resource', () => {
4854
expect(parsed.logging.device.activeSessionIds).toEqual([]);
4955
expect(parsed.debug.currentSessionId).toBe(null);
5056
expect(parsed.debug.sessionIds).toEqual([]);
57+
expect(parsed.watcher).toEqual({ running: false, watchedPath: null });
58+
expect(parsed.video.activeSessionIds).toEqual([]);
59+
expect(parsed.swiftPackage.activePids).toEqual([]);
60+
expect(parsed.activity).toEqual({ activeOperationCount: 0, byCategory: {} });
61+
expect(parsed.process.pid).toBeTypeOf('number');
62+
expect(parsed.process.uptimeMs).toBeTypeOf('number');
63+
expect(parsed.process.rssBytes).toBeTypeOf('number');
64+
expect(parsed.process.heapUsedBytes).toBeTypeOf('number');
5165
});
5266
});
5367
});

0 commit comments

Comments
 (0)