fix(agent): filter discovery by spawn permissions
This commit is contained in:
parent
96fd887cad
commit
b8f4257cee
7 changed files with 170 additions and 34 deletions
|
|
@ -98,7 +98,7 @@ Note:
|
||||||
|
|
||||||
### Discovery Multi-Agent (Automatica)
|
### 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.
|
Questa discovery serve soprattutto a rendere affidabile la delega tramite `spawn` con `agent_id` esplicito.
|
||||||
|
|
||||||
|
|
@ -112,9 +112,10 @@ Ogni entry include:
|
||||||
|
|
||||||
Dettagli importanti:
|
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`.
|
- 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.
|
- `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:
|
Forma dell'oggetto iniettato:
|
||||||
|
|
@ -122,11 +123,6 @@ Forma dell'oggetto iniettato:
|
||||||
```json
|
```json
|
||||||
{
|
{
|
||||||
"agents": [
|
"agents": [
|
||||||
{
|
|
||||||
"id": "main",
|
|
||||||
"name": "Main Assistant",
|
|
||||||
"description": "Agent generalista per richieste quotidiane."
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "research",
|
"id": "research",
|
||||||
"name": "Research Agent",
|
"name": "Research Agent",
|
||||||
|
|
|
||||||
|
|
@ -238,7 +238,7 @@ Notes:
|
||||||
|
|
||||||
### Agent Discovery (Automatic)
|
### 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`.
|
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:
|
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`.
|
- 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.
|
- `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:
|
Example injected shape:
|
||||||
|
|
@ -262,11 +263,6 @@ Example injected shape:
|
||||||
```json
|
```json
|
||||||
{
|
{
|
||||||
"agents": [
|
"agents": [
|
||||||
{
|
|
||||||
"id": "main",
|
|
||||||
"name": "Main Assistant",
|
|
||||||
"description": "Generalist agent for day-to-day requests."
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
"id": "research",
|
"id": "research",
|
||||||
"name": "Research Agent",
|
"name": "Research Agent",
|
||||||
|
|
|
||||||
|
|
@ -26,7 +26,7 @@ type ContextBuilder struct {
|
||||||
skillsLoader *skills.SkillsLoader
|
skillsLoader *skills.SkillsLoader
|
||||||
memory *MemoryStore
|
memory *MemoryStore
|
||||||
splitOnMarker bool
|
splitOnMarker bool
|
||||||
agentDiscovery func(workspace string) []AgentDescriptor
|
agentDiscovery func(agentID string) []AgentDescriptor
|
||||||
promptRegistry *PromptRegistry
|
promptRegistry *PromptRegistry
|
||||||
|
|
||||||
// Cache for system prompt to avoid rebuilding on every call.
|
// 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(
|
func (cb *ContextBuilder) WithAgentDiscovery(
|
||||||
discover func(workspace string) []AgentDescriptor,
|
agentID string,
|
||||||
|
discover func(agentID string) []AgentDescriptor,
|
||||||
) *ContextBuilder {
|
) *ContextBuilder {
|
||||||
cb.agentDiscovery = discover
|
cb.agentDiscovery = discover
|
||||||
if discover != nil {
|
if discover != nil {
|
||||||
if err := cb.RegisterPromptContributor(agentDiscoveryPromptContributor{
|
if err := cb.RegisterPromptContributor(agentDiscoveryPromptContributor{
|
||||||
workspace: cb.workspace,
|
agentID: agentID,
|
||||||
discover: discover,
|
discover: discover,
|
||||||
}); err != nil {
|
}); err != nil {
|
||||||
logger.WarnCF("agent", "Failed to register agent discovery prompt contributor", map[string]any{
|
logger.WarnCF("agent", "Failed to register agent discovery prompt contributor", map[string]any{
|
||||||
"error": err.Error(),
|
"error": err.Error(),
|
||||||
|
|
|
||||||
|
|
@ -60,6 +60,41 @@ func (r *AgentRegistry) ListAgents(workspace string) []AgentDescriptor {
|
||||||
return descriptors
|
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.
|
// GetAgentDescriptor returns the structured discovery payload for one agent.
|
||||||
func (r *AgentRegistry) GetAgentDescriptor(agentID string) (*AgentDescriptor, bool) {
|
func (r *AgentRegistry) GetAgentDescriptor(agentID string) (*AgentDescriptor, bool) {
|
||||||
r.mu.RLock()
|
r.mu.RLock()
|
||||||
|
|
@ -195,7 +230,7 @@ func cleanWorkspacePath(path string) string {
|
||||||
}
|
}
|
||||||
|
|
||||||
func formatAgentDiscoverySection(agents []AgentDescriptor) string {
|
func formatAgentDiscoverySection(agents []AgentDescriptor) string {
|
||||||
if len(agents) <= 1 {
|
if len(agents) == 0 {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -212,7 +247,7 @@ func formatAgentDiscoverySection(agents []AgentDescriptor) string {
|
||||||
|
|
||||||
var header strings.Builder
|
var header strings.Builder
|
||||||
header.WriteString("# Agent Discovery\n\n")
|
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(
|
header.WriteString(
|
||||||
"Choose a peer based on its description. Use only agent IDs listed here when calling spawn.\n\n",
|
"Choose a peer based on its description. Use only agent IDs listed here when calling spawn.\n\n",
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -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) {
|
func TestContextBuilder_BuildMessagesIncludesAgentDiscoverySection(t *testing.T) {
|
||||||
mainWorkspace := setupWorkspace(t, map[string]string{
|
mainWorkspace := setupWorkspace(t, map[string]string{
|
||||||
"AGENT.md": `---
|
"AGENT.md": `---
|
||||||
|
|
@ -90,9 +114,29 @@ Investigate deeply.
|
||||||
})
|
})
|
||||||
defer cleanupWorkspace(t, researchWorkspace)
|
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{
|
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: "research", Workspace: researchWorkspace},
|
||||||
|
{ID: "restricted", Workspace: restrictedWorkspace},
|
||||||
})
|
})
|
||||||
cfg.Tools.ReadFile.Enabled = true
|
cfg.Tools.ReadFile.Enabled = true
|
||||||
cfg.Tools.WriteFile.Enabled = true
|
cfg.Tools.WriteFile.Enabled = true
|
||||||
|
|
@ -121,13 +165,16 @@ Investigate deeply.
|
||||||
if !strings.Contains(systemPrompt, "# Agent Discovery") {
|
if !strings.Contains(systemPrompt, "# Agent Discovery") {
|
||||||
t.Fatalf("expected discovery section in system prompt, got %q", systemPrompt)
|
t.Fatalf("expected discovery section in system prompt, got %q", systemPrompt)
|
||||||
}
|
}
|
||||||
if !strings.Contains(systemPrompt, `"id": "main"`) ||
|
if strings.Contains(systemPrompt, `"id": "main"`) {
|
||||||
!strings.Contains(systemPrompt, `"id": "research"`) {
|
t.Fatalf("did not expect self descriptor in discovery section, got %q", systemPrompt)
|
||||||
t.Fatalf("expected self and peer descriptors in discovery section, got %q", systemPrompt)
|
|
||||||
}
|
}
|
||||||
if !strings.Contains(systemPrompt, `"name": "main"`) ||
|
if !strings.Contains(systemPrompt, `"id": "research"`) ||
|
||||||
!strings.Contains(systemPrompt, `"description": "Research specialist"`) {
|
!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"`} {
|
for _, forbidden := range []string{`"current_agent_id"`, `"available_tools"`, `"model"`, `"channels"`, `"skills"`, `"mcpServers"`, `"tools"`} {
|
||||||
if strings.Contains(systemPrompt, forbidden) {
|
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) {
|
func TestContextBuilder_BuildMessagesOmitsAgentDiscoverySectionForSingleton(t *testing.T) {
|
||||||
mainWorkspace := setupWorkspace(t, map[string]string{
|
mainWorkspace := setupWorkspace(t, map[string]string{
|
||||||
"AGENT.md": `---
|
"AGENT.md": `---
|
||||||
|
|
|
||||||
|
|
@ -94,8 +94,8 @@ func (c mcpServerPromptContributor) ContributePrompt(
|
||||||
}
|
}
|
||||||
|
|
||||||
type agentDiscoveryPromptContributor struct {
|
type agentDiscoveryPromptContributor struct {
|
||||||
workspace string
|
agentID string
|
||||||
discover func(workspace string) []AgentDescriptor
|
discover func(agentID string) []AgentDescriptor
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c agentDiscoveryPromptContributor) PromptSource() PromptSourceDescriptor {
|
func (c agentDiscoveryPromptContributor) PromptSource() PromptSourceDescriptor {
|
||||||
|
|
@ -115,7 +115,7 @@ func (c agentDiscoveryPromptContributor) ContributePrompt(
|
||||||
if c.discover == nil {
|
if c.discover == nil {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
content := formatAgentDiscoverySection(c.discover(c.workspace))
|
content := formatAgentDiscoverySection(c.discover(c.agentID))
|
||||||
if strings.TrimSpace(content) == "" {
|
if strings.TrimSpace(content) == "" {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -57,7 +57,7 @@ func NewAgentRegistry(
|
||||||
|
|
||||||
for _, instance := range registry.agents {
|
for _, instance := range registry.agents {
|
||||||
if instance.ContextBuilder != nil {
|
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 {
|
if !ok {
|
||||||
return false
|
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
|
return false
|
||||||
}
|
}
|
||||||
targetNorm := routing.NormalizeAgentID(targetAgentID)
|
|
||||||
for _, allowed := range parent.Subagents.AllowAgents {
|
for _, allowed := range parent.Subagents.AllowAgents {
|
||||||
if allowed == "*" {
|
if allowed == "*" {
|
||||||
return true
|
return true
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue