Merge pull request #2535 from afjcjsbx/feat/mcp-channel-commands

feat(commands): add MCP slash commands and tool details
2026-06-12 18:08:54 +00:00 · 2026-04-22 14:54:28 +02:00
parent f367a9c010 175682f152
commit 68ceb54b36
12 changed files with 570 additions and 4 deletions
@@ -4,11 +4,15 @@ package agent

 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"sort"
 	"strings"

 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/commands"
+	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/logger"
 	"github.com/sipeed/picoclaw/pkg/providers"
 )

@@ -133,6 +137,120 @@ func (al *AgentLoop) buildCommandsRuntime(
 		Config:          cfg,
 		ListAgentIDs:    registry.ListAgentIDs,
 		ListDefinitions: al.cmdRegistry.Definitions,
+		ListMCPServers: func(ctx context.Context) []commands.MCPServerInfo {
+			if cfg == nil {
+				return nil
+			}
+
+			if len(cfg.Tools.MCP.Servers) == 0 {
+				return nil
+			}
+
+			if err := al.ensureMCPInitialized(ctx); err != nil {
+				logger.WarnCF("agent", "Failed to refresh MCP status for command",
+					map[string]any{
+						"error": err.Error(),
+					})
+			}
+
+			connected := make(map[string]int)
+			if manager := al.mcp.getManager(); manager != nil {
+				for serverName, conn := range manager.GetServers() {
+					connected[serverName] = len(conn.Tools)
+				}
+			}
+
+			servers := make([]commands.MCPServerInfo, 0, len(cfg.Tools.MCP.Servers))
+			for serverName, serverCfg := range cfg.Tools.MCP.Servers {
+				toolCount, isConnected := connected[serverName]
+				servers = append(servers, commands.MCPServerInfo{
+					Name:      serverName,
+					Enabled:   serverCfg.Enabled,
+					Deferred:  serverIsDeferred(cfg.Tools.MCP.Discovery.Enabled, serverCfg),
+					Connected: isConnected,
+					ToolCount: toolCount,
+				})
+			}
+
+			sort.Slice(servers, func(i, j int) bool {
+				return strings.ToLower(servers[i].Name) < strings.ToLower(servers[j].Name)
+			})
+
+			return servers
+		},
+		ListMCPTools: func(ctx context.Context, serverName string) ([]commands.MCPToolInfo, error) {
+			if cfg == nil {
+				return nil, fmt.Errorf("command unavailable: config not loaded")
+			}
+
+			serverName = strings.TrimSpace(serverName)
+			if serverName == "" {
+				return nil, fmt.Errorf("server name is required")
+			}
+
+			resolvedName := ""
+			var serverCfg config.MCPServerConfig
+			for name, candidate := range cfg.Tools.MCP.Servers {
+				if strings.EqualFold(name, serverName) {
+					resolvedName = name
+					serverCfg = candidate
+					break
+				}
+			}
+			if resolvedName == "" {
+				return nil, fmt.Errorf("MCP server '%s' is not configured", serverName)
+			}
+			if !serverCfg.Enabled {
+				return nil, fmt.Errorf("MCP server '%s' is configured but disabled", resolvedName)
+			}
+			if !cfg.Tools.IsToolEnabled("mcp") {
+				return nil, fmt.Errorf("MCP integration is disabled")
+			}
+
+			if err := al.ensureMCPInitialized(ctx); err != nil {
+				logger.WarnCF("agent", "Failed to initialize MCP runtime for command",
+					map[string]any{
+						"server": resolvedName,
+						"error":  err.Error(),
+					})
+			}
+
+			manager := al.mcp.getManager()
+			if manager == nil {
+				return nil, fmt.Errorf("MCP server '%s' is configured but not connected", resolvedName)
+			}
+
+			conn, ok := manager.GetServer(resolvedName)
+			if !ok {
+				return nil, fmt.Errorf("MCP server '%s' is configured but not connected", resolvedName)
+			}
+
+			toolInfos := make([]commands.MCPToolInfo, 0, len(conn.Tools))
+			for _, tool := range conn.Tools {
+				if tool == nil {
+					continue
+				}
+				name := strings.TrimSpace(tool.Name)
+				if name == "" {
+					continue
+				}
+
+				description := strings.TrimSpace(tool.Description)
+				if description == "" {
+					description = fmt.Sprintf("MCP tool from %s server", resolvedName)
+				}
+
+				toolInfos = append(toolInfos, commands.MCPToolInfo{
+					Name:        name,
+					Description: description,
+					Parameters:  summarizeMCPToolParameters(tool.InputSchema),
+				})
+			}
+			sort.Slice(toolInfos, func(i, j int) bool {
+				return toolInfos[i].Name < toolInfos[j].Name
+			})
+			return toolInfos, nil
+		},
 		GetEnabledChannels: func() []string {
 			if al.channelManager == nil {
 				return nil
@@ -236,6 +354,96 @@ func (al *AgentLoop) buildCommandsRuntime(
 	return rt
 }

+func summarizeMCPToolParameters(schema any) []commands.MCPToolParameterInfo {
+	schemaMap := normalizeMCPSchema(schema)
+	properties, ok := schemaMap["properties"].(map[string]any)
+	if !ok || len(properties) == 0 {
+		return nil
+	}
+
+	required := make(map[string]struct{})
+	switch raw := schemaMap["required"].(type) {
+	case []string:
+		for _, name := range raw {
+			required[name] = struct{}{}
+		}
+	case []any:
+		for _, value := range raw {
+			name, ok := value.(string)
+			if ok {
+				required[name] = struct{}{}
+			}
+		}
+	}
+
+	names := make([]string, 0, len(properties))
+	for name := range properties {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	params := make([]commands.MCPToolParameterInfo, 0, len(names))
+	for _, name := range names {
+		param := commands.MCPToolParameterInfo{Name: name}
+		if propMap, ok := properties[name].(map[string]any); ok {
+			if typeName, ok := propMap["type"].(string); ok {
+				param.Type = strings.TrimSpace(typeName)
+			}
+			if desc, ok := propMap["description"].(string); ok {
+				param.Description = strings.TrimSpace(desc)
+			}
+		}
+		_, param.Required = required[name]
+		params = append(params, param)
+	}
+	return params
+}
+
+func normalizeMCPSchema(schema any) map[string]any {
+	if schema == nil {
+		return map[string]any{
+			"type":       "object",
+			"properties": map[string]any{},
+			"required":   []string{},
+		}
+	}
+
+	if schemaMap, ok := schema.(map[string]any); ok {
+		return schemaMap
+	}
+
+	var jsonData []byte
+	switch raw := schema.(type) {
+	case json.RawMessage:
+		jsonData = raw
+	case []byte:
+		jsonData = raw
+	}
+
+	if jsonData == nil {
+		var err error
+		jsonData, err = json.Marshal(schema)
+		if err != nil {
+			return map[string]any{
+				"type":       "object",
+				"properties": map[string]any{},
+				"required":   []string{},
+			}
+		}
+	}
+
+	var result map[string]any
+	if err := json.Unmarshal(jsonData, &result); err != nil {
+		return map[string]any{
+			"type":       "object",
+			"properties": map[string]any{},
+			"required":   []string{},
+		}
+	}
+
+	return result
+}
+
 func (al *AgentLoop) setPendingSkills(sessionKey string, skillNames []string) {
 	sessionKey = strings.TrimSpace(sessionKey)
 	if sessionKey == "" || len(skillNames) == 0 {
@@ -67,6 +67,12 @@ func (r *mcpRuntime) hasManager() bool {
 	return r.manager != nil
 }

+func (r *mcpRuntime) getManager() *mcp.Manager {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return r.manager
+}
+
 // ensureMCPInitialized loads MCP servers/tools once so both Run() and direct
 // agent mode share the same initialization path.
 func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error {
@@ -100,6 +106,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error {
 		}

 		if err := mcpManager.LoadFromMCPConfig(ctx, al.cfg.Tools.MCP, workspacePath); err != nil {
+			al.mcp.setInitErr(fmt.Errorf("failed to load MCP servers: %w", err))
 			logger.WarnCF("agent", "Failed to load MCP servers, MCP tools will not be available",
 				map[string]any{
 					"error": err.Error(),
@@ -9,6 +9,7 @@ package agent
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"

 	"github.com/sipeed/picoclaw/pkg/config"
@@ -133,3 +134,48 @@ func TestServerIsDeferred(t *testing.T) {
 		})
 	}
 }
+
+func TestEnsureMCPInitialized_LoadFailureSetsInitErr(t *testing.T) {
+	al, cfg, _, _, cleanup := newTestAgentLoop(t)
+	defer cleanup()
+	defer al.Close()
+
+	cfg.Tools = config.ToolsConfig{
+		MCP: config.MCPConfig{
+			ToolConfig: config.ToolConfig{Enabled: true},
+			Servers: map[string]config.MCPServerConfig{
+				"broken": {
+					Enabled: true,
+					Command: "picoclaw-command-that-does-not-exist-for-mcp-tests",
+				},
+			},
+		},
+	}
+
+	err := al.ensureMCPInitialized(context.Background())
+	if err == nil {
+		t.Fatal("ensureMCPInitialized() error = nil, want load failure")
+	}
+	if !strings.Contains(err.Error(), "failed to load MCP servers") {
+		t.Fatalf("ensureMCPInitialized() error = %q, want wrapped load failure", err.Error())
+	}
+
+	initErr := al.mcp.getInitErr()
+	if initErr == nil {
+		t.Fatal("getInitErr() = nil, want cached load failure")
+	}
+	if !strings.Contains(initErr.Error(), "failed to load MCP servers") {
+		t.Fatalf("getInitErr() = %q, want wrapped load failure", initErr.Error())
+	}
+	if al.mcp.getManager() != nil {
+		t.Fatal("expected MCP manager to remain nil after load failure")
+	}
+
+	err = al.ensureMCPInitialized(context.Background())
+	if err == nil {
+		t.Fatal("second ensureMCPInitialized() error = nil, want cached load failure")
+	}
+	if !strings.Contains(err.Error(), "failed to load MCP servers") {
+		t.Fatalf("second ensureMCPInitialized() error = %q, want wrapped load failure", err.Error())
+	}
+}
@@ -2272,6 +2272,75 @@ func TestProcessMessage_CommandOutcomes(t *testing.T) {
 	}
 }

+func TestProcessMessage_MCPCommandsHandledWithoutLLMCall(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	deferred := true
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				ModelName:         "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+		Session: config.SessionConfig{
+			Dimensions: []string{"chat"},
+		},
+		Tools: config.ToolsConfig{
+			MCP: config.MCPConfig{
+				ToolConfig: config.ToolConfig{Enabled: true},
+				Discovery:  config.ToolDiscoveryConfig{Enabled: true},
+				Servers: map[string]config.MCPServerConfig{
+					"github": {
+						Enabled:  true,
+						Deferred: &deferred,
+					},
+				},
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &countingMockProvider{response: "LLM reply"}
+	al := NewAgentLoop(cfg, msgBus, provider)
+	helper := testHelper{al: al}
+
+	baseContext := bus.InboundContext{
+		Channel:  "whatsapp",
+		ChatID:   "chat1",
+		ChatType: "direct",
+		SenderID: "user1",
+	}
+
+	listResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{
+		Context: baseContext,
+		Content: "/list mcp",
+	})
+	if !strings.Contains(listResp, "- `github`") || !strings.Contains(listResp, "Deferred: yes") {
+		t.Fatalf("unexpected /list mcp reply: %q", listResp)
+	}
+	if provider.calls != 0 {
+		t.Fatalf("LLM should not be called for /list mcp, calls=%d", provider.calls)
+	}
+
+	showResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{
+		Context: baseContext,
+		Content: "/show mcp github",
+	})
+	if showResp != "MCP server 'github' is configured but not connected" {
+		t.Fatalf("unexpected /show mcp reply: %q", showResp)
+	}
+	if provider.calls != 0 {
+		t.Fatalf("LLM should not be called for /show mcp, calls=%d", provider.calls)
+	}
+}
+
 func TestProcessMessage_SwitchModelShowModelConsistency(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "agent-test-*")
 	if err != nil {