diff --git a/Makefile b/Makefile index 87b0ef8d0..afc76a6ad 100644 --- a/Makefile +++ b/Makefile @@ -204,6 +204,44 @@ check: deps fmt vet test run: build @$(BUILD_DIR)/$(BINARY_NAME) $(ARGS) +## docker-build: Build Docker image (minimal Alpine-based) +docker-build: + @echo "Building minimal Docker image (Alpine-based)..." + docker compose -f docker/docker-compose.yml build picoclaw-agent picoclaw-gateway + +## docker-build-full: Build Docker image with full MCP support (Node.js 24) +docker-build-full: + @echo "Building full-featured Docker image (Node.js 24)..." + docker compose -f docker/docker-compose.full.yml build picoclaw-agent picoclaw-gateway + +## docker-test: Test MCP tools in Docker container +docker-test: + @echo "Testing MCP tools in Docker..." + @chmod +x scripts/test-docker-mcp.sh + @./scripts/test-docker-mcp.sh + +## docker-run: Run picoclaw gateway in Docker (Alpine-based) +docker-run: + docker compose -f docker/docker-compose.yml --profile gateway up + +## docker-run-full: Run picoclaw gateway in Docker (full-featured) +docker-run-full: + docker compose -f docker/docker-compose.full.yml --profile gateway up + +## docker-run-agent: Run picoclaw agent in Docker (interactive, Alpine-based) +docker-run-agent: + docker compose -f docker/docker-compose.yml run --rm picoclaw-agent + +## docker-run-agent-full: Run picoclaw agent in Docker (interactive, full-featured) +docker-run-agent-full: + docker compose -f docker/docker-compose.full.yml run --rm picoclaw-agent + +## docker-clean: Clean Docker images and volumes +docker-clean: + docker compose -f docker/docker-compose.yml down -v + docker compose -f docker/docker-compose.full.yml down -v + docker rmi picoclaw:latest picoclaw:full 2>/dev/null || true + ## help: Show this help message help: @echo "picoclaw Makefile" @@ -219,6 +257,8 @@ help: @echo " make install # Install to ~/.local/bin" @echo " make uninstall # Remove from /usr/local/bin" @echo " make install-skills # Install skills to workspace" + @echo " make docker-build # Build minimal Docker image" + @echo " make docker-test # Test MCP tools in Docker" @echo "" @echo "Environment Variables:" @echo " INSTALL_PREFIX # Installation prefix (default: ~/.local)" diff --git a/config/config.example.json b/config/config.example.json index e292731b9..0c4991a49 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -243,6 +243,63 @@ "cron": { "exec_timeout_minutes": 5 }, + "mcp": { + "enabled": false, + "servers": { + "filesystem": { + "enabled": false, + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-filesystem", + "/tmp" + ] + }, + "github": { + "enabled": false, + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-github" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "YOUR_GITHUB_TOKEN" + } + }, + "brave-search": { + "enabled": false, + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-brave-search" + ], + "env": { + "BRAVE_API_KEY": "YOUR_BRAVE_API_KEY" + } + }, + "postgres": { + "enabled": false, + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-postgres", + "postgresql://user:password@localhost/dbname" + ] + }, + "slack": { + "enabled": false, + "command": "npx", + "args": [ + "-y", + "@modelcontextprotocol/server-slack" + ], + "env": { + "SLACK_BOT_TOKEN": "YOUR_SLACK_BOT_TOKEN", + "SLACK_TEAM_ID": "YOUR_SLACK_TEAM_ID" + } + } + } + }, "exec": { "enable_deny_patterns": false, "custom_deny_patterns": [] @@ -271,4 +328,4 @@ "host": "127.0.0.1", "port": 18790 } -} +} \ No newline at end of file diff --git a/docker/Dockerfile.full b/docker/Dockerfile.full new file mode 100644 index 000000000..30e1680d5 --- /dev/null +++ b/docker/Dockerfile.full @@ -0,0 +1,44 @@ +# ============================================================ +# Stage 1: Build the picoclaw binary +# ============================================================ +FROM golang:1.26.0-alpine AS builder + +RUN apk add --no-cache git make + +WORKDIR /src + +# Cache dependencies +COPY go.mod go.sum ./ +RUN go mod download + +# Copy source and build +COPY . . +RUN make build + +# ============================================================ +# Stage 2: Node.js-based runtime with full MCP support +# ============================================================ +FROM node:24-alpine3.23 + +# Install runtime dependencies +RUN apk add --no-cache \ + ca-certificates \ + curl \ + git \ + python3 \ + py3-pip + +# Install uv and symlink to system path +RUN curl -LsSf https://astral.sh/uv/install.sh | sh && \ + ln -s /root/.local/bin/uv /usr/local/bin/uv && \ + ln -s /root/.local/bin/uvx /usr/local/bin/uvx && \ + uv --version + +# Copy binary +COPY --from=builder /src/build/picoclaw /usr/local/bin/picoclaw + +# Create picoclaw home directory +RUN /usr/local/bin/picoclaw onboard + +ENTRYPOINT ["picoclaw"] +CMD ["gateway"] diff --git a/docker/docker-compose.full.yml b/docker/docker-compose.full.yml new file mode 100644 index 000000000..6f34448c4 --- /dev/null +++ b/docker/docker-compose.full.yml @@ -0,0 +1,44 @@ +services: + # ───────────────────────────────────────────── + # PicoClaw Agent (one-shot query) - Full MCP Support + # docker compose -f docker/docker-compose.full.yml run --rm picoclaw-agent -m "Hello" + # ───────────────────────────────────────────── + picoclaw-agent: + build: + context: .. + dockerfile: docker/Dockerfile.full + container_name: picoclaw-agent-full + profiles: + - agent + volumes: + - ../config/config.json:/root/.picoclaw/config.json:ro + - picoclaw-workspace:/root/.picoclaw/workspace + - picoclaw-npm-cache:/root/.npm # npm cache for faster MCP server installs + entrypoint: ["picoclaw", "agent"] + stdin_open: true + tty: true + + # ───────────────────────────────────────────── + # PicoClaw Gateway (Long-running Bot) - Full MCP Support + # docker compose -f docker/docker-compose.full.yml --profile gateway up + # ───────────────────────────────────────────── + picoclaw-gateway: + build: + context: .. + dockerfile: docker/Dockerfile.full + container_name: picoclaw-gateway-full + restart: unless-stopped + profiles: + - gateway + volumes: + # Configuration file + - ../config/config.json:/root/.picoclaw/config.json:ro + # Persistent workspace (sessions, memory, logs) + - picoclaw-workspace:/root/.picoclaw/workspace + # NPM cache for faster MCP server installs + - picoclaw-npm-cache:/root/.npm + command: ["gateway"] + +volumes: + picoclaw-workspace: + picoclaw-npm-cache: # Cache npm packages to speed up MCP server installations diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md index 8aba1aa91..6204fb0c8 100644 --- a/docs/tools_configuration.md +++ b/docs/tools_configuration.md @@ -8,6 +8,7 @@ PicoClaw's tools configuration is located in the `tools` field of `config.json`. { "tools": { "web": { ... }, + "mcp": { ... }, "exec": { ... }, "cron": { ... }, "skills": { ... } @@ -21,35 +22,35 @@ Web tools are used for web search and fetching. ### Brave -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `enabled` | bool | false | Enable Brave search | -| `api_key` | string | - | Brave Search API key | -| `max_results` | int | 5 | Maximum number of results | +| Config | Type | Default | Description | +| ------------- | ------ | ------- | ------------------------- | +| `enabled` | bool | false | Enable Brave search | +| `api_key` | string | - | Brave Search API key | +| `max_results` | int | 5 | Maximum number of results | ### DuckDuckGo -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `enabled` | bool | true | Enable DuckDuckGo search | -| `max_results` | int | 5 | Maximum number of results | +| Config | Type | Default | Description | +| ------------- | ---- | ------- | ------------------------- | +| `enabled` | bool | true | Enable DuckDuckGo search | +| `max_results` | int | 5 | Maximum number of results | ### Perplexity -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `enabled` | bool | false | Enable Perplexity search | -| `api_key` | string | - | Perplexity API key | -| `max_results` | int | 5 | Maximum number of results | +| Config | Type | Default | Description | +| ------------- | ------ | ------- | ------------------------- | +| `enabled` | bool | false | Enable Perplexity search | +| `api_key` | string | - | Perplexity API key | +| `max_results` | int | 5 | Maximum number of results | ## Exec Tool The exec tool is used to execute shell commands. -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `enable_deny_patterns` | bool | true | Enable default dangerous command blocking | -| `custom_deny_patterns` | array | [] | Custom deny patterns (regular expressions) | +| Config | Type | Default | Description | +| ---------------------- | ----- | ------- | ------------------------------------------ | +| `enable_deny_patterns` | bool | true | Enable default dangerous command blocking | +| `custom_deny_patterns` | array | [] | Custom deny patterns (regular expressions) | ### Functionality @@ -80,10 +81,7 @@ By default, PicoClaw blocks the following dangerous commands: "tools": { "exec": { "enable_deny_patterns": true, - "custom_deny_patterns": [ - "\\brm\\s+-r\\b", - "\\bkillall\\s+python" - ] + "custom_deny_patterns": ["\\brm\\s+-r\\b", "\\bkillall\\s+python"] } } } @@ -93,9 +91,84 @@ By default, PicoClaw blocks the following dangerous commands: The cron tool is used for scheduling periodic tasks. -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | +| Config | Type | Default | Description | +| ---------------------- | ---- | ------- | ---------------------------------------------- | +| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit | + +## MCP Tool + +The MCP tool enables integration with external Model Context Protocol servers. + +### Global Config + +| Config | Type | Default | Description | +| --------- | ------ | ------- | ----------------------------------- | +| `enabled` | bool | false | Enable MCP integration globally | +| `servers` | object | `{}` | Map of server name to server config | + +### Per-Server Config + +| Config | Type | Required | Description | +| ---------- | ------ | -------- | ------------------------------------------ | +| `enabled` | bool | yes | Enable this MCP server | +| `type` | string | no | Transport type: `stdio`, `sse`, `http` | +| `command` | string | stdio | Executable command for stdio transport | +| `args` | array | no | Command arguments for stdio transport | +| `env` | object | no | Environment variables for stdio process | +| `env_file` | string | no | Path to environment file for stdio process | +| `url` | string | sse/http | Endpoint URL for `sse`/`http` transport | +| `headers` | object | no | HTTP headers for `sse`/`http` transport | + +### Transport Behavior + +- If `type` is omitted, transport is auto-detected: + - `url` is set → `sse` + - `command` is set → `stdio` +- `http` and `sse` both use `url` + optional `headers`. +- `env` and `env_file` are only applied to `stdio` servers. + +### Configuration Examples + +#### 1) Stdio MCP server + +```json +{ + "tools": { + "mcp": { + "enabled": true, + "servers": { + "filesystem": { + "enabled": true, + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"] + } + } + } + } +} +``` + +#### 2) Remote SSE/HTTP MCP server + +```json +{ + "tools": { + "mcp": { + "enabled": true, + "servers": { + "remote-mcp": { + "enabled": true, + "type": "sse", + "url": "https://example.com/mcp", + "headers": { + "Authorization": "Bearer YOUR_TOKEN" + } + } + } + } + } +} +``` ## Skills Tool @@ -103,13 +176,13 @@ The skills tool configures skill discovery and installation via registries like ### Registries -| Config | Type | Default | Description | -|--------|------|---------|-------------| -| `registries.clawhub.enabled` | bool | true | Enable ClawHub registry | -| `registries.clawhub.base_url` | string | `https://clawhub.ai` | ClawHub base URL | -| `registries.clawhub.search_path` | string | `/api/v1/search` | Search API path | -| `registries.clawhub.skills_path` | string | `/api/v1/skills` | Skills API path | -| `registries.clawhub.download_path` | string | `/api/v1/download` | Download API path | +| Config | Type | Default | Description | +| ---------------------------------- | ------ | -------------------- | ----------------------- | +| `registries.clawhub.enabled` | bool | true | Enable ClawHub registry | +| `registries.clawhub.base_url` | string | `https://clawhub.ai` | ClawHub base URL | +| `registries.clawhub.search_path` | string | `/api/v1/search` | Search API path | +| `registries.clawhub.skills_path` | string | `/api/v1/skills` | Skills API path | +| `registries.clawhub.download_path` | string | `/api/v1/download` | Download API path | ### Configuration Example @@ -136,8 +209,10 @@ The skills tool configures skill discovery and installation via registries like All configuration options can be overridden via environment variables with the format `PICOCLAW_TOOLS_
_`: For example: + - `PICOCLAW_TOOLS_WEB_BRAVE_ENABLED=true` - `PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS=false` - `PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES=10` +- `PICOCLAW_TOOLS_MCP_ENABLED=true` -Note: Array-type environment variables are not currently supported and must be set via the config file. +Note: Nested map-style config (for example `tools.mcp.servers..*`) is configured in `config.json` rather than environment variables. diff --git a/go.mod b/go.mod index 7892cade6..9f755bbc9 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/gorilla/websocket v1.5.3 github.com/larksuite/oapi-sdk-go/v3 v3.5.3 github.com/mdp/qrterminal/v3 v3.2.1 + github.com/modelcontextprotocol/go-sdk v1.3.0 github.com/mymmrac/telego v1.6.0 github.com/open-dingtalk/dingtalk-stream-sdk-go v0.9.1 github.com/openai/openai-go/v3 v3.22.0 @@ -81,6 +82,7 @@ require ( github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasthttp v1.69.0 // indirect github.com/valyala/fastjson v1.6.7 // indirect + github.com/yosida95/uritemplate/v3 v3.0.2 // indirect golang.org/x/arch v0.24.0 // indirect golang.org/x/crypto v0.48.0 // indirect golang.org/x/net v0.50.0 // indirect diff --git a/go.sum b/go.sum index d1ee1d629..9041826a5 100644 --- a/go.sum +++ b/go.sum @@ -66,6 +66,8 @@ github.com/go-test/deep v1.1.1/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncV github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang-jwt/jwt/v5 v5.2.2 h1:Rl4B7itRWVtYIHFrSNd7vhTiz9UpLdi6gZhZ3wEeDy8= +github.com/golang-jwt/jwt/v5 v5.2.2/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= @@ -130,6 +132,8 @@ github.com/mattn/go-sqlite3 v1.14.34 h1:3NtcvcUnFBPsuRcno8pUtupspG/GM+9nZ88zgJcp github.com/mattn/go-sqlite3 v1.14.34/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/mdp/qrterminal/v3 v3.2.1 h1:6+yQjiiOsSuXT5n9/m60E54vdgFsw0zhADHhHLrFet4= github.com/mdp/qrterminal/v3 v3.2.1/go.mod h1:jOTmXvnBsMy5xqLniO0R++Jmjs2sTm9dFSuQ5kpz/SU= +github.com/modelcontextprotocol/go-sdk v1.3.0 h1:gMfZkv3DzQF5q/DcQePo5rahEY+sguyPfXDfNBcT0Zs= +github.com/modelcontextprotocol/go-sdk v1.3.0/go.mod h1:AnQ//Qc6+4nIyyrB4cxBU7UW9VibK4iOZBeyP/rF1IE= github.com/mymmrac/telego v1.6.0 h1:Zc8rgyHozvd/7ZgyrigyHdAF9koHYMfilYfyB6wlFC0= github.com/mymmrac/telego v1.6.0/go.mod h1:xt6ZWA8zi8KmuzryE1ImEdl9JSwjHNpM4yhC7D8hU4Y= github.com/ncruces/go-strftime v1.0.0 h1:HMFp8mLCTPp341M/ZnA4qaf7ZlsbTc+miZjCLOFAw7w= @@ -212,6 +216,8 @@ github.com/vektah/gqlparser/v2 v2.5.27 h1:RHPD3JOplpk5mP5JGX8RKZkt2/Vwj/PZv0HxTd github.com/vektah/gqlparser/v2 v2.5.27/go.mod h1:D1/VCZtV3LPnQrcPBeR/q5jkSQIPti0uYCP/RI0gIeo= github.com/xyproto/randomstring v1.0.5 h1:YtlWPoRdgMu3NZtP45drfy1GKoojuR7hmRcnhZqKjWU= github.com/xyproto/randomstring v1.0.5/go.mod h1:rgmS5DeNXLivK7YprL0pY+lTuhNQW3iGxZ18UQApw/E= +github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= +github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 00b0f096a..88afa6119 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -23,6 +23,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/constants" "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/mcp" "github.com/sipeed/picoclaw/pkg/media" "github.com/sipeed/picoclaw/pkg/providers" "github.com/sipeed/picoclaw/pkg/routing" @@ -58,7 +59,11 @@ type processOptions struct { const defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json." -func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop { +func NewAgentLoop( + cfg *config.Config, + msgBus *bus.MessageBus, + provider providers.LLMProvider, +) *AgentLoop { registry := NewAgentRegistry(cfg, provider) // Register shared tools to all agents @@ -170,6 +175,71 @@ func registerSharedTools( func (al *AgentLoop) Run(ctx context.Context) error { al.running.Store(true) + // Initialize MCP servers for all agents + if al.cfg.Tools.MCP.Enabled { + mcpManager := mcp.NewManager() + defaultAgent := al.registry.GetDefaultAgent() + var workspacePath string + if defaultAgent != nil && defaultAgent.Workspace != "" { + workspacePath = defaultAgent.Workspace + } else { + workspacePath = al.cfg.WorkspacePath() + } + + if err := mcpManager.LoadFromMCPConfig(ctx, al.cfg.Tools.MCP, workspacePath); err != nil { + logger.WarnCF("agent", "Failed to load MCP servers, MCP tools will not be available", + map[string]any{ + "error": err.Error(), + }) + } else { + // Ensure MCP connections are cleaned up on exit, only if initialization succeeded + defer func() { + if err := mcpManager.Close(); err != nil { + logger.ErrorCF("agent", "Failed to close MCP manager", + map[string]any{ + "error": err.Error(), + }) + } + }() + + // Register MCP tools for all agents + servers := mcpManager.GetServers() + uniqueTools := 0 + totalRegistrations := 0 + agentIDs := al.registry.ListAgentIDs() + agentCount := len(agentIDs) + + for serverName, conn := range servers { + uniqueTools += len(conn.Tools) + for _, tool := range conn.Tools { + for _, agentID := range agentIDs { + agent, ok := al.registry.GetAgent(agentID) + if !ok { + continue + } + mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) + agent.Tools.Register(mcpTool) + totalRegistrations++ + logger.DebugCF("agent", "Registered MCP tool", + map[string]any{ + "agent_id": agentID, + "server": serverName, + "tool": tool.Name, + "name": mcpTool.Name(), + }) + } + } + } + logger.InfoCF("agent", "MCP tools registered successfully", + map[string]any{ + "server_count": len(servers), + "unique_tools": uniqueTools, + "total_registrations": totalRegistrations, + "agent_count": agentCount, + }) + } + } + for al.running.Load() { select { case <-ctx.Done(): @@ -310,7 +380,10 @@ func (al *AgentLoop) RecordLastChatID(chatID string) error { return al.state.SetLastChatID(chatID) } -func (al *AgentLoop) ProcessDirect(ctx context.Context, content, sessionKey string) (string, error) { +func (al *AgentLoop) ProcessDirect( + ctx context.Context, + content, sessionKey string, +) (string, error) { return al.ProcessDirectWithChannel(ctx, content, sessionKey, "cli", "direct") } @@ -331,7 +404,10 @@ func (al *AgentLoop) ProcessDirectWithChannel( // ProcessHeartbeat processes a heartbeat request without session history. // Each heartbeat is independent and doesn't accumulate context. -func (al *AgentLoop) ProcessHeartbeat(ctx context.Context, content, channel, chatID string) (string, error) { +func (al *AgentLoop) ProcessHeartbeat( + ctx context.Context, + content, channel, chatID string, +) (string, error) { agent := al.registry.GetDefaultAgent() if agent == nil { return "", fmt.Errorf("no default agent for heartbeat") @@ -356,13 +432,16 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) } else { logContent = utils.Truncate(msg.Content, 80) } - logger.InfoCF("agent", fmt.Sprintf("Processing message from %s:%s: %s", msg.Channel, msg.SenderID, logContent), + logger.InfoCF( + "agent", + fmt.Sprintf("Processing message from %s:%s: %s", msg.Channel, msg.SenderID, logContent), map[string]any{ "channel": msg.Channel, "chat_id": msg.ChatID, "sender_id": msg.SenderID, "session_key": msg.SessionKey, - }) + }, + ) // Route system messages to processSystemMessage if msg.Channel == "system" { @@ -423,9 +502,15 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) }) } -func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMessage) (string, error) { +func (al *AgentLoop) processSystemMessage( + ctx context.Context, + msg bus.InboundMessage, +) (string, error) { if msg.Channel != "system" { - return "", fmt.Errorf("processSystemMessage called with non-system message channel: %s", msg.Channel) + return "", fmt.Errorf( + "processSystemMessage called with non-system message channel: %s", + msg.Channel, + ) } logger.InfoCF("agent", "Processing system message", @@ -483,14 +568,22 @@ func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMe } // runAgentLoop is the core message processing logic. -func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opts processOptions) (string, error) { +func (al *AgentLoop) runAgentLoop( + ctx context.Context, + agent *AgentInstance, + opts processOptions, +) (string, error) { // 0. Record last channel for heartbeat notifications (skip internal channels) if opts.Channel != "" && opts.ChatID != "" { // Don't record internal channels (cli, system, subagent) if !constants.IsInternalChannel(opts.Channel) { channelKey := fmt.Sprintf("%s:%s", opts.Channel, opts.ChatID) if err := al.RecordLastChannel(channelKey); err != nil { - logger.WarnCF("agent", "Failed to record last channel", map[string]any{"error": err.Error()}) + logger.WarnCF( + "agent", + "Failed to record last channel", + map[string]any{"error": err.Error()}, + ) } } } @@ -572,7 +665,10 @@ func (al *AgentLoop) targetReasoningChannelID(channelName string) (chatID string return "" } -func (al *AgentLoop) handleReasoning(ctx context.Context, reasoningContent, channelName, channelID string) { +func (al *AgentLoop) handleReasoning( + ctx context.Context, + reasoningContent, channelName, channelID string, +) { if reasoningContent == "" || channelName == "" || channelID == "" { return } @@ -665,22 +761,33 @@ func (al *AgentLoop) runLLMIteration( callLLM := func() (*providers.LLMResponse, error) { if len(agent.Candidates) > 1 && al.fallback != nil { - fbResult, fbErr := al.fallback.Execute(ctx, agent.Candidates, + fbResult, fbErr := al.fallback.Execute( + ctx, + agent.Candidates, func(ctx context.Context, provider, model string) (*providers.LLMResponse, error) { - return agent.Provider.Chat(ctx, messages, providerToolDefs, model, map[string]any{ - "max_tokens": agent.MaxTokens, - "temperature": agent.Temperature, - "prompt_cache_key": agent.ID, - }) + return agent.Provider.Chat( + ctx, + messages, + providerToolDefs, + model, + map[string]any{ + "max_tokens": agent.MaxTokens, + "temperature": agent.Temperature, + "prompt_cache_key": agent.ID, + }, + ) }, ) if fbErr != nil { return nil, fbErr } if fbResult.Provider != "" && len(fbResult.Attempts) > 0 { - logger.InfoCF("agent", fmt.Sprintf("Fallback: succeeded with %s/%s after %d attempts", - fbResult.Provider, fbResult.Model, len(fbResult.Attempts)+1), - map[string]any{"agent_id": agent.ID, "iteration": iteration}) + logger.InfoCF( + "agent", + fmt.Sprintf("Fallback: succeeded with %s/%s after %d attempts", + fbResult.Provider, fbResult.Model, len(fbResult.Attempts)+1), + map[string]any{"agent_id": agent.ID, "iteration": iteration}, + ) } return fbResult.Response, nil } @@ -731,10 +838,14 @@ func (al *AgentLoop) runLLMIteration( } if isContextError && retry < maxRetries { - logger.WarnCF("agent", "Context window error detected, attempting compression", map[string]any{ - "error": err.Error(), - "retry": retry, - }) + logger.WarnCF( + "agent", + "Context window error detected, attempting compression", + map[string]any{ + "error": err.Error(), + "retry": retry, + }, + ) if retry == 0 && !constants.IsInternalChannel(opts.Channel) { al.bus.PublishOutbound(ctx, bus.OutboundMessage{ @@ -766,7 +877,12 @@ func (al *AgentLoop) runLLMIteration( return "", iteration, fmt.Errorf("LLM call failed after retries: %w", err) } - go al.handleReasoning(ctx, response.Reasoning, opts.Channel, al.targetReasoningChannelID(opts.Channel)) + go al.handleReasoning( + ctx, + response.Reasoning, + opts.Channel, + al.targetReasoningChannelID(opts.Channel), + ) logger.DebugCF("agent", "LLM response", map[string]any{ @@ -1068,7 +1184,11 @@ func formatMessagesForLog(messages []providers.Message) string { for _, tc := range msg.ToolCalls { fmt.Fprintf(&sb, " - ID: %s, Type: %s, Name: %s\n", tc.ID, tc.Type, tc.Name) if tc.Function != nil { - fmt.Fprintf(&sb, " Arguments: %s\n", utils.Truncate(tc.Function.Arguments, 200)) + fmt.Fprintf( + &sb, + " Arguments: %s\n", + utils.Truncate(tc.Function.Arguments, 200), + ) } } } @@ -1097,7 +1217,11 @@ func formatToolsForLog(toolDefs []providers.ToolDefinition) string { fmt.Fprintf(&sb, " [%d] Type: %s, Name: %s\n", i, tool.Type, tool.Function.Name) fmt.Fprintf(&sb, " Description: %s\n", tool.Function.Description) if len(tool.Function.Parameters) > 0 { - fmt.Fprintf(&sb, " Parameters: %s\n", utils.Truncate(fmt.Sprintf("%v", tool.Function.Parameters), 200)) + fmt.Fprintf( + &sb, + " Parameters: %s\n", + utils.Truncate(fmt.Sprintf("%v", tool.Function.Parameters), 200), + ) } } sb.WriteString("]") @@ -1194,7 +1318,9 @@ func (al *AgentLoop) summarizeBatch( existingSummary string, ) (string, error) { var sb strings.Builder - sb.WriteString("Provide a concise summary of this conversation segment, preserving core context and key points.\n") + sb.WriteString( + "Provide a concise summary of this conversation segment, preserving core context and key points.\n", + ) if existingSummary != "" { sb.WriteString("Existing context: ") sb.WriteString(existingSummary) diff --git a/pkg/config/config.go b/pkg/config/config.go index c4c175495..c84cd90cf 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -565,6 +565,7 @@ type ToolsConfig struct { Exec ExecConfig `json:"exec"` Skills SkillsToolsConfig `json:"skills"` MediaCleanup MediaCleanupConfig `json:"media_cleanup"` + MCP MCPConfig `json:"mcp"` } type SkillsToolsConfig struct { @@ -594,6 +595,34 @@ type ClawHubRegistryConfig struct { MaxResponseSize int `json:"max_response_size" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_RESPONSE_SIZE"` } +// MCPServerConfig defines configuration for a single MCP server +type MCPServerConfig struct { + // Enabled indicates whether this MCP server is active + Enabled bool `json:"enabled"` + // Command is the executable to run (e.g., "npx", "python", "/path/to/server") + Command string `json:"command"` + // Args are the arguments to pass to the command + Args []string `json:"args,omitempty"` + // Env are environment variables to set for the server process (stdio only) + Env map[string]string `json:"env,omitempty"` + // EnvFile is the path to a file containing environment variables (stdio only) + EnvFile string `json:"env_file,omitempty"` + // Type is "stdio", "sse", or "http" (default: stdio if command is set, sse if url is set) + Type string `json:"type,omitempty"` + // URL is used for SSE/HTTP transport + URL string `json:"url,omitempty"` + // Headers are HTTP headers to send with requests (sse/http only) + Headers map[string]string `json:"headers,omitempty"` +} + +// MCPConfig defines configuration for all MCP servers +type MCPConfig struct { + // Enabled globally enables/disables MCP integration + Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_MCP_ENABLED"` + // Servers is a map of server name to server configuration + Servers map[string]MCPServerConfig `json:"servers,omitempty"` +} + func LoadConfig(path string) (*Config, error) { cfg := DefaultConfig() diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index fb0fd4451..9fc09c5f1 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -361,6 +361,10 @@ func DefaultConfig() *Config { TTLSeconds: 300, }, }, + MCP: MCPConfig{ + Enabled: false, + Servers: map[string]MCPServerConfig{}, + }, }, Heartbeat: HeartbeatConfig{ Enabled: true, diff --git a/pkg/mcp/manager.go b/pkg/mcp/manager.go new file mode 100644 index 000000000..8b6d6d9aa --- /dev/null +++ b/pkg/mcp/manager.go @@ -0,0 +1,526 @@ +package mcp + +import ( + "bufio" + "context" + "errors" + "fmt" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/logger" +) + +// headerTransport is an http.RoundTripper that adds custom headers to requests +type headerTransport struct { + base http.RoundTripper + headers map[string]string +} + +func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) { + // Clone the request to avoid modifying the original + req = req.Clone(req.Context()) + + // Add custom headers + for key, value := range t.headers { + req.Header.Set(key, value) + } + + // Use the base transport + base := t.base + if base == nil { + base = http.DefaultTransport + } + return base.RoundTrip(req) +} + +// loadEnvFile loads environment variables from a file in .env format +// Each line should be in the format: KEY=value +// Lines starting with # are comments +// Empty lines are ignored +func loadEnvFile(path string) (map[string]string, error) { + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("failed to open env file: %w", err) + } + defer file.Close() + + envVars := make(map[string]string) + scanner := bufio.NewScanner(file) + lineNum := 0 + + for scanner.Scan() { + lineNum++ + line := strings.TrimSpace(scanner.Text()) + + // Skip empty lines and comments + if line == "" || strings.HasPrefix(line, "#") { + continue + } + + // Parse KEY=value + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("invalid format at line %d: %s", lineNum, line) + } + + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + + if key == "" { + return nil, fmt.Errorf("invalid format at line %d: empty key", lineNum) + } + + // Remove surrounding quotes if present + if len(value) >= 2 { + if (value[0] == '"' && value[len(value)-1] == '"') || + (value[0] == '\'' && value[len(value)-1] == '\'') { + value = value[1 : len(value)-1] + } + } + + envVars[key] = value + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading env file: %w", err) + } + + return envVars, nil +} + +// ServerConnection represents a connection to an MCP server +type ServerConnection struct { + Name string + Client *mcp.Client + Session *mcp.ClientSession + Tools []*mcp.Tool +} + +// Manager manages multiple MCP server connections +type Manager struct { + servers map[string]*ServerConnection + mu sync.RWMutex + closed bool + wg sync.WaitGroup // tracks in-flight CallTool calls +} + +// NewManager creates a new MCP manager +func NewManager() *Manager { + return &Manager{ + servers: make(map[string]*ServerConnection), + } +} + +// LoadFromConfig loads MCP servers from configuration +func (m *Manager) LoadFromConfig(ctx context.Context, cfg *config.Config) error { + return m.LoadFromMCPConfig(ctx, cfg.Tools.MCP, cfg.WorkspacePath()) +} + +// LoadFromMCPConfig loads MCP servers from MCP configuration and workspace path. +// This is the minimal dependency version that doesn't require the full Config object. +func (m *Manager) LoadFromMCPConfig( + ctx context.Context, + mcpCfg config.MCPConfig, + workspacePath string, +) error { + if !mcpCfg.Enabled { + logger.InfoCF("mcp", "MCP integration is disabled", nil) + return nil + } + + if len(mcpCfg.Servers) == 0 { + logger.InfoCF("mcp", "No MCP servers configured", nil) + return nil + } + + logger.InfoCF("mcp", "Initializing MCP servers", + map[string]any{ + "count": len(mcpCfg.Servers), + }) + + var wg sync.WaitGroup + errs := make(chan error, len(mcpCfg.Servers)) + enabledCount := 0 + + for name, serverCfg := range mcpCfg.Servers { + if !serverCfg.Enabled { + logger.DebugCF("mcp", "Skipping disabled server", + map[string]any{ + "server": name, + }) + continue + } + + enabledCount++ + wg.Add(1) + go func(name string, serverCfg config.MCPServerConfig, workspace string) { + defer wg.Done() + + // Resolve relative envFile paths relative to workspace + if serverCfg.EnvFile != "" && !filepath.IsAbs(serverCfg.EnvFile) { + if workspace == "" { + err := fmt.Errorf( + "workspace path is empty while resolving relative envFile %q for server %s", + serverCfg.EnvFile, + name, + ) + logger.ErrorCF("mcp", "Invalid MCP server configuration", + map[string]any{ + "server": name, + "env_file": serverCfg.EnvFile, + "error": err.Error(), + }) + errs <- err + return + } + serverCfg.EnvFile = filepath.Join(workspace, serverCfg.EnvFile) + } + + if err := m.ConnectServer(ctx, name, serverCfg); err != nil { + logger.ErrorCF("mcp", "Failed to connect to MCP server", + map[string]any{ + "server": name, + "error": err.Error(), + }) + errs <- fmt.Errorf("failed to connect to server %s: %w", name, err) + } + }(name, serverCfg, workspacePath) + } + + wg.Wait() + close(errs) + + // Collect errors + var allErrors []error + for err := range errs { + allErrors = append(allErrors, err) + } + + connectedCount := len(m.GetServers()) + + // If all enabled servers failed to connect, return aggregated error + if enabledCount > 0 && connectedCount == 0 { + logger.ErrorCF("mcp", "All MCP servers failed to connect", + map[string]any{ + "failed": len(allErrors), + "total": enabledCount, + }) + return errors.Join(allErrors...) + } + + if len(allErrors) > 0 { + logger.WarnCF("mcp", "Some MCP servers failed to connect", + map[string]any{ + "failed": len(allErrors), + "connected": connectedCount, + "total": enabledCount, + }) + // Don't fail completely if some servers successfully connected + } + + logger.InfoCF("mcp", "MCP server initialization complete", + map[string]any{ + "connected": connectedCount, + "total": enabledCount, + }) + + return nil +} + +// ConnectServer connects to a single MCP server +func (m *Manager) ConnectServer( + ctx context.Context, + name string, + cfg config.MCPServerConfig, +) error { + logger.InfoCF("mcp", "Connecting to MCP server", + map[string]any{ + "server": name, + "command": cfg.Command, + "args_count": len(cfg.Args), + }) + + // Create client + client := mcp.NewClient(&mcp.Implementation{ + Name: "picoclaw", + Version: "1.0.0", + }, nil) + + // Create transport based on configuration + // Auto-detect transport type if not explicitly specified + var transport mcp.Transport + transportType := cfg.Type + + // Auto-detect: if URL is provided, use SSE; if command is provided, use stdio + if transportType == "" { + if cfg.URL != "" { + transportType = "sse" + } else if cfg.Command != "" { + transportType = "stdio" + } else { + return fmt.Errorf("either URL or command must be provided") + } + } + + switch transportType { + case "sse", "http": + if cfg.URL == "" { + return fmt.Errorf("URL is required for SSE/HTTP transport") + } + logger.DebugCF("mcp", "Using SSE/HTTP transport", + map[string]any{ + "server": name, + "url": cfg.URL, + }) + + sseTransport := &mcp.StreamableClientTransport{ + Endpoint: cfg.URL, + } + + // Add custom headers if provided + if len(cfg.Headers) > 0 { + // Create a custom HTTP client with header-injecting transport + sseTransport.HTTPClient = &http.Client{ + Transport: &headerTransport{ + base: http.DefaultTransport, + headers: cfg.Headers, + }, + } + logger.DebugCF("mcp", "Added custom HTTP headers", + map[string]any{ + "server": name, + "header_count": len(cfg.Headers), + }) + } + + transport = sseTransport + case "stdio": + if cfg.Command == "" { + return fmt.Errorf("command is required for stdio transport") + } + logger.DebugCF("mcp", "Using stdio transport", + map[string]any{ + "server": name, + "command": cfg.Command, + }) + // Create command with context + cmd := exec.CommandContext(ctx, cfg.Command, cfg.Args...) + + // Build environment variables with proper override semantics + // Use a map to ensure config variables override file variables + envMap := make(map[string]string) + + // Start with parent process environment + for _, e := range cmd.Environ() { + if idx := strings.Index(e, "="); idx > 0 { + envMap[e[:idx]] = e[idx+1:] + } + } + + // Load environment variables from file if specified + if cfg.EnvFile != "" { + envVars, err := loadEnvFile(cfg.EnvFile) + if err != nil { + return fmt.Errorf("failed to load env file %s: %w", cfg.EnvFile, err) + } + for k, v := range envVars { + envMap[k] = v + } + logger.DebugCF("mcp", "Loaded environment variables from file", + map[string]any{ + "server": name, + "envFile": cfg.EnvFile, + "var_count": len(envVars), + }) + } + + // Environment variables from config override those from file + for k, v := range cfg.Env { + envMap[k] = v + } + + // Convert map to slice + env := make([]string, 0, len(envMap)) + for k, v := range envMap { + env = append(env, fmt.Sprintf("%s=%s", k, v)) + } + cmd.Env = env + + transport = &mcp.CommandTransport{Command: cmd} + default: + return fmt.Errorf( + "unsupported transport type: %s (supported: stdio, sse, http)", + transportType, + ) + } + + // Connect to server + session, err := client.Connect(ctx, transport, nil) + if err != nil { + return fmt.Errorf("failed to connect: %w", err) + } + + // Get server info + initResult := session.InitializeResult() + logger.InfoCF("mcp", "Connected to MCP server", + map[string]any{ + "server": name, + "serverName": initResult.ServerInfo.Name, + "serverVersion": initResult.ServerInfo.Version, + "protocol": initResult.ProtocolVersion, + }) + + // List available tools if supported + var tools []*mcp.Tool + if initResult.Capabilities.Tools != nil { + for tool, err := range session.Tools(ctx, nil) { + if err != nil { + logger.WarnCF("mcp", "Error listing tool", + map[string]any{ + "server": name, + "error": err.Error(), + }) + continue + } + tools = append(tools, tool) + } + + logger.InfoCF("mcp", "Listed tools from MCP server", + map[string]any{ + "server": name, + "toolCount": len(tools), + }) + } + + // Store connection + m.mu.Lock() + m.servers[name] = &ServerConnection{ + Name: name, + Client: client, + Session: session, + Tools: tools, + } + m.mu.Unlock() + + return nil +} + +// GetServers returns all connected servers +func (m *Manager) GetServers() map[string]*ServerConnection { + m.mu.RLock() + defer m.mu.RUnlock() + + result := make(map[string]*ServerConnection, len(m.servers)) + for k, v := range m.servers { + result[k] = v + } + return result +} + +// GetServer returns a specific server connection +func (m *Manager) GetServer(name string) (*ServerConnection, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + + conn, ok := m.servers[name] + return conn, ok +} + +// CallTool calls a tool on a specific server +func (m *Manager) CallTool( + ctx context.Context, + serverName, toolName string, + arguments map[string]any, +) (*mcp.CallToolResult, error) { + m.mu.RLock() + if m.closed { + m.mu.RUnlock() + return nil, fmt.Errorf("manager is closed") + } + conn, ok := m.servers[serverName] + if ok { + m.wg.Add(1) + } + m.mu.RUnlock() + + if !ok { + return nil, fmt.Errorf("server %s not found", serverName) + } + defer m.wg.Done() + + params := &mcp.CallToolParams{ + Name: toolName, + Arguments: arguments, + } + + result, err := conn.Session.CallTool(ctx, params) + if err != nil { + return nil, fmt.Errorf("failed to call tool: %w", err) + } + + return result, nil +} + +// Close closes all server connections +func (m *Manager) Close() error { + m.mu.Lock() + if m.closed { + m.mu.Unlock() + return nil + } + m.closed = true + m.mu.Unlock() + + // Wait for all in-flight CallTool calls to finish before closing sessions + m.wg.Wait() + + m.mu.Lock() + defer m.mu.Unlock() + + logger.InfoCF("mcp", "Closing all MCP server connections", + map[string]any{ + "count": len(m.servers), + }) + + var errs []error + for name, conn := range m.servers { + if err := conn.Session.Close(); err != nil { + logger.ErrorCF("mcp", "Failed to close server connection", + map[string]any{ + "server": name, + "error": err.Error(), + }) + errs = append(errs, fmt.Errorf("server %s: %w", name, err)) + } + } + + m.servers = make(map[string]*ServerConnection) + + if len(errs) > 0 { + return fmt.Errorf("failed to close %d server(s): %w", len(errs), errors.Join(errs...)) + } + + return nil +} + +// GetAllTools returns all tools from all connected servers +func (m *Manager) GetAllTools() map[string][]*mcp.Tool { + m.mu.RLock() + defer m.mu.RUnlock() + + result := make(map[string][]*mcp.Tool) + for name, conn := range m.servers { + if len(conn.Tools) > 0 { + result[name] = conn.Tools + } + } + return result +} diff --git a/pkg/mcp/manager_test.go b/pkg/mcp/manager_test.go new file mode 100644 index 000000000..6dd71a3c2 --- /dev/null +++ b/pkg/mcp/manager_test.go @@ -0,0 +1,298 @@ +package mcp + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestLoadEnvFile(t *testing.T) { + tests := []struct { + name string + content string + expected map[string]string + expectErr bool + }{ + { + name: "basic env file", + content: `API_KEY=secret123 +DATABASE_URL=postgres://localhost/db +PORT=8080`, + expected: map[string]string{ + "API_KEY": "secret123", + "DATABASE_URL": "postgres://localhost/db", + "PORT": "8080", + }, + expectErr: false, + }, + { + name: "with comments and empty lines", + content: `# This is a comment +API_KEY=secret123 + +# Another comment +DATABASE_URL=postgres://localhost/db + +PORT=8080`, + expected: map[string]string{ + "API_KEY": "secret123", + "DATABASE_URL": "postgres://localhost/db", + "PORT": "8080", + }, + expectErr: false, + }, + { + name: "with quoted values", + content: `API_KEY="secret with spaces" +NAME='single quoted' +PLAIN=no-quotes`, + expected: map[string]string{ + "API_KEY": "secret with spaces", + "NAME": "single quoted", + "PLAIN": "no-quotes", + }, + expectErr: false, + }, + { + name: "with spaces around equals", + content: `API_KEY = secret123 +DATABASE_URL= postgres://localhost/db +PORT =8080`, + expected: map[string]string{ + "API_KEY": "secret123", + "DATABASE_URL": "postgres://localhost/db", + "PORT": "8080", + }, + expectErr: false, + }, + { + name: "invalid format - no equals", + content: `INVALID_LINE`, + expectErr: true, + }, + { + name: "empty file", + content: ``, + expected: map[string]string{}, + expectErr: false, + }, + { + name: "only comments", + content: `# Comment 1 +# Comment 2`, + expected: map[string]string{}, + expectErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + envFile := filepath.Join(tmpDir, ".env") + + if err := os.WriteFile(envFile, []byte(tt.content), 0o644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + result, err := loadEnvFile(envFile) + + if tt.expectErr { + if err == nil { + t.Errorf("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if len(result) != len(tt.expected) { + t.Errorf("Expected %d variables, got %d", len(tt.expected), len(result)) + } + + for key, expectedValue := range tt.expected { + if actualValue, ok := result[key]; !ok { + t.Errorf("Expected key %s not found", key) + } else if actualValue != expectedValue { + t.Errorf("For key %s: expected %q, got %q", key, expectedValue, actualValue) + } + } + }) + } +} + +func TestLoadEnvFileNotFound(t *testing.T) { + _, err := loadEnvFile("/nonexistent/file.env") + if err == nil { + t.Error("Expected error for nonexistent file") + } +} + +func TestEnvFilePriority(t *testing.T) { + // Create a temporary .env file + tmpDir := t.TempDir() + envFile := filepath.Join(tmpDir, ".env") + + envContent := `API_KEY=from_file +DATABASE_URL=from_file +SHARED_VAR=from_file` + + if err := os.WriteFile(envFile, []byte(envContent), 0o644); err != nil { + t.Fatalf("Failed to create .env file: %v", err) + } + + // Load envFile + envVars, err := loadEnvFile(envFile) + if err != nil { + t.Fatalf("Failed to load env file: %v", err) + } + + // Verify envFile variables + if envVars["API_KEY"] != "from_file" { + t.Errorf("Expected API_KEY=from_file, got %s", envVars["API_KEY"]) + } + + // Simulate config.Env overriding envFile + configEnv := map[string]string{ + "SHARED_VAR": "from_config", + "NEW_VAR": "from_config", + } + + // Merge: envFile first, then config overrides + merged := make(map[string]string) + for k, v := range envVars { + merged[k] = v + } + for k, v := range configEnv { + merged[k] = v + } + + // Verify priority: config.Env should override envFile + if merged["SHARED_VAR"] != "from_config" { + t.Errorf( + "Expected SHARED_VAR=from_config (config should override file), got %s", + merged["SHARED_VAR"], + ) + } + if merged["API_KEY"] != "from_file" { + t.Errorf("Expected API_KEY=from_file, got %s", merged["API_KEY"]) + } + if merged["NEW_VAR"] != "from_config" { + t.Errorf("Expected NEW_VAR=from_config, got %s", merged["NEW_VAR"]) + } +} + +func TestLoadFromMCPConfig_EmptyWorkspaceWithRelativeEnvFile(t *testing.T) { + mgr := NewManager() + + mcpCfg := config.MCPConfig{ + Enabled: true, + Servers: map[string]config.MCPServerConfig{ + "test-server": { + Enabled: true, + Command: "echo", + Args: []string{"ok"}, + EnvFile: ".env", + }, + }, + } + + err := mgr.LoadFromMCPConfig(context.Background(), mcpCfg, "") + if err == nil { + t.Fatal("expected error for relative env_file with empty workspace path, got nil") + } + + if !strings.Contains(err.Error(), "workspace path is empty") { + t.Fatalf("expected workspace path validation error, got: %v", err) + } +} + +func TestNewManager_InitialState(t *testing.T) { + mgr := NewManager() + if mgr == nil { + t.Fatal("expected manager instance, got nil") + } + if len(mgr.GetServers()) != 0 { + t.Fatalf("expected no servers on new manager, got %d", len(mgr.GetServers())) + } +} + +func TestLoadFromMCPConfig_DisabledOrEmptyServers(t *testing.T) { + mgr := NewManager() + + err := mgr.LoadFromMCPConfig(context.Background(), config.MCPConfig{Enabled: false}, "/tmp") + if err != nil { + t.Fatalf("expected nil error when MCP disabled, got: %v", err) + } + + err = mgr.LoadFromMCPConfig(context.Background(), config.MCPConfig{Enabled: true}, "/tmp") + if err != nil { + t.Fatalf("expected nil error when no servers configured, got: %v", err) + } +} + +func TestGetServers_ReturnsCopy(t *testing.T) { + mgr := NewManager() + mgr.servers["s1"] = &ServerConnection{Name: "s1"} + + servers := mgr.GetServers() + delete(servers, "s1") + + if _, ok := mgr.GetServer("s1"); !ok { + t.Fatal("expected internal manager state to remain unchanged") + } +} + +func TestGetAllTools_FiltersEmptyTools(t *testing.T) { + mgr := NewManager() + mgr.servers["empty"] = &ServerConnection{Name: "empty", Tools: nil} + mgr.servers["with-tools"] = &ServerConnection{Name: "with-tools", Tools: []*sdkmcp.Tool{{}}} + + all := mgr.GetAllTools() + if _, ok := all["empty"]; ok { + t.Fatal("expected server without tools to be excluded") + } + if _, ok := all["with-tools"]; !ok { + t.Fatal("expected server with tools to be included") + } +} + +func TestCallTool_ErrorsForClosedOrMissingServer(t *testing.T) { + t.Run("manager closed", func(t *testing.T) { + mgr := NewManager() + mgr.closed = true + + _, err := mgr.CallTool(context.Background(), "s1", "tool", nil) + if err == nil || !strings.Contains(err.Error(), "manager is closed") { + t.Fatalf("expected manager closed error, got: %v", err) + } + }) + + t.Run("server missing", func(t *testing.T) { + mgr := NewManager() + + _, err := mgr.CallTool(context.Background(), "missing", "tool", nil) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected server not found error, got: %v", err) + } + }) +} + +func TestClose_IdempotentOnEmptyManager(t *testing.T) { + mgr := NewManager() + + if err := mgr.Close(); err != nil { + t.Fatalf("first close should succeed, got: %v", err) + } + if err := mgr.Close(); err != nil { + t.Fatalf("second close should be idempotent, got: %v", err) + } +} diff --git a/pkg/tools/mcp_tool.go b/pkg/tools/mcp_tool.go new file mode 100644 index 000000000..6e53cf354 --- /dev/null +++ b/pkg/tools/mcp_tool.go @@ -0,0 +1,246 @@ +package tools + +import ( + "context" + "encoding/json" + "fmt" + "hash/fnv" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// MCPManager defines the interface for MCP manager operations +// This allows for easier testing with mock implementations +type MCPManager interface { + CallTool( + ctx context.Context, + serverName, toolName string, + arguments map[string]any, + ) (*mcp.CallToolResult, error) +} + +// MCPTool wraps an MCP tool to implement the Tool interface +type MCPTool struct { + manager MCPManager + serverName string + tool *mcp.Tool +} + +// NewMCPTool creates a new MCP tool wrapper +func NewMCPTool(manager MCPManager, serverName string, tool *mcp.Tool) *MCPTool { + return &MCPTool{ + manager: manager, + serverName: serverName, + tool: tool, + } +} + +// sanitizeIdentifierComponent normalizes a string so it can be safely used +// as part of a tool/function identifier for downstream providers. +// It: +// - lowercases the string +// - replaces any character not in [a-z0-9_-] with '_' +// - collapses multiple consecutive '_' into a single '_' +// - trims leading/trailing '_' +// - falls back to "unnamed" if the result is empty +// - truncates overly long components to a reasonable length +func sanitizeIdentifierComponent(s string) string { + const maxLen = 64 + + s = strings.ToLower(s) + var b strings.Builder + b.Grow(len(s)) + + prevUnderscore := false + for _, r := range s { + isAllowed := (r >= 'a' && r <= 'z') || + (r >= '0' && r <= '9') || + r == '_' || r == '-' + + if !isAllowed { + // Normalize any disallowed character to '_' + if !prevUnderscore { + b.WriteRune('_') + prevUnderscore = true + } + continue + } + + if r == '_' { + if prevUnderscore { + continue + } + prevUnderscore = true + } else { + prevUnderscore = false + } + + b.WriteRune(r) + } + + result := strings.Trim(b.String(), "_") + if result == "" { + result = "unnamed" + } + + if len(result) > maxLen { + result = result[:maxLen] + } + + return result +} + +// Name returns the tool name, prefixed with the server name. +// The total length is capped at 64 characters (OpenAI-compatible API limit). +// A short hash of the original (unsanitized) server and tool names is appended +// whenever sanitization is lossy or the name is truncated, ensuring that two +// names which differ only in disallowed characters remain distinct after sanitization. +func (t *MCPTool) Name() string { + // Prefix with server name to avoid conflicts, and sanitize components + sanitizedServer := sanitizeIdentifierComponent(t.serverName) + sanitizedTool := sanitizeIdentifierComponent(t.tool.Name) + full := fmt.Sprintf("mcp_%s_%s", sanitizedServer, sanitizedTool) + + // Check if sanitization was lossless (only lowercasing, no char replacement/truncation) + lossless := strings.ToLower(t.serverName) == sanitizedServer && + strings.ToLower(t.tool.Name) == sanitizedTool + + const maxTotal = 64 + if lossless && len(full) <= maxTotal { + return full + } + + // Sanitization was lossy or name too long: append hash of the ORIGINAL names + // (not the sanitized names) so different originals always yield different hashes. + h := fnv.New32a() + _, _ = h.Write([]byte(t.serverName + "\x00" + t.tool.Name)) + suffix := fmt.Sprintf("%08x", h.Sum32()) // 8 chars + + base := full + if len(base) > maxTotal-9 { + base = strings.TrimRight(full[:maxTotal-9], "_") + } + return base + "_" + suffix +} + +// Description returns the tool description +func (t *MCPTool) Description() string { + desc := t.tool.Description + if desc == "" { + desc = fmt.Sprintf("MCP tool from %s server", t.serverName) + } + // Add server info to description + return fmt.Sprintf("[MCP:%s] %s", t.serverName, desc) +} + +// Parameters returns the tool parameters schema +func (t *MCPTool) Parameters() map[string]any { + // The InputSchema is already a JSON Schema object + schema := t.tool.InputSchema + + // Handle nil schema + if schema == nil { + return map[string]any{ + "type": "object", + "properties": map[string]any{}, + "required": []string{}, + } + } + + // Try direct conversion first (fast path) + if schemaMap, ok := schema.(map[string]any); ok { + return schemaMap + } + + // Handle json.RawMessage and []byte - unmarshal directly + var jsonData []byte + if rawMsg, ok := schema.(json.RawMessage); ok { + jsonData = rawMsg + } else if bytes, ok := schema.([]byte); ok { + jsonData = bytes + } + + if jsonData != nil { + var result map[string]any + if err := json.Unmarshal(jsonData, &result); err == nil { + return result + } + // Fallback on error + return map[string]any{ + "type": "object", + "properties": map[string]any{}, + "required": []string{}, + } + } + + // For other types (structs, etc.), convert via JSON marshal/unmarshal + var err error + jsonData, err = json.Marshal(schema) + if err != nil { + // Fallback to empty schema if marshaling fails + 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 { + // Fallback to empty schema if unmarshaling fails + return map[string]any{ + "type": "object", + "properties": map[string]any{}, + "required": []string{}, + } + } + + return result +} + +// Execute executes the MCP tool +func (t *MCPTool) Execute(ctx context.Context, args map[string]any) *ToolResult { + result, err := t.manager.CallTool(ctx, t.serverName, t.tool.Name, args) + if err != nil { + return ErrorResult(fmt.Sprintf("MCP tool execution failed: %v", err)).WithError(err) + } + + if result == nil { + nilErr := fmt.Errorf("MCP tool returned nil result without error") + return ErrorResult("MCP tool execution failed: nil result").WithError(nilErr) + } + + // Handle error result from server + if result.IsError { + errMsg := extractContentText(result.Content) + return ErrorResult(fmt.Sprintf("MCP tool returned error: %s", errMsg)). + WithError(fmt.Errorf("MCP tool error: %s", errMsg)) + } + + // Extract text content from result + output := extractContentText(result.Content) + + return &ToolResult{ + ForLLM: output, + IsError: false, + } +} + +// extractContentText extracts text from MCP content array +func extractContentText(content []mcp.Content) string { + var parts []string + for _, c := range content { + switch v := c.(type) { + case *mcp.TextContent: + parts = append(parts, v.Text) + case *mcp.ImageContent: + // For images, just indicate that an image was returned + parts = append(parts, fmt.Sprintf("[Image: %s]", v.MIMEType)) + default: + // For other content types, use string representation + parts = append(parts, fmt.Sprintf("[Content: %T]", v)) + } + } + return strings.Join(parts, "\n") +} diff --git a/pkg/tools/mcp_tool_test.go b/pkg/tools/mcp_tool_test.go new file mode 100644 index 000000000..95bb0f992 --- /dev/null +++ b/pkg/tools/mcp_tool_test.go @@ -0,0 +1,492 @@ +package tools + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// MockMCPManager is a mock implementation of MCPManager interface for testing +type MockMCPManager struct { + callToolFunc func(ctx context.Context, serverName, toolName string, arguments map[string]any) (*mcp.CallToolResult, error) +} + +func (m *MockMCPManager) CallTool( + ctx context.Context, + serverName, toolName string, + arguments map[string]any, +) (*mcp.CallToolResult, error) { + if m.callToolFunc != nil { + return m.callToolFunc(ctx, serverName, toolName, arguments) + } + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "mock result"}, + }, + IsError: false, + }, nil +} + +// TestNewMCPTool verifies MCP tool creation +func TestNewMCPTool(t *testing.T) { + manager := &MockMCPManager{} + tool := &mcp.Tool{ + Name: "test_tool", + Description: "A test tool", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "input": map[string]any{ + "type": "string", + "description": "Test input", + }, + }, + }, + } + + mcpTool := NewMCPTool(manager, "test_server", tool) + + if mcpTool == nil { + t.Fatal("NewMCPTool should not return nil") + } + // Verify tool properties we can access + if mcpTool.Name() != "mcp_test_server_test_tool" { + t.Errorf("Expected tool name with prefix, got '%s'", mcpTool.Name()) + } +} + +// TestMCPTool_Name verifies tool name with server prefix +func TestMCPTool_Name(t *testing.T) { + tests := []struct { + name string + serverName string + toolName string + expected string + }{ + { + name: "simple name", + serverName: "github", + toolName: "create_issue", + expected: "mcp_github_create_issue", + }, + { + name: "filesystem server", + serverName: "filesystem", + toolName: "read_file", + expected: "mcp_filesystem_read_file", + }, + { + name: "remote server", + serverName: "remote-api", + toolName: "fetch_data", + expected: "mcp_remote-api_fetch_data", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := &MockMCPManager{} + tool := &mcp.Tool{Name: tt.toolName} + mcpTool := NewMCPTool(manager, tt.serverName, tool) + + result := mcpTool.Name() + if result != tt.expected { + t.Errorf("Expected name '%s', got '%s'", tt.expected, result) + } + }) + } +} + +// TestMCPTool_Description verifies tool description generation +func TestMCPTool_Description(t *testing.T) { + tests := []struct { + name string + serverName string + toolDescription string + expectContains []string + }{ + { + name: "with description", + serverName: "github", + toolDescription: "Create a GitHub issue", + expectContains: []string{"[MCP:github]", "Create a GitHub issue"}, + }, + { + name: "empty description", + serverName: "filesystem", + toolDescription: "", + expectContains: []string{"[MCP:filesystem]", "MCP tool from filesystem server"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := &MockMCPManager{} + tool := &mcp.Tool{ + Name: "test_tool", + Description: tt.toolDescription, + } + mcpTool := NewMCPTool(manager, tt.serverName, tool) + + result := mcpTool.Description() + + for _, expected := range tt.expectContains { + if !strings.Contains(result, expected) { + t.Errorf("Description should contain '%s', got: %s", expected, result) + } + } + }) + } +} + +// TestMCPTool_Parameters verifies parameter schema conversion +func TestMCPTool_Parameters(t *testing.T) { + tests := []struct { + name string + inputSchema any + expectType string + checkProperty string + expectProperty bool + }{ + { + name: "map schema", + inputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "query": map[string]any{ + "type": "string", + "description": "Search query", + }, + }, + "required": []string{"query"}, + }, + expectType: "object", + checkProperty: "query", + expectProperty: true, + }, + { + name: "nil schema", + inputSchema: nil, + expectType: "object", + expectProperty: false, + }, + { + name: "json.RawMessage schema", + inputSchema: []byte(`{ + "type": "object", + "properties": { + "repo": { + "type": "string", + "description": "Repository name" + }, + "stars": { + "type": "integer", + "description": "Minimum stars" + } + }, + "required": ["repo"] + }`), + expectType: "object", + checkProperty: "repo", + expectProperty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + manager := &MockMCPManager{} + tool := &mcp.Tool{ + Name: "test_tool", + InputSchema: tt.inputSchema, + } + mcpTool := NewMCPTool(manager, "test_server", tool) + + params := mcpTool.Parameters() + + if params == nil { + t.Fatal("Parameters should not be nil") + } + + if params["type"] != tt.expectType { + t.Errorf("Expected type '%s', got '%v'", tt.expectType, params["type"]) + } + + // Check if property exists when expected + if tt.checkProperty != "" { + properties, ok := params["properties"].(map[string]any) + if !ok && tt.expectProperty { + t.Errorf("Expected properties to be a map") + return + } + if ok { + _, hasProperty := properties[tt.checkProperty] + if hasProperty != tt.expectProperty { + t.Errorf("Expected property '%s' existence: %v, got: %v", + tt.checkProperty, tt.expectProperty, hasProperty) + } + } + } + }) + } +} + +// TestMCPTool_Execute_Success tests successful tool execution +func TestMCPTool_Execute_Success(t *testing.T) { + manager := &MockMCPManager{ + callToolFunc: func(ctx context.Context, serverName, toolName string, arguments map[string]any) (*mcp.CallToolResult, error) { + // Verify correct parameters passed + if serverName != "github" { + t.Errorf("Expected serverName 'github', got '%s'", serverName) + } + if toolName != "search_repos" { + t.Errorf("Expected toolName 'search_repos', got '%s'", toolName) + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "Found 3 repositories"}, + }, + IsError: false, + }, nil + }, + } + + tool := &mcp.Tool{ + Name: "search_repos", + Description: "Search GitHub repositories", + } + mcpTool := NewMCPTool(manager, "github", tool) + + ctx := context.Background() + args := map[string]any{ + "query": "golang mcp", + } + + result := mcpTool.Execute(ctx, args) + + if result == nil { + t.Fatal("Result should not be nil") + } + if result.IsError { + t.Errorf("Expected no error, got error: %s", result.ForLLM) + } + if result.ForLLM != "Found 3 repositories" { + t.Errorf("Expected 'Found 3 repositories', got '%s'", result.ForLLM) + } +} + +// TestMCPTool_Execute_ManagerError tests execution when manager returns error +func TestMCPTool_Execute_ManagerError(t *testing.T) { + manager := &MockMCPManager{ + callToolFunc: func(ctx context.Context, serverName, toolName string, arguments map[string]any) (*mcp.CallToolResult, error) { + return nil, fmt.Errorf("connection failed") + }, + } + + tool := &mcp.Tool{Name: "test_tool"} + mcpTool := NewMCPTool(manager, "test_server", tool) + + ctx := context.Background() + result := mcpTool.Execute(ctx, map[string]any{}) + + if result == nil { + t.Fatal("Result should not be nil") + } + if !result.IsError { + t.Error("Expected IsError to be true") + } + if !strings.Contains(result.ForLLM, "MCP tool execution failed") { + t.Errorf("Error message should mention execution failure, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "connection failed") { + t.Errorf("Error message should include original error, got: %s", result.ForLLM) + } +} + +// TestMCPTool_Execute_ServerError tests execution when server returns error +func TestMCPTool_Execute_ServerError(t *testing.T) { + manager := &MockMCPManager{ + callToolFunc: func(ctx context.Context, serverName, toolName string, arguments map[string]any) (*mcp.CallToolResult, error) { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "Invalid API key"}, + }, + IsError: true, + }, nil + }, + } + + tool := &mcp.Tool{Name: "test_tool"} + mcpTool := NewMCPTool(manager, "test_server", tool) + + ctx := context.Background() + result := mcpTool.Execute(ctx, map[string]any{}) + + if result == nil { + t.Fatal("Result should not be nil") + } + if !result.IsError { + t.Error("Expected IsError to be true") + } + if !strings.Contains(result.ForLLM, "MCP tool returned error") { + t.Errorf("Error message should mention server error, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "Invalid API key") { + t.Errorf("Error message should include server message, got: %s", result.ForLLM) + } +} + +// TestMCPTool_Execute_MultipleContent tests execution with multiple content items +func TestMCPTool_Execute_MultipleContent(t *testing.T) { + manager := &MockMCPManager{ + callToolFunc: func(ctx context.Context, serverName, toolName string, arguments map[string]any) (*mcp.CallToolResult, error) { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: "First line"}, + &mcp.TextContent{Text: "Second line"}, + &mcp.TextContent{Text: "Third line"}, + }, + IsError: false, + }, nil + }, + } + + tool := &mcp.Tool{Name: "multi_output"} + mcpTool := NewMCPTool(manager, "test_server", tool) + + ctx := context.Background() + result := mcpTool.Execute(ctx, map[string]any{}) + + if result.IsError { + t.Errorf("Expected no error, got: %s", result.ForLLM) + } + + expected := "First line\nSecond line\nThird line" + if result.ForLLM != expected { + t.Errorf("Expected '%s', got '%s'", expected, result.ForLLM) + } +} + +// TestExtractContentText_TextContent tests text content extraction +func TestExtractContentText_TextContent(t *testing.T) { + content := []mcp.Content{ + &mcp.TextContent{Text: "Hello World"}, + &mcp.TextContent{Text: "Second message"}, + } + + result := extractContentText(content) + expected := "Hello World\nSecond message" + + if result != expected { + t.Errorf("Expected '%s', got '%s'", expected, result) + } +} + +// TestExtractContentText_ImageContent tests image content extraction +func TestExtractContentText_ImageContent(t *testing.T) { + content := []mcp.Content{ + &mcp.ImageContent{ + Data: []byte("base64data"), + MIMEType: "image/png", + }, + } + + result := extractContentText(content) + + if !strings.Contains(result, "[Image:") { + t.Errorf("Expected image indicator, got: %s", result) + } + if !strings.Contains(result, "image/png") { + t.Errorf("Expected MIME type in output, got: %s", result) + } +} + +// TestExtractContentText_MixedContent tests mixed content types +func TestExtractContentText_MixedContent(t *testing.T) { + content := []mcp.Content{ + &mcp.TextContent{Text: "Description"}, + &mcp.ImageContent{ + Data: []byte("data"), + MIMEType: "image/jpeg", + }, + &mcp.TextContent{Text: "More text"}, + } + + result := extractContentText(content) + + if !strings.Contains(result, "Description") { + t.Errorf("Should contain text content, got: %s", result) + } + if !strings.Contains(result, "[Image:") { + t.Errorf("Should contain image indicator, got: %s", result) + } + if !strings.Contains(result, "More text") { + t.Errorf("Should contain second text, got: %s", result) + } +} + +// TestExtractContentText_EmptyContent tests empty content array +func TestExtractContentText_EmptyContent(t *testing.T) { + content := []mcp.Content{} + + result := extractContentText(content) + + if result != "" { + t.Errorf("Expected empty string for empty content, got: %s", result) + } +} + +// TestMCPTool_InterfaceCompliance verifies MCPTool implements Tool interface +func TestMCPTool_InterfaceCompliance(t *testing.T) { + manager := &MockMCPManager{} + tool := &mcp.Tool{Name: "test"} + mcpTool := NewMCPTool(manager, "test_server", tool) + + // Verify it implements Tool interface + var _ Tool = mcpTool +} + +// TestMCPTool_Parameters_MapSchema tests schema that's already a map +func TestMCPTool_Parameters_MapSchema(t *testing.T) { + manager := &MockMCPManager{} + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + "description": "The name parameter", + }, + }, + "required": []string{"name"}, + } + + tool := &mcp.Tool{ + Name: "test_tool", + InputSchema: schema, + } + mcpTool := NewMCPTool(manager, "test_server", tool) + + params := mcpTool.Parameters() + + // Should return the schema as-is when it's already a map + if params["type"] != "object" { + t.Errorf("Expected type 'object', got '%v'", params["type"]) + } + + props, ok := params["properties"].(map[string]any) + if !ok { + t.Error("Properties should be a map") + } + + nameParam, ok := props["name"].(map[string]any) + if !ok { + t.Error("Name parameter should exist") + } + + if nameParam["type"] != "string" { + t.Errorf("Name type should be 'string', got '%v'", nameParam["type"]) + } +} diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index d37a093a8..0ba983e02 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -25,7 +25,12 @@ func NewToolRegistry() *ToolRegistry { func (r *ToolRegistry) Register(tool Tool) { r.mu.Lock() defer r.mu.Unlock() - r.tools[tool.Name()] = tool + name := tool.Name() + if _, exists := r.tools[name]; exists { + logger.WarnCF("tools", "Tool registration overwrites existing tool", + map[string]any{"name": name}) + } + r.tools[name] = tool } func (r *ToolRegistry) Get(name string) (Tool, bool) { diff --git a/scripts/test-docker-mcp.sh b/scripts/test-docker-mcp.sh new file mode 100755 index 000000000..9d582ffa0 --- /dev/null +++ b/scripts/test-docker-mcp.sh @@ -0,0 +1,49 @@ +#!/bin/sh +# Test script for MCP tools in Docker (full-featured image) + +set -e + +COMPOSE_FILE="docker/docker-compose.full.yml" +SERVICE="picoclaw-agent" + +echo "🧪 Testing MCP tools in Docker container (full-featured image)..." +echo "" + +# Build the image +echo "📦 Building Docker image..." +docker compose -f "$COMPOSE_FILE" build "$SERVICE" + +# Test npx +echo "✅ Testing npx..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'npx --version' + +# Test npm +echo "✅ Testing npm..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'npm --version' + +# Test node +echo "✅ Testing Node.js..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'node --version' + +# Test git +echo "✅ Testing git..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'git --version' + +# Test python +echo "✅ Testing Python..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'python3 --version' + +# Test uv +echo "✅ Testing uv..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'uv --version' + +# Test MCP server installation (quick) +echo "✅ Testing @modelcontextprotocol/server-filesystem MCP server install with npx..." +docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c '