fix(core): defer afterDelete and afterUnpublish hooks so Workers doesn't cancel them#1588
Conversation
…n't cancel them runAfterDeleteHooks and runAfterUnpublishHooks dispatched plugin hooks fire-and-forget. On Cloudflare Workers a promise not handed to waitUntil is canceled when the response returns, so plugin cleanup work (R2 deletes, search-index removal) is killed mid-flight -- and a half-completed plugin-storage write can wedge the isolate and hang later admin requests. Wrap both dispatchers in after(), exactly as runAfterSaveHooks and runAfterPublishHooks already do. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 7259317 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 |
@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://cdn.statically.io/img/avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right fix for the right problem. afterDelete and afterUnpublish hook dispatch was the only remaining lifecycle cleanup path that dispatched on a bare promise, so on Cloudflare Workers it could be canceled as soon as the HTTP response went out. Wrapping both in after() matches the existing afterSave / afterPublish pattern exactly, which is the documented way to defer bookkeeping past the response and hand the lifetime to waitUntil.
I read the full diff, the after() implementation, the existing runAfterSaveHooks / runAfterPublishHooks implementations, and the new test. The code changes mirror the established pattern closely: trusted hook awaited with try/catch, sandboxed plugins collected into tasks and awaited via Promise.allSettled, and the entry-point comments updated to reflect deferred execution. The test directly verifies the scheduling contract by mocking after() and asserting the hook handler is not run inline but is flushed from the deferred queue, and it fails on the pre-fix behavior.
No blocking issues, no AGENTS.md convention violations, no SQL/Tailwind/Lingui concerns, and the changeset is present and correctly names the emdash package. LGTM.
There was a problem hiding this comment.
Pull request overview
This PR fixes a Cloudflare Workers runtime-lifetime bug where content:afterDelete and content:afterUnpublish plugin hooks could be canceled when the HTTP response completes, by deferring those hook dispatchers through the existing after() mechanism (which integrates with Workers waitUntil when available).
Changes:
- Wrap
runAfterDeleteHooksandrunAfterUnpublishHooksinafter()and align their trusted/sandboxed hook execution structure with the existingafterSave/afterPublishimplementations. - Add an integration test that mocks
after()to assert delete/unpublish hook work is scheduled (deferred) rather than executed inline. - Add a patch changeset documenting the Workers-specific fix and user impact.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/core/src/emdash-runtime.ts | Defers after-delete and after-unpublish hook dispatch via after() and awaits sandboxed hook tasks with Promise.allSettled inside the deferred work. |
| packages/core/tests/integration/runtime/after-hooks-deferred.test.ts | Adds coverage that pins the “deferred via after()” contract for delete and unpublish lifecycle hooks. |
| .changeset/defer-delete-unpublish-hooks.md | Documents the patch-level fix for dropped lifecycle hooks on Cloudflare Workers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Re-review of #1588. This is the same fix I previously approved: runAfterDeleteHooks and runAfterUnpublishHooks now wrap their hook dispatch in after(), mirroring the existing runAfterSaveHooks / runAfterPublishHooks pattern so Cloudflare Workers’ waitUntil can keep the work alive past the HTTP response.
I re-read the diff, the full emdash-runtime.ts implementation around all four lifecycle dispatchers, the after() implementation, and the new after-hooks-deferred.test.ts. The changes remain consistent with the established deferred pattern: trusted hooks are awaited with try/catch, sandboxed plugins are collected into tasks and awaited via Promise.allSettled, and the entry-point comments now correctly say "deferred past the response via after()". I also checked that no other bare-promise hook dispatches remain in emdash-runtime.ts.
The test directly verifies the scheduling contract by mocking after() and asserting the handlers are not run inline but are flushed from the deferred queue. The changeset is present and names the emdash package.
No blocking issues and no AGENTS.md violations. The prior approval stands; LGTM.
What does this PR do?
runAfterDeleteHooksandrunAfterUnpublishHooksdispatch plugin hooks fire-and-forget (a bare promise with.catch()), unlikerunAfterSaveHooksandrunAfterPublishHooks, which already wrap their dispatch inafter().On Cloudflare Workers a promise that isn't handed to the host's lifetime extender (
waitUntil, whichafter()wraps) is canceled the moment the HTTP response is returned. So any real work a plugin does incontent:afterDelete/content:afterUnpublish— deleting uploaded files from R2, removing search-index entries, plugin-storage writes — gets killed mid-flight. In practice a half-completed plugin-storage write duringafterDeletecould wedge the storage backend for the isolate and hang every subsequent authenticated admin request (observed as the admin "Move to trash" action freezing the content list).The fix wraps both dispatchers in
after(), identical to the existingafterSave/afterPublishhandling — trusted hook in a try/catch, sandboxed plugins collected intotasksand awaited viaPromise.allSettled.Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpasses (0 diagnostics)pnpm testpasses (full core suite: 4071 passed; the only failures are pre-existing Windows-env issues — file-URL/tar/file-SQLite — that fail identically on a cleanmain)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
New test
tests/integration/runtime/after-hooks-deferred.test.tsdrives a real soft-delete and unpublish throughEmDashRuntimeand asserts the hook work is scheduled viaafter()rather than run inline-and-abandoned. It fails on stockmain(the handler runs inline) and passes with this change.