Drive Google provider adapters (Gemini, Vertex) from declarative specs - #7153
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors Gemini and Vertex adapters to use shared declarative provider specifications while preserving behavior.
Changes:
- Adds centralized Google provider specifications and factory logic.
- Simplifies Gemini and Vertex wrappers.
- Adds behavior tests and updates architecture documentation.
Show a summary per file
| File | Description |
|---|---|
docs/authentication-architecture.md |
Documents the specification module. |
containers/api-proxy/providers/vertex.js |
Delegates Vertex creation to the shared factory. |
containers/api-proxy/providers/google-provider-specs.js |
Defines Gemini and Vertex specifications. |
containers/api-proxy/providers/google-adapter.test.js |
Tests derived configuration and behavior. |
containers/api-proxy/providers/google-adapter.js |
Builds adapters from provider specifications. |
containers/api-proxy/providers/gemini.js |
Delegates Gemini creation to the shared factory. |
Review details
Tip
Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
✅ Copilot review passed with no inline comments. @copilot Add the |
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤
|
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤
|
|
✅ Contribution Check completed successfully! PR #7153 appears to follow CONTRIBUTING.md: the change is well-contained, adds unit tests for the new declarative provider-spec path, and updates the architecture docs. No missing guideline items requiring a review comment were found.
|
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed...
|
|
🛡️ Smoke Copilot Network Isolation reports failed while checking network isolation. Investigate the egress model. EGRESS_RESULT allow=pass deny=pass. Allowed domain (api.github.com) reachable HTTP 200; blocked domain (example.com) denied via proxy 403. Overall: PASS. No PR context detected (empty trigger event), so comment/label skipped per instructions. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
✅ Build Test Suite completed successfully!
|
|
✅ Smoke Claude passed
|
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.
|
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓
|
|
✅ Security Guard completed successfully! Security review of PR #7153: No security issues found. Changes refactor Google provider adapter configuration by centralizing specs into a declarative object. All existing security controls (authentication, URL transformation, port assignments, target domains) are preserved. Input validation is actually strengthened with a runtime provider key check. No egress, capability, or firewall changes.
|
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed...
|
|
🔌 Smoke Services — All services reachable! ✅
|
|
✅ Smoke Gemini completed. All facets verified. 💎 Smoke test completed with FAIL status. Connectivity and MCP titles were restricted.
|
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 Warning Firewall blocked 5 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed12.pkgs.visualstudio.com"
- "msfeed17.pkgs.visualstudio.com"
- "msfeed2.pkgs.visualstudio.com"
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Smoke Test: Copilot BYOK (Direct) Mode
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall: PASS cc
|
|
Smoke Test: Copilot Engine —
Overall: PASS
|
|
Smoke Test: API Proxy OTEL Tracing — all scenarios passed
Overall: PASS.
|
Smoke Test Results
Overall Status: FAIL
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Note: Java's
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version differs between host and chroot environment.
|
|
Merged PRs: "docs: clarify /usr/local read-only mount and hardcoded-binary-path workarounds"; "Upgrade gh-aw extension to latest pre-release and recompile workflows" Warning Firewall blocked 5 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "msfeed12.pkgs.visualstudio.com"
- "msfeed17.pkgs.visualstudio.com"
- "msfeed2.pkgs.visualstudio.com"
- "msfeed25.pkgs.visualstudio.com"
- "registry.npmjs.org"See Network Configuration for more information.
|
|
Smoke Test: Docker Sbx — PASS ✅
Recent merged PRs:
Overall: PASS cc
|
providers/gemini.jsandproviders/vertex.jswere near-identical wrappers aroundcreateGoogleApiKeyAdapter(), each hand-repeating port, target, paths, health service name, and two error message strings. Any new Google-family provider would copy the same boilerplate, with drift risk on the strings that are actually derivable.Changes
New
providers/google-provider-specs.js—GOOGLE_PROVIDER_SPECStable with one entry per provider:name,label,port,envConstants,defaultTarget,validationPath,modelsPath, and an optionaltransformRequestUrlhook (used only by Gemini to strip?key=).providers/google-adapter.js— addscreateGoogleProviderAdapter(providerKey, env, deps), which resolves the spec and derives the previously duplicated strings from it:healthServiceName→awf-api-proxy-${name}unconfiguredErrorMessage→${label} proxy not configured (no ${KEY_ENV})...healthErrorMessage→${KEY_ENV} not configured in api-proxy sidecarUnknown keys throw.
createGoogleApiKeyAdapter()is unchanged and still exported.providers/gemini.js/providers/vertex.js— reduced to one-line delegations; module boundaries and exported factory names are preserved, soproviders/index.jsand existing callers are untouched.providers/google-adapter.test.js— covers message/port/target derivation for both providers, transform presence vs. absence,modelsPath: nullyielding no models fetch config, andx-goog-api-keyauth headers.docs/authentication-architecture.md— file-reference row updated for the new module.Adding another Google-backed provider is now a spec entry plus a trivial wrapper:
Behavior is intended to be byte-identical: adapter name, port,
isEnabled(), the 503 unconfigured body, the/healthbody, reflection info, models fetch config, and URL-transform presence were compared before and after.Notes for reviewers
labelfield ('Gemini','Vertex AI') exists solely to reproduce the existing error copy exactly rather than deriving it fromname.transformRequestUrlis spread conditionally so Vertex keepstransformRequestUrl === undefinedrather than gaining a no-op identity function —providers/index.jsdocuments that property as optional.