Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@railway-app
Copy link

railway-app bot commented Jan 12, 2026

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

Service Status Web Updated (UTC)
website ✅ Success (View Logs) Web Jan 12, 2026 at 9:57 pm
frontend-inspector ❌ Build Failed (View Logs) Web Jan 12, 2026 at 9:56 pm
frontend-cloud ❌ Build Failed (View Logs) Web Jan 12, 2026 at 9:56 pm

Copy link
Member Author

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 12, 2026

PR Review: Environment Variable Renaming (RIVETKIT_* → RIVET_*)

Summary

This PR renames environment variables from RIVETKIT_* to RIVET_* for consistency with the broader Rivet ecosystem. The changes are clean and comprehensive.


✅ Strengths

  1. Comprehensive Coverage: All instances of RIVETKIT_ENDPOINT and RIVETKIT_LOG_* environment variables have been updated across:

    • Core library code (env-vars.ts)
    • Example scripts (3 Cloudflare Workers examples)
    • Test configurations (vitest.base.ts, commented test code)
    • Documentation (architecture.mdx, logging.mdx)
  2. Backward Compatibility: The getLogLevel() function maintains backward compatibility with a fallback to the deprecated LOG_LEVEL env var:

    getEnvUniversal("RIVET_LOG_LEVEL") ?? getEnvUniversal("LOG_LEVEL")

    This is good practice for a smooth migration path.

  3. Improved Error Messages: The error messages in registry/config/index.ts were clarified:

    • Before: "advertiseEndpoint is required in production mode without endpoint"
    • After: "must specify either endpoint or advertiseEndpoint when in production mode" (line 169)
    • After: "advertiseEndpoint not specified in production mode" (line 215)

    These are more precise and easier to understand.

  4. Clean Execution: No remaining references to old RIVETKIT_ENDPOINT or RIVETKIT_LOG_* variables were found in the codebase.


📝 Observations

  1. Function Names Not Updated: The functions in env-vars.ts still use "Rivetkit" in their names:

    • getRivetkitInspectorToken() (line 34)
    • getRivetkitInspectorDisable() (line 36)

    While they now read from RIVET_* environment variables, keeping the old function names maintains API compatibility but may cause confusion. This is acceptable if it's intentional for backward compatibility, but consider documenting this or planning a future deprecation.

  2. Deprecation Comment: The deprecation notice for LOG_LEVEL (line 40) is helpful:

    // DEPRECATED: LOG_LEVEL will be removed in a future version

    Consider adding a similar comment about when/if the old function names will be updated.

  3. Commented Test Code: The changes in driver-tests.test.ts are within commented-out code (lines 204-211). While this is fine, ensure this test code is either:

    • Re-enabled soon with the correct env vars
    • Removed if permanently deprecated

🔍 Potential Issues

No blocking issues found. The changes are straightforward and safe.


🧪 Testing Recommendations

  1. Environment Variable Testing: Verify that all examples still work with the new RIVET_* variables:

    RIVET_ENDPOINT=http://localhost:8787 npm run client
  2. Backward Compatibility: Test that the LOG_LEVEL fallback works as expected for existing users who haven't migrated yet.

  3. Documentation: Confirm that all documentation accurately reflects the new variable names (already done in this PR for architecture.mdx and logging.mdx).


🎯 Minor Suggestions

  1. Migration Guide: Consider adding a brief migration note somewhere visible (e.g., in a CHANGELOG or release notes) to help users transition:

    Breaking Change: Environment variables renamed
    - RIVETKIT_ENDPOINT → RIVET_ENDPOINT
    - RIVETKIT_LOG_* → RIVET_LOG_*
    - RIVETKIT_INSPECTOR_* → RIVET_INSPECTOR_*
    
    The LOG_LEVEL fallback is temporarily maintained for RIVET_LOG_LEVEL.
    
  2. Other RIVETKIT Variables: The codebase still has references to CUSTOM_RIVETKIT_DEVTOOLS_URL and CONN_SPEAKS_RIVETKIT_SYMBOL. These are internal symbols/build variables rather than user-facing environment variables, so they're fine to keep as-is, but document the distinction if needed.


✅ Recommendation

APPROVE - This is a clean, well-executed renaming that improves consistency. The changes are comprehensive, maintain backward compatibility where appropriate, and improve error messaging. No blocking issues identified.


Code Quality: ⭐⭐⭐⭐⭐
Test Coverage: ⭐⭐⭐⭐ (manual testing recommended)
Documentation: ⭐⭐⭐⭐⭐
Security: ✅ No concerns

@NathanFlurry NathanFlurry force-pushed the 01-12-doc_default_to_serverless_pai branch from 12b30bc to d14b367 Compare January 12, 2026 21:57
@NathanFlurry NathanFlurry force-pushed the 01-12-chore_rivetkit_update_rivetkit__env_var_names_to_rivet__ branch from 4833a99 to 4fa140b Compare January 12, 2026 21:57
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3839 January 12, 2026 21:57 Destroyed
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 12, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 4fa140b

@graphite-app graphite-app bot force-pushed the 01-12-doc_default_to_serverless_pai branch from d14b367 to eaef6b8 Compare January 12, 2026 22:00
@graphite-app graphite-app bot force-pushed the 01-12-chore_rivetkit_update_rivetkit__env_var_names_to_rivet__ branch from 4fa140b to 8ea1aba Compare January 12, 2026 22:01
@railway-app railway-app bot temporarily deployed to rivet-frontend / rivet-pr-3839 January 12, 2026 22:01 Destroyed
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 12, 2026

Merge activity

  • Jan 12, 10:01 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Jan 12, 10:02 PM UTC: CI is running for this pull request on a draft pull request (#3842) due to your merge queue CI optimization settings.
  • Jan 12, 10:03 PM UTC: Merged by the Graphite merge queue via draft PR: #3842.

@graphite-app graphite-app bot closed this Jan 12, 2026
@graphite-app graphite-app bot deleted the 01-12-chore_rivetkit_update_rivetkit__env_var_names_to_rivet__ branch January 12, 2026 22:03
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