Skip to content

feat(rivetkit): redesign sandbox actor module#4424

Open
NathanFlurry wants to merge 3 commits intomainfrom
sandbox-actor-design
Open

feat(rivetkit): redesign sandbox actor module#4424
NathanFlurry wants to merge 3 commits intomainfrom
sandbox-actor-design

Conversation

@NathanFlurry
Copy link
Member

Summary

  • redesign the RivetKit sandbox actor module around a dedicated config and actor runtime layout
  • add sandbox provider parity coverage and driver-test-suite wiring for sandbox actors
  • update sandbox and actor input docs to cover the new API and dynamic provider flow

Testing

  • pnpm test sandbox-providers

@railway-app
Copy link

railway-app bot commented Mar 14, 2026

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

Service Status Web Updated (UTC)
frontend-inspector 😴 Sleeping (View Logs) Web Mar 14, 2026 at 6:03 am
frontend-cloud 😴 Sleeping (View Logs) Web Mar 14, 2026 at 6:02 am
website ❌ Build Failed (View Logs) Web Mar 14, 2026 at 5:53 am
mcp-hub ✅ Success (View Logs) Web Mar 14, 2026 at 5:51 am
ladle ❌ Build Failed (View Logs) Web Mar 14, 2026 at 5:51 am

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 14, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4424

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4424

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4424

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4424

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4424

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4424

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4424

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4424

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4424

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4424

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4424

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4424

commit: 791500a

@claude
Copy link

claude bot commented Mar 14, 2026

PR Review: feat(rivetkit): redesign sandbox actor module

This is a significant and well-structured PR that introduces a clean provider-plugin architecture for sandbox actors.

STRENGTHS

  • Clean provider abstraction: The SandboxActorProvider interface is minimal and well-designed, with create(), connectAgent(), destroy(), and optional wake() covering the full lifecycle.
  • Proxy action generation: Dynamically generating actor actions from SandboxAgent methods via buildProxyActions() is elegant and reduces boilerplate.
  • SDK parity testing: The parity test enforcing that SANDBOX_AGENT_ACTION_METHODS matches the actual SDK .d.ts is excellent and will catch drift automatically.
  • Post-destroy read access: Keeping read-only session/event access via SQLite after destroy is a thoughtful UX decision.
  • Documentation: The new sandbox.mdx is comprehensive with all three providers documented including option tables.

ISSUES AND CONCERNS

  1. Bug: Inconsistent state on partial provider init failure
    In actor/index.ts, when the actor wakes and the resolved provider name differs from state.providerName, it throws an error. However, if createProvider is async and throws partway through (e.g., missing env variable), state.providerName may not be set, leaving the actor in an inconsistent state on the next wake.

  2. Missing error cleanup in Docker provider
    In providers/docker.ts, if container.start() fails after container.create() succeeds, the created container is never cleaned up. Consider wrapping in a try/catch that calls container.remove({ force: true }) on failure.

  3. Idle timeout constants should be configurable
    The 30-second idle warning and 5-minute stale clear in actor/session.ts are hardcoded. These should be exposed as options in SandboxActorConfig since different use cases (quick scripts vs. long-running agents) have very different needs.

  4. Daytona provider: signed URL exposure risk
    In providers/daytona.ts, verify the preview URL used to connect the agent is only held in memory (in _vars) and not persisted to actor state, which could leak it.

  5. E2B provider: background process cleanup
    In providers/e2b.ts, if the actor is destroyed before the background process exits, zombie processes may remain in the E2B sandbox. Consider calling process.kill() during provider destroy().

  6. Missing onDestroy path test
    actor-sandbox.ts tests dispose, but confirm the actor onDestroy hook calls provider.destroy() and that force-eviction (not graceful dispose) is also exercised in tests.

  7. SANDBOX_AGENT_ACTION_METHODS maintenance burden
    The action method list in types.ts is manually maintained and checked by a parity test. Consider auto-generating this list from the SDK types at build time to eliminate the manual sync step.

MINOR NOTES

  • Comment style: A few comments use dash-separated fragments (e.g., // Create sandbox - if not already created). Per CLAUDE.md conventions, prefer complete sentences: // Create the sandbox if not already created.
  • Em dash usage in comments violates CLAUDE.md style. Replace with periods.
  • sandbox.mdx frontmatter: Verify the skill field is present (required per CLAUDE.md docs frontmatter rules).
  • Vercel example parity: Confirm the sandbox-coding-agent Vercel example was regenerated via ./scripts/vercel-examples/generate-vercel-examples.ts, or add skipVercel: true to the example package.json.

TEST COVERAGE

The driver test (actor-sandbox.ts) and provider mock tests (sandbox-providers.test.ts) are solid. The parity test is particularly valuable. Consider adding: a test for the provider mismatch error path on wake, and a test for post-destroy read-only access (sessions/events still queryable after destroy()).

OVERALL

High-quality work. The architecture is sound, the documentation is thorough, and the parity test is a great safeguard. Main actionable items: error cleanup in Docker on failed start, making idle timeouts configurable, and style nits per CLAUDE.md.

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