test(engine): stabilize runner and envoy suites

[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

• test(engine): cover envoy Sleep crash policy wake and recovery paths #4816
• refactor(sqlite-storage): extract DBHead to vbare-versioned protocol crate #4815
• refactor(sqlite-storage): rename SqliteOrigin variants for clarity #4814
• fix(sqlite-storage): allow invalidate_v1_migration on non-migrating META #4813
• test(engine): stabilize runner and envoy suites #4809 👈 (View in Graphite)
• fix(pegboard): reject empty actor kv values #4808
• fix(pegboard): stabilize actor2 validation and lifecycle #4804 : 1 other dependent PR (#4805 )
• fix(pegboard): decode envoy actor eviction keys #4803
• fix(pegboard-envoy): subscribe before registering envoy #4802
• fix(envoy-client): clear processed_command_idx after ack send #4812
• fix(envoy-client): dedupe replayed commands by index #4811
• fix(envoy-client): complete command and request lifecycles #4801
• fix(envoy-protocol): advertise current protocol version #4800
• fix(guard): serialize gateway actor keys correctly #4655
• fix(rivetkit-native): expose full hibernation metadata to JS #4657
• chore(rivetkit): remove inline tests #4668
• fix(api): subscribe before namespace workflow dispatch #4654
• fix(rivetkit): restore hibernatable sockets and hydrate serverless starts #4658
• fix(pegboard): persist and replay hibernating requests #4656
• fix(rivetkit): keep internal error exposure behavior consistent #4661
• test(rivetkit): stabilize actor db driver tests #4664
• fix(test): stabilize lifecycle, sleep, queue, and run edge cases #4660 : 1 other dependent PR (#4665 )
• test(rivetkit): re-enable gateway URL and direct-registry coverage #4659
• fix(rivetkit): add lifecycle error retry and gateway HTTP routing #4666
• fix(rivetkit-core): remove legacy timeouts #4799
• docs: add replay guidance notes #4796
• fix(inspector): stream patched state updates #4795
• fix(rivetkit): refresh stale actor handles #4794
• test(rivetkit-napi): move private tests to crate root #4793
• test(rivetkit-core): move private tests to crate root #4792
• test(runner-configs): assert envoy protocol version #4791
• fix(serverless): split metadata protocol errors #4789 : 1 other dependent PR (#4790 )
• chore(sqlite): add open close lifecycle for envoy actors #4788
• fix(rivetkit): share lifecycle shutdown grace #4810
• 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 4809 Code Review: stabilize runner and envoy suites

This is a DRAFT PR. Review provided for early feedback.

Overview

Large test-infrastructure overhaul (11,713 additions, 816 deletions, 43 files). The PR:

1. Rewrites test_runner.rs — Native Pegboard Runner WebSocket client speaking /runners/connect directly instead of bridging through Envoy. Gives accurate coverage for the legacy runner path.
2. Refactors test_envoy.rs — Wraps rivet-test-envoy into an in-process Envoy struct.
3. Adds new Envoy test suites (tests/envoy/) — alarm, KV CRUD/list/drop/delete-range/misc, lifecycle, actor API, auth.
4. Stabilizes flaky runner tests — approximately 40 tests marked ignored with detailed explanations; adds retry logic for setup helpers.
5. Removes actor_import_export_e2e.rs.

The direction is correct: separating the two connection paths into distinct test clients ensures envoy tests exercise the envoy protocol and runner tests exercise the runner protocol.

Issues

Critical — Spin-wait in Runner::wait_ready() violates CLAUDE.md

CLAUDE.md prohibits: "Never poll a shared-state counter with loop { if ready; sleep(Nms).await; }. Pair the counter with a tokio::sync::Notify..."

The current implementation polls every 25ms using AtomicBool. Fix by adding a tokio::sync::Notify field, signalling it in handle_message when ToClientInit arrives, and awaiting notified() in wait_ready().

Medium — actors mutex held across on_stop().await

In Runner::handle_stop_actor(), the actors mutex guard remains alive while on_stop().await executes. This blocks concurrent handle_start_actor and has_actor calls. Fix by scoping the lock acquisition so the guard drops before calling the callback: take the actor out of the map inside a block that owns the guard, then call on_stop() after the block ends.

Medium — Misleading _kv_tx name in test_envoy.rs

In TestEnvoyCallbacks::actor_config(), the variable _kv_tx is moved into ActorConfig::kv_request_tx. It is not unused. The underscore prefix signals "intentionally unused" to readers and clippy. Rename to kv_tx.

Medium — Unexplained behaviour changes in actors_scheduling_errors.rs

The assertion "no_runners_available" is changed to "no_runner_config_configured" without explanation. Also, actor_crash_destroy_policy flips from asserting "no error on destroy" to asserting "Crashed error is set with a crash message". If these reflect real engine behaviour changes they should be documented in the PR description or in a comment.

Low — std::sync::Mutex in async code

Test actors use std::sync::Mutex for notify-sender patterns. CLAUDE.md requires parking_lot::Mutex for forced-sync contexts in async code. No guard crosses an .await so the risk is low, but the rule should be followed consistently.

Low — Approximately 40 ignored tests with no tracking path

Each ignore comment names the failure symptom clearly. However no linked issue tracks re-enabling them. Several failures ("times out in full engine sweep") suggest a systemic setup problem; fixing it would unblock many tests at once. Consider linking a tracking issue in the ignore reason.

Low — Deprecated polling helper used in new envoy alarm tests

wait_for_actor_wake_polling documents itself as "DEPRECATED for other tests" in its docstring, yet new envoy alarm tests (basic_alarm, alarm_in_the_past, etc.) call it. Use the event-driven wait_for_actor_wake_from_alarm where possible to reduce timing flakiness.

Positive observations

• Excellent test coverage in the new envoy suite: alarm edge cases (overdue, clear, replace, multiple sets, multi-cycle), KV CRUD/batch/list/range/drop, auth rejection via WebSocket close frame, pagination regression tests.
• Correct separation: the native Runner client speaks the real runner protocol (/runners/connect), catching regressions the old envoy-bridged approach missed.
• Retry logic in setup_test_namespace and upsert_normal_runner_config (60s timeout, 100ms backoff) addresses "replica not configured yet" races without papering them over.
• Deterministic DC ordering via dcs.sort_by_key prevents tests from depending on HashMap iteration order.
• Regression test alarm_overdue_during_sleep_transition_fires_via_reallocation is a well-documented, targeted regression for the Decision::Sleep overdue-alarm race.
• with_auth_admin_token on TestOpts makes auth tests self-contained.
• The bulk_create_actors name change (all actors share the same name, differentiated by key) better matches production actor semantics.

Minor style notes

• get_current_timestamp_ms() in actors_alarm.rs is a one-liner over rivet_util::timestamp::now(). Calling it directly removes the indirection.
• Comments in AlarmAndSleepActor::on_start say "gen 2" but the condition checks generation == 1 (runner protocol starts generations at 1). Align comments with the actual values.

Review generated by claude[bot]