fix: prevent token refresh thundering herd

[aaguiarz]

Summary

Ensures concurrent token header reads share one in-flight refresh instead of issuing parallel token exchange requests.

Changes

• Added regression test asserting 5 parallel calls only trigger 1 token request
• Added an internal refreshAccessTokenPromise lock in Credentials
• Reused the same refresh promise for concurrent callers and cleared it when settled

Potential Breaking Change

• Concurrent expired-token requests now share a single refresh result.
• Under transient token endpoint failures, all waiting callers may fail together (instead of independent parallel refresh attempts), which changes failure/retry dynamics.

Fixes #332

Summary by CodeRabbit

• Improvements
  • Optimized concurrent authentication token requests to prevent duplicate requests and reduce server load.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30e32bb5-1ac4-4270-b415-0e3a0aca4a9e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

Implements deduplication of concurrent token refresh requests in Credentials by caching an in-flight promise. When multiple callers request access tokens simultaneously with no cached token available, they now await the same token refresh promise instead of triggering multiple simultaneous token exchanges. Includes test verification for concurrent request behavior.

Changes

Cohort / File(s)	Summary
Token Request Deduplication credentials/credentials.ts	Adds refreshAccessTokenPromise private member to cache in-flight client credentials token requests. In getAccessTokenValue, reuses the promise for concurrent callers requesting tokens without a valid cache, clearing it after completion.
Concurrent Request Tests tests/credentials.test.ts	Adds test case verifying concurrent calls to getAccessTokenHeader trigger only a single token endpoint request, with all callers receiving the same token response.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 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 'fix: prevent token refresh thundering herd' clearly and concisely summarizes the main objective—preventing multiple concurrent token refresh requests (thundering herd problem).
Linked Issues check	✅ Passed	The PR fully implements the requirements from issue #332: adds an internal refreshAccessTokenPromise lock to deduplicate concurrent token refresh requests and includes regression tests verifying only one token request occurs.
Out of Scope Changes check	✅ Passed	All changes are scoped to the token refresh deduplication fix: internal Credentials lock mechanism and regression tests; no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-332-concurrent-token-refresh

---

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 and usage tips.}

[dosubot]

Related Documentation

Checked 8 published document(s) in 1 knowledge base(s). No updates required.

^{How did I do? Any feedback?}  

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.82%. Comparing base (7331c56) to head (b62bdd2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   85.79%   85.82%   +0.03%     
==========================================
  Files          26       26              
  Lines        1267     1270       +3     
  Branches      225      250      +25     
==========================================
+ Hits         1087     1090       +3     
  Misses        110      110              
  Partials       70       70              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR addresses issue #332 by deduplicating concurrent access token refreshes so multiple callers share a single in-flight token exchange instead of triggering parallel requests.

Changes:

• Added an in-flight refresh lock (refreshAccessTokenPromise) in Credentials to dedupe concurrent refresh calls.
• Added a regression test asserting 5 concurrent getAccessTokenHeader() calls only trigger 1 token request.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/credentials.test.ts	Adds concurrency regression test for single token exchange under parallel access-token header reads.
credentials/credentials.ts	Introduces a shared in-flight refresh promise to prevent concurrent token refresh thundering herd.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aaguiarz]

Addressed the actionable test-review comment in commit d31b421.

Updated the concurrency regression test to enforce intent directly with nock:

• switched interceptor from .times(5) to .once()
• captured the scope and added expect(scope.isDone()).toBe(true)

This now verifies both:

• all 5 callers receive the shared token
• exactly one token endpoint request was made

Validated locally with:

• npm test -- tests/credentials.test.ts -t "single token request for concurrent"

On the autogenerated-file note: agreed in principle; this PR remains scoped to the runtime fix and test behavior.

[aaguiarz]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aaguiarz]

Addressed the actionable review feedback from #333 (review) in commit 5ab0260.

What changed:

• Removed the redundant tokenRequestCount assertion from the concurrent token-read test, since nock(...).once() + scope.isDone() already enforces single request behavior.
• Added a new regression test (should clear shared refresh promise after failure and retry on the next call) that verifies:
  • concurrent callers share the same rejection during a failed token refresh, and
  • a subsequent call retries successfully (proving the in-flight refresh promise is cleared after rejection).

Validation run:

• npm test -- tests/credentials.test.ts

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rhamzeh]

@aaguiarz, change looks good - but can you:

• rebase on/merge main to fix the conflicts
• add a note about this fix in the changelog?

[SoulPancake]

Just resolved the conflicts for this one,
Functionally everything else is same