Skip to content

Commit 12ecfa4

Browse files
35C4n0r35C4n0r
committed
refactor: update MCP file handling to support new configuration format and improve error messages
1 parent 2fbafff commit 12ecfa4

File tree

2 files changed

+180
-41
lines changed

2 files changed

+180
-41
lines changed

‎x/acpio/acpio.go‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ type acpClient struct {
3434
agentIO *ACPAgentIO
3535
}
3636

37-
type McpConfig struct {
38-
McpServers []acp.McpServer `json:"mcpServers"`
39-
}
40-
4137
var _ acp.Client = (*acpClient)(nil)
4238

4339
func (c *acpClient) SessionUpdate(ctx context.Context, params acp.SessionNotification) error {
@@ -194,7 +190,7 @@ func getSupportedMCPConfig(mcpFilePath string, logger *slog.Logger, initResp *ac
194190

195191
mcpFile, err := os.Open(mcpFilePath)
196192
if err != nil {
197-
return nil, xerrors.Errorf("Failed to open mcp file: %w", err)
193+
return nil, xerrors.Errorf("failed to open mcp file: %w", err)
198194
}
199195

200196
defer func() {
@@ -203,22 +199,35 @@ func getSupportedMCPConfig(mcpFilePath string, logger *slog.Logger, initResp *ac
203199
}
204200
}()
205201

206-
var allMcpList McpConfig
202+
var claudeConfig AgentapiMcpConfig
207203
decoder := json.NewDecoder(mcpFile)
208204

209-
if err = decoder.Decode(&allMcpList); err != nil {
210-
return nil, xerrors.Errorf("Failed to decode mcp file: %w", err)
205+
if err = decoder.Decode(&claudeConfig); err != nil {
206+
return nil, xerrors.Errorf("failed to decode mcp file: %w", err)
211207
}
212208

213-
// Only send the MCPs that are supported by the agents
209+
// Convert MCP format to ACP format and filter by agent capabilities
214210
var supportedMCPList []acp.McpServer
215-
for _, mcp := range allMcpList.McpServers {
216-
if (mcp.Http != nil && !initResp.AgentCapabilities.McpCapabilities.Http) || (mcp.Sse != nil && !initResp.AgentCapabilities.McpCapabilities.Sse) {
211+
for name, server := range claudeConfig.McpServers {
212+
mcpServer, err := server.convertAgentapiMcpToAcp(name)
213+
if err != nil {
214+
logger.Warn("Skipping invalid MCP server", "name", name, "error", err)
215+
continue
216+
}
217+
218+
// Filter based on agent capabilities
219+
if mcpServer.Http != nil && !initResp.AgentCapabilities.McpCapabilities.Http {
220+
logger.Debug("Skipping HTTP MCP server (agent doesn't support HTTP)", "name", name)
221+
continue
222+
}
223+
if mcpServer.Sse != nil && !initResp.AgentCapabilities.McpCapabilities.Sse {
224+
logger.Debug("Skipping SSE MCP server (agent doesn't support SSE)", "name", name)
217225
continue
218226
}
219-
supportedMCPList = append(supportedMCPList, mcp)
227+
228+
supportedMCPList = append(supportedMCPList, mcpServer)
220229
}
221-
return supportedMCPList, err
230+
return supportedMCPList, nil
222231
}
223232

224233
// Write sends a message to the agent via ACP prompt

‎x/acpio/mcp_internal_test.go‎

Lines changed: 158 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestGetSupportedMCPConfig(t *testing.T) {
2525
initResp := &acp.InitializeResponse{}
2626
_, err := getSupportedMCPConfig("/nonexistent/path/mcp.json", logger, initResp)
2727
require.Error(t, err)
28-
assert.Contains(t, err.Error(), "Failed to open mcp file")
28+
assert.Contains(t, err.Error(), "failed to open mcp file")
2929
})
3030

3131
t.Run("invalid JSON returns error", func(t *testing.T) {
@@ -37,21 +37,23 @@ func TestGetSupportedMCPConfig(t *testing.T) {
3737
initResp := &acp.InitializeResponse{}
3838
_, err = getSupportedMCPConfig(mcpFile, logger, initResp)
3939
require.Error(t, err)
40-
assert.Contains(t, err.Error(), "Failed to decode mcp file")
40+
assert.Contains(t, err.Error(), "failed to decode mcp file")
4141
})
4242

4343
t.Run("stdio servers always included", func(t *testing.T) {
4444
tmpDir := t.TempDir()
4545
mcpFile := filepath.Join(tmpDir, "mcp.json")
46+
// Claude MCP format: mcpServers is a map with server name as key
4647
mcpContent := `{
47-
"mcpServers": [
48-
{
49-
"name": "test-stdio",
48+
"mcpServers": {
49+
"test-stdio": {
5050
"command": "/usr/bin/test",
5151
"args": ["--stdio"],
52-
"env": []
52+
"env": {
53+
"DEBUG": "true"
54+
}
5355
}
54-
]
56+
}
5557
}`
5658
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
5759
require.NoError(t, err)
@@ -69,20 +71,27 @@ func TestGetSupportedMCPConfig(t *testing.T) {
6971
assert.Len(t, result, 1)
7072
assert.NotNil(t, result[0].Stdio)
7173
assert.Equal(t, "test-stdio", result[0].Stdio.Name)
74+
assert.Equal(t, "/usr/bin/test", result[0].Stdio.Command)
75+
assert.Equal(t, []string{"--stdio"}, result[0].Stdio.Args)
76+
// Check env was converted correctly
77+
assert.Len(t, result[0].Stdio.Env, 1)
78+
assert.Equal(t, "DEBUG", result[0].Stdio.Env[0].Name)
79+
assert.Equal(t, "true", result[0].Stdio.Env[0].Value)
7280
})
7381

7482
t.Run("http servers filtered when capability is false", func(t *testing.T) {
7583
tmpDir := t.TempDir()
7684
mcpFile := filepath.Join(tmpDir, "mcp.json")
7785
mcpContent := `{
78-
"mcpServers": [
79-
{
86+
"mcpServers": {
87+
"test-http": {
8088
"type": "http",
81-
"name": "test-http",
8289
"url": "https://example.com/mcp",
83-
"headers": []
90+
"headers": {
91+
"Authorization": "Bearer token123"
92+
}
8493
}
85-
]
94+
}
8695
}`
8796
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
8897
require.NoError(t, err)
@@ -104,14 +113,15 @@ func TestGetSupportedMCPConfig(t *testing.T) {
104113
tmpDir := t.TempDir()
105114
mcpFile := filepath.Join(tmpDir, "mcp.json")
106115
mcpContent := `{
107-
"mcpServers": [
108-
{
116+
"mcpServers": {
117+
"test-http": {
109118
"type": "http",
110-
"name": "test-http",
111119
"url": "https://example.com/mcp",
112-
"headers": []
120+
"headers": {
121+
"Authorization": "Bearer token123"
122+
}
113123
}
114-
]
124+
}
115125
}`
116126
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
117127
require.NoError(t, err)
@@ -129,26 +139,28 @@ func TestGetSupportedMCPConfig(t *testing.T) {
129139
assert.Len(t, result, 1)
130140
assert.NotNil(t, result[0].Http)
131141
assert.Equal(t, "test-http", result[0].Http.Name)
142+
assert.Equal(t, "https://example.com/mcp", result[0].Http.Url)
143+
// Check headers were converted correctly
144+
assert.Len(t, result[0].Http.Headers, 1)
145+
assert.Equal(t, "Authorization", result[0].Http.Headers[0].Name)
146+
assert.Equal(t, "Bearer token123", result[0].Http.Headers[0].Value)
132147
})
133148

134149
t.Run("mixed servers filtered correctly", func(t *testing.T) {
135150
tmpDir := t.TempDir()
136151
mcpFile := filepath.Join(tmpDir, "mcp.json")
137152
mcpContent := `{
138-
"mcpServers": [
139-
{
140-
"name": "stdio-server",
153+
"mcpServers": {
154+
"stdio-server": {
141155
"command": "/usr/bin/stdio-mcp",
142-
"args": [],
143-
"env": []
156+
"args": []
144157
},
145-
{
158+
"http-server": {
146159
"type": "http",
147-
"name": "http-server",
148160
"url": "https://example.com/mcp",
149-
"headers": []
161+
"headers": {}
150162
}
151-
]
163+
}
152164
}`
153165
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
154166
require.NoError(t, err)
@@ -175,10 +187,10 @@ func TestGetSupportedMCPConfig(t *testing.T) {
175187
assert.Len(t, result, 2)
176188
})
177189

178-
t.Run("empty mcpServers array returns empty slice", func(t *testing.T) {
190+
t.Run("empty mcpServers object returns empty slice", func(t *testing.T) {
179191
tmpDir := t.TempDir()
180192
mcpFile := filepath.Join(tmpDir, "mcp.json")
181-
mcpContent := `{"mcpServers": []}`
193+
mcpContent := `{"mcpServers": {}}`
182194
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
183195
require.NoError(t, err)
184196

@@ -187,4 +199,122 @@ func TestGetSupportedMCPConfig(t *testing.T) {
187199
require.NoError(t, err)
188200
assert.Empty(t, result)
189201
})
202+
203+
t.Run("server without command or url returns error", func(t *testing.T) {
204+
tmpDir := t.TempDir()
205+
mcpFile := filepath.Join(tmpDir, "mcp.json")
206+
mcpContent := `{
207+
"mcpServers": {
208+
"invalid-server": {
209+
"args": ["--foo"]
210+
}
211+
}
212+
}`
213+
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
214+
require.NoError(t, err)
215+
216+
initResp := &acp.InitializeResponse{}
217+
result, err := getSupportedMCPConfig(mcpFile, logger, initResp)
218+
require.NoError(t, err)
219+
// Invalid servers are skipped with a warning, not an error
220+
assert.Empty(t, result)
221+
})
222+
223+
t.Run("http server inferred from url field", func(t *testing.T) {
224+
tmpDir := t.TempDir()
225+
mcpFile := filepath.Join(tmpDir, "mcp.json")
226+
// No explicit type, but has url - should be inferred as http
227+
mcpContent := `{
228+
"mcpServers": {
229+
"inferred-http": {
230+
"url": "https://example.com/mcp"
231+
}
232+
}
233+
}`
234+
err := os.WriteFile(mcpFile, []byte(mcpContent), 0o644)
235+
require.NoError(t, err)
236+
237+
initResp := &acp.InitializeResponse{
238+
AgentCapabilities: acp.AgentCapabilities{
239+
McpCapabilities: acp.McpCapabilities{
240+
Http: true,
241+
},
242+
},
243+
}
244+
result, err := getSupportedMCPConfig(mcpFile, logger, initResp)
245+
require.NoError(t, err)
246+
assert.Len(t, result, 1)
247+
assert.NotNil(t, result[0].Http)
248+
assert.Equal(t, "inferred-http", result[0].Http.Name)
249+
})
250+
}
251+
252+
func TestConvertAgentapiMcpToAcp(t *testing.T) {
253+
t.Run("converts stdio server correctly", func(t *testing.T) {
254+
server := AgentapiMcpServer{
255+
Command: "/usr/bin/mcp-server",
256+
Args: []string{"--arg1", "--arg2"},
257+
Env: map[string]string{
258+
"KEY1": "value1",
259+
"KEY2": "value2",
260+
},
261+
}
262+
263+
result, err := server.convertAgentapiMcpToAcp("my-server")
264+
require.NoError(t, err)
265+
require.NotNil(t, result.Stdio)
266+
assert.Equal(t, "my-server", result.Stdio.Name)
267+
assert.Equal(t, "/usr/bin/mcp-server", result.Stdio.Command)
268+
assert.Equal(t, []string{"--arg1", "--arg2"}, result.Stdio.Args)
269+
assert.Len(t, result.Stdio.Env, 2)
270+
})
271+
272+
t.Run("converts http server correctly", func(t *testing.T) {
273+
server := AgentapiMcpServer{
274+
Type: "http",
275+
URL: "https://api.example.com/mcp",
276+
Headers: map[string]string{
277+
"Authorization": "Bearer token",
278+
"X-Custom": "value",
279+
},
280+
}
281+
282+
result, err := server.convertAgentapiMcpToAcp("api-server")
283+
require.NoError(t, err)
284+
require.NotNil(t, result.Http)
285+
assert.Equal(t, "api-server", result.Http.Name)
286+
assert.Equal(t, "https://api.example.com/mcp", result.Http.Url)
287+
assert.Len(t, result.Http.Headers, 2)
288+
})
289+
290+
t.Run("returns error for stdio without command", func(t *testing.T) {
291+
server := AgentapiMcpServer{
292+
Type: "stdio",
293+
Args: []string{"--arg"},
294+
}
295+
296+
_, err := server.convertAgentapiMcpToAcp("bad-server")
297+
require.Error(t, err)
298+
assert.Contains(t, err.Error(), "missing command")
299+
})
300+
301+
t.Run("returns error for http without url", func(t *testing.T) {
302+
server := AgentapiMcpServer{
303+
Type: "http",
304+
}
305+
306+
_, err := server.convertAgentapiMcpToAcp("bad-server")
307+
require.Error(t, err)
308+
assert.Contains(t, err.Error(), "missing url")
309+
})
310+
311+
t.Run("returns error for unsupported type", func(t *testing.T) {
312+
server := AgentapiMcpServer{
313+
Type: "websocket",
314+
}
315+
316+
_, err := server.convertAgentapiMcpToAcp("bad-server")
317+
require.Error(t, err)
318+
assert.Contains(t, err.Error(), "unsupported server type")
319+
})
190320
}

0 commit comments

Comments
 (0)