diff --git a/internal/mcp/bridge.go b/internal/mcp/bridge.go index 32d70ef..f705cd0 100644 --- a/internal/mcp/bridge.go +++ b/internal/mcp/bridge.go @@ -67,8 +67,43 @@ func NewServiceBridge( } } +// bridgeActionAliases maps observed wrong action names that agents have called +// to the real bridge action they probably meant. This is a small, hand-curated +// whitelist of guesses seen in production logs — not a fuzzy-match layer. +// Keep entries minimal and only add names that have a single unambiguous target. +var bridgeActionAliases = map[string]string{ + // read_channel → fetch messages from a channel (requires channel_id/name) + "read_channel": "get_channel_messages", + // search → unified search over messages + "search": "search_messages", + // read_dm → DMs land in the inbox + "read_dm": "read_inbox", + // my_status → no exact bridge equivalent; agents probing for "what's new" + // most often want read_inbox. (The top-level MCP `my_status` tool is the + // canonical way; this alias keeps call() from failing.) + "my_status": "read_inbox", + // read_article → wiki retrieval + "read_article": "get_article", +} + +// bridgeTopLevelOnly lists action names that exist as top-level MCP tools +// (registered on the MCP server), not as call() bridge actions. Agents that +// invoke these via call() / execute() get a targeted error pointing them at +// the real tool rather than a generic "unknown action". +var bridgeTopLevelOnly = map[string]string{ + "rewrite_core_memory": "memory_rewrite_core", +} + // Call dispatches an action by name to the appropriate service method. func (b *ServiceBridge) Call(ctx context.Context, actionName string, args map[string]any) (any, error) { + // Apply aliases for common wrong-name guesses observed in agent logs. + if real, ok := bridgeActionAliases[actionName]; ok { + actionName = real + } + // Catch wrong guesses that map to top-level MCP tools (not bridge actions). + if real, ok := bridgeTopLevelOnly[actionName]; ok { + return nil, fmt.Errorf("'%s' is a top-level MCP tool, not a call() action; invoke it as a tool directly (real name: %s)", actionName, real) + } switch actionName { // --- Messaging --- case "read_inbox": @@ -173,10 +208,126 @@ func (b *ServiceBridge) Call(ctx context.Context, actionName string, args map[st return b.callQueryReputation(ctx, args) default: + if suggestion := suggestBridgeAction(actionName); suggestion != "" { + return nil, fmt.Errorf("unknown action: %s (did you mean: %s)", actionName, suggestion) + } return nil, fmt.Errorf("unknown action: %s", actionName) } } +// knownBridgeActions enumerates every action name routable through Call(). +// Kept in sync with the switch in Call() by hand — there are not many. Used +// only to power "did you mean" suggestions when an unknown action arrives. +var knownBridgeActions = []string{ + // Messaging + "read_inbox", "claim_messages", "mark_done", "search_messages", + "discover_agents", "send_message", + // Channels + "create_channel", "join_channel", "leave_channel", "list_channels", + "invite_to_channel", "kick_from_channel", "get_channel_messages", + "send_channel_message", "update_channel", + // Swarm + "post_task", "bid_task", "accept_bid", "complete_task", "list_tasks", + // Attachments + "upload_attachment", "download_attachment", + // Reactions + "react", "unreact", "get_reactions", "list_by_state", + // Threads + "get_replies", + // Trust + "get_trust", + // SQL Query + "query", + // Wiki + "create_article", "get_article", "update_article", "list_articles", + "get_backlinks", + // Marketplace + "post_auction", "bid", "award", "mark_task_done", "read_skill_card", + "query_reputation", +} + +// suggestBridgeAction returns the closest known action name to `name`, or "" +// if nothing is plausibly close. Strategy: cheap prefix/substring check first, +// then Levenshtein within distance 3. Distance threshold scales with the +// length of the input so very short strings don't false-match. +func suggestBridgeAction(name string) string { + if name == "" { + return "" + } + lower := strings.ToLower(name) + + // 1. Substring match — common when agents drop or add a prefix/suffix. + for _, candidate := range knownBridgeActions { + if strings.Contains(candidate, lower) || strings.Contains(lower, candidate) { + return candidate + } + } + + // 2. Levenshtein on full names. + threshold := 3 + if len(name) <= 6 { + threshold = 2 + } + bestDist := threshold + 1 + best := "" + for _, candidate := range knownBridgeActions { + d := levenshtein(lower, candidate) + if d < bestDist { + bestDist = d + best = candidate + } + } + if bestDist <= threshold { + return best + } + return "" +} + +// levenshtein computes the Levenshtein edit distance between a and b using +// a single rolling row of O(min(len)) space. Pure Go, no deps. +func levenshtein(a, b string) int { + if a == b { + return 0 + } + if len(a) == 0 { + return len(b) + } + if len(b) == 0 { + return len(a) + } + // Ensure a is the shorter — minimises row width. + if len(a) > len(b) { + a, b = b, a + } + prev := make([]int, len(a)+1) + curr := make([]int, len(a)+1) + for i := 0; i <= len(a); i++ { + prev[i] = i + } + for j := 1; j <= len(b); j++ { + curr[0] = j + for i := 1; i <= len(a); i++ { + cost := 1 + if a[i-1] == b[j-1] { + cost = 0 + } + del := prev[i] + 1 + ins := curr[i-1] + 1 + sub := prev[i-1] + cost + m := del + if ins < m { + m = ins + } + if sub < m { + m = sub + } + curr[i] = m + } + prev, curr = curr, prev + } + return prev[len(a)] +} + // --- Messaging implementations --- func (b *ServiceBridge) callSendMessage(ctx context.Context, args map[string]any) (any, error) { diff --git a/internal/mcp/bridge_test.go b/internal/mcp/bridge_test.go index 98342c8..48231a8 100644 --- a/internal/mcp/bridge_test.go +++ b/internal/mcp/bridge_test.go @@ -3,6 +3,7 @@ package mcp import ( "context" "log/slog" + "strings" "testing" _ "modernc.org/sqlite" @@ -486,3 +487,172 @@ func TestBridge_React_Toggle_Removes_WorkflowState(t *testing.T) { } var _ = storage.RunMigrations + +// TestBridge_ActionAliases verifies that each observed wrong action name from +// production logs resolves to the correct real action. We don't validate the +// full result payload — only that the call succeeds (i.e. dispatch reached the +// real handler) rather than returning "unknown action". +func TestBridge_ActionAliases(t *testing.T) { + tests := []struct { + alias string + realName string + args map[string]any + setupCh string // optional: channel name to create+join before the call + expectErr string // optional: substring of expected error (when call reaches real handler but fails for unrelated reasons) + }{ + { + alias: "search", + realName: "search_messages", + args: map[string]any{"query": "anything"}, + }, + { + alias: "my_status", + realName: "read_inbox", + args: map[string]any{"limit": 5}, + }, + { + alias: "read_dm", + realName: "read_inbox", + args: map[string]any{"limit": 5}, + }, + { + alias: "read_channel", + realName: "get_channel_messages", + args: map[string]any{"channel_name": "alias-ch"}, + setupCh: "alias-ch", + }, + { + alias: "read_article", + realName: "get_article", + args: map[string]any{"slug": "anything"}, + expectErr: "wiki not available", // bridge has no wikiService → dispatch reached real handler + }, + } + + for _, tt := range tests { + t.Run(tt.alias, func(t *testing.T) { + bridge, _, _, channelService := newTestBridge(t) + ctx := context.Background() + + if tt.setupCh != "" { + ch, err := channelService.CreateChannel(ctx, channels.CreateChannelRequest{ + Name: tt.setupCh, Type: "standard", CreatedBy: "agent-a", + }) + if err != nil { + t.Fatalf("create channel: %v", err) + } + if err := channelService.JoinChannel(ctx, ch.ID, "agent-a"); err != nil { + t.Fatalf("join channel: %v", err) + } + } + + _, err := bridge.Call(ctx, tt.alias, tt.args) + if tt.expectErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.expectErr) + } + if !strings.Contains(err.Error(), tt.expectErr) { + t.Fatalf("error %q does not contain %q", err.Error(), tt.expectErr) + } + if strings.Contains(err.Error(), "unknown action") { + t.Fatalf("alias %q should have been resolved, got unknown-action error: %v", tt.alias, err) + } + return + } + if err != nil { + t.Fatalf("alias %q (real=%s) failed: %v", tt.alias, tt.realName, err) + } + }) + } +} + +// TestBridge_TopLevelToolHint verifies that wrong-guess names which map to +// top-level MCP tools (not bridge actions) produce a targeted hint pointing +// at the real tool, instead of a generic "unknown action" error. +func TestBridge_TopLevelToolHint(t *testing.T) { + bridge, _, _, _ := newTestBridge(t) + ctx := context.Background() + + _, err := bridge.Call(ctx, "rewrite_core_memory", map[string]any{}) + if err == nil { + t.Fatal("expected error for top-level-tool wrong-guess") + } + msg := err.Error() + if !strings.Contains(msg, "top-level MCP tool") { + t.Errorf("error should mention top-level MCP tool, got: %v", err) + } + if !strings.Contains(msg, "memory_rewrite_core") { + t.Errorf("error should name the real tool memory_rewrite_core, got: %v", err) + } +} + +// TestBridge_UnknownAction_Suggestion verifies that an unknown action that is +// close to a real one (Levenshtein-wise) returns a "did you mean" suggestion. +func TestBridge_UnknownAction_Suggestion(t *testing.T) { + bridge, _, _, _ := newTestBridge(t) + ctx := context.Background() + + tests := []struct { + input string + wantContains string + }{ + // substring hit: "send" is contained in "send_message" + {input: "send", wantContains: "send_message"}, + // Levenshtein: typo "sned_message" → "send_message" (distance 2) + {input: "sned_message", wantContains: "send_message"}, + // Levenshtein: "list_channel" → "list_channels" (distance 1) + {input: "list_channel", wantContains: "list_channels"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + _, err := bridge.Call(ctx, tt.input, map[string]any{}) + if err == nil { + t.Fatalf("expected error for %q", tt.input) + } + if !strings.Contains(err.Error(), "did you mean") { + t.Errorf("error should contain 'did you mean', got: %v", err) + } + if !strings.Contains(err.Error(), tt.wantContains) { + t.Errorf("error should suggest %q, got: %v", tt.wantContains, err) + } + }) + } +} + +// TestBridge_UnknownAction_NoSuggestion verifies that a truly distant unknown +// action returns the plain "unknown action" error without a misleading +// suggestion. +func TestBridge_UnknownAction_NoSuggestion(t *testing.T) { + bridge, _, _, _ := newTestBridge(t) + ctx := context.Background() + + _, err := bridge.Call(ctx, "xyzzy_quux_frobnicate", map[string]any{}) + if err == nil { + t.Fatal("expected error") + } + if strings.Contains(err.Error(), "did you mean") { + t.Errorf("distant action should not get a suggestion, got: %v", err) + } +} + +func TestLevenshtein(t *testing.T) { + tests := []struct { + a, b string + want int + }{ + {"", "", 0}, + {"abc", "abc", 0}, + {"", "abc", 3}, + {"abc", "", 3}, + {"kitten", "sitting", 3}, + {"send", "sned", 2}, + {"list_channel", "list_channels", 1}, + } + for _, tt := range tests { + got := levenshtein(tt.a, tt.b) + if got != tt.want { + t.Errorf("levenshtein(%q, %q) = %d, want %d", tt.a, tt.b, got, tt.want) + } + } +}