From ee15656e6cbbed3847d723c2cc31cb447b9bac53 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Sat, 21 Feb 2026 02:56:05 +0900 Subject: [PATCH] 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 --- pkg/agent/loop.go | 11 +++-- pkg/agent/loop_test.go | 36 +++++++++++++++ pkg/tools/registry.go | 25 ++++++++++- pkg/tools/registry_test.go | 90 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 pkg/tools/registry_test.go diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 033e92039..0fd2b482b 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -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") } diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 37cabb961..83067db95 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -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) + } + } +} diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index c8cf92863..957226eb3 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -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 { diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go new file mode 100644 index 000000000..a161b3f12 --- /dev/null +++ b/pkg/tools/registry_test.go @@ -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") + } +}