GitNexus/gitnexus/test/integration/parse-impl-clone-skip.test.ts
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

291 lines
12 KiB
TypeScript

/**
* #2112 — Integration regression test for the worker result clone-safety net.
*
* Reproduces the deterministic large-repo killer: a parse worker whose
* accumulated result carries a value the structured-clone algorithm can't
* serialize (the reporter's case was a node `properties` value pointing at a
* native `toString`). Before the fix, `parentPort.postMessage({type:'result',
* data})` threw a `DataCloneError` SENDER-side; the worker re-posted it 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.
*
* Runs with REAL `worker_threads` + `createWorkerPool` over the production
* pool / merge / graph wiring, under `workerPoolSize: 1` (matching the
* conservative workaround that still failed in the issue). The GREEN worker is
* an ESM module that statically imports and calls the REAL built
* `postResultCloneSafe` from `dist/` — so this exercises the actual production
* delivery wiring across a real `postMessage` boundary (the fake-worker doubles
* used by the unit suite bypass structured clone entirely and can't reproduce
* the failure).
*
* Build prerequisite: the worker imports `dist/.../post-result.js`, so
* `node scripts/build.js` must run first (the `pretest:integration` step does
* this; a stale `dist/` would test old behavior).
*/
import { describe, it, expect, afterEach, beforeEach } from 'vitest';
import { tmpdir } from 'node:os';
import { mkdtempSync, writeFileSync, mkdirSync, rmSync, statSync } from 'node:fs';
import path from 'node:path';
import { pathToFileURL } from 'node:url';
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
import { _captureLogger } from '../../src/core/logger.js';
// file:// URL of the BUILT production result-delivery helper, imported by the
// ESM test worker so it exercises the REAL postResultCloneSafe wiring (the
// {type:'warning'} post + skippedPaths append), not a re-implementation.
const POST_RESULT_URL = new URL('../../dist/core/ingestion/workers/post-result.js', import.meta.url)
.href;
const ACCUMULATED_INIT = `{
nodes: [], relationships: [], symbols: [], calls: [], assignments: [],
routes: [], fetchCalls: [], fetchWrapperDefs: [], decoratorRoutes: [],
routerIncludes: [], routerImports: [], toolDefs: [], ormQueries: [],
constructorBindings: [], fileScopeBindings: [], parsedFiles: [],
skippedLanguages: {}, fileCount: 0,
}`;
/**
* Synthesizes a Function node per file. For `poison.ts` it leaks an own native
* `toString` into the node's `properties` — the exact #2112 shape that throws
* `DataCloneError` across the real worker boundary.
*/
const SUB_BATCH_HANDLER = `
if (msg && msg.type === 'sub-batch') {
for (const file of msg.files) {
const baseName = file.path.split('/').pop().replace(/\\.ts$/, '');
const properties = {
name: baseName, filePath: file.path, startLine: 1, endLine: 1,
language: 'typescript', isExported: true,
};
if (file.path.endsWith('poison.ts')) {
properties.toString = Object.prototype.toString; // native fn → non-cloneable
}
accumulated.nodes.push({ id: 'func:' + file.path, label: 'Function', properties });
accumulated.fileCount++;
}
parentPort.postMessage({ type: 'progress', filesProcessed: accumulated.fileCount });
parentPort.postMessage({ type: 'sub-batch-done' });
return;
}`;
/**
* GREEN worker: delivers via the REAL production postResultCloneSafe, so this
* test covers the actual wiring (the {type:'warning'} post + skippedPaths
* append), not a re-implementation that could drift from production.
*/
const CLONE_SAFE_WORKER = `
import { parentPort } from 'node:worker_threads';
import { postResultCloneSafe } from '${POST_RESULT_URL}';
const accumulated = ${ACCUMULATED_INIT};
parentPort.postMessage({ type: 'ready' });
parentPort.on('message', (msg) => {
${SUB_BATCH_HANDLER}
if (msg && msg.type === 'flush') {
postResultCloneSafe(accumulated);
}
});
`;
/** RED control: posts the non-cloneable result raw (no clone-safety net). */
const RAW_WORKER = `
import { parentPort } from 'node:worker_threads';
const accumulated = ${ACCUMULATED_INIT};
parentPort.postMessage({ type: 'ready' });
parentPort.on('message', (msg) => {
${SUB_BATCH_HANDLER}
if (msg && msg.type === 'flush') {
parentPort.postMessage({ type: 'result', data: accumulated });
}
});
`;
/**
* GETTER worker: poison.ts's node carries an own-enumerable getter that THROWS.
* structuredClone invokes getters, so this surfaces a RangeError — NOT a
* DataCloneError — at the boundary. The net must still recover (route it into
* the sanitizer), not re-throw past it. Delivers via the real postResultCloneSafe.
*/
const GETTER_WORKER = `
import { parentPort } from 'node:worker_threads';
import { postResultCloneSafe } from '${POST_RESULT_URL}';
const accumulated = ${ACCUMULATED_INIT};
parentPort.postMessage({ type: 'ready' });
parentPort.on('message', (msg) => {
if (msg && msg.type === 'sub-batch') {
for (const file of msg.files) {
const baseName = file.path.split('/').pop().replace(/\\.ts$/, '');
const properties = {
name: baseName, filePath: file.path, startLine: 1, endLine: 1,
language: 'typescript', isExported: true,
};
if (file.path.endsWith('poison.ts')) {
Object.defineProperty(properties, 'boom', {
enumerable: true,
get() { throw new RangeError('boom getter'); },
});
}
accumulated.nodes.push({ id: 'func:' + file.path, label: 'Function', properties });
accumulated.fileCount++;
}
parentPort.postMessage({ type: 'progress', filesProcessed: accumulated.fileCount });
parentPort.postMessage({ type: 'sub-batch-done' });
return;
}
if (msg && msg.type === 'flush') {
postResultCloneSafe(accumulated);
}
});
`;
const FIXTURE_FILES = {
'src/good_a.ts': 'export function good_a() { return 1; }\n',
'src/poison.ts': 'export function poison() { return 2; }\n',
'src/good_c.ts': 'export function good_c() { return 3; }\n',
};
const nodeNames = (graph: ReturnType<typeof createKnowledgeGraph>): Set<string> => {
const names = new Set<string>();
for (const n of graph.nodes.values()) {
if (n.label === 'Function') {
const name = (n.properties as { name?: string }).name;
if (name) names.add(name);
}
}
return names;
};
// These cases deliberately inject non-cloneable values, so they're meaningless
// under a global GITNEXUS_STRICT_CLONE=1 run (strict turns the sanitize into a
// throw). Skip the whole suite there — a global strict lane's value is running
// the REAL-extractor integration tests under strict, not this synthetic one.
// The strict-mode case below sets the flag itself (self-contained).
const STRICT = process.env.GITNEXUS_STRICT_CLONE === '1';
describe.skipIf(STRICT)('#2112: worker result clone-safety integration (POOL_SIZE=1)', () => {
let tempDir: string;
let repoDir: string;
const writeWorker = (script: string): URL => {
const p = path.join(tempDir, `clone-skip-worker-${Math.abs(hash(script))}.mjs`);
writeFileSync(p, script);
return pathToFileURL(p) as URL;
};
// Stable name without Math.random (banned in this harness elsewhere) — index by content.
const hash = (s: string): number => {
let h = 0;
for (let i = 0; i < s.length; i++) h = (h * 31 + s.charCodeAt(i)) | 0;
return h;
};
const runWith = async (workerUrl: URL): Promise<ReturnType<typeof createKnowledgeGraph>> => {
const filePaths = Object.keys(FIXTURE_FILES);
const scanned = filePaths.map((rel) => ({
path: rel,
size: statSync(path.join(repoDir, rel)).size,
}));
const graph = createKnowledgeGraph();
await runChunkedParseAndResolve(
graph,
scanned,
filePaths,
filePaths.length,
repoDir,
1, // deterministic start time (Date.now is banned in this harness)
() => {},
{
skipWorkers: false,
workerUrlForTest: workerUrl,
workerPoolSize: 1, // poison lands on the only slot — the issue's workaround config
},
);
return graph;
};
beforeEach(() => {
tempDir = mkdtempSync(path.join(tmpdir(), 'parse-impl-clone-skip-'));
repoDir = path.join(tempDir, 'repo');
mkdirSync(repoDir, { recursive: true });
for (const [rel, content] of Object.entries(FIXTURE_FILES)) {
const full = path.join(repoDir, rel);
mkdirSync(path.dirname(full), { recursive: true });
writeFileSync(full, content);
}
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
it('GREEN: a non-cloneable result is sanitized and delivered; the run completes with all files', async () => {
// Capture the production telemetry (parsing-processor logs the per-file skip
// with its path + reason) so this asserts the REAL skippedPaths/warning
// wiring surfaced, not just that the graph ended up correct.
const cap = _captureLogger();
try {
const graph = await runWith(writeWorker(CLONE_SAFE_WORKER));
const names = nodeNames(graph);
// Survivors AND the sanitized poison file are all present — the run did not abort.
expect(names.has('good_a')).toBe(true);
expect(names.has('good_c')).toBe(true);
// The poison node is delivered with its legitimate data intact (only the
// leaked native `toString` was stripped), so it still lands in the graph.
expect(names.has('poison')).toBe(true);
// The clone-safety telemetry surfaced the offending file AND the exact
// stripped key path — the wiring this suite claims to cover.
const msgs = cap.records().map((r) => String(r.msg ?? ''));
const skipLine = msgs.find(
(m) => m.includes('poison.ts') && m.includes('properties.toString'),
);
expect(
skipLine,
`expected a sanitize warning naming poison.ts + properties.toString; saw: ${msgs.join(' | ')}`,
).toBeDefined();
} finally {
cap.restore();
}
});
it('GREEN: a throwing getter (RangeError, not DataCloneError) is recovered, not re-thrown', async () => {
// structuredClone invokes getters; a throwing getter surfaces its own
// RangeError at the boundary. The net must route it into the sanitizer
// (which drops the offending property) rather than re-throwing past it and
// re-arming the POOL_SIZE=1 worker-death cascade. Without the fix this run
// rejects; with it, all files (incl. the sanitized poison node) are present.
const graph = await runWith(writeWorker(GETTER_WORKER));
const names = nodeNames(graph);
expect(names.has('good_a')).toBe(true);
expect(names.has('good_c')).toBe(true);
expect(names.has('poison')).toBe(true);
});
it('strict mode (GITNEXUS_STRICT_CLONE=1) surfaces the leak loudly with the key path, not silent sanitize', async () => {
// The spawned worker inherits process.env, so postResultCloneSafe runs in
// strict mode: instead of sanitizing + delivering, it THROWS with the exact
// offending key path → the run rejects (a real future extractor leak would
// fail CI loudly at its origin instead of being silently stripped in prod).
const prev = process.env.GITNEXUS_STRICT_CLONE;
process.env.GITNEXUS_STRICT_CLONE = '1';
try {
await expect(runWith(writeWorker(CLONE_SAFE_WORKER))).rejects.toThrow(
/STRICT_CLONE|not structured-cloneable|properties\.toString/i,
);
} finally {
if (prev === undefined) delete process.env.GITNEXUS_STRICT_CLONE;
else process.env.GITNEXUS_STRICT_CLONE = prev;
}
});
it('RED control: without clone-safety, the same poison result aborts the parse phase', async () => {
// Pre-fix behavior: the raw non-cloneable result throws DataCloneError in
// the worker; under POOL_SIZE=1 the pool exhausts the slot's respawn
// budget and rejects. The matcher is the specific contract — not a bare
// .toThrow() — so an unrelated failure (spawn error, stale dist) can't pass
// it and mask a broken RED→GREEN flip.
await expect(runWith(writeWorker(RAW_WORKER))).rejects.toThrow(
/circuit breaker|consecutive failures|respawn budget|could not be cloned/i,
);
});
});