From fc341ed8bc7d0db06945c7b39f4561073ce2ec18 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 11 Aug 2026 11:49:00 +0300 Subject: [PATCH 1/4] fix: prevent futile search and grep retries Add bounded empty-result guidance, explicit deferred-search continuation, and shared indexing error details across MCP and CLI output. Preserve backend diagnostics and structured JSON while preventing identical retry loops. --- docs/implementation/cli-commands.md | 2 +- docs/implementation/mcp-cli-parity.md | 16 +- docs/implementation/tools.md | 21 +- .../services/code-navigation-service.test.ts | 260 +++++++++++++++++- .../src/services/code-navigation-service.ts | 139 ++++++---- .../shared/code-navigation-error-map.test.ts | 57 ++++ .../src/shared/code-navigation-error-map.ts | 30 ++ .../mcp/src/shared/grep-repo-response.test.ts | 57 +++- packages/mcp/src/shared/grep-repo-response.ts | 7 + .../mcp/src/shared/grep-repo-text.test.ts | 60 +++- packages/mcp/src/shared/grep-repo-text.ts | 85 +++++- .../mcp/src/shared/repository-target.test.ts | 6 + packages/mcp/src/shared/repository-target.ts | 6 + packages/mcp/src/shared/search-lifecycle.ts | 7 + .../shared/unified-search-response.test.ts | 162 ++++++++++- .../mcp/src/shared/unified-search-response.ts | 71 ++++- .../src/shared/unified-search-status-text.ts | 34 ++- .../src/shared/unified-search-text.test.ts | 185 ++++++++++++- .../mcp/src/shared/unified-search-text.ts | 214 ++++++++++++-- packages/mcp/src/tools/search-status.test.ts | 71 +++++ packages/mcp/src/tools/search-status.ts | 2 +- packages/mcp/src/tools/search.test.ts | 7 + packages/mcp/src/tools/search.ts | 2 +- src/commands/code/code-nav-cli-helpers.ts | 29 +- src/commands/code/files.test.ts | 18 +- src/commands/code/grep.test.ts | 18 +- src/commands/format-mapped-error.test.ts | 11 + src/commands/format-mapped-error.ts | 21 +- src/commands/search.test.ts | 41 ++- src/commands/search.ts | 30 +- 30 files changed, 1505 insertions(+), 164 deletions(-) create mode 100644 packages/mcp/src/shared/search-lifecycle.ts diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index bfa95ac5..46eefa42 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -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/checking casing or using conceptual `search`, and explicitly rejects an unchanged repeat. 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 a4283390..dbe0a460 100644 --- a/docs/implementation/mcp-cli-parity.md +++ b/docs/implementation/mcp-cli-parity.md @@ -119,6 +119,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 +135,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 +150,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 +358,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 f29e4ee1..01c4b2bb 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -20,7 +20,7 @@ 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` | `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`, `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. | | `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. | @@ -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`. `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`. Deferred search reports ready/total counts, says `Do not repeat search.`, and gives the exact continuation `search_status` call with `search_ref`. The response never suggests changing `allow_partial_results` after deferral. + **Listing anatomy** (`code_files` text-v1): ``` @@ -247,6 +254,8 @@ code_grep | matches in files | pattern="..." [regex,case-sensitive] 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.` 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/check casing or switch to conceptual `search`. The same decision text is shared with CLI terminal stderr while plain CLI stdout remains grep-compatible and empty. + `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 b1aedf28..13bfaabe 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, @@ -1169,12 +1166,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 +1182,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 +1326,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 +1354,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 +1471,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 +1510,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 e571e4df..3f7c8172 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 @@ -558,12 +561,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 +597,7 @@ export class CodeNavigationTargetNotFoundError extends Error { public readonly availableVersions?: AvailableVersion[], public readonly repoUrl?: string, public readonly requestedRef?: string, + public readonly metadata?: CodeNavigationErrorMetadata, ) { super(message); this.name = "CodeNavigationTargetNotFoundError"; @@ -622,6 +636,7 @@ export class CodeNavigationVersionNotFoundError extends Error { public readonly requestedVersion: string | undefined, public readonly latestIndexed: string | undefined, public readonly availableVersions: AvailableVersion[] | undefined, + public readonly metadata?: CodeNavigationErrorMetadata, ) { super(message); this.name = "CodeNavigationVersionNotFoundError"; @@ -640,6 +655,7 @@ export class CodeNavigationRefNotFoundError extends Error { public readonly requestedRef: string | undefined, public readonly availableRefs: AvailableRef[] | undefined, public readonly suggestedRefs: SuggestedRef[] | undefined, + public readonly metadata?: CodeNavigationErrorMetadata, ) { super(message); this.name = "CodeNavigationRefNotFoundError"; @@ -688,6 +704,7 @@ export class CodeNavigationBackendError extends Error { * April 2026 `extensions.retryable` contract on GraphQL errors. */ public readonly retryable?: boolean, + public readonly metadata?: CodeNavigationErrorMetadata, ) { super(message); this.name = "CodeNavigationBackendError"; @@ -2023,6 +2040,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 +2076,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 +2116,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { ? extensions.latest_indexed : undefined, parseAvailableVersions(extensions), + errorMetadata, ); case "REF_NOT_FOUND": @@ -2104,12 +2126,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 +2146,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { undefined, parseGraphQLRepoUrl(extensions), parseGraphQLGitRef(extensions), + errorMetadata, ); case "FILE_NOT_FOUND": @@ -2162,6 +2192,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { undefined, code, retryable, + errorMetadata, ); // `code` was present but not one of the recognised values — @@ -2184,30 +2215,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 +2414,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 +2862,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 +2957,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 76090d24..5d820c34 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 31774a39..7f230c12 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 23186629..2954fcd9 100644 --- a/packages/mcp/src/shared/grep-repo-response.test.ts +++ b/packages/mcp/src/shared/grep-repo-response.test.ts @@ -170,7 +170,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 +180,79 @@ 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: 1 scanned | 1 in scope"); + expect(stderr).toContain("Do not repeat this grep unchanged."); + expect(stderr).toContain("shorten or change the pattern"); }); - 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: 0 scanned | 0 in scope"); + expect(stderr).toContain( + "loosen path, path_prefix, globs, extensions, or exclusion filters", + ); + }); + + 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 grep results 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("increase --limit"); + expect(stderr).not.toContain("--max-matches"); }); it("verbose mode renders minimal symbol hints", () => { diff --git a/packages/mcp/src/shared/grep-repo-response.ts b/packages/mcp/src/shared/grep-repo-response.ts index f8b15687..79dd079b 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( diff --git a/packages/mcp/src/shared/grep-repo-text.test.ts b/packages/mcp/src/shared/grep-repo-text.test.ts index 569cb3d9..a6a12af2 100644 --- a/packages/mcp/src/shared/grep-repo-text.test.ts +++ b/packages/mcp/src/shared/grep-repo-text.test.ts @@ -32,11 +32,44 @@ 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: 120 scanned | 120 in scope"); + expect(text).toContain("Do not repeat this grep unchanged."); + expect(text).toContain("shorten or change the pattern"); + expect(text).toContain("check casing"); + expect(text).toContain("use search for conceptual intent"); + }); + + 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: 0 scanned | 0 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 +91,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", () => { @@ -155,6 +189,30 @@ describe("renderGrepRepoText", () => { expect(text).toContain("More matches available. Pass cursor=ABC123"); }); + it("pages an empty incomplete result instead of changing the grep", () => { + const text = renderGrepRepoText( + envelope({ + hasMore: true, + nextCursor: "ABC123", + }), + ); + + 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("surfaces truncation on an empty incomplete result", () => { + const text = renderGrepRepoText( + envelope({ + truncatedReason: "limit", + }), + ); + + expect(text).toContain("Truncated: limit."); + expect(text).not.toContain("Do not repeat this grep unchanged."); + }); + it("renders pattern-type and case-sensitive flags in header when set", () => { const text = renderGrepRepoText( envelope({ diff --git a/packages/mcp/src/shared/grep-repo-text.ts b/packages/mcp/src/shared/grep-repo-text.ts index 9eb07964..cfbd9d4e 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,79 @@ 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) { + lines.push( + surface === "cli" + ? `Truncated: ${envelope.truncatedReason}. Narrow the file selectors or increase --limit.` + : `Truncated: ${envelope.truncatedReason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + ); + } + if (envelope.hasMore && envelope.nextCursor) { + lines.push( + surface === "cli" + ? `More grep results 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."); + lines.push( + envelope.filesInScope === 0 + ? "next: loosen path, path_prefix, globs, extensions, or exclusion filters." + : "next: shorten or change the pattern, check casing, or use search for conceptual intent.", + ); + return lines; +} + +function formatEmptyGrepFileCounts(envelope: LeanGrepRepoEnvelope): string { + if (envelope.filesScanned < envelope.filesInScope) { + return `files: ${envelope.filesInScope} in scope | ${envelope.filesScanned} content-scanned after index pruning`; + } + return `files: ${envelope.filesScanned} scanned | ${envelope.filesInScope} in scope`; +} + +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${ diff --git a/packages/mcp/src/shared/repository-target.test.ts b/packages/mcp/src/shared/repository-target.test.ts index de663215..ddafe61f 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 17f42076..b0e5de38 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 00000000..b4d6bfec --- /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 6e95c4be..4b750d16 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" }], @@ -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", () => { diff --git a/packages/mcp/src/shared/unified-search-response.ts b/packages/mcp/src/shared/unified-search-response.ts index 2c7f9e9c..7253081c 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, @@ -848,14 +853,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 +871,7 @@ function warningForEntry( entry.codeIndexState ) { if ( + !isHealthySearchLifecycleState(entry.codeIndexState) && entry.codeIndexState !== "STALE" && !(entry.codeIndexState === "INDEXING" && options.completed) ) { @@ -901,7 +907,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 +967,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 +983,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 +994,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 +1088,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 1a24de4f..27cf3481 100644 --- a/packages/mcp/src/shared/unified-search-status-text.ts +++ b/packages/mcp/src/shared/unified-search-status-text.ts @@ -4,9 +4,10 @@ import type { UnifiedSearchStatusResultPayload, } from "./unified-search-response.js"; import { + appendEmptySearchGuidance, + appendSourceStatusNotes, appendUnifiedSearchHits, formatProgressTarget, - formatSourceStatus, } from "./unified-search-text.js"; const SEP = " | "; @@ -35,9 +36,10 @@ export function renderUnifiedSearchStatusText(payload: StatusPayload): string { } const result = payload.result; - if (result) appendResult(lines, result); + if (result) appendResult(lines, result, payload.completed); if (!payload.completed) { + lines.push("Do not repeat search."); lines.push( `next: call search_status search_ref=${quote(payload.searchRef)}`, ); @@ -58,6 +60,7 @@ function buildHeader(payload: StatusPayload): string { function appendResult( lines: string[], result: UnifiedSearchStatusResultPayload, + completed: boolean, ): void { lines.push(""); if (result.warnings && result.warnings.length > 0) { @@ -66,7 +69,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("No hits yet."); + } } else { appendUnifiedSearchHits(lines, result.results); } @@ -78,12 +91,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,10 +106,8 @@ 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}`; + return `progress: ${progress.status}, ${progress.targetsReady}/${progress.targetsTotal} targets ready, ${progress.elapsedMs}ms elapsed`; } function quote(value: string): string { diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index 4594654e..dfb9897e 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,39 @@ describe("renderUnifiedSearchSuccess", () => { const text = renderUnifiedSearchSuccess(incomplete); expect(text).toContain("1 partial"); expect(text).toContain("searchRef=ref_abc-123"); + expect(text).toContain("Indexing in progress. Do not repeat search."); + expect(text).toContain('Call search_status with search_ref="ref_abc-123".'); + expect(text).not.toContain("searchRef=ref_abc-123 to follow up"); + }); + + 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( - "Indexing in progress. Call search_status with searchRef=ref_abc-123", + "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 +388,9 @@ 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 requestedRef when repo follow-up lacks served gitRef", () => { @@ -278,6 +453,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 a0870ae6..f429c1fe 100644 --- a/packages/mcp/src/shared/unified-search-text.ts +++ b/packages/mcp/src/shared/unified-search-text.ts @@ -16,9 +16,11 @@ */ 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 +28,7 @@ import type { UnifiedSearchErrorPayload, UnifiedSearchHitPayload, UnifiedSearchIncompletePayload, + UnifiedSearchQueryEcho, } from "./unified-search-response.js"; const SUMMARY_WRAP_WIDTH = 76; @@ -43,13 +46,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,9 +75,10 @@ export function renderUnifiedSearchSuccess( return lines.join("\n"); } -function noHitsYetMessage(payload: SearchSuccessPayload): string { - if (payload.completed) return "No hits."; - const status = payload.progress?.status; +function noHitsYetMessage( + progress: UnifiedSearchIncompletePayload["progress"], +): string { + const status = progress?.status; if (status === "TIMEOUT") return "No hits yet - timed out waiting."; if (status === "FAILED") return "No hits - search failed."; if (status === "SEARCHING") return "No hits yet - searching."; @@ -216,15 +234,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 = @@ -244,16 +260,19 @@ function buildTrailer(payload: SearchSuccessPayload): string[] { : status === "SEARCHING" ? "Search in progress." : "Indexing in progress."; + if (payload.progress) { + lines.push( + `progress: ${payload.progress.targetsReady}/${payload.progress.targetsTotal} targets ready.`, + ); + } + lines.push(`${action} Do not repeat search.`); lines.push( - `${action} Call search_status with searchRef=${payload.searchRef} to follow up.`, + `Call search_status with search_ref=${JSON.stringify(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 +286,146 @@ function buildTrailer(payload: SearchSuccessPayload): string[] { return lines; } +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 across ${sources} 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 +473,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 +492,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 +532,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/tools/search-status.test.ts b/packages/mcp/src/tools/search-status.test.ts index 9073c045..abfc5552 100644 --- a/packages/mcp/src/tools/search-status.test.ts +++ b/packages/mcp/src/tools/search-status.test.ts @@ -64,6 +64,7 @@ describe("searchStatusTool", () => { expect(tool.description).toContain("partial hits"); expect(tool.description).toContain("allow_partial_results"); + expect(tool.description).toContain("instead of repeating `search`"); }); it("adds local MCP auth remediation to auth errors", async () => { @@ -108,6 +109,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({ @@ -292,7 +332,38 @@ 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("Do not repeat search."); expect(text).toContain('next: call search_status search_ref="ref-text"'); + 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 90a6e3c0..bbcb9ba9 100644 --- a/packages/mcp/src/tools/search-status.ts +++ b/packages/mcp/src/tools/search-status.ts @@ -36,7 +36,7 @@ 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`."; export function createSearchStatusTool( service: CodeNavigationService, diff --git a/packages/mcp/src/tools/search.test.ts b/packages/mcp/src/tools/search.test.ts index 1bc4ce86..55916236 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 363e38cb..f650abaa 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/src/commands/code/code-nav-cli-helpers.ts b/src/commands/code/code-nav-cli-helpers.ts index 170ee81c..faa8fd7b 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 f4e819bd..6cfe5afd 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 63222d01..c526eb95 100644 --- a/src/commands/code/grep.test.ts +++ b/src/commands/code/grep.test.ts @@ -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 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 8908b70a..9cfd204e 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 e9e68779..2654198c 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 c94c09f1..638fe24c 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), diff --git a/src/commands/search.ts b/src/commands/search.ts index f4ccadf3..0f5c3418 100644 --- a/src/commands/search.ts +++ b/src/commands/search.ts @@ -16,6 +16,7 @@ import { knownSymbolCategoryList, knownSymbolKindList, type LeanTargetResolution, + type MappedError, parseUnifiedSearchTargetSpec, requireAuth, shouldUseColors, @@ -29,6 +30,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 { @@ -359,21 +361,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: { From 02af216655675dfa24e2a35a2a023a3da9fc03e9 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 11 Aug 2026 12:08:58 +0300 Subject: [PATCH 2/4] fix: refine search and grep recovery guidance Stop polling terminal search sessions and keep empty grep pivots accurate for case sensitivity and each command surface. Standardize continuation copy and compact scan context. --- docs/implementation/tools.md | 6 +-- .../mcp/src/shared/grep-repo-response.test.ts | 34 ++++++++++-- packages/mcp/src/shared/grep-repo-response.ts | 2 +- .../mcp/src/shared/grep-repo-text.test.ts | 24 +++++++-- packages/mcp/src/shared/grep-repo-text.ts | 39 ++++++++++---- .../src/shared/unified-search-status-text.ts | 12 ++--- .../src/shared/unified-search-text.test.ts | 54 ++++++++++++++++++- .../mcp/src/shared/unified-search-text.ts | 31 ++++++++--- packages/mcp/src/tools/search-status.test.ts | 23 +++++++- src/commands/code/grep.test.ts | 4 +- 10 files changed, 190 insertions(+), 39 deletions(-) diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 01c4b2bb..90a15c2b 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -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. 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`. `format` defaults to `text-v1`; 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. @@ -217,7 +217,7 @@ More hits available. Pass offset=N for the next page or limit=N to widen. **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`. Deferred search reports ready/total counts, says `Do not repeat search.`, and gives the exact continuation `search_status` call with `search_ref`. The response never suggests changing `allow_partial_results` after deferral. +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 continuation `next: call search_status with search_ref="...".` 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): @@ -254,7 +254,7 @@ code_grep | matches in files | pattern="..." [regex,case-sensitive] 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.` 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/check casing or switch to conceptual `search`. The same decision text is shared with CLI terminal stderr while plain CLI stdout remains grep-compatible and empty. +Empty grep adds scanned/in-scope counts, served target/ref context when known, and `Do not repeat this grep unchanged.` Equal counts collapse to `files scanned: N (full 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. `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`. diff --git a/packages/mcp/src/shared/grep-repo-response.test.ts b/packages/mcp/src/shared/grep-repo-response.test.ts index 2954fcd9..fe3183f3 100644 --- a/packages/mcp/src/shared/grep-repo-response.test.ts +++ b/packages/mcp/src/shared/grep-repo-response.test.ts @@ -187,9 +187,33 @@ describe("formatGrepRepoTerminal", () => { expect(stdout).toContain("0 matches in 0 files"); expect(stdout).toContain("No matches."); - expect(stderr).toContain("files: 1 scanned | 1 in scope"); + 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 stdout silence and explains empty scope on stderr", () => { @@ -209,9 +233,9 @@ describe("formatGrepRepoTerminal", () => { }); expect(stdout).toBe(""); - expect(stderr).toContain("files: 0 scanned | 0 in scope"); + expect(stderr).toContain("files scanned: 0 (full scope)"); expect(stderr).toContain( - "loosen path, path_prefix, globs, extensions, or exclusion filters", + "loosen the optional path-prefix argument, --path, --glob, --ext, or exclusion flags", ); }); @@ -231,7 +255,7 @@ describe("formatGrepRepoTerminal", () => { useColors: false, }); - expect(stderr).toContain("More grep results available — rerun"); + expect(stderr).toContain("More matches available — rerun"); expect(stderr).toContain("--cursor 'next-page'"); expect(stderr).not.toContain("Do not repeat this grep unchanged."); }); @@ -423,7 +447,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 79dd079b..24c53393 100644 --- a/packages/mcp/src/shared/grep-repo-response.ts +++ b/packages/mcp/src/shared/grep-repo-response.ts @@ -605,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 a6a12af2..ab0369e7 100644 --- a/packages/mcp/src/shared/grep-repo-text.test.ts +++ b/packages/mcp/src/shared/grep-repo-text.test.ts @@ -37,13 +37,20 @@ describe("renderGrepRepoText", () => { expect(text).toContain("code_grep | 0 matches in 0 files"); expect(text).toContain('pattern="applyEdit"'); expect(text).toContain("No matches."); - expect(text).toContain("files: 120 scanned | 120 in scope"); + 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).toContain("check casing"); + 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({ @@ -53,7 +60,7 @@ describe("renderGrepRepoText", () => { }), ); - expect(text).toContain("files: 0 scanned | 0 in scope"); + expect(text).toContain("files scanned: 0 (full scope)"); expect(text).toContain("served=v5.2.1"); expect(text).toContain( "loosen path, path_prefix, globs, extensions, or exclusion filters", @@ -213,6 +220,17 @@ describe("renderGrepRepoText", () => { expect(text).not.toContain("Do not repeat this grep unchanged."); }); + it("humanizes deadline truncation", () => { + const text = renderGrepRepoText( + envelope({ + truncatedReason: "DEADLINE", + }), + ); + + expect(text).toContain("Truncated: time limit reached."); + expect(text).not.toContain("Truncated: DEADLINE."); + }); + it("renders pattern-type and case-sensitive flags in header when set", () => { const text = renderGrepRepoText( envelope({ diff --git a/packages/mcp/src/shared/grep-repo-text.ts b/packages/mcp/src/shared/grep-repo-text.ts index cfbd9d4e..f4722540 100644 --- a/packages/mcp/src/shared/grep-repo-text.ts +++ b/packages/mcp/src/shared/grep-repo-text.ts @@ -97,16 +97,17 @@ export function buildEmptyGrepGuidance( if (skipNotes.length > 0) lines.push(`Note: ${skipNotes.join(", ")}.`); if (envelope.truncatedReason) { + const reason = formatTruncationReason(envelope.truncatedReason); lines.push( surface === "cli" - ? `Truncated: ${envelope.truncatedReason}. Narrow the file selectors or increase --limit.` - : `Truncated: ${envelope.truncatedReason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + ? `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 grep results available — rerun with --cursor ${shellQuote(envelope.nextCursor)}` + ? `More matches available — rerun with --cursor ${shellQuote(envelope.nextCursor)}` : `More matches available. Pass cursor=${envelope.nextCursor} for the next page.`, ); } else if (envelope.hasMore) { @@ -115,11 +116,27 @@ export function buildEmptyGrepGuidance( if (envelope.truncatedReason || envelope.hasMore) return lines; lines.push("Do not repeat this grep unchanged."); - lines.push( - envelope.filesInScope === 0 - ? "next: loosen path, path_prefix, globs, extensions, or exclusion filters." - : "next: shorten or change the pattern, check casing, or use search for conceptual intent.", + 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; } @@ -127,7 +144,11 @@ function formatEmptyGrepFileCounts(envelope: LeanGrepRepoEnvelope): string { if (envelope.filesScanned < envelope.filesInScope) { return `files: ${envelope.filesInScope} in scope | ${envelope.filesScanned} content-scanned after index pruning`; } - return `files: ${envelope.filesScanned} scanned | ${envelope.filesInScope} in scope`; + return `files scanned: ${envelope.filesScanned} (full scope)`; +} + +function formatTruncationReason(reason: string): string { + return reason === "DEADLINE" ? "time limit reached" : reason; } function formatGrepServedTarget( @@ -170,7 +191,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/unified-search-status-text.ts b/packages/mcp/src/shared/unified-search-status-text.ts index 27cf3481..32b6c9b6 100644 --- a/packages/mcp/src/shared/unified-search-status-text.ts +++ b/packages/mcp/src/shared/unified-search-status-text.ts @@ -5,6 +5,7 @@ import type { } from "./unified-search-response.js"; import { appendEmptySearchGuidance, + appendIncompleteSearchNextAction, appendSourceStatusNotes, appendUnifiedSearchHits, formatProgressTarget, @@ -39,9 +40,10 @@ export function renderUnifiedSearchStatusText(payload: StatusPayload): string { if (result) appendResult(lines, result, payload.completed); if (!payload.completed) { - lines.push("Do not repeat search."); - lines.push( - `next: call search_status search_ref=${quote(payload.searchRef)}`, + appendIncompleteSearchNextAction( + lines, + payload.progress?.status, + payload.searchRef, ); } @@ -109,7 +111,3 @@ function formatProgress(progress: { }): string { return `progress: ${progress.status}, ${progress.targetsReady}/${progress.targetsTotal} targets ready, ${progress.elapsedMs}ms elapsed`; } - -function quote(value: string): string { - return JSON.stringify(value); -} diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index dfb9897e..0caa4f02 100644 --- a/packages/mcp/src/shared/unified-search-text.test.ts +++ b/packages/mcp/src/shared/unified-search-text.test.ts @@ -295,11 +295,39 @@ describe("renderUnifiedSearchSuccess", () => { const text = renderUnifiedSearchSuccess(incomplete); expect(text).toContain("1 partial"); expect(text).toContain("searchRef=ref_abc-123"); - expect(text).toContain("Indexing in progress. Do not repeat search."); - expect(text).toContain('Call search_status with search_ref="ref_abc-123".'); + expect(text).toContain("Indexing in progress.\nDo not repeat search."); + expect(text).toContain( + 'next: call search_status with search_ref="ref_abc-123".', + ); 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" }, @@ -393,6 +421,28 @@ describe("renderUnifiedSearchSuccess", () => { ); }); + 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", () => { const text = renderUnifiedSearchSuccess( completed([ diff --git a/packages/mcp/src/shared/unified-search-text.ts b/packages/mcp/src/shared/unified-search-text.ts index f429c1fe..c85f65d2 100644 --- a/packages/mcp/src/shared/unified-search-text.ts +++ b/packages/mcp/src/shared/unified-search-text.ts @@ -254,7 +254,7 @@ function buildTrailer( 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" @@ -265,10 +265,8 @@ function buildTrailer( `progress: ${payload.progress.targetsReady}/${payload.progress.targetsTotal} targets ready.`, ); } - lines.push(`${action} Do not repeat search.`); - lines.push( - `Call search_status with search_ref=${JSON.stringify(payload.searchRef)}.`, - ); + lines.push(action); + appendIncompleteSearchNextAction(lines, status, payload.searchRef); } if (options.includeSourceStatus) { @@ -286,6 +284,27 @@ function buildTrailer( 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( + status === "TIMEOUT" + ? "next: rerun search with a larger wait_timeout_ms." + : "next: rerun search.", + ); + return; + } + + lines.push("Do not repeat search."); + lines.push( + `next: call search_status with search_ref=${JSON.stringify(searchRef)}.`, + ); +} + function appendWarnings(lines: string[], warnings: string[] | undefined): void { if (!warnings || warnings.length === 0) return; lines.push("warnings:"); @@ -398,7 +417,7 @@ function formatEmptySearchHeadline( const sources = Array.from( new Set(sourceStatus.map((entry) => entry.source)), ).join(", "); - return `No hits across ${sources} sources.`; + return `No hits from any source (${sources}).`; } const entry = sourceStatus[0]; diff --git a/packages/mcp/src/tools/search-status.test.ts b/packages/mcp/src/tools/search-status.test.ts index abfc5552..05928faf 100644 --- a/packages/mcp/src/tools/search-status.test.ts +++ b/packages/mcp/src/tools/search-status.test.ts @@ -185,6 +185,25 @@ 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 with a larger wait_timeout_ms."); + 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("surfaces progress freshness warnings", async () => { @@ -333,7 +352,9 @@ describe("searchStatusTool", () => { expect(text).toContain("search_status | searching | searchRef=ref-text"); expect(text).toContain("progress: SEARCHING, 0/1 targets ready"); expect(text).toContain("Do not repeat search."); - expect(text).toContain('next: call search_status search_ref="ref-text"'); + expect(text).toContain( + 'next: call search_status with search_ref="ref-text".', + ); expect(text).not.toContain("searchRef=ref-text to follow up"); expect(() => JSON.parse(text)).toThrow(); }); diff --git a/src/commands/code/grep.test.ts b/src/commands/code/grep.test.ts index c526eb95..6c37b5f3 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(); @@ -542,7 +542,7 @@ describe("pkgGrepAction", () => { "Do not repeat this grep unchanged.", ); expect(errorWrites.join("")).toContain( - "use search for conceptual intent", + "use githits search for conceptual intent", ); expect(process.exitCode).toBe(1); } finally { From 0c1953da5e63913c39d734868c3eb270b23b1ab1 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 11 Aug 2026 12:49:00 +0300 Subject: [PATCH 3/4] fix: add bounded search status waits Expose backend progress waiting through MCP and CLI with the shared bounded default. Keep terminal-session recovery consistent across text and JSON, and align grep truncation guidance with normalized producer values. --- docs/implementation/cli-commands.md | 4 +- docs/implementation/mcp-cli-parity.md | 2 + docs/implementation/tools.md | 8 +- .../services/code-navigation-service.test.ts | 43 +++++++++++ .../src/services/code-navigation-service.ts | 18 +++-- .../mcp/src/shared/grep-repo-response.test.ts | 47 +++++++++++- .../mcp/src/shared/grep-repo-text.test.ts | 38 +--------- packages/mcp/src/shared/grep-repo-text.ts | 14 +++- .../shared/unified-search-response.test.ts | 29 ++++++- .../mcp/src/shared/unified-search-response.ts | 5 +- .../src/shared/unified-search-status-text.ts | 13 +++- .../src/shared/unified-search-text.test.ts | 2 +- .../mcp/src/shared/unified-search-text.ts | 15 ++-- packages/mcp/src/smoke-test.ts | 2 + packages/mcp/src/tools/search-status.test.ts | 76 ++++++++++++++++++- packages/mcp/src/tools/search-status.ts | 22 +++++- scripts/cli-smoke.ts | 16 +++- src/commands/search.test.ts | 64 ++++++++++++++++ src/commands/search.ts | 30 ++++++-- 19 files changed, 370 insertions(+), 78 deletions(-) diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index 46eefa42..45e8a570 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).** 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/checking casing or using conceptual `search`, and explicitly rejects an unchanged repeat. An incomplete empty page instead preserves truncation or `--cursor` continuation guidance. +**`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 dbe0a460..0eeed645 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 diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 90a15c2b..7aa64f8e 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -21,7 +21,7 @@ The CLI mirrors the production MCP tool contract where equivalent tools exist. C | `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 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`, `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_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. | @@ -217,7 +217,7 @@ More hits available. Pass offset=N for the next page or limit=N to widen. **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 continuation `next: call search_status with search_ref="...".` 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. +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): @@ -248,13 +248,13 @@ 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.` Equal counts collapse to `files scanned: N (full 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. +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`. 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 13bfaabe..ebb3f127 100644 --- a/packages/core-internal/src/services/code-navigation-service.test.ts +++ b/packages/core-internal/src/services/code-navigation-service.test.ts @@ -735,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( diff --git a/packages/core-internal/src/services/code-navigation-service.ts b/packages/core-internal/src/services/code-navigation-service.ts index 3f7c8172..ad8a7b73 100644 --- a/packages/core-internal/src/services/code-navigation-service.ts +++ b/packages/core-internal/src/services/code-navigation-service.ts @@ -528,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; @@ -946,8 +949,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 @@ -1831,13 +1834,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), }); } @@ -1914,6 +1920,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { private async executeUnifiedSearchStatus( token: string, searchRef: string, + waitTimeoutMs: number, ): Promise { let response: PkgseerGraphqlResponse; try { @@ -1923,6 +1930,7 @@ export class CodeNavigationServiceImpl implements CodeNavigationService { variables: { searchRef, includeResults: true, + waitTimeoutMs, }, }); } catch (cause) { diff --git a/packages/mcp/src/shared/grep-repo-response.test.ts b/packages/mcp/src/shared/grep-repo-response.test.ts index fe3183f3..151c27f4 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: [ @@ -233,7 +234,7 @@ describe("formatGrepRepoTerminal", () => { }); expect(stdout).toBe(""); - expect(stderr).toContain("files scanned: 0 (full scope)"); + 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", ); @@ -275,10 +276,54 @@ describe("formatGrepRepoTerminal", () => { 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", () => { const envelope = buildGrepRepoSuccessPayload( { diff --git a/packages/mcp/src/shared/grep-repo-text.test.ts b/packages/mcp/src/shared/grep-repo-text.test.ts index ab0369e7..01315653 100644 --- a/packages/mcp/src/shared/grep-repo-text.test.ts +++ b/packages/mcp/src/shared/grep-repo-text.test.ts @@ -60,7 +60,7 @@ describe("renderGrepRepoText", () => { }), ); - expect(text).toContain("files scanned: 0 (full scope)"); + 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", @@ -169,20 +169,6 @@ describe("renderGrepRepoText", () => { expect(text).toContain(" --"); }); - it("renders truncation notice when truncatedReason is set", () => { - const text = renderGrepRepoText( - envelope({ - totalMatches: 50, - uniqueFilesMatched: 7, - truncatedReason: "limit", - hasMore: true, - matches: [match()], - }), - ); - expect(text).toContain("Truncated: limit."); - expect(text).toContain("max_matches"); - }); - it("renders next-cursor note when hasMore", () => { const text = renderGrepRepoText( envelope({ @@ -209,28 +195,6 @@ describe("renderGrepRepoText", () => { expect(text).not.toContain("shorten or change the pattern"); }); - it("surfaces truncation on an empty incomplete result", () => { - const text = renderGrepRepoText( - envelope({ - truncatedReason: "limit", - }), - ); - - expect(text).toContain("Truncated: limit."); - expect(text).not.toContain("Do not repeat this grep unchanged."); - }); - - it("humanizes deadline truncation", () => { - const text = renderGrepRepoText( - envelope({ - truncatedReason: "DEADLINE", - }), - ); - - expect(text).toContain("Truncated: time limit reached."); - expect(text).not.toContain("Truncated: DEADLINE."); - }); - it("renders pattern-type and case-sensitive flags in header when set", () => { const text = renderGrepRepoText( envelope({ diff --git a/packages/mcp/src/shared/grep-repo-text.ts b/packages/mcp/src/shared/grep-repo-text.ts index f4722540..9cd27e94 100644 --- a/packages/mcp/src/shared/grep-repo-text.ts +++ b/packages/mcp/src/shared/grep-repo-text.ts @@ -141,6 +141,9 @@ export function buildEmptyGrepGuidance( } 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`; } @@ -148,7 +151,16 @@ function formatEmptyGrepFileCounts(envelope: LeanGrepRepoEnvelope): string { } function formatTruncationReason(reason: string): string { - return reason === "DEADLINE" ? "time limit reached" : reason; + 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( diff --git a/packages/mcp/src/shared/unified-search-response.test.ts b/packages/mcp/src/shared/unified-search-response.test.ts index 4b750d16..d2543b62 100644 --- a/packages/mcp/src/shared/unified-search-response.test.ts +++ b/packages/mcp/src/shared/unified-search-response.test.ts @@ -219,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', }, }); }); @@ -1557,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', }, }); }); @@ -1591,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 7253081c..9e81564f 100644 --- a/packages/mcp/src/shared/unified-search-response.ts +++ b/packages/mcp/src/shared/unified-search-response.ts @@ -562,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; } diff --git a/packages/mcp/src/shared/unified-search-status-text.ts b/packages/mcp/src/shared/unified-search-status-text.ts index 32b6c9b6..1ccd1eca 100644 --- a/packages/mcp/src/shared/unified-search-status-text.ts +++ b/packages/mcp/src/shared/unified-search-status-text.ts @@ -9,6 +9,7 @@ import { appendSourceStatusNotes, appendUnifiedSearchHits, formatProgressTarget, + noHitsYetMessage, } from "./unified-search-text.js"; const SEP = " | "; @@ -37,7 +38,14 @@ export function renderUnifiedSearchStatusText(payload: StatusPayload): string { } const result = payload.result; - if (result) appendResult(lines, result, payload.completed); + if (result) { + appendResult( + lines, + result, + payload.completed, + payload.completed ? undefined : payload.progress, + ); + } if (!payload.completed) { appendIncompleteSearchNextAction( @@ -63,6 +71,7 @@ function appendResult( lines: string[], result: UnifiedSearchStatusResultPayload, completed: boolean, + progress: UnifiedSearchStatusIncompletePayload["progress"] | undefined, ): void { lines.push(""); if (result.warnings && result.warnings.length > 0) { @@ -80,7 +89,7 @@ function appendResult( sourceStatus: result.sourceStatus, }); } else { - lines.push("No hits yet."); + lines.push(noHitsYetMessage(progress)); } } else { appendUnifiedSearchHits(lines, result.results); diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index 0caa4f02..ddf8d9cd 100644 --- a/packages/mcp/src/shared/unified-search-text.test.ts +++ b/packages/mcp/src/shared/unified-search-text.test.ts @@ -297,7 +297,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain("searchRef=ref_abc-123"); expect(text).toContain("Indexing in progress.\nDo not repeat search."); expect(text).toContain( - 'next: call search_status with search_ref="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"); }); diff --git a/packages/mcp/src/shared/unified-search-text.ts b/packages/mcp/src/shared/unified-search-text.ts index c85f65d2..daa4ac43 100644 --- a/packages/mcp/src/shared/unified-search-text.ts +++ b/packages/mcp/src/shared/unified-search-text.ts @@ -15,6 +15,7 @@ * `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 { @@ -75,11 +76,11 @@ export function renderUnifiedSearchSuccess( return lines.join("\n"); } -function noHitsYetMessage( - progress: UnifiedSearchIncompletePayload["progress"], +export function noHitsYetMessage( + progress: { status?: string } | undefined, ): string { const status = progress?.status; - if (status === "TIMEOUT") return "No hits yet - timed out waiting."; + 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."; @@ -291,17 +292,13 @@ export function appendIncompleteSearchNextAction( ): void { if (status === "FAILED" || status === "TIMEOUT") { lines.push("Do not call search_status again for this session."); - lines.push( - status === "TIMEOUT" - ? "next: rerun search with a larger wait_timeout_ms." - : "next: rerun search.", - ); + lines.push("next: rerun search."); return; } lines.push("Do not repeat search."); lines.push( - `next: call search_status with search_ref=${JSON.stringify(searchRef)}.`, + `next: call search_status with search_ref=${JSON.stringify(searchRef)} and wait_timeout_ms=${DEFAULT_WAIT_TIMEOUT_MS}.`, ); } diff --git a/packages/mcp/src/smoke-test.ts b/packages/mcp/src/smoke-test.ts index 54c4a793..a516fcdc 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 05928faf..325ccf53 100644 --- a/packages/mcp/src/tools/search-status.test.ts +++ b/packages/mcp/src/tools/search-status.test.ts @@ -67,6 +67,36 @@ describe("searchStatusTool", () => { 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 () => { const tool = createSearchStatusTool( createMockCodeNavigationService({ @@ -170,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 () => { @@ -186,7 +218,7 @@ describe("searchStatusTool", () => { 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 with a larger wait_timeout_ms."); + expect(text).toContain("next: rerun search."); expect(text).not.toContain("next: call search_status"); }); @@ -206,6 +238,46 @@ describe("searchStatusTool", () => { 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({ @@ -353,7 +425,7 @@ describe("searchStatusTool", () => { expect(text).toContain("progress: SEARCHING, 0/1 targets ready"); expect(text).toContain("Do not repeat search."); expect(text).toContain( - 'next: call search_status with search_ref="ref-text".', + '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(); diff --git a/packages/mcp/src/tools/search-status.ts b/packages/mcp/src/tools/search-status.ts index bbcb9ba9..235afb4d 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); while it is active, continue with `search_status` instead of repeating `search`."; + "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/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index f59c7376..73888b00 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/search.test.ts b/src/commands/search.test.ts index 638fe24c..5ce379d1 100644 --- a/src/commands/search.test.ts +++ b/src/commands/search.test.ts @@ -1049,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(() => {}); @@ -1126,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 0f5c3418..73547767 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,7 @@ import { knownSymbolCategoryList, knownSymbolKindList, type LeanTargetResolution, + MAX_WAIT_TIMEOUT_MS, type MappedError, parseUnifiedSearchTargetSpec, requireAuth, @@ -51,6 +53,7 @@ export interface SearchCommandOptions { } export interface SearchStatusCommandOptions { + wait?: string; json?: boolean; } @@ -131,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) { @@ -185,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 @@ -266,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(); @@ -336,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; } @@ -543,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") { @@ -554,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"); } From 92e84402da17993e6e22bb40b0ce13773b839420 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 11 Aug 2026 14:13:31 +0300 Subject: [PATCH 4/4] fix: make error metadata declaration-safe Use explicit undefined types with defaults for public error metadata constructor parameters so the standalone MCP declaration build succeeds without changing call-site behavior. --- .../src/services/code-navigation-service.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/core-internal/src/services/code-navigation-service.ts b/packages/core-internal/src/services/code-navigation-service.ts index ad8a7b73..afadb17e 100644 --- a/packages/core-internal/src/services/code-navigation-service.ts +++ b/packages/core-internal/src/services/code-navigation-service.ts @@ -600,7 +600,9 @@ export class CodeNavigationTargetNotFoundError extends Error { public readonly availableVersions?: AvailableVersion[], public readonly repoUrl?: string, public readonly requestedRef?: string, - public readonly metadata?: CodeNavigationErrorMetadata, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationTargetNotFoundError"; @@ -639,7 +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, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationVersionNotFoundError"; @@ -658,7 +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, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationRefNotFoundError"; @@ -707,7 +713,9 @@ export class CodeNavigationBackendError extends Error { * April 2026 `extensions.retryable` contract on GraphQL errors. */ public readonly retryable?: boolean, - public readonly metadata?: CodeNavigationErrorMetadata, + public readonly metadata: + | CodeNavigationErrorMetadata + | undefined = undefined, ) { super(message); this.name = "CodeNavigationBackendError";