feat(core): add content schedule hooks - #1659
Conversation
🦋 Changeset detectedLatest commit: 38c9c90 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 |
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
Scope checkThis PR changes 754 lines across 22 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. |
There was a problem hiding this comment.
Pull request overview
Adds new plugin lifecycle hooks for content scheduling/unscheduling so plugins can react when entries transition into/out of the scheduled state, and updates docs/templates/tests accordingly.
Changes:
- Introduces
content:afterScheduleandcontent:afterUnschedulehook types, manifest support, and hook pipeline execution. - Fires the new hooks from
EmDashRuntimeafter successful schedule/unschedule operations (deferred viaafter()). - Adds unit tests and updates hook reference docs across templates and the docs site.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| templates/starter/.agents/skills/creating-plugins/references/hooks.md | Documents the new schedule/unschedule hooks in the starter template plugin-hook reference. |
| templates/starter-cloudflare/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the Cloudflare starter template. |
| templates/portfolio/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the portfolio template. |
| templates/portfolio-cloudflare/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the Cloudflare portfolio template. |
| templates/marketing/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the marketing template. |
| templates/marketing-cloudflare/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the Cloudflare marketing template. |
| templates/blog/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the blog template. |
| templates/blog-cloudflare/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the Cloudflare blog template. |
| templates/blank/.agents/skills/creating-plugins/references/hooks.md | Same documentation update for the blank template. |
| skills/creating-plugins/references/hooks.md | Updates the canonical plugin-creation hook reference to include the new hooks. |
| packages/core/tests/unit/plugins/schedule-hooks.test.ts | Adds focused tests ensuring the runtime fires content:afterSchedule/content:afterUnschedule. |
| packages/core/tests/unit/plugins/hooks.test.ts | Extends HookPipeline tests to register/enforce capability requirements for the new hook names. |
| packages/core/src/plugins/types.ts | Adds handler types and hook interface entries for the new schedule/unschedule hooks. |
| packages/core/src/plugins/manifest-schema.ts | Allows the new hook names in manifest validation (HOOK_NAMES). |
| packages/core/src/plugins/manager.ts | Exposes runContentAfterSchedule / runContentAfterUnschedule on PluginManager. |
| packages/core/src/plugins/index.ts | Re-exports the new handler types from the public plugin API surface. |
| packages/core/src/plugins/hooks.ts | Registers, capability-gates, and executes the new hook handlers in HookPipeline. |
| packages/core/src/plugin-types.ts | Updates the hook handler name→type mapping to include the new hooks. |
| packages/core/src/emdash-runtime.ts | Fires new hooks after successful schedule/unschedule operations using deferred execution. |
| docs/src/content/docs/reference/hooks.mdx | Adds the new hooks to the public “Hook Reference” docs page. |
| docs/src/content/docs/plugins/creating-plugins/hooks.mdx | Documents the new hooks in the “Creating plugins” docs and updates the quick reference table. |
| .changeset/content-schedule-hooks.md | Adds a minor-version changeset noting the new hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `content:afterDelete` | After content is deleted | Nothing | No | | ||
| | `content:afterSchedule` | After content is scheduled | Nothing | No | | ||
| | `content:afterUnschedule` | After content is unscheduled | Nothing | No | | ||
| | `media:beforeUpload` | Before file is uploaded | File metadata | No | | ||
| | `media:afterUpload` | After file is uploaded | Nothing | No | |
There was a problem hiding this comment.
I noticed this, but do not want to mix concerns in the 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: |
|
The formatting issue would reformat every table in the references, which would conflict with other PRs, so perhaps it is better to leave it to a later PR? |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
There are a lot of conflicts here after merging your other PR. Can you also see why the query count jumps by so much? |
There was a problem hiding this comment.
This PR adds content:afterSchedule and content:afterUnschedule hooks so plugins can react to manual scheduling/unscheduling actions. The approach is idiomatic: it extends the existing publish/unpublish hook family, registers the new hooks with the same content:read capability gate, wires them into EmDashRuntime.handleContentSchedule/handleContentUnschedule via the deferred after() helper, and adds runtime tests.
The implementation appears correct and safe: hooks are fire-and-forget, sandboxed plugin errors are isolated, and the schedule/unschedule result is returned before hooks run. Docs, the manifest schema, plugin types, and the changeset are all updated. I found no logic bugs, SQL issues, authorization gaps, or locale-filtering concerns in the changed code.
The main blocking convention issue is that this is a new feature without a linked maintainer-approved Discussion, which AGENTS.md/CONTRIBUTING requires for features. Beyond that, there are some maintainability nits: near-duplicate implementations in EmDashRuntime and HookPipeline, a test helper that relies on timer ordering rather than explicit synchronization, and a reused event-type name that is semantically misleading for schedule/unschedule.
Findings
-
[needs fixing]
.changeset/content-schedule-hooks.md:5This changeset introduces a new user-facing feature, but the PR links the feature request to an internal chat rather than a maintainer-approved Discussion in the Ideas category. AGENTS.md and
CONTRIBUTING.mdrequire an approved Discussion for features before merge. Please open/link the approved Discussion so the design, naming, and scope have maintainer sign-off. -
[suggestion]
packages/core/src/plugins/types.ts:728ContentPublishStateChangeEventis reused forcontent:afterSchedule/content:afterUnschedule, but scheduling/unscheduling are not publish-state changes. The comment also only mentions publish/unpublish. Consider introducing an alias such asContentScheduleStateChangeEvent(with the same shape) and using it for the schedule hooks, so the public API name matches its semantics. -
[suggestion]
packages/core/src/emdash-runtime.ts:3227-3288runAfterScheduleHooksandrunAfterUnscheduleHooksare nearly identical copies of each other and ofrunAfterPublishHooks. Consider extracting a shared helper such asrunDeferredContentHook(name, runner, content, collection)to keep the error handling, sandboxed-plugin dispatch, and lifetime management in one place and avoid drift if one path is patched later. -
[suggestion]
packages/core/src/plugins/hooks.ts:743-822runContentAfterScheduleandrunContentAfterUnscheduleduplicate the same loop, event construction, and result/error handling. A private generic such asrunContentStateChangeHook(name, content, collection)would remove the duplication and make adding future state-change hooks a one-line addition. -
[suggestion]
packages/core/tests/unit/plugins/schedule-hooks.test.ts:13flushDeferredHooks()relies onsetTimeout(0)to drain microtasks created byafter(). This works today becauseafter()schedules its callback viaPromise.resolve().then(), but it is fragile ifafter()ever uses macrotasks or if a hook schedules its own timers. A more robust test would await the actual deferred promise; if that isn't exposed, a comment explaining the assumption would help future maintainers.
a7187bf to
971418f
Compare
There was a problem hiding this comment.
This is a re-review after the previous round. The PR still adds content:afterSchedule and content:afterUnschedule hooks in an idiomatic way: the runtime defers execution with after(), sandboxed-plugin errors are isolated, and the schedule/unschedule result is returned before hooks run. One prior maintainability nit has been addressed — HookPipeline now uses a shared runContentPublishStateHook helper instead of two near-duplicate methods.
I checked the updated diff and related files statically. I did not find any new logic bugs, SQL issues, authorization gaps, or missing locale filters. The remaining blockers from the previous review are still open:
- The feature links to an internal chat rather than a maintainer-approved Discussion, which AGENTS.md/CONTRIBUTING.md requires for new features.
- The public event type still says
ContentPublishStateChangeEvent(and its comment still says "publish or unpublish"), even though schedule/unschedule are not publish-state changes. EmDashRuntimestill has near-duplicaterunAfterScheduleHooks/runAfterUnscheduleHelpers.- The test helper
flushDeferredHooks()still relies on timer ordering rather than the deferred promise.
Verdict: comment — the code is functionally sound, but the missing approved Discussion is a process blocker that needs resolving before merge.
Findings
-
[needs fixing]
.changeset/content-schedule-hooks.md:5This changeset introduces a new user-facing feature, but the PR links the feature request to an internal chat rather than a maintainer-approved Discussion in the Ideas category. AGENTS.md and
CONTRIBUTING.mdrequire an approved Discussion for features before merge. Please open/link the approved Discussion so the design, naming, and scope have maintainer sign-off. -
[suggestion]
packages/core/src/plugins/types.ts:730ContentPublishStateChangeEventis now used forcontent:afterScheduleandcontent:afterUnschedule, but scheduling/unscheduling are not publish-state changes and the interface comment still says it's fired only after publish or unpublish. Consider introducing an alias such asContentScheduleStateChangeEvent(with the same shape) and updating the doc comment so the public API name matches the new semantics. -
[suggestion]
packages/core/src/emdash-runtime.ts:3420-3479runAfterScheduleHooksandrunAfterUnscheduleHooksare nearly identical copies of each other and ofrunAfterPublishHooks/runAfterRestoreHooks. This follows the existing pattern, but adding two more copies increases the surface for drift. Consider extracting a shared private helper such asrunDeferredContentHook(name, content, collection)so trusted-plugin dispatch, sandboxed-plugin dispatch, and error handling live in one place. -
[suggestion]
packages/core/tests/unit/plugins/schedule-hooks.test.ts:13-15flushDeferredHooks()relies onsetTimeout(0)to drain the microtasks created byafter(). This works today becauseafter()schedules viaPromise.resolve().then(), but it is fragile ifafter()ever uses macrotasks or if a hook's timeout races. A more robust test would await the actual deferred promise; if that isn't exposed, add a comment explaining the assumption.
There was a problem hiding this comment.
This PR adds content:afterSchedule and content:afterUnschedule hooks so plugins can react to explicit schedule/unschedule actions. The approach is still idiomatic: it extends the existing afterPublish hook family, gates the new hooks with content:read, defers execution via after(), and keeps the HTTP route envelope unchanged by calling the runtime wrappers.
Since the last review, the author has addressed the HookPipeline duplication for schedule/unschedule by introducing a shared private helper (runContentPublishStateHook). The implementation remains safe: hooks are fire-and-forget, errors in individual plugins are isolated, and the schedule/unschedule result is returned before deferred hooks run. Docs, the manifest schema, plugin types, and the changeset are all updated.
Still outstanding from the prior review:
- The feature is still missing a maintainer-approved GitHub Discussion; the PR description links only an internal chat. AGENTS.md and CONTRIBUTING.md require an approved Discussion for new features.
ContentPublishStateChangeEventis reused for schedule/unschedule, and its doc comment still only mentions publish/unpublish, so the public API name is semantically misleading for the new hooks.EmDashRuntimestill has near-identicalrunAfterScheduleHooks/runAfterUnscheduleHooksimplementations that could be collapsed into a shared helper.flushDeferredHooks()in the new test relies on a singlesetTimeout(0)to drain deferred work, which is fragile ifafter()'s scheduling strategy changes.
None of these are runtime bugs, but the missing approved Discussion is a required-process item that should be resolved before merge.
Findings
-
[needs fixing]
.changeset/content-schedule-hooks.md:5This changeset introduces a new user-facing feature, but the PR still links the feature request to an internal chat rather than a maintainer-approved Discussion in the Ideas category. AGENTS.md and
CONTRIBUTING.mdrequire an approved Discussion for features before merge. Please open/link the approved Discussion so the design, naming, and scope have maintainer sign-off. -
[suggestion]
packages/core/src/plugins/types.ts:727-732ContentPublishStateChangeEventis now reused forcontent:afterRestore,content:afterSchedule, andcontent:afterUnschedule, but its name and comment only mention publish/unpublish. Scheduling/unscheduling are not publish-state changes, so the public type name is semantically wrong for the new hooks.Consider either introducing an alias such as
ContentScheduleStateChangeEvent(with the same shape) and using it for the schedule hooks, or at least update the comment to cover all consumers:/** * Content publish state change hook event (fired after publish, unpublish, * restore, schedule, or unschedule). */ -
[suggestion]
packages/core/src/emdash-runtime.ts:3420-3480runAfterScheduleHooksandrunAfterUnscheduleHooksare near-identical copies; only the hook name differs. Collapsing them (and ideallyrunAfterPublishHooks/runAfterRestoreHooks) into a single private helper such asrunDeferredContentHook(name, content, collection)would remove the duplication and prevent future patches from diverging across the four state-change paths. -
[suggestion]
packages/core/tests/unit/plugins/schedule-hooks.test.ts:13flushDeferredHooks()relies onsetTimeout(0)to drain the microtasks created byafter(). This works today becauseafter()resolves via a Promise microtask, but it is fragile ifafter()ever useswaitUntil/macrotasks or if a hook schedules its own timers.A more robust test would await the actual deferred promise if
after()exposes it; if not, add a comment documenting the assumption so future maintainers know what the helper is depending on.
There was a problem hiding this comment.
This PR adds content:afterSchedule and content:afterUnschedule hooks so plugins can react to explicit schedule/unschedule actions, and since the last review the implementation is cleaner and more consistent.
The approach is idiomatic: it extends the existing afterPublish hook family, reuses a single event shape, gates the new hooks behind content:read, and keeps the route envelope unchanged by calling the runtime wrappers.
What changed for the better since the prior review:
- The
HookPipelineduplication for publish/unpublish/restore/schedule/unschedule is now collapsed into a single privaterunContentStateChangeHook. EmDashRuntimesimilarly unifies the four state-change paths through a newrunDeferredContentHookhelper, sorunAfterScheduleHooks/runAfterUnscheduleHooksare no longer copy-pasted.ContentPublishStateChangeEventhas been renamed toContentStateChangeEvent, with semantic aliases (ContentPublishStateChangeEvent,ContentRestoreStateChangeEvent,ContentScheduleStateChangeEvent) so each handler type reads correctly.- The new tests mock
after()directly and flush captured tasks, which is more robust than the previoussetTimeout(0)helper. - The manifest schema, exports, docs, and changeset are all updated.
One real issue remains: the PR still does not link a maintainer-approved GitHub Discussion in the Ideas category; it only references an internal chat. AGENTS.md and CONTRIBUTING.md require an approved Discussion for new features before merge.
I also noticed that the refactor moved content:afterUnpublish onto after() (it was previously synchronous-fire-and-forget). That is actually a positive consistency change with publish/restore, but it is a behavioral change that is worth being aware of when merging.
| "emdash": minor | ||
| --- | ||
|
|
||
| Adds content scheduling hooks for plugins to react when entries are scheduled or unscheduled. |
There was a problem hiding this comment.
[needs fixing] This changeset introduces a new user-facing feature, but the PR description still links the feature request to an internal chat rather than a maintainer-approved Discussion in the Ideas category. AGENTS.md and CONTRIBUTING.md require an approved Discussion for features before merge. Please open or link the approved Discussion so the design, naming, and scope have maintainer sign-off.
There was a problem hiding this comment.
Already discussed internally.
* feat(core): add content schedule hooks * Address schedule hook review --------- Co-authored-by: Matt Kane <mkane@cloudflare.com>
* feat(core): add content schedule hooks * Address schedule hook review --------- Co-authored-by: Matt Kane <mkane@cloudflare.com>
What does this PR do?
Adds hooks for content scheduling/unscheduling so plugins can stay in sync and know what posts are visible or not.
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output