Route Antigravity AWF target through Gemini provider key#34839
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot invalid arguments see https://github.com/github/gh-aw/actions/runs/26429242977/job/77798928425#step:31:1 |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 13f69a2 (plus follow-up clarification in a015dfd). The Antigravity invocation now uses supported |
|
@copilot review antigravity docs to configure keys to avoid authentication issue as in https://github.com/github/gh-aw/actions/runs/26429852770/job/77800686815#step:31:1 |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in 4aeed01 (with follow-up safety fix in 875ea78).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #34839 does not have the 'implementation' label and has only 78 new lines of code in business logic directories (below the 100-line threshold). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧪 Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Pull request overview
Updates AWF config generation and Antigravity workflow execution so Antigravity traffic routes through the supported apiProxy.targets.gemini provider key (avoiding the rejected targets.antigravity key), while also aligning secret/env handling for Gemini-proxy authentication.
Changes:
- Map Antigravity API target resolution onto
apiProxy.targets.gemini(with explicit precedence and override logging). - Update Antigravity engine execution to use the current non-interactive permissions flag and to alias
GEMINI_API_KEYfromANTIGRAVITY_API_KEYfor proxy auth (while excluding it from the sandbox env). - Add/adjust tests and locked smoke workflow output to validate the new routing and env behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/awf_config.go | Routes Antigravity target selection through the gemini provider key and updates target-key documentation/comments. |
| pkg/workflow/awf_config_test.go | Adds tests asserting Antigravity target mapping uses targets.gemini and does not emit targets.antigravity. |
| pkg/workflow/antigravity_engine.go | Updates Antigravity CLI invocation flags; adds GEMINI_API_KEY aliasing and excludes it from sandbox env. |
| pkg/workflow/antigravity_engine_test.go | Updates assertions for new Antigravity CLI flags and validates GEMINI_API_KEY alias/exclusion behavior. |
| .github/workflows/smoke-antigravity.lock.yml | Regenerates locked workflow to reflect targets.gemini, new CLI args, and GEMINI_API_KEY env/exclusion. |
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: 2
| // Route the Antigravity-resolved API target through the "gemini" provider key | ||
| // to match AWF's supported target providers. | ||
| geminiTarget := GetGeminiAPITarget(config.WorkflowData, config.EngineName) | ||
| if geminiTarget != "" && geminiTarget != antigravityTarget { |
| assert.Contains(t, jsonStr, `"gemini"`, "should include gemini target for antigravity engine") | ||
| assert.Contains(t, jsonStr, "generativelanguage.googleapis.com", "should include default Gemini API hostname") | ||
| assert.NotContains(t, jsonStr, `"antigravity"`, "should not include unsupported antigravity target key") | ||
| }) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 1.3M
| } | ||
| // The Antigravity CLI and AWF's Gemini API proxy both rely on a Gemini provider key. | ||
| // Keep GEMINI_API_KEY aligned with the effective ANTIGRAVITY_API_KEY by default so the | ||
| // workflow can authenticate non-interactively without requiring users to duplicate secrets. |
There was a problem hiding this comment.
Silent empty-string assignment if ANTIGRAVITY_API_KEY is absent from env: env["GEMINI_API_KEY"] will be set to "" rather than being omitted, injecting a blank credential into the step environment.
💡 Suggested fix
if v := env["ANTIGRAVITY_API_KEY"]; v != "" {
env["GEMINI_API_KEY"] = v
}
}If ANTIGRAVITY_API_KEY is missing or empty (e.g., due to a misconfigured workflow or a future reordering of env population), the current code silently forwards an empty string as GEMINI_API_KEY. The proxy will then receive a blank API key and fail with a confusing auth error rather than a clear "key not set" diagnostic. An explicit non-empty guard prevents this silent path.
| if geminiTarget := GetGeminiAPITarget(config.WorkflowData, config.EngineName); geminiTarget != "" { | ||
| targets["gemini"] = &AWFAPITargetConfig{Host: geminiTarget} | ||
| // Route the Antigravity-resolved API target through the "gemini" provider key | ||
| // to match AWF's supported target providers. |
There was a problem hiding this comment.
User's GEMINI_API_BASE_URL is silently dropped when ANTIGRAVITY_API_BASE_URL is also set: the conflict is logged only via awfConfigLog.Printf (a debug-only logger), so the operator never sees the warning unless DEBUG= is configured.
💡 Suggested fix
Surface the conflict as a visible warning using the console formatter, or return an error requiring the user to resolve the ambiguity explicitly:
if geminiTarget != "" && geminiTarget != antigravityTarget {
// Surface to stderr, not just debug log
fmt.Fprintf(os.Stderr, "warning: ANTIGRAVITY_API_BASE_URL overrides GEMINI_API_BASE_URL for the gemini proxy target; unset one to resolve ambiguity\n")
}A user who has configured both ANTIGRAVITY_API_BASE_URL and GEMINI_API_BASE_URL will silently route all Gemini traffic through the Antigravity endpoint with no indication their Gemini config was discarded. This is a misconfiguration that should be surfaced loudly, not buried in debug output.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /tdd — approving with two non-blocking suggestions.
📋 Key Themes & Highlights
Key Themes
- Root cause fix is correct: remapping
targets.antigravity→targets.geminidirectly addresses the schema rejection; the approach is clean and explicit. - CLI flag migration is clean: replacing
--yolo/--skip-trust/--output-format stream-jsonwith--dangerously-skip-permissionsaligns with the new Antigravity CLI API; tests assert both the presence and the absence of old flags. - Two minor gaps worth a follow-up:
- The dual-URL ambiguity path (
GEMINI_API_BASE_URL+ANTIGRAVITY_API_BASE_URLboth set) has a warning log but no regression test — see inline comment. - Silent empty-string propagation of
GEMINI_API_KEYwhenANTIGRAVITY_API_KEYis absent could produce opaque 401s — see inline comment.
- The dual-URL ambiguity path (
Positive Highlights
- ✅ Three focused new tests covering default target, custom
ANTIGRAVITY_API_BASE_URL, and explicit absence of the unsupportedantigravitykey - ✅
GEMINI_API_KEYcorrectly added to bothExcludeEnvVarNames(hidden from agent) andallowedSecrets(passed throughFilterEnvForSecrets) — dual-listing is intentional and the comment explains why - ✅ Precedence logic (
antigravitybeatsgemini) is explicit with an if/else-if rather than two independent ifs - ✅ Lock file regenerated end-to-end, making the fix observable in CI
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.5M
| assert.NotContains(t, jsonStr, `"antigravity"`, "should not include unsupported antigravity target key") | ||
| }) | ||
|
|
||
| t.Run("no API targets section when no custom endpoints are configured", func(t *testing.T) { |
There was a problem hiding this comment.
[/tdd] Missing test for the dual-URL ambiguity warning path — the new branch at awf_config.go:308 (both GEMINI_API_BASE_URL and ANTIGRAVITY_API_BASE_URL set) is not covered.
💡 Suggested test sketch
t.Run("antigravity overrides gemini when both base URLs are set", func(t *testing.T) {
config := AWFCommandConfig{
EngineName: "antigravity",
WorkflowData: &WorkflowData{
EngineConfig: &EngineConfig{
ID: "antigravity",
Env: map[string]string{
"ANTIGRAVITY_API_BASE_URL": "(antigravity.internal/redacted),
"GEMINI_API_BASE_URL": "(gemini.internal/redacted),
},
},
NetworkPermissions: &NetworkPermissions{
Firewall: &FirewallConfig{Enabled: true},
},
},
}
jsonStr, err := BuildAWFConfigJSON(config)
require.NoError(t, err)
// Antigravity target wins; Gemini base URL is ignored
assert.Contains(t, jsonStr, "antigravity.internal")
assert.NotContains(t, jsonStr, "gemini.internal")
})This path emits a log warning today but has no assertion ensuring the correct target wins.
| // Keep GEMINI_API_KEY aligned with the effective ANTIGRAVITY_API_KEY by default so the | ||
| // workflow can authenticate non-interactively without requiring users to duplicate secrets. | ||
| if _, hasGeminiKey := env["GEMINI_API_KEY"]; !hasGeminiKey { | ||
| env["GEMINI_API_KEY"] = env["ANTIGRAVITY_API_KEY"] |
There was a problem hiding this comment.
[/diagnose] If ANTIGRAVITY_API_KEY is absent from env, this silently sets GEMINI_API_KEY to an empty string, which will pass the FilterEnvForSecrets check and appear in the step env as an empty value — a subtle auth failure that's hard to diagnose.
💡 Suggested guard
if apiKey, hasAPIKey := env["ANTIGRAVITY_API_KEY"]; hasAPIKey && apiKey != "" {
env["GEMINI_API_KEY"] = apiKey
}Or at minimum log a warning when propagation is skipped, so the misconfiguration surfaces early in the run log rather than as a 401 from the proxy.
🧪 Test Quality Sentinel Report🔶 Test Quality Score: 50/100 — Needs Improvement
📊 Metrics & Test Classification (2 new tests + 5 modified assertion groups analyzed)
Test Classification Details
Language SupportTests analyzed:
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 50/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). No coding-guideline violations detected. Note: neither of the two new test cases covers an error path; consider adding error/edge-case coverage to improve the score.
Smoke Antigravity failed because generated AWF config used
apiProxy.targets.antigravity, which the current firewall schema/runtime rejects. Antigravity now runs via the new harness but still depends on Gemini provider routing, so config generation must emit a supported provider target.Config generation update
BuildAWFConfigJSONnow maps Antigravity target resolution toapiProxy.targets.geminiinstead of emittingapiProxy.targets.antigravity.Contract/documentation alignment in code
Coverage additions
generativelanguage.googleapis.com) throughtargets.geminiANTIGRAVITY_API_BASE_URLmapping throughtargets.geminitargets.antigravity