feat(plugins): predicate-guarded atomic updateIf for plugin storage (no-oversell)

[vedanshujain]

What does this PR do?

Adds ctx.storage.<collection>.updateIf(id, { where, set?, delta? }) — a predicate-guarded atomic update for plugin storage, and the no-oversell primitive. Today plugin storage offers only unconditional put (whole-doc upsert), so "sell one iff in stock" is impossible in-process without a racy read-modify-write.

updateIf runs the guard and the arithmetic in one statement:

UPDATE _plugin_storage
SET data = <json_set/jsonb_set expr>, updated_at = ?
WHERE plugin_id = ? AND collection = ? AND id = ? AND <guard>
RETURNING data

No read-then-write, no interactive transaction — so N concurrent guarded decrements serialize correctly (exactly M of N apply, final stock 0, never oversell).

• where reuses the numeric-correct WhereClause translation from query() (landed upstream in fix(core): plugin storage where-filters fail on Postgres with boolean = integer (#920) #1898 + the numeric fix), so a multi-digit guard like stock >= 10 compares numerically on Postgres.
• set writes wholesale field values; delta applies integer inc/dec in-SQL over COALESCE(base, 0) (a delta on a missing/null field starts from 0). Integer-only deltas enforced at runtime; a field may not appear in both set and delta; at least one is required. set/delta are separate args so {inc:n} is never mistaken for a value.
• Returns { applied: true, data } or { applied: false } (row absent or guard failed — intentionally indistinguishable). Never inserts.

Backed by pluginDataWriteExpr (dialect-correct json_set/jsonb_set, values bound as params). No new column, no migration.

Isolation: the { applied: false } contract assumes READ COMMITTED (Postgres' default), where the losing concurrent writers cleanly resolve to { applied: false } via EvalPlanQual. Under REPEATABLE READ / SERIALIZABLE a loser aborts instead, so updateIf traps SQLSTATE 40001/40P01 and throws a typed StorageSerializationError (with cause + sqlState) telling the caller to retry — rather than surfacing a bare driver error. The no-oversell safety invariant holds under every isolation level (a losing writer never applies); only the result shape degrades to a throw. SQLite/D1 serialize writes and never hit this path.

Tests

• storage-updateif.test.ts — guard pass/fail/missing, integer inc/dec + integer round-trip, float-delta rejection, COALESCE-from-0 on missing/null, wholesale set, set+delta, both-fields / neither-provided rejection, guard operator coverage (equality, multi-digit gte, in, startsWith, non-indexed field), empty-in no-op.
• storage-no-oversell.test.ts — M-of-N concurrent guarded decrements (the acceptance bar; Postgres is the real race, SQLite proves SQL correctness).

Both dialects; Postgres via EMDASH_TEST_PG. Full core plugin suites green (795 tests), typecheck / lint / format clean.

Type of change

• Bug fix
• Feature (requires maintainer-approved Discussion) — see Plugin storage has no atomic primitive - am I missing something? #632
• Refactor (no behavior change)
• Translation
• Documentation
• Performance improvement
• Tests
• Chore (dependencies, CI, tooling)

Checklist

• I have read CONTRIBUTING.md
• pnpm typecheck passes
• pnpm lint passes
• pnpm test passes (or targeted tests for my change) — full core plugin suites, both dialects
• pnpm format has been run
• I have added/updated tests for my changes (if applicable)
• User-visible strings in the admin UI are wrapped for translation (if applicable) — N/A
• I have added a changeset (if this PR changes a published package)
• New features link to an approved Discussion — approved by @swissky in Plugin storage has no atomic primitive - am I missing something? #632

AI-generated code disclosure

• This PR includes AI-generated code — model/tool: Claude Opus 4.8 (Claude Code)

[changeset-bot]

🦋 Changeset detected

Latest commit: 11fffb5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

Name	Type
emdash	Minor
@emdash-cms/cloudflare	Minor
@emdash-cms/sandbox-workerd	Patch
@emdash-cms/fixture-perf-site	Patch
@emdash-cms/perf-demo-site	Patch
@emdash-cms/cache-demo-site	Patch
@emdash-cms/do-demo-site	Patch
@emdash-cms/do-solo-demo-site	Patch
@emdash-cms/admin	Minor
@emdash-cms/auth	Minor
@emdash-cms/blocks	Minor
create-emdash	Minor
@emdash-cms/gutenberg-to-portable-text	Minor
@emdash-cms/x402	Minor
@emdash-cms/auth-atproto	Patch
@emdash-cms/release-service	Patch
@emdash-cms/plugin-embeds	Patch

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

[github-actions]

Scope check

This PR changes 732 lines across 9 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.

[pkg-pr-new]

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2169

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2169

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2169

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2169

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2169

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2169

emdash

npm i https://pkg.pr.new/emdash@2169

create-emdash

npm i https://pkg.pr.new/create-emdash@2169

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2169

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2169

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2169

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2169

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2169

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/@emdash-cms/registry-moderation@2169

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2169

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2169

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2169

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2169

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2169

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2169

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2169

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2169

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2169

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2169

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2169

commit: 11fffb5

[github-actions]

This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity.

If you're still working on this, please push an update or leave a comment.

[github-actions]

Overlapping PRs

This PR modifies files that are also changed by other open PRs:

• feat(plugins): add revision based conditional writes to storage and KV #2980 (3 shared files)

This may cause merge conflicts or duplicated work. A maintainer will coordinate.

[emdashbot]

The change is the right solution to the right problem: plugin storage had no in-process predicate-guarded atomic write, so "sell one iff in stock" required a racy read-modify-write. Using a single UPDATE … WHERE <guard> RETURNING and composing set/delta with dialect-safe json_set/jsonb_set is idiomatic for EmDash and fits the existing repository layer.

What I checked: the diff, the full changed source files, the validateJsonFieldName helper, existing plugin-storage query/where code, the canonical skills/creating-plugins/references/storage.md docs, and the unit/integration tests. I did not run the test suite, linter, or builds (no shell). Statically the implementation looks sound: field names are validated before interpolation, values are bound, the empty-in: [] short-circuit is correct, and the tests cover the core guard/arithmetic/validation paths for both dialects.

Headline issues:

1. Reviewer-addressed comment left in tests. storage-updateif.test.ts contains // … (#reviewer flag). AGENTS.md explicitly forbids comments that reference review threads/PRs because they become stale the moment the change merges.

2. Serialization-error messaging is misleading about 40P01. The code and docstrings claim 40001/40P01 aborts happen "under an isolation level stricter than READ COMMITTED". That is not true for 40P01 (deadlock): deadlocks can occur under READ COMMITTED too, not only under REPEATABLE READ/SERIALIZABLE. The resulting error message tells the caller to "run it at READ COMMITTED" even when they already are, which is bad guidance. The mapping itself is reasonable; the prose just needs to stop attributing deadlocks to isolation level.

3. Canonical plugin-storage docs are now out of date. skills/creating-plugins/references/storage.md still lists the old StorageCollection API and has no updateIf section. Since the PR adds a public plugin-facing method, the canonical reference should describe the guard/set/delta semantics and show the updated interface.

---

Findings

• [needs fixing] packages/core/tests/integration/plugins/storage-updateif.test.ts:185

  This inline comment ends with (#reviewer flag), which is addressed to the reviewer/review thread. AGENTS.md forbids comments that reference PRs, issues, or review threads in source — they become stale the moment the change merges; that context belongs in the commit message or PR description, not in a test.

  		// `{ stock: undefined }` has a key but no DEFINED entry — presence is
  		// derived from defined entries, so this hits the "at least one" guard
  		// instead of doing a no-op write that bumps `updated_at`.
  

• [needs fixing] packages/core/src/database/repositories/plugin-storage.ts:49-52

  This docstring asserts that 40001/40P01 aborts happen "under an isolation level stricter than READ COMMITTED". That is incorrect for 40P01 (deadlock_detected): deadlocks can occur under READ COMMITTED as well. The same inaccurate narrative is repeated in the StorageSerializationError docstring in packages/core/src/plugins/storage-query.ts.

  Update the comment to describe the SQLSTATEs without falsely tying 40P01 to strict isolation.

  /**
   * SQLSTATEs a losing concurrent `updateIf` writer may abort with:
   * `40001` (serialization_failure) and `40P01` (deadlock_detected).
   * The `{ applied: false }` contract assumes READ COMMITTED; these aborts
   * are surfaced as `StorageSerializationError` so callers can retry.
   */
  

  Also update the mapSerializationFailure docstring immediately below to remove the cross-reference to that false claim.

• [needs fixing] packages/core/src/database/repositories/plugin-storage.ts:88-93

  This error message tells users that the abort happened "under REPEATABLE READ / SERIALIZABLE" and advises them to "run it at READ COMMITTED". Because the code maps 40P01 (deadlock) to this same error, a deadlock under READ COMMITTED would receive advice that does not apply. Drop the false isolation-level attribution and simply advise a retry.

  			`updateIf lost a concurrent race (SQLSTATE ${sqlState}). Under ` +
  				`READ COMMITTED (the default) a losing writer normally resolves to ` +
  				`{ applied: false }, but a serialization failure or deadlock can still ` +
  				`abort. Retry the call. The no-oversell safety invariant still holds ` +
  				`— a losing writer never applies.`,
  

• [needs fixing] skills/creating-plugins/references/storage.md:147-156

  The PR adds ctx.storage.<collection>.updateIf(...) as a public plugin API, but the canonical storage reference still shows the old StorageCollection interface without updateIf and has no section explaining predicate-guarded updates. The changeset README says useful examples/explanations should not live only in the changeset or PR description.

  Update the interface block and add a ### Conditional Updates section before ### Full API covering the where guard, set, integer delta, return shape, and StorageSerializationError.

  interface StorageCollection<T = unknown> {
  	get(id: string): Promise<T | null>;
  	put(id: string, data: T): Promise<void>;
  	delete(id: string): Promise<boolean>;
  	exists(id: string): Promise<boolean>;
  	getMany(ids: string[]): Promise<Map<string, T>>;
  	putMany(items: Array<{ id: string; data: T }>): Promise<void>;
  	deleteMany(ids: string[]): Promise<number>;
  	query(options?: QueryOptions): Promise<PaginatedResult<{ id: string; data: T }>>;
  	count(where?: WhereClause): Promise<number>;
  	updateIf(id: string, args: UpdateIfArgs<T>): Promise<UpdateIfResult<T>>;
  }
  

[logelog]

@vedanshujain, would you be open to me contributing to this PR? I've prepared and tested additional changes from my earlier combined implementation against your b25bf0e5 commit:

• Argument validation so malformed guards cannot become unconditional writes, plus safe integer counter and overflow checks.
• Public exports for the update types and serialization error.
• Cloudflare and Workerd sandbox support, including preserving guards during JSON transport and carrying safe retry metadata.
• Documentation for the arguments, counter behavior and transaction retry requirements.

These preserve your existing return shape and add no revision migration. The changes have been tested on SQLite, PostgreSQL and D1, including the sandbox paths.

If this approach works for you, I'd like to contribute these fixes here and help get your PR ready to merge first. After that, I'd welcome your contribution to my revision based writes PR so we can finish that part together. Let me know how you'd prefer to receive the commits.

[vedanshujain]

@logelog sounds good to me, please do. I added the access to the fork so you can push commit. I was already working on vedanshujain#9 before I saw your message, it addresses the argument validation but rest three are still open.

[vedanshujain]

I am marking the PR as draft again so we can get it polished before asking maintainer's time for review. Once @logelog adds their commit, we will mark this ready for review and merge again

[logelog]

Thanks @vedanshujain. I've pushed three commits directly to this branch and merged the latest main. Your fix for range filters with no defined bound is retained. The additions cover:

• Safe integer counter validation and overflow checks, with the whole update left unapplied when a stored counter or result is invalid.
• Public exports for the update types and serialization error.
• Cloudflare and Workerd sandbox support, including preserving guards during transport and carrying safe retry metadata.
• Documentation for the arguments, counter behavior and retrying an entire transaction after a serialization failure or deadlock.

The shared filter precedence and return shape are preserved, with no migration added. Local checks passed on SQLite, PostgreSQL and D1, including the sandbox paths; build, typecheck and lint also passed.

Please take a look at the additions. Once you're happy with them and CI passes, we can mark this ready for review and move on to the revision work together.

[vedanshujain]

Looks great, thanks so much! I am marking this for review.

[emdashbot]

This remains the right approach to the right problem: a single-statement UPDATE … WHERE <guard> RETURNING is the idiomatic way to add predicate-guarded atomic writes to plugin storage. The follow-up diff addresses all three findings from the previous review and adds thorough sandbox-transport handling plus the undefined-bound range-filter hardening.

What I checked: the full diff, the changed core/cloudflare/workerd source files, validate.ts, package.json exports, the new integration/workerd tests, and the updated canonical docs & changesets. I did not run the test suite, linter, or builds (no shell).

Headline: the prior review comments are resolved.

• The #reviewer flag test comment has been removed.
• The StorageSerializationError docstrings and message no longer falsely tie 40P01 (deadlock) to strict isolation levels; the prose now correctly notes that deadlocks can occur at READ COMMITTED too.
• Canonical plugin-storage docs (skills/creating-plugins/references/storage.md and all template copies) now include the updateIf interface and a detailed “Conditional Updates” section covering where, set, integer delta, return shape, and retry semantics.

New areas covered in this revision also look solid:

• The range-filter hardening (buildCondition throws when every bound is undefined; parseStorageUpdate/copyStorageWhere reject the same) prevents a guard built from an optional value from silently widening to “match everything.”
• Both Cloudflare RPC and Workerd HTTP sandbox transports serialize StorageSerializationError as {__emdashStorageError: …} / {error: {…}} and reconstruct a retryable error on the plugin side, with private SQL/cause stripped.
• Field names are validated before JSON-path interpolation; values are bound parameters; json_set/jsonb_set are composed correctly for both dialects; integer-safety guards are applied in-SQL.
• Tests cover no-oversell, guard operators, malformed inputs, sandbox serialization, and Workerd wrapper marshaling.

I did not find a new correctness issue, SQL-injection surface, or AGENTS.md convention violation. Empty findings.