Skip to content
Merged
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
16 changes: 14 additions & 2 deletions src/test-utils/mock-executors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export function createMockExecutor(
output?: string;
error?: string;
process?: unknown;
exitCode?: number;
shouldThrow?: Error;
}
| Error
| string,
Expand All @@ -42,6 +44,13 @@ export function createMockExecutor(
};
}

// If shouldThrow is specified, return executor that rejects with that error
if (result.shouldThrow) {
return async () => {
throw result.shouldThrow;
};
}

const mockProcess = {
pid: 12345,
stdout: null,
Expand All @@ -50,7 +59,7 @@ export function createMockExecutor(
stdio: [null, null, null],
killed: false,
connected: false,
exitCode: result.success === false ? 1 : 0,
exitCode: result.exitCode ?? (result.success === false ? 1 : 0),
signalCode: null,
spawnargs: [],
spawnfile: 'sh',
Expand All @@ -61,6 +70,7 @@ export function createMockExecutor(
output: result.output ?? '',
error: result.error,
process: (result.process ?? mockProcess) as ChildProcess,
exitCode: result.exitCode ?? (result.success === false ? 1 : 0),
});
}

Expand Down Expand Up @@ -104,6 +114,7 @@ export function createCommandMatchingMockExecutor(
output?: string;
error?: string;
process?: unknown;
exitCode?: number;
}
>,
): CommandExecutor {
Expand Down Expand Up @@ -132,7 +143,7 @@ export function createCommandMatchingMockExecutor(
stdio: [null, null, null],
killed: false,
connected: false,
exitCode: result.success === false ? 1 : 0,
exitCode: result.exitCode ?? (result.success === false ? 1 : 0),
signalCode: null,
spawnargs: [],
spawnfile: 'sh',
Expand All @@ -143,6 +154,7 @@ export function createCommandMatchingMockExecutor(
output: result.output ?? '',
error: result.error,
process: (result.process ?? mockProcess) as ChildProcess,
exitCode: result.exitCode ?? (result.success === false ? 1 : 0),
};
};
}
Expand Down
1 change: 1 addition & 0 deletions src/utils/CommandExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export interface CommandResponse {
output: string;
error?: string;
process: ChildProcess;
exitCode?: number;
}
263 changes: 263 additions & 0 deletions src/utils/__tests__/build-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
/**
* Tests for build-utils Sentry classification logic
*/

import { describe, it, expect } from 'vitest';
import { createMockExecutor } from '../../test-utils/mock-executors.ts';
import { executeXcodeBuildCommand } from '../build-utils.ts';
import { XcodePlatform } from '../xcode.ts';

describe('build-utils Sentry Classification', () => {
const mockPlatformOptions = {
platform: XcodePlatform.macOS,
logPrefix: 'Test Build',
};

const mockParams = {
scheme: 'TestScheme',
configuration: 'Debug',
projectPath: '/path/to/project.xcodeproj',
};

describe('Exit Code 64 Classification (MCP Error)', () => {
it('should trigger Sentry logging for exit code 64 (invalid arguments)', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'xcodebuild: error: invalid option',
exitCode: 64,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] xcodebuild: error: invalid option');
expect(result.content[1].text).toContain('❌ Test Build build failed for scheme TestScheme');
});
});

describe('Other Exit Codes Classification (User Error)', () => {
it('should not trigger Sentry logging for exit code 65 (user error)', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'Scheme TestScheme was not found',
exitCode: 65,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] Scheme TestScheme was not found');
expect(result.content[1].text).toContain('❌ Test Build build failed for scheme TestScheme');
});

it('should not trigger Sentry logging for exit code 66 (file not found)', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'project.xcodeproj cannot be opened',
exitCode: 66,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] project.xcodeproj cannot be opened');
});

it('should not trigger Sentry logging for exit code 70 (destination error)', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'Unable to find a destination matching the provided destination specifier',
exitCode: 70,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] Unable to find a destination matching');
});

it('should not trigger Sentry logging for exit code 1 (general build failure)', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'Build failed with errors',
exitCode: 1,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] Build failed with errors');
});
});

describe('Spawn Error Classification (Environment Error)', () => {
it('should not trigger Sentry logging for ENOENT spawn error', async () => {
const spawnError = new Error('spawn xcodebuild ENOENT') as NodeJS.ErrnoException;
spawnError.code = 'ENOENT';

const mockExecutor = createMockExecutor({
success: false,
error: '',
shouldThrow: spawnError,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain(
'Error during Test Build build: spawn xcodebuild ENOENT',
);
});

it('should not trigger Sentry logging for EACCES spawn error', async () => {
const spawnError = new Error('spawn xcodebuild EACCES') as NodeJS.ErrnoException;
spawnError.code = 'EACCES';

const mockExecutor = createMockExecutor({
success: false,
error: '',
shouldThrow: spawnError,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain(
'Error during Test Build build: spawn xcodebuild EACCES',
);
});

it('should not trigger Sentry logging for EPERM spawn error', async () => {
const spawnError = new Error('spawn xcodebuild EPERM') as NodeJS.ErrnoException;
spawnError.code = 'EPERM';

const mockExecutor = createMockExecutor({
success: false,
error: '',
shouldThrow: spawnError,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain(
'Error during Test Build build: spawn xcodebuild EPERM',
);
});

it('should trigger Sentry logging for non-spawn exceptions', async () => {
const otherError = new Error('Unexpected internal error');

const mockExecutor = createMockExecutor({
success: false,
error: '',
shouldThrow: otherError,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain(
'Error during Test Build build: Unexpected internal error',
);
});
});

describe('Success Case (No Sentry Logging)', () => {
it('should not trigger any error logging for successful builds', async () => {
const mockExecutor = createMockExecutor({
success: true,
output: 'BUILD SUCCEEDED',
exitCode: 0,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBeFalsy();
expect(result.content[0].text).toContain(
'✅ Test Build build succeeded for scheme TestScheme',
);
});
});

describe('Exit Code Undefined Cases', () => {
it('should not trigger Sentry logging when exitCode is undefined', async () => {
const mockExecutor = createMockExecutor({
success: false,
error: 'Some error without exit code',
exitCode: undefined,
});

const result = await executeXcodeBuildCommand(
mockParams,
mockPlatformOptions,
false,
'build',
mockExecutor,
);

expect(result.isError).toBe(true);
expect(result.content[0].text).toContain('❌ [stderr] Some error without exit code');
});
});
});
19 changes: 16 additions & 3 deletions src/utils/build-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,13 @@ export async function executeXcodeBuildCommand(
}

if (!result.success) {
log('error', `${platformOptions.logPrefix} ${buildAction} failed: ${result.error}`);
const isMcpError = result.exitCode === 64;

// Create concise error response with warnings/errors included
log(
isMcpError ? 'error' : 'warning',
`${platformOptions.logPrefix} ${buildAction} failed: ${result.error}`,
{ sentry: isMcpError },
);
const errorResponse = createTextResponse(
`❌ ${platformOptions.logPrefix} ${buildAction} failed for scheme ${params.scheme}.`,
true,
Expand Down Expand Up @@ -339,7 +343,16 @@ Future builds will use the generated Makefile for improved performance.
return consolidateContentForClaudeCode(successResponse);
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error);
log('error', `Error during ${platformOptions.logPrefix} ${buildAction}: ${errorMessage}`);

const isSpawnError =
error instanceof Error &&
'code' in error &&
['ENOENT', 'EACCES', 'EPERM'].includes((error as NodeJS.ErrnoException).code ?? '');

log('error', `Error during ${platformOptions.logPrefix} ${buildAction}: ${errorMessage}`, {
sentry: !isSpawnError,
});

return consolidateContentForClaudeCode(
createTextResponse(
`Error during ${platformOptions.logPrefix} ${buildAction}: ${errorMessage}`,
Expand Down
1 change: 1 addition & 0 deletions src/utils/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ async function defaultExecutor(
output: stdout,
error: success ? undefined : stderr,
process: childProcess,
exitCode: code ?? undefined,
};

resolve(response);
Expand Down
Loading