Commit graph

9 commits

Author SHA1 Message Date
samuel mbabhazi
9772bcd24f
feat(api_impact): list the callers of the requesting function as indirect consumers (#3720)
* feat(api_impact): list the callers of the requesting function as indirect consumers

The FETCHES edge is file-level, so api_impact saw only the file that sends
the request. The files that call its request function never mention the URL
and were missing from the report (#3644).

The edge's reason now names the callables whose spans contain the request
sites, one `|via:<id>` field each, under the containment rule the R3-6 sink
set already uses; that rule moves into EnclosingCallableIndex so both stay
one. Several sites of one file for one route now share the edge instead of
colliding on its id. api_impact follows the CALLS edges into those callables
one level up and reports the callers as `indirectConsumers`, apart from the
direct consumers and outside the risk level. Direct consumer files and the
handler file are left out. An index built before the attribution carries no
field. Expo navigation sites carry their real line so they attribute to the
component that contains them.

Closes #3644

* fix(api_impact): attribute a request site by its AST enclosing callable

A helper defined on the line of the request has the smaller span, so the
line-only lookup named it and the callers of the real requesting function
vanished from indirectConsumers. The parser now records the enclosing
callable of each fetch site and of each wrapper call (through the call's
position), and attribution prefers that identity; the span lookup remains
for sites found by text scan. The same preference applies to the R3-6 sink
set. A real analyze of the fixture followed by api_impact pins the helper
case and the wrapper case.

* test(bench): rebaseline receiver resolution for the indirect-consumers fixture

The api-impact-indirect-consumers fixture adds three receiver-unresolved
sites to the corpus: two `.then(...)` calls on the untyped return of
`fetch(...)` and of the local wrapper `apiFetch(...)`, and one
`req.query.path` read inside an inline Express handler with an untyped
`req`. Call drops 138 -> 140, reads 30 -> 31, all kinds 168 -> 171. No
shape-arm state and no existing site changed.

* fix(api_impact): preserve module-scope request attribution

Keep AST-confirmed module scope distinct from missing attribution for direct
fetches, HTTP clients, and wrapper calls. Prevent overlapping callable spans
from inventing indirect consumers or process sinks.

Preserve span fallback for callable forms the generic identity walk cannot
name. Add eight regression cases, including real analyze/api_impact coverage.
Rebaseline receiver-resolution counts for the two then calls and one property
read added by this PR's existing fixtures; shape expectations are unchanged.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-10-25 18:29:09 +00:00
Gergő Magyar
01af833875
fix(scope): preserve lexical bindings in HTTP wrapper detection (#3632)
* docs(scope): clarify lexical binding ownership and wrapper provenance

Checkpoint for #3630. Publishing the implemented source, regression tests, and benchmark harness as a draft PR; full-suite and final scaling validation remain in progress.

* fix(scope): publish #3630 docs/plans

* fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance

* fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance/fixtures

* fix(scope): publish #3630 gitnexus/bench/lexical-binding-provenance/results

* fix(scope): publish #3630 gitnexus/src/core/ingestion

* fix(scope): publish #3630 gitnexus/src/core/ingestion/languages/typescript

* fix(scope): publish #3630 gitnexus/src/core/ingestion/languages

* fix(scope): publish #3630 gitnexus/src/core/ingestion/pipeline-phases

* fix(scope): publish #3630 gitnexus/src/core/ingestion/route-extractors

* fix(scope): publish #3630 gitnexus/src/core/ingestion/workers

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/lib

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/packages/http

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/packages/http/src

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/server

* fix(scope): publish #3630 gitnexus/test/fixtures/lang-resolution/fetch-wrapper-target-identity/web

* fix(scope): publish #3630 gitnexus/test/integration/resolvers

* fix(scope): publish #3630 gitnexus/test/unit

* fix(scope): publish #3630 gitnexus/test/unit/group

* fix(scope): publish #3630 gitnexus/test/unit/scope-resolution

* fix(scope): publish #3630 gitnexus/test/unit/scope-resolution/typescript

* fix(scope): publish #3630 gitnexus-shared/src/scope-resolution

* fix(scope): treat wrapped assignment targets as writes

`(app) = new Map()`, `(app as T) = x` and `app! = x` assign the same binding as
`app = x`, but route-fact invalidation and the shared pattern walker stopped at
the wrapper, so Express provenance survived the write and produced a false
route. Recurse through parenthesized, as/satisfies and non-null targets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(routes): resolve wrappers through overloads, declarator lists and barrels

Three wrapper shapes lost their identity:

- Bodyless overload signatures were indexed as separate definitions sharing
  the implementation's binding, so the binding looked ambiguous and both local
  and imported calls were rejected. Signatures now declare no callable.
- Facts for `const base = 0, request = (url) => fetch(url)` followed the first
  declarator. The captured name now selects its own declarator.
- An import resolved to a workspace barrel stopped at index.ts. Workers now
  record named, star and imported-then-exported re-exports, and resolution
  follows them to the declaring module: local and named exports shadow
  `export *`, star exports never carry `default`, and conflicting star exports
  block the name. No repository-wide name fallback is restored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(scope): assert closure ownership against the declaration

The closure-write assertion compared one lookup with itself, which also passes
when both are undefined. Compare it with the outer declaration and the shadowing
parameter, and keep the memoization check explicit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(routes): share lexical helpers and release per-file HTTP contexts

- Share binding keys, parameter patterns and transparent expression wrappers
  from lexical-bindings instead of four local copies; drop the unused
  moduleStem export; name the inferred-source predicates in lookup.
- Key the per-file HTTP context by its tree rather than its ParsedFile.
  Workers retain ParsedFiles for the whole chunk, which kept every file's AST
  and traversal maps alive until the chunk flushed.
- Index re-exports by name, memoize barrel walks per starting export, and keep
  facts without import targets as-is instead of copying them.
- Move TS-only wrapper cases into the TS describe.each instead of branching
  inside the per-language loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(routes): keep wrapper identity for HOCs, CommonJS, object keys and fetch packages

Four definition shapes that main linked by name lost their wrapper identity:

- `const load = useCallback((url) => fetch(url))` and curried HOCs produced no
  facts because definition lookup stopped at the call; it now follows the
  callable argument, as the structural definition capture does.
- `module.exports = { request }`, `exports.x = …` and `module.exports.x = …`
  were not exports, so destructured `require` callers missed the wrapper.
  CommonJS export assignments are recorded when `module`/`exports` are the
  unshadowed globals, including functions defined inline in the export object.
- An object-literal key or method name was resolved as a lexical binding and
  collided with a same-named module wrapper. Only binding identifiers now name
  a definition's binding.
- `import fetch from 'node-fetch'` no longer counted as fetch. Imports of known
  fetch implementations (node-fetch, cross-fetch, isomorphic-fetch, the fetch
  member of undici and whatwg-fetch) are fetch evidence again; parameters and
  other local names still are not.

Reference identifiers are also resolved only when their name is bound to a
callable or class region; other identifiers cannot add a reachability edge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(routes): drive the forwarding fallback case from data and cover re-export merges

Replace the boolean-switched forwarding fallback case with table rows instead
of branching inside the test, and assert that barrel re-export facts survive
worker sub-batch merges.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): reject stale or dirty call-graph exports

The committed comparison named a candidate revision that is not on this branch,
and nothing stopped it. Comparison now requires both exports to come from clean
checkouts and the candidate export to come from the checkout running the
comparison.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): regenerate the call-graph comparison on the reviewed head

Clean baseline 565e50276 and clean candidate 8a5f7b738 on the frozen 432-file
corpus (fingerprint 88be1faf...): exactly one CALLS target changes, the
annotated closure probe `declaredOwner.writeAndRead` now calls `Base.m`
instead of `Foo.m`. The other 431 CALLS and all 11 METHOD_OVERRIDES edges are
unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): emit export provenance and review notes from the comparison

The comparison report carried export provenance and the expected-change
explanation only as hand-added fields, which a regeneration dropped. Emit
both exports' revisions, dirty flags and fingerprints, the unchanged CALLS
and METHOD_OVERRIDES counts, and an optional `--note` explanation directly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): record calibrated scaling reports and the regenerated comparison

Duration-calibrated-v2 runs (5 warmups, 15 measured batches, 500 ms pilot
target) pinned to the same CPU, baseline then candidate back to back:

- Baseline 565e50276 exceeds the 1.8 normalized ceiling on six arms at 4x
  input: position-index 7.92, position-deep-completed 4.65, nested-helpers
  4.82, nested-helper-chain 14.05, reverse-chain 4.31, name-collisions 2.61
  (medians).
- Candidate 94f1a055b passes --check: every arm stays at or below 1.32 at 4x
  for both minimum and median, with correct semantic guards on every arm.

The comparison is regenerated from clean exports with its provenance and
review note emitted by the tool: one expected CALLS target change, 431 CALLS
and 11 METHOD_OVERRIDES unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(scope): replace recursion with explicit stacks in new traversals

The binding work added five recursive traversals whose depth follows input
shape: region summaries (callable nesting), barrel resolution (re-export
chain length), pattern binding names (destructuring depth), route-fact write
invalidation (pattern depth), and the position index's max-end tree. Each
now uses an explicit stack with the same visiting order and results.

Regression cases cover the inputs the recursive versions could not handle:
a 20,000-deep destructuring pattern and a 50,000-module re-export chain both
threw RangeError before this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* perf(routes): avoid a full-tree script walk when building lexical bindings

Route-fact extraction ran 1.78x slower than main on a 512-function file
(87 ms vs 49 ms) because createLexicalBindings walked every AST node in script
to detect collapsed parameter/body environments, which only functions with
parameter defaults or computed keys can have.

- Find functions with the native type search, skip plain parameter lists
  without any subtree search, and walk only the formals and bodies of the
  rest. Detection is unchanged, except that functions inside syntax-error
  regions are now also checked, which can only add fail-closed ambiguity.
- Read each node's id and start position once per lookup instead of
  repeatedly crossing into native getters.

The same extraction now takes about 56 ms against 51 ms on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* perf(routes): walk wrapper evidence with a tree cursor

The wrapper context visited every node through namedChildren, which
materializes each child as a SyntaxNode and records a parent edge by node
id. A tree cursor reads node types and offsets natively, so only the node
types the handlers inspect become SyntaxNodes. Declared identifiers are
keyed by start offset, and the walk records the ancestor facts later
queries need (stored binding, default export, declarator of a name)
instead of a parent map.

On the 512-wrapper scaling input, context construction drops from about
58 ms to 48 ms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): regenerate the call-graph comparison on the cursor walk

The candidate export at 6061ca71b has the same fingerprint as the previous
candidate: the traversal rewrite changes no edge. The one expected target
change and the 431 unchanged CALLS and 11 METHOD_OVERRIDES remain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): record final scaling reports on the cursor walk

Baseline 565e50276 and candidate 61de364ff ran back to back on CPU 23
with the duration-calibrated-v2 protocol. Every candidate check stays at
or below a 1.32 normalized factor against the 1.8 ceiling; the baseline
fails six of nine arms. Workload fingerprints agree except the documented
name-collision false positive in the baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(plan): checkpoint the final review, scaling and suite results for #3630

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(routes): treat export aliases of one wrapper as one star-export target

Star-export candidates were deduplicated by the resolved export name, so
`export { request, request as send }` reached through two barrels counted
as two targets and blocked the import, dropping the consumer's FETCHES
edge. Candidates are now keyed by the declaration each export name names,
matching ECMAScript ResolveExport's binding comparison; distinct
declarations exported under the same name stay ambiguous.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): regenerate scaling and call-graph reports on the alias fix

Baseline 565e50276 and candidate 4d62bd045 ran back to back on CPU 23.
Every candidate check stays at or below 1.22 against the 1.8 ceiling;
the baseline still fails six of nine arms. The CALLS/METHOD_OVERRIDES
export is unchanged (fingerprint 4db24f57), so the comparison only
records the new candidate revision.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(plan): record the export-alias fix and the regenerated reports for #3630

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(scope): keep the type a write gives an undeclared name

A constructor write to a name no scope declares (`client = new ApiClient()`
in sloppy JavaScript) declares nothing, so the lookup stays absent. The
carried constructor fact was dropped on that exit, which lost the receiver
type main had inferred for later reads in the writing scope. The absent
result now carries it, and findReceiverTypeBinding uses it before the
namespace fallbacks. Only complete-claim providers carry such a fact.

Tests pin the carried type per inferred source, the undeclared write, and
preservePosition across a module redirect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(routes): close the wrapper identity gaps found in review

- CommonJS wrappers captured as their assignment (`exports.x = function`,
  `module.exports.x = () =>`) produce facts again: findFunction follows the
  assignment's right side.
- Name-only fallback considers only definitions that can be the target: an
  unresolvable import names a module-level export, an undeclared global a
  top-level function. Nested and unexported helpers no longer erase a
  wrapper's edges.
- Only possible wrappers and module-level functions are emitted, and the
  routes phase builds the index only when routes and candidate calls exist.
- A rebound binding (later assignment, `var` initializer over a function
  declaration, reassigned export) proves no wrapper across files.
- `(<T>x) = ...` is a write; transparent wrappers skip type arguments.
- Calls inside recovered syntax are still collected; instance-field
  initializers resolve after the module body, so a later declaration is
  visible.
- Local exports are always recorded, so a star export conflicting with a
  non-callable export is ambiguous, and fully resolved barrel frames are
  shared across walks.
- HTTP hooks run per file under a guard; a failing file or one without a
  ParsedFile still records its function names so fallback declines.
- namesFetch is iterative, large patterns no longer spread into push, the
  facts cache is keyed by the name node, and the provider context cache
  checks its ParsedFile.

Tests cover each case; the behavior rows fail on the previous source.
The TS-only call cases no longer branch inside the test file, call targets
are asserted exactly, the function-type list is drift-checked, and the
persistence test joins the cross-platform worker list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(routes): describe wrapper fallback, barrel and recall boundaries

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): count the new CommonJS fixture's dropped write site

`lib/cjs-member.js` in the fetch-wrapper target-identity fixture writes
`module.exports.arrowRequest`, a member write whose receiver is not typed.
That adds one `receiver-unresolved` write (190 -> 191 all-kind drops,
19 -> 20 writes). Call drops and shape-arm states are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): regenerate scaling and call-graph reports on the review fixes

Baseline 565e50276 and candidate 528ea1478 ran back to back on CPU 23.
Every candidate check stays at or below 1.27 against the 1.8 ceiling; the
baseline still fails six of nine arms. The CALLS/METHOD_OVERRIDES export
is unchanged (fingerprint 4db24f57), so the comparison only records the
new candidate revision.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(plan): checkpoint the review fixes and regenerated reports for #3630

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(routes): share CommonJS checks and trim per-node work

- `isCommonJsGlobal`/`isModuleExports` in lexical-bindings replace the two
  copies in fetch-wrapper and framework-route-facts (both resolve the same
  scope from the same start position).
- LEXICAL_FUNCTION_TYPES is derived from the TypeScript FUNCTION_NODE_TYPES
  instead of a third hand-kept copy; the drift test still pins it.
- `unwrap` reads UNWRAPPED_TYPES, the set the cursor walk mirrors, and the
  class-body region is no longer shadowed under the name `deferred`.
- framework-route-facts reads each node's type once per loop iteration.
- The position index returns the latest-starting scope when it already
  contains the point, before the max-end tree search; that entry is the
  search's rightmost match.
- carried() returns a shared empty set for argument forms that cannot carry
  a value, the resolution file set is built without intermediate arrays,
  and the routes phase reads the call list once.

No behavior change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): regenerate scaling and call-graph reports on the refactor

Baseline 565e50276 and candidate 3db8657ba ran back to back on CPU 23.
Every candidate check stays at or below 1.32 against the 1.8 ceiling; the
baseline still fails six of nine arms. The CALLS/METHOD_OVERRIDES export
is unchanged (fingerprint 4db24f57). The later main merge (e2a92dab8)
touches no measured file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(plan): checkpoint the simplification pass and its reports for #3630

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address PR review feedback (#3632)

- Destructuring defaults (`{ fetch = globalThis.fetch } = {}`, renamed,
  array and local forms) are alias evidence for their leaf binding and
  no longer count a second initialization, which also invalidated
  bindings like `[send = other] = values`.
- `export default (request)` records the default export: the value is
  unwrapped before the identifier check.
- The HTTP fallback records and the collapsed-environment test row now
  say what they stand for.

Regression rows cover each form in JS, TS and TSX and fail on the
previous source; two fixture consumers cover imported wrappers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore(bench): refresh lexical binding reports after review fixes

Regenerate clean baseline and candidate measurements at c263c4f72.
All candidate scaling checks pass, the semantic delta remains one CALLS
target, and receiver-resolution counts match the existing baseline.

* docs(plan): checkpoint refreshed review-fix measurements

Record c263c4f72 review fixes, the regenerated scaling and semantic
results, and the passing receiver and CI checks.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-20 11:36:06 +01:00
Gergő Magyar
f8209df2e2
fix(routes): resolve Express receivers and router mount paths (#3603)
* fix(routes): resolve Express receiver provenance and mounted paths

Require statically proven framework receivers, compose Router mount paths,
and share the same provider resolution between ingestion and group routes.
Preserve worker and parse-cache parity with per-file facts.

* fix(review): apply review findings

Invalidate exported Routers through namespace property mutations and retain
uncertain incoming mounts so invalid parents cannot revive local paths.

* refactor(routes): reuse export checks and fixture writer

* fix(routes): evaluate fluent call receivers once

* fix(review): preserve provider paths and uncertain mounts

* fix(test): distinguish parameterized router mutation cases

* docs: remove Express route implementation plan
2026-10-16 18:25:40 +01:00
Richie Wee
b51f25606c
fix(routes): match configured fetch wrapper calls that pass type arguments (#3567)
Some checks failed
Devcontainer Smoke / Build devcontainer image (push) Has been cancelled
Devcontainer Smoke / Config-transform unit tests (push) Has been cancelled
Skill copy sync / shipped skills drift guard (push) Has been cancelled
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
2026-10-15 20:08:56 +03:00
Gergő Magyar
8fa8e64388
fix(rename): extend safe refactoring and flow coverage across languages (#3557) 2026-10-13 08:38:17 +03:00
azizur100389
4f16bd8023
fix(impact): report scope extraction omissions (#3071)
* fix(impact): surface scope extraction omissions

* fix(impact): preserve complete index fixtures

* fix(impact): preserve scope completeness evidence

* test(analyze): model successful scope extraction in harnesses

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-08-29 08:38:20 +01:00
Gergő Magyar
5f4964b4e6
fix: resolve imported/composed FastAPI route path constants (#2391) (#2393)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat(routes): add pure Python string-constant resolver (#2391 U1)

* feat(routes): extract Python module constants from tree (#2391 U2)

* feat(routes): capture non-literal FastAPI decorator args + per-file constants, bump parse-cache schema (#2391 U3)

* feat(routes): resolve composed decorator route constants in parse-impl + skip floor (#2391 U4)

* feat(routes): resolve composed FastAPI route constants in group HTTP-contract layer (#2391 U5)

* test(routes): multi-hop, ingestion↔group parity, and warm-cache regression locks (#2391 U6)

* docs(routes): mark the language-agnostic seam for cross-language const resolution (#2391)

* refactor(routes): extract language-agnostic constant-fold core; Python becomes a binding (#2391)

The fold, cycle guard, and depth cap now live in constant-resolver.ts and take a
pluggable ImportResolver. python-const-resolver.ts supplies the Python import
semantics + tree extractor and re-exports the same surface, so no call site
changes. A Spring/Kotlin/C# binding can now reuse the core with its own resolver
(proven by constant-resolver.test.ts driving it with a Java-style resolver).

* fix(routes): treat the constant-fold cycle guard as a recursion stack (#2391)

The `visited` set in `foldName` was added-to but never removed on unwind, so a
constant referenced more than once in a single fold — `A + A`, a reused
separator (`SLASH + PATH + SLASH`), or a diamond `X = P + Q` where P and Q share
a base — tripped the cycle guard on its second occurrence and the whole route
was silently dropped by the skip floor. Pop the guard in `finally` so it tracks
the ACTIVE resolution stack, not every name ever seen: a true cycle (a name
still on the stack) is still caught, but a name that already resolved and popped
folds again. Re-computation stays bounded by MAX_RESOLVE_DEPTH, so no blowup is
reintroduced.

Locked in constant-resolver.test.ts (A+A, reused separator, shared-base
diamond); the pre-existing real-cycle and depth-cap cases still return null.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): make module-constant binding writes mutually exclusive (#2391)

`extractPythonModuleConstants` kept `literals`, `exprs`, and `imports` as three
independent maps: `setName` cleared literals+exprs but never `imports`, and an
import never cleared a prior literal/expr. Since `foldName` checks
literals > exprs > imports regardless of source order, a name that was both
imported and locally (re)assigned kept both bindings and the wrong one won —
`from .c import ROUTE; ROUTE = os.getenv(...)` resolved the STALE import instead
of dropping, a confidently wrong route path (the exact skip-floor invariant
this feature is meant to uphold).

Treat the three maps as one logical namespace: any write to one clears the
other two for that name (via `imports.delete` in `setName` and a `bindImport`
helper), so last-binding-in-source-order wins, matching Python. An import both
imported and dynamically rebound now drops. Folding `+=`/`+` onto an imported
base remains deferred (it drops safely, never a stale value).

Locked in python-const-resolver.test.ts: dynamic-rebind drops, literal-shadows-
import, import-shadows-literal, and `+=`-on-import drops.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): widen the group cost-gate to catch literal-leading concats (#2391)

`NONLITERAL_ROUTE_DECORATOR_RE` required the first decorator argument to START
with an identifier, so a string-literal-leading concat like
`@router.get("/api" + SUFFIX)` never tripped `hasComposedRoute`. When such a
route was the ONLY composed shape in a repo, the group layer left `constantsByFile`
empty and dropped the route, while the ingestion side (which has no gate)
resolved `/api/users` and emitted a Route node — an R4 provider/graph parity break.

Widen the gate to also fire on a string-literal-leading `+`-concat, detected by a
`+` before the closing paren on the decorator line. Gating on the `+` (not merely
a leading quote) keeps a plain literal route `@router.get("/x")` OFF the gate, so a
literal-only repo still pays no parse pass.

Locked in fastapi-composed-provider.test.ts: a sole literal-leading concat now
resolves (parseCalls>0 + provider emitted), plus previously-uncovered
`@app.<verb>(CONST)` EXPR-branch resolution; the literal-only no-parse gate case
still passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): correct package-init and over-deep relative import resolution (#2391)

Two edges in `resolvePythonImport`:

- `from . import X` (empty module after the dots) resolved to a sibling
  `<dir>.py` instead of the package `<dir>/__init__.py`. Resolve the bare-package
  case to `__init__.py`.
- An over-deep relative import (more extra dots than the importing file has
  directory levels) silently clamped `dirOf('')` to `''` and could match an
  unrelated root-level `<name>.py` — a wrong file. Guard with `walk > depth →
  null` so an import that escapes above the repo root drops (skip floor).

Both preserve the exact-match / ambiguity→null behavior for ordinary relative and
absolute imports.

Locked in python-const-resolver.test.ts: `from . import` → `__init__.py` (and
null when absent), and an over-deep import returns null even when the clamped
target file exists.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bound parseConstOperands recursion depth (#2391)

`parseConstOperands` recursed on `binary_operator` children with no depth bound.
A stack overflow is not currently reachable (tree-sitter caps expression nesting
below the JS stack limit, so it throws on a deep `+`-chain before this runs), but
add a depth guard (cap 64, mirroring the fold engine's MAX_RESOLVE_DEPTH) as
defense-in-depth: a pathological chain now floors to null (skip) rather than
relying on tree-sitter's limit. The `depth` parameter defaults to 0, so all
existing callers are unaffected.

Locked in python-const-resolver.test.ts: a 100-term `+` chain yields no binding
(null) instead of throwing; ordinary short chains still fold.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(routes): read each .py once in buildPythonRepoContext (#2391)

The group repo-context builder read every `.py` file from disk twice: once in
the `include_router` cross-file pre-pass and again in the #2391 constant
cost-gate loop — an unconditional 2x read on every Python repo, on every group
extraction. Hoist a single read pass that populates one `pyContents` map (and
computes the composed-route cost gate); both the include_router pre-pass and the
constant-map pass now consume the cached content. Behavior-preserving — a
literal-only repo still does one read and zero parses.

Covered by the existing group unit + integration suites (R4 parity and
include_router prefix joins unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(routes): tidy constant-resolver docs and declaration order (#2391)

Three no-behavior nits from the PR #2393 review:
- Name `conditional_expression` (`x if c else y`) in the `parseConstOperands`
  jsdoc list of shapes that deferred to null.
- Move `NONLITERAL_ROUTE_DECORATOR_RE` above `buildPythonRepoContext`, which
  references it — it read as a forward reference before (runtime-safe, but
  confusing).
- Correct the integration-test comment that called `/v2/api/v1/widgets/get`
  "ingestion-only garnish": the group side emits it too (asserted separately);
  the four paths in that block are the shared-parity set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(routes): fold `X += "…"` onto an imported base constant (#2391)

Previously `from .c import BASE; BASE += "/v1"` dropped (the extractor could not
represent "the imported prior value" as an operand without self-referencing X and
tripping the cycle guard). Preserve the imported prior under a synthetic `$imp$N`
key — `$` can never appear in a Python identifier, so it cannot collide with a
real name — and reference it, so the augmented assignment folds to
`<imported BASE>/v1`. Extractor-only: no change to the `Operand` type, the fold
core, or the cache shape, so no SCHEMA_BUMP. An imported base that is itself
unresolvable still drops (skip floor preserved — never a wrong path).

Locked in python-const-resolver.test.ts: single and chained `+=` fold onto an
imported base; an unresolvable base still drops.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(routes): resolve bare decorator constants via the by-name entry (#2391)

The group `resolveExprArg` hand-built `[{ kind: 'ref', name }]` and called
`resolveOperands` for a bare-constant decorator argument — exactly what the
language-agnostic core's `resolveConstant(file, name, repo)` seam does. Call it
directly for the identifier case. This gives the previously test-only by-name
entry point a real production caller (it is the documented reuse seam for future
JVM/other bindings), drops the synthetic operand construction, and lets the now-
unused `Operand` type import go. Behavior-identical — the `+`-concat path still
parses to an operand list and folds via `resolveOperands`.

Guarded by the existing group provider suite (bare-constant and concat cases).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(routes): parse each .py once in buildPythonRepoContext (#2391)

The repo-context builder ran two parse loops — the include_router prefix pre-pass
and the #2391 constant-map pass — so an include_router file in a composed repo was
tree-sitter-parsed twice. Merge them into a single pass that parses each `.py` at
most once and feeds both extractions from the same tree; a file that needs neither
pass is still not parsed at all (cost gates unchanged). Complements the earlier
single-read-pass change (this is the single-parse counterpart).

Behavior-preserving (prefixes, R4 parity, and cost gates verified by the group +
integration suites). Locked with a parseCalls assertion: a file needing both
passes is parsed once, not twice.

Note: a cross-run (cross-process) constant-map cache — the other deferred perf
idea — remains out of scope; it needs disk persistence + invalidation and would
add hashing/IO cost on the common path, so it fails the minimal-change bar this
single-parse dedup meets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bound constant-fold work and output to prevent OOM (#2391)

The `finally`-popped cycle guard (recursion-stack semantics) correctly folds
diamonds/repeated refs, but popping the guard removed the accidental work cap the
old seen-ever set provided: a wide shared-descendant DAG re-folds each child once
per reference, and a self-multiplying concat (`X = A + A; A = B + B; …`) builds a
genuinely exponential string. Reviewers reproduced ~16.8M folds escalating to
`RangeError: Invalid string length` and heap OOM — and neither fold call site is
wrapped in try/catch, so it crashed the whole phase rather than dropping the route.

Two complementary bounds, both flooring to null (skip), never a wrong value:
- a never-popped `memo` in `foldName` caps recomputation at O(nodes) (successes
  only — a null may be transient on a cyclic branch);
- a `MAX_FOLD_LENGTH` (8192) cap in `foldExpr` drops a fold whose output grows
  past any real route path, bounding the string size the depth cap does not.

Corrects the prior "≤ 2^8 folds" comment (output grows multiplicatively, not
additively). Locked with a 64^4-fanout construction that now drops in ~ms instead
of OOMing; diamonds/cycles/depth-cap behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): snapshot assignment RHS refs at the assignment line (#2391)

`ROUTE = BASE` was stored as a lazy `ref(BASE)`, resolved against BASE's FINAL
binding. So `ROUTE = BASE; BASE += "/v1"` (or `ROUTE = API; API = "/other"`)
resolved ROUTE to the MUTATED value — a confidently wrong path, since Python
assigns by value at the `ROUTE =` line. This was latent for local constants at
the base of this feature and the `+=`-on-import work extended it to imports.

Snapshot each assignment/`+=` RHS reference to a bound name into that name's
current frozen value at the assignment line (`freeze`/`snapshot`): a literal
value, a copy of the current expr (whose refs are already frozen), or an import
preserved under a `$imp$N` alias. Unbound refs (forward references) stay lazy.
A later rebind of the aliased name can no longer change the earlier binding.
`freeze` also unifies the previous `currentOps` + inline import-alias logic.

Locked in python-const-resolver.test.ts: aliased-import-then-`+=`,
aliased-local-then-`+=`, aliased-local-then-rebind all resolve to the pre-mutation
value; normal reference chains still fold.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): fold group identifier args via resolveOperands for parity (#2391)

Resolving a bare-constant decorator arg through `resolveConstant` entered
`foldName` at depth 0, whereas the ingestion side folds `routePathOperands`
through `resolveOperands([{ref}])`, entering at depth 1. At the MAX_RESOLVE_DEPTH
boundary the group tolerated one more hop than ingestion, so a deep alias/re-export
chain resolved in the group provider set but dropped from the graph Route nodes —
an R4 parity break. Restore the operand-list path in the group so both subsystems
share identical fold-entry depth. (`resolveConstant` reverts to the documented
agnostic-core seam.)

Locked in constant-resolver.test.ts: a 4-hop chain that `resolveOperands([ref])`
drops but `resolveConstant` resolves, documenting why the group must use the
operand-list entry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): match multiline literal-leading concats in the cost gate (#2391)

`NONLITERAL_ROUTE_DECORATOR_RE` used `[^)\n]*` so it only saw a literal-leading
`+`-concat when the `+` was on the same line as the opening quote. A
Black-formatted `@router.get(\n "/api"\n + SUFFIX\n)` therefore failed the gate,
and when it was the only composed route in a repo the group dropped it while
ingestion (which parses the tree, not the raw line) resolved it — an R4 parity
break. Drop the `\n` exclusion: `[^)]*` spans the wrapped argument but stays
bounded by the decorator's own closing paren, so a plain literal route still
never trips the gate.

Locked in fastapi-composed-provider.test.ts with a multiline concat fixture.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bump SCHEMA_BUMP for changed extractor output + E2E snapshot lock (#2391)

`extractPythonModuleConstants` now emits DIFFERENT `moduleConstants` for the same
source (binding mutual-exclusivity clears stale imports; RHS refs are snapshotted;
`$imp$N` aliases). That output is cached verbatim in the parse cache, so a warm
shard built at the pre-fix version would replay stale — in one case actively
wrong — folded values, and the correctness fixes would silently no-op on upgrade.
Bump SCHEMA_BUMP 11→12 to force re-extraction (same warm-cache-replay class the
original 10→11 bump addressed for the field addition).

Also adds the first end-to-end coverage for the new behavior through the real
ingestion pipeline: app/snapshot.py aliases a constant (`SNAP = API_V1`) then
mutates the source (`API_V1 += "/mutated"`), and the test asserts the Route node
is `/api/v1`, never `/api/v1/mutated` — a case the pure-function unit tests
covered but the pipeline did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-07 13:23:05 +01:00
henry201605
d7ff76e6e9
fix(ingestion/routes): resolve Spring interface-inherited routes (#2288) (#2290) 2026-06-25 09:22:20 +01:00
Gergő Magyar
3d30b94c46
fix(parse): survive non-cloneable worker results so large-repo analyze doesn't crash (#2112) (#2135)
* fix(parse): survive non-cloneable worker results so large-repo analyze doesn't crash (#2112)

A parse worker delivers its accumulated result to the main thread via
postMessage, which structured-clones the payload synchronously on the
worker thread and throws a DataCloneError on the first value it can't
serialize. The reporter's case was a node `properties` value pointing at
a native `toString`. The worker re-posted the throw as {type:'error'},
the pool counted it as a worker death, and under
GITNEXUS_WORKER_POOL_SIZE=1 the same graph re-threw on every respawn
until the slot's budget was exhausted and the whole parse phase aborted
-- defeating even the conservative single-worker workaround.

Add a clone-safety net at the worker result boundary. On a clone failure
the worker isolates the offending file, strips the non-cloneable value
from a plain extraction record (keeping the record -- strictly-missing
data, never wrong) or drops a whole ParsedFile so scope-resolution
re-derives it on the main thread with intact edge data, records the
affected paths on the result, warns naming the field + file so the leak
is diagnosable, and re-posts. Healthy runs are byte-identical: the net
runs only after a real DataCloneError, so there is zero overhead on the
fast path. Skipped paths surface via the parsing processor alongside the
skipped-language telemetry. The strip drops the same values the store
path's JSON.stringify already silently removes, so store/no-store runs
converge.

Scope: PR-1 -- failure mode C, the deterministic POOL_SIZE=1 killer. The
timeout/native-abort graceful-degradation cascade (failure modes A & B)
is coupled to downstream-exclusion + a hard worker watchdog and is
tracked as follow-up work.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): fail-closed clone-safety recovery + bound recursion depth (#2135 review)

The clone-safety recovery path could re-arm the #2112 worker-death cascade it
was built to prevent: in postResultCloneSafe the sanitizer call and the re-post
sat outside the try/catch, and containsNonCloneable/stripNonCloneable recursed
with a cycle guard but no depth bound. A throw inside the sanitizer (a RangeError
from a deeply-nested record, reproduced at depth >=3000) escaped to the message
handler's {type:'error'}, which under GITNEXUS_WORKER_POOL_SIZE=1 is the
respawn-budget-exhaustion abort.

Wrap the sanitizer + re-post in their own try/catch so any throw fails closed to
a primitive-only {type:'error'} deliberately, and thread a MAX_CLONE_DEPTH bound
through both scan/strip functions so an over-deep subtree is treated as
non-cloneable (dropped/undefined) instead of overflowing the stack. The
isStructuredCloneable catch-all is left broad on purpose — it bounds
structuredClone's own internal recursion in the non-plain-object probe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): harden clone-safety against throwing getters and detached buffers (#2135 review)

Two sanitizer-defeat vectors let the re-post throw a DataCloneError again:

- A throwing getter on a record: containsNonCloneable/stripNonCloneable read
  obj[key], so a getter that throws escaped the scan/strip pass. Read defensively
  — a throwing property read is treated as non-cloneable (scan returns true,
  strip drops the property).
- A detached ArrayBuffer/TypedArray: both passed buffers/views through
  unconditionally, but structuredClone rejects a detached one, so the re-post
  threw. Route buffers/views through the authoritative isStructuredCloneable
  probe instead. No byteLength heuristic — a legitimately empty new Uint8Array(0)
  also has byteLength 0 yet clones fine, so a length check would false-positive.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): memoize stripped copies so DAG-aliased records aren't over-dropped (#2135 review)

stripNonCloneable carried a shared `seen` WeakSet and returned the ORIGINAL
(un-stripped) value on revisit. When a non-cloneable was reachable via two paths
(a DAG), the second path spliced the original function-bearing object back into
the output, so the rebuilt element failed the last-resort isStructuredCloneable
guard and the whole record was dropped as "unsalvageable" — contradicting the
"record kept, value stripped" contract.

Replace the WeakSet with a Map<object, stripped-copy>: allocate the empty copy,
memoize it before recursing into children (so cycles return the in-progress
copy), and return the memoized copy on revisit. DAG-aliased subtrees now collapse
to one shared stripped copy and are kept-and-stripped, not dropped. The array
branch moves from .map() to allocate-then-push so its identity can be
pre-inserted. Object Map/Set keys aren't identity-preserved across stripping —
acceptable because parse-result Maps are primitive-keyed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(parse): single-pass clone-safety scan preserving array identity (#2135 review)

makeWorkerResultCloneSafe scanned each dirty array twice — a field-level
whole-array containsNonCloneable probe, then a per-element pass — and always
reassigned the field. Fold into one per-element pass that builds the output
array lazily (copying the clean prefix only once the first dirty element
appears) and reassigns the field only when something changed. A fully-clean
array is now scanned once and keeps its referential identity; the clean prefix
of a dirty array is copied by reference. Behavior is otherwise identical
(failure-path-only code).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(parse): drop unused generic + pin clone-safe field names to keyof (#2135 review)

makeWorkerResultCloneSafe carried a generic `<T extends Record<string,unknown>>`
that was never load-bearing (it mutates in place and returns {skipped}), and the
call site passed untyped string-literal option sets — so renaming `parsedFiles`
or `skippedPaths` would silently disable the drop-whole / skip protection.

Drop the generic (plain `Record<string,unknown>` param) and type the option sets
at the call site as `Set<keyof ParseWorkerResult>`, so a field rename is now a
compile error. The `as unknown as Record<string,unknown>` widening stays — it's
the standard cast for a no-index-signature interface (TS rejects a single-step
`as`); the function genuinely operates structurally on the result's arrays.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): keep the per-file reason in the clone-safety skip log (#2135 review)

The processor's skipped-file warning logged only the paths, dropping the
per-file reason the worker already attached — losing the distinction between a
recoverable "stripped N value(s)" and a whole-record "dropped" entry. Format each
entry as `path (reason)` so the aggregate line carries the diagnostic detail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): deterministic findFilePath attribution for ParsedNode (#2135 review)

findFilePath swept all child objects one level deep in Object.keys order, so a
ParsedNode could be attributed to a sibling child's path-like key instead of its
real path at properties.filePath. Check the known `properties` child first, then
fall back to the generic sweep, so node attribution is deterministic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): zero skippedPaths in the slim cache result (#2135 review)

slimParseWorkerResultsForCache spread the worker result without clearing the
clone-safety skippedPaths telemetry, so a sanitized result persisted its skip
list into the on-disk parse-cache shard. Replay already ignores the field; zero
it (like calls/assignments/parsedFiles) to keep shards lean and the intent
explicit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(parse): exercise real postResultCloneSafe wiring + tighten RED control (#2135 review)

The integration GREEN worker re-implemented postResultCloneSafe inline, so the
production wiring (the {type:'warning'} post + the skippedPaths append) had no
coverage, and the RED control asserted a bare .rejects.toThrow() that any
failure would satisfy.

Extract postResultCloneSafe into a side-effect-free module (post-result.ts) —
importing it from the parse-worker entry module would construct the parser, post
ready, and attach the real handler — and have the GREEN test worker import and
call the real one. Tighten the RED matcher to the actual abort contract
(/circuit breaker|consecutive failures|respawn budget|could not be cloned/),
which also documents that the raw poison result aborts via the pool's
consecutive-failure circuit breaker under POOL_SIZE=1.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): recover the clone-safety net from any post failure, not only DataCloneError (#2135 review)

The V8 structured-clone research surfaced the net's one real correctness hole:
structuredClone invokes getters, and a getter that THROWS surfaces its own error
(a RangeError, etc.) — NOT a DataCloneError (confirmed against a real
MessageChannel). postResultCloneSafe gated recovery on isDataCloneError, so such
a throw re-threw past the sanitizer and re-armed, under POOL_SIZE=1, the
worker-death cascade the net exists to prevent.

Attempt the sanitize + re-post recovery for ANY first-post failure (the sanitizer
already reads properties defensively, so a throwing getter is dropped), falling
closed to a primitive-only {type:'error'} only if the re-post still fails. Adds
an integration case: a node with a throwing getter is recovered and delivered,
not re-thrown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(parse): name the exact offending key path in the clone-skip diagnostic (#2135 review)

The clone-safety net's skip reason named only the array field + file ("stripped
1 value from nodes"), not the offending property key — which is precisely why
the original #2112 leak stayed unpinned. Thread a dotted key path through
stripNonCloneable (recording each stripped value's path: properties.toString,
meta.data[3], …) and surface the first few in the reason ("from nodes:
properties.toString"). Now a single log line — or the contract/strict checks —
names the leaking property, so a residual runtime escape can be fixed at source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(parse): clone contract — a representative ParseWorkerResult is structured-cloneable (#2135 review)

Shape-regression guard: builds a representative ParseWorkerResult (typed as the
real interface) and asserts isStructuredCloneable. Typing it as ParseWorkerResult
makes adding a new boundary field a compile error here until the test is updated,
and the runtime assert catches a field whose type regresses to a non-cloneable
shape — independent of language input.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(parse): strict-mode clone gate (GITNEXUS_STRICT_CLONE) — fail loudly instead of silent sanitize (#2135 review)

The runtime net's silent recovery in production is exactly what let the original
#2112 leak stay unpinned. Add an opt-in strict mode (GITNEXUS_STRICT_CLONE=1,
inherited by workers): on a clone failure, postResultCloneSafe THROWS with the
exact offending key path instead of sanitizing + delivering, so a leak
introduced by a future provider/extractor change fails loudly at its origin
(CI/dev) rather than being quietly stripped. Off in production, where the net
keeps the run alive.

Adds a self-contained integration case (sets the flag, asserts the poison run
rejects with the key path) and skips the synthetic-poison suite under a global
strict run (its value there is running the REAL-extractor integration tests
under strict). Wiring a strict CI lane (GITNEXUS_STRICT_CLONE=1 on a vitest
integration step) is left to the maintainer — it needs a green full-suite
verification and touches the protected workflow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(server): don't ship pipelineResult across the analyze-worker IPC boundary (#2112)

The forked analyze worker reports completion to the parent over
child_process IPC, which uses Node's DEFAULT 'json' serialization
(api.ts forks with no `serialization:` option). `AnalyzeResult.pipelineResult`
is populated on every successful analysis and carries `pipelineResult.graph`
— the live KnowledgeGraph closure object. Sending the raw result is wrong
three ways: (1) the graph's nodes/relationships getters force-materialize
the entire graph into two arrays and JSON-stringify them on every analyze,
discarded immediately (a multi-hundred-MB no-op on a large repo — the #2112
scenario); (2) the graph's methods are own function properties that JSON
drops silently, so a surviving graph is a husk whose forEachNode() throws far
from the cause; (3) a BigInt/circular value anywhere in the payload makes
process.send throw TypeError synchronously — caught and re-sent as
{type:'error'}, mis-reporting a SUCCESSFUL analysis (DB already written) as a
FAILURE. This is the #2112 failure family on the server path, and unlike the
parse-worker result boundary it has no clone-safety net.

The parent (api.ts) reads only result.repoName; pipelineResult's real
consumers (CLI skill generation, cli/analyze.ts) call runFullAnalysis
in-process and never cross this fork. So project the result down to an
explicit JSON-safe allowlist of scalar fields. Typed as
Omit<AnalyzeResult,'pipelineResult'> so a future non-serializable field added
to AnalyzeResult fails to compile until handled here deliberately.

Found by the #2112 cross-process serialization-boundary audit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(ingestion): Cloneable<T> + assertCloneable() compile-time clone-boundary guard (#2143)

The runtime clone-safety net is the production backstop; this is its
compile-time complement. The worker result is plain data except a few
`unknown`-typed sinks (a node's `properties` bag, the provider
`extractTemplateConstraints` / `collectCaptureSideChannel` hook returns) —
`unknown` lets a non-serializable value (a function, a leaked tree-sitter
SyntaxNode, …) cross the structured-clone boundary with no compile-time
guard. That is the structural hole #2112 leaked through.

`Cloneable<T>` is a homomorphic recursive mapped type that maps a function or
symbol member to `never`, so a struct carrying one is no longer assignable to
its own `Cloneable<T>`. `assertCloneable(value)` is a runtime identity (zero
cost) whose parameter is `T extends Cloneable<T> ? T : Cloneable<T>`, so a
clone-unsafe argument fails to compile, naming the offending key.

Because it is a homomorphic mapped type it preserves `interface` shapes and
`readonly` modifiers and needs NO index signature on the payload types — this
sidesteps the "closed interface is not assignable to a recursive
index-signature type" wall that blocked the original value-typed-`Cloneable`
attempt (the reason #2143 was deferred from PR #2135). The conditional
parameter type avoids the `T extends Cloneable<T>` circular-constraint error.

Tests: runtime identity contract, plus type-level @ts-expect-error assertions
(enforced by tsconfig.test.json) that a function/symbol member is rejected and
clean interface payloads are accepted. Applied to the real provider hooks in
the next commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(ingestion): guard provider clone-boundary hooks with assertCloneable (#2143)

Apply the compile-time guard to the provider hooks that feed the `unknown`-typed
worker-result sinks, so a future non-serializable value in their payloads is a
compile error at the source site rather than a runtime DataCloneError at the
worker post:

- C++  extractTemplateConstraints  (CppConstraintPayload)
- C++  collectCaptureSideChannel    (CppCaptureSideChannel)
- C    collectCaptureSideChannel    (CCaptureSideChannel)
- Kotlin collectCaptureSideChannel  (KotlinCaptureSideChannel)

The C++ template-constraint adapter previously returned `unknown`; it now
returns the concrete `CppConstraintPayload | undefined` and routes its payload
through `assertCloneable`. The side-channel hooks are wrapped at their provider
wiring sites. `assertCloneable` is a runtime identity, so behavior is unchanged
(C static-linkage + C++ constraint suites stay green); the guarantee is the
type-check — src tsc now proves every nested member of those real payload trees
is structured-clone safe.

Test: type-level assertions (enforced by tsconfig.test.json) that each concrete
payload type is `Cloneable<T>`, INDEPENDENT of the provider wiring — so the
regression is caught even if the assertCloneable wrapper is later removed.
Proven non-vacuous (a function-bearing type fails the same assertion).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): scan an array's non-index own properties in the clone sanitizer (#2135 review)

structuredClone serializes an array's NON-index own-enumerable properties (e.g.
`arr.meta = fn`) and throws DataCloneError on a non-cloneable one. The clone
sanitizer's array branches iterated numeric indices only, so such an array was
waved through (containsNonCloneable returned false, makeWorkerResultCloneSafe
left the field unrewritten with skipped:[]) — the re-post then threw, fell
through to the fail-closed {type:'error'}, and re-armed the POOL_SIZE=1 cascade
the net exists to prevent.

Add isArrayIndexKey() and, in BOTH containsNonCloneable and stripNonCloneable
array branches (kept in lockstep), scan/strip the non-index own-enumerable keys
after the index loop. A cloneable non-index prop is carried onto the stripped
copy; a non-cloneable one is stripped and recorded. Not reachable from current
parse output (no extractor attaches non-index array props) — a defense-in-depth
hole closed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): contain a throw inside the clone sanitizer instead of escaping to fail-closed (#2135 review)

findFilePath was documented "never throws" but read element properties
unguarded in its generic sweep — a throwing getter at a non-path key (or a
Proxy with a throwing ownKeys trap) threw out of makeWorkerResultCloneSafe, past
postResultCloneSafe's recovery, to the fail-closed {type:'error'} that under
POOL_SIZE=1 re-arms the cascade the net prevents. Likewise a Proxy with a
throwing getPrototypeOf trap throws inside containsNonCloneable's instanceof
checks.

- findFilePath/pathFromChild now read via safeGet (try/catch) and guard
  Object.keys, honoring the "never throws" contract.
- Each element's sanitize in makeWorkerResultCloneSafe is wrapped: a throw during
  scan/strip drops that one element (recorded as "sanitizer error") rather than
  sinking the whole result — so one pathological element can't fail-close the run.
- Corrected the makeWorkerResultCloneSafe JSDoc ("ONLY after a DataCloneError" →
  after ANY post failure, matching the caller) and documented the deliberate
  failure-path double-traversal (the non-allocating pre-scan is what preserves
  clean-element referential identity).

Tests: a throwing getter on a path-less element is stripped & delivered (not
escaped); a Proxy structural-trap element is dropped, clean siblings survive.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(parse): add a final cloneable postcondition gate to the clone sanitizer (#2135 review)

makeWorkerResultCloneSafe rewrote only ARRAY result fields, so a future
non-array sink (a nested object / Map result field) carrying a non-cloneable
value — or an array field whose own non-index property the element loop didn't
reach — would survive the sanitizer and throw on the re-post. Add a final
`if (!isStructuredCloneable(result))` gate that strips any remaining offending
field in place, making "the returned result is structured-cloneable" a hard
postcondition independent of future ParseWorkerResult shape. Failure-path-only
and a no-op once the array loop already made the result clean (the per-field
probe short-circuits every clean field, so it adds no work or skip entries then).

Tests: a function on a non-array result field is stripped & the result becomes
cloneable; the gate adds no skip entry when the array loop already cleaned up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(parse): reject an `any`-typed member in the Cloneable<T> compile-time guard (#2135 review)

`Cloneable<any>` previously resolved to `any` (not `never`), so a payload with
an `any`-typed member — the most likely escape hatch, since `unknown` is already
blocked — passed `assertCloneable` with no compile error. Add an `IsAny<T>`
branch (the canonical `0 extends 1 & T` probe) as the FIRST arm so `any` resolves
to `never`, matching how `unknown` is already rejected. It must precede the
primitive arm: `any extends CloneablePrimitive` would otherwise resolve to `any`
and re-admit it.

The IsAny-first arm perturbs inference for a bare `undefined` literal argument
(T infers as `unknown` → never); real consumers pass `X | undefined` unions
(the provider hooks), which are unaffected (src tsc clean), so the runtime
identity test now uses a `string | undefined` value — the realistic shape.

Tests: an `any` member fails `assertCloneable` (@ts-expect-error, enforced by
tsconfig.test.json) and `Cloneable<any>` resolves to `never` at the type level.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(server): type the analyze-worker IPC projection as a Pick allowlist, not Omit (#2135 review)

`AnalyzeResultIpc = Omit<AnalyzeResult,'pipelineResult'>` kept every other field
in the type — including optional ones like `isPrimaryBranch?` — so the type
advertised a field the runtime allowlist never sends, and the doc-comment's
"a future field fails to compile until handled here" only held for REQUIRED
fields. Switch to `Pick<AnalyzeResult, …the six scalar fields…>`: the allowlist
IS the type, so the projection return literal is exhaustive by construction
(omitting a key is a compile error) and a new `AnalyzeResult` field is simply
absent from the wire until deliberately added here. `isPrimaryBranch` is
intentionally excluded (nothing consumes it server-side over this fork; the
parent reads only `repoName`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(parse): remove the now-dead isDataCloneError export (#2135 review)

postResultCloneSafe recovers on ANY fast-path post failure and never inspects
the error type (a throwing getter surfaces a RangeError, not a DataCloneError —
gating on the type was the original net-gap bug). isDataCloneError has no
production caller; it was only exercised by its own unit test. Remove the
function and that test block.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(parse): use the exported SkippedPath type in parsing-processor (#2135 review)

The clone-safety telemetry accumulator inlined `Array<{path,reason}>` — a
structural duplicate of the exported `SkippedPath`. Import and use the canonical
type so a future rename of its fields is a compile error here instead of a silent
structural drift.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(parse): document the cloneable-return contract on the worker-boundary hooks (#2135 review)

extractTemplateConstraints and collectCaptureSideChannel return `unknown` and
feed values across the worker structured-clone boundary, but the hook contracts
didn't state the cloneability requirement — a future language implementing them
without care could leak a non-serializable value. Document that the return MUST
be structured-clone-safe and should be wrapped with assertCloneable, so the
guarantee is a compile error at the source (#2143).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(parse): assert the clone-skip telemetry surfaces in the GREEN integration case (#2135 review)

The GREEN clone-safety integration test asserted only graph content (all files
present), not that the skippedPaths / {type:'warning'} wiring its docstring
claims to cover actually fired. Capture the production logger via _captureLogger
and assert the sanitize telemetry names the offending file (poison.ts) AND the
exact stripped key path (properties.toString) — proving the worker's
skippedPaths append + the parsing-processor warning surfaced end to end.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(server): cover the IPC projection against a real KnowledgeGraph (#2135 review)

The IPC projection tests used a hand-built hostile object. Add a case that puts
a real createKnowledgeGraph (whose nodes/relationships getters would materialize
the whole graph under JSON.stringify) in pipelineResult and asserts the
projection drops it entirely — the serialized payload stays under 300 bytes
(a materialized 50-node graph would be thousands), with the scalar fields intact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(parse): cover the unsalvageable-drop branch and the skippedPaths merge union (#2135 review)

Two untested clone-safety branches from the tri-review:

- "dropped unsalvageable": a dirty element whose stripped copy is STILL not
  structured-cloneable must be dropped, not delivered (else the re-post throws).
  Add a deterministic test (a non-plain member with a stateful getter that the
  strip-time probe sees clean but that turns into a function on the post-strip
  verification) asserting the element is dropped and the run survives.

- mergeResult skippedPaths union across sub-batches. mergeResult (and its
  appendAll helper) was module-private in the parse-worker ENTRY module, which a
  main-thread test can't import (it runs MessagePort setup). Extract it to a
  side-effect-free result-merge.ts (mirroring post-result.ts) and unit-test the
  union (including the `??=` target-init path), the skippedLanguages sum, and
  array append. parse-worker imports it back; verified the built worker still
  parses + merges via the real-worker integration path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(parse): root-prettier format the clone-safety review-fix files (#2135 review)

Clears the failing `quality / format` CI gate (root prettier, not the
gitnexus-local config). Reformats the pre-existing #2143 wrapping lines in
c-cpp.ts + kotlin.ts plus the clone-safety review-fix files touched in this
PR-update (clone-safety.ts and the new/updated tests). Formatting-only — no
behavior change; tsc, the type-level assertions (tsconfig.test.json), and the
unit + integration suites stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(parse): avoid js/trivial-conditional in the type-level clone assertions (#2135 review)

CodeQL flagged the `expect(a && b && c).toBe(true)` lines in the type-level test
assertions as js/trivial-conditional: after type erasure the operands are
constant `true`, so the `&&` chain always evaluates the same. Replace the `&&`
chain with array equality (`expect([...]).toEqual([true, ...])`) — no
conditional, and the real assertions remain the `const x: …IsNever = true` /
`: IsCloneable<…> = true` annotations (enforced by tsconfig.test.json, which
fail to compile if a guard regresses).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-10 13:47:22 +01:00