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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Fixed `suppressWarnings` being ignored in settled build, build-run, and test output. The flag was honored only while streaming, so warnings still reached the final MCP tool response ([#447](https://github.com/getsentry/XcodeBuildMCP/issues/447)).
- Fixed iOS scaffold orientation and device-family settings, LLDB command isolation and argument escaping, run-destination parsing without an active scheme, concurrent working-directory mutations, blocking physical-device name lookup, and unverified `xcodemake` downloads ([#459](https://github.com/getsentry/XcodeBuildMCP/issues/459)).
- Fixed simulator UI launching and keyboard controls to prefer Xcode 27's Device Hub when available, with Simulator.app as the legacy fallback.
- Fixed `stop_mac_app` app-name targeting so it no longer terminates unrelated processes whose command lines contain the app name, and reject unsafe process IDs before execution ([#306](https://github.com/getsentry/XcodeBuildMCP/issues/306)).
- Fixed incremental `xcodemake` builds when DerivedData uses an absolute path by updating the pinned wrapper and delegating Makefile reuse to it so its argument and freshness checks are always applied ([#466](https://github.com/getsentry/XcodeBuildMCP/issues/466)).

## [2.6.2]
Expand Down
118 changes: 90 additions & 28 deletions src/mcp/tools/macos/__tests__/stop_mac_app.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { describe, it, expect } from 'vitest';
import { schema, handler, stop_mac_appLogic } from '../stop_mac_app.ts';
import { allText, runLogic } from '../../../../test-utils/test-helpers.ts';
import {
allText,
createMockToolHandlerContext,
runLogic,
} from '../../../../test-utils/test-helpers.ts';
import { createMockExecutor, createNoopExecutor } from '../../../../test-utils/mock-executors.ts';

describe('stop_mac_app plugin', () => {
describe('Export Field Validation (Literal)', () => {
Expand All @@ -17,28 +22,53 @@ describe('stop_mac_app plugin', () => {

// Test invalid inputs
expect(schema.appName.safeParse(null).success).toBe(false);
expect(schema.appName.safeParse('').success).toBe(false);
expect(schema.processId.safeParse('not-number').success).toBe(false);
expect(schema.processId.safeParse(null).success).toBe(false);
expect(schema.processId.safeParse(0).success).toBe(false);
expect(schema.processId.safeParse(-1).success).toBe(false);
expect(schema.processId.safeParse(1.5).success).toBe(false);
expect(schema.processId.safeParse(Number.NaN).success).toBe(false);
expect(schema.processId.safeParse(Number.MAX_SAFE_INTEGER + 1).success).toBe(false);
});
});

describe('Input Validation', () => {
it('should return exact validation error for missing parameters', async () => {
const mockExecutor = async () => ({ success: true, output: '', process: {} as any });
const result = await runLogic(() => stop_mac_appLogic({}, mockExecutor));
const result = await runLogic(() => stop_mac_appLogic({}, createNoopExecutor()));

expect(result.isError).toBe(true);
expect(allText(result)).toContain('appName or processId');
});

it.each([0, -1, 1.5, Number.NaN, Number.MAX_SAFE_INTEGER + 1])(
'should reject unsafe process ID %s at the execution boundary',
async (processId) => {
const calls: string[][] = [];
const executor = createMockExecutor({ onExecute: (command) => calls.push(command) });
const { ctx, result, run } = createMockToolHandlerContext();

await run(() => stop_mac_appLogic({ processId }, executor));

expect(result.isError()).toBe(true);
expect(result.text()).toContain('processId must be a positive safe integer');
const structuredResult = ctx.structuredOutput?.result;
expect(structuredResult?.kind).toBe('stop-result');
if (structuredResult?.kind !== 'stop-result') {
throw new Error('Expected stop-result structured output.');
}
expect(structuredResult.artifacts).toEqual({ appName: '' });
expect(calls).toHaveLength(0);
},
);
});

describe('Command Generation', () => {
it('should generate correct command for process ID', async () => {
const calls: any[] = [];
const mockExecutor = async (command: string[]) => {
calls.push({ command });
return { success: true, output: '', process: {} as any };
};
const calls: string[][] = [];
const mockExecutor = createMockExecutor({
onExecute: (command) => calls.push(command),
});

await runLogic(() =>
stop_mac_appLogic(
Expand All @@ -50,35 +80,69 @@ describe('stop_mac_app plugin', () => {
);

expect(calls).toHaveLength(1);
expect(calls[0].command).toEqual(['kill', '1234']);
expect(calls[0]).toEqual(['kill', '1234']);
});

it('should generate correct command for app name', async () => {
const calls: any[] = [];
const mockExecutor = async (command: string[]) => {
calls.push({ command });
return { success: true, output: '', process: {} as any };
};
it('should target app names by literal process name', async () => {
const calls: string[][] = [];
const mockExecutor = createMockExecutor({
onExecute: (command) => calls.push(command),
});

await runLogic(() =>
stop_mac_appLogic(
{
appName: 'Calculator',
appName: 'Brimday',
},
mockExecutor,
),
);

expect(calls).toHaveLength(1);
expect(calls[0].command).toEqual(['pkill', '-f', 'Calculator']);
expect(calls[0]).toEqual(['killall', '--', 'Brimday']);
});

it('should pass long app executable names to killall', async () => {
const calls: string[][] = [];
const mockExecutor = createMockExecutor({
onExecute: (command) => calls.push(command),
});

await runLogic(() =>
stop_mac_appLogic(
{
appName: 'ThisIsAVeryLongApplicationName',
},
mockExecutor,
),
);

expect(calls[0]).toEqual(['killall', '--', 'ThisIsAVeryLongApplicationName']);
});

it('should treat app names as literal process names', async () => {
const calls: string[][] = [];
const mockExecutor = createMockExecutor({
onExecute: (command) => calls.push(command),
});

await runLogic(() =>
stop_mac_appLogic(
{
appName: '-Example.*[Test]',
},
mockExecutor,
),
);

expect(calls[0]).toEqual(['killall', '--', '-Example.*[Test]']);
});

it('should prioritize processId over appName', async () => {
const calls: any[] = [];
const mockExecutor = async (command: string[]) => {
calls.push({ command });
return { success: true, output: '', process: {} as any };
};
const calls: string[][] = [];
const mockExecutor = createMockExecutor({
onExecute: (command) => calls.push(command),
});

await runLogic(() =>
stop_mac_appLogic(
Expand All @@ -91,13 +155,13 @@ describe('stop_mac_app plugin', () => {
);

expect(calls).toHaveLength(1);
expect(calls[0].command).toEqual(['kill', '1234']);
expect(calls[0]).toEqual(['kill', '1234']);
});
});

describe('Response Processing', () => {
it('should return exact successful stop response by app name', async () => {
const mockExecutor = async () => ({ success: true, output: '', process: {} as any });
const mockExecutor = createMockExecutor({});

const result = await runLogic(() =>
stop_mac_appLogic(
Expand All @@ -112,7 +176,7 @@ describe('stop_mac_app plugin', () => {
});

it('should return exact successful stop response with both parameters (processId takes precedence)', async () => {
const mockExecutor = async () => ({ success: true, output: '', process: {} as any });
const mockExecutor = createMockExecutor({});

const result = await runLogic(() =>
stop_mac_appLogic(
Expand All @@ -128,9 +192,7 @@ describe('stop_mac_app plugin', () => {
});

it('should handle execution errors', async () => {
const mockExecutor = async () => {
throw new Error('Process not found');
};
const mockExecutor = createMockExecutor(new Error('Process not found'));

const result = await runLogic(() =>
stop_mac_appLogic(
Expand Down
28 changes: 21 additions & 7 deletions src/mcp/tools/macos/stop_mac_app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@ import {
} from '../../../utils/app-lifecycle-results.ts';

const stopMacAppSchema = z.object({
appName: z.string().optional(),
processId: z.number().optional(),
appName: z.string().min(1).optional(),
processId: z
.number()
.int()
.positive()
.refine(Number.isSafeInteger, 'processId must be a positive safe integer.')
.optional(),
});
Comment thread
cameroncooke marked this conversation as resolved.

type StopMacAppParams = z.infer<typeof stopMacAppSchema>;
Expand Down Expand Up @@ -54,20 +59,29 @@ export function createStopMacAppExecutor(
executor: CommandExecutor,
): NonStreamingExecutor<StopMacAppParams, StopMacAppResult> {
return async (params) => {
const artifacts = createStopMacAppArtifacts(params);

if (!params.appName && params.processId === undefined) {
return buildStopFailure(artifacts, 'Either appName or processId must be provided.');
return buildStopFailure({ appName: '' }, 'Either appName or processId must be provided.');
}

const target = params.processId ? `PID ${params.processId}` : params.appName!;
if (
params.processId !== undefined &&
(!Number.isSafeInteger(params.processId) || params.processId <= 0)
) {
return buildStopFailure(
{ appName: params.appName ?? '' },
'processId must be a positive safe integer.',
);
}

const artifacts = createStopMacAppArtifacts(params);
const target = params.processId !== undefined ? `PID ${params.processId}` : params.appName!;
log('info', `Stopping macOS app: ${target}`);

try {
const command =
params.processId !== undefined
? ['kill', String(params.processId)]
: ['pkill', '-f', params.appName!];
: ['killall', '--', params.appName!];
Comment thread
cameroncooke marked this conversation as resolved.
const result = await executor(command, 'Stop macOS App');
Comment thread
cameroncooke marked this conversation as resolved.

if (!result.success) {
Expand Down
6 changes: 3 additions & 3 deletions src/smoke-tests/__tests__/e2e-mcp-device-macos.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ beforeAll(async () => {
'xctrace list devices': { success: true, output: 'No devices found.' },
open: { success: true, output: '' },
kill: { success: true, output: '' },
pkill: { success: true, output: '' },
killall: { success: true, output: '' },
'defaults read': { success: true, output: 'io.sentry.MyApp' },
PlistBuddy: { success: true, output: 'io.sentry.MyApp' },
xcresulttool: { success: true, output: '{}' },
Expand Down Expand Up @@ -328,7 +328,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
expect(commandStrs.some((c) => c.includes('kill') && c.includes('54321'))).toBe(true);
});

it('stop_mac_app captures pkill command with appName', async () => {
it('stop_mac_app captures killall command with appName', async () => {
harness.resetCapturedCommands();
const result = await harness.client.callTool({
name: 'stop_mac_app',
Expand All @@ -338,7 +338,7 @@ describe('MCP Device and macOS Tool Invocation (e2e)', () => {
expectContent(result);

const commandStrs = harness.capturedCommands.map((c) => c.command.join(' '));
expect(commandStrs.some((c) => c.includes('MyMacApp'))).toBe(true);
expect(commandStrs.some((c) => c === 'killall -- MyMacApp')).toBe(true);
});

it('get_mac_app_path captures xcodebuild showBuildSettings command', async () => {
Expand Down
Loading