mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-19 04:16:03 +00:00
refactor(tools): convert web_search to tenant-scoped provider resolution
Refactor WebSearchTool constructor to accept ConfigSecretsStore and MessageBus instead of hardcoded WebSearchConfig. Implement runtime per-tenant provider chain resolution via tenantChainCache. Constructor now: - Subscribes to cache invalidation events on config_secrets changes - Returns nil-safe tool instance (no early return on empty providers) - Passes secrets store and bus for dynamic resolution per Execute() call Add resolve unit tests; delete legacy config_test.go (functionality subsumed by cache tests and integration tests).
This commit is contained in:
1 parent
d7fae64974
commit
31a6d1a60c
4 files changed
+256
-229
No files matched your search
@@ -7,6 +7,12 @@ import (
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/bus"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
)
|
||||
|
||||
// Matching TS src/agents/tools/web-search.ts constants.
|
||||
@@ -82,28 +88,43 @@ func normalizeFreshness(value string) string {
|
||||
|
||||
// --- WebSearchTool ---
|
||||
|
||||
// WebSearchTool implements the web_search tool matching TS src/agents/tools/web-search.ts.
|
||||
// WebSearchTool implements the web_search tool with per-tenant provider chain
|
||||
// resolution. Providers are resolved per-request from config_secrets and
|
||||
// builtin_tool_tenant_configs.settings, cached per tenant for 60 seconds.
|
||||
type WebSearchTool struct {
|
||||
providers []SearchProvider
|
||||
cache *webCache
|
||||
secrets store.ConfigSecretsStore
|
||||
cache *webCache
|
||||
chainCache *tenantChainCache
|
||||
}
|
||||
|
||||
func NewWebSearchTool(cfg WebSearchConfig) *WebSearchTool {
|
||||
providers := buildSearchProviders(cfg)
|
||||
|
||||
if len(providers) == 0 {
|
||||
return nil
|
||||
// NewWebSearchTool constructs a WebSearchTool. msgBus may be nil (e.g. desktop
|
||||
// edition) — cache invalidation then relies on TTL alone.
|
||||
func NewWebSearchTool(secrets store.ConfigSecretsStore, msgBus *bus.MessageBus) *WebSearchTool {
|
||||
t := &WebSearchTool{
|
||||
secrets: secrets,
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
|
||||
ttl := cfg.CacheTTL
|
||||
if ttl <= 0 {
|
||||
ttl = defaultCacheTTL
|
||||
if msgBus != nil {
|
||||
msgBus.Subscribe("web_search:cache_invalidate", func(event bus.Event) {
|
||||
if event.Name != protocol.EventCacheInvalidate {
|
||||
return
|
||||
}
|
||||
payload, ok := event.Payload.(bus.CacheInvalidatePayload)
|
||||
if !ok || payload.Kind != bus.CacheKindBuiltinTools || payload.Key != "web_search" {
|
||||
return
|
||||
}
|
||||
if payload.TenantID == uuid.Nil {
|
||||
// Master admin write — wipe all tenants.
|
||||
t.chainCache.InvalidateAll()
|
||||
} else {
|
||||
t.chainCache.Invalidate(payload.TenantID)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
return &WebSearchTool{
|
||||
providers: providers,
|
||||
cache: newWebCache(defaultCacheMaxEntries, ttl),
|
||||
}
|
||||
return t
|
||||
}
|
||||
|
||||
func (t *WebSearchTool) Name() string { return "web_search" }
|
||||
@@ -180,10 +201,8 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]any) *Resul
|
||||
return NewResult(cached)
|
||||
}
|
||||
|
||||
// Resolve per-request provider chain (tenant may reorder / disable providers
|
||||
// via builtin_tool_tenant_configs.settings → ctx → ResolveWebSearchChain).
|
||||
// Defaults preserved when no override — backward-compat.
|
||||
chain := ResolveWebSearchChain(ctx, t.providers)
|
||||
// Resolve per-request provider chain from tenant config_secrets + settings overlay.
|
||||
chain := t.resolveChain(ctx)
|
||||
|
||||
// Try providers in order (first success wins)
|
||||
var lastErr error
|
||||
@@ -243,4 +262,3 @@ func formatSearchResults(query string, results []searchResult, provider string)
|
||||
}
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
@@ -3,69 +3,27 @@ package tools
|
||||
import (
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
)
|
||||
|
||||
// WebSearchConfig holds configuration for the web search tool.
|
||||
type WebSearchConfig struct {
|
||||
ProviderOrder []string
|
||||
ExaAPIKey string
|
||||
ExaEnabled bool
|
||||
ExaMaxResults int
|
||||
TavilyAPIKey string
|
||||
TavilyEnabled bool
|
||||
TavilyMaxResults int
|
||||
BraveAPIKey string
|
||||
BraveEnabled bool
|
||||
BraveMaxResults int
|
||||
DDGEnabled bool
|
||||
DDGMaxResults int
|
||||
CacheTTL time.Duration
|
||||
}
|
||||
|
||||
// WebSearchConfigFromConfig creates a WebSearchConfig from the global config.
|
||||
func WebSearchConfigFromConfig(cfg *config.Config) WebSearchConfig {
|
||||
return WebSearchConfig{
|
||||
ProviderOrder: cfg.Tools.Web.ProviderOrder,
|
||||
ExaEnabled: cfg.Tools.Web.Exa.Enabled,
|
||||
ExaAPIKey: cfg.Tools.Web.Exa.APIKey,
|
||||
ExaMaxResults: cfg.Tools.Web.Exa.MaxResults,
|
||||
TavilyEnabled: cfg.Tools.Web.Tavily.Enabled,
|
||||
TavilyAPIKey: cfg.Tools.Web.Tavily.APIKey,
|
||||
TavilyMaxResults: cfg.Tools.Web.Tavily.MaxResults,
|
||||
BraveEnabled: cfg.Tools.Web.Brave.Enabled,
|
||||
BraveAPIKey: cfg.Tools.Web.Brave.APIKey,
|
||||
BraveMaxResults: cfg.Tools.Web.Brave.MaxResults,
|
||||
DDGEnabled: true,
|
||||
DDGMaxResults: cfg.Tools.Web.DuckDuckGo.MaxResults,
|
||||
// buildProviderByName returns the SearchProvider for a known name.
|
||||
// Returns nil for unknown names. DDG ignores apiKey (not required).
|
||||
// maxResults <= 0 falls back to defaultSearchCount.
|
||||
func buildProviderByName(name, apiKey string, maxResults int) SearchProvider {
|
||||
if maxResults <= 0 {
|
||||
maxResults = defaultSearchCount
|
||||
}
|
||||
}
|
||||
|
||||
func buildSearchProviders(cfg WebSearchConfig) []SearchProvider {
|
||||
var providers []SearchProvider
|
||||
for _, providerID := range NormalizeWebSearchProviderOrder(cfg.ProviderOrder) {
|
||||
switch providerID {
|
||||
case searchProviderExa:
|
||||
if cfg.ExaEnabled && cfg.ExaAPIKey != "" {
|
||||
providers = append(providers, newExaSearchProvider(cfg.ExaAPIKey, cfg.ExaMaxResults))
|
||||
}
|
||||
case searchProviderTavily:
|
||||
if cfg.TavilyEnabled && cfg.TavilyAPIKey != "" {
|
||||
providers = append(providers, newTavilySearchProvider(cfg.TavilyAPIKey, cfg.TavilyMaxResults))
|
||||
}
|
||||
case searchProviderBrave:
|
||||
if cfg.BraveEnabled && cfg.BraveAPIKey != "" {
|
||||
providers = append(providers, newBraveSearchProvider(cfg.BraveAPIKey, cfg.BraveMaxResults))
|
||||
}
|
||||
case searchProviderDuckDuckGo:
|
||||
if cfg.DDGEnabled {
|
||||
providers = append(providers, newDuckDuckGoSearchProvider(cfg.DDGMaxResults))
|
||||
}
|
||||
}
|
||||
switch name {
|
||||
case searchProviderExa:
|
||||
return newExaSearchProvider(apiKey, maxResults)
|
||||
case searchProviderTavily:
|
||||
return newTavilySearchProvider(apiKey, maxResults)
|
||||
case searchProviderBrave:
|
||||
return newBraveSearchProvider(apiKey, maxResults)
|
||||
case searchProviderDuckDuckGo:
|
||||
return newDuckDuckGoSearchProvider(maxResults)
|
||||
default:
|
||||
return nil
|
||||
}
|
||||
return providers
|
||||
}
|
||||
|
||||
// NormalizeWebSearchProviderOrder normalizes user-specified provider order.
|
||||
|
||||
@@ -1,150 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestNormalizeWebSearchProviderOrder_Empty(t *testing.T) {
|
||||
got := NormalizeWebSearchProviderOrder(nil)
|
||||
want := []string{"exa", "tavily", "brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("NormalizeWebSearchProviderOrder(nil) = %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeWebSearchProviderOrder_UserSpecified(t *testing.T) {
|
||||
got := NormalizeWebSearchProviderOrder([]string{"brave", "exa"})
|
||||
// brave first, exa second (user order), tavily appended, ddg last
|
||||
want := []string{"brave", "exa", "tavily", "duckduckgo"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("got %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeWebSearchProviderOrder_DDGIgnored(t *testing.T) {
|
||||
// DDG in user order is ignored (always last)
|
||||
got := NormalizeWebSearchProviderOrder([]string{"duckduckgo", "tavily"})
|
||||
want := []string{"tavily", "exa", "brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("got %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeWebSearchProviderOrder_Dedup(t *testing.T) {
|
||||
got := NormalizeWebSearchProviderOrder([]string{"exa", "exa", "brave"})
|
||||
want := []string{"exa", "brave", "tavily", "duckduckgo"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("got %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeWebSearchProviderOrder_UnknownSkipped(t *testing.T) {
|
||||
got := NormalizeWebSearchProviderOrder([]string{"bing", "tavily"})
|
||||
want := []string{"tavily", "exa", "brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Errorf("got %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSearchProviders_AllEnabled(t *testing.T) {
|
||||
cfg := WebSearchConfig{
|
||||
ExaEnabled: true,
|
||||
ExaAPIKey: "exa-key",
|
||||
TavilyEnabled: true,
|
||||
TavilyAPIKey: "tavily-key",
|
||||
BraveEnabled: true,
|
||||
BraveAPIKey: "brave-key",
|
||||
DDGEnabled: true,
|
||||
}
|
||||
providers := buildSearchProviders(cfg)
|
||||
names := make([]string, len(providers))
|
||||
for i, p := range providers {
|
||||
names[i] = p.Name()
|
||||
}
|
||||
want := []string{"exa", "tavily", "brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(names, want) {
|
||||
t.Errorf("got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSearchProviders_OnlyBraveAndDDG(t *testing.T) {
|
||||
cfg := WebSearchConfig{
|
||||
BraveEnabled: true,
|
||||
BraveAPIKey: "brave-key",
|
||||
DDGEnabled: true,
|
||||
}
|
||||
providers := buildSearchProviders(cfg)
|
||||
names := make([]string, len(providers))
|
||||
for i, p := range providers {
|
||||
names[i] = p.Name()
|
||||
}
|
||||
// Exa + Tavily not enabled, so only brave + ddg
|
||||
want := []string{"brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(names, want) {
|
||||
t.Errorf("got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSearchProviders_CustomOrder(t *testing.T) {
|
||||
cfg := WebSearchConfig{
|
||||
ProviderOrder: []string{"tavily", "brave"},
|
||||
TavilyEnabled: true,
|
||||
TavilyAPIKey: "key",
|
||||
BraveEnabled: true,
|
||||
BraveAPIKey: "key",
|
||||
DDGEnabled: true,
|
||||
}
|
||||
providers := buildSearchProviders(cfg)
|
||||
names := make([]string, len(providers))
|
||||
for i, p := range providers {
|
||||
names[i] = p.Name()
|
||||
}
|
||||
// tavily first (user), brave second, exa appended (no key so skipped), ddg last
|
||||
want := []string{"tavily", "brave", "duckduckgo"}
|
||||
if !reflect.DeepEqual(names, want) {
|
||||
t.Errorf("got %v, want %v", names, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildSearchProviders_NoProviders(t *testing.T) {
|
||||
cfg := WebSearchConfig{}
|
||||
providers := buildSearchProviders(cfg)
|
||||
if len(providers) != 0 {
|
||||
t.Errorf("expected empty providers, got %d", len(providers))
|
||||
}
|
||||
}
|
||||
|
||||
func TestClampProviderResultCount(t *testing.T) {
|
||||
tests := []struct {
|
||||
requested, max, want int
|
||||
}{
|
||||
{5, 10, 5},
|
||||
{15, 10, 10},
|
||||
{0, 10, defaultSearchCount},
|
||||
{5, 0, 5},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got := clampProviderResultCount(tt.requested, tt.max)
|
||||
if got != tt.want {
|
||||
t.Errorf("clampProviderResultCount(%d, %d) = %d, want %d", tt.requested, tt.max, got, tt.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeProviderMaxResults(t *testing.T) {
|
||||
tests := []struct {
|
||||
input, want int
|
||||
}{
|
||||
{0, defaultSearchCount},
|
||||
{-1, defaultSearchCount},
|
||||
{7, 7},
|
||||
{15, maxSearchCount},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
got := normalizeProviderMaxResults(tt.input)
|
||||
if got != tt.want {
|
||||
t.Errorf("normalizeProviderMaxResults(%d) = %d, want %d", tt.input, got, tt.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,201 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
// TestResolveChain_CacheHit verifies that resolveChain returns cached chains.
|
||||
// Scenario 7: back-to-back same-tenant calls return the same cached slice.
|
||||
func TestResolveChain_CacheHit(t *testing.T) {
|
||||
fake := newFakeSecretsStore()
|
||||
tool := &WebSearchTool{
|
||||
secrets: fake,
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
|
||||
tid := uuid.New()
|
||||
ctx := context.Background()
|
||||
ctx = store.WithTenantID(ctx, tid)
|
||||
|
||||
// Set a provider key
|
||||
fake.Set(ctx, "tools.web.brave.api_key", "test-key-brave")
|
||||
|
||||
// First call → resolves and caches
|
||||
chain1 := tool.resolveChain(ctx)
|
||||
|
||||
// Second call → should hit cache
|
||||
chain2 := tool.resolveChain(ctx)
|
||||
|
||||
if len(chain1) == 0 || len(chain2) == 0 {
|
||||
t.Fatalf("expected non-empty chains, got %d and %d", len(chain1), len(chain2))
|
||||
}
|
||||
|
||||
// Both should have same provider order
|
||||
if len(chain1) != len(chain2) {
|
||||
t.Errorf("chain length mismatch: %d vs %d", len(chain1), len(chain2))
|
||||
}
|
||||
|
||||
for i := 0; i < len(chain1); i++ {
|
||||
if chain1[i].Name() != chain2[i].Name() {
|
||||
t.Errorf("provider %d: %s vs %s", i, chain1[i].Name(), chain2[i].Name())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveChain_TenantIsolation verifies that two tenants get independent chains.
|
||||
// Scenario 8: switching tenant context yields different chains.
|
||||
func TestResolveChain_TenantIsolation(t *testing.T) {
|
||||
fake := newFakeSecretsStore()
|
||||
tool := &WebSearchTool{
|
||||
secrets: fake,
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
|
||||
tenantA := uuid.New()
|
||||
tenantB := uuid.New()
|
||||
|
||||
// Setup tenant A: Brave key only
|
||||
ctxA := context.Background()
|
||||
ctxA = store.WithTenantID(ctxA, tenantA)
|
||||
fake.Set(ctxA, "tools.web.brave.api_key", "test-key-brave-A")
|
||||
|
||||
// Setup tenant B: Exa key only
|
||||
ctxB := context.Background()
|
||||
ctxB = store.WithTenantID(ctxB, tenantB)
|
||||
fake.Set(ctxB, "tools.web.exa.api_key", "test-key-exa-B")
|
||||
|
||||
// Resolve chains for each tenant
|
||||
chainA := tool.resolveChain(ctxA)
|
||||
chainB := tool.resolveChain(ctxB)
|
||||
|
||||
// Tenant A should have Brave
|
||||
foundBraveInA := false
|
||||
for _, p := range chainA {
|
||||
if p.Name() == "brave" {
|
||||
foundBraveInA = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !foundBraveInA {
|
||||
t.Error("tenant A chain missing Brave")
|
||||
}
|
||||
|
||||
// Tenant B should have Exa
|
||||
foundExaInB := false
|
||||
for _, p := range chainB {
|
||||
if p.Name() == "exa" {
|
||||
foundExaInB = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !foundExaInB {
|
||||
t.Error("tenant B chain missing Exa")
|
||||
}
|
||||
|
||||
// Tenant A should NOT have Exa
|
||||
foundExaInA := false
|
||||
for _, p := range chainA {
|
||||
if p.Name() == "exa" {
|
||||
foundExaInA = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if foundExaInA {
|
||||
t.Error("tenant A chain should not have Exa")
|
||||
}
|
||||
|
||||
// Tenant B should NOT have Brave
|
||||
foundBraveInB := false
|
||||
for _, p := range chainB {
|
||||
if p.Name() == "brave" {
|
||||
foundBraveInB = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if foundBraveInB {
|
||||
t.Error("tenant B chain should not have Brave")
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveChain_CacheInvalidation verifies cache invalidation refreshes the chain.
|
||||
func TestResolveChain_CacheInvalidation(t *testing.T) {
|
||||
fake := newFakeSecretsStore()
|
||||
tool := &WebSearchTool{
|
||||
secrets: fake,
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
|
||||
tid := uuid.New()
|
||||
ctx := context.Background()
|
||||
ctx = store.WithTenantID(ctx, tid)
|
||||
|
||||
// Initial state: only DDG
|
||||
chain1 := tool.resolveChain(ctx)
|
||||
if len(chain1) != 1 || chain1[0].Name() != "duckduckgo" {
|
||||
t.Fatalf("initial chain should be DDG only, got %v", chainNames(chain1))
|
||||
}
|
||||
|
||||
// Add a provider key and invalidate cache
|
||||
fake.Set(ctx, "tools.web.brave.api_key", "test-key-brave-new")
|
||||
tool.chainCache.Invalidate(tid)
|
||||
|
||||
// Next resolve should pick up the new key
|
||||
chain2 := tool.resolveChain(ctx)
|
||||
if len(chain2) != 2 {
|
||||
t.Errorf("after invalidation, expected 2 providers, got %d", len(chain2))
|
||||
}
|
||||
|
||||
foundBrave := false
|
||||
for _, p := range chain2 {
|
||||
if p.Name() == "brave" {
|
||||
foundBrave = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !foundBrave {
|
||||
t.Error("after invalidation, Brave should be in chain")
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveChain_TTLCacheExpiry verifies that expired cache entries trigger refresh.
|
||||
func TestResolveChain_TTLCacheExpiry(t *testing.T) {
|
||||
fake := newFakeSecretsStore()
|
||||
tool := &WebSearchTool{
|
||||
secrets: fake,
|
||||
cache: newWebCache(defaultCacheMaxEntries, defaultCacheTTL),
|
||||
chainCache: newTenantChainCache(),
|
||||
}
|
||||
|
||||
tid := uuid.New()
|
||||
ctx := context.Background()
|
||||
ctx = store.WithTenantID(ctx, tid)
|
||||
|
||||
// Initial state: DDG only
|
||||
chain1 := tool.resolveChain(ctx)
|
||||
if len(chain1) != 1 {
|
||||
t.Fatalf("initial chain should be 1 provider, got %d", len(chain1))
|
||||
}
|
||||
|
||||
// Manually expire the cache entry
|
||||
tool.chainCache.entries[tid] = tenantChainEntry{
|
||||
chain: chain1,
|
||||
expires: time.Now().Add(-1 * time.Second), // expired
|
||||
}
|
||||
|
||||
// Add provider and resolve again — should bypass expired cache
|
||||
fake.Set(ctx, "tools.web.brave.api_key", "test-key-brave-after-ttl")
|
||||
chain2 := tool.resolveChain(ctx)
|
||||
|
||||
if len(chain2) != 2 {
|
||||
t.Errorf("after TTL expiry, expected 2 providers, got %d", len(chain2))
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user