mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
1295 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gergo Magyar
|
6fed52e1a6 |
fix(cfg): synthesize a protected block for an empty Kotlin try {} (#2195)
An empty `try {}` body produced zero protected blocks, so visitTry's throw-edge
loop wired nothing to the catch and the try's entry fell through to the finally
— leaving the catch handler block + its error binding orphaned (unreachable from
ENTRY), a malformed CFG with stranded def/use facts. Mirror the existing
empty-`catch` synthesis: when the try body is empty and there is a catch or
finally, synthesize one protected block so the catch handler(s) are wired and
the try entry is the body, not the finally. Found by the per-language CFG
verification swarm. Non-empty try is byte-identical; kotlin suite 31 passed,
tsc clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
5d284b8f5f |
fix(cfg): wire Swift multi-catch throw edges to every handler (#2195)
visitDo routed the protected body's throw edge only to handlerEntries[0], so a
do { try r() } catch A {} catch {} left the 2nd..Nth catch handlers UNREACHABLE
from ENTRY — orphaned blocks whose error bindings + def/use facts were stranded
in a dead component (a soundness gap for idiomatic Swift typed multi-catch).
Swift tries the catch clauses in order and the thrown type is unknown at CFG
time, so every protected block may reach ANY clause: edge each protected block
to every handlerEntry. Found by the per-language CFG/CDG verification swarm
(reproduced: 2-catch=1, 3-catch=3 unreachable blocks). swift suite 26 passed,
tsc clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
c97d1b2dfc |
fix(cfg): bind Swift switch-case value patterns (case let n) (#2195)
A switch value-binding (case let n where …, case .some(let v)) was never declared in prescan, so n/v resolved to a synthetic module binding and a body use(n) did not link to any def — a very common Swift idiom silently lost its data dependence. Declare the switch_pattern's bindings (prescan, reusing declarePattern) and emit them as MAY-defs on the dispatch block (a case may not match) via a new switchPatternFacts, propagated into the case body. swift suite 25 passed, tsc + grammar gate green. (The rare ?? / ternary-arm may-def — Swift assignment-as-expression — remains a separate follow-up.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
324bde0368 |
fix(cfg): harvest all names of a Dart multi-variable declaration (#2195)
`var a = 1, b = 2;` is one initialized_variable_definition whose first binding is the name/value field pair and whose subsequent bindings are trailing initialized_identifier children. Both prescan (declareInitializedVar) and the walkValue case read only the name/value fields, so every name after the first was never declared or def'd — `b` resolved to a synthetic module binding and its REACHING_DEF/taint flow was lost. Iterate the trailing initialized_identifier nodes in both phases. (Dart-3 record/list pattern declarations `var (a,b)=pair` remain a separate follow-up.) dart suite 35 passed, tsc + grammar gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
31936a0929 |
fix(cfg): harvest Go select channel-receive binding as a def (#2195)
walkValue had no receive_statement case, so a select receive (case v := <-ch:) fell to the default descent: v was recorded as a USE of an uninitialized var and the channel-sourced definition was invisible to REACHING_DEF/taint — channels are a primary taint source in Go. Add the case mirroring short_var_declaration: def each left identifier, use the <-ch right, attach resultDefs for the := short form. prescan already declared the binding; this completes the phase-2 fact. go:branchy bench fingerprint unchanged; go suite 40 passed, grammar gate green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
33446d61c3 |
fix(cfg): harvest Kotlin x++/--x as a def (#2195)
The Kotlin harvester had no postfix_expression/prefix_expression case, so an increment/decrement fell to the default descent and recorded its operand as a use only — never a def. Every sibling harvester (Java/C#/C++/Dart/TS/PHP) models inc/dec, so a Kotlin counting loop (while/for using i++) silently dropped the loop-carried reaching-def of the counter. Add the case: def AND use the operand when it is a plain simple_identifier and the operator is ++/-- (other pre/postfix forms — -x, !x, x!!, x? — stay pure reads, byte-identical to the old descent). Characterization tests for postfix + prefix added; kotlin suite 30 passed, grammar gate + bench --check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
a805e8fa6f |
feat(cfg): surface CDG soundness skips at warn, not just debug (#2195)
skippedUnsoundFunctions (a function whose EXIT is not reverse-reachable from all blocks, so control dependence is withheld) was only reported inside the per-language logger.debug stats line — while the taint/RD coverage-gap and cap-drop counts surface unconditionally at warn. A language that systematically trapped EXIT (an unmodeled non-terminating / multi-terminal shape the synthetic-escape pass can't bridge) would silently lose all CDG. Add a parallel unconditional warn (R8) alongside the R4 taint-gap warn. Observability only — no graph change; emit-layer skip counting stays covered by cfg-emit's skippedUnsoundFunctions test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
194c36befc |
fix(cfg): put embedded-script CFGs in file coordinates via lineOffset (#2195)
A Vue SFC <script> block parses at row 0 but lives at lineOffset in the .vue file. Every other worker-emitted graph node adds lineOffset to reach file coordinates, but collectFunctionCfgs built FunctionCfgs from the extracted script's raw rows and never offset them. Two consequences for .vue files: - inter-procedural taint silently resolved NOTHING — the summary-harvest join keys graph Function/Method nodes by their (offset) startLine but looked up the CFG's (unoffset) functionStartLine, missing by exactly lineOffset, so no FunctionSummary was ever produced; - persisted BasicBlock startLine/endLine (and the id's functionStartLine segment) pointed at the wrong .vue line, breaking source mapping. Thread lineOffset into collectFunctionCfgs and shift every CFG source-line field (functionStartLine/End, block start/end, statement + non-synthetic binding lines) into file coordinates at the one production chokepoint. A 0 offset returns the CFG unchanged, so .ts/.js/etc. stay byte-identical (bench --check fingerprints unchanged; worker-roundtrip + pipeline-pdg green). Unit tests for the shift + the 0-offset no-op added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
8a5478c6f0 |
fix(cfg): harvest C# out-var and deconstruction-declaration defs (#2195)
Two idiomatic C# write shapes recorded ZERO defs, silently breaking REACHING_DEF/taint: - out-var (G(out var n) / G(out int n)) parses as a declaration_expression; it was neither declared (phase 1) nor def'd (phase 2), so n resolved to a synthetic module binding and the callee-written value had no reaching def. - deconstruction declaration (var (a, b) = T()) has a variable_declarator whose name slot is a tuple_pattern (null name field), so declareVariableDeclaration + the variable_declaration walk skipped it entirely (only the assignment form (a,b)=T() was handled). Both a and b were dropped. Declare + def the declaration_expression's identifier (must-def: out params are definitely-assigned), and route a null-name variable_declarator through the tuple_pattern via the existing declareForeachTarget/defTupleTargets helpers. Characterization tests added; csharp suite 42 passed, grammar gate + bench --check green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
a654c1c764 |
fix(cfg): model C++ co_return as a return terminator to EXIT (#2195)
co_return_statement was neither in CPP_CONTROL_FLOW_TYPES nor dispatched, so a coroutine's co_return coalesced into a straight-line block and emitted a spurious seq fallthrough to the following statement instead of an edge to EXIT — statements after co_return looked reachable and the terminator edge was missing, corrupting CFG/CDG for coroutines. Add the node type to the C++ control-flow set and dispatch it through visitReturn (block -> EXIT 'return', no fallthrough). C path untouched; co_await/co_yield remain plain expressions. Characterization test added; c-cpp suite + grammar-literal gate green, bench --check unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
5165d71d7b |
fix(cfg): harvest C++ structured-binding defs (auto [a,b]=e) (#2195)
The C++ def/use harvester only recorded a def when an init_declarator's declarator was a plain identifier, so a structured binding (auto [a,b] = mk(), incl. the auto& reference form whose binding sits under a reference_declarator) declared only the first name in phase 1 and emitted ZERO defs in phase 2 — a,b were walked as spurious uses and later use(a)/use(b) resolved to a synthetic module binding, silently corrupting REACHING_DEF/taint for an idiomatic C++17 shape. Unwrap the structured_binding_declarator in both phases and def every identifier leaf; result-of-initializer flows to the whole list. Inert for C (no structured bindings). Characterization tests added (plain + reference form). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
213ae2526e |
style(cfg): apply prettier + drop unused imports across the PDG files (#2195)
The merge of main into this branch pulled in the stricter quality gates (prettier --check . and eslint .), which surfaced pre-existing formatting in the PDG/CFG rollout (line-width wrapping across the visitor + harvest files, bench, tests) plus 9 no-unused-imports errors. Mechanical autofix only — npm run format + lint:fix equivalent, scoped to gitnexus/: removes unused FunctionCfg/ SiteRecord type imports left by the U8 helper consolidation and stale FinalizerFrame imports in python.ts/ruby.ts. No behavior change: tsc clean, cfg unit suite 617 passed, eslint 0 errors. (gitnexus-web class-order noise is a local tailwind-plugin artifact CI does not flag — left untouched.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
8b704aa9a3 |
ci(codeql): exclude nested test fixtures from the CodeQL gate (#2195)
The CodeQL results gate failed on test/integration/cfg/fixtures/python-hazards.py
('total' may be used before init, unused vars) — but that file is an intentional
CFG/PDG hazard fixture, exactly the synthetic broken-code the existing
'**/test/fixtures/**' exclusion is meant to skip. That glob does not match the
deeper test/integration/cfg/fixtures/ path, so the hazard fixtures leaked into
the scan. Add '**/test/**/fixtures/**' to cover fixtures nested anywhere under a
test tree. Analyze (python) and Analyze (javascript-typescript) both already pass
— production code is clean; this only silences fixture noise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergő Magyar
|
bbb49e1c47
|
Merge branch 'main' into feat/pdg-language-visitors | ||
|
Gergo Magyar
|
da1154f53e |
feat(cfg): defensive per-statement cap on harvested taint sites (#2195 U11)
A statement's harvested sites[] had no explicit bound — a pathological or machine-generated statement (hundreds of nested calls) could grow it without limit. Add DEFAULT_PDG_MAX_SITES_PER_STATEMENT (512, mirroring the PDG edge/fact cap style): openCallSite/addMemberRead check-before-push and stop at the cap, keeping the first 512 sites fully intact and setting an observable sitesTruncated flag. A cap-dropped openCallSite returns a -1 sentinel that pushFrame/setSite*/the occurrence fan-out all tolerate (no dangling parent/via, no clobber of kept sites). Generous enough that no real statement is affected: bench --check fingerprints unchanged, cfg unit suite 617 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
85d25c87cf |
test(cfg): pin *-harvest.ts literals to their own grammar in the gate (#2195 U10)
The grammar-literal validation gate scans cfg/visitors/, but a <lang>-harvest.ts basename was not in BASENAME_LANGS, so fileLanguages() fell it through to the weak ALL_LANGS valid-if-any bucket — a node-type literal dead in its own grammar but valid in some other grammar would pass undetected. Strip the -harvest suffix and reuse the visitor basename map so go-harvest -> Go, c-cpp-harvest -> C+C++, typescript-harvest -> TS, etc. The two language-agnostic harvesters (call-site-harvest, scope-tree-harvest) name no grammar and stay valid-if-any. Also corrects the now-inaccurate mode2Files comment. Adds a fileLanguages unit test; the existing gate stays green (no harvest file has a dead literal), and a scratch probe confirmed a bogus go-harvest literal is now caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
f47e952ba1 |
test(cfg): consolidate copied visitor-test helpers into cfg-harness (#2195 U8)
The 13 *-visitor.test.ts files each copied a byte-identical set of CFG-shape helpers (edgeKinds/block/reaches/reachable/bindingIdx/allSites/hasAnySites, ~380 lines total). Export them once from test/helpers/cfg-harness.ts and import per file (only the subset each references). Also drop each file's local exitReachableFromAll — a re-implementation of the production isExitReachableFromAllBlocks (semantically identical: false iff some entry-reachable non-EXIT block can't reach EXIT) — and point its live call sites at the already-imported production function. Pure test-only mechanical move, behavior-preserving: tsc clean, test/unit/cfg/ 613 passed unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
ccbfef2eb6 |
refactor(cfg): consolidate no-site def/use accumulator into DefUseAccumulator (#2195 U7)
The Kotlin/Python/Ruby/Rust/Dart/Swift harvesters each carried a byte-identical copy of the no-site def/use accumulator (~270 lines total; only Ruby's adds the live useCount() emit-guard helper). Extract it as an exported DefUseAccumulator beside CallSiteFactAccumulator in call-site-harvest.ts (the PR's own model for the with-site superset); the six harvesters import it under their existing local FactAccumulator name. Pure byte-equivalent move, no logic change: cfg unit suite 613 passed, tsc clean, bench --check fingerprints unchanged (TS/Go paths untouched). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
7ebd579524 |
refactor(cfg): consolidate scope-tree substrate into ScopeTreeHarvester (#2195 U6)
The Go/Java/C#/C-C++ def/use harvesters each carried a byte-identical copy of the lexical scope-tree machinery (Scope record, two-phase resolution cache, openScope/nearestScopeOf/resolve/def/use/conditional/bindingTable, ~270 lines total). Extract it into an abstract ScopeTreeHarvester base; the four harvesters now extend it and supply only their genuine per-language variation (the prescan switch, plus Go's _-blank-identifier overrides of declare/def/use). Net -422 lines. Mechanical and byte-equivalent: cfg unit suite 613 passed, bench --check fingerprints unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
7192036183 |
refactor(cfg): standardize harvester API table()->bindingTable() (#2195 U9)
The binding-table accessor was named table() in the C/C++/C#/Go harvests but bindingTable() in the other 7. Rename the 4 (definitions + their visitor call sites) to the majority name bindingTable(). Pure rename; the 4 visitor suites stay green and tsc confirms no call site was missed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
fdaba53229 |
refactor(cfg): remove dead useCount() from swift + rust harvests (#2195 U5)
useCount() was declared on the local FactAccumulator in swift-harvest.ts and rust-harvest.ts but never called (a copy-paste artifact; ruby's copy IS used in an emit guard, so it stays). Pure deletion — the swift/rust visitor suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
d945f61290 |
test(cfg): gate vendored-grammar worker assertions on isLanguageAvailable (#2195 U4)
The Swift/Kotlin/Dart worker-mode pipeline-pdg cases require a vendored grammar prebuild that may be absent on a CI platform — they'd go red there. Mark those three REMAINING_LANGS entries `vendored` and gate both the --pdg-on and --pdg-off `it`s on isLanguageAvailable(SupportedLanguages [lang]) → it.skip when the grammar can't load. Installed-grammar languages stay unconditional. Grammars present here, so all 30 run green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
3f17cb7fd5 |
test(cfg): isolate the non-terminating-loop hazard in worker CDG asserts (#2195 U3)
The pipeline-pdg worker-mode blocks asserted a whole-fixture cdg>0 aggregate (satisfied by any branching fn) while the comment claimed it proved the non-terminating-loop EXIT-reachability end-to-end. Add a per- language `hazard` marker + isolate the assertion: locate the hazard function's BasicBlocks by its anchor and assert >=1 CDG edge is sourced within it (a marker mutation now fails the test — non-vacuous). C# keeps the aggregate (its fixture has no infinite loop). Comments corrected. Switch the 7 visitor unit tests (java/csharp/dart/kotlin/php/swift/c-cpp) from the local exitReachableFromAll CFG-shape helper to the production isExitReachableFromAllBlocks + computeControlDependence on the hazard function, matching go/python/ruby/rust/vue. 241 unit + 30 pipeline green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
bbee2da92e |
fix(cfg): synthetic-escape pass restores CDG for exit-unreachable cycles (#2195 U1)
Unconditional goto-cycles (C/C++/C#/Go) wire a backward seq edge with no
structural exit-escape edge, so EXIT becomes non-reverse-reachable and
emitFileCdg silently skipped ALL control-dependence for the function.
New cfg/synthetic-escape.ts: a pure deterministic SCC routine (iterative
Tarjan, sorted adjacency) + augmentForPostDom(cfg). No-op when EXIT is
already reverse-reachable (terminating fns + visitor-escaped loops are
byte-identical — returns the same object). Otherwise it batch-bridges
every exit-less SCC by adding an ANALYSIS-ONLY escape edge from the SCC's
controlling block (highest out-degree branch; lowest-index tie-break) to
EXIT, on a shallow-cloned FunctionCfg — never mutating persisted
cfg.edges. emitFileCdg threads that augmented view through BOTH
isExitReachableFromAllBlocks AND computeControlDependence (the Ferrante
walk re-reads cfg.edges, so a tree-only augmentation would be wrong).
Precision (anti-masking): only a trapped region containing a control
point (>=2-successor block) is bridged — a branch-less trapped region
carries no recoverable control-dependence and is indistinguishable from a
genuine construction anomaly, so it stays on the skip path (the existing
disconnected-block skip test still skips, skippedUnsoundFunctions===1). A
residual non-cycle dangling block is never bridged.
repro `void handler(int a){ start: if(a>0){work();} goto start; }`:
before exitReachable=false/CDG=0 → after one synthetic 2->1 edge,
exitReachable=true, exact CDG = {2->2:T,2->2:F,2->3:T,2->4:T,2->4:F}
(pinned exactly, not CDG>0 — catches a wrong representative). AC2 property
test extended to the augmented graph; per-language goto-cycle regressions
(C/C++/C#/Go). 199 cfg tests green; bench --check fingerprints unchanged
(analysis-only, zero persisted drift).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
aa5edf12ab |
fix(cfg): surface skippedUnsoundFunctions in per-language stats (#2195 U2)
emitFileCdg computes skippedUnsoundFunctions (functions whose CDG is withheld because EXIT isn't reverse-reachable from all blocks) but run.ts dropped it on the floor — only cdgEdges/cdgDropped were aggregated. Add the aggregation + a stats-line segment so CDG coverage gaps are an explicit signal, not silent. Establishes the baseline skip count that makes the U1 synthetic-escape pass's effect (the drop to genuine anomalies only) measurable. Additive; no emit-logic change. The emit-side field is covered by cfg-emit.test.ts (asserts skippedUnsoundFunctions===1 + the warn on a disconnected-block CFG); the run.ts aggregation is a thin pass-through. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
417cc67a39 |
feat(cfg): Vue (reuse TS visitor) + worker-mode proof for all langs (#2195 U15)
Vue SFC <script> blocks are extracted and parsed with the TS grammar (parse-worker languageMap[Vue] = TypeScript.typescript), so wire vueProvider.cfgVisitor = createTypeScriptCfgVisitor() -- pure reuse, no Vue-specific visitor. vue-visitor.test.ts replicates the worker path (extractVueScript -> TS parse -> CFG) and confirms branch edges + EXIT reverse-reachable + CDG>0. Extend pipeline-pdg.test.ts with a worker-mode block covering all eight remaining languages (Python/PHP/Ruby/Rust/Swift/Kotlin/Dart/Vue): per- language temp repo, real worker pool, BasicBlock+CFG+REACHING_DEF+CDG all > 0 with --pdg (CDG>0 proves EXIT reverse-reachable end-to-end through the worker despite each fixture's non-terminating loop), == 0 without. Counts e.g. Ruby 122 BB/45 CDG, Vue 49 BB/11 CDG. 30 pipeline tests green. COBOL: documented as the deliberate PDG non-goal (no grammar, exotic PERFORM/GO-TO control flow) in cobol.ts + the worker-roundtrip gate. This completes PDG language coverage: every supported language except COBOL now builds CFG/REACHING_DEF/CDG under --pdg. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
2071cadf1e |
feat(cfg): Dart CFG visitor + def/use harvest (#2195 U14)
Add createDartCfgVisitor + dart-harvest (vendored tree-sitter-dart): if/else, C-for/for-in/while/do-while, switch (empty-case fallthrough + explicit continue-label) + switch_expression, try/on/catch/finally + rethrow + assert (throw edges), return/break/continue/throw, labeled loops, arrow bodies, closures. Dart splits a function into sibling signature + function_body nodes, so the body (or function_expression) is the CFG-bearing node. Wire into dartProvider. Every literal validated against the vendored grammar via the probe (only constant_pattern exists; removed speculative relational/logical pattern names). while (true) keeps EXIT reverse-reachable (production CDG probe: 3 edges). 34 real-parser tests; comprehensive sweep green (605). Gaps: labeled-loop grammar quirk (read via ERROR sibling), async straight-line, value-position if/switch inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
b302c63544 |
feat(cfg): Kotlin CFG visitor + def/use harvest (#2195 U13)
Add createKotlinCfgVisitor + kotlin-harvest (vendored tree-sitter-kotlin): if/else, when (subject + subjectless, no fallthrough), for/while/do-while, try/catch/finally, jump_expression (return/return@/break/break@/continue/ continue@/throw), labeled loops, control_structure_body unwrapping, expression-body functions. The grammar is field-less for control flow, so the visitor navigates by child type+position. Wire into kotlinProvider. Every literal validated against the vendored grammar via the probe (line_comment/multiline_comment, not comment). while (true) keeps EXIT reverse-reachable (production CDG probe: 3 edges; worker-mode fixture: BB=82, CDG=41). 28 real-parser tests; comprehensive sweep green (571). Gaps: value-position if/when/try inline, inline-fun non-local return, getters/setters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
0e137cce27 |
feat(cfg): Swift CFG visitor + def/use harvest (#2195 U12)
Add createSwiftCfgVisitor + swift-harvest (vendored tree-sitter-swift via requireVendoredGrammar): if/else + optional binding (if let), guard...else (diverging early exit), for-in/while/repeat-while (bottom-test), switch (no implicit fallthrough; explicit fallthrough keyword; where guards), do/catch + try/try?/try!, defer (LIFO finalizer at scope exit), labeled break/continue, control_transfer_statement (one node for break/continue/ return/throw). Wire into swiftProvider. Every literal validated against the vendored grammar via the probe (no block node; if-let folds into condition+bound_identifier; defer parses as a call_expression with trailing closure). while true keeps EXIT reverse-reachable (production CDG probe: 3 edges). 24 real-parser tests; comprehensive sweep green (543). Gaps: computed properties, defer block-scope approx, fatalError traps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
82159c93d4 |
feat(cfg): Rust CFG visitor + def/use harvest (#2195 U11)
Add createRustCfgVisitor + rust-harvest for the expression-oriented Rust:
if/else + if-let, loop (infinite -- structural escape edge), while/
while-let/for, match (no fallthrough) + guards, labeled break/continue
('outer), break-with-value, ? operator (try_expression) as an
early-return throw edge to EXIT, let-else (diverging else). visitLet
handles control-flow in value position (let x = loop/if/match). Wire into
rustProvider.
Every literal validated against tree-sitter-rust via the probe (label is
a named child not a field; line_comment; _ pattern). loop {} keeps EXIT
reverse-reachable (production CDG probe: 3 edges). 33 real-parser tests;
comprehensive sweep green (519). Gaps: panic, async/.await, macro bodies.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
ad0a50271e |
feat(cfg): Ruby CFG visitor + def/use harvest (#2195 U10)
Add createRubyCfgVisitor + ruby-harvest: if/unless/elsif/else + statement-modifier forms (x if c, x while c), while/until/for (until inverts the sense), case/when + case/in (pattern, no fallthrough), begin/rescue/else/ensure (ensure=finally, rescue=catch) + retry (loop-back into begin), return/break/next/redo, blocks/lambdas as their own closure CFGs. Wire into rubyProvider. Every literal validated against tree-sitter-ruby via the probe (case vs case_match, modifier nodes, typed rescue/ensure children). loop do / while true keep EXIT reverse-reachable (production CDG probe: 3 edges). 34 real-parser tests; comprehensive sweep green (486). Gaps: yield, expression-position if/case/begin inline, ivar/gvar non-local defs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
5e72cfe57a |
feat(cfg): PHP CFG visitor + def/use harvest (#2195 U9)
Add createPhpCfgVisitor + php-harvest: if/elseif/else (+ alt colon syntax), for/foreach/while/do-while, switch (fallthrough) + match (no fallthrough), try/catch/finally, break N/continue N (N-th enclosing loop), goto, return/throw. Wire into phpProvider. Every literal validated against tree-sitter-php (php_only) via the probe (for_statement initialize/condition/update; throw_expression not throw_statement; break/continue integer child). while(true) keeps EXIT reverse-reachable (production CDG probe: 3 edges; break 2 escapes the outer loop). 35 real-parser tests. Also repoint worker-roundtrip's "non-CFG language" gate test from Python (which now has a cfgVisitor) to COBOL (the permanent non-goal of the rollout) -- a stale assertion the Python commit invalidated. Full in-process sweep green (452 across 18 files). Gaps: match inline value, goto plain-block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
640d326a01 |
feat(cfg): Python CFG visitor + def/use harvest (#2195 U8)
Add createPythonCfgVisitor + python-harvest -- the most structurally divergent target (indentation blocks, elif, for/while-else, with, try/ except/except-group/else/finally, match/case, comprehensions, walrus), confirming the shared CfgBuilder/ControlFlowContext core carries no brace-family assumptions. for/while else-clause sits on the normal- completion edge (not break); with modeled as try/finally dispose; match has no fallthrough. Wire into pythonProvider. Every literal validated against tree-sitter-python via the probe. while True: keeps EXIT reverse-reachable (production CDG probe: 3 edges; fixture: 42 CDG edges). 37 real-parser tests; gate green; no regression (cfg unit 391, tsc clean). Gaps: async/generator suspension, comprehension scope over-approximation. No sites[] (taint substrate, separate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
abcae87cbd |
test(cfg): worker-mode PDG integration + bench parameterization (#2195 U7)
Prove the five C-family visitors build PDG through the REAL worker pipeline. pipeline-pdg.test.ts: per-language (C/C++/C#/Java/Go) temp repo run with pdg:true asserts BasicBlock+CFG+REACHING_DEF+CDG all > 0 (CDG>0 proves EXIT stays reverse-reachable end-to-end through the worker, incl. each fixture's non-terminating loop/select); a paired run with pdg off asserts == 0, the two flag-off graphs byte-identical (R3), no PDG types leak, pinned by a golden snapshot. Counts e.g. Go 151 BB / 56 CDG. Parameterize bench/cfg/measure.mjs by a per-language LANGS registry resolved generically via getLanguageGrammar + getProvider(X).cfgVisitor (no static import table). Default TS byte-identical -- all 6 TS fingerprints unchanged under --check; taint-dense stays TS-only (TS_JS_TAINT_MODEL never runs against model-less C-family CFGs). Add a go:branchy scenario+baseline (namespaced) -- its fingerprint shape (32 blocks/46 edges) matches TS branchy, cross-validating the Go visitor. 15 pipeline tests + bench --check PASS (7 scenarios); 354 unit cfg green; dist rebuilt clean. Absorbs the bench parameterization deferred from U1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
b294a387a5 |
feat(cfg): call-site sites[] taint substrate for C-family (#2195 U6)
Extend the C/C++/C#/Java/Go harvests with the call-site sites[] taint substrate (SiteRecord/SiteArgOccurrence), mirroring the TS shape so the shared taint matcher consumes all languages uniformly. Extract the grammar-agnostic site machinery into cfg/visitors/call-site-harvest.ts (CallSiteFactAccumulator -- names no language); each harvest adds only its per-grammar visitCall/walkChain over its call node (C/C++ call_expression, C# invocation_expression, Java method_invocation, Go call_expression). INERT BY DESIGN: no C-family taint model exists (registerBuiltinTaintModels is TS/JS only), so getSourceSinkConfig returns undefined for these languages and the harvested sites produce ZERO TAINTED edges -- the positive source->sink->TAINTED path is deferred with the model authoring. sites emitted only when non-empty; facts-only attachment, block/edge topology unchanged (pre-existing topology + def/use tests byte-identical). 23 new substrate tests; 574 green across the cfg/taint/emit suites; gate green; tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
fff2c1489a |
feat(cfg): Go CFG visitor + def/use harvest (#2195 U5)
Add createGoCfgVisitor + go-harvest, the highest-divergence target:
for_statement (all four shapes -- for_clause C-style, while-style,
range_clause, bare for{}), expression/type switch (no implicit
fallthrough) + explicit fallthrough_statement, select_statement, defer
(LIFO finalizer legs at function exit), go (call is straight-line; the
closure body is its own CFG via isFunction), labeled break/continue/goto,
multiple-return assigns (a, b := f() defines each LHS). Wire into
goProvider.
CRITICAL (review A2): every non-terminating shape -- for{}, for cond{},
select{} with no default -- emits a structural exit-escape edge so EXIT
stays reverse-reachable and the production CDG is not silently skipped.
Verified: for{} -> CDG=3, select{} -> CDG=1, for-range -> CDG=2, all
exitReachable=true.
Every literal validated against tree-sitter-go via the probe. 32
real-parser regression tests; grammar-literal gate green; no regression
(186 across all 5 visitors + gate, full cfg unit 331, tsc clean).
Documented gaps: panic/recover unwind, goroutine happens-before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
6dbe7373ff |
feat(cfg): Java CFG visitor + def/use harvest (#2195 U4)
Add createJavaCfgVisitor + java-harvest over the shared CfgBuilder / ControlFlowContext: if/else, classic for, enhanced-for, while, do-while, classic-vs-arrow switch (switch_block_statement_group fallthrough vs switch_rule no-fallthrough), try/catch/finally + try-with-resources (auto-close synthesized as a finalizer, closes on normal AND exception exit) + synchronized (monitor-release finalizer), labeled break/continue to the labeled frame, yield, return/throw/break/continue. Wire into javaProvider. Every literal validated against tree-sitter-java via the probe (switch_expression covers both switch forms, generic_type, line_comment, for init field). Edge kinds match the contract; functionStartColumn populated; while(true)/for(;;) keep EXIT reverse-reachable (production CDG probe: 3 edges; hazard fixture: 34 CDG edges). buildFunctionCfg returns undefined rather than throwing. 43 real-parser regression tests; grammar-literal gate green; no regression (cfg unit suite 304, tsc clean). Documented gaps: switch-as- expression-value inline, yield state machine, async/field-write defs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
aad3a702b9 |
feat(cfg): C# CFG visitor + def/use harvest (#2195 U3)
Add createCsharpCfgVisitor + csharp-harvest over the shared CfgBuilder / ControlFlowContext, modeling the C# statement taxonomy: if/else, for/foreach/while/do, switch_section (+ switch_expression arms), try/catch/catch_filter/finally, using + lock (deterministic finalizers -- dispose/release runs on normal AND exception exit, finally-* completion edges on crossing jumps), goto/labeled, yield (surface only), return/ throw/break/continue. Wire into csharpProvider. Every literal validated against tree-sitter-c-sharp via the introspection probe (record_declaration, no else_clause, switch_section, positional access where no field exists). Edge kinds match the contract; functionStartColumn populated; while(true) keeps EXIT reverse-reachable (production CDG probe: 3 edges). buildFunctionCfg returns undefined rather than throwing. 34 real-parser regression tests; grammar-literal gate green; no regression (cfg unit dir 256/256, tsc clean). Documented gaps: yield iterator state machine, goto case/default, async suspension points. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
018635074a |
feat(cfg): C and C++ CFG visitor + def/use harvest (#2195 U2)
Add createCCfgVisitor/createCppCfgVisitor over a shared CCfgWalk core. Grammar introspection confirmed tree-sitter-c and tree-sitter-cpp share every control-flow node type/field, so CppCfgWalk extends CCfgWalk with only the C++-only nodes (try/catch/throw/for_range_loop/lambda) via a visitExtra hook -- no language conditionals (AGENTS no-language-naming). Wire both into c-cpp.ts providers. Harvest (c-cpp-harvest.ts): two-phase binding table + per-statement defs/uses/mayDefs (no sites[] yet -- U6). Edge kinds match the TS contract; functionStartColumn populated; non-terminating loops (for(;;), while(1)) emit the structural exit-escape edge so EXIT stays reverse-reachable and CDG is not silently skipped -- verified against the production post-dominator + control-dependence solvers (for(;;) -> 3 CDG edges). buildFunctionCfg returns undefined rather than throwing. 23 real-parser regression tests; grammar-literal gate green (literals validated against both grammars). Documented gaps: C++ RAII destructors, setjmp/longjmp, computed goto (route to EXIT + warn). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergo Magyar
|
bc7cc3ffc8 |
test(cfg): language-agnostic CFG unit-test harness (#2195 U1)
Extract the grammar-agnostic engine from ts-cfg-harness into
makeCfgHarness(grammar, visitor, filePath) at test/helpers/cfg-harness.ts.
Function discovery delegates to visitor.isFunction, so the harness carries
no language-specific node-type knowledge -- each C-family visitor's unit
tests can drive the real worker-side builder against real source.
ts-cfg-harness becomes a thin TS binding re-exporting the same
parse/collectFunctions/cfgOf/cfgsOf (behavior-preserving: all 5 existing
consumers -- taint propagate/model-match/summary-harvest/taint-emit + cfg
harvest -- pass unchanged, 223 tests green). New harness.test.ts proves
TS-faithfulness and isFunction-delegation via a stub visitor.
The bench parameterization (measure.mjs) is sequenced into U7, where the
first C-family scaling scenario makes the {grammar, visitorFactory} seam
validatable against a real non-TS language.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Gergo Magyar
|
5439c363de |
test(cfg): validate cfg/visitors literals + drop 3 dead TS node types
Extend the grammar-literal CI gate (test/helpers/literal-collectors.ts) to scan cfg/visitors/*.ts, mapping each visitor file to its grammar via the existing basename rule (c-cpp -> C/C++, csharp -> C#, java -> Java, go -> Go, typescript -> TS). Closes the gap where the gate never validated CFG visitor node-type literals -- the prerequisite for adding C-family visitors safely (#2195 U1). The newly-scanned TS visitor surfaced 3 dead literals absent from every grammar it serves (typescript/javascript/tsx all = 0): for_of_statement (for-of parses as for_in_statement), async_function_declaration and async_arrow_function (async functions are function_declaration / arrow_function + an async child). Removed them; behavior-preserving -- the cases never matched, bench --check fingerprints unchanged, TS visitor unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Goutham Krishna Mandati
|
4c73b18387
|
feat(mcp): add trace tool for shortest call path between symbols (#2173)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* feat(mcp): add trace tool for shortest call path between symbols (#1821) Implement the \ race\ MCP tool and \gitnexus trace\ CLI command that finds the shortest directed call path between two symbols using BFS over CALLS + HAS_METHOD edges. - MCP tool definition in tools.ts with READ_ONLY annotations - Directed BFS in local-backend.ts with parent-map path reconstruction - Symbol resolution via resolveSymbolCandidates (name/UID/file-hint) - Gap reporting with furthest reachable node and depth tracking - CLI wiring: gitnexus trace <from> <to> [--from-uid] [--to-uid] [--depth] - i18n keys in en.ts and zh-CN.ts + help-i18n.ts registration - ARCHITECTURE.md tools table entry - 16 unit tests (11 BFS core + 5 CLI wiring) * test(mcp): account for trace tool in tools.test.ts count The trace tool makes GITNEXUS_TOOLS length 15; update the hardcoded count, add 'trace' to the expected-names list, and refresh the stale "13 tools" comment and it() title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): sanitize trace maxDepth to reject 0/NaN/negative `Math.min(params.maxDepth ?? 10, 30)` had no lower bound and `??` does not recover 0 or NaN, so `--depth 0|-5|abc` made the BFS loop run zero iterations and return a false `no_path`. Clamp at the real boundary with a `Number.isInteger && > 0` guard (the MCP inputSchema minimum is advisory only), and reject a non-numeric `--depth` in the CLI up front rather than forwarding NaN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): check trace target before applying test-file filter The `isTestFilePath` filter ran before the target-equality check, but resolveSymbolCandidates does not exclude test-file symbols. A target (or a required hop) that lives in a test file was therefore skipped under the default includeTests=false and produced a false no_path with a misleading dynamic-dispatch suggestion. Match the explicitly-requested target first; non-target test-file nodes are still filtered. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(mcp): bound trace BFS with per-level LIMIT and visited cap The per-level query had no LIMIT and the visited set was uncapped, so a high-fanout hub could materialize an unbounded frontier. Cap per-level rows (interpolated LIMIT — Kuzu does not bind LIMIT) and the total visited set; either cap sets a `truncated` flag so a resulting no_path reports that the search was cut short rather than implying the graph was exhausted. Note: the sibling impact BFS shares the same unbounded pattern; applying the cap there is deferred (out of scope for this PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): clarify trace traverses call + class-member edges trace was advertised as a "shortest call path" but also traverses HAS_METHOD (class→member) containment edges so a class-rooted trace can descend into its methods. Keep that capability (consistent with impact/ context) and make the docs honest: rename EDGE_TYPES→TRAVERSAL_EDGE_TYPES, state the call + class-member traversal in the MCP/CLI/i18n/ARCHITECTURE descriptions, and note each hop's edge type is reported in edges[]. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): set status:'error' on trace failure responses Every trace return path sets a `status` discriminator except the caught-error path, so a consumer switching on `result.status` saw undefined on failure. Add status:'error' to both the backend trace() catch and the CLI traceCommand catch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): return a friendly error for non-string trace from/to A non-string from/to reaching resolveSymbolCandidates surfaced a low-level "x.includes is not a function" via name.includes. Guard the four name/uid params at the top of _traceImpl and return a structured status:'error' with a clear message instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): single row-decode + drop dead field in trace BFS Decode each BFS row once into named locals instead of repeating `(row.x ?? row[N])` across the two parent.set calls and the furthest-tracking. Drop the `type` field from the parent map value (it was written but never read), and rename the internal `deepestInfo` to `lastReached` for accuracy (the output field `furthest` is unchanged). Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): dedicated trace includeTests i18n key + guard coverage `trace|--include-tests` reused the impact help key, so rewording the impact option would silently change trace's help text. Add a dedicated help.option.trace.includeTests key in en + zh-CN and repoint it. Add CLI coverage for the (already symmetric) --from-uid/--to-uid flag-value guard and for --include-tests forwarding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): faithful BFS mock + expand trace coverage Fix makeResolveMock: concatenate neighbors across ALL frontier ids (it returned only the first node's, so a multi-node frontier was unmodelled) and key the UID branch on params.uid (the old query-text match never fired). Add coverage: shortest path through the second frontier node (proves the mock fix), confidence floor fallback, HAS_METHOD traversal with a mixed edge-type chain, no_path furthest:null, and from_file disambiguation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(trace): apply root prettier formatting to trace files The root `quality / format` gate (prettier --check, printWidth 100) runs on the full repo and flagged the trace sources/tests (the local config masks it). Reformat to root style — no behavior change; trace + tools suites and tsc stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): document the trace tool for AI agents Add `trace` to the GitNexus skill docs so agents reach for it instead of hand-chaining context/impact hops. The guide gains a Tools Reference row and a "shortest path between two symbols" subsection (params, result shape, status/furthest/truncated semantics); the debugging skill gains a "how does A reach B?" pattern row and a trace tool example. Mirrored to the .claude and claude-plugin copies (byte-identical) and the cursor copy (compact style). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): drop unused trace test fixtures (CodeQL js/unused-local-variable) CodeQL flagged two unused locals in the trace BFS tests: the top-level SYMBOL_C and a SYMBOL_D inside the maxDepth test (both defined, never referenced). Remove them. No behavior change — 58 trace/tools tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Parafee41
|
1967512211
|
fix(cli): preserve trailing spaces in git roots (#2192) | ||
|
Gergő Magyar
|
fb068a9480
|
fix(group): pin repos during sync so large groups resolve cross-links (#2191)
* fix(lbug): pin repos to exempt them from automatic pool eviction [#2189] Add a pinnedRepos set and pinRepo/unpinRepo to the LadybugDB pool adapter. evictLRU and the idle-timeout sweep skip pinned repos; closeOne clears the pin on teardown so explicit close always wins and pins never leak across operations. Behavior is byte-identical when nothing is pinned. Bounded multi-repo callers (group sync) can now keep more than MAX_POOL_SIZE repos resident through deferred cross-repo resolution. * fix(group): pin repos during sync so >MAX_POOL_SIZE groups resolve [#2189] syncGroup now pins each repo immediately after initLbug and releases the pin (unpin then close) in the finally. This keeps every group member resident through the deferred manifest/workspace resolution that runs after the init loop, so cross-links anchor to real graph symbols instead of falling back to synthetic UIDs when a group has more than MAX_POOL_SIZE repos. Release is unpin-before-close plus closeOne's own pin-clear, so pins never leak across syncs in the long-lived MCP server even on error. * style(test): apply prettier formatting to #2189 test files * fix(review): apply autofix feedback Clarify the pinRepo docstring: the pin does not survive teardown (closeOne clears it) and the repoId must match the key passed to initLbug. Addresses a code-review finding that the prior 'or later holds' wording contradicted closeOne's unconditional pin-clear. * refactor(lbug): reference-count pool pins so overlapping holders are safe [#2189] Change pinnedRepos from Set<string> to Map<string,number>. pinRepo increments the lease count; unpinRepo decrements and deletes the key at 0 (flooring at zero, unknown-id no-op). evictLRU, the idle sweep, and closeOne are transparent to the swap (has()/delete() keep their semantics: skip while count>=1, force-clear on teardown). A boolean Set could not represent two simultaneous holders, so the first release wrongly cleared a pin another holder still needed — the concurrent overlapping group_sync teardown race from the PR #2191 review (Finding 1). Reference counts let two windows of one sync, or two concurrent syncs sharing a repo, coexist safely: the repo stays exempt until the last lease releases. * refactor(lbug): pinRepo returns a leak-proof release disposer [#2189] pinRepo now returns a release() disposer (mirroring addPoolCloseListener) that releases its own lease exactly once — a double-call is a guarded no-op, so it can never over-decrement a sibling holder's reference count. Callers can use the leak-proof pattern `const release = pinRepo(id); try { … } finally { release(); }`. unpinRepo stays exported for explicit pairing. Addresses the PR #2191 review's P3: the exported pin primitive had no built-in pairing, so a caller that forgot to unpin would disable eviction for a repo permanently. * refactor(group): windowed manifest resolution bounds sync pool residency [#2189] Replace whole-sync pinning with windowed deferred resolution. The init loop extracts contracts without pinning (repos evict naturally); manifest links are pre-sorted and partitioned into windows whose referenced in-group repos number <= getMaxResidentRepos(), and each window re-inits + leases only its own repos, resolves, then RELEASES the leases (not closeLbug — released repos stay evictable for the LRU, which avoids stomping a concurrent MCP reader). Peak per-sync pool residency is now bounded by getMaxResidentRepos() distinct repos regardless of group size, removing the unbounded-mmap crash risk the PR #2191 review flagged (Finding 3) — without a new magic-number threshold (it reuses MAX_POOL_SIZE via an intent-named accessor). #2189 stays fixed: each window resolves against live, freshly-leased pools, so cross-links anchor to real graph symbols. partitionManifestWindows is a pure, unit-tested function (every link in exactly one window — the contract-dedup invariant). New sync-windowed-resolution.test.ts asserts the partition bound and, through the real pool, that concurrently-open Databases never exceed the resident cap for a group larger than it. Rewrote the sync.test.ts pinning block (init loop no longer pins; per-window lease/release; release-not-close). |
||
|
Gergő Magyar
|
7c3d4e6862
|
feat(pdg): control dependence — post-dominators + CDG (Ferrante) [M5 #2085] (#2188)
* feat(pdg): add CDG + POST_DOMINATE edge types (M5 #2085) * feat(pdg): post-dominator tree on reverse CFG (M5 #2085) * feat(pdg): Ferrante control-dependence over the post-dom tree (M5 #2085) * feat(pdg): emitFileCdg + optional POST_DOMINATE debug edges (M5 #2085) * feat(pdg): wire CDG emission in-phase + pdgModeMismatch CDG-cap stamp (M5 #2085) * test(pdg): CDG snapshot + end-to-end pipeline answerability (M5 #2085) * fix(review): apply autofix feedback (M5 #2085) * fix(pdg): label CDG edges by controller arm sense, not edge kind (#2188 F1/F2/F4) Tri-review (with Codex as the independent engine) found the CDG 'T'/'F' label was wrong for the commonest control flow: the M1 TS visitor wires a condition's fall-through FALSE arm as `seq`/`loop-back`, but `branchSense` mapped both to 'T', so guard clauses, if-no-else, and loop `break` got 'T' instead of 'F' (F1, P1). The structural CDG edges were correct; only the label — the AC3 "under what condition does X run?" answer — was wrong. - F1: replace edge-kind `branchSense` with controller-arm-sense `labelFor`. An ambiguous fall-through edge (seq/loop-back) takes the COMPLEMENT of its source block's explicit cond-true/cond-false sibling arm. This correctly handles do/while (loop-back = TRUE arm) and inner-if-in-loop (loop-back = FALSE arm) — the ambiguity a kind→label table cannot resolve. Adds real-parser regression tests (the hand-built tests used a fictional cond-false edge and missed it). - F2: correct the false "sound over-approximation that never drops a real dependence" claim in post-dominators.ts — exit-unreachable regions both drop and invent control dependences (latent for the current TS visitor, which keeps EXIT reverse-reachable). Reframe the exit-less-loop test to characterize, not bless, the degenerate behavior. - F4: make the AC2 property-test reference compute post-dominance INDEPENDENTLY (node-removal reachability, no shared code with post-dominators.ts), so a post-dom direction bug can no longer pass both the impl and the reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): root-prettier format + run-analyze pdg stamp gains maxCdgEdgesPerFunction (#2085) Two deterministic CI failures from the M5 CDG work: - quality/format: basicblock-roundtrip.test.ts failed CI's root `prettier --check .` (the pre-commit hook uses the gitnexus-local prettier config, which differs); reformatted with the root config. - tests/ubuntu/coverage: run-analyze.test.ts pinned the resolved RepoMeta.pdg shape (DEFAULTS) and the all-zero cap override without the new maxCdgEdgesPerFunction key (default 5000); added it so resolvePdgConfig toEqual and pdgModeMismatch(DEFAULTS) pass. (The stale-test sweep missed this file in PR #2188 — same trap M2 hit.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(mcp): add pdg_query tool definition (controls/flows modes) [M6 #2086] * feat(mcp): pdg_query backend — controls (CDG) + flows (REACHING_DEF) + e2e test [M6 #2086] * feat(mcp): document PDG edges + pdg_query (schema, cypher, skill, --pdg-gated ai-context) [M6 #2086] * fix(mcp): correct pdg_query symbol-anchor lower bound + harden inputs [PR #2188 review] Tri-review (Codex + adversarial + correctness lanes) of the M6 pdg_query surface found the symbol-anchor window over-includes a neighbor function's block. The upper bound was widened to the 1-based BasicBlock basis (symEnd+1) but the lower bound was left 0-based, so a block on the line directly above the target function leaked into the result. Shift both bounds +1 ([symStart+1, symEnd+1]) so the window is the function's true block span. Also from the same review: - pdg_query no longer throws on a no-arguments MCP call: the dispatch passes raw `params`, so default it to {} → a clean mode-validation error instead of a TypeError. (`explain` shares this latent pattern — pre-existing follow-up.) - tools.ts: the controls-mode description no longer hard-codes the 'F' branch sense for guards — `if (!ok) return;` rides the predicate's 'T' arm; the guard:true flag is label-agnostic (regex on the dependent block text). Tests: a hand-seeded adjacency regression (verified failing without the lower-bound +1) + a no-arguments validation test. Skill doc updated to document the two-sided [symStart+1, symEnd+1] window. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): drop always-true anchor conditional in pdg_query [CodeQL #2188] CodeQL alert 756 flagged `...(anchor ? { anchor } : {})` in _pdgQueryImpl as a useless conditional: `anchor` is unconditionally assigned in both the file-path and symbol branches before the return (the not-found/ambiguous/no-layer paths return earlier), so it is always truthy. Drop `| undefined` from the declaration (TypeScript definite-assignment holds across both branches) and emit `anchor` directly. No runtime change — the `anchor` field was already present on every result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): add hasPdg to the noStats bridge expectation [#2188] The M6 work threaded `hasPdg: options.pdg === true` into the AIContextOptions passed to generateAIContextFiles on the --skills regeneration path, but this test's strict .toEqual expectation predated it (4 keys vs 3 → CI failure). Add `hasPdg: false` (the value on this non---pdg path). The assertion stays strict; the #1477 noStats bridging it guards is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli): collapse generateGitNexusContent params to an options bag [#2188] The function had grown to 9 positional params; reaching `hasPdg` meant passing six `undefined`s (the M6 review's maintainability flag). Collapse params 3-9 (generatedSkills, groupNames, noStats, skipSkills, runnerPath, defaultBranch, hasPdg) into a `GitNexusContentOptions` object with the defaults moved to destructuring. The body is unchanged (same local names); the single production caller and the test calls become self-documenting named fields. Pure refactor — generated AGENTS.md/CLAUDE.md content is byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): skip CDG for exit-unreachable CFGs (unsound post-dominance) [#2188] M5 review P2: computePostDominators roots only at cfg.exitIndex and nothing enforced that EXIT is reachable from every block. For an entry-reachable region that cannot reach EXIT (a non-terminating loop, or a multi-terminal CFG a future visitor might emit) the EXIT-rooted reverse walk degenerates — it both drops real control dependences and invents spurious ones. Add a pure precondition predicate `isExitReachableFromAllBlocks` (co-located with the algorithm it guards) and gate it in emitFileCdg: a CFG that violates it is skipped for CDG (counted as skippedUnsoundFunctions + one onWarn), while its CFG and REACHING_DEF projections — which do not depend on post-dominance — are kept. A CDG-specific gate, not a widening of isEmitSafeCfg, so the blast radius is exactly the unsound CDG. The current TS visitor always satisfies the precondition (every loop gets a structural header→loopExit edge), so CDG output for real fixtures is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cfg): bound computeControlDependence materialization (heap parity) [#2188] M5 review P2: unlike computeReachingDefs (maxFacts) and the emit-side edge cap, computeControlDependence materialized the full deduped seen/out before emitFileCdg's per-function cap could trim it — O(edges × post-dom depth) heap for a deeply nested function. Add a `maxEdges` ceiling (default 0 = unbounded) returning {edges, truncated}, mirroring computeReachingDefs's {facts, truncated}. The ceiling is checked before pushing a new unique edge, so `truncated` means a genuine overflow (not merely "reached cap"). emitFileCdg passes a FIXED materialization ceiling (8× the default edge cap) — deliberately NOT derived from the runtime edge cap, because CDG's materialization IS the deduped-edge quantity the cap reports on (deriving it would pre-truncate that set and lose the exact dropped count). A ceiling hit is surfaced via onWarn + the truncated flag — never silent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(mcp): share resolveBlockAnchor; fix explain's anchor off-by-one [#2188] M6 review P2 (duplication) + the flagged pre-existing _explainImpl correctness follow-up. _pdgQueryImpl and _explainImpl each carried a near-identical symbol↔block anchor resolver that had DRIFTED: pdg_query used the corrected [symStart+1, symEnd+1] window (BasicBlock startLine is 1-based, the symbol span 0-based) while _explainImpl still used [symStart, symEnd] — dropping a taint source on the function's final line AND leaking a neighbor's block on the line directly above. Extract one `resolveBlockAnchor` helper, used by both, that applies the correct window and a single (bare) clause convention (callers compose their own WHERE). This removes ~50 duplicated lines and fixes explain's anchor in one place. A hand-seeded characterization test (taint-explain Block 4) pins both bounds — verified to FAIL on the pre-fix window (it returned the line-10 neighbor instead of the line-15 final-line source). Existing taint-explain + pdg-query suites are unchanged (their fixtures have interior sources/sinks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): pdg_query reports "status unknown" when the layer can't be confirmed [#2188] M6 review P3 (Codex): when meta is UNREADABLE and the bounded global existence probe returns zero rows of the edge type, _pdgQueryImpl asserted "no PDG layer" — but a genuinely edge-free layer (all-linear functions) is indistinguishable from a missing one via that probe. Soften only that fallback path to an inconclusive "PDG layer status unknown — was this repo indexed with --pdg?" note. The meta-stamped path (stamp present, cap absent ⇒ layer truly missing) keeps the definitive "no PDG layer" wording. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mcp): cover pdg_query ambiguous / pagination / Windows-path gaps [#2188] M6 review test-gap follow-ups, all hand-seeded with controlled data: - ambiguous symbol name → status:'ambiguous' + ranked candidates shape (uid/name/filePath/score), never a silent guess; - total/truncated page boundary in both directions (limit below the match count sets truncated with the full total; limit above it omits truncated); - a Windows-style filePath containing ':' resolves and fnLineOf decodes the function-line segment correctly (split-from-right past the drive letter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(skills): ship gitnexus-pdg-query skill mirrors + add pdg_query to the guide [#2086] M6 bundled pdg_query into this PR, but the skill shipped only in the canonical gitnexus/skills/ root. Mirror it (byte-identical) to the two hand-maintained roots the sibling taint skill uses — .claude/skills/gitnexus/ and the plugin — so Claude Code + plugin users get it too. Also extend the gitnexus-guide tool reference (all 3 copies, now byte-identical): add a `pdg_query` row + a "Control & data dependence" section mirroring the taint/`explain` section, and reconcile the pre-existing drift where only the .claude copy carried the `check` tool row (a real registered tool) — all three now list it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): refresh CFG/PDG section for the full M1–M6 stack [#2086] The PR body had deferred the "ARCHITECTURE docs refresh" to #2086; now that M6 ships here, do it: - MCP tools table gains `explain` and `pdg_query` (were absent). - "Optional CFG/PDG emission" was M1-only; rewrite to cover the whole opt-in stack — M1 CFG, M2 REACHING_DEF, M3/M4 taint, M5 CDG (Ferrante over CHK post-dominators, with the exit-unreachable skip), M6 read surface (pdg_query + explain, anchored + LIMIT-bounded, shared resolveBlockAnchor) — and note the no-Function→BasicBlock-edge join. - LadybugDB schema notes the `--pdg` additions: the `BasicBlock` node table and the CFG/REACHING_DEF/CDG/TAINTED/SANITIZES/TAINT_PATH relation types, kept out of the default VALID_RELATION_TYPES / web schema. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
96dc368d96
|
fix(ci): align tree-sitter readiness + grammar-update workflows on a shared manifest (#858) (#2187)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* chore(ci): add shared vendored-grammars manifest; monitor reads it .github/vendored-grammars.json is the single source of truth for the vendored tree-sitter grammars (c/swift/kotlin/dart/proto): name, upstream coords, and policy holds. update-vendored-grammars.mjs now builds its GRAMMARS map from the manifest (behavior-preserving — same exported shape). Adds manifest-agreement tests so the loader can't silently skew from the file. * fix(ci): classify vendored grammars from manifest, drop bare "?" (#858) The readiness report decided "is this vendored?" via is_vendored_pin (a file: package.json spec) — but the 5 vendored grammars aren't in package.json, so they were misrouted through the npm path and rendered bare "?" for ABI (read from an empty node_modules), plus a spurious "? (fetch failed)" for github-only proto. Now vendored grammars are classified by membership in the shared manifest and their ABI is read from gitnexus/vendor/<name>/src/parser.c (always in a checkout). github-only vendored grammars skip the npm peer-dep fetch; the tree-sitter-c hold is surfaced from the manifest (held, not plain "Ready"); and every remaining unintrospectable value renders a labeled token, never a bare "?". --assert-current now covers the vendored grammars too instead of skipping them. Adds a stdlib unittest suite incl. a manifest⇄vendor-dir consistency guard. * docs(ci): document the shared vendored-grammars manifest Both tree-sitter workflow headers now point at .github/vendored-grammars.json as the shared source of truth; the readiness workflow gains a PR-path trigger on the manifest + test, and runs the readiness unit tests on validation events. CONTRIBUTING.md documents the manifest contract under CI automation contracts. * fix(review): apply autofix feedback - Guard manifest reads in both scripts with a clear error (was an opaque module-import traceback that crashed the script and test collection). - Never render a bare "?": relabel the npm-path ABI/version/peer sentinels and the vendored upstream-ABI miss to labeled tokens; the report is now ?-free regardless of node_modules/network, and the test is hermetic. - Add a VENDORED_NAMES ⊆ GRAMMARS guard + manifest-missing error test. - Drop now-dead is_vendored_pin/is_vendored/_(vendored)_. - Compose held + out-of-range vendored blocker reasons instead of overwriting. - Reword the shared-manifest docs to not over-claim shared upstream coords. * fix(ci): apply root prettier formatting to mjs + ts test The quality/format gate runs root `prettier --check .` (printWidth 100, the gitnexus-local config differs and falsely passed locally). * test(ci): make both tree-sitter scripts testable offline The scripts hit live npm/GitHub, which makes the report run flaky and the monitor's detect/apply logic untestable. Add hermetic seams: - readiness: --offline flag (+ GITNEXUS_TS_READINESS_OFFLINE env) no-ops the npm registry + upstream fetches; the report renders deterministically (vendored ABIs from the repo, npm columns marked 'offline', no bare '?'). 3 tests assert an offline run touches ZERO network (urlopen patched to raise). - monitor: detect() and apply() accept injected deps (vendoredVersion/ resolveUpstream/fetchSource/readAbi) so the newer/ABI/hold gating runs offline with fixtures; apply gains --dry-run (validates but writes nothing). 6 tests cover newer/same-version/held-c/ABI-15/applicable + a no-mutation dry-run. * fix(review): keep --assert-current hermetic + harden the no-bare-? invariant Tri-review findings (PR #2187): - P2 REGRESSION: --assert-current (documented 'hermetic and offline', run in CI without --offline) routed the 5 vendored grammars through vendored_drift_summary, which fetches upstream parser.c + commit sha — 10 discarded network calls per run. Fix: read the vendored ABI locally via a new vendored_abi_from_repo() helper (also used by vendored_drift_summary). Now verifiably network-free. - Unify the upstream-ABI miss sentinel: prose said 'n/a (generated at build)' while the matrix said 'n/a' — and 'generated at build' is a wrong cause (swift HAS a committed parser.c). Both now render neutral 'n/a'. - Fix the stale assert_current docstring claiming swift is prebuilt-only/no parser.c. - Guard the last latent bare-? path (vendor package.json missing 'version'). Tests: AssertCurrent (network-free guard + out-of-range via the new injection point), malformed-JSON manifest, detect() error-path, explicit npm/github undefined assertions. 17 Python + 15 vitest, all hermetic. * fix(review): use a single unittest import style (CodeQL 753) CodeQL py/import-and-import-from flagged `import unittest` + `from unittest import mock`. Collapse to `from unittest import TestCase, main, mock`. * fix(review): explicit raise in _matrix_row (CodeQL 754) CodeQL py/mixed-returns flagged the implicit fall-through after self.fail() (which it doesn't model as NoReturn). End with an explicit raise AssertionError. * test(review): replace non-null assertions with a must() guard @typescript-eslint/no-non-null-assertion flagged 4 `!` operators. Add a narrowing must<T>(value, message) helper (throws on undefined) and a named baseResolveUpstream, removing every non-null assertion. * fix(review): unguessable heredoc delimiter for the report output The report embeds the manifest `hold` field (fork-PR-editable); a fixed DRIFT_EOF delimiter in a hold value could close the $GITHUB_OUTPUT heredoc early and inject output keys. Use DRIFT_EOF_$(openssl rand -hex 16) — a value the report cannot contain. (Randomized delimiter over base64: keeps REPORT raw markdown, no consumer-side decode.) * fix(review): scope issues:write to scheduled runs (two-job split) GitHub Actions has no step-level permissions, so the only way to keep PR runs (incl. forks) from receiving `issues: write` is to split the job. A `report` job (contents:read, all events) renders the report + the PR `:⚠️:` and exposes report/exit_code as job outputs; a schedule-only `upsert-issue` job (needs: report, issues:write, no checkout) consumes them for the issue upsert + close. The 'Check upgrade readiness' check name is preserved. * fix(review): launder npm-version '?' in disposition prose The disposition bucket prose interpolated r['npm_version'] raw, so a successful 200 npm /latest response lacking a 'version' key would render a bare '?' (the matrix cell already laundered it). Add npm_version_label ('unknown' for '?') and use it in all five bucket renderers. Test a version-less npm response. * refactor(review): load_vendored_manifest returns only the consumed 'hold' The readiness script reads only the grammar names + 'hold'; the 'key' and 'upstream' fields were phantom data (upstream-drift coords live in the script's own GRAMMARS map). Narrow the return to {hold}. * fix(review): unify detect()/apply() 'newer' check for github grammars detect() compared the bare sha7 while apply() compared up.version (the full <base>-g<sha7> provenance string apply() also writes). After the bot re-vendored a github grammar once, detect() reported a perpetual false 'update available' while apply() correctly saw 'already current' — a noisy job summary + wasted --apply subprocess (the PR-exists guard absorbed it before any duplicate PR). Extract a shared isNewer(up, have) helper used by both. Tests cover equal- provenance (false), first-vendoring plain-version (true, not suppressed), and sha-advanced (true). Coupled with U12 (the detect⇄apply agreement assertion lives there once apply()'s not-newer path returns instead of process.exit). * test(review): cover main()'s out-of-range + prebuilt-only vendored ABI branches main()'s vendored-ABI classification reads through vendored_abi_from_repo (the local-read seam --assert-current uses), so patching it drives the 'Vendored (ABI out of range)' blocker branch and the prebuilt-only (vendored_abi None → 'prebuilt' cell, not '?') branch — neither reachable today since all 5 vendor dirs ship parser.c at ABI 14. * test(review): monitor-side manifest⇄vendor-dir consistency guard Mirror the Python consistency guard on the monitor side — the monitor consumes the same manifest and is the side that WRITES files from manifest `name`, so manifest/vendor-dir drift must fail CI here too. * fix(review): validate grammar names at manifest load (path-traversal guard) The manifest `name` is joined into gitnexus/vendor/<name> paths in both scripts (and apply() WRITES there), so reject any name not matching tree-sitter-[a-z0-9-]+ at the single load chokepoint — defense-in-depth even though the live trust boundary already prevents exploitation. loadManifestGrammars gains an injectable `raw` arg + export for testing; tests reject a '../etc' name in both scripts. * refactor(review): apply() throws ApplyExit; CLI maps to exit codes apply()'s 4 process.exit calls killed the vitest worker, blocking in-process tests of its error branches. Replace them with a thrown ApplyExit{code}; the not-newer (already-current) path returns `have` instead of exit(0). The isMain CLI block try/catches and maps ApplyExit.code → process.exit, so the monitor's subprocess contract (exit 0/2/3) is byte-identical (verified via subprocess smoke). Tests cover unknown-key=2, held=3, ABI-reject=3, and not-newer (returns current, no throw, no write). * refactor(review): extract vendored render helper; trim docstrings (<1000 lines) Extract the 'Vendored parsers' prose render into _render_vendored_section() so main() coordinates named phases rather than inlining a ~450-line monolith, and condense the most verbose docstrings/comments. The script drops from 1092 to 999 lines (under the 1000 bar the maintainability review flagged). Behavior-preserving: the deterministic --offline render is byte-identical before/after (verified in-place), --assert-current still passes, and the full unit suite is green. * fix(review): row-diff regex captures only the Status cell The change-detection regex captured the whole row tail as group 2, so any non-status cell drift (e.g. an upstream-ABI bump) emitted a false-positive 'change' line. Capture only the Status cell ([^|]+? before the final |$). The workflow parseRows regex and the Python _ROW_DIFF_RE stay byte-identical; the stability test now asserts group 2 is the status string (e.g. c → 'Vendored — held') and contains no pipe. * fix(ci): hoist intro string out of the list literal (CodeQL 755) The U13 extraction moved the 'Vendored parsers' intro paragraph (implicitly concatenated string literals) INTO a list literal, tripping CodeQL py/implicit-string-concatenation-in-list (reads as a possibly-missing comma between elements). Hoist it into a parenthesized `intro` variable. Render is byte-identical. |
||
|
bluerose
|
89ffa71a52
|
feat(cli): add --embeddings-baseurl/-model/-auth-token/-dims flags to analyze (#2140)
* feat(cli): add --embeddings-baseurl/-model/-auth-token/-dims flags to analyze Add four CLI flags to `gitnexus analyze` that configure a custom OpenAI-compatible HTTP embedding endpoint by setting the GITNEXUS_EMBEDDING_URL / _MODEL / _API_KEY / _DIMS env vars the HTTP embedding client already reads. Flags override env vars; env vars keep working as before. URLs are validated (http/https) and dims must be a positive integer. Prints "Using custom embedding endpoint: <url>" when a URL+model pair is configured, and warns when the flags are passed without --embeddings. The new env keys are added to the analyze snapshot/restore set so programmatic callers don't leak state. The non-secret flags are also accepted from .gitnexusrc; the auth token is intentionally CLI/env-only. * fix(analyze): set GITNEXUS_EMBEDDING_DIMS from CLI flags before module import schema.ts reads EMBEDDING_DIMS at module-load time via the static-import chain (analyze.ts -> run-analyze.ts -> schema.ts). The previous approach of setting the env var inside analyzeCommandImpl ran AFTER schema.ts had already loaded with the default 384, causing "Expected: 384, Actual: 4096" errors when using --embeddings-dims 4096. Fix: use Commander's preAction hook to set GITNEXUS_EMBEDDING_* env vars before the lazy import of analyze.ts triggers the schema.ts module load. * fix(analyze): use hook callback arg instead of this in preAction Commander v14 passes the command as first argument, not as this binding. * refactor(cli): rename --embeddings-* analyze flags to singular --embedding-* Aligns the custom embedding endpoint flags with the existing singular tuning flags (--embedding-threads/--embedding-device): --embedding-base-url, --embedding-model, --embedding-auth-token, --embedding-dims. Renames the derived AnalyzeOptions fields and the .gitnexusrc KEY_SPECS keys to match. Behavior-preserving; the GITNEXUS_EMBEDDING_* env vars are unchanged. Refs #2140 review. * fix(cli): validate and normalize --embedding-dims before module-load reads it The preAction hook wrote GITNEXUS_EMBEDDING_DIMS unvalidated, so an invalid value (abc/0/-5/0x10) threw from schema.ts during the lazy import — surfacing as a raw unhandled rejection on the synchronous program.parse path instead of a friendly error. And '1e3' slipped through: schema.ts parseInt froze the vector column at FLOAT[1] while the impl's Number-based check accepted 1000, so http-client requested 1000-dim vectors against a 1-dim column. Extract a dependency-free normalizeEmbeddingDims helper (strict /^\d+$/ + positive, trim-then-validate, canonicalized) shared by both the hook (CLI path, before module-load) and analyzeCommandImpl (direct-call path). All three readers — schema.ts, http-client, and this helper — now agree on one value, and invalid input gets a clean message instead of a crash or a silent mismatch. Refs #2140 review. * fix(cli): mask credentials in the custom embedding endpoint confirmation A base URL with userinfo (http://user:pass@host/v1) or a query token (?api_key=…) passed the new-URL + http/https validation and was printed verbatim in the 'Using custom embedding endpoint:' line, leaking the secret to terminal scrollback and CI logs. Route it through the existing safeUrl() (now exported from http-client) which strips userinfo + query, keeping protocol/host/path. Single source of truth — no second sanitizer. Refs #2140 review. * fix(cli): drop the ineffective embeddingDims .gitnexusrc key embeddingDims as a .gitnexusrc key silently did nothing: .gitnexusrc loads in analyzeCommandImpl, AFTER the lazy import already ran schema.ts's module-load read of GITNEXUS_EMBEDDING_DIMS, so a config value never sized the vector column. Remove it (config now fails closed on the key, like the auth token); URL/MODEL stay as config keys because they're read lazily at runtime. Dims remains available via --embedding-dims or GITNEXUS_EMBEDDING_DIMS. Refs #2140 review. * refactor(cli): narrow the analyze preAction hook to GITNEXUS_EMBEDDING_DIMS Only DIMS is read at module-load (schema.ts), so only it must be set before the lazy import. URL/MODEL/API_KEY are read lazily at runtime, so analyzeCommandImpl is their sole setter — and because the impl's env snapshot is taken AFTER this hook ran, leaving those three in the hook leaked them past restore. Drop them from the hook (the impl already sets+restores them), and capture/restore the pre-hook DIMS baseline via a postAction hook so a CLI --embedding-dims override no longer leaks into a later in-process program.parseAsync. Refs #2140 review. * fix(cli): gate the custom-endpoint confirmation on the embedding flags The confirmation collapsed into one if/else chain that emits at most one message reflecting the run's intent. Gating on embeddingsEnabled stops the 'Using custom embedding endpoint' line from printing on every analyze run when GITNEXUS_EMBEDDING_URL+MODEL merely happen to be set in the environment, and ordering the '--embeddings absent' note first removes the contradiction where it printed alongside 'Using custom embedding endpoint'. Refs #2140 review. * test(cli): cover the custom embedding endpoint flags Adds direct-call (analyzeCommandImpl path) coverage the original PR lacked: URL validation (empty/invalid/non-http), model/token emptiness, dims validation incl. the 1e3 regression, credential masking in the confirmation line, confirmation gating (absent --embeddings; ambient env must not trigger it), CLI-over-env precedence, and the GITNEXUS_EMBEDDING_* snapshot/restore round-trip. Complements embedding-dims.test.ts and http-client-safe-url.test.ts. Refs #2140 review. * test(cli): e2e-cover the --embedding-dims crash path on the real CLI The dims-validation fix lives in the commander preAction hook, which only fires on the program.parse path; the direct analyzeCommand() unit tests bypass it. Add a subprocess e2e (run via tsx, no build) asserting that invalid --embedding-dims (abc/0/-5/1e3/3.5) produces the friendly flag-named error and exit 1 — NOT the raw schema.ts module-load throw that the original bug surfaced. Cases exit inside the hook (no repo/import/pipeline), so they're deterministic and fast. Updates the unit-suite comment to point at it. Refs #2140 review. --------- Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |
||
|
Minidoracat
|
912285064a
|
perf(hooks): cmdline-first Linux db-lock scan, drop the lsof fallback (#2180) (#2183)
* perf(hooks): cmdline-first Linux db-lock scan, drop the lsof fallback (#2180) The probe's Linux scan was O(processes × fds) — stat every fd of every process — so on a busy host it blew its budget and fell through to lsof, which then timed out (~2 s) and fail-closed. Every Grep/Glob/Bash hook spent ~2 s of CPU to conclude 'couldn't tell'. Rewrite linuxProcScanFindGitNexusServer (name kept; return type now tri-state 'owned' | 'not-owned' | 'timeout') as three phases: 0. /proc/<pid>/comm prefilter — kernel task->comm, never touches the target's memory maps; truncation-safe whitelist match (comm is capped at 15 visible chars). Calibrated to what a real server reports: @ladybugdb/core's worker_threads rename the main thread to 'MainThread', so that is whitelisted alongside the launcher basenames — omitting it would blind the probe to every server. 1. bounded /proc/<pid>/cmdline read (openSync+readSync, default 16 KiB with a floor of 4 KiB and a bounded escalation up to a hard ceiling) so a D-state holder cannot stall the hook and the mcp/serve mode token is never clipped off a long interpreter path. 2. dev+ino fd match for the 0–2 survivors only. Dispatch: 'owned' and 'timeout' both map to true. Timeout is now fail-closed (overload self-throttle) instead of falling through to lsof; the Linux lsof fallback is removed entirely. End-to-end semantics on busy hosts are unchanged (the old lsof arm also fail-closed there) — the ~2 s of wasted work and the orphan-spawning lsof are what's gone. macOS lsof+ps and Windows Restart Manager paths are untouched. Also: fix the budget parse bug (Number(raw && trim()) treated '0' as 1200; now parseInt-then-validate, with <= 0 an explicit immediate timeout) and add GITNEXUS_HOOK_PROC_ROOT so the Linux scan can be unit tested against a fixture procfs instead of the host's real /proc. Measured on a 583-process host with 6 background gitnexus mcp servers: owner detection 6–12 ms (was ~1216 ms + lsof timeout), ~100x. Tests: new hook-db-lock-probe.test.ts drives all three phases against a fake procfs (comm-truncation safety, Phase 0 trap, 4 KiB-boundary owner-miss guard, budget=0 immediate timeout, EACCES fail-closed) plus a live-/proc e2e that pins the fd-visible lbug-handle property against a real subprocess holder. The lsof/ps owner-detection suites are relaned to macOS (Linux no longer takes that path); the lsof orphan-reaping suite is removed (no lsof is spawned on Linux now) with a rationale note. Note: pre-commit typecheck skipped; remaining tsc errors are pre-existing on main (none in files touched here). * fix(hooks): honest EACCES verdict + real escalation coverage (#2183 review) Addresses the tri-review (maintainer + Codex): - [P2] Phase-2 fd-dir EACCES no longer claims 'owned'. /proc/<pid>/fd is owner-only (0500), so a cross-user/root gitnexus server serving ANY repo cleared Phase 0+1 and hit EACCES here, and the old catch returned 'owned' — falsely claiming it locks THIS repo's lbug (dev+ino never compared) and permanently suppressing augment. Split the failure shapes: ENOENT -> continue (raced away); EACCES/EPERM and transient EIO/ESTALE -> 'timeout' (unverifiable -> fail-closed, but honest, not a false ownership claim); ENOTDIR/other structural errors -> continue (not a real fd dir). Same fail-closed dispatcher outcome, no false 'owned', plus a GITNEXUS_DEBUG diagnostic so an operator can tell this skip path from a real owner. - [P2] The escalation test now actually iterates the escalation loop: the gitnexus token sits under 4 KB while the mode token is padded past GITNEXUS_HOOK_PROC_CMDLINE_MAX=4096, and a readSync spy asserts >1 read (the old 9 KB-under-16 KB-cap shape read once and never escalated). - escalation loop now re-checks the budget each iteration and returns a distinct timeout sentinel (never '' — an empty string would read as 'not a candidate' and could drop a real owner -> fail-open); the caller maps it to 'timeout'. - GITNEXUS_HOOK_PROC_ROOT is gated to test context so a stray production env export can't disable Linux owner detection (fail-open). - New uid-agnostic spy tests pin every fd-readdir errno branch (EACCES/EPERM/EIO/ESTALE -> timeout, ENOTDIR -> not-owned) regardless of the runner's uid (the disk chmod-000 tests no-op under root). Note: pre-commit typecheck skipped; remaining tsc errors are pre-existing on main (none in files touched here). * fix(hooks): drop the always-true outOfBudget presence guard (CodeQL #2183) CodeQL flagged `typeof outOfBudget === 'function' && outOfBudget()` as unneeded defensive code: readLinuxCmdline has a single caller (linuxProcScanFindGitNexusServer) that always passes the callback, so the typeof guard is dead. Drop it, leaving `if (outOfBudget())`, and note the invariant in the comment. Mirrored in the byte-identical plugin copy. * fix(hooks): parse numeric hook env with Number() so scientific notation works (#2183 review) getCmdlineMaxBytes and resolveLinuxProcBudgetMs parsed their env via Number.parseInt(raw, 10), so a value like "16e3" silently became 16 (parseInt stops at 'e') instead of 16000. Switch both to Number(String(raw).trim()), which honors scientific notation and is stricter on trailing garbage ("123abc" -> NaN -> default) — matching the repo-majority Number()+isFinite env idiom (src/cli/analyze.ts, src/core/embeddings/hf-env.ts). The two functions had DIFFERENT guard skeletons, so a verbatim swap would regress the budget: resolveLinuxProcBudgetMs used `raw != null ?` with no empty-string short-circuit, and Number("")===0 (vs parseInt("")===NaN) would make a set-but-empty GITNEXUS_HOOK_LINUX_PROC_BUDGET_MS="" resolve to budget 0 => immediate fail-CLOSED timeout => augment permanently skipped. Added the `&& String(raw).trim()` guard so ''/whitespace fall to the 1200 default while "0" still parses to the deliberate #2180 immediate-timeout vector. Exported both helpers for white-box tests (the values are otherwise only observable indirectly through scan timing) and added platform-independent coverage: "16e3"->16000, ""/whitespace->1200 (the regression guard), "0"->0, "123abc"/unset->1200, cmdline "8e3"->8000, "2e3"/""/unset->16384. Both byte-identical hook-db-lock-probe.cjs copies updated together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(hooks): allocUnsafe the per-chunk cmdline read buffer (#2183 review) readLinuxCmdline allocated each per-chunk read buffer with Buffer.alloc(chunkCap), zero-filling memory that readSync immediately and fully overwrites. Switch the hot read buffer to Buffer.allocUnsafe — safe because readSync initializes exactly [0, bytes), only buf.subarray(0, bytes) is consumed, and Buffer.concat deep-copies that slice into `collected`, so the uninitialized tail can never reach the decoded cmdline. The zero-length `collected = Buffer.alloc(0)` is left unchanged (allocUnsafe gains nothing on a 0-length buffer). The existing D3 multi-chunk decode tests cover the read path and stay green. Both byte-identical hook-db-lock-probe.cjs copies updated together. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(hooks): harden the live /proc owner-detection e2e against CI flake (#2183 review) Two flake mechanisms, fixed without weakening what the e2e proves: - Holder readiness (the genuine false-FAIL): the pid-file poll was 200x25ms=5s; a loaded runner can be slow to spawn the child, tripping expect(holderPid).toBeGreaterThan(0). Widened to ~10s and raised the per-test timeout 20s -> 40s. - Scan budget (kept the assertion honest): the live scan ran at the default 1200ms. Because the dispatcher maps a budget 'timeout' to owned=TRUE, a busy host exhausting 1200ms before reaching the holder would make the assertion pass for the WRONG reason (a hollow timeout, not real fd-visible detection). Set a generous explicit 10000ms budget via the existing setEnv() helper so the module afterEach restores it (replacing the raw `delete process.env...` that bypassed env tracking). Raised the coarse timing regression guard to sit ABOVE the budget (5000 -> 15000) so a legitimately-slow-but-correct scan can't trip it. The load-bearing asserts (dev+ino fd-visibility precheck, owned===true for our own lbug) are unchanged. Verified the e2e executes (not skipped) on Linux. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(changelog): empty the root CHANGELOG [Unreleased] section Per maintainer request, nothing should sit under [Unreleased] in the root CHANGELOG.md (the release-owned changelog is gitnexus/CHANGELOG.md, whose [Unreleased] is already empty). Removes all three accumulated blocks — Fixed (#2163), Performance (#2180), Changed (KuzuDB->LadybugDB) — leaving only the [Unreleased] header above [1.5.3]. Pure removal; no release sections touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Gergő Magyar
|
6dc6544365
|
fix(web): chat-only mode for large projects to prevent WebUI hang (#2178) (#2185)
* feat(web): add graph-load skip decision helper and node threshold (#2178) * feat(web): skip graph download in connectToServer for chat-only mode (#2178) * feat(web): add graphMode state and empty-graph chat-only handling (#2178) * feat(web): read and thread ?skipGraph URL param through connect flow (#2178) * feat(web): chat-only empty state with load-graph-anyway escape hatch (#2178) * style(web): apply prettier formatting to graph-load files (#2178) * fix(review): apply autofix feedback - Fail-safe confirm + authoritative node count (P1: prevent re-triggering the hang via Load-graph-anyway when count unknown) - In-flight guard on loadGraphAnyway (P1: double-fire) - Honor explicit ?skipGraph in onAnalyzeComplete and DropZone (R6/U4) - Extract buildGraphFromConnectResult shared helper (DRY across 3 connect sites) - Add tests: switchRepo skip path, threshold config override, loadGraphAnyway error path, confirm fail-safe, in-flight guard * fix(review): address tri-review findings - P1 (correctness+adversarial+risk): stop the cross-repo / F5 chat-only leak. loadGraphAnyway no longer persists ?skipGraph=0, and onAnalyzeComplete + DropZone no longer inherit a stale ?skipGraph for a different repo — both could bypass auto-detect and re-trigger the #2178 hang. ?skipGraph is now a bookmark hint honored only by the initial auto-connect; in-session repo changes auto-detect. - P2 (performance): auto-detect now also skips on edge count (edge-driven force-layout cliff), not just nodes; LARGE_GRAPH_EDGE_THRESHOLD default 50K. - P2 (julik): reset graphMode/chatOnlyNodeCount at the top of switchRepo so a failed switch can't leave a stale chat-only overlay. - P2 (julik): set serverBaseUrl before awaiting handleServerConnect in auto-connect so the Load-graph-anyway button isn't briefly a no-op. - P2 (risk): hide the misleading '0 nodes / 0 edges' stats in chat-only mode (Header + StatusBar). - P2 (performance): guard the GraphCanvas layout effect against the empty chat-only graph. - Tests: edge-threshold decision + connectToServer edge-trigger; load-anyway no longer asserts URL persistence. * fix(web): make Load-graph-anyway cancellable, unmount-safe, fail-safe confirm (#2178) - AbortController + mountedRef: cancel the in-flight download on unmount and guard every post-await setState by the mounted ref (an abort surfaces as a BackendError, not a DOMException AbortError, so name-checks would miss it) - Stale-result guard: a load-anyway that resolves after a concurrent switchRepo no longer clobbers the new repo's graph/mode/count - GraphCanvas confirm fails SAFE (treat as declined) when window.confirm is unavailable or throws, instead of silently proceeding into a large download * fix(web): make the AI agent and chat surface aware of chat-only mode (#2178) - buildDynamicSystemPrompt + createGraphRAGAgent take a chatOnly flag and append a note (both prompt branches) that supersedes VISUAL GROUNDING: the graph isn't loaded, [[Type:Name]] node citations won't highlight, prefer [[path:START-END]] - initializeAgent resolves chatOnly = opts ?? graphModeRef.current==='chatOnly': connect-flow callers (handleServerConnect, switchRepo, loadGraphAnyway re-init) pass it explicitly; lazy/settings re-inits fall back to live mode via the ref - loadGraphAnyway re-inits the agent (chatOnly:false) after a full load so the prompt drops the note - RightPanel shows a chat-only banner so the degradation is visible where AI output renders (en + zh-CN) * fix(web): streaming circuit breaker for graphs with missing size stats (#2178) - GraphTooLargeError + a mid-stream breaker in parseNdjsonGraphResponse: count nodes/relationships as they arrive and abort (cancel reader in try/finally, then throw) the moment either crosses its limit — reusing the existing node/ edge thresholds, no new magic constant. Throwing right after the offending push means a later error record in the same chunk can't pre-empt it. - fetchGraph gains optional maxNodes/maxEdges (off by default → existing callers unchanged). connectToServer arms them only for auto-detect downloads (skipGraph !== false) and catches GraphTooLargeError → chat-only, re-throwing every other error. This backstops the no-stats fail-open path that could otherwise re-trigger the original hang. * chore(autofix): apply prettier + eslint fixes via /autofix command --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Gergő Magyar
|
9ff7337f1e
|
fix(mcp): rename query/cypher params so Claude Code can call them (#2186)
* fix(mcp): advertise search_query/statement params for query/cypher tools (#2175) Claude Code drops a tool-call argument named exactly 'query', making the query and cypher tools unusable from it. Rename the advertised required parameters to search_query and statement so the client transmits them. Handler-side backward-compat for the legacy 'query' key follows in the next commit. * fix(mcp): accept search_query/statement with legacy query fallback (#2175) Resolve the new advertised param names in the backend while still accepting the legacy 'query' key, so curl/HTTP, other MCP clients, the CLI, the group path, and the internal executeCypher() all keep working. Alias is normalized once at the callTool chokepoint (covers group-forward + search alias); query() and cypher() dual-read defensively. New name wins when both are supplied. Updates the required-error message and adds dual-accept unit + integration coverage. * fix(cli): pass canonical search_query/statement params to query/cypher tools (#2175) Stop the CLI from depending on the deprecated 'query' alias. No user-facing change — the positional args are unchanged and the backend accepts both keys. * fix(mcp): generators advertise search_query in query() examples (#2175) Update the three doc/example generators (ai-context AGENTS/CLAUDE block, skill-gen community skills, resources repo hint) so future analyze runs emit query({search_query: ...}) — the param name Claude Code actually transmits. Tests assert the new form is present and the legacy query({query: form is absent (the #2059 generator-test pattern). * docs(mcp): advertise search_query/statement in skill & guidance examples (#2175) Sync the committed agent-facing docs to the renamed params so a Claude Code agent following them emits the transmittable key: AGENTS.md/CLAUDE.md gitnexus block, the canonical gitnexus/skills/* source and its installed/plugin/cursor mirrors, and the README examples. Scoped rewrite of the two call prefixes only (query({query: -> search_query, cypher({query: -> statement). * style(mcp): prettier line-wrap for #2175 alias-resolution edits * fix(review): uniform search_query precedence + cypher empty guard (#2175) Code-review findings (correctness/adversarial/api-contract/maintainability consensus): - Group-mode query inverted the 'new name wins' rule: the callTool chokepoint backfilled params.query only when empty and the @group-forward read params.query directly, so a both-keys (or whitespace-legacy) group call let the legacy value win — unlike the local path. Replace the hidden param mutation with a self-contained 'search_query ?? query' resolve at the group-forward; precedence is now uniformly new-wins at every consumer site. - cypher() now returns the same friendly required-param error as query() when neither statement nor query is supplied, instead of a raw DB prepare error. - Document the legacy alias as permanent (third-party clients may send query=). Adds group-forward alias tests (both-keys + legacy-only), empty/whitespace search_query, the search-alias path, and the cypher empty-statement guard. * fix(review): non-string alias safety + drop stale chokepoint comment (#2175) Tri-review findings (correctness/adversarial/security + maintainability): - Non-string statement/search_query/query (the MCP envelope is not schema-validated) hit .trim() and threw TypeError to the server boundary instead of a friendly required-param error. Introduce resolveAliasString() (new name wins; non-string -> undefined) used by query(), cypher(), and the group-forward, so all three return the structured error. Empirically verified (123 ?? '' -> 123, (123).trim() throws) — this overrides a critic refutation that mis-read ?? as a string coercion. - Remove the stale query() comment claiming alias resolution happens at a callTool chokepoint; that mutation was removed earlier in this PR — each site resolves the alias itself. - Document GroupToolPort.query's intentionally-narrower required type vs the wider LocalBackend impl. Adds non-string and empty-new-key precedence tests. * fix(mcp): alias falls back to legacy value when new key is blank (#2175) PR #2186 review finding: resolveAliasString used `canonical ?? legacy` (nullish), so an explicitly empty/whitespace new-name value (e.g. {search_query:'', query:'real'}) won and was rejected — discarding a valid legacy value, contradicting the 'new name wins when both supplied' intent. Resolve to the first NON-BLANK string instead (new preferred when it carries a real value, else legacy). Covers query(), cypher(), and the group-forward (all route through the helper); non-string still resolves to a friendly error. Flips the presence-based test and adds whitespace/cypher/group fallback cases. * fix(mcp): drop legacy "query" mention from query/cypher schema descriptions (#2175) PR #2186 review finding: the search_query/statement inputSchema descriptions named the legacy "query" key — the exact arg Claude Code drops — and description text is read by an LLM choosing arguments, weakly nudging it to send "query". Trim the descriptions to their clean form and move the legacy-alias note to a code comment next to the schema (preserved for maintainers / non-CC clients). properties/required unchanged (no `query`). |