From c9332cf3620adaf0b898725676ec8aca9fa17f87 Mon Sep 17 00:00:00 2001 From: Yves Junqueira <202998+nictuku@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:20:33 -0300 Subject: [PATCH] fix(mcp): decode and show each tool's title in tools-list ToolDefinition had no top-level Title field, so every tool decoded as untitled. tools/list showed 0/63 titled against a google-ads-mcp that returns 63/63 -- and because --json and the on-disk tools cache both re-marshal this struct, the field vanished from those too. That is worse than cosmetic: a title is a BLOCKING Claude Connectors Directory submission requirement, and the pipeboard.co docs point at this command as the live compliance check. Following that advice reported the whole fleet as non-compliant when it was fine. MCP puts the display name in two places: the top-level title (spec 2025-06-18, what Pipeboard servers populate) and the legacy annotations.title. Only the latter was modeled. Add the top-level field, prefer it via DisplayTitle() with the legacy one as fallback, and surface it in all three output modes -- a title column in the compact list, a Title: line under --verbose, and the raw field under --json. An untitled tool renders "(no title!)" rather than blank, since on a Pipeboard server that is a real defect worth seeing. --- cmd/mcp.go | 16 ++++++++-- cmd/mcp_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ internal/client/mcp.go | 24 +++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/cmd/mcp.go b/cmd/mcp.go index 855a642..2810e4a 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -116,6 +116,7 @@ func runMCPToolsList(cmd *cobra.Command, args []string) error { if mcpToolsListVerbose { for _, tool := range tools { fmt.Printf("=== %s ===\n", tool.Name) + fmt.Printf("Title: %s\n", titleOrMissing(tool)) fmt.Printf("Category: %s\n", toolCategoryLabel(tool.Annotations)) if tool.Annotations != nil { annJSON, _ := json.MarshalIndent(tool.Annotations, "", " ") @@ -133,16 +134,27 @@ func runMCPToolsList(cmd *cobra.Command, args []string) error { } } else { for _, tool := range tools { - fmt.Printf("%-7s %-40s %s\n", + fmt.Printf("%-7s %-40s %-32s %s\n", "["+toolCategoryLabel(tool.Annotations)+"]", tool.Name, - truncate(tool.Description, 80)) + truncate(titleOrMissing(tool), 32), + truncate(tool.Description, 60)) } } return nil } +// titleOrMissing renders a tool's display title, or a loud placeholder when the +// server sent none. A missing title is a blocking Claude Connectors submission +// defect, so it gets called out rather than silently blank. +func titleOrMissing(tool client.ToolDefinition) string { + if t := tool.DisplayTitle(); t != "" { + return t + } + return "(no title!)" +} + // toolCategoryLabel returns a short tag describing a tool's behavior based on // its MCP annotations. Categories follow the MCP hint semantics; empty // annotations fall back to "?" because the spec defaults are pessimistic diff --git a/cmd/mcp_test.go b/cmd/mcp_test.go index 9f5cf58..e610132 100644 --- a/cmd/mcp_test.go +++ b/cmd/mcp_test.go @@ -276,3 +276,73 @@ func TestEmitJSONError_InJSONMode(t *testing.T) { t.Errorf("error = %v, want EOF", got["error"]) } } + +func TestToolDefinition_TitleRoundTrip(t *testing.T) { + // A tools/list response as Pipeboard servers actually send it: `title` is a + // TOP-LEVEL field (MCP spec 2025-06-18), not `annotations.title`. Decoding + // used to drop it, so `tools-list` showed every tool as untitled and the + // on-disk cache persisted that — hence the round-trip assertion. + raw := []byte(`{ + "name": "execute_google_ads_gaql_query", + "title": "Execute Google Ads GAQL Query", + "description": "Runs a GAQL query", + "inputSchema": {"type":"object","properties":{}}, + "annotations": {"readOnlyHint": true} + }`) + + var td client.ToolDefinition + if err := json.Unmarshal(raw, &td); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if td.Title != "Execute Google Ads GAQL Query" { + t.Fatalf("Title = %q, want the server's top-level title", td.Title) + } + + out, err := json.Marshal(td) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var back map[string]interface{} + if err := json.Unmarshal(out, &back); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + if back["title"] != "Execute Google Ads GAQL Query" { + t.Errorf("re-marshaled title = %v, want it preserved for --json and the cache", back["title"]) + } +} + +func TestToolDisplayTitle(t *testing.T) { + tests := []struct { + name string + td client.ToolDefinition + want string + }{ + {"top-level title", client.ToolDefinition{Title: "Get Campaigns"}, "Get Campaigns"}, + {"legacy annotations.title fallback", client.ToolDefinition{ + Annotations: &client.ToolAnnotations{Title: "Get Campaigns"}, + }, "Get Campaigns"}, + {"top-level wins over annotations", client.ToolDefinition{ + Title: "Get Campaigns", + Annotations: &client.ToolAnnotations{Title: "Stale Legacy Title"}, + }, "Get Campaigns"}, + {"neither -> empty, never the machine name", client.ToolDefinition{Name: "get_campaigns"}, ""}, + {"nil annotations", client.ToolDefinition{Name: "get_campaigns", Annotations: nil}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.td.DisplayTitle(); got != tt.want { + t.Errorf("DisplayTitle() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestTitleOrMissing(t *testing.T) { + if got := titleOrMissing(client.ToolDefinition{Title: "Get Campaigns"}); got != "Get Campaigns" { + t.Errorf("titleOrMissing = %q, want the title", got) + } + // An untitled tool is a blocking submission defect — say so, don't render blank. + if got := titleOrMissing(client.ToolDefinition{Name: "get_campaigns"}); got != "(no title!)" { + t.Errorf("titleOrMissing = %q, want the missing-title placeholder", got) + } +} diff --git a/internal/client/mcp.go b/internal/client/mcp.go index 48dad18..4a8027d 100644 --- a/internal/client/mcp.go +++ b/internal/client/mcp.go @@ -70,13 +70,37 @@ type ToolAnnotations struct { } // ToolDefinition represents a tool from tools/list. +// +// Title is the tool's human-readable display name. Note there are TWO places a +// title can live: this top-level field (MCP spec 2025-06-18 onward, what +// Pipeboard servers populate) and the legacy `annotations.title`. Without the +// top-level field here, every tool decoded as untitled — including in `--json` +// output and the on-disk tools cache, both of which re-marshal this struct — so +// `tools-list` reported 0/63 titled against a server that returns 63/63. type ToolDefinition struct { Name string `json:"name"` + Title string `json:"title,omitempty"` Description string `json:"description"` InputSchema json.RawMessage `json:"inputSchema"` Annotations *ToolAnnotations `json:"annotations,omitempty"` } +// DisplayTitle returns the tool's human-readable title, preferring the +// top-level field and falling back to the legacy `annotations.title`. Empty +// when the server sends neither — which for a Pipeboard server means a real +// compliance gap (a title is a blocking Claude Connectors submission +// requirement), so callers should show its absence rather than paper over it +// with the machine name. +func (t ToolDefinition) DisplayTitle() string { + if t.Title != "" { + return t.Title + } + if t.Annotations != nil { + return t.Annotations.Title + } + return "" +} + // ToolsListResult is the result of tools/list. type ToolsListResult struct { Tools []ToolDefinition `json:"tools"`