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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## [Unreleased]

### Fixed

- Fixed `debug_attach_sim` so an explicit `pid` overrides an inherited `bundleId` session default before mutual-exclusion validation ([#410](https://github.com/getsentry/XcodeBuildMCP/issues/410)).

## [2.5.1]

### Fixed
Expand Down
46 changes: 44 additions & 2 deletions src/mcp/tools/debugging/__tests__/debugging-tools.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
import { sessionStore } from '../../../../utils/session-store.ts';
import { DebuggerManager, type DebuggerToolContext } from '../../../../utils/debugger/index.ts';
import {
DebuggerManager,
__clearTestDebuggerToolContextOverride,
__setTestDebuggerToolContextOverride,
type DebuggerToolContext,
} from '../../../../utils/debugger/index.ts';
import type { DebuggerBackend } from '../../../../utils/debugger/backends/DebuggerBackend.ts';
import type { BreakpointSpec, DebugSessionInfo } from '../../../../utils/debugger/types.ts';

Expand Down Expand Up @@ -104,6 +109,10 @@ describe('debug_attach_sim', () => {
sessionStore.clear();
});

afterEach(() => {
__clearTestDebuggerToolContextOverride();
});

describe('Export Field Validation', () => {
it('should have handler function', () => {
expect(typeof attachHandler).toBe('function');
Expand All @@ -124,6 +133,39 @@ describe('debug_attach_sim', () => {
const text = result.content[0].text;
expect(text).toContain('simulatorId');
});

it('uses explicit pid instead of inherited bundleId session default', async () => {
const ctx = createTestContext();
__setTestDebuggerToolContextOverride(ctx);

sessionStore.setDefaults({
simulatorId: 'test-sim-uuid',
bundleId: 'com.default.app',
});

const result = await attachHandler({ pid: 1234 });

expect(result.isError).toBeFalsy();
const text = result.content[0].text;
expect(text).toContain('Attached');
expect(text).toContain('1234');
});

it('rejects when pid and bundleId are both explicit', async () => {
__setTestDebuggerToolContextOverride(createTestContext());
sessionStore.setDefaults({ simulatorId: 'test-sim-uuid' });

const result = await attachHandler({
pid: 1234,
bundleId: 'com.test.app',
});

expect(result.isError).toBe(true);
const text = result.content[0].text;
expect(text).toContain('Mutually exclusive parameters provided');
expect(text).toContain('bundleId');
expect(text).toContain('pid');
});
});

describe('Logic Behavior', () => {
Expand Down
5 changes: 4 additions & 1 deletion src/mcp/tools/debugging/debug_attach_sim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ export const handler = createSessionAwareToolWithContext<DebugAttachSimParams, D
requirements: [
{ oneOf: ['simulatorId', 'simulatorName'], message: 'Provide simulatorId or simulatorName' },
],
exclusivePairs: [['simulatorId', 'simulatorName']],
exclusivePairs: [
['simulatorId', 'simulatorName'],
['bundleId', 'pid'],
],
},
);
35 changes: 35 additions & 0 deletions src/utils/__tests__/session-aware-tool-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,41 @@ describe('createSessionAwareTool', () => {
expect(msg).toContain('workspacePath');
});

it('allows exclusive pairs to include tool-local keys that are not session defaults', async () => {
const attachSchema = z
.object({
simulatorId: z.string(),
bundleId: z.string().optional(),
pid: z.number().optional(),
})
.refine((v) => !(v.bundleId && v.pid), {
message: 'bundleId and pid are mutually exclusive',
});

const attachHandler = createSessionAwareTool<z.infer<typeof attachSchema>>({
internalSchema: attachSchema,
logicFunction: async (params) => {
const ctx = getHandlerContext();
ctx.emit(statusFragment('success', JSON.stringify(params)));
},
getExecutor: () => createMockExecutor({ success: true }),
requirements: [{ allOf: ['simulatorId'] }],
exclusivePairs: [['bundleId', 'pid']],
});

sessionStore.setDefaults({
simulatorId: 'SIM-123',
bundleId: 'com.default.app',
});

const result = await invokeAndCollect(attachHandler, { pid: 1234 });
expect(result.isError).toBe(false);

const parsed = JSON.parse(result.text.replace(/\n/g, '').replace(/^.*?(\{.*\}).*$/, '$1'));
expect(parsed).toMatchObject({ simulatorId: 'SIM-123', pid: 1234 });
expect(parsed.bundleId).toBeUndefined();
});

it('prefers first key when both values of exclusive pair come from session defaults', async () => {
const echoHandler = createSessionAwareTool<Params>({
internalSchema: z.object({
Expand Down
4 changes: 3 additions & 1 deletion src/utils/session-default-args.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { SessionDefaults } from './session-store.ts';

export type ExclusiveParameterGroup = readonly string[];

export function hasConcreteSessionDefaultValue(value: unknown): boolean {
if (value === null || value === undefined) {
return false;
Expand Down Expand Up @@ -31,7 +33,7 @@ export function pickSessionDefaultsForKeys(
export function mergeSessionDefaultArgs(opts: {
defaults: Record<string, unknown>;
explicitArgs: Record<string, unknown>;
exclusivePairs?: (keyof SessionDefaults)[][];
exclusivePairs?: readonly ExclusiveParameterGroup[];
}): Record<string, unknown> {
const sanitizedArgs: Record<string, unknown> = {};
for (const [key, value] of Object.entries(opts.explicitArgs)) {
Expand Down
8 changes: 4 additions & 4 deletions src/utils/typed-tool-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { setStructuredErrorOutput } from './structured-error.ts';

import { sessionStore, type SessionDefaults } from './session-store.ts';
import { isSessionDefaultsOptOutEnabled } from './environment.ts';
import { mergeSessionDefaultArgs } from './session-default-args.ts';
import { mergeSessionDefaultArgs, type ExclusiveParameterGroup } from './session-default-args.ts';

/**
* Result returned by tool handlers when invoked without a ToolHandlerContext
Expand Down Expand Up @@ -206,7 +206,7 @@ export function createSessionAwareTool<TParams>(opts: {
logicFunction: (params: TParams, executor: CommandExecutor) => Promise<void>;
getExecutor: () => CommandExecutor;
requirements?: SessionRequirement[];
exclusivePairs?: (keyof SessionDefaults)[][];
exclusivePairs?: readonly ExclusiveParameterGroup[];
}): ToolHandler {
return createSessionAwareHandler({
internalSchema: opts.internalSchema,
Expand All @@ -222,7 +222,7 @@ export function createSessionAwareToolWithContext<TParams, TContext>(opts: {
logicFunction: (params: TParams, context: TContext) => Promise<void>;
getContext: () => TContext;
requirements?: SessionRequirement[];
exclusivePairs?: (keyof SessionDefaults)[][];
exclusivePairs?: readonly ExclusiveParameterGroup[];
}): ToolHandler {
return createSessionAwareHandler(opts);
}
Expand All @@ -232,7 +232,7 @@ function createSessionAwareHandler<TParams, TContext>(opts: {
logicFunction: (params: TParams, context: TContext) => Promise<void>;
getContext: () => TContext;
requirements?: SessionRequirement[];
exclusivePairs?: (keyof SessionDefaults)[][];
exclusivePairs?: readonly ExclusiveParameterGroup[];
}): ToolHandler {
const {
internalSchema,
Expand Down
Loading