From f668edf35ecb4b526d5ff95b85fe480a92b78065 Mon Sep 17 00:00:00 2001 From: PhotoPortfolio Developer Date: Sun, 22 Feb 2026 20:45:44 +0800 Subject: [PATCH] Fix search extraction and Windows test stability - Support /WebAPI tool-call extraction and stripping alongside JSON wrapper format. - Make codex CLI mock tests cross-platform (Windows .cmd + Unix shell). - Stabilize auth store tests by setting effective home env per OS and skipping POSIX mode check on Windows. - Keep web-search provider-required config test explicit by disabling legacy fallbacks in the test fixture. Test results: - runTests targeted reruns: passed (4/4) - runTests full suite: passed (650/650) --- pkg/auth/store_test.go | 42 +++++---- pkg/config/config_test.go | 2 +- pkg/providers/codex_cli_provider_test.go | 47 ++++++++-- pkg/providers/tool_call_extract.go | 104 ++++++++++++++++++++++- 4 files changed, 169 insertions(+), 26 deletions(-) diff --git a/pkg/auth/store_test.go b/pkg/auth/store_test.go index d96b460a1..ecd0b059e 100644 --- a/pkg/auth/store_test.go +++ b/pkg/auth/store_test.go @@ -3,10 +3,28 @@ package auth import ( "os" "path/filepath" + "runtime" "testing" "time" ) +func setTestHome(t *testing.T, home string) { + t.Helper() + + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", home) + t.Setenv("HOMEDRIVE", filepath.VolumeName(home)) + rel := home[len(filepath.VolumeName(home)):] + if rel == "" { + rel = `\` + } + t.Setenv("HOMEPATH", rel) + return + } + + t.Setenv("HOME", home) +} + func TestAuthCredentialIsExpired(t *testing.T) { tests := []struct { name string @@ -52,9 +70,7 @@ func TestAuthCredentialNeedsRefresh(t *testing.T) { func TestStoreRoundtrip(t *testing.T) { tmpDir := t.TempDir() - origHome := os.Getenv("HOME") - t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + setTestHome(t, tmpDir) cred := &AuthCredential{ AccessToken: "test-access-token", @@ -88,10 +104,12 @@ func TestStoreRoundtrip(t *testing.T) { } func TestStoreFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file permission bits are not enforced on Windows") + } + tmpDir := t.TempDir() - origHome := os.Getenv("HOME") - t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + setTestHome(t, tmpDir) cred := &AuthCredential{ AccessToken: "secret-token", @@ -115,9 +133,7 @@ func TestStoreFilePermissions(t *testing.T) { func TestStoreMultiProvider(t *testing.T) { tmpDir := t.TempDir() - origHome := os.Getenv("HOME") - t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + setTestHome(t, tmpDir) openaiCred := &AuthCredential{AccessToken: "openai-token", Provider: "openai", AuthMethod: "oauth"} anthropicCred := &AuthCredential{AccessToken: "anthropic-token", Provider: "anthropic", AuthMethod: "token"} @@ -148,9 +164,7 @@ func TestStoreMultiProvider(t *testing.T) { func TestDeleteCredential(t *testing.T) { tmpDir := t.TempDir() - origHome := os.Getenv("HOME") - t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + setTestHome(t, tmpDir) cred := &AuthCredential{AccessToken: "to-delete", Provider: "openai", AuthMethod: "oauth"} if err := SetCredential("openai", cred); err != nil { @@ -172,9 +186,7 @@ func TestDeleteCredential(t *testing.T) { func TestLoadStoreEmpty(t *testing.T) { tmpDir := t.TempDir() - origHome := os.Getenv("HOME") - t.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", origHome) + setTestHome(t, tmpDir) store, err := LoadStore() if err != nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 13ce31923..c7e1063b8 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -401,7 +401,7 @@ func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) { 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 { + if err := os.WriteFile(configPath, []byte(`{"tools":{"web":{"search":{"provider":""},"brave":{"enabled":false},"perplexity":{"enabled":false},"duckduckgo":{"enabled":false}}}}`), 0o600); err != nil { t.Fatalf("WriteFile() error: %v", err) } diff --git a/pkg/providers/codex_cli_provider_test.go b/pkg/providers/codex_cli_provider_test.go index 7e4e1bc15..6d6f32787 100644 --- a/pkg/providers/codex_cli_provider_test.go +++ b/pkg/providers/codex_cli_provider_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -401,12 +402,23 @@ func TestCodexCliProvider_GetDefaultModel(t *testing.T) { func createMockCodexCLI(t *testing.T, events []string) string { t.Helper() tmpDir := t.TempDir() - scriptPath := filepath.Join(tmpDir, "codex") + scriptName := "codex" + if runtime.GOOS == "windows" { + scriptName = "codex.cmd" + } + scriptPath := filepath.Join(tmpDir, scriptName) var sb strings.Builder - sb.WriteString("#!/bin/bash\n") - for _, event := range events { - sb.WriteString(fmt.Sprintf("echo '%s'\n", event)) + if runtime.GOOS == "windows" { + sb.WriteString("@echo off\r\n") + for _, event := range events { + sb.WriteString(fmt.Sprintf("echo %s\r\n", event)) + } + } else { + sb.WriteString("#!/bin/bash\n") + for _, event := range events { + sb.WriteString(fmt.Sprintf("echo '%s'\n", event)) + } } if err := os.WriteFile(scriptPath, []byte(sb.String()), 0755); err != nil { @@ -473,12 +485,25 @@ func TestCodexCliProvider_MockCLI_Error(t *testing.T) { func TestCodexCliProvider_MockCLI_WithModel(t *testing.T) { // Mock script that captures args to verify model flag is passed tmpDir := t.TempDir() - scriptPath := filepath.Join(tmpDir, "codex") - script := `#!/bin/bash + scriptName := "codex" + if runtime.GOOS == "windows" { + scriptName = "codex.cmd" + } + scriptPath := filepath.Join(tmpDir, scriptName) + + var script string + if runtime.GOOS == "windows" { + script = "@echo off\r\n" + + "echo %* > \"" + filepath.Join(tmpDir, "args.txt") + "\"\r\n" + + "echo {\"type\":\"item.completed\",\"item\":{\"id\":\"1\",\"type\":\"agent_message\",\"text\":\"ok\"}}\r\n" + + "echo {\"type\":\"turn.completed\"}\r\n" + } else { + script = `#!/bin/bash # Write args to a file for verification echo "$@" > "` + filepath.Join(tmpDir, "args.txt") + `" echo '{"type":"item.completed","item":{"id":"1","type":"agent_message","text":"ok"}}' echo '{"type":"turn.completed"}'` + } if err := os.WriteFile(scriptPath, []byte(script), 0755); err != nil { t.Fatal(err) @@ -519,8 +544,16 @@ echo '{"type":"turn.completed"}'` func TestCodexCliProvider_MockCLI_ContextCancel(t *testing.T) { // Script that sleeps forever tmpDir := t.TempDir() - scriptPath := filepath.Join(tmpDir, "codex") + scriptName := "codex" + if runtime.GOOS == "windows" { + scriptName = "codex.cmd" + } + scriptPath := filepath.Join(tmpDir, scriptName) + script := "#!/bin/bash\nsleep 60" + if runtime.GOOS == "windows" { + script = "@echo off\r\ntimeout /t 60 /nobreak >nul\r\n" + } if err := os.WriteFile(scriptPath, []byte(script), 0755); err != nil { t.Fatal(err) diff --git a/pkg/providers/tool_call_extract.go b/pkg/providers/tool_call_extract.go index 97a219283..f646ebd30 100644 --- a/pkg/providers/tool_call_extract.go +++ b/pkg/providers/tool_call_extract.go @@ -9,6 +9,18 @@ import ( // Both ClaudeCliProvider and CodexCliProvider use this to extract // tool calls that the model outputs in its response text. func extractToolCallsFromText(text string) []ToolCall { + if calls := extractJSONWrapperToolCalls(text); len(calls) > 0 { + return calls + } + + if call, ok := extractWebAPIToolCall(text); ok { + return []ToolCall{call} + } + + return nil +} + +func extractJSONWrapperToolCalls(text string) []ToolCall { start := strings.Index(text, `{"tool_calls"`) if start == -1 { return nil @@ -56,17 +68,103 @@ func extractToolCallsFromText(text string) []ToolCall { return result } +func extractWebAPIToolCall(text string) (ToolCall, bool) { + start := strings.Index(text, "/WebAPI") + if start == -1 { + return ToolCall{}, false + } + + jsonStart := strings.Index(text[start:], "{") + if jsonStart == -1 { + return ToolCall{}, false + } + jsonStart += start + + jsonEnd := findMatchingBrace(text, jsonStart) + if jsonEnd == jsonStart { + return ToolCall{}, false + } + + jsonStr := text[jsonStart:jsonEnd] + + var webAPICall struct { + Name string `json:"name"` + Arguments map[string]interface{} `json:"arguments"` + } + + if err := json.Unmarshal([]byte(jsonStr), &webAPICall); err != nil { + return ToolCall{}, false + } + + if strings.TrimSpace(webAPICall.Name) == "" { + return ToolCall{}, false + } + + argBytes, err := json.Marshal(webAPICall.Arguments) + if err != nil { + argBytes = []byte("{}") + } + + return ToolCall{ + Type: "function", + Name: webAPICall.Name, + Arguments: webAPICall.Arguments, + Function: &FunctionCall{ + Name: webAPICall.Name, + Arguments: string(argBytes), + }, + }, true +} + // stripToolCallsFromText removes tool call JSON from response text. func stripToolCallsFromText(text string) string { + if stripped, ok := stripJSONWrapperToolCalls(text); ok { + return stripped + } + + if stripped, ok := stripWebAPIToolCall(text); ok { + return stripped + } + + return text +} + +func stripJSONWrapperToolCalls(text string) (string, bool) { start := strings.Index(text, `{"tool_calls"`) if start == -1 { - return text + return "", false } end := findMatchingBrace(text, start) if end == start { - return text + return "", false } - return strings.TrimSpace(text[:start] + text[end:]) + return strings.TrimSpace(text[:start] + text[end:]), true +} + +func stripWebAPIToolCall(text string) (string, bool) { + start := strings.Index(text, "/WebAPI") + if start == -1 { + return "", false + } + + endTag := strings.Index(text[start:], "") + if endTag == -1 { + jsonStart := strings.Index(text[start:], "{") + if jsonStart == -1 { + return "", false + } + jsonStart += start + + jsonEnd := findMatchingBrace(text, jsonStart) + if jsonEnd == jsonStart { + return "", false + } + + return strings.TrimSpace(text[:start] + text[jsonEnd:]), true + } + + endTag += start + len("") + return strings.TrimSpace(text[:start] + text[endTag:]), true }