fix(pkg-deps): resolve runtime.version on MCP + reject silent no-op flag combos

docs/implementation/cli-commands.md

@@ -166,7 +166,7 @@ Analyses dependencies for a package on npm, PyPI, Hex, Crates, vcpkg, or Zig. De
 
 **Package spec.** `<registry>:<name>[@<version>]`. `@<version>` is accepted (same as `pkg vulns`); defaults to latest. Tag-style inputs such as `@v4.18.0` are rejected client-side with `INVALID_ARGUMENT` — callers must use the canonical version. Only `npm`, `pypi`, `hex`, `crates`, `vcpkg`, and `zig` are supported; other registries are rejected client-side with `pkg deps only supports npm, pypi, hex, crates, vcpkg, and zig. Got: ${registry}.`
 
-**Two views.** The default runtime view collapses to a single-column list from `dependencies.direct` — the flat answer to "what does this pull in?". The structured groups view (`--groups`, or implicitly via `--lifecycle`) iterates `dependencyGroups.groups` and preserves registry-specific condition metadata (PyPI extras, Crates features, NuGet TFMs). Dev / peer / build / optional deps live only in the groups view — the wire's `direct[]` is always runtime-only.
+**Two views.** The default runtime view collapses to a single-column list from `dependencies.direct` — the flat answer to "what does this pull in?". The structured groups view (`--groups`, or implicitly via `--lifecycle`) iterates `dependencyGroups.groups` and preserves registry-specific condition metadata (PyPI extras, Crates features). Dev / peer / build / optional deps live only in the groups view — the wire's `direct[]` is always runtime-only.
 
 **Lifecycle filter.** `-l, --lifecycle <phases>` accepts a comma-separated list of canonical lowercase tokens (`runtime`, `development`, `build`, `peer`, `optional`). Uppercase and whitespace are tolerated. Filters server-side via the backend's `lifecycle: [String!]` input, which only affects `dependencyGroups`; `direct[]` and `transitive[]` are returned regardless. Unknown tokens are rejected with `INVALID_ARGUMENT` and the canonical list.
 
@@ -176,7 +176,7 @@ Analyses dependencies for a package on npm, PyPI, Hex, Crates, vcpkg, or Zig. De
 
 **Verbose (`--verbose`).** In both plain and transitive modes, each dep expands to a multi-line block: the first line is `name@version`, followed by indented `- <constraint> required by <importer>@<importer-version>, …` bullets. Importers that share a constraint are collapsed onto one bullet with a comma-separated list. In plain mode each direct dep has exactly one importer (the root package itself); in transitive mode a popular leaf may list many importers grouped by constraint. Conflicts expand into a `Conflicts (N):` table (`name: range1, range2, …`, one row per package); circular dependencies expand into a `Circular dependencies (N):` list (`a → b → a` arrow chain).
 
-**JSON envelope.** Preprocessed: `runtime.items[].version` surfaces the resolved version alongside the constraint. Under `--transitive`, `transitive.packages[]` carries `{name, version, importers[]}` records so agents get the same provenance signal as the verbose terminal output without decoding the raw DAG. `transitive.conflicts[]` and `transitive.circularDependencies[]` are typed (`{name, requiredVersions}` / `{cycle: string[]}`) when the observed backend shape decodes; raw passthrough otherwise. The raw DAG itself is deliberately **not** in the envelope — a future dedicated `pkg deps-dag` command will expose it under a typed contract for graph visualisation (mermaid / DOT / interactive viewer).
+**JSON envelope.** Preprocessed: `runtime.items[].version` surfaces the resolved version alongside the constraint. Under `--transitive`, `transitive.packages[]` carries `{name, version}` records by default; `--verbose` opts each entry into an `importers[]` array with importer name / version / constraint (roughly quadruples envelope size on heavy graphs, so it's off by default). `transitive.conflicts[]` and `transitive.circularDependencies[]` are typed (`{name, requiredVersions}` / `{cycle: string[]}`) when the observed backend shape decodes; raw passthrough otherwise. The raw DAG itself is deliberately **not** in the envelope — a future dedicated `pkg deps-dag` command will expose it under a typed contract for graph visualisation (mermaid / DOT / interactive viewer).
 
 **Output envelope.** `{registry, name, version, requestedVersion?, runtime?, groups?, transitive?, filter?}`. Data-first: the `runtime` block emits whenever the backend returned `dependencies.direct` (including `{count: 0, items: []}` for zero-dep packages); the `groups` block emits whenever the backend returned `dependencyGroups` (including `{items: []}` when a lifecycle filter matched nothing, so agents distinguish "backend has no groups concept" from "filter excluded everything"). Each group carries its members under `items` (matches the top-level `runtime.items` naming so dependency lists share one key throughout the envelope). `filter.lifecycles` echoes the canonicalised, deduplicated, display-order-sorted list the backend received — not the raw CSV input.
 

docs/implementation/tools.md

@@ -25,7 +25,7 @@ Both expose the same tools with identical names, parameters, and descriptions. T
 | `search_symbols` | `target`, `query?`, `keywords?`, `match_mode?`, `category?`, `kind?`, `file_path?`, `limit?`, `file_intent?`, `wait_timeout_ms?` | Capability-gated code navigation search over indexed dependency source. |
 | `package_summary` | `registry`, `package_name` | Package overview: latest version, license, description, repository, downloads, GitHub metadata, install command, and known vulnerabilities. Always returns the latest published version. |
 | `package_vulnerabilities` | `registry`, `package_name`, `version?`, `min_severity?`, `include_withdrawn?` | Known vulnerabilities for a package on npm, PyPI, Hex, or Crates. Count summary, per-advisory OSV ID + severity + affected/fix ranges, and upgrade paths. Malware is surfaced in a disjoint bucket. |
-| `package_dependencies` | `registry`, `package_name`, `version?`, `lifecycle?`, `include_transitive?`, `max_depth?` | Direct runtime dependency list plus, when the backend has them, structured groups for dev / peer / build / optional with registry-specific condition metadata (PyPI extras, Crates features). Optional transitive block with aggregate edge counts, conflicts, circular-dependency flags, and an opaque DAG. |
+| `package_dependencies` | `registry`, `package_name`, `version?`, `lifecycle?`, `include_transitive?`, `include_importers?`, `max_depth?` | Direct runtime dependency list (each `{name, version, constraint}` — the backend resolves each constraint to a concrete version) plus, when the backend has them, structured groups for dev / peer / build / optional with registry-specific condition metadata (PyPI extras, Crates features). Optional transitive block with aggregate edge counts, the preprocessed install footprint as `{name, version}`, typed conflicts and circular-dependency cycles; opt into per-package importer provenance with `include_importers`. |
 
 `search_symbols`, `package_summary`, `package_vulnerabilities`, and `package_dependencies` are only registered when the startup token advertises `code_navigation` capability. The backend endpoint can be overridden via `GITHITS_CODE_NAV_URL` for local development. Capability gating keeps the tools hidden from public/default flows while the feature is still rolling out.
 
@@ -79,7 +79,7 @@ Both expose the same tools with identical names, parameters, and descriptions. T
 
 **Version validation.** Same rule as `package_vulnerabilities`: tag-style `v`-prefixed inputs are rejected client-side with `INVALID_ARGUMENT` before the backend call.
 
-**MCP schema notes.** Permissive (`registry: z.string()`, `package_name: z.string()`, …) with validation in-handler via `buildPackageDependenciesParams`. Deliberately no `include_groups` input — with the data-first envelope emitting `groups` unconditionally when the backend returns `dependencyGroups`, the flag would be a silently ignored no-op. `max_depth` has no client-side default on the MCP surface so the backend's full-graph default applies; the CLI's `--depth` defaults to 3 as a human guardrail.
+**MCP schema notes.** Permissive (`registry: z.string()`, `package_name: z.string()`, …) with validation in-handler via `buildPackageDependenciesParams`. Deliberately no `include_groups` input — with the data-first envelope emitting `groups` unconditionally when the backend returns `dependencyGroups`, the flag would be a silently ignored no-op. Neither the MCP surface nor the CLI applies a depth default: `max_depth` / `--depth` is optional and, when omitted, the backend's full-graph traversal is used. `include_importers` requires `include_transitive: true`; `max_depth` and CLI `--depth` require the transitive view — passing them alone is rejected with `INVALID_ARGUMENT` rather than silently ignored.
 
 `package_dependencies` shares its envelope builder with the CLI `githits pkg deps` command via `src/shared/package-dependencies-request.ts` and `src/shared/package-dependencies-response.ts`. The terminal formatter is CLI-only. The parity test (`src/tools/package-dependencies-parity.test.ts`) asserts `toEqual` across every service-sourced success / error fixture (runtime, zero-dep, full-view, optional-lifecycle, multi-lifecycle, filter-matched-nothing, Crates-target-cfg dedup round-trip, transitive, versioned match / diff, NOT_FOUND, VERSION_NOT_FOUND, BACKEND_ERROR) and `toMatchObject` for builder-sourced `INVALID_ARGUMENT` (unsupported registry, tag-style version, unknown lifecycle).
 

src/commands/pkg/deps.test.ts

@@ -183,6 +183,24 @@ describe("pkgDepsAction", () => {
     exitSpy.mockRestore();
   });
 
+  it("rejects --depth without --transitive (avoids silently ignored flag)", async () => {
+    const errorSpy = spyOn(console, "error").mockImplementation(() => {});
+    const exitSpy = spyOn(process, "exit").mockImplementation(() => {
+      throw new Error("process.exit");
+    });
+
+    try {
+      await pkgDepsAction("npm:express", { depth: "3" }, createDeps());
+    } catch {
+      /* expected */
+    }
+
+    const msg = errorSpy.mock.calls[0]?.[0] as string;
+    expect(msg).toContain("--depth requires --transitive");
+    errorSpy.mockRestore();
+    exitSpy.mockRestore();
+  });
+
   it("rejects unsupported registry (nuget) with tool-specific message", async () => {
     const errorSpy = spyOn(console, "error").mockImplementation(() => {});
     const exitSpy = spyOn(process, "exit").mockImplementation(() => {

src/commands/pkg/deps.ts

@@ -58,6 +58,11 @@ export async function pkgDepsAction(
     const parsed = parsePackageSpec(spec);
 
     const userDepth = resolveDepth(options);
+    if (userDepth !== undefined && !options.transitive) {
+      throw new InvalidPackageSpecError(
+        "--depth requires --transitive. Omit --depth, or add --transitive to cap the transitive traversal.",
+      );
+    }
     // Always fetch the transitive DAG on the wire — even in plain
     // mode we need it to resolve the concrete version for each
     // direct dep (`name@version` in display), and for `--verbose`

src/shared/package-dependencies-response.ts

@@ -1,32 +1,47 @@
 /**
  * Hand-crafted response envelope for the `package_dependencies` tool.
  * Shared by CLI `--json` output and MCP `content[0].text`. The terminal
- * formatter is CLI-only.
+ * formatter is CLI-only; it reads from the same envelope shape agents
+ * consume so the two surfaces can never drift.
  *
- * Key design commitments (locked in the plan):
+ * Key design commitments:
  *
  * - **Data-first envelope.** Whenever the backend returned
- *   `dependencies.direct`, we emit a `runtime` block with the flat
- *   list and a client-computed count. Whenever the backend returned
- *   `dependencyGroups`, we emit a `groups` block with every returned
- *   group verbatim. Agents don't branch on flags; they branch on
- *   what's in the envelope. Lifecycle filtering is server-side and
- *   visible via the optional `filter` metadata block.
+ *   `dependencies.direct`, we emit a `runtime` block with
+ *   `{count, items: [{name, version?, constraint?}]}`. Whenever the
+ *   backend returned `dependencyGroups`, we emit a `groups` block
+ *   with every returned group. Agents branch on what's in the
+ *   envelope, not on caller flags.
+ * - **Preprocessed transitive.** When the caller sets
+ *   `includeTransitive`, the envelope's `transitive.packages[]` lists
+ *   every unique install with its resolved version. Adding
+ *   `includeImporters` populates per-package `importers[]` with the
+ *   upstream node name, its own version, and the constraint it
+ *   declared — the same signal the terminal `--verbose` view
+ *   renders, but derived client-side so agents don't have to decode
+ *   the backend's `GenericJSON` DAG themselves.
+ * - **Typed conflicts / cycles with raw fallback.** `transitive.conflicts`
+ *   and `transitive.circularDependencies` ship as typed arrays
+ *   (`{name, requiredVersions}` / `{cycle: string[]}`) when every
+ *   entry decodes against the observed backend shape. If any entry
+ *   fails we fall back to raw `GenericJSON[]` passthrough for that
+ *   field so no data is silently lost — agents discriminate by
+ *   checking for the typed fields on the first element.
  * - **Null vs empty matters.** `dependencyGroups: null` → omit
  *   `groups` entirely ("backend has no groups concept"). Non-null
  *   with zero members after filtering → `groups: { items: [] }`
- *   ("filter matched nothing"). Both map to different envelope
- *   shapes so agents can tell them apart.
+ *   ("filter matched nothing").
+ * - **No raw DAG, no `uniqueDependencies`.** The backend DAG is
+ *   deliberately not exposed from this tool's envelope — a future
+ *   `pkg deps-dag` command will surface it under a typed contract
+ *   for graph visualisation. `uniqueDependencies` is subsumed by
+ *   `packages[]`. `groups.environmentConstraints` remains raw
+ *   `GenericJSON[]` pending a live observation to type it against.
  * - **No v-prefix normalisation.** Inherited from P2; tag-style
  *   inputs are rejected in the request builder before we get here.
  * - **Terminal-only dedup.** JSON preserves every tuple the backend
  *   sent (including Crates target-cfg duplicates). Terminal
  *   rendering strips duplicates inside each group for scannability.
- * - **`transitive.dag` is opaque passthrough.** Backend declares it
- *   `GenericJSON`; we neither parse nor render it. Agents that want
- *   structured DAG analysis read `transitive.dag` from JSON. Same
- *   rule applies to `conflicts`, `circularDependencies`, and
- *   `environmentConstraints`.
  */
 
 import type {
@@ -932,69 +947,6 @@ function isTypedCycleArray(
   return Array.isArray(obj.cycle);
 }
 
-/**
- * Best-effort decoder for a `transitive.conflicts[]` entry. Backend
- * ships these as `GenericJSON`; observed shape on npm:jest is:
- *
- *   {
- *     package_name: string,
- *     required_versions: string[],                // deduped constraint ranges
- *     conflicting_edges: [{ data: { version_constraint, dependency_type },
- *                           from: "npm", to: "npm" }, ...]
- *   }
- *
- * Note `from`/`to` are registry strings, not importer node IDs — so
- * per-range provenance is lost. That's a backend gap; see
- * `/tmp/githits-cli-pkg-intel-backend-gaps.md` item #9 for follow-up.
- */
-interface DecodedConflict {
-  name: string;
-  ranges: string[];
-}
-
-function decodeConflictEntry(raw: unknown): DecodedConflict | null {
-  if (!raw || typeof raw !== "object") return null;
-  const obj = raw as Record<string, unknown>;
-  const name =
-    typeof obj.package_name === "string"
-      ? obj.package_name
-      : typeof obj.packageName === "string"
-        ? obj.packageName
-        : null;
-  if (!name) return null;
-  const rangesRaw = obj.required_versions ?? obj.requiredVersions;
-  if (!Array.isArray(rangesRaw)) return null;
-  const ranges: string[] = [];
-  for (const r of rangesRaw) {
-    if (typeof r === "string" && r.length > 0 && !ranges.includes(r)) {
-      ranges.push(r);
-    }
-  }
-  if (ranges.length === 0) return null;
-  ranges.sort();
-  return { name, ranges };
-}
-
-/**
- * Best-effort decoder for a `transitive.circularDependencies[]` entry.
- * No live observation yet; designed to handle plausible shapes:
- *
- *   { cycle: string[] }            — array of package names along the loop
- *   { packages: string[] }         — alias
- *   string[]                       — a bare array
- */
-function decodeCycleEntry(raw: unknown): string[] | null {
-  if (Array.isArray(raw) && raw.every((x) => typeof x === "string")) {
-    return raw as string[];
-  }
-  if (!raw || typeof raw !== "object") return null;
-  const obj = raw as Record<string, unknown>;
-  const source = obj.cycle ?? obj.packages ?? obj.path;
-  if (!Array.isArray(source)) return null;
-  const names = source.filter((x): x is string => typeof x === "string");
-  return names.length > 0 ? names : null;
-}
-
 // --------------------------------------------------------------------
 // Groups block (separate; shown when --groups or --lifecycle)
 // --------------------------------------------------------------------
@@ -1298,96 +1250,3 @@ function decodeEdges(raw: unknown): DagEdge[] | null {
   }
   return out;
 }
-
-interface ProvenanceEntry {
-  name: string;
-  constraint?: string;
-  /**
-   * The importer's own resolved version (e.g. `express@5.2.1` →
-   * `"5.2.1"`). Populated when {@link buildProvenanceLookup} was
-   * called with `includeImporterVersion = true` and the DAG node
-   * for the importer carried a version. Optional because older
-   * DAG shapes may lack version metadata.
-   */
-  importerVersion?: string;
-}
-
-/**
- * Build a lookup `key → importers[]` where `key` matches the strings
- * in `transitive.uniqueDependencies`. Observed backend strings are
- * `name@version`; we index by both `name@version` and bare `name` so
- * either form works.
- *
- * When `includeImporterVersion` is true, each entry carries the
- * importer's own resolved version — used by the multi-line verbose
- * renderer to display `- <constraint> required by <importer>@<version>`.
- */
-function buildProvenanceLookup(
-  dag: DecodedDag,
-  includeImporterVersion = false,
-): Map<string, ProvenanceEntry[]> {
-  const nodes = dag.nodes;
-  const incoming = new Map<number, DagEdge[]>();
-  for (const edge of dag.edges) {
-    const list = incoming.get(edge.toIdx);
-    if (list) {
-      list.push(edge);
-    } else {
-      incoming.set(edge.toIdx, [edge]);
-    }
-  }
-
-  const lookup = new Map<string, ProvenanceEntry[]>();
-  for (let i = 0; i < nodes.length; i++) {
-    const node = nodes[i];
-    if (!node) continue;
-    const importers = incoming.get(i) ?? [];
-    const entries: ProvenanceEntry[] = [];
-    const seen = new Set<string>();
-    for (const edge of importers) {
-      const from = nodes[edge.fromIdx];
-      if (!from) continue;
-      const key = `${from.name}\u0000${from.version ?? ""}\u0000${edge.constraint ?? ""}`;
-      if (seen.has(key)) continue;
-      seen.add(key);
-      const entry: ProvenanceEntry = {
-        name: from.name,
-        constraint: edge.constraint,
-      };
-      if (includeImporterVersion && from.version) {
-        entry.importerVersion = from.version;
-      }
-      entries.push(entry);
-    }
-    entries.sort((a, b) => {
-      if (a.name !== b.name) return a.name < b.name ? -1 : 1;
-      const av = a.importerVersion ?? "";
-      const bv = b.importerVersion ?? "";
-      return av < bv ? -1 : av > bv ? 1 : 0;
-    });
-
-    // Index by both `name@version` and bare `name`.
-    if (node.version) {
-      lookup.set(`${node.name}@${node.version}`, entries);
-    }
-    const existingBare = lookup.get(node.name);
-    if (existingBare) {
-      // Multiple versions of the same name — merge importers.
-      for (const e of entries) {
-        const key = `${e.name}\u0000${e.importerVersion ?? ""}\u0000${e.constraint ?? ""}`;
-        if (
-          !existingBare.some(
-            (x) =>
-              `${x.name}\u0000${x.importerVersion ?? ""}\u0000${x.constraint ?? ""}` ===
-              key,
-          )
-        ) {
-          existingBare.push(e);
-        }
-      }
-    } else {
-      lookup.set(node.name, [...entries]);
-    }
-  }
-  return lookup;
-}