Skip to content

Commit f09579c

Browse files
cameroncookecodex
andcommitted
fix(build): Handle unexpected throws in build-only tools
Wrap build-only executor xcodebuild invocations in try/catch so unexpected execution exceptions are converted into failed build domain results. Add focused regression tests for device, simulator, and macOS build-only tools to confirm thrown execution errors no longer escape the handler. Fixes #334 Co-Authored-By: OpenAI Codex <codex@openai.com>
1 parent 54837d4 commit f09579c

7 files changed

Lines changed: 166 additions & 58 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+
- Added error handling around build-only tool execution paths (`build_device`, `build_sim`, `build_macos`) so unexpected execution throws are reported as failed build results instead of escaping the handler ([#334](https://github.com/getsentry/XcodeBuildMCP/issues/334)).
2627

2728
## [2.3.2]
2829

src/mcp/tools/device/__tests__/build_device.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { describe, it, expect, beforeEach } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { computeScopedDerivedDataPath } from '../../../../utils/derived-data-path.ts';
33
import * as z from 'zod';
44
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
55
import { expectPendingBuildResponse, runToolLogic } from '../../../../test-utils/test-helpers.ts';
66
import { schema, handler, buildDeviceLogic } from '../build_device.ts';
77
import { sessionStore } from '../../../../utils/session-store.ts';
8+
import * as buildUtils from '../../../../utils/build/index.ts';
89
import type { CommandExecutor } from '../../../../utils/execution/index.ts';
910

1011
const runHandlerWithExecutor = handler as unknown as (
@@ -30,6 +31,7 @@ function createSpyExecutor(): {
3031
describe('build_device plugin', () => {
3132
beforeEach(() => {
3233
sessionStore.clear();
34+
vi.restoreAllMocks();
3335
});
3436

3537
describe('Export Field Validation (Literal)', () => {
@@ -309,6 +311,26 @@ describe('build_device plugin', () => {
309311
expectPendingBuildResponse(result);
310312
});
311313

314+
it('should return error result when executeXcodeBuildCommand throws unexpectedly', async () => {
315+
const executeSpy = vi
316+
.spyOn(buildUtils, 'executeXcodeBuildCommand')
317+
.mockRejectedValueOnce(new Error('Unexpected build error'));
318+
319+
const { result } = await runToolLogic(() =>
320+
buildDeviceLogic(
321+
{
322+
projectPath: '/path/to/MyProject.xcodeproj',
323+
scheme: 'MyScheme',
324+
},
325+
createMockExecutor({ success: true, output: 'Build succeeded' }),
326+
),
327+
);
328+
329+
expect(executeSpy).toHaveBeenCalledOnce();
330+
expect(result.isError()).toBe(true);
331+
expectPendingBuildResponse(result);
332+
});
333+
312334
it('should include optional parameters in command', async () => {
313335
const spy = createSpyExecutor();
314336

src/mcp/tools/device/build_device.ts

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -81,29 +81,43 @@ export function createBuildDeviceExecutor(
8181
};
8282
const started = createDomainStreamingPipeline('build_device', 'BUILD', ctx, 'build-result');
8383

84-
const buildResult = await executeXcodeBuildCommand(
85-
processedParams,
86-
{
87-
platform,
88-
logPrefix: `${platform} Device Build`,
89-
},
90-
params.preferXcodebuild ?? false,
91-
'build',
92-
executor,
93-
undefined,
94-
started.pipeline,
95-
);
84+
try {
85+
const buildResult = await executeXcodeBuildCommand(
86+
processedParams,
87+
{
88+
platform,
89+
logPrefix: `${platform} Device Build`,
90+
},
91+
params.preferXcodebuild ?? false,
92+
'build',
93+
executor,
94+
undefined,
95+
started.pipeline,
96+
);
9697

97-
return createBuildDomainResult({
98-
started,
99-
succeeded: !buildResult.isError,
100-
target: 'device',
101-
artifacts: {
102-
buildLogPath: started.pipeline.logPath,
103-
},
104-
fallbackErrorMessages: collectFallbackErrorMessages(started, [], buildResult.content),
105-
request: createBuildDeviceRequest(params),
106-
});
98+
return createBuildDomainResult({
99+
started,
100+
succeeded: !buildResult.isError,
101+
target: 'device',
102+
artifacts: {
103+
buildLogPath: started.pipeline.logPath,
104+
},
105+
fallbackErrorMessages: collectFallbackErrorMessages(started, [], buildResult.content),
106+
request: createBuildDeviceRequest(params),
107+
});
108+
} catch (error) {
109+
const errorMessage = error instanceof Error ? error.message : String(error);
110+
return createBuildDomainResult({
111+
started,
112+
succeeded: false,
113+
target: 'device',
114+
artifacts: {
115+
buildLogPath: started.pipeline.logPath,
116+
},
117+
fallbackErrorMessages: collectFallbackErrorMessages(started, [errorMessage]),
118+
request: createBuildDeviceRequest(params),
119+
});
120+
}
107121
};
108122
}
109123

src/mcp/tools/macos/__tests__/build_macos.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { describe, it, expect, beforeEach } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { computeScopedDerivedDataPath } from '../../../../utils/derived-data-path.ts';
33
import * as z from 'zod';
44
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
55
import { expectPendingBuildResponse, runToolLogic } from '../../../../test-utils/test-helpers.ts';
66
import { sessionStore } from '../../../../utils/session-store.ts';
77
import { schema, handler, buildMacOSLogic } from '../build_macos.ts';
8+
import * as buildUtils from '../../../../utils/build/index.ts';
89

910
const runBuildMacOS = (
1011
params: Parameters<typeof buildMacOSLogic>[0],
@@ -29,6 +30,7 @@ function createSpyExecutor(): {
2930
describe('build_macos plugin', () => {
3031
beforeEach(() => {
3132
sessionStore.clear();
33+
vi.restoreAllMocks();
3234
});
3335

3436
describe('Export Field Validation (Literal)', () => {
@@ -150,6 +152,24 @@ describe('build_macos plugin', () => {
150152
});
151153
});
152154

155+
it('should return error result when executeXcodeBuildCommand throws unexpectedly', async () => {
156+
const executeSpy = vi
157+
.spyOn(buildUtils, 'executeXcodeBuildCommand')
158+
.mockRejectedValueOnce(new Error('Unexpected macOS build error'));
159+
160+
const { result } = await runBuildMacOS(
161+
{
162+
projectPath: '/path/to/MyProject.xcodeproj',
163+
scheme: 'MyScheme',
164+
},
165+
createMockExecutor({ success: true, output: 'BUILD SUCCEEDED' }),
166+
);
167+
168+
expect(executeSpy).toHaveBeenCalledOnce();
169+
expect(result.isError()).toBe(true);
170+
expectPendingBuildResponse(result);
171+
});
172+
153173
it('should return exact exception handling response', async () => {
154174
const mockExecutor = async () => {
155175
throw new Error('Network error');

src/mcp/tools/macos/build_macos.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,34 @@ export function createBuildMacOSExecutor(
7676
return async (params, ctx) => {
7777
const configuration = params.configuration ?? 'Debug';
7878
const started = createDomainStreamingPipeline('build_macos', 'BUILD', ctx, 'build-result');
79-
const buildResult = await executeXcodeBuildCommand(
80-
{ ...params, configuration },
81-
{
82-
platform: XcodePlatform.macOS,
83-
arch: params.arch,
84-
logPrefix: 'macOS Build',
85-
},
86-
params.preferXcodebuild ?? false,
87-
'build',
88-
executor,
89-
undefined,
90-
started.pipeline,
91-
);
79+
let buildResult;
80+
try {
81+
buildResult = await executeXcodeBuildCommand(
82+
{ ...params, configuration },
83+
{
84+
platform: XcodePlatform.macOS,
85+
arch: params.arch,
86+
logPrefix: 'macOS Build',
87+
},
88+
params.preferXcodebuild ?? false,
89+
'build',
90+
executor,
91+
undefined,
92+
started.pipeline,
93+
);
94+
} catch (error) {
95+
const errorMessage = error instanceof Error ? error.message : String(error);
96+
return createBuildDomainResult({
97+
started,
98+
succeeded: false,
99+
target: 'macos',
100+
artifacts: {
101+
buildLogPath: started.pipeline.logPath,
102+
},
103+
fallbackErrorMessages: collectFallbackErrorMessages(started, [errorMessage]),
104+
request: createBuildMacOSRequest(params),
105+
});
106+
}
92107

93108
let bundleId: string | undefined;
94109
if (!buildResult.isError) {

src/mcp/tools/simulator/__tests__/build_sim.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, beforeEach } from 'vitest';
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
22
import { computeScopedDerivedDataPath } from '../../../../utils/derived-data-path.ts';
33
import * as z from 'zod';
44
import {
@@ -7,6 +7,7 @@ import {
77
} from '../../../../test-utils/mock-executors.ts';
88
import { expectPendingBuildResponse, runToolLogic } from '../../../../test-utils/test-helpers.ts';
99
import { sessionStore } from '../../../../utils/session-store.ts';
10+
import * as buildUtils from '../../../../utils/build/index.ts';
1011

1112
import { schema, handler, build_simLogic } from '../build_sim.ts';
1213

@@ -18,6 +19,7 @@ const runBuildSimLogic = (
1819
describe('build_sim tool', () => {
1920
beforeEach(() => {
2021
sessionStore.clear();
22+
vi.restoreAllMocks();
2123
});
2224

2325
describe('Export Field Validation (Literal)', () => {
@@ -462,6 +464,25 @@ describe('build_sim tool', () => {
462464
expectPendingBuildResponse(result);
463465
});
464466

467+
it('should return error result when executeXcodeBuildCommand throws unexpectedly', async () => {
468+
const executeSpy = vi
469+
.spyOn(buildUtils, 'executeXcodeBuildCommand')
470+
.mockRejectedValueOnce(new Error('Unexpected simulator build error'));
471+
472+
const { result } = await runBuildSimLogic(
473+
{
474+
workspacePath: '/path/to/workspace',
475+
scheme: 'MyScheme',
476+
simulatorName: 'iPhone 17',
477+
},
478+
createMockExecutor({ success: true, output: 'BUILD SUCCEEDED' }),
479+
);
480+
481+
expect(executeSpy).toHaveBeenCalledOnce();
482+
expect(result.isError()).toBe(true);
483+
expectPendingBuildResponse(result);
484+
});
485+
465486
it('should handle build warnings', async () => {
466487
const mockExecutor = createMockExecutor({
467488
success: true,

src/mcp/tools/simulator/build_sim.ts

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -173,26 +173,41 @@ export function createBuildSimExecutor(
173173
}
174174

175175
const started = createDomainStreamingPipeline('build_sim', 'BUILD', ctx, 'build-result');
176-
const buildResult = await executeXcodeBuildCommand(
177-
resolved.sharedBuildParams,
178-
resolved.platformOptions,
179-
params.preferXcodebuild ?? false,
180-
'build',
181-
executor,
182-
undefined,
183-
started.pipeline,
184-
);
185-
186-
return createBuildDomainResult({
187-
started,
188-
succeeded: !buildResult.isError,
189-
target: 'simulator',
190-
artifacts: {
191-
buildLogPath: started.pipeline.logPath,
192-
},
193-
fallbackErrorMessages: collectFallbackErrorMessages(started, [], buildResult.content),
194-
request: resolved.invocationRequest,
195-
});
176+
177+
try {
178+
const buildResult = await executeXcodeBuildCommand(
179+
resolved.sharedBuildParams,
180+
resolved.platformOptions,
181+
params.preferXcodebuild ?? false,
182+
'build',
183+
executor,
184+
undefined,
185+
started.pipeline,
186+
);
187+
188+
return createBuildDomainResult({
189+
started,
190+
succeeded: !buildResult.isError,
191+
target: 'simulator',
192+
artifacts: {
193+
buildLogPath: started.pipeline.logPath,
194+
},
195+
fallbackErrorMessages: collectFallbackErrorMessages(started, [], buildResult.content),
196+
request: resolved.invocationRequest,
197+
});
198+
} catch (error) {
199+
const errorMessage = error instanceof Error ? error.message : String(error);
200+
return createBuildDomainResult({
201+
started,
202+
succeeded: false,
203+
target: 'simulator',
204+
artifacts: {
205+
buildLogPath: started.pipeline.logPath,
206+
},
207+
fallbackErrorMessages: collectFallbackErrorMessages(started, [errorMessage]),
208+
request: resolved.invocationRequest,
209+
});
210+
}
196211
};
197212
}
198213

0 commit comments

Comments
 (0)