Commit graph

3 commits

Author SHA1 Message Date
azizur100389
ff0124e067
feat(cpp): parse CUDA source extensions (#2213)
* feat(cpp): parse CUDA source extensions

* test(cpp): characterize CUDA parser limitations

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-06-16 07:32:53 +01:00
Hector Prats
d69eadfb7f
fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV (#1433)
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
Release Candidate / Check if release candidate should run (push) Waiting to run
Release Candidate / ci (push) Blocked by required conditions
Release Candidate / Publish release candidate to npm (push) Blocked by required conditions
Release Candidate / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
* fix(windows): 32767-char tree-sitter crash + VECTOR extension SIGSEGV

tree-sitter 0.21.x on Windows crashes with SIGSEGV when parsing source
strings longer than 32 767 chars (signed 16-bit integer overflow in the
native binding). Five call sites passed raw file content without any
length guard:

  - captures.ts (C# scope extraction)
  - namespace-siblings.ts (extractFileStructure)
  - parse-worker.ts (worker thread parse path)
  - parsing-processor.ts (sequential parse fallback)

Fix: truncate at the last newline before the limit so the fragment stays
syntactically coherent. Files truncated mid-class produce ERROR roots;
captures.ts returns [] for any ERROR-root tree so the legacy DAG handles
the file silently without orphaned scope errors.

Additional C# scope fixes:
  - scope-tree.ts: Module scopes may share the same range as a top-level
    namespace_declaration (files with no leading `using` directives). The
    rangeStrictlyContains check rejects equal ranges. Added
    rangeNonStrictlyContains for Module parents.
  - scope-extractor.ts: pass1BuildScopes stack-pop used strict containment;
    same Module == Namespace range case caused orphaned scopes. Added
    moduleAwareContains helper.
  - scope-extractor-bridge.ts: empty captures from ERROR-root files still
    called extractScope -> "no Module scope found" warning. Added early
    return for empty/non-array captures.
  - namespace-siblings.ts: three sites pushed onto binding arrays frozen by
    finalize-algorithm. Fixed with spread-copy before mutation.

lbug-adapter.ts: INSTALL VECTOR in loadVectorExtension calls the KuzuDB
native extension installer, which crashes with SIGSEGV on Windows via an
unhandled error path in native code. JS try/catch cannot intercept native
signals. Skip extension loading on win32 — vector/embedding search is
unavailable on Windows but all graph index queries work correctly.

Verified on: Windows 11, Node.js 24, gitnexus 1.6.3, pcf8-game codebase
(61 757 nodes / 111 796 edges / 300 flows after fix).

* fix(windows): skip FTS extension load in pool-adapter on Windows to prevent SIGSEGV

LOAD EXTENSION fts crashes the process with SIGSEGV on Windows when the
FTS extension binary is not installed locally. This is an @ladybugdb/core
native bug — the extension loader hits an unhandled error path that raises
a native signal instead of a JS exception, so try/catch cannot protect here.

Add a process.platform === 'win32' guard in both doInitLbug and
initLbugWithDb. When skipped, bm25-index.js catches the resulting
Kuzu catalog errors (CREATE_FTS_INDEX not defined) and returns empty
BM25 results gracefully. All graph queries (cypher, context, impact)
are unaffected.

This is patch 9 of the Windows fix series for gitnexus on Windows:
patch 8 (same PR) already fixed INSTALL VECTOR SIGSEGV in lbug-adapter.ts.
pool-adapter.ts is the separate MCP-server code path that was not covered.

* fix: address codeql findings on PR #1433

The four `lastIndexOf('\n', ...)` calls were committed with a literal
newline inside the single-quoted string instead of the `\n` escape, so
the files do not parse — `tsc` and CodeQL both flagged them. Replace
the embedded newline with `'\n'`.

Also remove the two helpers that were superseded during review and
became dead code: `rangeNonStrictlyContains` in scope-tree.ts (the
equal-range carve-out is handled by `rangeStrictlyContains` +
`rangesEqual` in `canParentScope`) and `moduleAwareContains` in
scope-extractor.ts (`pass1BuildScopes` calls `canParentScope` directly).

* fix(windows): replace 32767-char truncation with chunked-input parsing

The tree-sitter 0.21.x Node binding crashes (SIGSEGV) on Windows when
parser.parse(string, ...) is handed a JS string longer than 32 767 chars.
The crash is in the bindings V8 string-to-buffer conversion and cannot
be intercepted from JS. Previous mitigation truncated source at the last
newline before that boundary, silently losing the file tail and producing
ERROR-root trees from mid-class cuts.

Switch to the callback (Parser.Input) overload via a new parseSourceSafe
helper. tree-sitter pulls source in 16 KiB chunks via repeated callback
invocations, bypassing the broken conversion path. Files are parsed in
full, no data loss, no platform-specific code path.

Removes the now-unnecessary ERROR-root short-circuit in csharp/captures.ts
and the empty-captures shim in scope-extractor-bridge.ts; both existed only
to swallow truncation-induced parse failures.

* fix(windows): cover all parse sites and correct vector-extension state

Address adversarial review on PR #1433:

1. Extend parseSourceSafe to all remaining parser.parse() call sites that
   handle full file content. The first commit only converted the four
   sites with active truncation hacks; cache-miss paths in
   call-processor (x2), heritage-processor (x2), import-processor, and
   the Go/Python/TypeScript captures + Go range-binding still called
   parser.parse() directly. On Windows those would still SIGSEGV for
   files > 32767 chars.

2. Stop setting vectorExtensionLoaded = true on the win32 short-circuit
   in lbug-adapter.ts. The flag means "successfully loaded" and is
   checked by an early-return at the top of loadVectorExtension; setting
   it on the skip path made the second call return true and let
   QUERY_VECTOR_INDEX run against a DB without the extension.

3. Drop the placeholder issues/... URL in the same comment.

4. Add unit tests for parseSourceSafe at boundary values: 16 KiB
   (direct/callback boundary), the 32 767 Windows crash boundary,
   single-line > chunk size, CRLF near boundary, and large all-Chinese
   source. Confirms the callback path is correct for non-ASCII content,
   which is also exercised by the existing csharp-captures large-file
   test.

Researched the chunking concern: tree-sitter Node binding sets
TSInputEncodingUTF16 and divides byte_index by 2 in ByteCountToJS before
calling the JS callback, so the index argument is a UTF-16 code-unit
offset — matching String.prototype.slice. Splitting tokens across chunks
is safe by API contract; the lexer is chunk-agnostic.

* fix(windows): extend parseSourceSafe to group/embeddings + lint enforcement

Closes the remaining Windows SIGSEGV exposure flagged by the Codex
adversarial review on PR #1433. Six pre-existing parser.parse(content)
call sites bypassed parseSourceSafe and could crash the process on
Windows when a contract IDL, route file, or embedding-target source
exceeded 32 767 chars. Adds a lint rule so the regression vector closes
permanently.

Production code:
- Relocate parseSourceSafe from ingestion/utils/ to core/tree-sitter/
  so group/ and embeddings/ can import without crossing into ingestion
  internals. core/tree-sitter/ already houses parser-loader.ts and is
  the natural shared facade. All 11 existing importers updated; no shim
  left behind in the old location.
- Route through parseSourceSafe in 5 group extractors (grpc, thrift,
  http-route, include, tree-sitter-scanner) and the embeddings
  ensureAndParse helper.
- The seventh direct .parse() call in grpc-patterns/proto.ts:49 is a
  module-load grammar smoke test parsing a 36-char literal. Trivially
  safe by inspection, intentionally direct, filtered out by the lint
  rule via the string-literal-arg skip.

Tests:
- 5 caller-side regression tests with a vi.spyOn assertion on
  parseSourceSafe. The spy is what catches a regression: parser.parse
  on a 40 000-char input succeeds on Linux/macOS, so a "no throw"
  assertion alone would silently pass with the bypass reintroduced.
- The vi.mock boilerplate is centralised in
  gitnexus/test/helpers/parse-source-safe-mock.ts, dynamic-imported
  inside each mock factory so vitest's hoister does not race the
  static import binding.

Lint:
- New custom ESLint rule gitnexus/require-safe-parse, scoped to
  gitnexus/src/core/**, fails on direct <parser>.parse(<non-literal>,
  ...) calls and auto-fixes them to parseSourceSafe(<parser>, ...).
  Skips JSON/URL/marked/Number/Math, string-literal first args
  (smoke tests), test files, and the helper itself. Auto-fix rewrites
  the call site only; the developer adds the import after tsc
  surfaces the missing identifier — same tradeoff as
  unused-imports/no-unused-imports.

Plan: docs/plans/2026-05-10-001-fix-windows-parse-safety-group-and-embeddings-plan.md

* fix(test): use mkdtempSync in http-route-extractor regression test

Address CodeQL js/insecure-temporary-file warning on the new Windows-
SIGSEGV regression test. The test was using path.join(tmpDir, "large-input")
which, when nested inside a Date.now()-based parent tmpDir, lets CodeQL flag
the directory as a predictable-name temp file with race-condition risk.
Switch to fs.mkdtempSync(path.join(tmpDir, "large-input-")) so the suffix
is a secure unique random string.

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-05-10 16:00:36 +01:00
WENJIE HUANG
32b5c0e3fc
feat: add IncludeExtractor for C++ cross-repo include tracking (group) (#1156)
* feat: add IncludeExtractor for C++ cross-repo include tracking (group)

* fix: address CodeQL warnings on include-extractor

- Remove unused HEADER_GLOB constant in include-extractor.ts
- Use fs.mkdtempSync for secure temp dir creation in tests
  (CodeQL: 'Insecure temporary file')

* fix(group): close missing ); in manifest-extractor include branch

The 'include' branch in ManifestExtractor.resolveSymbol was missing
the closing ); for the executor() call, causing a syntax error that
broke ESLint, Prettier, and the full test CI on all platforms.

Reported by Claude PR review on #1156.

* chore: drop test/global-setup.ts + test/vitest.d.ts

Upstream removed these in commit 3f0c74fe (ladybugdb 0.16.0 upgrade).
Commit 3f5d21c5 accidentally restored them during a rebase dance.

* style(group): reformat VALID_CONTRACT_TYPES array to satisfy prettier

Adding 'include' pushed the array over prettier's 100-char limit,
so prettier prefers multi-line. Apply the reformat to unbreak
ci-quality/format job.

* fix(include-extractor): address PR #1156 Claude review findings #3-#7

Claude Deep Review raised 7 findings on the IncludeExtractor. #1/#2
(BLOCKERs) were fixed earlier. This commit closes the remaining five.

#3 HIGH  case-sensitive FS -> provider contract-id collision
  Document the deliberate case-folding trade-off on normalizeIncludePath
  (matches C/C++ convention on Windows/macOS; collapses Foo.h & foo.h on
  Linux). Add a unit test pinning the behavior.

#4 HIGH  suffixResolve short-suffix match silently drops cross-repo include
  When a local file ends with the same basename as an external include
  (e.g. local internal/api.h vs. #include "ext/api.h"), suffixResolve
  returned a bogus local hit and suppressed the cross-repo consumer.
  Replace the suffixResolve lookup inside include-extractor with a
  strict isLocalInclude() that only accepts full-path hits via
  SuffixIndex.get / getInsensitive. Callers of suffixResolve elsewhere
  are unaffected. Add 3 unit tests covering the regression.

#5 MEDIUM regex fallback matched #include inside /* ... */
  Strip block comments before running the fallback regex scan.
  Add a unit test.

#6 MEDIUM meta.source was hard-coded to 'tree_sitter'
  Track the actual extraction path with an extractionSource local and
  write it into meta.source so downstream audits can distinguish
  tree-sitter parses from regex fallbacks. Add 2 unit tests.

#7 MEDIUM missing end-to-end coverage
  Add test/integration/group/include-extractor-sync.test.ts with 3
  cases exercising extractor -> syncGroup -> CrossLink (mocked
  contracts, mixed-case/backslash normalization, real temp repos).

Tests: 21 unit + 3 integration, all green.

* fix(lbug): robust Windows lock acquisition for CI integration tests

LadybugDB's `new Database()` raises `Could not set lock on file` from
local_file_system.cpp synchronously inside the constructor — before any
query is issued, so `withLbugDb`'s query-time retry never sees it. On
Windows CI this surfaces as flaky integration tests due to AV-scanner
holds, libuv handle-release lag, and stale `.wal` sidecars from aborted
prior runs.

This change closes the gap at *open time*:

- `openLbugConnection` now wraps `new lbug.Database()` in a bounded
  busy-retry (5x100ms back-off) inside `lbug-config.ts`. Errors that
  exhaust the budget are tagged via `LBUG_OPEN_RETRY_EXHAUSTED` so
  `withLbugDb`'s outer 3x retry skips re-retrying a freshly-exhausted
  path (eliminates the 3x5=15-attempt / ~6s tail latency).
- For recognized test fixtures only (immediate-parent dir matches a
  known prefix AND resolves under `os.tmpdir()`), one final stale-
  sidecar sweep removes `.wal`/`.lock` and retries once. Production
  paths never enter this branch.
- `safeClose` on Windows runs a bounded `fs.open` probe to absorb
  native handle-release lag; logs a warning if the probe exhausts so
  operators can spot AV interference.
- `isDbBusyError` is now defined in `lbug-config.ts` as the single
  source of truth, re-exported from `lbug-adapter.ts` for compatibility.
- New tests cover open-time retry (happy/retry/exhaust/non-busy/tag),
  stale-sidecar sweep (test-fixture-only, production-rejection,
  preserves-original-error), `isTestFixturePath` direct unit suite
  (accept/reject/traversal/nested/trailing-sep), and
  `waitForWindowsHandleRelease` (openable/ENOENT/no-leak).
- The two new test files are added to vitest's existing serialized
  `lbug-db` project (already `fileParallelism: false`).

Closes the chronic Windows CI flake on lbug-touching integration tests
while preserving the existing single-writable-Database-per-process
LadybugDB contract. No public API surface changed.

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

* refactor(lbug): drop isDbBusyError re-export, import from lbug-config directly

The re-export from lbug-adapter.ts was a transitional convenience — with
the matcher now living in lbug-config.ts, having two import paths for the
same symbol invites future drift. Updated the two real consumers
(lbug-lock-retry.test.ts, lbug-open-retry.test.ts) to import from
lbug-config directly, removed the re-export equality test (now vacuous),
and refreshed the explanatory comment so it no longer references a
re-export pattern that doesn't exist.

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

* fix(lbug): silence benign LadybugDB v0.16.1 schema-init lock warnings on Windows

doInitLbug logs "⚠️ Schema creation warning: ... Could not set lock on
file" on every CREATE NODE TABLE call after the first init on a given
dbPath, on Windows. The lock is internal to LadybugDB v0.16.1 and is
resolved before the table is created — same tolerance pattern as the
existing "already exists" filter. Genuine cross-process lock contention
still surfaces on the next operation through withLbugDb's retry, so
filtering at the schema-init catch only suppresses noise, not signal.

Also extend the safeClose Windows handle-release probe to cover the
.wal sidecar (the previous Database's WAL handle was the slowest to
release, surfacing as the schema-query lock contention) and switch the
probe back to 'r+' so it actually detects exclusive locks.

Test loop in lbug-close-handle-release.test.ts simplified to 10 plain
iterations now that the underlying noise is filtered upstream.

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

* chore(lbug): isDbBusyError review fixes

- Drop redundant `could not set lock` term — already subsumed by `lock`.
- Document the intentionally-broad matcher: graph-DB lock-shaped errors
  ("deadlock", "unlock failed", "lock contention", "could not open lock
  file") are all treated as transient. If a non-transient surfaces,
  tighten the matcher rather than raise the retry budget.
- Add positive test cases covering those lock-shaped strings so the
  intent is visible and a future tightening would deliberately break
  these.
- Fix the open-retry back-off comment: max sleep is 100+200+300+400 =
  1000ms (no sleep after the final attempt), not 1.5s.

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

* fix(group): address PR #1156 follow-up review findings

Addresses two blockers and two mediums from the deep review.

BLOCKER 1: Windows CI ENOTEMPTY in sync.test.ts
  After this PR added writeBridge() to syncGroup, the existing test
  "writes registry to groupDir when skipWrite is false" fails on
  windows-latest. LadybugDB's checkpoint thread briefly outlives
  closeBridgeDb, holding a Win32 lock on bridge.lbug; the test's
  fs.rmSync then fails with ENOTEMPTY. Switched the test cleanup to
  cleanupTempDir from test/helpers/test-db.ts which already tolerates
  EBUSY/EPERM/EACCES/ENOTEMPTY with bounded retries — same pattern
  used elsewhere for LadybugDB-touching tests.

BLOCKER 2: Graph provider absolute-path bug
  extractProvidersGraph queried File.filePath from the LadybugDB graph
  but never stripped the repo root, so provider contract IDs ended up
  as include::/abs/path/foo.h while consumers emitted include::foo.h.
  These never matched through runExactMatch — silently producing 0
  cross-links for any indexed C++ repo (the primary use case).
  Now passes repoPath into extractProvidersGraph and applies
  path.relative(); rows that resolve outside repoPath (stale absolute
  paths from another machine, system headers somehow indexed) are
  dropped instead of polluting the registry.

MEDIUM: `../` relative includes produce spurious noise
  `#include "../foo.h"` is almost always intra-repo, but the suffix
  index can never match a `..`-prefixed path so it became a consumer
  contract no provider could satisfy. Now skipped before matching;
  covers both forward-slash and backslash forms.

MEDIUM: writeBridge error in sync.ts propagates uncaught
  contracts.json is the canonical source of truth and was just written
  successfully when writeBridge runs. A bridge-only failure (disk full,
  schema error, permission denied) shouldn't mask the registry. Wrapped
  writeBridge in try/catch with a logger.warn surfacing the path and
  recovery instructions.

Tests added:
  - extractProvidersGraph repo-relative ID generation (stub Cypher
    executor returns absolute paths)
  - extractProvidersGraph drops rows whose path resolves outside repo
  - `../foo.h` forward-slash skip
  - `..\foo.h` backslash-form skip

Skipped findings:
  - canExtract() removal (#5, low): canExtract is part of the
    ContractExtractor interface; every other extractor implements the
    same `return true` shape. Removing it from IncludeExtractor would
    break the interface contract — keeping for consistency.

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

* fix(group): close PR #1156 Codex adversarial findings

Two HIGH findings from the Codex adversarial review on
feat/group-include-extractor:

1. Default-on extraction silently changes existing groups (BLOCKER)
   DEFAULT_DETECT.includes was true, so any pre-existing group.yaml
   that omits the new field would gain a wave of include::* contracts
   on the next sync after upgrade. Flipped to false (opt-in). The
   integration test already declares includes: true explicitly so it
   survives unchanged; the unit extractor tests bypass parseGroupConfig
   entirely; the sync test uses extractorOverride. Only config-parser
   needed regression tests covering omitted/explicit/false variants.

2. IncludeExtractor scans outside the indexed file universe (BLOCKER)
   The extractor was running glob('**/*', { ignore: STANDARD_IGNORES })
   twice with a hand-rolled 9-pattern list, no .gitignore/.gitnexusignore
   honoring, and no max-file-size cap. That meant File:<path> contracts
   could appear for files ingestion would never index, producing
   cross-links group impact cannot fan out to (silent false-negatives).
   Refactored to a single discoverIndexableFiles() helper that mirrors
   walkRepositoryPaths exactly: createIgnoreFilter + getMaxFileSizeBytes,
   one discovery pass shared by provider and consumer paths. Dropped
   STANDARD_IGNORES and SOURCE_GLOB entirely.

   third_party and 3rdparty (the C/C++ vendored-deps conventions) were
   in the local ignore list but not in the canonical DEFAULT_IGNORE_LIST
   used by ingestion. Folded both into the canonical set rather than
   keep a parallel list — the whole point of the Codex finding is that
   two file-discovery implementations drift. Single source of truth.

Tests: 5 new regression tests for the discovery alignment (.gitignore,
.gitnexusignore, max-file-size on both provider and consumer paths)
plus 4 for the opt-in default. All 30 include-extractor tests + the
494-test group suite + ignore-service tests pass.

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

* fix(review): apply autofix feedback

ce-code-review surfaced 6 safe_auto findings on commit a9936a9b:

- T1 (testing, P2): the sync.ts:174 gate was untested with includes:false.
  Added a sync-level test mirroring the existing thrift-off pattern at
  sync.test.ts:545, asserting zero include contracts when the gate is
  disabled in a real syncGroup call.

- T3 (testing, P3): third_party and 3rdparty entries in DEFAULT_IGNORE_LIST
  had no regression test. Added both to ignore-service.test.ts's
  dependency-directories it.each block.

- M1 (maintainability, P3): discoverIndexableFiles JSDoc lacked a
  fork-warning relative to walkRepositoryPaths. Added a MAINTENANCE
  note explaining why the duplication is tolerated and the contract
  the two implementations must keep.

- M2 (maintainability, P3): thrift-extractor still hand-rolls its
  ignore array with no signal that DEFAULT_IGNORE_LIST additions
  silently do not apply there. Added TODO(#1156-followup) comments
  above both call sites.

- M3 (maintainability, P3): SOURCE_EXTENSIONS duplicated the four
  HEADER_EXTENSIONS entries with no expressed subset relationship.
  Spread HEADER_EXTENSIONS into SOURCE_EXTENSIONS so future header-
  extension additions propagate.

- C1+T4 (correctness+testing, P3, cross-reviewer corroborated):
  discoverIndexableFiles swallowed all fs.stat errors silently,
  including EACCES/EMFILE/EIO. Narrowed the catch to ENOENT (the
  documented benign glob/stat race) and added a logger.warn for
  any other code so operators can spot permission/resource issues.

All 629 tests pass; typecheck + prettier clean.

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

* fix(group): use retryRename in writeContractRegistry to absorb Windows EPERM

`storage.ts:62` used raw `fsp.rename` for the contracts.json atomic swap.
On Windows, AV scanners and concurrent renames briefly hold the
destination handle between rename calls, surfacing as EPERM/EBUSY.
The `insecure-tempfile.test.ts > concurrent writes do not collide`
test was flaking with `EPERM: operation not permitted, rename` on
windows-latest CI.

`bridge-db.ts` already has a battle-tested `retryRename(src, dst, 3)`
helper used at six call sites for exactly this pattern. Reusing it
here keeps the Windows-rename policy single-source-of-truth across
the group package.

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

* fix(group): drop macro-style #include from consumer contracts

Tree-sitter's `(_) @import.source` wildcard matches the identifier node
of `#include PLATFORM_HEADER`, so the cleaned value `PLATFORM_HEADER`
slipped past the system-header / `..` filters and was emitted as a
permanently orphaned consumer contract (no file is named after a macro
identifier, so no provider can ever match). Add a shape guard that
skips cleaned values lacking both a path separator and an extension
dot, plus regression tests for single and multi-macro files.

Also document `IncludeExtractor.canExtract()` as unused by sync.ts
(gated via `config.detect.includes` instead) and kept solely for
ContractExtractor interface uniformity.

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

---------

Co-authored-by: HuangWenjie <zhoudeng.hwj@alibaba-inc.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 09:31:59 +01:00