refactor(oidc): extract duplicate cache-update/refresh-scheduling into base class#3880
Conversation
…vider Eliminates ~40 lines of duplicate token cache-update and refresh-scheduling code from all four OIDC providers. The repeated block (set cached value, record expiry, compute refresh delay, schedule next refresh) is now a single protected helper in the base class. - Add `_storeAndScheduleRefresh(value, expiresIn)` to BaseOidcTokenProvider - Add abstract `_setCachedValue(value)` to BaseOidcTokenProvider - Implement `_setCachedValue` in AnthropicOidcTokenProvider (-> _cachedToken) - Implement `_setCachedValue` in GcpOidcTokenProvider (-> _cachedToken) - Implement `_setCachedValue` in OidcTokenProvider/Azure (-> _cachedToken) - Implement `_setCachedValue` in AwsOidcTokenProvider (-> _cachedCredentials) - Drop now-unused REFRESH_FACTOR / MIN_REFRESH_MARGIN_SECS imports from each subclass (still exported from oidc-token-provider-base for external use) All 42 existing tests continue to pass.
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Refactors the OIDC token provider implementations in containers/api-proxy to remove duplicated “cache + expiry + refresh scheduling” logic by centralizing it in BaseOidcTokenProvider.
Changes:
- Added
BaseOidcTokenProvider._storeAndScheduleRefresh(value, expiresIn)to perform cache assignment, expiry tracking, and refresh scheduling in one place. - Added abstract hook
BaseOidcTokenProvider._setCachedValue(value)and implemented it in each provider (token vs credentials). - Updated all four OIDC providers to call
_storeAndScheduleRefresh(...)and removed now-unused imports ofREFRESH_FACTOR/MIN_REFRESH_MARGIN_SECS.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/oidc-token-provider-base.js | Introduces _storeAndScheduleRefresh and the new abstract _setCachedValue hook; keeps refresh math centralized. |
| containers/api-proxy/oidc-token-provider.js | Azure provider now uses the shared base helper and implements _setCachedValue for _cachedToken. |
| containers/api-proxy/gcp-oidc-token-provider.js | GCP provider now uses the shared base helper and implements _setCachedValue for _cachedToken. |
| containers/api-proxy/aws-oidc-token-provider.js | AWS provider now uses the shared base helper and implements _setCachedValue for _cachedCredentials. |
| containers/api-proxy/anthropic-oidc-token-provider.js | Anthropic provider now uses the shared base helper and implements _setCachedValue for _cachedToken. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 0
Smoke Test: Claude Engine
Result: PASS
|
🔥 Smoke Test: Copilot BYOK Offline Mode
Running in BYOK offline mode ( PR author: Overall: PARTIAL — BYOK inference working; pre-step smoke data not injected (template variables unevaluated)
|
🔬 Smoke Test Results
Overall: PASS PR by
|
|
✅ [awf] ARC/DinD chroot: auto-stage runner binary and critical /etc files for split-filesystem Docker hosts Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. ✅
|
Gemini Engine Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🧪 Chroot Version Comparison
Result: FAILED — Python and Node.js versions differ between host and chroot.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL —
|
The "store cached value + record expiry + compute refresh delay + schedule next refresh" block was copy-pasted verbatim across all four OIDC provider subclasses (~11 lines × 4 = ~40 duplicate lines). The
REFRESH_FACTOR/MIN_REFRESH_MARGIN_SECSmath lived in four places, meaning any tuning change required four edits.Changes
oidc-token-provider-base.js— adds two new members toBaseOidcTokenProvider:_storeAndScheduleRefresh(value, expiresIn)— canonical implementation of expiry tracking + refresh scheduling math_setCachedValue(value)— abstract hook (mirrors existing_getCachedValue()) for subclasses to assign to their own fieldAll four providers (
anthropic,gcp,oidc/Azure,aws) — replace the duplicated block withthis._storeAndScheduleRefresh(value, expiresIn), implement_setCachedValue, and drop the now-unusedREFRESH_FACTOR/MIN_REFRESH_MARGIN_SECSimportsBefore / After
AWS differs only in that
_setCachedValueassigns tothis._cachedCredentialsinstead ofthis._cachedToken.