Skip to content

Commit d099331

Browse files
cameroncookecodex
andcommitted
fix(simulator): Reap orphaned OSLog helpers by workspace
Persist workspace identity with simulator OSLog helper ownership and reconcile same-workspace helpers whose owner PID is no longer alive. This lets a new server startup clean up detached log stream helpers left behind by abnormal termination without killing helpers from other active workspaces or sessions. Remove the dead legacy simulator log capture path and its lifecycle/status plumbing so cleanup behavior is owned by the durable OSLog registry path. Fixes #382 Co-Authored-By: Codex <noreply@openai.com>
1 parent a7628c9 commit d099331

22 files changed

Lines changed: 672 additions & 1048 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## [Unreleased]
4+
5+
### Fixed
6+
7+
- Fixed simulator OSLog helper cleanup so server and daemon startup reconcile same-workspace orphaned log streams without stopping helpers owned by live sessions in other workspaces ([#382](https://github.com/getsentry/XcodeBuildMCP/issues/382)).
8+
39
## [2.5.0-beta.1]
410

511
### Breaking
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
# Investigation: Orphaned simctl OSLog processes when MCP server exits abnormally
2+
3+
**Issue:** [#382](https://github.com/getsentry/XcodeBuildMCP/issues/382)
4+
**Date:** 2026-05-01
5+
**Status:** Confirmed and fixed for simulator OSLog helpers.
6+
7+
## Summary
8+
9+
The issue described orphaned simulator helper processes after abnormal server exits. The final fix is deliberately narrow:
10+
11+
- `simctl spawn … log stream …` OSLog helpers are registry-backed and now reconciled safely across MCP/daemon restarts.
12+
- `simctl launch --console-pty …` remains intentionally unregistered. It is tied to the launched app lifecycle, and adding a durable registry for it would overreach the accepted design.
13+
- The old `src/utils/log_capture.ts` path was dead production code and has been removed.
14+
15+
The fix does not change public tool schemas.
16+
17+
## Final design
18+
19+
### OSLog helpers are workspace-scoped
20+
21+
Durable OSLog registry records now store owner identity as:
22+
23+
```ts
24+
owner: {
25+
instanceId: string;
26+
pid: number;
27+
workspaceKey: string;
28+
}
29+
```
30+
31+
The `workspaceKey` is configured during MCP, CLI, and daemon startup from the same workspace-root hashing used by daemon socket paths. Existing internal records without `owner.workspaceKey` are treated as invalid and pruned. No registry versioning or migration was added.
32+
33+
### Reconciliation only reaps safe orphans
34+
35+
Startup reconciliation runs for both MCP server startup and daemon startup. It scans durable OSLog records and only stops a helper when all of these are true:
36+
37+
1. The record belongs to the current workspace.
38+
2. The owner PID is not the current process.
39+
3. The owner PID is no longer alive.
40+
4. The helper process still matches the expected `simctl spawn … log stream …` command.
41+
42+
Records from other workspaces are skipped. Records owned by live processes are skipped, including live foreign MCP/daemon sessions in the same workspace.
43+
44+
### App-scoped cleanup is no longer global
45+
46+
`stop_app_sim` and relaunch cleanup still clean OSLog helpers for the target simulator/bundle, but only when the record is:
47+
48+
- owned by the current runtime instance, or
49+
- in the same workspace with a dead owner PID.
50+
51+
This prevents one active session from killing another active session’s OSLog helper.
52+
53+
### Last-chance cleanup covers local live helpers
54+
55+
MCP lifecycle and daemon lifecycle now install synchronous `exit` cleanup for in-process OSLog sessions. This only signals locally known child processes; it does not perform async registry deletion. If a helper survives, the next startup reconciliation can reap it from the durable registry.
56+
57+
Daemon crash handling was also brought closer to MCP parity with `uncaughtException` and `unhandledRejection` handling that requests shutdown with a non-zero exit code.
58+
59+
### Dead log-capture path removed
60+
61+
`src/utils/log_capture.ts` duplicated old simulator log-capture behavior, but there were no production callers. It also left misleading lifecycle/status fields such as `activeLogSessions` and `simulatorLogSessionCount`.
62+
63+
Removed:
64+
65+
- `src/utils/log_capture.ts`
66+
- `src/utils/__tests__/log_capture.test.ts`
67+
- `src/utils/__tests__/log_capture_escape.test.ts`
68+
- legacy re-exports from `src/utils/log-capture/index.ts`
69+
- legacy shutdown/status references to `stopAllLogCaptures`, `activeLogSessions`, and `simulatorLogSessionCount`
70+
71+
## Why console-PTY is not registered
72+
73+
`simctl launch --console-pty --terminate-running-process …` is app-lifecycle-bound. It exists to capture the launched app’s stdout/stderr while the app is running. The accepted fix avoids adding a console-PTY registry because that would create a second durable lifecycle model for a helper whose lifetime is already coupled to the app launch.
74+
75+
The strategic cleanup surface is the OSLog helper registry, because OSLog helpers are detached, long-lived, and already have durable records that can be reconciled after abnormal exits.
76+
77+
## Verification expectations
78+
79+
Automated coverage should verify:
80+
81+
- OSLog registry records require `owner.workspaceKey`.
82+
- records without `owner.workspaceKey` are pruned.
83+
- same-workspace dead-owner OSLog helpers are reconciled.
84+
- other-workspace records are skipped.
85+
- same-workspace live-owner records are skipped.
86+
- app-scoped cleanup skips live foreign owners.
87+
- lifecycle snapshots and session status do not expose deleted legacy log-capture fields.
88+
- synchronous exit cleanup signals only live local OSLog helpers.
89+
90+
Manual smoke testing should verify:
91+
92+
1. Clean shutdown stops helpers owned by the current server.
93+
2. After `SIGKILL` of an MCP server, restarting in the same workspace reaps the orphaned `simctl spawn … log stream …` helper.
94+
3. Two live sessions in the same workspace do not kill each other’s OSLog helpers.
95+
4. Startup from workspace A does not kill workspace B helpers.
96+
5. `stop_app_sim` does not kill live foreign-owner OSLog helpers.
97+
98+
## Known boundary
99+
100+
This fix does not promise to kill every possible `simctl launch --console-pty` process. That helper is intentionally not part of the durable OSLog registry. The implemented production safety guarantee is: workspace-scoped OSLog helpers are cleaned up without killing helpers owned by other live sessions.

src/cli.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { listCliWorkflowIdsFromManifest } from './runtime/tool-catalog.ts';
88
import { flushAndCloseSentry, initSentry, recordBootstrapDurationMetric } from './utils/sentry.ts';
99
import { coerceLogLevel, setLogLevel, type LogLevel } from './utils/logger.ts';
1010
import { hydrateSentryDisabledEnvFromProjectConfig } from './utils/sentry-config.ts';
11+
import { configureRuntimeWorkspaceKey } from './utils/runtime-instance.ts';
1112

1213
function findTopLevelCommand(argv: string[]): string | undefined {
1314
const flagsWithValue = new Set(['--socket', '--log-level', '--style']);
@@ -133,6 +134,7 @@ async function main(): Promise<void> {
133134
cwd: result.runtime.cwd,
134135
projectConfigPath: result.configPath,
135136
});
137+
configureRuntimeWorkspaceKey(workspaceKey);
136138

137139
const cliExposedWorkflowIds = await listCliWorkflowIdsFromManifest({
138140
excludeWorkflows: ['session-management', 'workflow-discovery'],

src/daemon.ts

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ import {
4242
} from './utils/sentry.ts';
4343
import { isXcodemakeBinaryAvailable, isXcodemakeEnabled } from './utils/xcodemake/index.ts';
4444
import { hydrateSentryDisabledEnvFromProjectConfig } from './utils/sentry-config.ts';
45+
import { configureRuntimeWorkspaceKey } from './utils/runtime-instance.ts';
46+
import {
47+
reconcileSimulatorLaunchOsLogOrphansForWorkspace,
48+
terminateLiveSimulatorLaunchOsLogSessionsSync,
49+
} from './utils/log-capture/index.ts';
4550

4651
async function checkExistingDaemon(socketPath: string): Promise<boolean> {
4752
return new Promise<boolean>((resolve) => {
@@ -128,6 +133,7 @@ async function main(): Promise<void> {
128133
cwd: result.runtime.cwd,
129134
projectConfigPath: result.configPath,
130135
});
136+
configureRuntimeWorkspaceKey(workspaceKey);
131137

132138
const logPath = resolveDaemonLogPath(workspaceKey);
133139
if (logPath) {
@@ -153,6 +159,20 @@ async function main(): Promise<void> {
153159

154160
log('info', `[Daemon] Workspace: ${workspaceRoot}`);
155161
log('info', `[Daemon] Socket: ${socketPath}`);
162+
try {
163+
const reconciliation = await reconcileSimulatorLaunchOsLogOrphansForWorkspace(workspaceKey);
164+
if (reconciliation.stoppedSessionCount > 0 || reconciliation.errorCount > 0) {
165+
log(
166+
reconciliation.errorCount > 0 ? 'warn' : 'info',
167+
`[Daemon] Simulator OSLog reconciliation: ${JSON.stringify(reconciliation)}`,
168+
);
169+
}
170+
} catch (error) {
171+
log(
172+
'warn',
173+
`[Daemon] Simulator OSLog reconciliation failed: ${error instanceof Error ? error.message : String(error)}`,
174+
);
175+
}
156176
if (logPath) {
157177
log('info', `[Daemon] Logs: ${logPath}`);
158178
}
@@ -268,7 +288,7 @@ async function main(): Promise<void> {
268288
};
269289

270290
// Unified shutdown handler
271-
const shutdown = (): void => {
291+
const shutdown = (exitCode = 0): void => {
272292
if (isShuttingDown) {
273293
return;
274294
}
@@ -292,7 +312,7 @@ async function main(): Promise<void> {
292312

293313
log('info', '[Daemon] Cleanup complete');
294314
void flushAndCloseSentry(2000).finally(() => {
295-
process.exit(0);
315+
process.exit(exitCode);
296316
});
297317
});
298318

@@ -393,8 +413,20 @@ async function main(): Promise<void> {
393413
});
394414
});
395415

396-
process.on('SIGTERM', shutdown);
397-
process.on('SIGINT', shutdown);
416+
const handleCrash = (reason: unknown): void => {
417+
recordDaemonLifecycleMetric('crash');
418+
const message = reason instanceof Error ? reason.message : String(reason);
419+
log('error', `[Daemon] Crash: ${message}`, { sentry: true });
420+
shutdown(1);
421+
};
422+
423+
process.on('exit', () => {
424+
terminateLiveSimulatorLaunchOsLogSessionsSync();
425+
});
426+
process.on('SIGTERM', () => shutdown(0));
427+
process.on('SIGINT', () => shutdown(0));
428+
process.on('uncaughtException', handleCrash);
429+
process.on('unhandledRejection', handleCrash);
398430
}
399431

400432
main().catch(async (err) => {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { ChildProcess } from 'node:child_process';
77
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
88
import { clearDaemonActivityRegistry } from '../../../daemon/activity-registry.ts';
99
import { getDefaultDebuggerManager } from '../../../utils/debugger/index.ts';
10-
import { activeLogSessions } from '../../../utils/log_capture.ts';
1110
import { activeDeviceLogSessions } from '../../../utils/log-capture/device-log-sessions.ts';
1211
import {
1312
clearAllSimulatorLaunchOsLogSessionsForTests,
@@ -34,9 +33,12 @@ describe('session-status resource', () => {
3433
beforeEach(async () => {
3534
registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-session-status-'));
3635
setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir);
37-
setRuntimeInstanceForTests({ instanceId: 'session-status-test', pid: process.pid });
36+
setRuntimeInstanceForTests({
37+
instanceId: 'session-status-test',
38+
pid: process.pid,
39+
workspaceKey: 'workspace-a',
40+
});
3841
setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true);
39-
activeLogSessions.clear();
4042
activeDeviceLogSessions.clear();
4143
clearAllProcesses();
4244
await clearAllSimulatorLaunchOsLogSessionsForTests();
@@ -45,7 +47,6 @@ describe('session-status resource', () => {
4547
});
4648

4749
afterEach(async () => {
48-
activeLogSessions.clear();
4950
activeDeviceLogSessions.clear();
5051
clearAllProcesses();
5152
await clearAllSimulatorLaunchOsLogSessionsForTests();
@@ -64,7 +65,6 @@ describe('session-status resource', () => {
6465
expect(result.contents).toHaveLength(1);
6566
const parsed = JSON.parse(result.contents[0].text);
6667

67-
expect(parsed.logging.simulator.activeSessionIds).toEqual([]);
6868
expect(parsed.logging.simulator.activeLaunchOsLogSessions).toEqual([]);
6969
expect(parsed.logging.device.activeSessionIds).toEqual([]);
7070
expect(parsed.debug.currentSessionId).toBe(null);

src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ describe('stop_app_sim tool', () => {
6767
trackedChildren.clear();
6868
registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-stop-app-sim-'));
6969
setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir);
70-
setRuntimeInstanceForTests({ instanceId: 'stop-app-sim-test', pid: process.pid });
70+
setRuntimeInstanceForTests({
71+
instanceId: 'stop-app-sim-test',
72+
pid: process.pid,
73+
workspaceKey: 'workspace-a',
74+
});
7175
setSimulatorLaunchOsLogRecordActiveOverrideForTests(async (record) => {
7276
const child = trackedChildren.get(record.helperPid);
7377
return child ? child.exitCode == null : true;

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ describe('mcp lifecycle coordinator', () => {
6161
beforeEach(async () => {
6262
registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-mcp-lifecycle-'));
6363
setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir);
64-
setRuntimeInstanceForTests({ instanceId: 'mcp-lifecycle-test', pid: process.pid });
64+
setRuntimeInstanceForTests({
65+
instanceId: 'mcp-lifecycle-test',
66+
pid: process.pid,
67+
workspaceKey: 'workspace-a',
68+
});
6569
setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true);
6670
await clearAllSimulatorLaunchOsLogSessionsForTests();
6771
vi.restoreAllMocks();
@@ -130,6 +134,29 @@ describe('mcp lifecycle coordinator', () => {
130134
expect(onShutdown.mock.calls[0]?.[0]?.reason).toBe('unhandled-rejection');
131135
});
132136

137+
it('terminates live local OSLog sessions on process exit', async () => {
138+
const processRef = new TestProcess();
139+
const onShutdown = vi.fn().mockResolvedValue(undefined);
140+
const child = createTrackedChild(555);
141+
await registerSimulatorLaunchOsLogSession({
142+
process: child,
143+
simulatorUuid: 'sim-1',
144+
bundleId: 'io.sentry.app',
145+
logFilePath: '/tmp/app.log',
146+
});
147+
const coordinator = createMcpLifecycleCoordinator({
148+
commandExecutor: createMockExecutor({ output: '' }),
149+
processRef,
150+
onShutdown,
151+
});
152+
153+
coordinator.attachProcessHandlers();
154+
processRef.emit('exit');
155+
156+
expect(child.kill).toHaveBeenCalledWith('SIGTERM');
157+
expect(onShutdown).not.toHaveBeenCalled();
158+
});
159+
133160
it('maps broken stdout pipes to shutdowns', async () => {
134161
const suppressSpy = vi
135162
.spyOn(shutdownState, 'suppressProcessStdioWrites')
@@ -179,7 +206,11 @@ describe('mcp lifecycle snapshot', () => {
179206
beforeEach(async () => {
180207
registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-mcp-lifecycle-'));
181208
setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir);
182-
setRuntimeInstanceForTests({ instanceId: 'mcp-lifecycle-test', pid: process.pid });
209+
setRuntimeInstanceForTests({
210+
instanceId: 'mcp-lifecycle-test',
211+
pid: process.pid,
212+
workspaceKey: 'workspace-a',
213+
});
183214
setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true);
184215
await clearAllSimulatorLaunchOsLogSessionsForTests();
185216
});

0 commit comments

Comments
 (0)