[Detail Bug] Rate limiting: shorter reset times can overwrite longer ones, causing premature provider retries

[detail-app]

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_0f98f61d-4144-4195-9c4b-65934428fa3d

Introduced in #71 by @WilliamAGH on Jul 12, 2026

Summary

• Context: Rate-limit tracking is used by provider routing to decide whether a primary/secondary API provider is currently available.
• Bug: recordRateLimit() unconditionally overwrites reset time instead of selecting the maximum
• Actual vs. expected: Actual: a later call with a shorter reset time replaces an existing longer reset, making the provider appear available sooner. Expected: keep the latest (max) reset deadline across repeated rate-limit signals.
• Impact: Provider may be retried prematurely while still rate-limited

Code with Bug

// RateLimitState.java
public void recordRateLimit(String provider, Instant resetTime, String rateLimitWindow) {
    ProviderState state = providerStates.computeIfAbsent(provider, providerKey -> new ProviderState());

    Duration windowDuration = parseRateLimitWindow(rateLimitWindow);

    if (resetTime == null) {
        resetTime = Instant.now().plus(windowDuration);
    }

    state.rateLimitedUntil = resetTime; // <-- BUG 🔴 overwrites existing deadline instead of keeping the max
    state.consecutiveFailures.incrementAndGet();
    state.totalFailures.incrementAndGet();
    state.lastFailure = Instant.now();

    safeSaveState();
    log.info("[{}] Rate limited until {}", sanitizeLogValue(provider), state.rateLimitedUntil);
}

// ProviderCircuitState.java
synchronized void recordRateLimit(long retryAfterSeconds) {
    Instant retryTime = ...;
    nextRetryTime = retryTime;  // <-- BUG 🔴 also overwrites; no max-selection across repeated rate limits
    circuitOpen = true;
}

Explanation

• When multiple rate-limit responses arrive close together, a subsequent response can produce a smaller reset/retry-after than a previously recorded one.
• Both the availability gates (RateLimitState.rateLimitedUntil and ProviderCircuitState.nextRetryTime) overwrite their stored deadlines with the last observed value rather than keeping the latest deadline.
• These two mechanisms are not independent: RateLimitService.applyRateLimit() feeds both from the same RateLimitDecision, so when one is shortened, the other is shortened too. This removes any redundancy and can cause isProviderAvailable() checks to return true earlier than they should.

Codebase Inconsistency

• OpenAiProviderRoutingService.isProviderCandidateAvailable() only applies the “primary backoff” check to the configured primary; secondary provider selection relies solely on rateLimitService.isProviderAvailable(provider), which ultimately depends on the (potentially shortened) stored deadlines.

Failing Test

@Test
void recordRateLimit_shouldRetainLongerResetTime_whenDecreasingResetReceived() {
    Instant longerReset = Instant.now().plus(Duration.ofMinutes(2));
    Instant shorterReset = Instant.now().plus(Duration.ofMinutes(1));

    rateLimitState.recordRateLimit(PROVIDER_NAME, longerReset, "2m");
    rateLimitState.recordRateLimit(PROVIDER_NAME, shorterReset, "1m");

    assertEquals(longerReset, providerState.getRateLimitedUntil()); // FAILS
}

Test output:

RateLimitStateTest > recordRateLimit_shouldRetainLongerResetTime_whenDecreasingResetReceived() FAILED
    org.opentest4j.AssertionFailedError at RateLimitStateTest.java:72

Recommended Fix

• Update both RateLimitState.recordRateLimit() and ProviderCircuitState.recordRateLimit() to keep the maximum (latest) deadline when a new rate-limit signal is recorded (i.e., only replace the stored deadline if the new one is later).
• Add/adjust tests to cover the case where a later call provides a shorter reset time.

History

This bug was introduced in commit 8c596e2. Earlier code also overwrote rateLimitedUntil, but compounded backoff always extended the deadline on repeated failures. Commit 8c596e2 removed the compounding backoff to honor provider-declared resets, exposing the overwrite behavior: a shorter subsequent reset can now shorten an existing rate-limit window, enabling premature retries.