Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions internal/tool/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,33 @@ func TestToolRegistry_ToDefinitions(t *testing.T) {
}
}

// List() must be deterministic: it backs ToDefinitions(), whose output forms
// the front of the provider prompt prefix. Non-deterministic tool order breaks
// provider-side prompt/KV caching on every run (see List() doc comment).
func TestToolRegistry_ListDeterministicOrder(t *testing.T) {
r := registerTestTools(t)
first := r.ToDefinitions()
if len(first) < 2 {
t.Skipf("need multiple tools to test ordering, got %d", len(first))
}
for i := 0; i < 20; i++ {
again := r.ToDefinitions()
if len(again) != len(first) {
t.Fatalf("iteration %d: definition count changed: %d vs %d", i, len(again), len(first))
}
for j := range first {
if again[j].Name != first[j].Name {
t.Fatalf("iteration %d: order changed at position %d: %q vs %q (prompt-cache prefix instability)", i, j, first[j].Name, again[j].Name)
}
}
}
for i := 1; i < len(first); i++ {
if first[i-1].Name >= first[i].Name {
t.Fatalf("definitions not sorted by name at position %d: %q >= %q", i, first[i-1].Name, first[i].Name)
}
}
}

func TestToolRegistry_Unregister(t *testing.T) {
r := registerTestTools(t)
before := len(r.ToolNames())
Expand Down
21 changes: 17 additions & 4 deletions internal/tool/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"sort"
"strings"
"sync"

Expand Down Expand Up @@ -152,12 +153,24 @@ func (r *Registry) List() []Tool {
for _, t := range r.tools {
out = append(out, t)
}
// Deterministic order by name. r.tools is a map, so Go map iteration
// randomizes the order across calls. Downstream consumers of List() -
// ToolNames() and ToDefinitions() - feed the provider request payload,
// and tool definitions sit at the very front of the prompt prefix
// (tools → system → messages on Anthropic; same conceptual position for
// OpenAI-compatible auto prefix caching). A randomized tool order
// produces a different byte prefix on every run, invalidating the
// provider-side prompt/KV cache before any cache_control breakpoint can
// take effect, which inflates TTFT and input-token cost (Manus,
// "Context Engineering for AI Agents", 2025: KV-cache hit rate is the
// single most cost-impactful property of an agent's prompt prefix).
sort.Slice(out, func(i, j int) bool { return out[i].Name() < out[j].Name() })
return out
}

// CloseAll calls Close() on every registered tool that implements Closer.
// This releases resources like browser processes, network connections, etc.
// Errors are collected but do not stop cleanup all tools are attempted.
// Errors are collected but do not stop cleanup - all tools are attempted.
func (r *Registry) CloseAll() []error {
r.mu.RLock()
tools := make([]Tool, 0, len(r.tools))
Expand All @@ -179,7 +192,7 @@ func (r *Registry) CloseAll() []error {

// ToDefinitions converts all available tools to provider.ToolDefinition for
// the LLM. Tools implementing AvailabilityChecker with Available()==false are
// excluded e.g. restart before a host injects its requester (#346), so
// excluded - e.g. restart before a host injects its requester (#346), so
// hosts without restart support never advertise a guaranteed-failing tool.
func (r *Registry) ToDefinitions() []provider.ToolDefinition {
tools := r.List()
Expand Down Expand Up @@ -229,7 +242,7 @@ func (r *Registry) GetMeta(name string) (ToolMeta, bool) {
// Cloner is an optional interface that tools can implement to provide a deep copy.
// Tools that hold mutable state (e.g., WorkingDir) MUST implement Clone so that
// each agent gets its own independent tool instances. Tools without mutable state
// can safely skip this interface they will be shared between agents.
// can safely skip this interface - they will be shared between agents.
//
// This is critical for correctness in concurrent scenarios (sub-agents, swarm
// teammates using different worktrees). Without cloning, syncToolWorkingDir would
Expand All @@ -251,7 +264,7 @@ func (r *Registry) Clone() *Registry {
if c, ok := t.(Cloner); ok {
newReg.tools[name] = c.Clone()
} else {
// Stateless tool safe to share the same instance.
// Stateless tool - safe to share the same instance.
newReg.tools[name] = t
}
}
Expand Down
Loading