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)
frontend-cloud	❌ Build Failed (View Logs)	Web	Apr 8, 2026 at 8:55 pm
kitchen-sink	❌ Build Failed (View Logs)	Web	Apr 8, 2026 at 8:54 pm
frontend-inspector	🕒 Building (View Logs)	Web	Apr 8, 2026 at 8:54 pm
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]

• 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 formatting/style cleanups across epoxy, a new pegboard snapshot test scenario, and a few behavioral fixes in pegboard-runner and pegboard-outbound. The scope is reasonable and the logic changes are generally sound.

---

Positives

Parallel preloaded_kv fetch (pegboard-outbound/src/lib.rs)
Good improvement. Moving fetch_preloaded_kv into the tokio::try_join! alongside pool_res and namespace_res eliminates a sequential latency point. The let db = ctx.udb()? binding moved before the join is correct since it is needed inside the closure.

Eviction LifecycleResult refactoring (pegboard-runner)
The new Evicted variant cleanly separates eviction (an expected, controlled shutdown) from unexpected errors. Previously, both tasks returned Err(WsError::Eviction) which caused the runner to hit the error path and unconditionally run ClearIdx. Now ClearIdx is skipped on eviction, which appears to be the intended behavior.

Metric name prefixes (pegboard-runner/src/metrics.rs)
Adding _runner_ to the metric names is a good hygiene fix to avoid naming collisions across pegboard packages.

Snapshot test scenario
The pb-actor-v1-pre-migration scenario is valuable for verifying that sleeping Rivet Actors created before envoys were introduced survive the migration correctly.

---

Issues / Observations

1. Metric renames are a breaking change for monitoring

pegboard_event_multiplexer_count   -> pegboard_runner_event_multiplexer_count
pegboard_ingested_events_total     -> pegboard_runner_ingested_events_total

Any Grafana dashboards, alerting rules, or recording rules referencing the old metric names will silently break. If these metrics are wired into production dashboards, note them in the PR description or handle the migration (e.g. temporarily keep old names with a deprecation label).

2. Potential flakiness in the snapshot scenario (pb_actor_v1_pre_migration.rs:51)

tokio::time::sleep(Duration::from_secs(5)).await;

A fixed 5-second sleep assumes the actor reliably reaches sleep state in time. Under CI load or on slower machines this could be flaky. Polling for the actor workflow state would be more robust. That said, if this generator is run once manually rather than in CI, it is acceptable as-is.

3. Eviction priority not explicit in lifecycle match (lib.rs:226-237)

The match handles Aborted preference explicitly but not Evicted:

(Ok(res), Ok(LifecycleResult::Aborted), _) => Ok(res),
(Ok(LifecycleResult::Aborted), Ok(res), _) => Ok(res),
// Unlikely case
(res, _, _) => res,

Consider (Ok(Closed), Ok(Evicted), _): this falls through to the catch-all and returns Ok(Closed). The eviction check if let Ok(LifecycleResult::Evicted) = &lifecycle_res then misses it, and ClearIdx runs. In practice both tasks should see the eviction signal and both return Evicted, so this is likely a non-issue. An explicit match arm or a comment explaining why the catch-all is safe here would help future readers.

4. Removal of replica_ids() (test_cluster.rs)

The #[allow(dead_code)] annotation already suggested it was unused, so removal is fine. Just confirming there are no callers outside this file.

5. Minor: comment style (lib.rs)

Per CLAUDE.md conventions, comments should be complete sentences. // Clear alloc idx if not evicted should be // Clear the alloc index if the runner was not evicted.

---

Verdict

The logic changes are correct and the refactoring is well-motivated. The main thing to address before merge is awareness of the metric rename impact on dashboards/alerting. The eviction match ordering edge case is worth a comment even if it cannot occur in practice.

Generated with Claude Code

[MasterPtato]

Merge activity

• Apr 8, 8:52 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 8, 8:54 PM UTC: Graphite rebased this pull request as part of a merge.
• Apr 8, 8:55 PM UTC: @MasterPtato merged this pull request with Graphite.