fix(provider-http): bound admission delays

[Asherlc]

Summary

• compute adaptive-budget waits from the remaining rolling-window duration
• compute Strava quota pacing from elapsed time since the last admitted request
• cover the production Redis admission path so a paced request reaches MULTI/EXEC instead of retrying forever
• record the confirmed production root cause and validation in the incident baseline

Root cause

admissionDelayMs() returned the complete quota pacing interval on every atomic Redis claim recheck. The claim loop slept for that interval and recalculated the same full interval indefinitely, so the active BullMQ job renewed its lock without making progress. The inferred-budget branch had the same fixed-delay defect.

Validation

• pnpm lint
• pnpm typecheck
• CI=1 pnpm test (13,888 passed, 21 skipped)
• focused Vitest: 82 passed
• targeted Biome check
• targeted Stryker: 81.50%, zero surviving mutants in the changed admission block
• git diff --check

Refs DOFEK-SERVER-4N
Refs DOFEK-SERVER-2K

---

Summary by cubic

Fixes provider admission pacing by using remaining time instead of full intervals, and hardens persisted pacing so waits stay bounded. This stops atomic Redis claim loops and lets Strava jobs make steady progress.

• Bug Fixes
  • Adaptive budget: wait only the remainder of the rolling window; bound waits if the clock moves behind the window start.
  • Strava quota: subtract elapsed time since lastRequestMs; require both short-limit and short-usage to pace; admit immediately if no prior timestamp.
  • Timestamp self-heal: clamp future lastRequestMs and windowStartMs, preserve past timestamps, and set lastRequestMs on quota save when missing.
  • Atomic Redis path: claims now reach MULTI/EXEC after the pacing interval (and complete immediately with quota state but no prior timestamp).
  • Tests and docs: added focused cases for window sliding and atomic Strava admission; updated incident notes with the official Strava rate-limit citation.

^{Written for commit d6c18bd. Summary will update on new commits.}

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[sourcery-ai]

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 04ca6832-78b9-43df-8b21-9bb2d79dd99e

📥 Commits

Reviewing files that changed from the base of the PR and between ef6798d and d6c18bd.

📒 Files selected for processing (5)

• docs/production-incident-baseline.md
• packages/provider-http/src/adaptive-rate-limit.test.ts
• packages/provider-http/src/adaptive-rate-limit.ts
• src/lib/provider-adaptive-rate-limit.test.ts
• src/lib/provider-adaptive-rate-limit.ts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Storybook previews for bba0d86e are ready:

• Web Storybook
• Mobile Storybook

This comment updates automatically on each PR push.

[qodo-code-review]

PR Summary by Qodo

Fix provider admission pacing by using remaining window/interval delays

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Compute admission waits from remaining rolling-window/quota interval, not full interval.
• Prevent Redis WATCH claim loops from sleeping forever under Strava quota pacing.
• Add focused unit tests for Strava tiers, budget soft-cap, and atomic Redis admission.

Diagram

graph TD
  A["BullMQ worker"] --> B["Provider admission"] --> D[("Redis store")]
  B --> C["admissionDelayMs()"] --> B
  C --> E["Strava quota pacing"]
  C --> F["Adaptive budget pacing"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Persist nextEligibleAtMs in state

• ➕ Simplifies delay computation to max(0, nextEligibleAtMs - nowMs)
• ➕ Avoids re-deriving remaining delay rules in multiple branches
• ➖ Requires a state schema change and careful migration/parse compatibility
• ➖ Must still handle multiple pacing dimensions (throttle + budget + quota) when updating nextEligibleAtMs

2. Use a Redis Lua script for atomic admit-or-return-delay

• ➕ Single round-trip; no WATCH loop and no race window between GET and claim
• ➕ Can compute and update state atomically based on nowMs
• ➖ More complex operationally (script management, testing, observability)
• ➖ Harder to reuse in the in-memory store implementation

3. Switch to a token-bucket/leaky-bucket model for quota/budget

• ➕ Clearer pacing semantics; naturally produces remaining wait times
• ➕ May better match provider quotas than window-based soft caps
• ➖ Bigger behavioral change; higher risk of regressions in existing providers
• ➖ Requires retuning and more extensive validation across providers

Recommendation: The PR’s approach (compute remaining-time delays from lastRequestMs/windowStartMs) is the best near-term fix: it preserves the existing pacing model while guaranteeing convergence to zero, which is critical for the Redis atomic claim loop to make progress. Longer-term, a Lua-based atomic admit-or-delay could reduce complexity and contention, but it’s a larger operational step than necessary for this incident-driven regression fix.

Files changed (4) +218 / -36

Bug fix (1) +21 / -8

adaptive-rate-limit.tsBound budget/quota waits to remaining time since eligibility +21/-8

Bound budget/quota waits to remaining time since eligibility

• Changes admissionDelayMs to compute adaptive-budget soft-cap waits from the remaining rolling-window duration instead of a fixed throttle interval. Reworks Strava short-quota pacing to compute a quota interval and then subtract elapsed time since lastRequestMs via a shared remainingIntervalDelayMs helper, ensuring the delay eventually reaches zero.

packages/provider-http/src/adaptive-rate-limit.ts

Tests (2) +163 / -18

adaptive-rate-limit.test.tsExpand admissionDelayMs unit coverage for remaining-delay semantics +136/-18

Expand admissionDelayMs unit coverage for remaining-delay semantics

• Updates and adds tests asserting remaining-window behavior for inferred-budget soft caps, remaining-interval behavior for throttle, and convergence-to-zero behavior for Strava tiered and proportional quota pacing. Adds guard-rail coverage for missing Strava quota fields and nonzero timestamp subtraction.

packages/provider-http/src/adaptive-rate-limit.test.ts

provider-adaptive-rate-limit.test.tsRegression test: atomic Redis Strava admission reaches EXEC after pacing +27/-0

Regression test: atomic Redis Strava admission reaches EXEC after pacing

• Adds a production-mode Vitest case using fake timers to simulate a persisted Strava quota state and validate that atomic WATCH admission completes after the pacing interval elapses (i.e., does not loop forever). Ensures the claim path reaches a successful exec attempt under the production Redis admission behavior.

src/lib/provider-adaptive-rate-limit.test.ts

Documentation (1) +34 / -10

production-incident-baseline.mdDocument confirmed admission-delay root cause and validation +34/-10

Document confirmed admission-delay root cause and validation

• Updates the 2026-07-26 incident entry with the confirmed follow-up root cause: admissionDelayMs repeatedly returning a full interval during atomic claim rechecks, causing infinite WATCH-loop retries. Adds concrete production observations (Strava job state and Redis command trace) and refines follow-up/validation notes for the admission fix.

docs/production-incident-baseline.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 149 rules

✅ Skills: fix-provider, write-tests, cloudflare

1. Atomic quota wait can loop ✓ Resolved 🐞 Bug ☼ Reliability

Description

remainingIntervalDelayMs() returns the full interval when lastRequestMs is null, so Strava quota
pacing never decreases across atomic Redis WATCH rechecks for persisted states that have quota
fields but a null lastRequestMs, causing awaitAdmissionAtomically() to sleep/retry forever
without reaching MULTI/EXEC.

Code

packages/provider-http/src/adaptive-rate-limit.ts[R181-188]

+function remainingIntervalDelayMs(
+  lastRequestMs: number | null,
+  intervalMs: number,
+  nowMs: number,
+): number {
+  if (lastRequestMs == null) return intervalMs;
+  return Math.max(0, intervalMs - Math.max(0, nowMs - lastRequestMs));
+}

Relevance

⭐⭐⭐ High

Reliability bug can cause infinite WATCH sleep loop; team accepts bounding/termination fixes for
stuck retry loops.

PR-#1394

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Quota pacing uses remainingIntervalDelayMs(state.lastRequestMs, quotaIntervalMs, nowMs) which
returns a fixed interval when lastRequestMs is null; the atomic loop retries while delay > 0
without mutating the key, so a fixed delay prevents reaching the transaction.
recordSuccessWithStore() can store Strava quota fields but does not update lastRequestMs, making
this state shape possible.

packages/provider-http/src/adaptive-rate-limit.ts[148-170]
packages/provider-http/src/adaptive-rate-limit.ts[181-188]
src/lib/provider-adaptive-rate-limit.ts[115-165]
src/lib/provider-adaptive-rate-limit.ts[167-182]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`remainingIntervalDelayMs(lastRequestMs, intervalMs, nowMs)` returns `intervalMs` when `lastRequestMs` is `null`. In the atomic Redis admission loop, the state is reloaded on each retry but is not mutated until `MULTI/EXEC`. If the persisted state keeps `lastRequestMs: null` while Strava quota fields are present, `admissionDelayMs()` will keep returning the same positive delay forever and the atomic loop will never progress.

This is especially risky because `recordSuccessWithStore()` can persist Strava quota fields without setting `lastRequestMs`, so this persisted shape is representable.

### Issue Context
The PR fixes the common production root cause (fixed interval on each recheck when `lastRequestMs` exists), but the null-timestamp case still produces a fixed interval on every recheck, which recreates the same “retry forever” failure mode.

### Fix Focus Areas
- packages/provider-http/src/adaptive-rate-limit.ts[148-170]
- packages/provider-http/src/adaptive-rate-limit.ts[181-188]
- src/lib/provider-adaptive-rate-limit.ts[115-165]
- src/lib/provider-adaptive-rate-limit.ts[167-182]

### Proposed fix
Make it impossible (or self-healing) for persisted Strava quota state to have `lastRequestMs: null`:
- In `recordSuccessWithStore(...)`, when applying Strava quota headers, if `state.lastRequestMs == null`, set `lastRequestMs` to `Date.now()` before saving. This matches reality (a request just succeeded) and gives atomic admission a stable baseline so the remaining delay converges to 0.
- Optionally harden parsing/loading: if Strava quota fields are present but `lastRequestMs` is null, set `lastRequestMs = windowStartMs` (or `nowMs`) when loading, so atomic admission cannot loop indefinitely.

Add a regression test for Redis atomic admission where the stored state includes Strava quota fields and `lastRequestMs: null`, and assert admission completes after the pacing interval elapses.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Strava quota claims lack citation ✓ Resolved 📘 Rule violation § Compliance

Description

The incident baseline adds Strava quota/rate-limit behavior claims (e.g., a 15‑minute window and a
200-request short limit) without an adjacent primary-source citation. This can leave documentation
making unverified third‑party behavior assertions.

Code

docs/production-incident-baseline.md[R18382-18388]

+  lock. PostgreSQL had no active application query. Its persisted adaptive
+  state contained a `200`-request short limit, usage `1`, and one admitted
+  request at `17:30:21Z`. A 20-second Redis command trace then showed the same
+  client repeatedly executing `GET`, `WATCH`, and `GET` for the Strava
+  admission key about every 4.5 seconds without ever reaching `MULTI`/`EXEC`.
+  The 4.5-second interval exactly matched
+  `ceil(15 minutes / (200 - 1))`.

Relevance

⭐⭐⭐ High

Docs here routinely require adjacent primary citations for third‑party behavior claims; strong
accepted precedent.

PR-#2044
PR-#1858

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1505719 requires adjacent official citations for third-party behavior claims in
docs. The added paragraph asserts specific Strava quota pacing/window details (e.g., `ceil(15
minutes / (200 - 1)) and a 200`-request short limit) but provides no nearby link to Strava’s
official rate-limit documentation.

Rule 1505719: Cite third-party behavior claims in docs with primary sources
docs/production-incident-baseline.md[18382-18388]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`docs/production-incident-baseline.md` now includes third-party behavior claims about Strava API quota limits/windowing without an adjacent primary-source citation link.

## Issue Context
PR Compliance ID 1505719 requires that newly added/modified third-party behavior claims in docs have nearby citations to official or primary sources.

## Fix Focus Areas
- docs/production-incident-baseline.md[18382-18388]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Clock rollback inflates delays ✓ Resolved 🐞 Bug ☼ Reliability

Description

budgetAdmissionDelayMs() computes the inferred-budget wait as `windowStartMs +
ADAPTIVE_RATE_WINDOW_MS - nowMs without handling nowMs < windowStartMs`, so a backward
Date.now() jump can inflate admission delays by roughly the rollback amount and stall work until
wall time catches up.

Code

packages/provider-http/src/adaptive-rate-limit.ts[R190-193]

+function budgetAdmissionDelayMs(state: ProviderAdaptiveRateState, nowMs: number): number {
  if (state.inferredBudget == null) return 0;

  const softCap = Math.floor(state.inferredBudget * ADAPTIVE_BUDGET_SAFETY_RATIO);

Relevance

⭐⭐ Medium

Clock rollback handling is plausible but edge-case/semantic; no close module precedent for clamping
time skew.

PR-#1903
PR-#867

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

slideAdaptiveWindow() never corrects a future windowStartMs when nowMs goes backwards, and the
new inferred-budget branch directly uses windowStartMs - nowMs to compute the sleep, which can be
inflated by negative elapsed time.

packages/provider-http/src/adaptive-rate-limit.ts[114-124]
packages/provider-http/src/adaptive-rate-limit.ts[190-205]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`budgetAdmissionDelayMs()` uses `state.windowStartMs + ADAPTIVE_RATE_WINDOW_MS - nowMs` to compute remaining-window sleep. If the wall clock moves backwards (so `nowMs < windowStartMs`), `slideAdaptiveWindow()` will keep the existing window and the computed delay is inflated beyond the intended 5-minute window, potentially stalling admissions.

### Issue Context
This behavior is introduced by switching from a fixed `throttleMs` budget delay to a remaining-window calculation.

### Fix Focus Areas
- packages/provider-http/src/adaptive-rate-limit.ts[114-124]
- packages/provider-http/src/adaptive-rate-limit.ts[190-205]

### Proposed fix
Add a rollback guard so elapsed time never goes negative:
- Option A (preferred): In `slideAdaptiveWindow`, if `nowMs < state.windowStartMs`, reset `windowStartMs` to `nowMs` and `requestCount` to `0` (treat as a new window).
- Option B: In `budgetAdmissionDelayMs`, compute `effectiveNowMs = Math.max(nowMs, state.windowStartMs)` and use that in the remaining-window calculation.

Add/adjust a unit test that sets `windowStartMs` greater than `nowMs` and asserts the delay is bounded (<= `ADAPTIVE_RATE_WINDOW_MS`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[Asherlc]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Asherlc]

Addressed all three Qodo findings in d6c18bd:

• Null Strava timestamps now admit immediately, and successful quota responses persist the actual completion timestamp when the state has no admitted-request baseline. An atomic Redis regression proves this shape reaches MULTI/EXEC.
• Backward clock adjustments rebase future window/request timestamps while preserving the request count, and inferred-budget waits are bounded to one rolling window.
• The incident entry now cites Strava's official rate-limit documentation adjacent to the 15-minute/200-request claim.

Validation: 91 focused tests pass; targeted Biome and git diff --check pass; targeted Stryker passes with no survivors in either changed block.

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.