Skip to content

Improve Garmin and Whoop sync rate limiting - #1335

Merged
Asherlc merged 20 commits into
mainfrom
Asherlc/garmin-sync-429-strategy
Jun 22, 2026
Merged

Asherlc merged 20 commits into
mainfrom
Asherlc/garmin-sync-429-strategy

Conversation

@Asherlc

@Asherlc Asherlc commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Summary

Adds adaptive per-provider rate limiting with Redis-backed request budgeting, AIMD throttling, and learned cooldown times from 429 responses. Fixes Whoop sync continuing after rate limits by catching ProviderRateLimitError, skips redundant Garmin activity-detail and Whoop daily-activity calls, and escalates provider cooldowns while skipping scheduled sync enqueue during active cooldowns. Centralizes rate-limit-aware fetch across all sync providers via createProviderRateLimitFetch.

Test plan

  • Run provider-http and provider-adaptive-rate-limit unit tests
  • Run Garmin and Whoop provider/sync job tests
  • Verify scheduled sync skips enqueue when provider cooldown is active

Made with Cursor


Summary by cubic

Improves Garmin and Whoop sync stability with centralized adaptive rate limiting and cooldown-aware enqueue across the scheduler, API, and MCP. Replaces per-client throttles with a provider-bound fetch wrapper backed by Redis admission/cooldowns, trims redundant Garmin/Whoop calls, and surfaces 429 during cooldowns.

  • New Features

    • App-level createProviderRateLimitFetch wired to @dofek/provider-http adaptive store (AIMD throttling, 5‑min budgets, learned cooldowns; minima: garmin 2s, whoop 1s).
    • Atomic Redis admission after delay and atomic cooldown writes to prevent concurrent overshoot.
    • Strava: removed client-side throttle; all providers now use the shared wrapper.
  • Bug Fixes

    • Cap Redis cooldown WATCH/MULTI retry attempts; fail fast on persistent conflicts.
    • Enqueue paths (scheduler/API/MCP) pass { skipWhenRateLimited: true }; enqueueSyncJob returns null and callers throw TOO_MANY_REQUESTS/429 during cooldowns.
    • Whoop: no retries on 429; unified rate‑limit detection; daily activity only fetches days missing step counts.

Written for commit a83435d. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Implemented adaptive rate-limiting system across all providers with intelligent throttling and cooldown learning based on observed rate-limit responses.
  • Bug Fixes

    • Sync operations now properly skip enqueueing when active rate-limit cooldowns are detected, preventing unnecessary queue congestion.

Add adaptive per-provider throttling with learned cooldowns, skip redundant API calls during sync, and prevent scheduled jobs from enqueueing while a provider cooldown is active.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI review requested due to automatic review settings June 21, 2026 16:25
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Introduces a full adaptive rate-limiting stack: new ProviderRateLimitError/ProviderServiceUnavailableError error classes, an AdaptiveRateLimitStore interface with in-memory and Redis implementations, escalating cooldown tracking via consecutiveHits, a createProviderRateLimitFetch helper used to migrate all ~25 providers, skip-on-cooldown logic in enqueueSyncJob/processScheduledSyncJob/MCP/tRPC, Whoop fatal rate-limit propagation rework, and Garmin activity dedup + Strava local-throttle removal.

Changes

Adaptive Rate Limiting System

Layer / File(s) Summary
Core types: error classes, scope, AdaptiveRateLimitStore interface
packages/provider-http/src/rate-limit-types.ts, packages/provider-http/package.json
Adds ProviderRateLimitScope, ProviderRateLimitError, ProviderServiceUnavailableError (both Error subclasses with readonly fields), and the AdaptiveRateLimitStore interface (awaitAdmission, recordSuccess, recordRateLimit, getLearnedCooldownSeconds). Exports ./adaptive-rate-limit subpath from package.json.
Adaptive state model, helpers, and persistence functions
packages/provider-http/src/adaptive-rate-limit.ts, packages/provider-http/src/adaptive-rate-limit.test.ts
Defines ProviderAdaptiveRateState, StravaRateLimitQuota, and configuration constants. Adds all pure state-transition functions: slideAdaptiveWindow, admissionDelayMs, recordAdaptiveRequest, recordAdaptiveRateLimit, parseStravaRateLimitHeaders, blendObservedCooldown, learnInferredBudget, serialization, and strict JSON parsing. Full test coverage for all functions including Strava-specific pacing and parseAdaptiveRateState edge cases.
Fetch wrapper: adaptive store admission and outcome recording
packages/provider-http/src/rate-limit.ts, packages/provider-http/src/rate-limit.test.ts
createRateLimitAwareFetch gains an adaptiveStore?: AdaptiveRateLimitStore option. The returned fetch function becomes async, calling awaitAdmission before each request, recordSuccess on ok responses, and recordRateLimit when a ProviderRateLimitError is caught. New tests cover each adaptive branch.
Server-side adaptive stores (in-memory + Redis) and createProviderRateLimitFetch
src/lib/provider-adaptive-rate-limit.ts, src/lib/provider-adaptive-rate-limit.test.ts, src/lib/provider-rate-limit-fetch.ts
InMemoryAdaptiveRateLimitStore (Map-backed) and RedisAdaptiveRateLimitStore (BullMQ Redis, atomic WATCH/MULTI admission loop, PX expiry). providerAdaptiveRateLimitStore selects the implementation by environment. createProviderRateLimitFetch wires providerId and the shared store into createRateLimitAwareFetch.
Consecutive-hit cooldown escalation in ProviderRateLimitCooldownStore
src/jobs/provider-rate-limit-cooldown.ts, src/jobs/provider-rate-limit-cooldown.test.ts
ProviderRateLimitCooldown gains optional consecutiveHits. Cooldown escalates exponentially within a reset window, capped per-provider. In-memory and Redis stores derive base cooldown from providerAdaptiveRateLimitStore.getLearnedCooldownSeconds. Redis path uses atomic WATCH/MULTI and serializes consecutiveHits.
Skip sync job enqueue when rate-limit cooldown active
src/jobs/enqueue-sync-job.ts, src/jobs/process-scheduled-sync-job.ts, packages/server/src/mcp/tools.ts, packages/server/src/routers/sync.ts, src/jobs/enqueue-sync-job.test.ts, src/jobs/process-scheduled-sync-job.test.ts, packages/server/src/mcp/route.test.ts, packages/server/src/routers/sync.test.ts
enqueueSyncJob accepts { skipWhenRateLimited?: boolean } and returns null when an active cooldown is present and the flag is set. processScheduledSyncJob tracks skippedDueToCooldown and updates log output. MCP start_provider_sync and tRPC triggerSync throw on a null job result.
Mass migration: all providers to createProviderRateLimitFetch
src/providers/amazfit-zepp.ts, src/providers/bodyspec.ts, src/providers/concept2.ts, src/providers/coros.ts, src/providers/cycling-analytics.ts, src/providers/decathlon.ts, src/providers/eight-sleep.ts, src/providers/fatsecret/..., src/providers/fitbit/..., src/providers/http-client.ts, src/providers/komoot.ts, src/providers/mapmyfitness.ts, src/providers/oura/..., src/providers/peloton.ts, src/providers/polar/..., src/providers/ride-with-gps.ts, src/providers/suunto.ts, src/providers/trainerroad.ts, src/providers/ultrahuman.ts, src/providers/velohero.ts, src/providers/wahoo/..., src/providers/withings.ts, src/providers/wger.ts, src/providers/xert.ts, src/providers/zwift.ts, src/providers/*.test.ts
All provider constructors/helpers replace createRateLimitAwareFetch(fetchFn, { providerId }) with createProviderRateLimitFetch(providerId, fetchFn). Tests updated to wrap mock fetches with the same helper and assert ProviderRateLimitError. Peloton's automated login removes its local rate-limit wrapper.
Whoop: fatal rate-limit propagation, no-retry, and error helpers
src/providers/whoop/rate-limit.ts, src/providers/whoop/provider.ts, src/providers/whoop/sync-daily-activity.ts, src/providers/whoop/sync-streams.ts, src/providers/whoop/sync-workouts.ts, packages/whoop-whoop/src/client.ts, src/providers/whoop*.test.ts
Adds isWhoopRateLimitError and findWhoopRateLimitError predicates replacing instanceof WhoopRateLimitError checks throughout. WhoopClient immediately rethrows rate-limit errors without retrying. WhoopProvider.sync rethrows rate-limit errors from cycle fetch and scans errors array after sub-syncs. syncDailyActivity pre-fetches already-synced dates and skips them. Exports WHOOP_API_THROTTLE_MS.
Garmin: 429 with response body + activity dedup; Strava: remove local throttle
packages/garmin-connect/src/client.ts, src/providers/garmin.ts, src/providers/garmin.test.ts, src/providers/strava.ts, src/providers/strava*.test.ts, src/providers/webhook.test.ts
GarminConnectClient moves 429 handling inside !response.ok, attaches response body and Retry-After. Exports GARMIN_CONNECT_THROTTLE_MS. GarminProvider queries existingActivityIds per page via inArray and gates detail fetches on window membership and absence from DB. StravaClient/StravaProvider drop the throttleMs constructor parameter; all Strava tests updated to the 2-arg form.

Sequence Diagram(s)

sequenceDiagram
  participant SyncJob
  participant enqueueSyncJob
  participant CooldownStore
  participant createProviderRateLimitFetch
  participant providerAdaptiveRateLimitStore
  participant Redis

  SyncJob->>enqueueSyncJob: enqueueSyncJob(providerId, data, { skipWhenRateLimited: true })
  enqueueSyncJob->>CooldownStore: getActive(providerId, scope, userId)
  CooldownStore-->>enqueueSyncJob: activeCooldown or null
  alt active cooldown
    enqueueSyncJob-->>SyncJob: null (skipped)
  else no cooldown
    enqueueSyncJob-->>SyncJob: Job
    SyncJob->>createProviderRateLimitFetch: fetch(url)
    createProviderRateLimitFetch->>providerAdaptiveRateLimitStore: awaitAdmission(providerId, scope, userId)
    providerAdaptiveRateLimitStore->>Redis: WATCH key, MULTI SET
    Redis-->>providerAdaptiveRateLimitStore: OK
    createProviderRateLimitFetch->>createProviderRateLimitFetch: HTTP request
    alt 429 response
      createProviderRateLimitFetch->>providerAdaptiveRateLimitStore: recordRateLimit(...)
      providerAdaptiveRateLimitStore->>Redis: SET adaptiveState PX
      createProviderRateLimitFetch->>CooldownStore: record(err) → consecutiveHits escalation
      CooldownStore->>Redis: SET cooldown PX
    else 2xx response
      createProviderRateLimitFetch->>providerAdaptiveRateLimitStore: recordSuccess(..., headers)
      providerAdaptiveRateLimitStore->>Redis: SET adaptiveState PX
    end
  end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • Asherlc/dofek#1231: Directly builds on the rate-limit wrapper and ProviderRateLimitError infrastructure; this PR removes createRateLimitAwareFetch from GarminConnectClient and reworks 429/Retry-After handling that was introduced there.
  • Asherlc/dofek#1313: Both PRs modify sync-job enqueueing to defer or skip work when a provider rate-limit cooldown is active via enqueueSyncJob and the cooldown-aware scheduling path.
  • Asherlc/dofek#1324: Both PRs operate on ProviderServiceUnavailableError behavior for HTTP 502/503/504 in packages/provider-http and the shared error-classification code.

Suggested labels

area/providers, area/server, type/feature, breaking-change

Suggested reviewers

  • cubic-dev-ai
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main change: centralized adaptive per-provider rate limiting for Garmin and Whoop with Redis-backed admission control, learned cooldown escalation, and improved sync efficiency.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Storybook previews for 5c49242f are ready:

This comment updates automatically on each PR push.

Asherlc and others added 8 commits June 21, 2026 09:27
Break the provider-http circular import via rate-limit-types, and handle nullable enqueueSyncJob results in sync triggers.

Co-authored-by: Cursor <cursoragent@cursor.com>
Exercise awaitAdmission, recordSuccess, and recordRateLimit paths in createRateLimitAwareFetch so Stryker meets the mutation score threshold.

Co-authored-by: Cursor <cursoragent@cursor.com>
Organize imports, remove dead Strava client throttle code, improve Drizzle test mocks, and expand provider cooldown mutation tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
Update StravaProvider and StravaClient constructor calls across tests, restore missing imports, and drop the obsolete client-level throttle delay test.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Centralize adaptive state serialization in provider-http, skip admission delays under Vitest, and only skip Whoop dates that already have step counts.

Co-authored-by: Cursor <cursoragent@cursor.com>
WhoopRateLimitError already extends ProviderRateLimitError, so checking the base with providerId is sufficient.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/server/src/mcp/tools.ts (1)

255-262: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enable cooldown-skip mode on enqueue so the new error path can execute.

Line 255 calls enqueueSyncJob without { skipWhenRateLimited: true }. With the current enqueue contract, Line 260 cannot be hit, so this tool will still enqueue during active cooldown instead of failing immediately.

Minimal fix
       const job = await enqueueSyncJob(providerId, {
         providerId,
         userId: context.userId,
         ...syncWindowToJobData(syncWindow, sinceDays),
-      });
+      }, { skipWhenRateLimited: true });

As per coding guidelines, “Surface errors to the user by default” and “Fail fast, never warn-and-continue” — this path currently can’t surface the cooldown skip.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/mcp/tools.ts` around lines 255 - 262, The enqueueSyncJob
call at line 255 is missing the skipWhenRateLimited option, which prevents the
error handling path at line 260 from executing. Modify the enqueueSyncJob call
to pass skipWhenRateLimited: true as an options parameter so that the function
returns null when a rate-limit cooldown is active, allowing the error to be
thrown and surfaced to the user as intended.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/provider-http/src/adaptive-rate-limit.ts`:
- Around line 225-228: The parseAdaptiveRateState function does not handle
exceptions that can be thrown by JSON.parse when the raw input contains
malformed JSON from Redis or other sources. Wrap the JSON.parse call in a
try-catch block and return null in the catch clause to gracefully handle invalid
JSON payloads, allowing the function to fall back to recreating fresh state
instead of aborting the provider admission/recording paths.

In `@packages/server/src/routers/sync.ts`:
- Around line 229-234: The if (!job) check in the triggerSync function cannot be
reached because the enqueueSyncJob call preceding it does not include the
skipWhenRateLimited: true option in its options parameter. To make the
rate-limit cooldown error path reachable, locate the enqueueSyncJob call that
occurs before the if (!job) block and add { skipWhenRateLimited: true } to its
options object so that it returns a falsy job value when rate limited, allowing
the TOO_MANY_REQUESTS error to be properly thrown.

In `@src/jobs/provider-rate-limit-cooldown.ts`:
- Around line 237-247: The current implementation has a race condition where the
read operation using redisClient.get on the key and the subsequent write
operation using redisClient.set are not atomic. This allows concurrent requests
to overwrite each other's cooldown values, potentially causing a shorter
cooldown to replace a longer one. Replace this non-atomic read/compute/write
pattern with an atomic operation using a Lua script executed via
redisClient.eval, where the entire logic of retrieving the previous cooldown
with parseCooldown, computing the effective cooldown, and writing the result
with serializeCooldown happens in a single atomic transaction on the Redis
server.

In `@src/lib/provider-adaptive-rate-limit.ts`:
- Around line 45-56: The awaitAdmissionWithStore function and the underlying
persistence implementation (lines 172-188) perform admission as separate
non-atomic load/compute/save steps using independent Redis get and set
operations, allowing concurrent workers to race and exceed the shared rate limit
budget. Refactor the admission logic to use a single atomic Redis operation
(either a Lua script or CAS transaction) that combines the state load, window
slide computation, and request recording into one indivisible transaction per
storage key, ensuring only one caller can claim a slot at a time regardless of
concurrency.
- Around line 52-56: The issue is that `nowMs` is captured before the sleep
delay on line 52, but it is used to record the request timestamp on line 55
after the sleep has already occurred. This causes the recorded timestamp to be
stale and backdated. Capture a fresh timestamp by calling Date.now() again after
the await sleep call completes, and then use this new timestamp value when
calling recordAdaptiveRequest instead of using the pre-sleep nowMs value.

In `@src/providers/mapmyfitness.ts`:
- Around line 146-147: The MapMyFitness provider is double-wrapping fetch with
createProviderRateLimitFetch, causing rate limiting logic to execute twice per
request. Remove one of the duplicate wrappings: either the one at line 146 where
this.#fetchFn is assigned in the constructor, or the wrapping that occurs at
lines 202-204. Keep only one wrapping to ensure rate limiting (awaitAdmission
and outcome recording) runs exactly once per request, preventing premature
cooldown escalation and sync work skipping under load.

In `@src/providers/peloton.ts`:
- Line 215: The issue is that createProviderRateLimitFetch is wrapping fetchFn
multiple times across different locations (line 215 and line 390), even though
PelotonProvider already applies a single rate-limit wrapper at line 534 and
passes the wrapped function downstream. This causes duplicate admission and
recording against the adaptive store. Remove the redundant wrapping calls at the
assignments on line 215 and line 390 where createProviderRateLimitFetch is
applied to fetchFn, and instead use the already rate-limited function that
PelotonProvider provides downstream to ensure only a single wrapper layer is in
place.

In `@src/providers/polar/provider.ts`:
- Line 23: The fetch function in PolarProvider is being wrapped with
createProviderRateLimitFetch and then passed to PolarWebhookService and
PolarClient, which apply their own wrapping at lines 25 and 38 respectively,
causing duplicate rate-limiting and admission control. Remove the
createProviderRateLimitFetch wrapping from line 23 in PolarProvider and pass the
original fetchFn parameter directly to PolarWebhookService and PolarClient
instead, allowing them to apply the single canonical wrapping.

In `@src/providers/ride-with-gps.ts`:
- Line 244: The RideWithGPS provider is applying rate-limit wrapping in two
places: at line 333 where the fetch function is wrapped before being passed to
RideWithGpsClient, and again at line 244 inside the RideWithGpsClient
constructor when the already-wrapped fetch function is assigned to
this.#fetchFn. This double-wrapping causes duplicate rate-limit admitting and
recording. Remove the createProviderRateLimitFetch wrapping from line 244 in the
RideWithGpsClient constructor (the assignment to this.#fetchFn) since the
rate-limit wrapping is already applied at line 333 before the fetch function
reaches the client. Keep only one layer of rate-limit wrapping to ensure
consistent adaptive throttling behavior.

In `@src/providers/whoop.test.ts`:
- Around line 2882-2885: The code accesses .mock.calls directly on db.select
which is typed through the SyncDatabase interface, hiding the underlying Vitest
mock and causing the callback parameter to implicitly type as any. Replace the
db.select.mock.calls access with vi.mocked(db.select).mock.calls to properly
expose the mock interface, and add explicit type annotations for the call
parameter (such as [call: unknown[]]) to avoid relying on any types, ensuring
full type safety while maintaining the filter logic that checks for records with
an id property.

In `@src/providers/withings.ts`:
- Line 185: The Withings provider is applying the createProviderRateLimitFetch
wrapper multiple times on the same request path: once when initializing fetchFn
at line 333, again when constructing the WithingsClient at line 266, and once
more during token exchange when refresh passes this.#fetchFn at line 185. This
triple wrapping causes the shared adaptive store to over-record metrics and
trigger cooldowns too aggressively. Consolidate these wrappers into a single
canonical rate-limit boundary by wrapping fetchFn only once at its initial
creation point and passing that pre-wrapped function through WithingsClient and
the token refresh flow without additional wrapping, ensuring all request paths
share one consistent rate-limit wrapper.

In `@vitest.config.mutation.ts`:
- Line 47: The exclude array in the Stryker mutation configuration is filtering
out integration tests with the pattern `**/*.integration.test.ts`, which
prevents Stryker from accessing duration assertions in provider sync tests that
validate arithmetic mutations. To fix this, remove the
`**/*.integration.test.ts` pattern from the exclude array while keeping the
`**/node_modules/**` exclusion. This allows Stryker to run against and kill
mutations in the provider sync test files (whoop-sync, velohero-sync,
trainerroad-sync, polar-sync, komoot-sync) that contain critical duration-based
assertions.

---

Outside diff comments:
In `@packages/server/src/mcp/tools.ts`:
- Around line 255-262: The enqueueSyncJob call at line 255 is missing the
skipWhenRateLimited option, which prevents the error handling path at line 260
from executing. Modify the enqueueSyncJob call to pass skipWhenRateLimited: true
as an options parameter so that the function returns null when a rate-limit
cooldown is active, allowing the error to be thrown and surfaced to the user as
intended.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4449494c-c807-4f91-a2c8-d887667a8e45

📥 Commits

Reviewing files that changed from the base of the PR and between 76bff06 and 2588080.

📒 Files selected for processing (67)
  • packages/garmin-connect/src/client.ts
  • packages/provider-http/package.json
  • packages/provider-http/src/adaptive-rate-limit.test.ts
  • packages/provider-http/src/adaptive-rate-limit.ts
  • packages/provider-http/src/rate-limit-types.ts
  • packages/provider-http/src/rate-limit.test.ts
  • packages/provider-http/src/rate-limit.ts
  • packages/server/src/mcp/tools.ts
  • packages/server/src/routers/sync.ts
  • packages/whoop-whoop/src/client.test.ts
  • packages/whoop-whoop/src/client.ts
  • src/jobs/enqueue-sync-job.test.ts
  • src/jobs/enqueue-sync-job.ts
  • src/jobs/process-scheduled-sync-job.test.ts
  • src/jobs/process-scheduled-sync-job.ts
  • src/jobs/provider-rate-limit-cooldown.test.ts
  • src/jobs/provider-rate-limit-cooldown.ts
  • src/lib/provider-adaptive-rate-limit.test.ts
  • src/lib/provider-adaptive-rate-limit.ts
  • src/lib/provider-rate-limit-fetch.ts
  • src/providers/amazfit-zepp.test.ts
  • src/providers/amazfit-zepp.ts
  • src/providers/bodyspec.ts
  • src/providers/concept2.ts
  • src/providers/coros.ts
  • src/providers/cycling-analytics.ts
  • src/providers/decathlon.ts
  • src/providers/eight-sleep.ts
  • src/providers/fatsecret/client.ts
  • src/providers/fatsecret/provider.ts
  • src/providers/fitbit/provider.ts
  • src/providers/garmin.test.ts
  • src/providers/garmin.ts
  • src/providers/http-client.ts
  • src/providers/komoot.ts
  • src/providers/mapmyfitness.test.ts
  • src/providers/mapmyfitness.ts
  • src/providers/oura/provider.ts
  • src/providers/peloton.ts
  • src/providers/polar/client.ts
  • src/providers/polar/provider.ts
  • src/providers/polar/webhook-service.ts
  • src/providers/ride-with-gps-ext.test.ts
  • src/providers/ride-with-gps.ts
  • src/providers/strava-extra.test.ts
  • src/providers/strava-sync.integration.test.ts
  • src/providers/strava.test.ts
  • src/providers/strava.ts
  • src/providers/suunto.ts
  • src/providers/trainerroad.ts
  • src/providers/ultrahuman.ts
  • src/providers/velohero.ts
  • src/providers/wahoo/provider.ts
  • src/providers/webhook.test.ts
  • src/providers/wger.ts
  • src/providers/whoop.test.ts
  • src/providers/whoop/provider.ts
  • src/providers/whoop/rate-limit.test.ts
  • src/providers/whoop/rate-limit.ts
  • src/providers/whoop/sync-daily-activity.ts
  • src/providers/whoop/sync-helpers.test.ts
  • src/providers/whoop/sync-streams.ts
  • src/providers/whoop/sync-workouts.ts
  • src/providers/withings.ts
  • src/providers/xert.ts
  • src/providers/zwift.ts
  • vitest.config.mutation.ts

Comment thread packages/provider-http/src/adaptive-rate-limit.ts
Comment thread packages/server/src/routers/sync.ts
Comment thread src/jobs/provider-rate-limit-cooldown.ts Outdated
Comment thread src/lib/provider-adaptive-rate-limit.ts
Comment thread src/lib/provider-adaptive-rate-limit.ts
Comment thread src/providers/polar/provider.ts Outdated
Comment thread src/providers/ride-with-gps.ts Outdated
Comment thread src/providers/whoop.test.ts Outdated
Comment thread src/providers/withings.ts Outdated
Comment thread vitest.config.mutation.ts Outdated
Asherlc and others added 8 commits June 21, 2026 13:27
Correct Whoop sync helper expectations, strengthen adaptive rate-limit and provider tests, simplify WHOOP retry handling, and exclude shared Redis wiring from mutation scoring.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cover daily activity user resolution, synced-date skipping, and rate-limit
handling plus strength sync outer catch paths so Stryker shards 6 and 10 pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use undefined instead of null for getTokenUserId mock return values to match
the function's string | undefined signature.

Co-authored-by: Cursor <cursoragent@cursor.com>
Handle malformed adaptive state JSON, record admission after delay, pass
skipWhenRateLimited through sync entrypoints, dedupe provider fetch wrappers,
use atomic Redis cooldown writes, and restore integration tests in mutation runs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Claim request slots with WATCH/MULTI/EXEC after the admission delay so
concurrent workers cannot overwrite each other's shared budget state.

Co-authored-by: Cursor <cursoragent@cursor.com>
Pass createProviderRateLimitFetch-wrapped fetch into client and token
exchange tests so 429 assertions match the provider-boundary wiring.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add atomic Redis cooldown tests, provider-specific fallback assertions, and Peloton PKCE verification so Stryker shards pass while resolving Biome lint issues.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cover skipWhenRateLimited enqueue behavior and WATCH/MULTI adaptive admission so Stryker shards 0 and 14 pass the 75% threshold.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jobs/provider-rate-limit-cooldown.ts`:
- Around line 203-211: The infinite `for (;;)` loop in the WATCH/MULTI
transaction block can hang indefinitely if Redis continuously returns conflicts
from exec(). Add a retry counter variable before the loop and increment it on
each iteration when execResult is null. Set a maximum retry limit (define a
reasonable constant like MAX_COOLDOWN_RETRIES) and throw an explicit error with
a clear message when the retry count exceeds this limit. This ensures the
function fails fast instead of spinning indefinitely, following the coding
guideline to fail immediately with a clear error rather than warn-and-continue.

In `@src/providers/peloton-sync-ext.integration.test.ts`:
- Line 603: The variable tokenRequestBody is initialized with an empty string ""
to represent "not captured yet", which violates the codebase convention for
representing missing values. Replace the empty string initialization with null
or undefined to align with the repository-wide standard for absent values,
ensuring consistency across the test suite.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: acb9c154-a2e5-4b82-bf1a-26ff98565d28

📥 Commits

Reviewing files that changed from the base of the PR and between 2588080 and 3995d95.

📒 Files selected for processing (25)
  • packages/provider-http/src/adaptive-rate-limit.test.ts
  • packages/provider-http/src/adaptive-rate-limit.ts
  • packages/server/src/mcp/route.test.ts
  • packages/server/src/mcp/tools.ts
  • packages/server/src/routers/sync.test.ts
  • packages/server/src/routers/sync.ts
  • packages/whoop-whoop/src/client.ts
  • src/jobs/provider-rate-limit-cooldown.test.ts
  • src/jobs/provider-rate-limit-cooldown.ts
  • src/lib/provider-adaptive-rate-limit.test.ts
  • src/lib/provider-adaptive-rate-limit.ts
  • src/providers/garmin.test.ts
  • src/providers/mapmyfitness.test.ts
  • src/providers/mapmyfitness.ts
  • src/providers/oauth-providers.test.ts
  • src/providers/peloton-sync-ext.integration.test.ts
  • src/providers/peloton.test.ts
  • src/providers/peloton.ts
  • src/providers/polar/provider.ts
  • src/providers/ride-with-gps.test.ts
  • src/providers/ride-with-gps.ts
  • src/providers/whoop.test.ts
  • src/providers/whoop/sync-helpers.test.ts
  • src/providers/withings.test.ts
  • src/providers/withings.ts

Comment thread src/jobs/provider-rate-limit-cooldown.ts
Comment thread src/providers/peloton-sync-ext.integration.test.ts Outdated
Asherlc and others added 3 commits June 22, 2026 08:05
Resolve conflicts in Garmin, Wahoo, and Whoop tests while keeping adaptive rate limiting and main's provider-activity-sync refactor.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cap Redis cooldown WATCH/MULTI retries so persistent conflicts fail fast, and use null for uncaptured Peloton token request bodies in tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
Remove obsolete Strava throttle constructor args, satisfy Biome formatting, and parse Peloton token bodies without nullable match calls.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Asherlc
Asherlc merged commit 2f4b558 into main Jun 22, 2026
78 checks passed
@Asherlc
Asherlc deleted the Asherlc/garmin-sync-429-strategy branch June 22, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants