Commit graph

8 commits

Author SHA1 Message Date
Zander Raycraft
fca30c7e26
fix(audit): Centralize heritage supertype matching (#1921/#1922) (#1940)
* fix(audit): Centralizes heritage supertype matching so qualified, generic, scoped, and interface bases produce inheritance edges across all OO languages, with per-language configs and fixtures.

* fix(audit): Harden parsing for #1922 with per-parse timeouts, ERROR/partial parse flags, tree-sitter pinned to 0.21.1, and CI ABI checks for every grammar.

* fix: action lint passing

* fix: feedback from triage review

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-06-01 13:16:17 +01:00
Gergő Magyar
d3de5fa5d5
fix(install): materialize vendored grammars to fix Windows EPERM (#1728) (#1729)
* fix(install): materialize vendored grammars to fix Windows EPERM (#1728)

Stop using file: optionalDependencies for tree-sitter-dart/proto/swift,
which made npm symlink vendor paths on install and fail on Windows without
symlink privileges. Copy vendor trees into node_modules at postinstall
instead; keep native builds and #836 vendor hygiene.

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

* fix(install): atomic materialize swap + fail-soft tests (#1728, #836)

Hardens PR #1729 against two issues the original implementation could
still hit:

1. Torn-state on rmSync→cpSync. The previous loop deleted the
   destination before copying. If cpSync threw — the exact Windows EPERM
   scenario this PR targets — a previously-working grammar was silently
   wiped. Now we copy to {dest}.materialize-tmp first and renameSync into
   place, so an interrupted copy leaves the prior materialization intact.

2. Fail-soft try/catch had no test coverage. Adds two POSIX-only tests
   (chmod 0o555 to deterministically force cpSync to throw) that verify
   (a) a single grammar failure does not abort the other two, and (b) an
   existing materialization survives a partial-copy failure. Skipped on
   Windows where chmod doesn't enforce write restriction; runs on Linux
   CI.

Other test improvements locking in the install-hygiene invariants:

- All three vendored grammars (dart/proto/swift) checked, not just dart.
- GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 short-circuit is exercised.
- Vendor cleanliness (#836): no node_modules/build under vendor/.
- Idempotent re-runs (clean overwrite verified via sentinel file).
- Missing-vendor warn+continue path now has explicit coverage.
- Vendored package manifests asserted to carry no install script or
  runtime dependencies.
- package.json optionalDependencies asserted free of vendored grammars.
- package-lock.json assertion tightened from `if (entry !== undefined)
  { expect(entry.link).not.toBe(true); }` (vacuous when entry is absent,
  i.e. the expected post-fix state) to `expect(...).toBeUndefined()`.

Verified locally:
- npx tsc --noEmit: clean
- vitest test/unit/materialize-vendor-grammars.test.ts: 8 pass + 2
  POSIX-only skipped on Windows
- npm pack tarball: no vendor/*/node_modules or vendor/*/build entries
- Isolated global install (clean + upgrade + SKIP env) into temp prefix:
  succeeds; gitnexus --version → 1.6.5; vendor stays clean post-install.

* fix(install): address review feedback — Swift parity, atomicity, CI smoke

Resolves all findings from the automated production-readiness review on
verify/issue-1728-symlink.

Swift warning parity (review #2):
  Add tree-sitter-swift to OPTIONAL_GRAMMARS in src/cli/optional-grammars.ts
  alongside Dart and Proto. Before this commit, Swift was materialized at
  postinstall and probed by build-tree-sitter-swift.cjs but the runtime
  warnMissingOptionalGrammars() never warned when it failed to load —
  users got silent Swift degradation from the optional-grammars surface
  (parser-loader's separate unavailableNote only fires on demand). Now
  the warning path matches the materialize path.

README env-var table (review #1):
  Update the GITNEXUS_SKIP_OPTIONAL_GRAMMARS row at README.md line 248 to
  list all three vendored grammars (dart, proto, swift). The quick note
  earlier in the README already mentioned all three; only the table row
  was stale.

Atomicity hardening (review #3):
  materialize-vendor-grammars.cjs now copies to {dest}.materialize-tmp,
  renames the existing dest to {dest}.materialize-bak (if present), then
  renames the partial into dest, then removes the backup. If the
  partial→dest rename fails (e.g. Windows AV scanner racing the swap),
  the catch block restores from backup so the previously-materialized
  grammar is preserved. Closes the narrow torn-state window where the
  prior implementation could leave dest deleted after rmSync succeeded
  but renameSync failed.

Swift probe docs (review #4):
  build-tree-sitter-swift.cjs script header rewritten to describe what
  the script actually does — probe node-gyp-build at install time so
  missing-prebuild failures surface as install-time warnings instead of
  first-parse runtime errors. The script does not "activate" anything;
  the runtime require() in parser-loader does the actual load. Console
  warning text updated to match ("prebuild probe" not "activation").

Windows packaged-install smoke test (review #5):
  New CI job `packaged-install-smoke` in .github/workflows/ci-tests.yml
  matrices on windows-latest and ubuntu-latest. Runs npm pack, installs
  the produced tarball globally into RUNNER_TEMP, then asserts:
    * no vendor/*/node_modules or vendor/*/build (#836 invariant)
    * tree-sitter-{dart,proto,swift} in node_modules are real
      directories, not junctions/symlinks (#1728 invariant)
    * gitnexus --version runs against the installed CLI
  Closes the coverage gap where the existing windows-latest job only
  ran `npm ci` in the source checkout — exercising postinstall but not
  the tarball reify step that historically tripped EPERM.

Verified locally:
  npx tsc --noEmit: clean
  vitest test/unit/materialize-vendor-grammars.test.ts test/unit/cli-commands.test.ts:
    18 pass + 2 POSIX-only skipped on Windows
  prettier + eslint on all changed files: clean

* fix(ci): disable credential persistence on packaged-install-smoke checkout

GitHub Advanced Security (zizmor artipacked) flagged the new
packaged-install-smoke job's actions/checkout step as a potential
credential-persistence risk. The job runs `npm pack` + global install
and never pushes back, so the GITHUB_TOKEN that checkout would persist
in .git/config provides no value and only widens the leak surface (any
future artifact-upload step in this job would carry the token).

Disable persistence explicitly via `persist-credentials: false` on this
job's checkout. Scoped to the new job — pre-existing checkouts above
are left unchanged.

* fix(ci): use find instead of ls for tarball lookup (SC2012)

actionlint shellcheck SC2012 flagged `TARBALL=$(ls gitnexus-*.tgz | head -n1)`.
Switch to `find . -maxdepth 1 -name 'gitnexus-*.tgz' -print -quit` which
handles non-alphanumeric filenames safely. Also add an explicit
empty-result check so the failure mode is a clear error message instead
of a silent `npm install -g ""` later.

* fix(tests): sabotage vendor src (not partial path) in POSIX fail-soft tests

The fail-soft tests in materialize-vendor-grammars.test.ts pre-chmod'd
the destination's .materialize-tmp partial directory to 0o555 to force
cpSync to throw. After the atomicity rewrite (`fix(install): atomic
materialize swap + fail-soft tests`), the materialize script now starts
each grammar's loop with `fs.rmSync(partial, { force: true })`, which
deletes the chmod'd sabotage before cpSync runs — so cpSync succeeds and
the partial is then renamed into dest, leaving the test's `finally`
block with no path to chmod back (ENOENT) and the assertion that proto
remained unmaterialized failing because it materialized cleanly.

Fix: sabotage the *vendor source* directory (which the script reads from
but never modifies) by chmod'ing it to 0o000. cpSync then fails on
readdir, the catch block fires per-grammar, dart and swift still
materialize from their unaffected sources, and the existing-dest
preservation test verifies that a sabotaged second-run leaves the prior
materialization (and its sentinel file) intact.

Tests now pass locally (8 pass + 2 POSIX-only skipped on Windows) and
should pass on macOS/Ubuntu CI where the sabotage runs.

* fix(tests): restrict fail-soft tests to Linux (macOS Node cpSync abort)

Node 22 on macOS aborts the process with `libc++abi: terminating due
to uncaught exception filesystem_error` when fs.cpSync hits a source
directory it can't read — the abort happens at the C++ filesystem layer
and bypasses Node's JS try/catch entirely (nodejs/node#51399). My
chmod-0o000-the-source sabotage strategy triggers this SIGABRT on
macOS CI before the production script's `try { cpSync } catch` ever
runs, so the test sees a child-process crash instead of the fail-soft
warning it's verifying.

The production script's fail-soft is correct on Linux (where EACCES
surfaces as a normal JS exception) and effectively untestable on macOS
via permission sabotage. Real installs don't hit this — npm always
ships vendor/ with readable permissions — so the macOS gap is a test
artifact, not a behavior gap.

Restrict the two chmod-based tests to Linux only by replacing
`skipOnWin` with `linuxOnly`. Linux CI continues to verify both the
one-grammar-fails-others-succeed and existing-materialization-preserved
invariants. macOS and Windows runs skip these two scenarios; the other
8 tests still run on every platform.

* fix(tests): remove materialize unit tests, rely on CI smoke job

The materialize-vendor-grammars.test.ts file has been a recurring source
of platform-specific CI noise:

  - Windows: chmod doesn't enforce read/write restrictions the way POSIX
    does, so the fail-soft tests had to be skipped there.
  - macOS Node 22: cpSync against an unreadable source aborts the process
    with a libc++ filesystem_error (nodejs/node#51399) that bypasses JS
    try/catch entirely — making the chmod-based fail-soft tests
    unrunnable on macOS too.
  - The "vendor-cleanliness" and "idempotency" tests on Windows
    intermittently flake due to fs.cpSync timing on the GitHub runner.

The invariants these tests verified are now covered by stronger,
more realistic surfaces:

  - packaged-install-smoke (ci-tests.yml): runs `npm pack` then
    `npm install -g ./gitnexus-*.tgz` on windows-latest and
    ubuntu-latest, then asserts no vendor/*/node_modules,
    no vendor/*/build (#836), no junctions/symlinks on the
    materialized grammar directories (#1728), and a working
    `gitnexus --version`. This is the actual end-user install path.

  - cli-commands.test.ts (kept, unmodified): asserts package.json
    declares no `file:` optionalDependencies for vendored grammars,
    the Swift vendor manifest carries no install script or
    dependencies, and the postinstall chain runs
    materialize-vendor-grammars.cjs + build-tree-sitter-swift.cjs.
    These are static manifest checks — deterministic, fast, no
    flake risk.

Removing the dynamic script-execution tests trades unit-level coverage
for end-to-end smoke coverage that actually exercises the
`file:` → cpSync change against a real npm install lifecycle, on
the platform the fix targets (windows-latest).

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-21 16:47:22 +01:00
Alex Macdonald-Smith
d91428ad9d
feat(cli): add gitnexus publish for opt-in understand-quickly registry (#1425)
* feat(cli): add `gitnexus publish` for opt-in understand-quickly registry

Adds a small, opt-in command that fires a single `repository_dispatch`
event at `looptech-ai/understand-quickly` to ask the registry for an
instant resync of the current repo's entry. No graph file is uploaded;
the registry pulls from raw.githubusercontent.com per the protocol at
https://github.com/looptech-ai/understand-quickly/blob/main/docs/integrations/protocol.md.

  - Pure helpers (id parsing, payload construction, validation) live in
    `gitnexus-shared/src/integrations/understand-quickly.ts` so the
    package stays Node-free and the same logic is testable in isolation.
  - The CLI command lives in `gitnexus/src/cli/publish.ts`. Without
    `UNDERSTAND_QUICKLY_TOKEN` it is a no-op (exits 0 with one
    informational line); with the token it POSTs the dispatch and
    surfaces 204 / 401 / 404 / 5xx distinctly.
  - The id defaults to `<owner>/<repo>` parsed from the `origin` remote
    and can be overridden with `--id`.
  - Refuses to publish when no `.gitnexus/` index exists, with a
    `gitnexus analyze` hint.

Tests: a new vitest unit covers the pure helpers (8 + 8 + 2 cases) and
the no-token no-op path with a `fetch` spy that fails the test if the
network is touched. README gets a one-paragraph "Publishing to
understand-quickly" section near the existing CLI docs.

* fix(uq-publish): address review blockers + high-severity items

Addresses CodeQL polynomial-regex (HIGH), token-gate ordering, distinct
401/403/404/422 response branches, fetch timeout, expanded test coverage,
tightened owner/repo validation, and non-GitHub remote rejection.

See response thread on PR #1425 for the per-finding rationale.

Signed-off-by: amacsmith <alex.mac@looptech.ai>

* fix(publish): address Claude review on PR #1425

- AbortError → TimeoutError: AbortSignal.timeout() throws a
  DOMException with name 'TimeoutError', not Error{name:'AbortError'}.
  Match the pattern used in core/embeddings/http-client.ts so the
  user-facing "timed out after 15000ms" message actually fires. Update
  the regression test to throw a real DOMException — the previous fake
  was a false-green.
- isValidOwnerRepo: forbid trailing hyphen in the owner segment.
  GitHub rejects this at account-creation time; allowing it here meant
  hand-typed --id values like 'my-org-/repo' would pass our regex and
  422 from GitHub.
- Add publish-command coverage to cli-index-help.test.ts (asserts on
  --id, --skip-git, the registry name, and the token env var) and
  cli-commands.test.ts (asserts publishCommand is exported as a
  function). Catches accidental command-registration deletion.

---------

Signed-off-by: amacsmith <alex.mac@looptech.ai>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-05-09 09:52:26 +01:00
Gergő Magyar
1f6df5fdbb
fix(swift): use official prebuilt parser runtime (#1130)
* fix(swift): use official prebuilt parser runtime

Vendor the official tree-sitter-swift 0.7.1 runtime package so Swift parsing works without source-building, while keeping the repo on the current tree-sitter runtime until the broader upgrade is ready. Also preserves Swift resolver correctness for overloaded owned functions and extension-backed type duplicates now that Swift is available by default.

Made-with: Cursor

* fix(swift): move duplicate type ordering into provider

Keep Swift extension candidate ordering behind the LanguageProvider contract and cover the Swift 0.7 init scanner path so parser runtime changes do not leak language-specific logic into shared resolution.

Made-with: Cursor

* fix(swift): address parser runtime review

Add explicit Swift prebuild checks and vendor guidance so parser runtime packaging remains observable and maintainable.
2026-04-28 09:57:42 +01:00
Gergő Magyar
780ee83d52
fix(install): vendor tree-sitter-dart source (#1125)
Avoid remote git/SSH downloads for the Dart grammar during Docker and npm installs by resolving tree-sitter-dart from vendored source and building it during postinstall.

Made-with: Cursor
2026-04-27 21:15:37 +01:00
Gergő Magyar
acf6fbdd39
feat: configure eslint with unused import removal (#564)
* feat: configure eslint with unused import removal

Add ESLint v9 (flat config) for code quality:
- eslint-plugin-unused-imports for auto-removing dead imports
- @typescript-eslint for TypeScript-aware linting
- eslint-plugin-react-hooks for React hooks rules
- eslint-config-prettier to avoid formatting conflicts
- lint-staged runs eslint --fix before prettier on .ts/.tsx
- CI lint job added to ci-quality.yml

* refactor: remove unused imports via eslint --fix

Auto-fixed by eslint-plugin-unused-imports. No logic changes.

* chore: add eslint fix commit to .git-blame-ignore-revs
2026-03-28 15:28:09 +00:00
Abhigyan Patwari
60c93d7d4a
feat: upgrade @ladybugdb/core to 0.15.2 and remove segfault workarounds (#374)
* feat: upgrade @ladybugdb/core to 0.15.2 and remove segfault workarounds

The upstream fix (ladybug-nodejs#1) resolves the child QueryResult lifetime
segfault, making .close() safe on all platforms. This removes 6 workaround
sites:

- Remove `dangerouslyIgnoreUnhandledErrors` from vitest config
- Remove platform-conditional .close() guards in global-setup and test helper
- Delete test/setup.ts (process._getActiveHandles unref hack)
- Replace no-op cleanup in test-indexed-db.ts with real adapter close
- Fix pool adapter closeOne() to properly close connections with shared
  Database refcount guard and orphaned connection handling in checkin()
- Update segfault-related comments across the codebase

Also bumps @ladybugdb/wasm-core to ^0.15.2 in gitnexus-web for consistency.

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

* fix: keep dangerouslyIgnoreUnhandledErrors for macOS N-API exit crash

The N-API destructor ordering crash during worker fork exit on macOS is
independent of the QueryResult lifetime fix in 0.15.2. Tests pass, but
the exit triggers a crash. Keep the flag with an updated comment
explaining the actual cause. Can be removed once LadybugDB fixes all
destructor ordering issues upstream.

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

* ci: unify test run for single-pass coverage

- Update `npm test` to run all tests (unit + integration + lbug-db)
  via `vitest run` instead of `vitest run test/unit`
- Add `test:unit` script for running unit tests only
- Remove `ci-integration.yml` — the per-file lbug-db process isolation
  is no longer needed with `dangerouslyIgnoreUnhandledErrors` and
  `fileParallelism: false` handling fork exit issues
- Update `ci-unit-tests.yml` to run all tests with build + coverage
- Simplify `ci.yml` gate (two jobs: quality + tests)
- Simplify `ci-report.yml` (single coverage artifact, no merge step)

* fix: update cli-commands test for renamed test:all → test:unit script

* fix: set USERPROFILE in setup-skills test for Windows compatibility

os.homedir() checks USERPROFILE on Windows, not HOME.

* fix: add isolate: false to lbug-db project to prevent fork crashes

On macOS, N-API destructors crash fork workers on exit. With
isolate: true (default), vitest recycles the fork between files,
triggering the crash after each file. After several crashes, the
remaining lbug-db files never execute.

isolate: false keeps all 8 lbug-db files in a single fork — the
fork only exits once after all files complete, and that single exit
crash is caught by dangerouslyIgnoreUnhandledErrors.

* fix: add unique sequence.groupOrder to vitest projects

Vitest v4 requires unique groupOrder when projects have different
maxWorkers (lbug-db has fileParallelism: false → maxWorkers: 1).

* fix: await async close() in global-setup and remove isolate: false

global-setup.ts called conn.close() and db.close() without await —
these return Promise<void> in @ladybugdb/core 0.15.2.  The setup
function returned before the DB was fully closed, so vitest forks
hit a stale file lock when opening the same DB path, crashing the
lbug-db worker before any test ran.

isolate: false caused native state corruption after 2-3 open/close
cycles in the same fork (vitest-specific, not reproducible in plain
Node.js).  Without it, each file gets its own module scope and the
N-API destructor crash at fork exit is caught by
dangerouslyIgnoreUnhandledErrors.

Also fixes fire-and-forget close() calls in the pool adapter —
try/catch around an async close() never catches rejections; changed
to .catch(() => {}) for proper unhandled-rejection prevention.

Before: 0/8 lbug-db files ran on macOS CI (fork crash).
After:  8/8 pass, 84 files, 3077 tests, zero errors.

* fix: update project index references in AGENTS.md and CLAUDE.md to reflect correct symbol counts and relationships

* feat: enhance lbug adapter with external database support and write operation validation

* feat: create ci-tests workflow for comprehensive test coverage across platforms

* ci: move PR report inline to ci.yml, delete ci-report.yml

The old ci-report.yml used workflow_run which always runs code from
the default branch (main). This meant the PR comment used main's
stale report template that still referenced the old unit/integration
split architecture — causing "Merge coverage reports" failures.

Moving the report inline to ci.yml means it runs from the PR branch
and uses the current report template. The report now shows:
- per-platform status (Ubuntu/Windows/macOS columns)
- unified test counts from the single vitest run
- coverage with base branch (main) delta comparison
- commit SHA for traceability

Also removes the save-pr-meta job since the report no longer needs
a separate workflow_run trigger.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
2026-03-19 08:25:43 +00:00
abhigyanpatwari
8a100a76d3 test: add test suite with vitest (unit + integration + fixtures)
- 59 test files covering unit and integration tests
- vitest config with coverage thresholds and fork pooling
- Test fixtures (mini-repo + multi-language sample code)
- Add vitest + coverage-v8 to devDependencies
- Add test scripts (test, test:integration, test:all, test:watch, test:coverage)
- Move typescript to devDependencies where it belongs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-01 20:07:02 +05:30