diff --git a/changes/code-diff-cli-dogfood.added.md b/changes/code-diff-cli-dogfood.added.md new file mode 100644 index 0000000..7d1ad43 --- /dev/null +++ b/changes/code-diff-cli-dogfood.added.md @@ -0,0 +1,6 @@ +--- +"githits": minor +"@githits/mcp": none +--- + +- **Add silent CodeDiff CLI dogfooding** - Adds `githits code diff` with bounded Git-like views, repository-relative glob filtering, reversible path quoting, apply-safe patch handling, structured completeness diagnostics, and JSON output without exposing an MCP tool or agent guidance yet. diff --git a/docs/implementation/cli-commands.md b/docs/implementation/cli-commands.md index f9f0a04..59755c8 100644 --- a/docs/implementation/cli-commands.md +++ b/docs/implementation/cli-commands.md @@ -32,6 +32,7 @@ The CLI exposes setup/auth commands, `doctor`, `example`, `languages`, `feedback | `pkg upgrade-review [spec]` | single package spec with current version plus `--to`, OR repeatable `--package` ranges | `--to`, repeatable `--package`, `--no-transitive-security`, `--dependency-issues`, `--min-severity`, `--verbose`, `--json` | Compare current and target versions for upgrade evidence: vulnerabilities, changelog entries, deprecation metadata, peer changes, dependency changes, and transitive security evidence by default. Reports facts only. | | `docs list ` | package spec (optional `@version`) | `--limit`, `--after`, `--verbose`, `--json` | List hosted/crawled and repository-backed documentation pages for a package. Entries include page IDs for `docs read`; JSON includes exact repo-file follow-up metadata when available. | | `docs read ` | page ID from `docs list` or search results | `--lines`, `--verbose`, `--json` | Read a documentation page by page ID. Default output is content-only; `--lines` fetches a bounded range for long pages. | +| `code diff ..` | unversioned package/repository target and exact range, or `--repo-url` and range | `--patch`, `--stat`, `--name-only`, `--name-status`, `--max-files`, `--max-patch-bytes`, `--verbose`, `--json`, one glob after `--` | Silently dogfood a bounded exact-tree diff; not exposed through MCP or agent guidance | | `code files [spec] [path-prefix]` | package spec OR `--repo-url` with optional `--git-ref`; optional `[path-prefix]` | `--path`, repeatable `--glob`, repeatable `--ext`, repeatable `--file-type`, repeatable `--language`, repeatable `--file-intent`, repeatable `--exclude-intent`, `--exclude-docs`, `--exclude-tests`, `--hidden`, `--limit`, `--wait`, `--verbose`, `--json` | List files in an indexed dependency. Selectors (`[path-prefix]`, `--path`, `--glob`) are OR-ed; the other flags filter that scope down further. Plain output is one path per line; `--verbose` adds language / type / size annotations. Indexing errors include elapsed/expected duration when available plus retry via `--wait` or indexed refs/versions from the error detail. | | `code read ` | package spec OR `--repo-url` with optional `--git-ref`; plus `` | `--lines`, `--start`, `--end`, `--wait`, `--verbose`, `--json` | Read a file's contents. Plain output is the raw file bytes (pipe-friendly); `--verbose` adds a header and a line-number gutter. `--lines 10-40` concise form; `--start`/`--end` equivalent. Binary files show a sentinel line. | | `code grep [spec] [path-prefix]` | package spec OR `--repo-url` with optional `--git-ref`; plus `` and optional `[path-prefix]` | `--path`, repeatable `--glob`, repeatable `--ext`, `--regex`, `--case-sensitive`, `-C/-A/-B`, `--exclude-docs`, `--exclude-tests`, `--limit`, `--per-file-limit`, `--cursor`, `--symbol-field`, `--wait`, `--verbose`, `--json` | Deterministic text grep over indexed dependency or repository source. Defaults to whole-target, literal, ASCII case-insensitive matching; non-ASCII letters match case-sensitively. Narrow with `[path-prefix]`, `--path`, `--glob`, or `--ext`. Plain output is `file:line:text`; `--verbose` groups matches by file. | @@ -502,6 +503,45 @@ Reads a documentation page returned by `docs list` or search results. Default ou **Troubleshooting.** Same debug areas as the `pkg` family. +### `githits code diff` + +```sh +githits code diff npm:express 4.18.1..4.18.2 +githits code diff npm:express 4.18.1..4.18.2 --stat +githits code diff npm:express 4.18.1..4.18.2 --name-status -- 'lib/**/*.js' +githits code diff --repo-url https://github.com/expressjs/express v4.18.1..v4.18.2 --name-only +``` + +Compares two exact source trees left-to-right. Package targets must omit a +version and repository targets must omit a ref because both endpoints belong +in the required two-dot range. Three-dot merge-base syntax and `--git-ref` are +rejected. The optional value after `--` is one repository-relative bounded +glob, not a full Git pathspec. A backslash escapes one following non-slash +character according to the backend grammar. + +Patch output is the default. `--stat`, `--name-only`, and `--name-status` +select cheaper views and are mutually exclusive with `--patch` and each other. +`--max-files` applies to every view; `--max-patch-bytes` is patch-only. The CLI +does not send client defaults for either bound. + +The selected Git-like view stays on stdout. Completeness, truncation, scope, +content-safety, and display-only path warnings stay on stderr; `--verbose` adds +exact resolutions and scope facts there. JSON keeps the same evidence as a +lean selected-view envelope, including authoritative paths in normalized patch +headers. Text paths use reversible Git-style quoting rather than deleting +control characters. Empty authoritative diffs exit 0. Caller-selected +`--max-files` and `--max-patch-bytes` bounds may intentionally produce partial +patches and still exit 0 with warnings. Unexpectedly incomplete or non-applicable +plain patches are suppressed and exit 1; name, stat, and JSON views preserve +their structured partial evidence. Suppression diagnostics name binary and +metadata-only causes and direct terminal users to stat/name views. Patch output +is applicable unified-diff content but may omit Git metadata such as index and +mode headers. Request, auth, resolution, and backend errors also exit 1. + +This is a silent dogfood surface. It is normally registered and documented for +maintainers, but no MCP tool, MCP instruction, Agent Skill, or plugin guidance +promotes it yet. + ### `githits code files` ``` diff --git a/docs/implementation/code-diff.md b/docs/implementation/code-diff.md index 6a103a5..054175b 100644 --- a/docs/implementation/code-diff.md +++ b/docs/implementation/code-diff.md @@ -3,13 +3,13 @@ ## Purpose The transport-neutral adapter exposes PkgSeer's exact-tree `codeDiff` GraphQL -operation to the public `@githits/mcp/client` runtime. It provides the typed -boundary needed by the later CLI and MCP surfaces without choosing their -ergonomics or claiming that a patch proves compatibility. +operation to the public `@githits/mcp/client` runtime. The root package also +registers `githits code diff` as an intentionally unpromoted CLI dogfood +surface. Neither layer claims that a patch proves compatibility. -Phase 1 deliberately adds no CLI command or MCP tool. Those surfaces must first -settle Git-like view names, path-glob behavior, output defaults, and parity -tests. +The CLI is being exercised before any MCP tool or agent instruction is added. +This keeps agent-facing signatures out of the public surface until the Git-like +ergonomics and evidence envelope have been dogfooded. ## Addressing and modes @@ -75,6 +75,54 @@ when adopting the package version containing this adapter. The existing test factories provide deterministic default results so current tool tests remain focused on their own behavior. +## Silent CLI dogfood contract + +The CLI accepts either an unversioned package/repository target followed by an +explicit `from..to` range, or `--repo-url ` followed by that range: + +```sh +githits code diff npm:express 4.18.1..4.18.2 +githits code diff npm:express 4.18.1..4.18.2 --name-status +githits code diff --repo-url https://github.com/expressjs/express v4.18.1..v4.18.2 -- 'lib/**/*.js' +``` + +The default is bounded patch output. `--patch`, `--stat`, `--name-only`, and +`--name-status` are mutually exclusive; the inventory-backed name views avoid +requesting stats or patches. One optional repository-relative glob follows +`--`. It is the backend's bounded `*`/`?`/exact-`**` grammar, not a Git +pathspec. A backslash escapes exactly one following non-slash character; this +mirrors PkgSeer's `CodeDiff.Raw.PathGlob` compiler rather than shell or Git +escaping. `--max-files` applies to every view and `--max-patch-bytes` applies +only to patch output. Omitted bounds remain absent on the wire so the backend +owns its defaults. + +Plain stdout contains only the selected Git-like projection. Resolution, +scope, truncation, unprojectable-file, content-coverage, path-encoding, and +content-safety diagnostics go to stderr. `--verbose` adds exact identity and +scope diagnostics without changing the primary stream. `--json` emits a lean +camel-case data envelope whose file objects include only fields relevant to the +selected view, except that `pathEncoding` is always retained to distinguish +display-only byte escapes. Text views use reversible Git-style quoting for +control characters, quotes, and backslashes instead of changing path identity. +The response projector replaces the raw content service's `a/file` and +`b/file` patch placeholders with the authoritative Git-quoted file path, so +plain and JSON patches agree; added and deleted sides use `/dev/null` like Git. + +An empty authoritative diff exits 0. Name, stat, and JSON views retain partial +evidence with explicit completeness fields and diagnostics. Plain patch mode +suppresses stdout and exits 1 when unexpected truncation, failed or unavailable +content, binary/metadata-only changes, display-only paths, unprojectable files, +or content-safety changes would make the stream unsafe to apply. An explicit +`--max-files` authorizes file-count truncation, and an explicit +`--max-patch-bytes` authorizes aggregate patch-budget omissions; neither +authorizes unrelated failure classes. Suppression diagnostics name +binary/metadata-only causes and direct humans to stat/name views while JSON +retains structured partial evidence. The applicable patch stream is unified +diff content; the backend does not provide Git index or mode headers. +Validation, authentication, resolution, and raw-field errors exit 1 through +the shared CLI error envelope. No `code_diff` MCP tool, instruction, skill, or +plugin promotion exists during this phase. + ## Key reference files | File | Responsibility | @@ -82,5 +130,7 @@ focused on their own behavior. | `packages/core-internal/src/services/code-navigation-service.ts` | GraphQL query, validation, schemas, normalization, and errors | | `packages/core-internal/src/services/code-navigation-service.test.ts` | Wire-selection, variables, normalization, and failure fixtures | | `packages/mcp/src/client.ts` | Public client type/value re-exports | +| `packages/mcp/src/shared/code-diff-{request,response,text}.ts` | CLI-internal normalization, lean projection, and Git-like rendering | +| `src/commands/code/diff.ts` | Commander syntax, service call, stream routing, and CLI errors | | `scripts/validate-public-packages.ts` | Packed-package runtime and no-network TypeScript consumer checks | -| `docs/plans/code-diff-cli-mcp.md` | Phase 2 CLI/MCP ergonomics and scope decisions | +| `docs/plans/code-diff-cli-mcp.md` | Remaining rollout phases and dogfood acceptance evidence | diff --git a/docs/plans/code-diff-cli-mcp.md b/docs/plans/code-diff-cli-mcp.md index f84ef9f..d94f8ac 100644 --- a/docs/plans/code-diff-cli-mcp.md +++ b/docs/plans/code-diff-cli-mcp.md @@ -1,597 +1,612 @@ -# Plan: Raw code diff CLI and MCP surface +# Plan: CodeDiff CLI dogfood and agent rollout -> Overall status: Phase 1 complete; Phase 2 ergonomics require focused design -> against the accepted Git-like direction and the narrower backend glob -> contract. +> Overall status: Phase 1 is merged. Phase 2 is implemented and locally/live +> verified and reviewed as a silent CLI-only dogfood launch; merge is pending. +> MCP and agent-facing exposure remain deferred until the CLI contract has +> been exercised after merge. > -> Reoriented: 2026-08-17 against `githits` `origin/main` at `fa27604` and -> the PkgSeer backend `main` GraphQL schema and implementation. +> Reoriented: 2026-08-17 against `githits` `origin/main` at `77417aa` and +> PkgSeer backend `origin/main` at `c0cf92e` with GraphQL schema hash +> `sha256:28413c4e9b31`. -## Problem and expected outcome +## Problem and overall expected outcome Package changelogs are often missing or too abstract to explain an upgrade. -GitHits should expose PkgSeer's authoritative raw `codeDiff` evidence through: - -- `githits code diff` for humans and shell automation; and -- `code_diff` for agents. - -Both surfaces must compare the exact resolved `from` and `to` trees, return -bounded source-file evidence, distinguish missing content from an empty diff, -and preserve enough exact identity for `code_read` follow-ups. They must not -claim compatibility, safety, or semantic impact that the raw patch does not -prove. - -The completed effort also lets package-changelog responses steer callers to -the stable diff operation through typed data. Structural/symbol diff remains a -separate, unexposed backend capability. - -## Verified current state - -### Repository state - -- The working branch was rebased on 2026-08-17 against `origin/main` at - `fa27604`. -- The CodeDiff client adapter now exists; no CLI command, MCP tool, response - formatter, or changelog action consumer exists in the current GitHits tree. -- `CodeNavigationService` owns source navigation and is the verified service - boundary for the new operation. CLI commands live under `src/commands/code/`; - MCP tools and shared request/response modules live under - `packages/mcp/src/tools/` and `packages/mcp/src/shared/`. +GitHits needs a bounded source diff that compares the exact resolved `from` +and `to` trees and remains truthful about package scope, truncation, omitted +content, and failures. + +The rollout is deliberately staged: + +1. land the transport-neutral CodeDiff client; +2. expose `githits code diff` as a normally registered but unpromoted CLI + dogfood surface; +3. use dogfood evidence to stabilize the contract before adding `code_diff`, + MCP instructions, or agent guidance; and +4. later consume typed changelog steering when PkgSeer publishes that + contract. + +The completed effort gives humans and agents the same authoritative raw diff +facts without claiming compatibility, upgrade safety, semantic impact, rename +identity, or any other conclusion the raw evidence cannot prove. Structural +CodeDiff remains a separate, unexposed backend evaluation surface. + +## Verified current state and evidence + +### GitHits repository + +- Phase 1 merged through PR #287 at `origin/main` `77417aa`. The merged delta + adds `CodeNavigationService.codeDiff`, exact request/result/error types, + mode-minimal GraphQL selections, runtime validation, fixtures, public client + exports, and `docs/implementation/code-diff.md`. +- The declaration-build pipeline failure was fixed before merge. Phase 1's + focused tests, full tests, build, and public-package validation passed. +- No `githits code diff` command, `code_diff` MCP tool, response formatter, + MCP instruction, Agent Skill guidance, plugin asset, or changelog action + consumer exists on `origin/main`. +- `CodeNavigationService` is already available through the CLI container and + its mock factory. Existing indexed `code` commands keep Commander and + terminal adaptation under `src/commands/code/` while reusing pure request, + response, text, and error helpers through the workspace-only + `@githits/mcp/internal` boundary. +- `githits resolve` is the verified silent-rollout precedent: it is a normally + registered, documented CLI command backed by workspace-internal pure + helpers, but it has no MCP tool or Agent Skill promotion. - Root `githits` is version `0.9.2`; public `@githits/mcp` is version `0.9.1`. - User-visible implementation will require one independent changelog fragment - with pending impacts for both artifacts. Version bumps happen during release - preparation, not feature implementation. -- `CodeNavigationService` and `CodeNavigationServiceImpl` are publicly - re-exported through `@githits/mcp/client`, and `CodeNavigationService` is part - of public `McpToolServices`. Phase 1 therefore adds public client API even - though it adds no MCP tool or CLI command. Custom service implementations - must add the required method when adopting that package version. + A new CLI command affects `githits` only unless implementation changes a + public MCP export or behavior. +- Phase 2 implementation began from `origin/main` at `77417aa`; subsequent + branch commits contain only the silent CLI dogfood delta described here. ### Backend contract -PkgSeer backend `main` replaced GitHub Compare with credential-free aigrep -RawDiff and transferred the lasting contract into its repository-internal -CodeDiff implementation guide. - -The committed GraphQL schema and implementation now prove: - -- `codeDiff` resolves package versions or repository refs to full immutable - commit SHAs before raw execution. -- Raw compares the two exact trees directly. Forward, reverse, and diverged - pairs share the same directional contract; identical SHAs perform no raw - subprocess work. -- A broad authoritative inventory is filtered by verified publication scope, - optional `pathPrefix` and `pathGlob`, then relevance-sorted before - `maxFiles`. The old provider-order and 300-file upstream cap blockers are - gone. -- `RawCodeDiffSummary` reports scoped counts before projectability and - `maxFiles`: `filesChanged`, added/deleted/modified, mode/type changes, - `inventoryComplete`, and `unprojectableFiles`. -- `hasMoreFiles` reports additional projectable files after `maxFiles`; it does - not include overlong unprojectable paths. -- Selection controls work. Inventory requires one raw call; selecting stats or - patches performs one additional exact-path call only for retained files. -- Content truth is typed independently through `contentCoverage`, optional - `contentFailure`, and per-file `contentStatus` / - `contentOmissionReason`. Paths also expose `pathEncoding` and - `contentSafety`. -- Backend bounds/defaults are `maxFiles=50` within `1..300` and - `maxPatchBytes=262144` within `1024..2097152`, with a fixed 131072-byte - per-file patch ceiling. Numeric values are clamped by the backend; the client - must reject out-of-range values instead of silently changing agent input. -- `pathPrefix` and `pathGlob` are independently optional, compose by - intersection, reject explicit empty values, and have 1024-byte limits. -- First-inventory failures are GraphQL field errors. Failures after an - authoritative inventory remain data under `contentFailure`, preserving the - inventory and selected file identities. -- Raw file status is only `ADDED`, `DELETED`, or `MODIFIED`. The contract does - not expose rename identity, so the client must not infer or advertise - renames. - -The local backend checkout matches its `origin/main`, but deployment of this -exact schema to the development and production GraphQL endpoints has not been -verified in this reorientation. The schema changelog still labels the -replacement change as pending. The user reported on 2026-08-17 that a backend -GraphQL documentation round is still needed. Phase 1 can use the committed SDL -and implementation contract; if that round changes fields or semantics rather -than descriptions, reorient before implementation continues. - -### Contradictions retired from the old proposal - -| Old proposal statement | Current verified contract | -|---|---| -| GitHub `BASE...HEAD` cannot produce reverse patches | Exact-tree RawDiff supports forward, reverse, and diverged transformations | -| GitHub exposes at most 300 upstream files | Authoritative inventory has no GitHub Compare file window | -| Upstream ordering can consume the file/patch budget | PkgSeer applies publication scope and relevance before caller limits | -| Counts are repository-wide before caller scope | Summary counts are recomputed after publication and caller scope | -| `compareUrl`, direction counts, previous rename path, and patch-omission enums are available | Those fields were removed; use exact resolutions, content coverage/failure, content status, and omission strings | -| Only patch/stat views need consideration | Backend has distinct Inventory, Stats, and Patches execution modes | +PkgSeer `origin/main` now contains the completed GraphQL documentation round. +The SDL and implementation documentation prove: + +- `codeDiff` accepts exactly one complete addressing form: package + `registry/name/fromVersion/toVersion` or public GitHub repository + `repoUrl/fromRef/toRef`. +- Both endpoints resolve to full immutable commit SHAs before raw work. Forward, + reverse, diverged, and identical pairs use the same direct tree-to-tree + contract; identical SHAs perform no raw subprocess work. +- GraphQL selection determines Inventory, Stats, or Patches work. Phase 1 maps + those shapes to `inventory`, `stats`, and `patches` service modes and proves + their selected fields at the wire boundary. +- `pathGlob` and `pathPrefix` are repository-relative even for package scope. + Package publication evidence restricts the inventory but does not rewrite + returned paths or caller filters to package-relative values. +- `pathGlob` supports a bounded subset: `*` and `?` stay within a component and + an exact `**` component spans components. A backslash escapes one following + non-slash character. Unsupported shell/pathspec syntax is rejected. Only one + glob is accepted per request. +- Server defaults are `maxFiles=50` and `maxPatchBytes=262144`; each patch also + has a fixed 131072-byte ceiling. The server clamps numeric options, but the + CLI must reject out-of-range user values rather than silently altering them. +- `summary`, `hasMoreFiles`, and `contentCoverage` answer different questions. + `summary` covers the full filtered inventory before projectability and + `maxFiles`; `hasMoreFiles` covers omitted projectable paths; content coverage + covers only returned file evidence. +- Content outcomes distinguish inventory-only, stats, patch, binary, + metadata-only, omitted, and unavailable rows. A failed post-inventory content + phase remains data with an authoritative inventory; a first-inventory or + resolution failure is a typed GraphQL error. +- The documentation round was description/contract clarification plus tests; + it did not replace Phase 1's GraphQL shapes. The later correction at + `1388b42` clarified repository-relative path identity and is consistent with + this plan. + +Deployment of schema hash `sha256:28413c4e9b31` to the development endpoint has +not been authenticated or introspected during this replan. That is an external +readiness check due before Phase 2 is represented as live-validated, not a +blocker for isolated implementation and fixtures. + +### Reorientation contradiction and resolution + +The merged plan still grouped CLI, MCP, instructions, plugin assets, parity, +and agent evaluation into one public Phase 2. That conflicts with the user's +2026-08-17 decision to launch the CLI silently, dogfood it like `resolve`, and +leave the MCP tool and instructions out. This revision splits those outcomes +into separate phases. Do not implement the old combined Phase 2. ## Scope -### In scope +### Overall scope -- Package addressing for all registries accepted by the existing code-target - parser, when PkgSeer can resolve both published versions to public GitHub +- Package comparisons for registries already accepted by the code-target + parser when PkgSeer can resolve both published versions to public GitHub source. -- Public GitHub repository addressing with explicit `from` and `to` refs. -- Raw inventory, stats, and patch projections, subject to the Phase 2 view - decision. -- Prefix/glob filtering, file and patch budgets, compact text, structured JSON, - typed errors, CLI/MCP parity, smoke coverage, and agent evaluation. -- Later consumption of a typed package-changelog diff action once PkgSeer - publishes that action contract. +- Public GitHub repository comparisons with explicit base and target refs. +- Authoritative raw Inventory, Stats, and Patches projections. +- Repository-relative glob filtering, file and patch budgets, exact resolved + identity, pipe-friendly terminal output, structured JSON, typed errors, and + truthful completeness signals. +- CLI-only dogfooding before MCP/agent exposure. +- Later MCP parity and typed changelog steering after their dependencies are + verified. -### Non-goals +### Overall non-goals - Public structural/symbol diff. -- Compatibility, upgrade-safety, API-stability, or semantic-version verdicts. -- Private repositories, local worktrees, proprietary code, or uncommitted - changes. -- Client-side ranking, patch inversion, rename detection, patch synthesis, or - fallback to a hosted compare API. -- A client cache, queue, lock, concurrency gate, retry layer, or feature flag. -- Reproducing Git's full revision or pathspec language. - -## Target architecture +- Compatibility, safety, API-stability, or semantic-version verdicts. +- Rename/copy detection or compare-era ahead/behind/diverged relationships. +- Working-tree, staged, local-path, private-repository, or arbitrary Git host + diff. +- Full Git revision grammar, three-dot merge-base semantics, multiple + pathspecs, pathspec magic, exclusions, attributes, or unbounded output. +- Automatic indexing, background rollout machinery, feature flags, telemetry + infrastructure, or persistent storage. +- Changelog action parsing before PkgSeer commits the typed action contract. + +## Target architecture and end state ### Boundaries and responsibilities -`packages/core-internal` owns the transport-neutral contract: - -- Add explicit CodeDiff request/result/error types and - `CodeNavigationService.codeDiff(params)`. -- Build the GraphQL variables from one normalized package or repository target. -- Use a mode-specific selection for inventory, stats, and patches. Never select - structural fields. Patch mode selects patch fields; stats omits patch and - patch-only fields; inventory omits per-file `additions`, `deletions`, and - patch content. The authoritative summary's added/deleted file counts remain - selected in every mode. -- Validate the external response with Zod and preserve backend enum/string - values needed to represent content truth. -- Handle root errors separately from `raw` field-local errors. A field-local - raw error must carry parsed GraphQL extensions plus the successfully returned - package/from/to resolutions; ordinary CodeNavigation methods may keep their - existing fail-on-any-error behavior. - -`packages/mcp` owns public request and response behavior: - -- A dedicated diff-target builder reuses existing package/repository parsers - but rejects target-embedded package versions and repository refs because the - comparison endpoints own both identities. -- Shared pure modules normalize view, filters, explicit defaults, and error - envelopes once for CLI/MCP parity. -- A data-first camelCase JSON envelope exposes exact resolutions, scoped - summary, scope, content coverage/failure, files, `hasMoreFiles`, and only the - effective/caller-supplied filters needed to interpret truncation. -- Compact text shows the requested range and exact resolved SHAs once, then - scoped summary, scope warning if unknown, relevance-ordered files, selected - stats/patches, and only actionable recovery guidance. -- MCP validation failures and service errors remain JSON error envelopes in - every requested format. - -The root CLI owns Commander syntax and terminal error adaptation. The MCP -server owns tool registration, annotations, descriptions, instructions, and -public package exports. Both call the same shared builders and formatters. - -### Data flow +- `CodeNavigationService.codeDiff` remains the only network/service boundary. + It owns authenticated GraphQL execution, exact variable construction, + mode-minimal selections, runtime validation, and typed CodeDiff errors. +- Small pure CodeDiff request, response, text, and error helpers live under + `packages/mcp/src/shared/` and are exported only through + `packages/mcp/src/internal.ts` during CLI dogfooding. This follows the + existing `resolve` pattern and lets a later MCP adapter reuse an exercised + contract without exposing a public `@githits/mcp` API prematurely. +- `src/commands/code/diff.ts` owns Commander syntax, CLI-native validation + wording, dependency acquisition, spinner lifecycle, stdout/stderr routing, + JSON serialization, and process exit behavior. It does not construct + GraphQL or duplicate backend response mapping. +- Phase 3 adds a thin MCP `code_diff` adapter only after Phase 2 evidence is + reviewed. It reuses the pure contract, adds agent-native descriptions and + errors, and introduces no second CodeDiff service path. +- Changelog steering later produces typed calls to the stable CLI/MCP + invocation; it never parses human changelog prose for suggestions. + +The root CLI may import `@githits/mcp/internal`. Public packages and future +remote MCP servers must not. No new module from Phase 2 is exported by +`@githits/mcp`, `@githits/mcp/client`, or `@githits/mcp/smoke-test`. + +### Data flow during silent dogfood ```text -CLI args / MCP input - -> shared target, endpoint, view, and option normalization +CLI target + from..to + view/filter/bounds + -> pure CLI request normalization -> CodeNavigationService.codeDiff - -> mode-minimal GraphQL selection of codeDiff.raw - -> Zod-validated transport result or typed partial/root error - -> shared data-first envelope - -> CLI text/JSON or MCP text-v1/JSON + -> mode-minimal codeDiff.raw GraphQL selection + -> typed CodeDiff result or error + -> pure selected-view JSON/text projection + -> primary view on stdout + diagnostics on stderr +``` + +Phase 3 later adds MCP input/output adapters on either side of the same pure +normalization and projection modules. + +### CLI dogfood contract + +The initial syntax is: + +```text +githits code diff .. [options] [-- ] +githits code diff --repo-url .. [options] [-- ] ``` -### Failure behavior - -- Invalid target/range/filter/budget input returns `INVALID_ARGUMENT` without a - network request. -- Shared resolution errors retain backend `code`, `retryable`, side, retry - delay, publication/ref candidates, and ambiguity kinds when supplied. -- Raw first-call failures preserve partial root resolution data when GraphQL - returned it. Codes such as `RAW_DIFF_LIMIT_EXCEEDED`, - `RAW_DIFF_UNAVAILABLE`, `RATE_LIMITED`, `TIMEOUT`, and `UPSTREAM_ERROR` must - not collapse when they imply different recovery. -- A post-inventory `contentFailure` is a successful partial evidence result, - not a thrown request error. Text and JSON must show that inventory is - authoritative while requested content is partial or failed. -- `UNKNOWN` package scope is visibly repository-wide. Empty scoped inventory - is not described as missing evidence when `inventoryComplete=true`. -- `BYTE_ESCAPED` paths remain evidence but cannot be reused as if they were an - exact UTF-8 path. Follow-up hints are emitted only for stable UTF-8 paths. - -## Assumptions and open decisions - -### Verified assumptions - -1. `code` remains the correct command/tool family because the output is source - evidence and composes with `code_read`. -2. Raw remains read-only from the caller's perspective and does not enqueue - indexing. MCP annotations can therefore use the existing read-only shape. -3. The exact-tree schema on backend `main` is the implementation target; no - compatibility adapter for the removed GitHub Compare schema will be added. -4. Structural exposure remains independently blocked by backend quality and - uncertainty work documented in PkgSeer's implementation guide. - -### Resolved product direction (2026-08-17) - -1. **CLI addressing:** use the compact - `githits code diff ..` form for packages and repositories, - with paired `--from` / `--to` only as the delimiter escape hatch. -2. **Public views:** expose Inventory, Stats, and Patches behavior. Design the - names, defaults, flags, output, and option interactions to match `git diff` - expectations as closely as the backend can truthfully support; validate the - ergonomics with agents rather than treating the provisional - `inventory | stat | patch` names as final. -3. **Path scoping:** when a caller supplies path scope, make backend `pathGlob` - the primary public mechanism. An omitted path filter still means the whole - resolved package/repository scope. Retain `pathPrefix` only when it provides - a materially clearer exact-subtree escape hatch. - -The backend glob is not a Git pathspec. It accepts one pattern, keeps `*` and -`?` within one component, and lets an exact `**` component span directories. -It rejects character classes, brace alternatives, negation, empty components, -and multiple patterns. CLI design should investigate a familiar -`-- ` position after the range, but help/tool descriptions must call -the value a bounded glob and document the difference rather than claiming full -Git pathspec compatibility. MCP should use `path_glob` as the primary field. - -The exact client defaults for returned files and cumulative patch bytes are -not a product decision yet. Start with 20 files and 32 KiB as evaluation -hypotheses only. Phase 2 must measure them against the backend's relevance -ordering and select the smallest useful defaults before public descriptions or -tests pin them. - -### External unknowns - -- Whether the exact schema is deployed to the development endpoint. Evidence: - authenticated schema/introspection or representative dev calls. Due before - Phase 2 live smoke; lack of deployment does not block the isolated Phase 1 - adapter and fixtures. -- Whether the backend GraphQL documentation round is description-only. - Evidence: inspect the resulting SDL hash/changelog and implementation diff. - Due before Phase 1 merges if the round lands while Phase 1 is open; any shape - or semantic change requires immediate reorientation. -- The future PkgSeer changelog action discriminator and argument fields. Due - before Phase 3; resolve from the committed backend schema, not prose. - -### Overall dependencies - -- The exact-tree PkgSeer GraphQL contract remains stable through Phase 1 and is - deployed before Phase 2 live validation. -- Phase 2 follows the transport-neutral Phase 1 API and the resolved Git-like - product direction; Phase 3 follows the stable Phase 2 invocation and a - committed backend changelog-action contract. -- Existing CodeNavigation authentication, request-header, content-safety, - smoke, plugin-generation, and package-boundary workflows remain the shared - infrastructure. This effort does not replace them. +- `` accepts an unversioned package target such as `npm:express` or an + existing compact public GitHub target. `--repo-url` remains available for + consistency with the other `code` commands. +- Versions/refs belong only in `..`. A version embedded in a package + target, a ref embedded in a repository target, `--git-ref`, an empty side, + more than one `..` separator, or `...` is rejected before network I/O. +- Direction is always left-to-right. Reverse comparison is expressed by + swapping the endpoints; no compare relationship or merge-base behavior is + inferred. +- The default view is patch output, matching ordinary `git diff`. Explicit + `-p`/`--patch`, `--stat`, `--name-only`, and `--name-status` are supported and + mutually exclusive. `--name-only` and `--name-status` both use the cheap + Inventory service mode; they differ only in the selected output projection. +- One value after `--` maps to backend `pathGlob` and is the primary public + path filter. It is called a repository-relative glob in help and errors, not + a full Git pathspec. Multiple values and unsupported pathspec syntax are + rejected. `pathPrefix` is not exposed during dogfooding; evidence must show a + distinct need before adding a second path control. +- `--max-files` is valid for every view. `--max-patch-bytes` is valid only for + patch view. Omitted values are omitted from the service request so dogfooding + measures the documented backend defaults rather than pinning a separate + client default. +- `--json` returns a data-first envelope containing normalized target identity, + public view, exact `from`/`to` resolutions, scoped summary, scope, + `contentCoverage`, optional `contentFailure`, selected file facts, and + `hasMoreFiles`. Every file keeps `pathEncoding` so byte-escaped display paths + cannot be mistaken for reusable identities. Beyond that safety fact, file + objects contain only facts selected for the view: path-only for `name-only`, + path/status for `name-status`, line counts and content status for `stat`, and + bounded patch/omission/safety facts for patch. +- Default text keeps stdout compatible with the chosen Git-style view: + patches for patch mode, bare paths for `--name-only`, status plus path for + `--name-status`, and line-count rows plus a total for `--stat`. Resolution, + scope, truncation, unprojectable-path, safety, and incomplete-content + diagnostics go to stderr so piping the primary output remains useful. +- `--verbose` adds requested/resolved identities, full summary, effective + scope/filter, returned count, and content coverage to terminal output. JSON + already carries these facts and does not change shape under `--verbose`. +- No rename wording is emitted. Binary and metadata-only changes explicitly + state that content differs without claiming a textual patch. Byte-escaped + paths are marked display-only and are never suggested as exact follow-up + identities. + +### Failure and exit behavior + +- Empty authoritative diffs exit 0, matching normal `git diff` behavior. +- Client validation, authentication, root/shared-resolution errors, and raw + field errors exit 1 and use the existing CLI JSON error envelope on stderr. +- CodeDiff-specific backend details are mapped into a bounded CLI error shape; + arbitrary GraphQL extensions and raw backend codes are not passed through. + Preserve safe side, published-version/ref candidates, retry timing, stage, + limit kind, repository identity, and any partial exact resolutions. +- `PARTIAL` or `FAILED` content coverage inside a successful result remains a + successful evidence envelope and exits 0 because the authoritative inventory + is usable. Text emits an unmistakable stderr warning; JSON retains structured + coverage/failure and per-file statuses. Dogfooding must revisit whether this + is suitable for agent automation before Phase 3. +- Terminal-visible backend text and paths use existing sanitization. Patches + use the backend's content-safety projection; the CLI does not restore removed + content or log patches. + +## Overall assumptions and unknowns + +### Assumptions + +1. Phase 1's public service signatures are sufficient for the CLI; dogfooding + should change normalization and presentation before changing that boundary. +2. Git-like syntax means matching familiar behavior where backend semantics + are equivalent, not accepting Git flags or pathspecs that would be ignored + or approximated. +3. Silent rollout means normal CLI registration, help, implementation docs, + smoke coverage, and a release fragment, while omitting MCP registration, + MCP instructions, Agent Skills, plugin guidance, and proactive agent + evaluation. +4. Manual dogfood notes and existing debug facilities are sufficient for this + phase. No feature flag, telemetry schema, counter, or rollout service is + authorized. + +### Unknowns and product decisions + +- **Development deployment:** whether the dev endpoint serves schema hash + `sha256:28413c4e9b31`. Resolve with authenticated introspection or a + representative CLI call before claiming authenticated live smoke passed. +- **Client defaults:** whether later public agent use needs file/patch limits + smaller than backend defaults. Resolve from Phase 2 dogfood evidence before + Phase 3 schema/descriptions pin defaults. +- **Agent view schema:** whether MCP should expose Git-like view names or the + service's Inventory/Stats/Patches names. Resolve after CLI dogfood and before + Phase 3 implementation. +- **Content-failure exit semantics:** Phase 2 deliberately keeps successful + authoritative inventory at exit 0. Reassess from shell and automation + dogfooding before agent exposure. +- **Changelog action shape:** discriminator, fields, and placement do not yet + exist as a committed client contract. Resolve from future PkgSeer SDL and + implementation before the changelog-steering phase. + +There is no blocking product decision for Phase 2. ## Cross-cutting considerations ### Security and privacy -- Never print, persist, or place credentials in command output, fixtures, - plans, telemetry, or tool-call evidence. Live validation may use existing - process-local authentication only. -- Keep public-Git scope explicit. Do not accept credential-bearing repository - URLs or weaken the existing repository parser. -- Treat paths and patches as untrusted. Respect backend `contentSafety`, keep - formatter-owned layout separate, and do not invent exact-path hints after - identity-changing normalization. - -### Performance and data minimization - -- Mode-minimal GraphQL selections are part of the contract and require wire - tests. Inventory must not request stats or patches; stat must not request - patches. -- Evaluate response bytes, model tokens, materially relevant patched files, - end-to-end time, and agent follow-up quality before fixing client defaults. - Use release builds for any reported performance numbers. -- Do not add client-side ranking. Backend relevance runs over authoritative - scoped inventory before bounds and is the correct ownership boundary. +- Only already-supported public package and public GitHub addressing is + accepted. Existing URL parsing must continue rejecting credential-bearing or + path-bearing repository URLs rather than silently canonicalizing them. +- Do not expose, print, persist, fixture, or record credentials. Authenticated + smoke may use an existing credential only through the normal client path and + must not print its value. +- Dogfood notes may record target/version pairs, aggregate sizes, latency, and + outcome categories, but never patches, file bodies, tokens, or arbitrary + backend failure prose. +- Preserve content-safety and terminal-sanitization behavior in every text and + JSON path. + +### Performance and data fetching + +- Each CLI call makes one CodeDiff GraphQL request through the existing + service. No local Git checkout, GitHub Compare fallback, cache, queue, or + background indexing is introduced. +- View choice must reach the service unchanged so Inventory never selects line + counts and Stats never selects patches or omission reasons. Existing Phase 1 + wire tests remain the authority for this boundary. +- Omitted limits stay omitted. Dogfood observation is not a performance claim + or optimization benchmark; any later optimization requires the repository's + benchmark-first workflow. ### Compatibility, release, and rollback -- Phase 2 changes both public artifacts and therefore adds one changelog - fragment with pending SemVer impacts for `githits` and `@githits/mcp`. -- Use the repository-internal `githits-plugin-maintenance` skill for MCP - instructions, root Agent Skill, plugin/extension manifests, and generated - assets. Edit canonical inputs, run `bun run plugins:generate`, then - `bun run plugins:check`. -- Rollback is the previous CLI/MCP package version; this feature writes no - persistent data and needs no cleanup or migration. +- Phase 2 adds a root CLI command and therefore adds one independent fragment + with pending impacts `githits: minor` and `@githits/mcp: none`. +- Phase 2 does not bump versions directly. Release preparation owns package and + generated-manifest versions. +- Workspace-internal shared modules must not appear in public export maps or + declarations reachable through `@githits/mcp`, `@githits/mcp/client`, or + `@githits/mcp/smoke-test`. +- The feature writes no state and needs no migration. Rollback is the previous + root CLI release; no cleanup is required. ### Durable documentation -Implementation starts a focused CodeDiff document in Phase 1 and updates -`docs/implementation/tools.md`, -`docs/implementation/cli-commands.md`, MCP/CLI parity documentation where the -new shared contract matters, and a focused CodeDiff implementation document. -The durable document must explain exact-tree identity, scope/summary semantics, -content coverage/failure, view-minimal fetching, and non-goals. It must not -copy transient rollout detail from this plan. +- Update `docs/implementation/code-diff.md` with the exercised CLI projection, + exact-tree identity, repository-relative glob semantics, output/failure + contract, and current absence of an MCP tool. +- Update `docs/implementation/cli-commands.md` with command examples, piping, + view flags, JSON shape, warnings, exit behavior, and the silent dogfood + posture. +- Do not edit MCP instructions, `docs/implementation/tools.md`, root Agent + Skills/guidance, generated plugin assets, or `CHANGELOG.md` in Phase 2. ## Phase map | Phase | Status | Outcome | |---|---|---| -| 1. Transport-neutral CodeDiff adapter | Complete | Typed exact-tree request/result/error support in `CodeNavigationService`, with no public command/tool yet | -| 2. Public CLI/MCP raw diff | Design refinement pending; no product blocker | Git-like shared request/response rendering, validated defaults, `githits code diff`, and `code_diff` with tests, smoke, docs, assets, and release fragment | -| 3. Changelog steering | Blocked on backend action contract and Phase 2 | Consume typed PkgSeer changelog actions and render context-appropriate diff calls | +| 1. Transport-neutral CodeDiff adapter | Complete and merged | Typed exact-tree request/result/error support with no CLI command or MCP tool | +| 2. Silent CLI dogfood | Implemented, verified, and reviewed; merge pending | Git-like `githits code diff`, CLI-only tests/smoke/docs/release fragment, and initial live dogfood evidence with no agent exposure | +| 3. MCP and agent rollout | Blocked on Phase 2 evidence | Stable `code_diff`, CLI/MCP parity, instructions, assets, smoke, and agent evaluation | +| 4. Changelog steering | Blocked on backend action contract and stable Phase 3 invocation | Typed sparse-changelog actions that point to valid CLI/MCP diff calls | | Structural track | Out of scope / backend-blocked | Separate future proposal only after backend says structural evidence is externally safe | ## Phase 1: transport-neutral CodeDiff adapter -**Status:** complete. - -**Expected outcome:** `packages/core-internal` can request and validate all -three raw modes through `CodeNavigationService.codeDiff`, represent partial -content separately from field errors, and preserve exact root identity on raw -field errors. The method and types become public through -`@githits/mcp/client`; no MCP tool, CLI command, or user-facing default is -introduced. - -**Assumptions:** backend `main` exact-tree schema is the intended final client -contract; service callers choose an explicit mode; no compatibility with the -removed compare schema is required. - -**Unknowns/product decisions:** none for this client adapter. The deployment -state is not needed for isolated tests. - -**Dependencies:** current `CodeNavigationService`, shared request-header/token -provider behavior, the inspected PkgSeer backend `main` SDL, and existing -service mock factories. - -**Likely files:** - -- `packages/core-internal/src/services/code-navigation-service.ts` -- `packages/core-internal/src/services/code-navigation-service.test.ts` -- `packages/core-internal/src/index.ts` if public workspace exports require it -- `packages/mcp/src/client.ts` -- `packages/mcp/src/mcp/server.ts` for the descriptor-only service stub -- `packages/mcp/src/services/test-helpers.ts` -- `src/services/test-helpers.ts` -- `scripts/validate-public-packages.ts` -- `docs/implementation/code-diff.md` -- one independent `changes/*.added.md` fragment - -**Contracts and edge cases:** - -- Params use a normalized unversioned package/repository target plus explicit - `from`, `to`, raw mode, and optional raw bounds/filters. -- Package variables populate only `registry/name/fromVersion/toVersion`; - repository variables populate only `repoUrl/fromRef/toRef`. Empty opposite - fields are omitted, not sent as empty strings. -- Each mode has a test-visible minimal selection. Shared identity, summary, - scope, files, `hasMoreFiles`, and `contentSafety` remain selected where used; - content fields appear only in the mode that consumes them. -- Runtime schemas cover every committed enum and nullable field. Unknown - omission/failure strings remain bounded backend facts rather than closed - client enums unless the SDL defines an enum. -- Identical SHAs, empty authoritative inventory, unknown scope, moved package - roots, unprojectable paths, binary/metadata-only files, byte-escaped paths, - partial/failed content, and field-local raw errors all have fixtures. -- Field-local errors preserve root data; root/shared-resolution errors do not - fabricate a result. - -**Ordered implementation:** - -1. Add service-level request/result/error interfaces and update both mock - factories first so compilation exposes every consumer. -2. Add Zod schemas for CodeDiff root, raw modes, GraphQL extensions, and partial - responses. Keep them local to the service until a second consumer proves a - shared schema is needed. -3. Add mode-minimal query selections and exact variable construction. Prefer - the smallest explicit query construction that lets tests assert selected - fields; do not introduce a general GraphQL query builder. -4. Implement `codeDiff` using the existing authenticated request boundary and - a dedicated mapper for CodeDiff-specific partial/root errors. -5. Add focused async service tests for variables, selections, normalization, - malformed responses, root errors, raw field errors with root data, and - post-inventory failure-as-data. -6. Document the client adapter's exact-tree identity, mode-minimal fetching, - partial/error contract, public export boundary, and current lack of a CLI or - MCP tool. Add a changelog fragment with `githits: none` and - `@githits/mcp: minor`; the required interface method is a public client API - addition and custom service implementations must adopt it. -7. Run focused tests, the full `bun test`, `bun run build`, and - `bun run validate:packages`. Inspect built declarations and packed artifacts - to ensure private `@githits/core-internal` names do not leak. - -**Acceptance criteria:** - -- All three modes produce exact variables and minimal selections proven by - wire-level tests. -- Every schema field needed by later text/JSON consumers survives typed - normalization without compare-era fields. -- Partial content remains successful evidence; field-local and root errors are - distinguishable and retain all safe actionable metadata the backend sent. -- Existing navigation behavior and error mapping are unchanged. -- Durable documentation and the changelog fragment state the public client API - impact without advertising an unimplemented tool or command. -- Focused tests, `bun test`, `bun run build`, and - `bun run validate:packages` pass. - -## Phase 2: public CLI/MCP raw diff - -**Status:** product direction resolved; detailed ergonomics must be validated -after Phase 1 and before the public schema/help is pinned. - -**Expected outcome:** users and agents can invoke the same exact-tree raw diff -through `githits code diff` and `code_diff`, with compact truthful output, -lossless selected JSON, useful defaults, and complete local/built/live -validation. - -**Assumptions:** Phase 1 result types remain sufficient; the deployed dev -schema matches the committed SDL before live smoke; the selected public modes -map directly to backend selection modes. - -**Unknowns/product decisions:** no remaining product blocker. Exact Git-like -view names/flags and the single-glob CLI spelling require usability design; -client file/patch defaults require workload evidence. Both are due before the -public schema, descriptions, and tests are finalized. - -**Dependencies:** completed Phase 1; deployed development backend for final -smoke; plugin-maintenance workflow; current smoke/eval harnesses. - -**Likely files/components:** - -- shared `code-diff-request.ts`, `code-diff-response.ts`, and - `code-diff-text.ts` modules with focused tests; -- `packages/mcp/src/shared/code-navigation-error-map.ts` for CodeDiff error - envelope mapping; -- `packages/mcp/src/tools/code-diff.ts`, tool index/server registration, - instructions, public types/exports, and test helpers; -- `src/commands/code/diff.ts`, command registration/help, CLI helpers, and - parity tests; -- smoke-test tool inventories, CLI/MCP smoke scripts, targeted agent workloads, - canonical Agent Skill/guidance, generated plugin assets, implementation - docs, and one `changes/*.added.md` fragment. - -**Behavior and constraints:** - -- MCP input is `target + from + to`, not the backend's coupled flat XOR groups. - The target may be compact or structured but must not embed an endpoint. -- The normal CLI begins with `githits code diff ..`. View and - path controls should borrow familiar Git spelling where semantics align. - Familiar spelling must not imply unsupported Git pathspec, revision, rename, - or unbounded-output behavior. -- `path_glob` is the primary MCP path filter. Evaluate one CLI glob after `--` - as the closest honest match to `git diff ... -- `; do not accept - multiple values or unsupported pathspec magic unless the backend contract - gains equivalent semantics. -- CLI and MCP reject empty endpoints, mixed/partial range forms, explicit empty - filters, invalid glob syntax, out-of-range numeric values, and no-op option - combinations before network I/O. `maxPatchBytes` is valid only for the patch - view; other views reject it instead of accepting an ignored coupled option. -- JSON is a stable data-first projection, not a raw GraphQL wrapper. It retains - selected content truth and exact full SHAs without prose-only facts. -- Text shows exact range direction as `from -> to`; it does not recreate the - removed AHEAD/BEHIND/DIVERGED model. Equal full SHAs are labeled identical. -- Summary counts describe the complete scoped inventory. Returned-file counts, - `hasMoreFiles`, and `unprojectableFiles` remain separate. -- Patch omission actions depend on `contentStatus`, omission reason, and - coverage. Raising total bytes is never suggested for a fixed per-file, - binary, metadata-only, invalid-UTF-8, or transport failure. -- No rename wording appears because the backend does not preserve rename - identity. - -**Ordered implementation:** - -1. Reorient against Phase 1 types and the completed backend documentation - round. Build small CLI/MCP ergonomics fixtures for common `git diff` - expectations, then pin only the spellings whose semantics match. -2. Add pure target/range/view/filter normalization tests, then implement the - shared request builder. Reuse existing parsers without loosening other code - tools. -3. Add data-first response and text fixtures covering every scope, summary, - coverage, failure, content status, path encoding, truncation, identity, and - error state; then implement the smallest formatter modules that satisfy - them. -4. Add MCP and CLI adapters, registrations, read-only annotations, JSON parity, - and surface-native hints. Extend the internal closed `MappedErrorCode` union - and exhaustive mapping switch for `CodeDiffError`: map backend details to a - closed recovery category rather than passing `details.code` through, while - preserving publication/ref candidates and retry timing in the bounded error - details. This is internal error mapping, not a public API change. -5. Before pinning defaults, run representative small patch, medium minor, - large major, non-root monorepo, root-workspace, unknown-scope, +**Status:** complete and merged through PR #287. + +**Expected outcome:** the transport-neutral service can request and validate +exact-tree raw CodeDiff Inventory, Stats, and Patches without exposing a CLI or +MCP surface. + +**Assumptions:** the committed exact-tree backend contract is the client +target; no compatibility adapter for the removed GitHub Compare shape is +needed. + +**Unknowns/product decisions:** none remaining for this completed phase. + +**Dependencies:** completed backend exact-tree SDL and existing authenticated +CodeNavigation request boundary. + +**Observed outcome and acceptance:** mode-minimal variables/selections, runtime +schemas, partial/root error separation, fixtures, mock factories, public client +exports, durable documentation, release fragment, focused tests, full tests, +build, and public-package validation merged. No user command/tool or agent +guidance was added. + +## Phase 2: silent CLI dogfood + +**Status:** implemented, verified, and reviewed; merge pending. Authenticated +package smoke against `npm:express` `5.2.0..5.2.1` succeeded; broader +representative dogfooding remains the post-merge gate before Phase 3. + +**Expected outcome:** developers and deliberate CLI users can run an +authoritative exact-tree raw diff through `githits code diff` with familiar +Git-style views, pipe-friendly text, structured JSON, and truthful bounded +evidence. Agents receive no new tool, instructions, suggestions, or packaged +guidance. + +**Assumptions:** Phase 1 types remain sufficient; existing code-target parsing, +container injection, auth handling, sanitization, spinner, and CLI error +envelopes can be reused without changing their unrelated behavior; backend +defaults are acceptable evaluation defaults but are not yet endorsed for agent +use. + +**Unknowns/product decisions:** none blocking implementation. The development +deployment, later client defaults, Phase 3 MCP view names, and final +content-failure automation semantics remain evidence questions described +above. + +**Dependencies:** current `origin/main`; merged Phase 1; committed backend SDL +and CodeDiff guide; existing CLI container and smoke harness. Authenticated dev +deployment is required only for authenticated live validation. + +### Likely files and components + +- `packages/mcp/src/shared/code-diff-request.ts` and focused tests for target, + range, view, glob, and numeric normalization; +- `packages/mcp/src/shared/code-diff-response.ts` and focused tests for + selected-view success/error envelopes; +- `packages/mcp/src/shared/code-diff-text.ts` and focused tests for + pipe-friendly views and diagnostic facts; +- a small CodeDiff-specific error mapper beside those modules if the existing + generic code-navigation mapper cannot preserve the required bounded details + without broadening unrelated error behavior; +- `packages/mcp/src/internal.ts` for workspace-only exports, with no public + export-map or package-index change; +- `src/commands/code/diff.ts` and `src/commands/code/diff.test.ts`; +- `src/commands/code/index.ts` registration and code-group help; +- `scripts/cli-smoke.ts` structural command/help/auth/JSON coverage; +- `docs/implementation/code-diff.md`, + `docs/implementation/cli-commands.md`, and one new + `changes/*.added.md` fragment. + +Do not add or edit an MCP tool, MCP server registration, MCP instruction, +smoke-test tool inventory, Agent Skill, `AGENTS.md`, plugin/marketplace manifest, +plugin generator, or generated agent asset in this phase. + +### Contracts and edge cases + +- Parse package and repository forms into the Phase 1 target union without + own-key leakage from the opposite target shape. +- Accept exactly one non-empty `from..to` pair. Preserve endpoint spelling in + the request and exact resolved identity in the result. Identical endpoints + and equal resolved SHAs are valid; empty sides, whitespace-only sides, + `...`, or ambiguous separators are invalid. +- Reject versions/refs embedded in target syntax because comparison endpoints + own both identities. Reject `--git-ref` for the same reason. +- Map patch, stat, name-only, and name-status views to the minimal Phase 1 + service mode. Explicit conflicting view flags are invalid instead of relying + on Commander option order. +- Accept at most one non-empty repository-relative glob after `--`. Reject + multiple values, absolute paths, invalid encoding/length, and unsupported + backend glob syntax before network I/O. +- Validate `--max-files` against 1..300 and `--max-patch-bytes` against + 1024..2097152. Reject patch bytes outside patch view. Do not populate omitted + option keys. +- Cover package scope, repository scope, unknown scope, moved package roots, + empty inventory, identical SHA, reverse/diverged pairs, truncation, + unprojectable and byte-escaped paths, binary and metadata-only files, + content omissions, partial/failed content, safety modifications, root errors, + and raw field errors with partial resolutions. +- Keep summary counts separate from returned file count. Never infer that + `contentCoverage: COMPLETE` means the full inventory was returned. +- Preserve pipe-friendly stdout in every view and send non-primary diagnostics + to stderr. Tests must assert the two streams separately. + +### Ordered implementation + +1. Start from current `origin/main`. Add focused behavior fixtures for the + accepted CLI grammar and the four Git-style views, including rejected Git + syntax the backend cannot honor. +2. Write request-normalization tests, then implement the smallest pure builder + that produces `CodeDiffParams`. Reuse existing package/repository parsing + without loosening other `code` commands. +3. Write data-first response fixtures for all scope, summary, coverage, + content-status, safety, truncation, identity, and error states. Implement + selected-view envelopes and the bounded CodeDiff error mapping. +4. Write stdout/stderr formatter fixtures, then implement patch, stat, + name-only, name-status, verbose context, and actionable warnings. Do not + synthesize rename or compare-relationship facts. +5. Add the thin Commander action and registration. Use the existing container, + auth gate, spinner, color/sanitization, JSON error, and exit paths. +6. Extend CLI smoke with command/help registration, unauthenticated auth + handling, invalid local grammar, and built-product coverage. If an existing + credential is available without exposing it and dev serves the schema, run + representative package and repository calls; otherwise record the live + check as unavailable. +7. Dogfood representative small patch, medium release, large release, + non-root monorepo package, root-workspace package, unknown package scope, generated/lockfile-heavy, binary, reverse, diverged, and identical cases. - Compare 16/32/64 KiB and practical file caps using release builds. Record - aggregate counts, bytes/tokens, latency, content status, and agent follow-up - quality without persisting patches or credentials. -6. Pin defaults and descriptions from the evidence. Run targeted Claude and - Codex `agent:e2e` workloads when practical; inspect tool calls, - `toolIssues`, `instructionIssues`, follow-ups, and unsupported conclusions. -7. Update smoke inventories, durable docs, canonical guidance, and changelog - fragment. Use `githits-plugin-maintenance`, generate assets, and inspect the - complete diff. -8. Run `bun test`, `bun run build`, `bun run smoke:cli`, - `bun run smoke:mcp`, `bun run smoke:cli:built`, - `bun run smoke:mcp:built`, `bun run plugins:check`, and external package - export/declaration validation. Run authenticated dev smoke only if an - existing credential is available without exposing it; unauthenticated smoke - must still pass its auth-handling contract. - -**Acceptance criteria:** - -- Equivalent CLI/MCP calls send equivalent normalized service params and - produce the same JSON success/error facts. -- Inventory/stat/patch behavior matches the accepted public view set and each - view fetches no unused GraphQL fields. -- CLI addressing, view flags, `--` path placement, help, and errors feel - familiar to `git diff` users while explicitly rejecting unsupported - pathspec/revision behavior; agents select the intended view and filter in - targeted evaluations. -- Ordinary package upgrades return materially relevant bounded evidence under - the measured defaults; incomplete evidence is unmistakable and leads to a - valid focused follow-up. -- Reverse and diverged calls are direct exact-tree transformations, not - compare-era relationship claims. -- Scope, authoritative summary, returned/projectable counts, content coverage, - partial content failure, and per-file status are impossible to confuse in - text or JSON. -- All required unit, parity, smoke, built-product, plugin, package-boundary, - and agent evaluations pass. Any unavailable authenticated dev check is - reported explicitly rather than represented as passed. - -## Phase 3: changelog steering - -**Status:** blocked on Phase 2 and a committed backend action contract. - -**Expected outcome:** sparse/missing range changelogs carry typed package-only -diff arguments that GitHits maps to the active MCP or CLI syntax without -matching human prose. - -**Assumptions:** the action remains transport-neutral and package-addressed; -Phase 2 invocation is stable. - -**Unknowns/product decisions:** exact discriminator, field names, and success / -error placement. Resolve from the future SDL and backend implementation during -phase-boundary reorientation. - -**Dependencies:** deployed Phase 2 and committed/deployed PkgSeer action schema. + Record only target/version pairs and aggregate outcome/size/latency notes. + Evaluate view spelling, stdout piping, one-glob sufficiency, backend default + usefulness, warnings, and recovery behavior; add no instrumentation. +8. Update durable CLI/CodeDiff documentation and add the CLI-only release + fragment. Inspect the complete diff to confirm MCP instructions, tool + inventories, Agent Skills, and generated assets are unchanged. +9. Run focused tests, `bun test`, `bun run build`, `bun run smoke:cli`, + `bun run smoke:cli:built`, `bun run validate:packages`, formatting/lint + checks required by the changed files, and package-artifact inspection. Do + not report an unavailable authenticated smoke as passed. + +### Acceptance criteria + +- The documented package and repository examples normalize to exact Phase 1 + service params; invalid/mixed/empty forms fail before service invocation. +- Default patch, explicit patch, stat, name-only, and name-status output use the + intended minimal service modes and preserve Git-like stdout discipline. +- Omitted bounds are absent on the wire; explicit bounds and the single glob + are validated and forwarded exactly. +- Text and JSON cannot confuse full scoped summary, returned/projectable + files, content coverage, content failure, or per-file content status. +- Exact resolutions and safe recovery facts survive success and typed error + projections without arbitrary GraphQL extensions or unsafe terminal text. +- Empty and identical diffs succeed; real errors fail; successful partial or + failed content evidence succeeds with unmistakable structured/text warnings. +- CLI help states the bounded glob and revision limitations without claiming + full Git pathspec or revision compatibility. +- No `code_diff` tool, MCP instruction, agent guidance, plugin asset, or public + `@githits/mcp` export is added or changed. +- Durable docs describe the dogfood posture and one release fragment records + `githits: minor`, `@githits/mcp: none`. +- Focused tests, full tests, build, CLI smoke, built CLI smoke, package + validation, and relevant format/lint checks pass. Authenticated dev results + are recorded accurately. + +**Observed outcome and acceptance:** the command, pure request/result/error +adapters, Git-like text formatter, smoke coverage, durable documentation, and +release fragment are complete without MCP or agent-facing exposure. The full +suite passed 2,889 tests, build, built CLI smoke, format, lint, and public +package validation. Authenticated source smoke exercises a live package diff +and asserts that text patch headers contain the authoritative file path. +Preflight, internal runtime review, and the retained external Claude review are +clean after correcting delimiter enforcement, Git-like stat presentation, glob +grammar, bounded recovery lists, and CLI-native validation messages. Initial +post-review dogfooding exposed backend `a/file` and `b/file` patch placeholders; +the text formatter now binds them to the returned path and uses `/dev/null` for +added or deleted sides. + +## Phase 3: MCP and agent rollout + +**Status:** blocked on Phase 2 merge, dogfood evidence, and phase-boundary +reorientation. + +**Expected outcome:** the exercised raw-diff contract becomes a public +`code_diff` MCP tool with CLI/MCP JSON parity, agent-native errors and +descriptions, minimal instructions/guidance, generated assets, complete smoke +coverage, and targeted agent evaluation. + +**Assumptions:** dogfooding identifies a stable view/filter contract and useful +bounded defaults; no Phase 1 service redesign is required; structural remains +unexposed. + +**Unknowns/product decisions:** MCP view names, public defaults, content-failure +automation semantics, whether `pathPrefix` has proven necessary, and any CLI +signature corrections found during dogfooding. Resolve from Phase 2 evidence +before adding tactical Phase 3 steps. + +**Dependencies:** merged Phase 2; verified deployed backend; accepted dogfood +findings; MCP/package release workflow; plugin-maintenance workflow; smoke and +agent-evaluation harnesses. + +**Acceptance criteria:** equivalent CLI/MCP requests produce equivalent +normalized service params and JSON facts; every view remains mode-minimal; +agents choose correct views/filters and avoid unsupported conclusions in +targeted Claude and Codex evaluations; MCP/CLI smoke, built smoke, plugin +generation/checks, package validation, and required release fragments pass; +durable instructions remain concise and truthful. Tactical files and steps are +intentionally deferred until Phase 2 reorientation supplies evidence. + +## Phase 4: changelog steering + +**Status:** blocked on a stable Phase 3 invocation and a committed/deployed +PkgSeer changelog-action contract. + +**Expected outcome:** sparse or missing package range changelogs carry typed +package-only diff arguments that GitHits maps to the active CLI or MCP syntax +without matching human prose. + +**Assumptions:** the future action remains transport-neutral and +package-addressed; the stabilized diff invocation can represent it directly. + +**Unknowns/product decisions:** exact discriminator, field names, success/error +placement, and fallback behavior. Resolve from the future SDL and backend +implementation during phase-boundary reorientation. + +**Dependencies:** merged Phase 3 and committed/deployed PkgSeer action schema. **Acceptance criteria:** empty, partial, and missing-content range outcomes -steer to a valid CodeDiff call in both contexts; latest/repository outcomes and -unknown actions remain truthful; structural stays unexposed. Tactical files -and steps are intentionally deferred until the backend contract exists. +steer to valid CodeDiff calls in both contexts; latest/repository outcomes and +unknown actions remain truthful; structural stays unexposed. Tactical detail is +intentionally deferred until the backend contract exists. ## Phase-boundary reorientation -After each phase merges, fetch and rebase on current `origin/main`, inspect the -merged delta and current PkgSeer SDL/changelog/implementation guide, then update -this same plan before detailing or implementing the next phase. Record: +After each phase merges and before detailing or implementing the next phase: -- the merged outcome and tests actually passed; -- changed schema fields, assumptions, decisions, dependencies, or release - boundaries; -- contradictions between this plan, implementation, backend behavior, or user - comments; and -- whether the next phase is `READY`, `REPLAN`, or `PRODUCT INPUT NEEDED`. +1. run `$next-steps` against current `origin/main` and fetch the backend's + current `origin/main`; +2. inspect the merged delta, tests actually passed, SDL hash/changelog, + deployment evidence, and dogfood findings; +3. record changed assumptions, decisions, dependencies, release boundaries, + and contradictions in this same plan; +4. classify the next phase as `READY`, `REPLAN`, or `PRODUCT INPUT NEEDED`; and +5. add tactical detail only when its blocking evidence and product decisions + are resolved. -Do not carry forward compare-era fields, an unmeasured client default, or an -unverified changelog action shape. +Do not rebase or rewrite branch history unless explicitly requested. Do not +carry forward compare-era fields, unmeasured client defaults, or an unverified +changelog action shape. ## Overall acceptance, completion, and plan cleanup The effort is complete only when: - raw CodeDiff is released through CLI and MCP with exact-tree identity, - truthful scoped inventory/content states, and measured useful defaults; + truthful scope/inventory/content states, and evidence-backed defaults; - typed changelog steering is consumed in both CLI and MCP contexts; - structural remains honestly unexposed; -- durable docs match the implementation and no compare-era contract remains in - active guidance; and +- durable implementation docs match shipped behavior and no compare-era + contract remains in active guidance; and - all required unit, build, parity, smoke, built-product, package-boundary, - plugin, live-when-authenticated, and targeted agent validation has passed or - an unavailable authenticated check is explicitly reported. + plugin, and agent validations for the affected phases pass. -Keep this plan through final implementation review. Then transfer remaining -lasting decisions to `docs/implementation/`, delete this plan as the final -substantive cleanup, and search the repository for stale compare-era or plan -references. +Keep this plan through implementation review. After the final phase merges, +transfer all lasting architecture, contracts, operational findings, and +rollout decisions to `docs/implementation/`, then delete this temporary plan so +it cannot become stale guidance. diff --git a/packages/mcp/src/internal.ts b/packages/mcp/src/internal.ts index feea71b..747fffa 100644 --- a/packages/mcp/src/internal.ts +++ b/packages/mcp/src/internal.ts @@ -1,5 +1,8 @@ export * from "./index.js"; export { getMcpToolDefinitions } from "./mcp/server.js"; +export * from "./shared/code-diff-request.js"; +export * from "./shared/code-diff-response.js"; +export * from "./shared/code-diff-text.js"; export * from "./shared/code-navigation.js"; export * from "./shared/code-navigation-defaults.js"; export * from "./shared/code-navigation-error-map.js"; diff --git a/packages/mcp/src/shared/code-diff-path.ts b/packages/mcp/src/shared/code-diff-path.ts new file mode 100644 index 0000000..6b260cb --- /dev/null +++ b/packages/mcp/src/shared/code-diff-path.ts @@ -0,0 +1,30 @@ +/** Quote a UTF-8 path using Git's reversible double-quoted path syntax. */ +export function quoteGitPath(path: string): string { + let quoted = false; + let output = ""; + + for (const character of path) { + const codePoint = character.codePointAt(0); + if (character === '"' || character === "\\") { + quoted = true; + output += `\\${character}`; + continue; + } + if ( + codePoint !== undefined && + (codePoint <= 0x1f || + (codePoint >= 0x7f && codePoint <= 0x9f) || + codePoint === 0x2028 || + codePoint === 0x2029) + ) { + quoted = true; + for (const byte of new TextEncoder().encode(character)) { + output += `\\${byte.toString(8).padStart(3, "0")}`; + } + continue; + } + output += character; + } + + return quoted ? `"${output}"` : output; +} diff --git a/packages/mcp/src/shared/code-diff-request.test.ts b/packages/mcp/src/shared/code-diff-request.test.ts new file mode 100644 index 0000000..15d75b3 --- /dev/null +++ b/packages/mcp/src/shared/code-diff-request.test.ts @@ -0,0 +1,301 @@ +import { describe, expect, it } from "bun:test"; +import { + buildCodeDiffParams, + type CodeDiffRequestInput, + type CodeDiffView, +} from "./code-diff-request.js"; +import { InvalidPackageSpecError } from "./package-spec.js"; + +function invalid(input: CodeDiffRequestInput, message?: string): void { + expect(() => buildCodeDiffParams(input)).toThrow(InvalidPackageSpecError); + if (message !== undefined) { + expect(() => buildCodeDiffParams(input)).toThrow(message); + } +} + +describe("buildCodeDiffParams", () => { + it("builds an unversioned package target and preserves trimmed endpoints", () => { + const result = buildCodeDiffParams({ + target: " npm:express ", + range: " v4.18.1 .. v4.18.2 ", + }); + + expect(result).toEqual({ + params: { + target: { registry: "NPM", packageName: "express" }, + from: "v4.18.1", + to: "v4.18.2", + mode: "patches", + }, + view: "patch", + }); + expect(Object.hasOwn(result.params.target, "version")).toBe(false); + expect(Object.hasOwn(result.params.target, "repoUrl")).toBe(false); + expect(Object.hasOwn(result.params, "options")).toBe(false); + }); + + it("builds a compact repository target without the parser's ref key", () => { + const result = buildCodeDiffParams({ + target: "github:expressjs/express", + range: "main..release", + }); + + expect(result.params.target).toEqual({ + repoUrl: "https://github.com/expressjs/express", + }); + expect(Object.hasOwn(result.params.target, "registry")).toBe(false); + expect(Object.hasOwn(result.params.target, "packageName")).toBe(false); + expect(Object.hasOwn(result.params.target, "gitRef")).toBe(false); + }); + + it("builds an explicit repository URL", () => { + const { params } = buildCodeDiffParams({ + repoUrl: "https://github.com/expressjs/express", + range: "v4.18.1..v4.18.2", + }); + + expect(params.target).toEqual({ + repoUrl: "https://github.com/expressjs/express", + }); + }); + + it.each([ + ["patch", "patches"], + ["stat", "stats"], + ["name-only", "inventory"], + ["name-status", "inventory"], + ] as const)("maps %s to service mode %s", (view, mode) => { + const result = buildCodeDiffParams({ + target: "npm:express", + range: "1..2", + view, + }); + + expect(result.view).toBe(view); + expect(result.params.mode).toBe(mode); + }); + + it("forwards only explicitly supplied options", () => { + const result = buildCodeDiffParams({ + target: "npm:express", + range: "1..2", + pathGlob: "src/**/*.ts", + maxFiles: 300, + maxPatchBytes: 2_097_152, + }); + + expect(result.params.options).toEqual({ + pathGlob: "src/**/*.ts", + maxFiles: 300, + maxPatchBytes: 2_097_152, + }); + expect(Object.hasOwn(result.params.options ?? {}, "pathPrefix")).toBe( + false, + ); + }); + + it("omits options when every bound and glob is omitted", () => { + const { params } = buildCodeDiffParams({ + target: "npm:express", + range: "1..2", + maxFiles: undefined, + maxPatchBytes: undefined, + pathGlob: undefined, + }); + + expect(Object.hasOwn(params, "options")).toBe(false); + }); + + it("accepts representative bounded repository-relative globs", () => { + for (const pathGlob of [ + "src/**/*.ts", + "**/*.ts", + "src/?odule/file?.ts", + "src/\\[generated\\]/literal\\*", + "docs/guide\\!/*.md", + "dir with spaces/*.ts", + ]) { + expect( + buildCodeDiffParams({ + target: "npm:express", + range: "1..2", + pathGlob, + }).params.options?.pathGlob, + ).toBe(pathGlob); + } + }); + + it("rejects absent, mixed, and incomplete addressing", () => { + invalid({ range: "1..2" }, "exactly one"); + invalid( + { + target: "npm:express", + repoUrl: "https://github.com/a/b", + range: "1..2", + }, + "either", + ); + invalid({ target: " ", range: "1..2" }); + invalid({ repoUrl: " ", range: "1..2" }); + invalid({ repoUrl: "npm:express", range: "1..2" }, "repository target"); + }); + + it("gives supported registries without suggesting embedded versions", () => { + try { + buildCodeDiffParams({ target: "nppm:express", range: "1..2" }); + throw new Error("Expected target validation to fail."); + } catch (error) { + expect(error).toBeInstanceOf(InvalidPackageSpecError); + expect((error as Error).message).toContain("supported registries"); + expect((error as Error).message).toContain("npm"); + expect((error as Error).message).not.toContain("[@]"); + } + }); + + it("rejects versions and refs embedded in target addressing", () => { + invalid( + { target: "npm:express@4.18.1", range: "1..2" }, + "must not include a version", + ); + invalid( + { target: "github:expressjs/express#main", range: "1..2" }, + "must not include a ref", + ); + invalid( + { + repoUrl: "https://github.com/expressjs/express#main", + range: "1..2", + }, + "must not include a ref", + ); + invalid( + { + target: "https://github.com/expressjs/express@main", + range: "1..2", + }, + "must not include a ref", + ); + }); + + it("rejects invalid ranges and accepts identical endpoints", () => { + for (const range of [ + "1", + "..2", + "1..", + "..", + "1...2", + "1..2..3", + "1....2", + "1..2..", + "1.. ..2", + ]) { + invalid({ target: "npm:express", range }); + } + + expect( + buildCodeDiffParams({ target: "npm:express", range: " same .. same " }) + .params, + ).toMatchObject({ from: "same", to: "same" }); + }); + + it("rejects invalid path glob forms", () => { + for (const pathGlob of [ + "", + "/src/*.ts", + "src//*.ts", + "src/./*.ts", + "src/../*.ts", + "src/", + "src/*.ts/", + "src/**.ts", + "src/a***.ts", + "src/[test].ts", + "src/{test}.ts", + "src/!test.ts", + "src/foo\\", + "src/foo\\/*.ts", + ":", + ":(exclude)lib/**", + ":(glob)src/*", + ":/src/**", + ":!lib/**", + ":^lib/**", + ]) { + invalid({ target: "npm:express", range: "1..2", pathGlob }); + } + + invalid( + { + target: "npm:express", + range: "1..2", + pathGlob: "a".repeat(1025), + }, + "1024", + ); + invalid( + { + target: "npm:express", + range: "1..2", + pathGlob: "é".repeat(513), + }, + "1024", + ); + invalid( + { + target: "npm:express", + range: "1..2", + pathGlob: "bad\ud800", + }, + "valid UTF-8", + ); + }); + + it("validates numeric bounds and patch-byte view compatibility", () => { + for (const maxFiles of [ + 0, + 301, + 1.5, + Number.NaN, + Number.POSITIVE_INFINITY, + ]) { + invalid({ target: "npm:express", range: "1..2", maxFiles }, "maxFiles"); + } + for (const maxPatchBytes of [1023, 2_097_153, 1.5, Number.NaN]) { + invalid( + { target: "npm:express", range: "1..2", maxPatchBytes }, + "maxPatchBytes", + ); + } + invalid( + { + target: "npm:express", + range: "1..2", + view: "stat", + maxPatchBytes: 1024, + }, + "only", + ); + + expect( + buildCodeDiffParams({ + target: "npm:express", + range: "1..2", + maxFiles: 1, + maxPatchBytes: 1024, + }).params.options, + ).toEqual({ maxFiles: 1, maxPatchBytes: 1024 }); + }); + + it("rejects unknown runtime view values, including inherited keys", () => { + for (const view of ["summary", "toString"]) { + invalid( + { + target: "npm:express", + range: "1..2", + view: view as CodeDiffView, + }, + "view", + ); + } + }); +}); diff --git a/packages/mcp/src/shared/code-diff-request.ts b/packages/mcp/src/shared/code-diff-request.ts new file mode 100644 index 0000000..8ff6fec --- /dev/null +++ b/packages/mcp/src/shared/code-diff-request.ts @@ -0,0 +1,355 @@ +import type { + CodeDiffParams, + CodeNavigationTarget, +} from "@githits/core-internal"; +import { parseCodeNavigationTargetSpec } from "./code-navigation-target.js"; +import { InvalidPackageSpecError, KNOWN_REGISTRIES } from "./package-spec.js"; + +export type CodeDiffView = "patch" | "stat" | "name-only" | "name-status"; + +export interface CodeDiffRequestInput { + target?: string; + repoUrl?: string; + range: string; + view?: CodeDiffView; + pathGlob?: string; + maxFiles?: number; + maxPatchBytes?: number; +} + +export interface CodeDiffRequestBuildResult { + params: CodeDiffParams; + view: CodeDiffView; +} + +const VIEW_TO_MODE: Record = { + patch: "patches", + stat: "stats", + "name-only": "inventory", + "name-status": "inventory", +}; + +const MAX_PATH_GLOB_BYTES = 1024; +export const CODE_DIFF_MAX_FILES_MIN = 1; +export const CODE_DIFF_MAX_FILES_MAX = 300; +export const CODE_DIFF_MAX_PATCH_BYTES_MIN = 1024; +export const CODE_DIFF_MAX_PATCH_BYTES_MAX = 2_097_152; + +export function buildCodeDiffParams( + input: CodeDiffRequestInput, +): CodeDiffRequestBuildResult { + const target = buildTarget(input); + const { from, to } = parseRange(input.range); + const view = normaliseView(input.view); + const pathGlob = normalisePathGlob(input.pathGlob); + const maxFiles = normaliseIntegerOption( + input.maxFiles, + "maxFiles", + CODE_DIFF_MAX_FILES_MIN, + CODE_DIFF_MAX_FILES_MAX, + ); + const maxPatchBytes = normaliseIntegerOption( + input.maxPatchBytes, + "maxPatchBytes", + CODE_DIFF_MAX_PATCH_BYTES_MIN, + CODE_DIFF_MAX_PATCH_BYTES_MAX, + ); + + if (maxPatchBytes !== undefined && view !== "patch") { + throw invalid( + "`maxPatchBytes` is valid only when the CodeDiff view is `patch`.", + ); + } + + const options = buildOptions({ maxFiles, maxPatchBytes, pathGlob }); + const params: CodeDiffParams = { + target, + from, + to, + mode: VIEW_TO_MODE[view], + }; + if (options !== undefined) params.options = options; + + return { params, view }; +} + +function buildTarget(input: CodeDiffRequestInput): CodeDiffParams["target"] { + const hasTarget = input.target !== undefined; + const hasRepoUrl = input.repoUrl !== undefined; + + if (hasTarget && hasRepoUrl) { + throw invalid("Provide either `target` or `repoUrl`, not both."); + } + if (!hasTarget && !hasRepoUrl) { + throw invalid("Provide exactly one of `target` or `repoUrl`."); + } + + const raw = hasTarget ? input.target : input.repoUrl; + if (typeof raw !== "string") { + throw invalid("CodeDiff target must be a string."); + } + + const parsed = parseTarget(raw); + if (hasTarget && parsed.version !== undefined) { + throw invalid( + "Package targets must not include a version; put both versions in `range`.", + ); + } + if (parsed.gitRef !== undefined) { + throw invalid( + "Repository targets must not include a ref; put both refs in `range`.", + ); + } + + const hasPackageKeys = + Object.hasOwn(parsed, "registry") || Object.hasOwn(parsed, "packageName"); + const hasRepoKey = Object.hasOwn(parsed, "repoUrl"); + if (hasRepoUrl && hasPackageKeys) { + throw invalid("`repoUrl` must identify a repository target."); + } + if (hasPackageKeys && hasRepoKey) { + throw invalid( + "CodeDiff target cannot combine package and repository keys.", + ); + } + if (hasPackageKeys) { + if ( + !Object.hasOwn(parsed, "registry") || + !Object.hasOwn(parsed, "packageName") || + parsed.registry === undefined || + parsed.packageName === undefined + ) { + throw invalid( + "CodeDiff package target must include a registry and name.", + ); + } + return { + registry: parsed.registry, + packageName: parsed.packageName, + }; + } + if (hasRepoKey && parsed.repoUrl !== undefined) { + return { repoUrl: parsed.repoUrl }; + } + throw invalid("CodeDiff target must be a package or repository target."); +} + +function parseTarget(raw: string): CodeNavigationTarget { + try { + return parseCodeNavigationTargetSpec(raw); + } catch { + throw invalid( + `Invalid CodeDiff target. Expected an unversioned package target \`:\` (for example \`npm:express\`; supported registries: ${KNOWN_REGISTRIES.join(", ")}) or an unversioned repository target (for example \`github:expressjs/express\`).`, + ); + } +} + +function parseRange(raw: string): { from: string; to: string } { + if (typeof raw !== "string") { + throw invalid("CodeDiff range must be a string in the form `from..to`."); + } + const range = raw.trim(); + if (range.includes("...")) { + throw invalid( + "CodeDiff range must use one `from..to` separator, not `...`.", + ); + } + + const separator = range.indexOf(".."); + if (separator === -1 || range.indexOf("..", separator + 2) !== -1) { + throw invalid( + "CodeDiff range must contain exactly one `from..to` separator.", + ); + } + + const from = range.slice(0, separator).trim(); + const to = range.slice(separator + 2).trim(); + if (!from || !to) { + throw invalid("CodeDiff range endpoints must not be empty."); + } + return { from, to }; +} + +function normaliseView(raw: CodeDiffView | undefined): CodeDiffView { + if (raw === undefined) return "patch"; + if (typeof raw === "string" && Object.hasOwn(VIEW_TO_MODE, raw)) { + return raw; + } + throw invalid( + "CodeDiff view must be patch, stat, name-only, or name-status.", + ); +} + +function normalisePathGlob(raw: string | undefined): string | undefined { + if (raw === undefined) return undefined; + if (typeof raw !== "string") { + throw invalid("`pathGlob` must be a string when supplied."); + } + + const pathGlob = raw; + if (pathGlob.length === 0) { + throw invalid("`pathGlob` must not be empty when supplied."); + } + if (hasInvalidUtf16(pathGlob)) { + throw invalid("`pathGlob` must be valid UTF-8."); + } + if (new TextEncoder().encode(pathGlob).byteLength > MAX_PATH_GLOB_BYTES) { + throw invalid( + `\`pathGlob\` must be at most ${MAX_PATH_GLOB_BYTES} UTF-8 bytes.`, + ); + } + + validatePathGlobGrammar(pathGlob); + return pathGlob; +} + +interface GlobToken { + char: string; + escaped: boolean; +} + +function validatePathGlobGrammar(pathGlob: string): void { + if ( + pathGlob === ":" || + pathGlob.startsWith(":(") || + pathGlob.startsWith(":/") || + pathGlob.startsWith(":!") || + pathGlob.startsWith(":^") + ) { + throw invalid( + "`pathGlob` does not support Git pathspec magic; pass one bounded glob.", + ); + } + + const components: GlobToken[][] = []; + let component: GlobToken[] = []; + const characters = Array.from(pathGlob); + + for (let index = 0; index < characters.length; index += 1) { + const character = characters[index]; + if (character === undefined) break; + if (character === "/") { + addGlobComponent(components, component); + component = []; + continue; + } + if (character === "\\") { + const escaped = characters[index + 1]; + if (escaped === undefined || escaped === "/") { + throw invalid( + "`pathGlob` backslashes must escape one following non-slash character.", + ); + } + component.push({ char: escaped, escaped: true }); + index += 1; + continue; + } + if ( + character === "[" || + character === "]" || + character === "{" || + character === "}" || + character === "!" + ) { + throw invalid( + "`pathGlob` does not support unescaped brackets, braces, or `!`.", + ); + } + component.push({ char: character, escaped: false }); + } + addGlobComponent(components, component); + + for (const tokens of components) { + const isGlobstar = + tokens.length === 2 && + tokens.every(({ char, escaped }) => char === "*" && !escaped); + if (isGlobstar) continue; + + let previous: GlobToken | undefined; + for (const token of tokens) { + if ( + previous?.char === "*" && + !previous.escaped && + token.char === "*" && + !token.escaped + ) { + throw invalid( + "`pathGlob` allows adjacent stars only as an exact `**` component.", + ); + } + previous = token; + } + } +} + +function addGlobComponent( + components: GlobToken[][], + component: GlobToken[], +): void { + if (component.length === 0) { + throw invalid( + "`pathGlob` must use non-empty repository-relative components.", + ); + } + const literal = component.map(({ char }) => char).join(""); + if (literal === "." || literal === "..") { + throw invalid("`pathGlob` must not contain `.` or `..` components."); + } + components.push(component); +} + +function hasInvalidUtf16(value: string): boolean { + for (let index = 0; index < value.length; index += 1) { + const code = value.charCodeAt(index); + if (code >= 0xd800 && code <= 0xdbff) { + const next = value.charCodeAt(index + 1); + if (next >= 0xdc00 && next <= 0xdfff) { + index += 1; + continue; + } + return true; + } + if (code >= 0xdc00 && code <= 0xdfff) return true; + } + return false; +} + +function normaliseIntegerOption( + value: number | undefined, + name: string, + minimum: number, + maximum: number, +): number | undefined { + if (value === undefined) return undefined; + if (!Number.isInteger(value) || value < minimum || value > maximum) { + throw invalid( + `\`${name}\` must be an integer from ${minimum} through ${maximum}.`, + ); + } + return value; +} + +function buildOptions(input: { + maxFiles: number | undefined; + maxPatchBytes: number | undefined; + pathGlob: string | undefined; +}): CodeDiffParams["options"] { + if ( + input.maxFiles === undefined && + input.maxPatchBytes === undefined && + input.pathGlob === undefined + ) { + return undefined; + } + const options: NonNullable = {}; + if (input.maxFiles !== undefined) options.maxFiles = input.maxFiles; + if (input.maxPatchBytes !== undefined) { + options.maxPatchBytes = input.maxPatchBytes; + } + if (input.pathGlob !== undefined) options.pathGlob = input.pathGlob; + return options; +} + +function invalid(message: string): InvalidPackageSpecError { + return new InvalidPackageSpecError(message); +} diff --git a/packages/mcp/src/shared/code-diff-response.test.ts b/packages/mcp/src/shared/code-diff-response.test.ts new file mode 100644 index 0000000..2214af9 --- /dev/null +++ b/packages/mcp/src/shared/code-diff-response.test.ts @@ -0,0 +1,463 @@ +import { describe, expect, it } from "bun:test"; +import type { CodeDiffParams, CodeDiffResult } from "@githits/core-internal"; +import { + type BuildCodeDiffPayloadOptions, + buildCodeDiffSuccessPayload, +} from "./code-diff-response.js"; + +const packageTarget: CodeDiffParams["target"] = { + registry: "NPM", + packageName: "request-package", +}; + +const repositoryTarget: CodeDiffParams["target"] = { + repoUrl: "https://github.com/example/repository", +}; + +function makeResult(): CodeDiffResult { + return { + package: { + registry: "PYPI", + name: "canonical-package", + repoUrl: "https://github.com/example/canonical-package", + }, + fromResolution: { + requested: " from ", + resolvedVersion: "1.0.0", + ref: "refs/tags/v1.0.0", + commitSha: "0123456789abcdef0123456789abcdef01234567", + refKind: "TAG", + versionSource: "REGISTRY", + }, + toResolution: { + requested: "to-ref", + resolvedVersion: undefined, + ref: "to-ref", + commitSha: "fedcba9876543210fedcba9876543210fedcba98", + refKind: "HEAD", + versionSource: "GIT_HEAD", + }, + raw: { + summary: { + filesChanged: 8, + added: 3, + deleted: 2, + modified: 3, + modeChanged: 1, + typeChanged: 2, + inventoryComplete: false, + unprojectableFiles: 1, + }, + scope: { + status: "PACKAGE", + fromSubpath: "", + toSubpath: "", + pathPrefix: "src", + pathGlob: "src/**/*.ts", + }, + contentCoverage: "PARTIAL", + contentFailure: undefined, + files: [ + { + path: "first\\x80.ts", + pathEncoding: "BYTE_ESCAPED", + status: "ADDED", + modeChanged: true, + typeChanged: true, + additions: 0, + deletions: 2, + contentStatus: "OMITTED", + contentOmissionReason: "invalid_utf8", + contentSafety: { + filtered: true, + modifications: ["HTML_COMMENTS_STRIPPED", "IMAGES_REPLACED"], + }, + }, + { + path: "second.ts", + pathEncoding: "UTF8", + status: "MODIFIED", + modeChanged: false, + typeChanged: false, + additions: 4, + deletions: 1, + patch: "@@ -1 +1 @@\n-old\n+new", + contentStatus: "PATCH", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + hasMoreFiles: true, + }, + }; +} + +function options( + target: CodeDiffParams["target"] = packageTarget, + view: BuildCodeDiffPayloadOptions["view"] = "patch", +): BuildCodeDiffPayloadOptions { + return { target, view }; +} + +describe("buildCodeDiffSuccessPayload", () => { + it("uses canonical package facts and omits opposite target keys", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "name-only"), + ); + + expect(payload.target).toEqual({ + kind: "package", + registry: "pypi", + name: "canonical-package", + repoUrl: "https://github.com/example/canonical-package", + }); + expect(Object.keys(payload.target)).toEqual([ + "kind", + "registry", + "name", + "repoUrl", + ]); + expect(Object.hasOwn(payload.target, "repoUrl")).toBe(true); + expect(Object.hasOwn(payload.target, "gitRef")).toBe(false); + }); + + it("uses normalized package facts when the result has no package", () => { + const result = makeResult(); + result.package = undefined; + const payload = buildCodeDiffSuccessPayload(result, options()); + + expect(payload.target).toEqual({ + kind: "package", + registry: "npm", + name: "request-package", + }); + expect(Object.keys(payload.target)).toEqual(["kind", "registry", "name"]); + expect(Object.hasOwn(payload.target, "repoUrl")).toBe(false); + }); + + it("projects repository targets without package-shaped keys", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(repositoryTarget, "name-status"), + ); + + expect(payload.target).toEqual({ + kind: "repository", + repoUrl: "https://github.com/example/repository", + }); + expect(Object.keys(payload.target)).toEqual(["kind", "repoUrl"]); + expect(Object.hasOwn(payload.target, "registry")).toBe(false); + expect(Object.hasOwn(payload.target, "name")).toBe(false); + }); + + it("preserves full resolution identity and lowercases enum values", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "name-only"), + ); + + expect(payload.from).toEqual({ + requested: " from ", + resolvedVersion: "1.0.0", + ref: "refs/tags/v1.0.0", + commitSha: "0123456789abcdef0123456789abcdef01234567", + refKind: "tag", + versionSource: "registry", + }); + expect(payload.to).toEqual({ + requested: "to-ref", + ref: "to-ref", + commitSha: "fedcba9876543210fedcba9876543210fedcba98", + refKind: "head", + versionSource: "git_head", + }); + expect(Object.hasOwn(payload.to, "resolvedVersion")).toBe(false); + }); + + it("preserves summary, root package scope, truncation, and coverage facts", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "name-only"), + ); + + expect(payload.summary).toEqual({ + filesChanged: 8, + added: 3, + deleted: 2, + modified: 3, + modeChanged: 1, + typeChanged: 2, + inventoryComplete: false, + unprojectableFiles: 1, + }); + expect(payload.scope).toEqual({ + status: "package", + fromSubpath: "", + toSubpath: "", + pathPrefix: "src", + pathGlob: "src/**/*.ts", + }); + expect(payload.contentCoverage).toBe("partial"); + expect(payload.hasMoreFiles).toBe(true); + expect(payload.files.map((file) => file.path)).toEqual([ + "first\\x80.ts", + "second.ts", + ]); + }); + + it("preserves different package roots on each comparison side", () => { + const result = makeResult(); + result.raw.scope = { + status: "PACKAGE", + fromSubpath: "packages/old-name", + toSubpath: "packages/new-name", + }; + + const payload = buildCodeDiffSuccessPayload( + result, + options(packageTarget, "name-only"), + ); + + expect(payload.scope).toEqual({ + status: "package", + fromSubpath: "packages/old-name", + toSubpath: "packages/new-name", + }); + }); + + it("projects name-only with mandatory path encoding and no other fields", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "name-only"), + ); + + expect(payload.files).toEqual([ + { path: "first\\x80.ts", pathEncoding: "byte_escaped" }, + { path: "second.ts", pathEncoding: "utf8" }, + ]); + }); + + it("projects name-status with lowercase status only", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "name-status"), + ); + + expect(payload.files).toEqual([ + { + path: "first\\x80.ts", + pathEncoding: "byte_escaped", + status: "added", + }, + { path: "second.ts", pathEncoding: "utf8", status: "modified" }, + ]); + }); + + it("projects stat facts and omits patch-only fields", () => { + const result = makeResult(); + for (const file of result.raw.files) { + file.contentStatus = "STATS"; + file.patch = undefined; + file.contentOmissionReason = undefined; + } + const payload = buildCodeDiffSuccessPayload( + result, + options(packageTarget, "stat"), + ); + + expect(payload.files).toEqual([ + { + path: "first\\x80.ts", + pathEncoding: "byte_escaped", + status: "added", + modeChanged: true, + typeChanged: true, + additions: 0, + deletions: 2, + contentStatus: "stats", + }, + { + path: "second.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 4, + deletions: 1, + contentStatus: "stats", + }, + ]); + expect(Object.hasOwn(payload.files[0]!, "patch")).toBe(false); + expect(Object.hasOwn(payload.files[0]!, "contentSafety")).toBe(false); + }); + + it("projects realistic patch and omission facts with safety", () => { + const payload = buildCodeDiffSuccessPayload( + makeResult(), + options(packageTarget, "patch"), + ); + + expect(payload.files).toEqual([ + { + path: "first\\x80.ts", + pathEncoding: "byte_escaped", + status: "added", + modeChanged: true, + typeChanged: true, + additions: 0, + deletions: 2, + contentStatus: "omitted", + contentOmissionReason: "invalid_utf8", + contentSafety: { + filtered: true, + modifications: ["html_comments_stripped", "images_replaced"], + }, + }, + { + path: "second.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 4, + deletions: 1, + contentStatus: "patch", + patch: "@@ -1 +1 @@\n-old\n+new", + contentSafety: { filtered: false, modifications: [] }, + }, + ]); + }); + + it("binds placeholder patch headers to Git-quoted authoritative paths", () => { + const result = makeResult(); + result.raw.files = [ + { + path: "src/line\nname.ts", + pathEncoding: "UTF8", + status: "MODIFIED", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 1, + patch: "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n+new\n", + contentStatus: "PATCH", + contentSafety: { filtered: false, modifications: [] }, + }, + { + path: "added file.ts", + pathEncoding: "UTF8", + status: "ADDED", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 0, + patch: "--- /dev/null\n+++ b/file\n@@ -0,0 +1 @@\n+new\n", + contentStatus: "PATCH", + contentSafety: { filtered: false, modifications: [] }, + }, + { + path: "deleted.ts", + pathEncoding: "UTF8", + status: "DELETED", + modeChanged: false, + typeChanged: false, + additions: 0, + deletions: 1, + patch: "--- a/file\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n", + contentStatus: "PATCH", + contentSafety: { filtered: false, modifications: [] }, + }, + ]; + + const payload = buildCodeDiffSuccessPayload(result, options()); + + expect( + payload.files.map((file) => ("patch" in file ? file.patch : null)), + ).toEqual([ + '--- "a/src/line\\012name.ts"\n+++ "b/src/line\\012name.ts"\n@@ -1 +1 @@\n-old\n+new\n', + "--- /dev/null\n+++ b/added file.ts\n@@ -0,0 +1 @@\n+new\n", + "--- a/deleted.ts\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n", + ]); + }); + + it("preserves upstream patches whose headers are already authoritative", () => { + const result = makeResult(); + const file = result.raw.files[1]; + if (!file) throw new Error("Expected second fixture file."); + file.patch = "--- a/second.ts\n+++ b/second.ts\n@@ -1 +1 @@\n-old\n+new\n"; + + const payload = buildCodeDiffSuccessPayload(result, options()); + + expect(payload.files[1]).toMatchObject({ patch: file.patch }); + }); + + it("preserves content failure fields for failed content coverage", () => { + const result = makeResult(); + result.raw.contentCoverage = "FAILED"; + result.raw.contentFailure = { + code: "RAW_DIFF_LIMIT_EXCEEDED", + retryable: false, + retryAfterMs: 0, + stage: "content", + limitKind: "max_content_entries", + }; + const unavailable = result.raw.files[1]; + if (!unavailable) throw new Error("Expected second fixture file."); + unavailable.contentStatus = "UNAVAILABLE"; + unavailable.additions = undefined; + unavailable.deletions = undefined; + unavailable.patch = undefined; + const payload = buildCodeDiffSuccessPayload(result, options()); + + expect(payload.contentFailure).toEqual({ + code: "RAW_DIFF_LIMIT_EXCEEDED", + retryable: false, + retryAfterMs: 0, + stage: "content", + limitKind: "max_content_entries", + }); + expect(payload.contentCoverage).toBe("failed"); + expect(payload.files[1]).toMatchObject({ + path: "second.ts", + contentStatus: "unavailable", + }); + }); + + it("supports empty identical inventory results and unknown scope", () => { + const result = makeResult(); + result.raw = { + ...result.raw, + summary: { + filesChanged: 0, + added: 0, + deleted: 0, + modified: 0, + modeChanged: 0, + typeChanged: 0, + inventoryComplete: true, + unprojectableFiles: 0, + }, + scope: { status: "UNKNOWN" }, + contentCoverage: "NOT_REQUESTED", + files: [], + hasMoreFiles: false, + }; + result.toResolution = { + ...result.fromResolution, + requested: "same", + }; + result.fromResolution = { ...result.toResolution }; + + const payload = buildCodeDiffSuccessPayload( + result, + options(repositoryTarget, "name-only"), + ); + + expect(payload.from.commitSha).toBe(payload.to.commitSha); + expect(payload.from.requested).toBe("same"); + expect(payload.to.requested).toBe("same"); + expect(payload.scope).toEqual({ status: "unknown" }); + expect(payload.contentCoverage).toBe("not_requested"); + expect(payload.files).toEqual([]); + }); +}); diff --git a/packages/mcp/src/shared/code-diff-response.ts b/packages/mcp/src/shared/code-diff-response.ts new file mode 100644 index 0000000..165bacc --- /dev/null +++ b/packages/mcp/src/shared/code-diff-response.ts @@ -0,0 +1,309 @@ +import type { + CodeDiffParams, + CodeDiffResult, + RawCodeDiffFile, +} from "@githits/core-internal"; +import { quoteGitPath } from "./code-diff-path.js"; +import type { CodeDiffView } from "./code-diff-request.js"; + +export type CodeDiffEnvelopeRefKind = + | "sha" + | "tag" + | "branch" + | "head" + | "unknown"; +export type CodeDiffEnvelopeVersionSource = + | "registry" + | "git_head" + | "tag" + | "release"; +export type CodeDiffEnvelopeScopeStatus = "package" | "repository" | "unknown"; +export type CodeDiffEnvelopeFileStatus = "added" | "deleted" | "modified"; +export type CodeDiffEnvelopePathEncoding = "utf8" | "byte_escaped"; +export type CodeDiffEnvelopeContentStatus = + | "not_requested" + | "stats" + | "patch" + | "binary" + | "metadata_only" + | "omitted" + | "unavailable"; +export type CodeDiffEnvelopeContentCoverage = + | "not_requested" + | "complete" + | "partial" + | "failed"; + +export interface LeanCodeDiffPackageTarget { + kind: "package"; + registry: string; + name: string; + repoUrl?: string; +} + +export interface LeanCodeDiffRepositoryTarget { + kind: "repository"; + repoUrl: string; +} + +export type LeanCodeDiffTarget = + | LeanCodeDiffPackageTarget + | LeanCodeDiffRepositoryTarget; + +export interface LeanCodeDiffResolution { + requested: string; + resolvedVersion?: string; + ref: string; + commitSha: string; + refKind: CodeDiffEnvelopeRefKind; + versionSource?: CodeDiffEnvelopeVersionSource; +} + +export interface LeanCodeDiffSummary { + filesChanged: number; + added: number; + deleted: number; + modified: number; + modeChanged: number; + typeChanged: number; + inventoryComplete: boolean; + unprojectableFiles: number; +} + +export interface LeanCodeDiffScope { + status: CodeDiffEnvelopeScopeStatus; + fromSubpath?: string; + toSubpath?: string; + pathPrefix?: string; + pathGlob?: string; +} + +export interface LeanCodeDiffContentFailure { + code: string; + retryable: boolean; + retryAfterMs?: number; + stage?: string; + limitKind?: string; +} + +export interface LeanCodeDiffContentSafety { + filtered: boolean; + modifications: string[]; +} + +export interface LeanCodeDiffFileBase { + path: string; + pathEncoding: CodeDiffEnvelopePathEncoding; +} + +export interface LeanCodeDiffNameOnlyFile extends LeanCodeDiffFileBase {} + +export interface LeanCodeDiffNameStatusFile extends LeanCodeDiffFileBase { + status: CodeDiffEnvelopeFileStatus; +} + +export interface LeanCodeDiffStatFile extends LeanCodeDiffNameStatusFile { + modeChanged: boolean; + typeChanged: boolean; + additions?: number; + deletions?: number; + contentStatus: CodeDiffEnvelopeContentStatus; +} + +export interface LeanCodeDiffPatchFile extends LeanCodeDiffStatFile { + patch?: string; + contentOmissionReason?: string; + contentSafety: LeanCodeDiffContentSafety; +} + +export type LeanCodeDiffFile = + | LeanCodeDiffNameOnlyFile + | LeanCodeDiffNameStatusFile + | LeanCodeDiffStatFile + | LeanCodeDiffPatchFile; + +export interface LeanCodeDiffEnvelope { + target: LeanCodeDiffTarget; + view: CodeDiffView; + from: LeanCodeDiffResolution; + to: LeanCodeDiffResolution; + summary: LeanCodeDiffSummary; + scope: LeanCodeDiffScope; + contentCoverage: CodeDiffEnvelopeContentCoverage; + contentFailure?: LeanCodeDiffContentFailure; + files: LeanCodeDiffFile[]; + hasMoreFiles: boolean; +} + +export interface BuildCodeDiffPayloadOptions { + target: CodeDiffParams["target"]; + view: CodeDiffView; +} + +export function buildCodeDiffSuccessPayload( + result: CodeDiffResult, + options: BuildCodeDiffPayloadOptions, +): LeanCodeDiffEnvelope { + const envelope: LeanCodeDiffEnvelope = { + target: buildTarget(result, options.target), + view: options.view, + from: projectResolution(result.fromResolution), + to: projectResolution(result.toResolution), + summary: projectSummary(result.raw.summary), + scope: projectScope(result.raw.scope), + contentCoverage: lower(result.raw.contentCoverage), + files: result.raw.files.map((file) => projectFile(file, options.view)), + hasMoreFiles: result.raw.hasMoreFiles, + }; + + if (result.raw.contentFailure != null) { + envelope.contentFailure = projectContentFailure(result.raw.contentFailure); + } + return envelope; +} + +function buildTarget( + result: CodeDiffResult, + target: CodeDiffParams["target"], +): LeanCodeDiffTarget { + if ("registry" in target) { + const packageInfo = result.package; + return { + kind: "package", + registry: lower(packageInfo?.registry ?? target.registry), + name: packageInfo?.name ?? target.packageName, + ...(packageInfo?.repoUrl != null ? { repoUrl: packageInfo.repoUrl } : {}), + }; + } + return { kind: "repository", repoUrl: target.repoUrl }; +} + +function projectResolution( + resolution: CodeDiffResult["fromResolution"], +): LeanCodeDiffResolution { + const projected: LeanCodeDiffResolution = { + requested: resolution.requested, + ref: resolution.ref, + commitSha: resolution.commitSha, + refKind: lower(resolution.refKind), + }; + if (resolution.resolvedVersion != null) { + projected.resolvedVersion = resolution.resolvedVersion; + } + if (resolution.versionSource != null) { + projected.versionSource = lower(resolution.versionSource); + } + return projected; +} + +function projectSummary( + summary: CodeDiffResult["raw"]["summary"], +): LeanCodeDiffSummary { + return { + filesChanged: summary.filesChanged, + added: summary.added, + deleted: summary.deleted, + modified: summary.modified, + modeChanged: summary.modeChanged, + typeChanged: summary.typeChanged, + inventoryComplete: summary.inventoryComplete, + unprojectableFiles: summary.unprojectableFiles, + }; +} + +function projectScope( + scope: CodeDiffResult["raw"]["scope"], +): LeanCodeDiffScope { + const projected: LeanCodeDiffScope = { status: lower(scope.status) }; + if (scope.fromSubpath != null) projected.fromSubpath = scope.fromSubpath; + if (scope.toSubpath != null) projected.toSubpath = scope.toSubpath; + if (scope.pathPrefix != null) projected.pathPrefix = scope.pathPrefix; + if (scope.pathGlob != null) projected.pathGlob = scope.pathGlob; + return projected; +} + +function projectContentFailure( + failure: NonNullable, +): LeanCodeDiffContentFailure { + const projected: LeanCodeDiffContentFailure = { + code: failure.code, + retryable: failure.retryable, + }; + if (failure.retryAfterMs != null) { + projected.retryAfterMs = failure.retryAfterMs; + } + if (failure.stage != null) projected.stage = failure.stage; + if (failure.limitKind != null) projected.limitKind = failure.limitKind; + return projected; +} + +function projectFile( + file: RawCodeDiffFile, + view: CodeDiffView, +): LeanCodeDiffFile { + const base: LeanCodeDiffFileBase = { + path: file.path, + pathEncoding: lower(file.pathEncoding), + }; + if (view === "name-only") return base; + + const nameStatus: LeanCodeDiffNameStatusFile = { + ...base, + status: lower(file.status), + }; + if (view === "name-status") return nameStatus; + + const stat: LeanCodeDiffStatFile = { + ...nameStatus, + modeChanged: file.modeChanged, + typeChanged: file.typeChanged, + contentStatus: lower(file.contentStatus), + }; + if (file.additions != null) stat.additions = file.additions; + if (file.deletions != null) stat.deletions = file.deletions; + if (view === "stat") return stat; + + const patch: LeanCodeDiffPatchFile = { + ...stat, + contentSafety: { + filtered: file.contentSafety.filtered, + modifications: file.contentSafety.modifications.map(lower), + }, + }; + if (file.patch != null) { + patch.patch = bindPatchHeaders(file.patch, file.path, patch.status); + } + if (file.contentOmissionReason != null) { + patch.contentOmissionReason = file.contentOmissionReason; + } + return patch; +} + +/** Bind the raw diff service's content-only placeholders to its owning file. */ +function bindPatchHeaders( + patch: string, + path: string, + status: CodeDiffEnvelopeFileStatus, +): string { + const firstEnd = patch.indexOf("\n"); + if (firstEnd < 0) return patch; + const secondStart = firstEnd + 1; + const secondEnd = patch.indexOf("\n", secondStart); + if (secondEnd < 0) return patch; + + const originalFrom = patch.slice(0, firstEnd); + const originalTo = patch.slice(secondStart, secondEnd); + if (originalFrom !== "--- a/file" && originalTo !== "+++ b/file") { + return patch; + } + + const fromPath = status === "added" ? "/dev/null" : quoteGitPath(`a/${path}`); + const toPath = status === "deleted" ? "/dev/null" : quoteGitPath(`b/${path}`); + const from = originalFrom === "--- a/file" ? `--- ${fromPath}` : originalFrom; + const to = originalTo === "+++ b/file" ? `+++ ${toPath}` : originalTo; + return `${from}\n${to}\n${patch.slice(secondEnd + 1)}`; +} + +function lower(value: T): Lowercase { + return value.toLowerCase() as Lowercase; +} diff --git a/packages/mcp/src/shared/code-diff-text.test.ts b/packages/mcp/src/shared/code-diff-text.test.ts new file mode 100644 index 0000000..31d8def --- /dev/null +++ b/packages/mcp/src/shared/code-diff-text.test.ts @@ -0,0 +1,531 @@ +import { describe, expect, it } from "bun:test"; +import type { LeanCodeDiffEnvelope } from "./code-diff-response.js"; +import { formatCodeDiffTerminal } from "./code-diff-text.js"; + +function envelope( + overrides: Partial = {}, +): LeanCodeDiffEnvelope { + return { + target: { kind: "package", registry: "npm", name: "example" }, + view: "patch", + from: { + requested: "1.0.0", + resolvedVersion: "1.0.0", + ref: "v1.0.0", + commitSha: "0123456789abcdef0123456789abcdef01234567", + refKind: "tag", + versionSource: "registry", + }, + to: { + requested: "2.0.0", + resolvedVersion: "2.0.0", + ref: "v2.0.0", + commitSha: "fedcba9876543210fedcba9876543210fedcba98", + refKind: "tag", + versionSource: "registry", + }, + summary: { + filesChanged: 2, + added: 1, + deleted: 0, + modified: 1, + modeChanged: 0, + typeChanged: 0, + inventoryComplete: true, + unprojectableFiles: 0, + }, + scope: { status: "package", fromSubpath: "", toSubpath: "" }, + contentCoverage: "complete", + files: [], + hasMoreFiles: false, + ...overrides, + }; +} + +const options = { useColors: false } as const; + +describe("formatCodeDiffTerminal", () => { + it("renders name-only with reversible Git quoting", () => { + const result = formatCodeDiffTerminal( + envelope({ + view: "name-only", + contentCoverage: "not_requested", + files: [ + { path: "src/a.ts", pathEncoding: "utf8" }, + { path: "bad\u001b[31m.ts", pathEncoding: "utf8" }, + { path: "line\u2028separator.ts", pathEncoding: "utf8" }, + ], + }), + options, + ); + + expect(result).toEqual({ + stdout: + 'src/a.ts\n"bad\\033[31m.ts"\n"line\\342\\200\\250separator.ts"\n', + stderr: undefined, + }); + }); + + it("renders name-status with Git status letters", () => { + const result = formatCodeDiffTerminal( + envelope({ + view: "name-status", + contentCoverage: "not_requested", + files: [ + { path: "a.ts", pathEncoding: "utf8", status: "added" }, + { path: "d.ts", pathEncoding: "utf8", status: "deleted" }, + { path: "m.ts", pathEncoding: "utf8", status: "modified" }, + ], + }), + options, + ); + + expect(result.stdout).toBe("A\ta.ts\nD\td.ts\nM\tm.ts\n"); + }); + + it("renders returned stat rows without claiming full-inventory line totals", () => { + const result = formatCodeDiffTerminal( + envelope({ + view: "stat", + files: [ + { + path: "text.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 4, + deletions: 2, + contentStatus: "stats", + }, + { + path: "image.png", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "binary", + }, + ], + }), + options, + ); + + expect(result.stdout).toBe( + " text.ts | 6 ++++--\n image.png | binary content differs\n 2 files changed, 4 insertions(+), 2 deletions(-)\n", + ); + }); + + it("marks stat totals as returned-only when file projection is truncated", () => { + const result = formatCodeDiffTerminal( + envelope({ + view: "stat", + hasMoreFiles: true, + files: [ + { + path: "one.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 0, + contentStatus: "stats", + }, + ], + }), + options, + ); + + expect(result.stdout).toContain("1 returned file changed, 1 insertion(+)"); + }); + + it("keeps a visible sign for every non-zero stat direction", () => { + const result = formatCodeDiffTerminal( + envelope({ + view: "stat", + summary: { + filesChanged: 1, + added: 0, + deleted: 0, + modified: 1, + modeChanged: 0, + typeChanged: 0, + inventoryComplete: true, + unprojectableFiles: 0, + }, + files: [ + { + path: "skewed.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 100, + contentStatus: "stats", + }, + ], + }), + options, + ); + + expect(result.stdout.split("\n")[0]).toContain("+"); + expect(result.stdout.split("\n")[0]).toContain("-"); + }); + + it("renders normalized patches and an explicitly budgeted omission", () => { + const result = formatCodeDiffTerminal( + envelope({ + files: [ + { + path: "a.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 1, + patch: "--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-old\n+new\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + { + path: "added file.ts", + pathEncoding: "utf8", + status: "added", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 0, + patch: "--- /dev/null\n+++ b/added file.ts\n@@ -0,0 +1 @@\n+new\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + { + path: "deleted.ts", + pathEncoding: "utf8", + status: "deleted", + modeChanged: false, + typeChanged: false, + additions: 0, + deletions: 1, + patch: "--- a/deleted.ts\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + { + path: "large.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "omitted", + contentOmissionReason: "total_patch_bytes", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + { ...options, explicitMaxPatchBytes: true }, + ); + + expect(result.stdout).toBe( + "--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-old\n+new\n--- /dev/null\n+++ b/added file.ts\n@@ -0,0 +1 @@\n+new\n--- a/deleted.ts\n+++ /dev/null\n@@ -1 +0,0 @@\n-old\nPatch omitted: large.ts (total_patch_bytes)\n", + ); + }); + + it("accepts both backend names for an explicit patch-budget omission", () => { + for (const reason of ["content_budget", "total_patch_bytes"]) { + const result = formatCodeDiffTerminal( + envelope({ + contentCoverage: "partial", + files: [ + { + path: "large.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "omitted", + contentOmissionReason: reason, + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + { ...options, explicitMaxPatchBytes: true }, + ); + + expect(result.stdout).toContain(`Patch omitted: large.ts (${reason})`); + expect(result.exitCode).toBeUndefined(); + } + }); + + it("suppresses patch streams that cannot represent binary changes", () => { + const result = formatCodeDiffTerminal( + envelope({ + files: [ + { + path: "image.png", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "binary", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + options, + ); + + expect(result.stdout).toBe(""); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + "1 binary change cannot be represented as an applicable text patch", + ); + expect(result.stderr).toContain("Use --stat or --name-status"); + }); + + it("preserves patches without backend placeholder headers", () => { + const result = formatCodeDiffTerminal( + envelope({ + files: [ + { + path: "a.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 1, + patch: "@@ -1 +1 @@\n-old\n+new\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + options, + ); + + expect(result.stdout).toBe("@@ -1 +1 @@\n-old\n+new\n"); + }); + + it("suppresses unexpectedly incomplete patch streams", () => { + const result = formatCodeDiffTerminal( + envelope({ + hasMoreFiles: true, + files: [ + { + path: "one.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 1, + patch: "--- a/one.ts\n+++ b/one.ts\n@@ -1 +1 @@\n-old\n+new\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + options, + ); + + expect(result.stdout).toBe(""); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("Patch output was suppressed"); + }); + + it("keeps explicitly file-limited patch streams successful", () => { + const result = formatCodeDiffTerminal( + envelope({ + hasMoreFiles: true, + files: [ + { + path: "one.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + additions: 1, + deletions: 1, + patch: "--- a/one.ts\n+++ b/one.ts\n@@ -1 +1 @@\n-old\n+new\n", + contentStatus: "patch", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + { ...options, explicitMaxFiles: true }, + ); + + expect(result.stdout).toContain("--- a/one.ts"); + expect(result.exitCode).toBeUndefined(); + }); + + it("does not let explicit limits authorize unrelated patch failures", () => { + const result = formatCodeDiffTerminal( + envelope({ + contentCoverage: "failed", + files: [ + { + path: "one.ts", + pathEncoding: "utf8", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "unavailable", + contentSafety: { filtered: false, modifications: [] }, + }, + ], + }), + { + ...options, + explicitMaxFiles: true, + explicitMaxPatchBytes: true, + }, + ); + + expect(result.stdout).toBe(""); + expect(result.exitCode).toBe(1); + }); + + it("keeps an empty authoritative diff silent in plain mode", () => { + expect(formatCodeDiffTerminal(envelope({ files: [] }), options)).toEqual({ + stdout: "", + stderr: undefined, + }); + }); + + it("does not treat an all-unprojectable result as an empty diff", () => { + const result = formatCodeDiffTerminal( + envelope({ + summary: { + filesChanged: 1, + added: 0, + deleted: 0, + modified: 1, + modeChanged: 0, + typeChanged: 0, + inventoryComplete: true, + unprojectableFiles: 1, + }, + files: [], + }), + options, + ); + + expect(result.stdout).toBe(""); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("1 matching path(s)"); + expect(result.stderr).toContain("Patch output was suppressed"); + }); + + it("renders completeness, scope, encoding, safety, and content warnings", () => { + const result = formatCodeDiffTerminal( + envelope({ + summary: { + filesChanged: 8, + added: 2, + deleted: 2, + modified: 4, + modeChanged: 1, + typeChanged: 1, + inventoryComplete: false, + unprojectableFiles: 2, + }, + scope: { status: "unknown" }, + contentCoverage: "failed", + contentFailure: { + code: "RAW_DIFF_LIMIT_EXCEEDED", + retryable: false, + stage: "content", + limitKind: "max_content_entries", + }, + files: [ + { + path: "bad\\x80.ts", + pathEncoding: "byte_escaped", + status: "modified", + modeChanged: false, + typeChanged: false, + contentStatus: "unavailable", + contentSafety: { + filtered: true, + modifications: ["invisible_controls_stripped"], + }, + }, + ], + hasMoreFiles: true, + }), + options, + ); + + expect(result.stderr).toContain( + "Showing changes for the entire repository", + ); + expect(result.stderr).toContain("Unrelated files may be included"); + expect(result.stderr).toContain("inventory is incomplete"); + expect(result.stderr).toContain("More matching files"); + expect(result.stderr).toContain("Add a path glob after `--`"); + expect(result.stderr).toContain("2 matching path(s)"); + expect(result.stderr).toContain("Requested content failed"); + expect(result.stderr).toContain("Use --stat or --name-status"); + expect(result.stderr).toContain("--json"); + expect(result.stderr).toContain("display-only byte escapes"); + expect(result.stderr).toContain("modified for content safety"); + }); + + it("explains that an unknown package scope applies the glob repository-wide", () => { + const result = formatCodeDiffTerminal( + envelope({ + scope: { status: "unknown", pathGlob: "packages/**/*.ts" }, + }), + options, + ); + + expect(result.stderr).toContain( + "the path glob was applied across the entire repository", + ); + expect(result.stderr).toContain( + "Matching files from other packages may be included", + ); + expect(result.stderr).toContain("narrow the path glob if needed"); + expect(result.stderr).not.toContain("add a path glob"); + }); + + it("offers a path glob when unknown package scope is otherwise complete", () => { + const result = formatCodeDiffTerminal( + envelope({ scope: { status: "unknown" } }), + options, + ); + + expect(result.stderr).toContain( + "Showing changes for the entire repository", + ); + expect(result.stderr).toContain( + "Add a path glob after `--` to narrow the diff", + ); + }); + + it("adds full exact identity and scope facts only in verbose diagnostics", () => { + const result = formatCodeDiffTerminal( + envelope({ + scope: { + status: "package", + fromSubpath: "packages/old", + toSubpath: "packages/new", + pathGlob: "packages/**/*.ts", + }, + }), + { useColors: false, verbose: true }, + ); + + expect(result.stdout).toBe(""); + expect(result.stderr).toContain("target: npm:example"); + expect(result.stderr).toContain("0123456789abcdef0123456789abcdef01234567"); + expect(result.stderr).toContain("fedcba9876543210fedcba9876543210fedcba98"); + expect(result.stderr).toContain('roots "packages/old" -> "packages/new"'); + expect(result.stderr).toContain('glob "packages/**/*.ts"'); + }); +}); diff --git a/packages/mcp/src/shared/code-diff-text.ts b/packages/mcp/src/shared/code-diff-text.ts new file mode 100644 index 0000000..2b6135a --- /dev/null +++ b/packages/mcp/src/shared/code-diff-text.ts @@ -0,0 +1,414 @@ +import { quoteGitPath } from "./code-diff-path.js"; +import type { + LeanCodeDiffEnvelope, + LeanCodeDiffFile, + LeanCodeDiffPatchFile, + LeanCodeDiffStatFile, +} from "./code-diff-response.js"; +import { warning } from "./colors.js"; +import { sanitizeTerminalText } from "./resolve-target-response.js"; + +export interface FormatCodeDiffTerminalOptions { + useColors: boolean; + verbose?: boolean; + explicitMaxFiles?: boolean; + explicitMaxPatchBytes?: boolean; +} + +export interface FormattedCodeDiffTerminal { + stdout: string; + stderr?: string; + exitCode?: 1; +} + +const EXPLICIT_PATCH_BUDGET_OMISSION_REASONS = new Set([ + "content_budget", + "total_patch_bytes", +]); + +/** Render a Git-like primary stream plus truthful bounded-evidence diagnostics. */ +export function formatCodeDiffTerminal( + envelope: LeanCodeDiffEnvelope, + options: FormatCodeDiffTerminalOptions, +): FormattedCodeDiffTerminal { + const diagnostics = buildDiagnostics(envelope, options); + const suppressPatch = shouldSuppressPatch(envelope, options); + if (suppressPatch) { + diagnostics.push( + warn( + "Patch output was suppressed because the result is not safely applicable. Use --stat or --name-status to inspect changes, or --json for structured partial evidence.", + options, + ), + ); + } + return { + stdout: suppressPatch ? "" : formatPrimaryOutput(envelope), + stderr: diagnostics.length > 0 ? `${diagnostics.join("\n")}\n` : undefined, + ...(suppressPatch ? { exitCode: 1 as const } : {}), + }; +} + +function formatPrimaryOutput(envelope: LeanCodeDiffEnvelope): string { + switch (envelope.view) { + case "name-only": + return formatLines(envelope.files.map((file) => quoteGitPath(file.path))); + case "name-status": + return formatLines( + envelope.files.map( + (file) => + `${statusLetter(requireStatus(file))}\t${quoteGitPath(file.path)}`, + ), + ); + case "stat": + return formatStat(envelope); + case "patch": + return formatPatches(envelope.files); + } +} + +function formatStat(envelope: LeanCodeDiffEnvelope): string { + const rows: Array<{ + path: string; + additions?: number; + deletions?: number; + contentStatus: LeanCodeDiffStatFile["contentStatus"]; + }> = []; + let additions = 0; + let deletions = 0; + + for (const file of envelope.files) { + const stat = requireStat(file); + const path = quoteGitPath(stat.path); + if (stat.additions !== undefined && stat.deletions !== undefined) { + rows.push({ + path, + additions: stat.additions, + deletions: stat.deletions, + contentStatus: stat.contentStatus, + }); + additions += stat.additions; + deletions += stat.deletions; + continue; + } + rows.push({ path, contentStatus: stat.contentStatus }); + } + + if (rows.length === 0) return ""; + const pathWidth = Math.max(...rows.map(({ path }) => path.length)); + const countWidth = Math.max( + 1, + ...rows.map(({ additions, deletions }) => + additions !== undefined && deletions !== undefined + ? String(additions + deletions).length + : 0, + ), + ); + const lines = rows.map(({ path, additions, deletions, contentStatus }) => { + const detail = + additions !== undefined && deletions !== undefined + ? formatStatCounts(additions, deletions, countWidth) + : contentLabel(contentStatus); + return ` ${path.padEnd(pathWidth)} | ${detail}`; + }); + const noun = rows.length === 1 ? "file" : "files"; + const inventoryFullyRepresented = + envelope.summary.inventoryComplete && + envelope.summary.unprojectableFiles === 0 && + !envelope.hasMoreFiles && + rows.length === envelope.summary.filesChanged; + const qualifier = inventoryFullyRepresented ? "" : "returned "; + const totals = [`${rows.length} ${qualifier}${noun} changed`]; + if (additions > 0) { + totals.push( + `${additions} ${additions === 1 ? "insertion" : "insertions"}(+)`, + ); + } + if (deletions > 0) { + totals.push( + `${deletions} ${deletions === 1 ? "deletion" : "deletions"}(-)`, + ); + } + lines.push(` ${totals.join(", ")}`); + return formatLines(lines); +} + +function formatStatCounts( + additions: number, + deletions: number, + countWidth: number, +): string { + const total = additions + deletions; + if (total === 0) return "0".padStart(countWidth); + const barWidth = Math.min(total, 40); + let pluses = Math.round((additions / total) * barWidth); + if (additions > 0) pluses = Math.max(1, pluses); + if (deletions > 0) pluses = Math.min(barWidth - 1, pluses); + return `${String(total).padStart(countWidth)} ${"+".repeat(pluses)}${"-".repeat(barWidth - pluses)}`; +} + +function formatPatches(files: LeanCodeDiffFile[]): string { + let output = ""; + for (const file of files) { + const patchFile = requirePatch(file); + if (patchFile.patch !== undefined) { + const patch = patchFile.patch; + output += patch; + if (!patch.endsWith("\n")) output += "\n"; + continue; + } + output += `${patchFallback(patchFile)}\n`; + } + return output; +} + +function patchFallback(file: LeanCodeDiffPatchFile): string { + const path = quoteGitPath(file.path); + switch (file.contentStatus) { + case "binary": + return `Binary file ${path} differs`; + case "metadata_only": + return `Metadata changed: ${path}`; + case "omitted": { + const reason = file.contentOmissionReason + ? ` (${safe(file.contentOmissionReason)})` + : ""; + return `Patch omitted: ${path}${reason}`; + } + case "unavailable": + return `Patch unavailable: ${path}`; + default: + return `No textual patch: ${path}`; + } +} + +function shouldSuppressPatch( + envelope: LeanCodeDiffEnvelope, + options: FormatCodeDiffTerminalOptions, +): boolean { + if (envelope.view !== "patch") return false; + if ( + !envelope.summary.inventoryComplete || + envelope.summary.unprojectableFiles > 0 || + (envelope.hasMoreFiles && !options.explicitMaxFiles) || + envelope.contentCoverage === "failed" || + (envelope.contentCoverage === "partial" && !options.explicitMaxPatchBytes) + ) { + return true; + } + if (envelope.files.length === 0) return false; + + return envelope.files.some((file) => { + const patchFile = requirePatch(file); + if ( + patchFile.pathEncoding === "byte_escaped" || + patchFile.contentSafety.filtered + ) { + return true; + } + if (patchFile.patch !== undefined) return false; + return !( + options.explicitMaxPatchBytes && + patchFile.contentStatus === "omitted" && + patchFile.contentOmissionReason !== undefined && + EXPLICIT_PATCH_BUDGET_OMISSION_REASONS.has( + patchFile.contentOmissionReason, + ) + ); + }); +} + +function buildDiagnostics( + envelope: LeanCodeDiffEnvelope, + options: FormatCodeDiffTerminalOptions, +): string[] { + const lines: string[] = []; + if (options.verbose) appendVerboseContext(lines, envelope); + + if (envelope.scope.status === "unknown") { + let message = + "Showing changes for the entire repository because GitHits could not identify this package's directory. Unrelated files may be included."; + if (envelope.scope.pathGlob) { + message = + "GitHits could not identify this package's directory, so the path glob was applied across the entire repository. Matching files from other packages may be included; narrow the path glob if needed."; + } else if (!envelope.hasMoreFiles) { + message += " Add a path glob after `--` to narrow the diff."; + } + lines.push(warn(message, options)); + } + if (!envelope.summary.inventoryComplete) { + lines.push( + warn("The authoritative file inventory is incomplete.", options), + ); + } + if (envelope.hasMoreFiles) { + const recovery = envelope.scope.pathGlob + ? "Narrow the path glob or raise --max-files (up to 300)." + : "Add a path glob after `--` or raise --max-files (up to 300)."; + lines.push( + warn( + `More matching files exist than the ${envelope.files.length} returned. ${recovery}`, + options, + ), + ); + } + if (envelope.summary.unprojectableFiles > 0) { + lines.push( + warn( + `${envelope.summary.unprojectableFiles} matching path(s) could not be projected safely.`, + options, + ), + ); + } + if (envelope.contentCoverage === "partial") { + lines.push( + warn( + envelope.view === "patch" + ? "Requested content is partial; inspect per-file status with --stat or --json." + : "Requested content is partial; inspect the returned rows or use --json.", + options, + ), + ); + } else if (envelope.contentCoverage === "failed") { + const failure = envelope.contentFailure; + const detail = failure + ? ` (${[failure.code, failure.stage, failure.limitKind] + .filter((value): value is string => Boolean(value)) + .map(safe) + .join(", ")})` + : ""; + lines.push( + warn( + `Requested content failed after the file inventory completed${detail}.`, + options, + ), + ); + } + + const byteEscaped = envelope.files.filter( + (file) => file.pathEncoding === "byte_escaped", + ).length; + if (byteEscaped > 0) { + lines.push( + warn( + `${byteEscaped} path(s) are display-only byte escapes and cannot be reused as exact identities.`, + options, + ), + ); + } + const filtered = envelope.files.filter( + (file) => "contentSafety" in file && file.contentSafety.filtered, + ).length; + if (filtered > 0) { + lines.push( + warn(`${filtered} patch(es) were modified for content safety.`, options), + ); + } + if (envelope.view === "patch") { + const binary = envelope.files.filter( + (file) => "contentStatus" in file && file.contentStatus === "binary", + ).length; + if (binary > 0) { + lines.push( + warn( + `${binary} binary ${binary === 1 ? "change" : "changes"} cannot be represented as an applicable text patch.`, + options, + ), + ); + } + const metadataOnly = envelope.files.filter( + (file) => + "contentStatus" in file && file.contentStatus === "metadata_only", + ).length; + if (metadataOnly > 0) { + lines.push( + warn( + `${metadataOnly} metadata-only ${metadataOnly === 1 ? "change" : "changes"} cannot be represented as an applicable text patch.`, + options, + ), + ); + } + } + return lines; +} + +function appendVerboseContext( + lines: string[], + envelope: LeanCodeDiffEnvelope, +): void { + const target = + envelope.target.kind === "package" + ? `${envelope.target.registry}:${safe(envelope.target.name)}` + : safe(envelope.target.repoUrl); + lines.push(`target: ${target}`); + lines.push( + `range: ${safe(envelope.from.requested)} (${envelope.from.commitSha}) -> ${safe(envelope.to.requested)} (${envelope.to.commitSha})`, + ); + lines.push( + `summary: ${envelope.summary.filesChanged} changed, ${envelope.summary.added} added, ${envelope.summary.deleted} deleted, ${envelope.summary.modified} modified`, + ); + const roots = + envelope.scope.fromSubpath !== undefined || + envelope.scope.toSubpath !== undefined + ? `, roots ${JSON.stringify(envelope.scope.fromSubpath ?? "?")} -> ${JSON.stringify(envelope.scope.toSubpath ?? "?")}` + : ""; + const filter = envelope.scope.pathGlob + ? `, glob ${JSON.stringify(safe(envelope.scope.pathGlob))}` + : ""; + lines.push(`scope: ${envelope.scope.status}${roots}${filter}`); + lines.push( + `returned: ${envelope.files.length}, content: ${envelope.contentCoverage}`, + ); +} + +function requireStatus( + file: LeanCodeDiffFile, +): "added" | "deleted" | "modified" { + if (!("status" in file)) + throw new Error("CodeDiff status view lacks status."); + return file.status; +} + +function requireStat(file: LeanCodeDiffFile): LeanCodeDiffStatFile { + if (!("contentStatus" in file)) { + throw new Error("CodeDiff stat view lacks content status."); + } + return file; +} + +function requirePatch(file: LeanCodeDiffFile): LeanCodeDiffPatchFile { + if (!("contentSafety" in file)) { + throw new Error("CodeDiff patch view lacks content safety."); + } + return file; +} + +function statusLetter(status: "added" | "deleted" | "modified"): string { + return status === "added" ? "A" : status === "deleted" ? "D" : "M"; +} + +function contentLabel(status: LeanCodeDiffStatFile["contentStatus"]): string { + switch (status) { + case "binary": + return "binary content differs"; + case "metadata_only": + return "metadata differs"; + case "omitted": + return "content omitted"; + case "unavailable": + return "content unavailable"; + default: + return "line statistics unavailable"; + } +} + +function formatLines(lines: string[]): string { + return lines.length > 0 ? `${lines.join("\n")}\n` : ""; +} + +function warn(text: string, options: FormatCodeDiffTerminalOptions): string { + return warning(text, options.useColors); +} + +function safe(value: string): string { + return sanitizeTerminalText(value); +} diff --git a/packages/mcp/src/shared/code-navigation-error-map.test.ts b/packages/mcp/src/shared/code-navigation-error-map.test.ts index 6e15834..9b64bc2 100644 --- a/packages/mcp/src/shared/code-navigation-error-map.test.ts +++ b/packages/mcp/src/shared/code-navigation-error-map.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import { AuthenticationError, ClientUpdateRequiredError, + CodeDiffError, CodeNavigationAccessError, CodeNavigationBackendError, CodeNavigationFeatureFlagRequiredError, @@ -64,6 +65,127 @@ describe("mapCodeNavigationError", () => { }); }); + it("classifies CodeDiff version failures with bounded recovery details", () => { + const error = new CodeDiffError("Version was not published.", { + code: "VERSION_NOT_FOUND", + retryable: false, + side: "from", + registry: "NPM", + publishedVersions: ["2.0.0", "1.0.0"], + publishedVersionsTruncated: true, + }); + + expect(mapCodeNavigationError(error)).toEqual({ + code: "VERSION_NOT_FOUND", + message: "Version was not published.", + retryable: false, + details: { + side: "from", + registry: "NPM", + publishedVersions: ["2.0.0", "1.0.0"], + publishedVersionsTruncated: true, + }, + }); + }); + + it("classifies ambiguous CodeDiff refs without exposing the raw code", () => { + const error = new CodeDiffError("Ref is ambiguous.", { + code: "AMBIGUOUS_REF", + repoUrl: "https://github.com/example/repo", + gitRef: "release", + availableRefs: [{ ref: "refs/heads/release" }], + suggestedRefs: [{ ref: "refs/tags/release", version: "1.0.0" }], + refKinds: ["BRANCH", "TAG"], + }); + + expect(mapCodeNavigationError(error)).toEqual({ + code: "REF_NOT_FOUND", + message: "Ref is ambiguous.", + retryable: false, + details: { + repoUrl: "https://github.com/example/repo", + gitRef: "release", + availableRefs: [{ ref: "refs/heads/release" }], + suggestedRefs: [{ ref: "refs/tags/release", version: "1.0.0" }], + refKinds: ["BRANCH", "TAG"], + }, + }); + }); + + it("preserves exact root identity for a CodeDiff raw-field failure", () => { + const error = new CodeDiffError( + "Raw diff limit exceeded.", + { + code: "RAW_DIFF_LIMIT_EXCEEDED", + retryable: false, + stage: "content", + limitKind: "max_content_entries", + }, + { + package: { + registry: "NPM", + name: "example", + repoUrl: "https://github.com/example/repo", + }, + fromResolution: { + requested: "1.0.0", + resolvedVersion: "1.0.0", + ref: "v1.0.0", + commitSha: "0123456789abcdef0123456789abcdef01234567", + refKind: "TAG", + versionSource: "REGISTRY", + }, + toResolution: { + requested: "2.0.0", + resolvedVersion: "2.0.0", + ref: "v2.0.0", + commitSha: "fedcba9876543210fedcba9876543210fedcba98", + refKind: "TAG", + versionSource: "REGISTRY", + }, + }, + ); + + const mapped = mapCodeNavigationError(error); + expect(mapped.code).toBe("BACKEND_ERROR"); + expect(mapped.retryable).toBe(false); + expect(mapped.details).toMatchObject({ + stage: "content", + limitKind: "max_content_entries", + codeDiffResolution: { + package: { registry: "NPM", name: "example" }, + from: { commitSha: "0123456789abcdef0123456789abcdef01234567" }, + to: { commitSha: "fedcba9876543210fedcba9876543210fedcba98" }, + }, + }); + expect(mapped.details).not.toHaveProperty("graphqlCode"); + }); + + it.each([ + ["VALIDATION_ERROR", "INVALID_ARGUMENT", false], + ["REF_NOT_FOUND", "REF_NOT_FOUND", false], + ["REPOSITORY_NOT_FOUND", "NOT_FOUND", false], + ["TIMEOUT", "TIMEOUT", true], + ["RATE_LIMITED", "RATE_LIMITED", true], + ["RAW_DIFF_UNAVAILABLE", "BACKEND_ERROR", false], + [undefined, "BACKEND_ERROR", false], + ] as const)( + "maps CodeDiff code %s to %s", + (graphqlCode, expectedCode, retryable) => { + const mapped = mapCodeNavigationError( + new CodeDiffError("failure", { + code: graphqlCode, + retryAfterMs: 250, + }), + ); + expect(mapped).toMatchObject({ + code: expectedCode, + retryable, + details: { retryAfterMs: 250 }, + }); + }, + ); + it("classifies CodeNavigationTargetNotFoundError as NOT_FOUND", () => { const err = new CodeNavigationTargetNotFoundError("Package not found", [ { version: "5.2.1", ref: "v5.2.1" }, @@ -590,6 +712,7 @@ describe("mapCodeNavigationError debug instrumentation", () => { process.env.GITHITS_DEBUG = "*"; const errors: unknown[] = [ new CodeNavigationTargetNotFoundError("x"), + new CodeDiffError("x", { code: "RAW_DIFF_UNAVAILABLE" }), new CodeNavigationFileNotFoundError("x", "some/path"), new CodeNavigationIndexingError("x"), new CodeNavigationRefNotFoundError( diff --git a/packages/mcp/src/shared/code-navigation-error-map.ts b/packages/mcp/src/shared/code-navigation-error-map.ts index 11ac24f..06d3b49 100644 --- a/packages/mcp/src/shared/code-navigation-error-map.ts +++ b/packages/mcp/src/shared/code-navigation-error-map.ts @@ -5,6 +5,9 @@ import { type AvailableVersion, CLIENT_UPDATE_REQUIRED_REASON, ClientUpdateRequiredError, + CodeDiffError, + type CodeDiffPackageInfo, + type CodeDiffRefResolution, CodeNavigationAccessError, CodeNavigationBackendError, type CodeNavigationErrorMetadata, @@ -91,6 +94,28 @@ export interface MappedErrorDetails { termsUrl?: string; /** Authenticated web UI where the user can accept the current terms. */ acceptanceUrl?: string; + /** CodeDiff comparison side involved in a bounded resolver failure. */ + side?: string; + /** Published package versions supplied for CodeDiff recovery. */ + publishedVersions?: string[]; + publishedVersionsTruncated?: boolean; + /** Canonical registry supplied by a CodeDiff resolver failure. */ + registry?: string; + /** Retry delay retained at millisecond precision for CodeDiff. */ + retryAfterMs?: number; + /** Bounded raw-diff failure stage and limit identifier. */ + stage?: string; + limitKind?: string; + /** Git ref supplied by a CodeDiff resolver failure. */ + gitRef?: string; + /** Ref classifications retained for ambiguous-ref recovery. */ + refKinds?: string[]; + /** Immutable root identity retained when only the raw field failed. */ + codeDiffResolution?: { + package?: CodeDiffPackageInfo; + from: CodeDiffRefResolution; + to: CodeDiffRefResolution; + }; } export interface MappedError { @@ -155,6 +180,9 @@ function classify(error: unknown): MappedError { if (error instanceof ClientUpdateRequiredError) { return buildUpdateRequiredError(error.reason, error.currentVersion); } + if (error instanceof CodeDiffError) { + return classifyCodeDiffError(error); + } if (error instanceof CodeNavigationVersionNotFoundError) { const details: MappedErrorDetails = {}; preserveBackendMetadata(details, error.metadata); @@ -311,6 +339,70 @@ function classify(error: unknown): MappedError { return { code: "UNKNOWN", message: "Unknown error", retryable: false }; } +function classifyCodeDiffError(error: CodeDiffError): MappedError { + const details: MappedErrorDetails = {}; + const source = error.details; + + if (source?.side !== undefined) details.side = source.side; + if (source?.publishedVersions !== undefined) { + details.publishedVersions = source.publishedVersions; + } + if (source?.publishedVersionsTruncated !== undefined) { + details.publishedVersionsTruncated = source.publishedVersionsTruncated; + } + if (source?.registry !== undefined) details.registry = source.registry; + if (source?.retryAfterMs !== undefined) { + details.retryAfterMs = source.retryAfterMs; + } + if (source?.stage !== undefined) details.stage = source.stage; + if (source?.limitKind !== undefined) details.limitKind = source.limitKind; + if (source?.repoUrl !== undefined) details.repoUrl = source.repoUrl; + if (source?.gitRef !== undefined) details.gitRef = source.gitRef; + if (source?.availableRefs !== undefined) { + details.availableRefs = source.availableRefs.map((entry) => ({ ...entry })); + } + if (source?.suggestedRefs !== undefined) { + details.suggestedRefs = source.suggestedRefs.map((entry) => ({ ...entry })); + } + if (source?.refKinds !== undefined) details.refKinds = source.refKinds; + if (error.partial !== undefined) { + details.codeDiffResolution = { + package: error.partial.package, + from: error.partial.fromResolution, + to: error.partial.toResolution, + }; + } + + const build = ( + code: MappedErrorCode, + defaultRetryable: boolean, + ): MappedError => ({ + code, + message: error.message, + retryable: source?.retryable ?? defaultRetryable, + details: Object.keys(details).length > 0 ? details : undefined, + }); + + switch (source?.code) { + case "VALIDATION_ERROR": + return build("INVALID_ARGUMENT", false); + case "VERSION_NOT_FOUND": + return build("VERSION_NOT_FOUND", false); + case "REF_NOT_FOUND": + case "AMBIGUOUS_REF": + return build("REF_NOT_FOUND", false); + case "REPOSITORY_NOT_FOUND": + case "PACKAGE_NOT_FOUND": + return build("NOT_FOUND", false); + case "TIMEOUT": + return build("TIMEOUT", true); + case "RATE_LIMITED": + return build("RATE_LIMITED", true); + default: + return build("BACKEND_ERROR", false); + } +} + export function buildUpdateRequiredError( reason: string = CLIENT_UPDATE_REQUIRED_REASON, currentVersion?: string, diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index fe0e54d..12f5f71 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -492,6 +492,16 @@ async function assertUnauthenticatedBehavior(): Promise { "resolve help should disclose query privacy guidance", ); + const codeDiffHelp = await runCliWithEnv(["code", "diff", "--help"], env); + assert(codeDiffHelp.exitCode === 0, "code diff help should succeed"); + assert( + codeDiffHelp.stdout.includes("..") && + codeDiffHelp.stdout.includes("repository-relative") && + codeDiffHelp.stdout.includes("--name-status") && + !codeDiffHelp.stdout.includes("--git-ref"), + "code diff help should expose the bounded dogfood contract", + ); + for (const command of ["init", "login"] as const) { const commandHelp = await runCliWithEnv([command, "--help"], env); assert(commandHelp.exitCode === 0, `${command} help should succeed`); @@ -541,6 +551,23 @@ async function assertUnauthenticatedBehavior(): Promise { "unauthenticated languages JSON envelope", ); + const codeDiffAuth = await runCliWithEnv( + ["code", "diff", "npm:express", "5.2.0..5.2.1", "--json"], + env, + ); + assert( + codeDiffAuth.exitCode !== 0 && codeDiffAuth.stdout.trim() === "", + "unauthenticated code diff JSON should fail with clean stdout", + ); + const codeDiffAuthPayload = assertCleanErrorEnvelope( + codeDiffAuth.stderr, + "unauthenticated code diff", + ); + assert( + codeDiffAuthPayload.code === "AUTH_REQUIRED", + "unauthenticated code diff should preserve the shared auth envelope", + ); + const resolveJson = await runCliWithEnv( ["resolve", "express", "--json"], env, @@ -1165,6 +1192,84 @@ async function runLiveSmoke(): Promise { "code grep invalid json missing CLI-native recovery", ); + const codeDiffJson = assertJsonOutput( + await runCli([ + "code", + "diff", + "npm:express", + "5.2.0..5.2.1", + "--name-only", + "--max-files", + "2", + "--json", + "--", + "**/*.js", + ]), + "code diff json", + ); + assertRecord(codeDiffJson, "code diff json"); + assert( + codeDiffJson.view === "name-only" && + Array.isArray(codeDiffJson.files) && + typeof codeDiffJson.from === "object" && + typeof codeDiffJson.to === "object", + "code diff json missing selected view or exact resolutions", + ); + + const codeDiffPatchText = assertTerminalOutput( + await runCli([ + "code", + "diff", + "npm:express", + "5.2.0..5.2.1", + "--", + "lib/utils.js", + ]), + "code diff patch", + ); + assert( + codeDiffPatchText.startsWith("--- a/lib/utils.js\n+++ b/lib/utils.js\n") && + !codeDiffPatchText.includes("--- a/file\n+++ b/file\n"), + "code diff patch should bind headers to the authoritative file path", + ); + + const codeDiffInvalid = await runCli([ + "code", + "diff", + "npm:express", + "5.2.0...5.2.1", + "--json", + ]); + const codeDiffInvalidEnvelope = assertCleanErrorEnvelope( + codeDiffInvalid.stderr, + "code diff invalid json", + ); + assert( + codeDiffInvalid.exitCode !== 0 && + codeDiffInvalidEnvelope.code === "INVALID_ARGUMENT" && + codeDiffInvalidEnvelope.error.includes("not `...`"), + "code diff invalid JSON should explain the two-dot range", + ); + + const codeDiffBareGlob = await runCli([ + "code", + "diff", + "npm:express", + "5.2.0..5.2.1", + "**/*.js", + "--json", + ]); + const codeDiffBareGlobEnvelope = assertCleanErrorEnvelope( + codeDiffBareGlob.stderr, + "code diff bare glob JSON", + ); + assert( + codeDiffBareGlob.exitCode !== 0 && + codeDiffBareGlobEnvelope.code === "INVALID_ARGUMENT" && + codeDiffBareGlobEnvelope.error.includes("after `--`"), + "code diff should require the Git-style glob delimiter", + ); + const searchText = assertTerminalOutput( await runCli([ "search", diff --git a/src/commands/code/diff.test.ts b/src/commands/code/diff.test.ts new file mode 100644 index 0000000..59b0051 --- /dev/null +++ b/src/commands/code/diff.test.ts @@ -0,0 +1,544 @@ +import { describe, expect, it, mock, spyOn } from "bun:test"; +import { Command } from "commander"; +import { + createMockCodeNavigationService, + defaultCodeDiffResult, +} from "../../services/test-helpers.js"; +import { + type CodeDiffCommandDependencies, + codeDiffAction, + formatCodeDiffError, + registerCodeDiffCommand, +} from "./diff.js"; + +function dependencies( + overrides: Partial = {}, +): CodeDiffCommandDependencies { + return { + codeNavigationService: createMockCodeNavigationService(), + codeNavigationUrl: "https://pkgseer.dev/graphql", + hasValidToken: true, + mcpUrl: "https://mcp.githits.com", + ...overrides, + }; +} + +describe("codeDiffAction", () => { + it("uses patch mode by default and omits backend defaults", async () => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const stdout = spyOn(process.stdout, "write").mockImplementation( + (() => true) as typeof process.stdout.write, + ); + + await codeDiffAction( + "npm:express", + "4.18.1..4.18.2", + undefined, + {}, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + ); + + expect(codeDiff).toHaveBeenCalledWith({ + target: { registry: "NPM", packageName: "express" }, + from: "4.18.1", + to: "4.18.2", + mode: "patches", + }); + expect(String(stdout.mock.calls[0]?.[0])).toContain("@@ -1 +1 @@"); + stdout.mockRestore(); + }); + + it("maps repo mode, inventory view, bounds, and one positional glob", async () => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const stdout = spyOn(process.stdout, "write").mockImplementation( + (() => true) as typeof process.stdout.write, + ); + + await codeDiffAction( + "v1..v2", + "src/**/*.ts", + undefined, + { + repoUrl: "https://github.com/expressjs/express", + nameStatus: true, + maxFiles: "12", + }, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + true, + ); + + expect(codeDiff).toHaveBeenCalledWith({ + target: { repoUrl: "https://github.com/expressjs/express" }, + from: "v1", + to: "v2", + mode: "inventory", + options: { maxFiles: 12, pathGlob: "src/**/*.ts" }, + }); + expect(String(stdout.mock.calls[0]?.[0])).toBe("M\tlib/express.js\n"); + stdout.mockRestore(); + }); + + it("emits the selected-view JSON envelope", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + + await codeDiffAction( + "npm:express", + "4.18.1..4.18.2", + undefined, + { nameOnly: true, json: true }, + dependencies(), + ); + + const payload = JSON.parse(log.mock.calls[0]?.[0] as string); + expect(payload.view).toBe("name-only"); + expect(payload.from.commitSha).toBe("from-sha"); + expect(payload.files).toEqual([ + { path: "lib/express.js", pathEncoding: "utf8" }, + ]); + log.mockRestore(); + }); + + it("emits authoritative patch headers in JSON", async () => { + const log = spyOn(console, "log").mockImplementation(() => {}); + const result = structuredClone(defaultCodeDiffResult); + result.raw.files[0] = { + ...result.raw.files[0]!, + patch: "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n+new\n", + }; + + await codeDiffAction( + "npm:express", + "4.18.1..4.18.2", + undefined, + { json: true }, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ + codeDiff: mock(() => Promise.resolve(result)), + }), + }), + ); + + const payload = JSON.parse(log.mock.calls[0]?.[0] as string); + expect(payload.files[0].path).toBe("lib/express.js"); + expect(payload.files[0].patch).toStartWith( + "--- a/lib/express.js\n+++ b/lib/express.js\n", + ); + log.mockRestore(); + }); + + it("suppresses failed post-inventory patch content and exits nonzero", async () => { + const stdout = spyOn(process.stdout, "write").mockImplementation( + (() => true) as typeof process.stdout.write, + ); + const stderr = spyOn(process.stderr, "write").mockImplementation( + (() => true) as typeof process.stderr.write, + ); + const result = structuredClone(defaultCodeDiffResult); + result.raw.contentCoverage = "FAILED"; + result.raw.contentFailure = { + code: "RAW_DIFF_LIMIT_EXCEEDED", + retryable: false, + stage: "content", + limitKind: "max_content_entries", + }; + result.raw.files[0] = { + ...result.raw.files[0]!, + patch: undefined, + additions: undefined, + deletions: undefined, + contentStatus: "UNAVAILABLE", + }; + + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + try { + await codeDiffAction( + "npm:express", + "4.18.1..4.18.2", + undefined, + {}, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ + codeDiff: mock(() => Promise.resolve(result)), + }), + }), + ); + } catch { + // process.exit is mocked as a throw. + } + + expect(String(stderr.mock.calls[0]?.[0])).toContain( + "Requested content failed", + ); + expect(String(stderr.mock.calls[0]?.[0])).toContain( + "Patch output was suppressed", + ); + expect(stdout).not.toHaveBeenCalled(); + expect(exit).toHaveBeenCalledWith(1); + stdout.mockRestore(); + stderr.mockRestore(); + exit.mockRestore(); + }); + + it.each([ + [{ patch: true, stat: true }, "Choose only one diff view", undefined], + [ + { stat: true, maxPatchBytes: "2048" }, + "`--max-patch-bytes` is valid only", + "maxPatchBytes", + ], + [{ maxFiles: "0" }, "--max-files expects", undefined], + ] as const)( + "rejects invalid options before network I/O", + async (options, text, forbidden) => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + try { + await codeDiffAction( + "npm:express", + "1.0.0..2.0.0", + undefined, + options, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ + codeDiff, + }), + }), + ); + } catch { + // process.exit is mocked as a throw. + } + + expect(error.mock.calls[0]?.[0]).toContain(text); + if (forbidden) expect(error.mock.calls[0]?.[0]).not.toContain(forbidden); + expect(codeDiff).not.toHaveBeenCalled(); + error.mockRestore(); + exit.mockRestore(); + }, + ); + + it("rejects a third positional in repo mode", async () => { + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + try { + await codeDiffAction( + "v1..v2", + "src/**", + "extra", + { repoUrl: "https://github.com/x/y" }, + dependencies(), + ); + } catch { + // process.exit is mocked as a throw. + } + expect(error.mock.calls[0]?.[0]).toContain("at most one"); + error.mockRestore(); + exit.mockRestore(); + }); + + it("uses CLI-native wording for invalid path globs", async () => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + try { + await codeDiffAction( + "npm:express", + "1.0.0..2.0.0", + ":(exclude)lib/**", + {}, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + true, + ); + } catch { + // process.exit is mocked as a throw. + } + expect(error.mock.calls[0]?.[0]).toContain("``"); + expect(error.mock.calls[0]?.[0]).toContain("pathspec magic"); + expect(error.mock.calls[0]?.[0]).not.toContain("pathGlob"); + expect(codeDiff).not.toHaveBeenCalled(); + error.mockRestore(); + exit.mockRestore(); + }); + + it.each([ + { + arg1: "npm:express@5.0.0", + arg2: "1.0.0..2.0.0", + options: {}, + expected: "`..`", + forbidden: "`range`", + }, + { + arg1: "1.0.0..2.0.0", + arg2: undefined, + options: { repoUrl: "npm:express" }, + expected: "`--repo-url`", + forbidden: "`repoUrl`", + }, + ])("uses CLI names for target validation", async (testCase) => { + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + try { + await codeDiffAction( + testCase.arg1, + testCase.arg2, + undefined, + testCase.options, + dependencies(), + ); + } catch { + // process.exit is mocked as a throw. + } + expect(error.mock.calls[0]?.[0]).toContain(testCase.expected); + expect(error.mock.calls[0]?.[0]).not.toContain(testCase.forbidden); + error.mockRestore(); + exit.mockRestore(); + }); + + it("requires the Git-style -- delimiter before a path glob", async () => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + try { + await codeDiffAction( + "npm:express", + "1.0.0..2.0.0", + "src/**/*.ts", + {}, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + ); + } catch { + // process.exit is mocked as a throw. + } + expect(error.mock.calls[0]?.[0]).toContain("after `--`"); + expect(codeDiff).not.toHaveBeenCalled(); + error.mockRestore(); + exit.mockRestore(); + }); + + it("uses diff-specific target guidance", async () => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + + try { + await codeDiffAction( + "express", + "1.0.0..2.0.0", + undefined, + {}, + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + ); + } catch { + // process.exit is mocked as a throw. + } + + expect(error.mock.calls[0]?.[0]).toContain( + "unversioned package target `:`", + ); + expect(error.mock.calls[0]?.[0]).not.toContain("[@]"); + expect(error.mock.calls[0]?.[0]).not.toContain("[#ref|@ref]"); + expect(error.mock.calls[0]?.[0]).toContain("supported registries"); + expect(error.mock.calls[0]?.[0]).toContain("npm"); + expect(codeDiff).not.toHaveBeenCalled(); + error.mockRestore(); + exit.mockRestore(); + }); +}); + +describe("formatCodeDiffError", () => { + it("renders bounded recovery fields", () => { + const output = formatCodeDiffError({ + code: "VERSION_NOT_FOUND", + message: "Version was not found.", + retryable: false, + details: { + side: "from", + publishedVersions: ["2.0.0", "1.0.0"], + publishedVersionsTruncated: true, + stage: "resolution", + }, + }); + + expect(output).toContain("side: from"); + expect(output).toContain("stage: resolution"); + expect(output).toContain("2.0.0, 1.0.0 (+more)"); + }); + + it("marks locally truncated recovery lists", () => { + const output = formatCodeDiffError({ + code: "REF_NOT_FOUND", + message: "Ref was not found.", + retryable: false, + details: { + availableRefs: Array.from({ length: 10 }, (_, index) => ({ + ref: `ref-${index}`, + })), + }, + }); + + expect(output).toContain("ref-0, ref-1"); + expect(output).toContain("(+2 more)"); + }); + + it("preserves a local lower bound when the backend also truncated", () => { + const output = formatCodeDiffError({ + code: "VERSION_NOT_FOUND", + message: "Version was not found.", + retryable: false, + details: { + publishedVersions: Array.from( + { length: 10 }, + (_, index) => `${index}.0.0`, + ), + publishedVersionsTruncated: true, + }, + }); + + expect(output).toContain("(+2+ more)"); + }); +}); + +describe("registerCodeDiffCommand", () => { + it("registers the dogfood surface without --git-ref", () => { + const parent = new Command("code"); + const command = registerCodeDiffCommand(parent); + + expect(command.name()).toBe("diff"); + expect( + command.options.some((option) => option.long === "--name-status"), + ).toBe(true); + expect(command.options.some((option) => option.long === "--git-ref")).toBe( + false, + ); + }); + + it("shows the two concrete invocation forms in help", () => { + const program = new Command("githits"); + const command = registerCodeDiffCommand(program.command("code")); + const help = command.helpInformation(); + + expect(help).toContain( + "githits code diff [options] .. [-- ]", + ); + expect(help).toContain( + "githits code diff [options] --repo-url .. [-- ]", + ); + expect(help).not.toContain("[target-or-range] [range-or-path-glob]"); + expect(help).toContain("Target examples: `npm:express`"); + expect(help).toContain("suppressed patch output exits 1"); + }); + + it.each(["src/**/*.ts", "--"])( + "accepts path glob %s when the raw argv suffix is -- ", + async (pathGlob) => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const stdout = spyOn(process.stdout, "write").mockImplementation( + (() => true) as typeof process.stdout.write, + ); + const program = new Command("githits"); + const code = program.command("code"); + registerCodeDiffCommand(code, async () => + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + ); + + await program.parseAsync([ + "node", + "githits", + "code", + "diff", + "npm:express", + "1.0.0..2.0.0", + "--", + pathGlob, + ]); + + const calls = codeDiff.mock.calls as unknown as Array< + [{ options?: { pathGlob?: string } }] + >; + expect(calls[0]?.[0].options?.pathGlob).toBe(pathGlob); + stdout.mockRestore(); + }, + ); + + it.each([ + [ + "glob before trailing delimiter", + [ + "node", + "githits", + "code", + "diff", + "npm:express", + "1.0.0..2.0.0", + "src/**/*.ts", + "--", + ], + ], + [ + "root delimiter", + [ + "node", + "githits", + "--", + "code", + "diff", + "npm:express", + "1.0.0..2.0.0", + "src/**/*.ts", + ], + ], + ] as const)("rejects a path glob with a %s", async (_label, argv) => { + const codeDiff = mock(() => Promise.resolve(defaultCodeDiffResult)); + const error = spyOn(console, "error").mockImplementation(() => {}); + const exit = spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit"); + }); + const program = new Command("githits"); + const code = program.command("code"); + registerCodeDiffCommand(code, async () => + dependencies({ + codeNavigationService: createMockCodeNavigationService({ codeDiff }), + }), + ); + + try { + await program.parseAsync([...argv]); + } catch { + // process.exit is mocked as a throw. + } + + expect(error.mock.calls[0]?.[0]).toContain("after `--`"); + expect(codeDiff).not.toHaveBeenCalled(); + error.mockRestore(); + exit.mockRestore(); + }); +}); diff --git a/src/commands/code/diff.ts b/src/commands/code/diff.ts new file mode 100644 index 0000000..94daaf3 --- /dev/null +++ b/src/commands/code/diff.ts @@ -0,0 +1,374 @@ +import type { CodeNavigationService } from "@githits/core-internal"; +import { + buildCodeDiffParams, + buildCodeDiffSuccessPayload, + CODE_DIFF_MAX_FILES_MAX, + CODE_DIFF_MAX_FILES_MIN, + CODE_DIFF_MAX_PATCH_BYTES_MAX, + CODE_DIFF_MAX_PATCH_BYTES_MIN, + type CodeDiffRequestBuildResult, + type CodeDiffRequestInput, + type CodeDiffView, + formatCodeDiffTerminal, + InvalidPackageSpecError, + type MappedError, + requireAuth, + sanitizeTerminalText, + shouldUseColors, +} from "@githits/mcp/internal"; +import type { Command } from "commander"; +import { createContainer } from "../../container.js"; +import { startSpinner } from "../../shared/spinner.js"; +import { SPINNER_MESSAGES } from "../../shared/spinner-messages.js"; +import { formatMappedErrorForTerminal } from "../format-mapped-error.js"; +import { + handleCodeNavCommandError, + parseIntCliOption, +} from "./code-nav-cli-helpers.js"; + +export interface CodeDiffCommandOptions { + repoUrl?: string; + patch?: boolean; + stat?: boolean; + nameOnly?: boolean; + nameStatus?: boolean; + maxFiles?: string; + maxPatchBytes?: string; + verbose?: boolean; + json?: boolean; +} + +export interface CodeDiffCommandDependencies { + codeNavigationService: CodeNavigationService | undefined; + codeNavigationUrl: string | undefined; + hasValidToken: boolean; + mcpUrl: string; +} + +export type CodeDiffCommandDependencyFactory = + () => Promise; + +interface RootCommandWithRawArgs extends Command { + rawArgs?: string[]; +} + +/** Execute the silent-dogfood CodeDiff CLI adapter. */ +export async function codeDiffAction( + arg1: string | undefined, + arg2: string | undefined, + arg3: string | undefined, + options: CodeDiffCommandOptions, + deps: CodeDiffCommandDependencies, + pathGlobAfterDoubleDash = false, +): Promise { + let terminalExitCode: 1 | undefined; + try { + requireAuth(deps); + } catch (error) { + if (options.json) { + handleCodeNavCommandError(error, true, formatCodeDiffError); + } + throw error; + } + + try { + if (!deps.codeNavigationUrl || !deps.codeNavigationService) { + throw new InvalidPackageSpecError( + "Code navigation is not configured for this environment.", + ); + } + + const positionals = resolvePositionals( + arg1, + arg2, + arg3, + options.repoUrl !== undefined, + ); + if (positionals.pathGlob !== undefined && !pathGlobAfterDoubleDash) { + throw new InvalidPackageSpecError( + "Pass the repository-relative after `--`.", + ); + } + const view = resolveView(options); + const build = buildCliCodeDiffParams({ + target: positionals.target, + repoUrl: options.repoUrl, + range: positionals.range, + view, + pathGlob: positionals.pathGlob, + maxFiles: parseIntCliOption( + options.maxFiles, + "--max-files", + CODE_DIFF_MAX_FILES_MIN, + CODE_DIFF_MAX_FILES_MAX, + ), + maxPatchBytes: parseIntCliOption( + options.maxPatchBytes, + "--max-patch-bytes", + CODE_DIFF_MAX_PATCH_BYTES_MIN, + CODE_DIFF_MAX_PATCH_BYTES_MAX, + ), + }); + + const spinner = startSpinner(SPINNER_MESSAGES.code, !options.json); + const result = await deps.codeNavigationService + .codeDiff(build.params) + .finally(() => spinner.stop()); + const payload = buildCodeDiffSuccessPayload(result, { + target: build.params.target, + view: build.view, + }); + + if (options.json) { + console.log(JSON.stringify(payload)); + return; + } + + const formatted = formatCodeDiffTerminal(payload, { + useColors: shouldUseColors(), + verbose: options.verbose ?? false, + explicitMaxFiles: options.maxFiles !== undefined, + explicitMaxPatchBytes: options.maxPatchBytes !== undefined, + }); + if (formatted.stdout) process.stdout.write(formatted.stdout); + if (formatted.stderr) process.stderr.write(formatted.stderr); + terminalExitCode = formatted.exitCode; + } catch (error) { + handleCodeNavCommandError( + error, + options.json ?? false, + formatCodeDiffError, + ); + } + + if (terminalExitCode !== undefined) process.exit(terminalExitCode); +} + +function buildCliCodeDiffParams( + input: CodeDiffRequestInput, +): CodeDiffRequestBuildResult { + try { + return buildCodeDiffParams(input); + } catch (error) { + if (!(error instanceof InvalidPackageSpecError)) throw error; + const rewritten = error.message + .replace(/`maxPatchBytes`/g, "`--max-patch-bytes`") + .replace(/`maxFiles`/g, "`--max-files`") + .replace(/`pathGlob`/g, "``") + .replace(/`repoUrl`/g, "`--repo-url`") + .replace(/`range`/g, "`..`") + .replace(/CodeDiff view/g, "Diff view"); + if (rewritten === error.message) throw error; + throw new InvalidPackageSpecError(rewritten); + } +} + +interface ResolvedPositionals { + target?: string; + range: string; + pathGlob?: string; +} + +function resolvePositionals( + arg1: string | undefined, + arg2: string | undefined, + arg3: string | undefined, + repoMode: boolean, +): ResolvedPositionals { + if (repoMode) { + if (arg3 !== undefined) { + throw new InvalidPackageSpecError( + "Pass at most one repository-relative after `--`.", + ); + } + if (arg1 === undefined) { + throw new InvalidPackageSpecError( + "A .. range is required after --repo-url.", + ); + } + return { range: arg1, pathGlob: arg2 }; + } + + if (arg1 === undefined) { + throw new InvalidPackageSpecError( + "An unversioned and .. range are required.", + ); + } + if (arg2 === undefined) { + throw new InvalidPackageSpecError( + "A .. range is required after the target.", + ); + } + return { target: arg1, range: arg2, pathGlob: arg3 }; +} + +function resolveView(options: CodeDiffCommandOptions): CodeDiffView { + const selected: CodeDiffView[] = []; + if (options.patch) selected.push("patch"); + if (options.stat) selected.push("stat"); + if (options.nameOnly) selected.push("name-only"); + if (options.nameStatus) selected.push("name-status"); + if (selected.length > 1) { + throw new InvalidPackageSpecError( + "Choose only one diff view: --patch, --stat, --name-only, or --name-status.", + ); + } + return selected[0] ?? "patch"; +} + +/** Render bounded CodeDiff diagnostics without exposing raw GraphQL details. */ +export function formatCodeDiffError(mapped: MappedError): string { + const safe = (value: string): string => sanitizeTerminalText(value); + const safeBlock = (value: string): string => + value + .split("\n") + .map((line) => safe(line)) + .join("\n"); + const lines = [safeBlock(formatMappedErrorForTerminal(mapped))]; + const details = mapped.details; + if (!details) return lines[0] as string; + + if (details.side) lines.push(` side: ${safe(details.side)}`); + if (details.stage) lines.push(` stage: ${safe(details.stage)}`); + if (details.limitKind) lines.push(` limit: ${safe(details.limitKind)}`); + if (details.publishedVersions?.length) { + lines.push( + ` published versions: ${formatRecoveryList( + details.publishedVersions, + safe, + details.publishedVersionsTruncated, + )}`, + ); + } + if (details.availableRefs?.length) { + lines.push( + ` available refs: ${formatRecoveryList( + details.availableRefs.map((entry) => entry.version ?? entry.ref), + safe, + )}`, + ); + } + if (details.suggestedRefs?.length) { + lines.push( + ` suggested refs: ${formatRecoveryList( + details.suggestedRefs.map((entry) => entry.version ?? entry.ref), + safe, + )}`, + ); + } + return lines.join("\n"); +} + +function formatRecoveryList( + values: string[], + safe: (value: string) => string, + backendTruncated = false, +): string { + const limit = 8; + const shown = values.slice(0, limit).map(safe).join(", "); + const omitted = values.length - limit; + if (backendTruncated) { + return omitted > 0 ? `${shown} (+${omitted}+ more)` : `${shown} (+more)`; + } + return omitted > 0 ? `${shown} (+${omitted} more)` : shown; +} + +const CODE_DIFF_DESCRIPTION = `Compare two exact dependency source trees. + +The default output is a bounded patch, matching ordinary \`git diff\` where the +backend contract permits it. Select --stat, --name-only, or --name-status for +cheaper projections. Exactly one view may be selected. + +Addressing: an unversioned plus .., or --repo-url plus +... Versions and refs belong in the range, not the target. Direction +is always left-to-right; three-dot merge-base syntax is not supported. + +After \`--\`, one optional applies a repository-relative bounded +glob. It supports *, ?, and an exact ** path component; it is not a full Git +pathspec. A backslash escapes one following non-slash character. + +Target examples: \`npm:express\` or \`github:expressjs/express\`; keep versions +and refs in ... Empty diffs exit 0; suppressed patch output exits 1. +Patch output is applicable unified-diff content and may omit Git metadata such +as index and mode headers.`; + +export function registerCodeDiffCommand( + codeCommand: Command, + createDependencies: CodeDiffCommandDependencyFactory = createCodeDiffCommandDependencies, +): Command { + return codeCommand + .command("diff") + .summary("Compare two dependency source trees") + .description(CODE_DIFF_DESCRIPTION) + .usage( + "[options] .. [-- ]\n githits code diff [options] --repo-url .. [-- ]", + ) + .argument( + "[target-or-range]", + "Package mode: target. Repository mode: from..to range.", + ) + .argument( + "[range-or-path-glob]", + "Package mode: from..to range. Repository mode: path glob.", + ) + .argument( + "[path-glob]", + "One repository-relative glob; must be passed after `--`.", + ) + .option("--repo-url ", "Public GitHub repository URL addressing") + .option("-p, --patch", "Emit bounded patches (default)") + .option("--stat", "Emit per-file line statistics") + .option("--name-only", "Emit changed paths only") + .option("--name-status", "Emit change status and path") + .option("--max-files ", "Maximum returned files (1-300)") + .option( + "--max-patch-bytes ", + "Maximum aggregate patch bytes (1024-2097152; patch view only)", + ) + .option("-v, --verbose", "Emit exact resolution and scope diagnostics") + .option("--json", "Emit the JSON envelope") + .action( + async ( + arg1: string | undefined, + arg2: string | undefined, + arg3: string | undefined, + options: CodeDiffCommandOptions, + command: Command, + ) => { + const deps = await createDependencies(); + await codeDiffAction( + arg1, + arg2, + arg3, + options, + deps, + pathGlobFollowsDoubleDash( + command, + options.repoUrl !== undefined ? arg2 : arg3, + ), + ); + }, + ); +} + +function pathGlobFollowsDoubleDash( + command: Command, + pathGlob: string | undefined, +): boolean { + if (pathGlob === undefined) return false; + let root = command; + while (root.parent) root = root.parent; + const rawArgs = (root as RootCommandWithRawArgs).rawArgs; + if (!rawArgs) return false; + return rawArgs.at(-2) === "--" && rawArgs.at(-1) === pathGlob; +} + +async function createCodeDiffCommandDependencies(): Promise { + const deps = await createContainer(); + return { + codeNavigationService: deps.codeNavigationService, + codeNavigationUrl: deps.codeNavigationUrl, + hasValidToken: deps.hasValidToken, + mcpUrl: deps.mcpUrl, + }; +} diff --git a/src/commands/code/index.test.ts b/src/commands/code/index.test.ts index 616f3d8..855150f 100644 --- a/src/commands/code/index.test.ts +++ b/src/commands/code/index.test.ts @@ -14,5 +14,8 @@ describe("registerCodeCommandGroup", () => { expect( codeCommand?.commands.some((command) => command.name() === "files"), ).toBe(true); + expect( + codeCommand?.commands.some((command) => command.name() === "diff"), + ).toBe(true); }); }); diff --git a/src/commands/code/index.ts b/src/commands/code/index.ts index 0038001..8e0f31c 100644 --- a/src/commands/code/index.ts +++ b/src/commands/code/index.ts @@ -1,4 +1,5 @@ import type { Command } from "commander"; +import { registerCodeDiffCommand } from "./diff.js"; import { registerCodeFilesCommand } from "./files.js"; import { registerCodeGrepCommand } from "./grep.js"; import { registerCodeReadCommand } from "./read.js"; @@ -13,10 +14,11 @@ export async function registerCodeCommandGroup( .command("code") .summary("Inspect dependency source code and symbols") .description( - "List files, read files, and grep substrings inside indexed dependency source. Every command accepts either `` (registry:name[@version]) or `--repo-url [--git-ref ]`. Omitted package versions use the latest release; omitted repo refs use the default-branch intent. For package-level metadata use `githits pkg`.", + "List files, read files, grep substrings, and compare exact trees inside indexed dependency source. Read/list/grep accept either `` (registry:name[@version]) or `--repo-url [--git-ref ]`; diff keeps both versions or refs in its required `..` range. For package-level metadata use `githits pkg`.", ); registerCodeFilesCommand(codeCommand); registerCodeReadCommand(codeCommand); registerCodeGrepCommand(codeCommand); + registerCodeDiffCommand(codeCommand); }