Stack for rivet-dev/rivet

• fix(depot): fix migrating v1 -> v2 databases #5183 mxwxqyss · 2026-06-05 23:09:16
• docs(rivetkit): add rust quickstart preview #5182 lpzwyzmw · 2026-06-05 23:09:15
• feat(rivetkit): add rust counter example #5181 mtrynmln · 2026-06-05 23:09:14 👈
• refactor(depot): remove sqlite cold storage #5180 oxsukmwr · 2026-06-05 23:09:13
• main

Check out this stack: jj-stack get https://github.com/rivet-dev/rivet/pull/5181
Pull/update this stack: jj-stack sync
Publish local edits: jj-stack submit
Merge when ready: jj-stack merge

Code Review: feat(rivetkit): add rust counter example

Overview

This PR adds a standalone examples/rust-counter crate that demonstrates how to build directly on rivetkit-core, including a runnable server and an e2e test that spins up a temporary rivet-engine process. Good addition for developers exploring the Rust path.

Issues

Silent fallback on corrupt state (src/lib.rs:112)

If the stored snapshot is valid CBOR but does not contain a "count" field (e.g. a format mismatch), decode_count silently returns 0 via .unwrap_or_default() and the actor carries on unaware. Should return an explicit error instead.

Brittle error-string matching in wait_for_action (tests/e2e.rs:311)

Retrying on message.contains("actor_ready_timeout") || message.contains("Service Unavailable") is fragile - wording changes or HTTP middleware tweaks silently break the retry logic. Consider matching on HTTP status code (503 / known retry-able codes) instead.

action_output silently returns Null for malformed responses (tests/e2e.rs:334)

If the engine returns valid JSON with no output key, assert_eq!(action_output(&first)?, json!(1)) fails with an unhelpful message. Return an error when the field is absent so the failure message includes the actual body.

Undocumented dual-mode parsing in decode_increment_amount (src/lib.rs:91)

The function accepts either a bare integer or an array whose first element is an integer, with no documentation. The README shows {"args":[]} (empty array), meaning this always defaults to 1. Either add a one-line comment explaining the expected encoding, or simplify to only support the array form to match the README.

Smaller notes

• workspace_root() (tests/e2e.rs:380) uses .ancestors().nth(2), relying on the example staying exactly two levels under the workspace root. Acceptable for an example, but a comment would clarify the assumption.
• The polling loops in wait_for_engine_health, wait_for_envoy_ready, and wait_for_action match the pattern CLAUDE.md reserves for external-process health polling, so they are a valid exception - a brief comment on each noting that event-driven wakeup is not possible here would make that intent explicit.
• shutdown_child uses start_kill (SIGKILL). Fine for test teardown; a SIGTERM + timeout approach would be cleaner if the engine needs to flush state, but not critical for an example.
• serde dep split is correct: serde_json in main deps (needed by lib), serde and reqwest in dev-deps (tests only).

Positive

• All ActorEvent variants are matched explicitly - no _ => fallthrough, consistent with CLAUDE.md.
• Test includes fail-fast logic: checks whether the engine process has already exited before the health deadline.
• Engine stdout/stderr tail is captured and surfaced in failure messages - very developer-friendly.
• TestEngine::Drop sends start_kill as a safety net while the explicit shutdown() path waits; clean double-coverage.
• CancellationToken used throughout for clean shutdown.