Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
34 changes: 27 additions & 7 deletions src/mcp/tools/simulator/__tests__/build_run_sim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(),
);
});
});

Expand Down
32 changes: 29 additions & 3 deletions src/mcp/tools/simulator/__tests__/build_sim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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', () => {
Expand Down
35 changes: 32 additions & 3 deletions src/mcp/tools/simulator/__tests__/test_sim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(),
);
});
});
Expand Down
10 changes: 1 addition & 9 deletions src/mcp/tools/simulator/build_run_sim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 1 addition & 9 deletions src/mcp/tools/simulator/build_sim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
50 changes: 26 additions & 24 deletions src/mcp/tools/simulator/test_sim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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<PreparedTestSimExecution> {
const preparedTestSource = hasPreparedTestSource(params);
const extraArgs = resolveExtraArgs(params);
const configuration = preparedTestSource ? undefined : params.configuration;
const inferred = await inferPlatform(
{
Expand Down Expand Up @@ -182,7 +204,7 @@ async function prepareTestSimExecution(
workspacePath: params.workspacePath,
scheme: params.scheme!,
configuration,
extraArgs: params.extraArgs,
extraArgs,
destinationName,
},
fileSystemExecutor,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
65 changes: 65 additions & 0 deletions src/smoke-tests/__tests__/e2e-mcp-sessions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
Loading