⚠️ No Changeset found

Latest commit: 65b1c5e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Greptile Summary

This PR adds a maxRetryAfter guard (default 60 s) to the withRetry decorator: when the retry-after response header specifies a delay longer than the cap, the error is re-thrown immediately rather than sleeping for hours. The core implementation in retry.ts is correct and well-tested.

Confidence Score: 5/5

Safe to merge; all remaining feedback is P2 and does not affect production behaviour.

The production change in retry.ts is minimal, correct, and covered by tests. The only finding is a pre-existing test design issue (Unix-timestamp path not actually exercised) that was made more visible by this PR but does not affect the new feature.

src/core/api/retry.test.ts – the Unix-timestamp test case should be updated to use a future timestamp or mock Date.now so it covers the intended code path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[429 / RetriableError caught] --> B{isLastAttempt?}
    B -- Yes --> Z[throw error]
    B -- No --> C{retry-after header present?}
    C -- No --> D[Exponential backoff delay\ncapped at maxDelay]
    C -- Yes --> E[Parse retry-after value]
    E --> F{Is it a Unix timestamp?}
    F -- Yes --> G[delay = timestamp × 1000 − Date.now]
    F -- No --> H[delay = value × 1000]
    G --> I{delay > maxRetryAfter?}
    H --> I
    I -- Yes --> Z
    I -- No --> J[Call onRetryAttempt callback]
    D --> J
    J --> K[await setTimeout delay]
    K --> L[Retry attempt]

This is a comment left during a code review.
Path: src/core/api/retry.test.ts
Line: 112-146

Comment:
**Unix timestamp test no longer tests Unix timestamp path**

The `fixedDate` is `2010-01-01`, giving `retryTimestamp ≈ 1,262,304,000`. In 2026, `Date.now() / 1000 ≈ 1,745,000,000`, so the condition `retryValue > Date.now() / 1000` evaluates to **false** — the code takes the delta-seconds branch, not the Unix-timestamp branch. The test passes numerically (delta-seconds delay of `1262304000 * 1000 === fixedDate.getTime()`), but it isn't covering the intended path anymore.

Adding `maxRetryAfter: Number.POSITIVE_INFINITY` was necessary to avoid the new cap kicking in, but it also highlights that the test silently stopped testing Unix-timestamp behaviour some time ago. Consider using a future timestamp (or mocking `Date.now`) so the Unix path is actually exercised.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: cap retry-after delay to prevent si..." | Re-trigger Greptile}

No changeset needed — per project guidelines, contributors don't create changelog entries. Maintainers handle versioning and changelog curation during the release process.

Thanks for the thorough review! The P2 feedback about the Unix timestamp test is spot on — addressed in 65b1c5e.

The test now stubs Date.now to a fixed value (1700000000000) and sets retryTimestamp = fakeNow / 1000 + 1 (1 second in the future), ensuring retryValue > Date.now() / 1000 is true and the Unix-timestamp branch is actually exercised. The maxRetryAfter: Infinity workaround is no longer needed since the computed delay is just 1000ms, well within the default cap.