[SLOP(gpt-5)] fix(rivetkit): normalize protocol args source-side

[NathanFlurry]

No description provided.

[NathanFlurry]

Stack for rivet-dev/rivet

• [SLOP(gpt-5)] feat(cli): add rivetkit cli #5255 kprpzusk · 2026-06-13 23:00:59
• [SLOP(claude-opus-4-8[1m]-medium)] docs(website): consolidate landing pages, deploy quickstarts, and docs restructuring #5254 psyxxqon · 2026-06-13 23:00:58
• [SLOP(claude-opus-4-8)] refactor(rivetkit): deprecate actor kv api across core and typescript #5253 polvovvk · 2026-06-13 23:00:57
• [SLOP(claude-opus-4-8)] docs(examples): split effect example into hello-world-effect and chat-room-effect #5252 wolqzmyo · 2026-06-13 23:00:56
• [SLOP(gpt-5)] fix(rivetkit): normalize protocol args source-side #5251 lvpxytwl · 2026-06-13 00:10:46 👈
• [SLOP(claude-opus-4-8)] docs(actors): apply docs audit fixes #5250 rtqklwqu · 2026-06-13 00:10:45
• [SLOP(claude-opus-4-8)] docs(actors): rewrite in-memory state page, remove sandbox page, fix stateSaveInterval default, forbid em dashes #5249 pqwwounv · 2026-06-13 00:10:44
• [SLOP(claude-opus-4-8-medium)] docs(pool-configuration): fix runner-config SDK snippet to use RivetClient from engine-api-full #5248 tmoxrznq · 2026-06-13 00:10:42
• [SLOP(claude-opus-4-8-medium)] docs(inspector-tabs): mark illustrative type-shape snippets as @nocheck #5247 yosxkyox · 2026-06-13 00:10:41
• [SLOP(claude-opus-4-8-medium)] docs(quickstart): clean up rust quickstart to import shared actor and auto-download engine #5246 lrzpsprl · 2026-06-13 00:10:40
• [SLOP(claude-opus-4-8-medium)] fix(rivetkit-wasm): add missing inspector_tabs field to ActorConfigInput conversion #5245 ookuputp · 2026-06-13 00:10:39
• main

Get stack: forklift get 5251
Push local edits: forklift submit
Merge when ready: forklift merge 5251

[claude]

Code Review

Overview

This PR normalizes RivetKit action and event args to always be array-shaped at the source side before crossing the protocol boundary. The key semantic change is in positional_value (Rust): named structs are now wrapped as a single object element [{...}] instead of being flattened positionally into [val1, val2, ...]. This fixes cross-language interoperability so TypeScript/React clients can consume Rust struct events, allowing the limitation notes to be removed from docs.

---

Correctness concerns

Breaking wire format without a backward-compat path

The old encoding for a named struct {first: "a", second: "b"} was ["a", "b"] (values spread positionally). The new encoding is [{first: "a", second: "b"}] (object wrapped in array). Existing deployed clients that produce the old ["a", "b"] format will now fail to decode on an upgraded server. CLAUDE.md explicitly states: "clients send their latest request protocol version, and servers handle older-client compatibility and negotiation." No backward-compat fallback is implemented here. At minimum, this warrants an explicit note in the PR body / commit about the compatibility decision.

decode_event_args loses error chain

In typed_client.rs, every arm of decode_event_args uses the pattern:

.map_err(|error| anyhow::anyhow!(error.to_string()))

to_string() on an error discards the underlying cause chain. Prefer anyhow::anyhow!("{error:#}") (which uses the alternate debug formatter and preserves context) or restructure so deserialize_cbor_value returns an anyhow::Error directly.

decode_event_args multi-element array arm

When values.len() > 1, the arm passes CborValue::Array(values) through to deserialize_cbor_value. If the target Event type is a named struct (expects a CBOR map), this will always fail deserialization. There is no path in the current encoding that produces a multi-element array for a struct event, so this arm is unreachable in practice. Either document why it is kept (future multi-arg events?) or make it an explicit error instead of a silent deserialize failure.

Inspector properties null check parity

In inspector.rs the property-object check uses !properties.is_object(). JsonValue::Null satisfies is_null() but not is_object(), so null as a properties value correctly returns 400. The TypeScript side explicitly checks body.properties === null. The behavior is consistent, but the Rust side is relying on the implicit semantics of is_object() — worth a comment for clarity.

---

Code quality

Duplication of normalize_json_args / normalizeArgs

The normalization logic (array → itself, null → empty, other → wrap) is now duplicated between Rust (http.rs) and TypeScript (native.ts). This is an inherent cross-language limitation, but both new helper functions are clearly named and co-located — good.

properties validation duplicated across Rust and TypeScript

The args-vs-properties mutex validation and the properties-must-be-object check are implemented identically in inspector.rs and native.ts. Any future rule change must be applied in both. A cross-reference comment would help maintainability.

encode_varargs naming

encode_varargs is pub(crate) in action.rs and called from context.rs. The name is slightly misleading since it now encodes a single struct as [struct] rather than variadic args. Something like encode_as_arg_array might better reflect the new semantics.

Bare encoding path in http.rs

The diff shows the JSON and CBOR paths gain normalize_json_args, but the Bare path is not visible. If Bare args deserialization also previously silently dropped non-array payloads, it may need the same treatment. Worth confirming (or noting it is fine because BARE is strongly typed).

---

Tests

Missing unit test for normalize_json_args

The Rust helper normalize_json_args has no dedicated unit test. Behavior is covered transitively, but an explicit test with Null, a scalar, and an array input would make the invariant obvious and prevent regression.

No test for backward-compat decoding

Given the wire format change (old ["a", "b"] → new [{first, second}]), there is no test verifying what happens when an old-format payload arrives at the new server. If that is intentionally unsupported, a test asserting the expected error would be a useful safety net.

Test update in client.rs is correct

The update to cbor(&vec![SiblingNotice {...}]) correctly reflects the new array-wrapped encoding.

---

Docs

The docs changes are an improvement:

• Removing the two limitation notes about Rust struct events is correct now that cross-language event consumption works.
• The state.mdx change from per-actor new Pool() to a module-level shared pool is a meaningful best-practice fix.
• The rust.mdx React example now shows useEvent properly instead of reading from the action return value as a workaround.
• The section header rename from "Durable vs Ephemeral" to "Types of State" is accurate since the section covers three categories.

---

Summary

Area	Finding
Protocol invariant	Clearly documented in CLAUDE.md — good
Wire format change	Breaking for existing clients; no backward-compat path; needs explicit acknowledgment
Error chain loss	map_err(|e| anyhow!(e.to_string())) in decode_event_args discards the cause chain
Multi-element event arm	Effectively unreachable; document intent or make it an explicit error
Test coverage	normalize_json_args lacks a unit test; no backward-compat regression test
Docs	Clean improvement; limitation notes correctly removed