Skip to content

Commit 9a57de8

Browse files
cameroncookecodex
andcommitted
fix(utils): Replace sync device name resolver calls
Convert device-name-resolver to use async child process and fs operations so resolving display names no longer blocks the Node event loop. Keep the public resolver API synchronous by triggering async refresh in background and serving cached values when available. Add focused tests for async loading, cache population, and in-flight deduplication. Fixes #333 Co-Authored-By: OpenAI Codex <codex@openai.com>
1 parent 54837d4 commit 9a57de8

3 files changed

Lines changed: 175 additions & 36 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
- Added `toggle_connect_hardware_keyboard` tool to toggle the iOS Simulator hardware keyboard connection ([#346](https://github.com/getsentry/XcodeBuildMCP/issues/346)).
2424
- Fixed `xcode_tools_bridge_disconnect` immediately re-syncing proxied tools after a manual disconnect ([#343](https://github.com/getsentry/XcodeBuildMCP/issues/343)).
2525
- Stopped suggesting an unsupported `--device-id`/`deviceId` argument in the `device list` next-step hint for `device build`/`build_device`; device targeting flows through session defaults ([#350](https://github.com/getsentry/XcodeBuildMCP/pull/350) by [@MukundaKatta](https://github.com/MukundaKatta)).
26+
- Replaced blocking `execSync`/`readFileSync`/`unlinkSync` in the device name resolver with async process/file operations and background cache refresh, avoiding event-loop blocking during device name formatting ([#333](https://github.com/getsentry/XcodeBuildMCP/issues/333)).
2627

2728
## [2.3.2]
2829

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const { execFileMock, readFileMock, unlinkMock } = vi.hoisted(() => ({
4+
execFileMock: vi.fn(),
5+
readFileMock: vi.fn(),
6+
unlinkMock: vi.fn(),
7+
}));
8+
9+
vi.mock('node:child_process', () => ({
10+
execFile: execFileMock,
11+
}));
12+
13+
vi.mock('node:fs/promises', () => ({
14+
readFile: readFileMock,
15+
unlink: unlinkMock,
16+
}));
17+
18+
async function flushAsyncWork(): Promise<void> {
19+
await Promise.resolve();
20+
await new Promise((resolve) => setTimeout(resolve, 0));
21+
}
22+
23+
describe('device-name-resolver', () => {
24+
beforeEach(() => {
25+
vi.clearAllMocks();
26+
vi.resetModules();
27+
});
28+
29+
afterEach(() => {
30+
vi.useRealTimers();
31+
});
32+
33+
it('loads device names asynchronously and caches resolved names', async () => {
34+
execFileMock.mockImplementation(
35+
(
36+
_file: string,
37+
_args: string[],
38+
_options: unknown,
39+
callback: (error: Error | null, stdout: string, stderr: string) => void,
40+
) => {
41+
callback(null, '', '');
42+
return {};
43+
},
44+
);
45+
46+
readFileMock.mockResolvedValue(
47+
JSON.stringify({
48+
result: {
49+
devices: [
50+
{
51+
identifier: 'device-1',
52+
deviceProperties: { name: 'iPhone 15 Pro' },
53+
hardwareProperties: { udid: 'udid-1' },
54+
},
55+
],
56+
},
57+
}),
58+
);
59+
unlinkMock.mockResolvedValue(undefined);
60+
61+
const { resolveDeviceName, formatDeviceId } = await import('../device-name-resolver.ts');
62+
63+
expect(resolveDeviceName('device-1')).toBeUndefined();
64+
expect(execFileMock).toHaveBeenCalledTimes(1);
65+
66+
await flushAsyncWork();
67+
68+
expect(resolveDeviceName('device-1')).toBe('iPhone 15 Pro');
69+
expect(resolveDeviceName('udid-1')).toBe('iPhone 15 Pro');
70+
expect(formatDeviceId('device-1')).toBe('iPhone 15 Pro (device-1)');
71+
expect(readFileMock).toHaveBeenCalledTimes(1);
72+
expect(unlinkMock).toHaveBeenCalledTimes(1);
73+
});
74+
75+
it('does not spawn duplicate refreshes while one is in flight', async () => {
76+
let callback: ((error: Error | null, stdout: string, stderr: string) => void) | undefined;
77+
78+
execFileMock.mockImplementation(
79+
(
80+
_file: string,
81+
_args: string[],
82+
_options: unknown,
83+
cb: (error: Error | null, stdout: string, stderr: string) => void,
84+
) => {
85+
callback = cb;
86+
return {};
87+
},
88+
);
89+
90+
readFileMock.mockResolvedValue(
91+
JSON.stringify({
92+
result: { devices: [{ identifier: 'device-2', deviceProperties: { name: 'iPad' } }] },
93+
}),
94+
);
95+
unlinkMock.mockResolvedValue(undefined);
96+
97+
const { resolveDeviceName } = await import('../device-name-resolver.ts');
98+
99+
expect(resolveDeviceName('device-2')).toBeUndefined();
100+
expect(resolveDeviceName('device-2')).toBeUndefined();
101+
expect(execFileMock).toHaveBeenCalledTimes(1);
102+
103+
callback?.(null, '', '');
104+
await flushAsyncWork();
105+
106+
expect(resolveDeviceName('device-2')).toBe('iPad');
107+
});
108+
});

src/utils/device-name-resolver.ts

Lines changed: 66 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,93 @@
1-
import { execSync } from 'node:child_process';
2-
import { readFileSync, unlinkSync } from 'node:fs';
1+
import { execFile } from 'node:child_process';
2+
import { readFile, unlink } from 'node:fs/promises';
33
import { tmpdir } from 'node:os';
44
import { join } from 'node:path';
5+
import { promisify } from 'node:util';
56

67
const CACHE_TTL_MS = 30_000;
8+
const execFileAsync = promisify(execFile);
79

810
let cachedDevices: Map<string, string> | null = null;
911
let cacheTimestamp = 0;
12+
let loadPromise: Promise<void> | null = null;
1013

1114
interface DeviceCtlEntry {
1215
identifier: string;
1316
deviceProperties: { name: string };
1417
hardwareProperties?: { udid?: string };
1518
}
1619

17-
function loadDeviceNames(): Map<string, string> {
18-
if (cachedDevices && Date.now() - cacheTimestamp < CACHE_TTL_MS) {
19-
return cachedDevices;
20-
}
20+
function cacheIsFresh(): boolean {
21+
return cachedDevices !== null && Date.now() - cacheTimestamp < CACHE_TTL_MS;
22+
}
2123

24+
function createDeviceMap(data: { result?: { devices?: DeviceCtlEntry[] } }): Map<string, string> {
2225
const map = new Map<string, string>();
23-
const tmpFile = join(tmpdir(), `devicectl-list-${process.pid}.json`);
24-
25-
try {
26-
execSync(`xcrun devicectl list devices --json-output ${tmpFile}`, {
27-
encoding: 'utf8',
28-
timeout: 10_000,
29-
stdio: 'pipe',
30-
});
31-
32-
const data = JSON.parse(readFileSync(tmpFile, 'utf8')) as {
33-
result?: { devices?: DeviceCtlEntry[] };
34-
};
35-
36-
for (const device of data.result?.devices ?? []) {
37-
const name = device.deviceProperties.name;
38-
map.set(device.identifier, name);
39-
if (device.hardwareProperties?.udid) {
40-
map.set(device.hardwareProperties.udid, name);
41-
}
26+
27+
for (const device of data.result?.devices ?? []) {
28+
const name = device.deviceProperties.name;
29+
map.set(device.identifier, name);
30+
if (device.hardwareProperties?.udid) {
31+
map.set(device.hardwareProperties.udid, name);
4232
}
43-
} catch {
44-
// Device list unavailable -- return empty map, will fall back to UUID only
45-
} finally {
33+
}
34+
35+
return map;
36+
}
37+
38+
async function refreshDeviceNames(): Promise<void> {
39+
if (cacheIsFresh()) {
40+
return;
41+
}
42+
43+
if (loadPromise) {
44+
return loadPromise;
45+
}
46+
47+
const tmpFile = join(tmpdir(), `devicectl-list-${process.pid}-${Date.now()}.json`);
48+
49+
loadPromise = (async () => {
4650
try {
47-
unlinkSync(tmpFile);
51+
await execFileAsync('xcrun', ['devicectl', 'list', 'devices', '--json-output', tmpFile], {
52+
encoding: 'utf8',
53+
timeout: 10_000,
54+
});
55+
56+
const data = JSON.parse(await readFile(tmpFile, 'utf8')) as {
57+
result?: { devices?: DeviceCtlEntry[] };
58+
};
59+
60+
cachedDevices = createDeviceMap(data);
61+
cacheTimestamp = Date.now();
4862
} catch {
49-
// ignore
63+
// Device list unavailable -- keep existing cache and fall back to UUID only
64+
if (cachedDevices === null) {
65+
cachedDevices = new Map();
66+
cacheTimestamp = Date.now();
67+
}
68+
} finally {
69+
loadPromise = null;
70+
try {
71+
await unlink(tmpFile);
72+
} catch {
73+
// ignore
74+
}
5075
}
51-
}
76+
})();
5277

53-
cachedDevices = map;
54-
cacheTimestamp = Date.now();
55-
return map;
78+
return loadPromise;
79+
}
80+
81+
function ensureDeviceNamesRefresh(): void {
82+
void refreshDeviceNames();
5683
}
5784

5885
export function resolveDeviceName(deviceId: string): string | undefined {
59-
const names = loadDeviceNames();
60-
return names.get(deviceId);
86+
if (!cacheIsFresh()) {
87+
ensureDeviceNamesRefresh();
88+
}
89+
90+
return cachedDevices?.get(deviceId);
6191
}
6292

6393
export function formatDeviceId(deviceId: string): string {

0 commit comments

Comments
 (0)