fix: fuzzy tool name resolution for LLM tool call mismatches
LLMs (e.g. MiniMax) sometimes call "readfile" instead of "read_file". Add NormalizeToolName to strip underscores/hyphens and lowercase, with fuzzy fallback in ToolRegistry.Get(). Also normalize the interview tool allow-list so blocked tools aren't bypassed by name variants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
45604daf25
commit
ee15656e6c
4 changed files with 156 additions and 6 deletions
|
|
@ -1584,16 +1584,19 @@ func isPlanPreExecution(status string) bool {
|
|||
// isToolAllowedDuringInterview checks whether a tool call is permitted while the
|
||||
// plan is in a pre-execution state. Read-type tools are always allowed. Write-type
|
||||
// tools (edit_file, append_file, write_file) are only allowed when targeting MEMORY.md.
|
||||
// Uses normalized names so "readfile" matches "read_file", etc.
|
||||
func isToolAllowedDuringInterview(toolName string, args map[string]interface{}) bool {
|
||||
norm := tools.NormalizeToolName(toolName)
|
||||
|
||||
// Read-type tools: always allowed
|
||||
switch toolName {
|
||||
case "read_file", "list_dir", "web_search", "web_fetch":
|
||||
switch norm {
|
||||
case "readfile", "listdir", "websearch", "webfetch":
|
||||
return true
|
||||
}
|
||||
|
||||
// Write-type tools: allowed only when targeting MEMORY.md
|
||||
switch toolName {
|
||||
case "edit_file", "append_file", "write_file":
|
||||
switch norm {
|
||||
case "editfile", "appendfile", "writefile":
|
||||
path, _ := args["path"].(string)
|
||||
return strings.HasSuffix(path, "MEMORY.md")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1319,3 +1319,39 @@ Test
|
|||
t.Error("expected plan to be cleared after completion")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsToolAllowedDuringInterview_FuzzyNames(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
args map[string]interface{}
|
||||
want bool
|
||||
}{
|
||||
// Exact names — read tools allowed
|
||||
{"read_file", nil, true},
|
||||
{"list_dir", nil, true},
|
||||
{"web_search", nil, true},
|
||||
{"web_fetch", nil, true},
|
||||
// Fuzzy variants — should also be allowed
|
||||
{"readfile", nil, true},
|
||||
{"ReadFile", nil, true},
|
||||
{"listdir", nil, true},
|
||||
{"websearch", nil, true},
|
||||
{"webfetch", nil, true},
|
||||
// Write to MEMORY.md — allowed
|
||||
{"edit_file", map[string]interface{}{"path": "/ws/memory/MEMORY.md"}, true},
|
||||
{"editfile", map[string]interface{}{"path": "/ws/memory/MEMORY.md"}, true},
|
||||
{"EditFile", map[string]interface{}{"path": "/ws/memory/MEMORY.md"}, true},
|
||||
// Write to non-MEMORY.md — blocked
|
||||
{"edit_file", map[string]interface{}{"path": "/ws/main.go"}, false},
|
||||
{"editfile", map[string]interface{}{"path": "/ws/main.go"}, false},
|
||||
// exec — always blocked
|
||||
{"exec", nil, false},
|
||||
{"Exec", nil, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got := isToolAllowedDuringInterview(tt.name, tt.args)
|
||||
if got != tt.want {
|
||||
t.Errorf("isToolAllowedDuringInterview(%q, %v) = %v, want %v", tt.name, tt.args, got, tt.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package tools
|
|||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
|
|
@ -10,6 +11,16 @@ import (
|
|||
"github.com/sipeed/picoclaw/pkg/providers"
|
||||
)
|
||||
|
||||
// NormalizeToolName strips underscores and hyphens and lowercases for
|
||||
// fuzzy tool name matching. LLMs sometimes call "readfile" instead of
|
||||
// "read_file", etc.
|
||||
func NormalizeToolName(s string) string {
|
||||
s = strings.ToLower(s)
|
||||
s = strings.ReplaceAll(s, "_", "")
|
||||
s = strings.ReplaceAll(s, "-", "")
|
||||
return s
|
||||
}
|
||||
|
||||
type ToolRegistry struct {
|
||||
tools map[string]Tool
|
||||
mu sync.RWMutex
|
||||
|
|
@ -30,8 +41,18 @@ func (r *ToolRegistry) Register(tool Tool) {
|
|||
func (r *ToolRegistry) Get(name string) (Tool, bool) {
|
||||
r.mu.RLock()
|
||||
defer r.mu.RUnlock()
|
||||
tool, ok := r.tools[name]
|
||||
return tool, ok
|
||||
// Exact match first
|
||||
if tool, ok := r.tools[name]; ok {
|
||||
return tool, true
|
||||
}
|
||||
// Fuzzy fallback: normalize and compare (handles "readfile" → "read_file" etc.)
|
||||
norm := NormalizeToolName(name)
|
||||
for _, tool := range r.tools {
|
||||
if NormalizeToolName(tool.Name()) == norm {
|
||||
return tool, true
|
||||
}
|
||||
}
|
||||
return nil, false
|
||||
}
|
||||
|
||||
func (r *ToolRegistry) Execute(ctx context.Context, name string, args map[string]interface{}) *ToolResult {
|
||||
|
|
|
|||
90
pkg/tools/registry_test.go
Normal file
90
pkg/tools/registry_test.go
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// stubTool is a minimal Tool implementation for testing.
|
||||
type stubTool struct {
|
||||
name string
|
||||
}
|
||||
|
||||
func (s *stubTool) Name() string { return s.name }
|
||||
func (s *stubTool) Description() string { return "stub" }
|
||||
func (s *stubTool) Parameters() map[string]interface{} {
|
||||
return map[string]interface{}{"type": "object", "properties": map[string]interface{}{}}
|
||||
}
|
||||
func (s *stubTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult {
|
||||
return &ToolResult{ForLLM: "ok"}
|
||||
}
|
||||
|
||||
func TestNormalizeToolName(t *testing.T) {
|
||||
tests := []struct {
|
||||
input, want string
|
||||
}{
|
||||
{"read_file", "readfile"},
|
||||
{"readfile", "readfile"},
|
||||
{"ReadFile", "readfile"},
|
||||
{"read-file", "readfile"},
|
||||
{"edit_file", "editfile"},
|
||||
{"web_search", "websearch"},
|
||||
{"EXEC", "exec"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got := NormalizeToolName(tt.input)
|
||||
if got != tt.want {
|
||||
t.Errorf("NormalizeToolName(%q) = %q, want %q", tt.input, got, tt.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryGet_ExactMatch(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.Register(&stubTool{name: "read_file"})
|
||||
|
||||
tool, ok := r.Get("read_file")
|
||||
if !ok || tool.Name() != "read_file" {
|
||||
t.Errorf("exact match failed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryGet_FuzzyMatch(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.Register(&stubTool{name: "read_file"})
|
||||
r.Register(&stubTool{name: "edit_file"})
|
||||
r.Register(&stubTool{name: "web_search"})
|
||||
|
||||
tests := []struct {
|
||||
query string
|
||||
wantName string
|
||||
}{
|
||||
{"readfile", "read_file"},
|
||||
{"ReadFile", "read_file"},
|
||||
{"read-file", "read_file"},
|
||||
{"editfile", "edit_file"},
|
||||
{"EditFile", "edit_file"},
|
||||
{"websearch", "web_search"},
|
||||
{"WebSearch", "web_search"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
tool, ok := r.Get(tt.query)
|
||||
if !ok {
|
||||
t.Errorf("Get(%q) not found, want %q", tt.query, tt.wantName)
|
||||
continue
|
||||
}
|
||||
if tool.Name() != tt.wantName {
|
||||
t.Errorf("Get(%q).Name() = %q, want %q", tt.query, tool.Name(), tt.wantName)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryGet_NotFound(t *testing.T) {
|
||||
r := NewToolRegistry()
|
||||
r.Register(&stubTool{name: "read_file"})
|
||||
|
||||
_, ok := r.Get("totally_unknown")
|
||||
if ok {
|
||||
t.Errorf("Get(totally_unknown) should return false")
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue