feat(rust/engine-runner): minimal rust engine runner

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[railway-app]

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

Service	Status	Web	Updated (UTC)
frontend-cloud	😴 Sleeping (View Logs)	Web	Mar 5, 2026 at 5:17 am
frontend-inspector	😴 Sleeping (View Logs)	Web	Feb 28, 2026 at 4:13 am
website	😴 Sleeping (View Logs)	Web	Feb 26, 2026 at 5:04 am
mcp-hub	✅ Success (View Logs)	Web	Feb 24, 2026 at 8:18 am
ladle	❌ Build Failed (View Logs)	Web	Feb 24, 2026 at 8:17 am

[NathanFlurry]

• DO NOT MERGE: sandbox benches #4597
• fix(envoy-client): add tls #4595
• chore: fix driver test suite #4591
• chore: tunnel auth #4588
• chore: remove udb as a dependency of envoy-client #4587
• chore: revert actor v2 hack #4586
• fix: fix pool metadata protocol version refresh, rivetkit native ws #4585
• feat: US-001 - Define SqliteKv trait in rivetkit-sqlite-native #4584
• feat(rust/engine-runner): minimal rust engine runner #4289 👈 (View in Graphite)
• chore: delete kv channel #4583
• feat(engine): rust runner #4582
• chore: remove non-engine drivers #4574 : 1 other dependent PR (#4581 )
• feat: dynamic actors #4397
• fix: driver test suite & sqlite reliability #4559
• fix: remove serverless token #4572
• fix: misc token fixes #4569
• fix(pb): clean up actor stop decision handling #4566
• fix(envoy): use global instance, add signal handlers #4565
• chore: misc fixes, add pb snapshot test #4561
• fix(cache): make in memory cache global #4560
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Code Review: feat(rust/engine-runner): minimal rust engine runner

This PR adds a minimal but functional Rust engine runner with E2E tests and examples. Good work on establishing parity with the TypeScript runner. Here are my observations:

---

Summary

The PR introduces:

• A counter.rs example demonstrating SDK usage
• Shared E2E test infrastructure (tests/common/mod.rs)
• Four E2E test suites: runner mode, serverless, serverless upsert, and WebSocket (including hibernation)
• Refactoring of existing engine tests from TestActor to Actor trait
• CLAUDE.md documentation for engine runner parity requirements

---

Issues

Creating a new reqwest::Client per request

Per CLAUDE.md: "Never build a new reqwest client from scratch. Use rivet_pools::reqwest::client().await? to access an existing reqwest client instance."

In tests/common/mod.rs, new reqwest::Client instances are created repeatedly:

// create_actor
let client = reqwest::Client::new();
// get_actor
let client = reqwest::Client::new();
// actor_request_with_retry — inside a loop
let client = reqwest::Client::new();

The actor_request_with_retry method creates a new client on every retry iteration. At minimum the client should be constructed once outside the loop, and ideally a shared client should be stored on EngineProcess and reused across all calls. The pool-based helper (rivet_pools::reqwest::client()) may not be applicable in test code, but a single shared client per EngineProcess should be used instead.

BUILD_RESULT caches errors without retrying

ensure_engine_binary() uses OnceLock<Result<PathBuf, String>>. If the first build attempt fails (e.g. a transient compile error), that failure is cached permanently for the rest of the test run, meaning all subsequent tests will also fail without retrying the build. Consider documenting this caveat or not caching failure cases.

wait_for_port silently swallows errors during polling

Err(_) if start.elapsed() <= timeout => tokio::time::sleep(...).await,
Err(err) => return Err(err).with_context(...)

The underscore pattern discards the error during the wait period, so a persistent "address in use" or permission error will loop silently for 30 seconds before surfacing. Logging the transient error at debug level would make diagnosing stuck tests easier.

Unintentional removal of runner_id from lifecycle log

In actors_lifecycle.rs:

-tracing::info!(?actor_id, runner_id = ?runner.runner_id, "actor allocated to runner");
+tracing::info!(?actor_id, "actor allocated to runner");

The PR description does not mention this change. Was runner_id removed because the field no longer exists on the struct, or is this an accidental omission? If it was removed because the field was renamed or restructured, a brief note in the PR description would help reviewers.

---

Minor Notes

Magic numbers — Timeouts (30s, 10s for WebSocket close) and retry intervals (250ms) are repeated across multiple files. Extracting them to named constants at the top of common/mod.rs would make intent clearer and simplify future tuning:

const ACTOR_REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
const RETRY_INTERVAL: Duration = Duration::from_millis(250);

400 Bad Request retry — The retry loop retries on 400 without inspecting the body. A 400 is typically a client error (malformed request) rather than a transient condition, so retrying it unconditionally could mask bugs. If this is intentional (e.g. the guard can return 400 transiently during startup), a comment explaining why would be helpful.

Example does not actually run — examples/counter.rs constructs a runner and returns without calling runner.start().await. The println! message acknowledges this, but the example might be clearer if the call is shown commented-out in the source rather than omitted entirely.

#[allow(dead_code)] on public methods — Several methods in common/mod.rs carry this attribute. Since this is test-only code used selectively across test files, this is reasonable, but it is worth confirming each annotated method is actually used somewhere to avoid accumulating dead infrastructure.

---

What's Good

• Comprehensive test matrix: runner, serverless, serverless upsert, and WebSocket including hibernation/restore — all three runner modes are covered.
• Test infrastructure is well-structured with proper teardown (Drop on EngineProcess) and serialized execution via semaphore to avoid port conflicts.
• WebSocket hibernation test (websocket_hibernation_restore_runner_e2e) exercises a non-trivial production scenario and validates the SDK's metadata tracking design.
• CLAUDE.md parity documentation is concise and actionable.
• TestActor to Actor rename is a clean, mechanical refactor that reduces test-only type proliferation.

[MasterPtato]

Merge activity

• Apr 8, 8:52 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 8, 9:20 PM UTC: Graphite rebased this pull request as part of a merge.
• Apr 8, 9:20 PM UTC: @MasterPtato merged this pull request with Graphite.