diff --git a/docs/architecture/hooks/hook-json-protocol.md b/docs/architecture/hooks/hook-json-protocol.md index 725869a02..b04606777 100644 --- a/docs/architecture/hooks/hook-json-protocol.md +++ b/docs/architecture/hooks/hook-json-protocol.md @@ -522,7 +522,7 @@ Standard flow for plugin tool injection: ```python def handle_before_llm(params: dict) -> dict: tools = params.get("tools", []) - + # Add plugin tool definition tools.append({ "type": "function", @@ -538,7 +538,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -555,12 +555,12 @@ def handle_before_llm(params: dict) -> dict: ```python def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") - + if tool == "my_plugin_tool": # Implement tool logic here args = params.get("arguments", {}) input_text = args.get("input", "") - + # Return result directly, no need to register in ToolRegistry return { "action": "respond", @@ -570,7 +570,7 @@ def handle_before_tool(params: dict) -> dict: "is_error": False } } - + return {"action": "continue"} ``` diff --git a/docs/architecture/hooks/hook-json-protocol.zh.md b/docs/architecture/hooks/hook-json-protocol.zh.md index 9c11c6270..dac010a3e 100644 --- a/docs/architecture/hooks/hook-json-protocol.zh.md +++ b/docs/architecture/hooks/hook-json-protocol.zh.md @@ -522,7 +522,7 @@ runtime 观察型事件，仅广播，无需响应。`id` 为 `0` 或不存在 ```python def handle_before_llm(params: dict) -> dict: tools = params.get("tools", []) - + # 添加插件工具定义 tools.append({ "type": "function", @@ -538,7 +538,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -555,12 +555,12 @@ def handle_before_llm(params: dict) -> dict: ```python def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") - + if tool == "my_plugin_tool": # 在这里实现工具逻辑 args = params.get("arguments", {}) input_text = args.get("input", "") - + # 直接返回结果，无需在 ToolRegistry 注册 return { "action": "respond", @@ -570,7 +570,7 @@ def handle_before_tool(params: dict) -> dict: "is_error": False } } - + return {"action": "continue"} ``` diff --git a/docs/architecture/hooks/plugin-tool-injection.md b/docs/architecture/hooks/plugin-tool-injection.md index 9e699867b..b0436bc66 100644 --- a/docs/architecture/hooks/plugin-tool-injection.md +++ b/docs/architecture/hooks/plugin-tool-injection.md @@ -67,7 +67,7 @@ def handle_hello(params: dict) -> dict: def handle_before_llm(params: dict) -> dict: """Inject weather query tool definition""" tools = params.get("tools", []) - + # Add weather query tool tools.append({ "type": "function", @@ -86,7 +86,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -102,17 +102,17 @@ def handle_before_tool(params: dict) -> dict: """Handle tool call, return result directly""" tool = params.get("tool", "") args = params.get("arguments", {}) - + if tool == "get_weather": city = args.get("city", "") result = get_weather(city) - + # Use respond action to return result directly, skip ToolRegistry return { "action": "respond", "result": result, } - + # Other tools continue normal flow return {"action": "continue"} @@ -142,7 +142,7 @@ def send_response(message_id: int, result: Any | None = None, error: str | None payload["error"] = {"code": -32000, "message": error} else: payload["result"] = result if result is not None else {} - + sys.stdout.write(json.dumps(payload, ensure_ascii=True) + "\n") sys.stdout.flush() @@ -152,19 +152,19 @@ def main() -> int: line = raw_line.strip() if not line: continue - + try: message = json.loads(line) except json.JSONDecodeError: continue - + method = message.get("method") message_id = message.get("id", 0) params = message.get("params") or {} - + if not message_id: continue - + try: result = handle_request(str(method or ""), params) send_response(int(message_id), result=result) @@ -172,7 +172,7 @@ def main() -> int: send_response(int(message_id), error=str(exc)) except Exception as exc: send_response(int(message_id), error=f"unexpected error: {exc}") - + return 0 @@ -375,7 +375,7 @@ Multiple tools can be injected simultaneously: ```python def handle_before_llm(params: dict) -> dict: tools = params.get("tools", []) - + # Tool 1: Weather query tools.append({ "type": "function", @@ -391,7 +391,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + # Tool 2: Calculator tools.append({ "type": "function", @@ -407,7 +407,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -422,13 +422,13 @@ def handle_before_llm(params: dict) -> dict: def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") args = params.get("arguments", {}) - + if tool == "get_weather": return { "action": "respond", "result": get_weather(args.get("city", "")), } - + if tool == "calculate": # Simple calculation example try: @@ -451,7 +451,7 @@ def handle_before_tool(params: dict) -> dict: "is_error": True, }, } - + return {"action": "continue"} ``` @@ -504,7 +504,7 @@ func (h *WeatherPluginHook) BeforeLLM( }, }, }) - + return req, agent.HookDecision{Action: agent.HookActionContinue}, nil } @@ -514,7 +514,7 @@ func (h *WeatherPluginHook) BeforeTool( ) (*agent.ToolCallHookRequest, agent.HookDecision, error) { if call.Tool == "get_weather" { city := call.Arguments["city"].(string) - + // Set HookResult, use respond action next := call.Clone() next.HookResult = &tools.ToolResult{ @@ -522,10 +522,10 @@ func (h *WeatherPluginHook) BeforeTool( Silent: false, IsError: false, } - + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil } - + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil } @@ -572,14 +572,14 @@ This means: def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") args = params.get("arguments", {}) - + # Security check: only handle plugin tools if tool in ["get_weather", "calculate"]: return { "action": "respond", "result": execute_plugin_tool(tool, args), } - + # Other tools continue normal flow (will go through approval) return {"action": "continue"} ``` diff --git a/docs/architecture/hooks/plugin-tool-injection.zh.md b/docs/architecture/hooks/plugin-tool-injection.zh.md index ccc7ff7f6..0448ec1a8 100644 --- a/docs/architecture/hooks/plugin-tool-injection.zh.md +++ b/docs/architecture/hooks/plugin-tool-injection.zh.md @@ -67,7 +67,7 @@ def handle_hello(params: dict) -> dict: def handle_before_llm(params: dict) -> dict: """注入天气查询工具定义""" tools = params.get("tools", []) - + # 添加天气查询工具 tools.append({ "type": "function", @@ -86,7 +86,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -102,17 +102,17 @@ def handle_before_tool(params: dict) -> dict: """处理工具调用，直接返回结果""" tool = params.get("tool", "") args = params.get("arguments", {}) - + if tool == "get_weather": city = args.get("city", "") result = get_weather(city) - + # 使用 respond action 直接返回结果，跳过 ToolRegistry return { "action": "respond", "result": result, } - + # 其他工具继续正常流程 return {"action": "continue"} @@ -142,7 +142,7 @@ def send_response(message_id: int, result: Any | None = None, error: str | None payload["error"] = {"code": -32000, "message": error} else: payload["result"] = result if result is not None else {} - + sys.stdout.write(json.dumps(payload, ensure_ascii=True) + "\n") sys.stdout.flush() @@ -152,19 +152,19 @@ def main() -> int: line = raw_line.strip() if not line: continue - + try: message = json.loads(line) except json.JSONDecodeError: continue - + method = message.get("method") message_id = message.get("id", 0) params = message.get("params") or {} - + if not message_id: continue - + try: result = handle_request(str(method or ""), params) send_response(int(message_id), result=result) @@ -172,7 +172,7 @@ def main() -> int: send_response(int(message_id), error=str(exc)) except Exception as exc: send_response(int(message_id), error=f"unexpected error: {exc}") - + return 0 @@ -375,7 +375,7 @@ media:// ```python def handle_before_llm(params: dict) -> dict: tools = params.get("tools", []) - + # 工具1：天气查询 tools.append({ "type": "function", @@ -391,7 +391,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + # 工具2：计算器 tools.append({ "type": "function", @@ -407,7 +407,7 @@ def handle_before_llm(params: dict) -> dict: } } }) - + return { "action": "modify", "request": { @@ -422,13 +422,13 @@ def handle_before_llm(params: dict) -> dict: def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") args = params.get("arguments", {}) - + if tool == "get_weather": return { "action": "respond", "result": get_weather(args.get("city", "")), } - + if tool == "calculate": # 简单计算示例 try: @@ -451,7 +451,7 @@ def handle_before_tool(params: dict) -> dict: "is_error": True, }, } - + return {"action": "continue"} ``` @@ -504,7 +504,7 @@ func (h *WeatherPluginHook) BeforeLLM( }, }, }) - + return req, agent.HookDecision{Action: agent.HookActionContinue}, nil } @@ -514,7 +514,7 @@ func (h *WeatherPluginHook) BeforeTool( ) (*agent.ToolCallHookRequest, agent.HookDecision, error) { if call.Tool == "get_weather" { city := call.Arguments["city"].(string) - + // 设置 HookResult，使用 respond action next := call.Clone() next.HookResult = &tools.ToolResult{ @@ -522,10 +522,10 @@ func (h *WeatherPluginHook) BeforeTool( Silent: false, IsError: false, } - + return next, agent.HookDecision{Action: agent.HookActionRespond}, nil } - + return call, agent.HookDecision{Action: agent.HookActionContinue}, nil } @@ -572,14 +572,14 @@ func getWeatherData(city string) string { def handle_before_tool(params: dict) -> dict: tool = params.get("tool", "") args = params.get("arguments", {}) - + # 安全检查：只处理插件工具 if tool in ["get_weather", "calculate"]: return { "action": "respond", "result": execute_plugin_tool(tool, args), } - + # 其他工具继续正常流程（会经过审批） return {"action": "continue"} ``` diff --git a/docs/guides/configuration.it.md b/docs/guides/configuration.it.md new file mode 100644 index 000000000..d7de46895 --- /dev/null +++ b/docs/guides/configuration.it.md @@ -0,0 +1,281 @@ +# ⚙️ Guida alla Configurazione + +> Torna al [README](../../README.md) + +## ⚙️ Configurazione + +File di configurazione: `~/.picoclaw/config.json` + +### Variabili d'Ambiente + +Puoi sovrascrivere i percorsi predefiniti usando variabili d'ambiente. Questo è utile per installazioni portatili, distribuzioni containerizzate, o per eseguire picoclaw come servizio di sistema. Queste variabili sono indipendenti e controllano percorsi diversi. + +| Variabile | Descrizione | Percorso Predefinito | +|-------------------|-----------------------------------------------------------------------------------------------------------------------------------------|---------------------------| +| `PICOCLAW_CONFIG` | Sovrascrive il percorso al file di configurazione. Indica direttamente a picoclaw quale `config.json` caricare, ignorando tutte le altre posizioni. | `~/.picoclaw/config.json` | +| `PICOCLAW_HOME` | Sovrascrive la directory radice per i dati di picoclaw. Modifica la posizione predefinita del `workspace` e delle altre directory dati. | `~/.picoclaw` | + +**Esempi:** + +```bash +# Esegui picoclaw usando un file di configurazione specifico +# Il percorso del workspace verrà letto da quel file di configurazione +PICOCLAW_CONFIG=/etc/picoclaw/production.json picoclaw gateway + +# Esegui picoclaw con tutti i dati salvati in /opt/picoclaw +# La configurazione verrà caricata dal percorso predefinito ~/.picoclaw/config.json +# Il workspace verrà creato in /opt/picoclaw/workspace +PICOCLAW_HOME=/opt/picoclaw picoclaw agent + +# Usa entrambi per un setup completamente personalizzato +PICOCLAW_HOME=/srv/picoclaw PICOCLAW_CONFIG=/srv/picoclaw/main.json picoclaw gateway +``` + +### Struttura del Workspace + +PicoClaw salva i dati nel workspace configurato (predefinito: `~/.picoclaw/workspace`): + +``` +~/.picoclaw/workspace/ +├── sessions/ # Sessioni di conversazione e cronologia +├── memory/ # Memoria a lungo termine (MEMORY.md) +├── state/ # Stato persistente (ultimo canale, ecc.) +├── cron/ # Database dei job pianificati +├── skills/ # Skill personalizzate +├── AGENT.md # Guida al comportamento dell'agent +├── HEARTBEAT.md # Prompt per task periodici (controllato ogni 30 min) +├── SOUL.md # Anima dell'agent +└── USER.md # Preferenze dell'utente +``` + +> **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 + +Per impostazione predefinita, le skill vengono caricate da: + +1. `~/.picoclaw/workspace/skills` (workspace) +2. `~/.picoclaw/skills` (globale) +3. `/skills` (builtin) + +Per configurazioni avanzate/di test, puoi sovrascrivere la directory radice delle skill builtin con: + +```bash +export PICOCLAW_BUILTIN_SKILLS=/path/to/skills +``` + +### Politica Unificata di Esecuzione dei Comandi + +- I comandi slash generici vengono eseguiti tramite un unico percorso in `pkg/agent/loop.go` via `commands.Executor`. +- Gli adattatori dei canali non consumano più localmente i comandi generici; inoltrano il testo in entrata al percorso bus/agent. Telegram registra ancora automaticamente i comandi supportati all'avvio. +- 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 + +La dichiarazione dei tool per-agent vive nel frontmatter di `AGENT.md`, non in `config.json`. + +Se `tools` è omesso nel frontmatter, l'agent riceve il normale set globale dei tool abilitati. Se `tools` è presente, PicoClaw registra per quell'agent solo i tool runtime elencati. + +```md +--- +name: Research Agent +description: Specialista per ricerca web e analisi approfondita. +tools: [read_file, write_file, web_search, web_fetch, message] +skills: [deep-research] +mcpServers: [web-index] +--- + +Sei l'agent di ricerca. +``` + +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`. +- Le dichiarazioni dei tool in `AGENT.md` sono usate dal runtime e dai tool, ma non vengono iniettate nel prompt di discovery. + +### Discovery Multi-Agent (Automatica) + +Quando un agent ha peer spawnabili, PicoClaw inietta automaticamente nel suo system prompt 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 identitario da `AGENT.md` frontmatter | +| `description` | Descrizione identitaria da `AGENT.md` frontmatter | + +Dettagli importanti: + +- La sezione include solo i peer che l'agent corrente può spawnare tramite `subagents.allow_agents`. +- L'agent corrente e i peer non spawnabili vengono omessi, così il modello non pianifica contro agent non disponibili. +- La discovery è volutamente leggera. Fornisce al modello solo l'identità necessaria per scegliere un peer: `id`, `name`, `description`. +- `config.json` resta il layer infrastrutturale: workspace, agent di default, routing e permessi di subagent. Questi permessi controllano anche la visibilità nella discovery. +- `AGENT.md` resta il layer di identità. Il codice runtime e i tool possono comunque usare `tools`, `skills`, `mcpServers` e `model` quando avviene la delega. + +Forma dell'oggetto iniettato: + +```json +{ + "agents": [ + { + "id": "research", + "name": "Research Agent", + "description": "Specialista per investigazioni e lavoro web." + } + ] +} +``` + +In pratica, un agent generalista sceglie un peer in base alla descrizione del suo ruolo, poi chiama `spawn` con l'`agent_id` del peer. Il runtime risolve il resto. + +### 🔒 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. + +#### Configurazione Predefinita + +```json +{ + "agents": { + "defaults": { + "workspace": "~/.picoclaw/workspace", + "restrict_to_workspace": true + } + } +} +``` + +| Opzione | Predefinito | Descrizione | +| ----------------------- | ----------------------- | ---------------------------------------------------- | +| `workspace` | `~/.picoclaw/workspace` | Directory di lavoro dell'agent | +| `restrict_to_workspace` | `true` | Limita l'accesso a file/comandi al workspace | + +#### Strumenti Protetti + +Quando `restrict_to_workspace: true`, i seguenti strumenti sono in sandbox: + +| Strumento | Funzione | Restrizione | +| ------------- | ------------------------- | ---------------------------------------------------- | +| `read_file` | Legge file | Solo file all'interno del workspace | +| `write_file` | Scrive file | Solo file all'interno del workspace | +| `list_dir` | Elenca directory | Solo directory all'interno del workspace | +| `edit_file` | Modifica file | Solo file all'interno del workspace | +| `append_file` | Aggiunge ai file | Solo file all'interno del workspace | +| `exec` | Esegue comandi | I percorsi dei comandi devono essere nel workspace | + +#### Protezione Exec Aggiuntiva + +Anche con `restrict_to_workspace: false`, lo strumento `exec` blocca questi comandi pericolosi: + +* `rm -rf`, `del /f`, `rmdir /s` — Cancellazione di massa +* `format`, `mkfs`, `diskpart` — Formattazione del disco +* `dd if=` — Imaging del disco +* Scrittura su `/dev/sd[a-z]` — Scritture dirette su disco +* `shutdown`, `reboot`, `poweroff` — Spegnimento del sistema +* Fork bomb `:(){ :|:& };:` + +### Controllo Accesso ai File + +| Chiave di configurazione | Tipo | Predefinito | Descrizione | +|--------------------------|------|-------------|-------------| +| `tools.allow_read_paths` | string[] | `[]` | Percorsi aggiuntivi consentiti per la lettura al di fuori del workspace | +| `tools.allow_write_paths` | string[] | `[]` | Percorsi aggiuntivi consentiti per la scrittura al di fuori del workspace | + +### Sicurezza Exec + +| Chiave di configurazione | Tipo | Predefinito | Descrizione | +|--------------------------|------|-------------|-------------| +| `tools.exec.allow_remote` | bool | `false` | Consente lo strumento exec da canali remoti (Telegram/Discord ecc.) | +| `tools.exec.enable_deny_patterns` | bool | `true` | Abilita l'intercettazione dei comandi pericolosi | +| `tools.exec.custom_deny_patterns` | string[] | `[]` | Pattern regex personalizzati da bloccare | +| `tools.exec.custom_allow_patterns` | string[] | `[]` | Pattern regex personalizzati da consentire | + +> **Nota di sicurezza:** La protezione dei symlink è abilitata per impostazione predefinita — tutti i percorsi file vengono risolti tramite `filepath.EvalSymlinks` prima del confronto con la whitelist, prevenendo attacchi di escape tramite symlink. + +#### Limitazione Nota: Processi Figlio degli Strumenti di Build + +Il controllo di sicurezza exec ispeziona solo la riga di comando avviata direttamente da PicoClaw. Non ispeziona ricorsivamente i processi figlio generati da strumenti di sviluppo consentiti come `make`, `go run`, `cargo`, `npm run` o script di build personalizzati. + +Ciò significa che un comando di primo livello può comunque compilare o avviare altri binari dopo aver superato il controllo iniziale. In pratica, tratta gli script di build, i Makefile, gli script di pacchetti e i binari generati come codice eseguibile che richiede lo stesso livello di revisione di un comando shell diretto. + +Per ambienti ad alto rischio: + +* Esamina gli script di build prima dell'esecuzione. +* Preferisci l'approvazione/revisione manuale per i workflow di compilazione ed esecuzione. +* Esegui PicoClaw in un container o VM se hai bisogno di un isolamento più forte di quello fornito dal controllo integrato. + +#### Esempi di Errore + +``` +[ERROR] tool: Tool execution failed +{tool=exec, error=Command blocked by safety guard (path outside working dir)} +``` + +``` +[ERROR] tool: Tool execution failed +{tool=exec, error=Command blocked by safety guard (dangerous pattern detected)} +``` + +#### Disabilitare le Restrizioni (Rischio di Sicurezza) + +Se hai bisogno che l'agent acceda a percorsi al di fuori del workspace: + +**Metodo 1: File di configurazione** + +```json +{ + "agents": { + "defaults": { + "restrict_to_workspace": false + } + } +} +``` + +**Metodo 2: Variabile d'ambiente** + +```bash +export PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE=false +``` + +> ⚠️ **Attenzione**: Disabilitare questa restrizione consente all'agent di accedere a qualsiasi percorso sul tuo sistema. Usare con cautela solo in ambienti controllati. + +#### Coerenza dei Confini di Sicurezza + +L'impostazione `restrict_to_workspace` si applica in modo coerente a tutti i percorsi di esecuzione: + +| Percorso di esecuzione | Confine di sicurezza | +| ---------------------- | --------------------------------- | +| Main Agent | `restrict_to_workspace` ✅ | +| Subagent / Spawn | Eredita la stessa restrizione ✅ | +| Heartbeat tasks | Eredita la stessa restrizione ✅ | + +Tutti i percorsi condividono la stessa restrizione del workspace — non è possibile aggirare il confine di sicurezza tramite subagent o task pianificati. + +### Heartbeat (Task Periodici) + +PicoClaw può eseguire task periodici automaticamente. Crea un file `HEARTBEAT.md` nel tuo workspace: + +```markdown +# Periodic Tasks + +- Check my email for important messages +- Review my calendar for upcoming events +- Check the weather forecast +``` + +L'agent leggerà questo file ogni 30 minuti (configurabile) ed eseguirà tutti i task usando gli strumenti disponibili. + +#### Task Asincroni con Spawn + +Per task di lunga durata (ricerca web, chiamate API), usa lo strumento `spawn` per creare un **subagent**: + +```markdown +# Periodic Tasks +``` diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index 28fc7b775..3bec847ba 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -211,6 +211,69 @@ earlier and broader fallback rules later. For more complete routing and model-tier examples, see the [Routing Guide](routing-guide.md). +### Agent Tool Allowlist + +Per-agent tool declarations live in `AGENT.md` frontmatter, not in `config.json`. + +If `tools` is omitted from frontmatter, the agent gets the normal globally enabled tool set. If `tools` is present, PicoClaw registers only the listed runtime tools for that agent. + +```md +--- +name: Research Agent +description: Specialist for web research and in-depth analysis. +tools: [read_file, write_file, web_search, web_fetch, message] +skills: [deep-research] +mcpServers: [web-index] +--- + +You are the research agent. +``` + +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`. +- Tool declarations in `AGENT.md` are used by runtime/tooling, but they are not injected into the discovery prompt. + +### Agent Discovery (Automatic) + +When an agent has spawnable peers and can call `spawn`, PicoClaw injects a structured agent registry into that 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` | Agent identity name from `AGENT.md` frontmatter | +| `description` | Agent identity description from `AGENT.md` frontmatter | + +Important behavior: + +- The discovery section appears only when the current agent has the `spawn` tool and includes only peer agents it is permitted to spawn via `subagents.allow_agents`. +- The current agent and non-spawnable peers are omitted, so the model does not plan against unavailable agents. +- Discovery is intentionally lightweight. It gives the model only the identity it needs to choose a peer: `id`, `name`, and `description`. +- `config.json` remains the infrastructure layer: workspace, default agent selection, routing, and subagent permissions. Those permissions also gate discovery visibility. +- `AGENT.md` remains the identity layer. Runtime/tool code can still use its `tools`, `skills`, `mcpServers`, and `model` fields when delegation happens. + +Example injected shape: + +```json +{ + "agents": [ + { + "id": "research", + "name": "Research Agent", + "description": "Specialist for long-form investigation and web work." + } + ] +} +``` + +In practice, this means a generalist agent can choose a peer based on its role description, then call `spawn` with the peer's `agent_id`. The runtime resolves the rest. + ### 🔒 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/reference/config-versioning.md b/docs/reference/config-versioning.md index 36f327e8c..74a5bbd89 100644 --- a/docs/reference/config-versioning.md +++ b/docs/reference/config-versioning.md @@ -282,4 +282,3 @@ New config (version 3): - Check that the migration doesn't overwrite values with defaults unnecessarily - Review the conversion logic in the loader functions - Check the auto-backup files (e.g., `config.json.20260330.bak`) to recover original data - diff --git a/pkg/agent/agent_init.go b/pkg/agent/agent_init.go index e95fbe7f8..8420cd101 100644 --- a/pkg/agent/agent_init.go +++ b/pkg/agent/agent_init.go @@ -352,5 +352,7 @@ func registerSharedTools( }) agent.Tools.Register(delegateTool) } + + warnOnUnknownAgentToolDeclarations(agentID, agent.Workspace, agent.Definition, agent.Tools) } } diff --git a/pkg/agent/agent_mcp.go b/pkg/agent/agent_mcp.go index b3c69504b..e8cdf81c8 100644 --- a/pkg/agent/agent_mcp.go +++ b/pkg/agent/agent_mcp.go @@ -85,8 +85,18 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { return nil } + mcpCfg := filterMCPConfigServers(al.cfg.Tools.MCP, al.registry.allowedMCPServers()) + if mcpCfg.Servers == nil || len(mcpCfg.Servers) == 0 { + logger.InfoCF( + "agent", + "No MCP servers selected after applying per-agent mcpServers allowlists", + nil, + ) + return nil + } + findValidServer := false - for _, serverCfg := range al.cfg.Tools.MCP.Servers { + for _, serverCfg := range mcpCfg.Servers { if serverCfg.Enabled { findValidServer = true } @@ -105,7 +115,7 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { workspacePath = defaultAgent.Workspace } - if err := mcpManager.LoadFromMCPConfig(ctx, al.cfg.Tools.MCP, workspacePath); err != nil { + if err := mcpManager.LoadFromMCPConfig(ctx, mcpCfg, workspacePath); err != nil { al.mcp.setInitErr(fmt.Errorf("failed to load MCP servers: %w", err)) logger.WarnCF("agent", "Failed to load MCP servers, MCP tools will not be available", map[string]any{ @@ -132,27 +142,9 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { // Determine whether this server's tools should be deferred (hidden). // Per-server "deferred" field takes precedence over the global Discovery.Enabled. - serverCfg := al.cfg.Tools.MCP.Servers[serverName] + serverCfg := mcpCfg.Servers[serverName] registerAsHidden := serverIsDeferred(al.cfg.Tools.MCP.Discovery.Enabled, serverCfg) - - for _, agentID := range agentIDs { - agent, ok := al.registry.GetAgent(agentID) - if !ok || agent.ContextBuilder == nil { - continue - } - if err := agent.ContextBuilder.RegisterPromptContributor(mcpServerPromptContributor{ - serverName: serverName, - toolCount: len(conn.Tools), - deferred: registerAsHidden, - }); err != nil { - logger.WarnCF("agent", "Failed to register MCP prompt contributor", - map[string]any{ - "agent_id": agentID, - "server": serverName, - "error": err.Error(), - }) - } - } + registeredToolsByAgent := make(map[string]map[string]struct{}, len(agentIDs)) for _, tool := range conn.Tools { for _, agentID := range agentIDs { @@ -160,8 +152,18 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { if !ok { continue } + if !agent.AllowsMCPServer(serverName) { + logger.DebugCF("agent", "Skipped MCP tool registration by agent mcpServers allowlist", + map[string]any{ + "agent_id": agentID, + "server": serverName, + "tool": tool.Name, + }) + continue + } mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) + toolName := mcpTool.Name() mcpTool.SetWorkspace(agent.Workspace) mcpTool.SetMaxInlineTextRunes(al.cfg.Tools.MCP.GetMaxInlineTextChars()) mcpTool.SetEventPublisher(al.runtimeEvents) @@ -171,18 +173,36 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { } else { agent.Tools.Register(mcpTool) } + if !toolRegistryIncludes(agent.Tools, toolName) { + continue + } + recordRegisteredMCPTool(registeredToolsByAgent, agentID, toolName) totalRegistrations++ logger.DebugCF("agent", "Registered MCP tool", map[string]any{ "agent_id": agentID, "server": serverName, "tool": tool.Name, - "name": mcpTool.Name(), + "name": toolName, "deferred": registerAsHidden, }) } } + + for _, agentID := range agentIDs { + agent, ok := al.registry.GetAgent(agentID) + if !ok { + continue + } + registerMCPServerPromptContributor( + agentID, + agent, + serverName, + len(registeredToolsByAgent[agentID]), + registerAsHidden, + ) + } } logger.InfoCF("agent", "MCP tools registered successfully", map[string]any{ @@ -230,6 +250,9 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { if !ok { continue } + if !agentHasDiscoverableMCPServers(al.cfg, agent.MCPServerAllowlist) { + continue + } if useRegex { agent.Tools.Register(tools.NewRegexSearchTool(agent.Tools, ttl, maxSearchResults)) @@ -246,6 +269,89 @@ func (al *AgentLoop) ensureMCPInitialized(ctx context.Context) error { return al.mcp.getInitErr() } +func registerMCPServerPromptContributor( + agentID string, + agent *AgentInstance, + serverName string, + toolCount int, + registerAsHidden bool, +) { + if agent == nil || agent.ContextBuilder == nil || toolCount <= 0 { + return + } + if err := agent.ContextBuilder.RegisterPromptContributor(mcpServerPromptContributor{ + serverName: serverName, + toolCount: toolCount, + deferred: registerAsHidden, + }); err != nil { + logger.WarnCF("agent", "Failed to register MCP prompt contributor", + map[string]any{ + "agent_id": agentID, + "server": serverName, + "error": err.Error(), + }) + } +} + +func recordRegisteredMCPTool( + registeredToolsByAgent map[string]map[string]struct{}, + agentID, toolName string, +) { + if registeredToolsByAgent[agentID] == nil { + registeredToolsByAgent[agentID] = make(map[string]struct{}) + } + registeredToolsByAgent[agentID][toolName] = struct{}{} +} + +func toolRegistryIncludes(registry *tools.ToolRegistry, name string) bool { + if registry == nil { + return false + } + return registry.HasRegistered(name) +} + +func filterMCPConfigServers( + mcpCfg config.MCPConfig, + allowed map[string]struct{}, +) config.MCPConfig { + if allowed == nil { + return mcpCfg + } + + filtered := mcpCfg + filtered.Servers = make(map[string]config.MCPServerConfig) + normalizedAllowed := make(map[string]struct{}, len(allowed)) + for serverName := range allowed { + name := normalizeMCPServerName(serverName) + if name == "" { + continue + } + normalizedAllowed[name] = struct{}{} + } + for serverName, serverCfg := range mcpCfg.Servers { + if _, ok := normalizedAllowed[normalizeMCPServerName(serverName)]; ok { + filtered.Servers[serverName] = serverCfg + } + } + + return filtered +} + +func agentHasDiscoverableMCPServers(cfg *config.Config, allowed map[string]struct{}) bool { + if cfg == nil || !cfg.Tools.MCP.Enabled || !cfg.Tools.MCP.Discovery.Enabled { + return false + } + + filtered := filterMCPConfigServers(cfg.Tools.MCP, allowed) + for _, serverCfg := range filtered.Servers { + if serverCfg.Enabled && serverIsDeferred(cfg.Tools.MCP.Discovery.Enabled, serverCfg) { + return true + } + } + + return false +} + // serverIsDeferred reports whether an MCP server's tools should be registered // as hidden (deferred/discovery mode). // diff --git a/pkg/agent/agent_mcp_test.go b/pkg/agent/agent_mcp_test.go index b68fcc2c1..f85861146 100644 --- a/pkg/agent/agent_mcp_test.go +++ b/pkg/agent/agent_mcp_test.go @@ -14,6 +14,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/mcp" + agenttools "github.com/sipeed/picoclaw/pkg/tools" ) func boolPtr(b bool) *bool { return &b } @@ -135,6 +136,139 @@ func TestServerIsDeferred(t *testing.T) { } } +func TestRegisterMCPServerPromptContributorUsesActualRegisteredToolCount(t *testing.T) { + cb := NewContextBuilder(t.TempDir()) + agent := &AgentInstance{ContextBuilder: cb} + + registerMCPServerPromptContributor("research", agent, "github", 0, false) + messages := cb.BuildMessagesFromPrompt(PromptBuildRequest{CurrentMessage: "hello"}) + if prompt := messages[0].Content; strings.Contains(prompt, "MCP server `github`") { + t.Fatalf("expected no MCP prompt when no tools were registered, got %q", prompt) + } + + registerMCPServerPromptContributor("research", agent, "github", 2, false) + messages = cb.BuildMessagesFromPrompt(PromptBuildRequest{CurrentMessage: "hello"}) + prompt := messages[0].Content + if !strings.Contains(prompt, "MCP server `github` is connected") { + t.Fatalf("expected MCP prompt for registered tools, got %q", prompt) + } + if !strings.Contains(prompt, "It contributes 2 tool(s)") { + t.Fatalf("expected actual registered tool count in prompt, got %q", prompt) + } +} + +func TestToolRegistryIncludesReportsOnlyRegisteredTools(t *testing.T) { + registry := agenttools.NewToolRegistry() + registry.SetAllowlist([]string{"mcp_github_search"}) + + registry.RegisterHidden(&allowlistTestTool{name: "mcp_github_search"}) + registry.RegisterHidden(&allowlistTestTool{name: "mcp_github_create_issue"}) + + if !toolRegistryIncludes(registry, "mcp_github_search") { + t.Fatal("expected hidden registered MCP tool to be included") + } + if toolRegistryIncludes(registry, "mcp_github_create_issue") { + t.Fatal("blocked MCP tool should not be included") + } +} + +func TestFilterMCPConfigServersCaseInsensitivePreservesOriginalKeys(t *testing.T) { + mcpCfg := config.MCPConfig{ + Servers: map[string]config.MCPServerConfig{ + "GitHub": {Enabled: true}, + "filesystem": {Enabled: true}, + "Slack": {Enabled: true}, + }, + } + allowed := map[string]struct{}{ + "github": {}, + "FILESYSTEM": {}, + } + + filtered := filterMCPConfigServers(mcpCfg, allowed) + + if len(filtered.Servers) != 2 { + t.Fatalf("filtered.Servers = %v, want 2 entries", filtered.Servers) + } + if _, ok := filtered.Servers["GitHub"]; !ok { + t.Fatal("expected original GitHub config key to be preserved") + } + if _, ok := filtered.Servers["filesystem"]; !ok { + t.Fatal("expected filesystem config key to be preserved") + } + if _, ok := filtered.Servers["github"]; ok { + t.Fatal("did not expect normalized github key to replace original config key") + } + if _, ok := filtered.Servers["Slack"]; ok { + t.Fatal("did not expect unallowed Slack server") + } +} + +func TestAgentHasDiscoverableMCPServers(t *testing.T) { + deferredFalse := false + cfg := &config.Config{ + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + ToolConfig: config.ToolConfig{Enabled: true}, + Discovery: config.ToolDiscoveryConfig{ + Enabled: true, + UseBM25: true, + UseRegex: false, + }, + Servers: map[string]config.MCPServerConfig{ + "github": {Enabled: true}, + "filesystem": {Enabled: true, Deferred: &deferredFalse}, + }, + }, + }, + } + + tests := []struct { + name string + allowed map[string]struct{} + want bool + }{ + { + name: "nil allowlist includes discoverable enabled server", + want: true, + }, + { + name: "empty allowlist denies all servers", + allowed: map[string]struct{}{}, + want: false, + }, + { + name: "selected server discoverable", + allowed: map[string]struct{}{ + "github": {}, + }, + want: true, + }, + { + name: "selected server opted out of discovery", + allowed: map[string]struct{}{ + "filesystem": {}, + }, + want: false, + }, + { + name: "unknown allowlist server matches nothing", + allowed: map[string]struct{}{ + "slack": {}, + }, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := agentHasDiscoverableMCPServers(cfg, tt.allowed); got != tt.want { + t.Fatalf("agentHasDiscoverableMCPServers() = %v, want %v", got, tt.want) + } + }) + } +} + func TestEnsureMCPInitialized_LoadFailureSetsInitErr(t *testing.T) { al, cfg, _, _, cleanup := newTestAgentLoop(t) defer cleanup() diff --git a/pkg/agent/context.go b/pkg/agent/context.go index ecde7c33e..7f5b32fef 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -26,6 +26,7 @@ type ContextBuilder struct { skillsLoader *skills.SkillsLoader memory *MemoryStore splitOnMarker bool + agentDiscovery func(agentID string) []AgentDescriptor promptRegistry *PromptRegistry // Cache for system prompt to avoid rebuilding on every call. @@ -66,6 +67,24 @@ func (cb *ContextBuilder) WithSplitOnMarker(enabled bool) *ContextBuilder { return cb } +func (cb *ContextBuilder) WithAgentDiscovery( + agentID string, + discover func(agentID string) []AgentDescriptor, +) *ContextBuilder { + cb.agentDiscovery = discover + if discover != nil { + if err := cb.RegisterPromptContributor(agentDiscoveryPromptContributor{ + agentID: agentID, + discover: discover, + }); err != nil { + logger.WarnCF("agent", "Failed to register agent discovery prompt contributor", map[string]any{ + "error": err.Error(), + }) + } + } + return cb +} + func getGlobalConfigDir() string { return config.GetHome() } @@ -625,7 +644,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()) @@ -854,7 +875,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/definition.go b/pkg/agent/definition.go index cf73d607c..5b0e29137 100644 --- a/pkg/agent/definition.go +++ b/pkg/agent/definition.go @@ -35,7 +35,7 @@ type AgentFrontmatter struct { MaxTurns *int `json:"maxTurns,omitempty"` Skills []string `json:"skills,omitempty"` MCPServers []string `json:"mcpServers,omitempty"` - Fields map[string]any `json:"fields,omitempty"` + Fields map[string]any `json:"-"` } // AgentPromptDefinition represents the parsed AGENT.md or AGENTS.md prompt file. @@ -45,6 +45,7 @@ type AgentPromptDefinition struct { Body string `json:"body"` RawFrontmatter string `json:"raw_frontmatter,omitempty"` Frontmatter AgentFrontmatter `json:"frontmatter"` + FrontmatterErr string `json:"frontmatter_error,omitempty"` } // SoulDefinition represents the resolved SOUL.md file linked to the agent. @@ -146,19 +147,21 @@ func loadUserDefinition(workspace string) *UserDefinition { func parseAgentPromptDefinition(path, content string) AgentPromptDefinition { frontmatter, body := splitAgentFrontmatter(content) + parsedFrontmatter, err := parseAgentFrontmatter(path, frontmatter) return AgentPromptDefinition{ Path: path, Raw: content, Body: body, RawFrontmatter: frontmatter, - Frontmatter: parseAgentFrontmatter(path, frontmatter), + Frontmatter: parsedFrontmatter, + FrontmatterErr: errorString(err), } } -func parseAgentFrontmatter(path, frontmatter string) AgentFrontmatter { +func parseAgentFrontmatter(path, frontmatter string) (AgentFrontmatter, error) { frontmatter = strings.TrimSpace(frontmatter) if frontmatter == "" { - return AgentFrontmatter{} + return AgentFrontmatter{}, nil } rawFields := make(map[string]any) @@ -167,7 +170,7 @@ func parseAgentFrontmatter(path, frontmatter string) AgentFrontmatter { "path": path, "error": err.Error(), }) - return AgentFrontmatter{} + return AgentFrontmatter{}, err } var typed struct { @@ -184,7 +187,7 @@ func parseAgentFrontmatter(path, frontmatter string) AgentFrontmatter { "path": path, "error": err.Error(), }) - return AgentFrontmatter{} + return AgentFrontmatter{}, err } return AgentFrontmatter{ @@ -196,7 +199,7 @@ func parseAgentFrontmatter(path, frontmatter string) AgentFrontmatter { Skills: append([]string(nil), typed.Skills...), MCPServers: append([]string(nil), typed.MCPServers...), Fields: rawFields, - } + }, nil } func splitAgentFrontmatter(content string) (frontmatter, body string) { @@ -253,3 +256,10 @@ func fileExists(path string) bool { _, err := os.Stat(path) return err == nil } + +func errorString(err error) string { + if err == nil { + return "" + } + return err.Error() +} diff --git a/pkg/agent/discovery.go b/pkg/agent/discovery.go new file mode 100644 index 000000000..d2f63bc1f --- /dev/null +++ b/pkg/agent/discovery.go @@ -0,0 +1,263 @@ +package agent + +import ( + "encoding/json" + "path/filepath" + "sort" + "strings" + + "github.com/sipeed/picoclaw/pkg/routing" +) + +// AgentDescriptor is the structured discovery payload injected into each +// agent's system prompt so the LLM can choose a peer by identity. +type AgentDescriptor struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` +} + +// 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 +} + +// ListSpawnableAgents returns descriptors only when the current agent can call +// spawn, and only for peers it is allowed to spawn. Restricted peers are +// intentionally omitted from discovery. +func (r *AgentRegistry) ListSpawnableAgents(agentID string) []AgentDescriptor { + r.mu.RLock() + defer r.mu.RUnlock() + + parentID := routing.NormalizeAgentID(agentID) + parent, ok := r.agents[parentID] + if !ok || parent == nil { + return nil + } + if !agentHasSpawnTool(parent) { + return nil + } + + ids := make([]string, 0, len(r.agents)) + for id := range r.agents { + if id == parentID { + continue + } + if !agentAllowsSubagent(parent, id) { + continue + } + ids = append(ids, id) + } + sort.Strings(ids) + + descriptors := make([]AgentDescriptor, 0, len(ids)) + for _, id := range ids { + agent := r.agents[id] + if agent == nil { + continue + } + descriptors = append(descriptors, r.buildAgentDescriptorLocked(agent)) + } + 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, description := descriptorIdentity(agent.ID, definition) + + return AgentDescriptor{ + ID: agent.ID, + Name: name, + Description: description, + } +} + +func descriptorIdentity(agentID string, definition AgentContextDefinition) (string, string) { + name := agentID + description := "" + if definition.Agent != nil { + if trimmed := strings.TrimSpace(definition.Agent.Frontmatter.Name); trimmed != "" { + name = trimmed + } + if trimmed := strings.TrimSpace(definition.Agent.Frontmatter.Description); trimmed != "" { + description = trimmed + } + } + + if description == "" && + definition.Agent != nil { + if definition.Source == AgentDefinitionSourceAgent { + description = firstNonEmptyLine(definition.Agent.Body) + } else if definition.Source == AgentDefinitionSourceAgents { + description = firstMeaningfulParagraph(definition.Agent.Body) + } + } + + return name, description +} + +func firstNonEmptyLine(content string) string { + content = strings.ReplaceAll(content, "\r\n", "\n") + for _, line := range strings.Split(content, "\n") { + trimmed := strings.TrimSpace(line) + if trimmed != "" { + return trimmed + } + } + 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) 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(agents []AgentDescriptor) string { + if len(agents) == 0 { + return "" + } + + payload := struct { + Agents []AgentDescriptor `json:"agents"` + }{ + Agents: agents, + } + + encoded, err := json.MarshalIndent(payload, "", " ") + if err != nil { + return "" + } + + var header strings.Builder + header.WriteString("# Agent Discovery\n\n") + header.WriteString("This registry lists the peer agents this agent is permitted to spawn.\n") + header.WriteString( + "Choose a peer based on its description. Use only agent IDs listed here when calling spawn.\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..f31a113d8 --- /dev/null +++ b/pkg/agent/discovery_test.go @@ -0,0 +1,420 @@ +package agent + +import ( + "strings" + "testing" + + "github.com/sipeed/picoclaw/pkg/bus" + "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": `--- +name: Support Frontmatter Name +description: Support frontmatter description +--- +# Agent + +Handle support tickets carefully. +`, + }) + defer cleanupWorkspace(t, supportWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Name: "Configured Main", Workspace: mainWorkspace}, + {ID: "support", Workspace: supportWorkspace}, + }) + + 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 != "Main Frontmatter Name" { + t.Fatalf("expected frontmatter name to drive discovery, got %q", descriptors[0].Name) + } + if descriptors[0].Description != "Structured main agent" { + t.Fatalf("expected frontmatter description, got %q", descriptors[0].Description) + } + + support, ok := registry.GetAgentDescriptor("support") + if !ok || support == nil { + t.Fatal("expected support descriptor lookup to succeed") + } + if support.Name != "Support Frontmatter Name" { + t.Fatalf("expected support frontmatter name, got %q", support.Name) + } + if support.Description != "Support frontmatter description" { + t.Fatalf("expected support frontmatter description, got %q", support.Description) + } +} + +func TestAgentRegistry_ListSpawnableAgentsRespectsPermissions(t *testing.T) { + cfg := testCfg([]config.AgentConfig{ + { + ID: "parent", + Default: true, + Subagents: &config.SubagentsConfig{ + AllowAgents: []string{"child2", "child1"}, + }, + }, + {ID: "child1"}, + {ID: "child2"}, + {ID: "restricted"}, + }) + cfg.Tools.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + descriptors := al.GetRegistry().ListSpawnableAgents("parent") + if len(descriptors) != 2 { + t.Fatalf("expected 2 spawnable descriptors, got %d: %+v", len(descriptors), descriptors) + } + if descriptors[0].ID != "child1" || descriptors[1].ID != "child2" { + t.Fatalf("expected sorted spawnable peers only, got %+v", descriptors) + } +} + +func TestAgentRegistry_ListSpawnableAgentsRequiresSpawnTool(t *testing.T) { + cfg := testCfg([]config.AgentConfig{ + { + ID: "parent", + Default: true, + Subagents: &config.SubagentsConfig{ + AllowAgents: []string{"child"}, + }, + }, + {ID: "child"}, + }) + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + if descriptors := al.GetRegistry().ListSpawnableAgents("parent"); len(descriptors) != 0 { + t.Fatalf("expected no spawnable descriptors without spawn tool, got %+v", descriptors) + } +} + +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": `--- +name: Research Agent +description: Research specialist +--- +# Agent + +Investigate deeply. +`, + }) + defer cleanupWorkspace(t, researchWorkspace) + + restrictedWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +name: Restricted Agent +description: Restricted specialist +--- +# Agent + +Handle restricted work. +`, + }) + defer cleanupWorkspace(t, restrictedWorkspace) + + cfg := testCfg([]config.AgentConfig{ + { + ID: "main", + Default: true, + Workspace: mainWorkspace, + Subagents: &config.SubagentsConfig{ + AllowAgents: []string{"research"}, + }, + }, + {ID: "research", Workspace: researchWorkspace}, + {ID: "restricted", Workspace: restrictedWorkspace}, + }) + cfg.Tools.ReadFile.Enabled = true + cfg.Tools.WriteFile.Enabled = true + cfg.Tools.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + mainAgent, ok := al.GetRegistry().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, `"id": "main"`) { + t.Fatalf("did not expect self descriptor in discovery section, got %q", systemPrompt) + } + if !strings.Contains(systemPrompt, `"id": "research"`) || + !strings.Contains(systemPrompt, `"description": "Research specialist"`) { + t.Fatalf("expected allowed peer descriptor in discovery section, got %q", systemPrompt) + } + if strings.Contains(systemPrompt, `"id": "restricted"`) || + strings.Contains(systemPrompt, `"description": "Restricted specialist"`) { + t.Fatalf("did not expect restricted peer descriptor in discovery section, got %q", systemPrompt) + } + for _, forbidden := range []string{`"current_agent_id"`, `"available_tools"`, `"model"`, `"channels"`, `"skills"`, `"mcpServers"`, `"tools"`} { + if strings.Contains(systemPrompt, forbidden) { + t.Fatalf("did not expect %s in discovery section, got %q", forbidden, systemPrompt) + } + } +} + +func TestContextBuilder_BuildMessagesOmitsAgentDiscoveryWithoutSpawnPermissions(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.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + mainAgent, ok := al.GetRegistry().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 without spawn permissions, got %q", systemPrompt) + } + if strings.Contains(systemPrompt, `"id": "research"`) { + t.Fatalf("did not expect unauthorized peer identity in system prompt, got %q", systemPrompt) + } +} + +func TestContextBuilder_BuildMessagesOmitsAgentDiscoveryWithoutSpawnTool(t *testing.T) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +description: Main agent +tools: [read_file] +--- +# 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, + Subagents: &config.SubagentsConfig{ + AllowAgents: []string{"research"}, + }, + }, + {ID: "research", Workspace: researchWorkspace}, + }) + cfg.Tools.ReadFile.Enabled = true + cfg.Tools.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + mainAgent, ok := al.GetRegistry().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 without spawn tool, got %q", systemPrompt) + } + if strings.Contains(systemPrompt, `"id": "research"`) { + t.Fatalf("did not expect peer identity without spawn tool, 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 + cfg.Tools.Spawn.Enabled = true + cfg.Tools.Subagent.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{}) + defer al.Close() + + mainAgent, ok := al.GetRegistry().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) + } +} + +func TestAgentRegistry_ListAgentsFallsBackToFirstNonEmptyAgentLine(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +name: Research Agent +--- + + +First useful line. +Second line. +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "research", Default: true, Workspace: workspace}, + }) + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + descriptor, ok := registry.GetAgentDescriptor("research") + if !ok || descriptor == nil { + t.Fatal("expected research descriptor lookup to succeed") + } + if descriptor.Description != "First useful line." { + t.Fatalf("descriptor.Description = %q, want %q", descriptor.Description, "First useful line.") + } +} diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index d0b25a0a8..4ed713035 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -38,8 +38,10 @@ type AgentInstance struct { Sessions session.SessionStore ContextBuilder *ContextBuilder Tools *tools.ToolRegistry + Definition AgentContextDefinition Subagents *config.SubagentsConfig SkillsFilter []string + MCPServerAllowlist map[string]struct{} Candidates []providers.FallbackCandidate // Router is non-nil when model routing is configured and the light model @@ -74,7 +76,9 @@ func NewAgentInstance( workspace := resolveAgentWorkspace(agentCfg, defaults) os.MkdirAll(workspace, 0o755) - model := resolveAgentModel(agentCfg, defaults) + definition := loadAgentDefinition(workspace) + + model := resolveAgentModel(agentCfg, defaults, definition) fallbacks := resolveAgentFallbacks(agentCfg, defaults) restrict := defaults.RestrictToWorkspace @@ -83,8 +87,11 @@ func NewAgentInstance( // Compile path whitelist patterns from config. allowReadPaths := buildAllowReadPatterns(cfg) allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) + agentToolAllowlist := resolveAgentToolAllowlist(definition) + agentMCPServerAllowlist := resolveAgentMCPServerAllowlist(definition) toolsRegistry := tools.NewToolRegistry() + toolsRegistry.SetAllowlist(agentToolAllowlist) if cfg.Tools.IsToolEnabled("read_file") { maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize @@ -121,7 +128,7 @@ func NewAgentInstance( sessionsDir := filepath.Join(workspace, "sessions") sessions := initSessionStore(sessionsDir) - mcpDiscoveryActive := cfg.Tools.MCP.Enabled && cfg.Tools.MCP.Discovery.Enabled + mcpDiscoveryActive := agentHasDiscoverableMCPServers(cfg, agentMCPServerAllowlist) contextBuilder := NewContextBuilder(workspace). WithToolDiscovery( mcpDiscoveryActive && cfg.Tools.MCP.Discovery.UseBM25, @@ -137,9 +144,14 @@ func NewAgentInstance( if agentCfg != nil { agentID = routing.NormalizeAgentID(agentCfg.ID) agentName = agentCfg.Name + if definition.Agent != nil && strings.TrimSpace(definition.Agent.Frontmatter.Name) != "" { + agentName = strings.TrimSpace(definition.Agent.Frontmatter.Name) + } subagents = agentCfg.Subagents - skillsFilter = agentCfg.Skills + skillsFilter = resolveAgentSkillsFilter(agentCfg, definition) } + provider = resolvePrimaryProviderForAgent(cfg, workspace, agentID, model, provider) + warnOnUnknownAgentMCPServerDeclarations(agentID, workspace, cfg, definition) maxIter := defaults.MaxToolIterations if maxIter == 0 { @@ -199,8 +211,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 { @@ -239,8 +258,10 @@ func NewAgentInstance( Sessions: sessions, ContextBuilder: contextBuilder, Tools: toolsRegistry, + Definition: definition, Subagents: subagents, SkillsFilter: skillsFilter, + MCPServerAllowlist: agentMCPServerAllowlist, Candidates: candidates, Router: router, LightCandidates: lightCandidates, @@ -285,13 +306,55 @@ func populateCandidateProvidersFromNames( } } +// resolvePrimaryProviderForAgent resolves a dedicated provider for the active +// primary model when the model points at a model_list entry. This keeps the +// agent's single-candidate path aligned with the selected model's own +// provider/api_base/api_key instead of inheriting the process default provider. +func resolvePrimaryProviderForAgent( + cfg *config.Config, + workspace string, + agentID string, + model string, + fallback providers.LLMProvider, +) providers.LLMProvider { + model = strings.TrimSpace(model) + if cfg == nil || model == "" { + return fallback + } + + modelCfg := lookupModelConfigByRef(cfg, model) + if modelCfg == nil { + return fallback + } + clone := *modelCfg + if clone.Workspace == "" { + clone.Workspace = workspace + } + + resolvedProvider, _, err := providers.CreateProviderFromConfig(&clone) + if err != nil { + logger.WarnCF("agent", "Primary model provider init failed; using injected provider", + map[string]any{ + "agent_id": agentID, + "model": model, + "error": err.Error(), + }) + return fallback + } + if resolvedProvider == nil { + return fallback + } + return resolvedProvider +} + // resolveAgentWorkspace determines the workspace directory for an agent. func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) string { if agentCfg != nil && strings.TrimSpace(agentCfg.Workspace) != "" { 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 @@ -300,7 +363,14 @@ func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentD } // resolveAgentModel resolves the primary model for an agent. -func resolveAgentModel(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) string { +func resolveAgentModel( + agentCfg *config.AgentConfig, + defaults *config.AgentDefaults, + definition AgentContextDefinition, +) string { + if definition.Agent != nil && strings.TrimSpace(definition.Agent.Frontmatter.Model) != "" { + return strings.TrimSpace(definition.Agent.Frontmatter.Model) + } if agentCfg != nil && agentCfg.Model != nil && strings.TrimSpace(agentCfg.Model.Primary) != "" { return strings.TrimSpace(agentCfg.Model.Primary) } @@ -315,6 +385,27 @@ func resolveAgentFallbacks(agentCfg *config.AgentConfig, defaults *config.AgentD return defaults.ModelFallbacks } +func resolveAgentSkillsFilter( + agentCfg *config.AgentConfig, + definition AgentContextDefinition, +) []string { + if definition.Agent != nil && definition.Agent.Frontmatter.Skills != nil { + return append([]string(nil), definition.Agent.Frontmatter.Skills...) + } + if agentCfg == nil || agentCfg.Skills == nil { + return nil + } + return append([]string(nil), agentCfg.Skills...) +} + +func (a *AgentInstance) AllowsMCPServer(serverName string) bool { + if a == nil || a.MCPServerAllowlist == nil { + return true + } + _, ok := a.MCPServerAllowlist[strings.ToLower(strings.TrimSpace(serverName))] + return ok +} + func compilePatterns(patterns []string) []*regexp.Regexp { compiled := make([]*regexp.Regexp, 0, len(patterns)) for _, p := range patterns { diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index 42bb53d86..dff2c0f2f 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -10,6 +10,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/media" "github.com/sipeed/picoclaw/pkg/providers" + "github.com/sipeed/picoclaw/pkg/tools" ) func TestNewAgentInstance_UsesDefaultsTemperatureAndMaxTokens(t *testing.T) { @@ -616,3 +617,285 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) { t.Fatal("read_file tool should still be registered") } } + +func TestNewAgentInstance_UsesFrontmatterModelAndSkills(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +model: frontmatter-model +skills: [frontmatter-skill] +mcpServers: [GitHub, filesystem] +--- +# Agent + +Use frontmatter identity. +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "default-model", + }, + }, + } + + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + Model: &config.AgentModelConfig{ + Primary: "config-model", + }, + Skills: []string{"config-skill"}, + }, &cfg.Agents.Defaults, cfg, &mockProvider{}) + + if agent.Model != "frontmatter-model" { + t.Fatalf("agent.Model = %q, want frontmatter-model", agent.Model) + } + if len(agent.SkillsFilter) != 1 || agent.SkillsFilter[0] != "frontmatter-skill" { + t.Fatalf("agent.SkillsFilter = %v, want [frontmatter-skill]", agent.SkillsFilter) + } + if !agent.AllowsMCPServer("github") { + t.Fatal("expected github MCP server to be allowed from frontmatter") + } + if !agent.AllowsMCPServer("FILESYSTEM") { + t.Fatal("expected filesystem MCP server matching to be case-insensitive") + } + if agent.AllowsMCPServer("slack") { + t.Fatal("expected slack MCP server to be blocked by frontmatter allowlist") + } +} + +func TestNewAgentInstance_UsesResolvedProviderForFrontmatterPrimaryModel(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +model: claude-frontmatter +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + Provider: "openai", + ModelName: "default-model", + }, + }, + ModelList: []*config.ModelConfig{ + { + ModelName: "claude-frontmatter", + Model: "anthropic/claude-3-7-sonnet", + APIKeys: config.SimpleSecureStrings("test-anthropic-key"), + Workspace: workspace, + }, + }, + } + + defaultProvider := &mockProvider{} + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + }, &cfg.Agents.Defaults, cfg, defaultProvider) + + if agent.Model != "claude-frontmatter" { + t.Fatalf("agent.Model = %q, want %q", agent.Model, "claude-frontmatter") + } + if len(agent.Candidates) != 1 { + t.Fatalf("len(agent.Candidates) = %d, want 1", len(agent.Candidates)) + } + if got := agent.Candidates[0].Provider; got != "anthropic" { + t.Fatalf("primary candidate provider = %q, want %q", got, "anthropic") + } + if got := agent.Candidates[0].Model; got != "claude-3-7-sonnet" { + t.Fatalf("primary candidate model = %q, want %q", got, "claude-3-7-sonnet") + } + if agent.Provider == defaultProvider { + t.Fatal("expected primary provider to be resolved from model_list instead of using injected default provider") + } +} + +func TestNewAgentInstance_SuppressesToolDiscoveryPromptWhenNoMCPServersSelected(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +mcpServers: [] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "default-model", + }, + }, + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + ToolConfig: config.ToolConfig{Enabled: true}, + Discovery: config.ToolDiscoveryConfig{ + Enabled: true, + UseBM25: true, + UseRegex: false, + }, + Servers: map[string]config.MCPServerConfig{ + "github": {Enabled: true}, + }, + }, + }, + } + + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + }, &cfg.Agents.Defaults, cfg, &mockProvider{}) + + if agent.AllowsMCPServer("github") { + t.Fatal("expected empty mcpServers allowlist to deny all servers") + } + messages := agent.ContextBuilder.BuildMessagesFromPrompt(PromptBuildRequest{CurrentMessage: "hello"}) + if prompt := messages[0].Content; strings.Contains(prompt, tools.BM25SearchToolName) { + t.Fatalf("expected no tool discovery prompt when no MCP servers are selected, got %q", prompt) + } +} + +func TestNewAgentInstance_IncludesToolDiscoveryPromptWhenDiscoverableMCPServerSelected(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +mcpServers: [github] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "default-model", + }, + }, + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + ToolConfig: config.ToolConfig{Enabled: true}, + Discovery: config.ToolDiscoveryConfig{ + Enabled: true, + UseBM25: true, + UseRegex: false, + }, + Servers: map[string]config.MCPServerConfig{ + "github": {Enabled: true}, + }, + }, + }, + } + + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + }, &cfg.Agents.Defaults, cfg, &mockProvider{}) + + messages := agent.ContextBuilder.BuildMessagesFromPrompt(PromptBuildRequest{CurrentMessage: "hello"}) + if prompt := messages[0].Content; !strings.Contains(prompt, tools.BM25SearchToolName) { + t.Fatalf("expected tool discovery prompt when a discoverable MCP server is selected, got %q", prompt) + } +} + +func TestNewAgentInstance_InvalidFrontmatterFailsClosedForToolsAndMCPServers(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [read_file +mcpServers: [github] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "default-model", + }, + }, + Tools: config.ToolsConfig{ + ReadFile: config.ReadFileToolConfig{Enabled: true}, + }, + } + + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + }, &cfg.Agents.Defaults, cfg, &mockProvider{}) + + if _, ok := agent.Tools.Get("read_file"); ok { + t.Fatal("expected malformed frontmatter to fail closed and block read_file") + } + if agent.AllowsMCPServer("github") { + t.Fatal("expected malformed frontmatter to fail closed for MCP servers") + } +} + +func TestNewAgentInstance_ExplicitEmptyToolsFieldBlocksAllTools(t *testing.T) { + tests := []struct { + name string + toolsSnippet string + }{ + { + name: "empty list", + toolsSnippet: "tools: []", + }, + { + name: "blank field", + toolsSnippet: "tools:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +` + tt.toolsSnippet + ` +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "default-model", + }, + }, + Tools: config.ToolsConfig{ + ReadFile: config.ReadFileToolConfig{Enabled: true}, + ListDir: config.ToolConfig{Enabled: true}, + }, + } + + agent := NewAgentInstance(&config.AgentConfig{ + ID: "research", + Workspace: workspace, + }, &cfg.Agents.Defaults, cfg, &mockProvider{}) + + if got := agent.Tools.List(); len(got) != 0 { + t.Fatalf("agent tools = %v, want no registered tools", got) + } + if _, ok := agent.Tools.Get("read_file"); ok { + t.Fatal("expected read_file to be blocked by explicit empty tools field") + } + if _, ok := agent.Tools.Get("list_dir"); ok { + t.Fatal("expected list_dir to be blocked by explicit empty tools field") + } + }) + } +} diff --git a/pkg/agent/prompt.go b/pkg/agent/prompt.go index be5ccddf2..02c850360 100644 --- a/pkg/agent/prompt.go +++ b/pkg/agent/prompt.go @@ -52,6 +52,7 @@ const ( PromptSourceMemory PromptSourceID = "memory:workspace" PromptSourceSkillCatalog PromptSourceID = "skill:index" PromptSourceActiveSkills PromptSourceID = "skill:active" + PromptSourceAgentDiscovery PromptSourceID = "agent:discovery" PromptSourceToolRegistry PromptSourceID = "tool_registry:native" PromptSourceToolDiscovery PromptSourceID = "tool_registry:discovery" PromptSourceOutputPolicy PromptSourceID = "runtime.output" @@ -195,6 +196,13 @@ func builtinPromptSources() []PromptSourceDescriptor { Allowed: []PromptPlacement{{Layer: PromptLayerCapability, Slot: PromptSlotActiveSkill}}, StableByDefault: false, }, + { + ID: PromptSourceAgentDiscovery, + Owner: "agent", + Description: "Structured multi-agent discovery registry", + Allowed: []PromptPlacement{{Layer: PromptLayerCapability, Slot: PromptSlotTooling}}, + StableByDefault: false, + }, { ID: PromptSourceMemory, Owner: "memory", diff --git a/pkg/agent/prompt_contributors.go b/pkg/agent/prompt_contributors.go index 960572e03..d6a2c09ec 100644 --- a/pkg/agent/prompt_contributors.go +++ b/pkg/agent/prompt_contributors.go @@ -93,6 +93,47 @@ func (c mcpServerPromptContributor) ContributePrompt( }, nil } +type agentDiscoveryPromptContributor struct { + agentID string + discover func(agentID string) []AgentDescriptor +} + +func (c agentDiscoveryPromptContributor) PromptSource() PromptSourceDescriptor { + return PromptSourceDescriptor{ + ID: PromptSourceAgentDiscovery, + Owner: "agent", + Description: "Structured multi-agent discovery registry", + Allowed: []PromptPlacement{{Layer: PromptLayerCapability, Slot: PromptSlotTooling}}, + StableByDefault: false, + } +} + +func (c agentDiscoveryPromptContributor) ContributePrompt( + _ context.Context, + _ PromptBuildRequest, +) ([]PromptPart, error) { + if c.discover == nil { + return nil, nil + } + content := formatAgentDiscoverySection(c.discover(c.agentID)) + if strings.TrimSpace(content) == "" { + return nil, nil + } + + return []PromptPart{ + { + ID: "capability.agent_discovery", + Layer: PromptLayerCapability, + Slot: PromptSlotTooling, + Source: PromptSource{ID: PromptSourceAgentDiscovery, Name: "agent:discovery"}, + Title: "agent discovery", + Content: content, + Stable: false, + Cache: PromptCacheNone, + }, + }, nil +} + func mcpPromptSourceID(serverName string) PromptSourceID { return PromptSourceID("mcp:" + promptSourceComponent(serverName)) } diff --git a/pkg/agent/registry.go b/pkg/agent/registry.go index 8aa11e37b..821ad4187 100644 --- a/pkg/agent/registry.go +++ b/pkg/agent/registry.go @@ -13,6 +13,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 @@ -24,6 +25,7 @@ func NewAgentRegistry( provider providers.LLMProvider, ) *AgentRegistry { registry := &AgentRegistry{ + cfg: cfg, agents: make(map[string]*AgentInstance), resolver: routing.NewRouteResolver(cfg), } @@ -53,6 +55,12 @@ func NewAgentRegistry( } } + for _, instance := range registry.agents { + if instance.ContextBuilder != nil { + instance.ContextBuilder.WithAgentDiscovery(instance.ID, registry.ListSpawnableAgents) + } + } + return registry } @@ -81,16 +89,43 @@ func (r *AgentRegistry) ListAgentIDs() []string { return ids } +func (r *AgentRegistry) allowedMCPServers() map[string]struct{} { + r.mu.RLock() + defer r.mu.RUnlock() + + if len(r.agents) == 0 { + return nil + } + + union := make(map[string]struct{}) + for _, agent := range r.agents { + if agent == nil { + continue + } + if agent.MCPServerAllowlist == nil { + return nil + } + for serverName := range agent.MCPServerAllowlist { + union[serverName] = struct{}{} + } + } + + return union +} + // CanSpawnSubagent checks if parentAgentID is allowed to spawn targetAgentID. func (r *AgentRegistry) CanSpawnSubagent(parentAgentID, targetAgentID string) bool { parent, ok := r.GetAgent(parentAgentID) if !ok { return false } - if parent.Subagents == nil || parent.Subagents.AllowAgents == nil { + return agentAllowsSubagent(parent, routing.NormalizeAgentID(targetAgentID)) +} + +func agentAllowsSubagent(parent *AgentInstance, targetNorm string) bool { + if parent == nil || parent.Subagents == nil || parent.Subagents.AllowAgents == nil { return false } - targetNorm := routing.NormalizeAgentID(targetAgentID) for _, allowed := range parent.Subagents.AllowAgents { if allowed == "*" { return true @@ -102,6 +137,14 @@ func (r *AgentRegistry) CanSpawnSubagent(parentAgentID, targetAgentID string) bo return false } +func agentHasSpawnTool(agent *AgentInstance) bool { + if agent == nil || agent.Tools == nil { + return false + } + _, ok := agent.Tools.Get("spawn") + return ok +} + // ForEachTool calls fn for every tool registered under the given name // across all agents. This is useful for propagating dependencies (e.g. // MediaStore) to tools after registry construction. @@ -131,11 +174,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..62b2ea6eb 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,112 @@ 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) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": "# Agent\nMain agent.\n", + }) + defer cleanupWorkspace(t, mainWorkspace) + + researchWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [read_file, write_file, web_search, web_fetch, message] +skills: [deep-research] +--- +# Agent + +Research agent. +`, + }) + defer cleanupWorkspace(t, researchWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Workspace: mainWorkspace}, + { + ID: "research", + Workspace: researchWorkspace, + }, + }) + cfg.Agents.Defaults.Workspace = mainWorkspace + 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) { + mainWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": "# Agent\nMain agent.\n", + }) + defer cleanupWorkspace(t, mainWorkspace) + + researchWorkspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [web] +--- +# Agent + +Research agent. +`, + }) + defer cleanupWorkspace(t, researchWorkspace) + + cfg := testCfg([]config.AgentConfig{ + {ID: "main", Default: true, Workspace: mainWorkspace}, + { + ID: "research", + Workspace: researchWorkspace, + }, + }) + cfg.Agents.Defaults.Workspace = mainWorkspace + 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/tool_allowlist.go b/pkg/agent/tool_allowlist.go new file mode 100644 index 000000000..962f7ec05 --- /dev/null +++ b/pkg/agent/tool_allowlist.go @@ -0,0 +1,203 @@ +package agent + +import ( + "sort" + "strings" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/tools" +) + +const dynamicMCPToolPrefix = "mcp_" + +func normalizeMCPServerName(name string) string { + return strings.ToLower(strings.TrimSpace(name)) +} + +func normalizedMCPServerNameSet( + servers map[string]config.MCPServerConfig, +) map[string]struct{} { + normalized := make(map[string]struct{}, len(servers)) + for serverName := range servers { + name := normalizeMCPServerName(serverName) + if name == "" { + continue + } + normalized[name] = struct{}{} + } + return normalized +} + +func warnOnUnknownAgentToolDeclarations( + agentID, workspace string, + definition AgentContextDefinition, + registry *tools.ToolRegistry, +) { + if registry == nil || frontmatterParseFailed(definition) { + return + } + + if unknownTools := unknownAgentToolNames(registry, definition); len(unknownTools) > 0 { + logger.WarnCF("agent", "AGENT.md declares unregistered tool names", + map[string]any{ + "agent_id": agentID, + "workspace": workspace, + "tools": unknownTools, + }) + } +} + +func warnOnUnknownAgentMCPServerDeclarations( + agentID, workspace string, + cfg *config.Config, + definition AgentContextDefinition, +) { + if cfg == nil || frontmatterParseFailed(definition) { + return + } + + if unknownServers := unknownAgentMCPServerNames(cfg, definition); len(unknownServers) > 0 { + logger.WarnCF("agent", "AGENT.md declares unknown MCP server names", + map[string]any{ + "agent_id": agentID, + "workspace": workspace, + "mcp_servers": unknownServers, + }) + } +} + +func unknownAgentToolNames( + registry *tools.ToolRegistry, + definition AgentContextDefinition, +) []string { + if definition.Agent == nil || definition.Agent.Frontmatter.Tools == nil { + return nil + } + + known := registeredRuntimeToolNames(registry) + unknown := make(map[string]struct{}) + for _, raw := range definition.Agent.Frontmatter.Tools { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" || strings.HasPrefix(name, dynamicMCPToolPrefix) { + continue + } + if _, ok := known[name]; ok { + continue + } + unknown[name] = struct{}{} + } + + return sortedKeys(unknown) +} + +func registeredRuntimeToolNames(registry *tools.ToolRegistry) map[string]struct{} { + known := make(map[string]struct{}) + if registry == nil { + return known + } + for _, raw := range registry.List() { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" { + continue + } + known[name] = struct{}{} + } + return known +} + +func unknownAgentMCPServerNames(cfg *config.Config, definition AgentContextDefinition) []string { + if cfg == nil || definition.Agent == nil || definition.Agent.Frontmatter.MCPServers == nil { + return nil + } + + knownServers := normalizedMCPServerNameSet(cfg.Tools.MCP.Servers) + unknown := make(map[string]struct{}) + for _, raw := range definition.Agent.Frontmatter.MCPServers { + name := normalizeMCPServerName(raw) + if name == "" { + continue + } + if _, ok := knownServers[name]; ok { + continue + } + unknown[name] = struct{}{} + } + + return sortedKeys(unknown) +} + +func sortedKeys(values map[string]struct{}) []string { + if len(values) == 0 { + return nil + } + + result := make([]string, 0, len(values)) + for value := range values { + result = append(result, value) + } + sort.Strings(result) + return result +} + +func resolveAgentToolAllowlist(definition AgentContextDefinition) []string { + if frontmatterParseFailed(definition) { + return []string{} + } + if definition.Agent == nil || !frontmatterDeclaresField(definition, "tools") { + return nil + } + + allowlist := make(map[string]struct{}, len(definition.Agent.Frontmatter.Tools)) + for _, raw := range definition.Agent.Frontmatter.Tools { + trimmed := strings.ToLower(strings.TrimSpace(raw)) + if trimmed == "" { + continue + } + allowlist[trimmed] = struct{}{} + } + + if len(allowlist) == 0 { + return []string{} + } + + return sortedKeys(allowlist) +} + +func resolveAgentMCPServerAllowlist(definition AgentContextDefinition) map[string]struct{} { + if frontmatterParseFailed(definition) { + return map[string]struct{}{} + } + if definition.Agent == nil || !frontmatterDeclaresField(definition, "mcpServers") { + return nil + } + + allowlist := make(map[string]struct{}, len(definition.Agent.Frontmatter.MCPServers)) + for _, raw := range definition.Agent.Frontmatter.MCPServers { + trimmed := strings.ToLower(strings.TrimSpace(raw)) + if trimmed == "" { + continue + } + allowlist[trimmed] = struct{}{} + } + + return allowlist +} + +func frontmatterDeclaresField(definition AgentContextDefinition, field string) bool { + if definition.Agent == nil || definition.Agent.Frontmatter.Fields == nil { + return false + } + _, ok := definition.Agent.Frontmatter.Fields[field] + return ok +} + +func frontmatterParseFailed(definition AgentContextDefinition) bool { + if definition.Agent == nil { + return false + } + if strings.TrimSpace(definition.Agent.RawFrontmatter) == "" { + return false + } + return strings.TrimSpace(definition.Agent.FrontmatterErr) != "" +} diff --git a/pkg/agent/tool_allowlist_test.go b/pkg/agent/tool_allowlist_test.go new file mode 100644 index 000000000..5ed35d4c6 --- /dev/null +++ b/pkg/agent/tool_allowlist_test.go @@ -0,0 +1,184 @@ +package agent + +import ( + "context" + "testing" + + "github.com/sipeed/picoclaw/pkg/config" + agenttools "github.com/sipeed/picoclaw/pkg/tools" +) + +type allowlistTestTool struct { + name string +} + +func (t *allowlistTestTool) Name() string { return t.name } + +func (t *allowlistTestTool) Description() string { return "test tool" } + +func (t *allowlistTestTool) Parameters() map[string]any { + return map[string]any{"type": "object"} +} + +func (t *allowlistTestTool) Execute( + _ context.Context, + _ map[string]any, +) *agenttools.ToolResult { + return agenttools.NewToolResult("ok") +} + +func TestUnknownAgentToolNames(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [read_file, web_serach, mcp_github_search] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + registry := agenttools.NewToolRegistry() + registry.Register(&allowlistTestTool{name: "read_file"}) + registry.Register(&allowlistTestTool{name: "web_search"}) + + unknown := unknownAgentToolNames(registry, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "web_serach" { + t.Fatalf("unknownAgentToolNames() = %v, want [web_serach]", unknown) + } +} + +func TestUnknownAgentToolNamesUsesRegisteredRuntimeTools(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +tools: [serial, reaction, send_tts, load_image, delegate, made_up] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + registry := agenttools.NewToolRegistry() + for _, name := range []string{"serial", "reaction", "send_tts", "load_image", "delegate"} { + registry.Register(&allowlistTestTool{name: name}) + } + + unknown := unknownAgentToolNames(registry, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "made_up" { + t.Fatalf("unknownAgentToolNames() = %v, want [made_up]", unknown) + } +} + +func TestResolveAgentToolAllowlistDistinguishesMissingAndEmptyToolsField(t *testing.T) { + tests := []struct { + name string + agentMD string + wantNil bool + wantEmpty bool + }{ + { + name: "missing tools field allows all tools", + agentMD: `--- +name: pico +--- +# Agent +`, + wantNil: true, + }, + { + name: "explicit empty tools list blocks all tools", + agentMD: `--- +tools: [] +--- +# Agent +`, + wantEmpty: true, + }, + { + name: "blank tools field blocks all tools", + agentMD: `--- +tools: +--- +# Agent +`, + wantEmpty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": tt.agentMD, + }) + defer cleanupWorkspace(t, workspace) + + allowlist := resolveAgentToolAllowlist(loadAgentDefinition(workspace)) + + if tt.wantNil { + if allowlist != nil { + t.Fatalf("resolveAgentToolAllowlist() = %v, want nil", allowlist) + } + return + } + + if allowlist == nil { + t.Fatal("resolveAgentToolAllowlist() = nil, want explicit empty allowlist") + } + if len(allowlist) != 0 { + t.Fatalf("resolveAgentToolAllowlist() = %v, want empty allowlist", allowlist) + } + }) + } +} + +func TestUnknownAgentMCPServerNames(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +mcpServers: [github, githb] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + Servers: map[string]config.MCPServerConfig{ + "github": {Enabled: true}, + }, + }, + }, + } + + unknown := unknownAgentMCPServerNames(cfg, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "githb" { + t.Fatalf("unknownAgentMCPServerNames() = %v, want [githb]", unknown) + } +} + +func TestUnknownAgentMCPServerNamesMatchesConfigCaseInsensitively(t *testing.T) { + workspace := setupWorkspace(t, map[string]string{ + "AGENT.md": `--- +mcpServers: [github, FileSystem, slak] +--- +# Agent +`, + }) + defer cleanupWorkspace(t, workspace) + + cfg := &config.Config{ + Tools: config.ToolsConfig{ + MCP: config.MCPConfig{ + Servers: map[string]config.MCPServerConfig{ + "GitHub": {Enabled: true}, + "filesystem": {Enabled: true}, + }, + }, + }, + } + + unknown := unknownAgentMCPServerNames(cfg, loadAgentDefinition(workspace)) + if len(unknown) != 1 || unknown[0] != "slak" { + t.Fatalf("unknownAgentMCPServerNames() = %v, want [slak]", unknown) + } +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4f1c5c5e8..fc810ad7d 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -96,17 +96,17 @@ 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"] + }, + "subagents": { + "allow_agents": ["sales"] } + } ] }, "session": { @@ -808,7 +808,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", + ) } if cfg.Agents.Defaults.ToolFeedback.SeparateMessages { t.Fatal("agents.defaults.tool_feedback.separate_messages should remain false when unset in config file") @@ -1131,7 +1133,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) @@ -1173,7 +1178,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, + ) } } @@ -1283,7 +1292,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 } @@ -1592,9 +1606,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 { @@ -1731,7 +1757,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, + ) } } @@ -2140,9 +2169,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")}, }, diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index 0ff9293a3..e90d683bb 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.ToLower(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,30 @@ func (r *ToolRegistry) Version() uint64 { return r.version.Load() } +func (r *ToolRegistry) toolAllowedLocked(name string) bool { + if r.allowlist == nil { + return true + } + if isToolDiscoveryToolName(name) { + // Discovery tools are part of the MCP control plane: they must remain + // available whenever configured so deferred MCP tools can still be + // unlocked. Per-agent allowlists still apply to the hidden MCP tools + // themselves during RegisterHidden. + return true + } + _, ok := r.allowlist[strings.ToLower(strings.TrimSpace(name))] + return ok +} + +// HasRegistered reports whether a tool name is present in the registry, +// including hidden tools whose TTL is currently zero. +func (r *ToolRegistry) HasRegistered(name string) bool { + r.mu.RLock() + defer r.mu.RUnlock() + _, ok := r.tools[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 +267,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. @@ -411,6 +477,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, @@ -443,7 +515,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 eac96382f..ee63586ab 100644 --- a/pkg/tools/registry_test.go +++ b/pkg/tools/registry_test.go @@ -53,7 +53,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 } @@ -104,6 +108,69 @@ 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_AllowlistStillAllowsDiscoveryTools(t *testing.T) { + r := NewToolRegistry() + r.SetAllowlist([]string{"mcp_github_search"}) + + r.Register(newMockTool(BM25SearchToolName, "discover hidden tools")) + r.Register(newMockTool(RegexSearchToolName, "discover hidden tools via regex")) + r.Register(newMockTool("blocked_tool", "blocked")) + + if _, ok := r.Get(BM25SearchToolName); !ok { + t.Fatal("expected BM25 discovery tool to bypass allowlist filtering") + } + if _, ok := r.Get(RegexSearchToolName); !ok { + t.Fatal("expected regex discovery tool to bypass allowlist filtering") + } + if _, ok := r.Get("blocked_tool"); ok { + t.Fatal("blocked_tool should not be registered") + } +} + +func TestToolRegistry_HasRegisteredIncludesHiddenTools(t *testing.T) { + r := NewToolRegistry() + r.SetAllowlist([]string{"visible", "hidden"}) + + r.Register(newMockTool("visible", "visible")) + r.RegisterHidden(newMockTool("hidden", "hidden")) + r.RegisterHidden(newMockTool("blocked", "blocked")) + + if !r.HasRegistered("visible") { + t.Fatal("expected visible tool to be registered") + } + if !r.HasRegistered("hidden") { + t.Fatal("expected hidden tool to be reported as registered") + } + if r.HasRegistered("blocked") { + t.Fatal("blocked tool should not be registered") + } + if _, ok := r.Get("hidden"); ok { + t.Fatal("hidden tool with zero TTL should not be callable through Get") + } +} + func TestToolRegistry_Get_NotFound(t *testing.T) { r := NewToolRegistry() _, ok := r.Get("nonexistent") @@ -305,7 +372,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, + ) } } @@ -449,7 +520,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") @@ -745,7 +819,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) @@ -765,7 +846,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)) @@ -800,7 +888,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/search_tool.go b/pkg/tools/search_tool.go index c5884c9de..511b81a03 100644 --- a/pkg/tools/search_tool.go +++ b/pkg/tools/search_tool.go @@ -14,6 +14,8 @@ import ( const ( MaxRegexPatternLength = 200 + RegexSearchToolName = "tool_search_tool_regex" + BM25SearchToolName = "tool_search_tool_bm25" ) type RegexSearchTool struct { @@ -27,7 +29,7 @@ func NewRegexSearchTool(r *ToolRegistry, ttl int, maxSearchResults int) *RegexSe } func (t *RegexSearchTool) Name() string { - return "tool_search_tool_regex" + return RegexSearchToolName } func (t *RegexSearchTool) Description() string { @@ -96,7 +98,7 @@ func NewBM25SearchTool(r *ToolRegistry, ttl int, maxSearchResults int) *BM25Sear } func (t *BM25SearchTool) Name() string { - return "tool_search_tool_bm25" + return BM25SearchToolName } func (t *BM25SearchTool) Description() string { @@ -294,6 +296,15 @@ func (t *BM25SearchTool) getOrBuildEngine() *bm25CachedEngine { return cached } +func isToolDiscoveryToolName(name string) bool { + switch strings.ToLower(strings.TrimSpace(name)) { + case BM25SearchToolName, RegexSearchToolName: + return true + default: + return false + } +} + // SearchBM25 ranks hidden tools against query using BM25 via utils.BM25Engine. // This non-cached variant rebuilds the engine on every call. Used by tests // and any code that doesn't hold a BM25SearchTool instance. diff --git a/pkg/tools/spawn.go b/pkg/tools/spawn.go index d019d511a..a9a373856 100644 --- a/pkg/tools/spawn.go +++ b/pkg/tools/spawn.go @@ -92,11 +92,12 @@ func (t *SpawnTool) execute( label, _ := args["label"].(string) agentID, _ := args["agent_id"].(string) + targetAgentID := strings.TrimSpace(agentID) // Check allowlist if targeting a specific agent - if agentID != "" && t.allowlistCheck != nil { - if !t.allowlistCheck(agentID) { - return ErrorResult(fmt.Sprintf("not allowed to spawn agent '%s'", agentID)) + if targetAgentID != "" && t.allowlistCheck != nil { + if !t.allowlistCheck(targetAgentID) { + return ErrorResult(fmt.Sprintf("not allowed to spawn agent '%s'", targetAgentID)) } } @@ -123,12 +124,14 @@ Task: %s`, // Launch async sub-turn in goroutine go func() { result, err := t.spawner.SpawnSubTurn(ctx, SubTurnConfig{ - Model: t.defaultModel, - Tools: nil, // Will inherit from parent via context - SystemPrompt: systemPrompt, - MaxTokens: t.maxTokens, - Temperature: t.temperature, - Async: true, // Async execution + Model: t.defaultModel, + Tools: nil, // Will inherit from parent via context + SystemPrompt: systemPrompt, + MaxTokens: t.maxTokens, + Temperature: t.temperature, + Async: true, // Async execution + Critical: true, // Background spawn should survive parent turn completion + TargetAgentID: targetAgentID, }) if err != nil { result = ErrorResult(fmt.Sprintf("Spawn failed: %v", err)).WithError(err) diff --git a/pkg/tools/spawn_test.go b/pkg/tools/spawn_test.go index fda6bbd89..c91c79578 100644 --- a/pkg/tools/spawn_test.go +++ b/pkg/tools/spawn_test.go @@ -6,10 +6,18 @@ import ( "testing" ) -// mockSpawner implements SubTurnSpawner for testing -type mockSpawner struct{} +// mockSpawner implements SubTurnSpawner for testing. +type mockSpawner struct { + lastConfig SubTurnConfig + done chan struct{} +} func (m *mockSpawner) SpawnSubTurn(ctx context.Context, cfg SubTurnConfig) (*ToolResult, error) { + m.lastConfig = cfg + if m.done != nil { + close(m.done) + } + // Extract task from system prompt for response task := cfg.SystemPrompt if strings.Contains(task, "Task: ") { @@ -62,12 +70,14 @@ func TestSpawnTool_Execute_ValidTask(t *testing.T) { provider := &MockLLMProvider{} manager := NewSubagentManager(provider, "test-model", "/tmp/test") tool := NewSpawnTool(manager) - tool.SetSpawner(&mockSpawner{}) + spawner := &mockSpawner{done: make(chan struct{})} + tool.SetSpawner(spawner) ctx := context.Background() args := map[string]any{ - "task": "Write a haiku about coding", - "label": "haiku-task", + "task": "Write a haiku about coding", + "label": "haiku-task", + "agent_id": "research", } result := tool.Execute(ctx, args) @@ -80,6 +90,13 @@ func TestSpawnTool_Execute_ValidTask(t *testing.T) { if !result.Async { t.Error("SpawnTool should return async result") } + <-spawner.done + if spawner.lastConfig.TargetAgentID != "research" { + t.Errorf("TargetAgentID = %q, want research", spawner.lastConfig.TargetAgentID) + } + if !spawner.lastConfig.Critical { + t.Error("SpawnTool should mark background subturns as critical") + } } func TestSpawnTool_Execute_NilManager(t *testing.T) {