From b8f4257ceefbc798abd6d7fc86c18f11bb78e734 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Thu, 7 May 2026 18:26:09 +0200 Subject: [PATCH] fix(agent): filter discovery by spawn permissions --- docs/guides/configuration.it.md | 12 ++-- docs/guides/configuration.md | 12 ++-- pkg/agent/context.go | 9 +-- pkg/agent/discovery.go | 39 ++++++++++- pkg/agent/discovery_test.go | 117 +++++++++++++++++++++++++++++-- pkg/agent/prompt_contributors.go | 6 +- pkg/agent/registry.go | 9 ++- 7 files changed, 170 insertions(+), 34 deletions(-) diff --git a/docs/guides/configuration.it.md b/docs/guides/configuration.it.md index 4b0153e2c..d7de46895 100644 --- a/docs/guides/configuration.it.md +++ b/docs/guides/configuration.it.md @@ -98,7 +98,7 @@ Note: ### Discovery Multi-Agent (Automatica) -Quando esiste più di un agent, PicoClaw inietta automaticamente nel system prompt di ogni agent un registry strutturato dei peer. Non serve una chiamata aggiuntiva a un tool `list_agents`. +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. @@ -112,9 +112,10 @@ Ogni entry include: Dettagli importanti: -- La sezione include anche l'entry dell'agent corrente, quindi c'è self-awareness. +- 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. +- `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: @@ -122,11 +123,6 @@ Forma dell'oggetto iniettato: ```json { "agents": [ - { - "id": "main", - "name": "Main Assistant", - "description": "Agent generalista per richieste quotidiane." - }, { "id": "research", "name": "Research Agent", diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index ce0db46fe..4cbe9dd82 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -238,7 +238,7 @@ Notes: ### Agent Discovery (Automatic) -When more than one agent exists, PicoClaw injects a structured agent registry into each agent's system prompt on every turn. No extra `list_agents` tool call is required. +When an agent has spawnable peers, 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`. @@ -252,9 +252,10 @@ Each entry includes: Important behavior: -- The discovery section includes the current agent's own entry, so the model has self-awareness. +- The discovery section includes only peer agents the current agent 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. +- `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: @@ -262,11 +263,6 @@ Example injected shape: ```json { "agents": [ - { - "id": "main", - "name": "Main Assistant", - "description": "Generalist agent for day-to-day requests." - }, { "id": "research", "name": "Research Agent", diff --git a/pkg/agent/context.go b/pkg/agent/context.go index b5776b59c..7f5b32fef 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -26,7 +26,7 @@ type ContextBuilder struct { skillsLoader *skills.SkillsLoader memory *MemoryStore splitOnMarker bool - agentDiscovery func(workspace string) []AgentDescriptor + agentDiscovery func(agentID string) []AgentDescriptor promptRegistry *PromptRegistry // Cache for system prompt to avoid rebuilding on every call. @@ -68,13 +68,14 @@ func (cb *ContextBuilder) WithSplitOnMarker(enabled bool) *ContextBuilder { } func (cb *ContextBuilder) WithAgentDiscovery( - discover func(workspace string) []AgentDescriptor, + agentID string, + discover func(agentID string) []AgentDescriptor, ) *ContextBuilder { cb.agentDiscovery = discover if discover != nil { if err := cb.RegisterPromptContributor(agentDiscoveryPromptContributor{ - workspace: cb.workspace, - discover: discover, + agentID: agentID, + discover: discover, }); err != nil { logger.WarnCF("agent", "Failed to register agent discovery prompt contributor", map[string]any{ "error": err.Error(), diff --git a/pkg/agent/discovery.go b/pkg/agent/discovery.go index d08ed1880..8c1c5bb82 100644 --- a/pkg/agent/discovery.go +++ b/pkg/agent/discovery.go @@ -60,6 +60,41 @@ func (r *AgentRegistry) ListAgents(workspace string) []AgentDescriptor { return descriptors } +// ListSpawnableAgents returns descriptors only for agents the current agent 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 + } + + 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() @@ -195,7 +230,7 @@ func cleanWorkspacePath(path string) string { } func formatAgentDiscoverySection(agents []AgentDescriptor) string { - if len(agents) <= 1 { + if len(agents) == 0 { return "" } @@ -212,7 +247,7 @@ func formatAgentDiscoverySection(agents []AgentDescriptor) string { var header strings.Builder header.WriteString("# Agent Discovery\n\n") - header.WriteString("This registry is authoritative for the current PicoClaw instance.\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", ) diff --git a/pkg/agent/discovery_test.go b/pkg/agent/discovery_test.go index 28da55e25..bceee54d7 100644 --- a/pkg/agent/discovery_test.go +++ b/pkg/agent/discovery_test.go @@ -66,6 +66,30 @@ Handle support tickets carefully. } } +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"}, + }) + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + descriptors := registry.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 TestContextBuilder_BuildMessagesIncludesAgentDiscoverySection(t *testing.T) { mainWorkspace := setupWorkspace(t, map[string]string{ "AGENT.md": `--- @@ -90,9 +114,29 @@ 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}, + { + 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 @@ -121,13 +165,16 @@ Investigate deeply. if !strings.Contains(systemPrompt, "# Agent Discovery") { t.Fatalf("expected discovery section in system prompt, got %q", systemPrompt) } - if !strings.Contains(systemPrompt, `"id": "main"`) || - !strings.Contains(systemPrompt, `"id": "research"`) { - t.Fatalf("expected self and peer descriptors in discovery section, got %q", systemPrompt) + if strings.Contains(systemPrompt, `"id": "main"`) { + t.Fatalf("did not expect self descriptor in discovery section, got %q", systemPrompt) } - if !strings.Contains(systemPrompt, `"name": "main"`) || + if !strings.Contains(systemPrompt, `"id": "research"`) || !strings.Contains(systemPrompt, `"description": "Research specialist"`) { - t.Fatalf("expected minimal identity fields in discovery section, got %q", systemPrompt) + 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) { @@ -136,6 +183,64 @@ Investigate deeply. } } +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 + + registry := NewAgentRegistry(cfg, &mockRegistryProvider{}) + mainAgent, ok := registry.GetAgent("main") + if !ok || mainAgent == nil { + t.Fatal("expected main agent") + } + + messages := mainAgent.ContextBuilder.BuildMessages( + nil, + "", + "handle locally", + nil, + "telegram", + "chat-1", + "", + "", + ) + if len(messages) == 0 { + t.Fatal("expected messages") + } + + systemPrompt := messages[0].Content + if strings.Contains(systemPrompt, "# Agent Discovery") { + t.Fatalf("did not expect discovery section 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_BuildMessagesOmitsAgentDiscoverySectionForSingleton(t *testing.T) { mainWorkspace := setupWorkspace(t, map[string]string{ "AGENT.md": `--- diff --git a/pkg/agent/prompt_contributors.go b/pkg/agent/prompt_contributors.go index 863df57b2..d6a2c09ec 100644 --- a/pkg/agent/prompt_contributors.go +++ b/pkg/agent/prompt_contributors.go @@ -94,8 +94,8 @@ func (c mcpServerPromptContributor) ContributePrompt( } type agentDiscoveryPromptContributor struct { - workspace string - discover func(workspace string) []AgentDescriptor + agentID string + discover func(agentID string) []AgentDescriptor } func (c agentDiscoveryPromptContributor) PromptSource() PromptSourceDescriptor { @@ -115,7 +115,7 @@ func (c agentDiscoveryPromptContributor) ContributePrompt( if c.discover == nil { return nil, nil } - content := formatAgentDiscoverySection(c.discover(c.workspace)) + content := formatAgentDiscoverySection(c.discover(c.agentID)) if strings.TrimSpace(content) == "" { return nil, nil } diff --git a/pkg/agent/registry.go b/pkg/agent/registry.go index c6a1246ac..a4d1a860d 100644 --- a/pkg/agent/registry.go +++ b/pkg/agent/registry.go @@ -57,7 +57,7 @@ func NewAgentRegistry( for _, instance := range registry.agents { if instance.ContextBuilder != nil { - instance.ContextBuilder.WithAgentDiscovery(registry.ListAgents) + instance.ContextBuilder.WithAgentDiscovery(instance.ID, registry.ListSpawnableAgents) } } @@ -119,10 +119,13 @@ func (r *AgentRegistry) CanSpawnSubagent(parentAgentID, targetAgentID string) bo 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