From e5b6e7e1237f30f75652d4dcc006bb1d9b7ad437 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Wed, 25 Jun 2025 10:58:14 +0300 Subject: [PATCH] convert to newer config format automatically --- cmd/script.go | 8 +- cmd/script_test.go | 2 +- internal/config/config.go | 17 +++- internal/config/config_test.go | 138 +++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 9 deletions(-) diff --git a/cmd/script.go b/cmd/script.go index ac3766cb..c8e3e445 100644 --- a/cmd/script.go +++ b/cmd/script.go @@ -389,19 +389,19 @@ func findVariablesWithDefaults(content string) []Variable { varName := match[1] if !seenVars[varName] { seenVars[varName] = true - + // Check if the original match contains the :- pattern hasDefault := strings.Contains(match[0], ":-") - + variable := Variable{ Name: varName, HasDefault: hasDefault, } - + if hasDefault && len(match) >= 3 { variable.DefaultValue = match[2] // Can be empty string } - + variables = append(variables, variable) } } diff --git a/cmd/script_test.go b/cmd/script_test.go index d1016335..5656f1c6 100644 --- a/cmd/script_test.go +++ b/cmd/script_test.go @@ -269,4 +269,4 @@ Hello John! Please analyze /tmp.` if result != expected { t.Errorf("substituteVariables() backward compatibility failed.\nGot:\n%s\nWant:\n%s", result, expected) } -} \ No newline at end of file +} diff --git a/internal/config/config.go b/internal/config/config.go index 4abdb275..e9e3897e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -84,6 +84,14 @@ func (s *MCPServerConfig) UnmarshalJSON(data []byte) error { s.AllowedTools = legacyConfig.AllowedTools s.ExcludedTools = legacyConfig.ExcludedTools + // Infer type from legacy format for better compatibility + // Only set Type when it doesn't change existing transport behavior + if legacyConfig.Command != "" { + s.Type = "local" // This maps to "stdio" which matches legacy behavior + } + // Don't set Type for URL-only configs to preserve legacy "sse" behavior + // The URL will be handled by the legacy fallback logic in GetTransportType() + return nil } @@ -109,6 +117,11 @@ type Config struct { // GetTransportType returns the transport type for the server config func (s *MCPServerConfig) GetTransportType() string { + // Legacy format support - check explicit transport first + if s.Transport != "" { + return s.Transport + } + // New simplified format if s.Type != "" { switch s.Type { @@ -123,10 +136,6 @@ func (s *MCPServerConfig) GetTransportType() string { } } - // Legacy format support - if s.Transport != "" { - return s.Transport - } // Backward compatibility: infer transport type if len(s.Command) > 0 { return "stdio" diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9b9173e0..ecc6615c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -91,6 +91,11 @@ func TestMCPServerConfig_LegacyFormat(t *testing.T) { t.Fatalf("Failed to unmarshal legacy format: %v", err) } + // Verify Type field is now set correctly for legacy format + if config.Type != "local" { + t.Errorf("Expected type 'local' for legacy command format, got '%s'", config.Type) + } + if len(config.Command) != 3 { t.Errorf("Expected 3 command parts, got %d", len(config.Command)) } @@ -263,6 +268,139 @@ func TestEnsureConfigExists(t *testing.T) { } } +func TestMCPServerConfig_LegacyFormatTypeInference(t *testing.T) { + tests := []struct { + name string + jsonData string + expectedType string + expectedTransport string + }{ + { + name: "Legacy command format should infer local type", + jsonData: `{ + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + }`, + expectedType: "local", + expectedTransport: "stdio", + }, + { + name: "Legacy URL format should preserve legacy behavior", + jsonData: `{ + "url": "https://api.example.com/mcp" + }`, + expectedType: "", // Don't set Type to preserve legacy transport behavior + expectedTransport: "sse", + }, + { + name: "Legacy format with explicit transport should still infer type", + jsonData: `{ + "command": "python", + "args": ["-m", "my_mcp_server"], + "transport": "stdio" + }`, + expectedType: "local", + expectedTransport: "stdio", + }, + { + name: "Legacy format with URL and explicit transport should preserve explicit transport", + jsonData: `{ + "url": "https://remote-server.com", + "transport": "sse" + }`, + expectedType: "", // Don't set Type to preserve legacy behavior + expectedTransport: "sse", + }, + { + name: "Empty legacy format should not set type", + jsonData: `{ + "env": {"VAR": "value"} + }`, + expectedType: "", + expectedTransport: "stdio", + }, + } + + 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) + } + + transportType := config.GetTransportType() + if transportType != tt.expectedTransport { + t.Errorf("Expected transport type '%s', got '%s'", tt.expectedTransport, transportType) + } + }) + } +} + +func TestIssue76_ExactReproduction(t *testing.T) { + // Test the exact config from GitHub issue #76 + jsonData := `{ + "mcpServers": { + "filesystem": { + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-filesystem", + "C:\\test" + ] + } + } + }` + + var cfg Config + err := json.Unmarshal([]byte(jsonData), &cfg) + if err != nil { + t.Fatalf("Error unmarshaling config: %v", err) + } + + // Verify the server config was parsed correctly + if len(cfg.MCPServers) != 1 { + t.Fatalf("Expected 1 server, got %d", len(cfg.MCPServers)) + } + + serverConfig, exists := cfg.MCPServers["filesystem"] + if !exists { + t.Fatal("Expected 'filesystem' server to exist") + } + + // Verify Type field is now set correctly + if serverConfig.Type != "local" { + t.Errorf("Expected type 'local', got '%s'", serverConfig.Type) + } + + // Verify command was parsed correctly + expectedCommand := []string{"npx", "-y", "@modelcontextprotocol/server-filesystem", "C:\\test"} + if len(serverConfig.Command) != len(expectedCommand) { + t.Errorf("Expected %d command parts, got %d", len(expectedCommand), len(serverConfig.Command)) + } + for i, expected := range expectedCommand { + if i >= len(serverConfig.Command) || serverConfig.Command[i] != expected { + t.Errorf("Command part %d: expected '%s', got '%s'", i, expected, serverConfig.Command[i]) + } + } + + // Verify transport type detection works + transportType := serverConfig.GetTransportType() + if transportType != "stdio" { + t.Errorf("Expected transport type 'stdio', got '%s'", transportType) + } + + // Most importantly, verify validation passes + err = cfg.Validate() + if err != nil { + t.Errorf("Validation should pass but failed with: %v", err) + } +} + func TestEnsureConfigExistsWhenFileExists(t *testing.T) { // Create a temporary directory for testing tempDir, err := os.MkdirTemp("", "mcphost_config_test")