Skip to content

Commit 0cc00a8

Browse files
cameroncookecodex
cameroncooke
and
codex
committed
feat(mcp): Add opt-in idle timeout shutdown
Add an MCP server idle timeout controlled by XCODEBUILDMCP_MCP_IDLE_TIMEOUT_MS. The timeout is disabled by default and only shuts down after the server is idle, there are no in-flight MCP requests, and no registered runtime operations remain. Track JSON-RPC request lifecycles at the stdio transport boundary so the idle controller observes normal MCP requests without changing individual tool handlers. Add unit coverage and a process-level MCP e2e test that proves the real server exits gracefully after the configured idle timeout. Fixes #394 Co-Authored-By: Codex <noreply@openai.com>
1 parent e1e0859 commit 0cc00a8

12 files changed

Lines changed: 796 additions & 14 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Added
6+
7+
- Added opt-in MCP server idle shutdown via `XCODEBUILDMCP_MCP_IDLE_TIMEOUT_MS`, allowing unused MCP server processes to gracefully exit after a configured idle period ([#394](https://github.com/getsentry/XcodeBuildMCP/issues/394)).
8+
59
### Fixed
610

711
- Fixed remaining `/bin/sh -c` shell-injection sites in bundle ID extraction and macOS launch flows by invoking `defaults` and `PlistBuddy` directly with argv arrays so user-supplied app paths are no longer interpreted by a shell ([#367](https://github.com/getsentry/XcodeBuildMCP/issues/367)).

‎src/daemon/idle-shutdown.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,19 @@
1+
import { DEFAULT_IDLE_CHECK_INTERVAL_MS, parseIdleTimeoutEnv } from '../utils/idle-timeout.ts';
12
import type { DaemonActivitySnapshot } from './activity-registry.ts';
23

34
export const DAEMON_IDLE_TIMEOUT_ENV_KEY = 'XCODEBUILDMCP_DAEMON_IDLE_TIMEOUT_MS';
45
export const DEFAULT_DAEMON_IDLE_TIMEOUT_MS = 10 * 60 * 1000;
5-
export const DEFAULT_DAEMON_IDLE_CHECK_INTERVAL_MS = 30 * 1000;
6+
export const DEFAULT_DAEMON_IDLE_CHECK_INTERVAL_MS = DEFAULT_IDLE_CHECK_INTERVAL_MS;
67

78
export function resolveDaemonIdleTimeoutMs(
89
env: NodeJS.ProcessEnv = process.env,
910
fallbackMs: number = DEFAULT_DAEMON_IDLE_TIMEOUT_MS,
1011
): number {
11-
const raw = env[DAEMON_IDLE_TIMEOUT_ENV_KEY]?.trim();
12-
if (!raw) {
13-
return fallbackMs;
14-
}
15-
16-
const parsed = Number(raw);
17-
if (!Number.isFinite(parsed) || parsed < 0) {
18-
return fallbackMs;
19-
}
20-
21-
return Math.floor(parsed);
12+
return parseIdleTimeoutEnv({
13+
env,
14+
envKey: DAEMON_IDLE_TIMEOUT_ENV_KEY,
15+
defaultTimeoutMs: fallbackMs,
16+
}).timeoutMs;
2217
}
2318

2419
export function hasActiveRuntimeSessions(snapshot: DaemonActivitySnapshot): boolean {

‎src/server/__tests__/mcp-idle-shutdown.test.ts‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import {
3+
acquireDaemonActivity,
4+
clearDaemonActivityRegistry,
5+
} from '../../daemon/activity-registry.ts';
6+
import {
7+
DEFAULT_MCP_IDLE_TIMEOUT_MS,
8+
MCP_IDLE_TIMEOUT_ENV_KEY,
9+
createMcpIdleShutdownController,
10+
resolveMcpIdleCheckIntervalMs,
11+
resolveMcpIdleTimeoutConfig,
12+
resolveMcpIdleTimeoutMs,
13+
} from '../mcp-idle-shutdown.ts';
14+
15+
describe('MCP idle shutdown', () => {
16+
beforeEach(() => {
17+
clearDaemonActivityRegistry();
18+
});
19+
20+
afterEach(() => {
21+
clearDaemonActivityRegistry();
22+
vi.useRealTimers();
23+
vi.restoreAllMocks();
24+
});
25+
26+
describe('resolveMcpIdleTimeoutMs', () => {
27+
it('defaults to disabled when env is not set', () => {
28+
expect(resolveMcpIdleTimeoutMs({})).toBe(DEFAULT_MCP_IDLE_TIMEOUT_MS);
29+
});
30+
31+
it('uses configured positive timeout', () => {
32+
expect(resolveMcpIdleTimeoutMs({ [MCP_IDLE_TIMEOUT_ENV_KEY]: '15000' })).toBe(15000);
33+
});
34+
35+
it('uses configured zero timeout', () => {
36+
expect(resolveMcpIdleTimeoutMs({ [MCP_IDLE_TIMEOUT_ENV_KEY]: '0' })).toBe(0);
37+
});
38+
39+
it('falls back to disabled for invalid timeout values', () => {
40+
expect(resolveMcpIdleTimeoutMs({ [MCP_IDLE_TIMEOUT_ENV_KEY]: '-1' })).toBe(0);
41+
expect(resolveMcpIdleTimeoutMs({ [MCP_IDLE_TIMEOUT_ENV_KEY]: 'NaN' })).toBe(0);
42+
});
43+
44+
it('reports invalid raw values for startup logging', () => {
45+
expect(resolveMcpIdleTimeoutConfig({ [MCP_IDLE_TIMEOUT_ENV_KEY]: '-1' })).toEqual({
46+
timeoutMs: 0,
47+
rawValue: '-1',
48+
invalid: true,
49+
});
50+
});
51+
});
52+
53+
describe('resolveMcpIdleCheckIntervalMs', () => {
54+
it('keeps the check interval within the minimum and default bounds', () => {
55+
expect(resolveMcpIdleCheckIntervalMs(1, 30_000)).toBe(100);
56+
expect(resolveMcpIdleCheckIntervalMs(250, 30_000)).toBe(250);
57+
expect(resolveMcpIdleCheckIntervalMs(60_000, 30_000)).toBe(30_000);
58+
expect(resolveMcpIdleCheckIntervalMs(0, 30_000)).toBe(30_000);
59+
});
60+
});
61+
62+
it('does not start a timer when disabled', async () => {
63+
vi.useFakeTimers();
64+
const requestShutdown = vi.fn();
65+
const controller = createMcpIdleShutdownController({
66+
timeoutMs: 0,
67+
intervalMs: 10,
68+
requestShutdown,
69+
});
70+
71+
controller.start();
72+
await vi.advanceTimersByTimeAsync(100);
73+
74+
expect(requestShutdown).not.toHaveBeenCalled();
75+
});
76+
77+
it('starts an unref interval when enabled', () => {
78+
const unref = vi.fn();
79+
const timer = { unref } as unknown as NodeJS.Timeout;
80+
const setIntervalSpy = vi.spyOn(globalThis, 'setInterval').mockReturnValue(timer);
81+
const clearIntervalSpy = vi
82+
.spyOn(globalThis, 'clearInterval')
83+
.mockImplementation(() => undefined);
84+
const controller = createMcpIdleShutdownController({
85+
timeoutMs: 1000,
86+
intervalMs: 50,
87+
requestShutdown: vi.fn(),
88+
});
89+
90+
controller.start();
91+
controller.stop();
92+
93+
expect(setIntervalSpy).toHaveBeenCalledTimes(1);
94+
expect(setIntervalSpy).toHaveBeenCalledWith(expect.any(Function), 50);
95+
expect(unref).toHaveBeenCalledTimes(1);
96+
expect(clearIntervalSpy).toHaveBeenCalledWith(timer);
97+
});
98+
99+
it('requests shutdown after the configured idle period', async () => {
100+
vi.useFakeTimers();
101+
let nowMs = 0;
102+
const requestShutdown = vi.fn();
103+
const controller = createMcpIdleShutdownController({
104+
timeoutMs: 1000,
105+
intervalMs: 100,
106+
nowMs: () => nowMs,
107+
requestShutdown,
108+
});
109+
110+
controller.start();
111+
nowMs = 1000;
112+
await vi.advanceTimersByTimeAsync(100);
113+
114+
expect(requestShutdown).toHaveBeenCalledTimes(1);
115+
});
116+
117+
it('does not request shutdown while a request is in flight', async () => {
118+
vi.useFakeTimers();
119+
let nowMs = 0;
120+
const requestShutdown = vi.fn();
121+
const controller = createMcpIdleShutdownController({
122+
timeoutMs: 1000,
123+
intervalMs: 100,
124+
nowMs: () => nowMs,
125+
requestShutdown,
126+
});
127+
128+
controller.start();
129+
controller.markRequestStarted();
130+
nowMs = 2000;
131+
await vi.advanceTimersByTimeAsync(100);
132+
133+
expect(requestShutdown).not.toHaveBeenCalled();
134+
expect(controller.getInFlightRequestCount()).toBe(1);
135+
});
136+
137+
it('does not request shutdown while daemon activity is active', async () => {
138+
vi.useFakeTimers();
139+
let nowMs = 0;
140+
const requestShutdown = vi.fn();
141+
const release = acquireDaemonActivity('video.capture');
142+
const controller = createMcpIdleShutdownController({
143+
timeoutMs: 1000,
144+
intervalMs: 100,
145+
nowMs: () => nowMs,
146+
requestShutdown,
147+
});
148+
149+
controller.start();
150+
nowMs = 1000;
151+
await vi.advanceTimersByTimeAsync(100);
152+
expect(requestShutdown).not.toHaveBeenCalled();
153+
154+
release();
155+
await vi.advanceTimersByTimeAsync(100);
156+
expect(requestShutdown).toHaveBeenCalledTimes(1);
157+
});
158+
159+
it('does not request shutdown after the controller is stopped', async () => {
160+
vi.useFakeTimers();
161+
let nowMs = 0;
162+
const requestShutdown = vi.fn();
163+
const controller = createMcpIdleShutdownController({
164+
timeoutMs: 1000,
165+
intervalMs: 100,
166+
nowMs: () => nowMs,
167+
requestShutdown,
168+
});
169+
170+
controller.start();
171+
controller.stop();
172+
nowMs = 1000;
173+
await vi.advanceTimersByTimeAsync(100);
174+
175+
expect(requestShutdown).not.toHaveBeenCalled();
176+
});
177+
178+
it('resets the idle baseline when a request completes', async () => {
179+
vi.useFakeTimers();
180+
let nowMs = 0;
181+
const requestShutdown = vi.fn();
182+
const controller = createMcpIdleShutdownController({
183+
timeoutMs: 1000,
184+
intervalMs: 100,
185+
nowMs: () => nowMs,
186+
requestShutdown,
187+
});
188+
189+
controller.start();
190+
controller.markRequestStarted();
191+
nowMs = 800;
192+
controller.markRequestCompleted();
193+
expect(controller.getInFlightRequestCount()).toBe(0);
194+
195+
nowMs = 1700;
196+
await vi.advanceTimersByTimeAsync(100);
197+
expect(requestShutdown).not.toHaveBeenCalled();
198+
199+
nowMs = 1800;
200+
await vi.advanceTimersByTimeAsync(100);
201+
expect(requestShutdown).toHaveBeenCalledTimes(1);
202+
});
203+
});

‎src/server/__tests__/mcp-lifecycle.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ describe('mcp lifecycle snapshot', () => {
290290
expect(isTransportDisconnectReason('stdin-close')).toBe(true);
291291
expect(isTransportDisconnectReason('stdout-error')).toBe(true);
292292
expect(isTransportDisconnectReason('stderr-error')).toBe(true);
293+
expect(isTransportDisconnectReason('idle-timeout')).toBe(false);
293294
expect(isTransportDisconnectReason('sigterm')).toBe(false);
294295
});
295296
});

‎src/server/__tests__/server.test.ts‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js';
2+
import type {
3+
Transport,
4+
TransportSendOptions,
5+
} from '@modelcontextprotocol/sdk/shared/transport.js';
6+
import { describe, expect, it, vi } from 'vitest';
7+
import {
8+
instrumentMcpRequestLifecycle,
9+
type McpRequestLifecycleObserver,
10+
} from '../request-lifecycle.ts';
11+
12+
class TestTransport implements Transport {
13+
onmessage?: Transport['onmessage'];
14+
sentMessages: JSONRPCMessage[] = [];
15+
failNextSend = false;
16+
17+
async start(): Promise<void> {
18+
return undefined;
19+
}
20+
21+
async close(): Promise<void> {
22+
return undefined;
23+
}
24+
25+
async send(message: JSONRPCMessage, _options?: TransportSendOptions): Promise<void> {
26+
if (this.failNextSend) {
27+
this.failNextSend = false;
28+
throw new Error('broken pipe');
29+
}
30+
31+
this.sentMessages.push(message);
32+
}
33+
}
34+
35+
async function createStartedInstrumentedTransport(observer: McpRequestLifecycleObserver): Promise<{
36+
transport: TestTransport;
37+
downstreamOnMessage: ReturnType<typeof vi.fn>;
38+
}> {
39+
const transport = new TestTransport();
40+
const downstreamOnMessage = vi.fn();
41+
instrumentMcpRequestLifecycle(transport, observer);
42+
43+
transport.onmessage = downstreamOnMessage;
44+
await transport.start();
45+
46+
return { transport, downstreamOnMessage };
47+
}
48+
49+
describe('MCP server transport request lifecycle instrumentation', () => {
50+
it('marks request start and matching result response completion', async () => {
51+
const onRequestStarted = vi.fn();
52+
const onRequestCompleted = vi.fn();
53+
const { transport, downstreamOnMessage } = await createStartedInstrumentedTransport({
54+
onRequestStarted,
55+
onRequestCompleted,
56+
});
57+
58+
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'tools/list' });
59+
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
60+
61+
expect(onRequestStarted).toHaveBeenCalledTimes(1);
62+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
63+
expect(downstreamOnMessage).toHaveBeenCalledTimes(1);
64+
});
65+
66+
it('marks matching error response completion', async () => {
67+
const onRequestCompleted = vi.fn();
68+
const { transport } = await createStartedInstrumentedTransport({ onRequestCompleted });
69+
70+
transport.onmessage?.({ jsonrpc: '2.0', id: 2, method: 'tools/call' });
71+
await transport.send({
72+
jsonrpc: '2.0',
73+
id: 2,
74+
error: { code: -32603, message: 'failed' },
75+
});
76+
77+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
78+
});
79+
80+
it('ignores notifications', async () => {
81+
const onRequestStarted = vi.fn();
82+
const onRequestCompleted = vi.fn();
83+
const { transport } = await createStartedInstrumentedTransport({
84+
onRequestStarted,
85+
onRequestCompleted,
86+
});
87+
88+
transport.onmessage?.({ jsonrpc: '2.0', method: 'notifications/initialized' });
89+
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
90+
91+
expect(onRequestStarted).not.toHaveBeenCalled();
92+
expect(onRequestCompleted).not.toHaveBeenCalled();
93+
});
94+
95+
it('ignores unmatched responses', async () => {
96+
const onRequestCompleted = vi.fn();
97+
const { transport } = await createStartedInstrumentedTransport({ onRequestCompleted });
98+
99+
await transport.send({ jsonrpc: '2.0', id: 'missing', result: {} });
100+
101+
expect(onRequestCompleted).not.toHaveBeenCalled();
102+
});
103+
104+
it('completes duplicate responses only once', async () => {
105+
const onRequestCompleted = vi.fn();
106+
const { transport } = await createStartedInstrumentedTransport({ onRequestCompleted });
107+
108+
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'initialize' });
109+
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
110+
await transport.send({ jsonrpc: '2.0', id: '1', result: {} });
111+
112+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
113+
});
114+
115+
it('marks completion after a matching response send rejects', async () => {
116+
const onRequestCompleted = vi.fn();
117+
const { transport } = await createStartedInstrumentedTransport({ onRequestCompleted });
118+
transport.failNextSend = true;
119+
120+
transport.onmessage?.({ jsonrpc: '2.0', id: '1', method: 'initialize' });
121+
await expect(transport.send({ jsonrpc: '2.0', id: '1', result: {} })).rejects.toThrow(
122+
'broken pipe',
123+
);
124+
125+
expect(onRequestCompleted).toHaveBeenCalledTimes(1);
126+
});
127+
});

0 commit comments

Comments
 (0)