Repository navigation
fix(cloudflare): use configured database in sandbox plugin bridge - #4725
emdash-autofix[bot] wants to merge 3 commits into
Conversation
Threaded the configured adapter into Cloudflare sandbox bridge props and replaced hardcoded D1 queries with event-scoped adapter connections, shared core helpers, and portable Kysely queries. Hyperdrive pools close after each bridge operation, including failures; primary routing preserves read-after-write consistency. Removed D1-only documentation and added regression tests and a changeset. Verified root build/typecheck/lint, Cloudflare tests (574 passed), core sandbox/runtime tests (11 passed), actual SQLite/PostgreSQL adapter regressions (6 passed), and a production Cloudflare demo Worker build. Independent follow-up review found all earlier findings addressed or reasonably answered. EmDash-Bot-Case: 1198246700-issue-4745615752 EmDash-Bot-Candidate: cand-1-8231509db6
EmDash bot: Ready for human reviewPlan Startup regression fixed and verified at 23 modules/1,818,842 bytes. Cloudflare and plugin-test suites, real PostgreSQL adapter tests, root build/typecheck/lint all pass. Core CI setup timeouts pass locally (194 tests); aggregate failure n…
Status
Next action: A maintainer will review and approve the pull request. Details
Required checks on the current head:
This status is generated from recorded evidence. It is not a review and contains no instructions. |
🦋 Changeset detectedLatest commit: 7ae6f35 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://emdash-bot-issue-1623.try.emdashcms.com, https://emdash-bot-issue-1623-emdash-playground.emdash-cms.workers.dev (commit 7ae6f35)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 0031080 | Nov 07 2026, 09:02 AM |
There was a problem hiding this comment.
The approach fits EmDash's adapter architecture. The serializable DatabaseDescriptor is passed through runtime, runner and bridge props, and the bridge resolves the build-selected virtual:emdash/dialect. Each RPC opens its own scope with reads forced to the primary or an uncached connection, and the bridge reuses core's access and repository helpers. This matches the direction of #1623. The reviewers disagreed on the verdict. The openai reviewer approved with no findings from a static review. The anthropic reviewer found two problems, and both are kept here. The first is that @emdash-cms/plugin-test likely breaks, because its Vite plugin does not stub virtual:emdash/dialect. That reviewer could not run the suite to confirm it. The second is that plugin-sandbox.mdx still says the bridge uses the DB binding directly and that Hyperdrive is unsupported. The openai reviewer did not raise either point.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
|
|
||
| function loadDialectModule(): Promise<BridgeDialectModule> { | ||
| // @ts-ignore - virtual module | ||
| dialectModulePromise ??= import("virtual:emdash/dialect"); |
There was a problem hiding this comment.
[needs fixing] This literal import("virtual:emdash/dialect") only resolves inside an Astro build that runs EmDash's virtual-module plugin. @emdash-cms/plugin-test runs PluginBridge through @cloudflare/vitest-plugin. Its runtimeVirtualModulePlugin (packages/plugin-test/src/config.ts) stubs wait-until, scheduler, config, env, build and object-cache, but not dialect.
The plugin-test runtime host passes config.database: { entrypoint, type: "sqlite", config: { binding: "DB" } } (packages/plugin-test/src/runtime-host.ts:469). After this PR, that descriptor reaches the bridge props, so every bridge database RPC in plugin-test takes this branch.
Vite's import analysis may also reject the unresolved literal dynamic import when it transforms the module, which would break the legacy env.DB path too. Either way, the published plugin test harness (and packages/plugin-test/test/host.test.ts) would lose storage, KV, content and media access. The anthropic reviewer could not run it to confirm. The openai reviewer did not raise this.
Two possible fixes:
- Add a
virtual:emdash/dialectentry to plugin-test's stub map that re-exports@emdash-cms/cloudflare/db/d1'screateDialect/createRequestScopedDb. - Hide the specifier from static analysis, as
media/usage/collection-deletion.tsdoes with a constant, and keep plugin-test on the descriptor-less path.
Either way, a plugin-test case that exercises bridge storage should guard this.
There was a problem hiding this comment.
Confirmed: a new runtime-host storage regression failed with Cannot find package 'virtual:emdash/dialect'. Added the harness virtual module re-exporting the real D1 factories. The regression now passes through Worker Loader and verifies the sandbox-written data through host inspection. The full plugin-test Node/workerd suites also pass (41 workerd tests), as do its typecheck and focused type-aware lint.
There was a problem hiding this comment.
Also checked resolution from an external plugin project with no direct Cloudflare dependency. A Vite build regression reproduced the bare-specifier resolution failure, so the virtual module now uses the D1 entrypoint resolved relative to plugin-test via import.meta.resolve(). Final verification passes: 5 Node tests, 41 workerd tests, package typecheck, and focused type-aware lint.
| "@emdash-cms/cloudflare": patch | ||
| --- | ||
|
|
||
| Fixes sandboxed plugins on Cloudflare to use the site's configured D1, Hyperdrive, or Durable Objects SQL database instead of always using the `DB` D1 binding. Plugin database operations now open an event-scoped connection, so Hyperdrive connections are not reused across bridge calls. |
There was a problem hiding this comment.
[needs fixing] The PR removes the D1-only notes from database.mdx and the adapter JSDoc. It misses the same claim in docs/src/content/docs/deployment/plugin-sandbox.mdx:93-95:
The bridge that sandboxed plugins use for database access talks to the D1 binding named
DBdirectly, independent of the configured adapter. Sandboxed plugins are not available on a Hyperdrive deployment.
After this change, that statement contradicts shipped behavior and the changeset. It is also the page readers hit when setting up the sandbox runner. Remove the Aside, or replace it with a short note that the bridge uses the configured d1(), hyperdrive() or durableObjects() database. Also consider stating plainly in this changeset that sandboxed plugins now work on Hyperdrive deployments. That is the user-facing capability; "event-scoped connection" is an internal detail.
There was a problem hiding this comment.
Updated both the database-access table and the stale Aside in plugin-sandbox.mdx to describe the configured D1, Hyperdrive, or Durable Objects SQL database. The changeset now explicitly says sandboxed plugins work on Hyperdrive deployments and includes the plugin-test patch.
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
upgrade-emdash
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-ai-search
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Addressed PR review by supplying plugin-test's virtual dialect with a D1 entrypoint resolved relative to the package, covering both workerd storage integration and external-project Vite resolution with failing-before/passing-after regression tests. Corrected the remaining sandbox deployment documentation and made Hyperdrive support explicit in the changeset, including a plugin-test patch. Verified the root build, all 5 plugin-test Node tests and 41 workerd tests, package typecheck, focused type-aware lint, formatting, and zero root lint:quick diagnostics. Independent follow-up review cleared the significant findings; the pre-existing descriptor-less fallback diagnostic remains unchanged. EmDash-Bot-Case: 1198246700-issue-4745615752 EmDash-Bot-Candidate: cand-2-73edfac72c
There was a problem hiding this comment.
The approach fits the adapter architecture and #1623. The serializable DatabaseDescriptor is forwarded to the bridge props, and the bridge resolves the build-selected virtual:emdash/dialect. Each RPC opens its own database scope, forces reads to the primary, and closes the scope in finally. env.DB remains only as the fallback when no descriptor is set. Both reviewers approved and neither raised any findings. Both earlier findings are fixed at this head. @emdash-cms/plugin-test now stubs virtual:emdash/dialect, resolving the D1 entrypoint relative to the harness package. Regression tests cover Worker Loader storage access and resolution from an external consumer project. The sandbox deployment page and the changeset now describe configured-adapter support, including Hyperdrive. Both reviews were static, and neither ran the test suite.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
Fixed the CI Measure regression by restoring lazy loading for bridge database helpers and SQL. Reproduced startup failure at 28 modules/2,272,211 bytes; the starter production build now passes startup:check at 23 modules/1,818,842 bytes. Verified root build/typecheck/lint, 574 Cloudflare tests, plugin-test's 5 Node and 41 workerd tests, and 6 real SQLite/PostgreSQL adapter regressions. The core shard's three PostgreSQL setup timeouts did not reproduce: both affected files pass locally (194 tests); Tests was only the aggregate failure. Independent review found no blockers. EmDash-Bot-Case: 1198246700-issue-4745615752 EmDash-Bot-Candidate: cand-3-ba50d0c441
There was a problem hiding this comment.
The approach fits the adapter architecture and addresses #1623. The serializable DatabaseDescriptor is forwarded to the bridge props, and PluginBridge resolves the build-selected virtual:emdash/dialect. Each RPC opens its own database scope and closes it in finally. env.DB is used only as a fallback when no descriptor is set. Both reviewers approved with no findings. Both earlier findings are fixed at this head. @emdash-cms/plugin-test now stubs virtual:emdash/dialect, resolving the D1 entrypoint relative to the harness package, with Worker Loader and external-consumer regression tests. The sandbox docs and changeset now describe configured-adapter and Hyperdrive support. Both reviews were static, and neither ran the test suite.
Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.
What does this PR do?
Fixes Cloudflare sandbox plugins using
env.DBregardless of the site's database adapter. Threads the configured database descriptor through core runtime and runner into bridge props, resolves the build-selected virtual dialect, and creates a database scope for each database RPC. Hyperdrive pools close infinally; D1 and DO SQL reads use primary routing so separate plugin calls observe preceding writes. Bridge-owned connections and DO stubs are not cached across RPCs. Only the configured adapter is bundled, so D1 sites do not acquire a dependency onpg.Replaces direct D1 queries with shared core access/repository helpers and portable Kysely queries, preserving capability checks, plugin namespaces, content write fences, and the existing content-read fallback for partially migrated schemas. Removes the D1-only notes from the adapter JSDoc and deployment docs. Includes a patch changeset for both changed packages.
Runtime-owned mutations (content actions, taxonomy writes, moderation, and content creation) continue using the existing invocation-specific
PluginRuntimeBridgeRPC target, not the loopback callback fallback. That target already restores the originating request's ALS context withAsyncLocalStorage.snapshot(). A new regression test clears the request context on the plugin side and verifies that concurrent runtime callbacks still query their respective real databases. No callback connection-lifecycle change is needed on that production path.The Node/workerd runner continues using its supplied host database: its handlers run in Node, where pools are not bound to a Cloudflare event. Production Cloudflare bridge calls need separate scopes; N database bridge calls can therefore incur N connection setups with Hyperdrive. The old descriptor-less D1 fallback is retained for compatibility; this PR does not redesign standalone legacy callback APIs or their configuration diagnostics. Misconfigured D1 coalescing without sessions can still emit adapter warnings on each new scope.
Closes #1623
Type of change
Your connection to this change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI assistance disclosure
Screenshots / test output
Screenshots: Not applicable.
NODE_OPTIONS=--max-old-space-size=8192(the default heap was insufficient).A Maintainer (@ascorbic) assigned this issue to the EmDash bot.