mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-05-25 16:00:35 +00:00
fix: address PR review feedback for MCP tools support
- Avoid logging sensitive cfg.Args in ConnectServer; log args_count instead - Sanitize server/tool name components in MCPTool.Name() to ensure valid identifiers for downstream providers (lowercase, [a-z0-9_-] only) - Add slack as 5th MCP server example in config.example.json - Move Dockerfile.full and docker-compose.full.yml into docker/ directory for consistency with existing docker/Dockerfile and docker/docker-compose.yml - Fix all Makefile docker-* targets to reference correct compose file paths - Fix docker/docker-compose.full.yml build context (.. ) and volume paths - Fix scripts/test-docker-mcp.sh compose file path and replace cowsay test with actual @modelcontextprotocol/server-filesystem MCP server test
This commit is contained in:
@@ -207,12 +207,12 @@ run: build
|
|||||||
## docker-build: Build Docker image (minimal Alpine-based)
|
## docker-build: Build Docker image (minimal Alpine-based)
|
||||||
docker-build:
|
docker-build:
|
||||||
@echo "Building minimal Docker image (Alpine-based)..."
|
@echo "Building minimal Docker image (Alpine-based)..."
|
||||||
docker compose build picoclaw-agent picoclaw-gateway
|
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: Build Docker image with full MCP support (Node.js 24)
|
||||||
docker-build-full:
|
docker-build-full:
|
||||||
@echo "Building full-featured Docker image (Node.js 24)..."
|
@echo "Building full-featured Docker image (Node.js 24)..."
|
||||||
docker compose -f docker-compose.full.yml build picoclaw-agent picoclaw-gateway
|
docker compose -f docker/docker-compose.full.yml build picoclaw-agent picoclaw-gateway
|
||||||
|
|
||||||
## docker-test: Test MCP tools in Docker container
|
## docker-test: Test MCP tools in Docker container
|
||||||
docker-test:
|
docker-test:
|
||||||
@@ -222,24 +222,24 @@ docker-test:
|
|||||||
|
|
||||||
## docker-run: Run picoclaw gateway in Docker (Alpine-based)
|
## docker-run: Run picoclaw gateway in Docker (Alpine-based)
|
||||||
docker-run:
|
docker-run:
|
||||||
docker compose --profile gateway up
|
docker compose -f docker/docker-compose.yml --profile gateway up
|
||||||
|
|
||||||
## docker-run-full: Run picoclaw gateway in Docker (full-featured)
|
## docker-run-full: Run picoclaw gateway in Docker (full-featured)
|
||||||
docker-run-full:
|
docker-run-full:
|
||||||
docker compose -f docker-compose.full.yml --profile gateway up
|
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: Run picoclaw agent in Docker (interactive, Alpine-based)
|
||||||
docker-run-agent:
|
docker-run-agent:
|
||||||
docker compose run --rm picoclaw-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: Run picoclaw agent in Docker (interactive, full-featured)
|
||||||
docker-run-agent-full:
|
docker-run-agent-full:
|
||||||
docker compose -f docker-compose.full.yml run --rm picoclaw-agent
|
docker compose -f docker/docker-compose.full.yml run --rm picoclaw-agent
|
||||||
|
|
||||||
## docker-clean: Clean Docker images and volumes
|
## docker-clean: Clean Docker images and volumes
|
||||||
docker-clean:
|
docker-clean:
|
||||||
docker compose down -v
|
docker compose -f docker/docker-compose.yml down -v
|
||||||
docker compose -f docker-compose.full.yml down -v
|
docker compose -f docker/docker-compose.full.yml down -v
|
||||||
docker rmi picoclaw:latest picoclaw:full 2>/dev/null || true
|
docker rmi picoclaw:latest picoclaw:full 2>/dev/null || true
|
||||||
|
|
||||||
## help: Show this help message
|
## help: Show this help message
|
||||||
|
|||||||
@@ -279,6 +279,18 @@
|
|||||||
"@modelcontextprotocol/server-postgres",
|
"@modelcontextprotocol/server-postgres",
|
||||||
"postgresql://user:password@localhost/dbname"
|
"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"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1,17 +1,17 @@
|
|||||||
services:
|
services:
|
||||||
# ─────────────────────────────────────────────
|
# ─────────────────────────────────────────────
|
||||||
# PicoClaw Agent (one-shot query) - Full MCP Support
|
# PicoClaw Agent (one-shot query) - Full MCP Support
|
||||||
# docker compose -f docker-compose.full.yml run --rm picoclaw-agent -m "Hello"
|
# docker compose -f docker/docker-compose.full.yml run --rm picoclaw-agent -m "Hello"
|
||||||
# ─────────────────────────────────────────────
|
# ─────────────────────────────────────────────
|
||||||
picoclaw-agent:
|
picoclaw-agent:
|
||||||
build:
|
build:
|
||||||
context: .
|
context: ..
|
||||||
dockerfile: Dockerfile.full
|
dockerfile: docker/Dockerfile.full
|
||||||
container_name: picoclaw-agent-full
|
container_name: picoclaw-agent-full
|
||||||
profiles:
|
profiles:
|
||||||
- agent
|
- agent
|
||||||
volumes:
|
volumes:
|
||||||
- ./config/config.json:/root/.picoclaw/config.json:ro
|
- ../config/config.json:/root/.picoclaw/config.json:ro
|
||||||
- picoclaw-workspace:/root/.picoclaw/workspace
|
- picoclaw-workspace:/root/.picoclaw/workspace
|
||||||
- picoclaw-npm-cache:/root/.npm # npm cache for faster MCP server installs
|
- picoclaw-npm-cache:/root/.npm # npm cache for faster MCP server installs
|
||||||
entrypoint: ["picoclaw", "agent"]
|
entrypoint: ["picoclaw", "agent"]
|
||||||
@@ -20,19 +20,19 @@ services:
|
|||||||
|
|
||||||
# ─────────────────────────────────────────────
|
# ─────────────────────────────────────────────
|
||||||
# PicoClaw Gateway (Long-running Bot) - Full MCP Support
|
# PicoClaw Gateway (Long-running Bot) - Full MCP Support
|
||||||
# docker compose -f docker-compose.full.yml --profile gateway up
|
# docker compose -f docker/docker-compose.full.yml --profile gateway up
|
||||||
# ─────────────────────────────────────────────
|
# ─────────────────────────────────────────────
|
||||||
picoclaw-gateway:
|
picoclaw-gateway:
|
||||||
build:
|
build:
|
||||||
context: .
|
context: ..
|
||||||
dockerfile: Dockerfile.full
|
dockerfile: docker/Dockerfile.full
|
||||||
container_name: picoclaw-gateway-full
|
container_name: picoclaw-gateway-full
|
||||||
restart: unless-stopped
|
restart: unless-stopped
|
||||||
profiles:
|
profiles:
|
||||||
- gateway
|
- gateway
|
||||||
volumes:
|
volumes:
|
||||||
# Configuration file
|
# Configuration file
|
||||||
- ./config/config.json:/root/.picoclaw/config.json:ro
|
- ../config/config.json:/root/.picoclaw/config.json:ro
|
||||||
# Persistent workspace (sessions, memory, logs)
|
# Persistent workspace (sessions, memory, logs)
|
||||||
- picoclaw-workspace:/root/.picoclaw/workspace
|
- picoclaw-workspace:/root/.picoclaw/workspace
|
||||||
# NPM cache for faster MCP server installs
|
# NPM cache for faster MCP server installs
|
||||||
+3
-3
@@ -243,9 +243,9 @@ func (m *Manager) ConnectServer(
|
|||||||
) error {
|
) error {
|
||||||
logger.InfoCF("mcp", "Connecting to MCP server",
|
logger.InfoCF("mcp", "Connecting to MCP server",
|
||||||
map[string]any{
|
map[string]any{
|
||||||
"server": name,
|
"server": name,
|
||||||
"command": cfg.Command,
|
"command": cfg.Command,
|
||||||
"args": cfg.Args,
|
"args_count": len(cfg.Args),
|
||||||
})
|
})
|
||||||
|
|
||||||
// Create client
|
// Create client
|
||||||
|
|||||||
+59
-2
@@ -35,10 +35,67 @@ func NewMCPTool(manager MCPManager, serverName string, tool *mcp.Tool) *MCPTool
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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
|
// Name returns the tool name, prefixed with the server name
|
||||||
func (t *MCPTool) Name() string {
|
func (t *MCPTool) Name() string {
|
||||||
// Prefix with server name to avoid conflicts
|
// Prefix with server name to avoid conflicts, and sanitize components
|
||||||
return fmt.Sprintf("mcp_%s_%s", t.serverName, t.tool.Name)
|
sanitizedServer := sanitizeIdentifierComponent(t.serverName)
|
||||||
|
sanitizedTool := sanitizeIdentifierComponent(t.tool.Name)
|
||||||
|
return fmt.Sprintf("mcp_%s_%s", sanitizedServer, sanitizedTool)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Description returns the tool description
|
// Description returns the tool description
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
COMPOSE_FILE="docker-compose.full.yml"
|
COMPOSE_FILE="docker/docker-compose.full.yml"
|
||||||
SERVICE="picoclaw-agent"
|
SERVICE="picoclaw-agent"
|
||||||
|
|
||||||
echo "🧪 Testing MCP tools in Docker container (full-featured image)..."
|
echo "🧪 Testing MCP tools in Docker container (full-featured image)..."
|
||||||
@@ -38,8 +38,8 @@ echo "✅ Testing uv..."
|
|||||||
docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'uv --version'
|
docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'uv --version'
|
||||||
|
|
||||||
# Test MCP server installation (quick)
|
# Test MCP server installation (quick)
|
||||||
echo "✅ Testing MCP server install with npx..."
|
echo "✅ Testing @modelcontextprotocol/server-filesystem MCP server install with npx..."
|
||||||
docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'npx -y cowsay "MCP works!"'
|
docker compose -f "$COMPOSE_FILE" run --rm --entrypoint sh "$SERVICE" -c 'npx -y @modelcontextprotocol/server-filesystem --help'
|
||||||
|
|
||||||
echo ""
|
echo ""
|
||||||
echo "🎉 All MCP tools are working correctly!"
|
echo "🎉 All MCP tools are working correctly!"
|
||||||
|
|||||||
Reference in New Issue
Block a user