fix(gateway): send actor key components as repeated rvt-key-part params - #5808
Abhikansh3 wants to merge 3 commits into
Conversation
| for part in key { | ||
| push_query_param(&mut params, "rvt-key-part", part); |
There was a problem hiding this comment.
🟠 Medium · The Rust client test suite still requires the removed parameter
The existing packages/client/tests/bare.rs coverage still asserts rvt-key for both gateway URL builders and the raw HTTP/WebSocket handlers. This builder now emits only rvt-key-part, so those tests deterministically fail (and their HashMap query helpers cannot represent repeated parts). Update those assertions and query helpers to collect repeated values, as the new inline tests do, so cargo test -p rivetkit-client covers and accepts the new wire format.
| | `rvt-namespace` | Yes | Namespace the actor lives in. | | ||
| | `rvt-method` | Yes | `get` to fail if the actor does not exist, or `getOrCreate` to create it on first use. | | ||
| | `rvt-key` | No | Actor key. Separate multi-part keys with commas: `rvt-key=org-1,room-2`. Omit for a keyless actor. | | ||
| | `rvt-key-part` | No | One key part. Repeat for a multi-part key, in order: `rvt-key-part=org-1&rvt-key-part=room-2`. Omit entirely for a keyless actor. | |
There was a problem hiding this comment.
🟠 Medium · The canonical API reference omits the new query parameter
The generated gateway reference still comes from rivetkit-openapi/openapi.json plus docs/api/endpoints.json, and both sources still define only rvt-key; consequently every generated gateway/inspector page continues to expose the comma-separated form and generated clients/spec consumers cannot discover rvt-key-part. Add the repeated array-style query parameter and its shared description/examples to those source files, then regenerate the API reference as required by docs/CLAUDE.md.
|
Ready for review. Covered with unit/integration tests in the guard, both RivetKit clients, and an end-to-end driver test against a locally built |
a790ba5 to
dd17113
Compare
Description
Fixes #5807. Query-based gateway routing (
getForKey/getOrCreateForKey, used byclient.foo.get([...])/.getOrCreate([...])) serialized a compound actor key by joining its components with a plain comma into thervt-keyquery parameter, with no escaping. Because of that, two logically distinct keys serialized identically and collided:["tenant,admin"](one component containing a literal comma)["tenant", "admin"](two separate components)Both produced
rvt-key=tenant%2Cadmin, and the guard decoded both back to["tenant", "admin"], silently routing requests to the wrong actor.rvt-key-partquery parameter per key component instead of a single comma-joinedrvt-keyvalue. Each occurrence is independently percent-encoded, so no escaping scheme is needed and a component containing a comma is expressed exactly as given.rvt-key-partoccurrences (the onlyrvt-*param allowed to repeat) and uses them directly as the key array when present.rvt-keyparam is kept and still decoded exactly as before, so older/pinned clients are unaffected. If both are present,rvt-key-parttakes precedence.actor-keys.mdx,actor-routing.mdx) that previously documented the comma-in-a-key-part case as "cannot be expressed" — in reality it silently collided rather than erroring.rivetkit-typescript/CLAUDE.mdthat prescribed the comma scheme as intended design, and removed a couple of references to a TS-side query-gateway parser that was deleted during the Rust cutover (parsing now lives solely in the Rust guard).Type of change
How Has This Been Tested?
cargo test -p rivet-guard --test parse_actor_path: 38/38 pass, including 4 new tests covering the collision directly (e.g.rvt-key-part=tenant%2Cadminvsrvt-key-part=tenant&rvt-key-part=adminnow decode to different arrays) plus all pre-existing legacy comma-format tests unchanged.cargo test -p rivetkit-client --lib remote_manager: 3/3 new tests pass, same collision check from the Rust client's URL-building side.pnpm exec vitest run tests/actor-gateway-url.test.ts(rivetkit-typescript): 14/14 pass, including the issue's own suggested regression set (["a,b"]vs["a","b"],[""]vs[],["a,","b"]vs["a","","b"],[","]vs["",""]).rivet-enginebinary and rantests/driver/gateway-routing.test.tsagainst it. The new end-to-end test (distinguishes a comma-containing component from a two-component key, creating two actors via the real SDK and asserting they resolve to different actor IDs) passes on every native-runtime variant (local/remote sqlite × bare/cbor/json encoding, 6/6). Thewasmruntime variant of this suite currently fails in my environment due to a pre-existing missingrivetkit-wasmbuild artifact unrelated to this change.cargo clippyon both touched crates: no new warnings.cargo test -p rivetkit-client --test bare: 21/21 pass, including the pre-existing integration tests that assert on gateway query parameters, updated to checkrvt-key-partinstead of the now-removedrvt-key.Checklist:
Update
Addressed the two review findings from the automated review:
rivetkit-rust/packages/client/tests/bare.rshad pre-existing integration tests asserting on the now-removedrvt-keyparameter with aHashMap-based query helper that couldn't represent repeated parameters. Updated those tests and added a helper that collects repeatedrvt-key-partoccurrences.rivetkit-openapi/openapi.jsonanddocs/api/endpoints.jsonnow definervt-key-partalongsidervt-key, and the generated gateway/inspector reference pages were regenerated withpnpm docs:gen-apiso the new parameter is discoverable in the public API reference.