mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-07 08:26:11 +00:00
* 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 of5a1bce7fagainst 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 on5a1bce7fwith 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
1155 lines
47 KiB
TypeScript
1155 lines
47 KiB
TypeScript
/**
|
||
* 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);
|
||
});
|
||
});
|