Repository navigation
feat(coding-agent): Vertex Model Garden open models via ADC - #3296
Conversation
…on Vertex via ADC Adds the vertex-model-garden builtin extension (vertex-garden: openapi global endpoint; vertex-claude: rawPredict/streamRawPredict) and a lazy ADC helper in pi-ai shared with google-vertex.
code-yeongyu
left a comment
There was a problem hiding this comment.
Thanks a lot for this, @sleegme! Open Vertex Model Garden models via ADC is a really useful addition. The credential handling is careful: tokens go only to aiplatform.googleapis.com, nothing is logged or written to disk, and the new dependency adds no install scripts. A few changes are needed before we can merge.
Blocking
1. The provider counts as configured instead of ambient, so it takes over the automatic default model.
packages/coding-agent/src/core/extensions/builtin/vertex-model-garden/index.ts:11,21 registers apiKey: "$GOOGLE_CLOUD_PROJECT". Any set env-var key is treated as an explicitly configured provider, while the existing google-vertex ADC provider is deliberately marked ambient: true (packages/ai/src/providers/google-vertex.ts:92). Ambient providers never outrank configured ones.
As a result, anyone who has GOOGLE_CLOUD_PROJECT set, including people who only use it for unrelated gcloud tooling, silently gets a Model Garden model (deepseek-ai/deepseek-v4.1-flash-maas) as their automatic default instead of their current one. The two models also show as available when ADC is missing; the failure only appears on the first request.
Suggested fix: make the provider ambient like the other cloud providers, or gate it on an explicit opt-in (a setting or models.json entry). Please add a default-model-selection test for a user with only Vertex ADC plus GOOGLE_CLOUD_PROJECT, asserting the default stays unchanged.
Should fix
2. The project ID travels through the API-key slot.
vertex-model-garden/transport.ts:16-17 sends options?.apiKey as the project. A value stored for this provider via /login or auth.json would win over GOOGLE_CLOUD_PROJECT and end up in the request URL path, even if it is a real secret. Because the env var always provides apiKey, the ADC quota-project / getProjectId() fallbacks in packages/ai/src/api/google-vertex-adc.ts:41-44 are also unreachable from this provider. Suggestion: read the project from its own option or env var, not from the key slot.
3. The real ADC path has no tests.
Both tests inject a fake access resolver (test/suite/vertex-model-garden-extension.test.ts:10-14, 113-116). The wire-shape test against the real transport is great. Nothing exercises getVertexAdcAccess (empty-token error, project precedence, quota project; google-vertex-adc.ts:36-51), the default resolver, or the registration/availability gating. This can run offline, for example by pointing GOOGLE_APPLICATION_CREDENTIALS at a fixture with a fake token endpoint.
4. Paid models show a cost of zero.
vertex-model-garden/models.ts:6,22-24 uses ZERO_COST for two paid MaaS models, so session cost and budget displays would report $0 for real spend. Please use the published prices, or leave cost unset rather than showing zero.
Optional
models.ts:7declaresreasoning: truewithsupportsReasoningEffort: false, so the user's thinking-level choice is silently ignored for both models.- 403 (API not enabled / missing
aiplatform.endpoints.predict) and 404 (model not enabled in Model Garden) come back through the generic error path. A Vertex-specific hint, such as "enable the model in Model Garden" or "run gcloud auth application-default login", would help users. - The
GoogleAuthcache ingoogle-vertex-adc.ts:15-27lives for the whole process, so switching identity with a latergcloud auth application-default loginis not picked up. - We could not confirm that both model IDs exist on the global endpoint. Worth a quick check.
Thanks again, and thanks for your patience!
# Conflicts: # packages/ai/CHANGELOG.md # packages/ai/changes.md
Review fixes for code-yeongyu#3296: - Register `vertex-garden` through a new `ambientAuth` provider-config hook (composer + extension types) instead of an env-var apiKey slot, so the lane is available while Google ADC is present without counting as configured and never takes the automatic default over a provider the user configured. - Read the request project from GOOGLE_CLOUD_PROJECT/GCLOUD_PROJECT, the ADC quota project or getProjectId(), never from the apiKey slot, so a stored secret cannot reach the request URL. - Cover the real ADC path offline (external-account fixture on a loopback token endpoint): token/project precedence, quota project, empty-token error, registration/availability gating and automatic default selection. - Price both models at their published Vertex MaaS list prices and mark every thinking level unsupported, so no level is offered that the wire ignores. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
|
Thanks for the thorough review — all four points are addressed at 1 (blocking) — ambient, not configured.
Tests (all in
The last one is the requested default-model-selection test. On the pre-fix code it fails exactly as you described — the Model Garden model wins ( 2 — project no longer travels through the API-key slot. 3 — offline tests for the real ADC path.
4 — no $0 cost. Both models now carry the published list prices from cloud.google.com ("Deepseek's models" / "GLM's models" on the Agent Platform pricing page, cited in Optional items. Done: Live QA (isolated sandbox, your ADC project One heads-up, not from this PR: Evidence lives under |
code-yeongyu
left a comment
There was a problem hiding this comment.
Thanks a lot, @sleegme! Every blocking point from the first review is fixed. Making the provider ambient means it no longer takes over anyone's default model, the project ID has its own path, the real ADC flow is tested offline, and the prices and thinking levels are right. We found no security concerns in the new diff.
One thing left: a changelog entry
The required changelog gate fails at this head:
$ node scripts/check-pr-changelog.mjs --base 2fc705f55d
missing changes.md coverage: packages/coding-agent/src/core/extensions/types.ts
The new entry in packages/coding-agent/src/core/extensions/changes.md names that path under "What changed", but the policy requires the exact path under all four headings. Add a - \packages/coding-agent/src/core/extensions/types.ts`: ...` bullet under "Why", "Why an extension could not handle it" and "Expected merge conflict zones". If you'd rather not deal with it, "Allow edits by maintainers" is on, so just say so and we'll add the lines ourselves.
A suggestion (not blocking)
Models shown as available with no project set. auth.ts:20-22 only checks that the ADC file exists, while the built-in google-vertex also needs a project and a location (packages/ai/src/providers/google-vertex.ts:84-89). A gcloud user with nothing else configured and no GOOGLE_CLOUD_PROJECT would get these models as the automatic default, and the first request would fail with the project-ID error. Consider reporting the provider as unavailable when no project can be resolved.
Thanks again for the quick turnaround!
|
Thanks again, @sleegme. Since "Allow edits by maintainers" is on, we added the missing changelog coverage ourselves to save you a round: commit b03514c ("docs: changes.md coverage for vertex-garden") puts the We're running the full checks on this head next. The suggestion about reporting the provider as unavailable when no project is set is still open if you'd like to take it, but it isn't blocking. |
# Conflicts: # packages/coding-agent/src/core/changes.md # packages/coding-agent/src/core/extensions/builtin/changes.md
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 38252180 | Triggered | Generic High Entropy Secret | 1b1e12e | packages/coding-agent/test/suite/regressions/3408-subscription-requested-window.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Heads-up on the GitGuardian finding, @sleegme: nothing to do on your side. It points at a dummy token in one of our own regression tests ( |
|
Thanks for adding the changes.md lines and the heads-up on GitGuardian! I took the non-blocking suggestion too:
Checks on this head: scoped tests 8/8, One trade-off: a user whose project comes only from the ADC quota project (no env var) will now see the lane as unavailable, even though a request would resolve it. |
|
Thank you @sleegme, requiring a project for the ambient path is the right call, and mirroring the builtin |
|
Thanks again @sleegme. We just published a senpi release, which moved |
# Conflicts: # packages/ai/CHANGELOG.md
|
Merged |
|
@sleegme, I owe you a correction: the project check we suggested turns out to be wrong for this lane, sorry for the churn. Our review of the new commit found that the lane's own request path resolves the project more broadly than the new gate.
It throws "Vertex AI requires a project ID…" only when all three are missing. The new check in Suggested fix: revert Everything else in the PR passed review and local evidence (install, |
|
Thanks for the update. We reviewed the delta (1b1e12e..68ace29; your main merge at 51244ad leaves the PR's own lines unchanged). One change is needed before merge. HIGH: the new availability gate is stricter than the request path. Two ways forward, either is fine:
MEDIUM: a hidden lane gives no message. With (b), a user who ran LOW: empty-string handling differs from the builtin ( |
|
Review verdict for the new commit HIGH: the availability gate is stricter than the request path. It hides a working setup and contradicts the documented contract.
MEDIUM: a hidden lane gives the user no signal. When the gate drops the lane, it vanishes from listing and auth status ( LOW: empty strings are handled differently from the builtin. Suggested patch: revert patch (equivalent to
|
…roject resolves" This reverts commit 68ace29. The env-only project gate is stricter than the lane request path: getVertexAdcAccess resolves the project from GOOGLE_CLOUD_PROJECT/GCLOUD_PROJECT, then the ADC quota project, then the gcloud project, and only then fails with a clear error. The gate hid a working ADC + quota-project setup. The suggestion behind it was ours; see the review on code-yeongyu#3296. Co-authored with @sleegme, whose Vertex Model Garden work this PR is.
|
@sleegme, as mentioned, we've pushed the revert ourselves so this can make the next senpi release: |
# Conflicts: # packages/ai/CHANGELOG.md # packages/coding-agent/src/core/extensions/changes.md
…er the v2026.10.16-2 release
|
@sleegme, one more housekeeping push from our side so this can merge right away: |
|
Merged as |
Summary
Adds the builtin
vertex-model-gardenextension for Vertex Model Garden open models via Google Application Default Credentials only. Configure ADC (gcloud auth application-default login); the lane joins the same shared credential chain the builtingoogle-vertexprovider already uses — no token script or local proxy.The
vertex-gardenstarter list is exactly:deepseek-ai/deepseek-v4.1-flash-maas— DeepSeek V4.1 Flash (Vertex), global-only, reasoning enabled, 1,000,000-token context from senpi's existingdeepseek-v4-1-flashmodel data. Published list price $0.30/$1.20 per 1M tokens, cache hit $0.03 (promotional: at no charge through 2026-10-16).zai-org/glm-5.2-maas— GLM 5.2 (Vertex), published list price $1.40/$4.40 per 1M tokens, cached input $0.14.Marketplace partner models, including Claude and Grok, are excluded because they are card-billed and promotional credits do not apply.
Request path: global OpenAI-compatible endpoint, ADC bearer auth,
x-goog-user-projectquota header, existing OpenAI streaming. The project comes fromGOOGLE_CLOUD_PROJECT/GCLOUD_PROJECT, else the ADC quota project, elsegetProjectId()— never from an API-key slot.google-auth-librarystays a pinned direct dependency, reached through a Node-only lazy boundary.Maintainer review fixes (head
4a0780618, merged withorigin/main2fc705f55)vertex-gardenno longer registersapiKey: "$GOOGLE_CLOUD_PROJECT". It declares anambientAuthhook on the extension provider config (packages/coding-agent/src/core/provider-composer.ts,.../extensions/types.ts, composed into an ambientApiKeyAuthin.../core/provider-api-key-auth.ts), mirroring the builtin google-vertex ADC lane (packages/ai/src/providers/google-vertex.ts:92). Availability is the resolution of the ADC chain itself (.../vertex-model-garden/auth.ts), so the models are no longer listed without ADC, the auth status carriesambient: true, and the lane never outranks a provider the user configured nor becomes the automatic default over it.fallbackEligible: () => falseis unchanged..../vertex-model-garden/transport.ts(defaultResolveVertexAccess) passesoptions.envonly, so the ADC quota-project andgetProjectId()fallbacks are reachable and a stored secret can never reach the request URL.packages/ai/test/google-vertex-adc.test.ts(5 cases: token minted by the file's own token endpoint,GOOGLE_CLOUD_PROJECT/GCLOUD_PROJECTprecedence, explicit-option precedence, ADC quota-project fallback, empty-token error).packages/coding-agent/test/suite/vertex-model-garden-extension.test.tsadds: default resolver ignores the apiKey slot, quota fallback reachable, registration/availability gating (ambient + available with ADC, unavailable without), and default-model selection unchanged for a user with only Vertex ADC +GOOGLE_CLOUD_PROJECT(+ location for google-vertex). The fixture is a workload-identityexternal_accountADC file whosetoken_urland subject-token URL point at a loopback server — aservice_accountfile cannot be used offline because that flow hardcodes Google's token endpoint — so no request leaves the machine. No network, no real credentials..../builtin/changes.md).Also, since the optional note applied: every thinking level is marked unsupported (
thinkingLevelMapallnull) because these endpoints accept no thinking parameters, so no level is offered that would be silently ignored. The 403/404 hint suggestion was left out to keep the diff scoped; it needs a wire-error seam the transport does not own today.Validation at
4a0780618packages/ai5/5 (test/google-vertex-adc.test.ts),packages/coding-agent7/7 (test/suite/vertex-model-garden-extension.test.ts). Provider/auth-adjacent files also run green (82 + 33 tests).red-pre-fix.txt.node scripts/check-pr-changelog.mjsexactly as CI runs it (CHANGELOG_GATE_BASE= PR base322c56ef…, no label): exit 0, 16 production paths covered;node scripts/audit-changes-md.mjs: 489/489 covered, exit 0.NODE_OPTIONS=--max-old-space-size=6144 bun run check: exit 2 — the only failures are two pre-existingtscerrors fromorigin/main's catalog regeneration (packages/ai/test/context-overflow.test.ts:581andpackages/ai/test/total-tokens.test.ts:749still nameanthropic/claude-sonnet-4, which main's regeneration dropped from the OpenRouter catalog). A cleanorigin/mainworktree fails identically, and both files are untouched by this branch. Every other step (biome, pinned deps, ts-imports, lockfiles, platform lock, browser smoke) passed; full receipt inroot-check.txt.able-brace-426917-c5on the global endpoint, isolated senpi sandbox, real auth asserted unchanged: both IDs exist and answer —git diffscan for home paths, tokens and credential patterns: clean. Evidence stays ignored and uncommitted.Receipt excerpts:
Receipt hashes (
local-ignore/qa-evidence/20261016-vertex-garden-maintainer-fixes/, also archived in the shared checkout):Branch history
a638f4dfa: mergedorigin/main(earlier base) with a merge commit, preserving both sides of both tracker conflicts.ada7c709f: trimmed the implementation to the two ADC open models.308fc3f7: earlier gate fixes (changelog entries, tracker coverage, duplicate-test removal).cd403e1b7: merge oforigin/mainat2fc705f55; tracker conflicts resolved by keeping both sides.4a0780618: this review round — ambient registration, project off the key slot, offline ADC tests, published prices, thinking-level consistency.Normal push; no force-push or rebase. Do not merge: maintainer re-review is required.