fix(agent): hide discovery when spawn is unavailable
This commit is contained in:
parent
c6a09a35e2
commit
148583e7bb
4 changed files with 132 additions and 12 deletions
|
|
@ -238,7 +238,7 @@ Notes:
|
||||||
|
|
||||||
### Agent Discovery (Automatic)
|
### Agent Discovery (Automatic)
|
||||||
|
|
||||||
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.
|
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`.
|
This registry is intended to make delegation concrete and reliable, especially when using `spawn` with a target `agent_id`.
|
||||||
|
|
||||||
|
|
@ -252,7 +252,7 @@ Each entry includes:
|
||||||
|
|
||||||
Important behavior:
|
Important behavior:
|
||||||
|
|
||||||
- The discovery section includes only peer agents the current agent is permitted to spawn via `subagents.allow_agents`.
|
- 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.
|
- 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. Those permissions also gate discovery visibility.
|
- `config.json` remains the infrastructure layer: workspace, default agent selection, routing, and subagent permissions. Those permissions also gate discovery visibility.
|
||||||
|
|
|
||||||
|
|
@ -60,8 +60,9 @@ func (r *AgentRegistry) ListAgents(workspace string) []AgentDescriptor {
|
||||||
return descriptors
|
return descriptors
|
||||||
}
|
}
|
||||||
|
|
||||||
// ListSpawnableAgents returns descriptors only for agents the current agent is
|
// ListSpawnableAgents returns descriptors only when the current agent can call
|
||||||
// allowed to spawn. Restricted peers are intentionally omitted from discovery.
|
// spawn, and only for peers it is allowed to spawn. Restricted peers are
|
||||||
|
// intentionally omitted from discovery.
|
||||||
func (r *AgentRegistry) ListSpawnableAgents(agentID string) []AgentDescriptor {
|
func (r *AgentRegistry) ListSpawnableAgents(agentID string) []AgentDescriptor {
|
||||||
r.mu.RLock()
|
r.mu.RLock()
|
||||||
defer r.mu.RUnlock()
|
defer r.mu.RUnlock()
|
||||||
|
|
@ -71,6 +72,9 @@ func (r *AgentRegistry) ListSpawnableAgents(agentID string) []AgentDescriptor {
|
||||||
if !ok || parent == nil {
|
if !ok || parent == nil {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
if !agentHasSpawnTool(parent) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
ids := make([]string, 0, len(r.agents))
|
ids := make([]string, 0, len(r.agents))
|
||||||
for id := range r.agents {
|
for id := range r.agents {
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/sipeed/picoclaw/pkg/bus"
|
||||||
"github.com/sipeed/picoclaw/pkg/config"
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -79,9 +80,13 @@ func TestAgentRegistry_ListSpawnableAgentsRespectsPermissions(t *testing.T) {
|
||||||
{ID: "child2"},
|
{ID: "child2"},
|
||||||
{ID: "restricted"},
|
{ID: "restricted"},
|
||||||
})
|
})
|
||||||
|
cfg.Tools.Spawn.Enabled = true
|
||||||
|
cfg.Tools.Subagent.Enabled = true
|
||||||
|
|
||||||
registry := NewAgentRegistry(cfg, &mockRegistryProvider{})
|
al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{})
|
||||||
descriptors := registry.ListSpawnableAgents("parent")
|
defer al.Close()
|
||||||
|
|
||||||
|
descriptors := al.GetRegistry().ListSpawnableAgents("parent")
|
||||||
if len(descriptors) != 2 {
|
if len(descriptors) != 2 {
|
||||||
t.Fatalf("expected 2 spawnable descriptors, got %d: %+v", len(descriptors), descriptors)
|
t.Fatalf("expected 2 spawnable descriptors, got %d: %+v", len(descriptors), descriptors)
|
||||||
}
|
}
|
||||||
|
|
@ -90,6 +95,27 @@ func TestAgentRegistry_ListSpawnableAgentsRespectsPermissions(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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) {
|
func TestContextBuilder_BuildMessagesIncludesAgentDiscoverySection(t *testing.T) {
|
||||||
mainWorkspace := setupWorkspace(t, map[string]string{
|
mainWorkspace := setupWorkspace(t, map[string]string{
|
||||||
"AGENT.md": `---
|
"AGENT.md": `---
|
||||||
|
|
@ -140,9 +166,13 @@ Handle restricted work.
|
||||||
})
|
})
|
||||||
cfg.Tools.ReadFile.Enabled = true
|
cfg.Tools.ReadFile.Enabled = true
|
||||||
cfg.Tools.WriteFile.Enabled = true
|
cfg.Tools.WriteFile.Enabled = true
|
||||||
|
cfg.Tools.Spawn.Enabled = true
|
||||||
|
cfg.Tools.Subagent.Enabled = true
|
||||||
|
|
||||||
registry := NewAgentRegistry(cfg, &mockRegistryProvider{})
|
al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{})
|
||||||
mainAgent, ok := registry.GetAgent("main")
|
defer al.Close()
|
||||||
|
|
||||||
|
mainAgent, ok := al.GetRegistry().GetAgent("main")
|
||||||
if !ok || mainAgent == nil {
|
if !ok || mainAgent == nil {
|
||||||
t.Fatal("expected main agent")
|
t.Fatal("expected main agent")
|
||||||
}
|
}
|
||||||
|
|
@ -211,9 +241,13 @@ Investigate deeply.
|
||||||
{ID: "research", Workspace: researchWorkspace},
|
{ID: "research", Workspace: researchWorkspace},
|
||||||
})
|
})
|
||||||
cfg.Tools.ReadFile.Enabled = true
|
cfg.Tools.ReadFile.Enabled = true
|
||||||
|
cfg.Tools.Spawn.Enabled = true
|
||||||
|
cfg.Tools.Subagent.Enabled = true
|
||||||
|
|
||||||
registry := NewAgentRegistry(cfg, &mockRegistryProvider{})
|
al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{})
|
||||||
mainAgent, ok := registry.GetAgent("main")
|
defer al.Close()
|
||||||
|
|
||||||
|
mainAgent, ok := al.GetRegistry().GetAgent("main")
|
||||||
if !ok || mainAgent == nil {
|
if !ok || mainAgent == nil {
|
||||||
t.Fatal("expected main agent")
|
t.Fatal("expected main agent")
|
||||||
}
|
}
|
||||||
|
|
@ -241,6 +275,76 @@ Investigate deeply.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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) {
|
func TestContextBuilder_BuildMessagesOmitsAgentDiscoverySectionForSingleton(t *testing.T) {
|
||||||
mainWorkspace := setupWorkspace(t, map[string]string{
|
mainWorkspace := setupWorkspace(t, map[string]string{
|
||||||
"AGENT.md": `---
|
"AGENT.md": `---
|
||||||
|
|
@ -257,9 +361,13 @@ Generalist.
|
||||||
{ID: "main", Default: true, Workspace: mainWorkspace},
|
{ID: "main", Default: true, Workspace: mainWorkspace},
|
||||||
})
|
})
|
||||||
cfg.Tools.ReadFile.Enabled = true
|
cfg.Tools.ReadFile.Enabled = true
|
||||||
|
cfg.Tools.Spawn.Enabled = true
|
||||||
|
cfg.Tools.Subagent.Enabled = true
|
||||||
|
|
||||||
registry := NewAgentRegistry(cfg, &mockRegistryProvider{})
|
al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockRegistryProvider{})
|
||||||
mainAgent, ok := registry.GetAgent("main")
|
defer al.Close()
|
||||||
|
|
||||||
|
mainAgent, ok := al.GetRegistry().GetAgent("main")
|
||||||
if !ok || mainAgent == nil {
|
if !ok || mainAgent == nil {
|
||||||
t.Fatal("expected main agent")
|
t.Fatal("expected main agent")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -137,6 +137,14 @@ func agentAllowsSubagent(parent *AgentInstance, targetNorm string) bool {
|
||||||
return false
|
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
|
// ForEachTool calls fn for every tool registered under the given name
|
||||||
// across all agents. This is useful for propagating dependencies (e.g.
|
// across all agents. This is useful for propagating dependencies (e.g.
|
||||||
// MediaStore) to tools after registry construction.
|
// MediaStore) to tools after registry construction.
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue