fix(pegboard): persist and replay hibernating requests

[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(guard): serialize gateway actor keys correctly #4655
• fix(api): subscribe before namespace workflow dispatch #4654
• fix(rivetkit): restore hibernatable sockets and hydrate serverless starts #4658
• fix(rivetkit-native): expose full hibernation metadata to JS #4657
• fix(pegboard): persist and replay hibernating requests #4656 👈 (View in Graphite)
• 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
• test(rivetkit): re-enable gateway URL and direct-registry coverage #4659
• chore(engine): publish engine bases in ci #4649 : 2 other dependent PRs (#4650 , #4652 )
• chore: lockfile lefthook #4647
• fix(rivetkit): surface native sqlite kv errors to callers #4645
• fix(rivetkit): prevent sleep races during disconnect and db work #4644
• fix(rivetkit): isolate engine envoys and propagate startup failures #4643
• test(rivetkit): remove wasm sqlite-only fixtures and coverage #4642
• chore(sqlite-vfs): remove wasm sqlite packages and workspace wiring #4641
• chore(rivetkit): remove wasm sqlite runtime #4614
• fix(sqlite-native): restore native startup kv preload #4639
• fix(sqlite-native): delete metadata before chunk range #4638
• fix(sqlite-native): keep truncate cache coherent #4637
• fix(sqlite-vfs): use delete range for truncate cleanup #4636
• fix(sqlite-native): restore kv error hook #4635
• perf(sqlite-native): avoid cloning cached read chunks #4634
• perf(sqlite-native): gate kv operation labels #4633
• perf(sqlite-native): reuse read cache for partial writes #4632
• perf(sqlite-native): remove delete file existence probe #4631
• fix(sqlite-native): gate native read cache behind env var #4630
• fix(pegboard): isolate runner config dc lookup failures #4625 : 1 other dependent PR (#4626 )
• docs(pegboard): note runner config upsert split write risk #4624
• fix(pegboard): unpack global runner config keys correctly #4623
• fix(pegboard-gateway): enforce tunnel message state #4622
• fix(pegboard-runner): clear terminal tunnel routes #4621
• fix(pegboard): skip protocol version keys in runner pool backfill #4620
• fix(pegboard): create missing normal runner configs #4619
• fix(epoxy): reuse cached optimistic value #4618
• main

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

[claude]

Code Review: fix(pegboard): persist and replay hibernating requests

Overview

This PR fixes a correctness bug: in-flight requests to sleeping actors were not persisted to the DB, so they were lost on gateway or workflow restart. The fix:

1. Persists each hibernating request via upsert at the start of the keepalive task.
2. Populates CommandStartActor.hibernating_requests in the workflow activities directly, replacing the prior ephemeral guard-level population.
3. Adds !actor.sleeping guard to prevent sleeping actors from being treated as ready.
4. Adds .max(1) guards to prevent zero-duration intervals/timeouts.

---

Correctness

Responsibility shift in send_outbound (actor2/runtime.rs)

The removed comment said the field was left empty because a guard populated it ephemerally before reaching the runner. The diff does not show whether that guard-side code was removed. If both paths remain, hibernating_requests could be double-populated. Please confirm the guard-side path is removed or a no-op.

Idempotency race in insert_and_send_commands

The hibernating request list is fetched before the DB transaction. If the activity is retried, the list may differ (e.g. a request was GCd between retries). The persisted CommandStartActor could carry a stale list. Likely acceptable within the retry window, but worth a brief comment.

send_outbound vs insert_and_send_commands divergence (actor2)

send_outbound fetches and sends hibernating requests but does not persist them to the command key store (that only happens in insert_and_send_commands). If the actor restarts after send_outbound fires but before the command is durably stored, the requests may not be replayed. Is this intentional?

---

Code Quality

Double clone of commands_for_tx

In both actor/runtime.rs and actor2/runtime.rs, commands is cloned to commands_for_tx outside the closure and then cloned again inside. This is correct for the retry semantics of udb().run() but subtle. A brief comment would help future readers.

Logging level

upsert.rs, delete.rs, and list.rs all log at tracing::info!. These fire on every keepalive and actor start. Consider tracing::debug! to avoid log noise in production, unless these are intentional audit-level events.

---

Style

Log messages are lowercase with structured fields consistent with CLAUDE.md. Gateway/gateway2 and actor/actor2 runtime changes are kept in parity as required.

---

Unrelated Change

envoy-client/src/connection.rs adds "rivet_target.envoy" to the WebSocket subprotocol list. This appears unrelated to the main fix. A brief explanation in the PR description (or a separate commit) would clarify intent.

---

Minor

The .max(1) guards on ping interval and hibernation timeout are good defensive coding. A one-line comment explaining why zero is disallowed (prevents busy-loop) would add clarity.

---

Summary

The core logic is sound. Key questions before graduating from DRAFT:

1. Is the guard-side hibernating_requests population definitively removed?
2. Is the send_outbound / insert_and_send_commands divergence in actor2 intentional?
3. Confirm or document the acceptable race in activity retry idempotency.
4. Clarify the unrelated envoy-client protocol change.

Generated with Claude Code