From 0f377cae71f68677872e5154007c37d74e5868c4 Mon Sep 17 00:00:00 2001 From: bntvllnt <32437578+bntvllnt@users.noreply.github.com> Date: Wed, 1 Jul 2026 04:26:35 +0200 Subject: [PATCH] test: expand operation registry parity coverage --- docs/mcp-tools.md | 2 + llms-full.txt | 1 + roadmap.md | 5 +- src/community/index.ts | 45 ++++- src/mcp/index.ts | 18 +- tests/helpers/mcp.ts | 10 +- tests/operation-registry.e2e.test.ts | 242 +++++++++++++++++++++++++-- tests/operation-registry.test.ts | 18 ++ tests/phase3-red.test.ts | 15 +- 9 files changed, 323 insertions(+), 33 deletions(-) diff --git a/docs/mcp-tools.md b/docs/mcp-tools.md index 17d1a9e..07c02f5 100644 --- a/docs/mcp-tools.md +++ b/docs/mcp-tools.md @@ -2,6 +2,8 @@ 17 tools available via MCP stdio. +Operation tools return JSON text payloads. Invalid operation inputs return `isError: true` with `{ "error": "..." }` using the same descriptor validation messages as CLI bad-argument exits. + ## 1. codebase_overview High-level summary of the entire codebase. diff --git a/llms-full.txt b/llms-full.txt index 51cae9f..0cfb417 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -447,3 +447,4 @@ Gate modes: all, new-only. Returns pass/warn/fail verdict, findings, and summary - **JSON mode**: `--json` outputs stable JSON schema to stdout. - **Exit codes**: 0 = success, 1 = runtime error, 2 = bad args/usage. - **MCP mode**: `codebase-intelligence ` (no subcommand) starts MCP stdio server. +- **MCP operation errors**: Invalid operation inputs return `isError: true` with JSON text `{ "error": "..." }` using the same descriptor validation messages as CLI bad-argument exits. diff --git a/roadmap.md b/roadmap.md index 407d680..85a11e9 100644 --- a/roadmap.md +++ b/roadmap.md @@ -273,13 +273,14 @@ Collapse CLI + MCP operation duplication into one descriptor registry before add - 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. +- Expand CH-P1-01 coverage from overview/representative CLI/MCP operations to every operation. +- Expand CH-P1-02 coverage for descriptor validation, CLI parse failure, and cache reuse through registry-adapted commands. **Remaining:** - 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 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. +- Extend CH-P1-02 coverage to MCP/stdio graph-load behavior after the shared graph-load pipeline exists. ### Type/Shape Layer diff --git a/src/community/index.ts b/src/community/index.ts index bb4e3cc..6eb12d0 100644 --- a/src/community/index.ts +++ b/src/community/index.ts @@ -6,46 +6,73 @@ import type { CodebaseGraph, Cluster } from "../types/index.js"; export function detectCommunities(graph: CodebaseGraph): Cluster[] { const undirected = new Graph({ type: "undirected" }); - for (const node of graph.nodes) { + const fileNodes = graph.nodes + .filter((node) => node.type === "file") + .sort((a, b) => a.id.localeCompare(b.id)); + + for (const node of fileNodes) { if (node.type !== "file") continue; if (!undirected.hasNode(node.id)) { undirected.addNode(node.id, { module: node.module }); } } + const dependencyEdges = new Map(); + for (const edge of graph.edges) { if (!undirected.hasNode(edge.source) || !undirected.hasNode(edge.target)) continue; if (edge.source === edge.target) continue; - if (!undirected.hasEdge(edge.source, edge.target) && !undirected.hasEdge(edge.target, edge.source)) { - undirected.addEdge(edge.source, edge.target, { weight: edge.weight }); + + const source = edge.source.localeCompare(edge.target) <= 0 ? edge.source : edge.target; + const target = source === edge.source ? edge.target : edge.source; + const key = `${source}\t${target}`; + const existing = dependencyEdges.get(key); + if (existing) { + existing.weight += edge.weight; + } else { + dependencyEdges.set(key, { source, target, weight: edge.weight }); } } + const sortedDependencyEdges = [...dependencyEdges.values()].sort((a, b) => + `${a.source}\t${a.target}`.localeCompare(`${b.source}\t${b.target}`) + ); + + for (const edge of sortedDependencyEdges) { + undirected.addEdge(edge.source, edge.target, { weight: edge.weight }); + } + if (undirected.order === 0) return []; - const communities = louvain(undirected) as Record; + const communities = louvain(undirected, { randomWalk: false }) as Record; const clusterMap = new Map(); - for (const [nodeId, clusterId] of Object.entries(communities)) { + for (const [nodeId, clusterId] of Object.entries(communities).sort(([a], [b]) => a.localeCompare(b))) { const existing = clusterMap.get(clusterId) ?? []; existing.push(nodeId); clusterMap.set(clusterId, existing); } - const clusters: Cluster[] = []; - for (const [clusterId, files] of clusterMap) { + const clusters: Array> = []; + for (const files of clusterMap.values()) { + files.sort((a, b) => a.localeCompare(b)); const commonModule = findDominantModule(files, graph); const cohesion = computeClusterCohesion(files, graph); clusters.push({ - id: `cluster-${clusterId}`, name: commonModule, files, cohesion, }); } - return clusters.sort((a, b) => b.files.length - a.files.length); + return clusters + .sort((a, b) => + b.files.length - a.files.length || + a.name.localeCompare(b.name) || + (a.files[0] ?? "").localeCompare(b.files[0] ?? "") + ) + .map((cluster, index) => ({ id: `cluster-${index}`, ...cluster })); } function findDominantModule(files: string[], graph: CodebaseGraph): string { diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 6198810..6865401 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -54,16 +54,28 @@ function errorPayload( return nextSteps ? { ...payload, nextSteps } : payload; } +function mcpInputSchema( + operation: Operation, +): z.ZodType> { + const shape: z.ZodRawShape = {}; + for (const [key, schema] of Object.entries(operation.inputShape)) { + shape[key] = schema.catch((context: { input: unknown }) => context.input); + } + return z.object(shape).passthrough(); +} + function registerOperationTool( server: McpServer, graph: CodebaseGraph, operation: Operation, options: OperationToolOptions = {}, ): void { - server.tool( + server.registerTool( operation.mcpTool, - operation.description, - operation.inputShape, + { + description: operation.description, + inputSchema: mcpInputSchema(operation), + }, async (rawInput) => { const parsed = parseOperationInput(operation, rawInput); if (!parsed.ok) { diff --git a/tests/helpers/mcp.ts b/tests/helpers/mcp.ts index 6ab3e0f..f73b5a7 100644 --- a/tests/helpers/mcp.ts +++ b/tests/helpers/mcp.ts @@ -5,13 +5,14 @@ import { registerTools } from "../../src/mcp/index.js"; import { setGraph, setIndexedHead, setRoot } from "../../src/server/graph-store.js"; import { getFixturePipeline, getFixtureSrcPath } from "./pipeline.js"; -export interface ToolPayload { +interface ToolPayload { payload: Record; isError: boolean; } export interface FixtureMcp { listTools(): Promise; + listToolMetadata(): Promise>; callTool(name: string, args?: Record): Promise>; callToolWithMeta(name: string, args?: Record): Promise; } @@ -59,6 +60,13 @@ export async function createFixtureMcp(rootDir = getFixtureSrcPath()): Promise tool.name); }, + async listToolMetadata(): Promise> { + const result = await client.listTools(); + return result.tools.map((tool) => ({ + name: tool.name, + inputSchema: tool.inputSchema, + })); + }, async callTool(name: string, args: Record = {}): Promise> { const result = await client.callTool({ name, arguments: args }); return parsePayload(firstTextContent(result)); diff --git a/tests/operation-registry.e2e.test.ts b/tests/operation-registry.e2e.test.ts index 3ef460a..00f1f80 100644 --- a/tests/operation-registry.e2e.test.ts +++ b/tests/operation-registry.e2e.test.ts @@ -1,9 +1,11 @@ -import { spawnSync, execSync } from "node:child_process"; +import { spawnSync, execSync, type SpawnSyncReturns } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { beforeAll, describe, expect, it } from "vitest"; import type { OverviewResult } from "../src/core/index.js"; -import { operations, runOperation, type Operation } from "../src/operations/index.js"; +import { operations, runOperation, type Operation, type OperationContext } from "../src/operations/index.js"; import type { CodebaseGraph } from "../src/types/index.js"; import { createFixtureMcp, type FixtureMcp } from "./helpers/mcp.js"; import { getFixturePipeline, getFixtureSrcPath } from "./helpers/pipeline.js"; @@ -76,8 +78,9 @@ async function expectMcpMatchesRegistry( mcpArgs: Record, graph: CodebaseGraph, mcp: FixtureMcp, + context: OperationContext = {}, ): Promise { - const registryResult = runOperation(operation, graph, input); + const registryResult = runOperation(operation, graph, input, context); expect(registryResult.ok).toBe(true); if (!registryResult.ok) throw new Error(registryResult.error); @@ -85,15 +88,30 @@ async function expectMcpMatchesRegistry( expect(withoutNextSteps(mcpPayload)).toEqual(registryResult.data); } -function runCliJson(args: string[]): Record { - const res = spawnSync("node", [cli, ...args, "--json", "--force"], { +interface CliRunOptions { + force?: boolean; +} + +function runCli(args: string[], options: CliRunOptions = {}): SpawnSyncReturns { + const cliArgs = ["node", cli, ...args, "--json"]; + if (options.force !== false) { + cliArgs.push("--force"); + } + + return spawnSync(cliArgs[0], cliArgs.slice(1), { cwd: repoRoot, env: { ...process.env }, encoding: "utf-8", }); +} + +function runCliJson(args: string[], options: CliRunOptions = {}): Record { + const res = runCli(args, options); expect(res.status).toBe(0); - expect(res.stderr).toContain("Parsed"); + if (options.force !== false) { + expect(res.stderr).toContain("Parsed"); + } return parseJsonObject(res.stdout); } @@ -102,12 +120,14 @@ function expectCliMatchesRegistry( input: TInput, cliArgs: string[], graph: CodebaseGraph, + context: OperationContext = {}, + runOptions: CliRunOptions = {}, ): void { - const registryResult = runOperation(operation, graph, input); + const registryResult = runOperation(operation, graph, input, context); expect(registryResult.ok).toBe(true); if (!registryResult.ok) throw new Error(registryResult.error); - const cliPayload = runCliJson(cliArgs); + const cliPayload = runCliJson(cliArgs, runOptions); expect(withoutCache(cliPayload)).toEqual(registryResult.data); } @@ -134,45 +154,162 @@ describe("operation registry chained parity", () => { expect(normalizeOverviewPayload(mcpPayload)).toEqual(registryOverview); }); - it("CH-P1-01: registry-adapted CLI JSON commands match descriptor runs", () => { + it("CH-P1-01: registry-adapted CLI JSON commands match descriptor runs for every operation", () => { const { codebaseGraph } = getFixturePipeline(); + runCliJson(["overview", getFixtureSrcPath()]); + const cachedRun = { force: false }; + expectCliMatchesRegistry(operations.overview, {}, ["overview", getFixtureSrcPath()], codebaseGraph, {}, cachedRun); expectCliMatchesRegistry( operations.fileContext, { filePath: "index.ts" }, ["file", getFixtureSrcPath(), "index.ts"], codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.dependents, + { filePath: "auth/auth-service.ts", depth: 2 }, + ["dependents", getFixtureSrcPath(), "auth/auth-service.ts", "--depth", "2"], + codebaseGraph, + {}, + cachedRun, ); expectCliMatchesRegistry( operations.hotspots, { metric: "coupling", limit: 3 }, ["hotspots", getFixtureSrcPath(), "--metric", "coupling", "--limit", "3"], codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry(operations.moduleStructure, {}, ["modules", getFixtureSrcPath()], codebaseGraph, {}, cachedRun); + expectCliMatchesRegistry( + operations.forces, + { cohesionThreshold: 0.6, tensionThreshold: 0.3, escapeThreshold: 0.5 }, + ["forces", getFixtureSrcPath(), "--cohesion", "0.6", "--tension", "0.3", "--escape", "0.5"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.deadExports, + { limit: 5 }, + ["dead-exports", getFixtureSrcPath(), "--limit", "5"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.opportunities, + { limit: 5 }, + ["opportunities", getFixtureSrcPath(), "--limit", "5"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry(operations.groups, {}, ["groups", getFixtureSrcPath()], codebaseGraph, {}, cachedRun); + expectCliMatchesRegistry( + operations.symbolContext, + { name: "getUserById" }, + ["symbol", getFixtureSrcPath(), "getUserById"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.search, + { query: "auth", limit: 5 }, + ["search", getFixtureSrcPath(), "auth", "--limit", "5"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.changes, + { scope: "all" }, + ["changes", getFixtureSrcPath(), "--scope", "all"], + codebaseGraph, + { rootDir: getFixtureSrcPath() }, + cachedRun, ); expectCliMatchesRegistry( operations.impact, { symbol: "getUserById" }, ["impact", getFixtureSrcPath(), "getUserById"], codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.rename, + { oldName: "getUserById", newName: "findUserById", dryRun: true }, + ["rename", getFixtureSrcPath(), "getUserById", "findUserById"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.processes, + { entryPoint: "main", limit: 1 }, + ["processes", getFixtureSrcPath(), "--entry", "main", "--limit", "1"], + codebaseGraph, + {}, + cachedRun, + ); + expectCliMatchesRegistry( + operations.clusters, + { minFiles: 2 }, + ["clusters", getFixtureSrcPath(), "--min-files", "2"], + codebaseGraph, + {}, + cachedRun, ); }); - 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", - }); + it("CH-P1-02: invalid input uses shared descriptor validation before indexing", async () => { + const res = runCli(["hotspots", getFixtureSrcPath(), "--metric", "bad"], { force: false }); expect(res.status).toBe(2); expect(res.stderr).toContain("Error: metric: Invalid enum value"); expect(res.stderr).not.toContain("Parsing"); + + const cliError = res.stderr.trim().replace(/^Error: /, ""); + const mcp = await createFixtureMcp(); + const mcpResult = await mcp.callToolWithMeta("find_hotspots", { metric: "bad" }); + expect(mcpResult.isError).toBe(true); + expect(mcpResult.payload).toEqual({ error: cliError }); + }); + + it("CH-P1-02: CLI graph adapter reports parse failures without JSON noise", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "cbi-empty-")); + try { + const res = runCli(["overview", tempDir], { force: false }); + expect(res.status).toBe(1); + expect(res.stdout).toBe(""); + expect(res.stderr).toContain("Error: No TypeScript files found"); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it("CH-P1-02: CLI graph adapter reuses cache after a successful registry command", () => { + const first = runCli(["overview", getFixtureSrcPath()]); + expect(first.status).toBe(0); + expect(first.stderr).toContain("Index saved"); + + const second = runCli(["overview", getFixtureSrcPath()], { force: false }); + expect(second.status).toBe(0); + expect(second.stderr).toContain("Using cached index"); + expect(withoutCache(parseJsonObject(second.stdout))).toEqual(withoutCache(parseJsonObject(first.stdout))); }); - it("CH-P1-01: registry-adapted MCP tools match descriptor runs", async () => { + it("CH-P1-01: registry-adapted MCP tools match descriptor runs for every operation", async () => { const { codebaseGraph } = getFixturePipeline(); const mcp = await createFixtureMcp(); + await expectMcpMatchesRegistry(operations.overview, {}, {}, codebaseGraph, mcp); await expectMcpMatchesRegistry( operations.fileContext, { filePath: "index.ts" }, @@ -180,6 +317,13 @@ describe("operation registry chained parity", () => { codebaseGraph, mcp, ); + await expectMcpMatchesRegistry( + operations.dependents, + { filePath: "auth/auth-service.ts", depth: 2 }, + { filePath: "auth/auth-service.ts", depth: 2 }, + codebaseGraph, + mcp, + ); await expectMcpMatchesRegistry( operations.hotspots, { metric: "coupling", limit: 3 }, @@ -187,6 +331,51 @@ describe("operation registry chained parity", () => { codebaseGraph, mcp, ); + await expectMcpMatchesRegistry(operations.moduleStructure, {}, {}, codebaseGraph, mcp); + await expectMcpMatchesRegistry( + operations.forces, + { cohesionThreshold: 0.6, tensionThreshold: 0.3, escapeThreshold: 0.5 }, + { cohesionThreshold: 0.6, tensionThreshold: 0.3, escapeThreshold: 0.5 }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.deadExports, + { limit: 5 }, + { limit: 5 }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.opportunities, + { limit: 5 }, + { limit: 5 }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry(operations.groups, {}, {}, codebaseGraph, mcp); + await expectMcpMatchesRegistry( + operations.symbolContext, + { name: "getUserById" }, + { name: "getUserById" }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.search, + { query: "auth", limit: 5 }, + { query: "auth", limit: 5 }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.changes, + { scope: "all" }, + { scope: "all" }, + codebaseGraph, + mcp, + { rootDir: getFixtureSrcPath() }, + ); await expectMcpMatchesRegistry( operations.impact, { symbol: "getUserById" }, @@ -194,5 +383,26 @@ describe("operation registry chained parity", () => { codebaseGraph, mcp, ); + await expectMcpMatchesRegistry( + operations.rename, + { oldName: "getUserById", newName: "findUserById", dryRun: true }, + { oldName: "getUserById", newName: "findUserById", dryRun: true }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.processes, + { entryPoint: "main", limit: 1 }, + { entryPoint: "main", limit: 1 }, + codebaseGraph, + mcp, + ); + await expectMcpMatchesRegistry( + operations.clusters, + { minFiles: 2 }, + { minFiles: 2 }, + codebaseGraph, + mcp, + ); }); }); diff --git a/tests/operation-registry.test.ts b/tests/operation-registry.test.ts index ad56713..b2e571b 100644 --- a/tests/operation-registry.test.ts +++ b/tests/operation-registry.test.ts @@ -99,6 +99,24 @@ describe("operation registry", () => { expect(await mcp.listTools()).toEqual([...operationList.map((operation) => operation.mcpTool), "check"]); }); + it("keeps MCP operation input fields discoverable while registry validation owns errors", async () => { + const mcp = await createFixtureMcp(); + const tools = await mcp.listToolMetadata(); + const hotspots = tools.find((tool) => tool.name === "find_hotspots"); + + expect(hotspots?.inputSchema).toMatchObject({ + type: "object", + properties: { + metric: { + enum: expect.arrayContaining(["coupling", "blast_radius"]), + }, + limit: { + type: "integer", + }, + }, + }); + }); + it("keeps registry-adapted MCP lookup errors in the existing envelope", async () => { const mcp = await createFixtureMcp(); const result = await mcp.callToolWithMeta("impact_analysis", { symbol: "MissingSymbol" }); diff --git a/tests/phase3-red.test.ts b/tests/phase3-red.test.ts index 2267d19..0871a0f 100644 --- a/tests/phase3-red.test.ts +++ b/tests/phase3-red.test.ts @@ -145,6 +145,17 @@ describe("3.6 — Louvain clustering", () => { expect(cluster.cohesion).toBeGreaterThanOrEqual(0); } }); + + it("returns stable cluster ids and membership across repeated runs", async () => { + const { detectCommunities } = await import("../src/community/index.js"); + const { codebaseGraph } = getFixturePipeline(); + + const first = detectCommunities(codebaseGraph); + const second = detectCommunities(codebaseGraph); + + expect(second).toEqual(first); + expect(first.map((cluster) => cluster.id)).toEqual(first.map((_, index) => `cluster-${index}`)); + }); }); describe("3.7 — detect_changes MCP tool", () => { @@ -164,8 +175,8 @@ describe("3.8 — detect_changes without git", () => { it("detect_changes returns clear error when git unavailable", async () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ci-detect-no-git-")); try { - const { callToolWithMeta } = await createFixtureMcp(dir); - const { payload, isError } = await callToolWithMeta("detect_changes"); + const mcp = await createFixtureMcp(dir); + const { payload, isError } = await mcp.callToolWithMeta("detect_changes"); expect(isError).toBe(true); expect(payload).toHaveProperty("scope", "all");