From 2209c90cd5deed4a571183383021d9f4cd68ffb8 Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Tue, 28 Apr 2026 22:46:53 +0100 Subject: [PATCH 1/2] 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 --- CHANGELOG.md | 1 + .../simulator-defaults-refresh.test.ts | 148 ++++++++++++++++++ src/utils/simulator-defaults-refresh.ts | 8 + 3 files changed, 157 insertions(+) create mode 100644 src/utils/__tests__/simulator-defaults-refresh.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d22b0d3..69885cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Added `toggle_connect_hardware_keyboard` tool to toggle the iOS Simulator hardware keyboard connection ([#346](https://github.com/getsentry/XcodeBuildMCP/issues/346)). - Fixed `xcode_tools_bridge_disconnect` immediately re-syncing proxied tools after a manual disconnect ([#343](https://github.com/getsentry/XcodeBuildMCP/issues/343)). - 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)). +- 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)). ## [2.3.2] diff --git a/src/utils/__tests__/simulator-defaults-refresh.test.ts b/src/utils/__tests__/simulator-defaults-refresh.test.ts new file mode 100644 index 00000000..b90ee359 --- /dev/null +++ b/src/utils/__tests__/simulator-defaults-refresh.test.ts @@ -0,0 +1,148 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + persistSessionDefaultsPatchMock, + resolveSimulatorNameToIdMock, + resolveSimulatorIdToNameMock, + inferPlatformMock, + logMock, +} = vi.hoisted(() => ({ + persistSessionDefaultsPatchMock: vi.fn(), + resolveSimulatorNameToIdMock: vi.fn(), + resolveSimulatorIdToNameMock: vi.fn(), + inferPlatformMock: vi.fn(), + logMock: vi.fn(), +})); + +vi.mock('../config-store.ts', () => ({ + persistSessionDefaultsPatch: persistSessionDefaultsPatchMock, +})); + +vi.mock('../simulator-resolver.ts', () => ({ + resolveSimulatorNameToId: resolveSimulatorNameToIdMock, + resolveSimulatorIdToName: resolveSimulatorIdToNameMock, +})); + +vi.mock('../infer-platform.ts', () => ({ + inferPlatform: inferPlatformMock, +})); + +vi.mock('../logger.ts', () => ({ + log: logMock, +})); + +import { sessionStore } from '../session-store.ts'; +import { scheduleSimulatorDefaultsRefresh } from '../simulator-defaults-refresh.ts'; + +describe('scheduleSimulatorDefaultsRefresh', () => { + const originalNodeEnv = process.env.NODE_ENV; + const originalVitestEnv = process.env.VITEST; + + beforeEach(() => { + sessionStore.clearAll(); + persistSessionDefaultsPatchMock.mockReset(); + resolveSimulatorNameToIdMock.mockReset(); + resolveSimulatorIdToNameMock.mockReset(); + inferPlatformMock.mockReset(); + logMock.mockReset(); + + process.env.NODE_ENV = 'development'; + delete process.env.VITEST; + + inferPlatformMock.mockResolvedValue({ + platform: 'iOS Simulator', + source: 'simulator-runtime', + }); + }); + + afterEach(() => { + process.env.NODE_ENV = originalNodeEnv; + if (originalVitestEnv == null) { + delete process.env.VITEST; + } else { + process.env.VITEST = originalVitestEnv; + } + + vi.useRealTimers(); + }); + + async function runRefresh(options: { simulatorId: string; simulatorName: string }) { + vi.useFakeTimers(); + + sessionStore.setDefaults({ + simulatorId: options.simulatorId, + simulatorName: options.simulatorName, + }); + const expectedRevision = sessionStore.getRevision(); + + const scheduled = scheduleSimulatorDefaultsRefresh({ + expectedRevision, + reason: 'startup-hydration', + profile: null, + persist: false, + simulatorId: options.simulatorId, + simulatorName: options.simulatorName, + }); + + expect(scheduled).toBe(true); + await vi.runAllTimersAsync(); + } + + it('does not patch defaults when both values are set and name resolves to same id', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: true, + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + }); + inferPlatformMock.mockResolvedValue({ + platform: 'iOS Simulator', + source: 'default', + }); + + await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); + + expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1); + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + }); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + + it('patches simulatorId in memory when both are set and name resolves to a different id', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: true, + simulatorId: 'SIM-2', + simulatorName: 'iPhone 17 Pro', + }); + + await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); + + expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1); + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-2', + simulatorName: 'iPhone 17 Pro', + simulatorPlatform: 'iOS Simulator', + }); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + + it('keeps the existing simulatorId when name lookup fails and logs a warning', async () => { + resolveSimulatorNameToIdMock.mockRejectedValue(new Error('simctl failed')); + + await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); + + expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1); + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + }); + expect(logMock).toHaveBeenCalledWith( + 'warn', + expect.stringContaining( + 'Background simulator defaults refresh failed (startup-hydration): Error: simctl failed', + ), + ); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/utils/simulator-defaults-refresh.ts b/src/utils/simulator-defaults-refresh.ts index 1cbad03a..57a27276 100644 --- a/src/utils/simulator-defaults-refresh.ts +++ b/src/utils/simulator-defaults-refresh.ts @@ -58,6 +58,14 @@ async function refreshSimulatorDefaults( } } + if (simulatorId && simulatorName) { + const resolution = await resolveSimulatorNameToId(executor, simulatorName); + if (resolution.success && resolution.simulatorId !== simulatorId) { + simulatorId = resolution.simulatorId; + patch.simulatorId = resolution.simulatorId; + } + } + if (!simulatorName && simulatorId) { const resolution = await resolveSimulatorIdToName(executor, simulatorId); if (resolution.success) { From a5f286009cb4da4523784ecda9b9ab04953c8f78 Mon Sep 17 00:00:00 2001 From: Cameron Cooke Date: Wed, 29 Apr 2026 09:41:28 +0100 Subject: [PATCH 2/2] fix(simulator): Avoid duplicate simulator name lookup Resolve simulator IDs from simulatorName in a single branch so name-only defaults do not fall through into the both-set reconciliation path. Keep simulatorName as the source of truth when present and only patch simulatorId when the resolved value differs. Add regression coverage for the name-only refresh path to ensure it performs one resolver call. Refs #357 Co-Authored-By: OpenAI Codex --- .../simulator-defaults-refresh.test.ts | 30 +++++++++++++++---- src/utils/simulator-defaults-refresh.ts | 14 ++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/utils/__tests__/simulator-defaults-refresh.test.ts b/src/utils/__tests__/simulator-defaults-refresh.test.ts index b90ee359..bb39e8f2 100644 --- a/src/utils/__tests__/simulator-defaults-refresh.test.ts +++ b/src/utils/__tests__/simulator-defaults-refresh.test.ts @@ -66,13 +66,14 @@ describe('scheduleSimulatorDefaultsRefresh', () => { vi.useRealTimers(); }); - async function runRefresh(options: { simulatorId: string; simulatorName: string }) { + async function runRefresh(options: { simulatorId?: string; simulatorName?: string }) { vi.useFakeTimers(); - sessionStore.setDefaults({ - simulatorId: options.simulatorId, - simulatorName: options.simulatorName, - }); + const defaults = { + ...(options.simulatorId != null ? { simulatorId: options.simulatorId } : {}), + ...(options.simulatorName != null ? { simulatorName: options.simulatorName } : {}), + }; + sessionStore.setDefaults(defaults); const expectedRevision = sessionStore.getRevision(); const scheduled = scheduleSimulatorDefaultsRefresh({ @@ -88,6 +89,25 @@ describe('scheduleSimulatorDefaultsRefresh', () => { await vi.runAllTimersAsync(); } + it('resolves simulatorName to simulatorId once when only name is set', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: true, + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + }); + + await runRefresh({ simulatorName: 'iPhone 17 Pro' }); + + expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1); + expect(resolveSimulatorIdToNameMock).not.toHaveBeenCalled(); + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + simulatorPlatform: 'iOS Simulator', + }); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + it('does not patch defaults when both values are set and name resolves to same id', async () => { resolveSimulatorNameToIdMock.mockResolvedValue({ success: true, diff --git a/src/utils/simulator-defaults-refresh.ts b/src/utils/simulator-defaults-refresh.ts index 57a27276..fab02a32 100644 --- a/src/utils/simulator-defaults-refresh.ts +++ b/src/utils/simulator-defaults-refresh.ts @@ -50,23 +50,13 @@ async function refreshSimulatorDefaults( const executor = options.executor ?? getDefaultCommandExecutor(); try { - if (!simulatorId && simulatorName) { - const resolution = await resolveSimulatorNameToId(executor, simulatorName); - if (resolution.success) { - simulatorId = resolution.simulatorId; - patch.simulatorId = resolution.simulatorId; - } - } - - if (simulatorId && simulatorName) { + if (simulatorName) { const resolution = await resolveSimulatorNameToId(executor, simulatorName); if (resolution.success && resolution.simulatorId !== simulatorId) { simulatorId = resolution.simulatorId; patch.simulatorId = resolution.simulatorId; } - } - - if (!simulatorName && simulatorId) { + } else if (simulatorId) { const resolution = await resolveSimulatorIdToName(executor, simulatorId); if (resolution.success) { simulatorName = resolution.simulatorName;