refactor(api-proxy): extract createProviderAuthScaffold to deduplicate provider adapter boilerplate#5760
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the api-proxy provider adapters to reduce duplicated, security-sensitive credential boilerplate by introducing a shared helper (createProviderAuthScaffold) in adapter-factory.js. The change centralizes env-var credential/target/basePath reading (via existing createBaseAdapterConfig) and standardizes bodyTransform extraction across non-Copilot providers.
Changes:
- Added
createProviderAuthScaffold(env, deps, envVars)and exported it fromadapter-factory.js. - Updated OpenAI, Anthropic, and Gemini provider adapters to use
createProviderAuthScaffoldinstead of duplicating the same setup logic. - Added unit tests for
createProviderAuthScaffoldto cover env reading, defaults, normalization, andbodyTransformbehavior.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/adapter-factory.js | Adds createProviderAuthScaffold helper and exports it to dedupe adapter setup logic. |
| containers/api-proxy/providers/openai.js | Switches OpenAI adapter to use the new helper while preserving BYOK/OIDC logic. |
| containers/api-proxy/providers/anthropic.js | Switches Anthropic adapter to use the new helper and composes transforms using the returned value. |
| containers/api-proxy/providers/gemini.js | Switches Gemini adapter to use the new helper and passes through bodyTransform. |
| containers/api-proxy/adapter-factory.test.js | Adds unit tests for the new helper (env/defaults/normalization/bodyTransform). |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Low
| /** | ||
| * Bundle the repeated env-reading and deps-extraction boilerplate common to | ||
| * all non-Copilot provider adapters: | ||
| * | ||
| * createBaseAdapterConfig(env, envVars) → { apiKey, rawTarget, basePath } | ||
| * deps.bodyTransform || null → bodyTransform | ||
| * |
| * @param {Record<string, string|undefined>} env - Environment variables | ||
| * @param {{ bodyTransform?: ((body: Buffer) => Buffer|null)|null }} [deps={}] - Injected dependencies | ||
| * @param {object} envVars | ||
| * @param {string} envVars.keyEnvVar - e.g. 'GEMINI_API_KEY' | ||
| * @param {string} envVars.targetEnvVar - e.g. 'GEMINI_API_TARGET' | ||
| * @param {string} envVars.basePathEnvVar - e.g. 'GEMINI_API_BASE_PATH' | ||
| * @param {string} envVars.defaultTarget - e.g. 'generativelanguage.googleapis.com' | ||
| * @returns {{ apiKey: string|undefined, rawTarget: string, basePath: string, | ||
| * bodyTransform: ((body: Buffer) => Buffer|null)|null }} | ||
| */ |
|
@copilot address review feedback |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓 |
|
❌ Security Guard failed. Please review the logs for details. |
|
✅ Smoke Gemini completed. All facets verified. 💎 Connectivity to github.com failed (SSL error 35). File test passed. PR listing pending. |
|
✅ Smoke Claude passed |
|
✅ Contribution Check completed successfully! Contribution guidelines check complete for PR #5760: no important missing items found in the pre-fetched PR metadata, diffs, and CONTRIBUTING.md content. No comment needed. |
|
✅ Build Test Suite completed successfully! |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓 |
… async bodyTransform type
Both JSDoc issues addressed in the latest commit:
|
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
🔬 Smoke Test: Copilot PAT Auth — PR #5760
Overall: PARTIAL — MCP connectivity confirmed; pre-step outputs not interpolated. Auth mode: PAT (COPILOT_GITHUB_TOKEN) | PR by
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
✅ Smoke Test: Copilot BYOK (Direct) Mode
Status: PASS
|
🔬 Smoke Test Results
Overall: PR: refactor(api-proxy): extract createProviderAuthScaffold to deduplicate provider adapter boilerplate
|
|
✅ fix: sysroot filter should not drop workspace custom mounts 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.
|
Chroot Version Comparison Results
Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.
|
Smoke Test: API Proxy OpenTelemetry Tracing — Results
Overall: All scenarios pass. OTEL tracing integration is complete and functioning correctly on PR #5760.
|
|
Smoke test summary: FAIL. Connectivity error 35. 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.
|
|
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) GitHub MCP: fix: sysroot filter should not drop workspace custom mounts; fix: filter split-fs-invisible mounts when sysroot-stage is active (arc-dind) ✅ GitHub.com connectivity: HTTP 200 ✅ File I/O: smoke-test ✅ BYOK inference: ✅ Overall: PASS
|
|
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Notes
|
All non-Copilot provider adapters (OpenAI, Anthropic, Gemini) independently repeated the same two-step security-sensitive boilerplate: reading credentials via
createBaseAdapterConfigand extractingdeps.bodyTransform || null. Consolidating this into a single helper reduces the number of places where credential env-var names are referenced directly.Changes
adapter-factory.js— NewcreateProviderAuthScaffold(env, deps, envVars)bundlescreateBaseAdapterConfig+bodyTransformextraction, returning{ apiKey, rawTarget, basePath, bodyTransform }. Exported alongside existing helpers.providers/gemini.js— Direct replacement;createBaseAdapterConfigimport swapped forcreateProviderAuthScaffold.providers/anthropic.js— Same replacement;bodyTransformdestructured asdepsBodyTransformsince it's subsequently composed withoptimisationsTransform.providers/openai.js— Same replacement with renamed destructuring (apiKey: openaiApiKeyetc.) to preserve the existing BYOK target/key override logic.adapter-factory.test.js— 9 new unit tests covering env-var reading, default fallbacks,bodyTransformpassthrough, null/undefined/omitted deps, whitespace trimming, and protocol prefix stripping.