test(mcp): cover remaining cli gaps

Replace remaining todo tests with real MCP contract coverage and clean detect_changes no-git errors.

Author: bntvllnt

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- **MCP contract coverage now exercises the remaining explicit todo stories.**
+  Search responses are verified as file-grouped with symbol locations and
+  `nextSteps`; `symbol_context` is verified for `AuthService`; `detect_changes`
+  is verified against a real non-git directory.
+
+### Fixed
+
+- **`detect_changes` no-git errors no longer leak raw `git diff` usage output.**
+  The command now suppresses git stderr and returns the structured
+  `Git not available or not in a git repository` error with recovery steps.
+
 ## [2.5.0-canary] - 2026-06-30
 
 > Canary base for 2.5.0. Merging to `main` publishes `2.5.0-canary.<sha>` with

src/core/index.ts

@@ -404,7 +404,12 @@ export function computeChanges(
       default: diffCmd = "git diff --relative HEAD --name-only"; break;
     }
 
-    const output = execSync(diffCmd, { cwd: path.resolve(rootDir), encoding: "utf-8", timeout: 5000 }).trim();
+    const output = execSync(diffCmd, {
+      cwd: path.resolve(rootDir),
+      encoding: "utf-8",
+      timeout: 5000,
+      stdio: ["ignore", "pipe", "ignore"],
+    }).trim();
     const changedFiles = output ? output.split("\n").filter((f) => f.length > 0) : [];
 
     const changedSymbols: Array<{ file: string; symbols: string[] }> = [];

tests/helpers/mcp.ts

@@ -0,0 +1,69 @@
+import { Client } from "@modelcontextprotocol/sdk/client/index.js";
+import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
+import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
+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 {
+  payload: Record<string, unknown>;
+  isError: boolean;
+}
+
+export interface FixtureMcp {
+  callTool(name: string, args?: Record<string, unknown>): Promise<Record<string, unknown>>;
+  callToolWithMeta(name: string, args?: Record<string, unknown>): Promise<ToolPayload>;
+}
+
+function hasText(value: unknown): value is { text: string } {
+  return typeof value === "object" && value !== null && "text" in value && typeof value.text === "string";
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+function firstTextContent(result: unknown): string {
+  if (!isRecord(result) || !Array.isArray(result.content) || !hasText(result.content[0])) {
+    throw new Error("MCP tool result did not include text content");
+  }
+  return result.content[0].text;
+}
+
+function parsePayload(text: string): Record<string, unknown> {
+  const parsed: unknown = JSON.parse(text);
+  if (!isRecord(parsed)) {
+    throw new Error("MCP tool result was not a JSON object");
+  }
+  return parsed;
+}
+
+export async function createFixtureMcp(rootDir = getFixtureSrcPath()): Promise<FixtureMcp> {
+  const { codebaseGraph } = getFixturePipeline();
+  setGraph(codebaseGraph);
+  setRoot(rootDir);
+  setIndexedHead("abc123-test");
+
+  const server = new McpServer({ name: "test", version: "0.1.0" });
+  registerTools(server, codebaseGraph);
+
+  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+  await server.connect(serverTransport);
+
+  const client = new Client({ name: "test-client", version: "0.1.0" });
+  await client.connect(clientTransport);
+
+  return {
+    async callTool(name: string, args: Record<string, unknown> = {}): Promise<Record<string, unknown>> {
+      const result = await client.callTool({ name, arguments: args });
+      return parsePayload(firstTextContent(result));
+    },
+    async callToolWithMeta(name: string, args: Record<string, unknown> = {}): Promise<ToolPayload> {
+      const result = await client.callTool({ name, arguments: args });
+      return {
+        payload: parsePayload(firstTextContent(result)),
+        isError: result.isError === true,
+      };
+    },
+  };
+}

tests/phase1-mcp-api.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, beforeAll } from "vitest";
 import { getFixturePipeline } from "./helpers/pipeline.js";
 import { setGraph } from "../src/server/graph-store.js";
+import { createFixtureMcp } from "./helpers/mcp.js";
 
 beforeAll(() => {
   const { codebaseGraph } = getFixturePipeline();
@@ -16,5 +17,29 @@ describe("1.3 — analyze_forces responds to threshold params", () => {
 });
 
 describe("1.8 — symbol_context MCP tool", () => {
-  it.todo("calling with 'AuthService' returns callers, callees, metrics, nextSteps");
+  it("calling with 'AuthService' returns callers, callees, metrics, nextSteps", async () => {
+    const { callTool } = await createFixtureMcp();
+    const result = await callTool("symbol_context", { name: "AuthService" });
+
+    expect(result).toHaveProperty("name", "AuthService");
+    expect(result).toHaveProperty("file", "auth/auth-service.ts");
+    expect(result).toHaveProperty("type", "class");
+    expect(result).toHaveProperty("callers");
+    expect(result).toHaveProperty("callees");
+    expect(result).toHaveProperty("fanIn");
+    expect(result).toHaveProperty("fanOut");
+    expect(result).toHaveProperty("pageRank");
+    expect(result).toHaveProperty("betweenness");
+    expect(result).toHaveProperty("nextSteps");
+
+    expect(Array.isArray(result.callers)).toBe(true);
+    expect(Array.isArray(result.callees)).toBe(true);
+    expect(typeof result.fanIn).toBe("number");
+    expect(typeof result.fanOut).toBe("number");
+    expect(typeof result.pageRank).toBe("number");
+    expect(typeof result.betweenness).toBe("number");
+    const nextSteps = result.nextSteps;
+    expect(Array.isArray(nextSteps)).toBe(true);
+    if (Array.isArray(nextSteps)) expect(nextSteps.length).toBeGreaterThan(0);
+  });
 });

tests/phase2-red.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, beforeAll } from "vitest";
 import { getFixturePipeline } from "./helpers/pipeline.js";
 import { setGraph } from "../src/server/graph-store.js";
+import { createFixtureMcp } from "./helpers/mcp.js";
 
 beforeAll(() => {
   const { codebaseGraph } = getFixturePipeline();
@@ -71,7 +72,53 @@ describe("2.2 — BM25 empty results return suggestions", () => {
 });
 
 describe("2.3 — search MCP tool", () => {
-  it.todo("search tool returns file-grouped results with symbol locations and nextSteps");
+  it("search tool returns file-grouped results with symbol locations and nextSteps", async () => {
+    const { callTool } = await createFixtureMcp();
+    const result = await callTool("search", { query: "auth", limit: 5 });
+
+    expect(result).toHaveProperty("query", "auth");
+    expect(result).toHaveProperty("results");
+    expect(result).toHaveProperty("nextSteps");
+
+    const results = result.results;
+    expect(Array.isArray(results)).toBe(true);
+    if (!Array.isArray(results)) return;
+    expect(results.length).toBeGreaterThan(0);
+
+    const files = new Set<string>();
+    const first = results[0];
+    expect(typeof first).toBe("object");
+    expect(first).not.toBeNull();
+    if (typeof first !== "object" || first === null) return;
+    expect(first).toHaveProperty("file");
+    expect(first).toHaveProperty("symbols");
+
+    for (const item of results) {
+      expect(typeof item).toBe("object");
+      expect(item).not.toBeNull();
+      if (typeof item !== "object" || item === null) continue;
+      const file = "file" in item ? item.file : undefined;
+      const symbols = "symbols" in item ? item.symbols : undefined;
+      expect(typeof file).toBe("string");
+      if (typeof file === "string") files.add(file);
+      expect(Array.isArray(symbols)).toBe(true);
+      if (!Array.isArray(symbols) || symbols.length === 0) continue;
+      const symbol = symbols[0];
+      expect(typeof symbol).toBe("object");
+      expect(symbol).not.toBeNull();
+      if (typeof symbol !== "object" || symbol === null) continue;
+      expect(symbol).toHaveProperty("name");
+      expect(symbol).toHaveProperty("type");
+      expect(symbol).toHaveProperty("loc");
+      const loc = "loc" in symbol ? symbol.loc : undefined;
+      expect(typeof loc).toBe("number");
+    }
+
+    expect(files.size).toBe(results.length);
+    const nextSteps = result.nextSteps;
+    expect(Array.isArray(nextSteps)).toBe(true);
+    if (Array.isArray(nextSteps)) expect(nextSteps.length).toBeGreaterThan(0);
+  });
 });
 
 describe("2.6 — existing tools include nextSteps", () => {

tests/phase3-red.test.ts

@@ -1,5 +1,6 @@
 import { describe, it, expect, beforeAll } from "vitest";
 import { getFixturePipeline } from "./helpers/pipeline.js";
+import { createFixtureMcp } from "./helpers/mcp.js";
 import { setGraph } from "../src/server/graph-store.js";
 import path from "path";
 import fs from "fs";
@@ -160,6 +161,24 @@ describe("3.7 — detect_changes MCP tool", () => {
 });
 
 describe("3.8 — detect_changes without git", () => {
-  it.todo("detect_changes returns clear error when git unavailable");
+  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");
+
+      expect(isError).toBe(true);
+      expect(payload).toHaveProperty("scope", "all");
+      expect(payload).toHaveProperty("error");
+      expect(String(payload.error)).toContain("Git not available");
+      expect(payload).toHaveProperty("nextSteps");
+      const nextSteps = payload.nextSteps;
+      expect(Array.isArray(nextSteps)).toBe(true);
+      if (Array.isArray(nextSteps)) {
+        expect(nextSteps.join(" ")).toContain("git repository");
+      }
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
 });
-