-
Notifications
You must be signed in to change notification settings - Fork 156
Description
I had to add the following code in the optimizer
if proxy_mode == ToolHiveProxyMode.STREAMABLE:
# Strip fragments for streamable-http
# (fragments not supported by streamable-http client)
parsed = urlparse(url)
# Fix path: streamable-http uses /mcp endpoint, not /sse
path = parsed.path
if path.endswith("/sse"):
path = path.replace("/sse", "/mcp")
elif not path.endswith("/mcp"):
# Only add /mcp if the path doesn't already contain /mcp
# This prevents double-adding /mcp to URLs like /mcp/test-server
if "/mcp" not in path:
# If path doesn't end with /mcp or /sse, and doesn't contain /mcp,
# ensure it ends with /mcp
if path.endswith("/"):
path = path + "mcp"
else:
path = path + "/mcp"
# Reconstruct URL without fragment and with corrected path
normalized_tuple = (
parsed.scheme,
parsed.netloc,
path,
parsed.params,
parsed.query,
"", # Empty fragment
)
normalized = str(urlunparse(normalized_tuple))
if normalized != url:
logger.debug(
"Normalized URL for streamable-http",
original_url=url,
normalized_url=normalized,
workload=self.workload.name,
)
return normalized
Why This Code Was Added
The normalization addresses inconsistencies in URLs returned by ToolHive:
-
Fragment stripping: ToolHive’s
GenerateMCPServerURLshould not add fragments for streamable-http (line 42), but edge cases (legacy configs, bugs, remote URLs) can still include them. The streamable-http client library doesn’t support fragments. -
Path normalization:
- Converting
/sse→/mcpfor streamable-http - Adding
/mcpif missing (especially for remote URLs where ToolHive uses the remote path as-is) - Preventing double-adding
/mcp(e.g.,/mcp/test-server)
- Converting
-
SSE vs streamable-http: SSE needs fragments for container identification; streamable-http does not.
These are misconfigurations in how remote MCP servers are configured. Here's the breakdown:
The Problem
Looking at ToolHive's GenerateMCPServerURL (lines 41-42):
if isStreamable {
return fmt.Sprintf("%s%s", base, path) // Uses remote path as-is
}When a remote streamable-http MCP server is configured with:
https://api.example.com/custom/endpoint→ ToolHive generateshttp://localhost:8080/custom/endpointhttps://api.example.com/sse→ ToolHive generateshttp://localhost:8080/sse
These are misconfigurations because:
- The remote server URL should match the transport type
- For streamable-http, it should end with
/mcp - For SSE, it should end with
/sse
ToolHive Should Still Normalize
Even with misconfigurations, ToolHive should normalize proxy URLs because:
- ToolHive controls the proxy endpoint: The proxy should expose a consistent endpoint (
/mcpfor streamable-http) regardless of the remote server's path. - The remote path is an implementation detail: ToolHive should route internally to the remote server's actual path, but expose a standard proxy endpoint.
- User error tolerance: Users may misconfigure remote URLs; ToolHive should handle it gracefully.
The Real Issue
The bug is in ToolHive's GenerateMCPServerURL function. It should:
- For streamable-http: Always generate
/mcpendpoint (normalize the proxy URL) - Route internally to the remote server's actual path
- Not blindly copy the remote path to the proxy URL
Recommendation
- Fix ToolHive: Update
GenerateMCPServerURLto normalize proxy URLs for streamable-http (always use/mcp). - Keep mcp-optimizer normalization: As a defensive measure for:
- Legacy URLs from older ToolHive versions
- Edge cases ToolHive might miss
- Kubernetes mode where URLs come from CRDs
The normalization in mcp-optimizer is a workaround for ToolHive's bug, but it's still valuable as defense-in-depth.