From 7598cddcec4f869b01997464ecc56462e9183b74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:03:10 +0000 Subject: [PATCH 01/11] Initial plan From 46fd00f062ff8489d9c41326ead92031ea1d8e2a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:18:59 +0000 Subject: [PATCH 02/11] fix: make checkAndWaitForRateLimit context-cancellable Replace the 4 bare time.Sleep calls in checkAndWaitForRateLimit with a new sleepWithContext helper that selects on both the timer and ctx.Done(). Add ctx context.Context as the first parameter and update the call site in logs_orchestrator.go to pass activeCtx so that Ctrl-C or a deadline wakes the rate-limit wait immediately. Also propagate context errors from checkAndWaitForRateLimit in the orchestrator loop so the top-of-loop cancellation check handles the graceful shutdown path. Closes #43812 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_orchestrator.go | 7 ++++++- pkg/cli/logs_rate_limit.go | 33 ++++++++++++++++++++++------- pkg/cli/logs_rate_limit_test.go | 37 +++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 1d3742eb61d..5a26804cd56 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -367,7 +367,12 @@ outerLoop: // always sleeps at least APICallCooldown but will also block until the // reset window when the remaining budget is nearly exhausted. if iteration > 0 { - if rlErr := checkAndWaitForRateLimit(verbose); rlErr != nil { + if rlErr := checkAndWaitForRateLimit(activeCtx, verbose); rlErr != nil { + if errors.Is(rlErr, context.Canceled) || errors.Is(rlErr, context.DeadlineExceeded) { + // Context was cancelled or timed out during the rate-limit wait; + // let the top-of-loop check handle the graceful exit path. + continue + } logsOrchestratorLog.Printf("Rate limit check failed (using static cooldown): %v", rlErr) } } diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 592732d545a..5f9f0fbc330 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -11,6 +11,7 @@ package cli import ( + "context" "encoding/json" "fmt" "os" @@ -70,21 +71,40 @@ func fetchRateLimit() (rateLimitResource, error) { return resp.Resources.Core, nil } +// sleepWithContext pauses for duration d and returns nil when the timer fires. +// If ctx is cancelled before the timer expires, it stops the timer and returns +// ctx.Err() so callers can propagate cancellation immediately. +func sleepWithContext(ctx context.Context, d time.Duration) error { + timer := time.NewTimer(d) + select { + case <-timer.C: + return nil + case <-ctx.Done(): + timer.Stop() + return ctx.Err() + } +} + // checkAndWaitForRateLimit queries the GitHub API rate limit and sleeps until // the reset window when the remaining core request budget falls at or below // RateLimitThreshold. It always waits at least APICallCooldown between // successive calls so that even when requests are plentiful the orchestrator // does not hammer the API. // +// ctx is checked on every sleep so that a user cancellation (Ctrl-C) or +// deadline expiry wakes the function early and propagates the context error. +// // If the rate limit cannot be fetched (e.g. network error) the function falls // back to the static APICallCooldown sleep and returns the error so callers // can decide whether to surface it. -func checkAndWaitForRateLimit(verbose bool) error { +func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { rl, err := fetchRateLimit() if err != nil { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) - time.Sleep(APICallCooldown) + if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { + return sleepErr + } return err } @@ -94,8 +114,7 @@ func checkAndWaitForRateLimit(verbose bool) error { if waitDur <= 0 { // Reset has already passed; apply minimal cooldown and carry on. logsRateLimitLog.Print("Rate limit reset has already passed, applying minimal cooldown") - time.Sleep(APICallCooldown) - return nil + return sleepWithContext(ctx, APICallCooldown) } // Add a small buffer so we don't resume right on the boundary. @@ -107,8 +126,7 @@ func checkAndWaitForRateLimit(verbose bool) error { ) fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) logsRateLimitLog.Printf("Sleeping for rate limit reset: duration=%s", waitDur) - time.Sleep(waitDur) - return nil + return sleepWithContext(ctx, waitDur) } if verbose { @@ -118,6 +136,5 @@ func checkAndWaitForRateLimit(verbose bool) error { } // Even when budget is healthy, apply the minimum inter-call cooldown. - time.Sleep(APICallCooldown) - return nil + return sleepWithContext(ctx, APICallCooldown) } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index cab86429a6f..9439a63cdde 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "encoding/json" "strconv" "testing" @@ -83,3 +84,39 @@ func TestRateLimitResourceIsBelowThreshold(t *testing.T) { func jsonInt(n int64) string { return strconv.FormatInt(n, 10) } + +// TestSleepWithContextCancellation verifies that sleepWithContext returns ctx.Err() +// immediately when the context is cancelled before the timer fires. +func TestSleepWithContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately + + start := time.Now() + err := sleepWithContext(ctx, 10*time.Second) + elapsed := time.Since(start) + + require.ErrorIs(t, err, context.Canceled) + assert.Less(t, elapsed, time.Second, "sleepWithContext should return quickly when context is already cancelled") +} + +// TestSleepWithContextDeadlineExceeded verifies that sleepWithContext respects a +// deadline that expires before the sleep duration. +func TestSleepWithContextDeadlineExceeded(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + defer cancel() + + start := time.Now() + err := sleepWithContext(ctx, 10*time.Second) + elapsed := time.Since(start) + + require.ErrorIs(t, err, context.DeadlineExceeded) + assert.Less(t, elapsed, time.Second, "sleepWithContext should return as soon as the deadline expires") +} + +// TestSleepWithContextTimerFires verifies that sleepWithContext returns nil when the +// timer fires before context cancellation. +func TestSleepWithContextTimerFires(t *testing.T) { + ctx := context.Background() + err := sleepWithContext(ctx, time.Millisecond) + require.NoError(t, err, "sleepWithContext should return nil when timer fires normally") +} From 27b1fd4d635b3183e5a5263832d96a282f8907c3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:20:06 +0000 Subject: [PATCH 03/11] fix: use defer timer.Stop() in sleepWithContext for clean resource handling Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 5f9f0fbc330..c42cf435cc6 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -76,11 +76,11 @@ func fetchRateLimit() (rateLimitResource, error) { // ctx.Err() so callers can propagate cancellation immediately. func sleepWithContext(ctx context.Context, d time.Duration) error { timer := time.NewTimer(d) + defer timer.Stop() select { case <-timer.C: return nil case <-ctx.Done(): - timer.Stop() return ctx.Err() } } From d518cf3819037607f190963233fe4cb4204b5c86 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:36:00 +0000 Subject: [PATCH 04/11] fix: harden rate-limit wait cancellation handling Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_orchestrator.go | 5 +-- pkg/cli/logs_rate_limit.go | 15 +++++++-- pkg/cli/logs_rate_limit_test.go | 55 +++++++++++++++++++++++++++++++-- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index 5a26804cd56..f8a651bf7df 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -369,8 +369,9 @@ outerLoop: if iteration > 0 { if rlErr := checkAndWaitForRateLimit(activeCtx, verbose); rlErr != nil { if errors.Is(rlErr, context.Canceled) || errors.Is(rlErr, context.DeadlineExceeded) { - // Context was cancelled or timed out during the rate-limit wait; - // let the top-of-loop check handle the graceful exit path. + // Context was cancelled or timed out during the rate-limit wait. + // Use continue (not break) so the top-of-loop activeCtx.Done() path + // preserves Canceled vs DeadlineExceeded behavior. continue } logsOrchestratorLog.Printf("Rate limit check failed (using static cooldown): %v", rlErr) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index c42cf435cc6..6bdc15a8fef 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -13,6 +13,7 @@ package cli import ( "context" "encoding/json" + "errors" "fmt" "os" "time" @@ -23,6 +24,7 @@ import ( ) var logsRateLimitLog = logger.New("cli:logs_rate_limit") +var fetchRateLimitFunc = fetchRateLimit // rateLimitResponse models the JSON returned by `gh api rate_limit`. // Only the "core" resource bucket is used because log downloads and @@ -75,6 +77,15 @@ func fetchRateLimit() (rateLimitResource, error) { // If ctx is cancelled before the timer expires, it stops the timer and returns // ctx.Err() so callers can propagate cancellation immediately. func sleepWithContext(ctx context.Context, d time.Duration) error { + if ctx == nil { + ctx = context.Background() + } + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + timer := time.NewTimer(d) defer timer.Stop() select { @@ -98,12 +109,12 @@ func sleepWithContext(ctx context.Context, d time.Duration) error { // back to the static APICallCooldown sleep and returns the error so callers // can decide whether to surface it. func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { - rl, err := fetchRateLimit() + rl, err := fetchRateLimitFunc() if err != nil { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return sleepErr + return fmt.Errorf("rate-limit fetch failed and cooldown wait was interrupted: %w", errors.Join(err, sleepErr)) } return err } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index 9439a63cdde..e7d075cd022 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -5,6 +5,7 @@ package cli import ( "context" "encoding/json" + stderrors "errors" "strconv" "testing" "time" @@ -96,7 +97,7 @@ func TestSleepWithContextCancellation(t *testing.T) { elapsed := time.Since(start) require.ErrorIs(t, err, context.Canceled) - assert.Less(t, elapsed, time.Second, "sleepWithContext should return quickly when context is already cancelled") + assert.Less(t, elapsed, 100*time.Millisecond, "sleepWithContext should return quickly when context is already cancelled") } // TestSleepWithContextDeadlineExceeded verifies that sleepWithContext respects a @@ -110,13 +111,61 @@ func TestSleepWithContextDeadlineExceeded(t *testing.T) { elapsed := time.Since(start) require.ErrorIs(t, err, context.DeadlineExceeded) - assert.Less(t, elapsed, time.Second, "sleepWithContext should return as soon as the deadline expires") + assert.Less(t, elapsed, 200*time.Millisecond, "sleepWithContext should return as soon as the deadline expires") } // TestSleepWithContextTimerFires verifies that sleepWithContext returns nil when the // timer fires before context cancellation. func TestSleepWithContextTimerFires(t *testing.T) { ctx := context.Background() - err := sleepWithContext(ctx, time.Millisecond) + start := time.Now() + err := sleepWithContext(ctx, 5*time.Millisecond) + elapsed := time.Since(start) require.NoError(t, err, "sleepWithContext should return nil when timer fires normally") + assert.Less(t, elapsed, time.Second, "timer should have fired and returned promptly") +} + +func TestSleepWithContextNilContext(t *testing.T) { + var nilCtx context.Context + err := sleepWithContext(nilCtx, 2*time.Millisecond) + require.NoError(t, err, "nil context should fall back to background context") +} + +func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { + oldFetchRateLimitFunc := fetchRateLimitFunc + fetchRateLimitFunc = func() (rateLimitResource, error) { + return rateLimitResource{ + Limit: 5000, + Remaining: 0, + Reset: time.Now().Add(10 * time.Minute).Unix(), + }, nil + } + t.Cleanup(func() { fetchRateLimitFunc = oldFetchRateLimitFunc }) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + err := checkAndWaitForRateLimit(ctx, false) + elapsed := time.Since(start) + + require.ErrorIs(t, err, context.Canceled) + assert.Less(t, elapsed, 100*time.Millisecond, "cancelled context should interrupt rate-limit wait promptly") +} + +func TestCheckAndWaitForRateLimitFetchErrorAndCancelledContext(t *testing.T) { + oldFetchRateLimitFunc := fetchRateLimitFunc + fetchErr := stderrors.New("fetch failure") + fetchRateLimitFunc = func() (rateLimitResource, error) { + return rateLimitResource{}, fetchErr + } + t.Cleanup(func() { fetchRateLimitFunc = oldFetchRateLimitFunc }) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := checkAndWaitForRateLimit(ctx, false) + require.Error(t, err) + require.ErrorIs(t, err, fetchErr) + require.ErrorIs(t, err, context.Canceled) } From 9cd56890fa49ac1b53cd048103c1b6c6cbe69092 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:39:15 +0000 Subject: [PATCH 05/11] test: clarify expected fetch error naming Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index e7d075cd022..d0c1cda7792 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -155,9 +155,9 @@ func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { func TestCheckAndWaitForRateLimitFetchErrorAndCancelledContext(t *testing.T) { oldFetchRateLimitFunc := fetchRateLimitFunc - fetchErr := stderrors.New("fetch failure") + expectedFetchErr := stderrors.New("fetch failure") fetchRateLimitFunc = func() (rateLimitResource, error) { - return rateLimitResource{}, fetchErr + return rateLimitResource{}, expectedFetchErr } t.Cleanup(func() { fetchRateLimitFunc = oldFetchRateLimitFunc }) @@ -166,6 +166,6 @@ func TestCheckAndWaitForRateLimitFetchErrorAndCancelledContext(t *testing.T) { err := checkAndWaitForRateLimit(ctx, false) require.Error(t, err) - require.ErrorIs(t, err, fetchErr) + require.ErrorIs(t, err, expectedFetchErr) require.ErrorIs(t, err, context.Canceled) } From 0b24dc2d308aa64e03489007375988813e79047b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:42:25 +0000 Subject: [PATCH 06/11] test: tighten nil-context and naming consistency Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index d0c1cda7792..e1b2558f4aa 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -127,8 +127,11 @@ func TestSleepWithContextTimerFires(t *testing.T) { func TestSleepWithContextNilContext(t *testing.T) { var nilCtx context.Context + start := time.Now() err := sleepWithContext(nilCtx, 2*time.Millisecond) + elapsed := time.Since(start) require.NoError(t, err, "nil context should fall back to background context") + assert.Less(t, elapsed, time.Second, "nil context should not block longer than timer duration") } func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { @@ -153,7 +156,7 @@ func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { assert.Less(t, elapsed, 100*time.Millisecond, "cancelled context should interrupt rate-limit wait promptly") } -func TestCheckAndWaitForRateLimitFetchErrorAndCancelledContext(t *testing.T) { +func TestCheckAndWaitForRateLimitFetchErrorAndCanceledContext(t *testing.T) { oldFetchRateLimitFunc := fetchRateLimitFunc expectedFetchErr := stderrors.New("fetch failure") fetchRateLimitFunc = func() (rateLimitResource, error) { From 34ffadc2954b75d210b45f8670c09b57bc979fb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:45:53 +0000 Subject: [PATCH 07/11] fix: refine cancellation test thresholds and error wording Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- pkg/cli/logs_rate_limit_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 6bdc15a8fef..34f12613773 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -114,7 +114,7 @@ func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return fmt.Errorf("rate-limit fetch failed and cooldown wait was interrupted: %w", errors.Join(err, sleepErr)) + return fmt.Errorf("rate-limit fetch failed and cooldown wait encountered a context error: %w", errors.Join(err, sleepErr)) } return err } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index e1b2558f4aa..055edeb1425 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -97,7 +97,7 @@ func TestSleepWithContextCancellation(t *testing.T) { elapsed := time.Since(start) require.ErrorIs(t, err, context.Canceled) - assert.Less(t, elapsed, 100*time.Millisecond, "sleepWithContext should return quickly when context is already cancelled") + assert.Less(t, elapsed, 500*time.Millisecond, "sleepWithContext should return quickly when context is already cancelled") } // TestSleepWithContextDeadlineExceeded verifies that sleepWithContext respects a @@ -111,7 +111,7 @@ func TestSleepWithContextDeadlineExceeded(t *testing.T) { elapsed := time.Since(start) require.ErrorIs(t, err, context.DeadlineExceeded) - assert.Less(t, elapsed, 200*time.Millisecond, "sleepWithContext should return as soon as the deadline expires") + assert.Less(t, elapsed, time.Second, "sleepWithContext should return as soon as the deadline expires") } // TestSleepWithContextTimerFires verifies that sleepWithContext returns nil when the From 14aea99faf99c480634a18ef08056b6d35b7f588 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:49:20 +0000 Subject: [PATCH 08/11] fix: simplify joined cancellation error wording Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 34f12613773..79beab106a6 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -114,7 +114,7 @@ func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return fmt.Errorf("rate-limit fetch failed and cooldown wait encountered a context error: %w", errors.Join(err, sleepErr)) + return fmt.Errorf("rate-limit fetch failed during context cancellation: %w", errors.Join(err, sleepErr)) } return err } From 9b495f80558e1845d93df792f80e22bd2d602a1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:52:46 +0000 Subject: [PATCH 09/11] test: strengthen nil-context fallback assertions Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- pkg/cli/logs_rate_limit_test.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 79beab106a6..6fce7dc969a 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -114,7 +114,7 @@ func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return fmt.Errorf("rate-limit fetch failed during context cancellation: %w", errors.Join(err, sleepErr)) + return fmt.Errorf("rate-limit fetch failed during context termination: %w", errors.Join(err, sleepErr)) } return err } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index 055edeb1425..63c1bcdad20 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -132,6 +132,10 @@ func TestSleepWithContextNilContext(t *testing.T) { elapsed := time.Since(start) require.NoError(t, err, "nil context should fall back to background context") assert.Less(t, elapsed, time.Second, "nil context should not block longer than timer duration") + + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + require.ErrorIs(t, sleepWithContext(canceledCtx, 2*time.Millisecond), context.Canceled) } func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { @@ -156,7 +160,7 @@ func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) { assert.Less(t, elapsed, 100*time.Millisecond, "cancelled context should interrupt rate-limit wait promptly") } -func TestCheckAndWaitForRateLimitFetchErrorAndCanceledContext(t *testing.T) { +func TestCheckAndWaitForRateLimitFetchErrorAndContextDone(t *testing.T) { oldFetchRateLimitFunc := fetchRateLimitFunc expectedFetchErr := stderrors.New("fetch failure") fetchRateLimitFunc = func() (rateLimitResource, error) { From 58a11343700f06c39b0de299c2ceb837096bcbce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:56:10 +0000 Subject: [PATCH 10/11] test: split nil and canceled context coverage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- pkg/cli/logs_rate_limit_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 6fce7dc969a..1d71f53b0b4 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -114,7 +114,7 @@ func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return fmt.Errorf("rate-limit fetch failed during context termination: %w", errors.Join(err, sleepErr)) + return fmt.Errorf("rate-limit fetch failed and context terminated during fallback cooldown: %w", errors.Join(err, sleepErr)) } return err } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index 63c1bcdad20..64f5e52e2fd 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -132,7 +132,9 @@ func TestSleepWithContextNilContext(t *testing.T) { elapsed := time.Since(start) require.NoError(t, err, "nil context should fall back to background context") assert.Less(t, elapsed, time.Second, "nil context should not block longer than timer duration") +} +func TestSleepWithContextAlreadyCanceled(t *testing.T) { canceledCtx, cancel := context.WithCancel(context.Background()) cancel() require.ErrorIs(t, sleepWithContext(canceledCtx, 2*time.Millisecond), context.Canceled) From abcf075a0088ebcc1c7d17e134e3145ce5252085 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:59:35 +0000 Subject: [PATCH 11/11] fix: clarify fallback cooldown context error wording Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_rate_limit.go | 2 +- pkg/cli/logs_rate_limit_test.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/cli/logs_rate_limit.go b/pkg/cli/logs_rate_limit.go index 1d71f53b0b4..81e1deae5cf 100644 --- a/pkg/cli/logs_rate_limit.go +++ b/pkg/cli/logs_rate_limit.go @@ -114,7 +114,7 @@ func checkAndWaitForRateLimit(ctx context.Context, verbose bool) error { // Best-effort: fall back to static cooldown so the caller can continue. logsRateLimitLog.Printf("Could not fetch rate limit, using static cooldown: %v", err) if sleepErr := sleepWithContext(ctx, APICallCooldown); sleepErr != nil { - return fmt.Errorf("rate-limit fetch failed and context terminated during fallback cooldown: %w", errors.Join(err, sleepErr)) + return fmt.Errorf("rate-limit fetch failed and context was canceled or timed out during fallback cooldown: %w", errors.Join(err, sleepErr)) } return err } diff --git a/pkg/cli/logs_rate_limit_test.go b/pkg/cli/logs_rate_limit_test.go index 64f5e52e2fd..cae157ef644 100644 --- a/pkg/cli/logs_rate_limit_test.go +++ b/pkg/cli/logs_rate_limit_test.go @@ -97,7 +97,7 @@ func TestSleepWithContextCancellation(t *testing.T) { elapsed := time.Since(start) require.ErrorIs(t, err, context.Canceled) - assert.Less(t, elapsed, 500*time.Millisecond, "sleepWithContext should return quickly when context is already cancelled") + assert.Less(t, elapsed, time.Second, "sleepWithContext should return quickly when context is already cancelled") } // TestSleepWithContextDeadlineExceeded verifies that sleepWithContext respects a @@ -137,7 +137,11 @@ func TestSleepWithContextNilContext(t *testing.T) { func TestSleepWithContextAlreadyCanceled(t *testing.T) { canceledCtx, cancel := context.WithCancel(context.Background()) cancel() - require.ErrorIs(t, sleepWithContext(canceledCtx, 2*time.Millisecond), context.Canceled) + start := time.Now() + err := sleepWithContext(canceledCtx, 2*time.Millisecond) + elapsed := time.Since(start) + require.ErrorIs(t, err, context.Canceled) + assert.Less(t, elapsed, time.Second, "already-canceled context should return promptly") } func TestCheckAndWaitForRateLimitContextCancelled(t *testing.T) {