feat!: migrate to SQLite storage backend and fix key-value era defects - #65
Open
naporin0624 wants to merge 26 commits into
Open
feat!: migrate to SQLite storage backend and fix key-value era defects#65naporin0624 wants to merge 26 commits into
naporin0624 wants to merge 26 commits into
Conversation
Design for migrating y-durableobjects from the legacy key-value Durable Object storage backend to SQLite, bundled with the bug fixes that the forced breaking change makes it possible to ship at once. Key decisions: - Single `updates` table with a `kind` column; snapshots and incremental updates are no longer distinguished. This removes the 128KiB document ceiling, the 128-key delete limit, and the key-ordering hazard by construction. - Compaction fires on a row-count threshold and on last disconnect only; no alarms. - awareness clientID ownership is persisted via serializeAttachment so it survives hibernation. - On persistence failure, close every connection and discard in-memory state; CRDT clients re-send on reconnect, so the fault self-heals. - No ORM. Drizzle's async API risks breaking implicit-transaction atomicity; Kysely compiles synchronously but earns nothing against three static queries. SQL generation is isolated in queries.ts so Kysely can be swapped in later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
PRAGMA is rejected on Durable Object SQLite (SQLITE_AUTH), so the user_version migration runner in the spec would not have worked. Replaced it with a schema_version table. Also recorded the other probed behaviours: BLOB columns come back as ArrayBuffer (conversion required on read), both Uint8Array and ArrayBuffer bind fine, AUTOINCREMENT is allowed, and all 41 existing tests pass unchanged on the SQLite backend because the KV API is transparently backed by __cf_kv. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Ten tasks, seventy TDD steps, each ending on a green suite and a commit. Ordered so the backend switch lands first (verified to pass unchanged because the KV API is backed by __cf_kv), then the new storage layer is built alongside the old one, then the Durable Object swaps over and the key-value implementation is deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Also register SqlStorage as an ESLint global (eslint.config.js), mirroring the existing DurableObjectState entry, since it is an ambient type from worker-configuration.d.ts used without import in schema.ts.
Adds YSqliteStorage implementing the new YStorage interface (getUpdate, storeUpdate, commit, destroy) backed by the `updates` table from Task 1's schema migration. Reads restore updates in seq order and merge continuation fragments; writes insert standalone rows. commit() is a placeholder for Task 3's compaction. The legacy KV-backed TransactionStorage is untouched and both implementations coexist until Task 4 swaps the Durable Object over. Also ignores docs/superpowers and .superpowers in Prettier so `pnpm fmt` stops reformatting the plan/spec markdown on every task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Review fix round 1 for Task 2. #readAll previously fell through to treating a kind=1 continuation row with no preceding row as a standalone update, feeding raw fragment bytes to Y.mergeUpdates. It now throws an explicit error naming the offending seq. SELECT_ALL_UPDATES and UpdateRow gained the seq column needed to report it, and a regression test inserts a malformed row directly to cover the corrupt-table path. Also rewrites the misleading comment on the 1000-update round-trip test: it never proved seq-ordered restoration (Y.mergeUpdates converges regardless of update arrival order), so the comment and test name now describe what the test actually covers. Genuine ordering coverage lands in Task 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Implements commit() to merge accumulated update rows once maxRows is exceeded and split the merged result into maxChunkBytes fragments, removing the legacy 128KiB key-value ceiling. Adds a byte-equality regression test for chunk reassembly ordering, since ordering is the one place concatenating fragments out of seq order silently corrupts data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
…old start Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Prevents an earlier unsubscribe() call from deleting a newer listener that notify() registered for the same origin after the first one, which would otherwise silently stop message delivery to that origin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Track each connection's owned awareness clientIDs in a SessionRegistry, persisted into the WebSocket's attachment so ownership survives Durable Object hibernation. unregisterWebSocket() now removes only the disconnecting connection's clientIDs instead of the whole room's, fixing the bug where any one client leaving wiped every participant's cursor/presence state.
…tates unregisterWebSocket() called removeAwarenessStates() while the departing socket was still a registered WSSharedDoc listener. removeAwarenessStates synchronously broadcasts to all listeners including the departing one, and send() on an already-closed Workers WebSocket throws — which the outer catch swallowed before sessions.remove(ws) could run, leaking the session and its listener for the rest of the Durable Object's life and breaking delivery to every other listener reached after it. Reorder unregisterWebSocket to unsubscribe first. Also harden WSSharedDoc.broadcast with a per-listener try/catch so one dead connection can never abort delivery to the rest of the room, and replace the awareness-on-close regression test with one that drives a real awareness-protocol message through both connections and closes one via webSocketClose, since the previous version never actually exercised the broadcast path that broke.
The prior fix commit (f492015) reordered unregisterWebSocket so the departing socket's WSSharedDoc listener is unsubscribed before its awareness states are removed, but nothing discriminated that ordering on its own: with the broadcast guard in place, re-reversing the two lines still passed every existing test. Add a test that subscribes to the awareness "update" event and records whether the departing socket is still registered in sessions at the moment the event fires, then asserts it was not -- proving the unsubscribe ran first, independent of whether ws.send() throws. Verified by toggling: re-reversing the two lines (with the broadcast guard untouched) makes this test fail while the pre-existing awareness-on-close test still passes, reproducing exactly the gap this closes.
Persistence used to happen from an unawaited floating promise in the doc "update" listener, so a caller could observe a write that hadn't actually landed in storage yet, and a rejected storeUpdate() was silently dropped. Route every update through a serialized persist queue that webSocketMessage and updateYDoc both await, and if a write fails, close every connection with 1011 and abort() the Durable Object so the next start rehydrates from storage instead of serving state that has silently drifted ahead of it. Also wrap webSocketMessage's doc.update() in a try/catch so a malformed message closes only the offending socket instead of crashing the whole Durable Object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
…verage A throwing ws.close() (e.g. on a socket already closed by the exception boundary, or the socket whose own error caused the failure) used to escape the close loop in onPersistFailure and abort() unwrapped, which meant state.abort() was never reached and the failure policy silently degraded to "some sockets stay open, in-memory doc still ahead of storage." Wrap the per-socket close() in try/catch, matching the same pattern already used in WSSharedDoc.broadcast(). Also tightens the malformed-message test to assert the offending socket was actually closed, adds a regression test proving a throwing close() no longer blocks abort() or the remaining sockets, and corrects a rehydration-test comment made stale by the earlier idempotency guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
…estroy API Registers a ping/pong WebSocketAutoResponse pair so client keepalives no longer wake the Durable Object from hibernation, asserts the SQLite storage backend is in use (failing loudly with a migration pointer otherwise), and adds a destroy() API that closes all connections and deletes the room's storage. destroy()'s socket-close loop guards each close() individually (same shape as onPersistFailure) so one already-closed/errored socket can't prevent storage.destroy() from running. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
…getYDoc updateYDoc previously ran its argument through WSSharedDoc.update(), which expects a sync-protocol-framed message, while getYDoc() returns a raw encodeStateAsUpdate() result. That made it impossible to feed getYDoc()'s output back into updateYDoc(), silently breaking the most natural export/ import use case and the README's own example, and blocking the v1-to-v2 migration recipe (v1's getYDoc() already returned a raw update). updateYDoc now calls applyUpdate(this.doc, update, RPC_ORIGIN) directly, so it takes a raw Yjs update. Updated every updateYDoc call site in tests that was wrapping its payload with createSyncMessage (including the POST /rooms/:id/update e2e test, which forwards its body straight to updateYDoc), while leaving webSocketMessage's framed-message call sites untouched. Added a round-trip test that exports from one room via getYDoc and imports into another via updateYDoc. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Fix round 1 on Task 9's updateYDoc change: nothing proved an update applied via updateYDoc actually reaches connected WebSocket clients, only that it round-trips through storage between two Durable Objects. Adds a test that connects a client, calls updateYDoc with a raw update, and decodes the broadcast the socket receives (past the outer sync-message wrapper, via y-protocols/sync's readSyncMessage) to assert its content matches — not just that some send happened. Verified the test discriminates: temporarily made updateYDoc pass the room's own connected socket as the update origin (a live WSSharedDoc broadcast-listener key, standing in for RPC_ORIGIN colliding with one), confirmed the test fails with "expected spy to be called at least once", then reverted with no production diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Update README and CLAUDE.md for the SQLite storage backend, add a migration-from-v1 recipe (using the two-step get/idFromName form, since getByName isn't declared in this repo's worker-configuration.d.ts), and correct the same getByName inaccuracy in the design spec. Add the v2 changeset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
- Changeset now documents the persistence-failure mass-disconnect behavior (close 1011 + state.abort()) and why it's safe. - CLAUDE.md's Testing Approach no longer claims storage tests use in-memory implementations; they run against the real SQLite backend via runInDurableObject. - Plan document's Task 9 section and self-review table no longer instruct switching to getByName, which isn't declared in this repo's worker-configuration.d.ts and wouldn't typecheck. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
The embedded README migration-recipe snippet in Task 10 of the plan still used getByName, which isn't declared in this repo's worker-configuration.d.ts. Switch it to the same two-step get(idFromName(id)) form used everywhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
Addresses the last-gate review of feat/sqlite-migration-v2: chunk oversized updates in storeUpdate() (not just commit()) so a single update over Cloudflare's 2MB row limit no longer wedges a room permanently, floor the compaction threshold so a small maxRows can't thrash, guard the remaining unguarded socket paths (onStart's hibernation-wake loop, WSSharedDoc.send), make server-initiated closes actually unregister the session instead of relying on webSocketClose firing, have destroy() drain the persist queue and discard the in-memory Doc before deleting, fix the now-broken README RPC extension example, and clean up several smaller export/type/lint inconsistencies flagged in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
The fix wave's own unregisterWebSocket() call in webSocketMessage's catch block sat after a bare ws.close(1003, ...). workerd can throw closing an already-errored socket -- the same hazard already guarded in destroy() and onPersistFailure -- which would both skip the unregister (reinstating the IMPORTANT 3 leak) and reject webSocketMessage, resetting the DO on a single malformed frame. Wrap close() in the same try/catch/log pattern used elsewhere so neither depends on close() succeeding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
🦋 Changeset detectedLatest commit: 4ca5c95 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 yjs-worker with
|
| Latest commit: |
4ca5c95
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c447cd0b.yjs-worker.pages.dev |
| Branch Preview URL: | https://feat-sqlite-migration-v2.yjs-worker.pages.dev |
commit: |
Contributor
📊 Package size report 109%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
- Document that wrangler.toml is only a fresh-environment test fixture, and that a deployed v1 Worker must append a migration rather than rewrite the "v1" tag; show the concrete two-binding wrangler config and npm-aliasing recipe for running v1 and v2 side by side in README. - Make awareness clientID ownership exclusive in SessionRegistry.track() so an overlapping reconnect can't leave both the old and new socket claiming the same ID, which previously caused a false departure broadcast when the old socket closed. - Destroy the old WSSharedDoc before replacing it in destroy(), so the Awareness instance it owns (and its repeating setInterval) is cleaned up instead of leaking for the rest of the Durable Object's lifetime. - Validate maxChunkBytes and maxRows as positive integers in YSqliteStorage, since a non-positive maxChunkBytes previously caused an infinite loop in #split(). compaction floor from the row count actually loaded (not reset to maxRows), so a rehydrated large document doesn't immediately recompact on its first write after waking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
…ibernate y-protocols' Awareness installs a repeating setInterval in its constructor (every 3s by default) purely to time out stale remote clients -- the "renew local clock" branch never fires here since WSSharedDoc immediately calls setLocalState(null). Any pending setInterval/setTimeout blocks Durable Object hibernation entirely, so every YDurableObjects instance has been staying awake, and billed for duration, for its whole lifetime, regardless of the ping/pong auto-response. Clear the interval right after Awareness is constructed, accepting the loss of the timeout GC (awareness is already cleaned up on disconnect and rebuilt from nothing on restart). Guard the fix with a runtime type check on the underscore-prefixed, non-public `_checkInterval` field so a future y-protocols internals change fails loudly at construction instead of silently regressing hibernation. Update README, the sqlite-migration changeset, and the design spec's 6-6 section to reflect that the interval -- not the ping/pong response -- was actually blocking hibernation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj
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.
Why
Durable Object namespaces backed by key-value storage can no longer be created by accounts without an existing one, and they are unavailable on the Free plan entirely.
y-durableobjectsv1 requires that backend, so it is not deployable for new users.Storage type is immutable once a namespace exists — Cloudflare rejects an in-place switch with
storage_type_mismatch— so migrating forces a breaking release. This PR takes that one opportunity to also fix the defects the key-value backend's constraints had caused or hidden.Defects fixed
getYDoc()returned a raw update butupdateYDoc()required a framed message, so export/import silently failedlist()restored updates out of orderqueryAwarenessandauthwere unhandled; unknown types were silently discardedDesign
A single
updatestable. Snapshots and incremental updates are not distinguished — compaction merges every row into one update withY.mergeUpdatesand writes it back. When a merged result (or a single incoming update) exceeds the SQLite row limit it is split into byte fragments, marked with akindcolumn, and reassembled on read. That one decision removes C-2, C-3 and H-5 from the design rather than patching each.Yjs transaction origin is now threaded through
WSSharedDoc, which fixes H-1 and H-2 and also supplies the per-connection awareness ownership that fixes C-4 — those turned out to share a root cause. Ownership is persisted in the WebSocket attachment so it survives hibernation.On a storage write failure the library fails closed: every connection is closed with
1011and the in-memory document is discarded. Yjs clients hold the full document and re-send on reconnect, so the fault self-heals — the alternative was serving in-memory state that storage does not have.Design doc:
docs/superpowers/specs/2026-08-14-sqlite-migration-design.mdBreaking changes
new_sqlite_classes. A v1 namespace cannot be converted in place — see the README's migration section.updateYDoc()takes a raw Yjs update instead of a sync-protocol message, so it round-trips withgetYDoc().YTransactionStorage→YStorage;WebSocketAttachment→SessionAttachment.WSSharedDoc.notify(listener)→notify(origin, listener);update(message)→update(message, origin).Also added
destroy()to delete a room's data, and a"ping"/"pong"auto-response so keepalives stop waking the object from hibernation — the largest single lever on duration billing.Verification
69 tests, typecheck, lint and build all pass. Every task was reviewed independently and a whole-branch review ran at the end; seven tests that turned out to pass against broken implementations were rewritten, and each new test was verified to fail without its fix.
One thing needs manual checking before release: the key-value-backend rejection path in
assertSqliteBackendcannot be tested, because the test environment cannot provide a key-value-backed Durable Object. Real hibernation cannot be forced in@cloudflare/vitest-pool-workerseither, so those paths are covered by construction rather than by test.🤖 Generated with Claude Code
https://claude.ai/code/session_015mdNeCzNPZysRHzmkFp9Bj