From e7b880d4ec530e83845f810cbf92e36c3e07d004 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Sat, 24 Oct 2026 09:36:16 +0300 Subject: [PATCH 1/4] feat: expose package-path advisories in CLI and MCP grep Select and validate backend path warnings in both response modes, preserve them through shared projections, and display safely quoted unverified alternatives without changing search scope. Cover short-wait partial pages, quoting, native failures and CLI/MCP parity; document production schema and review prerequisites. --- changes/package-path-advisories.added.md | 6 + docs/implementation/unified-grep.md | 97 +++++++++ docs/plans/package-path-advisories.md | 151 ++++++++++++++ .../agentic/probes/package-path-advisories.md | 6 + .../src/services/grep-service.test.ts | 73 +++++++ .../src/services/grep-service.ts | 31 +++ packages/mcp/src/client.ts | 2 + .../shared/fixtures/grep-text/mixed-100.json | 1 + .../fixtures/grep-text/repository-100.json | 1 + .../mcp/src/shared/grep-path-warnings.test.ts | 127 ++++++++++++ packages/mcp/src/shared/grep-response.test.ts | 1 + packages/mcp/src/shared/grep-response.ts | 2 +- .../src/shared/grep-text-rendering.test.ts | 1 + packages/mcp/src/shared/grep-text.ts | 18 ++ .../mcp/src/shared/indexing-estimates.test.ts | 1 + packages/mcp/src/smoke-test.ts | 4 + packages/mcp/src/tools/grep.test.ts | 1 + scripts/cli-smoke.ts | 4 + src/commands/grep.test.ts | 4 +- src/mcp-public-surface.test.ts | 1 + src/services/test-helpers.ts | 1 + src/tools/grep-parity.test.ts | 189 ++++++++++++++++++ 22 files changed, 720 insertions(+), 2 deletions(-) create mode 100644 changes/package-path-advisories.added.md create mode 100644 docs/plans/package-path-advisories.md create mode 100644 eval/agentic/probes/package-path-advisories.md create mode 100644 packages/mcp/src/shared/grep-path-warnings.test.ts diff --git a/changes/package-path-advisories.added.md b/changes/package-path-advisories.added.md new file mode 100644 index 00000000..55d7820d --- /dev/null +++ b/changes/package-path-advisories.added.md @@ -0,0 +1,6 @@ +--- +"githits": minor +"@githits/mcp": minor +--- + +- **Package-path advisories** - Unified grep preserves backend path warnings in CLI/MCP JSON and displays unverified, safely quoted target/path alternatives beside matches and pending results. Requires a backend deployment exposing `GrepResult.pathWarnings` before client release; injected grep providers must return the required warning array. diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index 2063f0bd..e23b798c 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -388,3 +388,100 @@ different package, repository or site target; unknown reasons are not given an invented remedy. Native public messages and error codes have their own reason line. Terminal preparation reasons receive no preparation retry. Auth, requests and JSON are unchanged. The shared helper owns only footer layout; grep owns these semantics. + +## Package-path advisories + +The backend owns path diagnosis using the selected snapshot's canonical package +repository subpath and native path observations. Clients do not infer package +roots from names, probe alternatives, rewrite selectors or retry automatically. +`GrepResult.pathWarnings` is a required array, selected and validated in both +compact and detailed modes. Each warning preserves `requestedInputIndices`, +`target`, `path`, `repositorySubpath`, `message`, and `suggestions { target path }`. +An empty list means no supported diagnosis was returned; zero matches alone are +not path evidence. Missing or malformed fields are protocol errors. + +The shared projection reparses and deep-copies this metadata along with all +hits, native target statuses/public messages, omissions, preparation estimates, +traversal and cursors. The service returns short-wait pages through the same +parser. Pending-only results without authoritative evidence have an empty list; +mixed partial results retain proven warnings beside pending targets. The client +does not synthesize or suppress warnings based on hit counts or pending state. +CLI `--json` and MCP `format: "json"` preserve every warning field and backend +input attribution. Public `@githits/mcp/client` exports `GrepPathWarning` and +`GrepPathSuggestion`; injected `GrepService` providers must supply the array. + +`formatGrepText` displays each backend message once under `Path advisories`, +after preparation/coverage and before matches. Terminal controls in prose are +escaped and prose wraps to the caller's width. Suggested pairs are explicitly +unverified: they do not claim the path exists or will produce hits. Copyable +pair rows stay unwrapped. CLI uses shell-quoted `target=... path=...` values; +MCP uses a JSON object with `target` and `path`. These are advisory values, +not complete commands or MCP grep arguments: callers retain the original +selector kind (`--path` / `--path-prefix`, or `path_selectors[].kind`) and other +controls when choosing an alternative. The warning has no selector-kind field, +so the renderer cannot safely invent one. Full commit SHAs remain intact. +No additional diagnostic target/path sentence repeats the message's own pairs. +Native failures, public messages and real results remain visible independently. + +### Deployment and release prerequisite + +Backend PR [#3550](https://github.com/githits-com/pkgseer-backend/pull/3550) +merged at `b26172c5fc97287256e55d22bc64aa384297184c`. On 2026-10-24, +read-only GraphQL validation of a skipped `grep` selection accepted every +warning field at `https://oss-dev.githits.dev/api/graphql`. Production at +`https://oss.githits.dev/api/graphql` rejected `GrepResult.pathWarnings`. +Introspection is disabled at both endpoints; the validation query uses +`@skip(if: true)` to check selections without executing grep. + +Before releasing a client that selects this field, verify the configured +backend accepts the full selection below. Production needs the backend change +deployed first; merging its PR is not proof of deployment. No schema fallback +or downgraded query is provided: an older endpoint rejects the entire grep +query. Native advisory generation additionally requires the backend's released +aigrep 0.27.3/API56 path-evidence integration. Schema support alone does not +prove native advisory behavior; qualify an applicable request on that endpoint. + +```graphql +query { + grep(targets: [{target: "npm:express"}], pattern: "router") @skip(if: true) { + pathWarnings { + requestedInputIndices target path repositorySubpath message + suggestions { target path } + } + } +} +``` + +Ship both the root `githits` CLI and public `@githits/mcp` package for their +respective consumers. Hosted MCP clients receive this behavior only after +`remote-mcp` upgrades that published package and deploys it through its own +approved workflow. This client PR authorizes no merge, release, publication or +deployment. The backend backlog's client-adoption entry remains for its owner +to close after this client work ships; it is not edited across worktrees. + +### Client qualification (2026-10-24) + +- Built Node CLI and local stdio MCP against dev: prefix and exact selectors + both retained one advisory, both suggested pairs and the full repository SHA + in text and JSON. Prefix returned zero hits with `COMPLETE`; exact returned + zero hits with `NON_RESUMABLE_PARTIAL` and its original + `GREP_PATH_NOT_INDEXED` failure. CLI/MCP warnings, hits, target statuses, + traversal and cursors matched. Secret-free local evidence is retained in + `/tmp/package-path-live-results/`; these transient files are not release proof + for any other endpoint. +- The targeted `eval/agentic/probes/package-path-advisories.md` local MCP/dev + run used Codex (`gpt-6-luna`, high). Its trace contained one `quick_start` and + one `grep` with the requested prefix; no alternative query was executed. + The final answer reported the advisory and proposed alternatives, with + `status: inconclusive` and `confidence: high`. The harness completed in + 14.8 seconds, with no recorded isolation violations. This is observed agent + behavior, not a graded usefulness/quality result. +- Deterministic parser/projection/formatter and CLI/MCP parity tests cover + multiple warnings and input indices, zero hits, existing failures/cursors, + mixed hits/pending and pending-only short-wait pages, quoting and controls. +- Full authenticated live smoke currently stops at an unrelated Go recovery + probe before grep: `go:github.com/docker/docker@v20.10.0` now resolves to + `v20.10.0+incompatible`, contradicting the probe's expected missing version. + The broad live suites are not claimed green. Their auth/registration modes + and the affected grep qualification above are independent checks. No failure + assertion was weakened to hide this backend/fixture discrepancy. diff --git a/docs/plans/package-path-advisories.md b/docs/plans/package-path-advisories.md new file mode 100644 index 00000000..f8a20fb2 --- /dev/null +++ b/docs/plans/package-path-advisories.md @@ -0,0 +1,151 @@ +# Unified grep package-path advisories + +Status: IMPLEMENTED; internal implementation review clean after one test fix. +External plan/implementation clean review unavailable: required Claude Opus 5.5 hit account usage limit before a +settled report; dispatch fenced, no external clean round claimed. + +## Outcome and verified evidence + +CLI and MCP agents receive backend-authored package-path advisories in shared +text and lossless JSON, including empty and partial/pending pages. No client +heuristics, alternative queries, request rewrites, retries or scope changes. + +Current main is `0a97eb12a15dca43b8c802256597db42a96c8a35` (2026-10-24). +Core `grep-service.ts` neither selects nor validates `pathWarnings`; +`projectGrepResult` reparses using that allowlist; both frontends already use +that projection and `formatGrepText`. Short waits have no separate client page +conversion: the service returns the validated backend result directly. + +Backend PR #3550 merged at `b26172c5fc97287256e55d22bc64aa384297184c`. +Read-only skipped-selection GraphQL validation on 2026-10-24 accepted all fields +at `https://oss-dev.githits.dev/api/graphql` and rejected `pathWarnings` at +`https://oss.githits.dev/api/graphql`. Introspection is disabled at both. +Production backend availability is a release prerequisite, not assumed. + +## Scope, ownership and target architecture + +Core owns the typed wire contract and schema validation. Add required +`pathWarnings` with `requestedInputIndices`, `target`, `path`, +`repositorySubpath`, `message`, `suggestions { target path }` in both compact +and detailed selections; public MCP client exports expose the new types. +The existing shared grep formatter owns presentation for both frontends; a +second frontend renderer would duplicate policy. Render each backend message +once, followed by exact, unverified suggested pairs. Keep copyable pair rows +unwrapped and shell-quote CLI values / JSON-quote MCP values. Do not repeat a +separate diagnostic target/path sentence already contained in the message. +Warnings are independent of coverage, native public messages and preparation. +Preserve every match, target, index, failure, cursor and pending entry. + +No schema fallback, new infrastructure, descriptor/skill changes, backend edits, +merge, release, publication or deployment. No optimization claim or benchmark +needed: this adds response metadata and bounded rendering only. + +## Assumptions, unknowns and dependencies + +- Backend owns diagnosis and alternatives; suggestions are unverified (public + contract and durable backend guide verified read-only). +- Required non-null warning arrays are validated, including empty arrays; no + compatibility guess for an old backend. +- Public providers must supply the required array; tests/fixtures will follow it. +- Deployment state can change; revalidate production support before release. +- Unknowns/product decisions: none for implementation. Live dev authentication + and native advisory availability must be measured during qualification; any + limitation is reported precisely and does not justify fabricated evidence. +- Dependency: current schema on dev; production deployment before client release. + +## Phase 1 — advisories survive every client result shape (IMPLEMENTED; external review blocked) + +Expected outcome: backend warnings remain usable alongside real results and +preparation/failure evidence across CLI and MCP, with no query behavior changes. +Assumptions/unknowns/dependencies: as above; no unresolved product decisions. + +1. Add core warning/suggestion interfaces, required schema array, full selection + and public client exports. Update existing response fixtures for the contract. +2. Add shared advisory rendering after coverage/preparation and before matches. + Keep native message prose safe, suggestions exact and syntax-specific, and + state that callers must retain selector kind and other search controls. +3. Add parser tests in both modes (complete field selection, malformed/missing + fields, empty warnings, no request rewrites); projection deep-copy tests; + renderer tests for zero hits, hits, real failures, multiple warnings/indices, + SHA targets, spaces/quotes/metacharacters/controls and unwrapped pairs. +4. Extend CLI/MCP parity for mixed and pending-only short-wait responses, text + and JSON, preserving controls and every advisory. Add smoke contract coverage. +5. Update `docs/implementation/unified-grep.md` with response/display ownership + and deployment/release prerequisites; add independent changes fragment. + +Acceptance/verification: +- Focused `bun test` for grep service, response, text, CLI/MCP and parity suites; + then full `bun test`, typecheck, formatting, lint, build and public-package + validation using repository commands. +- `GITHITS_ENV=dev bun run smoke:cli` and `smoke:mcp`; authenticated affected + CLI/MCP qualification if credentials are available, otherwise secret-free + auth/registration smoke and explicit limitation. +- Targeted `bun run agent:e2e` against local MCP/dev for grep behavior; inspect + actual tool calls, final answer/confidence, metrics and isolation artifacts. +- Internal pre-flight and one external Claude reviewer per round, until clean + or workflow limit, with valid findings fixed and evidence recorded. + +## Delivery and cleanup + +After clean implementation review, migrate durable knowledge and actual +validation results into implementation docs, then delete this plan in a final +commit within the same PR. Any major deferred work belongs in the backlog. +Push a reviewed draft PR with exact check results and rollout prerequisites. +Report that the backend backlog entry needs its owner's cross-repository +closure; do not edit that worktree. No new refactoring is currently needed. + + +## Actual validation and review record + +- Baseline focused suites: 169 pass. New core regression tests failed four cases + before implementation, proving absent selection/retention/validation. +- `bun test`: 6,679 pass, 0 fail across 258 files (27.05 s). +- Focused eight grep suites: 182 pass, 0 fail, 1,352 expectations after the platform fix. +- `bun run typecheck`, `bun run format:check`, root build and MCP build pass. + `bun run lint` passes with 19 preexisting warnings confined to unchanged + `packages/mcp/src/shared/repository-target.ts`. +- `bun run validate:packages:mcp-publish` passes, including packed consumers + outside workspace aliases. npm publish dry-run skipped because current + `@githits/mcp@0.35.0` is already published; no version bump/publication attempted. +- Default `bun run smoke:cli` and `bun run smoke:mcp` pass with credentials + absent and file auth selected (live cohorts explicitly skipped AUTH_REQUIRED). + Source CLI unauthenticated and MCP registration smoke pass; built equivalents + `smoke:cli:built` and `smoke:mcp:built` pass. Initial built-smoke launch raced + package validator's own build cleanup; rerunning after validation finished + passed. No product failure or cleanup change was needed. +- Default live smoke first stopped on inaccessible keychain under isolated HOME. + Reproduced normal dev `pkg info` success vs scoped-HOME keychain error. Passing + the existing token provider's credential only through child-process memory + resolved authentication, then both full live suites stopped on the existing Go + recovery probe. Its requested `v20.10.0` now succeeds as `v20.10.0+incompatible`. + A nonexistent version failed but offered no alternatives, so it cannot replace + this probe's recovery contract. No unrelated assertion was weakened. +- Built Node dev CLI/MCP affected qualification passed prefix + exact text/JSON, + matching all warnings/hits/statuses/cursor/traversal; exact retained native + `GREP_PATH_NOT_INDEXED`. Local evidence: `/tmp/package-path-live-results/`. +- `GITHITS_ENV=dev CODEX_HOME=/Users/jpl/.codex-eval bun run agent:e2e --agent + codex --surface mcp --server local --workload + eval/agentic/probes/package-path-advisories.md --out + .agent-eval/package-path-advisories` passed harness validation (14.8 s). + URL overrides were removed for coherent dev endpoints. Inspected actual + tool-calls/final/metrics/report: one grep preserved prefix, advisory recognized, + alternatives unexecuted; final inconclusive/high confidence, no recorded + isolation violations. No graded quality claim. +- Internal code review round 1: direction sound; fix unconditional Bash replay + on Windows. Closure: retained quote-structure assertions on every platform, + gated only Bash execution, inspected related new tests; focused six tests pass + with 34 expectations. Internal round 2: clean, no findings. +- Required external Claude Opus 5.5 hit an account usage limit during plan review + before reporting. Dispatch `ctx_bec274ae3118` was fenced and its reviewer + terminal closed. No substitute model, no external clean round claimed. + +## Remaining gates and lifecycle + +Implementation and internal review are complete. The draft must state external +review unavailable and full live smoke blocked by the existing recovery probe. +A clean external review is still required by the repository workflow; retain +this plan for that review. Once clean, delete it in the final commit of this PR, +keeping durable contract, rollout and qualification facts in unified-grep.md. +Production backend deployment and subsequent CLI/MCP releases remain external +prerequisites, not authorized operations. No other implementation is deferred, +and no new refactoring opportunity or infrastructure is needed for this change. diff --git a/eval/agentic/probes/package-path-advisories.md b/eval/agentic/probes/package-path-advisories.md new file mode 100644 index 00000000..0cccd3b1 --- /dev/null +++ b/eval/agentic/probes/package-path-advisories.md @@ -0,0 +1,6 @@ +# Workload: Package-path advisory + +Find the literal `skipLocked` in `npm:drizzle-orm` under the source path prefix +`drizzle-orm/src/pg-core`. Explain the returned result, any coverage limitation, +and any actionable path guidance. Preserve the requested scope; report possible +next actions without executing a different search. diff --git a/packages/core-internal/src/services/grep-service.test.ts b/packages/core-internal/src/services/grep-service.test.ts index 4777873f..d099e15d 100644 --- a/packages/core-internal/src/services/grep-service.test.ts +++ b/packages/core-internal/src/services/grep-service.test.ts @@ -9,6 +9,7 @@ import { GrepGraphQLError, GrepNetworkError, type GrepParams, + type GrepPathWarning, type GrepResult, GrepServiceImpl, MalformedGrepResponseError, @@ -28,6 +29,7 @@ const params: GrepParams = { function result(): GrepResult { return { indexingEstimates: [], + pathWarnings: [], hits: [], targets: [], unavailableTargets: [], @@ -581,3 +583,74 @@ describe("unified grep service", () => { await expect(s.grep(params)).rejects.toThrow(); }); }); + +const pathWarning: GrepPathWarning = { + requestedInputIndices: [1, 0], + target: "npm:x", + path: "packages/x/src", + repositorySubpath: "packages/x", + message: + 'Package paths are relative to the package root. Did you mean target "npm:x" with path "src"?', + suggestions: [ + { target: "npm:x", path: "src" }, + { target: `github:o/r@${"a".repeat(40)}`, path: "packages/x/src" }, + ], +}; + +describe("backend package-path advisories", () => { + for (const detailed of [false, true]) { + it(`selects and preserves every advisory field in ${detailed ? "detailed" : "compact"} mode without changing controls`, async () => { + const page = detailed ? detailedMixed() : mixed(); + page.pathWarnings = [ + pathWarning, + { ...pathWarning, path: "packages/x/test", requestedInputIndices: [2] }, + ]; + const requested = { + ...params, + includeDetailedFields: detailed, + waitTimeoutMs: 0, + }; + const fetcher = mock( + async (_url: Parameters[0], init?: RequestInit) => { + const body = JSON.parse(String(init?.body)); + expect(body.variables).toEqual(requested); + expect(body.query).toMatch( + /pathWarnings\s*\{\s*requestedInputIndices target path repositorySubpath message\s*suggestions\s*\{ target path \}\s*\}/, + ); + return response({ data: { grep: page } }); + }, + ); + expect(await service(fetcher).grep(requested)).toEqual(page); + expect(fetcher).toHaveBeenCalledTimes(1); + }); + + it(`requires and validates the complete advisory contract in ${detailed ? "detailed" : "compact"} mode`, () => { + const page = detailed ? detailedMixed() : mixed(); + expect(parseGrepResult(page, detailed).pathWarnings).toEqual([]); + for (const value of [ + undefined, + null, + {}, + [null], + [{ ...pathWarning, requestedInputIndices: [-1] }], + [{ ...pathWarning, requestedInputIndices: [0.5] }], + [{ ...pathWarning, suggestions: [{ target: "npm:x", path: null }] }], + ]) { + expect(() => + parseGrepResult({ ...page, pathWarnings: value }, detailed), + ).toThrow(MalformedGrepResponseError); + } + for (const field of Object.keys(pathWarning)) { + const warning = { ...pathWarning }; + Reflect.deleteProperty(warning, field); + expect(() => + parseGrepResult({ ...page, pathWarnings: [warning] }, detailed), + ).toThrow(MalformedGrepResponseError); + } + const warning = { ...pathWarning, suggestions: [{ path: "src" }] }; + expect(() => + parseGrepResult({ ...page, pathWarnings: [warning] }, detailed), + ).toThrow(MalformedGrepResponseError); + }); + } +}); diff --git a/packages/core-internal/src/services/grep-service.ts b/packages/core-internal/src/services/grep-service.ts index aded0ed0..a1cc6c0b 100644 --- a/packages/core-internal/src/services/grep-service.ts +++ b/packages/core-internal/src/services/grep-service.ts @@ -160,7 +160,22 @@ export interface GrepUnavailableTarget { progressRef: string | null; suggestedSiteTargets: string[] | null; } +/** An unverified backend-authored alternative; never applied by the client. */ +export interface GrepPathSuggestion { + target: string; + path: string; +} +/** Snapshot-backed path diagnosis, independent of matches and coverage failures. */ +export interface GrepPathWarning { + requestedInputIndices: number[]; + target: string; + path: string; + repositorySubpath: string; + message: string; + suggestions: GrepPathSuggestion[]; +} export interface GrepResult { + pathWarnings: GrepPathWarning[]; indexingEstimates?: DiscoveryIndexingEstimate[]; hits: GrepHit[]; targets: GrepTargetStatus[]; @@ -276,6 +291,18 @@ function resultSchema(detailed: boolean): z.ZodType { sourceMatchEndByte: selected(nonnegativeInt), }; return z.object({ + pathWarnings: z.array( + z.object({ + requestedInputIndices: z.array(nonnegativeInt), + target: z.string(), + path: z.string(), + repositorySubpath: z.string(), + message: z.string(), + suggestions: z.array( + z.object({ target: z.string(), path: z.string() }), + ), + }), + ), indexingEstimates: indexingEstimatesSchema, hits: z.array( z.discriminatedUnion("__typename", [ @@ -398,6 +425,10 @@ const GRAPHQL_QUERY = `query Grep( contextLinesAfter: $contextLinesAfter, maxMatches: $maxMatches, cursor: $cursor, waitTimeoutMs: $waitTimeoutMs) { traversal nextCursor totalMatches + pathWarnings { + requestedInputIndices target path repositorySubpath message + suggestions { target path } + } ${INDEXING_ESTIMATES_SELECTION} hits { __typename diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index be1d3ff2..9ca2862d 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -36,6 +36,8 @@ export type { GrepLineSlice, GrepParams, GrepPathSelector, + GrepPathSuggestion, + GrepPathWarning, GrepReadAction, GrepReadiness, GrepRepositoryHit, diff --git a/packages/mcp/src/shared/fixtures/grep-text/mixed-100.json b/packages/mcp/src/shared/fixtures/grep-text/mixed-100.json index 6062eb19..73b79316 100644 --- a/packages/mcp/src/shared/fixtures/grep-text/mixed-100.json +++ b/packages/mcp/src/shared/fixtures/grep-text/mixed-100.json @@ -1,4 +1,5 @@ { + "pathWarnings": [], "traversal": "RESUMABLE_LIMIT", "nextCursor": "TUdSUALkFPOMGmQmwMT9g3skeFq0I3qsSqWvFi2a7YQGlMZAKwACAABAAAEBZAAAAAAABAAAAAAAAAAAAAEgfKalBjGIr_089cv89Ti326x0GkmlpkM2twwG6FwuJwbjYzYyAAEAAAAAAgFkAAAAAAAEAIAAAAAAAAABAi2I6eMfCYe6LjvxHR-aNkEsz2Oumeyt0hFne_LMeIQTdmbjgUmp1C3fffsRaoTNAtvgGoT0aFOJQRyVtcNRvIUfNR0AAAAAADIAAAAAAAA", "totalMatches": 100, diff --git a/packages/mcp/src/shared/fixtures/grep-text/repository-100.json b/packages/mcp/src/shared/fixtures/grep-text/repository-100.json index 795219de..43469bd3 100644 --- a/packages/mcp/src/shared/fixtures/grep-text/repository-100.json +++ b/packages/mcp/src/shared/fixtures/grep-text/repository-100.json @@ -1,4 +1,5 @@ { + "pathWarnings": [], "traversal": "RESUMABLE_LIMIT", "nextCursor": "TUdSUAKW_xQN7o9Xb708KubQPUzUWIQwl9Drl5Y-eCSTrQjkPgABAABAAAEBZAAAAAAABAAAAAAAAAAAAAEgfKalBjGIr_089cv89Ti326x0GkmlpkM2twwG6FwuJwbjYzYAAAAQAD8egBfC0rKREwAAAE8AAAAAAA", "totalMatches": 100, diff --git a/packages/mcp/src/shared/grep-path-warnings.test.ts b/packages/mcp/src/shared/grep-path-warnings.test.ts new file mode 100644 index 00000000..20114bb5 --- /dev/null +++ b/packages/mcp/src/shared/grep-path-warnings.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "bun:test"; +import { type GrepPathWarning, parseGrepResult } from "@githits/core-internal"; +import mixed from "./fixtures/grep-text/mixed-100.json"; +import { projectGrepResult } from "./grep-response.js"; +import { formatGrepText } from "./grep-text.js"; + +const warning: GrepPathWarning = { + requestedInputIndices: [2, 0], + target: "npm:x", + path: "packages/x/src", + repositorySubpath: "packages/x", + message: + 'Package paths are relative to the package root. Did you mean target "npm:x" with path "src"?', + suggestions: [ + { target: "npm:x", path: "src" }, + { target: `github:o/r@${"a".repeat(40)}`, path: "packages/x/src" }, + ], +}; + +function page() { + return { + ...parseGrepResult(mixed), + pathWarnings: structuredClone([ + warning, + { ...warning, path: "packages/x/test", requestedInputIndices: [1] }, + ]), + }; +} + +describe("grep path advisories", () => { + it("preserves every field and ordered input index without aliasing provider data", () => { + const input = page(); + const before = structuredClone(input); + const projected = projectGrepResult(input); + expect(projected).toEqual(before); + projected.pathWarnings[0]!.requestedInputIndices.push(5); + projected.pathWarnings[0]!.suggestions[0]!.path = "changed"; + projected.pathWarnings[1]!.message = "changed"; + expect(input).toEqual(before); + }); + + it("keeps advisories, real matches, coverage failures, public messages and cursors together", () => { + const input = page(); + Object.assign(input.targets[0]!, { + traversal: "FAILED", + errorCode: "GREP_PATH_NOT_INDEXED", + publicMessage: "The requested exact path is not indexed.", + retryable: false, + }); + input.traversal = "NON_RESUMABLE_PARTIAL"; + const before = structuredClone(input); + const text = formatGrepText(input); + expect(text).toContain("Found 100 matches"); + expect(text).toContain("GREP_PATH_NOT_INDEXED"); + expect(text).toContain("The requested exact path is not indexed."); + expect(text).toContain("Path advisories:"); + expect(text.split("Package paths are relative")).toHaveLength(3); + expect(text).toContain( + `target='github:o/r@${"a".repeat(40)}' path='packages/x/src'`, + ); + expect(text).toContain("--cursor"); + expect(text).toContain("100"); + expect(input).toEqual(before); + }); + + for (const syntax of ["cli", "mcp"] as const) { + it(`shows a successful zero-hit advisory in ${syntax} without inventing a failure`, () => { + const input = page(); + Object.assign(input, { + hits: [], + totalMatches: 0, + traversal: "COMPLETE", + nextCursor: null, + }); + for (const target of input.targets) + Object.assign(target, { traversal: "COMPLETE", readiness: "CURRENT" }); + const text = formatGrepText(input, { syntax }); + expect(text).toStartWith("No matches found."); + expect(text).toContain("Path advisories:"); + expect(text).toContain("Unverified alternatives"); + expect(text).not.toContain("search failed"); + expect(text).not.toContain("still being prepared"); + expect(text).not.toContain("Retry"); + }); + + it(`quotes exact ${syntax} pairs with spaces, quotes, metacharacters and controls without wrapping`, async () => { + const input = page(); + const target = `github:o/weird repo'";$()\\name@${"b".repeat(40)}`; + const path = "src/a 'quoted' \"file\";$HOME`echo nope`\\é\n\u001b[31m.ts"; + input.pathWarnings = [ + { + ...warning, + message: "Public\u001b[31m message", + suggestions: [{ target, path }], + }, + ]; + const text = formatGrepText(input, { + syntax, + width: 35, + useColors: false, + }); + expect(text).toContain("Public\\u001b[31m message"); + expect(text).not.toContain("\u001b"); + const row = text.split("\n").find((line) => line.startsWith(" "))!; + expect(row.length).toBeGreaterThan(35); + if (syntax === "mcp") { + expect(JSON.parse(row.trim())).toEqual({ target, path }); + } else { + expect(row).toStartWith( + " target='github:o/weird repo'\"'\"'\";$()\\name@", + ); + expect(row).toContain(" path=$'src/"); + // Bash is not a Windows prerequisite; quote structure is asserted on every host. + if (process.platform === "win32") return; + // Execute assignments only: quoted substitutions must remain literal data. + const child = Bun.spawn( + ["bash", "-c", `${row.trim()}; printf '%s\\0%s' "$target" "$path"`], + { stdout: "pipe", stderr: "pipe" }, + ); + expect(await new Response(child.stdout).text()).toBe( + `${target}\0${path}`, + ); + expect(await child.exited).toBe(0); + } + }); + } +}); diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index f5aa1a55..eaab980e 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -56,6 +56,7 @@ const hit: GrepHit = { function result(overrides: Partial = {}): GrepResult { return { indexingEstimates: [], + pathWarnings: [], hits: [hit], targets: [target], unavailableTargets: [], diff --git a/packages/mcp/src/shared/grep-response.ts b/packages/mcp/src/shared/grep-response.ts index cc421143..71d037c0 100644 --- a/packages/mcp/src/shared/grep-response.ts +++ b/packages/mcp/src/shared/grep-response.ts @@ -1,6 +1,6 @@ import { type GrepResult, parseGrepResult } from "@githits/core-internal"; -/** Preserve the selected wire payload without aliasing caller-owned arrays. */ +/** Preserve selected wire fields, including path advisories, without aliasing provider data. */ export function projectGrepResult(result: GrepResult): GrepResult { // The wire contract requires the array; injected providers may predate it. const projected = parseGrepResult({ diff --git a/packages/mcp/src/shared/grep-text-rendering.test.ts b/packages/mcp/src/shared/grep-text-rendering.test.ts index 5bfd149b..7fb2f5f7 100644 --- a/packages/mcp/src/shared/grep-text-rendering.test.ts +++ b/packages/mcp/src/shared/grep-text-rendering.test.ts @@ -68,6 +68,7 @@ function page( ): GrepResult { return { indexingEstimates: [], + pathWarnings: [], hits, targets: [scope()], totalMatches: hits.length, diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index fb38de3a..265384bb 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -198,6 +198,24 @@ export function formatGrepText( ? `${result.hits.length ? "Some requested content could not be searched; the" : "The"} service did not explain why or whether retrying would help.` : "The search did not finish, and no continuation cursor was returned. The service did not explain why the full search stopped or whether retrying would help.", ); + if (result.pathWarnings.length) { + lines.push("", "Path advisories:"); + for (const warning of result.pathWarnings) { + prose(` - ${warning.message}`); + if (warning.suggestions.length) + prose( + " Unverified alternatives (keep selector kind and other controls):", + ); + for (const suggestion of warning.suggestions) { + // Pair rows are data, not a guessed grep invocation: the warning has no selector kind. + lines.push( + options.syntax === "mcp" + ? ` ${JSON.stringify({ target: suggestion.target, path: suggestion.path })}` + : ` target=${shellQuoteExact(suggestion.target)} path=${shellQuoteExact(suggestion.path)}`, + ); + } + } + } for (const [index, group] of groups.entries()) { const first = group.first; const target = first.read.target; diff --git a/packages/mcp/src/shared/indexing-estimates.test.ts b/packages/mcp/src/shared/indexing-estimates.test.ts index a3179c55..74e30b0a 100644 --- a/packages/mcp/src/shared/indexing-estimates.test.ts +++ b/packages/mcp/src/shared/indexing-estimates.test.ts @@ -32,6 +32,7 @@ const documentation: DiscoveryIndexingEstimate = { }; function pending(overrides: Partial = {}): GrepResult { return { + pathWarnings: [], hits: [], targets: [], totalMatches: 0, diff --git a/packages/mcp/src/smoke-test.ts b/packages/mcp/src/smoke-test.ts index ebc6b898..65923e82 100644 --- a/packages/mcp/src/smoke-test.ts +++ b/packages/mcp/src/smoke-test.ts @@ -282,6 +282,10 @@ function assertGrepResult( context: string, ): Record { assertRecord(value, context); + assert( + Array.isArray(value.pathWarnings), + `${context}: pathWarnings must be an array`, + ); assert(Array.isArray(value.hits), `${context}: hits must be an array`); assert(Array.isArray(value.targets), `${context}: targets must be an array`); assert( diff --git a/packages/mcp/src/tools/grep.test.ts b/packages/mcp/src/tools/grep.test.ts index da917049..70a0034d 100644 --- a/packages/mcp/src/tools/grep.test.ts +++ b/packages/mcp/src/tools/grep.test.ts @@ -16,6 +16,7 @@ import { CODE_GREP_GUARDRAIL } from "./guardrails.js"; const emptyPage: GrepResult = { indexingEstimates: [], + pathWarnings: [], hits: [], targets: [], unavailableTargets: [], diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index d7c0807c..39b49358 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -2293,6 +2293,10 @@ async function runLiveSmoke(env: Record): Promise { "unified grep source JSON", ); assertRecord(grepPage, "unified grep source JSON"); + assert( + Array.isArray(grepPage.pathWarnings), + "unified grep must retain pathWarnings", + ); assert( Array.isArray(grepPage.hits) && grepPage.hits.length === 1 && diff --git a/src/commands/grep.test.ts b/src/commands/grep.test.ts index 6b141d12..507654cb 100644 --- a/src/commands/grep.test.ts +++ b/src/commands/grep.test.ts @@ -112,7 +112,9 @@ describe("unified grep CLI", () => { includeDetailedFields: true, waitTimeoutMs: 30000, }); - expect(log.mock.calls[0]?.[0]).toBe(JSON.stringify(defaultGrepResult)); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual( + defaultGrepResult, + ); } finally { log.mockRestore(); } diff --git a/src/mcp-public-surface.test.ts b/src/mcp-public-surface.test.ts index 6bf4bc2c..349c6b5e 100644 --- a/src/mcp-public-surface.test.ts +++ b/src/mcp-public-surface.test.ts @@ -521,6 +521,7 @@ describe("public MCP package surface", () => { it("routes unified grep through the remote server API", async () => { const grep = mock(() => Promise.resolve({ + pathWarnings: [], hits: [], targets: [], unavailableTargets: [], diff --git a/src/services/test-helpers.ts b/src/services/test-helpers.ts index 72c6fb98..69f93770 100644 --- a/src/services/test-helpers.ts +++ b/src/services/test-helpers.ts @@ -1111,6 +1111,7 @@ export function createMockListService( export const defaultGrepResult: GrepResult = { indexingEstimates: [], + pathWarnings: [], hits: [], targets: [], unavailableTargets: [], diff --git a/src/tools/grep-parity.test.ts b/src/tools/grep-parity.test.ts index 6f105bb2..672f451a 100644 --- a/src/tools/grep-parity.test.ts +++ b/src/tools/grep-parity.test.ts @@ -161,3 +161,192 @@ describe("unified grep CLI/MCP parity", () => { } }); }); + +describe("package-path advisory CLI/MCP parity", () => { + for (const shape of ["zero-hit", "mixed-pending", "pending-only"] as const) { + it(`preserves ${shape} short-wait pages in text and JSON without rewriting requests`, async () => { + const path = "packages/x/src 'quoted';$HOME"; + const warning = { + requestedInputIndices: [0, 2], + target: "npm:x", + path, + repositorySubpath: "packages/x", + message: + 'Package paths are relative to the package root. Did you mean target "npm:x" with path "src"?', + suggestions: [ + { target: "npm:x", path: "src 'quoted';$HOME" }, + { target: `github:o/r@${"c".repeat(40)}`, path }, + ], + }; + const result: GrepResult = { + ...defaultGrepResult, + pathWarnings: + shape === "pending-only" + ? [] + : [ + warning, + { + ...warning, + requestedInputIndices: [1], + path: "packages/x/test", + }, + ], + traversal: shape === "zero-hit" ? "COMPLETE" : "NON_RESUMABLE_PARTIAL", + unavailableTargets: + shape === "zero-hit" + ? [] + : [ + { + inputIndex: 1, + target: "npm:pending", + reason: "repository_indexing", + retryable: true, + progressRef: "opaque-progress", + suggestedSiteTargets: null, + }, + ], + }; + if (shape !== "pending-only") { + result.targets = [ + { + targetIndex: 0, + requestedInputIndices: [0, 2], + kind: "REPOSITORY", + target: "npm:x", + traversal: "COMPLETE", + readiness: "CURRENT", + errorCode: null, + retryable: false, + publicMessage: null, + requestedRef: "v1", + commitSha: "c".repeat(40), + corpus: "ALL", + filesScanned: 0, + filesInScope: 0, + binaryFilesSkipped: 0, + filesTooLargeSkipped: 0, + fileIssues: [], + fileIssuesOmitted: 0, + repoUrl: "https://github.com/o/r", + canonicalSite: null, + }, + ]; + } + if (shape === "mixed-pending") { + result.totalMatches = 1; + result.hits = [ + { + __typename: "GrepRepositoryHit", + targetIndex: 0, + filePath: "src/a.ts", + line: 1, + lineSlice: { + content: "router", + startByte: 0, + endByte: 6, + originalLineBytes: 6, + }, + contextBeforeSlices: [], + contextAfterSlices: [], + matchStartByte: 0, + matchEndByte: 6, + contentSafety: { filtered: false }, + read: { + target: `github:o/r@${"c".repeat(40)}`, + path: "packages/x/src/a.ts", + startLine: 1, + endLine: 1, + }, + }, + ]; + } + const before = structuredClone(result); + const grep = mock((_params: GrepParams) => Promise.resolve(result)); + const grepService = createMockGrepService({ grep }); + const write = spyOn(process.stdout, "write").mockImplementation( + () => true, + ); + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + const runCli = async (json: boolean): Promise => { + const program = new Command("githits"); + registerGrepCommand(program, async () => ({ + grepService, + hasValidToken: true, + mcpUrl: "https://example.test", + createSpinner: () => ({ stop: () => {} }), + })); + await program.parseAsync( + [ + "grep", + "router", + "npm:x", + "npm:pending", + "npm:x", + "--path-prefix", + path, + "--wait", + "0", + ...(json ? ["--json"] : []), + ], + { from: "user" }, + ); + }; + const tool = createParityMcpTool("grep", { grepService }); + const args = { + targets: ["npm:x", "npm:pending", "npm:x"].map((target) => ({ + target, + path_selectors: [{ kind: "prefix", value: path }], + })), + pattern: "router", + wait_timeout_ms: 0, + }; + await runCli(false); + const cli = write.mock.calls.map(([chunk]) => String(chunk)).join(""); + const mcp = await tool.handler(args); + expect(mcp.isError).toBeUndefined(); + const text = String(mcp.content[0]?.text); + if (shape === "pending-only") { + expect(cli).toContain("No matches available yet"); + expect(text).toContain("No matches available yet"); + expect(text).not.toContain("Path advisories:"); + expect(cli).not.toContain("Path advisories:"); + } else { + for (const output of [cli, text]) { + expect(output).toContain("Path advisories:"); + expect(output.split("Package paths are relative")).toHaveLength(3); + expect(output).toContain("Unverified alternatives"); + if (shape === "mixed-pending") { + expect(output).toContain("1: router"); + expect(output).toContain("Preparing:"); + expect(output).toContain("npm:pending"); + } else expect(output).toStartWith("No matches found."); + } + expect(text).toContain(JSON.stringify(warning.suggestions[1])); + expect(cli).toContain(`target='github:o/r@${"c".repeat(40)}'`); + } + expect(grep.mock.calls[0]?.[0]).toEqual(grep.mock.calls[1]?.[0]); + await runCli(true); + const json = await tool.handler({ ...args, format: "json" }); + expect(json.isError).toBeUndefined(); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual(before); + expect(JSON.parse(String(json.content[0]?.text))).toEqual(before); + expect(grep).toHaveBeenCalledTimes(4); + expect(grep.mock.calls[2]?.[0]).toEqual(grep.mock.calls[3]?.[0]); + expect(grep.mock.calls[3]?.[0]).toMatchObject({ + waitTimeoutMs: 0, + targets: args.targets.map(({ target }) => ({ + target, + corpus: "ALL", + pathSelectors: [{ kind: "PREFIX", value: path }], + allowUnscoped: true, + })), + }); + expect(result).toEqual(before); + } finally { + write.mockRestore(); + log.mockRestore(); + } + }); + } +}); From b7866dfb5c3f8d62cdedb33d6524168e9e2238c3 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Sat, 24 Oct 2026 09:40:42 +0300 Subject: [PATCH 2/4] fix: escape terminal controls in MCP advisory pairs Reuse the shared control escaper after JSON serialization so C1 controls cannot remain literal in advisory rows. Extend the existing quote test to prove terminal safety and exact JSON and shell round trips. --- docs/plans/package-path-advisories.md | 13 ++++++++++++- packages/mcp/src/shared/grep-path-warnings.test.ts | 4 +++- packages/mcp/src/shared/grep-text.ts | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/plans/package-path-advisories.md b/docs/plans/package-path-advisories.md index f8a20fb2..c48cf015 100644 --- a/docs/plans/package-path-advisories.md +++ b/docs/plans/package-path-advisories.md @@ -100,7 +100,7 @@ closure; do not edit that worktree. No new refactoring is currently needed. - Baseline focused suites: 169 pass. New core regression tests failed four cases before implementation, proving absent selection/retention/validation. - `bun test`: 6,679 pass, 0 fail across 258 files (27.05 s). -- Focused eight grep suites: 182 pass, 0 fail, 1,352 expectations after the platform fix. +- Focused eight grep suites: 182 pass, 0 fail, 1,354 expectations after platform/control quoting fixes. - `bun run typecheck`, `bun run format:check`, root build and MCP build pass. `bun run lint` passes with 19 preexisting warnings confined to unchanged `packages/mcp/src/shared/repository-target.ts`. @@ -149,3 +149,14 @@ keeping durable contract, rollout and qualification facts in unified-grep.md. Production backend deployment and subsequent CLI/MCP releases remain external prerequisites, not authorized operations. No other implementation is deferred, and no new refactoring opportunity or infrastructure is needed for this change. + +Final self-check found that JSON.stringify leaves C1 controls literal even +though selector normalization permits them. Added C1 to the pair round-trip +regression (failed in MCP mode before the fix), and reused the formatter's +existing escapeControls for JSON pair rows; no new boundary or helper. CLI +already uses shellQuoteExact for those controls. Checked all new advisory rows: +prose uses existing escaping, CLI rows use shell quoting, MCP rows now escape +C0/C1 while JSON.parse retains the exact value. Initial PR CI at e7b880d passed +Linux/Windows tests, Build & Checks, MCP package validation and Bun/Node 20–26 +compatibility. Follow-up quoting fix: final internal review clean, focused suites pass, +public package validation (including fresh root/MCP builds) passes. diff --git a/packages/mcp/src/shared/grep-path-warnings.test.ts b/packages/mcp/src/shared/grep-path-warnings.test.ts index 20114bb5..80279f34 100644 --- a/packages/mcp/src/shared/grep-path-warnings.test.ts +++ b/packages/mcp/src/shared/grep-path-warnings.test.ts @@ -86,7 +86,8 @@ describe("grep path advisories", () => { it(`quotes exact ${syntax} pairs with spaces, quotes, metacharacters and controls without wrapping`, async () => { const input = page(); const target = `github:o/weird repo'";$()\\name@${"b".repeat(40)}`; - const path = "src/a 'quoted' \"file\";$HOME`echo nope`\\é\n\u001b[31m.ts"; + const path = + "src/a 'quoted' \"file\";$HOME`echo nope`\\é\n\u001b[31m\u009b[31m.ts"; input.pathWarnings = [ { ...warning, @@ -101,6 +102,7 @@ describe("grep path advisories", () => { }); expect(text).toContain("Public\\u001b[31m message"); expect(text).not.toContain("\u001b"); + expect(text).not.toContain("\u009b"); const row = text.split("\n").find((line) => line.startsWith(" "))!; expect(row.length).toBeGreaterThan(35); if (syntax === "mcp") { diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index 265384bb..6c402be6 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -210,7 +210,7 @@ export function formatGrepText( // Pair rows are data, not a guessed grep invocation: the warning has no selector kind. lines.push( options.syntax === "mcp" - ? ` ${JSON.stringify({ target: suggestion.target, path: suggestion.path })}` + ? ` ${escapeControls(JSON.stringify({ target: suggestion.target, path: suggestion.path }))}` : ` target=${shellQuoteExact(suggestion.target)} path=${shellQuoteExact(suggestion.path)}`, ); } From 8e5ab068e34284dcade5ef7c672660ca8ff0aed8 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Sat, 24 Oct 2026 13:45:27 +0300 Subject: [PATCH 3/4] fix: keep package-path warnings compact in text and JSON Summarize backend evidence as one warning per distinct correction instead of dumping explanations and alternate targets. Share wording across CLI and MCP text/JSON, preserve native results and scope, and cover control-safe warning parity. --- changes/package-path-advisories.added.md | 2 +- docs/implementation/unified-grep.md | 57 ++-- docs/plans/package-path-advisories.md | 276 ++++++++---------- .../mcp/src/shared/grep-path-warnings.test.ts | 137 +++++---- packages/mcp/src/shared/grep-response.test.ts | 9 +- packages/mcp/src/shared/grep-response.ts | 41 ++- packages/mcp/src/shared/grep-text.ts | 22 +- packages/mcp/src/smoke-test.ts | 5 +- packages/mcp/src/tools/grep.ts | 8 +- scripts/cli-smoke.ts | 3 +- src/commands/grep.ts | 4 +- src/tools/grep-parity.test.ts | 31 +- 12 files changed, 314 insertions(+), 281 deletions(-) diff --git a/changes/package-path-advisories.added.md b/changes/package-path-advisories.added.md index 55d7820d..24fcba0c 100644 --- a/changes/package-path-advisories.added.md +++ b/changes/package-path-advisories.added.md @@ -3,4 +3,4 @@ "@githits/mcp": minor --- -- **Package-path advisories** - Unified grep preserves backend path warnings in CLI/MCP JSON and displays unverified, safely quoted target/path alternatives beside matches and pending results. Requires a backend deployment exposing `GrepResult.pathWarnings` before client release; injected grep providers must return the required warning array. +- **Package-path advisories** - Unified grep shows one compact warning per distinct package-path correction in CLI/MCP text and JSON, preserving results, failures and pending targets. Requires a backend exposing `GrepResult.pathWarnings`; injected grep providers must supply the warning array. diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index e23b798c..6b535e44 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -400,28 +400,30 @@ compact and detailed modes. Each warning preserves `requestedInputIndices`, An empty list means no supported diagnosis was returned; zero matches alone are not path evidence. Missing or malformed fields are protocol errors. -The shared projection reparses and deep-copies this metadata along with all -hits, native target statuses/public messages, omissions, preparation estimates, -traversal and cursors. The service returns short-wait pages through the same -parser. Pending-only results without authoritative evidence have an empty list; -mixed partial results retain proven warnings beside pending targets. The client -does not synthesize or suppress warnings based on hit counts or pending state. -CLI `--json` and MCP `format: "json"` preserve every warning field and backend -input attribution. Public `@githits/mcp/client` exports `GrepPathWarning` and -`GrepPathSuggestion`; injected `GrepService` providers must supply the array. - -`formatGrepText` displays each backend message once under `Path advisories`, -after preparation/coverage and before matches. Terminal controls in prose are -escaped and prose wraps to the caller's width. Suggested pairs are explicitly -unverified: they do not claim the path exists or will produce hits. Copyable -pair rows stay unwrapped. CLI uses shell-quoted `target=... path=...` values; -MCP uses a JSON object with `target` and `path`. These are advisory values, -not complete commands or MCP grep arguments: callers retain the original -selector kind (`--path` / `--path-prefix`, or `path_selectors[].kind`) and other -controls when choosing an alternative. The warning has no selector-kind field, -so the renderer cannot safely invent one. Full commit SHAs remain intact. -No additional diagnostic target/path sentence repeats the message's own pairs. -Native failures, public messages and real results remain visible independently. +The shared projection validates and deep-copies the page, retaining hits, native +statuses/public messages, omissions, preparation estimates, traversal and cursors. +It replaces advisory objects with `pathWarnings: string[]` for CLI `--json` and +MCP `format: "json"`. Text uses the same compact sentences, one physical line +per distinct warning, after coverage/preparation and before matches: + +```text +Warning: Path is prefixed with package path "packages/example". Did you mean "src"? +``` + +The package root and correction come directly from backend fields: the correction +is the suggestion whose target equals the warning's target. There is no local +prefix-stripping heuristic. When no same-target suggestion exists, the sentence +stops after the package path. Values are JSON-quoted; text escapes terminal +controls without wrapping warning lines. Identical sentences are emitted once. +Neither output format dumps the verbose backend message or alternative pairs. +A question suggests a possible correction; it does not assert that a path exists. + +The core `GrepService` contract retains complete backend metadata, including the +public `GrepPathWarning`/`GrepPathSuggestion` types exported by +`@githits/mcp/client`. Injected providers must supply the required array. Short +waits use the same parser: pending-only pages without evidence have no warning, +and mixed pages keep proven warnings beside pending targets. No warning changes +native failures, real results or search behavior. ### Deployment and release prerequisite @@ -430,12 +432,15 @@ merged at `b26172c5fc97287256e55d22bc64aa384297184c`. On 2026-10-24, read-only GraphQL validation of a skipped `grep` selection accepted every warning field at `https://oss-dev.githits.dev/api/graphql`. Production at `https://oss.githits.dev/api/graphql` rejected `GrepResult.pathWarnings`. +A recheck at **2026-10-24 10:25 UTC** accepted every field on production. The +original package/prefix request returned one advisory with zero hits and +`COMPLETE`; the backend deployment prerequisite is now satisfied. Introspection is disabled at both endpoints; the validation query uses `@skip(if: true)` to check selections without executing grep. Before releasing a client that selects this field, verify the configured -backend accepts the full selection below. Production needs the backend change -deployed first; merging its PR is not proof of deployment. No schema fallback +backend accepts the full selection below. Backend schema support must be verified on the configured endpoint; merging +its PR is not proof of deployment. No schema fallback or downgraded query is provided: an older endpoint rejects the entire grep query. Native advisory generation additionally requires the backend's released aigrep 0.27.3/API56 path-evidence integration. Schema support alone does not @@ -462,8 +467,8 @@ to close after this client work ships; it is not edited across worktrees. ### Client qualification (2026-10-24) - Built Node CLI and local stdio MCP against dev: prefix and exact selectors - both retained one advisory, both suggested pairs and the full repository SHA - in text and JSON. Prefix returned zero hits with `COMPLETE`; exact returned + both retained one advisory and complete backend data before the compact UX + revision. The current frontend emits only compact warning strings. Prefix returned zero hits with `COMPLETE`; exact returned zero hits with `NON_RESUMABLE_PARTIAL` and its original `GREP_PATH_NOT_INDEXED` failure. CLI/MCP warnings, hits, target statuses, traversal and cursors matched. Secret-free local evidence is retained in diff --git a/docs/plans/package-path-advisories.md b/docs/plans/package-path-advisories.md index c48cf015..382afc53 100644 --- a/docs/plans/package-path-advisories.md +++ b/docs/plans/package-path-advisories.md @@ -1,162 +1,124 @@ # Unified grep package-path advisories -Status: IMPLEMENTED; internal implementation review clean after one test fix. -External plan/implementation clean review unavailable: required Claude Opus 5.5 hit account usage limit before a -settled report; dispatch fenced, no external clean round claimed. - -## Outcome and verified evidence - -CLI and MCP agents receive backend-authored package-path advisories in shared -text and lossless JSON, including empty and partial/pending pages. No client -heuristics, alternative queries, request rewrites, retries or scope changes. - -Current main is `0a97eb12a15dca43b8c802256597db42a96c8a35` (2026-10-24). -Core `grep-service.ts` neither selects nor validates `pathWarnings`; -`projectGrepResult` reparses using that allowlist; both frontends already use -that projection and `formatGrepText`. Short waits have no separate client page -conversion: the service returns the validated backend result directly. - -Backend PR #3550 merged at `b26172c5fc97287256e55d22bc64aa384297184c`. -Read-only skipped-selection GraphQL validation on 2026-10-24 accepted all fields -at `https://oss-dev.githits.dev/api/graphql` and rejected `pathWarnings` at -`https://oss.githits.dev/api/graphql`. Introspection is disabled at both. -Production backend availability is a release prerequisite, not assumed. - -## Scope, ownership and target architecture - -Core owns the typed wire contract and schema validation. Add required -`pathWarnings` with `requestedInputIndices`, `target`, `path`, -`repositorySubpath`, `message`, `suggestions { target path }` in both compact -and detailed selections; public MCP client exports expose the new types. -The existing shared grep formatter owns presentation for both frontends; a -second frontend renderer would duplicate policy. Render each backend message -once, followed by exact, unverified suggested pairs. Keep copyable pair rows -unwrapped and shell-quote CLI values / JSON-quote MCP values. Do not repeat a -separate diagnostic target/path sentence already contained in the message. -Warnings are independent of coverage, native public messages and preparation. -Preserve every match, target, index, failure, cursor and pending entry. - -No schema fallback, new infrastructure, descriptor/skill changes, backend edits, -merge, release, publication or deployment. No optimization claim or benchmark -needed: this adds response metadata and bounded rendering only. - -## Assumptions, unknowns and dependencies - -- Backend owns diagnosis and alternatives; suggestions are unverified (public - contract and durable backend guide verified read-only). -- Required non-null warning arrays are validated, including empty arrays; no - compatibility guess for an old backend. -- Public providers must supply the required array; tests/fixtures will follow it. -- Deployment state can change; revalidate production support before release. -- Unknowns/product decisions: none for implementation. Live dev authentication - and native advisory availability must be measured during qualification; any - limitation is reported precisely and does not justify fabricated evidence. -- Dependency: current schema on dev; production deployment before client release. - -## Phase 1 — advisories survive every client result shape (IMPLEMENTED; external review blocked) - -Expected outcome: backend warnings remain usable alongside real results and -preparation/failure evidence across CLI and MCP, with no query behavior changes. -Assumptions/unknowns/dependencies: as above; no unresolved product decisions. - -1. Add core warning/suggestion interfaces, required schema array, full selection - and public client exports. Update existing response fixtures for the contract. -2. Add shared advisory rendering after coverage/preparation and before matches. - Keep native message prose safe, suggestions exact and syntax-specific, and - state that callers must retain selector kind and other search controls. -3. Add parser tests in both modes (complete field selection, malformed/missing - fields, empty warnings, no request rewrites); projection deep-copy tests; - renderer tests for zero hits, hits, real failures, multiple warnings/indices, - SHA targets, spaces/quotes/metacharacters/controls and unwrapped pairs. -4. Extend CLI/MCP parity for mixed and pending-only short-wait responses, text - and JSON, preserving controls and every advisory. Add smoke contract coverage. -5. Update `docs/implementation/unified-grep.md` with response/display ownership - and deployment/release prerequisites; add independent changes fragment. - -Acceptance/verification: -- Focused `bun test` for grep service, response, text, CLI/MCP and parity suites; - then full `bun test`, typecheck, formatting, lint, build and public-package - validation using repository commands. -- `GITHITS_ENV=dev bun run smoke:cli` and `smoke:mcp`; authenticated affected - CLI/MCP qualification if credentials are available, otherwise secret-free - auth/registration smoke and explicit limitation. -- Targeted `bun run agent:e2e` against local MCP/dev for grep behavior; inspect - actual tool calls, final answer/confidence, metrics and isolation artifacts. -- Internal pre-flight and one external Claude reviewer per round, until clean - or workflow limit, with valid findings fixed and evidence recorded. +Status: compact UX revision implemented and internally reviewed clean. Local +validation passed; external review unavailable (Claude quota). + +## Outcome and scope + +CLI/MCP agents receive one compact warning per distinct backend path correction +in both text and JSON. Preserve native results, failures, preparation, cursors +and exact query scope. No local diagnosis, path probing, request rewrites or +retries. No infrastructure, new options, merge, release, publish or deploy. + +User feedback supersedes the original verbose text and advisory-object JSON: + +```text +Warning: Path is prefixed with package path "drizzle-orm". Did you mean "src/pg-core"? +``` + +## Verified state and dependencies + +- Based on main `0a97eb12a15dca43b8c802256597db42a96c8a35`; draft PR #516 + includes initial adoption commits `e7b880d` and `b7866df`. +- Core selects/validates all `pathWarnings` fields. Shared projection reparses; + CLI/MCP already share the text formatter. Short waits return the backend page + directly, without a separate conversion that could drop warnings. +- Backend #3550 merged at `b26172c5fc97287256e55d22bc64aa384297184c`. + Production initially rejected `pathWarnings`. Recheck at 2026-10-24 10:25 UTC + accepted all fields; the original prefix request returned a warning with zero + hits and COMPLETE. The backend deployment prerequisite is satisfied. +- Client releases are still required. Hosted clients additionally require a + published MCP package upgrade/deployment by remote-mcp, separately authorized. + +## Ownership and implementation + +Core owns backend metadata and its validation. The shared response projection +owns frontend JSON; its `GrepResponse` retains result data but changes +`pathWarnings` to `string[]`. The existing shared text formatter continues to +accept core results and uses the same compact summary function. CLI/MCP both +call projection validation before formatting their raw service result. + +The summary uses `repositorySubpath` and the backend suggestion for the same +`target`; it never strips a prefix or guesses a correction. If that suggestion +is absent, omit the question. Quote path values with JSON quoting; text escapes +controls and leaves each warning on one physical line. Deduplicate identical +sentences. Neither frontend format dumps backend messages or alternative pairs. +The public core service types retain complete evidence for service consumers. + +Assumptions: backend owns diagnosis; suggestions are possibilities, not verified +paths. Both are established by the backend contract and read-only guide. +Unknowns/product decisions: none. No performance claim or runtime optimization. + +## Phase 1 — compact advisories survive every result shape (IMPLEMENTED) + +Expected outcome: one useful warning per correction across text/JSON and CLI/MCP, +including zero-hit, failure, mixed pending and pending-only results. +Dependencies and assumptions: as above. Product decisions: none. + +Acceptance: +- Core rejects missing/malformed warning fields in compact/detailed modes. +- Projection copies all result data and exposes only compact warning strings. +- Text/JSON warnings agree, without verbose messages or alternative pairs. +- Duplicate warnings collapse; distinct warnings survive. Unusual path values + remain unambiguous; text contains no active terminal controls or wrapped lines. +- Native failures, hits, preparation, cursors and request controls remain intact. +- Relevant tests, required smoke modes, build/type/format/lint/package checks, + targeted agent eval and the repository review workflow complete. + +## Validation and review + +Initial adoption evidence: 6,679 unit tests passed; 182 focused tests passed. +CI at b7866df passed Linux/Windows tests, Build & Checks, MCP package validation, +and Bun/Node 20–26 compatibility. Built Node CLI/MCP dev qualification passed +prefix/exact text/JSON with the native exact failure preserved. The initial +agent eval used one original-scope grep and reported the advisory without +executing alternatives; final inconclusive/high confidence, no isolation issues +or graded quality claim. + +Default source and built smoke modes passed unauthenticated. Full authenticated +smoke stops before grep on an existing recovery fixture: Go docker v20.10.0 now +resolves v20.10.0+incompatible instead of failing. A missing version supplies no +alternatives, so it cannot replace that fixture's recovery contract. No failure +assertion is weakened. The affected grep path is qualified independently. + +Compact revision: eight new warning tests cover summary-only projection, +non-aliasing, deduplication, no same-target suggestion, one-line output at narrow +widths, control escaping, and independent native failures/results. Existing +CLI/MCP parity covers zero-hit, mixed pending and pending-only pages without +rewrites. Compact tests failed on all seven original cases before implementation. +Internal compact-plan review: direction sound, no findings. + +Required external Claude Opus 5.5 previously hit its account usage limit before +a settled report; dispatch fenced and terminal closed. No external clean round +is claimed. Apply the review workflow to this revision and record actual checks. ## Delivery and cleanup -After clean implementation review, migrate durable knowledge and actual -validation results into implementation docs, then delete this plan in a final -commit within the same PR. Any major deferred work belongs in the backlog. -Push a reviewed draft PR with exact check results and rollout prerequisites. -Report that the backend backlog entry needs its owner's cross-repository -closure; do not edit that worktree. No new refactoring is currently needed. - - -## Actual validation and review record - -- Baseline focused suites: 169 pass. New core regression tests failed four cases - before implementation, proving absent selection/retention/validation. -- `bun test`: 6,679 pass, 0 fail across 258 files (27.05 s). -- Focused eight grep suites: 182 pass, 0 fail, 1,354 expectations after platform/control quoting fixes. -- `bun run typecheck`, `bun run format:check`, root build and MCP build pass. - `bun run lint` passes with 19 preexisting warnings confined to unchanged - `packages/mcp/src/shared/repository-target.ts`. -- `bun run validate:packages:mcp-publish` passes, including packed consumers - outside workspace aliases. npm publish dry-run skipped because current - `@githits/mcp@0.35.0` is already published; no version bump/publication attempted. -- Default `bun run smoke:cli` and `bun run smoke:mcp` pass with credentials - absent and file auth selected (live cohorts explicitly skipped AUTH_REQUIRED). - Source CLI unauthenticated and MCP registration smoke pass; built equivalents - `smoke:cli:built` and `smoke:mcp:built` pass. Initial built-smoke launch raced - package validator's own build cleanup; rerunning after validation finished - passed. No product failure or cleanup change was needed. -- Default live smoke first stopped on inaccessible keychain under isolated HOME. - Reproduced normal dev `pkg info` success vs scoped-HOME keychain error. Passing - the existing token provider's credential only through child-process memory - resolved authentication, then both full live suites stopped on the existing Go - recovery probe. Its requested `v20.10.0` now succeeds as `v20.10.0+incompatible`. - A nonexistent version failed but offered no alternatives, so it cannot replace - this probe's recovery contract. No unrelated assertion was weakened. -- Built Node dev CLI/MCP affected qualification passed prefix + exact text/JSON, - matching all warnings/hits/statuses/cursor/traversal; exact retained native - `GREP_PATH_NOT_INDEXED`. Local evidence: `/tmp/package-path-live-results/`. -- `GITHITS_ENV=dev CODEX_HOME=/Users/jpl/.codex-eval bun run agent:e2e --agent - codex --surface mcp --server local --workload - eval/agentic/probes/package-path-advisories.md --out - .agent-eval/package-path-advisories` passed harness validation (14.8 s). - URL overrides were removed for coherent dev endpoints. Inspected actual - tool-calls/final/metrics/report: one grep preserved prefix, advisory recognized, - alternatives unexecuted; final inconclusive/high confidence, no recorded - isolation violations. No graded quality claim. -- Internal code review round 1: direction sound; fix unconditional Bash replay - on Windows. Closure: retained quote-structure assertions on every platform, - gated only Bash execution, inspected related new tests; focused six tests pass - with 34 expectations. Internal round 2: clean, no findings. -- Required external Claude Opus 5.5 hit an account usage limit during plan review - before reporting. Dispatch `ctx_bec274ae3118` was fenced and its reviewer - terminal closed. No substitute model, no external clean round claimed. - -## Remaining gates and lifecycle - -Implementation and internal review are complete. The draft must state external -review unavailable and full live smoke blocked by the existing recovery probe. -A clean external review is still required by the repository workflow; retain -this plan for that review. Once clean, delete it in the final commit of this PR, -keeping durable contract, rollout and qualification facts in unified-grep.md. -Production backend deployment and subsequent CLI/MCP releases remain external -prerequisites, not authorized operations. No other implementation is deferred, -and no new refactoring opportunity or infrastructure is needed for this change. - -Final self-check found that JSON.stringify leaves C1 controls literal even -though selector normalization permits them. Added C1 to the pair round-trip -regression (failed in MCP mode before the fix), and reused the formatter's -existing escapeControls for JSON pair rows; no new boundary or helper. CLI -already uses shellQuoteExact for those controls. Checked all new advisory rows: -prose uses existing escaping, CLI rows use shell quoting, MCP rows now escape -C0/C1 while JSON.parse retains the exact value. Initial PR CI at e7b880d passed -Linux/Windows tests, Build & Checks, MCP package validation and Bun/Node 20–26 -compatibility. Follow-up quoting fix: final internal review clean, focused suites pass, -public package validation (including fresh root/MCP builds) passes. +Update draft PR #516 with compact behavior and exact verification. Keep durable +contract/rollout knowledge in docs/implementation/unified-grep.md. After clean +external review, delete this plan in a final commit of this PR. If review remains +unavailable, retain it and report that gate. The backend backlog's client-adoption +entry needs its owner's closure after shipping; do not edit that worktree. +No unrelated implementation, new infrastructure or refactoring is deferred. + +Compact revision validation completed: +- `bun test`: 6,681 passed / 0 failed, 258 files, 26,896 expectations (27.15 s). +- Eight focused grep suites after final DEL/C1 fix: 184 passed / 0 failed, + 1,359 expectations. Regression first failed both text/JSON control cases. +- Internal implementation review found DEL/C1 were escaped only in text. + Moved that escaping into the shared summary, checked root/correction and both + consumers, and asserted parsed-JSON/text equality. Full re-review clean. +- Typecheck/format and changed-file checks pass; lint has the same 19 warnings + in unchanged repository-target.ts. Fresh root/MCP builds and outside-alias + package validation pass via validate:packages:mcp-publish; publication dry-run + skipped because MCP 0.35.0 is already published. +- Default source smoke suites pass unauthenticated (live cohorts AUTH_REQUIRED); + built CLI/MCP smoke modes pass. Built dev CLI/MCP prefix/exact text+JSON + qualification passes with one compact sentence and native exact error intact. +- Local/dev agent eval completes in 20.5 s: quick_start once, same-scope grep + twice (text then JSON), no alternative executed; final inconclusive/high + confidence. Calls/final/metrics/report inspected; no isolation issues recorded, + no graded quality claim. Artifacts .agent-eval/package-path-compact. +- External Opus 5.5 review retried on this revision, again blocked by account + usage limit before a report. Dispatch ctx_56d2271550a5 fenced and terminal + closed. Plan retained for the required clean external round. diff --git a/packages/mcp/src/shared/grep-path-warnings.test.ts b/packages/mcp/src/shared/grep-path-warnings.test.ts index 80279f34..e8cc317b 100644 --- a/packages/mcp/src/shared/grep-path-warnings.test.ts +++ b/packages/mcp/src/shared/grep-path-warnings.test.ts @@ -9,37 +9,51 @@ const warning: GrepPathWarning = { target: "npm:x", path: "packages/x/src", repositorySubpath: "packages/x", - message: - 'Package paths are relative to the package root. Did you mean target "npm:x" with path "src"?', + message: "Verbose backend explanation that must stay internal.", suggestions: [ - { target: "npm:x", path: "src" }, { target: `github:o/r@${"a".repeat(40)}`, path: "packages/x/src" }, + { target: "npm:x", path: "src" }, ], }; +const summary = + 'Path is prefixed with package path "packages/x". Did you mean "src"?'; function page() { return { ...parseGrepResult(mixed), pathWarnings: structuredClone([ warning, - { ...warning, path: "packages/x/test", requestedInputIndices: [1] }, + { + ...warning, + requestedInputIndices: [1], + suggestions: [{ target: warning.target, path: "test" }], + }, ]), }; } -describe("grep path advisories", () => { - it("preserves every field and ordered input index without aliasing provider data", () => { +describe("compact grep path warnings", () => { + it("projects warning strings without verbose backend metadata or aliasing provider data", () => { const input = page(); const before = structuredClone(input); const projected = projectGrepResult(input); - expect(projected).toEqual(before); - projected.pathWarnings[0]!.requestedInputIndices.push(5); - projected.pathWarnings[0]!.suggestions[0]!.path = "changed"; - projected.pathWarnings[1]!.message = "changed"; + expect(projected).toEqual({ + ...before, + pathWarnings: [ + summary, + 'Path is prefixed with package path "packages/x". Did you mean "test"?', + ], + }); + projected.pathWarnings.push("changed"); + projected.hits.pop(); expect(input).toEqual(before); + const json = JSON.stringify(projectGrepResult(input)); + expect(json).not.toContain(warning.message); + expect(json).not.toContain('"suggestions"'); + expect(json).not.toContain(warning.suggestions[0]!.target); }); - it("keeps advisories, real matches, coverage failures, public messages and cursors together", () => { + it("keeps native failures, matches and cursors beside compact warning lines", () => { const input = page(); Object.assign(input.targets[0]!, { traversal: "FAILED", @@ -48,23 +62,45 @@ describe("grep path advisories", () => { retryable: false, }); input.traversal = "NON_RESUMABLE_PARTIAL"; - const before = structuredClone(input); const text = formatGrepText(input); expect(text).toContain("Found 100 matches"); expect(text).toContain("GREP_PATH_NOT_INDEXED"); expect(text).toContain("The requested exact path is not indexed."); - expect(text).toContain("Path advisories:"); - expect(text.split("Package paths are relative")).toHaveLength(3); - expect(text).toContain( - `target='github:o/r@${"a".repeat(40)}' path='packages/x/src'`, - ); + expect( + text.split("\n").filter((line) => line.startsWith("Warning:")), + ).toEqual([ + `Warning: ${summary}`, + 'Warning: Path is prefixed with package path "packages/x". Did you mean "test"?', + ]); expect(text).toContain("--cursor"); - expect(text).toContain("100"); - expect(input).toEqual(before); + expect(text).not.toContain(warning.message); + expect(text).not.toContain("Path advisories:"); + expect(text).not.toContain("Unverified alternatives"); + }); + + it("collapses duplicate warning text in both output formats", () => { + const input = page(); + input.pathWarnings = [warning, structuredClone(warning)]; + expect(projectGrepResult(input).pathWarnings).toEqual([summary]); + expect( + formatGrepText(input) + .split("\n") + .filter((line) => line.startsWith("Warning:")), + ).toEqual([`Warning: ${summary}`]); + }); + + it("does not invent a correction when the backend has no same-target suggestion", () => { + const input = page(); + input.pathWarnings = [ + { ...warning, suggestions: [warning.suggestions[0]!] }, + ]; + const expected = 'Path is prefixed with package path "packages/x".'; + expect(projectGrepResult(input).pathWarnings).toEqual([expected]); + expect(formatGrepText(input)).toContain(`Warning: ${expected}`); }); for (const syntax of ["cli", "mcp"] as const) { - it(`shows a successful zero-hit advisory in ${syntax} without inventing a failure`, () => { + it(`keeps the ${syntax} warning on one line even at narrow widths`, () => { const input = page(); Object.assign(input, { hits: [], @@ -74,56 +110,43 @@ describe("grep path advisories", () => { }); for (const target of input.targets) Object.assign(target, { traversal: "COMPLETE", readiness: "CURRENT" }); - const text = formatGrepText(input, { syntax }); + const text = formatGrepText(input, { syntax, width: 35 }); expect(text).toStartWith("No matches found."); - expect(text).toContain("Path advisories:"); - expect(text).toContain("Unverified alternatives"); + expect( + text.split("\n").filter((line) => line.startsWith("Warning:")), + ).toEqual([ + `Warning: ${summary}`, + 'Warning: Path is prefixed with package path "packages/x". Did you mean "test"?', + ]); expect(text).not.toContain("search failed"); expect(text).not.toContain("still being prepared"); - expect(text).not.toContain("Retry"); }); - it(`quotes exact ${syntax} pairs with spaces, quotes, metacharacters and controls without wrapping`, async () => { + it(`quotes unusual ${syntax} paths and escapes terminal controls`, () => { const input = page(); - const target = `github:o/weird repo'";$()\\name@${"b".repeat(40)}`; const path = "src/a 'quoted' \"file\";$HOME`echo nope`\\é\n\u001b[31m\u009b[31m.ts"; input.pathWarnings = [ { ...warning, - message: "Public\u001b[31m message", - suggestions: [{ target, path }], + repositorySubpath: "packages/\u009bx\u007f", + suggestions: [{ target: warning.target, path }], }, ]; - const text = formatGrepText(input, { - syntax, - width: 35, - useColors: false, - }); - expect(text).toContain("Public\\u001b[31m message"); - expect(text).not.toContain("\u001b"); - expect(text).not.toContain("\u009b"); - const row = text.split("\n").find((line) => line.startsWith(" "))!; - expect(row.length).toBeGreaterThan(35); - if (syntax === "mcp") { - expect(JSON.parse(row.trim())).toEqual({ target, path }); - } else { - expect(row).toStartWith( - " target='github:o/weird repo'\"'\"'\";$()\\name@", - ); - expect(row).toContain(" path=$'src/"); - // Bash is not a Windows prerequisite; quote structure is asserted on every host. - if (process.platform === "win32") return; - // Execute assignments only: quoted substitutions must remain literal data. - const child = Bun.spawn( - ["bash", "-c", `${row.trim()}; printf '%s\\0%s' "$target" "$path"`], - { stdout: "pipe", stderr: "pipe" }, - ); - expect(await new Response(child.stdout).text()).toBe( - `${target}\0${path}`, - ); - expect(await child.exited).toBe(0); - } + const text = formatGrepText(input, { syntax, width: 35 }); + const row = text.split("\n").find((line) => line.startsWith("Warning:"))!; + expect(row).toContain('Did you mean "src/a'); + expect(row).toContain( + '\\"file\\";$HOME`echo nope`\\\\é\\n\\u001b[31m\\u009b[31m.ts"?', + ); + expect(row).not.toContain("\u001b"); + expect(row).not.toContain("\u009b"); + const projected = projectGrepResult(input); + const warnings = JSON.parse(JSON.stringify(projected)).pathWarnings; + expect(warnings).toEqual(projected.pathWarnings); + expect(row.slice("Warning: ".length)).toBe(warnings[0]); + expect(warnings[0]).not.toContain("\u009b"); + expect(warnings[0]).not.toContain("\u007f"); }); } }); diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index eaab980e..fcd4b601 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -71,7 +71,10 @@ describe("unified grep result and text", () => { it("accepts a pre-metadata injected provider while preserving absent metadata", () => { const provider = result(); delete provider.indexingEstimates; - expect(projectGrepResult(provider)).toEqual(provider); + expect(projectGrepResult(provider)).toEqual({ + ...provider, + pathWarnings: [], + }); expect(projectGrepResult(provider)).not.toHaveProperty("indexingEstimates"); }); it("retains an unvisited selected site and explains its continuation", () => { @@ -92,7 +95,7 @@ describe("unified grep result and text", () => { traversal: "RESUMABLE_LIMIT", nextCursor: "opaque", }); - expect(projectGrepResult(page)).toEqual(page); + expect(projectGrepResult(page)).toEqual({ ...page, pathWarnings: [] }); const output = formatGrepText(page); expect(output).toContain(" - site:docs.test (not reached on this page)"); expect(output).not.toContain("not visited in this page"); @@ -183,7 +186,7 @@ describe("unified grep result and text", () => { ...data, unknown: "drop", } as GrepResult); - expect(projected).toEqual(data); + expect(projected).toEqual({ ...data, pathWarnings: [] }); expect(projected.targets[0]?.publicMessage).toBeNull(); expect(projected.hits[0]?.sourceMatchStartByte).toBe(10); projected.hits.pop(); diff --git a/packages/mcp/src/shared/grep-response.ts b/packages/mcp/src/shared/grep-response.ts index 71d037c0..e3b00ddf 100644 --- a/packages/mcp/src/shared/grep-response.ts +++ b/packages/mcp/src/shared/grep-response.ts @@ -1,7 +1,37 @@ -import { type GrepResult, parseGrepResult } from "@githits/core-internal"; +import { + type GrepPathWarning, + type GrepResult, + parseGrepResult, +} from "@githits/core-internal"; -/** Preserve selected wire fields, including path advisories, without aliasing provider data. */ -export function projectGrepResult(result: GrepResult): GrepResult { +/** Frontend pages retain result data and expose only compact warning strings. */ +export interface GrepResponse extends Omit { + pathWarnings: string[]; +} + +/** Summarize backend evidence once per distinct correction, without guessing paths. */ +export function summarizeGrepPathWarnings( + warnings: GrepPathWarning[], +): string[] { + return [ + ...new Set( + warnings.map((warning) => { + const suggestion = warning.suggestions.find( + (entry) => entry.target === warning.target, + ); + return `Path is prefixed with package path ${JSON.stringify(warning.repositorySubpath)}.${suggestion ? ` Did you mean ${JSON.stringify(suggestion.path)}?` : ""}`.replace( + // biome-ignore lint/suspicious/noControlCharactersInRegex: JSON leaves DEL/C1 literal; keep both output formats terminal-safe. + /[\u007f-\u009f]/g, + (character) => + `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ); + }), + ), + ]; +} + +/** Validate and copy the page, compacting only advisory presentation. */ +export function projectGrepResult(result: GrepResult): GrepResponse { // The wire contract requires the array; injected providers may predate it. const projected = parseGrepResult({ ...result, @@ -9,5 +39,8 @@ export function projectGrepResult(result: GrepResult): GrepResult { }); if (result.indexingEstimates === undefined) delete projected.indexingEstimates; - return projected; + return { + ...projected, + pathWarnings: summarizeGrepPathWarnings(projected.pathWarnings), + }; } diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index 6c402be6..a19b58d2 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -6,6 +6,7 @@ import type { } from "@githits/core-internal"; import { colors, highlightMatch } from "./colors.js"; import { grepPreparationReason } from "./grep-preparation-text.js"; +import { summarizeGrepPathWarnings } from "./grep-response.js"; import { formatPreparationRow } from "./indexing-estimates-text.js"; import { indexingWaitMs } from "./indexing-wait.js"; import { @@ -198,24 +199,9 @@ export function formatGrepText( ? `${result.hits.length ? "Some requested content could not be searched; the" : "The"} service did not explain why or whether retrying would help.` : "The search did not finish, and no continuation cursor was returned. The service did not explain why the full search stopped or whether retrying would help.", ); - if (result.pathWarnings.length) { - lines.push("", "Path advisories:"); - for (const warning of result.pathWarnings) { - prose(` - ${warning.message}`); - if (warning.suggestions.length) - prose( - " Unverified alternatives (keep selector kind and other controls):", - ); - for (const suggestion of warning.suggestions) { - // Pair rows are data, not a guessed grep invocation: the warning has no selector kind. - lines.push( - options.syntax === "mcp" - ? ` ${escapeControls(JSON.stringify({ target: suggestion.target, path: suggestion.path }))}` - : ` target=${shellQuoteExact(suggestion.target)} path=${shellQuoteExact(suggestion.path)}`, - ); - } - } - } + if (result.pathWarnings.length) lines.push(""); + for (const warning of summarizeGrepPathWarnings(result.pathWarnings)) + lines.push(`Warning: ${escapeControls(warning)}`); for (const [index, group] of groups.entries()) { const first = group.first; const target = first.read.target; diff --git a/packages/mcp/src/smoke-test.ts b/packages/mcp/src/smoke-test.ts index 65923e82..7c649515 100644 --- a/packages/mcp/src/smoke-test.ts +++ b/packages/mcp/src/smoke-test.ts @@ -283,8 +283,9 @@ function assertGrepResult( ): Record { assertRecord(value, context); assert( - Array.isArray(value.pathWarnings), - `${context}: pathWarnings must be an array`, + Array.isArray(value.pathWarnings) && + value.pathWarnings.every((warning) => typeof warning === "string"), + `${context}: pathWarnings must contain compact warning strings`, ); assert(Array.isArray(value.hits), `${context}: hits must be an array`); assert(Array.isArray(value.targets), `${context}: targets must be an array`); diff --git a/packages/mcp/src/tools/grep.ts b/packages/mcp/src/tools/grep.ts index ece09f80..d5a98ec5 100644 --- a/packages/mcp/src/tools/grep.ts +++ b/packages/mcp/src/tools/grep.ts @@ -168,13 +168,15 @@ export function createGrepTool( waitTimeoutMs: args.wait_timeout_ms, includeDetailedFields: args.format === "json", }); - const projected = projectGrepResult( - await service.grep(params, ...serviceRequestArgs(context?.signal)), + const result = await service.grep( + params, + ...serviceRequestArgs(context?.signal), ); + const projected = projectGrepResult(result); return textResult( args.format === "json" ? JSON.stringify(projected) - : formatGrepText(projected, { + : formatGrepText(result, { syntax: "mcp", useColors: false, requestedTargets: params.targets.map((target) => target.target), diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index 39b49358..85be6336 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -2294,7 +2294,8 @@ async function runLiveSmoke(env: Record): Promise { ); assertRecord(grepPage, "unified grep source JSON"); assert( - Array.isArray(grepPage.pathWarnings), + Array.isArray(grepPage.pathWarnings) && + grepPage.pathWarnings.every((warning) => typeof warning === "string"), "unified grep must retain pathWarnings", ); assert( diff --git a/src/commands/grep.ts b/src/commands/grep.ts index 5c9514ab..5e25bd2e 100644 --- a/src/commands/grep.ts +++ b/src/commands/grep.ts @@ -90,7 +90,7 @@ export async function grepAction( if (options.json) console.log(JSON.stringify(projected)); else process.stdout.write( - `${formatGrepText(projected, { useColors: shouldUseColors(), width: process.stdout.columns || 80, syntax: "cli", requestedTargets: params.targets.map((target) => target.target) })}\n`, + `${formatGrepText(result, { useColors: shouldUseColors(), width: process.stdout.columns || 80, syntax: "cli", requestedTargets: params.targets.map((target) => target.target) })}\n`, ); } catch (error) { const mapped = mapGrepError(error); @@ -204,7 +204,7 @@ export function registerGrepCommand( "--wait ", "Wait for target preparation (0-300000 ms, default 30000)", ) - .option("--json", "Emit the detailed lossless JSON page") + .option("--json", "Emit the detailed JSON page") .action( async ( pattern: string, diff --git a/src/tools/grep-parity.test.ts b/src/tools/grep-parity.test.ts index 672f451a..a1a5c6ee 100644 --- a/src/tools/grep-parity.test.ts +++ b/src/tools/grep-parity.test.ts @@ -313,24 +313,41 @@ describe("package-path advisory CLI/MCP parity", () => { expect(cli).not.toContain("Path advisories:"); } else { for (const output of [cli, text]) { - expect(output).toContain("Path advisories:"); - expect(output.split("Package paths are relative")).toHaveLength(3); - expect(output).toContain("Unverified alternatives"); + expect( + output.split("\n").filter((line) => line.startsWith("Warning:")), + ).toEqual([ + 'Warning: Path is prefixed with package path "packages/x". Did you mean "src \'quoted\';$HOME"?', + ]); + expect(output).not.toContain(warning.message); + expect(output).not.toContain("Unverified alternatives"); if (shape === "mixed-pending") { expect(output).toContain("1: router"); expect(output).toContain("Preparing:"); expect(output).toContain("npm:pending"); } else expect(output).toStartWith("No matches found."); } - expect(text).toContain(JSON.stringify(warning.suggestions[1])); - expect(cli).toContain(`target='github:o/r@${"c".repeat(40)}'`); + expect(text).not.toContain(JSON.stringify(warning.suggestions[1])); + expect(cli).not.toContain( + `target='${warning.suggestions[1]!.target}' path=`, + ); } expect(grep.mock.calls[0]?.[0]).toEqual(grep.mock.calls[1]?.[0]); await runCli(true); const json = await tool.handler({ ...args, format: "json" }); expect(json.isError).toBeUndefined(); - expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual(before); - expect(JSON.parse(String(json.content[0]?.text))).toEqual(before); + const expected = { + ...before, + pathWarnings: [ + ...new Set( + before.pathWarnings.map( + () => + 'Path is prefixed with package path "packages/x". Did you mean "src \'quoted\';$HOME"?', + ), + ), + ], + }; + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual(expected); + expect(JSON.parse(String(json.content[0]?.text))).toEqual(expected); expect(grep).toHaveBeenCalledTimes(4); expect(grep.mock.calls[2]?.[0]).toEqual(grep.mock.calls[3]?.[0]); expect(grep.mock.calls[3]?.[0]).toMatchObject({ From ce5c3b267a577842c318fffaad4ac457150fa9b8 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Sat, 24 Oct 2026 23:13:01 +0300 Subject: [PATCH 4/4] docs: complete package-path advisory review and plan cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the clean Claude round’s documentation corrections, retain the compact qualification evidence, and move the existing authenticated smoke fixture gap to the backlog. Remove the completed implementation plan after review without changing runtime behavior. --- docs/implementation/unified-grep.md | 60 ++++++------- docs/plans/open-backlog.md | 18 ++++ docs/plans/package-path-advisories.md | 124 -------------------------- 3 files changed, 48 insertions(+), 154 deletions(-) delete mode 100644 docs/plans/package-path-advisories.md diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index 6b535e44..1c819b73 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -413,8 +413,9 @@ Warning: Path is prefixed with package path "packages/example". Did you mean "sr The package root and correction come directly from backend fields: the correction is the suggestion whose target equals the warning's target. There is no local prefix-stripping heuristic. When no same-target suggestion exists, the sentence -stops after the package path. Values are JSON-quoted; text escapes terminal -controls without wrapping warning lines. Identical sentences are emitted once. +stops after the package path. The shared summary JSON-quotes values and escapes +DEL/C1 controls for both text and JSON. Text keeps each warning on one physical +line. Identical sentences are emitted once. Neither output format dumps the verbose backend message or alternative pairs. A question suggests a possible correction; it does not assert that a path exists. @@ -439,10 +440,9 @@ Introspection is disabled at both endpoints; the validation query uses `@skip(if: true)` to check selections without executing grep. Before releasing a client that selects this field, verify the configured -backend accepts the full selection below. Backend schema support must be verified on the configured endpoint; merging -its PR is not proof of deployment. No schema fallback -or downgraded query is provided: an older endpoint rejects the entire grep -query. Native advisory generation additionally requires the backend's released +backend accepts the full selection below; merging its PR is not proof of +deployment. No schema fallback or downgraded query is provided: an older +endpoint rejects the entire grep query. Native advisory generation additionally requires the backend's released aigrep 0.27.3/API56 path-evidence integration. Schema support alone does not prove native advisory behavior; qualify an applicable request on that endpoint. @@ -466,27 +466,27 @@ to close after this client work ships; it is not edited across worktrees. ### Client qualification (2026-10-24) -- Built Node CLI and local stdio MCP against dev: prefix and exact selectors - both retained one advisory and complete backend data before the compact UX - revision. The current frontend emits only compact warning strings. Prefix returned zero hits with `COMPLETE`; exact returned - zero hits with `NON_RESUMABLE_PARTIAL` and its original - `GREP_PATH_NOT_INDEXED` failure. CLI/MCP warnings, hits, target statuses, - traversal and cursors matched. Secret-free local evidence is retained in - `/tmp/package-path-live-results/`; these transient files are not release proof - for any other endpoint. -- The targeted `eval/agentic/probes/package-path-advisories.md` local MCP/dev - run used Codex (`gpt-6-luna`, high). Its trace contained one `quick_start` and - one `grep` with the requested prefix; no alternative query was executed. - The final answer reported the advisory and proposed alternatives, with - `status: inconclusive` and `confidence: high`. The harness completed in - 14.8 seconds, with no recorded isolation violations. This is observed agent - behavior, not a graded usefulness/quality result. -- Deterministic parser/projection/formatter and CLI/MCP parity tests cover - multiple warnings and input indices, zero hits, existing failures/cursors, - mixed hits/pending and pending-only short-wait pages, quoting and controls. -- Full authenticated live smoke currently stops at an unrelated Go recovery - probe before grep: `go:github.com/docker/docker@v20.10.0` now resolves to - `v20.10.0+incompatible`, contradicting the probe's expected missing version. - The broad live suites are not claimed green. Their auth/registration modes - and the affected grep qualification above are independent checks. No failure - assertion was weakened to hide this backend/fixture discrepancy. +The compact implementation at `8e5ab06` was verified through built Node CLI and +local stdio MCP against dev. Prefix and exact selectors returned the same compact +warning string in text and JSON. Prefix returned zero hits with `COMPLETE`; +exact returned zero hits with `NON_RESUMABLE_PARTIAL` and the original +`GREP_PATH_NOT_INDEXED` failure. Hits, statuses, traversal and cursors matched. + +The targeted `eval/agentic/probes/package-path-advisories.md` local MCP/dev run +used Codex (`gpt-6-luna`, high). Its trace contained one `quick_start` and two +grep calls with the same requested prefix (text, then JSON); no alternative +query was executed. The final answer recognized the correction and reported +`status: inconclusive` / `confidence: high`. No isolation violations were +recorded. This is observed behavior, not a graded usefulness/quality result. + +Validation at that revision: 6,681 unit tests and 184 focused grep tests passed; +CI passed Linux/Windows tests, build checks, MCP package validation and Bun/Node +20/22/24/26 compatibility. Regression coverage includes projection, deduplication, +no same-target suggestion, one-line warnings, unusual values and DEL/C1 parity, +zero hits, native failures/cursors and mixed/pending-only short-wait results. + +Source and built unauthenticated smoke modes passed. Full authenticated smoke +stops before grep at an existing recovery fixture; see the +[backlog entry](../plans/open-backlog.md#authenticated-smoke-recovery-fixture--2026-10-24). +The affected grep qualification above passed independently; no failure assertion +was weakened to bypass that broader-suite gap. diff --git a/docs/plans/open-backlog.md b/docs/plans/open-backlog.md index 231fbe85..ae1073ec 100644 --- a/docs/plans/open-backlog.md +++ b/docs/plans/open-backlog.md @@ -1,5 +1,23 @@ # Open backlog +## Authenticated smoke recovery fixture — 2026-10-24 + +During [PR #516](https://github.com/githits-com/githits-cli/pull/516), both full +authenticated smoke suites stopped before grep: the recovery probe expects +`go:github.com/docker/docker@v20.10.0` to fail, but dev now resolves it as +`v20.10.0+incompatible`. A nonexistent version returned `VERSION_NOT_FOUND` +without alternatives, so it cannot simply replace the existing probe, which +also checks backend-supplied recovery guidance. Unauthenticated smoke and the +affected built CLI/MCP grep qualification passed independently. + +Ownership: the CLI/MCP smoke cohorts own the fixture and assertions; backend +recovery metadata owns the supplied alternatives. Investigate that boundary +before choosing a replacement fixture or correcting a verified producer issue. +Keep the assertions for error code, alternatives and native CLI/MCP recovery +syntax; do not turn the probe into a successful lookup or weaken coverage. +Acceptance: both authenticated dev smoke suites pass with a verified failing +version/ref and supplied alternatives, and continue through their live cohorts. + ## Literal path round-trips — 2026-10-22 The path audit reproduced two accepted-input inconsistencies beyond ordinary diff --git a/docs/plans/package-path-advisories.md b/docs/plans/package-path-advisories.md deleted file mode 100644 index 382afc53..00000000 --- a/docs/plans/package-path-advisories.md +++ /dev/null @@ -1,124 +0,0 @@ -# Unified grep package-path advisories - -Status: compact UX revision implemented and internally reviewed clean. Local -validation passed; external review unavailable (Claude quota). - -## Outcome and scope - -CLI/MCP agents receive one compact warning per distinct backend path correction -in both text and JSON. Preserve native results, failures, preparation, cursors -and exact query scope. No local diagnosis, path probing, request rewrites or -retries. No infrastructure, new options, merge, release, publish or deploy. - -User feedback supersedes the original verbose text and advisory-object JSON: - -```text -Warning: Path is prefixed with package path "drizzle-orm". Did you mean "src/pg-core"? -``` - -## Verified state and dependencies - -- Based on main `0a97eb12a15dca43b8c802256597db42a96c8a35`; draft PR #516 - includes initial adoption commits `e7b880d` and `b7866df`. -- Core selects/validates all `pathWarnings` fields. Shared projection reparses; - CLI/MCP already share the text formatter. Short waits return the backend page - directly, without a separate conversion that could drop warnings. -- Backend #3550 merged at `b26172c5fc97287256e55d22bc64aa384297184c`. - Production initially rejected `pathWarnings`. Recheck at 2026-10-24 10:25 UTC - accepted all fields; the original prefix request returned a warning with zero - hits and COMPLETE. The backend deployment prerequisite is satisfied. -- Client releases are still required. Hosted clients additionally require a - published MCP package upgrade/deployment by remote-mcp, separately authorized. - -## Ownership and implementation - -Core owns backend metadata and its validation. The shared response projection -owns frontend JSON; its `GrepResponse` retains result data but changes -`pathWarnings` to `string[]`. The existing shared text formatter continues to -accept core results and uses the same compact summary function. CLI/MCP both -call projection validation before formatting their raw service result. - -The summary uses `repositorySubpath` and the backend suggestion for the same -`target`; it never strips a prefix or guesses a correction. If that suggestion -is absent, omit the question. Quote path values with JSON quoting; text escapes -controls and leaves each warning on one physical line. Deduplicate identical -sentences. Neither frontend format dumps backend messages or alternative pairs. -The public core service types retain complete evidence for service consumers. - -Assumptions: backend owns diagnosis; suggestions are possibilities, not verified -paths. Both are established by the backend contract and read-only guide. -Unknowns/product decisions: none. No performance claim or runtime optimization. - -## Phase 1 — compact advisories survive every result shape (IMPLEMENTED) - -Expected outcome: one useful warning per correction across text/JSON and CLI/MCP, -including zero-hit, failure, mixed pending and pending-only results. -Dependencies and assumptions: as above. Product decisions: none. - -Acceptance: -- Core rejects missing/malformed warning fields in compact/detailed modes. -- Projection copies all result data and exposes only compact warning strings. -- Text/JSON warnings agree, without verbose messages or alternative pairs. -- Duplicate warnings collapse; distinct warnings survive. Unusual path values - remain unambiguous; text contains no active terminal controls or wrapped lines. -- Native failures, hits, preparation, cursors and request controls remain intact. -- Relevant tests, required smoke modes, build/type/format/lint/package checks, - targeted agent eval and the repository review workflow complete. - -## Validation and review - -Initial adoption evidence: 6,679 unit tests passed; 182 focused tests passed. -CI at b7866df passed Linux/Windows tests, Build & Checks, MCP package validation, -and Bun/Node 20–26 compatibility. Built Node CLI/MCP dev qualification passed -prefix/exact text/JSON with the native exact failure preserved. The initial -agent eval used one original-scope grep and reported the advisory without -executing alternatives; final inconclusive/high confidence, no isolation issues -or graded quality claim. - -Default source and built smoke modes passed unauthenticated. Full authenticated -smoke stops before grep on an existing recovery fixture: Go docker v20.10.0 now -resolves v20.10.0+incompatible instead of failing. A missing version supplies no -alternatives, so it cannot replace that fixture's recovery contract. No failure -assertion is weakened. The affected grep path is qualified independently. - -Compact revision: eight new warning tests cover summary-only projection, -non-aliasing, deduplication, no same-target suggestion, one-line output at narrow -widths, control escaping, and independent native failures/results. Existing -CLI/MCP parity covers zero-hit, mixed pending and pending-only pages without -rewrites. Compact tests failed on all seven original cases before implementation. -Internal compact-plan review: direction sound, no findings. - -Required external Claude Opus 5.5 previously hit its account usage limit before -a settled report; dispatch fenced and terminal closed. No external clean round -is claimed. Apply the review workflow to this revision and record actual checks. - -## Delivery and cleanup - -Update draft PR #516 with compact behavior and exact verification. Keep durable -contract/rollout knowledge in docs/implementation/unified-grep.md. After clean -external review, delete this plan in a final commit of this PR. If review remains -unavailable, retain it and report that gate. The backend backlog's client-adoption -entry needs its owner's closure after shipping; do not edit that worktree. -No unrelated implementation, new infrastructure or refactoring is deferred. - -Compact revision validation completed: -- `bun test`: 6,681 passed / 0 failed, 258 files, 26,896 expectations (27.15 s). -- Eight focused grep suites after final DEL/C1 fix: 184 passed / 0 failed, - 1,359 expectations. Regression first failed both text/JSON control cases. -- Internal implementation review found DEL/C1 were escaped only in text. - Moved that escaping into the shared summary, checked root/correction and both - consumers, and asserted parsed-JSON/text equality. Full re-review clean. -- Typecheck/format and changed-file checks pass; lint has the same 19 warnings - in unchanged repository-target.ts. Fresh root/MCP builds and outside-alias - package validation pass via validate:packages:mcp-publish; publication dry-run - skipped because MCP 0.35.0 is already published. -- Default source smoke suites pass unauthenticated (live cohorts AUTH_REQUIRED); - built CLI/MCP smoke modes pass. Built dev CLI/MCP prefix/exact text+JSON - qualification passes with one compact sentence and native exact error intact. -- Local/dev agent eval completes in 20.5 s: quick_start once, same-scope grep - twice (text then JSON), no alternative executed; final inconclusive/high - confidence. Calls/final/metrics/report inspected; no isolation issues recorded, - no graded quality claim. Artifacts .agent-eval/package-path-compact. -- External Opus 5.5 review retried on this revision, again blocked by account - usage limit before a report. Dispatch ctx_56d2271550a5 fenced and terminal - closed. Plan retained for the required clean external round.