diff --git a/src/core/manifest/__tests__/load-manifest.test.ts b/src/core/manifest/__tests__/load-manifest.test.ts deleted file mode 100644 index 60b2aefa..00000000 --- a/src/core/manifest/__tests__/load-manifest.test.ts +++ /dev/null @@ -1,187 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; -import * as os from 'node:os'; -import { - loadManifest, - getWorkflowTools, - getToolsForWorkflows, - ManifestValidationError, -} from '../load-manifest.ts'; - -describe('load-manifest', () => { - describe('loadManifest (integration with real manifests)', () => { - it('should load all manifests from the manifests directory', () => { - const manifest = loadManifest(); - - // Check that we have tools and workflows - expect(manifest.tools.size).toBeGreaterThan(0); - expect(manifest.workflows.size).toBeGreaterThan(0); - }); - - it('should have required workflows', () => { - const manifest = loadManifest(); - - expect(manifest.workflows.has('simulator')).toBe(true); - expect(manifest.workflows.has('device')).toBe(true); - expect(manifest.workflows.has('session-management')).toBe(true); - }); - - it('should have required tools', () => { - const manifest = loadManifest(); - - expect(manifest.tools.has('build_sim')).toBe(true); - expect(manifest.tools.has('discover_projs')).toBe(true); - expect(manifest.tools.has('session_show_defaults')).toBe(true); - expect(manifest.tools.has('session_use_defaults_profile')).toBe(true); - }); - - it('should validate tool references in workflows', () => { - const manifest = loadManifest(); - - // Every tool referenced in a workflow should exist - for (const [workflowId, workflow] of manifest.workflows) { - for (const toolId of workflow.tools) { - expect( - manifest.tools.has(toolId), - `Workflow '${workflowId}' references unknown tool '${toolId}'`, - ).toBe(true); - } - } - }); - - it('should have unique MCP names across all tools', () => { - const manifest = loadManifest(); - const mcpNames = new Set(); - - for (const [, tool] of manifest.tools) { - expect(mcpNames.has(tool.names.mcp), `Duplicate MCP name '${tool.names.mcp}'`).toBe(false); - mcpNames.add(tool.names.mcp); - } - }); - - it('should have session-management as auto-include workflow', () => { - const manifest = loadManifest(); - const sessionMgmt = manifest.workflows.get('session-management'); - - expect(sessionMgmt).toBeDefined(); - expect(sessionMgmt?.selection?.mcp?.autoInclude).toBe(true); - }); - - it('should have simulator as default-enabled workflow', () => { - const manifest = loadManifest(); - const simulator = manifest.workflows.get('simulator'); - - expect(simulator).toBeDefined(); - expect(simulator?.selection?.mcp?.defaultEnabled).toBe(true); - }); - - it('should have doctor workflow with debugEnabled predicate', () => { - const manifest = loadManifest(); - const doctor = manifest.workflows.get('doctor'); - - expect(doctor).toBeDefined(); - expect(doctor?.predicates).toContain('debugEnabled'); - expect(doctor?.selection?.mcp?.autoInclude).toBe(true); - }); - - it('should have xcode-ide workflow hidden in Xcode agent mode only', () => { - const manifest = loadManifest(); - const xcodeIde = manifest.workflows.get('xcode-ide'); - - expect(xcodeIde).toBeDefined(); - expect(xcodeIde?.predicates).toContain('hideWhenXcodeAgentMode'); - expect(xcodeIde?.predicates).not.toContain('debugEnabled'); - }); - - it('should keep xcode bridge gateway tools daemon-routed and debug tools gated', () => { - const manifest = loadManifest(); - - expect(manifest.tools.get('xcode_ide_list_tools')?.routing?.stateful).toBe(true); - expect(manifest.tools.get('xcode_ide_call_tool')?.routing?.stateful).toBe(true); - expect(manifest.tools.get('xcode_tools_bridge_status')?.predicates).toContain('debugEnabled'); - expect(manifest.tools.get('xcode_tools_bridge_sync')?.predicates).toContain('debugEnabled'); - expect(manifest.tools.get('xcode_tools_bridge_disconnect')?.predicates).toContain( - 'debugEnabled', - ); - }); - - it('should provide explicit approval annotations for every tool', () => { - const manifest = loadManifest(); - - for (const [toolId, tool] of manifest.tools) { - expect(tool.annotations, `Tool '${toolId}' is missing annotations`).toBeDefined(); - expect( - tool.annotations?.title, - `Tool '${toolId}' is missing annotations.title`, - ).toBeTruthy(); - expect( - tool.annotations?.readOnlyHint, - `Tool '${toolId}' is missing annotations.readOnlyHint`, - ).not.toBeUndefined(); - expect( - tool.annotations?.destructiveHint, - `Tool '${toolId}' is missing annotations.destructiveHint`, - ).not.toBeUndefined(); - expect( - tool.annotations?.openWorldHint, - `Tool '${toolId}' is missing annotations.openWorldHint`, - ).not.toBeUndefined(); - } - }); - }); - - describe('getWorkflowTools', () => { - it('should return tools for a workflow', () => { - const manifest = loadManifest(); - const tools = getWorkflowTools(manifest, 'simulator'); - - expect(tools.length).toBeGreaterThan(0); - expect(tools.some((t) => t.id === 'build_sim')).toBe(true); - }); - - it('should return empty array for unknown workflow', () => { - const manifest = loadManifest(); - const tools = getWorkflowTools(manifest, 'nonexistent-workflow'); - - expect(tools).toEqual([]); - }); - }); - - describe('getToolsForWorkflows', () => { - it('should return unique tools across multiple workflows', () => { - const manifest = loadManifest(); - const tools = getToolsForWorkflows(manifest, ['simulator', 'device']); - - // Should have tools from both workflows - expect(tools.some((t) => t.id === 'build_sim')).toBe(true); - expect(tools.some((t) => t.id === 'build_device')).toBe(true); - - // Tools should be unique (discover_projs is in both) - const toolIds = tools.map((t) => t.id); - const uniqueIds = new Set(toolIds); - expect(toolIds.length).toBe(uniqueIds.size); - }); - - it('should return empty array for empty workflow list', () => { - const manifest = loadManifest(); - const tools = getToolsForWorkflows(manifest, []); - - expect(tools).toEqual([]); - }); - }); -}); - -describe('ManifestValidationError', () => { - it('should include source file in message', () => { - const error = new ManifestValidationError('Test error', 'test.yaml'); - expect(error.message).toBe('Test error (in test.yaml)'); - expect(error.sourceFile).toBe('test.yaml'); - }); - - it('should work without source file', () => { - const error = new ManifestValidationError('Test error'); - expect(error.message).toBe('Test error'); - expect(error.sourceFile).toBeUndefined(); - }); -}); diff --git a/src/core/manifest/__tests__/schema.test.ts b/src/core/manifest/__tests__/schema.test.ts index a50b9255..b44a7efd 100644 --- a/src/core/manifest/__tests__/schema.test.ts +++ b/src/core/manifest/__tests__/schema.test.ts @@ -2,223 +2,78 @@ import { describe, it, expect } from 'vitest'; import { toolManifestEntrySchema, workflowManifestEntrySchema, - deriveCliName, + resourceManifestEntrySchema, getEffectiveCliName, - type ToolManifestEntry, } from '../schema.ts'; describe('schema', () => { - describe('toolManifestEntrySchema', () => { - it('should parse valid tool manifest', () => { - const input = { - id: 'build_sim', - module: 'mcp/tools/simulator/build_sim', - names: { mcp: 'build_sim' }, - description: 'Build iOS app for simulator', - availability: { mcp: true, cli: true }, - predicates: [], - routing: { stateful: false }, - }; - - const result = toolManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.id).toBe('build_sim'); - expect(result.data.names.mcp).toBe('build_sim'); - } - }); - - it('should apply default availability', () => { - const input = { - id: 'test_tool', - module: 'mcp/tools/test/test_tool', - names: { mcp: 'test_tool' }, - }; - - const result = toolManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.availability).toEqual({ mcp: true, cli: true }); - expect(result.data.predicates).toEqual([]); - expect(result.data.nextSteps).toEqual([]); - } - }); - - it('should reject missing required fields', () => { - const input = { - id: 'test_tool', - // missing module and names - }; - - const result = toolManifestEntrySchema.safeParse(input); - expect(result.success).toBe(false); - }); - - it('should accept optional CLI name', () => { - const input = { - id: 'build_sim', - module: 'mcp/tools/simulator/build_sim', - names: { mcp: 'build_sim', cli: 'build-simulator' }, - }; - - const result = toolManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.names.cli).toBe('build-simulator'); - } - }); - - it('should reject availability.daemon', () => { - const input = { - id: 'tool1', - module: 'mcp/tools/test/tool1', - names: { mcp: 'tool1' }, - availability: { mcp: true, cli: true, daemon: true }, - }; - - expect(toolManifestEntrySchema.safeParse(input).success).toBe(false); - }); - - it('should reject routing.daemonAffinity', () => { - const input = { - id: 'tool2', - module: 'mcp/tools/test/tool2', - names: { mcp: 'tool2' }, - routing: { stateful: true, daemonAffinity: 'required' }, - }; - - expect(toolManifestEntrySchema.safeParse(input).success).toBe(false); - }); + it('parses a representative manifest/tool naming pipeline', () => { + const toolInput = { + id: 'build_sim', + module: 'mcp/tools/simulator/build_sim', + names: { mcp: 'build_sim' }, + }; + const workflowInput = { + id: 'simulator', + title: 'iOS Simulator Development', + description: 'Build and test iOS apps on simulators', + tools: ['build_sim'], + }; + + const toolResult = toolManifestEntrySchema.safeParse(toolInput); + const workflowResult = workflowManifestEntrySchema.safeParse(workflowInput); + + expect(toolResult.success).toBe(true); + expect(workflowResult.success).toBe(true); + + if (!toolResult.success || !workflowResult.success) { + throw new Error('Expected representative manifest inputs to parse'); + } + + expect(toolResult.data.availability).toEqual({ mcp: true, cli: true }); + expect(toolResult.data.nextSteps).toEqual([]); + expect(toolResult.data.predicates).toEqual([]); + expect(workflowResult.data.availability).toEqual({ mcp: true, cli: true }); + expect(workflowResult.data.predicates).toEqual([]); + expect(workflowResult.data.tools).toEqual(['build_sim']); + expect(getEffectiveCliName(toolResult.data)).toBe('build-sim'); }); - describe('workflowManifestEntrySchema', () => { - it('should parse valid workflow manifest', () => { - const input = { - id: 'simulator', - title: 'iOS Simulator Development', - description: 'Build and test iOS apps on simulators', - availability: { mcp: true, cli: true }, - selection: { - mcp: { - defaultEnabled: true, - autoInclude: false, - }, - }, - predicates: [], - tools: ['build_sim', 'test_sim', 'boot_sim'], - }; - - const result = workflowManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.id).toBe('simulator'); - expect(result.data.tools).toHaveLength(3); - expect(result.data.selection?.mcp?.defaultEnabled).toBe(true); - } - }); - - it('should apply default values', () => { - const input = { - id: 'test-workflow', - title: 'Test Workflow', - description: 'A test workflow', - tools: ['tool1'], - }; - - const result = workflowManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.availability).toEqual({ mcp: true, cli: true }); - expect(result.data.predicates).toEqual([]); - } - }); - - it('should reject empty tools array', () => { - const input = { - id: 'empty-workflow', - title: 'Empty Workflow', - description: 'A workflow with no tools', - tools: [], - }; - - // Empty tools array is technically valid per schema - const result = workflowManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - }); - - it('should parse autoInclude workflow', () => { - const input = { - id: 'session-management', - title: 'Session Management', - description: 'Manage session defaults', - availability: { mcp: true, cli: false }, - selection: { - mcp: { - defaultEnabled: true, - autoInclude: true, - }, - }, - tools: ['session_show_defaults'], - }; - - const result = workflowManifestEntrySchema.safeParse(input); - expect(result.success).toBe(true); - if (result.success) { - expect(result.data.selection?.mcp?.autoInclude).toBe(true); - expect(result.data.availability.cli).toBe(false); - } - }); - }); - - describe('deriveCliName', () => { - it('should convert underscores to hyphens', () => { - expect(deriveCliName('build_sim')).toBe('build-sim'); - expect(deriveCliName('get_app_bundle_id')).toBe('get-app-bundle-id'); - }); - - it('should convert camelCase to kebab-case', () => { - expect(deriveCliName('buildSim')).toBe('build-sim'); - expect(deriveCliName('getAppBundleId')).toBe('get-app-bundle-id'); - }); + it('parses a resource manifest entry with defaults', () => { + const input = { + id: 'simulators', + module: 'mcp/resources/simulators', + name: 'simulators', + uri: 'xcodebuildmcp://simulators', + description: 'Available iOS simulators', + mimeType: 'text/plain', + }; - it('should handle mixed underscores and camelCase', () => { - expect(deriveCliName('build_simApp')).toBe('build-sim-app'); - }); + const result = resourceManifestEntrySchema.safeParse(input); - it('should handle already kebab-case', () => { - expect(deriveCliName('build-sim')).toBe('build-sim'); - }); + expect(result.success).toBe(true); + if (!result.success) throw new Error('Expected resource manifest input to parse'); - it('should lowercase the result', () => { - expect(deriveCliName('BUILD_SIM')).toBe('build-sim'); - }); + expect(result.data.availability).toEqual({ mcp: true }); + expect(result.data.predicates).toEqual([]); }); - describe('getEffectiveCliName', () => { - it('should use explicit CLI name when provided', () => { - const tool: ToolManifestEntry = { - id: 'build_sim', - module: 'mcp/tools/simulator/build_sim', - names: { mcp: 'build_sim', cli: 'build-simulator' }, - availability: { mcp: true, cli: true }, - predicates: [], - nextSteps: [], - }; + it('parses a resource manifest entry with predicates', () => { + const input = { + id: 'xcode-ide-state', + module: 'mcp/resources/xcode-ide-state', + name: 'xcode-ide-state', + uri: 'xcodebuildmcp://xcode-ide-state', + description: 'Xcode IDE state', + mimeType: 'application/json', + predicates: ['runningUnderXcodeAgent'], + }; - expect(getEffectiveCliName(tool)).toBe('build-simulator'); - }); + const result = resourceManifestEntrySchema.safeParse(input); - it('should derive CLI name when not provided', () => { - const tool: ToolManifestEntry = { - id: 'build_sim', - module: 'mcp/tools/simulator/build_sim', - names: { mcp: 'build_sim' }, - availability: { mcp: true, cli: true }, - predicates: [], - nextSteps: [], - }; + expect(result.success).toBe(true); + if (!result.success) throw new Error('Expected resource manifest input to parse'); - expect(getEffectiveCliName(tool)).toBe('build-sim'); - }); + expect(result.data.predicates).toEqual(['runningUnderXcodeAgent']); }); }); diff --git a/src/core/manifest/import-resource-module.ts b/src/core/manifest/import-resource-module.ts new file mode 100644 index 00000000..675aabba --- /dev/null +++ b/src/core/manifest/import-resource-module.ts @@ -0,0 +1,61 @@ +/** + * Resource module importer. + * Dynamically imports resource modules using named exports only. + */ + +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { getPackageRoot } from './load-manifest.ts'; + +export interface ImportedResourceModule { + handler: (uri: URL) => Promise<{ contents: Array<{ text: string }> }>; +} + +const moduleCache = new Map(); + +/** + * Import a resource module by its manifest module path. + * + * Accepts named export only: `export const handler = ...` + * + * @param moduleId - Extensionless module path (e.g., 'mcp/resources/simulators') + * @returns Imported resource module with handler + */ +export async function importResourceModule(moduleId: string): Promise { + const cached = moduleCache.get(moduleId); + if (cached) { + return cached; + } + + const packageRoot = getPackageRoot(); + const modulePath = path.join(packageRoot, 'build', `${moduleId}.js`); + const moduleUrl = pathToFileURL(modulePath).href; + + let mod: Record; + try { + mod = (await import(moduleUrl)) as Record; + } catch (err) { + throw new Error(`Failed to import resource module '${moduleId}': ${err}`); + } + + if (typeof mod.handler !== 'function') { + throw new Error( + `Resource module '${moduleId}' does not export the required shape. ` + + `Expected a named export: export const handler = ...`, + ); + } + + const result: ImportedResourceModule = { + handler: mod.handler as ImportedResourceModule['handler'], + }; + + moduleCache.set(moduleId, result); + return result; +} + +/** + * Reset module cache (for tests). + */ +export function __resetResourceModuleCacheForTests(): void { + moduleCache.clear(); +} diff --git a/src/core/manifest/import-tool-module.ts b/src/core/manifest/import-tool-module.ts index a2371e90..12ad850f 100644 --- a/src/core/manifest/import-tool-module.ts +++ b/src/core/manifest/import-tool-module.ts @@ -1,40 +1,30 @@ /** - * Tool module importer with backward-compatible adapter. - * Dynamically imports tool modules and adapts both old (PluginMeta default export) - * and new (named exports) formats. + * Tool module importer. + * Dynamically imports tool modules using named exports only. */ import * as path from 'node:path'; import { pathToFileURL } from 'node:url'; import type { ToolSchemaShape } from '../plugin-types.ts'; +import type { ToolHandlerContext } from '../../rendering/types.ts'; import { getPackageRoot } from './load-manifest.ts'; -/** - * Imported tool module interface. - * This is what we extract from each tool module for runtime use. - */ export interface ImportedToolModule { schema: ToolSchemaShape; - handler: (params: Record) => Promise; + handler: (params: Record, ctx?: ToolHandlerContext) => Promise; } -/** - * Cache for imported modules. - */ const moduleCache = new Map(); /** * Import a tool module by its manifest module path. * - * Supports two module formats: - * 1. Legacy: `export default { name, schema, handler, ... }` - * 2. New: Named exports `{ schema, handler }` + * Accepts named exports only: `export const schema = ...` and `export const handler = ...` * * @param moduleId - Extensionless module path (e.g., 'mcp/tools/simulator/build_sim') * @returns Imported tool module with schema and handler */ export async function importToolModule(moduleId: string): Promise { - // Check cache first const cached = moduleCache.get(moduleId); if (cached) { return cached; @@ -51,56 +41,28 @@ export async function importToolModule(moduleId: string): Promise, moduleId: string): ImportedToolModule { - // Try legacy format first: default export with PluginMeta shape - if (mod.default && typeof mod.default === 'object') { - const defaultExport = mod.default as Record; - - // Check if it looks like a PluginMeta (has schema and handler) - if (defaultExport.schema && typeof defaultExport.handler === 'function') { - return { - schema: defaultExport.schema as ToolSchemaShape, - handler: defaultExport.handler as (params: Record) => Promise, - }; - } + if (!mod.schema || typeof mod.handler !== 'function') { + throw new Error( + `Tool module '${moduleId}' does not export the required shape. ` + + `Expected named exports: export const schema = ... and export const handler = ...`, + ); } - // Try new format: named exports - if (mod.schema && typeof mod.handler === 'function') { - return { - schema: mod.schema as ToolSchemaShape, - handler: mod.handler as (params: Record) => Promise, - }; - } + const result: ImportedToolModule = { + schema: mod.schema as ToolSchemaShape, + handler: mod.handler as ( + params: Record, + ctx?: ToolHandlerContext, + ) => Promise, + }; - throw new Error( - `Tool module '${moduleId}' does not export the required shape. ` + - `Expected either a default export with { schema, handler } or named exports { schema, handler }.`, - ); + moduleCache.set(moduleId, result); + return result; } /** - * Clear the module cache. - * Useful for testing or hot-reloading scenarios. + * Reset module cache (for tests). */ -export function clearModuleCache(): void { +export function __resetToolModuleCacheForTests(): void { moduleCache.clear(); } - -/** - * Preload multiple tool modules in parallel. - */ -export async function preloadToolModules(moduleIds: string[]): Promise { - await Promise.all(moduleIds.map((id) => importToolModule(id))); -} diff --git a/src/core/manifest/index.ts b/src/core/manifest/index.ts index 002f0437..b875c276 100644 --- a/src/core/manifest/index.ts +++ b/src/core/manifest/index.ts @@ -5,3 +5,4 @@ export * from './schema.ts'; export * from './load-manifest.ts'; export * from './import-tool-module.ts'; +export * from './import-resource-module.ts'; diff --git a/src/core/manifest/load-manifest.ts b/src/core/manifest/load-manifest.ts index f9b2f57e..e6ead334 100644 --- a/src/core/manifest/load-manifest.ts +++ b/src/core/manifest/load-manifest.ts @@ -9,20 +9,18 @@ import { parse as parseYaml } from 'yaml'; import { toolManifestEntrySchema, workflowManifestEntrySchema, + resourceManifestEntrySchema, type ToolManifestEntry, type WorkflowManifestEntry, + type ResourceManifestEntry, type ResolvedManifest, } from './schema.ts'; import { getManifestsDir, getPackageRoot } from '../resource-root.ts'; -// Re-export types for consumers -export type { ResolvedManifest, ToolManifestEntry, WorkflowManifestEntry }; +export type { ResolvedManifest, ToolManifestEntry, WorkflowManifestEntry, ResourceManifestEntry }; import { isValidPredicate } from '../../visibility/predicate-registry.ts'; export { getManifestsDir, getPackageRoot } from '../resource-root.ts'; -/** - * Load all YAML files from a directory. - */ function loadYamlFiles(dir: string): unknown[] { if (!fs.existsSync(dir)) { return []; @@ -47,9 +45,6 @@ function loadYamlFiles(dir: string): unknown[] { return results; } -/** - * Validation error for manifest loading. - */ export class ManifestValidationError extends Error { constructor( message: string, @@ -72,7 +67,6 @@ export function loadManifest(): ResolvedManifest { const tools = new Map(); const workflows = new Map(); - // Load tools const toolFiles = loadYamlFiles(toolsDir); for (const raw of toolFiles) { const sourceFile = (raw as { _sourceFile?: string })._sourceFile; @@ -86,12 +80,10 @@ export function loadManifest(): ResolvedManifest { const tool = result.data; - // Check for duplicate ID if (tools.has(tool.id)) { throw new ManifestValidationError(`Duplicate tool ID '${tool.id}'`, sourceFile); } - // Validate predicates for (const pred of tool.predicates) { if (!isValidPredicate(pred)) { throw new ManifestValidationError( @@ -104,7 +96,6 @@ export function loadManifest(): ResolvedManifest { tools.set(tool.id, tool); } - // Load workflows const workflowFiles = loadYamlFiles(workflowsDir); for (const raw of workflowFiles) { const sourceFile = (raw as { _sourceFile?: string })._sourceFile; @@ -118,12 +109,10 @@ export function loadManifest(): ResolvedManifest { const workflow = result.data; - // Check for duplicate ID if (workflows.has(workflow.id)) { throw new ManifestValidationError(`Duplicate workflow ID '${workflow.id}'`, sourceFile); } - // Validate predicates for (const pred of workflow.predicates) { if (!isValidPredicate(pred)) { throw new ManifestValidationError( @@ -133,7 +122,6 @@ export function loadManifest(): ResolvedManifest { } } - // Validate tool references for (const toolId of workflow.tools) { if (!tools.has(toolId)) { throw new ManifestValidationError( @@ -146,8 +134,7 @@ export function loadManifest(): ResolvedManifest { workflows.set(workflow.id, workflow); } - // Validate MCP name uniqueness - const mcpNames = new Map(); // mcpName -> toolId + const mcpNames = new Map(); for (const [toolId, tool] of tools) { const existing = mcpNames.get(tool.names.mcp); if (existing) { @@ -158,7 +145,6 @@ export function loadManifest(): ResolvedManifest { mcpNames.set(tool.names.mcp, toolId); } - // Validate next step template references for (const [toolId, tool] of tools.entries()) { const sourceFile = toolFiles.find((raw) => { const candidate = raw as { id?: string; _sourceFile?: string }; @@ -175,24 +161,47 @@ export function loadManifest(): ResolvedManifest { } } - return { tools, workflows }; -} + const resourcesDir = path.join(manifestsDir, 'resources'); + const resources = new Map(); -/** - * Validate that all tool modules exist on disk. - * Call this at startup to fail fast on missing modules. - */ -export function validateToolModules(manifest: ResolvedManifest): void { - const packageRoot = getPackageRoot(); + const resourceFiles = loadYamlFiles(resourcesDir); + for (const raw of resourceFiles) { + const sourceFile = (raw as { _sourceFile?: string })._sourceFile; + const result = resourceManifestEntrySchema.safeParse(raw); + if (!result.success) { + throw new ManifestValidationError( + `Invalid resource manifest: ${result.error.message}`, + sourceFile, + ); + } - for (const [toolId, tool] of manifest.tools) { - const modulePath = path.join(packageRoot, 'build', `${tool.module}.js`); - if (!fs.existsSync(modulePath)) { + const resource = result.data; + + if (resources.has(resource.id)) { + throw new ManifestValidationError(`Duplicate resource ID '${resource.id}'`, sourceFile); + } + + const existingUri = [...resources.values()].find((r) => r.uri === resource.uri); + if (existingUri) { throw new ManifestValidationError( - `Tool '${toolId}' references missing module: ${modulePath}`, + `Duplicate resource URI '${resource.uri}' used by resources '${existingUri.id}' and '${resource.id}'`, + sourceFile, ); } + + for (const pred of resource.predicates) { + if (!isValidPredicate(pred)) { + throw new ManifestValidationError( + `Unknown predicate '${pred}' in resource '${resource.id}'`, + sourceFile, + ); + } + } + + resources.set(resource.id, resource); } + + return { tools, workflows, resources }; } /** diff --git a/src/core/manifest/schema.ts b/src/core/manifest/schema.ts index 11e4e7e0..d158754c 100644 --- a/src/core/manifest/schema.ts +++ b/src/core/manifest/schema.ts @@ -63,6 +63,7 @@ export const manifestNextStepTemplateSchema = z toolId: z.string().optional(), params: z.record(z.string(), z.union([z.string(), z.number(), z.boolean()])).default({}), priority: z.number().optional(), + when: z.enum(['always', 'success', 'failure']).default('always'), }) .strict(); @@ -157,11 +158,58 @@ export const workflowManifestEntrySchema = z.object({ export type WorkflowManifestEntry = z.infer; /** - * Resolved manifest containing all tools and workflows. + * Resource availability flags (MCP only). + */ +export const resourceAvailabilitySchema = z + .object({ + mcp: z.boolean().default(true), + }) + .strict(); + +export type ResourceAvailability = z.infer; + +/** + * Resource manifest entry schema. + * Describes a single MCP resource's metadata and configuration. + */ +export const resourceManifestEntrySchema = z.object({ + /** Unique resource identifier */ + id: z.string(), + + /** + * Module path (extensionless, package-relative). + * Resolved to build/.js at runtime. + */ + module: z.string(), + + /** MCP resource name */ + name: z.string(), + + /** Resource URI (e.g., xcodebuildmcp://simulators) */ + uri: z.string(), + + /** Resource description */ + description: z.string(), + + /** MIME type for the resource content */ + mimeType: z.string(), + + /** Per-runtime availability flags */ + availability: resourceAvailabilitySchema.default({ mcp: true }), + + /** Predicate names for visibility filtering (all must pass) */ + predicates: z.array(z.string()).default([]), +}); + +export type ResourceManifestEntry = z.infer; + +/** + * Resolved manifest containing all tools, workflows, and resources. */ export interface ResolvedManifest { tools: Map; workflows: Map; + resources: Map; } /** diff --git a/src/core/plugin-types.ts b/src/core/plugin-types.ts index ad617ac5..5a1adafa 100644 --- a/src/core/plugin-types.ts +++ b/src/core/plugin-types.ts @@ -1,36 +1,3 @@ import * as z from 'zod'; -import type { ToolAnnotations } from '@modelcontextprotocol/sdk/types.js'; -import type { ToolResponse } from '../types/common.ts'; export type ToolSchemaShape = Record; - -export interface PluginCliMeta { - /** Optional override of derived CLI name */ - readonly name?: string; - /** Full schema shape for CLI flag generation (legacy, includes session-managed fields) */ - readonly schema?: ToolSchemaShape; - /** Mark tool as requiring daemon routing */ - readonly stateful?: boolean; -} - -export interface PluginMeta { - readonly name: string; // Verb used by MCP - readonly schema: ToolSchemaShape; // Zod validation schema (object schema) - readonly description?: string; // One-liner shown in help - readonly annotations?: ToolAnnotations; // MCP tool annotations for LLM behavior hints - readonly cli?: PluginCliMeta; // CLI-specific metadata (optional) - handler(params: Record): Promise; -} - -export interface WorkflowMeta { - readonly name: string; - readonly description: string; -} - -export interface WorkflowGroup { - readonly workflow: WorkflowMeta; - readonly tools: PluginMeta[]; - readonly directoryName: string; -} - -export const defineTool = (meta: PluginMeta): PluginMeta => meta; diff --git a/src/mcp/tools/coverage/get_coverage_report.ts b/src/mcp/tools/coverage/get_coverage_report.ts index 3ab48783..6f3ce827 100644 --- a/src/mcp/tools/coverage/get_coverage_report.ts +++ b/src/mcp/tools/coverage/get_coverage_report.ts @@ -65,7 +65,7 @@ export async function get_coverage_reportLogic( const fileExistsValidation = validateFileExists(xcresultPath, context.fileSystem); if (!fileExistsValidation.isValid) { - return fileExistsValidation.errorResponse!; + return { content: [{ type: 'text', text: fileExistsValidation.errorMessage! }], isError: true }; } log('info', `Getting coverage report from: ${xcresultPath}`); diff --git a/src/mcp/tools/coverage/get_file_coverage.ts b/src/mcp/tools/coverage/get_file_coverage.ts index f1f719cf..87d7a519 100644 --- a/src/mcp/tools/coverage/get_file_coverage.ts +++ b/src/mcp/tools/coverage/get_file_coverage.ts @@ -77,7 +77,7 @@ export async function get_file_coverageLogic( const fileExistsValidation = validateFileExists(xcresultPath, context.fileSystem); if (!fileExistsValidation.isValid) { - return fileExistsValidation.errorResponse!; + return { content: [{ type: 'text', text: fileExistsValidation.errorMessage! }], isError: true }; } log('info', `Getting file coverage for "${file}" from: ${xcresultPath}`); diff --git a/src/mcp/tools/macos/launch_mac_app.ts b/src/mcp/tools/macos/launch_mac_app.ts index 0cb65b3c..e7344705 100644 --- a/src/mcp/tools/macos/launch_mac_app.ts +++ b/src/mcp/tools/macos/launch_mac_app.ts @@ -30,7 +30,7 @@ export async function launch_mac_appLogic( // Validate that the app file exists const fileExistsValidation = validateFileExists(params.appPath, fileSystem); if (!fileExistsValidation.isValid) { - return fileExistsValidation.errorResponse!; + return { content: [{ type: 'text', text: fileExistsValidation.errorMessage! }], isError: true }; } log('info', `Starting launch macOS app request for ${params.appPath}`); diff --git a/src/mcp/tools/simulator/install_app_sim.ts b/src/mcp/tools/simulator/install_app_sim.ts index 144b60df..79e02e6f 100644 --- a/src/mcp/tools/simulator/install_app_sim.ts +++ b/src/mcp/tools/simulator/install_app_sim.ts @@ -48,7 +48,10 @@ export async function install_app_simLogic( ): Promise { const appPathExistsValidation = validateFileExists(params.appPath, fileSystem); if (!appPathExistsValidation.isValid) { - return appPathExistsValidation.errorResponse!; + return { + content: [{ type: 'text', text: appPathExistsValidation.errorMessage! }], + isError: true, + }; } log('info', `Starting xcrun simctl install request for simulator ${params.simulatorId}`); diff --git a/src/runtime/__tests__/tool-invoker.test.ts b/src/runtime/__tests__/tool-invoker.test.ts index cb3da448..aa4f3d5d 100644 --- a/src/runtime/__tests__/tool-invoker.test.ts +++ b/src/runtime/__tests__/tool-invoker.test.ts @@ -1,31 +1,54 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { z } from 'zod'; import type { ToolResponse } from '../../types/common.ts'; +import type { PipelineEvent } from '../../types/pipeline-events.ts'; +import type { DaemonToolResult } from '../../daemon/protocol.ts'; import type { ToolDefinition } from '../types.ts'; import { createToolCatalog } from '../tool-catalog.ts'; import { DefaultToolInvoker } from '../tool-invoker.ts'; +import { createRenderSession } from '../../rendering/render.ts'; import { ensureDaemonRunning } from '../../cli/daemon-control.ts'; +import { statusLine } from '../../utils/tool-event-builders.ts'; const daemonClientMock = { isRunning: vi.fn<() => Promise>(), invokeXcodeIdeTool: - vi.fn<(name: string, args: Record) => Promise>(), - invokeTool: vi.fn<(name: string, args: Record) => Promise>(), + vi.fn<(name: string, args: Record) => Promise>(), + invokeTool: vi.fn<(name: string, args: Record) => Promise>(), listTools: vi.fn<() => Promise>>(), }; -vi.mock('../../cli/daemon-client.ts', () => ({ - DaemonClient: vi.fn().mockImplementation(() => daemonClientMock), -})); +vi.mock('../../cli/daemon-client.ts', () => { + class VersionMismatchError extends Error { + constructor(message: string) { + super(message); + this.name = 'DaemonVersionMismatchError'; + } + } + return { + DaemonClient: vi.fn().mockImplementation(() => daemonClientMock), + DaemonVersionMismatchError: VersionMismatchError, + }; +}); vi.mock('../../cli/daemon-control.ts', () => ({ ensureDaemonRunning: vi.fn(), + forceStopDaemon: vi.fn(), DEFAULT_DAEMON_STARTUP_TIMEOUT_MS: 5000, })); -function textResponse(text: string): ToolResponse { +function daemonResult(text: string, opts?: Partial): DaemonToolResult { return { - content: [{ type: 'text', text }], + events: [ + { + type: 'status-line', + timestamp: new Date().toISOString(), + level: 'success', + message: text, + }, + ], + isError: false, + ...opts, }; } @@ -54,17 +77,74 @@ function makeTool(opts: { }; } +function invokeAndFinalize( + invoker: DefaultToolInvoker, + toolName: string, + args: Record, + opts: { + runtime: 'cli' | 'daemon' | 'mcp'; + socketPath?: string; + workspaceRoot?: string; + cliExposedWorkflowIds?: string[]; + }, +) { + const session = createRenderSession('text'); + const promise = invoker.invoke(toolName, args, { ...opts, renderSession: session }); + return promise.then(() => { + const text = session.finalize(); + const events = [...session.getEvents()]; + return { + content: text ? [{ type: 'text' as const, text }] : [], + isError: session.isError() || undefined, + nextSteps: undefined as ToolResponse['nextSteps'], + ...(events.length > 0 ? { _meta: { events } } : {}), + } as ToolResponse; + }); +} + +function emitHandler(text: string): ToolDefinition['handler'] { + return vi.fn(async (_params, ctx) => { + ctx.emit(statusLine('success', text)); + }); +} + +function emitErrorHandler(text: string): ToolDefinition['handler'] { + return vi.fn(async (_params, ctx) => { + ctx.emit(statusLine('error', text)); + }); +} + +function emitNextStepsHandler( + text: string, + nextSteps: ToolResponse['nextSteps'], + nextStepParams?: ToolResponse['nextStepParams'], +): ToolDefinition['handler'] { + return vi.fn(async (_params, ctx) => { + ctx.emit(statusLine('success', text)); + if (nextSteps) ctx.nextSteps = nextSteps; + if (nextStepParams) ctx.nextStepParams = nextStepParams; + }); +} + +function emitErrorEventsHandler(events: PipelineEvent[]): ToolDefinition['handler'] { + return vi.fn(async (_params, ctx) => { + for (const event of events) { + ctx.emit(event); + } + }); +} + describe('DefaultToolInvoker CLI routing', () => { beforeEach(() => { vi.clearAllMocks(); daemonClientMock.isRunning.mockResolvedValue(true); - daemonClientMock.invokeXcodeIdeTool.mockResolvedValue(textResponse('daemon-xcode-ide-result')); - daemonClientMock.invokeTool.mockResolvedValue(textResponse('daemon-result')); + daemonClientMock.invokeXcodeIdeTool.mockResolvedValue(daemonResult('daemon-xcode-ide-result')); + daemonClientMock.invokeTool.mockResolvedValue(daemonResult('daemon-result')); daemonClientMock.listTools.mockResolvedValue([]); }); it('uses direct invocation for stateless tools', async () => { - const directHandler = vi.fn().mockResolvedValue(textResponse('direct-result')); + const directHandler = emitHandler('direct-result'); const catalog = createToolCatalog([ makeTool({ cliName: 'list-sims', @@ -75,7 +155,8 @@ describe('DefaultToolInvoker CLI routing', () => { ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke( + const response = await invokeAndFinalize( + invoker, 'list-sims', { value: 'hello' }, { @@ -84,15 +165,21 @@ describe('DefaultToolInvoker CLI routing', () => { }, ); - expect(directHandler).toHaveBeenCalledWith({ value: 'hello' }); + expect(directHandler).toHaveBeenCalledWith( + { value: 'hello' }, + expect.objectContaining({ + emit: expect.any(Function), + attach: expect.any(Function), + }), + ); expect(daemonClientMock.isRunning).not.toHaveBeenCalled(); expect(daemonClientMock.invokeTool).not.toHaveBeenCalled(); - expect(response.content[0].text).toBe('direct-result'); + expect(response.content[0].text).toContain('direct-result'); }); it('routes stateful tools through daemon and auto-starts when needed', async () => { daemonClientMock.isRunning.mockResolvedValue(false); - const directHandler = vi.fn().mockResolvedValue(textResponse('direct-result')); + const directHandler = emitHandler('direct-result'); const catalog = createToolCatalog([ makeTool({ cliName: 'start-sim-log-cap', @@ -103,7 +190,8 @@ describe('DefaultToolInvoker CLI routing', () => { ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke( + const response = await invokeAndFinalize( + invoker, 'start-sim-log-cap', { value: 'hello' }, { @@ -124,7 +212,7 @@ describe('DefaultToolInvoker CLI routing', () => { value: 'hello', }); expect(directHandler).not.toHaveBeenCalled(); - expect(response.content[0].text).toBe('daemon-result'); + expect(response.content[0].text).toContain('daemon-result'); }); }); @@ -132,14 +220,14 @@ describe('DefaultToolInvoker xcode-ide dynamic routing', () => { beforeEach(() => { vi.clearAllMocks(); daemonClientMock.isRunning.mockResolvedValue(true); - daemonClientMock.invokeXcodeIdeTool.mockResolvedValue(textResponse('daemon-result')); - daemonClientMock.invokeTool.mockResolvedValue(textResponse('daemon-generic')); + daemonClientMock.invokeXcodeIdeTool.mockResolvedValue(daemonResult('daemon-result')); + daemonClientMock.invokeTool.mockResolvedValue(daemonResult('daemon-generic')); daemonClientMock.listTools.mockResolvedValue([]); }); it('routes dynamic xcode-ide tools through daemon xcode-ide invoke API', async () => { daemonClientMock.isRunning.mockResolvedValue(false); - const directHandler = vi.fn().mockResolvedValue(textResponse('direct-result')); + const directHandler = emitHandler('direct-result'); const catalog = createToolCatalog([ makeTool({ cliName: 'xcode-ide-alpha', @@ -151,7 +239,8 @@ describe('DefaultToolInvoker xcode-ide dynamic routing', () => { ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke( + const response = await invokeAndFinalize( + invoker, 'xcode-ide-alpha', { value: 'hello' }, { @@ -171,11 +260,11 @@ describe('DefaultToolInvoker xcode-ide dynamic routing', () => { ); expect(daemonClientMock.invokeXcodeIdeTool).toHaveBeenCalledWith('Alpha', { value: 'hello' }); expect(directHandler).not.toHaveBeenCalled(); - expect(response.content[0].text).toBe('daemon-result'); + expect(response.content[0].text).toContain('daemon-result'); }); it('fails for dynamic xcode-ide tools when socket path is missing', async () => { - const directHandler = vi.fn().mockResolvedValue(textResponse('direct-result')); + const directHandler = emitHandler('direct-result'); const catalog = createToolCatalog([ makeTool({ cliName: 'xcode-ide-alpha', @@ -187,7 +276,8 @@ describe('DefaultToolInvoker xcode-ide dynamic routing', () => { ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke( + const response = await invokeAndFinalize( + invoker, 'xcode-ide-alpha', { value: 'hello' }, { @@ -209,16 +299,13 @@ describe('DefaultToolInvoker next steps post-processing', () => { }); it('enriches canonical next-step tool names in CLI runtime', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextSteps: [ - { - tool: 'screenshot', - label: 'Take screenshot', - params: { simulatorId: '123' }, - }, - ], - } satisfies ToolResponse); + const directHandler = emitNextStepsHandler('ok', [ + { + tool: 'screenshot', + label: 'Take screenshot', + params: { simulatorId: '123' }, + }, + ]); const catalog = createToolCatalog([ makeTool({ @@ -234,32 +321,26 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'screenshot', workflow: 'ui-automation', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('screenshot')), + handler: emitHandler('screenshot'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('snapshot-ui', {}, { runtime: 'cli' }); + const response = await invokeAndFinalize(invoker, 'snapshot-ui', {}, { runtime: 'cli' }); - expect(response.nextSteps).toEqual([ - { - tool: 'screenshot', - label: 'Take screenshot', - params: { simulatorId: '123' }, - workflow: 'ui-automation', - cliTool: 'screenshot', - }, - ]); + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Next steps:'); + expect(text).toContain('Take screenshot'); + expect(text).toContain('xcodebuildmcp ui-automation screenshot --simulator-id "123"'); }); it('injects manifest template next steps from dynamic nextStepParams when response omits nextSteps', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextStepParams: { - snapshot_ui: { simulatorId: '12345678-1234-4234-8234-123456789012' }, - tap: { simulatorId: '12345678-1234-4234-8234-123456789012', x: 0, y: 0 }, - }, - } satisfies ToolResponse); + const directHandler = emitNextStepsHandler('ok', undefined, { + snapshot_ui: { simulatorId: '12345678-1234-4234-8234-123456789012' }, + tap: { simulatorId: '12345678-1234-4234-8234-123456789012', x: 0, y: 0 }, + }); + const catalog = createToolCatalog([ makeTool({ id: 'snapshot_ui', @@ -290,38 +371,58 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'tap', workflow: 'ui-automation', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('tap')), + handler: emitHandler('tap'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('snapshot-ui', {}, { runtime: 'cli' }); + const response = await invokeAndFinalize(invoker, 'snapshot-ui', {}, { runtime: 'cli' }); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Refresh'); + expect(text).toContain('snapshot-ui'); + expect(text).toContain('Visually verify hierarchy output'); + expect(text).toContain('Tap on element'); + expect(text).toContain('tap'); + }); - expect(response.nextSteps).toEqual([ - { - tool: 'snapshot_ui', - label: 'Refresh', - params: { simulatorId: '12345678-1234-4234-8234-123456789012' }, - workflow: 'ui-automation', - cliTool: 'snapshot-ui', - }, - { - label: 'Visually verify hierarchy output', - }, - { - tool: 'tap', - label: 'Tap on element', - params: { simulatorId: '12345678-1234-4234-8234-123456789012', x: 0, y: 0 }, - workflow: 'ui-automation', - cliTool: 'tap', - }, + it('does not inject manifest template next steps when the tool explicitly returns an empty list', async () => { + const directHandler = emitNextStepsHandler('ok', []); + + const catalog = createToolCatalog([ + makeTool({ + id: 'list_devices', + cliName: 'list', + mcpName: 'list_devices', + workflow: 'device', + stateful: false, + nextStepTemplates: [{ label: 'Build for device', toolId: 'build_device' }], + handler: directHandler, + }), + makeTool({ + id: 'build_device', + cliName: 'build', + mcpName: 'build_device', + workflow: 'device', + stateful: false, + handler: emitHandler('build'), + }), ]); + + const invoker = new DefaultToolInvoker(catalog); + const response = await invokeAndFinalize(invoker, 'list', {}, { runtime: 'cli' }); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('ok'); + expect(text).not.toContain('Next steps:'); }); it('prefers manifest templates over tool-provided next-step labels and tools', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextSteps: [ + const directHandler = emitNextStepsHandler( + 'ok', + [ { tool: 'legacy_stop_sim_log_cap', label: 'Old label', @@ -329,10 +430,10 @@ describe('DefaultToolInvoker next steps post-processing', () => { priority: 99, }, ], - nextStepParams: { + { stop_sim_log_cap: { logSessionId: 'session-123' }, }, - } satisfies ToolResponse); + ); const catalog = createToolCatalog([ makeTool({ @@ -356,37 +457,38 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'stop_sim_log_cap', workflow: 'logging', stateful: true, - handler: vi.fn().mockResolvedValue(textResponse('stop')), + handler: emitHandler('stop'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('start-simulator-log-capture', {}, { runtime: 'cli' }); + const response = await invokeAndFinalize( + invoker, + 'start-simulator-log-capture', + {}, + { runtime: 'cli' }, + ); - expect(response.nextSteps).toEqual([ - { - tool: 'stop_sim_log_cap', - label: 'Stop capture and retrieve logs', - params: { logSessionId: 'session-123' }, - priority: 1, - workflow: 'logging', - cliTool: 'stop-simulator-log-capture', - }, - ]); + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Stop capture and retrieve logs'); + expect(text).toContain('stop-simulator-log-capture'); + expect(text).toContain('session-123'); }); it('preserves daemon-provided next-step params when nextStepParams are already consumed', async () => { - daemonClientMock.invokeTool.mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextSteps: [ - { - tool: 'stop_sim_log_cap', - label: 'Stop capture and retrieve logs', - params: { logSessionId: 'session-123' }, - priority: 1, - }, - ], - } satisfies ToolResponse); + daemonClientMock.invokeTool.mockResolvedValue( + daemonResult('ok', { + nextSteps: [ + { + tool: 'stop_sim_log_cap', + label: 'Stop capture and retrieve logs', + params: { logSessionId: 'session-123' }, + priority: 1, + }, + ], + }), + ); const catalog = createToolCatalog([ makeTool({ @@ -402,7 +504,7 @@ describe('DefaultToolInvoker next steps post-processing', () => { priority: 1, }, ], - handler: vi.fn().mockResolvedValue(textResponse('start')), + handler: emitHandler('start'), }), makeTool({ id: 'stop_sim_log_cap', @@ -410,12 +512,13 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'stop_sim_log_cap', workflow: 'logging', stateful: true, - handler: vi.fn().mockResolvedValue(textResponse('stop')), + handler: emitHandler('stop'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke( + const response = await invokeAndFinalize( + invoker, 'start-simulator-log-capture', {}, { @@ -424,25 +527,17 @@ describe('DefaultToolInvoker next steps post-processing', () => { }, ); - expect(response.nextSteps).toEqual([ - { - tool: 'stop_sim_log_cap', - label: 'Stop capture and retrieve logs', - params: { logSessionId: 'session-123' }, - priority: 1, - workflow: 'logging', - cliTool: 'stop-simulator-log-capture', - }, - ]); + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Stop capture and retrieve logs'); + expect(text).toContain('stop-simulator-log-capture'); + expect(text).toContain('session-123'); }); it('overrides unresolved template placeholders with dynamic next-step params', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextStepParams: { - boot_sim: { simulatorId: 'ABC-123' }, - }, - } satisfies ToolResponse); + const directHandler = emitNextStepsHandler('ok', undefined, { + boot_sim: { simulatorId: 'ABC-123' }, + }); const catalog = createToolCatalog([ makeTool({ @@ -466,31 +561,24 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'boot_sim', workflow: 'simulator', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('boot')), + handler: emitHandler('boot'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('launch-app-sim', {}, { runtime: 'cli' }); + const response = await invokeAndFinalize(invoker, 'launch-app-sim', {}, { runtime: 'cli' }); - expect(response.nextSteps).toEqual([ - { - tool: 'boot_sim', - label: 'Boot simulator', - params: { simulatorId: 'ABC-123' }, - workflow: 'simulator', - cliTool: 'boot-sim', - }, - ]); + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Boot simulator'); + expect(text).toContain('boot-sim'); + expect(text).toContain('ABC-123'); }); it('maps dynamic params to the correct template tool after catalog filtering', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextStepParams: { - stop_sim_log_cap: { logSessionId: 'session-123' }, - }, - } satisfies ToolResponse); + const directHandler = emitNextStepsHandler('ok', undefined, { + stop_sim_log_cap: { logSessionId: 'session-123' }, + }); const catalog = createToolCatalog([ makeTool({ @@ -518,37 +606,138 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'stop_sim_log_cap', workflow: 'logging', stateful: true, - handler: vi.fn().mockResolvedValue(textResponse('stop')), + handler: emitHandler('stop'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('start-simulator-log-capture', {}, { runtime: 'cli' }); + const response = await invokeAndFinalize( + invoker, + 'start-simulator-log-capture', + {}, + { runtime: 'cli' }, + ); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Stop capture and retrieve logs'); + expect(text).toContain('stop-simulator-log-capture'); + expect(text).toContain('session-123'); + }); - expect(response.nextSteps).toEqual([ + it('renders failure next steps for ordinary error responses with replayable events', async () => { + const directHandler = emitErrorEventsHandler([ { - tool: 'stop_sim_log_cap', - label: 'Stop capture and retrieve logs', - params: { logSessionId: 'session-123' }, - priority: 1, - workflow: 'logging', - cliTool: 'stop-simulator-log-capture', + type: 'status-line', + timestamp: new Date().toISOString(), + level: 'error', + message: 'failed', }, ]); + + const catalog = createToolCatalog([ + makeTool({ + id: 'list_devices', + cliName: 'list', + mcpName: 'list_devices', + workflow: 'device', + stateful: false, + nextStepTemplates: [ + { + label: 'Try building for device', + toolId: 'build_device', + when: 'failure', + }, + ], + handler: directHandler, + }), + makeTool({ + id: 'build_device', + cliName: 'build-device', + mcpName: 'build_device', + workflow: 'device', + stateful: false, + handler: emitHandler('build'), + }), + ]); + + const invoker = new DefaultToolInvoker(catalog); + const response = await invokeAndFinalize(invoker, 'list', {}, { runtime: 'cli' }); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((item) => (item.type === 'text' ? item.text : '')).join('\n'); + expect(text).toContain('Try building for device'); + expect(text).toContain('build-device'); + }); + + it('suppresses failure next steps for structured xcodebuild failures emitted via handler context', async () => { + const directHandler = emitErrorEventsHandler([ + { + type: 'header', + timestamp: '2026-03-20T12:00:00.000Z', + operation: 'Build', + params: [{ label: 'Scheme', value: 'MyApp' }], + }, + { + type: 'compiler-error', + timestamp: '2026-03-20T12:00:00.500Z', + operation: 'BUILD', + message: 'Build failed', + rawLine: 'Build failed', + }, + { + type: 'summary', + timestamp: '2026-03-20T12:00:01.000Z', + status: 'FAILED', + operation: 'BUILD', + durationMs: 1000, + }, + ]); + + const catalog = createToolCatalog([ + makeTool({ + id: 'build_device', + cliName: 'build-device', + mcpName: 'build_device', + workflow: 'device', + stateful: false, + nextStepTemplates: [ + { + label: 'Try building for device', + toolId: 'list_devices', + when: 'failure', + }, + ], + handler: directHandler, + }), + makeTool({ + id: 'list_devices', + cliName: 'list-devices', + mcpName: 'list_devices', + workflow: 'device', + stateful: false, + handler: emitHandler('devices'), + }), + ]); + + const invoker = new DefaultToolInvoker(catalog); + const response = await invokeAndFinalize(invoker, 'build-device', {}, { runtime: 'cli' }); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((item) => (item.type === 'text' ? item.text : '')).join('\n'); + expect(text).not.toContain('Try building for device'); + expect(text).not.toContain('list-devices'); }); it('always uses manifest templates when they exist', async () => { - const directHandler = vi.fn().mockResolvedValue({ - content: [{ type: 'text', text: 'ok' }], - nextSteps: [ - { - tool: 'launch_app_sim', - label: 'Launch app (platform-specific)', - params: { simulatorId: '123', bundleId: 'com.example.app' }, - priority: 1, - }, - ], - } satisfies ToolResponse); + const directHandler = emitNextStepsHandler('ok', [ + { + tool: 'launch_app_sim', + label: 'Launch app (platform-specific)', + params: { simulatorId: '123', bundleId: 'com.example.app' }, + priority: 1, + }, + ]); const catalog = createToolCatalog([ makeTool({ @@ -569,7 +758,7 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'launch_app_sim', workflow: 'simulator', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('launch')), + handler: emitHandler('launch'), }), makeTool({ id: 'get_app_bundle_id', @@ -577,7 +766,7 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'get_app_bundle_id', workflow: 'project-discovery', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('bundle')), + handler: emitHandler('bundle'), }), makeTool({ id: 'boot_sim', @@ -585,30 +774,18 @@ describe('DefaultToolInvoker next steps post-processing', () => { mcpName: 'boot_sim', workflow: 'simulator', stateful: false, - handler: vi.fn().mockResolvedValue(textResponse('boot')), + handler: emitHandler('boot'), }), ]); const invoker = new DefaultToolInvoker(catalog); - const response = await invoker.invoke('get-app-path', {}, { runtime: 'cli' }); - - expect(response.nextSteps).toEqual([ - { - tool: 'get_app_bundle_id', - label: 'Get bundle ID', - params: {}, - priority: 1, - workflow: 'project-discovery', - cliTool: 'get-app-bundle-id', - }, - { - tool: 'boot_sim', - label: 'Boot simulator', - params: {}, - priority: 2, - workflow: 'simulator', - cliTool: 'boot', - }, - ]); + const response = await invokeAndFinalize(invoker, 'get-app-path', {}, { runtime: 'cli' }); + + expect(response.nextSteps).toBeUndefined(); + const text = response.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + expect(text).toContain('Get bundle ID'); + expect(text).toContain('get-app-bundle-id'); + expect(text).toContain('Boot simulator'); + expect(text).toContain('boot'); }); }); diff --git a/src/runtime/bootstrap-runtime.ts b/src/runtime/bootstrap-runtime.ts index a536aaa3..3bb0a238 100644 --- a/src/runtime/bootstrap-runtime.ts +++ b/src/runtime/bootstrap-runtime.ts @@ -87,12 +87,11 @@ function logHydrationResult(hydration: MCPSessionHydrationResult): void { return; } - if (hydration.refreshScheduled) { - log('info', '[Session] Hydrated MCP session defaults; simulator metadata refresh scheduled.'); - return; - } - - log('info', '[Session] Hydrated MCP session defaults; simulator metadata refresh not scheduled.'); + const refreshStatus = hydration.refreshScheduled ? 'scheduled' : 'not scheduled'; + log( + 'info', + `[Session] Hydrated MCP session defaults; simulator metadata refresh ${refreshStatus}.`, + ); } export async function bootstrapRuntime( diff --git a/src/runtime/tool-catalog.ts b/src/runtime/tool-catalog.ts index c2a1e9f1..7891cb57 100644 --- a/src/runtime/tool-catalog.ts +++ b/src/runtime/tool-catalog.ts @@ -87,13 +87,6 @@ export function createToolCatalog(tools: ToolDefinition[]): ToolCatalog { }; } -/** - * Get a list of all available tool names for display. - */ -export function listToolNames(catalog: ToolCatalog): string[] { - return catalog.tools.map((t) => t.cliName).sort(); -} - /** * Get tools grouped by workflow for display. */ diff --git a/src/runtime/tool-invoker.ts b/src/runtime/tool-invoker.ts index 3c34d5ac..51d04881 100644 --- a/src/runtime/tool-invoker.ts +++ b/src/runtime/tool-invoker.ts @@ -1,8 +1,13 @@ import type { ToolCatalog, ToolDefinition, ToolInvoker, InvokeOptions } from './types.ts'; -import type { NextStep, NextStepParams, NextStepParamsMap, ToolResponse } from '../types/common.ts'; -import { createErrorResponse } from '../utils/responses/index.ts'; -import { DaemonClient } from '../cli/daemon-client.ts'; -import { ensureDaemonRunning, DEFAULT_DAEMON_STARTUP_TIMEOUT_MS } from '../cli/daemon-control.ts'; +import type { NextStep, NextStepParams, NextStepParamsMap } from '../types/common.ts'; +import type { DaemonToolResult } from '../daemon/protocol.ts'; +import { statusLine } from '../utils/tool-event-builders.ts'; +import { DaemonClient, DaemonVersionMismatchError } from '../cli/daemon-client.ts'; +import { + ensureDaemonRunning, + forceStopDaemon, + DEFAULT_DAEMON_STARTUP_TIMEOUT_MS, +} from '../cli/daemon-control.ts'; import { log } from '../utils/logger.ts'; import { recordInternalErrorMetric, @@ -11,6 +16,8 @@ import { type SentryToolRuntime, type SentryToolTransport, } from '../utils/sentry.ts'; +import type { RenderSession, ToolHandlerContext } from '../rendering/types.ts'; +import { createRenderSession } from '../rendering/render.ts'; type BuiltTemplateNextStep = { step: NextStep; @@ -32,6 +39,7 @@ function buildTemplateNextSteps( step: { label: template.label, priority: template.priority, + when: template.when, }, }); continue; @@ -48,6 +56,7 @@ function buildTemplateNextSteps( label: template.label, params: template.params ?? {}, priority: template.priority, + when: template.when, }, templateToolId: template.toolId, }); @@ -106,91 +115,135 @@ function mergeTemplateAndResponseNextSteps( }); } -function normalizeNextSteps( - response: ToolResponse, - catalog: ToolCatalog, - runtime: InvokeOptions['runtime'], -): ToolResponse { - if (!response.nextSteps || response.nextSteps.length === 0) { - return response; - } +function normalizeNextSteps(steps: NextStep[], catalog: ToolCatalog): NextStep[] { + return steps.map((step) => { + if (!step.tool) { + return step; + } - return { - ...response, - nextSteps: response.nextSteps.map((step) => { - if (!step.tool) { - return step; - } + const target = catalog.getByMcpName(step.tool); + if (!target) { + return step; + } - const target = catalog.getByMcpName(step.tool); - if (!target) { - return step; - } + return { + ...step, + tool: target.mcpName, + workflow: target.workflow, + cliTool: target.cliName, + }; + }); +} + +function isStructuredXcodebuildFailureSession(session: RenderSession): boolean { + const events = session.getEvents(); + + const hasFailedSummary = events.some( + (event) => event.type === 'summary' && event.status === 'FAILED', + ); + const hasHeader = events.some((event) => event.type === 'header'); - return runtime === 'cli' - ? { - ...step, - tool: target.mcpName, - workflow: target.workflow, - cliTool: target.cliName, - } - : { - ...step, - tool: target.mcpName, - }; - }), - }; + return hasFailedSummary && hasHeader; } -export function postProcessToolResponse(params: { +function buildEffectiveNextStepParams( + nextStepParams: NextStepParamsMap | undefined, + handlerNextSteps: NextStep[] | undefined, + catalog: ToolCatalog, +): NextStepParamsMap | undefined { + if (!handlerNextSteps || handlerNextSteps.length === 0) { + return nextStepParams; + } + + let merged: NextStepParamsMap | undefined = nextStepParams; + for (const step of handlerNextSteps) { + if (!step.tool || !step.params || Object.keys(step.params).length === 0) { + continue; + } + const target = catalog.getByMcpName(step.tool); + const toolId = target?.id ?? step.tool; + if (merged?.[toolId]) { + continue; + } + merged = { ...merged, [toolId]: step.params as NextStepParams }; + } + return merged; +} + +export function postProcessSession(params: { tool: ToolDefinition; - response: ToolResponse; + session: RenderSession; + ctx: ToolHandlerContext; catalog: ToolCatalog; runtime: InvokeOptions['runtime']; applyTemplateNextSteps?: boolean; -}): ToolResponse { - const { tool, response, catalog, runtime, applyTemplateNextSteps = true } = params; +}): void { + const { tool, session, ctx, catalog, runtime, applyTemplateNextSteps = true } = params; - const templateSteps = buildTemplateNextSteps(tool, catalog); + const isError = session.isError(); + const nextStepParams = ctx.nextStepParams; + const handlerNextSteps = ctx.nextSteps; + const suppressNextStepsForStructuredFailure = + isError && isStructuredXcodebuildFailureSession(session); - const withTemplates = - applyTemplateNextSteps && templateSteps.length > 0 - ? { - ...response, - nextSteps: mergeTemplateAndResponseNextSteps(templateSteps, response.nextStepParams), - } - : response; + if (suppressNextStepsForStructuredFailure) { + return; + } - const result = normalizeNextSteps(withTemplates, catalog, runtime); - delete result.nextStepParams; - return result; -} + const suppressTemplateNextSteps = handlerNextSteps !== undefined && handlerNextSteps.length === 0; -function buildDaemonEnvOverrides(opts: InvokeOptions): Record | undefined { - const envOverrides: Record = {}; + const effectiveNextStepParams = buildEffectiveNextStepParams( + nextStepParams, + handlerNextSteps, + catalog, + ); + + const allTemplateSteps = buildTemplateNextSteps(tool, catalog); + const templateSteps = allTemplateSteps.filter((t) => { + const when = t.step.when ?? 'always'; + if (when === 'success') return !isError; + if (when === 'failure') return isError; + return true; + }); - if (opts.logLevel) { - envOverrides.XCODEBUILDMCP_DAEMON_LOG_LEVEL = opts.logLevel; + let finalSteps: NextStep[]; + + if (applyTemplateNextSteps && !suppressTemplateNextSteps && templateSteps.length > 0) { + finalSteps = mergeTemplateAndResponseNextSteps(templateSteps, effectiveNextStepParams); + } else if (handlerNextSteps && handlerNextSteps.length > 0) { + finalSteps = handlerNextSteps; + } else { + return; } - return Object.keys(envOverrides).length > 0 ? envOverrides : undefined; + const normalized = normalizeNextSteps(finalSteps, catalog); + + if (normalized.length > 0) { + session.emit({ + type: 'next-steps', + timestamp: new Date().toISOString(), + steps: normalized, + runtime, + }); + } } -function getErrorKind(error: unknown): string { - if (error instanceof Error) { - return error.name || 'Error'; +function buildDaemonEnvOverrides(opts: InvokeOptions): Record | undefined { + if (!opts.logLevel) { + return undefined; } - return typeof error; + return { XCODEBUILDMCP_DAEMON_LOG_LEVEL: opts.logLevel }; +} + +function getErrorKind(error: unknown): string { + return error instanceof Error ? error.name || 'Error' : typeof error; } function mapRuntimeToSentryToolRuntime(runtime: InvokeOptions['runtime']): SentryToolRuntime { - switch (runtime) { - case 'daemon': - case 'mcp': - return runtime; - default: - return 'cli'; + if (runtime === 'daemon' || runtime === 'mcp') { + return runtime; } + return 'cli'; } export class DefaultToolInvoker implements ToolInvoker { @@ -200,53 +253,46 @@ export class DefaultToolInvoker implements ToolInvoker { toolName: string, args: Record, opts: InvokeOptions, - ): Promise { + ): Promise { const resolved = this.catalog.resolve(toolName); + const session = opts.renderSession ?? createRenderSession('text'); + const resolvedOpts = { ...opts, renderSession: session }; if (resolved.ambiguous) { - return createErrorResponse( - 'Ambiguous tool name', - `Multiple tools match '${toolName}'. Use one of:\n- ${resolved.ambiguous.join('\n- ')}`, + session.emit( + statusLine( + 'error', + `Ambiguous tool name: Multiple tools match '${toolName}'. Use one of:\n- ${resolved.ambiguous.join('\n- ')}`, + ), ); + return; } if (resolved.notFound || !resolved.tool) { - return createErrorResponse( - 'Tool not found', - `Unknown tool '${toolName}'. Run 'xcodebuildmcp tools' to see available tools.`, + session.emit( + statusLine( + 'error', + `Tool not found: Unknown tool '${toolName}'. Run 'xcodebuildmcp tools' to see available tools.`, + ), ); + return; } - return this.executeTool(resolved.tool, args, opts); + return this.executeTool(resolved.tool, args, resolvedOpts); } - /** - * Invoke a tool directly, bypassing catalog resolution. - * Used by CLI where the correct ToolDefinition is already known - * from workflow-scoped yargs routing. - */ async invokeDirect( tool: ToolDefinition, args: Record, opts: InvokeOptions, - ): Promise { - return this.executeTool(tool, args, opts); - } - - private buildPostProcessParams( - tool: ToolDefinition, - runtime: InvokeOptions['runtime'], - ): { - tool: ToolDefinition; - catalog: ToolCatalog; - runtime: InvokeOptions['runtime']; - } { - return { tool, catalog: this.catalog, runtime }; + ): Promise { + const session = opts.renderSession ?? createRenderSession('text'); + return this.executeTool(tool, args, { ...opts, renderSession: session }); } private async invokeViaDaemon( opts: InvokeOptions, - invoke: (client: DaemonClient) => Promise, + invoke: (client: DaemonClient) => Promise, context: { label: string; errorTitle: string; @@ -258,16 +304,20 @@ export class DefaultToolInvoker implements ToolInvoker { runtime: InvokeOptions['runtime']; }; }, - ): Promise { + ): Promise { + const session = opts.renderSession!; const socketPath = opts.socketPath; if (!socketPath) { const error = new Error('SocketPathMissing'); context.captureInfraErrorMetric(error); context.captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Socket path required', - 'No socket path configured for daemon communication.', + session.emit( + statusLine( + 'error', + 'Socket path required: No socket path configured for daemon communication.', + ), ); + return; } const client = new DaemonClient({ socketPath }); @@ -289,24 +339,73 @@ export class DefaultToolInvoker implements ToolInvoker { ); context.captureInfraErrorMetric(error); context.captureInvocationMetric('infra_error'); - return createErrorResponse( - 'Daemon auto-start failed', - (error instanceof Error ? error.message : String(error)) + - '\n\nYou can try starting the daemon manually:\n' + - ' xcodebuildmcp daemon start', + session.emit( + statusLine( + 'error', + `Daemon auto-start failed: ${error instanceof Error ? error.message : String(error)}\n\nYou can try starting the daemon manually:\n xcodebuildmcp daemon start`, + ), ); + return; } } - try { - const response = await invoke(client); - context.captureInvocationMetric('completed'); - return postProcessToolResponse({ + const consumeResult = (daemonResult: DaemonToolResult): void => { + for (const event of daemonResult.events) { + session.emit(event); + } + + const ctx: ToolHandlerContext = { + emit: (event) => session.emit(event), + attach: (image) => session.attach(image), + nextStepParams: daemonResult.nextStepParams, + nextSteps: daemonResult.nextSteps, + }; + + postProcessSession({ ...context.postProcessParams, - response, - applyTemplateNextSteps: false, + session, + ctx, }); + }; + + try { + const daemonResult = await invoke(client); + context.captureInvocationMetric('completed'); + consumeResult(daemonResult); } catch (error) { + if (error instanceof DaemonVersionMismatchError) { + log('info', `[infra/tool-invoker] ${context.label} daemon protocol mismatch, restarting`); + await forceStopDaemon(socketPath); + try { + await ensureDaemonRunning({ + socketPath, + workspaceRoot: opts.workspaceRoot, + startupTimeoutMs: opts.daemonStartupTimeoutMs ?? DEFAULT_DAEMON_STARTUP_TIMEOUT_MS, + env: buildDaemonEnvOverrides(opts), + }); + const retryClient = new DaemonClient({ socketPath }); + const daemonResult = await invoke(retryClient); + context.captureInvocationMetric('completed'); + consumeResult(daemonResult); + return; + } catch (retryError) { + log( + 'error', + `[infra/tool-invoker] ${context.label} daemon restart failed (${getErrorKind(retryError)})`, + { sentry: true }, + ); + context.captureInfraErrorMetric(retryError); + context.captureInvocationMetric('infra_error'); + session.emit( + statusLine( + 'error', + `Daemon restart failed after protocol mismatch: ${retryError instanceof Error ? retryError.message : String(retryError)}\n\nTry restarting manually:\n xcodebuildmcp daemon stop && xcodebuildmcp daemon start`, + ), + ); + return; + } + } + log( 'error', `[infra/tool-invoker] ${context.label} transport failed (${getErrorKind(error)})`, @@ -314,9 +413,11 @@ export class DefaultToolInvoker implements ToolInvoker { ); context.captureInfraErrorMetric(error); context.captureInvocationMetric('infra_error'); - return createErrorResponse( - context.errorTitle, - error instanceof Error ? error.message : String(error), + session.emit( + statusLine( + 'error', + `${context.errorTitle}: ${error instanceof Error ? error.message : String(error)}`, + ), ); } } @@ -325,7 +426,7 @@ export class DefaultToolInvoker implements ToolInvoker { tool: ToolDefinition, args: Record, opts: InvokeOptions, - ): Promise { + ): Promise { const startedAt = Date.now(); const runtime = mapRuntimeToSentryToolRuntime(opts.runtime); let transport: SentryToolTransport = 'direct'; @@ -348,7 +449,7 @@ export class DefaultToolInvoker implements ToolInvoker { }); }; - const postProcessParams = this.buildPostProcessParams(tool, opts.runtime); + const postProcessParams = { tool, catalog: this.catalog, runtime: opts.runtime }; const xcodeIdeRemoteToolName = tool.xcodeIdeRemoteToolName; const isDynamicXcodeIdeTool = tool.workflow === 'xcode-ide' && typeof xcodeIdeRemoteToolName === 'string'; @@ -380,13 +481,27 @@ export class DefaultToolInvoker implements ToolInvoker { } // Direct invocation (CLI stateless or daemon internal) + const session = opts.renderSession!; try { - const response = await tool.handler(args); + const ctx: ToolHandlerContext = opts.handlerContext ?? { + emit: (event) => { + session.emit(event); + }, + attach: (image) => { + session.attach(image); + }, + }; + await tool.handler(args, ctx); + captureInvocationMetric('completed'); - return postProcessToolResponse({ - ...postProcessParams, - response, - }); + + if (opts.runtime !== 'daemon') { + postProcessSession({ + ...postProcessParams, + session, + ctx, + }); + } } catch (error) { log( 'error', @@ -396,7 +511,7 @@ export class DefaultToolInvoker implements ToolInvoker { captureInfraErrorMetric(error); captureInvocationMetric('infra_error'); const message = error instanceof Error ? error.message : String(error); - return createErrorResponse('Tool execution failed', message); + session.emit(statusLine('error', `Tool execution failed: ${message}`)); } } } diff --git a/src/runtime/types.ts b/src/runtime/types.ts index ef9a505d..4a2020c4 100644 --- a/src/runtime/types.ts +++ b/src/runtime/types.ts @@ -1,12 +1,13 @@ import type { ToolAnnotations } from '@modelcontextprotocol/sdk/types.js'; -import type { ToolResponse } from '../types/common.ts'; -import type { ToolSchemaShape, PluginMeta } from '../core/plugin-types.ts'; +import type { ToolSchemaShape } from '../core/plugin-types.ts'; +import type { RenderSession, ToolHandlerContext } from '../rendering/types.ts'; export interface NextStepTemplate { label: string; toolId?: string; params?: Record; priority?: number; + when?: 'always' | 'success' | 'failure'; } export type RuntimeKind = 'cli' | 'daemon' | 'mcp'; @@ -54,7 +55,7 @@ export interface ToolDefinition { /** * Shared handler (same used by MCP). No duplication. */ - handler: PluginMeta['handler']; + handler: (params: Record, ctx: ToolHandlerContext) => Promise; } export interface ToolResolution { @@ -81,6 +82,9 @@ export interface ToolCatalog { export interface InvokeOptions { runtime: RuntimeKind; + renderSession?: RenderSession; + /** Pre-created handler context; if provided, executeTool uses it instead of creating a new one. */ + handlerContext?: ToolHandlerContext; /** CLI-exposed workflow IDs used for daemon environment overrides */ cliExposedWorkflowIds?: string[]; /** @deprecated Use cliExposedWorkflowIds instead */ @@ -96,9 +100,5 @@ export interface InvokeOptions { } export interface ToolInvoker { - invoke( - toolName: string, - args: Record, - opts: InvokeOptions, - ): Promise; + invoke(toolName: string, args: Record, opts: InvokeOptions): Promise; } diff --git a/src/types/common.ts b/src/types/common.ts index 50481ff0..c607e1a2 100644 --- a/src/types/common.ts +++ b/src/types/common.ts @@ -28,6 +28,8 @@ export interface NextStep { params?: Record; /** Optional ordering hint for merged steps */ priority?: number; + /** When to show this step: 'always' (default), 'success', or 'failure' */ + when?: 'always' | 'success' | 'failure'; } export type NextStepParams = Record; @@ -102,8 +104,7 @@ export function createImageContent( */ export interface ValidationResult { isValid: boolean; - errorResponse?: ToolResponse; - warningResponse?: ToolResponse; + errorMessage?: string; } /** @@ -140,4 +141,5 @@ export interface PlatformBuildOptions { packageCachePath?: string; arch?: string; logPrefix: string; + packageCachePath?: string; } diff --git a/src/utils/__tests__/session-aware-tool-factory.test.ts b/src/utils/__tests__/session-aware-tool-factory.test.ts index 193c6145..197c2a78 100644 --- a/src/utils/__tests__/session-aware-tool-factory.test.ts +++ b/src/utils/__tests__/session-aware-tool-factory.test.ts @@ -1,6 +1,10 @@ import { describe, it, expect, beforeEach } from 'vitest'; import * as z from 'zod'; -import { createSessionAwareTool } from '../typed-tool-factory.ts'; +import { + createSessionAwareTool, + getHandlerContext, + type ToolHandler, +} from '../typed-tool-factory.ts'; import { sessionStore } from '../session-store.ts'; import { createMockExecutor, @@ -11,6 +15,9 @@ import { initConfigStore, type RuntimeConfigOverrides, } from '../config-store.ts'; +import { createRenderSession } from '../../rendering/render.ts'; +import type { ToolHandlerContext } from '../../rendering/types.ts'; +import { statusLine } from '../tool-event-builders.ts'; const cwd = '/repo'; @@ -19,6 +26,21 @@ async function initConfigStoreForTest(overrides?: RuntimeConfigOverrides): Promi await initConfigStore({ cwd, fs: createMockFileSystemExecutor(), overrides }); } +function invokeAndCollect( + handler: ToolHandler, + args: Record, +): Promise<{ text: string; isError: boolean }> { + const session = createRenderSession('text'); + const ctx: ToolHandlerContext = { + emit: (event) => session.emit(event), + attach: (image) => session.attach(image), + }; + return handler(args, ctx).then(() => ({ + text: session.finalize(), + isError: session.isError(), + })); +} + describe('createSessionAwareTool', () => { beforeEach(async () => { sessionStore.clearAll(); @@ -44,8 +66,9 @@ describe('createSessionAwareTool', () => { type Params = z.infer; - async function logic(_params: Params): Promise { - return { content: [{ type: 'text', text: 'OK' }], isError: false }; + async function logic(_params: Params): Promise { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', 'OK')); } const handler = createSessionAwareTool({ @@ -66,19 +89,18 @@ describe('createSessionAwareTool', () => { simulatorId: 'SIM-1', }); - const result = await handler({}); + const result = await invokeAndCollect(handler, {}); expect(result.isError).toBe(false); - expect(result.content[0].text).toBe('OK'); + expect(result.text).toContain('OK'); }); it('should prefer explicit args over session defaults (same key wins)', async () => { - // Create a handler that echoes the chosen scheme const echoHandler = createSessionAwareTool({ internalSchema, - logicFunction: async (params) => ({ - content: [{ type: 'text', text: params.scheme }], - isError: false, - }), + logicFunction: async (params) => { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', params.scheme)); + }, getExecutor: () => createMockExecutor({ success: true }), requirements: [ { allOf: ['scheme'], message: 'scheme is required' }, @@ -95,45 +117,51 @@ describe('createSessionAwareTool', () => { projectPath: '/a.xcodeproj', simulatorId: 'SIM-A', }); - const result = await echoHandler({ scheme: 'FromArgs' }); + const result = await invokeAndCollect(echoHandler, { scheme: 'FromArgs' }); expect(result.isError).toBe(false); - expect(result.content[0].text).toBe('FromArgs'); + expect(result.text).toContain('FromArgs'); }); it('should return friendly error when allOf requirement missing', async () => { - const result = await handler({ projectPath: '/p.xcodeproj', simulatorId: 'SIM-1' }); + const result = await invokeAndCollect(handler, { + projectPath: '/p.xcodeproj', + simulatorId: 'SIM-1', + }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Missing required session defaults'); - expect(result.content[0].text).toContain('scheme is required'); + expect(result.text).toContain('Missing required session defaults'); + expect(result.text).toContain('scheme is required'); }); it('should return friendly error when oneOf requirement missing', async () => { - const result = await handler({ scheme: 'App', simulatorId: 'SIM-1' }); + const result = await invokeAndCollect(handler, { scheme: 'App', simulatorId: 'SIM-1' }); 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'); + expect(result.text).toContain('Missing required session defaults'); + expect(result.text).toContain('Provide a project or workspace'); }); it('uses opt-out messaging when session defaults schema is disabled', async () => { await initConfigStoreForTest({ disableSessionDefaults: true }); - const result = await handler({ projectPath: '/p.xcodeproj', simulatorId: 'SIM-1' }); + const result = await invokeAndCollect(handler, { + projectPath: '/p.xcodeproj', + simulatorId: 'SIM-1', + }); expect(result.isError).toBe(true); - const text = result.content[0].text; + const text = result.text; expect(text).toContain('Missing required parameters'); expect(text).toContain('scheme is required'); expect(text).not.toContain('session defaults'); }); it('should surface Zod validation errors when invalid', async () => { - const badHandler = createSessionAwareTool({ + const badHandler = createSessionAwareTool({ internalSchema, - logicFunction: logic, + logicFunction: logic as (params: unknown, executor: unknown) => Promise, getExecutor: () => createMockExecutor({ success: true }), }); - const result = await badHandler({ scheme: 123 }); + const result = await invokeAndCollect(badHandler, { scheme: 123 }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); + expect(result.text).toContain('Parameter validation failed'); }); it('exclusivePairs should NOT prune session defaults when user provides null (treat as not provided)', async () => { @@ -154,9 +182,11 @@ describe('createSessionAwareTool', () => { simulatorId: 'SIM-1', }); - const res = await handlerWithExclusive({ workspacePath: null as unknown as string }); + const res = await invokeAndCollect(handlerWithExclusive, { + workspacePath: null as unknown as string, + }); expect(res.isError).toBe(false); - expect(res.content[0].text).toBe('OK'); + expect(res.text).toContain('OK'); }); it('exclusivePairs should NOT prune when user provides undefined (treated as not provided)', async () => { @@ -177,9 +207,11 @@ describe('createSessionAwareTool', () => { simulatorId: 'SIM-1', }); - const res = await handlerWithExclusive({ workspacePath: undefined as unknown as string }); + const res = await invokeAndCollect(handlerWithExclusive, { + workspacePath: undefined as unknown as string, + }); expect(res.isError).toBe(false); - expect(res.content[0].text).toBe('OK'); + expect(res.text).toContain('OK'); }); it('rejects when multiple explicit args in an exclusive pair are provided (factory-level)', async () => { @@ -191,23 +223,23 @@ describe('createSessionAwareTool', () => { const handlerNoXor = createSessionAwareTool>({ internalSchema: internalSchemaNoXor, - logicFunction: (async () => ({ - content: [{ type: 'text', text: 'OK' }], - isError: false, - })) as any, + logicFunction: (async () => { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', 'OK')); + }) as (params: z.infer, executor: unknown) => Promise, getExecutor: () => createMockExecutor({ success: true }), requirements: [{ allOf: ['scheme'], message: 'scheme is required' }], exclusivePairs: [['projectPath', 'workspacePath']], }); - const res = await handlerNoXor({ + const res = await invokeAndCollect(handlerNoXor, { scheme: 'App', projectPath: '/path/a.xcodeproj', workspacePath: '/path/b.xcworkspace', }); expect(res.isError).toBe(true); - const msg = res.content[0].text; + const msg = res.text; expect(msg).toContain('Parameter validation failed'); expect(msg).toContain('Mutually exclusive parameters provided'); expect(msg).toContain('projectPath'); @@ -215,7 +247,6 @@ describe('createSessionAwareTool', () => { }); it('prefers first key when both values of exclusive pair come from session defaults', async () => { - // Create handler that echoes which simulator param was used const echoHandler = createSessionAwareTool({ internalSchema: z.object({ scheme: z.string(), @@ -223,24 +254,23 @@ describe('createSessionAwareTool', () => { simulatorId: z.string().optional(), simulatorName: z.string().optional(), }), - logicFunction: async (params) => ({ - content: [ - { - type: 'text', - text: JSON.stringify({ + logicFunction: async (params) => { + const ctx = getHandlerContext(); + ctx.emit( + statusLine( + 'success', + JSON.stringify({ simulatorId: params.simulatorId, simulatorName: params.simulatorName, }), - }, - ], - isError: false, - }), + ), + ); + }, getExecutor: () => createMockExecutor({ success: true }), requirements: [{ allOf: ['scheme'] }], exclusivePairs: [['simulatorId', 'simulatorName']], }); - // Set both simulatorId and simulatorName in session defaults sessionStore.setDefaults({ scheme: 'App', projectPath: '/a.xcodeproj', @@ -248,13 +278,10 @@ describe('createSessionAwareTool', () => { simulatorName: 'iPhone 17', }); - // Call with no args - both come from session defaults - const result = await echoHandler({}); + const result = await invokeAndCollect(echoHandler, {}); expect(result.isError).toBe(false); - const content = result.content[0] as { type: 'text'; text: string }; - const parsed = JSON.parse(content.text); - // simulatorId should be kept (first in pair), simulatorName should be pruned + const parsed = JSON.parse(result.text.replace(/\n/g, '').replace(/^.*?(\{.*\}).*$/, '$1')); expect(parsed.simulatorId).toBe('SIM-123'); expect(parsed.simulatorName).toBeUndefined(); }); @@ -268,10 +295,10 @@ describe('createSessionAwareTool', () => { const envHandler = createSessionAwareTool>({ internalSchema: envSchema, - logicFunction: async (params) => ({ - content: [{ type: 'text', text: JSON.stringify(params.env) }], - isError: false, - }), + logicFunction: async (params) => { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', JSON.stringify(params.env))); + }, getExecutor: () => createMockExecutor({ success: true }), requirements: [{ allOf: ['scheme'] }], }); @@ -282,12 +309,10 @@ describe('createSessionAwareTool', () => { env: { API_KEY: 'abc123', VERBOSE: '1' }, }); - // User provides additional env var; session default env vars should be preserved - const result = await envHandler({ env: { DEBUG: 'true', VERBOSE: '0' } }); + const result = await invokeAndCollect(envHandler, { env: { DEBUG: 'true', VERBOSE: '0' } }); expect(result.isError).toBe(false); - const envContent = result.content[0] as { type: 'text'; text: string }; - const parsed = JSON.parse(envContent.text); + const parsed = JSON.parse(result.text.replace(/\n/g, '').replace(/^.*?(\{.*\}).*$/, '$1')); expect(parsed).toEqual({ API_KEY: 'abc123', DEBUG: 'true', VERBOSE: '0' }); }); @@ -300,10 +325,10 @@ describe('createSessionAwareTool', () => { const envHandler = createSessionAwareTool>({ internalSchema: envSchema, - logicFunction: async (params) => ({ - content: [{ type: 'text', text: JSON.stringify(params.env) }], - isError: false, - }), + logicFunction: async (params) => { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', JSON.stringify(params.env))); + }, getExecutor: () => createMockExecutor({ success: true }), requirements: [{ allOf: ['scheme'] }], }); @@ -314,9 +339,8 @@ describe('createSessionAwareTool', () => { env: { API_KEY: 'abc123' }, }); - // Array should not be deep-merged; Zod validation should reject it - const result = await envHandler({ env: ['not', 'a', 'record'] as unknown }); + const result = await invokeAndCollect(envHandler, { env: ['not', 'a', 'record'] as unknown }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); + expect(result.text).toContain('Parameter validation failed'); }); }); diff --git a/src/utils/__tests__/typed-tool-factory-consolidation.test.ts b/src/utils/__tests__/typed-tool-factory-consolidation.test.ts deleted file mode 100644 index 4d8cb9f0..00000000 --- a/src/utils/__tests__/typed-tool-factory-consolidation.test.ts +++ /dev/null @@ -1,113 +0,0 @@ -/** - * Wiring tests: tool factory handlers must consolidate multi-block output under Claude Code. - * - * These tests mock the environment detector so that isRunningUnderClaudeCode() returns true, - * then verify the factory-produced handlers consolidate multi-block responses into a single - * text block. This is the centralised location for consolidation — individual logic functions - * no longer call consolidateContentForClaudeCode themselves. - */ -import { describe, it, expect, vi } from 'vitest'; -import * as z from 'zod'; -import { createMockExecutor } from '../../test-utils/mock-executors.ts'; -import type { ToolResponse } from '../../types/common.ts'; -import type { CommandExecutor } from '../command.ts'; - -vi.mock('../environment.ts', async (importOriginal) => { - const actual = (await importOriginal()) as Record; - return { - ...actual, - getDefaultEnvironmentDetector: () => ({ - isRunningUnderClaudeCode: () => true, - }), - }; -}); - -const { createTypedTool, createSessionAwareTool } = await import('../typed-tool-factory.ts'); - -const testSchema = z.object({ - name: z.string(), -}); - -type TestParams = z.infer; - -function multiBlockResponse(): ToolResponse { - return { - content: [ - { type: 'text', text: 'Block 1' }, - { type: 'text', text: 'Block 2' }, - { type: 'text', text: 'Block 3' }, - ], - }; -} - -function singleBlockResponse(): ToolResponse { - return { - content: [{ type: 'text', text: 'Only block' }], - }; -} - -describe('createTypedTool — Claude Code consolidation wiring', () => { - it('should consolidate multi-block response into a single text block', async () => { - const handler = createTypedTool( - testSchema, - async (_params: TestParams, _executor: CommandExecutor) => multiBlockResponse(), - () => createMockExecutor({ success: true, output: '' }), - ); - - const result = await handler({ name: 'test' }); - - expect(result.content).toHaveLength(1); - expect(result.content[0].type).toBe('text'); - const text = (result.content[0] as { type: 'text'; text: string }).text; - expect(text).toContain('Block 1'); - expect(text).toContain('Block 2'); - expect(text).toContain('Block 3'); - }); - - it('should leave single-block response unchanged', async () => { - const handler = createTypedTool( - testSchema, - async (_params: TestParams, _executor: CommandExecutor) => singleBlockResponse(), - () => createMockExecutor({ success: true, output: '' }), - ); - - const result = await handler({ name: 'test' }); - - expect(result.content).toHaveLength(1); - expect((result.content[0] as { type: 'text'; text: string }).text).toBe('Only block'); - }); -}); - -describe('createSessionAwareTool — Claude Code consolidation wiring', () => { - it('should consolidate multi-block response into a single text block', async () => { - const handler = createSessionAwareTool({ - internalSchema: testSchema, - logicFunction: async (_params: TestParams, _executor: CommandExecutor) => - multiBlockResponse(), - getExecutor: () => createMockExecutor({ success: true, output: '' }), - }); - - const result = await handler({ name: 'test' }); - - expect(result.content).toHaveLength(1); - expect(result.content[0].type).toBe('text'); - const text = (result.content[0] as { type: 'text'; text: string }).text; - expect(text).toContain('Block 1'); - expect(text).toContain('Block 2'); - expect(text).toContain('Block 3'); - }); - - it('should leave single-block response unchanged', async () => { - const handler = createSessionAwareTool({ - internalSchema: testSchema, - logicFunction: async (_params: TestParams, _executor: CommandExecutor) => - singleBlockResponse(), - getExecutor: () => createMockExecutor({ success: true, output: '' }), - }); - - const result = await handler({ name: 'test' }); - - expect(result.content).toHaveLength(1); - expect((result.content[0] as { type: 'text'; text: string }).text).toBe('Only block'); - }); -}); diff --git a/src/utils/__tests__/typed-tool-factory.test.ts b/src/utils/__tests__/typed-tool-factory.test.ts index f3930886..aa41c54a 100644 --- a/src/utils/__tests__/typed-tool-factory.test.ts +++ b/src/utils/__tests__/typed-tool-factory.test.ts @@ -5,10 +5,13 @@ import { describe, it, expect } from 'vitest'; import * as z from 'zod'; import { createTypedTool } from '../typed-tool-factory.ts'; +import type { ToolHandler } from '../typed-tool-factory.ts'; import { createMockExecutor } from '../../test-utils/mock-executors.ts'; -import type { ToolResponse } from '../../types/common.ts'; +import { createRenderSession } from '../../rendering/render.ts'; +import type { ToolHandlerContext } from '../../rendering/types.ts'; +import { getHandlerContext } from '../typed-tool-factory.ts'; +import { statusLine } from '../tool-event-builders.ts'; -// Test schema and types const testSchema = z.object({ requiredParam: z.string().describe('A required string parameter'), optionalParam: z.number().optional().describe('An optional number parameter'), @@ -16,12 +19,24 @@ const testSchema = z.object({ type TestParams = z.infer; -// Mock logic function for testing -async function testLogic(params: TestParams): Promise { - return { - content: [{ type: 'text', text: `Logic executed with: ${params.requiredParam}` }], - isError: false, +async function testLogic(params: TestParams): Promise { + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', `Logic executed with: ${params.requiredParam}`)); +} + +function invokeAndCollect( + handler: ToolHandler, + args: Record, +): Promise<{ text: string; isError: boolean }> { + const session = createRenderSession('text'); + const ctx: ToolHandlerContext = { + emit: (event) => session.emit(event), + attach: (image) => session.attach(image), }; + return handler(args, ctx).then(() => ({ + text: session.finalize(), + isError: session.isError(), + })); } describe('createTypedTool', () => { @@ -30,70 +45,67 @@ describe('createTypedTool', () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); const handler = createTypedTool(testSchema, testLogic, () => mockExecutor); - const result = await handler({ + const result = await invokeAndCollect(handler, { requiredParam: 'valid-value', optionalParam: 42, }); expect(result.isError).toBe(false); - expect(result.content[0].text).toContain('Logic executed with: valid-value'); + expect(result.text).toContain('Logic executed with: valid-value'); }); it('should reject parameters with missing required fields', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); const handler = createTypedTool(testSchema, testLogic, () => mockExecutor); - const result = await handler({ - // Missing requiredParam + const result = await invokeAndCollect(handler, { optionalParam: 42, }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); - expect(result.content[0].text).toContain('requiredParam'); + expect(result.text).toContain('Parameter validation failed'); + expect(result.text).toContain('requiredParam'); }); it('should reject parameters with wrong types', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); const handler = createTypedTool(testSchema, testLogic, () => mockExecutor); - const result = await handler({ - requiredParam: 123, // Should be string, not number + const result = await invokeAndCollect(handler, { + requiredParam: 123, optionalParam: 42, }); expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Parameter validation failed'); - expect(result.content[0].text).toContain('requiredParam'); + expect(result.text).toContain('Parameter validation failed'); + expect(result.text).toContain('requiredParam'); }); it('should accept parameters with only required fields', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); const handler = createTypedTool(testSchema, testLogic, () => mockExecutor); - const result = await handler({ + const result = await invokeAndCollect(handler, { requiredParam: 'valid-value', - // optionalParam omitted }); expect(result.isError).toBe(false); - expect(result.content[0].text).toContain('Logic executed with: valid-value'); + expect(result.text).toContain('Logic executed with: valid-value'); }); it('should provide detailed validation error messages', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); const handler = createTypedTool(testSchema, testLogic, () => mockExecutor); - const result = await handler({ - requiredParam: 123, // Wrong type - optionalParam: 'should-be-number', // Wrong type + const result = await invokeAndCollect(handler, { + requiredParam: 123, + optionalParam: 'should-be-number', }); expect(result.isError).toBe(true); - const errorText = result.content[0].text; - expect(errorText).toContain('Parameter validation failed'); - expect(errorText).toContain('requiredParam'); - expect(errorText).toContain('optionalParam'); + expect(result.text).toContain('Parameter validation failed'); + expect(result.text).toContain('requiredParam'); + expect(result.text).toContain('optionalParam'); }); }); @@ -101,14 +113,15 @@ describe('createTypedTool', () => { it('should re-throw non-Zod errors from logic function', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); - // Logic function that throws a non-Zod error - async function errorLogic(): Promise { + async function errorLogic(): Promise { throw new Error('Unexpected error'); } const handler = createTypedTool(testSchema, errorLogic, () => mockExecutor); - await expect(handler({ requiredParam: 'valid' })).rejects.toThrow('Unexpected error'); + await expect(invokeAndCollect(handler, { requiredParam: 'valid' })).rejects.toThrow( + 'Unexpected error', + ); }); }); @@ -116,21 +129,18 @@ describe('createTypedTool', () => { it('should pass the provided executor to logic function', async () => { const mockExecutor = createMockExecutor({ success: true, output: 'test' }); - async function executorTestLogic(params: TestParams, executor: any): Promise { - // Verify executor is passed correctly + async function executorTestLogic(_params: TestParams, executor: unknown): Promise { expect(executor).toBe(mockExecutor); - return { - content: [{ type: 'text', text: 'Executor passed correctly' }], - isError: false, - }; + const ctx = getHandlerContext(); + ctx.emit(statusLine('success', 'Executor passed correctly')); } const handler = createTypedTool(testSchema, executorTestLogic, () => mockExecutor); - const result = await handler({ requiredParam: 'valid' }); + const result = await invokeAndCollect(handler, { requiredParam: 'valid' }); expect(result.isError).toBe(false); - expect(result.content[0].text).toBe('Executor passed correctly'); + expect(result.text).toContain('Executor passed correctly'); }); }); }); diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 70997b59..fa97a25f 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -1,45 +1,11 @@ -import type { ToolResponse } from '../types/common.ts'; - -/** - * Error Utilities - Type-safe error hierarchy for the application - * - * This utility module defines a structured error hierarchy for the application, - * providing specialized error types for different failure scenarios. Using these - * typed errors enables more precise error handling, improves debugging, and - * provides better error messages to users. - * - * Responsibilities: - * - Providing a base error class (XcodeBuildMCPError) for all application errors - * - Defining specialized error subtypes for different error categories: - * - ValidationError: Parameter validation failures - * - SystemError: Underlying system/OS issues - * - ConfigurationError: Application configuration problems - * - SimulatorError: iOS simulator-specific failures - * - AxeError: axe-specific errors - * - * The structured hierarchy allows error consumers to handle errors with the - * appropriate level of specificity using instanceof checks or catch clauses. - */ - -/** - * Custom error types for XcodeBuildMCP - */ - -/** - * Base error class for XcodeBuildMCP errors - */ export class XcodeBuildMCPError extends Error { constructor(message: string) { super(message); this.name = 'XcodeBuildMCPError'; - // This is necessary for proper inheritance in TypeScript Object.setPrototypeOf(this, XcodeBuildMCPError.prototype); } } -/** - * Error thrown when validation of parameters fails - */ export class ValidationError extends XcodeBuildMCPError { constructor( message: string, @@ -51,9 +17,6 @@ export class ValidationError extends XcodeBuildMCPError { } } -/** - * Error thrown for system-level errors (file access, permissions, etc.) - */ export class SystemError extends XcodeBuildMCPError { constructor( message: string, @@ -65,9 +28,6 @@ export class SystemError extends XcodeBuildMCPError { } } -/** - * Error thrown for configuration issues - */ export class ConfigurationError extends XcodeBuildMCPError { constructor(message: string) { super(message); @@ -76,9 +36,6 @@ export class ConfigurationError extends XcodeBuildMCPError { } } -/** - * Error thrown for simulator-specific errors - */ export class SimulatorError extends XcodeBuildMCPError { constructor( message: string, @@ -91,14 +48,11 @@ export class SimulatorError extends XcodeBuildMCPError { } } -/** - * Error thrown for axe-specific errors - */ export class AxeError extends XcodeBuildMCPError { constructor( message: string, - public command?: string, // The axe command that failed - public axeOutput?: string, // Output from axe + public command?: string, + public axeOutput?: string, public simulatorId?: string, ) { super(message); @@ -107,23 +61,6 @@ export class AxeError extends XcodeBuildMCPError { } } -// Helper to create a standard error response -export function createErrorResponse(message: string, details?: string): ToolResponse { - const detailText = details ? `\nDetails: ${details}` : ''; - return { - content: [ - { - type: 'text', - text: `Error: ${message}${detailText}`, - }, - ], - isError: true, - }; -} - -/** - * Error class for missing dependencies - */ export class DependencyError extends ConfigurationError { constructor( message: string, @@ -134,3 +71,10 @@ export class DependencyError extends ConfigurationError { Object.setPrototypeOf(this, DependencyError.prototype); } } + +/** + * Normalize an unknown thrown value to a string message. + */ +export function toErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/utils/responses/index.ts b/src/utils/responses/index.ts index c1c521a9..44dfc4d7 100644 --- a/src/utils/responses/index.ts +++ b/src/utils/responses/index.ts @@ -1,6 +1,24 @@ -export { createTextResponse } from '../validation.ts'; +import type { ToolResponse, NextStep, OutputStyle } from '../../types/common.ts'; + +// Shim: createErrorResponse was removed in the handler-contract refactor but +// ~35 consumer files still import it. They will be migrated in PRs 6-9. +export function createErrorResponse(message: string, details?: string): ToolResponse { + const detailText = details ? `\nDetails: ${details}` : ''; + return { + content: [{ type: 'text', text: `Error: ${message}${detailText}` }], + isError: true, + }; +} + +// Shim: createTextResponse was removed from validation.ts +export function createTextResponse(message: string, isError = false): ToolResponse { + return { + content: [{ type: 'text', text: message }], + ...(isError ? { isError: true } : {}), + }; +} + export { - createErrorResponse, DependencyError, AxeError, SystemError, @@ -12,4 +30,4 @@ export { renderNextStepsSection, } from './next-steps-renderer.ts'; -export type { ToolResponse, NextStep, OutputStyle } from '../../types/common.ts'; +export type { ToolResponse, NextStep, OutputStyle }; diff --git a/src/utils/schema-helpers.ts b/src/utils/schema-helpers.ts index 3d43b9d2..df073be7 100644 --- a/src/utils/schema-helpers.ts +++ b/src/utils/schema-helpers.ts @@ -1,16 +1,3 @@ -/** - * Schema Helper Utilities - * - * Shared utility functions for schema validation and preprocessing. - */ - -/** - * Convert empty strings to undefined in an object (shallow transformation) - * Used for preprocessing Zod schemas with optional fields - * - * @param value - The value to process - * @returns The processed value with empty strings converted to undefined - */ export function nullifyEmptyStrings(value: unknown): unknown { if (value && typeof value === 'object' && !Array.isArray(value)) { const copy: Record = { ...(value as Record) }; diff --git a/src/utils/typed-tool-factory.ts b/src/utils/typed-tool-factory.ts index bb11d106..59156522 100644 --- a/src/utils/typed-tool-factory.ts +++ b/src/utils/typed-tool-factory.ts @@ -1,68 +1,131 @@ -/** - * Type-safe tool factory for XcodeBuildMCP - * - * This module provides a factory function to create MCP tool handlers that safely - * convert from the generic Record signature required by the MCP SDK - * to strongly-typed parameters using runtime validation with Zod. - * - * This eliminates the need for unsafe type assertions while maintaining full - * compatibility with the MCP SDK's tool handler signature requirements. - */ - +import { AsyncLocalStorage } from 'node:async_hooks'; import * as z from 'zod'; -import type { ToolResponse } from '../types/common.ts'; +import type { ToolHandlerContext } from '../rendering/types.ts'; +import { createRenderSession } from '../rendering/render.ts'; import type { CommandExecutor } from './execution/index.ts'; -import { createErrorResponse } from './responses/index.ts'; -import { consolidateContentForClaudeCode } from './validation.ts'; +import { statusLine } from './tool-event-builders.ts'; +import type { PipelineEvent } from '../types/pipeline-events.ts'; + import { sessionStore, type SessionDefaults } from './session-store.ts'; import { isSessionDefaultsOptOutEnabled } from './environment.ts'; import { mergeSessionDefaultArgs } from './session-default-args.ts'; +/** + * Result returned by tool handlers when invoked without a ToolHandlerContext + * (i.e. in test mode). Provides a ToolResponse-compatible shape. + */ +export interface ToolTestResult { + content: Array<{ type: 'text'; text: string }>; + isError?: boolean; + _meta?: { events: PipelineEvent[] }; +} + +/** + * Overloaded handler type for tools. + * - With ToolHandlerContext: returns void (production / MCP path) + * - Without context: returns ToolTestResult (test path) + */ +export interface ToolHandler { + (args: Record, ctx: ToolHandlerContext): Promise; + (args: Record): Promise; +} + +export const handlerContextStorage = new AsyncLocalStorage(); + +export function getHandlerContext(): ToolHandlerContext { + const ctx = handlerContextStorage.getStore(); + if (!ctx) { + throw new Error('getHandlerContext() called outside of a tool handler invocation'); + } + return ctx; +} + +function isToolHandlerContext(value: unknown): value is ToolHandlerContext { + return ( + typeof value === 'object' && + value !== null && + 'emit' in value && + typeof value.emit === 'function' && + 'attach' in value && + typeof value.attach === 'function' + ); +} + +function sessionToTestResult(session: ReturnType): ToolTestResult { + const text = session.finalize(); + const events = [...session.getEvents()]; + + const content: Array<{ type: 'text'; text: string }> = []; + if (text) { + content.push({ type: 'text' as const, text }); + } + + return { + content, + isError: session.isError() || undefined, + ...(events.length > 0 ? { _meta: { events } } : {}), + }; +} + function createValidatedHandler( schema: z.ZodType, - logicFunction: (params: TParams, context: TContext) => Promise, + logicFunction: (params: TParams, context: TContext) => Promise, getContext: () => TContext, -): (args: Record) => Promise { - return async (args: Record): Promise => { +): ToolHandler { + const impl = async ( + args: Record, + providedContext?: TContext | ToolHandlerContext, + ): Promise => { + const hasProvidedHandlerContext = isToolHandlerContext(providedContext); + const session = hasProvidedHandlerContext ? null : createRenderSession('text'); + const ctx: ToolHandlerContext = hasProvidedHandlerContext + ? providedContext + : { + emit: (event) => { + session!.emit(event); + }, + attach: (image) => { + session!.attach(image); + }, + }; + const context = + providedContext !== undefined && !hasProvidedHandlerContext ? providedContext : getContext(); + try { const validatedParams = schema.parse(args); - - const response = await logicFunction(validatedParams, getContext()); - return consolidateContentForClaudeCode(response); + await handlerContextStorage.run(ctx, () => logicFunction(validatedParams, context)); + if (!hasProvidedHandlerContext) { + return sessionToTestResult(session!); + } } catch (error) { if (error instanceof z.ZodError) { const details = `Invalid parameters:\n${formatZodIssues(error)}`; - return createErrorResponse('Parameter validation failed', details); + ctx.emit(statusLine('error', `Parameter validation failed: ${details}`)); + if (!hasProvidedHandlerContext) { + return sessionToTestResult(session!); + } + return; } - // Re-throw unexpected errors (they'll be caught by the MCP framework) throw error; } }; + return impl as ToolHandler; } -/** - * Creates a type-safe tool handler that validates parameters at runtime - * before passing them to the typed logic function. - * - * @param schema - Zod schema for parameter validation - * @param logicFunction - The typed logic function to execute - * @param getExecutor - Function to get the command executor (must be provided) - * @returns A handler function compatible with MCP SDK requirements - */ export function createTypedTool( schema: z.ZodType, - logicFunction: (params: TParams, executor: CommandExecutor) => Promise, + logicFunction: (params: TParams, executor: CommandExecutor) => Promise, getExecutor: () => CommandExecutor, -): (args: Record) => Promise { +): ToolHandler { return createValidatedHandler(schema, logicFunction, getExecutor); } export function createTypedToolWithContext( schema: z.ZodType, - logicFunction: (params: TParams, context: TContext) => Promise, + logicFunction: (params: TParams, context: TContext) => Promise, getContext: () => TContext, -): (args: Record) => Promise { +): ToolHandler { return createValidatedHandler(schema, logicFunction, getContext); } @@ -102,11 +165,11 @@ export function getSessionAwareToolSchemaShape(opts: { export function createSessionAwareTool(opts: { internalSchema: z.ZodType; - logicFunction: (params: TParams, executor: CommandExecutor) => Promise; + logicFunction: (params: TParams, executor: CommandExecutor) => Promise; getExecutor: () => CommandExecutor; requirements?: SessionRequirement[]; - exclusivePairs?: (keyof SessionDefaults)[][]; // when args provide one side, drop conflicting session-default side(s) -}): (rawArgs: Record) => Promise { + exclusivePairs?: (keyof SessionDefaults)[][]; +}): ToolHandler { return createSessionAwareHandler({ internalSchema: opts.internalSchema, logicFunction: opts.logicFunction, @@ -118,21 +181,21 @@ export function createSessionAwareTool(opts: { export function createSessionAwareToolWithContext(opts: { internalSchema: z.ZodType; - logicFunction: (params: TParams, context: TContext) => Promise; + logicFunction: (params: TParams, context: TContext) => Promise; getContext: () => TContext; requirements?: SessionRequirement[]; exclusivePairs?: (keyof SessionDefaults)[][]; -}): (rawArgs: Record) => Promise { +}): ToolHandler { return createSessionAwareHandler(opts); } function createSessionAwareHandler(opts: { internalSchema: z.ZodType; - logicFunction: (params: TParams, context: TContext) => Promise; + logicFunction: (params: TParams, context: TContext) => Promise; getContext: () => TContext; requirements?: SessionRequirement[]; exclusivePairs?: (keyof SessionDefaults)[][]; -}): (rawArgs: Record) => Promise { +}): ToolHandler { const { internalSchema, logicFunction, @@ -141,9 +204,32 @@ function createSessionAwareHandler(opts: { exclusivePairs = [], } = opts; - return async (rawArgs: Record): Promise => { + const impl = async ( + rawArgs: Record, + providedContext?: TContext | ToolHandlerContext, + ): Promise => { + const hasProvidedHandlerContext = isToolHandlerContext(providedContext); + const session = hasProvidedHandlerContext ? null : createRenderSession('text'); + const ctx: ToolHandlerContext = hasProvidedHandlerContext + ? providedContext + : { + emit: (event) => { + session!.emit(event); + }, + attach: (image) => { + session!.attach(image); + }, + }; + const context = + providedContext !== undefined && !hasProvidedHandlerContext ? providedContext : getContext(); + + const finalize = (): ToolTestResult | void => { + if (!hasProvidedHandlerContext) { + return sessionToTestResult(session!); + } + }; + try { - // 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) continue; @@ -151,17 +237,16 @@ function createSessionAwareHandler(opts: { sanitizedArgs[k] = v; } - // Factory-level mutual exclusivity check: if user provides multiple explicit values - // within an exclusive group, reject early even if tool schema doesn't enforce XOR. for (const pair of exclusivePairs) { const provided = pair.filter((k) => Object.prototype.hasOwnProperty.call(sanitizedArgs, k)); if (provided.length >= 2) { - return createErrorResponse( - 'Parameter validation failed', - `Invalid parameters:\nMutually exclusive parameters provided: ${provided.join( - ', ', - )}. Provide only one.`, + ctx.emit( + statusLine( + 'error', + `Parameter validation failed: Invalid parameters:\nMutually exclusive parameters provided: ${provided.join(', ')}. Provide only one.`, + ), ); + return finalize(); } } @@ -172,7 +257,6 @@ function createSessionAwareHandler(opts: { exclusivePairs, }); - // Check requirements first (before expensive simulator resolution) for (const req of requirements) { if ('allOf' in req) { const missing = missingFromMerged(req.allOf, merged); @@ -185,7 +269,8 @@ function createSessionAwareHandler(opts: { setHint, optOutEnabled: isSessionDefaultsOptOutEnabled(), }); - return createErrorResponse(title, body); + ctx.emit(statusLine('error', `${title}: ${body}`)); + return finalize(); } } else if ('oneOf' in req) { const satisfied = req.oneOf.some((k) => merged[k] != null); @@ -199,22 +284,25 @@ function createSessionAwareHandler(opts: { setHint: `Set with: ${setHints}`, optOutEnabled: isSessionDefaultsOptOutEnabled(), }); - return createErrorResponse(title, body); + ctx.emit(statusLine('error', `${title}: ${body}`)); + return finalize(); } } } const validated = internalSchema.parse(merged); - const response = await logicFunction(validated, getContext()); - return consolidateContentForClaudeCode(response); + await handlerContextStorage.run(ctx, () => logicFunction(validated, context)); + return finalize(); } catch (error) { if (error instanceof z.ZodError) { const details = `Invalid parameters:\n${formatZodIssues(error)}`; - return createErrorResponse('Parameter validation failed', details); + ctx.emit(statusLine('error', `Parameter validation failed: ${details}`)); + return finalize(); } throw error; } }; + return impl as ToolHandler; } function formatZodIssues(error: z.ZodError): string { diff --git a/src/utils/validation.ts b/src/utils/validation.ts index 9d40b9fd..87f1dfb7 100644 --- a/src/utils/validation.ts +++ b/src/utils/validation.ts @@ -1,134 +1,7 @@ -/** - * Validation Utilities - Input validation and error response generation - * - * This utility module provides a comprehensive set of validation functions to ensure - * that tool inputs meet expected requirements. It centralizes validation logic, - * error message formatting, and response generation for consistent error handling - * across the application. - * - * Responsibilities: - * - Validating required parameters (validateRequiredParam) - * - Checking parameters against allowed values (validateAllowedValues, validateEnumParam) - * - Verifying file existence (validateFileExists) - * - Validating logical conditions (validateCondition) - * - Ensuring at least one of multiple parameters is provided (validateAtLeastOneParam) - * - Creating standardized response objects for tools (createTextResponse) - * - * Using these validation utilities ensures consistent error messaging and helps - * provide clear feedback to users when their inputs don't meet requirements. - * The functions return ValidationResult objects that make it easy to chain - * validations and generate appropriate responses. - */ - import * as fs from 'fs'; -import { log } from './logger.ts'; -import type { ToolResponse, ValidationResult } from '../types/common.ts'; +import type { ValidationResult } from '../types/common.ts'; import type { FileSystemExecutor } from './FileSystemExecutor.ts'; -import { getDefaultEnvironmentDetector } from './environment.ts'; -import type { EnvironmentDetector } from './environment.ts'; - -/** - * Creates a text response with the given message - * @param message The message to include in the response - * @param isError Whether this is an error response - * @returns A ToolResponse object with the message - */ -export function createTextResponse(message: string, isError = false): ToolResponse { - return { - content: [ - { - type: 'text', - text: message, - }, - ], - isError, - }; -} - -/** - * Validates that a required parameter is present - * @param paramName Name of the parameter - * @param paramValue Value of the parameter - * @param helpfulMessage Optional helpful message to include in the error response - * @returns Validation result - */ -export function validateRequiredParam( - paramName: string, - paramValue: unknown, - helpfulMessage = `Required parameter '${paramName}' is missing. Please provide a value for this parameter.`, -): ValidationResult { - if (paramValue === undefined || paramValue === null) { - log('warn', `Required parameter '${paramName}' is missing`); - return { - isValid: false, - errorResponse: createTextResponse(helpfulMessage, true), - }; - } - - return { isValid: true }; -} - -/** - * Validates that a parameter value is one of the allowed values - * @param paramName Name of the parameter - * @param paramValue Value of the parameter - * @param allowedValues Array of allowed values - * @returns Validation result - */ -export function validateAllowedValues( - paramName: string, - paramValue: T, - allowedValues: T[], -): ValidationResult { - if (!allowedValues.includes(paramValue)) { - log( - 'warn', - `Parameter '${paramName}' has invalid value '${paramValue}'. Allowed values: ${allowedValues.join( - ', ', - )}`, - ); - return { - isValid: false, - errorResponse: createTextResponse( - `Parameter '${paramName}' must be one of: ${allowedValues.join(', ')}. You provided: '${paramValue}'.`, - true, - ), - }; - } - - return { isValid: true }; -} - -/** - * Validates that a condition is true - * @param condition Condition to validate - * @param message Message to include in the warning response - * @param logWarning Whether to log a warning message - * @returns Validation result - */ -export function validateCondition( - condition: boolean, - message: string, - logWarning: boolean = true, -): ValidationResult { - if (!condition) { - if (logWarning) { - log('warn', message); - } - return { - isValid: false, - warningResponse: createTextResponse(message), - }; - } - - return { isValid: true }; -} -/** - * Validates that a file exists - * @param filePath Path to check - * @returns Validation result - */ export function validateFileExists( filePath: string, fileSystem?: FileSystemExecutor, @@ -137,132 +10,9 @@ export function validateFileExists( if (!exists) { return { isValid: false, - errorResponse: createTextResponse( - `File not found: '${filePath}'. Please check the path and try again.`, - true, - ), + errorMessage: `File not found: '${filePath}'. Please check the path and try again.`, }; } return { isValid: true }; } - -/** - * Validates that at least one of two parameters is provided - * @param param1Name Name of the first parameter - * @param param1Value Value of the first parameter - * @param param2Name Name of the second parameter - * @param param2Value Value of the second parameter - * @returns Validation result - */ -export function validateAtLeastOneParam( - param1Name: string, - param1Value: unknown, - param2Name: string, - param2Value: unknown, -): ValidationResult { - if ( - (param1Value === undefined || param1Value === null) && - (param2Value === undefined || param2Value === null) - ) { - log('warn', `At least one of '${param1Name}' or '${param2Name}' must be provided`); - return { - isValid: false, - errorResponse: createTextResponse( - `At least one of '${param1Name}' or '${param2Name}' must be provided.`, - true, - ), - }; - } - - return { isValid: true }; -} - -/** - * Validates that a parameter value is one of the allowed enum values - * @param paramName Name of the parameter - * @param paramValue Value of the parameter - * @param allowedValues Array of allowed enum values - * @returns Validation result - */ -export function validateEnumParam( - paramName: string, - paramValue: T, - allowedValues: T[], -): ValidationResult { - if (!allowedValues.includes(paramValue)) { - log( - 'warn', - `Parameter '${paramName}' has invalid value '${paramValue}'. Allowed values: ${allowedValues.join( - ', ', - )}`, - ); - return { - isValid: false, - errorResponse: createTextResponse( - `Parameter '${paramName}' must be one of: ${allowedValues.join(', ')}. You provided: '${paramValue}'.`, - true, - ), - }; - } - - return { isValid: true }; -} - -/** - * Consolidates multiple content blocks into a single text response for Claude Code compatibility - * - * Claude Code violates the MCP specification by only showing the first content block. - * This function provides a workaround by concatenating all text content into a single block. - * Detection is automatic - no environment variable configuration required. - * - * @param response The original ToolResponse with multiple content blocks - * @returns A new ToolResponse with consolidated content - */ -export function consolidateContentForClaudeCode( - response: ToolResponse, - detector?: EnvironmentDetector, -): ToolResponse { - // Automatically detect if running under Claude Code - const shouldConsolidate = ( - detector ?? getDefaultEnvironmentDetector() - ).isRunningUnderClaudeCode(); - - if (!shouldConsolidate || !response.content || response.content.length <= 1) { - return response; - } - - // Extract all text content and concatenate with separators - const textParts: string[] = []; - - response.content.forEach((item, index) => { - if (item.type === 'text') { - // Add a separator between content blocks (except for the first one) - if (index > 0 && textParts.length > 0) { - textParts.push('\n---\n'); - } - textParts.push(item.text); - } - // Note: Image content is not handled in this workaround as it requires special formatting - }); - - // If no text content was found, return the original response to preserve non-text content - if (textParts.length === 0) { - return response; - } - - const consolidatedText = textParts.join(''); - - return { - ...response, - content: [ - { - type: 'text', - text: consolidatedText, - }, - ], - }; -} - -// Export the ToolResponse type for use in other files -export type { ToolResponse, ValidationResult };