mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-05 08:06:02 +00:00
* 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>
179 lines
6.1 KiB
YAML
179 lines
6.1 KiB
YAML
name: Tests
|
|
|
|
on:
|
|
workflow_call:
|
|
|
|
permissions:
|
|
contents: read
|
|
|
|
jobs:
|
|
tests:
|
|
name: ubuntu / coverage
|
|
runs-on: ubuntu-latest
|
|
timeout-minutes: 25
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
- uses: ./.github/actions/setup-gitnexus
|
|
with:
|
|
build: 'true'
|
|
|
|
- name: Run all tests with coverage
|
|
run: >-
|
|
npx vitest run
|
|
--reporter=default
|
|
--reporter=json
|
|
--outputFile=test-results.json
|
|
--coverage
|
|
--coverage.reporter=json-summary
|
|
--coverage.reporter=json
|
|
--coverage.reporter=text
|
|
--coverage.thresholdAutoUpdate=false
|
|
--coverage.reportOnFailure=true
|
|
working-directory: gitnexus
|
|
|
|
# gitnexus-shared already built by setup-gitnexus action above
|
|
- name: Install gitnexus-web dependencies
|
|
run: npm ci
|
|
working-directory: gitnexus-web
|
|
|
|
- name: Run gitnexus-web unit tests
|
|
run: >-
|
|
npx vitest run
|
|
--reporter=default
|
|
--reporter=json
|
|
--outputFile=web-test-results.json
|
|
working-directory: gitnexus-web
|
|
|
|
- name: Run docker-server integration tests
|
|
run: node --test docker-server.test.mjs
|
|
|
|
- name: Upload test reports
|
|
if: always()
|
|
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
|
with:
|
|
name: test-reports
|
|
path: |
|
|
gitnexus/coverage/coverage-summary.json
|
|
gitnexus/coverage/coverage-final.json
|
|
gitnexus/test-results.json
|
|
gitnexus-web/web-test-results.json
|
|
retention-days: 5
|
|
|
|
cross-platform:
|
|
name: ${{ matrix.os }}
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
# Ubuntu already covered by the coverage job above
|
|
os: [windows-latest, macos-latest]
|
|
runs-on: ${{ matrix.os }}
|
|
timeout-minutes: 25
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
- uses: ./.github/actions/setup-gitnexus
|
|
with:
|
|
build: 'true'
|
|
- run: npx vitest run
|
|
working-directory: gitnexus
|
|
|
|
# End-to-end smoke test for the #1728 packaging fix: pack the published
|
|
# tarball, install it globally into a temp prefix, and assert no junction
|
|
# creation (the EPERM root cause) plus working CLI plus vendor cleanliness
|
|
# (#836). Runs on windows-latest because that is the platform the fix
|
|
# targets; the in-repo `npm ci` job above only exercises the dev-tree path
|
|
# and skips the tarball reify step where the historical EPERM occurred.
|
|
packaged-install-smoke:
|
|
name: packaged install smoke (${{ matrix.os }})
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
os: [windows-latest, ubuntu-latest]
|
|
runs-on: ${{ matrix.os }}
|
|
timeout-minutes: 15
|
|
steps:
|
|
# persist-credentials: false — this job runs npm pack + npm install -g
|
|
# from a tarball and never pushes back; the token in .git/config would
|
|
# be at risk of leaking through any future artifact-upload step
|
|
# (zizmor artipacked audit). Disable upfront.
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
with:
|
|
persist-credentials: false
|
|
- uses: ./.github/actions/setup-gitnexus
|
|
with:
|
|
build: 'true'
|
|
|
|
- name: Pack gitnexus tarball
|
|
shell: bash
|
|
run: npm pack
|
|
working-directory: gitnexus
|
|
|
|
- name: Install gitnexus tarball into isolated prefix
|
|
shell: bash
|
|
run: |
|
|
set -euo pipefail
|
|
PREFIX="$RUNNER_TEMP/gitnexus-smoke"
|
|
mkdir -p "$PREFIX"
|
|
TARBALL=$(find . -maxdepth 1 -name 'gitnexus-*.tgz' -print -quit)
|
|
if [ -z "$TARBALL" ]; then
|
|
echo "ERROR: no gitnexus-*.tgz tarball found in $(pwd)" >&2
|
|
exit 1
|
|
fi
|
|
echo "Installing $TARBALL into $PREFIX"
|
|
npm install -g --prefix "$PREFIX" "./$TARBALL" --no-audit --no-fund
|
|
echo "PREFIX=$PREFIX" >> "$GITHUB_ENV"
|
|
working-directory: gitnexus
|
|
|
|
- name: Assert no junctions or vendor build artifacts
|
|
shell: bash
|
|
run: |
|
|
set -euo pipefail
|
|
# Locate the installed gitnexus package across npm prefix layouts
|
|
# (lib/node_modules on POSIX, node_modules on Windows).
|
|
for candidate in "$PREFIX/lib/node_modules/gitnexus" "$PREFIX/node_modules/gitnexus"; do
|
|
if [ -d "$candidate" ]; then
|
|
INSTALLED="$candidate"
|
|
break
|
|
fi
|
|
done
|
|
if [ -z "${INSTALLED:-}" ]; then
|
|
echo "ERROR: installed gitnexus package not found under $PREFIX" >&2
|
|
ls -la "$PREFIX" || true
|
|
exit 1
|
|
fi
|
|
echo "Installed package at: $INSTALLED"
|
|
|
|
# #836 invariant: no node_modules/ or build/ under any vendor/*.
|
|
BAD=$(find "$INSTALLED/vendor" \( -name node_modules -o -name build \) -print 2>/dev/null || true)
|
|
if [ -n "$BAD" ]; then
|
|
echo "ERROR: vendor tree contains forbidden build artifacts (#836):" >&2
|
|
echo "$BAD" >&2
|
|
exit 1
|
|
fi
|
|
|
|
# #1728 invariant: materialized grammar dirs are real directories,
|
|
# not junctions/symlinks (which is what the EPERM regression created).
|
|
for name in tree-sitter-dart tree-sitter-proto tree-sitter-swift; do
|
|
entry="$INSTALLED/node_modules/$name"
|
|
if [ ! -e "$entry" ]; then
|
|
echo "WARN: $name not materialized (toolchain/prebuild may be unavailable on $RUNNER_OS)"
|
|
continue
|
|
fi
|
|
if [ -L "$entry" ]; then
|
|
echo "ERROR: $entry is a symlink/junction — #1728 regression" >&2
|
|
exit 1
|
|
fi
|
|
if [ ! -d "$entry" ]; then
|
|
echo "ERROR: $entry is not a directory" >&2
|
|
exit 1
|
|
fi
|
|
done
|
|
|
|
- name: Assert gitnexus --version works
|
|
shell: bash
|
|
run: |
|
|
set -euo pipefail
|
|
if [ "$RUNNER_OS" = "Windows" ]; then
|
|
"$PREFIX/gitnexus.cmd" --version
|
|
else
|
|
"$PREFIX/bin/gitnexus" --version
|
|
fi
|