chore: misc fixes, add pb snapshot test

[MasterPtato]

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-4561 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
kitchen-sink	❌ Build Failed (View Logs)	Web	Apr 7, 2026 at 1:31 am
frontend-inspector	❌ Build Failed (View Logs)	Web	Apr 6, 2026 at 12:42 am
frontend-cloud	❌ Build Failed (View Logs)	Web	Apr 6, 2026 at 12:42 am
mcp-hub	✅ Success (View Logs)	Web	Apr 6, 2026 at 12:42 am
website	❌ Build Failed (View Logs)	Web	Apr 6, 2026 at 12:42 am
ladle	❌ Build Failed (View Logs)	Web	Apr 6, 2026 at 12:42 am

[MasterPtato]

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

• chore: remove non-engine drivers #4574 : 2 dependent PRs (#4581 , #4582 )
• 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 👈 (View in Graphite)
• fix(cache): make in memory cache global #4560
• main

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

[claude]

PR Review: chore: misc fixes, add pb snapshot test

Summary

This PR combines two categories of changes:

1. Mass rustfmt reformatting across engine/packages/epoxy/, engine/packages/guard/, engine/packages/pegboard*, and engine/sdks/. Nearly all diffs are line-wrapping adjustments with no semantic changes.
2. Substantive changes: new pegboard snapshot test (pb-actor-v1-pre-migration), KV channel protocol rename (ToServer -> ToRivet), eviction-lifecycle refactor in the pegboard runner, preload-KV concurrency optimization in the outbound handler, and query-path cache-key function relocation.

---

Code Quality

Positive

• The ToServer -> ToRivet rename across the BARE schema and all downstream consumers (Rust, TypeScript, sqlite-native) is consistent and complete. The auto-generated TypeScript SDK is updated in lockstep as required by project conventions.
• The preload-KV parallelization in engine/packages/pegboard-outbound/src/lib.rs is a meaningful improvement: fetch_preloaded_kv now runs concurrently with the pool and namespace lookups inside tokio::try_join! instead of sequentially after them.
• Moving query_path_cache_key into cache/pegboard_gateway.rs as build_cache_key_query_based improves cohesion; both the call site and the implementation now live in the same module area.
• The LifecycleResult::Evicted variant cleanly separates the eviction path from error-as-exception in pegboard-runner. The previous code returned Err(WsError::Eviction) from a task expected to return Ok.
• The metric name corrections (pegboard_event_multiplexer_count -> pegboard_runner_event_multiplexer_count, same for ingested_events_total) fix a namespace collision with the broader pegboard package metrics.

Issues

1. Fixed sleep in test (engine/packages/pegboard/tests/actor_v1_pre_migration.rs): The test calls tokio::time::sleep(Duration::from_secs(3)) unconditionally to wait for a workflow to reach sleeping state. This is fragile under CI load. Consider polling with a timeout loop matching the pattern used in other tests.

2. Removed preload-KV population without comment (engine/packages/pegboard/src/workflows/actor2/runtime.rs): The block that fetched and attached preloaded_kv to CommandStartActor at send time has been deleted. No comment explains this. Future readers will not know it was intentional. A one-line comment pointing to where preload KV is now injected (in the outbound handler path) would help.

3. Comment style (engine/packages/pegboard-runner/src/lib.rs): The new comment // Clear alloc idx if not evicted is a sentence fragment. Per project conventions, comments must be complete sentences. Suggest: // Clear the allocation index when the runner disconnects without being evicted.

---

Potential Bugs

• Eviction error propagation: When lifecycle_res is Ok(LifecycleResult::Evicted) it is overwritten with Err(WsError::Eviction.build()) and the alloc-index clear is skipped. This is the correct intent. However, the if let ... else structure is slightly unexpected; a match with explicit arms would be clearer and less error-prone for future maintenance.

• Unnecessary KV fetch on missing pool: fetch_preloaded_kv now runs in parallel with pool/namespace lookups, but if pool_res is None the function returns early and the KV result is discarded. Not a correctness bug, just a minor efficiency note.

---

Performance

The outbound handler change eliminates a sequential await for KV preload by merging it into the existing try_join!. This is a genuine latency improvement for actor start. All formatting-only changes have no performance impact.

---

Security

No security concerns. The ToServer -> ToRivet rename is purely cosmetic at the wire level. BARE tag ordinals are unchanged (ToRivetRequest = 0, ToRivetPong = 1 matching the old values), so binary compatibility with existing clients is preserved.

---

Test Coverage

• The new actor_v1_pre_migration snapshot test is a good regression guard for the workflow migration path. It exercises sending a Wake signal to a pre-envoy actor and verifies the workflow does not die.
• Snapshot data files are stored via Git LFS (correct pattern for binary test fixtures).
• The test relies on a 3-second fixed sleep, which is fragile under CI load (noted above).
• The kv_list_edge_cases and kv_operations tests are correctly updated to use rivet_envoy_protocol types instead of rivet_runner_protocol::mk2 types.

---

Minor Nits (non-blocking)

• The newly added .instrument(tracing::info_span!("read_actor_metadata_tx")) in preload.rs is a welcome observability addition.
• Variable rename x -> m in the KvMetadata iterator maps is a small readability improvement.
• The indentation fix in migration.rs corrects a pre-existing formatting error.

---

Verdict

Approved with minor suggestions. The substantive changes (eviction lifecycle, preload-KV parallelization, protocol rename, snapshot test) are all correct. Main actionable items:

1. Add a comment near insert_and_send_commands explaining that preload KV attachment was moved to the outbound handler path.
2. Consider replacing the hard tokio::time::sleep(3) in the snapshot test with a polling loop.
3. Fix the comment // Clear alloc idx if not evicted to be a complete sentence per project conventions.

---

Generated with Claude Code