diff --git a/CHANGELOG.md b/CHANGELOG.md index f47e609a..a08b9336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- Fixed `mobilebuildmcp device build` failing with `deviceId requires buildForTesting to be true` when a phone is saved in session defaults. Regular builds ignore the saved installation destination, while explicit device arguments and installation commands retain their existing behavior. + ## [2.7.1] ### Changed diff --git a/src/cli/__tests__/register-tool-commands.test.ts b/src/cli/__tests__/register-tool-commands.test.ts index 7fdd606c..bf759af7 100644 --- a/src/cli/__tests__/register-tool-commands.test.ts +++ b/src/cli/__tests__/register-tool-commands.test.ts @@ -69,6 +69,7 @@ const baseRuntimeConfig: ResolvedRuntimeConfig = { }; function createApp(catalog: ToolCatalog, runtimeConfig: ResolvedRuntimeConfig = baseRuntimeConfig) { + const workflows = [...new Set(catalog.tools.map((tool) => tool.workflow))]; const app = yargs() .scriptName('mobilebuildmcp') .exitProcess(false) @@ -79,8 +80,8 @@ function createApp(catalog: ToolCatalog, runtimeConfig: ResolvedRuntimeConfig = registerToolCommands(app, catalog, { workspaceRoot: '/repo', runtimeConfig, - cliExposedWorkflowIds: ['simulator'], - workflowNames: ['simulator'], + cliExposedWorkflowIds: workflows, + workflowNames: workflows, }); return app; @@ -227,6 +228,109 @@ describe('registerToolCommands', () => { stdoutWrite.mockRestore(); }); + describe('device build destinations', () => { + const runtimeConfig: ResolvedRuntimeConfig = { + ...baseRuntimeConfig, + sessionDefaults: { + workspacePath: 'App.xcworkspace', + scheme: 'App', + deviceId: 'SAVED-DEVICE', + }, + sessionDefaultsProfiles: { + phone: { + workspacePath: 'Profile.xcworkspace', + scheme: 'ProfileApp', + deviceId: 'PROFILE-DEVICE', + }, + }, + activeSessionDefaultsProfile: undefined, + }; + + function createDeviceTool(overrides: Partial = {}) { + return createTool({ + id: 'build_device', + cliName: 'build', + mcpName: 'build_device', + workflow: 'device', + cliSchema: { + workspacePath: z.string(), + scheme: z.string(), + deviceId: z.string().optional(), + buildForTesting: z.boolean().optional(), + }, + ...overrides, + }); + } + + it.each([ + { description: 'normal builds', args: [] }, + { description: 'prepared builds', args: ['--build-for-testing'] }, + { description: 'named profiles', args: ['--profile', 'phone'] }, + ])('does not forward the saved device ID for $description', async ({ args }) => { + const invokeDirect = vi + .spyOn(DefaultToolInvoker.prototype, 'invokeDirect') + .mockResolvedValue(undefined); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + const tool = createDeviceTool(); + const app = createApp(createCatalog([tool]), runtimeConfig); + + await app.parseAsync(['device', 'build', ...args]); + + expect(invokeDirect).toHaveBeenCalledTimes(1); + const passedArgs = invokeDirect.mock.calls[0][1]; + expect(passedArgs).not.toHaveProperty('deviceId'); + expect(passedArgs).toMatchObject( + args.includes('phone') + ? { workspacePath: 'Profile.xcworkspace', scheme: 'ProfileApp' } + : { workspacePath: 'App.xcworkspace', scheme: 'App' }, + ); + expect(runtimeConfig.sessionDefaults?.deviceId).toBe('SAVED-DEVICE'); + expect(runtimeConfig.sessionDefaultsProfiles?.phone.deviceId).toBe('PROFILE-DEVICE'); + }); + + it.each([ + ['--device-id', 'EXPLICIT-DEVICE'], + ['--device-id', 'EXPLICIT-DEVICE', '--build-for-testing'], + ['--json', JSON.stringify({ deviceId: 'EXPLICIT-DEVICE', buildForTesting: true })], + ])('preserves explicit device arguments: %j', async (...args) => { + const invokeDirect = vi + .spyOn(DefaultToolInvoker.prototype, 'invokeDirect') + .mockResolvedValue(undefined); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + const tool = createDeviceTool(); + const app = createApp(createCatalog([tool]), runtimeConfig); + + await app.parseAsync(['device', 'build', ...args]); + + expect(invokeDirect).toHaveBeenCalledWith( + tool, + expect.objectContaining({ deviceId: 'EXPLICIT-DEVICE' }), + expect.any(Object), + ); + }); + + it.each([ + { id: 'build_run_device', cliName: 'build-and-run' }, + { id: 'install_app_device', cliName: 'install' }, + { id: 'test_device', cliName: 'test' }, + ])('retains the saved device ID for $cliName', async ({ id, cliName }) => { + const invokeDirect = vi + .spyOn(DefaultToolInvoker.prototype, 'invokeDirect') + .mockResolvedValue(undefined); + vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + const tool = createDeviceTool({ id, cliName }); + const app = createApp(createCatalog([tool]), runtimeConfig); + + await app.parseAsync(['device', cliName]); + + expect(invokeDirect).toHaveBeenCalledWith( + tool, + expect.objectContaining({ deviceId: 'SAVED-DEVICE' }), + expect.any(Object), + ); + }); + }); + it('hydrates required args from the explicit --profile override', async () => { process.argv = ['node', 'mobilebuildmcp', 'simulator', 'run-tool', '--profile', 'qa']; diff --git a/src/cli/session-defaults.ts b/src/cli/session-defaults.ts index 0097a630..d623cf33 100644 --- a/src/cli/session-defaults.ts +++ b/src/cli/session-defaults.ts @@ -46,13 +46,22 @@ export function getCliSessionDefaultsForTool(opts: { runtimeConfig: ResolvedRuntimeConfig; profileOverride?: string; }): Record { - return pickSchemaSessionDefaults( + const defaults = pickSchemaSessionDefaults( opts.tool.cliSchema, resolveCliSessionDefaults({ runtimeConfig: opts.runtimeConfig, profileOverride: opts.profileOverride, }), ); + + // Let the device build handler ignore the saved installation destination. + // Promoting it to an explicit CLI argument bypasses that handler's filtering. + // Explicit --device-id / --json values are merged separately by the caller. + if (opts.tool.id === 'build_device') { + delete defaults.deviceId; + } + + return defaults; } export function mergeCliSessionDefaults(opts: {