Skip to content
This repository was archived by the owner on Oct 22, 2025. It is now read-only.

chore(core): add support for RIVET_ENDPOINT env var as alternative to RIVET_ENGINE#1347

Closed
NathanFlurry wants to merge 1 commit into10-04-fix_core_strip_basepath_from_routes_for_actor_gateway_on_manager_routerfrom
10-04-chore_core_add_support_for_rivet_endpoint_env_var_as_alternative_to_rivet_engine
Closed

chore(core): add support for RIVET_ENDPOINT env var as alternative to RIVET_ENGINE#1347
NathanFlurry wants to merge 1 commit into10-04-fix_core_strip_basepath_from_routes_for_actor_gateway_on_manager_routerfrom
10-04-chore_core_add_support_for_rivet_endpoint_env_var_as_alternative_to_rivet_engine

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@vercel
Copy link

vercel bot commented Oct 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
rivetkit-serverless Error Error Oct 5, 2025 0:49am

@claude
Copy link

claude bot commented Oct 5, 2025

Claude finished @NathanFlurry's task —— View job


Pull Request Review

Reviewing changes for adding RIVET_ENDPOINT environment variable support.

Review Checklist:

  • Read and analyze modified files
  • Check code quality and best practices
  • Identify potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage

Summary

This PR adds support for RIVET_ENDPOINT as an alternative environment variable to RIVET_ENGINE in two configuration files. The changes are consistent and well-implemented.

Code Quality & Best Practices

Excellent:

  • Consistent implementation: Both client/config.ts and drivers/engine/config.ts have identical fallback logic
  • Backward compatibility: RIVET_ENGINE is checked first, maintaining existing behavior
  • Clean code structure: Uses proper Zod schema defaults with factory functions
  • Follows existing patterns: Matches the codebase's approach for other environment variables

No Bugs or Issues Identified

The implementation is correct:

  • Proper fallback chain: RIVET_ENGINERIVET_ENDPOINT → default
  • Consistent across both files
  • No breaking changes to existing functionality

Performance Considerations

No performance impact:

  • Environment variable reads are only done during config initialization
  • No runtime performance implications
  • Factory functions ensure lazy evaluation

Security Considerations

Secure implementation:

  • No sensitive data exposure
  • Follows existing patterns for environment variable handling
  • Maintains the same security model as RIVET_ENGINE

⚠️ Areas for Improvement

Documentation:

  • The comment in client/config.ts:24 should be updated to mention both environment variables:
    /** Endpoint to connect to the Rivet engine. Can be configured via RIVET_ENGINE or RIVET_ENDPOINT env var. */

Test Coverage:

  • Consider adding tests that verify the environment variable precedence
  • The existing test in driver-engine.test.ts:21 uses RIVET_ENDPOINT, which is good coverage

Missing Context:

  • PR description is empty - would benefit from explaining why RIVET_ENDPOINT is needed as an alternative

📝 Specific Suggestions

  1. Update comment in packages/rivetkit/src/client/config.ts:24:

    - /** Endpoint to connect to the Rivet engine. Can be configured via RIVET_ENGINE env var. */
    + /** Endpoint to connect to the Rivet engine. Can be configured via RIVET_ENGINE or RIVET_ENDPOINT env var. */
  2. Consider adding a test that verifies the precedence order explicitly.

Overall Assessment

✅ Approved - This is a clean, well-implemented change that adds useful functionality without breaking existing behavior. The implementation is consistent across both affected files and follows established patterns in the codebase.

The only minor improvement would be updating the documentation comment to reflect both environment variable options.

Copy link
Member Author

NathanFlurry commented Oct 5, 2025

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 either label to this PR to merge it via the merge queue:

  • merge queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

@graphite-app
Copy link

graphite-app bot commented Oct 5, 2025

Merge activity

  • Oct 5, 12:50 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Oct 5, 12:51 AM UTC: CI is running for this pull request on a draft pull request (#1349) due to your merge queue CI optimization settings.
  • Oct 5, 12:52 AM UTC: Merged by the Graphite merge queue via draft PR: #1349.

graphite-app bot pushed a commit that referenced this pull request Oct 5, 2025
@graphite-app graphite-app bot closed this Oct 5, 2025
@graphite-app graphite-app bot deleted the 10-04-chore_core_add_support_for_rivet_endpoint_env_var_as_alternative_to_rivet_engine branch October 5, 2025 00:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant