From 765a165475b77e6744631533badb6de170c17f99 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Sun, 29 Mar 2026 23:48:06 +0200 Subject: [PATCH] fix(agent): warn on unknown frontmatter capabilities --- pkg/agent/instance.go | 1 + pkg/agent/tool_allowlist.go | 143 +++++++++++++++++++++++++++++-- pkg/agent/tool_allowlist_test.go | 58 +++++++++++++ 3 files changed, 196 insertions(+), 6 deletions(-) create mode 100644 pkg/agent/tool_allowlist_test.go diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index f95a165af..2df65b905 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -135,6 +135,7 @@ func NewAgentInstance( subagents = agentCfg.Subagents skillsFilter = resolveAgentSkillsFilter(agentCfg, definition) } + warnOnUnknownAgentDeclarations(agentID, workspace, cfg, definition) maxIter := defaults.MaxToolIterations if maxIter == 0 { diff --git a/pkg/agent/tool_allowlist.go b/pkg/agent/tool_allowlist.go index f7434c188..b220f1903 100644 --- a/pkg/agent/tool_allowlist.go +++ b/pkg/agent/tool_allowlist.go @@ -3,8 +3,144 @@ package agent import ( "sort" "strings" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/logger" ) +const dynamicMCPToolPrefix = "mcp_" + +func warnOnUnknownAgentDeclarations( + agentID, workspace string, + cfg *config.Config, + definition AgentContextDefinition, +) { + if cfg == nil || frontmatterParseFailed(definition) { + return + } + + if unknownTools := unknownAgentToolNames(cfg, definition); len(unknownTools) > 0 { + logger.WarnCF("agent", "AGENT.md declares unknown tool names", + map[string]any{ + "agent_id": agentID, + "workspace": workspace, + "tools": unknownTools, + }) + } + + if unknownServers := unknownAgentMCPServerNames(cfg, definition); len(unknownServers) > 0 { + logger.WarnCF("agent", "AGENT.md declares unknown MCP server names", + map[string]any{ + "agent_id": agentID, + "workspace": workspace, + "mcp_servers": unknownServers, + }) + } +} + +func unknownAgentToolNames(cfg *config.Config, definition AgentContextDefinition) []string { + if definition.Agent == nil || definition.Agent.Frontmatter.Tools == nil { + return nil + } + + known := knownRuntimeToolNames(cfg) + unknown := make(map[string]struct{}) + for _, raw := range definition.Agent.Frontmatter.Tools { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) { + continue + } + if _, ok := known[name]; ok { + continue + } + unknown[name] = struct{}{} + } + + return sortedKeys(unknown) +} + +func unknownAgentMCPServerNames(cfg *config.Config, definition AgentContextDefinition) []string { + if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPServers == nil { + return nil + } + + unknown := make(map[string]struct{}) + for _, raw := range definition.Agent.Frontmatter.MCPServers { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" { + continue + } + if _, ok := cfg.Tools.MCP.Servers[name]; ok { + continue + } + unknown[name] = struct{}{} + } + + return sortedKeys(unknown) +} + +func knownRuntimeToolNames(cfg *config.Config) map[string]struct{} { + known := make(map[string]struct{}) + if cfg == nil { + return known + } + + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("read_file"), "read_file") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("write_file"), "write_file") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("list_dir"), "list_dir") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("exec"), "exec") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("edit_file"), "edit_file") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("append_file"), "append_file") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("cron"), "cron") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("web"), "web_search") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("web_fetch"), "web_fetch") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("i2c"), "i2c") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("spi"), "spi") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("message"), "message") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("send_file"), "send_file") + addKnownToolIfEnabled( + known, + cfg.Tools.IsToolEnabled("skills") && cfg.Tools.IsToolEnabled("find_skills"), + "find_skills", + ) + addKnownToolIfEnabled( + known, + cfg.Tools.IsToolEnabled("skills") && cfg.Tools.IsToolEnabled("install_skill"), + "install_skill", + ) + if cfg.Tools.IsToolEnabled("subagent") { + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("spawn"), "spawn") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("subagent"), "subagent") + addKnownToolIfEnabled(known, cfg.Tools.IsToolEnabled("spawn_status"), "spawn_status") + } + if cfg.Tools.IsToolEnabled("mcp") && cfg.Tools.MCP.Discovery.Enabled { + addKnownToolIfEnabled(known, cfg.Tools.MCP.Discovery.UseRegex, "tool_search_tool_regex") + addKnownToolIfEnabled(known, cfg.Tools.MCP.Discovery.UseBM25, "tool_search_tool_bm25") + } + + return known +} + +func addKnownToolIfEnabled(known map[string]struct{}, enabled bool, name string) { + if !enabled { + return + } + known[name] = struct{}{} +} + +func sortedKeys(values map[string]struct{}) []string { + if len(values) == 0 { + return nil + } + + result := make([]string, 0, len(values)) + for value := range values { + result = append(result, value) + } + sort.Strings(result) + return result +} + func resolveAgentToolAllowlist(definition AgentContextDefinition) []string { if frontmatterParseFailed(definition) { return []string{} @@ -22,12 +158,7 @@ func resolveAgentToolAllowlist(definition AgentContextDefinition) []string { allowlist[trimmed] = struct{}{} } - result := make([]string, 0, len(allowlist)) - for name := range allowlist { - result = append(result, name) - } - sort.Strings(result) - return result + return sortedKeys(allowlist) } func resolveAgentMCPServerAllowlist(definition AgentContextDefinition) map[string]struct{} { diff --git a/pkg/agent/tool_allowlist_test.go b/pkg/agent/tool_allowlist_test.go new file mode 100644 index 000000000..059ee9344 --- /dev/null +++ b/pkg/agent/tool_allowlist_test.go @@ -0,0 +1,58 @@ +package agent + +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestUnknownAgentToolNames(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [read_file, web_serach, mcp_github_search] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Tools: config.ToolsConfig{ + ReadFile: config.ReadFileToolConfig{Enabled: true}, + Web: config.WebToolsConfig{ + ToolConfig: config.ToolConfig{Enabled: true}, + }, + }, + } + + unknown := unknownAgentToolNames(cfg, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "web_serach" { + t.Fatalf("unknownAgentToolNames() = %v, want [web_serach]", unknown) + } +} + +func TestUnknownAgentMCPServerNames(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +mcpServers: [github, githb] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + Servers: map[string]config.MCPServerConfig{ + "github": {Enabled: true}, + }, + }, + }, + } + + unknown := unknownAgentMCPServerNames(cfg, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "githb" { + t.Fatalf("unknownAgentMCPServerNames() = %v, want [githb]", unknown) + } +}