Skip to content

Commit 38b57a7

Browse files
authored
feat: Workspace filesystem cleanup (#391)
Centralize workspace-scoped filesystem cleanup so log retention, daemon files, and simulator OSLog helpers are managed through multi-process-safe paths and locks. This keeps active workspace artifacts protected while pruning stale XcodeBuildMCP-owned files consistently.
1 parent b390867 commit 38b57a7

193 files changed

Lines changed: 5202 additions & 1022 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@ When reading issues:
8080
- When working on skill sources in `skills/`, use the `skill-creator` skill workflow.
8181
- After modifying any skill source, run `npx skill-check <skill-directory>` and address all errors/warnings before handoff.
8282
-
83+
## Multi-process filesystem state
84+
- XcodeBuildMCP explicitly supports multiple concurrent MCP server, daemon, CLI, test, and helper processes for the same or different workspaces.
85+
- Shared filesystem state under `~/Library/Developer/XcodeBuildMCP` must be multi-process safe.
86+
- Use workspace-key scoped directories for workspace-owned state.
87+
- Do not store runtime state under `~/.xcodebuildmcp`; `.xcodebuildmcp/config.yaml` is only project configuration.
88+
- Use shared lock and atomic-write helpers for mutable shared files.
89+
- Prefer one-record-per-file registries over shared aggregate files.
90+
- Cleanup must verify ownership before deleting shared artifacts.
91+
8392
## Style
8493
- Keep answers short and concise
8594
- No emojis in commits, issues, PR comments, or code

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111
- 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)).
1212
- Fixed Weather example test discovery and made CLI test progress visible while tests are running instead of leaving the last build phase displayed.
1313

14+
### Changed
15+
16+
- Centralized workspace log retention and startup/shutdown filesystem cleanup so XcodeBuildMCP-owned logs are pruned consistently while preserving active daemon and simulator OSLog outputs.
17+
1418
## [2.5.0-beta.1]
1519

1620
### Breaking

CLAUDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ When reading issues:
2121
- When working on skill sources in `skills/`, use the `skill-creator` skill workflow.
2222
- After modifying any skill source, run `npx skill-check <skill-directory>` and address all errors/warnings before handoff.
2323
-
24+
## Multi-process filesystem state
25+
- XcodeBuildMCP explicitly supports multiple concurrent MCP server, daemon, CLI, test, and helper processes for the same or different workspaces.
26+
- Shared filesystem state under `~/Library/Developer/XcodeBuildMCP` must be multi-process safe.
27+
- Use workspace-key scoped directories for workspace-owned state.
28+
- Do not store runtime state under `~/.xcodebuildmcp`; `.xcodebuildmcp/config.yaml` is only project configuration.
29+
- Use shared lock and atomic-write helpers for mutable shared files.
30+
- Prefer one-record-per-file registries over shared aggregate files.
31+
- Cleanup must verify ownership before deleting shared artifacts.
32+
2433
## Style
2534
- Keep answers short and concise
2635
- No emojis in commits, issues, PR comments, or code

src/cli.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22
import { bootstrapRuntime } from './runtime/bootstrap-runtime.ts';
33
import { buildCliToolCatalog } from './cli/cli-tool-catalog.ts';
44
import { buildYargsApp } from './cli/yargs-app.ts';
5-
import { getSocketPath, getWorkspaceKey, resolveWorkspaceRoot } from './daemon/socket-path.ts';
5+
import { getSocketPath } from './daemon/socket-path.ts';
66
import { startMcpServer } from './server/start-mcp-server.ts';
77
import { listCliWorkflowIdsFromManifest } from './runtime/tool-catalog.ts';
88
import { flushAndCloseSentry, initSentry, recordBootstrapDurationMetric } from './utils/sentry.ts';
99
import { coerceLogLevel, setLogLevel, type LogLevel } from './utils/logger.ts';
1010
import { hydrateSentryDisabledEnvFromProjectConfig } from './utils/sentry-config.ts';
11-
import { configureRuntimeWorkspaceKey } from './utils/runtime-instance.ts';
1211

1312
function findTopLevelCommand(argv: string[]): string | undefined {
1413
const flagsWithValue = new Set(['--socket', '--log-level', '--style']);
@@ -119,23 +118,13 @@ async function main(): Promise<void> {
119118
},
120119
});
121120

122-
// Compute workspace context for daemon routing
123-
const workspaceRoot = resolveWorkspaceRoot({
124-
cwd: result.runtime.cwd,
125-
projectConfigPath: result.configPath,
126-
});
121+
const { workspaceRoot, workspaceKey } = result;
127122

128123
const defaultSocketPath = getSocketPath({
129124
cwd: result.runtime.cwd,
130125
projectConfigPath: result.configPath,
131126
});
132127

133-
const workspaceKey = getWorkspaceKey({
134-
cwd: result.runtime.cwd,
135-
projectConfigPath: result.configPath,
136-
});
137-
configureRuntimeWorkspaceKey(workspaceKey);
138-
139128
const cliExposedWorkflowIds = await listCliWorkflowIdsFromManifest({
140129
excludeWorkflows: ['session-management', 'workflow-discovery'],
141130
});
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import type { DaemonRegistryEntry } from '../../daemon/daemon-registry.ts';
3+
4+
const originalEntry: DaemonRegistryEntry = {
5+
workspaceKey: 'workspace-a',
6+
workspaceRoot: '/workspaces/workspace-a',
7+
socketPath: '/tmp/xcodebuildmcp-daemon.sock',
8+
pid: 123_456,
9+
startedAt: '2026-05-05T00:00:00.000Z',
10+
enabledWorkflows: ['build'],
11+
version: '1.0.0',
12+
instanceId: 'daemon-instance-a',
13+
};
14+
15+
const changedEntry: DaemonRegistryEntry = {
16+
...originalEntry,
17+
instanceId: 'daemon-instance-b',
18+
};
19+
20+
const registryMocks = vi.hoisted(() => ({
21+
cleanupWorkspaceDaemonFiles: vi.fn(),
22+
findDaemonRegistryEntryBySocketPath: vi.fn(),
23+
isPidAlive: vi.fn(),
24+
readDaemonRegistryEntry: vi.fn(),
25+
release: vi.fn(),
26+
}));
27+
28+
vi.mock('../../daemon/daemon-registry.ts', () => ({
29+
acquireDaemonRegistryMutationLock: vi.fn(() => ({
30+
workspaceKey: 'workspace-a',
31+
release: registryMocks.release,
32+
})),
33+
cleanupWorkspaceDaemonFiles: registryMocks.cleanupWorkspaceDaemonFiles,
34+
findDaemonRegistryEntryBySocketPath: registryMocks.findDaemonRegistryEntryBySocketPath,
35+
readDaemonRegistryEntry: registryMocks.readDaemonRegistryEntry,
36+
}));
37+
38+
vi.mock('../../utils/process-liveness.ts', () => ({
39+
isPidAlive: registryMocks.isPidAlive,
40+
}));
41+
42+
import { forceStopDaemon } from '../daemon-control.ts';
43+
44+
describe('daemon control force-stop registry races', () => {
45+
beforeEach(() => {
46+
registryMocks.cleanupWorkspaceDaemonFiles.mockReset();
47+
registryMocks.findDaemonRegistryEntryBySocketPath.mockReset();
48+
registryMocks.isPidAlive.mockReset();
49+
registryMocks.readDaemonRegistryEntry.mockReset();
50+
registryMocks.release.mockReset();
51+
});
52+
53+
afterEach(() => {
54+
vi.restoreAllMocks();
55+
});
56+
57+
it('sends the initial signal before releasing the registry mutation lock', async () => {
58+
registryMocks.findDaemonRegistryEntryBySocketPath.mockReturnValue(originalEntry);
59+
registryMocks.readDaemonRegistryEntry.mockReturnValue(originalEntry);
60+
registryMocks.isPidAlive.mockReturnValue(false);
61+
const kill = vi.spyOn(process, 'kill').mockImplementation(() => {
62+
expect(registryMocks.release).not.toHaveBeenCalled();
63+
return true;
64+
});
65+
66+
await forceStopDaemon(originalEntry.socketPath);
67+
68+
expect(kill).toHaveBeenCalledWith(originalEntry.pid, 'SIGTERM');
69+
expect(registryMocks.release).toHaveBeenCalledOnce();
70+
expect(registryMocks.cleanupWorkspaceDaemonFiles).toHaveBeenCalledWith(
71+
originalEntry.workspaceKey,
72+
{
73+
pid: originalEntry.pid,
74+
socketPath: originalEntry.socketPath,
75+
instanceId: originalEntry.instanceId,
76+
allowLiveOwner: true,
77+
},
78+
);
79+
});
80+
81+
it('does not signal when daemon metadata changes before the initial signal', async () => {
82+
registryMocks.findDaemonRegistryEntryBySocketPath.mockReturnValue(originalEntry);
83+
registryMocks.readDaemonRegistryEntry.mockReturnValue(changedEntry);
84+
const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
85+
86+
await expect(forceStopDaemon(originalEntry.socketPath)).rejects.toThrow(
87+
'daemon registry metadata changed',
88+
);
89+
90+
expect(kill).not.toHaveBeenCalled();
91+
expect(registryMocks.cleanupWorkspaceDaemonFiles).not.toHaveBeenCalled();
92+
expect(registryMocks.release).toHaveBeenCalledOnce();
93+
});
94+
});
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import * as path from 'node:path';
5+
import { forceStopDaemon } from '../daemon-control.ts';
6+
import {
7+
readDaemonRegistryEntry,
8+
type DaemonRegistryEntry,
9+
writeDaemonRegistryEntry,
10+
} from '../../daemon/daemon-registry.ts';
11+
import {
12+
daemonDirForWorkspaceKey,
13+
setDaemonRunDirOverrideForTests,
14+
} from '../../daemon/socket-path.ts';
15+
import { setXcodeBuildMCPAppDirOverrideForTests } from '../../utils/log-paths.ts';
16+
17+
const daemonPid = 123_456;
18+
19+
function createMissingPidError(): NodeJS.ErrnoException {
20+
const error = new Error('no such process') as NodeJS.ErrnoException;
21+
error.code = 'ESRCH';
22+
return error;
23+
}
24+
25+
function createEntry(overrides: Partial<DaemonRegistryEntry> = {}): DaemonRegistryEntry {
26+
const workspaceKey = overrides.workspaceKey ?? 'workspace-a';
27+
return {
28+
workspaceKey,
29+
workspaceRoot: `/workspaces/${workspaceKey}`,
30+
socketPath: path.join(daemonDirForWorkspaceKey(workspaceKey), 'd.sock'),
31+
pid: daemonPid,
32+
startedAt: '2026-05-05T00:00:00.000Z',
33+
enabledWorkflows: ['build'],
34+
version: '1.0.0',
35+
instanceId: 'daemon-instance-a',
36+
...overrides,
37+
};
38+
}
39+
40+
describe('daemon control', () => {
41+
let appDir: string;
42+
let daemonRunDir: string;
43+
44+
beforeEach(() => {
45+
appDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-daemon-control-app-'));
46+
daemonRunDir = mkdtempSync(path.join(tmpdir(), 'xcodebuildmcp-daemon-control-run-'));
47+
setXcodeBuildMCPAppDirOverrideForTests(appDir);
48+
setDaemonRunDirOverrideForTests(daemonRunDir);
49+
});
50+
51+
afterEach(() => {
52+
vi.useRealTimers();
53+
vi.restoreAllMocks();
54+
setXcodeBuildMCPAppDirOverrideForTests(null);
55+
setDaemonRunDirOverrideForTests(null);
56+
rmSync(appDir, { recursive: true, force: true });
57+
rmSync(daemonRunDir, { recursive: true, force: true });
58+
});
59+
60+
it('does not unlink sockets without registry metadata', async () => {
61+
const socketPath = path.join(daemonRunDir, 'missing-registry.sock');
62+
mkdirSync(path.dirname(socketPath), { recursive: true, mode: 0o700 });
63+
writeFileSync(socketPath, 'socket placeholder');
64+
65+
await expect(forceStopDaemon(socketPath)).rejects.toThrow('registry metadata is missing');
66+
67+
expect(existsSync(socketPath)).toBe(true);
68+
});
69+
70+
it('unregisters daemon files only after SIGTERM stops the process', async () => {
71+
const entry = createEntry();
72+
writeDaemonRegistryEntry(entry);
73+
mkdirSync(path.dirname(entry.socketPath), { recursive: true, mode: 0o700 });
74+
writeFileSync(entry.socketPath, 'socket placeholder');
75+
let alive = true;
76+
const kill = vi.spyOn(process, 'kill').mockImplementation(((
77+
_pid: number,
78+
signal?: string | number,
79+
) => {
80+
if (signal === 0) {
81+
if (alive) {
82+
return true;
83+
}
84+
throw createMissingPidError();
85+
}
86+
if (signal === 'SIGTERM') {
87+
alive = false;
88+
}
89+
return true;
90+
}) as typeof process.kill);
91+
92+
await forceStopDaemon(entry.socketPath);
93+
94+
expect(kill).toHaveBeenCalledWith(entry.pid, 'SIGTERM');
95+
expect(readDaemonRegistryEntry(entry.workspaceKey)).toBeNull();
96+
expect(existsSync(entry.socketPath)).toBe(false);
97+
});
98+
99+
it('cleans daemon files when the stopped PID is reused before cleanup', async () => {
100+
const entry = createEntry();
101+
writeDaemonRegistryEntry(entry);
102+
mkdirSync(path.dirname(entry.socketPath), { recursive: true, mode: 0o700 });
103+
writeFileSync(entry.socketPath, 'socket placeholder');
104+
let zeroSignalChecksAfterTerm = 0;
105+
const kill = vi.spyOn(process, 'kill').mockImplementation(((
106+
_pid: number,
107+
signal?: string | number,
108+
) => {
109+
if (signal === 0) {
110+
zeroSignalChecksAfterTerm += 1;
111+
if (zeroSignalChecksAfterTerm === 1) {
112+
throw createMissingPidError();
113+
}
114+
return true;
115+
}
116+
return true;
117+
}) as typeof process.kill);
118+
119+
await forceStopDaemon(entry.socketPath);
120+
121+
expect(kill).toHaveBeenCalledWith(entry.pid, 'SIGTERM');
122+
expect(readDaemonRegistryEntry(entry.workspaceKey)).toBeNull();
123+
expect(existsSync(entry.socketPath)).toBe(false);
124+
});
125+
126+
it('uses SIGKILL when the process stays alive after SIGTERM', async () => {
127+
vi.useFakeTimers();
128+
const entry = createEntry();
129+
writeDaemonRegistryEntry(entry);
130+
mkdirSync(path.dirname(entry.socketPath), { recursive: true, mode: 0o700 });
131+
writeFileSync(entry.socketPath, 'socket placeholder');
132+
let alive = true;
133+
const kill = vi.spyOn(process, 'kill').mockImplementation(((
134+
_pid: number,
135+
signal?: string | number,
136+
) => {
137+
if (signal === 0) {
138+
if (alive) {
139+
return true;
140+
}
141+
throw createMissingPidError();
142+
}
143+
if (signal === 'SIGKILL') {
144+
alive = false;
145+
}
146+
return true;
147+
}) as typeof process.kill);
148+
149+
const stopped = forceStopDaemon(entry.socketPath);
150+
await vi.advanceTimersByTimeAsync(1500);
151+
await stopped;
152+
153+
expect(kill).toHaveBeenCalledWith(entry.pid, 'SIGTERM');
154+
expect(kill).toHaveBeenCalledWith(entry.pid, 'SIGKILL');
155+
expect(readDaemonRegistryEntry(entry.workspaceKey)).toBeNull();
156+
expect(existsSync(entry.socketPath)).toBe(false);
157+
});
158+
159+
it('preserves registry metadata and socket when the process remains alive', async () => {
160+
vi.useFakeTimers();
161+
const entry = createEntry();
162+
writeDaemonRegistryEntry(entry);
163+
mkdirSync(path.dirname(entry.socketPath), { recursive: true, mode: 0o700 });
164+
writeFileSync(entry.socketPath, 'socket placeholder');
165+
vi.spyOn(process, 'kill').mockImplementation((() => true) as typeof process.kill);
166+
167+
const stopped = forceStopDaemon(entry.socketPath);
168+
const stoppedExpectation = expect(stopped).rejects.toThrow(
169+
`Daemon PID ${entry.pid} did not exit after SIGKILL`,
170+
);
171+
await vi.advanceTimersByTimeAsync(3000);
172+
173+
await stoppedExpectation;
174+
expect(readDaemonRegistryEntry(entry.workspaceKey)).toEqual(entry);
175+
expect(existsSync(entry.socketPath)).toBe(true);
176+
});
177+
});

0 commit comments

Comments
 (0)