diff --git a/config/config.example.json b/config/config.example.json index 89c554d76..780249178 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -3,7 +3,7 @@ "defaults": { "workspace": "~/.picoclaw/workspace", "restrict_to_workspace": true, - "model": "", + "model": "glm-4.7", "max_tokens": 8192, "temperature": 0.7, "max_tool_iterations": 20 diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index aa2a71905..6072e8f9e 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -70,7 +70,7 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers Param: cfg.Tools.Web.Search.QueryParam, }, { - Provider: "duckduckgo", //fallback to duckduckgo if not configured + Provider: "duckduckgo", // fallback to duckduckgo if not configured MaxResults: 5, }, } diff --git a/pkg/config/config.go b/pkg/config/config.go index 8d29dd485..3d91b0892 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "github.com/caarlos0/env/v11" @@ -304,7 +305,7 @@ type CronToolsConfig struct { type ExecConfig struct { EnableDenyPatterns bool `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"` - CustomDenyPatterns []string `json:"custom_deny_patterns,omitempty"` + CustomDenyPatterns []string `json:"custom_deny_patterns,omitempty" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"` } type ToolsConfig struct { @@ -411,12 +412,12 @@ func DefaultConfig() *Config { Tools: ToolsConfig{ Web: WebToolsConfig{ Search: WebSearchConfig{ - Provider: "ollama", + Provider: "", APIKey: "", - Endpoint: "https://ollama.com/api/web_search", - RestType: "POST", - QueryParam: "query", - MaxResults: 5, + Endpoint: "", + RestType: "", + QueryParam: "", + MaxResults: 0, }, }, Cron: CronToolsConfig{ @@ -457,6 +458,10 @@ func LoadConfig(path string) (*Config, error) { return nil, err } + if strings.TrimSpace(cfg.Tools.Web.Search.Provider) == "" { + return nil, fmt.Errorf("Please check new config for web search as config example") + } + return cfg, nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index c65f8d94d..b3d4a58fa 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -284,19 +284,24 @@ func TestDefaultConfig_Channels(t *testing.T) { } } -// TestDefaultConfig_WebTools verifies web tools config -func TestDefaultConfig_WebTools(t *testing.T) { +// TestDefaultConfig_WebTools_EmptyDefaults verifies web search defaults are intentionally empty. +func TestDefaultConfig_WebTools_EmptyDefaults(t *testing.T) { cfg := DefaultConfig() - // Verify web tools defaults - if cfg.Tools.Web.Search.MaxResults != 5 { - t.Error("Expected Search MaxResults 5, got ", cfg.Tools.Web.Search.MaxResults) + if cfg.Tools.Web.Search.MaxResults != 0 { + t.Error("Expected Search MaxResults 0, got ", cfg.Tools.Web.Search.MaxResults) } - if cfg.Tools.Web.Search.Provider == "" { - t.Error("Search provider should not be empty by default") + if cfg.Tools.Web.Search.Provider != "" { + t.Error("Search provider should be empty by default") } - if cfg.Tools.Web.Search.QueryParam == "" { - t.Error("Search query param should not be empty by default") + if cfg.Tools.Web.Search.Endpoint != "" { + t.Error("Search endpoint should be empty by default") + } + if cfg.Tools.Web.Search.RestType != "" { + t.Error("Search rest_type should be empty by default") + } + if cfg.Tools.Web.Search.QueryParam != "" { + t.Error("Search query param should be empty by default") } } @@ -364,7 +369,7 @@ func TestDefaultConfig_OpenAIWebSearchEnabled(t *testing.T) { func TestLoadConfig_OpenAIWebSearchDefaultsTrueWhenUnset(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") - if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"api_base":""}}}`), 0o600); err != nil { + if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"api_base":""}},"tools":{"web":{"search":{"provider":"brave"}}}}`), 0o600); err != nil { t.Fatalf("WriteFile() error: %v", err) } @@ -380,7 +385,7 @@ func TestLoadConfig_OpenAIWebSearchDefaultsTrueWhenUnset(t *testing.T) { func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) { dir := t.TempDir() configPath := filepath.Join(dir, "config.json") - if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"web_search":false}}}`), 0o600); err != nil { + if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"web_search":false}},"tools":{"web":{"search":{"provider":"brave"}}}}`), 0o600); err != nil { t.Fatalf("WriteFile() error: %v", err) } @@ -392,3 +397,19 @@ func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) { t.Fatal("OpenAI codex web search should be false when disabled in config file") } } + +func TestLoadConfig_WebSearchProviderRequired(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + if err := os.WriteFile(configPath, []byte(`{"tools":{"web":{"search":{"provider":""}}}}`), 0o600); err != nil { + t.Fatalf("WriteFile() error: %v", err) + } + + _, err := LoadConfig(configPath) + if err == nil { + t.Fatal("expected error when web search provider is not set") + } + if err.Error() != "Please check new config for web search as config example" { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index e930d45f4..c00b9c7c9 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -649,6 +649,13 @@ func TestRunFullMigration(t *testing.T) { "apiKey": "sk-or-migrate-test", }, }, + "tools": map[string]interface{}{ + "web": map[string]interface{}{ + "search": map[string]interface{}{ + "provider": "brave", + }, + }, + }, "channels": map[string]interface{}{ "telegram": map[string]interface{}{ "enabled": true, diff --git a/pkg/providers/tool_call_extract_test.go b/pkg/providers/tool_call_extract_test.go new file mode 100644 index 000000000..b561528eb --- /dev/null +++ b/pkg/providers/tool_call_extract_test.go @@ -0,0 +1,41 @@ +package providers + +import "testing" + +func TestExtractToolCallsFromText_WebAPIFormat(t *testing.T) { + text := `I'll search for USD/EUR. +/WebAPI +{"name": "web_search", "arguments": {"query": "current USD to EUR exchange rate"}} +` + + calls := extractToolCallsFromText(text) + if len(calls) != 1 { + t.Fatalf("len(calls) = %d, want 1", len(calls)) + } + if calls[0].Name != "web_search" { + t.Fatalf("calls[0].Name = %q, want %q", calls[0].Name, "web_search") + } + if calls[0].Arguments["query"] != "current USD to EUR exchange rate" { + t.Fatalf("query arg mismatch: %+v", calls[0].Arguments) + } + + stripped := stripToolCallsFromText(text) + if stripped == text { + t.Fatalf("expected stripped text to remove webapi tool call block") + } +} + +func TestExtractToolCallsFromText_JSONWrapperFormat(t *testing.T) { + text := `before {"tool_calls":[{"id":"call_1","type":"function","function":{"name":"web_search","arguments":"{\"query\":\"btc price\"}"}}]} after` + + calls := extractToolCallsFromText(text) + if len(calls) != 1 { + t.Fatalf("len(calls) = %d, want 1", len(calls)) + } + if calls[0].Name != "web_search" { + t.Fatalf("calls[0].Name = %q, want %q", calls[0].Name, "web_search") + } + if calls[0].Arguments["query"] != "btc price" { + t.Fatalf("query arg mismatch: %+v", calls[0].Arguments) + } +} diff --git a/pkg/tools/web.go b/pkg/tools/web.go index 236ad06c2..984209699 100644 --- a/pkg/tools/web.go +++ b/pkg/tools/web.go @@ -191,7 +191,10 @@ func (p *OllamaSearchProvider) Search(ctx context.Context, query string, count i "count": count, } - bodyJSON, _ := json.Marshal(requestBody) + bodyJSON, err := json.Marshal(requestBody) + if err != nil { + return "", fmt.Errorf("failed to encode Ollama web search request: %w", err) + } req, err := http.NewRequestWithContext(ctx, "POST", p.baseURL, strings.NewReader(string(bodyJSON))) if err != nil { @@ -337,8 +340,8 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool { }, } } - // Priority order: Brave > Ollama > DuckDuckGo - priorityOrder := []string{"brave", "ollama", "duckduckgo"} + // Priority order: Brave > Ollama > Perplexity > DuckDuckGo + priorityOrder := []string{"brave", "ollama", "perplexity", "duckduckgo"} optMap := make(map[string]WebSearchToolOptions) // Build map of enabled providers @@ -350,6 +353,7 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool { var selectedOpt *WebSearchToolOptions var provider SearchProvider + var providerErrors []string // Try providers in priority order for _, providerName := range priorityOrder { @@ -361,12 +365,26 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool { selectedOpt = &opt var err error provider, err = createProvider(&opt) + if err != nil { + providerErrors = append(providerErrors, fmt.Sprintf("%s: %v", providerName, err)) + logger.WarnCF("tool", "Web search provider initialization failed", + map[string]interface{}{ + "provider": providerName, + "error": err.Error(), + }) + } if err == nil && provider != nil { break } } if provider == nil { + if len(providerErrors) > 0 { + logger.ErrorCF("tool", "No usable web search provider after trying configured providers", + map[string]interface{}{ + "errors": strings.Join(providerErrors, "; "), + }) + } return nil } @@ -375,6 +393,12 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool { if ddgOpt, exists := optMap["duckduckgo"]; exists { if p, err := createProvider(&ddgOpt); err == nil && p != nil { fallbackProvider = p + } else if err != nil { + logger.WarnCF("tool", "Fallback web search provider initialization failed", + map[string]interface{}{ + "provider": "duckduckgo", + "error": err.Error(), + }) } } } @@ -536,6 +560,38 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]interface{} ForUser: strings.Join(summary, "\n"), } } + + preview := result + if len(preview) > 512 { + preview = preview[:512] + "..." + } + + var raw map[string]interface{} + if err := json.Unmarshal([]byte(result), &parsed); err != nil { + logger.WarnCF("tool", "Failed to parse Ollama web search response", + map[string]interface{}{ + "provider": "ollama", + "query": query, + "error": err.Error(), + "preview": preview, + }) + } else if len(parsed.Results) == 0 { + logger.WarnCF("tool", "Ollama web search response has empty results", + map[string]interface{}{ + "provider": "ollama", + "query": query, + "preview": preview, + }) + } else if err := json.Unmarshal([]byte(result), &raw); err == nil { + if _, hasResults := raw["results"]; !hasResults { + logger.WarnCF("tool", "Ollama web search response missing expected 'results' field", + map[string]interface{}{ + "provider": "ollama", + "query": query, + "preview": preview, + }) + } + } } // Default: return as-is diff --git a/pkg/tools/web_test.go b/pkg/tools/web_test.go index bb9ff0561..73a771f34 100644 --- a/pkg/tools/web_test.go +++ b/pkg/tools/web_test.go @@ -3,12 +3,29 @@ package tools import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" "testing" ) +type failingSearchProvider struct { + err error +} + +func (p *failingSearchProvider) Search(ctx context.Context, query string, count int) (string, error) { + return "", p.err +} + +type staticSearchProvider struct { + result string +} + +func (p *staticSearchProvider) Search(ctx context.Context, query string, count int) (string, error) { + return p.result, nil +} + // TestWebTool_WebFetch_Success verifies successful URL fetching func TestWebTool_WebFetch_Success(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -181,10 +198,24 @@ func TestWebTool_WebSearch_NoApiKey(t *testing.T) { t.Errorf("Expected nil tool when Brave API key is empty") } - // No providers should return nil - tool = NewWebSearchTool() + // Perplexity without API key should return nil + tool = NewWebSearchTool(WebSearchToolOptions{Provider: "perplexity", APIKey: ""}) if tool != nil { - t.Errorf("Expected nil tool when no providers are provided") + t.Errorf("Expected nil tool when Perplexity API key is empty") + } + + // No providers should default to DuckDuckGo + tool = NewWebSearchTool() + if tool == nil { + t.Errorf("Expected non-nil tool when no providers are provided (DuckDuckGo default)") + } +} + +// TestWebTool_WebSearch_PerplexityProvider verifies Perplexity provider can be selected +func TestWebTool_WebSearch_PerplexityProvider(t *testing.T) { + tool := NewWebSearchTool(WebSearchToolOptions{Provider: "perplexity", APIKey: "test-key", MaxResults: 3}) + if tool == nil { + t.Fatal("Expected non-nil tool for Perplexity provider") } } @@ -202,6 +233,101 @@ func TestWebTool_WebSearch_MissingQuery(t *testing.T) { } } +// TestWebTool_WebSearch_FallbackOnPrimaryError verifies fallback to duckduckgo when primary provider fails. +func TestWebTool_WebSearch_FallbackOnPrimaryError(t *testing.T) { + tool := &WebSearchTool{ + provider: &failingSearchProvider{err: fmt.Errorf("Ollama API error: {\"error\": \"unauthorized\"}")}, + fallback: &staticSearchProvider{result: "Results for: test query (via DuckDuckGo)\n1. Example\n https://example.com"}, + maxResults: 5, + } + + ctx := context.Background() + result := tool.Execute(ctx, map[string]interface{}{"query": "test query"}) + + if result.IsError { + t.Fatalf("Expected successful fallback result, got error: %s", result.ForLLM) + } + + if !strings.Contains(result.ForUser, "DuckDuckGo") { + t.Errorf("Expected ForUser to contain fallback provider result, got: %s", result.ForUser) + } + + if !strings.Contains(result.ForLLM, "Primary search provider failed") { + t.Errorf("Expected ForLLM to mention primary provider failure, got: %s", result.ForLLM) + } +} + +func TestWebTool_WebSearch_OllamaSummaryFormatting(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("method = %s, want %s", r.Method, http.MethodPost) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{ + "results": [ + {"title":"Result One","url":"https://example.com/1","content":"Snippet one"}, + {"title":"Result Two","url":"https://example.com/2","content":"Snippet two"} + ] + }`)) + })) + defer server.Close() + + tool := NewWebSearchTool(WebSearchToolOptions{ + Provider: "ollama", + BaseURL: server.URL, + Param: "query", + MaxResults: 5, + }) + if tool == nil { + t.Fatal("Expected non-nil tool for Ollama provider") + } + + result := tool.Execute(context.Background(), map[string]interface{}{"query": "golang"}) + if result.IsError { + t.Fatalf("Expected successful result, got error: %s", result.ForLLM) + } + + if !strings.Contains(result.ForUser, "Web search summary for: golang") { + t.Errorf("Expected summary header in ForUser, got: %s", result.ForUser) + } + if !strings.Contains(result.ForUser, "1. Result One") || !strings.Contains(result.ForUser, "https://example.com/1") { + t.Errorf("Expected first formatted result in ForUser, got: %s", result.ForUser) + } + if !strings.Contains(result.ForUser, "Snippet one") { + t.Errorf("Expected snippet content in ForUser, got: %s", result.ForUser) + } + if !strings.Contains(result.ForLLM, `"results"`) { + t.Errorf("Expected raw JSON in ForLLM, got: %s", result.ForLLM) + } +} + +func TestWebTool_WebSearch_OllamaNon2xxReturnsError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"error":"unauthorized"}`)) + })) + defer server.Close() + + tool := NewWebSearchTool(WebSearchToolOptions{ + Provider: "ollama", + BaseURL: server.URL, + Param: "query", + MaxResults: 5, + }) + if tool == nil { + t.Fatal("Expected non-nil tool for Ollama provider") + } + + result := tool.Execute(context.Background(), map[string]interface{}{"query": "golang"}) + if !result.IsError { + t.Fatalf("Expected error result on non-2xx response, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "Ollama API error") { + t.Errorf("Expected Ollama API error in ForLLM, got: %s", result.ForLLM) + } +} + // TestWebTool_WebFetch_HTMLExtraction verifies HTML text extraction func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {