Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ When reading issues:
- When working on skill sources in `skills/`, use the `skill-creator` skill workflow.
- After modifying any skill source, run `npx skill-check <skill-directory>` and address all errors/warnings before handoff.
-
## Multi-process filesystem state
- XcodeBuildMCP explicitly supports multiple concurrent MCP server, daemon, CLI, test, and helper processes for the same or different workspaces.
- Shared filesystem state under `~/Library/Developer/XcodeBuildMCP` must be multi-process safe.
- Use workspace-key scoped directories for workspace-owned state.
- Do not store runtime state under `~/.xcodebuildmcp`; `.xcodebuildmcp/config.yaml` is only project configuration.
- Use shared lock and atomic-write helpers for mutable shared files.
- Prefer one-record-per-file registries over shared aggregate files.
- Cleanup must verify ownership before deleting shared artifacts.

## Style
- Keep answers short and concise
- No emojis in commits, issues, PR comments, or code
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- 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)).
- Fixed Weather example test discovery and made CLI test progress visible while tests are running instead of leaving the last build phase displayed.

### Changed

- Centralized workspace log retention and startup/shutdown filesystem cleanup so XcodeBuildMCP-owned logs are pruned consistently while preserving active daemon and simulator OSLog outputs.

## [2.5.0-beta.1]

### Breaking
Expand Down
9 changes: 9 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ When reading issues:
- When working on skill sources in `skills/`, use the `skill-creator` skill workflow.
- After modifying any skill source, run `npx skill-check <skill-directory>` and address all errors/warnings before handoff.
-
## Multi-process filesystem state
- XcodeBuildMCP explicitly supports multiple concurrent MCP server, daemon, CLI, test, and helper processes for the same or different workspaces.
- Shared filesystem state under `~/Library/Developer/XcodeBuildMCP` must be multi-process safe.
- Use workspace-key scoped directories for workspace-owned state.
- Do not store runtime state under `~/.xcodebuildmcp`; `.xcodebuildmcp/config.yaml` is only project configuration.
- Use shared lock and atomic-write helpers for mutable shared files.
- Prefer one-record-per-file registries over shared aggregate files.
- Cleanup must verify ownership before deleting shared artifacts.

## Style
- Keep answers short and concise
- No emojis in commits, issues, PR comments, or code
Expand Down
15 changes: 2 additions & 13 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
import { bootstrapRuntime } from './runtime/bootstrap-runtime.ts';
import { buildCliToolCatalog } from './cli/cli-tool-catalog.ts';
import { buildYargsApp } from './cli/yargs-app.ts';
import { getSocketPath, getWorkspaceKey, resolveWorkspaceRoot } from './daemon/socket-path.ts';
import { getSocketPath } from './daemon/socket-path.ts';
import { startMcpServer } from './server/start-mcp-server.ts';
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']);
Expand Down Expand Up @@ -119,23 +118,13 @@ async function main(): Promise<void> {
},
});

// Compute workspace context for daemon routing
const workspaceRoot = resolveWorkspaceRoot({
cwd: result.runtime.cwd,
projectConfigPath: result.configPath,
});
const { workspaceRoot, workspaceKey } = result;

const defaultSocketPath = getSocketPath({
cwd: result.runtime.cwd,
projectConfigPath: result.configPath,
});

const workspaceKey = getWorkspaceKey({
cwd: result.runtime.cwd,
projectConfigPath: result.configPath,
});
configureRuntimeWorkspaceKey(workspaceKey);

const cliExposedWorkflowIds = await listCliWorkflowIdsFromManifest({
excludeWorkflows: ['session-management', 'workflow-discovery'],
});
Expand Down
28 changes: 21 additions & 7 deletions src/cli/daemon-control.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { spawn } from 'node:child_process';
import { fileURLToPath } from 'node:url';
import { dirname, resolve, basename } from 'node:path';
import { existsSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { existsSync, unlinkSync } from 'node:fs';
import { DaemonClient, DaemonVersionMismatchError } from './daemon-client.ts';
import { readDaemonRegistryEntry } from '../daemon/daemon-registry.ts';
import { removeStaleSocket } from '../daemon/socket-path.ts';
import {
cleanupWorkspaceDaemonFiles,
findDaemonRegistryEntryBySocketPath,
} from '../daemon/daemon-registry.ts';

/**
* Default timeout for daemon startup in milliseconds.
Expand Down Expand Up @@ -38,8 +40,7 @@
* sends SIGTERM, and removes the stale socket.
*/
export async function forceStopDaemon(socketPath: string): Promise<void> {
const workspaceKey = basename(dirname(socketPath));
const entry = readDaemonRegistryEntry(workspaceKey);
const entry = findDaemonRegistryEntryBySocketPath(socketPath);
if (entry?.pid) {
try {
process.kill(entry.pid, 'SIGTERM');
Expand All @@ -49,7 +50,20 @@
// Brief wait for the process to exit.
await new Promise((resolve) => setTimeout(resolve, 500));
}
removeStaleSocket(socketPath);
if (entry) {
cleanupWorkspaceDaemonFiles(entry.workspaceKey, {
pid: entry.pid,
socketPath,
Comment thread
sentry[bot] marked this conversation as resolved.
Outdated
});

Check warning on line 57 in src/cli/daemon-control.ts

View workflow job for this annotation

GitHub Actions / warden: find-bugs

forceStopDaemon may leave socket and registry in place when process hasn't exited within 500ms

forceStopDaemon sends SIGTERM, waits only 500ms, then calls cleanupWorkspaceDaemonFiles without allowLiveOwner: true. In daemon-registry.ts, canRemoveRegistryEntry requires !isPidAlive(entry.pid) when allowLiveOwner is not set, so if the daemon hasn't died yet the registry entry and socket file are not removed. Since forceStopDaemon's documented purpose is to remove the stale socket after a version-mismatch force-stop, callers (e.g. ensureDaemonRunning) may immediately try to start a new daemon while the old socket is still present, defeating the cleanup.

Check warning on line 57 in src/cli/daemon-control.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

[7EB-RM7] Forced-shutdown timeout is never cleared, causing concurrent cleanup and wrong exit code (additional location)

The `setTimeout` at line 344 is never cancelled when `server.close()` completes successfully. If the server closes (line 334) but the subsequent `cleanupArtifacts()` + `flushAndCloseSentry(2000)` chain takes longer than 5 seconds (the Sentry flush alone allows up to 2s), the timeout fires and invokes `cleanupArtifacts()` a second time concurrently with the in-flight cleanup, then calls `process.exit(1)`. This can (a) race two cleanup runs against the same workspace-keyed artifacts/locks, and (b) override the intended `exitCode` with `1`, masking clean shutdowns as failures.
Comment thread
cameroncooke marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
} else {
// Registry entry missing; cannot derive workspace key from socket path alone.
// Clean up the socket file directly.
try {
unlinkSync(socketPath);
} catch {
// Socket may already be gone.
}
}
}
Comment thread
sentry[bot] marked this conversation as resolved.

export interface StartDaemonBackgroundOptions {
Expand Down
144 changes: 91 additions & 53 deletions src/daemon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@
ensureSocketDir,
removeStaleSocket,
getSocketPath,
getWorkspaceKey,
resolveWorkspaceRoot,
logPathForWorkspaceKey,
} from './daemon/socket-path.ts';
import { startDaemonServer } from './daemon/daemon-server.ts';
import {
acquireDaemonRegistryMutationLock,
writeDaemonRegistryEntry,
removeDaemonRegistryEntry,
cleanupWorkspaceDaemonFiles,
type DaemonRegistryMutationLock,
} from './daemon/daemon-registry.ts';
import { log, normalizeLogLevel, setLogFile, setLogLevel } from './utils/logger.ts';
import { version } from './version.ts';
Expand All @@ -42,11 +40,11 @@
} 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';
cleanupOwnedWorkspaceFilesystemArtifacts,
runWorkspaceFilesystemLifecycleSweep,
terminateOwnedWorkspaceFilesystemArtifactsSync,
} from './utils/workspace-filesystem-lifecycle.ts';

async function checkExistingDaemon(socketPath: string): Promise<boolean> {
return new Promise<boolean>((resolve) => {
Expand Down Expand Up @@ -124,16 +122,7 @@
},
});

const workspaceRoot = resolveWorkspaceRoot({
cwd: result.runtime.cwd,
projectConfigPath: result.configPath,
});

const workspaceKey = getWorkspaceKey({
cwd: result.runtime.cwd,
projectConfigPath: result.configPath,
});
configureRuntimeWorkspaceKey(workspaceKey);
const { workspaceRoot, workspaceKey } = result;

const logPath = resolveDaemonLogPath(workspaceKey);
if (logPath) {
Expand All @@ -159,20 +148,27 @@

log('info', `[Daemon] Workspace: ${workspaceRoot}`);
log('info', `[Daemon] Socket: ${socketPath}`);
try {
const reconciliation = await reconcileSimulatorLaunchOsLogOrphansForWorkspace(workspaceKey);
if (reconciliation.stoppedSessionCount > 0 || reconciliation.errorCount > 0) {

const runStartupLifecycleSweep = async (): Promise<void> => {
try {
const lifecycle = await runWorkspaceFilesystemLifecycleSweep({
workspaceKey,
trigger: 'startup',
});
if (lifecycle.stopped > 0 || lifecycle.deleted > 0 || lifecycle.errors.length > 0) {
log(
lifecycle.errors.length > 0 ? 'warn' : 'info',
`[Daemon] Filesystem lifecycle: ${JSON.stringify(lifecycle)}`,
);
}
} catch (error) {
log(
reconciliation.errorCount > 0 ? 'warn' : 'info',
`[Daemon] Simulator OSLog reconciliation: ${JSON.stringify(reconciliation)}`,
'warn',
`[Daemon] Filesystem lifecycle failed: ${error instanceof Error ? error.message : String(error)}`,
);
}
} catch (error) {
log(
'warn',
`[Daemon] Simulator OSLog reconciliation failed: ${error instanceof Error ? error.message : String(error)}`,
);
}
};

if (logPath) {
log('info', `[Daemon] Logs: ${logPath}`);
}
Expand All @@ -187,6 +183,28 @@
process.exit(1);
}

const startupRegistryLock = acquireDaemonRegistryMutationLock(workspaceKey);
if (!startupRegistryLock) {
log('error', '[Daemon] Unable to acquire daemon registry lock');
console.error('Error: Unable to acquire daemon registry lock');
await flushAndCloseSentry(1000);
process.exit(1);
}
let pendingStartupRegistryLock: DaemonRegistryMutationLock | null = startupRegistryLock;
const releaseStartupRegistryLock = (): void => {
pendingStartupRegistryLock?.release();
pendingStartupRegistryLock = null;
};

Check warning on line 197 in src/daemon.ts

View check run for this annotation

@sentry/warden / warden: code-review

Daemon registry lock can leak if startup throws before server.listen callback

The startup registry lock is acquired at line 186, but is only released in three places: (1) the explicit early-exit when another daemon is detected after lock (line 201), (2) the `server.on('error', releaseStartupRegistryLock)` handler registered at line 412, and (3) the `finally` block inside the `server.listen` callback at line 433. If any synchronous or asynchronous code between lock acquisition and `server.listen` (e.g. `buildDaemonToolCatalogFromManifest`, `loadManifest`, `startDaemonServer`) throws, the lock is never released. The top-level `main().catch` handler calls `process.exit(1)` but does not invoke `releaseStartupRegistryLock`, which can leave a stale lock file blocking subsequent daemon startups for the same workspace.
Comment thread
github-actions[bot] marked this conversation as resolved.

const isRunningAfterLock = await checkExistingDaemon(socketPath);
if (isRunningAfterLock) {
releaseStartupRegistryLock();
log('error', '[Daemon] Another daemon is already running for this workspace');
console.error('Error: Daemon is already running for this workspace');
await flushAndCloseSentry(1000);
process.exit(1);
}

removeStaleSocket(socketPath);

const excludedWorkflows = ['session-management', 'workflow-discovery'];
Expand Down Expand Up @@ -302,28 +320,35 @@
recordDaemonLifecycleMetric('shutdown');
log('info', '[Daemon] Shutting down...');

// Close the server
const cleanupArtifacts = (): Promise<unknown> =>
cleanupOwnedWorkspaceFilesystemArtifacts({
workspaceKey,
trigger: 'shutdown',
daemonCleanup: {
pid: process.pid,
socketPath,
allowLiveOwner: true,
},
});

server.close(() => {
log('info', '[Daemon] Server closed');

// Remove registry entry and socket
removeDaemonRegistryEntry(workspaceKey);
removeStaleSocket(socketPath);

log('info', '[Daemon] Cleanup complete');
void flushAndCloseSentry(2000).finally(() => {
process.exit(exitCode);
void cleanupArtifacts().finally(() => {
log('info', '[Daemon] Cleanup complete');
void flushAndCloseSentry(2000).finally(() => {
process.exit(exitCode);
});
});
Comment thread
cameroncooke marked this conversation as resolved.
});

// Force exit if server doesn't close in time
setTimeout(() => {
log('warn', '[Daemon] Forced shutdown after timeout');
cleanupWorkspaceDaemonFiles(workspaceKey);
void flushAndCloseSentry(1000).finally(() => {
process.exit(1);
void cleanupArtifacts().finally(() => {
void flushAndCloseSentry(1000).finally(() => {
process.exit(1);
});
});
}, 5000);

Check warning on line 351 in src/daemon.ts

View workflow job for this annotation

GitHub Actions / warden: find-bugs

[JYJ-UYW] forceStopDaemon may leave socket and registry in place when process hasn't exited within 500ms (additional location)

forceStopDaemon sends SIGTERM, waits only 500ms, then calls cleanupWorkspaceDaemonFiles without allowLiveOwner: true. In daemon-registry.ts, canRemoveRegistryEntry requires !isPidAlive(entry.pid) when allowLiveOwner is not set, so if the daemon hasn't died yet the registry entry and socket file are not removed. Since forceStopDaemon's documented purpose is to remove the stale socket after a version-mismatch force-stop, callers (e.g. ensureDaemonRunning) may immediately try to start a new daemon while the old socket is still present, defeating the cleanup.

Check warning on line 351 in src/daemon.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

Forced-shutdown timeout is never cleared, causing concurrent cleanup and wrong exit code

The `setTimeout` at line 344 is never cancelled when `server.close()` completes successfully. If the server closes (line 334) but the subsequent `cleanupArtifacts()` + `flushAndCloseSentry(2000)` chain takes longer than 5 seconds (the Sentry flush alone allows up to 2s), the timeout fires and invokes `cleanupArtifacts()` a second time concurrently with the in-flight cleanup, then calls `process.exit(1)`. This can (a) race two cleanup runs against the same workspace-keyed artifacts/locks, and (b) override the intended `exitCode` with `1`, masking clean shutdowns as failures.
Comment thread
github-actions[bot] marked this conversation as resolved.
Outdated
};

const emitRequestGauges = (): void => {
Expand Down Expand Up @@ -384,32 +409,45 @@
idleCheckTimer.unref?.();
}

server.on('error', releaseStartupRegistryLock);

server.listen(socketPath, () => {
log('info', `[Daemon] Listening on ${socketPath}`);

// Write registry entry after successful listen
writeDaemonRegistryEntry({
workspaceKey,
workspaceRoot,
socketPath,
logPath: logPath ?? undefined,
pid: process.pid,
startedAt,
enabledWorkflows: daemonWorkflows,
version: String(version),
});
try {
writeDaemonRegistryEntry(
{
workspaceKey,
workspaceRoot,
socketPath,
logPath: logPath ?? undefined,
pid: process.pid,
startedAt,
enabledWorkflows: daemonWorkflows,
version: String(version),
},
{ lock: startupRegistryLock },
);
} finally {
releaseStartupRegistryLock();
}

writeLine(`Daemon started (PID: ${process.pid})`);
writeLine(`Workspace: ${workspaceRoot}`);
writeLine(`Socket: ${socketPath}`);
writeLine(`Tools: ${catalog.tools.length}`);
recordBootstrapDurationMetric('cli-daemon', Date.now() - daemonBootstrapStart);

// Filesystem orphan reconciliation and log retention run fire-and-forget after listen so
// a slow sweep cannot delay request serving. Request handlers must not assume orphans
// have been cleaned at startup.
setImmediate(() => {
void enrichSentryMetadata().catch((error) => {
const message = error instanceof Error ? error.message : String(error);
log('warn', `[Daemon] Failed to enrich Sentry metadata: ${message}`);
});
void runStartupLifecycleSweep();
});
});

Expand All @@ -421,7 +459,7 @@
};

process.on('exit', () => {
terminateLiveSimulatorLaunchOsLogSessionsSync();
terminateOwnedWorkspaceFilesystemArtifactsSync();
});
process.on('SIGTERM', () => shutdown(0));
process.on('SIGINT', () => shutdown(0));
Expand Down
Loading
Loading