Skip to content

fix: allow Windows Node 26 installs when Kotlin native parser cannot build - #325

Open
nikhiltokas wants to merge 1 commit into
trailhq:mainfrom
nikhiltokas:fix/windows-node26-kotlin-install
Open

fix: allow Windows Node 26 installs when Kotlin native parser cannot build#325
nikhiltokas wants to merge 1 commit into
trailhq:mainfrom
nikhiltokas:fix/windows-node26-kotlin-install

Conversation

@nikhiltokas

Copy link
Copy Markdown

Summary

Fixes Windows installs on Node 26 when tree-sitter-kotlin cannot compile its native addon.

  • Move tree-sitter-kotlin to optionalDependencies
  • Lazy-load the Kotlin parser only when .kt or .kts files are encountered
  • Preserve Kotlin indexing when the parser is available
  • Skip only Kotlin files with one actionable diagnostic when it is unavailable
  • Ensure an unavailable parser does not poison the extraction cache if it becomes available later
  • Add regression coverage for both available and unavailable parser behavior
  • Add Windows + Node 26 CI coverage, including installation of the packed tarball in a clean consumer project
  • Document the conditional Kotlin support behavior

Problem

On Windows 10 with Node v26.8.1, npm 11.6.0, Visual Studio 2022 Build Tools, and Python 3.14.4, installing Graft failed because tree-sitter-kotlin@0.3.8 fell back to native compilation:

cl : warning D9002: ignoring unknown option '-flto=thin'
LINK : warning LNK4044: unrecognized option '/flto=thin'; ignored
LINK : fatal error LNK1117: syntax error in option 'opt:lldltojobs=2'

tree-sitter-kotlin@0.3.8 is the latest published release and its package contains no Windows prebuilt binaries, so there is no compatible dependency upgrade available.

This change does not remove Kotlin support: npm still installs the parser automatically whenever it can. It only allows Graft itself to install and index non-Kotlin repositories when that optional native addon cannot build.

Validation

Tested on Windows with Node v26.8.1:

npm ci
npm run build
npm test
npm pack

Also installed the packed tarball into a clean temporary consumer project and verified:

npx graft --version

The new Windows Node 26 CI job runs:

npm ci
npm run build
npm test
npm pack

and installs the packed artifact in a clean consumer project.

@trailhq-graft

trailhq-graft Bot commented Sep 9, 2026

Copy link
Copy Markdown

🌱 graft blast radius

1 area changed → 6 areas can be affected. 18 dependent symbols, depth 2.
Tests: 1 area updated its tests.
Tag: @anirudhkumar-nanonets — 5 of 7 areas · @shhdwi — 6 of 7 areas · @bhavesh-gupta-investis — Synchronous Execution

flowchart TB
  A0(("Workspace Graph Freshness<br/>6 symbols"))
  A1(("Pull Request Review<br/>4 symbols"))
  A2(("MCP Tool Invocation<br/>3 symbols"))
  A3(("Graph CLI Entry<br/>3 symbols"))
  A4(("Viewer Build Script<br/>1 symbol"))
  AX(("1 smaller area<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3,A4 reached;
  classDef tail fill:#EEF2F3,stroke:#9AA4A9,stroke-width:1px,color:#3A4247;
  class AX tail;
Loading
Can be affected Symbols Nearest hop Reached from
Workspace Graph Freshness 6 src/graph/container.ts:L151-L208 extractContainer — calls, depth 1 Graph Extraction
Pull Request Review 4 src/app/review.ts:L45-L99 reviewPullRequest — calls, depth 1 Graph Extraction
MCP Tool Invocation 3 src/mcp/tools.ts:L216-L244 callTool — calls, depth 2 Graph Extraction
Graph CLI Entry 3 src/engine.ts:L91-L101 graph — calls, depth 1 Graph Extraction
Viewer Build Script 1 scripts/build-viewer.mjs:L1-L45 build-viewer.mjs — calls, depth 2 Graph Extraction
Synchronous Execution 1 src/claude/sync-run.ts:L19-L33 runSync — calls, depth 2 Graph Extraction
Who knows this code — 3 people across 7 areas
Area Who knows it
Graph Extraction · changed @anirudhkumar-nanonets — 22 commits, last 13d ago · @shhdwi — 10 commits, last 27d ago
Workspace Graph Freshness · affected @anirudhkumar-nanonets — 9 commits, last 29d ago · @shhdwi — 6 commits, last 28d ago
Pull Request Review · affected @anirudhkumar-nanonets — 5 commits, last 8d ago
MCP Tool Invocation · affected @shhdwi — 14 commits, last 28d ago · @anirudhkumar-nanonets — 7 commits, last 7d ago
Graph CLI Entry · affected @anirudhkumar-nanonets — 38 commits, last 7d ago · @shhdwi — 24 commits, last 27d ago
Viewer Build Script · affected @shhdwi — 2 commits, last 28d ago
Synchronous Execution · affected @shhdwi — 3 commits, last 2mo ago · @bhavesh-gupta-investis — 1 commit, last 16d ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 18 dependent symbols, grouped by area

Workspace Graph Freshness — 6 symbols in 4 files

  • src/graph/container.ts:L151-L208 — extractContainer (calls, depth 1)
    171: inner = extractFile(rel, script, lang.inner);
  • src/graph/refresh.ts:L150-L227 — ensureFreshGraph (calls, depth 1)
    216: await buildGraph(dir, { contextDir: opts.contextDir, graphOnly: true, onlyDirs });
  • src/graph/workspace.ts:L630-L655 — federateCheck (calls, depth 1)
    638: const g = await checkGraph(join(root, child));
  • src/graph/refresh.ts:L235-L261 — ensureFreshChildren (calls, depth 2)
  • src/graph/workspace-cli.ts:L49-L70 — buildChild (calls, depth 2)
  • src/graph/workspace-cli.ts:L126-L130 — runWorkspaceCheck (calls, depth 2)

Pull Request Review — 4 symbols in 4 files

  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 1)
    55: await buildGraph(checkout.dir);
  • src/app/review-process.ts:L179-L183 — childReviewer (references, depth 2)
  • src/app/review-worker.ts:L67-L87 — run (calls, depth 2)
  • src/app/server.ts:L31-L37 — AppSeams (references, depth 2)

MCP Tool Invocation — 3 symbols in 1 file

  • src/mcp/tools.ts:L216-L244 — callTool (calls, depth 2)
  • src/mcp/tools.ts:L247-L329 — callSingleTool (calls, depth 2)
    272: const g = await engine.checkGraph(root);
  • src/mcp/tools.ts:L153-L199 — callWorkspaceTool (calls, depth 2)

Graph CLI Entry — 3 symbols in 2 files

  • src/engine.ts:L91-L101 — graph (calls, depth 1)
    92: return buildGraph(dir, {
  • src/engine.ts:L82-L84 — checkGraph (calls, depth 1)
    82: checkGraph(dir: string): Promise<GraphCheckResult> {
  • src/cli.ts:L163-L173 — refreshBefore (calls, depth 2)

Viewer Build Script — 1 symbol in 1 file

  • scripts/build-viewer.mjs:L1-L45 — build-viewer.mjs (calls, depth 2)
    3: * assets). Runs as part of `npm run build`; the bundle ships in the package

Synchronous Execution — 1 symbol in 1 file

  • src/claude/sync-run.ts:L19-L33 — runSync (calls, depth 2)
Test signal per changed area — 1 ✓

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Graph Extraction — 5 of 7 reached · 1 test file changed here: test/graph-languages.test.ts
    • not reached: grammarFor, loadKotlinGrammar
35 test suites also reference this code

46 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/container-extract.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-bindings.test.ts
  • test/graph-extract-dedup.test.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • test/graph-r-phase3.test.ts
  • test/graph-r-phase4.test.ts
  • test/graph-r-phase5.test.ts
  • …15 more

⚠️ 4 changed files not in the graph (.github/workflows/ci.yml, README.md, package-lock.json, package.json) — no parser claims the extension, or the index predates the file.

graft blast · origin/main...HEAD · depth 2 · 9 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Sep 9, 2026
Ercaner1988 added a commit to Ercaner1988/Graft that referenced this pull request Sep 13, 2026
… depth language (trailhq#337 trailhq#217 trailhq#214 trailhq#325)

Four PRs were rewriting the same code path in extract.ts — one function now
does what they each asked for:

- Lazy loading (trailhq#217): a grammar (and the core tree-sitter binding itself) is
  required at most once, the first time entryFor()/depthExtensions() is asked
  about that language, not for all nine at module-import time. `graft mcp`
  never asks before the client's `initialize` reply, so a client can no
  longer see that handshake stall behind nine native loads.
- Per-language isolation (trailhq#337, unchanged): a grammar that will not load
  costs its own language, not the CLI.
- WASM fallback for every depth language, not just Kotlin/Java (trailhq#214): the
  breadth tier already had a "FALLBACK row" mechanism (a GENERIC_LANGS row
  reachable only when the matching depth grammar failed); tree-sitter-wasm
  ships a .wasm for all nine depth languages already, so the remaining seven
  now have one too. None has a queries/<name>.scm, so on the rare machine
  that actually reaches one it degrades to the node-kind walker (symbols
  only) instead of leaving the language unindexed.
- optionalDependencies (trailhq#325's core ask): the eight native grammar packages
  moved out of dependencies, so a platform lacking a prebuild for one no
  longer fails `npm install` for the other eight (core tree-sitter stays
  required — a missing core is a bigger question than one language, left for
  a follow-up).

New test: `graft mcp` answers `initialize` with tree-sitter-typescript broken
and never touches grammar loading at all (no warning on stderr) — the
concrete claim trailhq#217 exists for.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant