Skip to content

feat(plugins): atomic conditional writes for plugin storage (insert / updateIf) - #2

Open
vedanshujain wants to merge 1 commit into
fix/plugin-storage-postgres-jsonbfrom
feat/plugin-storage-conditional-writes
Open

feat(plugins): atomic conditional writes for plugin storage (insert / updateIf)#2
vedanshujain wants to merge 1 commit into
fix/plugin-storage-postgres-jsonbfrom
feat/plugin-storage-conditional-writes

Conversation

@vedanshujain

Copy link
Copy Markdown
Owner

Summary

Adds two additive, single-statement atomic primitives to the plugin StorageCollection<T> API, and wires up the previously-unused declared-uniqueIndexes creation. This closes the "plugin storage has no atomic conditional write" gap (discussion emdash-cms#632): today plugin storage offers only an unconditional put upsert, so an inventory decrement ("sell one iff in stock") is impossible in-process.

Stacked on #1 (fix/plugin-storage-postgres-jsonb) — that PR makes plugin-storage queries/guards numeric-correct on Postgres, which the updateIf guard depends on. Review #1 first; this PR's diff is feature-only.

insert(id: string, data: T): Promise<
  | { inserted: true }
  | { inserted: false; reason: "exists" | "unique_violation"; conflictField?: string }>;

updateIf(id: string, args: {
  where: WhereClause;                                    // reuses the existing query() DSL
  set?: Partial<T>;                                      // wholesale field set
  delta?: { [K in keyof T]?: { inc: number } | { dec: number } };  // integer deltas
}): Promise<{ applied: true; data: T } | { applied: false }>;

Design — single-statement, D1-safe, no migration

  • insert = INSERT … ON CONFLICT (plugin_id, collection, id) DO NOTHING; inserted from the affected-row count. A same-id collision → {inserted:false, reason:"exists"}. A partial UNIQUE expression-index collision (not the PK conflict target) throws → caught and classified as {inserted:false, reason:"unique_violation", conflictField?} (PG 23505 + error.constraint; SQLite SQLITE_CONSTRAINT_UNIQUE/message). Non-unique errors are re-thrown.
  • updateIf = ONE guarded UPDATE … SET … WHERE <guard> RETURNING data. The guard lives entirely in the WHERE (reusing the existing query() WhereClause translation) and the arithmetic is computed in SQL over the original row — no read-then-write, no interactive transaction, no SELECT … FOR UPDATE. That is what makes it correct on D1 (serialized writes, no interactive txn) and portable to Postgres (row-lock + predicate re-evaluation). applied comes from RETURNING; set/delta are separate args (no {inc:n}-as-value ambiguity); deltas use COALESCE(base,0)±n so a delta on a missing field can never null the data column. Integer-only deltas enforced at runtime (Number.isInteger).
  • Honored unique indexescreateStorageIndexes is now called from PluginManager.install() (fail-loud on error; CREATE UNIQUE INDEX IF NOT EXISTS, idempotent, partial-scoped per plugin/collection), and removeAllPluginIndexes from uninstall().
  • Both sandbox bridgesstorage/insert + storage/updateIf added to the packages/workerd bridge and the packages/cloudflare (production D1) bridge + wrappers, each collection-validated, so native and sandboxed plugins both get the primitives.
  • No migration — no new column (no version/CAS field); indexes are runtime DDL. get/put/delete/query/count semantics are unchanged.

No-oversell test evidence (the acceptance bar)

storage-no-oversell.test.ts (dialect-parameterized): seed stock M=5, fire N=20 concurrent updateIf({where:{stock:{gte:1}}, delta:{stock:{dec:1}}}) via Promise.all, assert exactly 5 applied:true, 15 applied:false, final stock 0.

  • Postgres (real concurrency, pool of separate connections): ✓ [postgres] exactly M of N guarded decrements apply under real concurrent connections (the true no-oversell race)applied=5, rejected=15, final=0.
  • The SQLite/better-sqlite3 variant is named to state that it serializes writes in-process, so it proves SQL correctness, not the race.
  • D1: exercised over the full workerd bridge on better-sqlite3 (the identical SQL dialect D1 uses; D1 serializes writes). No live Miniflare/D1 concurrency harness exists in the repo, so the genuine concurrent-race proof is the Postgres suite (labeled as such).

storage-conditional.test.ts covers insert new/replay/unique-collision; updateIf guard-pass/guard-fail/missing-row; integer inc/dec; float-delta rejection; wholesale set merge; combined set+delta; setdelta and neither-provided rejection; guard over equality + multi-digit RangeFilter + in + startsWith + non-indexed field + empty in:[]; delta-on-missing COALESCE. Both dialects; typecheck / lint / format clean; full core + workerd suites green.

Reviewed by two engineers independently — both approve.

Deferred follow-ups (out of scope, per the reporter's diagnosis in emdash-cms#632)

  • Version / optimistic-concurrency (CAS) column — not added; updateIf's predicate guard covers the inventory/idempotency cases without one.
  • getWithMeta, multi-row applyBatch / D1 batch() — deferred.
  • Production D1 unique-index provisioning — this PR wires unique-index creation into PluginManager.install() (the native/self-hosted path). The production D1 marketplace/registry install path runs through EmDashRuntime (handleMarketplaceInstallsyncMarketplacePlugins) and does not go through PluginManager, so declared uniqueIndexes are not yet materialized on that path — tracked as a follow-up (the updateIf/no-oversell guarantee needs no index and is unaffected; only insert's unique-field dedup depends on it).

Credit to the emdash-cms#632 reporter for diagnosing that plugin storage's unconditional-upsert-only surface blocks atomic inventory decrements.

… updateIf)

Add two additive, single-statement primitives to the plugin
StorageCollection<T> API:

- insert(id, data): insert-once via INSERT … ON CONFLICT (pk) DO NOTHING.
  A same-id collision returns { inserted:false, reason:"exists" }; a
  declared unique-index collision on a non-id field is classified as
  { inserted:false, reason:"unique_violation", conflictField? } and any
  other DB error is re-thrown.
- updateIf(id, { where, set?, delta? }): predicate-guarded atomic update
  via one UPDATE … SET json_set/jsonb_set(…) WHERE <pk> AND <guard>
  RETURNING data. The guard reuses the numeric-correct where translation
  from the prior fix, and integer deltas are applied in-SQL with
  COALESCE(base,0) ± n, so N concurrent guarded decrements cannot
  oversell. set and delta are separate args; floats are rejected at
  runtime; an empty in:[] guard short-circuits to applied:false.

Declared uniqueIndexes are now materialized as real unique indexes on
plugin install (fail-loud on error) and dropped on uninstall. The new
methods are delegated through the in-process context, the workerd bridge,
and the Cloudflare D1 bridge so all runners behave identically.

Proven no-oversell on real Postgres (and the SQLite/D1 dialect); both
dialects covered by the contract suite.
@vedanshujain
vedanshujain force-pushed the fix/plugin-storage-postgres-jsonb branch from d7d732d to 8b89c14 Compare July 19, 2026 13:09
@vedanshujain
vedanshujain force-pushed the feat/plugin-storage-conditional-writes branch from dfd52d0 to fbbb854 Compare July 19, 2026 13:09
@github-actions

Copy link
Copy Markdown

Overlapping PRs

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

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

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

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 github-actions Bot added stale and removed stale labels Aug 4, 2026
@github-actions github-actions Bot added stale and removed stale labels Aug 19, 2026
@github-actions github-actions Bot added stale and removed stale labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant