fix: add preinstall cleanup to prevent ENOTEMPTY on global upgrade

[Copilot]

Global upgrades (npm install -g gitnexus@<new>) fail with ENOTEMPTY because npm cannot rmdir the vendor/tree-sitter-proto/node_modules/node-addon-api directory left by the previous installation's file: dependency resolution.

Changes

• scripts/preinstall-cleanup.cjs — new preinstall hook that removes vendor/tree-sitter-proto/node_modules and vendor/tree-sitter-proto/build before dependency resolution, clearing stale artifacts from the prior install
• package.json — wire up "preinstall": "node scripts/preinstall-cleanup.cjs"
• .npmignore — exclude vendor/**/node_modules and vendor/**/build from the published tarball as a secondary guard

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• extension.ladybugdb.com
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/GitNexus/GitNexus/gitnexus/node_modules/.bin/vitest run test/unit xus/�� node install.js -I /.bin/as otlin_binding/bi/bin/sh /home/REDACTED/.ca-c -I t/tree_sitter_p /hom�� ease/obj.target/tree_sitter_dart_binding/src/scanner.o.d.raw | grep -v '^$' bash --64 p/bin/python3 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	Apr 15, 2026 0:35am

[github-actions]

Claude finished @magyargergo's task in 2m 23s —— View job

---

PR Review: fix: add preinstall cleanup to prevent ENOTEMPTY on global upgrade

Tasks:

• Load context (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
• Read changed files (preinstall-cleanup.cjs, package.json, .npmignore)
• Review against criteria
• Post final verdict

---

Scope note: Several criteria in the review prompt (ManifestExtractor invocation, config.links cross-links, resolveSymbol contracts, graph schema integrity, LadybugDB single-writer invariant, MCP tool/resource contracts, staleness behavior, embeddings preservation) have zero surface overlap with this diff. This review omits those headings as not applicable and focuses on what the diff actually touches.

---

Correctness & Functional Completeness

Mechanism is correct. The ENOTEMPTY failure occurs because a prior global install leaves vendor/tree-sitter-proto/node_modules/ (populated by the file:./vendor/tree-sitter-proto optional dependency resolution) in the global package prefix. A subsequent global upgrade can't atomically overwrite/remove those directories. Running fs.rmSync before npm resolves deps eliminates the stale artifacts and unblocks the upgrade.

Path resolution (preinstall-cleanup.cjs:21-23): path.join(__dirname, '..', 'vendor', 'tree-sitter-proto', ...) is correct. __dirname in a .cjs file is always the script's own directory (<pkg_root>/scripts/), so .. resolves to <pkg_root>/vendor/tree-sitter-proto/ regardless of the npm working directory. This is consistent with how patch-tree-sitter-swift.cjs:32 resolves node_modules/.

files array includes scripts/ (package.json:37) so preinstall-cleanup.cjs is present in the published tarball. ✅

Best-effort semantics (preinstall-cleanup.cjs:31): warn on error but don't exit non-zero, so a permission failure or unexpected filesystem state does not abort the install. This is the right call for cleanup housekeeping.

.npmignore secondary guard (gitnexus/.npmignore:13-14): Prevents vendor/tree-sitter-proto/node_modules and vendor/tree-sitter-proto/build from being accidentally included in a future publish (e.g. if npm pack runs after a local install). The files field already whitelists vendor/, so .npmignore is the correct mechanism to exclude subpaths. ✅

Code Clarity & Clean Code

Naming and cohesion: The script is clear, self-contained, and well-commented. The block comment references the causal issue (#836) and explains the mechanism. Follows the established pattern from scripts/patch-tree-sitter-swift.cjs.

existsSync + rmSync { force: true } redundancy (preinstall-cleanup.cjs:27-29): The { force: true } flag silences "no such file" errors from rmSync, making the outer existsSync check redundant (triple-safety: existsSync → try/catch → force:true). This is harmless but slightly over-defensive. — nit

No dead code, no barrel re-exports, no // removed comments. ✅

No drive-by refactors outside the stated scope. ✅

Test Coverage & Change Safety

No unit tests added. For a two-path filesystem cleanup script whose logic is: "if directory exists, delete it; else skip; catch and warn," unit coverage is low-value (would just mock fs and assert it was called). TESTING.md's regression-test trigger list covers graph schema, query contracts, MCP shapes, and embedding code paths — none of which are touched here. — non-blocking

TypeScript impact: No .ts files changed; tsc --noEmit is unaffected. ✅

Pre-commit hook parity: The changed files are .cjs and .json; the husky pre-commit runs tsc --noEmit + vitest run --project default on staged gitnexus/ files. Neither check applies to these file types. ✅

Performance

No hot-path code touched. The preinstall script runs once per npm install, performs two existsSync + conditional rmSync calls — O(1) with negligible overhead. No concern.

Operational Risk

Windows/cross-platform: path.join handles path separators correctly. fs.rmSync(dir, { recursive: true, force: true }) is stable on Windows since Node 14.14; the project requires Node ≥ 20 (engines.node). No FD lifecycle issues (pure sync, no streams). ✅

Runs on every npm install, not only global upgrades: The preinstall hook fires on local developer installs too, deleting vendor/tree-sitter-proto/node_modules and build before npm recreates them. This is intentional and safe — the directories are transient build artifacts. Confirmed by vendor/tree-sitter-proto/.gitignore (they're already gitignored). ✅

vendor/**/build scope in .npmignore (gitnexus/.npmignore:14): The double-glob vendor/**/build excludes any build/ directory at any depth under vendor/, not just tree-sitter-proto/build. With the current single vendor package this is fine; worth knowing if more vendor packages are added with intentional build artifacts to ship. — nit

Rollback: Safe without re-analyze. Reverting removes the preinstall hook; the ENOTEMPTY issue returns for global upgrades but no graph, index, or MCP state is affected.

Maintainability

Pipeline Phase DAG (ARCHITECTURE.md): Not applicable — this is packaging infrastructure, not an ingestion pipeline concern.

Future contributor ergonomics: The script location (scripts/preinstall-cleanup.cjs) mirrors the existing scripts/patch-tree-sitter-swift.cjs pattern. The package.json preinstall entry is the natural first place a contributor would look for lifecycle hooks. The issue reference in the comment (#836) provides full causal context. ✅

Non-Negotiables (GUARDRAILS.md)

Check	Result
No secrets, tokens, or machine-specific paths	✅
No blind find-and-replace renames	✅ (only new files added)
impact (upstream) for modified shared symbols	✅ (no shared symbols modified)
Conventional commit style	✅ fix: add preinstall cleanup to prevent ENOTEMPTY on global upgrade
PolyForm Noncommercial license untouched	✅

---

Final Verdict

✅ Ready to merge — no blocking issues.

The change is minimal, targeted, and correct. It directly addresses the ENOTEMPTY regression pattern (consistent with the prior ENOTEMPTY on Windows class of issues noted in the review criteria), uses established patterns from the codebase (patch-tree-sitter-swift.cjs), and is safe to roll back. The two nits (redundant existsSync and broad vendor/**/build glob) are cosmetic and do not affect correctness or safety.