feat(plugins): predicate-guarded atomic updateIf for plugin storage (no-oversell)#2169
Draft
vedanshujain wants to merge 2 commits into
Draft
feat(plugins): predicate-guarded atomic updateIf for plugin storage (no-oversell)#2169vedanshujain wants to merge 2 commits into
vedanshujain wants to merge 2 commits into
Conversation
Adds `ctx.storage.<collection>.updateIf(id, { where, set?, delta? })` — the
no-oversell primitive. The guard and the arithmetic run in a single
`UPDATE … SET data = <json_set/jsonb_set expr>, updated_at = ? WHERE <pk> AND
<guard> RETURNING data`, so there is no read-then-write and N concurrent guarded
decrements serialize correctly (exactly M of N apply, never oversell).
- `where` reuses the numeric-correct WhereClause translation from query(), 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 be in both set and
delta; at least one is required.
- Returns `{ applied: true, data }` or `{ applied: false }` (row absent OR guard
failed — intentionally indistinguishable). Never inserts.
Backed by `pluginDataWriteExpr` (json_set / jsonb_set, dialect-correct, values
bound). No new column, no migration. Tests: storage-updateif (guard pass/fail/
missing, inc/dec, float rejection, COALESCE-from-0, set/mixed/validation, guard
operator coverage, empty-`in`) and storage-no-oversell (M-of-N concurrent
decrements) — both dialects, Postgres via EMDASH_TEST_PG.
Scoped deliberately: `insert` (create-iff-absent) and the sandbox bridges are
separate follow-ups.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX9YciGFRZX9aF2UcQ6rUW
…rd bypass, doc isolation
Three follow-ups on the guarded PluginStorageRepository.updateIf primitive:
1. Trap Postgres serialization failures. Under an isolation level stricter
than READ COMMITTED the losing concurrent updateIf writers abort with
SQLSTATE 40001 (serialization_failure) / 40P01 (deadlock_detected) instead
of resolving to { applied: false }. A new exported StorageSerializationError
(in storage-query.ts, alongside StorageQueryError) carries the SQLSTATE and
cause and explains the READ COMMITTED assumption + retry guidance. A pure,
unit-testable mapSerializationFailure(err) helper wraps 40001/40P01 and
rethrows everything else unchanged; updateIf's catch is
`throw mapSerializationFailure(err)`. SQLSTATE is read from err.code (and
err.cause.code defensively) — confirmed empirically that Kysely propagates
node-pg's DatabaseError.code unwrapped.
2. Fix the all-undefined delta/set guard bypass (both reviewers). Presence is
now derived from DEFINED entries: undefined-valued set fields are filtered
and undefined delta specs skipped BEFORE the "at least one of set/delta"
check, so an all-undefined payload throws instead of doing a no-op write
that bumped updated_at and returned { applied: true }. undefined values in
set/delta are documented as ignored.
3. Doc the isolation contract on the updateIf doc-comment in both types.ts
(interface) and plugin-storage.ts (impl): { applied: false } assumes READ
COMMITTED; under REPEATABLE READ / SERIALIZABLE losing writers throw
StorageSerializationError — the no-oversell SAFETY invariant holds either way.
Tests: all-undefined delta/set throw (integration, both dialects); deterministic
mapSerializationFailure unit tests (40001, 40P01, .cause.code nesting, 23505
pass-through, plain Error pass-through). Full core plugin suites green on both
dialects (804 tests).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TX9YciGFRZX9aF2UcQ6rUW
🦋 Changeset detectedLatest commit: bd80510 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 |
Contributor
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
Contributor
Scope checkThis 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. |
@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: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 unconditionalput(whole-doc upsert), so "sell one iff in stock" is impossible in-process without a racy read-modify-write.updateIfruns the guard and the arithmetic in one statement:No read-then-write, no interactive transaction — so N concurrent guarded decrements serialize correctly (exactly M of N apply, final stock 0, never oversell).
wherereuses the numeric-correctWhereClausetranslation fromquery()(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 likestock >= 10compares numerically on Postgres.setwrites wholesale field values;deltaapplies integerinc/decin-SQL overCOALESCE(base, 0)(a delta on a missing/null field starts from 0). Integer-only deltas enforced at runtime; a field may not appear in bothsetanddelta; at least one is required.set/deltaare separate args so{inc:n}is never mistaken for a value.{ applied: true, data }or{ applied: false }(row absent or guard failed — intentionally indistinguishable). Never inserts.Backed by
pluginDataWriteExpr(dialect-correctjson_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, soupdateIftraps SQLSTATE40001/40P01and throws a typedStorageSerializationError(withcause+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, wholesaleset,set+delta, both-fields / neither-provided rejection, guard operator coverage (equality, multi-digitgte,in,startsWith, non-indexed field), empty-inno-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
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change) — full core plugin suites, both dialectspnpm formathas been runAI-generated code disclosure