diff --git a/docs/configuration.md b/docs/configuration.md index 3462767e6..9c201c787 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -246,6 +246,88 @@ In other words: **channel + account form the candidate set; peer/guild/team then - **Wildcard catches too much traffic?** Add more specific `peer/guild/team` rules for critical paths. - **Unexpected default fallback?** Confirm `agent_id` exists and is not misspelled. +### Agent Tool Allowlist + +You can restrict an individual agent to a subset of runtime tools with `agents.list[].tools`. + +If `tools` is omitted, the agent gets the normal globally enabled tool set. If `tools` is present, PicoClaw registers only the listed tools for that agent. + +```json +{ + "agents": { + "list": [ + { + "id": "research", + "name": "Research Agent", + "tools": ["read_file", "write_file", "web_search", "web_fetch", "message"] + } + ] + } +} +``` + +Notes: + +- This is an allowlist, not a preference hint. +- Tool names are matched against the runtime tool name 1:1. +- Use runtime tool names such as `web_search`, `web_fetch`, `spawn`, `subagent`, `send_file`. +- The `available_tools` field in Agent Discovery reflects the filtered runtime result. + +### Agent Discovery (Automatic) + +When more than one agent exists, PicoClaw injects a structured agent registry into each agent's system prompt on every turn. No extra `list_agents` tool call is required. + +This registry is intended to make delegation concrete and reliable, especially when using `spawn` with a target `agent_id`. + +Each entry includes: + +| Field | Meaning | +|-------|---------| +| `id` | Stable agent id | +| `name` | Human-friendly agent name | +| `description` | Short capability summary | +| `model` | Current model used by that agent | +| `available_tools` | Tool names currently visible to that agent | +| `channels` | Channels that route to that agent | + +Important behavior: + +- The discovery section includes the current agent's own entry, so the model has self-awareness. +- `available_tools` is the most important field for delegation. It reflects the tools the target agent can actually use, not just a natural-language description. +- `description` is sourced from `AGENT.md` frontmatter `description` when available, otherwise from the first meaningful paragraph of `AGENT.md`, and finally `SOUL.md`. +- `name` comes from `agents.list[].name` first, then `AGENT.md` frontmatter `name`, then falls back to the agent id. +- `channels` come from routing state: + - the default agent exposes enabled channels + - other agents expose channels that explicitly bind to them through `bindings` + +Example injected shape: + +```json +{ + "current_agent_id": "main", + "agents": [ + { + "id": "main", + "name": "Main Assistant", + "description": "Generalist agent for day-to-day requests.", + "model": "gpt-4o-mini", + "available_tools": ["read_file", "write_file", "exec", "spawn"], + "channels": ["telegram", "discord"] + }, + { + "id": "research", + "name": "Research Agent", + "description": "Specialist for long-form investigation and web work.", + "model": "claude-sonnet-4.5", + "available_tools": ["web_search", "web_fetch", "read_file"], + "channels": ["telegram"] + } + ] +} +``` + +In practice, this means a generalist agent can see that a peer has `["web_search", "web_fetch"]` while it only has local file tools, and can decide to delegate to that peer instead of guessing. + ### 🔒 Security Sandbox PicoClaw runs in a sandboxed environment by default. The agent can only access files and execute commands within the configured workspace. diff --git a/docs/it/configuration.md b/docs/it/configuration.md index 6a79a9543..9b0d4a198 100644 --- a/docs/it/configuration.md +++ b/docs/it/configuration.md @@ -42,14 +42,13 @@ PicoClaw salva i dati nel workspace configurato (predefinito: `~/.picoclaw/works ├── state/ # Stato persistente (ultimo canale, ecc.) ├── cron/ # Database dei job pianificati ├── skills/ # Skill personalizzate -├── AGENTS.md # Guida al comportamento dell'agent +├── AGENT.md # Guida al comportamento dell'agent ├── HEARTBEAT.md # Prompt per task periodici (controllato ogni 30 min) -├── IDENTITY.md # Identità dell'agent ├── SOUL.md # Anima dell'agent └── USER.md # Preferenze dell'utente ``` -> **Nota:** Le modifiche a `AGENTS.md`, `SOUL.md`, `USER.md`, `IDENTITY.md` e `memory/MEMORY.md` vengono rilevate automaticamente a runtime tramite il tracciamento della data di modifica (mtime). **Non è necessario riavviare il gateway** dopo aver modificato questi file — l'agent caricherà il nuovo contenuto alla prossima richiesta. +> **Nota:** Le modifiche a `AGENT.md`, `SOUL.md`, `USER.md` e `memory/MEMORY.md` vengono rilevate automaticamente a runtime tramite il tracciamento della data di modifica (mtime). **Non è necessario riavviare il gateway** dopo aver modificato questi file — l'agent caricherà il nuovo contenuto alla prossima richiesta. ### Sorgenti delle Skill @@ -72,6 +71,88 @@ export PICOCLAW_BUILTIN_SKILLS=/path/to/skills - Un comando slash sconosciuto (ad esempio `/foo`) viene passato all'elaborazione LLM come se fosse un messaggio dell'utente. - Un comando registrato ma non supportato sul canale corrente (ad esempio `/show` su WhatsApp) restituisce un errore esplicito all'utente e interrompe l'elaborazione. +### Allowlist dei Tool per Agent + +Puoi limitare un singolo agent a un sottoinsieme di tool runtime con `agents.list[].tools`. + +Se `tools` è omesso, l'agent riceve il normale set globale dei tool abilitati. Se `tools` è presente, PicoClaw registra per quell'agent solo i tool elencati. + +```json +{ + "agents": { + "list": [ + { + "id": "research", + "name": "Research Agent", + "tools": ["read_file", "write_file", "web_search", "web_fetch", "message"] + } + ] + } +} +``` + +Note: + +- È una allowlist reale, non un suggerimento per l'LLM. +- I nomi dei tool fanno match 1:1 con il nome runtime del tool. +- Se ti serve controllo preciso, usa i nomi runtime effettivi come `web_search`, `web_fetch`, `spawn`, `subagent`, `send_file`. +- Il campo `available_tools` nella Agent Discovery riflette il risultato filtrato reale. + +### Discovery Multi-Agent (Automatica) + +Quando esiste più di un agent, PicoClaw inietta automaticamente nel system prompt di ogni agent un registry strutturato dei peer. Non serve una chiamata aggiuntiva a un tool `list_agents`. + +Questa discovery serve soprattutto a rendere affidabile la delega tramite `spawn` con `agent_id` esplicito. + +Ogni entry include: + +| Campo | Significato | +|-------|-------------| +| `id` | ID stabile dell'agent | +| `name` | Nome leggibile dell'agent | +| `description` | Riassunto breve delle capacità | +| `model` | Modello attualmente usato da quell'agent | +| `available_tools` | Tool attualmente visibili a quell'agent | +| `channels` | Canali instradati verso quell'agent | + +Dettagli importanti: + +- La sezione include anche l'entry dell'agent corrente, quindi c'è self-awareness. +- `available_tools` è il campo più importante per delegare bene: l'LLM vede i tool reali del peer, non deve indovinarli dalla sola descrizione. +- `description` viene presa da `AGENT.md` frontmatter `description` quando presente; altrimenti dal primo paragrafo utile di `AGENT.md`, e in fallback da `SOUL.md`. +- `name` arriva prima da `agents.list[].name`, poi da `AGENT.md` frontmatter `name`, e in fallback dall'ID dell'agent. +- `channels` derivano dal routing: + - l'agent di default espone i canali abilitati + - gli altri agent espongono i canali che hanno un binding esplicito verso di loro + +Forma dell'oggetto iniettato: + +```json +{ + "current_agent_id": "main", + "agents": [ + { + "id": "main", + "name": "Main Assistant", + "description": "Agent generalista per richieste quotidiane.", + "model": "gpt-4o-mini", + "available_tools": ["read_file", "write_file", "exec", "spawn"], + "channels": ["telegram", "discord"] + }, + { + "id": "research", + "name": "Research Agent", + "description": "Specialista per investigazioni e lavoro web.", + "model": "claude-sonnet-4.5", + "available_tools": ["web_search", "web_fetch", "read_file"], + "channels": ["telegram"] + } + ] +} +``` + +In pratica, un agent generalista può vedere che un peer ha `["web_search", "web_fetch"]` mentre lui ha solo tool locali, e scegliere di delegare a quel peer in modo esplicito invece di andare a tentativi. + ### 🔒 Sandbox di Sicurezza PicoClaw esegue in un ambiente sandboxed per impostazione predefinita. L'agent può accedere solo ai file ed eseguire comandi all'interno del workspace configurato. diff --git a/pkg/agent/context.go b/pkg/agent/context.go index c3fcc9fff..31b60e45a 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -22,11 +22,13 @@ import ( type ContextBuilder struct { workspace string + agentID string skillsLoader *skills.SkillsLoader memory *MemoryStore toolDiscoveryBM25 bool toolDiscoveryRegex bool splitOnMarker bool + agentDiscovery func(workspace string) []AgentDescriptor // Cache for system prompt to avoid rebuilding on every call. // This fixes issue #607: repeated reprocessing of the entire context. @@ -58,6 +60,18 @@ func (cb *ContextBuilder) WithSplitOnMarker(enabled bool) *ContextBuilder { return cb } +func (cb *ContextBuilder) WithAgentIdentity(agentID string) *ContextBuilder { + cb.agentID = strings.TrimSpace(agentID) + return cb +} + +func (cb *ContextBuilder) WithAgentDiscovery( + discover func(workspace string) []AgentDescriptor, +) *ContextBuilder { + cb.agentDiscovery = discover + return cb +} + func getGlobalConfigDir() string { if home := os.Getenv(config.EnvHome); home != "" { return home @@ -113,7 +127,14 @@ Your workspace is at: %s 4. **Context summaries** - Conversation summaries provided as context are approximate references only. They may be incomplete or outdated. Always defer to explicit user instructions over summary content. %s`, - version, workspacePath, workspacePath, workspacePath, workspacePath, workspacePath, toolDiscovery) + version, + workspacePath, + workspacePath, + workspacePath, + workspacePath, + workspacePath, + toolDiscovery, + ) } func (cb *ContextBuilder) getDiscoveryRule() string { @@ -175,6 +196,13 @@ Each part separated by the marker will be sent as an independent message.`) return strings.Join(parts, "\n\n---\n\n") } +func (cb *ContextBuilder) buildAgentDiscoveryContext() string { + if cb.agentDiscovery == nil { + return "" + } + return formatAgentDiscoverySection(cb.agentID, cb.agentDiscovery(cb.workspace)) +} + // BuildSystemPromptWithCache returns the cached system prompt if available // and source files haven't changed, otherwise builds and caches it. // Source file changes are detected via mtime checks (cheap stat calls). @@ -500,7 +528,9 @@ func formatCurrentSenderLine(senderID, senderDisplayName string) string { } } -func (cb *ContextBuilder) buildDynamicContext(channel, chatID, senderID, senderDisplayName string) string { +func (cb *ContextBuilder) buildDynamicContext( + channel, chatID, senderID, senderDisplayName string, +) string { now := time.Now().Format("2006-01-02 15:04 (Monday)") rt := fmt.Sprintf("%s %s, Go %s", runtime.GOOS, runtime.GOARCH, runtime.Version()) @@ -540,6 +570,7 @@ func (cb *ContextBuilder) BuildMessages( // Build short dynamic context (time, runtime, session) — changes per request dynamicCtx := cb.buildDynamicContext(channel, chatID, senderID, senderDisplayName) + discoveryCtx := cb.buildAgentDiscoveryContext() // Compose a single system message: static (cached) + dynamic + optional summary. // Keeping all system content in one message ensures every provider adapter can @@ -550,16 +581,33 @@ func (cb *ContextBuilder) BuildMessages( // cache-aware adapters (Anthropic) can set per-block cache_control. // The static block is marked "ephemeral" — its prefix hash is stable // across requests, enabling LLM-side KV cache reuse. - stringParts := []string{staticPrompt, dynamicCtx} + stringParts := []string{staticPrompt} contentBlocks := []providers.ContentBlock{ - {Type: "text", Text: staticPrompt, CacheControl: &providers.CacheControl{Type: "ephemeral"}}, - {Type: "text", Text: dynamicCtx}, + { + Type: "text", + Text: staticPrompt, + CacheControl: &providers.CacheControl{Type: "ephemeral"}, + }, } + if discoveryCtx != "" { + stringParts = append(stringParts, discoveryCtx) + contentBlocks = append( + contentBlocks, + providers.ContentBlock{Type: "text", Text: discoveryCtx}, + ) + } + + stringParts = append(stringParts, dynamicCtx) + contentBlocks = append(contentBlocks, providers.ContentBlock{Type: "text", Text: dynamicCtx}) + if skillsText := cb.buildActiveSkillsContext(activeSkills); skillsText != "" { stringParts = append(stringParts, skillsText) - contentBlocks = append(contentBlocks, providers.ContentBlock{Type: "text", Text: skillsText}) + contentBlocks = append( + contentBlocks, + providers.ContentBlock{Type: "text", Text: skillsText}, + ) } if summary != "" { @@ -568,7 +616,10 @@ func (cb *ContextBuilder) BuildMessages( "for reference only. It may be incomplete or outdated — always defer to explicit instructions.\n\n%s", summary) stringParts = append(stringParts, summaryText) - contentBlocks = append(contentBlocks, providers.ContentBlock{Type: "text", Text: summaryText}) + contentBlocks = append( + contentBlocks, + providers.ContentBlock{Type: "text", Text: summaryText}, + ) } fullSystemPrompt := strings.Join(stringParts, "\n\n---\n\n") @@ -667,7 +718,11 @@ func sanitizeHistoryForProvider(history []providers.Message) []providers.Message case "assistant": if len(msg.ToolCalls) > 0 { if len(sanitized) == 0 { - logger.DebugCF("agent", "Dropping assistant tool-call turn at history start", map[string]any{}) + logger.DebugCF( + "agent", + "Dropping assistant tool-call turn at history start", + map[string]any{}, + ) continue } prev := sanitized[len(sanitized)-1] diff --git a/pkg/agent/context_budget_test.go b/pkg/agent/context_budget_test.go index 870f0fbe6..c8993746f 100644 --- a/pkg/agent/context_budget_test.go +++ b/pkg/agent/context_budget_test.go @@ -500,8 +500,11 @@ func TestEstimateMessageTokens_ReasoningContent(t *testing.T) { reasoningTokens := estimateMessageTokens(withReasoning) if reasoningTokens <= plainTokens { - t.Errorf("message with ReasoningContent (%d tokens) should exceed plain message (%d tokens)", - reasoningTokens, plainTokens) + t.Errorf( + "message with ReasoningContent (%d tokens) should exceed plain message (%d tokens)", + reasoningTokens, + plainTokens, + ) } } @@ -764,7 +767,11 @@ func TestEstimateMessageTokens_WithReasoningAndMedia(t *testing.T) { tokensNoReasoning := estimateMessageTokens(msgNoReasoning) if tokens <= tokensNoReasoning { - t.Errorf("reasoning content should add tokens: with=%d, without=%d", tokens, tokensNoReasoning) + t.Errorf( + "reasoning content should add tokens: with=%d, without=%d", + tokens, + tokensNoReasoning, + ) } } diff --git a/pkg/agent/context_cache_test.go b/pkg/agent/context_cache_test.go index 81a1534b9..ae6ff18cc 100644 --- a/pkg/agent/context_cache_test.go +++ b/pkg/agent/context_cache_test.go @@ -82,7 +82,16 @@ func TestSingleSystemMessage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - msgs := cb.BuildMessages(tt.history, tt.summary, tt.message, nil, "test", "chat1", "", "") + msgs := cb.BuildMessages( + tt.history, + tt.summary, + tt.message, + nil, + "test", + "chat1", + "", + "", + ) systemCount := 0 for _, m := range msgs { @@ -168,7 +177,16 @@ func TestBuildMessages_CurrentSenderDynamicContext(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - msgs := cb.BuildMessages(nil, "", "hello", nil, "discord", "chat1", tt.senderID, tt.senderDisplayName) + msgs := cb.BuildMessages( + nil, + "", + "hello", + nil, + "discord", + "chat1", + tt.senderID, + tt.senderDisplayName, + ) sys := msgs[0].Content if tt.wantSection { @@ -382,7 +400,10 @@ func TestNewFileCreationInvalidatesCache(t *testing.T) { // Cache should auto-invalidate because file went from absent -> present sp2 := cb.BuildSystemPromptWithCache() if !strings.Contains(sp2, tt.checkField) { - t.Errorf("cache not invalidated on new file creation: expected %q in prompt", tt.checkField) + t.Errorf( + "cache not invalidated on new file creation: expected %q in prompt", + tt.checkField, + ) } }) } diff --git a/pkg/agent/context_test.go b/pkg/agent/context_test.go index 0d7948eef..c3b9ed6a0 100644 --- a/pkg/agent/context_test.go +++ b/pkg/agent/context_test.go @@ -151,7 +151,19 @@ func TestSanitizeHistoryForProvider_MultiToolCallsThenNewRound(t *testing.T) { if len(result) != 9 { t.Fatalf("expected 9 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "assistant", "tool", "tool", "assistant", "user", "assistant", "tool", "assistant") + assertRoles( + t, + result, + "user", + "assistant", + "tool", + "tool", + "assistant", + "user", + "assistant", + "tool", + "assistant", + ) } func TestSanitizeHistoryForProvider_ConsecutiveMultiToolRounds(t *testing.T) { @@ -170,7 +182,18 @@ func TestSanitizeHistoryForProvider_ConsecutiveMultiToolRounds(t *testing.T) { if len(result) != 8 { t.Fatalf("expected 8 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "assistant", "tool", "tool", "assistant", "tool", "tool", "assistant") + assertRoles( + t, + result, + "user", + "assistant", + "tool", + "tool", + "assistant", + "tool", + "tool", + "assistant", + ) } func TestSanitizeHistoryForProvider_PlainConversation(t *testing.T) { @@ -304,5 +327,17 @@ func TestSanitizeHistoryForProvider_PartialToolResultsInMiddle(t *testing.T) { if len(result) != 9 { t.Fatalf("expected 9 messages, got %d: %+v", len(result), roles(result)) } - assertRoles(t, result, "user", "assistant", "tool", "assistant", "user", "user", "assistant", "tool", "assistant") + assertRoles( + t, + result, + "user", + "assistant", + "tool", + "assistant", + "user", + "user", + "assistant", + "tool", + "assistant", + ) } diff --git a/pkg/agent/definition_test.go b/pkg/agent/definition_test.go index 5ee996967..b3068d134 100644 --- a/pkg/agent/definition_test.go +++ b/pkg/agent/definition_test.go @@ -61,8 +61,12 @@ Act directly and use tools first. if len(definition.Agent.Frontmatter.Skills) != 2 { t.Fatalf("expected skills to be parsed, got %v", definition.Agent.Frontmatter.Skills) } - if len(definition.Agent.Frontmatter.MCPServers) != 1 || definition.Agent.Frontmatter.MCPServers[0] != "github" { - t.Fatalf("expected mcpServers to be parsed, got %v", definition.Agent.Frontmatter.MCPServers) + if len(definition.Agent.Frontmatter.MCPServers) != 1 || + definition.Agent.Frontmatter.MCPServers[0] != "github" { + t.Fatalf( + "expected mcpServers to be parsed, got %v", + definition.Agent.Frontmatter.MCPServers, + ) } if definition.Agent.Frontmatter.Fields["metadata"] == nil { t.Fatal("expected arbitrary frontmatter fields to remain available") @@ -96,7 +100,10 @@ func TestLoadAgentDefinitionFallsBackToLegacyAgentsMarkdown(t *testing.T) { t.Fatal("expected AGENTS.md to be loaded") } if definition.Agent.RawFrontmatter != "" { - t.Fatalf("legacy AGENTS.md should not have frontmatter, got %q", definition.Agent.RawFrontmatter) + t.Fatalf( + "legacy AGENTS.md should not have frontmatter, got %q", + definition.Agent.RawFrontmatter, + ) } if !strings.Contains(definition.Agent.Body, "Keep compatibility") { t.Fatalf("expected legacy body to be preserved, got %q", definition.Agent.Body) @@ -159,7 +166,10 @@ Keep going. len(definition.Agent.Frontmatter.Skills) != 0 || len(definition.Agent.Frontmatter.MCPServers) != 0 || len(definition.Agent.Frontmatter.Fields) != 0 { - t.Fatalf("expected invalid frontmatter to decode as empty struct, got %+v", definition.Agent.Frontmatter) + t.Fatalf( + "expected invalid frontmatter to decode as empty struct, got %+v", + definition.Agent.Frontmatter, + ) } } diff --git a/pkg/agent/discovery.go b/pkg/agent/discovery.go new file mode 100644 index 000000000..b630abd60 --- /dev/null +++ b/pkg/agent/discovery.go @@ -0,0 +1,341 @@ +package agent + +import ( + "encoding/json" + "fmt" + "path/filepath" + "sort" + "strings" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/routing" +) + +// AgentDescriptor is the structured discovery payload injected into each +// agent's system prompt so the LLM can make concrete delegation decisions. +type AgentDescriptor struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Model string `json:"model"` + AvailableTools []string `json:"available_tools"` + Channels []string `json:"channels"` +} + +// ListAgents returns structured descriptors for every agent in the current +// PicoClaw instance. The current workspace, when provided, is used only to +// order the matching agent first for prompt readability. +func (r *AgentRegistry) ListAgents(workspace string) []AgentDescriptor { + r.mu.RLock() + defer r.mu.RUnlock() + + ids := make([]string, 0, len(r.agents)) + for id := range r.agents { + ids = append(ids, id) + } + sort.Strings(ids) + + selfWorkspace := cleanWorkspacePath(workspace) + descriptors := make([]AgentDescriptor, 0, len(ids)) + for _, id := range ids { + agent := r.agents[id] + if agent == nil { + continue + } + descriptors = append(descriptors, r.buildAgentDescriptorLocked(agent)) + } + + if selfWorkspace == "" { + return descriptors + } + + sort.SliceStable(descriptors, func(i, j int) bool { + leftSelf := cleanWorkspacePath( + r.workspaceForAgentIDLocked(descriptors[i].ID), + ) == selfWorkspace + rightSelf := cleanWorkspacePath( + r.workspaceForAgentIDLocked(descriptors[j].ID), + ) == selfWorkspace + if leftSelf != rightSelf { + return leftSelf + } + return descriptors[i].ID < descriptors[j].ID + }) + + return descriptors +} + +// GetAgentDescriptor returns the structured discovery payload for one agent. +func (r *AgentRegistry) GetAgentDescriptor(agentID string) (*AgentDescriptor, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + + id := routing.NormalizeAgentID(agentID) + agent, ok := r.agents[id] + if !ok || agent == nil { + return nil, false + } + + descriptor := r.buildAgentDescriptorLocked(agent) + return &descriptor, true +} + +func (r *AgentRegistry) buildAgentDescriptorLocked(agent *AgentInstance) AgentDescriptor { + definition := loadAgentDefinition(agent.Workspace) + name := strings.TrimSpace(agent.Name) + if name == "" && definition.Agent != nil { + name = strings.TrimSpace(definition.Agent.Frontmatter.Name) + } + if name == "" { + name = agent.ID + } + + return AgentDescriptor{ + ID: agent.ID, + Name: name, + Description: agentDescriptionFromDefinition(definition), + Model: strings.TrimSpace(agent.Model), + AvailableTools: visibleToolNames(agent), + Channels: r.channelsForAgentLocked(agent.ID), + } +} + +func visibleToolNames(agent *AgentInstance) []string { + if agent == nil || agent.Tools == nil { + return []string{} + } + + defs := agent.Tools.ToProviderDefs() + names := make([]string, 0, len(defs)) + for _, def := range defs { + name := strings.TrimSpace(def.Function.Name) + if name == "" { + continue + } + names = append(names, name) + } + if names == nil { + return []string{} + } + return names +} + +func agentDescriptionFromDefinition(definition AgentContextDefinition) string { + if definition.Agent != nil { + if desc := strings.TrimSpace(definition.Agent.Frontmatter.Description); desc != "" { + return desc + } + if desc := firstMeaningfulParagraph(definition.Agent.Body); desc != "" { + return desc + } + } + if definition.Soul != nil { + if desc := firstMeaningfulParagraph(definition.Soul.Content); desc != "" { + return desc + } + } + return "" +} + +func firstMeaningfulParagraph(content string) string { + content = strings.ReplaceAll(content, "\r\n", "\n") + paragraphs := strings.Split(content, "\n\n") + for _, paragraph := range paragraphs { + lines := strings.Split(paragraph, "\n") + parts := make([]string, 0, len(lines)) + inFence := false + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "```") { + inFence = !inFence + continue + } + if inFence || trimmed == "" { + continue + } + if strings.HasPrefix(trimmed, "#") { + continue + } + if strings.HasPrefix(trimmed, "- ") || strings.HasPrefix(trimmed, "* ") { + trimmed = strings.TrimSpace(trimmed[2:]) + } + parts = append(parts, trimmed) + } + if len(parts) == 0 { + continue + } + return strings.Join(parts, " ") + } + return "" +} + +func (r *AgentRegistry) channelsForAgentLocked(agentID string) []string { + channels := make(map[string]struct{}) + + if defaultID := r.defaultAgentIDLocked(); defaultID != "" && defaultID == agentID { + for _, channel := range enabledChannels(r.cfg) { + channels[channel] = struct{}{} + } + } + + if r.cfg != nil { + for _, binding := range r.cfg.Bindings { + if routing.NormalizeAgentID(binding.AgentID) != agentID { + continue + } + channel := strings.ToLower(strings.TrimSpace(binding.Match.Channel)) + if channel == "" { + continue + } + channels[channel] = struct{}{} + } + } + + if len(channels) == 0 { + return []string{} + } + + result := make([]string, 0, len(channels)) + for channel := range channels { + result = append(result, channel) + } + sort.Strings(result) + return result +} + +func enabledChannels(cfg *config.Config) []string { + if cfg == nil { + return []string{} + } + + enabled := make([]string, 0, 16) + if cfg.Channels.WhatsApp.Enabled { + enabled = append(enabled, "whatsapp") + } + if cfg.Channels.Telegram.Enabled { + enabled = append(enabled, "telegram") + } + if cfg.Channels.Feishu.Enabled { + enabled = append(enabled, "feishu") + } + if cfg.Channels.Discord.Enabled { + enabled = append(enabled, "discord") + } + if cfg.Channels.MaixCam.Enabled { + enabled = append(enabled, "maixcam") + } + if cfg.Channels.QQ.Enabled { + enabled = append(enabled, "qq") + } + if cfg.Channels.DingTalk.Enabled { + enabled = append(enabled, "dingtalk") + } + if cfg.Channels.Slack.Enabled { + enabled = append(enabled, "slack") + } + if cfg.Channels.Matrix.Enabled { + enabled = append(enabled, "matrix") + } + if cfg.Channels.LINE.Enabled { + enabled = append(enabled, "line") + } + if cfg.Channels.OneBot.Enabled { + enabled = append(enabled, "onebot") + } + if cfg.Channels.WeCom.Enabled { + enabled = append(enabled, "wecom") + } + if cfg.Channels.Weixin.Enabled { + enabled = append(enabled, "weixin") + } + if cfg.Channels.Pico.Enabled { + enabled = append(enabled, "pico") + } + if cfg.Channels.PicoClient.Enabled { + enabled = append(enabled, "pico_client") + } + if cfg.Channels.IRC.Enabled { + enabled = append(enabled, "irc") + } + return enabled +} + +func (r *AgentRegistry) workspaceForAgentIDLocked(agentID string) string { + agent, ok := r.agents[routing.NormalizeAgentID(agentID)] + if !ok || agent == nil { + return "" + } + return agent.Workspace +} + +func (r *AgentRegistry) defaultAgentIDLocked() string { + if _, ok := r.agents[routing.DefaultAgentID]; ok { + return routing.DefaultAgentID + } + if r.cfg != nil && len(r.cfg.Agents.List) > 0 { + for _, agentCfg := range r.cfg.Agents.List { + if !agentCfg.Default { + continue + } + id := routing.NormalizeAgentID(agentCfg.ID) + if _, ok := r.agents[id]; ok { + return id + } + } + id := routing.NormalizeAgentID(r.cfg.Agents.List[0].ID) + if _, ok := r.agents[id]; ok { + return id + } + } + for id := range r.agents { + return id + } + return "" +} + +func cleanWorkspacePath(path string) string { + path = strings.TrimSpace(path) + if path == "" { + return "" + } + return filepath.Clean(path) +} + +func formatAgentDiscoverySection(currentAgentID string, agents []AgentDescriptor) string { + if len(agents) <= 1 { + return "" + } + + payload := struct { + CurrentAgentID string `json:"current_agent_id"` + Agents []AgentDescriptor `json:"agents"` + }{ + CurrentAgentID: strings.TrimSpace(currentAgentID), + Agents: agents, + } + + encoded, err := json.MarshalIndent(payload, "", " ") + if err != nil { + return "" + } + + var header strings.Builder + header.WriteString("# Agent Discovery\n\n") + if payload.CurrentAgentID != "" { + fmt.Fprintf( + &header, + "You are agent %q. This registry is authoritative for the current PicoClaw instance and includes your own entry.\n", + payload.CurrentAgentID, + ) + } else { + header.WriteString("This registry is authoritative for the current PicoClaw instance.\n") + } + header.WriteString( + "Delegate based on available_tools first, then model, channels, and description. Use only agent IDs listed here.\n\n", + ) + header.WriteString("```json\n") + header.Write(encoded) + header.WriteString("\n```") + + return header.String() +} diff --git a/pkg/agent/discovery_test.go b/pkg/agent/discovery_test.go new file mode 100644 index 000000000..a44f67dea --- /dev/null +++ b/pkg/agent/discovery_test.go @@ -0,0 +1,211 @@ +package agent + +import ( + "slices" + "strings" + "testing" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestAgentRegistry_ListAgentsBuildsStructuredDescriptors(t *testing.T) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +name: Main Frontmatter Name +description: Structured main agent +--- +# Agent + +Handle general requests. +`, + }) + defer cleanupWorkspace(t, mainWorkspace) + + supportWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `# Agent + +Handle support tickets carefully. +`, + "SOUL.md": "# Soul\nStay calm and precise.", + }) + defer cleanupWorkspace(t, supportWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Name: "Configured Main", Workspace: mainWorkspace}, + { + ID: "support", + Workspace: supportWorkspace, + Model: &config.AgentModelConfig{Primary: "support-model"}, + }, + }) + cfg.Tools.ReadFile.Enabled = true + cfg.Tools.WriteFile.Enabled = true + cfg.Channels.Telegram.Enabled = true + cfg.Bindings = []config.AgentBinding{ + { + AgentID: "support", + Match: config.BindingMatch{ + Channel: "telegram", + AccountID: "*", + }, + }, + } + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + + descriptors := registry.ListAgents(mainWorkspace) + if len(descriptors) != 2 { + t.Fatalf("expected 2 descriptors, got %d", len(descriptors)) + } + + if descriptors[0].ID != "main" { + t.Fatalf("expected current workspace agent first, got %q", descriptors[0].ID) + } + if descriptors[0].Name != "Configured Main" { + t.Fatalf("expected config name to win, got %q", descriptors[0].Name) + } + if descriptors[0].Description != "Structured main agent" { + t.Fatalf("expected frontmatter description, got %q", descriptors[0].Description) + } + if descriptors[0].Model != "gpt-4" { + t.Fatalf("expected inherited model, got %q", descriptors[0].Model) + } + if !slices.Contains(descriptors[0].AvailableTools, "read_file") || + !slices.Contains(descriptors[0].AvailableTools, "write_file") { + t.Fatalf("expected visible file tools in descriptor, got %v", descriptors[0].AvailableTools) + } + if !slices.Equal(descriptors[0].Channels, []string{"telegram"}) { + t.Fatalf( + "expected default agent to cover enabled telegram channel, got %v", + descriptors[0].Channels, + ) + } + + support, ok := registry.GetAgentDescriptor("support") + if !ok || support == nil { + t.Fatal("expected support descriptor lookup to succeed") + } + if support.Description != "Handle support tickets carefully." { + t.Fatalf("expected AGENT body fallback description, got %q", support.Description) + } + if support.Model != "support-model" { + t.Fatalf("expected explicit support model, got %q", support.Model) + } + if !slices.Equal(support.Channels, []string{"telegram"}) { + t.Fatalf("expected support channel binding, got %v", support.Channels) + } +} + +func TestContextBuilder_BuildMessagesIncludesAgentDiscoverySection(t *testing.T) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +description: Main agent +--- +# Agent + +Generalist. +`, + }) + defer cleanupWorkspace(t, mainWorkspace) + + researchWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +description: Research specialist +--- +# Agent + +Investigate deeply. +`, + }) + defer cleanupWorkspace(t, researchWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Workspace: mainWorkspace}, + {ID: "research", Workspace: researchWorkspace}, + }) + cfg.Tools.ReadFile.Enabled = true + cfg.Tools.WriteFile.Enabled = true + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + mainAgent, ok := registry.GetAgent("main") + if !ok || mainAgent == nil { + t.Fatal("expected main agent") + } + + messages := mainAgent.ContextBuilder.BuildMessages( + nil, + "", + "delegate wisely", + nil, + "telegram", + "chat-1", + "", + "", + ) + if len(messages) == 0 { + t.Fatal("expected messages") + } + + systemPrompt := messages[0].Content + if !strings.Contains(systemPrompt, "# Agent Discovery") { + t.Fatalf("expected discovery section in system prompt, got %q", systemPrompt) + } + if !strings.Contains(systemPrompt, `"current_agent_id": "main"`) { + t.Fatalf("expected current agent id in discovery section, got %q", systemPrompt) + } + if !strings.Contains(systemPrompt, `"id": "main"`) || + !strings.Contains(systemPrompt, `"id": "research"`) { + t.Fatalf("expected self and peer descriptors in discovery section, got %q", systemPrompt) + } + if !strings.Contains(systemPrompt, `"available_tools": [`) || + !strings.Contains(systemPrompt, `"read_file"`) || + !strings.Contains(systemPrompt, `"write_file"`) { + t.Fatalf("expected visible tool list in discovery section, got %q", systemPrompt) + } +} + +func TestContextBuilder_BuildMessagesOmitsAgentDiscoverySectionForSingleton(t *testing.T) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +description: Main agent +--- +# Agent + +Generalist. +`, + }) + defer cleanupWorkspace(t, mainWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Workspace: mainWorkspace}, + }) + cfg.Tools.ReadFile.Enabled = true + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + mainAgent, ok := registry.GetAgent("main") + if !ok || mainAgent == nil { + t.Fatal("expected main agent") + } + + messages := mainAgent.ContextBuilder.BuildMessages( + nil, + "", + "handle locally", + nil, + "telegram", + "chat-1", + "", + "", + ) + if len(messages) == 0 { + t.Fatal("expected messages") + } + + systemPrompt := messages[0].Content + if strings.Contains(systemPrompt, "# Agent Discovery") { + t.Fatalf("did not expect discovery section for singleton registry, got %q", systemPrompt) + } + if strings.Contains(systemPrompt, `"current_agent_id": "main"`) { + t.Fatalf("did not expect discovery payload for singleton registry, got %q", systemPrompt) + } +} diff --git a/pkg/agent/eventbus_test.go b/pkg/agent/eventbus_test.go index 19a1ea9eb..0b0e351dd 100644 --- a/pkg/agent/eventbus_test.go +++ b/pkg/agent/eventbus_test.go @@ -275,7 +275,13 @@ func TestAgentLoop_EmitsSteeringAndSkippedToolEvents(t *testing.T) { resultCh := make(chan string, 1) go func() { - resp, _ := al.ProcessDirectWithChannel(context.Background(), "do something", "test-session", "test", "chat1") + resp, _ := al.ProcessDirectWithChannel( + context.Background(), + "do something", + "test-session", + "test", + "chat1", + ) resultCh <- resp }() @@ -338,7 +344,11 @@ func TestAgentLoop_EmitsSteeringAndSkippedToolEvents(t *testing.T) { t.Fatalf("expected steering interrupt kind, got %q", interruptPayload.Kind) } if interruptPayload.ContentLen != len("change course") { - t.Fatalf("expected interrupt content len %d, got %d", len("change course"), interruptPayload.ContentLen) + t.Fatalf( + "expected interrupt content len %d, got %d", + len("change course"), + interruptPayload.ContentLen, + ) } } @@ -360,7 +370,9 @@ func TestAgentLoop_EmitsContextCompressEventOnRetry(t *testing.T) { }, } - contextErr := stringError("InvalidParameter: Total tokens of image and text exceed max message tokens") + contextErr := stringError( + "InvalidParameter: Total tokens of image and text exceed max message tokens", + ) provider := &failFirstMockProvider{ failures: 1, failError: contextErr, @@ -603,7 +615,12 @@ func collectEventStream(ch <-chan Event) []Event { } } -func waitForEvent(t *testing.T, ch <-chan Event, timeout time.Duration, match func(Event) bool) Event { +func waitForEvent( + t *testing.T, + ch <-chan Event, + timeout time.Duration, + match func(Event) bool, +) Event { t.Helper() timer := time.NewTimer(timeout) diff --git a/pkg/agent/hook_mount_test.go b/pkg/agent/hook_mount_test.go index 85d8f5c11..068f8da10 100644 --- a/pkg/agent/hook_mount_test.go +++ b/pkg/agent/hook_mount_test.go @@ -40,7 +40,11 @@ func (h *builtinAutoHook) AfterLLM( return next, HookDecision{Action: HookActionModify}, nil } -func newConfiguredHookLoop(t *testing.T, provider *llmHookTestProvider, hooks config.HooksConfig) *AgentLoop { +func newConfiguredHookLoop( + t *testing.T, + provider *llmHookTestProvider, + hooks config.HooksConfig, +) *AgentLoop { t.Helper() cfg := &config.Config{ @@ -102,7 +106,13 @@ func TestAgentLoop_ProcessDirectWithChannel_AutoMountsBuiltinHook(t *testing.T) }) defer al.Close() - resp, err := al.ProcessDirectWithChannel(context.Background(), "hello", "session-1", "cli", "direct") + resp, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "session-1", + "cli", + "direct", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -140,7 +150,13 @@ func TestAgentLoop_ProcessDirectWithChannel_AutoMountsProcessHook(t *testing.T) }) defer al.Close() - resp, err := al.ProcessDirectWithChannel(context.Background(), "hello", "session-1", "cli", "direct") + resp, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "session-1", + "cli", + "direct", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -172,7 +188,13 @@ func TestAgentLoop_ProcessDirectWithChannel_InvalidConfiguredHookFails(t *testin }) defer al.Close() - _, err := al.ProcessDirectWithChannel(context.Background(), "hello", "session-1", "cli", "direct") + _, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "session-1", + "cli", + "direct", + ) if err == nil { t.Fatal("expected invalid configured hook error") } diff --git a/pkg/agent/hook_process.go b/pkg/agent/hook_process.go index e5632913d..9b623ce42 100644 --- a/pkg/agent/hook_process.go +++ b/pkg/agent/hook_process.go @@ -98,7 +98,11 @@ type processHookAfterToolResponse struct { Result *ToolResultHookResponse `json:"result,omitempty"` } -func NewProcessHook(ctx context.Context, name string, opts ProcessHookOptions) (*ProcessHook, error) { +func NewProcessHook( + ctx context.Context, + name string, + opts ProcessHookOptions, +) (*ProcessHook, error) { if len(opts.Command) == 0 { return nil, fmt.Errorf("process hook command is required") } @@ -262,7 +266,10 @@ func (ph *ProcessHook) AfterTool( return resp.Result, HookDecision{Action: resp.Action, Reason: resp.Reason}, nil } -func (ph *ProcessHook) ApproveTool(ctx context.Context, req *ToolApprovalRequest) (ApprovalDecision, error) { +func (ph *ProcessHook) ApproveTool( + ctx context.Context, + req *ToolApprovalRequest, +) (ApprovalDecision, error) { if ph == nil || !ph.opts.ApproveTool { return ApprovalDecision{Approved: true}, nil } @@ -473,7 +480,11 @@ func (ph *ProcessHook) removePending(id uint64) { } } -func (al *AgentLoop) MountProcessHook(ctx context.Context, name string, opts ProcessHookOptions) error { +func (al *AgentLoop) MountProcessHook( + ctx context.Context, + name string, + opts ProcessHookOptions, +) error { if al == nil { return fmt.Errorf("agent loop is nil") } diff --git a/pkg/agent/hooks.go b/pkg/agent/hooks.go index c1ef58ffd..4f63d0652 100644 --- a/pkg/agent/hooks.go +++ b/pkg/agent/hooks.go @@ -79,8 +79,14 @@ type LLMInterceptor interface { } type ToolInterceptor interface { - BeforeTool(ctx context.Context, call *ToolCallHookRequest) (*ToolCallHookRequest, HookDecision, error) - AfterTool(ctx context.Context, result *ToolResultHookResponse) (*ToolResultHookResponse, HookDecision, error) + BeforeTool( + ctx context.Context, + call *ToolCallHookRequest, + ) (*ToolCallHookRequest, HookDecision, error) + AfterTool( + ctx context.Context, + result *ToolResultHookResponse, + ) (*ToolResultHookResponse, HookDecision, error) } type ToolApprover interface { @@ -295,7 +301,10 @@ func (hm *HookManager) dispatchEvents() { } } -func (hm *HookManager) BeforeLLM(ctx context.Context, req *LLMHookRequest) (*LLMHookRequest, HookDecision) { +func (hm *HookManager) BeforeLLM( + ctx context.Context, + req *LLMHookRequest, +) (*LLMHookRequest, HookDecision) { if hm == nil || req == nil { return req, HookDecision{Action: HookActionContinue} } @@ -326,7 +335,10 @@ func (hm *HookManager) BeforeLLM(ctx context.Context, req *LLMHookRequest) (*LLM return current, HookDecision{Action: HookActionContinue} } -func (hm *HookManager) AfterLLM(ctx context.Context, resp *LLMHookResponse) (*LLMHookResponse, HookDecision) { +func (hm *HookManager) AfterLLM( + ctx context.Context, + resp *LLMHookResponse, +) (*LLMHookResponse, HookDecision) { if hm == nil || resp == nil { return resp, HookDecision{Action: HookActionContinue} } diff --git a/pkg/agent/hooks_test.go b/pkg/agent/hooks_test.go index 49e1b1784..d112d4c07 100644 --- a/pkg/agent/hooks_test.go +++ b/pkg/agent/hooks_test.go @@ -293,7 +293,10 @@ func TestAgentLoop_Hooks_ToolInterceptorCanRewrite(t *testing.T) { type denyApprovalHook struct{} -func (h *denyApprovalHook) ApproveTool(ctx context.Context, req *ToolApprovalRequest) (ApprovalDecision, error) { +func (h *denyApprovalHook) ApproveTool( + ctx context.Context, + req *ToolApprovalRequest, +) (ApprovalDecision, error) { return ApprovalDecision{ Approved: false, Reason: "blocked", diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 880725660..4b3b4b3ee 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -72,12 +72,16 @@ func NewAgentInstance( // Compile path whitelist patterns from config. allowReadPaths := buildAllowReadPatterns(cfg) allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) + agentToolAllowlist := resolveAgentToolAllowlist(agentCfg) toolsRegistry := tools.NewToolRegistry() + toolsRegistry.SetAllowlist(agentToolAllowlist) if cfg.Tools.IsToolEnabled("read_file") { maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize - toolsRegistry.Register(tools.NewReadFileTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) + toolsRegistry.Register( + tools.NewReadFileTool(workspace, readRestrict, maxReadFileSize, allowReadPaths), + ) } if cfg.Tools.IsToolEnabled("write_file") { toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths)) @@ -180,8 +184,15 @@ func NewAgentInstance( if len(resolved) > 0 { lightModelCfg, err := resolvedModelConfig(cfg, rc.LightModel, workspace) if err != nil { - logger.WarnCF("agent", "Routing light model config invalid; routing disabled", - map[string]any{"light_model": rc.LightModel, "agent_id": agentID, "error": err.Error()}) + logger.WarnCF( + "agent", + "Routing light model config invalid; routing disabled", + map[string]any{ + "light_model": rc.LightModel, + "agent_id": agentID, + "error": err.Error(), + }, + ) } else { lp, _, err := providers.CreateProviderFromConfig(lightModelCfg) if err != nil { @@ -234,7 +245,8 @@ func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentD return expandHome(strings.TrimSpace(agentCfg.Workspace)) } // Use the configured default workspace (respects PICOCLAW_HOME) - if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" { + if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || + routing.NormalizeAgentID(agentCfg.ID) == "main" { return expandHome(defaults.Workspace) } // For named agents without explicit workspace, use default workspace with agent ID suffix diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index e296a18cb..a933a6493 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -156,7 +156,11 @@ func TestNewAgentInstance_ResolveCandidatesFromModelListAlias(t *testing.T) { t.Fatalf("len(Candidates) = %d, want 1", len(agent.Candidates)) } if agent.Candidates[0].Provider != tt.wantProvider { - t.Fatalf("candidate provider = %q, want %q", agent.Candidates[0].Provider, tt.wantProvider) + t.Fatalf( + "candidate provider = %q, want %q", + agent.Candidates[0].Provider, + tt.wantProvider, + ) } if agent.Candidates[0].Model != tt.wantModel { t.Fatalf("candidate model = %q, want %q", agent.Candidates[0].Model, tt.wantModel) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ef2951365..2193bbad3 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -192,7 +192,11 @@ func registerSharedTools( Proxy: cfg.Tools.Web.Proxy, }) if err != nil { - logger.ErrorCF("agent", "Failed to create web search tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web search tool", + map[string]any{"error": err.Error()}, + ) } else if searchTool != nil { agent.Tools.Register(searchTool) } @@ -205,7 +209,11 @@ func registerSharedTools( cfg.Tools.Web.FetchLimitBytes, cfg.Tools.Web.PrivateHostWhitelist) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } else { agent.Tools.Register(fetchTool) } @@ -475,7 +483,12 @@ func (al *AgentLoop) Run(ctx context.Context) error { "queue_depth": al.pendingSteeringCountForScope(target.SessionKey), }) - continued, continueErr := al.Continue(ctx, target.SessionKey, target.Channel, target.ChatID) + continued, continueErr := al.Continue( + ctx, + target.SessionKey, + target.Channel, + target.ChatID, + ) if continueErr != nil { logger.WarnCF("agent", "Failed to continue queued steering", map[string]any{ @@ -503,14 +516,22 @@ func (al *AgentLoop) Run(ctx context.Context) error { "queue_depth": al.pendingSteeringCountForScope(target.SessionKey), }) - continued, continueErr := al.Continue(ctx, target.SessionKey, target.Channel, target.ChatID) + continued, continueErr := al.Continue( + ctx, + target.SessionKey, + target.Channel, + target.ChatID, + ) if continueErr != nil { - logger.WarnCF("agent", "Failed to continue queued steering after shutdown drain", + logger.WarnCF( + "agent", + "Failed to continue queued steering after shutdown drain", map[string]any{ "channel": target.Channel, "chat_id": target.ChatID, "error": continueErr.Error(), - }) + }, + ) return } if continued == "" { @@ -565,11 +586,15 @@ func (al *AgentLoop) drainBusToSteering(ctx context.Context, activeScope, active msgScope, _, scopeOK := al.resolveSteeringTarget(msg) if !scopeOK || msgScope != activeScope { if err := al.requeueInboundMessage(msg); err != nil { - logger.WarnCF("agent", "Failed to requeue non-steering inbound message", map[string]any{ - "error": err.Error(), - "channel": msg.Channel, - "sender_id": msg.SenderID, - }) + logger.WarnCF( + "agent", + "Failed to requeue non-steering inbound message", + map[string]any{ + "error": err.Error(), + "channel": msg.Channel, + "sender_id": msg.SenderID, + }, + ) } continue } @@ -603,7 +628,10 @@ func (al *AgentLoop) Stop() { al.running.Store(false) } -func (al *AgentLoop) PublishResponseIfNeeded(ctx context.Context, channel, chatID, response string) { +func (al *AgentLoop) PublishResponseIfNeeded( + ctx context.Context, + channel, chatID, response string, +) { if response == "" { return } @@ -1053,7 +1081,10 @@ var audioAnnotationRe = regexp.MustCompile(`\[(voice|audio)(?::[^\]]*)?\]`) // transcribeAudioInMessage resolves audio media refs, transcribes them, and // replaces audio annotations in msg.Content with the transcribed text. // Returns the (possibly modified) message and true if audio was transcribed. -func (al *AgentLoop) transcribeAudioInMessage(ctx context.Context, msg bus.InboundMessage) (bus.InboundMessage, bool) { +func (al *AgentLoop) transcribeAudioInMessage( + ctx context.Context, + msg bus.InboundMessage, +) (bus.InboundMessage, bool) { if al.transcriber == nil || al.mediaStore == nil || len(msg.Media) == 0 { return msg, false } @@ -1063,7 +1094,11 @@ func (al *AgentLoop) transcribeAudioInMessage(ctx context.Context, msg bus.Inbou for _, ref := range msg.Media { path, meta, err := al.mediaStore.ResolveWithMeta(ref) if err != nil { - logger.WarnCF("voice", "Failed to resolve media ref", map[string]any{"ref": ref, "error": err}) + logger.WarnCF( + "voice", + "Failed to resolve media ref", + map[string]any{"ref": ref, "error": err}, + ) continue } if !utils.IsAudioFile(meta.Filename, meta.ContentType) { @@ -1141,7 +1176,11 @@ func (al *AgentLoop) sendTranscriptionFeedback( ReplyToMessageID: messageID, }) if err != nil { - logger.WarnCF("voice", "Failed to send transcription feedback", map[string]any{"error": err.Error()}) + logger.WarnCF( + "voice", + "Failed to send transcription feedback", + map[string]any{"error": err.Error()}, + ) } } @@ -1342,7 +1381,9 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) return al.runAgentLoop(ctx, agent, opts) } -func (al *AgentLoop) resolveMessageRoute(msg bus.InboundMessage) (routing.ResolvedRoute, *AgentInstance, error) { +func (al *AgentLoop) resolveMessageRoute( + msg bus.InboundMessage, +) (routing.ResolvedRoute, *AgentInstance, error) { registry := al.GetRegistry() route := registry.ResolveRoute(routing.RouteInput{ Channel: msg.Channel, @@ -1358,7 +1399,10 @@ func (al *AgentLoop) resolveMessageRoute(msg bus.InboundMessage) (routing.Resolv agent = registry.GetDefaultAgent() } if agent == nil { - return routing.ResolvedRoute{}, nil, fmt.Errorf("no agent available for route (agent_id=%s)", route.AgentID) + return routing.ResolvedRoute{}, nil, fmt.Errorf( + "no agent available for route (agent_id=%s)", + route.AgentID, + ) } return route, agent, nil @@ -1683,7 +1727,11 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState) (turnResult, er ts.recordPersistedMessage(rootMsg) } - activeCandidates, activeModel, usedLight := al.selectCandidates(ts.agent, ts.userMessage, messages) + activeCandidates, activeModel, usedLight := al.selectCandidates( + ts.agent, + ts.userMessage, + messages, + ) activeProvider := ts.agent.Provider if usedLight && ts.agent.LightProvider != nil { activeProvider = ts.agent.LightProvider @@ -2656,12 +2704,15 @@ turnLoop: } if steerMsgs := al.dequeueSteeringMessagesForScope(ts.sessionKey); len(steerMsgs) > 0 { - logger.InfoCF("agent", "Steering arrived after turn completion; continuing turn before finalizing", + logger.InfoCF( + "agent", + "Steering arrived after turn completion; continuing turn before finalizing", map[string]any{ "agent_id": ts.agent.ID, "steering_count": len(steerMsgs), "session_key": ts.sessionKey, - }) + }, + ) pendingMessages = append(pendingMessages, steerMsgs...) finalContent = "" goto turnLoop @@ -2777,11 +2828,18 @@ func (al *AgentLoop) selectCandidates( "score": score, "threshold": agent.Router.Threshold(), }) - return agent.LightCandidates, resolvedCandidateModel(agent.LightCandidates, agent.Router.LightModel()), true + return agent.LightCandidates, resolvedCandidateModel( + agent.LightCandidates, + agent.Router.LightModel(), + ), true } // maybeSummarize triggers summarization if the session history exceeds thresholds. -func (al *AgentLoop) maybeSummarize(agent *AgentInstance, sessionKey string, turnScope turnEventScope) { +func (al *AgentLoop) maybeSummarize( + agent *AgentInstance, + sessionKey string, + turnScope turnEventScope, +) { newHistory := agent.Sessions.GetHistory(sessionKey) tokenEstimate := al.estimateTokens(newHistory) threshold := agent.ContextWindow * agent.SummarizeTokenPercent / 100 @@ -2815,7 +2873,10 @@ type compressionResult struct { // prompt is built dynamically by BuildMessages and is NOT stored here. // The compression note is recorded in the session summary so that // BuildMessages can include it in the next system prompt. -func (al *AgentLoop) forceCompression(agent *AgentInstance, sessionKey string) (compressionResult, bool) { +func (al *AgentLoop) forceCompression( + agent *AgentInstance, + sessionKey string, +) (compressionResult, bool) { history := agent.Sessions.GetHistory(sessionKey) if len(history) <= 2 { return compressionResult{}, false @@ -2968,7 +3029,11 @@ func formatToolsForLog(toolDefs []providers.ToolDefinition) string { } // summarizeSession summarizes the conversation history for a session. -func (al *AgentLoop) summarizeSession(agent *AgentInstance, sessionKey string, turnScope turnEventScope) { +func (al *AgentLoop) summarizeSession( + agent *AgentInstance, + sessionKey string, + turnScope turnEventScope, +) { ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() @@ -3320,7 +3385,10 @@ func (al *AgentLoop) applyExplicitSkillCommand( skillName, ok := agent.ContextBuilder.ResolveSkillName(arg) if !ok { - return true, true, fmt.Sprintf("Unknown skill: %s\nUse /list skills to see installed skills.", arg) + return true, true, fmt.Sprintf( + "Unknown skill: %s\nUse /list skills to see installed skills.", + arg, + ) } if len(parts) < 3 { @@ -3347,7 +3415,10 @@ func (al *AgentLoop) applyExplicitSkillCommand( return true, false, "" } -func (al *AgentLoop) buildCommandsRuntime(agent *AgentInstance, opts *processOptions) *commands.Runtime { +func (al *AgentLoop) buildCommandsRuntime( + agent *AgentInstance, + opts *processOptions, +) *commands.Runtime { registry := al.GetRegistry() cfg := al.GetConfig() rt := &commands.Runtime{ @@ -3391,7 +3462,10 @@ func (al *AgentLoop) buildCommandsRuntime(agent *AgentInstance, opts *processOpt rt.ListSkillNames = agent.ContextBuilder.ListSkillNames } rt.GetModelInfo = func() (string, string) { - return agent.Model, resolvedCandidateProvider(agent.Candidates, cfg.Agents.Defaults.Provider) + return agent.Model, resolvedCandidateProvider( + agent.Candidates, + cfg.Agents.Defaults.Provider, + ) } rt.SwitchModel = func(value string) (string, error) { value = strings.TrimSpace(value) @@ -3405,7 +3479,12 @@ func (al *AgentLoop) buildCommandsRuntime(agent *AgentInstance, opts *processOpt return "", fmt.Errorf("failed to initialize model %q: %w", value, err) } - nextCandidates := resolveModelCandidates(cfg, cfg.Agents.Defaults.Provider, modelCfg.Model, agent.Fallbacks) + nextCandidates := resolveModelCandidates( + cfg, + cfg.Agents.Defaults.Provider, + modelCfg.Model, + agent.Fallbacks, + ) if len(nextCandidates) == 0 { return "", fmt.Errorf("model %q did not resolve to any provider candidates", value) } diff --git a/pkg/agent/loop_mcp.go b/pkg/agent/loop_mcp.go index 97debbc33..644f7168e 100644 --- a/pkg/agent/loop_mcp.go +++ b/pkg/agent/loop_mcp.go @@ -65,7 +65,11 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { } if al.cfg.Tools.MCP.Servers == nil || len(al.cfg.Tools.MCP.Servers) == 0 { - logger.WarnCF("agent", "MCP is enabled but no servers are configured, skipping MCP initialization", nil) + logger.WarnCF( + "agent", + "MCP is enabled but no servers are configured, skipping MCP initialization", + nil, + ) return nil } @@ -76,7 +80,11 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { } } if !findValidServer { - logger.WarnCF("agent", "MCP is enabled but no valid servers are configured, skipping MCP initialization", nil) + logger.WarnCF( + "agent", + "MCP is enabled but no valid servers are configured, skipping MCP initialization", + nil, + ) return nil } @@ -193,10 +201,14 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { } if useRegex { - agent.Tools.Register(tools.NewRegexSearchTool(agent.Tools, ttl, maxSearchResults)) + agent.Tools.Register( + tools.NewRegexSearchTool(agent.Tools, ttl, maxSearchResults), + ) } if useBM25 { - agent.Tools.Register(tools.NewBM25SearchTool(agent.Tools, ttl, maxSearchResults)) + agent.Tools.Register( + tools.NewBM25SearchTool(agent.Tools, ttl, maxSearchResults), + ) } } } diff --git a/pkg/agent/loop_media.go b/pkg/agent/loop_media.go index e8314c10d..6958f51cb 100644 --- a/pkg/agent/loop_media.go +++ b/pkg/agent/loop_media.go @@ -25,7 +25,11 @@ import ( // Non-image files (documents, audio, video) have their local path injected // into Content so the agent can access them via file tools like read_file. // Returns a new slice; original messages are not mutated. -func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message { +func resolveMediaRefs( + messages []providers.Message, + store media.MediaStore, + maxSize int, +) []providers.Message { if store == nil { return messages } diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 25d20c689..9911c5cb7 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -591,7 +591,9 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) imagePath := filepath.Join(tmpDir, "screen.png") if err := os.WriteFile(imagePath, []byte("fake screenshot"), 0o644); err != nil { @@ -613,7 +615,10 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. t.Fatalf("processMessage() error = %v", err) } if response != "" { - t.Fatalf("expected no final response when media tool already handled delivery, got %q", response) + t.Fatalf( + "expected no final response when media tool already handled delivery, got %q", + response, + ) } if provider.calls != 1 { t.Fatalf("expected exactly 1 LLM call, got %d", provider.calls) @@ -626,13 +631,20 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } - if telegramChannel.sentMedia[0].Channel != "telegram" || telegramChannel.sentMedia[0].ChatID != "chat1" { + if telegramChannel.sentMedia[0].Channel != "telegram" || + telegramChannel.sentMedia[0].ChatID != "chat1" { t.Fatalf("unexpected sent media target: %+v", telegramChannel.sentMedia[0]) } if len(telegramChannel.sentMedia[0].Parts) != 1 { - t.Fatalf("expected exactly 1 sent media part, got %d", len(telegramChannel.sentMedia[0].Parts)) + t.Fatalf( + "expected exactly 1 sent media part, got %d", + len(telegramChannel.sentMedia[0].Parts), + ) } select { @@ -660,7 +672,8 @@ func TestProcessMessage_MediaToolHandledSkipsFollowUpLLMAndFinalText(t *testing. t.Fatal("expected session history to be saved") } last := history[len(history)-1] - if last.Role != "assistant" || last.Content != "Requested output delivered via tool attachment." { + if last.Role != "assistant" || + last.Content != "Requested output delivered via tool attachment." { t.Fatalf("expected handled assistant summary in history, got %+v", last) } } @@ -685,7 +698,9 @@ func TestProcessMessage_HandledToolProcessesQueuedSteeringBeforeReturning(t *tes store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) imagePath := filepath.Join(tmpDir, "screen-steering.png") if err := os.WriteFile(imagePath, []byte("fake screenshot"), 0o644); err != nil { @@ -714,7 +729,10 @@ func TestProcessMessage_HandledToolProcessesQueuedSteeringBeforeReturning(t *tes t.Fatalf("expected 2 LLM calls after queued steering, got %d", provider.calls) } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } } @@ -733,7 +751,9 @@ func TestProcessMessage_MediaArtifactCanBeForwardedBySendFile(t *testing.T) { store := media.NewFileMediaStore() al.SetMediaStore(store) telegramChannel := &fakeMediaChannel{fakeChannel: fakeChannel{id: "rid-telegram"}} - al.SetChannelManager(newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel)) + al.SetChannelManager( + newStartedTestChannelManager(t, msgBus, store, "telegram", telegramChannel), + ) mediaDir := media.TempDir() if err := os.MkdirAll(mediaDir, 0o700); err != nil { @@ -766,13 +786,20 @@ func TestProcessMessage_MediaArtifactCanBeForwardedBySendFile(t *testing.T) { } if len(telegramChannel.sentMedia) != 1 { - t.Fatalf("expected exactly 1 synchronously sent media message, got %d", len(telegramChannel.sentMedia)) + t.Fatalf( + "expected exactly 1 synchronously sent media message, got %d", + len(telegramChannel.sentMedia), + ) } - if telegramChannel.sentMedia[0].Channel != "telegram" || telegramChannel.sentMedia[0].ChatID != "chat1" { + if telegramChannel.sentMedia[0].Channel != "telegram" || + telegramChannel.sentMedia[0].ChatID != "chat1" { t.Fatalf("unexpected sent media target: %+v", telegramChannel.sentMedia[0]) } if len(telegramChannel.sentMedia[0].Parts) != 1 { - t.Fatalf("expected exactly 1 sent media part, got %d", len(telegramChannel.sentMedia[0].Parts)) + t.Fatalf( + "expected exactly 1 sent media part, got %d", + len(telegramChannel.sentMedia[0].Parts), + ) } select { @@ -1183,7 +1210,10 @@ func (m *handledMediaWithSteeringTool) Parameters() map[string]any { } } -func (m *handledMediaWithSteeringTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult { +func (m *handledMediaWithSteeringTool) Execute( + ctx context.Context, + args map[string]any, +) *tools.ToolResult { if err := m.loop.Steer(providers.Message{Role: "user", Content: "what about this instead?"}); err != nil { return tools.ErrorResult(err.Error()).WithError(err) } @@ -1336,7 +1366,11 @@ func newStrictChatCompletionTestServer( })) } -func (h testHelper) executeAndGetResponse(tb testing.TB, ctx context.Context, msg bus.InboundMessage) string { +func (h testHelper) executeAndGetResponse( + tb testing.TB, + ctx context.Context, + msg bus.InboundMessage, +) string { // Use a short timeout to avoid hanging timeoutCtx, cancel := context.WithTimeout(ctx, responseTimeout) defer cancel() @@ -1467,7 +1501,10 @@ func TestProcessMessage_CommandOutcomes(t *testing.T) { t.Fatalf("unexpected /foo reply: %q", fooResp) } if provider.calls != 1 { - t.Fatalf("LLM should be called exactly once after /foo passthrough, calls=%d", provider.calls) + t.Fatalf( + "LLM should be called exactly once after /foo passthrough, calls=%d", + provider.calls, + ) } newResp := helper.executeAndGetResponse(t, context.Background(), bus.InboundMessage{ @@ -1617,7 +1654,10 @@ func TestProcessMessage_SwitchModelRejectsUnknownAlias(t *testing.T) { } if provider.calls != 0 { - t.Fatalf("LLM should not be called for rejected /switch and /show, calls=%d", provider.calls) + t.Fatalf( + "LLM should not be called for rejected /switch and /show, calls=%d", + provider.calls, + ) } } @@ -1635,7 +1675,13 @@ func TestProcessMessage_SwitchModelRoutesSubsequentRequestsToSelectedProvider(t remoteCalls := 0 remoteModel := "" - remoteServer := newChatCompletionTestServer(t, "remote", "remote reply", &remoteCalls, &remoteModel) + remoteServer := newChatCompletionTestServer( + t, + "remote", + "remote reply", + &remoteCalls, + &remoteModel, + ) defer remoteServer.Close() cfg := &config.Config{ @@ -1958,7 +2004,9 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { msgBus := bus.NewMessageBus() // Create a provider that fails once with a context error - contextErr := fmt.Errorf("InvalidParameter: Total tokens of image and text exceed max message tokens") + contextErr := fmt.Errorf( + "InvalidParameter: Total tokens of image and text exceed max message tokens", + ) provider := &failFirstMockProvider{ failures: 1, failError: contextErr, @@ -2039,7 +2087,13 @@ func TestAgentLoop_EmptyModelResponseUsesAccurateFallback(t *testing.T) { provider := &simpleMockProvider{response: ""} al := NewAgentLoop(cfg, msgBus, provider) - response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "empty-response", "test", "chat1") + response, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "empty-response", + "test", + "chat1", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -2071,7 +2125,13 @@ func TestAgentLoop_ToolLimitUsesDedicatedFallback(t *testing.T) { al := NewAgentLoop(cfg, msgBus, provider) al.RegisterTool(&toolLimitTestTool{}) - response, err := al.ProcessDirectWithChannel(context.Background(), "hello", "tool-limit", "test", "chat1") + response, err := al.ProcessDirectWithChannel( + context.Background(), + "hello", + "tool-limit", + "test", + "chat1", + ) if err != nil { t.Fatalf("ProcessDirectWithChannel failed: %v", err) } @@ -2389,7 +2449,9 @@ func TestHandleReasoning(t *testing.T) { break } if msg.Content == "should timeout" { - t.Fatal("expected reasoning message to be dropped when bus is full, but it was published") + t.Fatal( + "expected reasoning message to be dropped when bus is full, but it was published", + ) } } } @@ -2483,7 +2545,12 @@ func TestProcessHeartbeat_DoesNotPublishToolFeedback(t *testing.T) { provider := &toolFeedbackProvider{filePath: heartbeatFile} al := NewAgentLoop(cfg, msgBus, provider) - response, err := al.ProcessHeartbeat(context.Background(), "check heartbeat tasks", "telegram", "chat-1") + response, err := al.ProcessHeartbeat( + context.Background(), + "check heartbeat tasks", + "telegram", + "chat-1", + ) if err != nil { t.Fatalf("ProcessHeartbeat() error = %v", err) } @@ -2968,8 +3035,14 @@ func TestProcessMessage_ContextOverflowRecovery(t *testing.T) { agent := al.GetRegistry().GetDefaultAgent() for i := 0; i < 5; i++ { - agent.Sessions.AddFullMessage(sessionKey, providers.Message{Role: "user", Content: "heavy message"}) - agent.Sessions.AddFullMessage(sessionKey, providers.Message{Role: "assistant", Content: "response"}) + agent.Sessions.AddFullMessage( + sessionKey, + providers.Message{Role: "user", Content: "heavy message"}, + ) + agent.Sessions.AddFullMessage( + sessionKey, + providers.Message{Role: "assistant", Content: "response"}, + ) } response, err := al.processMessage(context.Background(), bus.InboundMessage{ diff --git a/pkg/agent/model_resolution.go b/pkg/agent/model_resolution.go index 140cff718..d5c2f74ea 100644 --- a/pkg/agent/model_resolution.go +++ b/pkg/agent/model_resolution.go @@ -26,7 +26,8 @@ func buildModelListResolver(cfg *config.Config) func(raw string) (string, bool) return "", false } - if mc, err := cfg.GetModelConfig(raw); err == nil && mc != nil && strings.TrimSpace(mc.Model) != "" { + if mc, err := cfg.GetModelConfig(raw); err == nil && mc != nil && + strings.TrimSpace(mc.Model) != "" { return ensureProtocol(mc.Model), true } @@ -78,7 +79,10 @@ func resolvedCandidateProvider(candidates []providers.FallbackCandidate, fallbac return fallback } -func resolvedModelConfig(cfg *config.Config, modelName, workspace string) (*config.ModelConfig, error) { +func resolvedModelConfig( + cfg *config.Config, + modelName, workspace string, +) (*config.ModelConfig, error) { if cfg == nil { return nil, fmt.Errorf("config is nil") } diff --git a/pkg/agent/registry.go b/pkg/agent/registry.go index 58b7ce440..46f54f5c8 100644 --- a/pkg/agent/registry.go +++ b/pkg/agent/registry.go @@ -12,6 +12,7 @@ import ( // AgentRegistry manages multiple agent instances and routes messages to them. type AgentRegistry struct { + cfg *config.Config agents map[string]*AgentInstance resolver *routing.RouteResolver mu sync.RWMutex @@ -23,6 +24,7 @@ func NewAgentRegistry( provider providers.LLMProvider, ) *AgentRegistry { registry := &AgentRegistry{ + cfg: cfg, agents: make(map[string]*AgentInstance), resolver: routing.NewRouteResolver(cfg), } @@ -52,6 +54,14 @@ func NewAgentRegistry( } } + for id, instance := range registry.agents { + if instance.ContextBuilder != nil { + instance.ContextBuilder. + WithAgentIdentity(id). + WithAgentDiscovery(registry.ListAgents) + } + } + return registry } @@ -130,11 +140,13 @@ func (r *AgentRegistry) Close() { func (r *AgentRegistry) GetDefaultAgent() *AgentInstance { r.mu.RLock() defer r.mu.RUnlock() - if agent, ok := r.agents["main"]; ok { - return agent + if id := r.defaultAgentIDLocked(); id != "" { + if agent, ok := r.agents[id]; ok { + return agent + } } - for _, agent := range r.agents { - return agent + for id := range r.agents { + return r.agents[id] } return nil } diff --git a/pkg/agent/registry_test.go b/pkg/agent/registry_test.go index b173ef967..2b577ab93 100644 --- a/pkg/agent/registry_test.go +++ b/pkg/agent/registry_test.go @@ -2,8 +2,10 @@ package agent import ( "context" + "slices" "testing" + "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -200,6 +202,77 @@ func TestAgentInstance_FallbackExplicitEmpty(t *testing.T) { agent, _ := registry.GetAgent("no-fallback") if len(agent.Fallbacks) != 0 { - t.Errorf("expected 0 fallbacks (explicit empty), got %d: %v", len(agent.Fallbacks), agent.Fallbacks) + t.Errorf( + "expected 0 fallbacks (explicit empty), got %d: %v", + len(agent.Fallbacks), + agent.Fallbacks, + ) + } +} + +func TestNewAgentLoop_AgentToolAllowlistFiltersRuntimeTools(t *testing.T) { + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true}, + { + ID: "research", + Tools: []string{"read_file", "write_file", "web_search", "web_fetch", "message"}, + }, + }) + cfg.Tools.ReadFile.Enabled = true + cfg.Tools.WriteFile.Enabled = true + cfg.Tools.ListDir.Enabled = true + cfg.Tools.Exec.Enabled = true + cfg.Tools.Message.Enabled = true + cfg.Tools.Web.Enabled = true + cfg.Tools.Web.DuckDuckGo.Enabled = true + cfg.Tools.WebFetch.Enabled = true + cfg.Tools.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + research, ok := al.GetRegistry().GetAgent("research") + if !ok || research == nil { + t.Fatal("expected research agent") + } + + got := research.Tools.List() + want := []string{"message", "read_file", "web_fetch", "web_search", "write_file"} + if !slices.Equal(got, want) { + t.Fatalf("research tools = %v, want %v", got, want) + } + + for _, blocked := range []string{"exec", "list_dir", "spawn", "subagent"} { + if _, ok := research.Tools.Get(blocked); ok { + t.Fatalf("expected %q to be blocked by allowlist", blocked) + } + } +} + +func TestNewAgentLoop_AgentToolAllowlistRequiresExactRuntimeToolNames(t *testing.T) { + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true}, + { + ID: "research", + Tools: []string{"web"}, + }, + }) + cfg.Tools.Web.Enabled = true + cfg.Tools.Web.DuckDuckGo.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + research, ok := al.GetRegistry().GetAgent("research") + if !ok || research == nil { + t.Fatal("expected research agent") + } + + if _, ok := research.Tools.Get("web_search"); ok { + t.Fatal("web_search should not be registered when allowlist contains only web") + } + if slices.Contains(research.Tools.List(), "web_search") { + t.Fatalf("research tools = %v, expected web_search to be absent", research.Tools.List()) } } diff --git a/pkg/agent/steering.go b/pkg/agent/steering.go index ad6613e8c..7ce918dd8 100644 --- a/pkg/agent/steering.go +++ b/pkg/agent/steering.go @@ -325,7 +325,10 @@ func (al *AgentLoop) agentForSession(sessionKey string) *AgentInstance { // user has since enqueued steering messages. // // If no steering messages are pending, it returns an empty string. -func (al *AgentLoop) Continue(ctx context.Context, sessionKey, channel, chatID string) (string, error) { +func (al *AgentLoop) Continue( + ctx context.Context, + sessionKey, channel, chatID string, +) (string, error) { if active := al.GetActiveTurn(); active != nil { return "", fmt.Errorf("turn %s is still active", active.TurnID) } diff --git a/pkg/agent/steering_test.go b/pkg/agent/steering_test.go index 75ba9861d..deb4f07c5 100644 --- a/pkg/agent/steering_test.go +++ b/pkg/agent/steering_test.go @@ -896,7 +896,10 @@ func TestAgentLoop_Run_AutoContinuesLateSteeringMessage(t *testing.T) { defer cancelNoExtra() select { case out2 := <-msgBus.OutboundChan(): - t.Fatalf("expected stale direct response to be suppressed, got extra outbound %q", out2.Content) + t.Fatalf( + "expected stale direct response to be suppressed, got extra outbound %q", + out2.Content, + ) case <-noExtraCtx.Done(): } @@ -1044,7 +1047,11 @@ func TestAgentLoop_Continue_PreservesSteeringMedia(t *testing.T) { if err = os.WriteFile(pngPath, pngHeader, 0o644); err != nil { t.Fatalf("WriteFile failed: %v", err) } - ref, err := store.Store(pngPath, media.MediaMeta{Filename: "steer.png", ContentType: "image/png"}, "test") + ref, err := store.Store( + pngPath, + media.MediaMeta{Filename: "steer.png", ContentType: "image/png"}, + "test", + ) if err != nil { t.Fatalf("Store failed: %v", err) } @@ -1236,7 +1243,10 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) { t.Fatalf("expected 2 provider calls, got %d", calls) } if terminalToolsCount != 0 { - t.Fatalf("expected graceful terminal call to disable tools, got %d tool defs", terminalToolsCount) + t.Fatalf( + "expected graceful terminal call to disable tools, got %d tool defs", + terminalToolsCount, + ) } foundHint := false @@ -1247,7 +1257,8 @@ func TestAgentLoop_InterruptGraceful_UsesTerminalNoToolCall(t *testing.T) { if msg.Role == "user" && msg.Content == expectedHint { foundHint = true } - if msg.Role == "tool" && msg.ToolCallID == "call_2" && msg.Content == "Skipped due to graceful interrupt." { + if msg.Role == "tool" && msg.ToolCallID == "call_2" && + msg.Content == "Skipped due to graceful interrupt." { foundSkipped = true } } @@ -1539,7 +1550,8 @@ func TestAgentLoop_Steering_SkippedToolsHaveErrorResults(t *testing.T) { foundSkipped := false for _, m := range msgs { - if m.Role == "tool" && m.ToolCallID == "call_2" && m.Content == "Skipped due to queued user message." { + if m.Role == "tool" && m.ToolCallID == "call_2" && + m.Content == "Skipped due to queued user message." { foundSkipped = true break } @@ -1547,7 +1559,13 @@ func TestAgentLoop_Steering_SkippedToolsHaveErrorResults(t *testing.T) { if !foundSkipped { // Log what we actually got for i, m := range msgs { - t.Logf("msg[%d]: role=%s toolCallID=%s content=%s", i, m.Role, m.ToolCallID, truncate(m.Content, 80)) + t.Logf( + "msg[%d]: role=%s toolCallID=%s content=%s", + i, + m.Role, + m.ToolCallID, + truncate(m.Content, 80), + ) } t.Fatal("expected skipped tool result for call_2") } diff --git a/pkg/agent/subturn.go b/pkg/agent/subturn.go index f5ba412ab..4fcbb089c 100644 --- a/pkg/agent/subturn.go +++ b/pkg/agent/subturn.go @@ -505,7 +505,12 @@ func spawnSubTurn( // Event emissions: // - SubTurnResultDeliveredEvent: successful delivery to channel // - SubTurnOrphanResultEvent: delivery failed (parent finished or channel full) -func deliverSubTurnResult(al *AgentLoop, parentTS *turnState, childID string, result *tools.ToolResult) { +func deliverSubTurnResult( + al *AgentLoop, + parentTS *turnState, + childID string, + result *tools.ToolResult, +) { // Let GC clean up the pendingResults channel; parent Finish will no longer close it. // We use defer/recover to catch any unlikely channel panics if it were ever closed. defer func() { @@ -516,9 +521,14 @@ func deliverSubTurnResult(al *AgentLoop, parentTS *turnState, childID string, re "recover": r, }) if result != nil && al != nil { - al.emitEvent(EventKindSubTurnOrphan, + al.emitEvent( + EventKindSubTurnOrphan, parentTS.eventMeta("deliverSubTurnResult", "subturn.orphan"), - SubTurnOrphanPayload{ParentTurnID: parentTS.turnID, ChildTurnID: childID, Reason: "panic"}, + SubTurnOrphanPayload{ + ParentTurnID: parentTS.turnID, + ChildTurnID: childID, + Reason: "panic", + }, ) } } @@ -531,9 +541,14 @@ func deliverSubTurnResult(al *AgentLoop, parentTS *turnState, childID string, re // If parent turn has already finished, treat this as an orphan result if isFinished || resultChan == nil { if result != nil && al != nil { - al.emitEvent(EventKindSubTurnOrphan, + al.emitEvent( + EventKindSubTurnOrphan, parentTS.eventMeta("deliverSubTurnResult", "subturn.orphan"), - SubTurnOrphanPayload{ParentTurnID: parentTS.turnID, ChildTurnID: childID, Reason: "parent_finished"}, + SubTurnOrphanPayload{ + ParentTurnID: parentTS.turnID, + ChildTurnID: childID, + Reason: "parent_finished", + }, ) } return diff --git a/pkg/agent/subturn_test.go b/pkg/agent/subturn_test.go index 6a2ba835d..ef5a03b20 100644 --- a/pkg/agent/subturn_test.go +++ b/pkg/agent/subturn_test.go @@ -571,7 +571,8 @@ func TestHardAbortSessionRollback(t *testing.T) { } // Verify the content matches the initial state - if finalHistory[0].Content != "initial message 1" || finalHistory[1].Content != "initial response 1" { + if finalHistory[0].Content != "initial message 1" || + finalHistory[1].Content != "initial response 1" { t.Error("history content does not match initial state after rollback") } } @@ -1290,7 +1291,12 @@ func TestDeliverSubTurnResult_RaceWithFinish(t *testing.T) { finalOrphan := orphanCount mu.Unlock() - t.Logf("Delivered: %d, Orphan: %d, Total: %d", finalDelivered, finalOrphan, finalDelivered+finalOrphan) + t.Logf( + "Delivered: %d, Orphan: %d, Total: %d", + finalDelivered, + finalOrphan, + finalDelivered+finalOrphan, + ) // With the new drainPendingResults behavior, the total events may be >= numResults // because Finish() drains remaining results from the channel and emits them as orphans. diff --git a/pkg/agent/tool_allowlist.go b/pkg/agent/tool_allowlist.go new file mode 100644 index 000000000..41b1fb98b --- /dev/null +++ b/pkg/agent/tool_allowlist.go @@ -0,0 +1,30 @@ +package agent + +import ( + "sort" + "strings" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func resolveAgentToolAllowlist(agentCfg *config.AgentConfig) []string { + if agentCfg == nil || agentCfg.Tools == nil { + return nil + } + + allowlist := make(map[string]struct{}, len(agentCfg.Tools)) + for _, raw := range agentCfg.Tools { + trimmed := strings.ToLower(strings.TrimSpace(raw)) + if trimmed == "" { + continue + } + allowlist[trimmed] = struct{}{} + } + + result := make([]string, 0, len(allowlist)) + for name := range allowlist { + result = append(result, name) + } + sort.Strings(result) + return result +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 533f45a44..8f793526b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -106,18 +106,18 @@ const CurrentVersion = 1 // Config is the current config structure with version support type Config struct { - Version int `json:"version" yaml:"-"` // Config schema version for migration - Agents AgentsConfig `json:"agents" yaml:"-"` - Bindings []AgentBinding `json:"bindings,omitempty" yaml:"-"` - Session SessionConfig `json:"session,omitempty" yaml:"-"` - Channels ChannelsConfig `json:"channels" yaml:"channels"` - ModelList SecureModelList `json:"model_list" yaml:"model_list"` // New model-centric provider configuration - Gateway GatewayConfig `json:"gateway" yaml:"-"` - Hooks HooksConfig `json:"hooks,omitempty" yaml:"-"` - Tools ToolsConfig `json:"tools" yaml:",inline"` - Heartbeat HeartbeatConfig `json:"heartbeat" yaml:"-"` - Devices DevicesConfig `json:"devices" yaml:"-"` - Voice VoiceConfig `json:"voice" yaml:"-"` + Version int `json:"version" yaml:"-"` // Config schema version for migration + Agents AgentsConfig `json:"agents" yaml:"-"` + Bindings []AgentBinding `json:"bindings,omitempty" yaml:"-"` + Session SessionConfig `json:"session,omitempty" yaml:"-"` + Channels ChannelsConfig `json:"channels" yaml:"channels"` + ModelList SecureModelList `json:"model_list" yaml:"model_list"` // New model-centric provider configuration + Gateway GatewayConfig `json:"gateway" yaml:"-"` + Hooks HooksConfig `json:"hooks,omitempty" yaml:"-"` + Tools ToolsConfig `json:"tools" yaml:",inline"` + Heartbeat HeartbeatConfig `json:"heartbeat" yaml:"-"` + Devices DevicesConfig `json:"devices" yaml:"-"` + Voice VoiceConfig `json:"voice" yaml:"-"` // BuildInfo contains build-time version information BuildInfo BuildInfo `json:"build_info,omitempty" yaml:"-"` @@ -248,6 +248,7 @@ type AgentConfig struct { Name string `json:"name,omitempty"` Workspace string `json:"workspace,omitempty"` Model *AgentModelConfig `json:"model,omitempty"` + Tools []string `json:"tools,omitempty"` Skills []string `json:"skills,omitempty"` Subagents *SubagentsConfig `json:"subagents,omitempty"` } @@ -818,8 +819,8 @@ type GLMSearchConfig struct { BaseURL string `json:"base_url" yaml:"-" env:"PICOCLAW_TOOLS_WEB_GLM_BASE_URL"` // SearchEngine specifies the search backend: "search_std" (default), // "search_pro", "search_pro_sogou", or "search_pro_quark". - SearchEngine string `json:"search_engine" yaml:"-" env:"PICOCLAW_TOOLS_WEB_GLM_SEARCH_ENGINE"` - MaxResults int `json:"max_results" yaml:"-" env:"PICOCLAW_TOOLS_WEB_GLM_MAX_RESULTS"` + SearchEngine string `json:"search_engine" yaml:"-" env:"PICOCLAW_TOOLS_WEB_GLM_SEARCH_ENGINE"` + MaxResults int `json:"max_results" yaml:"-" env:"PICOCLAW_TOOLS_WEB_GLM_MAX_RESULTS"` } type BaiduSearchConfig struct { @@ -830,7 +831,7 @@ type BaiduSearchConfig struct { } type WebToolsConfig struct { - ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_"` + ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_"` Brave BraveConfig `yaml:"brave,omitempty" json:"brave"` Tavily TavilyConfig `yaml:"tavily,omitempty" json:"tavily"` DuckDuckGo DuckDuckGoConfig `yaml:"-" json:"duckduckgo"` @@ -843,13 +844,13 @@ type WebToolsConfig struct { // the client-side web_search tool is hidden to avoid duplicate search surfaces, // and the provider's built-in search is used instead. Falls back to client-side // search when the provider does not support native search. - PreferNative bool `json:"prefer_native" yaml:"-" env:"PICOCLAW_TOOLS_WEB_PREFER_NATIVE"` + PreferNative bool `yaml:"-" json:"prefer_native" env:"PICOCLAW_TOOLS_WEB_PREFER_NATIVE"` // Proxy is an optional proxy URL for web tools (http/https/socks5/socks5h). // For authenticated proxies, prefer HTTP_PROXY/HTTPS_PROXY env vars instead of embedding credentials in config. - Proxy string `json:"proxy,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WEB_PROXY"` - FetchLimitBytes int64 `json:"fetch_limit_bytes,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WEB_FETCH_LIMIT_BYTES"` - Format string `json:"format,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WEB_FORMAT"` - PrivateHostWhitelist FlexibleStringSlice `json:"private_host_whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WEB_PRIVATE_HOST_WHITELIST"` + Proxy string `yaml:"-" json:"proxy,omitempty" env:"PICOCLAW_TOOLS_WEB_PROXY"` + FetchLimitBytes int64 `yaml:"-" json:"fetch_limit_bytes,omitempty" env:"PICOCLAW_TOOLS_WEB_FETCH_LIMIT_BYTES"` + Format string `yaml:"-" json:"format,omitempty" env:"PICOCLAW_TOOLS_WEB_FORMAT"` + PrivateHostWhitelist FlexibleStringSlice `yaml:"-" json:"private_host_whitelist,omitempty" env:"PICOCLAW_TOOLS_WEB_PRIVATE_HOST_WHITELIST"` } type CronToolsConfig struct { @@ -887,37 +888,37 @@ type ReadFileToolConfig struct { } type ToolsConfig struct { - AllowReadPaths []string `json:"allow_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"` - AllowWritePaths []string `json:"allow_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"` + AllowReadPaths []string `json:"allow_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"` + AllowWritePaths []string `json:"allow_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"` // FilterSensitiveData controls whether to filter sensitive values (API keys, // tokens, secrets) from tool results before sending to the LLM. // Default: true (enabled) - FilterSensitiveData bool `json:"filter_sensitive_data" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_SENSITIVE_DATA"` + FilterSensitiveData bool `json:"filter_sensitive_data" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_SENSITIVE_DATA"` // FilterMinLength is the minimum content length required for filtering. // Content shorter than this will be returned unchanged for performance. // Default: 8 - FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` - Web WebToolsConfig `json:"web" yaml:"web,omitempty"` - Cron CronToolsConfig `json:"cron" yaml:"-"` - Exec ExecConfig `json:"exec" yaml:"-"` - Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` - MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` - MCP MCPConfig `json:"mcp" yaml:"-"` - AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` - EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` - FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` - I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` - InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` - ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` - Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` - ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` - SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` - Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` - SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` - SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` - Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` - WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` - WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` + FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` + Web WebToolsConfig `json:"web" yaml:"web,omitempty"` + Cron CronToolsConfig `json:"cron" yaml:"-"` + Exec ExecConfig `json:"exec" yaml:"-"` + Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` + MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` + MCP MCPConfig `json:"mcp" yaml:"-"` + AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` + EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` + FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` + I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` + InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` + ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` + Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` + ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` + SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` + Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` + SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` + SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` + Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` + WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` + WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` } // IsFilterSensitiveDataEnabled returns true if sensitive data filtering is enabled @@ -985,10 +986,10 @@ type MCPServerConfig struct { // MCPConfig defines configuration for all MCP servers type MCPConfig struct { - ToolConfig ` envPrefix:"PICOCLAW_TOOLS_MCP_"` + ToolConfig ` envPrefix:"PICOCLAW_TOOLS_MCP_"` Discovery ToolDiscoveryConfig ` json:"discovery"` // Servers is a map of server name to server configuration - Servers map[string]MCPServerConfig `json:"servers,omitempty"` + Servers map[string]MCPServerConfig ` json:"servers,omitempty"` } func LoadConfig(path string) (*Config, error) { @@ -999,7 +1000,10 @@ func LoadConfig(path string) (*Config, error) { data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { - logger.WarnF("config file not found, using default config", map[string]any{"path": path}) + logger.WarnF( + "config file not found, using default config", + map[string]any{"path": path}, + ) return DefaultConfig(), nil } logger.Errorf("failed to read config file: %v", err) @@ -1022,7 +1026,10 @@ func LoadConfig(path string) (*Config, error) { var cfg *Config switch versionInfo.Version { case 0: - logger.InfoF("config migrate start", map[string]any{"from": versionInfo.Version, "to": CurrentVersion}) + logger.InfoF( + "config migrate start", + map[string]any{"from": versionInfo.Version, "to": CurrentVersion}, + ) // Legacy config (no version field) v, e := loadConfigV0(data) if e != nil { @@ -1030,10 +1037,16 @@ func LoadConfig(path string) (*Config, error) { } cfg, e = v.Migrate() if e != nil { - logger.ErrorF("config migrate fail", map[string]any{"from": versionInfo.Version, "to": CurrentVersion}) + logger.ErrorF( + "config migrate fail", + map[string]any{"from": versionInfo.Version, "to": CurrentVersion}, + ) return nil, e } - logger.InfoF("config migrate success", map[string]any{"from": versionInfo.Version, "to": CurrentVersion}) + logger.InfoF( + "config migrate success", + map[string]any{"from": versionInfo.Version, "to": CurrentVersion}, + ) err = makeBackup(path) if err != nil { return nil, err @@ -1041,7 +1054,10 @@ func LoadConfig(path string) (*Config, error) { // Load existing security config and merge with migrated one to prevent data loss secErr := loadSecurityConfig(cfg, securityPath(path)) if secErr != nil && !os.IsNotExist(secErr) { - logger.WarnF("failed to load existing security config during migration", map[string]any{"error": secErr}) + logger.WarnF( + "failed to load existing security config during migration", + map[string]any{"error": secErr}, + ) return nil, fmt.Errorf("failed to load existing security config: %w", secErr) } defer func(cfg *Config) { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 75eb458b8..a22bcd7cb 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -113,17 +113,18 @@ func TestAgentConfig_FullParse(t *testing.T) { "name": "Sales Bot", "model": "gpt-4" }, - { - "id": "support", - "name": "Support Bot", - "model": { - "primary": "claude-opus", - "fallbacks": ["haiku"] - }, - "subagents": { - "allow_agents": ["sales"] - } + { + "id": "support", + "name": "Support Bot", + "model": { + "primary": "claude-opus", + "fallbacks": ["haiku"] + }, + "tools": ["read_file", "web_search"], + "subagents": { + "allow_agents": ["sales"] } + } ] }, "bindings": [ @@ -171,6 +172,10 @@ func TestAgentConfig_FullParse(t *testing.T) { if len(support.Model.Fallbacks) != 1 || support.Model.Fallbacks[0] != "haiku" { t.Errorf("support.Model.Fallbacks = %v", support.Model.Fallbacks) } + if len(support.Tools) != 2 || support.Tools[0] != "read_file" || + support.Tools[1] != "web_search" { + t.Errorf("support.Tools = %v", support.Tools) + } if support.Subagents == nil || len(support.Subagents.AllowAgents) != 1 { t.Errorf("support.Subagents = %+v", support.Subagents) } @@ -182,7 +187,8 @@ func TestAgentConfig_FullParse(t *testing.T) { if binding.AgentID != "support" || binding.Match.Channel != "telegram" { t.Errorf("binding = %+v", binding) } - if binding.Match.Peer == nil || binding.Match.Peer.Kind != "direct" || binding.Match.Peer.ID != "user123" { + if binding.Match.Peer == nil || binding.Match.Peer.Kind != "direct" || + binding.Match.Peer.ID != "user123" { t.Errorf("binding.Match.Peer = %+v", binding.Match.Peer) } @@ -387,7 +393,9 @@ func TestSaveConfig_PreservesDisabledTelegramPlaceholder(t *testing.T) { t.Fatalf("LoadConfig failed: %v", err) } if loaded.Channels.Telegram.Placeholder.Enabled { - t.Fatal("telegram placeholder should remain disabled after SaveConfig/LoadConfig round-trip") + t.Fatal( + "telegram placeholder should remain disabled after SaveConfig/LoadConfig round-trip", + ) } } @@ -510,7 +518,9 @@ func TestLoadConfig_ToolFeedbackDefaultsFalseWhenUnset(t *testing.T) { t.Fatalf("LoadConfig() error: %v", err) } if cfg.Agents.Defaults.ToolFeedback.Enabled { - t.Fatal("agents.defaults.tool_feedback.enabled should remain false when unset in config file") + t.Fatal( + "agents.defaults.tool_feedback.enabled should remain false when unset in config file", + ) } } @@ -764,7 +774,10 @@ func TestDefaultConfig_SummarizationThresholds(t *testing.T) { cfg := DefaultConfig() if cfg.Agents.Defaults.SummarizeMessageThreshold != 20 { - t.Errorf("SummarizeMessageThreshold = %d, want 20", cfg.Agents.Defaults.SummarizeMessageThreshold) + t.Errorf( + "SummarizeMessageThreshold = %d, want 20", + cfg.Agents.Defaults.SummarizeMessageThreshold, + ) } if cfg.Agents.Defaults.SummarizeTokenPercent != 75 { t.Errorf("SummarizeTokenPercent = %d, want 75", cfg.Agents.Defaults.SummarizeTokenPercent) @@ -806,7 +819,11 @@ func TestDefaultConfig_WorkspacePath_WithPicoclawHome(t *testing.T) { want := filepath.Join("/custom/picoclaw/home", "workspace") if cfg.Agents.Defaults.Workspace != want { - t.Errorf("Workspace path with PICOCLAW_HOME = %q, want %q", cfg.Agents.Defaults.Workspace, want) + t.Errorf( + "Workspace path with PICOCLAW_HOME = %q, want %q", + cfg.Agents.Defaults.Workspace, + want, + ) } } @@ -885,7 +902,12 @@ func TestFlexibleStringSlice_UnmarshalText(t *testing.T) { } if len(f) != len(tt.expected) { - t.Errorf("UnmarshalText(%q) length = %d, want %d", tt.input, len(f), len(tt.expected)) + t.Errorf( + "UnmarshalText(%q) length = %d, want %d", + tt.input, + len(f), + len(tt.expected), + ) return } @@ -1006,7 +1028,8 @@ func TestLoadConfig_TelegramPlaceholderTextAcceptsSingleString(t *testing.T) { if err != nil { t.Fatalf("LoadConfig() error = %v", err) } - if got := []string(cfg.Channels.Telegram.Placeholder.Text); len(got) != 1 || got[0] != "Thinking..." { + if got := []string(cfg.Channels.Telegram.Placeholder.Text); len(got) != 1 || + got[0] != "Thinking..." { t.Fatalf("placeholder.text = %#v, want [\"Thinking...\"]", got) } } @@ -1196,9 +1219,21 @@ func TestSaveConfig_MixedKeys(t *testing.T) { cfg := &Config{ Version: CurrentVersion, ModelList: []*ModelConfig{ - {ModelName: "plain", Model: "openai/gpt-4", APIKeys: SimpleSecureStrings("sk-new-plaintext")}, - {ModelName: "enc", Model: "openai/gpt-4", APIKeys: SimpleSecureStrings(alreadyEncrypted)}, - {ModelName: "file", Model: "openai/gpt-4", APIKeys: SimpleSecureStrings("file://api.key")}, + { + ModelName: "plain", + Model: "openai/gpt-4", + APIKeys: SimpleSecureStrings("sk-new-plaintext"), + }, + { + ModelName: "enc", + Model: "openai/gpt-4", + APIKeys: SimpleSecureStrings(alreadyEncrypted), + }, + { + ModelName: "file", + Model: "openai/gpt-4", + APIKeys: SimpleSecureStrings("file://api.key"), + }, }, } if err := SaveConfig(cfgPath, cfg); err != nil { @@ -1335,7 +1370,10 @@ func TestSaveConfig_UsesPassphraseProvider(t *testing.T) { raw, _ := os.ReadFile(filepath.Join(dir, SecurityConfigFile)) if !strings.Contains(string(raw), "enc://") { - t.Errorf("SaveConfig should have encrypted plaintext key via PassphraseProvider; got:\n%s", raw) + t.Errorf( + "SaveConfig should have encrypted plaintext key via PassphraseProvider; got:\n%s", + raw, + ) } } @@ -1587,9 +1625,13 @@ func TestFilterSensitiveData_AllTokenTypes(t *testing.T) { FilterMinLength: 8, // Web tool API keys Web: WebToolsConfig{ - Brave: BraveConfig{APIKeys: SecureStrings{NewSecureString("brave-api-key")}}, - Tavily: TavilyConfig{APIKeys: SecureStrings{NewSecureString("tavily-api-key")}}, - Perplexity: PerplexityConfig{APIKeys: SecureStrings{NewSecureString("perplexity-api-key")}}, + Brave: BraveConfig{APIKeys: SecureStrings{NewSecureString("brave-api-key")}}, + Tavily: TavilyConfig{ + APIKeys: SecureStrings{NewSecureString("tavily-api-key")}, + }, + Perplexity: PerplexityConfig{ + APIKeys: SecureStrings{NewSecureString("perplexity-api-key")}, + }, GLMSearch: GLMSearchConfig{APIKey: *NewSecureString("glm-search-key")}, BaiduSearch: BaiduSearchConfig{APIKey: *NewSecureString("baidu-search-key")}, }, @@ -1597,7 +1639,9 @@ func TestFilterSensitiveData_AllTokenTypes(t *testing.T) { Skills: SkillsToolsConfig{ Github: SkillsGithubConfig{Token: *NewSecureString("github-token-xyz")}, Registries: SkillsRegistriesConfig{ - ClawHub: ClawHubRegistryConfig{AuthToken: *NewSecureString("clawhub-auth-token")}, + ClawHub: ClawHubRegistryConfig{ + AuthToken: *NewSecureString("clawhub-auth-token"), + }, }, }, }, diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index bc4ab0649..20e2e531d 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -65,7 +65,11 @@ func DefaultConfig() *Config { Enabled: true, Text: FlexibleStringSlice{"Thinking... 💭"}, }, - Streaming: StreamingConfig{Enabled: true, ThrottleSeconds: 3, MinGrowthChars: 200}, + Streaming: StreamingConfig{ + Enabled: true, + ThrottleSeconds: 3, + MinGrowthChars: 200, + }, UseMarkdownV2: false, }, Feishu: FeishuConfig{ diff --git a/pkg/config/migration.go b/pkg/config/migration.go index fee800a76..43f9645a2 100644 --- a/pkg/config/migration.go +++ b/pkg/config/migration.go @@ -335,7 +335,8 @@ func v0ConvertProvidersToModelList(cfg *configV0) []modelConfigV0 { providerNames: []string{"github_copilot", "copilot"}, protocol: "github-copilot", buildConfig: func(p providersConfigV0) (modelConfigV0, bool) { - if p.GitHubCopilot.APIKey == "" && p.GitHubCopilot.APIBase == "" && p.GitHubCopilot.ConnectMode == "" { + if p.GitHubCopilot.APIKey == "" && p.GitHubCopilot.APIBase == "" && + p.GitHubCopilot.ConnectMode == "" { return modelConfigV0{}, false } return modelConfigV0{ diff --git a/pkg/config/migration_integration_test.go b/pkg/config/migration_integration_test.go index bc8160967..b6a70c2ef 100644 --- a/pkg/config/migration_integration_test.go +++ b/pkg/config/migration_integration_test.go @@ -72,7 +72,11 @@ func TestMigration_Integration_LegacyConfigWithoutWorkspace(t *testing.T) { // CRITICAL: Verify that user's settings are preserved // This was the bug - these settings were lost when Workspace was empty if cfg.Agents.Defaults.Provider != "openai" { - t.Errorf("Provider = %q, want %q (user's setting should be preserved)", cfg.Agents.Defaults.Provider, "openai") + t.Errorf( + "Provider = %q, want %q (user's setting should be preserved)", + cfg.Agents.Defaults.Provider, + "openai", + ) } // Old "model" field is migrated to "model_name" field if cfg.Agents.Defaults.ModelName != "gpt-4o" { @@ -299,7 +303,11 @@ func TestMigration_Integration_PreservesAllAgentsFields(t *testing.T) { t.Errorf("Agent.ID = %q, want %q", cfg.Agents.List[0].ID, "special-agent") } if cfg.Agents.List[0].Workspace != "/special/workspace" { - t.Errorf("Agent.Workspace = %q, want %q", cfg.Agents.List[0].Workspace, "/special/workspace") + t.Errorf( + "Agent.Workspace = %q, want %q", + cfg.Agents.List[0].Workspace, + "/special/workspace", + ) } // Workspace should have default since it was empty in legacy config @@ -362,7 +370,10 @@ func TestMigration_Integration_ChannelsConfigMigrated(t *testing.T) { // OneBot: group_trigger_prefix should be migrated to group_trigger.prefixes if len(cfg.Channels.OneBot.GroupTrigger.Prefixes) != 2 { - t.Errorf("len(OneBot.GroupTrigger.Prefixes) = %d, want 2", len(cfg.Channels.OneBot.GroupTrigger.Prefixes)) + t.Errorf( + "len(OneBot.GroupTrigger.Prefixes) = %d, want 2", + len(cfg.Channels.OneBot.GroupTrigger.Prefixes), + ) } else { if cfg.Channels.OneBot.GroupTrigger.Prefixes[0] != "/" { t.Errorf("Prefixes[0] = %q, want %q", cfg.Channels.OneBot.GroupTrigger.Prefixes[0], "/") @@ -443,13 +454,25 @@ func TestMigration_Integration_RoundTrip_SerializeAndLoad(t *testing.T) { // Verify configs are identical if cfg2.Agents.Defaults.Provider != cfg1.Agents.Defaults.Provider { - t.Errorf("Provider changed from %q to %q", cfg1.Agents.Defaults.Provider, cfg2.Agents.Defaults.Provider) + t.Errorf( + "Provider changed from %q to %q", + cfg1.Agents.Defaults.Provider, + cfg2.Agents.Defaults.Provider, + ) } if cfg2.Agents.Defaults.ModelName != cfg1.Agents.Defaults.ModelName { - t.Errorf("ModelName changed from %q to %q", cfg1.Agents.Defaults.ModelName, cfg2.Agents.Defaults.ModelName) + t.Errorf( + "ModelName changed from %q to %q", + cfg1.Agents.Defaults.ModelName, + cfg2.Agents.Defaults.ModelName, + ) } if cfg2.Agents.Defaults.MaxTokens != cfg1.Agents.Defaults.MaxTokens { - t.Errorf("MaxTokens changed from %d to %d", cfg1.Agents.Defaults.MaxTokens, cfg2.Agents.Defaults.MaxTokens) + t.Errorf( + "MaxTokens changed from %d to %d", + cfg1.Agents.Defaults.MaxTokens, + cfg2.Agents.Defaults.MaxTokens, + ) } } @@ -557,7 +580,11 @@ func TestMigration_Integration_ModelNameField(t *testing.T) { // GetModelName() should return model_name, not model (deprecated) if cfg.Agents.Defaults.GetModelName() != "deepseek-reasoner" { - t.Errorf("GetModelName() = %q, want %q", cfg.Agents.Defaults.GetModelName(), "deepseek-reasoner") + t.Errorf( + "GetModelName() = %q, want %q", + cfg.Agents.Defaults.GetModelName(), + "deepseek-reasoner", + ) } if len(cfg.Agents.Defaults.ModelFallbacks) != 1 { diff --git a/pkg/config/migration_test.go b/pkg/config/migration_test.go index aeabe9730..1ae3c7b71 100644 --- a/pkg/config/migration_test.go +++ b/pkg/config/migration_test.go @@ -91,9 +91,11 @@ func TestConvertProvidersToModelList_LiteLLM(t *testing.T) { func TestConvertProvidersToModelList_Multiple(t *testing.T) { cfg := &configV0{ Providers: providersConfigV0{ - OpenAI: openAIProviderConfigV0{providerConfigV0: providerConfigV0{APIKey: "openai-key"}}, - Groq: providerConfigV0{APIKey: "groq-key"}, - Zhipu: providerConfigV0{APIKey: "zhipu-key"}, + OpenAI: openAIProviderConfigV0{ + providerConfigV0: providerConfigV0{APIKey: "openai-key"}, + }, + Groq: providerConfigV0{APIKey: "groq-key"}, + Zhipu: providerConfigV0{APIKey: "zhipu-key"}, }, } @@ -142,8 +144,13 @@ func TestConvertProvidersToModelList_AllProviders(t *testing.T) { // Other providers have no configuration, so they won't be converted. cfg := &configV0{ Providers: providersConfigV0{ - OpenAI: openAIProviderConfigV0{providerConfigV0: providerConfigV0{APIKey: "key1"}}, - LiteLLM: providerConfigV0{APIKey: "key-litellm", APIBase: "http://localhost:4000/v1"}, + OpenAI: openAIProviderConfigV0{ + providerConfigV0: providerConfigV0{APIKey: "key1"}, + }, + LiteLLM: providerConfigV0{ + APIKey: "key-litellm", + APIBase: "http://localhost:4000/v1", + }, Anthropic: providerConfigV0{APIKey: "key2"}, OpenRouter: providerConfigV0{APIKey: "key3"}, Groq: providerConfigV0{APIKey: "key4"}, @@ -261,7 +268,11 @@ func TestConvertProvidersToModelList_PreservesUserModel_DeepSeek(t *testing.T) { // Should use user's model, not default if result[0].Model != "deepseek/deepseek-reasoner" { - t.Errorf("Model = %q, want %q (user's configured model)", result[0].Model, "deepseek/deepseek-reasoner") + t.Errorf( + "Model = %q, want %q (user's configured model)", + result[0].Model, + "deepseek/deepseek-reasoner", + ) } } @@ -371,7 +382,9 @@ func TestConvertProvidersToModelList_MultipleProviders_PreservesUserModel(t *tes }, }, Providers: providersConfigV0{ - OpenAI: openAIProviderConfigV0{providerConfigV0: providerConfigV0{APIKey: "sk-openai"}}, + OpenAI: openAIProviderConfigV0{ + providerConfigV0: providerConfigV0{APIKey: "sk-openai"}, + }, DeepSeek: providerConfigV0{APIKey: "sk-deepseek"}, }, } @@ -391,7 +404,11 @@ func TestConvertProvidersToModelList_MultipleProviders_PreservesUserModel(t *tes } case "deepseek": if mc.Model != "deepseek/deepseek-reasoner" { - t.Errorf("DeepSeek Model = %q, want %q (user's)", mc.Model, "deepseek/deepseek-reasoner") + t.Errorf( + "DeepSeek Model = %q, want %q (user's)", + mc.Model, + "deepseek/deepseek-reasoner", + ) } } } @@ -489,7 +506,11 @@ func TestConvertProvidersToModelList_NoProviderField_SingleProvider(t *testing.T // ModelName should be the user's model value for backward compatibility if result[0].ModelName != "glm-4.7" { - t.Errorf("ModelName = %q, want %q (user's model for backward compatibility)", result[0].ModelName, "glm-4.7") + t.Errorf( + "ModelName = %q, want %q (user's model for backward compatibility)", + result[0].ModelName, + "glm-4.7", + ) } // Model should use the user's model with protocol prefix @@ -510,8 +531,10 @@ func TestConvertProvidersToModelList_NoProviderField_MultipleProviders(t *testin }, }, Providers: providersConfigV0{ - OpenAI: openAIProviderConfigV0{providerConfigV0: providerConfigV0{APIKey: "openai-key"}}, - Zhipu: providerConfigV0{APIKey: "zhipu-key"}, + OpenAI: openAIProviderConfigV0{ + providerConfigV0: providerConfigV0{APIKey: "openai-key"}, + }, + Zhipu: providerConfigV0{APIKey: "zhipu-key"}, }, } @@ -571,7 +594,11 @@ func TestBuildModelWithProtocol_NoPrefix(t *testing.T) { func TestBuildModelWithProtocol_AlreadyHasPrefix(t *testing.T) { result := buildModelWithProtocol("openrouter", "openrouter/auto") if result != "openrouter/auto" { - t.Errorf("buildModelWithProtocol(openrouter, openrouter/auto) = %q, want %q", result, "openrouter/auto") + t.Errorf( + "buildModelWithProtocol(openrouter, openrouter/auto) = %q, want %q", + result, + "openrouter/auto", + ) } } @@ -613,6 +640,10 @@ func TestConvertProvidersToModelList_LegacyModelWithProtocolPrefix(t *testing.T) // Model should NOT have duplicated prefix if result[0].Model != "openrouter/auto" { - t.Errorf("Model = %q, want %q (should not duplicate prefix)", result[0].Model, "openrouter/auto") + t.Errorf( + "Model = %q, want %q (should not duplicate prefix)", + result[0].Model, + "openrouter/auto", + ) } } diff --git a/pkg/config/model_config_test.go b/pkg/config/model_config_test.go index 6e88f4783..f001885af 100644 --- a/pkg/config/model_config_test.go +++ b/pkg/config/model_config_test.go @@ -17,7 +17,11 @@ func TestGetModelConfig_Found(t *testing.T) { Version: CurrentVersion, ModelList: []*ModelConfig{ {ModelName: "test-model", Model: "openai/gpt-4o", APIKeys: SimpleSecureStrings("key1")}, - {ModelName: "other-model", Model: "anthropic/claude", APIKeys: SimpleSecureStrings("key2")}, + { + ModelName: "other-model", + Model: "anthropic/claude", + APIKeys: SimpleSecureStrings("key2"), + }, }, } @@ -114,8 +118,16 @@ func TestGetModelConfig_RoundRobinStartsFromFirstMatch(t *testing.T) { func TestGetModelConfig_Concurrent(t *testing.T) { cfg := &Config{ ModelList: []*ModelConfig{ - {ModelName: "concurrent-model", Model: "openai/gpt-4o-1", APIKeys: SimpleSecureStrings("key1")}, - {ModelName: "concurrent-model", Model: "openai/gpt-4o-2", APIKeys: SimpleSecureStrings("key2")}, + { + ModelName: "concurrent-model", + Model: "openai/gpt-4o-1", + APIKeys: SimpleSecureStrings("key1"), + }, + { + ModelName: "concurrent-model", + Model: "openai/gpt-4o-2", + APIKeys: SimpleSecureStrings("key2"), + }, }, } @@ -290,7 +302,11 @@ func TestConfig_ValidateModelList(t *testing.T) { } if err != nil && tt.errMsg != "" { if !strings.Contains(err.Error(), tt.errMsg) { - t.Errorf("ValidateModelList() error = %v, want error containing %q", err, tt.errMsg) + t.Errorf( + "ValidateModelList() error = %v, want error containing %q", + err, + tt.errMsg, + ) } } }) diff --git a/pkg/config/multikey_test.go b/pkg/config/multikey_test.go index e58c6dc9e..28fd9ff7d 100644 --- a/pkg/config/multikey_test.go +++ b/pkg/config/multikey_test.go @@ -117,7 +117,10 @@ func TestExpandMultiKeyModels_WithExistingFallbacks(t *testing.T) { ModelName: "gpt-4", Model: "openai/gpt-4o", } - modelCfg.APIKeys = SimpleSecureStrings("key0", "key1") // Use internal field for multi-key testing + modelCfg.APIKeys = SimpleSecureStrings( + "key0", + "key1", + ) // Use internal field for multi-key testing modelCfg.Fallbacks = []string{"claude-3"} models := []*ModelConfig{modelCfg} @@ -196,7 +199,10 @@ func TestExpandMultiKeyModels_PreservesOtherFields(t *testing.T) { RequestTimeout: 30, ThinkingLevel: "high", } - modelCfg.APIKeys = SimpleSecureStrings("key0", "key1") // Use internal field for multi-key testing + modelCfg.APIKeys = SimpleSecureStrings( + "key0", + "key1", + ) // Use internal field for multi-key testing models := []*ModelConfig{modelCfg} result := expandMultiKeyModels(models) diff --git a/pkg/config/security.go b/pkg/config/security.go index 79dd26e14..c31e877c2 100644 --- a/pkg/config/security.go +++ b/pkg/config/security.go @@ -304,11 +304,13 @@ func (s *SecureString) UnmarshalJSON(value []byte) error { func (s SecureString) MarshalYAML() (any, error) { // Preserve raw value if it is already a reference (enc:// or file://) - if strings.HasPrefix(s.raw, credential.EncScheme) || strings.HasPrefix(s.raw, credential.FileScheme) { + if strings.HasPrefix(s.raw, credential.EncScheme) || + strings.HasPrefix(s.raw, credential.FileScheme) { return s.raw, nil } // If resolved is a reference format (e.g. set via Set), copy back to raw - if strings.HasPrefix(s.resolved, credential.EncScheme) || strings.HasPrefix(s.resolved, credential.FileScheme) { + if strings.HasPrefix(s.resolved, credential.EncScheme) || + strings.HasPrefix(s.resolved, credential.FileScheme) { s.raw = s.resolved return s.raw, nil } diff --git a/pkg/config/security_integration_test.go b/pkg/config/security_integration_test.go index 6ca8637f4..5f0b61970 100644 --- a/pkg/config/security_integration_test.go +++ b/pkg/config/security_integration_test.go @@ -35,7 +35,10 @@ func TestJSONUnmarshalPrivateFields(t *testing.T) { t.Errorf("PublicField = %q, want 'pub'", s.PublicField) } if s.privateField != "" { - t.Errorf("privateField = %q, want empty because unexported fields are ignored", s.privateField) + t.Errorf( + "privateField = %q, want empty because unexported fields are ignored", + s.privateField, + ) } } @@ -352,13 +355,21 @@ skills: // Verify Channel tokens via Key() methods // Telegram - assert.Equal(t, "123456789:ABCdefGHIjklMNOpqrsTUVwxyz", cfg.Channels.Telegram.Token.String()) + assert.Equal( + t, + "123456789:ABCdefGHIjklMNOpqrsTUVwxyz", + cfg.Channels.Telegram.Token.String(), + ) t.Logf("Telegram Token(): %s", cfg.Channels.Telegram.Token.String()) // Feishu assert.Equal(t, "feishu_test_app_secret", cfg.Channels.Feishu.AppSecret.String()) assert.Equal(t, "feishu_test_encrypt_key", cfg.Channels.Feishu.EncryptKey.String()) - assert.Equal(t, "feishu_test_verification_token", cfg.Channels.Feishu.VerificationToken.String()) + assert.Equal( + t, + "feishu_test_verification_token", + cfg.Channels.Feishu.VerificationToken.String(), + ) t.Logf("Feishu AppSecret(): %s", cfg.Channels.Feishu.AppSecret.String()) t.Logf("Feishu EncryptKey(): %s", cfg.Channels.Feishu.EncryptKey.String()) t.Logf("Feishu VerificationToken(): %s", cfg.Channels.Feishu.VerificationToken.String()) @@ -383,7 +394,11 @@ skills: // LINE assert.Equal(t, "line_test_channel_secret", cfg.Channels.LINE.ChannelSecret.String()) - assert.Equal(t, "line_test_channel_access_token", cfg.Channels.LINE.ChannelAccessToken.String()) + assert.Equal( + t, + "line_test_channel_access_token", + cfg.Channels.LINE.ChannelAccessToken.String(), + ) t.Logf("LINE ChannelSecret(): %s", cfg.Channels.LINE.ChannelSecret.String()) t.Logf("LINE ChannelAccessToken(): %s", cfg.Channels.LINE.ChannelAccessToken.String()) @@ -431,7 +446,11 @@ skills: assert.Equal(t, "ghp-github-from-file-abc123", cfg.Tools.Skills.Github.Token.String()) t.Logf("Github Token(): %s", cfg.Tools.Skills.Github.Token.String()) - assert.Equal(t, "clawhub-auth-token-from-file", cfg.Tools.Skills.Registries.ClawHub.AuthToken.String()) + assert.Equal( + t, + "clawhub-auth-token-from-file", + cfg.Tools.Skills.Registries.ClawHub.AuthToken.String(), + ) t.Logf("ClawHub AuthToken(): %s", cfg.Tools.Skills.Registries.ClawHub.AuthToken.String()) t.Log("All security keys are successfully accessible via their respective Key() methods") diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 60d9d5e5a..e205d7cf3 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -15,7 +15,10 @@ import ( // JobExecutor is the interface for executing cron jobs through the agent type JobExecutor interface { - ProcessDirectWithChannel(ctx context.Context, content, sessionKey, channel, chatID string) (string, error) + ProcessDirectWithChannel( + ctx context.Context, + content, sessionKey, channel, chatID string, + ) (string, error) // PublishResponseIfNeeded sends response to the outbound bus only when the // agent did not already deliver content through the message tool in this round. PublishResponseIfNeeded(ctx context.Context, channel, chatID, response string) @@ -34,8 +37,13 @@ type CronTool struct { // NewCronTool creates a new CronTool // execTimeout: 0 means no timeout, >0 sets the timeout duration func NewCronTool( - cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool, - execTimeout time.Duration, config *config.Config, + cronService *cron.CronService, + executor JobExecutor, + msgBus *bus.MessageBus, + workspace string, + restrict bool, + execTimeout time.Duration, + config *config.Config, ) (*CronTool, error) { allowCommand := true execEnabled := true @@ -156,7 +164,9 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult chatID := ToolChatID(ctx) if channel == "" || chatID == "" { - return ErrorResult("no session context (channel/chat_id not set). Use this tool in an active conversation.") + return ErrorResult( + "no session context (channel/chat_id not set). Use this tool in an active conversation.", + ) } message, ok := args["message"].(string) @@ -208,7 +218,9 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult // Validate type parameter (server-side whitelist, not just LLM schema hint) msgType, _ := args["type"].(string) if msgType != "" && msgType != "message" && msgType != "directive" { - return ErrorResult(fmt.Sprintf("invalid type %q, must be 'message' or 'directive'", msgType)) + return ErrorResult( + fmt.Sprintf("invalid type %q, must be 'message' or 'directive'", msgType), + ) } // GHSA-pv8c-p6jf-3fpp: command scheduling requires internal channel. When diff --git a/pkg/tools/cron_test.go b/pkg/tools/cron_test.go index 186c6a75e..251db5803 100644 --- a/pkg/tools/cron_test.go +++ b/pkg/tools/cron_test.go @@ -49,7 +49,11 @@ func (s *stubJobExecutor) PublishResponseIfNeeded( s.publishedChatID = chatID } -func newTestCronToolWithExecutorAndConfig(t *testing.T, executor JobExecutor, cfg *config.Config) *CronTool { +func newTestCronToolWithExecutorAndConfig( + t *testing.T, + executor JobExecutor, + cfg *config.Config, +) *CronTool { t.Helper() storePath := filepath.Join(t.TempDir(), "cron.json") cronService := cron.NewCronService(storePath, nil) @@ -102,7 +106,10 @@ func TestCronTool_CommandDoesNotRequireConfirmByDefault(t *testing.T) { }) if result.IsError { - t.Fatalf("expected command scheduling without confirm to succeed by default, got: %s", result.ForLLM) + t.Fatalf( + "expected command scheduling without confirm to succeed by default, got: %s", + result.ForLLM, + ) } if !strings.Contains(result.ForLLM, "Cron job added") { t.Errorf("expected 'Cron job added', got: %s", result.ForLLM) @@ -190,7 +197,10 @@ func TestCronTool_CommandAllowedFromInternalChannel(t *testing.T) { }) if result.IsError { - t.Fatalf("expected command scheduling to succeed from internal channel, got: %s", result.ForLLM) + t.Fatalf( + "expected command scheduling to succeed from internal channel, got: %s", + result.ForLLM, + ) } if !strings.Contains(result.ForLLM, "Cron job added") { t.Errorf("expected 'Cron job added', got: %s", result.ForLLM) @@ -225,7 +235,10 @@ func TestCronTool_NonCommandJobAllowedFromRemoteChannel(t *testing.T) { }) if result.IsError { - t.Fatalf("expected non-command reminder to succeed from remote channel, got: %s", result.ForLLM) + t.Fatalf( + "expected non-command reminder to succeed from remote channel, got: %s", + result.ForLLM, + ) } } @@ -297,7 +310,11 @@ func TestCronTool_ExecuteJobPublishesAgentResponse(t *testing.T) { t.Fatalf("sessionKey = %q, want cron-job-1", executor.lastKey) } if executor.lastChan != "telegram" || executor.lastChatID != "chat-1" { - t.Fatalf("executor target = %s/%s, want telegram/chat-1", executor.lastChan, executor.lastChatID) + t.Fatalf( + "executor target = %s/%s, want telegram/chat-1", + executor.lastChan, + executor.lastChatID, + ) } if executor.lastPrompt != "send me a poem" { t.Fatalf("prompt = %q, want original message", executor.lastPrompt) @@ -306,7 +323,11 @@ func TestCronTool_ExecuteJobPublishesAgentResponse(t *testing.T) { t.Fatalf("published response = %q, want generated reply", executor.publishedResp) } if executor.publishedChan != "telegram" || executor.publishedChatID != "chat-1" { - t.Fatalf("published target = %s/%s, want telegram/chat-1", executor.publishedChan, executor.publishedChatID) + t.Fatalf( + "published target = %s/%s, want telegram/chat-1", + executor.publishedChan, + executor.publishedChatID, + ) } } @@ -342,7 +363,10 @@ func TestCronTool_ExecuteJobSkipsWhenMessageToolAlreadySent(t *testing.T) { } if executor.publishedResp != "" { - t.Fatalf("expected no published response when message tool already sent, got: %q", executor.publishedResp) + t.Fatalf( + "expected no published response when message tool already sent, got: %q", + executor.publishedResp, + ) } } @@ -386,7 +410,9 @@ func TestCronTool_ExecuteJobDirectiveWithDeliverRoutesToAgent(t *testing.T) { } if executor.lastPrompt == "" { - t.Fatal("expected agent to be called for directive+deliver, but ProcessDirectWithChannel was not invoked") + t.Fatal( + "expected agent to be called for directive+deliver, but ProcessDirectWithChannel was not invoked", + ) } if executor.publishedResp != "agent processed" { t.Fatalf("published response = %q, want %q", executor.publishedResp, "agent processed") diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go index d5bebf4a2..78fc512c6 100644 --- a/pkg/tools/edit.go +++ b/pkg/tools/edit.go @@ -16,7 +16,11 @@ type EditFileTool struct { } // NewEditFileTool creates a new EditFileTool with optional directory restriction. -func NewEditFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *EditFileTool { +func NewEditFileTool( + workspace string, + restrict bool, + allowPaths ...[]*regexp.Regexp, +) *EditFileTool { var patterns []*regexp.Regexp if len(allowPaths) > 0 { patterns = allowPaths[0] @@ -79,7 +83,11 @@ type AppendFileTool struct { fs fileSystem } -func NewAppendFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *AppendFileTool { +func NewAppendFileTool( + workspace string, + restrict bool, + allowPaths ...[]*regexp.Regexp, +) *AppendFileTool { var patterns []*regexp.Regexp if len(allowPaths) > 0 { patterns = allowPaths[0] @@ -166,7 +174,10 @@ func replaceEditContent(content []byte, oldText, newText string) ([]byte, error) count := strings.Count(contentStr, oldText) if count > 1 { - return nil, fmt.Errorf("old_text appears %d times. Please provide more context to make it unique", count) + return nil, fmt.Errorf( + "old_text appears %d times. Please provide more context to make it unique", + count, + ) } newContent := strings.Replace(contentStr, oldText, newText, 1) diff --git a/pkg/tools/edit_test.go b/pkg/tools/edit_test.go index 83a7e778c..25f89fb88 100644 --- a/pkg/tools/edit_test.go +++ b/pkg/tools/edit_test.go @@ -76,7 +76,8 @@ func TestEditTool_EditFile_NotFound(t *testing.T) { } // Should mention file not found - if !strings.Contains(result.ForLLM, "not found") && !strings.Contains(result.ForUser, "not found") { + if !strings.Contains(result.ForLLM, "not found") && + !strings.Contains(result.ForUser, "not found") { t.Errorf("Expected 'file not found' message, got ForLLM: %s", result.ForLLM) } } @@ -103,7 +104,8 @@ func TestEditTool_EditFile_OldTextNotFound(t *testing.T) { } // Should mention old_text not found - if !strings.Contains(result.ForLLM, "not found") && !strings.Contains(result.ForUser, "not found") { + if !strings.Contains(result.ForLLM, "not found") && + !strings.Contains(result.ForUser, "not found") { t.Errorf("Expected 'not found' message, got ForLLM: %s", result.ForLLM) } } diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 39d45013d..35da9ecde 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -20,7 +20,11 @@ import ( const MaxReadFileSize = 64 * 1024 // 64KB limit to avoid context overflow -func validatePathWithAllowPaths(path, workspace string, restrict bool, patterns []*regexp.Regexp) (string, error) { +func validatePathWithAllowPaths( + path, workspace string, + restrict bool, + patterns []*regexp.Regexp, +) (string, error) { if workspace == "" { return path, fmt.Errorf("workspace is not defined") } @@ -483,7 +487,11 @@ type WriteFileTool struct { fs fileSystem } -func NewWriteFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *WriteFileTool { +func NewWriteFileTool( + workspace string, + restrict bool, + allowPaths ...[]*regexp.Regexp, +) *WriteFileTool { var patterns []*regexp.Regexp if len(allowPaths) > 0 { patterns = allowPaths[0] @@ -536,7 +544,9 @@ func (t *WriteFileTool) Execute(ctx context.Context, args map[string]any) *ToolR if !overwrite { if _, err := t.fs.Open(path); err == nil { - return ErrorResult(fmt.Sprintf("file: %s already exists. Set overwrite=true to replace.", path)) + return ErrorResult( + fmt.Sprintf("file: %s already exists. Set overwrite=true to replace.", path), + ) } } diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index 0b4dd310b..90b20b47e 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -59,8 +59,13 @@ func TestFilesystemTool_ReadFile_NotFound(t *testing.T) { } // Should contain error message - if !strings.Contains(result.ForLLM, "failed to open file") && !strings.Contains(result.ForUser, "failed to read") { - t.Errorf("Expected error message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser) + if !strings.Contains(result.ForLLM, "failed to open file") && + !strings.Contains(result.ForUser, "failed to read") { + t.Errorf( + "Expected error message, got ForLLM: %s, ForUser: %s", + result.ForLLM, + result.ForUser, + ) } } @@ -78,7 +83,8 @@ func TestFilesystemTool_ReadFile_MissingPath(t *testing.T) { } // Should mention required parameter - if !strings.Contains(result.ForLLM, "path is required") && !strings.Contains(result.ForUser, "path is required") { + if !strings.Contains(result.ForLLM, "path is required") && + !strings.Contains(result.ForUser, "path is required") { t.Errorf("Expected 'path is required' message, got ForLLM: %s", result.ForLLM) } } @@ -297,7 +303,12 @@ func TestFilesystemTool_WriteFile_OverwriteSandboxed(t *testing.T) { "content": "replaced in sandbox", "overwrite": true, }) - assert.False(t, result.IsError, "expected success in sandbox mode with overwrite=true, got: %s", result.ForLLM) + assert.False( + t, + result.IsError, + "expected success in sandbox mode with overwrite=true, got: %s", + result.ForLLM, + ) data, err := os.ReadFile(filepath.Join(workspace, testFile)) assert.NoError(t, err) @@ -325,7 +336,8 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) { } // Should list files and directories - if !strings.Contains(result.ForLLM, "file1.txt") || !strings.Contains(result.ForLLM, "file2.txt") { + if !strings.Contains(result.ForLLM, "file1.txt") || + !strings.Contains(result.ForLLM, "file2.txt") { t.Errorf("Expected files in listing, got: %s", result.ForLLM) } if !strings.Contains(result.ForLLM, "subdir") { @@ -349,8 +361,13 @@ func TestFilesystemTool_ListDir_NotFound(t *testing.T) { } // Should contain error message - if !strings.Contains(result.ForLLM, "failed to read") && !strings.Contains(result.ForUser, "failed to read") { - t.Errorf("Expected error message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser) + if !strings.Contains(result.ForLLM, "failed to read") && + !strings.Contains(result.ForUser, "failed to read") { + t.Errorf( + "Expected error message, got ForLLM: %s, ForUser: %s", + result.ForLLM, + result.ForUser, + ) } } @@ -397,7 +414,8 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) { // os.Root might return different errors depending on platform/implementation // but it definitely should error. // Our wrapper returns "access denied or file not found" - if !strings.Contains(result.ForLLM, "access denied") && !strings.Contains(result.ForLLM, "file not found") && + if !strings.Contains(result.ForLLM, "access denied") && + !strings.Contains(result.ForLLM, "file not found") && !strings.Contains(result.ForLLM, "no such file") { t.Fatalf("expected symlink escape error, got: %s", result.ForLLM) } @@ -416,10 +434,20 @@ func TestFilesystemTool_EmptyWorkspace_AccessDenied(t *testing.T) { }) // We EXPECT IsError=true (access blocked due to empty workspace) - assert.True(t, result.IsError, "Security Regression: Empty workspace allowed access! content: %s", result.ForLLM) + assert.True( + t, + result.IsError, + "Security Regression: Empty workspace allowed access! content: %s", + result.ForLLM, + ) // Verify it failed for the right reason - assert.Contains(t, result.ForLLM, "workspace is not defined", "Expected 'workspace is not defined' error") + assert.Contains( + t, + result.ForLLM, + "workspace is not defined", + "Expected 'workspace is not defined' error", + ) } // TestRootMkdirAll verifies that root.MkdirAll (used by atomicWriteFileInRoot) handles all cases: @@ -653,7 +681,10 @@ func TestWhitelistFs_BlocksSymlinkEscapeInAllowedDir(t *testing.T) { patterns := []*regexp.Regexp{regexp.MustCompile(`^` + regexp.QuoteMeta(allowedDir))} tool := NewReadFileTool(workspace, true, MaxReadFileSize, patterns) - result := tool.Execute(context.Background(), map[string]any{"path": filepath.Join(linkPath, "secret.txt")}) + result := tool.Execute( + context.Background(), + map[string]any{"path": filepath.Join(linkPath, "secret.txt")}, + ) if !result.IsError { t.Fatalf("expected symlink escape from allowed dir to be blocked, got: %s", result.ForLLM) } diff --git a/pkg/tools/i2c.go b/pkg/tools/i2c.go index 779b1d5a7..e3d5c152c 100644 --- a/pkg/tools/i2c.go +++ b/pkg/tools/i2c.go @@ -65,7 +65,9 @@ func (t *I2CTool) Parameters() map[string]any { func (t *I2CTool) Execute(ctx context.Context, args map[string]any) *ToolResult { if runtime.GOOS != "linux" { - return ErrorResult("I2C is only supported on Linux. This tool requires /dev/i2c-* device files.") + return ErrorResult( + "I2C is only supported on Linux. This tool requires /dev/i2c-* device files.", + ) } action, ok := args["action"].(string) @@ -83,7 +85,9 @@ func (t *I2CTool) Execute(ctx context.Context, args map[string]any) *ToolResult case "write": return t.writeDevice(args) default: - return ErrorResult(fmt.Sprintf("unknown action: %s (valid: detect, scan, read, write)", action)) + return ErrorResult( + fmt.Sprintf("unknown action: %s (valid: detect, scan, read, write)", action), + ) } } diff --git a/pkg/tools/i2c_linux.go b/pkg/tools/i2c_linux.go index 4eaaf8f09..ccd57b24b 100644 --- a/pkg/tools/i2c_linux.go +++ b/pkg/tools/i2c_linux.go @@ -55,7 +55,12 @@ func smbusProbe(fd int, addr int, hasQuick bool) bool { size: i2cSmbusQuick, data: nil, } - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), i2cSmbus, uintptr(unsafe.Pointer(&args))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + i2cSmbus, + uintptr(unsafe.Pointer(&args)), + ) return errno == 0 } @@ -67,7 +72,12 @@ func smbusProbe(fd int, addr int, hasQuick bool) bool { size: i2cSmbusByte, data: &data, } - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), i2cSmbus, uintptr(unsafe.Pointer(&args))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + i2cSmbus, + uintptr(unsafe.Pointer(&args)), + ) return errno == 0 } @@ -83,16 +93,29 @@ func (t *I2CTool) scan(args map[string]any) *ToolResult { devPath := fmt.Sprintf("/dev/i2c-%s", bus) fd, err := syscall.Open(devPath, syscall.O_RDWR, 0) if err != nil { - return ErrorResult(fmt.Sprintf("failed to open %s: %v (check permissions and i2c-dev module)", devPath, err)) + return ErrorResult( + fmt.Sprintf( + "failed to open %s: %v (check permissions and i2c-dev module)", + devPath, + err, + ), + ) } defer syscall.Close(fd) // Query adapter capabilities to determine available probe methods. // I2C_FUNCS writes an unsigned long, which is word-sized on Linux. var funcs uintptr - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), i2cFuncs, uintptr(unsafe.Pointer(&funcs))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + i2cFuncs, + uintptr(unsafe.Pointer(&funcs)), + ) if errno != 0 { - return ErrorResult(fmt.Sprintf("failed to query I2C adapter capabilities on %s: %v", devPath, errno)) + return ErrorResult( + fmt.Sprintf("failed to query I2C adapter capabilities on %s: %v", devPath, errno), + ) } hasQuick := funcs&i2cFuncSmbusQuick != 0 @@ -100,7 +123,10 @@ func (t *I2CTool) scan(args map[string]any) *ToolResult { if !hasQuick && !hasReadByte { return ErrorResult( - fmt.Sprintf("I2C adapter %s supports neither SMBus Quick nor Read Byte — cannot probe safely", devPath), + fmt.Sprintf( + "I2C adapter %s supports neither SMBus Quick nor Read Byte — cannot probe safely", + devPath, + ), ) } @@ -132,7 +158,9 @@ func (t *I2CTool) scan(args map[string]any) *ToolResult { } if len(found) == 0 { - return SilentResult(fmt.Sprintf("No devices found on %s. Check wiring and pull-up resistors.", devPath)) + return SilentResult( + fmt.Sprintf("No devices found on %s. Check wiring and pull-up resistors.", devPath), + ) } result, _ := json.MarshalIndent(map[string]any{ diff --git a/pkg/tools/mcp_tool.go b/pkg/tools/mcp_tool.go index 5bffb4e89..d4674d376 100644 --- a/pkg/tools/mcp_tool.go +++ b/pkg/tools/mcp_tool.go @@ -314,7 +314,10 @@ func (t *MCPTool) normalizeResultContent(ctx context.Context, content []mcp.Cont return result } -func (t *MCPTool) storeEmbeddedResource(ctx context.Context, content *mcp.EmbeddedResource) (string, string) { +func (t *MCPTool) storeEmbeddedResource( + ctx context.Context, + content *mcp.EmbeddedResource, +) (string, string) { if content == nil || content.Resource == nil { return "", "[MCP returned an embedded resource without data.]" } @@ -374,23 +377,39 @@ func (t *MCPTool) storeBinaryContent( dir := media.TempDir() if err := os.MkdirAll(dir, 0o700); err != nil { - return "", fmt.Sprintf("[MCP returned %s content (%s) but it could not be stored.]", kind, mimeType) + return "", fmt.Sprintf( + "[MCP returned %s content (%s) but it could not be stored.]", + kind, + mimeType, + ) } ext := extensionForMIMEType(mimeType) tmpFile, err := os.CreateTemp(dir, "mcp-*"+ext) if err != nil { - return "", fmt.Sprintf("[MCP returned %s content (%s) but it could not be stored.]", kind, mimeType) + return "", fmt.Sprintf( + "[MCP returned %s content (%s) but it could not be stored.]", + kind, + mimeType, + ) } tmpPath := tmpFile.Name() if _, err = tmpFile.Write(data); err != nil { _ = tmpFile.Close() _ = os.Remove(tmpPath) - return "", fmt.Sprintf("[MCP returned %s content (%s) but it could not be stored.]", kind, mimeType) + return "", fmt.Sprintf( + "[MCP returned %s content (%s) but it could not be stored.]", + kind, + mimeType, + ) } if err = tmpFile.Close(); err != nil { _ = os.Remove(tmpPath) - return "", fmt.Sprintf("[MCP returned %s content (%s) but it could not be stored.]", kind, mimeType) + return "", fmt.Sprintf( + "[MCP returned %s content (%s) but it could not be stored.]", + kind, + mimeType, + ) } scope := fmt.Sprintf( @@ -470,7 +489,10 @@ func summarizeEmbeddedResource(content *mcp.EmbeddedResource) string { normalizedMIMEType(resource.MIMEType), ) } - return fmt.Sprintf("[MCP returned embedded resource (%s).]", normalizedMIMEType(resource.MIMEType)) + return fmt.Sprintf( + "[MCP returned embedded resource (%s).]", + normalizedMIMEType(resource.MIMEType), + ) } func annotationsAllowUser(annotations *mcp.Annotations) bool { diff --git a/pkg/tools/mcp_tool_test.go b/pkg/tools/mcp_tool_test.go index 8bbac3bc7..3b514cd82 100644 --- a/pkg/tools/mcp_tool_test.go +++ b/pkg/tools/mcp_tool_test.go @@ -571,7 +571,10 @@ func TestMCPTool_Execute_EmbeddedResourceBlobStoredAsMedia(t *testing.T) { result := mcpTool.Execute(WithToolContext(context.Background(), "telegram", "chat-42"), nil) if len(result.Media) != 1 { - t.Fatalf("expected embedded resource blob to be stored as media, got %d refs", len(result.Media)) + t.Fatalf( + "expected embedded resource blob to be stored as media, got %d refs", + len(result.Media), + ) } path, _, err := store.ResolveWithMeta(result.Media[0]) if err != nil { diff --git a/pkg/tools/message_test.go b/pkg/tools/message_test.go index 05630972e..1b8bfab4a 100644 --- a/pkg/tools/message_test.go +++ b/pkg/tools/message_test.go @@ -43,7 +43,10 @@ func TestMessageTool_Execute_Success(t *testing.T) { // - ForLLM contains send status description if result.ForLLM != "Message sent to test-channel:test-chat-id" { - t.Errorf("Expected ForLLM 'Message sent to test-channel:test-chat-id', got '%s'", result.ForLLM) + t.Errorf( + "Expected ForLLM 'Message sent to test-channel:test-chat-id', got '%s'", + result.ForLLM, + ) } // - ForUser is empty (user already received message directly) @@ -88,7 +91,10 @@ func TestMessageTool_Execute_WithCustomChannel(t *testing.T) { t.Error("Expected Silent=true") } if result.ForLLM != "Message sent to custom-channel:custom-chat-id" { - t.Errorf("Expected ForLLM 'Message sent to custom-channel:custom-chat-id', got '%s'", result.ForLLM) + t.Errorf( + "Expected ForLLM 'Message sent to custom-channel:custom-chat-id', got '%s'", + result.ForLLM, + ) } } diff --git a/pkg/tools/normalization.go b/pkg/tools/normalization.go index 3a76c5d92..9cd9c65c6 100644 --- a/pkg/tools/normalization.go +++ b/pkg/tools/normalization.go @@ -215,28 +215,43 @@ func storeInlineDataURL( payload = strings.NewReplacer("\n", "", "\r", "", "\t", "", " ", "").Replace(payload) decoded, err := base64.StdEncoding.DecodeString(payload) if err != nil { - return "", fmt.Sprintf("[Tool returned inline media content (%s) that could not be decoded.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) that could not be decoded.]", + mimeType, + ) } dir := media.TempDir() if err = os.MkdirAll(dir, 0o700); err != nil { - return "", fmt.Sprintf("[Tool returned inline media content (%s) but it could not be stored.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) but it could not be stored.]", + mimeType, + ) } ext := extensionForMIMEType(mimeType) tmpFile, err := os.CreateTemp(dir, "tool-inline-*"+ext) if err != nil { - return "", fmt.Sprintf("[Tool returned inline media content (%s) but it could not be stored.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) but it could not be stored.]", + mimeType, + ) } tmpPath := tmpFile.Name() if _, err = tmpFile.Write(decoded); err != nil { tmpFile.Close() _ = os.Remove(tmpPath) - return "", fmt.Sprintf("[Tool returned inline media content (%s) but it could not be stored.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) but it could not be stored.]", + mimeType, + ) } if err = tmpFile.Close(); err != nil { _ = os.Remove(tmpPath) - return "", fmt.Sprintf("[Tool returned inline media content (%s) but it could not be stored.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) but it could not be stored.]", + mimeType, + ) } filename := sanitizeIdentifierComponent(toolName) + ext @@ -255,7 +270,10 @@ func storeInlineDataURL( }, scope) if err != nil { _ = os.Remove(tmpPath) - return "", fmt.Sprintf("[Tool returned inline media content (%s) but it could not be registered.]", mimeType) + return "", fmt.Sprintf( + "[Tool returned inline media content (%s) but it could not be registered.]", + mimeType, + ) } return ref, fmt.Sprintf(inlineMediaStoredMessage, mimeType) diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index 56af8d695..e16be0ccb 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "sort" + "strings" "sync" "sync/atomic" "time" @@ -24,6 +25,7 @@ type ToolRegistry struct { mu sync.RWMutex version atomic.Uint64 // incremented on Register/RegisterHidden for cache invalidation mediaStore media.MediaStore + allowlist map[string]struct{} } type mediaStoreAware interface { @@ -36,10 +38,40 @@ func NewToolRegistry() *ToolRegistry { } } +// SetAllowlist restricts registrations to the provided runtime tool names. +// A nil slice means "allow all". An empty-but-non-nil slice means "allow none". +func (r *ToolRegistry) SetAllowlist(names []string) { + r.mu.Lock() + defer r.mu.Unlock() + + if names == nil { + r.allowlist = nil + return + } + + allowlist := make(map[string]struct{}, len(names)) + for _, name := range names { + trimmed := strings.TrimSpace(name) + if trimmed == "" { + continue + } + allowlist[trimmed] = struct{}{} + } + r.allowlist = allowlist +} + func (r *ToolRegistry) Register(tool Tool) { r.mu.Lock() defer r.mu.Unlock() name := tool.Name() + if !r.toolAllowedLocked(name) { + logger.DebugCF( + "tools", + "Skipped core tool registration by agent allowlist", + map[string]any{"name": name}, + ) + return + } if _, exists := r.tools[name]; exists { logger.WarnCF("tools", "Tool registration overwrites existing tool", map[string]any{"name": name}) @@ -61,6 +93,14 @@ func (r *ToolRegistry) RegisterHidden(tool Tool) { r.mu.Lock() defer r.mu.Unlock() name := tool.Name() + if !r.toolAllowedLocked(name) { + logger.DebugCF( + "tools", + "Skipped hidden tool registration by agent allowlist", + map[string]any{"name": name}, + ) + return + } if _, exists := r.tools[name]; exists { logger.WarnCF("tools", "Hidden tool registration overwrites existing tool", map[string]any{"name": name}) @@ -128,6 +168,14 @@ func (r *ToolRegistry) Version() uint64 { return r.version.Load() } +func (r *ToolRegistry) toolAllowedLocked(name string) bool { + if r.allowlist == nil { + return true + } + _, ok := r.allowlist[name] + return ok +} + // HiddenToolSnapshot holds a consistent snapshot of hidden tools and the // registry version at which it was taken. Used by BM25SearchTool cache. type HiddenToolSnapshot struct { @@ -203,7 +251,9 @@ func (r *ToolRegistry) ExecuteWithContext( map[string]any{ "tool": name, }) - return ErrorResult(fmt.Sprintf("tool %q not found", name)).WithError(fmt.Errorf("tool not found")) + return ErrorResult( + fmt.Sprintf("tool %q not found", name), + ).WithError(fmt.Errorf("tool not found")) } // Validate arguments against the tool's declared schema. @@ -385,6 +435,12 @@ func (r *ToolRegistry) Clone() *ToolRegistry { tools: make(map[string]*ToolEntry, len(r.tools)), mediaStore: r.mediaStore, } + if r.allowlist != nil { + clone.allowlist = make(map[string]struct{}, len(r.allowlist)) + for name := range r.allowlist { + clone.allowlist[name] = struct{}{} + } + } for name, entry := range r.tools { clone.tools[name] = &ToolEntry{ Tool: entry.Tool, @@ -417,7 +473,10 @@ func (r *ToolRegistry) GetSummaries() []string { continue } - summaries = append(summaries, fmt.Sprintf("- `%s` - %s", entry.Tool.Name(), entry.Tool.Description())) + summaries = append( + summaries, + fmt.Sprintf("- `%s` - %s", entry.Tool.Name(), entry.Tool.Description()), + ) } return summaries } diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go index db52749f6..17b3cd127 100644 --- a/pkg/tools/registry_test.go +++ b/pkg/tools/registry_test.go @@ -44,7 +44,11 @@ type mockAsyncRegistryTool struct { lastCB AsyncCallback } -func (m *mockAsyncRegistryTool) ExecuteAsync(_ context.Context, args map[string]any, cb AsyncCallback) *ToolResult { +func (m *mockAsyncRegistryTool) ExecuteAsync( + _ context.Context, + args map[string]any, + cb AsyncCallback, +) *ToolResult { m.lastCB = cb return m.result } @@ -95,6 +99,28 @@ func TestToolRegistry_RegisterAndGet(t *testing.T) { } } +func TestToolRegistry_AllowlistFiltersRegistrations(t *testing.T) { + r := NewToolRegistry() + r.SetAllowlist([]string{"allowed_tool"}) + + r.Register(newMockTool("allowed_tool", "allowed")) + r.Register(newMockTool("blocked_tool", "blocked")) + r.RegisterHidden(newMockTool("hidden_blocked", "hidden blocked")) + + if _, ok := r.Get("allowed_tool"); !ok { + t.Fatal("expected allowed_tool to be registered") + } + if _, ok := r.Get("blocked_tool"); ok { + t.Fatal("blocked_tool should not be registered") + } + if _, ok := r.Get("hidden_blocked"); ok { + t.Fatal("hidden_blocked should not be registered") + } + if got := r.List(); len(got) != 1 || got[0] != "allowed_tool" { + t.Fatalf("registry list = %v, want [allowed_tool]", got) + } +} + func TestToolRegistry_Get_NotFound(t *testing.T) { r := NewToolRegistry() _, ok := r.Get("nonexistent") @@ -269,7 +295,11 @@ func TestToolRegistry_ToProviderDefs(t *testing.T) { t.Errorf("Name: want %q, got %q", want.Function.Name, got.Function.Name) } if got.Function.Description != want.Function.Description { - t.Errorf("Description: want %q, got %q", want.Function.Description, got.Function.Description) + t.Errorf( + "Description: want %q, got %q", + want.Function.Description, + got.Function.Description, + ) } } @@ -372,7 +402,10 @@ func TestToolRegistry_Clone(t *testing.T) { t.Errorf("expected parent to have 4 tools, got %d", r.Count()) } if clone.Count() != 3 { - t.Errorf("expected clone to still have 3 tools after parent mutation, got %d", clone.Count()) + t.Errorf( + "expected clone to still have 3 tools after parent mutation, got %d", + clone.Count(), + ) } if _, ok := clone.Get("spawn"); ok { t.Error("expected clone NOT to have 'spawn' tool registered on parent after cloning") @@ -668,7 +701,14 @@ func TestToolRegistry_ExecuteWithContext_SanitizesLargeBase64Payload(t *testing. result: SilentResult(payload), }) - result := r.ExecuteWithContext(context.Background(), "base64_tool", nil, "telegram", "chat-1", nil) + result := r.ExecuteWithContext( + context.Background(), + "base64_tool", + nil, + "telegram", + "chat-1", + nil, + ) if result.ForLLM != largeBase64OmittedMessage { t.Fatalf("expected sanitized payload, got %q", result.ForLLM) @@ -688,7 +728,14 @@ func TestToolRegistry_ExecuteWithContext_ExtractsInlineMediaDataURL(t *testing.T result: SilentResult(payload), }) - result := r.ExecuteWithContext(context.Background(), "inline_media_tool", nil, "telegram", "chat-42", nil) + result := r.ExecuteWithContext( + context.Background(), + "inline_media_tool", + nil, + "telegram", + "chat-42", + nil, + ) if len(result.Media) != 1 { t.Fatalf("expected 1 media ref, got %d", len(result.Media)) @@ -723,7 +770,14 @@ func TestToolRegistry_ExecuteWithContext_SanitizesInlineMediaWithoutStore(t *tes result: SilentResult(payload), }) - result := r.ExecuteWithContext(context.Background(), "inline_media_no_store", nil, "telegram", "chat-42", nil) + result := r.ExecuteWithContext( + context.Background(), + "inline_media_no_store", + nil, + "telegram", + "chat-42", + nil, + ) if strings.Contains(result.ForLLM, "data:image/png;base64") { t.Fatalf("expected inline data URL to be removed from ForLLM, got %q", result.ForLLM) diff --git a/pkg/tools/result.go b/pkg/tools/result.go index c81213125..1976eb88b 100644 --- a/pkg/tools/result.go +++ b/pkg/tools/result.go @@ -80,7 +80,10 @@ func (tr *ToolResult) ContentForLLM() string { } } if len(tr.ArtifactTags) > 0 { - artifactNote := "Local artifact paths: " + strings.Join(tr.ArtifactTags, " ") + "\n" + artifactPathsLLMNote + artifactNote := "Local artifact paths: " + strings.Join( + tr.ArtifactTags, + " ", + ) + "\n" + artifactPathsLLMNote if content == "" { content = artifactNote } else if !strings.Contains(content, artifactNote) { diff --git a/pkg/tools/result_test.go b/pkg/tools/result_test.go index 5f08cb4fa..87b2f1b4b 100644 --- a/pkg/tools/result_test.go +++ b/pkg/tools/result_test.go @@ -142,7 +142,11 @@ func TestToolResultJSONSerialization(t *testing.T) { t.Errorf("ForLLM mismatch: got '%s', want '%s'", decoded.ForLLM, tt.result.ForLLM) } if decoded.ForUser != tt.result.ForUser { - t.Errorf("ForUser mismatch: got '%s', want '%s'", decoded.ForUser, tt.result.ForUser) + t.Errorf( + "ForUser mismatch: got '%s', want '%s'", + decoded.ForUser, + tt.result.ForUser, + ) } if decoded.Silent != tt.result.Silent { t.Errorf("Silent mismatch: got %v, want %v", decoded.Silent, tt.result.Silent) diff --git a/pkg/tools/search_tool.go b/pkg/tools/search_tool.go index f41c80d90..21326504d 100644 --- a/pkg/tools/search_tool.go +++ b/pkg/tools/search_tool.go @@ -56,19 +56,38 @@ func (t *RegexSearchTool) Execute(ctx context.Context, args map[string]any) *Too } if len(pattern) > MaxRegexPatternLength { - logger.WarnCF("discovery", "Regex pattern rejected (too long)", map[string]any{"len": len(pattern)}) - return ErrorResult(fmt.Sprintf("Pattern too long: max %d characters allowed", MaxRegexPatternLength)) + logger.WarnCF( + "discovery", + "Regex pattern rejected (too long)", + map[string]any{"len": len(pattern)}, + ) + return ErrorResult( + fmt.Sprintf("Pattern too long: max %d characters allowed", MaxRegexPatternLength), + ) } logger.DebugCF("discovery", "Regex search", map[string]any{"pattern": pattern}) res, err := t.registry.SearchRegex(pattern, t.maxSearchResults) if err != nil { - logger.WarnCF("discovery", "Invalid regex pattern", map[string]any{"pattern": pattern, "error": err.Error()}) - return ErrorResult(fmt.Sprintf("Invalid regex pattern syntax: %v. Please fix your regex and try again.", err)) + logger.WarnCF( + "discovery", + "Invalid regex pattern", + map[string]any{"pattern": pattern, "error": err.Error()}, + ) + return ErrorResult( + fmt.Sprintf( + "Invalid regex pattern syntax: %v. Please fix your regex and try again.", + err, + ), + ) } - logger.InfoCF("discovery", "Regex search completed", map[string]any{"pattern": pattern, "results": len(res)}) + logger.InfoCF( + "discovery", + "Regex search completed", + map[string]any{"pattern": pattern, "results": len(res)}, + ) return formatDiscoveryResponse(t.registry, res, t.ttl) } @@ -138,7 +157,11 @@ func (t *BM25SearchTool) Execute(ctx context.Context, args map[string]any) *Tool } } - logger.InfoCF("discovery", "BM25 search completed", map[string]any{"query": query, "results": len(results)}) + logger.InfoCF( + "discovery", + "BM25 search completed", + map[string]any{"query": query, "results": len(results)}, + ) return formatDiscoveryResponse(t.registry, results, t.ttl) } @@ -150,7 +173,10 @@ type ToolSearchResult struct { Description string `json:"description"` } -func (r *ToolRegistry) SearchRegex(pattern string, maxSearchResults int) ([]ToolSearchResult, error) { +func (r *ToolRegistry) SearchRegex( + pattern string, + maxSearchResults int, +) ([]ToolSearchResult, error) { if maxSearchResults <= 0 { return nil, nil } @@ -188,7 +214,11 @@ func (r *ToolRegistry) SearchRegex(pattern string, maxSearchResults int) ([]Tool return results, nil } -func formatDiscoveryResponse(registry *ToolRegistry, results []ToolSearchResult, ttl int) *ToolResult { +func formatDiscoveryResponse( + registry *ToolRegistry, + results []ToolSearchResult, + ttl int, +) *ToolResult { if len(results) == 0 { return SilentResult("No tools found matching the query.") } @@ -274,7 +304,11 @@ func (t *BM25SearchTool) getOrBuildEngine() *bm25CachedEngine { cached := &bm25CachedEngine{engine: buildBM25Engine(docs)} t.cachedEngine = cached t.cacheVersion = snap.Version - logger.DebugCF("discovery", "BM25 engine rebuilt", map[string]any{"docs": len(docs), "version": snap.Version}) + logger.DebugCF( + "discovery", + "BM25 engine rebuilt", + map[string]any{"docs": len(docs), "version": snap.Version}, + ) return cached } diff --git a/pkg/tools/search_tools_test.go b/pkg/tools/search_tools_test.go index 3aae941cb..72cb11444 100644 --- a/pkg/tools/search_tools_test.go +++ b/pkg/tools/search_tools_test.go @@ -93,7 +93,10 @@ func TestRegexSearchTool_Execute(t *testing.T) { reg.mu.RLock() defer reg.mu.RUnlock() if reg.tools["mcp_read_file"].TTL != 5 { - t.Errorf("Expected TTL of 'mcp_read_file' to be promoted to 5, got %d", reg.tools["mcp_read_file"].TTL) + t.Errorf( + "Expected TTL of 'mcp_read_file' to be promoted to 5, got %d", + reg.tools["mcp_read_file"].TTL, + ) } if reg.tools["mcp_fetch_net"].TTL != 0 { t.Errorf("Expected 'mcp_fetch_net' to NOT be promoted (TTL=0)") diff --git a/pkg/tools/send_file.go b/pkg/tools/send_file.go index 44198381e..a344f4b5c 100644 --- a/pkg/tools/send_file.go +++ b/pkg/tools/send_file.go @@ -142,7 +142,10 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult(fmt.Sprintf("failed to register media: %v", err)) } - return MediaResult(fmt.Sprintf("File %q sent to user", filename), []string{ref}).WithResponseHandled() + return MediaResult( + fmt.Sprintf("File %q sent to user", filename), + []string{ref}, + ).WithResponseHandled() } // detectMediaType determines the MIME type of a file. diff --git a/pkg/tools/send_file_test.go b/pkg/tools/send_file_test.go index f36baf7d0..26b3c17ab 100644 --- a/pkg/tools/send_file_test.go +++ b/pkg/tools/send_file_test.go @@ -79,7 +79,11 @@ func TestSendFileTool_FileTooLarge(t *testing.T) { func TestSendFileTool_DefaultMaxSize(t *testing.T) { tool := NewSendFileTool("/tmp", false, 0, nil) if tool.maxFileSize != config.DefaultMaxMediaSize { - t.Errorf("expected default max size %d, got %d", config.DefaultMaxMediaSize, tool.maxFileSize) + t.Errorf( + "expected default max size %d, got %d", + config.DefaultMaxMediaSize, + tool.maxFileSize, + ) } } @@ -162,7 +166,11 @@ func TestSendFileTool_AllowsWhitelistedMediaTempPath(t *testing.T) { t.Cleanup(func() { _ = os.Remove(testPath) }) pattern := regexp.MustCompile( - "^" + regexp.QuoteMeta(filepath.Clean(mediaDir)) + "(?:" + regexp.QuoteMeta(string(os.PathSeparator)) + "|$)", + "^" + regexp.QuoteMeta( + filepath.Clean(mediaDir), + ) + "(?:" + regexp.QuoteMeta( + string(os.PathSeparator), + ) + "|$)", ) store := media.NewFileMediaStore() diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index 6ee1cb993..0d1c4c5db 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -113,7 +113,11 @@ var ( } ) -func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regexp) (*ExecTool, error) { +func NewExecTool( + workingDir string, + restrict bool, + allowPaths ...[]*regexp.Regexp, +) (*ExecTool, error) { return NewExecToolWithConfig(workingDir, restrict, nil, allowPaths...) } @@ -193,8 +197,16 @@ func (t *ExecTool) Parameters() map[string]any { "type": "object", "properties": map[string]any{ "action": map[string]any{ - "type": "string", - "enum": []string{"run", "list", "poll", "read", "write", "kill", "send-keys"}, + "type": "string", + "enum": []string{ + "run", + "list", + "poll", + "read", + "write", + "kill", + "send-keys", + }, "description": "Action: run (execute command), list (show sessions), poll (check status), read (get output), write (send input), kill (terminate), send-keys (send keys to PTY)", }, "command": map[string]any{ @@ -300,7 +312,12 @@ func (t *ExecTool) executeRun(ctx context.Context, args map[string]any) *ToolRes cwd := t.workingDir if wd, ok := args["cwd"].(string); ok && wd != "" { if t.restrictToWorkspace && t.workingDir != "" { - resolvedWD, err := validatePathWithAllowPaths(wd, t.workingDir, true, t.allowedPathPatterns) + resolvedWD, err := validatePathWithAllowPaths( + wd, + t.workingDir, + true, + t.allowedPathPatterns, + ) if err != nil { return ErrorResult("Command blocked by safety guard (" + err.Error() + ")") } @@ -326,7 +343,9 @@ func (t *ExecTool) executeRun(ctx context.Context, args map[string]any) *ToolRes if t.restrictToWorkspace && t.workingDir != "" && cwd != t.workingDir { resolved, err := filepath.EvalSymlinks(cwd) if err != nil { - return ErrorResult(fmt.Sprintf("Command blocked by safety guard (path resolution failed: %v)", err)) + return ErrorResult( + fmt.Sprintf("Command blocked by safety guard (path resolution failed: %v)", err), + ) } if isAllowedPath(resolved, t.allowedPathPatterns) { cwd = resolved @@ -364,7 +383,14 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult var cmd *exec.Cmd if runtime.GOOS == "windows" { - cmd = exec.CommandContext(cmdCtx, "powershell", "-NoProfile", "-NonInteractive", "-Command", command) + cmd = exec.CommandContext( + cmdCtx, + "powershell", + "-NoProfile", + "-NonInteractive", + "-Command", + command, + ) } else { cmd = exec.CommandContext(cmdCtx, "sh", "-c", command) } @@ -442,7 +468,10 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult maxLen := 10000 if len(output) > maxLen { - output = output[:maxLen] + fmt.Sprintf("\n... (truncated, %d more chars)", len(output)-maxLen) + output = output[:maxLen] + fmt.Sprintf( + "\n... (truncated, %d more chars)", + len(output)-maxLen, + ) } if err != nil { @@ -460,7 +489,11 @@ func (t *ExecTool) runSync(ctx context.Context, command, cwd string) *ToolResult } } -func (t *ExecTool) runBackground(ctx context.Context, command, cwd string, ptyEnabled bool) *ToolResult { +func (t *ExecTool) runBackground( + ctx context.Context, + command, cwd string, + ptyEnabled bool, +) *ToolResult { sessionID := generateSessionID() session := &ProcessSession{ ID: sessionID, @@ -553,7 +586,8 @@ func (t *ExecTool) runBackground(ctx context.Context, command, cwd string, ptyEn n, err := session.ptyMaster.Read(buf) if n > 0 { raw := string(buf[:n]) - if mode := detectPtyKeyMode(raw); mode != PtyKeyModeNotFound && mode != session.GetPtyKeyMode() { + if mode := detectPtyKeyMode(raw); mode != PtyKeyModeNotFound && + mode != session.GetPtyKeyMode() { session.SetPtyKeyMode(mode) } @@ -734,12 +768,16 @@ func (t *ExecTool) executeWrite(args map[string]any) *ToolResult { } if session.IsDone() { - return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + return ErrorResult( + fmt.Sprintf("process already exited with code %d", session.GetExitCode()), + ) } if err := session.Write(data); err != nil { if errors.Is(err, ErrSessionDone) { - return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + return ErrorResult( + fmt.Sprintf("process already exited with code %d", session.GetExitCode()), + ) } return ErrorResult(fmt.Sprintf("failed to write to session: %v", err)) } @@ -770,7 +808,9 @@ func (t *ExecTool) executeKill(args map[string]any) *ToolResult { } if session.IsDone() { - return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + return ErrorResult( + fmt.Sprintf("process already exited with code %d", session.GetExitCode()), + ) } if err := session.Kill(); err != nil { @@ -992,12 +1032,16 @@ func (t *ExecTool) executeSendKeys(args map[string]any) *ToolResult { } if session.IsDone() { - return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + return ErrorResult( + fmt.Sprintf("process already exited with code %d", session.GetExitCode()), + ) } if err := session.Write(data); err != nil { if errors.Is(err, ErrSessionDone) { - return ErrorResult(fmt.Sprintf("process already exited with code %d", session.GetExitCode())) + return ErrorResult( + fmt.Sprintf("process already exited with code %d", session.GetExitCode()), + ) } return ErrorResult(fmt.Sprintf("failed to send keys: %v", err)) } diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index a8de2f4c9..228ec1067 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -100,8 +100,13 @@ func TestShellTool_Timeout(t *testing.T) { } // Should mention timeout - if !strings.Contains(result.ForLLM, "timed out") && !strings.Contains(result.ForUser, "timed out") { - t.Errorf("Expected timeout message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser) + if !strings.Contains(result.ForLLM, "timed out") && + !strings.Contains(result.ForUser, "timed out") { + t.Errorf( + "Expected timeout message, got ForLLM: %s, ForUser: %s", + result.ForLLM, + result.ForUser, + ) } } @@ -156,7 +161,11 @@ func TestShellTool_DangerousCommand(t *testing.T) { } if !strings.Contains(result.ForLLM, "blocked") && !strings.Contains(result.ForUser, "blocked") { - t.Errorf("Expected 'blocked' message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser) + t.Errorf( + "Expected 'blocked' message, got ForLLM: %s, ForUser: %s", + result.ForLLM, + result.ForUser, + ) } } @@ -177,7 +186,11 @@ func TestShellTool_DangerousCommand_KillBlocked(t *testing.T) { t.Errorf("Expected kill command to be blocked") } if !strings.Contains(result.ForLLM, "blocked") && !strings.Contains(result.ForUser, "blocked") { - t.Errorf("Expected blocked message, got ForLLM: %s, ForUser: %s", result.ForLLM, result.ForUser) + t.Errorf( + "Expected blocked message, got ForLLM: %s, ForUser: %s", + result.ForLLM, + result.ForUser, + ) } } @@ -269,7 +282,10 @@ func TestShellTool_WorkingDir_OutsideWorkspace(t *testing.T) { }) if !result.IsError { - t.Fatalf("expected working_dir outside workspace to be blocked, got output: %s", result.ForLLM) + t.Fatalf( + "expected working_dir outside workspace to be blocked, got output: %s", + result.ForLLM, + ) } if !strings.Contains(result.ForLLM, "blocked") { t.Errorf("expected 'blocked' in error, got: %s", result.ForLLM) @@ -444,7 +460,10 @@ func TestShellTool_DevNullAllowed(t *testing.T) { } for _, cmd := range commands { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if result.IsError && strings.Contains(result.ForLLM, "blocked") { t.Errorf("command should not be blocked: %s\n error: %s", cmd, result.ForLLM) } @@ -473,7 +492,10 @@ func TestShellTool_BlockDevices(t *testing.T) { } for _, cmd := range blocked { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if !result.IsError { t.Errorf("expected block device write to be blocked: %s", cmd) } @@ -497,9 +519,16 @@ func TestShellTool_SafePathsInWorkspaceRestriction(t *testing.T) { } for _, cmd := range commands { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { - t.Errorf("safe path should not be blocked by workspace check: %s\n error: %s", cmd, result.ForLLM) + t.Errorf( + "safe path should not be blocked by workspace check: %s\n error: %s", + cmd, + result.ForLLM, + ) } } } @@ -591,7 +620,10 @@ func TestShellTool_CustomAllowPatterns(t *testing.T) { "command": "git push origin main", }) if result.IsError && strings.Contains(result.ForLLM, "blocked") { - t.Errorf("custom allow pattern should exempt 'git push origin main', got: %s", result.ForLLM) + t.Errorf( + "custom allow pattern should exempt 'git push origin main', got: %s", + result.ForLLM, + ) } // "git push upstream main" should still be blocked (does not match allow pattern). @@ -629,7 +661,11 @@ func TestShellTool_URLsNotBlocked(t *testing.T) { result := tool.Execute(ctx, map[string]any{"action": "run", "command": cmd}) cancel() if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { - t.Errorf("command with URL should not be blocked by workspace check: %s\n error: %s", cmd, result.ForLLM) + t.Errorf( + "command with URL should not be blocked by workspace check: %s\n error: %s", + cmd, + result.ForLLM, + ) } } } @@ -652,7 +688,10 @@ func TestShellTool_FileURISandboxing(t *testing.T) { } for _, cmd := range blockedCommands { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("file:// URI outside workspace should be blocked: %s", cmd) } @@ -670,9 +709,16 @@ func TestShellTool_FileURISandboxing(t *testing.T) { } for _, cmd := range allowedCommands { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") { - t.Errorf("file:// URI inside workspace should be allowed: %s\n error: %s", cmd, result.ForLLM) + t.Errorf( + "file:// URI inside workspace should be allowed: %s\n error: %s", + cmd, + result.ForLLM, + ) } } } @@ -696,7 +742,10 @@ func TestShellTool_URLBypassPrevented(t *testing.T) { } for _, cmd := range blockedCommands { - result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd}) + result := tool.Execute( + context.Background(), + map[string]any{"action": "run", "command": cmd}, + ) if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") { t.Errorf("bypass attempt should be blocked: %q\n got: %s", cmd, result.ForLLM) } @@ -1221,7 +1270,9 @@ func TestShellTool_PTY_ProcessGroupKill(t *testing.T) { // The binary is created in /tmp/test_pgroup.c and compiled as part of test setup. testBinary := "/tmp/test_pgroup" if _, err := os.Stat(testBinary); os.IsNotExist(err) { - t.Skip("Test binary /tmp/test_pgroup not found - run: gcc -o /tmp/test_pgroup /tmp/test_pgroup.c") + t.Skip( + "Test binary /tmp/test_pgroup not found - run: gcc -o /tmp/test_pgroup /tmp/test_pgroup.c", + ) } tool, err := NewExecTool("", false) @@ -1555,8 +1606,16 @@ func TestDetectPtyKeyMode(t *testing.T) { {"rmkx only", "\x1b[?1l\x1b>", PtyKeyModeCSI}, {"both smkx first", "\x1b[?1h\x1b=...\x1b[?1l\x1b>", PtyKeyModeCSI}, {"both rmkx first", "\x1b[?1l\x1b>...\x1b[?1h\x1b=", PtyKeyModeSS3}, - {"multiple toggles smkx last", "\x1b[?1h\x1b=...\x1b[?1l\x1b>...\x1b[?1h\x1b=", PtyKeyModeSS3}, - {"multiple toggles rmkx last", "\x1b[?1l\x1b>...\x1b[?1h\x1b=...\x1b[?1l\x1b>", PtyKeyModeCSI}, + { + "multiple toggles smkx last", + "\x1b[?1h\x1b=...\x1b[?1l\x1b>...\x1b[?1h\x1b=", + PtyKeyModeSS3, + }, + { + "multiple toggles rmkx last", + "\x1b[?1l\x1b>...\x1b[?1h\x1b=...\x1b[?1l\x1b>", + PtyKeyModeCSI, + }, {"partial smkx", "\x1b[?1h", PtyKeyModeSS3}, {"partial rmkx", "\x1b[?1l", PtyKeyModeCSI}, } diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go index 71bfe730b..ffb4b0c52 100644 --- a/pkg/tools/skills_install.go +++ b/pkg/tools/skills_install.go @@ -96,7 +96,11 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To if !force { if _, err := os.Stat(targetDir); err == nil { return ErrorResult( - fmt.Sprintf("skill %q already installed at %s. Use force=true to reinstall.", slug, targetDir), + fmt.Sprintf( + "skill %q already installed at %s. Use force=true to reinstall.", + slug, + targetDir, + ), ) } } else { @@ -142,7 +146,9 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To "error": rmErr.Error(), }) } - return ErrorResult(fmt.Sprintf("skill %q is flagged as malicious and cannot be installed", slug)) + return ErrorResult( + fmt.Sprintf("skill %q is flagged as malicious and cannot be installed", slug), + ) } // Write origin metadata. @@ -162,7 +168,10 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To // Build result with moderation warning if suspicious. var output string if result.IsSuspicious { - output = fmt.Sprintf("⚠️ Warning: skill %q is flagged as suspicious (may contain risky patterns).\n\n", slug) + output = fmt.Sprintf( + "⚠️ Warning: skill %q is flagged as suspicious (may contain risky patterns).\n\n", + slug, + ) } output += fmt.Sprintf("Successfully installed skill %q v%s from %s registry.\nLocation: %s\n", slug, result.Version, registry.Name(), targetDir) diff --git a/pkg/tools/skills_search.go b/pkg/tools/skills_search.go index 2b6cffd38..8f7401dfa 100644 --- a/pkg/tools/skills_search.go +++ b/pkg/tools/skills_search.go @@ -17,7 +17,10 @@ type FindSkillsTool struct { // NewFindSkillsTool creates a new FindSkillsTool. // registryMgr is the shared registry manager (built from config in createToolRegistry). // cache is the search cache for deduplicating similar queries. -func NewFindSkillsTool(registryMgr *skills.RegistryManager, cache *skills.SearchCache) *FindSkillsTool { +func NewFindSkillsTool( + registryMgr *skills.RegistryManager, + cache *skills.SearchCache, +) *FindSkillsTool { return &FindSkillsTool{ registryMgr: registryMgr, cache: cache, diff --git a/pkg/tools/spawn_status.go b/pkg/tools/spawn_status.go index 416fd2226..22202d899 100644 --- a/pkg/tools/spawn_status.go +++ b/pkg/tools/spawn_status.go @@ -77,10 +77,12 @@ func (t *SpawnStatusTool) Execute(ctx context.Context, args map[string]any) *Too } // Restrict lookup to tasks that belong to this conversation. - if callerChannel != "" && taskCopy.OriginChannel != "" && taskCopy.OriginChannel != callerChannel { + if callerChannel != "" && taskCopy.OriginChannel != "" && + taskCopy.OriginChannel != callerChannel { return ErrorResult(fmt.Sprintf("No subagent found with task ID: %s", taskID)) } - if callerChatID != "" && taskCopy.OriginChatID != "" && taskCopy.OriginChatID != callerChatID { + if callerChatID != "" && taskCopy.OriginChatID != "" && + taskCopy.OriginChatID != callerChatID { return ErrorResult(fmt.Sprintf("No subagent found with task ID: %s", taskID)) } diff --git a/pkg/tools/spawn_status_test.go b/pkg/tools/spawn_status_test.go index 9c772d61a..22b885fb7 100644 --- a/pkg/tools/spawn_status_test.go +++ b/pkg/tools/spawn_status_test.go @@ -195,7 +195,12 @@ func TestSpawnStatusTool_TaskID_NonString(t *testing.T) { for _, badVal := range []any{42, 3.14, true, map[string]any{"x": 1}, []string{"a"}} { result := tool.Execute(context.Background(), map[string]any{"task_id": badVal}) if !result.IsError { - t.Errorf("Expected error for task_id=%T(%v), got success: %s", badVal, badVal, result.ForLLM) + t.Errorf( + "Expected error for task_id=%T(%v), got success: %s", + badVal, + badVal, + result.ForLLM, + ) } if !strings.Contains(result.ForLLM, "task_id must be a string") { t.Errorf("Expected type-error message, got: %s", result.ForLLM) @@ -319,7 +324,10 @@ func TestSpawnStatusTool_SortByCreatedTimestamp(t *testing.T) { t.Fatalf("Both task IDs should appear in output:\n%s", result.ForLLM) } if pos2 > pos10 { - t.Errorf("Expected subagent-2 (created first) to appear before subagent-10, but got:\n%s", result.ForLLM) + t.Errorf( + "Expected subagent-2 (created first) to appear before subagent-10, but got:\n%s", + result.ForLLM, + ) } } diff --git a/pkg/tools/spi.go b/pkg/tools/spi.go index 0ca17e84f..cdf23db86 100644 --- a/pkg/tools/spi.go +++ b/pkg/tools/spi.go @@ -69,7 +69,9 @@ func (t *SPITool) Parameters() map[string]any { func (t *SPITool) Execute(ctx context.Context, args map[string]any) *ToolResult { if runtime.GOOS != "linux" { - return ErrorResult("SPI is only supported on Linux. This tool requires /dev/spidev* device files.") + return ErrorResult( + "SPI is only supported on Linux. This tool requires /dev/spidev* device files.", + ) } action, ok := args["action"].(string) @@ -124,7 +126,9 @@ func (t *SPITool) list() *ToolResult { // parseSPIArgs extracts and validates common SPI parameters // //nolint:unused // Used by spi_linux.go -func parseSPIArgs(args map[string]any) (device string, speed uint32, mode uint8, bits uint8, errMsg string) { +func parseSPIArgs( + args map[string]any, +) (device string, speed uint32, mode uint8, bits uint8, errMsg string) { dev, ok := args["device"].(string) if !ok || dev == "" { return "", 0, 0, 0, "device is required (e.g. \"2.0\" for /dev/spidev2.0)" diff --git a/pkg/tools/spi_linux.go b/pkg/tools/spi_linux.go index 9def73662..d03c4ef92 100644 --- a/pkg/tools/spi_linux.go +++ b/pkg/tools/spi_linux.go @@ -38,25 +38,46 @@ type spiTransfer struct { func configureSPI(devPath string, mode uint8, bits uint8, speed uint32) (int, *ToolResult) { fd, err := syscall.Open(devPath, syscall.O_RDWR, 0) if err != nil { - return -1, ErrorResult(fmt.Sprintf("failed to open %s: %v (check permissions and spidev module)", devPath, err)) + return -1, ErrorResult( + fmt.Sprintf( + "failed to open %s: %v (check permissions and spidev module)", + devPath, + err, + ), + ) } // Set SPI mode - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), spiIocWrMode, uintptr(unsafe.Pointer(&mode))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + spiIocWrMode, + uintptr(unsafe.Pointer(&mode)), + ) if errno != 0 { syscall.Close(fd) return -1, ErrorResult(fmt.Sprintf("failed to set SPI mode %d: %v", mode, errno)) } // Set bits per word - _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), spiIocWrBitsPerWord, uintptr(unsafe.Pointer(&bits))) + _, _, errno = syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + spiIocWrBitsPerWord, + uintptr(unsafe.Pointer(&bits)), + ) if errno != 0 { syscall.Close(fd) return -1, ErrorResult(fmt.Sprintf("failed to set bits per word %d: %v", bits, errno)) } // Set max speed - _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), spiIocWrMaxSpeedHz, uintptr(unsafe.Pointer(&speed))) + _, _, errno = syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + spiIocWrMaxSpeedHz, + uintptr(unsafe.Pointer(&speed)), + ) if errno != 0 { syscall.Close(fd) return -1, ErrorResult(fmt.Sprintf("failed to set SPI speed %d Hz: %v", speed, errno)) @@ -117,7 +138,12 @@ func (t *SPITool) transfer(args map[string]any) *ToolResult { bitsPerWord: bits, } - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), spiIocMessage1, uintptr(unsafe.Pointer(&xfer))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + spiIocMessage1, + uintptr(unsafe.Pointer(&xfer)), + ) runtime.KeepAlive(txBuf) runtime.KeepAlive(rxBuf) if errno != 0 { @@ -174,7 +200,12 @@ func (t *SPITool) readDevice(args map[string]any) *ToolResult { bitsPerWord: bits, } - _, _, errno := syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), spiIocMessage1, uintptr(unsafe.Pointer(&xfer))) + _, _, errno := syscall.Syscall( + syscall.SYS_IOCTL, + uintptr(fd), + spiIocMessage1, + uintptr(unsafe.Pointer(&xfer)), + ) runtime.KeepAlive(txBuf) runtime.KeepAlive(rxBuf) if errno != 0 { diff --git a/pkg/tools/subagent_tool_test.go b/pkg/tools/subagent_tool_test.go index 89ac7d4b5..601d3f937 100644 --- a/pkg/tools/subagent_tool_test.go +++ b/pkg/tools/subagent_tool_test.go @@ -316,7 +316,11 @@ func TestSubagentTool_ForUserTruncation(t *testing.T) { // ForUser should be truncated to 500 chars + "..." maxUserLen := 500 if len(result.ForUser) > maxUserLen+3 { // +3 for "..." - t.Errorf("ForUser should be truncated to ~%d chars, got: %d", maxUserLen, len(result.ForUser)) + t.Errorf( + "ForUser should be truncated to ~%d chars, got: %d", + maxUserLen, + len(result.ForUser), + ) } // ForLLM should have full content diff --git a/pkg/tools/toolloop.go b/pkg/tools/toolloop.go index 387813e94..df72301a2 100644 --- a/pkg/tools/toolloop.go +++ b/pkg/tools/toolloop.go @@ -64,7 +64,13 @@ func RunToolLoop( llmOpts = map[string]any{} } // 3. Call LLM - response, err := config.Provider.Chat(ctx, messages, providerToolDefs, config.Model, llmOpts) + response, err := config.Provider.Chat( + ctx, + messages, + providerToolDefs, + config.Model, + llmOpts, + ) if err != nil { logger.ErrorCF("toolloop", "LLM call failed", map[string]any{ @@ -148,7 +154,14 @@ func RunToolLoop( var toolResult *ToolResult if config.Tools != nil { - toolResult = config.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, channel, chatID, nil) + toolResult = config.Tools.ExecuteWithContext( + ctx, + tc.Name, + tc.Arguments, + channel, + chatID, + nil, + ) } else { toolResult = ErrorResult("No tools available") } diff --git a/pkg/tools/validate_test.go b/pkg/tools/validate_test.go index e7f4f619a..accff9e3c 100644 --- a/pkg/tools/validate_test.go +++ b/pkg/tools/validate_test.go @@ -151,7 +151,10 @@ func TestValidateToolArgs(t *testing.T) { schema: map[string]any{ "type": "object", "properties": map[string]any{ - "color": map[string]any{"type": "string", "enum": []any{"red", "green", "blue"}}, + "color": map[string]any{ + "type": "string", + "enum": []any{"red", "green", "blue"}, + }, }, }, args: map[string]any{"color": "red"}, @@ -161,7 +164,10 @@ func TestValidateToolArgs(t *testing.T) { schema: map[string]any{ "type": "object", "properties": map[string]any{ - "color": map[string]any{"type": "string", "enum": []any{"red", "green", "blue"}}, + "color": map[string]any{ + "type": "string", + "enum": []any{"red", "green", "blue"}, + }, }, }, args: map[string]any{"color": "yellow"}, @@ -172,7 +178,10 @@ func TestValidateToolArgs(t *testing.T) { schema: map[string]any{ "type": "object", "properties": map[string]any{ - "color": map[string]any{"type": "string", "enum": []string{"red", "green", "blue"}}, + "color": map[string]any{ + "type": "string", + "enum": []string{"red", "green", "blue"}, + }, }, }, args: map[string]any{"color": "green"}, @@ -182,7 +191,10 @@ func TestValidateToolArgs(t *testing.T) { schema: map[string]any{ "type": "object", "properties": map[string]any{ - "color": map[string]any{"type": "string", "enum": []string{"red", "green", "blue"}}, + "color": map[string]any{ + "type": "string", + "enum": []string{"red", "green", "blue"}, + }, }, }, args: map[string]any{"color": "yellow"}, @@ -342,7 +354,11 @@ func TestValidateToolArgs_RegistryIntegration(t *testing.T) { } // Extra property — should fail with validation error - result = r.Execute(context.Background(), "read_file", map[string]any{"path": "/x", "__inject": true}) + result = r.Execute( + context.Background(), + "read_file", + map[string]any{"path": "/x", "__inject": true}, + ) if !result.IsError { t.Error("expected validation error for extra property") } diff --git a/pkg/tools/web_test.go b/pkg/tools/web_test.go index de6187cfa..2c0de25f7 100644 --- a/pkg/tools/web_test.go +++ b/pkg/tools/web_test.go @@ -54,7 +54,8 @@ func TestWebTool_WebFetch_Success(t *testing.T) { } // ForUser should contain summary - if !strings.Contains(result.ForUser, "bytes") && !strings.Contains(result.ForUser, "extractor") { + if !strings.Contains(result.ForUser, "bytes") && + !strings.Contains(result.ForUser, "extractor") { t.Errorf("Expected ForUser to contain summary, got: %s", result.ForUser) } } @@ -75,7 +76,11 @@ func TestWebTool_WebFetch_JSON(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -100,7 +105,11 @@ func TestWebTool_WebFetch_JSON(t *testing.T) { func TestWebTool_WebFetch_InvalidURL(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -125,7 +134,11 @@ func TestWebTool_WebFetch_InvalidURL(t *testing.T) { func TestWebTool_WebFetch_UnsupportedScheme(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -141,7 +154,8 @@ func TestWebTool_WebFetch_UnsupportedScheme(t *testing.T) { } // Should mention only http/https allowed - if !strings.Contains(result.ForLLM, "http/https") && !strings.Contains(result.ForUser, "http/https") { + if !strings.Contains(result.ForLLM, "http/https") && + !strings.Contains(result.ForUser, "http/https") { t.Errorf("Expected scheme error message, got ForLLM: %s", result.ForLLM) } } @@ -150,7 +164,11 @@ func TestWebTool_WebFetch_UnsupportedScheme(t *testing.T) { func TestWebTool_WebFetch_MissingURL(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -164,7 +182,8 @@ func TestWebTool_WebFetch_MissingURL(t *testing.T) { } // Should mention URL is required - if !strings.Contains(result.ForLLM, "url is required") && !strings.Contains(result.ForUser, "url is required") { + if !strings.Contains(result.ForLLM, "url is required") && + !strings.Contains(result.ForUser, "url is required") { t.Errorf("Expected 'url is required' message, got ForLLM: %s", result.ForLLM) } } @@ -184,7 +203,11 @@ func TestWebTool_WebFetch_Truncation(t *testing.T) { tool, err := NewWebFetchTool(1000, format, testFetchLimit) // Limit to 1000 chars if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -216,7 +239,10 @@ func TestWebTool_WebFetch_Truncation(t *testing.T) { // Text should end with the truncation notice if text, ok := resultMap["text"].(string); ok { if !strings.HasSuffix(text, "[Content truncated due to size limit]") { - t.Errorf("Expected text to end with truncation notice, got: %q", text[max(0, len(text)-60):]) + t.Errorf( + "Expected text to end with truncation notice, got: %q", + text[max(0, len(text)-60):], + ) } } } @@ -263,11 +289,13 @@ func TestWebTool_WebFetch_TruncationNotice(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", tt.contentType) - w.WriteHeader(http.StatusOK) - w.Write([]byte(tt.body)) - })) + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", tt.contentType) + w.WriteHeader(http.StatusOK) + w.Write([]byte(tt.body)) + }), + ) defer server.Close() tool, err := NewWebFetchTool(maxChars, tt.format, testFetchLimit) @@ -291,7 +319,11 @@ func TestWebTool_WebFetch_TruncationNotice(t *testing.T) { } if !strings.HasSuffix(text, truncationNotice) { - t.Errorf("expected text to end with %q, got suffix: %q", truncationNotice, text[max(0, len(text)-60):]) + t.Errorf( + "expected text to end with %q, got suffix: %q", + truncationNotice, + text[max(0, len(text)-60):], + ) } if truncated, ok := resultMap["truncated"].(bool); !ok || !truncated { @@ -360,7 +392,11 @@ func TestWebFetchTool_PayloadTooLarge(t *testing.T) { // Initialize the tool tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } // Prepare the arguments pointing to the URL of our local mock server @@ -380,7 +416,8 @@ func TestWebFetchTool_PayloadTooLarge(t *testing.T) { // Search for the exact error string we set earlier in the Execute method expectedErrorMsg := fmt.Sprintf("size exceeded %d bytes limit", testFetchLimit) - if !strings.Contains(result.ForLLM, expectedErrorMsg) && !strings.Contains(result.ForUser, expectedErrorMsg) { + if !strings.Contains(result.ForLLM, expectedErrorMsg) && + !strings.Contains(result.ForUser, expectedErrorMsg) { t.Errorf("test failed: expected error %q, but got: %+v", expectedErrorMsg, result) } } @@ -533,7 +570,11 @@ func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -718,7 +759,13 @@ func TestWebTool_WebFetch_PrivateHostAllowedByCIDRWhitelist(t *testing.T) { defer server.Close() host, _ := serverHostAndPort(t, server.URL) - tool, err := NewWebFetchToolWithConfig(50000, "", format, testFetchLimit, []string{singleHostCIDR(t, host)}) + tool, err := NewWebFetchToolWithConfig( + 50000, + "", + format, + testFetchLimit, + []string{singleHostCIDR(t, host)}, + ) if err != nil { t.Fatalf("Failed to create web fetch tool: %v", err) } @@ -753,7 +800,10 @@ func TestWebTool_WebFetch_PrivateHostAllowedForTests(t *testing.T) { }) if result.IsError { - t.Errorf("expected success when private host access is allowed in tests, got %q", result.ForLLM) + t.Errorf( + "expected success when private host access is allowed in tests, got %q", + result.ForLLM, + ) } } @@ -973,7 +1023,11 @@ func TestIsPrivateOrRestrictedIP_Table(t *testing.T) { func TestWebTool_WebFetch_MissingDomain(t *testing.T) { tool, err := NewWebFetchTool(50000, format, testFetchLimit) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } ctx := context.Background() @@ -995,9 +1049,19 @@ func TestWebTool_WebFetch_MissingDomain(t *testing.T) { } func TestNewWebFetchToolWithProxy(t *testing.T) { - tool, err := NewWebFetchToolWithProxy(1024, "http://127.0.0.1:7890", format, testFetchLimit, nil) + tool, err := NewWebFetchToolWithProxy( + 1024, + "http://127.0.0.1:7890", + format, + testFetchLimit, + nil, + ) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } else if tool.maxChars != 1024 { t.Fatalf("maxChars = %d, want %d", tool.maxChars, 1024) } @@ -1008,7 +1072,11 @@ func TestNewWebFetchToolWithProxy(t *testing.T) { tool, err = NewWebFetchToolWithProxy(0, "http://127.0.0.1:7890", format, testFetchLimit, nil) if err != nil { - logger.ErrorCF("agent", "Failed to create web fetch tool", map[string]any{"error": err.Error()}) + logger.ErrorCF( + "agent", + "Failed to create web fetch tool", + map[string]any{"error": err.Error()}, + ) } if tool.maxChars != 50000 { @@ -1017,7 +1085,13 @@ func TestNewWebFetchToolWithProxy(t *testing.T) { } func TestNewWebFetchToolWithConfig_InvalidPrivateHostWhitelist(t *testing.T) { - _, err := NewWebFetchToolWithConfig(1024, "", format, testFetchLimit, []string{"not-an-ip-or-cidr"}) + _, err := NewWebFetchToolWithConfig( + 1024, + "", + format, + testFetchLimit, + []string{"not-an-ip-or-cidr"}, + ) if err == nil { t.Fatal("expected invalid whitelist entry to fail") } @@ -1173,7 +1247,11 @@ func TestWebTool_TavilySearch_RangeMapping(t *testing.T) { w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode(map[string]any{ "results": []map[string]any{ - {"title": "Recent result", "url": "https://example.com/recent", "content": "snippet"}, + { + "title": "Recent result", + "url": "https://example.com/recent", + "content": "snippet", + }, }, }) })) @@ -1303,7 +1381,10 @@ func TestWebFetchTool_CloudflareChallenge_RetryFailsToo(t *testing.T) { // Should not be an error — the retry response is used as-is (403 is a valid HTTP response) if result.IsError { - t.Fatalf("expected non-error result even when retry is also blocked, got: %s", result.ForLLM) + t.Fatalf( + "expected non-error result even when retry is also blocked, got: %s", + result.ForLLM, + ) } // Status in the JSON result should reflect the 403 if !strings.Contains(result.ForLLM, "403") { @@ -1468,7 +1549,10 @@ func TestWebTool_GLMSearch_Success(t *testing.T) { t.Errorf("Expected Content-Type application/json, got %s", r.Header.Get("Content-Type")) } if r.Header.Get("Authorization") != "Bearer test-glm-key" { - t.Errorf("Expected Authorization Bearer test-glm-key, got %s", r.Header.Get("Authorization")) + t.Errorf( + "Expected Authorization Bearer test-glm-key, got %s", + r.Header.Get("Authorization"), + ) } var payload map[string]any @@ -1534,14 +1618,21 @@ func TestWebTool_GLMSearch_RangeMapping(t *testing.T) { t.Fatalf("failed to decode payload: %v", err) } if payload["search_recency_filter"] != "oneMonth" { - t.Fatalf("expected search_recency_filter=oneMonth, got %v", payload["search_recency_filter"]) + t.Fatalf( + "expected search_recency_filter=oneMonth, got %v", + payload["search_recency_filter"], + ) } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode(map[string]any{ "search_result": []map[string]any{ - {"title": "Recent GLM Result", "content": "snippet", "link": "https://example.com/glm-range"}, + { + "title": "Recent GLM Result", + "content": "snippet", + "link": "https://example.com/glm-range", + }, }, }) })) @@ -1573,14 +1664,21 @@ func TestWebTool_BaiduSearch_RangeMapping(t *testing.T) { t.Fatalf("failed to decode payload: %v", err) } if payload["search_recency_filter"] != "week" { - t.Fatalf("expected search_recency_filter=week for day fallback, got %v", payload["search_recency_filter"]) + t.Fatalf( + "expected search_recency_filter=week for day fallback, got %v", + payload["search_recency_filter"], + ) } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) json.NewEncoder(w).Encode(map[string]any{ "references": []map[string]any{ - {"title": "Recent Baidu Result", "url": "https://example.com/baidu", "content": "snippet"}, + { + "title": "Recent Baidu Result", + "url": "https://example.com/baidu", + "content": "snippet", + }, }, }) }))