Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 14, 2026

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

Service Status Web Updated (UTC)
frontend-inspector 😴 Sleeping (View Logs) Web Jan 14, 2026 at 8:46 am
website 😴 Sleeping (View Logs) Web Jan 14, 2026 at 8:46 am
frontend-cloud 😴 Sleeping (View Logs) Web Jan 14, 2026 at 8:45 am

Copy link
Member Author

NathanFlurry commented Jan 14, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


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.

@claude
Copy link

claude bot commented Jan 14, 2026

PR Review: feat(rivetkit): allow defining public namespace & token for client redirects via metadata endpoint

Overview

This PR enhances RivetKit's configuration system to support specifying public namespace and token credentials for client redirects through the metadata endpoint. The changes introduce cleaner URL-based auth syntax and better separation between backend and frontend credentials.

Positive Aspects

1. Well-Structured Configuration Flow

The refactoring significantly improves how endpoint parsing and configuration resolution works:

  • Centralized endpoint parsing logic in tryParseEndpoint
  • Clear separation between backend connection config and public client config
  • Proper use of Zod transforms and refinement context for validation

2. Comprehensive URL Auth Support

The URL auth syntax is consistently supported across RIVET_ENDPOINT and RIVET_PUBLIC_ENDPOINT with proper handling of percent-encoded characters.

3. Good Documentation

  • Clear JSDoc comments explaining config options
  • Updated documentation in connect-backend.mdx with examples
  • Comments explaining validation logic

4. Test Coverage

The endpoint parser tests are comprehensive covering basic parsing, error cases, duplicate credential detection, and edge cases.

Issues & Concerns

1. Critical: Potential Configuration Confusion

In registry/config/index.ts:214, the serveManager default logic could leave clients without a valid endpoint in edge cases. Add validation to ensure that if serveManager is false and not in dev mode, either endpoint or publicEndpoint must be specified.

2. Medium: Error Message Quality

In registry/config/index.ts:207-210, error messages for namespace mismatch could be more actionable with guidance on how to fix the issue.

3. Medium: Inconsistent Default Handling

Client and registry configs have different default endpoint logic that could cause confusion. Client uses window.location.origin while registry uses hardcoded localhost. Consider consolidating or documenting the differences.

4. Minor Issues

  • Comment clarity in runtime/index.ts:85-89 about unreachable code could be improved
  • Could combine duplicate credential errors in tryParseEndpoint for clarity
  • Type safety improvements possible in api-utils.ts getEndpoint function

5. Documentation Typo

In connect-backend.mdx:3 - exlpicitly should be explicitly

Security Considerations

Good practices: No credential logging and proper URL encoding handling.

Important consideration: The metadata endpoint now returns clientToken. Add documentation clarifying that publicToken should be a limited-privilege token suitable for public exposure, not the backends privileged token.

Testing Recommendations

  1. Add integration tests for client connection flow with URL-embedded credentials
  2. Test default behaviors in dev vs prod mode
  3. Test namespace validation between public and private endpoints

Summary

This is a solid PR that improves RivetKits configuration flexibility. The core implementation is sound with good structure and test coverage.

Recommendation: Approve with minor changes

The PR is functionally correct and safe to merge, but would benefit from:

  • Adding validation for serveManager/publicEndpoint edge cases
  • Improving error messages
  • Adding security documentation for clientToken/publicToken
  • Fixing the documentation typo

Suggested Follow-ups:

  1. Add integration tests for metadata endpoint flow
  2. Consolidate default endpoint logic between client and registry configs
  3. Create configuration flow diagram
  4. Consider removing getEndpoint per TODO in api-utils.ts

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 14, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: a53dc0f

@NathanFlurry NathanFlurry force-pushed the 01-13-feat_rivetkit_allow_defining_public_namespace_token_for_client_redirects_via_metadata_endpoint branch from 3c56ebe to bcd4a21 Compare January 14, 2026 08:09
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3891 January 14, 2026 08:09 Destroyed
@NathanFlurry NathanFlurry force-pushed the 01-13-do_not_merge_cloudflare_w_o_do_support branch from 4662ef8 to f35697e Compare January 14, 2026 08:21
@NathanFlurry NathanFlurry force-pushed the 01-13-feat_rivetkit_allow_defining_public_namespace_token_for_client_redirects_via_metadata_endpoint branch from bcd4a21 to 62bde92 Compare January 14, 2026 08:21
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3891 January 14, 2026 08:21 Destroyed
@NathanFlurry NathanFlurry changed the base branch from 01-13-do_not_merge_cloudflare_w_o_do_support to graphite-base/3891 January 14, 2026 08:35
@NathanFlurry NathanFlurry force-pushed the 01-13-feat_rivetkit_allow_defining_public_namespace_token_for_client_redirects_via_metadata_endpoint branch from 62bde92 to a53dc0f Compare January 14, 2026 08:35
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3891 January 14, 2026 08:35 Destroyed
@NathanFlurry NathanFlurry changed the base branch from graphite-base/3891 to 01-13-chore_engine_provide_detailed_error_for_invalid_serverless_sse_event January 14, 2026 08:35
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 14, 2026

Merge activity

  • Jan 14, 8:52 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Jan 14, 8:52 PM UTC: CI is running for this pull request on a draft pull request (#3898) due to your merge queue CI optimization settings.
  • Jan 14, 8:53 PM UTC: Merged by the Graphite merge queue via draft PR: #3898.

graphite-app bot pushed a commit that referenced this pull request Jan 14, 2026
@graphite-app graphite-app bot closed this Jan 14, 2026
@graphite-app graphite-app bot deleted the 01-13-feat_rivetkit_allow_defining_public_namespace_token_for_client_redirects_via_metadata_endpoint branch January 14, 2026 20:53
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