chore(core): enable overriding default server addr for logs & inspector

[NathanFlurry]

No description provided.

[vercel]

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

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

[claude]

Claude finished @NathanFlurry's task —— View job

---

PR Review: Enable Overriding Default Server Address

Comprehensive review of the changes to enable overriding default server address for logs & inspector.

Tasks:

• Read and analyze all modified files
• Check code quality and adherence to repo conventions
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security implications
• Assess test coverage
• Provide comprehensive feedback

---

🔍 Summary of Changes

This PR introduces functionality to override the default server address for logs and inspector. The main changes include:

1. New configuration field: Added overrideServerAddress to RunConfigSchema in packages/rivetkit/src/registry/run-config.ts:37
2. Inspector URL handling: Updated getInspectorUrl() in packages/rivetkit/src/inspector/utils.ts:68-72 to use the override
3. Welcome message logic: Updated server display logic in packages/rivetkit/src/registry/mod.ts:117-119
4. Driver configuration: Added pass-through for server address overrides in packages/rivetkit/src/drivers/default.ts:32-37
5. Code cleanup: Removed unused imports from packages/rivetkit/src/client/config.ts

✅ Positive Aspects

1. Clear intent: The feature adds useful functionality for customizing server endpoints in different environments
2. Consistent naming: The overrideServerAddress field follows camelCase conventions per CLAUDE.md
3. Proper fallback logic: The inspector uses defaultEndpoint first, then falls back to overrideServerAddress
4. Non-breaking: All changes are additive and maintain backward compatibility

⚠️ Issues & Recommendations

High Priority

1. Missing JSDoc documentation (packages/rivetkit/src/registry/run-config.ts:37)

   • The new overrideServerAddress field lacks documentation
   • Should explain when/why to use this vs other endpoint configurations
   • Consider adding usage examples

2. Unclear relationship between endpoint fields (packages/rivetkit/src/registry/run-config.ts:25-37)

   • Multiple endpoint-related fields: endpoint, overrideServerAddress, inspector.defaultEndpoint
   • No clear documentation of precedence or when to use each
   • Risk of user confusion about which field to set

3. Inconsistent validation patterns (packages/rivetkit/src/registry/run-config.ts:37)

   // Current - no validation
   overrideServerAddress: z.string().optional(),
   
   // Should match endpoint field pattern
   overrideServerAddress: z.string().url().optional(),

Medium Priority

4. Missing input validation (packages/rivetkit/src/inspector/utils.ts:69)

   • No validation that overrideServerAddress is a valid URL
   • Could cause runtime errors if malformed URL is provided

5. Potential runtime issues (packages/rivetkit/src/inspector/utils.ts:71)

   // Current code
   url.searchParams.set("u", overrideDefaultEndpoint);
   
   // Risk: if overrideDefaultEndpoint is not a valid URL base,
   // this could create malformed inspector URLs

6. Test coverage gap

   • No visible tests for the new overrideServerAddress functionality
   • Should test inspector URL generation with various override scenarios
   • Should test welcome message display logic

Low Priority

7. Minor duplication (packages/rivetkit/src/registry/mod.ts:117-119)

   • The conditional logic for endpoint display could be simplified
   • Consider extracting to a helper function for clarity

8. Unused import cleanup (packages/rivetkit/src/client/config.ts)

   • Good cleanup, but verify these imports weren't used elsewhere

🔒 Security Considerations

• URL injection risk: The overrideServerAddress should be validated as a proper URL to prevent injection attacks
• Information disclosure: Ensure override addresses don't accidentally expose internal infrastructure details in logs

🚀 Performance Considerations

• No performance impact: Changes are configuration-only and don't affect runtime performance
• Memory usage: Minimal impact from additional optional string field

🧪 Testing Recommendations

// Suggested test cases
describe('overrideServerAddress', () => {
  it('should use override address in inspector URL when set')
  it('should fallback to inspector.defaultEndpoint when override not set')
  it('should validate override address is a proper URL')
  it('should display override address in welcome message')
  it('should handle undefined/null override gracefully')
})

📋 Action Items

1. Add comprehensive JSDoc for overrideServerAddress field
2. Add URL validation using z.string().url() schema
3. Clarify field relationships in documentation
4. Add unit tests for new functionality
5. Consider extracting endpoint display logic to helper function

🎯 Overall Assessment

Good foundation with room for improvement. The feature is well-implemented but needs better documentation and validation. The changes follow repo conventions and maintain backward compatibility.

Recommendation: Address documentation and validation issues before merging.

---

[NathanFlurry]

• feat: freestyle example #1317
• chore(core): fix passing inspector token for serverless runners #1348
• chore(core): add support for RIVET_ENDPOINT env var as alternative to RIVET_ENGINE #1347
• fix(core): strip basePath from routes for actor gateway on manager router #1346
• chore(core): enable overriding default server addr for logs & inspector #1345 👈 (View in Graphite)
• main

---

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]

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.