fix overwriting mcp config bug (#1394)

Co-authored-by: Jordan Stephens <[email protected]>
Co-authored-by: Lio李歐 <[email protected]>

Author: 3 people

src/pkg/cli/compose/fixup_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/DefangLabs/defang/src/pkg"
 	"github.com/DefangLabs/defang/src/pkg/cli/client"
 	composeTypes "github.com/compose-spec/compose-go/v2/types"
 )
@@ -32,7 +33,7 @@ func TestFixup(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if err := compare(actual, path+".fixup"); err != nil {
+		if err := pkg.Compare(actual, path+".fixup"); err != nil {
 			t.Error(err)
 		}
 	})

src/pkg/cli/compose/loader_test.go

@@ -1,17 +1,13 @@
 package compose
 
 import (
-	"bytes"
 	"context"
-	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
 	"testing"
 
-	"github.com/hexops/gotextdiff"
-	"github.com/hexops/gotextdiff/myers"
-	"github.com/hexops/gotextdiff/span"
+	"github.com/DefangLabs/defang/src/pkg"
 )
 
 func TestLoader(t *testing.T) {
@@ -28,41 +24,12 @@ func TestLoader(t *testing.T) {
 		}
 
 		// Compare the output with the golden file
-		if err := compare(yaml, path+".golden"); err != nil {
+		if err := pkg.Compare(yaml, path+".golden"); err != nil {
 			t.Error(err)
 		}
 	})
 }
 
-func compare(actual []byte, goldenFile string) error {
-	// Replace the absolute path in context to make the golden file portable
-	absPath, _ := filepath.Abs(goldenFile)
-	actual = bytes.ReplaceAll(actual, []byte(filepath.Dir(absPath)), []byte{'.'})
-
-	golden, err := os.ReadFile(goldenFile)
-	if err != nil {
-		if !os.IsNotExist(err) {
-			return fmt.Errorf("Failed to read golden file: %w", err)
-		}
-		return os.WriteFile(goldenFile, actual, 0644)
-	} else {
-		if err := diff(string(actual), string(golden)); err != nil {
-			return fmt.Errorf("%s %w", goldenFile, err)
-		}
-	}
-	return nil
-}
-
-func diff(actualRaw, goldenRaw string) error {
-	if actualRaw == goldenRaw {
-		return nil
-	}
-
-	edits := myers.ComputeEdits(span.URIFromPath("expected"), goldenRaw, actualRaw)
-	diff := fmt.Sprint(gotextdiff.ToUnified("expected", "actual", goldenRaw, edits))
-	return fmt.Errorf("mismatch:\n%s", diff)
-}
-
 func testRunCompose(t *testing.T, f func(t *testing.T, path string)) {
 	t.Helper()
 

src/pkg/cli/compose/validation_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/DefangLabs/defang/src/pkg"
 	"github.com/DefangLabs/defang/src/pkg/cli/client"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
@@ -66,7 +67,7 @@ func TestValidationAndConvert(t *testing.T) {
 		logs = bytes.NewBufferString(strings.Join(logLines, ""))
 
 		// Compare the logs with the warnings file
-		if err := compare(logs.Bytes(), path+".warnings"); err != nil {
+		if err := pkg.Compare(logs.Bytes(), path+".warnings"); err != nil {
 			t.Error(err)
 		}
 	})

src/pkg/mcp/setup.go

@@ -244,17 +244,10 @@ func handleVSCodeConfig(configPath string) error {
 	}
 
 	// Check if the file exists
-	if _, err := os.Stat(configPath); err == nil {
-		// File exists, read it
-		data, err := os.ReadFile(configPath)
-		if err != nil {
-			return fmt.Errorf("failed to read config file: %w", err)
-		}
-
-		// Parse the JSON into a generic map to preserve all settings
+	if data, err := os.ReadFile(configPath); err == nil {
+		// File exists, parse it
 		if err := json.Unmarshal(data, &existingData); err != nil {
-			// If we can't parse it, start fresh
-			existingData = make(map[string]any)
+			return fmt.Errorf("failed to unmarshal existing vscode config %w", err)
 		}
 
 		// Check if "servers" section exists
@@ -271,6 +264,8 @@ func handleVSCodeConfig(configPath string) error {
 		} else {
 			return errors.New("failed to assert 'servers' section as map[string]any")
 		}
+	} else if !os.IsNotExist(err) {
+		return fmt.Errorf("failed to read config file: %w", err)
 	} else {
 		// File doesn't exist, create a new config with minimal settings
 		existingData = map[string]any{
@@ -296,28 +291,33 @@ func handleVSCodeConfig(configPath string) error {
 
 func handleStandardConfig(configPath string) error {
 	// For all other clients, use the standard format
+	var existingData map[string]any
 	var config MCPConfig
 
 	// Check if the file exists
-	if _, err := os.Stat(configPath); err == nil {
-		// File exists, read it
-		data, err := os.ReadFile(configPath)
-		if err != nil {
-			return fmt.Errorf("failed to read config file: %w", err)
+	if data, err := os.ReadFile(configPath); err == nil {
+		// Parse the JSON into a generic map to preserve all settings
+		if err := json.Unmarshal(data, &existingData); err != nil {
+			return fmt.Errorf("failed to unmarshal existing config: %w", err)
 		}
 
-		// Parse the JSON
-		if err := json.Unmarshal(data, &config); err != nil {
-			// If we can't parse it, start fresh
-			config = MCPConfig{
-				MCPServers: make(map[string]MCPServerConfig),
+		// Try to extract MCPServers from existing data
+		if mcpServersData, ok := existingData["mcpServers"]; ok {
+			// Convert back to MCPConfig structure
+			mcpServersJSON, err := json.Marshal(map[string]any{"mcpServers": mcpServersData})
+			if err != nil {
+				return fmt.Errorf("failed to marshal mcpServers: %w", err)
+			}
+			err = json.Unmarshal(mcpServersJSON, &config)
+			if err != nil {
+				return fmt.Errorf("failed to unmarshal mcpServers: %w", err)
 			}
 		}
+	} else if !os.IsNotExist(err) {
+		return fmt.Errorf("failed to read config file: %w", err)
 	} else {
 		// File doesn't exist, create a new config
-		config = MCPConfig{
-			MCPServers: make(map[string]MCPServerConfig),
-		}
+		existingData = make(map[string]any)
 	}
 
 	if config.MCPServers == nil {
@@ -331,8 +331,11 @@ func handleStandardConfig(configPath string) error {
 	// Add or update the Defang MCP server config
 	config.MCPServers["defang"] = *defangConfig
 
+	// Update the existingData with the new MCPServers
+	existingData["mcpServers"] = config.MCPServers
+
 	// Write the config to the file
-	data, err := json.MarshalIndent(config, "", "  ")
+	data, err := json.MarshalIndent(existingData, "", "  ")
 	if err != nil {
 		return fmt.Errorf("failed to marshal config: %w", err)
 	}