From 5a3c8c0611241b52263b628fd87cdcb0250259a0 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 28 Sep 2026 16:20:11 +0300 Subject: [PATCH 1/6] feat: add unified top-level grep command Search ordered repository, package and hosted-document targets through Query.grep with regex and case-sensitive defaults. Preserve exact reads, coverage and continuation while retaining legacy CLI and MCP behavior. Record the dev small-page backend protocol blocker in the governing plan. --- README.md | 1 + changes/unified-grep-cli.added.md | 6 + docs/implementation/cli-commands.md | 3 +- docs/implementation/config.md | 2 +- docs/implementation/unified-grep.md | 103 +++ docs/plans/unified-grep.md | 712 ++++++++++++++++++ packages/core-internal/src/index.ts | 1 + .../src/services/grep-service.test.ts | 360 +++++++++ .../src/services/grep-service.ts | 536 +++++++++++++ packages/mcp/src/internal.ts | 4 + .../mcp/src/shared/grep-error-map.test.ts | 76 ++ packages/mcp/src/shared/grep-error-map.ts | 150 ++++ packages/mcp/src/shared/grep-request.test.ts | 166 ++++ packages/mcp/src/shared/grep-request.ts | 336 +++++++++ packages/mcp/src/shared/grep-response.test.ts | 301 ++++++++ packages/mcp/src/shared/grep-response.ts | 6 + packages/mcp/src/shared/grep-text.ts | 237 ++++++ packages/mcp/src/shared/mapped-error.ts | 2 + scripts/cli-smoke.ts | 84 +++ src/cli.test.ts | 3 + src/cli.ts | 2 + src/commands/grep.test.ts | 347 +++++++++ src/commands/grep.ts | 250 ++++++ src/commands/index.ts | 1 + src/container.test.ts | 3 + src/container.ts | 18 + src/services/test-helpers.ts | 22 + src/shared/cli-error-diagnostics.ts | 6 +- 28 files changed, 3735 insertions(+), 3 deletions(-) create mode 100644 changes/unified-grep-cli.added.md create mode 100644 docs/implementation/unified-grep.md create mode 100644 docs/plans/unified-grep.md create mode 100644 packages/core-internal/src/services/grep-service.test.ts create mode 100644 packages/core-internal/src/services/grep-service.ts create mode 100644 packages/mcp/src/shared/grep-error-map.test.ts create mode 100644 packages/mcp/src/shared/grep-error-map.ts create mode 100644 packages/mcp/src/shared/grep-request.test.ts create mode 100644 packages/mcp/src/shared/grep-request.ts create mode 100644 packages/mcp/src/shared/grep-response.test.ts create mode 100644 packages/mcp/src/shared/grep-response.ts create mode 100644 packages/mcp/src/shared/grep-text.ts create mode 100644 src/commands/grep.test.ts create mode 100644 src/commands/grep.ts diff --git a/README.md b/README.md index d9136c5a..ceb4b3e0 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,7 @@ broader open-source ecosystem, not just model memory or local repo context: | Code navigation | `search`, `search_status`, `code_files`, `code_grep` | `githits search`, `githits search-status`, `githits code ...` | | Documentation discovery | `docs_list` | `githits docs list` | | Read source files or documentation sections | `read` | `githits read [path]` | +| Grep source and hosted documentation together | CLI only (MCP migration pending) | `githits grep ` | | Package inspection | `pkg_info`, `pkg_vulns`, `pkg_deps`, `pkg_changelog`, `pkg_upgrade_review` | `githits pkg ...` | Use GitHits when your agent needs to: diff --git a/changes/unified-grep-cli.added.md b/changes/unified-grep-cli.added.md new file mode 100644 index 00000000..7a81f60c --- /dev/null +++ b/changes/unified-grep-cli.added.md @@ -0,0 +1,6 @@ +--- +"githits": minor +"@githits/mcp": none +--- + +- **Unified grep CLI** - Add `githits grep` across ordered package, repository and hosted documentation targets, defaulting to regex, case-sensitive matching and zero context, with `-F`, `-i`, rg-style `-s`, exact read actions and explicit partial coverage. Legacy CLI `code grep` and MCP `code_grep` remain available pending the MCP migration. diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index 0b3b06cf..d938b449 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -2,7 +2,7 @@ ## Purpose -The CLI exposes setup/auth commands, `doctor`, `example`, top-level indexed `search` / `search-status`, `read`, `list`, and the `code`, `docs`, and `pkg` command groups by default. `resolve` and `code diff` are experimental, host-config-gated commands. MCP-parity commands share business logic with the MCP tools through the same service interfaces and shared utilities. Unified search shares its presentation model and text formatter with MCP; `list` uses the shared request, result, error, and path-only text helpers, while MCP tool registration remains a later increment. +The CLI exposes setup/auth commands, `doctor`, `example`, top-level indexed `search` / `search-status`, `read`, `list`, `grep`, and the `code`, `docs`, and `pkg` command groups by default. `resolve` and `code diff` are experimental, host-config-gated commands. MCP-parity commands share business logic with the MCP tools through the same service interfaces and shared utilities. Unified search shares its presentation model and text formatter with MCP; `list` uses the shared request, result, error, and path-only text helpers, while MCP tool registration remains a later increment. ## Experimental CLI commands @@ -58,6 +58,7 @@ envelope when `--json` is requested; terminal output remains human-readable. | `pkg upgrade-review [spec]` | single package spec with current version plus `--to`, positional package range, OR repeatable `--package` ranges | `--to`, repeatable `--package`, `--no-transitive-security`, `--dependency-issues`, `--min-severity`, `--verbose`, `--json` | Compare current and target versions for upgrade evidence: vulnerabilities, changelog entries, deprecation metadata, peer changes, dependency changes, and transitive security evidence by default. Reports facts only. | | `docs list ` *(legacy compatibility)* | package spec (optional `@version`) | `--limit`, `--after`, `--verbose`, `--json` | Help points hosted-site browsing to `githits list site:` and package-local docs to the package target. Existing execution remains unchanged: text emits target-based read commands; JSON retains `docsReadTarget`, stable `pageId`, provenance `sourceUrl`, and exact repo-file metadata when available. | | `list [paths...]` | package, repository, or `site:` target; optional literal paths/globs | `-R, --recursive`, `-s, --silent`, repeatable `--file-type`, `--language`, `--intent`, `--limit`, `--after`, `--wait`, `--json` | List one package/repository source inventory, including package-local documentation files, or one explicitly targeted hosted site. Text is one path per line with `/` on directories; the header reuses backend-authored read targets for follow-up, while `--silent` emits only paths for piping. JSON carries exact actions, cursors, and metadata. | +| `grep ` | ordered package, repository and `site:` operands | `-F/--fixed-strings`, `-i/--ignore-case`, `-s/--case-sensitive`, `-A`, `-B`, `-C`, repeatable `--path`, `--path-prefix`, `--glob`, `--corpus`, `--limit`, `--cursor`, `--wait`, `--json` | Regex, case-sensitive and zero-context defaults; all repository files plus independently selected hosted package docs. Global page cap, exact reads and explicit coverage. See [unified grep](unified-grep.md). Legacy `code grep` remains unchanged. | | `read [path]` | docs target/page ID, explicit `site:` target with page path, compact `target#symbol`, or package/repo target with exact path or selector | `--selector`, `--lines`, `--start`, `--end`, `--wait`, `--verbose`, `--json`; `--repo-url` and `--git-ref` retain legacy repo addressing | Compact unified read passes the locator unchanged to the backend and presents the returned code, docs, or symbol-resolution type. A `site:` path selects hosted documentation; other exact paths narrow code selection. `--selector` selects a docs heading or indexed code symbol. HTTP(S) URL fragments and emitted repository docs page IDs retain their backend-resolved documentation behavior. The compact path calls `ReadService`/`Query.read` once. `--repo-url` remains the legacy compatibility path without selector. See [unified read](unified-read.md). | | `docs read ` (deprecated alias) | emitted `docsReadTarget` or historical page ID | `--lines`, `--verbose`, `--json` | Read a documentation page by preferred target or compatible page ID. Default output is content-only; `--lines` fetches a bounded range for long pages. | | `code diff ..` *(experimental; config-gated)* | unversioned package/repository target and exact range, or `--repo-url` and range | `--patch`, `--stat`, `--name-only`, `--name-status`, `--max-files`, `--max-patch-bytes`, `--verbose`, `--json`, one glob after `--` | Silently dogfood bounded repository-wide tree diffs resolved from package versions or repository refs; local-only MCP `code_diff` is available when experimental tools are enabled, while public/remote MCP and shared Agent Skill guidance remain unchanged | diff --git a/docs/implementation/config.md b/docs/implementation/config.md index feea7dde..c6a94041 100644 --- a/docs/implementation/config.md +++ b/docs/implementation/config.md @@ -74,7 +74,7 @@ The container (`src/container.ts`) resolves authentication in priority order: | `/search` | Full access | Full access | Blocked | | `/functions/v1/settings/me` | Full access | Full access | Blocked | -Package/source access uses the OSS service URL selected by `GITHITS_ENV` unless `GITHITS_CODE_NAV_URL` overrides it. MCP registration for `search`, `search_status`, `docs_*`, `pkg_*`, `code_files`, `read`, and `code_grep` is always on; CLI registration for top-level `search` / `search-status` / `read` / `list` plus the `githits code`, `githits pkg`, and `githits docs` groups is also always on. +Package/source access uses the OSS service URL selected by `GITHITS_ENV` unless `GITHITS_CODE_NAV_URL` overrides it. MCP registration for `search`, `search_status`, `docs_*`, `pkg_*`, `code_files`, `read`, and `code_grep` is always on; CLI registration for top-level `search` / `search-status` / `read` / `list` / `grep` plus the `githits code`, `githits pkg`, and `githits docs` groups is also always on. ## Environment Variables diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md new file mode 100644 index 00000000..b299bf88 --- /dev/null +++ b/docs/implementation/unified-grep.md @@ -0,0 +1,103 @@ +# Unified grep + +`githits grep ` searches ordered package, repository and +`site:` operands through `Query.grep`. MCP still exposes `code_grep`; +replacing it is Phase 2. Legacy `githits code grep` keeps its existing behavior. + +```sh +githits grep 'router' npm:express --path lib/express.js +githits grep -Fi 'router' npm:express site:expressjs.com +githits grep 'router' npm:express site:expressjs.com --json +githits grep -F -- '--foo' github:example/repository +``` + +## Matching and scope + +The client explicitly sends RE2 regex mode, case-sensitive matching, zero +context on each side and `ALL` repository corpus. Backend defaults differ. +`-F/--fixed-strings` opts into literal matching; `-i/--ignore-case` uses backend +Unicode folding. `-s/--case-sensitive` follows rg; traditional grep uses `-s` +to suppress errors. The last case flag wins, including short flag clusters. +RE2 and backend anchoring restrictions apply. Invalid or unsupported regexes +fail without a literal retry. + +`-A/--after-context`, `-B/--before-context` and `-C/--context` accept 0–10. +An explicit side overrides `-C` regardless of order. Every supplied value is +validated without clamping. `--limit` caps the entire page (1–1,000, backend +default 100), unlike grep/rg's per-file `-m`. `--wait` accepts 0–300,000 ms. +Empty cursors start page one; nonblank cursors remain opaque and unchanged. + +Repeatable `--path`, `--path-prefix` and `--glob` selectors are OR-ed within +each source operand. `--corpus source|documentation|all` controls repository +files. Global CLI source controls apply uniformly to source operands. Sites +receive only their target; source flags with only site operands fail. +Packages can also expand to selected hosted docs independently of repository +corpus and paths. `--corpus source` therefore does not exclude hosted docs. + +The backend owns resolution, expansion/deduplication, package boundaries, site +authority, selector matching and continuation. The client accepts 1–20 caller +targets and up to 1,000 selectors per source target. It sends `allowUnscoped: +true` for sources without weakening explicit selectors. Native expanded limits +remain eight repositories and eight sites. + +## Ownership and selection + +Core `services/grep-service.ts` owns transport-neutral types, the query, +allowlisting/validation and typed failures, reusing shared HTTP, headers, +diagnostics and token refresh. Root composition owns configuration discovery. +MCP `shared/grep-{request,response,error-map,text}.ts` owns frontend normalization, +projection, failure classification and presentation. Projection reuses the +core wire schema rather than maintaining another allowlist. The root command +owns Commander syntax, auth gating, spinners, diagnostics and exits. Shared +helpers remain workspace-internal in Phase 1; no public MCP tool/service is added. + +Compact text selects complete line/context slices, exact reads, scope +provenance/statuses, scan/skip counts, issue summaries, omissions, page count, +traversal and cursor. JSON additionally selects duplicate `lineContent`, hit +repository identities, display/physical byte coordinates, safety modifications, +issue byte details and full scope identities. Those fields use conditional +`@include(if: $includeDetailedFields)` selections. Missing selected fields or +unknown hit branches fail. Selected nulls stay null; excluded details stay +absent. JSON keeps camelCase fields without `hasMore` or an invented global total. + +## Results and recovery + +`totalMatches` counts this page. Physical scope `targetIndex` differs from +caller attribution in `requestedInputIndices`; producer order is retained. +Only consecutive compatible hits group together. Overlapping context merges, +match/context lines are numbered, and omitted line bytes are marked. Prose +wraps to caller width; source and executable actions remain intact. Terminal +controls and locator backslashes are escaped; backend Unicode is preserved. + +Read actions are backend-authored. Display paths can be package-relative while +read paths are repository-root paths at an exact commit. Hosted actions use +persisted URLs and read latest active content, which can change after search. +The client never hydrates hits or guesses paths. + +Stale/failed scopes, skips, issues, omitted issue counts, safety normalization +and unavailable targets stay visible on zero-hit pages. `No matches.` is +exhaustive only for complete traversal without coverage gaps. Other empty +pages report incomplete coverage. Cursors and terminal omissions can coexist; +both are shown. Continue with identical ordered operands and controls. +`CURSOR_EXPIRED` is a successful result requiring explicit restart; retained +sibling hits and omissions remain visible. + +Complete/partial pages exit zero, including zero hits. Failures exit nonzero; +JSON errors go to stderr with clean stdout. Preparation errors map to `INDEXING` +and preserve up to 20 public `targetIssues` with backend keys and per-input +recovery data. Invalid cursors map to `INVALID_ARGUMENT` with distinct +`graphqlCode`. Protocol, transport, auth, terms, update, deadline and HTTP +failures retain mapped categories. There is no legacy fallback or automatic +preparation retry/cursor restart. + +Focused tests cover query variables/selections, union validation, ordered +mixed hits, exact reads, coverage, context precedence, case flags, errors and +refresh. CLI smoke covers registration, unauthenticated errors and source +grep. Fresh mixed-site, pagination, read replay, case and corpus conformance +is checked against dev before Phase 1 signoff. + +Current dev limitation (2026-09-28): mixed/package requests with `--limit 1` +return backend `GREP_BACKEND_PROTOCOL_ERROR`. Mixed continuation at limit 100 +and pinned-repository continuation at limit 1 work. The client preserves the +requested limit and reports the typed error; small mixed-page live acceptance +and Phase 1 signoff await the backend correction. diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md new file mode 100644 index 00000000..450dffe4 --- /dev/null +++ b/docs/plans/unified-grep.md @@ -0,0 +1,712 @@ +# Unified grep client adoption + +## Status and outcome + +**Status: IN PROGRESS.** Phase 1 implementation is authorized via `$orchestrate`. +The requested sequence is two increments: introduce the top-level +CLI command first, then replace the advertised MCP `code_grep` tool. + +When complete, `githits grep` and MCP `grep` execute one backend `Query.grep` +page over ordered package, repository, and explicit site targets. Their shared +output preserves exact read actions, continuation, per-scope readiness and +traversal, and unavailable selected package docs. Existing `githits code grep` +remains a compatibility command with its original single-source controls. + +Overall assumptions: “top level” means CLI first, following the existing +`githits list` rollout. Package targets include selected hosted docs as the +backend specifies. Removing MCP `code_grep` in Phase 2 is explicitly requested; +retaining its CLI counterpart follows unified read/list compatibility policy. +**User-confirmed 2026-09-28:** mirror normal grep/rg where supported. Both new +surfaces default to RE2 regex, case-sensitive matching and zero context, with +explicit literal/case-insensitive/context opt-ins. CLI uses familiar flag +spellings, and source targets default to `ALL` repository corpus (indexed +source and documentation), with explicit corpus narrowing. The retained legacy +CLI keeps its existing defaults and controls +for compatibility. Backend case-sensitive support was verified from the schema, +`Grep.Request`, regex-validation forwarding and `MULTI_GREP` wire encoding; +fresh matching conformance is a Phase 1 acceptance check. +Overall product decisions: none blocking the proposed design. The CLI argument +order and whole-target convenience below are design proposals, not previously +user-confirmed preferences. Backend production readiness is an operational +unknown, not permission to change backend deployment or publication policy. +Dependencies: the checked-in backend contract, existing auth/transport helpers, +and Phase 1 before Phase 2. Completion criteria: both phases merged, their +surface-specific validation passing, and durable docs updated. Hosted adoption +is tracked separately from package/client completion. + +## Verified evidence + +Inspected on 2026-09-28 from client commit +`1739290b03ebcb8dee920f536c30f9ac1e0145e8` and backend checkout commit +`518e45d301d0ba3f451ff56034551addc2bfc7fe`. The schema file's SHA-256 is +`cbddb30fa7d08ac5af41799828767c607a704dc88880c33ae201e14c5d2cc672`. + +Canonical local evidence: + +- `~/proj/githits/pkgseer-backend/priv/graphql/schema.graphql`, `Query.grep` + and `Grep*` definitions; its `CHANGELOG.md` records unified grep under + Unreleased. +- Backend `lib/pkg_seer/grep/request.ex`, `lib/pkg_seer/grep.ex`, and + `lib/pkg_seer/grep/preparation.ex` verify defaults, site-option rejection, + scope/status indexing, terminal omissions, and error extensions. +- Backend `docs/plans/UNIFIED_GREP_GRAPHQL.md` records deployed-dev mixed-hit, + pagination, deduplication, no-docs omission, and read-action replay evidence. + Its deployment status says production still selects v5. This is recorded + backend evidence, not a fresh client-side production probe. +- Client `docs/implementation/unified-read.md`, + `docs/implementation/unified-list.md`, and `docs/plans/unified-list.md` + establish the CLI-first service/shared-helper/MCP migration pattern. MCP + `list` adoption remains a separate pending increment; grep must not assume it + has merged or migrate list in this work. +- `src/commands/code/grep.ts`, `packages/mcp/src/tools/grep-repo.ts`, and + `packages/mcp/src/shared/grep-repo-{request,response,text}.ts` own legacy grep. + It calls `CodeNavigationService.grepRepo`, not unified `Query.grep`. +- `packages/mcp/src/tools/tool-services.ts` requires `ReadService` but has no + unified grep service. `src/container.ts` already composes read/list services + in both authenticated and unauthenticated paths. + +The existing formatter was executed with a one-hit fixture: a summary, one file +heading, and `19: var Router = require("router");`. Preserve that concise match +presentation. The current full `code_grep` descriptor from +`getMcpToolDescriptors()` serializes to **7,161 UTF-8 bytes**. That is a +descriptor-size baseline, not a runtime performance or model-token measurement. + +Verified contradiction: legacy `GREP_REPO_PATTERN_NOTE` describes ASCII-only +case folding, while the current backend `grepRepo.caseSensitive` schema +documents Unicode-aware folding. New guidance must use the backend contract. +The retained CLI help also says “Enable ASCII case-sensitive matching” in +`src/commands/code/grep.ts`. Phase 2 corrects both strings alongside guidance migration +(therefore both artifacts' documented surfaces change). This design-only +increment changes no production tool descriptions. + +### Backend contract + +`grep(targets:, pattern:, patternType:, caseSensitive:, contextLinesBefore:, +contextLinesAfter:, maxMatches:, cursor:, waitTimeoutMs:)` returns `GrepResult`. + +| Contract | Verified behavior | +| --- | --- | +| Caller targets | 1–20 ordered objects, each with compact `target`; expanded scopes are capped at eight repositories and eight sites by the backend | +| Source controls | Per-target `corpus` (`SOURCE`, `DOCUMENTATION`, `ALL`), typed `pathSelectors` (`EXACT`, `PREFIX`, `GLOB`), and `allowUnscoped` | +| Site controls | Scope lives in `site:host[/scope]`; setting any repository-only field, even `allowUnscoped: false`, is invalid | +| Corpus and package expansion | Backend default `SOURCE` selects repository source files; new client explicitly defaults repository corpus to `ALL`; selected hosted package docs are added independently of corpus and source selectors | +| Selectors | OR within a source target; package-relative or repository-relative; package subpath is applied by the backend; explicit site targets never use these selectors | +| Pattern | Backend literal default or RE2; 1–200 UTF-8 bytes, valid Unicode, no NUL; preserve whitespace patterns; default case-insensitive matching is Unicode-aware | +| Defaults | Backend literal pattern, case-insensitive, two context lines per side, 100 matches per page, zero wait; the new client explicitly sends REGEX, caseSensitive true and zero context, omitting absent page/wait controls | +| Bounds | Context 0–10 per side; matches 1–1000 globally; wait 0–300000 ms | +| Continuation | Opaque base64url cursor; replay identical ordered targets and search controls; continuation performs no waiting or work admission | +| Hits | Repository/site union, physical scope `targetIndex`, bounded line/context slices, display/physical byte coordinates, content safety, exact `read` action | +| Scope statuses | `targetIndex`, original `requestedInputIndices`, readiness, traversal, retryability, safe message, requested/served identity, scan/skip counts and file issues | +| Omissions | Terminal selected package-doc omissions accompany source hits in `unavailableTargets`; other unready scopes fail preparation before dispatch | +| Coverage | `COMPLETE`, `RESUMABLE_LIMIT`, `NON_RESUMABLE_PARTIAL`, `FAILED`, or `CURSOR_EXPIRED`; a null cursor alone never proves completeness | + +A hit's `targetIndex` indexes a dispatched physical scope, not the caller's +target list. Backend expansion and deduplication can map one input to several +scopes and several inputs to one scope. Preserve `requestedInputIndices` and +producer order; do not expand, deduplicate, sort, or merge target identities in +the client. + +Repository read actions contain the exact snapshot target, repository-root +`path`, and line bounds. A displayed package-relative `filePath` is not that +path. Site actions contain the exact persisted page URL and bounds; they open +current active hosted content, without a historical snapshot guarantee. Replay +each backend action unchanged; do not construct one from display coordinates. + +## Scope and compatibility + +Include the new service, shared request/error/result/text helpers, top-level +CLI command, later MCP adapter/provider exports, active guidance/recovery +migration, tests, smoke coverage, targeted agent evals, docs, and per-phase +release fragments. No backend edits, deployment, publication, client target +preparation, retries, polling, pagination loops, or new infrastructure. + +Unified grep does not expose legacy `extensions`, `exclude_doc_files`, +`exclude_test_files`, `max_matches_per_file`, or `symbol_fields`. Do not +simulate these with post-filtering: that would change page membership, limits, +and coverage. `corpus` is not an equivalent documentation exclusion control; +it also does not remove hosted package docs. Regex limitations remain backend +owned. `search` retains ranked/conceptual discovery; `list` retains inventory. + +Phase 1 leaves the MCP catalog and legacy CLI execution unchanged. Phase 2 +removes advertised MCP `code_grep` without an alias or legacy-root fallback. +Callers needing removed source controls can continue using `githits code grep`. +Document this capability change alongside the new mixed-source behavior. +Do not remove legacy helper modules needed by that CLI command. +The default preparation wait changes from legacy grep's 30 seconds to zero, +following unified list. A cold target now returns +`GREP_TARGET_PREPARATION_REQUIRED` unless the caller supplies `--wait` or +`wait_timeout_ms`; migration guidance must state this explicitly. Other changed +defaults are case-sensitive matching (legacy insensitive) and 100 matches +(legacy 50); zero context agrees with legacy grep, while overriding the new +backend's two-line default. Context above ten is rejected rather than clamped. +The new grep defaults to RE2 instead of the legacy literal mode. Metacharacters +therefore change meaning, and malformed regexes return their typed errors. +Literal migration uses CLI `-F/--fixed-strings` or MCP `pattern_type: literal`. +Do not retry an invalid or unsupported regex as a literal. Multi-file regexes +need the backend's usable literal anchor; broad patterns such as `.*` cannot be +advertised as unrestricted scans. An exact-file scope can support patterns +that the content-index route cannot. Help and errors must explain these +boundaries without claiming full ripgrep regex compatibility. +Its default source corpus is `ALL`, covering indexed repository source and +documentation files together, closer to ordinary grep/rg text search than the +backend's source-only default. Explicit `corpus: source` or `documentation` +narrows repository files; neither controls package-selected hosted docs. + +## Target architecture and ownership + +**Core owns the backend contract:** add +`packages/core-internal/src/services/grep-service.ts` with `GrepService`, +`GrepServiceImpl`, request/result types, GraphQL selection, Zod validation, and +grep-specific errors. Reuse `postPkgseerGraphql`, `executeWithTokenRefresh`, +existing timeout/terms/client-update conventions, injected `TokenProvider`, +fetch, client headers and `ServiceDiagnostics`. The service makes one grep +request per page apart from existing auth refresh. It discovers no diagnostics +environment or output destination and validates network settings only on use. +Extending `CodeNavigationService.grepRepo` would make a legacy source-only +contract own mixed site semantics; a separate narrow service follows list/read. + +**Shared MCP helpers own surface semantics:** add `grep-request.ts`, +`grep-error-map.ts`, `grep-response.ts`, and `grep-text.ts` under +`packages/mcp/src/shared/`. CLI and MCP must use the same input builder, +allowlisted result projection, mapped errors, and formatter. This is the +existing cross-surface boundary; root CLI helpers would be unavailable to the +public MCP package. Do not introduce a generic unified-operation framework. + +**Adapters own invocation only:** root CLI owns Commander flags, auth entry, +spinner, process output and exit status. MCP owns its Zod schema, cancellation +and tool result envelope. Phase 1 injects the new service through root +`src/container.ts`; Phase 2 adds required `grepService` to `McpToolServices` +and the request-scoped provider seam. Publish stable service/types/implementation +through `@githits/mcp/client` and public provider-facing types through +`@githits/mcp`. Keep root helper exports workspace-only via `internal.ts`. +Never expose private package imports or filesystem dependencies in artifacts. + +Data flow: CLI flags or MCP arguments → shared request builder → `GrepService` +→ one `Query.grep` page → validated discriminated result → shared projector +→ shared text or JSON → surface output. There is no read/hydration per hit. + +### Proposed caller surfaces + +CLI: + +```sh +githits grep 'router' npm:express +githits grep 'require' github:expressjs/express --path lib/express.js +githits grep 'middleware' hex:plug@1.18.1 site:hexdocs.pm/plug +``` + +Use `grep `: grep convention keeps one pattern before +an ordered list of scopes. Shell quoting owns regex/glob protection. Support +`-F/--fixed-strings` for literal mode, `-i/--ignore-case`, +`-s/--case-sensitive`, repeatable `--path`, +`--path-prefix`, and `--glob`, `--corpus source|documentation|all`, +`-B/--before-context`, `-A/--after-context`, `-C/--context`, `--limit`, +`--cursor`, `--wait`, and `--json`. Reuse established legacy grep flags so +users need no new vocabulary for equivalent controls. CLI context normalization +uses the explicit side value, else symmetric context, else zero. The shared +builder sends the two resolved values explicitly. Validate all supplied context +values as 0–10, without legacy clamping. The MCP schema keeps its two independent +side controls and does not gain symmetric-context/override mechanics. +The legacy `--regex` flag is unnecessary on the new command because regex is +the user-requested default; `-F` follows familiar grep literal-mode spelling. +When case flags are repeated or combined, the last `-i`/`-s` wins, following +rg. The case-sensitive `-s/--case-sensitive` spelling specifically follows rg; +grep's `-s` instead suppresses error messages, so no grep equivalence is claimed +for that flag. Short options are command-scoped: existing `list -s` means +silent and `pkg vulns -s` means severity. Accept the grep-specific meaning here +for rg compatibility; the parser never shares these command options. +`--limit` remains a global page cap: do not alias it as +`-m/--max-count`, which grep/rg define per file. Native regex syntax and scope +limits are documented differences, not claims of complete grep/rg parity. +Omitted case controls resolve to backend `caseSensitive: true`; `-i` or MCP +`ignore_case: true` resolves to false. MCP `ignore_case: false` preserves the +default. The shared caller input is `ignoreCase`, with one inversion when +building backend `GrepParams`; do not expose a second MCP case boolean. +Document `--` before a leading-dash pattern, for example +`githits grep -- '--foo' github:example/repo`; add a CLI regression case. + +| Default or convention | New CLI and MCP behavior | Reason for any departure | +| --- | --- | --- | +| Pattern mode | Regex; explicit literal opt-in | RE2 is backend-supported; grep BRE and PCRE modes cannot be promised | +| Case and context | Case sensitive, zero context | Matches grep/rg; explicitly override backend defaults | +| Repository text corpus | `ALL`, source plus documentation; optional narrowing | Matches ordinary text search within the available index | +| Results per page | 100 by default, 1–1000; `--limit`, resumable cursor | Remote response bound; `-m` would falsely imply a per-file limit | +| Context ceiling | 10 lines per side | Backend contract; larger windows use exact `read` actions | +| Readiness wait | Zero; explicit `--wait` | Existing index/preparation boundary; no hidden retry or polling | +| Output | Numbered source/context lines with coverage summary and exact read actions | Remote provenance and partial coverage must remain visible | +| Search scope | Supplied indexed targets, with selected package docs | Backend target/authority contract; no local file, unindexed, hidden-file or ignore-policy parity claim | + +The CLI's source flags apply uniformly to every package/repository operand. +Site operands retain their own target scope and receive no repository fields. +Explicit source flags with only site operands are an input error. Help must +state that source flags do not narrow selected hosted package docs. Different +source selectors per input are supported in the shared request model and MCP, +not by a second JSON argument syntax in this first CLI increment. + +Proposed MCP arguments: + +```json +{ + "targets": [ + {"target": "npm:express", "path_selectors": [{"kind": "exact", "value": "lib/express.js"}]}, + {"target": "site:expressjs.com/en/5x"} + ], + "pattern": "router", + "pattern_type": "regex", + "ignore_case": false, + "context_lines_before": 0, + "context_lines_after": 0, + "max_matches": 100, + "format": "text" +} +``` + +`targets` is a required array of objects with required `target`, optional +`corpus`, and optional `path_selectors` of `{kind: exact|prefix|glob, value}`. +An omitted source `corpus` resolves to `all`; explicit sites have no corpus +field at all. The builder sends the resolved source enum, without changing +target identity or performing client-side file classification. +Pattern is required. Other top-level options are optional `pattern_type`, +`ignore_case`, `context_lines_before`, `context_lines_after`, `max_matches`, +`cursor`, `wait_timeout_ms`, and `format: text|json` (text default). +Omitting `pattern_type` selects regex, `ignore_case` selects false, and either +context side selects zero. The shared request builder owns these +user-facing defaults and resolves required `GrepParams.patternType`, +`caseSensitive`, `contextLinesBefore` and `contextLinesAfter`; the service +always sends these controls, so differing backend defaults cannot change the +new tool's meaning. Explicit `pattern_type: literal`, `ignore_case: true`, and +nonzero context remain independent options. The negative case-control name +avoids an agent-facing default-true boolean. Descriptor and CLI help state +all three defaults and their opt-ins. +Avoid string/object unions, top-level source filters, and coupled booleans. + +Whole-target grep remains convenient as in current `code_grep`: the builder +sends `allowUnscoped: true` for source targets, including explicit selectors. +This permits match-all globs without introducing an exposed default-true flag; +it does not remove or widen selectors or package boundaries. Never send it, +including false, for sites. This is an explicit design choice: a grep request +already expresses the intention to search the supplied scopes. Backend +authority, corpus validation and native limits remain authoritative. + +### Validation and output contracts + +Preserve ordered target/selector values and nonempty cursor bytes; do not parse +package/provider refs to reconstruct them. Validate empty target arrays, +blank targets, invalid Unicode/NUL patterns, byte/count/integer bounds, +blank selector entries, and repository fields on explicit sites. A whitespace +literal pattern is valid. Empty optional selector arrays mean omitted; a blank +optional cursor means page one. Preserve `false`, zero context/wait, and nullable +response fields. Do not clamp context above ten. Native expansion, site +authority, package boundaries, selector matching and cursor decoding stay in +the backend. Unknown hit union branches or malformed selected fields are +service errors, never empty results. +Normalize empty selector arrays to absence before validating site fields; +nonempty site selectors and any explicit site corpus are invalid. Send neither +the source corpus default nor `allowUnscoped` for a site. + +JSON retains every selected backend field under its camelCase name, including +`__typename`, nulls, line slices, both offset coordinate systems, read actions, +statuses, file issues, omitted issue counts, omissions and cursor. Do not copy +legacy `hasMore`, filter echoes, unique-file counts, or a global-total fiction. + +Text leads with page match count and traversal, then concise scope status and +match blocks. Group only consecutive compatible hits by physical scope and +locator, preserving producer order. Reuse numbered `line: content` matches, +`line- content` context, and visible slice omission markers. Escape terminal +controls and locator backslashes; preserve backend Unicode. Keep prose wrapped +to caller width, source lines intact, formatter punctuation ASCII, and meaning +independent of color. CLI and MCP use the same formatter with color/width inputs. +No new highlight-offset processing is needed in compact text. + +Each block carries its backend-authored read target/path and line action(s); +identical actions may be printed once. Show warnings for stale/failed/unready +scopes, skipped/issue-bearing files, safety normalization, and unavailable docs +even when hits are empty. Say “No matches” with exhaustive meaning only for +complete traversal and no omissions, scope failures, skipped or issue-bearing +content; otherwise report zero +returned matches with the coverage reason. A nonempty cursor is emitted with +an instruction to replay the same ordered targets and controls. A partial page +may have a cursor as well as omissions: show both. Cursor expiry requires an +explicit caller restart; never auto-retry or discard successful sibling hits. + +Minimal fetching is part of this contract: + +| Selection | Compact text | JSON detail | +| --- | --- | --- | +| Result traversal, cursor, page count; scope indexing, input mapping, readiness/traversal/errors/retryability; omissions including progress/suggested scopes | Required | Required | +| Hit kind, scope index, repository display path or page URL, line, complete line/context slice fields, exact read action; safety `filtered` | Required | Required | +| Scope target/requested ref/served commit/corpus; scan/skip counts; file issue path/code/line, issue safety `filtered`, and omitted count | Required for provenance and coverage notes | Required | +| Duplicate hit `lineContent`, hit repo URL/commit/repository path; display/physical match offsets; hit/issue safety modifications; issue `lineBytes` and match byte coordinates; scope repo URL/canonical site/URL prefixes | Omitted | Required | + +Use one document with `includeDetailedFields` directives (or equivalent +separate selections if simpler), model excluded fields as absent and selected +nullables as null. Detailed `contentSafety` selects `filtered` and +`modifications`; `modifications` is an enum list with no subfields. +Do not request redundant text or offsets just because the legacy query did. +Test actual document/variables and omitted selections; formatter mocks alone +cannot prove this. The text formatter uses slice `content` directly. + +Map whole-request errors through the existing `MappedError` envelope while +preserving bounded public GraphQL extensions. Specifically retain +`GREP_TARGET_PREPARATION_REQUIRED.target_issues`, input indices, retryability, +reasons and progress references; source/file recovery must use its matching +target and path. Keep `GREP_CURSOR_INVALID`, service/protocol errors, auth, +terms, required-client-update, timeout, HTTP and malformed-response distinct. +Do not parse public messages for routing or convert successful partial results +into whole-request failures. No legacy-root schema fallback. CLI exits zero +for valid complete or partial result pages, including zero hits, and nonzero +for request/service failures; coverage is explicit in text/JSON. This follows +the existing CLI's successful-empty-result convention. + +## Ordered phases + +### Phase 1 — top-level CLI mixed grep (IN PROGRESS) + +Implementation checkpoint (2026-09-28): + +- Core query/types/runtime validation, shared request/projection/error/text, + root command and both auth branches are implemented. Projection reuses the + core wire allowlist. No MCP catalog/public-provider or legacy CLI changes. +- Corrected selector validation to 1,000 per target after tracing backend + `GrepRepo.canonical_scope`; a two-target 2,000-selector fixture verifies it. +- Native `aigrep-grep` `MatchIter` emits individual matches, including several + on one line. The formatter keeps different slice windows in separate blocks. +- `bun test`: 5,094 passed / 0 failed, 18,504 assertions across 222 files. + Typecheck, build, formatting and public-package validation pass. Source + CLI/MCP unauthenticated smoke passes; built Node CLI/MCP smoke passes. + Internal review is clean after the repeated-context correction. External + implementation review remains pending. +- Authenticated `GITHITS_ENV=dev bun run smoke:cli` fails at the strict new + `router npm:express@5.2.1 --path lib/express.js --limit 1 --json` assertion + with the backend protocol error below. Earlier stable smoke assertions pass; + subsequent assertions are not reached. Authenticated MCP smoke is in progress. +- Fresh dev authentication now works through the default macOS Keychain. No + credentials were printed. Express source and mixed source/site requests + return both hit kinds; selected/explicit site attribution is `[0, 1]`. + Repository and hosted exact reads replay successfully. Case-sensitive and + ignore-case behavior passes for both hit kinds; `ALL` includes `Readme.md` + while `source` excludes it and retains independently selected hosted docs. + A site-only absent literal returns complete zero hits without omissions. +- Fresh evidence contradicts the historical Plug fixture: package-only + `middleware` currently returns complete zero hits, while explicit + `site:hexdocs.pm/plug` fails with nonretryable `site_ambiguous`. Use verified + `npm:express` plus `site:expressjs.com` for current conformance instead. +- Pagination works with two distinct mixed pages at limit 100, and two + distinct pinned-repository pages at limit 1. Mixed/package requests at limit + 1 fail before client projection with backend + `GREP_BACKEND_PROTOCOL_ERROR` / retryable true. This also affects the new + strict live CLI smoke. Keep the requested limit and surface the typed error; + do not add a fallback or alter the smoke to conceal it. Small mixed-page + acceptance remains UNPROVEN until the backend is fixed and replayed. + + + +Expected outcome: users can grep ordered source/site scopes with one CLI +invocation and replay exact reads or continuation, while MCP and the legacy +CLI command retain current behavior. + +Assumptions: existing read/list service wiring and transport conventions remain +applicable; the backend owns target expansion and preparation. Unknowns: small mixed-page backend protocol failure remains unresolved. Fresh +authenticated dev replay of that case must pass before signoff. Production +conformance is outside this increment. Production v6 promotion is not a Phase 1 dependency. +Product decisions: none blocking implementation of this proposal. +Dependencies: backend `Query.grep` and dev v6 access for mixed-source validation. + +Implementation order: + +1. Add core grep interfaces/implementation and tests; export them internally + from `packages/core-internal/src/index.ts`. Follow narrow list-service + transport/error ownership, retaining structured preparation issues. +2. Add shared request/error/projector/formatter helpers and tests, with a + private workspace export in `packages/mcp/src/internal.ts`. Separate union + payloads from the legacy `GrepRepoResult`/`LeanGrepRepoEnvelope`. +3. Wire `grepService` in both root container paths and add a service mock + factory following `test-helpers.ts`. Add `src/commands/grep.ts`, its tests, + command-index export and `src/cli.ts` registration. Network config remains + lazy on help/local-only paths; cover malformed env regressions. +4. Add CLI smoke assertions in `scripts/cli-smoke.ts`; preserve existing MCP + catalog assertions. Update CLI reference/help and create + `docs/implementation/unified-grep.md` with architecture, coverage semantics, + action replay and compatibility limits. Add a fragment declaring + `githits: minor`, `@githits/mcp: none` because the public MCP API/catalog is + unchanged. Reassess if implementation actually alters a public export. + +Acceptance and evidence: + +- Pure request tests cover ordered multi-target inputs, global CLI source + controls, per-target selectors, sites, empty arrays/strings, whitespace + patterns, multi-byte patterns, explicit false/zero, and numeric bounds. + Omitted pattern mode sends `REGEX`; `-F`/explicit literal sends `LITERAL`. + Omitted case/context send true/0/0; `-i` sends false; combined case flags + honor last-flag precedence; `-C` sets both sides and `-A/-B` override their + respective side. MCP `ignore_case: true` sends backend false, while omitted + or explicit false sends true. Backend false/zero are preserved on the wire. + Omitted source corpus sends `ALL`; explicit source/documentation narrowing + is preserved, and sites receive neither corpus nor scan-permission fields. + Patterns with metacharacters retain their bytes in each mode; malformed or + anchorless regex errors are surfaced without a literal retry. +- Core service tests assert one grep operation per page, exact compact/detail + wire variables/selections, both hit kinds, null/absence fidelity, preparation + issues, unknown unions/malformed fields, auth refresh, terms, client-update, + transport/deadline errors, and no call to `grepRepo` or per-hit reads. +- Projector/formatter tests cover multiple input-to-scope mappings, monorepo + display/read path distinction, safe slices, context, producer order, no-hit + complete vs partial pages, stale scopes, target-local failures with sibling + hits, file issues including issue-bearing zero-hit pages, terminal omissions, + a cursor plus omissions, and exact + backend action/cursor preservation. Status indices must resolve each hit. + Include a `CURSOR_EXPIRED` page with hits/omissions: preserve sibling hits + and omissions, show explicit restart guidance, and make no automatic retry. +- CLI tests prove flags map to the shared builder, successful-empty/partial + exit behavior, source-only flags on site-only requests fail, errors preserve + per-input recovery, leading-dash patterns after `--` reach the service + unchanged, and the legacy command remains unchanged. +- Run targeted `bun test` for the new modules/container/command first, then + required `bun test`, `bun run typecheck`, `bun run build`, + `bun run smoke:cli`, and `bun run smoke:mcp`. Smoke must pass unauthenticated + auth handling; authenticated affected paths provide deeper evidence. Built + smoke suites are required if launch behavior or CI validation changes. +- With `GITHITS_ENV=dev` and unintended URL overrides removed without printing + credentials, verify: `router` in `npm:express` with exact `lib/express.js`; + `router` in `npm:express` plus `site:expressjs.com` (both hit kinds + and selected/explicit site deduplication); that mixed request with + `--limit 1` over two pages; and a complete zero-hit site request. + Replay emitted repository and hosted read actions, not guessed paths. + Compare the same known repository literal in its actual spelling and a + changed-case spelling with case-sensitive and `-i` requests; repeat on an + available v6 page to establish both native hit kinds respect the flag. + Confirm the default `ALL` source target includes a known documentation-file + literal and an explicit `source` request narrows it, without misrepresenting + independently selected hosted docs as filtered out. + Native documented caps, typed preparation/terminal omission shapes, and + package-boundary isolation remain unit/fixture cases unless fresh live + evidence is available. Record live outcomes and endpoint readiness accurately. + +No optimization is proposed and no latency claim is made. There is no existing +unified-grep runtime baseline to compare. If implementation begins optimizing +formatting, add a narrow reproducible built-output size fixture first: 100 +repository hits at one served commit and 100 mixed repository/site hits with +one omission and cursor, before/after the same cases. Do not benchmark the +whole search/navigation suite or report debug-build timings. + +### Phase 2 — MCP `grep` replaces `code_grep` (PENDING Phase 1) + +Expected outcome: the advertised MCP catalog has one mixed-source `grep` tool; +agents receive the same reads, pagination and truthful coverage as CLI users. +Legacy source-only flags disappear from MCP, with migration documented. + +Assumptions: Phase 1 semantics and shared helpers prove sufficient; required +provider service additions follow the existing read-service precedent. +Unknowns: current main's list consolidation status and backend production v6 +status must be rechecked at this boundary. Resolve routing against whatever +inventory tool is actually advertised, without taking ownership of list work. +Product decisions: none; MCP removal is requested. Dependencies: Phase 1 merged, +public package compatibility validation, and dev access for MCP conformance. + +Implementation order: + +1. Add `packages/mcp/src/tools/grep.ts` with the proposed schema, + `readOnlyHint: true` and existing open-world annotations. Inject + `GrepService`, delegate to the Phase 1 helpers, and preserve caller + cancellation. Required `grepService` joins `McpToolServices`; update + providers, local server composition, descriptor-only service stubs and + mock factories. Export stable grep service types/implementation via + `client.ts` and necessary provider types via `index.ts`. +2. Replace `createGrepRepoTool` in the stable MCP catalog, not merely its name. + Remove obsolete MCP-only registration/exports/tests; retain the legacy CLI + service/request/output dependencies. Add root CLI/MCP parity tests for + structured inputs and JSON results. +3. Update active grep-specific instructions, tool descriptions, error/recovery + actions, quick-start guide, public skills, README/CLI and implementation + references, smoke catalog assertions, eval expectations and current tool + counts. Keep unrelated and historical descriptions/results intact. Where + `code_grep` actions currently carry legacy flags, construct valid unified + target entries or remove the unsupported action with truthful guidance; + never mechanically rename incompatible payloads. +4. Keep `buildMcpQuickStart()` and the public skill's terminal guide in exact + parity. Follow the documented stable-guide lifecycle exception: backing + behavior, builder and terminal guide change in the same Phase 2 PR, accepting + the bounded main-to-release window; do not invent a deploy-first requirement. + Leave behavior-dependent onboarding skill updates for the release boundary. + Follow the public Agent Skill lifecycle for behavior-dependent + guidance: authored root `skills/` inputs only, `plugins:generate` and + `plugins:check`, never generated-file edits. Add a migration fragment with + explicit `githits: minor`, `@githits/mcp: minor` (current pre-1.0 MCP breaking + provider/catalog change); confirm versions and release policy at execution. + The fragment must call out regex replacing literal as the default, literal + opt-in, all other changed matching/output defaults, and zero replacing the + legacy 30-second preparation wait. + Update durable docs with the actual final public schema and API migration. + +Acceptance and evidence: + +- Catalog tests advertise `grep` and exclude `code_grep` in stable descriptors + and local server registration; all information tools remain read-only. + First sentence: “Find regex or literal matches across source and documentation.” + (under 79 characters, no internal periods). First-80 tests and field + descriptions must independently explain package hosted-doc inclusion, + all-corpus default, per-target scopes, regex/case/context defaults and opt-ins, exact + reads and continuation. +- Handler/provider/parity tests prove the shared request/result/text/error + semantics, structured multi-target calls, explicit false/empty arrays, + regex/case-sensitive/zero-context/all-corpus defaults and explicit opt-ins, + read-action fidelity and no + legacy service execution. +- Run required unit tests, typecheck, build, plugin parity/generation checks + and CLI/MCP smoke; run built smoke if launch/CI validation changes. + Validate the packed public MCP package from outside root path aliases, + including an external request-scoped provider supplying `grepService`. +- Authenticated MCP smoke repeats the Phase 1 package/mixed/two-page/read + cases against dev and checks truthful partial/omission handling. Production + smoke expectations must respect v5/v6 readiness rather than assume sites work. +- Run `bun run agent:e2e --agent codex --server local --guidance-profile + descriptors --workload eval/agentic/workloads/code-grep-investigation.md` + and the matching Claude run. Add one focused mixed-docs workload: find the + known `middleware` literal in Plug source and hosted docs, then reopen the + returned evidence. Run both descriptor-only agents for it; use the full + guidance profile if broad instruction changes warrant it. Inspect + `tool-calls.json`, `final.json`, `metrics.json`, and + `isolation-violations.json`; report tool use, confidence and measured cost, + not ungraded usefulness claims. +- Compare the new full `grep` descriptor's serialized UTF-8 bytes with the + 7,161-byte baseline using the same `getMcpToolDescriptors()` projection; + explain capability/size changes without claiming a latency improvement. + +Hosted clients see this replacement only after `@githits/mcp` publication, +`remote-mcp` dependency adoption/provider migration, and deployment. Those +steps require separate authorization and belong to that repository's owner; +this plan adds no hosted server implementation or third phase. Rolling back +the client/package increment restores the prior catalog; callers cannot carry +unified cursors into the legacy root. + +## Phase boundary and completion + +After Phase 1 merges, reorient against current `origin/main`: record actual +validation, confirm backend schema/deployment, reconcile read/list changes, +reassess Phase 2 public compatibility and instruction dependencies, and adjust +detail before implementation. Use the next-step readiness workflow if available; +do not proceed from stale assumptions or treat package release as deployment. + +After Phase 2 merges, move lasting decisions and migration/operational facts to +`docs/implementation/unified-grep.md`, transfer any actual major deferred work +to the repository backlog, then delete this plan. No deferred development or +new infrastructure is proposed. The maintenance opportunity is to retire the +MCP-only legacy adapter after migration while preserving shared CLI helpers; +do not absorb a general code-navigation refactor. + +## Design review + +Phase 1 execution sequence (one Luna worker, sequential dispatches): + +1. Coordinator: core service/types/query/validation and transport tests. +2. Luna: shared request normalization plus its isolated behavioral tests. +3. Coordinator: result projection, mapped errors and coverage-aware formatter. +4. Luna: container and central mock wiring against the settled service seam. +5. Coordinator: CLI adapter and flag/action tests; live conformance and smoke. +6. Luna: root command registration against the tested command factory. +7. Coordinator: docs, release fragment, verification, review, stable commits + and draft PR. Phase 2 remains pending. Worker returns verified uncommitted + slices; coordinator owns commits. The previously untracked plan is included + with Phase 1 delivery and remains through Phase 2 review. + +Internal technical pre-flight and one external Claude review are required by +the planning workflow. Internal review: clean after one minor correction; +`ContentSafety.modifications` is an enum list, so the query-selection wording +now explicitly forbids subfields. Sibling scan: the selection table, response +contract and service test criteria retain the actual schema shape; no other +modification sub-selection was proposed. Subsequent internal passes checked the +full revised defaults and closure records and were clean after simplifying one +leading-dash example. External review: **clean in round 3** after applying its +one minor acceptance-test clarification. Rejected +findings must remain rejected without new evidence. + +External round 1: four accepted plan findings, no code findings. Closure: + +- CLI flag inconsistency → unsupported vocabulary change → checked legacy + Commander definitions and live `rg --help` → reuse `-A/-B/-C` and `--limit`; + user subsequently confirmed regex default and normal grep/rg conventions, + so literal opt-in uses standard `-F/--fixed-strings` and case flags use + `-i/-s`. CLI-specific normalization does not enlarge the MCP schema. +- Default wait migration omitted → changed cold-target behavior undocumented + → checked legacy defaults and all compatibility/fragment/test sections → + record 30 seconds to zero and typed preparation failures; also state existing + page-size differences and the user-requested regex/case/context defaults. +- ASCII help sibling omitted → stale claim in a retained surface → checked + shared pattern note and CLI flag help → schedule both corrections together + in Phase 2, keeping Phase 1 MCP unchanged. +- File-issue safety selection incomplete → compact output could miss path + normalization warnings → checked backend file-issue type, complete selection + table and formatter criteria → compact includes issue `filtered`; detailed + includes `lineBytes`, match coordinates and enum modifications. Coordinator + also closed the related zero-hit issue case: issue-bearing/skipped content + cannot be called an exhaustive no-match result. + +User steering resolved the matching-default preference: regex, case sensitive, +zero context; default repository corpus also uses `ALL` to honor the subsequent +directive to mirror grep/rg unless a concrete reason prevents it. The table +above records supported conventions and unavoidable departures. These and the +CLI contract revisions were material plan changes and were reviewed in round 2 +on the complete revised plan. + +External round 2: four accepted plan findings; no code findings or infrastructure: + +- Default-true MCP case boolean → violated agent-schema convention → checked + AGENTS.md and architectural guidelines, then all case/default/example/test + references → rename the MCP knob to `ignore_case` with default false and + invert once into backend `caseSensitive`. Matching remains case-sensitive + by default as the user requested; no alias or second case knob is added. +- Leading-dash patterns omitted → Commander may parse a pattern as an option + → checked positional syntax and CLI acceptance → document `--` and add one + unchanged-pattern regression case, avoiding a redundant `-e` syntax now. +- Case-sensitive `-s` overlaps existing command-local meanings → checked + list/vulnerability option definitions and grep/rg meanings → explicitly + accept rg's grep-command meaning and document command-scoped parsing. This + adds no cross-command parser ambiguity or shared behavior. +- Stale review status and punctuation → checked review section and complete + prose → correct both. No contract or implementation change for this note. + +External round 3: direct review found no issues. Its one permitted fresh-context +final check confirmed the backend contract and raised one accepted minor +test-criterion clarification: `CURSOR_EXPIRED` is a successful result traversal, +so the projector/formatter criteria now explicitly require preserving returned +hits/omissions and showing restart guidance without auto-retry. Verified against +`grep.ex`'s traversal mapping; sibling scan covered the response, error, output +and test contracts and found the behavior already specified, with only the +explicit acceptance example missing. The round is clean once this wording is +applied, under the repository's minor-doc finding rule. + +All product steering is resolved. No rejected findings, unresolved review +items, deferred development, or new infrastructure. Phase 1 can begin from +this design; fresh authenticated conformance remains implementation acceptance. + + +Implementation preflight closure (2026-09-28): + +- Config registration list omitted grep: accepted; added it to + `docs/implementation/config.md`, checked the CLI reference, README and root + registration for the same stale list. +- Package validation checkpoint was stale: accepted; recorded the completed + public-artifact validation. Fresh dev proof remains explicitly unproven. +- Remove inspected commit/schema identifiers: rejected. They identify the + verified evidence snapshot, not transient runtime IDs or current-HEAD claims; + preserving reproducibility in the working plan follows the verification rule. +- Narrow raw corpus input to an enum: rejected. The CLI passes unvalidated + option strings to the shared normalizer, which owns enum validation. An enum + here would require a cast or duplicate validation in the adapter. Clarified the + boundary in JSDoc; the unused input alias is already absent. + + +Internal code-review closure (2026-09-28): + +- Repeated context flags could hide an invalid earlier value: accepted. The + failure class was loss of raw option occurrences before validation. Scanned + all three context flags and symmetric/side precedence. Commander now collects + every context occurrence; CLI conversion validates each through the shared + numeric-context helper and chooses the final valid value. No new parser + metadata or duplicated numeric bounds. Focused tests cover invalid-first + repetitions for -A/-B/-C and valid repeated values with side precedence. + +The full revised internal code-review round is clean. The earlier context +finding is closed; no additional code findings were raised. Remaining live +small mixed-page acceptance is an external backend dependency, not a client +fallback or reduced scope. No refactor or new infrastructure was needed. diff --git a/packages/core-internal/src/index.ts b/packages/core-internal/src/index.ts index a84ec1fd..59a78ffe 100644 --- a/packages/core-internal/src/index.ts +++ b/packages/core-internal/src/index.ts @@ -5,6 +5,7 @@ export * from "./services/code-navigation-service.js"; export * from "./services/config.js"; export * from "./services/execute-with-token-refresh.js"; export * from "./services/githits-service.js"; +export * from "./services/grep-service.js"; export * from "./services/list-service.js"; export * from "./services/package-intelligence-service.js"; export * from "./services/promote-version-not-found.js"; diff --git a/packages/core-internal/src/services/grep-service.test.ts b/packages/core-internal/src/services/grep-service.test.ts new file mode 100644 index 00000000..91b23eed --- /dev/null +++ b/packages/core-internal/src/services/grep-service.test.ts @@ -0,0 +1,360 @@ +import { describe, expect, it, mock, spyOn } from "bun:test"; +import { FetchTimeoutError } from "../shared/fetch-timeout.js"; +import { TermsAcceptanceRequiredError } from "../shared/terms-acceptance.js"; +import { ClientUpdateRequiredError } from "./client-update-required-error.js"; +import { AuthenticationError } from "./githits-service.js"; +import { + GrepAccessError, + GrepBackendError, + GrepGraphQLError, + GrepNetworkError, + type GrepParams, + type GrepResult, + GrepServiceImpl, + MalformedGrepResponseError, +} from "./grep-service.js"; +import { createMockTokenProvider } from "./test-helpers.js"; + +const params: GrepParams = { + targets: [{ target: "npm:x", corpus: "ALL", allowUnscoped: true }], + pattern: "router", + patternType: "REGEX", + caseSensitive: true, + contextLinesBefore: 0, + contextLinesAfter: 0, + includeDetailedFields: false, +}; +function result(): GrepResult { + return { + hits: [], + targets: [], + unavailableTargets: [], + traversal: "COMPLETE", + nextCursor: null, + totalMatches: 0, + }; +} +function response(data: unknown, status = 200): Response { + return new Response(JSON.stringify(data), { + status, + headers: { "Content-Type": "application/json" }, + }); +} +function service( + fetcher: (...args: Parameters) => Promise, +): GrepServiceImpl { + return new GrepServiceImpl( + "https://example.test", + createMockTokenProvider(), + fetcher as typeof fetch, + ); +} +const safety = { filtered: false }; +const slice = { + content: "router", + startByte: 0, + endByte: 6, + originalLineBytes: 6, +}; +function mixed(): GrepResult { + return { + ...result(), + totalMatches: 2, + hits: [ + { + __typename: "GrepRepositoryHit", + targetIndex: 0, + filePath: "lib/a.ts", + line: 2, + lineSlice: slice, + contextBeforeSlices: [], + contextAfterSlices: [], + read: { + target: "github:o/r@sha", + path: "packages/x/lib/a.ts", + startLine: 2, + endLine: 2, + }, + contentSafety: safety, + }, + { + __typename: "GrepSiteHit", + targetIndex: 1, + pageUrl: "https://docs.test/page", + line: 3, + lineSlice: slice, + contextBeforeSlices: [], + contextAfterSlices: [], + read: { + target: "https://docs.test/page", + path: null, + startLine: 3, + endLine: 3, + }, + contentSafety: safety, + }, + ], + targets: [0, 1].map((targetIndex) => ({ + targetIndex, + requestedInputIndices: [0], + kind: targetIndex === 0 ? ("REPOSITORY" as const) : ("SITE" as const), + target: "scope", + traversal: "COMPLETE" as const, + readiness: "CURRENT" as const, + errorCode: null, + retryable: false, + publicMessage: null, + requestedRef: null, + commitSha: targetIndex === 0 ? "sha" : null, + corpus: targetIndex === 0 ? ("ALL" as const) : null, + filesScanned: null, + filesInScope: null, + binaryFilesSkipped: null, + filesTooLargeSkipped: null, + fileIssues: null, + fileIssuesOmitted: null, + })), + }; +} + +describe("unified grep service", () => { + it("allows the advertised preparation wait before the transport deadline", async () => { + const timeout = spyOn(AbortSignal, "timeout").mockImplementation( + (milliseconds: number) => { + expect(milliseconds).toBeGreaterThan(300_000); + return new AbortController().signal; + }, + ); + try { + await service(async () => response({ data: { grep: result() } })).grep({ + ...params, + waitTimeoutMs: 300_000, + }); + expect(timeout).toHaveBeenCalledTimes(1); + } finally { + timeout.mockRestore(); + } + }); + it("sends one grep page with exact controls and conditional detail selections", async () => { + const fetcher = mock( + async (_url: Parameters[0], init?: RequestInit) => { + const body = JSON.parse(String(init?.body)); + expect(body.variables).toEqual({ + ...params, + caseSensitive: false, + waitTimeoutMs: 0, + cursor: "opaque", + }); + expect(body.query).toContain("grep(targets: $targets"); + expect(body.query).not.toContain("grepRepo("); + for (const field of [ + "lineContent", + "sourceMatchStartByte", + "repoUrl", + "lineBytes", + "modifications", + "urlPrefixes", + ]) + expect(body.query).toContain( + `${field} @include(if: $includeDetailedFields)`, + ); + expect(body.query).toContain("read { target path startLine endLine }"); + expect(body.query).toContain("fragment LineSlice on GrepRepoLineSlice"); + return response({ data: { grep: mixed() } }); + }, + ); + const out = await service(fetcher).grep({ + ...params, + caseSensitive: false, + waitTimeoutMs: 0, + cursor: "opaque", + }); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(out).toEqual(mixed()); + expect(out.hits[0]?.lineContent).toBeUndefined(); + expect(out.hits[1]?.read.path).toBeNull(); + }); + it("requires all selected detail fields and preserves nulls and both coordinate systems", async () => { + const data = mixed(); + for (const hit of data.hits) { + Object.assign(hit, { + lineContent: "router", + matchStartByte: 0, + matchEndByte: 6, + sourceMatchStartByte: 5, + sourceMatchEndByte: 11, + contentSafety: { filtered: false, modifications: [] }, + }); + if (hit.__typename === "GrepRepositoryHit") + Object.assign(hit, { + repoUrl: "https://github.com/o/r", + commitSha: "sha", + repositoryFilePath: "packages/x/lib/a.ts", + }); + } + for (const target of data.targets) + Object.assign(target, { + repoUrl: null, + canonicalSite: null, + urlPrefixes: [], + }); + const fetcher = mock( + async (_url: Parameters[0], init?: RequestInit) => { + expect( + JSON.parse(String(init?.body)).variables.includeDetailedFields, + ).toBe(true); + return response({ data: { grep: data } }); + }, + ); + expect( + await service(fetcher).grep({ ...params, includeDetailedFields: true }), + ).toEqual(data); + delete data.hits[0]?.lineContent; + await expect( + service(async () => response({ data: { grep: data } })).grep({ + ...params, + includeDetailedFields: true, + }), + ).rejects.toBeInstanceOf(MalformedGrepResponseError); + }); + it("rejects malformed unions, missing statuses, missing continuations and non-JSON responses", async () => { + for (const data of [ + { ...mixed(), hits: [{ ...mixed().hits[0], __typename: "UnknownHit" }] }, + { ...mixed(), targets: [] }, + { ...result(), traversal: "RESUMABLE_LIMIT", nextCursor: null }, + { ...result(), totalMatches: "0" }, + ]) + await expect( + service(async () => response({ data: { grep: data } })).grep(params), + ).rejects.toBeInstanceOf(MalformedGrepResponseError); + await expect( + service(async () => new Response("invalid json")).grep(params), + ).rejects.toBeInstanceOf(MalformedGrepResponseError); + }); + it("returns cursor-expired and partial pages with sibling hits and omissions unchanged", async () => { + const data = { + ...mixed(), + traversal: "CURSOR_EXPIRED" as const, + unavailableTargets: [ + { + inputIndex: 0, + target: "npm:x", + reason: "documentation_site_no_docs", + retryable: false, + progressRef: null, + suggestedSiteTargets: null, + }, + ], + }; + expect( + await service(async () => response({ data: { grep: data } })).grep( + params, + ), + ).toEqual(data); + }); + it("retains typed preparation extensions and performs no fallback or retry", async () => { + const extensions = { + code: "GREP_TARGET_PREPARATION_REQUIRED", + retryable: true, + target_issues: [ + { + input_index: 0, + reason: "repository_indexing", + progress_ref: "index:1", + retryable: true, + }, + ], + }; + const fetcher = mock(async () => + response({ errors: [{ message: "Targets not ready", extensions }] }), + ); + try { + await service(fetcher).grep(params); + throw new Error("Expected failure"); + } catch (error) { + expect(error).toBeInstanceOf(GrepGraphQLError); + expect((error as GrepGraphQLError).extensions).toEqual(extensions); + } + expect(fetcher).toHaveBeenCalledTimes(1); + }); + it("refreshes authentication once using the injected provider", async () => { + const forceRefresh = mock(async () => "mock-refreshed"); + const fetcher = mock(async () => + response( + fetcher.mock.calls.length === 1 + ? { + errors: [ + { + message: "unauthorized", + extensions: { code: "UNAUTHORIZED" }, + }, + ], + } + : { data: { grep: result() } }, + ), + ); + await new GrepServiceImpl( + "https://example.test", + createMockTokenProvider({ forceRefresh }), + fetcher as unknown as typeof fetch, + ).grep(params); + expect(forceRefresh).toHaveBeenCalledTimes(1); + expect(fetcher).toHaveBeenCalledTimes(2); + await expect( + service(async () => response({}, 401)).grep(params), + ).rejects.toBeInstanceOf(AuthenticationError); + }); + it("classifies HTTP, transport, timeout, terms and client-update failures", async () => { + await expect( + service(async () => response({}, 403)).grep(params), + ).rejects.toBeInstanceOf(GrepAccessError); + await expect( + service(async () => response({}, 502)).grep(params), + ).rejects.toBeInstanceOf(GrepBackendError); + await expect( + service(async () => { + throw new Error("socket down"); + }).grep(params), + ).rejects.toBeInstanceOf(GrepNetworkError); + await expect( + service(async () => { + throw new FetchTimeoutError(1000); + }).grep(params), + ).rejects.toMatchObject({ graphqlCode: "TIMEOUT", retryable: true }); + await expect( + service(async () => + response({ + errors: [ + { + message: "terms", + extensions: { code: "TERMS_ACCEPTANCE_REQUIRED" }, + }, + ], + }), + ).grep(params), + ).rejects.toBeInstanceOf(TermsAcceptanceRequiredError); + await expect( + service(async () => + response({ + errors: [ + { + message: "update", + extensions: { code: "CLIENT_UPDATE_REQUIRED" }, + }, + ], + }), + ).grep(params), + ).rejects.toBeInstanceOf(ClientUpdateRequiredError); + await expect( + service(async () => + response({ + errors: [{ message: 'Cannot query field "grep" on type "Query".' }], + }), + ).grep(params), + ).rejects.toMatchObject({ name: "GrepBackendError", retryable: false }); + }); + it("defers malformed endpoint validation until the network operation", async () => { + const s = new GrepServiceImpl("invalid", createMockTokenProvider()); + await expect(s.grep(params)).rejects.toThrow(); + }); +}); diff --git a/packages/core-internal/src/services/grep-service.ts b/packages/core-internal/src/services/grep-service.ts new file mode 100644 index 00000000..592abdc4 --- /dev/null +++ b/packages/core-internal/src/services/grep-service.ts @@ -0,0 +1,536 @@ +import { z } from "zod"; +import { + DEFAULT_FETCH_TIMEOUT_MS, + isFetchTimeoutError, +} from "../shared/fetch-timeout.js"; +import { parseHttpErrorDetail } from "../shared/http-error-detail.js"; +import { + type PkgseerGraphqlResponse, + PkgseerTransportError, + postPkgseerGraphql, +} from "../shared/pkgseer-graphql.js"; +import type { ClientHeaderBuilder } from "../shared/request-headers.js"; +import { + ClientUpdateRequiredError, + isClientUpdateRequiredGraphQLError, + isGraphQLSchemaMismatchError, +} from "./client-update-required-error.js"; +import type { ContentModification } from "./code-navigation-service.js"; +import { executeWithTokenRefresh } from "./execute-with-token-refresh.js"; +import { + AuthenticationError, + isTokenRefreshableError, + SERVER_AUTHENTICATION_REJECTED_MESSAGE, +} from "./githits-service.js"; +import { + type ServiceDiagnostics, + withServiceDiagnostics, +} from "./runtime-diagnostics.js"; +import type { TokenProvider } from "./token-provider.js"; + +export type GrepCorpus = "SOURCE" | "DOCUMENTATION" | "ALL"; +export type GrepTraversal = + | "COMPLETE" + | "RESUMABLE_LIMIT" + | "NON_RESUMABLE_PARTIAL" + | "FAILED" + | "CURSOR_EXPIRED"; +export type GrepReadiness = + | "CURRENT" + | "STALE" + | "NOT_AVAILABLE" + | "MISSING_REF" + | "READER_OPEN_FAILED" + | "INCOMPLETE" + | "RESOURCE_LIMIT" + | "VERSION_UNSUPPORTED" + | "READ_FAILED"; +export interface GrepPathSelector { + kind: "EXACT" | "PREFIX" | "GLOB"; + value: string; +} +export interface GrepTarget { + target: string; + corpus?: GrepCorpus; + pathSelectors?: GrepPathSelector[]; + allowUnscoped?: boolean; +} +export interface GrepParams { + targets: GrepTarget[]; + pattern: string; + patternType: "REGEX" | "LITERAL"; + caseSensitive: boolean; + contextLinesBefore: number; + contextLinesAfter: number; + maxMatches?: number; + cursor?: string; + waitTimeoutMs?: number; + includeDetailedFields: boolean; +} +export interface GrepLineSlice { + content: string; + startByte: number; + endByte: number; + originalLineBytes: number; +} +export interface GrepContentSafety { + filtered: boolean; + modifications?: ContentModification[]; +} +export interface GrepReadAction { + target: string; + path: string | null; + startLine: number; + endLine: number; +} +interface GrepHitBase { + targetIndex: number; + line: number; + lineSlice: GrepLineSlice; + contextBeforeSlices: GrepLineSlice[]; + contextAfterSlices: GrepLineSlice[]; + read: GrepReadAction; + contentSafety: GrepContentSafety; + lineContent?: string; + matchStartByte?: number; + matchEndByte?: number; + sourceMatchStartByte?: number; + sourceMatchEndByte?: number; +} +export interface GrepRepositoryHit extends GrepHitBase { + __typename: "GrepRepositoryHit"; + filePath: string; + repoUrl?: string; + commitSha?: string; + repositoryFilePath?: string; +} +export interface GrepSiteHit extends GrepHitBase { + __typename: "GrepSiteHit"; + pageUrl: string; +} +export type GrepHit = GrepRepositoryHit | GrepSiteHit; +export interface GrepFileIssue { + filePath: string; + code: string; + line: number; + contentSafety: GrepContentSafety; + lineBytes?: number; + matchStartByte?: number | null; + matchEndByte?: number | null; +} +export interface GrepTargetStatus { + targetIndex: number; + requestedInputIndices: number[]; + kind: "REPOSITORY" | "SITE"; + target: string; + traversal: GrepTraversal; + readiness: GrepReadiness; + errorCode: string | null; + retryable: boolean; + publicMessage: string | null; + requestedRef: string | null; + commitSha: string | null; + corpus: GrepCorpus | null; + filesScanned: number | null; + filesInScope: number | null; + binaryFilesSkipped: number | null; + filesTooLargeSkipped: number | null; + fileIssues: GrepFileIssue[] | null; + fileIssuesOmitted: number | null; + repoUrl?: string | null; + canonicalSite?: string | null; + urlPrefixes?: string[]; +} +export interface GrepUnavailableTarget { + inputIndex: number; + target: string; + reason: string; + retryable: boolean; + progressRef: string | null; + suggestedSiteTargets: string[] | null; +} +export interface GrepResult { + hits: GrepHit[]; + targets: GrepTargetStatus[]; + unavailableTargets: GrepUnavailableTarget[]; + traversal: GrepTraversal; + nextCursor: string | null; + totalMatches: number; +} +export interface GrepService { + grep(params: GrepParams): Promise; +} +interface GrepServiceRuntime { + clientHeaders?: ClientHeaderBuilder; + userAgent?: string; + clientVersion?: string; + diagnostics?: ServiceDiagnostics; +} + +/** Public backend error details, retained without interpreting message text. */ +export class GrepGraphQLError extends Error { + constructor( + message: string, + public readonly extensions: Record = {}, + ) { + super(message); + this.name = "GrepGraphQLError"; + } +} +export class GrepBackendError extends Error { + constructor( + message: string, + public readonly status?: number, + public readonly graphqlCode?: string, + public readonly retryable?: boolean, + ) { + super(message); + this.name = "GrepBackendError"; + } +} +export class GrepAccessError extends Error { + constructor(message: string) { + super(message); + this.name = "GrepAccessError"; + } +} +export class GrepNetworkError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = "GrepNetworkError"; + } +} +export class MalformedGrepResponseError extends Error { + constructor() { + super("Malformed response from the grep service."); + this.name = "MalformedGrepResponseError"; + } +} + +const traversal = z.enum([ + "COMPLETE", + "RESUMABLE_LIMIT", + "NON_RESUMABLE_PARTIAL", + "FAILED", + "CURSOR_EXPIRED", +]); +const corpus = z.enum(["SOURCE", "DOCUMENTATION", "ALL"]); +const nonnegativeInt = z.number().int().nonnegative(); +const nullableString = z.string().nullable(); +const lineSlice = z.object({ + content: z.string(), + startByte: nonnegativeInt, + endByte: nonnegativeInt, + originalLineBytes: nonnegativeInt, +}); +const readAction = z.object({ + target: z.string(), + path: nullableString, + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), +}); +const modifications = z.array( + z.enum([ + "INVISIBLE_CONTROLS_STRIPPED", + "HTML_COMMENTS_STRIPPED", + "IMAGES_REPLACED", + "UNSAFE_LINKS_NEUTRALIZED", + ]), +); + +/** Conditional fields must exist when selected, and stay absent in compact results. */ +function resultSchema(detailed: boolean): z.ZodType { + const selected = (schema: T): T | z.ZodOptional => + detailed ? schema : schema.optional(); + const safety = z.object({ + filtered: z.boolean(), + modifications: selected(modifications), + }); + const common = { + targetIndex: nonnegativeInt, + line: z.number().int().positive(), + lineSlice, + contextBeforeSlices: z.array(lineSlice), + contextAfterSlices: z.array(lineSlice), + read: readAction, + contentSafety: safety, + lineContent: selected(z.string()), + matchStartByte: selected(nonnegativeInt), + matchEndByte: selected(nonnegativeInt), + sourceMatchStartByte: selected(nonnegativeInt), + sourceMatchEndByte: selected(nonnegativeInt), + }; + return z.object({ + hits: z.array( + z.discriminatedUnion("__typename", [ + z.object({ + ...common, + __typename: z.literal("GrepRepositoryHit"), + filePath: z.string(), + repoUrl: selected(z.string()), + commitSha: selected(z.string()), + repositoryFilePath: selected(z.string()), + }), + z.object({ + ...common, + __typename: z.literal("GrepSiteHit"), + pageUrl: z.string(), + }), + ]), + ), + targets: z.array( + z.object({ + targetIndex: nonnegativeInt, + requestedInputIndices: z.array(nonnegativeInt), + kind: z.enum(["REPOSITORY", "SITE"]), + target: z.string(), + traversal, + readiness: z.enum([ + "CURRENT", + "STALE", + "NOT_AVAILABLE", + "MISSING_REF", + "READER_OPEN_FAILED", + "INCOMPLETE", + "RESOURCE_LIMIT", + "VERSION_UNSUPPORTED", + "READ_FAILED", + ]), + errorCode: nullableString, + retryable: z.boolean(), + publicMessage: nullableString, + requestedRef: nullableString, + commitSha: nullableString, + corpus: corpus.nullable(), + filesScanned: nonnegativeInt.nullable(), + filesInScope: nonnegativeInt.nullable(), + binaryFilesSkipped: nonnegativeInt.nullable(), + filesTooLargeSkipped: nonnegativeInt.nullable(), + fileIssuesOmitted: nonnegativeInt.nullable(), + fileIssues: z + .array( + z.object({ + filePath: z.string(), + contentSafety: safety, + code: z.string(), + line: nonnegativeInt, + lineBytes: selected(nonnegativeInt), + matchStartByte: selected(nonnegativeInt.nullable()), + matchEndByte: selected(nonnegativeInt.nullable()), + }), + ) + .nullable(), + repoUrl: selected(nullableString), + canonicalSite: selected(nullableString), + urlPrefixes: selected(z.array(z.string())), + }), + ), + unavailableTargets: z.array( + z.object({ + inputIndex: nonnegativeInt, + target: z.string(), + reason: z.string(), + retryable: z.boolean(), + progressRef: nullableString, + suggestedSiteTargets: z.array(z.string()).nullable(), + }), + ), + traversal, + nextCursor: nullableString, + totalMatches: nonnegativeInt, + }); +} + +/** Validate and allowlist selected fields; detailed mode requires its selections. */ +export function parseGrepResult(value: unknown, detailed = false): GrepResult { + const parsed = resultSchema(detailed).safeParse(value); + if (!parsed.success) throw new MalformedGrepResponseError(); + const result = parsed.data; + const indices = new Set(result.targets.map((target) => target.targetIndex)); + if ( + indices.size !== result.targets.length || + result.hits.some((hit) => !indices.has(hit.targetIndex)) || + (result.traversal === "RESUMABLE_LIMIT" && !result.nextCursor) + ) + throw new MalformedGrepResponseError(); + return result; +} + +const GRAPHQL_QUERY = `query Grep( + $targets: [GrepTargetInput!]!, $pattern: String!, $patternType: GrepPatternType, + $caseSensitive: Boolean, $contextLinesBefore: Int, $contextLinesAfter: Int, + $maxMatches: Int, $cursor: String, $waitTimeoutMs: Int, $includeDetailedFields: Boolean! +) { + grep(targets: $targets, pattern: $pattern, patternType: $patternType, + caseSensitive: $caseSensitive, contextLinesBefore: $contextLinesBefore, + contextLinesAfter: $contextLinesAfter, maxMatches: $maxMatches, + cursor: $cursor, waitTimeoutMs: $waitTimeoutMs) { + traversal nextCursor totalMatches + hits { + __typename + ... on GrepRepositoryHit { + filePath + repoUrl @include(if: $includeDetailedFields) + commitSha @include(if: $includeDetailedFields) + repositoryFilePath @include(if: $includeDetailedFields) + ...RepositoryMatch + } + ... on GrepSiteHit { pageUrl ...SiteMatch } + } + targets { + targetIndex requestedInputIndices kind target traversal readiness errorCode retryable publicMessage + requestedRef commitSha corpus filesScanned filesInScope binaryFilesSkipped filesTooLargeSkipped fileIssuesOmitted + repoUrl @include(if: $includeDetailedFields) + canonicalSite @include(if: $includeDetailedFields) + urlPrefixes @include(if: $includeDetailedFields) + fileIssues { + filePath code line contentSafety { filtered modifications @include(if: $includeDetailedFields) } + lineBytes @include(if: $includeDetailedFields) + matchStartByte @include(if: $includeDetailedFields) + matchEndByte @include(if: $includeDetailedFields) + } + } + unavailableTargets { inputIndex target reason retryable progressRef suggestedSiteTargets } + } +} +fragment RepositoryMatch on GrepRepositoryHit { + targetIndex line lineSlice { ...LineSlice } contextBeforeSlices { ...LineSlice } contextAfterSlices { ...LineSlice } + read { target path startLine endLine } contentSafety { filtered modifications @include(if: $includeDetailedFields) } + lineContent @include(if: $includeDetailedFields) + matchStartByte @include(if: $includeDetailedFields) matchEndByte @include(if: $includeDetailedFields) + sourceMatchStartByte @include(if: $includeDetailedFields) sourceMatchEndByte @include(if: $includeDetailedFields) +} +fragment SiteMatch on GrepSiteHit { + targetIndex line lineSlice { ...LineSlice } contextBeforeSlices { ...LineSlice } contextAfterSlices { ...LineSlice } + read { target path startLine endLine } contentSafety { filtered modifications @include(if: $includeDetailedFields) } + lineContent @include(if: $includeDetailedFields) + matchStartByte @include(if: $includeDetailedFields) matchEndByte @include(if: $includeDetailedFields) + sourceMatchStartByte @include(if: $includeDetailedFields) sourceMatchEndByte @include(if: $includeDetailedFields) +} +fragment LineSlice on GrepRepoLineSlice { content startByte endByte originalLineBytes }`; + +const graphQLResponse = z.object({ + data: z.object({ grep: z.unknown().optional() }).nullable().optional(), + errors: z + .array( + z.object({ + message: z.string(), + extensions: z.record(z.string(), z.unknown()).optional(), + }), + ) + .optional(), +}); + +/** One transport-neutral request for a mixed-source grep page. */ +export class GrepServiceImpl implements GrepService { + constructor( + private readonly endpointUrl: string, + private readonly tokenProvider: TokenProvider, + private readonly fetchFn: typeof fetch = globalThis.fetch, + private readonly runtime: GrepServiceRuntime = {}, + ) {} + + async grep(params: GrepParams): Promise { + return withServiceDiagnostics( + this.runtime.diagnostics, + "grep.request", + () => + executeWithTokenRefresh({ + getToken: () => this.tokenProvider.getToken(), + forceRefresh: () => this.tokenProvider.forceRefresh(), + shouldRefresh: isTokenRefreshableError, + executeWithToken: (token) => this.executeGrep(token, params), + }), + ); + } + + private async executeGrep( + token: string, + params: GrepParams, + ): Promise { + let response: PkgseerGraphqlResponse; + const { includeDetailedFields, ...controls } = params; + try { + response = await postPkgseerGraphql({ + endpointUrl: this.endpointUrl, + token, + query: GRAPHQL_QUERY, + variables: { ...controls, includeDetailedFields }, + timeoutMs: Math.max( + DEFAULT_FETCH_TIMEOUT_MS, + (params.waitTimeoutMs ?? 0) + 30_000, + ), + fetchFn: this.fetchFn, + clientHeaders: this.runtime.clientHeaders, + userAgent: this.runtime.userAgent, + diagnostics: this.runtime.diagnostics, + }); + } catch (cause) { + if (!(cause instanceof PkgseerTransportError)) throw cause; + if (isFetchTimeoutError(cause.cause)) + throw new GrepBackendError( + "Grep request timed out.", + undefined, + "TIMEOUT", + true, + ); + throw new GrepNetworkError( + "Could not reach the package and documentation service. Check your connection.", + { cause }, + ); + } + if (response.status < 200 || response.status >= 300) + throw httpError(response); + const envelope = graphQLResponse.safeParse(response.parsedBody); + if (!envelope.success) throw new MalformedGrepResponseError(); + const errors = envelope.data.errors; + if (errors?.length) { + const message = errors.map((error) => error.message).join(", "); + const extensions = + errors.find((error) => error.extensions?.code)?.extensions ?? {}; + const code = + typeof extensions.code === "string" ? extensions.code : undefined; + if (isClientUpdateRequiredGraphQLError({ message, code })) + throw new ClientUpdateRequiredError( + undefined, + undefined, + this.runtime.clientVersion, + ); + if (isGraphQLSchemaMismatchError({ message, code })) + throw new GrepBackendError( + "Backend protocol mismatch: the endpoint must support unified grep. Run `githits update-check` to check your client version.", + undefined, + code, + false, + ); + if (code === "AUTHENTICATION_REQUIRED" || code === "UNAUTHORIZED") + throw new AuthenticationError( + SERVER_AUTHENTICATION_REJECTED_MESSAGE, + "server", + ); + if (code === "FORBIDDEN" || code === "ACCESS_DENIED") + throw new GrepAccessError(message); + throw new GrepGraphQLError(message, extensions); + } + return parseGrepResult(envelope.data.data?.grep, includeDetailedFields); + } +} + +function httpError(response: PkgseerGraphqlResponse): Error { + const detail = parseHttpErrorDetail(response.responseBody, [ + "message", + "error", + "detail", + ]); + if (response.status === 401) + return new AuthenticationError( + SERVER_AUTHENTICATION_REJECTED_MESSAGE, + "server", + ); + if (response.status === 403) + return new GrepAccessError(detail ?? "Grep access denied."); + return new GrepBackendError( + detail ?? `Request failed with status ${response.status}`, + response.status, + ); +} diff --git a/packages/mcp/src/internal.ts b/packages/mcp/src/internal.ts index a114d87a..ab62f493 100644 --- a/packages/mcp/src/internal.ts +++ b/packages/mcp/src/internal.ts @@ -24,9 +24,13 @@ export * from "./shared/follow-up-command-text.js"; export * from "./shared/format-date.js"; export * from "./shared/format-number.js"; export * from "./shared/githits-service-error-map.js"; +export * from "./shared/grep-error-map.js"; export * from "./shared/grep-repo-request.js"; export * from "./shared/grep-repo-response.js"; export * from "./shared/grep-repo-text.js"; +export * from "./shared/grep-request.js"; +export * from "./shared/grep-response.js"; +export * from "./shared/grep-text.js"; export * from "./shared/list-error-map.js"; export * from "./shared/list-files-request.js"; export * from "./shared/list-files-response.js"; diff --git a/packages/mcp/src/shared/grep-error-map.test.ts b/packages/mcp/src/shared/grep-error-map.test.ts new file mode 100644 index 00000000..d72ee2cf --- /dev/null +++ b/packages/mcp/src/shared/grep-error-map.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "bun:test"; +import { + GrepBackendError, + GrepGraphQLError, + GrepNetworkError, + MalformedGrepResponseError, +} from "@githits/core-internal"; +import { mapGrepError } from "./grep-error-map.js"; + +describe("unified grep errors", () => { + it("retains bounded public per-input preparation details and explicit retryability", () => { + const issue = { + input_index: 1, + target: "npm:x", + reason: "repository_indexing", + retryable: true, + progress_ref: "index:1", + suggested_refs: ["v1"], + private_debug: "drop", + }; + const error = mapGrepError( + new GrepGraphQLError("not ready", { + code: "GREP_TARGET_PREPARATION_REQUIRED", + retryable: false, + target_issues: Array(25).fill(issue), + }), + ); + expect(error.code).toBe("INDEXING"); + expect(error.retryable).toBe(false); + expect(error.details?.targetIssues).toHaveLength(20); + expect(error.details?.targetIssues?.[0]).not.toHaveProperty( + "private_debug", + ); + expect(error.details?.hint).toContain( + "Input 1: repository_indexing; progress index:1", + ); + }); + it("keeps cursor invalid, protocol, transport, deadline and HTTP failures distinct", () => { + expect( + mapGrepError( + new GrepGraphQLError("invalid", { + code: "GREP_CURSOR_INVALID", + retryable: false, + }), + ), + ).toMatchObject({ + code: "INVALID_ARGUMENT", + retryable: false, + details: { graphqlCode: "GREP_CURSOR_INVALID" }, + }); + expect( + mapGrepError( + new GrepGraphQLError("bad response", { + code: "GREP_BACKEND_PROTOCOL_ERROR", + retryable: true, + }), + ), + ).toMatchObject({ code: "PROTOCOL_ERROR", retryable: true }); + expect(mapGrepError(new MalformedGrepResponseError()).code).toBe( + "PROTOCOL_ERROR", + ); + expect(mapGrepError(new GrepNetworkError("socket")).code).toBe("NETWORK"); + expect( + mapGrepError(new GrepBackendError("deadline", undefined, "TIMEOUT", true)) + .code, + ).toBe("TIMEOUT"); + expect(mapGrepError(new GrepBackendError("rate", 429))).toMatchObject({ + code: "RATE_LIMITED", + retryable: true, + details: { status: 429 }, + }); + expect(mapGrepError(new GrepBackendError("server", 503)).retryable).toBe( + true, + ); + }); +}); diff --git a/packages/mcp/src/shared/grep-error-map.ts b/packages/mcp/src/shared/grep-error-map.ts new file mode 100644 index 00000000..d5d3fbe4 --- /dev/null +++ b/packages/mcp/src/shared/grep-error-map.ts @@ -0,0 +1,150 @@ +import { + AuthenticationError, + ClientUpdateRequiredError, + GrepAccessError, + GrepBackendError, + GrepGraphQLError, + GrepNetworkError, + MalformedGrepResponseError, +} from "@githits/core-internal"; +import { buildUpdateRequiredError } from "./code-navigation-error-map.js"; +import { InvalidGrepRequestError } from "./grep-request.js"; +import type { + MappedError, + MappedErrorCode, + MappedErrorDetails, +} from "./mapped-error.js"; +import { AuthRequiredError } from "./require-auth.js"; +import { mapTermsAcceptanceError } from "./terms-acceptance-error-map.js"; + +/** Classify failures without interpreting backend prose or retrying requests. */ +export function mapGrepError(error: unknown): MappedError { + const terms = mapTermsAcceptanceError(error); + if (terms) return terms; + if (error instanceof ClientUpdateRequiredError) + return buildUpdateRequiredError(error.reason, error.currentVersion); + if ( + error instanceof AuthenticationError || + error instanceof AuthRequiredError + ) + return { + code: "AUTH_REQUIRED", + message: error.message, + retryable: false, + details: { + authSource: + error instanceof AuthenticationError ? error.source : "local", + }, + }; + if (error instanceof InvalidGrepRequestError) + return { + code: "INVALID_ARGUMENT", + message: error.message, + retryable: false, + }; + if (error instanceof GrepAccessError) + return { code: "ACCESS_DENIED", message: error.message, retryable: false }; + if (error instanceof GrepNetworkError) + return { code: "NETWORK", message: error.message, retryable: true }; + if (error instanceof MalformedGrepResponseError) + return { code: "PROTOCOL_ERROR", message: error.message, retryable: false }; + if (error instanceof GrepBackendError) { + const code: MappedErrorCode = + error.graphqlCode === "TIMEOUT" + ? "TIMEOUT" + : error.status === 429 + ? "RATE_LIMITED" + : "BACKEND_ERROR"; + return { + code, + message: error.message, + retryable: + error.retryable ?? + (code === "TIMEOUT" || + code === "RATE_LIMITED" || + (error.status !== undefined && error.status >= 500)), + details: { + ...(error.status !== undefined ? { status: error.status } : {}), + ...(error.graphqlCode ? { graphqlCode: error.graphqlCode } : {}), + }, + }; + } + if (error instanceof GrepGraphQLError) { + const ext = error.extensions; + const graphqlCode = typeof ext.code === "string" ? ext.code : undefined; + const code: MappedErrorCode = + graphqlCode === "GREP_TARGET_PREPARATION_REQUIRED" + ? "INDEXING" + : graphqlCode === "GREP_CURSOR_INVALID" || + graphqlCode === "VALIDATION_ERROR" || + graphqlCode === "INVALID_ARGUMENT" + ? "INVALID_ARGUMENT" + : graphqlCode === "GREP_BACKEND_PROTOCOL_ERROR" + ? "PROTOCOL_ERROR" + : graphqlCode === "TIMEOUT" + ? "TIMEOUT" + : graphqlCode === "RATE_LIMITED" + ? "RATE_LIMITED" + : "BACKEND_ERROR"; + const details: MappedErrorDetails = { graphqlCode }; + if (Array.isArray(ext.target_issues)) { + details.targetIssues = ext.target_issues + .slice(0, 20) + .filter(isRecord) + .map((issue) => + Object.fromEntries( + Object.entries(issue).filter(([key]) => PUBLIC_ISSUE_KEYS.has(key)), + ), + ); + details.hint = details.targetIssues + .map( + (issue) => + `Input ${issue.input_index}: ${issue.reason}${typeof issue.progress_ref === "string" ? `; progress ${issue.progress_ref}` : ""}${typeof issue.file_path === "string" ? `; path ${issue.file_path}` : ""}`, + ) + .join("\n"); + } + if (graphqlCode === "GREP_CURSOR_INVALID") + details.hint = + "Restart explicitly without --cursor, using the same targets and controls."; + return { + code, + message: error.message, + retryable: + typeof ext.retryable === "boolean" + ? ext.retryable + : code === "INDEXING" || + code === "TIMEOUT" || + code === "RATE_LIMITED", + details, + }; + } + return { + code: "UNKNOWN", + message: error instanceof Error ? error.message : "Unknown error", + retryable: false, + }; +} + +const PUBLIC_ISSUE_KEYS = new Set([ + "input_index", + "target", + "reason", + "retryable", + "progress_ref", + "file_path", + "repo_url", + "git_ref", + "requested_ref", + "available_refs", + "available_versions", + "message", + "suggested_refs", + "published_versions", + "published_versions_truncated", + "suggested_site_targets", + "registry", + "package", +]); +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/packages/mcp/src/shared/grep-request.test.ts b/packages/mcp/src/shared/grep-request.test.ts new file mode 100644 index 00000000..9a19d986 --- /dev/null +++ b/packages/mcp/src/shared/grep-request.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "bun:test"; +import { buildGrepParams, InvalidGrepRequestError } from "./grep-request.js"; + +const input = { + targets: [{ target: "npm:express" }], + pattern: "router", + includeDetailedFields: false, +}; + +describe("unified grep request normalization", () => { + it("enforces selector bounds per target rather than across targets", () => { + const pathSelectors = Array.from({ length: 1000 }, () => ({ + kind: "exact" as const, + value: "a", + })); + expect( + buildGrepParams({ + ...input, + targets: [ + { target: "npm:x", pathSelectors }, + { target: "npm:y", pathSelectors }, + ], + }).targets, + ).toHaveLength(2); + expect(() => + buildGrepParams({ + ...input, + targets: [ + { + target: "npm:x", + pathSelectors: [...pathSelectors, { kind: "exact", value: "b" }], + }, + ], + }), + ).toThrow("1000"); + }); + it("sends explicit grep defaults without inventing page or wait controls", () => { + expect(buildGrepParams(input)).toEqual({ + targets: [{ target: "npm:express", corpus: "ALL", allowUnscoped: true }], + pattern: "router", + patternType: "REGEX", + caseSensitive: true, + contextLinesBefore: 0, + contextLinesAfter: 0, + includeDetailedFields: false, + }); + }); + it("preserves ordered scopes, raw locators and selector bytes without expansion", () => { + const targets = [ + { + target: "npm:example@1", + corpus: "source", + pathSelectors: [ + { kind: "glob" as const, value: "src/**/*.ts" }, + { kind: "exact" as const, value: "file with spaces.ts" }, + ], + }, + { target: "site:example.test/api?x=%2F", pathSelectors: [] }, + { target: "github:owner/repo@refs/heads/main" }, + { target: "npm:example@1" }, + ]; + const params = buildGrepParams({ + ...input, + targets, + cursor: " opaque ", + pattern: " ", + }); + expect(params.targets).toEqual([ + { + target: targets[0]!.target, + corpus: "SOURCE", + allowUnscoped: true, + pathSelectors: [ + { kind: "GLOB", value: "src/**/*.ts" }, + { kind: "EXACT", value: "file with spaces.ts" }, + ], + }, + { target: targets[1]!.target }, + { target: targets[2]!.target, corpus: "ALL", allowUnscoped: true }, + { target: targets[3]!.target, corpus: "ALL", allowUnscoped: true }, + ]); + expect(params.cursor).toBe(" opaque "); + expect(params.pattern).toBe(" "); + }); + it("inverts ignoreCase exactly once and preserves explicit literal and zero controls", () => { + const p = buildGrepParams({ + ...input, + pattern: "a(b)", + patternType: "literal", + ignoreCase: true, + contextLinesBefore: 0, + contextLinesAfter: 3, + maxMatches: 1, + waitTimeoutMs: 0, + includeDetailedFields: true, + }); + expect(p).toMatchObject({ + pattern: "a(b)", + patternType: "LITERAL", + caseSensitive: false, + contextLinesBefore: 0, + contextLinesAfter: 3, + maxMatches: 1, + waitTimeoutMs: 0, + includeDetailedFields: true, + }); + expect(buildGrepParams({ ...input, ignoreCase: false }).caseSensitive).toBe( + true, + ); + }); + it("omits empty optional arrays and cursors before rejecting site-only incompatibilities", () => { + expect( + buildGrepParams({ + ...input, + targets: [{ target: "site:example.test", pathSelectors: [] }], + cursor: " ", + }).targets, + ).toEqual([{ target: "site:example.test" }]); + expect(buildGrepParams({ ...input, cursor: "" }).cursor).toBeUndefined(); + for (const target of [ + { target: "site:example.test", corpus: "all" }, + { + target: "site:example.test", + pathSelectors: [{ kind: "prefix" as const, value: "api/" }], + }, + ]) { + expect(() => buildGrepParams({ ...input, targets: [target] })).toThrow( + InvalidGrepRequestError, + ); + } + }); + it("validates pattern bytes and Unicode without trimming valid patterns", () => { + expect( + buildGrepParams({ ...input, pattern: "é".repeat(100) }).pattern, + ).toBe("é".repeat(100)); + for (const pattern of ["", "é".repeat(101), "a\0b", "\ud800"]) + expect(() => buildGrepParams({ ...input, pattern })).toThrow( + InvalidGrepRequestError, + ); + }); + it("rejects invalid caller shapes and bounded controls instead of clamping", () => { + for (const patch of [ + { targets: [] }, + { targets: Array.from({ length: 21 }, () => ({ target: "npm:x" })) }, + { targets: [{ target: " " }] }, + { contextLinesBefore: 11 }, + { contextLinesAfter: -1 }, + { contextLinesBefore: 0.5 }, + { maxMatches: 0 }, + { maxMatches: 1001 }, + { waitTimeoutMs: 300001 }, + { waitTimeoutMs: NaN }, + { + targets: [ + { + target: "npm:x", + pathSelectors: [{ kind: "exact" as const, value: " " }], + }, + ], + }, + ]) + expect(() => buildGrepParams({ ...input, ...patch })).toThrow( + InvalidGrepRequestError, + ); + }); +}); diff --git a/packages/mcp/src/shared/grep-request.ts b/packages/mcp/src/shared/grep-request.ts new file mode 100644 index 00000000..cf088d00 --- /dev/null +++ b/packages/mcp/src/shared/grep-request.ts @@ -0,0 +1,336 @@ +import type { + GrepCorpus, + GrepParams, + GrepPathSelector, +} from "@githits/core-internal"; +import { InvalidArgumentError } from "./package-spec.js"; + +const MAX_TARGETS = 20; +const MAX_PATTERN_BYTES = 200; +const MAX_PATH_SELECTORS = 1000; + +type GrepPatternTypeInput = "literal" | "regex"; +type GrepPathSelectorKindInput = "exact" | "prefix" | "glob"; + +export interface GrepRequestTargetInput { + target: string; + /** + * Raw CLI or MCP corpus value; this builder validates and maps it to the + * backend enum. + */ + corpus?: string; + pathSelectors?: readonly { + kind: GrepPathSelectorKindInput; + value: string; + }[]; +} + +export interface GrepRequestInput { + targets: readonly GrepRequestTargetInput[]; + pattern: string; + includeDetailedFields: boolean; + patternType?: GrepPatternTypeInput; + ignoreCase?: boolean; + contextLinesBefore?: number; + contextLinesAfter?: number; + maxMatches?: number; + waitTimeoutMs?: number; + cursor?: string; +} + +/** A caller-input error raised while normalizing a unified grep request. */ +export class InvalidGrepRequestError extends InvalidArgumentError { + constructor( + public readonly field: string, + message: string, + ) { + super(message); + this.name = "InvalidGrepRequestError"; + } +} + +/** Normalize caller controls into the exact backend parameters for one grep page. */ +export function buildGrepParams(input: GrepRequestInput): GrepParams { + if (!isRecord(input)) { + throw invalid("request", "A grep request is required."); + } + if (!Array.isArray(input.targets) || input.targets.length === 0) { + throw invalid("targets", "`targets` must contain at least one target."); + } + if (input.targets.length > MAX_TARGETS) { + throw invalid( + "targets", + `Targets may contain at most ${MAX_TARGETS} entries.`, + ); + } + if (typeof input.pattern !== "string") { + throw invalid("pattern", "`pattern` must be a string."); + } + validateUnicode( + input.pattern, + "pattern", + "`pattern` must contain valid Unicode.", + ); + if (input.pattern.includes("\0")) { + throw invalid("pattern", "`pattern` cannot contain NUL."); + } + const patternBytes = Buffer.byteLength(input.pattern, "utf8"); + if (patternBytes < 1 || patternBytes > MAX_PATTERN_BYTES) { + throw invalid( + "pattern", + `Pattern must be 1 to ${MAX_PATTERN_BYTES} UTF-8 bytes.`, + ); + } + if (typeof input.includeDetailedFields !== "boolean") { + throw invalid( + "includeDetailedFields", + "`includeDetailedFields` must be a boolean.", + ); + } + + const patternType = normalizePatternType(input.patternType); + const ignoreCase = normalizeBoolean(input.ignoreCase, "ignoreCase"); + const contextLinesBefore = normalizeGrepContextLines( + input.contextLinesBefore, + "contextLinesBefore", + ); + const contextLinesAfter = normalizeGrepContextLines( + input.contextLinesAfter, + "contextLinesAfter", + ); + const maxMatches = normalizeInteger(input.maxMatches, "maxMatches", 1, 1000); + const waitTimeoutMs = normalizeInteger( + input.waitTimeoutMs, + "waitTimeoutMs", + 0, + 300_000, + ); + const cursor = normalizeCursor(input.cursor); + + const targets = input.targets.map((targetInput, index) => { + const field = `targets[${index}]`; + if (!isRecord(targetInput) || typeof targetInput.target !== "string") { + throw invalid( + `${field}.target`, + "Each target must include a string `target`.", + ); + } + const target = targetInput.target; + validateUnicode( + target, + `${field}.target`, + "Target must contain valid Unicode.", + ); + if (target.trim().length === 0) { + throw invalid(`${field}.target`, "Target cannot be blank."); + } + + const corpus = normalizeCorpus(targetInput.corpus, `${field}.corpus`); + const pathSelectors = normalizePathSelectors( + targetInput.pathSelectors, + `${field}.pathSelectors`, + ); + + if (isSiteTarget(target)) { + if (targetInput.corpus !== undefined) { + throw invalid( + `${field}.corpus`, + "Corpus cannot be used with a site target.", + ); + } + if (pathSelectors.length > 0) { + throw invalid( + `${field}.pathSelectors`, + "Path selectors cannot be used with a site target.", + ); + } + return { target }; + } + + return { + target, + corpus: corpus ?? "ALL", + allowUnscoped: true, + ...(pathSelectors.length > 0 ? { pathSelectors } : {}), + }; + }); + + return { + targets, + pattern: input.pattern, + patternType, + caseSensitive: !(ignoreCase ?? false), + contextLinesBefore: contextLinesBefore ?? 0, + contextLinesAfter: contextLinesAfter ?? 0, + ...(maxMatches !== undefined ? { maxMatches } : {}), + ...(cursor !== undefined ? { cursor } : {}), + ...(waitTimeoutMs !== undefined ? { waitTimeoutMs } : {}), + includeDetailedFields: input.includeDetailedFields, + }; +} + +function normalizePatternType( + value: GrepPatternTypeInput | undefined, +): GrepParams["patternType"] { + if (value === undefined || value === "regex") return "REGEX"; + if (value === "literal") return "LITERAL"; + throw invalid("patternType", "`patternType` must be `literal` or `regex`."); +} + +function normalizeCorpus( + value: unknown, + field: string, +): GrepCorpus | undefined { + if (value === undefined) return undefined; + if (typeof value !== "string") { + throw invalid(field, "Corpus must be `source`, `documentation`, or `all`."); + } + const normalized = value.toUpperCase(); + if ( + normalized === "SOURCE" || + normalized === "DOCUMENTATION" || + normalized === "ALL" + ) { + return normalized; + } + throw invalid(field, "Corpus must be `source`, `documentation`, or `all`."); +} + +function normalizePathSelectors( + value: unknown, + field: string, +): GrepPathSelector[] { + if (value === undefined) return []; + if (!Array.isArray(value)) { + throw invalid(field, "Path selectors must be an array."); + } + if (value.length === 0) return []; + if (value.length > MAX_PATH_SELECTORS) { + throw invalid( + field, + `A target may contain at most ${MAX_PATH_SELECTORS} path selectors.`, + ); + } + return value.map((selector, index) => { + const itemField = `${field}[${index}]`; + if (!isRecord(selector)) { + throw invalid(itemField, "A path selector must be an object."); + } + const kind = normalizeSelectorKind(selector.kind, `${itemField}.kind`); + if (typeof selector.value !== "string") { + throw invalid(`${itemField}.value`, "Selector value must be a string."); + } + const selectorValue = selector.value; + validateUnicode( + selectorValue, + `${itemField}.value`, + "Selector values must contain valid Unicode.", + ); + if (selectorValue.trim().length === 0) { + throw invalid(`${itemField}.value`, "Selector values cannot be blank."); + } + if (selectorValue.includes("\0")) { + throw invalid( + `${itemField}.value`, + "Selector values cannot contain NUL.", + ); + } + return { kind, value: selectorValue }; + }); +} + +function normalizeSelectorKind( + value: unknown, + field: string, +): GrepPathSelector["kind"] { + if (typeof value !== "string") { + throw invalid(field, "Selector kind must be `exact`, `prefix`, or `glob`."); + } + const normalized = value.toUpperCase(); + if ( + normalized === "EXACT" || + normalized === "PREFIX" || + normalized === "GLOB" + ) { + return normalized; + } + throw invalid(field, "Selector kind must be `exact`, `prefix`, or `glob`."); +} + +function normalizeBoolean( + value: boolean | undefined, + field: string, +): boolean | undefined { + if (value === undefined) return undefined; + if (typeof value !== "boolean") { + throw invalid(field, `${field} must be a boolean.`); + } + return value; +} + +function normalizeInteger( + value: number | undefined, + field: string, + minimum: number, + maximum: number, +): number | undefined { + if (value === undefined) return undefined; + if ( + typeof value !== "number" || + !Number.isSafeInteger(value) || + value < minimum || + value > maximum + ) { + throw invalid( + field, + `${field} must be a safe integer from ${minimum} to ${maximum}.`, + ); + } + return value; +} + +/** Validate one context side using the shared grep request bounds. */ +export function normalizeGrepContextLines( + value: number | undefined, + field: string, +): number | undefined { + return normalizeInteger(value, field, 0, 10); +} + +function normalizeCursor(value: string | undefined): string | undefined { + if (value === undefined) return undefined; + if (typeof value !== "string") { + throw invalid("cursor", "`cursor` must be a string."); + } + return value.trim().length === 0 ? undefined : value; +} + +function isSiteTarget(target: string): boolean { + return target.trim().toLowerCase().startsWith("site:"); +} + +function validateUnicode(value: string, field: string, message: string): void { + if (hasLoneSurrogate(value)) throw invalid(field, message); +} + +function hasLoneSurrogate(value: string): boolean { + for (let index = 0; index < value.length; index += 1) { + const codeUnit = value.charCodeAt(index); + if (codeUnit >= 0xd800 && codeUnit <= 0xdbff) { + const nextCodeUnit = value.charCodeAt(index + 1); + if (!(nextCodeUnit >= 0xdc00 && nextCodeUnit <= 0xdfff)) return true; + index += 1; + } else if (codeUnit >= 0xdc00 && codeUnit <= 0xdfff) { + return true; + } + } + return false; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function invalid(field: string, message: string): InvalidGrepRequestError { + return new InvalidGrepRequestError(field, message); +} diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts new file mode 100644 index 00000000..fdfd96c9 --- /dev/null +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -0,0 +1,301 @@ +import { describe, expect, it } from "bun:test"; +import type { + GrepHit, + GrepResult, + GrepTargetStatus, +} from "@githits/core-internal"; +import { projectGrepResult } from "./grep-response.js"; +import { formatGrepText, formatReadAction } from "./grep-text.js"; + +const slice = { + content: "router", + startByte: 0, + endByte: 6, + originalLineBytes: 6, +}; +const target: GrepTargetStatus = { + targetIndex: 7, + requestedInputIndices: [1, 0], + kind: "REPOSITORY", + target: "npm:x", + traversal: "COMPLETE", + readiness: "CURRENT", + errorCode: null, + retryable: false, + publicMessage: null, + requestedRef: "v1", + commitSha: "abc", + corpus: "ALL", + filesScanned: 1, + filesInScope: 1, + binaryFilesSkipped: 0, + filesTooLargeSkipped: 0, + fileIssues: [], + fileIssuesOmitted: 0, +}; +const hit: GrepHit = { + __typename: "GrepRepositoryHit", + targetIndex: 7, + filePath: "lib/a.ts", + line: 2, + lineSlice: slice, + contextBeforeSlices: [], + contextAfterSlices: [], + contentSafety: { filtered: false }, + read: { + target: "github:o/r@abc", + path: "packages/x/lib/a.ts", + startLine: 2, + endLine: 2, + }, +}; +function result(overrides: Partial = {}): GrepResult { + return { + hits: [hit], + targets: [target], + unavailableTargets: [], + traversal: "COMPLETE", + nextCursor: null, + totalMatches: 1, + ...overrides, + }; +} + +describe("unified grep result and text", () => { + it("preserves different match windows on the same long physical line", () => { + const first = { + ...hit, + lineSlice: { + content: "first router", + startByte: 0, + endByte: 12, + originalLineBytes: 1000, + }, + }; + const second = { + ...hit, + lineSlice: { + content: "second router", + startByte: 500, + endByte: 513, + originalLineBytes: 1000, + }, + }; + const output = formatGrepText( + result({ hits: [first, second], totalMatches: 2 }), + ); + expect(output).toContain("first router [...]"); + expect(output).toContain("[...] second router [...]"); + expect(output.match(/\[7\] lib\/a.ts/g)).toHaveLength(2); + }); + it("allowlists fields, preserves selected nulls, details and independent arrays", () => { + const data = result({ + hits: [ + { + ...hit, + lineContent: "router", + matchStartByte: 0, + matchEndByte: 6, + sourceMatchStartByte: 10, + sourceMatchEndByte: 16, + contentSafety: { + filtered: true, + modifications: ["INVISIBLE_CONTROLS_STRIPPED"], + }, + }, + ], + }); + const projected = projectGrepResult({ + ...data, + unknown: "drop", + } as GrepResult); + expect(projected).toEqual(data); + expect(projected.targets[0]?.publicMessage).toBeNull(); + expect(projected.hits[0]?.sourceMatchStartByte).toBe(10); + projected.hits.pop(); + expect(data.hits).toHaveLength(1); + const compact = projectGrepResult(result()); + expect(compact.hits[0]).not.toHaveProperty("lineContent"); + expect(compact).not.toHaveProperty("hasMore"); + }); + it("preserves producer ordering and uses server reads rather than display paths", () => { + const site: GrepHit = { + ...hit, + __typename: "GrepSiteHit", + targetIndex: 4, + pageUrl: "https://docs.test/p", + read: { + target: "https://docs.test/p", + path: null, + startLine: 2, + endLine: 2, + }, + }; + const output = formatGrepText( + result({ + hits: [hit, site, hit], + targets: [ + target, + { + ...target, + targetIndex: 4, + kind: "SITE", + commitSha: null, + corpus: null, + }, + ], + }), + ); + expect(output.match(/\[7\] lib\/a.ts/g)).toHaveLength(2); + expect(output.indexOf("[7] lib/a.ts")).toBeLessThan( + output.indexOf("[4] https://docs.test/p"), + ); + expect(output).toContain( + "githits read 'github:o/r@abc' 'packages/x/lib/a.ts' --lines 2-2", + ); + expect(output).toContain("githits read 'https://docs.test/p' --lines 2-2"); + expect(output).toContain("inputs 1, 0"); + expect(output).toContain("latest active content"); + }); + it("merges overlapping context while keeping match markers and slice omissions", () => { + const output = formatGrepText( + result({ + hits: [ + { + ...hit, + lineSlice: { + ...slice, + startByte: 10, + endByte: 16, + originalLineBytes: 30, + }, + contextBeforeSlices: [{ ...slice, content: "before" }], + contextAfterSlices: [{ ...slice, content: "after" }], + }, + { ...hit, line: 3, read: { ...hit.read, startLine: 3, endLine: 3 } }, + ], + }), + ); + expect(output).toContain("1- before"); + expect(output).toContain("2: [...] router [...]"); + expect(output).toContain("3: router"); + expect(output).not.toContain("3- after"); + }); + it("keeps all coverage warnings visible on zero-hit complete or partial pages", () => { + const scopes = [ + { ...target, readiness: "STALE" as const }, + { + ...target, + traversal: "FAILED" as const, + errorCode: "READ_FAILED", + publicMessage: "reader unavailable", + }, + { ...target, binaryFilesSkipped: 1 }, + { ...target, filesTooLargeSkipped: 1 }, + { + ...target, + fileIssues: [ + { + filePath: "a", + code: "LINE_TOO_LARGE", + line: 0, + contentSafety: { filtered: true }, + }, + ], + fileIssuesOmitted: 2, + }, + ]; + for (const scope of scopes) { + const output = formatGrepText( + result({ hits: [], totalMatches: 0, targets: [scope] }), + ); + expect(output).toContain("Zero returned matches"); + expect(output).not.toContain("No matches."); + } + const output = formatGrepText( + result({ hits: [], totalMatches: 0, targets: [scopes[4]!] }), + ); + expect(output).toContain("(aggregate)"); + expect(output).toContain("2 additional file issue"); + expect(output).toContain("safety normalization"); + expect(formatGrepText(result({ hits: [], totalMatches: 0 }))).toContain( + "No matches.", + ); + }); + it("shows cursor and terminal omissions together and requires explicit expiry restart", () => { + const omission = { + inputIndex: 2, + target: "npm:y", + reason: "docs_not_ready", + retryable: true, + progressRef: "crawl:1", + suggestedSiteTargets: ["site:docs.test"], + }; + const output = formatGrepText( + result({ + traversal: "NON_RESUMABLE_PARTIAL", + nextCursor: "opaque", + unavailableTargets: [omission], + }), + ); + expect(output).toContain("docs_not_ready"); + expect(output).toContain("crawl:1"); + expect(output).toContain("Suggested site"); + expect(output).toContain("--cursor 'opaque'"); + expect(output).toContain("same ordered targets and controls"); + expect( + formatGrepText( + result({ + targets: [{ ...target, traversal: "RESUMABLE_LIMIT" }], + traversal: "RESUMABLE_LIMIT", + nextCursor: "opaque", + }), + ), + ).toContain("Coverage: RESUMABLE_LIMIT"); + expect( + formatGrepText( + result({ traversal: "CURSOR_EXPIRED", unavailableTargets: [omission] }), + ), + ).toContain("Restart explicitly"); + }); + it("escapes terminal controls and locator backslashes, preserves Unicode and wraps prose only", () => { + const content = `${"界".repeat(100)}\x1b[31m`; + const output = formatGrepText( + result({ + hits: [ + { + ...hit, + filePath: "a\\b\x1b", + lineSlice: { + content, + startByte: 0, + endByte: 310, + originalLineBytes: 310, + }, + }, + ], + targets: [ + { + ...target, + publicMessage: "Long words should wrap into multiple prose lines", + readiness: "STALE", + }, + ], + }), + { width: 25 }, + ); + expect(output).toContain("a\\\\b\\u001b"); + expect(output).toContain(`${"界".repeat(100)}\\u001b[31m`); + expect(output).not.toContain("\x1b"); + expect(output.replace(/\s+/g, " ")).toContain("Long words should"); + expect( + formatReadAction({ ...hit.read, target: "github:o/r@abc\n" }, "cli"), + ).toContain("\\x0a"); + expect(formatReadAction(hit.read, "mcp")).toContain( + 'path="packages/x/lib/a.ts"', + ); + expect(formatReadAction({ ...hit.read, path: "-README.md" }, "cli")).toBe( + "githits read --lines 2-2 -- 'github:o/r@abc' '-README.md'", + ); + }); +}); diff --git a/packages/mcp/src/shared/grep-response.ts b/packages/mcp/src/shared/grep-response.ts new file mode 100644 index 00000000..79a86d85 --- /dev/null +++ b/packages/mcp/src/shared/grep-response.ts @@ -0,0 +1,6 @@ +import { type GrepResult, parseGrepResult } from "@githits/core-internal"; + +/** Preserve the selected wire payload without aliasing caller-owned arrays. */ +export function projectGrepResult(result: GrepResult): GrepResult { + return parseGrepResult(result); +} diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts new file mode 100644 index 00000000..3248b2b3 --- /dev/null +++ b/packages/mcp/src/shared/grep-text.ts @@ -0,0 +1,237 @@ +import type { + GrepHit, + GrepLineSlice, + GrepReadAction, + GrepResult, + GrepTargetStatus, +} from "@githits/core-internal"; +import { colors } from "./colors.js"; +import { shellQuoteExact } from "./shell-quote.js"; +import { terminalWidth } from "./terminal-width.js"; + +export interface GrepTextOptions { + useColors?: boolean; + width?: number; + syntax?: "cli" | "mcp"; +} +interface RenderLine { + number: number; + slice: GrepLineSlice; + match: boolean; +} + +/** Render one page, retaining source order, exact reads and coverage signals. */ +export function formatGrepText( + result: GrepResult, + options: GrepTextOptions = {}, +): string { + const lines: string[] = []; + const prose = (value: string): void => { + lines.push(...wrap(escapeText(value), options.width ?? 80)); + }; + const title = `${result.totalMatches} match${result.totalMatches === 1 ? "" : "es"} in this page | ${result.traversal}`; + prose(title); + if (options.useColors) lines[0] = `${colors.bold}${lines[0]}${colors.reset}`; + for (const scope of result.targets) { + prose( + `[${scope.targetIndex}] ${scope.target} | inputs ${scope.requestedInputIndices.join(", ")} | ${scope.readiness} / ${scope.traversal}${scope.corpus ? ` | corpus ${scope.corpus}` : ""}`, + ); + if (scope.commitSha) + prose( + ` Served commit ${scope.commitSha}${scope.requestedRef ? `; requested ref ${scope.requestedRef}` : ""}`, + ); + if (scope.filesScanned !== null || scope.filesInScope !== null) + prose( + ` Files scanned ${scope.filesScanned ?? "unknown"} / in scope ${scope.filesInScope ?? "unknown"}`, + ); + if ( + scope.readiness !== "CURRENT" || + scope.traversal !== "COMPLETE" || + scope.errorCode + ) + prose( + ` Coverage: ${scope.errorCode ?? (scope.readiness === "CURRENT" ? scope.traversal : scope.readiness)}; retryable ${scope.retryable}${scope.publicMessage ? `; ${scope.publicMessage}` : ""}`, + ); + if (scope.binaryFilesSkipped) + prose(` Skipped ${scope.binaryFilesSkipped} binary file(s).`); + if (scope.filesTooLargeSkipped) + prose(` Skipped ${scope.filesTooLargeSkipped} oversized file(s).`); + for (const issue of scope.fileIssues ?? []) + prose( + ` File issue: ${issue.filePath}${issue.line > 0 ? `:${issue.line}` : " (aggregate)"} | ${issue.code}${issue.contentSafety.filtered ? " | safety normalization applied" : ""}`, + ); + if (scope.fileIssuesOmitted) + prose(` ${scope.fileIssuesOmitted} additional file issue(s) omitted.`); + } + for (const omitted of result.unavailableTargets) { + prose( + `Unavailable input ${omitted.inputIndex}: ${omitted.target} | ${omitted.reason} | retryable ${omitted.retryable}`, + ); + if (omitted.progressRef) prose(` Progress: ${omitted.progressRef}`); + for (const target of omitted.suggestedSiteTargets ?? []) + prose(` Suggested site: ${target}`); + } + if (result.hits.length === 0) + prose( + isExhaustive(result) + ? "No matches." + : "Zero returned matches; coverage is incomplete (see scope and omission details).", + ); + for (const group of consecutiveGroups(result.hits)) { + const first = group[0]; + if (!first) continue; + lines.push("", `[${first.targetIndex}] ${escapeText(locator(first))}`); + const rendered = new Map(); + for (const hit of group) { + hit.contextBeforeSlices.forEach((slice, index) => { + addLine( + rendered, + hit.line - hit.contextBeforeSlices.length + index, + slice, + false, + ); + }); + addLine(rendered, hit.line, hit.lineSlice, true); + hit.contextAfterSlices.forEach((slice, index) => { + addLine(rendered, hit.line + index + 1, slice, false); + }); + } + let previous: number | undefined; + for (const line of [...rendered.values()].sort( + (a, b) => a.number - b.number, + )) { + if (previous !== undefined && line.number > previous + 1) + lines.push(" --"); + lines.push( + ` ${line.number}${line.match ? ":" : "-"} ${renderSlice(line.slice)}`, + ); + previous = line.number; + } + const actions = new Set( + group.map((hit) => formatReadAction(hit.read, options.syntax ?? "cli")), + ); + for (const action of actions) lines.push(` Read: ${action}`); + if (group.some((hit) => hit.contentSafety.filtered)) + prose( + " Safety normalization applied; physical source coordinates are retained in JSON.", + ); + } + if (result.nextCursor) { + lines.push(""); + prose( + "Continue with the same ordered targets and controls, using this cursor:", + ); + lines.push( + options.syntax === "mcp" + ? ` cursor=${JSON.stringify(result.nextCursor)}` + : ` --cursor ${shellQuoteExact(result.nextCursor)}`, + ); + } + if (result.traversal === "CURSOR_EXPIRED") + prose( + "Cursor expired. Restart explicitly without the cursor; retained matches and omissions are shown above.", + ); + else if (result.traversal !== "COMPLETE" && !result.nextCursor) + prose("Traversal is incomplete and has no continuation cursor."); + if (result.targets.some((scope) => scope.kind === "SITE")) + prose( + "Hosted page reads use the latest active content; pages can change after this search.", + ); + return lines.join("\n"); +} + +function isExhaustive(result: GrepResult): boolean { + return ( + result.traversal === "COMPLETE" && + result.unavailableTargets.length === 0 && + result.targets.every((scope) => !hasCoverageGap(scope)) + ); +} +function hasCoverageGap(scope: GrepTargetStatus): boolean { + return ( + scope.readiness !== "CURRENT" || + scope.traversal !== "COMPLETE" || + scope.errorCode !== null || + Boolean( + scope.binaryFilesSkipped || + scope.filesTooLargeSkipped || + scope.fileIssues?.length || + scope.fileIssuesOmitted, + ) + ); +} +function locator(hit: GrepHit): string { + return hit.__typename === "GrepRepositoryHit" ? hit.filePath : hit.pageUrl; +} +function consecutiveGroups(hits: GrepHit[]): GrepHit[][] { + const groups: GrepHit[][] = []; + let previousKey: string | undefined; + let slices = new Map(); + for (const hit of hits) { + const key = JSON.stringify([ + hit.targetIndex, + hit.__typename, + locator(hit), + hit.read.target, + hit.read.path, + ]); + const previousSlice = slices.get(hit.line); + const differentWindow = + previousSlice !== undefined && + (previousSlice.startByte !== hit.lineSlice.startByte || + previousSlice.endByte !== hit.lineSlice.endByte || + previousSlice.content !== hit.lineSlice.content); + if (key !== previousKey || differentWindow) { + groups.push([]); + slices = new Map(); + } + groups[groups.length - 1]?.push(hit); + slices.set(hit.line, hit.lineSlice); + previousKey = key; + } + return groups; +} +function addLine( + lines: Map, + number: number, + slice: GrepLineSlice, + match: boolean, +): void { + if (match || !lines.has(number)) lines.set(number, { number, slice, match }); +} +function renderSlice(slice: GrepLineSlice): string { + return `${slice.startByte > 0 ? "[...] " : ""}${escapeText(slice.content)}${slice.endByte < slice.originalLineBytes ? " [...]" : ""}`; +} +/** Replay the backend action verbatim, using the caller's argument syntax. */ +export function formatReadAction( + action: GrepReadAction, + syntax: "cli" | "mcp", +): string { + if (syntax === "mcp") + return `read target=${JSON.stringify(action.target)}${action.path !== null ? ` path=${JSON.stringify(action.path)}` : ""} start_line=${action.startLine} end_line=${action.endLine}`; + if (action.target.startsWith("-") || action.path?.startsWith("-")) + return `githits read --lines ${action.startLine}-${action.endLine} -- ${shellQuoteExact(action.target)}${action.path !== null ? ` ${shellQuoteExact(action.path)}` : ""}`; + return `githits read ${shellQuoteExact(action.target)}${action.path !== null ? ` ${shellQuoteExact(action.path)}` : ""} --lines ${action.startLine}-${action.endLine}`; +} +function escapeText(value: string): string { + // biome-ignore lint/suspicious/noControlCharactersInRegex: Escape terminal controls deliberately. + return value.replace(/[\\\u0000-\u001f\u007f-\u009f]/g, (character) => + character === "\\" + ? "\\\\" + : `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ); +} +function wrap(text: string, width: number): string[] { + const lines: string[] = []; + const indent = text.match(/^ */)?.[0] ?? ""; + let current = indent; + for (const word of text.trimStart().split(" ")) { + const hasWord = current.length > indent.length; + if (hasWord && terminalWidth(`${current} ${word}`) > width) { + lines.push(current); + current = `${indent}${word}`; + } else current = hasWord ? `${current} ${word}` : `${indent}${word}`; + } + if (current) lines.push(current); + return lines; +} diff --git a/packages/mcp/src/shared/mapped-error.ts b/packages/mcp/src/shared/mapped-error.ts index 03c97f5c..c5548aa0 100644 --- a/packages/mcp/src/shared/mapped-error.ts +++ b/packages/mcp/src/shared/mapped-error.ts @@ -35,6 +35,8 @@ export type MappedErrorCode = | "UNKNOWN"; export interface MappedErrorDetails { + /** Bounded public unified-grep preparation issues, retaining backend keys. */ + targetIssues?: Record[]; action?: string; hint?: string; availableVersions?: AvailableVersion[]; diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index bc7756d3..d078137e 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -81,6 +81,7 @@ export const EXPECTED_STABLE_TOP_LEVEL_COMMANDS = [ "settings", "read", "list", + "grep", "search", "search-status", "code", @@ -1175,6 +1176,22 @@ async function assertUnauthenticatedBehavior(): Promise { "unauthenticated list JSON must keep stdout clean", ); assertJsonErrorCode(listJson, "unauthenticated list JSON", "AUTH_REQUIRED"); + const grepJson = await runCliWithEnv( + ["grep", "router", SMOKE_PACKAGE_SPEC, "--json"], + env, + ); + assert( + grepJson.exitCode !== 0 && grepJson.stdout.trim() === "", + "unauthenticated grep JSON must keep stdout clean", + ); + assertJsonErrorCode(grepJson, "unauthenticated grep JSON", "AUTH_REQUIRED"); + const grepHelp = await runCliWithEnv(["grep", "--help"], env); + assert( + grepHelp.exitCode === 0 && + grepHelp.stdout.includes("--fixed-strings") && + grepHelp.stdout.includes("global page cap"), + "grep help must expose matching defaults and page limits", + ); for (const group of ["code", "docs"]) { const help = await runCliWithEnv([group, "read", "--help"], env); assert( @@ -1966,6 +1983,73 @@ async function runLiveSmoke(env: Record): Promise { ); } + const grepPage = assertJsonOutput( + await runCli([ + "grep", + "router", + SMOKE_PACKAGE_SPEC, + "--path", + "lib/express.js", + "--limit", + "1", + "--json", + ]), + "unified grep source JSON", + ); + assertRecord(grepPage, "unified grep source JSON"); + assert( + Array.isArray(grepPage.hits) && + grepPage.hits.length === 1 && + Array.isArray(grepPage.targets), + "unified grep must return a bounded source match and statuses", + ); + const grepHit = grepPage.hits[0] as unknown; + assertRecord(grepHit, "unified grep source hit"); + assert( + grepHit.__typename === "GrepRepositoryHit" && + grepHit.filePath === "lib/express.js" && + typeof grepHit.sourceMatchStartByte === "number", + "unified grep must retain source identities and physical coordinates", + ); + assertRecord(grepHit.read, "unified grep exact read"); + const grepRead = grepHit.read; + assert( + typeof grepRead.target === "string" && + typeof grepRead.path === "string" && + typeof grepRead.startLine === "number" && + typeof grepRead.endLine === "number", + "unified grep read action must be complete", + ); + assertJsonOutput( + await runCli([ + "read", + grepRead.target, + grepRead.path, + "--lines", + `${grepRead.startLine}-${grepRead.endLine}`, + "--json", + ]), + "unified grep exact read replay", + ); + const grepText = assertTerminalOutput( + await runCli([ + "grep", + "router", + SMOKE_PACKAGE_SPEC, + "--path", + "lib/express.js", + "--limit", + "1", + ]), + "unified grep source text", + ); + assert( + grepText.includes("match in this page") && + grepText.includes("lib/express.js") && + grepText.includes("Read: githits read"), + "unified grep text must retain result, locator and read action", + ); + const packageListText = assertTerminalOutput( await runCli(["list", SMOKE_PACKAGE_SPEC, "--limit", "2"]), "list package terminal", diff --git a/src/cli.test.ts b/src/cli.test.ts index 46d5010e..c738689c 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -4,6 +4,7 @@ import { registerCodeCommandGroup, registerDocsCommandGroup, registerExampleCommand, + registerGrepCommand, registerListCommand, registerPkgCommandGroup, registerUnifiedSearchCommands, @@ -109,6 +110,7 @@ async function createProgramForHelpSurface(): Promise { registerExampleCommand(program); registerListCommand(program); + registerGrepCommand(program); await registerUnifiedSearchCommands(program); await registerCodeCommandGroup(program, { experimentalTools: true }); await registerDocsCommandGroup(program); @@ -451,6 +453,7 @@ describe("CLI help surface", () => { expect(help).toMatch(/^\s{2}example\b/m); expect(help).not.toMatch(/^\s{2}languages\b/m); expect(help).toMatch(/^\s{2}list\b/m); + expect(help).toMatch(/^\s{2}grep\b/m); expect(help).not.toMatch(/^\s{2}feedback\b/m); expect(help).toMatch(/^\s{2}search\b/m); expect(help).toMatch(/^\s{2}code\b/m); diff --git a/src/cli.ts b/src/cli.ts index f67bee87..02b2a35d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -15,6 +15,7 @@ import { registerDocsCommandGroup, registerDoctorCommand, registerExampleCommand, + registerGrepCommand, registerInitCommand, registerListCommand, registerLoginCommand, @@ -153,6 +154,7 @@ async function main(): Promise { registerSettingsCommand(program); registerReadCommand(program); registerListCommand(program); + registerGrepCommand(program); const registrationArgv = stripRootRegistrationOptions(argv); const updateCheckTask = startUpdateCheckTaskForInvocation({ args: argv, diff --git a/src/commands/grep.test.ts b/src/commands/grep.test.ts new file mode 100644 index 00000000..9bd3fb0e --- /dev/null +++ b/src/commands/grep.test.ts @@ -0,0 +1,347 @@ +import { describe, expect, it, mock, spyOn } from "bun:test"; +import { GrepGraphQLError, type GrepParams } from "@githits/core-internal"; +import { Command } from "commander"; +import { + createMockGrepService, + defaultGrepResult, +} from "../services/test-helpers.js"; +import { + type GrepCommandDependencies, + type GrepCommandOptions, + grepAction, + registerGrepCommand, +} from "./grep.js"; + +function deps( + overrides: Partial = {}, +): GrepCommandDependencies { + return { + grepService: createMockGrepService(), + hasValidToken: true, + mcpUrl: "https://example.test", + createSpinner: () => ({ stop: () => {} }), + ...overrides, + }; +} + +describe("unified grep CLI", () => { + it("registers grep/rg flags and explains remote differences", () => { + const help = registerGrepCommand(new Command()).helpInformation(); + expect(help).toContain(" "); + for (const flag of [ + "-F, --fixed-strings", + "-i, --ignore-case", + "-s, --case-sensitive", + "-A, --after-context", + "-B, --before-context", + "-C, --context", + "--path-prefix", + "--corpus", + "--cursor", + "--limit", + "--wait", + "--json", + ]) + expect(help).toContain(flag); + expect(help).toContain("RE2"); + expect(help).toContain("global page cap"); + expect(help).toContain("independently"); + expect(help).not.toContain("--literal"); + expect(help).not.toContain("--regex"); + }); + it("parses ordered mixed operands and interleaved selectors with explicit defaults", async () => { + const grep = mock((_params: GrepParams) => + Promise.resolve(defaultGrepResult), + ); + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + const program = new Command("githits"); + registerGrepCommand(program, async () => + deps({ grepService: createMockGrepService({ grep }) }), + ); + await program.parseAsync( + [ + "grep", + "router", + "npm:x", + "--glob", + "**/*.ts", + "site:docs.test", + "--path", + "a.ts", + "npm:x", + "--json", + ], + { from: "user" }, + ); + expect(grep.mock.calls[0]?.[0]).toEqual({ + targets: [ + { + target: "npm:x", + corpus: "ALL", + allowUnscoped: true, + pathSelectors: [ + { kind: "GLOB", value: "**/*.ts" }, + { kind: "EXACT", value: "a.ts" }, + ], + }, + { target: "site:docs.test" }, + { + target: "npm:x", + corpus: "ALL", + allowUnscoped: true, + pathSelectors: [ + { kind: "GLOB", value: "**/*.ts" }, + { kind: "EXACT", value: "a.ts" }, + ], + }, + ], + pattern: "router", + patternType: "REGEX", + caseSensitive: true, + contextLinesBefore: 0, + contextLinesAfter: 0, + includeDetailedFields: true, + }); + expect(log.mock.calls[0]?.[0]).toBe(JSON.stringify(defaultGrepResult)); + } finally { + log.mockRestore(); + } + }); + it("makes the last case flag win including short clusters and repeats", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + for (const [flags, expected] of [ + [[], true], + [["-i"], false], + [["-is"], true], + [["-si"], false], + [["-i", "-s", "-i"], false], + [["--ignore-case", "--case-sensitive"], true], + ] as const) { + const grep = mock((_params: GrepParams) => + Promise.resolve(defaultGrepResult), + ); + const program = new Command(); + registerGrepCommand(program, async () => + deps({ grepService: createMockGrepService({ grep }) }), + ); + await program.parseAsync( + ["grep", ...flags, "router", "npm:x", "--json"], + { from: "user" }, + ); + expect(grep.mock.calls[0]?.[0].caseSensitive).toBe(expected); + } + } finally { + log.mockRestore(); + } + }); + it("uses literal mode, side-over-symmetric context and leading-dash patterns", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + for (const flags of [ + ["-A", "1", "-C", "2", "-B", "0"], + ["-C", "2", "-A", "1", "-B", "0"], + ]) { + const grep = mock((_params: GrepParams) => + Promise.resolve(defaultGrepResult), + ); + const program = new Command(); + registerGrepCommand(program, async () => + deps({ grepService: createMockGrepService({ grep }) }), + ); + await program.parseAsync( + ["grep", "-F", ...flags, "--json", "--", "--foo", "github:o/r"], + { from: "user" }, + ); + expect(grep.mock.calls[0]?.[0]).toMatchObject({ + pattern: "--foo", + patternType: "LITERAL", + contextLinesBefore: 0, + contextLinesAfter: 1, + }); + } + } finally { + log.mockRestore(); + } + }); + it("validates every repeated context flag before choosing the final value", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("exit"); + }); + try { + for (const flag of ["-A", "-B", "-C"]) { + const d = deps(); + const program = new Command(); + registerGrepCommand(program, async () => d); + await expect( + program.parseAsync( + ["grep", "router", "npm:x", flag, "11", flag, "0", "--json"], + { from: "user" }, + ), + ).rejects.toThrow("exit"); + expect(d.grepService.grep).not.toHaveBeenCalled(); + expect(JSON.parse(String(error.mock.calls.at(-1)?.[0])).code).toBe( + "INVALID_ARGUMENT", + ); + } + const grep = mock((_params: GrepParams) => + Promise.resolve(defaultGrepResult), + ); + const program = new Command(); + registerGrepCommand(program, async () => + deps({ grepService: createMockGrepService({ grep }) }), + ); + await program.parseAsync( + [ + "grep", + "router", + "npm:x", + "-A", + "1", + "-A", + "3", + "-B", + "0", + "-B", + "4", + "-C", + "1", + "-C", + "2", + "--json", + ], + { from: "user" }, + ); + expect(grep.mock.calls[0]?.[0]).toMatchObject({ + contextLinesBefore: 4, + contextLinesAfter: 3, + }); + } finally { + log.mockRestore(); + error.mockRestore(); + exit.mockRestore(); + } + }); + it("rejects all invalid context controls and source flags on site-only operands", async () => { + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("exit"); + }); + try { + for (const options of [ + { context: ["11"], beforeContext: ["0"], afterContext: ["0"] }, + { beforeContext: ["-1"] }, + { afterContext: ["1.2"] }, + { corpus: "all" }, + { path: ["a"] }, + { glob: ["*"] }, + ] satisfies GrepCommandOptions[]) { + const d = deps(); + await expect( + grepAction( + "router", + ["site:docs.test"], + { ...options, json: true }, + d, + ), + ).rejects.toThrow("exit"); + expect(d.grepService.grep).not.toHaveBeenCalled(); + expect(JSON.parse(String(error.mock.calls.at(-1)?.[0])).code).toBe( + "INVALID_ARGUMENT", + ); + } + } finally { + exit.mockRestore(); + error.mockRestore(); + } + }); + it("keeps partial and cursor-expired pages successful and always stops the spinner", async () => { + const write = spyOn(process.stdout, "write").mockImplementation(() => true); + const stop = mock(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("unexpected exit"); + }); + try { + await grepAction( + "router", + ["npm:x"], + {}, + deps({ + createSpinner: () => ({ stop }), + grepService: createMockGrepService({ + grep: async () => ({ + ...defaultGrepResult, + traversal: "CURSOR_EXPIRED", + }), + }), + }), + ); + expect(exit).not.toHaveBeenCalled(); + expect(stop).toHaveBeenCalledTimes(1); + expect(write.mock.calls[0]?.[0]).toContain("Restart explicitly"); + } finally { + write.mockRestore(); + exit.mockRestore(); + } + }); + it("emits clean structured auth and preparation errors without writing stdout", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("exit"); + }); + const stop = mock(() => {}); + try { + await expect( + grepAction( + "router", + ["npm:x"], + { json: true }, + deps({ hasValidToken: false }), + ), + ).rejects.toThrow("exit"); + expect(JSON.parse(String(error.mock.calls[0]?.[0])).code).toBe( + "AUTH_REQUIRED", + ); + await expect( + grepAction( + "router", + ["npm:x"], + { json: true }, + deps({ + createSpinner: () => ({ stop }), + grepService: createMockGrepService({ + grep: async () => { + throw new GrepGraphQLError("not ready", { + code: "GREP_TARGET_PREPARATION_REQUIRED", + retryable: true, + target_issues: [ + { + input_index: 0, + reason: "indexing", + retryable: true, + progress_ref: "index:1", + }, + ], + }); + }, + }), + }), + ), + ).rejects.toThrow("exit"); + expect( + JSON.parse(String(error.mock.calls.at(-1)?.[0])).details.targetIssues[0] + .progress_ref, + ).toBe("index:1"); + expect(stop).toHaveBeenCalledTimes(1); + expect(log).not.toHaveBeenCalled(); + } finally { + log.mockRestore(); + error.mockRestore(); + exit.mockRestore(); + } + }); +}); diff --git a/src/commands/grep.ts b/src/commands/grep.ts new file mode 100644 index 00000000..b5322664 --- /dev/null +++ b/src/commands/grep.ts @@ -0,0 +1,250 @@ +import type { GrepService } from "@githits/core-internal"; +import { + buildGrepParams, + formatGrepText, + type GrepRequestTargetInput, + InvalidGrepRequestError, + mapGrepError, + normalizeGrepContextLines, + projectGrepResult, + requireAuth, + sanitizeTerminalText, + shouldUseColors, +} from "@githits/mcp/internal"; +import type { Command } from "commander"; +import { createContainer } from "../container.js"; +import { recordCliErrorClassification } from "../shared/cli-error-diagnostics.js"; +import { type Spinner, startSpinner } from "../shared/spinner.js"; +import { + buildCliMappedErrorPayload, + formatMappedErrorForTerminal, +} from "./format-mapped-error.js"; + +export interface GrepCommandOptions { + fixedStrings?: boolean; + ignoreCase?: boolean; + caseSensitive?: boolean; + afterContext?: string[]; + beforeContext?: string[]; + context?: string[]; + path?: string[]; + pathPrefix?: string[]; + glob?: string[]; + corpus?: string; + limit?: string; + cursor?: string; + wait?: string; + json?: boolean; + pathSelectors?: GrepRequestTargetInput["pathSelectors"]; +} +export interface GrepCommandDependencies { + grepService: GrepService; + hasValidToken: boolean; + mcpUrl: string; + createSpinner?: () => Spinner; +} +export type GrepCommandDependenciesFactory = + () => Promise; + +/** Execute one mixed-source page with grep/rg matching defaults. */ +export async function grepAction( + pattern: string, + targets: string[], + options: GrepCommandOptions, + deps: GrepCommandDependencies, +): Promise { + try { + requireAuth(deps); + const context = contextValue(options.context, "--context"); + const before = contextValue(options.beforeContext, "--before-context"); + const after = contextValue(options.afterContext, "--after-context"); + const sourceOptions = + options.corpus !== undefined || + options.path !== undefined || + options.pathPrefix !== undefined || + options.glob !== undefined; + if (sourceOptions && targets.every(isSite)) + throw new InvalidGrepRequestError( + "targets", + "Source flags require a package or repository operand.", + ); + const pathSelectors = options.pathSelectors ?? [ + ...(options.path ?? []).map((value) => ({ + kind: "exact" as const, + value, + })), + ...(options.pathPrefix ?? []).map((value) => ({ + kind: "prefix" as const, + value, + })), + ...(options.glob ?? []).map((value) => ({ + kind: "glob" as const, + value, + })), + ]; + const params = buildGrepParams({ + pattern, + targets: targets.map((target) => + isSite(target) + ? { target } + : { target, corpus: options.corpus, pathSelectors }, + ), + patternType: options.fixedStrings ? "literal" : "regex", + ignoreCase: options.ignoreCase, + contextLinesBefore: before ?? context, + contextLinesAfter: after ?? context, + maxMatches: numeric(options.limit), + waitTimeoutMs: numeric(options.wait), + cursor: options.cursor, + includeDetailedFields: options.json === true, + }); + const spinner = + deps.createSpinner?.() ?? + startSpinner("Searching indexed source and documentation", !options.json); + const result = await deps.grepService + .grep(params) + .finally(() => spinner.stop()); + const projected = projectGrepResult(result); + if (options.json) console.log(JSON.stringify(projected)); + else + process.stdout.write( + `${formatGrepText(projected, { useColors: shouldUseColors(), width: process.stdout.columns || 80, syntax: "cli" })}\n`, + ); + } catch (error) { + const mapped = mapGrepError(error); + if (error instanceof InvalidGrepRequestError) { + const label = CLI_FIELDS[error.field]; + if (label) mapped.message = mapped.message.replace(error.field, label); + } + recordCliErrorClassification("grep", error, mapped); + if (options.json) + console.error(JSON.stringify(buildCliMappedErrorPayload(mapped))); + else + console.error( + formatMappedErrorForTerminal({ + ...mapped, + message: sanitizeTerminalText(mapped.message), + details: { + ...mapped.details, + ...(mapped.details?.hint + ? { hint: sanitizeTerminalText(mapped.details.hint) } + : {}), + }, + }), + ); + process.exit(1); + } +} +const CLI_FIELDS: Record = { + maxMatches: "--limit", + waitTimeoutMs: "--wait", + contextLinesBefore: "--before-context", + contextLinesAfter: "--after-context", + patternType: "pattern mode", + ignoreCase: "--ignore-case", + cursor: "--cursor", +}; +function numeric(value: string | undefined): number | undefined { + return value === undefined + ? undefined + : /^\d+$/.test(value.trim()) + ? Number(value) + : Number.NaN; +} +function contextValue( + values: string[] | undefined, + field: string, +): number | undefined { + return values + ?.map((value) => normalizeGrepContextLines(numeric(value), field)) + .at(-1); +} +function isSite(target: string): boolean { + return target.trim().toLowerCase().startsWith("site:"); +} +function collect(value: string, previous: string[] = []): string[] { + return [...previous, value]; +} + +/** Register the top-level grep command without changing legacy code grep. */ +export function registerGrepCommand( + program: Command, + dependenciesFactory: GrepCommandDependenciesFactory = createContainer, +): Command { + const command = program + .command("grep") + .summary("Find regex or literal matches across source and documentation") + .description( + "Search ordered package, repository, and site: targets. Defaults: RE2 regex, case-sensitive, zero context, and all indexed repository files. Package targets also include selected hosted documentation independently of --corpus and path filters. Matching uses indexed content, not local files. Regex supports RE2 and backend anchoring requirements; unsupported or anchorless expressions fail explicitly. -s follows rg (case-sensitive); grep uses -s to suppress errors. Unlike grep/rg -m, --limit is a global page cap. Partial results and exact read actions remain visible. Use -- before a leading-dash pattern.", + ) + .argument("", "RE2 regex, or literal with -F") + .argument("", "Ordered package, repository, or site: operands") + .option("-F, --fixed-strings", "Match a literal string") + .option("-i, --ignore-case", "Ignore case (Unicode folding)") + .option( + "-s, --case-sensitive", + "Match case sensitively (rg convention; last case flag wins)", + ) + .option("-A, --after-context ", "Trailing context lines (0-10)", collect) + .option("-B, --before-context ", "Leading context lines (0-10)", collect) + .option( + "-C, --context ", + "Context on both sides; -A/-B override their side (0-10)", + collect, + ) + .option( + "--path ", + "Exact source path, applied to every source operand (repeatable)", + collect, + ) + .option( + "--path-prefix ", + "Source path prefix (repeatable)", + collect, + ) + .option("--glob ", "Source path glob (repeatable)", collect) + .option( + "--corpus ", + "Repository files: source, documentation, or all (default: all)", + ) + .option( + "--limit ", + "Global page match cap (1-1000; backend default: 100)", + ) + .option( + "--cursor ", + "Continue with the same ordered operands and controls", + ) + .option("--wait ", "Wait for target preparation (0-300000 ms)") + .option("--json", "Emit the detailed lossless JSON page") + .action( + async ( + pattern: string, + targets: string[], + options: GrepCommandOptions, + ) => { + await grepAction( + pattern, + targets, + options, + await dependenciesFactory(), + ); + }, + ); + command.on("option:case-sensitive", () => + command.setOptionValue("ignoreCase", false), + ); + for (const [name, kind] of [ + ["path", "exact"], + ["path-prefix", "prefix"], + ["glob", "glob"], + ] as const) { + command.on(`option:${name}`, (value: string) => + command.setOptionValue("pathSelectors", [ + ...(command.getOptionValue("pathSelectors") ?? []), + { kind, value }, + ]), + ); + } + return command; +} diff --git a/src/commands/index.ts b/src/commands/index.ts index 568ee00f..6c316ad9 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -20,6 +20,7 @@ export { exampleAction, registerExampleCommand, } from "./example.js"; +export { grepAction, registerGrepCommand } from "./grep.js"; export { type InitDependencies, type InitOptions, diff --git a/src/container.test.ts b/src/container.test.ts index 0892ccfd..2f0ad566 100644 --- a/src/container.test.ts +++ b/src/container.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { AgenticAskServiceImpl, + GrepServiceImpl, ListServiceImpl, ReadServiceImpl, ResolveTargetServiceImpl, @@ -408,6 +409,7 @@ describe("createContainer", () => { expect(deps.agenticAskService).toBeInstanceOf(AgenticAskServiceImpl); expect(deps.readService).toBeInstanceOf(ReadServiceImpl); expect(deps.listService).toBeInstanceOf(ListServiceImpl); + expect(deps.grepService).toBeInstanceOf(GrepServiceImpl); }), ); }); @@ -423,6 +425,7 @@ describe("createContainer", () => { expect(deps.agenticAskService).toBeInstanceOf(AgenticAskServiceImpl); expect(deps.readService).toBeInstanceOf(ReadServiceImpl); expect(deps.listService).toBeInstanceOf(ListServiceImpl); + expect(deps.grepService).toBeInstanceOf(GrepServiceImpl); }), ), ); diff --git a/src/container.ts b/src/container.ts index 59801e38..e1bbed28 100644 --- a/src/container.ts +++ b/src/container.ts @@ -10,6 +10,8 @@ import { createStaticTokenProvider, type GitHitsService, GitHitsServiceImpl, + type GrepService, + GrepServiceImpl, getApiUrl, getCodeNavigationUrl, getEnvApiToken, @@ -304,6 +306,8 @@ export interface Dependencies { packageIntelligenceService: PackageIntelligenceService; /** Unified source/site inventory service used by `githits list`. */ listService: ListService; + /** Unified source/site grep service used by `githits grep`. */ + grepService: GrepService; /** Unified compact code/documentation read service. */ readService: ReadService; /** Resolves fuzzy package/repository names for the CLI dogfood surface. */ @@ -392,6 +396,12 @@ export async function createContainer( fetchFn, serviceRuntime, ); + const grepService = new GrepServiceImpl( + codeNavigationUrl, + tokenProvider, + fetchFn, + serviceRuntime, + ); const resolveTargetService = new ResolveTargetServiceImpl( codeNavigationUrl, tokenProvider, @@ -420,6 +430,7 @@ export async function createContainer( packageIntelligenceService, readService, listService, + grepService, resolveTargetService, agenticAskService, githitsService: new GitHitsServiceImpl( @@ -476,6 +487,12 @@ export async function createContainer( fetchFn, serviceRuntime, ); + const grepService = new GrepServiceImpl( + codeNavigationUrl, + tokenManager, + fetchFn, + serviceRuntime, + ); const resolveTargetService = new ResolveTargetServiceImpl( codeNavigationUrl, tokenManager, @@ -504,6 +521,7 @@ export async function createContainer( packageIntelligenceService, readService, listService, + grepService, resolveTargetService, agenticAskService, githitsService: new RefreshingGitHitsService( diff --git a/src/services/test-helpers.ts b/src/services/test-helpers.ts index 47207752..c3c481c5 100644 --- a/src/services/test-helpers.ts +++ b/src/services/test-helpers.ts @@ -7,7 +7,10 @@ import type { CodeNavigationService, DependencyReport, GitHitsService, + GrepParams, GrepRepoResult, + GrepResult, + GrepService, ListParams, ListResult, ListService, @@ -1048,6 +1051,25 @@ export function createMockListService( }; } +export const defaultGrepResult: GrepResult = { + hits: [], + targets: [], + unavailableTargets: [], + traversal: "COMPLETE", + nextCursor: null, + totalMatches: 0, +}; + +/** Creates a mock unified grep service with a valid empty result. */ +export function createMockGrepService( + impl: Partial = {}, +): GrepService { + return { + grep: mock((_params: GrepParams) => Promise.resolve(defaultGrepResult)), + ...impl, + }; +} + export const defaultResolveTargetResult: ResolveTargetResult = { best: { kind: "PACKAGE", diff --git a/src/shared/cli-error-diagnostics.ts b/src/shared/cli-error-diagnostics.ts index 5532c32d..c5353959 100644 --- a/src/shared/cli-error-diagnostics.ts +++ b/src/shared/cli-error-diagnostics.ts @@ -6,7 +6,11 @@ import { } from "@githits/mcp/internal"; import { debugLog } from "./debug-log.js"; -export type CliErrorDiagnosticsArea = "code-nav" | "list" | "pkg-intel"; +export type CliErrorDiagnosticsArea = + | "code-nav" + | "list" + | "pkg-intel" + | "grep"; /** * Classify an error for a CLI command and retain the existing opt-in From c41c88d0c923782e0d88af59ac44d2d2c3623250 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 28 Sep 2026 16:32:16 +0300 Subject: [PATCH 2/6] fix: preserve grep content and simplify CLI recovery Preserve backslashes in matched and context lines, provide retryable preparation wait guidance, and share neutral cursor recovery. Use one ordered selector path and the canonical shared site classifier; record focused and full verification and the unchanged backend signoff blocker. --- docs/implementation/unified-grep.md | 3 +- docs/plans/unified-grep.md | 53 ++++++++++++++++--- .../mcp/src/shared/grep-error-map.test.ts | 5 +- packages/mcp/src/shared/grep-error-map.ts | 2 +- packages/mcp/src/shared/grep-request.ts | 5 +- packages/mcp/src/shared/grep-response.test.ts | 30 +++++++++++ packages/mcp/src/shared/grep-text.ts | 15 +++--- src/commands/grep.test.ts | 46 ++++++++++++++-- src/commands/grep.ts | 51 +++++++----------- 9 files changed, 155 insertions(+), 55 deletions(-) diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index b299bf88..862d5797 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -85,7 +85,8 @@ sibling hits and omissions remain visible. Complete/partial pages exit zero, including zero hits. Failures exit nonzero; JSON errors go to stderr with clean stdout. Preparation errors map to `INDEXING` and preserve up to 20 public `targetIssues` with backend keys and per-input -recovery data. Invalid cursors map to `INVALID_ARGUMENT` with distinct +recovery data. Retryable preparation errors include CLI `--wait ` recovery +guidance. Invalid cursors map to `INVALID_ARGUMENT` with distinct `graphqlCode`. Protocol, transport, auth, terms, update, deadline and HTTP failures retain mapped categories. There is no legacy fallback or automatic preparation retry/cursor restart. diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md index 450dffe4..28164b2a 100644 --- a/docs/plans/unified-grep.md +++ b/docs/plans/unified-grep.md @@ -360,7 +360,7 @@ the existing CLI's successful-empty-result convention. ## Ordered phases -### Phase 1 — top-level CLI mixed grep (IN PROGRESS) +### Phase 1 — top-level CLI mixed grep (IMPLEMENTED; LIVE SIGNOFF BLOCKED) Implementation checkpoint (2026-09-28): @@ -371,15 +371,15 @@ Implementation checkpoint (2026-09-28): `GrepRepo.canonical_scope`; a two-target 2,000-selector fixture verifies it. - Native `aigrep-grep` `MatchIter` emits individual matches, including several on one line. The formatter keeps different slice windows in separate blocks. -- `bun test`: 5,094 passed / 0 failed, 18,504 assertions across 222 files. +- `bun test`: 5,095 passed / 0 failed, 18,512 assertions across 222 files. Typecheck, build, formatting and public-package validation pass. Source CLI/MCP unauthenticated smoke passes; built Node CLI/MCP smoke passes. - Internal review is clean after the repeated-context correction. External - implementation review remains pending. + Revised internal review is clean after both finding closures. External + implementation round 2 remains pending. - Authenticated `GITHITS_ENV=dev bun run smoke:cli` fails at the strict new `router npm:express@5.2.1 --path lib/express.js --limit 1 --json` assertion with the backend protocol error below. Earlier stable smoke assertions pass; - subsequent assertions are not reached. Authenticated MCP smoke is in progress. + subsequent assertions are not reached. Authenticated MCP smoke passes for stable and experimental cohorts. - Fresh dev authentication now works through the default macOS Keychain. No credentials were printed. Express source and mixed source/site requests return both hit kinds; selected/explicit site attribution is `[0, 1]`. @@ -400,7 +400,6 @@ Implementation checkpoint (2026-09-28): acceptance remains UNPROVEN until the backend is fixed and replayed. - Expected outcome: users can grep ordered source/site scopes with one CLI invocation and replay exact reads or continuation, while MCP and the legacy CLI command retain current behavior. @@ -679,7 +678,6 @@ All product steering is resolved. No rejected findings, unresolved review items, deferred development, or new infrastructure. Phase 1 can begin from this design; fresh authenticated conformance remains implementation acceptance. - Implementation preflight closure (2026-09-28): - Config registration list omitted grep: accepted; added it to @@ -695,7 +693,6 @@ Implementation preflight closure (2026-09-28): here would require a cast or duplicate validation in the adapter. Clarified the boundary in JSDoc; the unused input alias is already absent. - Internal code-review closure (2026-09-28): - Repeated context flags could hide an invalid earlier value: accepted. The @@ -710,3 +707,43 @@ The full revised internal code-review round is clean. The earlier context finding is closed; no additional code findings were raised. Remaining live small mixed-page acceptance is an external backend dependency, not a client fallback or reduced scope. No refactor or new infrastructure was needed. + +External implementation round 1 closure (2026-09-28): + +- Source/context backslashes doubled: accepted medium output-fidelity finding. + Ordinary regex/string/path lines displayed different code. Formatter content + now escapes terminal controls only; locator/prose escaping remains explicit. + Added a source+context regression with regex escapes, a Windows path and ESC. +- Retryable preparation had no wait recovery: accepted low CLI UX finding. + CLI owns flag-specific recovery; append `--wait ` guidance only to + retryable INDEXING, preserving all public per-input details. Shared error + classification remains transport-neutral. Checked nonretryable behavior. +- Shared invalid-cursor hint used CLI syntax: accepted low wording finding. + Use neutral “without the cursor” for CLI/MCP reuse; no new syntax switch. +- Parallel selector collectors and site predicates: accepted low simplicity + finding. CLI now has one ordered event-built selector list used by both real + calls and direct tests. The existing shared normalizer exports its site + predicate internally for CLI reuse. No new module or public MCP API. +- Stale MCP-smoke progress and blank lines: accepted; recorded passing current + authenticated MCP smoke and removed repeated blank lines. + +No rejected findings in this implementation round. No final subagent check +ran because round 1 had code findings. Re-review is required after focused +verification and the full revised internal pass. The backend small-page +acceptance blocker remains unchanged and explicit. + +Final revision verification: `bun test` passes 5,095 tests across 222 files +(18,512 assertions); the four changed grep modules pass 25 focused tests +(160 assertions). Typecheck, changed-file Biome, build and public-package +validation pass. The coordinator initially started built smoke concurrently +with package validation, which rebuilds `dist`; both smoke entry checks failed +while the file was absent. This was a verification sequencing error, not a +product failure. Kept those logs; built Node CLI and MCP smoke both pass when rerun after +validation completes. +The revised internal code-review round is clean. External round 2 is pending. + +The authenticated CLI text path also passes for `-F 'var Router'` against the +emitted pinned Express repository and exact `lib/express.js`: one complete +match, current scope, numbered source line and exact replay action. This is +supplemental proof; it does not replace the failing strict package limit-1 +smoke or satisfy the small mixed-page criterion. diff --git a/packages/mcp/src/shared/grep-error-map.test.ts b/packages/mcp/src/shared/grep-error-map.test.ts index d72ee2cf..6b7bea4b 100644 --- a/packages/mcp/src/shared/grep-error-map.test.ts +++ b/packages/mcp/src/shared/grep-error-map.test.ts @@ -46,7 +46,10 @@ describe("unified grep errors", () => { ).toMatchObject({ code: "INVALID_ARGUMENT", retryable: false, - details: { graphqlCode: "GREP_CURSOR_INVALID" }, + details: { + graphqlCode: "GREP_CURSOR_INVALID", + hint: "Restart explicitly without the cursor, using the same targets and controls.", + }, }); expect( mapGrepError( diff --git a/packages/mcp/src/shared/grep-error-map.ts b/packages/mcp/src/shared/grep-error-map.ts index d5d3fbe4..a0c65216 100644 --- a/packages/mcp/src/shared/grep-error-map.ts +++ b/packages/mcp/src/shared/grep-error-map.ts @@ -105,7 +105,7 @@ export function mapGrepError(error: unknown): MappedError { } if (graphqlCode === "GREP_CURSOR_INVALID") details.hint = - "Restart explicitly without --cursor, using the same targets and controls."; + "Restart explicitly without the cursor, using the same targets and controls."; return { code, message: error.message, diff --git a/packages/mcp/src/shared/grep-request.ts b/packages/mcp/src/shared/grep-request.ts index cf088d00..116d5f12 100644 --- a/packages/mcp/src/shared/grep-request.ts +++ b/packages/mcp/src/shared/grep-request.ts @@ -131,7 +131,7 @@ export function buildGrepParams(input: GrepRequestInput): GrepParams { `${field}.pathSelectors`, ); - if (isSiteTarget(target)) { + if (isGrepSiteTarget(target)) { if (targetInput.corpus !== undefined) { throw invalid( `${field}.corpus`, @@ -305,7 +305,8 @@ function normalizeCursor(value: string | undefined): string | undefined { return value.trim().length === 0 ? undefined : value; } -function isSiteTarget(target: string): boolean { +/** Classify targets by a trimmed, case-insensitive `site:` prefix. */ +export function isGrepSiteTarget(target: string): boolean { return target.trim().toLowerCase().startsWith("site:"); } diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index fdfd96c9..148f7b79 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -62,6 +62,36 @@ function result(overrides: Partial = {}): GrepResult { } describe("unified grep result and text", () => { + it("preserves source and context backslashes while escaping terminal controls", () => { + const source = String.raw`const re = /\d+/; s.split("\n")`; + const context = String.raw`const path = "C:\src\file.ts"`; + const output = formatGrepText( + result({ + hits: [ + { + ...hit, + lineSlice: { + content: `${source}\x1b`, + startByte: 0, + endByte: source.length + 1, + originalLineBytes: source.length + 1, + }, + contextBeforeSlices: [ + { + content: context, + startByte: 0, + endByte: context.length, + originalLineBytes: context.length, + }, + ], + }, + ], + }), + ); + expect(output).toContain(`2: ${source}\\u001b`); + expect(output).toContain(`1- ${context}`); + expect(output).not.toContain("\x1b"); + }); it("preserves different match windows on the same long physical line", () => { const first = { ...hit, diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index 3248b2b3..a6feaeaf 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -200,7 +200,7 @@ function addLine( if (match || !lines.has(number)) lines.set(number, { number, slice, match }); } function renderSlice(slice: GrepLineSlice): string { - return `${slice.startByte > 0 ? "[...] " : ""}${escapeText(slice.content)}${slice.endByte < slice.originalLineBytes ? " [...]" : ""}`; + return `${slice.startByte > 0 ? "[...] " : ""}${escapeControls(slice.content)}${slice.endByte < slice.originalLineBytes ? " [...]" : ""}`; } /** Replay the backend action verbatim, using the caller's argument syntax. */ export function formatReadAction( @@ -214,11 +214,14 @@ export function formatReadAction( return `githits read ${shellQuoteExact(action.target)}${action.path !== null ? ` ${shellQuoteExact(action.path)}` : ""} --lines ${action.startLine}-${action.endLine}`; } function escapeText(value: string): string { - // biome-ignore lint/suspicious/noControlCharactersInRegex: Escape terminal controls deliberately. - return value.replace(/[\\\u0000-\u001f\u007f-\u009f]/g, (character) => - character === "\\" - ? "\\\\" - : `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + return escapeControls(value.replace(/\\/g, "\\\\")); +} +function escapeControls(value: string): string { + return value.replace( + // biome-ignore lint/suspicious/noControlCharactersInRegex: Escape terminal controls deliberately. + /[\u0000-\u001f\u007f-\u009f]/g, + (character) => + `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, ); } function wrap(text: string, width: number): string[] { diff --git a/src/commands/grep.test.ts b/src/commands/grep.test.ts index 9bd3fb0e..eabcedd4 100644 --- a/src/commands/grep.test.ts +++ b/src/commands/grep.test.ts @@ -69,6 +69,10 @@ describe("unified grep CLI", () => { "site:docs.test", "--path", "a.ts", + "--path-prefix", + "lib/", + "--glob", + "**/*.js", "npm:x", "--json", ], @@ -83,6 +87,8 @@ describe("unified grep CLI", () => { pathSelectors: [ { kind: "GLOB", value: "**/*.ts" }, { kind: "EXACT", value: "a.ts" }, + { kind: "PREFIX", value: "lib/" }, + { kind: "GLOB", value: "**/*.js" }, ], }, { target: "site:docs.test" }, @@ -93,6 +99,8 @@ describe("unified grep CLI", () => { pathSelectors: [ { kind: "GLOB", value: "**/*.ts" }, { kind: "EXACT", value: "a.ts" }, + { kind: "PREFIX", value: "lib/" }, + { kind: "GLOB", value: "**/*.js" }, ], }, ], @@ -236,8 +244,8 @@ describe("unified grep CLI", () => { { beforeContext: ["-1"] }, { afterContext: ["1.2"] }, { corpus: "all" }, - { path: ["a"] }, - { glob: ["*"] }, + { pathSelectors: [{ kind: "exact", value: "a" }] }, + { pathSelectors: [{ kind: "glob", value: "*" }] }, ] satisfies GrepCommandOptions[]) { const d = deps(); await expect( @@ -336,7 +344,39 @@ describe("unified grep CLI", () => { JSON.parse(String(error.mock.calls.at(-1)?.[0])).details.targetIssues[0] .progress_ref, ).toBe("index:1"); - expect(stop).toHaveBeenCalledTimes(1); + expect( + JSON.parse(String(error.mock.calls.at(-1)?.[0])).details.hint, + ).toContain("--wait "); + await expect( + grepAction( + "router", + ["site:docs.test"], + { json: true }, + deps({ + createSpinner: () => ({ stop }), + grepService: createMockGrepService({ + grep: async () => { + throw new GrepGraphQLError("ambiguous", { + code: "GREP_TARGET_PREPARATION_REQUIRED", + retryable: false, + target_issues: [ + { + input_index: 0, + reason: "site_ambiguous", + retryable: false, + }, + ], + }); + }, + }), + }), + ), + ).rejects.toThrow("exit"); + const ambiguous = JSON.parse(String(error.mock.calls.at(-1)?.[0])); + expect(ambiguous.retryable).toBe(false); + expect(ambiguous.details.hint).toContain("site_ambiguous"); + expect(ambiguous.details.hint).not.toContain("--wait"); + expect(stop).toHaveBeenCalledTimes(2); expect(log).not.toHaveBeenCalled(); } finally { log.mockRestore(); diff --git a/src/commands/grep.ts b/src/commands/grep.ts index b5322664..30c98c2e 100644 --- a/src/commands/grep.ts +++ b/src/commands/grep.ts @@ -4,6 +4,7 @@ import { formatGrepText, type GrepRequestTargetInput, InvalidGrepRequestError, + isGrepSiteTarget, mapGrepError, normalizeGrepContextLines, projectGrepResult, @@ -27,9 +28,6 @@ export interface GrepCommandOptions { afterContext?: string[]; beforeContext?: string[]; context?: string[]; - path?: string[]; - pathPrefix?: string[]; - glob?: string[]; corpus?: string; limit?: string; cursor?: string; @@ -58,34 +56,18 @@ export async function grepAction( const context = contextValue(options.context, "--context"); const before = contextValue(options.beforeContext, "--before-context"); const after = contextValue(options.afterContext, "--after-context"); + const pathSelectors = options.pathSelectors ?? []; const sourceOptions = - options.corpus !== undefined || - options.path !== undefined || - options.pathPrefix !== undefined || - options.glob !== undefined; - if (sourceOptions && targets.every(isSite)) + options.corpus !== undefined || pathSelectors.length > 0; + if (sourceOptions && targets.every(isGrepSiteTarget)) throw new InvalidGrepRequestError( "targets", "Source flags require a package or repository operand.", ); - const pathSelectors = options.pathSelectors ?? [ - ...(options.path ?? []).map((value) => ({ - kind: "exact" as const, - value, - })), - ...(options.pathPrefix ?? []).map((value) => ({ - kind: "prefix" as const, - value, - })), - ...(options.glob ?? []).map((value) => ({ - kind: "glob" as const, - value, - })), - ]; const params = buildGrepParams({ pattern, targets: targets.map((target) => - isSite(target) + isGrepSiteTarget(target) ? { target } : { target, corpus: options.corpus, pathSelectors }, ), @@ -112,6 +94,17 @@ export async function grepAction( ); } catch (error) { const mapped = mapGrepError(error); + if (mapped.code === "INDEXING" && mapped.retryable) { + mapped.details = { + ...mapped.details, + hint: [ + mapped.details?.hint, + "Retry with --wait to wait for target preparation (up to 300000 ms).", + ] + .filter(Boolean) + .join("\n"), + }; + } if (error instanceof InvalidGrepRequestError) { const label = CLI_FIELDS[error.field]; if (label) mapped.message = mapped.message.replace(error.field, label); @@ -159,9 +152,6 @@ function contextValue( ?.map((value) => normalizeGrepContextLines(numeric(value), field)) .at(-1); } -function isSite(target: string): boolean { - return target.trim().toLowerCase().startsWith("site:"); -} function collect(value: string, previous: string[] = []): string[] { return [...previous, value]; } @@ -195,14 +185,9 @@ export function registerGrepCommand( .option( "--path ", "Exact source path, applied to every source operand (repeatable)", - collect, - ) - .option( - "--path-prefix ", - "Source path prefix (repeatable)", - collect, ) - .option("--glob ", "Source path glob (repeatable)", collect) + .option("--path-prefix ", "Source path prefix (repeatable)") + .option("--glob ", "Source path glob (repeatable)") .option( "--corpus ", "Repository files: source, documentation, or all (default: all)", From efa39fd2a069da93eb06cb88bbe16d5cf58112db Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 28 Sep 2026 16:40:26 +0300 Subject: [PATCH 3/6] docs: record unified grep review and live blocker Record clean external round two and its fresh-context final check alongside current validation and orchestration evidence. Keep Phase 1 live signoff pending backend small-page correction and replay; Phase 2 remains pending merge. --- docs/plans/unified-grep.md | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md index 28164b2a..ffb1b4cd 100644 --- a/docs/plans/unified-grep.md +++ b/docs/plans/unified-grep.md @@ -375,7 +375,7 @@ Implementation checkpoint (2026-09-28): Typecheck, build, formatting and public-package validation pass. Source CLI/MCP unauthenticated smoke passes; built Node CLI/MCP smoke passes. Revised internal review is clean after both finding closures. External - implementation round 2 remains pending. + implementation round 2 is clean, including its fresh-context final check. - Authenticated `GITHITS_ENV=dev bun run smoke:cli` fails at the strict new `router npm:express@5.2.1 --path lib/express.js --limit 1 --json` assertion with the backend protocol error below. Earlier stable smoke assertions pass; @@ -740,10 +740,34 @@ with package validation, which rebuilds `dist`; both smoke entry checks failed while the file was absent. This was a verification sequencing error, not a product failure. Kept those logs; built Node CLI and MCP smoke both pass when rerun after validation completes. -The revised internal code-review round is clean. External round 2 is pending. +The revised internal code-review round is clean. External round 2 is clean. The authenticated CLI text path also passes for `-F 'var Router'` against the emitted pinned Express repository and exact `lib/express.js`: one complete match, current scope, numbered source line and exact replay action. This is supplemental proof; it does not replace the failing strict package limit-1 smoke or satisfy the small mixed-page criterion. + +External implementation round 2 is clean with no findings. The reviewer +verified all round 1 closures over the full revised delta; its one permitted +fresh-context final code-reviewer check also returned no findings. Projection +using the shared schema preserves optional detailed fields and was verified +as valid. The reviewer is retained for follow-up through merge approval. + +Phase 1 client implementation and review are complete; **live signoff remains +blocked**, and the draft must not be treated as merge-ready. Required next +proof is replaying the strict mixed/package `--limit 1` case and authenticated +CLI smoke after the backend correction. The backend/native grep owner must +resolve the protocol failure; the current evidence establishes where the error +is returned, not its exact internal cause. Phase 2 remains pending Phase 1 +merge. No backend worktree was changed, no scope was reduced and no fallback, +retry, polling mechanism or infrastructure was added. + +Orchestration delivery: eight sequential Luna dispatches covered request +normalization, DI/mock wiring, registration and bounded follow-up exports or +mechanical fixes. The per-target selector-cap correction came from the +coordinator's initial interpretation of the contract; it cost one corrective +dispatch and focused verification. During the final predicate export, the +coordinator corrected its evidence command to include the required detailed +mode flag and preserve raw target bytes. Transport, projection, formatting, +errors, CLI semantics, validation and review remained coordinator-owned. From 67cb848c44c41434a6b7aca51ae131ce94d0f329 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 28 Sep 2026 20:31:49 +0300 Subject: [PATCH 4/6] fix: accept unvisited unified grep scopes Accept the documented UNSPECIFIED readiness value while retaining strict malformed-output validation. Preserve scope attribution and continuation, explain unvisited scopes in CLI output and guidance, and cover compact/detailed one-match pages and live dev acceptance after the backend fix. --- changes/unified-grep-cli.added.md | 2 +- docs/implementation/cli-commands.md | 2 +- docs/implementation/unified-grep.md | 16 ++- docs/plans/unified-grep.md | 122 +++++++++++------- .../src/services/grep-service.test.ts | 81 ++++++++---- .../src/services/grep-service.ts | 3 + packages/mcp/src/shared/grep-response.test.ts | 26 ++++ packages/mcp/src/shared/grep-text.ts | 2 +- scripts/cli-smoke.ts | 20 +++ src/commands/grep.ts | 2 +- 10 files changed, 194 insertions(+), 82 deletions(-) diff --git a/changes/unified-grep-cli.added.md b/changes/unified-grep-cli.added.md index 7a81f60c..64f6d72e 100644 --- a/changes/unified-grep-cli.added.md +++ b/changes/unified-grep-cli.added.md @@ -3,4 +3,4 @@ "@githits/mcp": none --- -- **Unified grep CLI** - Add `githits grep` across ordered package, repository and hosted documentation targets, defaulting to regex, case-sensitive matching and zero context, with `-F`, `-i`, rg-style `-s`, exact read actions and explicit partial coverage. Legacy CLI `code grep` and MCP `code_grep` remain available pending the MCP migration. +- **Unified grep CLI** - Add `githits grep` across ordered package, repository and hosted documentation targets, defaulting to regex, case-sensitive matching and zero context, with `-F`, `-i`, rg-style `-s`, exact read actions and explicit partial coverage, including retained unvisited scopes and cursor continuation. Legacy CLI `code grep` and MCP `code_grep` remain available pending the MCP migration. diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index d938b449..cb2f1f2a 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -58,7 +58,7 @@ envelope when `--json` is requested; terminal output remains human-readable. | `pkg upgrade-review [spec]` | single package spec with current version plus `--to`, positional package range, OR repeatable `--package` ranges | `--to`, repeatable `--package`, `--no-transitive-security`, `--dependency-issues`, `--min-severity`, `--verbose`, `--json` | Compare current and target versions for upgrade evidence: vulnerabilities, changelog entries, deprecation metadata, peer changes, dependency changes, and transitive security evidence by default. Reports facts only. | | `docs list ` *(legacy compatibility)* | package spec (optional `@version`) | `--limit`, `--after`, `--verbose`, `--json` | Help points hosted-site browsing to `githits list site:` and package-local docs to the package target. Existing execution remains unchanged: text emits target-based read commands; JSON retains `docsReadTarget`, stable `pageId`, provenance `sourceUrl`, and exact repo-file metadata when available. | | `list [paths...]` | package, repository, or `site:` target; optional literal paths/globs | `-R, --recursive`, `-s, --silent`, repeatable `--file-type`, `--language`, `--intent`, `--limit`, `--after`, `--wait`, `--json` | List one package/repository source inventory, including package-local documentation files, or one explicitly targeted hosted site. Text is one path per line with `/` on directories; the header reuses backend-authored read targets for follow-up, while `--silent` emits only paths for piping. JSON carries exact actions, cursors, and metadata. | -| `grep ` | ordered package, repository and `site:` operands | `-F/--fixed-strings`, `-i/--ignore-case`, `-s/--case-sensitive`, `-A`, `-B`, `-C`, repeatable `--path`, `--path-prefix`, `--glob`, `--corpus`, `--limit`, `--cursor`, `--wait`, `--json` | Regex, case-sensitive and zero-context defaults; all repository files plus independently selected hosted package docs. Global page cap, exact reads and explicit coverage. See [unified grep](unified-grep.md). Legacy `code grep` remains unchanged. | +| `grep ` | ordered package, repository and `site:` operands | `-F/--fixed-strings`, `-i/--ignore-case`, `-s/--case-sensitive`, `-A`, `-B`, `-C`, repeatable `--path`, `--path-prefix`, `--glob`, `--corpus`, `--limit`, `--cursor`, `--wait`, `--json` | Regex, case-sensitive and zero-context defaults; all repository files plus independently selected hosted package docs. Global page cap, exact reads and explicit coverage; unvisited scopes remain visible and use the same cursor continuation. See [unified grep](unified-grep.md). Legacy `code grep` remains unchanged. | | `read [path]` | docs target/page ID, explicit `site:` target with page path, compact `target#symbol`, or package/repo target with exact path or selector | `--selector`, `--lines`, `--start`, `--end`, `--wait`, `--verbose`, `--json`; `--repo-url` and `--git-ref` retain legacy repo addressing | Compact unified read passes the locator unchanged to the backend and presents the returned code, docs, or symbol-resolution type. A `site:` path selects hosted documentation; other exact paths narrow code selection. `--selector` selects a docs heading or indexed code symbol. HTTP(S) URL fragments and emitted repository docs page IDs retain their backend-resolved documentation behavior. The compact path calls `ReadService`/`Query.read` once. `--repo-url` remains the legacy compatibility path without selector. See [unified read](unified-read.md). | | `docs read ` (deprecated alias) | emitted `docsReadTarget` or historical page ID | `--lines`, `--verbose`, `--json` | Read a documentation page by preferred target or compatible page ID. Default output is content-only; `--lines` fetches a bounded range for long pages. | | `code diff ..` *(experimental; config-gated)* | unversioned package/repository target and exact range, or `--repo-url` and range | `--patch`, `--stat`, `--name-only`, `--name-status`, `--max-files`, `--max-patch-bytes`, `--verbose`, `--json`, one glob after `--` | Silently dogfood bounded repository-wide tree diffs resolved from package versions or repository refs; local-only MCP `code_diff` is available when experimental tools are enabled, while public/remote MCP and shared Agent Skill guidance remain unchanged | diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index 862d5797..7b4bb0cf 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -74,6 +74,13 @@ read paths are repository-root paths at an exact commit. Hosted actions use persisted URLs and read latest active content, which can change after search. The client never hydrates hits or guesses paths. +`UNSPECIFIED` readiness means this page stopped before visiting that scope. +The scope stays in `targets`, retains its input attribution, and reports +`RESUMABLE_LIMIT` traversal. Continue with `nextCursor` and identical ordered +operands/controls to inspect it. Readiness has not yet been observed; this +status does not indicate target failure or unavailable content. Text explains +the unvisited scope, while JSON preserves the backend enum and full status. + Stale/failed scopes, skips, issues, omitted issue counts, safety normalization and unavailable targets stay visible on zero-hit pages. `No matches.` is exhaustive only for complete traversal without coverage gaps. Other empty @@ -97,8 +104,7 @@ refresh. CLI smoke covers registration, unauthenticated errors and source grep. Fresh mixed-site, pagination, read replay, case and corpus conformance is checked against dev before Phase 1 signoff. -Current dev limitation (2026-09-28): mixed/package requests with `--limit 1` -return backend `GREP_BACKEND_PROTOCOL_ERROR`. Mixed continuation at limit 100 -and pinned-repository continuation at limit 1 work. The client preserves the -requested limit and reports the typed error; small mixed-page live acceptance -and Phase 1 signoff await the backend correction. +Dev supports package/mixed `--limit 1` pages, including retained unvisited +scopes. Production deployment of that backend change remains blocked as of +2026-09-28; fresh client validation uses dev. Unknown readiness values and +other malformed output still fail validation. diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md index ffb1b4cd..5dc61231 100644 --- a/docs/plans/unified-grep.md +++ b/docs/plans/unified-grep.md @@ -360,54 +360,53 @@ the existing CLI's successful-empty-result convention. ## Ordered phases -### Phase 1 — top-level CLI mixed grep (IMPLEMENTED; LIVE SIGNOFF BLOCKED) +### Phase 1 — top-level CLI mixed grep (IMPLEMENTED; REVIEWING CORRECTION) -Implementation checkpoint (2026-09-28): +Implementation checkpoint after the backend small-page correction (2026-09-28): - Core query/types/runtime validation, shared request/projection/error/text, root command and both auth branches are implemented. Projection reuses the core wire allowlist. No MCP catalog/public-provider or legacy CLI changes. -- Corrected selector validation to 1,000 per target after tracing backend - `GrepRepo.canonical_scope`; a two-target 2,000-selector fixture verifies it. -- Native `aigrep-grep` `MatchIter` emits individual matches, including several - on one line. The formatter keeps different slice windows in separate blocks. -- `bun test`: 5,095 passed / 0 failed, 18,512 assertions across 222 files. - Typecheck, build, formatting and public-package validation pass. Source - CLI/MCP unauthenticated smoke passes; built Node CLI/MCP smoke passes. - Revised internal review is clean after both finding closures. External - implementation round 2 is clean, including its fresh-context final check. -- Authenticated `GITHITS_ENV=dev bun run smoke:cli` fails at the strict new - `router npm:express@5.2.1 --path lib/express.js --limit 1 --json` assertion - with the backend protocol error below. Earlier stable smoke assertions pass; - subsequent assertions are not reached. Authenticated MCP smoke passes for stable and experimental cohorts. -- Fresh dev authentication now works through the default macOS Keychain. No - credentials were printed. Express source and mixed source/site requests - return both hit kinds; selected/explicit site attribution is `[0, 1]`. - Repository and hosted exact reads replay successfully. Case-sensitive and - ignore-case behavior passes for both hit kinds; `ALL` includes `Readme.md` - while `source` excludes it and retains independently selected hosted docs. - A site-only absent literal returns complete zero hits without omissions. -- Fresh evidence contradicts the historical Plug fixture: package-only - `middleware` currently returns complete zero hits, while explicit - `site:hexdocs.pm/plug` fails with nonretryable `site_ambiguous`. Use verified - `npm:express` plus `site:expressjs.com` for current conformance instead. -- Pagination works with two distinct mixed pages at limit 100, and two - distinct pinned-repository pages at limit 1. Mixed/package requests at limit - 1 fail before client projection with backend - `GREP_BACKEND_PROTOCOL_ERROR` / retryable true. This also affects the new - strict live CLI smoke. Keep the requested limit and surface the typed error; - do not add a fallback or alter the smoke to conceal it. Small mixed-page - acceptance remains UNPROVEN until the backend is fixed and replayed. - +- Backend PR #2832 is merged; dev includes the fix in + `c7389fe5a2f3489902c5b8a2c20014093f6960f3`. The updated schema at + `~/proj/githits/pkgseer-backend/priv/graphql/schema.graphql` documents + `UNSPECIFIED`: a scope not visited before the page limit, retained with + `RESUMABLE_LIMIT` traversal and original input attribution. Production + deployment remains blocked, per the user; validation is dev-only. +- Captured complete detailed package/mixed first pages reproduced the CLI's + missing-enum parser failure, while their CURRENT continuation pages passed. + The exact live CLI repro also reached dev and failed at this parser. + Compact and detailed one-match parser regressions failed before the fix. +- The core type/Zod enum now explicitly accepts `UNSPECIFIED`; unknown + readiness and other malformed fields remain rejected. All eight complete + captured detailed pages parse with deep equality, preserving every selected + status, attribution, hit, traversal and cursor. The four backend-only compact + captures omit the CLI's required slices/read/status fields and are not client + parser fixtures; fresh client compact-query replay provides that proof. +- Text explains an unvisited scope and retains continuation; JSON keeps the + enum unchanged. CLI cursor help and durable docs explain the state. Strict + live smoke asserts retained selected-site attribution and resumability. +- Current focused checks: 28 passed, 0 failed, 175 assertions across the + service, projector/formatter and CLI tests. Full tests pass: 5,098 tests / 0 failures, 18,527 assertions across 222 files; + typecheck, formatting and public-package validation pass (including builds). + Exact live CLI repro and mixed two-page replay pass; fresh core service + compact/detailed limit-1 replay preserves both scopes and attribution, with + UNSPECIFIED on page one and CURRENT on page two. Authenticated dev CLI smoke passes for stable and experimental cohorts; + built Node CLI/MCP smoke passes. Authenticated dev MCP smoke also passes. + Prior PR CI and review were clean; this correction requires re-review. +- Selector bounds remain 1,000 per target; native individual match/slice + semantics and ordered output remain unchanged. Original proof files under + `/tmp/unified-grep-*` are preserved; new evidence uses `/tmp/nuckelavee-grep-*`. Expected outcome: users can grep ordered source/site scopes with one CLI invocation and replay exact reads or continuation, while MCP and the legacy CLI command retain current behavior. Assumptions: existing read/list service wiring and transport conventions remain -applicable; the backend owns target expansion and preparation. Unknowns: small mixed-page backend protocol failure remains unresolved. Fresh -authenticated dev replay of that case must pass before signoff. Production -conformance is outside this increment. Production v6 promotion is not a Phase 1 dependency. +applicable; the backend owns target expansion and preparation. The documented +unvisited-scope state is accepted explicitly, without changing budgets or +adding retries/fallbacks. Unknowns: the retained external reviewer must finish the correction review. Production deployment is blocked and outside this increment; +production readiness is not a Phase 1 dev-acceptance dependency. Product decisions: none blocking implementation of this proposal. Dependencies: backend `Query.grep` and dev v6 access for mixed-source validation. @@ -730,7 +729,7 @@ External implementation round 1 closure (2026-09-28): No rejected findings in this implementation round. No final subagent check ran because round 1 had code findings. Re-review is required after focused verification and the full revised internal pass. The backend small-page -acceptance blocker remains unchanged and explicit. +blocker at that review checkpoint is resolved by the correction below. Final revision verification: `bun test` passes 5,095 tests across 222 files (18,512 assertions); the four changed grep modules pass 25 focused tests @@ -742,11 +741,11 @@ product failure. Kept those logs; built Node CLI and MCP smoke both pass when re validation completes. The revised internal code-review round is clean. External round 2 is clean. -The authenticated CLI text path also passes for `-F 'var Router'` against the +At the initial pre-correction delivery, the authenticated CLI text path passed for `-F 'var Router'` against the emitted pinned Express repository and exact `lib/express.js`: one complete -match, current scope, numbered source line and exact replay action. This is -supplemental proof; it does not replace the failing strict package limit-1 -smoke or satisfy the small mixed-page criterion. +match, current scope, numbered source line and exact replay action. That was +supplemental proof; the later corrected replay covers the strict package +limit-1 and small mixed-page criteria. External implementation round 2 is clean with no findings. The reviewer verified all round 1 closures over the full revised delta; its one permitted @@ -754,14 +753,11 @@ fresh-context final code-reviewer check also returned no findings. Projection using the shared schema preserves optional detailed fields and was verified as valid. The reviewer is retained for follow-up through merge approval. -Phase 1 client implementation and review are complete; **live signoff remains -blocked**, and the draft must not be treated as merge-ready. Required next -proof is replaying the strict mixed/package `--limit 1` case and authenticated -CLI smoke after the backend correction. The backend/native grep owner must -resolve the protocol failure; the current evidence establishes where the error -is returned, not its exact internal cause. Phase 2 remains pending Phase 1 -merge. No backend worktree was changed, no scope was reduced and no fallback, -retry, polling mechanism or infrastructure was added. +The initial delivery was blocked on backend small-page protocol validation. +Backend PR #2832 resolved that failure; the resulting unvisited-scope enum +requires the client correction recorded in the current checkpoint above. +Phase 2 remains pending Phase 1 merge. No backend worktree is changed by this +client correction, and no limits, scopes or acceptance requirements are reduced. Orchestration delivery: eight sequential Luna dispatches covered request normalization, DI/mock wiring, registration and bounded follow-up exports or @@ -771,3 +767,29 @@ dispatch and focused verification. During the final predicate export, the coordinator corrected its evidence command to include the required detailed mode flag and preserve raw target bytes. Transport, projection, formatting, errors, CLI semantics, validation and review remained coordinator-owned. + +Post-deployment contract correction (2026-09-28): accepted the verified enum +omission reported by the backend agent. Root cause was the core client's +readiness allowlist lagging the documented GraphQL enum. Core owns that wire +validation; shared text owns the unvisited-scope explanation. Scope is this +existing PR's correction, with internal delta review and the retained external +Claude session required before completion. No new infrastructure or public MCP +API/catalog/guide change. Production remains blocked; no deployment is authorized. + +Correction validation (2026-09-28): `bun test` 5,098 pass / 0 fail, +18,527 assertions in 222 files; focused grep tests 28 pass / 0 fail. +Typecheck, formatting/Biome, build/public-package validation and built Node +CLI/MCP smoke pass. Authenticated dev CLI smoke passes all 154 steps across +stable and experimental cohorts; authenticated MCP smoke passes. The exact +source repro and mixed two-page CLI/client compact+detailed replays pass. +All dev commands unset `GITHITS_API_TOKEN`, select `GITHITS_ENV=dev`, and set +`GITHITS_MCP_URL=https://mcp-dev.githits.com`, +`GITHITS_API_URL=https://api-dev.githits.com`, and +`GITHITS_CODE_NAV_URL=https://pkgseer-backend-dev.fly.dev` inline. Keychain +access worked in this lane; earlier stalled attempts from the backend lane +are not claimed as passing evidence. No credentials were printed. + +The changed-delta internal review is clean. External correction review and +updated PR CI remain pending. Production remains blocked; no production query, +backend edit or deployment was performed. Original proof artifacts remain +unchanged; new proof is under `/tmp/nuckelavee-grep-*`. diff --git a/packages/core-internal/src/services/grep-service.test.ts b/packages/core-internal/src/services/grep-service.test.ts index 91b23eed..f052db47 100644 --- a/packages/core-internal/src/services/grep-service.test.ts +++ b/packages/core-internal/src/services/grep-service.test.ts @@ -12,6 +12,7 @@ import { type GrepResult, GrepServiceImpl, MalformedGrepResponseError, + parseGrepResult, } from "./grep-service.js"; import { createMockTokenProvider } from "./test-helpers.js"; @@ -117,7 +118,56 @@ function mixed(): GrepResult { }; } +function detailedMixed(): GrepResult { + const data = mixed(); + for (const hit of data.hits) { + Object.assign(hit, { + lineContent: "router", + matchStartByte: 0, + matchEndByte: 6, + sourceMatchStartByte: 5, + sourceMatchEndByte: 11, + contentSafety: { filtered: false, modifications: [] }, + }); + if (hit.__typename === "GrepRepositoryHit") + Object.assign(hit, { + repoUrl: "https://github.com/o/r", + commitSha: "sha", + repositoryFilePath: "packages/x/lib/a.ts", + }); + } + for (const target of data.targets) + Object.assign(target, { + repoUrl: null, + canonicalSite: null, + urlPrefixes: [], + }); + return data; +} + describe("unified grep service", () => { + for (const detailed of [false, true]) { + it(`preserves an unvisited scope on a ${detailed ? "detailed" : "compact"} one-match page and its visited continuation`, () => { + const first = detailed ? detailedMixed() : mixed(); + first.hits = [first.hits[0]!]; + first.totalMatches = 1; + first.traversal = "RESUMABLE_LIMIT"; + first.nextCursor = "page-two"; + Object.assign(first.targets[1]!, { + readiness: "UNSPECIFIED", + traversal: "RESUMABLE_LIMIT", + requestedInputIndices: [0, 1], + }); + const second = detailed ? detailedMixed() : mixed(); + second.hits = [second.hits[1]!]; + second.totalMatches = 1; + second.targets[1]!.requestedInputIndices = [0, 1]; + expect(parseGrepResult(first, detailed)).toEqual(first); + expect(parseGrepResult(second, detailed)).toEqual(second); + expect(first.targets[1]!.errorCode).toBeNull(); + expect(first.unavailableTargets).toEqual([]); + }); + } it("allows the advertised preparation wait before the transport deadline", async () => { const timeout = spyOn(AbortSignal, "timeout").mockImplementation( (milliseconds: number) => { @@ -175,29 +225,7 @@ describe("unified grep service", () => { expect(out.hits[1]?.read.path).toBeNull(); }); it("requires all selected detail fields and preserves nulls and both coordinate systems", async () => { - const data = mixed(); - for (const hit of data.hits) { - Object.assign(hit, { - lineContent: "router", - matchStartByte: 0, - matchEndByte: 6, - sourceMatchStartByte: 5, - sourceMatchEndByte: 11, - contentSafety: { filtered: false, modifications: [] }, - }); - if (hit.__typename === "GrepRepositoryHit") - Object.assign(hit, { - repoUrl: "https://github.com/o/r", - commitSha: "sha", - repositoryFilePath: "packages/x/lib/a.ts", - }); - } - for (const target of data.targets) - Object.assign(target, { - repoUrl: null, - canonicalSite: null, - urlPrefixes: [], - }); + const data = detailedMixed(); const fetcher = mock( async (_url: Parameters[0], init?: RequestInit) => { expect( @@ -221,6 +249,13 @@ describe("unified grep service", () => { for (const data of [ { ...mixed(), hits: [{ ...mixed().hits[0], __typename: "UnknownHit" }] }, { ...mixed(), targets: [] }, + { + ...mixed(), + targets: mixed().targets.map((target) => ({ + ...target, + readiness: "UNKNOWN_READINESS", + })), + }, { ...result(), traversal: "RESUMABLE_LIMIT", nextCursor: null }, { ...result(), totalMatches: "0" }, ]) diff --git a/packages/core-internal/src/services/grep-service.ts b/packages/core-internal/src/services/grep-service.ts index 592abdc4..19908009 100644 --- a/packages/core-internal/src/services/grep-service.ts +++ b/packages/core-internal/src/services/grep-service.ts @@ -35,7 +35,9 @@ export type GrepTraversal = | "NON_RESUMABLE_PARTIAL" | "FAILED" | "CURSOR_EXPIRED"; +/** Observed readiness; UNSPECIFIED denotes a scope not yet visited in this page. */ export type GrepReadiness = + | "UNSPECIFIED" | "CURRENT" | "STALE" | "NOT_AVAILABLE" @@ -286,6 +288,7 @@ function resultSchema(detailed: boolean): z.ZodType { target: z.string(), traversal, readiness: z.enum([ + "UNSPECIFIED", "CURRENT", "STALE", "NOT_AVAILABLE", diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index 148f7b79..b984f16b 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -62,6 +62,32 @@ function result(overrides: Partial = {}): GrepResult { } describe("unified grep result and text", () => { + it("retains an unvisited selected site and explains its continuation", () => { + const page = result({ + targets: [ + target, + { + ...target, + targetIndex: 8, + requestedInputIndices: [0, 1], + kind: "SITE", + target: "site:docs.test", + corpus: null, + readiness: "UNSPECIFIED", + traversal: "RESUMABLE_LIMIT", + }, + ], + traversal: "RESUMABLE_LIMIT", + nextCursor: "opaque", + }); + expect(projectGrepResult(page)).toEqual(page); + const output = formatGrepText(page); + expect(output).toContain("inputs 0, 1 | UNSPECIFIED / RESUMABLE_LIMIT"); + expect(output).toContain("Coverage: not visited in this page"); + expect(output).toContain("--cursor 'opaque'"); + expect(output).not.toContain("Unavailable input"); + expect(output).toContain("2: router"); + }); it("preserves source and context backslashes while escaping terminal controls", () => { const source = String.raw`const re = /\d+/; s.split("\n")`; const context = String.raw`const path = "C:\src\file.ts"`; diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index a6feaeaf..5a38945f 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -50,7 +50,7 @@ export function formatGrepText( scope.errorCode ) prose( - ` Coverage: ${scope.errorCode ?? (scope.readiness === "CURRENT" ? scope.traversal : scope.readiness)}; retryable ${scope.retryable}${scope.publicMessage ? `; ${scope.publicMessage}` : ""}`, + ` Coverage: ${scope.errorCode ?? (scope.readiness === "UNSPECIFIED" ? "not visited in this page" : scope.readiness === "CURRENT" ? scope.traversal : scope.readiness)}; retryable ${scope.retryable}${scope.publicMessage ? `; ${scope.publicMessage}` : ""}`, ); if (scope.binaryFilesSkipped) prose(` Skipped ${scope.binaryFilesSkipped} binary file(s).`); diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index d078137e..eff09b45 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -2003,6 +2003,26 @@ async function runLiveSmoke(env: Record): Promise { Array.isArray(grepPage.targets), "unified grep must return a bounded source match and statuses", ); + const selectedSite = (grepPage.targets as unknown[]).find( + (target) => + typeof target === "object" && + target !== null && + "kind" in target && + target.kind === "SITE", + ); + assertRecord(selectedSite, "unified grep retained selected site"); + assert( + Array.isArray(selectedSite.requestedInputIndices) && + selectedSite.requestedInputIndices.includes(0), + "unified grep must retain selected-site input attribution", + ); + if (selectedSite.readiness === "UNSPECIFIED") + assert( + selectedSite.traversal === "RESUMABLE_LIMIT" && + selectedSite.errorCode === null && + typeof grepPage.nextCursor === "string", + "unvisited grep scope must retain resumable traversal without failure", + ); const grepHit = grepPage.hits[0] as unknown; assertRecord(grepHit, "unified grep source hit"); assert( diff --git a/src/commands/grep.ts b/src/commands/grep.ts index 30c98c2e..b1fcfd6d 100644 --- a/src/commands/grep.ts +++ b/src/commands/grep.ts @@ -198,7 +198,7 @@ export function registerGrepCommand( ) .option( "--cursor ", - "Continue with the same ordered operands and controls", + "Continue with the same ordered operands and controls, including unvisited scopes", ) .option("--wait ", "Wait for target preparation (0-300000 ms)") .option("--json", "Emit the detailed lossless JSON page") From 036e2c8a62738a872bb18f6764f20b26b194f650 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Mon, 28 Sep 2026 20:37:47 +0300 Subject: [PATCH 5/6] docs: record unified grep correction acceptance Record the clean correction review, passing dev acceptance and CI. Keep production deployment blocked and MCP replacement pending the CLI phase merge. --- docs/plans/unified-grep.md | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md index 5dc61231..14468312 100644 --- a/docs/plans/unified-grep.md +++ b/docs/plans/unified-grep.md @@ -360,7 +360,7 @@ the existing CLI's successful-empty-result convention. ## Ordered phases -### Phase 1 — top-level CLI mixed grep (IMPLEMENTED; REVIEWING CORRECTION) +### Phase 1 — top-level CLI mixed grep (COMPLETE; PENDING MERGE) Implementation checkpoint after the backend small-page correction (2026-09-28): @@ -393,7 +393,8 @@ Implementation checkpoint after the backend small-page correction (2026-09-28): compact/detailed limit-1 replay preserves both scopes and attribution, with UNSPECIFIED on page one and CURRENT on page two. Authenticated dev CLI smoke passes for stable and experimental cohorts; built Node CLI/MCP smoke passes. Authenticated dev MCP smoke also passes. - Prior PR CI and review were clean; this correction requires re-review. + Correction CI and internal/external delta review are clean, including the + external reviewer's fresh-context final check. - Selector bounds remain 1,000 per target; native individual match/slice semantics and ordered output remain unchanged. Original proof files under `/tmp/unified-grep-*` are preserved; new evidence uses `/tmp/nuckelavee-grep-*`. @@ -405,7 +406,7 @@ CLI command retain current behavior. Assumptions: existing read/list service wiring and transport conventions remain applicable; the backend owns target expansion and preparation. The documented unvisited-scope state is accepted explicitly, without changing budgets or -adding retries/fallbacks. Unknowns: the retained external reviewer must finish the correction review. Production deployment is blocked and outside this increment; +adding retries/fallbacks. Production deployment is blocked and outside this increment; production readiness is not a Phase 1 dev-acceptance dependency. Product decisions: none blocking implementation of this proposal. Dependencies: backend `Query.grep` and dev v6 access for mixed-source validation. @@ -731,7 +732,7 @@ ran because round 1 had code findings. Re-review is required after focused verification and the full revised internal pass. The backend small-page blocker at that review checkpoint is resolved by the correction below. -Final revision verification: `bun test` passes 5,095 tests across 222 files +Initial pre-correction revision verification: `bun test` passes 5,095 tests across 222 files (18,512 assertions); the four changed grep modules pass 25 focused tests (160 assertions). Typecheck, changed-file Biome, build and public-package validation pass. The coordinator initially started built smoke concurrently @@ -789,7 +790,19 @@ All dev commands unset `GITHITS_API_TOKEN`, select `GITHITS_ENV=dev`, and set access worked in this lane; earlier stalled attempts from the backend lane are not claimed as passing evidence. No credentials were printed. -The changed-delta internal review is clean. External correction review and -updated PR CI remain pending. Production remains blocked; no production query, +The changed-delta internal review and external round 3 are clean. The retained +Claude reviewer checked `git diff efa39fd..67cb848`; its fresh-context final +code-reviewer check also found no issues. Two non-blocking observations are +closed without changes: the manual authenticated dev smoke intentionally +requires the selected hosted-doc scope, as verified in live responses; the +Coverage line preserves `retryable: false` while the unvisited-scope text and +cursor guide continuation. Retryability does not replace pagination, and +altering the backend status would violate the lossless result contract. + +Correction CI is green: [Main](https://github.com/githits-com/githits-cli/actions/runs/36459079262) +passes build/checks, Linux/Windows tests, Bun and Node 20/22/24/26 compatibility; +[MCP package validation](https://github.com/githits-com/githits-cli/actions/runs/36459078714) +passes. The reviewer remains retained through merge approval. Production +remains blocked; no production query, backend edit or deployment was performed. Original proof artifacts remain unchanged; new proof is under `/tmp/nuckelavee-grep-*`. From 932356c0bbad1354e553e83d4b8df71c7c8392ff Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 29 Sep 2026 12:21:34 +0300 Subject: [PATCH 6/6] docs: record production unified grep verification Record successful production source and mixed pagination replays plus authenticated CLI and MCP smoke. Replace the resolved deployment blocker with the verified production contract without changing implementation. --- docs/implementation/unified-grep.md | 10 +++--- docs/plans/unified-grep.md | 51 ++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index 7b4bb0cf..61fe9acf 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -104,7 +104,9 @@ refresh. CLI smoke covers registration, unauthenticated errors and source grep. Fresh mixed-site, pagination, read replay, case and corpus conformance is checked against dev before Phase 1 signoff. -Dev supports package/mixed `--limit 1` pages, including retained unvisited -scopes. Production deployment of that backend change remains blocked as of -2026-09-28; fresh client validation uses dev. Unknown readiness values and -other malformed output still fail validation. +Dev and production support package/mixed `--limit 1` pages, including retained +unvisited scopes. Production client replay on 2026-09-29 verified the exact +source repro, mixed two-page CLI continuation and compact/detailed service +pages: both scopes and input attribution are retained, with a source hit on +page one and a hosted-doc hit on page two. Unknown readiness values and other +malformed output still fail validation. diff --git a/docs/plans/unified-grep.md b/docs/plans/unified-grep.md index 14468312..6d1f0c14 100644 --- a/docs/plans/unified-grep.md +++ b/docs/plans/unified-grep.md @@ -27,8 +27,8 @@ for compatibility. Backend case-sensitive support was verified from the schema, fresh matching conformance is a Phase 1 acceptance check. Overall product decisions: none blocking the proposed design. The CLI argument order and whole-target convenience below are design proposals, not previously -user-confirmed preferences. Backend production readiness is an operational -unknown, not permission to change backend deployment or publication policy. +user-confirmed preferences. Production grep conformance is verified below; +that does not change backend deployment or publication authorization. Dependencies: the checked-in backend contract, existing auth/transport helpers, and Phase 1 before Phase 2. Completion criteria: both phases merged, their surface-specific validation passing, and durable docs updated. Hosted adoption @@ -362,7 +362,7 @@ the existing CLI's successful-empty-result convention. ### Phase 1 — top-level CLI mixed grep (COMPLETE; PENDING MERGE) -Implementation checkpoint after the backend small-page correction (2026-09-28): +Implementation checkpoint after the backend small-page correction and production verification (2026-09-29): - Core query/types/runtime validation, shared request/projection/error/text, root command and both auth branches are implemented. Projection reuses the @@ -371,8 +371,10 @@ Implementation checkpoint after the backend small-page correction (2026-09-28): `c7389fe5a2f3489902c5b8a2c20014093f6960f3`. The updated schema at `~/proj/githits/pkgseer-backend/priv/graphql/schema.graphql` documents `UNSPECIFIED`: a scope not visited before the page limit, retained with - `RESUMABLE_LIMIT` traversal and original input attribution. Production - deployment remains blocked, per the user; validation is dev-only. + `RESUMABLE_LIMIT` traversal and original input attribution. The user confirmed + production deployment on 2026-09-29; fresh production client replay passes + the exact source repro, mixed two-page CLI continuation and compact/detailed + service pages, retaining both scopes and source/site hits. - Captured complete detailed package/mixed first pages reproduced the CLI's missing-enum parser failure, while their CURRENT continuation pages passed. The exact live CLI repro also reached dev and failed at this parser. @@ -406,8 +408,8 @@ CLI command retain current behavior. Assumptions: existing read/list service wiring and transport conventions remain applicable; the backend owns target expansion and preparation. The documented unvisited-scope state is accepted explicitly, without changing budgets or -adding retries/fallbacks. Production deployment is blocked and outside this increment; -production readiness is not a Phase 1 dev-acceptance dependency. +adding retries/fallbacks. Production grep conformance is verified; deployment +and publication remain outside this increment's authorization. Product decisions: none blocking implementation of this proposal. Dependencies: backend `Query.grep` and dev v6 access for mixed-source validation. @@ -496,8 +498,9 @@ Legacy source-only flags disappear from MCP, with migration documented. Assumptions: Phase 1 semantics and shared helpers prove sufficient; required provider service additions follow the existing read-service precedent. -Unknowns: current main's list consolidation status and backend production v6 -status must be rechecked at this boundary. Resolve routing against whatever +Unknowns: current main's list consolidation status must be rechecked at this +boundary. Production grep v6 conformance passed on 2026-09-29; recheck the +deployed contract when Phase 2 starts. Resolve routing against whatever inventory tool is actually advertised, without taking ownership of list work. Product decisions: none; MCP removal is requested. Dependencies: Phase 1 merged, public package compatibility validation, and dev access for MCP conformance. @@ -775,7 +778,8 @@ readiness allowlist lagging the documented GraphQL enum. Core owns that wire validation; shared text owns the unvisited-scope explanation. Scope is this existing PR's correction, with internal delta review and the retained external Claude session required before completion. No new infrastructure or public MCP -API/catalog/guide change. Production remains blocked; no deployment is authorized. +API/catalog/guide change. Production was blocked at this 2026-09-28 checkpoint; +no deployment was authorized by the client correction request. Correction validation (2026-09-28): `bun test` 5,098 pass / 0 fail, 18,527 assertions in 222 files; focused grep tests 28 pass / 0 fail. @@ -802,7 +806,30 @@ altering the backend status would violate the lossless result contract. Correction CI is green: [Main](https://github.com/githits-com/githits-cli/actions/runs/36459079262) passes build/checks, Linux/Windows tests, Bun and Node 20/22/24/26 compatibility; [MCP package validation](https://github.com/githits-com/githits-cli/actions/runs/36459078714) -passes. The reviewer remains retained through merge approval. Production -remains blocked; no production query, +passes. The reviewer remains retained through merge approval. At this +2026-09-28 checkpoint, production remained blocked; no production query, backend edit or deployment was performed. Original proof artifacts remain unchanged; new proof is under `/tmp/nuckelavee-grep-*`. + +Production verification (2026-09-29), after the user confirmed deployment: + +- Replayed `bun run src/cli.ts grep router npm:express@5.2.1 --path + lib/express.js --limit 1 --json`: one source hit, retained selected SITE + scope with UNSPECIFIED/RESUMABLE_LIMIT, and a continuation cursor. +- Mixed CLI pages with identical ordered `npm:express` and + `site:expressjs.com` operands and `--limit 1` returned distinct source then + hosted-doc hits. Both scopes and site attribution `[0,1]` remain present; + the site becomes CURRENT on page two. CLI text explains the unvisited scope + and cursor. Compact and detailed service queries pass the same assertions. +- `bun run smoke:cli` passes all 154 steps, stable and experimental live + cohorts; `bun run smoke:mcp` passes all 65 steps. All three verification + processes exit zero; none skipped authenticated coverage. +- Every command uses `env -u GITHITS_API_TOKEN GITHITS_ENV=prod` with inline + `GITHITS_MCP_URL=https://mcp.githits.com`, + `GITHITS_API_URL=https://api.githits.com`, and + `GITHITS_CODE_NAV_URL=https://oss.githits.dev`. No inherited URL overrides + or API tokens were present. Production credentials stayed private. +- Evidence: `/tmp/nuckelavee-grep-prod-replay.ts`, its `.log` and + `-results.json`, plus `/tmp/nuckelavee-grep-prod-smoke-cli.log` and + `/tmp/nuckelavee-grep-prod-smoke-mcp.log`. Original dev proof is preserved. + No implementation change, backend edit, merge or deployment was performed.