Conversation
thdxr
commented
Mar 10, 2026
thdxr
commented
- refactor: lsp server and core improvements
- sync
- Update packages/opencode/src/npm/index.ts
- Update packages/opencode/src/util/which.ts
- sync
- core: dynamically resolve formatter executable paths at runtime
- core: disable npm bin links to fix package installation in sandboxed environments
- core: fix dependency installation failures behind corporate proxies or in CI by disabling Bun cache when network interception is detected
- core: enable npm bin links on non-Windows platforms to allow plugin executables to work while keeping them disabled on Windows CI where symlink permissions are restricted
- core: fix npm dependency installation on Windows CI by disabling bin links when symlink permissions are restricted
- tui: export sessions using consistent Filesystem API instead of Bun.write
- sync
- core: fix CLI tools from npm packages not being accessible after install on Windows
- sync
- core: log npm install errors to console for debugging dependency failures
- sync
- sync
- sync
- core: fix custom tool loading to properly resolve module paths
- tui: fix Windows plugin loading by using direct paths instead of file URLs
- fix: work around Bun/Windows UV_FS_O_FILEMAP incompatibility in tar (fix: work around Bun/Windows UV_FS_O_FILEMAP incompatibility in tar #16853)
- core: add Node.js runtime support
- refactor(server): replace Bun serve with Hono node adapters
- core: bundle database migrations into node build and auto-start server on port 1338
- sync
- core: remove shell execution and server URL from plugin API
- core: return structured server info with stop method from workspace server
- core: cleaner error output and more flexible custom tool directories
Greptile SummaryThis PR is a major architectural refactor that adds Node.js runtime support alongside Bun, replacing Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Plugin / LSP / Formatter needs binary] --> B{Binary on system PATH?}
B -- Yes --> C[Use system binary]
B -- No --> D[Npm.which called]
D --> E{dot-bin dir exists and non-empty?}
E -- No --> F[Npm.add via arborist]
F --> G{Package has bin entries?}
G -- Yes --> H[Return first file from dot-bin]
G -- No --> I["⚠️ Recurse back to Npm.which - INFINITE LOOP"]
E -- Yes --> H
H --> J{Platform?}
J -- Linux or Mac --> K[Correct executable]
J -- Windows --> L["⚠️ files-0 may be wrong file type"]
subgraph DB["DB Init"]
M[Server starts] --> N{Runtime?}
N -- Bun --> O["#db → db.bun.ts - BunSQLiteDatabase"]
N -- Node.js --> P["#db → db.node.ts - NodeSQLiteDatabase"]
O --> Q["migrate from bun-sqlite/migrator ✅"]
P --> R["migrate from bun-sqlite/migrator ⚠️ type mismatch"]
end
subgraph SRV["Server Lifecycle"]
S[Server.listen called] --> T[createAdaptorServer]
T --> U[Await listening event]
U --> V[Read actual bound port]
V --> W[Return Listener with stop method]
end
Last reviewed commit: 7925df1 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function which(pkg: string) { | ||
| const dir = path.join(directory(pkg), "node_modules", ".bin") | ||
| const files = await readdir(dir).catch(() => []) | ||
| if (!files.length) { | ||
| await add(pkg) | ||
| return which(pkg) | ||
| } | ||
| return path.join(dir, files[0]) | ||
| } |
There was a problem hiding this comment.
Infinite recursion if package has no bin entries
If add(pkg) successfully installs a package that has no entries in node_modules/.bin (e.g., it only exposes a library API), readdir(dir) will return [] again after the install and the function recurses forever. The add() function will also return early on the second call (via the loadVirtual() check finding an existing tree), meaning there's no escape condition.
For example, if typescript-language-server installs but places its binary somewhere other than the expected .bin path, or if there's a transient filesystem race, this will loop indefinitely.
A guard against unbounded recursion is needed:
| export async function which(pkg: string) { | |
| const dir = path.join(directory(pkg), "node_modules", ".bin") | |
| const files = await readdir(dir).catch(() => []) | |
| if (!files.length) { | |
| await add(pkg) | |
| return which(pkg) | |
| } | |
| return path.join(dir, files[0]) | |
| } | |
| export async function which(pkg: string, retry = false): Promise<string> { | |
| const dir = path.join(directory(pkg), "node_modules", ".bin") | |
| const files = await readdir(dir).catch(() => []) | |
| if (!files.length) { | |
| if (retry) throw new InstallFailedError({ pkg }) | |
| await add(pkg) | |
| return which(pkg, true) | |
| } | |
| return path.join(dir, files[0]) | |
| } |
| await add(pkg) | ||
| return which(pkg) | ||
| } | ||
| return path.join(dir, files[0]) |
There was a problem hiding this comment.
First file in .bin may not be the correct executable on Windows
files[0] returns the alphabetically first file in the .bin directory. On Windows, npm typically creates three variants for each binary: the bare script (no extension), a .cmd file, and a .ps1 file. Since readdir returns sorted results, files[0] would be the alphabetically first entry — which could be the bare script (not executable on Windows without Node.js on PATH) rather than the .cmd wrapper that Windows shells actually need.
For example, for biome, the sorted list might be ["biome", "biome.cmd", "biome.ps1"]. On Windows, the process spawner needs biome.cmd to launch correctly from a shell, but files[0] returns the bare biome script.
Consider filtering for the platform-appropriate extension:
const ext = process.platform === "win32" ? ".cmd" : ""
const match = files.find((f) => path.extname(f) === ext) ?? files[0]
return path.join(dir, match)| import { type SQLiteBunDatabase } from "drizzle-orm/bun-sqlite" | ||
| import { migrate } from "drizzle-orm/bun-sqlite/migrator" |
There was a problem hiding this comment.
bun-sqlite migrator imported at top-level, incompatible with Node.js init()
migrate is imported from drizzle-orm/bun-sqlite/migrator at the top of this file. In the Bun build, #db resolves to db.bun.ts (returning a BunSQLiteDatabase) so migrate(db, entries) at line 104 is type-compatible. However, in the Node.js build (#db → db.node.ts), init(Path) returns a drizzle-orm/node-sqlite database object.
The bun-sqlite migrator is typed to accept BunSQLiteDatabase exclusively, so calling migrate(db, entries) where db is a NodeSQLiteDatabase is a type mismatch and may fail at runtime if the migrator internally relies on bun-specific methods.
The Node.js build needs to use drizzle-orm/node-sqlite/migrator instead. This could be addressed the same way init is handled — via the #db import condition, or by branching the migrate import similarly.
| } | ||
|
|
||
| export function Listen(opts: { hostname: string; port: number }) { | ||
| return Bun.serve({ | ||
| hostname: opts.hostname, | ||
| port: opts.port, | ||
| const server = createAdaptorServer({ | ||
| fetch: App().fetch, | ||
| }) | ||
| server.listen(opts.port, opts.hostname) | ||
| return { | ||
| hostname: opts.hostname, | ||
| port: opts.port, | ||
| stop() { | ||
| return new Promise<void>((resolve, reject) => { | ||
| server.close((err) => { | ||
| if (err) { | ||
| reject(err) | ||
| return | ||
| } | ||
| resolve() | ||
| }) | ||
| }) | ||
| }, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Listen returns opts.port before the socket actually binds
server.listen(opts.port, opts.hostname) is asynchronous — it schedules a bind and returns immediately. The returned object then uses opts.port as the reported port. If opts.port === 0 (OS-assigned ephemeral port), the caller receives port: 0 instead of the actual bound port, making the address unusable.
By contrast, the main Server.listen correctly awaits the "listening" event and reads server.address().port. The same pattern should be applied here:
export async function Listen(opts: { hostname: string; port: number }) {
const server = createAdaptorServer({ fetch: App().fetch })
await new Promise<void>((resolve, reject) => {
server.once("error", reject)
server.once("listening", resolve)
server.listen(opts.port, opts.hostname)
})
const addr = server.address()
const boundPort = addr && typeof addr !== "string" ? addr.port : opts.port
return {
hostname: opts.hostname,
port: boundPort,
stop() { ... }
}
}| export function close() { | ||
| const sqlite = state.sqlite | ||
| if (!sqlite) return | ||
| sqlite.close() | ||
| state.sqlite = undefined | ||
| Client().$client.close() | ||
| Client.reset() | ||
| } |
There was a problem hiding this comment.
close() initializes the lazy client just to close it
The old code used a separate state.sqlite reference and guarded with if (!sqlite) return before closing. With the new implementation, calling close() when the database was never opened will trigger Client() (the lazy initializer), which opens a brand-new SQLite connection — just to immediately close it. This is wasteful and may produce unexpected side effects (e.g., WAL and PRAGMA setup, migration runs).
Consider adding a guard to avoid initializing a client that was never needed:
export function close() {
if (!Client.initialized()) return // assuming lazy() exposes an initialized flag
Client().$client.close()
Client.reset()
}If lazy() doesn't expose an initialized() check, a separate boolean flag like the old state.sqlite pattern would work.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Formatters now determine their executable location when enabled rather than using hardcoded paths. This ensures formatters work correctly regardless of how the tool was installed or where executables are located on the system.
…r in CI by disabling Bun cache when network interception is detected
…xecutables to work while keeping them disabled on Windows CI where symlink permissions are restricted
…links when symlink permissions are restricted
Enable running opencode on Node.js by adding platform-specific database adapters and replacing Bun-specific shell execution with cross-platform Process utility.
Plugins no longer receive shell access or server URL to prevent unauthorized execution and limit plugin sandbox surface area.
…erver - Enables graceful server shutdown for workspace management - Removes unsupported serverUrl getter that threw errors in plugin context
- Removed debug console.log when dependency installation fails so users see clean warning messages instead of raw error dumps - Fixed database connection cleanup to prevent resource leaks between sessions - Added support for loading custom tools from both .opencode/tool (singular) and .opencode/tools (plural) directories, matching common naming conventions
…s APIs for OAuth servers and retry logic
2 participants
