From ccf6b4743dad62e730d4b40f853b487f31adbd65 Mon Sep 17 00:00:00 2001 From: svjack Date: Sun, 27 Sep 2026 21:16:11 +0800 Subject: [PATCH] feat(mcp): add read_file + grep tools (REST parity for /api/file slice + /api/grep) (#3377) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mcp): add read_file + grep tools (REST parity for /api/file slice + /api/grep) * chore(autofix): apply prettier + eslint fixes via /autofix command * Address PR review feedback (#3377) - Fail read_file and grep when full source is unavailable, matching the HTTP 410 contract instead of an empty grep or a not-found on a missing checkout. - Reject branch on those tools so a pinned index is not labeled onto checkout bytes, and stop advertising branch in their schemas. - Point the grep hint at a 0-based read_file window, pass caseSensitive and literal through, and test the handlers. Co-authored-by: Cursor * Address PR review feedback (#3377) - Keep read_file and grep in the multi-repo schema requirement without advertising branch. - Reject negative maxLines and return integer slice bounds for fractional line positions. Co-authored-by: Cursor * Address PR review feedback (#3377) - Reject a negative read_file endLine before slicing so JavaScript does not treat it as an offset from the end of the file. - Drop the fractional startLine/endLine claim so the integer schema is the advertised contract. Co-authored-by: Cursor * Address PR review feedback (#3377) - Skip indexed grep paths whose realpath leaves the checkout so a symlink cannot return lines from outside the repo. Co-authored-by: Cursor * fix(bench): record the 19-tool MCP roster read_file and grep are real tools, so tools/list and GITNEXUS_TOOLS both moved from 17 to 19. The timing ratios were already inside budget. Co-authored-by: Cursor * refactor(mcp): share read_file and grep contracts with existing helpers Boolean grep flags go through isFlagTrue, the whole-file cap is one constant, and checkout tools stay on the per-repo schema without advertising branch. --------- Co-authored-by: svjack Co-authored-by: Gergő Magyar Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- README.md | 6 +- gitnexus-cursor-integration/README.md | 2 +- gitnexus/README.md | 6 +- gitnexus/bench/mcp-tools-list/baselines.json | 4 +- gitnexus/src/mcp/local/local-backend.ts | 209 ++++++++++++++ gitnexus/src/mcp/read-only-policy.ts | 2 + gitnexus/src/mcp/server.ts | 7 +- gitnexus/src/mcp/tools.ts | 102 ++++++- gitnexus/src/server/grep-params.ts | 3 + gitnexus/test/unit/mcp-read-file-grep.test.ts | 263 ++++++++++++++++++ gitnexus/test/unit/mcp-read-only.test.ts | 2 + gitnexus/test/unit/server.test.ts | 20 ++ gitnexus/test/unit/tools.test.ts | 23 +- 13 files changed, 634 insertions(+), 15 deletions(-) create mode 100644 gitnexus/test/unit/mcp-read-file-grep.test.ts diff --git a/README.md b/README.md index a5e9862b0..bfcca5117 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ flowchart TB ## What Your AI Agent Gets -### 17 MCP tools (15 per-repo + 2 group) +### 19 MCP tools (17 per-repo + 2 group) | Tool | What It Does | | ---------------- | ---------------------------------------------------------------------- | @@ -177,10 +177,12 @@ flowchart TB | `api_impact` | Pre-change impact report for an API route handler | | `explain` | Explain persisted taint findings (source→sink flows, `--pdg` indexes) | | `pdg_query` | Query control/data dependence at statement level (`--pdg` indexes) | +| `read_file` | Read a checkout file (optional 0-indexed slice; `maxLines` cap) | +| `grep` | Regex search of the working tree for indexed files (1-based hits) | | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> Per-repo read-only tools take an optional `repo` parameter. Omit it when only one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout; otherwise pass it explicitly. Mutating tools require `repo` when multiple repos are indexed and no MCP default exists. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree — switching branches and re-running `gitnexus analyze` updates it incrementally. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> Per-repo read-only tools take an optional `repo` parameter. Omit it when only one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout; otherwise pass it explicitly. Mutating tools require `repo` when multiple repos are indexed and no MCP default exists. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`, except `read_file` and `grep`, which read the checkout and do not accept `branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree — switching branches and re-running `gitnexus analyze` updates it incrementally. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ### Resources for instant context diff --git a/gitnexus-cursor-integration/README.md b/gitnexus-cursor-integration/README.md index 0d044aaa9..0f8d9b119 100644 --- a/gitnexus-cursor-integration/README.md +++ b/gitnexus-cursor-integration/README.md @@ -8,7 +8,7 @@ Static config that adds GitNexus knowledge-graph augmentation and skill files to | Layer | What it does | How it's installed | | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------- | -| **MCP** | `gitnexus` MCP server with 17 tools (`query`, `context`, `impact`, `detect_changes`, `rename`, …) | `npx gitnexus setup` writes `~/.cursor/mcp.json` automatically. | +| **MCP** | `gitnexus` MCP server with 19 tools (`query`, `context`, `impact`, `detect_changes`, `rename`, …) | `npx gitnexus setup` writes `~/.cursor/mcp.json` automatically. | | **Skills** | All bundled markdown skills (`/gitnexus-exploring`, `/gitnexus-debugging`, `/gitnexus-impact-analysis`, `/gitnexus-refactoring`, `/gitnexus-guide`, `/gitnexus-cli`, `/gitnexus-review`, `/gitnexus-plan`, `/gitnexus-work`, `/gitnexus-lfg`, `/gitnexus-pdg-query`, `/gitnexus-taint-analysis`) | `npx gitnexus setup` copies them to `~/.cursor/skills/gitnexus/`. | | **Hooks** _(this README)_ | `postToolUse` hook that enriches `Shell` / `Read` / `Grep` tool calls with graph context — same augmentation Claude Code gets | **Manual** — copy the files described below into your project's `.cursor/`. | diff --git a/gitnexus/README.md b/gitnexus/README.md index 1c14777e7..eff30e026 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -210,7 +210,7 @@ Note that the bundled Graphology path is no longer the slow option it once was: ## MCP Tools -Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: +Your AI agent gets **19 tools** (17 per-repo + 2 group) automatically: | Tool | What It Does | | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | @@ -229,10 +229,12 @@ Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: | `api_impact` | Pre-change impact report for an API route handler | | `explain` | Explain persisted taint findings (source→sink flows, `--pdg` indexes) | | `pdg_query` | Query control/data dependence at statement level (`--pdg` indexes) | +| `read_file` | Read a checkout file (optional 0-indexed slice; `maxLines` cap) | +| `grep` | Regex search of the working tree for indexed files (1-based hits; optional `caseSensitive` / `literal`) | | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> Read-only tools can omit `repo` when one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout. Otherwise—and for mutating tools with multiple indexed repos and no MCP default—specify it explicitly: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`; omitting it queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> Read-only tools can omit `repo` when one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout. Otherwise—and for mutating tools with multiple indexed repos and no MCP default—specify it explicitly: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`, except `read_file` and `grep`, which read the checkout and do not accept `branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ## MCP Resources diff --git a/gitnexus/bench/mcp-tools-list/baselines.json b/gitnexus/bench/mcp-tools-list/baselines.json index 4c055f99d..2f5b405a4 100644 --- a/gitnexus/bench/mcp-tools-list/baselines.json +++ b/gitnexus/bench/mcp-tools-list/baselines.json @@ -8,8 +8,8 @@ "list_repos": 200, "_shape_note": "THE FLOOR. Without these three, every ratio below is a ceiling over nothing. listTools_vs_listRepos_ratio only asserts something while the corpus still pays N parallel rev-list processes. Shrink it to three happy-path rows and both arms are cheap; the ratio still passes, asserting a property the corpus no longer has.", - "tools_listed": 17, - "_tools_note": "Exact GITNEXUS_TOOLS roster size returned by client.listTools(). A schema path that throws, filters, or returns [] still looks fast on the ratio arm.", + "tools_listed": 19, + "_tools_note": "Exact GITNEXUS_TOOLS roster size returned by client.listTools(). 19 after read_file and grep were added as the MCP twins of GET /api/file and GET /api/grep (was 17). GITNEXUS_TOOLS.length and listTools must move together; a schema path that throws, filters, or returns [] still looks fast on the ratio arm.", "schema_read_only_requires_repo": true, "schema_mutating_requires_repo": true, diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 1c5dec3fe..5396cd497 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -46,6 +46,8 @@ import { getGitRoot, } from '../../storage/git.js'; import { realpathSync } from 'fs'; +import { parseGrepQuery, GREP_TIME_BUDGET_MS } from '../../server/grep-params.js'; +import { runGrepScanInWorker } from '../../server/grep-scan.js'; import { listRegisteredRepos, canonicalizePath, @@ -130,6 +132,8 @@ import { QUERY_MAX_LIMIT, QUERY_MAX_MAX_SYMBOLS, CONTEXT_CHAIN_MAX_DEPTH, + CHECKOUT_SOURCE_TOOLS, + READ_FILE_DEFAULT_MAX_LINES, } from '../tools.js'; import { foldNumericToolArgumentAliases } from '../tool-arguments.js'; import { findImportCycles, IMPORT_CYCLE_LIMIT } from '../../core/graph/import-cycles.js'; @@ -2877,6 +2881,15 @@ export class LocalBackend { return this.callToolAtGroupRepo(method, p); } + // These tools read the checkout, not a pinned index. `branch` would still + // rewrite lbugPath/lastCommit and withToolStaleness would label checkout + // bytes with that pin. Reject before selectToolRepository applies it. + if (CHECKOUT_SOURCE_TOOLS.has(method) && p.branch !== undefined && p.branch !== '') { + return { + error: `${method} follows the checked-out working tree and does not accept "branch". Omit it.`, + }; + } + // Resolve repo from optional param (re-reads registry on miss). An optional // `branch` param scopes the resolved handle to that branch's index (#2106). const repo = await this.selectToolRepository( @@ -2926,6 +2939,10 @@ export class LocalBackend { return this.apiImpact(repo, p); case 'trace': return this.trace(repo, p); + case 'read_file': + return this.withToolStaleness(repo, await this.readFile(repo, p)); + case 'grep': + return this.withToolStaleness(repo, await this.grep(repo, p)); default: throw new Error(`Unknown tool: ${method}`); } @@ -9452,6 +9469,198 @@ export class LocalBackend { return result; } + /** + * Same predicate as HTTP `getSourceAvailability`: full retention plus a live + * checkout. Meta is the graph directory so a published shared-store commit + * still carries `contentRetention`. Wording matches HTTP 410. + */ + private async fullSourceUnavailable(repo: RepoHandle): Promise<{ + error: string; + code: 'source-unavailable'; + reason: 'content-retention' | 'checkout-missing'; + } | null> { + const meta = await loadMeta(path.dirname(repo.lbugPath)); + const retention = contentRetentionFromMeta(meta); + const checkoutIsDir = retention === 'full' ? await checkoutIsDirectory(repo.repoPath) : false; + if (isFullSourceAvailable(retention, checkoutIsDir)) return null; + const reason = retention !== 'full' ? 'content-retention' : 'checkout-missing'; + const because = reason === 'content-retention' ? 'content retention' : 'source checkout'; + return { + error: `Full source is unavailable because the ${because} is unavailable.`, + code: 'source-unavailable', + reason, + }; + } + + /** + * MCP read_file — repo-contained checkout read with an optional 0-indexed + * line slice. The realpath re-check matches GET /api/file. The lexical + * barrier stays inline for CodeQL and is narrowed to the `..` segment, so + * a file named `..config` is not a traversal. ENOENT is file-not-found + * only after the checkout directory is known to exist. + */ + private async readFile( + repo: RepoHandle, + params: { path?: unknown; startLine?: unknown; endLine?: unknown; maxLines?: unknown }, + ): Promise { + const rawPath = params?.path; + if (typeof rawPath !== 'string' || rawPath === '') { + return { error: 'Missing required argument "path" (repo-relative file path).' }; + } + const unavailable = await this.fullSourceUnavailable(repo); + if (unavailable) return unavailable; + const toInteger = (v: unknown): number | undefined => + typeof v === 'number' && Number.isFinite(v) ? Math.trunc(v) : undefined; + const startLine = toInteger(params?.startLine); + const endLine = toInteger(params?.endLine); + if (endLine !== undefined && startLine === undefined) { + return { error: '"endLine" requires "startLine".' }; + } + const repoRoot = path.resolve(repo.repoPath); + const fullPath = path.resolve(repoRoot, rawPath); + const fullRel = path.relative(repoRoot, fullPath); + // `startsWith('..')` is the CodeQL path-injection sanitizer. Narrow it to + // the `..` segment so a repo file named `..config` is not a traversal. + if ( + path.isAbsolute(fullRel) || + (fullRel.startsWith('..') && (fullRel === '..' || fullRel.startsWith(`..${path.sep}`))) + ) { + return { error: 'Path traversal denied.' }; + } + let realRoot: string; + let realFull: string; + try { + [realRoot, realFull] = await Promise.all([fs.realpath(repoRoot), fs.realpath(fullPath)]); + } catch (err: any) { + if (err?.code === 'ENOENT') return { error: `File not found: ${rawPath}` }; + throw err; + } + const realRel = path.relative(realRoot, realFull); + if (realRel === '..' || realRel.startsWith(`..${path.sep}`) || path.isAbsolute(realRel)) { + return { error: 'Path traversal denied.' }; + } + const raw = await fs.readFile(realFull, 'utf-8'); + const lines = raw.split('\n'); + if (startLine !== undefined) { + if (endLine !== undefined && endLine < 0) { + return { error: '"endLine" must be an integer >= 0.' }; + } + const start = Math.max(0, startLine); + const end = endLine !== undefined ? Math.min(lines.length, endLine + 1) : lines.length; + return { + path: fullRel, + content: lines.slice(start, end).join('\n'), + startLine: start, + endLine: end - 1, + totalLines: lines.length, + }; + } + const requestedMaxLines = toInteger(params?.maxLines); + if (requestedMaxLines !== undefined && requestedMaxLines < 0) { + return { error: '"maxLines" must be an integer >= 0 (0 = no cap).' }; + } + const maxLines = requestedMaxLines ?? READ_FILE_DEFAULT_MAX_LINES; + if (maxLines > 0 && lines.length > maxLines) { + return { + path: fullRel, + content: lines.slice(0, maxLines).join('\n'), + startLine: 0, + endLine: maxLines - 1, + totalLines: lines.length, + truncated: true, + suggestion: 'Re-issue with startLine/endLine for the window you need.', + }; + } + return { path: fullRel, content: raw, totalLines: lines.length }; + } + + /** + * MCP grep — HTTP GET /api/grep twin. The file list is indexed File nodes + * that still have content; bytes come from the live checkout (same worker). + * Fails like HTTP 410 when full source is unavailable instead of returning + * an empty hit list. + */ + private async grep( + repo: RepoHandle, + params: { + pattern?: unknown; + fileFilter?: unknown; + limit?: unknown; + caseSensitive?: unknown; + literal?: unknown; + }, + ): Promise { + const unavailable = await this.fullSourceUnavailable(repo); + if (unavailable) return unavailable; + await this.ensureInitialized(repo); + let parsed; + try { + parsed = parseGrepQuery({ + pattern: params?.pattern, + fileFilter: params?.fileFilter, + limit: params?.limit, + caseSensitive: params?.caseSensitive, + literal: params?.literal, + }); + } catch (err: any) { + return { error: err?.message || 'Invalid grep query.' }; + } + const fileRows: Array<{ filePath?: string }> = await executeQuery( + repo.lbugPath, + `MATCH (n:File) WHERE n.content IS NOT NULL RETURN n.filePath AS filePath`, + ); + const filePaths: string[] = []; + for (const row of fileRows) { + const filePath: string = row.filePath || ''; + if (parsed.fileFilter && !filePath.toLowerCase().includes(parsed.fileFilter)) continue; + filePaths.push(filePath); + } + // The shared scanner only checks a lexical prefix, then readFile follows + // symlinks. Drop paths whose realpath leaves the checkout, same as read_file. + const repoRoot = path.resolve(repo.repoPath); + const realRoot = await fs.realpath(repoRoot); + const containedPaths: string[] = []; + for (const filePath of filePaths) { + const fullPath = path.resolve(repoRoot, filePath); + const fullRel = path.relative(repoRoot, fullPath); + if ( + path.isAbsolute(fullRel) || + (fullRel.startsWith('..') && (fullRel === '..' || fullRel.startsWith(`..${path.sep}`))) + ) { + continue; + } + let realFull: string; + try { + realFull = await fs.realpath(fullPath); + } catch { + continue; + } + const realRel = path.relative(realRoot, realFull); + if (realRel === '..' || realRel.startsWith(`..${path.sep}`) || path.isAbsolute(realRel)) { + continue; + } + containedPaths.push(filePath); + } + const { results, timedOut } = await runGrepScanInWorker({ + repoRoot, + filePaths: containedPaths, + pattern: parsed.regex.source, + flags: parsed.regex.flags, + limit: parsed.limit, + deadlineMs: Date.now() + GREP_TIME_BUDGET_MS, + }); + return { + results, + ...(timedOut ? { timedOut: true as const } : {}), + ...(timedOut + ? { + suggestion: + 'Wall-clock budget expired — re-issue narrower (fileFilter or a tighter pattern).', + } + : {}), + }; + } + private async routeMap(repo: RepoHandle, params: { route?: string }): Promise { await this.ensureInitialized(repo); diff --git a/gitnexus/src/mcp/read-only-policy.ts b/gitnexus/src/mcp/read-only-policy.ts index 51fc21de1..24c550cca 100644 --- a/gitnexus/src/mcp/read-only-policy.ts +++ b/gitnexus/src/mcp/read-only-policy.ts @@ -6,6 +6,8 @@ export const MCP_READ_ONLY_TOOLS = new Set([ 'list_repos', 'query', 'context', + 'read_file', + 'grep', 'detect_changes', 'check', 'impact', diff --git a/gitnexus/src/mcp/server.ts b/gitnexus/src/mcp/server.ts index d2353b1cc..6bfb6d478 100644 --- a/gitnexus/src/mcp/server.ts +++ b/gitnexus/src/mcp/server.ts @@ -7,7 +7,7 @@ * * Supports multiple indexed repositories via the global registry. * - * Tools: list_repos, query, cypher, context, impact, detect_changes, rename + * Tools: list_repos, query, cypher, context, read_file, grep, impact, detect_changes, rename * Resources: repos, repo/{name}/context, repo/{name}/clusters, ... */ @@ -80,6 +80,11 @@ function getNextStepHint(toolName: string, args: Record | undefined case 'cypher': return `\n\n---\n**Next:** To explore a result symbol, use context({name: ""${repoParam}}). For schema reference, READ gitnexus://repo/${repoPath}/schema.`; + case 'read_file': + return `\n\n---\n**Next:** To pin a symbol seen in the file, use context({name: ""${repoParam}}). To find other occurrences, use grep({pattern: ""${repoParam}}).`; + + case 'grep': + return `\n\n---\n**Next:** Read the hit window with read_file({path: ""${repoParam}, startLine: , endLine: }). Grep line is 1-based; read_file is 0-based. Or pin the symbol with context({name: ""${repoParam}}).`; // Legacy tool names — still return useful hints case 'search': return `\n\n---\n**Next:** To understand a result in context, use context({name: ""${repoParam}}).`; diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 2bdda85be..02b1bacd1 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -64,6 +64,9 @@ const DESTRUCTIVE_TOOL_ANNOTATIONS: ToolAnnotations = { export const LIST_REPOS_DEFAULT_LIMIT = 50; export const LIST_REPOS_MAX_LIMIT = 200; +/** Whole-file `read_file` cap. The schema default and the handler fallback share this. */ +export const READ_FILE_DEFAULT_MAX_LINES = 2000; + /** * Pagination bounds for the `explain` tool (#2083 M3 U6). Findings are sparse * and capped per function at analyze time, but a large repo can still @@ -1018,6 +1021,94 @@ DESTINATION TRACE (cross-repo): for an "@groupName" trace, OMIT to/to_uid/to_fil required: [], }, }, + { + name: 'read_file', + description: `Read a file from the repository checkout, optionally sliced to a 0-indexed line range. +Returns checkout bytes plus totalLines and the slice bounds. The realpath re-check matches HTTP GET /api/file. The lexical barrier is the CodeQL \`startsWith('..')\` form narrowed to the \`..\` segment, so a file named \`..config\` stays readable. A whole-file read is capped by maxLines (default ${READ_FILE_DEFAULT_MAX_LINES}, 0 = no cap), which is stricter than the uncapped HTTP body. + +WHEN TO USE: After query()/cypher()/context() gave you a filePath (or file:line), read the surrounding source: header context (open/variable/import lines), a full declaration, or any line window. Prefer context({name, include_content: true}) when you already have the symbol — it returns the symbol span plus call edges in one call. +AFTER THIS: Use the read text to ground signatures verbatim; never invent names from memory. + +Paths that escape the repository are refused. A missing file returns a not-found error only when the checkout directory exists. When full source is unavailable (content retention is not "full", or the checkout directory is gone), the result is code "source-unavailable" — not an empty body and not "file not found". This tool reads the checkout and does not accept branch.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, + inputSchema: { + type: 'object', + properties: { + path: { + type: 'string', + description: + 'Repository-contained file path (e.g. "Mathlib/Analysis/SpecificLimits/Basic.lean"). Paths that escape the repository, including ".." escapes, are refused.', + }, + startLine: { + type: 'integer', + description: 'Optional 0-indexed first line of the slice (inclusive).', + minimum: 0, + }, + endLine: { + type: 'integer', + description: 'Optional 0-indexed last line of the slice (inclusive). Requires startLine.', + minimum: 0, + }, + maxLines: { + type: 'integer', + description: `Maximum lines returned for a whole-file read (default ${READ_FILE_DEFAULT_MAX_LINES}, 0 = no cap). Ignored when startLine is set. Negative values are rejected.`, + default: READ_FILE_DEFAULT_MAX_LINES, + minimum: 0, + }, + repo: { + type: 'string', + description: `Indexed repository name or path. ${CWD_AWARE_REPO_OMISSION}`, + }, + }, + required: ['path'], + }, + }, + { + name: 'grep', + description: `Regex search of the live checkout for files the index retained — the MCP twin of HTTP GET /api/grep. +The file list is indexed File nodes that still have content. Bytes are read from the working tree, so edits since the last analyze are visible. Hits are 1-based. read_file startLine/endLine are 0-based. + +WHEN TO USE: Only after graph tools came back empty or ambiguous — exact-name pinning, docstring fallback, or literal tokens the index does not model (e.g. tactic names inside proof bodies, notation). Graph first (query/context/cypher); grep is the offline-capable fallback, never the default. +AFTER THIS: Read the matching line with read_file({path, startLine: hit.line - 1, endLine: hit.line - 1}) or pin the symbol with context({name}). + +Optional caseSensitive and literal match HTTP /api/grep (default: case-insensitive regex). When full source is unavailable the result is code "source-unavailable", not an empty hit list. This tool reads the checkout and does not accept branch. Results carry timedOut: true when the wall-clock budget expired first — re-issue narrower (fileFilter or a tighter pattern).`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, + inputSchema: { + type: 'object', + properties: { + pattern: { + type: 'string', + description: 'Regex pattern (max 200 chars) matched against file content lines.', + }, + fileFilter: { + type: 'string', + description: 'Optional case-insensitive substring filter on file paths.', + }, + limit: { + type: 'number', + description: 'Maximum hits returned (default 50, max 200).', + default: 50, + minimum: 1, + maximum: 200, + }, + caseSensitive: { + type: 'boolean', + description: + 'Optional. When true, match case. Default is case-insensitive, matching HTTP /api/grep.', + }, + literal: { + type: 'boolean', + description: + 'Optional. When true, treat pattern as a literal substring (escaped), matching HTTP /api/grep literal=1. Default is a regex.', + }, + repo: { + type: 'string', + description: `Indexed repository name or path. ${CWD_AWARE_REPO_OMISSION}`, + }, + }, + required: ['pattern'], + }, + }, ]; /** @@ -1025,9 +1116,16 @@ DESTINATION TRACE (cross-repo): for an "@groupName" trace, OMIT to/to_uid/to_fil * of truth: the schema property is injected here so it cannot drift from the * server-side default in `local-backend.ts` (`resolveRepo(repo, branch)`). * `list_repos` and the `group_*` tools are intentionally excluded — they are - * not single-repo, single-branch operations. + * not single-repo, single-branch operations. `read_file` and `grep` are in + * this set so `repo` stays required with the other per-repo tools, and the + * loop below skips `branch` for `CHECKOUT_SOURCE_TOOLS` — a pin would label + * checkout bytes with another commit. */ +export const CHECKOUT_SOURCE_TOOLS = new Set(['read_file', 'grep']); + export const REPO_SCOPED_TOOLS = new Set([ + 'read_file', + 'grep', 'query', 'cypher', 'context', @@ -1056,6 +1154,8 @@ for (const tool of GITNEXUS_TOOLS) { // advertising the aliases instead would re-break Claude Code on `query`. tool.inputSchema.additionalProperties = false; if (!REPO_SCOPED_TOOLS.has(tool.name)) continue; + // Checkout reads follow the working tree. Do not advertise `branch`. + if (CHECKOUT_SOURCE_TOOLS.has(tool.name)) continue; if (tool.inputSchema.properties.branch) continue; // Optional — `required` is left unchanged so omitting `branch` keeps today's // workspace-index behavior. Ignored in group mode (repo starts "@"). diff --git a/gitnexus/src/server/grep-params.ts b/gitnexus/src/server/grep-params.ts index 4707b5b96..d5310a546 100644 --- a/gitnexus/src/server/grep-params.ts +++ b/gitnexus/src/server/grep-params.ts @@ -40,6 +40,9 @@ export interface ParsedGrepQuery { } const isFlagTrue = (value: unknown, name: string): boolean => { + // MCP passes booleans. HTTP query strings stay '1' / 'true'. Null and + // undefined stay an empty string so an omitted flag is still false. + if (typeof value === 'boolean') return value; const s = assertString(value ?? '', name).toLowerCase(); return s === '1' || s === 'true'; }; diff --git a/gitnexus/test/unit/mcp-read-file-grep.test.ts b/gitnexus/test/unit/mcp-read-file-grep.test.ts new file mode 100644 index 000000000..3f9186011 --- /dev/null +++ b/gitnexus/test/unit/mcp-read-file-grep.test.ts @@ -0,0 +1,263 @@ +/** + * Handler tests for MCP read_file and grep. + * + * Registry tests only count tool names. These call LocalBackend.callTool + * against a temp checkout and a mocked File-node query, with the real grep + * worker, so retention, containment, the 1-based/0-based handoff, and the + * checkout-vs-pin contract are actually executed. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +const { lbugMocks } = vi.hoisted(() => ({ + lbugMocks: { + initLbug: vi.fn().mockResolvedValue(undefined), + executeQuery: vi.fn().mockResolvedValue([]), + executeParameterized: vi.fn().mockResolvedValue([]), + ensureVectorExtension: vi.fn().mockResolvedValue(true), + closeLbug: vi.fn().mockResolvedValue(undefined), + isLbugReady: vi.fn().mockReturnValue(true), + }, +})); + +vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + listRegisteredRepos: vi.fn().mockResolvedValue([]), + }; +}); + +vi.mock('../../src/core/git-staleness.js', () => ({ + checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkStalenessAsync: vi.fn().mockResolvedValue({ isStale: false, commitsBehind: 0 }), + checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), +})); + +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { executeQuery } from '../../src/mcp/core/lbug-adapter.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; + +const NAME = 'checkout-src'; +const INDEXED_AT = '2024-06-01T12:00:00Z'; + +const dirs: string[] = []; + +const writeMeta = async (storage: string, contentRetention: 'full' | 'symbol' | 'none') => { + await fs.writeFile( + path.join(storage, 'gitnexus.json'), + JSON.stringify({ contentRetention, indexedAt: INDEXED_AT }), + 'utf-8', + ); +}; + +describe('LocalBackend read_file and grep', () => { + let root: string; + let storage: string; + let backend: LocalBackend; + let symlinks = false; + + beforeEach(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), 'gnx-mcp-src-')); + dirs.push(root); + storage = path.join(root, '.gitnexus'); + await fs.mkdir(path.join(storage, 'lbug'), { recursive: true }); + await writeMeta(storage, 'full'); + await fs.mkdir(path.join(root, 'src'), { recursive: true }); + await fs.writeFile(path.join(root, 'src', 'auth.ts'), 'signOrder()\nnoop\n', 'utf-8'); + await fs.writeFile(path.join(root, 'literals.txt'), 'a.b\naxb\n', 'utf-8'); + await fs.writeFile(path.join(root, 'case.txt'), 'Token\ntoken\n', 'utf-8'); + await fs.writeFile(path.join(root, '..config'), 'dotfile\n', 'utf-8'); + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'gnx-mcp-out-')); + dirs.push(outside); + await fs.writeFile(path.join(outside, 'secret.txt'), 'top secret\n', 'utf-8'); + try { + await fs.symlink(path.join(outside, 'secret.txt'), path.join(root, 'escape.txt')); + symlinks = true; + } catch { + symlinks = false; + } + + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: NAME, + path: root, + storagePath: storage, + indexedAt: INDEXED_AT, + lastCommit: 'abc123', + } as Awaited>[number], + ]); + vi.mocked(executeQuery).mockResolvedValue([ + { filePath: 'src/auth.ts' }, + { filePath: 'literals.txt' }, + { filePath: 'case.txt' }, + { filePath: '..config' }, + ]); + backend = new LocalBackend(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + const read = (params: Record) => + backend.callTool('read_file', { repo: NAME, ...params }); + const grep = (params: Record) => + backend.callTool('grep', { repo: NAME, ...params }); + + it('slices 0-based and maps a 1-based grep hit onto that window', async () => { + const hits = await grep({ pattern: 'signOrder' }); + expect(hits.results).toEqual([ + expect.objectContaining({ filePath: 'src/auth.ts', line: 1, text: 'signOrder()' }), + ]); + const hitLine = hits.results[0].line as number; + const window = await read({ + path: 'src/auth.ts', + startLine: hitLine - 1, + endLine: hitLine - 1, + }); + expect(window.content).toBe('signOrder()'); + const offByOne = await read({ path: 'src/auth.ts', startLine: hitLine, endLine: hitLine }); + expect(offByOne.content).toBe('noop'); + }); + + it('rejects endLine without startLine instead of returning the whole file', async () => { + const out = await read({ path: 'src/auth.ts', endLine: 0 }); + expect(out).toEqual({ error: '"endLine" requires "startLine".' }); + }); + + it('caps a whole-file read at maxLines', async () => { + const out = await read({ path: 'src/auth.ts', maxLines: 1 }); + expect(out.truncated).toBe(true); + expect(out.content).toBe('signOrder()'); + expect(out.totalLines).toBe(3); + }); + + it('rejects a negative maxLines and reports truncated integer slice bounds', async () => { + const negative = await read({ path: 'src/auth.ts', maxLines: -1 }); + expect(negative).toEqual({ error: '"maxLines" must be an integer >= 0 (0 = no cap).' }); + const fractional = await read({ path: 'src/auth.ts', startLine: 0.5, endLine: 0.5 }); + expect(fractional.content).toBe('signOrder()'); + expect(fractional.startLine).toBe(0); + expect(fractional.endLine).toBe(0); + }); + + it('rejects a negative endLine instead of slicing from the end of the file', async () => { + const out = await read({ path: 'src/auth.ts', startLine: 0, endLine: -2 }); + expect(out).toEqual({ error: '"endLine" must be an integer >= 0.' }); + expect(out.content).toBeUndefined(); + }); + + it('reads a contained absolute path and a file whose name starts with ..', async () => { + const absolute = await read({ path: path.join(root, 'src', 'auth.ts') }); + expect(absolute.content).toContain('signOrder()'); + const dot = await read({ path: '..config' }); + expect(dot.content).toBe('dotfile\n'); + }); + + it('does not grep through a symlink that leaves the checkout', async () => { + if (!symlinks) return; + vi.mocked(executeQuery).mockResolvedValue([ + { filePath: 'escape.txt' }, + { filePath: 'src/auth.ts' }, + ]); + const leaked = await grep({ pattern: 'secret' }); + expect(leaked.results).toEqual([]); + const kept = await grep({ pattern: 'signOrder' }); + expect(kept.results).toEqual([ + expect.objectContaining({ filePath: 'src/auth.ts', line: 1, text: 'signOrder()' }), + ]); + }); + + it('refuses traversal, a symlink out of the repo, and a missing file', async () => { + expect((await read({ path: '../secret.txt' })).error).toBe('Path traversal denied.'); + expect((await read({ path: '/etc/passwd' })).error).toBe('Path traversal denied.'); + if (symlinks) { + expect((await read({ path: 'escape.txt' })).error).toBe('Path traversal denied.'); + } + expect((await read({ path: 'missing.ts' })).error).toBe('File not found: missing.ts'); + }); + + it('errors when retention is not full, instead of an empty grep or a file body', async () => { + await writeMeta(storage, 'symbol'); + vi.mocked(executeQuery).mockClear(); + const file = await read({ path: 'src/auth.ts' }); + const hits = await grep({ pattern: 'signOrder' }); + expect(file).toMatchObject({ code: 'source-unavailable', reason: 'content-retention' }); + expect(file.content).toBeUndefined(); + expect(hits).toMatchObject({ code: 'source-unavailable', reason: 'content-retention' }); + expect(hits.results).toBeUndefined(); + expect(executeQuery).not.toHaveBeenCalled(); + }); + + it('errors when the checkout directory is gone, including for grep', async () => { + const orphanStorage = await fs.mkdtemp(path.join(os.tmpdir(), 'gnx-mcp-meta-')); + dirs.push(orphanStorage); + await fs.mkdir(path.join(orphanStorage, 'lbug'), { recursive: true }); + await writeMeta(orphanStorage, 'full'); + const missingCheckout = path.join(orphanStorage, 'gone'); + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'gone', + path: missingCheckout, + storagePath: orphanStorage, + indexedAt: INDEXED_AT, + lastCommit: 'abc123', + } as Awaited>[number], + ]); + const other = new LocalBackend(); + const file = await other.callTool('read_file', { repo: 'gone', path: 'src/auth.ts' }); + const hits = await other.callTool('grep', { repo: 'gone', pattern: 'signOrder' }); + expect(file).toMatchObject({ code: 'source-unavailable', reason: 'checkout-missing' }); + expect(file.error).not.toMatch(/File not found/); + expect(hits).toMatchObject({ code: 'source-unavailable', reason: 'checkout-missing' }); + expect(hits.results).toBeUndefined(); + }); + + it('sees checkout edits after indexing and honors literal plus caseSensitive', async () => { + await fs.writeFile(path.join(root, 'src', 'auth.ts'), 'TOKEN_MAIN\n', 'utf-8'); + const edited = await grep({ pattern: 'TOKEN_MAIN' }); + expect(edited.results).toEqual([expect.objectContaining({ filePath: 'src/auth.ts', line: 1 })]); + const stale = await grep({ pattern: 'signOrder' }); + expect(stale.results).toEqual([]); + + const regex = await grep({ pattern: 'a.b', fileFilter: 'literals' }); + expect(regex.results.map((hit: { text: string }) => hit.text).sort()).toEqual(['a.b', 'axb']); + const literal = await grep({ pattern: 'a.b', literal: true, fileFilter: 'literals' }); + expect(literal.results).toEqual([expect.objectContaining({ text: 'a.b' })]); + + const insensitive = await grep({ pattern: 'Token', fileFilter: 'case.txt' }); + expect(insensitive.results).toHaveLength(2); + const sensitive = await grep({ pattern: 'Token', caseSensitive: true, fileFilter: 'case.txt' }); + expect(sensitive.results).toEqual([expect.objectContaining({ text: 'Token', line: 1 })]); + }); + + it('rejects branch before resolving a pinned index', async () => { + const file = await backend.callTool('read_file', { + path: 'src/auth.ts', + branch: 'release', + }); + const hits = await backend.callTool('grep', { pattern: 'signOrder', branch: 'release' }); + expect(file.error).toMatch(/does not accept "branch"/); + expect(file.content).toBeUndefined(); + expect(hits.error).toMatch(/does not accept "branch"/); + expect(hits.results).toBeUndefined(); + expect(executeQuery).not.toHaveBeenCalled(); + }); +}); + +afterEach(async () => { + await Promise.all(dirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))); +}); diff --git a/gitnexus/test/unit/mcp-read-only.test.ts b/gitnexus/test/unit/mcp-read-only.test.ts index e6831396b..fd27d93ef 100644 --- a/gitnexus/test/unit/mcp-read-only.test.ts +++ b/gitnexus/test/unit/mcp-read-only.test.ts @@ -10,10 +10,12 @@ const READ_ONLY_TOOLS = [ 'context', 'detect_changes', 'explain', + 'grep', 'impact', 'list_repos', 'pdg_query', 'query', + 'read_file', 'route_map', 'shape_check', 'tool_map', diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts index 544987e09..da5c37d90 100644 --- a/gitnexus/test/unit/server.test.ts +++ b/gitnexus/test/unit/server.test.ts @@ -131,6 +131,11 @@ describe('createMCPServer', () => { expect(query?.inputSchema.required).toContain('repo'); expect(listRepos?.inputSchema.required).not.toContain('repo'); + for (const name of ['read_file', 'grep'] as const) { + const tool = tools.tools.find((candidate) => candidate.name === name); + expect(tool?.inputSchema.required, name).toContain('repo'); + expect(tool?.inputSchema.properties, name).not.toHaveProperty('branch'); + } expect( GITNEXUS_TOOLS.find((tool) => tool.name === 'query')?.inputSchema.required, ).not.toContain('repo'); @@ -283,6 +288,21 @@ describe('getNextStepHint (via tool call response)', () => { ), ).toBe(true); }); + + it('grep hint converts the 1-based hit line to a 0-based read_file window', async () => { + const backend = createMockBackend({ + callTool: vi + .fn() + .mockResolvedValue({ results: [{ filePath: 'a.ts', line: 1, text: 'signOrder()' }] }), + }); + const { text } = await callToolThroughServer(backend, 'grep', { + pattern: 'signOrder', + repo: 'demo', + }); + expect(text).toContain('startLine: '); + expect(text).toContain('endLine: '); + expect(text).toContain('Grep line is 1-based; read_file is 0-based'); + }); }); describe('MCP output budgets', () => { diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 3cb464ef8..7d6000039 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -2,7 +2,7 @@ * Unit Tests: MCP Tool Definitions * * Tests: GITNEXUS_TOOLS from tools.ts - * - All 17 tools are defined (per-repo + group_list/group_sync) + * - All 19 tools are defined (per-repo + group_list/group_sync) * - Each tool has valid name, description, inputSchema * - Required fields are correct * - Optional repo parameter is present on tools that need it @@ -23,8 +23,8 @@ const MUTATING_TOOLS = new Set(['rename', 'group_sync']); const OPEN_WORLD_READ_ONLY_TOOLS = new Set(['query']); describe('GITNEXUS_TOOLS', () => { - it('exports all tools (8 base + 1 explain + 1 pdg_query + 3 route/tool/shape + 1 api_impact + 1 trace + 2 group)', () => { - expect(GITNEXUS_TOOLS).toHaveLength(17); + it('exports all tools (8 base + 1 explain + 1 pdg_query + 3 route/tool/shape + 1 api_impact + 1 trace + 2 group + read_file + grep)', () => { + expect(GITNEXUS_TOOLS).toHaveLength(19); }); it('contains all expected tool names', () => { @@ -43,6 +43,8 @@ describe('GITNEXUS_TOOLS', () => { 'pdg_query', 'api_impact', 'trace', + 'read_file', + 'grep', ]), ); }); @@ -354,10 +356,11 @@ describe('GITNEXUS_TOOLS', () => { } }); - it('per-repo tools have an optional branch scope param (#2106); group/list tools do not', () => { + it('per-repo tools have an optional branch scope param (#2106); group/list and checkout file tools do not', () => { + const noBranch = new Set(['list_repos', 'read_file', 'grep', ...GROUP_TOOLS]); for (const tool of GITNEXUS_TOOLS) { - if (tool.name === 'list_repos' || GROUP_TOOLS.has(tool.name)) { - expect(tool.inputSchema.properties.branch).toBeUndefined(); + if (noBranch.has(tool.name)) { + expect(tool.inputSchema.properties.branch, tool.name).toBeUndefined(); continue; } expect(tool.inputSchema.properties.branch, tool.name).toBeDefined(); @@ -367,6 +370,14 @@ describe('GITNEXUS_TOOLS', () => { } }); + it('grep advertises the HTTP caseSensitive and literal flags', () => { + const grep = GITNEXUS_TOOLS.find((tool) => tool.name === 'grep')!; + expect(grep.inputSchema.properties.caseSensitive.type).toBe('boolean'); + expect(grep.inputSchema.properties.literal.type).toBe('boolean'); + expect(grep.description).toContain('hit.line - 1'); + expect(grep.description).toMatch(/working tree/i); + }); + it('group tools without backend repo param omit repo property', () => { for (const name of ['group_list', 'group_sync'] as const) { const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!;