* fix(lib): add stripWindowsLongPathPrefix for path comparisons (#2667)
A caller can hand GitNexus a `\\?\`-prefixed path — the usual MAX_PATH
workaround on Windows — and `path.resolve` preserves the prefix, so it
reaches every string comparison GitNexus keys paths on. It also poisons
relativization: `path.win32.relative` cannot express a relative path
between a prefixed and an un-prefixed form of the same directory, so it
returns the absolute target instead. That absolute string is the shape
reported in #2667.
The helper is deliberately scoped to the comparison domain. libuv's
`fs__capture_path` does not re-add the prefix for over-MAX_PATH paths, so
stripping a filesystem-facing path would break long-path access on hosts
that have not opted into LongPathsEnabled. `\\?\Volume{GUID}\…` is left
alone because the remainder is not a usable path.
The test is fixture-free and takes an explicit `platform`, mirroring
`normalizeAnalyzerRootPath`, and is registered on the cross-platform
matrix since the whole transform is a POSIX no-op.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* fix(storage): normalize the `\\?\` prefix in canonicalizePath (#2667)
`canonicalizePath` is the single comparison key for the repo registry,
MCP repo resolution and the server repo routes, and `registryPathEquals`
compares its output as a plain string. A caller-supplied `\\?\` prefix
therefore matched nothing: a repo registered as `D:\repo` was invisible
to a caller passing `\\?\D:\repo`, which surfaces as "repo not found" or
a duplicate registration from `analyze`, `remove`, `clean`, the MCP
`repo` parameter and the server routes.
Both branches are normalized. The realpath branch was already safe —
libuv's `fs__realpath` strips the prefix itself — but the `catch`
fallback returns `path.resolve(p)` untouched, and that is exactly the
branch a path which is not on disk takes.
Safe despite the CRITICAL blast radius (27 impacted, 12 direct
dependents) because the result is only ever compared, never opened: all
23 call sites feed `registryPathEquals` or a string comparison. Both
operands are canonicalized, so the equality relation is preserved and
behaviour is unchanged for every un-prefixed input.
The two regression assertions run only on windows-latest, where the file
already runs via the cross-platform matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* docs(core): correct two false comments about Windows paths (#2667)
Both comments assert the opposite of how the platform and the analyzer
actually behave, and both would send the next investigator of #2667 the
wrong way.
`analyzer-identity.ts` claimed the `\\?\` prefix is one that
`realpathSync.native` "can emit for paths over MAX_PATH". libuv's
`fs__realpath_handle` strips the prefix unconditionally and rewrites
`\\?\UNC\` back to `\\`, erroring if neither is present, so realpath
never returns one. The prefix can only arrive from caller-supplied
input. The optional group in the regex stays as a labelled defensive
no-op, and the function's behaviour is unchanged on purpose: these
identity fields are compared between an `analyze` and a later `status`
run, so this is not the place to reshape a path.
`include-extractor.ts` claimed "gitnexus analyze stores absolute paths in
the File.filePath column". A full self-index at 89bbdcf5 had 0 of 239,070
nodes with an absolute or backslash-bearing filePath: File nodes are
built from the walker's repo-relative forward-slash paths. The
relativization guard below it stays, now described as what it is — a
guard against rows this process did not write.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* test(lib): pin that path.resolve preserves the `\\?\` prefix (#2667)
The `canonicalizePath` regression tests for the `catch` fallback can only
run on windows-latest, so the fact they rest on is invisible in the Ubuntu
suite. Pin it here, in the fixture-free file that runs everywhere:
`path.win32.resolve` carries the prefix through untouched, which is all
the fallback branch used to do before this fix.
Also pins the forward-slash spelling (`//?/D:/…`), which the helper
deliberately does not match because `resolve` folds it into the backslash
form first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* fix(lib): match the `\\?\UNC\` token case-insensitively (#2667)
The namespace `\\?\` addresses is the Windows object namespace, which is
case-insensitive, so `\\?\unc\server\share` is as valid as the uppercase
spelling. Matching only `UNC` left the lowercase form prefixed, which is
the same registry mismatch #2667 is about, reached through a network
share instead of a drive.
The drive branch was already case-insensitive (`[A-Za-z]`), so the two
branches disagreed with each other. Probed against the built artifact:
`\\?\unc\…`, `\\?\Unc\…` and `\\?\UNC\…` now all yield
`\\server\share\…`, and `\\?\Volume{…}` is still left alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* test(storage): guard the canonicalizePath fix on Linux too (#2667)
The two `canonicalizePath` assertions in `repo-manager.test.ts` drive the
real `realpathSync.native`, so they are `it.skipIf(win32)` and only run on
the windows-latest matrix leg. The Ubuntu gate — the one every PR runs —
had no coverage of the behaviour at all.
This runs the same wiring anywhere by injecting only the platform
primitives: `path` becomes `path.win32` (Node's real Windows path
implementation, not a stand-in), `realpathSync.native` gets its two actual
behaviours (libuv strips `\\?\` for a path on disk, throws ENOENT for one
that is not), and the real `stripWindowsLongPathPrefix` is pinned to
win32 rather than defaulting to the host. `canonicalizePath` and
`registryPathEquals` run unmodified.
Pinning the helper is a module mock rather than an override of
`process.platform`, which is shared by every test file in a worker.
Verified to discriminate: against the pre-fix tree at 89bbdcf5 it fails 3
of 5, and reverting just the two strip calls on this branch reproduces the
same 3 failures with `expected '\\?\D:\Projects\moved-away' to be
'D:\Projects\moved-away'`. The two that pass either way are the realpath
branch and the un-prefixed no-op, neither of which ever leaked.
Not registered in scripts/cross-platform-tests.ts: it simulates Windows
rather than needing it, so its home is the Ubuntu suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* fix(lib): require the component that makes a stripped path usable (#2667)
Both regexes were under-anchored, so the slice could emit something worse
than the input it was handed. `\\?\UNC` has no share to keep and became
the bare root `\\`; `\\?\D:foo` is drive-relative and became `D:foo`,
which is not absolute and would resolve against the process cwd if a
future caller ever passed it to `fs`. `canonicalizePath` previously
always returned an absolute path and had stopped doing so.
Each pattern now requires the part that makes the remainder a real path —
a share name after `UNC\`, a separator after the drive colon. Malformed
extended paths are left untouched and simply fail to match a registry
entry, which is the safe direction.
Also from review: document `\\.\` as a deliberate non-goal alongside
`\\?\Volume{GUID}\` (most of what it addresses is not a filesystem path),
correct the canonicalizePath docblock, which still claimed entries are
canonicalised at write time — `registerRepo` stores `path.resolve` and
the paragraph added two lines above says compare-only — and reword the
cross-platform registration comment, which claimed the test is only
meaningful on windows-latest when every assertion passes an explicit
'win32' and runs identically on Ubuntu.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* fix(group): keep repo-relative rows in the graph provider strategy (#2667)
`extractProvidersGraph` relativised every row with
`path.relative(normalizedRepoPath, absolute)`. The rows analyze actually
writes are repo-relative — which is exactly what the comment corrected
earlier in this branch establishes — and `path.relative` resolves a
relative second argument against the PROCESS CWD. So from any cwd other
than the repo root, every row came back `..`-prefixed, the containment
guard dropped it, and the strategy silently returned [] and fell through
to the filesystem fallback.
Only absolute rows go through `path.relative` now. The containment guard
is unchanged, so foreign and escaping rows are still rejected.
Found by three independent reviewers reading the comment this branch
corrected and following it to its consequence. The regression test fails
without the guard (`expected false to be true`) and passes with it;
vitest runs from `gitnexus/`, never the fixture dir, so it exercises the
cwd mismatch by construction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
* test: close the coverage gaps the review surfaced (#2667)
Adds the assertions the reviewers named as missing, and corrects one more
comment that gave the right conclusion for the wrong reason.
analyzer-identity: the comment said the `\\?\` prefix is preserved so the
identity fields keep a stable compare shape. That is true but secondary.
The load-bearing reason is that these roots are READ FROM — resolveBuildRoot
joins package.json onto packageRoot, collectBuildEntries walks buildRoot,
and the lockfile lookup walks packageRoot's ancestors — so stripping here
would break analyzer identity on a deep checkout for exactly the reason the
ingress strip was withdrawn.
Helper: near-miss spellings (`\\??\`, `\\?\\`, single-backslash, GLOBALROOT)
and forward-slash/mixed-separator forms are pinned as untouched, plus
degenerate and empty input.
canonicalizePath: volume-GUID and `\\.\` are asserted unmatched through
canonicalizePath itself, not just the helper, so the deliberate branch
asymmetry is pinned where it is consumed.
assertSafeStoragePath: prefixed path + prefixed storagePath passes, mixed
form throws. This guard fronts fs.rm(recursive) and deliberately does NOT
canonicalize; "complete the fix by stripping here too" is the tempting
follow-up and would widen what the recursive delete accepts.
resolveRegisteredRepoEntry: the consumer surface the fix exists for — an
MCP `repo` argument or `?repo=` value in the prefixed spelling now resolves
its un-prefixed entry, and a prefixed path naming no entry still fails
closed. Verified to discriminate: reverting the strip fails this test along
with the three catch-branch ones.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119fkrRFdQDQKh58LY9Tqh2
---------
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* 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>
* 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>
Gergő Magyar