diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index bfa95ac..45e8a57 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -13,7 +13,7 @@ The CLI exposes setup/auth commands, `doctor`, `example`, `languages`, `feedback | `login` | — | `--no-browser`, `--port `, `--force` | Authenticate with browser OAuth using a loopback callback on the machine running GitHits | | `example ` | `` | `-l, --lang `, `--license `, `--explain`, `--json` | Search for code examples | | `search ` | `--in ` | `--source `, `--kind `, `--category `, `--path-prefix `, `--intent `, `--public`, `--name `, `--lang `, `--allow-partial`, `--limit `, `--offset `, `--wait `, `--json` | Unified indexed search across dependency/repository code, docs, and symbols. Defaults to 10 results. | -| `search-status ` | `` | `--json` | Check progress, fetch partial hits, or fetch final results for a prior unified search | +| `search-status ` | `` | `--wait `, `--json` | Check progress, fetch partial hits, or fetch final results for a prior unified search; waits up to 20 seconds by default | | `languages [query]` | — | `--json` | List or filter supported languages | | `feedback [solution_id]` | `--accept` or `--reject` | `-m, --message `, `--tool `, `--json` | Submit solution-tied or generic session feedback | | `doctor` | — | `--json` | Print redacted diagnostics for GitHits runtime, environment, service URLs, config, and auth storage | @@ -413,7 +413,7 @@ Deterministic text grep over indexed dependency or repository source. Defaults t **`--verbose`.** Adds a summary header and grouped file sections with a `>` marker on match lines. -**`stdout` vs `stderr` routing (plain mode).** The pagination hint for `nextCursor` goes to **stderr** so stdout stays machine-friendly. +**`stdout` vs `stderr` routing (plain mode).** Pagination and zero-match decision guidance go to **stderr** so stdout stays machine-friendly. Empty guidance reports scanned/in-scope counts and the served ref/version when known. A completed scan with zero files in scope recommends loosening selectors; otherwise it recommends changing the pattern or using conceptual `search`, and explicitly rejects an unchanged repeat. When the failed call enabled `--case-sensitive`, it also recommends dropping that flag. An incomplete empty page instead preserves truncation or `--cursor` continuation guidance. **Exit codes (grep-compatible).** diff --git a/docs/implementation/mcp-cli-parity.md b/docs/implementation/mcp-cli-parity.md index a428339..0eeed64 100644 --- a/docs/implementation/mcp-cli-parity.md +++ b/docs/implementation/mcp-cli-parity.md @@ -73,6 +73,8 @@ test suite anchors the doc. - **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 behaviour. + `search_status.wait_timeout_ms` maps to `search-status --wait `; + both default to the shared 20-second bounded wait. - **Public enum values** are lowercase strings on both surfaces (`production`, `test`, `summary`, `all`). - **Service coercion** from lowercase enum values to the internal @@ -119,6 +121,9 @@ test suite anchors the doc. `allow_partial_results` / `--allow-partial` opt into backend partial payloads while indexing continues; incomplete JSON envelopes may then carry non-empty `results` plus the `searchRef`. +- Completed empty search JSON retains zero-result source/target context; + healthy source status remains suppressed for non-empty success. Text advice + is renderer-only and never replaces structured JSON. ### `PARITY-ERROR-ENVELOPE` @@ -132,6 +137,9 @@ test suite anchors the doc. mechanism, not a convention. - MCP error text is always valid JSON. A client that parses `content[0].text` on error gets the same envelope as CLI `--json`. +- Backend error messages, hints, indexing estimates, available versions/refs, + and suggested refs are preserved when supplied. Clients do not replace + specific backend guidance or synthesize target candidates. - The REST-backed `example`, `languages`, and `feedback` CLI commands preserve this envelope for generic transport/backend failures as well as typed auth failures. Human mode renders the same message as terminal text. @@ -144,6 +152,11 @@ test suite anchors the doc. CLI-only instructions like `--verbose` or `--lifecycle all`. - Default MCP success output should be compact `text-v1`; programmatic parity tests must pass `format: "json"` explicitly. +- Empty `code_grep` decision guidance is shared between MCP text and CLI + terminal stderr, with surface-native cursor syntax. Incomplete empty pages + render truncation/pagination guidance instead of completed-result pivots. + CLI stdout remains empty for grep-compatible zero-match behavior; JSON + remains the shared structured envelope. ## Checklist for adding a new dual-surface tool @@ -347,4 +360,7 @@ envelope shape. 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. + carries `matches[].symbol` when the backend hydrates it. Empty text uses + shared scan/scope/served-target context and branches recovery on whether + `filesInScope` is zero. Completed scans reject an unchanged repeat; + incomplete empty pages preserve truncation/pagination continuation instead. diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index f29e4ee..7aa64f8 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -20,8 +20,8 @@ The CLI mirrors the production MCP tool contract where equivalent tools exist. C | `get_example` | `query`, `language?`, `license_mode?`, `format?` | Search for canonical code examples. Defaults to markdown with a trailing `solution_id: ...` line for `feedback`; pass `format: "json"` for `{result, solution_id?}`. If `language` is omitted, the backend infers it from the query. | | `search_language` | `query`, `format?` | Find supported programming language names before searching. Defaults to one compact line per match (`name (Display Name) aliases: ...`); pass `format: "json"` for structured matches. | | `feedback` | `solution_id?`, `accepted`, `feedback_text?`, `tool_name?` | Submit feedback on a `get_example` result, another GitHits tool result, or the current GitHits session. | -| `search` | `query`, `target?`, `targets?`, `source?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?`, `format?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Required inputs are `query` plus either `target` or `targets`; every other argument is optional. Omit `source` to let GitHits select the best sources; use `source:"docs"` for guides/reference pages, `source:"code"` for source and tests, and `source:"symbol"` for API/entity lookup when you want to restrict results to one evidence type. Omit `file_intent` to search across all intents; set it only when you want to narrow code results. For docs-only search, code/symbol-only filters (`category`, `kind`, `file_intent`, `public_only`) are ignored client-side because the backend docs source rejects them. Complete-by-default; `limit` defaults to 10 to keep agent output compact. Set `allow_partial_results: true` to receive available partial hits while indexing continues. `format` defaults to `text-v1` for compact agent output; pass `format: "json"` for the structured envelope. | -| `search_status` | `search_ref`, `format?` | Check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results for a prior unified search. Defaults to compact `text-v1`; pass `format: "json"` for the structured envelope. | +| `search` | `query`, `target?`, `targets?`, `source?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?`, `format?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Required inputs are `query` plus either `target` or `targets`; every other argument is optional. Omit `source` to let GitHits select the best sources; use `source:"docs"` for guides/reference pages, `source:"code"` for source and tests, and `source:"symbol"` for exact API/entity lookup. Omit `file_intent` to search across all intents; set it only when you want to narrow code results. For docs-only search, code/symbol-only filters (`category`, `kind`, `file_intent`, `public_only`) are ignored client-side because the backend docs source rejects them. Complete-by-default; `limit` defaults to 10. Set `allow_partial_results: true` only on the initial call to receive available partial hits while indexing continues. A deferred response must be continued with `search_status`, not a repeated or fingerprint-modified `search`. Completed empty text gives bounded query/filter/source pivots. `format` defaults to `text-v1`; pass `format: "json"` for the structured envelope. | +| `search_status` | `search_ref`, `wait_timeout_ms?`, `format?` | Check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results for a prior unified search. `wait_timeout_ms` waits up to 60 seconds for progress or completion and defaults to 20 seconds, preventing tight status polling. Defaults to compact `text-v1`; pass `format: "json"` for the structured envelope. | | `docs_list` | `registry`, `package_name`, `version?`, `limit?`, `after?`, `format?` | List hosted/crawled and repository-backed documentation pages for a package. Defaults to compact `text-v1` with ready-to-call `docs_read` follow-ups; repo-backed entries include exact source metadata for `code_read` follow-up when available. | | `docs_read` | `page_id`, `start_line?`, `end_line?`, `format?` | Read a documentation page by page ID. Defaults to `text-v1` with a 150-line MCP text cap; explicit line ranges are supported. `format: "json"` preserves full-document default while still honoring explicit ranges. Repo-backed pages include exact file follow-up metadata. | | `pkg_info` | `registry`, `package_name`, `verbose?`, `format?` | Latest-version package triage: license, description, repository popularity (stars/forks/issues and `[ARCHIVED]` when applicable), downloads, publish age, and vulnerability status. Example: `{registry:"npm", package_name:"express"}`. Set `verbose: true` for GitHub language/topics/last-pushed, recent advisories, and recent changes. Pass `format: "json"` for structured fields. | @@ -31,7 +31,7 @@ The CLI mirrors the production MCP tool contract where equivalent tools exist. C | `pkg_upgrade_review` | `registry?`, `package_name?`, `current_version?`, `target_version?`, `packages?`, `skip_transitive_security?`, `include_dependency_issues?`, `min_severity?`, `verbose?`, `format?` | Evidence for dependency upgrades. Accepts a single package or repeatable batch, compares current vs target direct vulnerabilities, changelog range evidence, target deprecation metadata, peer dependency changes, dependency changes, and transitive security evidence by default. `skip_transitive_security:true` disables transitive vulnerability evidence when latency matters. Reports facts only; callers decide whether to accept the upgrade. | | `code_files` | `target`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `file_types?`, `languages?`, `file_intent?`, `file_intents?`, `exclude_file_intents?`, `exclude_doc_files?`, `exclude_test_files?`, `include_hidden?`, `limit?`, `wait_timeout_ms?`, `format?` | List files in an indexed dependency. Returns `{total, hasMore, files: [{path, name, language, fileType, byteSize}], resolution, indexedVersion, targetResolution?}` in JSON mode. Dual addressing via `target.registry + target.package_name` (spec) or `target.repo_url + target.git_ref?` (repo, omitted ref means default branch intent). Selectors (`path`, `path_prefix`, `globs`) are OR-ed; the other filters intersect on top. `INDEXING` errors include immediate retry candidates in `details.availableVersions` / `details.availableRefs` when available; repository ref suggestions use `suggestedRefs` and are not immediate retry guarantees. `format` defaults to `text-v1` (paths-only listing); pass `format: "json"` for the structured envelope. | | `code_read` | `target`, `path`, `start_line?`, `end_line?`, `wait_timeout_ms?`, `format?` | Read a file from an indexed dependency. `target` accepts the structured object or compact string (`npm:react@18.2.0`, `github:facebook/react#HEAD`, `github.com/facebook/react#HEAD`, `https://github.com/facebook/react#HEAD`, `github:facebook/react@HEAD`, or any repo form without `#ref`/`@ref` for default branch intent). User-facing output canonicalizes repo targets as `github:owner/repo#ref` so refs can contain `@` safely. Package compact strings require an explicit registry prefix. **MCP per-call span cap: 150 lines** — broader requests (or no range) are silently truncated to the first 150 lines from the caller's start, with a hint explaining the cap and the original request. Defaults to `text-v1` with line-numbered content; pass `format: "json"` for the structured envelope. Binary files set `isBinary: true` and omit `content`; `targetResolution` may explain fallback/indexing provenance. On `NOT_FOUND` / `FILE_NOT_FOUND` call `code_files` to discover the actual path. The cap is MCP-only; the CLI command `githits code read` honors arbitrary ranges. | -| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?`, `format?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. `format` defaults to `text-v1` (matches grouped by file, grep -A/-B notation for context); pass `format: "json"` for the structured envelope. | +| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?`, `format?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. Empty text reports scanned/in-scope counts and served identity, then branches between loosening selectors (zero files in scope) and changing the literal/pattern or using conceptual `search`. Disabling case sensitivity is suggested only when the failed call enabled it. `format` defaults to `text-v1`; pass `format: "json"` for the structured envelope. | `search`, `search_status`, `docs_list`, `docs_read`, `pkg_info`, `pkg_vulns`, `pkg_deps`, `pkg_changelog`, `pkg_upgrade_review`, `code_files`, `code_read`, and `code_grep` are registered by default. The package/source service URL defaults to the GitHits-managed endpoint and can be overridden via `GITHITS_CODE_NAV_URL` for local development. @@ -55,7 +55,7 @@ Treat failures as live backend or contract findings, not deterministic unit-test **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`. -**Promoted `warnings[]`.** Noteworthy `sourceStatus` entries — sources reporting `incompatibleQueryFeatures`, `ignoredQueryFeatures`, `incompatibleFilters`, `ignoredFilters`, lifecycle anomalies (`indexingStatus`, `codeIndexState`), or a free-form `note` — are also surfaced as a top-level `warnings: string[]` in the completed/incomplete payloads (and appended after parser warnings inside the `search_status` result block). The structured detail still lives in `sourceStatus`; `warnings[]` is the agent-visible signal that something about execution did not match the request. Mitigates backend issue B5: docs-only search plus a `kind:`/`lang:` qualifier returns `results: []` with the only diagnostic buried inside `sourceStatus[].note`. The text-v1 renderer prints the warnings as a `warnings:` preamble. Implementation in `buildSourceStatusWarnings` (`packages/mcp/src/shared/unified-search-response.ts`); remove once the backend surfaces these at the top level itself. +**Promoted `warnings[]`.** Noteworthy `sourceStatus` entries — sources reporting `incompatibleQueryFeatures`, `ignoredQueryFeatures`, `incompatibleFilters`, `ignoredFilters`, lifecycle anomalies (`indexingStatus`, `codeIndexState`), or a free-form `note` — are also surfaced as a top-level `warnings: string[]` in the completed/incomplete payloads (and appended after parser warnings inside the `search_status` result block). The structured detail still lives in `sourceStatus`; `warnings[]` is the agent-visible signal that something about execution did not match the request. On completed empty results, healthy source entries are also retained with zero `resultCount` and served identity; requested/fresh labels emit only when they materially differ from served. Healthy `INDEXED` / `CURRENT` / non-divergent `STALE` states never become warnings. Successful non-empty responses keep the prior compact projection. The text-v1 renderer prints backend warnings and source notes before empty-result advice. Implementation in `buildSourceStatusWarnings` and empty-result compaction (`packages/mcp/src/shared/unified-search-response.ts`). ### `pkg_info` response shape @@ -152,13 +152,14 @@ All three code-navigation tools share the same indexing-retry contract. The stat **`INDEXING` error envelope**: ```json { - "error": "Target is indexing. Running for 12 seconds. Similar refs usually index in 7 to 19 seconds. Retry, or wait until ready with CLI `--wait 60000` / MCP `wait_timeout_ms: 60000`. Indexing ref: ref_...", + "error": "Target is indexing", "code": "INDEXING", "retryable": true, "details": { "indexingRef": "ref_…", "availableVersions": [{"version": "4.21.0", "ref": "v4.21.0"}], "availableRefs": [{"ref": "main"}], + "hint": "Backend says this ref is queued. Wait until ready with CLI `--wait 60000` or MCP `wait_timeout_ms: 60000`.", "indexingEstimate": { "lowerSeconds": 7, "upperSeconds": 19, @@ -170,7 +171,9 @@ All three code-navigation tools share the same indexing-retry contract. The stat } ``` -`details.indexingEstimate` is populated when the backend returns duration telemetry. The user-facing message includes elapsed time and the observed lower/upper duration range when present; otherwise it falls back to the backend hint or the generic "usually completes within 30 seconds" guidance. `details.availableVersions` and `details.availableRefs` are populated when the backend returned already-indexed artifacts alongside the sentinel. Agents can pick one to retry against immediately without waiting. `details.suggestedRefs` appears on `REF_NOT_FOUND` and inside `details.targetResolution` when the backend has fuzzy repository-ref candidates; these are suggestions only, not immediate retry guarantees. When `details.targetResolution` is present, it is explanatory provenance only; follow-up commands still use served locators / legacy served fields rather than reconstructing targets from the originally requested identity. +Backend GraphQL errors preserve the backend message verbatim and carry its `hint`, `indexingEstimate`, and available artifacts in `details`; client prose does not replace them. CLI terminal errors render a preserved backend hint beneath the message, and human `search` / `search-status` indexing errors use the same detail formatter as `code files` / `code read` / `code grep`. A `PACKAGE_INDEXING` error receives appended CLI `--wait` / MCP `wait_timeout_ms` fallback guidance only when neither the backend message nor hint names a wait argument; the backend text remains intact. Data-path indexing sentinels have no backend message or hint, so the client supplies the same wait guidance while structured detail lines carry the indexing ref and estimate. `details.availableVersions` and `details.availableRefs` are already indexed and immediately queryable. `details.suggestedRefs` appears on `REF_NOT_FOUND` and inside `details.targetResolution`; these are fuzzy suggestions and may require indexing. The client never fabricates candidates. + +**Follow-up — error metadata carrier consolidation.** Target, version, and ref errors currently carry available artifacts both as legacy constructor fields and in common error metadata; `CodeNavigationIndexingError` also carries `hint` as a standalone constructor field. Consolidate those carriers in a dedicated refactor; changing the internal error API is outside this response-formatting slice and has no user-visible anti-looping benefit. **Retry default**: `DEFAULT_WAIT_TIMEOUT_MS = 20_000` (shared, defined in `packages/mcp/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. @@ -186,7 +189,7 @@ The `hint` field is emitted only when the cap *actually truncated* the response **Why text-v1 default.** A 10-hit `search` JSON envelope runs 5–7 KB after compaction; the same hits in `text-v1` land around 3–4 KB. The savings come from dropped quoting, dropped key repetition, and dropped fields that an agent does not need at the per-call decision point (highlights byte offsets, repeated locator scaffolding). The token budget belongs to the agent's reasoning, not to JSON structure. -**Format stability.** The text format is a public contract, locked with snapshot-style tests (`packages/mcp/src/shared/unified-search-text.test.ts`, `packages/mcp/src/shared/list-files-text.test.ts`). The `text-v1` version tag exists so we can evolve the format without silently breaking downstream parsers — future incompatible changes ship as `text-v2`. +**Format stability.** The text format is a public contract, locked with snapshot-style tests (`packages/mcp/src/shared/unified-search-text.test.ts`, `packages/mcp/src/tools/search-status.test.ts`, `packages/mcp/src/shared/list-files-text.test.ts`, `packages/mcp/src/shared/grep-repo-text.test.ts`). The `text-v1` version tag exists so incompatible evolution can ship as `text-v2`. **ASCII-only.** Separators are ` | `; ellipsis is `...`; no box-drawing or Latin-1 punctuation. Tokenizer behavior for multi-byte UTF-8 varies across BPE variants, and the format runs into Claude, Codex CLI, OpenCode, Cline, Cursor, etc. — ASCII keeps it predictable. @@ -212,6 +215,10 @@ More hits available. Pass offset=N for the next page or limit=N to widen. `` compacts to `code` / `symbol` / `docs` / `repo-docs`. `` is a ready-to-call follow-up when possible: `code_read target="npm:pkg@version" path="..." start_line=N end_line=M` for code/symbol hits and `docs_read page_id="..."` for documentation hits. If a code/symbol hit lacks a file path, text mode prints `follow-up unavailable: missing filePath` rather than fabricating a path. +**Follow-up — crawled-doc section anchors.** Unified search can label a crawled documentation hit with a matching section title while returning only its page ID. Without a line anchor, `docs_read` must start at the beginning of the page. Carrying section ranges through search results requires backend/search-location support and is outside the CLI response-formatting slice. + +Completed empty search renders backend warnings/source notes first, then served target/freshness context and `Do not repeat this search unchanged.` Generic pivots are conditional: filter removal appears only when filters exist, symbol search is omitted when already selected, and standalone site searches do not suggest `code_grep`. If the completed source is still indexing, query rewriting is suppressed in favor of a larger `wait_timeout_ms` or an indexed alternative labelled `queryable now`. Active deferred search reports ready/total counts, says `Do not repeat search.`, and gives the exact bounded continuation `next: call search_status with search_ref="..." and wait_timeout_ms=20000.` Terminal `FAILED` and `TIMEOUT` sessions prohibit further status calls and direct the caller to rerun `search` instead. The response never suggests changing `allow_partial_results` after deferral. + **Listing anatomy** (`code_files` text-v1): ``` @@ -241,12 +248,14 @@ code_grep | matches in files | pattern="..." [regex,case-sensitive] 142: matching line 143- context-after line [blank] -[Truncated: limit. Pass narrower path/path_prefix/globs or increase max_matches.] +[Truncated: time limit reached. Pass narrower path/path_prefix/globs or increase max_matches.] [More matches available. Pass cursor= for the next page.] ``` Standard grep -A/-B notation: `:` separator on match lines, `-` on context lines. Non-adjacent blocks within the same file are separated by `--`. The `()` after the file path is the per-file match count; the header sums across files. Header flags (`regex`, `case-sensitive`) appear only when the request used them. Scope filters are not echoed in text mode; agents already have the tool call arguments in context, and `format: "json"` preserves exact request/filter metadata for programmatic use. Match-line offsets, file content hashes, file intent, and symbol metadata are dropped in text mode — agents that need them can request `format: "json"`. +Empty grep adds scanned/in-scope counts, served target/ref context when known, and `Do not repeat this grep unchanged.` Positive equal counts collapse to `files scanned: N (full scope)`; zero scope says `no files in scope`. When the content index prunes candidates before verification, unequal counts explicitly identify the smaller value as `content-scanned after index pruning`, so it cannot be mistaken for an incomplete whole-target scan. Zero in-scope files direct the caller to loosen selectors; a nonzero scope directs it to change the pattern or switch to conceptual `search`. A case-sensitive request also suggests disabling case sensitivity. MCP guidance uses structured argument names while CLI guidance uses positional/flag syntax. The same decision text is shared with CLI terminal stderr while plain CLI stdout remains grep-compatible and empty. Backend truncation enums are normalized to lowercase in JSON and rendered as `match limit reached`, `per-file match limit reached`, or `time limit reached` in text. + `context_lines`, `context_lines_before`, and `context_lines_after` accept integers from 0 through 10. The MCP JSON Schema advertises the range so agent clients reject invalid calls before dispatch; direct CLI/internal callers retain the same request-builder validation. The asymmetric fields override the corresponding side of `context_lines`. **Docs read cap.** `docs_read` text output is capped at 150 lines per call, including explicit larger ranges. Its response reports the actual returned range and total line count for the next bounded read; JSON mode preserves explicitly requested ranges. diff --git a/packages/core-internal/src/services/code-navigation-service.test.ts b/packages/core-internal/src/services/code-navigation-service.test.ts index b1aedf2..ebb3f12 100644 --- a/packages/core-internal/src/services/code-navigation-service.test.ts +++ b/packages/core-internal/src/services/code-navigation-service.test.ts @@ -15,6 +15,7 @@ import { CodeNavigationRefNotFoundError, CodeNavigationServiceImpl, CodeNavigationTargetNotFoundError, + CodeNavigationVersionNotFoundError, } from "./code-navigation-service.js"; import { createMockTokenProvider } from "./test-helpers.js"; @@ -253,13 +254,9 @@ describe("CodeNavigationServiceImpl", () => { expect(error).toBeInstanceOf(CodeNavigationIndexingError); const typed = error as CodeNavigationIndexingError; expect(typed.indexingRef).toBe("ref_xyz"); - expect(typed.message).toContain("Running for 12 seconds."); - expect(typed.message).toContain( - "Similar refs usually index in 7 to 19 seconds.", - ); - expect(typed.message).not.toContain( - "Indexing usually completes within 30 seconds", - ); + expect(typed.message).toContain("--wait 60000"); + expect(typed.message).not.toContain("Running for 12 seconds."); + expect(typed.message).not.toContain("Similar refs usually index"); expect(typed.indexingEstimate).toEqual({ lowerSeconds: 7, upperSeconds: 19, @@ -738,6 +735,49 @@ describe("CodeNavigationServiceImpl", () => { expect(body.variables.allowPartialResults).toBe(true); }); + it("forwards the search-status wait window to GraphQL", async () => { + let capturedBody = ""; + globalThis.fetch = mock((_, init?: RequestInit) => { + capturedBody = String(init?.body ?? ""); + return Promise.resolve( + new Response( + JSON.stringify({ + data: { + discoverySearchProgress: { + searchRef: "search-ref-wait", + status: "SEARCHING", + targetsTotal: 1, + targetsReady: 1, + elapsedMs: 500, + query: "router", + queryWarnings: [], + sources: ["CODE"], + results: null, + }, + }, + }), + { headers: { "Content-Type": "application/json" } }, + ), + ); + }) as unknown as typeof fetch; + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + globalThis.fetch, + ); + + await service.searchStatus("search-ref-wait", 25_000); + + const body = JSON.parse(capturedBody); + expect(body.query).toContain("$waitTimeoutMs: Int"); + expect(body.query).toContain("waitTimeoutMs: $waitTimeoutMs"); + expect(body.variables).toEqual({ + searchRef: "search-ref-wait", + includeResults: true, + waitTimeoutMs: 25_000, + }); + }); + it("emits safe debug logging for unified search request shape without query text", async () => { process.env.GITHITS_DEBUG = "code-nav"; const stderrSpy = spyOn(process.stderr, "write").mockImplementation( @@ -1169,12 +1209,12 @@ describe("CodeNavigationServiceImpl", () => { } catch (error) { expect(error).toBeInstanceOf(CodeNavigationIndexingError); const typed = error as CodeNavigationIndexingError; - expect(typed.message).toContain("Running for 3 seconds."); - expect(typed.message).toContain( - "Similar refs usually index in 1 second.", + expect(typed.message).toBe("Target is indexing"); + expect(typed.hint).toContain( + "Backend says this ref is queued for indexing.", ); - expect(typed.message).not.toContain("Backend says"); - expect(typed.message).toContain("--wait 60000"); + expect(typed.hint).toContain("--wait 60000"); + expect(typed.hint).toContain("wait_timeout_ms: 60000"); expect(typed.indexingEstimate).toEqual({ lowerSeconds: 1, upperSeconds: 1, @@ -1185,6 +1225,139 @@ describe("CodeNavigationServiceImpl", () => { } }); + it("does not repeat a backend hint already present in the indexing message", async () => { + const backendHint = "Backend says this ref is queued for indexing."; + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: `Target is indexing. ${backendHint}`, + extensions: { + code: "PACKAGE_INDEXING", + hint: backendHint, + }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + try { + await service.listFiles({ + target: { registry: "NPM", packageName: "express" }, + }); + throw new Error("expected listFiles to throw"); + } catch (error) { + expect(error).toBeInstanceOf(CodeNavigationIndexingError); + const typed = error as CodeNavigationIndexingError; + expect( + `${typed.message} ${typed.hint}`.match(new RegExp(backendHint, "g")), + ).toHaveLength(1); + expect(typed.hint).toContain("--wait 60000"); + expect(typed.hint).not.toContain(backendHint); + } + }); + + it("does not append fallback guidance when the message or hint names a wait argument", async () => { + const cases = [ + { + message: "Target is indexing", + hint: "Call again with wait_timeout_ms: 45000.", + waitArgument: "wait_timeout_ms: 45000", + }, + { + message: "Target is indexing. Wait with --wait 60000.", + hint: "Backend says this ref is queued.", + waitArgument: "--wait 60000", + }, + ]; + + for (const testCase of cases) { + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: testCase.message, + extensions: { + code: "PACKAGE_INDEXING", + hint: testCase.hint, + }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + try { + await service.listFiles({ + target: { registry: "NPM", packageName: "express" }, + }); + throw new Error("expected listFiles to throw"); + } catch (error) { + expect(error).toBeInstanceOf(CodeNavigationIndexingError); + const typed = error as CodeNavigationIndexingError; + const combined = `${typed.message} ${typed.hint}`; + expect(typed.hint).toBe(testCase.hint); + expect(combined.split(testCase.waitArgument)).toHaveLength(2); + expect(combined).not.toContain("Wait until ready with CLI"); + } + } + }); + + it("preserves a bare PACKAGE_INDEXING message and supplies wait guidance", async () => { + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: "Target is indexing", + extensions: { + code: "PACKAGE_INDEXING", + indexing_ref: "idx-error", + }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + try { + await service.listFiles({ + target: { registry: "NPM", packageName: "express" }, + }); + throw new Error("expected listFiles to throw"); + } catch (error) { + expect(error).toBeInstanceOf(CodeNavigationIndexingError); + const typed = error as CodeNavigationIndexingError; + expect(typed.message).toBe("Target is indexing"); + expect(typed.hint).toContain("--wait 60000"); + expect(typed.hint).toContain("wait_timeout_ms: 60000"); + } + }); + it("throws CodeNavigationBackendError when backend emits GREP_FILE_TOO_LARGE", async () => { mockFetch(() => Promise.resolve( @@ -1196,6 +1369,10 @@ describe("CodeNavigationServiceImpl", () => { extensions: { code: "GREP_FILE_TOO_LARGE", file_path: "dist/bundle.js", + hint: "Use a narrower source path.", + available_versions: [{ version: "5.2.1", ref: "v5.2.1" }], + available_refs: [{ ref: "main", version: null }], + suggested_refs: [{ ref: "v5.2.1", version: null }], }, }, ], @@ -1220,6 +1397,106 @@ describe("CodeNavigationServiceImpl", () => { expect((error as CodeNavigationBackendError).graphqlCode).toBe( "GREP_FILE_TOO_LARGE", ); + expect((error as CodeNavigationBackendError).metadata).toEqual({ + hint: "Use a narrower source path.", + availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], + availableRefs: [{ ref: "main", version: undefined }], + suggestedRefs: [{ ref: "v5.2.1", version: undefined }], + }); + } + }); + + it("preserves GraphQL VERSION_NOT_FOUND guidance and alternatives", async () => { + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: "Version 4 is not indexed.", + extensions: { + code: "VERSION_NOT_FOUND", + package: "npm/express", + requested_version: "4", + latest_indexed: "5.2.1", + hint: "Use an indexed version.", + available_versions: [{ version: "5.2.1", ref: "v5.2.1" }], + }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + try { + await service.search({ + targets: [{ registry: "NPM", packageName: "express", version: "4" }], + query: "router", + }); + throw new Error("expected VERSION_NOT_FOUND"); + } catch (error) { + expect(error).toBeInstanceOf(CodeNavigationVersionNotFoundError); + const typed = error as CodeNavigationVersionNotFoundError; + expect(typed.message).toBe("Version 4 is not indexed."); + expect(typed.availableVersions).toEqual([ + { version: "5.2.1", ref: "v5.2.1" }, + ]); + expect(typed.metadata?.hint).toBe("Use an indexed version."); + } + }); + + it("preserves GraphQL NOT_FOUND guidance and alternatives", async () => { + mockFetch(() => + Promise.resolve( + new Response( + JSON.stringify({ + errors: [ + { + message: "Target not found. Check the package name.", + extensions: { + code: "NOT_FOUND", + hint: "Use the canonical registry package name.", + available_versions: [{ version: "5.2.1", ref: "v5.2.1" }], + available_refs: [{ ref: "main", version: null }], + suggested_refs: [{ ref: "v5.2.1", version: null }], + }, + }, + ], + }), + { status: 200 }, + ), + ), + ); + const service = new CodeNavigationServiceImpl( + BASE_URL, + createMockTokenProvider(), + ); + + try { + await service.search({ + targets: [{ registry: "NPM", packageName: "missing" }], + query: "router", + }); + throw new Error("expected NOT_FOUND"); + } catch (error) { + expect(error).toBeInstanceOf(CodeNavigationTargetNotFoundError); + const typed = error as CodeNavigationTargetNotFoundError; + expect(typed.message).toBe("Target not found. Check the package name."); + expect(typed.availableVersions).toEqual([ + { version: "5.2.1", ref: "v5.2.1" }, + ]); + expect(typed.metadata).toEqual({ + hint: "Use the canonical registry package name.", + availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], + availableRefs: [{ ref: "main", version: undefined }], + suggestedRefs: [{ ref: "v5.2.1", version: undefined }], + }); } }); @@ -1237,6 +1514,7 @@ describe("CodeNavigationServiceImpl", () => { retryable: false, repo_url: "https://github.com/openai/codex", git_ref: "1.2.3", + hint: "Choose one of the indexed refs.", available_refs: [{ ref: "main", version: null }], suggested_refs: [ { ref: "codex@1.2.3", version: null }, @@ -1275,6 +1553,7 @@ describe("CodeNavigationServiceImpl", () => { { ref: "codex@1.2.3", version: undefined }, { ref: "v1.2.3", version: undefined }, ]); + expect(typed.metadata?.hint).toBe("Choose one of the indexed refs."); } }); diff --git a/packages/core-internal/src/services/code-navigation-service.ts b/packages/core-internal/src/services/code-navigation-service.ts index e571e4d..afadb17 100644 --- a/packages/core-internal/src/services/code-navigation-service.ts +++ b/packages/core-internal/src/services/code-navigation-service.ts @@ -20,6 +20,9 @@ import { } from "./githits-service.js"; import type { TokenProvider } from "./token-provider.js"; +const INDEXING_WAIT_HINT = + "Wait until ready with CLI `--wait 60000` or MCP `wait_timeout_ms: 60000`."; + /** * Back-compat alias — the canonical registry union now lives in * `src/shared/pkgseer-registry.ts`. Re-exported here so existing @@ -525,7 +528,10 @@ export interface GrepRepoResult { export interface CodeNavigationService { search(params: UnifiedSearchParams): Promise; - searchStatus(searchRef: string): Promise; + searchStatus( + searchRef: string, + waitTimeoutMs?: number, + ): Promise; listFiles(params: ListFilesParams): Promise; readFile(params: ReadFileParams): Promise; grepRepo(params: GrepRepoParams): Promise; @@ -558,12 +564,22 @@ export class CodeNavigationIndexingError extends Error { public readonly indexingEstimate: | IndexingDurationEstimate | undefined = undefined, + public readonly hint: string | undefined = undefined, ) { super(message); this.name = "CodeNavigationIndexingError"; } } +export interface CodeNavigationErrorMetadata { + hint?: string; + availableVersions?: AvailableVersion[]; + availableRefs?: AvailableRef[]; + suggestedRefs?: SuggestedRef[]; + targetResolution?: TargetResolution; + indexingEstimate?: IndexingDurationEstimate; +} + export class CodeNavigationUnresolvableError extends Error { constructor(message: string) { super(message); @@ -584,6 +600,9 @@ export class CodeNavigationTargetNotFoundError extends Error { public readonly availableVersions?: AvailableVersion[], public readonly repoUrl?: string, public readonly requestedRef?: string, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationTargetNotFoundError"; @@ -622,6 +641,9 @@ export class CodeNavigationVersionNotFoundError extends Error { public readonly requestedVersion: string | undefined, public readonly latestIndexed: string | undefined, public readonly availableVersions: AvailableVersion[] | undefined, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationVersionNotFoundError"; @@ -640,6 +662,9 @@ export class CodeNavigationRefNotFoundError extends Error { public readonly requestedRef: string | undefined, public readonly availableRefs: AvailableRef[] | undefined, public readonly suggestedRefs: SuggestedRef[] | undefined, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationRefNotFoundError"; @@ -688,6 +713,9 @@ export class CodeNavigationBackendError extends Error { * April 2026 `extensions.retryable` contract on GraphQL errors. */ public readonly retryable?: boolean, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationBackendError"; @@ -929,8 +957,8 @@ query UnifiedSearch( }`; const UNIFIED_SEARCH_STATUS_QUERY = ` -query UnifiedSearchStatus($searchRef: String!, $includeResults: Boolean!) { - discoverySearchProgress(searchRef: $searchRef, includeResults: $includeResults) { +query UnifiedSearchStatus($searchRef: String!, $includeResults: Boolean!, $waitTimeoutMs: Int) { + discoverySearchProgress(searchRef: $searchRef, includeResults: $includeResults, waitTimeoutMs: $waitTimeoutMs) { searchRef status targetsTotal @@ -1814,13 +1842,16 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { }); } - async searchStatus(searchRef: string): Promise { + async searchStatus( + searchRef: string, + waitTimeoutMs = 0, + ): Promise { return executeWithTokenRefresh({ getToken: () => this.tokenProvider.getToken(), forceRefresh: () => this.tokenProvider.forceRefresh(), shouldRefresh: (error) => error instanceof AuthenticationError, executeWithToken: (token) => - this.executeUnifiedSearchStatus(token, searchRef), + this.executeUnifiedSearchStatus(token, searchRef, waitTimeoutMs), }); } @@ -1897,6 +1928,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { private async executeUnifiedSearchStatus( token: string, searchRef: string, + waitTimeoutMs: number, ): Promise { let response: PkgseerGraphqlResponse; try { @@ -1906,6 +1938,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { variables: { searchRef, includeResults: true, + waitTimeoutMs, }, }); } catch (cause) { @@ -2023,6 +2056,10 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { : undefined; const indexingRef = getGraphQLIndexingRef(errors); const indexingEstimate = parseIndexingDurationEstimate(extensions); + const errorMetadata = parseGraphQLErrorMetadata( + extensions, + indexingEstimate, + ); if (isClientUpdateRequiredGraphQLError({ message, code })) { return new ClientUpdateRequiredError( @@ -2055,16 +2092,16 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { switch (code) { case "PACKAGE_INDEXING": return new CodeNavigationIndexingError( - this.createIndexingMessage( - indexingRef, - indexingEstimate, - typeof extensions?.hint === "string" ? extensions.hint : undefined, - ), + message, indexingRef, parseAvailableVersions(extensions), parseAvailableRefs(extensions), parseTargetResolution(extensions), indexingEstimate, + appendIndexingWaitHint( + message, + typeof extensions?.hint === "string" ? extensions.hint : undefined, + ), ); case "GREP_PATTERN_TOO_SHORT": @@ -2095,6 +2132,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { ? extensions.latest_indexed : undefined, parseAvailableVersions(extensions), + errorMetadata, ); case "REF_NOT_FOUND": @@ -2104,12 +2142,19 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { parseGraphQLGitRef(extensions), parseAvailableRefs(extensions), parseSuggestedRefs(extensions), + errorMetadata, ); case "NOT_FOUND": case "PACKAGE_NOT_FOUND": case "NO_REPOSITORY_URL": - return new CodeNavigationTargetNotFoundError(message); + return new CodeNavigationTargetNotFoundError( + message, + parseAvailableVersions(extensions), + parseGraphQLRepoUrl(extensions), + parseGraphQLGitRef(extensions), + errorMetadata, + ); case "REPOSITORY_NOT_FOUND": return new CodeNavigationTargetNotFoundError( @@ -2117,6 +2162,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { undefined, parseGraphQLRepoUrl(extensions), parseGraphQLGitRef(extensions), + errorMetadata, ); case "FILE_NOT_FOUND": @@ -2162,6 +2208,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { undefined, code, retryable, + errorMetadata, ); // `code` was present but not one of the recognised values — @@ -2184,30 +2231,23 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { return new CodeNavigationUnresolvableError(message); } if (isTargetNotFoundMessage(message)) { - return new CodeNavigationTargetNotFoundError(message); + return new CodeNavigationTargetNotFoundError( + message, + parseAvailableVersions(extensions), + parseGraphQLRepoUrl(extensions), + parseGraphQLGitRef(extensions), + errorMetadata, + ); } } - return new CodeNavigationBackendError(message, undefined, code, retryable); - } - - private createIndexingMessage( - indexingRef?: string, - estimate?: IndexingDurationEstimate, - backendHint?: string, - ): string { - const retryGuidance = - "Retry, or wait until ready with CLI `--wait 60000` / MCP `wait_timeout_ms: 60000`."; - const estimateMessage = formatIndexingDurationEstimate(estimate); - const base = estimateMessage - ? `Target is indexing. ${estimateMessage} ${retryGuidance}` - : backendHint - ? appendRetryGuidance(backendHint, retryGuidance) - : `Target is indexing. Usually completes within 30 seconds. ${retryGuidance}`; - if (indexingRef) { - return `${base} Indexing ref: ${indexingRef}.`; - } - return base; + return new CodeNavigationBackendError( + message, + undefined, + code, + retryable, + errorMetadata, + ); } private normaliseUnifiedSearchOutcome( @@ -2390,10 +2430,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { data.indexingEstimate, ); throw new CodeNavigationIndexingError( - this.createIndexingMessage( - data.indexingRef ?? targetResolution?.indexingRef, - indexingEstimate, - ), + `Target is indexing. ${INDEXING_WAIT_HINT}`, data.indexingRef ?? targetResolution?.indexingRef, normaliseAvailableVersions(data.availableVersions) ?? targetResolution?.availableVersions, @@ -2841,6 +2878,24 @@ function parseSuggestedRefs( return parseAvailableArtifacts(raw); } +function parseGraphQLErrorMetadata( + extensions: Record | undefined, + indexingEstimate: IndexingDurationEstimate | undefined, +): CodeNavigationErrorMetadata | undefined { + const metadata: CodeNavigationErrorMetadata = {}; + if (typeof extensions?.hint === "string") metadata.hint = extensions.hint; + const availableVersions = parseAvailableVersions(extensions); + if (availableVersions?.length) metadata.availableVersions = availableVersions; + const availableRefs = parseAvailableRefs(extensions); + if (availableRefs?.length) metadata.availableRefs = availableRefs; + const suggestedRefs = parseSuggestedRefs(extensions); + if (suggestedRefs?.length) metadata.suggestedRefs = suggestedRefs; + const targetResolution = parseTargetResolution(extensions); + if (targetResolution) metadata.targetResolution = targetResolution; + if (indexingEstimate) metadata.indexingEstimate = indexingEstimate; + return Object.keys(metadata).length > 0 ? metadata : undefined; +} + function parseGraphQLRepoUrl( extensions: Record | undefined, ): string | undefined { @@ -2918,34 +2973,20 @@ function normaliseIndexingDurationEstimate( return Object.keys(out).length > 0 ? out : undefined; } -function formatIndexingDurationEstimate( - estimate: IndexingDurationEstimate | undefined, +function appendIndexingWaitHint( + message: string, + backendHint: string | undefined, ): string | undefined { - if (!estimate) return undefined; - const parts: string[] = []; - if (typeof estimate.elapsedSeconds === "number") { - parts.push(`Running for ${formatSeconds(estimate.elapsedSeconds)}.`); - } - if ( - typeof estimate.lowerSeconds === "number" && - typeof estimate.upperSeconds === "number" - ) { - const duration = - estimate.lowerSeconds === estimate.upperSeconds - ? formatSeconds(estimate.lowerSeconds) - : `${estimate.lowerSeconds} to ${formatSeconds(estimate.upperSeconds)}`; - parts.push(`Similar refs usually index in ${duration}.`); + const hintAlreadyInMessage = Boolean( + backendHint && message.includes(backendHint), + ); + const existingGuidance = `${message} ${backendHint ?? ""}`; + if (/(?:--wait\b|wait_timeout_ms|waitTimeoutMs)/i.test(existingGuidance)) { + return hintAlreadyInMessage ? undefined : backendHint; } - return parts.length > 0 ? parts.join(" ") : undefined; -} - -function formatSeconds(seconds: number): string { - return `${seconds} ${seconds === 1 ? "second" : "seconds"}`; -} - -function appendRetryGuidance(hint: string, retryGuidance: string): string { - if (hint.includes("--wait") || hint.includes("wait_timeout_ms")) return hint; - return `${hint} ${retryGuidance}`; + return backendHint && !hintAlreadyInMessage + ? `${backendHint} ${INDEXING_WAIT_HINT}` + : INDEXING_WAIT_HINT; } function parseAvailableArtifacts(raw: unknown): AvailableVersion[] | undefined { diff --git a/packages/mcp/src/shared/code-navigation-error-map.test.ts b/packages/mcp/src/shared/code-navigation-error-map.test.ts index 76090d2..5d820c3 100644 --- a/packages/mcp/src/shared/code-navigation-error-map.test.ts +++ b/packages/mcp/src/shared/code-navigation-error-map.test.ts @@ -62,6 +62,33 @@ describe("mapCodeNavigationError", () => { }); }); + it("preserves backend metadata on typed not-found errors", () => { + const err = new CodeNavigationTargetNotFoundError( + "Target not found.", + undefined, + undefined, + undefined, + { + hint: "Use the canonical package name.", + availableRefs: [{ ref: "main" }], + suggestedRefs: [{ ref: "v5.2.1" }], + indexingEstimate: { lowerSeconds: 3, upperSeconds: 8 }, + }, + ); + + expect(mapCodeNavigationError(err)).toEqual({ + code: "NOT_FOUND", + message: "Target not found.", + retryable: false, + details: { + hint: "Use the canonical package name.", + availableRefs: [{ ref: "main" }], + suggestedRefs: [{ ref: "v5.2.1" }], + indexingEstimate: { lowerSeconds: 3, upperSeconds: 8 }, + }, + }); + }); + it("classifies CodeNavigationTargetNotFoundError without availableVersions", () => { const err = new CodeNavigationTargetNotFoundError("Package not found"); expect(mapCodeNavigationError(err)).toEqual({ @@ -177,6 +204,7 @@ describe("mapCodeNavigationError", () => { undefined, undefined, { lowerSeconds: 7, upperSeconds: 19, sampleCount: 9 }, + "Use an indexed version now or wait for this target.", ); expect(mapCodeNavigationError(err)).toEqual({ code: "INDEXING", @@ -186,6 +214,7 @@ describe("mapCodeNavigationError", () => { indexingRef: "idx-42", availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], indexingEstimate: { lowerSeconds: 7, upperSeconds: 19, sampleCount: 9 }, + hint: "Use an indexed version now or wait for this target.", }, }); }); @@ -271,6 +300,34 @@ describe("mapCodeNavigationError", () => { }); }); + it("preserves backend metadata on generic GraphQL errors", () => { + const err = new CodeNavigationBackendError( + "Backend-specific failure.", + undefined, + "FUTURE_ERROR", + false, + { + hint: "Follow the backend-specific recovery path.", + availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], + availableRefs: [{ ref: "main" }], + suggestedRefs: [{ ref: "v5.2.1" }], + }, + ); + + expect(mapCodeNavigationError(err)).toEqual({ + code: "BACKEND_ERROR", + message: "Backend-specific failure.", + retryable: false, + details: { + hint: "Follow the backend-specific recovery path.", + availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], + availableRefs: [{ ref: "main" }], + suggestedRefs: [{ ref: "v5.2.1" }], + graphqlCode: "FUTURE_ERROR", + }, + }); + }); + it("classifies CodeNavigationBackendError with TIMEOUT as TIMEOUT (retryable)", () => { const err = new CodeNavigationBackendError( "Backend timed out", diff --git a/packages/mcp/src/shared/code-navigation-error-map.ts b/packages/mcp/src/shared/code-navigation-error-map.ts index 31774a3..7f230c1 100644 --- a/packages/mcp/src/shared/code-navigation-error-map.ts +++ b/packages/mcp/src/shared/code-navigation-error-map.ts @@ -7,6 +7,7 @@ import { ClientUpdateRequiredError, CodeNavigationAccessError, CodeNavigationBackendError, + type CodeNavigationErrorMetadata, CodeNavigationFeatureFlagRequiredError, CodeNavigationFileNotFoundError, CodeNavigationGraphQLError, @@ -45,6 +46,7 @@ export type MappedErrorCode = export interface MappedErrorDetails { action?: string; + hint?: string; availableVersions?: AvailableVersion[]; availableRefs?: AvailableRef[]; suggestedRefs?: SuggestedRef[]; @@ -127,6 +129,7 @@ function classify(error: unknown): MappedError { } if (error instanceof CodeNavigationVersionNotFoundError) { const details: MappedErrorDetails = {}; + preserveBackendMetadata(details, error.metadata); if (error.packageName) details.package = error.packageName; if (error.requestedVersion) { details.requestedVersion = error.requestedVersion; @@ -144,6 +147,7 @@ function classify(error: unknown): MappedError { } if (error instanceof CodeNavigationTargetNotFoundError) { const details: MappedErrorDetails = {}; + preserveBackendMetadata(details, error.metadata); if (error.availableVersions && error.availableVersions.length > 0) { details.availableVersions = error.availableVersions; } @@ -158,6 +162,7 @@ function classify(error: unknown): MappedError { } if (error instanceof CodeNavigationRefNotFoundError) { const details: MappedErrorDetails = {}; + preserveBackendMetadata(details, error.metadata); if (error.repoUrl) details.repoUrl = error.repoUrl; if (error.requestedRef) details.requestedRef = error.requestedRef; if (error.availableRefs && error.availableRefs.length > 0) { @@ -196,6 +201,7 @@ function classify(error: unknown): MappedError { if (error.indexingEstimate) { details.indexingEstimate = error.indexingEstimate; } + if (error.hint) details.hint = error.hint; return { code: "INDEXING", message: error.message, @@ -302,6 +308,7 @@ export function buildUpdateRequiredError( */ function classifyBackendError(error: CodeNavigationBackendError): MappedError { const details: MappedErrorDetails = {}; + preserveBackendMetadata(details, error.metadata); if (typeof error.status === "number") details.status = error.status; if (error.graphqlCode) details.graphqlCode = error.graphqlCode; @@ -330,6 +337,29 @@ function classifyBackendError(error: CodeNavigationBackendError): MappedError { } } +function preserveBackendMetadata( + details: MappedErrorDetails, + metadata: CodeNavigationErrorMetadata | undefined, +): void { + if (!metadata) return; + if (metadata.hint) details.hint = metadata.hint; + if (metadata.availableVersions?.length) { + details.availableVersions = metadata.availableVersions; + } + if (metadata.availableRefs?.length) { + details.availableRefs = metadata.availableRefs; + } + if (metadata.suggestedRefs?.length) { + details.suggestedRefs = metadata.suggestedRefs; + } + if (metadata.targetResolution) { + details.targetResolution = metadata.targetResolution; + } + if (metadata.indexingEstimate) { + details.indexingEstimate = metadata.indexingEstimate; + } +} + function addRefSuggestions( message: string, refs: AvailableRef[] | undefined, diff --git a/packages/mcp/src/shared/grep-repo-response.test.ts b/packages/mcp/src/shared/grep-repo-response.test.ts index 2318662..151c27f 100644 --- a/packages/mcp/src/shared/grep-repo-response.test.ts +++ b/packages/mcp/src/shared/grep-repo-response.test.ts @@ -4,6 +4,7 @@ import { buildGrepRepoSuccessPayload, formatGrepRepoTerminal, } from "./grep-repo-response.js"; +import { renderGrepRepoText } from "./grep-repo-text.js"; const baseResult: GrepRepoResult = { matches: [ @@ -170,7 +171,7 @@ describe("formatGrepRepoTerminal", () => { expect(stdout).toContain("> 10 const app = express();"); }); - it("verbose mode explains zero-match results", () => { + it("verbose mode explains zero-match results and pattern pivots", () => { const envelope = buildGrepRepoSuccessPayload( { ...baseResult, @@ -180,30 +181,147 @@ describe("formatGrepRepoTerminal", () => { }, baseOptions, ); - const { stdout } = formatGrepRepoTerminal(envelope, { + const { stdout, stderr } = formatGrepRepoTerminal(envelope, { useColors: false, verbose: true, }); expect(stdout).toContain("0 matches in 0 files"); expect(stdout).toContain("No matches."); + expect(stderr).toContain("files scanned: 1 (full scope)"); + expect(stderr).toContain("Do not repeat this grep unchanged."); + expect(stderr).toContain("shorten or change the pattern"); + expect(stderr).toContain("use githits search for conceptual intent"); + expect(stderr).not.toContain("case-sensitive"); + }); + + it("uses CLI syntax when a case-sensitive empty grep can be broadened", () => { + const envelope = buildGrepRepoSuccessPayload( + { + ...baseResult, + matches: [], + totalMatches: 0, + uniqueFilesMatched: 0, + }, + { + ...baseOptions, + caseSensitive: true, + explicit: { ...baseOptions.explicit, caseSensitive: true }, + }, + ); + const { stderr } = formatGrepRepoTerminal(envelope, { + useColors: false, + }); + + expect(stderr).toContain("drop --case-sensitive"); + expect(stderr).not.toContain("case_sensitive"); }); - it("plain mode preserves grep-style silence for zero-match results", () => { + it("plain mode preserves grep-style stdout silence and explains empty scope on stderr", () => { const envelope = buildGrepRepoSuccessPayload( { ...baseResult, matches: [], totalMatches: 0, uniqueFilesMatched: 0, + filesScanned: 0, + filesInScope: 0, }, baseOptions, ); - const { stdout } = formatGrepRepoTerminal(envelope, { + const { stdout, stderr } = formatGrepRepoTerminal(envelope, { useColors: false, }); expect(stdout).toBe(""); + expect(stderr).toContain("files scanned: 0 (no files in scope)"); + expect(stderr).toContain( + "loosen the optional path-prefix argument, --path, --glob, --ext, or exclusion flags", + ); + }); + + it("preserves CLI cursor guidance for an empty incomplete page", () => { + const envelope = buildGrepRepoSuccessPayload( + { + ...baseResult, + matches: [], + totalMatches: 0, + uniqueFilesMatched: 0, + hasMore: true, + nextCursor: "next-page", + }, + baseOptions, + ); + const { stderr } = formatGrepRepoTerminal(envelope, { + useColors: false, + }); + + expect(stderr).toContain("More matches available — rerun"); + expect(stderr).toContain("--cursor 'next-page'"); + expect(stderr).not.toContain("Do not repeat this grep unchanged."); + }); + + it("uses the real CLI limit flag for an empty truncated result", () => { + const envelope = buildGrepRepoSuccessPayload( + { + ...baseResult, + matches: [], + totalMatches: 0, + uniqueFilesMatched: 0, + truncatedReason: "MAX_MATCHES", + }, + baseOptions, + ); + const { stderr } = formatGrepRepoTerminal(envelope, { + useColors: false, + }); + + expect(stderr).toContain("Truncated: match limit reached."); + expect(stderr).toContain("increase --limit"); + expect(stderr).not.toContain("--max-matches"); + }); + + it.each([ + ["MAX_MATCHES", "max_matches", "match limit reached"], + [ + "MAX_MATCHES_PER_FILE", + "max_matches_per_file", + "per-file match limit reached", + ], + ["DEADLINE", "deadline", "time limit reached"], + ] as const)( + "humanizes producer-normalized %s truncation", + (backendReason, normalizedReason, humanReason) => { + const envelope = buildGrepRepoSuccessPayload( + { + ...baseResult, + matches: [], + totalMatches: 0, + uniqueFilesMatched: 0, + truncatedReason: backendReason, + }, + baseOptions, + ); + + expect(envelope.truncatedReason).toBe(normalizedReason); + expect(renderGrepRepoText(envelope)).toContain( + `Truncated: ${humanReason}.`, + ); + expect( + formatGrepRepoTerminal(envelope, { useColors: false }).stderr, + ).toContain(`Truncated: ${humanReason}.`); + }, + ); + + it("humanizes a producer-built truncation trailer with matches", () => { + const envelope = buildGrepRepoSuccessPayload( + { ...baseResult, truncatedReason: "MAX_MATCHES" }, + baseOptions, + ); + + expect(renderGrepRepoText(envelope)).toContain( + "Truncated: match limit reached.", + ); }); it("verbose mode renders minimal symbol hints", () => { @@ -374,7 +492,7 @@ describe("formatGrepRepoTerminal", () => { }); expect(rendered.stderr).toBe( - "More grep results available — rerun with --cursor 'cursor_abc123'\n", + "More matches available — rerun with --cursor 'cursor_abc123'\n", ); }); diff --git a/packages/mcp/src/shared/grep-repo-response.ts b/packages/mcp/src/shared/grep-repo-response.ts index f8b1568..24c5339 100644 --- a/packages/mcp/src/shared/grep-repo-response.ts +++ b/packages/mcp/src/shared/grep-repo-response.ts @@ -1,5 +1,6 @@ import type { GrepRepoMatch, GrepRepoResult } from "@githits/core-internal"; import { colorize, dim, highlightRanges } from "./colors.js"; +import { buildEmptyGrepGuidance } from "./grep-repo-text.js"; import { shellQuote } from "./shell-quote.js"; import { buildTargetResolutionNotes, @@ -586,6 +587,12 @@ function formatTerminalNotes( ): string | undefined { const lines: string[] = []; + if (envelope.matches.length === 0) { + return `${buildEmptyGrepGuidance(envelope, "cli") + .map((line) => dim(line, useColors)) + .join("\n")}\n`; + } + if (shouldSuggestNarrowingScope(envelope)) { lines.push( dim( @@ -598,7 +605,7 @@ function formatTerminalNotes( if (envelope.hasMore && envelope.nextCursor) { lines.push( dim( - `More grep results available — rerun with --cursor ${shellQuote(envelope.nextCursor)}`, + `More matches available — rerun with --cursor ${shellQuote(envelope.nextCursor)}`, useColors, ), ); diff --git a/packages/mcp/src/shared/grep-repo-text.test.ts b/packages/mcp/src/shared/grep-repo-text.test.ts index 569cb3d..0131565 100644 --- a/packages/mcp/src/shared/grep-repo-text.test.ts +++ b/packages/mcp/src/shared/grep-repo-text.test.ts @@ -32,11 +32,51 @@ function match(overrides: Partial = {}): LeanGrepRepoMatch { } describe("renderGrepRepoText", () => { - it("renders zero-match header and empty body", () => { + it("renders scoped zero-match context and pattern pivots", () => { const text = renderGrepRepoText(envelope()); expect(text).toContain("code_grep | 0 matches in 0 files"); expect(text).toContain('pattern="applyEdit"'); expect(text).toContain("No matches."); + expect(text).toContain("files scanned: 120 (full scope)"); + expect(text).toContain("Do not repeat this grep unchanged."); + expect(text).toContain("shorten or change the pattern"); + expect(text).not.toContain("casing"); + expect(text).not.toContain("case_sensitive"); + expect(text).toContain("use search for conceptual intent"); + }); + + it("advises disabling case sensitivity only when it was enabled", () => { + const text = renderGrepRepoText(envelope({ caseSensitive: true })); + + expect(text).toContain("set case_sensitive: false"); + }); + + it("advises loosening selectors when no files are in scope", () => { + const text = renderGrepRepoText( + envelope({ + filesScanned: 0, + filesInScope: 0, + indexedVersion: "v5.2.1", + }), + ); + + expect(text).toContain("files scanned: 0 (no files in scope)"); + expect(text).toContain("served=v5.2.1"); + expect(text).toContain( + "loosen path, path_prefix, globs, extensions, or exclusion filters", + ); + expect(text).not.toContain("use search for conceptual intent"); + }); + + it("explains when the content index pruned files before verification", () => { + const text = renderGrepRepoText( + envelope({ filesScanned: 1, filesInScope: 206 }), + ); + + expect(text).toContain( + "files: 206 in scope | 1 content-scanned after index pruning", + ); + expect(text).not.toContain("files: 1 scanned | 206 in scope"); }); it("renders single-file matches grouped under the file with line gutter", () => { @@ -58,6 +98,7 @@ describe("renderGrepRepoText", () => { " 142: export function applyEdit(input: string): string {", ); expect(text).toContain(" 287: const result = applyEdit(input, hunk);"); + expect(text).not.toContain("Do not repeat this grep unchanged."); }); it("renders matches across multiple files with blank-line separators", () => { @@ -128,31 +169,30 @@ describe("renderGrepRepoText", () => { expect(text).toContain(" --"); }); - it("renders truncation notice when truncatedReason is set", () => { + it("renders next-cursor note when hasMore", () => { const text = renderGrepRepoText( envelope({ totalMatches: 50, uniqueFilesMatched: 7, - truncatedReason: "limit", hasMore: true, + nextCursor: "ABC123", matches: [match()], }), ); - expect(text).toContain("Truncated: limit."); - expect(text).toContain("max_matches"); + expect(text).toContain("More matches available. Pass cursor=ABC123"); }); - it("renders next-cursor note when hasMore", () => { + it("pages an empty incomplete result instead of changing the grep", () => { const text = renderGrepRepoText( envelope({ - totalMatches: 50, - uniqueFilesMatched: 7, hasMore: true, nextCursor: "ABC123", - matches: [match()], }), ); + expect(text).toContain("More matches available. Pass cursor=ABC123"); + expect(text).not.toContain("Do not repeat this grep unchanged."); + expect(text).not.toContain("shorten or change the pattern"); }); it("renders pattern-type and case-sensitive flags in header when set", () => { diff --git a/packages/mcp/src/shared/grep-repo-text.ts b/packages/mcp/src/shared/grep-repo-text.ts index 9eb0796..9cd27e9 100644 --- a/packages/mcp/src/shared/grep-repo-text.ts +++ b/packages/mcp/src/shared/grep-repo-text.ts @@ -15,7 +15,11 @@ import type { LeanGrepRepoEnvelope, LeanGrepRepoMatch, } from "./grep-repo-response.js"; -import { buildTargetResolutionNotes } from "./target-resolution.js"; +import { shellQuote } from "./shell-quote.js"; +import { + buildTargetResolutionNotes, + formatTargetResolutionIdentity, +} from "./target-resolution.js"; const SEP = " | "; @@ -37,11 +41,7 @@ export function renderGrepRepoText(envelope: LeanGrepRepoEnvelope): string { if (envelope.matches.length === 0) { lines.push("No matches."); - const trailer = buildTrailer(envelope); - if (trailer.length > 0) { - lines.push(""); - for (const t of trailer) lines.push(t); - } + for (const note of buildEmptyGrepGuidance(envelope)) lines.push(note); return lines.join("\n"); } @@ -74,6 +74,112 @@ export function renderGrepRepoText(envelope: LeanGrepRepoEnvelope): string { return lines.join("\n"); } +/** Shared empty-result context used by MCP text and CLI terminal output. */ +export function buildEmptyGrepGuidance( + envelope: LeanGrepRepoEnvelope, + surface: "mcp" | "cli" = "mcp", +): string[] { + const lines = [formatEmptyGrepFileCounts(envelope)]; + const served = formatGrepServedTarget(envelope); + if (served) lines.push(served); + for (const note of buildTargetResolutionNotes(envelope.targetResolution)) { + lines.push(note); + } + const skipNotes: string[] = []; + if (envelope.binaryFilesSkipped) { + skipNotes.push(`${envelope.binaryFilesSkipped} binary file(s) skipped`); + } + if (envelope.filesTooLargeSkipped) { + skipNotes.push( + `${envelope.filesTooLargeSkipped} oversized file(s) skipped`, + ); + } + if (skipNotes.length > 0) lines.push(`Note: ${skipNotes.join(", ")}.`); + + if (envelope.truncatedReason) { + const reason = formatTruncationReason(envelope.truncatedReason); + lines.push( + surface === "cli" + ? `Truncated: ${reason}. Narrow the file selectors or increase --limit.` + : `Truncated: ${reason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + ); + } + if (envelope.hasMore && envelope.nextCursor) { + lines.push( + surface === "cli" + ? `More matches available — rerun with --cursor ${shellQuote(envelope.nextCursor)}` + : `More matches available. Pass cursor=${envelope.nextCursor} for the next page.`, + ); + } else if (envelope.hasMore) { + lines.push("More matches available."); + } + if (envelope.truncatedReason || envelope.hasMore) return lines; + + lines.push("Do not repeat this grep unchanged."); + if (envelope.filesInScope === 0) { + lines.push( + surface === "cli" + ? "next: loosen the optional path-prefix argument, --path, --glob, --ext, or exclusion flags." + : "next: loosen path, path_prefix, globs, extensions, or exclusion filters.", + ); + return lines; + } + + const pivots = ["shorten or change the pattern"]; + if (envelope.caseSensitive) { + pivots.push( + surface === "cli" ? "drop --case-sensitive" : "set case_sensitive: false", + ); + } + pivots.push( + surface === "cli" + ? "use githits search for conceptual intent" + : "use search for conceptual intent", + ); + lines.push(`next: ${pivots.join("; ")}.`); + return lines; +} + +function formatEmptyGrepFileCounts(envelope: LeanGrepRepoEnvelope): string { + if (envelope.filesInScope === 0) { + return `files scanned: ${envelope.filesScanned} (no files in scope)`; + } + if (envelope.filesScanned < envelope.filesInScope) { + return `files: ${envelope.filesInScope} in scope | ${envelope.filesScanned} content-scanned after index pruning`; + } + return `files scanned: ${envelope.filesScanned} (full scope)`; +} + +function formatTruncationReason(reason: string): string { + switch (reason) { + case "deadline": + return "time limit reached"; + case "max_matches": + return "match limit reached"; + case "max_matches_per_file": + return "per-file match limit reached"; + default: + return reason; + } +} + +function formatGrepServedTarget( + envelope: LeanGrepRepoEnvelope, +): string | undefined { + const resolved = formatTargetResolutionIdentity( + envelope.targetResolution?.served, + ); + if (resolved) { + const state = envelope.targetResolution?.freshness; + return `target: served=${resolved}${state ? ` | state=${state}` : ""}`; + } + const servedRef = + envelope.indexedVersion ?? + envelope.resolution?.resolvedRef ?? + envelope.gitRef; + return servedRef ? `target: served=${servedRef}` : undefined; +} + function buildHeader(envelope: LeanGrepRepoEnvelope): string { const parts = [ `code_grep${SEP}${envelope.totalMatches} match${ @@ -97,7 +203,7 @@ function buildTrailer(envelope: LeanGrepRepoEnvelope): string[] { if (envelope.truncatedReason) { lines.push( - `Truncated: ${envelope.truncatedReason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + `Truncated: ${formatTruncationReason(envelope.truncatedReason)}. Pass narrower path/path_prefix/globs or increase max_matches.`, ); } diff --git a/packages/mcp/src/shared/repository-target.test.ts b/packages/mcp/src/shared/repository-target.test.ts index de66321..ddafe61 100644 --- a/packages/mcp/src/shared/repository-target.test.ts +++ b/packages/mcp/src/shared/repository-target.test.ts @@ -28,4 +28,10 @@ describe("formatRepositoryTargetLabel", () => { it("does not rewrite package-style labels", () => { expect(formatRepositoryTargetLabel("npm:express@5.2.1")).toBeUndefined(); }); + + it("does not URL-encode human-readable default-branch labels", () => { + expect( + formatRepositoryTargetLabel("expressjs/express default branch"), + ).toBeUndefined(); + }); }); diff --git a/packages/mcp/src/shared/repository-target.ts b/packages/mcp/src/shared/repository-target.ts index 17f4207..b0e5de3 100644 --- a/packages/mcp/src/shared/repository-target.ts +++ b/packages/mcp/src/shared/repository-target.ts @@ -140,6 +140,12 @@ export function formatRepositoryTargetLabel(label: string): string | undefined { const [owner, repoName, ...rest] = repoLabel.split("/"); if (!owner || !repoName || rest.length > 0) return undefined; if (owner.includes(":") || repoName.includes(":")) return undefined; + if ( + !GITHUB_OWNER_PATTERN.test(owner) || + !GITHUB_REPO_PATTERN.test(repoName) + ) { + return undefined; + } const gitRef = atRefDelimiter === -1 ? undefined : label.slice(atRefDelimiter + 1); if (gitRef === "") return undefined; diff --git a/packages/mcp/src/shared/search-lifecycle.ts b/packages/mcp/src/shared/search-lifecycle.ts new file mode 100644 index 0000000..b4d6bfe --- /dev/null +++ b/packages/mcp/src/shared/search-lifecycle.ts @@ -0,0 +1,7 @@ +/** + * Fully healthy lifecycle states. STALE remains searchable but is not healthy: + * render it as provenance, while warnings stay conditional on target divergence. + */ +export function isHealthySearchLifecycleState(state: string): boolean { + return state === "INDEXED" || state === "CURRENT"; +} diff --git a/packages/mcp/src/shared/unified-search-response.test.ts b/packages/mcp/src/shared/unified-search-response.test.ts index 6e95c4b..d2543b6 100644 --- a/packages/mcp/src/shared/unified-search-response.test.ts +++ b/packages/mcp/src/shared/unified-search-response.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; import { + CodeNavigationIndexingError, CodeNavigationRefNotFoundError, CodeNavigationTargetNotFoundError, type UnifiedSearchOutcome, @@ -13,6 +14,59 @@ import { buildUnifiedSearchSuccessPayload, } from "./unified-search-response.js"; +describe("buildUnifiedSearchErrorPayload", () => { + it("preserves backend indexing guidance, estimates, and alternatives", () => { + const payload = buildUnifiedSearchErrorPayload( + new CodeNavigationIndexingError( + "Backend indexing message.", + "idx-42", + [{ version: "5.2.1", ref: "v5.2.1" }], + [{ ref: "main" }], + undefined, + { lowerSeconds: 7, upperSeconds: 19, sampleCount: 9 }, + "Backend indexing hint.", + ), + ); + + expect(payload).toEqual({ + error: "Backend indexing message.", + code: "INDEXING", + retryable: true, + details: { + indexingRef: "idx-42", + availableVersions: [{ version: "5.2.1", ref: "v5.2.1" }], + availableRefs: [{ ref: "main" }], + indexingEstimate: { lowerSeconds: 7, upperSeconds: 19, sampleCount: 9 }, + hint: "Backend indexing hint.", + }, + }); + }); + + it("preserves backend not-found messages and alternatives", () => { + const payload = buildUnifiedSearchErrorPayload( + new CodeNavigationTargetNotFoundError( + "Backend target message.", + [{ version: "5.2.1", ref: "v5.2.1" }], + undefined, + undefined, + { + hint: "Backend target hint.", + availableRefs: [{ ref: "main" }], + suggestedRefs: [{ ref: "v5.2.1" }], + }, + ), + ); + + expect(payload.error).toBe("Backend target message."); + expect(payload.details?.availableVersions).toEqual([ + { version: "5.2.1", ref: "v5.2.1" }, + ]); + expect(payload.details?.hint).toBe("Backend target hint."); + expect(payload.details?.availableRefs).toEqual([{ ref: "main" }]); + expect(payload.details?.suggestedRefs).toEqual([{ ref: "v5.2.1" }]); + }); +}); + describe("buildUnifiedSearchSuccessPayload", () => { const params: UnifiedSearchParams = { targets: [{ registry: "NPM", packageName: "express" }], @@ -165,7 +219,7 @@ describe("buildUnifiedSearchSuccessPayload", () => { targetsTotal: 1, elapsedMs: 200, query: "router middleware", - next: 'search_status search_ref="search-ref-123"', + next: 'search_status search_ref="search-ref-123" wait_timeout_ms=20000', }, }); }); @@ -849,7 +903,12 @@ describe("buildUnifiedSearchSuccessPayload", () => { }, ); - expect(payload.sourceStatus).toBeUndefined(); + expect(payload.sourceStatus).toEqual([ + { + source: "docs", + targetLabel: "site:expressjs.com", + }, + ]); expect(payload.warnings).toBeUndefined(); }); @@ -1158,7 +1217,7 @@ describe("buildUnifiedSearchSuccessPayload — sourceStatus warnings on complete expect(payload.warnings).toBeUndefined(); }); - it("omits indexing-only targetResolution on completed empty results", () => { + it("retains source and target context on completed empty results", () => { if (defaultUnifiedSearchOutcome.state !== "completed") { throw new Error("expected completed fixture"); } @@ -1176,8 +1235,12 @@ describe("buildUnifiedSearchSuccessPayload — sourceStatus warnings on complete { source: "CODE", targetLabel: "github:expressjs/express#master", + requestedTargetLabel: "expressjs/express default branch", + freshTargetLabel: "expressjs/express@master", + servedTargetLabel: "expressjs/express@master", indexingStatus: "INDEXING", codeIndexState: "INDEXING", + resultCount: 0, appliedFilters: [], ignoredFilters: [], incompatibleFilters: [], @@ -1215,7 +1278,100 @@ describe("buildUnifiedSearchSuccessPayload — sourceStatus warnings on complete expect(payload.completed).toBe(true); expect(payload.results).toEqual([]); expect(payload.warnings).toBeUndefined(); - expect(payload.sourceStatus).toBeUndefined(); + expect(payload.sourceStatus?.[0]).toMatchObject({ + source: "code", + targetLabel: "github:expressjs/express#master", + requestedTarget: "expressjs/express default branch", + servedTarget: "github:expressjs/express#master", + indexingStatus: "INDEXING", + codeIndexState: "INDEXING", + resultCount: 0, + }); + }); + + it("does not warn for healthy lifecycle states on completed empty results", () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed fixture"); + } + const sourceStatus = defaultUnifiedSearchOutcome.result.sourceStatus[0]; + if (!sourceStatus) throw new Error("expected source status fixture"); + const outcome: UnifiedSearchOutcome = { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + results: [], + page: { + ...defaultUnifiedSearchOutcome.result.page, + returned: 0, + hasMore: false, + }, + sourceStatus: [ + { + ...sourceStatus, + indexingStatus: "INDEXED", + codeIndexState: "CURRENT", + resultCount: 0, + }, + ], + }, + }; + + const payload = buildUnifiedSearchSuccessPayload( + params, + "router middleware", + "router middleware", + outcome, + ); + + expect(payload.warnings).toBeUndefined(); + expect(payload.sourceStatus?.[0]).toMatchObject({ + indexingStatus: "INDEXED", + codeIndexState: "CURRENT", + resultCount: 0, + }); + }); + + it("omits redundant requested and fresh labels on completed empty results", () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed fixture"); + } + const sourceStatus = defaultUnifiedSearchOutcome.result.sourceStatus[0]; + if (!sourceStatus) throw new Error("expected source status fixture"); + const outcome: UnifiedSearchOutcome = { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + results: [], + page: { + ...defaultUnifiedSearchOutcome.result.page, + returned: 0, + hasMore: false, + }, + sourceStatus: [ + { + ...sourceStatus, + targetLabel: "npm:express@5.2.1", + requestedTargetLabel: "npm:express@5.2.1", + freshTargetLabel: "npm:express@v5.2.1", + servedTargetLabel: "npm:express@5.2.1", + indexingStatus: "INDEXED", + codeIndexState: "CURRENT", + resultCount: 0, + }, + ], + }, + }; + + const payload = buildUnifiedSearchSuccessPayload( + params, + "router middleware", + "router middleware", + outcome, + ); + + expect(payload.sourceStatus?.[0]?.servedTarget).toBe("npm:express@5.2.1"); + expect(payload.sourceStatus?.[0]?.requestedTarget).toBeUndefined(); + expect(payload.sourceStatus?.[0]?.freshTarget).toBeUndefined(); }); it("omits warnings[] for current targetResolution on floating repo targets", () => { @@ -1401,7 +1557,7 @@ describe("buildUnifiedSearchStatusPayload", () => { targetsTotal: 1, elapsedMs: 200, query: "router middleware", - next: 'search_status search_ref="search-ref-123"', + next: 'search_status search_ref="search-ref-123" wait_timeout_ms=20000', }, }); }); @@ -1435,4 +1591,29 @@ describe("buildUnifiedSearchStatusPayload", () => { ], }); }); + + it.each(["FAILED", "TIMEOUT"] as const)( + "replaces status polling for a terminal %s session", + (status) => { + const payload = buildUnifiedSearchStatusPayload({ + state: "incomplete", + completed: false, + searchRef: `search-ref-${status.toLowerCase()}`, + progress: { + searchRef: `search-ref-${status.toLowerCase()}`, + status, + targetsTotal: 1, + targetsReady: 0, + elapsedMs: 60_000, + query: "router middleware", + queryWarnings: [], + sources: ["CODE"], + }, + }); + if (payload.completed) throw new Error("expected incomplete payload"); + + expect(payload.progress?.next).toBe("rerun search"); + expect(payload.progress?.next).not.toContain("search_status"); + }, + ); }); diff --git a/packages/mcp/src/shared/unified-search-response.ts b/packages/mcp/src/shared/unified-search-response.ts index 2c7f9e9..9e81564 100644 --- a/packages/mcp/src/shared/unified-search-response.ts +++ b/packages/mcp/src/shared/unified-search-response.ts @@ -12,6 +12,7 @@ import { DEFAULT_WAIT_TIMEOUT_MS } from "./code-navigation-defaults.js"; import { mapCodeNavigationError } from "./code-navigation-error-map.js"; import { buildSearchHitFollowUpCommand } from "./follow-up-command-text.js"; import { formatRepositoryTargetLabel } from "./repository-target.js"; +import { isHealthySearchLifecycleState } from "./search-lifecycle.js"; import { buildResolutionFromRetryCandidates, buildTargetResolutionNotes, @@ -270,6 +271,7 @@ export function buildUnifiedSearchSuccessPayload( if (outcome.searchRef) completed.searchRef = outcome.searchRef; const sourceStatus = compactSourceStatus(outcome.result.sourceStatus, { completed: true, + includeEmptyResultContext: completed.results.length === 0, }); if (sourceStatus) completed.sourceStatus = sourceStatus; const combinedWarnings = combineWarnings( @@ -377,7 +379,10 @@ function buildUnifiedSearchStatusResultPayload( if (result.sources.length > 0) { payload.sources = result.sources.map((entry) => entry.toLowerCase()); } - const sourceStatus = compactSourceStatus(result.sourceStatus, options); + const sourceStatus = compactSourceStatus(result.sourceStatus, { + ...options, + includeEmptyResultContext: options.completed && result.results.length === 0, + }); if (sourceStatus) payload.sourceStatus = sourceStatus; const combinedWarnings = combineWarnings( result.queryWarnings, @@ -557,7 +562,10 @@ function compactProgress( >; } if (progress.expiresAt) payload.expiresAt = progress.expiresAt; - payload.next = `search_status search_ref=${JSON.stringify(progress.searchRef)}`; + payload.next = + progress.status === "FAILED" || progress.status === "TIMEOUT" + ? "rerun search" + : `search_status search_ref=${JSON.stringify(progress.searchRef)} wait_timeout_ms=${DEFAULT_WAIT_TIMEOUT_MS}`; return payload; } @@ -848,14 +856,14 @@ function warningForEntry( if (entry.ignoredFilters?.length) { reasons.push(`ignored filters [${entry.ignoredFilters.join(", ")}]`); } - // Healthy lifecycle states (`INDEXED`, `CURRENT`, `STALE`) are - // already filtered out upstream in `compactSourceStatusEntry`; if - // these fields are present here, the state is genuinely worth - // surfacing. + // Empty completed results intentionally retain lifecycle context. STALE is + // usable and warns only when target divergence produced a freshness warning. if ( !terminalLifecycleReason && reasons.length === 0 && entry.indexingStatus && + !isHealthySearchLifecycleState(entry.indexingStatus) && + entry.indexingStatus !== "STALE" && !(entry.indexingStatus === "INDEXING" && options.completed) ) { reasons.push(`indexing status ${entry.indexingStatus}`); @@ -866,6 +874,7 @@ function warningForEntry( entry.codeIndexState ) { if ( + !isHealthySearchLifecycleState(entry.codeIndexState) && entry.codeIndexState !== "STALE" && !(entry.codeIndexState === "INDEXING" && options.completed) ) { @@ -901,7 +910,10 @@ function terminalLifecycleWarningReason( new Set([entry.indexingStatus, entry.codeIndexState].filter(Boolean)), ) as string[]; const terminalStates = states.filter( - (state) => state !== "INDEXING" && state !== "STALE", + (state) => + !isHealthySearchLifecycleState(state) && + state !== "INDEXING" && + state !== "STALE", ); if (terminalStates.length === 0) return undefined; const status = terminalStates.join("/"); @@ -958,7 +970,10 @@ function projectDocCoverage( function compactSourceStatus( sourceStatus: UnifiedSearchSourceStatus[] | undefined, - options: { completed?: boolean } = {}, + options: { + completed?: boolean; + includeEmptyResultContext?: boolean; + } = {}, ): UnifiedSearchSourceStatusPayload[] | undefined { if (!sourceStatus || sourceStatus.length === 0) return undefined; const compact: UnifiedSearchSourceStatusPayload[] = []; @@ -971,7 +986,10 @@ function compactSourceStatus( function compactSourceStatusEntry( entry: UnifiedSearchSourceStatus, - options: { completed?: boolean }, + options: { + completed?: boolean; + includeEmptyResultContext?: boolean; + }, ): UnifiedSearchSourceStatusPayload | undefined { const payload: UnifiedSearchSourceStatusPayload = { source: entry.source.toLowerCase(), @@ -979,6 +997,40 @@ function compactSourceStatusEntry( }; let interesting = false; + if (options.includeEmptyResultContext) { + const servedTarget = entry.servedTargetLabel + ? formatTargetLabel(entry.servedTargetLabel) + : undefined; + const comparisonTarget = servedTarget ?? payload.targetLabel; + const requestedTarget = entry.requestedTargetLabel + ? formatTargetLabel(entry.requestedTargetLabel) + : undefined; + const freshTarget = entry.freshTargetLabel + ? formatTargetLabel(entry.freshTargetLabel) + : undefined; + if ( + requestedTarget && + canonicalTargetLabel(requestedTarget) !== + canonicalTargetLabel(comparisonTarget) + ) { + payload.requestedTarget = requestedTarget; + } + if ( + freshTarget && + canonicalTargetLabel(freshTarget) !== + canonicalTargetLabel(comparisonTarget) + ) { + payload.freshTarget = freshTarget; + } + if (servedTarget) payload.servedTarget = servedTarget; + if (entry.indexingStatus) payload.indexingStatus = entry.indexingStatus; + if (entry.codeIndexState) payload.codeIndexState = entry.codeIndexState; + if (typeof entry.resultCount === "number") { + payload.resultCount = entry.resultCount; + } + interesting = true; + } + const staleDiverges = entry.codeIndexState === "STALE" && labelsDiverge({ @@ -1039,7 +1091,11 @@ function compactSourceStatusEntry( payload.coverage = coverage; interesting = true; } - if (typeof entry.resultCount === "number" && entry.resultCount > 0) { + if ( + !options.includeEmptyResultContext && + typeof entry.resultCount === "number" && + entry.resultCount > 0 + ) { payload.resultCount = entry.resultCount; } if (entry.ignoredFilters.length > 0) { diff --git a/packages/mcp/src/shared/unified-search-status-text.ts b/packages/mcp/src/shared/unified-search-status-text.ts index 1a24de4..1ccd1ec 100644 --- a/packages/mcp/src/shared/unified-search-status-text.ts +++ b/packages/mcp/src/shared/unified-search-status-text.ts @@ -4,9 +4,12 @@ import type { UnifiedSearchStatusResultPayload, } from "./unified-search-response.js"; import { + appendEmptySearchGuidance, + appendIncompleteSearchNextAction, + appendSourceStatusNotes, appendUnifiedSearchHits, formatProgressTarget, - formatSourceStatus, + noHitsYetMessage, } from "./unified-search-text.js"; const SEP = " | "; @@ -35,11 +38,20 @@ export function renderUnifiedSearchStatusText(payload: StatusPayload): string { } const result = payload.result; - if (result) appendResult(lines, result); + if (result) { + appendResult( + lines, + result, + payload.completed, + payload.completed ? undefined : payload.progress, + ); + } if (!payload.completed) { - lines.push( - `next: call search_status search_ref=${quote(payload.searchRef)}`, + appendIncompleteSearchNextAction( + lines, + payload.progress?.status, + payload.searchRef, ); } @@ -58,6 +70,8 @@ function buildHeader(payload: StatusPayload): string { function appendResult( lines: string[], result: UnifiedSearchStatusResultPayload, + completed: boolean, + progress: UnifiedSearchStatusIncompletePayload["progress"] | undefined, ): void { lines.push(""); if (result.warnings && result.warnings.length > 0) { @@ -66,7 +80,17 @@ function appendResult( lines.push(""); } if (result.results.length === 0) { - lines.push("No hits."); + if (completed) { + appendSourceStatusNotes(lines, result.sourceStatus); + if (result.sourceStatus?.length) lines.push(""); + appendEmptySearchGuidance(lines, { + query: result.query, + showQuery: true, + sourceStatus: result.sourceStatus, + }); + } else { + lines.push(noHitsYetMessage(progress)); + } } else { appendUnifiedSearchHits(lines, result.results); } @@ -78,12 +102,13 @@ function appendResult( lines.push(""); lines.push(`More hits available.${nextOffsetHint}`); } - if (result.sourceStatus && result.sourceStatus.length > 0) { + if ( + result.results.length > 0 && + result.sourceStatus && + result.sourceStatus.length > 0 + ) { lines.push(""); - lines.push("source notes:"); - for (const entry of result.sourceStatus) { - lines.push(` - ${formatSourceStatus(entry)}`); - } + appendSourceStatusNotes(lines, result.sourceStatus); } } @@ -92,12 +117,6 @@ function formatProgress(progress: { targetsReady: number; targetsTotal: number; elapsedMs: number; - next?: string; }): string { - const next = progress.next ? `; next: ${progress.next}` : ""; - return `progress: ${progress.status}, ${progress.targetsReady}/${progress.targetsTotal} targets ready, ${progress.elapsedMs}ms elapsed${next}`; -} - -function quote(value: string): string { - return JSON.stringify(value); + return `progress: ${progress.status}, ${progress.targetsReady}/${progress.targetsTotal} targets ready, ${progress.elapsedMs}ms elapsed`; } diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index 4594654..ddf8d9c 100644 --- a/packages/mcp/src/shared/unified-search-text.test.ts +++ b/packages/mcp/src/shared/unified-search-text.test.ts @@ -78,11 +78,153 @@ function completed( } describe("renderUnifiedSearchSuccess", () => { - it("renders an empty completed envelope with a clear message", () => { - const text = renderUnifiedSearchSuccess(completed([])); + it("renders an empty completed envelope with bounded anti-retry guidance", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + query: { + raw: "diff myers", + filters: { kind: "function" }, + }, + sourceStatus: [ + { + source: "code", + targetLabel: "npm:express@5.2.1", + requestedTarget: "npm:express latest", + servedTarget: "npm:express@5.2.1", + codeIndexState: "CURRENT", + resultCount: 0, + }, + ], + }), + ); expect(text).toContain("0 hits"); expect(text).toContain('query="diff myers"'); - expect(text).toContain("No hits."); + expect(text).toContain( + "No hits for code on npm:express@5.2.1 (requested npm:express latest; current).", + ); + expect(text).toContain("Do not repeat this search unchanged."); + expect(text).toContain("shorten or broaden the query"); + expect(text).toContain("remove restrictive filters"); + expect(text).toContain('source="symbol"'); + expect(text).toContain("known literal or regex"); + }); + + it("directs completed indexing results to wait or indexed alternatives", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + sourceStatus: [ + { + source: "code", + targetLabel: "npm:express@5.2.1", + servedTarget: "npm:express@5.2.1", + indexingStatus: "INDEXING", + codeIndexState: "INDEXING", + resultCount: 0, + targetResolution: { + freshness: "indexing", + availableVersions: [{ version: "5.1.0", ref: "v5.1.0" }], + availableRefs: [], + }, + }, + ], + }), + ); + + expect(text).toContain("wait_timeout_ms"); + expect(text).toContain("queryable now"); + expect(text).not.toContain("shorten or broaden the query"); + }); + + it("does not suggest symbol search when already using the symbol source", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + query: { raw: "Router", sources: ["symbol"] }, + }), + ); + + expect(text).not.toContain('source="symbol"'); + expect(text).not.toContain("remove restrictive filters"); + }); + + it("does not call explicit public_only=false restrictive", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + query: { raw: "Router", filters: { publicOnly: false } }, + }), + ); + + expect(text).not.toContain("remove restrictive filters"); + }); + + it("does not suggest code_grep for a standalone docs site", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + query: { raw: "middleware", sources: ["docs"] }, + sourceStatus: [ + { + source: "docs", + targetLabel: "site:expressjs.com", + resultCount: 0, + targetResolution: { + requested: { site: "site:expressjs.com" }, + served: { site: "site:expressjs.com" }, + freshness: "current", + availableVersions: [], + availableRefs: [], + }, + }, + ], + }), + ); + + expect(text).not.toContain("code_grep"); + }); + + it("prefers a failed lifecycle state over a healthy sibling", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + warnings: ["Source 'code' for npm:express@5.2.1: status FAILED"], + sourceStatus: [ + { + source: "code", + targetLabel: "npm:express@5.2.1", + servedTarget: "npm:express@5.2.1", + indexingStatus: "FAILED", + codeIndexState: "CURRENT", + resultCount: 0, + }, + ], + }), + ); + + expect(text).toContain("No hits for code on npm:express@5.2.1 (failed)."); + expect(text).not.toContain( + "No hits for code on npm:express@5.2.1 (current).", + ); + }); + + it("prefers a stale lifecycle state over a healthy sibling", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + sourceStatus: [ + { + source: "code", + targetLabel: "npm:express@5.2.1", + servedTarget: "npm:express@5.2.1", + indexingStatus: "STALE", + codeIndexState: "CURRENT", + resultCount: 0, + }, + ], + }), + ); + + expect(text).toContain( + "No hits for code on npm:express@5.2.1 (previous-snapshot).", + ); + expect(text).not.toContain( + "No hits for code on npm:express@5.2.1 (current).", + ); }); it("renders a single code hit with locator, title, and summary", () => { @@ -153,9 +295,67 @@ describe("renderUnifiedSearchSuccess", () => { const text = renderUnifiedSearchSuccess(incomplete); expect(text).toContain("1 partial"); expect(text).toContain("searchRef=ref_abc-123"); + expect(text).toContain("Indexing in progress.\nDo not repeat search."); expect(text).toContain( - "Indexing in progress. Call search_status with searchRef=ref_abc-123", + 'next: call search_status with search_ref="ref_abc-123" and wait_timeout_ms=20000.', ); + expect(text).not.toContain("searchRef=ref_abc-123 to follow up"); + }); + + it.each(["FAILED", "TIMEOUT"] as const)( + "stops polling a terminal %s session", + (status) => { + const incomplete: UnifiedSearchIncompletePayload = { + query: { raw: "myers" }, + completed: false, + hasMore: false, + results: [], + searchRef: `ref-${status.toLowerCase()}`, + progress: { + status, + targetsReady: 0, + targetsTotal: 1, + elapsedMs: 20_000, + }, + }; + + const text = renderUnifiedSearchSuccess(incomplete); + expect(text).toContain( + "Do not call search_status again for this session.", + ); + expect(text).toContain("next: rerun search"); + expect(text).not.toContain("next: call search_status"); + }, + ); + + it("labels deferred indexed alternatives as immediately queryable", () => { + const incomplete: UnifiedSearchIncompletePayload = { + query: { raw: "router" }, + completed: false, + hasMore: false, + results: [], + searchRef: "ref-indexing", + progress: { + status: "INDEXING", + targetsReady: 0, + targetsTotal: 1, + elapsedMs: 100, + targets: [ + { + requested: "npm:express latest", + availableVersions: [{ version: "4.18.2", ref: "v4.18.2" }], + availableRefs: [{ ref: "main" }], + }, + ], + }, + }; + + const text = renderUnifiedSearchSuccess(incomplete); + expect(text).toContain("0/1 targets"); + expect(text).toContain( + "queryable now: versions=4.18.2@v4.18.2 | refs=main", + ); + expect(text).not.toContain("allow_partial_results"); }); it("wraps long summaries at the configured width", () => { @@ -216,6 +416,31 @@ describe("renderUnifiedSearchSuccess", () => { ); // Source notes block still rendered for structured detail. expect(text).toContain("source notes:"); + expect(text.indexOf("warnings:")).toBeLessThan( + text.indexOf("Do not repeat this search unchanged."), + ); + }); + + it("uses a compact headline when every requested source is empty", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + sourceStatus: [ + { + source: "code", + targetLabel: "npm:zod@4.3.6", + resultCount: 0, + }, + { + source: "docs", + targetLabel: "npm:zod@4.3.6", + resultCount: 0, + }, + ], + }), + ); + + expect(text).toContain("No hits from any source (code, docs)."); + expect(text).not.toContain("No hits across"); }); it("uses requestedRef when repo follow-up lacks served gitRef", () => { @@ -278,6 +503,8 @@ describe("renderUnifiedSearchSuccess", () => { it("omits the warnings preamble when no warnings are present", () => { const text = renderUnifiedSearchSuccess(completed([codeHit()])); expect(text).not.toContain("warnings:"); + expect(text).not.toContain("Do not repeat this search unchanged."); + expect(text).not.toContain('source="symbol"'); }); it("separates multiple hits with a blank line", () => { diff --git a/packages/mcp/src/shared/unified-search-text.ts b/packages/mcp/src/shared/unified-search-text.ts index a0870ae..daa4ac4 100644 --- a/packages/mcp/src/shared/unified-search-text.ts +++ b/packages/mcp/src/shared/unified-search-text.ts @@ -15,10 +15,13 @@ * `docs/implementation/tools.md` when changing the format. */ +import { DEFAULT_WAIT_TIMEOUT_MS } from "./code-navigation-defaults.js"; import { buildSearchHitFollowUpCommand } from "./follow-up-command-text.js"; +import { isHealthySearchLifecycleState } from "./search-lifecycle.js"; import { buildResolutionFromRetryCandidates, buildTargetResolutionNotes, + formatTargetResolutionIdentity, type LeanTargetResolution, } from "./target-resolution.js"; import type { @@ -26,6 +29,7 @@ import type { UnifiedSearchErrorPayload, UnifiedSearchHitPayload, UnifiedSearchIncompletePayload, + UnifiedSearchQueryEcho, } from "./unified-search-response.js"; const SUMMARY_WRAP_WIDTH = 76; @@ -43,13 +47,27 @@ export function renderUnifiedSearchSuccess( lines.push(buildHeader(payload)); lines.push(""); - if (payload.results.length === 0) { - lines.push(payload.completed ? "No hits." : noHitsYetMessage(payload)); + const completedEmpty = payload.completed && payload.results.length === 0; + if (completedEmpty) { + appendWarnings(lines, payload.warnings); + appendSourceStatusNotes(lines, payload.sourceStatus); + if (lines[lines.length - 1] !== "") lines.push(""); + appendEmptySearchGuidance(lines, { + query: payload.query, + sourceStatus: payload.sourceStatus, + }); + } else if (payload.results.length === 0) { + lines.push( + noHitsYetMessage("progress" in payload ? payload.progress : undefined), + ); } else { appendUnifiedSearchHits(lines, payload.results); } - const trailer = buildTrailer(payload); + const trailer = buildTrailer(payload, { + includeWarnings: !completedEmpty, + includeSourceStatus: !completedEmpty, + }); if (trailer.length > 0) { lines.push(""); for (const line of trailer) lines.push(line); @@ -58,10 +76,11 @@ export function renderUnifiedSearchSuccess( return lines.join("\n"); } -function noHitsYetMessage(payload: SearchSuccessPayload): string { - if (payload.completed) return "No hits."; - const status = payload.progress?.status; - if (status === "TIMEOUT") return "No hits yet - timed out waiting."; +export function noHitsYetMessage( + progress: { status?: string } | undefined, +): string { + const status = progress?.status; + if (status === "TIMEOUT") return "No hits - search timed out."; if (status === "FAILED") return "No hits - search failed."; if (status === "SEARCHING") return "No hits yet - searching."; return "No hits yet - indexing."; @@ -216,15 +235,13 @@ function formatLineRange(start?: number, end?: number): string { return `:${start}-${end}`; } -function buildTrailer(payload: SearchSuccessPayload): string[] { +function buildTrailer( + payload: SearchSuccessPayload, + options: { includeWarnings: boolean; includeSourceStatus: boolean }, +): string[] { const lines: string[] = []; - if (payload.warnings && payload.warnings.length > 0) { - lines.push("warnings:"); - for (const warning of payload.warnings) { - lines.push(` - ${warning}`); - } - } + if (options.includeWarnings) appendWarnings(lines, payload.warnings); if (payload.hasMore) { const nextOffsetHint = @@ -238,22 +255,23 @@ function buildTrailer(payload: SearchSuccessPayload): string[] { const status = payload.progress?.status; const action = status === "TIMEOUT" - ? "Search timed out waiting for fresh data." + ? "Search timed out before completion." : status === "FAILED" ? "Search failed before completion." : status === "SEARCHING" ? "Search in progress." : "Indexing in progress."; - lines.push( - `${action} Call search_status with searchRef=${payload.searchRef} to follow up.`, - ); + if (payload.progress) { + lines.push( + `progress: ${payload.progress.targetsReady}/${payload.progress.targetsTotal} targets ready.`, + ); + } + lines.push(action); + appendIncompleteSearchNextAction(lines, status, payload.searchRef); } - if (payload.sourceStatus && payload.sourceStatus.length > 0) { - lines.push("source notes:"); - for (const entry of payload.sourceStatus) { - lines.push(` - ${formatSourceStatus(entry)}`); - } + if (options.includeSourceStatus) { + appendSourceStatusNotes(lines, payload.sourceStatus); } const progress = "progress" in payload ? payload.progress : undefined; @@ -267,6 +285,163 @@ function buildTrailer(payload: SearchSuccessPayload): string[] { return lines; } +export function appendIncompleteSearchNextAction( + lines: string[], + status: string | undefined, + searchRef: string, +): void { + if (status === "FAILED" || status === "TIMEOUT") { + lines.push("Do not call search_status again for this session."); + lines.push("next: rerun search."); + return; + } + + lines.push("Do not repeat search."); + lines.push( + `next: call search_status with search_ref=${JSON.stringify(searchRef)} and wait_timeout_ms=${DEFAULT_WAIT_TIMEOUT_MS}.`, + ); +} + +function appendWarnings(lines: string[], warnings: string[] | undefined): void { + if (!warnings || warnings.length === 0) return; + lines.push("warnings:"); + for (const warning of warnings) lines.push(` - ${warning}`); +} + +export function appendSourceStatusNotes( + lines: string[], + sourceStatus: + | UnifiedSearchCompletedPayload["sourceStatus"] + | UnifiedSearchIncompletePayload["sourceStatus"], +): void { + if (!sourceStatus || sourceStatus.length === 0) return; + lines.push("source notes:"); + for (const entry of sourceStatus) { + lines.push(` - ${formatSourceStatus(entry)}`); + } +} + +export function appendEmptySearchGuidance( + lines: string[], + options: { + query?: UnifiedSearchQueryEcho; + showQuery?: boolean; + sourceStatus?: UnifiedSearchCompletedPayload["sourceStatus"]; + }, +): void { + if (options.showQuery && options.query?.raw) { + lines.push(`query=${quote(options.query.raw)}`); + } + lines.push(formatEmptySearchHeadline(options.sourceStatus)); + lines.push("Do not repeat this search unchanged."); + if (hasIndexingSource(options.sourceStatus)) { + const hasAlternatives = options.sourceStatus?.some( + (entry) => + Boolean(entry.targetResolution?.availableVersions.length) || + Boolean(entry.targetResolution?.availableRefs.length), + ); + lines.push( + hasAlternatives + ? 'next: query an indexed version/ref labelled "queryable now", or rerun with a larger wait_timeout_ms to wait for indexing.' + : "next: rerun with a larger wait_timeout_ms to wait for indexing.", + ); + return; + } + + const pivots = ["shorten or broaden the query"]; + if (hasRestrictiveSearchFilters(options.query)) { + pivots.push("remove restrictive filters"); + } + if (!options.query?.sources?.includes("symbol")) { + pivots.push('use source="symbol" for an exact API/entity name'); + } + if (!isStandaloneSiteSearch(options.sourceStatus)) { + pivots.push("use code_grep for a known literal or regex"); + } + lines.push(`next: ${pivots.join("; ")}.`); +} + +function hasIndexingSource( + sourceStatus: UnifiedSearchCompletedPayload["sourceStatus"], +): boolean { + return Boolean( + sourceStatus?.some( + (entry) => + entry.targetResolution?.freshness === "indexing" || + entry.indexingStatus === "INDEXING" || + entry.codeIndexState === "INDEXING", + ), + ); +} + +function hasRestrictiveSearchFilters( + query: UnifiedSearchQueryEcho | undefined, +): boolean { + const filters = query?.filters; + return Boolean( + filters?.kind || + filters?.category || + filters?.pathPrefix || + filters?.fileIntent || + filters?.publicOnly === true || + (query?.raw && + /(?:^|\s)(?:kind|category|path|lang|name|intent):/i.test(query.raw)), + ); +} + +function isStandaloneSiteSearch( + sourceStatus: UnifiedSearchCompletedPayload["sourceStatus"], +): boolean { + return Boolean( + sourceStatus?.length && + sourceStatus.every((entry) => { + const resolution = entry.targetResolution; + return Boolean( + entry.targetLabel.startsWith("site:") || + resolution?.requested?.site || + resolution?.resolvedRequested?.site || + resolution?.served?.site, + ); + }), + ); +} + +function formatEmptySearchHeadline( + sourceStatus: UnifiedSearchCompletedPayload["sourceStatus"], +): string { + if (!sourceStatus || sourceStatus.length === 0) return "No hits."; + if (sourceStatus.length > 1) { + const sources = Array.from( + new Set(sourceStatus.map((entry) => entry.source)), + ).join(", "); + return `No hits from any source (${sources}).`; + } + + const entry = sourceStatus[0]; + if (!entry) return "No hits."; + const served = + entry.servedTarget ?? + formatTargetResolutionIdentity(entry.targetResolution?.served) ?? + entry.targetLabel; + const requested = + entry.requestedTarget ?? + formatTargetResolutionIdentity(entry.targetResolution?.requested); + // STALE is headline-worthy provenance even when it is not warning-worthy. + const unhealthyIndexState = [entry.indexingStatus, entry.codeIndexState].find( + (state) => state && !isHealthySearchLifecycleState(state), + ); + const freshness = + unhealthyIndexState ?? + entry.targetResolution?.freshness ?? + entry.codeIndexState ?? + entry.indexingStatus; + const context: string[] = []; + if (requested && requested !== served) context.push(`requested ${requested}`); + if (freshness) context.push(describeFreshness(freshness)); + const suffix = context.length > 0 ? ` (${context.join("; ")})` : ""; + return `No hits for ${entry.source} on ${served}${suffix}.`; +} + export function formatProgressTarget(target: { requested?: string; resolvedRequested?: string; @@ -314,9 +489,13 @@ export function describeFreshness(value: string): string { export function formatSourceStatus(entry: { source: string; targetLabel: string; + requestedTarget?: string; + freshTarget?: string; + servedTarget?: string; targetResolution?: LeanTargetResolution; indexingStatus?: string; codeIndexState?: string; + resultCount?: number; ignoredFilters?: string[]; incompatibleFilters?: string[]; ignoredQueryFeatures?: string[]; @@ -329,6 +508,14 @@ export function formatSourceStatus(entry: { } const parts: string[] = [`${entry.source} (${entry.targetLabel})`]; + if (entry.requestedTarget) parts.push(`requested=${entry.requestedTarget}`); + if (entry.freshTarget) parts.push(`fresh=${entry.freshTarget}`); + if (entry.servedTarget && entry.servedTarget !== entry.targetLabel) { + parts.push(`served=${entry.servedTarget}`); + } + if (typeof entry.resultCount === "number") { + parts.push(`results=${entry.resultCount}`); + } if (entry.indexingStatus) parts.push(`indexState=${entry.indexingStatus}`); if (entry.codeIndexState) parts.push(`codeIndex=${entry.codeIndexState}`); if (entry.ignoredFilters?.length) { @@ -361,7 +548,10 @@ function terminalLifecycleReason(entry: { new Set([entry.indexingStatus, entry.codeIndexState].filter(Boolean)), ) as string[]; const terminalStates = states.filter( - (state) => state !== "INDEXING" && state !== "STALE", + (state) => + !isHealthySearchLifecycleState(state) && + state !== "INDEXING" && + state !== "STALE", ); if (terminalStates.length === 0) return undefined; const status = terminalStates.join("/"); diff --git a/packages/mcp/src/smoke-test.ts b/packages/mcp/src/smoke-test.ts index 54c4a79..a516fcd 100644 --- a/packages/mcp/src/smoke-test.ts +++ b/packages/mcp/src/smoke-test.ts @@ -762,6 +762,7 @@ async function runLiveSmoke(caller: McpSmokeCaller): Promise { const statusJson = assertJsonResult( await callTool(caller, "search_status", { search_ref: searchRef, + wait_timeout_ms: 0, format: "json", }), "search_status json", @@ -775,6 +776,7 @@ async function runLiveSmoke(caller: McpSmokeCaller): Promise { assertErrorCode( await callTool(caller, "search_status", { search_ref: "smoke-invalid-search-ref", + wait_timeout_ms: 0, }), "search_status invalid ref", "NOT_FOUND", diff --git a/packages/mcp/src/tools/search-status.test.ts b/packages/mcp/src/tools/search-status.test.ts index 9073c04..325ccf5 100644 --- a/packages/mcp/src/tools/search-status.test.ts +++ b/packages/mcp/src/tools/search-status.test.ts @@ -64,6 +64,37 @@ describe("searchStatusTool", () => { expect(tool.description).toContain("partial hits"); expect(tool.description).toContain("allow_partial_results"); + expect(tool.description).toContain("instead of repeating `search`"); + }); + + it("waits up to the shared default and forwards explicit wait windows", async () => { + const searchStatus = mock((_searchRef: string, _waitTimeoutMs?: number) => + Promise.resolve(defaultUnifiedSearchOutcome), + ); + const tool = createSearchStatusTool( + createMockCodeNavigationService({ searchStatus }), + ); + + await tool.handler({ search_ref: "search-ref-default" }, {}); + expect(searchStatus.mock.calls[0]).toEqual(["search-ref-default", 20_000]); + + searchStatus.mockClear(); + await tool.handler( + { search_ref: "search-ref-explicit", wait_timeout_ms: 45_000 }, + {}, + ); + expect(searchStatus.mock.calls[0]).toEqual(["search-ref-explicit", 45_000]); + }); + + it("bounds the wait timeout in the public schema", () => { + const tool = createSearchStatusTool(createMockCodeNavigationService()); + const waitSchema = tool.schema.wait_timeout_ms; + if (!waitSchema) throw new Error("expected wait_timeout_ms schema"); + + expect(waitSchema.safeParse(0).success).toBe(true); + expect(waitSchema.safeParse(60_000).success).toBe(true); + expect(waitSchema.safeParse(-1).success).toBe(false); + expect(waitSchema.safeParse(60_001).success).toBe(false); }); it("adds local MCP auth remediation to auth errors", async () => { @@ -108,6 +139,45 @@ describe("searchStatusTool", () => { expect(payload).not.toHaveProperty("query"); }); + it("keeps completed empty JSON structured", async () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed outcome fixture"); + } + const completedOutcome = defaultUnifiedSearchOutcome; + const tool = createSearchStatusTool( + createMockCodeNavigationService({ + searchStatus: mock(() => + Promise.resolve({ + ...completedOutcome, + result: { + ...completedOutcome.result, + results: [], + page: { + ...completedOutcome.result.page, + returned: 0, + }, + sourceStatus: completedOutcome.result.sourceStatus.map( + (entry) => ({ ...entry, resultCount: 0 }), + ), + }, + }), + ), + }), + ); + + const result = await tool.handler( + { search_ref: "search-ref-123", format: "json" }, + {}, + ); + const payload = JSON.parse(result.content[0]?.text ?? "{}"); + + expect(payload.result.results).toEqual([]); + expect(payload.result.sourceStatus[0].resultCount).toBe(0); + expect(result.content[0]?.text).not.toContain( + "Do not repeat this search unchanged.", + ); + }); + it("surfaces TIMEOUT status without pretending the search is still running", async () => { const tool = createSearchStatusTool( createMockCodeNavigationService({ @@ -130,6 +200,8 @@ describe("searchStatusTool", () => { expect(result.isError).toBeUndefined(); expect(payload.completed).toBe(false); expect(payload.progress.status).toBe("TIMEOUT"); + expect(payload.progress.next).toBe("rerun search"); + expect(payload.progress.next).not.toContain("search_status"); }); it("renders TIMEOUT text without claiming active indexing", async () => { @@ -145,8 +217,67 @@ describe("searchStatusTool", () => { const text = result.content[0]?.text ?? ""; expect(text).toContain("search_status | timeout | searchRef=ref-timeout"); expect(text).not.toContain("search_status | indexing"); + expect(text).toContain("Do not call search_status again for this session."); + expect(text).toContain("next: rerun search."); + expect(text).not.toContain("next: call search_status"); }); + it("stops polling a failed search session", async () => { + const tool = createSearchStatusTool( + createMockCodeNavigationService({ + searchStatus: mock(() => + Promise.resolve(createIncompleteOutcome("FAILED", "ref-failed")), + ), + }), + ); + + const result = await tool.handler({ search_ref: "ref-failed" }, {}); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("Do not call search_status again for this session."); + expect(text).toContain("next: rerun search."); + expect(text).not.toContain("next: call search_status"); + }); + + it.each([ + ["FAILED", "No hits - search failed."], + ["TIMEOUT", "No hits - search timed out."], + ] as const)( + "does not promise future hits for a terminal %s partial result", + async (status, expectedMessage) => { + const incomplete = createIncompleteOutcome( + status, + `ref-${status.toLowerCase()}`, + ); + incomplete.result = { + query: "router", + queryWarnings: [], + sources: ["CODE"], + results: [], + page: { + offset: 0, + limit: 10, + returned: 0, + hasMore: false, + }, + partialResults: true, + sourceStatus: [], + }; + const tool = createSearchStatusTool( + createMockCodeNavigationService({ + searchStatus: mock(() => Promise.resolve(incomplete)), + }), + ); + + const result = await tool.handler( + { search_ref: incomplete.searchRef }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain(expectedMessage); + expect(text).not.toContain("No hits yet"); + }, + ); + it("surfaces progress freshness warnings", async () => { const tool = createSearchStatusTool( createMockCodeNavigationService({ @@ -292,7 +423,40 @@ describe("searchStatusTool", () => { expect(result.isError).toBeUndefined(); expect(text).toContain("search_status | searching | searchRef=ref-text"); expect(text).toContain("progress: SEARCHING, 0/1 targets ready"); - expect(text).toContain('next: call search_status search_ref="ref-text"'); + expect(text).toContain("Do not repeat search."); + expect(text).toContain( + 'next: call search_status with search_ref="ref-text" and wait_timeout_ms=20000.', + ); + expect(text).not.toContain("searchRef=ref-text to follow up"); expect(() => JSON.parse(text)).toThrow(); }); + + it("renders immediately queryable alternatives while deferred", async () => { + const tool = createSearchStatusTool( + createMockCodeNavigationService({ + searchStatus: mock(() => + Promise.resolve( + createIncompleteOutcome("INDEXING", "ref-alternatives", { + targets: [ + { + requested: "npm:express latest", + availableVersions: [{ version: "4.18.2", ref: "v4.18.2" }], + availableRefs: [{ ref: "main" }], + }, + ], + }), + ), + ), + }), + ); + + const result = await tool.handler({ search_ref: "ref-alternatives" }, {}); + const text = result.content[0]?.text ?? ""; + + expect(text).toContain( + "queryable now: versions=4.18.2@v4.18.2 | refs=main", + ); + expect(text).toContain("Do not repeat search."); + expect(text).not.toContain("allow_partial_results: true"); + }); }); diff --git a/packages/mcp/src/tools/search-status.ts b/packages/mcp/src/tools/search-status.ts index 90a6e3c..235afb4 100644 --- a/packages/mcp/src/tools/search-status.ts +++ b/packages/mcp/src/tools/search-status.ts @@ -1,5 +1,9 @@ import type { CodeNavigationService } from "@githits/core-internal"; import { z } from "zod"; +import { + DEFAULT_WAIT_TIMEOUT_MS, + MAX_WAIT_TIMEOUT_MS, +} from "../shared/code-navigation-defaults.js"; import { buildUnifiedSearchErrorPayload, buildUnifiedSearchStatusPayload, @@ -16,6 +20,7 @@ import { export interface SearchStatusArgs { search_ref: string; + wait_timeout_ms?: number; format?: "json" | "text" | "text-v1"; } @@ -26,6 +31,15 @@ const schema: ZodRawShape = { .describe( "The `searchRef` field from a prior `search` response (camelCase in the response, snake_case as this parameter). Pass it through unchanged.", ), + wait_timeout_ms: z.coerce + .number() + .int() + .min(0) + .max(MAX_WAIT_TIMEOUT_MS) + .optional() + .describe( + "Milliseconds to wait for progress or completion before returning the latest status (0-60000; default 20000).", + ), format: z .enum(["text-v1", "text", "json"]) .default("text-v1") @@ -36,7 +50,8 @@ const schema: ZodRawShape = { const DESCRIPTION = "Use only after `search` returns a `searchRef`. Check progress, fetch partial hits (when the original request used `allow_partial_results: true`), or fetch final results for a prior `search` that returned a `searchRef`. " + - "Pass the `searchRef` from that response as `search_ref` here (response field is camelCase; this parameter is snake_case)."; + "Pass the `searchRef` from that response as `search_ref` here (response field is camelCase; this parameter is snake_case); while it is active, continue with `search_status` instead of repeating `search`. " + + "The tool waits up to 20 seconds by default; set `wait_timeout_ms` from 0 to 60000 to change that bounded wait."; export function createSearchStatusTool( service: CodeNavigationService, @@ -48,7 +63,10 @@ export function createSearchStatusTool( annotations: READ_ONLY_TOOL_ANNOTATIONS, handler: async (args) => { try { - const outcome = await service.searchStatus(args.search_ref); + const outcome = await service.searchStatus( + args.search_ref, + args.wait_timeout_ms ?? DEFAULT_WAIT_TIMEOUT_MS, + ); const payload = buildUnifiedSearchStatusPayload(outcome); if (isTextFormat(args.format)) { return textResult(renderUnifiedSearchStatusText(payload)); diff --git a/packages/mcp/src/tools/search.test.ts b/packages/mcp/src/tools/search.test.ts index 1bc4ce8..5591623 100644 --- a/packages/mcp/src/tools/search.test.ts +++ b/packages/mcp/src/tools/search.test.ts @@ -10,6 +10,13 @@ import { import { createSearchTool } from "./search.js"; describe("searchTool", () => { + it("directs deferred calls to search_status instead of repeated search", () => { + const tool = createSearchTool(createMockCodeNavigationService()); + + expect(tool.description).toContain("do not repeat `search`"); + expect(tool.description).toContain("`search_status`"); + }); + it("returns unified search payload from service", async () => { const tool = createSearchTool(createMockCodeNavigationService()); diff --git a/packages/mcp/src/tools/search.ts b/packages/mcp/src/tools/search.ts index 363e38c..f650aba 100644 --- a/packages/mcp/src/tools/search.ts +++ b/packages/mcp/src/tools/search.ts @@ -234,7 +234,7 @@ const DESCRIPTION = "Required: `query` plus either `target` or `targets`; pass `target` or `targets`, not both. " + "Omit `source` to let GitHits select the best sources; set it only to restrict results to docs, code, or symbols. " + 'Structured parameters combine with the `query` using AND semantics. For `source:"docs"`, code/symbol-only filters (`category`, `kind`, `file_intent`, `public_only`) are ignored because docs search does not support them. ' + - "Complete by default — if indexing is still running, the response carries a `searchRef` and no hits; pass it to `search_status` to follow up. " + + "Complete by default — if indexing is still running, the response carries a `searchRef` and no hits; do not repeat `search`, pass that reference to `search_status`. " + "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)." + `\n\n${SEARCH_GUARDRAIL}`; diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index f59c737..73888b0 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -1077,7 +1077,13 @@ async function runLiveSmoke(): Promise { ); if (typeof searchJson.searchRef === "string") { const statusJson = assertJsonOutput( - await runCli(["search-status", searchJson.searchRef, "--json"]), + await runCli([ + "search-status", + searchJson.searchRef, + "--wait", + "0", + "--json", + ]), "search-status json", ); assertRecord(statusJson, "search-status json"); @@ -1087,7 +1093,13 @@ async function runLiveSmoke(): Promise { ); } else { assertJsonErrorCode( - await runCli(["search-status", "smoke-invalid-search-ref", "--json"]), + await runCli([ + "search-status", + "smoke-invalid-search-ref", + "--wait", + "0", + "--json", + ]), "search-status invalid json error", "NOT_FOUND", ); diff --git a/src/commands/code/code-nav-cli-helpers.ts b/src/commands/code/code-nav-cli-helpers.ts index 170ee81..faa8fd7 100644 --- a/src/commands/code/code-nav-cli-helpers.ts +++ b/src/commands/code/code-nav-cli-helpers.ts @@ -83,8 +83,8 @@ export function resolveCliCodeNavTarget( * `indexingRef` + a sample of `availableVersions` as dimmed * detail lines under the error message. * - * Common to `code files` / `code read` / `code grep` since all three - * share the same indexing-retry story. + * Shared by human `search` / `search-status` errors and the indexed + * `code files` / `code read` / `code grep` commands. */ export function formatIndexingError(mapped: MappedError): string { if (mapped.code === "UPDATE_REQUIRED") { @@ -93,7 +93,24 @@ export function formatIndexingError(mapped: MappedError): string { if (mapped.code !== "INDEXING") return formatMappedErrorForTerminal(mapped); const detail = mapped.details ?? {}; const lines = [mapped.message]; + if (detail.hint && !mapped.message.includes(detail.hint)) { + lines.push(` hint: ${detail.hint}`); + } if (detail.indexingRef) lines.push(` indexing ref: ${detail.indexingRef}`); + const estimate = detail.indexingEstimate; + if (estimate) { + const bounds = + typeof estimate.lowerSeconds === "number" && + typeof estimate.upperSeconds === "number" + ? `${estimate.lowerSeconds}-${estimate.upperSeconds}s` + : undefined; + const elapsed = + typeof estimate.elapsedSeconds === "number" + ? `${estimate.elapsedSeconds}s elapsed` + : undefined; + const summary = [bounds, elapsed].filter(Boolean).join(", "); + if (summary) lines.push(` indexing estimate: ${summary}`); + } const versions = detail.availableVersions; if (versions && versions.length > 0) { const shown = versions @@ -130,16 +147,16 @@ export function formatFileErrorWithFilesHint(mapped: MappedError): string { return formatMappedErrorForTerminal(mapped); } if (mapped.code === "FILE_NOT_FOUND") { - return `${mapped.message}\n Use \`code files\` to list available paths.`; + return `${formatMappedErrorForTerminal(mapped)}\n Use \`code files\` to list available paths.`; } if ( mapped.code === "NOT_FOUND" && looksLikeMissingFileMessage(mapped.message) ) { - return `${mapped.message}\n Use \`code files\` to list available paths.`; + return `${formatMappedErrorForTerminal(mapped)}\n Use \`code files\` to list available paths.`; } if (mapped.code === "REF_NOT_FOUND") { - return `${mapped.message}\n Check that the repository URL and git ref exist and are publicly accessible.`; + return `${formatMappedErrorForTerminal(mapped)}\n Check that the repository URL and git ref exist and are publicly accessible.`; } if (looksLikeMissingNavpackMessage(mapped.message)) { return [ @@ -151,7 +168,7 @@ export function formatFileErrorWithFilesHint(mapped: MappedError): string { const retry = mapped.retryable ? "Retry in a moment; if it persists, narrow the target or file an issue." : "Narrow the target (path, path-prefix, glob) and retry; if it persists, file an issue."; - return `${mapped.message}\n ${retry}`; + return `${formatMappedErrorForTerminal(mapped)}\n ${retry}`; } return formatIndexingError(mapped); } diff --git a/src/commands/code/files.test.ts b/src/commands/code/files.test.ts index f4e819b..6cfe5af 100644 --- a/src/commands/code/files.test.ts +++ b/src/commands/code/files.test.ts @@ -517,10 +517,18 @@ describe("pkgFilesAction", () => { const service = createMockCodeNavigationService({ listFiles: mock(() => Promise.reject( - new CodeNavigationIndexingError("Target is indexing.", "ref_xyz", [ - { version: "4.21.0", ref: "v4.21.0" }, - { version: "4.20.1", ref: "v4.20.1" }, - ]), + new CodeNavigationIndexingError( + "Target is indexing. Backend says this ref is queued.", + "ref_xyz", + [ + { version: "4.21.0", ref: "v4.21.0" }, + { version: "4.20.1", ref: "v4.20.1" }, + ], + undefined, + undefined, + { lowerSeconds: 7, upperSeconds: 19, elapsedSeconds: 3 }, + "Backend says this ref is queued.", + ), ), ), }); @@ -537,6 +545,8 @@ describe("pkgFilesAction", () => { const output = errorSpy.mock.calls[0]?.[0] as string; expect(output).toContain("indexing"); expect(output).toContain("indexing ref: ref_xyz"); + expect(output.match(/Backend says this ref is queued\./g)).toHaveLength(1); + expect(output).toContain("indexing estimate: 7-19s, 3s elapsed"); expect(output).toContain("indexed refs/versions: 4.21.0, 4.20.1"); errorSpy.mockRestore(); exitSpy.mockRestore(); diff --git a/src/commands/code/grep.test.ts b/src/commands/code/grep.test.ts index 63222d0..6c37b5f 100644 --- a/src/commands/code/grep.test.ts +++ b/src/commands/code/grep.test.ts @@ -168,7 +168,7 @@ describe("pkgGrepAction", () => { const stderr = stderrWrites.join(""); expect(stderr).toContain( - "More grep results available — rerun with --cursor 'cursor_abc123'", + "More matches available — rerun with --cursor 'cursor_abc123'", ); stdoutSpy.mockRestore(); stderrSpy.mockRestore(); @@ -497,9 +497,10 @@ describe("pkgGrepAction", () => { } }); - it("prints zero-match text before setting exit code 1", async () => { + it("prints zero-match guidance before setting exit code 1", async () => { const originalExitCode = process.exitCode; const writes: string[] = []; + const errorWrites: string[] = []; const writeSpy = spyOn(process.stdout, "write").mockImplementation((( chunk: string | Uint8Array, ) => { @@ -508,6 +509,14 @@ describe("pkgGrepAction", () => { ); return true; }) as typeof process.stdout.write); + const errorWriteSpy = spyOn(process.stderr, "write").mockImplementation((( + chunk: string | Uint8Array, + ) => { + errorWrites.push( + typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk), + ); + return true; + }) as typeof process.stderr.write); try { const service = createMockCodeNavigationService({ grepRepo: mock(() => @@ -529,9 +538,16 @@ describe("pkgGrepAction", () => { ); expect(writes.join("")).toContain("No matches."); + expect(errorWrites.join("")).toContain( + "Do not repeat this grep unchanged.", + ); + expect(errorWrites.join("")).toContain( + "use githits search for conceptual intent", + ); expect(process.exitCode).toBe(1); } finally { writeSpy.mockRestore(); + errorWriteSpy.mockRestore(); process.exitCode = originalExitCode ?? 0; } }); diff --git a/src/commands/format-mapped-error.test.ts b/src/commands/format-mapped-error.test.ts index 8908b70..9cfd204 100644 --- a/src/commands/format-mapped-error.test.ts +++ b/src/commands/format-mapped-error.test.ts @@ -135,6 +135,17 @@ describe("formatMappedErrorForTerminal", () => { ).toBe("No matching version found"); }); + it("preserves backend hints in terminal errors", () => { + expect( + formatMappedErrorForTerminal({ + code: "NOT_FOUND", + message: "Backend target message.", + retryable: false, + details: { hint: "Use the canonical package name." }, + }), + ).toBe("Backend target message.\n hint: Use the canonical package name."); + }); + it("formats UPDATE_REQUIRED with update command", () => { expect( formatMappedErrorForTerminal({ diff --git a/src/commands/format-mapped-error.ts b/src/commands/format-mapped-error.ts index e9e6877..2654198 100644 --- a/src/commands/format-mapped-error.ts +++ b/src/commands/format-mapped-error.ts @@ -26,7 +26,7 @@ export function formatMappedErrorForTerminal(mapped: MappedError): string { } if (mapped.code === "RATE_LIMITED") { if (mapped.retryable !== true || hasRetryGuidance(mapped.message)) { - return mapped.message; + return appendBackendHint(mapped, mapped.message); } const retryAfterSeconds = mapped.details?.retryAfterSeconds; if ( @@ -36,18 +36,21 @@ export function formatMappedErrorForTerminal(mapped: MappedError): string { ) { const seconds = Math.ceil(retryAfterSeconds); const unit = seconds === 1 ? "second" : "seconds"; - return `${mapped.message} Try again in ${seconds} ${unit}.`; + return appendBackendHint( + mapped, + `${mapped.message} Try again in ${seconds} ${unit}.`, + ); } - return `${mapped.message} Try again shortly.`; + return appendBackendHint(mapped, `${mapped.message} Try again shortly.`); } if (mapped.code === "TIMEOUT") { if (mapped.retryable !== true || hasRetryGuidance(mapped.message)) { - return mapped.message; + return appendBackendHint(mapped, mapped.message); } - return `${mapped.message} Try again.`; + return appendBackendHint(mapped, `${mapped.message} Try again.`); } if (mapped.code !== "UPDATE_REQUIRED") { - return mapped.message; + return appendBackendHint(mapped, mapped.message); } const detail = mapped.details ?? {}; const updateCommand = @@ -57,6 +60,12 @@ export function formatMappedErrorForTerminal(mapped: MappedError): string { return [mapped.message, "", "Update with:", ` ${updateCommand}`].join("\n"); } +function appendBackendHint(mapped: MappedError, text: string): string { + const hint = mapped.details?.hint; + if (!hint || text.includes(hint)) return text; + return `${text}\n hint: ${hint}`; +} + function hasRetryGuidance(message: string): boolean { return /\b(?:retry|try again)\b/i.test(message); } diff --git a/src/commands/search.test.ts b/src/commands/search.test.ts index c94c09f..5ce379d 100644 --- a/src/commands/search.test.ts +++ b/src/commands/search.test.ts @@ -6,7 +6,10 @@ import type { UnifiedSearchProgress, UnifiedSearchSessionStatus, } from "@githits/core-internal"; -import { AuthenticationError } from "@githits/core-internal"; +import { + AuthenticationError, + CodeNavigationIndexingError, +} from "@githits/core-internal"; import { AuthRequiredError } from "@githits/mcp/internal"; import { createMockCodeNavigationService, @@ -79,6 +82,42 @@ describe("searchAction", () => { exitSpy.mockRestore(); }); + it("renders indexing wait guidance and structured details in human output", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + const indexingError = new CodeNavigationIndexingError( + "Target is indexing.", + "idx-search", + [{ version: "5.2.1", ref: "v5.2.1" }], + undefined, + undefined, + { lowerSeconds: 7, upperSeconds: 19, elapsedSeconds: 3 }, + "Wait until ready with CLI `--wait 60000` or MCP `wait_timeout_ms: 60000`.", + ); + + await expect( + searchAction( + "router middleware", + { in: ["npm:express"] }, + createDeps({ + codeNavigationService: createMockCodeNavigationService({ + search: mock(() => Promise.reject(indexingError)), + }), + }), + ), + ).rejects.toThrow("process.exit"); + + const output = String(errorSpy.mock.calls[0]?.[0]); + expect(output).toContain("--wait 60000"); + expect(output).toContain("indexing ref: idx-search"); + expect(output).toContain("indexing estimate: 7-19s, 3s elapsed"); + expect(output).toContain("indexed refs/versions: 5.2.1"); + errorSpy.mockRestore(); + exitSpy.mockRestore(); + }); + it("calls unified search service with parsed targets and filters", async () => { const search = mock((_: UnifiedSearchParams) => Promise.resolve(defaultUnifiedSearchOutcome), @@ -1010,6 +1049,44 @@ describe("searchStatusAction", () => { consoleSpy.mockRestore(); }); + it("waits up to the shared default and forwards an explicit status wait", async () => { + const searchStatus = mock((_searchRef: string, _waitTimeoutMs?: number) => + Promise.resolve(createIncompleteOutcome("SEARCHING", "search-ref-wait")), + ); + const deps = createDeps({ + codeNavigationService: createMockCodeNavigationService({ searchStatus }), + }); + const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + + await searchStatusAction("search-ref-wait", {}, deps); + expect(searchStatus.mock.calls[0]).toEqual(["search-ref-wait", 20_000]); + + searchStatus.mockClear(); + await searchStatusAction("search-ref-wait", { wait: "45" }, deps); + expect(searchStatus.mock.calls[0]).toEqual(["search-ref-wait", 45_000]); + + consoleSpy.mockRestore(); + }); + + it("rejects an out-of-range search-status wait", async () => { + const errorSpy = spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + try { + await expect( + searchStatusAction("search-ref-wait", { wait: "61" }, createDeps()), + ).rejects.toThrow("process.exit"); + expect(String(errorSpy.mock.calls[0]?.[0])).toContain( + "--wait expects an integer between 0 and 60. Got 61.", + ); + } finally { + errorSpy.mockRestore(); + exitSpy.mockRestore(); + } + }); + it("includes target details for incomplete search refs", async () => { const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); @@ -1087,10 +1164,36 @@ describe("searchStatusAction", () => { const output = String(consoleSpy.mock.calls[0]?.[0]); expect(output).toContain("Search timed out."); + expect(output).toContain("This search session is terminal."); + expect(output).toContain("Start a new search."); + expect(output).not.toContain("longer wait"); expect(output).not.toContain("Search still in progress."); consoleSpy.mockRestore(); }); + it("replaces polling guidance in terminal search-status JSON", async () => { + const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + + await searchStatusAction( + "search-ref-timeout", + { json: true }, + createDeps({ + codeNavigationService: createMockCodeNavigationService({ + searchStatus: mock(() => + Promise.resolve( + createIncompleteOutcome("TIMEOUT", "search-ref-timeout"), + ), + ), + }), + }), + ); + + const payload = JSON.parse(String(consoleSpy.mock.calls[0]?.[0])); + expect(payload.progress.next).toBe("rerun search"); + expect(payload.progress.next).not.toContain("search_status"); + consoleSpy.mockRestore(); + }); + it("renders FAILED as terminal status instead of in-progress", async () => { const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); diff --git a/src/commands/search.ts b/src/commands/search.ts index f4ccadf..7354776 100644 --- a/src/commands/search.ts +++ b/src/commands/search.ts @@ -7,6 +7,7 @@ import { buildUnifiedSearchParams, buildUnifiedSearchStatusPayload, buildUnifiedSearchSuccessPayload, + DEFAULT_WAIT_TIMEOUT_MS, dim, formatProgressTarget, highlight, @@ -16,6 +17,8 @@ import { knownSymbolCategoryList, knownSymbolKindList, type LeanTargetResolution, + MAX_WAIT_TIMEOUT_MS, + type MappedError, parseUnifiedSearchTargetSpec, requireAuth, shouldUseColors, @@ -29,6 +32,7 @@ import { type Command, Option } from "commander"; import { parseIntCliOption } from "../shared/cli-options.js"; import { startSpinner } from "../shared/spinner.js"; import { SPINNER_MESSAGES } from "../shared/spinner-messages.js"; +import { formatIndexingError } from "./code/code-nav-cli-helpers.js"; import { formatMappedErrorForTerminal } from "./format-mapped-error.js"; export interface SearchCommandOptions { @@ -49,6 +53,7 @@ export interface SearchCommandOptions { } export interface SearchStatusCommandOptions { + wait?: string; json?: boolean; } @@ -129,7 +134,10 @@ export async function searchStatusAction( try { const service = requireSearchService(deps); - const outcome = await service.searchStatus(searchRef); + const outcome = await service.searchStatus( + searchRef, + parseWaitMs(options.wait) ?? DEFAULT_WAIT_TIMEOUT_MS, + ); const payload = buildUnifiedSearchStatusPayload(outcome); if (options.json) { @@ -183,7 +191,8 @@ const SEARCH_STATUS_DESCRIPTION = `Check the status of a unified search started Pass the searchRef returned by githits search when the initial request could not complete within the wait window. This can return progress, partial hits when -the original request used --allow-partial, or final results.`; +the original request used --allow-partial, or final results. By default it waits +up to 20 seconds for progress before returning the latest status.`; export function registerSearchCommand(program: Command) { program @@ -264,6 +273,10 @@ export function registerSearchCommand(program: Command) { .summary("Check the status of a previous search") .description(SEARCH_STATUS_DESCRIPTION) .argument("", "Search reference returned by githits search") + .option( + "--wait ", + "Max seconds to wait for progress (0-60; default: 20)", + ) .option("--json", "Output as JSON") .action(async (searchRef: string, options: SearchStatusCommandOptions) => { const deps = await loadContainer(); @@ -334,7 +347,12 @@ function parseWaitMs(value: string | undefined): number | undefined { "--wait must be an integer between 0 and 60 seconds.", ); } - const seconds = parseIntCliOption(match.groups.seconds, "--wait", 0, 60); + const seconds = parseIntCliOption( + match.groups.seconds, + "--wait", + 0, + MAX_WAIT_TIMEOUT_MS / 1000, + ); if (seconds === undefined) return undefined; return seconds * 1000; } @@ -359,21 +377,31 @@ function handleSearchError( } function formatSearchErrorTerminal( - payload: { error: string; code: string; details?: Record }, + payload: { + error: string; + code: string; + retryable?: boolean; + details?: Record; + }, context: "search" | "status", ): string { + const mapped: MappedError = { + code: payload.code as MappedError["code"], + message: payload.error, + retryable: payload.retryable, + details: payload.details as MappedError["details"], + }; if (payload.code === "AUTH_REQUIRED") { - return formatMappedErrorForTerminal({ - code: "AUTH_REQUIRED", - message: payload.error, - retryable: false, - details: payload.details, - }); + return formatMappedErrorForTerminal(mapped); + } + if (payload.code === "INDEXING") { + return formatIndexingError(mapped); } + const formatted = formatMappedErrorForTerminal(mapped); if (context === "status" && payload.code === "NOT_FOUND") { - return `${payload.error}\n Search sessions expire; run \`githits search ...\` to start a new one.`; + return `${formatted}\n Search sessions expire; run \`githits search ...\` to start a new one.`; } - return payload.error; + return formatted; } function formatUnifiedSearchTerminal(payload: { @@ -531,9 +559,7 @@ function formatSearchStatusTerminal(payload: { } } if (status === "TIMEOUT") { - lines.push( - "Search timed out before completion. Retry with a longer wait or start a new search.", - ); + lines.push("This search session is terminal. Start a new search."); return lines.join("\n"); } if (status === "FAILED") { @@ -542,7 +568,9 @@ function formatSearchStatusTerminal(payload: { ); return lines.join("\n"); } - lines.push("Use `githits search-status ` to check again."); + lines.push( + `next: githits search-status ${payload.searchRef} --wait ${DEFAULT_WAIT_TIMEOUT_MS / 1000}`, + ); return lines.join("\n"); }