Skip to content

feat(rivetkit): add experimental Actor Runtime Socket - #5419

Merged
NathanFlurry merged 2 commits into
mainfrom
sqlite-uds
Jul 18, 2026
Merged

feat(rivetkit): add experimental Actor Runtime Socket#5419
NathanFlurry merged 2 commits into
mainfrom
sqlite-uds

Conversation

@NathanFlurry

@NathanFlurry NathanFlurry commented Jul 16, 2026

Copy link
Copy Markdown
Member

No description provided.

@NathanFlurry

NathanFlurry commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Stack for rivet-dev/rivet

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

change slvoypqn

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review: Actor Runtime Socket

Reviewed the full diff (protocol schema, rivetkit-core endpoint implementation, NAPI/wasm bridging, TS config/registry wiring, docs, and both Rust and TS test suites). This is a large, well-scoped addition and it's held to a high bar — I didn't find correctness bugs. Notes below are mostly confirmations and a couple of minor/non-blocking observations.

Architecture & conventions

  • Correctly layered per CLAUDE.md: all lifecycle/coordination logic lives in rivetkit-core (actor_runtime_socket.rs), NAPI (actor_context.rs) is a pure binding (provision_actor_runtime_socket just calls core and maps the error), and the wasm/TS side fails closed with a structured RivetError("actor_runtime_socket", "unsupported", ...) rather than silently no-op'ing — matches the fail-by-default rule.
  • New wire protocol correctly uses vbare/OwnedVersionedData with an embedded version prefix (engine/sdks/schemas/actor-runtime-socket-protocol/v1.bare, versioned.rs), not raw serde_bare, per the protocol-versioning convention.
  • CLAUDE.md was updated with the trust-boundary rationale for the socket (owner-only Unix socket, app-local trusted peer, no bearer token) — this preempted what would otherwise be a "no auth on this listener" security flag. The 0700 directory / 0600 socket permissions back that model up.
  • Docs page + limits.mdx/sqlite.mdx updates + sitemap/mod.ts entry are all present, satisfying the docs-sync requirement for new pages.

Concurrency correctness (the highest-risk part of this PR)

  • active_transaction's pending-lease wait (actor_runtime_socket.rs) correctly avoids the classic Notify missed-wakeup: it calls notified.as_mut().enable() before re-checking state, and there's a dedicated PendingWaitTestHook plus pending_lease_waiter_cannot_miss_begin_completion test exercising exactly that race. This matches the project's "arm the notification before re-checking" rule.
  • Writer/request_ids lock ordering is consistent both at admission and at response-completion time (documented inline), which is what makes immediate request-ID reuse safe — backed by completed_request_id_is_immediately_reusable and duplicate_request_id_goes_away tests.
  • BEGIN/COMMIT/ROLLBACK are deliberately kept cancellation-safe by running the coordinator call in its own tokio::spawn decoupled from the per-request response future, so a client disconnect during BEGIN can't orphan a transaction — covered by disconnect_during_begin_finishes_rollback_without_orphaning_transaction.
  • Shutdown draining (listener and per-connection) has bounded timeouts with abort fallback, and there's an explicit test for the drain-then-force-abort path (shutdown_answers_inflight_request_and_listener_removes_private_socket).

Minor / non-blocking observations

  • In ActorRuntimeSocketEndpoint::shutdown, the socket file is removed once by serve_listener itself on normal exit and again unconditionally in shutdown() after awaiting the task. The second removal is a no-op in the common path (guarded by the NotFound check), but it's slightly redundant — worth a one-line comment noting it's a safety net for the abort-timeout path specifically, since that's the only case where it actually does something.
  • configured_max_frame_bytes() re-reads and re-parses the RIVET_ACTOR_RUNTIME_SOCKET_MAX_FRAME_BYTES env var on every ActorRuntimeSocketEndpoint::new (i.e., per actor construction), rather than caching process-wide like process_socket_dir() does. Not a correctness issue given actor construction isn't hot, but it's an inconsistency in caching strategy between the two env-derived settings in the same file.
  • process_socket_dir()'s per-process temp directory is never cleaned up on process exit (only individual .sock files are removed as actors shut down). Likely fine since it lives under XDG_RUNTIME_DIR/tmp and is empty at that point, but worth confirming this doesn't accumulate empty directories across long-lived hosts that create/destroy many processes.

Test coverage

Test coverage is notably thorough on both sides:

  • Rust: handshake version-skew in both directions, duplicate/reused request IDs, GoAway suppression of late responses, oversized frame handling (both request and response direction), transient BEGIN failure retry, transaction expiry, and directory permission/retry behavior.
  • TS driver test (actor-db.test.ts): a full raw-socket integration test that opt-in/opt-out/wasm-unsupported/remote-backend rejection paths, cross-connection transaction isolation (socket vs. in-actor db.transaction), lease expiry, and socket lifecycle across sleep/destroy/regeneration. All vi.waitFor calls carry the required justification comment on the preceding line.

Overall this looks solid and ready modulo the minor notes above, which are polish items rather than blockers.

@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 9a64d57 to 6429b51 Compare July 16, 2026 07:26
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch 2 times, most recently from 2db45ba to 3672db2 Compare July 16, 2026 20:45
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 3672db2 to 4e26bfd Compare July 16, 2026 20:48
@NathanFlurry
NathanFlurry force-pushed the sqlite-uds branch 4 times, most recently from 7d0389d to 5118687 Compare July 16, 2026 21:23
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 4e26bfd to 663a5d0 Compare July 18, 2026 03:43
@NathanFlurry NathanFlurry changed the title feat(rivetkit): expose actor sqlite over unix sockets feat(rivetkit): add experimental Actor Runtime Socket Jul 18, 2026
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 663a5d0 to 32395fe Compare July 18, 2026 03:47
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 32395fe to 22376f6 Compare July 18, 2026 03:49
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from 22376f6 to d0fae14 Compare July 18, 2026 04:50
@NathanFlurry
NathanFlurry force-pushed the stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz branch from d0fae14 to c5083e2 Compare July 18, 2026 04:55
@NathanFlurry
NathanFlurry changed the base branch from stack/feat-sqlite-isolate-cross-job-transactions-with-leases-lnpumvwz to main July 18, 2026 05:46
@NathanFlurry
NathanFlurry merged commit f3c4ff1 into main Jul 18, 2026
15 of 18 checks passed
@NathanFlurry
NathanFlurry deleted the sqlite-uds branch July 18, 2026 05:46
@railway-app
railway-app Bot temporarily deployed to rivet-frontend / production July 18, 2026 05:46 Inactive
@railway-app
railway-app Bot temporarily deployed to rivet-frontend / preview July 18, 2026 05:46 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant