fix(rivetkit-core): scope actor stops to generation to prevent cross-generation kills

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/actors

• #5679 fix(rivetkit-core): scope actor stops to generation to preve … 👈
• main

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

_{change rolqmtpo}

[railway-app]

🚅 Deployed to the actors-pr-5679 environment in rivet-frontend

Service	Status	Web	Updated
kitchen-sink	😴 Sleeping (View Logs)	Web	Sep 9, 2026 at 3:43 am UTC
frontend-inspector	😴 Sleeping (View Logs)	Web	Sep 9, 2026 at 3:42 am UTC
frontend-cloud	😴 Sleeping (View Logs)	Web	Sep 9, 2026 at 3:40 am UTC
ladle	✅ Success (View Logs)	Web	Sep 9, 2026 at 12:12 am UTC
mcp-hub	✅ Success (View Logs)	Web	Sep 9, 2026 at 12:12 am UTC
website	❌ Build Failed (View Logs)	Web	Sep 9, 2026 at 12:11 am UTC

[claude]

Review

Solid, focused fix. Generation-scoping transition_actor_to_stopping and threading generation through PendingStop correctly closes the race where a stale/out-of-order StopActor message (e.g. for generation N-1) could tear down the currently-active newer generation N. The three-way TransitionResult (Transitioned/Stale/NoMatch) and the park_pending_stop "keep highest generation" tie-break are both well reasoned, and the enum match is exhaustive per repo convention (no _ fallthrough).

Findings

1. Discarded stale ActorStopHandles are dropped implicitly rather than completed explicitly (minor/quality)

In the new TransitionResult::Stale arm of stop_actor (mod.rs:1125-1132) and the "stale pending stop... during startup" arm (mod.rs:867-875), the incoming ActorStopHandle (in the first case) / pending_stop.stop_handle (in the second) is dropped without calling .complete() or .fail(). This isn't a hang, envoy-client's finalize_stop (engine/sdks/rust/envoy-client/src/actor.rs:500) handles a closed oneshot channel gracefully, but it logs "actor stop completion handle dropped before signaling teardown result" as a warn! every time this (now-expected) stale-generation path is hit. Since this PR makes "stale generation stop" a normal, anticipated occurrence rather than a bug condition, routing through it silently will generate recurring warning noise that dilutes the signal that warning is meant to provide for genuinely unexpected drops elsewhere. Consider calling stop_handle.complete() explicitly in both branches to document intent and keep that warning meaningful.

2. Pre-existing gap, surfaced by this change: parked stops are never cleaned up if start_actor fails (informational, not introduced by this PR)

In the Err(error) => arm of the startup-completion match (mod.rs, just after the Ok(instance) branch shown in the diff), starting_instances is removed and the error propagated, but pending_stops for that actor_id is never checked/drained. If a stop got parked while this failed start was in flight, its PendingStop (and the ActorStopHandle inside it) stays in the map indefinitely, and this time it really would leak/hang the caller, since the map itself holds the sender alive. This existed before this PR (the old code parked via plain insert_async too), but the new generation-aware parking makes it more likely a stop legitimately intended for the next start attempt gets stuck here. Worth a follow-up to drain/resolve pending_stops on start failure, even if out of scope for this fix.

3. No test coverage for the new generation-scoping behavior (test coverage)

The diff has no accompanying tests for the Stale/NoMatch/re-park paths in transition_actor_to_stopping and park_pending_stop, despite this being exactly the kind of ordering/race logic that regresses silently. tests/registry.rs and tests/work_registry.rs already exercise RegistryDispatcher directly and would be a natural place to add a case that starts generation N, sends a stale stop for N-1, and asserts the active instance survives; plus a case that parks a stop for a not-yet-started N+1 and asserts it's applied once that generation starts. (Note: tests/modules/registry.rs currently isn't wired into any [[test]] target and references fields/types like active_instances/ActorLifecycle that no longer exist in src/, so it looks like dead code unrelated to this PR, not a place to extend.)

Other notes

• envoy_callbacks.rs's _generation -> generation rename and the extra generation argument threading are correctly wired end-to-end; found no other call sites of RegistryDispatcher::stop_actor that needed updating (the other stop_actor call sites in sleep.rs/sqlite/mod.rs are EnvoyHandle::stop_actor, an unrelated client-side method).
• park_pending_stop's tie-break (>= keeps the newest, same-generation duplicates overwrite the older parked stop) is reasonable and matches pre-existing duplicate-stop handling elsewhere (e.g. the already-Stopping branch in transition_actor_to_stopping).

Nothing here blocks merging; #1 is a cheap clarity/robustness win, #2 and #3 are reasonable follow-ups.

🤖 Generated with Claude Code