DO NOT MERGE: serverless restart race condition

[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

[railway-app]

🚅 Deployed to the rivet-pr-4997 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
website	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am
mcp-hub	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am
frontend-inspector	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am
kitchen-sink	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am
frontend-cloud	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am
ladle	◻️ Removed (View Logs)	Web	May 9, 2026 at 7:34 am

[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

• TODO: ktichen sink counter latency #5051
• fix(rivetkit): use engine actor stop threshold for shutdown #5049
• DO NOT MERGE: serverless restart race condition #4997 👈 (View in Graphite)
• fix(rivetkit): exit pid1 after signal shutdown #5045
• fix(rivetkit-core): decrement active actor metrics #5044
• fix(envoy-client): dont abort drain after 20s #5043
• temp: patch wasm-pack to point to latest binary #5037
• fix(rivetkit): require engine ping for health #5022
• feat(rivetkit): add current actor metrics #5020
• feat(rivetkit): expose low-cardinality metrics routes #5019
• docs(serverless): document timeout tuning #5018
• feat(kitchen-sink): add load testing harness #5017
• fix(rivetkit): expose client actor metadata #5016
• fix(runtime): avoid bounded dispatch channel stalls #5015
• fix(sqlite): fence remote requests by generation #5014
• feat(rivetkit-core): expose metrics endpoint #5013
• WIP: counter latency monitor #5011
• main

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

[claude]

PR Review: DO NOT MERGE — Serverless Restart Race Condition

PR #4997 | Draft | +2075 / -0 lines | Author: @NathanFlurry

---

Overview

This is a diagnostic/investigation PR — no fix is shipped. It adds a self-contained test harness to reproduce and characterize a post-engine-restart race condition in which gateway traffic to already-warmed serverless actors hangs for ~0-3 seconds after the engine's /health endpoint returns.

Three files added:

• tests/driver/engine-restart-serverless.md — investigation document with timing sweep results and findings
• tests/driver/engine-restart-serverless.ts — standalone Node repro script (not Vitest)
• tests/fixtures/engine-restart-serverless-runtime.ts — serverless runtime fixture actor

The investigation correctly identifies that the bug is not SQLite-specific: the same hang occurs with KV heartbeats and no heartbeats. The root cause is gateway/serverless routing state for existing actors not settling immediately after engine health returns.

---

What Works Well

• Thorough investigation methodology. The sweep structure (coarse, narrow, edge, low repeat) is rigorous and the timing conclusions are well-supported.
• Clean process lifecycle management. OwnedEngine and ServerlessRuntime use try/finally for cleanup, a SIGKILL fallback with a 5s timer in stopChildProcess, and correct port exclusion when allocating multiple ports.
• Good coordination primitive. createCommitSignalServer is a neat way to precisely time the engine shutdown relative to the actor's in-flight commit.
• Correct cleanup sequencing. The finally block in main() closes the signal server, disposes the client, cleans the runtime, and then removes the engine temp dir in the right order.
• Explicit environment validation. Both the harness and the runtime fixture validate required env vars at startup and fail with actionable messages.

---

Issues

1. Inconsistent indentation in the runtime fixture (cosmetic)

engine-restart-serverless-runtime.ts around the heartbeat_sql_ok log event mixes indentation levels, unlike every other logRuntimeEvent call in the file.

2. ROLLBACK error is silently swallowed in tick

commitDuringEngineRestart correctly captures rollback errors and surfaces them. The tick action does not handle errors from the ROLLBACK call, so if the rollback itself throws the original error is lost. Make tick consistent with commitDuringEngineRestart.

3. WebSocket error event discards the underlying error

The error event listener in the harness calls finish(new Error("websocket error")) without capturing the ErrorEvent.message field from Node's ws-backed WebSocket. Capturing it would make failure logs more useful when debugging sweep results.

4. Investigation doc references local /tmp/ log files

Lines 229-244 of the .md reference files like /tmp/rivet-idle-none-immediate.log. These only exist on Nathan's machine and are not useful to anyone else reading the document. Either remove the section or note that these are illustrative paths.

5. Investigation doc belongs in .agent/research/

Per CLAUDE.md, all agent working files live in .agent/ at the repo root, with research docs going in .agent/research/. Placing engine-restart-serverless.md under tests/driver/ mixes investigation notes into the test tree. Consider moving it to .agent/research/engine-restart-serverless.md.

6. observePostRestartHeartbeat uses a fixed wall-clock sleep

The function waits a full POST_RESTART_HEARTBEAT_OBSERVATION_MS (12s by default) regardless of whether the actor heartbeat stops early. This is diagnostic tooling so a fixed wait is understandable, but it adds unnecessary wall time to every sweep run. Low priority given the DO NOT MERGE state.

7. withTimeout leaks the underlying promise

When the timeout fires and rejects, the original promise continues running in the background. This can produce confusing runtime output where the harness has already reported "timed out" but a delayed response still logs. Worth tracking if this harness evolves into a real gate test.

---

Minor Style Notes

• Long lines in runGatewayHealthDelaySweep and runGatewayWebSocketDelaySweep exceed the implicit line length of the surrounding code.
• TOKEN = "dev" is hardcoded — fine for local dev tooling.

---

Summary

This is solid diagnostic work. The race condition is well-characterized, the methodology is reproducible, and the conclusion (engine /health does not imply gateway routing readiness for existing actors) gives a clear signal for where to look for a fix. The issues above are mostly polish for when this investigation leads to a real test or is merged as reference material.