diff --git a/docs/session-aware-migration-todo.md b/docs/session-aware-migration-todo.md index 7f5be6c5..56fc6526 100644 --- a/docs/session-aware-migration-todo.md +++ b/docs/session-aware-migration-todo.md @@ -5,11 +5,11 @@ _Audit date: October 6, 2025_ Reference: `docs/session_management_plan.md` ## Utilities -- [ ] `src/mcp/tools/utilities/clean.ts` — session defaults: `projectPath`, `workspacePath`, `scheme`, `configuration`. +- [x] `src/mcp/tools/utilities/clean.ts` — session defaults: `projectPath`, `workspacePath`, `scheme`, `configuration`. ## Project Discovery -- [ ] `src/mcp/tools/project-discovery/list_schemes.ts` — session defaults: `projectPath`, `workspacePath`. -- [ ] `src/mcp/tools/project-discovery/show_build_settings.ts` — session defaults: `projectPath`, `workspacePath`, `scheme`. +- [x] `src/mcp/tools/project-discovery/list_schemes.ts` — session defaults: `projectPath`, `workspacePath`. +- [x] `src/mcp/tools/project-discovery/show_build_settings.ts` — session defaults: `projectPath`, `workspacePath`, `scheme`. ## Device Workflows - [ ] `src/mcp/tools/device/build_device.ts` — session defaults: `projectPath`, `workspacePath`, `scheme`, `configuration`. diff --git a/src/mcp/tools/project-discovery/__tests__/list_schemes.test.ts b/src/mcp/tools/project-discovery/__tests__/list_schemes.test.ts index de2f2425..c38e6abd 100644 --- a/src/mcp/tools/project-discovery/__tests__/list_schemes.test.ts +++ b/src/mcp/tools/project-discovery/__tests__/list_schemes.test.ts @@ -4,40 +4,35 @@ * Using dependency injection for deterministic testing */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; import { z } from 'zod'; import { createMockExecutor } from '../../../../test-utils/mock-executors.ts'; import plugin, { listSchemesLogic } from '../list_schemes.ts'; +import { sessionStore } from '../../../../utils/session-store.ts'; describe('list_schemes plugin', () => { + beforeEach(() => { + sessionStore.clear(); + }); + describe('Export Field Validation (Literal)', () => { it('should have correct name', () => { expect(plugin.name).toBe('list_schemes'); }); it('should have correct description', () => { - expect(plugin.description).toBe( - "Lists available schemes for either a project or a workspace. Provide exactly one of projectPath or workspacePath. Example: list_schemes({ projectPath: '/path/to/MyProject.xcodeproj' })", - ); + expect(plugin.description).toBe('Lists schemes for a project or workspace.'); }); it('should have handler function', () => { expect(typeof plugin.handler).toBe('function'); }); - it('should validate schema with valid inputs', () => { - const schema = z.object(plugin.schema); - expect(schema.safeParse({ projectPath: '/path/to/MyProject.xcodeproj' }).success).toBe(true); - expect(schema.safeParse({ projectPath: '/Users/dev/App.xcodeproj' }).success).toBe(true); - }); - - it('should validate schema with invalid inputs', () => { - const schema = z.object(plugin.schema); - // Base schema allows empty object - XOR validation is in refinements + it('should expose an empty public schema', () => { + const schema = z.object(plugin.schema).strict(); expect(schema.safeParse({}).success).toBe(true); - expect(schema.safeParse({ projectPath: 123 }).success).toBe(false); - expect(schema.safeParse({ projectPath: null }).success).toBe(false); - expect(schema.safeParse({ workspacePath: 123 }).success).toBe(false); + expect(schema.safeParse({ projectPath: '/path/to/MyProject.xcodeproj' }).success).toBe(false); + expect(Object.keys(plugin.schema)).toEqual([]); }); }); @@ -235,8 +230,8 @@ describe('list_schemes plugin', () => { // to verify Zod validation works properly. The createTypedTool wrapper handles validation. const result = await plugin.handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); - expect(result.content[0].text).toContain('Either projectPath or workspacePath is required'); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); }); }); @@ -244,7 +239,8 @@ describe('list_schemes plugin', () => { it('should error when neither projectPath nor workspacePath provided', async () => { const result = await plugin.handler({}); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Either projectPath or workspacePath is required'); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); }); it('should error when both projectPath and workspacePath provided', async () => { @@ -253,7 +249,7 @@ describe('list_schemes plugin', () => { workspacePath: '/path/to/workspace.xcworkspace', }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('mutually exclusive'); + expect(result.content[0].text).toContain('Mutually exclusive parameters provided'); }); it('should handle empty strings as undefined', async () => { @@ -262,7 +258,8 @@ describe('list_schemes plugin', () => { workspacePath: '', }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Either projectPath or workspacePath is required'); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); }); }); diff --git a/src/mcp/tools/project-discovery/__tests__/show_build_settings.test.ts b/src/mcp/tools/project-discovery/__tests__/show_build_settings.test.ts index 5fd7f186..6084c5a8 100644 --- a/src/mcp/tools/project-discovery/__tests__/show_build_settings.test.ts +++ b/src/mcp/tools/project-discovery/__tests__/show_build_settings.test.ts @@ -1,27 +1,32 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; import { z } from 'zod'; import { createMockExecutor } from '../../../../test-utils/mock-executors.ts'; import plugin, { showBuildSettingsLogic } from '../show_build_settings.ts'; +import { sessionStore } from '../../../../utils/session-store.ts'; describe('show_build_settings plugin', () => { + beforeEach(() => { + sessionStore.clear(); + }); describe('Export Field Validation (Literal)', () => { it('should have correct name', () => { expect(plugin.name).toBe('show_build_settings'); }); it('should have correct description', () => { - expect(plugin.description).toBe( - "Shows build settings from either a project or workspace using xcodebuild. Provide exactly one of projectPath or workspacePath, plus scheme. Example: show_build_settings({ projectPath: '/path/to/MyProject.xcodeproj', scheme: 'MyScheme' })", - ); + expect(plugin.description).toBe('Shows xcodebuild build settings.'); }); it('should have handler function', () => { expect(typeof plugin.handler).toBe('function'); }); - it('should have schema object', () => { - expect(plugin.schema).toBeDefined(); - expect(typeof plugin.schema).toBe('object'); + it('should expose an empty public schema', () => { + const schema = z.object(plugin.schema).strict(); + expect(schema.safeParse({}).success).toBe(true); + expect(schema.safeParse({ projectPath: '/path.xcodeproj' }).success).toBe(false); + expect(schema.safeParse({ scheme: 'App' }).success).toBe(false); + expect(Object.keys(plugin.schema)).toEqual([]); }); }); @@ -50,8 +55,8 @@ describe('show_build_settings plugin', () => { }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); - expect(result.content[0].text).toContain('projectPath'); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); }); it('should return success with build settings', async () => { @@ -169,7 +174,8 @@ describe('show_build_settings plugin', () => { }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Either projectPath or workspacePath is required'); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); }); it('should error when both projectPath and workspacePath provided', async () => { @@ -180,7 +186,7 @@ describe('show_build_settings plugin', () => { }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('mutually exclusive'); + expect(result.content[0].text).toContain('Mutually exclusive parameters provided'); }); it('should work with projectPath only', async () => { @@ -214,6 +220,28 @@ describe('show_build_settings plugin', () => { }); }); + describe('Session requirement handling', () => { + it('should require scheme when not provided', async () => { + const result = await plugin.handler({ + projectPath: '/path/to/MyProject.xcodeproj', + } as any); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('scheme is required'); + }); + + it('should surface project/workspace requirement even with scheme default', async () => { + sessionStore.setDefaults({ scheme: 'MyScheme' }); + + const result = await plugin.handler({}); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('Missing required session defaults'); + expect(result.content[0].text).toContain('Provide a project or workspace'); + }); + }); + describe('showBuildSettingsLogic function', () => { it('should return success with build settings', async () => { const calls: any[] = []; diff --git a/src/mcp/tools/project-discovery/list_schemes.ts b/src/mcp/tools/project-discovery/list_schemes.ts index 56102e66..82064ffc 100644 --- a/src/mcp/tools/project-discovery/list_schemes.ts +++ b/src/mcp/tools/project-discovery/list_schemes.ts @@ -11,7 +11,7 @@ import type { CommandExecutor } from '../../../utils/execution/index.ts'; import { getDefaultCommandExecutor } from '../../../utils/execution/index.ts'; import { createTextResponse } from '../../../utils/responses/index.ts'; import { ToolResponse } from '../../../types/common.ts'; -import { createTypedTool } from '../../../utils/typed-tool-factory.ts'; +import { createSessionAwareTool } from '../../../utils/typed-tool-factory.ts'; import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts'; // Unified schema: XOR between projectPath and workspacePath @@ -109,14 +109,22 @@ export async function listSchemesLogic( } } +const publicSchemaObject = baseSchemaObject.omit({ + projectPath: true, + workspacePath: true, +} as const); + export default { name: 'list_schemes', - description: - "Lists available schemes for either a project or a workspace. Provide exactly one of projectPath or workspacePath. Example: list_schemes({ projectPath: '/path/to/MyProject.xcodeproj' })", - schema: baseSchemaObject.shape, - handler: createTypedTool( - listSchemesSchema as z.ZodType, - listSchemesLogic, - getDefaultCommandExecutor, - ), + description: 'Lists schemes for a project or workspace.', + schema: publicSchemaObject.shape, + handler: createSessionAwareTool({ + internalSchema: listSchemesSchema as unknown as z.ZodType, + logicFunction: listSchemesLogic, + getExecutor: getDefaultCommandExecutor, + requirements: [ + { oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' }, + ], + exclusivePairs: [['projectPath', 'workspacePath']], + }), }; diff --git a/src/mcp/tools/project-discovery/show_build_settings.ts b/src/mcp/tools/project-discovery/show_build_settings.ts index 4a8f0fb4..e2a5b9db 100644 --- a/src/mcp/tools/project-discovery/show_build_settings.ts +++ b/src/mcp/tools/project-discovery/show_build_settings.ts @@ -11,7 +11,7 @@ import type { CommandExecutor } from '../../../utils/execution/index.ts'; import { getDefaultCommandExecutor } from '../../../utils/execution/index.ts'; import { createTextResponse } from '../../../utils/responses/index.ts'; import { ToolResponse } from '../../../types/common.ts'; -import { createTypedTool } from '../../../utils/typed-tool-factory.ts'; +import { createSessionAwareTool } from '../../../utils/typed-tool-factory.ts'; import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts'; // Unified schema: XOR between projectPath and workspacePath @@ -102,14 +102,24 @@ export async function showBuildSettingsLogic( } } +const publicSchemaObject = baseSchemaObject.omit({ + projectPath: true, + workspacePath: true, + scheme: true, +} as const); + export default { name: 'show_build_settings', - description: - "Shows build settings from either a project or workspace using xcodebuild. Provide exactly one of projectPath or workspacePath, plus scheme. Example: show_build_settings({ projectPath: '/path/to/MyProject.xcodeproj', scheme: 'MyScheme' })", - schema: baseSchemaObject.shape, - handler: createTypedTool( - showBuildSettingsSchema as z.ZodType, - showBuildSettingsLogic, - getDefaultCommandExecutor, - ), + description: 'Shows xcodebuild build settings.', + schema: publicSchemaObject.shape, + handler: createSessionAwareTool({ + internalSchema: showBuildSettingsSchema as unknown as z.ZodType, + logicFunction: showBuildSettingsLogic, + getExecutor: getDefaultCommandExecutor, + requirements: [ + { allOf: ['scheme'], message: 'scheme is required' }, + { oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' }, + ], + exclusivePairs: [['projectPath', 'workspacePath']], + }), }; diff --git a/src/mcp/tools/utilities/__tests__/clean.test.ts b/src/mcp/tools/utilities/__tests__/clean.test.ts index 59191cd1..2e4b0269 100644 --- a/src/mcp/tools/utilities/__tests__/clean.test.ts +++ b/src/mcp/tools/utilities/__tests__/clean.test.ts @@ -1,20 +1,43 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeEach } from 'vitest'; +import { z } from 'zod'; import tool, { cleanLogic } from '../clean.ts'; import { createMockExecutor } from '../../../../test-utils/mock-executors.ts'; +import { sessionStore } from '../../../../utils/session-store.ts'; describe('clean (unified) tool', () => { + beforeEach(() => { + sessionStore.clear(); + }); + it('exports correct name/description/schema/handler', () => { expect(tool.name).toBe('clean'); - expect(typeof tool.description).toBe('string'); - expect(tool.schema).toBeDefined(); + expect(tool.description).toBe('Cleans build products with xcodebuild.'); expect(typeof tool.handler).toBe('function'); + + const schema = z.object(tool.schema).strict(); + expect(schema.safeParse({}).success).toBe(true); + expect( + schema.safeParse({ + derivedDataPath: '/tmp/Derived', + extraArgs: ['--quiet'], + preferXcodebuild: true, + platform: 'iOS Simulator', + }).success, + ).toBe(true); + expect(schema.safeParse({ configuration: 'Debug' }).success).toBe(false); + + const schemaKeys = Object.keys(tool.schema).sort(); + expect(schemaKeys).toEqual( + ['derivedDataPath', 'extraArgs', 'platform', 'preferXcodebuild'].sort(), + ); }); it('handler validation: error when neither projectPath nor workspacePath provided', async () => { const result = await (tool as any).handler({}); expect(result.isError).toBe(true); - const text = String(result.content?.[1]?.text ?? result.content?.[0]?.text ?? ''); - expect(text).toContain('Invalid parameters'); + const text = String(result.content?.[0]?.text ?? ''); + expect(text).toContain('Missing required session defaults'); + expect(text).toContain('Provide a project or workspace'); }); it('handler validation: error when both projectPath and workspacePath provided', async () => { @@ -23,8 +46,8 @@ describe('clean (unified) tool', () => { workspacePath: '/w.xcworkspace', }); expect(result.isError).toBe(true); - const text = String(result.content?.[1]?.text ?? result.content?.[0]?.text ?? ''); - expect(text).toContain('Invalid parameters'); + const text = String(result.content?.[0]?.text ?? ''); + expect(text).toContain('Mutually exclusive parameters provided'); }); it('runs project-path flow via logic', async () => { @@ -45,8 +68,9 @@ describe('clean (unified) tool', () => { it('handler validation: requires scheme when workspacePath is provided', async () => { const result = await (tool as any).handler({ workspacePath: '/w.xcworkspace' }); expect(result.isError).toBe(true); - const text = String(result.content?.[1]?.text ?? result.content?.[0]?.text ?? ''); - expect(text).toContain('Invalid parameters'); + const text = String(result.content?.[0]?.text ?? ''); + expect(text).toContain('Parameter validation failed'); + expect(text).toContain('scheme is required when workspacePath is provided'); }); it('uses iOS platform by default', async () => { @@ -121,7 +145,8 @@ describe('clean (unified) tool', () => { platform: 'InvalidPlatform', }); expect(result.isError).toBe(true); - const text = String(result.content?.[1]?.text ?? result.content?.[0]?.text ?? ''); - expect(text).toContain('Invalid parameters'); + const text = String(result.content?.[0]?.text ?? ''); + expect(text).toContain('Parameter validation failed'); + expect(text).toContain('platform'); }); }); diff --git a/src/mcp/tools/utilities/clean.ts b/src/mcp/tools/utilities/clean.ts index f18db019..062319a4 100644 --- a/src/mcp/tools/utilities/clean.ts +++ b/src/mcp/tools/utilities/clean.ts @@ -6,7 +6,7 @@ */ import { z } from 'zod'; -import { createTypedTool } from '../../../utils/typed-tool-factory.ts'; +import { createSessionAwareTool } from '../../../utils/typed-tool-factory.ts'; import type { CommandExecutor } from '../../../utils/execution/index.ts'; import { getDefaultCommandExecutor } from '../../../utils/execution/index.ts'; import { executeXcodeBuildCommand } from '../../../utils/build/index.ts'; @@ -146,14 +146,24 @@ export async function cleanLogic( ); } +const publicSchemaObject = baseSchemaObject.omit({ + projectPath: true, + workspacePath: true, + scheme: true, + configuration: true, +} as const); + export default { name: 'clean', - description: - "Cleans build products for either a project or a workspace using xcodebuild. Provide exactly one of projectPath or workspacePath. Platform defaults to iOS if not specified. Example: clean({ projectPath: '/path/to/MyProject.xcodeproj', scheme: 'MyScheme', platform: 'iOS' })", - schema: baseSchemaObject.shape, - handler: createTypedTool( - cleanSchema as z.ZodType, - cleanLogic, - getDefaultCommandExecutor, - ), + description: 'Cleans build products with xcodebuild.', + schema: publicSchemaObject.shape, + handler: createSessionAwareTool({ + internalSchema: cleanSchema as unknown as z.ZodType, + logicFunction: cleanLogic, + getExecutor: getDefaultCommandExecutor, + requirements: [ + { oneOf: ['projectPath', 'workspacePath'], message: 'Provide a project or workspace' }, + ], + exclusivePairs: [['projectPath', 'workspacePath']], + }), }; diff --git a/src/utils/typed-tool-factory.ts b/src/utils/typed-tool-factory.ts index 86a68008..153757da 100644 --- a/src/utils/typed-tool-factory.ts +++ b/src/utils/typed-tool-factory.ts @@ -92,7 +92,9 @@ export function createSessionAwareTool(opts: { // Sanitize args: treat null/undefined as "not provided" so they don't override session defaults const sanitizedArgs: Record = {}; for (const [k, v] of Object.entries(rawArgs)) { - if (v !== null && v !== undefined) sanitizedArgs[k] = v; + if (v === null || v === undefined) continue; + if (typeof v === 'string' && v.trim() === '') continue; + sanitizedArgs[k] = v; } // Factory-level mutual exclusivity check: if user provides multiple explicit values