fix legacy mcp config

This commit is contained in:
Ed Zynda
2025-06-25 13:15:09 +03:00
parent e5b6e7e123
commit 0972a1600a
5 changed files with 226 additions and 5 deletions
+19 -1
View File
@@ -44,6 +44,7 @@ MCPHost supports a simplified configuration schema with three server types:
- Maintains full backward compatibility with existing configurations
- Automatic detection and conversion of legacy formats
- Custom `UnmarshalJSON` method handles format migration seamlessly
- **Bug Fix**: Improved stdio transport reliability for legacy configurations with external processes (Docker, NPX, etc.)
### Transport Mapping
- `"local"` type → `stdio` transport (launches local processes)
@@ -108,4 +109,21 @@ MCPHost includes a powerful script system for automation and reusable workflows.
### Testing
- **Test Coverage**: 25+ test cases covering all variable scenarios
- **Edge Cases**: Empty defaults, complex values, mixed required/optional variables
- **Backward Compatibility**: Ensures existing scripts continue working unchanged
- **Backward Compatibility**: Ensures existing scripts continue working unchanged
## Recent Bug Fixes
### Legacy MCP Server Configuration Fix
- **Issue**: Legacy stdio transport configurations (using `command` + `args` format) were failing with timeout errors
- **Root Cause**: MCP client creation was not properly handling legacy argument parsing when `Command` array was incomplete
- **Fix**: Added fallback logic in `internal/tools/mcp.go` to use legacy `Args` field when `Command` array only contains the command
- **Impact**: Fixes Docker-based MCP servers, NPX-based servers, and other external process configurations
- **Files Modified**:
- `internal/tools/mcp.go`: Added legacy args fallback logic
- `internal/config/config.go`: Enhanced headers support for remote servers
- **Testing**: Verified with both Docker (`ghcr.io/mark3labs/phalcon-mcp:latest`) and NPX (`@modelcontextprotocol/server-filesystem`) servers
### MCP Client Initialization Improvements
- **Timeout**: Increased initialization timeout from 10s to 30s for slower external processes
- **Capabilities**: Added explicit `ClientCapabilities{}` to initialization request for better compatibility
- **Headers**: Enhanced SSE transport to support custom headers for authentication
+22 -2
View File
@@ -138,7 +138,8 @@ For remote MCP servers accessible via HTTP:
"mcpServers": {
"websearch": {
"type": "remote",
"url": "https://api.example.com/mcp"
"url": "https://api.example.com/mcp",
"headers": ["Authorization: Bearer your-api-token"]
},
"weather": {
"type": "remote",
@@ -151,6 +152,7 @@ For remote MCP servers accessible via HTTP:
Each remote server entry requires:
- `type`: Must be set to `"remote"`
- `url`: The URL where the MCP server is accessible
- `headers`: (Optional) Array of HTTP headers for authentication and custom headers
Remote servers automatically use the StreamableHTTP transport for optimal performance.
@@ -246,7 +248,7 @@ All MCP server types support tool filtering to restrict which tools are availabl
### Legacy Configuration Support
MCPHost maintains full backward compatibility with the previous configuration format:
MCPHost maintains full backward compatibility with the previous configuration format. **Note**: A recent bug fix improved legacy stdio transport reliability for external MCP servers (Docker, NPX, etc.).
#### Legacy STDIO Format
```json
@@ -275,6 +277,24 @@ MCPHost maintains full backward compatibility with the previous configuration fo
}
```
#### Legacy Docker/Container Format
```json
{
"mcpServers": {
"phalcon": {
"command": "docker",
"args": [
"run",
"-i",
"--rm",
"ghcr.io/mark3labs/phalcon-mcp:latest",
"serve"
]
}
}
}
```
#### Legacy Streamable HTTP Format
```json
{
+3
View File
@@ -34,6 +34,7 @@ func (s *MCPServerConfig) UnmarshalJSON(data []byte) error {
Command []string `json:"command,omitempty"`
Environment map[string]string `json:"environment,omitempty"`
URL string `json:"url,omitempty"`
Headers []string `json:"headers,omitempty"`
Name string `json:"name,omitempty"`
Options map[string]any `json:"options,omitempty"`
AllowedTools []string `json:"allowedTools,omitempty"`
@@ -59,6 +60,7 @@ func (s *MCPServerConfig) UnmarshalJSON(data []byte) error {
s.Command = newConfig.Command
s.Environment = newConfig.Environment
s.URL = newConfig.URL
s.Headers = newConfig.Headers
s.Name = newConfig.Name
s.Options = newConfig.Options
s.AllowedTools = newConfig.AllowedTools
@@ -283,6 +285,7 @@ func createDefaultConfig(homeDir string) error {
# name: "fetch"
#
# # Remote MCP servers - connect via StreamableHTTP transport
# # Optional 'headers' field can be used for authentication and custom headers
# websearch:
# type: "remote"
# url: "https://api.example.com/mcp"
+155
View File
@@ -401,6 +401,161 @@ func TestIssue76_ExactReproduction(t *testing.T) {
}
}
func TestMCPServerConfig_RemoteFormatWithHeaders(t *testing.T) {
tests := []struct {
name string
jsonData string
expectedType string
expectedURL string
expectedHeaders []string
expectedTransport string
}{
{
name: "Remote server with headers",
jsonData: `{
"type": "remote",
"url": "https://api.example.com/mcp",
"headers": ["Authorization: Bearer token123", "X-API-Key: key456"]
}`,
expectedType: "remote",
expectedURL: "https://api.example.com/mcp",
expectedHeaders: []string{"Authorization: Bearer token123", "X-API-Key: key456"},
expectedTransport: "streamable",
},
{
name: "Remote server without headers",
jsonData: `{
"type": "remote",
"url": "https://api.example.com/mcp"
}`,
expectedType: "remote",
expectedURL: "https://api.example.com/mcp",
expectedHeaders: nil,
expectedTransport: "streamable",
},
{
name: "Legacy remote server with headers",
jsonData: `{
"url": "https://legacy.example.com/mcp",
"headers": ["Content-Type: application/json", "User-Agent: MCPHost/1.0"]
}`,
expectedType: "",
expectedURL: "https://legacy.example.com/mcp",
expectedHeaders: []string{"Content-Type: application/json", "User-Agent: MCPHost/1.0"},
expectedTransport: "sse",
},
{
name: "Legacy remote server with explicit transport and headers",
jsonData: `{
"url": "https://legacy.example.com/mcp",
"transport": "sse",
"headers": ["Authorization: Basic dXNlcjpwYXNz"]
}`,
expectedType: "",
expectedURL: "https://legacy.example.com/mcp",
expectedHeaders: []string{"Authorization: Basic dXNlcjpwYXNz"},
expectedTransport: "sse",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var config MCPServerConfig
err := json.Unmarshal([]byte(tt.jsonData), &config)
if err != nil {
t.Fatalf("Failed to unmarshal: %v", err)
}
if config.Type != tt.expectedType {
t.Errorf("Expected type '%s', got '%s'", tt.expectedType, config.Type)
}
if config.URL != tt.expectedURL {
t.Errorf("Expected URL '%s', got '%s'", tt.expectedURL, config.URL)
}
if len(config.Headers) != len(tt.expectedHeaders) {
t.Errorf("Expected %d headers, got %d", len(tt.expectedHeaders), len(config.Headers))
}
for i, expectedHeader := range tt.expectedHeaders {
if i >= len(config.Headers) || config.Headers[i] != expectedHeader {
t.Errorf("Header %d: expected '%s', got '%s'", i, expectedHeader, config.Headers[i])
}
}
transportType := config.GetTransportType()
if transportType != tt.expectedTransport {
t.Errorf("Expected transport type '%s', got '%s'", tt.expectedTransport, transportType)
}
})
}
}
func TestMCPServerConfig_HeadersParsing(t *testing.T) {
// Test that headers are properly parsed in both new and legacy formats
tests := []struct {
name string
jsonData string
expected []string
}{
{
name: "New format with multiple headers",
jsonData: `{
"type": "remote",
"url": "https://api.example.com",
"headers": [
"Authorization: Bearer abc123",
"Content-Type: application/json",
"X-Custom-Header: custom-value"
]
}`,
expected: []string{
"Authorization: Bearer abc123",
"Content-Type: application/json",
"X-Custom-Header: custom-value",
},
},
{
name: "Legacy format with headers",
jsonData: `{
"url": "https://legacy.example.com",
"headers": ["API-Key: secret123", "Accept: application/json"]
}`,
expected: []string{"API-Key: secret123", "Accept: application/json"},
},
{
name: "Empty headers array",
jsonData: `{
"type": "remote",
"url": "https://api.example.com",
"headers": []
}`,
expected: []string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var config MCPServerConfig
err := json.Unmarshal([]byte(tt.jsonData), &config)
if err != nil {
t.Fatalf("Failed to unmarshal: %v", err)
}
if len(config.Headers) != len(tt.expected) {
t.Errorf("Expected %d headers, got %d", len(tt.expected), len(config.Headers))
}
for i, expected := range tt.expected {
if i >= len(config.Headers) || config.Headers[i] != expected {
t.Errorf("Header %d: expected '%s', got '%s'", i, expected, config.Headers[i])
}
}
})
}
}
func TestEnsureConfigExistsWhenFileExists(t *testing.T) {
// Create a temporary directory for testing
tempDir, err := os.MkdirTemp("", "mcphost_config_test")
+27 -2
View File
@@ -228,6 +228,10 @@ func (m *MCPToolManager) createMCPClient(ctx context.Context, serverName string,
command = serverConfig.Command[0]
if len(serverConfig.Command) > 1 {
args = serverConfig.Command[1:]
} else if len(serverConfig.Args) > 0 {
// Legacy fallback: Command only has the command, Args has the arguments
// This handles cases where legacy config conversion didn't work properly
args = serverConfig.Args
}
}
@@ -245,6 +249,8 @@ func (m *MCPToolManager) createMCPClient(ctx context.Context, serverName string,
}
}
stdioClient, err := client.NewStdioMCPClient(command, env, args...)
if err != nil {
return nil, fmt.Errorf("failed to create stdio client: %v", err)
@@ -260,7 +266,25 @@ func (m *MCPToolManager) createMCPClient(ctx context.Context, serverName string,
case "sse":
// SSE client
sseClient, err := client.NewSSEMCPClient(serverConfig.URL)
var options []transport.ClientOption
// Add headers if specified
if len(serverConfig.Headers) > 0 {
headers := make(map[string]string)
for _, header := range serverConfig.Headers {
parts := strings.SplitN(header, ":", 2)
if len(parts) == 2 {
key := strings.TrimSpace(parts[0])
value := strings.TrimSpace(parts[1])
headers[key] = value
}
}
if len(headers) > 0 {
options = append(options, transport.WithHeaders(headers))
}
}
sseClient, err := client.NewSSEMCPClient(serverConfig.URL, options...)
if err != nil {
return nil, err
}
@@ -315,7 +339,7 @@ func (m *MCPToolManager) createMCPClient(ctx context.Context, serverName string,
func (m *MCPToolManager) initializeClient(ctx context.Context, client client.MCPClient) error {
// Create a timeout context for initialization to prevent deadlocks
initCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
initCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()
initRequest := mcp.InitializeRequest{}
@@ -324,6 +348,7 @@ func (m *MCPToolManager) initializeClient(ctx context.Context, client client.MCP
Name: "mcphost",
Version: "1.0.0",
}
initRequest.Params.Capabilities = mcp.ClientCapabilities{}
_, err := client.Initialize(initCtx, initRequest)
if err != nil {