From d099331fce66b0bff4c268b26f32b797a2de71cb Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Fri, 1 May 2026 12:23:36 +0100 Subject: [PATCH 1/3] 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 --- CHANGELOG.md | 6 + .../orphaned-simctl-log-stream-2026-05-01.md | 100 +++++ src/cli.ts | 2 + src/daemon.ts | 40 +- .../__tests__/session-status.test.ts | 10 +- .../simulator/__tests__/stop_app_sim.test.ts | 6 +- src/server/__tests__/mcp-lifecycle.test.ts | 35 +- src/server/__tests__/mcp-shutdown.test.ts | 29 +- src/server/bootstrap.ts | 21 +- src/server/mcp-lifecycle.ts | 13 +- src/server/mcp-shutdown.ts | 6 - src/utils/__tests__/log_capture.test.ts | 363 ---------------- .../__tests__/log_capture_escape.test.ts | 195 --------- .../simulator-launch-oslog-registry.test.ts | 23 +- .../simulator-launch-oslog-sessions.test.ts | 217 +++++++++- .../__tests__/simulator-steps-pid.test.ts | 6 +- src/utils/log-capture/index.ts | 24 +- .../simulator-launch-oslog-registry.ts | 6 +- .../simulator-launch-oslog-sessions.ts | 160 ++++++- src/utils/log_capture.ts | 405 ------------------ src/utils/runtime-instance.ts | 46 +- src/utils/session-status.ts | 7 +- 22 files changed, 672 insertions(+), 1048 deletions(-) create mode 100644 docs/investigations/orphaned-simctl-log-stream-2026-05-01.md delete mode 100644 src/utils/__tests__/log_capture.test.ts delete mode 100644 src/utils/__tests__/log_capture_escape.test.ts delete mode 100644 src/utils/log_capture.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d50dae590..374fa26e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- 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)). + ## [2.5.0-beta.1] ### Breaking diff --git a/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md b/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md new file mode 100644 index 000000000..3a3c3c345 --- /dev/null +++ b/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md @@ -0,0 +1,100 @@ +# Investigation: Orphaned simctl OSLog processes when MCP server exits abnormally + +**Issue:** [#382](https://github.com/getsentry/XcodeBuildMCP/issues/382) +**Date:** 2026-05-01 +**Status:** Confirmed and fixed for simulator OSLog helpers. + +## Summary + +The issue described orphaned simulator helper processes after abnormal server exits. The final fix is deliberately narrow: + +- `simctl spawn … log stream …` OSLog helpers are registry-backed and now reconciled safely across MCP/daemon restarts. +- `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. +- The old `src/utils/log_capture.ts` path was dead production code and has been removed. + +The fix does not change public tool schemas. + +## Final design + +### OSLog helpers are workspace-scoped + +Durable OSLog registry records now store owner identity as: + +```ts +owner: { + instanceId: string; + pid: number; + workspaceKey: string; +} +``` + +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. + +### Reconciliation only reaps safe orphans + +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: + +1. The record belongs to the current workspace. +2. The owner PID is not the current process. +3. The owner PID is no longer alive. +4. The helper process still matches the expected `simctl spawn … log stream …` command. + +Records from other workspaces are skipped. Records owned by live processes are skipped, including live foreign MCP/daemon sessions in the same workspace. + +### App-scoped cleanup is no longer global + +`stop_app_sim` and relaunch cleanup still clean OSLog helpers for the target simulator/bundle, but only when the record is: + +- owned by the current runtime instance, or +- in the same workspace with a dead owner PID. + +This prevents one active session from killing another active session’s OSLog helper. + +### Last-chance cleanup covers local live helpers + +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. + +Daemon crash handling was also brought closer to MCP parity with `uncaughtException` and `unhandledRejection` handling that requests shutdown with a non-zero exit code. + +### Dead log-capture path removed + +`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`. + +Removed: + +- `src/utils/log_capture.ts` +- `src/utils/__tests__/log_capture.test.ts` +- `src/utils/__tests__/log_capture_escape.test.ts` +- legacy re-exports from `src/utils/log-capture/index.ts` +- legacy shutdown/status references to `stopAllLogCaptures`, `activeLogSessions`, and `simulatorLogSessionCount` + +## Why console-PTY is not registered + +`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. + +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. + +## Verification expectations + +Automated coverage should verify: + +- OSLog registry records require `owner.workspaceKey`. +- records without `owner.workspaceKey` are pruned. +- same-workspace dead-owner OSLog helpers are reconciled. +- other-workspace records are skipped. +- same-workspace live-owner records are skipped. +- app-scoped cleanup skips live foreign owners. +- lifecycle snapshots and session status do not expose deleted legacy log-capture fields. +- synchronous exit cleanup signals only live local OSLog helpers. + +Manual smoke testing should verify: + +1. Clean shutdown stops helpers owned by the current server. +2. After `SIGKILL` of an MCP server, restarting in the same workspace reaps the orphaned `simctl spawn … log stream …` helper. +3. Two live sessions in the same workspace do not kill each other’s OSLog helpers. +4. Startup from workspace A does not kill workspace B helpers. +5. `stop_app_sim` does not kill live foreign-owner OSLog helpers. + +## Known boundary + +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. diff --git a/src/cli.ts b/src/cli.ts index 0168b4759..2c9e66d3d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -8,6 +8,7 @@ import { listCliWorkflowIdsFromManifest } from './runtime/tool-catalog.ts'; import { flushAndCloseSentry, initSentry, recordBootstrapDurationMetric } from './utils/sentry.ts'; import { coerceLogLevel, setLogLevel, type LogLevel } from './utils/logger.ts'; import { hydrateSentryDisabledEnvFromProjectConfig } from './utils/sentry-config.ts'; +import { configureRuntimeWorkspaceKey } from './utils/runtime-instance.ts'; function findTopLevelCommand(argv: string[]): string | undefined { const flagsWithValue = new Set(['--socket', '--log-level', '--style']); @@ -133,6 +134,7 @@ async function main(): Promise { cwd: result.runtime.cwd, projectConfigPath: result.configPath, }); + configureRuntimeWorkspaceKey(workspaceKey); const cliExposedWorkflowIds = await listCliWorkflowIdsFromManifest({ excludeWorkflows: ['session-management', 'workflow-discovery'], diff --git a/src/daemon.ts b/src/daemon.ts index e20930ce7..3ef425d6e 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -42,6 +42,11 @@ import { } from './utils/sentry.ts'; import { isXcodemakeBinaryAvailable, isXcodemakeEnabled } from './utils/xcodemake/index.ts'; import { hydrateSentryDisabledEnvFromProjectConfig } from './utils/sentry-config.ts'; +import { configureRuntimeWorkspaceKey } from './utils/runtime-instance.ts'; +import { + reconcileSimulatorLaunchOsLogOrphansForWorkspace, + terminateLiveSimulatorLaunchOsLogSessionsSync, +} from './utils/log-capture/index.ts'; async function checkExistingDaemon(socketPath: string): Promise { return new Promise((resolve) => { @@ -128,6 +133,7 @@ async function main(): Promise { cwd: result.runtime.cwd, projectConfigPath: result.configPath, }); + configureRuntimeWorkspaceKey(workspaceKey); const logPath = resolveDaemonLogPath(workspaceKey); if (logPath) { @@ -153,6 +159,20 @@ async function main(): Promise { log('info', `[Daemon] Workspace: ${workspaceRoot}`); log('info', `[Daemon] Socket: ${socketPath}`); + try { + const reconciliation = await reconcileSimulatorLaunchOsLogOrphansForWorkspace(workspaceKey); + if (reconciliation.stoppedSessionCount > 0 || reconciliation.errorCount > 0) { + log( + reconciliation.errorCount > 0 ? 'warn' : 'info', + `[Daemon] Simulator OSLog reconciliation: ${JSON.stringify(reconciliation)}`, + ); + } + } catch (error) { + log( + 'warn', + `[Daemon] Simulator OSLog reconciliation failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } if (logPath) { log('info', `[Daemon] Logs: ${logPath}`); } @@ -268,7 +288,7 @@ async function main(): Promise { }; // Unified shutdown handler - const shutdown = (): void => { + const shutdown = (exitCode = 0): void => { if (isShuttingDown) { return; } @@ -292,7 +312,7 @@ async function main(): Promise { log('info', '[Daemon] Cleanup complete'); void flushAndCloseSentry(2000).finally(() => { - process.exit(0); + process.exit(exitCode); }); }); @@ -393,8 +413,20 @@ async function main(): Promise { }); }); - process.on('SIGTERM', shutdown); - process.on('SIGINT', shutdown); + const handleCrash = (reason: unknown): void => { + recordDaemonLifecycleMetric('crash'); + const message = reason instanceof Error ? reason.message : String(reason); + log('error', `[Daemon] Crash: ${message}`, { sentry: true }); + shutdown(1); + }; + + process.on('exit', () => { + terminateLiveSimulatorLaunchOsLogSessionsSync(); + }); + process.on('SIGTERM', () => shutdown(0)); + process.on('SIGINT', () => shutdown(0)); + process.on('uncaughtException', handleCrash); + process.on('unhandledRejection', handleCrash); } main().catch(async (err) => { diff --git a/src/mcp/resources/__tests__/session-status.test.ts b/src/mcp/resources/__tests__/session-status.test.ts index 848bfb88b..e63cd7e1c 100644 --- a/src/mcp/resources/__tests__/session-status.test.ts +++ b/src/mcp/resources/__tests__/session-status.test.ts @@ -7,7 +7,6 @@ import type { ChildProcess } from 'node:child_process'; import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { clearDaemonActivityRegistry } from '../../../daemon/activity-registry.ts'; import { getDefaultDebuggerManager } from '../../../utils/debugger/index.ts'; -import { activeLogSessions } from '../../../utils/log_capture.ts'; import { activeDeviceLogSessions } from '../../../utils/log-capture/device-log-sessions.ts'; import { clearAllSimulatorLaunchOsLogSessionsForTests, @@ -34,9 +33,12 @@ describe('session-status resource', () => { beforeEach(async () => { registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-session-status-')); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); - setRuntimeInstanceForTests({ instanceId: 'session-status-test', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'session-status-test', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true); - activeLogSessions.clear(); activeDeviceLogSessions.clear(); clearAllProcesses(); await clearAllSimulatorLaunchOsLogSessionsForTests(); @@ -45,7 +47,6 @@ describe('session-status resource', () => { }); afterEach(async () => { - activeLogSessions.clear(); activeDeviceLogSessions.clear(); clearAllProcesses(); await clearAllSimulatorLaunchOsLogSessionsForTests(); @@ -64,7 +65,6 @@ describe('session-status resource', () => { expect(result.contents).toHaveLength(1); const parsed = JSON.parse(result.contents[0].text); - expect(parsed.logging.simulator.activeSessionIds).toEqual([]); expect(parsed.logging.simulator.activeLaunchOsLogSessions).toEqual([]); expect(parsed.logging.device.activeSessionIds).toEqual([]); expect(parsed.debug.currentSessionId).toBe(null); diff --git a/src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts b/src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts index 370a4bed0..c89893617 100644 --- a/src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts +++ b/src/mcp/tools/simulator/__tests__/stop_app_sim.test.ts @@ -67,7 +67,11 @@ describe('stop_app_sim tool', () => { trackedChildren.clear(); registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-stop-app-sim-')); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); - setRuntimeInstanceForTests({ instanceId: 'stop-app-sim-test', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'stop-app-sim-test', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async (record) => { const child = trackedChildren.get(record.helperPid); return child ? child.exitCode == null : true; diff --git a/src/server/__tests__/mcp-lifecycle.test.ts b/src/server/__tests__/mcp-lifecycle.test.ts index 97654afdf..f98413000 100644 --- a/src/server/__tests__/mcp-lifecycle.test.ts +++ b/src/server/__tests__/mcp-lifecycle.test.ts @@ -61,7 +61,11 @@ describe('mcp lifecycle coordinator', () => { beforeEach(async () => { registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-mcp-lifecycle-')); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); - setRuntimeInstanceForTests({ instanceId: 'mcp-lifecycle-test', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'mcp-lifecycle-test', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true); await clearAllSimulatorLaunchOsLogSessionsForTests(); vi.restoreAllMocks(); @@ -130,6 +134,29 @@ describe('mcp lifecycle coordinator', () => { expect(onShutdown.mock.calls[0]?.[0]?.reason).toBe('unhandled-rejection'); }); + it('terminates live local OSLog sessions on process exit', async () => { + const processRef = new TestProcess(); + const onShutdown = vi.fn().mockResolvedValue(undefined); + const child = createTrackedChild(555); + await registerSimulatorLaunchOsLogSession({ + process: child, + simulatorUuid: 'sim-1', + bundleId: 'io.sentry.app', + logFilePath: '/tmp/app.log', + }); + const coordinator = createMcpLifecycleCoordinator({ + commandExecutor: createMockExecutor({ output: '' }), + processRef, + onShutdown, + }); + + coordinator.attachProcessHandlers(); + processRef.emit('exit'); + + expect(child.kill).toHaveBeenCalledWith('SIGTERM'); + expect(onShutdown).not.toHaveBeenCalled(); + }); + it('maps broken stdout pipes to shutdowns', async () => { const suppressSpy = vi .spyOn(shutdownState, 'suppressProcessStdioWrites') @@ -179,7 +206,11 @@ describe('mcp lifecycle snapshot', () => { beforeEach(async () => { registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-mcp-lifecycle-')); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); - setRuntimeInstanceForTests({ instanceId: 'mcp-lifecycle-test', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'mcp-lifecycle-test', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async () => true); await clearAllSimulatorLaunchOsLogSessionsForTests(); }); diff --git a/src/server/__tests__/mcp-shutdown.test.ts b/src/server/__tests__/mcp-shutdown.test.ts index 5236b604f..cbf623a5b 100644 --- a/src/server/__tests__/mcp-shutdown.test.ts +++ b/src/server/__tests__/mcp-shutdown.test.ts @@ -4,7 +4,6 @@ const mocks = vi.hoisted(() => ({ stopXcodeStateWatcher: vi.fn(async () => undefined), shutdownXcodeToolsBridge: vi.fn(async () => undefined), disposeAll: vi.fn(async () => undefined), - stopAllLogCaptures: vi.fn(async () => ({ stoppedSessionCount: 0, errorCount: 0, errors: [] })), stopAllDeviceLogCaptures: vi.fn(async () => ({ stoppedSessionCount: 0, errorCount: 0, @@ -39,9 +38,6 @@ vi.mock('../../integrations/xcode-tools-bridge/index.ts', () => ({ vi.mock('../../utils/debugger/index.ts', () => ({ getDefaultDebuggerManager: () => ({ disposeAll: mocks.disposeAll }), })); -vi.mock('../../utils/log_capture.ts', () => ({ - stopAllLogCaptures: mocks.stopAllLogCaptures, -})); vi.mock('../../utils/log-capture/device-log-sessions.ts', () => ({ stopAllDeviceLogCaptures: mocks.stopAllDeviceLogCaptures, })); @@ -90,7 +86,6 @@ describe('runMcpShutdown', () => { activeOperationCount: 0, activeOperationByCategory: {}, debuggerSessionCount: 0, - simulatorLogSessionCount: 0, simulatorLaunchOsLogSessionCount: 0, ownedSimulatorLaunchOsLogSessionCount: 0, deviceLogSessionCount: 0, @@ -110,7 +105,6 @@ describe('runMcpShutdown', () => { expect(mocks.stopXcodeStateWatcher).toHaveBeenCalledTimes(1); expect(mocks.shutdownXcodeToolsBridge).toHaveBeenCalledTimes(1); expect(mocks.disposeAll).toHaveBeenCalledTimes(1); - expect(mocks.stopAllLogCaptures).toHaveBeenCalledTimes(1); expect(mocks.stopAllDeviceLogCaptures).toHaveBeenCalledTimes(1); expect(mocks.stopOwnedSimulatorLaunchOsLogSessions).toHaveBeenCalledTimes(1); expect(mocks.stopAllVideoCaptureSessions).toHaveBeenCalledTimes(1); @@ -118,7 +112,7 @@ describe('runMcpShutdown', () => { }); it('adds outer timeout headroom for one-item bulk cleanup', async () => { - mocks.stopAllLogCaptures.mockImplementationOnce(async () => { + mocks.stopOwnedSimulatorLaunchOsLogSessions.mockImplementationOnce(async () => { await wait(1050); return { stoppedSessionCount: 1, errorCount: 0, errors: [] }; }); @@ -139,9 +133,8 @@ describe('runMcpShutdown', () => { activeOperationCount: 0, activeOperationByCategory: {}, debuggerSessionCount: 0, - simulatorLogSessionCount: 1, - simulatorLaunchOsLogSessionCount: 0, - ownedSimulatorLaunchOsLogSessionCount: 0, + simulatorLaunchOsLogSessionCount: 1, + ownedSimulatorLaunchOsLogSessionCount: 1, deviceLogSessionCount: 0, videoCaptureSessionCount: 0, swiftPackageProcessCount: 0, @@ -152,12 +145,14 @@ describe('runMcpShutdown', () => { server: { close: async () => undefined }, }); - const simulatorLogsStep = result.steps.find((step) => step.name === 'simulator-logs.stop-all'); + const simulatorLogsStep = result.steps.find( + (step) => step.name === 'simulator-launch-oslogs.stop-owned', + ); expect(simulatorLogsStep?.status).toBe('completed'); }); it('uses an expanded timeout budget for sequential multi-item bulk cleanup steps', async () => { - mocks.stopAllLogCaptures.mockImplementationOnce(async () => { + mocks.stopOwnedSimulatorLaunchOsLogSessions.mockImplementationOnce(async () => { await wait(1100); return { stoppedSessionCount: 2, errorCount: 0, errors: [] }; }); @@ -178,9 +173,8 @@ describe('runMcpShutdown', () => { activeOperationCount: 0, activeOperationByCategory: {}, debuggerSessionCount: 0, - simulatorLogSessionCount: 2, - simulatorLaunchOsLogSessionCount: 0, - ownedSimulatorLaunchOsLogSessionCount: 0, + simulatorLaunchOsLogSessionCount: 2, + ownedSimulatorLaunchOsLogSessionCount: 2, deviceLogSessionCount: 0, videoCaptureSessionCount: 0, swiftPackageProcessCount: 0, @@ -191,7 +185,9 @@ describe('runMcpShutdown', () => { server: { close: async () => undefined }, }); - const simulatorLogsStep = result.steps.find((step) => step.name === 'simulator-logs.stop-all'); + const simulatorLogsStep = result.steps.find( + (step) => step.name === 'simulator-launch-oslogs.stop-owned', + ); expect(simulatorLogsStep?.status).toBe('completed'); }); @@ -216,7 +212,6 @@ describe('runMcpShutdown', () => { activeOperationCount: 0, activeOperationByCategory: {}, debuggerSessionCount: 1, - simulatorLogSessionCount: 0, simulatorLaunchOsLogSessionCount: 0, ownedSimulatorLaunchOsLogSessionCount: 0, deviceLogSessionCount: 0, diff --git a/src/server/bootstrap.ts b/src/server/bootstrap.ts index abe0e2b99..0f4081cf0 100644 --- a/src/server/bootstrap.ts +++ b/src/server/bootstrap.ts @@ -7,7 +7,7 @@ import type { RuntimeConfigOverrides } from '../utils/config-store.ts'; import { getRegisteredWorkflows, registerWorkflowsFromManifest } from '../utils/tool-registry.ts'; import { bootstrapRuntime } from '../runtime/bootstrap-runtime.ts'; import { getXcodeToolsBridgeManager } from '../integrations/xcode-tools-bridge/index.ts'; -import { resolveWorkspaceRoot } from '../daemon/socket-path.ts'; +import { resolveWorkspaceRoot, workspaceKeyForRoot } from '../daemon/socket-path.ts'; import { detectXcodeRuntime } from '../utils/xcode-process.ts'; import { readXcodeIdeState } from '../utils/xcode-state-reader.ts'; import { sessionStore } from '../utils/session-store.ts'; @@ -15,6 +15,8 @@ import { startXcodeStateWatcher, lookupBundleId } from '../utils/xcode-state-wat import { getDefaultCommandExecutor } from '../utils/command.ts'; import type { PredicateContext } from '../visibility/predicate-types.ts'; import { createStartupProfiler, getStartupProfileNowMs } from './startup-profiler.ts'; +import { configureRuntimeWorkspaceKey } from '../utils/runtime-instance.ts'; +import { reconcileSimulatorLaunchOsLogOrphansForWorkspace } from '../utils/log-capture/index.ts'; export interface BootstrapOptions { enabledWorkflows?: string[]; @@ -76,6 +78,23 @@ export async function bootstrapServer( cwd: result.runtime.cwd, projectConfigPath: result.configPath, }); + const workspaceKey = workspaceKeyForRoot(workspaceRoot); + configureRuntimeWorkspaceKey(workspaceKey); + + try { + const reconciliation = await reconcileSimulatorLaunchOsLogOrphansForWorkspace(workspaceKey); + if (reconciliation.stoppedSessionCount > 0 || reconciliation.errorCount > 0) { + log( + reconciliation.errorCount > 0 ? 'warn' : 'info', + `[startup] Simulator OSLog reconciliation: ${JSON.stringify(reconciliation)}`, + ); + } + } catch (error) { + log( + 'warn', + `[startup] Simulator OSLog reconciliation failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } log('info', `🚀 Initializing server...`); diff --git a/src/server/mcp-lifecycle.ts b/src/server/mcp-lifecycle.ts index d6af79d4f..5f716d220 100644 --- a/src/server/mcp-lifecycle.ts +++ b/src/server/mcp-lifecycle.ts @@ -1,9 +1,11 @@ import process from 'node:process'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { getDefaultDebuggerManager } from '../utils/debugger/index.ts'; -import { activeLogSessions } from '../utils/log_capture.ts'; import { activeDeviceLogSessions } from '../utils/log-capture/device-log-sessions.ts'; -import { listActiveSimulatorLaunchOsLogSessions } from '../utils/log-capture/simulator-launch-oslog-sessions.ts'; +import { + listActiveSimulatorLaunchOsLogSessions, + terminateLiveSimulatorLaunchOsLogSessionsSync, +} from '../utils/log-capture/simulator-launch-oslog-sessions.ts'; import { activeProcesses } from '../mcp/tools/swift-package/active-processes.ts'; import { getDaemonActivitySnapshot } from '../daemon/activity-registry.ts'; import { listActiveVideoCaptureSessionIds } from '../utils/video_capture.ts'; @@ -61,7 +63,6 @@ export interface McpLifecycleSnapshot { activeOperationCount: number; activeOperationByCategory: Record; debuggerSessionCount: number; - simulatorLogSessionCount: number; simulatorLaunchOsLogSessionCount: number; ownedSimulatorLaunchOsLogSessionCount: number; deviceLogSessionCount: number; @@ -301,7 +302,6 @@ export async function buildMcpLifecycleSnapshot(options: { activeOperationCount: activitySnapshot.activeOperationCount, activeOperationByCategory: activitySnapshot.byCategory, debuggerSessionCount: getDefaultDebuggerManager().listSessions().length, - simulatorLogSessionCount: activeLogSessions.size, simulatorLaunchOsLogSessionCount: simulatorLaunchOsLogSessions.length, ownedSimulatorLaunchOsLogSessionCount: simulatorLaunchOsLogSessions.filter( (session) => session.ownedByCurrentProcess, @@ -366,6 +366,9 @@ export function createMcpLifecycleCoordinator( const handleUnhandledRejection = (reason: unknown): void => { void coordinator.shutdown('unhandled-rejection', reason); }; + const handleExit = (): void => { + terminateLiveSimulatorLaunchOsLogSessionsSync(); + }; let handlersAttached = false; @@ -384,6 +387,7 @@ export function createMcpLifecycleCoordinator( processRef.stderr?.once('error', handleStderrError); processRef.once('uncaughtException', handleUncaughtException); processRef.once('unhandledRejection', handleUnhandledRejection); + processRef.once('exit', handleExit); }, detachProcessHandlers(): void { @@ -400,6 +404,7 @@ export function createMcpLifecycleCoordinator( processRef.stderr?.removeListener('error', handleStderrError); processRef.removeListener('uncaughtException', handleUncaughtException); processRef.removeListener('unhandledRejection', handleUnhandledRejection); + processRef.removeListener('exit', handleExit); }, markPhase(phase: McpStartupPhase): void { diff --git a/src/server/mcp-shutdown.ts b/src/server/mcp-shutdown.ts index de876afb2..1427b3564 100644 --- a/src/server/mcp-shutdown.ts +++ b/src/server/mcp-shutdown.ts @@ -2,7 +2,6 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { getDefaultDebuggerManager } from '../utils/debugger/index.ts'; import { stopXcodeStateWatcher } from '../utils/xcode-state-watcher.ts'; import { shutdownXcodeToolsBridge } from '../integrations/xcode-tools-bridge/index.ts'; -import { stopAllLogCaptures } from '../utils/log_capture.ts'; import { stopAllDeviceLogCaptures } from '../utils/log-capture/device-log-sessions.ts'; import { stopOwnedSimulatorLaunchOsLogSessions } from '../utils/log-capture/simulator-launch-oslog-sessions.ts'; import { stopAllVideoCaptureSessions } from '../utils/video_capture.ts'; @@ -192,11 +191,6 @@ export async function runMcpShutdown(input: { timeoutMs: debuggerStepTimeoutMs(input.snapshot.debuggerSessionCount), operation: () => getDefaultDebuggerManager().disposeAll(), }, - { - name: 'simulator-logs.stop-all', - timeoutMs: bulkStepTimeoutMs(input.snapshot.simulatorLogSessionCount), - operation: () => stopAllLogCaptures(STEP_TIMEOUT_MS), - }, { name: 'simulator-launch-oslogs.stop-owned', timeoutMs: bulkStepTimeoutMs(input.snapshot.ownedSimulatorLaunchOsLogSessionCount), diff --git a/src/utils/__tests__/log_capture.test.ts b/src/utils/__tests__/log_capture.test.ts deleted file mode 100644 index 32554997d..000000000 --- a/src/utils/__tests__/log_capture.test.ts +++ /dev/null @@ -1,363 +0,0 @@ -import { ChildProcess } from 'child_process'; -import { Writable } from 'stream'; -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { - activeLogSessions, - startLogCapture, - stopLogCapture, - type SubsystemFilter, -} from '../log_capture.ts'; -import type { CommandExecutor } from '../CommandExecutor.ts'; -import type { FileSystemExecutor } from '../FileSystemExecutor.ts'; - -type CallHistoryEntry = { - command: string[]; - logPrefix?: string; - useShell?: boolean; - opts?: { env?: Record; cwd?: string }; - detached?: boolean; -}; - -function createMockExecutorWithCalls(callHistory: CallHistoryEntry[]): CommandExecutor { - const mockProcess: Partial = {}; - Object.assign(mockProcess, { - pid: 12345, - stdout: null, - stderr: null, - killed: false, - exitCode: null, - on: () => mockProcess, - }); - - return async (command, logPrefix, useShell, opts, detached) => { - callHistory.push({ command, logPrefix, useShell, opts, detached }); - return { success: true, output: '', process: mockProcess as ChildProcess }; - }; -} - -function expectPredicate( - call: CallHistoryEntry, - bundleId: string, - subsystemFilter: SubsystemFilter, -): void { - const predicateIndex = call.command.indexOf('--predicate'); - expect(predicateIndex).toBeGreaterThan(-1); - const predicate = call.command[predicateIndex + 1]; - - switch (subsystemFilter) { - case 'app': - expect(predicate).toBe(`subsystem == "${bundleId}"`); - return; - case 'swiftui': - expect(predicate).toBe(`subsystem == "${bundleId}" OR subsystem == "com.apple.SwiftUI"`); - return; - default: { - const subsystems = [bundleId, ...subsystemFilter]; - const expected = subsystems.map((s) => `subsystem == "${s}"`).join(' OR '); - expect(predicate).toBe(expected); - } - } -} - -type InMemoryFileRecord = { content: string; mtimeMs: number }; - -function createInMemoryFileSystemExecutor(): FileSystemExecutor { - const files = new Map(); - const tempDir = '/virtual/tmp'; - - return { - mkdir: async () => {}, - readFile: async (path) => { - const record = files.get(path); - if (!record) { - throw new Error(`Missing file: ${path}`); - } - return record.content; - }, - writeFile: async (path, content) => { - files.set(path, { content, mtimeMs: Date.now() }); - }, - createWriteStream: (path) => { - const chunks: Buffer[] = []; - - const stream = new Writable({ - write(chunk, _encoding, callback) { - chunks.push(Buffer.from(chunk)); - callback(); - }, - final(callback) { - const existing = files.get(path)?.content ?? ''; - files.set(path, { - content: existing + Buffer.concat(chunks).toString('utf8'), - mtimeMs: Date.now(), - }); - callback(); - }, - }); - - return stream as unknown as ReturnType; - }, - cp: async () => {}, - readdir: async (dir) => { - const prefix = `${dir}/`; - return Array.from(files.keys()) - .filter((filePath) => filePath.startsWith(prefix)) - .map((filePath) => filePath.slice(prefix.length)); - }, - stat: async (path) => { - const record = files.get(path); - if (!record) { - throw new Error(`Missing file: ${path}`); - } - return { isDirectory: (): boolean => false, mtimeMs: record.mtimeMs }; - }, - rm: async (path) => { - files.delete(path); - }, - existsSync: (path) => files.has(path), - mkdtemp: async (prefix) => `${tempDir}/${prefix}mock-temp`, - tmpdir: () => tempDir, - }; -} - -beforeEach(() => { - activeLogSessions.clear(); -}); - -afterEach(() => { - activeLogSessions.clear(); -}); - -describe('startLogCapture', () => { - it('creates log stream command with app subsystem by default', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { simulatorUuid: 'sim-uuid', bundleId: 'io.sentry.app' }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(1); - expect(callHistory[0].command).toEqual([ - 'xcrun', - 'simctl', - 'spawn', - 'sim-uuid', - 'log', - 'stream', - '--level=debug', - '--predicate', - 'subsystem == "io.sentry.app"', - ]); - expect(callHistory[0].logPrefix).toBe('OS Log Capture'); - expect(callHistory[0].useShell).toBe(false); - expect(callHistory[0].detached).toBe(true); - }); - - it('creates log stream command without predicate when filter is all', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - subsystemFilter: 'all', - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(1); - expect(callHistory[0].command).toEqual([ - 'xcrun', - 'simctl', - 'spawn', - 'sim-uuid', - 'log', - 'stream', - '--level=debug', - ]); - }); - - it('creates log stream command with SwiftUI predicate', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - subsystemFilter: 'swiftui', - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(1); - expectPredicate(callHistory[0], 'io.sentry.app', 'swiftui'); - }); - - it('creates log stream command with custom subsystem predicate', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - subsystemFilter: ['com.apple.UIKit', 'com.apple.CoreData'], - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(1); - expectPredicate(callHistory[0], 'io.sentry.app', ['com.apple.UIKit', 'com.apple.CoreData']); - }); - - it('creates console capture and log stream commands when captureConsole is true', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - captureConsole: true, - args: ['--flag', 'value'], - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(2); - expect(callHistory[0].command).toEqual([ - 'xcrun', - 'simctl', - 'launch', - '--console-pty', - '--terminate-running-process', - 'sim-uuid', - 'io.sentry.app', - '--flag', - 'value', - ]); - expect(callHistory[0].logPrefix).toBe('Console Log Capture'); - expect(callHistory[0].useShell).toBe(false); - expect(callHistory[0].detached).toBe(true); - - expect(callHistory[1].logPrefix).toBe('OS Log Capture'); - expect(callHistory[1].useShell).toBe(false); - expect(callHistory[1].detached).toBe(true); - }); - - it('passes SIMCTL_CHILD_-prefixed env to console launch executor when env is provided', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - captureConsole: true, - env: { STAGING_ENABLED: '1', DEBUG: 'true' }, - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory).toHaveLength(2); - // Console launch call should have prefixed env - expect(callHistory[0].opts).toEqual({ - env: { - SIMCTL_CHILD_STAGING_ENABLED: '1', - SIMCTL_CHILD_DEBUG: 'true', - }, - }); - // OS log stream call should not have env - expect(callHistory[1].opts).toBeUndefined(); - }); - - it('does not pass env opts to executor when env is not provided', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - const result = await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - captureConsole: true, - }, - executor, - fileSystem, - ); - - expect(result.error).toBeUndefined(); - expect(callHistory[0].opts).toBeUndefined(); - }); -}); - -describe('stopLogCapture', () => { - it('returns error when session is missing', async () => { - const fileSystem = createInMemoryFileSystemExecutor(); - const result = await stopLogCapture('missing-session', fileSystem); - - expect(result.logContent).toBe(''); - expect(result.error).toBe('Log capture session not found: missing-session'); - }); - - it('kills active processes and returns log content', async () => { - const fileSystem = createInMemoryFileSystemExecutor(); - const logFilePath = `${fileSystem.tmpdir()}/session.log`; - await fileSystem.writeFile(logFilePath, 'test log content'); - const logStream = fileSystem.createWriteStream(logFilePath, { flags: 'a' }); - - let killCount = 0; - const runningProcess = { - killed: false, - exitCode: null, - kill: () => { - killCount += 1; - }, - } as unknown as ChildProcess; - - const finishedProcess = { - killed: false, - exitCode: 0, - kill: () => { - killCount += 1; - }, - } as unknown as ChildProcess; - - activeLogSessions.set('session-1', { - processes: [runningProcess, finishedProcess], - logFilePath, - simulatorUuid: 'sim-uuid', - bundleId: 'io.sentry.app', - logStream, - }); - - const result = await stopLogCapture('session-1', fileSystem); - - expect(result.error).toBeUndefined(); - expect(result.logContent).toBe('test log content'); - expect(killCount).toBe(1); - expect(activeLogSessions.has('session-1')).toBe(false); - }); -}); diff --git a/src/utils/__tests__/log_capture_escape.test.ts b/src/utils/__tests__/log_capture_escape.test.ts deleted file mode 100644 index 13cf3ebb9..000000000 --- a/src/utils/__tests__/log_capture_escape.test.ts +++ /dev/null @@ -1,195 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import type { ChildProcess } from 'node:child_process'; -import type { WriteStream } from 'node:fs'; -import { activeLogSessions, startLogCapture } from '../log_capture.ts'; -import type { CommandExecutor } from '../CommandExecutor.ts'; -import type { FileSystemExecutor } from '../FileSystemExecutor.ts'; -import { Writable } from 'stream'; - -type CallHistoryEntry = { - command: string[]; - logPrefix?: string; - useShell?: boolean; - opts?: { env?: Record; cwd?: string }; - detached?: boolean; -}; - -function createMockExecutorWithCalls(callHistory: CallHistoryEntry[]): CommandExecutor { - const mockProcess = { - pid: 12345, - stdout: null, - stderr: null, - killed: false, - exitCode: null, - on: () => mockProcess, - } as unknown as ChildProcess; - - return async (command, logPrefix, useShell, opts, detached) => { - callHistory.push({ command, logPrefix, useShell, opts, detached }); - return { success: true, output: '', process: mockProcess }; - }; -} - -type InMemoryFileRecord = { content: string; mtimeMs: number }; - -function createInMemoryFileSystemExecutor(): FileSystemExecutor { - const files = new Map(); - const tempDir = '/virtual/tmp'; - - return { - mkdir: async () => {}, - readFile: async (path) => { - const record = files.get(path); - if (!record) throw new Error(`Missing file: ${path}`); - return record.content; - }, - writeFile: async (path, content) => { - files.set(path, { content, mtimeMs: Date.now() }); - }, - createWriteStream: (path) => { - const chunks: Buffer[] = []; - const stream = new Writable({ - write(chunk, _encoding, callback) { - chunks.push(Buffer.from(chunk)); - callback(); - }, - final(callback) { - const existing = files.get(path)?.content ?? ''; - files.set(path, { - content: existing + Buffer.concat(chunks).toString('utf8'), - mtimeMs: Date.now(), - }); - callback(); - }, - }); - return stream as unknown as WriteStream; - }, - cp: async () => {}, - readdir: async (dir) => { - const prefix = `${dir}/`; - return Array.from(files.keys()) - .filter((fp) => fp.startsWith(prefix)) - .map((fp) => fp.slice(prefix.length)); - }, - stat: async (path) => { - const record = files.get(path); - if (!record) throw new Error(`Missing file: ${path}`); - return { isDirectory: () => false, mtimeMs: record.mtimeMs }; - }, - rm: async (path) => { - files.delete(path); - }, - existsSync: (path) => files.has(path), - mkdtemp: async (prefix) => `${tempDir}/${prefix}mock-temp`, - tmpdir: () => tempDir, - }; -} - -beforeEach(() => { - activeLogSessions.clear(); -}); -afterEach(() => { - activeLogSessions.clear(); -}); - -describe('NSPredicate injection protection (escapePredicateString)', () => { - it('escapes double quotes in bundleId so they cannot break NSPredicate', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - // Malicious bundleId containing a double-quote to break out of predicate - const maliciousBundleId = 'io.evil" OR 1==1 OR subsystem == "x'; - - await startLogCapture( - { simulatorUuid: 'sim-uuid', bundleId: maliciousBundleId, subsystemFilter: 'app' }, - executor, - fileSystem, - ); - - expect(callHistory).toHaveLength(1); - const predicateIndex = callHistory[0].command.indexOf('--predicate'); - expect(predicateIndex).toBeGreaterThan(-1); - const predicate = callHistory[0].command[predicateIndex + 1]; - - // The quotes should be escaped so the predicate is: - // subsystem == "io.evil\" OR 1==1 OR subsystem == \"x" - // NOT broken out to: subsystem == "io.evil" OR 1==1 OR ... - expect(predicate).toBe('subsystem == "io.evil\\" OR 1==1 OR subsystem == \\"x"'); - // Verify the predicate does NOT contain a non-escaped split - expect(predicate.startsWith('subsystem == "')).toBe(true); - expect(predicate.endsWith('"')).toBe(true); - }); - - it('escapes backslashes in bundleId', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - await startLogCapture( - { simulatorUuid: 'sim-uuid', bundleId: 'io.test\\evil', subsystemFilter: 'app' }, - executor, - fileSystem, - ); - - const predicateIndex = callHistory[0].command.indexOf('--predicate'); - const predicate = callHistory[0].command[predicateIndex + 1]; - expect(predicate).toBe('subsystem == "io.test\\\\evil"'); - }); - - it('escapes double quotes in custom subsystem filter array', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - await startLogCapture( - { - simulatorUuid: 'sim-uuid', - bundleId: 'io.safe.app', - subsystemFilter: ['com.evil" OR 1==1 OR subsystem == "x'], - }, - executor, - fileSystem, - ); - - const predicateIndex = callHistory[0].command.indexOf('--predicate'); - const predicate = callHistory[0].command[predicateIndex + 1]; - - // Both the safe bundleId and the malicious subsystem should be present - // The malicious one must have escaped quotes - expect(predicate).toContain('subsystem == "io.safe.app"'); - expect(predicate).toContain('subsystem == "com.evil\\" OR 1==1 OR subsystem == \\"x"'); - }); - - it('escapes quotes in "swiftui" mode', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - await startLogCapture( - { simulatorUuid: 'sim-uuid', bundleId: 'io.evil"app', subsystemFilter: 'swiftui' }, - executor, - fileSystem, - ); - - const predicateIndex = callHistory[0].command.indexOf('--predicate'); - const predicate = callHistory[0].command[predicateIndex + 1]; - expect(predicate).toBe('subsystem == "io.evil\\"app" OR subsystem == "com.apple.SwiftUI"'); - }); - - it('normal bundleId without special chars passes through unchanged', async () => { - const callHistory: CallHistoryEntry[] = []; - const executor = createMockExecutorWithCalls(callHistory); - const fileSystem = createInMemoryFileSystemExecutor(); - - await startLogCapture( - { simulatorUuid: 'sim-uuid', bundleId: 'io.sentry.app', subsystemFilter: 'app' }, - executor, - fileSystem, - ); - - const predicateIndex = callHistory[0].command.indexOf('--predicate'); - const predicate = callHistory[0].command[predicateIndex + 1]; - expect(predicate).toBe('subsystem == "io.sentry.app"'); - }); -}); diff --git a/src/utils/__tests__/simulator-launch-oslog-registry.test.ts b/src/utils/__tests__/simulator-launch-oslog-registry.test.ts index 3870e6be4..4450a160f 100644 --- a/src/utils/__tests__/simulator-launch-oslog-registry.test.ts +++ b/src/utils/__tests__/simulator-launch-oslog-registry.test.ts @@ -16,9 +16,8 @@ function createRecord( overrides?: Partial[0]>, ) { return { - version: 1 as const, sessionId: 'session-1', - owner: { instanceId: 'instance-1', pid: 1234 }, + owner: { instanceId: 'instance-1', pid: 1234, workspaceKey: 'workspace-a' }, simulatorUuid: 'sim-1', bundleId: 'io.sentry.app', helperPid: process.pid, @@ -62,6 +61,26 @@ describe.sequential('simulator launch OSLog registry', () => { await expect(listSimulatorLaunchOsLogRegistryRecords()).resolves.toEqual([]); }); + it('prunes records without owner workspace keys', async () => { + writeFileSync( + path.join(registryDir, 'missing-workspace.json'), + `${JSON.stringify({ + ...createRecord(), + owner: { instanceId: 'instance-1', pid: 1234 }, + })}\n`, + ); + + await expect(listSimulatorLaunchOsLogRegistryRecords()).resolves.toEqual([]); + }); + + it('does not require registry version fields', async () => { + await writeSimulatorLaunchOsLogRegistryRecord(createRecord()); + + await expect(listSimulatorLaunchOsLogRegistryRecords()).resolves.toEqual([ + expect.objectContaining({ sessionId: 'session-1' }), + ]); + }); + it('prunes stale records whose process is gone', async () => { await writeSimulatorLaunchOsLogRegistryRecord( createRecord({ sessionId: 'stale', helperPid: 999999, expectedCommandParts: ['simctl'] }), diff --git a/src/utils/__tests__/simulator-launch-oslog-sessions.test.ts b/src/utils/__tests__/simulator-launch-oslog-sessions.test.ts index d7c36eac3..c1a991d84 100644 --- a/src/utils/__tests__/simulator-launch-oslog-sessions.test.ts +++ b/src/utils/__tests__/simulator-launch-oslog-sessions.test.ts @@ -9,11 +9,14 @@ import { setRuntimeInstanceForTests } from '../runtime-instance.ts'; import { clearAllSimulatorLaunchOsLogSessionsForTests, listActiveSimulatorLaunchOsLogSessions, + reconcileSimulatorLaunchOsLogOrphansForWorkspace, registerSimulatorLaunchOsLogSession, + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests, setSimulatorLaunchOsLogRegistryDirOverrideForTests, stopAllSimulatorLaunchOsLogSessions, stopOwnedSimulatorLaunchOsLogSessions, stopSimulatorLaunchOsLogSessionsForApp, + terminateLiveSimulatorLaunchOsLogSessionsSync, } from '../log-capture/simulator-launch-oslog-sessions.ts'; import { setSimulatorLaunchOsLogRecordActiveOverrideForTests } from '../log-capture/simulator-launch-oslog-registry.ts'; @@ -65,7 +68,11 @@ describe('simulator launch OSLog sessions', () => { trackedChildren.clear(); registryDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-oslog-sessions-')); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); - setRuntimeInstanceForTests({ instanceId: 'current-instance', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async (record) => { const child = trackedChildren.get(record.helperPid); return child ? child.exitCode == null : true; @@ -75,6 +82,7 @@ describe('simulator launch OSLog sessions', () => { afterEach(async () => { await clearAllSimulatorLaunchOsLogSessionsForTests(); setSimulatorLaunchOsLogRecordActiveOverrideForTests(null); + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests(null); setRuntimeInstanceForTests(null); setSimulatorLaunchOsLogRegistryDirOverrideForTests(null); trackedChildren.clear(); @@ -101,6 +109,52 @@ describe('simulator launch OSLog sessions', () => { ]); }); + it('lists only sessions from the current workspace', async () => { + await registerSimulatorLaunchOsLogSession({ + process: createMockChild({ pid: 151 }), + simulatorUuid: 'sim-1', + bundleId: 'app.current', + logFilePath: '/tmp/current.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'other-instance', + pid: 4242, + workspaceKey: 'workspace-b', + }); + await registerSimulatorLaunchOsLogSession({ + process: createMockChild({ pid: 152 }), + simulatorUuid: 'sim-1', + bundleId: 'app.other', + logFilePath: '/tmp/other.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); + + await expect(listActiveSimulatorLaunchOsLogSessions()).resolves.toEqual([ + expect.objectContaining({ bundleId: 'app.current', pid: 151 }), + ]); + }); + + it('returns empty status and no-ops owner cleanup before runtime workspace configuration', async () => { + await registerSimulatorLaunchOsLogSession({ + process: createMockChild({ pid: 161 }), + simulatorUuid: 'sim-1', + bundleId: 'app.current', + logFilePath: '/tmp/current.log', + }); + setRuntimeInstanceForTests(null); + + await expect(listActiveSimulatorLaunchOsLogSessions()).resolves.toEqual([]); + await expect(stopOwnedSimulatorLaunchOsLogSessions()).resolves.toEqual({ + stoppedSessionCount: 0, + errorCount: 0, + errors: [], + }); + }); + it('removes sessions when the child exits', async () => { const child = createMockChild({ pid: 202 }); await registerSimulatorLaunchOsLogSession({ @@ -154,14 +208,22 @@ describe('simulator launch OSLog sessions', () => { logFilePath: '/tmp/current.log', }); - setRuntimeInstanceForTests({ instanceId: 'foreign-instance', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'foreign-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); await registerSimulatorLaunchOsLogSession({ process: createMockChild({ pid: 402 }), simulatorUuid: 'sim-2', bundleId: 'app.foreign', logFilePath: '/tmp/foreign.log', }); - setRuntimeInstanceForTests({ instanceId: 'current-instance', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); const result = await stopOwnedSimulatorLaunchOsLogSessions(); @@ -267,4 +329,153 @@ describe('simulator launch OSLog sessions', () => { expect.objectContaining({ bundleId: 'app.a', pid: 701 }), ]); }); + + it('does not stop same-app sessions owned by a live foreign instance', async () => { + const foreignChild = createMockChild({ pid: 801 }); + setRuntimeInstanceForTests({ + instanceId: 'foreign-instance', + pid: 4242, + workspaceKey: 'workspace-a', + }); + await registerSimulatorLaunchOsLogSession({ + process: foreignChild, + simulatorUuid: 'sim-1', + bundleId: 'app.a', + logFilePath: '/tmp/foreign.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests((pid) => pid === 4242); + + const result = await stopSimulatorLaunchOsLogSessionsForApp('sim-1', 'app.a'); + + expect(result).toEqual({ stoppedSessionCount: 0, errorCount: 0, errors: [] }); + expect(foreignChild.kill).not.toHaveBeenCalled(); + await expect(listActiveSimulatorLaunchOsLogSessions()).resolves.toEqual([ + expect.objectContaining({ bundleId: 'app.a', pid: 801, ownedByCurrentProcess: false }), + ]); + }); + + it('reconciles same-workspace sessions owned by dead processes', async () => { + const orphanChild = createMockChild({ pid: 901 }); + setRuntimeInstanceForTests({ + instanceId: 'dead-instance', + pid: 4242, + workspaceKey: 'workspace-a', + }); + await registerSimulatorLaunchOsLogSession({ + process: orphanChild, + simulatorUuid: 'sim-1', + bundleId: 'app.a', + logFilePath: '/tmp/orphan.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests(() => false); + + const result = await reconcileSimulatorLaunchOsLogOrphansForWorkspace('workspace-a', 1); + + expect(result).toEqual({ + scannedSessionCount: 1, + eligibleOrphanCount: 1, + stoppedSessionCount: 1, + skippedLiveOwnerCount: 0, + skippedDifferentWorkspaceCount: 0, + errorCount: 0, + errors: [], + }); + expect(orphanChild.kill).toHaveBeenCalledWith('SIGTERM'); + await expect(listActiveSimulatorLaunchOsLogSessions()).resolves.toEqual([]); + }); + + it('does not reconcile sessions from a different workspace', async () => { + const otherWorkspaceChild = createMockChild({ pid: 1001 }); + setRuntimeInstanceForTests({ + instanceId: 'dead-instance', + pid: 4242, + workspaceKey: 'workspace-b', + }); + await registerSimulatorLaunchOsLogSession({ + process: otherWorkspaceChild, + simulatorUuid: 'sim-1', + bundleId: 'app.a', + logFilePath: '/tmp/other.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests(() => false); + + const result = await reconcileSimulatorLaunchOsLogOrphansForWorkspace('workspace-a', 1); + + expect(result).toEqual({ + scannedSessionCount: 1, + eligibleOrphanCount: 0, + stoppedSessionCount: 0, + skippedLiveOwnerCount: 0, + skippedDifferentWorkspaceCount: 1, + errorCount: 0, + errors: [], + }); + expect(otherWorkspaceChild.kill).not.toHaveBeenCalled(); + }); + + it('does not reconcile sessions whose owner pid is still alive', async () => { + const liveOwnerChild = createMockChild({ pid: 1101 }); + setRuntimeInstanceForTests({ + instanceId: 'live-instance', + pid: 4242, + workspaceKey: 'workspace-a', + }); + await registerSimulatorLaunchOsLogSession({ + process: liveOwnerChild, + simulatorUuid: 'sim-1', + bundleId: 'app.a', + logFilePath: '/tmp/live.log', + }); + setRuntimeInstanceForTests({ + instanceId: 'current-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); + setSimulatorLaunchOsLogOwnerPidAliveOverrideForTests((pid) => pid === 4242); + + const result = await reconcileSimulatorLaunchOsLogOrphansForWorkspace('workspace-a', 1); + + expect(result.stoppedSessionCount).toBe(0); + expect(result.skippedLiveOwnerCount).toBe(1); + expect(liveOwnerChild.kill).not.toHaveBeenCalled(); + }); + + it('terminates only live local sessions synchronously', async () => { + const liveChild = createMockChild({ pid: 1201 }); + const exitedChild = createMockChild({ pid: 1202, exitCode: 0 }); + + await registerSimulatorLaunchOsLogSession({ + process: liveChild, + simulatorUuid: 'sim-1', + bundleId: 'app.live', + logFilePath: '/tmp/live.log', + }); + await registerSimulatorLaunchOsLogSession({ + process: exitedChild, + simulatorUuid: 'sim-1', + bundleId: 'app.exited', + logFilePath: '/tmp/exited.log', + }); + + const result = terminateLiveSimulatorLaunchOsLogSessionsSync(); + + expect(result).toEqual({ attemptedCount: 1, errorCount: 0, errors: [] }); + expect(liveChild.kill).toHaveBeenCalledWith('SIGTERM'); + expect(exitedChild.kill).not.toHaveBeenCalled(); + }); }); diff --git a/src/utils/__tests__/simulator-steps-pid.test.ts b/src/utils/__tests__/simulator-steps-pid.test.ts index 476072829..3c570a09b 100644 --- a/src/utils/__tests__/simulator-steps-pid.test.ts +++ b/src/utils/__tests__/simulator-steps-pid.test.ts @@ -72,7 +72,11 @@ describe.sequential('launchSimulatorAppWithLogging PID resolution', () => { logDir = path.join(registryDir, 'logs'); setSimulatorLaunchOsLogRegistryDirOverrideForTests(registryDir); setSimulatorLogDirOverrideForTests(logDir); - setRuntimeInstanceForTests({ instanceId: 'launch-test-instance', pid: process.pid }); + setRuntimeInstanceForTests({ + instanceId: 'launch-test-instance', + pid: process.pid, + workspaceKey: 'workspace-a', + }); setSimulatorLaunchOsLogRecordActiveOverrideForTests(async (record) => { const child = trackedChildren.get(record.helperPid); return child ? child.exitCode == null : true; diff --git a/src/utils/log-capture/index.ts b/src/utils/log-capture/index.ts index ec1092232..f4f576db3 100644 --- a/src/utils/log-capture/index.ts +++ b/src/utils/log-capture/index.ts @@ -1,26 +1,8 @@ -import { - activeLogSessions, - startLogCapture, - stopAllLogCaptures, - stopLogCapture, -} from '../log_capture.ts'; -import { - listActiveSimulatorLaunchOsLogSessions, - stopAllSimulatorLaunchOsLogSessions, - stopOwnedSimulatorLaunchOsLogSessions, - stopSimulatorLaunchOsLogSessionsForApp, -} from './simulator-launch-oslog-sessions.ts'; - -export type { SubsystemFilter } from '../log_capture.ts'; - -export function listActiveSimulatorLogSessionIds(): string[] { - return Array.from(activeLogSessions.keys()).sort(); -} - -export { startLogCapture, stopLogCapture, stopAllLogCaptures }; export { listActiveSimulatorLaunchOsLogSessions, + reconcileSimulatorLaunchOsLogOrphansForWorkspace, stopAllSimulatorLaunchOsLogSessions, stopOwnedSimulatorLaunchOsLogSessions, stopSimulatorLaunchOsLogSessionsForApp, -}; + terminateLiveSimulatorLaunchOsLogSessionsSync, +} from './simulator-launch-oslog-sessions.ts'; diff --git a/src/utils/log-capture/simulator-launch-oslog-registry.ts b/src/utils/log-capture/simulator-launch-oslog-registry.ts index f372dddbf..58279d18b 100644 --- a/src/utils/log-capture/simulator-launch-oslog-registry.ts +++ b/src/utils/log-capture/simulator-launch-oslog-registry.ts @@ -6,11 +6,9 @@ import { SIMULATOR_LAUNCH_OSLOG_REGISTRY_DIR } from '../log-paths.ts'; import type { RuntimeInstance } from '../runtime-instance.ts'; const execFileAsync = promisify(execFile); -const REGISTRY_VERSION = 1; const PROCESS_SAMPLE_CHUNK_SIZE = 100; export interface SimulatorLaunchOsLogRegistryRecord { - version: 1; sessionId: string; owner: RuntimeInstance; simulatorUuid: string; @@ -45,7 +43,6 @@ function isRecord(value: unknown): value is SimulatorLaunchOsLogRegistryRecord { const record = value as Partial; return ( - record.version === REGISTRY_VERSION && typeof record.sessionId === 'string' && typeof record.simulatorUuid === 'string' && typeof record.bundleId === 'string' && @@ -59,6 +56,9 @@ function isRecord(value: unknown): value is SimulatorLaunchOsLogRegistryRecord { typeof record.owner === 'object' && record.owner !== null && typeof record.owner.instanceId === 'string' && + record.owner.instanceId.length > 0 && + typeof record.owner.workspaceKey === 'string' && + record.owner.workspaceKey.length > 0 && typeof record.owner.pid === 'number' && Number.isInteger(record.owner.pid) && record.owner.pid > 0 diff --git a/src/utils/log-capture/simulator-launch-oslog-sessions.ts b/src/utils/log-capture/simulator-launch-oslog-sessions.ts index 0c9be9190..ad0ef7b5a 100644 --- a/src/utils/log-capture/simulator-launch-oslog-sessions.ts +++ b/src/utils/log-capture/simulator-launch-oslog-sessions.ts @@ -1,7 +1,7 @@ import type { ChildProcess } from 'node:child_process'; import { randomUUID } from 'node:crypto'; import { acquireDaemonActivity } from '../../daemon/activity-registry.ts'; -import { getRuntimeInstance } from '../runtime-instance.ts'; +import { getRuntimeInstance, getRuntimeInstanceIfConfigured } from '../runtime-instance.ts'; import { clearSimulatorLaunchOsLogRegistryForTests, compareOsLogSortKeys, @@ -36,7 +36,18 @@ export interface SimulatorLaunchOsLogSessionSummary { ownedByCurrentProcess: boolean; } +export interface SimulatorLaunchOsLogReconciliationResult { + scannedSessionCount: number; + eligibleOrphanCount: number; + stoppedSessionCount: number; + skippedLiveOwnerCount: number; + skippedDifferentWorkspaceCount: number; + errorCount: number; + errors: string[]; +} + const activeSimulatorLaunchOsLogSessions = new Map(); +let ownerPidAliveOverrideForTests: ((pid: number) => boolean) | null = null; function delay(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); @@ -61,6 +72,20 @@ function buildExpectedCommandParts(simulatorUuid: string, bundleId: string): str return ['simctl', 'spawn', simulatorUuid, 'log', 'stream', bundleId]; } +function isOwnerPidAlive(pid: number): boolean { + if (ownerPidAliveOverrideForTests) { + return ownerPidAliveOverrideForTests(pid); + } + + try { + process.kill(pid, 0); + return true; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + return code !== 'ESRCH'; + } +} + function finalizeLiveSession(sessionId: string, session: SimulatorLaunchOsLogSession): void { const current = activeSimulatorLaunchOsLogSessions.get(sessionId); if (current === session) { @@ -156,6 +181,20 @@ async function stopRecord( throw new Error('Timed out waiting for simulator launch OSLog process to exit'); } +function isSameWorkspaceReplaceableRecord( + record: SimulatorLaunchOsLogRegistryRecord, + workspaceKey: string, + currentInstanceId: string, +): boolean { + if (record.owner.workspaceKey !== workspaceKey) { + return false; + } + if (record.owner.instanceId === currentInstanceId) { + return true; + } + return !isOwnerPidAlive(record.owner.pid); +} + export async function registerSimulatorLaunchOsLogSession(params: { process: ChildProcess; simulatorUuid: string; @@ -193,7 +232,6 @@ export async function registerSimulatorLaunchOsLogSession(params: { try { await writeSimulatorLaunchOsLogRegistryRecord({ - version: 1, sessionId, owner: getRuntimeInstance(), simulatorUuid: params.simulatorUuid, @@ -213,14 +251,24 @@ export async function registerSimulatorLaunchOsLogSession(params: { export async function listActiveSimulatorLaunchOsLogSessions(): Promise< SimulatorLaunchOsLogSessionSummary[] > { - const currentInstanceId = getRuntimeInstance().instanceId; + const currentInstance = getRuntimeInstanceIfConfigured(); + if (!currentInstance) { + return []; + } return (await listSimulatorLaunchOsLogRegistryRecords()) - .map((record) => toSummary(record, currentInstanceId)) + .filter((record) => record.owner.workspaceKey === currentInstance.workspaceKey) + .map((record) => toSummary(record, currentInstance.instanceId)) .sort(compareOsLogSortKeys); } export async function getActiveSimulatorLaunchOsLogSessionCount(): Promise { - return (await listSimulatorLaunchOsLogRegistryRecords()).length; + const currentInstance = getRuntimeInstanceIfConfigured(); + if (!currentInstance) { + return 0; + } + return (await listSimulatorLaunchOsLogRegistryRecords()).filter( + (record) => record.owner.workspaceKey === currentInstance.workspaceKey, + ).length; } async function stopMatchingRecords( @@ -251,8 +299,19 @@ export async function stopSimulatorLaunchOsLogSessionsForApp( bundleId: string, timeoutMs = 1000, ): Promise<{ stoppedSessionCount: number; errorCount: number; errors: string[] }> { + const currentInstance = getRuntimeInstanceIfConfigured(); + if (!currentInstance) { + return { stoppedSessionCount: 0, errorCount: 0, errors: [] }; + } return stopMatchingRecords( - (record) => record.simulatorUuid === simulatorUuid && record.bundleId === bundleId, + (record) => + record.simulatorUuid === simulatorUuid && + record.bundleId === bundleId && + isSameWorkspaceReplaceableRecord( + record, + currentInstance.workspaceKey, + currentInstance.instanceId, + ), timeoutMs, ); } @@ -260,8 +319,14 @@ export async function stopSimulatorLaunchOsLogSessionsForApp( export async function stopOwnedSimulatorLaunchOsLogSessions( timeoutMs = 1000, ): Promise<{ stoppedSessionCount: number; errorCount: number; errors: string[] }> { - const currentInstanceId = getRuntimeInstance().instanceId; - return stopMatchingRecords((record) => record.owner.instanceId === currentInstanceId, timeoutMs); + const currentInstance = getRuntimeInstanceIfConfigured(); + if (!currentInstance) { + return { stoppedSessionCount: 0, errorCount: 0, errors: [] }; + } + return stopMatchingRecords( + (record) => record.owner.instanceId === currentInstance.instanceId, + timeoutMs, + ); } export async function stopAllSimulatorLaunchOsLogSessions( @@ -270,6 +335,79 @@ export async function stopAllSimulatorLaunchOsLogSessions( return stopMatchingRecords(() => true, timeoutMs); } +export async function reconcileSimulatorLaunchOsLogOrphansForWorkspace( + workspaceKey: string, + timeoutMs = 1000, +): Promise { + const records = await listSimulatorLaunchOsLogRegistryRecords(); + const errors: string[] = []; + let eligibleOrphanCount = 0; + let stoppedSessionCount = 0; + let skippedLiveOwnerCount = 0; + let skippedDifferentWorkspaceCount = 0; + + for (const record of records) { + if (record.owner.workspaceKey !== workspaceKey) { + skippedDifferentWorkspaceCount += 1; + continue; + } + + if (record.owner.pid === process.pid || isOwnerPidAlive(record.owner.pid)) { + skippedLiveOwnerCount += 1; + continue; + } + + eligibleOrphanCount += 1; + try { + await stopRecord(record, timeoutMs); + stoppedSessionCount += 1; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + errors.push(`${record.sessionId}: ${message}`); + } + } + + return { + scannedSessionCount: records.length, + eligibleOrphanCount, + stoppedSessionCount, + skippedLiveOwnerCount, + skippedDifferentWorkspaceCount, + errorCount: errors.length, + errors, + }; +} + +export function terminateLiveSimulatorLaunchOsLogSessionsSync(signal: NodeJS.Signals = 'SIGTERM'): { + attemptedCount: number; + errorCount: number; + errors: string[]; +} { + const errors: string[] = []; + let attemptedCount = 0; + + for (const [sessionId, session] of activeSimulatorLaunchOsLogSessions.entries()) { + if (session.hasEnded || session.process.exitCode !== null) { + continue; + } + + attemptedCount += 1; + try { + session.process.kill?.(signal); + finalizeLiveSession(sessionId, session); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + errors.push(`${sessionId}: ${message}`); + } + } + + return { + attemptedCount, + errorCount: errors.length, + errors, + }; +} + export async function clearAllSimulatorLaunchOsLogSessionsForTests(): Promise { for (const [sessionId, session] of activeSimulatorLaunchOsLogSessions.entries()) { finalizeLiveSession(sessionId, session); @@ -281,3 +419,9 @@ export async function clearAllSimulatorLaunchOsLogSessionsForTests(): Promise boolean) | null, +): void { + ownerPidAliveOverrideForTests = override; +} diff --git a/src/utils/log_capture.ts b/src/utils/log_capture.ts deleted file mode 100644 index b7a47f9d7..000000000 --- a/src/utils/log_capture.ts +++ /dev/null @@ -1,405 +0,0 @@ -import * as path from 'node:path'; -import type { ChildProcess } from 'node:child_process'; -import type { Writable } from 'node:stream'; -import { finished } from 'node:stream/promises'; -import { v4 as uuidv4 } from 'uuid'; -import { log } from '../utils/logger.ts'; -import type { CommandExecutor } from './command.ts'; -import { getDefaultCommandExecutor, getDefaultFileSystemExecutor } from './command.ts'; -import { normalizeSimctlChildEnv } from './environment.ts'; -import type { FileSystemExecutor } from './FileSystemExecutor.ts'; -import { acquireDaemonActivity } from '../daemon/activity-registry.ts'; -import { LOG_DIR as APP_LOG_DIR } from './log-paths.ts'; - -/** - * Log file retention policy: - * - Old log files (older than LOG_RETENTION_DAYS) are automatically deleted from the temp directory - * - Cleanup runs on every new log capture start - */ -const LOG_RETENTION_DAYS = 3; - -export interface LogSession { - processes: ChildProcess[]; - logFilePath: string; - simulatorUuid: string; - bundleId: string; - logStream: Writable; - releaseActivity?: () => void; -} - -/** - * Subsystem filter options for log capture. - * - 'app': Only capture logs from the app's bundle ID subsystem (default) - * - 'all': Capture all logs (no subsystem filtering) - * - 'swiftui': Capture logs from app + SwiftUI subsystem (useful for Self._printChanges()) - * - string[]: Custom array of subsystems to capture (always includes the app's bundle ID) - */ -export type SubsystemFilter = 'app' | 'all' | 'swiftui' | string[]; - -/** - * Escape a string for safe use inside a double-quoted NSPredicate string literal. - * Backslash-escapes any backslashes and double quotes so the value cannot - * break out of the `"..."` context in predicates like `subsystem == "VALUE"`. - */ -function escapePredicateString(value: string): string { - return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); -} - -/** - * Build the predicate string for log filtering based on subsystem filter option. - */ -function buildLogPredicate(bundleId: string, subsystemFilter: SubsystemFilter): string | null { - if (subsystemFilter === 'all') { - return null; - } - - const safeBundleId = escapePredicateString(bundleId); - - if (subsystemFilter === 'app') { - return `subsystem == "${safeBundleId}"`; - } - - if (subsystemFilter === 'swiftui') { - return `subsystem == "${safeBundleId}" OR subsystem == "com.apple.SwiftUI"`; - } - - const subsystems = new Set([bundleId, ...subsystemFilter]); - const predicates = Array.from(subsystems).map( - (s) => `subsystem == "${escapePredicateString(s)}"`, - ); - return predicates.join(' OR '); -} - -export const activeLogSessions: Map = new Map(); - -/** - * Start a log capture session for an iOS simulator. - * Returns { sessionId, logFilePath, processes, error? } - */ -export async function startLogCapture( - params: { - simulatorUuid: string; - bundleId: string; - captureConsole?: boolean; - args?: string[]; - env?: Record; - subsystemFilter?: SubsystemFilter; - }, - executor: CommandExecutor = getDefaultCommandExecutor(), - fileSystem: FileSystemExecutor = getDefaultFileSystemExecutor(), -): Promise<{ sessionId: string; logFilePath: string; processes: ChildProcess[]; error?: string }> { - // Clean up old logs before starting a new session - await cleanOldLogs(fileSystem); - - const { - simulatorUuid, - bundleId, - captureConsole = false, - args = [], - env, - subsystemFilter = 'app', - } = params; - const logSessionId = uuidv4(); - const ts = new Date().toISOString().replace(/:/g, '-').replace('.', '-').slice(0, -1) + 'Z'; - const logFileName = `${bundleId}_${ts}_pid${process.pid}.log`; - const logsDir = APP_LOG_DIR; - const logFilePath = path.join(logsDir, logFileName); - - let logStream: Writable | null = null; - const processes: ChildProcess[] = []; - const closeFailedCapture = async (): Promise => { - for (const process of processes) { - try { - if (!process.killed && process.exitCode === null) { - process.kill('SIGTERM'); - } - } catch (error) { - log( - 'warn', - `Failed to stop log capture process during cleanup: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } - } - - if (logStream) { - logStream.end(); - try { - await finished(logStream); - } catch (error) { - log( - 'warn', - `Failed to flush log stream during cleanup: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } - } - }; - - try { - await fileSystem.mkdir(logsDir, { recursive: true }); - await fileSystem.writeFile(logFilePath, ''); - logStream = fileSystem.createWriteStream(logFilePath, { flags: 'a' }); - logStream.write(`\n--- Log capture for bundle ID: ${bundleId} ---\n`); - - if (captureConsole) { - const launchCommand = [ - 'xcrun', - 'simctl', - 'launch', - '--console-pty', - '--terminate-running-process', - simulatorUuid, - bundleId, - ]; - if (args.length > 0) { - launchCommand.push(...args); - } - - const launchOpts = env ? { env: normalizeSimctlChildEnv(env) } : undefined; - const stdoutLogResult = await executor( - launchCommand, - 'Console Log Capture', - false, - launchOpts, - true, - ); - - if (!stdoutLogResult.success) { - await closeFailedCapture(); - return { - sessionId: '', - logFilePath: '', - processes: [], - error: stdoutLogResult.error ?? 'Failed to start console log capture', - }; - } - - stdoutLogResult.process.stdout?.pipe(logStream, { end: false }); - stdoutLogResult.process.stderr?.pipe(logStream, { end: false }); - processes.push(stdoutLogResult.process); - } - - const logPredicate = buildLogPredicate(bundleId, subsystemFilter); - const osLogCommand = [ - 'xcrun', - 'simctl', - 'spawn', - simulatorUuid, - 'log', - 'stream', - '--level=debug', - ]; - - if (logPredicate) { - osLogCommand.push('--predicate', logPredicate); - } - - const osLogResult = await executor(osLogCommand, 'OS Log Capture', false, undefined, true); - - if (!osLogResult.success) { - await closeFailedCapture(); - return { - sessionId: '', - logFilePath: '', - processes: [], - error: osLogResult.error ?? 'Failed to start OS log capture', - }; - } - - osLogResult.process.stdout?.pipe(logStream, { end: false }); - osLogResult.process.stderr?.pipe(logStream, { end: false }); - processes.push(osLogResult.process); - - for (const process of processes) { - process.on('close', (code) => { - log('info', `A log capture process for session ${logSessionId} exited with code ${code}.`); - }); - process.unref?.(); - (process.stdout as any)?.unref?.(); - (process.stderr as any)?.unref?.(); - } - - const releaseActivity = acquireDaemonActivity('logging.simulator'); - activeLogSessions.set(logSessionId, { - processes, - logFilePath, - simulatorUuid, - bundleId, - logStream, - releaseActivity, - }); - - log('info', `Log capture started with session ID: ${logSessionId}`); - return { sessionId: logSessionId, logFilePath, processes }; - } catch (error) { - await closeFailedCapture(); - const message = error instanceof Error ? error.message : String(error); - log('error', `Failed to start log capture: ${message}`); - return { sessionId: '', logFilePath: '', processes: [], error: message }; - } -} - -interface StopLogSessionOptions { - timeoutMs?: number; - readLogContent?: boolean; - fileSystem: FileSystemExecutor; -} - -interface StopLogSessionResult { - logContent: string; - logFilePath?: string; - error?: string; -} - -interface MinimalWritable { - end: () => void; - destroy?: (error?: Error) => void; -} - -function createTimeoutPromise(timeoutMs: number): Promise<'timed_out'> { - return new Promise((resolve) => { - const timer = setTimeout(() => resolve('timed_out'), timeoutMs); - timer.unref?.(); - }); -} - -async function closeLogStreamWithTimeout( - stream: MinimalWritable, - timeoutMs: number, -): Promise { - stream.end(); - const closePromise = finished(stream as Writable) - .then(() => 'closed' as const) - .catch(() => 'closed' as const); - const outcome = await Promise.race([closePromise, createTimeoutPromise(timeoutMs)]); - if (outcome === 'timed_out') { - stream.destroy?.(); - } -} - -async function stopLogSession( - logSessionId: string, - options: StopLogSessionOptions, -): Promise { - const session = activeLogSessions.get(logSessionId); - if (!session) { - return { logContent: '', error: `Log capture session not found: ${logSessionId}` }; - } - - activeLogSessions.delete(logSessionId); - - try { - for (const process of session.processes) { - if (!process.killed && process.exitCode === null) { - process.kill('SIGTERM'); - } - } - - await closeLogStreamWithTimeout( - session.logStream as MinimalWritable, - options.timeoutMs ?? 1000, - ); - - if (options.readLogContent) { - if (!options.fileSystem.existsSync(session.logFilePath)) { - return { logContent: '', error: `Log file not found: ${session.logFilePath}` }; - } - const fileContent = await options.fileSystem.readFile(session.logFilePath, 'utf-8'); - return { logContent: fileContent, logFilePath: session.logFilePath }; - } - - return { logContent: '' }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { logContent: '', error: message }; - } finally { - session.releaseActivity?.(); - } -} - -/** - * Stop a log capture session and retrieve the log content. - */ -export async function stopLogCapture( - logSessionId: string, - fileSystem: FileSystemExecutor = getDefaultFileSystemExecutor(), -): Promise<{ logContent: string; logFilePath?: string; error?: string }> { - const result = await stopLogSession(logSessionId, { - fileSystem, - readLogContent: true, - timeoutMs: 1000, - }); - - if (result.error) { - log('error', `Failed to stop log capture session ${logSessionId}: ${result.error}`); - return { logContent: '', error: result.error }; - } - - log('info', `Log capture session ${logSessionId} stopped.`); - return result; -} - -export async function stopAllLogCaptures(timeoutMs = 1000): Promise<{ - stoppedSessionCount: number; - errorCount: number; - errors: string[]; -}> { - const sessionIds = Array.from(activeLogSessions.keys()); - const errors: string[] = []; - for (const sessionId of sessionIds) { - const result = await stopLogSession(sessionId, { - fileSystem: getDefaultFileSystemExecutor(), - readLogContent: false, - timeoutMs, - }); - if (result.error) { - errors.push(`${sessionId}: ${result.error}`); - } - } - - return { - stoppedSessionCount: sessionIds.length - errors.length, - errorCount: errors.length, - errors, - }; -} - -/** - * Deletes log files older than LOG_RETENTION_DAYS from the temp directory. - * Runs quietly; errors are logged but do not throw. - */ -async function cleanOldLogs(fileSystem: FileSystemExecutor): Promise { - const logsDir = APP_LOG_DIR; - let files: unknown[]; - try { - files = await fileSystem.readdir(logsDir); - } catch { - return; - } - const now = Date.now(); - const retentionMs = LOG_RETENTION_DAYS * 24 * 60 * 60 * 1000; - const fileNames = files.filter((file): file is string => typeof file === 'string'); - - await Promise.all( - fileNames - .filter((f) => f.endsWith('.log')) - .map(async (f) => { - const filePath = path.join(logsDir, f); - try { - const stat = await fileSystem.stat(filePath); - if (now - stat.mtimeMs > retentionMs) { - await fileSystem.rm(filePath, { force: true }); - log('info', `Deleted old log file: ${filePath}`); - } - } catch (err) { - log( - 'warn', - `Error during log cleanup for ${filePath}: ${err instanceof Error ? err.message : String(err)}`, - ); - } - }), - ); -} diff --git a/src/utils/runtime-instance.ts b/src/utils/runtime-instance.ts index 1ebcf335b..c799617ef 100644 --- a/src/utils/runtime-instance.ts +++ b/src/utils/runtime-instance.ts @@ -3,18 +3,62 @@ import { randomUUID } from 'node:crypto'; export interface RuntimeInstance { instanceId: string; pid: number; + workspaceKey: string; } let runtimeInstance: RuntimeInstance | null = null; +let runtimeWorkspaceKey: string | null = null; + +export function configureRuntimeWorkspaceKey(workspaceKey: string): void { + const normalizedWorkspaceKey = workspaceKey.trim(); + if (normalizedWorkspaceKey.length === 0) { + throw new Error('Runtime workspace key cannot be empty'); + } + + runtimeWorkspaceKey = normalizedWorkspaceKey; + if (runtimeInstance) { + runtimeInstance = { ...runtimeInstance, workspaceKey: normalizedWorkspaceKey }; + } +} export function getRuntimeInstance(): RuntimeInstance { - runtimeInstance ??= { + if (runtimeInstance) { + return runtimeInstance; + } + + if (!runtimeWorkspaceKey) { + throw new Error('Runtime workspace key has not been configured'); + } + + runtimeInstance = { instanceId: randomUUID(), pid: process.pid, + workspaceKey: runtimeWorkspaceKey, }; return runtimeInstance; } +export function getRuntimeInstanceIfConfigured(): RuntimeInstance | null { + if (runtimeInstance) { + return runtimeInstance; + } + if (!runtimeWorkspaceKey) { + return null; + } + return getRuntimeInstance(); +} + export function setRuntimeInstanceForTests(instance: RuntimeInstance | null): void { runtimeInstance = instance; + runtimeWorkspaceKey = instance?.workspaceKey ?? null; +} + +export function setRuntimeWorkspaceKeyForTests(workspaceKey: string | null): void { + if (workspaceKey === null) { + runtimeWorkspaceKey = null; + runtimeInstance = null; + return; + } + + configureRuntimeWorkspaceKey(workspaceKey); } diff --git a/src/utils/session-status.ts b/src/utils/session-status.ts index d4d506188..441da676b 100644 --- a/src/utils/session-status.ts +++ b/src/utils/session-status.ts @@ -1,8 +1,5 @@ import { getDefaultDebuggerManager } from './debugger/index.ts'; -import { - listActiveSimulatorLaunchOsLogSessions, - listActiveSimulatorLogSessionIds, -} from './log-capture/index.ts'; +import { listActiveSimulatorLaunchOsLogSessions } from './log-capture/index.ts'; import { activeDeviceLogSessions } from './log-capture/device-log-sessions.ts'; import { getDaemonActivitySnapshot } from '../daemon/activity-registry.ts'; import { activeProcesses } from '../mcp/tools/swift-package/active-processes.ts'; @@ -12,7 +9,6 @@ import { getWatchedPath, isWatcherRunning } from './xcode-state-watcher.ts'; export type SessionRuntimeStatusSnapshot = { logging: { simulator: { - activeSessionIds: string[]; activeLaunchOsLogSessions: Array<{ sessionId: string; simulatorUuid: string; @@ -63,7 +59,6 @@ export async function getSessionRuntimeStatusSnapshot(): Promise Date: Fri, 1 May 2026 12:26:39 +0100 Subject: [PATCH 2/3] docs: Document transient investigation note policy Do not keep completed investigation notes in pull requests. Record unresolved follow-up work in GitHub issues instead so branches only carry durable project changes. Co-Authored-By: Codex --- AGENTS.md | 4 +- CLAUDE.md | 4 +- .../orphaned-simctl-log-stream-2026-05-01.md | 100 ------------------ 3 files changed, 6 insertions(+), 102 deletions(-) delete mode 100644 docs/investigations/orphaned-simctl-log-stream-2026-05-01.md diff --git a/AGENTS.md b/AGENTS.md index b493eb4eb..c4f13e25d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,7 +87,9 @@ When reading issues: - Technical prose only, be kind but direct (e.g., "Thanks @user" not "Thanks so much @user!") ## Docs -- +- Do not commit transient investigation notes, prompt exports, or scratch analysis docs after the work is complete. +- If an investigation leaves unresolved follow-up work, move it to a GitHub issue instead of preserving the transient doc in the branch. + ### Changelog Location: `CHANGELOG.md` diff --git a/CLAUDE.md b/CLAUDE.md index 40ab6744f..3bc17849a 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,7 +28,9 @@ When reading issues: - Technical prose only, be kind but direct (e.g., "Thanks @user" not "Thanks so much @user!") ## Docs -- +- Do not commit transient investigation notes, prompt exports, or scratch analysis docs after the work is complete. +- If an investigation leaves unresolved follow-up work, move it to a GitHub issue instead of preserving the transient doc in the branch. + ### Changelog Location: `CHANGELOG.md` diff --git a/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md b/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md deleted file mode 100644 index 3a3c3c345..000000000 --- a/docs/investigations/orphaned-simctl-log-stream-2026-05-01.md +++ /dev/null @@ -1,100 +0,0 @@ -# Investigation: Orphaned simctl OSLog processes when MCP server exits abnormally - -**Issue:** [#382](https://github.com/getsentry/XcodeBuildMCP/issues/382) -**Date:** 2026-05-01 -**Status:** Confirmed and fixed for simulator OSLog helpers. - -## Summary - -The issue described orphaned simulator helper processes after abnormal server exits. The final fix is deliberately narrow: - -- `simctl spawn … log stream …` OSLog helpers are registry-backed and now reconciled safely across MCP/daemon restarts. -- `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. -- The old `src/utils/log_capture.ts` path was dead production code and has been removed. - -The fix does not change public tool schemas. - -## Final design - -### OSLog helpers are workspace-scoped - -Durable OSLog registry records now store owner identity as: - -```ts -owner: { - instanceId: string; - pid: number; - workspaceKey: string; -} -``` - -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. - -### Reconciliation only reaps safe orphans - -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: - -1. The record belongs to the current workspace. -2. The owner PID is not the current process. -3. The owner PID is no longer alive. -4. The helper process still matches the expected `simctl spawn … log stream …` command. - -Records from other workspaces are skipped. Records owned by live processes are skipped, including live foreign MCP/daemon sessions in the same workspace. - -### App-scoped cleanup is no longer global - -`stop_app_sim` and relaunch cleanup still clean OSLog helpers for the target simulator/bundle, but only when the record is: - -- owned by the current runtime instance, or -- in the same workspace with a dead owner PID. - -This prevents one active session from killing another active session’s OSLog helper. - -### Last-chance cleanup covers local live helpers - -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. - -Daemon crash handling was also brought closer to MCP parity with `uncaughtException` and `unhandledRejection` handling that requests shutdown with a non-zero exit code. - -### Dead log-capture path removed - -`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`. - -Removed: - -- `src/utils/log_capture.ts` -- `src/utils/__tests__/log_capture.test.ts` -- `src/utils/__tests__/log_capture_escape.test.ts` -- legacy re-exports from `src/utils/log-capture/index.ts` -- legacy shutdown/status references to `stopAllLogCaptures`, `activeLogSessions`, and `simulatorLogSessionCount` - -## Why console-PTY is not registered - -`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. - -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. - -## Verification expectations - -Automated coverage should verify: - -- OSLog registry records require `owner.workspaceKey`. -- records without `owner.workspaceKey` are pruned. -- same-workspace dead-owner OSLog helpers are reconciled. -- other-workspace records are skipped. -- same-workspace live-owner records are skipped. -- app-scoped cleanup skips live foreign owners. -- lifecycle snapshots and session status do not expose deleted legacy log-capture fields. -- synchronous exit cleanup signals only live local OSLog helpers. - -Manual smoke testing should verify: - -1. Clean shutdown stops helpers owned by the current server. -2. After `SIGKILL` of an MCP server, restarting in the same workspace reaps the orphaned `simctl spawn … log stream …` helper. -3. Two live sessions in the same workspace do not kill each other’s OSLog helpers. -4. Startup from workspace A does not kill workspace B helpers. -5. `stop_app_sim` does not kill live foreign-owner OSLog helpers. - -## Known boundary - -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. From 0414d6da64073a212bcf3ef1d1443c881a10421e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 1 May 2026 11:33:28 +0000 Subject: [PATCH 3/3] Remove unused setRuntimeWorkspaceKeyForTests helper Co-authored-by: Cameron Cooke --- src/utils/runtime-instance.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/utils/runtime-instance.ts b/src/utils/runtime-instance.ts index c799617ef..1f3f60967 100644 --- a/src/utils/runtime-instance.ts +++ b/src/utils/runtime-instance.ts @@ -52,13 +52,3 @@ export function setRuntimeInstanceForTests(instance: RuntimeInstance | null): vo runtimeInstance = instance; runtimeWorkspaceKey = instance?.workspaceKey ?? null; } - -export function setRuntimeWorkspaceKeyForTests(workspaceKey: string | null): void { - if (workspaceKey === null) { - runtimeWorkspaceKey = null; - runtimeInstance = null; - return; - } - - configureRuntimeWorkspaceKey(workspaceKey); -}