fix(mcp): resolve tool descriptions/schemas consistently across all MCP tool-listing paths (#1369)

* fix(mcp): return bare tool names with descriptions for already-connected MCP servers, fixing empty descriptions/schemas in agent prompts * fix(mcp): resolve live tool descriptions/schemas in ListToolsForAgent for legacy prefixed tool_allow entries The previous fix (d612da8e) only corrected the admin "browse/select tools" endpoint. The actual runtime path building the LLM system prompt's tool list, Manager.ListToolsForAgent, still looked up tool_cache/registry entries keyed by the raw stored tool_allow name. Agents with grants captured before that fix have tool_allow persisted as registered (prefixed) names instead of bare MCP tool names, so every lookup missed and every tool in the prompt preview came back with an empty description and schema. Add bareMCPToolName to normalize both legacy-prefixed and current bare tool_allow/tool_deny entries before matching, and resolveMCPToolInfo to prefer the live registry's current description/schema (falling back to the settings tool_cache) instead of trusting whatever was persisted verbatim. Both the explicit-allow and cache-enumeration branches now share this single resolution path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-10-13 22:15:47 +00:00 · 2026-07-06 04:39:07 +07:00
1 parent ca097052b6
commit a3fe5f7858
@@ -18,9 +18,14 @@ import (
 	"github.com/nextlevelbuilder/goclaw/pkg/protocol"
 )

-// MCPToolLister returns discovered tool names for a specific MCP server.
+// MCPToolLister returns discovered tool names/info for a specific MCP server.
 type MCPToolLister interface {
 	ServerToolNames(serverName string) []string
+	// ServerToolInfos returns the original (bare, unprefixed) tool name and
+	// real description for each tool of an already-connected server. Bare
+	// names match the shape returned by mcp.DiscoverTools, so callers don't
+	// have to special-case "already connected" vs. "on-demand discovery".
+	ServerToolInfos(serverName string) []mcp.ToolInfo
 }

 // MCPPoolEvictor evicts pooled connections for a tenant+server (called on credential rotation).
@@ -89,14 +89,19 @@ func (h *MCPHandler) handleListServerTools(w http.ResponseWriter, r *http.Reques
 		return
 	}

-	// Try runtime Manager first — returns names only (no descriptions available).
+	// Try runtime Manager first — returns the original (bare, unprefixed) MCP
+	// tool names with their real descriptions, sourced from the live
+	// *BridgeTool for each registered tool. This must match the bare-name
+	// shape returned by the DiscoverTools fallback below: tool_allow grants
+	// saved from whichever shape this endpoint happens to return are matched
+	// against bare names by ListToolsForAgent's tool_cache lookups
+	// (internal/mcp/manager.go), so returning the registered/prefixed name
+	// here (as before) silently broke prompt-preview schemas for any server
+	// that was already live-connected when its grants were configured.
 	var tools []mcpbridge.ToolInfo
 	if h.mgr != nil {
-		if names := h.mgr.ServerToolNames(srv.Name); len(names) > 0 {
-			tools = make([]mcpbridge.ToolInfo, len(names))
-			for i, n := range names {
-				tools[i] = mcpbridge.ToolInfo{Name: n}
-			}
+		if infos := h.mgr.ServerToolInfos(srv.Name); len(infos) > 0 {
+			tools = infos
 		}
 	}

@@ -3,9 +3,12 @@ package mcp
 import (
 	"encoding/json"
 	"sort"
+	"sync/atomic"
 	"testing"

 	"github.com/google/uuid"
+	mcpclient "github.com/mark3labs/mcp-go/client"
+	mcpgo "github.com/mark3labs/mcp-go/mcp"

 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	"github.com/nextlevelbuilder/goclaw/internal/tools"
@@ -194,6 +197,119 @@ func TestListToolsForAgent_LegacyCacheShapeDegradesGracefully(t *testing.T) {
 	}
 }

+// TestListToolsForAgent_LegacyPrefixedToolAllowResolvesLiveDescriptions
+// reproduces the production regression: an agent's MCP grant has a persisted
+// tool_allow list containing OLD registered-name-shaped entries (e.g.
+// "mcp_goclaw__goclaw_agent_get" instead of the bare "goclaw_agent_get"),
+// captured before the grant-capture fix in mcp_tools.go. Before this fix,
+// ListToolsForAgent looked up tool_cache/registry entries keyed by the raw
+// stored name, which never matched the bare-name keys used elsewhere,
+// silently producing empty descriptions/schemas for every tool. This test
+// asserts descriptions and parameter schemas are still resolved correctly —
+// both from a live-registered tool and from the settings tool_cache — by
+// normalizing the legacy prefixed entries to bare names first.
+func TestListToolsForAgent_LegacyPrefixedToolAllowResolvesLiveDescriptions(t *testing.T) {
+	serverID := uuid.New()
+
+	// One tool ("goclaw_agent_get") is live-registered in the tool registry
+	// (simulating an already-connected self-hosted MCP server), and its
+	// description/schema must come from there, not from settings.
+	registry := tools.NewRegistry()
+	clientPtr := &atomic.Pointer[mcpclient.Client]{}
+	connected := &atomic.Bool{}
+	connected.Store(true)
+	liveTool := NewBridgeTool(
+		"goclaw",
+		mcpgo.Tool{
+			Name:        "goclaw_agent_get",
+			Description: "Fetch an agent by ID",
+			InputSchema: mcpgo.ToolInputSchema{
+				Type:       "object",
+				Properties: map[string]any{"agent_id": map[string]any{"type": "string"}},
+				Required:   []string{"agent_id"},
+			},
+		},
+		clientPtr, "", 0, connected, uuid.Nil, nil,
+	)
+	registry.Register(liveTool)
+
+	// The other tool ("goclaw_agent_list") has no live registration but does
+	// have a settings tool_cache entry — description must come from there.
+	toolCache := map[string]store.CachedToolInfo{
+		"goclaw_agent_list": {
+			Description: "List all agents",
+			Parameters:  json.RawMessage(`{"type":"object","properties":{}}`),
+		},
+	}
+	settings, err := json.Marshal(map[string]any{"tool_cache": toolCache})
+	if err != nil {
+		t.Fatalf("marshal settings: %v", err)
+	}
+
+	mockStore := &mockMCPStore{
+		accessible: []store.MCPAccessInfo{
+			{
+				Server: store.MCPServerData{
+					BaseModel: store.BaseModel{ID: serverID},
+					Name:      "goclaw",
+					Enabled:   true,
+					Settings:  settings,
+				},
+				// Legacy-persisted entries: already-prefixed registered names,
+				// exactly the shape a pre-fix grant capture would have stored.
+				ToolAllow: []string{
+					"mcp_goclaw__goclaw_agent_get",
+					"mcp_goclaw__goclaw_agent_list",
+				},
+				ToolDeny: nil,
+			},
+		},
+	}
+
+	mgr := NewManager(registry, WithStore(mockStore))
+
+	got, err := mgr.ListToolsForAgent(t.Context(), uuid.New(), "user-1")
+	if err != nil {
+		t.Fatalf("ListToolsForAgent: %v", err)
+	}
+	if len(got) != 2 {
+		t.Fatalf("expected 2 tool entries, got %d: %+v", len(got), got)
+	}
+
+	sort.Slice(got, func(i, j int) bool { return got[i].RegisteredName < got[j].RegisteredName })
+
+	// Live-registered tool: description/schema resolved from the registry.
+	live := got[0]
+	if live.RegisteredName != "mcp_goclaw__goclaw_agent_get" {
+		t.Fatalf("unexpected registered name: %q", live.RegisteredName)
+	}
+	if live.Description != "Fetch an agent by ID" {
+		t.Fatalf("expected live description resolved from registry, got %q (has_desc=%v)", live.Description, live.Description != "")
+	}
+	if len(live.Parameters) == 0 {
+		t.Fatal("expected live parameter schema resolved from registry, got empty")
+	}
+	var liveSchema map[string]any
+	if err := json.Unmarshal(live.Parameters, &liveSchema); err != nil {
+		t.Fatalf("unmarshal live parameters: %v", err)
+	}
+	if _, ok := liveSchema["properties"].(map[string]any)["agent_id"]; !ok {
+		t.Errorf("expected 'agent_id' property in live schema, got %+v", liveSchema)
+	}
+
+	// Cache-only tool: description/schema resolved from settings tool_cache.
+	cached := got[1]
+	if cached.RegisteredName != "mcp_goclaw__goclaw_agent_list" {
+		t.Fatalf("unexpected registered name: %q", cached.RegisteredName)
+	}
+	if cached.Description != "List all agents" {
+		t.Fatalf("expected cached description resolved despite legacy prefixed tool_allow entry, got %q", cached.Description)
+	}
+	if len(cached.Parameters) == 0 {
+		t.Fatal("expected cached parameter schema resolved despite legacy prefixed tool_allow entry, got empty")
+	}
+}
+
 // TestListToolsForAgent_EmptyToolAllowNoCacheFallsBackToPlaceholder verifies
 // that when a server has never been connected (no tool_cache present), the
 // existing single-placeholder behavior is preserved.
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -674,6 +675,67 @@ type MCPToolPreviewInfo struct {
 	Parameters json.RawMessage
 }

+// mcpPreviewDiscoveryTimeout bounds the on-demand tool discovery
+// ListToolsForAgent performs when a server has never been live-connected for
+// this agent (empty tool_cache and empty registry). Kept short since it's on
+// the hot path of a prompt-preview HTTP request, unlike the longer
+// discoverToolsTimeout used by the dedicated admin "test connection"/"browse
+// tools" endpoints.
+const mcpPreviewDiscoveryTimeout = 5 * time.Second
+
+// bareMCPToolName strips a persisted "{effectivePrefix}__" prefix from a
+// stored tool_allow/tool_deny entry, if present.
+//
+// Historically, some agent grants were captured while their MCP server was
+// already live-connected and ended up storing the registered (prefixed) tool
+// name instead of the bare original MCP tool name — see ServerToolInfos'
+// doc comment for how that mismatch happened. Those legacy rows are never
+// rewritten automatically, so ListToolsForAgent must accept both shapes
+// going forward: bare names (the current, correct shape) and prefixed names
+// (already-persisted legacy grants), normalizing to bare before matching
+// against tool_cache keys or the live tool registry.
+func bareMCPToolName(stored, effectivePrefix string) string {
+	if bare, ok := strings.CutPrefix(stored, effectivePrefix+"__"); ok {
+		return bare
+	}
+	return stored
+}
+
+// resolveMCPToolInfo resolves the description and parameter schema for a bare
+// MCP tool name, preferring the CURRENT live registry entry (if the server is
+// connected right now) over the settings-persisted tool_cache snapshot, which
+// can be stale or entirely absent. Admin-authored hints always take priority
+// over both sources; the server's global hint is the last-resort fallback.
+func (m *Manager) resolveMCPToolInfo(toolName, effectivePrefix string, hints ToolHints, toolCache map[string]store.CachedToolInfo) (string, json.RawMessage) {
+	desc := hints.HintFor(toolName)
+	var params json.RawMessage
+
+	if tool, ok := m.registry.Get(effectivePrefix + "__" + toolName); ok {
+		if bridgeTool, isBridge := tool.(*BridgeTool); isBridge {
+			if desc == "" {
+				desc = bridgeTool.Description()
+			}
+			if schema := bridgeTool.Parameters(); schema != nil {
+				if schemaJSON, err := json.Marshal(schema); err == nil {
+					params = schemaJSON
+				}
+			}
+		}
+	}
+
+	cached := toolCache[toolName]
+	if desc == "" {
+		desc = cached.Description
+	}
+	if params == nil {
+		params = cached.Parameters
+	}
+	if desc == "" && hints.Global != "" {
+		desc = hints.Global
+	}
+	return desc, params
+}
+
 // ListToolsForAgent returns a best-effort list of MCP tool names and descriptions
 // for a given agent+user based on store configuration only — no actual MCP
 // server connections are made. It is intended for prompt preview.
@@ -737,10 +799,42 @@ func (m *Manager) ListToolsForAgent(ctx context.Context, agentID uuid.UUID, user
 			}
 		}

-		// Build deny set
+		// Nothing persisted yet and no live registry entries either (the
+		// server has never actually been connected for this agent, e.g. a
+		// freshly added MCP server before the agent's first real chat turn
+		// triggers Manager.LoadForAgent). ListToolsForAgent is otherwise
+		// documented as "no actual MCP server connections are made", which
+		// left the prompt preview permanently showing empty descriptions and
+		// "{type: object}" schemas until a real session happened to connect.
+		// Do a single bounded on-demand discovery here (same live path the
+		// admin "browse tools" endpoint already uses, see handleListServerTools
+		// in internal/http/mcp_tools.go) so the preview reflects real tool
+		// data immediately, and persist it so subsequent calls hit the cache.
+		if len(toolCache) == 0 && len(m.ServerToolInfos(info.Server.Name)) == 0 {
+			if rs := m.resolveServerCredentials(ctx, info, userID); rs != nil {
+				discovered, err := discoverRawTools(ctx, info.Server.Transport, info.Server.Command, rs.args, rs.env, info.Server.URL, rs.headers, mcpPreviewDiscoveryTimeout)
+				if err != nil {
+					slog.Debug("mcp.ListToolsForAgent.on_demand_discovery_failed", "server", info.Server.Name, "error", err)
+				} else {
+					toolCache = buildCachedToolInfo(discovered)
+					if info.Server.ID != uuid.Nil && m.store != nil && len(toolCache) > 0 {
+						go func(sid uuid.UUID, cache map[string]store.CachedToolInfo) {
+							cacheCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+							defer cancel()
+							if cacheErr := m.store.CacheToolDescriptions(cacheCtx, sid, cache); cacheErr != nil {
+								slog.Debug("mcp.ListToolsForAgent.cache_tool_descriptions_failed", "server_id", sid, "error", cacheErr)
+							}
+						}(info.Server.ID, toolCache)
+					}
+				}
+			}
+		}
+
+		// Build deny set, normalizing legacy prefixed entries to bare names
+		// (see bareMCPToolName) so they match tool_cache/registry lookups.
 		denySet := make(map[string]struct{}, len(info.ToolDeny))
 		for _, d := range info.ToolDeny {
-			denySet[d] = struct{}{}
+			denySet[bareMCPToolName(d, effectivePrefix)] = struct{}{}
 		}

 		if len(info.ToolAllow) == 0 {
@@ -754,19 +848,12 @@ func (m *Manager) ListToolsForAgent(ctx context.Context, agentID uuid.UUID, user
 						continue
 					}
 					registeredName := effectivePrefix + "__" + toolName
-					cached := toolCache[toolName]
-					desc := hints.HintFor(toolName)
-					if desc == "" {
-						desc = cached.Description
-					}
-					if desc == "" && hints.Global != "" {
-						desc = hints.Global
-					}
+					desc, params := m.resolveMCPToolInfo(toolName, effectivePrefix, hints, toolCache)
 					serverTools = append(serverTools, registeredName)
 					result = append(result, MCPToolPreviewInfo{
 						RegisteredName: registeredName,
 						Description:    desc,
-						Parameters:     cached.Parameters,
+						Parameters:     params,
 					})
 				}
 				slog.Debug("mcp.ListToolsForAgent.server_tools_added_from_cache", "server", info.Server.Name, "tools", serverTools)
@@ -788,25 +875,22 @@ func (m *Manager) ListToolsForAgent(ctx context.Context, agentID uuid.UUID, user
 		}

 		var serverTools []string
-		for _, toolName := range info.ToolAllow {
+		for _, storedName := range info.ToolAllow {
+			// Normalize legacy prefixed entries (persisted before the grant-capture
+			// fix in mcp_tools.go) to the bare tool name so they resolve against
+			// tool_cache and the live registry the same as current bare entries.
+			toolName := bareMCPToolName(storedName, effectivePrefix)
 			if _, denied := denySet[toolName]; denied {
 				slog.Debug("mcp.ListToolsForAgent.tool_denied", "server", info.Server.Name, "tool", toolName)
 				continue
 			}
 			registeredName := effectivePrefix + "__" + toolName
-			cached := toolCache[toolName]
-			desc := hints.HintFor(toolName)
-			if desc == "" {
-				desc = cached.Description
-			}
-			if desc == "" && hints.Global != "" {
-				desc = hints.Global
-			}
+			desc, params := m.resolveMCPToolInfo(toolName, effectivePrefix, hints, toolCache)
 			serverTools = append(serverTools, registeredName)
 			result = append(result, MCPToolPreviewInfo{
 				RegisteredName: registeredName,
 				Description:    desc,
-				Parameters:     cached.Parameters,
+				Parameters:     params,
 			})
 		}
 		slog.Debug("mcp.ListToolsForAgent.server_tools_added", "server", info.Server.Name, "tools", serverTools)
@@ -47,6 +47,57 @@ func (m *Manager) ServerToolNames(serverName string) []string {
 	return nil
 }

+// ServerToolInfos returns ToolInfo (original/bare tool name + real description)
+// for a server that is already live-connected in this Manager, by looking up
+// each registered (prefixed) tool name in the registry and reading its
+// original MCP name and description off the live *BridgeTool.
+//
+// This exists so callers needing tool metadata for an already-connected
+// server (e.g. handleListServerTools) get the SAME bare-name shape as the
+// on-demand DiscoverTools fallback (manager_tools.go DiscoverTools), instead
+// of the registered/prefixed names returned by ServerToolNames. Mixing the
+// two shapes previously caused a server's tool_allow grant (built from
+// whichever name shape the UI happened to see, depending on whether the
+// server was already connected) to end up keyed by a prefixed name that
+// never matches the bare-name keys used by buildCachedToolInfo
+// (manager_connect.go) and ListToolsForAgent's tool_cache lookups
+// (manager.go), silently producing empty description/parameters in the
+// prompt preview for tools whose grant was captured while the server was
+// live-connected — most commonly a server (like goclaw's own CRUD server)
+// that self-connects and therefore tends to already be live by the time an
+// operator configures its grants.
+func (m *Manager) ServerToolInfos(serverName string) []ToolInfo {
+	m.mu.RLock()
+	var registeredNames []string
+	if _, isPool := m.poolServers[serverName]; isPool {
+		registeredNames = m.poolToolNames[serverName]
+	} else if ss, ok := m.servers[serverName]; ok {
+		registeredNames = ss.toolNames
+	}
+	m.mu.RUnlock()
+
+	if len(registeredNames) == 0 {
+		return nil
+	}
+
+	infos := make([]ToolInfo, 0, len(registeredNames))
+	for _, registeredName := range registeredNames {
+		tool, ok := m.registry.Get(registeredName)
+		if !ok {
+			continue
+		}
+		bridgeTool, ok := tool.(*BridgeTool)
+		if !ok {
+			continue
+		}
+		infos = append(infos, ToolInfo{
+			Name:        bridgeTool.OriginalName(),
+			Description: bridgeTool.Description(),
+		})
+	}
+	return infos
+}
+
 // updateMCPGroup rebuilds the "mcp" group with all MCP tool names across servers.
 // Must be called with m.mu NOT held (it acquires RLock).
 func (m *Manager) updateMCPGroup() {
@@ -116,11 +167,33 @@ type ToolInfo struct {
 // DiscoverTools connects temporarily to an MCP server, lists its tools, and disconnects.
 // Used for on-demand discovery when no persistent Manager connection exists (DB-backed servers).
 func DiscoverTools(ctx context.Context, transportType, command string, args []string, env map[string]string, url string, headers map[string]string) ([]ToolInfo, error) {
+	mcpTools, err := discoverRawTools(ctx, transportType, command, args, env, url, headers, discoverToolsTimeout)
+	if err != nil {
+		return nil, err
+	}
+
+	result := make([]ToolInfo, 0, len(mcpTools))
+	for _, t := range mcpTools {
+		result = append(result, ToolInfo{Name: t.Name, Description: t.Description})
+	}
+	return result, nil
+}
+
+// discoverToolsTimeout bounds a full DiscoverTools/discoverRawTools round trip
+// (connect + initialize + list tools) against a remote MCP server.
+const discoverToolsTimeout = 15 * time.Second
+
+// discoverRawTools connects temporarily to an MCP server, lists its tools with
+// their full mcpgo.Tool definitions (including input schema), and disconnects.
+// Shared by DiscoverTools (name+description only, for the admin browse UI) and
+// the ListToolsForAgent on-demand fallback (full schema, for prompt preview
+// caching), so both callers see identical live tool data.
+func discoverRawTools(ctx context.Context, transportType, command string, args []string, env map[string]string, url string, headers map[string]string, timeout time.Duration) ([]mcpgo.Tool, error) {
 	if err := ValidateServerConfig(transportType, command, args, url); err != nil {
 		return nil, fmt.Errorf("invalid MCP server config: %w", err)
 	}

-	ctx, cancel := context.WithTimeout(ctx, 15*time.Second)
+	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()

 	client, err := createClient(transportType, command, args, env, url, headers)
@@ -147,9 +220,5 @@ func DiscoverTools(ctx context.Context, transportType, command string, args []st
 		return nil, fmt.Errorf("list tools: %w", err)
 	}

-	result := make([]ToolInfo, 0, len(toolsResult.Tools))
-	for _, t := range toolsResult.Tools {
-		result = append(result, ToolInfo{Name: t.Name, Description: t.Description})
-	}
-	return result, nil
+	return toolsResult.Tools, nil
 }
@@ -0,0 +1,125 @@
+package mcp
+
+import (
+	"context"
+	"encoding/json"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/google/uuid"
+	mcpgo "github.com/mark3labs/mcp-go/mcp"
+	mcpserver "github.com/mark3labs/mcp-go/server"
+
+	"github.com/nextlevelbuilder/goclaw/internal/store"
+	"github.com/nextlevelbuilder/goclaw/internal/tools"
+)
+
+// TestSelfConnectedServer_CacheRetainsDescriptionAndSchema is a regression
+// test for a reported bug where connecting to an already-connected MCP
+// server as a client showed tools with an empty description and empty
+// parameters in the system-prompt preview (`- function: {description: '',
+// parameters: {type: object}}`), while newly-discovered tools came through
+// with full schema.
+//
+// It builds a minimal generic MCP server directly with the underlying
+// mark3labs/mcp-go library (no dependency on any goclaw-specific server such
+// as the CRUD server), serves it over streamable-http via httptest
+// (mirroring a goclaw gateway connecting to an MCP endpoint), connects to it
+// with the exact client path used by the Manager (connectAndDiscover), and
+// proves the resulting tool cache (buildCachedToolInfo) — and the full
+// downstream pipeline through ListToolsForAgent with a bare-name tool_allow
+// grant — retains the real description and JSON Schema.
+func TestSelfConnectedServer_CacheRetainsDescriptionAndSchema(t *testing.T) {
+	srv := mcpserver.NewMCPServer("test-server", "1.0.0", mcpserver.WithToolCapabilities(false))
+
+	tool := mcpgo.NewTool("test_tool",
+		mcpgo.WithDescription("A simple test tool used to verify description/schema propagation."),
+		mcpgo.WithString("param",
+			mcpgo.Description("An example string parameter."),
+			mcpgo.Required(),
+		),
+	)
+	srv.AddTool(tool, func(_ context.Context, _ mcpgo.CallToolRequest) (*mcpgo.CallToolResult, error) {
+		return mcpgo.NewToolResultText("ok"), nil
+	})
+
+	httpSrv := mcpserver.NewStreamableHTTPServer(srv)
+	ts := httptest.NewServer(httpSrv)
+	defer ts.Close()
+
+	// "goclaw" is the same ClientInfo.Name used by connectAndDiscover in
+	// production (manager_connect.go).
+	ss, mcpTools, err := connectAndDiscover(context.Background(), "goclaw", "streamable-http", "", nil, nil, ts.URL, nil, 10)
+	if err != nil {
+		t.Fatalf("connectAndDiscover: %v", err)
+	}
+	defer func() { _ = ss.client.Close() }()
+
+	cache := buildCachedToolInfo(mcpTools)
+	entry, ok := cache["test_tool"]
+	if !ok {
+		t.Fatalf("expected test_tool in cache, got %+v", cache)
+	}
+	if entry.Description == "" {
+		t.Fatal("expected non-empty description for test_tool, got empty (regression)")
+	}
+	if len(entry.Parameters) == 0 {
+		t.Fatal("expected non-empty parameters schema for test_tool, got empty (regression)")
+	}
+
+	var schema map[string]any
+	if err := json.Unmarshal(entry.Parameters, &schema); err != nil {
+		t.Fatalf("unmarshal cached parameters: %v", err)
+	}
+	props, ok := schema["properties"].(map[string]any)
+	if !ok || len(props) == 0 {
+		t.Fatalf("expected non-empty properties in cached schema, got %+v", schema)
+	}
+	if _, ok := props["param"]; !ok {
+		t.Fatalf("expected 'param' property in cached schema, got %+v", props)
+	}
+
+	// End-to-end: feed the cache into ListToolsForAgent exactly as it is
+	// persisted in production (settings.tool_cache), with an explicit
+	// tool_allow grant using the BARE tool name — the shape ListToolsForAgent
+	// expects (internal/mcp/manager.go) and the shape ServerToolInfos returns
+	// (internal/mcp/manager_tools.go).
+	settings, err := json.Marshal(map[string]any{"tool_cache": cache})
+	if err != nil {
+		t.Fatalf("marshal settings: %v", err)
+	}
+	serverID := uuid.New()
+	mockStore := &mockMCPStore{
+		accessible: []store.MCPAccessInfo{
+			{
+				Server: store.MCPServerData{
+					BaseModel: store.BaseModel{ID: serverID},
+					Name:      "goclaw",
+					Enabled:   true,
+					Settings:  settings,
+				},
+				ToolAllow: []string{"test_tool"},
+			},
+		},
+	}
+	mgr := NewManager(tools.NewRegistry(), WithStore(mockStore))
+	previews, err := mgr.ListToolsForAgent(t.Context(), uuid.New(), "user-1")
+	if err != nil {
+		t.Fatalf("ListToolsForAgent: %v", err)
+	}
+	var found *MCPToolPreviewInfo
+	for i := range previews {
+		if previews[i].RegisteredName == "mcp_goclaw__test_tool" {
+			found = &previews[i]
+		}
+	}
+	if found == nil {
+		t.Fatalf("expected mcp_goclaw__test_tool in preview, got %+v", previews)
+	}
+	if found.Description == "" {
+		t.Fatal("preview: expected non-empty description (regression)")
+	}
+	if len(found.Parameters) == 0 {
+		t.Fatal("preview: expected non-empty parameters (regression)")
+	}
+}