Skip to content

Commit 2a99fab

Browse files
Copilotpelikhan
andcommitted
Fix awmg to properly write modified config file back
- Read original config file before rewriting to preserve non-proxied servers - Only rewrite servers that gateway actually proxies (in config.MCPServers) - Preserve safeinputs/safeoutputs with original command/args configuration - Remove gateway section from rewritten config (per requirement) - Add comprehensive logging for config reading, merging, and writing - Fixed existing tests to not expect gateway section - Added TestRewriteMCPConfigForGateway_PreservesNonProxiedServers test - Added TestRewriteMCPConfigForGateway_NoGatewaySection test - All 43 CLI tests passing (including 2 new config rewrite tests) This fixes the issue where the config file wasn't properly written back with the correct server configurations. The gateway now: 1. Reads original config to get ALL servers 2. Rewrites only proxied servers to gateway URLs 3. Preserves non-proxied servers unchanged 4. Removes gateway section 5. Writes complete merged config back to file Co-authored-by: pelikhan <[email protected]>
1 parent a3b4961 commit 2a99fab

File tree

3 files changed

+253
-30
lines changed

3 files changed

+253
-30
lines changed

pkg/cli/mcp_gateway_command.go

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,22 @@ func rewriteMCPConfigForGateway(configPath string, config *MCPGatewayConfig) err
347347
gatewayLog.Printf("Rewriting MCP config file: %s", configPath)
348348
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Rewriting MCP config file: %s", configPath)))
349349

350+
// Read the original config file to preserve non-proxied servers
351+
gatewayLog.Printf("Reading original config from %s", configPath)
352+
originalConfigData, err := os.ReadFile(configPath)
353+
if err != nil {
354+
gatewayLog.Printf("Failed to read original config: %v", err)
355+
fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to read original config: %v", err)))
356+
return fmt.Errorf("failed to read original config: %w", err)
357+
}
358+
359+
var originalConfig map[string]any
360+
if err := json.Unmarshal(originalConfigData, &originalConfig); err != nil {
361+
gatewayLog.Printf("Failed to parse original config: %v", err)
362+
fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to parse original config: %v", err)))
363+
return fmt.Errorf("failed to parse original config: %w", err)
364+
}
365+
350366
port := config.Gateway.Port
351367
if port == 0 {
352368
port = 8080
@@ -356,13 +372,27 @@ func rewriteMCPConfigForGateway(configPath string, config *MCPGatewayConfig) err
356372
gatewayLog.Printf("Gateway URL: %s", gatewayURL)
357373
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Gateway URL: %s", gatewayURL)))
358374

359-
// Create new config that points all servers to the gateway
375+
// Get original mcpServers to preserve non-proxied servers
376+
var originalMCPServers map[string]any
377+
if servers, ok := originalConfig["mcpServers"].(map[string]any); ok {
378+
originalMCPServers = servers
379+
} else {
380+
originalMCPServers = make(map[string]any)
381+
}
382+
383+
// Create merged config with rewritten proxied servers and preserved non-proxied servers
360384
rewrittenConfig := make(map[string]any)
361385
mcpServers := make(map[string]any)
362386

363-
gatewayLog.Printf("Transforming %d servers to point to gateway", len(config.MCPServers))
364-
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Transforming %d servers to point to gateway", len(config.MCPServers))))
387+
// First, copy all servers from original (preserves non-proxied servers like safeinputs/safeoutputs)
388+
for serverName, serverConfig := range originalMCPServers {
389+
mcpServers[serverName] = serverConfig
390+
}
365391

392+
gatewayLog.Printf("Transforming %d proxied servers to point to gateway", len(config.MCPServers))
393+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Transforming %d proxied servers to point to gateway", len(config.MCPServers))))
394+
395+
// Then, overwrite with gateway URLs for proxied servers only
366396
for serverName := range config.MCPServers {
367397
serverURL := fmt.Sprintf("%s/mcp/%s", gatewayURL, serverName)
368398

@@ -386,18 +416,8 @@ func rewriteMCPConfigForGateway(configPath string, config *MCPGatewayConfig) err
386416

387417
rewrittenConfig["mcpServers"] = mcpServers
388418

389-
// Preserve gateway settings
390-
if config.Gateway.Port != 0 || config.Gateway.APIKey != "" {
391-
gatewayLog.Print("Preserving gateway settings in rewritten config")
392-
gatewaySettings := make(map[string]any)
393-
if config.Gateway.Port != 0 {
394-
gatewaySettings["port"] = config.Gateway.Port
395-
}
396-
if config.Gateway.APIKey != "" {
397-
gatewaySettings["apiKey"] = config.Gateway.APIKey
398-
}
399-
rewrittenConfig["gateway"] = gatewaySettings
400-
}
419+
// Do NOT include gateway section in rewritten config (per requirement)
420+
gatewayLog.Print("Gateway section removed from rewritten config")
401421

402422
// Marshal to JSON with indentation
403423
data, err := json.MarshalIndent(rewrittenConfig, "", " ")
@@ -419,7 +439,8 @@ func rewriteMCPConfigForGateway(configPath string, config *MCPGatewayConfig) err
419439

420440
gatewayLog.Printf("Successfully rewrote MCP config file")
421441
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Successfully rewrote MCP config: %s", configPath)))
422-
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" %d servers now point to gateway at %s", len(config.MCPServers), gatewayURL)))
442+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" %d proxied servers now point to gateway at %s", len(config.MCPServers), gatewayURL)))
443+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf(" %d total servers in config", len(mcpServers))))
423444

424445
return nil
425446
}

pkg/cli/mcp_gateway_command_test.go

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -606,20 +606,11 @@ if customURL != expectedCustomURL {
606606
t.Errorf("Expected custom URL %s, got %s", expectedCustomURL, customURL)
607607
}
608608

609-
// Verify gateway settings are preserved
610-
gateway, ok := rewrittenConfig["gateway"].(map[string]any)
611-
if !ok {
612-
t.Fatal("gateway settings not found")
613-
}
614-
615-
port, ok := gateway["port"].(float64) // JSON numbers unmarshal to float64
616-
if !ok {
617-
t.Fatal("gateway port not found")
618-
}
619-
620-
if int(port) != 8080 {
621-
t.Errorf("Expected port 8080, got %d", int(port))
622-
}
609+
// Verify gateway settings are NOT included in rewritten config
610+
_, hasGateway := rewrittenConfig["gateway"]
611+
if hasGateway {
612+
t.Error("Gateway section should not be included in rewritten config")
613+
}
623614
}
624615

625616
func TestRewriteMCPConfigForGateway_WithAPIKey(t *testing.T) {
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
package cli
2+
3+
import (
4+
"encoding/json"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
)
9+
10+
// TestRewriteMCPConfigForGateway_PreservesNonProxiedServers tests that
11+
// servers not being proxied (like safeinputs/safeoutputs) are preserved unchanged
12+
func TestRewriteMCPConfigForGateway_PreservesNonProxiedServers(t *testing.T) {
13+
// Create a temporary config file
14+
tmpDir := t.TempDir()
15+
configFile := filepath.Join(tmpDir, "test-config.json")
16+
17+
// Initial config with both proxied and non-proxied servers
18+
initialConfig := map[string]any{
19+
"mcpServers": map[string]any{
20+
"safeinputs": map[string]any{
21+
"command": "gh",
22+
"args": []string{"aw", "mcp-server", "--mode", "safe-inputs"},
23+
},
24+
"safeoutputs": map[string]any{
25+
"command": "gh",
26+
"args": []string{"aw", "mcp-server", "--mode", "safe-outputs"},
27+
},
28+
"github": map[string]any{
29+
"command": "docker",
30+
"args": []string{"run", "-i", "--rm", "ghcr.io/github-mcp-server"},
31+
},
32+
},
33+
"gateway": map[string]any{
34+
"port": 8080,
35+
},
36+
}
37+
38+
initialJSON, _ := json.Marshal(initialConfig)
39+
if err := os.WriteFile(configFile, initialJSON, 0644); err != nil {
40+
t.Fatalf("Failed to write config file: %v", err)
41+
}
42+
43+
// Gateway config only includes external server (github), not internal servers
44+
gatewayConfig := &MCPGatewayConfig{
45+
MCPServers: map[string]MCPServerConfig{
46+
"github": {
47+
Command: "docker",
48+
Args: []string{"run", "-i", "--rm", "ghcr.io/github-mcp-server"},
49+
},
50+
},
51+
Gateway: GatewaySettings{
52+
Port: 8080,
53+
},
54+
}
55+
56+
// Rewrite the config
57+
if err := rewriteMCPConfigForGateway(configFile, gatewayConfig); err != nil {
58+
t.Fatalf("rewriteMCPConfigForGateway failed: %v", err)
59+
}
60+
61+
// Read back the rewritten config
62+
rewrittenData, err := os.ReadFile(configFile)
63+
if err != nil {
64+
t.Fatalf("Failed to read rewritten config: %v", err)
65+
}
66+
67+
var rewrittenConfig map[string]any
68+
if err := json.Unmarshal(rewrittenData, &rewrittenConfig); err != nil {
69+
t.Fatalf("Failed to parse rewritten config: %v", err)
70+
}
71+
72+
// Verify structure
73+
mcpServers, ok := rewrittenConfig["mcpServers"].(map[string]any)
74+
if !ok {
75+
t.Fatal("mcpServers not found or wrong type")
76+
}
77+
78+
// Should have all 3 servers: 2 preserved + 1 rewritten
79+
if len(mcpServers) != 3 {
80+
t.Errorf("Expected 3 servers in rewritten config, got %d", len(mcpServers))
81+
}
82+
83+
// Verify safeinputs is preserved with original command/args
84+
safeinputs, ok := mcpServers["safeinputs"].(map[string]any)
85+
if !ok {
86+
t.Fatal("safeinputs server not found")
87+
}
88+
89+
safeinputsCommand, ok := safeinputs["command"].(string)
90+
if !ok || safeinputsCommand != "gh" {
91+
t.Errorf("Expected safeinputs to preserve original command 'gh', got '%v'", safeinputsCommand)
92+
}
93+
94+
safeinputsArgs, ok := safeinputs["args"].([]any)
95+
if !ok {
96+
t.Error("Expected safeinputs to have args array")
97+
} else if len(safeinputsArgs) < 3 {
98+
t.Errorf("Expected safeinputs to have at least 3 args, got %d", len(safeinputsArgs))
99+
}
100+
101+
// Verify safeoutputs is preserved with original command/args
102+
safeoutputs, ok := mcpServers["safeoutputs"].(map[string]any)
103+
if !ok {
104+
t.Fatal("safeoutputs server not found")
105+
}
106+
107+
safeoutputsCommand, ok := safeoutputs["command"].(string)
108+
if !ok || safeoutputsCommand != "gh" {
109+
t.Errorf("Expected safeoutputs to preserve original command 'gh', got '%v'", safeoutputsCommand)
110+
}
111+
112+
safeoutputsArgs, ok := safeoutputs["args"].([]any)
113+
if !ok {
114+
t.Error("Expected safeoutputs to have args array")
115+
} else if len(safeoutputsArgs) < 3 {
116+
t.Errorf("Expected safeoutputs to have at least 3 args, got %d", len(safeoutputsArgs))
117+
}
118+
119+
// Verify github server points to gateway (was rewritten)
120+
github, ok := mcpServers["github"].(map[string]any)
121+
if !ok {
122+
t.Fatal("github server not found")
123+
}
124+
125+
githubURL, ok := github["url"].(string)
126+
if !ok {
127+
t.Fatal("github server should have url (rewritten)")
128+
}
129+
130+
expectedURL := "http://localhost:8080/mcp/github"
131+
if githubURL != expectedURL {
132+
t.Errorf("Expected github URL %s, got %s", expectedURL, githubURL)
133+
}
134+
135+
// Verify github server does NOT have command/args (was rewritten)
136+
if _, hasCommand := github["command"]; hasCommand {
137+
t.Error("Rewritten github server should not have 'command' field")
138+
}
139+
140+
// Verify gateway settings are NOT included in rewritten config
141+
_, hasGateway := rewrittenConfig["gateway"]
142+
if hasGateway {
143+
t.Error("Gateway section should not be included in rewritten config")
144+
}
145+
}
146+
147+
// TestRewriteMCPConfigForGateway_NoGatewaySection tests that gateway section is removed
148+
func TestRewriteMCPConfigForGateway_NoGatewaySection(t *testing.T) {
149+
// Create a temporary config file
150+
tmpDir := t.TempDir()
151+
configFile := filepath.Join(tmpDir, "test-config.json")
152+
153+
initialConfig := map[string]any{
154+
"mcpServers": map[string]any{
155+
"github": map[string]any{
156+
"command": "gh",
157+
"args": []string{"aw", "mcp-server"},
158+
},
159+
},
160+
"gateway": map[string]any{
161+
"port": 8080,
162+
"apiKey": "test-key",
163+
},
164+
}
165+
166+
initialJSON, _ := json.Marshal(initialConfig)
167+
if err := os.WriteFile(configFile, initialJSON, 0644); err != nil {
168+
t.Fatalf("Failed to write config file: %v", err)
169+
}
170+
171+
gatewayConfig := &MCPGatewayConfig{
172+
MCPServers: map[string]MCPServerConfig{
173+
"github": {
174+
Command: "gh",
175+
Args: []string{"aw", "mcp-server"},
176+
},
177+
},
178+
Gateway: GatewaySettings{
179+
Port: 8080,
180+
APIKey: "test-key",
181+
},
182+
}
183+
184+
// Rewrite the config
185+
if err := rewriteMCPConfigForGateway(configFile, gatewayConfig); err != nil {
186+
t.Fatalf("rewriteMCPConfigForGateway failed: %v", err)
187+
}
188+
189+
// Read back the rewritten config
190+
rewrittenData, err := os.ReadFile(configFile)
191+
if err != nil {
192+
t.Fatalf("Failed to read rewritten config: %v", err)
193+
}
194+
195+
var rewrittenConfig map[string]any
196+
if err := json.Unmarshal(rewrittenData, &rewrittenConfig); err != nil {
197+
t.Fatalf("Failed to parse rewritten config: %v", err)
198+
}
199+
200+
// Verify gateway settings are NOT included in rewritten config
201+
_, hasGateway := rewrittenConfig["gateway"]
202+
if hasGateway {
203+
t.Error("Gateway section should not be included in rewritten config")
204+
}
205+
206+
// Verify mcpServers still exists
207+
_, hasMCPServers := rewrittenConfig["mcpServers"]
208+
if !hasMCPServers {
209+
t.Error("mcpServers section should be present in rewritten config")
210+
}
211+
}

0 commit comments

Comments
 (0)