fix(rivetkit): share lifecycle shutdown grace

[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

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• fix(serverless): split metadata protocol errors #4789 : 2 dependent PRs (#4790 , #4791 )
• chore(sqlite): add open close lifecycle for envoy actors #4788
• fix(rivetkit): share lifecycle shutdown grace #4810 👈 (View in Graphite)
• fix(pegboard): move sqlite v1 migration into actor workflow #4787 : 1 other dependent PR (#4797 )
• test(rivetkit): cover c.db access from onWake/onSleep/onDestroy #4786
• fix(rivetkit-napi): run onMigrate before createState/onCreate/createVars #4785
• refactor(rivetkit-core): merge ready+started into lifecycle_started, drop redundant napi calls #4784
• fix(rivetkit): drain native sleep side tasks reliably #4782 : 1 other dependent PR (#4783 )
• fix(rivetkit): route raw request fetches to actors #4781
• fix(pegboard): refresh runner config after envoy connect #4778 : 2 other dependent PRs (#4779 , #4798 )
• chore(kitchen-sink): configurable endpoint #4766 : 2 other dependent PRs (#4754 , #4777 )
• refactor(rivetkit-core): rename StopReason to ShutdownKind and kill enum fallthroughs #4765
• chore(envoy-client): log command and event payloads #4751
• fix(rivetkit): minor sleep-cleanup follow-ups #4761
• docs(rivetkit): clean up stale setPreventSleep references in docs and JSDoc #4760
• docs(rivetkit): document keepAwake/waitUntil counter-arm race and surface bridge errors #4759
• test(engine): force overdue branch in alarm-during-sleep test with negative offset #4758
• fix(rivetkit-core): return stopping not starting when sleep/destroy called mid-shutdown #4757
• test(rivetkit): remove sleepWithPreventSleep fixture and tests #4756
• test(engine): add regression test for alarm-during-sleep-transition race #4755
• test(rivetkit-core): cover sleep/destroy guards and prevent_sleep no-op #4750
• docs: add keepAwake vs waitUntil table and internal sleep-sequence invariants #4749
• feat(rivetkit): expose c.keepAwake(promise) through native path #4748
• refactor(rivetkit): deprecate setPreventSleep to no-op in TypeScript #4747
• fix(rivetkit-core): enumerate non-drained counters in grace deadline warn log #4746
• fix(rivetkit-core): abort graceful cleanup task when grace deadline elapses #4745
• fix(rivetkit-core): error on sleep/destroy before startup or already-requested #4743
• fix(rivetkit-core): cap SerializeState shutdown timeout at 15s #4742
• refactor(rivetkit-core): remove prevent_sleep field and deprecate set_prevent_sleep to no-op #4741
• fix(api-public): remove orphan import_export actors module #4764
• main

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

[claude]

PR Review: fix(rivetkit): share lifecycle shutdown grace

Summary

The core fix is correct and well-motivated: collapsing the per-phase shutdown budgets (run handler, onSleep/onDestroy, shutdown tasks) into a single shared wall-clock grace period prevents the total actor teardown time from exceeding the engine's hard-kill threshold. The Rust task_lifecycle.rs test uses start_paused = true + tokio::time::advance for deterministic behavior — a good pattern.

---

Issues to Address Before Merge

1. _ => {} on a closed enum in task_lifecycle.rs (~line 88) — CLAUDE.md violation

match event {
    ActorEvent::SerializeState { reply, .. } => { ... }
    ActorEvent::RunGracefulCleanup { reply, .. } => { ... }
    _ => {}  // silently drops all other variants
}

CLAUDE.md explicitly prohibits _ => fall-through on Rust enums. ActorEvent is a closed internal enum with many variants. Silent drops could mask future bugs. Use _ => unreachable!() or enumerate all expected silent variants explicitly.

2. #waitForRunHandler receives a pre-computed duration instead of an absolute deadline (mod.ts ~line 980)

const shutdownTaskDeadlineTs = Date.now() + this.#getEffectiveSleepGracePeriod();
await this.#waitForRunHandler(shutdownTaskDeadlineTs - Date.now()); // fragile

Every other shutdown helper (#callOnSleep, #callOnDestroy, #waitShutdownTasks, #waitForIdleSleepWindow) receives the absolute deadlineTs and computes remaining budget internally. #waitForRunHandler computing remaining at the call site means any code inserted between the two lines silently eats into the budget. Change the signature to accept deadlineTs: number for consistency and robustness.

3. runStopTimeout override is still set in actor-driver.ts (~line 1066) but no longer consumed

Before this PR, runStopTimeout gave the run handler its own bounded budget. Now the run handler uses the shared grace period, but staticActor.overrides.runStopTimeout = drainMax is still being set. Either remove the dead assignment or document why it is retained.

4. Rust destroy-grace path still uses effective_on_destroy_timeout() while TS uses sleepGracePeriod for both modes

In task.rs start_grace:

ShutdownKind::Destroy => self.factory.config().effective_on_destroy_timeout(),

The TS side now uses sleepGracePeriod as the shared budget for both sleep and destroy. The Rust destroy path uses a separate timeout. The new test only covers ShutdownKind::Sleep. If this asymmetry is intentional, add a comment. If unintentional, add a ShutdownKind::Destroy symmetric test.

5. Test coverage regression from removing lifecycle.rs

The deleted lifecycle.rs contained 13 tests across startup, sleep-shutdown, and destroy-shutdown paths. The new task_lifecycle.rs provides 2 tests. Scenarios not visibly covered elsewhere include:

• startup_catches_run_handler_panics
• startup_restores_connections_and_processes_overdue_events
• startup_runs_on_migrate_before_on_wake_for_new_and_restored_actors
• sleep_shutdown_reports_error_when_on_sleep_fails
• destroy_shutdown_skips_idle_wait_and_disconnects_all_connections
• destroy_shutdown_reports_error_when_on_destroy_fails

Please confirm these are covered elsewhere, or add coverage for the destroy-mode equivalents at minimum.

---

Minor

• std::sync::Mutex in task_lifecycle.rs: NotifyOnDrop uses Drop (a forced-sync context), so parking_lot::Mutex is correct per CLAUDE.md rather than std::sync::Mutex.
• The fixture rename to use sleepGracePeriod: 100 clearly reflects the new behavior.
• The actor-driver.ts wiring of sleepGracePeriod as an override is the right approach.

---

Reviewed using the repository's CLAUDE.md conventions.

[NathanFlurry]

Landed in main as ad4ef06 (identical patch, different parent SHA due to rebase). Closing.