diff --git a/CHANGELOG.md b/CHANGELOG.md index a5949b3b9..bfe73e9f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/mcp/tools/debugging/__tests__/debugging-tools.test.ts b/src/mcp/tools/debugging/__tests__/debugging-tools.test.ts index 2ee3c4ef4..9493b8e55 100644 --- a/src/mcp/tools/debugging/__tests__/debugging-tools.test.ts +++ b/src/mcp/tools/debugging/__tests__/debugging-tools.test.ts @@ -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'; @@ -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'); @@ -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', () => { diff --git a/src/mcp/tools/debugging/debug_attach_sim.ts b/src/mcp/tools/debugging/debug_attach_sim.ts index fa0305d2c..8d256f767 100644 --- a/src/mcp/tools/debugging/debug_attach_sim.ts +++ b/src/mcp/tools/debugging/debug_attach_sim.ts @@ -293,6 +293,9 @@ export const handler = createSessionAwareToolWithContext { 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>({ + 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({ internalSchema: z.object({ diff --git a/src/utils/session-default-args.ts b/src/utils/session-default-args.ts index cdf12dc84..d16a1810e 100644 --- a/src/utils/session-default-args.ts +++ b/src/utils/session-default-args.ts @@ -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; @@ -31,7 +33,7 @@ export function pickSessionDefaultsForKeys( export function mergeSessionDefaultArgs(opts: { defaults: Record; explicitArgs: Record; - exclusivePairs?: (keyof SessionDefaults)[][]; + exclusivePairs?: readonly ExclusiveParameterGroup[]; }): Record { const sanitizedArgs: Record = {}; for (const [key, value] of Object.entries(opts.explicitArgs)) { diff --git a/src/utils/typed-tool-factory.ts b/src/utils/typed-tool-factory.ts index a6690caa8..d1f43f223 100644 --- a/src/utils/typed-tool-factory.ts +++ b/src/utils/typed-tool-factory.ts @@ -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 @@ -206,7 +206,7 @@ export function createSessionAwareTool(opts: { logicFunction: (params: TParams, executor: CommandExecutor) => Promise; getExecutor: () => CommandExecutor; requirements?: SessionRequirement[]; - exclusivePairs?: (keyof SessionDefaults)[][]; + exclusivePairs?: readonly ExclusiveParameterGroup[]; }): ToolHandler { return createSessionAwareHandler({ internalSchema: opts.internalSchema, @@ -222,7 +222,7 @@ export function createSessionAwareToolWithContext(opts: { logicFunction: (params: TParams, context: TContext) => Promise; getContext: () => TContext; requirements?: SessionRequirement[]; - exclusivePairs?: (keyof SessionDefaults)[][]; + exclusivePairs?: readonly ExclusiveParameterGroup[]; }): ToolHandler { return createSessionAwareHandler(opts); } @@ -232,7 +232,7 @@ function createSessionAwareHandler(opts: { logicFunction: (params: TParams, context: TContext) => Promise; getContext: () => TContext; requirements?: SessionRequirement[]; - exclusivePairs?: (keyof SessionDefaults)[][]; + exclusivePairs?: readonly ExclusiveParameterGroup[]; }): ToolHandler { const { internalSchema,