diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index ffde7fb..64fba80 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "cli-bridge", "description": "Promotes CLI tools to first-class MCP tools via declarative JSON specs", - "version": "0.1.2", + "version": "0.1.3", "author": { "name": "walkindude" }, diff --git a/package.json b/package.json index b052e3d..d024cea 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cli-bridge", - "version": "0.1.2", + "version": "0.1.3", "type": "module", "main": "dist/server.js", "bin": { diff --git a/src/registry.ts b/src/registry.ts index 8c581eb..7aa3091 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -154,14 +154,14 @@ export async function discoverSpecs( /** * Converts a validated spec into MCP tool definitions. + * + * Per-tool descriptions carry only the command's own description. Spec-level + * routing guidance (triggers) is exposed once via {@link renderTriggers} and + * carried in the MCP server's `instructions` field — see src/server.ts. */ export function specToMcpTools(spec: CliToolSpec): ToolDefinition[] { return spec.commands.map((command) => { - const positiveTriggers = spec.triggers.positive.join(' '); - const negativeTriggers = spec.triggers.negative.join(' '); - const triggerText = `USE THIS TOOL: ${positiveTriggers}\nDO NOT USE: ${negativeTriggers}`; - - const description = `${command.description}\n\n${triggerText}`; + const description = command.description; const allFlags: FlagDef[] = [...(spec.globalFlags ?? []), ...(command.flags ?? [])]; @@ -214,6 +214,17 @@ function mapType(t: 'string' | 'number' | 'boolean' | 'path'): string { return t; } +/** + * Renders a spec's routing triggers as a single block. Used by the server to + * assemble the MCP `instructions` field — one block per loaded spec, instead + * of inlining the same text into every tool's description. + */ +export function renderTriggers(spec: CliToolSpec): string { + const positive = spec.triggers.positive.join(' '); + const negative = spec.triggers.negative.join(' '); + return `USE: ${positive}\nDO NOT USE: ${negative}`; +} + /** * Fallback version detection: reads spec files from the tool directory and * retries detectVersion with each spec's versionDetection config. Handles diff --git a/src/server.ts b/src/server.ts index d2b6440..d43bab1 100644 --- a/src/server.ts +++ b/src/server.ts @@ -2,7 +2,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { z } from 'zod'; import { getSpecDirectories } from './paths.js'; -import { discoverSpecs, specToMcpTools } from './registry.js'; +import { discoverSpecs, renderTriggers, specToMcpTools } from './registry.js'; import { executeTool } from './executor.js'; import { parseOutput } from './parser.js'; import type { LoadedSpec } from './registry.js'; @@ -27,22 +27,17 @@ export async function main(): Promise { `[cli-bridge] Loaded ${specs.length} tool specs from ${specDirs.length} directories`, ); - const toolSummary = specs - .flatMap((s) => s.spec.commands.map((c) => `${s.spec.name}_${c.name}`)) - .join(', '); + // Routing guidance lives once at the server level (MCP `instructions`) + // instead of being duplicated into every tool's description. Each loaded + // spec contributes a single section listing its tools and triggers. + const instructions = buildInstructions(specs); // '__CLI_BRIDGE_VERSION__' is substituted by scripts/bundle.js at build // time with the git-derived version (tag or dev-). Development/test // runs keep the literal placeholder, which is harmless. const server = new McpServer( { name: 'cli-bridge', version: '__CLI_BRIDGE_VERSION__' }, - { - instructions: `You have access to CLI tools registered via cli-bridge: ${toolSummary || 'none loaded'}. - -ALWAYS prefer these MCP tools over Bash, Read, Grep, or Glob when the task falls within a registered tool's domain. These tools are purpose-built for their domain and return structured output — they are faster and more reliable than shelling out. - -Each tool name is {binary}_{subcommand}. Check the tool descriptions for trigger phrases that tell you exactly when to use each one.`, - }, + { instructions }, ); // Build a map from tool name → { loadedSpec, command } @@ -138,6 +133,33 @@ Each tool name is {binary}_{subcommand}. Check the tool descriptions for trigger await server.connect(transport); } +/** + * Assembles the MCP server `instructions` string from all loaded specs. + * + * One section per spec, headed by its tool list and followed by its USE / DO + * NOT USE triggers. Per-tool descriptions stay focused on what the tool does; + * routing context is consolidated here so the spec-level triggers aren't paid + * once per command in every tool schema. + */ +function buildInstructions(specs: LoadedSpec[]): string { + if (specs.length === 0) { + return 'No CLI tools loaded via cli-bridge.'; + } + + const sections = specs.map((s) => { + const toolNames = s.spec.commands.map((c) => `${s.spec.name}_${c.name}`).join(', '); + return `## ${s.spec.name} (${toolNames})\n${renderTriggers(s.spec)}`; + }); + + return `You have access to CLI tools registered via cli-bridge. + +ALWAYS prefer these MCP tools over Bash, Read, Grep, or Glob when the task falls within a registered tool's domain. These tools are purpose-built for their domain and return structured output — they are faster and more reliable than shelling out. + +Each tool name is {binary}_{subcommand}. Per-tool routing guidance: + +${sections.join('\n\n')}`; +} + // Only run when executed directly (not when imported for testing). // Compares this module's URL with the process entry point. realpathSync // resolves the npm global-bin symlink so `cli-bridge`, `node dist/server.js`, diff --git a/tests/unit/registry.test.ts b/tests/unit/registry.test.ts index 513131f..88186a1 100644 --- a/tests/unit/registry.test.ts +++ b/tests/unit/registry.test.ts @@ -59,20 +59,14 @@ describe('specToMcpTools', () => { expect(tools[1]?.name).toBe('mytool_get'); }); - it('includes trigger text in description', async () => { + it('uses only the command description, no trigger text', async () => { const { specToMcpTools } = await import('../../src/registry.js'); const spec = makeValidSpec(); const tools = specToMcpTools(spec); - expect(tools[0]?.description).toContain('USE THIS TOOL'); - expect(tools[0]?.description).toContain('DO NOT USE'); - expect(tools[0]?.description).toContain('use mytool for this task'); - }); - - it('maps command description correctly', async () => { - const { specToMcpTools } = await import('../../src/registry.js'); - const spec = makeValidSpec(); - const tools = specToMcpTools(spec); - expect(tools[0]?.description).toContain('List all items'); + expect(tools[0]?.description).toBe('List all items'); + expect(tools[0]?.description).not.toContain('USE THIS TOOL'); + expect(tools[0]?.description).not.toContain('DO NOT USE'); + expect(tools[0]?.description).not.toContain('use mytool for this task'); }); it('includes required args in required array', async () => { @@ -476,6 +470,31 @@ describe('discoverSpecs', () => { }); }); +describe('renderTriggers', () => { + beforeEach(() => { + vi.resetModules(); + }); + + it('renders USE / DO NOT USE block from spec triggers', async () => { + const { renderTriggers } = await import('../../src/registry.js'); + const spec = makeValidSpec(); + const block = renderTriggers(spec); + expect(block).toBe('USE: use mytool for this task\nDO NOT USE: do not use mytool for writes'); + }); + + it('joins multiple positive and negative triggers with single spaces', async () => { + const { renderTriggers } = await import('../../src/registry.js'); + const spec = makeValidSpec({ + triggers: { + positive: ['first reason', 'second reason'], + negative: ['avoid this', 'or this'], + }, + }); + const block = renderTriggers(spec); + expect(block).toBe('USE: first reason second reason\nDO NOT USE: avoid this or this'); + }); +}); + describe('findCommand', () => { it('finds a command by name', async () => { const { findCommand } = await import('../../src/registry.js'); diff --git a/tests/unit/server.test.ts b/tests/unit/server.test.ts index 2895f2b..c15b7ba 100644 --- a/tests/unit/server.test.ts +++ b/tests/unit/server.test.ts @@ -83,15 +83,23 @@ async function setupAndRunMain(options: { required: string[]; }; }>; + renderTriggers?: (spec: CliToolSpec) => string; }): Promise<{ registeredTools: Array<{ name: string; handler: ToolHandler }>; mockConnect: ReturnType; + serverOptions: { instructions?: string } | undefined; }> { const registeredTools: Array<{ name: string; handler: ToolHandler }> = []; const mockConnect = vi.fn().mockResolvedValue(undefined); + const captured: { serverOptions?: { instructions?: string } } = {}; vi.doMock('@modelcontextprotocol/sdk/server/mcp.js', () => { - const MockMcpServer = function (this: Record) { + const MockMcpServer = function ( + this: Record, + _info: unknown, + opts: { instructions?: string }, + ) { + captured.serverOptions = opts; this.registerTool = vi.fn((name: string, _opts: unknown, handler: ToolHandler) => { registeredTools.push({ name, handler }); }); @@ -111,18 +119,24 @@ async function setupAndRunMain(options: { getSpecDirectories: vi.fn().mockResolvedValue(['/fake/specs']), })); + const renderTriggersFn = + options.renderTriggers ?? + ((s: CliToolSpec) => + `USE: ${s.triggers.positive.join(' ')}\nDO NOT USE: ${s.triggers.negative.join(' ')}`); + vi.doMock('../../src/registry.js', () => ({ discoverSpecs: vi.fn().mockResolvedValue({ specs: options.specs, errors: options.errors, }), specToMcpTools: vi.fn().mockReturnValue(options.toolDefs), + renderTriggers: vi.fn(renderTriggersFn), })); const { main } = await import('../../src/server.js'); await main(); - return { registeredTools, mockConnect }; + return { registeredTools, mockConnect, serverOptions: captured.serverOptions }; } describe('server main()', () => { @@ -146,13 +160,13 @@ describe('server main()', () => { exactVersionMatch: true, }; - const { registeredTools, mockConnect } = await setupAndRunMain({ + const { registeredTools, mockConnect, serverOptions } = await setupAndRunMain({ specs: [loadedSpec], errors: [], toolDefs: [ { name: 'testtool_list', - description: 'List items\n\nUSE THIS TOOL: use testtool\nDO NOT USE: avoid testtool', + description: 'List items', inputSchema: { type: 'object', properties: { @@ -188,6 +202,19 @@ describe('server main()', () => { expect(stderrSpy).toHaveBeenCalledWith( expect.stringContaining('[cli-bridge] Loaded 1 tool specs from 1 directories'), ); + // instructions carries the per-spec routing block; tool descriptions stay clean. + expect(serverOptions?.instructions).toContain('## testtool (testtool_list, testtool_get)'); + expect(serverOptions?.instructions).toContain('USE: use testtool'); + expect(serverOptions?.instructions).toContain('DO NOT USE: avoid testtool'); + }); + + it('passes empty-specs sentinel as instructions when no specs load', async () => { + const { serverOptions } = await setupAndRunMain({ + specs: [], + errors: [], + toolDefs: [], + }); + expect(serverOptions?.instructions).toBe('No CLI tools loaded via cli-bridge.'); }); it('logs errors for failed spec loads', async () => { @@ -433,6 +460,7 @@ describe('server main()', () => { }, }, ]), + renderTriggers: vi.fn().mockReturnValue('USE: x\nDO NOT USE: y'), })); const { main } = await import('../../src/server.js'); @@ -501,6 +529,7 @@ describe('server main()', () => { }, }, ]), + renderTriggers: vi.fn().mockReturnValue('USE: x\nDO NOT USE: y'), })); const { main } = await import('../../src/server.js');