Skip to content

Commit 2209c90

Browse files
cameroncookecodex
andcommitted
fix(simulator): Reconcile simulatorId when name and id are both set
Refresh simulator defaults now validates simulatorName even when simulatorId is already set. When the resolved id differs, session defaults are patched in memory only so machine-specific UUID drift does not break simulator commands. Add focused tests for the both-set behavior (match, mismatch, and lookup failure) and document the fix in the unreleased changelog. Fixes #357 Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent 54837d4 commit 2209c90

3 files changed

Lines changed: 157 additions & 0 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+
- Fixed simulator defaults refresh to reconcile stale `simulatorId` values in memory when both `simulatorId` and `simulatorName` are configured, while still avoiding config write-back churn across contributors ([#357](https://github.com/getsentry/XcodeBuildMCP/issues/357)).
2627

2728
## [2.3.2]
2829

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const {
4+
persistSessionDefaultsPatchMock,
5+
resolveSimulatorNameToIdMock,
6+
resolveSimulatorIdToNameMock,
7+
inferPlatformMock,
8+
logMock,
9+
} = vi.hoisted(() => ({
10+
persistSessionDefaultsPatchMock: vi.fn(),
11+
resolveSimulatorNameToIdMock: vi.fn(),
12+
resolveSimulatorIdToNameMock: vi.fn(),
13+
inferPlatformMock: vi.fn(),
14+
logMock: vi.fn(),
15+
}));
16+
17+
vi.mock('../config-store.ts', () => ({
18+
persistSessionDefaultsPatch: persistSessionDefaultsPatchMock,
19+
}));
20+
21+
vi.mock('../simulator-resolver.ts', () => ({
22+
resolveSimulatorNameToId: resolveSimulatorNameToIdMock,
23+
resolveSimulatorIdToName: resolveSimulatorIdToNameMock,
24+
}));
25+
26+
vi.mock('../infer-platform.ts', () => ({
27+
inferPlatform: inferPlatformMock,
28+
}));
29+
30+
vi.mock('../logger.ts', () => ({
31+
log: logMock,
32+
}));
33+
34+
import { sessionStore } from '../session-store.ts';
35+
import { scheduleSimulatorDefaultsRefresh } from '../simulator-defaults-refresh.ts';
36+
37+
describe('scheduleSimulatorDefaultsRefresh', () => {
38+
const originalNodeEnv = process.env.NODE_ENV;
39+
const originalVitestEnv = process.env.VITEST;
40+
41+
beforeEach(() => {
42+
sessionStore.clearAll();
43+
persistSessionDefaultsPatchMock.mockReset();
44+
resolveSimulatorNameToIdMock.mockReset();
45+
resolveSimulatorIdToNameMock.mockReset();
46+
inferPlatformMock.mockReset();
47+
logMock.mockReset();
48+
49+
process.env.NODE_ENV = 'development';
50+
delete process.env.VITEST;
51+
52+
inferPlatformMock.mockResolvedValue({
53+
platform: 'iOS Simulator',
54+
source: 'simulator-runtime',
55+
});
56+
});
57+
58+
afterEach(() => {
59+
process.env.NODE_ENV = originalNodeEnv;
60+
if (originalVitestEnv == null) {
61+
delete process.env.VITEST;
62+
} else {
63+
process.env.VITEST = originalVitestEnv;
64+
}
65+
66+
vi.useRealTimers();
67+
});
68+
69+
async function runRefresh(options: { simulatorId: string; simulatorName: string }) {
70+
vi.useFakeTimers();
71+
72+
sessionStore.setDefaults({
73+
simulatorId: options.simulatorId,
74+
simulatorName: options.simulatorName,
75+
});
76+
const expectedRevision = sessionStore.getRevision();
77+
78+
const scheduled = scheduleSimulatorDefaultsRefresh({
79+
expectedRevision,
80+
reason: 'startup-hydration',
81+
profile: null,
82+
persist: false,
83+
simulatorId: options.simulatorId,
84+
simulatorName: options.simulatorName,
85+
});
86+
87+
expect(scheduled).toBe(true);
88+
await vi.runAllTimersAsync();
89+
}
90+
91+
it('does not patch defaults when both values are set and name resolves to same id', async () => {
92+
resolveSimulatorNameToIdMock.mockResolvedValue({
93+
success: true,
94+
simulatorId: 'SIM-1',
95+
simulatorName: 'iPhone 17 Pro',
96+
});
97+
inferPlatformMock.mockResolvedValue({
98+
platform: 'iOS Simulator',
99+
source: 'default',
100+
});
101+
102+
await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' });
103+
104+
expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1);
105+
expect(sessionStore.getAll()).toEqual({
106+
simulatorId: 'SIM-1',
107+
simulatorName: 'iPhone 17 Pro',
108+
});
109+
expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled();
110+
});
111+
112+
it('patches simulatorId in memory when both are set and name resolves to a different id', async () => {
113+
resolveSimulatorNameToIdMock.mockResolvedValue({
114+
success: true,
115+
simulatorId: 'SIM-2',
116+
simulatorName: 'iPhone 17 Pro',
117+
});
118+
119+
await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' });
120+
121+
expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1);
122+
expect(sessionStore.getAll()).toEqual({
123+
simulatorId: 'SIM-2',
124+
simulatorName: 'iPhone 17 Pro',
125+
simulatorPlatform: 'iOS Simulator',
126+
});
127+
expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled();
128+
});
129+
130+
it('keeps the existing simulatorId when name lookup fails and logs a warning', async () => {
131+
resolveSimulatorNameToIdMock.mockRejectedValue(new Error('simctl failed'));
132+
133+
await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' });
134+
135+
expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1);
136+
expect(sessionStore.getAll()).toEqual({
137+
simulatorId: 'SIM-1',
138+
simulatorName: 'iPhone 17 Pro',
139+
});
140+
expect(logMock).toHaveBeenCalledWith(
141+
'warn',
142+
expect.stringContaining(
143+
'Background simulator defaults refresh failed (startup-hydration): Error: simctl failed',
144+
),
145+
);
146+
expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled();
147+
});
148+
});

src/utils/simulator-defaults-refresh.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ async function refreshSimulatorDefaults(
5858
}
5959
}
6060

61+
if (simulatorId && simulatorName) {
62+
const resolution = await resolveSimulatorNameToId(executor, simulatorName);
63+
if (resolution.success && resolution.simulatorId !== simulatorId) {
64+
simulatorId = resolution.simulatorId;
65+
patch.simulatorId = resolution.simulatorId;
66+
}
67+
}
68+
6169
if (!simulatorName && simulatorId) {
6270
const resolution = await resolveSimulatorIdToName(executor, simulatorId);
6371
if (resolution.success) {

0 commit comments

Comments
 (0)