chore(rivetkit): clean up logging configuration

[NathanFlurry]

No description provided.

[vercel]

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

Project	Deployment	Review	Updated (UTC)
rivet-site	Ready	Preview, Comment	Jan 6, 2026 2:34am
rivetkit-serverless	Error		Jan 6, 2026 2:34am

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
rivet-cloud	Ignored		Jan 6, 2026 2:34am
rivet-inspector	Ignored	Preview	Jan 6, 2026 2:34am

[NathanFlurry]

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

• feat(engine): return error for pending actors #3730 : 2 dependent PRs (#3675 , #3740 )
• chore(gas): add support for find_workflows #3739
• chore(docs): document abort signal #3673
• chore(rivetkit): clean up logging configuration #3672 👈 (View in Graphite)
• chore(rivetkit): RIVET_EXPOSE_ERRORS #3671
• chore(rivetkit): specialize context types for better compiler errors #3669
• fix(rivetkit): fix c.client not respecting correct origin #3668
• fix(rivetkit): allow providing no args to createRivetKit #3667
• main

---

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]

Code Review - PR #3672: Clean up logging configuration

Overview

This PR standardizes RivetKit's logging environment variable naming by adding the RIVETKIT_ prefix to all logging-related configuration variables. This improves clarity and avoids namespace conflicts.

✅ Positive Aspects

1. Consistent Naming Convention: The RIVETKIT_ prefix clearly identifies all RivetKit-specific configuration, which is a good practice for library/framework configuration
2. Comprehensive Coverage: All logging-related variables have been updated consistently across:
   • Source code
   • Tests
   • Documentation
   • Configuration files
3. Documentation Updated: Both the architecture and logging docs have been updated to reflect the new variable names
4. Clean Refactoring: The changes are straightforward variable renames with no logic changes

🔍 Code Quality Observations

Good Practices

• Function Naming: The refactored function names (getLogMessage(), getLogErrorStack(), getLogHeaders()) are clearer than the previous names with mixed prefixes
• Consistent Pattern: All getter functions follow the same pattern with === "1" checks for boolean flags

Minor Code Style Issues

actor-conn.ts:466 - The formatting changes introduce inconsistent line breaks. While the changes are cosmetic and likely from an auto-formatter, they don't match the project's existing style:

// Line 47-50: Added unnecessary line breaks for a simple union type
export type ActorConnStatus =
	| "idle"
	| "connecting"
	| "connected"
	| "disconnected";

actor-conn.ts - Several other formatting changes (lines 287, 413, 469, 601, 1090) add line breaks that seem unnecessary. These appear to be auto-formatter artifacts rather than intentional improvements.

🐛 Potential Issues

None identified - This is a clean refactoring with no logic changes.

🔒 Security Considerations

No security concerns - The changes are purely configuration-related and don't affect security boundaries.

⚡ Performance Considerations

No performance impact - Variable renames and environment variable reads have no performance implications.

🧪 Test Coverage

Good coverage:

• Test configuration files updated (vitest.base.ts, driver-tests.test.ts)
• All new variable names are properly used in test setups
• Existing tests should catch any issues with the refactoring

Recommendation: Run the full test suite to ensure no hardcoded old variable names remain in other test files not shown in this diff.

📋 Recommendations

1. Check for Missed References: Search the entire codebase for any remaining references to the old variable names:

   grep -r "LOG_LEVEL" --include="*.ts" --include="*.tsx" --include="*.md"
   grep -r "LOG_TARGET" --include="*.ts" --include="*.tsx" --include="*.md"
   grep -r "_RIVETKIT_" --include="*.ts" --include="*.tsx" --include="*.md"
   grep -r "_RIVET_LOG_" --include="*.ts" --include="*.tsx" --include="*.md"

2. Add Migration Note: Consider adding a migration guide or changelog entry for users who may have these environment variables configured in their deployments

3. Code Formatting: Based on CLAUDE.md, you should not run cargo fmt automatically. However, the TypeScript formatting changes in this PR seem inconsistent. Consider:

   • Reverting the unnecessary formatting changes in actor-conn.ts
   • Or running the project's TypeScript formatter consistently across all changed files

4. Environment Variable Documentation: The documentation table in logging.mdx is excellent. Consider adding backward compatibility notes if this is a breaking change for existing users.

🎯 Summary

Verdict: ✅ Approve with minor suggestions

This is a well-executed refactoring that improves the configuration interface. The changes are consistent, well-documented, and low-risk. The only concerns are cosmetic formatting changes that may not align with project conventions.

Breaking Change Alert: Users with existing deployments using the old variable names will need to update their configuration. Ensure this is communicated in release notes.

---

Review generated by Claude Code

[claude]

PR Review: chore(rivetkit): clean up logging configuration

Summary

This PR standardizes logging-related environment variables by adding a consistent RIVETKIT_ prefix. The changes are well-organized and improve clarity around what configuration options are available.

Positive Observations

✅ Consistent Naming Convention: The migration from inconsistent prefixes (LOG_*, _RIVETKIT_*, _RIVET_*) to a unified RIVETKIT_LOG_* pattern significantly improves discoverability and clarity.

✅ Comprehensive Updates: All references across tests, documentation, and code have been updated consistently.

✅ Documentation Updated: The logging documentation in website/src/content/docs/general/logging.mdx now documents all available logging configuration options, which is a significant improvement.

✅ No Breaking Logic Changes: The PR purely refactors environment variable names without changing any logging behavior.

Issues Found

1. Potential Breaking Change Not Documented ⚠️

This is a breaking change for existing users who have configured logging using the old environment variable names (LOG_LEVEL, LOG_TARGET, LOG_TIMESTAMP, _RIVETKIT_ERROR_STACK, _RIVETKIT_LOG_MESSAGE, _RIVET_LOG_HEADERS).

Recommendation:

• Add migration notes to the PR description or changelog
• Consider adding temporary backwards compatibility that reads from old env vars with a deprecation warning
• Document the breaking change in release notes

2. Function Rename Inconsistency

In env-vars.ts, some function names were updated while others weren't:

• ✅ Renamed: getRivetkitLogMessage → getLogMessage
• ✅ Renamed: getRivetkitErrorStack → getLogErrorStack
• ✅ Renamed: getRivetLogHeaders → getLogHeaders
• ❌ Not renamed: getRivetkitInspectorToken, getRivetkitInspectorDisable

For consistency, consider either:

• Renaming all getRivetkit* functions to get* for logging-related functions only (current approach), OR
• Keeping the getRivetkit prefix for all RivetKit-specific configs

The current approach is acceptable if logging configs are intentionally treated differently.

3. Inspector Environment Variable Inconsistency

Line 33 in env-vars.ts:

export const getRivetkitInspectorDisable = (): boolean =>
	getEnvUniversal("RIVETKIT_INSPECTOR_DISABLE") === "1";

This changed from checking for truthiness (!!getEnvUniversal(...)) to strict equality (=== "1"). While this is more consistent with other boolean checks in the file, it's technically a behavior change:

Before: Any truthy value would disable the inspector
After: Only the string "1" disables the inspector

Recommendation: If intentional, document this. If unintentional, this should be reverted or called out as a separate fix.

4. Formatting-Only Changes Mixed In

Several formatting-only changes appear in actor-conn.ts:

• Line 47-50: Multi-line type union formatting
• Line 287-288: Conditional formatting
• Line 413-415, 601-604, 1090-1093: Object formatting

These are likely from automatic formatting, but per CLAUDE.md:

DO NOT RUN CARGO FMT AUTOMATICALLY (note for humans: we need to run cargo fmt when everything is merged together and make sure lefthook is working)

While this is Rust-specific advice, the principle applies: formatting changes should ideally be separate from logical changes or at least not manually triggered.

Recommendation: These are minor and don't affect functionality, so acceptable to keep, but be aware of the project's preference against mixing formatting changes.

Code Quality

✅ Proper Error Handling: No changes to error handling logic
✅ Type Safety: All TypeScript types remain intact
✅ Test Coverage: Test configurations updated to use new variable names
✅ Import Cleanup: Removed unused imports in actor-conn.ts

Performance Considerations

No performance impact - this is purely a configuration change.

Security Considerations

No security concerns. The logging configuration options remain safe.

Minor Observations

1. Good: The new RIVETKIT_LOG_MESSAGE and RIVETKIT_LOG_ERROR_STACK variable names are much clearer than the previous underscore-prefixed versions
2. Good: Documentation now includes all logging options in a table format
3. Typo in example: Documentation shows node server.js but this is a Rust/TypeScript workspace - consider using a more appropriate example

Recommendations Summary

Must Address:

• Document the breaking change in PR description/release notes
• Decide on migration strategy (breaking change vs backward compatibility)

Should Consider:

• Review getRivetkitInspectorDisable behavior change (truthiness → strict equality)
• Consider consistent function naming pattern for all RivetKit config getters

Nice to Have:

• Update documentation example command to be more representative of the actual project

Conclusion

This is a well-executed refactoring that improves the developer experience through consistent naming. The primary concern is ensuring users are aware of the breaking changes and have a clear migration path. Once breaking changes are documented, this is ready to merge.

Overall Rating: ⭐⭐⭐⭐ (4/5) - Excellent refactoring, just needs breaking change documentation.

[graphite-app]

Merge activity

• Jan 6, 2:19 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jan 6, 2:20 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3667.
• Jan 6, 2:20 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3667.
• Jan 6, 2:21 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jan 6, 2:22 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3667.
• Jan 6, 2:22 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3667.
• Jan 6, 2:31 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jan 6, 2:31 AM UTC: CI is running for this pull request on a draft pull request (#3754) due to your merge queue CI optimization settings.
• Jan 6, 2:32 AM UTC: Merged by the Graphite merge queue via draft PR: #3754.