Commit graph

2 commits

Author SHA1 Message Date
Yayler
3aa62be717
feat: add gitnexus auto-sync for scheduled remote clone and analyze (#2493)
* adds an opt-in auto sync and analysis loop for GitNexus

* adds an opt-in auto sync and analysis loop for GitNexus,gitnexus watch [init|start|restart|stop|status]

* adds an opt-in auto sync and analysis loop for GitNexus,gitnexus watch [init|start|restart|stop|status]

* fix: address PR review cleanup

* Prettier code style

* merge main

* fix(watch): protect local repos and cancel active analysis

* fix(watch): harden auto-sync lifecycle and locking

- validate watch process identity before lifecycle operations\n- serialize registry, analysis, and LadybugDB access with recoverable locks\n- harden clone paths, symlinks, hooks, quarantine, and worker timeouts\n- install procps in the CLI image for reliable Docker watch control\n- add focused regression coverage for lifecycle, locks, clone, and registry behavior

* update agents & claude md

* merge main

* fix(watch): harden auto-sync lifecycle

* fix(watch): normalize SSH repo identity paths

* fix(watch): normalize SSH repo identity paths

* fix(watch): safely cancel analysis across platforms

* fix(auto-sync): close worker and group sync failure paths

* fix(auto-sync): drop retired allowStale from group sync

allowStale was removed from SyncOptions, which broke typecheck and CI on this PR.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(watch): satisfy prefer-const and Prettier in auto-sync

The watch timers are assigned exactly once, so prefer-const rejected the
deferred `let` declarations. They are only read from `stop()` and the control
poll, both of which run after the assignments, so binding them at creation is
safe and drops the now-dead undefined guards.

Remaining files are formatting only.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(watch): make lock identity absolute and stop three fail-open paths

Lock owner identity was rendered by `ps -o lstart=` through localtime and the
active locale, so the same live process produced a different string under a
different TZ. A mismatch reads as PID reuse, so one daemon could reclaim a
mutex another still held. Pin TZ=UTC and LC_ALL=C.

The owner record also carried no hostname, so a holder on another machine was
judged by this kernel's view of its PID — always "stale" — and its lock stolen
whenever GITNEXUS_HOME is a shared volume. Record and compare the hostname, as
the index lock already does.

Ownership verification threw unconditionally on win32, which is reached once
per project per tick, so watch reported `running` and then failed every repo
forever. POSIX uid/mode cannot be checked there; skip those two assertions and
keep the dangerous-root, symlink, containment and internal-root guards.

Also: quarantine sweep now refuses a symlinked root instead of deleting
through it; an unreadable state file propagates instead of being rewritten as
empty state, which used to erase every repo's analyzed commit and failure
count; a failed staging cleanup no longer strands a published lock with no
release handle; and the concurrency runner settles every worker before
surfacing a failure so cancellation cannot orphan a live analyze fork.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(watch): land the deferred review findings

Six findings that were deferred from the review backlog, plus the docs they
change.

Worker heap: admission allowed `floor(availableMemoryGB / 2)` slots while every
fork was handed the whole machine's heap cap, so the budget meant nothing as
soon as an operator raised max_concurrency. Divide the cap by the repos
actually analyzed in parallel. The default single-project path is unchanged.

Registration: the parent registered without a branch, so it always took the
primary/flat arm and relabelled a pinned branch entry on the branch-fallback
path. Reproduce the worker's own resolveBranchPlacement decision instead.

Cancellation: requestCancellation cleared the only timer and settled nothing,
so a worker wedged past its safe point left the promise pending forever,
wedging activeRun and hanging `watch stop`. Add a 5s grace after which the
parent stops waiting and releases the IPC channel's hold on its event loop.
The child is still never killed — it may be inside native work.

overwrite_local_changes: `checkout --force` rewrites tracked files only, so
untracked sources survived and were indexed as if they came from the remote.
`git clean -fd -e /.gitnexus` after checkout; no -x/-X, so ignored paths and
GitNexus's own storage survive.

Quarantine: age alone never bounds a repo that fails every tick, since each
partial clone is younger than the retention window. Keep the five newest per
repo.

Validation: repo_git_timeout is now bounded by the lesser of an hour and the
sync interval, which is also the guard for the bare-number-means-seconds slip
(`600000` meant ~7 days and cleared the timer ceiling). And the remote URL's
final segment is validated at config load rather than failing once per tick
inside the sync loop.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(watch): release an errored worker, and stop rejecting dotted repo names

Three findings from the latest review pass.

The 'error' handler settles immediately rather than waiting out the grace, so
cleanup() clears the grace timer that would otherwise have released the child.
An errored IPC channel does not mean the worker stopped, so release it on that
path too — still no kill.

The traversal guard tested the raw path for '..', which also rejected an
ordinary name like owner/foo..bar that the repository-name rule accepts.
Traversal is a whole segment, so test segments.

The heap-cap test left two runs and their real timers pending; it now stubs
timers and settles both promises. Registration coverage now pins the branch
slot rather than leaving it implicit.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(watch): validate namespace segments and pin the stopped process identity

Replacing the raw-string `..` test with a per-segment one dropped a guard: a
segment like `..\..\outside` is not literally `..`, so it passed, and those
segments build the clone path — on Windows the backslashes are separators.
Hold every namespace segment to the same charset as the repo name, which keeps
a separator out of a segment while still allowing an ordinary `foo..bar`. The
final segment keeps its own check so a bad repo name keeps its own message.

The stop wait polled liveness by pid alone, so a pid reused mid-wait would
have it wait on an unrelated process and then report the watch stopped. Compare
the process start time recorded for the owner, which also returns sooner.

Registration now omits `branch` for a primary index instead of passing it as
undefined, so that call keeps the shape it had before this branch.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(cli): ship auto-sync as the remote daemon, reserve gitnexus watch.

Keep analyze --watch for local incremental re-index and stop the top-level watch verb from starting a clone/pull loop.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): reject invalid branch refs and verify status identity (#2493)

Reject leading slashes and per-component trailing dots in configured branches, and verify the live watch owner before trusting a stored error status.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): reject ownerIds that can escape the watch directory (#2493)

Stop interpolating a tampered ownerId into the stop-request filename; only basename-safe values are treated as owners.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): recognize auto-sync in the watch-process identity check (#2493)

Stop/status were still looking for a standalone watch token after the command rename, so a live gitnexus auto-sync start process would be refused as unrelated.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

* fix(auto-sync): reject boolean max_concurrency instead of coercing it to 1 (#2493)

Number(true) is 1, so a YAML boolean would have passed the integer check and silently meant one worker.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): swallow status errors in the watch finally path (#2493)

An uncaught updateStatus rejection in finally became an unhandled
rejection. Skip the clone-root symlink test on Windows, where directory
symlinks need privileges. Align the group-lock comment with fail-closed
registry timeouts.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): catch cancelling status-write failures (#2493)

Fire-and-forget updateStatus('cancelling') could become an unhandled
rejection, the same class as the finally-path status write.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): ignore queued interval ticks after stop (#2493)

clearInterval does not cancel a timer callback already queued. Guard
runSafely on stopping so shutdown cannot start a new un-cancellable run.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(auto-sync): report stored watch status timestamps (#2493)

status should show when the watch last entered a state, not when the
CLI queried it. The failure-count test still expects 1 after a new
commit resets the streak; rename it so that reset is explicit.

Co-authored-by: Cursor <cursoragent@cursor.com>

* style(auto-sync): apply prettier to starter status logger (#2493)

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: weiyf <weiyf3634@163.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-09-02 19:40:18 +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