Skip to content

fix(rivetkit): preserve query gateway skip ready wait#5008

Merged
NathanFlurry merged 1 commit into
mainfrom
skip-ready-wait/query-gateway
May 9, 2026
Merged

fix(rivetkit): preserve query gateway skip ready wait#5008
NathanFlurry merged 1 commit into
mainfrom
skip-ready-wait/query-gateway

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

@NathanFlurry NathanFlurry commented May 9, 2026

Stack Context

Standalone fix for query-backed RivetKit gateway requests with skipReadyWait.

What?

  • Keeps query-backed actor handles on the gateway rvt-* URL path when skipReadyWait is enabled.
  • Preserves direct actor ID optimized routing for normal getForId handles.
  • Adds HTTP and websocket regression coverage for query-backed skip-ready-wait routing.

Why?

skipReadyWait was causing query-backed handles to resolve through the actor API first, which turned a single gateway round trip into an extra resolve/create request. The gateway URL path already supports rvt-skip-ready-wait, so the client should pass the query through and let the gateway resolve it.

Verification

  • pnpm --dir rivetkit-typescript/packages/rivetkit exec vitest run tests/remote-engine-client-public-token.test.ts
  • pnpm --dir rivetkit-typescript/packages/rivetkit exec tsc --noEmit --pretty false
  • git diff --check

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 9, 2026

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

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web May 9, 2026 at 7:33 am
website 😴 Sleeping (View Logs) Web May 9, 2026 at 6:39 am
frontend-cloud ❌ Build Failed (View Logs) Web May 9, 2026 at 6:34 am
frontend-inspector ❌ Build Failed (View Logs) Web May 9, 2026 at 6:33 am
ladle ❌ Build Failed (View Logs) Web May 9, 2026 at 6:28 am
mcp-hub ✅ Success (View Logs) Web May 9, 2026 at 6:28 am

@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5008 May 9, 2026 06:27 Destroyed
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 9, 2026

Code Review

Overview

This PR fixes two related bugs where skipReadyWait for query-backed actor handles (e.g., getOrCreate) was causing an unnecessary extra resolve/create round trip to the actor API before the real gateway request. The gateway already supports rvt-skip-ready-wait in the URL query params, so the client should pass the query through and let the gateway handle resolution.


Bug Fix Correctness

actor-conn.ts — The removed #resolveGatewayTargetForSkipReadyWait() was calling resolveGatewayTarget() (i.e., the actor API) for query-backed handles under skipReadyWait, defeating its own purpose. Replacing it with an unconditional getGatewayTarget(this.#actorResolutionState) is correct since getGatewayTarget already handles the getForId case by returning a directId target.

actor-handle.ts — inverted condition was a real bug. The old WebSocket path had:

const target = gatewayOptions.skipReadyWait
    ? await this.#resolveActionTarget(false)   // resolved through API when skipReadyWait=true — wrong
    : getGatewayTarget(this.#actorResolutionState);  // used query target when skipReadyWait=false — wrong

The new #resolveGatewayRequestTarget helper corrects this and unifies the logic across HTTP, queue, and WebSocket paths. Good cleanup.


Issues

Test isolation: new WebSocket coverage is appended inside an existing test.

The HTTP test is correctly a standalone test() block. But the WebSocket coverage was appended at the end of "uses metadata clientToken for actor websocket gateway requests", relying on mocks, globals, and socket counts (expect(sockets).toHaveLength(4)) established earlier in that test. This has a few problems:

  • If the parent test ever gains another WebSocket call, the toHaveLength(4) assertion will fail for an unrelated reason.
  • The new scenario can't be run or debugged in isolation via a Vitest filter.
  • It makes the existing test harder to read — it now tests two distinct behaviors.

Recommend extracting the new scenario into its own test("query handle websocket keeps skip ready wait on gateway URL", ...) that sets up its own sockets array and mocks, following the same pattern as the HTTP test added in this same PR.


Minor Notes

  • Import reordering (createClient moved up) is a nice touch for consistency.
  • The resolveGatewayTarget import removal from actor-conn.ts is correct — it was only used by the deleted method.
  • #resolveGatewayRequestTarget takes gatewayOptions as a parameter rather than closing over this.#gatewayOptions — this is fine since callers already have the resolved options in scope and it keeps the method pure.

Summary

The core fix is correct and the simplification is an improvement. The one thing worth addressing before merge is extracting the WebSocket regression test into its own test() block for isolation and readability.

@NathanFlurry NathanFlurry changed the base branch from metrics/endpoint-sanity-test to graphite-base/5008 May 9, 2026 07:33
@NathanFlurry NathanFlurry force-pushed the skip-ready-wait/query-gateway branch from ac432a0 to d4c5d79 Compare May 9, 2026 07:33
@NathanFlurry NathanFlurry force-pushed the graphite-base/5008 branch from f188ab9 to 24c5d49 Compare May 9, 2026 07:33
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5008 May 9, 2026 07:33 Destroyed
@NathanFlurry NathanFlurry changed the base branch from graphite-base/5008 to sqlite-vfs/fatal-only-fence-mismatch May 9, 2026 07:33
@NathanFlurry NathanFlurry marked this pull request as ready for review May 9, 2026 07:34
Base automatically changed from sqlite-vfs/fatal-only-fence-mismatch to main May 9, 2026 07:50
@NathanFlurry NathanFlurry merged commit d4c5d79 into main May 9, 2026
13 of 28 checks passed
@NathanFlurry NathanFlurry deleted the skip-ready-wait/query-gateway branch May 9, 2026 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant