From 0d23f7915169d52ce7cd2cf52b1ec3ecd5a2079e Mon Sep 17 00:00:00 2001 From: Supra4E8C Date: Mon, 6 Jul 2026 00:14:46 +0800 Subject: [PATCH] fix(auth): keep jittered cooldown waits within max-retry-interval shouldRetryAfterError approves a retry using the pre-jitter wait, so the jitter added in waitForCooldown could push the actual sleep up to 2s past the configured max-retry-interval ceiling. Clamp the jitter range to the remaining headroom below maxWait so the documented maximum stays a hard bound; waits at the ceiling simply sleep unjittered. --- sdk/cliproxy/auth/conductor.go | 19 +++++++++----- sdk/cliproxy/auth/cooldown_backoff_test.go | 30 ++++++++++++++++------ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/sdk/cliproxy/auth/conductor.go b/sdk/cliproxy/auth/conductor.go index 02f1cc894..842ef7ca0 100644 --- a/sdk/cliproxy/auth/conductor.go +++ b/sdk/cliproxy/auth/conductor.go @@ -2288,7 +2288,7 @@ func (m *Manager) Execute(ctx context.Context, providers []string, req cliproxye if !shouldRetry { break } - if errWait := waitForCooldown(ctx, wait); errWait != nil { + if errWait := waitForCooldown(ctx, wait, maxWait); errWait != nil { return cliproxyexecutor.Response{}, errWait } } @@ -2325,7 +2325,7 @@ func (m *Manager) ExecuteCount(ctx context.Context, providers []string, req clip if !shouldRetry { break } - if errWait := waitForCooldown(ctx, wait); errWait != nil { + if errWait := waitForCooldown(ctx, wait, maxWait); errWait != nil { return cliproxyexecutor.Response{}, errWait } } @@ -2356,7 +2356,7 @@ func (m *Manager) ExecuteStream(ctx context.Context, providers []string, req cli if !shouldRetry { break } - if errWait := waitForCooldown(ctx, wait); errWait != nil { + if errWait := waitForCooldown(ctx, wait, maxWait); errWait != nil { return nil, errWait } } @@ -3485,8 +3485,10 @@ const cooldownWaitJitterCap = 2 * time.Second // jitteredCooldownWait adds a small random delay to a cooldown wait so // concurrent requests waiting on the same recovery deadline do not wake in -// lockstep and stampede the first credential that recovers. -func jitteredCooldownWait(wait time.Duration) time.Duration { +// lockstep and stampede the first credential that recovers. The jitter never +// pushes the total wait past maxWait, which callers have already enforced as +// the retry ceiling; maxWait <= 0 means no ceiling. +func jitteredCooldownWait(wait, maxWait time.Duration) time.Duration { if wait <= 0 { return wait } @@ -3494,17 +3496,20 @@ func jitteredCooldownWait(wait time.Duration) time.Duration { if jitterRange > cooldownWaitJitterCap { jitterRange = cooldownWaitJitterCap } + if maxWait > 0 && jitterRange > maxWait-wait { + jitterRange = maxWait - wait + } if jitterRange <= 0 { return wait } return wait + rand.N(jitterRange) } -func waitForCooldown(ctx context.Context, wait time.Duration) error { +func waitForCooldown(ctx context.Context, wait, maxWait time.Duration) error { if wait <= 0 { return nil } - timer := time.NewTimer(jitteredCooldownWait(wait)) + timer := time.NewTimer(jitteredCooldownWait(wait, maxWait)) defer timer.Stop() select { case <-ctx.Done(): diff --git a/sdk/cliproxy/auth/cooldown_backoff_test.go b/sdk/cliproxy/auth/cooldown_backoff_test.go index ffe0f6cea..b1d77ebce 100644 --- a/sdk/cliproxy/auth/cooldown_backoff_test.go +++ b/sdk/cliproxy/auth/cooldown_backoff_test.go @@ -155,27 +155,41 @@ func TestApplyAuthFailureStateQuotaBackoffOncePerWindow(t *testing.T) { func TestJitteredCooldownWaitBounds(t *testing.T) { cases := []struct { wait time.Duration + maxWait time.Duration maxJitter time.Duration }{ - {time.Second, 250 * time.Millisecond}, - {8 * time.Second, 2 * time.Second}, - {30 * time.Second, 2 * time.Second}, + {time.Second, 0, 250 * time.Millisecond}, + {8 * time.Second, 0, 2 * time.Second}, + {30 * time.Second, 0, 2 * time.Second}, + {time.Second, 30 * time.Second, 250 * time.Millisecond}, + {29 * time.Second, 30 * time.Second, time.Second}, } for _, tc := range cases { for i := 0; i < 200; i++ { - got := jitteredCooldownWait(tc.wait) + got := jitteredCooldownWait(tc.wait, tc.maxWait) if got < tc.wait || got >= tc.wait+tc.maxJitter { - t.Fatalf("jitteredCooldownWait(%v) = %v, want in [%v, %v)", tc.wait, got, tc.wait, tc.wait+tc.maxJitter) + t.Fatalf("jitteredCooldownWait(%v, %v) = %v, want in [%v, %v)", tc.wait, tc.maxWait, got, tc.wait, tc.wait+tc.maxJitter) + } + if tc.maxWait > 0 && got > tc.maxWait { + t.Fatalf("jitteredCooldownWait(%v, %v) = %v exceeds maxWait", tc.wait, tc.maxWait, got) } } } - if got := jitteredCooldownWait(0); got != 0 { + + // maxWait is a hard ceiling: zero headroom disables jitter entirely. + for i := 0; i < 50; i++ { + if got := jitteredCooldownWait(30*time.Second, 30*time.Second); got != 30*time.Second { + t.Fatalf("expected wait at maxWait to stay unjittered, got %v", got) + } + } + + if got := jitteredCooldownWait(0, time.Minute); got != 0 { t.Fatalf("expected zero wait to stay zero, got %v", got) } - if got := jitteredCooldownWait(-time.Second); got != -time.Second { + if got := jitteredCooldownWait(-time.Second, time.Minute); got != -time.Second { t.Fatalf("expected negative wait to pass through, got %v", got) } - if got := jitteredCooldownWait(3); got != 3 { + if got := jitteredCooldownWait(3, 0); got != 3 { t.Fatalf("expected sub-4ns wait to stay unchanged, got %v", got) } }