chore(rivetkit): remove unneeded headers & protocols in requests

[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 5, 2025 4:06am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 5, 2025 4:06am
rivet-inspector	Ignored	Preview		Nov 5, 2025 4:06am
rivet-site	Ignored	Preview		Nov 5, 2025 4:06am

[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

• chore(pegboard): allow configuring reschedule_backoff_max_exponent #3370
• fix(next-js): auto shut down runners when source code updates #3369
• fix(rivetkit): fix duplicate runner key in serverless #3366
• chore(runner): improve runner connection logs #3365
• chore(rivetkit): add runner sse ping interval #3362
• chore(rivetkit): log close code when client ws closes #3361
• chore(rivetkit): log runner id with actor logs #3360
• chore(scripts): add endian converter script #3359
• fix(guard): fix url decoding actor id + token #3357
• chore(rivetkit): remove unneeded headers & protocols in requests #3356 👈 (View in Graphite)
• fix(rivetkit): reduce pegboard actor retry for spawned engines #3355
• chore(pegboard): allow configuring base_retry_timeout, actor_start_threshold, actor_stop_threshold, and retry_reset_duration #3354
• feat(pb): add reschedule_ts to actors #3363
• fix: fix all dashboards #3358
• chore: change log levels #3333
• chore: misc bug fixes, add logs for outbound req #3332
• fix: use correct vbare functions for runner tunnel #3300 : 1 other dependent PR (#3301 )
• feat(grafana): fix api dashboards #3193
• fix: docker compose build #3273
• chore: fix remaining warnings for all targets #3305
• fix(gas): fix from_workflow when creating standalone ctx from op ctx #3342
• ci: udpate ci filter to succeed if no relevant changes #3343
• 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]

PR Review: Remove Unneeded Headers & Protocols in Requests

Summary

This PR removes redundant routing information from client requests. The HEADER_RIVET_ACTOR header (HTTP) and WS_PROTOCOL_ACTOR protocol (WebSocket) are no longer sent by the client since the actor ID is already embedded in the URL path.

Code Quality - Approved

Positive aspects:

• Clean removal of redundant code
• Follows DRY principle by eliminating duplicate routing information
• Simplifies the client-side API surface
• Maintains backward compatibility since the gateway still accepts these headers/protocols from other sources

Architecture notes:

• The gateway at manager/gateway.ts:241-242 and manager/gateway.ts:303 still parses these values from headers/protocols, which is correct for supporting other clients/transports
• The URL-based routing is the primary routing mechanism, making header/protocol-based actor ID transmission redundant for the remote manager driver

Potential Issues

1. Missing cleanup in test infrastructure
The test-inline-client-driver.ts at line 210 still includes the WS_PROTOCOL_ACTOR protocol. This is actually correct - the test driver uses a different transport mechanism that doesn't have the actor ID in the URL path, so it needs the protocol. However, this should be documented to avoid confusion.

2. Consider updating actor-router-consts.ts
The constants HEADER_RIVET_ACTOR (line 25) and WS_PROTOCOL_ACTOR (line 32) are still exported and remain in ALLOWED_PUBLIC_HEADERS (line 58). While they're still used by the gateway and other parts of the system, consider adding a comment indicating they're for gateway/internal use only, not for remote manager driver clients.

Security Considerations - Approved

• Reducing the number of places sensitive routing information appears is a minor security improvement
• The HEADER_RIVET_TOKEN is still properly included, maintaining authentication
• No security concerns introduced by this change

Performance Considerations - Approved

• Minor performance improvement by reducing header/protocol overhead
• Negligible impact, but every byte counts in high-throughput scenarios

Test Coverage - Needs Verification

Concern: The PR doesn't show any test updates or verification that this change doesn't break existing functionality.

Recommendations:

1. Run the full driver test suite to verify WebSocket connections still work
2. Specifically verify tests in driver-test-suite/tests/

Recommendations

1. Add a comment in the PR description explaining the rationale (actor ID in URL path makes headers/protocols redundant)
2. Run integration tests to verify the gateway still correctly routes requests
3. Consider documenting the routing mechanism in actor-router-consts.ts to clarify when headers/protocols vs URL-based routing should be used
4. Optional: Add a migration note if this is a breaking change for any third-party clients (though it appears to be purely internal to remote-manager-driver)

Verdict: Approve with Minor Suggestions

This is a solid cleanup PR that removes redundant code. The implementation is correct, and the change is safe since the gateway continues to support both routing mechanisms. My only concern is ensuring adequate test coverage for this change.

Great work on keeping the codebase clean!

[claude]

PR Review: Remove Unneeded Headers & Protocols in Requests

Summary

This PR removes redundant routing information from client requests. The HEADER_RIVET_ACTOR header (HTTP) and WS_PROTOCOL_ACTOR protocol (WebSocket) are no longer sent by the remote manager driver client since the actor ID is already embedded in the URL path.

Code Quality ✅ Approved

Positive aspects:

• Clean removal of redundant code
• Follows DRY principle by eliminating duplicate routing information
• Simplifies the client-side API surface
• Maintains backward compatibility since the gateway still accepts these headers/protocols from other sources

Architecture notes:

• The gateway at manager/gateway.ts:241-242 and manager/gateway.ts:303 still parses these values from headers/protocols, which is correct for supporting other clients/transports
• The URL-based routing is the primary routing mechanism, making header/protocol-based actor ID transmission redundant for the remote manager driver

Potential Issues

1. Missing cleanup in Cloudflare Workers driver

The cloudflare-workers/src/manager-driver.ts file still includes WS_PROTOCOL_ACTOR at lines 97 and 218:

// Line 97 in openWebSocket
protocols.push(`${WS_PROTOCOL_ACTOR}${actorId}`);

// Line 218 in proxyWebSocket  
protocols.push(`${WS_PROTOCOL_ACTOR}${actorId}`);

This appears to be the same redundant pattern that was removed from remote-manager-driver. Consider removing it there as well for consistency.

2. Test inline client driver is intentionally different

The test-inline-client-driver.ts at line 210 still includes the WS_PROTOCOL_ACTOR protocol. This is actually correct - the test driver uses a different transport mechanism that does not have the actor ID in the URL path, so it needs the protocol. However, this could be documented to avoid confusion.

3. Consider adding clarifying comments

The constants HEADER_RIVET_ACTOR (line 25) and WS_PROTOCOL_ACTOR (line 32) in actor-router-consts.ts are still exported and remain in ALLOWED_PUBLIC_HEADERS (line 58). While they are still used by the gateway and other parts of the system, consider adding a comment indicating they are for gateway/internal use only, not for remote manager driver clients.

Security Considerations ✅ Approved

• Reducing the number of places sensitive routing information appears is a minor security improvement
• The HEADER_RIVET_TOKEN is still properly included, maintaining authentication
• No security concerns introduced by this change

Performance Considerations ✅ Approved

• Minor performance improvement by reducing header/protocol overhead
• Negligible impact, but every byte counts in high-throughput scenarios

Test Coverage ⚠️ Needs Verification

Concern: The PR does not show any test updates or verification that this change does not break existing functionality.

Recommendations:

1. Run the full driver test suite to verify WebSocket and HTTP connections still work correctly
2. Specifically verify tests in driver-test-suite/tests/
3. Ensure both path-based and header-based routing continue to function at the gateway level

Recommendations

1. ✅ The PR commit message is clear and follows the conventional commit format
2. ⚠️ Consider applying the same cleanup to cloudflare-workers/src/manager-driver.ts for consistency
3. ⚠️ Run integration tests to verify the gateway still correctly routes requests
4. 💡 Consider documenting the routing mechanism in actor-router-consts.ts to clarify when headers/protocols vs URL-based routing should be used
5. 💡 Optional: Add inline comments explaining why test-inline-client-driver still uses these protocols

Verdict: ✅ Approve with Minor Suggestions

This is a solid cleanup PR that removes redundant code. The implementation is correct, and the change is safe since the gateway continues to support both routing mechanisms. My main concern is ensuring the same cleanup is applied to the Cloudflare Workers driver for consistency, and verifying test coverage.

The changes align well with the principle of simplifying the API surface while maintaining backward compatibility. Great work on keeping the codebase clean!

[claude]

PR Review: Remove Unneeded Headers & Protocols

Summary

This PR removes redundant actor ID headers and WebSocket protocols from client requests. The actor ID is already passed in the URL path via /gateway/{actor_id}/..., making the additional headers and protocols unnecessary.

---

Code Quality ✅

Positive aspects:

• Clean removal of redundant code without introducing breaking changes
• Properly maintains backward compatibility by keeping both path-based and header-based routing on the server side
• Simplifies the client code by removing unnecessary parameters

Changes reviewed:

1. actor-http-client.ts: Removed HEADER_RIVET_ACTOR header (line 78 in old code)
2. actor-websocket-client.ts: Removed WS_PROTOCOL_ACTOR protocol and the actorId parameter from buildWebSocketProtocols()
3. mod.ts: Updated function call to match new signature

---

Architecture Analysis ✅

The server-side routing logic confirms this change is safe:

Path-based routing (primary, used by this client):

• /gateway/{actor_id}/{...path} - Actor ID extracted from URL path
• Server code: engine/packages/guard/src/routing/pegboard_gateway.rs:16-29 and rivetkit-typescript/packages/rivetkit/src/manager/gateway.ts:170-196

Header-based routing (fallback, still supported):

• Uses x-rivet-actor header or rivet_actor.* protocol for legacy clients
• Server code: engine/packages/guard/src/routing/pegboard_gateway.rs:31-104 and rivetkit-typescript/packages/rivetkit/src/manager/gateway.ts:213-338

Since the client uses path-based routing through /gateway/{actorId}/... URLs, the actor ID headers/protocols were indeed redundant.

---

Potential Concerns ⚠️

1. Breaking Change Risk (Low)

• The removed headers/protocols are NOT used by the server when path-based routing is active
• However, if any external systems or middleware inspect these headers/protocols, they will no longer be present
• Recommendation: Verify no monitoring, logging, or debugging tools depend on these headers

2. Test Coverage

• The changes touch code imported by test files (test-inline-client-driver.ts)
• Recommendation: Run the full test suite to ensure no regressions:

  cd rivetkit-typescript/packages/rivetkit
  pnpm test

3. Documentation

• No documentation updates included in this PR
• Question: Are there any docs that reference these headers/protocols that should be updated?

---

Security Considerations ✅

• Token handling remains unchanged and secure
• The HEADER_RIVET_TOKEN is still properly set when present
• Actor ID validation still occurs server-side via path parsing
• No security regression identified

---

Performance Considerations ✅

Benefits:

• Slightly reduced request overhead (fewer headers/protocols)
• Cleaner, more maintainable code
• No performance regressions expected

---

Recommendations

Before merging:

1. ✅ Run full test suite for rivetkit-typescript
2. ✅ Verify no breaking changes for any external consumers
3. ✅ Consider adding a note in CHANGELOG if this is a user-facing change
4. ⚠️ Check if any documentation references the removed headers/protocols

Code approval: ✅ Approved pending test verification

The changes are well-structured, maintain backward compatibility on the server side, and properly simplify the client code by removing redundant data transmission.

[claude]

Code Review

Summary

This PR removes unnecessary headers and protocols from RivetKit client requests. The changes are safe and correct - the client uses path-based routing (/gateway/{actorId}/...) which embeds the actor ID directly in the URL path, making the HEADER_RIVET_ACTOR header and WS_PROTOCOL_ACTOR protocol redundant.

Positive Findings

✅ Correct Path-Based Routing: The client correctly uses path-based routing:

• HTTP: /gateway/${actorId}${url.pathname}${url.search} (actor-http-client.ts:16)
• WebSocket: /gateway/${actorId}${path} (actor-websocket-client.ts:31)

✅ Server Compatibility: The server gateway properly handles path-based routing:

• TypeScript gateway parses actor ID from path via parseActorPath() (manager/gateway.ts:171-195)
• Rust guard routes path-based requests via route_request_path_based() (guard/src/routing/pegboard_gateway.rs:16-29)

✅ Header-Based Routing Preserved: The removed header/protocol were only used for header-based routing (manager/gateway.ts:241-265), which is a fallback mechanism not used by the remote manager driver.

✅ Clean Code: Removal reduces code complexity and eliminates redundant data transmission.

Important Note

⚠️ Rust Client Not Updated: The Rust client at rivetkit-rust/packages/client/src/remote_manager.rs still uses header-based routing and includes both:

• HEADER_RIVET_ACTOR for HTTP (line 257)
• WS_PROTOCOL_ACTOR for WebSocket (line 298)

This is fine since the server supports both routing methods, but for consistency, consider:

1. Updating the Rust client to use path-based routing like TypeScript
2. Or documenting why each client uses different routing approaches

Test Coverage

The PR doesn't include test changes. Consider:

• Running existing integration tests to verify HTTP/WebSocket communication still works
• Checking if any driver tests specifically validate the removed headers/protocols

Verdict

LGTM - The changes are correct and safe. The code is cleaner without the redundant headers/protocols. The only consideration is Rust client consistency as noted above.

---

🤖 Generated with Claude Code

[claude]

Pull Request Review

Summary

This PR removes the HEADER_RIVET_ACTOR header and WS_PROTOCOL_ACTOR protocol from client requests in the remote-manager-driver. The changes affect HTTP and WebSocket client implementations.

Analysis

Files Changed:

• actor-http-client.ts: Removed HEADER_RIVET_ACTOR header from HTTP requests
• actor-websocket-client.ts: Removed WS_PROTOCOL_ACTOR protocol from WebSocket connections
• mod.ts: Updated function signature to remove actorId parameter

Critical Issue: Breaking Change ⚠️

This PR introduces a breaking change that will cause runtime failures. The removed header and protocol are still actively used and required by multiple components:

1. Server-Side Gateway Still Expects These Values

In manager/gateway.ts:241-242:

} else if (protocol.startsWith(WS_PROTOCOL_ACTOR)) {
    actorId = protocol.substring(WS_PROTOCOL_ACTOR.length);
}

And at line 303:

const actorId = c.req.header(HEADER_RIVET_ACTOR);

The gateway expects WS_PROTOCOL_ACTOR for WebSocket connections and HEADER_RIVET_ACTOR for HTTP header-based routing.

2. Test Infrastructure Depends on It

In manager/router.ts:544-545:

if (protocol.startsWith(WS_PROTOCOL_ACTOR)) {
    actorId = protocol.substring(WS_PROTOCOL_ACTOR.length);
}

The test inline driver endpoint uses this protocol to extract the actor ID.

3. Cloudflare Workers Driver Still Sends It

In cloudflare-workers/src/manager-driver.ts:97,218:

protocols.push(\`\${WS_PROTOCOL_ACTOR}\${actorId}\`);

The Cloudflare Workers implementation still includes this protocol, creating an inconsistency across drivers.

Impact Assessment

Severity: High 🔴

1. WebSocket Routing Failure: Without WS_PROTOCOL_ACTOR, the gateway's header-based routing (handleWebSocketGateway) will fail to extract the actor ID, resulting in a "Missing actor header" error.

2. HTTP Routing Still Works: The HTTP changes are less critical because:

   • Path-based routing (/gateway/{actor_id}/...) doesn't rely on this header
   • Header-based routing will fail, but it's a fallback mechanism

3. Driver Inconsistency: The Cloudflare Workers driver and remote-manager-driver now behave differently.

Questions for Clarification

1. What's the architectural intent?

   • Are you moving entirely to path-based routing (/gateway/{actor_id}/...)?
   • Should header-based routing be removed from the gateway?

2. Why remove these identifiers?

   • The commit message says "unneeded" but they're clearly still used
   • Is there security/privacy concern about sending actor IDs?
   • Are these redundant with path-based routing?

3. Migration strategy?

   • Should the gateway code be updated simultaneously?
   • Do you need backward compatibility during transition?

Recommendations

Option A: Revert (Safe) ✅

Revert this PR until the server-side gateway is updated to not require these values.

Option B: Coordinated Update (Proper Fix)

1. Update this PR to also modify:
   • manager/gateway.ts: Remove header-based routing that depends on these values
   • manager/router.ts: Update test infrastructure
   • cloudflare-workers/src/manager-driver.ts: Make it consistent
2. Ensure all routing goes through path-based URLs (/gateway/{actor_id}/...)
3. Add tests to verify the new routing works
4. Update documentation about the routing change

Option C: Feature Flag (Gradual Migration)

If you need to support both routing methods temporarily, consider a feature flag approach.

Testing Recommendations

Before merging any version of this change:

1. Integration tests: Verify WebSocket connections work with header-based routing removed
2. Driver tests: Ensure all driver implementations (inline, remote, Cloudflare) behave consistently
3. End-to-end tests: Test actor communication through the gateway

Code Quality Notes

The code changes themselves are clean and well-structured. The issue is architectural coordination between client and server.

---

Verdict: ❌ Cannot approve in current state due to breaking changes. Please clarify the architectural intent and either revert or complete the full migration across all affected components.

[claude]

Pull Request Review

Summary

This PR removes redundant HEADER_RIVET_ACTOR header and WS_PROTOCOL_ACTOR WebSocket protocol from client requests. The actorId is already encoded in the URL path (/gateway/{actorId}/...), making these additional parameters unnecessary.

Code Quality - APPROVED

Positive aspects:

• Clean removal of redundant code
• Consistent with the path-based routing architecture
• Reduces protocol overhead slightly
• Simplifies the client API by removing unnecessary parameters

The changes are correct because:

1. The URL already contains the actorId in actor-http-client.ts:16 and actor-websocket-client.ts:31
2. The gateway uses parseActorPath() to extract actorId from the URL path (gateway.ts:171)
3. Path-based routing is the primary method, with header-based routing as a fallback (gateway.ts:198-210)

Architecture Alignment - APPROVED

The changes align well with the codebase's routing strategy:

• Path-based routing (primary): /gateway/{actor_id}/{...path}
• Header-based routing (fallback): Uses headers when path parsing fails

Since the client always constructs URLs with actorId in the path, the header/protocol are indeed redundant.

Potential Concerns

1. Server-side code still reads these values

The gateway code still attempts to parse WS_PROTOCOL_ACTOR and HEADER_RIVET_ACTOR in fallback paths:

• gateway.ts:241-242: WebSocket protocol parsing
• gateway.ts:303: HTTP header reading

Impact: Low - These are fallback mechanisms. Since path-based routing succeeds first, the fallback code is never reached.

2. Other drivers may still send these values

Looking at cloudflare-workers/src/manager-driver.ts:97,218, the Cloudflare Workers driver still sends WS_PROTOCOL_ACTOR. This inconsistency should be addressed in a follow-up:

• Either remove it from all drivers (consistency)
• Or document why different drivers use different approaches

3. Constants still defined and exported

actor-router-consts.ts:25,32 still exports these constants, and they're still in ALLOWED_PUBLIC_HEADERS (line 58). Consider:

• If truly unneeded, remove from exports (breaking change)
• If kept for backwards compatibility, add deprecation comments
• Update CORS allowed headers list if they're no longer used

Security Considerations - APPROVED

No security concerns. The change actually slightly improves security posture by:

• Reducing information leakage in headers/protocols
• Relying on URL-based routing which is more standard

Performance - APPROVED

Minor positive impact:

• Slightly smaller WebSocket handshake
• Fewer headers to process
• Negligible but positive

Test Coverage - NEEDS VERIFICATION

No test modifications included. Consider:

1. Checking existing tests still pass
2. Adding tests to verify actorId extraction from URL path works correctly
3. Testing that header-based routing fallback still works (if needed)

Recommendations

Before merging:

1. Run full test suite to ensure no regressions
2. Consider updating Cloudflare Workers driver for consistency
3. Document why constants remain in actor-router-consts.ts or remove them
4. Update ALLOWED_PUBLIC_HEADERS if these headers are truly unused

Follow-up work:

• Create issue to audit and clean up unused constants across all drivers
• Consider deprecation path if external code depends on these headers

Verdict: APPROVE with minor suggestions

The core changes are correct and safe. The redundant parameters are indeed unnecessary given the URL-based routing. The suggestions above are for consistency and completeness, but don't block merging.

Excellent cleanup work!

[claude]

Code Review

Summary

This PR removes the HEADER_RIVET_ACTOR header from HTTP requests and the WS_PROTOCOL_ACTOR protocol from WebSocket connections in the remote-manager-driver. The changes make sense because this driver uses path-based routing (/gateway/{actorId}/...) instead of header-based routing, so these headers/protocols are redundant.

✅ Positive Aspects

1. Correct optimization: The remote-manager-driver constructs URLs like /gateway/${actorId}${path} (lines 15-17 in actor-http-client.ts and line 31 in actor-websocket-client.ts), which means the actor ID is already in the path. The removed header/protocol were indeed redundant.

2. Clean code: Removing unused imports improves code clarity and reduces bundle size.

3. Path-based routing: The gateway correctly parses actor IDs from the path (gateway.ts:346-443), so it doesn't need the header/protocol for routing.

⚠️ Important Observations

Architecture Context

The codebase has two different routing mechanisms:

1. Path-based routing (what this PR's driver uses):

   • Format: /gateway/{actor_id}/{path}
   • Actor ID extracted from URL path
   • Used by: remote-manager-driver ✅

2. Header/Protocol-based routing (fallback mechanism):

   • HTTP: Uses x-rivet-actor header (gateway.ts:303)
   • WebSocket: Uses rivet_actor.{actorId} protocol (gateway.ts:241-242)
   • Used by: cloudflare-workers manager-driver (cloudflare-workers/src/manager-driver.ts:97)

The Key Insight

The remote-manager-driver was incorrectly sending BOTH the path-based actor ID AND the header/protocol-based actor ID. This PR correctly removes the redundant header/protocol since path-based routing takes precedence.

🔍 Testing Considerations

Critical: Since this changes the wire protocol between client and server, you should verify:

1. ✅ Path-based routing still works - The gateway parses actor IDs from paths (gateway.ts:171-196)
2. ✅ Token authentication preserved - HEADER_RIVET_TOKEN is still sent correctly
3. ⚠️ Backward compatibility - If any old servers or middleware relied on these redundant headers/protocols, they may break

Recommendation: Run the driver test suite to ensure WebSocket and HTTP routing still work:

cd rivetkit-typescript && pnpm test

🛡️ Security & Performance

• Security: ✅ No concerns. Token authentication is preserved via HEADER_RIVET_TOKEN
• Performance: ✅ Minor improvement by reducing header/protocol overhead
• Bundle size: ✅ Slightly smaller by removing unused imports

📝 Code Quality

Following CLAUDE.md conventions:

• ✅ Imports properly managed at file top
• ✅ Changes are minimal and focused
• ✅ No formatting issues introduced

Verdict

LGTM ✅ - This is a correct cleanup that removes redundant routing information. The remote-manager-driver uses path-based routing, so sending the actor ID via headers/protocols was unnecessary.

Before merging: Run the test suite to confirm backward compatibility:

cd rivetkit-typescript/packages/rivetkit && pnpm test

[claude]

Code Review for PR #3356

Summary

This PR removes the HEADER_RIVET_ACTOR header and WS_PROTOCOL_ACTOR WebSocket protocol from client requests in the remote-manager-driver. While the cleanup appears intentional, this is a breaking change that will cause failures in existing server-side code.

---

Critical Issues

1. Breaking Change - WebSocket Protocol Parsing ⚠️

Problem: The server-side gateway still expects WS_PROTOCOL_ACTOR in WebSocket connections.

Location: rivetkit-typescript/packages/rivetkit/src/manager/gateway.ts:241-242

for (const protocol of protocolList) {
    if (protocol.startsWith(WS_PROTOCOL_TARGET)) {
        target = protocol.substring(WS_PROTOCOL_TARGET.length);
    } else if (protocol.startsWith(WS_PROTOCOL_ACTOR)) {
        actorId = protocol.substring(WS_PROTOCOL_ACTOR.length);  // Still parsing this!
    } else if (protocol.startsWith(WS_PROTOCOL_ENCODING)) {
        encodingRaw = protocol.substring(WS_PROTOCOL_ENCODING.length);
    }
    // ...
}

After your changes, the client no longer sends WS_PROTOCOL_ACTOR, so:

• Header-based WebSocket routing will fail to extract actorId
• This will trigger the "Missing actor header" error at line 264
• WebSocket connections using header-based routing (non-path-based) will break

Also affects: rivetkit-typescript/packages/rivetkit/src/manager/router.ts:544-545 (test inline driver)

2. Breaking Change - HTTP Header ⚠️

Problem: While HTTP header-based routing uses HEADER_RIVET_ACTOR (which differs from the removed HEADER_RIVET_ACTOR constant used in the client), the naming suggests these should be the same.

Location: The client was sending this header in buildGuardHeadersForHttp() at actor-http-client.ts:78

Impact: If any server-side code expects this header, HTTP routing may also fail.

---

Questions

1. Is this intentional? Are you deprecating header-based routing in favor of path-based routing only?
2. Migration path: Should existing deployments be updated to use path-based routing (/gateway/{actor_id}/...) instead?
3. Backwards compatibility: How will this affect existing clients using header-based or protocol-based routing?

---

Recommendations

If the removal is intentional:

1. Update server-side code to remove parsing of WS_PROTOCOL_ACTOR from:

   • rivetkit-typescript/packages/rivetkit/src/manager/gateway.ts:241-242
   • rivetkit-typescript/packages/rivetkit/src/manager/router.ts:544-545

2. Add migration documentation explaining:

   • That header/protocol-based routing is deprecated
   • How to migrate to path-based routing
   • Version compatibility matrix

3. Update constants file to remove or deprecate:

   • WS_PROTOCOL_ACTOR (line 32 in actor-router-consts.ts)
   • HEADER_RIVET_ACTOR (line 25, if truly unused)

4. Add tests to ensure:

   • Path-based routing works correctly
   • Appropriate error messages when using deprecated routing methods
   • No regressions in the migration

If the removal is unintentional:

1. Revert this PR and investigate why these headers/protocols were being sent in the first place
2. Ensure test coverage exists for both routing methods

---

Code Quality Notes

The actual code changes are clean and well-structured:

• Import cleanup is correct
• Function signature updates are consistent
• No formatting or style issues

The concern is purely about the functional impact of removing these values without corresponding server-side changes.

---

Security Considerations

No direct security concerns with this change, though removing authentication/routing headers without a clear migration path could potentially:

• Break existing authentication flows
• Expose actors if fallback routing logic is insecure

---

Performance Considerations

Positive: Removing unused protocol strings will slightly reduce WebSocket handshake size. No other performance impacts expected.

---

Test Coverage

Missing: This PR lacks test coverage demonstrating that:

1. The removed headers/protocols weren't actually needed
2. WebSocket and HTTP routing still work correctly after removal
3. No regression in the driver test suite

Recommendation: Run the full driver test suite and add specific tests for both path-based and header-based routing scenarios.

---

Verdict: ⚠️ Request Changes - This appears to be a breaking change that requires coordinated server-side updates or clarification on the migration strategy.

[claude]

Pull Request Review

I've reviewed the changes in this PR that remove the HEADER_RIVET_ACTOR and WS_PROTOCOL_ACTOR from client requests. Here's my analysis:

Overview

The PR removes actor ID information from client requests to the remote manager driver by:

1. Removing HEADER_RIVET_ACTOR header from HTTP requests
2. Removing WS_PROTOCOL_ACTOR protocol from WebSocket connections
3. Cleaning up related imports

Code Quality ✅

The code changes are clean and well-structured:

• Proper removal of unused imports
• Consistent changes across both HTTP and WebSocket clients
• Function signatures updated correctly

Potential Issues ⚠️

1. Critical: Cloudflare Workers Driver Still Uses These Protocols

The Cloudflare Workers driver (rivetkit-typescript/packages/cloudflare-workers/src/manager-driver.ts) still sends these protocols:

• Line 97: protocols.push(\${WS_PROTOCOL_ACTOR}${actorId}`)`
• Line 218: protocols.push(\${WS_PROTOCOL_ACTOR}${actorId}`)`

While the server-side gateway reads this protocol (gateway.ts:241-242, router.ts:544-545), the remote-manager-driver client in this PR no longer sends it. This creates an inconsistency between different driver implementations.

Impact: This may cause issues if:

• There are systems that rely on protocol consistency between drivers
• The test suite validates protocol matching
• Future code assumes all drivers send the same protocols

2. Server-Side Still Expects These Values (Header-Based Routing)

Looking at manager/gateway.ts:

• Lines 241-242: Parse WS_PROTOCOL_ACTOR from protocols (for header-based WebSocket routing)
• Line 303: Read HEADER_RIVET_ACTOR header (for header-based HTTP routing)

The server supports two routing modes:

1. Path-based routing: /gateway/{actor_id}/{...path} - Actor ID in URL (doesn't need these headers/protocols)
2. Header-based routing: Actor ID in headers/protocols (fallback mode)

Question: Does the remote-manager-driver always use path-based routing? If not, this change would break header-based routing.

3. Test Coverage Concern

The test inline driver (driver-test-suite/test-inline-client-driver.ts:210) still sends WS_PROTOCOL_ACTOR. This suggests:

• Either tests need updating
• Or this protocol is still required for testing scenarios

Security Considerations ✅

No security concerns identified. The actor ID is not sensitive information and is already present in the URL path for path-based routing.

Performance Considerations ✅

Minor improvement - slightly reduced header/protocol overhead.

Recommendations

1. Verify Routing Strategy: Confirm that remote-manager-driver always uses path-based routing and never relies on header-based routing. If it does use header-based routing, this change would be breaking.

2. Update Cloudflare Workers Driver: For consistency, consider updating the Cloudflare Workers driver to match this approach (if it also uses path-based routing).

3. Update Tests: Update driver-test-suite/test-inline-client-driver.ts:210 to remove WS_PROTOCOL_ACTOR if it's truly not needed.

4. Documentation: Consider adding a comment explaining why these headers/protocols are removed (e.g., "Actor ID is passed via path-based routing, so headers/protocols are redundant").

5. Backward Compatibility: Consider if any existing deployments rely on the header-based routing fallback. If so, this would be a breaking change.

Test Coverage

Please ensure tests cover:

• ✅ Path-based HTTP routing still works
• ✅ Path-based WebSocket routing still works
• ⚠️ Header-based routing fallback (if still supported)
• ⚠️ Integration with Cloudflare Workers driver

Verdict

Conditional Approval - The code quality is good, but please verify:

1. Remote-manager-driver exclusively uses path-based routing
2. This doesn't break any existing deployments using header-based routing
3. Consider updating other drivers and tests for consistency

---

Generated by Claude Code

[graphite-app]

Merge activity

• Nov 5, 5:07 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 5, 5:08 AM UTC: CI is running for this pull request on a draft pull request (#3371) due to your merge queue CI optimization settings.
• Nov 5, 5:11 AM UTC: Merged by the Graphite merge queue via draft PR: #3371.