pickTool: name the chosen tool in the error and match tool names leniently

tools.go

@@ -133,15 +133,82 @@ func (t *ToolDefinition[T]) Execute(args map[string]any) (string, any, error) {
 
 type Tools []ToolDefinitionInterface
 
+// Find returns the tool whose function name matches name. It first tries an
+// exact match (fast path, unchanged behavior). If none is found, it falls back
+// to a lenient match: local models frequently emit namespaced/case/separator
+// variants (e.g. "functions.foo", "tools/foo", "Foo", "foo-bar" for "foo_bar").
+// The lenient pass never overrides an exact match, so existing resolutions are
+// unaffected.
 func (t Tools) Find(name string) ToolDefinitionInterface {
 	for _, tool := range t {
 		if tool.Tool().Function.Name == name {
 			return tool
 		}
 	}
+	if matches := t.lenientMatches(name); len(matches) == 1 {
+		return matches[0]
+	}
 	return nil
 }
 
+// lenientMatches returns the tools whose normalized name equals the
+// normalized form of name. Find only accepts a lenient match when exactly one
+// tool matches: "github/list_issues" and "gitlab/list_issues" both normalize to
+// "list_issues", and picking whichever comes first would run the wrong tool.
+func (t Tools) lenientMatches(name string) []ToolDefinitionInterface {
+	norm := normalizeToolName(name)
+	if norm == "" {
+		return nil
+	}
+	var matches []ToolDefinitionInterface
+	for _, tool := range t {
+		if normalizeToolName(tool.Tool().Function.Name) == norm {
+			matches = append(matches, tool)
+		}
+	}
+	return matches
+}
+
+// notFoundError explains why name did not resolve to a tool, naming the
+// candidates when a lenient match was ambiguous.
+func (t Tools) notFoundError(name string) error {
+	if matches := t.lenientMatches(name); len(matches) > 1 {
+		names := make([]string, 0, len(matches))
+		for _, m := range matches {
+			names = append(names, m.Tool().Function.Name)
+		}
+		return fmt.Errorf("chosen tool %q not found: ambiguous, matches %s", name, strings.Join(names, ", "))
+	}
+	return fmt.Errorf("chosen tool %q not found", name)
+}
+
+// canonicalizeToolChoice rewrites tc.Name to the real name of the tool it
+// resolves to. Tool-call callbacks approve by name and execution resolves the
+// name again, so both must see the same, canonical name: otherwise a model that
+// calls "Bash" would get "bash" executed while an approval hook configured for
+// "bash" never sees it.
+func (t Tools) canonicalizeToolChoice(tc *ToolChoice) {
+	if tc == nil {
+		return
+	}
+	if tool := t.Find(tc.Name); tool != nil {
+		tc.Name = tool.Tool().Function.Name
+	}
+}
+
+// normalizeToolName reduces a tool name to a comparison key: it drops a leading
+// namespace segment ("functions.", "tools/", "namespace::"), lower-cases, trims
+// spaces, and treats '-' and '_' as equivalent. Used only as a lenient fallback
+// in Find; it never affects exact-match resolution.
+func normalizeToolName(s string) string {
+	if i := strings.LastIndexAny(s, "./:"); i >= 0 {
+		s = s[i+1:] // segment after the last separator (may be empty → empty key)
+	}
+	s = strings.ToLower(strings.TrimSpace(s))
+	s = strings.ReplaceAll(s, "-", "_")
+	return s
+}
+
 func (t Tools) ToOpenAI() []openai.Tool {
 	openaiTools := []openai.Tool{}
 	for _, tool := range t {
@@ -835,7 +902,7 @@ func pickTool(ctx context.Context, llm LLM, fragment Fragment, tools Tools, opts
 			}
 
 			toolChoices = append(toolChoices, &ToolChoice{
-				Name:      toolName,
+				Name:      chosenTool.Tool().Function.Name,
 				Arguments: make(map[string]any),
 				Reasoning: intentionReasoning,
 			})
@@ -861,11 +928,11 @@ func pickTool(ctx context.Context, llm LLM, fragment Fragment, tools Tools, opts
 		chosenTool := tools.Find(intentionResponse.Tool)
 		if chosenTool == nil {
 			xlog.Debug("[pickTool] Chosen tool not found", "tool", intentionResponse.Tool)
-			return nil, fmt.Errorf("chosen tool not found")
+			return nil, tools.notFoundError(intentionResponse.Tool)
 		}
 
 		toolChoices = append(toolChoices, &ToolChoice{
-			Name:      intentionResponse.Tool,
+			Name:      chosenTool.Tool().Function.Name,
 			Arguments: make(map[string]any),
 			Reasoning: intentionReasoning,
 		})
@@ -1614,6 +1681,13 @@ TOOL_LOOP:
 
 		xlog.Debug("Picked tools with args", "count", len(selectedToolResults))
 
+		// Canonicalize every selected tool name before the sink-state check and
+		// the tool-call callbacks, so what gets approved is what gets executed
+		// (native tool calls carry the model's own spelling).
+		for _, toolResult := range selectedToolResults {
+			tools.canonicalizeToolChoice(toolResult)
+		}
+
 		// Check for sink state and separate tools
 		var toolsToExecute []*ToolChoice
 		sinkStateName := ""

tools_lenient_test.go

@@ -0,0 +1,136 @@
+package cogito
+
+import (
+	"strings"
+	"sync"
+	"testing"
+)
+
+func TestNormalizeToolName(t *testing.T) {
+	cases := []struct{ in, want string }{
+		{"foo", "foo"},
+		{"Foo", "foo"},
+		{"functions.foo", "foo"},
+		{"tools/foo", "foo"},
+		{"namespace::foo", "foo"},
+		{"foo-bar", "foo_bar"},
+		{"Foo-Bar", "foo_bar"},
+		{"functions.Coach_Regeln", "coach_regeln"},
+		{"  foo  ", "foo"},
+		{"", ""},
+		{"functions.", ""}, // nothing after the separator → empty key
+	}
+	for _, c := range cases {
+		if got := normalizeToolName(c.in); got != c.want {
+			t.Errorf("normalizeToolName(%q) = %q, want %q", c.in, got, c.want)
+		}
+	}
+}
+
+func TestFindLenient(t *testing.T) {
+	tools := Tools{newNamedTool("coach_regeln")}
+
+	// exact + lenient variants local models routinely emit
+	for _, n := range []string{
+		"coach_regeln",           // exact
+		"functions.coach_regeln", // namespaced
+		"tools/coach_regeln",
+		"Coach_Regeln", // case
+		"coach-regeln", // separator
+	} {
+		if tools.Find(n) == nil {
+			t.Errorf("Find(%q) = nil, want match", n)
+		}
+	}
+
+	for _, n := range []string{"other_tool", "", "functions."} {
+		if tools.Find(n) != nil {
+			t.Errorf("Find(%q) matched, want nil", n)
+		}
+	}
+}
+
+// An exact match must always win, even when a lenient candidate appears earlier
+// in the slice — the lenient pass is a fallback, never an override.
+func TestFindExactWinsOverLenient(t *testing.T) {
+	tools := Tools{newNamedTool("functions.foo"), newNamedTool("foo")}
+	got := tools.Find("foo")
+	if got == nil || got.Tool().Function.Name != "foo" {
+		name := "<nil>"
+		if got != nil {
+			name = got.Tool().Function.Name
+		}
+		t.Errorf(`Find("foo") = %q, want exact "foo"`, name)
+	}
+}
+
+// A lenient match must hand the tool-call callback the CANONICAL tool name —
+// the name that is executed — not the model's spelling. Otherwise an approval
+// hook configured for "echo" would never see a call the model spelled "Echo".
+func TestLenientToolCallCallbackSeesCanonicalName(t *testing.T) {
+	for _, spelled := range []string{"Echo", "functions.echo", "tools/echo"} {
+		t.Run(spelled, func(t *testing.T) {
+			var mu sync.Mutex
+			var seen []string
+			cb := func(tc *ToolChoice, _ *SessionState) ToolCallDecision {
+				mu.Lock()
+				seen = append(seen, tc.Name)
+				mu.Unlock()
+				return ToolCallDecision{Approved: true}
+			}
+			var echoMu sync.Mutex
+			echoCount := 0
+			llm := newScriptedLLM(spelled, `{"text": "hi"}`, "done")
+			f := NewEmptyFragment().AddMessage(UserMessageRole, "say hi")
+			if _, err := ExecuteTools(llm, f, WithTools(newEchoTool(&echoMu, &echoCount)), WithToolCallBack(cb), WithIterations(1)); err != nil && err != ErrNoToolSelected {
+				t.Fatalf("ExecuteTools: %v", err)
+			}
+			if len(seen) != 1 || seen[0] != "echo" {
+				t.Fatalf("callback saw %v, want [echo]", seen)
+			}
+			if echoCount != 1 {
+				t.Fatalf("echo ran %d times, want 1", echoCount)
+			}
+		})
+	}
+}
+
+// When two tools normalize to the same name, a lenient lookup must not pick
+// either one: it is not found, and the error names the candidates.
+func TestFindAmbiguousLenientIsNotFound(t *testing.T) {
+	tools := Tools{newNamedTool("github/list_issues"), newNamedTool("gitlab/list_issues")}
+
+	if got := tools.Find("list_issues"); got != nil {
+		t.Fatalf(`Find("list_issues") = %q, want nil (ambiguous)`, got.Tool().Function.Name)
+	}
+	if got := tools.Find("functions.list_issues"); got != nil {
+		t.Fatalf(`Find("functions.list_issues") = %q, want nil (ambiguous)`, got.Tool().Function.Name)
+	}
+	// exact names still resolve
+	for _, n := range []string{"github/list_issues", "gitlab/list_issues"} {
+		if got := tools.Find(n); got == nil || got.Tool().Function.Name != n {
+			t.Fatalf("Find(%q) did not return the exact tool", n)
+		}
+	}
+	err := tools.notFoundError("list_issues")
+	if err == nil || !strings.Contains(err.Error(), "github/list_issues") || !strings.Contains(err.Error(), "gitlab/list_issues") {
+		t.Fatalf("notFoundError = %v, want both candidates named", err)
+	}
+	if e := (Tools{newNamedTool("foo")}).notFoundError("bar"); e == nil || strings.Contains(e.Error(), "ambiguous") {
+		t.Fatalf("plain not-found error = %v, want no ambiguity note", e)
+	}
+}
+
+// End to end: a model call to an ambiguous name must execute neither tool.
+func TestAmbiguousLenientToolCallExecutesNothing(t *testing.T) {
+	var mu sync.Mutex
+	a, b := 0, 0
+	toolA := NewToolDefinition[EchoArgs](echoRunner{mu: &mu, count: &a}, EchoArgs{}, "github/list_issues", "a")
+	toolB := NewToolDefinition[EchoArgs](echoRunner{mu: &mu, count: &b}, EchoArgs{}, "gitlab/list_issues", "b")
+	llm := newScriptedLLM("list_issues", `{"text": "x"}`, "done")
+	f := NewEmptyFragment().AddMessage(UserMessageRole, "list issues")
+	_, _ = ExecuteTools(llm, f, WithTools(toolA, toolB), WithIterations(1))
+	if a != 0 || b != 0 {
+		t.Fatalf("ambiguous name executed a tool (github=%d, gitlab=%d), want neither", a, b)
+	}
+}