Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 silently leaves stale socket and registry when SIGTERM target hasn't exited within 500ms

After sending SIGTERM and waiting 500ms, forceStopDaemon calls cleanupWorkspaceDaemonFiles with pid set but without allowLiveOwner=true. In canRemoveRegistryEntry, when allowLiveOwner is not true the function returns !isPidAlive(entry.pid); if the daemon hasn't yet exited (500ms is short for a graceful SIGTERM shutdown), removeRegistryAtPathIfOwned returns null and neither the registry entry nor the socket is removed. The previous implementation unconditionally called removeStaleSocket(socketPath), so this is a behavioral regression: ensureDaemonRunning will then proceed to startDaemonBackground while the old socket file still exists, which can cause the new daemon's bind() to fail with EADDRINUSE and leave the workspace unable to start a daemon.
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.
}
}

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

View check run for this annotation

@sentry/warden / warden: xcodebuildmcp-runtime-boundary-review

forceStopDaemon bypasses centralized cleanup with a direct unlinkSync fallback

When no registry entry is found for the socket path, forceStopDaemon falls back to unlinkSync(socketPath) directly instead of going through cleanupWorkspaceDaemonFiles. This creates a parallel cleanup path that skips the workspace-keyed mutation lock and ownership/PID checks that the rest of this PR introduces, and silently swallows the error. Per the runtime boundary guardrails ("Avoid silent fallbacks and parallel invocation paths"), this fallback can race with another live process owning the same socket path and remove its socket without an ownership check.
}
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;
};
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

Forced-shutdown timer can hang indefinitely waiting on async cleanup

The 5s forced-shutdown timer previously called the synchronous cleanupWorkspaceDaemonFiles and then exited. It now awaits the async cleanupOwnedWorkspaceFilesystemArtifacts via .finally() with no timeout. If that promise hangs (e.g., a stuck filesystem lock acquired in fs-lock-shared / workspace-filesystem-lifecycle), the daemon will never exit, defeating the purpose of the forced-shutdown path and causing process leaks across daemon restarts.

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 duplicate cleanup and exit-code race

When `shutdown()` runs, the 5-second `setTimeout` is scheduled but never cleared once `server.close()` succeeds. If cleanup or `flushAndCloseSentry(2000)` together take longer than 5 seconds (the sentry flush alone allows up to 2s, and `stopOwnedSimulatorLaunchOsLogSessions` defaults to 1s plus daemon-file cleanup), the timeout fires and runs `cleanupArtifacts()` a second time concurrently with the in-progress cleanup, then races to call `process.exit(1)` against the success path's `process.exit(exitCode)`. This can corrupt the daemon-file/socket cleanup (two concurrent `cleanupWorkspaceDaemonFiles` and `stopOwnedSimulatorLaunchOsLogSessions` calls) and override the intended exit code with 1.

Check warning on line 351 in src/daemon.ts

View check run for this annotation

@sentry/warden / warden: find-bugs

[WTC-ADE] Forced-shutdown timeout is never cleared, causing duplicate cleanup and exit-code race (additional location)

When `shutdown()` runs, the 5-second `setTimeout` is scheduled but never cleared once `server.close()` succeeds. If cleanup or `flushAndCloseSentry(2000)` together take longer than 5 seconds (the sentry flush alone allows up to 2s, and `stopOwnedSimulatorLaunchOsLogSessions` defaults to 1s plus daemon-file cleanup), the timeout fires and runs `cleanupArtifacts()` a second time concurrently with the in-progress cleanup, then races to call `process.exit(1)` against the success path's `process.exit(exitCode)`. This can corrupt the daemon-file/socket cleanup (two concurrent `cleanupWorkspaceDaemonFiles` and `stopOwnedSimulatorLaunchOsLogSessions` calls) and override the intended exit code with 1.
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