-
Notifications
You must be signed in to change notification settings - Fork 625
inject bun and uv runtime | 增加 bun 和 uv 的 runtime #519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce support for Bun and UV runtimes alongside Node, updating scripts, runtime detection, and command handling logic. The PowerpackServer now uses Bun for code execution, and the MCP client and server manager are refactored to handle Bun and UV runtimes, including registry selection. New unit tests verify these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ServerManager
participant McpClient
participant PowerpackServer
User->>ServerManager: startServer()
ServerManager->>McpClient: new McpClient(npmRegistry, uvRegistry)
ServerManager->>PowerpackServer: (unrelated to runtime setup)
McpClient->>McpClient: Detect bunRuntimePath, uvRuntimePath
McpClient->>McpClient: Process command/args for runtime substitution
McpClient->>McpClient: Update environment PATH with bun/uv paths
Note over McpClient: When executing code/tool
McpClient->>PowerpackServer: run_node_code (uses Bun runtime)
PowerpackServer->>PowerpackServer: executeBunCode()
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
package.json (1)
44-53: Consolidate and harden runtime-install/clean scripts
Six near-duplicate
installRuntime:*scripts inflatepackage.jsonand are easy to forget when flags change.
• Consider a single Node script (or Nx/Makefile) that derives platform/arch at runtime and invokestiny-runtime-injectoronce.
cleanRuntimeusesrm -rf→ fails on Windows shells. Prefer the cross-platformrimrafpackage (pnpm dlx rimraf runtime).
tiny-runtime-injectorwas removed fromdevDependenciesyet you still rely on it vianpx. That works only with internet access; vendoring it (or usingpnpm dlx) keeps CI/offline installs deterministic.src/main/presenter/mcpPresenter/serverManager.ts (1)
85-90: uvRegistry may desynchronise from the registry actually used
uvRegistryis pinned based on the fastest registry before you honourserverConfig.customNpmRegistry.
If a server specifies a custom npm registry different fromnpmmirror, you will still pass the mirror-specificuvRegistry, which could be incorrect.Either recompute
uvRegistryinsidestartServerafter resolving the effective npm registry, or letMcpClientderive it from the registry string.test/main/presenter/mcpClient.test.ts (2)
94-98: Make path checks platform-neutral
pathStr.includes('runtime/bun')/'runtime/uv'will fail on Windows becausepath.joinproduces back-slashes (runtime\\bun).
Usepath.normalizeor a regex insensitive to separators to keep the test green on all OSes.- return pathStr.includes('runtime/bun') || pathStr.includes('runtime/uv') + return /runtime[\\/](bun|uv)/.test(pathStr)
330-337: Prefer setting env vars toundefinedoverdeletein tests
delete process.env.TEST_VARworks but triggers Biome’s no-delete rule and is marginally slower. Simpler:- delete process.env.TEST_VAR + process.env.TEST_VAR = undefinedSame applies to
TEST_PATHbelow.src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts (2)
28-33: Update schema description to reference Bun instead of Node.js
RunNodeCodeArgsSchemastill describes “Node.js code”.
Replace wording with “JavaScript/TypeScript code (executed by Bun)” to avoid misleading clients.
240-249: Tool name mismatch after switching to BunThe tool is still exposed as
run_node_codebut now executes via Bun.
For backward compatibility keep the alias, but expose a new canonical name (e.g.run_js_code) and document thatrun_node_codeis deprecated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(3 hunks)src/main/presenter/mcpPresenter/inMemoryServers/powerpackServer.ts(7 hunks)src/main/presenter/mcpPresenter/mcpClient.ts(7 hunks)src/main/presenter/mcpPresenter/serverManager.ts(4 hunks)test/main/presenter/mcpClient.test.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
test/main/presenter/mcpClient.test.ts
[error] 376-376: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 390-390: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-check (x64)
🔇 Additional comments (1)
src/main/presenter/mcpPresenter/mcpClient.ts (1)
105-118: ```shell
#!/bin/bashShow the first 200 lines of mcpClient.ts for context
sed -n '1,200p' src/main/presenter/mcpPresenter/mcpClient.ts
Show lines 200–350 to include the runtime selection logic and command execution
sed -n '200,350p' src/main/presenter/mcpPresenter/mcpClient.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Pull Request Description (中文)
你的功能请求是否与某个问题有关?请描述一下。
增加了bun和uv作为runtime,去掉了原有的node runtime
牺牲了一点点兼容性,但是获得了快速的安装体验,开箱即用的舒适度以及更可靠的环境
同时包还有了几十到一百兆左右的减少
请描述你希望的解决方案
使用了
https://www.npmjs.com/package/tiny-runtime-injector
来注入 runtime
Summary by CodeRabbit