diff --git a/AGENTS.md b/AGENTS.md index b47afc47e..dc471e400 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,10 @@ ESM TypeScript project (`type: module`). Key layers: - Final settled output MUST render from the final structured/domain result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. - Streaming-capable renderers may observe fragment callbacks only for live progress. Fragment handling must not affect final structured output or final settled text. +## Error Handling +- Structured errors (domain results with `didError`) are for domain errors only: failures in the user's build/test/device/simulator workflow (compile errors, test failures, missing destinations, etc.). +- System errors with the MCP server or CLI itself (invalid internal state, unresolvable configuration, infrastructure failures) must NOT be wrapped in structured domain results — let them surface as runtime tool errors so they are clearly distinguishable from workflow outcomes. + ## Test Conventions - Vitest with colocated `__tests__/` directories using `*.test.ts` - Snapshot tests (`*.snapshot.test.ts`) must only assert generated tool output against fixtures. Move helper, parser, schema, setup, or behavior assertions to non-snapshot unit/integration tests. @@ -139,6 +143,8 @@ Use these sections under `## [Unreleased]`: - **External contributions**: `Added feature X ([#456](https://github.com/getsentry/XcodeBuildMCP/pull/456) by [@username](https://github.com/username))` ## Test Execution Rules +- **NEVER run the snapshot or smoke test suites without explicit user permission.** They are expensive (~7 min baseline, spawn real `xcodebuild`/`simctl`/`devicectl` processes and can wedge). This covers `npm run test:snapshot`, `npm run test:smoke`, and any direct `vitest run --config vitest.snapshot.config.ts` / `vitest.smoke.config.ts` invocation. Ask first, then run only if the user agrees. +- The default unit suite (`npm test` / `vitest run`), `npm run typecheck`, `npm run lint`, and `npm run build` are cheap and may be run freely without asking. - When running long test suites (snapshot tests, smoke tests), ALWAYS write full output to a log file and read it afterwards. NEVER pipe through `tail` or `grep` directly — that loses output you may need to debug failures. - Pattern: `DEVICE_ID=... npm run test:snapshot 2>&1 | tee /tmp/snapshot-results.txt` then read `/tmp/snapshot-results.txt` with the native read tool. - If you need a summary, read the log file and grep/filter it — the full output is always preserved. diff --git a/CLAUDE.md b/CLAUDE.md index e4a3aa8bd..e37749de9 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,13 @@ Use these sections under `## [Unreleased]`: - Final settled output MUST render from the final structured/domain result and next-step metadata. If final output needs data, add it to the final result type instead of reading it from fragments. - Streaming-capable renderers may observe fragment callbacks only for live progress. Fragment handling must not affect final structured output or final settled text. +## Error Handling +- Structured errors (domain results with `didError`) are for domain errors only: failures in the user's build/test/device/simulator workflow (compile errors, test failures, missing destinations, etc.). +- System errors with the MCP server or CLI itself (invalid internal state, unresolvable configuration, infrastructure failures) must NOT be wrapped in structured domain results — let them surface as runtime tool errors so they are clearly distinguishable from workflow outcomes. + ## Test Execution Rules +- **NEVER run the snapshot or smoke test suites without explicit user permission.** They are expensive (~7 min baseline, spawn real `xcodebuild`/`simctl`/`devicectl` processes and can wedge). This covers `npm run test:snapshot`, `npm run test:smoke`, and any direct `vitest run --config vitest.snapshot.config.ts` / `vitest.smoke.config.ts` invocation. Ask first, then run only if the user agrees. +- The default unit suite (`npm test` / `vitest run`), `npm run typecheck`, `npm run lint`, and `npm run build` are cheap and may be run freely without asking. - When running long test suites (snapshot tests, smoke tests), ALWAYS write full output to a log file and read it afterwards. NEVER pipe through `tail` or `grep` directly — that loses output you may need to debug failures. - Pattern: `DEVICE_ID=... npm run test:snapshot 2>&1 | tee /tmp/snapshot-results.txt` then read `/tmp/snapshot-results.txt` with the native read tool. - If you need a summary, read the log file and grep/filter it — the full output is always preserved. diff --git a/src/cli/commands/__tests__/setup.test.ts b/src/cli/commands/__tests__/setup.test.ts index c86265026..17db22484 100644 --- a/src/cli/commands/__tests__/setup.test.ts +++ b/src/cli/commands/__tests__/setup.test.ts @@ -216,6 +216,77 @@ describe('setup command', () => { expect(parsed.sessionDefaults?.scheme).toBe('App'); expect(parsed.sessionDefaults?.deviceId).toBe('DEVICE-1'); expect(parsed.sessionDefaults?.simulatorId).toBe('SIM-1'); + expect(parsed.sessionDefaults?.simulatorPlatform).toBe('iOS Simulator'); + expect(result.changedFields).toContainEqual( + expect.stringContaining('sessionDefaults.simulatorPlatform'), + ); + }); + + it('does not offer simulators whose runtime is unavailable', async () => { + const { fs, getStoredConfig, setTempFile } = createSetupFs(); + + const executor: CommandExecutor = async (command) => { + if (command[0] === 'xcrun' && command[1] === 'devicectl') { + setTempFile(command[5], mockDeviceListJson()); + return createMockCommandResponse({ success: true, output: '' }); + } + + if (command.includes('--json')) { + // Unavailable device is listed first: without filtering the picker would + // select it. The available device must be chosen instead. + return createMockCommandResponse({ + success: true, + output: JSON.stringify({ + devices: { + 'com.apple.CoreSimulator.SimRuntime.iOS-26-5': [ + { + name: 'iPhone 17 Pro', + udid: 'SIM-UNAVAILABLE', + state: 'Shutdown', + isAvailable: false, + }, + ], + 'com.apple.CoreSimulator.SimRuntime.iOS-17-0': [ + { + name: 'iPhone 15', + udid: 'SIM-AVAILABLE', + state: 'Shutdown', + isAvailable: true, + }, + ], + }, + }), + }); + } + + if (command[0] === 'xcrun') { + return createMockCommandResponse({ + success: true, + output: `== Devices ==\n-- iOS 17.0 --\n iPhone 15 (SIM-AVAILABLE) (Shutdown)`, + }); + } + + return createMockCommandResponse({ + success: true, + output: `Information about workspace "App":\n Schemes:\n App`, + }); + }; + + await runSetupWizard({ + cwd, + fs, + executor, + prompter: createTestPrompter(), + quietOutput: true, + }); + + const parsed = parseYaml(getStoredConfig()) as { + sessionDefaults?: Record; + }; + + expect(parsed.sessionDefaults?.simulatorId).toBe('SIM-AVAILABLE'); + expect(parsed.sessionDefaults?.simulatorName).toBe('iPhone 15'); + expect(parsed.sessionDefaults?.simulatorPlatform).toBe('iOS Simulator'); }); it('shows debug-gated workflows when existing config enables debug', async () => { diff --git a/src/cli/commands/setup.ts b/src/cli/commands/setup.ts index 3de1f6631..a24cd1db7 100644 --- a/src/cli/commands/setup.ts +++ b/src/cli/commands/setup.ts @@ -25,6 +25,8 @@ import type { FileSystemExecutor } from '../../utils/FileSystemExecutor.ts'; import type { CommandExecutor } from '../../utils/CommandExecutor.ts'; import { createDoctorDependencies } from '../../mcp/tools/doctor/lib/doctor.deps.ts'; import { XcodePlatform } from '../../types/common.ts'; +import { inferPlatformFromRuntime } from '../../utils/infer-platform.ts'; +import type { SimulatorPlatform } from '../../utils/platform-detection.ts'; type SetupPlatform = WorkflowTargetPlatform; @@ -54,6 +56,7 @@ interface SetupSelection { deviceId?: string; simulatorId?: string; simulatorName?: string; + simulatorPlatform?: SimulatorPlatform; clearDeviceDefault: boolean; clearSimulatorDefault: boolean; } @@ -329,6 +332,11 @@ function getChangedFields( beforeValue: beforeDefaults.simulatorName, afterValue: afterDefaults.simulatorName, }, + { + label: 'sessionDefaults.simulatorPlatform', + beforeValue: beforeDefaults.simulatorPlatform, + afterValue: afterDefaults.simulatorPlatform, + }, { label: 'setupPreferences.platforms', beforeValue: beforeConfig?.setupPreferences?.platforms, @@ -572,7 +580,7 @@ async function selectSimulator(opts: { stopMessage: 'Simulators loaded.', task: async () => { try { - return await listSimulators(opts.executor); + return await listSimulators(opts.executor, { enabled: true }); } catch { return []; } @@ -958,6 +966,9 @@ async function collectSetupSelection( deviceId: device?.udid, simulatorId: simulator?.udid, simulatorName: simulator?.name, + simulatorPlatform: simulator + ? (inferPlatformFromRuntime(simulator.runtime) ?? undefined) + : undefined, clearDeviceDefault: isMacOsOnly || (requiresDeviceDefault(enabledWorkflows) && device == null), clearSimulatorDefault: isMacOsOnly || (requiresSimulatorDefault(enabledWorkflows) && simulator == null), @@ -1001,6 +1012,9 @@ function selectionToMcpConfigJson(selection: SetupSelection): string { if (selection.simulatorName) { env.XCODEBUILDMCP_SIMULATOR_NAME = selection.simulatorName; } + if (selection.simulatorPlatform) { + env.XCODEBUILDMCP_SIMULATOR_PLATFORM = selection.simulatorPlatform; + } const mcpConfig = { mcpServers: { @@ -1093,7 +1107,7 @@ export async function runSetupWizard(deps?: Partial): Promise deleteSessionDefaultKeys.push('deviceId'); } if (selection.clearSimulatorDefault) { - deleteSessionDefaultKeys.push('simulatorId', 'simulatorName'); + deleteSessionDefaultKeys.push('simulatorId', 'simulatorName', 'simulatorPlatform'); } const persistedProjectPath = @@ -1119,6 +1133,7 @@ export async function runSetupWizard(deps?: Partial): Promise deviceId: selection.deviceId, simulatorId: selection.simulatorId, simulatorName: selection.simulatorName, + simulatorPlatform: selection.simulatorPlatform, }, setupPreferences: selection.platforms.length > 0 ? { platforms: [...selection.platforms] } : null, diff --git a/src/cli/register-tool-commands.ts b/src/cli/register-tool-commands.ts index f11e382f2..021a64fee 100644 --- a/src/cli/register-tool-commands.ts +++ b/src/cli/register-tool-commands.ts @@ -29,6 +29,7 @@ import { import { toCliJsonlEvent } from './jsonl-event.ts'; import { resolveSimulatorNameToId } from '../utils/simulator-resolver.ts'; import { getDefaultCommandExecutor } from '../utils/execution/index.ts'; +import { sessionStore } from '../utils/session-store.ts'; export interface RegisterToolCommandsOptions { workspaceRoot: string; @@ -357,6 +358,16 @@ function registerToolSubcommand( runtimeConfig: opts.runtimeConfig, profileOverride, }); + // Mirror the MCP bootstrap: hydrate the in-process session store with the + // full resolved defaults so session-aware logic (e.g. platform inference's + // cached simulatorPlatform) sees fields that are not tool schema parameters. + // Unlike the MCP server, the CLI deliberately does NOT schedule the + // background simulator defaults refresh: CLI runs must stay deterministic + // for CI workflows/scripts, so a stale or foreign simulator id fails fast + // instead of being silently re-resolved to a different device. + if (Object.keys(rawDefaults).length > 0) { + sessionStore.setDefaults(rawDefaults); + } const args = mergeCliSessionDefaults({ defaults: getCliSessionDefaultsForTool({ tool, diff --git a/src/mcp/tools/simulator/__tests__/build_sim.test.ts b/src/mcp/tools/simulator/__tests__/build_sim.test.ts index 34a2130f7..38f4f927f 100644 --- a/src/mcp/tools/simulator/__tests__/build_sim.test.ts +++ b/src/mcp/tools/simulator/__tests__/build_sim.test.ts @@ -142,24 +142,23 @@ describe('build_sim tool', () => { expect(result.content[0].text).toContain('simulatorName'); }); - it('should handle empty simulatorName parameter', async () => { + it('should error when no resolvable simulator is provided (empty simulatorName)', async () => { const mockExecutor = createMockExecutor({ success: false, output: '', error: 'For iOS Simulator platform, either simulatorId or simulatorName must be provided', }); - const { result } = await runBuildSimLogic( - { - workspacePath: '/path/to/workspace', - scheme: 'MyScheme', - simulatorName: '', - }, - mockExecutor, - ); - - expect(result.isError()).toBe(true); - expectPendingBuildResponse(result); + await expect( + runBuildSimLogic( + { + workspacePath: '/path/to/workspace', + scheme: 'MyScheme', + simulatorName: '', + }, + mockExecutor, + ), + ).rejects.toThrow(/Unable to determine the simulator platform/); }); }); diff --git a/src/mcp/tools/simulator/build_run_sim.ts b/src/mcp/tools/simulator/build_run_sim.ts index 3b8d6ad11..4ef01b3f0 100644 --- a/src/mcp/tools/simulator/build_run_sim.ts +++ b/src/mcp/tools/simulator/build_run_sim.ts @@ -136,10 +136,9 @@ async function prepareBuildRunSimExecution( ); const detectedPlatform = inferred.platform; const configuration = params.configuration; - const displayPlatform = - params.simulatorId && inferred.source !== 'simulator-runtime' - ? 'Simulator' - : String(detectedPlatform); + // Every successful inference source (cache, runtime, name heuristic) is + // authoritative, so the detected platform is always safe to display. + const displayPlatform = String(detectedPlatform); const platformName = detectedPlatform.replace(' Simulator', ''); return { diff --git a/src/server/bootstrap.ts b/src/server/bootstrap.ts index 899439351..d4b54ea88 100644 --- a/src/server/bootstrap.ts +++ b/src/server/bootstrap.ts @@ -169,6 +169,16 @@ export async function bootstrapServer( } if (Object.keys(syncedDefaults).length > 0) { + const currentDefaults = sessionStore.getAll(); + const selectorChanged = + (syncedDefaults.simulatorId != null && + syncedDefaults.simulatorId !== currentDefaults.simulatorId) || + (syncedDefaults.simulatorName != null && + syncedDefaults.simulatorName !== currentDefaults.simulatorName); + if (selectorChanged) { + // The cached platform belongs to the previous simulator selection. + sessionStore.clear(['simulatorPlatform']); + } sessionStore.setDefaults(syncedDefaults); log( 'info', diff --git a/src/utils/__tests__/infer-platform.test.ts b/src/utils/__tests__/infer-platform.test.ts index 520b65ace..b9647da55 100644 --- a/src/utils/__tests__/infer-platform.test.ts +++ b/src/utils/__tests__/infer-platform.test.ts @@ -154,7 +154,7 @@ describe('inferPlatform', () => { expect(result.source).toBe('simulator-runtime'); }); - it('falls back to build settings when simulator runtime cannot be resolved', async () => { + it('throws instead of guessing from build settings when the runtime cannot be resolved', async () => { const callHistory: string[][] = []; const mockExecutor: CommandExecutor = async (command) => { callHistory.push(command); @@ -172,93 +172,38 @@ describe('inferPlatform', () => { }); }; - const result = await inferPlatform( - { - simulatorId: 'SIM-UUID', - projectPath: '/tmp/Test.xcodeproj', - scheme: 'WatchScheme', - }, - mockExecutor, - ); - - expect(result.platform).toBe(XcodePlatform.watchOSSimulator); - expect(result.source).toBe('build-settings'); - expect(callHistory).toHaveLength(2); + await expect( + inferPlatform( + { + simulatorId: 'SIM-UUID', + projectPath: '/tmp/Test.xcodeproj', + scheme: 'WatchScheme', + }, + mockExecutor, + ), + ).rejects.toThrow(/Unable to determine the simulator platform/); + + // Platform inference no longer shells out to xcodebuild -showBuildSettings. + expect(callHistory).toHaveLength(1); expect(callHistory[0]).toEqual(['xcrun', 'simctl', 'list', 'devices', 'available', '--json']); - expect(callHistory[1]).toEqual([ - 'xcodebuild', - '-showBuildSettings', - '-scheme', - 'WatchScheme', - '-project', - '/tmp/Test.xcodeproj', - ]); - }); - - it('prefers workspace defaults when both projectPath and workspacePath are present in session defaults', async () => { - sessionStore.setDefaults({ - projectPath: '/tmp/Test.xcodeproj', - workspacePath: '/tmp/Test.xcworkspace', - scheme: 'WatchScheme', - }); - - const callHistory: string[][] = []; - const mockExecutor: CommandExecutor = async (command) => { - callHistory.push(command); - - if (command[0] === 'xcrun') { - return createMockCommandResponse({ - success: true, - output: JSON.stringify({ devices: {} }), - }); - } - - return createMockCommandResponse({ - success: true, - output: 'SDKROOT = watchsimulator', - }); - }; - - const result = await inferPlatform({ simulatorId: 'SIM-UUID' }, mockExecutor); - - expect(result.platform).toBe(XcodePlatform.watchOSSimulator); - expect(result.source).toBe('build-settings'); - expect(callHistory).toHaveLength(2); - expect(callHistory[1]).toEqual([ - 'xcodebuild', - '-showBuildSettings', - '-scheme', - 'WatchScheme', - '-workspace', - '/tmp/Test.xcworkspace', - ]); }); - it('defaults to iOS when simulator and build-settings inference both fail', async () => { - const mockExecutor: CommandExecutor = async (command) => { - if (command[0] === 'xcrun') { - return createMockCommandResponse({ - success: false, - error: 'simctl failed', - }); - } - - return createMockCommandResponse({ + it('throws when simulator inference fails and no platform is cached', async () => { + const mockExecutor: CommandExecutor = async () => + createMockCommandResponse({ success: false, - error: 'xcodebuild failed', + error: 'simctl failed', }); - }; - const result = await inferPlatform( - { - simulatorId: 'SIM-UUID', - workspacePath: '/tmp/Test.xcworkspace', - scheme: 'UnknownScheme', - }, - mockExecutor, - ); - - expect(result.platform).toBe(XcodePlatform.iOSSimulator); - expect(result.source).toBe('default'); + await expect( + inferPlatform( + { + simulatorId: 'SIM-UUID', + workspacePath: '/tmp/Test.xcworkspace', + scheme: 'UnknownScheme', + }, + mockExecutor, + ), + ).rejects.toThrow(/Unable to determine the simulator platform/); }); }); diff --git a/src/utils/__tests__/platform-detection.test.ts b/src/utils/__tests__/platform-detection.test.ts deleted file mode 100644 index c170f5184..000000000 --- a/src/utils/__tests__/platform-detection.test.ts +++ /dev/null @@ -1,116 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { createMockExecutor } from '../../test-utils/mock-executors.ts'; -import { XcodePlatform } from '../../types/common.ts'; -import { detectPlatformFromScheme } from '../platform-detection.ts'; - -describe('detectPlatformFromScheme', () => { - it('detects simulator platform from SDKROOT', async () => { - const executor = createMockExecutor({ - success: true, - output: 'SDKROOT = watchsimulator\nSUPPORTED_PLATFORMS = watchsimulator watchos', - }); - - const result = await detectPlatformFromScheme( - '/tmp/Test.xcodeproj', - undefined, - 'WatchScheme', - executor, - ); - - expect(result.platform).toBe(XcodePlatform.watchOSSimulator); - expect(result.sdkroot).toBe('watchsimulator'); - }); - - it('falls back to SUPPORTED_PLATFORMS when SDKROOT is missing', async () => { - const executor = createMockExecutor({ - success: true, - output: 'SUPPORTED_PLATFORMS = appletvsimulator appletvos', - }); - - const result = await detectPlatformFromScheme( - undefined, - '/tmp/Test.xcworkspace', - 'TVScheme', - executor, - ); - - expect(result.platform).toBe(XcodePlatform.tvOSSimulator); - expect(result.sdkroot).toBeNull(); - }); - - it('returns null platform for non-simulator SDKROOT values', async () => { - const executor = createMockExecutor({ - success: true, - output: 'SDKROOT = macosx\nSUPPORTED_PLATFORMS = macosx', - }); - - const result = await detectPlatformFromScheme( - '/tmp/Test.xcodeproj', - undefined, - 'MacScheme', - executor, - ); - - expect(result.platform).toBeNull(); - expect(result.sdkroot).toBe('macosx'); - }); - - it('prefers simulator SDKROOT when build settings contain multiple blocks', async () => { - const executor = createMockExecutor({ - success: true, - output: ` -Build settings for action build and target DeviceTarget: - SDKROOT = iphoneos - SUPPORTED_PLATFORMS = iphoneos - -Build settings for action build and target SimulatorTarget: - SDKROOT = iphonesimulator18.0 - SUPPORTED_PLATFORMS = iphonesimulator iphoneos -`, - }); - - const result = await detectPlatformFromScheme( - '/tmp/Test.xcodeproj', - undefined, - 'MixedScheme', - executor, - ); - - expect(result.platform).toBe(XcodePlatform.iOSSimulator); - expect(result.sdkroot).toBe('iphonesimulator18.0'); - }); - - it('returns error when both projectPath and workspacePath are provided', async () => { - const executor = createMockExecutor({ - success: true, - output: 'SDKROOT = iphonesimulator', - }); - - const result = await detectPlatformFromScheme( - '/tmp/Test.xcodeproj', - '/tmp/Test.xcworkspace', - 'AmbiguousScheme', - executor, - ); - - expect(result.platform).toBeNull(); - expect(result.error).toContain('mutually exclusive'); - }); - - it('surfaces command failure details', async () => { - const executor = createMockExecutor({ - success: false, - error: 'xcodebuild failed', - }); - - const result = await detectPlatformFromScheme( - '/tmp/Test.xcodeproj', - undefined, - 'BrokenScheme', - executor, - ); - - expect(result.platform).toBeNull(); - expect(result.error).toBe('xcodebuild failed'); - }); -}); diff --git a/src/utils/__tests__/simulator-defaults-refresh.test.ts b/src/utils/__tests__/simulator-defaults-refresh.test.ts index bb39e8f2c..cf108cbc4 100644 --- a/src/utils/__tests__/simulator-defaults-refresh.test.ts +++ b/src/utils/__tests__/simulator-defaults-refresh.test.ts @@ -66,12 +66,19 @@ describe('scheduleSimulatorDefaultsRefresh', () => { vi.useRealTimers(); }); - async function runRefresh(options: { simulatorId?: string; simulatorName?: string }) { + async function runRefresh(options: { + simulatorId?: string; + simulatorName?: string; + storedSimulatorPlatform?: 'iOS Simulator' | 'tvOS Simulator'; + }) { vi.useFakeTimers(); const defaults = { ...(options.simulatorId != null ? { simulatorId: options.simulatorId } : {}), ...(options.simulatorName != null ? { simulatorName: options.simulatorName } : {}), + ...(options.storedSimulatorPlatform != null + ? { simulatorPlatform: options.storedSimulatorPlatform } + : {}), }; sessionStore.setDefaults(defaults); const expectedRevision = sessionStore.getRevision(); @@ -108,28 +115,28 @@ describe('scheduleSimulatorDefaultsRefresh', () => { expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); }); - it('does not patch defaults when both values are set and name resolves to same id', async () => { + it('caches platform without patching id when both 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(resolveSimulatorIdToNameMock).not.toHaveBeenCalled(); expect(sessionStore.getAll()).toEqual({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro', + simulatorPlatform: 'iOS Simulator', }); expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); }); - it('patches simulatorId in memory when both are set and name resolves to a different id', async () => { + it('re-materializes the machine-local id when the name resolves to a different id', async () => { + // Team-sharing: config.yaml from SCM carries another machine's UDID; the + // canonical simulatorName must win and update the id for this machine. resolveSimulatorNameToIdMock.mockResolvedValue({ success: true, simulatorId: 'SIM-2', @@ -147,7 +154,70 @@ describe('scheduleSimulatorDefaultsRefresh', () => { expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); }); - it('keeps the existing simulatorId when name lookup fails and logs a warning', async () => { + it('keeps the stored id when no available simulator matches the name', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: false, + error: 'Simulator named "iPhone 17 Pro" not found.', + }); + + await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); + + expect(resolveSimulatorNameToIdMock).toHaveBeenCalledTimes(1); + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + simulatorPlatform: 'iOS Simulator', + }); + expect(logMock).toHaveBeenCalledWith( + 'info', + expect.stringContaining('Simulator name did not resolve during startup-hydration refresh'), + ); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + + it('keeps the existing defaults when the platform cannot be inferred', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: false, + error: 'Simulator named "iPhone 17 Pro" not found.', + }); + inferPlatformMock.mockRejectedValue(new Error('Unable to determine the simulator platform')); + + await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); + + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + }); + expect(logMock).toHaveBeenCalledWith( + 'info', + expect.stringContaining('Could not infer simulator platform during startup-hydration'), + ); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + + it('clears the stale cached platform when the id is remapped but recompute fails', async () => { + resolveSimulatorNameToIdMock.mockResolvedValue({ + success: true, + simulatorId: 'SIM-2', + simulatorName: 'iPhone 17 Pro', + }); + inferPlatformMock.mockRejectedValue(new Error('Unable to determine the simulator platform')); + + await runRefresh({ + simulatorId: 'SIM-1', + simulatorName: 'iPhone 17 Pro', + storedSimulatorPlatform: 'tvOS Simulator', + }); + + // The new id must not stay paired with the previous device's platform. + expect(sessionStore.getAll()).toEqual({ + simulatorId: 'SIM-2', + simulatorName: 'iPhone 17 Pro', + }); + expect(persistSessionDefaultsPatchMock).not.toHaveBeenCalled(); + }); + + it('keeps the existing defaults when name lookup fails and logs a warning', async () => { resolveSimulatorNameToIdMock.mockRejectedValue(new Error('simctl failed')); await runRefresh({ simulatorId: 'SIM-1', simulatorName: 'iPhone 17 Pro' }); diff --git a/src/utils/infer-platform.ts b/src/utils/infer-platform.ts index 28b596ad2..f3f3d8f68 100644 --- a/src/utils/infer-platform.ts +++ b/src/utils/infer-platform.ts @@ -2,15 +2,10 @@ import { XcodePlatform } from '../types/common.ts'; import type { CommandExecutor } from './execution/index.ts'; import { getDefaultCommandExecutor } from './execution/index.ts'; import { log } from './logging/index.ts'; -import { detectPlatformFromScheme, type SimulatorPlatform } from './platform-detection.ts'; +import type { SimulatorPlatform } from './platform-detection.ts'; import { sessionStore, type SessionDefaults } from './session-store.ts'; -type PlatformInferenceSource = - | 'simulator-platform-cache' - | 'simulator-name' - | 'simulator-runtime' - | 'build-settings' - | 'default'; +type PlatformInferenceSource = 'simulator-platform-cache' | 'simulator-name' | 'simulator-runtime'; export interface InferPlatformParams { projectPath?: string; @@ -55,7 +50,7 @@ function inferPlatformFromSimulatorName(simulatorName: string): SimulatorPlatfor return null; } -function inferPlatformFromRuntime(runtime: string): SimulatorPlatform | null { +export function inferPlatformFromRuntime(runtime: string): SimulatorPlatform | null { const value = runtime.toLowerCase(); if (value.includes('simruntime.watchos') || value.startsWith('watchos')) { @@ -138,34 +133,6 @@ function resolveCachedPlatform(params: InferPlatformParams): SimulatorPlatform | return null; } -function resolveProjectFromSession(params: InferPlatformParams): { - projectPath?: string; - workspacePath?: string; - scheme?: string; -} { - const defaults = params.sessionDefaults ?? sessionStore.getAll(); - const hasExplicitProjectPath = params.projectPath !== undefined; - const hasExplicitWorkspacePath = params.workspacePath !== undefined; - const projectPath = - params.projectPath ?? (params.workspacePath ? undefined : defaults.projectPath); - const workspacePath = - params.workspacePath ?? (params.projectPath ? undefined : defaults.workspacePath); - - if (projectPath && workspacePath && !hasExplicitProjectPath && !hasExplicitWorkspacePath) { - return { - projectPath: undefined, - workspacePath, - scheme: params.scheme ?? defaults.scheme, - }; - } - - return { - projectPath, - workspacePath, - scheme: params.scheme ?? defaults.scheme, - }; -} - async function inferPlatformFromSimctl( simulatorId: string | undefined, simulatorName: string | undefined, @@ -226,6 +193,22 @@ async function inferPlatformFromSimctl( return null; } +/** + * Determine the simulator platform for a simulator selector. + * + * Precedence (all sources are authoritative — derived from the device itself): + * 1. `sessionDefaults.simulatorPlatform` cache, only when the selector matches + * the session defaults it was computed for (written by setup and the + * background defaults refresh; invalidated whenever the selector changes). + * 2. The device's runtime from `simctl list devices available`. + * 3. The simulator name heuristic (e.g. "iPhone ..." -> iOS Simulator). + * + * There is deliberately NO fallback beyond these: guessing from project build + * settings (SUPPORTED_PLATFORMS) or defaulting to iOS produced wrong, + * non-deterministic destinations for multi-platform projects. If none of the + * sources resolve, this throws so callers surface a clear error instead of + * building for the wrong platform. + */ export async function inferPlatform( params: InferPlatformParams, executor: CommandExecutor = getDefaultCommandExecutor(), @@ -264,13 +247,16 @@ export async function inferPlatform( } } - const { projectPath, workspacePath, scheme } = resolveProjectFromSession(params); - if (scheme && (projectPath || workspacePath)) { - const detection = await detectPlatformFromScheme(projectPath, workspacePath, scheme, executor); - if (detection.platform) { - return { platform: detection.platform, source: 'build-settings' }; - } - } - - return { platform: XcodePlatform.iOSSimulator, source: 'default' }; + const selector = + simulatorIdForLookup != null + ? `simulator id "${simulatorIdForLookup}"` + : simulatorNameForLookup != null + ? `simulator name "${simulatorNameForLookup}"` + : 'the configured simulator'; + + throw new Error( + `Unable to determine the simulator platform for ${selector}. The simulator was not found ` + + `among available devices — its runtime may not be installed. Run 'xcodebuildmcp setup' to ` + + `select an available simulator, or use the simulator list tool to see installed simulators.`, + ); } diff --git a/src/utils/platform-detection.ts b/src/utils/platform-detection.ts index cf0231a9f..220848a6f 100644 --- a/src/utils/platform-detection.ts +++ b/src/utils/platform-detection.ts @@ -1,129 +1,7 @@ import { XcodePlatform } from '../types/common.ts'; -import type { CommandExecutor } from './execution/index.ts'; -import { getDefaultCommandExecutor } from './execution/index.ts'; -import { log } from './logging/index.ts'; export type SimulatorPlatform = | XcodePlatform.iOSSimulator | XcodePlatform.watchOSSimulator | XcodePlatform.tvOSSimulator | XcodePlatform.visionOSSimulator; - -export interface PlatformDetectionResult { - platform: SimulatorPlatform | null; - sdkroot: string | null; - supportedPlatforms: string[]; - error?: string; -} - -function sdkrootToSimulatorPlatform(sdkroot: string): SimulatorPlatform | null { - const sdkLower = sdkroot.toLowerCase(); - - if (sdkLower.startsWith('watchsimulator')) return XcodePlatform.watchOSSimulator; - if (sdkLower.startsWith('appletvsimulator')) return XcodePlatform.tvOSSimulator; - if (sdkLower.startsWith('xrsimulator')) return XcodePlatform.visionOSSimulator; - if (sdkLower.startsWith('iphonesimulator')) return XcodePlatform.iOSSimulator; - - return null; -} - -function supportedPlatformsToSimulatorPlatform(platforms: string[]): SimulatorPlatform | null { - const normalized = new Set(platforms.map((platform) => platform.toLowerCase())); - - if (normalized.has('watchsimulator')) return XcodePlatform.watchOSSimulator; - if (normalized.has('appletvsimulator')) return XcodePlatform.tvOSSimulator; - if (normalized.has('xrsimulator')) return XcodePlatform.visionOSSimulator; - if (normalized.has('iphonesimulator')) return XcodePlatform.iOSSimulator; - - return null; -} - -function extractBuildSettingValues(output: string, settingName: string): string[] { - const regex = new RegExp(`^\\s*${settingName}\\s*=\\s*(.+)$`, 'gm'); - const values: string[] = []; - - for (const match of output.matchAll(regex)) { - const value = match[1]?.trim(); - if (value) values.push(value); - } - - return values; -} - -export async function detectPlatformFromScheme( - projectPath: string | undefined, - workspacePath: string | undefined, - scheme: string, - executor: CommandExecutor = getDefaultCommandExecutor(), -): Promise { - const command = ['xcodebuild', '-showBuildSettings', '-scheme', scheme]; - - if (projectPath && workspacePath) { - return { - platform: null, - sdkroot: null, - supportedPlatforms: [], - error: 'projectPath and workspacePath are mutually exclusive for platform detection', - }; - } - - if (projectPath) { - command.push('-project', projectPath); - } else if (workspacePath) { - command.push('-workspace', workspacePath); - } else { - return { - platform: null, - sdkroot: null, - supportedPlatforms: [], - error: 'Either projectPath or workspacePath is required for platform detection', - }; - } - - try { - const result = await executor(command, 'Platform Detection', true); - if (!result.success) { - return { - platform: null, - sdkroot: null, - supportedPlatforms: [], - error: result.error ?? 'xcodebuild -showBuildSettings failed', - }; - } - - const output = result.output ?? ''; - const sdkroots = extractBuildSettingValues(output, 'SDKROOT'); - const supportedPlatforms = extractBuildSettingValues(output, 'SUPPORTED_PLATFORMS').flatMap( - (value) => value.split(/\s+/), - ); - - let sdkroot: string | null = null; - let platform: SimulatorPlatform | null = null; - - for (const sdkrootValue of sdkroots) { - const detected = sdkrootToSimulatorPlatform(sdkrootValue); - if (detected) { - platform = detected; - sdkroot = sdkrootValue; - break; - } - } - - if (!sdkroot && sdkroots.length > 0) sdkroot = sdkroots[0]; - - if (!platform && supportedPlatforms.length > 0) { - platform = supportedPlatformsToSimulatorPlatform(supportedPlatforms); - } - - return { platform, sdkroot, supportedPlatforms }; - } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error); - log('warn', `[Platform Detection] ${errorMessage}`); - return { - platform: null, - sdkroot: null, - supportedPlatforms: [], - error: errorMessage, - }; - } -} diff --git a/src/utils/session-defaults-schema.ts b/src/utils/session-defaults-schema.ts index d8ab15730..b09889a28 100644 --- a/src/utils/session-defaults-schema.ts +++ b/src/utils/session-defaults-schema.ts @@ -31,12 +31,22 @@ export const sessionDefaultsSchema = z.object({ configuration: nonEmptyString .optional() .describe("Build configuration for Xcode and SwiftPM tools (e.g. 'Debug' or 'Release')."), - simulatorName: nonEmptyString.optional(), - simulatorId: nonEmptyString.optional(), + simulatorName: nonEmptyString + .optional() + .describe( + 'Canonical, machine-portable simulator selector (safe to share via SCM); the background refresh re-resolves it to this machine’s UDID.', + ), + simulatorId: nonEmptyString + .optional() + .describe( + 'Machine-local simulator UDID, materialized from simulatorName when one is set; UDIDs are not portable across machines.', + ), simulatorPlatform: z .enum(['iOS Simulator', 'watchOS Simulator', 'tvOS Simulator', 'visionOS Simulator']) .optional() - .describe('Cached inferred simulator platform.'), + .describe( + 'Cached platform derived from the selected simulator’s runtime; must be cleared/recomputed whenever the simulator selector changes.', + ), deviceId: nonEmptyString.optional(), useLatestOS: z.boolean().optional(), arch: z.enum(['arm64', 'x86_64']).optional(), diff --git a/src/utils/simulator-defaults-refresh.ts b/src/utils/simulator-defaults-refresh.ts index fab02a324..c5e98d337 100644 --- a/src/utils/simulator-defaults-refresh.ts +++ b/src/utils/simulator-defaults-refresh.ts @@ -41,6 +41,31 @@ export function scheduleSimulatorDefaultsRefresh( return true; } +/** + * Background refresh that keeps the simulator session defaults coherent. + * + * Contract (do not change without understanding the team-sharing model): + * - `simulatorName` is the CANONICAL, machine-portable selector. Project + * config (`.xcodebuildmcp/config.yaml`) is commonly committed to SCM and + * shared across a team, and simulator UDIDs differ per machine even for + * identically-named simulators. + * - `simulatorId` is a machine-local materialization of that name. When a + * name is set, name -> id resolution runs here and MAY update the stored id + * (e.g. a teammate's UDID from a checked-out config is replaced with this + * machine's UDID for the same-named simulator). + * - id -> name resolution only runs when no name is stored (seeds the + * portable selector from a pinned id). + * - If resolution fails (no available simulator matches), the stored defaults + * are left untouched — never clobbered. + * - `simulatorPlatform` is a cache derived from the resolved device runtime; + * it must be recomputed whenever the selector changes and cleared by any + * code path that overwrites the selector without recomputing it. + * - This refresh runs only in MCP server paths (startup hydration, + * session-set-defaults). One-shot CLI invocations deliberately do NOT + * re-materialize ids: the CLI is used by CI workflows/scripts and must be + * deterministic — a stale/foreign UDID fails fast with a clear error rather + * than silently resolving to a different device. + */ async function refreshSimulatorDefaults( options: ScheduleSimulatorDefaultsRefreshOptions, ): Promise { @@ -51,10 +76,21 @@ async function refreshSimulatorDefaults( try { if (simulatorName) { + // simulatorName is the canonical, machine-portable selector: config.yaml is + // often shared in SCM, and simulator UDIDs differ per machine even for + // identically-named simulators. Re-resolving name -> id here materializes + // the id for THIS machine so downstream tools can pin an exact device. const resolution = await resolveSimulatorNameToId(executor, simulatorName); if (resolution.success && resolution.simulatorId !== simulatorId) { simulatorId = resolution.simulatorId; patch.simulatorId = resolution.simulatorId; + } else if (!resolution.success) { + // No available simulator matches the name — leave the stored defaults + // untouched rather than clobbering them. + log( + 'info', + `[Session] Simulator name did not resolve during ${options.reason} refresh: ${resolution.error}`, + ); } } else if (simulatorId) { const resolution = await resolveSimulatorIdToName(executor, simulatorId); @@ -65,27 +101,42 @@ async function refreshSimulatorDefaults( } const shouldRecomputePlatform = options.recomputePlatform ?? true; + let platformRecomputed = false; if (shouldRecomputePlatform && (simulatorId || simulatorName)) { - const inferred = await inferPlatform( - { - simulatorId, - simulatorName, - sessionDefaults: { - ...sessionStore.getAllForProfile(options.profile), - ...patch, + try { + const inferred = await inferPlatform( + { simulatorId, simulatorName, - simulatorPlatform: undefined, + sessionDefaults: { + ...sessionStore.getAllForProfile(options.profile), + ...patch, + simulatorId, + simulatorName, + simulatorPlatform: undefined, + }, }, - }, - executor, - ); - - if (inferred.source !== 'default') { + executor, + ); patch.simulatorPlatform = inferred.platform; + platformRecomputed = true; + } catch (error) { + log( + 'info', + `[Session] Could not infer simulator platform during ${options.reason} refresh: ${String(error)}`, + ); } } + const deleteKeys: (keyof SessionDefaults)[] = []; + if (patch.simulatorId != null && !platformRecomputed) { + // Invariant: a changed selector must never keep the previous device's + // cached platform. Drop the cache and let the next inference resolve + // it from simctl. + patch.simulatorPlatform = undefined; + deleteKeys.push('simulatorPlatform'); + } + if (Object.keys(patch).length === 0) { return; } @@ -104,7 +155,7 @@ async function refreshSimulatorDefaults( } if (options.persist) { - await persistSessionDefaultsPatch({ patch, profile: options.profile }); + await persistSessionDefaultsPatch({ patch, deleteKeys, profile: options.profile }); } } catch (error) { log( diff --git a/src/utils/xcode-state-watcher.ts b/src/utils/xcode-state-watcher.ts index a881fe2d4..ec16c711c 100644 --- a/src/utils/xcode-state-watcher.ts +++ b/src/utils/xcode-state-watcher.ts @@ -142,6 +142,10 @@ async function processFileChange(): Promise { // Update session defaults immediately with scheme/simulatorId if (Object.keys(updates).length > 0) { + if (updates.simulatorId && updates.simulatorId !== sessionStore.getAll().simulatorId) { + // The cached platform belongs to the previous simulator selection. + sessionStore.clear(['simulatorPlatform']); + } sessionStore.setDefaults(updates); log('info', `[xcode-watcher] Session defaults updated: ${JSON.stringify(updates)}`); }