Skip to content

refactor(oidc): extract duplicate cache-update/refresh-scheduling into base class#3880

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-token-cache-code
May 27, 2026
Merged

refactor(oidc): extract duplicate cache-update/refresh-scheduling into base class#3880
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-token-cache-code

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

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_SECS math lived in four places, meaning any tuning change required four edits.

Changes

  • oidc-token-provider-base.js — adds two new members to BaseOidcTokenProvider:

    • _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 field
  • All four providers (anthropic, gcp, oidc/Azure, aws) — replace the duplicated block with this._storeAndScheduleRefresh(value, expiresIn), implement _setCachedValue, and drop the now-unused REFRESH_FACTOR/MIN_REFRESH_MARGIN_SECS imports

Before / After

// Before (repeated in every provider):
const now = Math.floor(Date.now() / 1000);
this._cachedToken = access_token;
this._expiresAt = now + expires_in;
const refreshInSecs = Math.max(0, Math.min(
  expires_in * REFRESH_FACTOR,
  expires_in - MIN_REFRESH_MARGIN_SECS
));
this._scheduleRefresh(Math.floor(refreshInSecs * 1000));

// After (each provider):
this._storeAndScheduleRefresh(access_token, expires_in);

AWS differs only in that _setCachedValue assigns to this._cachedCredentials instead of this._cachedToken.

…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.
Copilot AI changed the title [WIP] Fix duplicate token cache-update and refresh-scheduling code refactor(oidc): extract duplicate cache-update/refresh-scheduling into base class May 26, 2026
Copilot finished work on behalf of lpcox May 26, 2026 22:28
Copilot AI requested a review from lpcox May 26, 2026 22:28
@lpcox lpcox marked this pull request as ready for review May 26, 2026 22:41
Copilot AI review requested due to automatic review settings May 26, 2026 22:41
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.57% 96.62% 📈 +0.05%
Statements 96.44% 96.48% 📈 +0.04%
Functions 98.22% 98.22% ➡️ +0.00%
Branches 90.66% 90.70% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 of REFRESH_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

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK Offline Mode

Test Result
GitHub MCP connectivity
GitHub.com HTTP ⚠️ pre-step data unavailable (template vars not expanded)
File write/read ⚠️ pre-step data unavailable (template vars not expanded)
BYOK inference (this response)

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

PR author: @Copilot · Assignees: @lpcox, @Copilot

Overall: PARTIAL — BYOK inference working; pre-step smoke data not injected (template variables unevaluated)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions github-actions Bot mentioned this pull request May 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
github.com HTTP (200)
File write/read

Overall: PASS

PR by @Copilot, assigned to @lpcox and @Copilot.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

✅ [awf] ARC/DinD chroot: auto-stage runner binary and critical /etc files for split-filesystem Docker hosts
✅ api-proxy: add Anthropic Workload Identity Federation provider
✅ refactor(oidc): extract duplicate cache-update/refresh-scheduling into base class
✅ GitHub PR query
✅ Playwright title
✅ Smoke file
✅ Build
Overall: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
S1: Module Loading otel.js loads successfully; exports: startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled
S2: Test Suite 33/33 tests passed (otel.test.js)
S3: Env Var Forwarding src/services/api-proxy-service.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
S4: Token Tracker Integration onUsage callback exists in token-tracker-http.js (line 237) as the OTEL hook point
S5: OTEL Diagnostics Graceful degradation confirmed — when OTEL_EXPORTER_OTLP_ENDPOINT is unset, spans fall back to local file (/var/log/api-proxy/otel.jsonl); no network export attempted

All scenarios pass. ✅

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions
Copy link
Copy Markdown
Contributor

Gemini Engine Smoke Test Results

  • GitHub MCP Testing: ❌
  • GitHub.com Connectivity: ❌
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

Overall Status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Chroot Version Comparison

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.16.0 v22.22.3
Go go1.22.12 go1.22.12

Result: FAILED — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #3880 · sonnet46 1M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

Check Result
Redis PING ❌ Timeout/no response
PostgreSQL pg_isready ❌ No response
PostgreSQL SELECT 1 ❌ Failed

Overall: FAILhost.docker.internal is unreachable from this environment. Service containers may not be running or the host alias is not resolvable.

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit 05ba913 into main May 27, 2026
67 of 71 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-token-cache-code branch May 27, 2026 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Duplicate Code] Token cache-update and refresh-scheduling block duplicated in all four OIDC providers

3 participants