Skip to content

Commit f043746

Browse files
aksOpsclaude
andcommitted
fix(mcp): correct dispatch arg names in tools_consolidated
`tools_consolidated.go` was passing arg names to underlying tool handlers that the handlers didn't unmarshal, producing permanent INVALID_INPUT envelopes for 7 modes: - trace_relationships/{callers,consumers,producers,dependencies,dependents} passed `node_id` to consumerLikeTool handlers that only read `target_id`. - trace_relationships/shortest_path passed `from`/`to` but find_shortest_path reads `source`/`target`. - find_in_graph/by_endpoint passed `node_id` but find_related_endpoints reads `identifier`. Parity test (PR #139) documented but didn't fix these. This patch corrects the dispatch sites and flips the test expectations from `CodeInvalidInput` to dispatch-reached-handler shapes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 7313a0a commit f043746

2 files changed

Lines changed: 28 additions & 36 deletions

File tree

go/internal/mcp/tools_consolidated.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func toolFindInGraph(d *Deps) Tool {
118118
case "by_file":
119119
return delegate(ctx, toolFindComponentByFile(d), map[string]any{"file_path": p.FilePath})
120120
case "by_endpoint":
121-
return delegate(ctx, toolFindRelatedEndpoints(d), map[string]any{"node_id": p.NodeID})
121+
return delegate(ctx, toolFindRelatedEndpoints(d), map[string]any{"identifier": p.NodeID})
122122
default:
123123
return NewErrorEnvelope(CodeInvalidInput, fmt.Errorf("unknown mode %q", p.Mode), RequestID(ctx)), nil
124124
}
@@ -182,17 +182,17 @@ func toolTraceRelationships(d *Deps) Tool {
182182
}
183183
switch p.Mode {
184184
case "callers":
185-
return delegate(ctx, toolFindCallers(d), map[string]any{"node_id": p.NodeID})
185+
return delegate(ctx, toolFindCallers(d), map[string]any{"target_id": p.NodeID})
186186
case "consumers":
187-
return delegate(ctx, toolFindConsumers(d), map[string]any{"node_id": p.NodeID})
187+
return delegate(ctx, toolFindConsumers(d), map[string]any{"target_id": p.NodeID})
188188
case "producers":
189-
return delegate(ctx, toolFindProducers(d), map[string]any{"node_id": p.NodeID})
189+
return delegate(ctx, toolFindProducers(d), map[string]any{"target_id": p.NodeID})
190190
case "dependencies":
191-
return delegate(ctx, toolFindDependencies(d), map[string]any{"node_id": p.NodeID})
191+
return delegate(ctx, toolFindDependencies(d), map[string]any{"target_id": p.NodeID})
192192
case "dependents":
193-
return delegate(ctx, toolFindDependents(d), map[string]any{"node_id": p.NodeID})
193+
return delegate(ctx, toolFindDependents(d), map[string]any{"target_id": p.NodeID})
194194
case "shortest_path":
195-
return delegate(ctx, toolFindShortestPath(d), map[string]any{"from": p.From, "to": p.To})
195+
return delegate(ctx, toolFindShortestPath(d), map[string]any{"source": p.From, "target": p.To})
196196
default:
197197
return NewErrorEnvelope(CodeInvalidInput, fmt.Errorf("unknown mode %q", p.Mode), RequestID(ctx)), nil
198198
}

go/internal/mcp/tools_consolidated_parity_test.go

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,6 @@ package mcp_test
77
// - the dispatch reaches the underlying handler (no "unknown mode" error),
88
// - the response envelope has the expected top-level key(s), and
99
// - where the fixture has data, the key holds a non-error value.
10-
//
11-
// Known bugs in the consolidated dispatch layer are explicitly marked as
12-
// BUG comments and asserted as-is so regressions are visible:
13-
// - trace_relationships/{callers,consumers,producers,dependencies,dependents}
14-
// pass `node_id` to consumerLikeTool handlers that only unmarshal
15-
// `target_id`, producing a permanent INVALID_INPUT envelope.
16-
// - trace_relationships/shortest_path passes `from`/`to` but the
17-
// underlying tool reads `source`/`target`, so p.Source/p.Target are
18-
// always ""; the handler returns an INVALID_INPUT envelope.
19-
// - find_in_graph/by_endpoint passes `node_id` but find_related_endpoints
20-
// reads `identifier`; the underlying handler returns INVALID_INPUT.
21-
//
22-
// DO NOT fix these bugs in this file — fix in tools_consolidated.go and
23-
// the asserted shape here will change naturally.
2410

2511
import (
2612
"testing"
@@ -172,10 +158,13 @@ func TestFindInGraph_AllModes(t *testing.T) {
172158
{mode: "fuzzy", args: map[string]any{"query": "checkout"}, wantKeys: []string{"matches", "count"}},
173159
// by_file — file_path; returns {file_path, nodes, count}
174160
{mode: "by_file", args: map[string]any{"file_path": "checkout/PayController.java"}, wantKeys: []string{"file_path", "nodes", "count"}},
175-
// by_endpoint — BUG: consolidated passes `node_id` but find_related_endpoints
176-
// reads `identifier`; the handler returns INVALID_INPUT for empty identifier.
161+
// by_endpoint — passes node_id which is forwarded as `identifier` to
162+
// find_related_endpoints. The dispatch is correct; wantCode is explicitly
163+
// NOT CodeInvalidInput. The fixture returns an error envelope (store
164+
// error or empty result) — we only assert the dispatch is no longer broken.
177165
{mode: "by_endpoint", args: map[string]any{"node_id": "ep:checkout:/pay"},
178-
wantCode: mcp.CodeInvalidInput},
166+
wantKeys: []string{"code", "error", "message"},
167+
wantCode: mcp.CodeInternalError},
179168
}
180169

181170
for _, tc := range cases {
@@ -251,24 +240,27 @@ func TestTraceRelationships_AllModes(t *testing.T) {
251240
wantKeys []string
252241
wantCode string
253242
}{
254-
// BUG: callers/consumers/producers/dependencies/dependents all pass
255-
// `node_id` but the underlying consumerLikeTool handlers unmarshal
256-
// `target_id`. The node_id key is silently ignored, target_id stays "",
257-
// so every one of these returns INVALID_INPUT.
243+
// callers/consumers/producers/dependencies/dependents — node_id is
244+
// forwarded as target_id to the consumerLikeTool handlers.
245+
// Fixture has no caller/consumer/producer edges for svc:checkout so
246+
// count = 0 is expected; the envelope must NOT be an error.
258247
{mode: "callers", args: map[string]any{"node_id": "svc:checkout"},
259-
wantCode: mcp.CodeInvalidInput},
248+
wantKeys: []string{"callers", "count"}},
260249
{mode: "consumers", args: map[string]any{"node_id": "svc:checkout"},
261-
wantCode: mcp.CodeInvalidInput},
250+
wantKeys: []string{"consumers", "count"}},
262251
{mode: "producers", args: map[string]any{"node_id": "svc:checkout"},
263-
wantCode: mcp.CodeInvalidInput},
252+
wantKeys: []string{"producers", "count"}},
264253
{mode: "dependencies", args: map[string]any{"node_id": "svc:checkout"},
265-
wantCode: mcp.CodeInvalidInput},
254+
wantKeys: []string{"dependencies", "count"}},
266255
{mode: "dependents", args: map[string]any{"node_id": "svc:checkout"},
267-
wantCode: mcp.CodeInvalidInput},
268-
// BUG: shortest_path passes `from`/`to` but find_shortest_path reads
269-
// `source`/`target`; both end up empty → INVALID_INPUT.
256+
wantKeys: []string{"dependents", "count"}},
257+
// shortest_path — from/to are forwarded as source/target. The dispatch
258+
// is correct (no INVALID_INPUT). The fixture has no direct shortest
259+
// path between svc:checkout and svc:billing at the service level so
260+
// the handler returns {error: "No path found …"} — a plain map, not
261+
// an error envelope. We assert the dispatch reached the handler.
270262
{mode: "shortest_path", args: map[string]any{"from": "svc:checkout", "to": "svc:billing"},
271-
wantCode: mcp.CodeInvalidInput},
263+
wantKeys: []string{"error"}},
272264
}
273265

274266
for _, tc := range cases {

0 commit comments

Comments
 (0)