chore: migrate from Node.js/npm to Bun#612
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis pull request migrates the project from npm and Node.js to Bun as the primary package manager and runtime. The migration includes: removing Node.js version pinning (.nvmrc files), updating the packageManager field in package.json to Bun, replacing all npm/npx commands with bun/bunx equivalents across scripts and workflows, updating GitHub Actions workflows to use Bun setup instead of Node setup, modifying the Dockerfile to include Bun binary stages, rewriting services/platform/server.js from Express to Bun's native HTTP server, and updating all documentation and configuration files to reflect Bun-based commands. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/setup-turbo/action.yml:
- Around line 5-7: The action's input default uses an unstable 'latest' for
bun-version; change the default value for the bun-version input (symbol:
bun-version) from 'latest' to a pinned version (e.g., '1.3.10') so CI is
reproducible across runs—update the default in the action.yml input definition
accordingly to match the packageManager setting used elsewhere.
In @.github/workflows/lint.yml:
- Around line 126-129: The GitHub Actions step "Setup Bun" currently sets
bun-version: latest which risks non-reproducible CI; change the step to pin a
specific Bun version (e.g., bun@1.3.10) by setting bun-version to that exact
version string, or implement logic to read the Bun version from package.json's
packageManager field and pass it to the "Setup Bun" step so the CI uses the
project-specified version instead of latest.
In `@services/platform/Dockerfile`:
- Around line 7-8: The Dockerfile uses an unpinned base image "FROM
oven/bun:latest AS bun-bin" which can lead to non-reproducible builds; change
the FROM line to pin the Bun image to the project packageManager version (e.g.,
use "oven/bun:1.3.10") so the bun-bin stage consistently uses the same Bun
release as declared in packageManager.
In `@services/platform/scripts/generate-admin-password.mjs`:
- Line 1: No code change is required—the bcryptjs usage and the new shebang in
generate-admin-password.mjs are valid under Bun; either leave the script as-is
or, if you want to remove the bcryptjs dependency, replace the bcryptjs import
and hash/compare calls with Bun's built-in API by deleting the bcryptjs
require/import and using Bun.password(...) to create the hash (and the
corresponding Bun verification method) in the functions that currently call
bcryptjs hashing/comparison.
In `@services/platform/server.js`:
- Around line 41-43: The code reads indexHtmlTemplate from
Bun.file(join(distDir, 'index.html')).text() without handling a missing file;
wrap that read in a try/catch (or check existence via Bun.file(...).exists())
around the indexHtmlTemplate assignment in the server startup logic so a missing
dist/index.html doesn't throw; on failure log a clear error including distDir
and the filename, set indexHtmlTemplate to a small safe fallback HTML (or null)
and ensure request handlers return a 500 or the fallback page if
indexHtmlTemplate is absent to avoid unhandled exceptions (refer to
indexHtmlTemplate, Bun.file, join, distDir).
- Around line 31-39: The current check using resolve(pathname.slice(1)) and
startsWith(distDir) can be bypassed by symlinks; update the logic around
filePath/Bun.file to also resolve real paths for both the candidate file and
distDir (e.g., use a realpath call) and compare those canonical paths to ensure
the file actually resides inside distDir, then verify existence via
Bun.file(...).exists() before returning the Response; reference the variables
pathname, distDir, filePath and the Bun.file existence check when making this
change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.jsonservices/platform/convex/migrations/backfill_thread_metadata.tsis excluded by!**/migrations/**services/platform/convex/migrations/backfill_workflow_schedules.tsis excluded by!**/migrations/**services/platform/convex/migrations/remove_deprecated_llm_fields.tsis excluded by!**/migrations/**services/platform/convex/migrations/trigger_steps_to_start.tsis excluded by!**/migrations/**
📒 Files selected for processing (44)
.agents/react-doctor/AGENTS.md.agents/react-doctor/SKILL.md.claude/CLAUDE.md.claude/hooks/format.sh.github/actions/setup-turbo/action.yml.github/workflows/build.yml.github/workflows/commitlint.yml.github/workflows/lint.yml.github/workflows/test.yml.gitignore.husky/commit-msg.nvmrcREADME.mddocs/zero-downtime-deployment.mdknip.config.tspackage.jsonpatches/convex-helpers@0.1.113.patchservices/platform/.nvmrcservices/platform/Dockerfileservices/platform/convex/README.mdservices/platform/convex/betterAuth/generated_schema.tsservices/platform/convex/betterAuth/schema.tsservices/platform/convex/integrations/create_integration.tsservices/platform/convex/workflow_engine/types/nodes.tsservices/platform/docker-entrypoint.shservices/platform/docs/sql-integrations/README.mdservices/platform/package.jsonservices/platform/scripts/README.mdservices/platform/scripts/dev.mjsservices/platform/scripts/encrypt-inline-secret.mjsservices/platform/scripts/generate-admin-password.mjsservices/platform/scripts/generate-openapi.tsservices/platform/scripts/sync-convex-env-from-dotenv.mjsservices/platform/server.jsservices/platform/stress-tests/poll.tsservices/platform/stress-tests/run-stress-test.tsservices/platform/stress-tests/scenarios/concurrent-starts.tsservices/platform/stress-tests/scenarios/loop-contention.tsservices/platform/stress-tests/scenarios/payload-pressure.tsservices/platform/stress-tests/scenarios/scheduler-overlap.tsservices/platform/stress-tests/scenarios/shard-comparison.tsservices/platform/stress-tests/scenarios/sustained-load.tstests/PROTEL_INTEGRATION_TESTING.mdtools/cli/package.json
💤 Files with no reviewable changes (2)
- services/platform/.nvmrc
- .nvmrc
| bun-version: | ||
| description: Bun version | ||
| default: 'latest' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pinning the default Bun version for reproducibility.
The default bun-version: 'latest' could lead to CI inconsistencies when new Bun versions are released. Since this composite action is used across multiple workflows, pinning the version here (e.g., to 1.3.10 per the PR's packageManager setting) would ensure consistent behavior across all jobs.
♻️ Proposed fix to pin default version
inputs:
bun-version:
description: Bun version
- default: 'latest'
+ default: '1.3.10'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bun-version: | |
| description: Bun version | |
| default: 'latest' | |
| bun-version: | |
| description: Bun version | |
| default: '1.3.10' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/setup-turbo/action.yml around lines 5 - 7, The action's
input default uses an unstable 'latest' for bun-version; change the default
value for the bun-version input (symbol: bun-version) from 'latest' to a pinned
version (e.g., '1.3.10') so CI is reproducible across runs—update the default in
the action.yml input definition accordingly to match the packageManager setting
used elsewhere.
| - name: Setup Bun | ||
| uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| node-version: '24' | ||
| cache: 'npm' | ||
| bun-version: latest |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pinning the Bun version for reproducible CI builds.
Using bun-version: latest in the Knip job may lead to inconsistent behavior if a new Bun version introduces breaking changes. The PR description mentions bun@1.3.10 as the project's packageManager version.
♻️ Proposed fix to pin Bun version
- name: Setup Bun
uses: oven-sh/setup-bun@v2
with:
- bun-version: latest
+ bun-version: '1.3.10'Alternatively, you could read the version from package.json#packageManager to keep it in sync automatically.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Setup Bun | |
| uses: oven-sh/setup-bun@v2 | |
| with: | |
| node-version: '24' | |
| cache: 'npm' | |
| bun-version: latest | |
| - name: Setup Bun | |
| uses: oven-sh/setup-bun@v2 | |
| with: | |
| bun-version: '1.3.10' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/lint.yml around lines 126 - 129, The GitHub Actions step
"Setup Bun" currently sets bun-version: latest which risks non-reproducible CI;
change the step to pin a specific Bun version (e.g., bun@1.3.10) by setting
bun-version to that exact version string, or implement logic to read the Bun
version from package.json's packageManager field and pass it to the "Setup Bun"
step so the CI uses the project-specified version instead of latest.
| # Bun binary for all stages | ||
| FROM oven/bun:latest AS bun-bin |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Pin the Bun image version for reproducible builds.
Using oven/bun:latest can cause unexpected behavior if the image updates. Pin to a specific version matching your packageManager field (1.3.10).
🔧 Proposed fix
-FROM oven/bun:latest AS bun-bin
+FROM oven/bun:1.3.10 AS bun-bin📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Bun binary for all stages | |
| FROM oven/bun:latest AS bun-bin | |
| # Bun binary for all stages | |
| FROM oven/bun:1.3.10 AS bun-bin |
🧰 Tools
🪛 Checkov (3.2.334)
[low] 8-8: Ensure the base image uses a non latest version tag
(CKV_DOCKER_7)
🪛 Hadolint (2.14.0)
[warning] 8-8: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag
(DL3007)
🪛 Trivy (0.69.1)
[warning] 8-8: ':latest' tag used
Specify a tag in the 'FROM' statement for image 'oven/bun'
Rule: DS-0001
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/Dockerfile` around lines 7 - 8, The Dockerfile uses an
unpinned base image "FROM oven/bun:latest AS bun-bin" which can lead to
non-reproducible builds; change the FROM line to pin the Bun image to the
project packageManager version (e.g., use "oven/bun:1.3.10") so the bun-bin
stage consistently uses the same Bun release as declared in packageManager.
| @@ -1,8 +1,8 @@ | |||
| #!/usr/bin/env node | |||
| #!/usr/bin/env bun | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the actual file content
echo "=== File content ==="
cat services/platform/scripts/generate-admin-password.mjs
echo ""
echo "=== Package.json bcryptjs version ==="
if [ -f "services/platform/package.json" ]; then
cat services/platform/package.json | jq '.dependencies.bcryptjs, .devDependencies.bcryptjs'
else
echo "Package.json not found at services/platform/"
fi
# Check if bcryptjs is in a root package.json
if [ -f "package.json" ]; then
echo ""
echo "=== Root package.json bcryptjs version ==="
cat package.json | jq '.dependencies.bcryptjs, .devDependencies.bcryptjs'
fiRepository: tale-project/tale
Length of output: 2098
🏁 Script executed:
# Check for native bindings in bcryptjs
echo "=== Checking for native bindings in bcryptjs ==="
if [ -d "node_modules/bcryptjs" ]; then
echo "bcryptjs found in node_modules"
# Look for binding.gyp which indicates native addons
if [ -f "node_modules/bcryptjs/binding.gyp" ]; then
echo "Found binding.gyp - has native bindings"
else
echo "No binding.gyp found - appears to be pure JavaScript"
fi
# Check package.json for native build scripts
cat node_modules/bcryptjs/package.json | jq '.scripts | keys'
else
echo "bcryptjs not found in node_modules"
fiRepository: tale-project/tale
Length of output: 144
🏁 Script executed:
# List the directory structure of bcryptjs to understand its nature
echo "=== bcryptjs directory structure ==="
if [ -d "node_modules/bcryptjs" ]; then
find node_modules/bcryptjs -type f -name "*.node" -o -name "*.gyp" 2>/dev/null | head -20
echo "Main files in bcryptjs:"
ls -la node_modules/bcryptjs/ | head -15
fiRepository: tale-project/tale
Length of output: 97
🌐 Web query:
bcryptjs Bun compatibility pure JavaScript
💡 Result:
-
Yes—
bcryptjsis Bun-compatible because it’s an optimized, zero-dependency, pure JavaScript bcrypt implementation (no native Node addons /node-gyp), so it runs anywhere JS runs (including Bun). (github.com) -
In Bun, you can generally use it like:
import bcrypt from "bcryptjs"; const hash = await bcrypt.hash("pw", 10); const ok = await bcrypt.compare("pw", hash);
(Async API available; sync variants exist.)
-
Tradeoff: being pure JS, it’s slower than native bindings (the project notes ~30% slower). (github.com)
-
Bun alternative: Bun has built-in password hashing/verification via
Bun.password(including bcrypt) if you’d rather avoid a dependency. (bun.sh)
Citations:
- 1: https://github.com/dcodeIO/bcrypt.js/?utm_source=openai
- 2: https://github.com/dcodeIO/bcrypt.js/?utm_source=openai
- 3: https://bun.sh/docs/runtime/hashing?utm_source=openai
bcryptjs is fully compatible with Bun—no action required.
bcryptjs is a pure JavaScript implementation with zero native dependencies, so it works seamlessly under Bun. The shebang change is appropriate, and the script will execute securely.
Alternatively, consider using Bun's built-in Bun.password API for bcrypt hashing to eliminate the dependency, though the current approach is perfectly valid and maintainable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/scripts/generate-admin-password.mjs` at line 1, No code
change is required—the bcryptjs usage and the new shebang in
generate-admin-password.mjs are valid under Bun; either leave the script as-is
or, if you want to remove the bcryptjs dependency, replace the bcryptjs import
and hash/compare calls with Bun's built-in API by deleting the bcryptjs
require/import and using Bun.password(...) to create the hash (and the
corresponding Bun verification method) in the functions that currently call
bcryptjs hashing/comparison.
| if (pathname !== '/') { | ||
| const filePath = resolve(distDir, pathname.slice(1)); | ||
| if (filePath.startsWith(distDir)) { | ||
| const file = Bun.file(filePath); | ||
| if (await file.exists()) { | ||
| return new Response(file); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Strengthen path traversal protection.
The resolve() function can normalize .. sequences, but consider using Bun.file().exists() check combined with realpath to handle symlinks and ensure the resolved path truly resides within distDir.
🛡️ Proposed improvement
if (pathname !== '/') {
- const filePath = resolve(distDir, pathname.slice(1));
- if (filePath.startsWith(distDir)) {
+ const filePath = join(distDir, pathname.slice(1));
+ const resolved = resolve(filePath);
+ if (resolved.startsWith(distDir + '/') || resolved === distDir) {
const file = Bun.file(filePath);
if (await file.exists()) {
return new Response(file);
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/server.js` around lines 31 - 39, The current check using
resolve(pathname.slice(1)) and startsWith(distDir) can be bypassed by symlinks;
update the logic around filePath/Bun.file to also resolve real paths for both
the candidate file and distDir (e.g., use a realpath call) and compare those
canonical paths to ensure the file actually resides inside distDir, then verify
existence via Bun.file(...).exists() before returning the Response; reference
the variables pathname, distDir, filePath and the Bun.file existence check when
making this change.
| if (!indexHtmlTemplate) { | ||
| indexHtmlTemplate = await Bun.file(join(distDir, 'index.html')).text(); | ||
| } |
There was a problem hiding this comment.
Add error handling for missing index.html.
If dist/index.html doesn't exist (e.g., build wasn't run), the server will throw an unhandled error. Consider adding a graceful fallback.
🛡️ Proposed improvement
if (!indexHtmlTemplate) {
- indexHtmlTemplate = await Bun.file(join(distDir, 'index.html')).text();
+ const indexFile = Bun.file(join(distDir, 'index.html'));
+ if (!(await indexFile.exists())) {
+ return new Response('Build not found. Run `bun run build` first.', { status: 503 });
+ }
+ indexHtmlTemplate = await indexFile.text();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/server.js` around lines 41 - 43, The code reads
indexHtmlTemplate from Bun.file(join(distDir, 'index.html')).text() without
handling a missing file; wrap that read in a try/catch (or check existence via
Bun.file(...).exists()) around the indexHtmlTemplate assignment in the server
startup logic so a missing dist/index.html doesn't throw; on failure log a clear
error including distDir and the filename, set indexHtmlTemplate to a small safe
fallback HTML (or null) and ensure request handlers return a 500 or the fallback
page if indexHtmlTemplate is absent to avoid unhandled exceptions (refer to
indexHtmlTemplate, Bun.file, join, distDir).
Replace Node.js/npm with Bun across the entire monorepo: - Set packageManager to bun@1.3.10 in root package.json - Replace all npm/npx commands with bun/bunx in scripts, CI, Docker - Replace Express.js production server with native Bun.serve() - Remove express and @types/express dependencies - Delete package-lock.json, add bun.lock - Delete .nvmrc files (no longer needed) - Update GitHub Actions to use oven-sh/setup-bun@v2 - Update Dockerfile to use oven/bun:latest base image - Update all shell scripts shebangs and commands - Add package-lock.json to .gitignore
Bun runs TypeScript natively, so all scripts can be properly typed: - scripts/dev.mjs → dev.ts - scripts/sync-convex-env-from-dotenv.mjs → sync-convex-env-from-dotenv.ts - scripts/encrypt-inline-secret.mjs → encrypt-inline-secret.ts - scripts/generate-admin-password.mjs → generate-admin-password.ts - server.js → server.ts - .storybook/main.js → main.ts Add @types/bun for Bun global type definitions. Update all references in package.json, Dockerfile, and docker-entrypoint.sh.
Users can now upload connector.ts files in addition to connector.js. TypeScript is transpiled to JavaScript at upload time using sucrase, so the stored code and VM sandbox execution remain plain JS.
yannickmonney
force-pushed
the
chore/migrate-to-bun
branch
from
0a5ac32 to
e62a76b
Compare
…cript All 9 example connector files now use TypeScript with full sandbox API type definitions. Tests transpile the .ts files via sucrase before passing to the VM sandbox.
…es, a11y) - Clean up knip config: add server.ts entry, remove stale ignores - Unexport 5 types only used locally (TextFileCategory, EnvSetupOptions, etc.) - Fix 27 no-shadow errors: rename shadowed variables across dialogs, convex mutations, stream buffer, error boundaries, and data table - Memoize 6 context values with useMemo to fix jsx-no-constructed-context-values - Cache error boundary context value in class component to avoid reconstruction - Hoist story ability constants to module scope - Fix a11y: use semantic button for folder names, use role="group" for drop zone
…e.json files fix(FileUpload): change dropzone role from button to group for accessibility
1 participant
Replace Node.js/npm with Bun across the entire monorepo:
Summary by CodeRabbit
Chores
Refactor