From f1151660b9e0c73a80d963dcc97d333176ba6a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Tue, 9 Jun 2026 09:04:01 +0100 Subject: [PATCH] fix(install): graceful Kotlin optional-grammar install + accurate toolchain docs (#2110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(install): document Kotlin optional-grammar toolchain behavior + graceful install probe tree-sitter-kotlin is a third-party npm optionalDependency that ships source-only (no upstream prebuilds) and compiles its native binding via node-gyp at install. It was the only optional grammar without a GitNexus install-time probe, and the README's GITNEXUS_SKIP_OPTIONAL_GRAMMARS "no toolchain needed" note omitted Kotlin entirely. This adds a fail-soft probe (mirroring the Swift one) that warns clearly and always exits 0 so install never breaks, wires it into postinstall, and corrects the optional-grammar docs in README.md and .devcontainer/README.md. Shipping prebuilt .node binaries (the literal request) needs an upstream/CI build matrix and is intentionally left as follow-up. Refs #2107 Co-Authored-By: Claude Opus 4.8 (1M context) * fix: address PR #2110 tri-review findings (Kotlin optional-grammar install) Addresses the four P2 findings from the PR #2110 tri-review: - F1: docs no longer imply GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 skips Kotlin's toolchain. npm compiles tree-sitter-kotlin via its own node-gyp-build step regardless of that variable; point to `npm install --omit=optional` as the real lever (README.md + .devcontainer/README.md). - F2: the install probe now surfaces its "Kotlin unavailable" guidance on the dir-absent branch — the dominant toolchain-less case, where npm prunes the failed optional dependency so the package dir is gone at postinstall. Gated on npm_config_omit so a deliberate `--omit=optional` stays silent. Still never throws or exits non-zero. - F3: add a behavioral test that executes the probe across its skip / dir-absent-warn / dir-absent-omit-silent paths and asserts exit code 0 (guards the postinstall "never exit non-zero" invariant a static assertion cannot). - F4: reframe prebuilt Kotlin as deferred Swift-parity follow-up — GitNexus already vendors its own self-built Swift prebuilds and could do the same for Kotlin — tracked in #2107, not an upstream-only blocker. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .devcontainer/README.md | 2 +- README.md | 6 +- gitnexus/package.json | 2 +- gitnexus/scripts/build-tree-sitter-kotlin.cjs | 74 +++++++++++ .../build-tree-sitter-kotlin-probe.test.ts | 115 ++++++++++++++++++ gitnexus/test/unit/cli-commands.test.ts | 10 ++ 6 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 gitnexus/scripts/build-tree-sitter-kotlin.cjs create mode 100644 gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts diff --git a/.devcontainer/README.md b/.devcontainer/README.md index 8817c1773..80642a121 100644 --- a/.devcontainer/README.md +++ b/.devcontainer/README.md @@ -310,7 +310,7 @@ VS Code's Ports panel shows forwarded ports once their listener starts. - **LadybugDB integration tests may fail in containers** (file-locking, `AGENTS.md` § Testing). Default to `npm run test:unit` inside the container; run integration tests on the host. Tracking issue: documented as a known limitation. - **Single-writer LadybugDB constraint** (`GUARDRAILS.md` § LadybugDB lock). Don't run `gitnexus analyze` on the host and inside the container against the same `.gitnexus/` directory simultaneously — the second writer will get `database busy`. -- **Native grammar builds add ~30s to first install.** Tree-sitter Dart/Proto/Swift grammars build during `gitnexus`'s `postinstall`. To skip them (loses parsing for those three languages), set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` in your shell or add it to `remoteEnv` and rebuild. +- **Native grammar builds add ~30s to first install.** Tree-sitter Dart/Proto/Swift grammars build during `gitnexus`'s `postinstall`; the `tree-sitter-kotlin` optional dependency (third-party npm package, source-only — no upstream prebuilds) compiles its native binding earlier, during npm's own dependency install, and is then probed by `build-tree-sitter-kotlin.cjs`. Set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` (in your shell or `remoteEnv`, then rebuild) to skip the Dart/Proto/Swift builds and silence the Kotlin probe — but npm still compiles `tree-sitter-kotlin` unless you also pass `--omit=optional`. Each loses parsing for the affected language(s); the install still succeeds. - **`tree-sitter-kotlin` warnings on install** are expected (per `AGENTS.md`). Ignore them. - **`.mcp.json` works inside the container**: `npx -y gitnexus@latest mcp` resolves cleanly because npm registry is reachable and the workspace bind mount exposes the same `.mcp.json` the host sees. - **Husky pre-commit fires inside the container** without extra setup. The root `npm install` (run automatically in `postCreateCommand`) installs the hook via `package.json` `prepare`. diff --git a/README.md b/README.md index 2df823d9a..44876243c 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,9 @@ That's it. This indexes the codebase, installs agent skills, registers Claude Co To configure MCP for your editor, run `npx gitnexus setup` once — or set it up manually below. -> **Faster install (no C++ toolchain needed):** set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` before `npm install -g gitnexus` to skip vendored grammar materialize/build (`tree-sitter-dart`, `tree-sitter-proto`, `tree-sitter-swift`). Dart/Proto/Swift files won't be parsed, but install completes in seconds without `python3`/`make`/`g++`. Strict `=1` only — any other value falls through to the rebuild. +> **Faster install (no C++ toolchain needed):** set `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1` before `npm install -g gitnexus` to skip the vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` — those three won't be parsed, but install completes in seconds without `python3`/`make`/`g++`. Strict `=1` only — any other value falls through to the rebuild. This variable does **not** control `tree-sitter-kotlin` (a third-party npm `optionalDependency` that npm compiles via its own `node-gyp-build` step regardless); to skip the Kotlin compile too, add `npm install --omit=optional` — which also drops the `node-gyp-build`/`node-addon-api` build deps and so disables the vendored builds as well. See the `tree-sitter-kotlin` note below. +> +> **About `tree-sitter-kotlin`:** unlike the vendored grammars, Kotlin support comes from a third-party npm `optionalDependency` that ships **source only** (no upstream prebuilt binaries) and compiles via node-gyp at install time. On a host without a C/C++ toolchain its native build soft-fails: npm skips the optional dependency, the `gitnexus` install still **succeeds**, and only Kotlin (`.kt`/`.kts`) parsing is unavailable. An install-time probe surfaces a single clear warning when the binding is missing (suppressed only if you opted out with `--omit=optional`), instead of leaving raw node-gyp output as the only signal, and it honors `GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1`. (GitNexus does **not** yet ship prebuilt Kotlin binaries. GitNexus already vendors its own self-built Swift prebuilds and could do the same for Kotlin — that's deferred Swift-parity follow-up work tracked in [#2107](https://github.com/abhigyanpatwari/GitNexus/issues/2107), not an upstream blocker.) ### MCP Setup @@ -328,7 +330,7 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD`| `max(3, poolSize)` | Per-slot consecutive deaths before the pool's circuit breaker trips. After tripping, every subsequent dispatch rejects until a fresh pool is created. | Hosts where a SIGSEGV-prone native grammar should trip the breaker sooner; CI runners that should fail loudly. | | `GITNEXUS_CHUNK_BYTE_BUDGET` | `2097152` (2 MB) | Chunk boundary used for cache-key composition and dispatch. Smaller = finer-grained cache hits but more dispatch overhead. | Tuning incremental-analyze cache behavior on monorepos. | | `GITNEXUS_NO_GITIGNORE` | unset | When set, skips `.gitignore` parsing. `.gitnexusignore` is still honored. | Indexing a repo whose `.gitignore` excludes files you actually want indexed (e.g., generated code committed for cross-repo lookup). | -| `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` | unset | When `=1` strictly, skips vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` at install time. | Installing on a host without a C++ toolchain or where Swift prebuilds don't match; you're willing to skip Dart/Proto/Swift parsing. | +| `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` | unset | When `=1` strictly, skips the vendored grammar materialize/build for `tree-sitter-dart`, `tree-sitter-proto`, and `tree-sitter-swift` at install time, and silences GitNexus's `tree-sitter-kotlin` probe. It does **not** stop npm from compiling `tree-sitter-kotlin` (a third-party `optionalDependency` with its own `node-gyp-build` step) — use `npm install --omit=optional` to skip that compile too. Without a toolchain the Kotlin build soft-fails, npm skips it, the install still succeeds, and only Kotlin parsing is lost. | Installing on a host without a C++ toolchain or where Swift prebuilds don't match; willing to skip Dart/Proto/Swift parsing (and, with `--omit=optional`, Kotlin). | #### Publishing to understand-quickly (opt-in) diff --git a/gitnexus/package.json b/gitnexus/package.json index cd065cef0..293b651ed 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -49,7 +49,7 @@ "test:watch": "vitest", "test:coverage": "vitest run --coverage", "test:cross-platform": "tsx scripts/run-cross-platform.ts", - "postinstall": "node scripts/materialize-vendor-grammars.cjs && node scripts/build-tree-sitter-dart.cjs && node scripts/build-tree-sitter-proto.cjs && node scripts/build-tree-sitter-swift.cjs", + "postinstall": "node scripts/materialize-vendor-grammars.cjs && node scripts/build-tree-sitter-dart.cjs && node scripts/build-tree-sitter-proto.cjs && node scripts/build-tree-sitter-swift.cjs && node scripts/build-tree-sitter-kotlin.cjs", "prepare": "node scripts/build.js", "prepack": "node scripts/build.js" }, diff --git a/gitnexus/scripts/build-tree-sitter-kotlin.cjs b/gitnexus/scripts/build-tree-sitter-kotlin.cjs new file mode 100644 index 000000000..e21a93766 --- /dev/null +++ b/gitnexus/scripts/build-tree-sitter-kotlin.cjs @@ -0,0 +1,74 @@ +#!/usr/bin/env node +/** + * Probe tree-sitter-kotlin native-binding availability at install time. + * + * Unlike Dart/Proto/Swift (vendored under vendor/ and materialized into + * node_modules/ at postinstall), tree-sitter-kotlin is a third-party npm + * `optionalDependency`. It ships SOURCE ONLY — no upstream `prebuilds/` dir — + * and its own `install` script runs `node-gyp-build`, which compiles the + * native binding from source via node-gyp. On a host without a C/C++ toolchain + * that build soft-fails: npm skips the optional dependency and the `gitnexus` + * install still succeeds. This probe surfaces a single, friendly install-time + * warning when the Kotlin binding is unavailable — whether npm pruned the + * optional dependency after a toolchain-less build failure (its dir is gone, + * which is the common case) or the dir survives but the binding won't load — + * instead of leaving a raw node-gyp error or a first-use runtime failure as the + * only signal. A deliberate opt-out (`--omit=optional`) stays silent. The probe + * does not copy, register, or mutate anything; the runtime require() path in + * parser-loader does the actual load. This probe MUST NEVER throw or exit + * non-zero — it must never break `gitnexus` install. + */ +const fs = require('fs'); +const path = require('path'); + +if (process.env.GITNEXUS_SKIP_OPTIONAL_GRAMMARS === '1') { + console.warn( + '[tree-sitter-kotlin] Skipping native-binding probe (GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1).', + ); + process.exit(0); +} + +const kotlinDir = path.join(__dirname, '..', 'node_modules', 'tree-sitter-kotlin'); + +// `--omit=optional` / `--no-optional` / `.npmrc omit=optional` surface to +// lifecycle scripts as `npm_config_omit` containing `optional` (a comma- or +// space-separated list, e.g. `dev,optional`). That is a deliberate opt-out, so +// an absent package for that reason should stay silent. Any OTHER absence means +// npm attempted the optional dependency's native build and pruned the package +// after it soft-failed (the toolchain-less case) — exactly when the guidance +// below is worth surfacing. +const omitsOptional = /(^|[,\s])optional([,\s]|$)/.test(process.env.npm_config_omit || ''); + +function warnKotlinUnavailable(err) { + if (err) { + console.warn('[tree-sitter-kotlin] Native-binding probe failed:', err.message); + } + console.warn( + '[tree-sitter-kotlin] Kotlin (.kt/.kts) parsing will be unavailable. Non-Kotlin functionality is unaffected.', + ); + console.warn( + '[tree-sitter-kotlin] This is expected on hosts without a C/C++ toolchain: tree-sitter-kotlin ships source only (no upstream prebuilt binaries) and compiles via node-gyp at install. Set GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1 to skip this probe.', + ); +} + +try { + if (!fs.existsSync(path.join(kotlinDir, 'bindings', 'node', 'index.js'))) { + // The package never materialized. If the user deliberately omitted optional + // dependencies, stay silent — they opted out. Otherwise npm pruned the + // package after its native build soft-failed (no toolchain), and this is the + // dominant real-world failure case: surface the guidance the raw node-gyp + // error would otherwise be the only signal of. + if (!omitsOptional) { + warnKotlinUnavailable(); + } + process.exit(0); + } + + const nodeGypBuild = require('node-gyp-build'); + nodeGypBuild(kotlinDir); +} catch (err) { + // The package is present but its native binding can't be loaded (e.g. the dir + // survived with --ignore-scripts, or a partial/ABI-mismatched build). + warnKotlinUnavailable(err); + process.exit(0); +} diff --git a/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts b/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts new file mode 100644 index 000000000..45e997f11 --- /dev/null +++ b/gitnexus/test/unit/build-tree-sitter-kotlin-probe.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Behavioral coverage for the postinstall probe `scripts/build-tree-sitter-kotlin.cjs`. + * + * The probe's hard invariant is that it MUST NEVER exit non-zero — it runs in + * `gitnexus`'s postinstall, so a non-zero exit would break `npm install gitnexus` + * for every user. The static package.json assertion in `cli-commands.test.ts` + * only checks wiring (the script is referenced in `postinstall`); it never runs + * the probe, so a regression that turned an `exit(0)` into `exit(1)`/`throw` + * would ship undetected. This suite executes the real script bytes across its + * branches and asserts exit code 0 every time. + * + * To exercise the "package absent" branches without mutating the repo's real + * node_modules, the probe is copied into an isolated temp `scripts/` dir; its + * `__dirname`-relative `../node_modules/tree-sitter-kotlin` then resolves to a + * non-existent path — the exact state npm leaves behind after it prunes the + * failed optional dependency on a toolchain-less host (see #2107 / PR #2110). + */ + +const probeSource = readFileSync( + fileURLToPath(new URL('../../scripts/build-tree-sitter-kotlin.cjs', import.meta.url)), + 'utf8', +); + +const UNAVAILABLE = 'Kotlin (.kt/.kts) parsing will be unavailable'; + +let tmpRoot: string; +let scriptPath: string; + +beforeAll(() => { + tmpRoot = mkdtempSync(path.join(tmpdir(), 'gn-kotlin-probe-')); + const scriptsDir = path.join(tmpRoot, 'scripts'); + mkdirSync(scriptsDir, { recursive: true }); + scriptPath = path.join(scriptsDir, 'build-tree-sitter-kotlin.cjs'); + writeFileSync(scriptPath, probeSource); +}); + +afterAll(() => { + rmSync(tmpRoot, { recursive: true, force: true }); +}); + +function runProbe(overrides: Record) { + const env: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined) env[k] = v; + } + // Normalize the two variables under test so the case is deterministic even + // when the test runner itself was launched under npm with these set. + delete env.GITNEXUS_SKIP_OPTIONAL_GRAMMARS; + delete env.npm_config_omit; + for (const [k, v] of Object.entries(overrides)) { + if (v === undefined) delete env[k]; + else env[k] = v; + } + return spawnSync(process.execPath, [scriptPath], { env, encoding: 'utf8', timeout: 10_000 }); +} + +describe('build-tree-sitter-kotlin.cjs install probe', () => { + it('exits 0 and reports skipping when GITNEXUS_SKIP_OPTIONAL_GRAMMARS=1', () => { + const r = runProbe({ GITNEXUS_SKIP_OPTIONAL_GRAMMARS: '1' }); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + expect(r.stderr).toContain('Skipping native-binding probe'); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('warns (and exits 0) when the package is absent and optionals were not omitted', () => { + // Regression guard for #2107 / PR #2110: npm prunes the failed optional + // dependency on a toolchain-less host, so the probe must surface its guidance + // on the dir-absent branch rather than silently exiting. + const r = runProbe({}); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + expect(r.stderr).toContain(UNAVAILABLE); + }); + + it('stays silent (and exits 0) when optionals were deliberately omitted (omit=optional)', () => { + const r = runProbe({ npm_config_omit: 'optional' }); + expect(r.status).toBe(0); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('treats a comma-joined list (dev,optional) as an opt-out and stays silent', () => { + const r = runProbe({ npm_config_omit: 'dev,optional' }); + expect(r.status).toBe(0); + expect(r.stderr).not.toContain(UNAVAILABLE); + }); + + it('still warns when only non-optional groups are omitted (omit=dev)', () => { + const r = runProbe({ npm_config_omit: 'dev' }); + expect(r.status).toBe(0); + expect(r.stderr).toContain(UNAVAILABLE); + }); + + it('never exits non-zero across env permutations (postinstall hard invariant)', () => { + const permutations: Record[] = [ + { GITNEXUS_SKIP_OPTIONAL_GRAMMARS: '1' }, + {}, + { npm_config_omit: 'optional' }, + { npm_config_omit: 'dev,optional' }, + { npm_config_omit: 'dev' }, + ]; + for (const overrides of permutations) { + const r = runProbe(overrides); + expect(r.status).toBe(0); + expect(r.signal).toBeNull(); + } + }); +}); diff --git a/gitnexus/test/unit/cli-commands.test.ts b/gitnexus/test/unit/cli-commands.test.ts index fcae5d0bd..0d6b56995 100644 --- a/gitnexus/test/unit/cli-commands.test.ts +++ b/gitnexus/test/unit/cli-commands.test.ts @@ -99,6 +99,16 @@ describe('CLI commands', () => { expect(swiftPkg.default.dependencies).toBeUndefined(); expect(swiftPkg.default.peerDependencies['tree-sitter']).toContain('^0.21.1'); }); + + it('declares tree-sitter-kotlin as an optionalDependency probed at postinstall (#2107)', async () => { + const pkg = await import('../../package.json', { with: { type: 'json' } }); + const optional = pkg.default.optionalDependencies ?? {}; + // Kotlin is a third-party npm optionalDependency (not vendored), so npm + // skips it when its source-only native build soft-fails — the gitnexus + // install still succeeds. + expect(optional['tree-sitter-kotlin']).toBeDefined(); + expect(pkg.default.scripts.postinstall).toContain('build-tree-sitter-kotlin.cjs'); + }); }); describe('analyzeCommand', () => {