fix(agent): treat empty AGENT.md tools as allow none
This commit is contained in:
parent
148583e7bb
commit
2ae25b1038
3 changed files with 124 additions and 1 deletions
|
|
@ -842,3 +842,60 @@ mcpServers: [github]
|
||||||
t.Fatal("expected malformed frontmatter to fail closed for MCP servers")
|
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")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -144,7 +144,7 @@ func resolveAgentToolAllowlist(definition AgentContextDefinition) []string {
|
||||||
if frontmatterParseFailed(definition) {
|
if frontmatterParseFailed(definition) {
|
||||||
return []string{}
|
return []string{}
|
||||||
}
|
}
|
||||||
if definition.Agent == nil || definition.Agent.Frontmatter.Tools == nil {
|
if definition.Agent == nil || !frontmatterDeclaresField(definition, "tools") {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -157,6 +157,10 @@ func resolveAgentToolAllowlist(definition AgentContextDefinition) []string {
|
||||||
allowlist[trimmed] = struct{}{}
|
allowlist[trimmed] = struct{}{}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if len(allowlist) == 0 {
|
||||||
|
return []string{}
|
||||||
|
}
|
||||||
|
|
||||||
return sortedKeys(allowlist)
|
return sortedKeys(allowlist)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -68,6 +68,68 @@ tools: [serial, reaction, send_tts, load_image, delegate, made_up]
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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) {
|
func TestUnknownAgentMCPServerNames(t *testing.T) {
|
||||||
workspace := setupWorkspace(t, map[string]string{
|
workspace := setupWorkspace(t, map[string]string{
|
||||||
"AGENT.md": `---
|
"AGENT.md": `---
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue