feat: support Bun as a Caplets process runtime#241
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughCaplets adds Bun 1.3.14+ support alongside Node.js 22+, introduces runtime-aware HTTP and telemetry behavior, replaces synchronous SQLite access with async libSQL storage, and expands CI, migration, smoke-test, and documentation coverage. ChangesCross-runtime support
Async SQLite storage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Preview DeployedLanding: https://pr-241.preview.caplets.dev Built from commit 6badbbd |
|
| Filename | Overview |
|---|---|
| packages/core/src/storage/database.ts | Replaces better-sqlite3 with @libsql/client; adds SqliteOperationQueue to serialize concurrent async operations; handles ephemeral (:memory:) databases with temp dirs; queue entries correctly cleaned up for ephemeral paths |
| packages/core/src/storage/migrations.ts | Migrates SQLite migration runner from better-sqlite3 exclusive transaction to libsql async write transaction; logic preserves hash/order integrity checks and schema-version upsert |
| packages/core/src/storage/caplet-records.ts | All SQLite calls migrated to async; importBundlesInTransaction and getInTransaction carry incorrect nested-Promise return type annotations (P2); runtime behavior unaffected |
| packages/core/src/serve/http.ts | Adds Bun-native HTTP/WebSocket server path alongside Node adapter; SSE preamble injection for Bun GET streams; double-stop behavior for graceful drain already noted in previous thread |
| packages/core/src/telemetry/providers.ts | Selects @sentry/bun vs @sentry/node SDK at runtime using runtimeDescriptor(); Bun path uses makeFetchTransport and defaultStackParser from @sentry/bun as expected |
| packages/core/src/telemetry/runtime-environment.ts | New module: runtimeDescriptor() detects bun vs node from process.versions; injectable for testing; straightforward implementation |
| packages/core/src/admin-api/router.ts | Replaces Readable.fromWeb (Node-only) with requestBodyReadable helper using manual ReadableStreamDefaultReader iteration, enabling Bun compatibility for bundle upload streaming |
| packages/core/src/storage/legacy-migration.ts | Removes runSynchronous wrapper; imports/verifications are now properly awaited inside the async SQLite transaction callback; cleaner error propagation |
| .github/workflows/ci.yml | Adds verify-bun job with Bun 1.3.14/latest matrix; runs pnpm verify (Node tests) + bun package-runtime smoke; release-blocking coverage for minimum and latest supported Bun versions |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([HTTP serve / DB open]) --> RuntimeCheck{process.versions.bun?}
RuntimeCheck -->|Yes| BunHTTP[Bun.serve with bunWebSocket handler]
RuntimeCheck -->|No| NodeHTTP["@hono/node-server with ws WebSocketServer"]
BunHTTP --> BunSSE[prependEventStreamPreamble for MCP GET streams]
BunHTTP --> BunUpload[requestBodyReadable via ReadableStreamDefaultReader]
RuntimeCheck -->|Yes| BunSentry["import('@sentry/bun') + makeFetchTransport"]
RuntimeCheck -->|No| NodeSentry["import('@sentry/node')"]
RuntimeCheck -->|Yes| LibSQL["@libsql/client createClient(file://...)"]
RuntimeCheck -->|No| LibSQL
LibSQL --> OpQueue[SqliteOperationQueue Promise mutex]
OpQueue --> SerializedClient[serializeSqliteClient Proxy]
SerializedClient --> DrizzleLibSQL["drizzle-orm/libsql"]
SerializedClient --> SerializedTx[serializeSqliteTransaction Proxy]
SerializedTx --> CommitRelease["commit / rollback / close → release()"]
Reviews (2): Last reviewed commit: "fix: address Bun runtime review findings" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/storage/project-bindings.ts (1)
321-371: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
{ behavior: "immediate" }for these sqlite read-then-write transactions.mutateBindingSqliteselects a binding row before conditionally inserting/updating it,grantPreparedVaultSqliteselects a subject before inserting/updating, andrevokeSqliteselects subject/key state before deleting. Add{ behavior: "immediate" }to each sqlitedb.transaction(...)call to follow the existing sqlite transaction pattern and avoid deferred-transaction lock-upgrade failures under concurrency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/project-bindings.ts` around lines 321 - 371, Use the SQLite immediate transaction mode for all three read-then-write flows: update mutateBindingSqlite in packages/core/src/storage/project-bindings.ts (lines 321-371), grantPreparedVaultSqlite in packages/core/src/storage/vault-grants.ts (lines 145-159), and revokeSqlite in packages/core/src/storage/vault-grants.ts (lines 1087-1098) so each db.transaction call receives { behavior: "immediate" } while preserving their existing mutation logic.
🧹 Nitpick comments (2)
packages/core/src/storage/database.ts (1)
353-447: 🚀 Performance & Scalability | 🔵 TrivialSerialization proxy and async mutex look correct.
acquire()assignsthis.tailbefore awaiting the predecessor (proper FIFO chaining), and the transaction proxy releases only oncommit/rollback/closewhile passing query methods through, so in-transaction statements don't re-enter the queue.One operational note: all client
execute/batchcalls for a given path are serialized through a single queue, so read concurrency is fully serialized process-wide per database file. If read throughput becomes a concern under load, consider a read path that doesn't hold the write mutex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/database.ts` around lines 353 - 447, Retain the current SqliteOperationQueue and transaction-proxy behavior; no correctness change is required. If optimizing throughput, update serializeSqliteClient’s read-operation path so read-only calls can avoid holding the per-path queue while preserving serialization for writes and transaction lifecycle methods.packages/core/src/storage/migrations.ts (1)
106-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
rollback()so a failingcommit()doesn't mask the original error.Per the libSQL docs,
rollback()cannot be called aftercommit(). Ifawait transaction.commit()throws, thecatchblock callsawait transaction.rollback(), which will throw a secondary error and mask the real commit failure (and may leave the transaction unclosed). Suppress the rollback error so the original propagates.♻️ Suggested guard
await transaction.commit(); } catch (error) { - await transaction.rollback(); + await transaction.rollback().catch(() => undefined); throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/storage/migrations.ts` around lines 106 - 110, Update the transaction error handling around transaction.commit() so rollback failures are suppressed while preserving and rethrowing the original commit error. Guard the await transaction.rollback() call in the catch block, ensuring any rollback exception cannot mask the original error or prevent transaction cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/admin-api/router.ts`:
- Around line 679-690: Update requestBodyChunks and its requestBodyReadable
integration so destroying the wrapped input also cancels the underlying Web
ReadableStream reader, not merely releases its lock. Track the active reader or
generator cleanup outside the async iterator and ensure destroyInput awaits or
triggers that cleanup, including when destroy completes synchronously during an
in-flight read. Preserve normal chunk iteration and lock release behavior.
In `@packages/core/src/serve/http.ts`:
- Around line 180-185: Update the binary branch in the WebSocket message handler
to pass only the message view’s exact byte range, using its byteOffset and
byteLength to create a bounded payload before calling createWSMessageEvent.
Preserve the existing string handling and event dispatch through
socket.data.events.onMessage.
In `@packages/core/src/storage/database.ts`:
- Around line 331-340: Update the HostStorage close callback in the returned
instance to remove the ephemeral path’s entry from SQLITE_OPERATION_QUEUES after
draining the queue and before cleanup. Restrict eviction to ephemeral
configurations so shared on-disk queues remain reusable, and preserve the
existing drain, client close, and temporary-root removal behavior.
In `@packages/core/src/storage/vault-grants.ts`:
- Around line 789-818: Update assertLegacyGrantsMatchSqlite to process grants
sequentially instead of using Promise.all: iterate through grants with an
awaited loop, perform each legacyGrantSubjectSqlite call and database select
only after the previous grant completes, preserve the existing conflict error
and result shape, and return the collected { existing, values } entries after
all grants are checked.
---
Outside diff comments:
In `@packages/core/src/storage/project-bindings.ts`:
- Around line 321-371: Use the SQLite immediate transaction mode for all three
read-then-write flows: update mutateBindingSqlite in
packages/core/src/storage/project-bindings.ts (lines 321-371),
grantPreparedVaultSqlite in packages/core/src/storage/vault-grants.ts (lines
145-159), and revokeSqlite in packages/core/src/storage/vault-grants.ts (lines
1087-1098) so each db.transaction call receives { behavior: "immediate" } while
preserving their existing mutation logic.
---
Nitpick comments:
In `@packages/core/src/storage/database.ts`:
- Around line 353-447: Retain the current SqliteOperationQueue and
transaction-proxy behavior; no correctness change is required. If optimizing
throughput, update serializeSqliteClient’s read-operation path so read-only
calls can avoid holding the per-path queue while preserving serialization for
writes and transaction lifecycle methods.
In `@packages/core/src/storage/migrations.ts`:
- Around line 106-110: Update the transaction error handling around
transaction.commit() so rollback failures are suppressed while preserving and
rethrowing the original commit error. Guard the await transaction.rollback()
call in the catch block, ensuring any rollback exception cannot mask the
original error or prevent transaction cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f0e3c4c-3ed8-41b9-a1a5-55de447b7cbb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (63)
.changeset/fuzzy-buns-travel.md.github/workflows/ci.ymlCONTEXT.mdREADME.mddocs/adr/0009-scope-runtime-support-by-execution-boundary.mdmise.tomlpackage.jsonpackages/cli/package.jsonpackages/core/package.jsonpackages/core/rolldown.config.tspackages/core/src/admin-api/router.tspackages/core/src/cli.tspackages/core/src/serve/http.tspackages/core/src/storage/backend-auth-flows.tspackages/core/src/storage/backend-auth.tspackages/core/src/storage/caplet-records.tspackages/core/src/storage/coordination.tspackages/core/src/storage/dashboard-sessions.tspackages/core/src/storage/database.tspackages/core/src/storage/idempotency.tspackages/core/src/storage/installations.tspackages/core/src/storage/legacy-migration.tspackages/core/src/storage/migrations.tspackages/core/src/storage/operator-activity.tspackages/core/src/storage/project-bindings.tspackages/core/src/storage/remote-security.tspackages/core/src/storage/setup-state.tspackages/core/src/storage/types.tspackages/core/src/storage/vault-grants.tspackages/core/src/storage/vault-state.tspackages/core/src/storage/vault-values.tspackages/core/src/telemetry/events.tspackages/core/src/telemetry/index.tspackages/core/src/telemetry/privacy.tspackages/core/src/telemetry/providers.tspackages/core/src/telemetry/runtime-environment.tspackages/core/src/telemetry/runtime.tspackages/core/test/backend-auth-flow-storage.test.tspackages/core/test/backend-auth-state-table.test.tspackages/core/test/backend-auth-storage.test.tspackages/core/test/caplet-bundle-streaming.test.tspackages/core/test/caplet-record-storage.test.tspackages/core/test/current-host-administration.test.tspackages/core/test/dashboard-session-store.test.tspackages/core/test/dashboard-session.test.tspackages/core/test/host-storage-config.test.tspackages/core/test/host-storage.test.tspackages/core/test/idempotency-storage.test.tspackages/core/test/installation-storage.test.tspackages/core/test/project-binding-storage.test.tspackages/core/test/remote-security-storage.test.tspackages/core/test/setup-state-storage.test.tspackages/core/test/storage-records-cli.test.tspackages/core/test/telemetry-events.test.tspackages/core/test/telemetry-providers.test.tspackages/core/test/telemetry-runtime.test.tspackages/core/test/vault-grant-storage.test.tspackages/core/test/vault-state-storage.test.tspackages/core/test/vault-value-storage.test.tspackages/sdk/README.mdpackages/sdk/package.jsonscripts/check-package-runtime.mjsscripts/package-runtime-plan-000-smoke.mjs
💤 Files with no reviewable changes (1)
- packages/sdk/package.json
|
Addressed the outside-diff and review-body findings in
Validation: core suite (2,490 tests), core typecheck/lint, and built-package Node and Bun lifecycle/RSS smokes passed. |
1 participant
Summary
@libsql/clientand Drizzle's asynchronous libSQL adapter, migrating storage operations and transaction boundariesVerification
pnpm verifypnpm --filter @caplets/core testnode scripts/check-package-runtime.mjsbun scripts/check-package-runtime.mjspnpm install --frozen-lockfileThe built-package smoke covers HTTP discovery, WebSockets, MCP lifecycle, authentication, project binding, shutdown, and bounded 256 MiB bundle upload/download behavior under both runtimes.
Summary by CodeRabbit