Conversation
Allow setting the expiry buffer and jitter while preserving existing defaults. Include refresh settings in the OAuth client cache key. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Keep ClientCredentials limited to token request parameters. Preserve refresh policy through request overrides. Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change adds configurable token-expiry buffer and jitter settings. OAuth2 token validation uses these settings, configuration overrides preserve them, and OAuth2 client cache keys include them. Tests and documentation cover the new behavior. ChangesOAuth2 refresh timing
Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant ApiClient
participant OAuth2Client
participant AccessToken
ApiClient->>OAuth2Client: Request access token
OAuth2Client->>AccessToken: Validate with buffer and jitter
AccessToken-->>OAuth2Client: Return validity result
OAuth2Client-->>ApiClient: Reuse or refresh token
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The SDK now supports configurable client-credentials token refresh timing while preserving defaults. Configuration overrides, cache separation, and refresh behavior are covered, with no concrete merge-blocking risk evident. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 7 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
should i remove codex for cla to be accepted? |
|
@dpkass yes, you'll need to remove codex |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (39.42%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #389 +/- ##
============================================
+ Coverage 39.34% 39.42% +0.08%
- Complexity 1336 1341 +5
============================================
Files 202 202
Lines 7791 7815 +24
Branches 912 913 +1
============================================
+ Hits 3065 3081 +16
- Misses 4579 4585 +6
- Partials 147 149 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @dpkass |
Description
What problem is being solved?
Five-minute access tokens are immediately considered stale by the fixed 300-second refresh buffer plus jitter, causing repeated token exchanges.
How is it being solved?
Make the refresh buffer and jitter configurable at the client level while preserving existing defaults.
What changes are made to solve it?
tokenExpiryBufferSecondsandtokenExpiryJitterSecondssettings toConfigurationand fluentClientConfiguration. Zero jitter disables jitter.Validation:
./gradlew build test-integrationpassed on Java 21. Formatting and focused tests also passed after removing a redundant setter-validation test.References
Closes #388
Review Checklist
main).Summary by CodeRabbit