fix(core): strip basePath from routes for actor gateway on manager router

[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

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat: 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 👈 (View in Graphite)
• chore(core): enable overriding default server addr for logs & inspector #1345
• 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.

[claude]

Claude finished @NathanFlurry's task —— View job

---

Code Review Analysis

Reviewing pull request: fix(core): strip basePath from routes for actor gateway on manager router

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze the modified file and understand the changes
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage implications
• Provide comprehensive feedback

---

Review Summary

This PR implements base path stripping in the actor gateway to ensure proper routing when the manager router is configured with a basePath. The changes are well-structured and address a legitimate routing issue.

Code Quality: ✅ Good

Strengths:

• Clean, readable implementation with clear intent
• Proper function signature updates to pass strippedPath parameter
• Consistent usage of the stripped path throughout both WebSocket and HTTP handlers
• Good logging that now uses the stripped path for better debugging

Adherence to repo guidelines:

• Follows the repository's logging conventions (logger().debug(...))
• Uses proper camelCase naming
• Maintains existing error handling patterns

Implementation Analysis: ✅ Solid

What the change does:

• Strips the configured basePath from incoming request paths before routing to actors
• Ensures the stripped path always starts with / for consistency
• Updates both WebSocket and HTTP gateway handlers to use the processed path
• Maintains query parameters correctly in both handlers

Logic flow:

1. Extract basePath from runConfig
2. If present and path starts with it, strip it via slice()
3. Normalize path to ensure it starts with /
4. Pass stripped path to both handler functions

Potential Issues: ⚠️ Minor Concerns

Edge case handling:
The current implementation handles most cases well, but consider these scenarios:

1. Empty path after stripping: If basePath equals the full path, strippedPath becomes empty and gets normalized to / - this is correct behavior.

2. Base path without leading slash: The code assumes basePath is well-formed. Consider defensive programming:

   const normalizedBasePath = runConfig.basePath?.startsWith('/') 
     ? runConfig.basePath 
     : `/${runConfig.basePath}`;

URL construction in HTTP handler:
Line 176 constructs the proxy URL using strippedPath directly:

const proxyUrl = new URL(`http://actor${strippedPath}${url.search}`);

This looks correct - the actor service expects paths without the base prefix.

Performance: ✅ Minimal Impact

• String operations (startsWith, slice) are O(n) but path lengths are typically small
• No unnecessary allocations or repeated processing
• The stripping logic runs once per request at the gateway entry point

Security: ✅ No Concerns

• No injection vulnerabilities - using safe string operations
• No exposure of internal paths
• Maintains existing header filtering and validation

Test Coverage: ⚠️ Gap

Missing test coverage:

• No existing tests for the gateway functionality found in the codebase
• Base path stripping logic should be tested with scenarios like:
  • Path equals base path exactly
  • Path with base path prefix
  • Path without base path prefix
  • Empty/undefined base path
  • WebSocket vs HTTP request handling

Recommendation: Consider adding integration tests for the gateway functionality, particularly around base path handling.

Minor Suggestions

1. Input validation (line 42-48): Consider adding a null/undefined check:

   if (runConfig.basePath && runConfig.basePath !== "/" && strippedPath.startsWith(runConfig.basePath)) {

2. Documentation: The JSDoc comment could mention the base path stripping behavior.

Overall Assessment: ✅ Approve

This is a solid fix that addresses a real routing issue. The implementation is clean, follows repository conventions, and handles the core use cases correctly. The main gap is test coverage, but the code change itself is low-risk and well-implemented.

Recommendation: Merge after considering the minor suggestions above.

---

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