diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 7a89ae70..e47e07f8 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,7 +6,7 @@ }, "metadata": { "description": "GitHits plugins for Claude Code - code examples from global open source", - "version": "0.2.2" + "version": "0.2.3" }, "plugins": [ { diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/.plugin/plugin.json b/.plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/.plugin/plugin.json +++ b/.plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 3be1a2b6..fa4f38c9 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -8,12 +8,10 @@ The CLI exposes MCP tools that mirror the backend's MCP server. This document ex GitHits has two MCP server implementations: -- **Backend** (`githits-backend`) — Python/FastMCP, runs as a hosted service at `mcp.githits.com` -- **CLI** (`githits-cli`) — TypeScript/MCP SDK, runs locally via `githits mcp start` +- **Backend** (`githits-backend`) — Python/FastMCP, runs as a hosted service at `mcp.githits.com`. Surfaces only the example-search workflow (`get_example`, `search_language`, `feedback`). +- **CLI** (`githits-cli`) — TypeScript/MCP SDK, runs locally via `githits mcp start`. Surfaces the full set, including unified `search`, package intelligence (`pkg_*`), and code navigation (`code_*`). -Both expose the same tools with identical names, parameters, and descriptions. The backend handles auth, analytics, and AI orchestration server-side. The CLI delegates to the REST API, which is a simpler path that still benefits from the backend's processing pipeline. - -> **Tools must stay in sync with the backend.** When the backend adds or modifies a tool, the CLI must be updated to match. The tool names, parameter names, descriptions, and schema constraints should be identical across both implementations. +The two surfaces overlap on the example-search workflow only; for the overlapping tools the parameters and descriptions stay aligned. Tools that exist only on the CLI surface (code navigation, package intelligence, unified `search`) are not constrained by backend parity. ## Current Tools @@ -22,15 +20,15 @@ Both expose the same tools with identical names, parameters, and descriptions. T | `get_example` | `query`, `language?`, `license_mode?` | Search for canonical code examples. Returns markdown-formatted results. If `language` is omitted, the backend infers it from the query. | | `search_language` | `query` | Find supported programming language names before searching. | | `feedback` | `solution_id`, `accepted`, `feedback_text?` | Submit feedback on a search result to improve quality. | -| `search` | `query`, `target?`, `targets?`, `sources?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Omit `file_intent` to search across all intents; set it only when you want to narrow results, and note that some sources may ignore it and report that in `sourceStatus`. Prefer `sources:["symbol"]` for symbol-shaped unified search. Complete-by-default; set `allow_partial_results: true` to receive available partial hits while indexing continues. | +| `search` | `query`, `target?`, `targets?`, `sources?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?`, `format?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Omit `file_intent` to search across all intents; set it only when you want to narrow results, and note that some sources may ignore it and report that in `sourceStatus`. Prefer `sources:["symbol"]` for symbol-shaped unified search. Complete-by-default; set `allow_partial_results: true` to receive available partial hits while indexing continues. `format` defaults to `text-v1` for compact agent output; pass `format: "json"` for the structured envelope. | | `search_status` | `search_ref` | Check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results for a prior unified search. | | `pkg_info` | `registry`, `package_name` | Package overview: latest version, license, description, repository, downloads, GitHub metadata, install command, and known vulnerabilities. Always returns the latest published version. | | `pkg_vulns` | `registry`, `package_name`, `version?`, `min_severity?`, `include_withdrawn?` | Known vulnerabilities for a package on npm, PyPI, Hex, or Crates. Count summary, per-advisory OSV ID + severity + affected/fix ranges, and upgrade paths. Malware is surfaced in a disjoint bucket. | | `pkg_deps` | `registry`, `package_name`, `version?`, `lifecycle?`, `include_transitive?`, `include_importers?`, `max_depth?` | Direct runtime dependency list (each `{name, version, constraint}` — the backend resolves each constraint to a concrete version) plus, when the backend has them, structured groups for dev / peer / build / optional with registry-specific condition metadata (PyPI extras, Crates features). Optional transitive block with aggregate edge counts, the preprocessed install footprint as `{name, version}`, typed conflicts and circular-dependency cycles; opt into per-package importer provenance with `include_importers`. | | `pkg_changelog` | `registry?`, `package_name?`, `repo_url?`, `from_version?`, `to_version?`, `limit?`, `git_ref?`, `include_bodies?` | Release notes or changelog entries for a package or GitHub repo. Default latest mode returns the ten most recent entries; `from_version` switches to range mode (no count cap). Dual addressing (spec vs repo URL) mutually exclusive. Response always includes `source` (`"releases"` / `"changelog_file"` / `"hexdocs"`), `mode` (`"latest"` / `"range"`), and `entries: { count, items }` with full markdown bodies by default; set `include_bodies: false` for a lean version / date / URL timeline. | -| `code_files` | `target`, `path_prefix?`, `limit?`, `wait_timeout_ms?` | List files in an indexed dependency. Returns `{total, hasMore, files: [{path, name, language, fileType, byteSize}], resolution, indexedVersion}`. Dual addressing via `target.registry + target.package_name` (spec) or `target.repo_url + target.git_ref` (repo). `path_prefix` is a literal directory prefix — NOT a glob (`*.ts` won't match); glob / pattern filtering is an upstream enhancement. Emits an `INDEXING` error envelope when the dependency is being indexed on-demand — retry with a longer `wait_timeout_ms` or pick a version from `details.availableVersions`. | -| `code_read` | `target`, `path`, `start_line?`, `end_line?`, `wait_timeout_ms?` | Read a file from an indexed dependency. Default full file; use `start_line` / `end_line` for a bounded range. Binary files set `isBinary: true` and omit `content` — agents branch on the flag. On `NOT_FOUND` / `FILE_NOT_FOUND` call `code_files` to discover the actual path. | -| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. | +| `code_files` | `target`, `path_prefix?`, `limit?`, `wait_timeout_ms?`, `format?` | List files in an indexed dependency. Returns `{total, hasMore, files: [{path, name, language, fileType, byteSize}], resolution, indexedVersion}` in JSON mode. Dual addressing via `target.registry + target.package_name` (spec) or `target.repo_url + target.git_ref` (repo). `path_prefix` is a literal directory prefix — NOT a glob (`*.ts` won't match); glob / pattern filtering is an upstream enhancement. Emits an `INDEXING` error envelope when the dependency is being indexed on-demand — retry with a longer `wait_timeout_ms` or pick a version from `details.availableVersions`. `format` defaults to `text-v1` (paths-only listing); pass `format: "json"` for the structured envelope. | +| `code_read` | `target`, `path`, `start_line?`, `end_line?`, `wait_timeout_ms?` | Read a file from an indexed dependency. **MCP per-call span cap: 150 lines** — broader requests (or no range) are silently truncated to the first 150 lines from the caller's start, with a `hint` field explaining the cap and the original request. Use `start_line` / `end_line` to pick a focused window — typical 80-150 lines around a known symbol from `search` / `code_grep`. Binary files set `isBinary: true` and omit `content`. On `NOT_FOUND` / `FILE_NOT_FOUND` call `code_files` to discover the actual path. The cap is MCP-only; the CLI command `githits code read` honors arbitrary ranges. | +| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?`, `format?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. `format` defaults to `text-v1` (matches grouped by file, grep -A/-B notation for context); pass `format: "json"` for the structured envelope. | `search`, `search_status`, `pkg_info`, `pkg_vulns`, `pkg_deps`, `pkg_changelog`, `code_files`, `code_read`, and `code_grep` are only registered when the current token explicitly carries the `code_navigation` feature flag. The package/source service URL defaults to `https://pkgseer.dev` in the default production environment and can be overridden via `GITHITS_CODE_NAV_URL` for local development. @@ -112,7 +110,7 @@ These three indexing-gated tools share an addressing and lifecycle contract (doc **`code_files` envelope**: `{registry?|repoUrl?+gitRef?, total, hasMore, indexedVersion?, resolution?, files: [{path, name?, language?, fileType?, byteSize?}], hint?, filter?}`. `fileType` values preserve the service vocabulary (`CONFIG`, `SOURCE`, `DOC`, `TEST`). `total` is capped at returned count when `hasMore: true` — the terminal formatter renders `N+ files` in that case to avoid misleading users. `filter.pathPrefix` / `filter.limit` echo only when the caller supplied them explicitly; default limit (200) never round-trips. -**`code_read` envelope**: `{registry?|repoUrl?+gitRef?, path, language?, totalLines?, startLine?, endLine?, content?, isBinary?}`. `path` (not `filePath`) so the key matches `code_files.files[].path` and `code_grep.filter.path` when exact-file grep is used. Binary files set `isBinary: true` and **omit** `content` (not `null`); agents branch on the flag. +**`code_read` envelope**: `{registry?|repoUrl?+gitRef?, path, language?, totalLines?, startLine?, endLine?, content?, isBinary?, hint?}`. `path` (not `filePath`) so the key matches `code_files.files[].path` and `code_grep.filter.path` when exact-file grep is used. Binary files set `isBinary: true` and **omit** `content` (not `null`); agents branch on the flag. `hint` is emitted only when the MCP span cap actually truncated the response — see "code_read span cap" below. **`code_grep` envelope**: `{registry?|name?|repoUrl?+gitRef?, pattern, patternType?, caseSensitive?, matches: [{filePath, line, matchStartByte, matchEndByte, lineContent, contextBefore?, contextAfter?, fileContentHash?, fileIntent?, symbol?}], nextCursor?, hasMore, truncatedReason?, filesScanned, filesInScope, binaryFilesSkipped?, filesTooLargeSkipped?, totalMatches, uniqueFilesMatched, indexedVersion?, resolution?, filter?}`. Default-valued fields (`patternType: literal`, `caseSensitive: false`, zero skipped counters, `truncatedReason: none`) are omitted. `filter` echoes only explicit caller filters. Match entries carry `filePath` so grep output chains directly into `code_read`. @@ -139,14 +137,86 @@ All three code-navigation tools share the same indexing-retry contract. The stat **`FILE_NOT_FOUND` vs `NOT_FOUND`**: `code_read` / `code_grep` can hit "path doesn't resolve" errors when an exact path scope is invalid. The classifier is pre-wired to emit `FILE_NOT_FOUND` when the backend sends `extensions.code: "FILE_NOT_FOUND"`, but today the backend emits generic `NOT_FOUND` for both "package missing" and "path missing". The distinction is filed upstream. CLI terminal output for `code read` / `code grep` emits the hint "Use `code files` to list available paths." on both codes so users have an actionable next step regardless of classification. +**`code_read` span cap (MCP-only)**: real session traces showed agents requesting 300-600 line windows (and occasional unbounded full-file reads) which dominated context cost. The MCP surface caps each `code_read` call at `MCP_READ_MAX_SPAN` (150 lines, defined in `src/tools/read-file.ts`). The cap is enforced *before* the backend call — `deriveBoundedRange` clamps the request, so the service does not transfer bytes that will be discarded. + +The `hint` field is emitted only when the cap *actually truncated* the response — i.e., the returned range comes up short of available content. `shouldEmitCappedHint` (in `src/tools/read-file.ts`) suppresses the hint in three cases the agent doesn't need it: (a) the cap clamp didn't fire (caller's range was already within the cap); (b) the file fits within the cap, so the response is the whole file even though the request was clamped; (c) the returned range reaches end of file. Binary files always skip the hint. When emitted, the hint reads from `payload.startLine` / `endLine` / `totalLines` (the actual returned range, not the pre-clamp request) and includes the original request for the agent to learn from. The CLI command `githits code read` does not apply the cap; humans piping whole files to disk continue to work. + +## Text response format (`format: "text-v1"`) + +`search`, `code_files`, and `code_grep` accept a `format` parameter on the MCP surface. The default is `"text-v1"` — a compact line-oriented format that drops JSON scaffolding to stay lean in agent context. Programmatic callers (parity tests, scripts that parse responses) pass `format: "json"` explicitly. `"text"` is accepted as an alias for `"text-v1"` to keep agent prompts terse. + +**Why text-v1 default.** A 10-hit `search` JSON envelope runs 5–7 KB after compaction; the same hits in `text-v1` land around 3–4 KB. The savings come from dropped quoting, dropped key repetition, and dropped fields that an agent does not need at the per-call decision point (highlights byte offsets, repeated locator scaffolding). The token budget belongs to the agent's reasoning, not to JSON structure. + +**Format stability.** The text format is a public contract, locked with snapshot-style tests (`src/shared/unified-search-text.test.ts`, `src/shared/list-files-text.test.ts`). The `text-v1` version tag exists so we can evolve the format without silently breaking downstream parsers — future incompatible changes ship as `text-v2`. + +**ASCII-only.** Separators are ` | `; ellipsis is `...`; no box-drawing or Latin-1 punctuation. Tokenizer behavior for multi-byte UTF-8 varies across BPE variants, and the format runs into Claude, Codex CLI, OpenCode, Cline, Cursor, etc. — ASCII keeps it predictable. + +**Hit anatomy** (`search` text-v1): + +``` +search | hits | query="..." +[blank] +[1] + + + + +[blank] +[2] ... +[blank] +More hits available. Pass offset=N for the next page or limit=N to widen. +``` + +`` compacts to `code` / `symbol` / `docs` / `repo-docs`. `` is `path:start-end` (optionally followed by ` | qualifiedPath | kind`) for code/symbol hits; `pageId: ` for documentation pages; or `sourceUrl` as a last resort. The locator line is copy-pasteable as `code_read` arguments. + +**Listing anatomy** (`code_files` text-v1): + +``` +code_files | [+] paths | [path_prefix="..."] [limit=N] +[blank] + + +... +[blank] +More files available. Pass limit=N to widen or refine path_prefix. +``` + +`` is `:@` for spec addressing or `@` for repo addressing. Filter echoes appear in the header only when the caller supplied them explicitly (defaults never echo). + +**Grep anatomy** (`code_grep` text-v1): + +``` +code_grep | matches in files | pattern="..." [regex,case-sensitive] [filter echo] +[blank] + () + 142: matching line content + 287: another match +[blank] + () + 140- context-before line + 141- context-before line + 142: matching line + 143- context-after line +[blank] +[Truncated: limit. Pass narrower path/path_prefix/globs or increase max_matches.] +[More matches available. Pass cursor= for the next page.] +``` + +Standard grep -A/-B notation: `:` separator on match lines, `-` on context lines. Non-adjacent blocks within the same file are separated by `--`. The `()` after the file path is the per-file match count; the header sums across files. Header flags (`regex`, `case-sensitive`) appear only when the request used them. Filter echoes (`path_prefix=`, `exts=`, `max_matches=`, etc.) appear only when the caller supplied them. Match-line offsets, file content hashes, file intent, and symbol metadata are dropped in text mode — agents that need them can request `format: "json"`. + +**Errors in text mode.** `search` errors render as text in `text-v1` mode: `search | ERROR | code= [| retryable]\n` followed by an indented `details:` block when present. `code_files` and `code_grep` keep errors JSON-formatted in either mode for now — revisit if agent feedback warrants. + ## Server instructions The MCP server advertises a short, cross-tool orientation via the protocol's server-level `instructions` field. This is distinct from per-tool `description` text: instructions cover rationale, workflow glue, and decisions that span multiple tools, while per-tool descriptions remain the source of truth for arguments, output shape, and tool-specific constraints. `src/commands/mcp-instructions.ts` owns two sections: -- **Core block** — always loaded. Introduces GitHits and the `get_example` / `search_language` / `feedback` workflow. -- **Package-tools block** — appended when `isPackageToolsCapabilityOpen(deps)` is true. Contains a preamble plus one bullet per package tool whose backing service is actually wired, so half-open service configurations never advertise a tool that was not registered. +- **Core block** — always loaded. Introduces GitHits, expands trigger criteria to include comparative cross-OSS questions and "how does X actually implement this" archaeology, and walks through the `get_example` / `search_language` / `feedback` workflow. +- **Package-tools block** — appended when `isPackageToolsCapabilityOpen(deps)` is true. Contains a preamble plus one bullet per package tool whose backing service is actually wired (so half-open service configurations never advertise a tool that was not registered), plus three cross-tool tips emitted only when code navigation is wired: + - **Reference-first, content-second**: locate symbols and lines first, then read narrowly with `code_read` using `start_line` / `end_line` windows around the match. + - **Multi-turn discovery**: anticipate 3+ calls? Delegate to a sub-task / sub-agent and ask for a compact synthesis rather than pulling raw `code_read` / `code_files` output into the main conversation. + - **Tool-selection tip**: contrasts `get_example` vs unified `search` vs `code_grep`/`code_read`. `isPackageToolsCapabilityOpen` is the single source of truth for whether package tools should surface in the current session. Both `getMcpToolDefinitions` and `buildMcpInstructions` import it so tool registration and the instruction text cannot drift. diff --git a/gemini-extension.json b/gemini-extension.json index 6d91c623..3646628a 100644 --- a/gemini-extension.json +++ b/gemini-extension.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants.", "mcpServers": { "githits": { diff --git a/package.json b/package.json index a8912b4c..62ee0996 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "githits", "description": "CLI companion for GitHits - code examples from global open source for developers and AI assistants", - "version": "0.2.2", + "version": "0.2.3", "type": "module", "files": [ "dist", diff --git a/plugins/claude/.claude-plugin/plugin.json b/plugins/claude/.claude-plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/plugins/claude/.claude-plugin/plugin.json +++ b/plugins/claude/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/src/commands/mcp-instructions.test.ts b/src/commands/mcp-instructions.test.ts index da307635..cfc2229a 100644 --- a/src/commands/mcp-instructions.test.ts +++ b/src/commands/mcp-instructions.test.ts @@ -147,6 +147,37 @@ describe("buildMcpInstructions", () => { expect(instructions).toContain("`search_status`"); expect(instructions).toContain("allow_partial_results"); expect(instructions).toContain("partial hits"); + // Reference-first and multi-turn strategy tips appear when + // code-navigation is wired so agents do not pull raw source into + // the main conversation by default. + expect(instructions).toContain("reference-first"); + expect(instructions).toContain("Delegate multi-call work to a sub-agent"); + expect(instructions).toContain("inherently multi-call"); + // Delegation framing leads the gated section so it lands before + // the per-tool bullets. + const multiTurnIdx = instructions.indexOf( + "Delegate multi-call work to a sub-agent", + ); + const firstBulletIdx = instructions.indexOf("- `"); + expect(multiTurnIdx).toBeGreaterThan(0); + expect(firstBulletIdx).toBeGreaterThan(multiTurnIdx); + }); + + it("expands core trigger criteria to cover comparative cross-OSS questions", () => { + const deps = createTestDeps(); + const instructions = buildMcpInstructions(deps); + expect(instructions).toContain("comparative across OSS projects"); + expect(instructions).toContain("how a real codebase implements"); + }); + + it("does not surface the strategy tips when code-navigation is not wired", () => { + const deps = createTestDeps({ + codeNavigationCapability: "enabled", + packageIntelligenceService: createMockPackageIntelligenceService(), + }); + const instructions = buildMcpInstructions(deps); + expect(instructions).not.toContain("reference-first"); + expect(instructions).not.toContain("Delegate multi-call work"); }); it("keeps the core block first when both sections are present", () => { diff --git a/src/commands/mcp-instructions.ts b/src/commands/mcp-instructions.ts index 5377034f..e4bda816 100644 --- a/src/commands/mcp-instructions.ts +++ b/src/commands/mcp-instructions.ts @@ -15,7 +15,7 @@ import type { Dependencies } from "../container.js"; * current session. */ -const CORE_BLOCK = `GitHits surfaces verified, canonical code examples from global open source. Use it when you're stuck, the user is frustrated by repeated failed attempts, you need up-to-date API usage, or the user mentions GitHits. +const CORE_BLOCK = `GitHits surfaces verified, canonical code examples from global open source. Use it when you're stuck, the user is frustrated by repeated failed attempts, you need up-to-date API usage, the question is comparative across OSS projects (e.g. "how does X vs Y handle Z"), the answer requires reading how a real codebase implements a feature, or the user mentions GitHits. Workflow: call \`get_example\` with one focused question, optionally passing \`language\` when the desired language is known; call \`search_language\` first only if you need to force a language and the exact name is uncertain. Send \`feedback\` on the returned solution_id so quality improves. Each search addresses a single issue; reuse context from prior results before re-searching.`; @@ -42,22 +42,28 @@ const PKG_CHANGELOG_BULLET = "- `pkg_changelog` — release notes for a package or GitHub repo, newest-first. Default latest mode returns the 10 most recent entries with full markdown bodies; `from_version` switches to range mode between two versions. Addressable via `registry` + `package_name` or `repo_url`. Set `include_bodies: false` for a version / date / URL timeline when bodies aren't needed."; const SEARCH_BULLET = - "- `search` — unified search across indexed dependency code, docs, and explicit symbols. Structured fields are the primary UX; omit `sources` for AUTO. Returns only trustworthy complete results by default; opt into partial hits with `allow_partial_results: true`. If indexing is still in progress, the response carries a `searchRef`."; + '- `search` — unified search across indexed dependency code, docs, and explicit symbols. Structured fields are the primary UX; omit `sources` for AUTO. Default response is a compact text listing (`text-v1`); pass `format: "json"` for the structured envelope with full locator fields, highlights, and source status. Returns only trustworthy complete results by default; opt into partial hits with `allow_partial_results: true`. If indexing is still in progress, the response carries a `searchRef`.'; const SEARCH_STATUS_BULLET = "- `search_status` — follow up a prior unified search by `searchRef`. Use it after `search` returns incomplete state to check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results."; const CODE_FILES_BULLET = - "- `code_files` — discover what files a dependency ships. Use `path_prefix` to scope to a subdirectory; the response includes each file's language, type, and byte size. Returned `path` values feed directly into `code_read` and help scope `code_grep`."; + '- `code_files` — discover what files a dependency ships. Use `path_prefix` to scope to a subdirectory. Default response is a paths-only listing (`text-v1`); pass `format: "json"` for the full envelope with each file\'s language, type, and byte size. Returned paths feed directly into `code_read` and help scope `code_grep`.'; const CODE_READ_BULLET = - "- `code_read` — fetch a file's contents from a dependency. Pass the same `path` emitted by `code_files`. Default returns the full file; pass `start_line` / `end_line` for a bounded range. Binary files set `isBinary: true` and omit `content` — branch on the flag. A `FILE_NOT_FOUND` (or `NOT_FOUND`) response is the signal to call `code_files` for the actual path."; + "- `code_read` — fetch a file's contents from a dependency. Pass the same `path` emitted by `code_files`. **MCP cap: 150 lines per call** — broader requests (or no range) silently truncate to the first 150 lines from your start, with a `hint` describing what was returned vs. requested. Pick a focused window from a `search` / `code_grep` match. Binary files set `isBinary: true` and omit `content`. A `FILE_NOT_FOUND` (or `NOT_FOUND`) response is the signal to call `code_files` for the actual path."; const CODE_GREP_BULLET = - "- `code_grep` — deterministic text grep over indexed source files. Use it when you know the exact text or regex to match; use `search` for discovery. Whole-target grep is the default; narrow with `path`, `path_prefix`, `globs`, or `extensions`. Returned `matches[].filePath` feeds directly into `code_read`."; + '- `code_grep` — deterministic text grep over indexed source files. Use it when you know the exact text or regex to match; use `search` for discovery. Whole-target grep is the default; narrow with `path`, `path_prefix`, `globs`, or `extensions`. Default response groups matches by file with line numbers (`text-v1`); pass `format: "json"` for the `matches[]` array with byte offsets and symbol metadata. Each match\'s `path:line` chains directly into `code_read`.'; const SEARCH_VS_SYMBOLS_TIP = - 'Prefer `get_example` for canonical example retrieval; prefer unified `search` for indexed dependency and repository discovery; use `sources:["symbol"]` when you want symbol-shaped results. Use `code_grep` for deterministic text matching and `code_read` for full-file inspection.'; + 'Prefer `get_example` for canonical example retrieval; prefer unified `search` for indexed dependency and repository discovery; use `sources:["symbol"]` when you want symbol-shaped results. Use `code_grep` for deterministic text matching and `code_read` for focused-window inspection of a known file.'; + +const REFERENCE_FIRST_TIP = + "Strategy — reference-first, content-second. Locate symbols and lines with `search` / `code_grep` first, then read only the lines you actually need with `code_read` using explicit `start_line` / `end_line` windows around the match (typically 80-150 lines). The MCP `code_read` surface caps each call at 150 lines; bigger requests are silently truncated. Each turn — including retries to widen or re-narrow — costs context, so pick a focused window the first time rather than starting wide and trimming."; + +const MULTI_TURN_TIP = + '**Delegate multi-call work to a sub-agent.** Code-navigation tools (`search`, `code_grep`, `code_read`, `code_files`) are inherently multi-call — answering even simple questions usually takes 3-10 tool calls, and reading raw source into the main conversation compounds quickly. The default approach for any cross-project comparison, codebase mapping, pattern survey, or "how does X actually work" investigation is to spawn a sub-task / sub-agent that does the digging and returns only a compact synthesis. Do the work in the main conversation only when the result genuinely belongs there (e.g., the user asked for a specific snippet to paste into their own code). When in doubt, delegate.'; /** * Whether the MCP session should register and describe package tools. @@ -116,10 +122,19 @@ export function buildMcpInstructions(deps: Dependencies): string { return sections.join("\n\n"); } - const parts = [PACKAGE_TOOLS_PREAMBLE, bullets.join("\n")]; - // The decision tip contrasts `get_example` with unified `search`, so - // it is only meaningful when unified search is actually registered. + const parts = [PACKAGE_TOOLS_PREAMBLE]; + // The multi-turn tip leads when code-navigation is wired — it is + // the highest-leverage decision the agent makes (delegate vs. + // run inline) and reading it before the per-tool bullets means + // the framing arrives before any specific tool fires. The + // reference-first / decision tips follow the bullets where they + // act as workflow guidance for the chosen tool. + if (deps.codeNavigationService) { + parts.push(MULTI_TURN_TIP); + } + parts.push(bullets.join("\n")); if (deps.codeNavigationService) { + parts.push(REFERENCE_FIRST_TIP); parts.push(SEARCH_VS_SYMBOLS_TIP); } diff --git a/src/shared/grep-repo-text.test.ts b/src/shared/grep-repo-text.test.ts new file mode 100644 index 00000000..636088d3 --- /dev/null +++ b/src/shared/grep-repo-text.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, it } from "bun:test"; +import type { + LeanGrepRepoEnvelope, + LeanGrepRepoMatch, +} from "./grep-repo-response.js"; +import { renderGrepRepoText } from "./grep-repo-text.js"; + +function envelope( + overrides: Partial = {}, +): LeanGrepRepoEnvelope { + return { + pattern: "applyEdit", + matches: [], + hasMore: false, + filesScanned: 120, + filesInScope: 120, + totalMatches: 0, + uniqueFilesMatched: 0, + ...overrides, + }; +} + +function match(overrides: Partial = {}): LeanGrepRepoMatch { + return { + filePath: "src/diff/foo.ts", + line: 142, + matchStartByte: 16, + matchEndByte: 25, + lineContent: "export function applyEdit(input: string): string {", + ...overrides, + }; +} + +describe("renderGrepRepoText", () => { + it("renders zero-match header and empty body", () => { + const text = renderGrepRepoText(envelope()); + expect(text).toContain("code_grep | 0 matches in 0 files"); + expect(text).toContain('pattern="applyEdit"'); + expect(text).toContain("No matches."); + }); + + it("renders single-file matches grouped under the file with line gutter", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 1, + matches: [ + match({ line: 142 }), + match({ + line: 287, + lineContent: " const result = applyEdit(input, hunk);", + }), + ], + }), + ); + expect(text).toContain("src/diff/foo.ts (2)"); + expect(text).toContain( + " 142: export function applyEdit(input: string): string {", + ); + expect(text).toContain(" 287: const result = applyEdit(input, hunk);"); + }); + + it("renders matches across multiple files with blank-line separators", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 2, + matches: [ + match({ filePath: "src/a.ts", line: 10, lineContent: "a-line" }), + match({ filePath: "src/b.ts", line: 20, lineContent: "b-line" }), + ], + }), + ); + expect(text).toContain("src/a.ts (1)"); + expect(text).toContain("src/b.ts (1)"); + const aIdx = text.indexOf("src/a.ts"); + const bIdx = text.indexOf("src/b.ts"); + expect(aIdx).toBeLessThan(bIdx); + // Two file blocks separated by a blank line. + const between = text.slice(aIdx, bIdx); + expect(between).toContain("\n\n"); + }); + + it("renders context lines with grep -A/-B separators", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [ + match({ + line: 142, + contextBefore: [ + "// Apply a unified diff patch", + "// Returns the new content", + ], + contextAfter: [' const lines = file.split("\\n");'], + }), + ], + }), + ); + expect(text).toContain(" 140- // Apply a unified diff patch"); + expect(text).toContain(" 141- // Returns the new content"); + expect(text).toContain( + " 142: export function applyEdit(input: string): string {", + ); + expect(text).toContain(' 143- const lines = file.split("\\n");'); + }); + + it("inserts a separator between non-adjacent blocks in the same file", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 1, + matches: [ + match({ + line: 50, + contextBefore: ["before-50"], + contextAfter: ["after-50"], + }), + match({ + line: 200, + contextBefore: ["before-200"], + contextAfter: ["after-200"], + }), + ], + }), + ); + expect(text).toContain(" --"); + }); + + it("renders truncation notice when truncatedReason is set", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 50, + uniqueFilesMatched: 7, + truncatedReason: "limit", + hasMore: true, + matches: [match()], + }), + ); + expect(text).toContain("Truncated: limit."); + expect(text).toContain("max_matches"); + }); + + it("renders next-cursor note when hasMore", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 50, + uniqueFilesMatched: 7, + hasMore: true, + nextCursor: "ABC123", + matches: [match()], + }), + ); + expect(text).toContain("More matches available. Pass cursor=ABC123"); + }); + + it("renders pattern-type and case-sensitive flags in header when set", () => { + const text = renderGrepRepoText( + envelope({ + patternType: "regex", + caseSensitive: true, + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [match()], + }), + ); + expect(text).toContain("regex,case-sensitive"); + }); + + it("echoes filter inputs in header when set", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [match()], + filter: { + pathPrefix: "src/integrations", + extensions: ["ts", "tsx"], + maxMatches: 30, + }, + }), + ); + expect(text).toContain('path_prefix="src/integrations"'); + expect(text).toContain("exts=ts,tsx"); + expect(text).toContain("max_matches=30"); + }); + + it("uses ASCII separators only", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + truncatedReason: "limit", + hasMore: true, + nextCursor: "X", + matches: [match()], + }), + ); + expect(text).not.toMatch(/[·…—–]/); + }); + + it("notes binary/oversized skips when present", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + binaryFilesSkipped: 4, + filesTooLargeSkipped: 2, + matches: [match()], + }), + ); + expect(text).toContain("4 binary file(s) skipped"); + expect(text).toContain("2 oversized file(s) skipped"); + }); +}); diff --git a/src/shared/grep-repo-text.ts b/src/shared/grep-repo-text.ts new file mode 100644 index 00000000..5735484d --- /dev/null +++ b/src/shared/grep-repo-text.ts @@ -0,0 +1,269 @@ +/** + * Line-oriented text renderer for `code_grep` MCP responses. + * + * Designed for agent context efficiency: matches grouped by file, + * `: ` for matches and `- ` for context + * (standard grep -A/-B convention), no per-match scaffolding bytes. + * Programmatic / parity callers stay on the JSON envelope by passing + * `format: "json"`. + * + * ASCII-only output. Format is a public contract — locked with + * snapshot-style tests in `grep-repo-text.test.ts`. + */ + +import type { + LeanGrepRepoEnvelope, + LeanGrepRepoMatch, +} from "./grep-repo-response.js"; + +const SEP = " | "; + +interface RenderLine { + lineNumber: number; + content: string; + isMatch: boolean; +} + +interface RenderBlock { + filePath: string; + lines: RenderLine[]; +} + +export function renderGrepRepoText(envelope: LeanGrepRepoEnvelope): string { + const lines: string[] = []; + lines.push(buildHeader(envelope)); + lines.push(""); + + if (envelope.matches.length === 0) { + lines.push("No matches."); + const trailer = buildTrailer(envelope); + if (trailer.length > 0) { + lines.push(""); + for (const t of trailer) lines.push(t); + } + return lines.join("\n"); + } + + const blocks = buildRenderBlocks(envelope.matches); + const blocksByFile = groupBlocksByFile(blocks); + const useContext = blocksHaveContext(blocks); + const matchCountsByFile = countMatchesByFile(envelope.matches); + + let firstFile = true; + for (const [filePath, fileBlocks] of blocksByFile) { + if (!firstFile) lines.push(""); + firstFile = false; + const matchCount = matchCountsByFile.get(filePath) ?? 0; + lines.push(`${filePath} (${matchCount})`); + fileBlocks.forEach((block, idx) => { + if (useContext && idx > 0) lines.push(" --"); + const gutterWidth = widestLineNumberInBlock(block); + for (const ln of block.lines) { + lines.push(renderLine(ln, gutterWidth, useContext)); + } + }); + } + + const trailer = buildTrailer(envelope); + if (trailer.length > 0) { + lines.push(""); + for (const t of trailer) lines.push(t); + } + + return lines.join("\n"); +} + +function buildHeader(envelope: LeanGrepRepoEnvelope): string { + const parts = [ + `code_grep${SEP}${envelope.totalMatches} match${ + envelope.totalMatches === 1 ? "" : "es" + } in ${envelope.uniqueFilesMatched} file${ + envelope.uniqueFilesMatched === 1 ? "" : "s" + }`, + ]; + parts.push(`pattern=${quote(envelope.pattern)}`); + + const flags: string[] = []; + if (envelope.patternType === "regex") flags.push("regex"); + if (envelope.caseSensitive) flags.push("case-sensitive"); + if (flags.length > 0) parts.push(flags.join(",")); + + const filterEcho = buildFilterEcho(envelope); + if (filterEcho) parts.push(filterEcho); + + return parts.join(SEP); +} + +function buildFilterEcho(envelope: LeanGrepRepoEnvelope): string { + const filter = envelope.filter; + if (!filter) return ""; + const parts: string[] = []; + if (filter.path) parts.push(`path=${quote(filter.path)}`); + if (filter.pathPrefix) parts.push(`path_prefix=${quote(filter.pathPrefix)}`); + if (filter.globs?.length) parts.push(`globs=${filter.globs.join(",")}`); + if (filter.extensions?.length) { + parts.push(`exts=${filter.extensions.join(",")}`); + } + if (typeof filter.maxMatches === "number") { + parts.push(`max_matches=${filter.maxMatches}`); + } + if (typeof filter.maxMatchesPerFile === "number") { + parts.push(`max_matches_per_file=${filter.maxMatchesPerFile}`); + } + return parts.join(" "); +} + +function buildTrailer(envelope: LeanGrepRepoEnvelope): string[] { + const lines: string[] = []; + + if (envelope.truncatedReason) { + lines.push( + `Truncated: ${envelope.truncatedReason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + ); + } + + if (envelope.hasMore && envelope.nextCursor) { + lines.push( + `More matches available. Pass cursor=${envelope.nextCursor} for the next page.`, + ); + } else if (envelope.hasMore) { + lines.push("More matches available."); + } + + // Skip-count notes are useful when something was excluded silently. + const skipNotes: string[] = []; + if (envelope.binaryFilesSkipped) { + skipNotes.push(`${envelope.binaryFilesSkipped} binary file(s) skipped`); + } + if (envelope.filesTooLargeSkipped) { + skipNotes.push( + `${envelope.filesTooLargeSkipped} oversized file(s) skipped`, + ); + } + if (skipNotes.length > 0) { + lines.push(`Note: ${skipNotes.join(", ")}.`); + } + + return lines; +} + +function buildRenderBlocks(matches: LeanGrepRepoMatch[]): RenderBlock[] { + if (matches.length === 0) return []; + + const linesByFile = new Map>(); + for (const match of matches) { + let lineMap = linesByFile.get(match.filePath); + if (!lineMap) { + lineMap = new Map(); + linesByFile.set(match.filePath, lineMap); + } + + const before = match.contextBefore ?? []; + const beforeStart = match.line - before.length; + for (let i = 0; i < before.length; i += 1) { + const lineNumber = beforeStart + i; + if (!lineMap.has(lineNumber)) { + lineMap.set(lineNumber, { + lineNumber, + content: before[i] ?? "", + isMatch: false, + }); + } + } + + // Match line: overwrite a previous context line at the same number + // with a match (matches win over context). + lineMap.set(match.line, { + lineNumber: match.line, + content: match.lineContent, + isMatch: true, + }); + + const after = match.contextAfter ?? []; + for (let i = 0; i < after.length; i += 1) { + const lineNumber = match.line + i + 1; + if (!lineMap.has(lineNumber)) { + lineMap.set(lineNumber, { + lineNumber, + content: after[i] ?? "", + isMatch: false, + }); + } + } + } + + const blocks: RenderBlock[] = []; + for (const [filePath, lineMap] of linesByFile) { + const sorted = [...lineMap.values()].sort( + (a, b) => a.lineNumber - b.lineNumber, + ); + let current: RenderLine[] = []; + for (const line of sorted) { + const previous = current[current.length - 1]; + if (!previous || line.lineNumber === previous.lineNumber + 1) { + current.push(line); + continue; + } + blocks.push({ filePath, lines: current }); + current = [line]; + } + if (current.length > 0) { + blocks.push({ filePath, lines: current }); + } + } + return blocks; +} + +function groupBlocksByFile(blocks: RenderBlock[]): Map { + const map = new Map(); + for (const block of blocks) { + const list = map.get(block.filePath) ?? []; + list.push(block); + map.set(block.filePath, list); + } + return map; +} + +function blocksHaveContext(blocks: RenderBlock[]): boolean { + for (const block of blocks) { + for (const line of block.lines) { + if (!line.isMatch) return true; + } + } + return false; +} + +function widestLineNumberInBlock(block: RenderBlock): number { + let max = 0; + for (const line of block.lines) { + const len = String(line.lineNumber).length; + if (len > max) max = len; + } + return max; +} + +function countMatchesByFile(matches: LeanGrepRepoMatch[]): Map { + const counts = new Map(); + for (const match of matches) { + counts.set(match.filePath, (counts.get(match.filePath) ?? 0) + 1); + } + return counts; +} + +function renderLine( + line: RenderLine, + gutterWidth: number, + useContext: boolean, +): string { + const gutter = String(line.lineNumber).padStart(gutterWidth, " "); + // Standard grep -A/-B notation: `:` separator on match lines, + // `-` on context lines. When there is no context anywhere in the + // response, every line is a match — we still keep the colon so + // the format is uniform. + const sep = !useContext || line.isMatch ? ":" : "-"; + return ` ${gutter}${sep} ${line.content}`; +} + +function quote(value: string): string { + return value.includes('"') ? `'${value}'` : `"${value}"`; +} diff --git a/src/shared/index.ts b/src/shared/index.ts index a4327a4d..e4a2599b 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -53,10 +53,12 @@ export { type LeanGrepRepoFilter, type LeanGrepRepoMatch, } from "./grep-repo-response.js"; +export { renderGrepRepoText } from "./grep-repo-text.js"; export { filterLanguages, type LanguageMatch, } from "./language-filter.js"; +export { renderListFilesText } from "./list-files-text.js"; export { buildListPackageDocsParams, type ListPackageDocsRequestBuildResult, @@ -185,3 +187,7 @@ export { type UnifiedSearchStatusResultPayload, } from "./unified-search-response.js"; export { parseUnifiedSearchTargetSpec } from "./unified-search-target.js"; +export { + renderUnifiedSearchError, + renderUnifiedSearchSuccess, +} from "./unified-search-text.js"; diff --git a/src/shared/list-files-text.test.ts b/src/shared/list-files-text.test.ts new file mode 100644 index 00000000..70c9b4ad --- /dev/null +++ b/src/shared/list-files-text.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "bun:test"; +import type { LeanListFilesEnvelope } from "./list-files-response.js"; +import { renderListFilesText } from "./list-files-text.js"; + +function envelope( + overrides: Partial = {}, +): LeanListFilesEnvelope { + return { + registry: "npm", + name: "express", + indexedVersion: "v5.2.1", + total: 2, + hasMore: false, + files: [ + { path: "src/index.js", language: "javascript", fileType: "SOURCE" }, + { path: "src/lib/app.js", language: "javascript", fileType: "SOURCE" }, + ], + ...overrides, + }; +} + +describe("renderListFilesText", () => { + it("renders a paths-only listing with version-tagged identity", () => { + const text = renderListFilesText(envelope()); + expect(text).toContain("code_files | 2 paths | npm:express@v5.2.1"); + expect(text).toContain("src/index.js"); + expect(text).toContain("src/lib/app.js"); + // No trailing metadata in default mode. + expect(text).not.toContain("javascript"); + expect(text).not.toContain("SOURCE"); + }); + + it("uses repo addressing when no registry is provided", () => { + const text = renderListFilesText( + envelope({ + registry: undefined, + name: undefined, + indexedVersion: undefined, + repoUrl: "https://github.com/cline/cline", + gitRef: "v3.4.2", + }), + ); + expect(text).toContain( + "code_files | 2 paths | https://github.com/cline/cline@v3.4.2", + ); + }); + + it("emits a truncation hint with N+ count when hasMore", () => { + const text = renderListFilesText(envelope({ hasMore: true, total: 2 })); + expect(text).toContain("code_files | 2+ paths"); + expect(text).toContain("More files available."); + }); + + it("echoes explicit filter inputs in the header", () => { + const text = renderListFilesText( + envelope({ filter: { pathPrefix: "src/lib", limit: 50 } }), + ); + expect(text).toContain('path_prefix="src/lib" limit=50'); + }); + + it("renders the empty-result hint when no files match", () => { + const text = renderListFilesText( + envelope({ + files: [], + total: 0, + hint: "No files match this path prefix.", + }), + ); + expect(text).toContain("code_files | 0 paths"); + expect(text).toContain("No files match this path prefix."); + }); + + it("uses ASCII separators throughout", () => { + const text = renderListFilesText( + envelope({ + hasMore: true, + filter: { pathPrefix: "src/", limit: 50 }, + }), + ); + expect(text).not.toMatch(/[·…—–]/); + }); +}); diff --git a/src/shared/list-files-text.ts b/src/shared/list-files-text.ts new file mode 100644 index 00000000..450a2436 --- /dev/null +++ b/src/shared/list-files-text.ts @@ -0,0 +1,88 @@ +/** + * Line-oriented text renderer for `code_files` MCP responses. + * + * Paths-only listing (one file per line) — the most compact useful + * shape for an agent that will follow up with `code_read`. This is + * the tool's default response format; programmatic / parity callers + * opt into the structured JSON envelope via `format: "json"`. + * + * ASCII-only output. Format is a public contract — locked with + * snapshot-style tests in `list-files-text.test.ts`. + */ + +import type { LeanListFilesEnvelope } from "./list-files-response.js"; + +const SEP = " | "; + +export function renderListFilesText(envelope: LeanListFilesEnvelope): string { + const lines: string[] = []; + lines.push(buildHeader(envelope)); + lines.push(""); + + if (envelope.files.length === 0) { + lines.push(envelope.hint ?? "No files match the requested filter."); + return lines.join("\n"); + } + + for (const entry of envelope.files) { + lines.push(entry.path); + } + + if (envelope.hasMore) { + lines.push(""); + lines.push( + "More files available. Pass limit=N to widen or refine path_prefix.", + ); + } + + if (envelope.hint) { + lines.push(""); + lines.push(envelope.hint); + } + + return lines.join("\n"); +} + +function buildHeader(envelope: LeanListFilesEnvelope): string { + const identity = buildIdentity(envelope); + const countValue = envelope.hasMore + ? `${envelope.files.length}+` + : String(envelope.total); + const parts = [ + `code_files${SEP}${countValue} path${countValue === "1" ? "" : "s"}`, + ]; + if (identity) parts.push(identity); + const filter = buildFilterEcho(envelope); + if (filter) parts.push(filter); + return parts.join(SEP); +} + +function buildIdentity(envelope: LeanListFilesEnvelope): string { + if (envelope.registry && envelope.name) { + const version = envelope.indexedVersion ?? envelope.resolution?.resolvedRef; + return version + ? `${envelope.registry}:${envelope.name}@${version}` + : `${envelope.registry}:${envelope.name}`; + } + if (envelope.repoUrl) { + return envelope.gitRef + ? `${envelope.repoUrl}@${envelope.gitRef}` + : envelope.repoUrl; + } + return ""; +} + +function buildFilterEcho(envelope: LeanListFilesEnvelope): string { + const parts: string[] = []; + if (envelope.filter?.pathPrefix) { + parts.push(`path_prefix=${quote(envelope.filter.pathPrefix)}`); + } + if (envelope.filter?.limit !== undefined) { + parts.push(`limit=${envelope.filter.limit}`); + } + return parts.join(" "); +} + +function quote(value: string): string { + return value.includes('"') ? `'${value}'` : `"${value}"`; +} diff --git a/src/shared/read-file-request.test.ts b/src/shared/read-file-request.test.ts index a8828518..ff002d21 100644 --- a/src/shared/read-file-request.test.ts +++ b/src/shared/read-file-request.test.ts @@ -9,7 +9,7 @@ const target: CodeNavigationTarget = { describe("buildReadFileParams — defaults and validation", () => { it("accepts a minimal request and defaults wait to 20000", () => { - const { params, startLineExplicit, endLineExplicit } = buildReadFileParams({ + const { params } = buildReadFileParams({ target, filePath: "src/index.js", }); @@ -17,8 +17,6 @@ describe("buildReadFileParams — defaults and validation", () => { expect(params.startLine).toBeUndefined(); expect(params.endLine).toBeUndefined(); expect(params.waitTimeoutMs).toBe(20000); - expect(startLineExplicit).toBe(false); - expect(endLineExplicit).toBe(false); }); it("trims whitespace around filePath", () => { @@ -36,7 +34,7 @@ describe("buildReadFileParams — defaults and validation", () => { }); it("passes line range through", () => { - const { params, startLineExplicit, endLineExplicit } = buildReadFileParams({ + const { params } = buildReadFileParams({ target, filePath: "src/index.js", startLine: 10, @@ -44,8 +42,6 @@ describe("buildReadFileParams — defaults and validation", () => { }); expect(params.startLine).toBe(10); expect(params.endLine).toBe(40); - expect(startLineExplicit).toBe(true); - expect(endLineExplicit).toBe(true); }); it("accepts open-ended start (end omitted)", () => { diff --git a/src/shared/read-file-request.ts b/src/shared/read-file-request.ts index 792b8333..9af1b52b 100644 --- a/src/shared/read-file-request.ts +++ b/src/shared/read-file-request.ts @@ -24,8 +24,6 @@ export interface ReadFileRequestInput { export interface ReadFileRequestBuildResult { params: ReadFileParams; - startLineExplicit: boolean; - endLineExplicit: boolean; } export function buildReadFileParams( @@ -56,8 +54,6 @@ export function buildReadFileParams( endLine, waitTimeoutMs, }, - startLineExplicit: input.startLine !== undefined, - endLineExplicit: input.endLine !== undefined, }; } diff --git a/src/shared/read-file-response.test.ts b/src/shared/read-file-response.test.ts index c2647698..f90f2765 100644 --- a/src/shared/read-file-response.test.ts +++ b/src/shared/read-file-response.test.ts @@ -101,6 +101,23 @@ describe("buildReadFileSuccessPayload", () => { ); expect(envelope.content).toBe(""); }); + + it("does not auto-populate the hint field — that policy belongs to the MCP handler", () => { + const wideResult: ReadFileResult = { + filePath: "src/big.ts", + language: "typescript", + totalLines: 5000, + startLine: 1, + endLine: 5000, + content: "// big file\n".repeat(5000), + isBinary: false, + }; + const envelope = buildReadFileSuccessPayload(wideResult, { + ...baseOptions, + requestedFilePath: "src/big.ts", + }); + expect(envelope.hint).toBeUndefined(); + }); }); describe("formatReadFileTerminal", () => { diff --git a/src/shared/read-file-response.ts b/src/shared/read-file-response.ts index 1d4248ed..dcaf1234 100644 --- a/src/shared/read-file-response.ts +++ b/src/shared/read-file-response.ts @@ -30,6 +30,14 @@ export interface LeanReadFileEnvelope { content?: string; /** Present and `true` when the file is binary; absent otherwise. */ isBinary?: boolean; + /** + * Optional one-line guidance for the agent. Set by the MCP tool + * handler when it caps the returned span (see + * `src/tools/read-file.ts`). Not auto-populated by this builder — + * the policy of when to nudge an agent belongs to the surface, not + * the response shape. + */ + hint?: string; } export interface BuildReadFilePayloadOptions { @@ -163,6 +171,10 @@ function formatVerboseBody( ); lines.push(`${gutter} ${bodyLines[i]}`); } + if (envelope.hint) { + lines.push(""); + lines.push(dim(envelope.hint, options.useColors)); + } lines.push(""); return lines.join("\n"); } diff --git a/src/shared/unified-search-text.test.ts b/src/shared/unified-search-text.test.ts new file mode 100644 index 00000000..a47fb68e --- /dev/null +++ b/src/shared/unified-search-text.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it } from "bun:test"; +import type { + UnifiedSearchCompletedPayload, + UnifiedSearchErrorPayload, + UnifiedSearchHitPayload, + UnifiedSearchIncompletePayload, +} from "./unified-search-response.js"; +import { + renderUnifiedSearchError, + renderUnifiedSearchSuccess, +} from "./unified-search-text.js"; + +function codeHit( + overrides: Partial = {}, +): UnifiedSearchHitPayload { + return { + type: "repository_code", + target: "cline/cline@v3.4.2", + title: "applyEdit", + summary: + "Search/replace block parser with fuzzy fallback when exact match fails.", + score: 0.87, + locator: { + registry: "npm", + packageName: "cline", + version: "v3.4.2", + filePath: "src/integrations/diff/strategies/multi-search-replace.ts", + startLine: 142, + endLine: 156, + kind: "function", + }, + ...overrides, + }; +} + +function docsHit(): UnifiedSearchHitPayload { + return { + type: "documentation_page", + target: "aider-AI/aider@v0.55.0", + title: "Edit Formats", + summary: "Compares whole-file, diff-fenced, udiff, and editblock formats.", + score: 0.83, + locator: { + pageId: "aider/edit-formats", + sourceUrl: "https://aider.chat/docs/more/edit-formats.html", + sourceKind: "hosted", + }, + }; +} + +function symbolHit(): UnifiedSearchHitPayload { + return { + type: "repository_symbol", + target: "continuedev/continue@v0.9.42", + title: "diffLines", + summary: "Myers diff core; line-level with O(ND) complexity.", + score: 0.91, + locator: { + filePath: "core/diff/myers.ts", + startLine: 48, + endLine: 112, + qualifiedPath: "core.diff.myers.diffLines", + kind: "function", + category: "callable", + language: "typescript", + }, + }; +} + +function completed( + results: UnifiedSearchHitPayload[], + overrides: Partial = {}, +): UnifiedSearchCompletedPayload { + return { + query: { raw: "diff myers" }, + completed: true, + hasMore: false, + results, + ...overrides, + }; +} + +describe("renderUnifiedSearchSuccess", () => { + it("renders an empty completed envelope with a clear message", () => { + const text = renderUnifiedSearchSuccess(completed([])); + expect(text).toContain("0 hits"); + expect(text).toContain('query="diff myers"'); + expect(text).toContain("No hits."); + }); + + it("renders a single code hit with locator, title, and summary", () => { + const text = renderUnifiedSearchSuccess(completed([codeHit()])); + expect(text).toContain("[1] cline/cline@v3.4.2 code 0.87"); + expect(text).toContain( + " src/integrations/diff/strategies/multi-search-replace.ts:142-156 function", + ); + expect(text).toContain(" applyEdit"); + expect(text).toContain( + " Search/replace block parser with fuzzy fallback when exact match fails.", + ); + }); + + it("uses pageId for documentation hits", () => { + const text = renderUnifiedSearchSuccess(completed([docsHit()])); + expect(text).toContain("[1] aider-AI/aider@v0.55.0 docs 0.83"); + expect(text).toContain(" pageId: aider/edit-formats"); + expect(text).toContain(" Edit Formats"); + }); + + it("renders qualifiedPath alongside file location for symbol hits", () => { + const text = renderUnifiedSearchSuccess(completed([symbolHit()])); + expect(text).toContain("[1] continuedev/continue@v0.9.42 symbol 0.91"); + expect(text).toContain( + " core/diff/myers.ts:48-112 core.diff.myers.diffLines | function", + ); + expect(text).toContain(" diffLines"); + }); + + it("uses ASCII separators throughout (no multi-byte chars)", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit(), symbolHit()]), + ); + // No common Unicode-Latin1 separator characters in the output. + expect(text).not.toMatch(/[·…—–]/); + }); + + it("emits a truncation hint with offset when hasMore", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { hasMore: true, nextOffset: 10 }), + ); + expect(text).toContain("More hits available. Pass offset=10"); + }); + + it("falls back to a plain widen hint when nextOffset is missing", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { hasMore: true }), + ); + expect(text).toContain("More hits available. Pass limit=N to widen."); + }); + + it("renders incomplete payloads with searchRef and progress hint", () => { + const incomplete: UnifiedSearchIncompletePayload = { + query: { raw: "myers" }, + completed: false, + hasMore: false, + results: [codeHit()], + searchRef: "ref_abc-123", + progress: { + status: "INDEXING", + targetsReady: 1, + targetsTotal: 2, + elapsedMs: 8200, + }, + }; + const text = renderUnifiedSearchSuccess(incomplete); + expect(text).toContain("1 partial"); + expect(text).toContain("searchRef=ref_abc-123"); + expect(text).toContain( + "Indexing in progress. Call search_status with searchRef=ref_abc-123", + ); + }); + + it("wraps long summaries at the configured width", () => { + const longSummary = + "This summary is intentionally long enough to force the wrap logic to break it across multiple lines so the renderer's wrap behaviour is verified."; + const text = renderUnifiedSearchSuccess( + completed([codeHit({ summary: longSummary })]), + ); + const summaryLines = text + .split("\n") + .filter( + (line) => line.startsWith(" ") && line.includes("intentionally"), + ); + expect(summaryLines.length).toBeGreaterThanOrEqual(1); + for (const line of text.split("\n")) { + // 4-space indent + content; allow some slack for the wrap target. + expect(line.length).toBeLessThanOrEqual(82); + } + }); + + it("renders source-status notes when the backend reports them", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { + sourceStatus: [ + { + source: "code", + targetLabel: "npm/cline@v3.4.2", + ignoredFilters: ["fileIntent"], + note: "fileIntent unsupported on code source", + }, + ], + }), + ); + expect(text).toContain("source notes:"); + expect(text).toContain("- code (npm/cline@v3.4.2)"); + expect(text).toContain("ignored=fileIntent"); + }); + + it("separates multiple hits with a blank line", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit(), docsHit(), symbolHit()]), + ); + const hitHeaders = text.split("\n").filter((line) => /^\[\d\]/.test(line)); + expect(hitHeaders).toHaveLength(3); + expect(text).toContain("[1] cline/cline@v3.4.2"); + expect(text).toContain("[2] aider-AI/aider@v0.55.0"); + expect(text).toContain("[3] continuedev/continue@v0.9.42"); + }); +}); + +describe("renderUnifiedSearchError", () => { + it("renders a basic error", () => { + const error: UnifiedSearchErrorPayload = { + error: "Target is still indexing.", + code: "INDEXING", + retryable: true, + details: { indexingRef: "ref_xyz" }, + }; + const text = renderUnifiedSearchError(error); + expect(text).toContain("search | ERROR | code=INDEXING | retryable"); + expect(text).toContain("Target is still indexing."); + expect(text).toContain("details:"); + expect(text).toContain(" indexingRef: ref_xyz"); + }); + + it("omits retryable marker when not set", () => { + const error: UnifiedSearchErrorPayload = { + error: "Bad request.", + code: "INVALID_ARGUMENT", + }; + const text = renderUnifiedSearchError(error); + expect(text).toBe("search | ERROR | code=INVALID_ARGUMENT\nBad request."); + }); + + it("serialises object detail values via JSON", () => { + const error: UnifiedSearchErrorPayload = { + error: "Indexing.", + code: "INDEXING", + details: { + availableVersions: [ + { version: "4.21.0", ref: "v4.21.0" }, + { version: "4.20.0", ref: "v4.20.0" }, + ], + }, + }; + const text = renderUnifiedSearchError(error); + expect(text).toContain('"version":"4.21.0"'); + }); +}); diff --git a/src/shared/unified-search-text.ts b/src/shared/unified-search-text.ts new file mode 100644 index 00000000..1618b56e --- /dev/null +++ b/src/shared/unified-search-text.ts @@ -0,0 +1,258 @@ +/** + * Line-oriented text renderer for unified `search` MCP responses. + * + * Designed for agent context efficiency: roughly 3-5 lines per hit + * and no JSON scaffolding. This is the tool's default response + * format — programmatic / parity callers opt into the structured + * JSON envelope by passing `format: "json"`. + * + * ASCII-only output — separators tokenize cleanly across BPE + * variants, and there are no Unicode characters that require + * client-side escaping. + * + * Format is a public contract — locked with snapshot-style tests in + * `unified-search-text.test.ts`. Update the spec in + * `docs/implementation/tools.md` when changing the format. + */ + +import type { + UnifiedSearchCompletedPayload, + UnifiedSearchErrorPayload, + UnifiedSearchHitPayload, + UnifiedSearchIncompletePayload, +} from "./unified-search-response.js"; + +const SUMMARY_WRAP_WIDTH = 76; +const SEP = " | "; + +type SearchSuccessPayload = + | UnifiedSearchCompletedPayload + | UnifiedSearchIncompletePayload; + +/** Render a successful unified-search payload as line-oriented text. */ +export function renderUnifiedSearchSuccess( + payload: SearchSuccessPayload, +): string { + const lines: string[] = []; + lines.push(buildHeader(payload)); + lines.push(""); + + if (payload.results.length === 0) { + lines.push(payload.completed ? "No hits." : "No hits yet - indexing."); + } else { + payload.results.forEach((hit, idx) => { + if (idx > 0) lines.push(""); + appendHit(lines, idx + 1, hit); + }); + } + + const trailer = buildTrailer(payload); + if (trailer.length > 0) { + lines.push(""); + for (const line of trailer) lines.push(line); + } + + return lines.join("\n"); +} + +/** Render an error envelope as compact text. */ +export function renderUnifiedSearchError( + payload: UnifiedSearchErrorPayload, +): string { + const lines: string[] = []; + const header = `search${SEP}ERROR${SEP}code=${payload.code}${ + payload.retryable ? `${SEP}retryable` : "" + }`; + lines.push(header); + lines.push(payload.error); + + if (payload.details && Object.keys(payload.details).length > 0) { + lines.push(""); + lines.push("details:"); + for (const [key, value] of Object.entries(payload.details)) { + lines.push(` ${key}: ${formatDetailValue(value)}`); + } + } + return lines.join("\n"); +} + +function buildHeader(payload: SearchSuccessPayload): string { + const count = payload.results.length; + const status = payload.completed + ? `${count} hit${count === 1 ? "" : "s"}` + : `${count} partial`; + const parts = [`search${SEP}${status}`]; + parts.push(`query=${quote(payload.query.raw)}`); + if (!payload.completed) { + parts.push(`searchRef=${payload.searchRef}`); + } + return parts.join(SEP); +} + +function appendHit( + lines: string[], + index: number, + hit: UnifiedSearchHitPayload, +): void { + const headerParts: string[] = [hit.target, shortType(hit.type)]; + if (typeof hit.score === "number") { + headerParts.push(formatScore(hit.score)); + } + lines.push(`[${index}] ${headerParts.join(" ")}`); + + const locator = buildLocatorLine(hit); + if (locator) lines.push(` ${locator}`); + + // Title is suppressed when it's literally the locator we just + // printed; the response builder already drops `qualifiedPath` when + // it equals `title`, so we don't double-check that here. + if (hit.title && hit.title !== hit.locator.filePath) { + lines.push(` ${hit.title}`); + } + + if (hit.summary) { + for (const wrapped of wrapText(hit.summary, SUMMARY_WRAP_WIDTH)) { + lines.push(` ${wrapped}`); + } + } +} + +/** + * Compact, agent-friendly type label. + * + * Backend types are uppercase enum-style; the JSON envelope already + * lowercases them. Text mode further compacts to a single token so a + * reader can scan the third column quickly. + */ +function shortType(type: string): string { + switch (type) { + case "repository_code": + return "code"; + case "repository_symbol": + return "symbol"; + case "documentation_page": + return "docs"; + case "repository_doc": + return "repo-docs"; + default: + return type; + } +} + +function buildLocatorLine(hit: UnifiedSearchHitPayload): string { + const loc = hit.locator; + if (loc.filePath) { + let line = `${loc.filePath}${formatLineRange(loc.startLine, loc.endLine)}`; + const tail: string[] = []; + if (loc.qualifiedPath) tail.push(loc.qualifiedPath); + if (loc.kind) tail.push(loc.kind); + if (tail.length > 0) line += ` ${tail.join(SEP)}`; + return line; + } + if (loc.pageId) return `pageId: ${loc.pageId}`; + if (loc.sourceUrl) return loc.sourceUrl; + return ""; +} + +function formatLineRange(start?: number, end?: number): string { + if (typeof start !== "number") return ""; + if (typeof end !== "number" || end === start) return `:${start}`; + return `:${start}-${end}`; +} + +function formatScore(score: number): string { + return score.toFixed(2); +} + +function buildTrailer(payload: SearchSuccessPayload): string[] { + const lines: string[] = []; + + if (payload.hasMore) { + const nextOffsetHint = + typeof payload.nextOffset === "number" + ? ` Pass offset=${payload.nextOffset} for the next page or limit=N to widen.` + : " Pass limit=N to widen."; + lines.push(`More hits available.${nextOffsetHint}`); + } + + if (!payload.completed && payload.searchRef) { + lines.push( + `Indexing in progress. Call search_status with searchRef=${payload.searchRef} to follow up.`, + ); + } + + if (payload.sourceStatus && payload.sourceStatus.length > 0) { + lines.push("source notes:"); + for (const entry of payload.sourceStatus) { + lines.push(` - ${formatSourceStatus(entry)}`); + } + } + + return lines; +} + +function formatSourceStatus(entry: { + source: string; + targetLabel: string; + indexingStatus?: string; + codeIndexState?: string; + ignoredFilters?: string[]; + incompatibleFilters?: string[]; + ignoredQueryFeatures?: string[]; + incompatibleQueryFeatures?: string[]; + note?: string; +}): string { + const parts: string[] = [`${entry.source} (${entry.targetLabel})`]; + if (entry.indexingStatus) parts.push(`indexing=${entry.indexingStatus}`); + if (entry.codeIndexState) parts.push(`codeIndex=${entry.codeIndexState}`); + if (entry.ignoredFilters?.length) { + parts.push(`ignored=${entry.ignoredFilters.join(",")}`); + } + if (entry.incompatibleFilters?.length) { + parts.push(`incompatible=${entry.incompatibleFilters.join(",")}`); + } + if (entry.ignoredQueryFeatures?.length) { + parts.push(`ignoredQuery=${entry.ignoredQueryFeatures.join(",")}`); + } + if (entry.incompatibleQueryFeatures?.length) { + parts.push( + `incompatibleQuery=${entry.incompatibleQueryFeatures.join(",")}`, + ); + } + if (entry.note) parts.push(entry.note); + return parts.join(SEP); +} + +function quote(value: string): string { + // Use single quotes when the value already contains a double quote; + // agents read either form. JSON-escape would be over-engineering for + // a header. + return value.includes('"') ? `'${value}'` : `"${value}"`; +} + +function formatDetailValue(value: unknown): string { + if (value === null || value === undefined) return ""; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") + return String(value); + return JSON.stringify(value); +} + +function wrapText(text: string, width: number): string[] { + const lines: string[] = []; + for (const paragraph of text.split(/\n/)) { + if (paragraph.length === 0) { + lines.push(""); + continue; + } + let remaining = paragraph.trim(); + while (remaining.length > width) { + let breakAt = remaining.lastIndexOf(" ", width); + if (breakAt <= 0) breakAt = width; + lines.push(remaining.slice(0, breakAt).trimEnd()); + remaining = remaining.slice(breakAt).trimStart(); + } + if (remaining.length > 0) lines.push(remaining); + } + return lines; +} diff --git a/src/tools/grep-repo-parity.test.ts b/src/tools/grep-repo-parity.test.ts index 58cc1d9b..46d5a9b6 100644 --- a/src/tools/grep-repo-parity.test.ts +++ b/src/tools/grep-repo-parity.test.ts @@ -81,6 +81,7 @@ interface McpArgs { pattern: string; path_prefix?: string; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } async function mcpJson( @@ -91,7 +92,10 @@ async function mcpJson( grepRepoMock ? { grepRepo: grepRepoMock as never } : {}, ); const tool = createGrepRepoTool(service); - const result = await tool.handler(args, {}); + // Parity is asserted against the JSON envelope. The MCP default is + // text-v1, so this helper opts into JSON to match the CLI `--json` + // payload shape. + const result = await tool.handler({ ...args, format: "json" }, {}); return JSON.parse(result.content[0]?.text ?? ""); } diff --git a/src/tools/grep-repo.test.ts b/src/tools/grep-repo.test.ts index cbde27bf..057fb90d 100644 --- a/src/tools/grep-repo.test.ts +++ b/src/tools/grep-repo.test.ts @@ -27,6 +27,7 @@ describe("createGrepRepoTool — metadata", () => { "exclude_doc_files", "exclude_test_files", "extensions", + "format", "globs", "max_matches", "max_matches_per_file", @@ -112,12 +113,13 @@ describe("createGrepRepoTool — happy path", () => { ); }); - it("emits the new envelope shape", async () => { + it("emits the JSON envelope shape when format=json", async () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); const result = await tool.handler( { target: { registry: "npm", package_name: "express" }, pattern: "middleware", + format: "json", }, {}, ); @@ -140,6 +142,53 @@ describe("createGrepRepoTool — happy path", () => { }); }); +describe("createGrepRepoTool — text format", () => { + it("defaults to text output when format is omitted", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | 1 match in 1 file"); + expect(text).toContain('pattern="middleware"'); + expect(text).toContain("src/index.js (1)"); + expect(() => JSON.parse(text)).toThrow(); + }); + + it("renders text output when format=text-v1", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + format: "text-v1", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | "); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | "); + }); +}); + describe("createGrepRepoTool — validation errors", () => { it("returns INVALID_ARGUMENT for empty pattern", async () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); diff --git a/src/tools/grep-repo.ts b/src/tools/grep-repo.ts index 950f3b0c..70388b51 100644 --- a/src/tools/grep-repo.ts +++ b/src/tools/grep-repo.ts @@ -8,6 +8,7 @@ import { GREP_REPO_SYMBOL_FIELDS, GREP_REPO_SYMBOL_FIELDS_NOTE, type GrepRepoSymbolField, + renderGrepRepoText, } from "../shared/index.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { @@ -36,6 +37,7 @@ export interface GrepRepoArgs { cursor?: string; symbol_fields?: GrepRepoSymbolField[]; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -78,13 +80,20 @@ const schema = { .optional() .describe(GREP_REPO_SYMBOL_FIELDS_NOTE), wait_timeout_ms: z.number().optional(), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact line-oriented output (matches grouped by file with grep -A/-B notation for context). Pass `format: "json"` for the structured envelope. `text` is an alias for `text-v1`. Errors stay JSON-formatted in either mode for now.', + ), }; const DESCRIPTION = "Deterministic text grep over indexed dependency and repository source files. " + "Use this when you know the text pattern you want; use `search` for discovery. " + - "Whole-target grep is the default. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. " + - "Matches chain directly into `code_read` via `matches[].filePath`."; + "Whole-target grep is the default — narrow with `path`, `path_prefix`, `globs`, or `extensions` to keep responses small. " + + 'Default response is a compact line-oriented listing (`format: "text-v1"`); pass `format: "json"` for the structured envelope. ' + + "Matches chain directly into `code_read` (the `path` and `line` from each match feed straight into `start_line` / `end_line`)."; export function createGrepRepoTool( service: CodeNavigationService, @@ -146,6 +155,9 @@ export function createGrepRepoTool( excludeTestFiles: build.params.excludeTestFiles, explicit: build.explicit, }); + if (isTextFormat(args.format)) { + return textResult(renderGrepRepoText(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -161,3 +173,11 @@ export function createGrepRepoTool( }, }; } + +/** + * Default response format is text-v1; programmatic callers opt into + * JSON explicitly via `format: "json"`. + */ +function isTextFormat(format: GrepRepoArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +} diff --git a/src/tools/list-files-parity.test.ts b/src/tools/list-files-parity.test.ts index 3eb4d944..21d07896 100644 --- a/src/tools/list-files-parity.test.ts +++ b/src/tools/list-files-parity.test.ts @@ -1,6 +1,8 @@ // PARITY TEST — enforces: -// PARITY-JSON-KEYS CLI --json output and MCP text payload parse to -// deepEqual JSON objects for equivalent inputs. +// PARITY-JSON-KEYS CLI --json output and MCP `format: "json"` payload +// parse to deepEqual JSON objects for equivalent +// inputs. The MCP default is `text-v1`; this helper +// opts into JSON to compare like-for-like. // PARITY-ERROR-ENVELOPE Both surfaces emit { error, code, retryable, details? }. import { describe, expect, it, mock, spyOn } from "bun:test"; @@ -84,6 +86,7 @@ interface McpArgs { path_prefix?: string; limit?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } async function mcpJson( @@ -94,7 +97,10 @@ async function mcpJson( listFilesMock ? { listFiles: listFilesMock as never } : {}, ); const tool = createListFilesTool(service); - const result = await tool.handler(args, {}); + // Parity is asserted against the JSON envelope. The MCP default is + // text-v1, so this helper opts into JSON to match the CLI `--json` + // payload shape. + const result = await tool.handler({ ...args, format: "json" }, {}); return JSON.parse(result.content[0]?.text ?? ""); } diff --git a/src/tools/list-files.test.ts b/src/tools/list-files.test.ts index 4f394d5a..f5db2ab4 100644 --- a/src/tools/list-files.test.ts +++ b/src/tools/list-files.test.ts @@ -19,6 +19,7 @@ describe("createListFilesTool — metadata", () => { expect(tool.name).toBe("code_files"); expect(tool.description).toContain("List files in an indexed dependency"); expect(Object.keys(tool.schema).sort()).toEqual([ + "format", "limit", "path_prefix", "target", @@ -51,7 +52,7 @@ describe("createListFilesTool — happy path", () => { it("emits the envelope with files, total, hasMore, resolution, indexedVersion", async () => { const tool = createListFilesTool(createMockCodeNavigationService()); const result = await tool.handler( - { target: { registry: "npm", package_name: "express" } }, + { target: { registry: "npm", package_name: "express" }, format: "json" }, {}, ); expect(result.isError).toBeUndefined(); @@ -81,6 +82,7 @@ describe("createListFilesTool — happy path", () => { repo_url: "https://github.com/expressjs/express", git_ref: "main", }, + format: "json", }, {}, ); @@ -102,6 +104,7 @@ describe("createListFilesTool — happy path", () => { { target: { registry: "npm", package_name: "express" }, path_prefix: "src/", + format: "json", }, {}, ); @@ -114,7 +117,7 @@ describe("createListFilesTool — happy path", () => { it("omits filter when caller only used defaults", async () => { const tool = createListFilesTool(createMockCodeNavigationService()); const result = await tool.handler( - { target: { registry: "npm", package_name: "express" } }, + { target: { registry: "npm", package_name: "express" }, format: "json" }, {}, ); const payload = parseText(result) as { filter?: unknown }; @@ -210,3 +213,62 @@ describe("createListFilesTool — service errors", () => { expect(payload.code).toBe("NOT_FOUND"); }); }); + +describe("createListFilesTool — text format", () => { + it("defaults to text output when format is omitted", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { target: { registry: "npm", package_name: "express" } }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths"); + // Confirm text payload is not valid JSON (proves text default). + expect(() => JSON.parse(text)).toThrow(); + }); + + it("returns line-oriented text when format=text-v1", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "text-v1", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths | npm:express@v5.2.1"); + expect(text).toContain("src/index.js"); + // Confirm the text payload is not valid JSON. + expect(() => JSON.parse(text)).toThrow(); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths"); + }); + + it("keeps JSON envelope when format=json (explicit)", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "json", + }, + {}, + ); + const payload = parseText(result) as { registry: string; total: number }; + expect(payload.registry).toBe("npm"); + expect(payload.total).toBe(2); + }); +}); diff --git a/src/tools/list-files.ts b/src/tools/list-files.ts index 5f11f959..722bed71 100644 --- a/src/tools/list-files.ts +++ b/src/tools/list-files.ts @@ -3,6 +3,7 @@ import type { CodeNavigationService } from "../services/index.js"; import { mapCodeNavigationError } from "../shared/code-navigation-error-map.js"; import { buildListFilesParams } from "../shared/list-files-request.js"; import { buildListFilesSuccessPayload } from "../shared/list-files-response.js"; +import { renderListFilesText } from "../shared/list-files-text.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { type CodeTargetArg, @@ -16,6 +17,7 @@ export interface ListFilesArgs { path_prefix?: string; limit?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -38,20 +40,27 @@ const schema = { .describe( "Max milliseconds to wait for indexing (0–60000, default 20000). On an `INDEXING` error envelope, retry with a longer timeout or pass a version from `details.availableVersions`.", ), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact paths-only listing tuned for agent context efficiency. Pass `format: "json"` for the structured envelope. `text` is an alias for `text-v1`. Errors stay JSON-formatted in either mode for now.', + ), }; const DESCRIPTION = - "List files in an indexed dependency. Response: " + - "`{total, hasMore, files: [{path, name, language, fileType, byteSize}], " + - "resolution, indexedVersion}`. Address via `target.registry` + " + - "`target.package_name` (package scope) or `target.repo_url` + " + - "`target.git_ref` (repo scope), mutually exclusive. `path_prefix` " + - "is a literal directory prefix — it does NOT accept globs " + - "(`*.ts`) or extension filters. The returned `path` values feed " + - "directly into `code_read` and help scope `code_grep`. Returns an `INDEXING` " + - "error envelope when the dependency is being indexed on-demand — " + - "retry with a longer `wait_timeout_ms` or use a version from " + - "`details.availableVersions`."; + "List files in an indexed dependency. Default response is a compact " + + 'paths-only listing (`format: "text-v1"`); pass `format: "json"` ' + + "for the structured envelope `{total, hasMore, files: [{path, name, " + + "language, fileType, byteSize}], resolution, indexedVersion}`. " + + "Address via `target.registry` + `target.package_name` (package " + + "scope) or `target.repo_url` + `target.git_ref` (repo scope), " + + "mutually exclusive. `path_prefix` is a literal directory prefix — " + + "it does NOT accept globs (`*.ts`) or extension filters. The " + + "returned paths feed directly into `code_read` and help scope " + + "`code_grep`. Returns an `INDEXING` error envelope when the " + + "dependency is being indexed on-demand — retry with a longer " + + "`wait_timeout_ms` or use a version from `details.availableVersions`."; export function createListFilesTool( service: CodeNavigationService, @@ -85,6 +94,9 @@ export function createListFilesTool( pathPrefix: build.params.pathPrefix, limit: build.params.limit, }); + if (isTextFormat(args.format)) { + return textResult(renderListFilesText(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -100,3 +112,11 @@ export function createListFilesTool( }, }; } + +/** + * Default response format is text-v1; programmatic callers opt into + * JSON explicitly via `format: "json"`. + */ +function isTextFormat(format: ListFilesArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +} diff --git a/src/tools/read-file-parity.test.ts b/src/tools/read-file-parity.test.ts index ba1faaff..0f576d55 100644 --- a/src/tools/read-file-parity.test.ts +++ b/src/tools/read-file-parity.test.ts @@ -91,7 +91,18 @@ async function mcpJson( ); const tool = createReadFileTool(service); const result = await tool.handler(args, {}); - return JSON.parse(result.content[0]?.text ?? ""); + const parsed = JSON.parse(result.content[0]?.text ?? "") as Record< + string, + unknown + >; + // The MCP surface adds a `hint` field when its per-call span cap + // truncates the request. The cap is intentionally MCP-only — the + // CLI command path honors arbitrary ranges so humans can pipe + // whole files. Strip the field here so the envelope shapes match + // for parity comparison; cap behavior is covered by + // `read-file.test.ts`. + delete parsed.hint; + return parsed; } describe("read_file parity", () => { diff --git a/src/tools/read-file.test.ts b/src/tools/read-file.test.ts index 4ec866cb..e63eef4b 100644 --- a/src/tools/read-file.test.ts +++ b/src/tools/read-file.test.ts @@ -227,3 +227,241 @@ describe("createReadFileTool — service errors", () => { expect((parseText(result) as { code: string }).code).toBe("INDEXING"); }); }); + +describe("createReadFileTool — span cap", () => { + it("clamps no-range request to 1..MCP_READ_MAX_SPAN before calling backend", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(1); + expect(calls[0]?.[0]?.endLine).toBe(150); + }); + + it("clamps a wide explicit range to start..start+149", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 200, + end_line: 600, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(200); + expect(calls[0]?.[0]?.endLine).toBe(349); + }); + + it("clamps start-only request to start..start+149", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 100, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(100); + expect(calls[0]?.[0]?.endLine).toBe(249); + }); + + it("does not clamp ranges within the cap", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 10, + end_line: 80, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(10); + expect(calls[0]?.[0]?.endLine).toBe(80); + }); + + // Hint tests need a backend that actually returns a >150-line span; + // the default 5-line mock would make the cap a no-op and the hint + // would (correctly) be suppressed. + function wideFileMock( + overrides: { + startLine?: number; + endLine?: number; + totalLines?: number; + } = {}, + ) { + const start = overrides.startLine ?? 1; + const end = overrides.endLine ?? 150; + const total = overrides.totalLines ?? 5000; + return mock(() => + Promise.resolve({ + filePath: "src/index.js", + language: "javascript", + totalLines: total, + startLine: start, + endLine: end, + content: "// big file\n".repeat(end - start + 1), + isBinary: false, + }), + ); + } + + it("emits hint with actual returned range and original request when capping", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock({ + startLine: 1, + endLine: 150, + totalLines: 5000, + }), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 1, + end_line: 600, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeDefined(); + // Returned range comes from the payload (1-150/5000), not the + // pre-clamp request — pre-Codex-fix this rendered the impossible + // "1-150/5" against the 5-line mock. + expect(payload.hint).toContain("Returned lines 1-150/5000"); + expect(payload.hint).toContain("MCP cap: 150 lines"); + expect(payload.hint).toContain("you requested lines 1-600"); + expect(payload.hint).toContain("80-150 lines"); + expect(payload.hint).toContain("retry also costs context"); + }); + + it("emits hint with 'no range' wording when caller passed nothing", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock(), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toContain("you requested no range"); + }); + + it("does not emit hint when range is within the cap", async () => { + const tool = createReadFileTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 10, + end_line: 80, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint when the cap was a no-op (file fits within cap)", async () => { + // Default 5-line mock: caller passed no range, cap clamped the + // request to 1..150 but the backend returned the whole 5-line + // file. No actual truncation happened — hint would point at + // nonexistent lines (Codex review P2). + const tool = createReadFileTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint when the returned range reaches end of file", async () => { + // Caller asked 100-600 against a 200-line file. Cap clamped the + // request to 100..249, backend returned 100..200 (EOF). The + // agent has all the available content already; hint would just + // suggest narrower windows that wouldn't help. + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock({ + startLine: 100, + endLine: 200, + totalLines: 200, + }), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 100, + end_line: 600, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint for binary files even when no range was supplied", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: mock(() => + Promise.resolve({ + filePath: "assets/logo.png", + isBinary: true, + }), + ), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "assets/logo.png", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); +}); diff --git a/src/tools/read-file.ts b/src/tools/read-file.ts index 3fe1cd11..969913c8 100644 --- a/src/tools/read-file.ts +++ b/src/tools/read-file.ts @@ -3,7 +3,10 @@ import type { CodeNavigationService } from "../services/index.js"; import { mapCodeNavigationError } from "../shared/code-navigation-error-map.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { buildReadFileParams } from "../shared/read-file-request.js"; -import { buildReadFileSuccessPayload } from "../shared/read-file-response.js"; +import { + buildReadFileSuccessPayload, + type LeanReadFileEnvelope, +} from "../shared/read-file-response.js"; import { type CodeTargetArg, codeTargetSchema, @@ -11,6 +14,18 @@ import { } from "./code-navigation-shared.js"; import { errorResult, type ToolDefinition, textResult } from "./types.js"; +/** + * Maximum line span the MCP `code_read` tool will return in one call. + * + * Real session traces showed agents requesting 300-600 line windows + * (and occasionally unbounded full-file reads) which dominated + * context cost. The cap forces agents to pick a focused window — + * typical 80-150 lines around a known symbol from `search` or + * `code_grep` results. CLI command `githits code read` bypasses this + * cap so humans piping a whole file to disk still work. + */ +export const MCP_READ_MAX_SPAN = 150; + export interface ReadFileArgs { target: CodeTargetArg; path: string; @@ -29,12 +44,14 @@ const schema = { start_line: z .number() .optional() - .describe("Starting line (1-indexed). Omit for the full file from line 1."), + .describe( + `Starting line (1-indexed). Omit to start at line 1. The MCP surface caps any single read at ${MCP_READ_MAX_SPAN} lines — pick a focused window from your prior \`search\` / \`code_grep\` hit.`, + ), end_line: z .number() .optional() .describe( - "Ending line (inclusive). Omit for end of file. Must be ≥ `start_line` when both are set.", + `Ending line (inclusive). Must be ≥ \`start_line\` when both are set. Omitting it implies \`start_line + ${MCP_READ_MAX_SPAN - 1}\` because the MCP surface caps each read at ${MCP_READ_MAX_SPAN} lines.`, ), wait_timeout_ms: z .number() @@ -45,18 +62,64 @@ const schema = { }; const DESCRIPTION = - "Read a file from an indexed dependency. Default returns the full " + - "file; use `start_line` / `end_line` for a bounded range. Response: " + - "`{path, language, totalLines, startLine, endLine, content, " + - "isBinary}`. Binary files set `isBinary: true` and omit `content` — " + - "agents branch on the flag rather than checking null. Pass the same " + - "`path` emitted by `code_files`. Address via " + - "`target.registry` + `target.package_name` (package scope) or " + - "`target.repo_url` + `target.git_ref` (repo scope), mutually " + - "exclusive. On `INDEXING` retry with a longer `wait_timeout_ms`. " + - "When the path doesn't resolve the response is a `NOT_FOUND` (or " + - "`FILE_NOT_FOUND`) error — call `code_files` to discover the " + - "actual paths."; + "Read a file from an indexed dependency. **MCP cap: " + + `${MCP_READ_MAX_SPAN} lines per call** — broader requests (or no ` + + `range) silently truncate to the first ${MCP_READ_MAX_SPAN} lines ` + + "from your start, with a `hint` describing what was returned vs. " + + "requested. Pick a focused window from a `search` / `code_grep` " + + "match. Response: `{path, language, totalLines, startLine, endLine, " + + "content, isBinary, hint?}`. Binary files set `isBinary: true` and " + + "omit `content`. Pass the same `path` emitted by `code_files`. " + + "Address via `target.registry` + `target.package_name` (package " + + "scope) or `target.repo_url` + `target.git_ref` (repo scope), " + + "mutually exclusive. On `INDEXING` retry with a longer " + + "`wait_timeout_ms`. On `NOT_FOUND` / `FILE_NOT_FOUND` call " + + "`code_files` to discover the actual path."; + +interface BoundedRange { + startLine: number; + endLine: number; + capped: boolean; +} + +/** + * Compute the effective `(startLine, endLine)` for the backend call, + * enforcing the MCP per-call span cap. + * + * - No range supplied: `1..MCP_READ_MAX_SPAN`. + * - `start_line` only: `start..start + MCP_READ_MAX_SPAN - 1`. + * - `end_line` only: treat start as 1; if span > cap, clamp. + * - Both supplied with span ≤ cap: untouched. + * - Both supplied with span > cap: clamp to `start..start + cap - 1`. + * + * The cap is enforced before the backend call so the service does not + * have to transfer bytes that will be discarded. + */ +export function deriveBoundedRange( + startLine: number | undefined, + endLine: number | undefined, +): BoundedRange { + const start = startLine ?? 1; + + if (endLine === undefined) { + return { + startLine: start, + endLine: start + MCP_READ_MAX_SPAN - 1, + capped: true, + }; + } + + const span = endLine - start + 1; + if (span > MCP_READ_MAX_SPAN) { + return { + startLine: start, + endLine: start + MCP_READ_MAX_SPAN - 1, + capped: true, + }; + } + + return { startLine: start, endLine, capped: false }; +} export function createReadFileTool( service: CodeNavigationService, @@ -71,11 +134,15 @@ export function createReadFileTool( if ("content" in target) return target; try { + // Cap before the backend call so we don't transfer bytes only + // to throw them away. CLI surface bypasses this — see the + // MCP_READ_MAX_SPAN doc-comment for rationale. + const bounded = deriveBoundedRange(args.start_line, args.end_line); const build = buildReadFileParams({ target, filePath: args.path, - startLine: args.start_line, - endLine: args.end_line, + startLine: bounded.startLine, + endLine: bounded.endLine, waitTimeoutMs: args.wait_timeout_ms, }); const result = await service.readFile(build.params); @@ -88,6 +155,15 @@ export function createReadFileTool( gitRef: target.gitRef, requestedFilePath: build.params.filePath, }); + + if (shouldEmitCappedHint(bounded, payload)) { + payload.hint = buildCappedHint( + payload, + args.start_line, + args.end_line, + ); + } + return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -103,3 +179,59 @@ export function createReadFileTool( }, }; } + +/** + * Whether to emit the cap hint. + * + * The hint is only useful when the response was actually truncated — + * i.e., the caller's intent ran past the end of what came back. If + * the caller asked for the whole file but the file fits within the + * cap, the response is the whole file and the hint would point at + * lines that don't exist (real bug found by Codex review). + * + * Suppression cases: + * - Cap clamp logic didn't fire at all. + * - Binary file (hint is irrelevant). + * - Backend didn't echo `endLine` / `totalLines` (we can't tell). + * - The returned end IS the end of the file. + */ +function shouldEmitCappedHint( + bounded: BoundedRange, + payload: LeanReadFileEnvelope, +): boolean { + if (!bounded.capped) return false; + if (payload.isBinary) return false; + if (payload.endLine === undefined) return false; + if (payload.totalLines === undefined) return false; + return payload.endLine < payload.totalLines; +} + +function buildCappedHint( + payload: LeanReadFileEnvelope, + originalStart: number | undefined, + originalEnd: number | undefined, +): string { + const requested = describeRequest(originalStart, originalEnd); + return ( + `Returned lines ${payload.startLine}-${payload.endLine}/${payload.totalLines} ` + + `(MCP cap: ${MCP_READ_MAX_SPAN} lines per call; you requested ${requested}). ` + + `Pick a focused start_line/end_line window — typical 80-150 lines around a search/code_grep match. ` + + `Each retry also costs context, so aim for one well-sized read.` + ); +} + +function describeRequest( + originalStart: number | undefined, + originalEnd: number | undefined, +): string { + if (originalStart === undefined && originalEnd === undefined) { + return "no range"; + } + if (originalEnd === undefined) { + return `start_line=${originalStart}, no end_line`; + } + if (originalStart === undefined) { + return `end_line=${originalEnd}, no start_line`; + } + return `lines ${originalStart}-${originalEnd}`; +} diff --git a/src/tools/search.test.ts b/src/tools/search.test.ts index 337e3e3a..39d45d2b 100644 --- a/src/tools/search.test.ts +++ b/src/tools/search.test.ts @@ -14,6 +14,7 @@ describe("searchTool", () => { { query: "router middleware", target: { registry: "npm", package_name: "express" }, + format: "json", }, {}, ); @@ -123,6 +124,7 @@ describe("searchTool", () => { { query: "middleware", target: { registry: "npm", package_name: "express" }, + format: "json", }, {}, ); @@ -138,4 +140,65 @@ describe("searchTool", () => { expect(payload.results[0]).not.toHaveProperty("followUp"); expect(payload.results[0]).not.toHaveProperty("alternateFollowUps"); }); + + it("defaults to text output when format is omitted", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router middleware", + target: { registry: "npm", package_name: "express" }, + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + expect(() => JSON.parse(text)).toThrow(); + }); + + it("renders text output when format=text-v1", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router middleware", + target: { registry: "npm", package_name: "express" }, + format: "text-v1", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + expect(text).toContain('query="router middleware"'); + // Confirm the text payload is not valid JSON. + expect(() => JSON.parse(text)).toThrow(); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router", + target: { registry: "npm", package_name: "express" }, + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + }); + + it("keeps the JSON envelope when format=json (explicit)", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router", + target: { registry: "npm", package_name: "express" }, + format: "json", + }, + {}, + ); + const payload = JSON.parse(result.content[0]?.text ?? "{}"); + expect(payload.completed).toBe(true); + }); }); diff --git a/src/tools/search.ts b/src/tools/search.ts index 70c0af31..7a0ede98 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -4,6 +4,8 @@ import { buildUnifiedSearchErrorPayload, buildUnifiedSearchParams, buildUnifiedSearchSuccessPayload, + renderUnifiedSearchError, + renderUnifiedSearchSuccess, toFileIntent, toSymbolCategory, toSymbolKind, @@ -72,6 +74,7 @@ export interface SearchArgs { limit?: number; offset?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -150,6 +153,12 @@ const schema = { limit: z.coerce.number().int().min(1).max(100).optional(), offset: z.coerce.number().int().min(0).optional(), wait_timeout_ms: z.coerce.number().int().min(0).max(60000).optional(), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact line-oriented output tuned for agent context efficiency. Pass `format: "json"` for the structured envelope (programmatic consumers, parity testing). `text` is an alias for `text-v1`. The text format is a public, snapshot-tested contract.', + ), }; const DESCRIPTION = @@ -217,11 +226,16 @@ export function createSearchTool( built.compiledQuery, outcome, ); + if (isTextFormat(args.format)) { + return textResult(renderUnifiedSearchSuccess(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { - return errorResult( - JSON.stringify(buildUnifiedSearchErrorPayload(error)), - ); + const payload = buildUnifiedSearchErrorPayload(error); + if (isTextFormat(args.format)) { + return errorResult(renderUnifiedSearchError(payload)); + } + return errorResult(JSON.stringify(payload)); } }, }; @@ -232,3 +246,12 @@ function isResolvedCodeTarget( ): target is ResolvedCodeTarget { return !("content" in target); } + +/** + * Default response format is text-v1 — agents consume the MCP surface + * and benefit from the compact form. Programmatic / parity callers + * opt into JSON explicitly. + */ +function isTextFormat(format: SearchArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +}