feat(sqlite): coordinate actor-local transactions

[NathanFlurry]

No description provided.

[NathanFlurry]

Stack for rivet-dev/rivet

• #5419 feat(rivetkit): add experimental Actor Runtime Socket
• #5420 feat(sqlite): coordinate actor-local transactions 👈
• main

Get stack: forklift get 5420
Push local edits: forklift submit
Merge when ready: forklift merge 5420

_{change lnpumvwz}

[claude]

Review: feat(sqlite) coordinate actor-local transactions

Reviewed the transaction coordination core (rivetkit-core/src/actor/sqlite/tx.rs), the envoy-client connection-session plumbing, and the TypeScript driver/database layer (common/database/*, db/drizzle.ts, registry runtime adapters).

Bugs

1. tx.close() inside a db.transaction() callback closes the whole actor database, not just the transaction (high confidence, confirmed by reading the diff)

• rivetkit-typescript/packages/rivetkit/src/common/database/mod.ts: createClient(target, transactionScoped) is used both for the top-level client and for the tx object passed into transaction/migration callbacks (const tx = createClient(transaction, true)). Its close field ignores target/transactionScoped entirely and always closes the outer db and sets the shared closed flag:

  close: async () => {
      if (!closed) {
          closed = true;
          await db.close();
      }
  },

  RawAccess.transaction's callback is typed (tx: RawAccess) => ..., and RawAccess.close is non-optional, so any code inside a transaction callback that calls tx.close() (a plausible cleanup call given the type) tears down the entire actor's SQLite connection mid-transaction. The subsequent transaction.commit() then fails, the rollback attempt is silently swallowed (catch {}), and the actor's db is permanently closed thereafter.
• The identical pattern is duplicated in rivetkit-typescript/packages/rivetkit/src/db/drizzle.ts (drizzleDb.close inside createDrizzleClient, same unconditional nativeDb.close()).
• Suggested fix: the transaction-scoped client should omit close, or have it throw an explicit "cannot close a transaction-scoped handle, use commit()/rollback()" error instead of silently closing the parent connection.
• No test exercises this path in mod.test.ts / drizzle.test.ts.

2. Transaction-expiry epoch bump misattributes TransactionExpiredError to unrelated callers (rivetkit-rust/packages/rivetkit-core/src/actor/sqlite/tx.rs)

• begin_transaction_inner captures epoch before waiting on gate.write_owned() (around tx.rs:197), then after acquiring the lock checks whether epoch changed and if so returns transaction_expired_error(state.last_expired_timeout) (tx.rs:207-213).
• expire_transaction_inner bumps the epoch twice, once before rollback and once after (with an explicit comment: "Calls submitted both before and during expiry cleanup must observe a changed epoch and fail; only calls submitted after cleanup may proceed").
• This is deliberate, but the effect is that a completely unrelated begin_transaction/other call that merely queued behind the gate while a different transaction expired gets rejected with an error that looks like its own transaction expired, populated from the other transaction's timeout. Callers can't distinguish "your request actually timed out" from "please just retry, an unrelated transaction was cleaning up." Consider a distinct error/retry signal (e.g. a Busy/Retry variant) instead of overloading TransactionExpiredError for this case.
• Related test gap: transaction_gate_serves_registered_waiters_in_fifo_order (tests/sqlite.rs) drives the gate directly and doesn't bump epoch, so this cross-caller misattribution path isn't covered by a test.

3. NativeCloseGate fences individual calls, not the full open-transaction span (rivetkit-typescript/packages/rivetkit/src/common/database/native-database.ts)

• wrapTransaction's exec/execute/commit/rollback each independently do gate.enter() / release() per round trip (native-database.ts ~394-452). NativeCloseGate.close() only waits out calls that are active at the moment close() is invoked (#active > 0); it doesn't hold a reservation across a transaction's full beginTransaction() → commit()/rollback() lifetime.
• If the actor is closed while a db.transaction(async (tx) => { await someNonDbAwait(); await tx.execute(...) }) callback is between two calls, gate.close() can flip #closed = true in the gap, and the next call inside the transaction throws a generic "Database is closed" error rather than being cleanly finalized, with no explicit rollback attempted from the TS side. Worth confirming the Rust-side coordinator's own connection-loss/expiry handling fully covers cleanup in this window, and adding a test that races close() against an in-flight transaction.

Minor / nits

• TransactionExpiredError::owner(_key, timeout) and ::parked(timeout) (tx.rs) are identical bodies; the unused _key param on owner implies per-transaction detail that isn't actually included.
• ws_send_for_session's new StaleSession early-return branch (engine/sdks/rust/envoy-client/src/connection/mod.rs) skips recording ws_tx_lock_hold_duration_seconds, unlike every other early-return path in that function (one of which even has a comment noting it observes duration on the early-return path). Minor observability gap on exactly the new code path operators will want visibility into.
• RemoteSqliteConnectionSessionLostError's Display (envoy-client/src/utils.rs) only prints expected, omitting current, which would help diagnose reconnect-storm scenarios.
• No test for a panic (as opposed to future-cancellation, which is already well covered) occurring inside finish_transaction_inner/expire_transaction_inner after the operation lock is acquired but before release_transaction runs; the transaction would only be freed by its own timeout watchdog, a bounded but real stuck-lock window.

What looks solid

• No wire-protocol/BARE schema changes were needed or made; connection_session is documented as client-local and never crosses the wire, so PROTOCOL_MK2_VERSION/PROTOCOL_VERSION were correctly left unbumped.
• The "sent remote SQL requests fail indeterminate on disconnect" invariant is untouched and still holds; the new session-affinity check only prevents unsent-request replay-on-reconnect from being sent on a stale connection.
• rivetkit-napi / rivetkit-wasm bindings stay pure marshaling, no lifecycle logic duplicated out of rivetkit-core.
• No inline #[cfg(test)] mod tests added in tx.rs/mod.rs; error artifacts match the existing flat sqlite.*.json shape; no _ => wildcard matches introduced on the relevant enums; cancellation-safety (dropped futures) around begin/commit/rollback is well tested via run_detached_transaction_task.
• vi.waitFor usages in the new driver tests carry the required adjacent justification comments.

Overall this is a substantial, mostly well-tested addition, but item #1 (tx.close() closing the whole actor db) should be fixed before merge since it's a straightforward, likely-to-be-hit footgun given the type signature invites it.

[railway-app]

🚅 Deployed to the rivet-pr-5420 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
website	😴 Sleeping (View Logs)	Web	Jul 16, 2026 at 6:44 am
kitchen-sink	😴 Sleeping (View Logs)	Web	Jul 16, 2026 at 6:44 am
frontend-inspector	😴 Sleeping (View Logs)	Web	Jul 16, 2026 at 6:43 am
frontend-cloud	😴 Sleeping (View Logs)	Web	Jul 16, 2026 at 6:41 am
ladle	✅ Success (View Logs)	Web	Jul 16, 2026 at 6:34 am
mcp-hub	✅ Success (View Logs)	Web	Jul 16, 2026 at 6:33 am