feat(engine): atomic write batches for D1/hosted#179
Conversation
…tion capability The database port gains an optional TransactionCapability that adapters attach when their driver supports interactive transactions, plus a runAtomic(db, fn) helper that uses it when present and falls back to plain sequential statements otherwise (unchanged D1 behavior). The Node better-sqlite3 adapter implements the capability with manual BEGIN IMMEDIATE / COMMIT / ROLLBACK, serialized through a promise queue so concurrent requests on the shared connection cannot interleave with an open transaction. Wrapped write paths (DB writes only — realtime/webhook fanout stays in routes, outside the transaction): - channel message send (message + attachments + deliveries + message_log) - DM send (message + attachments + delivery + message_log) - group DM send (message + attachments + deliveries) - thread reply (reply + deliveries) - markRead (read receipt + delivery transition + lastReadId) On self-host, a failure mid-send no longer leaves a message row with no delivery rows (silent durable-delivery loss). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Write paths gained transactional atomicity on Node in the transactional-write-paths change, but the hosted Cloudflare deployment runs on D1, which has no interactive transactions — a crash between the message insert and the deliveries insert still left a message with no delivery rows. D1 does execute db.batch([...]) atomically, and drizzle's DrizzleD1Database exposes batch() natively, so the hosted handle can get all-or-nothing writes with zero cloud-side changes. - ports/database.ts: add AtomicWrite (a built-but-unexecuted drizzle statement) and BatchCapability (D1-style atomic batch), and replace runAtomic(fn) with runAtomicWrites(db, statements). Resolution order: withTransaction (Node) -> batch (D1, detected structurally since only atomic-batch drivers expose the method; better-sqlite3's drizzle instance has no batch member) -> sequential (bare handles, historical behavior). - The five multi-statement write paths (channel send, DM send, group DM send, thread reply, markRead) now do all reads up front and hand runAtomicWrites a pure statement list, so the same list runs under a transaction, one atomic batch, or sequentially. No write depends on a prior write's DB-returned value (IDs are app-generated snowflakes); .returning() rows are recovered from the per-statement results. - message.ts reads attachment details directly from files by id before the writes (the junction rows don't exist yet mid-batch); dm.ts builds the message+attachment inserts via buildDmMessageWrites; console.ts gains buildMessageLogWrite so the log insert can join the batch. - Tests: fake D1-style batch handle (records SQL, executes all-or-nothing) asserting each path issues exactly one batch with the expected statement kinds, batch failure leaves no orphan rows, and bare handles still run sequentially. Failure injection now fires at statement execution (mid-atomic-unit) rather than at build time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Warning Review limit reached
More reviews will be available in 56 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64ad3eafa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| db | ||
| .select({ agentId: channelMembers.agentId, agentName: agents.name }) | ||
| .from(channelMembers) | ||
| .innerJoin(agents, eq(channelMembers.agentId, agents.id)) | ||
| .where(eq(channelMembers.channelId, channelId)), |
There was a problem hiding this comment.
When channel membership changes after this pre-read but before runAtomicWrites starts, the committed delivery rows are based on stale membership. For example, if an agent leaves the channel in that window, the later batch/transaction can still insert a delivery for them and fan out delivery.accepted for a message that was committed after they left; before this change, the Node path read members inside the transaction after BEGIN IMMEDIATE, so channel membership writes could not interleave. Consider deriving deliveries in the write itself (for example, INSERT ... SELECT from current channel_members) or otherwise acquiring the write lock before reading recipients.
Useful? React with 👍 / 👎.
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
…icity # Conflicts: # memory/workspace/.relay/state.json # packages/engine/src/__tests__/atomicity.test.ts # packages/engine/src/adapters/node/database.ts # packages/engine/src/engine/dm.ts # packages/engine/src/engine/groupDm.ts # packages/engine/src/engine/message.ts # packages/engine/src/engine/receipt.ts # packages/engine/src/engine/thread.ts # packages/engine/src/ports/database.ts # packages/engine/src/ports/index.ts
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
1 participant
Important
Stacked on #174 (
feature/transactional-write-paths). Do not merge before #174; this PR will be retargeted tomainonce #174 merges.Problem
#174 gave the five multi-statement write paths transactional atomicity, but only where the adapter can attach
withTransaction— i.e. the Node better-sqlite3 self-host path. The hosted Cloudflare deployment runs on D1, which has no interactive transactions, so cloud kept the non-atomic sequential path: a crash between themessagesinsert and thedeliveriesinserts still leaves a message with no delivery rows.D1 does execute
db.batch([...])atomically, and drizzle'sDrizzleD1Databaseexposesbatch()natively (verified against drizzle-orm 0.45.2 typings and runtime).Design
Capability detection —
runAtomicWrites(db, statements)resolves in priority order:withTransaction(explicitTransactionCapability, attached by the Node adapter) — statements run sequentially inside one transaction.batch(D1-styleBatchCapability, detected structurally viatypeof db.batch === 'function') — statements run as one atomic batch.Duck-typing was chosen for
batchdeliberately: the hosted handle is constructed in the cloud repo as a plaindrizzle(env.DB, { schema }), so structural detection gives the hosted engine atomicity with zero cloud-side changes. It cannot misfire: drizzle implementsbatch()per driver and only for drivers whose backend executes batches atomically — the better-sqlite3 drizzle instance has nobatchmember at all (and Node attacheswithTransaction, which takes priority anyway).BatchCapabilityis exported so a non-drizzle adapter can attach an implementation explicitly later.Write-path restructure — each of the five paths (channel send, DM send, group DM send, thread reply,
markRead) now does all reads (member lists, agent names, attachment details, auth checks) before the writes, then handsrunAtomicWritesa pure list of built-but-unexecuted drizzle statements (AtomicWrite). Drizzle builders are lazy thenables, so the same list works under a transaction, a batch, or sequential execution.All five paths batch fully — no fallbacks needed. No write depends on a prior write's DB-returned value: IDs are app-generated snowflakes, and the
logMessagebody equals the request text. The two places that looked dependent were restructured:message.tsread attachment details by joining themessage_attachmentsjunction rows it had just inserted; it now readsfilesby id directly (same data, order preserved), since the junction rows don't exist yet mid-batch..returning()rows (used forcreated_atin responses) are recovered from the per-statement batch results by index — D1'sbatchreturns each statement's mapped result.runAtomic(fn)is removed in favor ofrunAtomicWrites; it had no callers outside the five paths and a callback shape can't be batched. Business logic, fanout (still outside the atomic unit, in routes), and the EventQueue port are untouched. The Node adapter is unchanged apart from a comment.Tests
atomicity.test.tsnow covers all three handle shapes (13 tests, all passing):withTransaction, attaches abatch()that records each batch's SQL and executes it all-or-nothing inside one underlying SQLite transaction. Asserts: each of the five paths issues exactly one batch with the expected statement kinds;.returning()rows flow back through batch results; a mid-batch failure leaves no orphan message/delivery/log rows.Full engine suite + dependents via
turbo test --filter=...@relaycast/engine(4 tasks),tsc --noEmit, andeslint src/all pass.🤖 Generated with Claude Code