fix(rivetkit-core): report actor crashes as a sleep intent instead of a stop intent

[MasterPtato]

No description provided.

[MasterPtato]

Stack for rivet-dev/actors

• #5685 fix(universaldb): stop postgres commits from starving the co …
• #5670 fix(rivetkit): surface native runtime load failures instead …
• #5667 fix(envoy-client): fail sent vfs sqlite requests on disconne …
• #5666 fix(rivetkit-core): report actor crashes as a sleep intent i … 👈
• #5668 fix(envoy-client): ack start commands immediately instead of …
• main

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

_{change krtqytxz}

[claude]

Review

Solid change: the split of sleep_actor/stop_actor at the EnvoyHandle layer (stop_actor no longer accepts error) makes "a crash can never be reported as a destroy intent" a compile-time property instead of a convention, and the new rivetkit-core tests (repeated_stop_with_error_sends_one_sleep_intent, destroy_after_stop_with_error_still_sends_stop_intent, sleep_then_stop_with_error_reports_the_crash) cover the tricky interleavings well. The doc comments on stop_with_error/mark_errored_stop_requested/sleep_actor/stop_actor clearly explain why, per repo convention.

Likely CI break: stale is_destroy_requested() assertions in rivetkit (Rust wrapper)

rivetkit-rust/packages/rivetkit/src/start.rs has tests that don't look updated for this change:

• run_actor_error_requests_errored_stop (~line 1548)
• run_actor_panic_requests_errored_stop (~line 1579)

Both poll ctx.inner().is_destroy_requested() in a loop waiting for it to become true after run() fails/panics (which calls ctx.stop_with_error(...)), then assert it's true.

Before this PR, request_stop's errored branch set destroy_requested = true (shared with the deliberate-destroy path), so is_destroy_requested() correctly reported the crash. After this PR, the errored branch sets sleep_requested instead and never touches destroy_requested (context.rs:577-601). So is_destroy_requested() should stay false for the entire crash path now, meaning:

• the polling loop spins all 1000 iterations without breaking early, and
• the final assert!(ctx.inner().is_destroy_requested(), ...) should now fail.

This can't be trivially patched by swapping in the core-side equivalent: ActorContext::sleep_requested() (rivetkit-core/src/actor/context.rs:1465) is pub(crate) to rivetkit-core, so it isn't visible from the sibling rivetkit crate. The parallel test in rivetkit-core/tests/task.rs was correctly updated in this PR to assert ctx.sleep_requested() / !ctx.is_destroy_requested(), but the equivalent assertions in rivetkit's start.rs appear to have been missed. run_actor_shutdown_error_is_not_reported_as_crash (~line 1607) still passes since it only asserts the negative, but its meaning is now weaker since no crash path sets is_destroy_requested() anymore.

If rivetkit needs to distinguish "crashed and going to sleep" from "actually destroying" post-crash, this suggests a small public accessor is missing on ActorContext rather than just fixing the test expectations.

Minor: escalating a queued crash to destroy() can send a duplicate ActorIntentStop

In request_stop (context.rs:565-637), the same-error-branch case (repeated_stop_with_error_sends_one_sleep_intent) is careful to return early instead of double-scheduling request_stop_from_envoy when an error is already queued. The destroy-escalation case doesn't get the same treatment: if destroy() is called while an earlier stop_with_error(...) is still queued (not yet sent to the envoy), both calls schedule their own request_stop_from_envoy() task. destroy() clears stop_error before scheduling, so both scheduled tasks observe error = None and both call stop_actor(...), i.e. two ActorIntentStop events get sent to the envoy for one destroy.

This is likely harmless in practice, the engine's if let Transition::Running guard in actor2/mod.rs makes a second ActorIntentStop/ActorIntentSleep for the same generation a no-op, but it's an avoidable duplicate message, and the new destroy_after_stop_with_error_still_sends_stop_intent test (tests/sleep.rs) doesn't call assert_no_further_intent the way its sibling repeated_stop_with_error_sends_one_sleep_intent does, so this duplicate isn't caught by the test suite. Might be worth either short-circuiting the second schedule (mirroring the same-error-branch fix) or adding the missing assertion if the duplicate is considered acceptable.

Nit

sqlite/mod.rs's report_sqlite_worker_fatal calls config.handle.sleep_actor(...) directly on the EnvoyHandle, bypassing ActorContext::stop_with_error (this was already the case pre-PR for stop_actor, just updated to the new method). Worth double-checking this bypass is intentional going forward: it means the local sleep_requested/stop_error bookkeeping never reflects this crash path, so a caller checking ctx.sleep_requested() locally won't see it even though the envoy has been told.

---

🤖 Generated with Claude Code