Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 19, 2026

🚅 Environment rivet-pr-3975 in rivet-frontend has no services deployed.

Copy link
Member Author

NathanFlurry commented Jan 19, 2026


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 19, 2026

More templates

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3975

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3975

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3975

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3975

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3975

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3975

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3975

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3975

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3975

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3975

commit: 825c812

@claude
Copy link

claude bot commented Jan 19, 2026

PR Review: feat(rivetkit): expose getGatewayUrl

Summary

This PR adds a new getGatewayUrl() method to the ActorHandleRaw class, exposing the ability to retrieve the gateway URL for routing traffic to a specific Rivet Actor. The implementation adds a buildGatewayUrl method to the ManagerDriver interface and implements it across all driver implementations (Cloudflare Workers, FileSystem, and Remote).

Code Quality & Best Practices ✅

Strengths:

  • Clean interface design with the new method properly added to the ManagerDriver interface
  • Good code reuse: refactored existing URL building logic into buildActorGatewayUrl helper function
  • Consistent implementation across all three driver types
  • Properly handles URL encoding with encodeURIComponent
  • Follows existing patterns in the codebase (async methods, proper error handling via queryActor)

Minor Issues:

  1. Documentation - Missing JSDoc on public API (actor-handle.ts:282)

    • The new getGatewayUrl() method lacks a @returns tag
    • Consider adding: @returns {Promise<string>} The gateway URL for the actor
  2. Inconsistent documentation style (actor-handle.ts:242)

    • The resolve() method comment was simplified but the new getGatewayUrl() comment does not follow the same pattern
    • Both should be consistent in their level of detail

Potential Bugs or Issues 🔍

No Critical Issues Found

Minor observations:

  • The buildGatewayUrl implementations are all async, but most could be synchronous (FileSystem and Cloudflare). The Remote implementation needs to await metadataPromise, so keeping them all async for interface consistency is fine.
  • Token handling in buildActorGatewayUrl uses the @ symbol as a delimiter. This is an existing pattern, not introduced in this PR, but worth noting for security review if tokens could contain special characters.

Performance Considerations ⚡

Good:

  • The refactoring consolidates URL building logic, reducing code duplication
  • The RemoteManagerDriver.buildGatewayUrl properly awaits metadata initialization before building the URL, preventing race conditions

Note:

  • getGatewayUrl() calls queryActor() which may involve network calls in the Remote driver. This is expected behavior, but users should be aware this is not a purely synchronous property access.

Security Concerns 🔒

All Good:

  • Proper URL encoding via encodeURIComponent for both actorId and token parameters
  • Token is only included when defined (token !== undefined check)
  • No user input is directly concatenated into URLs without encoding

Test Coverage 🧪

Missing:

  • No tests added for the new getGatewayUrl() method
  • No tests for buildGatewayUrl() in any of the driver implementations

Recommendations:

  1. Add tests for ActorHandleRaw.getGatewayUrl() in the appropriate test suites (e.g., driver-memory.test.ts, driver-file-system.test.ts)
  2. Test cases should cover:
    • URL format correctness
    • Token inclusion/exclusion based on configuration
    • Proper URL encoding of special characters in actor IDs
    • Integration with different driver types

Architecture & Design 🏗️

Excellent:

  • The design properly separates concerns by putting URL building in the driver layer where runtime-specific details belong
  • The helper function buildActorGatewayUrl is well-factored and reusable
  • The public API on ActorHandleRaw provides a clean interface for users

Convention Compliance ✅

Based on CLAUDE.md guidelines:

  • ✅ Uses workspace dependencies appropriately
  • ✅ Code style appears consistent with existing patterns
  • ✅ Follows existing import patterns
  • ✅ Async/await usage is appropriate
  • ⚠️ Missing tests (see Test Coverage section above)

Recommendations Summary

  1. High Priority:

    • Add test coverage for the new functionality
  2. Medium Priority:

    • Improve JSDoc documentation with @returns tag
    • Ensure consistent documentation style across similar methods
  3. Low Priority:

    • Consider adding usage examples in comments or documentation

Conclusion

This is a clean, well-implemented feature addition. The code quality is good, follows existing patterns, and the refactoring improves code organization. The main gap is test coverage. Once tests are added, this PR will be ready to merge.

Status: Approve with recommendations


Review generated by Claude Code

import type { ClientConfig } from "@/client/config";
import { HEADER_RIVET_TOKEN } from "@/common/actor-router-consts";
import { combineUrlPath } from "@/utils";
import { buildActorGatewayUrl } from "./actor-websocket-client";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import statement needs to be sorted correctly according to Biome's rules. Run 'biome format --write .' to fix import sorting.

Spotted by Graphite Agent (based on CI logs)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@NathanFlurry NathanFlurry force-pushed the 01-19-feat_rivetkit_expose_getgatewayurl branch from fa8f73e to ed37d98 Compare January 19, 2026 18:54
@NathanFlurry NathanFlurry mentioned this pull request Jan 19, 2026
Comment on lines +16 to +26
export function buildActorGatewayUrl(
endpoint: string,
actorId: string,
token: string | undefined,
path = "",
): string {
const tokenSegment =
token !== undefined ? `@${encodeURIComponent(token)}` : "";
const gatewayPath = `/gateway/${encodeURIComponent(actorId)}${tokenSegment}${path}`;
return combineUrlPath(endpoint, gatewayPath);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new buildActorGatewayUrl function might not be properly formatted according to Biome standards. Run the Biome formatter on this file to ensure proper spacing, indentation, and line breaks. Also, ensure the imports at the top of the file are properly sorted.

Spotted by Graphite Agent (based on CI logs)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@NathanFlurry NathanFlurry force-pushed the 01-19-feat_rivetkit_expose_getgatewayurl branch from ed37d98 to 825c812 Compare January 20, 2026 19:48
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 21, 2026

Merge activity

  • Jan 21, 12:11 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Jan 21, 12:11 AM UTC: CI is running for this pull request on a draft pull request (#3988) due to your merge queue CI optimization settings.
  • Jan 21, 12:12 AM UTC: Merged by the Graphite merge queue via draft PR: #3988.

graphite-app bot pushed a commit that referenced this pull request Jan 21, 2026
@graphite-app graphite-app bot closed this Jan 21, 2026
@graphite-app graphite-app bot deleted the 01-19-feat_rivetkit_expose_getgatewayurl branch January 21, 2026 00:12
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.

2 participants