1680 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
gitnexus-release-bot[bot]	53e464d72b	release: v1.6.10-rc.106	2026-07-25 08:44:02 +00:00
Gergő Magyar	ad1b9227c4	fix: large-repo analyze OOM and false worker-timeout cascade (#2649) (#2679)	2026-07-25 09:16:17 +01:00
Gergő Magyar	2ec00b8952	fix(analyzer): reject cross-drive paths in the identity containment guard (#2688) ... `isInside()` paired its `..` checks with no absolute-path rejection, so on Windows it reported an unrelated drive as *inside* the parent. `path.relative` cannot express a relative path between two drives and returns the absolute target instead: path.win32.relative('C:\\parent\\src', 'D:\\other\\file.js') // 'D:\\other\\file.js' That string does not start with '..', so the guard passed it. Impact, per call site: - resolveInvokedArtifact: adopts `process.argv[1]` as the invoked analyzer artifact whenever it merely sits on another drive. That file is then absent from the validated build, so resolveAnalyzerRunnerIdentity throws — `analyze` and `status` fail outright on a multi-drive Windows install (e.g. a launcher on D: invoking a package installed on C:). This is how the bug surfaced: the GitHub Windows runner keeps the repo on D: and temp fixtures on C:. - cacheDirectory: the "trusted cache directory must be outside the package and build roots" guard wrongly fires for a directory on another drive, rejecting a legitimate configuration. - validateIdentityCache / cachedBuildDigestForPath: a containment check that can answer "inside" for a path on another drive is weaker than intended. Fix: reject an absolute `path.relative` result. This is the idiom the repo's other containment guards already use — server/api.ts, server/git-clone.ts and group/extractors/fs-utils.ts all pair the '..' check with `path.isAbsolute`; this function was the outlier. `pathApi` is injectable (defaulting to the platform-bound `path`) so the win32 semantics are unit-testable from a POSIX runner. The new test is fixture-free and registered on the cross-platform matrix; its cross-drive case fails without the guard and the same-drive/POSIX cases pass either way, proving the fix is narrow. Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>	2026-07-25 08:21:34 +01:00
Gergő Magyar	7316503ebc	perf(analyze): hold structural relationships out of the JS heap, on by default (#2680) (#2685) ... * refactor(lbug): extract SyncCsvWriter into a shared module `PdgEmitSink` (#2202) declared `SyncCsvWriter` as a private, non-exported class. The structural streaming sink for #2680 needs the same buffered sync-write + poison/openFailure IO discipline, and importing it is not possible while it is module-private — so the alternative was copying ~90 lines of it. Extract the class (and the chunk-rows default it uses) into `sync-csv-writer.ts` and have `PdgEmitSink` import it. `DEFAULT_PDG_EMIT_CHUNK_ROWS` stays exported as an alias so no existing caller changes. Pure refactor: no behaviour change. pdg-emit-sink.ts 396 -> 302 lines; tsc clean; the 23 existing #2202 tests pass unchanged. Refs #2680 * feat(lbug): add GraphEmitSink for streaming structural relationship emit Structural sibling of PdgEmitSink (#2202): a KnowledgeGraph façade that routes relationships no mid-pipeline phase reads back to bounded CSV-on-disk and never stores them. Nothing constructs it yet. Measurement drove the design. On a kernel-shaped synthetic graph (400k nodes, 2.7 edges/node): nodes only ...... 367 B/node nodes + edges ... 2075 B/node <- reproduces the #2649 ~2.1 KB/node => the relationship layer is 83% of graph heap, ~646 B/edge so streaming *relationships* is where the memory is; nodes stay resident (they are 17%, and two scope-resolution index builders scan them). Dropping just the redundant relationshipsByType/edgeIdsByNode indexes was also measured — 174 of 648 B/edge, ~1.3x — and is not a substitute. RETAINED_REL_TYPES is derived from an exhaustive audit of every relationship read site under src/, and each entry names its reader. An earlier draft carried 14 types, 5 of which no reachable phase reads. Two deliberate departures from PdgEmitSink, both because its invariants do not hold here: - dedup by relationship id, since no upstream per-file uniqueness guarantee exists for structural edges and COPY would violate the PK; - removeRelationship on an already-streamed id throws instead of no-oping, so a mutating consumer cannot corrupt the graph undetected. Also exposes hasStreamedSemanticEdge for the local-symbol pruner: without it a block-local symbol referenced only by a streamed edge looks unreferenced and gets pruned, leaving a CSV row pointing at a node with no row. Refs #2680 * feat(analyze): stream structural relationships to CSV under GITNEXUS_STREAM_GRAPH_EMIT Wires GraphEmitSink into the pipeline behind a full-rebuild-only flag, so relationships that no mid-pipeline phase reads back never enter the JS heap. Measured ~2.9x reduction of graph heap: 0.17 (nodes) + 0.83 * 0.21 (retained edges) = 0.344 retained. This is a constant factor, NOT O(chunk) — node identity and the resolution registries stay O(repo). The sink is armed at the PARSE boundary, not at graph construction. An exhaustive audit of every relationship read site under src/ found four mid-pipeline CALLS consumers, not the two an earlier draft assumed: - local-symbol-pruner (full iterRelationships scan, then removeNode) - communities / processes (whole-graph forEachRelationship) - mapCobolToGraph, which scans CALLS and REMOVES the unresolved ones — and runs BEFORE parse, so streaming from construction would have silently stopped COBOL cross-program call resolution - taintSummaries, gated on `pdg` and NOT on `skipGraphPhases`, so it needs its own gate or --pdg + this flag yields an empty taint layer Accordingly communities, processes, taintSummaries and callSummaries are all disabled under the flag, and the run logs what it is giving up. Two fixes that are correct independently of the flag: - runPipelineFromRepo keyed its community/process extraction off `!skipGraphPhases` while getPhaseOutput THROWS on a phase filtered out by any enabledWhen predicate — now a presence check, so filtered combinations return undefined instead of crashing. - loadGraphToLbug COPYs one job per CSV FILE rather than per label pair. #2202's throw-on-collision merge is only sound because BasicBlock pairs are disjoint; a streamed CALLS edge is Function|Function and always collides with the whole-graph CSV for that pair, so the structural manifest appends instead. The buffer-pool hint adds the streamed row count back in: the hint only ever shrinks the pool, so sizing it from the post-streaming relationshipCount would starve the COPY at exactly the scale this targets. detect_changes: 18 symbols / 10 files / 9 processes, all within the planned scope. Full suite green with the flag off. Refs #2680 * fix(mcp): stop impact() under-reporting risk on a streamed index An index built with streamed structural emit has no Process or Community rows, and impact()'s risk scorer uses processCount >= 5 and moduleCount >= 5 as two of its four CRITICAL escalation criteria. The missing-table errors are swallowed as benign without raising `partial`, so nothing distinguished 'this repo has no processes' from 'this index was built without them' — the same change would report LOW off a streamed index and CRITICAL off a complete one, with no signal either way. That is the false-clean shape #2283 ruled out for detect_changes, and it matters more here because the repo's own workflow mandates impact() before every symbol edit. Stamp `graphPhases: 'complete' | 'skipped'` into RepoMeta and have impact() attach riskUnderstated + an explanatory riskNote when the index is stamped skipped, so the reported level is explicitly a lower bound. Unlike the rest of RepoMeta.capabilities this stamp has a real programmatic reader. Also documents GITNEXUS_STREAM_GRAPH_EMIT in the README env table, including everything the flag disables. Refs #2680 * test(lbug): differential set-identity gate for streamed structural emit The acceptance property for #2680: for the same node/edge set, the rows reaching the bulk COPY must be identical whether streaming is on or off. With streaming on they arrive from two places — the residual in-memory graph via streamAllCSVsToDisk, plus the sink's per-pair CSVs — so the test asserts their UNION equals the single whole-graph emit. Also asserts the split is real (retained + streamed == total, streamed > 0), so a sink that silently streamed nothing cannot pass the equality vacuously. Verified discriminating: with sink.arm() commented out the test fails ('expected 0 to be greater than 0'); restored, it passes. Fixture spans both sides of RETAINED_REL_TYPES and includes a self-edge and a duplicate relationship id — the cases where a naive sink diverges from the whole-graph emit. Drives the sink directly rather than running analyze, matching pdg-emit-streaming-roundtrip.test.ts: the guarantee is about emitted rows, and the worker pool would add unrelated machinery without strengthening the assertion. Refs #2680 * fix(test): remove literal NUL byte and cover streamGraphEmit phase gating Two review findings, both verified before accepting. 1. The round-trip test contained a literal NUL byte as a key separator, which made Git treat the whole .ts file as BINARY — `git show --numstat` reported `-\t-` for it, so the file would not diff or blame and CI text tooling would skip it. Replaced with the escaped \\u0000 sequence; behaviour is identical, the file is text again. (Found by the Codex swarm lane.) 2. buildPhaseList's four new streamGraphEmit gating predicates and the flag-off default path had no test that would fail on revert — two review lanes flagged this independently. Reversing any enabledWhen condition would have passed the suite silently, which matters because an ungated taintSummaries yields an empty taint layer rather than an error. Added four cases: the streamed run drops communities/processes/ taintSummaries/callSummaries; it keeps mro/di (their reads are all in RETAINED_REL_TYPES); the flag-off list is untouched; and skipGraphPhases still works independently. Refs #2680 * fix(analyze): don't leak a temp dir when streaming is off; correct two overclaims Three review findings, all verified before accepting. 1. `graphEmitCsvDir: resolveNativeSafeStorageDir(...)` was evaluated unconditionally inside the pipeline-options literal. On a Windows non-ASCII storage path that helper mkdtempSyncs a REAL directory, so every analyze leaked one temp dir even with the flag off. Now resolved only when streaming is active, matching how the PDG sibling resolves inside its own guard. This was the only finding affecting flag-off users. 2. The retain-set comment claimed 'the differential round-trip test is what catches drift'. It cannot. addRelationship PARTITIONS edges between the graph and the CSVs, and the union of a partition is invariant under where the partition line falls — so that test stays green no matter how RETAINED_REL_TYPES is drawn. Only the read-site audit protects the invariant, and the comment now says so and names the grep to re-run. 3. The ~2.9x figure assigned streamed edges a retained cost of zero, ignoring the sink's own streamedIds/streamedEndpoints Sets — and relationship ids are plain concatenations of both endpoint ids, not hashes. Review measured those Sets at ~35% of full per-edge retention, not the '~a tenth' assumed, putting the real figure nearer ~1.7-2.2x; a member-dense Java/C# repo lands lower still, since the retained structural spine is a larger share there than in the TypeScript census the 0.21 came from. Code comment and README now give a range and say plainly that no end-to-end measurement on a real repository exists yet. Refs #2680 * fix(mcp): disclose degraded risk in detect_changes; stop pinning the sink Two more review findings, both cross-lane corroborated. 1. detect_changes derives risk_level SOLELY from affected-process count, and a graphPhases:'skipped' index has zero Process rows by construction. The STEP_IN_PROCESS query then succeeds with zero rows, so queryDegraded stays false and the tool returns risk_level 'low', affected_count 0, with no partial marker — for every change, forever. That is a false-clean on the gate this repo mandates before every commit, and it is the same #2283 shape the previous commit fixed in impact() while leaving its sibling untouched. Now carries the same riskUnderstated + riskNote disclosure. 2. PipelineResult.graphEmitSink had zero readers — the pruner predicate and the manifest are both threaded elsewhere — but returning it kept the sink, and therefore its O(streamed-edges) id and endpoint Sets, reachable through the entire COPY/FTS/embedding phase. That is precisely the phase this feature exists to fit inside RAM, so the field actively worked against the change's purpose. Dropped. Refs #2680 * refactor(2680): one named capability, one risk helper, a shorter header Pure cleanup pass — no behaviour change, 66 tests across the six affected suites still green, and the round-trip test still fails when the sink is left un-started. Three things were untidy: 1. The phase layer reached the sink through TWO loose callbacks bolted onto PipelineContext (`armStreaming`, `hasStreamedSemanticEdge`) — two fields, two wiring lines, no name for the thing they belonged to. Replaced by one `graphEmit?: GraphEmitControl`, a two-method interface declared beside the sink. Phases now say what they mean: `ctx.graphEmit?.beginStreaming()`. Also renames `arm()` to `beginStreaming()`, which needs no comment to explain. 2. The degraded-index risk disclosure was copy-pasted into impact() and detect_changes() — two meta probes, two near-identical prose blocks, and two long comments restating the same reasoning. Now one `streamedIndexRiskDisclosure()` helper carrying the explanation once; each caller passes only the clause naming which count is structurally zero for it. Same file, 45 lines in / 45 out, with the duplication gone. 3. The sink's file header had grown into a changelog of my own review corrections ('this once assumed', 'review measured'). A reader does not care what an earlier draft believed. Rewritten to state the design argument once — relationships are ~83% of graph heap, so they are what streams; nodes are the other 17% and are scanned, so they stay — under headings, with the honest 'this is an estimate, ~1.7-2.2x, no real-repo measurement yet' caveat kept in full. Refs #2680 * feat(analyze): make streamed graph emit the default, with nothing traded away Streaming was opt-in because it disabled the four phases that consume the whole CALLS graph — communities, processes, taintSummaries, callSummaries. That made it unshippable as a default: query() is process-grouped and clusters/skill-gen are community-backed, so every index would have silently lost them. The sink now answers a COMPLETE relationship read. It keeps streamed edges as four parallel columns over an interned node table — sourceId, targetId, type, confidence — and iterRelationships/iterRelationshipsByType/ forEachRelationship/relationshipCount return the retained edges concatenated with those. Every consumer therefore sees the whole graph and no phase knows streaming happened. Four fields, not six, because an audit showed community-processor, process-processor, taint-summaries and the pruner read only those — none keys on rel.id. That matters: relationship ids are unique long strings, and retaining them is precisely what made a fully-columnar attempt LOSE to the object graph (measured 838 MB vs 822 MB). Ids stay out of the columns; a read synthesizes one, which is safe because buildRelRow never persists it. Consequently deleted, not merely disabled: - the four enabledWhen gates and the 'what you give up' warning; - the pruner's hasStreamedSemanticEdge predicate and its plumbing — a complete scan sees streamed edges, so the dangling-edge hazard is gone by construction rather than by compensation; - the whole degraded-index apparatus: the graphPhases RepoMeta stamp, streamedIndexRiskDisclosure, and the riskUnderstated markers on impact() and detect_changes(). Nothing degrades, so nothing needs disclosing. Default is ON for full rebuilds; GITNEXUS_STREAM_GRAPH_EMIT=0 (or an explicit option) is the escape hatch, for bisecting a suspected streaming fault rather than routine use. Incremental runs still refuse it — the writeback reads relationships back out of the in-memory graph. Measured A/B, 400k nodes / 1.08M edges, all edges streamable (worst case for this design): 823 MB -> 626 MB, ~1.3x, all 1.08M edges still visible. That is deliberately less than the ~2.9x the retained-share formula implies — losslessness costs the dedup Set and the columns. The earlier, bigger number was bought by disabling phases. README and the file header both state 1.3x measured; neither claims O(chunk). New coverage: reads are complete (proven discriminating — 3 tests fail when the streamed leg is removed), endpoints/confidence survive the round trip, per-type lookup finds streamed types, and every CALLS-consuming phase stays registered under the flag. Refs #2680 * docs(2680): pin the invariants the default-on change relies on Review follow-ups. No behaviour change except the id-uniqueness fix. - pipeline.ts returns the RAW graph, not the sink, and that is load-bearing: phases read the sink so their scans are complete, but loadGraphToLbug feeds this value to streamAllCSVsToDisk, whose iterator would then emit every streamed edge a SECOND time on top of the per-pair CSVs the sink already wrote. Returning the sink there silently doubles every streamed relationship in the persisted graph, so the reason is now written down at the return site. - Synthesized ids now carry the column index, making them unique even when two streamed edges share (type, source, target) and differ only in reason/step. Harmless today because no consumer keys on relationship id, but real ids are unique and the synthesized ones should match, so a future id-keyed consumer cannot silently collapse two edges. - Recorded WHY dropping reason/step is safe, which is not the same argument as for id: the persisted row keeps their true values because buildRelRow receives the original relationship on the way through, so only in-memory reads see the 'streamed' placeholder. The ACCESSES reason:'read'|'write' distinction that MCP queries depend on therefore survives in the database. A future in-pipeline consumer needing either field must add a column rather than trust the placeholder. Also verified while chasing a review lead: removeNodesByFile has no production callers and removeNode has exactly one (the pruner), which reads through the sink and so sees streamed edges. The dangling-edge hazard the deleted hasStreamedSemanticEdge predicate used to compensate for is closed by construction, not by luck. Refs #2680 * fix(2680): fail loudly on a missing CSV dir, and guard the retain set Resolves both findings from the review of this branch. MEDIUM — pipeline.ts silently skipped streaming when `streamGraphEmit` was true but `graphEmitCsvDir` was absent. The CLI always supplies the dir, but streaming is on by DEFAULT now, and the callers that build PipelineOptions themselves (eval-server, MCP daemon, tests) are exactly the ones that would omit it — so they would ask for streaming, not get it, and still see a successful run. That is the silent-degraded-outcome shape the rest of this work exists to prevent, so it now throws with the resolution hint. Covered by a test asserting the rejection. LOW — RETAINED_REL_TYPES had no automated guard, and the round-trip test structurally cannot be one: addRelationship PARTITIONS edges between the graph and the CSVs, and a partition's union is invariant under where the line falls, so that test stays green for any partitioning including a wrong one. Drift there yields a silently incomplete mid-pipeline edge set, not a crash. Added a test that derives the required set by grepping every literal iterRelationshipsByType('X') under src/ and asserts the constant covers it, with CALLS as the documented exemption (taintSummaries reads it, which is why the sink answers a complete read rather than retaining it). Proven discriminating: removing EXTENDS from the constant fails with "expected [ 'EXTENDS' ] to deeply equal []". 128 tests green across the eight affected suites, including the index-lock suite that arrived with the #2677 merge. Refs #2680 * docs(2680): record the measured CPU cost, not just the memory win I measured memory before shipping and never measured time, which was a gap: reads now allocate, rebuilding objects instead of returning stored ones, and a real analyze does SIX full relationship scans (pruner, communities x2, processes x2, the taint fixpoint's CALLS pass). Same 400k-node / 1.08M-edge graph: heap 820 MB -> 623 MB (1.32x better) scans 96 ms -> 651 ms (6.8x WORSE) 6.8x on iteration is worth knowing, but the absolute number decides it: ~0.5 s here, ~2 s extrapolated to kernel scale, against an analyze measured in minutes — under 1% of wall-clock. The ~26M short-lived objects at kernel scale are young-generation churn (the cheap case), and being ~800 MB further from the heap ceiling matters more than the churn costs: #2649's cascade came from GC thrash NEAR the limit, not from allocation volume as such. Also names the first lever if these scans ever go hot — a per-type index over the columns, so iterRelationshipsByType stops scanning all streamed edges — and notes that it trades memory back, so it needs a measurement first. Refs #2680 * perf(2680): cut the iteration regression from 6.8x to 1.8x The memory win came with an unmeasured CPU cost. Iteration went from returning stored objects to rebuilding them, across the SIX full relationship scans an analyze performs (pruner, communities x2, processes x2, taint's CALLS pass). First measurement: 90 ms -> 651 ms, 6.8x worse. Fixed properly rather than documented away. Two causes, each measured before and after: 1. The ~150-character synthesized `id` was built eagerly on every read — 6.5M concatenations per analyze, for a field NO in-pipeline consumer reads. Isolating it (constant id) showed 436 ms of the 555 ms regression. Now a lazy prototype getter on a fixed-shape `StreamedRelationship` class: the string is built only if someone asks, and V8 keeps one hidden class across millions of instances. 2. Generator and iterator-protocol overhead on million-edge walks. `forEachRelationship` (community detection's form, called twice) now loops the columns directly, skipping both. `iterRelationships` keeps an iterator but reuses one result record — a hand-rolled version allocating a fresh {value, done} per edge measured WORSE than the generator (252 ms), which is why the obvious rewrite is not the one that shipped. heap 821 MB -> 623 MB (1.32x better) scans 90 ms -> 180 ms (was 651 ms) The residual ~90 ms is object allocation, 6.5M instances across six scans, and it is irreducible while the read API returns objects at all. The remaining fix for true parity is a field-wise callback passing sourceId/targetId/type/ confidence as primitives — all four hot consumers read only those — but that changes the KnowledgeGraph interface and its consumers, so it belongs in its own measured change rather than bolted on here. Refs #2680 * perf(2680): zero-allocation field scan brings iteration back to parity Third and final step on the iteration cost. The memory win had come with a 6.8x iteration regression; the previous commit cut that to 1.8x by making the synthesized id lazy and removing generator overhead. The residual was object allocation itself — 6.5M instances across the six full relationship scans an analyze performs — which no amount of tuning removes while the read API hands back objects. So the hot consumers stop asking for objects. Adds `KnowledgeGraph.forEachRelationshipFields`, which passes (sourceId, targetId, type, confidence) as primitives — exactly and only what every whole-graph scan reads. On the sink those come straight out of the columns, allocating nothing; on the object-based graph they are read off the stored relationship, so the flag-off path is unaffected. Converted the five whole-graph scans: community detection (x2), process extraction (x2), and the local-symbol pruner. `isFileDefinesEdge` now takes (type, sourceId) rather than a relationship. The taint fixpoint's by-type pass is left alone — one scan of six, and converting it would turn an indexed bucket lookup into a full scan on the object-based graph. heap 820 MB -> 623 MB (1.32x better) scans ~82 ms -> ~90 ms (was 651 ms; now parity within noise) Also deletes the pruner's `hasStreamedSemanticEdge` option, which has had no caller since the sink's reads became complete — a dead knob is worse than no knob. Verified: 104 tests across the eight affected suites, including the pruner's pipeline integration test (which needs the raised worker-ready timeout on this host; it passes cleanly with it and its failures are the known 5s handshake). Refs #2680 * perf(2680): compact dedup keys — 1.32x -> 1.59x, speed unchanged An audit of where duplicate relationship ids actually come from, then the saving it unlocked. The audit (instrumented analyze of this repo): 25 duplicate-id hits across 63,412 streamed edges — 0.04%, all CALLS, every one the SAME call site re-emitted when a file is resolved in more than one language pass. Three things follow, and they rule out the cheap options: - dedup cannot be dropped (25 != 0, and a duplicate reaching COPY is a wrong graph); - it cannot move to row contents, because emit-references builds ids as `...->target:line:col`, so two calls between the same pair at different sites have byte-identical CSV rows that the whole-graph emit keeps; - it cannot move to a per-file source guard like `pdgEmittedFiles`, because a later language pass can resolve genuinely NEW edges for the same file. What was left was the key itself. An id embeds both node ids in full (~200 chars here) while the endpoints are ALREADY interned for the columns, so the Set was storing them twice. Keys are now built from the interner indices plus the id's trailing disambiguator parsed into NUMBERS. Numbers, not substrings, and that is load-bearing: a key built by slicing inside a long string is a V8 sliced/cons string that keeps its parent alive, so the id would never be freed and the saving would silently fail to appear. An earlier attempt at this measured no improvement for exactly that reason. Unrecognized id shapes (`rel:contains:` has no tail) fall back to storing the id verbatim — correctness first, saving second. heap 821 MB -> 518 MB (1.59x, was 1.32x) scans ~83 ms -> ~88 ms (parity, unchanged) Speed is untouched by construction: dedup is on the WRITE path, and none of the six full scans reads it. Also fixes removeRelationship, which the test suite caught: it looked up the raw id in a Set that now holds compact keys, so it silently stopped throwing on an already-streamed edge. It cannot recompute a key from a bare id, so it is now conservative — anything the real graph does not hold is treated as possibly-streamed once streaming has begun and fails loudly. A genuinely-absent id throws where main returns false; acceptable because the only production caller (the COBOL resolver) runs before the sink is armed. 89 tests green across the six affected suites. Refs #2680 * fix(2680): dedup key dropped edges when tail segment counts differed Both findings from the review of this branch, and the coverage gap named alongside them. HIGH — the compact dedup key packed the id's trailing numeric segments as `|${a}|${b}`, with `b` defaulting to 0 when only one segment was present and the segment COUNT absent from the key. So `:7` and `:7:0` produced the same key and the second edge was silently discarded as a duplicate: a lost relationship, no error, no warning. Found by probe, not by reading — two distinct ids for one (source, target, type) went in and one edge came out. The key now carries `seen`. Nothing existing caught it. The round-trip test compares the UNION of graph and CSV rows, and a dropped edge is missing from both, so it stayed green; the duplicate test only feeds a genuinely identical id, which is the case that SHOULD collapse. Four new cases pin the boundary instead: differing segment counts stay distinct, two call sites between one pair stay distinct (the `:line:col` shape from emit-references), a truly repeated id still collapses, and a non-numeric tail falls back to the full id. Proven discriminating — reverting the fix fails with "expected 1 to be 2". This costs ~66 MB at 400k nodes / 1.08M edges (584 MB, was 518 MB), so the heap win is 1.40x rather than 1.59x. Not a trade worth making the other way: a silently missing relationship is the exact failure class the rest of this work exists to prevent. I am not asserting a mechanism for why two extra characters per key cost that much — it is stable and reproducible across runs, and inventing a cause is how I got the earlier cons-string diagnosis wrong. LOW — removeRelationship throws for an absent id once streaming has begun, where KnowledgeGraph.removeRelationship returns false. The behaviour is deliberate (a bare id cannot be turned back into a compact key, and answering "false" for an edge already on disk is the worse failure) but it was undocumented and untested. Now stated on the interface itself and pinned by two cases: absent-id-while-streaming throws, absent-id-before-streaming returns false. Coverage gap — added a test asserting forEachRelationshipFields yields the same (source, target, type, confidence) tuples as iterRelationships. That guards the five whole-graph scans converted in 9fa18384, where a divergence would silently skew community detection, process extraction and the pruner. Also records the verified scaling in the file header: linear at 100k/200k/ 400k/800k nodes, per-edge scan cost flat at ~13 ns in both arms, heap ratio drifting only 1.7x -> 1.5x as interner indices gain digits. No super-linear term. 135 tests green across the eight affected suites. Refs #2680 --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>	2026-07-25 07:43:51 +01:00
Gergő Magyar	df0110b06f	fix: index staleness — false-stale status after analyze (#2668) + inline staleness in query/context/impact/cypher tools (#2655) (#2683) ... * fix(analyzer): case-stabilize runner-identity path fields so status isn't false-stale (#2668) `gitnexus status` reported a freshly-analyzed, untouched repo as stale on Windows (econia/aptos-core, 1.6.10-aptos.0). `status`'s up-to-date check gates on `runnerIdentityIsCurrent`, which deep-compares the stamped runner identity against a freshly recomputed one. That comparison includes `build.rootPath`, `dependencyRuntime.manifestPath`/`lockfilePath`, and `runtime.executablePath` (only `invokedArtifact` is stripped), and `identityCacheKey` hashes packageRoot/buildRoot — all derived from paths that flow through `realpathSync.native`, which canonicalizes 8.3 names and symlinks but does NOT normalize the Windows drive-letter case. When `analyze` and `status` are launched under different drive-letter casing (`c:\...` vs `C:\...`, plausible across CLI shim / npx / server-worker entries), the two identities differ by that one byte and `status` reports stale. Fix: `normalizeAnalyzerRootPath(p, platform)` uppercases the Windows drive letter (POSIX no-op, platform-explicit for testability; preserves a `\\?\` extended-length prefix), applied at the single upstream source — `resolveBuildRoot`'s returned `{packageRoot, buildRoot}` — so every derived identity path field and the cache key inherit a case-stable root, plus at `runtime.executablePath` (process.execPath is the same compared class). The `runnerIdentityIsCurrent` gate is kept intact: a genuine analyzer change still differs in `build.digest`/`dependencyRuntime`, and analyze still rebuilds on real mismatch. Note: the drive-letter divergence was not reproduced on a Windows host (none available); the mechanical chain is verified in source and the fix is a correct defensive normalization that is a no-op on POSIX. If a `status --json` identity field-diff later shows `build.digest`/`dependencyRuntime`/`cliVersion` diverging instead, that indicates a genuinely different install (where "stale" is correct), not this bug. Migration: on Windows, an existing index stamped under the old (non-normalized) casing mismatches the normalized recompute once, triggering a single forced full re-analyze on first upgrade (and a one-time identity-cache recompute). One-time, Windows-only, POSIX no-op. Tests: pure `normalizeAnalyzerRootPath` unit tests (drive-letter uppercase, idempotence, drive-only scope, `\\?\` extended-length prefix, POSIX no-op). * feat(mcp): surface index staleness in query/context/impact/cypher tool responses (#2655) `checkStalenessAsync` already computes how many commits an index is behind the checkout's HEAD, and `list_repos` returns it as `staleness: {commitsBehind, hint}`. But the four hot read tools an agent actually calls in a session — `query`, `context`, `impact`, `cypher` — never surfaced it: `resolveRepo` only runs `maybeWarnSiblingDrift` (stderr, sibling-clone drift only), so a direct tool call gave zero indication the index might be behind HEAD. Thread the existing signal into those four tools at the single `callTool` dispatch chokepoint (after the one `resolveRepo`), reusing the `list_repos` `{commitsBehind, hint}` shape: - `stalenessForTool` computes `checkStalenessAsync` behind an in-flight-promise cache (5s TTL) keyed by lbugPath, so N concurrent tool calls share one `git rev-list` and flat/branch handles (same repoPath, different lastCommit) don't collide. The cache entry is evicted with the repo's other per-index state when the repo leaves the registry. - `withToolStaleness` skips the `git` spawn entirely for results that can't carry the field (via `canCarryStaleness`), so error-returning calls pay nothing. - `attachToolStaleness` adds a `staleness` field to an object result only when the index is behind HEAD. It NEVER changes an existing result's shape: raw-array results (non-tabular cypher rows) are returned untouched, because the CLI's `--limit` and other consumers branch on `Array.isArray`; error envelopes and already-annotated results are left as-is. Non-blocking: `checkStalenessAsync` swallows git failures to `{isStale:false}`, so a git error just omits the field — it never fails the tool. Deliberately out of scope: `@group`-targeted calls forward to `callToolAtGroupRepo` before the chokepoint (multi-repo, single-commit staleness is ill-defined); the legacy `search`/`explore` aliases; and `list_repos` / the `context` resource, which already carry the signal. Tests: `attachToolStaleness` branch matrix (stale object -> field; fresh -> unchanged; raw array -> unchanged; error envelope -> unchanged; idempotent; non-object -> unchanged; null-safe) and a flat-vs-branch cache-key regression test that fails when the cache is keyed by repoPath. * test(mcp): cover staleness tool-signal edge cases + harden the freshness boundary (#2655) Addresses the coverage gaps the review flagged on the #2655 staleness signal, plus one defensive guard so a failing freshness check can never fail a tool. Production (defense-in-depth, no behavior change on the happy path): - withToolStaleness now awaits stalenessForTool with a `.catch(() => undefined)` so a rejection degrades to no-staleness instead of failing query/cypher/ context/impact. - stalenessForTool wraps the check in `Promise.resolve(...).catch(...)` that evicts the cache entry on rejection — a transient failure isn't served as a permanently-rejecting promise for the rest of the TTL window, and the `Promise.resolve` wrap makes the boundary robust to a non-thenable return (a no-op for the real async checkStalenessAsync). A resolving promise is never evicted, so happy-path dedup is unchanged. Tests (gitnexus/test/unit/calltool-dispatch.test.ts): - F1: a rejecting checkStalenessAsync leaves the tool payload intact with no staleness field, and a later call recovers (proves the entry isn't poisoned). Written first and confirmed to fail without the guard. - F2: staleness attaches on query/context/impact object results and on cypher's tabular {markdown,row_count}; a raw-array cypher result keeps its shape. - F3: drift guard — exactly query/cypher/context/impact route through stalenessForTool; explain/pdg_query/detect_changes/check do not. - F4: the per-index cache dedupes within TOOL_STALENESS_TTL_MS and recomputes after it expires (driven via a Date.now spy, not fake timers). Tests (gitnexus/test/unit/analyzer-identity.test.ts): - F5: the produced identity's build.rootPath and runtime.executablePath are normalizer-stable, guarding that both call sites thread through normalizeAnalyzerRootPath (trivial on POSIX, a real regression guard on Windows CI). Plus a source comment noting the one-time Windows re-analyze on first upgrade. * test(mcp): run #2668 guard on Windows CI, document staleness field, cover staleness edge cases Addresses the review follow-ups on the staleness work: - Wire test/unit/analyzer-identity.test.ts into scripts/cross-platform-tests.ts (PLATFORM_LOGIC). Its "identity path fields are normalizer-stable" fixpoint is the Windows regression guard for the #2668 drive-letter normalization, but normalizeAnalyzerRootPath is a POSIX no-op, so the guard was only ever running (trivially green) on the Ubuntu full-suite and never on the windows-latest matrix where it actually bites. Now it runs where it matters. - Document the inline `staleness` field on query/context/impact/cypher responses in the gitnexus-guide skill (both the .claude source and the shipped gitnexus-claude-plugin mirror, kept in sync). - Add three staleness tests that pin behavior the prior tests only implied: * @group-routed calls never get the signal (forwarded before the wrapping switch) — locks the intentional skip so it can't silently flip. * one in-flight freshness check is shared across truly concurrent calls (two dispatched before checkStalenessAsync settles → a single spawn), not just sequential reuse of an already-resolved value. * a late rejection from a superseded cache entry does not evict the newer entry that replaced it after the TTL rolled over (the `=== entry` object-identity guard). The defensive stack in stalenessForTool/withToolStaleness (Promise.resolve wrap + guarded evict + outer catch) is retained deliberately: the wrap is load-bearing for the tests (a sibling describe's vi.resetAllMocks() makes the mock return undefined), and the guarded evict closes the superseded-entry edge now covered above. * fix(test): split the #2668 normalization guard into a portable cross-platform file Registering analyzer-identity.test.ts on the Windows/macOS matrix (previous commit) surfaced four pre-existing failures in that file on macOS 3/3 and windows 3/3. They are not new breakage: those fixture tests compare identity fields against the RAW temp-dir path while the identity resolves through realpathSync.native, so on macOS `/var/folders/...` is received as `/private/var/folders/...`. The file was simply never portable — it had only ever run in the Ubuntu full-suite. Reproduced locally by pointing TMPDIR at a symlink: the same four tests fail, and pass again without it. Move only the portable assertions — the pure `normalizeAnalyzerRootPath` cases (explicit `platform` argument) and the identity fixpoint guard (which compares each field against ITSELF normalized, never against the fixture path) — into test/unit/analyzer-identity-path-normalization.test.ts, and register that file on the matrix instead. The #2668 Windows regression guard still runs where it actually bites, without dragging four symlink-sensitive tests onto runners they were never written for. Verified: the new file passes with TMPDIR behind a symlink (the macOS condition); the heavy file is back to Ubuntu-only. * fix(test): keep the cross-platform #2668 file fixture-free so Windows stays green The split file still carried the fixture-based fixpoint guard, which fails on windows-latest: Invoked analyzer artifact is absent from the validated build: D:\a\...\node_modules\vitest\dist\workers\forks.js Cause is a pre-existing cross-drive defect in this module's `isInside()`, not the #2668 change. The GH Windows runner keeps the repo on D: and temp fixtures on C:. `path.win32.relative('C:\\...fixture', 'D:\\...forks.js')` cannot express a relative path across drives, so it returns the absolute target — which does not start with '..', so `isInside()` reports true. `resolveInvokedArtifact` therefore treats the vitest fork worker as the invoked artifact, it is absent from the fixture's validated build, and identity resolution throws. (Verified directly: `isInside` returns true cross-drive and false for the same-drive control.) Keep the cross-platform file strictly pure — only `normalizeAnalyzerRootPath` assertions with an explicit `platform` argument, no fixture and no filesystem — so it is green on every runner while still exercising the transform on real Windows. The fixture-based threading guard moves back to analyzer-identity.test.ts (Ubuntu-only), where the rest of that file's fixture tests already live, with a comment recording why it cannot be on the matrix. The underlying `isInside()` cross-drive bug is left untouched here (out of scope for this PR) but is worth its own fix: it also guards the trusted cache directory and the identity-cache path-escape check in validateIdentityCache, where a false "inside" verdict weakens validation on multi-drive Windows setups. --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>	2026-07-25 07:21:44 +01:00
jecanore	a500f70d6f	feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn (#2640) ... * feat(analyze): add opt-in --self-commit flag for AGENTS.md/CLAUDE.md churn Adds a new `--self-commit` flag to `gitnexus analyze`. When passed, any AGENTS.md/CLAUDE.md changes the run makes (including first-time creation) are auto-committed, scoped to only those two files (never `git add -A`). No-ops silently if neither exists, neither changed, or the repo has no git identity configured — never fails the surrounding analyze run. Complements #1478 (--no-stats): that flag removes the volatile counts entirely, this one keeps them but eliminates the dangling working-tree diff they otherwise leave behind on every run. Closes #2639. * fix(analyze): log a warning when --self-commit fails to commit Addresses review feedback on #2640: the commit step's catch block was silently swallowing failures (e.g. missing git identity) with no signal to the user. Logs via the existing pino logger (matching the rest of the codebase's convention) with the error and the file list, while still never throwing — analyze must not fail over this. New test forces a real commit failure (missing identity, with useConfigOnly + isolated HOME/XDG_CONFIG_HOME/GIT_CONFIG_NOSYSTEM so no ambient global git config on the CI runner can mask it) and asserts the warning is captured via logger's _captureLogger test hook. * fix(analyze): refuse to sweep pre-existing edits into --self-commit Addresses both state-safety blockers from review round 2 on #2640: 1. selfCommitContextFiles could not distinguish a pre-existing unstaged user edit in AGENTS.md/CLAUDE.md from this run's generated stats refresh — both just showed up as "the file is dirty" — so a user edit sitting in either file got silently swept into the generated commit. Fixed by snapshotting each candidate's cleanliness via the new snapshotSelfCommitSafety() BEFORE analyze writes to it; only files confirmed safe (nonexistent pre-run, i.e. first-time creation, or clean pre-run) are ever added/committed. A file already dirty pre-run is skipped and logged, never touched. 2. On a failed `git commit` (e.g. missing identity), the preceding `git add` had already staged the safe files, and analyze reported nothing happened while silently leaving them staged. Fixed with a `git reset -- <safe files>` in the commit-failure catch, restoring the index to its pre-add state for exactly the files this helper staged. Wired analyze.ts to call snapshotSelfCommitSafety() once before runFullAnalysis (which is where the actual AGENTS.md/CLAUDE.md write happens, on both the fast path and the primary run), threading the result through both existing selfCommitContextFiles() call sites. New tests: a pre-dirty AGENTS.md is skipped while a clean CLAUDE.md still commits normally, and a post-add commit failure leaves nothing staged. Updated all existing selfCommitContextFiles() call sites for the new required safety-map parameter. * i18n(cli): add zh-CN translation for --self-commit help text Addresses magyargergo's follow-up on #2640: --self-commit was missing from the analyze command's OPTION_DESCRIPTION_KEYS map, so its help text never went through localizeCliHelp and always rendered in English regardless of locale. Adds the help.option.analyze.selfCommit key to both en.ts and zh-CN.ts and wires it into help-i18n.ts, matching the existing --no-stats/--skills entries. --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-25 06:23:26 +01:00
Gergő Magyar	1e764cd475	fix(analyze): single-writer lock for the index write path (#2658) (#2677)	2026-07-25 05:08:13 +01:00
Copilot	d3d4fa31bb	fix(scope-resolution): gate C#/Kotlin free calls by instance ownership (#2563) (#2654) ... * Initial plan * fix(scope-resolution): gate C# and Kotlin free calls * fix(scope-resolution): keep Kotlin ownership gate safe * Apply remaining changes * perf(scope-resolution): benchmark and cache ownership gates * test(scope-resolution): simplify benchmark scaling loop * refactor(scope-resolution): encapsulate ownership cache * test(scope-resolution): enforce subquadratic ownership scaling * fix(scope-resolution): address ownership review findings * test(csharp): regenerate capture golden for #2563 fixtures The committed expected-captures.json was missing the new NamespaceOwnerCollision.cs entry and carried a stale SameFileCases.cs digest/count (56 → 67), so csharp-captures-golden.test.ts was the sole red check on the PR. Regenerate with UPDATE_GOLDEN=1 to match the fixtures the bench fingerprint already reflects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-24 13:31:56 +01:00
Copilot	450cebc268	fix(java): JLS binary-name identities for local classes, enums, records & interfaces (#2562) (#2653) ... * Initial plan * docs(plans): add Java local class naming plan * fix(java): model local class binary names * docs(java): clarify local class naming guards * fix(java): recognize local classes in compact constructors * chore: remove Java naming plan * fix(java): harden local type identities and scope * perf(java): linearize local type ordinal allocation * fix(java): harden ordinal benchmark follow-up * docs(java): clarify ordinal benchmark invariants * test(java): cover local type ownership paths --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>	2026-07-24 11:58:53 +01:00
MyShining	4af6fe8587	feat(spring): resolve constructor and standard injection (#2632)	2026-07-24 08:25:38 +01:00
dependabot[bot]	e34967eed5	chore(deps)(deps): bump express-rate-limit in /gitnexus (#2657) ... Bumps [express-rate-limit](https://github.com/express-rate-limit/express-rate-limit) from 8.5.2 to 8.6.0. - [Release notes](https://github.com/express-rate-limit/express-rate-limit/releases) - [Commits](https://github.com/express-rate-limit/express-rate-limit/compare/v8.5.2...v8.6.0) --- updated-dependencies: - dependency-name: express-rate-limit dependency-version: 8.6.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-24 06:48:07 +01:00
Abhigyan Patwari	91b22676ce	Merge pull request #2488 from ArgonarioD/main ... feat(cli): mirror skills to .agents/skills/ when .agents/ exists	2026-07-23 21:09:04 +05:30
Gergő Magyar	bd9889cdec	Merge branch 'main' into main	2026-07-23 15:04:24 +01:00
Gergő Magyar	170805647c	fix(rust): keep duplicate type names ambiguous in range binding (#2514) (#2652) ... * fix(rust): latch duplicate type-name ambiguity in range binding (#2514) The range-binding prepass tracked cross-file return and field types in two maps and used map presence itself as the ambiguity flag: the second definition of a name deleted it, but a third definition found it absent and re-inserted the last-scanned file's type. Odd duplicate counts (3, 5, ...) therefore resolved a genuinely ambiguous name to whichever file was scanned last, while even counts stayed ambiguous. Latch ambiguity in a dedicated Set per registry (ambiguousReturnTypes, ambiguousFieldTypes): once a name has two or more workspace definitions it never resolves again, regardless of duplicate count or file order. Adds integration coverage for two/three-duplicate functions and structs, permuted file order, and a unique-name over-suppression guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(rust): bump INCREMENTAL_SCHEMA_VERSION to 12 for the #2514 range-binding fix The duplicate-name ambiguity latch changes which cross-file Rust CALLS edges the range-binding prepass emits. The incremental writeback persists only changed-file nodes, so an incremental top-up against a pre-v12 index would keep the old spurious edges on every unchanged Rust file. Bump the schema version to force a one-time full re-analyze, matching the v7/v11 contract for edge-affecting resolver changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(rust): resolve import-disambiguated duplicate types in for-loops & destructuring Follow-up to the #2514 ambiguity latch. When several modules define the same function/struct name and a call site disambiguates it with a `use` import (including aliases and `use x::*` globs), range-binding now resolves the for-loop element type and the destructured field type to that specific imported definition, instead of leaving it unresolved. The bare-name return/field maps are (correctly) ambiguous for duplicates, but the call site's import pins a definition. range-binding records the full, untruncated return/field type per defining file, and resolveImportedDef() resolves a name to the single in-scope definition, mirroring Rust name resolution: - tier 1: explicit `use`/re-export imports and local defs (lookupBindingsAt); these shadow globs, so if any exist we decide within them alone; - tier 2: glob imports, consulted only when tier 1 is empty; a `wildcard-expanded` ImportEdge names the target module, so we resolve only when exactly one glob-target file actually defines the name. Two or more visible definitions stay unresolved, preserving the #2514 latch. normalizeRustReturnType is untouched (its Vec<T> -> Vec truncation is load-bearing for receiver resolution), so the full generic is read from the per-file map instead. Covered by integration tests: explicit / aliased / single-glob imports resolve to the imported definition; two globs that both export the name stay ambiguous; a local definition shadows a glob; no-import duplicates stay unresolved (#2514). INCREMENTAL_SCHEMA_VERSION stays at 12 (bumped by the #2514 commit in this PR); its note now also covers these added resolution edges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(rust): parse each file once in range-binding when the workspace fits a budget populateRustRangeBindings makes two passes over every file and, because the shared treeCache is empty in the analyze flow, re-parsed each file in both — a workspace of N files paid 2N parses. It now parses each file once and reuses the tree across both passes via an in-function store, gated by a source-byte budget: workspaces up to 16 MiB of Rust source (essentially every real repo) reuse trees; larger ones fall back to per-pass re-parsing so peak RSS stays bounded on huge repos (the memory-sensitive case keeps its current profile). Also collapses the parse+timeout boilerplate that was copy-pasted in both loops into one getOrParseTree helper, and adds a PROF-gated `rangeBind=` segment to the scope-resolution profiler for phase-level observability. Measured on a 500-file synthetic Rust workspace (PROF_SCOPE_RESOLUTION=1): the range-binding phase drops ~370ms -> ~320ms (~14%), parses 1000 -> 500. Behavior is unchanged (199 rust + range-binding-order + parse-timeout tests green); repos above the budget are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(rust): update schema-version gate to v12; regenerate golden + bench baseline for new fixtures CI surfaced three deterministic-artifact failures, all from this PR's own additions: - call-summary-schema-version.test.ts hardcoded INCREMENTAL_SCHEMA_VERSION === 11 (the #2604 window); #2514 bumped it to 12. Update the gate and extend the reuse-gate version history so a v11 stamp now forces a full re-analyze. - rust-captures-golden expected-captures.json drifted (130 -> 174 entries) because the new rust-import-* / rust-dup-* fixtures joined the rust-* corpus. Regenerated (UPDATE_GOLDEN=1): additions only, no existing captures changed — emitRustScopeCaptures is untouched. - bench/scope-capture/baselines.json rust fingerprint drifted for the same reason. Rebaselined with a provenance note; scaling 1.06 < 1.5 budget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-23 13:43:24 +01:00
Gergő Magyar	76f9f70183	fix(cli): LadybugDB native-load failures fail closed, incl. truncated-binary SIGBUS (#2441) (#2651) ... * test(cli): cover analyzer lazy-action native-load failure (#2441) createAnalyzerLbugLazyAction — the wrapper the `analyze` command uses — had only a happy-path test; its native-load-failure branch was untested, so a regression could silently reintroduce #2441 (analyze exiting 0 after a LadybugDB native load failure, writing no index while reporting success). Add a failure-path test asserting that when checkLbugNative() reports the binary cannot load, the analyzer module is NOT imported, process.exitCode is set to 1, and the repair message is written to stderr. Mirrors the existing createLbugLazyAction failure test. Verified discriminating: the test fails ("expected undefined to be 1") when the exitCode guard is removed from the analyzer branch, and passes with it restored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cli): probe LadybugDB native load out-of-process so a truncated binary fails closed (#2441) checkLbugNative() loaded lbugjs.node in-process to validate it. That catches clean load failures (missing dylib, zero-byte, garbage -> "file too short"), but a merely truncated/corrupted binary (valid header, missing pages) SIGBUSes the dynamic loader mid-dlopen — a signal, not a catchable throw — taking the whole CLI down with a raw exit 135 and no guidance. Load the binary in a throwaway child process instead. Only a child that RAN and failed (non-zero exit or a fatal signal) marks the binary bad; if the probe itself could not run — a spawn error or timeout, e.g. a no-subprocess sandbox or a non-Node execPath — the result is inconclusive and the command's own load stays authoritative rather than condemning a healthy binary. The probe forces ELECTRON_RUN_AS_NODE, removes the redundant in-process pre-load, and costs ~20ms. Regression tests: truncated binary -> ok:false; unspawnable probe -> ok:true. Verified: a 300KB-truncated native now exits 1 with the repair message (previously exit 135 SIGBUS); zero-byte/garbage stay graceful; good native still loads and indexes. 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-23 11:59:56 +01:00
Abhigyan Patwari	4f59831324	Merge pull request #2648 from abhigyanpatwari/dependabot/github_actions/softprops/action-gh-release-3.0.2 ... chore(deps): bump softprops/action-gh-release from 3.0.1 to 3.0.2	2026-07-23 14:24:38 +05:30
Gergő Magyar	f37c126f0c	Merge branch 'main' into dependabot/github_actions/softprops/action-gh-release-3.0.2	2026-07-23 09:23:26 +01:00
ArgonarioD	f812f709b6	Merge remote-tracking branch 'upstream/main'	2026-07-23 16:14:32 +08:00
Abhigyan Patwari	437c2bb4b5	Merge pull request #2647 from abhigyanpatwari/dependabot/github_actions/actions/setup-node-7.0.0 ... chore(deps): bump actions/setup-node from 6.4.0 to 7.0.0	2026-07-23 13:22:16 +05:30
ArgonarioD	39e9dc8b25	Merge remote-tracking branch 'upstream/main'	2026-07-23 14:50:09 +08:00
Abhigyan Patwari	fc21e40b64	Merge pull request #2643 from abhigyanpatwari/dependabot/npm_and_yarn/gitnexus-web/lru-cache-11.5.2 ... chore(deps)(deps): bump lru-cache from 11.5.1 to 11.5.2 in /gitnexus-web	2026-07-23 11:56:12 +05:30
Gergo Magyar	768161ceb2	fix(ci): sync review-agent workflow test with setup-node v7.0.0 pin ... The dependabot bump to actions/setup-node@8207627860 (v7.0.0) left the review-agent-workflow.test.ts pin allowlist pointing at the old v6.4.0 SHA, failing CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-23 06:00:27 +00:00
Abhigyan Patwari	adaafa3c4f	Merge pull request #2641 from abhigyanpatwari/dependabot/npm_and_yarn/gitnexus-web/vite-8.1.5 ... chore(deps)(deps-dev): bump vite from 8.1.4 to 8.1.5 in /gitnexus-web	2026-07-23 11:24:23 +05:30
Abhigyan Patwari	0d2cf725e7	Merge pull request #2642 from abhigyanpatwari/dependabot/npm_and_yarn/gitnexus-web/react-i18next-17.0.10 ... chore(deps)(deps): bump react-i18next from 17.0.8 to 17.0.10 in /gitnexus-web	2026-07-23 11:24:07 +05:30
Abhigyan Patwari	ac163d76a7	Merge pull request #2645 from abhigyanpatwari/dependabot/npm_and_yarn/gitnexus-web/langchain/langgraph-1.4.8 ... chore(deps)(deps): bump @langchain/langgraph from 1.4.7 to 1.4.8 in /gitnexus-web	2026-07-23 11:23:06 +05:30
Abhigyan Patwari	ebc281066f	Merge pull request #2646 from abhigyanpatwari/dependabot/npm_and_yarn/gitnexus-web/babel/types-8.0.0 ... chore(deps)(deps-dev): bump @babel/types from 7.29.7 to 8.0.0 in /gitnexus-web	2026-07-23 11:22:52 +05:30
Gergő Magyar	c145833518	Merge branch 'main' into dependabot/npm_and_yarn/gitnexus-web/lru-cache-11.5.2	2026-07-23 05:55:33 +01:00
Gergő Magyar	28d50cb958	Merge branch 'main' into dependabot/github_actions/softprops/action-gh-release-3.0.2	2026-07-23 05:55:14 +01:00
Gergő Magyar	a5b24f7bd8	Merge branch 'main' into dependabot/npm_and_yarn/gitnexus-web/babel/types-8.0.0	2026-07-23 05:55:01 +01:00
Gergő Magyar	2c1bd0d74a	Merge branch 'main' into dependabot/npm_and_yarn/gitnexus-web/vite-8.1.5	2026-07-23 05:54:46 +01:00
Gergő Magyar	55fb0456c3	Merge branch 'main' into dependabot/npm_and_yarn/gitnexus-web/react-i18next-17.0.10	2026-07-23 05:54:40 +01:00
Gergő Magyar	415d916bde	Merge branch 'main' into dependabot/github_actions/actions/setup-node-7.0.0	2026-07-23 04:45:43 +01:00
Gergő Magyar	cdbdf219dc	fix(lbug): reclaim missing-shadow WAL quarantine files on write-path init (#2638)	2026-07-22 21:30:52 +01:00
dependabot[bot]	e50c49949c	chore(deps): bump softprops/action-gh-release from 3.0.1 to 3.0.2 ... Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 3.0.1 to 3.0.2. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](718ea10b13...3d0d9888cb) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: 3.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:18:38 +00:00
dependabot[bot]	47f3932c8c	chore(deps): bump actions/setup-node from 6.4.0 to 7.0.0 ... Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.4.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](48b55a011b...8207627860) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:18:29 +00:00
dependabot[bot]	450a22aaa5	chore(deps)(deps-dev): bump @babel/types in /gitnexus-web ... Bumps [@babel/types](https://github.com/babel/babel/tree/HEAD/packages/babel-types) from 7.29.7 to 8.0.0. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v8.0.0/packages/babel-types) --- updated-dependencies: - dependency-name: "@babel/types" dependency-version: 8.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:14:44 +00:00
dependabot[bot]	dbce222310	chore(deps)(deps): bump @langchain/langgraph in /gitnexus-web ... Bumps [@langchain/langgraph](https://github.com/langchain-ai/langgraphjs/tree/HEAD/libs/langgraph-core) from 1.4.7 to 1.4.8. - [Release notes](https://github.com/langchain-ai/langgraphjs/releases) - [Changelog](https://github.com/langchain-ai/langgraphjs/blob/main/libs/langgraph-core/CHANGELOG.md) - [Commits](https://github.com/langchain-ai/langgraphjs/commits/@langchain/langgraph@1.4.8/libs/langgraph-core) --- updated-dependencies: - dependency-name: "@langchain/langgraph" dependency-version: 1.4.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:14:38 +00:00
dependabot[bot]	51f8ec0a60	chore(deps)(deps): bump lru-cache from 11.5.1 to 11.5.2 in /gitnexus-web ... Bumps [lru-cache](https://github.com/isaacs/node-lru-cache) from 11.5.1 to 11.5.2. - [Changelog](https://github.com/isaacs/node-lru-cache/blob/main/CHANGELOG.md) - [Commits](https://github.com/isaacs/node-lru-cache/compare/v11.5.1...v11.5.2) --- updated-dependencies: - dependency-name: lru-cache dependency-version: 11.5.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:14:28 +00:00
dependabot[bot]	60a2267b1f	chore(deps)(deps): bump react-i18next in /gitnexus-web ... Bumps [react-i18next](https://github.com/i18next/react-i18next) from 17.0.8 to 17.0.10. - [Changelog](https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md) - [Commits](https://github.com/i18next/react-i18next/compare/v17.0.8...v17.0.10) --- updated-dependencies: - dependency-name: react-i18next dependency-version: 17.0.10 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:14:22 +00:00
dependabot[bot]	16f3f01085	chore(deps)(deps-dev): bump vite from 8.1.4 to 8.1.5 in /gitnexus-web ... Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 8.1.4 to 8.1.5. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v8.1.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 8.1.5 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>	2026-07-22 20:14:16 +00:00
Gergő Magyar	9538be957d	fix(lbug): scale the buffer-pool budget by the OS page-size granule ratio (#2631) (#2636) ... * fix(lbug): scale the buffer-pool budget by the OS-page discard-granule ratio (#2631) LadybugDB bills buffer-pool budget per discard granule, not per 4 KiB frame: the engine's vm_region.cpp sets discardGranuleSize = max(frameSize, osPageSize), claimFrame charges the whole granule when its first frame becomes resident, and releaseFrame refunds only when the granule's last frame leaves — while BufferManager::reserve measures eviction progress in refunded bytes and throws 'The buffer pool is full and no memory could be freed!' after three zero-refund passes. On a 64 KiB-page kernel (Ascend/aarch64 openEuler — the #2631 reporter's host) that is 16 frames per granule: the same COPY bills up to 16× the budget it needs on x86, and whole eviction passes can evict frames yet refund nothing. Apple Silicon macOS (16 KiB pages) is the same mechanism at 4×. Measured with the reporter's exact command and version: vllm-ascend needs a (128, 256] MiB pool on 4 KiB pages — 64/128 MiB reproduce the reporter's byte-identical error, 256 MiB and the 576 MiB adaptive pool succeed — so their 64 KiB host cannot survive on a page-size-blind budget. Scale every derived pool size by granuleRatio = max(1, osPageSize/4096): the per-element estimate, the COPY-safety floor, and the default cap (still bounded by 80% of RAM). 4 KiB hosts are byte-identical to before — proven by pinning the existing sizing tests to an explicit 4096 page size, which also stops them drifting on 16 KiB Apple Silicon runners. GITNEXUS_LBUG_BUFFER_POOL_SIZE keeps absolute precedence and 0 still restores the native default. Also: bufferPoolExhaustionRemedy() gives the exhaustion error an actionable cause→consequence→remedy message; the isLbugPageSizeFrameError comment that called pool exhaustion 'a sizing problem, not a page-size one' is corrected — that framing inverted when #2582 made pool size a function of a page-size-blind estimate. Cannot execute on a 64 KiB kernel here: the scaled path is proven by unit stubs plus the engine-source math above; the env override remains the field escape hatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): actionable pool-exhaustion remedies at the COPY sites and a doctor pool line (#2631) The node-COPY throw and the relationship-COPY warning now append bufferPoolExhaustionRemedy() when the failure is the engine's pool-exhaustion class: the raw binder text gave the operator nothing to act on, and on non-4K-page hosts the pool bills up to pageSize/4KiB × faster than the sizing was calibrated for. The relationship path appends the remedy once per bulk load, not once per failed pair. doctor prints the effective pool size next to the page-size line ('pool size 2048 MiB', with an '(×N page-size scaling)' suffix on non-4K hosts) so support triage sees the sizing inputs at a glance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(lbug): re-anchor getEffectiveBufferPoolSize's placement and reuse granuleRatio in doctor Self-review fixes: the getter's insertion had orphaned resolveBufferManagerSize's doc comment (it read as documenting the wrong function), and doctor's scale note duplicated the granule math with a hardcoded 4096. granuleRatio is now exported (it already carried the test-seam default param) and doctor consumes it. No behavioral change — the sizing suite pins byte-identical outputs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): keep the hintless pool default unscaled and make both remedies visible (#2631) Review fixes: - Scale only the analyze-path cap (scaledAnalyzePoolCap), not defaultBufferPoolSize: the pool is an eager native allocation at DB open (measured, see POOL_BYTES_PER_ELEMENT), so a page-size-scaled hintless default would hand a long-lived MCP process up to 80% of RAM — the #2557 OOM exposure the 2 GiB cap removed. Fix the MAP_NORESERVE claim that contradicted that measurement. - Log the rel-pair pool remedy (loadGraphToLbug returns warnings that no call site reads) and dedup it with a local boolean instead of matching the remedy's own wording. - Label the GITNEXUS_LBUG_BUFFER_POOL_SIZE=0 sentinel as the native 80%-of-RAM default in both the remedy and doctor instead of '0 MiB'. - Extract poolSizeDoctorLine (pageSizeDoctorLines convention): mark env overrides, drop the scaling suffix that misdescribed absolute values. - Fold _resetOsPageSizeCacheForTest into _setOsPageSizeForTests(undefined). - Document the analyze-path scaling in both README env tables. --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 20:09:48 +01:00
Abhigyan Patwari	0eeecb37f3	fix(python): resolve calls through constructor-injected fields (#2628) ... * fix(python): resolve calls through injected fields * fix(ci): update python capture benchmark fingerprint * fix(python): make constructor field inference conservative --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>	2026-07-22 16:32:53 +01:00
Gergő Magyar	e814e28f1f	fix(deps): bump @ladybugdb/core to ^0.18.3 — rel-property IN-predicate fix (#2508) (#2634) ... LadybugDB ≤0.18.2 mis-evaluated `r.type IN [...]` on relationship table groups: the boolean-filter fallback skipped writing selection buffers for single-row unflat chunks, dropping/duplicating callers in context() and impact() (upstream LadybugDB#692, fixed by LadybugDB#699, shipped in 0.18.3). Floor the dependency at ^0.18.3 and lock core + all five platform packages. Resurrect the caller-identity regression test from PR #2553 (closed as superseded by the upstream fix): it pins context()/impact() to exact caller IDs across CodeRelation sub-table pairs so any future predicate regression fails loudly. Note: with CREATE-seeded data the test also passes on 0.18.2 (the upstream repro needs COPY-written chunk layouts) — it is a behavioural pin, not a bug reproduction. Fixes #2508 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-22 16:31:56 +01:00
Gergő Magyar	7f7255aef8	fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624) ... * feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input <MATCH (n:Function) WHERE n.filePath = '>' — path.join produces backslashes on Windows, and a backslash inside the seed helper's single-quoted Cypher literal breaks the parser. The graph stores repo-relative filePaths with forward slashes on every OS, so graph-side paths are POSIX literals now (the incremental-orchestration convention); path.join stays only for real filesystem access. The same Windows lane also proved the substance this suite exists for: lbug-vector-extension passed 7/7 on windows-latest — the extension installed, loaded, and built a real HNSW index there — and the pool vector-lane and DML gate suites passed too. This commit fixes the harness, not the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <abhigyan1.patwari@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 12:27:00 +01:00
Abhigyan Patwari	a84e029066	fix(eval): give vitest a writable .vite-temp inside read-only dependency mounts (#2630) ... * fix(eval): give vitest a writable .vite-temp inside read-only dependency mounts Every task verify command and every hidden oracle ends in `npx vitest run <test>`, and both run through run_verify with read_only_workspace=True. Vite transpiles a TypeScript config by writing <node_modules>/.vite-temp/<config>.timestamp-*.mjs before it loads anything, so against a read-only dependency mount vitest dies with EROFS before a single test executes: EROFS ... /workspace/gitnexus/node_modules/.vite-temp/vitest.config.ts.timestamp-*.mjs This is pre-existing and was masked: until #2627 the verify command died at `npx: not found`, short-circuiting the `&&` chain before vitest ran. Confirmed by reproducing it at that merge base with npx bypassed entirely (`./node_modules/.bin/vitest`), so it is independent of the node-prefix mount. Because it blocks the oracle as well as the authored-test verify, `resolved` stays 0/N without this. bwrap cannot create a mount point inside an already-read-only bind -- the same constraint that put SANDBOX_NODE under /opt/claude -- so overlaying a tmpfs only works if the directory already exists in the mounted bytes. It cannot be mkdir'd into the dependency snapshot after capture either: the snapshot is digest-bound and validate_dependency_binding fails closed on drift. So the empty directory is captured during dependency capture, before the manifest and both dependency digests are computed, making it part of the snapshot rather than an untracked mutation of it. The sandbox then overlays a tmpfs on exactly that path; everything else in the mount, and the whole workspace, stays read-only, and the overlay never reaches the host clone the credited patch comes from. Scoped to dependency mounts whose target basename is node_modules, so hidden oracle and skill mounts stay wholly read-only with no writable island. Note: this shifts sandbox_dependency_content_digest and sandbox_dependency_manifest_digest, so promotion evidence recorded before this change is no longer comparable. That is already true of any harness fix that changes what the sandbox exposes. Verified on the self-hosted runner through the real path -- TaskAssetCache .prepare -> stage_task_assets -> prepare_sandbox -> run_verify with the actual trivial-version-alias verify string: passed, 15/15 tests, no EROFS. Full eval suite there with GITNEXUS_REQUIRE_BWRAP_CANARY=1: 337 passed, 4 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(eval): only overlay .vite-temp where the mount source actually carries it The tmpfs overlay keyed purely on the mount target basename being node_modules, which also matched the trusted GitNexus runtime mount at /opt/gitnexus/node_modules. That mount's source is the built runtime and does not carry a .vite-temp, and bwrap cannot create a mount point inside an already-read-only bind, so the containment CI job failed: bwrap: Can't mkdir /opt/gitnexus/node_modules/.vite-temp: Read-only file system FAILED test_real_bubblewrap_runtime_mount_imports_cli_without_exposing_checkout My runner probe only exercised the dependency-mount path, so it missed this. Gate the overlay on the mount SOURCE actually containing the directory rather than on the target name. task_assets.py captures .vite-temp only into dependency-snapshot node_modules, so the overlay now fires exactly there and never on the runtime mount -- and the gate is correct by construction, since a tmpfs can only overlay a mount point that already exists in the bound bytes. Adds a regression test for a node_modules mount whose source has no captured .vite-temp (the runtime-mount shape) getting no overlay, and updates the positive test to create the directory in its mount source. Verified on the self-hosted runner: the exact failing test now passes, and the full containment selection is 124 passed, 4 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 10:16:25 +01:00
dependabot[bot]	735289e399	chore(deps)(deps): bump fast-uri from 3.1.2 to 3.1.4 in /gitnexus (#2626) ... Bumps [fast-uri](https://github.com/fastify/fast-uri) from 3.1.2 to 3.1.4. - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.2...v3.1.4) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-22 08:27:11 +01:00
Abhigyan Patwari	13095bc4bc	fix(eval): mount the node prefix for npx and catch nested Claude Code bootstrap noise (#2627) ... * fix(eval): ignore Claude Code bootstrap noise nested below the workspace root The planning-phase boundary check excluded Claude Code's own sandbox-bootstrap paths only at the workspace root: workspace_snapshot tested relative.parts[0] against WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE. But Claude Code bootstraps into whatever directory it is running in, and the benchmark's task prompts cd into gitnexus/, so the same noise landed one level down as gitnexus/.claude/.cc-writes -- whose parts[0] is "gitnexus", so it was never excluded. In skill-evolution run 29861768554 that accounted for 13 of 18 sessions, each failing with error_kind plan-evidence-invalid and the identical error_detail "phase changed unauthorized workspace path(s): gitnexus/.claude/.cc-writes". The same code path also guards the review phase (runner.py:499), so review arms hit it as review-evidence-invalid. Widening the whole set to match at any depth would be wrong: it also contains package.json, package-lock.json, node_modules and the .env family, and both gitnexus/package.json and gitnexus/.claude/settings.local.json are real tracked files whose edits must still be caught. So the root-anchored rule is unchanged, and a second narrow rule matches only the entries Claude Code itself creates inside a .claude directory (.cc-writes, agents, commands) at any depth -- never .claude itself. The predicate moves into _is_bootstrap_noise so it is directly testable. It is still evaluated before pending.append, so an excluded directory is never descended into. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(eval): mount the node install prefix so npx and npm resolve in the sandbox _runtime_mount_args bound only the `node` binary itself to SANDBOX_NODE. npm and npx are not standalone binaries -- they are symlinks into ../lib/node_modules/npm/bin/*-cli.js -- so the install prefix carrying both bin/ and lib/node_modules has to be mounted for them to resolve at all. On GitHub-hosted images node lives in /usr/local/bin, whose prefix (/usr/local) is already inside the wholesale /usr read-only bind, so npm and npx came along for free and the gap stayed invisible. A self-hosted runner's actions/setup-node installs into its own tool cache, outside /usr, so only the single node file was bound. Every task's verify command is "cd gitnexus && npx tsc --noEmit && npx vitest run <test>", so in skill-evolution run 29861768554 all 18 of 18 result records carried the identical verify_output "/bin/sh: 1: npx: not found" -- no run could resolve regardless of model output. It reached the model too: the session transcripts show 12 "npm: not found" failures, with gitnexus/scripts/build.js dying on `npm ci` with status 127. Binds Path(node_bin).resolve().parent.parent read-only at /opt/claude/nodejs, a fresh target outside the already-read-only trees (same constraint that put SANDBOX_NODE under /opt/claude), and adds its bin/ to SANDBOX_PATH. The bind is skipped when the prefix already sits inside /usr, /bin, /lib or /lib64, so the already-covered case does not widen the mount surface redundantly. SANDBOX_NODE is deliberately unchanged -- sanitized_graph.py and runner_sessions.py invoke it directly. SANDBOX_PATH is now derived from SANDBOX_NODE_PREFIX so the two cannot drift, and the minimal-mounts probe asserts against the constant instead of a duplicated literal. The real-Bubblewrap npx canary lives in test_proposer_sandbox.py deliberately: test_workflow_bench.py pins the set of files carrying the canary marker, and it runs in the eval-containment-linux job, where actions/setup-node also installs into the tool cache -- so the canary exercises the real failure shape. Combines plan steps 3-5 into one commit: the mount, SANDBOX_PATH and the pinned probe assertion are one behavioural change, and splitting them would leave a commit whose asserted PATH disagrees with the mounted reality. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(eval): only bind a verified node prefix, and stop excluding .claude/agents Addresses two findings from the branch review of the two preceding commits. 1. The prefix was derived as Path(node_bin).resolve().parent.parent with no check that the layout is really <prefix>/bin/node. Probed: /opt/bin/node bound ALL of /opt (every tool cache on a hosted runner), /mnt/tools/node bound /mnt, and a bare <dir>/node bound <dir>'s parent. That last shape is not hypothetical -- the pre-existing real-Bubblewrap node canary builds exactly it (tmp_path/toolcache/node), so eval-containment-linux would have silently read-only mounted the whole pytest tmp_path inside a containment test, passing while doing it. This function exists to keep the sandbox surface minimal, so an unrecognized layout now binds nothing extra and simply leaves npx unavailable, exactly as before the mount was added. 2. CLAUDE_BOOTSTRAP_ENTRIES also excluded "agents" and "commands" on the theory that they might appear nested too; only .cc-writes ever was observed. Every excluded name is a blind spot: once a .claude directory exists (gitnexus/.claude/settings.local.json is tracked) anything written under an excluded entry is invisible to the phase-boundary check, and Claude Code loads .claude/agents relative to its cwd -- which these tasks point at gitnexus/. Probed: a planning phase could plant gitnexus/.claude/agents/planted.md with the check reporting nothing, then the work phase reads it. Narrowed to .cc-writes alone; extend the set from an observed failure, never pre-emptively. Re-probed after both fixes: the over-broad mounts are gone while a genuine tool-cache prefix carrying npm still binds; planted agents/commands content is caught again; gitnexus/.claude/.cc-writes (the real run-29861768554 failure) stays ignored; and edits to gitnexus/.claude/settings.local.json are still caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(eval): gate the node-prefix bind on a working npx, not on an npm directory The guard tested (prefix)/lib/node_modules/npm as a proxy for "this prefix supplies npx". Test the property actually required instead: a working npx sitting beside node in a real bin/ directory. .exists() follows the symlink, so a dangling npx correctly fails the check -- it would not survive the mount either. The "bin" name requirement stays, because it is what keeps the parent.parent derivation honest; an npx sitting directly beside node in a flat directory would make that derivation name the wrong prefix. This matters because the guard can silently disable the fix it guards: if a runner's layout failed the proxy check, the prefix would not be bound and npx would still be missing, reproducing the original failure with no signal. Testing npx directly means the guard can only pass when the bind will actually achieve its purpose. Validated against a real extracted Node distribution (the official nodejs.org tarball layout that actions/setup-node unpacks into the tool cache) staged at a tool-cache-shaped path: bin/node is a real file, bin/npx resolves to ../lib/node_modules/npm/bin/npx-cli.js, and the prefix binds while SANDBOX_NODE is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 08:25:46 +01:00
dependabot[bot]	c35403b59d	chore(deps)(deps): bump js-yaml from 4.3.0 to 5.0.0 in /gitnexus (#2618) ... * chore(deps)(deps): bump js-yaml from 4.3.0 to 5.0.0 in /gitnexus Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.3.0 to 5.0.0. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](https://github.com/nodeca/js-yaml/compare/4.3.0...5.0.0) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * fix(spring-config): migrate YAML parsing to js-yaml 5 event API js-yaml 5 removed the loadAll `listener` callback, the EventType/State types, and DEFAULT_SCHEMA that spring-config relied on, breaking the build. Rebuild the per-key line tree from parseEvents()/constructFromEvents() (positions are source offsets → mapped to lines), apply the `<<` merge tag via CORE_SCHEMA.withTags(mergeTag) (CORE alone leaves merge keys unexpanded), and resolve aliases by anchor name, which lets the object-identity WeakMap go. Behavior preserved: 9 unit + 8 integration spring-config tests pass, including merged-key declaration-line, cyclic-alias termination, and the depth budget. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(spring-config): restore v4 tag coverage, cover the v5 rewrite with tests Review follow-up for #2618. CORE_SCHEMA.withTags(mergeTag) was a narrowing, not a port: js-yaml 5 throws "unknown tag" on !!timestamp/!!binary/!!set/!!omap/!!pairs, and an unknown tag aborts the whole parse, which readConfigKeys swallows — so an application.yml using any of them would have gone from its full key set to zero keys, silently. Carry the rest of what DEFAULT_SCHEMA was; none of these tags can execute code. Add tests for every path the review flagged as uncovered: multi-document files, empty/comment-only/bare-`---`/bare-scalar documents, sequence-form merge keys, and explicitly tagged values (which fail against the one-tag schema, so they target the changed line). Clear the anchor map per document. It cannot change output today — constructFromEvents rejects a cross-document alias before the event tree is built, now asserted — but it keeps both layers on YAML's scoping rule. Drop the stale @types/js-yaml devDependency; js-yaml 5 ships its own types and tsc --noEmit is clean without it. Lockfile hand-edited because npm uninstall also strips every libc field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(spring-config): flatten !!set members, walk YAML iteratively Review follow-up for #2618. js-yaml 5 constructs `!!set` as a native Set; v4 built a plain `{member: null}` object. Object.entries of a Set is empty, so a tagged set collapsed to a bare leaf key and lost every member. Enumerate the Set instead. Sets arrive as mapping events with key/value scalar pairs, so member lines resolve through the usual lookup. !!binary and !!timestamp are unaffected — both are scalar events and take the leaf path, which is why a Uint8Array never explodes into one key per byte. Convert findYamlMappingLocation and flattenYamlValue from recursion to an explicit stack. Children are pushed in reverse so pops happen in declaration order, preserving "first match" and `out` insertion order; `leave` frames release the cycle guard where the old `finally` did. The depth budget still throws at the same boundary with the same message. Cover the gaps the review named: !!pairs (both duplicate entries survive), anchor-name reuse resolving to the nearest preceding declaration, and marker-only leading documents staying index-aligned across the two streams. buildYamlEventTree keeps no node budget by design — one node per event over an already-materialized array, bounded by MAX_CONFIG_FILE_BYTES. The docstring now says so rather than implying MAX_YAML_TRAVERSAL_NODES covers it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>	2026-07-22 07:50:51 +01:00
ArgonarioD	6150a793e8	docs(cli): mention .agents/skills/ mirror in --skip-skills help + test ... Address review finding (LOW — docs/help staleness): the --skip-skills help text and README omitted that skills also mirror to .agents/skills/ when .agents/ exists. - index.ts + i18n (en/zh): --skip-skills now reads "directly under .claude/skills/ and .agents/skills/". - skip-git-cli.test.ts: assert the help text covers .agents/skills/. Co-Authored-By: Claude <noreply@anthropic.com>	2026-07-22 11:24:30 +08:00
ArgonarioD	38d0256474	Merge remote-tracking branch 'upstream/main'	2026-07-22 11:00:10 +08:00