Skip to content

Commit 0cf824b

Browse files
cameroncookecodex
andcommitted
fix(mcp): Add shutdown timeout headroom
Add explicit outer-step headroom for bulk cleanup so one-item and multi-item\nsteps are less likely to report false timeouts at the boundary.\n\nIncrease debugger dispose-all shutdown budget based on active session\ncount to align with debugger manager detach/dispose behavior.\n\nAdd regression coverage for one-item bulk headroom and debugger\ndispose-all timeout budgeting. Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent 60ae62f commit 0cf824b

2 files changed

Lines changed: 90 additions & 3 deletions

File tree

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

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,44 @@ describe('runMcpShutdown', () => {
106106
expect(mocks.stopAllTrackedProcesses).toHaveBeenCalledTimes(1);
107107
});
108108

109-
it('uses an expanded timeout budget for sequential bulk cleanup steps', async () => {
109+
it('adds outer timeout headroom for one-item bulk cleanup', async () => {
110+
mocks.stopAllLogCaptures.mockImplementationOnce(async () => {
111+
await wait(1050);
112+
return { stoppedSessionCount: 1, errorCount: 0, errors: [] };
113+
});
114+
115+
const result = await runMcpShutdown({
116+
reason: 'sigterm',
117+
snapshot: {
118+
pid: 1,
119+
ppid: 1,
120+
orphaned: false,
121+
phase: 'running',
122+
shutdownReason: 'sigterm',
123+
uptimeMs: 100,
124+
rssBytes: 1,
125+
heapUsedBytes: 1,
126+
watcherRunning: false,
127+
watchedPath: null,
128+
activeOperationCount: 0,
129+
activeOperationByCategory: {},
130+
debuggerSessionCount: 0,
131+
simulatorLogSessionCount: 1,
132+
deviceLogSessionCount: 0,
133+
videoCaptureSessionCount: 0,
134+
swiftPackageProcessCount: 0,
135+
matchingMcpProcessCount: 0,
136+
matchingMcpPeerSummary: [],
137+
anomalies: [],
138+
},
139+
server: { close: async () => undefined },
140+
});
141+
142+
const simulatorLogsStep = result.steps.find((step) => step.name === 'simulator-logs.stop-all');
143+
expect(simulatorLogsStep?.status).toBe('completed');
144+
});
145+
146+
it('uses an expanded timeout budget for sequential multi-item bulk cleanup steps', async () => {
110147
mocks.stopAllLogCaptures.mockImplementationOnce(async () => {
111148
await wait(1100);
112149
return { stoppedSessionCount: 2, errorCount: 0, errors: [] };
@@ -142,4 +179,40 @@ describe('runMcpShutdown', () => {
142179
const simulatorLogsStep = result.steps.find((step) => step.name === 'simulator-logs.stop-all');
143180
expect(simulatorLogsStep?.status).toBe('completed');
144181
});
182+
183+
it('uses a larger timeout budget for debugger dispose-all', async () => {
184+
mocks.disposeAll.mockImplementationOnce(async () => {
185+
await wait(1500);
186+
});
187+
188+
const result = await runMcpShutdown({
189+
reason: 'sigterm',
190+
snapshot: {
191+
pid: 1,
192+
ppid: 1,
193+
orphaned: false,
194+
phase: 'running',
195+
shutdownReason: 'sigterm',
196+
uptimeMs: 100,
197+
rssBytes: 1,
198+
heapUsedBytes: 1,
199+
watcherRunning: false,
200+
watchedPath: null,
201+
activeOperationCount: 0,
202+
activeOperationByCategory: {},
203+
debuggerSessionCount: 1,
204+
simulatorLogSessionCount: 0,
205+
deviceLogSessionCount: 0,
206+
videoCaptureSessionCount: 0,
207+
swiftPackageProcessCount: 0,
208+
matchingMcpProcessCount: 0,
209+
matchingMcpPeerSummary: [],
210+
anomalies: [],
211+
},
212+
server: { close: async () => undefined },
213+
});
214+
215+
const debuggerStep = result.steps.find((step) => step.name === 'debugger.dispose-all');
216+
expect(debuggerStep?.status).toBe('completed');
217+
});
145218
});

src/server/mcp-shutdown.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { isTransportDisconnectReason } from './mcp-lifecycle.ts';
1818
const DISCONNECT_SERVER_CLOSE_TIMEOUT_MS = 150;
1919
const DEFAULT_SERVER_CLOSE_TIMEOUT_MS = 1000;
2020
const STEP_TIMEOUT_MS = 1000;
21+
const STEP_TIMEOUT_HEADROOM_MS = 100;
22+
const DEBUGGER_STEP_BASE_TIMEOUT_MS = 2200;
2123
const DISCONNECT_FLUSH_TIMEOUT_MS = 250;
2224
const DEFAULT_FLUSH_TIMEOUT_MS = 1500;
2325

@@ -166,7 +168,19 @@ export async function runMcpShutdown(input: {
166168

167169
const perItemTimeoutMs = STEP_TIMEOUT_MS;
168170
const bulkStepTimeoutMs = (itemCount: number): number => {
169-
return Math.max(STEP_TIMEOUT_MS, itemCount * perItemTimeoutMs);
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+
);
176+
};
177+
178+
const debuggerStepTimeoutMs = (debuggerSessionCount: number): number => {
179+
const boundedCount = Math.max(1, debuggerSessionCount);
180+
return Math.max(
181+
DEBUGGER_STEP_BASE_TIMEOUT_MS,
182+
boundedCount * STEP_TIMEOUT_MS + STEP_TIMEOUT_HEADROOM_MS,
183+
);
170184
};
171185

172186
const cleanupSteps: Array<{
@@ -182,7 +196,7 @@ export async function runMcpShutdown(input: {
182196
},
183197
{
184198
name: 'debugger.dispose-all',
185-
timeoutMs: STEP_TIMEOUT_MS,
199+
timeoutMs: debuggerStepTimeoutMs(input.snapshot.debuggerSessionCount),
186200
operation: () => getDefaultDebuggerManager().disposeAll(),
187201
},
188202
{

0 commit comments

Comments
 (0)