From 04c6a0c7c69eb4ddb4a252245d29350545311773 Mon Sep 17 00:00:00 2001 From: Victor Solano Date: Mon, 10 Aug 2026 10:26:47 +0200 Subject: [PATCH] Fix simulator session argument overrides (#509) --- CHANGELOG.md | 4 ++ .../simulator/__tests__/build_run_sim.test.ts | 34 ++++++++-- .../simulator/__tests__/build_sim.test.ts | 32 ++++++++- .../simulator/__tests__/test_sim.test.ts | 35 +++++++++- src/mcp/tools/simulator/build_run_sim.ts | 10 +-- src/mcp/tools/simulator/build_sim.ts | 10 +-- src/mcp/tools/simulator/test_sim.ts | 50 +++++++------- .../__tests__/e2e-mcp-sessions.test.ts | 65 +++++++++++++++++++ .../session-defaults-disabled/test_sim.json | 10 +++ .../build_run_sim.json | 11 +++- .../session-defaults-enabled/build_sim.json | 11 +++- .../session-defaults-enabled/test_sim.json | 19 ++++++ 12 files changed, 234 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1130fa825..e4377c6f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed simulator build, build-and-run, and test MCP calls silently discarding explicit project, scheme, destination, and configuration arguments when session defaults were set. Explicit values now override defaults, and `test_sim` accepts typed `onlyTesting` and `skipTesting` selectors ([#509](https://github.com/getsentry/XcodeBuildMCP/issues/509)). + ### Changed - Dictionary-shaped MCP inputs now use client-compatible wire representations ([#491](https://github.com/getsentry/XcodeBuildMCP/issues/491)). The `env` and `testRunnerEnv` inputs on build, launch, test, and session-default tools are arrays of `{ "key": "...", "value": "..." }` entries, while `xcode_ide_call_tool.arguments` is a JSON object string. XcodeBuildMCP converts these values to their existing internal objects only after MCP input validation. diff --git a/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts b/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts index 2288dd033..b20f95f9f 100644 --- a/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts +++ b/src/mcp/tools/simulator/__tests__/build_run_sim.test.ts @@ -48,7 +48,7 @@ describe('build_run_sim tool', () => { expect(typeof handler).toBe('function'); }); - it('should expose only non-session fields in public schema', () => { + it('should expose optional session defaults for explicit overrides', () => { const schemaObj = z.strictObject(schema); expect(schemaObj.safeParse({}).success).toBe(true); @@ -64,16 +64,36 @@ describe('build_run_sim tool', () => { }).success, ).toBe(true); - expect(schemaObj.safeParse({ derivedDataPath: '/path/to/derived' }).success).toBe(false); + expect( + schemaObj.safeParse({ + workspacePath: '/path/to/workspace.xcworkspace', + scheme: 'ExplicitScheme', + simulatorName: 'iPhone 17', + configuration: 'Release', + derivedDataPath: '/path/to/derived', + preferXcodebuild: false, + }).success, + ).toBe(true); expect(schemaObj.safeParse({ extraArgs: [123] }).success).toBe(false); expect(schemaObj.safeParse({ launchArgs: [123] }).success).toBe(false); - expect(schemaObj.safeParse({ preferXcodebuild: false }).success).toBe(false); + expect(schemaObj.safeParse({ preferXcodebuild: 'false' }).success).toBe(false); const schemaKeys = Object.keys(schema).sort(); - expect(schemaKeys).toEqual(['extraArgs', 'launchArgs']); - expect(schemaKeys).not.toContain('scheme'); - expect(schemaKeys).not.toContain('simulatorName'); - expect(schemaKeys).not.toContain('projectPath'); + expect(schemaKeys).toEqual( + [ + 'configuration', + 'derivedDataPath', + 'extraArgs', + 'launchArgs', + 'preferXcodebuild', + 'projectPath', + 'scheme', + 'simulatorId', + 'simulatorName', + 'useLatestOS', + 'workspacePath', + ].sort(), + ); }); }); diff --git a/src/mcp/tools/simulator/__tests__/build_sim.test.ts b/src/mcp/tools/simulator/__tests__/build_sim.test.ts index 4748d6de8..bb054079e 100644 --- a/src/mcp/tools/simulator/__tests__/build_sim.test.ts +++ b/src/mcp/tools/simulator/__tests__/build_sim.test.ts @@ -29,7 +29,7 @@ describe('build_sim tool', () => { expect(typeof handler).toBe('function'); }); - it('should have correct public schema (only non-session fields)', () => { + it('should expose optional session defaults for explicit overrides', () => { const schemaObj = z.strictObject(schema); expect(schemaObj.safeParse({}).success).toBe(true); @@ -40,15 +40,41 @@ describe('build_sim tool', () => { }).success, ).toBe(true); - expect(schemaObj.safeParse({ derivedDataPath: '/path/to/derived' }).success).toBe(false); + expect( + schemaObj.safeParse({ + projectPath: '/path/to/project.xcodeproj', + scheme: 'ExplicitScheme', + simulatorName: 'iPhone 17', + configuration: 'Release', + derivedDataPath: '/path/to/derived', + preferXcodebuild: false, + }).success, + ).toBe(true); expect(schemaObj.safeParse({ extraArgs: [123] }).success).toBe(false); - expect(schemaObj.safeParse({ preferXcodebuild: false }).success).toBe(false); + expect(schemaObj.safeParse({ preferXcodebuild: 'false' }).success).toBe(false); expect( schemaObj.safeParse({ buildForTesting: true, testProductsPath: '/tmp/MyApp.xctestproducts', }).success, ).toBe(true); + + expect(Object.keys(schema).sort()).toEqual( + [ + 'buildForTesting', + 'configuration', + 'derivedDataPath', + 'extraArgs', + 'preferXcodebuild', + 'projectPath', + 'scheme', + 'simulatorId', + 'simulatorName', + 'testProductsPath', + 'useLatestOS', + 'workspacePath', + ].sort(), + ); }); it('should reject testProductsPath without buildForTesting', () => { diff --git a/src/mcp/tools/simulator/__tests__/test_sim.test.ts b/src/mcp/tools/simulator/__tests__/test_sim.test.ts index ca2a5b775..27e122a79 100644 --- a/src/mcp/tools/simulator/__tests__/test_sim.test.ts +++ b/src/mcp/tools/simulator/__tests__/test_sim.test.ts @@ -18,7 +18,7 @@ describe('test_sim tool', () => { expect(typeof handler).toBe('function'); }); - it('should expose only non-session fields in public schema', () => { + it('should expose optional session defaults for explicit overrides', () => { const schemaObj = z.strictObject(schema); expect(schemaObj.safeParse({}).success).toBe(true); @@ -31,12 +31,41 @@ describe('test_sim tool', () => { expect(schemaObj.safeParse({ derivedDataPath: 123 }).success).toBe(false); expect(schemaObj.safeParse({ extraArgs: ['--ok', 42] }).success).toBe(false); - expect(schemaObj.safeParse({ preferXcodebuild: true }).success).toBe(false); + expect( + schemaObj.safeParse({ + projectPath: '/path/to/project.xcodeproj', + scheme: 'ExplicitScheme', + simulatorName: 'iPhone 17', + configuration: 'Release', + preferXcodebuild: true, + onlyTesting: ['MyTests/LoginTests/testSuccess'], + skipTesting: ['MyTests/LoginTests/testFailure'], + }).success, + ).toBe(true); + expect(schemaObj.safeParse({ preferXcodebuild: 'true' }).success).toBe(false); + expect(schemaObj.safeParse({ onlyTesting: [42] }).success).toBe(false); expect(schemaObj.safeParse({ testRunnerEnv: { FOO: 123 } }).success).toBe(false); const schemaKeys = Object.keys(schema).sort(); expect(schemaKeys).toEqual( - ['extraArgs', 'progress', 'testProductsPath', 'testRunnerEnv', 'xctestrunPath'].sort(), + [ + 'configuration', + 'derivedDataPath', + 'extraArgs', + 'onlyTesting', + 'preferXcodebuild', + 'progress', + 'projectPath', + 'scheme', + 'simulatorId', + 'simulatorName', + 'skipTesting', + 'testProductsPath', + 'testRunnerEnv', + 'useLatestOS', + 'workspacePath', + 'xctestrunPath', + ].sort(), ); }); }); diff --git a/src/mcp/tools/simulator/build_run_sim.ts b/src/mcp/tools/simulator/build_run_sim.ts index f53a03b25..7c046765d 100644 --- a/src/mcp/tools/simulator/build_run_sim.ts +++ b/src/mcp/tools/simulator/build_run_sim.ts @@ -509,16 +509,8 @@ export function createBuildRunSimExecutor( }; } -const publicSchemaObject = baseSchemaObject.omit({ - projectPath: true, - workspacePath: true, +const publicSchemaObject = baseSchemaObject.partial({ scheme: true, - configuration: true, - simulatorId: true, - simulatorName: true, - useLatestOS: true, - derivedDataPath: true, - preferXcodebuild: true, } as const); export async function build_run_simLogic( diff --git a/src/mcp/tools/simulator/build_sim.ts b/src/mcp/tools/simulator/build_sim.ts index c89e343f4..703f2a454 100644 --- a/src/mcp/tools/simulator/build_sim.ts +++ b/src/mcp/tools/simulator/build_sim.ts @@ -187,16 +187,8 @@ export async function prepareBuildSimExecution( }; } -const publicSchemaObject = baseSchemaObject.omit({ - projectPath: true, - workspacePath: true, +const publicSchemaObject = baseSchemaObject.partial({ scheme: true, - configuration: true, - simulatorId: true, - simulatorName: true, - useLatestOS: true, - derivedDataPath: true, - preferXcodebuild: true, } as const); export function createBuildSimExecutor( diff --git a/src/mcp/tools/simulator/test_sim.ts b/src/mcp/tools/simulator/test_sim.ts index b2fa4486b..c748c4673 100644 --- a/src/mcp/tools/simulator/test_sim.ts +++ b/src/mcp/tools/simulator/test_sim.ts @@ -79,6 +79,14 @@ const baseSchemaObject = z.object({ configuration: z.string().optional().describe('Build configuration (Debug, Release, etc.)'), derivedDataPath: z.string().optional(), extraArgs: z.array(z.string()).optional(), + onlyTesting: z + .array(z.string()) + .optional() + .describe('Test identifiers to include (for example, Target/Suite/testMethod)'), + skipTesting: z + .array(z.string()) + .optional() + .describe('Test identifiers to exclude (for example, Target/Suite/testMethod)'), useLatestOS: z .boolean() .optional() @@ -118,12 +126,26 @@ interface PreparedTestSimExecution { warningMessage?: string; } +function resolveExtraArgs(params: TestSimulatorParams): string[] | undefined { + const selectorArgs = [ + ...(params.onlyTesting ?? []).map((selector) => `-only-testing:${selector}`), + ...(params.skipTesting ?? []).map((selector) => `-skip-testing:${selector}`), + ]; + + if (!params.extraArgs && selectorArgs.length === 0) { + return undefined; + } + + return [...(params.extraArgs ?? []), ...selectorArgs]; +} + async function prepareTestSimExecution( params: TestSimulatorParams, executor: CommandExecutor, fileSystemExecutor: FileSystemExecutor, ): Promise { const preparedTestSource = hasPreparedTestSource(params); + const extraArgs = resolveExtraArgs(params); const configuration = preparedTestSource ? undefined : params.configuration; const inferred = await inferPlatform( { @@ -182,7 +204,7 @@ async function prepareTestSimExecution( workspacePath: params.workspacePath, scheme: params.scheme!, configuration, - extraArgs: params.extraArgs, + extraArgs, destinationName, }, fileSystemExecutor, @@ -264,7 +286,7 @@ export function createTestSimExecutor( simulatorName: params.simulatorName, configuration: resolved.configuration, derivedDataPath: params.derivedDataPath, - extraArgs: params.extraArgs, + extraArgs: resolveExtraArgs(params), useLatestOS: false, preferXcodebuild: params.preferXcodebuild ?? false, platform: resolved.platform, @@ -294,29 +316,9 @@ export async function test_simLogic( setXcodebuildStructuredOutput(ctx, 'test-result', result, '3'); } -const publicSchemaObject = baseSchemaObject.omit({ - projectPath: true, - workspacePath: true, - scheme: true, - simulatorId: true, - simulatorName: true, - configuration: true, - useLatestOS: true, - derivedDataPath: true, - preferXcodebuild: true, -} as const); +const publicSchemaObject = baseSchemaObject; -const mcpPublicSchemaObject = mcpFullSchemaObject.omit({ - projectPath: true, - workspacePath: true, - scheme: true, - simulatorId: true, - simulatorName: true, - configuration: true, - useLatestOS: true, - derivedDataPath: true, - preferXcodebuild: true, -} as const); +const mcpPublicSchemaObject = mcpFullSchemaObject; export const schema = getSessionAwareToolSchemaShape({ sessionAware: publicSchemaObject, diff --git a/src/smoke-tests/__tests__/e2e-mcp-sessions.test.ts b/src/smoke-tests/__tests__/e2e-mcp-sessions.test.ts index 6b459b4c8..4f8889ac6 100644 --- a/src/smoke-tests/__tests__/e2e-mcp-sessions.test.ts +++ b/src/smoke-tests/__tests__/e2e-mcp-sessions.test.ts @@ -145,6 +145,71 @@ describe('MCP Session Management (e2e)', () => { expect(buildCommand).toContain('SessionScheme'); }); + it('explicit simulator build arguments override session defaults', async () => { + await harness.client.callTool({ + name: 'session_set_defaults', + arguments: { + scheme: 'DefaultScheme', + projectPath: '/default/project.xcodeproj', + simulatorId: 'AAAAAAAA-1111-2222-3333-444444444444', + configuration: 'Debug', + }, + }); + + harness.resetCapturedCommands(); + const result = await harness.client.callTool({ + name: 'build_sim', + arguments: { + scheme: 'ExplicitScheme', + projectPath: '/explicit/project.xcodeproj', + simulatorName: 'iPhone 17 Pro', + configuration: 'Release', + }, + }); + + expectContent(result); + const commandStrs = harness.capturedCommands.map((command) => command.command.join(' ')); + const buildCommand = commandStrs.find( + (command) => command.includes('xcodebuild') && command.includes('-scheme'), + ); + expect(buildCommand).toBeDefined(); + expect(buildCommand).toContain('ExplicitScheme'); + expect(buildCommand).toContain('/explicit/project.xcodeproj'); + expect(buildCommand).toContain('iPhone 17 Pro'); + expect(buildCommand).toContain('Release'); + expect(buildCommand).not.toContain('DefaultScheme'); + expect(buildCommand).not.toContain('/default/project.xcodeproj'); + }); + + it('accepts typed simulator test selectors', async () => { + await harness.client.callTool({ + name: 'session_set_defaults', + arguments: { + scheme: 'SessionScheme', + projectPath: '/session/project.xcodeproj', + simulatorId: 'AAAAAAAA-1111-2222-3333-444444444444', + }, + }); + + harness.resetCapturedCommands(); + const result = await harness.client.callTool({ + name: 'test_sim', + arguments: { + onlyTesting: ['MyTests/LoginTests/testSuccess'], + skipTesting: ['MyTests/LoginTests/testFailure'], + }, + }); + + expectContent(result); + const commandStrs = harness.capturedCommands.map((command) => command.command.join(' ')); + const testCommand = commandStrs.find( + (command) => command.includes('xcodebuild') && command.includes(' test'), + ); + expect(testCommand).toBeDefined(); + expect(testCommand).toContain('-only-testing:MyTests/LoginTests/testSuccess'); + expect(testCommand).toContain('-skip-testing:MyTests/LoginTests/testFailure'); + }); + it('updating session defaults changes subsequent tool behavior', async () => { // Set initial defaults await harness.client.callTool({ diff --git a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-disabled/test_sim.json b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-disabled/test_sim.json index a2ed4881f..f880c8e9d 100644 --- a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-disabled/test_sim.json +++ b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-disabled/test_sim.json @@ -11,12 +11,22 @@ "items": {"type":"string"}, "type": "array" }, + "onlyTesting": { + "description": "Test identifiers to include (for example, Target/Suite/testMethod)", + "items": {"type":"string"}, + "type": "array" + }, "preferXcodebuild": {"type":"boolean"}, "progress": {"description":"Show detailed test progress output (MCP defaults to true, CLI defaults to false)","type":"boolean"}, "projectPath": {"description":"Path to .xcodeproj file. Provide EITHER this OR workspacePath, not both","type":"string"}, "scheme": {"description":"The scheme to use in source mode","type":"string"}, "simulatorId": {"description":"UUID of the simulator (from list_sims). Provide EITHER this OR simulatorName, not both","type":"string"}, "simulatorName": {"description":"Name of the simulator (e.g., 'iPhone 17'). Provide EITHER this OR simulatorId, not both","type":"string"}, + "skipTesting": { + "description": "Test identifiers to exclude (for example, Target/Suite/testMethod)", + "items": {"type":"string"}, + "type": "array" + }, "testProductsPath": {"description":"Path to a prepared .xctestproducts package. Cannot be combined with source inputs","type":"string"}, "testRunnerEnv": { "description": "Environment variables to pass to the test runner (TEST_RUNNER_ prefix added automatically)", diff --git a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_run_sim.json b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_run_sim.json index aea99909c..fd5477d14 100644 --- a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_run_sim.json +++ b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_run_sim.json @@ -5,6 +5,8 @@ "inputSchema": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "configuration": {"description":"Build configuration (Debug, Release, etc.)","type":"string"}, + "derivedDataPath": {"type":"string"}, "extraArgs": { "description": "Additional xcodebuild/build-settings arguments (not app launch arguments)", "items": {"type":"string"}, @@ -14,7 +16,14 @@ "description": "Arguments passed to the launched app process on simulator runtime", "items": {"type":"string"}, "type": "array" - } + }, + "preferXcodebuild": {"type":"boolean"}, + "projectPath": {"description":"Path to .xcodeproj file. Provide EITHER this OR workspacePath, not both","type":"string"}, + "scheme": {"description":"The scheme to use (Required)","type":"string"}, + "simulatorId": {"description":"UUID of the simulator (from list_sims). Provide EITHER this OR simulatorName, not both","type":"string"}, + "simulatorName": {"description":"Name of the simulator (e.g., 'iPhone 17'). Provide EITHER this OR simulatorId, not both","type":"string"}, + "useLatestOS": {"description":"Whether to use the latest OS version for the named simulator","type":"boolean"}, + "workspacePath": {"description":"Path to .xcworkspace file. Provide EITHER this OR projectPath, not both","type":"string"} }, "type": "object" }, diff --git a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_sim.json b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_sim.json index 0e7aa8f76..715b94ce0 100644 --- a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_sim.json +++ b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/build_sim.json @@ -6,11 +6,20 @@ "$schema": "http://json-schema.org/draft-07/schema#", "properties": { "buildForTesting": {"description":"Build reusable test products without running tests (default: false)","type":"boolean"}, + "configuration": {"description":"Build configuration (Debug, Release, etc.)","type":"string"}, + "derivedDataPath": {"type":"string"}, "extraArgs": { "items": {"type":"string"}, "type": "array" }, - "testProductsPath": {"description":"Output path for the .xctestproducts bundle when buildForTesting is true","type":"string"} + "preferXcodebuild": {"type":"boolean"}, + "projectPath": {"description":"Path to .xcodeproj file. Provide EITHER this OR workspacePath, not both","type":"string"}, + "scheme": {"description":"The scheme to use (Required)","type":"string"}, + "simulatorId": {"description":"UUID of the simulator (from list_sims). Provide EITHER this OR simulatorName, not both","type":"string"}, + "simulatorName": {"description":"Name of the simulator (e.g., 'iPhone 17'). Provide EITHER this OR simulatorId, not both","type":"string"}, + "testProductsPath": {"description":"Output path for the .xctestproducts bundle when buildForTesting is true","type":"string"}, + "useLatestOS": {"description":"Whether to use the latest OS version for the named simulator","type":"boolean"}, + "workspacePath": {"description":"Path to .xcworkspace file. Provide EITHER this OR projectPath, not both","type":"string"} }, "type": "object" }, diff --git a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/test_sim.json b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/test_sim.json index a9c02ce97..f880c8e9d 100644 --- a/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/test_sim.json +++ b/src/snapshot-tests/__fixtures__/mcp-contracts/session-defaults-enabled/test_sim.json @@ -5,11 +5,28 @@ "inputSchema": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "configuration": {"description":"Build configuration (Debug, Release, etc.)","type":"string"}, + "derivedDataPath": {"type":"string"}, "extraArgs": { "items": {"type":"string"}, "type": "array" }, + "onlyTesting": { + "description": "Test identifiers to include (for example, Target/Suite/testMethod)", + "items": {"type":"string"}, + "type": "array" + }, + "preferXcodebuild": {"type":"boolean"}, "progress": {"description":"Show detailed test progress output (MCP defaults to true, CLI defaults to false)","type":"boolean"}, + "projectPath": {"description":"Path to .xcodeproj file. Provide EITHER this OR workspacePath, not both","type":"string"}, + "scheme": {"description":"The scheme to use in source mode","type":"string"}, + "simulatorId": {"description":"UUID of the simulator (from list_sims). Provide EITHER this OR simulatorName, not both","type":"string"}, + "simulatorName": {"description":"Name of the simulator (e.g., 'iPhone 17'). Provide EITHER this OR simulatorId, not both","type":"string"}, + "skipTesting": { + "description": "Test identifiers to exclude (for example, Target/Suite/testMethod)", + "items": {"type":"string"}, + "type": "array" + }, "testProductsPath": {"description":"Path to a prepared .xctestproducts package. Cannot be combined with source inputs","type":"string"}, "testRunnerEnv": { "description": "Environment variables to pass to the test runner (TEST_RUNNER_ prefix added automatically)", @@ -24,6 +41,8 @@ }, "type": "array" }, + "useLatestOS": {"description":"Whether to use the latest OS version for the named simulator","type":"boolean"}, + "workspacePath": {"description":"Path to .xcworkspace file. Provide EITHER this OR projectPath, not both","type":"string"}, "xctestrunPath": {"description":"Path to a prepared .xctestrun file. Cannot be combined with source inputs","type":"string"} }, "type": "object"