From acfd171151c56bfc245c07fb1e15685de34d104f Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 27 Apr 2026 13:34:13 +0300 Subject: [PATCH 1/2] refactor: agent-friendly tool naming and leaner search payloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshapes the MCP and CLI surfaces so agents and humans see the same vocabulary, and trims the JSON envelopes so each tool response says only what's actually useful. MCP tool names now mirror the CLI command path (`_`): list_files → code_files read_file → code_read grep_repo → code_grep package_summary → pkg_info package_vulnerabilities → pkg_vulns package_dependencies → pkg_deps package_changelog → pkg_changelog list_package_docs → docs_list read_package_doc → docs_read Top-level tools (`search`, `search_status`, `get_example`, `search_language`, `feedback`) keep their names since they have no CLI group to mirror. The mental model collapses to one rule: MCP tool name = CLI command path with `_` separator. The legacy symbol-search surface goes with the rename. The unified `search` tool with `sources:["symbol"]` covers the same use case, so the duplicate `code search-symbols` CLI command, `search_symbols` MCP tool, `searchSymbols` service method, and their request / response / test scaffolding are removed (~800 LOC). Internal types follow: `SearchSymbolsKind` → `SymbolKind`, `SearchSymbolsFileIntent` → `FileIntent`, `SearchSymbolsResolution` → `IndexResolution`. Search response is leaner. Hits drop the dead-weight `symbolRef` and `fileContentHash` (no follow-up tool consumed them) and now omit `qualifiedPath` when it equals `title` (the common case for top-level symbols). The `query` echo drops the redundant `targets` field, omits `compiled` when it equals `raw`, and only includes `limit` / `offset` / `waitTimeoutMs` / `allowPartialResults` when the caller set them. `progress` slims to status / counters / elapsed / expiry — no more duplicating top-level fields — and is omitted entirely on completed responses. `sourceStatus` entries are omitted unless something actionable is reported (ignored or incompatible filters / query features, non-healthy lifecycle states, free-form notes). Empty arrays and default-valued scalars fall out everywhere they don't change semantics. Hits also lose the redundant `followUp` / `alternateFollowUps` objects: each hit's `type` plus its `locator` already tells agents which tool to call next, and the `search` description now teaches that mapping in one sentence. `docs_list` entries lose the same duplicate `followUp` / `readFile` companion objects. `docs_read` / `docs read` gains `start_line` / `end_line` (MCP) and `--lines 10-40` (CLI) so long pages don't blow context windows. Slicing happens client-side for now and the response carries `totalLines` so the next slice can be targeted. `example` / `get_example` now surface `solution_id` as a top-level field on the JSON payload — extracted from the markdown body URL — so agents can pass it straight to `feedback` without scraping prose. The always-on tools (`get_example`, `search_language`, `feedback`) get the structured `{error, code, retryable}` envelope used by the capability-gated tools, so error handling is uniform across the surface. Smaller polish: - `githits --help` now eagerly registers `search` / `search-status` next to the other capability-gated groups; previous gate was stricter than the docs claimed. - `code files` / `code grep` show descriptive positional names (`spec-or-prefix`, `path-prefix`, `spec-or-pattern`, …) instead of `arg1` / `arg2` / `arg3`. - `search` description trims an explicit syntax/qualifier table down to a single sentence; examples stay. - `example` / `feedback` / `search_language` MCP descriptions drop Args/Returns boilerplate that duplicated the schema. - `search_language` now emits compact JSON (was pretty-printed). - `githits --no-color` no longer confuses the gated-group sniffer; `auth status` no longer prints the env-token prefix. - `mcp-cli-parity.md`, `tools.md`, and `cli-commands.md` updated to the new tool names and envelope shape. Bundle: 308 KB → ~290 KB raw (71 KB → 67 KB gzip). All 1376 tests pass. --- docs/implementation/cli-commands.md | 44 +- docs/implementation/config.md | 2 +- docs/implementation/mcp-cli-parity.md | 463 ++++------ docs/implementation/tools.md | 76 +- src/cli.ts | 43 +- src/commands/auth-status.test.ts | 7 +- src/commands/auth-status.ts | 1 - src/commands/code/files.ts | 8 +- src/commands/code/grep.ts | 12 +- src/commands/code/index.test.ts | 4 +- src/commands/code/index.ts | 4 +- src/commands/code/search-symbols.test.ts | 754 ----------------- src/commands/code/search-symbols.ts | 525 ------------ src/commands/docs/list.test.ts | 4 +- src/commands/docs/read.test.ts | 2 +- src/commands/docs/read.ts | 14 +- src/commands/example.ts | 14 +- src/commands/mcp-instructions.test.ts | 66 +- src/commands/mcp-instructions.ts | 58 +- src/commands/mcp.test.ts | 46 +- src/commands/pkg/info.ts | 3 +- src/commands/search.test.ts | 3 +- src/commands/search.ts | 120 +-- src/services/code-navigation-service.test.ts | 801 ------------------ src/services/code-navigation-service.ts | 418 +-------- src/services/index.ts | 11 +- src/services/test-helpers.ts | 26 - src/shared/code-navigation-defaults.ts | 2 +- src/shared/code-navigation-error-map.test.ts | 12 - src/shared/code-navigation.ts | 28 +- src/shared/docs-follow-up.ts | 42 +- src/shared/extract-solution-id.ts | 18 + src/shared/grep-repo-response.test.ts | 4 +- src/shared/grep-repo-response.ts | 50 +- src/shared/index.ts | 27 +- src/shared/list-package-docs-response.ts | 36 +- src/shared/package-summary-request.ts | 4 +- .../package-vulnerabilities-response.test.ts | 10 +- .../package-vulnerabilities-response.ts | 13 +- src/shared/parse-lines-option.ts | 67 ++ src/shared/read-package-doc-response.ts | 86 +- src/shared/search-symbols-request.test.ts | 86 -- src/shared/search-symbols-request.ts | 111 --- src/shared/search-symbols-response.test.ts | 115 --- src/shared/search-symbols-response.ts | 150 ---- src/shared/unified-search-request.test.ts | 4 - src/shared/unified-search-request.ts | 39 +- src/shared/unified-search-response.test.ts | 50 +- src/shared/unified-search-response.ts | 400 +++++---- src/tools/feedback.ts | 21 +- src/tools/get-example.ts | 12 +- src/tools/grep-repo.test.ts | 11 +- src/tools/grep-repo.ts | 8 +- src/tools/index.ts | 1 - src/tools/list-files.test.ts | 2 +- src/tools/list-files.ts | 4 +- src/tools/list-package-docs.test.ts | 2 +- src/tools/list-package-docs.ts | 6 +- src/tools/package-changelog.test.ts | 2 +- src/tools/package-changelog.ts | 2 +- src/tools/package-dependencies-parity.test.ts | 3 +- src/tools/package-dependencies.test.ts | 2 +- src/tools/package-dependencies.ts | 6 +- src/tools/package-summary-parity.test.ts | 2 +- src/tools/package-summary.test.ts | 2 +- src/tools/package-summary.ts | 4 +- .../package-vulnerabilities-parity.test.ts | 3 +- src/tools/package-vulnerabilities.test.ts | 2 +- src/tools/package-vulnerabilities.ts | 6 +- src/tools/read-file.test.ts | 2 +- src/tools/read-file.ts | 15 +- src/tools/read-package-doc.test.ts | 9 +- src/tools/read-package-doc.ts | 26 +- src/tools/search-language.ts | 13 +- src/tools/search-status.test.ts | 3 +- src/tools/search-symbols-parity.test.ts | 239 ------ src/tools/search-symbols.test.ts | 211 ----- src/tools/search-symbols.ts | 237 ------ src/tools/search.test.ts | 19 +- src/tools/search.ts | 16 +- src/tools/shared.ts | 31 +- 81 files changed, 1030 insertions(+), 4775 deletions(-) delete mode 100644 src/commands/code/search-symbols.test.ts delete mode 100644 src/commands/code/search-symbols.ts create mode 100644 src/shared/extract-solution-id.ts create mode 100644 src/shared/parse-lines-option.ts delete mode 100644 src/shared/search-symbols-request.test.ts delete mode 100644 src/shared/search-symbols-request.ts delete mode 100644 src/shared/search-symbols-response.test.ts delete mode 100644 src/shared/search-symbols-response.ts delete mode 100644 src/tools/search-symbols-parity.test.ts delete mode 100644 src/tools/search-symbols.test.ts delete mode 100644 src/tools/search-symbols.ts diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index b9ebc586..91b0e9d6 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -14,7 +14,6 @@ The CLI exposes three always-on top-level commands: `example`, `languages`, and | `search-status ` | `` | `--json` | Check progress, fetch partial hits, or fetch final results for a prior unified search | | `languages [query]` | — | `--json` | List or filter supported languages | | `feedback ` | `--accept` or `--reject` | `-m, --message `, `--json` | Submit feedback on a search result | -| `code search [query]` | package spec | `--keywords`, `--keyword`, `--match-mode`, `--category`, `--kind`, `--file`, `--intent`, `--limit`, `--wait`, `--json` | Search indexed dependency source code | | `pkg info ` | package spec | `--verbose`, `--json` | Show a package overview (latest version, downloads, license, vulnerabilities) | | `pkg vulns ` | package spec (optional `@version`) | `--severity`, `--include-withdrawn`, `--verbose`, `--json` | List known vulnerabilities for a package (npm/pypi/hex/crates) | | `pkg deps ` | package spec (optional `@version`) | `--groups`, `--lifecycle`, `--transitive`, `--depth`, `--verbose`, `--json` | Analyse dependencies: direct runtime deps, structured groups, optional transitive graph (npm/pypi/hex/crates/vcpkg/zig) | @@ -48,7 +47,7 @@ githits example "react hooks patterns" -l typescript --explain githits example "react hooks patterns" -l typescript --json ``` -Default output is markdown (the API response). With `--explain`, an AI-generated explanation is included alongside the code example. With `--json`, output is `{ "result": "" }`. The MCP `get_example` tool always sends `include_explanation: false` since LLMs don't need the extra context. +Default output is markdown (the API response). With `--explain`, an AI-generated explanation is included alongside the code example. With `--json`, output is `{ "result": "", "solution_id": "" }` (`solution_id` is omitted only if the markdown lacks a solution URL — pass it back to `feedback`). The MCP `get_example` tool always sends `include_explanation: false` since LLMs don't need the extra context. ### `githits search` @@ -62,19 +61,23 @@ githits search "composeArgs" --in npm:lodash --name composeArgs --json Unified search spans indexed dependency and repository code, docs, and explicit symbols. The positional query is the backend discovery syntax, not a raw pass-through to a per-source search engine. It supports implicit `AND`, uppercase `OR`, parentheses, unary `-`, quoted phrases, semantic qualifiers (`kind:`, `category:`, `path:`, `lang:`, `name:`, `intent:`), and routing qualifiers (`registry:`, `package:`, `version:`, `repo:`). Structured flags are compiled together with the query using `AND` semantics before the request reaches the backend. -**Decision guide.** Use `githits example` for canonical cross-project examples. Use `githits search` for indexed dependency/repository search. Use `githits search --source symbol` when you want symbol-shaped unified search without dropping to the older dedicated `code search` surface. +**Decision guide.** Use `githits example` for canonical cross-project examples. Use `githits search` for indexed dependency/repository search. Use `githits search --source symbol` when you want symbol-shaped unified search. **Targets.** `--in ` is repeatable and required. Package targets use `registry:name[@version]` (for example `npm:express`, `pypi:requests@2.32.3`). Repo targets use `https://github.com/org/repo[#ref]`; omitted refs default to `HEAD`. Exact duplicate targets are deduplicated while preserving order. Mixing package and repo targets in the same request is rejected client-side. -**Sources and filters.** `--source docs|code|symbol` is repeatable; omitting it delegates source selection to backend AUTO. Use `--source symbol` when you want symbol-shaped search results without dropping down to the older `code search` surface. `--category` is the broad filter (`callable`, `type`, `module`, `data`, `documentation`); `--kind` is the precise taxonomy. `--path-prefix`, `--intent`, and `--public` narrow the result set further. `--name` and `--lang` compile into query qualifiers instead of becoming separate backend fields. +**Sources and filters.** `--source docs|code|symbol` is repeatable; omitting it delegates source selection to backend AUTO. Use `--source symbol` when you want symbol-shaped search results. `--category` is the broad filter (`callable`, `type`, `module`, `data`, `documentation`); `--kind` is the precise taxonomy. `--path-prefix`, `--intent`, and `--public` narrow the result set further. `--name` and `--lang` compile into query qualifiers instead of becoming separate backend fields. **Intent filter.** When `--intent` is omitted, unified search sends no file-intent filter. Pass `--intent production` or another specific intent only when you want to narrow the result set. Some sources can still ignore `fileIntent`; when they do, the JSON `sourceStatus` block and terminal notes report that explicitly. **Complete-by-default results.** The CLI sends `allowPartialResults: false` unless `--allow-partial` is passed. If indexing does not complete within the wait window, the default behavior returns a `searchRef` and progress summary instead of partial hits. With `--allow-partial`, available hits are included while remaining sources continue indexing. `--wait` is in seconds (0-60, default 20). +The original unified-search plan envisaged hiding partial mode entirely in v1 to make results trustworthy by default. We kept the flag exposed because some agent and CLI flows benefit from "show me what you have so far"; the trust contract is preserved by keeping the *default* false. Users and agents must explicitly opt in, so a vanilla `search` call still cannot return incomplete results by surprise. + **Output.** Plain output preserves backend ranking order. It starts with a lightweight per-type count summary, then shows one result per block. The header line is optimized for scanning and copy-paste follow-up: `target path:range [type] - title`. For file-backed hits, that header can be turned directly into a `githits code read` call because `code read` accepts `path:start-end` suffixes. Summaries are rendered verbatim from the backend response. Labels are: `docs page` (hosted package docs), `repo doc` (documentation-like block from a repository file), `repo code` (code block from a repository file), and `repo symbol` (explicit symbol hit from the repository index). `--json` emits the shared success/error envelope used by the MCP `search` tool, including a full `query` echo for initial searches. -**Highlighting.** The CLI currently highlights headers and badges structurally, but does **not** attempt query-term match highlighting inside summaries. Unified search receives compiled query strings, not structured match spans, so robust highlighting should come from backend-provided match metadata rather than fragile client-side substring guesses. +**Highlighting.** The CLI applies the backend's structured `highlights` spans on titles and summaries, plus structural emphasis on headers and badges. It does **not** attempt client-side substring highlighting for terms the backend did not flag, since the compiled query is not a faithful match spec. + +**Source-status surfacing.** The JSON `sourceStatus` block is always passed through verbatim for debugging. Human-readable output surfaces only the actionable subset: ignored / incompatible filters, ignored / incompatible query features, free-form `note`s, and an `INDEXING` indicator when a source is still indexing on a partial-result payload. `STALE` (served from a slightly old index while a fresh reindex runs) is intentionally not shown in human output — agents and users do not need to second-guess otherwise-correct results, but it remains in JSON for diagnostics. ### `githits search-status` @@ -107,33 +110,6 @@ githits feedback abc123 --accept --message "Solved my problem" --json `--accept` and `--reject` are mutually exclusive (enforced by Commander's `.conflicts()` API). At least one must be provided (validated in the action function). JSON output is `{ "success": true, "message": "..." }`. -### `githits code search` - -This is now the older symbol-search surface. Prefer top-level `githits search --source symbol` for new flows unless you specifically need the legacy code-search UX or its exact JSON contract. - -``` -githits code search npm:express middleware -githits code search npm:express middleware --intent all -githits code search pypi:requests timeout --category callable --limit 10 -githits code search crates:serde Serialize --kind trait --limit 5 -githits code search npm:@types/node Buffer --file src/ --json -githits code search npm:express --keywords "router,handler" --match-mode and -``` - -Finds functions, classes, modules, and doc sections inside an indexed dependency by exact-token matches. Top-level `githits search --source symbol` is the preferred unified surface for symbol-shaped search. `code search` remains available for the older dedicated symbol-search UX and parity contract. - -**Package spec.** `:[@]`. Omit the registry to default to `npm`. Supported registries: `npm`, `pypi`, `hex`, `crates`, `nuget`, `maven`, `zig`, `vcpkg`, `packagist`. Scoped npm names are supported (`npm:@types/node`). - -**Filtering by symbol shape.** Prefer `--category` for broad filtering (`callable`, `type`, `module`, `data`, `documentation`) — it works across the full 27-value kind taxonomy without enumerating individual kinds. Reach for `--kind` when you want a specific construct, e.g. `--kind trait` (Rust) or `--kind namespace` (C#/C++/PHP). - -**Defaults.** No file-intent filter is sent by default. Pass `--intent production` or another specific intent to narrow results; `--intent all` remains accepted as an explicit no-filter alias. `--wait` defaults to 20 seconds (above the p50 indexing time of ~11 s); first-time queries against an unindexed package may need `--wait 60` (the backend ceiling) to block until indexing completes. On an INDEXING error, the response message points out the retry options. - -**Output.** Default terminal output leads each entry with `path:startLine-endLine [kind]`, followed by the symbol name and a 3-line dedented snippet. `--json` emits the shared success/error envelope also produced by the MCP `search_symbols` tool — see [`mcp-cli-parity.md`](./mcp-cli-parity.md) for the wire contract. The command is registered as `code search` with `code search-symbols` as a Commander alias. - -**Capability gate.** The `code` group is registered only when the startup token explicitly carries `code_navigation`, or when `GITHITS_CODE_NAVIGATION=1` is set for local override. - -**Troubleshooting.** Set `GITHITS_DEBUG=code-nav` to emit single-line JSON diagnostics to stderr without query text, tokens, or response bodies. When you need the exact code-navigation wire payload, `GITHITS_DEBUG=code-nav-wire` logs the GraphQL document plus serialized variables, which includes query text and resolved target values; it is explicit-only and is not enabled by `GITHITS_DEBUG=*`. - ### `githits pkg info` ``` @@ -151,7 +127,7 @@ Shows a concise overview for a single package: latest version, license, descript **`--verbose` + `--json`.** `--verbose` has no effect under `--json` — the JSON envelope always carries every field the verbose terminal view exposes (and more). The flag only affects human-readable output. -**Output envelope.** Success payload is hand-crafted for agent token efficiency: `{registry, name, version, description?, license?, homepage?, repository?, publishedAt?, downloads?, github?, install?, usage?, vulnerabilities?, recentChanges?}`. Omitted fields reflect backend nulls, not dropped data. Error envelope: `{error, code, retryable, details?}` — same shape as `search_symbols`, same classifier family. Under `--json` the error envelope is written to **stderr** so stdout stays clean for `jq`. +**Output envelope.** Success payload is hand-crafted for agent token efficiency: `{registry, name, version, description?, license?, homepage?, repository?, publishedAt?, downloads?, github?, install?, usage?, vulnerabilities?, recentChanges?}`. Omitted fields reflect backend nulls, not dropped data. Error envelope: `{error, code, retryable, details?}` — shared classifier family. Under `--json` the error envelope is written to **stderr** so stdout stays clean for `jq`. **Capability gate.** Same as `code`. @@ -361,7 +337,7 @@ Each command follows this pattern: | Shared Module | Used By | |---|---| | `GitHitsService` (via container) | `example`, `languages`, `feedback`, and always-on MCP tools | -| `CodeNavigationService` (via container) | top-level unified `search` / `search-status`, capability-gated MCP indexed-search tools, and `githits code search` | +| `CodeNavigationService` (via container) | top-level unified `search` / `search-status` plus capability-gated MCP indexed-search tools (`search`, `search_status`, `code_files`, `code_read`, `code_grep`) and the `githits code` command group | | `filterLanguages()` from `src/shared/language-filter.ts` | `search_language` MCP tool + `languages` CLI command | | `requireAuth()` from `src/shared/require-auth.ts` | all CLI commands and auth-required MCP tool handlers | diff --git a/docs/implementation/config.md b/docs/implementation/config.md index d793f855..c78ed55d 100644 --- a/docs/implementation/config.md +++ b/docs/implementation/config.md @@ -41,7 +41,7 @@ The container (`src/container.ts`) resolves authentication in priority order: Package/source access is different from the REST endpoints above: - the CLI resolves the package/source service URL from `GITHITS_CODE_NAV_URL`; in the default production environment it falls back to `https://pkgseer.dev`, but custom GitHits environments must set this explicitly -- MCP registration for `search`, `search_status`, `package_*`, `list_files`, `read_file`, and `grep_repo` happens only when the current token explicitly carries `code_navigation` +- MCP registration for `search`, `search_status`, `package_*`, `code_files`, `code_read`, and `code_grep` happens only when the current token explicitly carries `code_navigation` - CLI registration for top-level `search` / `search-status` plus the hidden `githits code` / `githits pkg` groups uses the same capability check, with one local-development escape hatch: `GITHITS_CODE_NAVIGATION=1` - if the capability is absent or unknown, those indexed tools and command groups are omitted from the surfaced interface diff --git a/docs/implementation/mcp-cli-parity.md b/docs/implementation/mcp-cli-parity.md index 471ed39f..2d0a224b 100644 --- a/docs/implementation/mcp-cli-parity.md +++ b/docs/implementation/mcp-cli-parity.md @@ -2,35 +2,25 @@ ## Purpose -This document started with `search_symbols` ↔ `githits code search`, -then expanded into the parity pattern used by the rest of the hidden -package/code tooling. Users and agents should be able to cross the -surface boundary without learning a new payload shape — parameter names -can differ per surface convention, but defaults, error behaviour, and -the serialised envelopes do not. - -`search_symbols` / `githits code search` remain a valid parity pair, but -they are no longer the preferred product entry point for symbol-shaped -search. New user-facing guidance should prefer unified top-level -`search` with `source=symbol` / `sources:["symbol"]` unless the older -dedicated symbol-search contract is specifically required. - -Top-level unified `search` and `search_status` follow the same pattern, -with one deliberate exception: `search_status` does not echo the -original structured request because the backend follow-up endpoint does -not expose the caller's original targets, filters, or defaulted fields. - -This document is **the pattern and checklist derived from -`search_symbols`**, not a permanent contract for every future -code-navigation tool. When tool #2 lands with a good reason to break a -rule, extend the rule or add a new one here rather than bending tool -#1's shape to fit. +Users and agents should be able to cross the surface boundary without +learning a new payload shape — parameter names can differ per surface +convention, but defaults, error behaviour, and the serialised +envelopes do not. + +The dual-surface tools today are unified `search` / `search_status`, +the file-exploration bundle (`code_files`, `code_read`, `code_grep`), +the package-intelligence tools (`pkg_info`, +`pkg_vulns`, `pkg_deps`, `pkg_changelog`), +and the docs surface (`docs_list`, `docs_read`). + +One deliberate exception: `search_status` does not echo the original +structured request because the backend follow-up endpoint does not +expose the caller's original targets or filters. ## Rule IDs -Rule IDs are cited from parity tests (e.g. -`src/tools/search-symbols-parity.test.ts`) in file header comments so -the test suite anchors the doc. +Rule IDs are cited from parity tests in file header comments so the +test suite anchors the doc. ### `PARITY-NAMING` @@ -38,13 +28,12 @@ the test suite anchors the doc. see; the JSON-schema description is the primary UX. - **CLI flags** use `--kebab-case`. They are the user-facing surface. `allow_partial_results` maps to CLI `--allow-partial` because the CLI - name reads better as a command flag while preserving the same behavior. + name reads better as a command flag while preserving the same behaviour. - **Public enum values** are lowercase strings on both surfaces (`production`, `test`, `summary`, `all`). - **Service coercion** from lowercase enum values to the internal request enums lives in `src/shared/code-navigation.ts` - (`toSearchSymbolsFileIntent`, `toSearchSymbolsKind`, - `toSearchSymbolsMatchMode`). + (`toFileIntent`, `toSymbolKind`, `toSymbolCategory`). ### `PARITY-DEFAULTS` @@ -52,27 +41,21 @@ the test suite anchors the doc. `src/shared/code-navigation-defaults.ts`. They never diverge silently. - Cross-tool defaults (e.g. `DEFAULT_WAIT_TIMEOUT_MS`) live without a - prefix. Tool-local sentinels that affect request shaping (for example - `FILE_INTENT_ALL`) also live there so both surfaces translate them the - same way. + prefix. Tool-local sentinels live there too so both surfaces translate + them the same way. - When a surface fills in a default for the caller, that default value - is applied at the shared request builder - (`buildSearchSymbolsParams`) — not at the surface — so both - surfaces apply defaults at the same point and under the same + is applied at the shared request builder — not at the surface — so + both surfaces apply defaults at the same point and under the same conditions. ### `PARITY-REQUEST` - Request construction for a dual-surface tool routes through a single - shared helper (`buildSearchSymbolsParams`). The helper: - 1. Fills in defaults, - 2. Translates user-facing sentinel values (e.g. `FILE_INTENT_ALL`) - into their wire-level equivalents, - 3. Returns a `defaulted` array naming the fields that were - client-applied, which feeds the response `query.defaulted` echo. + shared helper at `src/shared/-request.ts`. The helper fills in + defaults and translates user-facing sentinel values into wire-level + equivalents. - Cross-tool helpers (error classification, target resolution) live in - `src/shared/` without a tool-name prefix. Per-tool request builders - live in `src/shared/-request.ts`. + `src/shared/` without a tool-name prefix. ### `PARITY-JSON-KEYS` @@ -82,14 +65,8 @@ the test suite anchors the doc. whitespace, and trailing newlines are free. - **No leading-underscore keys.** `warning`, `hint`, and all other status fields are plain. -- `query` echoes the resolved request parameters. `query.defaulted` is - a string array naming the fields whose values the client filled in. - Empty array when every field was caller-set. -- `fileIntent` is echoed as a lowercase enum value, or the literal - `"all"` when no file-intent filter was applied. -- `returnedCount` is an explicit echo of `results.length`. -- `totalMatches` is the service-provided total (equal to - `returnedCount` today). +- Empty arrays and default-valued scalars are omitted in favour of + field absence wherever that does not change agent semantics. - Initial unified `search` responses include the full compiled request echo. Follow-up `search_status` responses intentionally omit that echo and return only backend-known fields: @@ -102,10 +79,10 @@ the test suite anchors the doc. ### `PARITY-ERROR-ENVELOPE` - Every error result, on both surfaces, carries - `{ error: string, code: MappedErrorCode, details?: object }`. + `{ error: string, code: MappedErrorCode, retryable?: boolean, details?: object }`. - `code` is mandatory. `UNKNOWN` is a last resort — every named error - class in the code-navigation stack maps to a specific code. The - classifier is tested by table in + class in the code-navigation and package-intelligence stacks maps to a + specific code. The classifier is tested by table in `src/shared/code-navigation-error-map.test.ts`; that test is the enforcement mechanism, not a convention. - MCP error text is always valid JSON. A client that parses @@ -128,15 +105,16 @@ When a new tool lands with both MCP and CLI surfaces: `src/shared/code-navigation-defaults.ts` with a `TOOLNAME_` prefix. - [ ] Request builder at `src/shared/-request.ts`. Both surfaces import it. -- [ ] Error classifier reused (`mapCodeNavigationError`). Add new - `MappedErrorCode` variants only when a genuinely new error class - exists; cover the new branch in the table test. +- [ ] Error classifier reused (`mapCodeNavigationError` / + `mapPackageIntelligenceError`). Add new `MappedErrorCode` variants + only when a genuinely new error class exists; cover the new branch + in the table test. - [ ] Response builder at `src/shared/-response.ts` emitting the shared success and error envelopes. JSON shape matches `PARITY-JSON-KEYS` rules. - [ ] Parity test at `src/tools/-parity.test.ts` that cites the rule IDs it enforces (in a file header comment). Covers at minimum: - successful search, zero-result, two error codes. + successful query, zero-result, two error codes. - [ ] MCP tool description mirrored across every shipped MCP surface before public release. @@ -148,14 +126,6 @@ When a new tool lands with both MCP and CLI surfaces: - **Shared MCP description copy.** Each tool's description targets a different decision the agent is making. Copy is not reusable. -## When to extend this document - -- A new rule ID is added when a future tool exposes a pattern that is - genuinely cross-tool (naming, shared helper location, JSON shape). -- An existing rule is revised when the tool that originally - established it (`search_symbols`) turns out to be the outlier. -- The checklist grows by exactly one bullet per rule. Keep it short. - ## Related files | File | Role | @@ -164,301 +134,160 @@ When a new tool lands with both MCP and CLI surfaces: | `src/shared/code-navigation-error-map.ts` | `mapCodeNavigationError` classifier and `MappedError` union. | | `src/shared/pkgseer-graphql.ts` | Low-level authenticated package/source POST helper shared by the service clients. | | `src/shared/pkgseer-registry.ts` | Registry taxonomy (`PkgseerRegistry` union + lowercase↔uppercase converters). | -| `src/shared/search-symbols-request.ts` | Shared request builder for `search_symbols`. | -| `src/shared/search-symbols-response.ts` | Shared JSON envelope builders for `search_symbols`. | -| `src/shared/unified-search-request.ts` | Shared request builder for top-level unified `search`; compiles structured query fields and applies defaulting. | -| `src/shared/unified-search-response.ts` | Shared JSON envelope builders for top-level unified `search` and follow-up `search_status`. | -| `src/shared/package-summary-request.ts` | Shared request builder for `package_summary`. | -| `src/shared/package-summary-response.ts` | Lean JSON envelope builder and terminal formatter for `package_summary`. | -| `src/shared/package-vulnerabilities-request.ts` | Shared request builder for `package_vulnerabilities`; owns the tool-local `supportsVulnerabilitiesRegistry` predicate and the severity-label → CVSS float map. | -| `src/shared/package-vulnerabilities-response.ts` | Lean JSON envelope builder for `package_vulnerabilities` (shared); terminal formatter (CLI-only). | -| `src/shared/package-dependencies-request.ts` | Shared request builder for `package_dependencies`; owns `supportsDependenciesRegistry` + lifecycle / depth validation. | -| `src/shared/package-dependencies-response.ts` | Lean JSON envelope builder for `package_dependencies` (shared); terminal formatter (CLI-only). | -| `src/shared/package-changelog-request.ts` | Shared request builder for `package_changelog`; owns spec-XOR-repo-URL validation, `@` rejection, `--from` / `--limit` mutex, tag-style version rejection, and the `explicitFilterFields` tracker. | -| `src/shared/package-changelog-response.ts` | JSON envelope builder for `package_changelog` (shared); terminal formatter (CLI-only). | -| `src/shared/list-files-request.ts` | Shared request builder for `list_files`; applies the shared `DEFAULT_WAIT_TIMEOUT_MS`, enforces limit bounds, tracks explicit-filter fields. | -| `src/shared/list-files-response.ts` | JSON envelope builder for `list_files` (shared); terminal formatter (CLI-only). Resolves the `hasMore` → `N+` header behaviour. | -| `src/shared/read-file-request.ts` | Shared request builder for `read_file`; trims filePath, validates start/end line positive-integer rules, rejects reversed ranges. | -| `src/shared/read-file-response.ts` | JSON envelope builder for `read_file` (shared); terminal formatter (CLI-only). Normalises the envelope key to `path` (not `filePath`) so `list_files` → `read_file` chains without renames. | -| `src/shared/grep-repo-request.ts` | Shared request builder for `grep_repo`; exports `GREP_REPO_PATTERN_NOTE` referenced by MCP description, MCP `pattern` describe, and CLI help. Compiles public scope inputs into backend `pathSelectors` and applies internal `allowUnscoped` when no scope filters are given. | -| `src/shared/grep-repo-response.ts` | JSON envelope builder for `grep_repo` (shared); terminal formatter (CLI-only) renders plain `file:line:text` or verbose grouped output and surfaces pagination via stderr. | -| `src/shared/code-navigation-error-map.ts` | `mapCodeNavigationError` classifier. Owns the `INDEXING` / `FILE_NOT_FOUND` / `NOT_FOUND` codes shared across all four code-nav tools. | -| `src/shared/code-navigation-defaults.ts` | `DEFAULT_WAIT_TIMEOUT_MS = 20_000` + `MAX_WAIT_TIMEOUT_MS = 60_000`. Both CLI and MCP request builders import these so defaults never diverge. | -| `src/tools/code-navigation-shared.ts` | `codeTargetSchema` + `resolveCodeTarget` — the single addressing primitive used by `search_symbols`, `list_files`, `read_file`, `grep_repo`. | +| `src/shared/unified-search-request.ts` | Shared request builder for unified `search`; compiles structured query fields and applies defaulting. | +| `src/shared/unified-search-response.ts` | Shared JSON envelope builders for unified `search` and follow-up `search_status`. | +| `src/shared/package-summary-request.ts` | Shared request builder for `pkg_info`. | +| `src/shared/package-summary-response.ts` | Lean JSON envelope builder and terminal formatter for `pkg_info`. | +| `src/shared/package-vulnerabilities-request.ts` | Shared request builder for `pkg_vulns`. | +| `src/shared/package-vulnerabilities-response.ts` | Lean JSON envelope builder for `pkg_vulns`. | +| `src/shared/package-dependencies-request.ts` | Shared request builder for `pkg_deps`. | +| `src/shared/package-dependencies-response.ts` | Lean JSON envelope builder for `pkg_deps`. | +| `src/shared/package-changelog-request.ts` | Shared request builder for `pkg_changelog`. | +| `src/shared/package-changelog-response.ts` | JSON envelope builder for `pkg_changelog`. | +| `src/shared/list-files-request.ts` | Shared request builder for `code_files`. | +| `src/shared/list-files-response.ts` | JSON envelope builder for `code_files`. | +| `src/shared/read-file-request.ts` | Shared request builder for `code_read`. | +| `src/shared/read-file-response.ts` | JSON envelope builder for `code_read`. Normalises envelope key to `path` (not `filePath`) so `code_files` → `code_read` chains without renames. | +| `src/shared/grep-repo-request.ts` | Shared request builder for `code_grep`. Exports `GREP_REPO_PATTERN_NOTE` referenced by MCP description, MCP `pattern` describe, and CLI help. | +| `src/shared/grep-repo-response.ts` | JSON envelope builder for `code_grep`. | +| `src/shared/list-package-docs-request.ts` / `list-package-docs-response.ts` | Shared request and envelope for `docs_list`. | +| `src/shared/read-package-doc-request.ts` / `read-package-doc-response.ts` | Shared request and envelope for `docs_read`. | +| `src/shared/code-navigation-error-map.ts` | Owns the `INDEXING` / `FILE_NOT_FOUND` / `NOT_FOUND` codes shared across all code-nav tools. | | `src/shared/package-intelligence-error-map.ts` | `mapPackageIntelligenceError` classifier (reuses `MappedError` from the code-nav map). | -| `src/services/promote-version-not-found.ts` | Shared helper that promotes generic backend errors with "no matching version" messages into typed `VERSION_NOT_FOUND`. Used by the `packageVulnerabilities`, `packageDependencies`, and `packageChangelog` executors. Handles both `version` (single-version queries) and `fromVersion` / `toVersion` (range queries), and skips `details.package` synthesis when registry/name aren't available (repo-URL mode). | -| `src/tools/search-symbols.ts` | MCP tool definition for `search_symbols`. | +| `src/services/promote-version-not-found.ts` | Shared helper that promotes generic backend errors with "no matching version" messages into typed `VERSION_NOT_FOUND`. | +| `src/tools/code-navigation-shared.ts` | `codeTargetSchema` + `resolveCodeTarget` — the addressing primitive used by `code_files`, `code_read`, `code_grep`, and unified `search`. | | `src/tools/search.ts` | MCP tool definition for unified `search`. | | `src/tools/search-status.ts` | MCP tool definition for `search_status`. | -| `src/tools/package-summary.ts` | MCP tool definition for `package_summary`. | -| `src/tools/package-vulnerabilities.ts` | MCP tool definition for `package_vulnerabilities`. | -| `src/tools/package-dependencies.ts` | MCP tool definition for `package_dependencies`. | -| `src/tools/package-changelog.ts` | MCP tool definition for `package_changelog`. | -| `src/tools/list-files.ts` | MCP tool definition for `list_files`. | -| `src/tools/read-file.ts` | MCP tool definition for `read_file`. | -| `src/tools/grep-repo.ts` | MCP tool definition for `grep_repo`. | -| `src/commands/code/search-symbols.ts` | CLI command. | +| `src/tools/package-summary.ts` | MCP tool definition for `pkg_info`. | +| `src/tools/package-vulnerabilities.ts` | MCP tool definition for `pkg_vulns`. | +| `src/tools/package-dependencies.ts` | MCP tool definition for `pkg_deps`. | +| `src/tools/package-changelog.ts` | MCP tool definition for `pkg_changelog`. | +| `src/tools/list-files.ts` | MCP tool definition for `code_files`. | +| `src/tools/read-file.ts` | MCP tool definition for `code_read`. | +| `src/tools/grep-repo.ts` | MCP tool definition for `code_grep`. | +| `src/tools/list-package-docs.ts` / `read-package-doc.ts` | MCP tool definitions for the docs surface. | | `src/commands/search.ts` | Top-level CLI commands for unified `search` and `search-status`. | -| `src/commands/pkg/info.ts` | CLI command for `pkg info`. | -| `src/commands/pkg/vulns.ts` | CLI command for `pkg vulns`. | -| `src/commands/pkg/deps.ts` | CLI command for `pkg deps`. | -| `src/commands/pkg/changelog.ts` | CLI command for `pkg changelog`. | -| `src/commands/code/files.ts` | CLI command for `code files`. | -| `src/commands/code/read.ts` | CLI command for `code read`. | -| `src/commands/code/grep.ts` | CLI command for `code grep`. | -| `src/tools/search-symbols-parity.test.ts` | Parity tests (cite rule IDs). | -| `src/tools/package-summary-parity.test.ts` | Parity tests for `package_summary` (cite rule IDs). | -| `src/tools/package-vulnerabilities-parity.test.ts` | Parity tests for `package_vulnerabilities` (cite rule IDs). | -| `src/tools/package-dependencies-parity.test.ts` | Parity tests for `package_dependencies` (cite rule IDs). | -| `src/tools/package-changelog-parity.test.ts` | Parity tests for `package_changelog` (cite rule IDs). | -| `src/tools/list-files-parity.test.ts` | Parity tests for `list_files` (cite rule IDs). | -| `src/tools/read-file-parity.test.ts` | Parity tests for `read_file` (cite rule IDs). | -| `src/tools/grep-repo-parity.test.ts` | Parity tests for `grep_repo` (cite rule IDs). | +| `src/commands/pkg/info.ts` / `vulns.ts` / `deps.ts` / `changelog.ts` | CLI commands for the `pkg` group. | +| `src/commands/code/files.ts` / `read.ts` / `grep.ts` | CLI commands for the `code` group. | +| `src/commands/docs/list.ts` / `read.ts` | CLI commands for the `docs` group. | +| `src/tools/*-parity.test.ts` | Parity tests; each cites the rule IDs it enforces. | ## Per-tool notes -### `package_summary` - -- **Permissive MCP schema + in-handler validation.** Matches the - `search_symbols` precedent. `buildPackageSummaryParams` is the - single validator used by both surfaces; raw Zod errors never - surface in the envelope. -- **Parity assertion policy** (coded in - `src/tools/package-summary-parity.test.ts`): - - `toEqual` for service-sourced fixtures (happy, minimal-fields, - `NOT_FOUND`, `BACKEND_ERROR`) — envelopes are byte-identical - because both surfaces route through the same classifier and - response builder. - - `toMatchObject` for the `INVALID_ARGUMENT` fixture. CLI's - `parsePackageSpec` and MCP's in-handler - `buildPackageSummaryParams` produce surface-specific error text; - same envelope shape, different message. +### `pkg_info` + +- **Permissive MCP schema + in-handler validation.** + `buildPackageSummaryParams` is the single validator used by both + surfaces; raw Zod errors never surface in the envelope. - **`@version` rejection.** CLI-only. The MCP tool has no `version` input. The CLI's `pkg info` throws `InvalidPackageSpecError` on any non-null parsed version — never silently swaps to latest. -### `package_vulnerabilities` +### `pkg_vulns` -- **Permissive MCP schema + in-handler validation.** Same pattern as - `package_summary`. `buildPackageVulnerabilitiesParams` is the - single validator used by both surfaces; raw Zod errors never - surface in the envelope. +- **Permissive MCP schema + in-handler validation.** + `buildPackageVulnerabilitiesParams` is the single validator. - **Filter-aware summary.** `minSeverity` + `includeWithdrawn` go straight to the service; the returned `vulnerabilityCount` - reflects the filtered set. No client-side filtering, no - `summary.filtered` dual-block. + reflects the filtered set. No client-side filtering. - **Partitioning bySeverity buckets.** `summary.bySeverity` carries a `malware` key for `isMalicious === true` advisories; severity bands for non-malicious advisories with a positive CVSS score; and `unrated` for non-malicious advisories with no score. Every - returned advisory lands in exactly one bucket — client-side - guarantee `MALWARE + crit + high + medium + low + unrated = - advisories.length`. The sum also equals `summary.total` whenever - the backend keeps `vulnerabilityCount` and `vulnerabilities[]` - consistent. Malware advisories sort first in the advisory list - regardless of severity score; `unrated` advisories sort last - within the active bucket. -- **Scope of the shared helper.** `buildPackageVulnerabilitiesSuccessPayload` - is shared between CLI `--json` and MCP `content[0].text` — that's - what enforces envelope parity. The terminal formatter - `formatPackageVulnerabilitiesTerminal` is CLI-only (MCP always - emits JSON). The parity doc's default rule (CLI-local rendering) - still applies to the formatter; the envelope builder is the - explicit shared-helper exception. -- **Parity assertion policy** (coded in - `src/tools/package-vulnerabilities-parity.test.ts`): - - `toEqual` for the service-sourced fixtures: happy, zero-vulns, - filtered-success, versioned-match (no `requestedVersion`), - versioned-real-diff (`requestedVersion` present), `NOT_FOUND`, - `VERSION_NOT_FOUND` (with structured details), `BACKEND_ERROR`. - - `toMatchObject` for builder-sourced `INVALID_ARGUMENT` cases - such as unsupported registry (`vcpkg`) and tag-style version - input (`v4.18.0`). -- **Typed `VERSION_NOT_FOUND`.** Mirrors the code-nav precedent: + returned advisory lands in exactly one bucket — `MALWARE + crit + + high + medium + low + unrated = advisories.length`. Malware + advisories sort first regardless of severity score; `unrated` + advisories sort last within the active bucket. +- **Typed `VERSION_NOT_FOUND`.** `PackageIntelligenceVersionNotFoundError` carries structured - fields sourced from the service response (`packageName`, - `requestedVersion`, `availableVersions`). The classifier emits - structured `details` in the error envelope. -- **Client-side `v`-prefix rejection.** `package_vulnerabilities` - validates version strings before the service call. Tag-style - inputs like `v4.18.0` are rejected as `INVALID_ARGUMENT` with an - actionable message instead of relying on the current production - backend, which returns a generic error for that input. + fields (`packageName`, `requestedVersion`, `availableVersions`). + The classifier emits structured `details` in the error envelope. +- **Client-side `v`-prefix rejection.** Tag-style inputs like + `v4.18.0` are rejected as `INVALID_ARGUMENT` with an actionable + message before the service call. -### `package_dependencies` +### `pkg_deps` - **Data-first envelope.** `runtime`, `groups`, and `transitive` are three independent blocks emitted based on what the backend returned and what the caller asked for, not on additional caller - flags. An MCP agent decides what to read based on what's in the - envelope — no branching on invocation inputs. + flags. - **No `include_groups` input.** The data-first envelope emits the `groups` block unconditionally when the backend returned `dependencyGroups`, so an `include_groups: true` input would be a - silently ignored no-op. Deliberately absent from the MCP schema. + silently ignored no-op. - **Dependency list naming.** Every list of dependencies in the envelope uses the `items` key: `runtime.items`, `groups.items` (array of groups), each group's nested `items` (array of member - deps). Symmetric and easy to parse. + deps). - **Lifecycle filter echo.** `filter.lifecycles` is the canonicalised, deduplicated, display-order-sorted array the - backend actually received (never the raw CSV). Emitted only when - the caller supplied a non-empty input. + backend actually received. Emitted only when the caller supplied + a non-empty input. - **Null vs empty matters.** `groups` is omitted entirely when the - backend returned `dependencyGroups: null` (zero-dep packages); - emitted with `items: []` when the backend returned a non-null + backend returned `dependencyGroups: null`; emitted with + `items: []` when the backend returned a non-null `dependencyGroups` with zero groups (filter matched nothing). - `runtime` is omitted when `dependencies: null` or `direct: null`; - emitted with `count: 0, items: []` when `direct: []`. - **Terminal-only dedup.** Crates feature groups can contain duplicate `{name, constraint}` tuples (target-cfg branching). The terminal formatter collapses them; the JSON envelope preserves - every duplicate the backend emitted. A parity fixture exercises - the round-trip. -- **Preprocessed transitive.** Backend declares `transitive.conflicts`, - `transitive.circularDependencies`, and the DAG as `GenericJSON`, - but the envelope builder decodes them using best-effort shape - detectors so agents see typed data. `transitive.packages[]` carries - `{name, version, importers[]}` records (importer name / version / - constraint pulled from the DAG); `conflicts[]` is typed + every duplicate the backend emitted. +- **Preprocessed transitive.** `transitive.packages[]` carries + `{name, version, importers[]}` records; `conflicts[]` is typed `{name, requiredVersions}` when decodable; `circularDependencies[]` - is typed `{cycle: string[]}` when decodable. When a decoder can't - match, that field falls back to raw `GenericJSON[]` so no data is - lost. The raw DAG itself is deliberately dropped from this tool's - envelope — a future `pkg deps-dag` command will expose it under a - typed contract. `groups.environmentConstraints` remains raw - `GenericJSON[]` (no live shape observed yet). -- **Parity assertion policy** (coded in - `src/tools/package-dependencies-parity.test.ts`): - - `toEqual` across the service-sourced success fixtures: happy - flat-runtime, zero-dep (omits `groups`), full-view, optional- - lifecycle (tokio features), multi-lifecycle filter, - filter-matched-nothing (`groups: {items: []}`), - Crates-target-cfg dedup round-trip, versioned match / diff, - `NOT_FOUND`, `VERSION_NOT_FOUND` with structured details, - `BACKEND_ERROR`. - - `toMatchObject` for builder-sourced `INVALID_ARGUMENT` cases: - unsupported registry (`nuget`), tag-style version (`v4.18.0`), - unknown lifecycle token (`dev`). - -### `package_changelog` + is typed `{cycle: string[]}` when decodable. The raw DAG is + deliberately dropped from this tool's envelope. + +### `pkg_changelog` - **Dual addressing — the only pkg-intel tool with it.** `registry` - + `package_name` XOR `repo_url` on both surfaces. Justified because - `packageChangelog` is intrinsically repo-level (its - sources are GitHub Releases, CHANGELOG.md, HexDocs); repo-URL - isn't a bolt-on, it's a peer addressing mode on the service - signature. `package_summary` / `package_vulnerabilities` / - `package_dependencies` omit it because their queries are - registry-metadata APIs without repo-URL alternatives. Future - pkg-intel tool authors should not cargo-cult the asymmetry. + + `package_name` XOR `repo_url` on both surfaces, because + `packageChangelog` is intrinsically repo-level. - **`@` rejected.** Other `pkg` commands give - `@version` a meaning (`for this exact version`), but changelog - has no single-version query — remapping to `to_version` would be - a client-invented semantic shift. Both CLI and MCP reject with - `INVALID_ARGUMENT` and redirect callers to `--to` / `to_version`. + `@version` a meaning, but changelog has no single-version query + — remapping to `to_version` would be a client-invented semantic + shift. Both surfaces redirect callers to `--to` / `to_version`. - **Mode mutex enforced client-side.** `--from` / `from_version` + - `--limit` / `limit` together → `INVALID_ARGUMENT`. The backend's - same-shape rejection is generic; we catch it with a specific hint - before the wire. -- **`filter.*` echo tracks explicit fields only.** Request builder - exposes an `explicitFilterFields` set (`fromVersion`, `toVersion`, - `limit`, `gitRef`). The envelope builder consults the set before - emitting `filter.*`, so backend-default values - (e.g. `limit: 10` from the wire echo) never round-trip as caller - intent. -- **`entries: { count, items }` shape.** Matches the `runtime: - { count, items }` convention from `package_dependencies`. - `entries.count === entries.items.length` by construction; the - backend's count field is never selected on the wire. -- **`version` kept when null, other per-entry nullables stripped.** - `version` is the primary key agents index by, so the slot is - always present (possibly `null`). Other nullable fields - (`normalizedVersion`, `publishedAt`, `htmlUrl`, `body`) are - stripped when absent to keep the envelope lean. `body` is - additionally stripped when `include_bodies: false`. -- **`metadata` dropped.** `ChangelogEntry.metadata` is backend - `GenericJSON`; v1 envelope drops it entirely rather than - guessing at its shape. Revisit via agent feedback - (`TODO(backend)` anchor on the service type). + `--limit` / `limit` together → `INVALID_ARGUMENT`. +- **`filter.*` echo tracks explicit fields only.** Backend-default + values never round-trip as caller intent. +- **`entries: { count, items }` shape.** Mirrors `runtime: {count, + items}` from `pkg_deps`. - **`source: null` promoted to `NOT_FOUND`.** The service layer promotes the null-source case to a typed - `PackageIntelligenceChangelogSourceNotFoundError` which the - shared classifier routes to `NOT_FOUND` with a message naming - the sources tried (GitHub Releases, CHANGELOG.md, HexDocs). - `source: "releases"` + `entries.items: []` is success — "no - entries in this range" is a legitimate neutral outcome. -- **`--verbose` vs `--no-body` vs `--json` interaction.** - Default terminal output shows each entry's body truncated at - 10 lines with a `… (+N more lines — use --verbose …)` footer. - `--verbose` lifts the cap (terminal-only — does not change - `--json` output). `--no-body` mirrors MCP's `include_bodies: - false` and affects both terminal (no body preview, no footer) - and `--json` (entry objects lose the `body` field) — explicit - opt-out, not silent truncation. `--no-body` + `--verbose` is - rejected with a specific hint because the two flags contradict. -- **`promoteGenericVersionNotFound` extension.** The shared helper - now recognises `fromVersion` / `toVersion` in addition to - `version`. Preference order: `version → fromVersion → toVersion`. - In repo-URL mode (no `registry` / `packageName`), - `details.package` is omitted; the error-map handles - `details.package === undefined` gracefully. -- **Parity assertion policy** (coded in - `src/tools/package-changelog-parity.test.ts`): - - `toEqual` across service-sourced fixtures: happy latest mode, - range mode (`--from` / `from_version`), repo-URL addressing, - `--no-body` / `include_bodies: false`, default bodies, empty - entries, `NOT_FOUND` (no source), `PackageIntelligenceTargetNotFoundError` - (package missing), `VERSION_NOT_FOUND` with structured details, - `BACKEND_ERROR`. - - `toMatchObject` for builder-sourced `INVALID_ARGUMENT` cases: - `@` rejection, `--from` + `--limit` mutex. - -### `list_files` / `read_file` / `grep_repo` (file-exploration bundle) + `PackageIntelligenceChangelogSourceNotFoundError` with a message + naming the sources tried (GitHub Releases, CHANGELOG.md, HexDocs). +- **`--verbose` / `--no-body` / `--json` interaction.** Default + terminal output truncates each entry's body at 10 lines. + `--verbose` lifts the cap (terminal-only). `--no-body` mirrors + MCP's `include_bodies: false` and affects both terminal and + `--json`. `--no-body` + `--verbose` is rejected. + +### `code_files` / `code_read` / `code_grep` (file-exploration bundle) All three reuse `codeTargetSchema` + `resolveCodeTarget` from `src/tools/code-navigation-shared.ts`. The indexing lifecycle is -shared (see `tools.md` "Indexing lifecycle" section). Parity -tests cover dual addressing, default + explicit filter echoes, -INDEXING error envelope, NOT_FOUND envelope, and INVALID_ARGUMENT -with full envelope shape (`{error, code, retryable}`) — the -partial-match policy is deliberately *not* used on INVALID_ARGUMENT -so envelope-drift surfaces in the test rather than at an agent. - -- **`list_files`**: `filter.path_prefix` / `filter.limit` echo - only when explicit. Default `limit: 200` never round-trips. - Backend returns `total` capped at returned count when - `hasMore: true`; terminal formatter renders `N+` to avoid - misleading users. -- **`read_file`**: envelope uses `path` (not `filePath`) to - match `list_files.files[].path`, so agent chains mechanically. - Binary files: `isBinary: true` + `content` omitted (not - `null`). Parity fixture locks this in. `fetchCodeContext` - on the backend doesn't return `availableVersions` on - INDEXING responses, so its `details` block carries only - `indexingRef` — MCP description calls this out explicitly. -- **`grep_repo`**: `GREP_REPO_PATTERN_NOTE` constant - (exported from `grep-repo-request.ts`) ensures the - literal-vs-regex disclosure is identical in the MCP - description, MCP `pattern` field describe, and CLI help text. - The shared request builder compiles `path`, `path_prefix`, and - `globs` into backend `pathSelectors`, keeps `allowUnscoped` - internal-only, and defaults grep to whole-target, literal, - ASCII case-insensitive matching; non-ASCII letters match - case-sensitively. Whole-target regexes must include at least one - literal substring the backend index can use for pre-filtering. - `symbol_fields` / `--symbol-field` passes backend symbol hydration - field names through to `symbolFields` and the response envelope - carries `matches[].symbol` when the backend hydrates it. The shared - response builder keeps CLI `--json` and MCP payloads byte-identical - for equivalent inputs. - -- **Parity assertion policy** (coded in the three parity - tests): - - `toEqual` across service-sourced fixtures: happy (package - and repo-URL addressing), filter echoes, INDEXING, NOT_FOUND, - and (for `read_file`) the binary fixture; (for - `read_file`) FILE_NOT_FOUND and line range. - - `toMatchObject` with explicit `retryable: false` assertion - for builder-sourced `INVALID_ARGUMENT` — both surfaces - must emit the same envelope keys so drift is loud. +shared (see `tools.md` "Indexing lifecycle" section). Parity tests +cover dual addressing, default + explicit filter echoes, INDEXING +error envelope, NOT_FOUND envelope, and INVALID_ARGUMENT with full +envelope shape. + +- **`code_files`**: `filter.path_prefix` / `filter.limit` echo only + when explicit. Default `limit: 200` never round-trips. Backend + returns `total` capped at returned count when `hasMore: true`; + terminal formatter renders `N+`. +- **`code_read`**: envelope uses `path` (not `filePath`) to match + `code_files.files[].path`. Binary files: `isBinary: true` + + `content` omitted (not `null`). `fetchCodeContext` on the + backend doesn't return `availableVersions` on INDEXING responses, + so its `details` block carries only `indexingRef`. +- **`code_grep`**: `GREP_REPO_PATTERN_NOTE` (exported from + `grep-repo-request.ts`) keeps the literal-vs-regex disclosure + identical across MCP description, MCP `pattern` describe, and + CLI help. The shared request builder compiles `path`, + `path_prefix`, and `globs` into backend `pathSelectors`, defaults + grep to whole-target literal ASCII case-insensitive matching; + whole-target regexes must include at least one literal substring. + `symbol_fields` / `--symbol-field` passes backend symbol + hydration through to `symbolFields`; the response envelope + carries `matches[].symbol` when the backend hydrates it. diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 0d18bf7c..f9c59385 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -24,41 +24,35 @@ Both expose the same tools with identical names, parameters, and descriptions. T | `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_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. | -| `package_summary` | `registry`, `package_name` | Package overview: latest version, license, description, repository, downloads, GitHub metadata, install command, and known vulnerabilities. Always returns the latest published version. | -| `package_vulnerabilities` | `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. | -| `package_dependencies` | `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`. | -| `package_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. | -| `list_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`. | -| `read_file` | `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 `list_files` to discover the actual path. | -| `grep_repo` | `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. | +| `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. | -`search`, `search_status`, `package_summary`, `package_vulnerabilities`, `package_dependencies`, `package_changelog`, `list_files`, `read_file`, and `grep_repo` 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. - -`search_symbols` is no longer registered by the local MCP server. It remains in the codebase as the legacy parity surface for the CLI `githits code search` command. The parity rules are codified in [`mcp-cli-parity.md`](./mcp-cli-parity.md); the parity test (`src/tools/search-symbols-parity.test.ts`) asserts that both surfaces emit identical JSON for equivalent inputs. +`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. **Unified `search` query syntax.** The `search.query` field is the backend discovery query syntax, not a raw pass-through to a per-source search engine. It supports implicit `AND`, uppercase `OR`, parentheses, unary `-`, quoted phrases, semantic qualifiers (`kind:`, `category:`, `path:`, `lang:`, `name:`, `intent:`), and routing qualifiers (`registry:`, `package:`, `version:`, `repo:`). The backend parses the query once and compiles it per source. Structured `name` and `language` inputs are compiled into `name:` / `lang:` qualifiers and AND-ed with the query before sending. Per-source support, ignored features, and incompatibilities are reported in `sourceStatus`. -**Legacy `search_symbols` response shape.** `search_symbols` always requests `mode: DETAILED` and always selects the `code`, `resolution`, `kind`, and `category` fields. Responses include each match's full source code, precise symbol kind (from the unified symbol taxonomy), broad symbol category, and line range. The tool does not expose `mode` or `verbose` inputs — the service layer makes the choice once so both surfaces get the richest response without callers juggling the knobs. The legacy `chunkType` field is no longer selected or surfaced client-side; `kind` is the single source of truth for taxonomy. The text payload is always valid JSON (whether the result is success or error), so MCP clients can parse `content[0].text` without branching on `isError`. - -**Legacy `search_symbols` filter parameters.** `category` is the preferred surface for filtering (`callable`, `type`, `module`, `data`, `documentation`); `kind` is for the "I want this specific construct" case (27-value taxonomy). Both filters may be combined; both route through the shared `buildSearchSymbolsParams` helper. - -### `package_summary` response shape +### `pkg_info` response shape -**Hand-crafted JSON envelope.** `package_summary` returns a lean JSON payload designed for agent token efficiency. Fields that do not add caller value are deliberately omitted. Null scalars are omitted; blocks (`github`, `vulnerabilities`, `downloads`, `recentChanges`) are omitted entirely when they carry no actionable data. `vulnerabilities` is omitted when `total === 0` or missing; when present, severity values include a CVSS-banded `severityLabel` (`critical` ≥9, `high` ≥7, `medium` ≥4, else `low`) for agent convenience. +**Hand-crafted JSON envelope.** `pkg_info` returns a lean JSON payload designed for agent token efficiency. Fields that do not add caller value are deliberately omitted. Null scalars are omitted; blocks (`github`, `vulnerabilities`, `downloads`, `recentChanges`) are omitted entirely when they carry no actionable data. `vulnerabilities` is omitted when `total === 0` or missing; when present, severity values include a CVSS-banded `severityLabel` (`critical` ≥9, `high` ≥7, `medium` ≥4, else `low`) for agent convenience. -**Validation.** The MCP schema is permissive (`registry: z.string()`, `package_name: z.string()`) — validation happens in-handler via `buildPackageSummaryParams`, producing the same structured `{error, code, retryable}` envelope as CLI. Matches the `search_symbols` pattern of never surfacing raw Zod errors to agents. +**Validation.** The MCP schema is permissive (`registry: z.string()`, `package_name: z.string()`) — validation happens in-handler via `buildPackageSummaryParams`, producing the same structured `{error, code, retryable}` envelope as CLI. Raw Zod errors are never surfaced to agents. **Always latest.** The query exposes no `version` input because the upstream `packageSummary` resolver always returns the latest published version. The CLI `githits pkg info` rejects `@` with `INVALID_ARGUMENT` rather than silently swapping — a silent-swap would break security-testing workflows that pin to an older vulnerable release. -`package_summary` shares its envelope builder, terminal formatter, and error classifier with the CLI `githits pkg info` command via `src/shared/package-summary-request.ts`, `src/shared/package-summary-response.ts`, and `src/shared/package-intelligence-error-map.ts`. The parity test (`src/tools/package-summary-parity.test.ts`) asserts `toEqual` between CLI `--json` and MCP `content[0].text` for service-sourced fixtures, and `toMatchObject` for the `INVALID_ARGUMENT` fixture where surface-specific error text is acceptable. +`pkg_info` shares its envelope builder, terminal formatter, and error classifier with the CLI `githits pkg info` command via `src/shared/package-summary-request.ts`, `src/shared/package-summary-response.ts`, and `src/shared/package-intelligence-error-map.ts`. The parity test (`src/tools/package-summary-parity.test.ts`) asserts `toEqual` between CLI `--json` and MCP `content[0].text` for service-sourced fixtures, and `toMatchObject` for the `INVALID_ARGUMENT` fixture where surface-specific error text is acceptable. -### `package_vulnerabilities` response shape +### `pkg_vulns` response shape **Filter-aware summary.** `min_severity` and `include_withdrawn` are passed straight through to the service. The returned `vulnerabilityCount` reflects the filtered set — there is no client-side filtering and no `summary.filtered` dual-block. Callers wanting the unfiltered view omit the flag. **Partitioning buckets.** Advisories with `isMalicious: true` count **only** under `summary.bySeverity.malware`; severity bands (`critical`/`high`/`medium`/`low`) count non-malicious advisories with a positive CVSS score; non-malicious advisories with no score count under `summary.bySeverity.unrated`. Every returned advisory lands in exactly one bucket — the client-side guarantee is `MALWARE + crit + high + medium + low + unrated = advisories.length`. The sum also equals `summary.total` whenever the backend keeps `vulnerabilityCount` and `vulnerabilities[]` consistent (the expected case on all shipped registries). The malware bucket sorts to the top of the advisory list regardless of score. The `unrated` bucket ensures the terminal breakdown line reconciles with the header total on Rust / PyPI packages where a non-trivial fraction of advisories ship without a CVSS score. -**Version validation.** `package_vulnerabilities` accepts canonical package versions only. Tag-style refs with a leading `v` (for example `v4.18.0`) are rejected client-side with `INVALID_ARGUMENT` before the backend call. This avoids the current production backend's unhelpful generic error for that input shape. This is intentionally narrow: proper ecosystem-aware version parsing and typed invalid-version errors belong in the backend, not in ad hoc CLI normalization rules. +**Version validation.** `pkg_vulns` accepts canonical package versions only. Tag-style refs with a leading `v` (for example `v4.18.0`) are rejected client-side with `INVALID_ARGUMENT` before the backend call. This avoids the current production backend's unhelpful generic error for that input shape. This is intentionally narrow: proper ecosystem-aware version parsing and typed invalid-version errors belong in the backend, not in ad hoc CLI normalization rules. **Typed `VERSION_NOT_FOUND`.** Mirrors the code-nav precedent: a dedicated `PackageIntelligenceVersionNotFoundError` carries structured `{ packageName, requestedVersion, availableVersions? }` fields. Classifier routes it to `VERSION_NOT_FOUND` with a structured `details` block. When the service only gets a generic "no matching version" error, it promotes that into the typed error so CLI / MCP surfaces still render an actionable envelope. `availableVersions` remains undefined in the fallback path unless the service supplied them. @@ -66,9 +60,9 @@ Both expose the same tools with identical names, parameters, and descriptions. T **Registry coverage.** Only npm, PyPI, Hex, and Crates have vulnerability data. The CLI + MCP reject the other five registries client-side with a tool-specific message (`pkg vulns only supports npm, pypi, hex, and crates. Got: ${registry}.`) — rejection predicate lives in `src/shared/package-vulnerabilities-request.ts` rather than the shared registry module, since it is a tool-specific capability matrix. -`package_vulnerabilities` shares its envelope builder with the CLI `githits pkg vulns` command via `src/shared/package-vulnerabilities-request.ts` and `src/shared/package-vulnerabilities-response.ts`. The terminal formatter is CLI-only (MCP always emits JSON). The parity test (`src/tools/package-vulnerabilities-parity.test.ts`) asserts `toEqual` across the service-sourced success and typed-error fixtures, and `toMatchObject` for builder-sourced `INVALID_ARGUMENT` fixtures such as unsupported registries and tag-style `v`-prefixed versions. +`pkg_vulns` shares its envelope builder with the CLI `githits pkg vulns` command via `src/shared/package-vulnerabilities-request.ts` and `src/shared/package-vulnerabilities-response.ts`. The terminal formatter is CLI-only (MCP always emits JSON). The parity test (`src/tools/package-vulnerabilities-parity.test.ts`) asserts `toEqual` across the service-sourced success and typed-error fixtures, and `toMatchObject` for builder-sourced `INVALID_ARGUMENT` fixtures such as unsupported registries and tag-style `v`-prefixed versions. -### `package_dependencies` response shape +### `pkg_deps` response shape **Data-first envelope.** `runtime`, `groups`, and `transitive` are three independent blocks emitted based on what the backend returned and what the caller asked for, not on additional caller flags. Agents branch on the envelope's shape rather than inferring from inputs. @@ -84,19 +78,19 @@ Both expose the same tools with identical names, parameters, and descriptions. T **Registry coverage.** Only npm, PyPI, Hex, Crates, vcpkg, and Zig support the `packageDependencies` query. NuGet / Maven / Packagist are rejected client-side with a tool-specific message (`pkg deps only supports npm, pypi, hex, crates, vcpkg, and zig. Got: ${registry}.`). Predicate lives in `src/shared/package-dependencies-request.ts`. -**Version validation.** Same rule as `package_vulnerabilities`: tag-style `v`-prefixed inputs are rejected client-side with `INVALID_ARGUMENT` before the backend call. +**Version validation.** Same rule as `pkg_vulns`: tag-style `v`-prefixed inputs are rejected client-side with `INVALID_ARGUMENT` before the backend call. **MCP schema notes.** Permissive (`registry: z.string()`, `package_name: z.string()`, …) with validation in-handler via `buildPackageDependenciesParams`. Deliberately no `include_groups` input — with the data-first envelope emitting `groups` unconditionally when the backend returns `dependencyGroups`, the flag would be a silently ignored no-op. Neither the MCP surface nor the CLI applies a depth default: `max_depth` / `--depth` is optional and, when omitted, the backend's full-graph traversal is used. `include_importers` requires `include_transitive: true`; `max_depth` and CLI `--depth` require the transitive view — passing them alone is rejected with `INVALID_ARGUMENT` rather than silently ignored. -`package_dependencies` shares its envelope builder with the CLI `githits pkg deps` command via `src/shared/package-dependencies-request.ts` and `src/shared/package-dependencies-response.ts`. The terminal formatter is CLI-only. The parity test (`src/tools/package-dependencies-parity.test.ts`) asserts `toEqual` across every service-sourced success / error fixture (runtime, zero-dep, full-view, optional-lifecycle, multi-lifecycle, filter-matched-nothing, Crates-target-cfg dedup round-trip, transitive, versioned match / diff, NOT_FOUND, VERSION_NOT_FOUND, BACKEND_ERROR) and `toMatchObject` for builder-sourced `INVALID_ARGUMENT` (unsupported registry, tag-style version, unknown lifecycle). +`pkg_deps` shares its envelope builder with the CLI `githits pkg deps` command via `src/shared/package-dependencies-request.ts` and `src/shared/package-dependencies-response.ts`. The terminal formatter is CLI-only. The parity test (`src/tools/package-dependencies-parity.test.ts`) asserts `toEqual` across every service-sourced success / error fixture (runtime, zero-dep, full-view, optional-lifecycle, multi-lifecycle, filter-matched-nothing, Crates-target-cfg dedup round-trip, transitive, versioned match / diff, NOT_FOUND, VERSION_NOT_FOUND, BACKEND_ERROR) and `toMatchObject` for builder-sourced `INVALID_ARGUMENT` (unsupported registry, tag-style version, unknown lifecycle). -### `package_changelog` response shape +### `pkg_changelog` response shape -**Data-first envelope.** The top level carries addressing (`registry` + `name` for spec addressing, or `repoUrl` for repo-URL addressing), `source` (`"releases"` / `"changelog_file"` / `"hexdocs"`), and `mode` (`"latest"` or `"range"`). Entries live under `entries: { count, items }` — matching the `{count, items}` shape used by `package_dependencies.runtime`. `count` is computed client-side from `items.length`, so the invariant holds regardless of backend drift. +**Data-first envelope.** The top level carries addressing (`registry` + `name` for spec addressing, or `repoUrl` for repo-URL addressing), `source` (`"releases"` / `"changelog_file"` / `"hexdocs"`), and `mode` (`"latest"` or `"range"`). Entries live under `entries: { count, items }` — matching the `{count, items}` shape used by `pkg_deps.runtime`. `count` is computed client-side from `items.length`, so the invariant holds regardless of backend drift. **Per-entry shape.** `{version, normalizedVersion?, publishedAt?, htmlUrl?, body?}`. `version` is kept in the envelope even when `null` so agents can write `items.map(e => e.version)` without guarding; every other nullable field is stripped when absent. `body` is additionally stripped when the caller set `include_bodies: false`. The backend's opaque per-entry `metadata` GenericJSON is deliberately dropped from the envelope in v1 — revisit via agent feedback. -**Dual addressing (`registry` + `package_name` XOR `repo_url`).** `package_changelog` is the only metadata-side MCP tool with dual addressing. `package_summary` / `package_vulnerabilities` / `package_dependencies` all accept only `registry` + `package_name` because they are registry-metadata lookups without repo-URL alternatives. `package_changelog` is intrinsically repo-level — its sources are GitHub Releases, CHANGELOG.md, and HexDocs — so `repoUrl` is a peer addressing mode, not a bolt-on. Future tool authors should not cargo-cult the asymmetry without reading this rationale. +**Dual addressing (`registry` + `package_name` XOR `repo_url`).** `pkg_changelog` is the only metadata-side MCP tool with dual addressing. `pkg_info` / `pkg_vulns` / `pkg_deps` all accept only `registry` + `package_name` because they are registry-metadata lookups without repo-URL alternatives. `pkg_changelog` is intrinsically repo-level — its sources are GitHub Releases, CHANGELOG.md, and HexDocs — so `repoUrl` is a peer addressing mode, not a bolt-on. Future tool authors should not cargo-cult the asymmetry without reading this rationale. **Mode selection.** `from_version` triggers range mode (returns every entry in `[fromVersion, toVersion]` with no cap). Latest mode is the default, capped by `limit` (1–50, backend default 10). `from_version` + `limit` is rejected client-side with `INVALID_ARGUMENT` rather than silently routed to one mode. @@ -104,27 +98,27 @@ Both expose the same tools with identical names, parameters, and descriptions. T **`filter.*` echo.** `filter` is emitted only when the caller explicitly supplied at least one of `from_version`, `to_version`, `limit`, or `git_ref`. Backend-default `limit: 10` / `toVersion: ` is never echoed. The request builder tracks explicit-vs-defaulted via an `explicitFilterFields` set so defaults don't round-trip as caller intent. -**Version validation.** Same rule as `package_vulnerabilities` / `package_dependencies`: tag-style `v`-prefixed inputs on `from_version` / `to_version` are rejected client-side with `INVALID_ARGUMENT`. `@` is also rejected — the `pkg changelog` family has no single-version query, and silently remapping to `to_version` would be a client-invented semantic shift. Hint text redirects callers to `--to` / `to_version`. +**Version validation.** Same rule as `pkg_vulns` / `pkg_deps`: tag-style `v`-prefixed inputs on `from_version` / `to_version` are rejected client-side with `INVALID_ARGUMENT`. `@` is also rejected — the `pkg changelog` family has no single-version query, and silently remapping to `to_version` would be a client-invented semantic shift. Hint text redirects callers to `--to` / `to_version`. **NOT_FOUND semantics.** Backend `source === null` is promoted to a typed `PackageIntelligenceChangelogSourceNotFoundError` at the service boundary, which the shared classifier routes to the `NOT_FOUND` envelope with a message naming the sources that were tried. Empty `entries.items: []` with a valid `source` is a success, not an error — "no entries in this range" is a legitimate neutral outcome. -**Overlap with `package_summary`.** `package_summary` already surfaces a short-form `recentChanges` block (from the backend's `latestChangelogs` field on `PackageSummaryResult`). For a quick "what shipped recently" glance embedded in a package overview, use `package_summary`. For the full range-capable, body-rich, `include_bodies`-toggleable changelog with `--no-body` timeline mode and repo-URL addressing, use `package_changelog`. +**Overlap with `pkg_info`.** `pkg_info` already surfaces a short-form `recentChanges` block (from the backend's `latestChangelogs` field on `PackageSummaryResult`). For a quick "what shipped recently" glance embedded in a package overview, use `pkg_info`. For the full range-capable, body-rich, `include_bodies`-toggleable changelog with `--no-body` timeline mode and repo-URL addressing, use `pkg_changelog`. -`package_changelog` shares its envelope builder with the CLI `githits pkg changelog` command via `src/shared/package-changelog-request.ts` and `src/shared/package-changelog-response.ts`. The terminal formatter is CLI-only. The parity test (`src/tools/package-changelog-parity.test.ts`) asserts `toEqual` across every service-sourced success / error fixture (happy latest, range mode, repo-URL addressing, `--no-body` / `include_bodies: false`, default bodies, empty entries, NOT_FOUND, PackageIntelligenceTargetNotFoundError, VERSION_NOT_FOUND, BACKEND_ERROR) and `toMatchObject` for builder-sourced `INVALID_ARGUMENT`. +`pkg_changelog` shares its envelope builder with the CLI `githits pkg changelog` command via `src/shared/package-changelog-request.ts` and `src/shared/package-changelog-response.ts`. The terminal formatter is CLI-only. The parity test (`src/tools/package-changelog-parity.test.ts`) asserts `toEqual` across every service-sourced success / error fixture (happy latest, range mode, repo-URL addressing, `--no-body` / `include_bodies: false`, default bodies, empty entries, NOT_FOUND, PackageIntelligenceTargetNotFoundError, VERSION_NOT_FOUND, BACKEND_ERROR) and `toMatchObject` for builder-sourced `INVALID_ARGUMENT`. -### `list_files` / `read_file` / `grep_repo` response shapes +### `code_files` / `code_read` / `code_grep` response shapes These three indexing-gated tools share an addressing and lifecycle contract (documented below) and then each projects its own data-first envelope. All three reuse the shipped `codeTargetSchema` + `resolveCodeTarget` from `src/tools/code-navigation-shared.ts` — no parallel addressing module. -**`list_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_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. -**`read_file` envelope**: `{registry?|repoUrl?+gitRef?, path, language?, totalLines?, startLine?, endLine?, content?, isBinary?}`. `path` (not `filePath`) so the key matches `list_files.files[].path` and `grep_repo.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?}`. `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. -**`grep_repo` envelope**: `{registry?|name?|repoUrl?+gitRef?, pattern, patternType, caseSensitive, matches: [{filePath, line, matchStartByte, matchEndByte, lineContent, contextBefore?, contextAfter?, fileContentHash?, fileIntent?}], nextCursor?, hasMore, truncatedReason, routeTaken?, filesScanned, filesInScope, binaryFilesSkipped, filesTooLargeSkipped, totalMatches, uniqueFilesMatched, indexedVersion?, resolution?, filter?}`. `filter` echoes only explicit caller filters. Match entries carry `filePath` so grep output chains directly into `read_file`. +**`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`. -### Indexing lifecycle (shared across `search_symbols`, `list_files`, `read_file`, `grep_repo`) +### Indexing lifecycle (shared across `code_files`, `code_read`, `code_grep`) -All four code-navigation tools share the same indexing-retry contract. The state can arrive through either an error response or a success sentinel (`codeIndexState: "INDEXING"`), and the service layer collapses both to the same typed `CodeNavigationIndexingError` before the envelope builder runs. Agents therefore never see a `codeIndexState` field in a success envelope; they branch on the error path instead. +All three code-navigation tools share the same indexing-retry contract. The state can arrive through either an error response or a success sentinel (`codeIndexState: "INDEXING"`), and the service layer collapses both to the same typed `CodeNavigationIndexingError` before the envelope builder runs. Agents therefore never see a `codeIndexState` field in a success envelope; they branch on the error path instead. **`INDEXING` error envelope**: ```json @@ -139,11 +133,11 @@ All four code-navigation tools share the same indexing-retry contract. The state } ``` -`details.availableVersions` is populated when the backend returned a list of already-indexed versions alongside the sentinel. Agents can pick one to retry against immediately without waiting. `read_file` / `fetchCodeContext` on the backend doesn't emit `availableVersions` on INDEXING responses, so its error detail carries only `indexingRef` — the MCP description calls this out so agents know to rely on the `wait_timeout_ms` retry path. +`details.availableVersions` is populated when the backend returned a list of already-indexed versions alongside the sentinel. Agents can pick one to retry against immediately without waiting. `code_read` / `fetchCodeContext` on the backend doesn't emit `availableVersions` on INDEXING responses, so its error detail carries only `indexingRef` — the MCP description calls this out so agents know to rely on the `wait_timeout_ms` retry path. **Retry default**: `DEFAULT_WAIT_TIMEOUT_MS = 20_000` (shared, defined in `src/shared/code-navigation-defaults.ts`). Applied inside each request builder so both CLI and MCP surfaces get the same default by construction. CLI's `--wait ` and MCP's `wait_timeout_ms` override. -**`FILE_NOT_FOUND` vs `NOT_FOUND`**: `read_file` / `grep_repo` 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. +**`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. ## Server instructions @@ -254,13 +248,9 @@ See `docs/guidelines/TESTING.md` for the full testing pattern. | `src/services/test-helpers.ts` | `createMockGitHitsService()` and `createMockCodeNavigationService()` factories | | `src/commands/mcp.ts` | Tool registration, MCP server setup, and TTY detection | | `src/services/githits-service.ts` | REST API client for example search, languages, and feedback | -| `src/services/code-navigation-service.ts` | Package/source service client for unified `search`, `search_status`, and `search_symbols` | +| `src/services/code-navigation-service.ts` | Package/source service client for unified `search`, `search_status`, `code_files`, `code_read`, and `code_grep` | | `src/shared/language-filter.ts` | Pure `filterLanguages()` function shared between MCP tool and CLI | -## Pending backend follow-ups - -- **`totalMatches` / pagination.** `searchSymbols.totalMatches` currently tracks `results.length` on the wire — equal to `returnedCount`, not the total before `limit`. Once the backend distinguishes them (and adds an `offset` arg), the CLI header flips from *"N match(es) (more available)"* to *"Showing N of M"* and a `--offset` flag can land for pagination. Tracked with the backend team; no frontend changes needed in the meantime. - ## Related Documentation - Backend tool definitions: `githits-backend/githits/api/mcp/server.py` diff --git a/src/cli.ts b/src/cli.ts index 22a4c2d4..c8cd5c42 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -58,10 +58,10 @@ program "after", ` Getting started: - githits init Set up MCP for your coding agents - githits login Authenticate with your GitHits account - githits mcp Start MCP server for your AI assistant - githits example "query" --lang python Get code examples + githits init Set up MCP for your coding agents + githits login Authenticate with your GitHits account + githits mcp Show MCP setup instructions + githits example "query" -l python Get code examples Learn more at https://githits.com Docs: https://app.githits.com/docs/ @@ -83,28 +83,30 @@ registerExampleCommand(program); registerLanguagesCommand(program); registerFeedbackCommand(program); const argv = process.argv.slice(2); -const helpInvocation = isHelpInvocation(argv); -const shouldLoadGatedHelpRegistration = needsGatedHelpRegistration(argv); +const registrationArgv = stripRootRegistrationOptions(argv); +const helpInvocation = isHelpInvocation(registrationArgv); +const shouldLoadGatedHelpRegistration = + needsGatedHelpRegistration(registrationArgv); const helpRegistrationOptions = shouldLoadGatedHelpRegistration - ? await loadHelpRegistrationOptions() + ? await loadHelpRegistrationOptions(registrationArgv) : undefined; -if (shouldEagerLoadSearchCommands(argv)) { +if (shouldEagerLoadSearchCommands(registrationArgv)) { await withTelemetrySpan("cli.register.search", () => registerUnifiedSearchCommands(program, helpRegistrationOptions), ); } -if (shouldEagerLoadGatedCommandGroup(argv, "code")) { +if (shouldEagerLoadGatedCommandGroup(registrationArgv, "code")) { await withTelemetrySpan("cli.register.code-group", () => registerCodeCommandGroup(program, helpRegistrationOptions), ); } -if (shouldEagerLoadGatedCommandGroup(argv, "pkg")) { +if (shouldEagerLoadGatedCommandGroup(registrationArgv, "pkg")) { await withTelemetrySpan("cli.register.pkg-group", () => registerPkgCommandGroup(program, helpRegistrationOptions), ); } -if (shouldEagerLoadGatedCommandGroup(argv, "docs")) { +if (shouldEagerLoadGatedCommandGroup(registrationArgv, "docs")) { await withTelemetrySpan("cli.register.docs-group", () => registerDocsCommandGroup(program, helpRegistrationOptions), ); @@ -119,6 +121,16 @@ registerAuthStatusCommand(authCommand); await withTelemetrySpan("cli.parse", () => program.parseAsync()); +/** + * Commander supports root options before subcommands, e.g. + * `githits --no-color pkg info`. Registration happens before Commander + * parses argv, so the lightweight gated-command sniff must ignore root-only + * flags or it will misclassify `--no-color` as the requested command. + */ +function stripRootRegistrationOptions(args: string[]): string[] { + return args.filter((arg) => arg !== "--no-color"); +} + /** * Argv-sniff optimisation for gated command groups. Returns `true` * when the user's invocation might need the group registered — i.e. @@ -144,9 +156,12 @@ function shouldEagerLoadGatedCommandGroup( function shouldEagerLoadSearchCommands(args: string[]): boolean { const [firstArg] = args; return ( + args.length === 0 || firstArg === "search" || firstArg === "search-status" || - (firstArg === "help" && isSearchHelpTarget(args[1])) + firstArg === "--help" || + firstArg === "-h" || + (firstArg === "help" && (!args[1] || isSearchHelpTarget(args[1]))) ); } @@ -186,7 +201,7 @@ function isSearchHelpTarget(value: string | undefined): boolean { return value === "search" || value === "search-status"; } -async function loadHelpRegistrationOptions() { +async function loadHelpRegistrationOptions(args: string[]) { const { resolveStartupCodeNavigationRegistrationState } = await import( "./container.js" ); @@ -194,7 +209,7 @@ async function loadHelpRegistrationOptions() { await resolveStartupCodeNavigationRegistrationState(); return { capability: registrationState.capability, - expiredStoredAuth: shouldUseExpiredStoredAuthFallbackForHelp(argv) + expiredStoredAuth: shouldUseExpiredStoredAuthFallbackForHelp(args) ? registrationState.expiredStoredAuth : false, }; diff --git a/src/commands/auth-status.test.ts b/src/commands/auth-status.test.ts index 0a26e37d..db66e024 100644 --- a/src/commands/auth-status.test.ts +++ b/src/commands/auth-status.test.ts @@ -120,16 +120,21 @@ describe("authStatusAction", () => { it("shows env token info when envApiToken is provided", async () => { const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + const envApiToken = createJwtToken({ + feature_flags: ["code_navigation"], + }); await authStatusAction( createDeps({ - envApiToken: createJwtToken({ feature_flags: ["code_navigation"] }), + envApiToken, }), ); const output = consoleSpy.mock.calls.map((c) => c[0]).join("\n"); expect(output).toContain("environment variable"); expect(output).toContain("GITHITS_API_TOKEN"); + expect(output).not.toContain("Token:"); + expect(output).not.toContain(envApiToken.slice(0, 8)); expect(output).toContain("Code navigation: enabled"); consoleSpy.mockRestore(); }); diff --git a/src/commands/auth-status.ts b/src/commands/auth-status.ts index b9f6b533..1a777929 100644 --- a/src/commands/auth-status.ts +++ b/src/commands/auth-status.ts @@ -67,7 +67,6 @@ export async function authStatusAction( if (envApiToken) { console.log("Authenticated via environment variable.\n"); console.log(` Source: GITHITS_API_TOKEN`); - console.log(` Token: ${envApiToken.slice(0, 8)}...`); displayCodeNavigationStatus( getCodeNavigationCapability(envApiToken), codeNavigationCliOverrideEnabled, diff --git a/src/commands/code/files.ts b/src/commands/code/files.ts index dfe2ab79..7560a038 100644 --- a/src/commands/code/files.ts +++ b/src/commands/code/files.ts @@ -174,12 +174,12 @@ export function registerCodeFilesCommand(pkgCommand: Command): Command { .summary("List files in an indexed dependency") .description(PKG_FILES_DESCRIPTION) .argument( - "[arg1]", - "In spec mode: package spec (e.g. npm:express). In --repo-url mode: the path-prefix.", + "[spec-or-prefix]", + "Spec mode: package spec (e.g. npm:express). Repo mode (with --repo-url): the path-prefix.", ) .argument( - "[arg2]", - "In spec mode: the path-prefix (literal directory, not a glob). Unused in --repo-url mode.", + "[path-prefix]", + "Spec mode only: literal directory prefix (not a glob). Ignored with --repo-url.", ) .option( "--repo-url ", diff --git a/src/commands/code/grep.ts b/src/commands/code/grep.ts index 0bb1abf9..6bfb8e11 100644 --- a/src/commands/code/grep.ts +++ b/src/commands/code/grep.ts @@ -270,16 +270,16 @@ export function registerCodeGrepCommand(pkgCommand: Command): Command { .summary("Deterministic text grep over indexed dependency source") .description(PKG_GREP_DESCRIPTION) .argument( - "[arg1]", - "In spec mode: package spec (e.g. npm:express). In --repo-url mode: the pattern.", + "[spec-or-pattern]", + "Spec mode: package spec (e.g. npm:express). Repo mode (with --repo-url): the pattern.", ) .argument( - "[arg2]", - "In spec mode: the pattern. In --repo-url mode: optional path-prefix.", + "[pattern-or-prefix]", + "Spec mode: the pattern. Repo mode: optional path-prefix.", ) .argument( - "[arg3]", - "In spec mode: optional path-prefix. Unused in --repo-url mode.", + "[path-prefix]", + "Spec mode only: optional path-prefix. Ignored with --repo-url.", ) .option( "--repo-url ", diff --git a/src/commands/code/index.test.ts b/src/commands/code/index.test.ts index 8ed6c905..65fe2c7c 100644 --- a/src/commands/code/index.test.ts +++ b/src/commands/code/index.test.ts @@ -29,7 +29,7 @@ describe("registerCodeCommandGroup", () => { ); expect(codeCommand).toBeDefined(); expect( - codeCommand?.commands.some((command) => command.name() === "search"), + codeCommand?.commands.some((command) => command.name() === "files"), ).toBe(true); }); @@ -46,7 +46,7 @@ describe("registerCodeCommandGroup", () => { ); expect(codeCommand).toBeDefined(); expect( - codeCommand?.commands.some((command) => command.name() === "search"), + codeCommand?.commands.some((command) => command.name() === "files"), ).toBe(true); }); diff --git a/src/commands/code/index.ts b/src/commands/code/index.ts index 00398140..5f09882b 100644 --- a/src/commands/code/index.ts +++ b/src/commands/code/index.ts @@ -7,7 +7,6 @@ import { import { registerCodeFilesCommand } from "./files.js"; import { registerCodeGrepCommand } from "./grep.js"; import { registerCodeReadCommand } from "./read.js"; -import { registerCodeSearchSymbolsCommand } from "./search-symbols.js"; export interface CodeCommandGroupOptions extends GatedCommandGroupOptions { capability?: CodeNavigationCapability; @@ -33,10 +32,9 @@ export async function registerCodeCommandGroup( .command("code") .summary("Source-level operations on indexed dependencies") .description( - "Search exact tokens, list files, read files, and grep substrings inside indexed dependency source. Every command accepts either `` (registry:name[@version]) or `--repo-url --git-ref `. For package-level metadata (versions, vulnerabilities, dependencies, changelog) use `githits pkg`.", + "List files, read files, and grep substrings inside indexed dependency source. Every command accepts either `` (registry:name[@version]) or `--repo-url --git-ref `. For symbol or unified discovery search use `githits search`; for package-level metadata use `githits pkg`.", ); - registerCodeSearchSymbolsCommand(codeCommand); registerCodeFilesCommand(codeCommand); registerCodeReadCommand(codeCommand); registerCodeGrepCommand(codeCommand); diff --git a/src/commands/code/search-symbols.test.ts b/src/commands/code/search-symbols.test.ts deleted file mode 100644 index c334c543..00000000 --- a/src/commands/code/search-symbols.test.ts +++ /dev/null @@ -1,754 +0,0 @@ -import { describe, expect, it, mock, spyOn } from "bun:test"; -import { CodeNavigationIndexingError } from "../../services/index.js"; -import { - createMockCodeNavigationService, - defaultSearchSymbolsResult, -} from "../../services/test-helpers.js"; -import { AuthRequiredError } from "../../shared/require-auth.js"; -import { - type SearchSymbolsCommandDependencies, - searchSymbolsAction, -} from "./search-symbols.js"; - -describe("searchSymbolsAction", () => { - const mcpUrl = "https://mcp.githits.com"; - - function createDeps( - overrides: Partial = {}, - ): SearchSymbolsCommandDependencies { - return { - codeNavigationService: createMockCodeNavigationService(), - codeNavigationUrl: "https://nav.example.com", - hasValidToken: true, - mcpUrl, - ...overrides, - }; - } - - it("prints human-readable results by default", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction("npm:express", "middleware", {}, createDeps()); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("1 match(es)"); - expect(output).toContain("useMiddleware"); - // Entry leads with `filePath:startLine-endLine [kind]`. - expect(output).toContain("src/app.js:42-48"); - expect(output).toContain("[function]"); - // Snippet is built from `code` and dedented, preserving structure. - expect(output).toContain("function useMiddleware(fn) {"); - consoleSpy.mockRestore(); - }); - - it("prints the shared JSON envelope when --json is provided", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - { json: true }, - createDeps(), - ); - - const output = consoleSpy.mock.calls[0]?.[0] as string; - const payload = JSON.parse(output); - expect(payload.results).toEqual(defaultSearchSymbolsResult.results); - expect(payload.returnedCount).toBe(1); - expect(payload.totalMatches).toBe(1); - expect(payload.hasMore).toBe(false); - expect(payload.version).toBe("4.18.0"); - expect(payload.query.target).toEqual({ - registry: "NPM", - packageName: "express", - version: undefined, - }); - expect(payload.query.fileIntent).toBe("all"); - expect(payload.query.defaulted).not.toContain("fileIntent"); - expect(payload.query.defaulted).toContain("waitTimeoutMs"); - expect(payload._warning).toBeUndefined(); - consoleSpy.mockRestore(); - }); - - it("prints the shared JSON error envelope on --json when the service fails", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction( - "npm:express", - "middleware", - { json: true }, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.reject( - new CodeNavigationIndexingError( - "Target is being indexed.", - "idx-123", - ), - ), - ), - }), - }), - ); - } catch { - // expected process.exit throw - } - - const output = errorSpy.mock.calls[0]?.[0] as string; - expect(JSON.parse(output)).toEqual({ - error: "Target is being indexed.", - code: "INDEXING", - retryable: true, - details: { indexingRef: "idx-123" }, - }); - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("surfaces the backend zero-result hint verbatim when it arrives", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "nonexistentterm12345", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "5.2.1", - hint: "120 chunks indexed across 45 files. Try broader search terms or use fetch_code_context to read specific files directly.", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain('No matches for "nonexistentterm12345"'); - expect(output).toContain("120 chunks indexed across 45 files"); - // Server hint replaces the client-side suggestion list. - expect(output).not.toContain("Try: drop --kind"); - consoleSpy.mockRestore(); - }); - - it("still filters the legacy '0 searchable chunks' phrasing when backend regresses", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "nonexistentterm12345", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "5.2.1", - hint: "Repository indexed but contains 0 searchable chunks.", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).not.toContain("0 searchable chunks"); - // Falls back to the client-side suggestion list. - expect(output).toContain("Try:"); - consoleSpy.mockRestore(); - }); - - it("rejects unknown registry prefixes with a clean error", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction("foobar:baz", "middleware", {}, createDeps()); - } catch { - // expected process.exit throw - } - - expect(errorSpy.mock.calls.map((call) => call[0]).join("\n")).toContain( - 'Unsupported registry "foobar"', - ); - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("sends no file-intent filter by default and preserves explicit intent choices", async () => { - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - - await searchSymbolsAction("npm:express", "middleware", {}, deps); - expect(searchSymbols.mock.calls[0]?.[0]?.fileIntent).toBeUndefined(); - - searchSymbols.mockClear(); - await searchSymbolsAction( - "npm:express", - "middleware", - { intent: "all" }, - deps, - ); - // `--intent all` resolves to "omit the GraphQL variable" — - // service call sees undefined fileIntent. - expect(searchSymbols.mock.calls[0]?.[0]?.fileIntent).toBeUndefined(); - - searchSymbols.mockClear(); - await searchSymbolsAction( - "npm:express", - "middleware", - { intent: "test" }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.fileIntent).toBe("TEST"); - - logSpy.mockRestore(); - }); - - it('echoes fileIntent: "all" in JSON output when --intent all is passed', async () => { - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - { intent: "all", json: true }, - createDeps(), - ); - - const payload = JSON.parse(logSpy.mock.calls[0]?.[0] as string); - expect(payload.query.fileIntent).toBe("all"); - expect(payload.query.defaulted).not.toContain("fileIntent"); - logSpy.mockRestore(); - }); - - it("parses --wait in seconds and converts to milliseconds at the service boundary", async () => { - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - { wait: "15" }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.waitTimeoutMs).toBe(15_000); - - searchSymbols.mockClear(); - await searchSymbolsAction( - "npm:express", - "middleware", - { wait: "5s" }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.waitTimeoutMs).toBe(5_000); - - logSpy.mockRestore(); - }); - - it("rejects invalid --wait inputs", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - const cases: Array<[string, string]> = [ - ["10ms", "seconds"], - ["abc", "between 0 and 60"], - ["-1", "between 0 and 60"], - ["61", "between 0 and 60"], - ]; - - for (const [value, substring] of cases) { - errorSpy.mockClear(); - try { - await searchSymbolsAction( - "npm:express", - "middleware", - { wait: value }, - createDeps(), - ); - } catch { - // expected process.exit throw - } - const output = errorSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain(substring); - } - - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("CLI parser errors classify as INVALID_ARGUMENT in JSON output", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction( - "npm:express", - "middleware", - { wait: "61", json: true }, - createDeps(), - ); - } catch { - // expected process.exit throw - } - - const output = errorSpy.mock.calls[0]?.[0] as string; - const parsed = JSON.parse(output); - expect(parsed.code).toBe("INVALID_ARGUMENT"); - expect(parsed.error).toContain("--wait"); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("suppresses the (requested X) annotation on trivial v-prefix differences", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [{ filePath: "lib/x.js", startLine: 1 }], - totalMatches: 1, - hasMore: false, - version: "v2.32.3", - resolution: { - requestedVersion: "2.32.3", - resolvedRef: "v2.32.3", - }, - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("indexed v2.32.3"); - expect(output).not.toContain("(requested 2.32.3)"); - consoleSpy.mockRestore(); - }); - - it("truncates 40-char commit SHA refs to 7 characters in the header", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:lodash", - "debounce", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [{ filePath: "lodash.js", startLine: 1 }], - totalMatches: 1, - hasMore: false, - version: "4f0b76e2eca13de1c1fe8b4305abc1f7d63f4b86", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("indexed 4f0b76e"); - expect(output).not.toContain("4f0b76e2eca13de1c1fe8b4305abc1f7d63f4b86"); - consoleSpy.mockRestore(); - }); - - it("suggests --intent all only when the caller explicitly narrowed file intent", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "Router", - { file: "nonexistent/", kind: "function", intent: "test" }, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "v5.2.1", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("drop --kind"); - expect(output).toContain("broaden or remove --file"); - expect(output).toContain("try --intent all"); - consoleSpy.mockRestore(); - }); - - it("omits `try --intent all` from the zero-result suggestion when the caller already chose all", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "Router", - { intent: "all" }, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "v5.2.1", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("try broader keywords"); - expect(output).not.toContain("try --intent all"); - consoleSpy.mockRestore(); - }); - - it("omits `try --intent all` from the zero-result suggestion when no intent filter was sent", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "Router", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "v5.2.1", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("try broader keywords"); - expect(output).not.toContain("try --intent all"); - consoleSpy.mockRestore(); - }); - - it("accepts --category and passes through to the service and echo", async () => { - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "Router", - { category: "callable", json: true }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.category).toBe("CALLABLE"); - - const payload = JSON.parse(logSpy.mock.calls[0]?.[0] as string); - expect(payload.query.category).toBe("callable"); - logSpy.mockRestore(); - }); - - it("rejects unknown --category values with a clean error", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction( - "npm:express", - "Router", - { category: "xyzzy" }, - createDeps(), - ); - } catch { - // expected process.exit throw - } - - const output = errorSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("--category must be one of"); - expect(output).toContain("callable"); - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("accepts expanded --kind values from the unified taxonomy", async () => { - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "crates:serde", - "Serialize", - { kind: "trait" }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.kind).toBe("TRAIT"); - - searchSymbols.mockClear(); - await searchSymbolsAction( - "npm:express", - "Router", - { kind: "namespace" }, - deps, - ); - expect(searchSymbols.mock.calls[0]?.[0]?.kind).toBe("NAMESPACE"); - - logSpy.mockRestore(); - }); - - it("adds `drop --category` to zero-result suggestions when a category was used", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "Router", - { category: "type" }, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [], - totalMatches: 0, - hasMore: false, - version: "v5.2.1", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("drop --category"); - consoleSpy.mockRestore(); - }); - - it("suppresses the `[fallback]` kind label in terminal output", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.resolve({ - results: [ - { - filePath: "lib/middleware/init.js", - startLine: 1, - endLine: 43, - kind: "fallback", - code: "module.exports = function () { return true; };", - }, - ], - totalMatches: 1, - hasMore: false, - version: "v5.2.1", - }), - ), - }), - }), - ); - - const output = consoleSpy.mock.calls.map((call) => call[0]).join("\n"); - expect(output).toContain("lib/middleware/init.js:1-43"); - expect(output).not.toContain("[fallback]"); - consoleSpy.mockRestore(); - }); - - it("accepts --kind, --intent, and --match-mode values case-insensitively", async () => { - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - "middleware", - { kind: "FUNCTION", intent: "TEST", matchMode: "AND" }, - deps, - ); - - expect(searchSymbols.mock.calls[0]?.[0]).toMatchObject({ - kind: "FUNCTION", - fileIntent: "TEST", - matchMode: "AND", - }); - logSpy.mockRestore(); - }); - - it("merges repeatable --keyword with comma-separated --keywords", async () => { - const searchSymbols = mock< - ( - params: import("../../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const deps = createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols, - }), - }); - - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - - await searchSymbolsAction( - "npm:express", - undefined, - { keywords: "router,handler", keyword: ["middleware", "router"] }, - deps, - ); - - // De-duplicates "router" while preserving first-seen order. - expect(searchSymbols.mock.calls[0]?.[0]?.keywords).toEqual([ - "router", - "handler", - "middleware", - ]); - - logSpy.mockRestore(); - }); - - it("throws AuthRequiredError when no valid token is available", async () => { - const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); - - await expect( - searchSymbolsAction( - "npm:express", - "middleware", - {}, - createDeps({ hasValidToken: false }), - ), - ).rejects.toThrow(AuthRequiredError); - - consoleSpy.mockRestore(); - }); - - it("exits when indexing is still in progress", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction( - "npm:express", - "middleware", - {}, - createDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.reject( - new CodeNavigationIndexingError( - "Target is being indexed.", - "idx-123", - ), - ), - ), - }), - }), - ); - } catch { - // expected process.exit throw - } - - expect(errorSpy.mock.calls.map((call) => call[0]).join("\n")).toContain( - "Target is being indexed", - ); - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); - - it("exits when both query and keywords are missing", async () => { - const errorSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - - try { - await searchSymbolsAction("npm:express", undefined, {}, createDeps()); - } catch { - // expected process.exit throw - } - - expect(errorSpy.mock.calls.map((call) => call[0]).join("\n")).toContain( - "Provide a query argument, or pass keywords via --keywords or repeated --keyword.", - ); - expect(exitSpy).toHaveBeenCalledWith(1); - errorSpy.mockRestore(); - exitSpy.mockRestore(); - }); -}); diff --git a/src/commands/code/search-symbols.ts b/src/commands/code/search-symbols.ts deleted file mode 100644 index e65128e7..00000000 --- a/src/commands/code/search-symbols.ts +++ /dev/null @@ -1,525 +0,0 @@ -import type { Command } from "commander"; -import { createContainer } from "../../container.js"; -import type { - CodeNavigationService, - SearchSymbolsResult, - SearchSymbolsResultEntry, -} from "../../services/index.js"; -import { - AuthRequiredError, - buildSearchSymbolsErrorPayload, - buildSearchSymbolsParams, - buildSearchSymbolsSuccessPayload, - FILE_INTENT_ALL, - type FileIntentInput, - InvalidArgumentError, - knownSymbolCategoryList, - knownSymbolKindList, - mapCodeNavigationError, - normaliseKeywords, - parsePackageSpec, - requireAuth, - type SearchSymbolsSuccessPayload, - toCodeNavigationRegistry, - toSearchSymbolsFileIntent, - toSearchSymbolsKind, - toSearchSymbolsMatchMode, - toSymbolCategory, -} from "../../shared/index.js"; - -export interface SearchSymbolsCommandOptions { - kind?: string; - category?: string; - limit?: string; - keywords?: string; - keyword?: string[]; // repeatable - matchMode?: string; - file?: string; - intent?: string; - wait?: string; - json?: boolean; -} - -export interface SearchSymbolsCommandDependencies { - codeNavigationService: CodeNavigationService | undefined; - codeNavigationUrl: string | undefined; - hasValidToken: boolean; - mcpUrl: string; -} - -/** - * Core code navigation search command logic. - */ -export async function searchSymbolsAction( - packageArg: string, - query: string | undefined, - options: SearchSymbolsCommandOptions, - deps: SearchSymbolsCommandDependencies, -): Promise { - requireAuth(deps); - - try { - if (!deps.codeNavigationUrl || !deps.codeNavigationService) { - throw new InvalidArgumentError( - "Code navigation is not configured for this environment.", - ); - } - - const keywords = normaliseKeywords(options.keywords, options.keyword); - if (!query && keywords.length === 0) { - throw new InvalidArgumentError( - "Provide a query argument, or pass keywords via --keywords or repeated --keyword.", - ); - } - - const parsed = parsePackageSpec(packageArg); - - const { params, defaulted } = buildSearchSymbolsParams({ - target: { - registry: toCodeNavigationRegistry(parsed.registry), - packageName: parsed.name, - version: parsed.version, - }, - query, - keywords: keywords.length > 0 ? keywords : undefined, - matchMode: parseMatchMode(options.matchMode), - kind: parseKind(options.kind), - category: parseCategory(options.category), - filePath: options.file, - limit: parseOptionalInt(options.limit, "--limit", 1, 50), - fileIntent: parseIntent(options.intent), - waitTimeoutMs: parseWaitSeconds(options.wait), - }); - - const result = await deps.codeNavigationService.searchSymbols(params); - const payload = buildSearchSymbolsSuccessPayload(params, defaulted, result); - - if (options.json) { - console.log(JSON.stringify(payload)); - return; - } - - console.log( - formatSearchSymbolsTerminal( - payload, - parsed.registry, - parsed.name, - parsed.version, - query, - ), - ); - } catch (error) { - handleSearchSymbolsCommandError(error, options.json ?? false); - } -} - -const SEARCH_SYMBOLS_DESCRIPTION = `Find functions, classes, modules, and doc sections inside an indexed dependency by exact-token search. - -Prefer top-level \`githits search --source symbol\` for new symbol-shaped workflows. \`githits code search\` remains available for the older dedicated symbol-search UX and JSON parity contract. - -Package spec: :[@]. Omit the registry to default to -npm. Supported registries: npm, pypi, hex, crates, nuget, maven, zig, vcpkg, -packagist. - -Filter by --category (broad: callable, type, module, data, documentation) -or --kind (precise: function, method, class, trait, …). Prefer --category -for most use cases; reach for --kind when you need a specific construct. - -By default no file-intent filter is sent. Pass --intent production (or another -specific intent) to narrow results; --intent all remains accepted as an explicit -no-filter alias. - -Examples: - githits code search npm:express middleware - githits code search npm:express middleware --intent all - githits code search pypi:requests timeout --category callable --limit 10 - githits code search crates:serde Serialize --kind trait --limit 5 - githits code search npm:@types/node Buffer --file src/ --json - githits code search npm:express --keywords "router,handler" --match-mode and`; - -/** - * Register the `code search` command. `search-symbols` is kept as an - * alias for continuity but is not the primary surface. - */ -export function registerCodeSearchSymbolsCommand(program: Command) { - program - .command("search [query]") - .alias("search-symbols") - .summary("Legacy symbol search over indexed package source") - .description(SEARCH_SYMBOLS_DESCRIPTION) - .option( - "--category ", - "Filter by broad symbol category: callable, type, module, data, documentation. Preferred over --kind for most use cases.", - ) - .option( - "--kind ", - "Filter by precise symbol kind (function, method, constructor, getter, setter, class, interface, trait, struct, enum, record, module, namespace, property, event, etc.). Prefer --category for broad filtering.", - ) - .option("--limit ", "Max results (1-50; default: 25)") - .option( - "--keywords ", - "Comma-separated keywords (alternative to the query argument)", - ) - .option( - "--keyword ", - "Single keyword (repeatable; combines with --keywords)", - collectRepeatable, - [] as string[], - ) - .option( - "--match-mode ", - "How to combine keywords: or (any match) or and (all match)", - ) - .option("--file ", "Filter to files matching path prefix") - .option( - "--intent ", - "File intent filter (production, test, benchmark, example, generated, fixture, build, vendor, all). Omit for no filter; `all` is an explicit alias.", - ) - .option( - "--wait ", - "Max seconds to wait for indexing (0-60; default: 20). Accepts `10` or `10s`. Indexing usually completes within 30 seconds; pass `--wait 60` to block on a first-time request.", - ) - .option("--json", "Output as JSON") - .action( - async ( - packageArg: string, - query: string | undefined, - options: SearchSymbolsCommandOptions, - ) => { - try { - const deps = await createContainer(); - await searchSymbolsAction(packageArg, query, options, deps); - } catch (error) { - if (error instanceof AuthRequiredError) { - process.exit(1); - } - throw error; - } - }, - ); -} - -function collectRepeatable(value: string, previous: string[]): string[] { - return [...previous, value]; -} - -/** - * Terminal formatter. Uses the DETAILED-mode response fields: every - * entry leads with `filePath:startLine-endLine [kind]`, followed by - * the symbol name (when present) and a 3-line dedented snippet taken - * from `code`. Preserves indentation — does NOT collapse whitespace. - */ -function formatSearchSymbolsTerminal( - payload: SearchSymbolsSuccessPayload, - registry: string, - packageName: string, - version: string | undefined, - query: string | undefined, -): string { - const lines: string[] = []; - - if (payload.warning) { - lines.push(`Warning: ${payload.warning}`); - lines.push(""); - } - - lines.push(formatHeader(payload)); - lines.push(""); - - if (payload.results.length === 0) { - lines.push( - formatZeroResultMessage( - query, - registry, - packageName, - version, - payload.query, - payload.hint, - ), - ); - return lines.join("\n").trimEnd(); - } - - for (const entry of payload.results) { - lines.push(...formatEntry(entry)); - lines.push(""); - } - - if (payload.hint) { - lines.push(`Note: ${payload.hint}`); - } - - return lines.join("\n").trimEnd(); -} - -function formatHeader(payload: SearchSymbolsSuccessPayload): string { - // `totalMatches` currently tracks `results.length` on the backend - // (see backend request B2). Until a real total is available, say - // "N match(es) (more available)" rather than lying with "of N". - let summary = `${payload.returnedCount} match(es)`; - if (payload.hasMore) summary += " (more available)"; - - if (payload.version) { - summary += ` · indexed ${displayVersion(payload.version)}`; - const requested = - payload.resolution?.requestedVersion ?? payload.resolution?.requestedRef; - if ( - requested && - !isTrivialRefDifference(requested, payload.version) && - !isTrivialRefDifference(requested, payload.resolution?.resolvedRef) - ) { - summary += ` (requested ${requested})`; - } - } - - return summary; -} - -/** - * Shorten long refs (commit SHAs) to an abbreviated form for display, - * preserve tag-style versions unchanged. - */ -function displayVersion(version: string): string { - // 40-char hex is a full SHA; truncate to 7. - if (/^[0-9a-f]{40}$/i.test(version)) return version.slice(0, 7); - return version; -} - -/** - * Suppress the "(requested X)" annotation when the only difference - * between the caller's ref and the resolved/indexed ref is a leading - * `v` prefix (backend normalisation). Users asking for `2.32.3` who - * got `v2.32.3` back should not be told they got a different version. - */ -function isTrivialRefDifference( - requested: string, - resolved: string | undefined, -): boolean { - if (!resolved) return false; - if (requested === resolved) return true; - const stripV = (v: string) => (v.startsWith("v") ? v.slice(1) : v); - return stripV(requested) === stripV(resolved); -} - -function formatEntry(entry: SearchSymbolsResultEntry): string[] { - const out: string[] = []; - const locationParts: string[] = []; - - if (entry.filePath) { - if (entry.startLine) { - locationParts.push( - entry.endLine && entry.endLine !== entry.startLine - ? `${entry.filePath}:${entry.startLine}-${entry.endLine}` - : `${entry.filePath}:${entry.startLine}`, - ); - } else { - locationParts.push(entry.filePath); - } - } - - const kindLabel = resolveKindLabel(entry); - if (kindLabel) locationParts.push(`[${kindLabel}]`); - - out.push( - locationParts.length > 0 ? locationParts.join(" ") : "unnamed match", - ); - - if (entry.name) out.push(` ${entry.name}`); - - const snippet = buildSnippet(entry.code); - for (const snippetLine of snippet) out.push(snippetLine); - - return out; -} - -/** - * Resolve the bracketed kind label shown at the end of the first - * per-entry line. The backend populates `kind` for every chunk - * (handling its own fallback from chunk-level classification to - * symbol-enrichment kind internally), so the client reads a single - * source of truth. - * - * The literal `"fallback"` label is suppressed — backend emits it - * for unclassified chunks where no taxonomy hit, and showing it to - * the user adds noise without signal. - */ -function resolveKindLabel(entry: SearchSymbolsResultEntry): string | undefined { - if (!entry.kind) return undefined; - const normalised = entry.kind.toLowerCase(); - if (normalised === "fallback") return undefined; - return normalised; -} - -/** - * Produce a short, indented, dedented snippet from the `code` field. - * The backend exposes `preview` in DETAILED mode, but the formatter - * owns snippet rendering client-side so we can control the truncation - * length and preserve indentation consistently. - */ -function buildSnippet(code: string | undefined, maxLines = 3): string[] { - if (!code) return []; - - const raw = code.split("\n"); - // Trim surrounding blank lines to avoid an empty first/last line in - // the snippet. - while (raw.length > 0 && raw[0]?.trim() === "") raw.shift(); - while (raw.length > 0 && raw[raw.length - 1]?.trim() === "") raw.pop(); - if (raw.length === 0) return []; - - const indent = commonLeadingIndent(raw); - const dedented = raw.map((line) => (indent > 0 ? line.slice(indent) : line)); - const truncated = dedented.length > maxLines; - const selected = dedented.slice(0, maxLines); - const visible = truncated ? [...selected, "…"] : selected; - return visible.map((line) => ` ${line}`); -} - -function commonLeadingIndent(lines: string[]): number { - let min = Number.POSITIVE_INFINITY; - for (const line of lines) { - if (line.trim() === "") continue; - const match = line.match(/^(\s*)/); - const len = match?.[1]?.length ?? 0; - if (len < min) min = len; - } - return Number.isFinite(min) ? min : 0; -} - -function formatZeroResultMessage( - query: string | undefined, - registry: string, - packageName: string, - version: string | undefined, - echo: SearchSymbolsSuccessPayload["query"], - serverHint: string | undefined, -): string { - const target = version - ? `${registry}:${packageName}@${version}` - : `${registry}:${packageName}`; - const queryText = query ? `"${query}"` : "the given keywords"; - const header = `No matches for ${queryText} in ${target}.`; - - // Prefer the server hint when present — the April 2026 backend - // rewrite made zero-result hints accurate (chunk/file counts, - // docs-only guidance). Fall back to the client-side suggestion - // list built from the filters the caller actually applied. - if (serverHint) { - return [header, serverHint].join("\n"); - } - - const suggestions: string[] = []; - if (echo.kind) suggestions.push("drop --kind"); - if (echo.category) suggestions.push("drop --category"); - if (echo.filePath) suggestions.push("broaden or remove --file"); - if (echo.fileIntent !== "all") suggestions.push("try --intent all"); - if (echo.matchMode === "and") suggestions.push("try --match-mode or"); - suggestions.push("try broader keywords"); - - return [header, `Try: ${suggestions.join(", ")}.`].join("\n"); -} - -function handleSearchSymbolsCommandError(error: unknown, json: boolean): never { - if (json) { - console.error(JSON.stringify(buildSearchSymbolsErrorPayload(error))); - process.exit(1); - } - - const mapped = mapCodeNavigationError(error); - console.error(`Failed to search symbols: ${mapped.message}`); - process.exit(1); -} - -function parseOptionalInt( - value: string | undefined, - optionName: string, - min: number, - max: number, -): number | undefined { - if (value === undefined) return undefined; - - const parsed = Number.parseInt(value, 10); - if (Number.isNaN(parsed) || parsed < min || parsed > max) { - throw new InvalidArgumentError( - `${optionName} must be a number between ${min} and ${max}`, - ); - } - - return parsed; -} - -function parseMatchMode(value: string | undefined) { - if (!value) return undefined; - const parsed = toSearchSymbolsMatchMode(value.toLowerCase()); - if (!parsed) { - throw new InvalidArgumentError("--match-mode must be 'or' or 'and'"); - } - return parsed; -} - -function parseKind(value: string | undefined) { - if (!value) return undefined; - const parsed = toSearchSymbolsKind(value.toLowerCase()); - if (!parsed) { - throw new InvalidArgumentError( - `--kind must be one of ${knownSymbolKindList().join(", ")}`, - ); - } - return parsed; -} - -function parseCategory(value: string | undefined) { - if (!value) return undefined; - const parsed = toSymbolCategory(value.toLowerCase()); - if (!parsed) { - throw new InvalidArgumentError( - `--category must be one of ${knownSymbolCategoryList().join(", ")}`, - ); - } - return parsed; -} - -function parseIntent(value: string | undefined): FileIntentInput { - if (value === undefined) return undefined; - const lower = value.toLowerCase(); - if (lower === "all") return FILE_INTENT_ALL; - const parsed = toSearchSymbolsFileIntent(lower); - if (!parsed) { - throw new InvalidArgumentError( - "--intent must be one of production, test, benchmark, example, generated, fixture, build, vendor, or all", - ); - } - return parsed; -} - -/** - * Parse the `--wait` flag as a seconds value, internally converting to - * milliseconds for the service layer. Accepts `10` or `10s`. Rejects - * negative values, non-numeric input, and values beyond the 60-second - * cap. - */ -function parseWaitSeconds(value: string | undefined): number | undefined { - if (value === undefined) return undefined; - const trimmed = value.trim(); - const withoutSuffix = trimmed.endsWith("s") ? trimmed.slice(0, -1) : trimmed; - if (trimmed.endsWith("ms")) { - throw new InvalidArgumentError( - "--wait is specified in seconds (e.g. `10` or `10s`), not milliseconds.", - ); - } - const parsed = Number.parseInt(withoutSuffix, 10); - if ( - Number.isNaN(parsed) || - parsed < 0 || - parsed > 60 || - withoutSuffix !== String(parsed) - ) { - throw new InvalidArgumentError( - "--wait must be a number of seconds between 0 and 60 (e.g. `10` or `10s`).", - ); - } - return parsed * 1000; -} - -// The `SearchSymbolsResult` type is imported above; retained so -// ambient TS modules that reference it through this file still work. -export type { SearchSymbolsResult }; diff --git a/src/commands/docs/list.test.ts b/src/commands/docs/list.test.ts index b1988c63..b97afd33 100644 --- a/src/commands/docs/list.test.ts +++ b/src/commands/docs/list.test.ts @@ -47,8 +47,8 @@ describe("docsListAction", () => { const payload = JSON.parse(String(logSpy.mock.calls[0]?.[0])); expect(payload.name).toBe("express"); - expect(payload.pages[0].followUp.pageId).toBe("123-getting-started"); - expect(payload.pages[1].readFile.path).toBe("README.md"); + expect(payload.pages[0].pageId).toBe("123-getting-started"); + expect(payload.pages[1].filePath).toBe("README.md"); logSpy.mockRestore(); }); diff --git a/src/commands/docs/read.test.ts b/src/commands/docs/read.test.ts index ac2b3109..c6a93e42 100644 --- a/src/commands/docs/read.test.ts +++ b/src/commands/docs/read.test.ts @@ -52,7 +52,7 @@ describe("docsReadAction", () => { const payload = JSON.parse(String(logSpy.mock.calls[0]?.[0])); expect(payload.pageId).toBe("github:expressjs/express@abc123/README.md"); - expect(payload.readFile.path).toBe("README.md"); + expect(payload.filePath).toBe("README.md"); logSpy.mockRestore(); }); diff --git a/src/commands/docs/read.ts b/src/commands/docs/read.ts index 459d0f9f..30d24aec 100644 --- a/src/commands/docs/read.ts +++ b/src/commands/docs/read.ts @@ -7,6 +7,7 @@ import { formatReadPackageDocTerminal, InvalidPackageSpecError, mapPackageIntelligenceError, + parseLinesOption, requireAuth, shouldUseColors, } from "../../shared/index.js"; @@ -14,6 +15,7 @@ import { export interface DocsReadCommandOptions { verbose?: boolean; json?: boolean; + lines?: string; } export interface DocsReadCommandDependencies { @@ -37,6 +39,8 @@ export async function docsReadAction( ); } + const range = options.lines ? parseLinesOption(options.lines) : undefined; + const build = buildReadPackageDocParams({ pageId }); const result = await deps.packageIntelligenceService.readPackageDoc( build.params, @@ -44,6 +48,7 @@ export async function docsReadAction( const payload = buildReadPackageDocSuccessPayload( result, build.params.pageId, + range, ); if (options.json) { @@ -85,8 +90,9 @@ const DOCS_READ_DESCRIPTION = `Read a documentation page by page ID. Use page IDs from githits docs list, githits search --json, or MCP doc/search results. Default output is content-only for easy piping; pass --verbose for a -metadata header. Repo-backed pages also expose exact file follow-up metadata in -JSON.`; +metadata header. Use --lines for a bounded line range (e.g. \`--lines 10-40\`, +\`--lines 10-\` for open-ended, or \`--lines -40\` for the first 40 lines) — +useful when a page is too long to read whole.`; export function registerDocsReadCommand(docsCommand: Command): Command { return docsCommand @@ -94,6 +100,10 @@ export function registerDocsReadCommand(docsCommand: Command): Command { .summary("Read a documentation page by page ID") .description(DOCS_READ_DESCRIPTION) .argument("", "Documentation page ID from docs/search results") + .option( + "--lines ", + "Bounded line range, e.g. 10-40, 10-, or -40 (1-indexed inclusive)", + ) .option("-v, --verbose", "Show metadata header before content") .option("--json", "Emit the JSON envelope") .action(async (pageId: string, options: DocsReadCommandOptions) => { diff --git a/src/commands/example.ts b/src/commands/example.ts index 4c6e9dd8..03e43c87 100644 --- a/src/commands/example.ts +++ b/src/commands/example.ts @@ -1,5 +1,6 @@ import { type Command, Option } from "commander"; import type { GitHitsService } from "../services/githits-service.js"; +import { extractSolutionId } from "../shared/extract-solution-id.js"; import { AuthRequiredError, requireAuth } from "../shared/require-auth.js"; export interface ExampleOptions { @@ -31,7 +32,11 @@ export async function exampleAction( }); if (options.json) { - console.log(JSON.stringify({ result })); + const solutionId = extractSolutionId(result); + const payload = solutionId + ? { result, solution_id: solutionId } + : { result }; + console.log(JSON.stringify(payload)); } else { console.log(result); } @@ -45,12 +50,7 @@ export async function exampleAction( const EXAMPLE_DESCRIPTION = `Get verified, canonical code examples from global open source. -This is the GitHits example-search surface. For dependency/package/repo source search, -use \ - - githits search - -instead. +For dependency, package, or repository source search, use \`githits search\` instead. Examples: githits example "how to use express middleware" --lang javascript diff --git a/src/commands/mcp-instructions.test.ts b/src/commands/mcp-instructions.test.ts index dce157d5..da307635 100644 --- a/src/commands/mcp-instructions.test.ts +++ b/src/commands/mcp-instructions.test.ts @@ -48,15 +48,15 @@ const KNOWN_TOOLS = [ "search_language", "feedback", "search_status", - "list_files", - "read_file", - "grep_repo", - "list_package_docs", - "read_package_doc", - "package_summary", - "package_vulnerabilities", - "package_dependencies", - "package_changelog", + "code_files", + "code_read", + "code_grep", + "docs_list", + "docs_read", + "pkg_info", + "pkg_vulns", + "pkg_deps", + "pkg_changelog", ] as const; function mentionedTools(instructions: string): Set { @@ -119,10 +119,10 @@ describe("buildMcpInstructions", () => { expect(instructions).toContain("feedback"); expect(instructions).toContain("get_example"); expect(instructions).not.toContain("Package tools"); - expect(instructions).not.toContain("package_summary"); - expect(instructions).not.toContain("package_vulnerabilities"); - expect(instructions).not.toContain("package_dependencies"); - expect(instructions).not.toContain("package_changelog"); + expect(instructions).not.toContain("pkg_info"); + expect(instructions).not.toContain("pkg_vulns"); + expect(instructions).not.toContain("pkg_deps"); + expect(instructions).not.toContain("pkg_changelog"); expect(instructions).not.toContain("search_status"); }); @@ -137,12 +137,12 @@ describe("buildMcpInstructions", () => { expect(instructions).toContain("GitHits surfaces verified"); expect(instructions).toContain("Package tools"); - expect(instructions).toContain("`package_summary`"); - expect(instructions).toContain("`list_package_docs`"); - expect(instructions).toContain("`read_package_doc`"); - expect(instructions).toContain("`package_vulnerabilities`"); - expect(instructions).toContain("`package_dependencies`"); - expect(instructions).toContain("`package_changelog`"); + expect(instructions).toContain("`pkg_info`"); + expect(instructions).toContain("`docs_list`"); + expect(instructions).toContain("`docs_read`"); + expect(instructions).toContain("`pkg_vulns`"); + expect(instructions).toContain("`pkg_deps`"); + expect(instructions).toContain("`pkg_changelog`"); expect(instructions).toContain("`search`"); expect(instructions).toContain("`search_status`"); expect(instructions).toContain("allow_partial_results"); @@ -185,7 +185,7 @@ describe("buildMcpInstructions", () => { expect(instructions).toContain("Package tools"); expect(instructions).toContain("`search`"); expect(instructions).toContain("`search_status`"); - expect(instructions).not.toContain("`package_summary`"); + expect(instructions).not.toContain("`pkg_info`"); expect(instructions).toContain("canonical example retrieval"); }); @@ -198,12 +198,12 @@ describe("buildMcpInstructions", () => { const instructions = buildMcpInstructions(deps); expect(instructions).toContain("Package tools"); - expect(instructions).toContain("`package_summary`"); - expect(instructions).toContain("`list_package_docs`"); - expect(instructions).toContain("`read_package_doc`"); - expect(instructions).toContain("`package_vulnerabilities`"); - expect(instructions).toContain("`package_dependencies`"); - expect(instructions).toContain("`package_changelog`"); + expect(instructions).toContain("`pkg_info`"); + expect(instructions).toContain("`docs_list`"); + expect(instructions).toContain("`docs_read`"); + expect(instructions).toContain("`pkg_vulns`"); + expect(instructions).toContain("`pkg_deps`"); + expect(instructions).toContain("`pkg_changelog`"); expect(instructions).not.toContain("`search_status`"); // The decision tip references unified search, so it must not // appear when unified search isn't registered. @@ -277,13 +277,13 @@ describe("buildMcpInstructions", () => { const packageTools = [ "search", "search_status", - "list_files", - "read_file", - "grep_repo", - "package_summary", - "package_vulnerabilities", - "package_dependencies", - "package_changelog", + "code_files", + "code_read", + "code_grep", + "pkg_info", + "pkg_vulns", + "pkg_deps", + "pkg_changelog", ]; for (const name of packageTools) { if (registered.has(name)) { diff --git a/src/commands/mcp-instructions.ts b/src/commands/mcp-instructions.ts index 66863f43..910d2e79 100644 --- a/src/commands/mcp-instructions.ts +++ b/src/commands/mcp-instructions.ts @@ -23,41 +23,41 @@ const PACKAGE_TOOLS_PREAMBLE = `Package tools work with third-party dependency s Package spec: \`registry:name[@version]\`.`; -const PACKAGE_SUMMARY_BULLET = - "- `package_summary` — instant package overview: latest version, license, downloads, quickstart, and active advisory count."; +const PKG_INFO_BULLET = + "- `pkg_info` — instant package overview: latest version, license, downloads, quickstart, and active advisory count."; -const LIST_PACKAGE_DOCS_BULLET = - "- `list_package_docs` — browse mixed package documentation pages from hosted docs and repository-backed docs. Each entry includes a stable pageId, source kind, source URL, and for repo docs exact file follow-up metadata."; +const DOCS_LIST_BULLET = + "- `docs_list` — browse mixed package documentation pages from hosted docs and repository-backed docs. Each entry includes a stable pageId, source kind, source URL, and for repo docs exact file follow-up metadata."; -const READ_PACKAGE_DOC_BULLET = - "- `read_package_doc` — read a documentation page by pageId. Works for both hosted docs and repo-backed docs. Repo-backed results additionally expose exact file follow-up metadata."; +const DOCS_READ_BULLET = + "- `docs_read` — read a documentation page by pageId. Works for both hosted docs and repo-backed docs."; -const PACKAGE_VULNERABILITIES_BULLET = - "- `package_vulnerabilities` — known CVE / OSV advisories for npm, PyPI, Hex, or Crates packages (optionally pinned to `@version`). Malicious-package advisories surface in a disjoint `malware` bucket; filter with `min_severity` or include retracted advisories with `include_withdrawn`."; +const PKG_VULNS_BULLET = + "- `pkg_vulns` — known CVE / OSV advisories for npm, PyPI, Hex, or Crates packages (optionally pinned to `@version`). Malicious-package advisories surface in a disjoint `malware` bucket; filter with `min_severity` or include retracted advisories with `include_withdrawn`."; -const PACKAGE_DEPENDENCIES_BULLET = - "- `package_dependencies` — direct runtime deps plus, when the backend has them, dev / peer / optional / feature groups. Pass `lifecycle` to filter groups server-side, `include_transitive` for the full graph, and `include_importers` when you also need per-package provenance. Supports npm, PyPI, Hex, Crates, vcpkg, and Zig."; +const PKG_DEPS_BULLET = + "- `pkg_deps` — direct runtime deps plus, when the backend has them, dev / peer / optional / feature groups. Pass `lifecycle` to filter groups server-side, `include_transitive` for the full graph, and `include_importers` when you also need per-package provenance. Supports npm, PyPI, Hex, Crates, vcpkg, and Zig."; -const PACKAGE_CHANGELOG_BULLET = - "- `package_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 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. Omit `file_intent` to search across all intents, or set it to narrow results; some sources may ignore that filter and report it in `sourceStatus`. 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. 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 LIST_FILES_BULLET = - "- `list_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 `read_file` and help scope `grep_repo`."; +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`."; -const READ_FILE_BULLET = - "- `read_file` — fetch a file's contents from a dependency. Pass the same `path` emitted by `list_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, not the null. A `FILE_NOT_FOUND` (or `NOT_FOUND`) response is the signal to call `list_files` for the actual path."; +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."; -const GREP_REPO_BULLET = - "- `grep_repo` — 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 `read_file`."; +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`."; 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 `grep_repo` for deterministic text matching and `read_file` 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 full-file inspection.'; /** * Whether the MCP session should register and describe package tools. @@ -97,19 +97,19 @@ export function buildMcpInstructions(deps: Dependencies): string { const bullets: string[] = []; if (deps.packageIntelligenceService) { - bullets.push(LIST_PACKAGE_DOCS_BULLET); - bullets.push(READ_PACKAGE_DOC_BULLET); - bullets.push(PACKAGE_SUMMARY_BULLET); - bullets.push(PACKAGE_VULNERABILITIES_BULLET); - bullets.push(PACKAGE_DEPENDENCIES_BULLET); - bullets.push(PACKAGE_CHANGELOG_BULLET); + bullets.push(DOCS_LIST_BULLET); + bullets.push(DOCS_READ_BULLET); + bullets.push(PKG_INFO_BULLET); + bullets.push(PKG_VULNS_BULLET); + bullets.push(PKG_DEPS_BULLET); + bullets.push(PKG_CHANGELOG_BULLET); } if (deps.codeNavigationService) { bullets.push(SEARCH_BULLET); bullets.push(SEARCH_STATUS_BULLET); - bullets.push(LIST_FILES_BULLET); - bullets.push(READ_FILE_BULLET); - bullets.push(GREP_REPO_BULLET); + bullets.push(CODE_FILES_BULLET); + bullets.push(CODE_READ_BULLET); + bullets.push(CODE_GREP_BULLET); } if (bullets.length === 0) { diff --git a/src/commands/mcp.test.ts b/src/commands/mcp.test.ts index c5786300..61a7aa53 100644 --- a/src/commands/mcp.test.ts +++ b/src/commands/mcp.test.ts @@ -79,9 +79,9 @@ describe("createMcpServer", () => { "feedback", "search", "search_status", - "list_files", - "read_file", - "grep_repo", + "code_files", + "code_read", + "code_grep", ]); }); @@ -107,9 +107,9 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).toContain("list_package_docs"); - expect(tools.map((tool) => tool.name)).toContain("read_package_doc"); - expect(tools.map((tool) => tool.name)).toContain("package_summary"); + expect(tools.map((tool) => tool.name)).toContain("docs_list"); + expect(tools.map((tool) => tool.name)).toContain("docs_read"); + expect(tools.map((tool) => tool.name)).toContain("pkg_info"); }); it("omits package_summary when capability is disabled", () => { @@ -120,7 +120,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).not.toContain("package_summary"); + expect(tools.map((tool) => tool.name)).not.toContain("pkg_info"); }); it("omits package_summary when service is missing even if capability enabled", () => { @@ -131,7 +131,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).not.toContain("package_summary"); + expect(tools.map((tool) => tool.name)).not.toContain("pkg_info"); }); it("adds package_summary for opaque env tokens when the gate is otherwise open", () => { @@ -143,7 +143,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.some((tool) => tool.name === "package_summary")).toBe(true); + expect(tools.some((tool) => tool.name === "pkg_info")).toBe(true); }); it("adds package and code-nav tools when local override is enabled", () => { @@ -156,8 +156,8 @@ describe("createMcpServer", () => { const names = getMcpToolDefinitions(deps).map((tool) => tool.name); expect(names).toContain("search"); - expect(names).toContain("list_package_docs"); - expect(names).toContain("read_package_doc"); + expect(names).toContain("docs_list"); + expect(names).toContain("docs_read"); }); it("preserves half-open invariant: whenever package_summary is advertised, unified search is too (enabled path)", () => { @@ -169,7 +169,7 @@ describe("createMcpServer", () => { }); const names = getMcpToolDefinitions(deps).map((t) => t.name); - if (names.includes("package_summary")) { + if (names.includes("pkg_info")) { expect(names).toContain("search"); expect(names).toContain("search_status"); } @@ -184,7 +184,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).toContain("package_vulnerabilities"); + expect(tools.map((tool) => tool.name)).toContain("pkg_vulns"); }); it("omits package_vulnerabilities when capability is disabled", () => { @@ -195,9 +195,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).not.toContain( - "package_vulnerabilities", - ); + expect(tools.map((tool) => tool.name)).not.toContain("pkg_vulns"); }); it("advertises package_summary and package_vulnerabilities together (shared predicate)", () => { @@ -209,8 +207,8 @@ describe("createMcpServer", () => { }); const names = getMcpToolDefinitions(deps).map((t) => t.name); - if (names.includes("package_summary")) { - expect(names).toContain("package_vulnerabilities"); + if (names.includes("pkg_info")) { + expect(names).toContain("pkg_vulns"); } }); @@ -222,7 +220,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).toContain("package_dependencies"); + expect(tools.map((tool) => tool.name)).toContain("pkg_deps"); }); it("omits package_dependencies when capability is disabled", () => { @@ -233,9 +231,7 @@ describe("createMcpServer", () => { }); const tools = getMcpToolDefinitions(deps); - expect(tools.map((tool) => tool.name)).not.toContain( - "package_dependencies", - ); + expect(tools.map((tool) => tool.name)).not.toContain("pkg_deps"); }); it("advertises every package tool together (shared predicate covers deps too)", () => { @@ -247,9 +243,9 @@ describe("createMcpServer", () => { }); const names = getMcpToolDefinitions(deps).map((t) => t.name); - if (names.includes("package_summary")) { - expect(names).toContain("package_vulnerabilities"); - expect(names).toContain("package_dependencies"); + if (names.includes("pkg_info")) { + expect(names).toContain("pkg_vulns"); + expect(names).toContain("pkg_deps"); } }); }); diff --git a/src/commands/pkg/info.ts b/src/commands/pkg/info.ts index ce889d84..d7015bd4 100644 --- a/src/commands/pkg/info.ts +++ b/src/commands/pkg/info.ts @@ -90,8 +90,7 @@ function handlePkgInfoCommandError(error: unknown, json: boolean): never { process.exit(1); } - // Bare mapped message — matches `search_symbols` structure, not its - // wording. Domain messages (`Package 'npm:foo' not found.`, + // Bare mapped message. Domain messages (`Package 'npm:foo' not found.`, // `pkg info always returns the latest version; omit @4.18.0.`) are // already caller-readable. console.error(mapped.message); diff --git a/src/commands/search.test.ts b/src/commands/search.test.ts index 8f41fd6d..efd45e0a 100644 --- a/src/commands/search.test.ts +++ b/src/commands/search.test.ts @@ -621,8 +621,7 @@ describe("searchStatusAction", () => { const payload = JSON.parse(String(consoleSpy.mock.calls[0]?.[0])); expect(payload.completed).toBe(true); expect(payload.searchRef).toBe("search-ref-123"); - expect(payload.result.query).toBe("router middleware"); - expect(payload.result.returnedCount).toBe(1); + expect(payload.result.results).toHaveLength(1); expect(payload).not.toHaveProperty("query.raw"); consoleSpy.mockRestore(); }); diff --git a/src/commands/search.ts b/src/commands/search.ts index d111c9b6..dbbc9612 100644 --- a/src/commands/search.ts +++ b/src/commands/search.ts @@ -22,9 +22,9 @@ import { parseUnifiedSearchTargetSpec, requireAuth, shouldUseColors, - toSearchSymbolsFileIntent, - toSearchSymbolsKind, + toFileIntent, toSymbolCategory, + toSymbolKind, type UnifiedSearchStatusIncompletePayload, type UnifiedSearchStatusResultPayload, } from "../shared/index.js"; @@ -80,10 +80,10 @@ export async function searchAction( targets: parseTargetSpecs(options.in), query, sources: parseSources(options.source), - kind: toSearchSymbolsKind(options.kind), + kind: toSymbolKind(options.kind), category: toSymbolCategory(options.category), pathPrefix: options.pathPrefix, - fileIntent: toSearchSymbolsFileIntent(options.intent), + fileIntent: toFileIntent(options.intent), publicOnly: options.public, name: options.name, language: options.lang, @@ -98,7 +98,6 @@ export async function searchAction( built.params, built.rawQuery, built.compiledQuery, - built.defaulted, outcome, ); @@ -152,32 +151,16 @@ export async function searchStatusAction( const SEARCH_DESCRIPTION = `Search code, docs, and symbols across indexed dependencies and repositories. -Use repeatable --in targets in package form (npm:express[@version]) or repo -form (https://github.com/org/repo[#ref]). Structured flags are AND-combined with -the discovery query. Omit --intent to search across all file intents; pass it -only when you want to narrow results. Results are complete-by-default: if indexing is -still in progress, search returns a searchRef instead of partial hits unless ---allow-partial is passed. - -Query syntax: - implicit AND foo bar - OR foo OR bar (must be uppercase) - grouping (foo OR bar) baz - exclude foo -bar - phrase "exact phrase" - qualifiers kind: category: path: lang: name: intent: - routing registry: package: version: repo: - -Decision guide: - githits example ... canonical cross-project examples - githits search ... indexed dependency/repository search - githits search --source symbol ... symbol-shaped unified search - -Plain output labels: - docs page hosted documentation page - repo doc documentation-like block from a repository file - repo code code block from a repository file - repo symbol explicit symbol hit from the repository index +Repeatable --in targets accept package form (npm:express[@version]) or repo +form (https://github.com/org/repo[#ref]). Structured flags are AND-combined +with the query. Complete by default — if indexing is still running, returns +a searchRef instead of partial hits unless --allow-partial is passed. Use +\`githits example\` for canonical cross-project examples; \`--source symbol\` +here returns symbol-shaped hits. + +The query supports implicit AND, uppercase OR, parens, unary -, "phrases", +and qualifiers (kind:, category:, path:, lang:, name:, intent:, registry:, +package:, version:, repo:). Examples: githits search "router middleware" --in npm:express @@ -424,7 +407,6 @@ function formatSearchErrorTerminal( function formatUnifiedSearchTerminal(payload: { completed: boolean; - returnedCount: number; hasMore: boolean; nextOffset?: number; results: Array<{ @@ -453,7 +435,7 @@ function formatUnifiedSearchTerminal(payload: { }>; searchRef?: string; progress?: { targetsReady?: number; targetsTotal?: number }; - query: { warnings?: string[]; defaulted?: ReadonlyArray }; + query: { warnings?: string[] }; sourceStatus?: SourceStatusEntry[]; }): string { const lines: string[] = []; @@ -480,10 +462,7 @@ function formatUnifiedSearchTerminal(payload: { lines.push("Partial results:"); } - const sourceStatusNotes = formatSourceStatusNotes( - payload.sourceStatus, - payload.query.defaulted, - ); + const sourceStatusNotes = formatSourceStatusNotes(payload.sourceStatus); if (payload.results.length === 0) { lines.push("No results."); @@ -594,45 +573,16 @@ function formatSearchStatusHeadline(status: string | undefined): string { function formatSearchStatusCompletedTerminal(payload: { completed: true; searchRef?: string; - progress?: { status?: string }; - result: { - query: string; - queryWarnings: string[]; - returnedCount: number; - hasMore: boolean; - nextOffset?: number; - sourceStatus?: SourceStatusEntry[]; - results: Array<{ - type: string; - target: string; - title?: string; - summary?: string; - highlights?: { - title?: Array; - summary?: Array; - }; - locator: { - filePath?: string; - gitRef?: string; - startLine?: number; - endLine?: number; - pageId?: string; - sourceKind?: string; - sourceUrl?: string; - requestedRef?: string; - }; - }>; - }; + result: UnifiedSearchStatusResultPayload; }): string { return formatUnifiedSearchTerminal({ completed: true, - returnedCount: payload.result.returnedCount, hasMore: payload.result.hasMore, nextOffset: payload.result.nextOffset, results: payload.result.results, searchRef: payload.searchRef, progress: undefined, - query: { warnings: payload.result.queryWarnings }, + query: { warnings: payload.result.warnings }, sourceStatus: payload.result.sourceStatus, }); } @@ -644,13 +594,12 @@ function formatSearchStatusPartialTerminal( ): string { return formatUnifiedSearchTerminal({ completed: false, - returnedCount: payload.result.returnedCount, hasMore: payload.result.hasMore, nextOffset: payload.result.nextOffset, results: payload.result.results, searchRef: payload.searchRef, progress: payload.progress, - query: { warnings: payload.result.queryWarnings }, + query: { warnings: payload.result.warnings }, sourceStatus: payload.result.sourceStatus, }); } @@ -658,8 +607,9 @@ function formatSearchStatusPartialTerminal( interface SourceStatusEntry { source: string; targetLabel: string; - ignoredFilters: string[]; - incompatibleFilters: string[]; + indexingStatus?: string; + ignoredFilters?: string[]; + incompatibleFilters?: string[]; ignoredQueryFeatures?: string[]; incompatibleQueryFeatures?: string[]; note?: string; @@ -667,37 +617,27 @@ interface SourceStatusEntry { function formatSourceStatusNotes( sourceStatus: SourceStatusEntry[] | undefined, - defaulted: ReadonlyArray | undefined, ): string[] { const useColors = shouldUseColors(); if (!sourceStatus) { return []; } - const defaultedSet = new Set(defaulted ?? []); const lines: string[] = []; for (const entry of sourceStatus) { const label = `${entry.source.toLowerCase()} on ${entry.targetLabel}`; - const ignoredFilters = entry.ignoredFilters.filter( - (name) => !defaultedSet.has(name), - ); - if (ignoredFilters.length > 0) { + if (entry.ignoredFilters && entry.ignoredFilters.length > 0) { lines.push( dim( - `Note: ${label} ignored filters: ${ignoredFilters.join(", ")}`, + `Note: ${label} ignored filters: ${entry.ignoredFilters.join(", ")}`, useColors, ), ); } - const incompatibleFilters = entry.incompatibleFilters.filter( - (name) => !defaultedSet.has(name), - ); - if (incompatibleFilters.length > 0) { + if (entry.incompatibleFilters && entry.incompatibleFilters.length > 0) { lines.push( dim( - `Note: ${label} incompatible filters: ${incompatibleFilters.join( - ", ", - )}`, + `Note: ${label} incompatible filters: ${entry.incompatibleFilters.join(", ")}`, useColors, ), ); @@ -721,6 +661,14 @@ function formatSourceStatusNotes( ), ); } + if (entry.indexingStatus === "INDEXING") { + lines.push( + dim( + `Note: ${label} still indexing — re-run with the searchRef for full results.`, + useColors, + ), + ); + } if (entry.note) { lines.push(dim(`Note: ${label}: ${entry.note}`, useColors)); } diff --git a/src/services/code-navigation-service.test.ts b/src/services/code-navigation-service.test.ts index bd38b9f1..7e3be1b4 100644 --- a/src/services/code-navigation-service.test.ts +++ b/src/services/code-navigation-service.test.ts @@ -43,807 +43,6 @@ describe("CodeNavigationServiceImpl", () => { else process.env.GITHITS_DEBUG = originalDebug; }); - it("sends GraphQL request and normalizes search results", async () => { - const fn = mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [ - { - name: "useMiddleware", - kind: "function", - category: "callable", - filePath: "src/app.js", - startLine: 42, - preview: "function useMiddleware(fn) {", - language: "javascript", - }, - ], - totalMatches: 1, - hasMore: false, - indexedVersion: "4.18.0", - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - const result = await service.searchSymbols({ - target: { registry: "NPM", packageName: "express", version: "4.18.0" }, - query: "middleware", - }); - - expect(result.version).toBe("4.18.0"); - expect(result.totalMatches).toBe(1); - expect(result.results[0]?.name).toBe("useMiddleware"); - - const [url, init] = fn.mock.calls[0] as unknown as [string, RequestInit]; - expect(url).toBe(`${BASE_URL}/api/graphql`); - expect(init.method).toBe("POST"); - expect((init.headers as Record).Authorization).toBe( - "Bearer mock-access-token", - ); - - const body = JSON.parse(init.body as string); - expect(body.variables.registry).toBe("NPM"); - expect(body.variables.packageName).toBe("express"); - expect(body.variables.query).toBe("middleware"); - }); - - it("emits safe debug logging for searchSymbols request shape without query text", async () => { - process.env.GITHITS_DEBUG = "code-nav"; - const stderrSpy = spyOn(process.stderr, "write").mockImplementation( - () => true as never, - ); - const fn = mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: "4.18.0", - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express", version: "4.18.0" }, - query: "middleware secret text", - waitTimeoutMs: 5000, - }); - - const call = stderrSpy.mock.calls[0]?.[0] as string; - const parsed = JSON.parse(call.trimEnd()); - expect(parsed.area).toBe("code-nav"); - expect(parsed.event).toBe("request"); - expect(parsed.operation).toBe("searchSymbols"); - expect(parsed.fileIntent).toBe("omitted"); - expect(parsed.queryMode).toBe("query"); - expect(parsed.presentVariableKeys).not.toContain("fileIntent"); - expect(call).not.toContain("middleware secret text"); - expect(fn).toHaveBeenCalledTimes(1); - stderrSpy.mockRestore(); - }); - - it("emits exact GraphQL and serialized variables for searchSymbols under code-nav-wire", async () => { - process.env.GITHITS_DEBUG = "code-nav-wire"; - const stderrSpy = spyOn(process.stderr, "write").mockImplementation( - () => true as never, - ); - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: "4.18.0", - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express", version: "4.18.0" }, - query: "middleware secret text", - waitTimeoutMs: 5000, - }); - - const call = stderrSpy.mock.calls[0]?.[0] as string; - const parsed = JSON.parse(call.trimEnd()); - expect(parsed.area).toBe("code-nav-wire"); - expect(parsed.event).toBe("wire-request"); - expect(parsed.operation).toBe("searchSymbols"); - expect(parsed.graphqlQuery).toContain("query SearchSymbols("); - expect(parsed.graphqlQuery).toContain("fileIntent: $fileIntent"); - expect(parsed.variables).toEqual({ - registry: "NPM", - packageName: "express", - query: "middleware secret text", - version: "4.18.0", - waitTimeoutMs: 5000, - }); - stderrSpy.mockRestore(); - }); - - it("retries once on 401 with refreshed token", async () => { - const responses = [ - Promise.resolve(new Response("", { status: 401 })), - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: null, - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - ), - ), - ]; - const fn = mock( - () => responses.shift() ?? Promise.reject(new Error("no response")), - ); - globalThis.fetch = fn as unknown as typeof fetch; - - const tokenProvider = createMockTokenProvider({ - forceRefresh: mock(() => Promise.resolve("mock-refreshed-token")), - }); - const service = new CodeNavigationServiceImpl( - BASE_URL, - tokenProvider, - globalThis.fetch, - ); - - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - - expect(tokenProvider.forceRefresh).toHaveBeenCalledTimes(1); - const secondCall = fn.mock.calls[1] as unknown as [string, RequestInit]; - expect( - (secondCall[1].headers as Record).Authorization, - ).toBe("Bearer mock-refreshed-token"); - }); - - it("throws indexing error when backend reports indexing in progress", async () => { - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: null, - diagnostics: { hint: null }, - warning: null, - codeIndexState: "INDEXING", - indexingRef: "idx-123", - availableVersions: [{ version: "4.18.0", ref: "v4.18.0" }], - }, - }, - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(CodeNavigationIndexingError); - }); - - it("throws malformed response error when payload does not match schema", async () => { - mockFetch(() => - Promise.resolve( - new Response(JSON.stringify({ data: { wrongField: {} } }), { - headers: { "Content-Type": "application/json" }, - }), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(MalformedCodeNavigationResponseError); - }); - - it("retries once when GraphQL returns UNAUTHORIZED", async () => { - const responses = [ - Promise.resolve( - new Response( - JSON.stringify({ - errors: [ - { - message: "Unauthorized", - extensions: { code: "UNAUTHORIZED" }, - }, - ], - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: null, - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ]; - const fn = mock( - () => responses.shift() ?? Promise.reject(new Error("no response")), - ); - globalThis.fetch = fn as unknown as typeof fetch; - - const tokenProvider = createMockTokenProvider({ - forceRefresh: mock(() => Promise.resolve("mock-refreshed-token")), - }); - const service = new CodeNavigationServiceImpl( - BASE_URL, - tokenProvider, - globalThis.fetch, - ); - - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - - expect(tokenProvider.forceRefresh).toHaveBeenCalledTimes(1); - }); - - it("throws authentication error when refresh does not produce a new token", async () => { - mockFetch(() => Promise.resolve(new Response("", { status: 401 }))); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider({ - forceRefresh: mock(() => Promise.resolve(undefined)), - }), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(AuthenticationError); - }); - - it("maps null data with GraphQL not-found error to CodeNavigationTargetNotFoundError", async () => { - // Observed live response for unknown packages: { data: null, errors: [...] } - // without `extensions.code`. Schema accepts null data; createGraphQLError - // then matches the message heuristically. - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: - "Package not found in registry: npm/nosuchpackage-zzzzz. Verify the package name and registry are correct.", - path: ["searchSymbols"], - }, - ], - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "nosuchpackage-zzzzz" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(CodeNavigationTargetNotFoundError); - }); - - it("maps NOT_FOUND extensions.code to CodeNavigationTargetNotFoundError", async () => { - // Forward-compat: once backend starts populating extensions.code - // (backend request B8), the structured path is preferred over the - // message heuristic. - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - errors: [ - { - message: "No such target", - extensions: { code: "NOT_FOUND" }, - }, - ], - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "unknown" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(CodeNavigationTargetNotFoundError); - }); - - it("wraps fetch failures in CodeNavigationNetworkError", async () => { - mockFetch(() => - Promise.reject( - Object.assign(new Error("connect ECONNREFUSED"), { - code: "ECONNREFUSED", - }), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }), - ).rejects.toBeInstanceOf(CodeNavigationNetworkError); - }); - - it("maps 5xx responses to CodeNavigationBackendError with status", async () => { - mockFetch(() => - Promise.resolve( - new Response("Internal Server Error", { - status: 502, - headers: { "Content-Type": "text/plain" }, - }), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - try { - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - expect.unreachable(); - } catch (err) { - expect(err).toBeInstanceOf(CodeNavigationBackendError); - expect((err as CodeNavigationBackendError).status).toBe(502); - } - }); - - it("dispatches extensions.code VERSION_NOT_FOUND into a typed error with structured fields", async () => { - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: - 'No version of npm/express matches "4". Available versions: 5.2.1, 5.1.0. Try: express@5.2.1 (exact) or express@^5.0.0 (latest 5.x).', - extensions: { - code: "VERSION_NOT_FOUND", - retryable: false, - package: "npm/express", - requested_version: "4", - latest_indexed: "5.2.1", - available_versions: [ - { version: "5.2.1", ref: "v5.2.1" }, - { version: "5.1.0", ref: "v5.1.0" }, - ], - }, - }, - ], - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - try { - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express", version: "4" }, - query: "middleware", - }); - expect.unreachable(); - } catch (err) { - expect(err).toBeInstanceOf(CodeNavigationVersionNotFoundError); - const typed = err as CodeNavigationVersionNotFoundError; - expect(typed.packageName).toBe("npm/express"); - expect(typed.requestedVersion).toBe("4"); - expect(typed.latestIndexed).toBe("5.2.1"); - expect(typed.availableVersions).toEqual([ - { version: "5.2.1", ref: "v5.2.1" }, - { version: "5.1.0", ref: "v5.1.0" }, - ]); - } - }); - - it("dispatches extensions.code VALIDATION_ERROR to CodeNavigationValidationError", async () => { - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: "Query too long.", - extensions: { code: "VALIDATION_ERROR", retryable: false }, - }, - ], - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "x".repeat(1000), - }), - ).rejects.toBeInstanceOf(CodeNavigationValidationError); - }); - - it("dispatches extensions.code TIMEOUT to CodeNavigationBackendError with graphqlCode=TIMEOUT and retryable=true", async () => { - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: "Backend timed out.", - extensions: { code: "TIMEOUT", retryable: true }, - }, - ], - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - try { - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - expect.unreachable(); - } catch (err) { - expect(err).toBeInstanceOf(CodeNavigationBackendError); - const typed = err as CodeNavigationBackendError; - expect(typed.graphqlCode).toBe("TIMEOUT"); - expect(typed.retryable).toBe(true); - } - }); - - it("ignores legacy message heuristics when extensions.code is present (extensions take precedence)", async () => { - // Backend emits UPSTREAM_ERROR; the message happens to include - // "not found" because the upstream complained. The new dispatch - // must NOT misclassify this as NOT_FOUND. - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: "Upstream registry said: package not found.", - extensions: { code: "UPSTREAM_ERROR", retryable: true }, - }, - ], - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - try { - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - expect.unreachable(); - } catch (err) { - expect(err).not.toBeInstanceOf(CodeNavigationTargetNotFoundError); - expect(err).toBeInstanceOf(CodeNavigationBackendError); - expect((err as CodeNavigationBackendError).graphqlCode).toBe( - "UPSTREAM_ERROR", - ); - } - }); - - it("falls back to message heuristics when extensions.code is absent (legacy backend)", async () => { - // Pre-deployment backend response shape — still must work - // during the rollover window. - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: - "Package not found in registry: npm/nosuchpackage-zzzzz.", - path: ["searchSymbols"], - }, - ], - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { registry: "NPM", packageName: "nosuchpackage-zzzzz" }, - query: "x", - }), - ).rejects.toBeInstanceOf(CodeNavigationTargetNotFoundError); - }); - - it("indexing error message guides callers to retry with a longer wait timeout", async () => { - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: null, - diagnostics: { hint: null }, - warning: null, - codeIndexState: "INDEXING", - indexingRef: "idx-123", - }, - }, - }), - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - try { - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - expect.unreachable(); - } catch (err) { - expect(err).toBeInstanceOf(CodeNavigationIndexingError); - const message = (err as Error).message; - expect(message).toContain("Target is still indexing"); - expect(message).toContain("usually completes within 30 seconds"); - expect(message).toContain("--wait 60000"); - expect(message).toContain("wait_timeout_ms: 60000"); - expect(message).toContain("idx-123"); - } - }); - - it("classifies 'could not resolve' messages as UNRESOLVABLE, not NOT_FOUND", async () => { - // Regression guard on two heuristics: the NOT_FOUND heuristic - // must not swallow resolution-failure phrasing, and the new - // UNRESOLVABLE heuristic must catch it so callers can - // distinguish "target does not exist" from "version/ref cannot - // be resolved". - mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: null, - errors: [ - { - message: - "Could not resolve version 25.6.0 to a Git ref for npm/@types/node.", - path: ["searchSymbols"], - }, - ], - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await expect( - service.searchSymbols({ - target: { - registry: "NPM", - packageName: "@types/node", - version: "25.6.0", - }, - query: "Buffer", - }), - ).rejects.toBeInstanceOf( - // Import it via the module's export so any rename is caught here. - (await import("./code-navigation-service.js")) - .CodeNavigationUnresolvableError, - ); - }); - - it("always sends mode: DETAILED and omits the $verbose variable from the GraphQL request", async () => { - // The service makes this choice once per request so both CLI and - // MCP consumers get the richest response. Confirm the request - // body inlines `mode: DETAILED` and carries no `verbose` variable. - const fn = mockFetch(() => - Promise.resolve( - new Response( - JSON.stringify({ - data: { - searchSymbols: { - results: [], - totalMatches: 0, - hasMore: false, - indexedVersion: null, - diagnostics: { hint: null }, - warning: null, - codeIndexState: "CURRENT", - }, - }, - }), - { headers: { "Content-Type": "application/json" } }, - ), - ), - ); - - const service = new CodeNavigationServiceImpl( - BASE_URL, - createMockTokenProvider(), - globalThis.fetch, - ); - - await service.searchSymbols({ - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - }); - - const [, init] = fn.mock.calls[0] as unknown as [string, RequestInit]; - const body = JSON.parse(init.body as string); - expect(Object.keys(body.variables)).not.toContain("verbose"); - expect(Object.keys(body.variables)).not.toContain("mode"); - // mode is inlined as a literal in the query body - expect(body.query).toContain("mode: DETAILED"); - expect(body.query).not.toContain("@include(if: $verbose)"); - }); - // ------------------------------------------------------------------ // listFiles // ------------------------------------------------------------------ diff --git a/src/services/code-navigation-service.ts b/src/services/code-navigation-service.ts index 88bd2917..90aada68 100644 --- a/src/services/code-navigation-service.ts +++ b/src/services/code-navigation-service.ts @@ -17,8 +17,6 @@ import type { TokenProvider } from "./token-manager.js"; */ export type CodeNavigationRegistry = PkgseerRegistry; -export type SearchSymbolsMatchMode = "OR" | "AND"; - /** * Precise symbol kind from the backend's unified symbol taxonomy. * Prefer `SymbolCategory` for broad filtering; use `SymbolKind` @@ -31,7 +29,7 @@ export type SearchSymbolsMatchMode = "OR" | "AND"; * - `class` excludes record/mixin/actor * - `interface` excludes protocol/annotation */ -export type SearchSymbolsKind = +export type SymbolKind = | "FUNCTION" | "METHOD" | "CONSTRUCTOR" @@ -73,7 +71,7 @@ export type SymbolCategory = | "DATA" | "DOCUMENTATION"; -export type SearchSymbolsFileIntent = +export type FileIntent = | "PRODUCTION" | "TEST" | "BENCHMARK" @@ -91,80 +89,13 @@ export interface CodeNavigationTarget { gitRef?: string; } -export interface SearchSymbolsParams { - target: CodeNavigationTarget; - query?: string; - keywords?: string[]; - matchMode?: SearchSymbolsMatchMode; - /** - * Precise symbol kind. Prefer `category` for broad filtering; - * use `kind` only when the caller wants a specific construct. - */ - kind?: SearchSymbolsKind; - /** - * Broad symbol category filter. Preferred surface for filtering - * navigation queries — works across the 27-value kind taxonomy - * without enumerating individual kinds. - */ - category?: SymbolCategory; - filePath?: string; - limit?: number; - fileIntent?: SearchSymbolsFileIntent; - waitTimeoutMs?: number; -} - -export interface SearchSymbolsResultEntry { - name?: string; - filePath?: string; - startLine?: number; - endLine?: number; - preview?: string; - code?: string; - language?: string; - symbolRef?: string; - qualifiedPath?: string; - /** - * Precise symbol kind (lowercase string) from the backend's - * unified symbol taxonomy. Populated for every chunk — the - * backend handles the fallback from chunk-level classification - * to enrichment-level kind internally, so callers can treat - * this as the single source of truth for taxonomy. - */ - kind?: string; - /** - * Broad category of the primary symbol — one of `callable`, - * `type`, `module`, `data`, `documentation`. Computed by the - * backend from `kind`. Null for kinds with no category - * (e.g. CSS rules). - */ - category?: string; - arity?: number; - isPublic?: boolean; - /** - * Number of symbols contained in this chunk (DETAILED mode only, - * populated when > 1). Schema coerced to `Int` in the April 2026 - * backend update; earlier versions returned a string array. - */ - containedSymbols?: number; -} - -export interface SearchSymbolsResolution { +export interface IndexResolution { requestedVersion?: string; requestedRef?: string; resolvedRef?: string; commitSha?: string; } -export interface SearchSymbolsResult { - results: SearchSymbolsResultEntry[]; - totalMatches: number; - hasMore: boolean; - version?: string; - resolution?: SearchSymbolsResolution; - hint?: string; - warning?: string; -} - export type UnifiedSearchSource = "AUTO" | "DOCS" | "CODE" | "SYMBOL"; export type UnifiedSearchResultType = @@ -182,8 +113,8 @@ export type UnifiedSearchSessionStatus = | "FAILED"; export interface UnifiedSearchFilters { - fileIntent?: SearchSymbolsFileIntent; - kind?: SearchSymbolsKind; + fileIntent?: FileIntent; + kind?: SymbolKind; category?: SymbolCategory; publicOnly?: boolean; pathPrefix?: string; @@ -327,7 +258,7 @@ export interface ListFilesResult { total: number; hasMore: boolean; indexedVersion?: string; - resolution?: SearchSymbolsResolution; + resolution?: IndexResolution; hint?: string; } @@ -433,13 +364,12 @@ export interface GrepRepoResult { totalMatches: number; uniqueFilesMatched: number; indexedVersion?: string; - resolution?: SearchSymbolsResolution; + resolution?: IndexResolution; } export interface CodeNavigationService { search(params: UnifiedSearchParams): Promise; searchStatus(searchRef: string): Promise; - searchSymbols(params: SearchSymbolsParams): Promise; listFiles(params: ListFilesParams): Promise; readFile(params: ReadFileParams): Promise; grepRepo(params: GrepRepoParams): Promise; @@ -583,95 +513,6 @@ export class CodeNavigationBackendError extends Error { } } -/** - * Raised by the service when the caller supplied neither a query - * nor any keywords. Name starts with `Invalid` so - * `mapCodeNavigationError` classifies it as `INVALID_ARGUMENT`. - */ -export class InvalidSearchSymbolsRequestError extends Error { - constructor(message: string) { - super(message); - this.name = "InvalidSearchSymbolsRequestError"; - } -} - -// Always requests `mode: DETAILED` — the service makes this choice -// once so both CLI and MCP get the richest response (kind, category, -// endLine, language). Consumers derive a snippet from `code` when -// needed; `preview` is available in both modes but the formatter -// owns snippet rendering client-side for consistent truncation. -const SEARCH_SYMBOLS_QUERY = ` -query SearchSymbols( - $registry: Registry - $packageName: String - $repoUrl: String - $gitRef: String - $query: String - $keywords: [String!] - $matchMode: MatchMode - $kind: SymbolKind - $category: SymbolCategory - $filePath: String - $version: String - $limit: Int - $fileIntent: FileIntent - $waitTimeoutMs: Int -) { - searchSymbols( - registry: $registry - packageName: $packageName - repoUrl: $repoUrl - gitRef: $gitRef - query: $query - keywords: $keywords - matchMode: $matchMode - kind: $kind - category: $category - filePath: $filePath - version: $version - limit: $limit - fileIntent: $fileIntent - mode: DETAILED - waitTimeoutMs: $waitTimeoutMs - ) { - results { - name - filePath - startLine - endLine - preview - code - language - symbolRef - qualifiedPath - kind - category - arity - isPublic - containedSymbols - } - totalMatches - hasMore - indexedVersion - resolution { - requestedVersion - requestedRef - resolvedRef - commitSha - } - diagnostics { - hint - } - warning - codeIndexState - indexingRef - availableVersions { - version - ref - } - } -}`; - const UNIFIED_SEARCH_QUERY = ` query UnifiedSearch( $targets: [SearchPackageInput!]! @@ -879,7 +720,7 @@ function debugUnifiedSearchRequest(variables: { } function debugGraphqlWireRequest( - operation: "search" | "searchSymbols", + operation: "search", graphqlQuery: string, variables: Record, ): void { @@ -892,55 +733,6 @@ function debugGraphqlWireRequest( }); } -function debugSearchSymbolsRequest(variables: { - registry?: string; - packageName?: string; - repoUrl?: string; - gitRef?: string; - query?: string; - keywords?: string[]; - matchMode?: SearchSymbolsMatchMode; - kind?: SearchSymbolsKind; - category?: SymbolCategory; - filePath?: string; - version?: string; - limit?: number; - fileIntent?: SearchSymbolsFileIntent; - waitTimeoutMs?: number; -}): void { - if (!isDebugAreaEnabled("code-nav")) return; - const serialised = serialiseForDebug(variables); - - debugLog("code-nav", { - event: "request", - operation: "searchSymbols", - targetType: typeof serialised.repoUrl === "string" ? "repo" : "package", - queryMode: Array.isArray(serialised.keywords) - ? typeof serialised.query === "string" - ? "query+keywords" - : "keywords" - : typeof serialised.query === "string" - ? "query" - : "none", - keywordsCount: Array.isArray(serialised.keywords) - ? serialised.keywords.length - : 0, - kind: typeof serialised.kind === "string" ? serialised.kind : undefined, - category: - typeof serialised.category === "string" ? serialised.category : undefined, - fileIntent: - typeof serialised.fileIntent === "string" - ? serialised.fileIntent - : "omitted", - presentVariableKeys: Object.keys(serialised).sort(), - hasLimit: typeof serialised.limit === "number", - waitTimeoutMs: - typeof serialised.waitTimeoutMs === "number" - ? serialised.waitTimeoutMs - : undefined, - }); -} - function serialiseForDebug( value: Record, ): Record { @@ -966,49 +758,6 @@ const availableVersionSchema = z.object({ ref: z.string(), }); -const searchSymbolsResultEntrySchema = z.object({ - name: z.string().nullable().optional(), - filePath: z.string().nullable().optional(), - startLine: z.number().int().nullable().optional(), - endLine: z.number().int().nullable().optional(), - preview: z.string().nullable().optional(), - code: z.string().nullable().optional(), - language: z.string().nullable().optional(), - symbolRef: z.string().nullable().optional(), - qualifiedPath: z.string().nullable().optional(), - kind: z.string().nullable().optional(), - category: z.string().nullable().optional(), - arity: z.number().int().nullable().optional(), - isPublic: z.boolean().nullable().optional(), - containedSymbols: z.number().int().nullable().optional(), -}); - -const searchSymbolsResponseSchema = z.object({ - results: z.array(searchSymbolsResultEntrySchema), - totalMatches: z.number().int(), - hasMore: z.boolean(), - indexedVersion: z.string().nullable().optional(), - resolution: z - .object({ - requestedVersion: z.string().nullable().optional(), - requestedRef: z.string().nullable().optional(), - resolvedRef: z.string().nullable().optional(), - commitSha: z.string().nullable().optional(), - }) - .nullable() - .optional(), - diagnostics: z - .object({ - hint: z.string().nullable().optional(), - }) - .nullable() - .optional(), - warning: z.string().nullable().optional(), - codeIndexState: z.string(), - indexingRef: z.string().nullable().optional(), - availableVersions: z.array(availableVersionSchema).nullable().optional(), -}); - const unifiedSearchSourceSchema = z.enum(["AUTO", "DOCS", "CODE", "SYMBOL"]); const unifiedSearchResultTypeSchema = z.enum([ @@ -1483,19 +1232,6 @@ query GrepRepo( }`; } -// `data` may be null (seen live for unknown packages that also carry -// `errors`), and `searchSymbols` may be null even when `data` is present. -// Both must parse successfully so the error-handling layer can classify. -const graphQLResponseSchema = z.object({ - data: z - .object({ - searchSymbols: searchSymbolsResponseSchema.nullable().optional(), - }) - .nullable() - .optional(), - errors: z.array(graphQLErrorSchema).optional(), -}); - const unifiedSearchGraphQLResponseSchema = z.object({ data: z .object({ @@ -1544,17 +1280,6 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { }); } - async searchSymbols( - params: SearchSymbolsParams, - ): Promise { - return executeWithTokenRefresh({ - getToken: () => this.tokenProvider.getToken(), - forceRefresh: () => this.tokenProvider.forceRefresh(), - shouldRefresh: (error) => error instanceof AuthenticationError, - executeWithToken: (token) => this.executeSearchSymbols(token, params), - }); - } - private async executeUnifiedSearch( token: string, params: UnifiedSearchParams, @@ -1703,130 +1428,6 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { }; } - private async executeSearchSymbols( - token: string, - params: SearchSymbolsParams, - ): Promise { - if (!params.query && (!params.keywords || params.keywords.length === 0)) { - throw new InvalidSearchSymbolsRequestError( - "Either query or keywords must be provided.", - ); - } - - let response: PkgseerGraphqlResponse; - const variables = { - registry: params.target.registry, - packageName: params.target.packageName, - repoUrl: params.target.repoUrl, - gitRef: params.target.gitRef, - query: params.query, - keywords: params.keywords, - matchMode: params.matchMode, - kind: params.kind, - category: params.category, - filePath: params.filePath, - version: params.target.version, - limit: params.limit, - fileIntent: params.fileIntent, - waitTimeoutMs: params.waitTimeoutMs, - }; - debugSearchSymbolsRequest(variables); - debugGraphqlWireRequest("searchSymbols", SEARCH_SYMBOLS_QUERY, variables); - try { - response = await postPkgseerGraphql({ - endpointUrl: this.codeNavigationUrl, - token, - query: SEARCH_SYMBOLS_QUERY, - variables, - fetchFn: this.fetchFn, - }); - } catch (cause) { - // Helper owns transport-level error wrapping; re-map to the - // domain error class so `mapCodeNavigationError` still routes - // to `NETWORK`. Other error classes bubble unchanged. - if (cause instanceof PkgseerTransportError) { - throw new CodeNavigationNetworkError( - "Could not reach the code navigation service. Check your connection or set GITHITS_CODE_NAV_URL.", - { cause }, - ); - } - throw cause; - } - - if (response.status < 200 || response.status >= 300) { - throw this.createHttpError(response); - } - - const parsed = graphQLResponseSchema.safeParse(response.parsedBody); - if (!parsed.success) { - throw new MalformedCodeNavigationResponseError( - "Malformed response from code navigation service.", - ); - } - - if (parsed.data.errors && parsed.data.errors.length > 0) { - throw this.createGraphQLError(parsed.data.errors); - } - - const data = parsed.data.data?.searchSymbols; - if (!data) { - // `data: null` or `data.searchSymbols: null` with no `errors` entry. - // Rare — the backend normally couples null payloads with errors. - throw new MalformedCodeNavigationResponseError( - "Malformed response from code navigation service.", - ); - } - - if (data.codeIndexState === "INDEXING") { - throw new CodeNavigationIndexingError( - this.createIndexingMessage(data.indexingRef ?? undefined), - data.indexingRef ?? undefined, - data.availableVersions?.map((entry) => ({ - version: entry.version ?? undefined, - ref: entry.ref, - })), - ); - } - - if (data.codeIndexState === "UNRESOLVABLE") { - throw new CodeNavigationUnresolvableError( - "The requested target or version could not be resolved.", - ); - } - - return { - results: data.results.map((entry) => ({ - name: entry.name ?? undefined, - filePath: entry.filePath ?? undefined, - startLine: entry.startLine ?? undefined, - endLine: entry.endLine ?? undefined, - preview: entry.preview ?? undefined, - code: entry.code ?? undefined, - language: entry.language ?? undefined, - symbolRef: entry.symbolRef ?? undefined, - qualifiedPath: entry.qualifiedPath ?? undefined, - kind: entry.kind ?? undefined, - category: entry.category ?? undefined, - arity: entry.arity ?? undefined, - isPublic: entry.isPublic ?? undefined, - containedSymbols: entry.containedSymbols ?? undefined, - })), - totalMatches: data.totalMatches, - hasMore: data.hasMore, - version: data.indexedVersion ?? undefined, - resolution: data.resolution - ? { - requestedVersion: data.resolution.requestedVersion ?? undefined, - requestedRef: data.resolution.requestedRef ?? undefined, - resolvedRef: data.resolution.resolvedRef ?? undefined, - commitSha: data.resolution.commitSha ?? undefined, - } - : undefined, - hint: data.diagnostics?.hint ?? undefined, - warning: data.warning ?? undefined, - }; - } - private createHttpError(response: PkgseerGraphqlResponse): Error { const status = response.status; const detail = parseDetail(response.responseBody); @@ -2129,8 +1730,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { * Shared sentinel-promotion for the file-exploration tools. When the backend * response carries `codeIndexState: "INDEXING"` (data-path variant), * throw the typed error so the envelope builder / caller never sees - * the raw sentinel. Mirrors the inline check `searchSymbols` does - * today. + * the raw sentinel. */ private throwIfIndexing(data: { codeIndexState: string; diff --git a/src/services/index.ts b/src/services/index.ts index 1cb2fcf3..3939e23c 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -29,6 +29,7 @@ export type { CodeNavigationRegistry, CodeNavigationService, CodeNavigationTarget, + FileIntent, GrepPathSelectorKind, GrepRepoMatch, GrepRepoParams, @@ -37,20 +38,15 @@ export type { GrepRepoResult, GrepRouteTaken, GrepTruncatedReason, + IndexResolution, ListFilesParams, ListFilesResult, NavigationSymbol, ReadFileParams, ReadFileResult, RepoFileEntry, - SearchSymbolsFileIntent, - SearchSymbolsKind, - SearchSymbolsMatchMode, - SearchSymbolsParams, - SearchSymbolsResolution, - SearchSymbolsResult, - SearchSymbolsResultEntry, SymbolCategory, + SymbolKind, UnifiedSearchCompleted, UnifiedSearchFilters, UnifiedSearchHit, @@ -79,7 +75,6 @@ export { CodeNavigationUnresolvableError, CodeNavigationValidationError, CodeNavigationVersionNotFoundError, - InvalidSearchSymbolsRequestError, MalformedCodeNavigationResponseError, } from "./code-navigation-service.js"; export { diff --git a/src/services/test-helpers.ts b/src/services/test-helpers.ts index 704f0deb..7196652b 100644 --- a/src/services/test-helpers.ts +++ b/src/services/test-helpers.ts @@ -15,7 +15,6 @@ import type { BrowserService } from "./browser-service.js"; import type { CodeNavigationService, GrepRepoResult, - SearchSymbolsResult, UnifiedSearchOutcome, } from "./code-navigation-service.js"; import type { ExecResult, ExecService } from "./exec-service.js"; @@ -103,30 +102,6 @@ export function createMockAuthService( }; } -/** - * Default code navigation search result for testing. Matches the - * DETAILED-mode response shape the service now always requests: - * `kind`, `category`, `endLine`, `language`, and `code` are - * populated; `preview` is null (callers build snippets from `code`). - */ -export const defaultSearchSymbolsResult: SearchSymbolsResult = { - results: [ - { - name: "useMiddleware", - kind: "function", - category: "callable", - filePath: "src/app.js", - startLine: 42, - endLine: 48, - code: "function useMiddleware(fn) {\n fn();\n return null;\n}", - language: "javascript", - }, - ], - totalMatches: 1, - hasMore: false, - version: "4.18.0", -}; - export const defaultUnifiedSearchOutcome: UnifiedSearchOutcome = { state: "completed", completed: true, @@ -379,7 +354,6 @@ export function createMockCodeNavigationService( return { search: mock(() => Promise.resolve(defaultUnifiedSearchOutcome)), searchStatus: mock(() => Promise.resolve(defaultUnifiedSearchOutcome)), - searchSymbols: mock(() => Promise.resolve(defaultSearchSymbolsResult)), listFiles: mock(() => Promise.resolve(defaultListFilesResult)), readFile: mock(() => Promise.resolve(defaultReadFileResult)), grepRepo: mock(() => Promise.resolve(defaultGrepRepoResult)), diff --git a/src/shared/code-navigation-defaults.ts b/src/shared/code-navigation-defaults.ts index 748d5d5b..b23575c7 100644 --- a/src/shared/code-navigation-defaults.ts +++ b/src/shared/code-navigation-defaults.ts @@ -34,6 +34,6 @@ export const FILE_INTENT_ALL = Symbol("FILE_INTENT_ALL"); * `FILE_INTENT_ALL` sentinel, or `undefined` (no filter requested). */ export type FileIntentInput = - | import("../services/index.js").SearchSymbolsFileIntent + | import("../services/index.js").FileIntent | typeof FILE_INTENT_ALL | undefined; diff --git a/src/shared/code-navigation-error-map.test.ts b/src/shared/code-navigation-error-map.test.ts index ed442444..0be71f3d 100644 --- a/src/shared/code-navigation-error-map.test.ts +++ b/src/shared/code-navigation-error-map.test.ts @@ -11,7 +11,6 @@ import { CodeNavigationUnresolvableError, CodeNavigationValidationError, CodeNavigationVersionNotFoundError, - InvalidSearchSymbolsRequestError, MalformedCodeNavigationResponseError, } from "../services/code-navigation-service.js"; import { AuthenticationError } from "../services/githits-service.js"; @@ -264,17 +263,6 @@ describe("mapCodeNavigationError", () => { }); }); - it("classifies InvalidSearchSymbolsRequestError as INVALID_ARGUMENT", () => { - const err = new InvalidSearchSymbolsRequestError( - "Either query or keywords must be provided.", - ); - expect(mapCodeNavigationError(err)).toEqual({ - code: "INVALID_ARGUMENT", - message: "Either query or keywords must be provided.", - retryable: false, - }); - }); - it("falls through to UNKNOWN for plain Error", () => { const err = new Error("mystery"); expect(mapCodeNavigationError(err)).toEqual({ diff --git a/src/shared/code-navigation.ts b/src/shared/code-navigation.ts index 47492364..7a4bf694 100644 --- a/src/shared/code-navigation.ts +++ b/src/shared/code-navigation.ts @@ -1,8 +1,7 @@ import type { - SearchSymbolsFileIntent, - SearchSymbolsKind, - SearchSymbolsMatchMode, + FileIntent, SymbolCategory, + SymbolKind, } from "../services/index.js"; import { type PkgseerRegistryArg, @@ -44,7 +43,7 @@ const symbolKindMap = { event: "EVENT", constant: "CONSTANT", doc_section: "DOC_SECTION", -} as const satisfies Record; +} as const satisfies Record; /** * Lowercase user-facing category values → the backend's uppercase @@ -67,12 +66,7 @@ const fileIntentMap = { fixture: "FIXTURE", build: "BUILD", vendor: "VENDOR", -} as const satisfies Record; - -const matchModeMap = { - or: "OR", - and: "AND", -} as const satisfies Record; +} as const satisfies Record; /** * Back-compat alias for {@link PkgseerRegistryArg}. The registry map @@ -88,15 +82,7 @@ export function toCodeNavigationRegistry(registry: CodeNavigationRegistryArg) { return toPkgseerRegistry(registry); } -export function toSearchSymbolsMatchMode( - mode: string | undefined, -): SearchSymbolsMatchMode | undefined { - return mode ? matchModeMap[mode as keyof typeof matchModeMap] : undefined; -} - -export function toSearchSymbolsKind( - kind: string | undefined, -): SearchSymbolsKind | undefined { +export function toSymbolKind(kind: string | undefined): SymbolKind | undefined { return kind ? symbolKindMap[kind as keyof typeof symbolKindMap] : undefined; } @@ -122,9 +108,9 @@ export function knownSymbolCategoryList(): ReadonlyArray { return Object.keys(symbolCategoryMap); } -export function toSearchSymbolsFileIntent( +export function toFileIntent( intent: string | undefined, -): SearchSymbolsFileIntent | undefined { +): FileIntent | undefined { return intent ? fileIntentMap[intent as keyof typeof fileIntentMap] : undefined; diff --git a/src/shared/docs-follow-up.ts b/src/shared/docs-follow-up.ts index df38112d..254a2c24 100644 --- a/src/shared/docs-follow-up.ts +++ b/src/shared/docs-follow-up.ts @@ -1,20 +1,4 @@ -import type { - PackageDocPage, - PackageDocPageSummary, - PackageDocSourceKind, -} from "../services/index.js"; - -export type DocReadFollowUp = { - type: "read_doc"; - pageId: string; -}; - -export type FileReadFollowUp = { - type: "read_file"; - repoUrl: string; - gitRef: string; - path: string; -}; +import type { PackageDocSourceKind } from "../services/index.js"; export function lowerDocSourceKind( value: PackageDocSourceKind | undefined, @@ -28,27 +12,3 @@ export function lowerDocSourceKind( return undefined; } } - -export function buildDocReadFollowUp( - pageId: string | undefined, -): DocReadFollowUp | undefined { - if (!pageId) return undefined; - return { type: "read_doc", pageId }; -} - -export function buildFileReadFollowUp( - entry: - | Pick - | Pick, -): FileReadFollowUp | undefined { - if (!entry.repoUrl || !entry.gitRef || !entry.filePath) { - return undefined; - } - - return { - type: "read_file", - repoUrl: entry.repoUrl, - gitRef: entry.gitRef, - path: entry.filePath, - }; -} diff --git a/src/shared/extract-solution-id.ts b/src/shared/extract-solution-id.ts new file mode 100644 index 00000000..d4ce770a --- /dev/null +++ b/src/shared/extract-solution-id.ts @@ -0,0 +1,18 @@ +/** + * GitHits' /search REST endpoint returns a markdown body whose footer + * links to the canonical solution page on app.githits.com. The + * `solution_id` needed for `feedback` is the UUID at the end of that + * URL. Until the backend exposes it as a structured field, surfaces + * pluck it out client-side so agents and JSON consumers can call + * `feedback` without parsing markdown. + * + * TODO(backend): have /search return `{ markdown, solution_id }` as + * a structured payload so this regex can go away. + */ + +const SOLUTION_URL_RE = /solutions\/([0-9a-fA-F-]{36})/; + +export function extractSolutionId(markdown: string): string | undefined { + const match = SOLUTION_URL_RE.exec(markdown); + return match?.[1]; +} diff --git a/src/shared/grep-repo-response.test.ts b/src/shared/grep-repo-response.test.ts index 3e08c66f..8262d8bc 100644 --- a/src/shared/grep-repo-response.test.ts +++ b/src/shared/grep-repo-response.test.ts @@ -85,14 +85,14 @@ describe("buildGrepRepoSuccessPayload", () => { it("projects the new envelope shape", () => { const envelope = buildGrepRepoSuccessPayload(baseResult, baseOptions); expect(envelope.pattern).toBe("express()"); - expect(envelope.patternType).toBe("literal"); + // patternType omitted when default ("literal") + expect(envelope.patternType).toBeUndefined(); expect(envelope.totalMatches).toBe(1); expect(envelope.matches[0]?.filePath).toBe("src/index.js"); expect(envelope.matches[0]?.symbol).toMatchObject({ name: "createRouter", qualifiedPath: "express.createRouter", }); - expect(envelope.routeTaken).toBe("content_index"); expect(envelope.filter).toEqual({ pathPrefix: "src/", globs: ["src/**/*.js"], diff --git a/src/shared/grep-repo-response.ts b/src/shared/grep-repo-response.ts index e5e8f57d..80e6cf5c 100644 --- a/src/shared/grep-repo-response.ts +++ b/src/shared/grep-repo-response.ts @@ -11,7 +11,6 @@ export interface LeanGrepRepoMatch { contextAfter?: string[]; fileContentHash?: string; fileIntent?: string; - symbolRowId?: string; symbol?: { symbolRef?: string; name?: string; @@ -62,17 +61,16 @@ export interface LeanGrepRepoEnvelope { repoUrl?: string; gitRef?: string; pattern: string; - patternType: "literal" | "regex"; - caseSensitive: boolean; + patternType?: "literal" | "regex"; + caseSensitive?: boolean; matches: LeanGrepRepoMatch[]; nextCursor?: string; hasMore: boolean; - truncatedReason: string; - routeTaken?: string; + truncatedReason?: string; filesScanned: number; filesInScope: number; - binaryFilesSkipped: number; - filesTooLargeSkipped: number; + binaryFilesSkipped?: number; + filesTooLargeSkipped?: number; totalMatches: number; uniqueFilesMatched: number; indexedVersion?: string; @@ -126,20 +124,28 @@ export function buildGrepRepoSuccessPayload( ): LeanGrepRepoEnvelope { const envelope: LeanGrepRepoEnvelope = { pattern: options.pattern, - patternType: options.patternType, - caseSensitive: options.caseSensitive, matches: result.matches.map(projectMatch), hasMore: result.hasMore, - truncatedReason: result.truncatedReason.toLowerCase(), - routeTaken: result.routeTaken?.toLowerCase(), filesScanned: result.filesScanned, filesInScope: result.filesInScope, - binaryFilesSkipped: result.binaryFilesSkipped, - filesTooLargeSkipped: result.filesTooLargeSkipped, totalMatches: result.totalMatches, uniqueFilesMatched: result.uniqueFilesMatched, }; + if (options.patternType !== "literal") { + envelope.patternType = options.patternType; + } + if (options.caseSensitive) envelope.caseSensitive = true; + if (result.binaryFilesSkipped > 0) { + envelope.binaryFilesSkipped = result.binaryFilesSkipped; + } + if (result.filesTooLargeSkipped > 0) { + envelope.filesTooLargeSkipped = result.filesTooLargeSkipped; + } + if (result.truncatedReason && result.truncatedReason !== "NONE") { + envelope.truncatedReason = result.truncatedReason.toLowerCase(); + } + if (options.registry) envelope.registry = options.registry; if (options.name) envelope.name = options.name; if (options.repoUrl) envelope.repoUrl = options.repoUrl; @@ -156,19 +162,23 @@ export function buildGrepRepoSuccessPayload( } function projectMatch(match: GrepRepoMatch): LeanGrepRepoMatch { - return { + const projected: LeanGrepRepoMatch = { filePath: match.filePath, line: match.line, matchStartByte: match.matchStartByte, matchEndByte: match.matchEndByte, lineContent: match.lineContent, - contextBefore: match.contextBefore, - contextAfter: match.contextAfter, - fileContentHash: match.fileContentHash, - fileIntent: match.fileIntent, - symbolRowId: match.symbolRowId, - symbol: match.symbol, }; + if (match.contextBefore && match.contextBefore.length > 0) { + projected.contextBefore = match.contextBefore; + } + if (match.contextAfter && match.contextAfter.length > 0) { + projected.contextAfter = match.contextAfter; + } + if (match.fileContentHash) projected.fileContentHash = match.fileContentHash; + if (match.fileIntent) projected.fileIntent = match.fileIntent; + if (match.symbol) projected.symbol = match.symbol; + return projected; } function projectResolution( diff --git a/src/shared/index.ts b/src/shared/index.ts index f0877080..a4327a4d 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -5,10 +5,9 @@ export { knownSymbolKindList, type SymbolCategoryArg, toCodeNavigationRegistry, - toSearchSymbolsFileIntent, - toSearchSymbolsKind, - toSearchSymbolsMatchMode, + toFileIntent, toSymbolCategory, + toSymbolKind, } from "./code-navigation.js"; export { DEFAULT_WAIT_TIMEOUT_MS, @@ -33,13 +32,8 @@ export { warning, } from "./colors.js"; export { debugLog } from "./debug-log.js"; -export { - buildDocReadFollowUp, - buildFileReadFollowUp, - type DocReadFollowUp, - type FileReadFollowUp, - lowerDocSourceKind, -} from "./docs-follow-up.js"; +export { lowerDocSourceKind } from "./docs-follow-up.js"; +export { extractSolutionId } from "./extract-solution-id.js"; export { buildGrepRepoParams, GREP_REPO_PATTERN_NOTE, @@ -140,6 +134,7 @@ export { type VulnSeverityLabel, vulnSeverityLabel, } from "./package-vulnerabilities-response.js"; +export { type LineRange, parseLinesOption } from "./parse-lines-option.js"; export { isKnownPkgseerRegistryArg, knownPkgseerRegistryArgs, @@ -159,18 +154,6 @@ export { type LeanPackageDocEnvelope, } from "./read-package-doc-response.js"; export { AuthRequiredError, requireAuth } from "./require-auth.js"; -export { - buildSearchSymbolsParams, - type SearchSymbolsRequestBuildResult, - type SearchSymbolsRequestInput, -} from "./search-symbols-request.js"; -export { - buildSearchSymbolsErrorPayload, - buildSearchSymbolsSuccessPayload, - type SearchSymbolsErrorPayload, - type SearchSymbolsQueryEcho, - type SearchSymbolsSuccessPayload, -} from "./search-symbols-response.js"; export { endTelemetrySpan, flushTelemetry, diff --git a/src/shared/list-package-docs-response.ts b/src/shared/list-package-docs-response.ts index 82d2382f..3e2eb6bd 100644 --- a/src/shared/list-package-docs-response.ts +++ b/src/shared/list-package-docs-response.ts @@ -1,13 +1,7 @@ import type { PackageDocsList } from "../services/index.js"; import { MalformedPackageIntelligenceResponseError } from "../services/index.js"; import { colorize, dim } from "./colors.js"; -import { - buildDocReadFollowUp, - buildFileReadFollowUp, - type DocReadFollowUp, - type FileReadFollowUp, - lowerDocSourceKind, -} from "./docs-follow-up.js"; +import { lowerDocSourceKind } from "./docs-follow-up.js"; import { toIsoDate } from "./format-date.js"; export interface LeanPackageDocListEntry { @@ -21,8 +15,6 @@ export interface LeanPackageDocListEntry { filePath?: string; linkName?: string; lastUpdatedAt?: string; - followUp: DocReadFollowUp; - readFile?: FileReadFollowUp; } export interface LeanPackageDocListFilter { @@ -59,20 +51,18 @@ export function buildListPackageDocsSuccessPayload( assertDocListEntry(page); const pageId = page.id as string; const lastUpdatedAt = toIsoDate(page.lastUpdatedAt); - return { - pageId, - title: page.title ?? undefined, - sourceKind: lowerDocSourceKind(page.sourceKind), - sourceUrl: page.sourceUrl ?? undefined, - repoUrl: page.repoUrl ?? undefined, - gitRef: page.gitRef ?? undefined, - requestedRef: page.requestedRef ?? undefined, - filePath: page.filePath ?? undefined, - linkName: page.linkName ?? undefined, - lastUpdatedAt: lastUpdatedAt ?? undefined, - followUp: buildDocReadFollowUp(pageId) as DocReadFollowUp, - readFile: buildFileReadFollowUp(page), - }; + const entry: LeanPackageDocListEntry = { pageId }; + if (page.title) entry.title = page.title; + const sourceKind = lowerDocSourceKind(page.sourceKind); + if (sourceKind) entry.sourceKind = sourceKind; + if (page.sourceUrl) entry.sourceUrl = page.sourceUrl; + if (page.repoUrl) entry.repoUrl = page.repoUrl; + if (page.gitRef) entry.gitRef = page.gitRef; + if (page.requestedRef) entry.requestedRef = page.requestedRef; + if (page.filePath) entry.filePath = page.filePath; + if (page.linkName) entry.linkName = page.linkName; + if (lastUpdatedAt) entry.lastUpdatedAt = lastUpdatedAt; + return entry; }), }; diff --git a/src/shared/package-summary-request.ts b/src/shared/package-summary-request.ts index 057a0de3..06a519aa 100644 --- a/src/shared/package-summary-request.ts +++ b/src/shared/package-summary-request.ts @@ -11,8 +11,8 @@ * enum via `toPkgseerRegistry`, rejecting unknown registries with * `UnsupportedRegistryError`. * - * Unlike `buildSearchSymbolsParams` this builder has no defaults to - * apply, so it returns only `{ params }` — no `defaulted` array. + * This builder has no defaults to apply, so it returns only + * `{ params }` — no `defaulted` array. */ import type { PackageSummaryParams } from "../services/index.js"; diff --git a/src/shared/package-vulnerabilities-response.test.ts b/src/shared/package-vulnerabilities-response.test.ts index 35b61deb..7a067349 100644 --- a/src/shared/package-vulnerabilities-response.test.ts +++ b/src/shared/package-vulnerabilities-response.test.ts @@ -251,12 +251,10 @@ describe("buildPackageVulnerabilitiesSuccessPayload — requestedVersion echo", }); it("surfaces requestedVersion on any non-empty divergence, including v-prefix forms", () => { - // Unlike `search_symbols` (which normalises `v`-prefixed refs - // via `isTrivialRefDifference`), the vulnerabilities query takes - // a version string, not a git ref. `v4.18.0` is a tag - // convention; no registry accepts it as a canonical version, so - // surfacing the divergence here points at a real caller mistake - // rather than masking it. + // The vulnerabilities query takes a version string, not a git + // ref. `v4.18.0` is a tag convention; no registry accepts it as + // a canonical version, so surfacing the divergence here points + // at a real caller mistake rather than masking it. const payload = buildPackageVulnerabilitiesSuccessPayload( defaultVulnerabilityReport, { requestedVersion: "v4.18.0" }, diff --git a/src/shared/package-vulnerabilities-response.ts b/src/shared/package-vulnerabilities-response.ts index adc45d1f..47defc6d 100644 --- a/src/shared/package-vulnerabilities-response.ts +++ b/src/shared/package-vulnerabilities-response.ts @@ -21,13 +21,12 @@ * legitimately exceed the returned list in paginated futures. * - `requestedVersion` surfaces whenever the backend-resolved * `version` differs from the caller's (trimmed) input. `v`-prefix - * normalisation that `search_symbols` does via - * `isTrivialRefDifference` is intentionally *not* applied here: - * the `v4.17.0` form is a git-tag convention, not a version-string - * convention — no supported registry (npm, PyPI, Hex, Crates) - * accepts it as a canonical version, so the backend will reject - * it rather than resolve it to `4.17.0`. Masking that error - * would hide a real caller mistake. + * normalisation is intentionally *not* applied here: the `v4.17.0` + * form is a git-tag convention, not a version-string convention — + * no supported registry (npm, PyPI, Hex, Crates) accepts it as a + * canonical version, so the backend will reject it rather than + * resolve it to `4.17.0`. Masking that error would hide a real + * caller mistake. * - `modifiedAt` is included only when it differs from `publishedAt`. * - Sort order: malware bucket first; within a bucket, severity desc, * then `publishedAt` desc, then `osvId` asc (deterministic diff --git a/src/shared/parse-lines-option.ts b/src/shared/parse-lines-option.ts new file mode 100644 index 00000000..689543c8 --- /dev/null +++ b/src/shared/parse-lines-option.ts @@ -0,0 +1,67 @@ +import { InvalidPackageSpecError } from "./package-spec.js"; + +export interface LineRange { + startLine?: number; + endLine?: number; +} + +/** + * Parse the CLI `--lines` concise form. Grammar: + * `"N-M"` → start=N, end=M (both integers) + * `"N-"` → start=N, end=EOF + * `"-M"` → start=1, end=M + * Single-line `"N"` is rejected so callers fall through to `--start`. + */ +export function parseLinesOption(raw: string): LineRange { + const trimmed = raw.trim(); + const dashIndex = trimmed.indexOf("-"); + if (dashIndex < 0) { + throw new InvalidPackageSpecError( + `--lines expects a range like \`10-40\`, \`10-\`, or \`-40\`. Single-line form isn't accepted — use --start ${trimmed}.`, + ); + } + + const startRaw = trimmed.slice(0, dashIndex).trim(); + const endRaw = trimmed.slice(dashIndex + 1).trim(); + + if (startRaw.length === 0 && endRaw.length === 0) { + throw new InvalidPackageSpecError( + "--lines requires at least one bound. Use `10-40`, `10-` for open end, or `-40` for open start.", + ); + } + + const startLine = + startRaw.length > 0 + ? requirePositiveInteger(startRaw, "--lines start") + : undefined; + const endLine = + endRaw.length > 0 + ? requirePositiveInteger(endRaw, "--lines end") + : undefined; + + if (startLine !== undefined && endLine !== undefined && startLine > endLine) { + throw new InvalidPackageSpecError( + `--lines range is reversed: ${startLine} > ${endLine}.`, + ); + } + + if (startLine === undefined && endLine !== undefined) { + return { startLine: 1, endLine }; + } + return { startLine, endLine }; +} + +function requirePositiveInteger(raw: string, label: string): number { + if (!/^\d+$/.test(raw)) { + throw new InvalidPackageSpecError( + `${label} must be a positive integer. Got '${raw}'.`, + ); + } + const parsed = Number.parseInt(raw, 10); + if (parsed < 1) { + throw new InvalidPackageSpecError( + `${label} must be ≥ 1 (lines are 1-indexed). Got ${parsed}.`, + ); + } + return parsed; +} diff --git a/src/shared/read-package-doc-response.ts b/src/shared/read-package-doc-response.ts index 3e0edc18..5e4690f9 100644 --- a/src/shared/read-package-doc-response.ts +++ b/src/shared/read-package-doc-response.ts @@ -1,14 +1,9 @@ import type { PackageDocResult } from "../services/index.js"; import { MalformedPackageIntelligenceResponseError } from "../services/index.js"; import { colorize } from "./colors.js"; -import { - buildDocReadFollowUp, - buildFileReadFollowUp, - type DocReadFollowUp, - type FileReadFollowUp, - lowerDocSourceKind, -} from "./docs-follow-up.js"; +import { lowerDocSourceKind } from "./docs-follow-up.js"; import { toIsoDate } from "./format-date.js"; +import type { LineRange } from "./parse-lines-option.js"; export interface LeanPackageDocEnvelope { registry?: string; @@ -18,6 +13,12 @@ export interface LeanPackageDocEnvelope { title?: string; format?: string; content?: string; + /** Total lines in the source page; present whenever a `content` slice + * was applied or the source content was non-empty. */ + totalLines?: number; + /** Set when caller scoped output to a line range. */ + startLine?: number; + endLine?: number; breadcrumbs?: string[]; linkName?: string; lastUpdatedAt?: string; @@ -29,13 +30,12 @@ export interface LeanPackageDocEnvelope { requestedRef?: string; filePath?: string; baseUrl?: string; - followUp: DocReadFollowUp; - readFile?: FileReadFollowUp; } export function buildReadPackageDocSuccessPayload( result: PackageDocResult, requestedPageId: string, + range?: LineRange, ): LeanPackageDocEnvelope { const pageId = result.page?.id; if (!pageId) { @@ -53,18 +53,21 @@ export function buildReadPackageDocSuccessPayload( ); } - const envelope: LeanPackageDocEnvelope = { - pageId, - followUp: buildDocReadFollowUp(pageId) as DocReadFollowUp, - }; + const envelope: LeanPackageDocEnvelope = { pageId }; if (result.registry) envelope.registry = result.registry.toLowerCase(); if (result.packageName) envelope.name = result.packageName; if (result.version) envelope.version = result.version; if (result.page?.title) envelope.title = result.page.title; if (result.page?.contentFormat) envelope.format = result.page.contentFormat; - if (result.page?.content !== undefined) - envelope.content = result.page.content; + if (result.page?.content !== undefined) { + const sliced = sliceContent(result.page.content, range); + envelope.content = sliced.content; + if (sliced.totalLines !== undefined) + envelope.totalLines = sliced.totalLines; + if (sliced.startLine !== undefined) envelope.startLine = sliced.startLine; + if (sliced.endLine !== undefined) envelope.endLine = sliced.endLine; + } if (result.page?.breadcrumbs && result.page.breadcrumbs.length > 0) { envelope.breadcrumbs = result.page.breadcrumbs; } @@ -72,9 +75,10 @@ export function buildReadPackageDocSuccessPayload( if (result.page?.lastUpdatedAt) { envelope.lastUpdatedAt = toIsoDate(result.page.lastUpdatedAt) ?? undefined; } - envelope.sourceKind = lowerDocSourceKind( + const sourceKind = lowerDocSourceKind( result.page?.sourceKind ?? result.sourceKind, ); + if (sourceKind) envelope.sourceKind = sourceKind; if (result.page?.source?.url) envelope.sourceUrl = result.page.source.url; if (result.page?.source?.label) envelope.sourceLabel = result.page.source.label; @@ -84,13 +88,57 @@ export function buildReadPackageDocSuccessPayload( envelope.requestedRef = result.page.requestedRef; if (result.page?.filePath) envelope.filePath = result.page.filePath; if (result.page?.baseUrl) envelope.baseUrl = result.page.baseUrl; - envelope.readFile = result.page - ? buildFileReadFollowUp(result.page) - : undefined; return envelope; } +interface SlicedContent { + content: string; + totalLines?: number; + startLine?: number; + endLine?: number; +} + +/** + * Apply a 1-indexed inclusive line range to the doc body. Backend + * `fetchPackageDoc` does not accept startLine/endLine yet, so the + * slicing happens client-side. When no range is given, the body is + * returned untouched and only `totalLines` is reported (handy for + * agents deciding whether to fetch a range on a follow-up call). + */ +function sliceContent( + content: string, + range: LineRange | undefined, +): SlicedContent { + if (content.length === 0) { + return { content }; + } + + // Strip a single trailing newline so totalLines reflects "lines of + // text", not "split positions". The original trailing newline is + // preserved on the unsliced full body. + const trimmed = content.endsWith("\n") ? content.slice(0, -1) : content; + const lines = trimmed.split("\n"); + const totalLines = lines.length; + + if ( + !range || + (range.startLine === undefined && range.endLine === undefined) + ) { + return { content, totalLines }; + } + + const startLine = Math.max(1, range.startLine ?? 1); + const endLine = Math.min(totalLines, range.endLine ?? totalLines); + + if (startLine > totalLines) { + return { content: "", totalLines, startLine, endLine: startLine - 1 }; + } + + const sliced = lines.slice(startLine - 1, endLine).join("\n"); + return { content: sliced, totalLines, startLine, endLine }; +} + export interface FormatReadPackageDocTerminalOptions { useColors: boolean; verbose?: boolean; diff --git a/src/shared/search-symbols-request.test.ts b/src/shared/search-symbols-request.test.ts deleted file mode 100644 index 68cea716..00000000 --- a/src/shared/search-symbols-request.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { FILE_INTENT_ALL } from "./code-navigation-defaults.js"; -import { buildSearchSymbolsParams } from "./search-symbols-request.js"; - -describe("buildSearchSymbolsParams", () => { - const baseTarget = { registry: "NPM", packageName: "express" } as const; - - it("leaves fileIntent unset when the caller omits it", () => { - const { params, defaulted } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - }); - - expect(params.fileIntent).toBeUndefined(); - expect(defaulted).not.toContain("fileIntent"); - }); - - it("defaults waitTimeoutMs to 20000 and records it as defaulted", () => { - const { params, defaulted } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - }); - - expect(params.waitTimeoutMs).toBe(20_000); - expect(defaulted).toContain("waitTimeoutMs"); - }); - - it("preserves explicit fileIntent values and does not mark as defaulted", () => { - const { params, defaulted } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - fileIntent: "TEST", - }); - - expect(params.fileIntent).toBe("TEST"); - expect(defaulted).not.toContain("fileIntent"); - }); - - it("preserves explicit waitTimeoutMs and does not mark as defaulted", () => { - const { params, defaulted } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - waitTimeoutMs: 5000, - }); - - expect(params.waitTimeoutMs).toBe(5000); - expect(defaulted).not.toContain("waitTimeoutMs"); - }); - - it("maps FILE_INTENT_ALL sentinel to undefined (omit filter at service layer)", () => { - const { params, defaulted } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - fileIntent: FILE_INTENT_ALL, - }); - - expect(params.fileIntent).toBeUndefined(); - // Explicit caller choice — not a silent default. - expect(defaulted).not.toContain("fileIntent"); - }); - - it("passes all other fields through unchanged", () => { - const { params } = buildSearchSymbolsParams({ - target: baseTarget, - query: "middleware", - keywords: ["a", "b"], - matchMode: "AND", - kind: "FUNCTION", - filePath: "lib/", - limit: 25, - waitTimeoutMs: 3000, - }); - - expect(params).toEqual({ - target: baseTarget, - query: "middleware", - keywords: ["a", "b"], - matchMode: "AND", - kind: "FUNCTION", - filePath: "lib/", - limit: 25, - fileIntent: undefined, - waitTimeoutMs: 3000, - }); - }); -}); diff --git a/src/shared/search-symbols-request.ts b/src/shared/search-symbols-request.ts deleted file mode 100644 index f96a660f..00000000 --- a/src/shared/search-symbols-request.ts +++ /dev/null @@ -1,111 +0,0 @@ -import type { - CodeNavigationTarget, - SearchSymbolsFileIntent, - SearchSymbolsKind, - SearchSymbolsMatchMode, - SearchSymbolsParams, - SymbolCategory, -} from "../services/index.js"; -import { - DEFAULT_WAIT_TIMEOUT_MS, - FILE_INTENT_ALL, - type FileIntentInput, -} from "./code-navigation-defaults.js"; - -/** - * Caller-facing, surface-agnostic input to `search_symbols`. Both the - * CLI command and the MCP tool normalise their respective arguments - * into this shape before invoking `buildSearchSymbolsParams`, so the - * two surfaces cannot diverge on default application or semantic - * translation. - * - * `fileIntent` is tri-valued: specific intent, `FILE_INTENT_ALL` - * (caller asked for all), or `undefined` (caller omitted the field and - * the request should carry no file-intent filter). - */ -export interface SearchSymbolsRequestInput { - target: CodeNavigationTarget; - query?: string; - keywords?: string[]; - matchMode?: SearchSymbolsMatchMode; - kind?: SearchSymbolsKind; - category?: SymbolCategory; - filePath?: string; - limit?: number; - fileIntent?: FileIntentInput; - waitTimeoutMs?: number; -} - -/** - * Result of building a search-symbols request. The `params` object is - * ready to hand to `CodeNavigationService.searchSymbols`; the - * `defaulted` array lists the fields whose values were filled in by - * this builder rather than supplied by the caller, for inclusion in - * the surface's `query.defaulted` echo on the JSON envelope. - */ -export interface SearchSymbolsRequestBuildResult { - params: SearchSymbolsParams; - defaulted: ReadonlyArray<"waitTimeoutMs">; -} - -/** - * Build a `SearchSymbolsParams` object from caller-facing input. - * - * Responsibilities: - * - Fill in the default for `waitTimeoutMs`. - * - Translate the `FILE_INTENT_ALL` sentinel to "omit the GraphQL - * variable" by leaving `fileIntent: undefined` on the outgoing - * params — omission returns all intents on the live backend. - * - Record which fields were defaulted so the surface can mark them - * in its JSON echo. - * - * Shared between CLI and MCP per PARITY-REQUEST and PARITY-DEFAULTS. - */ -export function buildSearchSymbolsParams( - input: SearchSymbolsRequestInput, -): SearchSymbolsRequestBuildResult { - const defaulted: Array<"waitTimeoutMs"> = []; - - const resolvedFileIntent = resolveFileIntent(input.fileIntent); - const resolvedWaitTimeoutMs = resolveWaitTimeoutMs( - input.waitTimeoutMs, - defaulted, - ); - - return { - params: { - target: input.target, - query: input.query, - keywords: input.keywords, - matchMode: input.matchMode, - kind: input.kind, - category: input.category, - filePath: input.filePath, - limit: input.limit, - fileIntent: resolvedFileIntent, - waitTimeoutMs: resolvedWaitTimeoutMs, - }, - defaulted, - }; -} - -function resolveFileIntent( - input: FileIntentInput, -): SearchSymbolsFileIntent | undefined { - if (input === FILE_INTENT_ALL) { - // Caller asked for all intents; send no filter to the backend. - return undefined; - } - return input; -} - -function resolveWaitTimeoutMs( - input: number | undefined, - defaulted: Array<"waitTimeoutMs">, -): number { - if (input === undefined) { - defaulted.push("waitTimeoutMs"); - return DEFAULT_WAIT_TIMEOUT_MS; - } - return input; -} diff --git a/src/shared/search-symbols-response.test.ts b/src/shared/search-symbols-response.test.ts deleted file mode 100644 index bcc60a41..00000000 --- a/src/shared/search-symbols-response.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { - CodeNavigationBackendError, - CodeNavigationIndexingError, - CodeNavigationVersionNotFoundError, -} from "../services/code-navigation-service.js"; -import type { - SearchSymbolsParams, - SearchSymbolsResult, -} from "../services/index.js"; -import { - buildSearchSymbolsErrorPayload, - buildSearchSymbolsSuccessPayload, -} from "./search-symbols-response.js"; - -const baseParams: SearchSymbolsParams = { - target: { registry: "NPM", packageName: "express" }, - query: "middleware", - fileIntent: "PRODUCTION", - waitTimeoutMs: 20_000, -}; - -const empty: SearchSymbolsResult = { - results: [], - totalMatches: 0, - hasMore: false, -}; - -describe("buildSearchSymbolsSuccessPayload — query echo", () => { - it("echoes category as a lowercase string when the caller supplied one", () => { - const payload = buildSearchSymbolsSuccessPayload( - { - ...baseParams, - category: "CALLABLE", - }, - [], - empty, - ); - expect(payload.query.category).toBe("callable"); - }); - - it("omits category from the echo when the caller did not supply one", () => { - const payload = buildSearchSymbolsSuccessPayload(baseParams, [], empty); - expect(payload.query.category).toBeUndefined(); - }); -}); - -describe("buildSearchSymbolsSuccessPayload — zero-result hint handling", () => { - it("passes the backend zero-result hint through in the success envelope", () => { - const payload = buildSearchSymbolsSuccessPayload(baseParams, [], { - ...empty, - version: "v5.2.1", - hint: "120 chunks indexed across 45 files. Try broader search terms or use fetch_code_context to read specific files directly.", - }); - expect(payload.hint).toContain("120 chunks indexed across 45 files"); - }); - - it("passes the docs-only hint through on zero-result", () => { - const payload = buildSearchSymbolsSuccessPayload(baseParams, [], { - ...empty, - hint: "This package has no searchable code chunks — it may be docs-only, binary-heavy, or use a language the extractor doesn't support.", - }); - expect(payload.hint).toContain("docs-only, binary-heavy"); - }); - - it("suppresses the legacy '0 searchable chunks' phrasing even when backend regresses", () => { - const payload = buildSearchSymbolsSuccessPayload(baseParams, [], { - ...empty, - hint: "Repository indexed but contains 0 searchable chunks.", - }); - expect(payload.hint).toBeUndefined(); - }); - - it("omits hint entirely when backend sends none", () => { - const payload = buildSearchSymbolsSuccessPayload(baseParams, [], empty); - expect(payload.hint).toBeUndefined(); - }); -}); - -describe("buildSearchSymbolsErrorPayload — retryable surfacing", () => { - it("surfaces retryable: true for INDEXING", () => { - const payload = buildSearchSymbolsErrorPayload( - new CodeNavigationIndexingError("still indexing", "idx-1"), - ); - expect(payload.code).toBe("INDEXING"); - expect(payload.retryable).toBe(true); - }); - - it("surfaces retryable: false for VERSION_NOT_FOUND and carries structured details", () => { - const payload = buildSearchSymbolsErrorPayload( - new CodeNavigationVersionNotFoundError( - "No version of npm/express matches '4'.", - "npm/express", - "4", - "5.2.1", - [{ version: "5.2.1", ref: "v5.2.1" }], - ), - ); - expect(payload.code).toBe("VERSION_NOT_FOUND"); - expect(payload.retryable).toBe(false); - expect(payload.details).toMatchObject({ - package: "npm/express", - requestedVersion: "4", - latestIndexed: "5.2.1", - }); - }); - - it("honours backend-supplied retryable override on CodeNavigationBackendError", () => { - const payload = buildSearchSymbolsErrorPayload( - new CodeNavigationBackendError("hiccup", 500, "INTERNAL_ERROR", true), - ); - expect(payload.code).toBe("BACKEND_ERROR"); - expect(payload.retryable).toBe(true); - }); -}); diff --git a/src/shared/search-symbols-response.ts b/src/shared/search-symbols-response.ts deleted file mode 100644 index db1e0197..00000000 --- a/src/shared/search-symbols-response.ts +++ /dev/null @@ -1,150 +0,0 @@ -import type { - CodeNavigationTarget, - SearchSymbolsParams, - SearchSymbolsResult, -} from "../services/index.js"; -import { mapCodeNavigationError } from "./code-navigation-error-map.js"; - -/** - * The canonical success envelope emitted by both the CLI (`--json`) - * and the MCP tool text payload. Both paths round-trip through - * `buildSearchSymbolsSuccessPayload` so the shapes can never drift. - * - * No leading-underscore keys — `warning` / `hint` are plain. - * `returnedCount` is an explicit echo of `results.length`; - * `totalMatches` carries the backend-provided total (today equal to - * `returnedCount` — see backend request B2). - */ -export interface SearchSymbolsSuccessPayload { - query: SearchSymbolsQueryEcho; - results: SearchSymbolsResult["results"]; - returnedCount: number; - totalMatches: number; - hasMore: boolean; - version?: string; - resolution?: SearchSymbolsResult["resolution"]; - warning?: string; - hint?: string; -} - -/** - * Echo of the resolved request. Agents and human readers see exactly - * which parameters were applied to the search; `defaulted` names the - * fields the client filled in rather than the caller supplying. - */ -export interface SearchSymbolsQueryEcho { - target: CodeNavigationTarget; - query?: string; - keywords?: string[]; - matchMode?: string; - kind?: string; - /** - * Lowercase broad-category filter applied to the query — one of - * `callable`, `type`, `module`, `data`, `documentation`. Absent - * when the caller supplied no category. - */ - category?: string; - filePath?: string; - fileIntent: string; // lowercase enum value or the literal "all" - limit?: number; - waitTimeoutMs: number; - defaulted: ReadonlyArray<"waitTimeoutMs">; -} - -export interface SearchSymbolsErrorPayload { - error: string; - code: string; - /** - * Whether the caller can retry the same request. Sourced from - * `mapCodeNavigationError(...).retryable`; populated on every - * error path so agents do not need a per-code retryability table. - */ - retryable?: boolean; - details?: Record; -} - -/** - * Build the success envelope from the already-resolved request - * `params` plus the raw service result and the `defaulted` array - * returned by `buildSearchSymbolsParams`. - * - * `buildSuccessPayload` is pure so it is cheap to cover in both the - * CLI and MCP surface tests and in the shared parity test. - */ -export function buildSearchSymbolsSuccessPayload( - params: SearchSymbolsParams, - defaulted: ReadonlyArray<"waitTimeoutMs">, - result: SearchSymbolsResult, -): SearchSymbolsSuccessPayload { - const payload: SearchSymbolsSuccessPayload = { - query: { - target: params.target, - query: params.query, - keywords: params.keywords, - matchMode: params.matchMode?.toLowerCase(), - kind: params.kind?.toLowerCase(), - category: params.category?.toLowerCase(), - filePath: params.filePath, - fileIntent: echoFileIntent(params.fileIntent), - limit: params.limit, - // waitTimeoutMs is always resolved to a concrete number by the builder. - waitTimeoutMs: params.waitTimeoutMs ?? 0, - defaulted, - }, - results: result.results, - returnedCount: result.results.length, - totalMatches: result.totalMatches, - hasMore: result.hasMore, - }; - - if (result.version) payload.version = result.version; - if (result.resolution) payload.resolution = result.resolution; - if (result.warning) payload.warning = result.warning; - // Pass the server hint through on all result counts. The April - // 2026 backend rewrite made zero-result hints accurate and - // actionable ("N chunks indexed across M files..." or "docs-only, - // binary-heavy, or uses an unsupported language..."). Defensive - // filter for the legacy "0 searchable chunks" string remains in - // place until every backend deploy is confirmed on the new - // contract. - if (result.hint && !isLegacyZeroChunksHint(result.hint)) { - payload.hint = result.hint; - } - - return payload; -} - -function isLegacyZeroChunksHint(hint: string): boolean { - return hint.toLowerCase().includes("0 searchable chunks"); -} - -/** - * Build the error envelope from any thrown error. Routes through - * `mapCodeNavigationError` so the emitted `code` is stable and every - * non-UNKNOWN branch carries a specific classification. - */ -export function buildSearchSymbolsErrorPayload( - error: unknown, -): SearchSymbolsErrorPayload { - const mapped = mapCodeNavigationError(error); - const payload: SearchSymbolsErrorPayload = { - error: mapped.message, - code: mapped.code, - }; - if (typeof mapped.retryable === "boolean") { - payload.retryable = mapped.retryable; - } - if (mapped.details && Object.keys(mapped.details).length > 0) { - payload.details = mapped.details as Record; - } - return payload; -} - -function echoFileIntent(resolved: SearchSymbolsParams["fileIntent"]): string { - // Omitted file intent means "search across all intents". Keep - // echoing that as the literal "all" so the legacy JSON contract - // stays stable whether the caller explicitly chose the alias or just - // omitted the filter. - if (resolved === undefined) return "all"; - return resolved.toLowerCase(); -} diff --git a/src/shared/unified-search-request.test.ts b/src/shared/unified-search-request.test.ts index ac809e25..08cef82b 100644 --- a/src/shared/unified-search-request.test.ts +++ b/src/shared/unified-search-request.test.ts @@ -14,7 +14,6 @@ describe("buildUnifiedSearchParams", () => { expect(built.params.limit).toBe(20); expect(built.params.offset).toBe(0); expect(built.params.waitTimeoutMs).toBe(20_000); - expect(built.defaulted).toEqual(["limit", "offset", "waitTimeoutMs"]); }); it("does not override explicit fileIntent", () => { @@ -25,7 +24,6 @@ describe("buildUnifiedSearchParams", () => { }); expect(built.params.filters).toEqual({ fileIntent: "TEST" }); - expect(built.defaulted).not.toContain("fileIntent"); }); it("leaves fileIntent unset for explicit docs-only searches", () => { @@ -36,7 +34,6 @@ describe("buildUnifiedSearchParams", () => { }); expect(built.params.filters).toBeUndefined(); - expect(built.defaulted).toEqual(["limit", "offset", "waitTimeoutMs"]); }); it("does not invent fileIntent when selected sources include code search", () => { @@ -47,7 +44,6 @@ describe("buildUnifiedSearchParams", () => { }); expect(built.params.filters).toBeUndefined(); - expect(built.defaulted).toEqual(["limit", "offset", "waitTimeoutMs"]); }); it("compiles structured name and language into AND-ed query qualifiers", () => { diff --git a/src/shared/unified-search-request.ts b/src/shared/unified-search-request.ts index 5a420857..13b5d0d9 100644 --- a/src/shared/unified-search-request.ts +++ b/src/shared/unified-search-request.ts @@ -1,8 +1,8 @@ import type { CodeNavigationTarget, - SearchSymbolsFileIntent, - SearchSymbolsKind, + FileIntent, SymbolCategory, + SymbolKind, UnifiedSearchFilters, UnifiedSearchParams, UnifiedSearchSource, @@ -15,10 +15,10 @@ export interface UnifiedSearchRequestInput { targets?: CodeNavigationTarget[]; query: string; sources?: UnifiedSearchSource[]; - kind?: SearchSymbolsKind; + kind?: SymbolKind; category?: SymbolCategory; pathPrefix?: string; - fileIntent?: SearchSymbolsFileIntent; + fileIntent?: FileIntent; publicOnly?: boolean; name?: string; language?: string; @@ -32,7 +32,6 @@ export interface UnifiedSearchRequestBuildResult { params: UnifiedSearchParams; rawQuery: string; compiledQuery: string; - defaulted: ReadonlyArray<"limit" | "offset" | "waitTimeoutMs">; } export function buildUnifiedSearchParams( @@ -40,16 +39,10 @@ export function buildUnifiedSearchParams( ): UnifiedSearchRequestBuildResult { const targets = resolveTargets(input.target, input.targets); const rawQuery = normaliseRequiredQuery(input.query); - const defaulted: Array<"limit" | "offset" | "waitTimeoutMs"> = []; - const limit = resolveNumber(input.limit, 20, "limit", defaulted); - const offset = resolveNumber(input.offset, 0, "offset", defaulted); - const waitTimeoutMs = resolveNumber( - input.waitTimeoutMs, - DEFAULT_WAIT_TIMEOUT_MS, - "waitTimeoutMs", - defaulted, - ); + const limit = input.limit ?? 20; + const offset = input.offset ?? 0; + const waitTimeoutMs = input.waitTimeoutMs ?? DEFAULT_WAIT_TIMEOUT_MS; const qualifierClauses = buildQualifierClauses({ name: input.name, @@ -78,7 +71,6 @@ export function buildUnifiedSearchParams( }, rawQuery, compiledQuery, - defaulted, }; } @@ -125,19 +117,6 @@ function normaliseRequiredQuery(query: string): string { return trimmed; } -function resolveNumber( - value: number | undefined, - fallback: number, - field: "limit" | "offset" | "waitTimeoutMs", - defaulted: Array<"limit" | "offset" | "waitTimeoutMs">, -): number { - if (value === undefined) { - defaulted.push(field); - return fallback; - } - return value; -} - function buildQualifierClauses(input: { name?: string; language?: string; @@ -182,10 +161,10 @@ function compileQuery(rawQuery: string, qualifierClauses: string[]): string { } function buildFilters(input: { - kind?: SearchSymbolsKind; + kind?: SymbolKind; category?: SymbolCategory; pathPrefix?: string; - fileIntent?: SearchSymbolsFileIntent; + fileIntent?: FileIntent; publicOnly?: boolean; }): UnifiedSearchFilters | undefined { const filters: UnifiedSearchFilters = {}; diff --git a/src/shared/unified-search-response.test.ts b/src/shared/unified-search-response.test.ts index 87ee963b..b429f11f 100644 --- a/src/shared/unified-search-response.test.ts +++ b/src/shared/unified-search-response.test.ts @@ -21,12 +21,11 @@ describe("buildUnifiedSearchSuccessPayload", () => { params, "router middleware", "router middleware", - ["limit", "offset", "waitTimeoutMs"], defaultUnifiedSearchOutcome, ); expect(payload.completed).toBe(true); - expect(payload.returnedCount).toBe(1); + expect(payload.results.length).toBe(1); expect(payload.results[0]).toEqual({ type: "repository_code", target: "npm:express@4.18.2", @@ -44,12 +43,26 @@ describe("buildUnifiedSearchSuccessPayload", () => { }); }); + it("omits default-valued query echo fields", () => { + const payload = buildUnifiedSearchSuccessPayload( + params, + "router middleware", + "router middleware", + defaultUnifiedSearchOutcome, + ); + + // Default limit/offset/waitTimeoutMs/allowPartialResults all omitted. + // No `compiled` because it equals raw. No `warnings` because empty. + expect(payload.query).toEqual({ + raw: "router middleware", + }); + }); + it("normalises incomplete outcomes without partial results", () => { const payload = buildUnifiedSearchSuccessPayload( params, "router middleware", "router middleware", - ["limit", "offset", "waitTimeoutMs"], { state: "incomplete", completed: false, @@ -68,16 +81,17 @@ describe("buildUnifiedSearchSuccessPayload", () => { ); expect(payload).toEqual({ - query: expect.objectContaining({ - filters: undefined, - defaulted: ["limit", "offset", "waitTimeoutMs"], - }), + query: { raw: "router middleware" }, completed: false, - returnedCount: 0, hasMore: false, results: [], searchRef: "search-ref-123", - progress: expect.objectContaining({ status: "INDEXING" }), + progress: { + status: "INDEXING", + targetsReady: 0, + targetsTotal: 1, + elapsedMs: 200, + }, }); }); @@ -90,7 +104,6 @@ describe("buildUnifiedSearchSuccessPayload", () => { { ...params, allowPartialResults: true }, "router middleware", "router middleware", - ["limit", "offset", "waitTimeoutMs"], { state: "incomplete", completed: false, @@ -114,11 +127,8 @@ describe("buildUnifiedSearchSuccessPayload", () => { expect(payload.completed).toBe(false); expect(payload.query.allowPartialResults).toBe(true); - expect(payload.returnedCount).toBe(1); + expect(payload.results.length).toBe(1); expect(payload.results[0]?.target).toBe("npm:express@4.18.2"); - expect(payload.sourceStatus).toEqual( - defaultUnifiedSearchOutcome.result.sourceStatus, - ); }); }); @@ -155,7 +165,12 @@ describe("buildUnifiedSearchStatusPayload", () => { expect(payload).toEqual({ completed: false, searchRef: "search-ref-123", - progress: expect.objectContaining({ status: "INDEXING" }), + progress: { + status: "INDEXING", + targetsReady: 0, + targetsTotal: 1, + elapsedMs: 200, + }, }); }); @@ -174,19 +189,14 @@ describe("buildUnifiedSearchStatusPayload", () => { } expect(payload.result).toEqual({ - query: "router middleware", - queryWarnings: [], sources: ["code"], - returnedCount: 1, hasMore: false, - nextOffset: undefined, results: [ expect.objectContaining({ type: "repository_code", target: "npm:express@4.18.2", }), ], - sourceStatus: defaultUnifiedSearchOutcome.result.sourceStatus, }); }); }); diff --git a/src/shared/unified-search-response.ts b/src/shared/unified-search-response.ts index 75da7323..fd76fdda 100644 --- a/src/shared/unified-search-response.ts +++ b/src/shared/unified-search-response.ts @@ -4,31 +4,26 @@ import type { UnifiedSearchIncomplete, UnifiedSearchOutcome, UnifiedSearchParams, + UnifiedSearchProgress, + UnifiedSearchSourceStatus, } from "../services/index.js"; import { MalformedCodeNavigationResponseError } from "../services/index.js"; +import { DEFAULT_WAIT_TIMEOUT_MS } from "./code-navigation-defaults.js"; import { mapCodeNavigationError } from "./code-navigation-error-map.js"; -import { - buildDocReadFollowUp, - buildFileReadFollowUp, -} from "./docs-follow-up.js"; -export type UnifiedSearchFollowUpPayload = - | { - type: "read_doc"; - pageId: string; - } - | { - type: "read_file"; - repoUrl: string; - gitRef: string; - path: string; - }; +/** + * Default values folded out of the JSON envelope when the caller did + * not set them. Agents asked for what they want; echoing the default + * back wastes tokens. The defaults must stay aligned with + * `buildUnifiedSearchParams`. + */ +const DEFAULT_LIMIT = 20; +const DEFAULT_OFFSET = 0; export interface UnifiedSearchQueryEcho { raw: string; - compiled: string; - warnings: string[]; - targets: UnifiedSearchParams["targets"]; + compiled?: string; + warnings?: string[]; sources?: string[]; filters?: { kind?: string; @@ -37,11 +32,15 @@ export interface UnifiedSearchQueryEcho { fileIntent?: string; publicOnly?: boolean; }; - allowPartialResults: boolean; - limit: number; - offset: number; - waitTimeoutMs: number; - defaulted: ReadonlyArray<"limit" | "offset" | "waitTimeoutMs">; + allowPartialResults?: true; + limit?: number; + offset?: number; + waitTimeoutMs?: number; +} + +export interface UnifiedSearchHighlightsPayload { + title?: Array; + summary?: Array; } export interface UnifiedSearchHitPayload { @@ -50,10 +49,7 @@ export interface UnifiedSearchHitPayload { title?: string; summary?: string; score?: number; - highlights?: { - title?: Array; - summary?: Array; - }; + highlights?: UnifiedSearchHighlightsPayload; locator: { registry?: string; packageName?: string; @@ -67,39 +63,53 @@ export interface UnifiedSearchHitPayload { filePath?: string; startLine?: number; endLine?: number; - fileContentHash?: string; - symbolRef?: string; qualifiedPath?: string; kind?: string; category?: string; language?: string; }; - followUp?: UnifiedSearchFollowUpPayload; - alternateFollowUps?: UnifiedSearchFollowUpPayload[]; +} + +export interface UnifiedSearchProgressPayload { + status: string; + targetsReady: number; + targetsTotal: number; + elapsedMs: number; + expiresAt?: string; +} + +export interface UnifiedSearchSourceStatusPayload { + source: string; + targetLabel: string; + indexingStatus?: string; + codeIndexState?: string; + resultCount?: number; + ignoredFilters?: string[]; + incompatibleFilters?: string[]; + ignoredQueryFeatures?: string[]; + incompatibleQueryFeatures?: string[]; + note?: string; } export interface UnifiedSearchCompletedPayload { query: UnifiedSearchQueryEcho; completed: true; - returnedCount: number; hasMore: boolean; nextOffset?: number; results: UnifiedSearchHitPayload[]; searchRef?: string; - progress?: UnifiedSearchCompleted["progress"]; - sourceStatus: UnifiedSearchCompleted["result"]["sourceStatus"]; + sourceStatus?: UnifiedSearchSourceStatusPayload[]; } export interface UnifiedSearchIncompletePayload { query: UnifiedSearchQueryEcho; completed: false; - returnedCount: number; hasMore: boolean; nextOffset?: number; results: UnifiedSearchHitPayload[]; searchRef: string; - progress?: UnifiedSearchIncomplete["progress"]; - sourceStatus?: UnifiedSearchCompleted["result"]["sourceStatus"]; + progress?: UnifiedSearchProgressPayload; + sourceStatus?: UnifiedSearchSourceStatusPayload[]; } export interface UnifiedSearchErrorPayload { @@ -110,27 +120,24 @@ export interface UnifiedSearchErrorPayload { } export interface UnifiedSearchStatusResultPayload { - query: string; - queryWarnings: string[]; - sources: string[]; - returnedCount: number; + warnings?: string[]; + sources?: string[]; hasMore: boolean; nextOffset?: number; results: UnifiedSearchHitPayload[]; - sourceStatus: UnifiedSearchCompleted["result"]["sourceStatus"]; + sourceStatus?: UnifiedSearchSourceStatusPayload[]; } export interface UnifiedSearchStatusCompletedPayload { completed: true; searchRef?: string; - progress?: UnifiedSearchCompleted["progress"]; result: UnifiedSearchStatusResultPayload; } export interface UnifiedSearchStatusIncompletePayload { completed: false; searchRef: string; - progress?: UnifiedSearchIncomplete["progress"]; + progress?: UnifiedSearchProgressPayload; result?: UnifiedSearchStatusResultPayload; } @@ -138,7 +145,6 @@ export function buildUnifiedSearchSuccessPayload( params: UnifiedSearchParams, rawQuery: string, compiledQuery: string, - defaulted: ReadonlyArray<"limit" | "offset" | "waitTimeoutMs">, outcome: UnifiedSearchOutcome, ): UnifiedSearchCompletedPayload | UnifiedSearchIncompletePayload { const warnings = @@ -147,47 +153,41 @@ export function buildUnifiedSearchSuccessPayload( : (outcome.result?.queryWarnings ?? outcome.progress?.queryWarnings ?? []); - const query = buildQueryEcho( - params, - rawQuery, - compiledQuery, - defaulted, - warnings, - ); + const query = buildQueryEcho(params, rawQuery, compiledQuery, warnings); if (outcome.state === "incomplete") { const result = outcome.result; const payload: UnifiedSearchIncompletePayload = { query, completed: false, - returnedCount: result?.results.length ?? 0, hasMore: result?.page.hasMore ?? false, results: result?.results.map(buildHitPayload) ?? [], searchRef: outcome.searchRef, - progress: outcome.progress, }; if (result?.page.hasMore === true) { payload.nextOffset = result.page.offset + result.page.returned; } - if (result) { - payload.sourceStatus = result.sourceStatus; - } + const progress = compactProgress(outcome.progress); + if (progress) payload.progress = progress; + const sourceStatus = compactSourceStatus(result?.sourceStatus); + if (sourceStatus) payload.sourceStatus = sourceStatus; return payload; } - return { + const completed: UnifiedSearchCompletedPayload = { query, completed: true, - returnedCount: outcome.result.results.length, hasMore: outcome.result.page.hasMore, - nextOffset: outcome.result.page.hasMore - ? outcome.result.page.offset + outcome.result.page.returned - : undefined, results: outcome.result.results.map(buildHitPayload), - searchRef: outcome.searchRef, - progress: outcome.progress, - sourceStatus: outcome.result.sourceStatus, }; + if (outcome.result.page.hasMore) { + completed.nextOffset = + outcome.result.page.offset + outcome.result.page.returned; + } + if (outcome.searchRef) completed.searchRef = outcome.searchRef; + const sourceStatus = compactSourceStatus(outcome.result.sourceStatus); + if (sourceStatus) completed.sourceStatus = sourceStatus; + return completed; } export function buildUnifiedSearchErrorPayload( @@ -214,101 +214,226 @@ export function buildUnifiedSearchStatusPayload( const payload: UnifiedSearchStatusIncompletePayload = { completed: false, searchRef: outcome.searchRef, - progress: outcome.progress, }; + const progress = compactProgress(outcome.progress); + if (progress) payload.progress = progress; if (outcome.result) { payload.result = buildUnifiedSearchStatusResultPayload(outcome.result); } return payload; } - return { + const payload: UnifiedSearchStatusCompletedPayload = { completed: true, - searchRef: outcome.searchRef, - progress: outcome.progress, result: buildUnifiedSearchStatusResultPayload(outcome.result), }; + if (outcome.searchRef) payload.searchRef = outcome.searchRef; + return payload; } function buildUnifiedSearchStatusResultPayload( result: UnifiedSearchCompleted["result"], ): UnifiedSearchStatusResultPayload { - return { - query: result.query, - queryWarnings: result.queryWarnings, - sources: result.sources.map((entry) => entry.toLowerCase()), - returnedCount: result.results.length, + const payload: UnifiedSearchStatusResultPayload = { hasMore: result.page.hasMore, - nextOffset: result.page.hasMore - ? result.page.offset + result.page.returned - : undefined, results: result.results.map(buildHitPayload), - sourceStatus: result.sourceStatus, }; + if (result.page.hasMore) { + payload.nextOffset = result.page.offset + result.page.returned; + } + if (result.queryWarnings.length > 0) { + payload.warnings = result.queryWarnings; + } + if (result.sources.length > 0) { + payload.sources = result.sources.map((entry) => entry.toLowerCase()); + } + const sourceStatus = compactSourceStatus(result.sourceStatus); + if (sourceStatus) payload.sourceStatus = sourceStatus; + return payload; } function buildQueryEcho( params: UnifiedSearchParams, rawQuery: string, compiledQuery: string, - defaulted: ReadonlyArray<"limit" | "offset" | "waitTimeoutMs">, warnings: string[], ): UnifiedSearchQueryEcho { - return { + const echo: UnifiedSearchQueryEcho = { raw: rawQuery, - compiled: compiledQuery, - warnings, - targets: params.targets, - sources: params.sources?.map((entry) => entry.toLowerCase()), - filters: params.filters - ? { - kind: params.filters.kind?.toLowerCase(), - category: params.filters.category?.toLowerCase(), - pathPrefix: params.filters.pathPrefix, - fileIntent: params.filters.fileIntent?.toLowerCase(), - publicOnly: params.filters.publicOnly, - } - : undefined, - allowPartialResults: params.allowPartialResults ?? false, - limit: params.limit ?? 0, - offset: params.offset ?? 0, - waitTimeoutMs: params.waitTimeoutMs ?? 0, - defaulted, }; + if (compiledQuery !== rawQuery) { + echo.compiled = compiledQuery; + } + if (warnings.length > 0) { + echo.warnings = warnings; + } + if (params.sources && params.sources.length > 0) { + echo.sources = params.sources.map((entry) => entry.toLowerCase()); + } + if (params.filters) { + const filters: UnifiedSearchQueryEcho["filters"] = {}; + if (params.filters.kind) filters.kind = params.filters.kind.toLowerCase(); + if (params.filters.category) + filters.category = params.filters.category.toLowerCase(); + if (params.filters.pathPrefix) + filters.pathPrefix = params.filters.pathPrefix; + if (params.filters.fileIntent) + filters.fileIntent = params.filters.fileIntent.toLowerCase(); + if (typeof params.filters.publicOnly === "boolean") + filters.publicOnly = params.filters.publicOnly; + if (Object.keys(filters).length > 0) echo.filters = filters; + } + if (params.allowPartialResults === true) { + echo.allowPartialResults = true; + } + if (params.limit !== undefined && params.limit !== DEFAULT_LIMIT) { + echo.limit = params.limit; + } + if (params.offset !== undefined && params.offset !== DEFAULT_OFFSET) { + echo.offset = params.offset; + } + if ( + params.waitTimeoutMs !== undefined && + params.waitTimeoutMs !== DEFAULT_WAIT_TIMEOUT_MS + ) { + echo.waitTimeoutMs = params.waitTimeoutMs; + } + return echo; } function buildHitPayload(hit: UnifiedSearchHit): UnifiedSearchHitPayload { assertSearchFollowUpInvariant(hit); - return { + const payload: UnifiedSearchHitPayload = { type: hit.resultType.toLowerCase(), target: hit.targetLabel, - title: hit.title, - summary: hit.summary, - score: hit.score, - highlights: hit.highlights, - locator: { - registry: hit.locator.registry, - packageName: hit.locator.packageName, - version: hit.locator.version, - pageId: hit.locator.pageId, - sourceKind: hit.locator.sourceKind, - sourceUrl: hit.locator.sourceUrl, - repoUrl: hit.locator.repoUrl, - gitRef: hit.locator.gitRef, - requestedRef: hit.locator.requestedRef, - filePath: hit.locator.filePath, - startLine: hit.locator.startLine, - endLine: hit.locator.endLine, - fileContentHash: hit.locator.fileContentHash, - symbolRef: hit.locator.symbolRef, - qualifiedPath: hit.locator.qualifiedPath, - kind: hit.locator.kind, - category: hit.locator.category, - language: hit.locator.language, - }, - followUp: buildPrimaryFollowUp(hit), - alternateFollowUps: buildAlternateFollowUps(hit), + locator: buildLocatorPayload(hit), }; + if (hit.title) payload.title = hit.title; + if (hit.summary) payload.summary = hit.summary; + if (typeof hit.score === "number") payload.score = hit.score; + const highlights = buildHighlights(hit.highlights); + if (highlights) payload.highlights = highlights; + return payload; +} + +function buildLocatorPayload( + hit: UnifiedSearchHit, +): UnifiedSearchHitPayload["locator"] { + const locator: UnifiedSearchHitPayload["locator"] = {}; + const src = hit.locator; + if (src.registry) locator.registry = src.registry; + if (src.packageName) locator.packageName = src.packageName; + if (src.version) locator.version = src.version; + if (src.pageId) locator.pageId = src.pageId; + if (src.sourceKind) locator.sourceKind = src.sourceKind; + if (src.sourceUrl) locator.sourceUrl = src.sourceUrl; + if (src.repoUrl) locator.repoUrl = src.repoUrl; + if (src.gitRef) locator.gitRef = src.gitRef; + if (src.requestedRef) locator.requestedRef = src.requestedRef; + if (src.filePath) locator.filePath = src.filePath; + if (typeof src.startLine === "number") locator.startLine = src.startLine; + if (typeof src.endLine === "number") locator.endLine = src.endLine; + // Top-level symbols often have qualifiedPath identical to title; + // skip the duplicate. Nested members (e.g. `MyClass.method`) still + // carry the disambiguating path. + if (src.qualifiedPath && src.qualifiedPath !== hit.title) { + locator.qualifiedPath = src.qualifiedPath; + } + if (src.kind) locator.kind = src.kind; + if (src.category) locator.category = src.category; + if (src.language) locator.language = src.language; + return locator; +} + +function buildHighlights( + highlights: UnifiedSearchHit["highlights"], +): UnifiedSearchHighlightsPayload | undefined { + if (!highlights) return undefined; + const compact: UnifiedSearchHighlightsPayload = {}; + if (highlights.title && highlights.title.length > 0) { + compact.title = highlights.title; + } + if (highlights.summary && highlights.summary.length > 0) { + compact.summary = highlights.summary; + } + return Object.keys(compact).length > 0 ? compact : undefined; +} + +function compactProgress( + progress: UnifiedSearchProgress | undefined, +): UnifiedSearchProgressPayload | undefined { + if (!progress) return undefined; + const payload: UnifiedSearchProgressPayload = { + status: progress.status, + targetsReady: progress.targetsReady, + targetsTotal: progress.targetsTotal, + elapsedMs: progress.elapsedMs, + }; + if (progress.expiresAt) payload.expiresAt = progress.expiresAt; + return payload; +} + +function compactSourceStatus( + sourceStatus: UnifiedSearchSourceStatus[] | undefined, +): UnifiedSearchSourceStatusPayload[] | undefined { + if (!sourceStatus || sourceStatus.length === 0) return undefined; + const compact: UnifiedSearchSourceStatusPayload[] = []; + for (const entry of sourceStatus) { + const slim = compactSourceStatusEntry(entry); + if (slim) compact.push(slim); + } + return compact.length > 0 ? compact : undefined; +} + +function compactSourceStatusEntry( + entry: UnifiedSearchSourceStatus, +): UnifiedSearchSourceStatusPayload | undefined { + const payload: UnifiedSearchSourceStatusPayload = { + source: entry.source.toLowerCase(), + targetLabel: entry.targetLabel, + }; + let interesting = false; + + // Suppress healthy lifecycle states. INDEXED means searchable; CURRENT + // and STALE both have data agents can use (STALE = served from a slightly + // older navpack while a reindex runs — we do not warn agents about it). + if (entry.indexingStatus && entry.indexingStatus !== "INDEXED") { + payload.indexingStatus = entry.indexingStatus; + interesting = true; + } + if ( + entry.codeIndexState && + entry.codeIndexState !== "CURRENT" && + entry.codeIndexState !== "STALE" + ) { + payload.codeIndexState = entry.codeIndexState; + interesting = true; + } + if (typeof entry.resultCount === "number" && entry.resultCount > 0) { + payload.resultCount = entry.resultCount; + } + if (entry.ignoredFilters.length > 0) { + payload.ignoredFilters = entry.ignoredFilters; + interesting = true; + } + if (entry.incompatibleFilters.length > 0) { + payload.incompatibleFilters = entry.incompatibleFilters; + interesting = true; + } + if (entry.ignoredQueryFeatures.length > 0) { + payload.ignoredQueryFeatures = entry.ignoredQueryFeatures; + interesting = true; + } + if (entry.incompatibleQueryFeatures.length > 0) { + payload.incompatibleQueryFeatures = entry.incompatibleQueryFeatures; + interesting = true; + } + if (entry.note) { + payload.note = entry.note; + interesting = true; + } + + return interesting ? payload : undefined; } function assertSearchFollowUpInvariant(hit: UnifiedSearchHit): void { @@ -331,28 +456,3 @@ function assertSearchFollowUpInvariant(hit: UnifiedSearchHit): void { ); } } - -function buildPrimaryFollowUp( - hit: UnifiedSearchHit, -): UnifiedSearchFollowUpPayload | undefined { - switch (hit.resultType) { - case "DOCUMENTATION_PAGE": - case "REPOSITORY_DOC": - return buildDocReadFollowUp(hit.locator.pageId); - case "REPOSITORY_CODE": - return buildFileReadFollowUp(hit.locator); - default: - return undefined; - } -} - -function buildAlternateFollowUps( - hit: UnifiedSearchHit, -): UnifiedSearchFollowUpPayload[] | undefined { - if (hit.resultType !== "REPOSITORY_DOC") { - return undefined; - } - - const readFile = buildFileReadFollowUp(hit.locator); - return readFile ? [readFile] : undefined; -} diff --git a/src/tools/feedback.ts b/src/tools/feedback.ts index 97c2564c..198e06f9 100644 --- a/src/tools/feedback.ts +++ b/src/tools/feedback.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import type { GitHitsService } from "../services/githits-service.js"; import { withErrorHandling } from "./shared.js"; -import { textResult, type ToolDefinition } from "./types.js"; +import { type ToolDefinition, textResult } from "./types.js"; interface FeedbackArgs { solution_id: string; @@ -27,24 +27,9 @@ const schema = { ), }; -const DESCRIPTION = `Submit feedback on a GitHits search result. +const DESCRIPTION = `Submit feedback on a GitHits example result. -Use this tool after receiving a search result to indicate whether the example was helpful. -This feedback helps improve GitHits' search quality. - -**When to use**: -- After using the search tool, provide feedback on whether the result was useful -- Use \`accepted=true\` if the example solved your problem or was helpful, and you used it -- Use \`accepted=false\` if the example was not relevant or unhelpful, and you did not use it -- Optionally provide textual feedback explaining why - -Args: - solution_id: The solution ID from a previous search result (shown in the result) - accepted: True if the example was helpful/good, False if unhelpful/bad - feedback_text: Optional text explaining why (e.g., "This solved problem X" or "Example was outdated") - -Returns: - Confirmation message or error`; +Call after \`get_example\` to record whether the returned example was used. \`accepted=true\` when it solved the problem or was useful; \`accepted=false\` when it was irrelevant or wrong. Use \`feedback_text\` to add a short reason. Feeds back into ranking quality.`; export function createFeedbackTool( service: GitHitsService, diff --git a/src/tools/get-example.ts b/src/tools/get-example.ts index 4905d587..83fc20b0 100644 --- a/src/tools/get-example.ts +++ b/src/tools/get-example.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import type { GitHitsService } from "../services/githits-service.js"; +import { extractSolutionId } from "../shared/extract-solution-id.js"; import { withErrorHandling } from "./shared.js"; import { type ToolDefinition, textResult } from "./types.js"; @@ -30,8 +31,7 @@ const schema = { const DESCRIPTION = `Get verified, canonical code examples from global open source. -Use this for example retrieval. For searching indexed dependency and repository code/docs, -use the unified \`search\` tool instead.`; +Returns JSON \`{result, solution_id?}\`. \`result\` is markdown — render or quote it directly. Pass \`solution_id\` to \`feedback\` after using or rejecting the example. For searching indexed dependency and repository code/docs, use the unified \`search\` tool instead.`; export function createGetExampleTool( service: GitHitsService, @@ -42,13 +42,17 @@ export function createGetExampleTool( schema, handler: async (args) => { return withErrorHandling("get example", async () => { - const result = await service.search({ + const markdown = await service.search({ query: args.query, language: args.language, licenseMode: args.license_mode, includeExplanation: false, }); - return textResult(result); + const solutionId = extractSolutionId(markdown); + const payload = solutionId + ? { result: markdown, solution_id: solutionId } + : { result: markdown }; + return textResult(JSON.stringify(payload)); }); }, }; diff --git a/src/tools/grep-repo.test.ts b/src/tools/grep-repo.test.ts index a0908828..cbde27bf 100644 --- a/src/tools/grep-repo.test.ts +++ b/src/tools/grep-repo.test.ts @@ -16,7 +16,7 @@ function parseText(result: { content: Array<{ text: string }> }): unknown { describe("createGrepRepoTool — metadata", () => { it("registers the correct tool name and schema keys", () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); - expect(tool.name).toBe("grep_repo"); + expect(tool.name).toBe("code_grep"); expect(tool.description).toContain("Deterministic text grep"); expect(Object.keys(tool.schema).sort()).toEqual([ "case_sensitive", @@ -123,14 +123,15 @@ describe("createGrepRepoTool — happy path", () => { ); const payload = parseText(result) as { pattern: string; - patternType: string; - caseSensitive: boolean; + patternType?: string; + caseSensitive?: boolean; totalMatches: number; matches: Array<{ filePath: string; line: number }>; }; expect(payload.pattern).toBe("middleware"); - expect(payload.patternType).toBe("literal"); - expect(payload.caseSensitive).toBe(false); + // patternType / caseSensitive omitted when default + expect(payload.patternType).toBeUndefined(); + expect(payload.caseSensitive).toBeUndefined(); expect(payload.totalMatches).toBe(1); expect(payload.matches[0]).toMatchObject({ filePath: "src/index.js", diff --git a/src/tools/grep-repo.ts b/src/tools/grep-repo.ts index 2caa5fdf..76cf19d9 100644 --- a/src/tools/grep-repo.ts +++ b/src/tools/grep-repo.ts @@ -50,13 +50,13 @@ const schema = { .string() .optional() .describe( - "Exact file path to grep. Shares the same path vocabulary as `read_file`.", + "Exact file path to grep. Shares the same path vocabulary as `code_read`.", ), path_prefix: z .string() .optional() .describe( - "Literal directory prefix to scope grep, matching `list_files` / `search` naming.", + "Literal directory prefix to scope grep, matching `code_files` / `search` naming.", ), globs: z .array(z.string()) @@ -89,13 +89,13 @@ 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 `read_file` via `matches[].filePath`."; + "Matches chain directly into `code_read` via `matches[].filePath`."; export function createGrepRepoTool( service: CodeNavigationService, ): ToolDefinition { return { - name: "grep_repo", + name: "code_grep", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/index.ts b/src/tools/index.ts index f5c6c0ea..dd11dff1 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -12,7 +12,6 @@ export { createReadPackageDocTool } from "./read-package-doc.js"; export { createSearchTool } from "./search.js"; export { createSearchLanguageTool } from "./search-language.js"; export { createSearchStatusTool } from "./search-status.js"; -export { createSearchSymbolsTool } from "./search-symbols.js"; export type { ToolDefinition, ToolHandler, diff --git a/src/tools/list-files.test.ts b/src/tools/list-files.test.ts index 5c0b1606..4f394d5a 100644 --- a/src/tools/list-files.test.ts +++ b/src/tools/list-files.test.ts @@ -16,7 +16,7 @@ function parseText(result: { content: Array<{ text: string }> }): unknown { describe("createListFilesTool — metadata", () => { it("registers the correct tool name, description, and schema keys", () => { const tool = createListFilesTool(createMockCodeNavigationService()); - expect(tool.name).toBe("list_files"); + expect(tool.name).toBe("code_files"); expect(tool.description).toContain("List files in an indexed dependency"); expect(Object.keys(tool.schema).sort()).toEqual([ "limit", diff --git a/src/tools/list-files.ts b/src/tools/list-files.ts index c570ff42..5f11f959 100644 --- a/src/tools/list-files.ts +++ b/src/tools/list-files.ts @@ -48,7 +48,7 @@ const DESCRIPTION = "`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 `read_file` and help scope `grep_repo`. Returns an `INDEXING` " + + "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`."; @@ -57,7 +57,7 @@ export function createListFilesTool( service: CodeNavigationService, ): ToolDefinition { return { - name: "list_files", + name: "code_files", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/list-package-docs.test.ts b/src/tools/list-package-docs.test.ts index f9cb29b9..a8be7dde 100644 --- a/src/tools/list-package-docs.test.ts +++ b/src/tools/list-package-docs.test.ts @@ -15,7 +15,7 @@ describe("createListPackageDocsTool", () => { const tool = createListPackageDocsTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("list_package_docs"); + expect(tool.name).toBe("docs_list"); expect(tool.annotations?.readOnlyHint).toBe(true); expect(Object.keys(tool.schema)).toEqual([ "registry", diff --git a/src/tools/list-package-docs.ts b/src/tools/list-package-docs.ts index cfb56b99..4d6c9ed0 100644 --- a/src/tools/list-package-docs.ts +++ b/src/tools/list-package-docs.ts @@ -35,14 +35,14 @@ const schema = { const DESCRIPTION = "List mixed package documentation pages from hosted docs and repository-backed docs. " + - "Every entry includes a stable pageId, source kind, source URL, and for repo docs exact file follow-up metadata. " + - "Use this when you need to browse what docs exist before reading a full page."; + "Every entry includes a stable `pageId`, `sourceKind` (`crawled` or `repo`), and source URL; repo-backed entries also expose `repoUrl` / `gitRef` / `filePath` for exact file reads. " + + "Pass a returned `pageId` to `docs_read`. Use this to browse before reading a full page."; export function createListPackageDocsTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "list_package_docs", + name: "docs_list", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/package-changelog.test.ts b/src/tools/package-changelog.test.ts index 6b50a94d..10476db6 100644 --- a/src/tools/package-changelog.test.ts +++ b/src/tools/package-changelog.test.ts @@ -18,7 +18,7 @@ describe("createPackageChangelogTool — metadata", () => { const tool = createPackageChangelogTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("package_changelog"); + expect(tool.name).toBe("pkg_changelog"); expect(tool.description).toContain("latest mode"); expect(tool.description).toContain("range mode"); expect(Object.keys(tool.schema).sort()).toEqual([ diff --git a/src/tools/package-changelog.ts b/src/tools/package-changelog.ts index c5a039ca..ee71df10 100644 --- a/src/tools/package-changelog.ts +++ b/src/tools/package-changelog.ts @@ -101,7 +101,7 @@ export function createPackageChangelogTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "package_changelog", + name: "pkg_changelog", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/package-dependencies-parity.test.ts b/src/tools/package-dependencies-parity.test.ts index 038076fd..813b928e 100644 --- a/src/tools/package-dependencies-parity.test.ts +++ b/src/tools/package-dependencies-parity.test.ts @@ -5,8 +5,7 @@ // details? } on every error path; MCP error text is // always valid JSON. // -// Assertion policy (matches shipped search_symbols / package_summary / -// package_vulnerabilities precedent): +// Assertion policy: // - Service-sourced success and error fixtures use `toEqual`: both // surfaces route through the same request builder and envelope // shaper, so envelopes are byte-identical. diff --git a/src/tools/package-dependencies.test.ts b/src/tools/package-dependencies.test.ts index 70dbb0a4..e49b879f 100644 --- a/src/tools/package-dependencies.test.ts +++ b/src/tools/package-dependencies.test.ts @@ -15,7 +15,7 @@ describe("createPackageDependenciesTool — metadata", () => { const tool = createPackageDependenciesTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("package_dependencies"); + expect(tool.name).toBe("pkg_deps"); expect(tool.description).toContain("npm, PyPI, Hex, Crates"); expect(Object.keys(tool.schema).sort()).toEqual([ "include_importers", diff --git a/src/tools/package-dependencies.ts b/src/tools/package-dependencies.ts index b63473d8..0a932ba6 100644 --- a/src/tools/package-dependencies.ts +++ b/src/tools/package-dependencies.ts @@ -19,9 +19,7 @@ export interface PackageDependenciesArgs { /** * Permissive schema — in-handler validation via * `buildPackageDependenciesParams` is the single validation path so - * raw Zod errors never surface to agents. Matches the shipped - * `search_symbols` / `package_summary` / `package_vulnerabilities` - * pattern. + * raw Zod errors never surface to agents. * * No `include_groups` input. The data-first envelope emits the * `groups` block unconditionally when the backend returned @@ -93,7 +91,7 @@ export function createPackageDependenciesTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "package_dependencies", + name: "pkg_deps", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/package-summary-parity.test.ts b/src/tools/package-summary-parity.test.ts index 69731a09..65bc6691 100644 --- a/src/tools/package-summary-parity.test.ts +++ b/src/tools/package-summary-parity.test.ts @@ -5,7 +5,7 @@ // on every error path; MCP error text is always valid // JSON. // -// Assertion policy (locked to match shipped `search_symbols` precedent): +// Assertion policy: // - Success fixtures + service-sourced error fixtures → `toEqual`. // Both surfaces route through the same classifier / envelope builder, // so envelopes are byte-identical. diff --git a/src/tools/package-summary.test.ts b/src/tools/package-summary.test.ts index e6feabd0..9bfd66ff 100644 --- a/src/tools/package-summary.test.ts +++ b/src/tools/package-summary.test.ts @@ -15,7 +15,7 @@ describe("createPackageSummaryTool — metadata", () => { const tool = createPackageSummaryTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("package_summary"); + expect(tool.name).toBe("pkg_info"); expect(tool.description).toContain("package overview"); expect(Object.keys(tool.schema)).toEqual(["registry", "package_name"]); expect(tool.annotations?.readOnlyHint).toBe(true); diff --git a/src/tools/package-summary.ts b/src/tools/package-summary.ts index db696293..5b72ff47 100644 --- a/src/tools/package-summary.ts +++ b/src/tools/package-summary.ts @@ -15,7 +15,7 @@ export interface PackageSummaryArgs { * via `buildPackageSummaryParams`. That way, malformed input produces * the structured `{error, code, retryable}` envelope (same as CLI), * rather than a raw Zod error that agents would have to parse - * separately. See `search_symbols` precedent. + * separately. */ const schema = { registry: z @@ -41,7 +41,7 @@ export function createPackageSummaryTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "package_summary", + name: "pkg_info", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/package-vulnerabilities-parity.test.ts b/src/tools/package-vulnerabilities-parity.test.ts index 67a52cac..303b5db7 100644 --- a/src/tools/package-vulnerabilities-parity.test.ts +++ b/src/tools/package-vulnerabilities-parity.test.ts @@ -5,8 +5,7 @@ // details? } on every error path; MCP error text is // always valid JSON. // -// Assertion policy (matches shipped search_symbols / package_summary -// precedent): +// Assertion policy: // - Service-sourced success and error fixtures use `toEqual`: both // surfaces route through the same classifier / envelope builder, // so envelopes are byte-identical. diff --git a/src/tools/package-vulnerabilities.test.ts b/src/tools/package-vulnerabilities.test.ts index ac722e73..8ec79a81 100644 --- a/src/tools/package-vulnerabilities.test.ts +++ b/src/tools/package-vulnerabilities.test.ts @@ -15,7 +15,7 @@ describe("createPackageVulnerabilitiesTool — metadata", () => { const tool = createPackageVulnerabilitiesTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("package_vulnerabilities"); + expect(tool.name).toBe("pkg_vulns"); expect(tool.description).toContain("npm, PyPI, Hex, or"); expect(Object.keys(tool.schema).sort()).toEqual([ "include_withdrawn", diff --git a/src/tools/package-vulnerabilities.ts b/src/tools/package-vulnerabilities.ts index 82db6a8a..742f7b45 100644 --- a/src/tools/package-vulnerabilities.ts +++ b/src/tools/package-vulnerabilities.ts @@ -16,8 +16,8 @@ export interface PackageVulnerabilitiesArgs { /** * Permissive schema by design — in-handler validation via * `buildPackageVulnerabilitiesParams` is the single validation path - * for both CLI and MCP. Matches shipped `search_symbols` / - * `package_summary` pattern: raw Zod errors never surface to agents. + * for both CLI and MCP. Raw Zod errors never surface to agents; the + * structured `{error, code, retryable}` envelope is returned instead. */ const schema = { registry: z @@ -59,7 +59,7 @@ export function createPackageVulnerabilitiesTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "package_vulnerabilities", + name: "pkg_vulns", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/read-file.test.ts b/src/tools/read-file.test.ts index 160b8535..4ec866cb 100644 --- a/src/tools/read-file.test.ts +++ b/src/tools/read-file.test.ts @@ -16,7 +16,7 @@ function parseText(result: { content: Array<{ text: string }> }): unknown { describe("createReadFileTool — metadata", () => { it("registers the correct tool name, description, and schema keys", () => { const tool = createReadFileTool(createMockCodeNavigationService()); - expect(tool.name).toBe("read_file"); + expect(tool.name).toBe("code_read"); expect(tool.description).toContain( "Read a file from an indexed dependency", ); diff --git a/src/tools/read-file.ts b/src/tools/read-file.ts index 53955fe4..3fe1cd11 100644 --- a/src/tools/read-file.ts +++ b/src/tools/read-file.ts @@ -24,7 +24,7 @@ const schema = { path: z .string() .describe( - "Path to the file. Package addressing: package-relative. Repo addressing: repo-relative. This is the same `path` key that `list_files` emits for each entry, so the `list_files` → `read_file` chain needs no renaming.", + "Path to the file. Package addressing: package-relative. Repo addressing: repo-relative. This is the same `path` key that `code_files` emits for each entry, so chaining needs no renaming.", ), start_line: z .number() @@ -50,20 +50,19 @@ const DESCRIPTION = "`{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 `list_files`. Address via " + + "`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` " + - "(note: `fetchCodeContext` doesn't emit `availableVersions` in " + - "details, only `indexingRef`). When the path doesn't resolve the " + - "response is a `NOT_FOUND` (or `FILE_NOT_FOUND`) error — call " + - "`list_files` to discover the actual paths."; + "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."; export function createReadFileTool( service: CodeNavigationService, ): ToolDefinition { return { - name: "read_file", + name: "code_read", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, diff --git a/src/tools/read-package-doc.test.ts b/src/tools/read-package-doc.test.ts index bebc5106..5c6d3783 100644 --- a/src/tools/read-package-doc.test.ts +++ b/src/tools/read-package-doc.test.ts @@ -12,9 +12,13 @@ describe("createReadPackageDocTool", () => { const tool = createReadPackageDocTool( createMockPackageIntelligenceService(), ); - expect(tool.name).toBe("read_package_doc"); + expect(tool.name).toBe("docs_read"); expect(tool.annotations?.readOnlyHint).toBe(true); - expect(Object.keys(tool.schema)).toEqual(["page_id"]); + expect(Object.keys(tool.schema)).toEqual([ + "page_id", + "start_line", + "end_line", + ]); }); it("calls service.readPackageDoc with the page ID", async () => { @@ -38,7 +42,6 @@ describe("createReadPackageDocTool", () => { ); const payload = parseText(result) as Record; expect(payload.pageId).toBe("github:expressjs/express@abc123/README.md"); - expect(payload.followUp).toBeDefined(); }); it("returns INVALID_ARGUMENT for empty page ID", async () => { diff --git a/src/tools/read-package-doc.ts b/src/tools/read-package-doc.ts index 9e8b92e6..f772f2f2 100644 --- a/src/tools/read-package-doc.ts +++ b/src/tools/read-package-doc.ts @@ -7,25 +7,40 @@ import { errorResult, type ToolDefinition, textResult } from "./types.js"; export interface ReadPackageDocArgs { page_id: string; + start_line?: number; + end_line?: number; } const schema = { page_id: z .string() .describe( - "Documentation page ID from list_package_docs or search results. Pass through unchanged; repo-backed IDs are snapshot-pinned.", + "Documentation page ID from `docs_list` or `search` results. Pass through unchanged; repo-backed IDs are snapshot-pinned.", + ), + start_line: z + .number() + .optional() + .describe( + "Starting line (1-indexed). Omit for the full page. Use with `end_line` to bound how much content the tool returns when a page is large.", + ), + end_line: z + .number() + .optional() + .describe( + "Ending line (inclusive). Omit for end of page. Must be ≥ `start_line` when both are set.", ), }; const DESCRIPTION = "Read a documentation page by page ID. Works for both hosted/crawled docs and repository-backed docs. " + - "Repo-backed results additionally include exact file follow-up metadata for read_file."; + "Pass `start_line` / `end_line` to fetch only a slice when a page is too long — response carries `totalLines` so you can target the next slice. " + + "Repo-backed results additionally include exact file follow-up metadata for `code_read`."; export function createReadPackageDocTool( service: PackageIntelligenceService, ): ToolDefinition { return { - name: "read_package_doc", + name: "docs_read", description: DESCRIPTION, schema, annotations: { readOnlyHint: true }, @@ -33,9 +48,14 @@ export function createReadPackageDocTool( try { const build = buildReadPackageDocParams({ pageId: args.page_id }); const result = await service.readPackageDoc(build.params); + const range = + args.start_line !== undefined || args.end_line !== undefined + ? { startLine: args.start_line, endLine: args.end_line } + : undefined; const payload = buildReadPackageDocSuccessPayload( result, build.params.pageId, + range, ); return textResult(JSON.stringify(payload)); } catch (error) { diff --git a/src/tools/search-language.ts b/src/tools/search-language.ts index 794ccf1c..0162caa5 100644 --- a/src/tools/search-language.ts +++ b/src/tools/search-language.ts @@ -17,16 +17,7 @@ const schema = { ), }; -const DESCRIPTION = `Search for a programming language supported by GitHits. - -Use this tool to find the correct language name before calling the search tool. -Returns up to 5 matching languages. - -Args: - query: Language name or partial name to search for (e.g., "python", "type", "java") - -Returns: - List of matching languages with name and display_name`; +const DESCRIPTION = `Find the correct language name for \`get_example\` when it is uncertain. Returns up to 5 matching languages by name, display name, or alias.`; export function createSearchLanguageTool( service: GitHitsService, @@ -39,7 +30,7 @@ export function createSearchLanguageTool( return withErrorHandling("search languages", async () => { const allLanguages = await service.getLanguages(); const result = filterLanguages(allLanguages, args.query); - return textResult(JSON.stringify(result, null, 2)); + return textResult(JSON.stringify(result)); }); }, }; diff --git a/src/tools/search-status.test.ts b/src/tools/search-status.test.ts index 05d5ab9d..c4e27a0d 100644 --- a/src/tools/search-status.test.ts +++ b/src/tools/search-status.test.ts @@ -71,8 +71,7 @@ describe("searchStatusTool", () => { expect(result.isError).toBeUndefined(); expect(payload.completed).toBe(true); expect(payload.searchRef).toBe(defaultUnifiedSearchOutcome.searchRef); - expect(payload.result.query).toBe("router middleware"); - expect(payload.result.returnedCount).toBe(1); + expect(payload.result.results).toHaveLength(1); expect(payload).not.toHaveProperty("query"); }); diff --git a/src/tools/search-symbols-parity.test.ts b/src/tools/search-symbols-parity.test.ts deleted file mode 100644 index 81692f4e..00000000 --- a/src/tools/search-symbols-parity.test.ts +++ /dev/null @@ -1,239 +0,0 @@ -// PARITY TEST — enforces rule IDs from docs/implementation/mcp-cli-parity.md: -// PARITY-JSON-KEYS CLI --json output and MCP text payload parse to -// deepEqual JSON objects for equivalent inputs. -// PARITY-ERROR-ENVELOPE Both surfaces emit { error, code, details? } on -// every error path; MCP error text is always valid -// JSON. -// -// Rule IDs are stable; changes to either this test or the parity doc -// are coordinated (renaming a rule ID here without updating the doc, -// or vice versa, should fail review). - -import { describe, expect, it, mock, spyOn } from "bun:test"; -import { - type SearchSymbolsCommandDependencies, - searchSymbolsAction, -} from "../commands/code/search-symbols.js"; -import { - CodeNavigationIndexingError, - CodeNavigationTargetNotFoundError, -} from "../services/index.js"; -import { - createMockCodeNavigationService, - defaultSearchSymbolsResult, -} from "../services/test-helpers.js"; -import { createSearchSymbolsTool } from "./search-symbols.js"; - -function cliDeps( - overrides: Partial = {}, -): SearchSymbolsCommandDependencies { - return { - codeNavigationService: createMockCodeNavigationService(), - codeNavigationUrl: "https://nav.example.com", - hasValidToken: true, - mcpUrl: "https://mcp.example.com", - ...overrides, - }; -} - -async function cliJson( - packageArg: string, - query: string | undefined, - options: Parameters[2] = {}, - deps: SearchSymbolsCommandDependencies = cliDeps(), -): Promise { - const logSpy = spyOn(console, "log").mockImplementation(() => {}); - const errSpy = spyOn(console, "error").mockImplementation(() => {}); - const exitSpy = spyOn(process, "exit").mockImplementation(() => { - throw new Error("process.exit"); - }); - try { - try { - await searchSymbolsAction( - packageArg, - query, - { ...options, json: true }, - deps, - ); - } catch { - // CLI error paths call process.exit — caught. - } - const fromLog = logSpy.mock.calls[0]?.[0] as string | undefined; - const fromErr = errSpy.mock.calls[0]?.[0] as string | undefined; - const raw = fromLog ?? fromErr; - return raw ? JSON.parse(raw) : undefined; - } finally { - logSpy.mockRestore(); - errSpy.mockRestore(); - exitSpy.mockRestore(); - } -} - -async function mcpJson( - args: Parameters["handler"]>[0], - searchMock?: () => Promise, -): Promise { - const service = createMockCodeNavigationService( - searchMock ? { searchSymbols: mock(searchMock) as never } : {}, - ); - const tool = createSearchSymbolsTool(service); - const result = await tool.handler(args, {}); - const text = result.content[0]?.text ?? ""; - return JSON.parse(text); -} - -describe("search_symbols CLI ↔ MCP JSON parity", () => { - it("PARITY-JSON-KEYS: successful search produces the same envelope", async () => { - const cliPayload = await cliJson("npm:express", "middleware"); - const mcpPayload = await mcpJson({ - target: { registry: "npm", package_name: "express" }, - query: "middleware", - }); - - expect(cliPayload).toEqual(mcpPayload); - }); - - it("PARITY-JSON-KEYS: zero-result search omits the `hint` key when server hint is empty", async () => { - const emptyResult = { results: [], totalMatches: 0, hasMore: false }; - const cliPayload = await cliJson( - "npm:express", - "nonexistent-token", - {}, - cliDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => Promise.resolve(emptyResult)), - }), - }), - ); - const mcpPayload = await mcpJson( - { - target: { registry: "npm", package_name: "express" }, - query: "nonexistent-token", - }, - () => Promise.resolve(emptyResult), - ); - - expect(cliPayload).toEqual(mcpPayload); - expect((cliPayload as Record).hint).toBeUndefined(); - }); - - it("PARITY-ERROR-ENVELOPE: NOT_FOUND emits the same envelope on both surfaces", async () => { - const err = new CodeNavigationTargetNotFoundError("Package not found", [ - { version: "5.2.1", ref: "v5.2.1" }, - ]); - const cliPayload = await cliJson( - "npm:nonexistent", - "middleware", - {}, - cliDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => Promise.reject(err)), - }), - }), - ); - const mcpPayload = await mcpJson( - { - target: { registry: "npm", package_name: "nonexistent" }, - query: "middleware", - }, - () => Promise.reject(err), - ); - - expect(cliPayload).toEqual(mcpPayload); - expect(cliPayload).toEqual({ - error: "Package not found", - code: "NOT_FOUND", - retryable: false, - details: { availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }] }, - }); - }); - - it("PARITY-ERROR-ENVELOPE: INVALID_ARGUMENT produces a valid-JSON envelope on both surfaces", async () => { - // CLI: unknown-registry prefix rejected by the package-spec parser. - const cliPayload = await cliJson("foobar:baz", "middleware"); - expect(cliPayload).toMatchObject({ - code: "INVALID_ARGUMENT", - error: expect.stringContaining("Unsupported registry"), - }); - - // MCP: resolveCodeTarget rejects an invalid target shape and emits - // the same envelope rather than a free-form error string. - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - const result = await tool.handler({ target: {}, query: "middleware" }, {}); - expect(result.isError).toBe(true); - const mcpPayload = JSON.parse(result.content[0]?.text ?? ""); - expect(mcpPayload).toMatchObject({ - code: "INVALID_ARGUMENT", - error: expect.stringContaining("Missing target"), - }); - }); - - it("PARITY-ERROR-ENVELOPE: INDEXING errors carry identical details", async () => { - const err = new CodeNavigationIndexingError( - "Target is being indexed.", - "idx-42", - ); - const cliPayload = await cliJson( - "npm:express", - "middleware", - {}, - cliDeps({ - codeNavigationService: createMockCodeNavigationService({ - searchSymbols: mock(() => Promise.reject(err)), - }), - }), - ); - const mcpPayload = await mcpJson( - { - target: { registry: "npm", package_name: "express" }, - query: "middleware", - }, - () => Promise.reject(err), - ); - - expect(cliPayload).toEqual(mcpPayload); - expect(cliPayload).toEqual({ - error: "Target is being indexed.", - code: "INDEXING", - retryable: true, - details: { indexingRef: "idx-42" }, - }); - }); - - it("PARITY-JSON-KEYS: query.defaulted lists the client-applied fields on both surfaces", async () => { - const cliPayload = (await cliJson("npm:express", "middleware")) as { - query: { defaulted: string[] }; - }; - const mcpPayload = (await mcpJson({ - target: { registry: "npm", package_name: "express" }, - query: "middleware", - })) as { query: { defaulted: string[] } }; - - expect(cliPayload.query.defaulted).toEqual(mcpPayload.query.defaulted); - expect(cliPayload.query.defaulted).not.toContain("fileIntent"); - expect(cliPayload.query.defaulted).toContain("waitTimeoutMs"); - }); - - it("PARITY-JSON-KEYS: no leading-underscore keys in success or default payloads", async () => { - const payload = (await cliJson("npm:express", "middleware")) as Record< - string, - unknown - >; - - const assertNoUnderscoreKeys = (value: unknown, path = "") => { - if (value === null || typeof value !== "object") return; - for (const key of Object.keys(value as Record)) { - expect(key).not.toMatch(/^_/); - assertNoUnderscoreKeys( - (value as Record)[key], - `${path}.${key}`, - ); - } - }; - - assertNoUnderscoreKeys(payload); - // And the fixture definitely exercises at least one success field - // so the assertion is not vacuous. - expect(payload.results).toEqual(defaultSearchSymbolsResult.results); - }); -}); diff --git a/src/tools/search-symbols.test.ts b/src/tools/search-symbols.test.ts deleted file mode 100644 index 0b8eb08a..00000000 --- a/src/tools/search-symbols.test.ts +++ /dev/null @@ -1,211 +0,0 @@ -import { describe, expect, it, mock } from "bun:test"; -import { CodeNavigationIndexingError } from "../services/index.js"; -import { - createMockCodeNavigationService, - defaultSearchSymbolsResult, -} from "../services/test-helpers.js"; -import { createSearchSymbolsTool } from "./search-symbols.js"; - -describe("searchSymbolsTool", () => { - it("returns tool metadata", () => { - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - expect(tool.name).toBe("search_symbols"); - expect(tool.description).toContain("exact-token matches"); - expect(tool.description).toContain("across all intents"); - expect(tool.description).toContain("`all`"); - // Category is the preferred filtering surface per the April 2026 - // backend taxonomy split. - expect(tool.description).toContain("category"); - }); - - it("calls service with normalized target and search params", async () => { - const searchSymbols = mock(() => - Promise.resolve({ results: [], totalMatches: 0, hasMore: false }), - ); - const tool = createSearchSymbolsTool( - createMockCodeNavigationService({ searchSymbols }), - ); - - await tool.handler( - { - target: { registry: "npm", package_name: "express", version: "4.18.0" }, - query: "middleware", - kind: "function", - limit: 10, - wait_timeout_ms: 5000, - }, - {}, - ); - - expect(searchSymbols).toHaveBeenCalledWith({ - target: { registry: "NPM", packageName: "express", version: "4.18.0" }, - query: "middleware", - keywords: undefined, - matchMode: undefined, - kind: "FUNCTION", - filePath: undefined, - limit: 10, - fileIntent: undefined, - waitTimeoutMs: 5000, - }); - }); - - it("returns the shared success envelope on success", async () => { - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - - const result = await tool.handler( - { - target: { registry: "npm", package_name: "express" }, - query: "middleware", - }, - {}, - ); - - expect(result.isError).toBeUndefined(); - const payload = JSON.parse(result.content[0]?.text ?? ""); - expect(payload.results).toEqual(defaultSearchSymbolsResult.results); - expect(payload.returnedCount).toBe(1); - expect(payload.totalMatches).toBe(1); - expect(payload.hasMore).toBe(false); - expect(payload.version).toBe("4.18.0"); - expect(payload.query.target).toEqual({ - registry: "NPM", - packageName: "express", - version: undefined, - }); - expect(payload.query.query).toBe("middleware"); - expect(payload.query.fileIntent).toBe("all"); - expect(payload.query.defaulted).not.toContain("fileIntent"); - expect(payload.query.defaulted).toContain("waitTimeoutMs"); - // No underscore-prefixed keys in the shared envelope. - expect(payload._warning).toBeUndefined(); - expect(payload._hint).toBeUndefined(); - }); - - it("emits the shared error envelope on indexing errors", async () => { - const tool = createSearchSymbolsTool( - createMockCodeNavigationService({ - searchSymbols: mock(() => - Promise.reject( - new CodeNavigationIndexingError( - "Target is being indexed.", - "idx-123", - [{ version: "4.18.0", ref: "v4.18.0" }], - ), - ), - ), - }), - ); - - const result = await tool.handler( - { - target: { registry: "npm", package_name: "express" }, - query: "middleware", - }, - {}, - ); - - expect(result.isError).toBe(true); - expect(JSON.parse(result.content[0]?.text ?? "")).toEqual({ - error: "Target is being indexed.", - code: "INDEXING", - retryable: true, - details: { - indexingRef: "idx-123", - availableVersions: [{ version: "4.18.0", ref: "v4.18.0" }], - }, - }); - }); - - it("emits a valid-JSON error envelope when query and keywords are missing", async () => { - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - const result = await tool.handler( - { target: { registry: "npm", package_name: "express" } }, - {}, - ); - - expect(result.isError).toBe(true); - expect(JSON.parse(result.content[0]?.text ?? "")).toEqual({ - error: "Provide either query or keywords.", - code: "INVALID_ARGUMENT", - }); - }); - - it("treats file_intent 'all' as the FILE_INTENT_ALL sentinel and echoes 'all'", async () => { - const searchSymbols = mock< - ( - params: import("../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const tool = createSearchSymbolsTool( - createMockCodeNavigationService({ searchSymbols }), - ); - const result = await tool.handler( - { - target: { registry: "npm", package_name: "express" }, - query: "middleware", - file_intent: "all", - }, - {}, - ); - - // Service receives undefined fileIntent — omit the GraphQL variable. - expect(searchSymbols.mock.calls[0]?.[0]?.fileIntent).toBeUndefined(); - const payload = JSON.parse(result.content[0]?.text ?? ""); - expect(payload.query.fileIntent).toBe("all"); - expect(payload.query.defaulted).not.toContain("fileIntent"); - }); - - it("echoes an explicit non-default file_intent without marking it as defaulted", async () => { - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - const result = await tool.handler( - { - target: { registry: "npm", package_name: "express" }, - query: "middleware", - file_intent: "test", - }, - {}, - ); - - const payload = JSON.parse(result.content[0]?.text ?? ""); - expect(payload.query.fileIntent).toBe("test"); - expect(payload.query.defaulted).not.toContain("fileIntent"); - }); - - it("rejects `mode` and `verbose` as unknown inputs (removed from schema)", () => { - const tool = createSearchSymbolsTool(createMockCodeNavigationService()); - // The compile-time SearchSymbolsArgs no longer declares these keys. - // At runtime the Zod schema also omits them; a defensive runtime - // assertion keeps us honest: - const schemaKeys = Object.keys(tool.schema); - expect(schemaKeys).not.toContain("mode"); - expect(schemaKeys).not.toContain("verbose"); - }); - - it("passes category and kind through to the service and echoes category lowercase", async () => { - const searchSymbols = mock< - ( - params: import("../services/index.js").SearchSymbolsParams, - ) => Promise - >(() => Promise.resolve({ results: [], totalMatches: 0, hasMore: false })); - const tool = createSearchSymbolsTool( - createMockCodeNavigationService({ searchSymbols }), - ); - - const result = await tool.handler( - { - target: { registry: "npm", package_name: "express" }, - query: "Router", - category: "callable", - kind: "trait", - }, - {}, - ); - - expect(searchSymbols.mock.calls[0]?.[0]?.category).toBe("CALLABLE"); - expect(searchSymbols.mock.calls[0]?.[0]?.kind).toBe("TRAIT"); - const payload = JSON.parse(result.content[0]?.text ?? ""); - expect(payload.query.category).toBe("callable"); - expect(payload.query.kind).toBe("trait"); - }); -}); diff --git a/src/tools/search-symbols.ts b/src/tools/search-symbols.ts deleted file mode 100644 index 75893aaf..00000000 --- a/src/tools/search-symbols.ts +++ /dev/null @@ -1,237 +0,0 @@ -import { z } from "zod"; -import type { CodeNavigationService } from "../services/index.js"; -import { - toSearchSymbolsFileIntent, - toSearchSymbolsKind, - toSearchSymbolsMatchMode, - toSymbolCategory, -} from "../shared/code-navigation.js"; -import { FILE_INTENT_ALL } from "../shared/code-navigation-defaults.js"; -import { buildSearchSymbolsParams } from "../shared/search-symbols-request.js"; -import { - buildSearchSymbolsErrorPayload, - buildSearchSymbolsSuccessPayload, -} from "../shared/search-symbols-response.js"; -import { - type CodeTargetArg, - codeTargetSchema, - resolveCodeTarget, -} from "./code-navigation-shared.js"; -import { errorResult, type ToolDefinition, textResult } from "./types.js"; - -export interface SearchSymbolsArgs { - target: CodeTargetArg; - query?: string; - keywords?: string[]; - match_mode?: "or" | "and"; - category?: "callable" | "type" | "module" | "data" | "documentation"; - kind?: - | "function" - | "method" - | "constructor" - | "getter" - | "setter" - | "operator" - | "class" - | "interface" - | "trait" - | "struct" - | "enum" - | "record" - | "protocol" - | "extension" - | "delegate" - | "mixin" - | "actor" - | "annotation" - | "type" - | "module" - | "namespace" - | "package" - | "object" - | "field" - | "property" - | "event" - | "constant" - | "doc_section"; - file_path?: string; - limit?: number; - file_intent?: - | "production" - | "test" - | "benchmark" - | "example" - | "generated" - | "fixture" - | "build" - | "vendor" - | "all"; - wait_timeout_ms?: number; -} - -const schema = { - target: codeTargetSchema, - query: z - .string() - .max(500) - .optional() - .describe( - "Search keywords - exact source tokens, not natural language. Required if keywords not provided.", - ), - keywords: z - .array(z.string()) - .max(20) - .optional() - .describe( - "Search keywords (max 20). Combined using match_mode. Can be used alone or together with query.", - ), - match_mode: z - .enum(["or", "and"]) - .optional() - .describe("How to combine keywords: or (any match) or and (all match)"), - category: z - .enum(["callable", "type", "module", "data", "documentation"]) - .optional() - .describe( - "Broad symbol category filter — the preferred surface for filtering. callable (function/method/constructor/getter/setter/operator), type (class/interface/trait/struct/enum/record/protocol/extension/etc.), module (module/namespace/package/object), data (field/property/event/constant), documentation (doc_section).", - ), - kind: z - .enum([ - "function", - "method", - "constructor", - "getter", - "setter", - "operator", - "class", - "interface", - "trait", - "struct", - "enum", - "record", - "protocol", - "extension", - "delegate", - "mixin", - "actor", - "annotation", - "type", - "module", - "namespace", - "package", - "object", - "field", - "property", - "event", - "constant", - "doc_section", - ]) - .optional() - .describe( - "Precise symbol kind filter. Prefer `category` for broad filtering; use `kind` only when you need a specific construct (e.g. trait vs interface).", - ), - file_path: z - .string() - .optional() - .describe( - "Filter results to files whose path starts with this value (e.g., 'src/' for a directory)", - ), - limit: z.coerce - .number() - .int() - .min(1) - .max(50) - .optional() - .describe("Max results to return (max 50)"), - file_intent: z - .enum([ - "production", - "test", - "benchmark", - "example", - "generated", - "fixture", - "build", - "vendor", - "all", - ]) - .optional() - .describe( - "Optional file intent filter. Omit it to search across all intents. `all` remains accepted as an explicit no-filter alias.", - ), - wait_timeout_ms: z.coerce - .number() - .int() - .min(0) - .max(60000) - .optional() - .describe( - "Max milliseconds to wait for indexing before returning. Defaults to 20000 (20 seconds) to cover typical indexing (p50 ~11s). On an INDEXING response, retry with wait_timeout_ms up to 60000 to block until ready.", - ), -}; - -const DESCRIPTION = - "Search a dependency's source code by exact-token matches. Use for symbol lookup, not natural-language questions. " + - 'Prefer unified `search` with `sources:["symbol"]` for new symbol-shaped workflows; use `search_symbols` when you specifically need this dedicated legacy contract. ' + - "Package scope: pass target.registry + target.package_name. Repo scope: pass target.repo_url + target.git_ref. " + - "Omit `file_intent` to search across all intents; `all` remains accepted as an explicit no-filter alias. " + - "`query` and `keywords` can combine; `match_mode` controls AND vs OR across keywords. " + - "Filter by `category` (broad: callable/type/module/data/documentation — preferred) or `kind` (precise construct like trait/record/namespace). " + - "Prefer `file_path` to scope to a directory (e.g., 'src/'). " + - "Responses include each match's source code, line range, and unified `kind`/`category` classification. " + - "If the response is an INDEXING error, the package is being indexed on-demand — retry the same call with `wait_timeout_ms: 60000` to block until ready."; - -export function createSearchSymbolsTool( - service: CodeNavigationService, -): ToolDefinition { - return { - name: "search_symbols", - description: DESCRIPTION, - schema, - annotations: { readOnlyHint: true }, - handler: async (args) => { - const target = resolveCodeTarget(args.target); - if ("content" in target) return target; - - if (!args.query && (!args.keywords || args.keywords.length === 0)) { - // Emit a valid-JSON error payload so MCP clients can always - // parse `content[0].text` (PARITY-ERROR-ENVELOPE). - return errorResult( - JSON.stringify({ - error: "Provide either query or keywords.", - code: "INVALID_ARGUMENT", - }), - ); - } - - try { - const { params, defaulted } = buildSearchSymbolsParams({ - target, - query: args.query, - keywords: args.keywords, - matchMode: toSearchSymbolsMatchMode(args.match_mode), - kind: toSearchSymbolsKind(args.kind), - category: toSymbolCategory(args.category), - filePath: args.file_path, - limit: args.limit, - fileIntent: - args.file_intent === "all" - ? FILE_INTENT_ALL - : toSearchSymbolsFileIntent(args.file_intent), - waitTimeoutMs: args.wait_timeout_ms, - }); - const result = await service.searchSymbols(params); - const payload = buildSearchSymbolsSuccessPayload( - params, - defaulted, - result, - ); - return textResult(JSON.stringify(payload)); - } catch (error) { - return errorResult( - JSON.stringify(buildSearchSymbolsErrorPayload(error)), - ); - } - }, - }; -} diff --git a/src/tools/search.test.ts b/src/tools/search.test.ts index 12a28b0a..337e3e3a 100644 --- a/src/tools/search.test.ts +++ b/src/tools/search.test.ts @@ -83,7 +83,7 @@ describe("searchTool", () => { ); }); - it("includes alternate read_file follow-up for repository docs", async () => { + it("preserves locator fields on repository_doc hits so agents can call read_package_doc or read_file", async () => { if (defaultUnifiedSearchOutcome.state !== "completed") { throw new Error("expected completed outcome fixture"); } @@ -128,17 +128,14 @@ describe("searchTool", () => { ); const payload = JSON.parse(result.content[0]?.text ?? "{}"); - expect(payload.results[0].followUp).toEqual({ - type: "read_doc", + expect(payload.results[0].type).toBe("repository_doc"); + expect(payload.results[0].locator).toMatchObject({ pageId: "github:expressjs/express@abc123/README.md", + repoUrl: "https://github.com/expressjs/express", + gitRef: "abc123", + filePath: "README.md", }); - expect(payload.results[0].alternateFollowUps).toEqual([ - { - type: "read_file", - repoUrl: "https://github.com/expressjs/express", - gitRef: "abc123", - path: "README.md", - }, - ]); + expect(payload.results[0]).not.toHaveProperty("followUp"); + expect(payload.results[0]).not.toHaveProperty("alternateFollowUps"); }); }); diff --git a/src/tools/search.ts b/src/tools/search.ts index 0a434860..70c0af31 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -4,9 +4,9 @@ import { buildUnifiedSearchErrorPayload, buildUnifiedSearchParams, buildUnifiedSearchSuccessPayload, - toSearchSymbolsFileIntent, - toSearchSymbolsKind, + toFileIntent, toSymbolCategory, + toSymbolKind, } from "../shared/index.js"; import { type CodeTargetArg, @@ -154,11 +154,12 @@ const schema = { const DESCRIPTION = "Search indexed dependency and repository code, docs, and explicit symbols. " + + "Provide either `target` for one target or `targets` for many; omit `sources` to use backend AUTO. " + "The query field uses GitHits discovery syntax (AND/OR/parens/qualifiers; see the parameter description). " + "Structured parameters combine with that query using AND semantics. " + - "Provide either `target` for one target or `targets` for many. Omit `sources` to use backend AUTO. " + - "Results are complete by default; set `allow_partial_results: true` to include available hits while indexing continues. " + - "Use `search_status` with that ref to continue."; + "Results are complete by default — if indexing is still running, the response carries a `searchRef` and no hits; pass it to `search_status` to follow up. " + + "Set `allow_partial_results: true` to opt into hits from sources that finished while others continue indexing. " + + "Each hit's `type` tells you the follow-up tool: `documentation_page` and `repository_doc` → `docs_read` with `locator.pageId`; `repository_code` and `repository_symbol` → `code_read` with `locator.filePath` (and `locator.startLine`/`endLine` when present)."; export function createSearchTool( service: CodeNavigationService, @@ -196,10 +197,10 @@ export function createSearchTool( sources: args.sources?.map( (entry) => entry.toUpperCase() as "DOCS" | "CODE" | "SYMBOL", ), - kind: toSearchSymbolsKind(args.kind), + kind: toSymbolKind(args.kind), category: toSymbolCategory(args.category), pathPrefix: args.path_prefix, - fileIntent: toSearchSymbolsFileIntent(args.file_intent), + fileIntent: toFileIntent(args.file_intent), publicOnly: args.public_only, name: args.name, language: args.language, @@ -214,7 +215,6 @@ export function createSearchTool( built.params, built.rawQuery, built.compiledQuery, - built.defaulted, outcome, ); return textResult(JSON.stringify(payload)); diff --git a/src/tools/shared.ts b/src/tools/shared.ts index 3da69527..ed4ae45f 100644 --- a/src/tools/shared.ts +++ b/src/tools/shared.ts @@ -1,7 +1,11 @@ +import { AuthenticationError } from "../services/githits-service.js"; import { errorResult, type ToolResult } from "./types.js"; /** - * Wraps a tool handler with standard error handling. + * Wraps a tool handler with the shared structured `{error, code, + * retryable}` error envelope. Used by always-on tools (`get_example`, + * `search_language`, `feedback`) so agents can branch on `code` + * uniformly with capability-gated tools instead of text-parsing. */ export async function withErrorHandling( operation: string, @@ -10,7 +14,28 @@ export async function withErrorHandling( try { return await fn(); } catch (error) { - const message = error instanceof Error ? error.message : "Unknown error"; - return errorResult(`Failed to ${operation}: ${message}`); + return errorResult(JSON.stringify(classify(operation, error))); } } + +interface ToolErrorEnvelope { + error: string; + code: string; + retryable: boolean; +} + +function classify(operation: string, error: unknown): ToolErrorEnvelope { + if (error instanceof AuthenticationError) { + return { + error: error.message, + code: "UNAUTHENTICATED", + retryable: false, + }; + } + const message = error instanceof Error ? error.message : "Unknown error"; + return { + error: `Failed to ${operation}: ${message}`, + code: "UNKNOWN", + retryable: false, + }; +} From 4648dd7c1266455c14f15bfa52146225079b4057 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 27 Apr 2026 13:36:03 +0300 Subject: [PATCH 2/2] chore(release): bump version to 0.2.1 Patch release covering the agent-friendly tool naming and search-payload trim. --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- .plugin/plugin.json | 2 +- gemini-extension.json | 2 +- package.json | 2 +- plugins/claude/.claude-plugin/plugin.json | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index db05d56c..9fdb0b98 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.0" + "version": "0.2.1" }, "plugins": [ { diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index e7a7f2e9..4b27f711 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.0", + "version": "0.2.1", "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 e7a7f2e9..4b27f711 100644 --- a/.plugin/plugin.json +++ b/.plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.0", + "version": "0.2.1", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/gemini-extension.json b/gemini-extension.json index db5c2d55..c9d927e6 100644 --- a/gemini-extension.json +++ b/gemini-extension.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.0", + "version": "0.2.1", "description": "Code examples from global open source for developers and AI assistants.", "mcpServers": { "githits": { diff --git a/package.json b/package.json index 94b93f2d..b75094c1 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.0", + "version": "0.2.1", "type": "module", "files": [ "dist", diff --git a/plugins/claude/.claude-plugin/plugin.json b/plugins/claude/.claude-plugin/plugin.json index e7a7f2e9..4b27f711 100644 --- a/plugins/claude/.claude-plugin/plugin.json +++ b/plugins/claude/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.0", + "version": "0.2.1", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits"