From f4c364d3a152df21032928d180692c7c434eb5b0 Mon Sep 17 00:00:00 2001 From: Sean Date: Mon, 13 Apr 2026 21:44:00 +0800 Subject: [PATCH 1/2] refactor(proxy): separate runtime identity from URL canonicalization --- internal/config/canon_edge_test.go | 17 ------- internal/config/config.go | 20 ++++++-- internal/config/proxy_url.go | 30 ++++++++++-- internal/config/proxy_url_test.go | 56 ++++++++++++++++++++++ internal/proxy/proxy.go | 10 ++-- internal/proxy/proxy_test.go | 22 +++++++++ internal/proxy/reload_state_test.go | 73 +++++++++++++++++++++++++++++ 7 files changed, 199 insertions(+), 29 deletions(-) delete mode 100644 internal/config/canon_edge_test.go diff --git a/internal/config/canon_edge_test.go b/internal/config/canon_edge_test.go deleted file mode 100644 index 89bd554..0000000 --- a/internal/config/canon_edge_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package config - -import "testing" - -func TestXXXCanonTrailingSlash(t *testing.T) { - cases := []struct{ in, want string }{ - {"http://proxy:8080/", "http://proxy:8080/"}, - {"http://proxy:8080", "http://proxy:8080"}, - {"http://user:pass@proxy:8080", "http://user:pass@proxy:8080"}, - {"http://USER:PASS@PROXY:8080", "http://USER:PASS@proxy:8080"}, - {"http://proxy:8080/path", "http://proxy:8080/path"}, - } - for _, c := range cases { - got := CanonicalProxyURL(c.in) - t.Logf("CanonicalProxyURL(%q) = %q (want %q, match=%v)", c.in, got, c.want, got == c.want) - } -} diff --git a/internal/config/config.go b/internal/config/config.go index 44f5448..d8d588e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -352,12 +352,18 @@ func (g GlobalConfig) NormalizedUpstreamProxyURL() string { return strings.TrimSpace(g.UpstreamProxyURL) } -// CanonicalUpstreamProxyURL returns a canonicalized upstream proxy URL suitable -// for policy key construction and identity comparison. +// CanonicalUpstreamProxyURL returns the canonical text form of the configured +// upstream proxy URL. func (g GlobalConfig) CanonicalUpstreamProxyURL() string { return CanonicalProxyURL(g.UpstreamProxyURL) } +// EffectiveUpstreamProxyIdentity returns the runtime proxy identity used for +// policy key construction and reload identity comparison. +func (g GlobalConfig) EffectiveUpstreamProxyIdentity() string { + return EffectiveProxyIdentity(g.UpstreamProxyURL) +} + func (p Provider) NormalizedProxyMode() ProviderProxyMode { mode := strings.ToLower(strings.TrimSpace(string(p.ProxyMode))) if mode == "" { @@ -370,12 +376,18 @@ func (p Provider) NormalizedProxyURL() string { return strings.TrimSpace(p.ProxyURL) } -// CanonicalProxyURL returns a canonicalized provider proxy URL suitable -// for policy key construction and identity comparison. +// CanonicalProxyURL returns the canonical text form of the configured provider +// proxy URL. func (p Provider) CanonicalProxyURL() string { return CanonicalProxyURL(p.ProxyURL) } +// EffectiveProxyIdentity returns the runtime proxy identity used for policy key +// construction and reload identity comparison. +func (p Provider) EffectiveProxyIdentity() string { + return EffectiveProxyIdentity(p.ProxyURL) +} + func ApplyUpstreamProxySettings(global *GlobalConfig, patch UpstreamProxySettingsPatch) error { if global == nil { return nil diff --git a/internal/config/proxy_url.go b/internal/config/proxy_url.go index 435ee5e..c1ae2a3 100644 --- a/internal/config/proxy_url.go +++ b/internal/config/proxy_url.go @@ -28,10 +28,11 @@ func ParseProxyURL(raw string) (*url.URL, error) { } } -// CanonicalProxyURL returns a canonical form of the given proxy URL. +// CanonicalProxyURL returns a canonical text form of the given proxy URL. // It trims whitespace, parses and validates the URL via ParseProxyURL, and -// normalizes the result so that semantically equivalent URLs -// (e.g. differing only in host case or default port) produce identical strings. +// normalizes the result so that equivalent textual forms +// (for example differing only in host case or default port) produce identical +// strings while preserving the rest of the URL as configured. // If the URL is empty or cannot be parsed, the trimmed input is returned unchanged. func CanonicalProxyURL(raw string) string { trimmed := strings.TrimSpace(raw) @@ -49,6 +50,29 @@ func CanonicalProxyURL(raw string) string { return parsed.String() } +// EffectiveProxyIdentity returns the runtime identity used for proxy policy +// comparison and transport sharing. It normalizes the authority portion of the +// URL and discards path, query, and fragment fields because they do not affect +// which proxy endpoint the runtime connects to. +// If the URL is empty or cannot be parsed, the trimmed input is returned unchanged. +func EffectiveProxyIdentity(raw string) string { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return "" + } + parsed, err := ParseProxyURL(trimmed) + if err != nil { + return trimmed + } + parsed.Host = canonicalHost(parsed.Host, parsed.Scheme) + parsed.Path = "" + parsed.RawPath = "" + parsed.RawQuery = "" + parsed.Fragment = "" + parsed.ForceQuery = false + return parsed.String() +} + // canonicalHost lowercases the hostname and strips the port when it matches // the default port for the given scheme. func canonicalHost(host, scheme string) string { diff --git a/internal/config/proxy_url_test.go b/internal/config/proxy_url_test.go index 04702e4..8c14f19 100644 --- a/internal/config/proxy_url_test.go +++ b/internal/config/proxy_url_test.go @@ -42,6 +42,7 @@ func TestCanonicalProxyURL(t *testing.T) { {name: "userinfo with default port stripped", input: "http://user:pass@proxy.example:80", want: "http://user:pass@proxy.example"}, {name: "preserves path", input: "http://proxy.example:8080/path", want: "http://proxy.example:8080/path"}, {name: "preserves trailing slash", input: "http://proxy.example:8080/", want: "http://proxy.example:8080/"}, + {name: "preserves query", input: "http://proxy.example:8080?x=1", want: "http://proxy.example:8080?x=1"}, } for _, tt := range tests { @@ -75,6 +76,55 @@ func TestCanonicalProxyURL_Equivalence(t *testing.T) { } } +func TestEffectiveProxyIdentity(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want string + }{ + {name: "empty", input: "", want: ""}, + {name: "trims whitespace", input: " http://proxy.example:8080 ", want: "http://proxy.example:8080"}, + {name: "lowercases host", input: "http://PROXY.EXAMPLE:8080", want: "http://proxy.example:8080"}, + {name: "strips default port", input: "http://proxy.example:80", want: "http://proxy.example"}, + {name: "preserves userinfo", input: "http://User:Pass@PROXY.EXAMPLE:80", want: "http://User:Pass@proxy.example"}, + {name: "drops trailing slash", input: "http://proxy.example:8080/", want: "http://proxy.example:8080"}, + {name: "drops path", input: "http://proxy.example:8080/path", want: "http://proxy.example:8080"}, + {name: "drops query", input: "http://proxy.example:8080?x=1", want: "http://proxy.example:8080"}, + {name: "drops fragment", input: "http://proxy.example:8080#frag", want: "http://proxy.example:8080"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := EffectiveProxyIdentity(tt.input) + if got != tt.want { + t.Errorf("EffectiveProxyIdentity(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestEffectiveProxyIdentity_Equivalence(t *testing.T) { + t.Parallel() + + pairs := []struct{ a, b string }{ + {"http://proxy.example", "http://proxy.example/"}, + {"http://proxy.example", "http://proxy.example?x=1"}, + {"http://proxy.example:80", "http://proxy.example"}, + {"http://USER:PASS@PROXY.EXAMPLE:80", "http://USER:PASS@proxy.example"}, + } + + for _, p := range pairs { + ca := EffectiveProxyIdentity(p.a) + cb := EffectiveProxyIdentity(p.b) + if ca != cb { + t.Errorf("EffectiveProxyIdentity(%q) = %q, EffectiveProxyIdentity(%q) = %q; want equal", p.a, ca, p.b, cb) + } + } +} + func TestProvider_CanonicalProxyURL(t *testing.T) { t.Parallel() @@ -82,6 +132,9 @@ func TestProvider_CanonicalProxyURL(t *testing.T) { if got := p.CanonicalProxyURL(); got != "http://proxy.example" { t.Errorf("Provider.CanonicalProxyURL() = %q, want %q", got, "http://proxy.example") } + if got := p.EffectiveProxyIdentity(); got != "http://proxy.example" { + t.Errorf("Provider.EffectiveProxyIdentity() = %q, want %q", got, "http://proxy.example") + } // Normalized accessor preserves original text if got := p.NormalizedProxyURL(); got != "HTTP://PROXY.EXAMPLE:80" { t.Errorf("Provider.NormalizedProxyURL() = %q, want %q", got, "HTTP://PROXY.EXAMPLE:80") @@ -95,6 +148,9 @@ func TestGlobalConfig_CanonicalUpstreamProxyURL(t *testing.T) { if got := g.CanonicalUpstreamProxyURL(); got != "http://proxy.example" { t.Errorf("GlobalConfig.CanonicalUpstreamProxyURL() = %q, want %q", got, "http://proxy.example") } + if got := g.EffectiveUpstreamProxyIdentity(); got != "http://proxy.example" { + t.Errorf("GlobalConfig.EffectiveUpstreamProxyIdentity() = %q, want %q", got, "http://proxy.example") + } // Normalized accessor preserves original text if got := g.NormalizedUpstreamProxyURL(); got != "HTTP://PROXY.EXAMPLE:80" { t.Errorf("GlobalConfig.NormalizedUpstreamProxyURL() = %q, want %q", got, "HTTP://PROXY.EXAMPLE:80") diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index abd9c8b..8fe69cd 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -220,19 +220,19 @@ func NewRouter(cfg *config.Config) *Router { // Initialize client proxies claudeProviders := config.GetEnabledProviders(cfg.Claude) if len(claudeProviders) > 0 { - r.proxies[ClientClaude] = newClientProxyWithGlobalProxy(ClientClaude, cfg.Claude.Mode, cfg.Claude.PinnedProvider, claudeProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.CanonicalUpstreamProxyURL(), telemetryStore) + r.proxies[ClientClaude] = newClientProxyWithGlobalProxy(ClientClaude, cfg.Claude.Mode, cfg.Claude.PinnedProvider, claudeProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.EffectiveUpstreamProxyIdentity(), telemetryStore) r.proxies[ClientClaude].applyRoutingRuntimeSettings(routingCfg) } codexProviders := config.GetEnabledProviders(cfg.OpenAI) if len(codexProviders) > 0 { - r.proxies[ClientOpenAI] = newClientProxyWithGlobalProxy(ClientOpenAI, cfg.OpenAI.Mode, cfg.OpenAI.PinnedProvider, codexProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.CanonicalUpstreamProxyURL(), telemetryStore) + r.proxies[ClientOpenAI] = newClientProxyWithGlobalProxy(ClientOpenAI, cfg.OpenAI.Mode, cfg.OpenAI.PinnedProvider, codexProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.EffectiveUpstreamProxyIdentity(), telemetryStore) r.proxies[ClientOpenAI].applyRoutingRuntimeSettings(routingCfg) } geminiProviders := config.GetEnabledProviders(cfg.Gemini) if len(geminiProviders) > 0 { - r.proxies[ClientGemini] = newClientProxyWithGlobalProxy(ClientGemini, cfg.Gemini.Mode, cfg.Gemini.PinnedProvider, geminiProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.CanonicalUpstreamProxyURL(), telemetryStore) + r.proxies[ClientGemini] = newClientProxyWithGlobalProxy(ClientGemini, cfg.Gemini.Mode, cfg.Gemini.PinnedProvider, geminiProviders, durations.ReactivateAfter, durations.UpstreamIdleTimeout, durations.ResponseHeaderTimeout, cbCfg, cfg.Global.NormalizedUpstreamProxyMode(), cfg.Global.EffectiveUpstreamProxyIdentity(), telemetryStore) r.proxies[ClientGemini].applyRoutingRuntimeSettings(routingCfg) } @@ -367,7 +367,7 @@ func effectiveProviderProxyPolicy(provider config.Provider, globalMode config.Gl case config.ProviderProxyModeDirect: return upstreamProxyPolicyKey{mode: upstreamProxyPolicyDirect} case config.ProviderProxyModeCustom: - return upstreamProxyPolicyKey{mode: upstreamProxyPolicyCustom, url: provider.CanonicalProxyURL()} + return upstreamProxyPolicyKey{mode: upstreamProxyPolicyCustom, url: provider.EffectiveProxyIdentity()} default: switch globalMode { case config.GlobalUpstreamProxyModeDirect: @@ -709,7 +709,7 @@ func (r *Router) reloadProviderConfigsLocked() error { } cbCfg := normalizeCircuitBreakerConfig(newCfg.Global.CircuitBreaker) globalProxyMode := newCfg.Global.NormalizedUpstreamProxyMode() - globalProxyURL := newCfg.Global.CanonicalUpstreamProxyURL() + globalProxyURL := newCfg.Global.EffectiveUpstreamProxyIdentity() newProxies := make(map[ClientType]*ClientProxy) if ps := config.GetEnabledProviders(newCfg.Claude); len(ps) > 0 { diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index e5c3667..7d3fbd0 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -276,6 +276,28 @@ func TestNewClientProxyWithGlobalProxy_SharesHTTPClientsByEffectivePolicy(t *tes } } +func TestNewClientProxyWithGlobalProxy_SharesHTTPClientsAcrossEquivalentCustomProxyForms(t *testing.T) { + t.Parallel() + + cp := newClientProxyWithGlobalProxy(ClientOpenAI, config.ClientModeAuto, "", []config.Provider{ + {Name: "plain", BaseURL: "http://plain.example", APIKey: "k1", ProxyMode: config.ProviderProxyModeCustom, ProxyURL: "http://proxy.example", Priority: 1}, + {Name: "slash", BaseURL: "http://slash.example", APIKey: "k2", ProxyMode: config.ProviderProxyModeCustom, ProxyURL: "http://proxy.example/", Priority: 2}, + {Name: "query", BaseURL: "http://query.example", APIKey: "k3", ProxyMode: config.ProviderProxyModeCustom, ProxyURL: "http://proxy.example?x=1", Priority: 3}, + {Name: "userinfo", BaseURL: "http://userinfo.example", APIKey: "k4", ProxyMode: config.ProviderProxyModeCustom, ProxyURL: "http://USER:PASS@PROXY.EXAMPLE:80/path", Priority: 4}, + {Name: "userinfo-plain", BaseURL: "http://userinfo-plain.example", APIKey: "k5", ProxyMode: config.ProviderProxyModeCustom, ProxyURL: "http://USER:PASS@proxy.example", Priority: 5}, + }, time.Hour, 0, testResponseHeaderTimeout, circuitBreakerConfig{}, config.GlobalUpstreamProxyModeEnvironment, "") + + if cp.upstreamHTTPClient(0) != cp.upstreamHTTPClient(1) || cp.upstreamHTTPClient(0) != cp.upstreamHTTPClient(2) { + t.Fatalf("equivalent authority-only custom proxy forms should share the same client") + } + if cp.upstreamHTTPClient(3) != cp.upstreamHTTPClient(4) { + t.Fatalf("equivalent custom proxy forms with matching userinfo should share the same client") + } + if cp.upstreamHTTPClient(0) == cp.upstreamHTTPClient(3) { + t.Fatalf("different proxy identities should not share the same client") + } +} + func TestNewClientProxy_UsesCustomSocksProxy(t *testing.T) { t.Parallel() diff --git a/internal/proxy/reload_state_test.go b/internal/proxy/reload_state_test.go index 48a3b8b..8d38d3a 100644 --- a/internal/proxy/reload_state_test.go +++ b/internal/proxy/reload_state_test.go @@ -410,6 +410,79 @@ func TestReloadProviderConfigsLocked_PreservesRuntimeStateAcrossEquivalentGlobal } } +func TestReloadProviderConfigsLocked_PreservesRuntimeStateAcrossEquivalentProviderCustomProxyForms(t *testing.T) { + dir := t.TempDir() + global := config.DefaultGlobalConfig() + global.ListenAddr = "127.0.0.1" + global.Port = 3333 + + codex := config.ClientConfig{ + Mode: config.ClientModeAuto, + Providers: []config.Provider{ + { + Name: "p1", + BaseURL: "https://p1.example", + APIKey: "k1", + Priority: 1, + ProxyMode: config.ProviderProxyModeCustom, + ProxyURL: "http://proxy.example", + }, + }, + } + writeProxyReloadFixture(t, dir, global, codex) + + cfg, err := config.Load(dir) + if err != nil { + t.Fatalf("config.Load: %v", err) + } + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate: %v", err) + } + + router := NewRouter(cfg) + oldProxy := router.proxies[ClientOpenAI] + now := time.Now() + + oldProxy.deactivated[0] = providerDeactivation{ + at: now.Add(-time.Second), + until: now.Add(30 * time.Second), + reason: "rate_limit", + status: http.StatusTooManyRequests, + message: "slow down", + } + oldProxy.keyDeactivated[0][0] = providerDeactivation{ + at: now.Add(-time.Second), + until: now.Add(20 * time.Second), + reason: "rate_limit", + status: http.StatusTooManyRequests, + message: "key cooldown", + } + oldProxy.breakers[0].state = circuitOpen + oldProxy.breakers[0].openedAt = now.Add(-5 * time.Second) + + codex.Providers[0].ProxyURL = "HTTP://PROXY.EXAMPLE:80/?ignored=1" + writeProxyReloadFixture(t, dir, global, codex) + + if err := router.reloadProviderConfigsLocked(); err != nil { + t.Fatalf("reloadProviderConfigsLocked: %v", err) + } + + newProxy := router.proxies[ClientOpenAI] + wantPolicy := upstreamProxyPolicyKey{mode: upstreamProxyPolicyCustom, url: "http://proxy.example"} + if newProxy.providerProxyPolicies[0] != wantPolicy { + t.Fatalf("proxy policy = %#v, want %#v", newProxy.providerProxyPolicies[0], wantPolicy) + } + if newProxy.deactivated[0].reason != "rate_limit" || newProxy.deactivated[0].message != "slow down" { + t.Fatalf("deactivation = %#v", newProxy.deactivated[0]) + } + if newProxy.keyDeactivated[0][0].message != "key cooldown" { + t.Fatalf("key deactivation = %#v", newProxy.keyDeactivated[0][0]) + } + if newProxy.breakers[0].state != circuitOpen { + t.Fatalf("breaker state = %s, want open", newProxy.breakers[0].state) + } +} + func TestReloadProviderConfigsLocked_DoesNotPreserveSuppressionStateWhenBaseURLChanges(t *testing.T) { router, dir := newReloadTestRouter(t) oldProxy := router.proxies[ClientOpenAI] From d695772b10eeeb6a612ee32a8cba2401e99ae152 Mon Sep 17 00:00:00 2001 From: Sean Date: Mon, 13 Apr 2026 22:12:25 +0800 Subject: [PATCH 2/2] chore(ci): fix lint warnings and bump Go patch version --- go.mod | 2 +- internal/telemetry/store.go | 2 +- internal/telemetry/usage_extractor.go | 4 +--- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 72643b8..aa10a81 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/lansespirit/Clipal -go 1.25.8 +go 1.25.9 require ( github.com/gen2brain/beeep v0.11.2 diff --git a/internal/telemetry/store.go b/internal/telemetry/store.go index 2a895d7..93e4652 100644 --- a/internal/telemetry/store.go +++ b/internal/telemetry/store.go @@ -139,7 +139,7 @@ func (s *Store) Record(clientType string, provider string, snapshot UsageSnapsho return nil } - delta := snapshot.UsageDelta.normalized() + delta := snapshot.normalized() if when.IsZero() { when = time.Now() } diff --git a/internal/telemetry/usage_extractor.go b/internal/telemetry/usage_extractor.go index 3f6110a..ffc98b0 100644 --- a/internal/telemetry/usage_extractor.go +++ b/internal/telemetry/usage_extractor.go @@ -238,9 +238,7 @@ func (e *UsageExtractor) processSSELine(line string) { e.eventName = strings.TrimSpace(strings.TrimPrefix(line, "event:")) case strings.HasPrefix(line, "data:"): data := strings.TrimPrefix(line, "data:") - if strings.HasPrefix(data, " ") { - data = data[1:] - } + data = strings.TrimPrefix(data, " ") e.dataLines = append(e.dataLines, data) } }