fix(core): event-scoped DB connection for cron, plugin contexts, and media providers - #1625
Merged
Conversation
…s, and media providers Long-lived subsystems built at runtime init captured the per-isolate singleton Kysely and never consulted the request/event-scoped connection in ALS. On connection-backed adapters (Postgres over Hyperdrive) a connection is bound to the event that opened it, so the cron sweep, plugin hook contexts, and media providers failed under workerd's cross-request I/O guard on warm isolates. - Add a resolveDb closure in EmDashRuntime.create() (ALS-aware, singleton fallback) and thread it into the cron executor, the plugin context factory (via additive getDb on PluginContextFactoryOptions and pipelineFactoryOptions, so rebuildHookPipeline keeps it), and the media provider context. - CronExecutor resolves its db per tick; PluginContextFactory resolves per createContext; the local media provider builds its repository per operation. - Give the Cron Trigger sweep its own event-scoped connection: runScheduledTasks opens a request-scoped db, runs the batch under it in ALS, and closes it, gated on the adapter being connection-backed (D1/SQLite keep the singleton). Stateless adapters (D1, Node SQLite) are unchanged. Sandboxed plugins remain D1-only (the bridge DO talks to a D1 binding directly; tracked in #1623). Closes #1622
🦋 Changeset detectedLatest commit: 0fe252e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
Contributor
Scope checkThis PR changes 540 lines across 10 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 0fe252e | Jun 25 2026, 06:21 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 0fe252e | Jun 25 2026, 06:21 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 0fe252e | Jun 25 2026, 06:20 PM |
EmDashRuntime captured a SchemaRegistry over the per-isolate singleton at construction, so request handlers (content update, revision restore, media normalization) queried it through the singleton. On a connection-backed adapter (Postgres over Hyperdrive) that socket belongs to an earlier event, so on a warm isolate the schema lookup throws and handleContentUpdate's catch treats a revision-enabled collection as non-revisioned, writing draft edits to live columns. Make schemaRegistry a getter that builds against this.db (the ALS-aware getter), matching the per-call registry already used in _buildManifest. Add a regression test asserting the registry resolves the event-scoped db, not the singleton.
@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-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@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: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is the right fix for #1622. The approach is additive and backwards-compatible: instead of capturing the per-isolate singleton Kysely in long-lived subsystems, the runtime threads an ALS-aware resolver through the cron executor, the plugin context factory (via pipelineFactoryOptions so rebuildHookPipeline() preserves it), and the local media provider. Stateless adapters fall back to the singleton unchanged, and the Cron Trigger sweep opens its own event-scoped connection on connection-backed adapters and closes it when done.
I traced the changed code and the relevant call sites. The component-level tests are reasonable; they prove each subsystem resolves the connection at use-time by pointing a resolver at two isolated SQLite databases. The Hyperdrive documentation updates correctly narrow the limitation to sandboxed plugins.
One real edge case in the new cron sweep path: the finally block guards scoped.commit() but not scoped.close(). If close() throws, it will mask the original return value or exception. This is inconsistent with finishScoped / closeSafely in middleware/scoped-db.ts, which guards close on every path. It should be guarded too.
Separate pre-existing note (not a blocker for this PR): EmDashRuntime.schemaRegistry still captures the singleton db, so the request-path content handlers use the request-scoped Kysely for repository queries but the per-isolate singleton for collection metadata reads. On Hyperdrive that is the same class of cross-event risk this PR fixes elsewhere, so it may need a follow-up.
| } catch (error) { | ||
| console.error("[scheduled] request-scoped db commit failed:", error); | ||
| } | ||
| scoped.close(); |
Contributor
There was a problem hiding this comment.
[needs fixing] The finally block carefully guards scoped.commit() so a failure there cannot skip close(), but scoped.close() itself is unguarded. If a connection-backed adapter's close() ever throws synchronously, it will replace the sweep's return value or the original exception.
This contradicts the lifecycle handling in middleware/scoped-db.ts (finishScoped/closeSafely), which defends close() on both the success and error paths. Mirror that here.
Suggested change
| scoped.close(); | |
| try { | |
| scoped.commit(); | |
| } catch (error) { | |
| console.error("[scheduled] request-scoped db commit failed:", error); | |
| } | |
| try { | |
| scoped.close(); | |
| } catch (error) { | |
| console.error("[scheduled] request-scoped db close failed:", error); | |
| } |
github-actions
Bot
added
review/awaiting-author
A throw from scoped.close() in the finally block would mask the sweep's result or exception, and a throwing commit() could skip close() and leak the connection. Guard both, matching closeSafely() in middleware/scoped-db.ts.
github-actions
Bot
added
review/needs-rereview
Merged
marcusbellamyshaw-cell
pushed a commit
to Emdash-Bug-Testing/emdash
that referenced
this pull request
Jul 22, 2026
…media providers (emdash-cms#1625) * fix(core): resolve DB connection at use-time for cron, plugin contexts, and media providers Long-lived subsystems built at runtime init captured the per-isolate singleton Kysely and never consulted the request/event-scoped connection in ALS. On connection-backed adapters (Postgres over Hyperdrive) a connection is bound to the event that opened it, so the cron sweep, plugin hook contexts, and media providers failed under workerd's cross-request I/O guard on warm isolates. - Add a resolveDb closure in EmDashRuntime.create() (ALS-aware, singleton fallback) and thread it into the cron executor, the plugin context factory (via additive getDb on PluginContextFactoryOptions and pipelineFactoryOptions, so rebuildHookPipeline keeps it), and the media provider context. - CronExecutor resolves its db per tick; PluginContextFactory resolves per createContext; the local media provider builds its repository per operation. - Give the Cron Trigger sweep its own event-scoped connection: runScheduledTasks opens a request-scoped db, runs the batch under it in ALS, and closes it, gated on the adapter being connection-backed (D1/SQLite keep the singleton). Stateless adapters (D1, Node SQLite) are unchanged. Sandboxed plugins remain D1-only (the bridge DO talks to a D1 binding directly; tracked in emdash-cms#1623). Closes emdash-cms#1622 * fix(core): resolve schemaRegistry against the event-scoped db EmDashRuntime captured a SchemaRegistry over the per-isolate singleton at construction, so request handlers (content update, revision restore, media normalization) queried it through the singleton. On a connection-backed adapter (Postgres over Hyperdrive) that socket belongs to an earlier event, so on a warm isolate the schema lookup throws and handleContentUpdate's catch treats a revision-enabled collection as non-revisioned, writing draft edits to live columns. Make schemaRegistry a getter that builds against this.db (the ALS-aware getter), matching the per-call registry already used in _buildManifest. Add a regression test asserting the registry resolves the event-scoped db, not the singleton. * fix(core): guard close() in the cron sweep teardown A throw from scoped.close() in the finally block would mask the sweep's result or exception, and a throwing commit() could skip close() and leak the connection. Guard both, matching closeSafely() in middleware/scoped-db.ts.
marcusbellamyshaw-cell
pushed a commit
to Emdash-Bug-Testing/emdash
that referenced
this pull request
Jul 22, 2026
…media providers (emdash-cms#1625) * fix(core): resolve DB connection at use-time for cron, plugin contexts, and media providers Long-lived subsystems built at runtime init captured the per-isolate singleton Kysely and never consulted the request/event-scoped connection in ALS. On connection-backed adapters (Postgres over Hyperdrive) a connection is bound to the event that opened it, so the cron sweep, plugin hook contexts, and media providers failed under workerd's cross-request I/O guard on warm isolates. - Add a resolveDb closure in EmDashRuntime.create() (ALS-aware, singleton fallback) and thread it into the cron executor, the plugin context factory (via additive getDb on PluginContextFactoryOptions and pipelineFactoryOptions, so rebuildHookPipeline keeps it), and the media provider context. - CronExecutor resolves its db per tick; PluginContextFactory resolves per createContext; the local media provider builds its repository per operation. - Give the Cron Trigger sweep its own event-scoped connection: runScheduledTasks opens a request-scoped db, runs the batch under it in ALS, and closes it, gated on the adapter being connection-backed (D1/SQLite keep the singleton). Stateless adapters (D1, Node SQLite) are unchanged. Sandboxed plugins remain D1-only (the bridge DO talks to a D1 binding directly; tracked in emdash-cms#1623). Closes emdash-cms#1622 * fix(core): resolve schemaRegistry against the event-scoped db EmDashRuntime captured a SchemaRegistry over the per-isolate singleton at construction, so request handlers (content update, revision restore, media normalization) queried it through the singleton. On a connection-backed adapter (Postgres over Hyperdrive) that socket belongs to an earlier event, so on a warm isolate the schema lookup throws and handleContentUpdate's catch treats a revision-enabled collection as non-revisioned, writing draft edits to live columns. Make schemaRegistry a getter that builds against this.db (the ALS-aware getter), matching the per-call registry already used in _buildManifest. Add a regression test asserting the registry resolves the event-scoped db, not the singleton. * fix(core): guard close() in the cron sweep teardown A throw from scoped.close() in the finally block would mask the sweep's result or exception, and a throwing commit() could skip close() and leak the connection. Guard both, matching closeSafely() in middleware/scoped-db.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes scheduled publishing, plugin cron, and database-querying plugin hooks work on connection-backed adapters (Postgres over Cloudflare Hyperdrive), and removes the corresponding "request path only" limitation from the
hyperdrive()adapter.On Workers a database connection is bound to the event that opened it; workerd rejects reuse from a later event. #1614 fixed the HTTP request path (per-request Kysely in ALS; the runtime
dbgetter prefers it). But several long-lived subsystems captured the per-isolate singleton Kysely at runtime init and never consulted ALS, so on a warm isolate they reused a socket opened by an earlier request:content/media/users/cronaccess),This threads an event-scoped DB resolver through those subsystems so each resolves the current connection at use-time:
EmDashRuntime.create()builds aresolveDb()closure (ALS-aware, singleton fallback) and passes it to the cron executor, the plugin context factory (via an additivegetDb?onPluginContextFactoryOptions, carried inpipelineFactoryOptionssorebuildHookPipeline()keeps it), and the media provider context.CronExecutorresolves its db per tick;PluginContextFactoryresolves percreateContext(); the local media provider builds its repository per operation.runScheduledTasks) opens its own event-scoped connection, runs the batch under it in ALS, and closes it — gated on the adapter being connection-backed (it exposesclose()), so D1 / Node SQLite keep using the singleton unchanged.Per a second opinion, this uses explicit resolver threading (with additive, backwards-compatible
getDb?on the externalMediaProviderContextcontract) rather than a transparent Kysely proxy.Stateless adapters (D1, Node SQLite) are unchanged — they set no ALS db on most paths, so the resolver falls back to the singleton, and
pnpm query-countsmatches the SQLite snapshot.Out of scope: sandboxed plugins remain D1-only — the sandbox bridge Durable Object talks to a D1 binding directly, independent of the configured adapter. That's a pre-existing constraint unrelated to connection scoping; filed as #1623. The docs now say so precisely.
Closes #1622
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change) — fullemdashsuite (4138) + newevent-scoped-dbtests (7) passpnpm formathas been runtests/integration/plugins/event-scoped-db.test.tscovering cron, plugin context, media provider resolution, and the pipeline-rebuild pathemdashpatch (and updates the unreleased@emdash-cms/cloudflareadapter changeset to drop the now-fixed limitation)AI-generated code disclosure
rebuildHookPipelineregression, now fixed and tested) and a GPT-5.5 second opinion on the resolver-vs-proxy approach.Screenshots / test output
Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
fix/event-scoped-db-1622. Updated automatically when the playground redeploys.