From a4b5a9eec13077908d8dfe7d7b1a02f59056ea99 Mon Sep 17 00:00:00 2001 From: Mauro Date: Thu, 19 Mar 2026 10:03:17 +0100 Subject: [PATCH] feat(mcp): per server deferred mode (#1654) * feat(mcp): per server deferred mode * fix deferred behavior --- docs/tools_configuration.md | 67 +++++++++++++++++++++++++++------ pkg/agent/loop_mcp.go | 25 ++++++++++++- pkg/agent/loop_mcp_test.go | 75 +++++++++++++++++++++++++++++++++++++ pkg/config/config.go | 4 ++ 4 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 pkg/agent/loop_mcp_test.go diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md index 08746e267..a38f0856f 100644 --- a/docs/tools_configuration.md +++ b/docs/tools_configuration.md @@ -158,7 +158,7 @@ and injected into the context for a configured number of turns (`ttl`). | Config | Type | Default | Description | |----------------------|------|---------|-----------------------------------------------------------------------------------------------------------------------------------| -| `enabled` | bool | false | If true, MCP tools are hidden and loaded on-demand via search. If false, all tools are loaded | +| `enabled` | bool | false | Global default: if `true`, all MCP tools are hidden and loaded on-demand via search; if `false`, all tools are loaded into context. Individual servers can override this with the per-server `deferred` field. | | `ttl` | int | 5 | Number of conversational turns a discovered tool remains unlocked | | `max_search_results` | int | 5 | Maximum number of tools returned per search query | | `use_bm25` | bool | true | Enable the natural language/keyword search tool (`tool_search_tool_bm25`). **Warning**: consumes more resources than regex search | @@ -169,16 +169,17 @@ and injected into the context for a configured number of turns (`ttl`). ### 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 | +| Config | Type | Required | Description | +|------------|---------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `enabled` | bool | yes | Enable this MCP server | +| `deferred` | bool | no | Override deferred mode for this server only. `true` = tools are hidden and discoverable via search; `false` = tools are always visible in context. When omitted, the global `discovery.enabled` value applies. | +| `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 @@ -291,6 +292,50 @@ dynamically only when requested by the user.* } ``` +#### 4) Mixed setup: per-server deferred override + +*Discovery is enabled globally, but `filesystem` is pinned as always-visible while `context7` follows the global +default (deferred). `aws` explicitly opts in to deferred mode even though it is the same as the global default.* + +```json +{ + "tools": { + "mcp": { + "enabled": true, + "discovery": { + "enabled": true, + "ttl": 5, + "max_search_results": 5, + "use_bm25": true + }, + "servers": { + "filesystem": { + "enabled": true, + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-filesystem", "/workspace"], + "deferred": false + }, + "context7": { + "enabled": true, + "command": "npx", + "args": ["-y", "@upstash/context7-mcp"] + }, + "aws": { + "enabled": true, + "command": "npx", + "args": ["-y", "aws-mcp-server"], + "deferred": true + } + } + } + } +} +``` + +> **Tip:** `deferred` on a per-server basis is independent of `discovery.enabled`. You can keep +> `discovery.enabled: false` globally (all tools visible by default) and still mark individual +> high-volume servers as `"deferred": true` to avoid polluting the context with their tools. + ## Skills Tool The skills tool configures skill discovery and installation via registries like ClawHub. diff --git a/pkg/agent/loop_mcp.go b/pkg/agent/loop_mcp.go index 962789a06..97debbc33 100644 --- a/pkg/agent/loop_mcp.go +++ b/pkg/agent/loop_mcp.go @@ -11,6 +11,7 @@ import ( "fmt" "sync" + "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/mcp" "github.com/sipeed/picoclaw/pkg/tools" @@ -111,6 +112,12 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { for serverName, conn := range servers { uniqueTools += len(conn.Tools) + + // Determine whether this server's tools should be deferred (hidden). + // Per-server "deferred" field takes precedence over the global Discovery.Enabled. + serverCfg := al.cfg.Tools.MCP.Servers[serverName] + registerAsHidden := serverIsDeferred(al.cfg.Tools.MCP.Discovery.Enabled, serverCfg) + for _, tool := range conn.Tools { for _, agentID := range agentIDs { agent, ok := al.registry.GetAgent(agentID) @@ -120,7 +127,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) - if al.cfg.Tools.MCP.Discovery.Enabled { + if registerAsHidden { agent.Tools.RegisterHidden(mcpTool) } else { agent.Tools.Register(mcpTool) @@ -133,6 +140,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { "server": serverName, "tool": tool.Name, "name": mcpTool.Name(), + "deferred": registerAsHidden, }) } } @@ -198,3 +206,18 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { return al.mcp.getInitErr() } + +// serverIsDeferred reports whether an MCP server's tools should be registered +// as hidden (deferred/discovery mode). +// +// The per-server Deferred field takes precedence over the global discoveryEnabled +// default. When Deferred is nil, discoveryEnabled is used as the fallback. +func serverIsDeferred(discoveryEnabled bool, serverCfg config.MCPServerConfig) bool { + if !discoveryEnabled { + return false + } + if serverCfg.Deferred != nil { + return *serverCfg.Deferred + } + return true +} diff --git a/pkg/agent/loop_mcp_test.go b/pkg/agent/loop_mcp_test.go new file mode 100644 index 000000000..35c3e49c8 --- /dev/null +++ b/pkg/agent/loop_mcp_test.go @@ -0,0 +1,75 @@ +// PicoClaw - Ultra-lightweight personal AI agent +// Inspired by and based on nanobot: https://github.com/HKUDS/nanobot +// License: MIT +// +// Copyright (c) 2026 PicoClaw contributors + +package agent + +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func boolPtr(b bool) *bool { return &b } + +func TestServerIsDeferred(t *testing.T) { + tests := []struct { + name string + discoveryEnabled bool + serverDeferred *bool + want bool + }{ + // --- global false always wins: per-server deferred is ignored --- + { + name: "global false: per-server deferred=true is ignored", + discoveryEnabled: false, + serverDeferred: boolPtr(true), + want: false, + }, + { + name: "global false: per-server deferred=false stays false", + discoveryEnabled: false, + serverDeferred: boolPtr(false), + want: false, + }, + // --- global true: per-server override applies --- + { + name: "global true: per-server deferred=false opts out", + discoveryEnabled: true, + serverDeferred: boolPtr(false), + want: false, + }, + { + name: "global true: per-server deferred=true stays true", + discoveryEnabled: true, + serverDeferred: boolPtr(true), + want: true, + }, + // --- no per-server override: fall back to global --- + { + name: "no per-server field, global discovery enabled", + discoveryEnabled: true, + serverDeferred: nil, + want: true, + }, + { + name: "no per-server field, global discovery disabled", + discoveryEnabled: false, + serverDeferred: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + serverCfg := config.MCPServerConfig{Deferred: tt.serverDeferred} + got := serverIsDeferred(tt.discoveryEnabled, serverCfg) + if got != tt.want { + t.Errorf("serverIsDeferred(discoveryEnabled=%v, deferred=%v) = %v, want %v", + tt.discoveryEnabled, tt.serverDeferred, got, tt.want) + } + }) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 4e380d431..78b3aa487 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -806,6 +806,10 @@ type ClawHubRegistryConfig struct { type MCPServerConfig struct { // Enabled indicates whether this MCP server is active Enabled bool `json:"enabled"` + // Deferred controls whether this server's tools are registered as hidden (deferred/discovery mode). + // When nil, the global Discovery.Enabled setting applies. + // When explicitly set to true or false, it overrides the global setting for this server only. + Deferred *bool `json:"deferred,omitempty"` // 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