Merge pull request #282 from yuchou87/mcp-tools-support

feat: Add mcp tools support
2026-03-03 00:18:14 +08:00 · 2026-03-03 00:18:14 +08:00 · 0150947e61
commit 0150947e61
parent 475d8f948b 4e348e39ac
16 changed files with 2105 additions and 62 deletions
--- a/40
+++ b/40
@ -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)"
--- 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
  }
-}
+}
--- a/docker/Dockerfile.full
+++ 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"]
--- a/docker/docker-compose.full.yml
+++ 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
--- 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_<SECTION>_<KEY>`:

 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.<name>.*`) is configured in `config.json` rather than environment variables.
--- 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
--- 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=
--- 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)
--- 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()

--- 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,
--- a/pkg/mcp/manager.go
+++ 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
+}
--- a/pkg/mcp/manager_test.go
+++ 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)
+	}
+}
--- a/pkg/tools/mcp_tool.go
+++ 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")
+}
--- a/pkg/tools/mcp_tool_test.go
+++ 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"])
+	}
+}
--- 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) {
--- a/scripts/test-docker-mcp.sh
+++ 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 '</dev/null timeout 5 npx -y @modelcontextprotocol/server-filesystem /tmp || true'
+
+echo ""
+echo "🎉 All MCP tools are working correctly!"
+echo ""
+echo "Next steps:"
+echo "  1. Configure MCP servers in config/config.json"
+echo "  2. Run: docker compose -f $COMPOSE_FILE --profile gateway up"