feat(core): add reliable incremental media usage indexing - #2394
Conversation
🦋 Changeset detectedLatest commit: 2203f92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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-playground | 2203f92 | Aug 11 2026, 04:49 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 2203f92 | Aug 11 2026, 04:49 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 2203f92 | Aug 11 2026, 04:50 PM |
Scope checkThis PR changes 12,598 lines across 96 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 |
docs | 2203f92 | Aug 11 2026, 04:55 PM |
|
All contributors have signed the CLA ✍️ ✅ |
@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/registry-verification
@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: |
|
recheck |
04b4c8b to
fbd6e20
Compare
There was a problem hiding this comment.
The PR builds durable, bounded incremental Media Usage indexing on top of Sequence 0: per-collection database triggers write versioned projection work, a lease-based processor drains that work, a fail-closed activation fence blocks writers during activation, and admin operator APIs plus client methods expose bounded work list/retry. It also threads a beforeContentWrite guard through plugin sandbox paths. The architecture fits EmDash: triggers keep the projection fresh without polling, handlers return the standard ApiResult envelope, admin routes check schema:manage + the admin scope, SQL identifiers are validated before reaching dynamic DDL, and the new middleware only fires on admin/state-changing paths so logged-out query counts are unaffected.
I read the full diff (~383 KB), the new core files under packages/core/src/media/usage/, the repository/handler/route layers, the plugin sandbox changes, the migration, and the tests. The code is careful about cross-dialect timestamps, trigger identity verification, generation leases, and stale-worker fencing. Most AGENTS.md conventions are respected.
I found one logic issue that should be fixed before merge: in packages/core/src/schema/registry.ts the field-mutation methods use invalidateContentMediaUsageSchemaChange() as a boolean probe before the transaction, but that function is not read-only—when incremental capture is active it immediately mutates _emdash_media_usage_index_status. This causes the collection’s coverage epoch to be bumped twice (or three times on error) for a single schema change, and it mutates coverage state even when the transaction later rolls back. Replace the pre-transaction call with a read-only activation check and call the invalidator only after the transaction succeeds or after a rolled-back mutation that actually modified schema. The same pattern repeats in addField, updateField, and deleteField.
That issue aside, the implementation is solid and well-tested.
| .executeTakeFirst(); | ||
|
|
||
| const sortOrder = input.sortOrder ?? (maxSort?.max ?? -1) + 1; | ||
| const activeCoverageInvalidated = await invalidateContentMediaUsageSchemaChange( |
There was a problem hiding this comment.
[needs fixing] activeCoverageInvalidated is set by calling invalidateContentMediaUsageSchemaChange(this.db, collectionSlug), which is not a read-only probe. When incremental capture is active it immediately runs UPDATE _emdash_media_usage_index_status … change_epoch = change_epoch + 1, status = 'stale' … (see src/media/usage/content-refresh.ts). Because the same function is called again after the transaction succeeds, a single field add/update/delete bumps the collection’s coverage epoch twice; if the transaction fails after schemaMutated = true, the catch block calls it a third time. It also mutates coverage state even when the schema change ultimately rolls back.
Use a read-only check (e.g., reading _emdash_media_usage_activation.state) to decide which post-transaction path to take, and only call the mutating invalidator after the transaction commits or after a mutation that actually changed schema:
// before the transaction
const incrementalCaptureActive = await isIncrementalCaptureActive(this.db);
// after the transaction succeeds
if (incrementalCaptureActive) {
await invalidateContentMediaUsageSchemaChange(this.db, collectionSlug);
} else {
await markContentMediaUsageCollectionStaleSafely(
this.db,
collectionSlug,
"CONTENT_USAGE_STALE",
);
}
// in the catch block
if (schemaMutated && incrementalCaptureActive) {
await invalidateContentMediaUsageSchemaChange(this.db, collectionSlug);
}The same duplicated pre-transaction invalidation appears in updateField (line ~928) and deleteField (line ~1083) and should be corrected the same way.
There was a problem hiding this comment.
I’ll decline this comment because the two updates are intentional safety checks. They prevent D1 from incorrectly reporting complete coverage during or after a failed schema change.
|
Aside from the docs comment, this is gtg |
1a6aa32 to
86f40db
Compare
86f40db to
9d6ecda
Compare
9d6ecda to
2203f92
Compare
What does this PR do?
Adds durable, bounded incremental Media Usage indexing on top of Sequence 0. It captures entry changes through portable database triggers, processes versioned projection work with stale-worker protection and fingerprint no-ops, runs bounded immediate and scheduled drivers, tracks trustworthy coverage epochs, exposes owner-authorized bounded work list/retry APIs and client methods, and drains existing internal cleanup records.
Activation remains caller-independent and fail-closed: it requires explicit
writersDrained: trueconfirmation and does not add an activation UI, API, or automatic caller. The change preserves V1 compatibility and leaves logged-out query counts unchanged.This is the Sequence 1 child of #2324 in the Media Library stack rooted at #2218. Keep it draft while the lower stack layers and this layer receive review; do not merge it independently.
Maintainer-approved project work from Matt’s handoff; no separate Discussion applies.
Closes: N/A — stacked child of #2218 through #2324
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — N/A; no admin UI is added.AI-generated code disclosure
Screenshots / test output
No visual changes.
Final local verification before opening this draft:
PG_CONNECTION_STRINGwas unsetTry 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
feature/media-usage-sequence-1. Updated automatically when the playground redeploys.