Skip to content

Conversation

@slyt3
Copy link
Contributor

@slyt3 slyt3 commented Dec 7, 2025

Problem

So deploying an MCPServer with transport: stdio and proxyMode: streamable-http, the status URL shows incorectly returns /sse endpoint instead of /mcp

Example

apiVersion: toolhive.stacklok.dev/v1alpha1
kind: MCPServer
metadata:
  name: arxiv
  namespace: toolhive-system
spec:
  image: ghcr.io/stacklok/dockyard/uvx/arxiv-mcp-server:0.3.1
  transport: stdio
  proxyMode: streamable-http
  proxyPort: 8080

Expected status URL:

http://arxiv.toolhive-system.svc.cluster.local:8080/mcp

Actual status URL:

http://arxiv.toolhive-system.svc.cluster.local:8080/sse#arxiv

Root Cause

The bug is in pkg/transport/url.go

This code is old and it shows that its treating stdio same as sse and ignoring proxyMode

isSSE := transportType == types.TransportTypeSSE.String() || 
         transportType == types.TransportTypeStdio.String()

So thinking logically we should assume that stdio always use SSE endpoint but stdio can use it either

  • proxyMode: sse/sse endpoint
  • proxyMode: streamable-http/mcp endpoint (default)

And the controller at cmd/thv-operator/controllers/mcpserver_controller.go:395 only passes Transport, never ProxyMode:

OLD CODE TOO missing proxyMode parameter

mcpServer.Status.URL = transport.GenerateMCPServerURL(
    mcpServer.Spec.Transport,
    host,
    int(mcpServer.GetProxyPort()),
    mcpServer.Name,
    "",
)

Solution

So I Added proxyMode parameter to GenerateMCPServerURL() function:

func GenerateMCPServerURL(transportType string, proxyMode string, host string, 
    port int, containerName, remoteURL string) string

Logic:

  1. For stdio transport: Use proxyMode to easily determine endpoint

    • proxyMode: "streamable-http"/mcp
    • proxyMode: "sse"/sse#name
    • proxyMode: "" (empty) → defaults to /mcp (matches CRD default)
  2. For other transports: JUST Ignore proxyMode its behaviour doesnt change xD

    • sse transport → always /sse#name
    • streamable-http transport → always /mcp

Type Conversions

Added string() conversions where ProxyMode is a typed string (was stuck on this one for whole night, was going crazy i just needed to convert ProxyMode string to function call, so proxyMode is defined as type ProxyMode string:

string(r.Config.ProxyMode)  

Testing

Unit Tests (15/15 passing)

$ go test ./pkg/transport/... -v -run TestGenerateMCPServerURL

Key test cases covering the bug:

Test Transport ProxyMode Expected Result
Bug case stdio streamable-http /mcp ✅ PASS
Edge case stdio sse /sse#name ✅ PASS
Default stdio "" (empty) /mcp ✅ PASS
Unchanged sse (ignored) /sse#name ✅ PASS
Unchanged streamable-http (ignored) /mcp ✅ PASS

All 15 tests pass including targetURI path handling.


Changes

7 files changed:

  • pkg/transport/url.go - Added proxyMode parameter and logic
  • pkg/transport/url_test.go - Added comprehensive test coverage
  • cmd/thv-operator/controllers/mcpserver_controller.go - Updated call site
  • pkg/runner/runner.go - Updated call site with type conversion
  • pkg/workloads/manager.go - Updated call site with type conversion
  • pkg/workloads/types/types.go - Updated call site with empty string default
  • pkg/vmcp/workloads/k8s.go - Updated call site with type conversion

Build status:

$ go build ./...

Impact

This fix ensures stdio transport respects the proxyMode setting:

Config Before (Bug) After (Fixed)
stdio + streamable-http /sse /mcp
stdio + sse /sse /sse
stdio + (empty) /sse /mcp
sse transport /sse /sse
streamable-http transport /mcp /mcp

Soooo since CRD is default for proxyMode is streamable-http` bug affects the default stdio configuration


Fixes #2920

@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Dec 8, 2025
@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.37%. Comparing base (28797fa) to head (8f11963).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...d/thv-operator/controllers/mcpserver_controller.go 0.00% 1 Missing ⚠️
pkg/runner/runner.go 0.00% 1 Missing ⚠️
pkg/vmcp/workloads/k8s.go 0.00% 1 Missing ⚠️
pkg/workloads/manager.go 0.00% 1 Missing ⚠️
pkg/workloads/types/types.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2935   +/-   ##
=======================================
  Coverage   56.36%   56.37%           
=======================================
  Files         323      323           
  Lines       31763    31777   +14     
=======================================
+ Hits        17904    17915   +11     
- Misses      12330    12333    +3     
  Partials     1529     1529           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCPServer status URL shows /sse instead of /mcp when using stdio transport with streamable-http proxyMode

1 participant