fix: remove manifest cache; build admin-only, per-request#884
Conversation
Closes #776, #873, #876, #877. The admin manifest (collection schemas, plugins, taxonomies) was loaded on every request via global middleware and cached per worker isolate. Public anonymous traffic carried the cost without ever reading it, and the cross-isolate cache produced four flavors of the same staleness bug — Workers keeps multiple warm isolates per region with no fan-out primitive, so a mutation on isolate A wasn't visible to isolate B until A was recycled. Fix: stop caching, stop loading on public requests. - Middleware no longer pre-loads the manifest. The three admin consumers — the manifest endpoint and the two WordPress import routes — call `await emdash.getManifest()` on demand. - `EmDashRuntime.getManifest()` rebuilds from the live database on every call. Within a single request, `requestCached` deduplicates concurrent callers. - `SchemaRegistry.listCollectionsWithFields()` does the build in two queries flat (one for collections, one for the fields of every returned collection) instead of N+1 — the cost the cache was hiding. The cleanup removes a chunk of machinery whose only job was working around the staleness it created: `_cachedManifest`, `_manifestPromise`, `_manifestCacheKey`, the `emdash:manifest_cache` options row, the post-migration cleanup, and the `invalidateManifest()` method itself. The 14 callers of `invalidateManifest()` either move to `invalidateUrlPatternCache()` (when they actually mutate URL patterns: collection create/update/delete, WP-import collection creation) or drop the call entirely (field/taxonomy/plugin mutations don't affect URL patterns). Net diff: -171 lines.
- Remove emdashManifest from published locals.d.ts ambient type
(the runtime stopped setting it, but the public type still required
it; user sites reading Astro.locals.emdashManifest would type-check
cleanly and crash at runtime).
- Move await emdash.getManifest() inside the try/catch in both WP
execute routes so DB errors during manifest build return the
standard { error: { code, message } } envelope via handleError().
- Replace stale invalidateManifest mock with invalidateUrlPatternCache
in MCP authorization test.
🦋 Changeset detectedLatest commit: bc9a3e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-i18n | bc9a3e6 | May 01 2026, 12:31 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-perf-coordinator | bc9a3e6 | May 01 2026, 12:30 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | bc9a3e6 | May 01 2026, 12:31 PM |
|
/review |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | bc9a3e6 | May 01 2026, 12:32 PM |
Scope checkThis PR changes 710 lines across 24 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-cache | bc9a3e6 | May 01 2026, 12:32 PM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@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-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
Pull request overview
Removes the worker-isolate + persisted admin manifest cache and switches the admin manifest to be built on demand (admin-only) from the live database, avoiding cross-isolate staleness issues on Cloudflare Workers while improving manifest build efficiency.
Changes:
- Stop pre-loading/storing
locals.emdashManifestin middleware; admin consumers now callawait emdash.getManifest()on demand. - Make
EmDashRuntime.getManifest()rebuild from the live DB each call (deduped per request viarequestCached), and removeinvalidateManifest()from runtime/handlers. - Add
SchemaRegistry.listCollectionsWithFields()to avoid the previous N+1 pattern when building the manifest; update tests and types accordingly.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/emdash-runtime.ts | Removes manifest caching/invalidation machinery; rebuilds manifest from live DB and uses requestCached for per-request dedupe. |
| packages/core/src/schema/registry.ts | Adds listCollectionsWithFields() to fetch collections + fields without N+1. |
| packages/core/src/astro/middleware.ts | Stops preloading manifest on every request; exposes getManifest() + invalidateUrlPatternCache() on locals.emdash. |
| packages/core/src/astro/middleware/auth.ts | Updates App.Locals typing by removing emdashManifest. |
| packages/core/src/astro/types.ts | Updates EmDashHandlers surface: remove invalidateManifest, add getManifest + invalidateUrlPatternCache. |
| packages/core/src/astro/routes/api/manifest.ts | Builds manifest lazily via emdash.getManifest() instead of reading locals.emdashManifest. |
| packages/core/src/astro/routes/api/import/wordpress/prepare.ts | Switches invalidation from manifest cache to URL pattern cache when new collections are created. |
| packages/core/src/astro/routes/api/import/wordpress/execute.ts | Loads manifest on demand inside the handler. |
| packages/core/src/astro/routes/api/import/wordpress-plugin/execute.ts | Loads manifest on demand inside the handler. |
| packages/core/src/astro/routes/api/schema/collections/index.ts | Uses invalidateUrlPatternCache() instead of manifest invalidation after collection creation. |
| packages/core/src/astro/routes/api/schema/collections/[slug]/index.ts | Uses invalidateUrlPatternCache() instead of manifest invalidation after collection update/delete. |
| packages/core/src/astro/routes/api/schema/collections/[slug]/fields/index.ts | Drops manifest invalidation after field create (manifest is rebuilt per call now). |
| packages/core/src/astro/routes/api/schema/collections/[slug]/fields/[fieldSlug].ts | Drops manifest invalidation after field update/delete. |
| packages/core/src/astro/routes/api/schema/collections/[slug]/fields/reorder.ts | Drops manifest invalidation after field reorder. |
| packages/core/src/astro/routes/api/taxonomies/index.ts | Drops manifest invalidation after taxonomy creation. |
| packages/core/src/mcp/server.ts | Replaces collection-level invalidation with invalidateUrlPatternCache(); removes field-level invalidation. |
| packages/core/locals.d.ts | Removes published locals.emdashManifest ambient type. |
| packages/core/tests/unit/runtime/manifest-build.test.ts | New unit tests pin the “always fresh” manifest build behavior and field-kind correctness (incl. json). |
| packages/core/tests/unit/runtime/invalidate-manifest.test.ts | Removes obsolete tests for invalidateManifest() persistence semantics. |
| packages/core/tests/unit/astro/manifest-route.test.ts | Updates manifest route tests to mock getManifest() instead of locals.emdashManifest. |
| packages/core/tests/unit/import/wp-prepare-invalidate.test.ts | Updates regression test to assert URL pattern cache invalidation behavior. |
| packages/core/tests/unit/mcp/authorization.test.ts | Updates MCP handler mock shape (invalidate URL pattern cache). |
| packages/core/tests/utils/mcp-runtime.ts | Updates MCP runtime handler wiring to include getManifest + invalidateUrlPatternCache. |
| .changeset/admin-only-manifest-no-cache.md | Documents the change and breaking surface updates via changeset. |
Comments suppressed due to low confidence (1)
packages/core/src/emdash-runtime.ts:980
- The comment says deleting the old persisted manifest cache row “lets the options table shrink without requiring a migration”, but the code only attempts the delete when migrations are applied. On upgrades where no migrations run, the large legacy row will remain indefinitely. Either try deleting unconditionally (best-effort, caught) or clarify the comment to match the opportunistic behavior.
// Best-effort cleanup of the persisted manifest cache row written by
// older versions. Always rebuilds from live DB now, so the row is
// dead weight; deleting it lets the options table shrink without
// requiring a migration.
if (applied.length > 0) {
try {
const options = new OptionsRepository(db);
await options.delete("emdash:manifest_cache");
} catch {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * List every collection together with its fields in two queries (one for | ||
| * collections, one for the fields of every returned collection), instead | ||
| * of the N+1 pattern of `listCollections` + per-collection `listFields`. | ||
| * | ||
| * Used by the manifest build, which previously paid N+1 round-trips on | ||
| * every admin request. Each round-trip costs ~80–150ms against the D1 | ||
| * primary on a busy link, so a 10-collection site spent ~1 s rebuilding | ||
| * a manifest that is now built fresh per admin request (no cache). | ||
| */ | ||
| async listCollectionsWithFields(): Promise<CollectionWithFields[]> { | ||
| const collectionRows = await this.db | ||
| .selectFrom("_emdash_collections") | ||
| .selectAll() | ||
| .orderBy("slug", "asc") | ||
| .execute(); | ||
|
|
||
| if (collectionRows.length === 0) return []; | ||
|
|
||
| const fieldsByCollection = new Map<string, Field[]>(); | ||
| // Chunk to stay under D1's bound-parameter limit. Typical sites have | ||
| // well under SQL_BATCH_SIZE collections, so this is a single query. | ||
| for (const idChunk of chunks( | ||
| collectionRows.map((c) => c.id), | ||
| SQL_BATCH_SIZE, | ||
| )) { |
| * Two queries flat (one for collections, one for the fields of every | ||
| * collection) via `listCollectionsWithFields()`, regardless of how | ||
| * many collections the site has. |
| The admin manifest (collection schemas, plugins, taxonomies) is built fresh from the live database on every admin request via two queries (`SchemaRegistry.listCollectionsWithFields()`), deduplicated within a single request by `requestCached`. Logged-out / public requests no longer touch it at all — the global middleware no longer pre-loads `locals.emdashManifest`. Admin routes that need it call `await emdash.getManifest()`. | ||
|
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adversarial pass on the manifest-cache removal. The change is sound — the staleness bug class genuinely goes away by elimination, the two-query rebuild is the right tradeoff for the post-cache cost, and the breaking-change story is documented in the changeset and reflected in the published locals.d.ts. Two small notes inline; neither blocks.
| // older versions. Always rebuilds from live DB now, so the row is | ||
| // dead weight; deleting it lets the options table shrink without | ||
| // requiring a migration. | ||
| if (applied.length > 0) { |
There was a problem hiding this comment.
Nit (low): the dead-weight emdash:manifest_cache row only gets cleaned up when there are pending migrations. Existing installs upgrading to this version that hit no new migrations on the upgrade boot will keep the row indefinitely. Functionally harmless — nothing reads it now — but it defeats the stated intent ("deleting it lets the options table shrink") for the most common upgrade path.
Either move the delete out from under the if (applied.length > 0) guard, or guard on a one-shot "already cleaned" flag in options so it runs once on the next cold boot regardless of migration activity.
| // Manifest is built fresh from the live database per admin request. | ||
| // `requestCached` inside `getManifest` dedupes if multiple consumers | ||
| // share the request. | ||
| const emdashManifest = emdash ? await emdash.getManifest() : null; |
There was a problem hiding this comment.
Minor inconsistency with the adversarial-review fix on the WP execute routes. Those moved await emdash.getManifest() inside a try/catch so a failure during manifest build returns the standard apiError envelope. Here the call is naked — if _buildManifest ever throws (it currently swallows the DB query error internally, so this is theoretical), the route returns an unstructured 500 instead of the { error: { code, message } } shape the admin client expects.
In practice safe today because _buildManifest has its own try/catch around the only DB call. Worth tightening for consistency, since any future code added to getManifest() (e.g. plugin manifest loading, marketplace lookup) wouldn't be covered by that internal swallow.
|
Review posted successfully at #884 (review). Posted a review with two non-blocking notes:
Overall the change is sound — the staleness bug class is eliminated by removing the cache, the two-query rebuild is the right tradeoff for the post-cache cost, and the breaking-change story is well documented. |
- emdash-runtime.ts: move emdash:manifest_cache cleanup out from under
the applied.length > 0 guard so existing installs upgrading without
pending migrations still shrink the options table. Gated on a
one-shot emdash:manifest_cache_cleaned flag so the delete only runs
once per install, not on every isolate cold boot.
- api/manifest.ts: wrap the route body in try/catch so any future
DB-touching addition to getManifest() returns the standard
{ error: { code, message } } envelope, matching the WP execute
routes. Currently safe because _buildManifest swallows DB errors
internally, but inconsistent.
- registry.ts, emdash-runtime.ts, changeset: replace 'two queries
flat' wording with 'O(1) query shapes, chunked at SQL_BATCH_SIZE',
which is what listCollectionsWithFields actually does. Two queries
in practice for typical sites; bounded constant factor on larger
ones; never N+1.
github-actions
Bot
added
the
query-count changed
Query-count snapshot changes8 routes changed, total Δ +8 queries. D1
Comparing snapshot files between base and head. Updated automatically on each push. |
The cleanup added in 57d92c4 gated deletion on a persistent `emdash:manifest_cache_cleaned` flag so the delete only ran once per install. But checking the flag was itself a query: `options.get()` on every isolate cold boot, costing +1 query for every cold request forever (visible in query-counts as cold +1 across all routes in 23bfdce). The trade isn't worth it. The legacy `emdash:manifest_cache` row is a few hundred bytes of dead weight in the options table on installs that ever wrote it. It is never on any read path (the runtime no longer reads or writes it). A leftover row costs nothing; a per-cold- boot lookup costs every cold request forever. Drop the cleanup entirely. Add a comment explaining why the row is intentionally left alone. Revert the d1 query-count snapshot to its pre-23bfdce7 baseline. 3088 tests pass. lint:json clean. typecheck clean.
What does this PR do?
Removes the worker-isolate manifest cache and stops loading the admin manifest on public requests.
The admin manifest (collection schemas, plugins, taxonomies) was previously loaded on every request via global middleware and cached per worker isolate, keyed by an
emdash:manifest_cacherow in_emdash_options. Public anonymous traffic carried the cost without ever reading it, and the cross-isolate cache produced four flavors of the same staleness bug: Workers keeps multiple warm isolates per region with no fan-out primitive, so a mutation on isolate A wasn't visible to isolate B until A was recycled.Fix: stop caching, stop loading on public requests.
api/manifest.tsand the two WordPress import routes) callawait emdash.getManifest()on demand.EmDashRuntime.getManifest()rebuilds from the live database on every call. Within a single request,requestCacheddeduplicates concurrent callers.SchemaRegistry.listCollectionsWithFields()does the build in two queries flat (one for collections, one for the fields of every returned collection) instead of N+1 — the cost the cache was hiding.The cleanup removes a chunk of machinery whose only job was working around the staleness it created:
_cachedManifest,_manifestPromise,_manifestCacheKey, theemdash:manifest_cacheoptions row, the post-migration cleanup, and theinvalidateManifest()method itself. The 14 callers ofinvalidateManifest()either move toinvalidateUrlPatternCache()(when they actually mutate URL patterns: collection create/update/delete, WP-import collection creation) or drop the call entirely (field/taxonomy/plugin mutations don't affect URL patterns).Closes #776, closes #873, closes #876, closes #877.
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
Notes
Breaking changes (called out in the changeset)
locals.emdash.invalidateManifestis removed. Plugin code that called this after schema changes should switch tolocals.emdash.invalidateUrlPatternCache(the only side effect that survived) — or drop the call entirely if the mutation didn't affect collection URL patterns.locals.emdashManifestis removed. Read it viaawait locals.emdash.getManifest()instead. The publishedlocals.d.tsambient type has been updated to match — user sites that still referenceAstro.locals.emdashManifestwill now get a compile error rather than silently typing asEmDashManifestand crashing at runtime.EmDashRuntime.invalidateManifest()is removed.EmDashRuntime.getManifest()is preserved with the same signature; its body now skips the cache layer.Performance
Net diff: -165 lines. The admin manifest build is now two queries flat (one for collections, one for the fields of every returned collection) instead of N+1. This is the cost the cache was hiding; with a
WHERE collection_id IN (...)replacement it's cheap enough to run per request.