diff --git a/docs/architecture.md b/docs/architecture.md index d1d826a..0427512 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -22,7 +22,7 @@ Core (shared computation) | result builders used by MCP, CLI, and operation descriptors |\ | \-> Operation Registry - | typed descriptors, input schemas, MCP adapter, result wrappers + | typed descriptors, input schemas, CLI/MCP adapters, result wrappers v MCP (stdio) CLI (terminal/CI) | 17 tools, 2 prompts, | 18 commands with text + JSON @@ -81,7 +81,7 @@ runOperation(operation, codebaseGraph, input, context) ## Key Design Decisions - **Dual interface**: MCP stdio for LLM agents, CLI subcommands for humans/CI. Both consume `src/core/`. -- **Operation registry foundation**: Analysis operations now have typed descriptors in `src/operations/` with operation names, CLI command names, MCP tool names, input schemas, and discriminated run results. MCP tool registration consumes those descriptors; CLI commands still call `src/core/` directly until the CLI adapter migration lands. +- **Operation registry foundation**: Analysis operations now have typed descriptors in `src/operations/` with operation names, CLI command names, MCP tool names, input schemas, and discriminated run results. MCP tool registration and CLI command execution consume those descriptors; CLI text formatting still lives in `src/cli.ts` until the formatter migration lands. - **graphology**: In-memory graph with O(1) neighbor lookup. PageRank and betweenness computed via graphology-metrics. - **Batch git churn**: Single `git log --all --name-only` call, parsed for all files. Avoids O(n) subprocess spawning. - **Monorepo import resolution**: Root `tsconfig.json` path aliases and local `package.json` package names resolve to source files before graph construction. diff --git a/llms-full.txt b/llms-full.txt index 6f93ffa..51cae9f 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -28,7 +28,7 @@ Core (shared computation) | result builders used by MCP, CLI, and operation descriptors |\ | \-> Operation Registry - | typed descriptors, input schemas, MCP adapter, result wrappers + | typed descriptors, input schemas, CLI/MCP adapters, result wrappers v MCP (stdio) + CLI | MCP: 17 tools, 2 prompts, 3 resources for LLM agents @@ -81,7 +81,7 @@ runOperation(operation, codebaseGraph, input, context) ## Key Design Decisions - **graphology**: In-memory graph with O(1) neighbor lookup. PageRank and betweenness computed via graphology-metrics. -- **Operation registry foundation**: Analysis operations have typed descriptors in `src/operations/` with operation names, CLI command names, MCP tool names, input schemas, and discriminated run results. MCP tool registration consumes those descriptors; CLI commands still call `src/core/` directly until the CLI adapter migration lands. +- **Operation registry foundation**: Analysis operations have typed descriptors in `src/operations/` with operation names, CLI command names, MCP tool names, input schemas, and discriminated run results. MCP tool registration and CLI command execution consume those descriptors; CLI text formatting still lives in `src/cli.ts` until the formatter migration lands. - **Batch git churn**: Single `git log --all --name-only` call, parsed for all files. Avoids O(n) subprocess spawning. - **Dead export detection**: Cross-references parsed exports against edge symbol lists. May miss `import *` or re-exports. - **Graceful degradation**: Non-git dirs get churn=0, no-test codebases get coverage=false. Never crashes. diff --git a/roadmap.md b/roadmap.md index a3da14c..407d680 100644 --- a/roadmap.md +++ b/roadmap.md @@ -270,15 +270,16 @@ Collapse CLI + MCP operation duplication into one descriptor registry before add - Reuse registry descriptors and input shapes in MCP tool registration. - Route MCP operation success/error envelopes through `runOperation(...)` while preserving existing `nextSteps` and `isError` contracts. - Add MCP registry parity coverage for representative operation descriptor runs. +- Reuse registry schemas in CLI coercion. +- Move CLI failures over descriptor-level validation errors. +- Add CLI registry parity coverage for representative descriptor runs and invalid input. **Remaining:** -- Reuse registry schemas in CLI coercion. -- Move CLI failures over descriptor-level validation errors. - Use one graph-load pipeline with progress callbacks. - Move text/SARIF/markdown formatting over result objects into formatters. -- Expand CH-P1-01 from overview/representative MCP operations to every operation. -- Add CH-P1-02 coverage for success, invalid input, parse failure, and cache reuse through registry adapters. +- Expand CH-P1-01 from overview/representative CLI/MCP operations to every operation. +- Expand CH-P1-02 coverage from CLI invalid input to success, parse failure, and cache reuse through every registry adapter. ### Type/Shape Layer diff --git a/src/cli.ts b/src/cli.ts index 6ba1b96..337a1e0 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -32,27 +32,12 @@ import { type IndexDirectoryResolution, } from "./persistence/index-dir.js"; import { - computeOverview, - computeFileContext, - computeHotspots, - HOTSPOT_METRICS, - isHotspotMetric, - computeSearch, - computeChanges, - CHANGE_SCOPES, - isChangeScope, - computeDependents, - computeModuleStructure, - computeForces, - computeDeadExports, - computeOpportunities, - computeGroups, - computeSymbolContext, - computeProcesses, - computeClusters, - impactAnalysis, - renameSymbol, -} from "./core/index.js"; + operations, + parseOperationInput, + runOperation, + type Operation, + type OperationRunResult, +} from "./operations/index.js"; import { installRepoFiles, installGlobalSkill, @@ -109,6 +94,52 @@ function outputJson(data: unknown): void { process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); } +function optionalIntegerInput(value: string | undefined): unknown { + if (value === undefined) return undefined; + const parsed = Number(value); + return Number.isInteger(parsed) ? parsed : value; +} + +function optionalNumberInput(value: string | undefined): unknown { + if (value === undefined) return undefined; + const parsed = Number(value); + return Number.isNaN(parsed) ? value : parsed; +} + +function printOperationError(result: { error: string; data?: unknown }): never { + process.stderr.write(`Error: ${result.error}\n`); + if (isJsonObject(result.data) && Array.isArray(result.data.suggestions) && result.data.suggestions.length > 0) { + process.stderr.write(`\nDid you mean:\n`); + for (const suggestion of result.data.suggestions) { + process.stderr.write(` ${suggestion}\n`); + } + } + process.exit(1); +} + +function parseCliOperationInput( + operation: Operation, + input: unknown, +): TInput { + const parsed = parseOperationInput(operation, input); + if (!parsed.ok) { + process.stderr.write(`Error: ${parsed.error}\n`); + process.exit(2); + } + return parsed.data; +} + +function runCliOperation( + operation: Operation, + graph: CodebaseGraph, + input: TInput, + context = {}, +): TResult { + const result: OperationRunResult = runOperation(operation, graph, input, context); + if (!result.ok) printOperationError(result); + return result.data; +} + /** Load (or parse+cache) the codebase graph for a target path. */ function loadGraph(targetPath: string, force = false): { graph: CodebaseGraph; headHash: string } { const resolved = path.resolve(targetPath); @@ -262,8 +293,9 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.overview, {}); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeOverview(graph); + const result = runCliOperation(operations.overview, graph, input); if (options.json) { outputJson(result); @@ -306,18 +338,12 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: HotspotOptions) => { - const metric = options.metric ?? "coupling"; - if (!isHotspotMetric(metric)) { - process.stderr.write(`Error: --metric must be one of: ${HOTSPOT_METRICS.join(", ")}\n`); - process.exit(2); - } - const limit = options.limit ? parseInt(options.limit, 10) : 10; - if (isNaN(limit) || limit < 1) { - process.stderr.write("Error: --limit must be a positive integer\n"); - process.exit(2); - } + const input = parseCliOperationInput(operations.hotspots, { + metric: options.metric ?? "coupling", + limit: optionalIntegerInput(options.limit), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeHotspots(graph, metric, limit); + const result = runCliOperation(operations.hotspots, graph, input); if (options.json) { outputJson(result); @@ -349,19 +375,10 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, filePath: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.fileContext, { filePath }); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeFileContext(graph, filePath); - - if ("error" in result) { - process.stderr.write(`Error: ${result.error}\n`); - if (result.suggestions.length > 0) { - process.stderr.write(`\nDid you mean:\n`); - for (const s of result.suggestions) { - process.stderr.write(` ${s}\n`); - } - } - process.exit(1); - } + const result = runCliOperation(operations.fileContext, graph, input); + if ("error" in result) printOperationError({ error: result.error, data: result }); if (options.json) { outputJson(result); @@ -428,13 +445,12 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, query: string, options: SearchOptions) => { + const input = parseCliOperationInput(operations.search, { + query, + limit: options.limit === undefined ? 20 : optionalIntegerInput(options.limit), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const limit = options.limit ? parseInt(options.limit, 10) : 20; - if (isNaN(limit) || limit < 1) { - process.stderr.write("Error: --limit must be a positive integer\n"); - process.exit(2); - } - const result = computeSearch(graph, query, limit); + const result = runCliOperation(operations.search, graph, input); if (options.json) { outputJson(result); @@ -469,20 +485,10 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: ChangesOptions) => { - const scope = options.scope; - if (scope !== undefined) { - if (!isChangeScope(scope)) { - process.stderr.write(`Error: --scope must be one of: ${CHANGE_SCOPES.join(", ")}\n`); - process.exit(2); - } - } + const input = parseCliOperationInput(operations.changes, { scope: options.scope }); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeChanges(graph, scope, path.resolve(targetPath)); - - if ("error" in result) { - process.stderr.write(`Error: ${result.error}\n`); - process.exit(1); - } + const result = runCliOperation(operations.changes, graph, input, { rootDir: path.resolve(targetPath) }); + if ("error" in result) printOperationError({ error: result.error, data: result }); if (options.json) { outputJson(result); @@ -542,18 +548,13 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, filePath: string, options: DependentsOptions) => { + const input = parseCliOperationInput(operations.dependents, { + filePath, + depth: optionalIntegerInput(options.depth), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const depth = options.depth ? parseInt(options.depth, 10) : undefined; - if (depth !== undefined && (isNaN(depth) || depth < 1)) { - process.stderr.write("Error: --depth must be a positive integer\n"); - process.exit(2); - } - const result = computeDependents(graph, filePath, depth); - - if ("error" in result) { - process.stderr.write(`Error: ${result.error}\n`); - process.exit(1); - } + const result = runCliOperation(operations.dependents, graph, input); + if ("error" in result) printOperationError({ error: result.error, data: result }); if (options.json) { outputJson(result); @@ -591,8 +592,9 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.moduleStructure, {}); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeModuleStructure(graph); + const result = runCliOperation(operations.moduleStructure, graph, input); if (options.json) { outputJson(result); @@ -638,11 +640,13 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: ForcesOptions) => { + const input = parseCliOperationInput(operations.forces, { + cohesionThreshold: optionalNumberInput(options.cohesion), + tensionThreshold: optionalNumberInput(options.tension), + escapeThreshold: optionalNumberInput(options.escape), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const cohesion = options.cohesion ? parseFloat(options.cohesion) : undefined; - const tension = options.tension ? parseFloat(options.tension) : undefined; - const escape = options.escape ? parseFloat(options.escape) : undefined; - const result = computeForces(graph, cohesion, tension, escape); + const result = runCliOperation(operations.forces, graph, input); if (options.json) { outputJson(result); @@ -735,13 +739,12 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: DeadExportsOptions) => { + const input = parseCliOperationInput(operations.deadExports, { + module: options.module, + limit: optionalIntegerInput(options.limit), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const limit = options.limit ? parseInt(options.limit, 10) : undefined; - if (limit !== undefined && (isNaN(limit) || limit < 1)) { - process.stderr.write("Error: --limit must be a positive integer\n"); - process.exit(2); - } - const result = computeDeadExports(graph, options.module, limit); + const result = runCliOperation(operations.deadExports, graph, input); if (options.json) { outputJson(result); @@ -774,13 +777,11 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: OpportunitiesOptions) => { + const input = parseCliOperationInput(operations.opportunities, { + limit: optionalIntegerInput(options.limit), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const limit = options.limit ? parseInt(options.limit, 10) : undefined; - if (limit !== undefined && (isNaN(limit) || limit < 1)) { - process.stderr.write("Error: --limit must be a positive integer\n"); - process.exit(2); - } - const result = computeOpportunities(graph, limit); + const result = runCliOperation(operations.opportunities, graph, input); if (options.json) { outputJson(result); @@ -812,8 +813,9 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.groups, {}); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeGroups(graph); + const result = runCliOperation(operations.groups, graph, input); if (options.json) { outputJson(result); @@ -841,13 +843,10 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, symbolName: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.symbolContext, { name: symbolName }); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = computeSymbolContext(graph, symbolName); - - if ("error" in result) { - process.stderr.write(`Error: ${result.error}\n`); - process.exit(1); - } + const result = runCliOperation(operations.symbolContext, graph, input); + if ("error" in result) printOperationError({ error: result.error, data: result }); if (options.json) { outputJson(result); @@ -893,13 +892,9 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, symbol: string, options: CliCommandOptions) => { + const input = parseCliOperationInput(operations.impact, { symbol }); const { graph } = loadGraph(targetPath, forceOption(options)); - const result = impactAnalysis(graph, symbol); - - if (result.notFound) { - process.stderr.write(`Error: Symbol not found: ${symbol}\n`); - process.exit(1); - } + const result = runCliOperation(operations.impact, graph, input); if (options.json) { outputJson(result); @@ -933,9 +928,10 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, oldName: string, newName: string, options: RenameOptions) => { - const { graph } = loadGraph(targetPath, forceOption(options)); const dryRun = options.dryRun !== false; - const result = renameSymbol(graph, oldName, newName, dryRun); + const input = parseCliOperationInput(operations.rename, { oldName, newName, dryRun }); + const { graph } = loadGraph(targetPath, forceOption(options)); + const result = runCliOperation(operations.rename, graph, input); if (options.json) { outputJson(result); @@ -967,13 +963,12 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: ProcessesOptions) => { + const input = parseCliOperationInput(operations.processes, { + entryPoint: options.entry, + limit: optionalIntegerInput(options.limit), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const limit = options.limit ? parseInt(options.limit, 10) : undefined; - if (limit !== undefined && (isNaN(limit) || limit < 1)) { - process.stderr.write("Error: --limit must be a positive integer\n"); - process.exit(2); - } - const result = computeProcesses(graph, options.entry, limit); + const result = runCliOperation(operations.processes, graph, input); if (options.json) { outputJson(result); @@ -1008,13 +1003,11 @@ program .option("--json", "Output as JSON") .option("--force", "Re-index even if HEAD unchanged") .action((targetPath: string, options: ClustersOptions) => { + const input = parseCliOperationInput(operations.clusters, { + minFiles: optionalIntegerInput(options.minFiles), + }); const { graph } = loadGraph(targetPath, forceOption(options)); - const minFiles = options.minFiles ? parseInt(options.minFiles, 10) : undefined; - if (minFiles !== undefined && (isNaN(minFiles) || minFiles < 1)) { - process.stderr.write("Error: --min-files must be a positive integer\n"); - process.exit(2); - } - const result = computeClusters(graph, minFiles); + const result = runCliOperation(operations.clusters, graph, input); if (options.json) { outputJson(result); diff --git a/tests/cli-commands.test.ts b/tests/cli-commands.test.ts index e8e0f9a..c2e31ec 100644 --- a/tests/cli-commands.test.ts +++ b/tests/cli-commands.test.ts @@ -609,7 +609,7 @@ describe("CLI core commands (integration)", () => { ); expect(result.status).toBe(2); - expect(result.stderr).toContain("Error: --metric must be one of:"); + expect(result.stderr).toContain("Error: metric: Invalid enum value"); expect(result.stderr).not.toContain("Parsing"); }); @@ -621,7 +621,7 @@ describe("CLI core commands (integration)", () => { ); expect(result.status).toBe(2); - expect(result.stderr).toContain("Error: --scope must be one of:"); + expect(result.stderr).toContain("Error: scope: Invalid enum value"); expect(result.stderr).not.toContain("Parsing"); }); }); diff --git a/tests/operation-registry.e2e.test.ts b/tests/operation-registry.e2e.test.ts index 1861889..3ef460a 100644 --- a/tests/operation-registry.e2e.test.ts +++ b/tests/operation-registry.e2e.test.ts @@ -1,5 +1,4 @@ import { spawnSync, execSync } from "node:child_process"; -import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { beforeAll, describe, expect, it } from "vitest"; @@ -65,6 +64,12 @@ function withoutNextSteps(payload: Record): Record): Record { + const normalized = { ...payload }; + delete normalized.cache; + return normalized; +} + async function expectMcpMatchesRegistry( operation: Operation, input: TInput, @@ -80,8 +85,8 @@ async function expectMcpMatchesRegistry( expect(withoutNextSteps(mcpPayload)).toEqual(registryResult.data); } -function runCliOverview(): Record { - const res = spawnSync("node", [cli, "overview", getFixtureSrcPath(), "--json", "--force"], { +function runCliJson(args: string[]): Record { + const res = spawnSync("node", [cli, ...args, "--json", "--force"], { cwd: repoRoot, env: { ...process.env }, encoding: "utf-8", @@ -92,8 +97,22 @@ function runCliOverview(): Record { return parseJsonObject(res.stdout); } +function expectCliMatchesRegistry( + operation: Operation, + input: TInput, + cliArgs: string[], + graph: CodebaseGraph, +): void { + const registryResult = runOperation(operation, graph, input); + expect(registryResult.ok).toBe(true); + if (!registryResult.ok) throw new Error(registryResult.error); + + const cliPayload = runCliJson(cliArgs); + expect(withoutCache(cliPayload)).toEqual(registryResult.data); +} + beforeAll(() => { - if (!fs.existsSync(cli)) execSync("pnpm build", { cwd: repoRoot, stdio: "inherit" }); + execSync("pnpm build", { cwd: repoRoot, stdio: "inherit" }); }, 120_000); describe("operation registry chained parity", () => { @@ -103,7 +122,7 @@ describe("operation registry chained parity", () => { expect(registryResult.ok).toBe(true); if (!registryResult.ok) throw new Error(registryResult.error); - const cliPayload = runCliOverview(); + const cliPayload = runCliJson(["overview", getFixtureSrcPath()]); expect(cliPayload).toHaveProperty("cache"); const mcp = await createFixtureMcp(); @@ -115,6 +134,41 @@ describe("operation registry chained parity", () => { expect(normalizeOverviewPayload(mcpPayload)).toEqual(registryOverview); }); + it("CH-P1-01: registry-adapted CLI JSON commands match descriptor runs", () => { + const { codebaseGraph } = getFixturePipeline(); + + expectCliMatchesRegistry( + operations.fileContext, + { filePath: "index.ts" }, + ["file", getFixtureSrcPath(), "index.ts"], + codebaseGraph, + ); + expectCliMatchesRegistry( + operations.hotspots, + { metric: "coupling", limit: 3 }, + ["hotspots", getFixtureSrcPath(), "--metric", "coupling", "--limit", "3"], + codebaseGraph, + ); + expectCliMatchesRegistry( + operations.impact, + { symbol: "getUserById" }, + ["impact", getFixtureSrcPath(), "getUserById"], + codebaseGraph, + ); + }); + + it("CH-P1-02: CLI invalid input uses descriptor validation before indexing", () => { + const res = spawnSync("node", [cli, "hotspots", getFixtureSrcPath(), "--metric", "bad", "--json"], { + cwd: repoRoot, + env: { ...process.env }, + encoding: "utf-8", + }); + + expect(res.status).toBe(2); + expect(res.stderr).toContain("Error: metric: Invalid enum value"); + expect(res.stderr).not.toContain("Parsing"); + }); + it("CH-P1-01: registry-adapted MCP tools match descriptor runs", async () => { const { codebaseGraph } = getFixturePipeline(); const mcp = await createFixtureMcp(); diff --git a/tools/verify-cli-real-codebases.mjs b/tools/verify-cli-real-codebases.mjs index b8c0d57..0fc3bf3 100644 --- a/tools/verify-cli-real-codebases.mjs +++ b/tools/verify-cli-real-codebases.mjs @@ -424,13 +424,13 @@ record("init: temp repo", () => { record("invalid hotspot metric exits 2", () => { const result = run(["hotspots", ".", "--metric", "nope", "--json"], [2]); - if (!result.stderr.includes("--metric must be one of")) throw new Error("missing metric error"); + if (!result.stderr.includes("metric: Invalid enum value")) throw new Error("missing metric error"); return "exit 2"; }); record("invalid changes scope exits 2", () => { const result = run(["changes", ".", "--scope", "nope", "--json"], [2]); - if (!result.stderr.includes("--scope must be one of")) throw new Error("missing scope error"); + if (!result.stderr.includes("scope: Invalid enum value")) throw new Error("missing scope error"); return "exit 2"; });