refactor: extract provider env var constants to a shared module#5207
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
✨ New Files (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
⚠️ Not ready to approve
The OpenAI provider adapter still reads several provider env vars via literal env.* properties, undermining the refactor’s goal of eliminating drift risk.
Pull request overview
Refactors the API proxy provider environment-variable “matrix” into shared constants modules to reduce duplicated string literals and drift risk between the TypeScript host wrapper and the api-proxy container adapters.
Changes:
- Added shared env var constant modules for host-side TypeScript (
src/api-proxy-env-constants.ts) and container-side CommonJS (containers/api-proxy/provider-env-constants.js). - Updated host-side config assembly and api-proxy service env forwarding to reference env vars via constants.
- Updated provider adapters (OpenAI/Anthropic/Gemini/Copilot) to use constants instead of inline env var string literals (with one remaining gap noted in comments).
File summaries
| File | Description |
|---|---|
| src/services/api-proxy-service-config.ts | Uses shared env var constants when building api-proxy container environment. |
| src/commands/build-config.ts | Resolves provider-related config values from process.env[...] using shared constants. |
| src/api-proxy-env-constants.ts | New TypeScript constants for provider env var names used on the host side. |
| containers/api-proxy/provider-env-constants.js | New CommonJS constants for provider env var names used in container adapters. |
| containers/api-proxy/providers/openai.js | Uses constants for base adapter config and auth header env var (but still has some literal env reads that should be migrated). |
| containers/api-proxy/providers/anthropic.js | Uses constants for key/target/base-path and auth-header env var names. |
| containers/api-proxy/providers/gemini.js | Uses constants for key/target/base-path env var names. |
| containers/api-proxy/providers/copilot.js | Uses constants for Copilot env var names when reading token/base-path. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const providerType = (env.COPILOT_PROVIDER_TYPE || '').trim().toLowerCase(); | ||
| const copilotAzureByokEnabled = providerType === 'azure'; | ||
| const customAuthHeader = (() => { |
|
@copilot address review feedback |
|
⏳ Copilot review left inline comments. @copilot To proceed:
|
Addressed in commit I updated |
|
✅ Contribution Check completed successfully! |
|
❌ Smoke Claude failed |
|
🔑 Smoke Copilot PAT reports failed. PAT auth path may have issues... |
|
🔌 Smoke Services — Service connectivity failed |
|
📡 Smoke OTel Tracing reports failed. OTel tracing regression detected. |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
🔬 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: FAIL (incomplete pre-step data) CC
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅ PASS
Running in direct BYOK mode. Agent sees placeholder credential only; sidecar holds real key.
|
|
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra
Overall: PASS
|
🔭 Smoke Test: API Proxy OTEL Tracing
All scenarios pass. ✅
|
|
Overall: PASS
|
🔬 Smoke Test Results
PR: refactor: extract provider env var constants to a shared module Overall: PASS
|
|
Refactor: extract createProviderOidcAuth to unify OIDC setup across provider adapters
|
Smoke Test: Services Connectivity
Overall: FAIL
|
🧪 Chroot Version Comparison Results
Overall: ❌ Not all runtimes matched — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results: PASS
Overall status: PASS 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
The API proxy provider env var matrix (
*_API_KEY,*_API_TARGET,*_API_BASE_PATH, auth-header overrides) was duplicated as string literals in both the TypeScript host wrapper and the JS container provider adapters — creating drift risk when adding or renaming provider settings.Changes
containers/api-proxy/provider-env-constants.js(new) — CommonJS source of truth for the container side; exportsOPENAI_ENV,ANTHROPIC_ENV,GEMINI_ENV,COPILOT_ENVobjectssrc/api-proxy-env-constants.ts(new) — TypeScript mirror (as const) for the host wrapper sideproviders/{openai,anthropic,gemini,copilot}.js— import and use constants instead of inline string literals increateBaseAdapterConfigcalls and directenv.*accessessrc/commands/build-config.ts— replaceprocess.env.OPENAI_API_KEYetc. withprocess.env[OPENAI_ENV.KEY]src/services/api-proxy-service-config.ts— use constants for both key-forwarding and target/base-path env var namesBefore:
After:
The two constants files (JS and TS) need to be kept in sync — both carry a comment pointing to their counterpart.