Skip to content

fix(build): inject tree-sitter worker path at compile time - #350

Merged
lavaman131 merged 1 commit into
mainfrom
lavaman131/hotfix/markdown-rendering
Mar 4, 2026
Merged

fix(build): inject tree-sitter worker path at compile time#350
lavaman131 merged 1 commit into
mainfrom
lavaman131/hotfix/markdown-rendering

Conversation

@lavaman131

@lavaman131 lavaman131 commented Mar 4, 2026

Copy link
Copy Markdown
Collaborator

Problem

When compiling Atomic with bun build --compile, Tree-sitter syntax highlighting failed in the binary because the file-embedding approach (import ... with { type: "file" }) resolved paths relative to import.meta.url, which points to the compiled binary rather than the original package location. This caused the parser worker to fail initialization.

Solution

Replace the file-embedding approach with compile-time path injection using Bun's define API, following the pattern used by OpenCode SDK. The worker path is now injected as a compile-time constant pointing to $bunfs/root/ (Bun's virtual filesystem in compiled binaries), with a runtime fallback for local development.

Changes

  • Add src/scripts/build-binary.ts: New build wrapper that uses Bun's define API to inject OTUI_TREE_SITTER_WORKER_PATH pointing to the worker's location in $bunfs/root/
  • Update build configuration: Modified package.json build script and all CI workflow build steps to use build-binary.ts
  • Refactor initTreeSitterAssets(): Now prefers compile-time constant with runtime fallback for local development
  • Add web-tree-sitter dependency: Required for Tree-sitter functionality
  • Add comprehensive tests:
    • Unit tests for worker path configuration logic
    • Binary integration test that verifies Tree-sitter highlighting works in compiled binaries

Testing

The binary integration test (tree-sitter-assets.binary.test.ts) compiles a test binary with the new build approach and verifies that markdown highlighting initializes successfully, ensuring the fix works end-to-end.

Replace file-embedding approach for the Tree-sitter parser worker with
compile-time path injection via Bun define API. This fixes worker
resolution failures in compiled binaries where import.meta.url points
to the binary rather than the original package location.

- Add src/scripts/build-binary.ts that wraps bun build --compile with
  OTUI_TREE_SITTER_WORKER_PATH define pointing to $bunfs root
- Update package.json build script and CI workflow to use build-binary.ts
- Refactor initTreeSitterAssets() to prefer compile-time constant with
  runtime fallback for local dev
- Add web-tree-sitter dependency
- Add unit and binary integration tests for worker path resolution

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lavaman131
lavaman131 merged commit 2298286 into main Mar 4, 2026
6 checks passed
@lavaman131
lavaman131 deleted the lavaman131/hotfix/markdown-rendering branch March 4, 2026 05:58
@claude

claude Bot commented Mar 4, 2026

Copy link
Copy Markdown

PR Review: fix(build): inject tree-sitter worker path at compile time

Summary

This PR addresses worker resolution failures in compiled binaries by replacing the file-embedding approach with compile-time path injection via Bun's define API. The approach is sound and follows the OpenCode pattern mentioned in the documentation.


✅ Strengths

Code Quality

  • Clean separation of concerns with a dedicated build-binary.ts script
  • Good use of TypeScript types (BuildOptions) and proper error handling
  • Well-documented module header in tree-sitter-assets.ts
  • Appropriate use of parseArgs from node:util for CLI argument parsing

Test Coverage

  • Both unit tests and binary integration tests are provided
  • Tests properly cover the runtime fallback behavior
  • Integration test correctly cleans up temp files with afterAll

Architecture

  • The compile-time constant injection is more efficient than runtime resolution
  • Graceful fallback for local development environments

⚠️ Issues to Address

1. Compile-time path not propagated (Potential Bug)

In src/ui/tree-sitter-assets.ts:65-70:

export function initTreeSitterAssets(): void {
  if (!process.env.OTUI_TREE_SITTER_WORKER_PATH && !getCompileTimeTreeSitterWorkerPath()) {
    process.env.OTUI_TREE_SITTER_WORKER_PATH = getRuntimeTreeSitterWorkerPath();
  }
  // ...
}

The logic checks if a compile-time path exists but never assigns it to the environment variable when it does exist. If OTUI_TREE_SITTER_WORKER_PATH is defined at compile-time but the env var isn't set, the TreeSitterClient may not receive the path. Consider:

if (!process.env.OTUI_TREE_SITTER_WORKER_PATH) {
  const compileTimePath = getCompileTimeTreeSitterWorkerPath();
  process.env.OTUI_TREE_SITTER_WORKER_PATH = compileTimePath ?? getRuntimeTreeSitterWorkerPath();
}

2. Type assertion smell in build-binary.ts:84

...(options.target ? { target: options.target as never } : {}),

Using as never is a code smell. Consider using proper typing or as Bun.Target (or the appropriate Bun type for compile targets).

3. Extra blank line

src/ui/tree-sitter-assets.ts:17-18 has an extra blank line after the declare statement.


💡 Suggestions

1. Additional test coverage
Consider adding tests for:

  • Error case when --outfile is missing
  • Invalid --target values (e.g., invalid-target)
  • Edge cases in inferTargetOs (e.g., just "win" without "windows")

2. Dependency clarity
The web-tree-sitter dependency is added but it's not clear from the diff if it's actively used or if it's a peer dependency requirement. A comment in the PR description or code would help clarify.

3. Error handling in binary test
In tree-sitter-assets.binary.test.ts:80-86, the JSON parsing could fail silently. Consider adding explicit assertions:

expect(outputLine).toBeDefined();
expect(() => JSON.parse(outputLine ?? "{}")).not.toThrow();

🔒 Security

No security concerns identified. The build script operates on local files and well-defined paths.

⚡ Performance

The compile-time constant injection approach is efficient and avoids runtime path resolution overhead in production builds.


Verdict

This is a solid fix that addresses a real issue with worker resolution in compiled binaries. The architecture follows established patterns and includes appropriate test coverage. Please address issue #1 (compile-time path propagation) as it appears to be a functional bug, and consider the other suggestions before merging.

lavaman131 added a commit that referenced this pull request Mar 26, 2026
Replace file-embedding approach for the Tree-sitter parser worker with
compile-time path injection via Bun define API. This fixes worker
resolution failures in compiled binaries where import.meta.url points
to the binary rather than the original package location.

- Add src/scripts/build-binary.ts that wraps bun build --compile with
  OTUI_TREE_SITTER_WORKER_PATH define pointing to $bunfs root
- Update package.json build script and CI workflow to use build-binary.ts
- Refactor initTreeSitterAssets() to prefer compile-time constant with
  runtime fallback for local dev
- Add web-tree-sitter dependency
- Add unit and binary integration tests for worker path resolution

Co-authored-by: lavaman131 <dev@example.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lavaman131 added a commit that referenced this pull request Mar 27, 2026
Replace file-embedding approach for the Tree-sitter parser worker with
compile-time path injection via Bun define API. This fixes worker
resolution failures in compiled binaries where import.meta.url points
to the binary rather than the original package location.

- Add src/scripts/build-binary.ts that wraps bun build --compile with
  OTUI_TREE_SITTER_WORKER_PATH define pointing to $bunfs root
- Update package.json build script and CI workflow to use build-binary.ts
- Refactor initTreeSitterAssets() to prefer compile-time constant with
  runtime fallback for local dev
- Add web-tree-sitter dependency
- Add unit and binary integration tests for worker path resolution

Co-authored-by: lavaman131 <dev@example.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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