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 2, 2026 11:37pm
rivetkit-serverless	Error		Jan 2, 2026 11:37pm

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
rivet-cloud	Ignored		Jan 2, 2026 11:37pm
rivet-inspector	Ignored	Preview	Jan 2, 2026 11:37pm

[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(dash): surface actor errors in ui #3743
• fix(dash): use ineligible icon for runners #3742
• feat(dash): show runner pool errors #3741
• chore: update engine-full sdk #3740
• feat(engine): return error for pending actors #3730
• 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]

Code Review

This PR standardizes the logging configuration environment variables with a consistent RIVETKIT_ prefix. Overall, the changes are well-executed and improve consistency. Here's my detailed feedback:

✅ Strengths

1. Consistent Naming Convention: All logging-related environment variables now follow a clear RIVETKIT_LOG_* pattern, making them easier to discover and understand.

2. Improved Documentation: The documentation updates in website/src/content/docs/general/logging.mdx now comprehensively document all logging configuration options.

3. Clean Refactoring: Function names have been appropriately updated to match the new environment variable names (e.g., getRivetkitLogMessage → getLogMessage, getRivetkitErrorStack → getLogErrorStack).

4. Thorough Coverage: All references across test files, documentation, and source code have been updated consistently.

5. Code Formatting: Good attention to code style consistency (e.g., proper line breaking in conditional statements).

---

📝 Minor Observations

1. Inconsistent Inspector Variable Naming

In env-vars.ts:32-33, this variable now checks for "1" explicitly (consistent with other boolean env vars), but previously used \!\! for truthy checking. This is a behavioral change:

• Before: Any truthy value would enable it
• After: Only the string "1" enables it

If this is intentional for consistency, this is good. If not, it could break existing deployments that use other truthy values.

2. Removed Prefix Underscores

The PR removes the leading underscores from internal environment variables:

• _RIVETKIT_LOG_MESSAGE → RIVETKIT_LOG_MESSAGE
• _RIVETKIT_ERROR_STACK → RIVETKIT_LOG_ERROR_STACK
• _RIVET_LOG_HEADERS → RIVETKIT_LOG_HEADERS

Good: This makes the API more public and consistent. The underscore prefix is typically used for "private" or "internal" variables, so removing it suggests these are now officially supported configuration options.

---

🔍 Potential Issues

Breaking Change for Existing Users

This is a breaking change for anyone currently using the old environment variable names:

• LOG_LEVEL → RIVETKIT_LOG_LEVEL
• LOG_TARGET → RIVETKIT_LOG_TARGET
• LOG_TIMESTAMP → RIVETKIT_LOG_TIMESTAMP
• etc.

Recommendation:

• Ensure this is documented in release notes or a migration guide
• Consider adding deprecation warnings or backward compatibility support (e.g., fallback to old names if new names aren't set) if you want to provide a smoother migration path

However, clean breaks can be preferable to avoid tech debt - this is a project decision.

---

🧪 Testing Considerations

✅ Good: Test configuration files have been updated

⚠️ Missing: No explicit tests verify:

1. The environment variables are correctly read
2. Logging behavior changes based on these variables
3. Backward compatibility (if that's desired)

Recommendation: Consider adding unit tests for the env-vars.ts helper functions.

---

🔒 Security

✅ No security concerns identified. The changes are purely refactoring of configuration variable names.

---

⚡ Performance

✅ No performance impact. The changes are configuration-related with no algorithmic changes.

---

📚 Documentation

✅ Excellent documentation updates in the logging and architecture docs.

Suggestion: Consider adding a migration note or changelog entry mentioning the environment variable naming changes.

---

🎯 Summary

Aspect	Rating	Notes
Code Quality	✅ Excellent	Clean, consistent refactoring
Best Practices	✅ Good	Follows naming conventions well
Potential Bugs	⚠️ Minor	Breaking change - needs migration plan
Performance	✅ No impact	Configuration changes only
Security	✅ No issues	No security concerns
Test Coverage	⚠️ Could improve	Tests updated but no new coverage for env var reading

---

✅ Recommendation

Approve with suggestions: This is a solid refactoring that improves consistency. The main consideration is the breaking change - ensure this is:

1. Documented in release notes
2. Communicated to users
3. Either supported with backward compatibility OR clearly marked as a breaking change

Specific Action Items:

1. Add migration guide or release notes documenting the environment variable name changes
2. Consider whether RIVETKIT_INSPECTOR_DISABLE behavior change (truthy → "1" only) needs documentation
3. Optional: Add unit tests for env var helper functions
4. Optional: Consider backward compatibility fallbacks if this needs to be a gentler migration

Great work on improving the consistency of the configuration API! 🚀