GitNexus/gitnexus/test/integration/cli-e2e.test.ts
Gergő Magyar a7b3fa1b81
feat(csharp): migrate C# to registry-primary scope-resolution (Closes #934) (#1019)
* feat(csharp-scope): unit 1 — scope query + captures orchestrator

First slice of the C# scope-resolution migration (issue #934, RFC #909
Ring 3). Closes `Unit 1` of
docs/plans/2026-04-21-004-feat-csharp-scope-resolution-plan.md.

Adds:
- src/core/ingestion/languages/csharp/query.ts — tree-sitter scope
  query covering compilation_unit, namespace (block + file-scoped),
  class-like (class/interface/struct/record/enum), method-like
  (method/constructor/destructor/local_function/operator), property
  and field declarations, using directives, type bindings (parameter
  annotations, local variable annotations, constructor inference,
  invocation alias), and references (free call, member call including
  null-conditional, constructor call, member write).
- src/core/ingestion/languages/csharp/captures.ts — pass-through
  orchestrator mirroring python/captures.ts. Import decomposition
  (Unit 2), receiver-type-binding synthesis (Unit 3), and arity
  metadata synthesis (Unit 5) stub out for future units.
- src/core/ingestion/languages/csharp/cache-stats.ts — PROF
  instrumentation mirror of python/cache-stats.ts.

Design notes:
- Return-type / field-type / property-type captures deferred.
  tree-sitter-c-sharp does not expose these under a clean named field
  that pattern-matches. When Unit 7 parity gate surfaces a gap, add
  positional patterns or a post-hoc extractor lookup.
- object_creation_expression with qualified_name type — the qualified
  name itself is the reference text; captured as a whole via a
  dedicated tag so interpretation in later units can split namespace
  + name.
- Null-conditional calls use positional descendant patterns because
  tree-sitter-c-sharp's member_binding_expression and
  conditional_access_expression don't expose named fields.

Coverage:
- 23/23 new unit tests in
  test/unit/scope-resolution/csharp/csharp-captures.test.ts cover
  every capture tag. Confirmed against tree-sitter-c-sharp via the
  probe-script loop during development; grammar drift would surface
  as a capture-shape assertion failure.
- tsc --noEmit clean.

No changes to shared infrastructure. Resolver wiring + registration
land in Unit 6.

* fix(csharp-scope): capture null-conditional receiver + operator decls

Adversarial review surfaced two Unit 1 bugs that would silently
corrupt the graph once C# is flipped on the scope-resolution path:

- `obj?.Save()` only emitted @reference.name, so receiver-bound
  resolution downgraded to the free-call fallback and could mis-link
  to an imported `Save`. Capture the conditional_access_expression
  receiver under @reference.receiver.
- `operator_declaration` had @scope.function but no @declaration.method
  owner, so calls inside operator bodies were attributed to the
  enclosing class and the operator itself disappeared from method
  lookup. Capture the operator token as @declaration.name (downstream
  csharpMethodConfig normalizes to op_Addition etc.).
- `conversion_operator_declaration` was missing from both scope and
  declaration sets. Added with the target type as the name anchor.

Arity metadata for overload resolution remains deferred to Unit 5 and
gated behind Unit 7's parity flip, as documented in captures.ts.

* chore(scope-resolution): drop unused python/scopes.scm sibling

The file was documentation-only — the authoritative scope query is
the embedded `PYTHON_SCOPE_QUERY` constant in `python/query.ts`.
Nothing loaded the `.scm` at runtime, so it drifted from the code.
Remove it and update the four doc comments that pointed at it:

- language-provider.ts: "scopes.scm query" → "scope query (embedded
  in each language's query.ts)".
- languages/python.ts: capture-vocabulary pointer → query.ts.
- python/query.ts header: drop the "edit both together" note.
- python/receiver-binding.ts: "keeps the .scm declarative" → "keeps
  the embedded scope query declarative".
- scope/walkers.ts: "Python's scopes.scm" → "Python's scope query".

Historical plan docs under docs/plans/ still reference scopes.scm but
are frozen artifacts, not living documentation. C# never had a .scm
sibling, so no action needed there.

* feat(csharp-scope): Unit 2 — import interpret + target resolver

Adds the three files Unit 2 of the C# scope-resolution plan calls for:

- `import-decomposer.ts` — inspects each `using_directive` node and
  synthesizes `@import.kind/source/name/alias` markers. Kinds:
    `namespace`  — `using X;` / `using X.Y.Z;`
    `alias`      — `using Alias = X.Y.Z;`         (generics stripped)
    `static`     — `using static X.Y;`
  `global using` maps to namespace (plan's deferred decision); the
  `global::` qualifier is stripped before emitting.
- `interpret.ts` — reads the markers and builds `ParsedImport`. Static
  using maps to `kind: 'wildcard'` since it brings members into
  unqualified scope; Unit 4's merge-bindings tiers wildcards lowest.
  Also provides `interpretCsharpTypeBinding` with nullable/single-arg
  generic/qualifier stripping so receiver-typed resolution sees the
  concrete class name.
- `import-target.ts` — suffix-match adapter returning a single primary
  file. Cross-file partial-class aggregation runs later at graph-bridge
  time (Unit 6). The csproj-based `resolveCSharpImportInternal` stays
  on the legacy path until Unit 7's parity gate surfaces a gap.
- `captures.ts` routes `@import.statement` matches through the
  decomposer so the interpreter sees the markers it needs.

Tests cover every using flavor + resolution edge cases. 38/38 scope-
resolution C# unit tests pass; tsc clean.

* feat(csharp-scope): Unit 3 — simple hooks (binding/import/receiver)

Adds simple-hooks.ts mirroring Python's pattern:

- `csharpBindingScopeFor` — delegates to innermost (block scope is
  already captured by @scope.block in the query).
- `csharpImportOwningScope` — binds `using` inside a namespace to that
  namespace's scope so imports don't leak into sibling namespaces.
  File-level using delegates to module. Function-body using (not legal
  C# but possible from malformed input) attaches to the function.
- `csharpReceiverBinding` — looks up `this` / `base` in the function
  scope's type bindings; returns null for statics, free functions, and
  non-Function scopes. `this` / `base` synthesis itself is deferred to
  a follow-up (matches Python's receiver-binding.ts pattern).

9 new tests pin delegation semantics. 47/47 C# scope-resolution unit
tests pass; tsc clean.

* feat(csharp-scope): Unit 4 — mergeBindings (using precedence)

Three-tier shadowing, same shape as Python's LEGB merge:
  0: local      — class members, locals, parameters
  1: using      — namespace / named / reexport (equal tier; compiler
                  requires explicit qualifier if two using collide)
  2: wildcard   — `using static X.Y;` static-member imports

Within the surviving tier, de-dup by DefId (last-write-wins) so a
re-declared `using` cleanly replaces its earlier binding. Explicit
interface implementations bind under their qualified name in the
extractor layer, so they don't collide with plain simple names here.

7 new tests pin precedence + dedup semantics. 54/54 C# scope-resolution
unit tests pass.

* feat(csharp-scope): Unit 5 — arity metadata synthesis + compatibility

Adversarial review flagged overload narrowing as a blocker for the Unit
7 flip. This lands the declaration-side metadata; callsite-side arity
synthesis is a separate gap we'll address if the parity gate surfaces
overload misresolution.

- `arity-metadata.ts` — reads `csharpMethodConfig.extractParameters`
  and produces `{ parameterCount, requiredParameterCount,
  parameterTypes }`. `params` variadic collapses parameterCount to
  undefined (matches Python's `*args` treatment) and appends a literal
  `'params'` marker to parameterTypes so the compatibility hook can
  detect it without re-reading the AST. Default-valued parameters
  contribute to optionalCount → requiredParameterCount = total − optional.
- `arity.ts` — `csharpArityCompatibility(def, callsite)` returns
  compatible / incompatible / unknown. Mirrors Python's three-verdict
  shape so the central registry's arity filter works without adapter
  logic per-verdict.
- `captures.ts` — on every @declaration.method / @declaration.constructor
  / @declaration.function match, synthesize
  @declaration.parameter-count, @declaration.required-parameter-count,
  and @declaration.parameter-types captures. Covers method_declaration,
  constructor_declaration, destructor_declaration, operator_declaration,
  conversion_operator_declaration, and local_function_statement.

12 new tests: 5 on captures-side synthesis (method + params + types +
variadic + constructor + local function), 7 on the compatibility hook.
66/66 C# scope-resolution unit tests pass; tsc clean.

* feat(csharp-scope): Unit 6 — wire csharpScopeResolver + register

Creates the public barrel (index.ts) and ScopeResolver (scope-resolver.ts)
and plumbs them into the provider + registry:

- `languages/csharp/index.ts` — re-exports the hook entry points and
  documents the 8 known limitations of the registry-primary path
  (csproj-driven namespace resolution, multi-file namespace expansion,
  type-based overload resolution, nested generics, dynamic, preprocessor
  branches, cross-file global using, expression-bodied members).
- `languages/csharp/scope-resolver.ts` — ScopeResolver shape mirroring
  Python's. `isSuperReceiver` matches the literal `base` keyword.
  `fieldFallbackOnMethodLookup: false` since C# is statically typed
  — the type-binding layer already produces precise owner types;
  `propagatesReturnTypesAcrossImports: true` since signatures are
  authoritative.
- `languages/csharp.ts` — adds the 9 hook entry points to the provider
  (emitScopeCaptures, interpretImport, interpretTypeBinding, four
  simple hooks, mergeBindings, arityCompatibility, resolveImportTarget).
- `scope-resolution/pipeline/registry.ts` — registers csharpScopeResolver
  alongside the Python entry.

MIGRATED_LANGUAGES stays at {Python} — the resolver sits idle until
Unit 7's parity gate confirms ≥99% fixture parity. 368/368
scope-resolution unit tests pass; tsc clean.

* feat(csharp-scope): parity Unit 1 — this/base receiver-binding synthesis

Closes 3 parity failures (51 → 48). Target bucket: Category C from the
parity plan.

Changes:
- `languages/csharp/receiver-binding.ts` (new): walks up from a
  function node to the enclosing class/struct/record/interface,
  synthesizes `@type-binding.self` captures with boundName `'this'`
  (and `'base'` when the enclosing type is a class/record with an
  explicit base_list entry). Skips static methods and interface /
  struct `base` cases. Anchors to the method's `body` block so the
  scope-extractor's positionIndex places the binding inside the
  function scope (not the enclosing class scope).
- `languages/csharp/captures.ts`: route `@scope.function` matches
  through the synth, emitting the receiver captures as separate
  matches.
- `languages/csharp/interpret.ts`: map `@type-binding.self` to
  `source: 'self'` (parity with Python).
- `languages/csharp/query.ts`: explicit patterns for `this.X()`,
  `base.X()`, and `this.X = ...` / `base.X = ...` assignment writes.
  `this` and `base` are anonymous tokens in tree-sitter-c-sharp so
  the existing `expression: (_)` pattern (named-only) didn't match.

Tests:
- 8 new unit tests for receiver-binding synthesis edge cases
  (class/struct/record/interface, static, nested, constructor,
  local function inside method).
- Parity: 48 failed | 127 passed (175) under REGISTRY_PRIMARY_CSHARP=1;
  legacy path 175/175 green.

* feat(csharp-scope): parity Unit 2a — foreach + pattern + field captures

Closes 11 parity failures (48 → 37). Partial Unit 2 progress.

Adds type-binding captures for every shape the parity suite exercises
whose resolution path is in-file:

- Typed foreach `foreach (User u in xs)` — @type-binding.annotation
  with bindingName `u` and type `User`.
- Var foreach `foreach (var u in xs)` — @type-binding.alias so the
  generic-stripper unwraps `List<User>` / `Dictionary<K,V>.Values` to
  the element type at chain-follow time. Matches Python's for-loop
  alias pattern.
- `is` pattern `if (obj is User u)` — @type-binding.annotation with
  scope narrowing simplified to function scope (matches Python's
  match-case treatment since we don't emit @scope.block).
- `switch_section > declaration_pattern` (`case User u:`) — no
  case_pattern_switch_label wrapper in tree-sitter-c-sharp.
- `recursive_pattern` (`is User { Age: 1 } u` / `case User { ... } u:`)
  — named binding via type+name fields on the pattern node.
- Field declaration `private City _city;` — @type-binding.annotation
  attached to the class scope for `this._city.X` resolution.
- Property declaration `public User Owner { get; set; }` — same.
- Assignment rebind `alias = Factory()` / `alias = new User()` —
  @type-binding.alias / @type-binding.constructor so reassignment
  propagates type info to later receiver-typed resolution.

Closed tests: foreach (3), var foreach Tier 1c (2), is-pattern (1),
switch pattern (2), recursive_pattern (3). Remaining 37 include
tests that need cross-file same-namespace visibility (field chains,
assignment chain, cross-file return-type propagation) — deferred to
Unit 5 where the IMPORTS/cross-file work lives.

74/74 scope-resolution unit tests pass; legacy path 175/175 green.

* feat(csharp-scope): parity Unit 2b — same-namespace cross-file visibility

Closes 3 parity failures (37 → 34). Adds the C#-specific implicit
import that has no syntactic counterpart: every type declared in
`namespace X` is visible to every other file also declaring
`namespace X`, without any `using` directive.

Changes:
- `scope-resolution/contract/scope-resolver.ts` — new optional hook
  `populateNamespaceSiblings(parsedFiles, indexes, { fileContents })`.
  Most languages leave it undefined; Python / TypeScript / Java need
  explicit imports so there's no analogous pass.
- `scope-resolution/pipeline/run.ts` — invoke the hook after
  `buildWorkspaceResolutionIndex` and before
  `propagateImportedReturnTypes` so the return-type pass sees
  cross-file sibling class bindings.
- `languages/csharp/namespace-siblings.ts` (new) — groups top-level
  class-like defs by namespace name (extracted from source via regex
  since `file_scoped_namespace_declaration` scope range covers only
  the declaration line, not the rest of the file). Injects sibling
  classes into each file's Module AND Namespace scope bindings with
  origin='namespace'. Local declarations shadow cross-file siblings
  via mergeBindings tier precedence.
- `languages/csharp/scope-resolver.ts` — wire the hook.

74/74 scope-resolution unit tests pass; legacy path 175/175 green;
34 parity failures remain (was 37) under REGISTRY_PRIMARY_CSHARP=1.

* feat(csharp-scope): parity Unit 2c — alias/await/return-type captures

Closes 7 parity failures (34 → 27). Adds the remaining type-binding
shapes the parity suite exercises:

- `var alias = u;` / `alias = u;` — identifier-to-identifier alias.
  The resolver's chain-follow walks alias → u → u's declared type.
- `var u = svc.GetUser();` — chained method call alias. Anchors on
  the method_access_expression's `name` field; chain-follow picks up
  GetUser's return type.
- `var u = await Factory();` / `await svc.Get();` — await propagation.
  Strips the `await_expression` wrapper; interpret layer's
  `stripGeneric` handles `Task<T>` / `ValueTask<T>` unwrapping.
- `public User GetUser() { ... }` — method return-type annotation
  via `@type-binding.return`. Required for `propagateImportedReturnTypes`
  to see the return type in later cross-file passes. Covers identifier,
  generic_name, qualified_name, and nullable_type return shapes.

74/74 scope-resolution unit tests pass; legacy path 175/175 green;
27 parity failures remain under REGISTRY_PRIMARY_CSHARP=1.

* feat(csharp-scope): parity Unit 3a — cross-namespace `using` binding

Closes 2 parity failures (27 → 25). Extends the namespace-siblings
pass to resolve `using X;` directives against known namespace
buckets: for each `using` that targets a namespace declared
somewhere in the workspace, inject that namespace's classes into
the importer's module scope with origin='namespace'.

This is the scope-resolution analog of legacy's csproj-driven
directory↔namespace mapping. Without it, `new User()` in
`Services/UserService.cs` (namespace MyApp.Services) can't see the
User class in `Models/User.cs` (namespace MyApp.Models) even with
`using MyApp.Models;` — the scope-resolver layer doesn't have
csproj metadata to translate the dotted namespace path into a
directory lookup.

Legacy 175/175 green; 25 parity failures remain.

* feat(csharp-scope): parity Unit 3b — constructor CALLS emission

Closes 3 parity failures (25 → 22). Adds constructor-form CALLS
edge emission + C# 12 primary constructor synthesis.

Changes:
- `scope-resolution/passes/free-call-fallback.ts`: when a site's
  callForm === 'constructor', look up the class def (not a callable)
  and pick its explicit Constructor def via workspaceIndex's
  memberByOwner — or fall back to the Class def itself for implicit
  constructors. Matches legacy behavior (targetLabel === 'Constructor'
  when explicit, 'Class' when implicit).
- `scope-resolution/pipeline/run.ts`: pass workspaceIndex to the
  free-call fallback.
- `languages/csharp/captures.ts`: synthesize @declaration.constructor
  for C# 12 primary constructors — `class User(string name, int age)`
  / `record Person(string First, string Last)`. The parameter_list is
  a named child of the class_declaration / record_declaration (not a
  separate constructor_declaration node). Skip the synthesis when
  the type already has an explicit constructor to avoid duplicates.
  Emits @declaration.parameter-count + required-parameter-count
  alongside.

Legacy 175/175 green; 376/376 scope-resolution unit tests pass;
22 parity failures remain.

* feat(csharp-scope): parity Unit 3c — static call + default-namespace

Closes 2 parity failures (22 → 21).

- `receiver-bound-calls.ts`: add Case 5 for class-as-receiver. When
  `Animal.Classify()` has an identifier receiver that resolves to a
  Class binding (rather than a variable with a typeBinding), look up
  the member on the class's MRO chain. Covers C#-style static calls
  and any type-qualified member access. Python doesn't hit this
  because `ClassName.method()` is syntactically identical to a free
  call there.
- `namespace-siblings.ts`: treat files with no `namespace X;`
  declaration as living in the default (empty-name) bucket, so
  types declared in no-namespace files share cross-file visibility.
  Required for fixtures without explicit namespaces (e.g. the
  method-enrichment fixture's Animal/App/Dog classes).

Legacy 175/175 green; 21 parity failures remain.

* feat(csharp-scope): parity Unit 4 — callsite arity synthesis (infra)

Synthesize @reference.arity on every invocation_expression and
object_creation_expression by counting `argument` named children of
the backing `argument_list`. Wires the capture-to-Callsite pipeline
shared extractor already consumes (`scope-extractor.ts:878`).

No parity-count movement: the remaining arity-adjacent failures
(overload disambiguation, optional-parameter dedup, variadic
resolution) need type-based argument inference or member-call dedup,
both explicitly deferred in the plan's Known Limitations section.
This commit is infrastructure — future work lands on top of it.

Legacy 175/175 green; 21 parity failures remain.

* feat(csharp-scope): parity Unit 5a — IMPORTS edge + static-using mapping

Closes 1 parity failure (21 → 20). Fixes cross-file IMPORTS edge
emission for C#:

- `languages/csharp/interpret.ts`: map `using static X.Y;` to
  `kind: 'namespace'` rather than `'wildcard'`. The File→File
  IMPORTS edge needs a non-wildcard kind to survive finalize's
  Phase 4 (wildcard-expanded edges drop to empty when the provider
  doesn't implement `expandsWildcardTo`). Unqualified static-member
  access is a deferred limitation — covered by the namespace-siblings
  cross-namespace pass for type lookups, and documented under the
  module's Known Limitations.
- `languages/csharp/import-target.ts`: progressive prefix stripping.
  `using CrossFile.Models;` in a repo laid out `Models/User.cs` (no
  `CrossFile/` directory) works because the legacy resolver consults
  csproj; the scope-resolver tries each suffix of the dotted path
  against `.cs` files. Also handles `using static NS.Type;` by
  stripping leading segments until a direct match lands.
- `test/unit/scope-resolution/csharp/csharp-imports.test.ts`: update
  the `using static` test to the new namespace-kind shape.

376/376 scope-resolution unit tests pass; legacy 175/175 green;
20 parity failures remain.

* feat(csharp-scope): parity Unit 5b — return-type module hoist + chain fallback

Closes 1 parity failure (20 → 19) and lays groundwork for Unit 6.
Based on investigation-agent findings, addresses cluster of 7
cross-file + chain tests whose return-type bindings were stuck at
Class scope and invisible to the chain-follow and propagation passes.

Changes:
- `languages/csharp/simple-hooks.ts::csharpBindingScopeFor`: when the
  declaration is a `@type-binding.return`, hoist the binding all the
  way to the Module scope. The central extractor's auto-hoist only
  promotes one level (Function → Class); for C# methods the parent
  is always a Class, so without this override the return binding
  never reaches Module where chain-follow and cross-file
  `propagateImportedReturnTypes` read from.
- `scope-resolution/passes/compound-receiver.ts`: when the
  class-scope typeBindings lookup at `objClass.typeBindings.get(
  methodName)` misses, walk up from the class scope through the
  parent chain (→ Module) for a return-type binding. Preserves the
  existing class-scope fast-path while restoring owner-chain lookup
  for languages that hoist to Module.

Python parity suite stays 204/204 green on both flag paths;
legacy C# 175/175 green; 19 C# parity failures remain.

* feat(csharp-scope): parity Unit 5c — switch-expr + reasons + ACCESSES 1.0

Closes 4 parity failures (19 → 15).

- `languages/csharp/query.ts`: add captures for `switch_expression_arm`
  with `declaration_pattern` and `recursive_pattern`. C# expression-
  switch (`obj switch { User u => ..., Repo { Name: "x" } r => ... }`)
  uses a different AST node from classic `switch_statement`'s
  `switch_section` — needed separate query patterns.
- `scope-resolution/passes/receiver-bound-calls.ts`: replace the
  self-describing `'scope-resolution: *-receiver'` reason strings
  (which fail legacy-parity consumer filters) with the legacy
  convention: `'import-resolved'` when the resolved member lives in
  a different file, `'global'` otherwise. Mirrors
  `free-call-fallback.ts`'s existing reason logic.
- `scope-resolution/passes/receiver-bound-calls.ts`: pass
  `confidence: 1.0` to `tryEmitEdge` for write/read ACCESSES edges,
  matching legacy DAG behavior (default 0.85 was legacy-CALLS).

Python parity 204/204 on both flag paths; legacy C# 175/175;
15 C# parity failures remain.

* feat(csharp-scope): parity Unit 5d — cross-file typeBinding mirror

Closes 3 parity failures (15 → 12).

`languages/csharp/namespace-siblings.ts`: extend the pass to mirror
method return-type bindings from accessible sibling files' Module
scopes into the importer's Module scope. "Accessible" =
same-namespace siblings + `using namespace X;` targets.

Without this mirror, `var u = svc.GetUser()` in App.cs couldn't
chain-follow to User even after Unit 5b's module-scope hoist:
`GetUser → User` lived on User.cs's Module scope, which isn't on
the ancestor chain of App.cs's function scope, and
`propagateImportedReturnTypes` only mirrors across explicit
ImportEdge targets (not same-namespace implicit visibility).

Closes: var-invocation return type, async/await u.Save (ambient
namespace), cross-file return-type propagation (via u.Save /
u.GetName in Program.cs).

Python parity 204/204 on both flag paths; legacy C# 175/175;
12 C# parity failures remain.

* feat(csharp-scope): parity Unit 5e — namespace-prefix bucket matching

Closes 2 parity failures (12 → 10).

`languages/csharp/namespace-siblings.ts`: when matching accessible
namespaces against class buckets, also probe every dotted prefix.
`using static CrossFile.Models.UserFactory;` parses into the
importer's accessible-namespace set as the full type path, but the
matching bucket is keyed on the containing namespace
(`CrossFile.Models`). Walking back through the dotted segments
ensures the static-using importer sees the containing namespace's
sibling files' return-type bindings.

Legacy 175/175 green; 10 C# parity failures remain.

* feat(csharp-scope): parity Unit 6a — class-like owner extension

Closes 1 parity failure (10 → 9). Extends `populateClassOwnedMembers`
to recognize Interface / Struct / Record / Enum / Trait as class-like
owners, not just Class.

The C# scope query collapses interface_declaration / struct_declaration
/ record_declaration / enum_declaration to @scope.class (they share
body-scope semantics), but the declaration-side tags produce defs of
type Interface / Struct / Record / Enum. `populateClassOwnedMembers`
previously only looked for Class-typed defs in class scopes, so
interface members (including C# 8+ default methods) never got
ownerIds — making them invisible to `findOwnedMember` via
`memberByOwner`.

With this fix, `user.Validate()` on a variable typed as `IValidator`
resolves correctly: receiver-bound-calls Case 4 finds IValidator via
findClassBindingInScope (which already accepted Interface), walks the
chain, and findOwnedMember locates Validate now that the interface
default has a proper ownerId.

Legacy C# 175/175 green; Python parity 204/204 on both flag paths;
9 C# parity failures remain.

* feat(csharp-scope): parity Unit 6b — member-call dedup + handled-site fix

Closes 1 parity failure (9 → 8). Adds the missing legacy-parity
behavior: collapse multiple member-call sites from the same caller
to the same target into one CALLS edge.

Changes:
- `scope-resolution/contract/scope-resolver.ts`: new optional
  `collapseMemberCallsByCallerTarget` flag. Default false (preserves
  the per-site invariant); C# sets it true.
- `scope-resolution/graph-bridge/edges.ts`: dedup key drops
  `line:col` when `collapseByCallerTarget` is on AND edgeType is
  `CALLS` (ACCESSES writes keep per-site granularity).
- `scope-resolution/passes/receiver-bound-calls.ts`: plumbs
  `collapse` through every `tryEmitEdge` call, and crucially marks
  `handledSites.add(siteKey)` whenever a resolved def was found —
  not only when the edge was freshly emitted. Otherwise the site
  leaked through to `emitReferencesViaLookup` which re-emitted a
  per-site edge, defeating the collapse.
- `languages/csharp/scope-resolver.ts`: opt in to the collapse.

Python parity 204/204 on both flag paths; legacy C# 175/175 green;
8 C# parity failures remain.

* feat(csharp-scope): parity Unit 6c — Dictionary.Values / .Keys unwrap

Closes 2 parity failures (8 → 6).

Dictionary<K,V>.Values in a foreach binds the element to V; .Keys
binds to K. Without this, `foreach (var user in data.Values)` where
`data: Dictionary<string, User>` couldn't propagate user's type to
User, and `user.Save()` stayed unresolved.

Changes:
- `languages/csharp/interpret.ts`: don't strip the qualifier when
  the final dotted segment is a known collection accessor
  (`Values` / `Keys`). Preserves the dotted form so downstream
  resolvers can unwrap the receiver's generic type based on the
  suffix.
- `scope-resolution/passes/compound-receiver.ts`: new
  `extractDictionaryArgs` helper splits `Dictionary<K, V>` at the
  top-level comma. In the dotted-access walk, detect trailing
  `.Values` / `.Keys` and return V/K via findClassBindingInScope
  instead of the normal class-walk (Dictionary itself isn't a
  local class def).
  - Handles nested cases: `this.data.Values` walks `this.data`
    recursively (resolving `data` as a field on `this`'s class)
    before applying the unwrap.
- `scope-resolution/passes/receiver-bound-calls.ts` Case 3b: when
  the typeRef's trailing segment is an accessor, pass the raw
  dotted path to `resolveCompoundReceiverClass` without appending
  `()` — the extra parens would misroute to the call-expression
  branch.

Python parity 204/204 on both flag paths; legacy C# 175/175 green;
6 C# parity failures remain.

* feat(csharp-scope): parity Unit 6d — using-static member injection

Closes 2 parity failures (6 → 4). `using static X.Y.Z;` now injects
every public static method of class Z into the importer's module
scope, so `Record("hi")` (without `Logger.` qualifier) resolves to
`Logger.Record` as a free call.

`languages/csharp/namespace-siblings.ts`: regex-scan each file's
source for `using static X.Y.Z;` directives. For each, look up the
class Z in the `X.Y` namespace bucket, walk its owning file's
localDefs for method/function members with `ownerId === Z.nodeId`,
and inject them as `origin: 'import'` bindings in the importer's
module-scope finalized bindings map. `findCallableBindingInScope`
then picks them up via its imported-bindings check.

Closes: variadic `Record(params string[])` + heritage arity
narrowing `WriteAudit`.

Python parity 204/204 on both flag paths; legacy C# 175/175 green;
4 C# parity failures remain (interface-dispatch pass + type-based
overload disambiguation).

* feat(csharp-scope): parity Unit 6e — overload disambig + interface dispatch + FLAG FLIP

Closes the final 4 parity failures (4 → 0). C# now runs the
registry-primary scope-resolution path by default — added to
MIGRATED_LANGUAGES.

Changes:
- `scope-resolution/scope/walkers.ts`: was already extended in
  Unit 6a to recognize Interface/Struct/Record/Enum as class-like
  owners (interface default methods get ownerIds).
- `scope-resolution/passes/receiver-bound-calls.ts`: build
  IMPLEMENTS edge index → emit secondary `interface-dispatch`
  CALLS edges to every implementor's same-named member when the
  primary receiver-typed edge targets an Interface method (closes
  heritage CreateUser CALLS-count test).
- `scope-resolution/passes/receiver-bound-calls.ts`: new
  `pickOverload` helper narrows multi-valued
  `membersByOwner.get(owner).get(name)` candidates by arity then
  argument types. Replaces the first-seen `findOwnedMember` lookup
  in Case 4 so receiver-typed overloaded calls pick the right def.
- `scope-resolution/passes/free-call-fallback.ts`: new
  `pickImplicitThisOverload` walks up to the enclosing class scope
  and applies the same arity + argument-type narrowing for free
  calls inside a class body (`Lookup("alice")` → `Lookup(string)`).
- `scope-resolution/workspace-index.ts`: new `membersByOwner`
  multi-valued index (`Map<owner, Map<name, Def[]>>`) preserves
  every overload alongside the existing first-seen `memberByOwner`.
- `scope-resolution/graph-bridge/node-lookup.ts` +
  `scope-resolution/graph-bridge/ids.ts`: include parameter-types
  suffix in the qualified lookup key for Method nodes. Legacy
  parse-phase encodes the type tag into the node id (`Method:f.cs:
  UserService.Lookup#1~int`); without this two same-arity overloads
  collapsed to one lookup entry and routed to the wrong graph node.
- `scope-resolution/contract/scope-resolver.ts`: new
  `collapseMemberCallsByCallerTarget` opt-in flag (was added in
  Unit 6b for member-call dedup; documented here).
- `gitnexus-shared/src/scope-resolution/reference-site.ts`: new
  `argumentTypes` field carrying inferred per-arg types.
- `scope-extractor.ts`: read @reference.parameter-types capture into
  `site.argumentTypes` and add it + the declaration-arity tags to
  KNOWN_SUB_TAGS so the anchor-detection picks the right anchor.
- `languages/csharp/captures.ts`: synthesize @reference.parameter-types
  by inferring arg types from literal AST nodes (integer_literal →
  'int', string_literal → 'string', constructor_expression →
  type-name, etc).
- `languages/csharp/scope-resolver.ts`: opt in to
  `collapseMemberCallsByCallerTarget`.
- `registry-primary-flag.ts`: **add CSharp to MIGRATED_LANGUAGES**.

Final state:
- C# parity: 175/175 green on flag-on AND flag-off.
- Python parity: 204/204 green on both flag paths (no regression).
- TypeScript clean.

51 → 0 failures across 18 commits on `feat/csharp-scope-resolution`.

* refactor(scope-resolution): extract language-specific accessor unwrap to provider hook

Optimizer pass: move C# Dictionary-family `.Values`/`.Keys` handling
out of the shared `compound-receiver.ts` (where it had hardcoded
regex + accessor names) into a provider-level
`unwrapCollectionAccessor` hook. The shared pass now takes an
arbitrary language-specific unwrap function; C# supplies its
Dictionary implementation in `languages/csharp/accessor-unwrap.ts`.

Related cleanup in `receiver-bound-calls.ts` Case 3b: replace the
hardcoded `tail === 'Values' || tail === 'Keys'` accessor check with
a try-dotted-walk-first / fall-back-to-call-form strategy. This
removes the last C#-specific branch in the shared pass and makes the
logic generalize cleanly to other languages that use property-style
accessors for collection views (Kotlin `.size`, future languages).

Changes:
- `scope-resolution/contract/scope-resolver.ts`: new optional
  `unwrapCollectionAccessor(receiverType, accessor) => string | undefined`
  hook. Documented as language-specific with examples.
- `scope-resolution/passes/compound-receiver.ts`: delete
  `extractDictionaryArgs`, accept `unwrapCollectionAccessor` via
  options, call it for trailing accessor segments.
- `scope-resolution/passes/receiver-bound-calls.ts`: plumb the hook
  through to `resolveCompoundReceiverClass`, remove the
  C#-hardcoded Case 3b accessor check.
- `languages/csharp/accessor-unwrap.ts` (new): C# Dictionary-family
  regex + element-type extraction.
- `languages/csharp/scope-resolver.ts`: opt in.

Audit outcome: everything else added across the 19 C# migration
commits is either correctly scoped to `languages/csharp/` (query,
captures, namespace-siblings, receiver-binding, interpret, imports)
or correctly generic in shared paths (argumentTypes field,
collapseMemberCallsByCallerTarget flag, overload narrowing via
parameterTypes, interface-dispatch via IMPLEMENTS edges, class-like
owner extension for Interface/Struct/Record/Enum, type-tagged node
IDs, module-scope return-type lookup fallback).

175/175 C# green on both flag paths; 204/204 Python green on both
flag paths; TypeScript clean.

* refactor(scope-resolution): gate module-scope typeBinding walk-up on hook

Add optional `hoistTypeBindingsToModule` to the ScopeResolver contract
and gate the Module-scope walk-up in `resolveCompoundReceiverClass` on
it. Only providers that hoist method return-type bindings to Module
scope (C#) opt in; Python and other providers no longer traverse that
fallback path.

Closes the architectural leak flagged in the production-readiness
review: the walk-up was unconditional and therefore widened Python's
code path despite existing only for C#.

No behavior change for C# (hook=true restores the prior lookup). No
behavior change for Python (hook undefined = walk-up skipped, matching
pre-PR behavior).

Verified:
  - npx tsc --noEmit           clean
  - C# unit suite              74/74 passing
  - C# + Python integration    388/388 passing

* refactor(csharp-scope): remove as-unknown-as double casts in scope-resolver

Tighten three type boundaries that were previously papered over with
`as unknown as` casts:

  * `CsharpResolveContext.allFilePaths`: `Set<string>` → `ReadonlySet<string>`.
    The orchestrator only hands out a read-only view; drop the widening
    cast at the resolver-adapter site.
  * `resolveCsharpImportTarget`: call passes the narrow context directly.
    `WorkspaceIndex` is `unknown` in the shared contract, so the
    `as unknown as WorkspaceIndex` cast was gratuitous — structural
    assignability covers it.
  * `csharpMergeBindings`: drop unused `_scope: Scope` parameter. The
    implementation never read it; the cast chain in `scope-resolver.ts`
    existed only to satisfy an unused slot. LanguageProvider.mergeBindings
    now wraps with a tiny arrow adapter; ScopeResolver.mergeBindings
    passes through directly.

No runtime behavior change. `grep 'as unknown as' csharp/scope-resolver.ts`
returns zero matches.

Verified:
  - npx tsc --noEmit           clean
  - C# unit + integration      462/462 passing (incl. Python integration)

* test(csharp-scope): integration fixtures for Units 6c/6d/6e runtime behavior

Close the integration-coverage gap flagged in the production-readiness
review. Units 6c (collection-accessor unwrap), 6d (using-static member
injection), and 6e (overload disambig + interface dispatch) previously
had only hook-level unit tests; the end-to-end wiring was exercised
only by the parity harness.

Three minimal fixtures + four new it() blocks:

  * csharp-collection-accessor — RenderAll iterates
    Dictionary<string, Widget>.Values and calls .Render(); asserts the
    CALLS edge lands on Widget.Render.
  * csharp-using-static — `using static Helpers.MathUtils;` makes
    Square(int) a free-callable in the consumer; asserts the CALLS
    edge lands on MathUtils.Square.
  * csharp-overload-interface — three assertions:
      1. Run → Log binds to the 2-arg overload only (arity narrowing);
         verified via target Method node's parameterTypes.length === 2.
      2. Run → Greet emits one primary edge to IGreeter.Greet plus two
         reason='interface-dispatch' siblings to En/FrGreeter.Greet.
      3. Interface-dispatch fan-out excludes the primary target.

Verified:
  - csharp integration        189/189 passing

* docs(scope-resolution): de-c#-ify optional-hook doc-comments on contract

Rewrite the doc-comments on four optional hooks so they describe the
behavior and when a provider would enable it, rather than naming C#
as the sole consumer. Hook names were already generic — only the
comments had baked in one-language framing, which risked discouraging
future reuse.

Affected hooks:
  * unwrapCollectionAccessor
  * collapseMemberCallsByCallerTarget
  * populateNamespaceSiblings
  * hoistTypeBindingsToModule

Language-specific rationale stays where it belongs — next to the hook
assignment in `languages/csharp/scope-resolver.ts`. Zero-match grep for
`C#|csharp|CSharp` in the contract file confirms the separation.

No code change.

* docs(csharp-scope): justify regex-based namespace-sibling detection

Record why `namespace-siblings.ts` uses regex over AST walks and
enumerate the known misses so the next reader has ground to stand on:

  * `global using static X.Y;` — no plain `using static` token.
  * Aliased `using static X = Y.Z;` — `=` breaks the pattern.
  * Attributed namespace declarations between `]` and `{`.
  * Multi-namespace files — first-wins attribution.
  * Preprocessor-gated namespace declarations — textual branch only.

Rationale: the pass is file-path-driven and the tree-sitter tree isn't
available at its call site (the orchestrator feeds raw fileContents);
re-parsing to count namespaces would cost more than the regex walk.
Refactor to AST-driven detection is deferred to a separate PR.

Mirrored the known-miss list into `csharp/index.ts`'s limitations
ledger so the operator-visible surface and the in-code justification
stay in sync.

No code change.

* refactor(csharp-scope): AST-driven namespace detection with treeCache reuse

Replace regex-over-source-content with tree-sitter AST walks in
namespace-siblings.ts; thread the orchestrator's treeCache through
the populateNamespaceSiblings hook so the pass reuses the same parse
trees `extractParsedFile` already consumed (single-source-of-truth
for the AST — no double-parse).

Behavior gains (no longer "known misses"):
  * `global using static X.Y;` is now detected.
  * Aliased `using static X = Y.Z;` is now detected.
  * Attributed namespace declarations (`[attr] namespace X`) parse
    correctly because tree-sitter sees them as one node.
  * Preprocessor-gated namespace declarations parse via the grammar.

Contract change (additive, optional):
  * `populateNamespaceSiblings` ctx now carries an optional
    `treeCache?: { get(filePath): unknown }`. Existing providers that
    don't set it on `RunScopeResolutionInput` see undefined, and the
    hook falls back to a fresh parse (current behavior preserved on
    cache miss).

Limitation ledger updated in csharp/index.ts: the AST-based detection
removes 4 of the 5 prior known misses; only "first-wins multi-namespace
file attribution" remains.

Verified:
  - npx tsc --noEmit                         clean
  - C# + Python integration                  393/393 passing

* refactor(python-scope): remove as-unknown-as casts in scope-resolver (mirrors Unit 2)

Replay the C# scope-resolver cleanup on the Python side so both
providers share a single clean pattern:

  * Drop `ws as unknown as WorkspaceIndex` — `WorkspaceIndex` is
    `unknown` in the shared contract, so the narrow context assigns
    structurally without a cast.
  * Drop `{ id: scopeId } as unknown as Scope` — `pythonMergeBindings`
    never read the scope (the parameter was `_scope`), so the stub
    was a type-only ghost. Signature is now `(bindings)` and the
    LanguageProvider slot wraps with an arrow adapter.
  * Drop `allFilePaths as Set<string>` — the orchestrator hands a
    `ReadonlySet<string>`; we copy it into a `Set` at the resolver
    adapter so the legacy downstream `resolvePythonImportInternal`
    chain (typed for mutable `Set<string>`) keeps working. The copy
    is O(N) once per import, trivial cost.

Left intact on purpose: the `(callsite, def) → (def, callsite)`
arrow wrapper on `arityCompatibility`. That's a documented shape
difference between `LanguageProvider.arityCompatibility(def, callsite)`
and `ScopeResolver.arityCompatibility(callsite, def)`; both providers
(Python + C#) carry the same wrapper. Reconciling is a separate
refactor across both contracts.

No runtime behavior change.

Verified:
  - npx tsc --noEmit                              clean
  - Python + C# unit + integration suites         529/529 passing

* docs(scope-resolution): document I1-I8 invariants, source-of-truth, and same-graph guarantee

Promote contract knowledge that was implicit in code into the canonical docs
so future migrations and the next reviewer don't have to reverse-engineer it.

contract/scope-resolver.ts:
  * Migration cookbook lists every optional hook (was: only the two
    booleans), with one-line guidance per hook including when to enable
    `hoistTypeBindingsToModule`.
  * Contract Invariants I1-I7 are now spelled out in full (was: only
    I1/I3/I5 summarized with a pointer to a plan file). Added new I8
    "post-finalize hooks may mutate Scope.typeBindings and indexes.bindings;
    consumers must not freeze or snapshot before all post-finalize hooks
    have run".
  * New "Semantic-model source of truth" section: ParsedFile is the
    single semantic model; passes that need AST-level facts must reuse
    the orchestrator's treeCache rather than re-parse.
  * New "Same-graph guarantee" section: legacy DAG and scope-resolution
    emit indistinguishable edges (node identity, edge vocabulary,
    confidence). CI parity workflow enforces this.

gitnexus-shared/src/scope-resolution/parsed-file.ts:
  * Added "Source-of-truth invariant" pointer paragraph.

ARCHITECTURE.md (Coexistence section):
  * Updated migrated-language list (Python + C#).
  * Added "Same-graph guarantee" subsection.
  * Added "Semantic-model source of truth" subsection.
  * Filled in the ScopeResolver hook table with the five optional hooks
    that landed in this branch (unwrapCollectionAccessor,
    collapseMemberCallsByCallerTarget, populateNamespaceSiblings,
    hoistTypeBindingsToModule, fieldFallbackOnMethodLookup).
  * Added C# rows to the code-references table.

Verified:
  - npx tsc --noEmit                                 clean
  - C# + Python integration                          393/393 passing

* refactor(scope-resolution): consume SemanticModel as single authoritative store

Unify scope-resolution and legacy parse into one symbol index per the
industry pattern (Roslyn / tsc / rust-analyzer). Scope-resolution
passes now consume `SemanticModel.methods` / `SemanticModel.fields` /
`SemanticModel.symbols` for all symbol-keyed lookups. The legacy DAG
already read from these; the drift — two parallel owner-keyed indexes
populated by two writers with divergent ownerId semantics — is closed.

Changes:

  * `MethodRegistry.lookupAllByOwner(owner, name)`: new API returning
    every overload without arity narrowing. Powers `findOwnedMember` /
    `pickOverload`.

  * `pipeline/run.ts` reconciliation pass: after
    `provider.populateOwners(parsed)`, iterate `parsed.localDefs[i]`
    and register methods/fields into the SemanticModel under the
    corrected ownerId. Idempotent — skips defs already present under
    `(ownerId, simple)` by nodeId, so unmigrated languages whose
    legacy extractor already set ownerId (C#) don't double-register.
    Closes the Python gap where class-body methods were invisible to
    `MethodRegistry` because the legacy Python method extractor
    couldn't resolve `enclosingClassId` at parse time.

  * `WorkspaceResolutionIndex` slimmed to Scope-valued maps only
    (`classScopeByDefId`, `moduleScopeByFile`). Dropped `memberByOwner`,
    `membersByOwner`, `defsByFileAndName`, `callablesBySimpleName` —
    all symbol-keyed duplicates of SemanticModel indexes.

  * Walker helpers now consume SemanticModel:
      - `findOwnedMember(owner, name, model)` → methods then fields
        fallback (ACCESSES writes target Property/Variable defs too).
      - `findExportedDefByName` fallback walks every Module scope's
        `origin === 'local'` bindings via `index.moduleScopeByFile`
        (preserves the module-export-visibility filter that
        SymbolTable.fileIndex can't cheaply encode).
      - `findExportedDef` reads `moduleScope.bindings` directly.

  * `pickOverload` in receiver-bound-calls.ts falls back to
    `model.fields.lookupFieldByOwner` when method lookup returns empty,
    fixing ACCESSES write edges that receive a Property target.

  * `phase.ts` threads `resolutionContext.model` into
    `RunScopeResolutionInput`.

Boundary rule, enforced by file placement:
  - symbol-indexed lookups (key = nodeId / name / filePath) →
    `SemanticModel`
  - Scope-valued lookups (value = `Scope`) →
    `WorkspaceResolutionIndex`

Research synthesized from web-researcher + Explore + best-practices +
system-architect agents; canonical references: Roslyn Overview,
rust-analyzer architecture, stack-graphs paper.

Verified:
  - npx tsc --noEmit                               clean
  - C# + Python integration                        393/393 passing

* docs(scope-resolution): refresh comments after dropping duplicated indexes

Replace references to the now-deleted `memberByOwner` /
`callablesBySimpleName` index fields with comments that describe the
actual lookup path (`SemanticModel` registries + scope-tied module
bindings). Pure doc cleanup; no behavior change.

* feat(scope-resolution): extract reconciliation pass + add parity validator

Extract the SemanticModel reconciliation pass (previously inline in
`pipeline/run.ts`) into a dedicated module with:

  * `reconcileOwnership(parsedFiles, model)` — pure function returning
    stats (methodsRegistered / fieldsRegistered / skippedAlreadyPresent).
    Idempotent; safe to re-run.
  * `validateOwnershipParity(parsedFiles, model, onWarn)` — dev-mode
    runtime validator for Contract Invariant I9. Walks every def with
    an `ownerId` and asserts it is reachable via
    `model.methods.lookupAllByOwner` or `model.fields.lookupFieldByOwner`.
    Soft-fails via `onWarn`; never throws.

Validator is gated on both `NODE_ENV !== 'production'` and
`VALIDATE_SEMANTIC_MODEL !== '0'` so production incurs zero cost but
development surfaces any drift between `parsed.localDefs` ownership and
the registries.

12 new unit tests cover:
  * happy path: method, property, Variable registration
  * edge case: defs without ownerId are skipped
  * idempotency: second call is a no-op
  * coexistence: defs the legacy extractor already registered (via
    `model.symbols.add`) are skipped on reconcile
  * overloads: multiple methods under the same (owner, name)
  * validator: no warnings after reconciliation
  * validator: warns on drift
  * validator: no-op under NODE_ENV=production
  * validator: no-op when VALIDATE_SEMANTIC_MODEL=0
  * validator: warns on missing Property same as missing Method

Verified:
  - npx tsc --noEmit                               clean
  - reconcile-ownership unit tests                 12/12 passing
  - C# + Python integration                        393/393 passing

* refactor(scope-resolution): narrow handles + tighten required params

Two small hygiene fixes that fell out of the unified-model work:

  * Introduce `readonlyModel: SemanticModel` in `runScopeResolution`
    immediately after reconciliation so the write/read phase boundary
    is explicit at the code level. Downstream passes (receiver-bound,
    free-call) receive the narrowed `SemanticModel` rather than the
    `MutableSemanticModel` that only the reconciliation pass needs.
    The type system now rejects accidental writes in the read phase.

  * Make `emitFreeCallFallback`'s `workspaceIndex` parameter required.
    It's now always passed (every caller threads it through), and the
    `workspaceIndex?` guard was dead code. Also drops the `| undefined`
    branch from `pickConstructorOrClass` which no caller can hit.

No behavior change.

* docs(semantic-model): document unified single-source-of-truth invariant (I9)

Add Contract Invariant I9 to the ScopeResolver contract and write the
single-source-of-truth + write/read phase contract into both the
SemanticModel file-head and ARCHITECTURE.md.

Three landing points so the rule is reachable from every entry:

  * contract/scope-resolver.ts — new I9 entry in the Contract
    Invariants list: scope-resolution passes consult SemanticModel
    exclusively for symbol-keyed lookups; WorkspaceResolutionIndex is
    reserved for Scope-valued maps. Documents the two-phase write
    (legacy parse + reconcileOwnership) and the narrowed-handle read
    posture. Calls out the reconciliation shim as transitional.

  * model/semantic-model.ts — new "Single-source-of-truth invariant"
    and "Write / read phase contract" sections in the file-head.
    Three ordered write phases (parse → reconcile → attachScopeIndexes),
    then frozen for readers.

  * ARCHITECTURE.md § "Semantic-model source of truth" — expanded
    subsection covering both invariants (ParsedFile = AST truth,
    SemanticModel = symbol truth), the write/read phase diagram, and
    the reconciliation-shim rationale.

No code change.

* test(scope-resolution): rewrite workspace-index test for slimmed index

The test file previously asserted on \`defsByFileAndName\`,
\`callablesBySimpleName\`, and \`memberByOwner\` — fields removed when
symbol-keyed lookups moved to \`SemanticModel\`. Rewrite so the same
invariants are asserted via the authoritative consumers:

  * New WorkspaceResolutionIndex shape test (scope-only maps).
  * \`findExportedDef\` module-export visibility tests:
    - keeps top-level class and function defs.
    - excludes class-body Variable defs (MAX_USERS = 100).
    - excludes class methods from module-export lookup.
  * \`findExportedDefByName\` fallback excludes class methods when a
    same-named module function exists.
  * \`findOwnedMember\` via the reconciled SemanticModel finds Python
    class methods after populateOwners + reconcileOwnership.

Total assertions preserved: every invariant from the old test file is
still pinned; the assertion surface shifted from the index shape to
the walker helpers.

Verified:
  - workspace-index.test.ts                        8/8 passing

* fix(tests): update registry-primary-flag test for C# migration

The "returns exactly the flipped languages" case expected `enabled.size === 1`
after toggling Python off and Go on. After the C# migration lands C# in
MIGRATED_LANGUAGES, C# is default-on too — so the size is now 2 (Go + C#)
unless C# is also opted out.

Turn off C# alongside Python in the test setup. Added a comment noting
that future migrations must add their REGISTRY_PRIMARY_<LANG>='false'
line here.

* refactor(scope-resolution): address PR #1019 review findings

Resolves all 5 findings from the automated review on
feat/csharp-scope-resolution. Shared ingestion code stays
language-agnostic; C# (and every class-like language) benefits.

F1 [high] Broaden class-like predicate
  Hoist `isClassLike` in `scope/walkers.ts` to an exported top-level
  helper covering Class | Interface | Struct | Record | Enum | Trait.
  Use it in `findClassBindingInScope`, `findEnclosingClassDef`, and
  `buildWorkspaceResolutionIndex` so C# records, structs, interfaces,
  and enums participate in scope chains and receiver binding the same
  way Python classes do.

F2 [medium] Remove stale comment in csharp simple-hooks
  `csharpReceiverBinding`'s doc claimed this/base synthesis was
  "planned for a follow-up"; synthesis has been implemented in
  receiver-binding.ts since the migration landed. Rewrite the doc to
  describe the actual behavior (non-null TypeRef on instance-method
  bodies, null on static/free functions).

F3 [medium] O(1) reverse lookup for classScopeId -> classDefId
  Add `classScopeIdToDefId: ReadonlyMap<ScopeId, string>` to
  `WorkspaceResolutionIndex`, populated as the inverse of
  `classScopeByDefId`. Replace the O(C) linear scan in
  `pickImplicitThisOverload` (free-call-fallback.ts) with an O(1)
  `Map.get` — turns per-site reverse resolution from linear in class
  count to constant time for every free call.

F4 [low] Extract narrowOverloadCandidates shared utility
  New `passes/overload-narrowing.ts` centralizes the arity + argument-
  type narrowing previously duplicated across `pickOverload`
  (receiver-bound-calls.ts) and `pickImplicitThisOverload`
  (free-call-fallback.ts). Both callsites now share identical
  narrowing semantics; variadic `params T` handling is preserved.
  Return type is `readonly SymbolDefinition[]` with no defensive
  spreads (allocations saved on the hot path).

F5 [low] Merge unreachable Case 5 into Case 2
  `Case 5` in `receiver-bound-calls.ts` was dead code — `Case 2`
  pre-empted it for every static/class-name receiver. Delete Case 5
  and lift its kind-aware read/write ACCESSES reason/confidence logic
  into Case 2 so static-style member access (e.g. `Interface.Member`,
  `TypeName.StaticMember`) gets the correct edge metadata.

Tests
  - New unit tests for `narrowOverloadCandidates` covering empty
    input, arity filtering, variadic params, type narrowing, and
    fallback semantics.
  - New unit tests for `classScopeIdToDefId` verifying inverse
    invariant and empty index behavior.
  - New C# integration fixtures and tests:
      * csharp-record-base — record inheritance + `base.Save()`
      * csharp-struct-overloads — struct with implicit-this overload
        narrowing (pinned exact edge count under registry-primary)
      * csharp-interface-receiver-static — interface-qualified static-
        style call exercises the merged Case 2.
  - Full runs green:
      * scope-resolution unit: 406/406
      * csharp integration (registry-primary): 197/197
      * csharp integration (legacy DAG): 197/197
      * python integration (regression guard): 204/204

Chore
  - Add `.context/` to root `.gitignore` to prevent agent scratch
    files from being committed.

Made-with: Cursor

* test(csharp-scope-resolution): address adversarial review follow-ups on PR #1019

Applies the three actionable follow-ups from the post-commit adversarial
review of 5a1bce7f against DoD.md. No runtime code changes.

- [medium] Strengthen bounds-only assertion in the struct-overloads
  suite: `methods.length` is now pinned to `toBe(2)` and the arity list
  to `toEqual([1, 2])`. Fixture `csharp-struct-overloads/src/Calc.cs`
  declares exactly two `Add` methods, so a regression that adds, drops,
  or merges an overload will now fail the test instead of silently
  passing a `>= 2` gate.

- [low] Pin the merged Case 2 kind-aware branch (receiver-bound-calls.ts
  lines 257-289) with a dedicated fixture and three new assertions:
  `csharp-class-static-field-access/src/Counters.cs` exercises
  `ClassName.Field = value` where the receiver resolves via
  `findClassBindingInScope` (no typeBinding on `Counters`). The test
  verifies (a) two distinct ACCESSES writes are emitted from a single
  method (per-site dedup from graph-bridge/edges.ts:80-87),
  (b) `reason === 'write'`, (c) `confidence === 1.0`, and (d) no
  spurious CALLS edges are produced for the same sites. This is the
  semantic upgrade lifted from the deleted Case 5; without a pinning
  test a future revert of the kind-aware branch would silently drop
  back to `import-resolved`/`global` at 0.85 for the same sites.
  Read-side coverage is intentionally not asserted because the C#
  tree-sitter query currently emits only `write.member` captures
  (languages/csharp/query.ts:485-501) — a read counterpart would have
  no reference site today and would give a false sense of coverage.

- [info] Left the `?? overloads[0]` fallback in place at
  receiver-bound-calls.ts:450 unchanged. With the package's current
  tsconfig (strict: false, no noUncheckedIndexedAccess) both the
  defensive fallback and a `candidates[0]!` assertion type-check
  identically, so the finding has no production-readiness impact.
  Keeping the fallback minimizes churn.

Validation (local, Windows PowerShell):
- `npx prettier --check test/integration/resolvers/csharp.test.ts` -> clean
- `npx tsc --noEmit` -> 0 errors
- `REGISTRY_PRIMARY_CSHARP=1 npx vitest run test/integration/resolvers/csharp.test.ts` -> 200/200
- `REGISTRY_PRIMARY_CSHARP=0 npx vitest run test/integration/resolvers/csharp.test.ts` -> 200/200
- `npx vitest run test/integration/resolvers/python.test.ts` -> 204/204
- `npm test` (full gitnexus suite) -> 6967 passed, 6 pre-existing failures
  (4x Swift overload/dedup, 1x Swift method-extraction unit, 1x Swift
  type-env unit, 1x LadybugDB lockfile on Windows). All six reproduce on
  5a1bce7f with these follow-up changes stashed, confirming they are
  environment/baseline failures unrelated to this work. Swift is not in
  MIGRATED_LANGUAGES so the merged Case 2 path cannot affect it.

Refs: PR #1019
Made-with: Cursor

* refactor(scope-resolution): address full-PR review findings on PR #1019

Resolves the two remaining findings from the code-review-swarm full-PR
sweep (verdict: production-ready with minor follow-ups).

[low] Complete the csharp/index.ts module-layout JSDoc.

`languages/csharp/index.ts` is the discovery surface for the C# scope-
resolution module decomposition (per AGENTS.md). The "Module layout"
list silently omitted three load-bearing modules — `accessor-unwrap.ts`
(`.Values`/`.Keys` receiver-type unwrap), `namespace-siblings.ts`
(AST-driven cross-file implicit-namespace visibility), and
`receiver-binding.ts` (`this`/`base` type-binding synthesis). Extended
the JSDoc list so the "single-concern" decomposition story is honest
and the next contributor can locate the right file without grep.
No behavior change.

[info] Replace the non-standard `'scope-resolution: super-receiver'`
      edge reason with the canonical `'global'` tier.

`passes/receiver-bound-calls.ts` emitted a non-canonical reason string
for the super/base branch, which falls outside the vocabulary declared
in ARCHITECTURE.md § Scope-Resolution Pipeline (`'import-resolved' |
'global' | 'local-call' | 'same-file' | 'interface-dispatch' | 'read'
| 'write'`). Super/base calls resolve through the MRO chain rather
than through import directives, so the correct canonical tier is
`'global'` (same classification the legacy DAG's `toResolveResult`
applies to non-same-file, non-import-scoped resolutions).

Locked the contract with `rel.reason === 'global'` assertions on the
existing `csharp-super-resolution` and `csharp-generic-parent-
resolution` suites, both of which go through the super-branch MRO
path. The `csharp-record-base` suite intentionally does not pin a
reason (records don't currently emit EXTENDS edges, so the MRO lookup
misses and the edge is produced by the reference-index fallback
instead of the super-branch). A code comment flags the pre-existing
Python-legacy asymmetry (Python legacy tier classifier marks
`super()` as `'import-resolved'` because the ancestor arrives via an
`import` statement); closing that gap requires realigning the legacy
tier classifier and is tracked separately.

Validation:
- `npx tsc --noEmit` passes.
- `npx prettier --check` clean on all four touched files.
- `test/integration/resolvers/csharp.test.ts` — 200/200 under both
  `REGISTRY_PRIMARY_CSHARP=0` (legacy DAG) and `REGISTRY_PRIMARY_CSHARP=1`
  (registry-primary), preserving same-graph parity on the super branch.
- `test/integration/resolvers/python.test.ts` — 204/204 under both
  `REGISTRY_PRIMARY_PYTHON=0` and `=1`.
- `test/unit/scope-resolution/` — 406/406 passing.

Unstaged: `gitnexus/package-lock.json` (drift from `npm install` run
to resolve the pre-existing missing `jsonc-parser` dependency — not
part of this change).

Made-with: Cursor

* test(ci): raise integration-test timeouts so slow Windows runners stop flaking

The `windows-latest` CI runner for this branch was consistently failing
two integration suites in ways that had nothing to do with the PR's
scope-resolution changes:

  * `cli-e2e.test.ts` — `analyze command runs pipeline on mini-repo`
    hit the default 30 s vitest test timeout, which raced the test's
    own 30 s subprocess timeout and prevented the existing
    `if (result.status === null) return;` slow-CI tolerance from ever
    firing. That single timeout then cascaded into the downstream
    `cypher`/`query`/`impact` tests (which exited non-zero because the
    mini-repo was never indexed) and the `EPIPE handling` test.
  * `skills-e2e.test.ts` — `beforeAll` hooks run a full
    `runSkillsCli(tmpDir)` subprocess that analyzes a fixture repo and
    generates skills. 50 s was enough on Linux/macOS but not on slow
    Windows CPUs, producing "Hook timed out in 50000ms" errors and
    cascading test failures across every language describe block.

Fix:
  * Bump the `analyze` test's vitest test-level timeout to 60 s so it
    exceeds the 30 s subprocess timeout and the slow-CI tolerance can
    actually activate.
  * Bump all 12 `runSkillsCli`-driven `beforeAll` hooks from 50 s to
    120 s.

No production-code behavior changes. No change to what the tests
assert — only the per-test/hook wall-clock budget.

Made-with: Cursor
2026-04-23 12:38:13 +01:00

1155 lines
47 KiB
TypeScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* P1 Integration Tests: CLI End-to-End
*
* Tests CLI commands via child process spawn:
* - statusCommand: verify stdout for unindexed repo
* - analyzeCommand: verify pipeline runs and creates .gitnexus/ output
*
* Uses process.execPath (never 'node' string), no shell: true.
* Accepts status === null (timeout) as valid on slow CI runners.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { spawnSync, spawn } from 'child_process';
import path from 'path';
import fs from 'fs';
import os from 'os';
import { fileURLToPath, pathToFileURL } from 'url';
import { createRequire } from 'module';
const testDir = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = path.resolve(testDir, '../..');
const cliEntry = path.join(repoRoot, 'src/cli/index.ts');
const FIXTURE_SRC = path.resolve(testDir, '..', 'fixtures', 'mini-repo');
// `MINI_REPO` is a *per-run temp copy* of the fixture, not the shared
// source. Writing into the shared source races with other suites that
// ingest it read-only (pipeline-graph-golden, pipeline.test) — those
// suites copy the source to their own tmp dir but the copy happens at
// `beforeAll`, so if this suite's analyze has already created AGENTS.md
// / CLAUDE.md / .claude/ in the source when the other suite's cpSync
// runs, the pollution is captured before the isolation kicks in.
//
// The deterministic fix: this suite never touches the shared source.
// `beforeAll` copies the fixture to a fresh mkdtemp'd directory whose
// basename is `mini-repo` (so `--repo mini-repo` lookup by basename
// still works), `afterAll` rms the parent tmpdir.
let MINI_REPO: string;
let tmpParent: string;
// Absolute file:// URL to tsx loader — needed when spawning CLI with cwd
// outside the project tree (bare 'tsx' specifier won't resolve there).
// Cannot use require.resolve('tsx/dist/loader.mjs') because the subpath is
// not in tsx's package.json exports; resolve the package root then join.
const _require = createRequire(import.meta.url);
const tsxPkgDir = path.dirname(_require.resolve('tsx/package.json'));
const tsxImportUrl = pathToFileURL(path.join(tsxPkgDir, 'dist', 'loader.mjs')).href;
beforeAll(() => {
// Copy the fixture into an isolated tmpdir named `mini-repo` so that the
// `--repo mini-repo` CLI arg (which matches by basename) still works.
tmpParent = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-cli-e2e-'));
MINI_REPO = path.join(tmpParent, 'mini-repo');
fs.cpSync(FIXTURE_SRC, MINI_REPO, { recursive: true });
// Initialize mini-repo as a git repo so the CLI analyze command
// can run the full pipeline (it requires a .git directory).
spawnSync('git', ['init'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['add', '-A'], { cwd: MINI_REPO, stdio: 'pipe' });
spawnSync('git', ['commit', '-m', 'initial commit'], {
cwd: MINI_REPO,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
});
afterAll(() => {
// Entire tmp copy goes away — no selective cleanup needed. The shared
// `test/fixtures/mini-repo/` source was never touched.
if (tmpParent) {
fs.rmSync(tmpParent, { recursive: true, force: true });
}
});
function runCli(command: string, cwd: string, timeoutMs = 15000) {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, command], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
// Pre-set --max-old-space-size so analyzeCommand's ensureHeap() sees it
// and skips the re-exec. The re-exec drops the tsx loader (--import tsx
// is not in process.argv), causing ERR_UNKNOWN_FILE_EXTENSION on .ts files.
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
});
}
/**
* Like runCli but accepts an arbitrary extra-args array so unhappy-path tests
* can pass flags (e.g. --help) or omit a command entirely.
*/
function runCliRaw(extraArgs: string[], cwd: string, timeoutMs = 15000) {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...extraArgs], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
});
}
/**
* Like runCliRaw but accepts extra env vars. Used by tests that need to
* isolate the global registry via GITNEXUS_HOME so they don't touch the
* developer / CI agent's real ~/.gitnexus/registry.json (#829).
*/
function runCliWithEnv(
extraArgs: string[],
cwd: string,
extraEnv: Record<string, string>,
timeoutMs = 15000,
) {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...extraArgs], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
...extraEnv,
},
});
}
/**
* Create a fresh git-initialised throwaway repo at `<parentTmp>/<basename>`
* and return its path. Used for tests that need multiple repos whose
* basenames intentionally collide (#829 reproduction).
*/
function makeMiniRepoCopy(basename: string, prefix: string): string {
const parent = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
const repo = path.join(parent, basename);
fs.cpSync(FIXTURE_SRC, repo, { recursive: true });
spawnSync('git', ['init'], { cwd: repo, stdio: 'pipe' });
spawnSync('git', ['add', '-A'], { cwd: repo, stdio: 'pipe' });
spawnSync('git', ['commit', '-m', 'initial commit'], {
cwd: repo,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
return repo;
}
describe('CLI end-to-end', () => {
it('status command exits cleanly', () => {
const result = runCli('status', MINI_REPO);
// Accept timeout as valid on slow CI
if (result.status === null) return;
expect(result.status).toBe(0);
const combined = result.stdout + result.stderr;
// mini-repo may or may not be indexed depending on prior test runs
expect(combined).toMatch(/Repository|not indexed/i);
});
// The vitest test-level timeout (60 s) must exceed the subprocess
// timeout (30 s) so the "Accept timeout as valid on slow CI"
// branch can actually fire on slow runners (Windows CI routinely
// comes in at ~2x macOS wall-clock). Without a larger test-level
// timeout, the default 30 s vitest timeout races the 30 s
// subprocess timeout and the `if (result.status === null) return;`
// tolerance never activates.
it('analyze command runs pipeline on mini-repo', () => {
const result = runCli('analyze', MINI_REPO, 30000);
// Accept timeout as valid on slow CI
if (result.status === null) return;
expect(
result.status,
[
`analyze exited with code ${result.status}`,
`stdout: ${result.stdout}`,
`stderr: ${result.stderr}`,
].join('\n'),
).toBe(0);
// Successful analyze should create .gitnexus/ output directory
const gitnexusDir = path.join(MINI_REPO, '.gitnexus');
expect(fs.existsSync(gitnexusDir)).toBe(true);
expect(fs.statSync(gitnexusDir).isDirectory()).toBe(true);
}, 60_000);
// ─── analyze --name <alias> + --allow-duplicate-name (#829) ──────
//
// End-to-end regression guard for the name-collision feature:
// 1. `analyze --name X` persists the alias to ~/.gitnexus/registry.json
// 2. A second `analyze --name X` on a DIFFERENT path is rejected with
// a collision error (exit code 1, "already used" in output)
// 3. `analyze --name X --allow-duplicate-name` bypasses the guard;
// both entries coexist in registry.json
// 4. Pipeline-re-index flags (e.g. --skills) WITHOUT
// --allow-duplicate-name must STILL hit the collision guard —
// the bypass must stay gated on its dedicated flag so it isn't
// silently triggered by unrelated pipeline signals
// (review round 2/3 design decision).
//
// This test invokes the real CLI → runFullAnalysis → registerRepo
// chain, so any wiring regression fails here.
describe('analyze --name <alias> and --allow-duplicate-name (#829)', () => {
// Path-equality assertions across CLI spawn boundaries are fragile
// cross-platform:
// - macOS: os.tmpdir() returns /var/folders/...; child processes
// resolve the symlink to /private/var/folders/...
// - Windows: os.tmpdir() on GitHub runners returns 8.3 short-name
// form (C:\Users\RUNNER~1\...); the child sees the long form
// (C:\Users\runneradmin\...). fs.realpathSync does NOT reliably
// expand 8.3 to long form.
// Rather than fight the platform-path quagmire, we assert STRUCTURAL
// properties: entry count, alias value, path basename, path
// distinctness. That covers the behavior this test is here to
// protect without depending on exact-string path equality.
it('--name alias stores; collision rejects; --allow-duplicate-name bypasses', () => {
// Isolate the global registry so this test never touches the
// developer's real ~/.gitnexus.
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-'));
// Two mini-repo copies whose basenames intentionally collide.
const repoA = makeMiniRepoCopy('collide-app', 'gn-collide-a-');
const repoB = makeMiniRepoCopy('collide-app', 'gn-collide-b-');
const parentA = path.dirname(repoA);
const parentB = path.dirname(repoB);
try {
// Step 1: analyze repoA with --name shared → registry entry created.
const r1 = runCliWithEnv(
['analyze', '--name', 'shared'],
repoA,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r1.status === null) return; // CI timeout tolerance
expect(
r1.status,
[`step 1 exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join(
'\n',
),
).toBe(0);
const registryPath = path.join(gnHome, 'registry.json');
const afterStep1 = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(Array.isArray(afterStep1)).toBe(true);
expect(afterStep1).toHaveLength(1);
expect(afterStep1[0].name).toBe('shared');
expect(path.basename(afterStep1[0].path)).toBe('collide-app');
// Step 2: analyze repoB with the SAME --name → collision error.
const r2 = runCliWithEnv(
['analyze', '--name', 'shared'],
repoB,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r2.status === null) return;
expect(r2.status).toBe(1);
const r2Output = `${r2.stdout}${r2.stderr}`;
expect(r2Output).toMatch(/Registry name collision|already used/i);
// Registry still has just the first entry — step 2 must not have
// silently added, overwritten, or corrupted anything.
const afterStep2 = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterStep2).toHaveLength(1);
// Registry still has only the step-1 entry — the failed call
// must not have silently added, overwritten, or corrupted state.
expect(afterStep2[0].path).toBe(afterStep1[0].path);
// Step 3: REGRESSION GUARD for the missing collision-bypass wire
// (originally a --force passthrough bug; per review round 3 the
// bypass moved to its own --allow-duplicate-name flag to avoid
// conflating it with pipeline re-index).
const r3 = runCliWithEnv(
['analyze', '--name', 'shared', '--allow-duplicate-name'],
repoB,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r3.status === null) return;
expect(
r3.status,
[
`step 3 (--allow-duplicate-name bypass) exited with ${r3.status}`,
`stdout: ${r3.stdout}`,
`stderr: ${r3.stderr}`,
].join('\n'),
).toBe(0);
const afterStep3 = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterStep3).toHaveLength(2);
expect(afterStep3.every((e: { name: string }) => e.name === 'shared')).toBe(true);
// Both entries point to distinct paths (we registered two different
// repos under the same alias) and both have the right basename.
const step3Basenames = afterStep3.map((e: { path: string }) => path.basename(e.path));
expect(step3Basenames).toEqual(['collide-app', 'collide-app']);
const step3Paths = new Set(afterStep3.map((e: { path: string }) => e.path));
expect(step3Paths.size).toBe(2);
// One of the two entries is the original from step 1 — unchanged.
expect(afterStep3.map((e: { path: string }) => e.path)).toContain(afterStep1[0].path);
// Step 4: REGRESSION GUARD for the design decision in review
// round 2/3 — pipeline-re-index flags must NOT bypass the
// registry collision guard. `--skills` triggers pipeline
// re-run (skills generation needs a fresh pipelineResult) but
// must leave the registry guard in force. Bypass requires the
// explicit --allow-duplicate-name flag.
const repoC = makeMiniRepoCopy('collide-app', 'gn-collide-c-');
const parentC = path.dirname(repoC);
try {
const r4 = runCliWithEnv(
['analyze', '--name', 'shared', '--skills'],
repoC,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r4.status === null) return;
expect(r4.status).toBe(1);
const r4Output = `${r4.stdout}${r4.stderr}`;
expect(r4Output).toMatch(/Registry name collision|already used/i);
// The error hint should point at the new flag.
expect(r4Output).toMatch(/--allow-duplicate-name/);
// Registry unchanged — still only A + B under "shared".
const afterStep4 = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterStep4).toHaveLength(2);
} finally {
fs.rmSync(parentC, { recursive: true, force: true });
}
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(parentA, { recursive: true, force: true });
fs.rmSync(parentB, { recursive: true, force: true });
}
}, 360000); // 6-min outer budget (4 × ~60s analyze calls + fixture setup)
});
// ─── gitnexus remove <target> (#664) ─────────────────────────────
//
// End-to-end regression guard for the remove command:
// 1. `remove <alias>` without --force is a dry-run (exit 0, preserves state)
// 2. `remove <alias> --force` deletes the .gitnexus/ directory
// AND unregisters from the global registry
// 3. `remove <unknown>` is idempotent (exit 0 with a warning)
// 4. `remove <ambiguous>` (two entries share the alias via
// --allow-duplicate-name) exits 1 with a disambiguation hint
// and leaves the registry unchanged.
//
// Every assertion reads the real registry.json on disk, so any
// regression in remove.ts → resolveRegistryEntry → unregisterRepo
// will surface here.
describe('remove <target> (#664)', () => {
it('dry-run lists, --force deletes, missing target is a no-op warning', () => {
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-remove-'));
const repoA = makeMiniRepoCopy('remove-me', 'gn-rm-a-');
const parentA = path.dirname(repoA);
try {
// Index the repo under a custom alias so we can target it by
// name below. `--name` guarantees a stable alias regardless of
// how the host resolves the basename/remote-inferred name.
const r1 = runCliWithEnv(
['analyze', '--name', 'alias-a'],
repoA,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r1.status === null) return;
expect(
r1.status,
[`analyze exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join(
'\n',
),
).toBe(0);
const registryPath = path.join(gnHome, 'registry.json');
const afterIndex = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterIndex).toHaveLength(1);
expect(afterIndex[0].name).toBe('alias-a');
// Storage dir must exist before remove so we can assert its
// disappearance below.
const storagePath = afterIndex[0].storagePath;
expect(fs.existsSync(storagePath)).toBe(true);
// Dry-run: must NOT delete. Use parentA as cwd so the test
// never runs with the to-be-removed storage dir as its cwd.
//
// Assert the FULL dry-run output shape, not just the `--force`
// hint (#1003 senior-reviewer NIT): `remove.ts` prints the
// alias, the resolved path, AND the storage path. Verifying
// all three appear catches silent format regressions
// (e.g. a future refactor that accidentally drops one of the
// three `console.log` lines, or swaps `entry.path` for
// `entry.name` in the output).
const r2 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000);
if (r2.status === null) return;
expect(r2.status).toBe(0);
const r2Output = `${r2.stdout}${r2.stderr}`;
expect(r2Output).toMatch(/Run with --force/i);
expect(r2Output, 'dry-run must surface the alias').toContain('alias-a');
expect(r2Output, 'dry-run must surface the repo path').toContain(afterIndex[0].path);
expect(r2Output, 'dry-run must surface the storage path').toContain(storagePath);
expect(fs.existsSync(storagePath)).toBe(true);
// Registry still has the entry.
expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(1);
// --force: must delete storage AND unregister.
const r3 = runCliWithEnv(
['remove', 'alias-a', '--force'],
parentA,
{ GITNEXUS_HOME: gnHome },
15000,
);
if (r3.status === null) return;
expect(
r3.status,
[
`remove --force exited with ${r3.status}`,
`stdout: ${r3.stdout}`,
`stderr: ${r3.stderr}`,
].join('\n'),
).toBe(0);
// Success-case output shape: `Removed: <alias>` header plus the
// same path-and-storagePath lines the dry-run prints (same NIT
// rationale — the success branch mirrors the dry-run's three
// console.log calls, so it has the same silent-regression risk).
const r3Output = `${r3.stdout}${r3.stderr}`;
expect(r3Output).toMatch(/Removed/i);
expect(r3Output, 'success output must surface the alias').toContain('alias-a');
expect(r3Output, 'success output must surface the repo path').toContain(afterIndex[0].path);
expect(r3Output, 'success output must surface the storage path').toContain(storagePath);
expect(fs.existsSync(storagePath)).toBe(false);
expect(JSON.parse(fs.readFileSync(registryPath, 'utf-8'))).toHaveLength(0);
// Idempotent: removing the same alias AGAIN must exit 0 with a
// warning (so `remove X && analyze Y` keeps working in scripts).
const r4 = runCliWithEnv(['remove', 'alias-a'], parentA, { GITNEXUS_HOME: gnHome }, 15000);
if (r4.status === null) return;
expect(r4.status).toBe(0);
expect(`${r4.stdout}${r4.stderr}`).toMatch(/Nothing to remove/i);
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(parentA, { recursive: true, force: true });
}
}, 180000); // 3-min outer budget (1 × ~60s analyze + 3 × fast remove calls)
it('ambiguous target (two entries share alias via --allow-duplicate-name) errors without mutating registry', () => {
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-rm-amb-'));
const repoA = makeMiniRepoCopy('dup', 'gn-dup-a-');
const repoB = makeMiniRepoCopy('dup', 'gn-dup-b-');
const parentA = path.dirname(repoA);
const parentB = path.dirname(repoB);
try {
// Two repos registered under the same alias — only possible via
// --allow-duplicate-name (#829).
const r1 = runCliWithEnv(
['analyze', '--name', 'shared'],
repoA,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r1.status === null) return;
expect(r1.status).toBe(0);
const r2 = runCliWithEnv(
['analyze', '--name', 'shared', '--allow-duplicate-name'],
repoB,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r2.status === null) return;
expect(r2.status).toBe(0);
const registryPath = path.join(gnHome, 'registry.json');
const before = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(before).toHaveLength(2);
// `remove shared` must refuse to guess — exit 1, disambiguation hint.
const r3 = runCliWithEnv(
['remove', 'shared', '--force'],
parentA,
{ GITNEXUS_HOME: gnHome },
15000,
);
if (r3.status === null) return;
expect(r3.status).toBe(1);
const r3Output = `${r3.stdout}${r3.stderr}`;
expect(r3Output).toMatch(/Multiple registered repos match/i);
// Both paths must be surfaced in the hint so the user knows
// which ones to disambiguate between.
expect(r3Output).toMatch(/dup/);
// Registry unchanged — the failed resolution must NOT have
// mutated state.
const after = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(after).toHaveLength(2);
// And path-based remove still works: pass the absolute path of
// repoA and it resolves unambiguously.
//
// We pull the path from the registry snapshot rather than
// passing the outer `repoA` variable directly. This is the
// belt-and-suspenders for cross-platform path normalisation
// (#1003 review): the path the registry recorded has already
// gone through the analyze-side canonicalisation (which on
// macOS expands /var → /private/var and on Windows expands 8.3
// → long-name). Passing that exact string back to `remove`
// guarantees the comparison succeeds even on runners where the
// outer `repoA` is the symlink/short-name form. The code-side
// fix in `canonicalizePath` makes this redundant in practice,
// but the test shouldn't depend on the code fix being perfect
// on every platform — it should prove correctness against the
// registry contract.
const repoAEntry = before.find(
(e: { path: string }) =>
path.basename(e.path) === 'dup' && e.path.includes(path.basename(parentA)),
);
expect(
repoAEntry,
'repoA entry must exist in registry before path-remove step',
).toBeDefined();
const r4 = runCliWithEnv(
['remove', repoAEntry.path, '--force'],
parentA,
{ GITNEXUS_HOME: gnHome },
15000,
);
if (r4.status === null) return;
expect(
r4.status,
[
`remove-by-path exited with ${r4.status}`,
`stdout: ${r4.stdout}`,
`stderr: ${r4.stderr}`,
].join('\n'),
).toBe(0);
const finalEntries = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(finalEntries).toHaveLength(1);
// The survivor is repoB (its path stays in the registry).
expect(path.basename(finalEntries[0].path)).toBe('dup');
// And it's NOT the one we just removed.
expect(finalEntries[0].path).not.toBe(repoAEntry.path);
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(parentA, { recursive: true, force: true });
fs.rmSync(parentB, { recursive: true, force: true });
}
}, 240000); // 4-min outer budget (2 × ~60s analyze + 2 × fast remove)
it('refuses to proceed when a registry entry points storagePath outside <repo>/.gitnexus (#1003)', () => {
// Regression guard for the safety gap flagged by @magyargergo on
// PR #1003: `~/.gitnexus/registry.json` is a user-writable JSON
// file, so a corrupted or hand-edited entry could point
// storagePath at the repo root (catastrophic: rm the working
// tree) or at any other arbitrary path. `remove --force` must
// refuse to call fs.rm when storagePath isn't the canonical
// `<entry.path>/.gitnexus`. We verify:
// 1. Exit code 1 with the actionable "registry entry corrupted"
// hint.
// 2. The .gitnexus/ storage dir is UNTOUCHED.
// 3. The repo itself (entry.path) is UNTOUCHED.
// 4. The registry entry is NOT removed (no partial mutation).
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-poison-'));
const repo = makeMiniRepoCopy('poisoned', 'gn-poison-');
const parent = path.dirname(repo);
try {
// Index the repo normally first so the registry has a valid
// entry we can then poison.
const r1 = runCliWithEnv(
['analyze', '--name', 'poisoned-alias'],
repo,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r1.status === null) return;
expect(r1.status).toBe(0);
const registryPath = path.join(gnHome, 'registry.json');
const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(original).toHaveLength(1);
// Poison the entry: set storagePath to the REPO ROOT itself.
// If the guard isn't in place, `remove --force` would call
// `fs.rm(repo, {recursive: true, force: true})` and wipe the
// entire working tree.
const poisoned = [{ ...original[0], storagePath: repo }];
fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2));
// Sanity: storage dir and working tree both still exist.
expect(fs.existsSync(path.join(repo, '.gitnexus'))).toBe(true);
expect(fs.existsSync(repo)).toBe(true);
expect(fs.existsSync(path.join(repo, '.git'))).toBe(true);
// Attempt the remove — must FAIL without deleting anything.
const r2 = runCliWithEnv(
['remove', 'poisoned-alias', '--force'],
parent,
{ GITNEXUS_HOME: gnHome },
15000,
);
if (r2.status === null) return;
expect(
r2.status,
[`remove should have exited 1`, `stdout: ${r2.stdout}`, `stderr: ${r2.stderr}`].join(
'\n',
),
).toBe(1);
const r2Output = `${r2.stdout}${r2.stderr}`;
// Must surface the actionable "registry corrupted" hint, not
// just a raw fs.rm error.
expect(r2Output).toMatch(/Refusing to remove/i);
expect(r2Output).toMatch(/registry\.json/i);
// Repo + .gitnexus dir + .git dir must all still exist — the
// guard aborts BEFORE fs.rm. This is the whole point of the
// test: the working tree is not allowed to disappear.
expect(fs.existsSync(repo), 'repo working tree must survive').toBe(true);
expect(fs.existsSync(path.join(repo, '.gitnexus')), 'storage dir must survive').toBe(true);
expect(fs.existsSync(path.join(repo, '.git')), '.git must survive').toBe(true);
// Registry unchanged — no partial mutation.
const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterRegistry).toHaveLength(1);
expect(afterRegistry[0].storagePath).toBe(repo); // still poisoned (we did that)
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(parent, { recursive: true, force: true });
}
}, 120000); // 2-min budget (1 × ~60s analyze + 1 × fast remove-refused)
});
// ─── clean --all: same safety guard applies (#1003 review) ───────
//
// The `clean --all` path iterates over the registry and calls
// `fs.rm(entry.storagePath)` — identical trust-the-registry pattern
// as `remove` had before the guard. A poisoned entry must be SKIPPED
// (not aborted), so clean --all preserves its existing per-repo
// error-tolerance semantics: one bad entry does not halt cleanup of
// the rest. We verify:
// 1. The poisoned entry is NOT deleted (working tree + .gitnexus
// survive), and the CLI prints a "Refusing to clean" message.
// 2. The poisoned entry is left in the registry (nothing was
// mutated for it).
// 3. A co-existing well-formed entry IS still cleaned (both its
// .gitnexus dir AND its registry entry are gone).
describe('clean --all with a poisoned registry entry (#1003)', () => {
it('skips poisoned entries, cleans valid ones, never deletes the working tree', () => {
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-clean-poison-'));
const repoBad = makeMiniRepoCopy('bad-repo', 'gn-clean-bad-');
const repoGood = makeMiniRepoCopy('good-repo', 'gn-clean-good-');
const parentBad = path.dirname(repoBad);
const parentGood = path.dirname(repoGood);
try {
// Analyze both so the registry has two well-formed entries.
for (const [repo, alias] of [
[repoBad, 'bad-alias'],
[repoGood, 'good-alias'],
] as const) {
const r = runCliWithEnv(
['analyze', '--name', alias],
repo,
{ GITNEXUS_HOME: gnHome },
60000,
);
if (r.status === null) return;
expect(r.status, `analyze ${alias} exited ${r.status}: ${r.stdout}${r.stderr}`).toBe(0);
}
const registryPath = path.join(gnHome, 'registry.json');
const original = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(original).toHaveLength(2);
// Poison the 'bad-alias' entry by pointing its storagePath at
// the repo root itself. If the guard isn't wired into the
// clean --all loop, `clean --all --force` would fs.rm the
// working tree.
const poisoned = original.map((e: { name: string; storagePath: string; path: string }) =>
e.name === 'bad-alias' ? { ...e, storagePath: repoBad } : e,
);
fs.writeFileSync(registryPath, JSON.stringify(poisoned, null, 2));
// Sanity: both working trees and .gitnexus dirs still exist.
expect(fs.existsSync(repoBad)).toBe(true);
expect(fs.existsSync(path.join(repoBad, '.gitnexus'))).toBe(true);
expect(fs.existsSync(path.join(repoBad, '.git'))).toBe(true);
expect(fs.existsSync(path.join(repoGood, '.gitnexus'))).toBe(true);
// clean --all --force from a neutral cwd (parentBad), so the
// command isn't "inside" either repo.
const r = runCliWithEnv(
['clean', '--all', '--force'],
parentBad,
{ GITNEXUS_HOME: gnHome },
30000,
);
if (r.status === null) return;
// clean --all's per-entry error handling always exits 0 at
// the end (it only logs per-repo failures). The important
// assertions are on side effects, not the exit code.
const output = `${r.stdout}${r.stderr}`;
expect(output).toMatch(/Refusing to clean/i);
expect(output).toMatch(/bad-alias/);
// Poisoned repo: working tree + .gitnexus + .git all SURVIVE.
expect(fs.existsSync(repoBad), 'poisoned repo working tree must survive').toBe(true);
expect(
fs.existsSync(path.join(repoBad, '.gitnexus')),
'poisoned repo .gitnexus must survive (guard refused to rm repo root)',
).toBe(true);
expect(fs.existsSync(path.join(repoBad, '.git')), '.git must survive').toBe(true);
// Good repo: its .gitnexus IS gone (cleanup succeeded despite
// the poisoned sibling entry — per-entry error tolerance is
// preserved).
expect(
fs.existsSync(path.join(repoGood, '.gitnexus')),
'good repo .gitnexus should be cleaned',
).toBe(false);
// But the good repo's working tree stays (clean never touches
// anything outside .gitnexus).
expect(fs.existsSync(repoGood), 'good repo working tree must survive').toBe(true);
// Registry post-state: poisoned entry still present (skipped,
// not mutated); good entry unregistered.
const afterRegistry = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
expect(afterRegistry).toHaveLength(1);
expect(afterRegistry[0].name).toBe('bad-alias');
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(parentBad, { recursive: true, force: true });
fs.rmSync(parentGood, { recursive: true, force: true });
}
}, 240000); // 4-min budget (2 × ~60s analyze + 1 × fast clean --all)
});
describe('unhappy path', () => {
it('exits with error when no command is given', () => {
const result = runCliRaw([], MINI_REPO);
// Accept timeout as valid on slow CI
if (result.status === null) return;
// Commander exits with code 1 when no subcommand is given and
// prints a usage/error message to stderr.
expect(result.status).toBe(1);
const combined = result.stdout + result.stderr;
expect(combined.length).toBeGreaterThan(0);
});
it('shows help with --help flag', () => {
const result = runCliRaw(['--help'], MINI_REPO);
// Accept timeout as valid on slow CI
if (result.status === null) return;
expect(result.status).toBe(0);
// Commander writes --help output to stdout.
expect(result.stdout).toMatch(/Usage:/i);
// The program name and at least one known subcommand should appear.
expect(result.stdout).toMatch(/gitnexus/i);
expect(result.stdout).toMatch(/analyze|status|serve/i);
});
it('fails with unknown command', () => {
const result = runCliRaw(['nonexistent'], MINI_REPO);
// Accept timeout as valid on slow CI
if (result.status === null) return;
// Commander exits with code 1 and prints an error to stderr for unknown commands.
expect(result.status).toBe(1);
expect(result.stderr).toMatch(/unknown command/i);
});
});
describe('CLI error handling', () => {
/**
* Helper to spawn CLI from a cwd outside the project tree.
* Uses the absolute file:// URL to tsx loader so the --import hook
* resolves even when cwd has no node_modules.
*/
function runCliOutsideProject(args: string[], cwd: string, timeoutMs = 15000) {
return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...args], {
cwd,
encoding: 'utf8',
timeout: timeoutMs,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
});
}
it('status on non-indexed repo reports not indexed', () => {
// Even though MINI_REPO is now in an isolated tmpdir, previous tests
// in this suite may have created MINI_REPO/.gitnexus via analyze,
// and findRepo() walks up so any `.gitnexus` along the path still
// counts. This test needs a GUARANTEED pristine repo to assert the
// "not indexed" output, so it mints its own throwaway tmp git repo.
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-noindex-'));
try {
spawnSync('git', ['init'], { cwd: tmpDir, stdio: 'pipe' });
spawnSync('git', ['commit', '--allow-empty', '-m', 'init'], {
cwd: tmpDir,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
const result = runCliOutsideProject(['status'], tmpDir);
if (result.status === null) return;
expect(result.status).toBe(0);
expect(result.stdout).toMatch(/Repository not indexed/);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('status on non-git directory reports not a git repo', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-nogit-'));
try {
const result = runCliOutsideProject(['status'], tmpDir);
if (result.status === null) return;
// status.ts doesn't set process.exitCode — just prints and returns
expect(result.status).toBe(0);
expect(result.stdout).toMatch(/Not a git repository/);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('analyze on non-git directory fails with exit code 1', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-nogit-'));
try {
// Pass the non-git path as a separate argument via runCliRaw
// (runCli passes the whole string as one arg which breaks path parsing)
const result = runCliRaw(['analyze', tmpDir], repoRoot);
if (result.status === null) return;
// analyze.ts sets process.exitCode = 1 for non-git paths
expect(result.status).toBe(1);
expect(result.stdout).toMatch(/not.*git repository/i);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
});
// ─── wiki command flags ─────────────────────────────────────────────
describe('wiki command flags', () => {
it('wiki --help shows --provider, --review, --verbose flags', () => {
const result = runCliRaw(['wiki', '--help'], repoRoot);
if (result.status === null) return;
expect(result.status).toBe(0);
expect(result.stdout).toContain('--provider <provider>');
expect(result.stdout).toContain('--review');
expect(result.stdout).toContain('-v, --verbose');
expect(result.stdout).toContain('--model <model>');
expect(result.stdout).toContain('--gist');
expect(result.stdout).toContain('--concurrency <n>');
});
it('wiki on non-git directory fails with exit code 1', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wiki-nogit-'));
try {
const result = runCliRaw(['wiki', tmpDir], repoRoot);
if (result.status === null) return;
expect(result.status).toBe(1);
expect(result.stdout).toMatch(/not.*git repository/i);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('wiki on non-indexed repo fails with "No GitNexus index"', () => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wiki-noindex-'));
try {
spawnSync('git', ['init'], { cwd: tmpDir, stdio: 'pipe' });
spawnSync('git', ['commit', '--allow-empty', '-m', 'init'], {
cwd: tmpDir,
stdio: 'pipe',
env: {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
},
});
// Must spawn outside project tree so it doesn't find parent .gitnexus
const result = spawnSync(
process.execPath,
['--import', tsxImportUrl, cliEntry, 'wiki', tmpDir],
{
cwd: tmpDir,
encoding: 'utf8',
timeout: 15000,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
},
);
if (result.status === null) return;
expect(result.status).toBe(1);
expect(result.stdout).toMatch(/No GitNexus index found/);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('wiki --provider cursor without API key does not prompt for key in non-TTY', () => {
// In non-TTY (piped stdin), --provider cursor should skip the API key prompt
// and proceed (or fail gracefully with Cursor CLI not found)
const result = runCliRaw(['wiki', MINI_REPO, '--provider', 'cursor'], repoRoot, 15000);
if (result.status === null) return;
const combined = result.stdout + result.stderr;
// Should NOT ask for API key — cursor provider doesn't need one
expect(combined).not.toMatch(/API key:/);
});
it('wiki --help includes --verbose flag description', () => {
const result = runCliRaw(['wiki', '--help'], repoRoot);
if (result.status === null) return;
expect(result.status).toBe(0);
expect(result.stdout).toMatch(/verbose/i);
});
});
// ─── stdout fd 1 tests (#324) ───────────────────────────────────────
// These tests verify that tool output goes to stdout (fd 1), not stderr.
// Requires analyze to have run first (the analyze test above populates .gitnexus/).
// All tool commands pass --repo to disambiguate when the global registry
// has multiple indexed repos (e.g. the parent project is also indexed).
describe('tool output goes to stdout via fd 1 (#324)', () => {
it('cypher: JSON appears on stdout, not stderr', () => {
const result = runCliRaw(
['cypher', 'MATCH (n) RETURN n.name LIMIT 3', '--repo', 'mini-repo'],
MINI_REPO,
);
if (result.status === null) return; // CI timeout tolerance
expect(result.status).toBe(0);
// stdout must contain valid JSON (array or object)
expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
// stderr must NOT contain JSON — only human-readable diagnostics allowed
const stderrTrimmed = result.stderr.trim();
if (stderrTrimmed.length > 0) {
expect(() => JSON.parse(stderrTrimmed)).toThrow();
}
});
it('query: JSON appears on stdout, not stderr', () => {
// "handler" is a generic term likely to match something in mini-repo
const result = runCliRaw(['query', 'handler', '--repo', 'mini-repo'], MINI_REPO);
if (result.status === null) return;
expect(result.status).toBe(0);
expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
});
it('impact: JSON appears on stdout, not stderr', () => {
const result = runCliRaw(
['impact', 'handleRequest', '--direction', 'upstream', '--repo', 'mini-repo'],
MINI_REPO,
);
if (result.status === null) return;
expect(result.status).toBe(0);
// impact may return an error object (symbol not found) or a real result —
// either way it must be valid JSON on stdout
expect(() => JSON.parse(result.stdout.trim())).not.toThrow();
});
it('stdout is pipeable: cypher output parses as valid JSON', () => {
const result = runCliRaw(
['cypher', 'MATCH (n:Function) RETURN n.name LIMIT 5', '--repo', 'mini-repo'],
MINI_REPO,
);
if (result.status === null) return;
expect(result.status).toBe(0);
// Simulate what jq does: parse stdout as JSON
const parsed = JSON.parse(result.stdout.trim());
expect(Array.isArray(parsed) || typeof parsed === 'object').toBe(true);
});
});
// ─── EPIPE clean exit test (#324) ───────────────────────────────────
describe('EPIPE handling (#324)', () => {
it('cypher: EPIPE exits with code 0, not stderr dump', () => {
return new Promise<void>((resolve, reject) => {
const child = spawn(
process.execPath,
[
'--import',
tsxImportUrl,
cliEntry,
'cypher',
'MATCH (n) RETURN n LIMIT 500',
'--repo',
'mini-repo',
],
{
cwd: MINI_REPO,
stdio: ['ignore', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
},
);
let stderrOutput = '';
child.stderr.on('data', (chunk: Buffer) => {
stderrOutput += chunk.toString();
});
// Destroy stdout immediately — simulates `| head -0` (consumer closes early)
child.stdout.once('data', () => {
child.stdout.destroy(); // triggers EPIPE on next write
});
const timer = setTimeout(() => {
child.kill('SIGTERM');
// Timeout is acceptable on CI — not a failure
resolve();
}, 20000);
child.on('close', (code) => {
clearTimeout(timer);
try {
// Clean EPIPE exit: code 0
expect(code).toBe(0);
// No JSON payload should appear on stderr
const trimmed = stderrOutput.trim();
if (trimmed.length > 0) {
expect(() => JSON.parse(trimmed)).toThrow();
}
resolve();
} catch (err) {
reject(err);
}
});
});
}, 25000);
});
// ─── eval-server READY signal test (#324) ───────────────────────────
describe('eval-server READY signal (#324)', () => {
it('READY signal appears on stdout, not stderr', () => {
return new Promise<void>((resolve, reject) => {
const child = spawn(
process.execPath,
['--import', tsxImportUrl, cliEntry, 'eval-server', '--port', '0', '--idle-timeout', '3'],
{
cwd: MINI_REPO,
stdio: ['ignore', 'pipe', 'pipe'],
env: {
...process.env,
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
},
},
);
let stdoutBuffer = '';
let foundOnStdout = false;
let foundOnStderr = false;
child.stdout.on('data', (chunk: Buffer) => {
stdoutBuffer += chunk.toString();
if (stdoutBuffer.includes('GITNEXUS_EVAL_SERVER_READY:')) {
foundOnStdout = true;
child.kill('SIGTERM');
}
});
child.stderr.on('data', (chunk: Buffer) => {
const text = chunk.toString();
if (text.includes('GITNEXUS_EVAL_SERVER_READY:')) {
foundOnStderr = true;
child.kill('SIGTERM');
}
});
const timer = setTimeout(() => {
child.kill('SIGTERM');
// Timeout is acceptable on CI — not a failure
resolve();
}, 30000);
child.on('close', () => {
clearTimeout(timer);
try {
if (foundOnStderr) {
reject(new Error('READY signal appeared on stderr instead of stdout'));
} else if (foundOnStdout) {
resolve();
} else {
// eval-server may not start on all CI environments — don't fail
resolve();
}
} catch (err) {
reject(err);
}
});
});
}, 35000);
});
});