chore(core): add token & optional headers to client #1296

NathanFlurry · 2025-09-24T21:31:14Z

No description provided.

vercel · 2025-09-24T21:31:18Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Sep 26, 2025 0:42am

NathanFlurry · 2025-09-24T21:31:31Z

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 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.

claude · 2025-09-24T21:31:34Z

Claude finished @NathanFlurry's task —— View job

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-82 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Minor Issues:

Type safety: config.headers is typed as Record<string, string> but could be undefined despite the default. Consider using config.headers ?? {} for extra safety.
- Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:37
Header precedence: Custom headers from config.headers are set before system headers, which means user headers could potentially override system headers like HEADER_RIVET_TOKEN.
- Location: packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-88

Suggested fix for header precedence:

// Add extra headers from config first
for (const [key, value] of Object.entries(runConfig.headers ?? {})) {
  headers.set(key, value);
}
// Add guard-specific headers (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data (not logged in debug output)
✅ WebSocket token transmission uses secure protocol headers vs URL parameters
✅ Authorization header follows RFC standards (Bearer ${token})
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Location: packages/rivetkit/src/common/actor-router-consts.ts:56 - Token header in CORS allowlist
Location: packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65 - Secure WebSocket token transmission

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
Existing test files don't cover the new configuration options

Recommendations:

Add unit tests for ClientConfigSchema validation
Add integration tests verifying token/headers are properly transmitted
Test edge cases (empty headers, invalid tokens, etc.)

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants
Backward compatibility maintained with optional fields

Documentation:

Code is well-commented, especially the security note about E2EE params
JSDoc comments could be added to new schema fields for better IDE support

🎯 Recommendations

High Priority: Fix header precedence issue to prevent user headers from overriding system headers
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to the new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR.

pkg-pr-new · 2025-09-24T21:33:03Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

commit: ab42017

claude · 2025-09-26T00:00:53Z

Claude finished @NathanFlurry's task —— View job

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-82 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Minor Issues:

Type safety: config.headers is typed as Record<string, string> but could be undefined despite the default. Consider using config.headers ?? {} for extra safety.
- Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:37
Header precedence: Custom headers from config.headers are set before system headers, which means user headers could potentially override system headers like HEADER_RIVET_TOKEN.
- Location: packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-88

Suggested fix for header precedence:

// Add extra headers from config first
for (const [key, value] of Object.entries(runConfig.headers ?? {})) {
  headers.set(key, value);
}
// Add guard-specific headers (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data (not logged in debug output)
✅ WebSocket token transmission uses secure protocol headers vs URL parameters
✅ Authorization header follows RFC standards (Bearer ${token})
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Location: packages/rivetkit/src/common/actor-router-consts.ts:56 - Token header in CORS allowlist
Location: packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65 - Secure WebSocket token transmission

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
Existing test files don't cover the new configuration options

Recommendations:

Add unit tests for ClientConfigSchema validation
Add integration tests verifying token/headers are properly transmitted
Test edge cases (empty headers, invalid tokens, etc.)

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants
Backward compatibility maintained with optional fields

Documentation:

Code is well-commented, especially the security note about E2EE params
JSDoc comments could be added to new schema fields for better IDE support

🎯 Recommendations

High Priority: Fix header precedence issue to prevent user headers from overriding system headers
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to the new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR.

claude · 2025-09-26T00:02:33Z

Claude finished @NathanFlurry's task —— View job

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-82 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Minor Issues:

Type safety: config.headers could benefit from defensive coding pattern using config.headers ?? {} for extra safety, though the default should handle this.
- Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:37
Header precedence: Custom headers from config.headers are set before system headers, which could potentially allow user headers to override critical system headers like HEADER_RIVET_TOKEN.
- Location: packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-88

Suggested fix for header precedence:

// Copy all headers from the original request
for (const [key, value] of actorRequest.headers.entries()) {
  headers.set(key, value);
}
// Add extra headers from config
for (const [key, value] of Object.entries(runConfig.headers ?? {})) {
  headers.set(key, value);
}
// Add guard-specific headers (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields
Object.entries() iteration is efficient for typical header counts

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data
✅ WebSocket token transmission uses secure protocol headers vs URL parameters
✅ Authorization header follows RFC standards (Bearer ${token})
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Location: packages/rivetkit/src/common/actor-router-consts.ts:56 - Token header in CORS allowlist
Location: packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65 - Secure WebSocket token transmission
Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:41-43 - Standard Bearer token format

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
Existing test files don't cover the new configuration options
No validation tests for ClientConfigSchema with the new fields

Recommendations:

Add unit tests for ClientConfigSchema validation with various token/header combinations
Add integration tests verifying token/headers are properly transmitted in both HTTP and WebSocket requests
Test edge cases (empty headers object, null token, invalid token format, etc.)
Test environment variable detection for RIVET_TOKEN

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants (HEADER_RIVET_TOKEN)
Backward compatibility maintained with optional fields and defaults
Consistent with existing patterns in the codebase

Documentation & Maintainability:

Code is well-commented, especially the security note about E2EE params
JSDoc comments could be added to new schema fields for better IDE support
Implementation follows existing conventions from CLAUDE.md

🎯 Recommendations

High Priority: Fix header precedence issue to ensure system headers always take precedence over user headers
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to the new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern consistently

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR to ensure long-term maintainability.

Summary of Changes:

✅ Adds token field with env var support (RIVET_TOKEN)
✅ Adds headers field for custom header injection
✅ Implements secure token transmission for both HTTP and WebSocket
✅ Maintains backward compatibility
⚠️ Minor header precedence fix needed
⚠️ Test coverage needed for new functionality

claude · 2025-09-26T00:03:01Z

Claude finished @NathanFlurry's task —— View job

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-82 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Minor Issues:

Type safety: config.headers is typed as Record<string, string> but could be undefined despite the default. Consider using config.headers ?? {} for extra safety.
- Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:37
Header precedence: Custom headers from config.headers are set before system headers, which means user headers could potentially override system headers like HEADER_RIVET_TOKEN.
- Location: packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:79-88

Suggested fix for header precedence:

// Add extra headers from config first
for (const [key, value] of Object.entries(runConfig.headers ?? {})) {
  headers.set(key, value);
}
// Add guard-specific headers (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data (not logged in debug output)
✅ WebSocket token transmission uses secure protocol headers vs URL parameters
✅ Authorization header follows RFC standards (Bearer ${token})
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Location: packages/rivetkit/src/common/actor-router-consts.ts:56 - Token header in CORS allowlist
Location: packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65 - Secure WebSocket token transmission

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
Existing test files don't cover the new configuration options

Recommendations:

Add unit tests for ClientConfigSchema validation
Add integration tests verifying token/headers are properly transmitted
Test edge cases (empty headers, invalid tokens, etc.)

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants
Backward compatibility maintained with optional fields

Documentation:

Code is well-commented, especially the security note about E2EE params
JSDoc comments could be added to new schema fields for better IDE support

🎯 Recommendations

High Priority: Fix header precedence issue to prevent user headers from overriding system headers
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to the new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR.

claude · 2025-09-26T00:42:07Z

Claude finished @NathanFlurry's task —— View job

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:75-82 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Critical Issue - Header Precedence:
The current implementation in actor-http-client.ts:75-82 allows user-defined headers to potentially override system headers. This could break authentication if a user accidentally sets x-rivet-token or other critical headers.

// Current order (problematic):
for (const [key, value] of Object.entries(runConfig.headers)) {
  headers.set(key, value); // User headers added first
}
headers.set(HEADER_RIVET_TARGET, "actor"); // Could be overridden
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token); // Could be overridden
}

Recommended fix:

// Copy all headers from the original request
for (const [key, value] of actorRequest.headers.entries()) {
  headers.set(key, value);
}
// Add user headers
for (const [key, value] of Object.entries(runConfig.headers)) {
  headers.set(key, value);
}
// Add system headers last (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Minor Issues:

Type safety: In api-utils.ts:37, consider using config.headers ?? {} for extra defensive coding, though the schema default should handle this.

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields
Object.entries() iteration is efficient for typical header counts

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data
✅ WebSocket token transmission uses secure protocol headers vs URL parameters (packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65)
✅ Authorization header follows RFC standards (Bearer ${token}) in api-utils.ts:41-43
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS (actor-router-consts.ts:56)
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Token is not logged in debug output
Proper separation between HTTP and WebSocket authentication mechanisms
Uses existing security patterns from the codebase

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
No validation tests for ClientConfigSchema with the new fields
No tests verifying headers are properly transmitted in HTTP/WebSocket requests

Recommendations:

Add unit tests for ClientConfigSchema validation with various token/header combinations
Add integration tests verifying token/headers are properly transmitted
Test edge cases (empty headers, invalid tokens, environment variable detection)
Test that system headers take precedence over user headers

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants (HEADER_RIVET_TOKEN)
Backward compatibility maintained with optional fields and sensible defaults
Follows existing code patterns from the project

Documentation & Maintainability:

Code includes helpful comments, especially the security note about E2EE params
Implementation is consistent with CLAUDE.md guidelines
JSDoc comments could be added to new schema fields for better IDE support

🎯 Recommendations

High Priority: Fix header precedence issue in actor-http-client.ts to ensure system headers always take precedence
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern consistently

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR.

Summary of Changes:

✅ Adds token field with env var support (RIVET_TOKEN)
✅ Adds headers field for custom header injection
✅ Implements secure token transmission for both HTTP and WebSocket
✅ Maintains backward compatibility
⚠️ Header precedence fix needed to prevent user headers from overriding system headers
⚠️ Test coverage needed for new functionality

claude · 2025-09-26T00:42:12Z

Claude finished @NathanFlurry's task —— View job

Claude is reviewing PR #1296

PR Review: Add Token & Optional Headers to Client

Reviewing PR #1296 for code quality, potential issues, performance, security, and test coverage.

Todo List:

Read repository guidelines (CLAUDE.md)
Read and analyze all modified files
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Evaluate test coverage
Provide comprehensive feedback

✅ Overall Assessment

This PR adds valuable authentication and customization capabilities by introducing token and headers fields to the client configuration. The implementation is well-structured and follows existing patterns in the codebase.

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Strengths:

Consistent pattern: The implementation follows existing header handling patterns (HEADER_RIVET_TOKEN, HEADER_CONN_TOKEN)
Proper separation: Token logic is cleanly separated between HTTP (Authorization header) and WebSocket (protocol) transports
Environment variable support: Good use of getEnvUniversal("RIVET_TOKEN") for automatic token detection
Zod validation: Proper schema definition with sensible defaults

Code locations:

packages/rivetkit/src/client/config.ts:17-22 - Clean schema definition
packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:75-83 - Proper header merging
packages/rivetkit/src/remote-manager-driver/api-utils.ts:36-43 - Correct Authorization header usage

Potential Issues ⚠️

Minor Issues:

Type safety: While config.headers has a default value of {}, the type is Record<string, string> | undefined. Consider using config.headers ?? {} for extra safety.
- Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:37
Header precedence: Custom headers from config.headers are set before system headers, which means user headers could potentially override system headers like HEADER_RIVET_TOKEN.
- Location: packages/rivetkit/src/remote-manager-driver/actor-http-client.ts:75-83

Suggested fix for header precedence:

// Copy all headers from the original request
for (const [key, value] of actorRequest.headers.entries()) {
  headers.set(key, value);
}
// Add extra headers from config
for (const [key, value] of Object.entries(runConfig.headers ?? {})) {
  headers.set(key, value);
}
// Add guard-specific headers (these should take precedence)
headers.set(HEADER_RIVET_TARGET, "actor");
headers.set(HEADER_RIVET_ACTOR, actorId);
if (runConfig.token) {
  headers.set(HEADER_RIVET_TOKEN, runConfig.token);
}

Performance Considerations ⭐⭐⭐⭐⭐

Good performance characteristics:

Headers are only processed when needed
No unnecessary copying or transformations
Minimal memory overhead for the new fields
Object.entries() iteration is efficient for typical header counts

Security Assessment ⭐⭐⭐⭐⭐

Strong security implementation:

✅ Tokens are properly handled as sensitive data
✅ WebSocket token transmission uses secure protocol headers vs URL parameters
✅ Authorization header follows RFC standards (Bearer ${token})
✅ HEADER_RIVET_TOKEN is included in ALLOWED_PUBLIC_HEADERS for CORS
✅ Environment variable support allows secure configuration without hardcoding

Security best practices followed:

Location: packages/rivetkit/src/common/actor-router-consts.ts:56 - Token header in CORS allowlist
Location: packages/rivetkit/src/remote-manager-driver/actor-websocket-client.ts:63-65 - Secure WebSocket token transmission
Location: packages/rivetkit/src/remote-manager-driver/api-utils.ts:41-43 - Standard Bearer token format

Test Coverage ⚠️

Missing test coverage:

No specific tests found for the new token and headers functionality
Existing test files don't cover the new configuration options
No validation tests for ClientConfigSchema with the new fields

Recommendations:

Add unit tests for ClientConfigSchema validation with various token/header combinations
Add integration tests verifying token/headers are properly transmitted in both HTTP and WebSocket requests
Test edge cases (empty headers object, null token, invalid token format, etc.)
Test environment variable detection for RIVET_TOKEN

Additional Observations

Good architectural decisions:

Clean separation between HTTP and WebSocket token handling
Reuse of existing header constants (HEADER_RIVET_TOKEN)
Backward compatibility maintained with optional fields and defaults
Consistent with existing patterns in the codebase

Documentation & Maintainability:

Code is well-commented, especially the security note about E2EE params
JSDoc comments could be added to new schema fields for better IDE support
Implementation follows existing conventions from CLAUDE.md

🎯 Recommendations

High Priority: Fix header precedence issue to ensure system headers always take precedence over user headers
Medium Priority: Add comprehensive test coverage for the new functionality
Low Priority: Consider adding JSDoc comments to the new schema fields
Low Priority: Use defensive coding with config.headers ?? {} pattern consistently

✅ Approval Status

This PR is ready to merge after addressing the header precedence issue. The implementation is solid, secure, and follows good practices. The missing test coverage should be addressed in a follow-up PR to ensure long-term maintainability.

Summary of Changes:

✅ Adds token field with env var support (RIVET_TOKEN)
✅ Adds headers field for custom header injection
✅ Implements secure token transmission for both HTTP and WebSocket
✅ Maintains backward compatibility
⚠️ Minor header precedence fix needed
⚠️ Test coverage needed for new functionality

graphite-app · 2025-09-26T07:06:08Z

Merge activity

Sep 26, 7:06 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Sep 26, 7:06 AM UTC: CI is running for this pull request on a draft pull request (#1312) due to your merge queue CI optimization settings.
Sep 26, 7:07 AM UTC: Merged by the Graphite merge queue via draft PR: #1312.

vercel bot had a problem deploying to Preview September 24, 2025 21:32 Failure

This was referenced Sep 24, 2025

fix(core): clean up websocket protocol handling #1297

Closed

chore(core): clean up token configs #1298

Closed

jog1t mentioned this pull request Sep 25, 2025

fix(inspector): allow connection to inspector when using engine driver #1269

Closed

NathanFlurry force-pushed the 09-24-chore_core_add_token_optional_headers_to_client branch from d7f6752 to ab42017 Compare September 26, 2025 00:00

NathanFlurry changed the base branch from main to graphite-base/1296 September 26, 2025 00:02

NathanFlurry force-pushed the 09-24-chore_core_add_token_optional_headers_to_client branch from ab42017 to 897b60a Compare September 26, 2025 00:02

NathanFlurry changed the base branch from graphite-base/1296 to 09-18-feat_serverless September 26, 2025 00:02

MasterPtato mentioned this pull request Sep 26, 2025

feat: serverless #1288

Closed

jog1t mentioned this pull request Sep 26, 2025

09-16-fix_next-js_fix_example_for_next-js #1250

Closed

NathanFlurry mentioned this pull request Sep 26, 2025

fix(core): fix missing ManagerDriver impls #1308

Closed

vercel bot had a problem deploying to Preview September 26, 2025 00:18 Failure

chore(core): add token & optional headers to client

72edb6d

NathanFlurry force-pushed the 09-18-feat_serverless branch from 8313206 to c313f20 Compare September 26, 2025 00:41

NathanFlurry force-pushed the 09-24-chore_core_add_token_optional_headers_to_client branch from 897b60a to 72edb6d Compare September 26, 2025 00:41

vercel bot had a problem deploying to Preview September 26, 2025 00:42 Failure

This was referenced Sep 26, 2025

chore(core): simplify connection handling & fix graceful reconnect #1309

Closed

chore(core): reimpl reconnect logic #1310

Closed

fix(core): fix getters on ActorConn proxy #1311

Closed

graphite-app bot pushed a commit that referenced this pull request Sep 26, 2025

chore(core): add token & optional headers to client (#1296)

a40f4e3

graphite-app bot closed this Sep 26, 2025

graphite-app bot deleted the 09-24-chore_core_add_token_optional_headers_to_client branch September 26, 2025 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(core): add token & optional headers to client #1296

chore(core): add token & optional headers to client #1296

Uh oh!

NathanFlurry commented Sep 24, 2025

Uh oh!

vercel bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Sep 24, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

graphite-app bot commented Sep 26, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chore(core): add token & optional headers to client #1296

chore(core): add token & optional headers to client #1296

Uh oh!

Conversation

NathanFlurry commented Sep 24, 2025

Uh oh!

vercel bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NathanFlurry commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add Token & Optional Headers to Client

Todo List:

✅ Overall Assessment

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Potential Issues ⚠️

Performance Considerations ⭐⭐⭐⭐⭐

Security Assessment ⭐⭐⭐⭐⭐

Test Coverage ⚠️

Additional Observations

🎯 Recommendations

✅ Approval Status

Uh oh!

pkg-pr-new bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add Token & Optional Headers to Client

Todo List:

✅ Overall Assessment

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Potential Issues ⚠️

Performance Considerations ⭐⭐⭐⭐⭐

Security Assessment ⭐⭐⭐⭐⭐

Test Coverage ⚠️

Additional Observations

🎯 Recommendations

✅ Approval Status

Uh oh!

claude bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add Token & Optional Headers to Client

Todo List:

✅ Overall Assessment

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Potential Issues ⚠️

Performance Considerations ⭐⭐⭐⭐⭐

Security Assessment ⭐⭐⭐⭐⭐

Test Coverage ⚠️

Additional Observations

🎯 Recommendations

✅ Approval Status

Uh oh!

claude bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add Token & Optional Headers to Client

Todo List:

✅ Overall Assessment

🔍 Detailed Analysis

Code Quality & Best Practices ⭐⭐⭐⭐⭐

Potential Issues ⚠️

Performance Considerations ⭐⭐⭐⭐⭐

Security Assessment ⭐⭐⭐⭐⭐

Test Coverage ⚠️

Additional Observations

🎯 Recommendations

✅ Approval Status

Uh oh!

claude bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add Token & Optional Headers to Client

Todo List:

vercel bot commented Sep 24, 2025 •

edited

Loading

NathanFlurry commented Sep 24, 2025 •

edited

Loading

claude bot commented Sep 24, 2025 •

edited

Loading

pkg-pr-new bot commented Sep 24, 2025 •

edited

Loading

claude bot commented Sep 26, 2025 •

edited

Loading

claude bot commented Sep 26, 2025 •

edited

Loading

claude bot commented Sep 26, 2025 •

edited

Loading

claude bot commented Sep 26, 2025 •

edited

Loading

claude bot commented Sep 26, 2025 •

edited

Loading

graphite-app bot commented Sep 26, 2025 •

edited

Loading