Repository navigation
fix(lsp): skip an unrunnable typescript-language-server and fall back to bunx - #131
Conversation
… to bunx
A mise shim for typescript-language-server resolved on PATH but exited 1
("No version is set for shim") in the Vue fixture, so the server died and
three Vue integration tests timed out. Unversioned bunx also picked up the
shim. Share the dart --version probe as findRunnableBin and run the bunx
fallback as typescript-language-server@latest so it installs from the
registry.
Closes #127
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the LSP server implementations by replacing the Dart-specific findSystemDart helper with a generic findRunnableBin utility. This utility is then used by both the Dart and TypeScript servers to locate and verify runnable binaries. The review feedback suggests prioritizing project-local installations of typescript-language-server before falling back to global or bunx executions, and making the log messages in findRunnableBin more generic to support non-PATH binary resolutions.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
 [Medium impact] The PR appears safe to merge, with no outstanding findings. SummaryThe PR skips language-server binaries that cannot run and gives TypeScript a pinned
Reviews (3) · Last reviewed commit: "chore(lsp): word runsVersion warnings fo..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Architecture diagram
sequenceDiagram
participant TS as TypescriptServer
participant FRB as findRunnableBin()
participant PATH as PATH Binary
participant BUNX as bunx CLI
participant REG as npm Registry
participant LSP as Language Server Process
Note over TS,LSP: LSP Server Discovery and Launch Flow
TS->>FRB: findRunnableBin('typescript-language-server', root)
FRB->>PATH: Bun.which('typescript-language-server', PATH)
alt Binary found on PATH
PATH-->>FRB: /path/to/typescript-language-server
FRB->>PATH: spawn --version (cwd=root, 10s timeout)
alt --version exit 0
PATH-->>FRB: exit 0
FRB-->>TS: Return binary path
TS->>LSP: spawn(server, ['--stdio'])
LSP-->>TS: LSP over stdio
else --version exit non-zero
PATH-->>FRB: exit 1 (mise shim, no version set)
FRB-->>TS: Return undefined
TS->>BUNX: spawn('bunx', ['typescript-language-server@latest', '--stdio'])
BUNX->>REG: Install typescript-language-server@latest
REG-->>BUNX: Package (e.g. 6.0.2)
BUNX-->>TS: Spawned LSP
TS->>LSP: LSP over stdio
end
else Binary not found
PATH-->>FRB: undefined
FRB-->>TS: Return undefined
TS->>BUNX: spawn('bunx', ['typescript-language-server@latest', '--stdio'])
BUNX->>REG: Install typescript-language-server@latest
REG-->>BUNX: Package
BUNX-->>TS: Spawned LSP
TS->>LSP: LSP over stdio
end
Note over TS,LSP: DartServer shares the same probe helper
TS->>FRB: findRunnableBin('dart', root)
alt Runnable dart found
FRB-->>TS: /path/to/dart
TS->>LSP: spawn(dart, ['language-server', ...])
else Not runnable
FRB-->>TS: undefined
TS->>LSP: Fallback to dart download
end
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
- Prefer a project-local typescript-language-server, probing it with --version first - Probe every PATH match in findRunnableBin instead of only the first - Pin the bunx fallback to the typescript-language-server version the Vue server installs - Launch the classic server through a shell on Windows so .cmd shims run - Test the TypescriptServer.spawn fallback order
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors binary resolution across the LSP servers by introducing a generic findRunnableBin utility in utils.ts that probes binaries on the PATH using version checks. It replaces the custom findSystemDart logic, improves the TypeScript server fallback mechanism to prefer project-local installations, and pins the fallback typescript-language-server version. Feedback on the changes highlights a critical issue in the new runsVersion utility, where spawning the version probe on Windows can fail or crash the application without enabling the shell option for .cmd/.bat shims and wrapping the Bun.spawn call in a try/catch block.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors binary resolution across LSP servers by introducing a generic findRunnableBin utility that verifies binaries on the PATH using a --version probe. It also updates the TypeScript server to prioritize project-local installations before falling back to the system path or a pinned version via bunx. Feedback highlights a high-severity compatibility issue on Windows within the new runsVersion helper, where spawning Node-based batch file shims directly without a shell will fail, and recommends wrapping the spawn call in a try/catch block to handle synchronous OS-level errors.
| const proc = Bun.spawn([bin, '--version'], { | ||
| cwd: root, | ||
| stdout: 'ignore', | ||
| stderr: 'ignore', | ||
| timeout: VERSION_PROBE_TIMEOUT_MS, | ||
| }) |
There was a problem hiding this comment.
[HIGH] Windows compatibility and error handling issues in runsVersion
Problem: On Windows, typescript-language-server and other Node-based tools are installed as .cmd or .bat batch file shims. Spawning these files directly via Bun.spawn without a shell will fail, causing runsVersion to always return false on Windows. Additionally, spawning process calls (such as Bun.spawn) when probing system binaries on the PATH can throw synchronous OS-level errors (e.g., ENOEXEC due to invalid ELF headers, or permission issues) which must be caught to prevent application crashes.
Rationale: Correctness, cross-platform compatibility, and robustness.
Suggestion: Pass shell: process.platform === 'win32' to Bun.spawn to ensure batch file shims can be executed on Windows, and wrap the Bun.spawn call in a try/catch block to handle synchronous errors gracefully.
| const proc = Bun.spawn([bin, '--version'], { | |
| cwd: root, | |
| stdout: 'ignore', | |
| stderr: 'ignore', | |
| timeout: VERSION_PROBE_TIMEOUT_MS, | |
| }) | |
| let proc; | |
| try { | |
| proc = Bun.spawn([bin, '--version'], { | |
| cwd: root, | |
| stdout: 'ignore', | |
| stderr: 'ignore', | |
| timeout: VERSION_PROBE_TIMEOUT_MS, | |
| shell: process.platform === 'win32', | |
| }); | |
| } catch (e) { | |
| return false; | |
| } |
References
- Always wrap process spawning calls (such as
Bun.spawn) in atry/catchblock when probing system binaries on the PATH. OS-level errors (e.g.,ENOEXECdue to invalid ELF headers, or permission issues) can cause the spawn call to throw synchronous errors, which must be caught to prevent application crashes and allow graceful fallbacks.
Summary
In the Vue test fixture,
Bun.which('typescript-language-server')resolved to a mise shim that exits 1 ("mise ERROR No version is set for shim: typescript-language-server") because no Node version is active there.TypescriptServer.spawnlaunched it, it exited at once,touchFilewaited for diagnostics that never came, and three Vue integration tests timed out at the 5s per-test limit. The last one failed withConnectionError: Connection is closed.The server now probes the binary with
--versionand skips it when it cannot run. The existing unversionedbunx typescript-language-serverfallback would not help, because bunx checks PATH first and runs the same shim. The fallback therefore runstypescript-language-server@latest, so bunx installs from the registry (printed 6.0.2).What changed
--versionprobe from fix(lsp): fall back to downloaded dart sdk when system dart cannot run #129 moves into an internal helperfindRunnableBin(name, root)inpackages/lsp/src/server/utils.ts. It is not added to the public re-export list.TypescriptServeruses it fortypescript-language-serverand falls back tobunx typescript-language-server@latest.DartServercalls the same helper; its behavior is unchanged.findSystemDarttofindRunnableBinand use a generic fake binary.Scope
Covers the typescript-language-server path and the shared probe. It does not touch the biome fallback (see Follow-up).
Blast radius
packages/lsponly. Machines wheretypescript-language-serverruns take the same path as before. Only an unrunnable binary now falls back to bunx.Verification
Run locally with Bun 1.4.3:
vue.integration.test.ts: 8 pass / 3 fail before, 11 pass / 0 fail after. The typescript server connects in the fixture in about 3.6s andworkspaceSymbol('add')finds it.bun test ./test/unit: 164 pass / 0 fail.bun testinpackages/lsp: 191 pass / 9 skip / 0 fail. The 9 skips are the Dart integration suite, skipped becausedarthere is only a broken mise shim.bun run typecheckpasses andbun run lintexits 0.Follow-up (not fixed here)
biome's
bunx biomefallback installs the unrelated npm packagebiome(0.3.3) instead of@biomejs/biome. That package does not understandlsp-proxy, so biome initialize times out after 45s in the Vue fixture. It slows the suite by about 50s but fails no test.Related issue
Closes #127
Checklist
findRunnableBin;packages/lspsuite passes locally)bun run test(left to CI)bun run lint)Summary by cubic
Fixes Vue integration tests timing out when
typescript-language-serveron PATH is an unrunnable mise shim that exits 1 before the server can connect.findRunnableBin, a shared helper that probes each PATH entry with--versionand skips any that fail to run.TypescriptServernow prefers a project-localtypescript-language-server(probed with--version), falls back tofindRunnableBin, then tobunx typescript-language-server@4.3.3, which installs from the registry instead of reusing the shim..cmdshims run.DartServeruses the same helper; its behavior is unchanged.findRunnableBinand made generic; fallback order is tested.Closes #127.
Written for commit f43239f. Summary will update on new commits.