Skip to content

fix: env-gather autodetect sees as_needed secrets (#1447) - #1452

Closed
ptone wants to merge 2 commits into
mainfrom
fix/envgather-autodetect-as-needed
Closed

fix: env-gather autodetect sees as_needed secrets (#1447)#1452
ptone wants to merge 2 commits into
mainfrom
fix/envgather-autodetect-as-needed

Conversation

@ptone

@ptone ptone commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #1447 — P0 bug that breaks all antigravity agents on single-node when GEMINI_API_KEY is an as_needed hub secret.

  • Adds AvailableAsNeededKeys field to CreateAgentRequest (broker) and RemoteCreateAgentRequest (hub) — carries the target key names of as_needed env-type secrets that the hub filtered out of ResolvedSecrets
  • Extends resolveSecrets() to return the filtered as_needed key names alongside the resolved secrets
  • Populates req.AvailableAsNeededKeys in buildCreateRequest so the broker receives the deferred keys
  • Includes AvailableAsNeededKeys in the broker's resolvedEnvKeys set during env-var autodetect, closing the chicken-and-egg gap where autodetect only saw already-resolved keys

Root cause

When a harness has default_type: oauth-token and GEMINI_API_KEY is stored as as_needed:

  1. Hub's resolveSecrets() correctly filters out as_needed env-type secrets
  2. Broker's autodetect probes resolvedEnvKeys — but GEMINI_API_KEY is in neither ResolvedEnv nor ResolvedSecrets
  3. Autodetect fails, falls back to default_type (oauth-token), which has no env requirements
  4. Hub's pass 2 is never asked for the key → agent starts without credentials

Test plan

  • go build ./... passes
  • go vet ./... passes
  • gofmt -l produces no output for changed files
  • go test ./pkg/runtimebroker/... passes (all tests including 2 new regression tests)
  • go test ./pkg/hub -run 'TestResolveSecrets|TestBuildEnvGather' passes (all resolve/gather tests)
  • New test TestEnvGather_AvailableAsNeededKeys_AutodetectAPIKey verifies autodetect selects api-key auth when GEMINI_API_KEY is in AvailableAsNeededKeys
  • New test TestEnvGather_NoAvailableAsNeededKeys_FallsBackToDefault verifies pre-fix fallback behavior (no AvailableAsNeededKeys → oauth-token default)
  • Existing TestResolveSecrets_HubScope_AsNeeded_Filtered extended to verify asNeededKeys return value

…edKeys (#1447)

When a harness has default_type: oauth-token (e.g. antigravity) and
GEMINI_API_KEY is stored as an as_needed hub secret, autodetect could
not see the key — it was filtered from ResolvedSecrets before the
broker ran, so autodetect fell back to oauth-token (no env requirements),
the hub's pass-2 was never asked for the key, and the agent started
without credentials.

Fix: add AvailableAsNeededKeys to the create-agent request. The hub
populates it with the target key names of as_needed env-type secrets
that were filtered out of ResolvedSecrets. The broker's autodetect
includes these keys when probing for env-var-based auth types, closing
the chicken-and-egg gap.

Changes:
- Add AvailableAsNeededKeys field to CreateAgentRequest (broker) and
  RemoteCreateAgentRequest (hub)
- Extend resolveSecrets() to return filtered as_needed key names as a
  second return value
- Populate req.AvailableAsNeededKeys in buildCreateRequest
- Include AvailableAsNeededKeys in broker's resolvedEnvKeys for
  env-var autodetect
- Add regression tests for both the fix and the pre-fix fallback
@ptone

ptone commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Code Review: APPROVE — Risk LOW

(Self-approval blocked by GitHub; posting as comment.)

Targeted, well-structured fix for the P0 chicken-and-egg bug (#1447). The change is narrow, correct, backward-compatible, and well-tested.

Findings

No Critical or Required findings.

Nit: asNeededKeys could contain duplicates if the same target key appears at multiple secret scopes. Harmless (inserted into a map[string]struct{} on the broker side), but a seen map in resolveSecrets() would deduplicate on the wire. Cosmetic given typical cardinality (1–3 keys).

What I verified

  • Autodetect correctness: AvailableAsNeededKeys is added to resolvedEnvKeys in the autodetect block, so DetectAuthTypeFromEnvVarsFromConfig sees the deferred keys and selects api-key instead of falling through to default_type: oauth-token.
  • Satisfaction check correctness: AvailableAsNeededKeys is intentionally excluded from the satisfaction check (~L2244), so the key still appears in the 202 needs response, triggering the hub's pass 2 to resolve it. This is the correct design.
  • Non-env-gather call sites: DispatchAgentStart and DispatchAgentRestart correctly discard the new return value with _.
  • Security: Only key names are transmitted, never secret values.
  • Backward compat: omitempty JSON tag on both hub and broker types; absent/empty field preserves pre-fix behavior (confirmed by fallback test).

Gates run

  • go build ./... — pass
  • go vet ./pkg/hub/... ./pkg/runtimebroker/... — pass
  • gofmt -l on all changed files — clean
  • go test ./pkg/runtimebroker/... -run TestEnvGather_* — pass
  • go test ./pkg/hub/... -run TestResolveSecrets|TestBuildEnvGather — pass

Address Gemini review feedback on PR GoogleCloudPlatform#1483: our new test functions
should use the canonical `projectPath` JSON key, not the legacy
`grovePath`.
@ptone ptone closed this Sep 7, 2026
@ptone
ptone deleted the fix/envgather-autodetect-as-needed branch September 7, 2026 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Env-gather autodetect blind to as_needed secrets — breaks harnesses with non-api-key default_type

1 participant