From 82756fa27f13433c03690093f96ae3e3457dc2f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=BE=E6=96=87=E9=94=8B0668000834?= Date: Wed, 11 Mar 2026 10:06:41 +0800 Subject: [PATCH 1/2] fix(config): support Chinese comma separator in allow_from environment variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add UnmarshalText method to FlexibleStringSlice to support both English (,) and Chinese (，) comma separators in environment variables. Includes comprehensive unit tests covering: - English commas, Chinese commas, mixed commas - Single values, whitespace trimming - Empty strings, edge cases Fixes #1280 --- pkg/config/config.go | 26 +++++++++ pkg/config/config_test.go | 116 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/pkg/config/config.go b/pkg/config/config.go index 13d5a7306..0fdf6480f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -17,6 +17,8 @@ var rrCounter atomic.Uint64 // FlexibleStringSlice is a []string that also accepts JSON numbers, // so allow_from can contain both "123" and 123. +// It also supports parsing comma-separated strings from environment variables, +// including both English (,) and Chinese (，) commas. type FlexibleStringSlice []string func (f *FlexibleStringSlice) UnmarshalJSON(data []byte) error { @@ -48,6 +50,30 @@ func (f *FlexibleStringSlice) UnmarshalJSON(data []byte) error { return nil } +// UnmarshalText implements encoding.TextUnmarshaler to support env variable parsing. +// It handles comma-separated values with both English (,) and Chinese (，) commas. +func (f *FlexibleStringSlice) UnmarshalText(text []byte) error { + if len(text) == 0 { + *f = nil + return nil + } + + s := string(text) + // Replace Chinese comma with English comma, then split + s = strings.ReplaceAll(s, "，", ",") + parts := strings.Split(s, ",") + + result := make([]string, 0, len(parts)) + for _, part := range parts { + part = strings.TrimSpace(part) + if part != "" { + result = append(result, part) + } + } + *f = result + return nil +} + type Config struct { Agents AgentsConfig `json:"agents"` Bindings []AgentBinding `json:"bindings,omitempty"` diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 8baf3e6fd..62753621b 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -482,3 +482,119 @@ func TestDefaultConfig_WorkspacePath_WithPicoclawHome(t *testing.T) { t.Errorf("Workspace path with PICOCLAW_HOME = %q, want %q", cfg.Agents.Defaults.Workspace, want) } } + +// TestFlexibleStringSlice_UnmarshalText tests UnmarshalText with various comma separators +func TestFlexibleStringSlice_UnmarshalText(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + { + name: "English commas only", + input: "123,456,789", + expected: []string{"123", "456", "789"}, + }, + { + name: "Chinese commas only", + input: "123，456，789", + expected: []string{"123", "456", "789"}, + }, + { + name: "Mixed English and Chinese commas", + input: "123,456，789", + expected: []string{"123", "456", "789"}, + }, + { + name: "Single value", + input: "123", + expected: []string{"123"}, + }, + { + name: "Values with whitespace", + input: " 123 , 456 , 789 ", + expected: []string{"123", "456", "789"}, + }, + { + name: "Empty string", + input: "", + expected: nil, + }, + { + name: "Only commas - English", + input: ",,", + expected: []string{}, + }, + { + name: "Only commas - Chinese", + input: "，，", + expected: []string{}, + }, + { + name: "Mixed commas with empty parts", + input: "123,,456，，789", + expected: []string{"123", "456", "789"}, + }, + { + name: "Complex mixed values", + input: "user1@example.com，user2@test.com, admin@domain.org", + expected: []string{"user1@example.com", "user2@test.com", "admin@domain.org"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var f FlexibleStringSlice + err := f.UnmarshalText([]byte(tt.input)) + if err != nil { + t.Fatalf("UnmarshalText(%q) error = %v", tt.input, err) + } + + if tt.expected == nil { + if f != nil { + t.Errorf("UnmarshalText(%q) = %v, want nil", tt.input, f) + } + return + } + + if len(f) != len(tt.expected) { + t.Errorf("UnmarshalText(%q) length = %d, want %d", tt.input, len(f), len(tt.expected)) + return + } + + for i, v := range tt.expected { + if f[i] != v { + t.Errorf("UnmarshalText(%q)[%d] = %q, want %q", tt.input, i, f[i], v) + } + } + }) + } +} + +// TestFlexibleStringSlice_UnmarshalText_EmptySliceConsistency tests nil vs empty slice behavior +func TestFlexibleStringSlice_UnmarshalText_EmptySliceConsistency(t *testing.T) { + t.Run("Empty string returns nil", func(t *testing.T) { + var f FlexibleStringSlice + err := f.UnmarshalText([]byte("")) + if err != nil { + t.Fatalf("UnmarshalText error = %v", err) + } + if f != nil { + t.Errorf("Empty string should return nil, got %v", f) + } + }) + + t.Run("Commas only returns empty slice", func(t *testing.T) { + var f FlexibleStringSlice + err := f.UnmarshalText([]byte(",,,")) + if err != nil { + t.Fatalf("UnmarshalText error = %v", err) + } + if f == nil { + t.Error("Commas only should return empty slice, not nil") + } + if len(f) != 0 { + t.Errorf("Expected empty slice, got %v", f) + } + }) +} From 718274bd1511bab17b5579d8517a39607d89bbf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=BE=E6=96=87=E9=94=8B0668000834?= Date: Thu, 12 Mar 2026 20:42:43 +0800 Subject: [PATCH 2/2] fix(providers): classify transport layer errors as retriable for fallback chain Add transport layer error patterns (connection reset by peer, connection refused, no route to host, unexpected EOF, broken pipe, etc.) to the error classifier. These errors are now classified as FailoverTimeout, making them retriable and allowing the fallback chain to continue to the next model instead of aborting. Fixes #1419 --- pkg/providers/error_classifier.go | 18 +++++++++++++++ pkg/providers/error_classifier_test.go | 32 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/pkg/providers/error_classifier.go b/pkg/providers/error_classifier.go index fd9bf1e81..48b11c237 100644 --- a/pkg/providers/error_classifier.go +++ b/pkg/providers/error_classifier.go @@ -50,6 +50,21 @@ var ( substr("context deadline exceeded"), } + // Transport layer error patterns that should trigger fallback. + transportPatterns = []errorPattern{ + substr("connection reset by peer"), + substr("connection refused"), + substr("no route to host"), + substr("unexpected eof"), + substr("broken pipe"), + substr("connection closed"), + substr("connection reset"), + substr("eof"), + substr("network is unreachable"), + substr("temporary failure in name resolution"), + substr("dial tcp"), + } + billingPatterns = []errorPattern{ rxp(`\b402\b`), substr("payment required"), @@ -195,6 +210,9 @@ func classifyByMessage(msg string) FailoverReason { if matchesAny(msg, timeoutPatterns) { return FailoverTimeout } + if matchesAny(msg, transportPatterns) { + return FailoverTimeout // Transport errors treated as timeout (retriable) + } if matchesAny(msg, authPatterns) { return FailoverAuth } diff --git a/pkg/providers/error_classifier_test.go b/pkg/providers/error_classifier_test.go index 67d9af62b..8806572ef 100644 --- a/pkg/providers/error_classifier_test.go +++ b/pkg/providers/error_classifier_test.go @@ -336,3 +336,35 @@ func TestIsImageSizeError(t *testing.T) { t.Error("should not match normal error") } } + +func TestClassifyError_TransportPatterns(t *testing.T) { + patterns := []string{ + "connection reset by peer", + "connection refused", + "no route to host", + "unexpected EOF", + "broken pipe", + "connection closed", + "connection reset", + "EOF", + "network is unreachable", + "temporary failure in name resolution", + "dial tcp: lookup api.openrouter.ai: no such host", + "read tcp 10.0.0.1:12345->10.0.0.2:443: connection reset by peer", + } + + for _, msg := range patterns { + err := errors.New(msg) + result := ClassifyError(err, "openrouter", "claude-3-opus") + if result == nil { + t.Errorf("pattern %q: expected non-nil", msg) + continue + } + if result.Reason != FailoverTimeout { + t.Errorf("pattern %q: reason = %q, want timeout", msg, result.Reason) + } + if !result.IsRetriable() { + t.Errorf("pattern %q: should be retriable", msg) + } + } +}