Skip to content

fix(engine): retry transient auth errors in withRateLimitRetry#1911

Merged
gsxdsm merged 3 commits into
Runfusion:mainfrom
flexi767:fix/transient-auth-retry
Jul 6, 2026
Merged

fix(engine): retry transient auth errors in withRateLimitRetry#1911
gsxdsm merged 3 commits into
Runfusion:mainfrom
flexi767:fix/transient-auth-retry

Conversation

@flexi767

@flexi767 flexi767 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Problem

A long-running agent session holds its OAuth access token in memory. When the token rotates mid-run (Claude Max access tokens have an ~8 h lifetime), the next API call fails with 401 {"type":"error","error":{"type":"authentication_error","message":"Invalid authentication credentials"}}. withRateLimitRetry only retries usage-limit errors and re-throws everything else immediately, so the task is marked failed and the operator is alerted — even though the credentials file has already been refreshed and the very next call would succeed.

We run Fusion continuously on a server against a Claude Max subscription and see this at essentially every ~8 h token boundary: whatever task or heartbeat happens to be in flight at rotation time fails with a spurious 401, then self-heals on retry (in one case the "failed" task had actually already completed and merged 11 minutes later). The engine's notification/oauth-* modules added in 0.55 alert on upcoming expiry, but nothing retries the in-flight call itself.

Fix

Extend withRateLimitRetry with a transient-auth branch:

  • isTransientAuthError matches "type": "authentication_error", invalid authentication credentials, token_expired / token expired, and OAuth-scope errors.
  • Auth errors get their own small budget: 2 retries at a flat ~5 s delay (±10 % jitter). Credential refresh completes within seconds, so the rate-limit backoff curve (30 s → 2 min) would just prolong the outage.
  • Auth retries decrement the loop counter, so they never consume rate-limit attempts; the existing usage-limit path is unchanged.
  • Genuinely bad credentials still propagate after ~10 s (initial + 2 quick retries), so real auth failures are not masked.
  • Abort-signal handling matches the existing path (no sleep when already aborted).

Testing

  • 4 new tests in rate-limit-retry.test.ts: retry-then-succeed on rotation, budget exhaustion (initial + 2), auth retries not consuming rate-limit attempts (maxRetries: 1 + auth error + 429 still succeeds), and pattern classification.
  • vitest run src/__tests__/rate-limit-retry.test.ts — 15/15 pass; tsc --noEmit clean.
  • We have been running this change (as a patch on the distributed bundle) in production since 2026-06-29 across 0.50 → 0.54 → 0.55; the ~8 h spurious-failure alerts stopped while real failures still surface.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved retry behavior for authentication-related failures, including credential rotation and expired/invalid tokens.
    • Transient authentication errors now use a short, consistent delay with a dedicated retry budget, without impacting existing rate-limit backoff behavior.
    • OAuth scope/permission failures are excluded and now surface immediately for re-authorization rather than being retried.
  • Tests
    • Added coverage for transient-auth retry timing, budget exhaustion, and abort-signal cancellation.
  • Documentation
    • Updated release notes to reflect the revised OAuth token-rotation retry behavior.

A long-running agent session holds its OAuth access token in memory. When
the token rotates mid-run (Claude Max tokens have an ~8h lifetime), the next
API call fails with 401 authentication_error and withRateLimitRetry re-throws
it immediately — the task is marked failed and the operator is paged, even
though the refreshed credentials make the very next call succeed. Observed
recurring at every ~8h token boundary, hitting whatever task or heartbeat is
in flight.

Add isTransientAuthError (authentication_error / invalid authentication
credentials / token_expired / oauth scope) with its own small retry budget:
2 retries at a flat ~5s delay (credential refresh completes within seconds,
so the 30s -> 2min rate-limit backoff curve would just prolong the outage).
Auth retries decrement the loop counter so they never consume rate-limit
attempts, and the existing rate-limit path is byte-for-byte unchanged.
Genuinely bad credentials still propagate after ~10s.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 05:30

Copilot AI 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.

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

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a3445803-1761-4e51-abd1-b6610584a95b

📥 Commits

Reviewing files that changed from the base of the PR and between df88cb7 and a486e0b.

📒 Files selected for processing (3)
  • .changeset/fix-transient-auth-scope-classification.md
  • packages/engine/src/__tests__/rate-limit-retry.test.ts
  • packages/engine/src/rate-limit-retry.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/fix-transient-auth-scope-classification.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/engine/src/tests/rate-limit-retry.test.ts
  • packages/engine/src/rate-limit-retry.ts

📝 Walkthrough

Walkthrough

This PR adds transient authentication retry handling to withRateLimitRetry. It introduces a separate classifier and retry budget for auth/credential-rotation failures, keeps those retries from consuming rate-limit retries, and expands test coverage and release notes.

Changes

Transient Auth Retry Handling

Layer / File(s) Summary
Transient-auth classifier and retry constants
packages/engine/src/rate-limit-retry.ts, .changeset/fix-transient-auth-scope-classification.md
Adds isTransientAuthError, auth retry constants, updated retry-scope documentation, and a changeset entry describing the new retry behavior.
Retry loop rework for auth vs. rate-limit errors
packages/engine/src/rate-limit-retry.ts
Branches the retry loop on transient-auth vs. usage-limit failures, applies a fixed-delay auth retry budget, preserves rate-limit backoff, and keeps abort handling and final error propagation.
Auth retry test coverage
packages/engine/src/__tests__/rate-limit-retry.test.ts
Adds tests for successful transient-auth retry, auth-budget exhaustion, budget separation from 429 retries, message-pattern classification, scope-failure non-retry behavior, and abort cancellation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: adding retries for transient auth errors in withRateLimitRetry.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR retries short-lived OAuth token rotation failures in the rate-limit retry wrapper.

  • Adds a separate retry budget for transient authentication errors.
  • Excludes OAuth scope errors so reauthorization failures surface immediately.
  • Adds tests for auth retry success, budget exhaustion, rate-limit budget separation, scope classification, and abort handling.
  • Adds a patch changeset for the published package.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/engine/src/rate-limit-retry.ts Adds transient authentication retries while keeping OAuth scope errors on the immediate failure path.
packages/engine/src/tests/rate-limit-retry.test.ts Adds focused coverage for transient auth retries, retry budgets, abort behavior, and scope-error classification.
.changeset/fix-transient-auth-scope-classification.md Adds a valid patch changeset for the retry behavior change.

Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/transient-a..." | Re-trigger Greptile

Comment thread packages/engine/src/rate-limit-retry.ts Outdated

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
packages/engine/src/__tests__/rate-limit-retry.test.ts (1)

222-242: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a test for abort during an auth retry.

The new auth path introduces its own abort short-circuit (authRetries >= AUTH_MAX_RETRIES || signal?.aborted in rate-limit-retry.ts), but no test exercises aborting while a transient-auth retry is pending. The existing abort test covers only the rate-limit backoff path. A small test asserting that an aborted signal rejects an in-flight auth retry would protect this new branch against regressions.

🤖 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/engine/src/__tests__/rate-limit-retry.test.ts` around lines 222 -
242, Add a focused test in rate-limit-retry.test.ts for aborting during the auth
retry path. Reuse withRateLimitRetry, vi.useFakeTimers, and an AbortController
to start a transient-auth retry, abort before the 5s auth delay elapses, and
assert the promise rejects instead of calling the function again. Make sure the
new test covers the auth-specific short-circuit in rate-limit-retry.ts (the
authRetries/AUTH_MAX_RETRIES branch) rather than the existing rate-limit backoff
path.
🤖 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.

Nitpick comments:
In `@packages/engine/src/__tests__/rate-limit-retry.test.ts`:
- Around line 222-242: Add a focused test in rate-limit-retry.test.ts for
aborting during the auth retry path. Reuse withRateLimitRetry, vi.useFakeTimers,
and an AbortController to start a transient-auth retry, abort before the 5s auth
delay elapses, and assert the promise rejects instead of calling the function
again. Make sure the new test covers the auth-specific short-circuit in
rate-limit-retry.ts (the authRetries/AUTH_MAX_RETRIES branch) rather than the
existing rate-limit backoff path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b60cbabf-2a73-4592-a17d-f411e7233a90

📥 Commits

Reviewing files that changed from the base of the PR and between aceee52 and df88cb7.

📒 Files selected for processing (2)
  • packages/engine/src/__tests__/rate-limit-retry.test.ts
  • packages/engine/src/rate-limit-retry.ts

@gsxdsm

gsxdsm commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Yes!! Thank you

gsxdsm added 2 commits July 4, 2026 23:13
Address review feedback on Runfusion#1911:

- Greptile P1 (blocking): OAuth scope/permission failures are permanent
  (operator must re-authorize), so they are removed from the transient-auth
  classifier. A new SCOPE_ERROR_RE exclusion runs BEFORE the transient match,
  so scope errors wrapped in a generic {"type":"authentication_error"}
  envelope are also excluded instead of being retried for ~10 s.

- CodeRabbit: add a test for abort during the auth-retry sleep, covering the
  auth-specific short-circuit (the existing abort test only exercised the
  rate-limit backoff path).

- Add a regression test asserting scope errors (plain text, JSON-wrapped, and
  OAuth error codes insufficient_scope/invalid_scope) are not retried.

- Add a changeset (@runfusion/fusion: patch) — engine retry behavior ships in
  the published CLI bundle.

- Add FNXC requirement comments encoding the retry-budget invariants
  (separate auth budget, flat ~5 s delay, no rate-limit-attempt consumption,
  abort short-circuit, scope exclusion ordering).

Tests: 17/17 (rate-limit-retry). tsc --noEmit clean. eslint --fix clean.
@gsxdsm gsxdsm merged commit 567bab0 into Runfusion:main Jul 6, 2026
7 checks passed
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.

3 participants