chore(rivetkit): add support for conn params on onRequest and onWebSocket

[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	Ready	Preview	Comment	Nov 13, 2025 9:41pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 9:41pm
rivet-inspector	Ignored	Preview		Nov 13, 2025 9:41pm
rivet-site	Ignored	Preview		Nov 13, 2025 9:41pm

[NathanFlurry]

• chore(rivetkit): switch dynamic node imports to use require #3461 : 2 dependent PRs (#3280 , #3455 )
• chore(rivetkit-typescript): remove dependency on node modules #3460
• feat(site): add typedoc docs #3446
• chore(rivetkit): remove deprecated packages #3447
• chore(rivetkit): add support for conn params on onRequest and onWebSocket #3445 👈 (View in Graphite)
• feat(rivetkit): add asyncapi spec #3442
• chore(rivetkit): add actor router to the openapi spec #3441
• fix(rivetkit): skip sending RivetKit messages to conns that do not support it #3440
• fix(rivetkit): fix race condition with websocket open events #3439
• fix(rivetkit): standardize error metadata return type #3438
• fix(rivetkit): remove incorrect getEndpoint call in metadata handler #3433 : 1 other dependent PR (#3451 )
• feat(rivetkit): add json protocol support #3432
• chore(rivetkit): add required zod validation for json encoding #3431
• chore(rivetkit): add listing actors by only name to manager api #3430
• refactor(rivetkit): extract WebSocket protocol parsing to shared utility #3429
• refactor(rivetkit): restructure connection lifecycle contexts #3428
• chore(rivetkit): add WebSocketContext and RequestContext #3427
• chore(rivetkit): add conn type #3426
• chore(rivetkit): add connection for raw http #3425
• chore(rivetkit): rename onFetch to onRequest #3424
• refactor(rivetkit): add state manager for conn with symbol-based access #3423
• fix(rivetkit): use loggerWithoutContext for websocket error when actor undefined #3412
• feat(rivetkit): add GET /actors/names #3411
• feat(api-public): expose actor kv api #3410
• chore(rivetkit): auto-generate inspector token for engine driver #3409
• chore(rivetkit): close websockets on out of sequence messages #3404
• fix(rivetkit): properly apply clientEndpoint from metadata lookup #3402
• fix(rivetkit): properly handle msgIndex for hibernatable websocket reconnection #3401
• chore(rivetkit): rename onstart/onstop to onwake/onsleep #3422
• chore(rivetkit): split ActorInstance logic in to multiple classes #3421
• chore(rivetkit): move conns to separate persisted kv keys #3420
• chore(rivetkit): remove deprecated conn tokens & sse support #3419
• 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

Summary

This PR adds support for passing connection parameters (connParams) to onRequest and onWebSocket handlers in RivetKit. The changes ensure that connection parameters are properly extracted from HTTP headers (for requests) and WebSocket protocols (for WebSocket connections) and passed through to the connection manager.

Positive Observations

✅ Consistent implementation - The changes follow the existing pattern used for the /connect endpoint, applying the same approach to raw request and WebSocket handlers.

✅ Proper use of existing utilities - Leverages getRequestConnParams() and parseWebSocketProtocols() functions that already exist in the codebase.

✅ Complete implementation - Updates all relevant drivers (engine and file-system) to ensure the feature works across different execution environments.

Issues & Recommendations

1. Missing Error Handling in parseWebSocketProtocols ⚠️

Location: rivetkit-typescript/packages/rivetkit/src/actor/router-endpoints.ts:586

The parseWebSocketProtocols function doesn't wrap the JSON.parse call in a try-catch block, but getRequestConnParams does. This inconsistency could lead to unhandled exceptions.

Current code:

const connParams = connParamsRaw ? JSON.parse(connParamsRaw) : undefined;

Recommendation:

let connParams: unknown = undefined;
if (connParamsRaw) {
    try {
        connParams = JSON.parse(connParamsRaw);
    } catch (err) {
        throw new errors.InvalidParams(
            `Invalid conn params JSON in WebSocket protocol: ${stringifyError(err)}`
        );
    }
}

This would match the error handling pattern in getRequestConnParams (lines 553-559).

2. Type Safety Concern with connParams ?? {} 🔍

Location: rivetkit-typescript/packages/rivetkit/src/actor/router-endpoints.ts:420

The fallback connParams ?? {} could mask issues where connParams is unexpectedly null or undefined.

Current code:

const conn = await actor.connectionManager.prepareAndConnectConn(
    driver,
    connParams ?? {},
    newRequest,
);

Consideration: Since getRequestConnParams can return null (line 550) and parseWebSocketProtocols can return undefined, the fallback to {} is appropriate. However, this behavior should be consistent across all call sites.

Current inconsistency:

• Line 352 (raw request): parameters (no fallback)
• Line 420 (raw websocket): connParams ?? {} (with fallback)

Recommendation: Make the behavior consistent. Either:

• Always use ?? {} at all call sites, OR
• Update getRequestConnParams to return {} instead of null on line 550

3. Missing Test Coverage ⚠️

There are no tests validating that:

• Connection parameters from HTTP headers are properly passed to onRequest handlers
• Connection parameters from WebSocket protocols are properly passed to onWebSocket handlers
• Invalid JSON in connection parameters is handled gracefully

Recommendation: Add driver tests similar to the existing connection tests to verify this functionality works end-to-end.

4. Documentation 📝

Since this is a new feature that affects the public API (onRequest and onWebSocket handler signatures now receive connParams), consider:

• Adding JSDoc comments to document the new behavior
• Updating any relevant API documentation or examples

Minor Observations

• The deleted comment on line 112 (// Parse configuration from Sec-WebSocket-Protocol header) was redundant, good cleanup.
• The debug logging addition on line 190 (connParams) is helpful for debugging.

Security Considerations

✅ The JSON parsing is properly wrapped in try-catch blocks (or should be after fixing issue #1)
✅ Uses standard JSON.parse which is safe for untrusted input
✅ Parameters are passed as unknown type, pushing validation to the application layer

Performance

No performance concerns - the changes are minimal and follow existing patterns.

Verdict

The implementation is solid and follows existing conventions well. The main concerns are:

1. Critical: Add error handling to parseWebSocketProtocols JSON parsing
2. Important: Resolve the inconsistency in null/undefined handling for connParams
3. Recommended: Add test coverage

Once these issues are addressed, this will be a clean addition to the codebase.

---

Review generated with assistance from Claude Code

[graphite-app]

Merge activity

• Nov 13, 9:34 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 9:34 PM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3441.
• Nov 13, 9:37 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 9:37 PM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3441.
• Nov 13, 9:43 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 9:43 PM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3441.
• Nov 13, 10:04 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 10:05 PM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3441.
• Nov 13, 10:11 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 10:12 PM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #3442.

[claude]

Code Review

This PR adds support for passing connection parameters (connParams) to onRequest and onWebSocket handlers in RivetKit. The implementation looks solid overall with good consistency. Here are my findings:

✅ Strengths

1. Consistent Implementation: The PR properly threads connParams through both the HTTP request and WebSocket paths, maintaining consistency across different driver implementations (engine and file-system).

2. Reuses Existing Patterns:

   • For raw HTTP requests, it correctly uses the existing getRequestConnParams() helper which reads from the x-rivet-conn-params header
   • For WebSockets, it reuses the existing parseWebSocketProtocols() function to extract connParams from the Sec-WebSocket-Protocol header

3. Proper Fallback Handling: In handleRawWebSocket() at router-endpoints.ts:420, there's a safe fallback: connParams ?? {} which prevents passing undefined when connParams aren't provided.

4. Clean Signature Changes: The function signatures are updated cleanly without breaking existing functionality.

🔍 Observations

1. Deleted Comment: In router.ts:113, a comment was removed:

   - // Parse configuration from Sec-WebSocket-Protocol header

   This seems intentional but the comment was actually helpful for understanding what the next line does. Consider keeping it or enhancing it.

2. Debug Logging Enhancement: Good addition of connParams to the debug log at router.ts:190 - this will help with debugging connection issues.

🧪 Testing Considerations

Important: I notice there are existing (commented-out) tests in /driver-test-suite/tests/raw-http-direct-registry.ts:122 that already include HEADER_CONN_PARAMS in test cases. While these tests are currently disabled, when they are re-enabled, they should verify that:

1. Connection parameters are properly passed through onRequest handlers
2. Connection parameters are properly passed through onWebSocket handlers
3. The parameters are accessible in the actor's connection object

Recommendation: Consider adding or updating driver tests to verify this new functionality works correctly for both:

• Raw HTTP requests via handleRawRequest()
• Raw WebSocket connections via handleRawWebSocket()

📊 Code Quality

• Type Safety: ✅ Properly typed (connParams: unknown | undefined)
• Error Handling: ✅ No new error paths introduced, existing error handling applies
• Null Safety: ✅ Proper handling with ?? operator
• Code Style: ✅ Follows repository patterns

🎯 Summary

This is a clean, well-implemented change that consistently adds connection parameter support across the actor routing layer. The implementation correctly handles both HTTP and WebSocket protocols using existing utility functions.

Approval Status: ✅ Approved with minor suggestion to consider test coverage

Minor Suggestion: Consider restoring or improving the removed comment in router.ts to maintain code documentation clarity.

[claude]

Code Review

Summary

This PR adds support for connection parameters (connParams) on onRequest and onWebSocket handlers. The changes ensure that connection parameters are properly extracted and passed through to the connection manager for both raw HTTP requests and WebSocket connections.

Positives

1. Consistent Parameter Passing: The changes ensure that connParams are consistently passed through the entire request/WebSocket handling chain, from router endpoints to the connection manager.

2. Proper Fallback Handling: In handleRawWebSocket, there is a sensible fallback (connParams ?? {}) to ensure an empty object is passed if no parameters are provided.

3. Clean Implementation: The changes are minimal and focused, affecting only the necessary touchpoints without over-engineering.

4. Logging Added: The debug logging in router.ts:184-191 includes the new connParams field, which is helpful for debugging.

Issues and Concerns

1. Inconsistent Parameter Extraction (Medium Priority)

In handleRawRequest (router-endpoints.ts:344), the parameters are extracted from headers using getRequestConnParams(c.req), while for WebSocket connections, parameters are extracted using parseWebSocketProtocols(protocols) from the Sec-WebSocket-Protocol header. This means HTTP requests use the HEADER_CONN_PARAMS header while WebSocket requests use the Sec-WebSocket-Protocol header.

Question: Is this intentional? Should both code paths support both methods of parameter passing? Consider documenting this behavior or making it consistent.

2. Missing Type Safety (Medium Priority)

The connParams parameter in handleRawWebSocket is typed as unknown | undefined. While this is flexible, it provides no type safety. Consider adding JSDoc comments explaining what structure is expected, or adding runtime validation if the structure is critical.

3. Missing Test Coverage (High Priority)

This change modifies critical connection handling code but does not include any tests. I could not find existing tests that verify connection parameters are properly passed through.

Recommendation: Add tests covering:

• Raw HTTP requests with connection parameters via headers
• WebSocket connections with parameters via Sec-WebSocket-Protocol
• WebSocket connections with no parameters (fallback behavior)
• Invalid/malformed connection parameters (error handling)

You could add these to tests/driver-file-system.test.ts or tests/driver-engine.test.ts.

4. Error Handling for HTTP Requests (Low Priority)

In handleRawRequest, getRequestConnParams can throw an InvalidParams error if JSON parsing fails. However, there is no try-catch around the connection creation. The error will bubble up, which is probably fine, but it is worth confirming this is the intended behavior.

5. Documentation Gap (Low Priority)

The PR description is empty. Consider adding:

• Why this change was needed
• How connection parameters should be used
• Examples of valid parameter structures
• Any breaking changes or migration notes

Minor Observations

1. Code Comment Removed: Line 113 in router.ts removed the comment about parsing configuration from Sec-WebSocket-Protocol header. While the code is self-explanatory, the comment provided useful context.

2. Parameter Naming: The PR uses both params (file-system/manager.ts:192) and connParams (router.ts:182). Ensuring consistency would improve readability.

Security Considerations

1. Input Validation: The getRequestConnParams function parses JSON from headers but only validates it is valid JSON, not the structure. Ensure that actors validate the parameters they receive to prevent injection or unexpected behavior.

2. Protocol Header Parsing: WebSocket protocol headers are user-controlled. The parseWebSocketProtocols function should be reviewed to ensure it properly handles malicious input.

Performance Considerations

No significant performance concerns. The added header/protocol parsing is minimal overhead.

Recommendations

Before merging:

1. Add test coverage for the new functionality
2. Consider documenting the parameter passing mechanisms (headers vs WebSocket protocols)
3. Add a PR description explaining the motivation and usage

Future improvements:

1. Type safety improvements for connParams
2. Unified parameter extraction for both HTTP and WebSocket
3. Runtime validation helpers for connection parameters

Conclusion

The implementation is clean and focused, but would benefit from test coverage and better documentation. The core functionality appears sound, assuming the dual parameter-passing mechanisms (headers for HTTP, WebSocket protocols for WS) are intentional.

Recommendation: Request changes - primarily to add test coverage before merging.

---

Review generated by Claude Code