From cac4f21746cd1e421af053c7281430e2fee07a6c Mon Sep 17 00:00:00 2001 From: wenjie Date: Thu, 23 Apr 2026 15:39:16 +0800 Subject: [PATCH] fix(tools): improve web search provider fallback (#2629) - centralize web search provider readiness and resolution logic - fall back when the configured provider is unavailable or invalid - allow native-search-capable models to use built-in search without the client tool - simplify the tools page and add direct access to web search settings - add backend, agent, and integration tests for the new selection behavior --- pkg/agent/agent_init.go | 28 +-- pkg/agent/agent_test.go | 52 +++++ pkg/agent/pipeline_llm.go | 6 +- pkg/agent/turn_coord_test.go | 64 ++++++ pkg/tools/integration/web.go | 216 +++++++++++++----- pkg/tools/integration/web_test.go | 104 +++++++-- pkg/tools/integration_facade.go | 13 ++ web/backend/api/tools.go | 115 +++------- web/backend/api/tools_test.go | 143 ++++++++++++ .../agent/tools/tool-library-tab.tsx | 49 +++- .../src/components/agent/tools/tools-page.tsx | 14 +- .../components/agent/tools/use-tools-page.ts | 6 - .../tools/web-search-general-settings.tsx | 4 +- .../components/agent/tools/web-search-tab.tsx | 21 +- web/frontend/src/i18n/locales/en.json | 10 +- web/frontend/src/i18n/locales/zh.json | 10 +- 16 files changed, 633 insertions(+), 222 deletions(-) diff --git a/pkg/agent/agent_init.go b/pkg/agent/agent_init.go index d7bfc22c7..611d634e8 100644 --- a/pkg/agent/agent_init.go +++ b/pkg/agent/agent_init.go @@ -100,33 +100,7 @@ func registerSharedTools( } if cfg.Tools.IsToolEnabled("web") { - searchTool, err := tools.NewWebSearchTool(tools.WebSearchToolOptions{ - BraveAPIKeys: cfg.Tools.Web.Brave.APIKeys.Values(), - BraveMaxResults: cfg.Tools.Web.Brave.MaxResults, - BraveEnabled: cfg.Tools.Web.Brave.Enabled, - TavilyAPIKeys: cfg.Tools.Web.Tavily.APIKeys.Values(), - TavilyBaseURL: cfg.Tools.Web.Tavily.BaseURL, - TavilyMaxResults: cfg.Tools.Web.Tavily.MaxResults, - TavilyEnabled: cfg.Tools.Web.Tavily.Enabled, - DuckDuckGoMaxResults: cfg.Tools.Web.DuckDuckGo.MaxResults, - DuckDuckGoEnabled: cfg.Tools.Web.DuckDuckGo.Enabled, - PerplexityAPIKeys: cfg.Tools.Web.Perplexity.APIKeys.Values(), - PerplexityMaxResults: cfg.Tools.Web.Perplexity.MaxResults, - PerplexityEnabled: cfg.Tools.Web.Perplexity.Enabled, - SearXNGBaseURL: cfg.Tools.Web.SearXNG.BaseURL, - SearXNGMaxResults: cfg.Tools.Web.SearXNG.MaxResults, - SearXNGEnabled: cfg.Tools.Web.SearXNG.Enabled, - GLMSearchAPIKey: cfg.Tools.Web.GLMSearch.APIKey.String(), - GLMSearchBaseURL: cfg.Tools.Web.GLMSearch.BaseURL, - GLMSearchEngine: cfg.Tools.Web.GLMSearch.SearchEngine, - GLMSearchMaxResults: cfg.Tools.Web.GLMSearch.MaxResults, - GLMSearchEnabled: cfg.Tools.Web.GLMSearch.Enabled, - BaiduSearchAPIKey: cfg.Tools.Web.BaiduSearch.APIKey.String(), - BaiduSearchBaseURL: cfg.Tools.Web.BaiduSearch.BaseURL, - BaiduSearchMaxResults: cfg.Tools.Web.BaiduSearch.MaxResults, - BaiduSearchEnabled: cfg.Tools.Web.BaiduSearch.Enabled, - Proxy: cfg.Tools.Web.Proxy, - }) + searchTool, err := tools.NewWebSearchTool(tools.WebSearchToolOptionsFromConfig(cfg)) if err != nil { logger.ErrorCF("agent", "Failed to create web search tool", map[string]any{"error": err.Error()}) } else if searchTool != nil { diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 2addc0535..b0aa3b468 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -161,6 +161,58 @@ func newTestAgentLoop( return al, cfg, msgBus, provider, func() { os.RemoveAll(tmpDir) } } +func TestNewAgentLoop_RegistersWebSearchTool(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Agents.Defaults.Workspace = t.TempDir() + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockProvider{}) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if _, ok := agent.Tools.Get("web_search"); !ok { + t.Fatal("expected web_search tool to be registered") + } +} + +func TestNewAgentLoop_RegistersWebSearchTool_WhenExplicitProviderUnavailable(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Agents.Defaults.Workspace = t.TempDir() + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Brave.Enabled = true + cfg.Tools.Web.Sogou.Enabled = true + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockProvider{}) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if _, ok := agent.Tools.Get("web_search"); !ok { + t.Fatal("expected web_search tool to fall back to auto provider selection") + } +} + +func TestNewAgentLoop_DoesNotRegisterWebSearchTool_WhenNoReadyProviders(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Agents.Defaults.Workspace = t.TempDir() + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Brave.Enabled = true + cfg.Tools.Web.Sogou.Enabled = false + cfg.Tools.Web.DuckDuckGo.Enabled = false + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &mockProvider{}) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if _, ok := agent.Tools.Get("web_search"); ok { + t.Fatal("expected web_search tool to be absent when no providers are ready") + } +} + func TestProcessMessage_IncludesCurrentSenderInDynamicContext(t *testing.T) { tmpDir, err := os.MkdirTemp("", "agent-test-*") if err != nil { diff --git a/pkg/agent/pipeline_llm.go b/pkg/agent/pipeline_llm.go index 29940bc01..7b3fee208 100644 --- a/pkg/agent/pipeline_llm.go +++ b/pkg/agent/pipeline_llm.go @@ -39,10 +39,10 @@ func (p *Pipeline) CallLLM( exec.providerToolDefs = ts.agent.Tools.ToProviderDefs() // Native web search support - _, hasWebSearch := ts.agent.Tools.Get("web_search") - exec.useNativeSearch = al.cfg.Tools.Web.PreferNative && hasWebSearch && + webSearchEnabled := al.cfg.Tools.IsToolEnabled("web") + exec.useNativeSearch = webSearchEnabled && al.cfg.Tools.Web.PreferNative && func() bool { - if ns, ok := ts.agent.Provider.(interface{ SupportsNativeSearch() bool }); ok { + if ns, ok := ts.agent.Provider.(providers.NativeSearchCapable); ok { return ns.SupportsNativeSearch() } return false diff --git a/pkg/agent/turn_coord_test.go b/pkg/agent/turn_coord_test.go index 7a362a662..c059d0a39 100644 --- a/pkg/agent/turn_coord_test.go +++ b/pkg/agent/turn_coord_test.go @@ -36,6 +36,35 @@ func (p *simpleConvProvider) GetDefaultModel() string { return "simple-model" } +type nativeSearchCaptureProvider struct { + lastOpts map[string]any +} + +func (p *nativeSearchCaptureProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.lastOpts = make(map[string]any, len(opts)) + for k, v := range opts { + p.lastOpts[k] = v + } + return &providers.LLMResponse{ + Content: "Using native search", + FinishReason: "stop", + }, nil +} + +func (p *nativeSearchCaptureProvider) GetDefaultModel() string { + return "native-search-model" +} + +func (p *nativeSearchCaptureProvider) SupportsNativeSearch() bool { + return true +} + // toolCallRespProvider returns a tool call response type toolCallRespProvider struct { toolName string @@ -257,6 +286,41 @@ func TestPipeline_CallLLM_WithToolCall(t *testing.T) { } } +func TestPipeline_CallLLM_UsesNativeSearchWithoutClientWebSearchTool(t *testing.T) { + provider := &nativeSearchCaptureProvider{} + al, agent, cleanup := newTurnCoordTestLoop(t, provider) + defer cleanup() + + if _, ok := agent.Tools.Get("web_search"); ok { + t.Fatal("expected no client-side web_search tool to be registered") + } + + al.cfg.Tools.Web.Enabled = true + al.cfg.Tools.Web.PreferNative = true + + pipeline := NewPipeline(al) + ts := newTurnState(agent, makeTestProcessOpts("test-session"), turnEventScope{ + turnID: "turn-1", + context: newTurnContext(nil, nil, nil), + }) + + exec, err := pipeline.SetupTurn(context.Background(), ts) + if err != nil { + t.Fatalf("SetupTurn failed: %v", err) + } + + ctrl, err := pipeline.CallLLM(context.Background(), context.Background(), ts, exec, 1) + if err != nil { + t.Fatalf("CallLLM failed: %v", err) + } + if ctrl != ControlBreak { + t.Fatalf("expected ControlBreak, got %v", ctrl) + } + if got, _ := provider.lastOpts["native_search"].(bool); !got { + t.Fatalf("expected native_search=true, got %#v", provider.lastOpts["native_search"]) + } +} + func TestPipeline_CallLLM_TimeoutRetry(t *testing.T) { errorPrv := &errorProvider{errType: "timeout"} al, agent, cleanup := newTurnCoordTestLoop(t, errorPrv) diff --git a/pkg/tools/integration/web.go b/pkg/tools/integration/web.go index 58db34589..56663ecda 100644 --- a/pkg/tools/integration/web.go +++ b/pkg/tools/integration/web.go @@ -1113,12 +1113,147 @@ type WebSearchToolOptions struct { Proxy string } +func WebSearchToolOptionsFromConfig(cfg *config.Config) WebSearchToolOptions { + return WebSearchToolOptions{ + Provider: cfg.Tools.Web.Provider, + BraveAPIKeys: cfg.Tools.Web.Brave.APIKeys.Values(), + BraveMaxResults: cfg.Tools.Web.Brave.MaxResults, + BraveEnabled: cfg.Tools.Web.Brave.Enabled, + TavilyAPIKeys: cfg.Tools.Web.Tavily.APIKeys.Values(), + TavilyBaseURL: cfg.Tools.Web.Tavily.BaseURL, + TavilyMaxResults: cfg.Tools.Web.Tavily.MaxResults, + TavilyEnabled: cfg.Tools.Web.Tavily.Enabled, + SogouMaxResults: cfg.Tools.Web.Sogou.MaxResults, + SogouEnabled: cfg.Tools.Web.Sogou.Enabled, + DuckDuckGoMaxResults: cfg.Tools.Web.DuckDuckGo.MaxResults, + DuckDuckGoEnabled: cfg.Tools.Web.DuckDuckGo.Enabled, + PerplexityAPIKeys: cfg.Tools.Web.Perplexity.APIKeys.Values(), + PerplexityMaxResults: cfg.Tools.Web.Perplexity.MaxResults, + PerplexityEnabled: cfg.Tools.Web.Perplexity.Enabled, + SearXNGBaseURL: cfg.Tools.Web.SearXNG.BaseURL, + SearXNGMaxResults: cfg.Tools.Web.SearXNG.MaxResults, + SearXNGEnabled: cfg.Tools.Web.SearXNG.Enabled, + GLMSearchAPIKey: cfg.Tools.Web.GLMSearch.APIKey.String(), + GLMSearchBaseURL: cfg.Tools.Web.GLMSearch.BaseURL, + GLMSearchEngine: cfg.Tools.Web.GLMSearch.SearchEngine, + GLMSearchMaxResults: cfg.Tools.Web.GLMSearch.MaxResults, + GLMSearchEnabled: cfg.Tools.Web.GLMSearch.Enabled, + BaiduSearchAPIKey: cfg.Tools.Web.BaiduSearch.APIKey.String(), + BaiduSearchBaseURL: cfg.Tools.Web.BaiduSearch.BaseURL, + BaiduSearchMaxResults: cfg.Tools.Web.BaiduSearch.MaxResults, + BaiduSearchEnabled: cfg.Tools.Web.BaiduSearch.Enabled, + Proxy: cfg.Tools.Web.Proxy, + } +} + +func WebSearchProviderReady(opts WebSearchToolOptions, name string) bool { + return opts.providerReady(name) +} + +func ResolveWebSearchProviderName(opts WebSearchToolOptions, query string) (string, error) { + return opts.resolveProviderName(query) +} + +var ( + knownWebSearchProviders = []string{ + "sogou", + "duckduckgo", + "brave", + "tavily", + "perplexity", + "searxng", + "glm_search", + "baidu_search", + } + autoPrimaryWebSearchProviders = []string{"perplexity", "brave", "searxng", "tavily"} + autoFallbackWebSearchProviders = []string{"baidu_search", "glm_search"} +) + +func isKnownWebSearchProvider(name string) bool { + name = strings.ToLower(strings.TrimSpace(name)) + for _, known := range knownWebSearchProviders { + if name == known { + return true + } + } + return false +} + +func (opts WebSearchToolOptions) providerReady(name string) bool { + switch strings.ToLower(strings.TrimSpace(name)) { + case "sogou": + return opts.SogouEnabled + case "duckduckgo": + return opts.DuckDuckGoEnabled + case "brave": + return opts.BraveEnabled && len(opts.BraveAPIKeys) > 0 + case "tavily": + return opts.TavilyEnabled && len(opts.TavilyAPIKeys) > 0 + case "perplexity": + return opts.PerplexityEnabled && len(opts.PerplexityAPIKeys) > 0 + case "searxng": + return opts.SearXNGEnabled && strings.TrimSpace(opts.SearXNGBaseURL) != "" + case "glm_search": + return opts.GLMSearchEnabled && strings.TrimSpace(opts.GLMSearchAPIKey) != "" + case "baidu_search": + return opts.BaiduSearchEnabled && strings.TrimSpace(opts.BaiduSearchAPIKey) != "" + default: + return false + } +} + +func (opts WebSearchToolOptions) normalizedProviderName() string { + providerName := strings.ToLower(strings.TrimSpace(opts.Provider)) + if providerName != "" && providerName != "auto" && !isKnownWebSearchProvider(providerName) { + // Tolerate stale or manually edited config values at runtime by + // treating them as "auto" and falling back to the next ready provider. + return "auto" + } + return providerName +} + +func (opts WebSearchToolOptions) resolveProviderName(query string) (string, error) { + providerName := opts.normalizedProviderName() + if providerName != "" && providerName != "auto" && opts.providerReady(providerName) { + return providerName, nil + } + + for _, name := range autoPrimaryWebSearchProviders { + if opts.providerReady(name) { + return name, nil + } + } + + sogouReady := opts.providerReady("sogou") + duckReady := opts.providerReady("duckduckgo") + if sogouReady && duckReady { + if prefersDuckDuckGoQuery(query) { + return "duckduckgo", nil + } + return "sogou", nil + } + if sogouReady { + return "sogou", nil + } + if duckReady { + return "duckduckgo", nil + } + + for _, name := range autoFallbackWebSearchProviders { + if opts.providerReady(name) { + return name, nil + } + } + + return "", nil +} + func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, int, error) { switch strings.ToLower(strings.TrimSpace(name)) { case "", "auto": return nil, 0, nil case "sogou": - if !opts.SogouEnabled { + if !opts.providerReady("sogou") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1134,7 +1269,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "perplexity": - if !opts.PerplexityEnabled { + if !opts.providerReady("perplexity") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, perplexityTimeout) @@ -1151,7 +1286,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "brave": - if !opts.BraveEnabled { + if !opts.providerReady("brave") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1168,7 +1303,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "searxng": - if !opts.SearXNGEnabled { + if !opts.providerReady("searxng") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1185,7 +1320,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "tavily": - if !opts.TavilyEnabled { + if !opts.providerReady("tavily") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1203,7 +1338,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "duckduckgo": - if !opts.DuckDuckGoEnabled { + if !opts.providerReady("duckduckgo") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1219,7 +1354,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "baidu_search": - if !opts.BaiduSearchEnabled { + if !opts.providerReady("baidu_search") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, perplexityTimeout) @@ -1237,7 +1372,7 @@ func (opts WebSearchToolOptions) providerByName(name string) (SearchProvider, in client: client, }, maxResults, nil case "glm_search": - if !opts.GLMSearchEnabled { + if !opts.providerReady("glm_search") { return nil, 0, nil } client, err := utils.CreateHTTPClient(opts.Proxy, searchTimeout) @@ -1297,62 +1432,35 @@ func prefersDuckDuckGoQuery(text string) bool { } func (opts WebSearchToolOptions) buildProviderResolver() (func(query string) (SearchProvider, int), error) { - providerName := strings.ToLower(strings.TrimSpace(opts.Provider)) - if providerName != "" && providerName != "auto" { - provider, maxResults, err := opts.providerByName(providerName) + providersByName := make(map[string]SearchProvider, len(knownWebSearchProviders)) + maxResultsByName := make(map[string]int, len(knownWebSearchProviders)) + + for _, name := range knownWebSearchProviders { + if !opts.providerReady(name) { + continue + } + provider, maxResults, err := opts.providerByName(name) if err != nil { return nil, err } if provider == nil { - return func(string) (SearchProvider, int) { return nil, 0 }, nil + continue } - return func(string) (SearchProvider, int) { return provider, maxResults }, nil + providersByName[name] = provider + maxResultsByName[name] = maxResults } - for _, name := range []string{"perplexity", "brave", "searxng", "tavily"} { - provider, maxResults, err := opts.providerByName(name) + return func(query string) (SearchProvider, int) { + name, err := opts.resolveProviderName(query) if err != nil { - return nil, err + return nil, 0 } - if provider != nil { - return func(string) (SearchProvider, int) { return provider, maxResults }, nil + provider, ok := providersByName[name] + if !ok { + return nil, 0 } - } - - sogouProvider, sogouMaxResults, err := opts.providerByName("sogou") - if err != nil { - return nil, err - } - duckProvider, duckMaxResults, err := opts.providerByName("duckduckgo") - if err != nil { - return nil, err - } - if sogouProvider != nil && duckProvider != nil { - return func(query string) (SearchProvider, int) { - if prefersDuckDuckGoQuery(query) { - return duckProvider, duckMaxResults - } - return sogouProvider, sogouMaxResults - }, nil - } - if sogouProvider != nil { - return func(string) (SearchProvider, int) { return sogouProvider, sogouMaxResults }, nil - } - if duckProvider != nil { - return func(string) (SearchProvider, int) { return duckProvider, duckMaxResults }, nil - } - - for _, name := range []string{"baidu_search", "glm_search"} { - provider, maxResults, err := opts.providerByName(name) - if err != nil { - return nil, err - } - if provider != nil { - return func(string) (SearchProvider, int) { return provider, maxResults }, nil - } - } - - return func(string) (SearchProvider, int) { return nil, 0 }, nil + return provider, maxResultsByName[name] + }, nil } func NewWebSearchTool(opts WebSearchToolOptions) (*WebSearchTool, error) { diff --git a/pkg/tools/integration/web_test.go b/pkg/tools/integration/web_test.go index 4ad5a3468..d47d8e7c9 100644 --- a/pkg/tools/integration/web_test.go +++ b/pkg/tools/integration/web_test.go @@ -385,24 +385,14 @@ func TestWebFetchTool_PayloadTooLarge(t *testing.T) { } } -// TestWebTool_WebSearch_NoApiKey verifies missing credentials are surfaced at execution time. +// TestWebTool_WebSearch_NoApiKey verifies providers without required credentials are not registered. func TestWebTool_WebSearch_NoApiKey(t *testing.T) { tool, err := NewWebSearchTool(WebSearchToolOptions{BraveEnabled: true, BraveAPIKeys: nil}) if err != nil { t.Fatalf("Unexpected error: %v", err) } - if tool == nil { - t.Fatalf("Expected tool when Brave is enabled, even without API keys") - } - - result := tool.Execute(context.Background(), map[string]any{ - "query": "test query", - }) - if !result.IsError { - t.Fatalf("Expected missing Brave API key to return error") - } - if !strings.Contains(result.ForLLM, "no API key provided") { - t.Fatalf("Unexpected error message: %s", result.ForLLM) + if tool != nil { + t.Fatalf("Expected nil tool when only enabled provider is missing credentials") } // Also nil when nothing is enabled @@ -1878,6 +1868,94 @@ func TestWebTool_AutoProviderPrefersConfiguredProvidersBeforeSogou(t *testing.T) } } +func TestWebTool_ExplicitProviderFallsBackWhenMissingCredentials(t *testing.T) { + tool, err := NewWebSearchTool(WebSearchToolOptions{ + Provider: "brave", + BraveEnabled: true, + SogouEnabled: true, + SogouMaxResults: 5, + }) + if err != nil { + t.Fatalf("NewWebSearchTool() error: %v", err) + } + if _, ok := tool.provider.(*SogouSearchProvider); !ok { + t.Fatalf("expected SogouSearchProvider after fallback, got %T", tool.provider) + } +} + +func TestWebTool_ExplicitProviderFallsBackWhenMissingBaseURL(t *testing.T) { + tool, err := NewWebSearchTool(WebSearchToolOptions{ + Provider: "searxng", + SearXNGEnabled: true, + SogouEnabled: true, + SogouMaxResults: 5, + }) + if err != nil { + t.Fatalf("NewWebSearchTool() error: %v", err) + } + if _, ok := tool.provider.(*SogouSearchProvider); !ok { + t.Fatalf("expected SogouSearchProvider after fallback, got %T", tool.provider) + } +} + +func TestWebTool_AutoProviderSkipsEnabledButUnreadyProviders(t *testing.T) { + tool, err := NewWebSearchTool(WebSearchToolOptions{ + Provider: "auto", + BraveEnabled: true, + SogouEnabled: true, + SogouMaxResults: 5, + }) + if err != nil { + t.Fatalf("NewWebSearchTool() error: %v", err) + } + if _, ok := tool.provider.(*SogouSearchProvider); !ok { + t.Fatalf("expected SogouSearchProvider when Brave has no API key, got %T", tool.provider) + } +} + +func TestResolveWebSearchProviderName_FallsBackFromExplicitUnavailableProvider(t *testing.T) { + got, err := ResolveWebSearchProviderName(WebSearchToolOptions{ + Provider: "brave", + BraveEnabled: true, + SogouEnabled: true, + SogouMaxResults: 5, + }, "") + if err != nil { + t.Fatalf("ResolveWebSearchProviderName() error: %v", err) + } + if got != "sogou" { + t.Fatalf("ResolveWebSearchProviderName() = %q, want sogou", got) + } +} + +func TestWebTool_UnknownExplicitProviderFallsBackToAuto(t *testing.T) { + tool, err := NewWebSearchTool(WebSearchToolOptions{ + Provider: "totally_unknown", + SogouEnabled: true, + SogouMaxResults: 5, + }) + if err != nil { + t.Fatalf("NewWebSearchTool() error: %v", err) + } + if _, ok := tool.provider.(*SogouSearchProvider); !ok { + t.Fatalf("expected SogouSearchProvider after fallback, got %T", tool.provider) + } +} + +func TestResolveWebSearchProviderName_FallsBackFromUnknownProvider(t *testing.T) { + got, err := ResolveWebSearchProviderName(WebSearchToolOptions{ + Provider: "totally_unknown", + SogouEnabled: true, + SogouMaxResults: 5, + }, "") + if err != nil { + t.Fatalf("ResolveWebSearchProviderName() error: %v", err) + } + if got != "sogou" { + t.Fatalf("ResolveWebSearchProviderName() = %q, want sogou", got) + } +} + type stubSearchProvider struct { result string calls []string diff --git a/pkg/tools/integration_facade.go b/pkg/tools/integration_facade.go index 00c00b810..b05a22fe2 100644 --- a/pkg/tools/integration_facade.go +++ b/pkg/tools/integration_facade.go @@ -4,6 +4,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/sipeed/picoclaw/pkg/audio/tts" + "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/media" "github.com/sipeed/picoclaw/pkg/skills" integrationtools "github.com/sipeed/picoclaw/pkg/tools/integration" @@ -72,6 +73,18 @@ func GetPreferredWebSearchLanguage() string { return integrationtools.GetPreferredWebSearchLanguage() } +func WebSearchToolOptionsFromConfig(cfg *config.Config) WebSearchToolOptions { + return integrationtools.WebSearchToolOptionsFromConfig(cfg) +} + +func WebSearchProviderReady(opts WebSearchToolOptions, name string) bool { + return integrationtools.WebSearchProviderReady(opts, name) +} + +func ResolveWebSearchProviderName(opts WebSearchToolOptions, query string) (string, error) { + return integrationtools.ResolveWebSearchProviderName(opts, query) +} + func NewWebSearchTool(opts WebSearchToolOptions) (*WebSearchTool, error) { return integrationtools.NewWebSearchTool(opts) } diff --git a/web/backend/api/tools.go b/web/backend/api/tools.go index 0a1bb50ee..c6c2deaae 100644 --- a/web/backend/api/tools.go +++ b/web/backend/api/tools.go @@ -261,6 +261,8 @@ func buildToolSupport(cfg *config.Config) []toolSupportItem { status, reasonCode = resolveDiscoveryToolSupport(cfg, cfg.Tools.MCP.Discovery.UseRegex) case "tool_search_tool_bm25": status, reasonCode = resolveDiscoveryToolSupport(cfg, cfg.Tools.MCP.Discovery.UseBM25) + case "web_search": + status, reasonCode = resolveWebSearchToolSupport(cfg) case "i2c", "spi": status, reasonCode = resolveHardwareToolSupport(cfg.Tools.IsToolEnabled(entry.ConfigKey)) default: @@ -304,6 +306,13 @@ func resolveDiscoveryToolSupport(cfg *config.Config, methodEnabled bool) (string return "enabled", "" } +func resolveWebSearchToolSupport(cfg *config.Config) (string, string) { + if !cfg.Tools.IsToolEnabled("web") { + return "disabled", "" + } + return "enabled", "" +} + func applyToolState(cfg *config.Config, toolName string, enabled bool) error { switch toolName { case "read_file": @@ -507,6 +516,7 @@ func normalizeWebSearchAPIKeys(apiKeys []string, apiKey string) ([]string, bool) } func buildWebSearchConfigResponse(cfg *config.Config) webSearchConfigResponse { + opts := picotools.WebSearchToolOptionsFromConfig(cfg) current := resolveCurrentWebSearchProvider(cfg) settings := map[string]webSearchProviderConfig{ "sogou": { @@ -563,59 +573,53 @@ func buildWebSearchConfigResponse(cfg *config.Config) webSearchConfigResponse { { ID: "sogou", Label: "Sogou", - Configured: cfg.Tools.Web.Sogou.Enabled, + Configured: picotools.WebSearchProviderReady(opts, "sogou"), Current: current == "sogou", }, { ID: "duckduckgo", Label: "DuckDuckGo", - Configured: cfg.Tools.Web.DuckDuckGo.Enabled, + Configured: picotools.WebSearchProviderReady(opts, "duckduckgo"), Current: current == "duckduckgo", }, { - ID: "brave", - Label: "Brave Search", - Configured: cfg.Tools.Web.Brave.Enabled && - len(cfg.Tools.Web.Brave.APIKeys.Values()) > 0, + ID: "brave", + Label: "Brave Search", + Configured: picotools.WebSearchProviderReady(opts, "brave"), Current: current == "brave", RequiresAuth: true, }, { - ID: "tavily", - Label: "Tavily", - Configured: cfg.Tools.Web.Tavily.Enabled && - len(cfg.Tools.Web.Tavily.APIKeys.Values()) > 0, + ID: "tavily", + Label: "Tavily", + Configured: picotools.WebSearchProviderReady(opts, "tavily"), Current: current == "tavily", RequiresAuth: true, }, { - ID: "perplexity", - Label: "Perplexity", - Configured: cfg.Tools.Web.Perplexity.Enabled && - len(cfg.Tools.Web.Perplexity.APIKeys.Values()) > 0, + ID: "perplexity", + Label: "Perplexity", + Configured: picotools.WebSearchProviderReady(opts, "perplexity"), Current: current == "perplexity", RequiresAuth: true, }, { - ID: "searxng", - Label: "SearXNG", - Configured: cfg.Tools.Web.SearXNG.Enabled && - strings.TrimSpace(cfg.Tools.Web.SearXNG.BaseURL) != "", - Current: current == "searxng", + ID: "searxng", + Label: "SearXNG", + Configured: picotools.WebSearchProviderReady(opts, "searxng"), + Current: current == "searxng", }, { - ID: "glm_search", - Label: "GLM Search", - Configured: cfg.Tools.Web.GLMSearch.Enabled && - cfg.Tools.Web.GLMSearch.APIKey.String() != "", + ID: "glm_search", + Label: "GLM Search", + Configured: picotools.WebSearchProviderReady(opts, "glm_search"), Current: current == "glm_search", RequiresAuth: true, }, { - ID: "baidu_search", - Label: "Baidu Search", - Configured: cfg.Tools.Web.BaiduSearch.Enabled && - cfg.Tools.Web.BaiduSearch.APIKey.String() != "", + ID: "baidu_search", + Label: "Baidu Search", + Configured: picotools.WebSearchProviderReady(opts, "baidu_search"), Current: current == "baidu_search", RequiresAuth: true, }, @@ -637,57 +641,12 @@ func buildWebSearchConfigResponse(cfg *config.Config) webSearchConfigResponse { } func resolveCurrentWebSearchProvider(cfg *config.Config) string { - selected := normalizeWebSearchProvider(cfg.Tools.Web.Provider) - if selected != "" && selected != "auto" && webSearchProviderConfigured(cfg, selected) { - return selected + if cfg == nil || !cfg.Tools.IsToolEnabled("web") { + return "" } - - for _, name := range []string{"perplexity", "brave", "searxng", "tavily"} { - if webSearchProviderConfigured(cfg, name) { - return name - } - } - - if webSearchProviderConfigured(cfg, "sogou") && webSearchProviderConfigured(cfg, "duckduckgo") { - if picotools.GetPreferredWebSearchLanguage() == "en" { - return "duckduckgo" - } - return "sogou" - } - if webSearchProviderConfigured(cfg, "sogou") { - return "sogou" - } - if webSearchProviderConfigured(cfg, "duckduckgo") { - return "duckduckgo" - } - - for _, name := range []string{"baidu_search", "glm_search"} { - if webSearchProviderConfigured(cfg, name) { - return name - } - } - return "" -} - -func webSearchProviderConfigured(cfg *config.Config, name string) bool { - switch name { - case "sogou": - return cfg.Tools.Web.Sogou.Enabled - case "duckduckgo": - return cfg.Tools.Web.DuckDuckGo.Enabled - case "brave": - return cfg.Tools.Web.Brave.Enabled && len(cfg.Tools.Web.Brave.APIKeys.Values()) > 0 - case "tavily": - return cfg.Tools.Web.Tavily.Enabled && len(cfg.Tools.Web.Tavily.APIKeys.Values()) > 0 - case "perplexity": - return cfg.Tools.Web.Perplexity.Enabled && len(cfg.Tools.Web.Perplexity.APIKeys.Values()) > 0 - case "searxng": - return cfg.Tools.Web.SearXNG.Enabled && strings.TrimSpace(cfg.Tools.Web.SearXNG.BaseURL) != "" - case "glm_search": - return cfg.Tools.Web.GLMSearch.Enabled && cfg.Tools.Web.GLMSearch.APIKey.String() != "" - case "baidu_search": - return cfg.Tools.Web.BaiduSearch.Enabled && cfg.Tools.Web.BaiduSearch.APIKey.String() != "" - default: - return false + selected, err := picotools.ResolveWebSearchProviderName(picotools.WebSearchToolOptionsFromConfig(cfg), "") + if err != nil { + return "" } + return selected } diff --git a/web/backend/api/tools_test.go b/web/backend/api/tools_test.go index 5105fc1d2..ffeae9b64 100644 --- a/web/backend/api/tools_test.go +++ b/web/backend/api/tools_test.go @@ -198,6 +198,66 @@ func TestHandleUpdateToolState(t *testing.T) { } } +func TestHandleListTools_ReportsWebSearchEnabledWhenToolIsOn(t *testing.T) { + tests := []struct { + name string + preferNative bool + }{ + {name: "without prefer_native", preferNative: false}, + {name: "with prefer_native", preferNative: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.Tools.Web.PreferNative = tt.preferNative + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Sogou.Enabled = false + cfg.Tools.Web.DuckDuckGo.Enabled = false + cfg.Tools.Web.Brave.Enabled = true + cfg.Tools.Web.Brave.SetAPIKeys(nil) + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/tools", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + var resp toolSupportResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + + for _, tool := range resp.Tools { + if tool.Name != "web_search" { + continue + } + if tool.Status != "enabled" || tool.ReasonCode != "" { + t.Fatalf("web_search = %#v, want enabled with no reason code", tool) + } + return + } + + t.Fatal("expected web_search in response") + }) + } +} + func TestHandleGetWebSearchConfig(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() @@ -206,6 +266,7 @@ func TestHandleGetWebSearchConfig(t *testing.T) { if err != nil { t.Fatalf("LoadConfig() error = %v", err) } + cfg.Tools.Web.PreferNative = false cfg.Tools.Web.Provider = "sogou" cfg.Tools.Web.Sogou.Enabled = true cfg.Tools.Web.Sogou.MaxResults = 6 @@ -242,6 +303,48 @@ func TestHandleGetWebSearchConfig(t *testing.T) { } } +func TestHandleGetWebSearchConfig_DoesNotExposeNativeAsCurrentService(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.Tools.Web.PreferNative = true + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Sogou.Enabled = false + cfg.Tools.Web.DuckDuckGo.Enabled = false + cfg.Tools.Web.Brave.Enabled = true + cfg.Tools.Web.Brave.SetAPIKeys(nil) + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/tools/web-search-config", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + var resp webSearchConfigResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if !resp.PreferNative { + t.Fatal("prefer_native should remain true in response") + } + if resp.CurrentService != "" { + t.Fatalf("current_service = %q, want empty when no external provider is ready", resp.CurrentService) + } +} + func TestHandleUpdateWebSearchConfig(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() @@ -393,6 +496,27 @@ func TestResolveCurrentWebSearchProvider_PrefersConfiguredProvidersBeforeSogou(t } } +func TestResolveCurrentWebSearchProvider_FallsBackWhenExplicitProviderUnavailable(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Brave.Enabled = true + cfg.Tools.Web.Sogou.Enabled = true + + if got := resolveCurrentWebSearchProvider(cfg); got != "sogou" { + t.Fatalf("resolveCurrentWebSearchProvider() = %q, want sogou", got) + } +} + +func TestResolveCurrentWebSearchProvider_FallsBackWhenProviderIsUnknown(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Tools.Web.Provider = "totally_unknown" + cfg.Tools.Web.Sogou.Enabled = true + + if got := resolveCurrentWebSearchProvider(cfg); got != "sogou" { + t.Fatalf("resolveCurrentWebSearchProvider() = %q, want sogou", got) + } +} + func TestResolveCurrentWebSearchProvider_UsesPreferredLanguageForSogouAndDuckDuckGo(t *testing.T) { cfg := config.DefaultConfig() cfg.Tools.Web.Provider = "auto" @@ -413,3 +537,22 @@ func TestResolveCurrentWebSearchProvider_UsesPreferredLanguageForSogouAndDuckDuc t.Fatalf("resolveCurrentWebSearchProvider() = %q, want sogou", got) } } + +func TestResolveCurrentWebSearchProvider_IgnoresPreferNativeInConfigView(t *testing.T) { + cfg := config.DefaultConfig() + cfg.ModelList = []*config.ModelConfig{{ + ModelName: "custom-default", + Model: "openai/gpt-4o", + APIKeys: config.SimpleSecureStrings("sk-default"), + }} + cfg.Agents.Defaults.ModelName = "custom-default" + cfg.Tools.Web.PreferNative = true + cfg.Tools.Web.Provider = "brave" + cfg.Tools.Web.Sogou.Enabled = false + cfg.Tools.Web.DuckDuckGo.Enabled = false + cfg.Tools.Web.Brave.Enabled = true + + if got := resolveCurrentWebSearchProvider(cfg); got != "" { + t.Fatalf("resolveCurrentWebSearchProvider() = %q, want empty when only native search would be available", got) + } +} diff --git a/web/frontend/src/components/agent/tools/tool-library-tab.tsx b/web/frontend/src/components/agent/tools/tool-library-tab.tsx index 638a7be23..6bbfeb091 100644 --- a/web/frontend/src/components/agent/tools/tool-library-tab.tsx +++ b/web/frontend/src/components/agent/tools/tool-library-tab.tsx @@ -1,7 +1,8 @@ -import { IconSearch } from "@tabler/icons-react" +import { IconSearch, IconSettings } from "@tabler/icons-react" import { useTranslation } from "react-i18next" import type { ToolSupportItem } from "@/api/tools" +import { Button } from "@/components/ui/button" import { Card, CardContent } from "@/components/ui/card" import { Input } from "@/components/ui/input" import { @@ -29,6 +30,7 @@ interface ToolLibraryTabProps { pendingToolName: string | null onSearchQueryChange: (value: string) => void onStatusFilterChange: (value: ToolStatusFilter) => void + onOpenWebSearchSettings: () => void onToggleTool: (name: string, enabled: boolean) => void } @@ -43,6 +45,7 @@ export function ToolLibraryTab({ pendingToolName, onSearchQueryChange, onStatusFilterChange, + onOpenWebSearchSettings, onToggleTool, }: ToolLibraryTabProps) { const { t } = useTranslation() @@ -131,6 +134,7 @@ export function ToolLibraryTab({ key={tool.name} tool={tool} isPending={pendingToolName === tool.name} + onOpenWebSearchSettings={onOpenWebSearchSettings} onToggleTool={onToggleTool} /> ))} @@ -146,10 +150,12 @@ export function ToolLibraryTab({ function ToolCard({ tool, isPending, + onOpenWebSearchSettings, onToggleTool, }: { tool: ToolSupportItem isPending: boolean + onOpenWebSearchSettings: () => void onToggleTool: (name: string, enabled: boolean) => void }) { const { t } = useTranslation() @@ -157,8 +163,10 @@ function ToolCard({ ? t(`pages.agent.tools.reasons.${tool.reason_code}`) : "" const isEnabled = tool.status === "enabled" + const isToggledOn = tool.status !== "disabled" const isDisabled = tool.status === "disabled" const isBlocked = tool.status === "blocked" + const isWebSearchTool = tool.name === "web_search" return ( - -
+ +
-

+

{tool.name}

- onToggleTool(tool.name, checked)} - className={cn( - "shrink-0", - isEnabled && "shadow-xs ring-1 ring-emerald-500/20", +
+ {isWebSearchTool && ( + )} - /> + onToggleTool(tool.name, checked)} + className={cn( + "shrink-0", + isEnabled && "shadow-xs ring-1 ring-emerald-500/20", + )} + /> +

diff --git a/web/frontend/src/components/agent/tools/tools-page.tsx b/web/frontend/src/components/agent/tools/tools-page.tsx index c490c46ad..7d2d0fac6 100644 --- a/web/frontend/src/components/agent/tools/tools-page.tsx +++ b/web/frontend/src/components/agent/tools/tools-page.tsx @@ -1,3 +1,4 @@ +import { useLayoutEffect, useRef } from "react" import { useTranslation } from "react-i18next" import { PageHeader } from "@/components/page-header" @@ -8,9 +9,9 @@ import { WebSearchTab } from "./web-search-tab" export function ToolsPage() { const { t } = useTranslation() + const scrollContainerRef = useRef(null) const { activeTab, - currentProviderLabel, expandedProvider, groupedTools, pendingToolName, @@ -34,12 +35,19 @@ export function ToolsPage() { updateWebSearchDraft, } = useToolsPage() + useLayoutEffect(() => { + scrollContainerRef.current?.scrollTo({ top: 0 }) + }, [activeTab]) + return (

-
+
{activeTab === "library" ? ( setActiveTab("web-search")} onToggleTool={toggleTool} /> ) : ( [provider.id, provider.label])) }, [webSearchDraft]) - const currentProviderLabel = webSearchDraft?.current_service - ? (providerLabelMap.get(webSearchDraft.current_service) ?? - webSearchDraft.current_service) - : t("pages.agent.tools.web_search.none", "None") - const pendingToolName = toggleToolMutation.isPending ? (toggleToolMutation.variables?.name ?? null) : null @@ -168,7 +163,6 @@ export function useToolsPage() { return { activeTab, - currentProviderLabel, expandedProvider, groupedTools: groupedTools.groupedTools, pendingToolName, diff --git a/web/frontend/src/components/agent/tools/web-search-general-settings.tsx b/web/frontend/src/components/agent/tools/web-search-general-settings.tsx index 33d6572cf..f3c8004b5 100644 --- a/web/frontend/src/components/agent/tools/web-search-general-settings.tsx +++ b/web/frontend/src/components/agent/tools/web-search-general-settings.tsx @@ -36,7 +36,7 @@ export function WebSearchGeneralSettings({ label={t("pages.agent.tools.web_search.provider", "Primary Provider")} description={t( "pages.agent.tools.web_search.provider_description", - "Select the default search engine that agents will fallback to.", + "Select the default provider to use when the web search tool handles a request.", )} >