fix(server): opt in to root MCP routing for Tailscale Funnel - #147
fix(server): opt in to root MCP routing for Tailscale Funnel#147dbc-hbin wants to merge 2 commits into
Conversation
…host Tailscale Funnel strips the configured path prefix before proxying to the backend, so a server exposed at /mcp receives requests at /. Register the MCP handler at the root path as well when publicBaseUrl is a *.ts.net host so ChatGPT can reach DevSpace through Funnel. Non-Tailscale setups keep the /mcp-only route unchanged.
📝 WalkthroughWalkthroughThe server reads an explicit Funnel flag and registers MCP requests at ChangesTailscale Funnel MCP routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TailscaleFunnel
participant ServerRouter
participant handleMcpRequest
TailscaleFunnel->>ServerRouter: Forward authenticated MCP request with stripped path
ServerRouter->>handleMcpRequest: Route / to MCP handler when mcpRootAlias is enabled
handleMcpRequest-->>ServerRouter: Return MCP response or error
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR detects Tailscale-hosted public URLs and exposes the existing MCP request handler at the backend root to accommodate Funnel path-prefix stripping.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified. The conditional root route reuses the existing authenticated MCP handler only for the intended Tailscale-host configuration, while repository callers consistently obtain the new flag through
|
| Filename | Overview |
|---|---|
| src/config.ts | Adds a derived boolean identifying *.ts.net public hostnames for conditional Funnel routing. |
| src/server.ts | Extracts the existing MCP callback and additionally mounts it at / for detected Tailscale hosts without changing its protocol or authorization logic. |
| src/config.test.ts | Covers the new configuration value for default, Tailscale, and ordinary public URLs. |
Sequence Diagram
sequenceDiagram
participant Client as MCP Client
participant Funnel as Tailscale Funnel
participant Server as DevSpace Server
Client->>Funnel: Request /mcp
Funnel->>Server: Forward request /
Server->>Server: Authenticate and handle MCP request
Server-->>Funnel: MCP response
Funnel-->>Client: MCP response
Reviews (1): Last reviewed commit: "fix(server): serve MCP at root when publ..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/server.ts (1)
1874-1878: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an integration regression test for the proxy-stripped MCP path.
isTailscaleFunnelis only covered throughloadConfig. Exercise the realcreateServer(...)app for Funnel and non-Funnel configs: send an authenticated MCP initialize request to/with Funnel enabled, and assert that root remains unavailable without Funnel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server.ts` around lines 1874 - 1878, Add an integration regression test around createServer that sends an authenticated MCP initialize request to / with isTailscaleFunnel enabled and verifies it succeeds, then runs the same request with Funnel disabled and verifies root remains unavailable while /mcp behavior is unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config.ts`:
- Around line 21-28: Remove the Tailscale-specific isTailscaleFunnel field and
ChatGPT routing documentation from ServerConfig. Move the hostname/path-prefix
translation into the Tailscale adapter, then pass createServer a generic MCP
route capability or explicit integration mode while keeping DevSpace policy in
the core configuration.
- Line 241: Update the isTailscaleFunnel configuration in the publicBaseUrl
setup so it is not inferred from a .ts.net hostname; instead, derive it from an
explicit Tailscale Funnel mode/capability setting or other Funnel-only
configuration, while preserving false for Tailscale Serve deployments.
---
Nitpick comments:
In `@src/server.ts`:
- Around line 1874-1878: Add an integration regression test around createServer
that sends an authenticated MCP initialize request to / with isTailscaleFunnel
enabled and verifies it succeeds, then runs the same request with Funnel
disabled and verifies root remains unavailable while /mcp behavior is unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cf294c1c-4c4c-46ed-87bc-fd11f223adad
📒 Files selected for processing (3)
src/config.test.tssrc/config.tssrc/server.ts
dbc-hbin
changed the title
|
Addressed the review feedback in 28e7544.
Validated with typecheck, focused config/server tests, and a production build. |
1 participant
Problem
ChatGPT cannot connect to a DevSpace server exposed through a path-based Tailscale Funnel route.
Tailscale Funnel strips the configured path prefix before proxying to the backend. The MCP client requests the documented public
/mcpendpoint, but a Funnel route targeting that prefix can forward the request to DevSpace as/. DevSpace only registers the MCP handler at/mcp, so the forwarded request returns 404.This was verified empirically with a path-echo backend: a request to a configured Funnel prefix arrived with that prefix removed.
Change
DEVSPACE_TAILSCALE_FUNNEL=1opt-in for path-stripping Funnel routes.mcpRootAliascapability./mcpand/only when the capability is enabled..ts.nethostname, because Tailscale Serve uses the same hostname form.The default remains
/mcponly. Tailscale Serve and ordinary reverse proxies do not expose the root alias unless explicitly configured.Verification
npm run typechecknpx tsx src/config.test.tsnpx tsx src/server.test.ts— 7/7 pass, including authenticated MCP initialize at/and/mcpnpm run buildThe route regression test verifies:
/returns 200;/remains 404;/mcpendpoint still returns 200.Development attribution
Built with the Codex desktop coding harness using
deepseek/deepseek-v4-flash. The submitted changes were inspected and validated against the local repository checkout with the commands listed above.Summary by CodeRabbit
New Features
/mcpregardless of the setting.Tests