Skip to content

gcp-ai: Vertex models and Google Cloud credits with your own gcloud/ADC login - #307

Open
Yaka88 wants to merge 7 commits into
magpie-community:mainfrom
Yaka88:add-gcp-ai
Open

Yaka88 wants to merge 7 commits into
magpie-community:mainfrom
Yaka88:add-gcp-ai

Conversation

@Yaka88

@Yaka88 Yaka88 commented Oct 22, 2026 •

Copy link
Copy Markdown

Adds @magpie-community/opencode-gcp-ai-auth, provider id gcp-ai.

What it signs in to, and how

Not a subscription. The plugin runs on the user's own Google Cloud credentials, which already exist on the machine: gcloud's ADC or its active user. Sign-in opens the Chrome profile the plugin keeps for Cloud Console, reads which Google account is signed in there, and — when there is no ADC yet — lets gcloud's own application-default login write one in that same window, so the models and the Console cookies describe one account.

  • The first sign-in uses the account Chrome already has. Signing in again opens Google's add-account page; an account that is already here adds nothing and says so.
  • Every ADC the plugin writes goes to <config>/gcp-ai/adc-<account>, one directory per account, so two Google accounts coexist. A directory the plugin did not create — gcloud's own, or a CLOUDSDK_CONFIG the user set — is only ever read; a sign-in never writes into it. The machine-wide CLOUDSDK_CONFIG variable is not consulted at all.
  • The plugin never handles a refresh token or client secret: gcloud writes and prints them, and the plugin only reads its stdout.

Models

Read from Vertex's publisher catalog, then each chat candidate is asked one free countTokens; only models that answer are listed. The list is ordered newest first by the catalog's release date. Models a project cannot call are never invented.

The config hook declares a small seed list, as scripts/check.mjs requires; the provider hook replaces it with the account's real list on sign-in. Both carry api: { id, npm: "@ai-sdk/google" }, so a request goes out as gemini-2.5-flash — Gemini's own protocol names the model in the path, without a publisher prefix.

Credits

Cloud Console has no API for promotional credits, so the balance is read from the Console page in that same Chrome profile and returned as provider.usage. A page that never names the account it rendered for is a failed read, not this account's balance: a silent authuser fallback is exactly what that check exists to catch.

Dependencies

None. The browser is driven over a CDP pipe with the plugin's own client; there is no playwright-core and no install script. The sign-in window itself is never driven — Google turns away a browser with a DevTools client attached — so the person approves consent in an ordinary Chrome window and the plugin closes it when gcloud finishes.

Platform

  • gcloud on Windows is gcloud.cmd, a batch file that execFile/spawn cannot run as a binary; every call there goes through cmd.exe with the arguments quoted and windowsVerbatimArguments.
  • The plugin's own files are under %APPDATA% / %LOCALAPPDATA% on Windows.
  • The exit handler only kills the Chromes this process started, synchronously: the plugin runs inside magpie's shared plugin host, and a SIGINT handler that called process.exit there would cut short every other plugin's shutdown.

Verification

  • bun test — 33 pass, 0 fail.
  • bun scripts/check.mjs gcp-ai — ✓ gcp-ai.
  • Two tests state their own timeouts because they open a real Chrome or wait out a network timeout.
  • Run on Linux with two real Google accounts, against Magpie 0.1.1336 and 0.1.1343. Windows and macOS have not been tried with a real account — the README says so.

🤖 Generated with Claude Code

@yetone yetone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work here, @Yaka88. It's a careful package, and Vertex through the user's own GCP project fits this repo. I checked out 1a34b51 in a fresh worktree and ran HOME=$(mktemp -d) bun test in packages/gcp-ai (19 pass, 2 skip) and bun scripts/check.mjs gcp-ai (ok). Then I tried the sign-in paths against a fake gcloud and a stubbed Chrome in a sandbox HOME. One path deletes the user's gcloud configuration, so this needs changes before it can merge.

Blocking

  1. lib/credentials.mjs:441 can rm -rf the user's own gcloud config directory. After gcloud auth application-default login, if the consent page approved a different account, the code runs rm(adc.directory, { recursive: true, force: true }). adc.directory isn't always a directory this run created: settings() in index.mjs passes cloudsdkConfig: auth.cloudsdkConfig || options.cloudsdkConfig || env.CLOUDSDK_CONFIG, and resolveAdcDirectory returns it unchanged.
    Repro: point CLOUDSDK_CONFIG at a directory holding configurations/config_default and credentials.db but no ADC file. Use a fake gcloud whose ADC token's userinfo says other@example.com, and a Chrome stub that shows me@example.com. Sign in as me@example.com. The error says "The consent page approved other@example.com, not me@example.com; nothing was saved". The whole directory is gone (ENOENT): every gcloud account, configuration and credential in it.
    Only ever delete the one file this run wrote (application_default_credentials.json), and only when it didn't exist before.
  2. The default-directory case leaves the wrong account's ADC behind. With no cloudsdkConfig, adc.directory is unset, so nothing is removed. ~/.config/gcloud/application_default_credentials.json keeps the wrong account's credentials, while the message says "nothing was saved". The README's claim that it "removes the ADC directory this run created" is wrong for both cases.
  3. Signing in writes the machine-wide ADC. On first sign-in, application-default login without a dedicated CLOUDSDK_CONFIG writes ~/.config/gcloud/application_default_credentials.json. Every Google client library on the machine then uses it, with this project as its quota project. The plugin should always sign in under its own CLOUDSDK_CONFIG (e.g. under the plugin's data directory) and never write the user's default.
  4. New dependency (package.json:11, playwright-core). No other package in this repo has runtime dependencies, and the maintainer has to approve adding one. Separately, the browser part needs a rethink (5-7).
  5. package.json:15 loads the icon from your own repo (raw.githubusercontent.com/Yaka88/google_vertex_plugin/...), which can change after review. assets/icon.png already ships. Reference that file instead, as the other packages do.

Browser automation

  1. Debugging port and /proc (lib/browser.mjs:48-130, 165). On Linux it launches Chrome with --remote-debugging-port, holding a Google session for up to 10 minutes, and any local process can drive it. It probes 127.0.0.1:9222 and scans /proc/*/cmdline. It also reads other users' Chrome command lines to find one. Please use a pipe transport (Playwright's launchPersistentContext on Linux too) and never attach to a Chrome you didn't start.
  2. The real Keychain / keyring (lib/browser.mjs:249). ignoreDefaultArgs: ["--password-store=basic", "--use-mock-keychain", ...] makes the dedicated profile use the macOS Keychain or the Linux keyring, which prompts the user and stores Google cookies there. Keep Playwright's defaults.
  3. Headless Chrome on every failed usage refresh (lib/credits.mjs:175-201, index.mjs usage). Credits are cached only on success. An account without billing.viewer, or a non-USD billing account, launches headless Chrome on every usage poll, with waits of up to 45 + 20 + 30 s each time. Please cache failures too (with a longer TTL), and make the Console scrape opt-in.
  4. Account check falls back to any email on the page (lib/browser.mjs:327-328). If the keyed field is missing, the first address anywhere in the HTML counts as the signed-in account. That could be a project member, a billing contact or an address in a notification. Treat "not found" as unknown and stop.

Correctness

  1. Models are probed only at locations/global (lib/models.mjs:69), whatever location the user configured. A model available only regionally (or a project restricted to a region by org policy) is dropped, or listed where it can't be called. Probe at the configured location.
  2. Seed ids differ from discovered ids (lib/models.mjs:130-150). Seed models have no api.id and no google/ prefix, while discovered ones use api.id: google/<model>. A model picked before the first discovery and the same model afterwards are two different ids.
  3. No Windows Chrome path. Sign-in fails on Windows. Either support it or say so in the README. From the code, only Linux sign-in looks to have been run. Please say in the PR what you ran it on, with a real account.

1-3 are the must-fix ones. 4-5 need the maintainer. Happy to re-review once they're addressed.

Sign-in
- Each Google account gets its own ADC directory under ~/.config/gcp-ai;
  an account's recorded project, profile and ADC now outrank the row and
  the environment, so several accounts on one machine stay apart
  (GOOGLE_VERTEX_PROJECT is a last resort for a machine with no record).
- The first account on a machine goes straight to Google's consent page;
  later ones start on the account picker so the sign-in can be approved as
  the account being added.
- A duplicate account adds nothing: only the ADC this run wrote is
  removed, and an ADC that was already there is left alone.

Models
- The config hook hands over Google's current chat catalog, so the row is
  usable before anyone is signed in (the same shape Kiro and WorkBuddy
  use). The provider hook replaces it with the account's own list.
- Each catalog entry is asked a free countTokens when the list is read,
  and only a 404 — Google saying the model is not deployed for this
  project — drops it. Retired models stay in Google's catalog and would
  otherwise be offered as models that cannot load.
- The config hook no longer seeds a list Magpie records as "seen", which
  is what kept the vendor's models off the row until a manual fetch.

Browser
- Chrome is driven over CDP on file descriptors 3 and 4 of a process this
  plugin starts; playwright-core and its install step are gone.
- The sign-in window still has no debugging client at all: no pipes are
  passed to it, which is what Google's consent flow requires.
- Credit reads take a tab in one kept browser, and the profile lock is a
  file under ~/.cache/magpie/gcp-ai so separate magpie processes do not
  open the same profile at once.

Credits
- A network-class failure is remembered for a minute instead of twenty,
  so one bad minute does not read as twenty minutes of "timed out".

Tests: 33 pass. scripts/check.mjs passes.

Co-Authored-By: Claude Code <noreply@anthropic.com>

@yetone yetone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the rework, @Yaka88. Dropping Playwright for a pipe-only CDP client and keeping the sign-in window undriven is a real improvement, and most of the first review is addressed. I checked out c0f2643 in a fresh worktree and ran bun test in packages/gcp-ai with a sandbox HOME and TMPDIR on macOS (Bun 1.4.2): 32 pass, 1 fail. bun scripts/check.mjs gcp-ai passes. Then I re-ran the earlier repros against the fake gcloud from test/gcloud.mjs, with a stubbed sign-in window and stubbed userinfo, Resource Manager and Service Usage responses.

Earlier points

  1. rm -rf of a user-supplied CLOUDSDK_CONFIG: fixed. With a directory holding configurations/config_default and credentials.db and no ADC, a consent approved as other@ keeps both files, and only the ADC this run wrote is removed. Making discard() always rm -rf the directory turns two tests red, so the fix is guarded.
  2. Wrong account's ADC left in the default directory: fixed. With no CLOUDSDK_CONFIG, a wrong-account consent leaves ~/.config/gcloud empty and nothing under ~/.config/gcp-ai.
  3. Writing the machine-wide ADC: fixed for gcloud's default directory. A first sign-in writes ~/.config/gcp-ai/adc-me-example-com/ and nothing in ~/.config/gcloud. It is not fixed for a CLOUDSDK_CONFIG the user set (see A below).
  4. playwright-core: fixed, no runtime dependencies.
  5. Icon: not fixed. package.json magpie.icon still points at raw.githubusercontent.com/Yaka88/google_vertex_plugin/main/assets/icon.png. Please point it at this repo's copy, as the other packages do.
  6. Debugging port and /proc: fixed (--remote-debugging-pipe only, and no attaching to a Chrome the plugin didn't start).
  7. Keychain/keyring: fixed (--use-mock-keychain / --password-store=basic).
  8. Headless Chrome on every failed poll: failures are now cached (20 min, or 60 s for network errors), thanks. The Console scrape is still on for every signed-in account, so a headless Chrome starts every 5 minutes per account. Making it opt-in is the maintainer's call, so this one isn't blocking.
  9. Account check: the any-email fallback is gone. But confirmAccount in lib/credits.mjs still returns when the keyed field isn't found, so a page that silently fell back to another authuser is still read as this account's balance. Please treat "not found" as a failed read.
  10. Models probed at global only: fixed, since countTokens now uses the configured location.
  11. Seed vs discovered: the keys now match. But seed models have no api.id, so until the provider hook runs, a request goes out as gemini-2.5-flash rather than google/gemini-2.5-flash. Can you confirm the OpenAI-compatible endpoint accepts the id without the prefix? Also, are all the seed ids (e.g. gemini-4-argon, gemini-3.8-flash) ones Vertex serves today?
  12. Windows: Chrome paths were added. gcloud is started with execFile/spawn("gcloud") and no shell, and on Windows that is gcloud.cmd, which Node won't run that way. Please either try it on Windows or say in the README that Windows is not supported. The PR still doesn't say what it was run on with a real Google account. Please add that.

Blocking

A. A sign-in overwrites an existing ADC in the user's own CLOUDSDK_CONFIG, then says nothing was added. settings() still passes env.CLOUDSDK_CONFIG as cloudsdkConfig. When that directory holds another account's ADC and an account is named (VERTEX_GCLOUD_ACCOUNT or the row's account), resolveAdcDirectory sees the owner mismatch and falls through to if (named && !isGcloudDefault(named)) return { directory: named, exists: false }. startLogin then runs application-default login into that directory. Because a file was already there, login.file is undefined, so nothing is undone.
Repro: CLOUDSDK_CONFIG holds application_default_credentials.json for other@example.com (refresh_token: "OTHERS-ORIGINAL"). Call startLogin({ cloudsdkConfig: sdk, account: "me@example.com", ... }) and approve as me@. The result is "That account was already signed in and its ADC already existed, so no new account or ADC was added", but the file now holds me@'s credentials. other@'s refresh token is gone. Approving as a third account gives the same message and leaves the third account's ADC there.
This is the same class as 1-3. A directory the plugin didn't create should only ever be read. New credentials should always go to adcDirectoryFor(account) under the plugin's root, and an ADC that already exists anywhere should never be the target of login.

B. A lock left by an exited process blocks for 10 minutes on macOS and Windows. abandoned() in lib/lock.mjs checks the pid only on Linux. Elsewhere it waits until the lock is 10 minutes old. This is the one red test on macOS (a lock left by a process that has ended is not waited on). It's also what happens in normal use: magpie's plugin host leaves with process.exit(0) when its stdin closes, and is killed outright in other paths. The async release() never runs, so the .lock file stays.
Repro: a child process does await take(file); process.exit(0). The lock now names a pid that isn't running. A second take(file, { timeout: 5000 }) fails after 5046 ms with "Another process is still using this". With the default 180 s timeout, every credits read and every sign-in (openSignInWindow → claimProfile) on that profile waits 3 minutes and then fails, for 10 minutes. process.kill(pid, 0) (ESRCH means gone) works on every platform, and could replace the Linux-only /proc check or sit beside it.

Smaller things (not blocking on their own)

  • When the new ADC's owner can't be confirmed (userinfo down right after consent), the ADC stays in ~/.config/gcp-ai/adc-pending-XXXX and nothing refers to it again. That leaves a refresh token on disk with no owner. knownAccounts also counts adc-pending-* as an account, because it only checks the adc- prefix.
  • adcSlug maps a.b@x.com and a-b@x.com to the same directory, so the second one is refused as "already signed in".
  • lib/browser.mjs installs SIGINT/SIGTERM handlers that call process.exit(130) inside magpie's shared plugin host. That cuts short the host's own shutdown for every other plugin. An exit handler that kills the child Chromes synchronously would be enough.
  • The PR description still talks about playwright-core, 21 tests and installing from Yaka88/google_vertex_plugin. Please update it.

Once A, B and the icon are fixed, I'll re-run the repros above. Thanks again, this is close.

Yaka88 and others added 3 commits October 24, 2026 16:22
- Inference speaks Gemini's native protocol: npm becomes @ai-sdk/google, the
  base ends at the publisher, and the fetch guard only allows the routes
  Gemini serves there (generateContent, streamGenerateContent, countTokens,
  the catalog). A tool call's thought signature is part of the conversation
  on that protocol, and the reply carries Vertex's own usage numbers.
- CLOUDSDK_CONFIG is never read from the environment. One variable pointed
  every account at one ADC, and a sign-in there overwrote credentials the
  plugin never wrote; a sign-in now always writes this plugin's own
  adc-<account> directory and refuses when that account already has one.
- Windows: gcloud is started through cmd.exe with windowsVerbatimArguments,
  since a .cmd cannot be exec'd directly; config and cache live under
  %APPDATA% and %LOCALAPPDATA%; BROWSER is neutralised with cmd's own rem.
- The lock asks the OS whether the holder is alive with signal 0, so a stale
  lock no longer stalls for ten minutes on macOS and Windows.
- The provider icon is inlined as a data: URI, the only form the host accepts
  besides an https URL; the previous one pointed at this repository's raw URL.

Co-Authored-By: Claude Code <noreply@anthropic.com>
… and the row test's timeout

The fixture's env list grew HOME, USERPROFILE and XDG_CONFIG_HOME at the
front, but the delete loop still started at VERTEX_TEST_GCLOUD, which is
index 3 now: it deleted the state path the fake gcloud reads, so every test
using the fixture crashed with ERR_INVALID_ARG_TYPE. The loop now starts
after it.

The third-account assertion expected a "gcp-ai/adc-…" path from before the
call was given adcRoot; with a root it is <root>/adc-third-example-com, so
the test now asserts the directory the plugin builds for that account, which
is what it was checking for.

The row test waits out a connection to an invalid project, so it states its
timeout as the browser tests do.

Co-Authored-By: Claude Code <noreply@anthropic.com>
…he review

- Credits: a Console page that never names the account is no longer read as
  this account's balance. A silent authuser fallback looks exactly like that,
  so "not found" is now a failed read rather than a pass.
- Seed models carry api.id, so a row that has not been signed in yet sends
  Gemini's id as it stands rather than a prefixed guess.
- knownAccounts no longer counts adc-pending-* staging directories as
  accounts; an unnamed sign-in whose owner cannot be confirmed removes the
  staging directory instead of leaving a refresh token on disk with no owner.
- adcSlug is readable, not unique: a second account whose slug another account
  already owns gets a short hash of its address appended, rather than being
  refused as "already signed in".
- No SIGINT/SIGTERM handler: this module runs inside magpie's shared plugin
  host, and exiting there cuts short every other plugin's shutdown. The exit
  handler now kills the Chromes this process started synchronously.
- README: what this was run on, and the disambiguated ADC directory name.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@Yaka88

Yaka88 commented Oct 24, 2026

Copy link
Copy Markdown
Author

Thanks — that was a thorough round, and both blockers were real. All three blocking items plus the smaller ones are addressed in 607e1a2.

Blocking

A. A sign-in overwriting an ADC in the user's own CLOUDSDK_CONFIG — fixed. settings() no longer reads env.CLOUDSDK_CONFIG at all, and startLogin no longer targets a directory it did not create: a named sign-in always writes adcDirectoryFor(account), and gcloudEnvironment() actively removes CLOUDSDK_CONFIG from the child's environment when the plugin has no directory of its own, so gcloud's token print cannot be answered from a directory the plugin never read. Your repro now ends with other@'s refresh_token: "OTHERS-ORIGINAL" intact and nothing written under it. A named sign-in for an account already filed here refuses before a window opens, and discard() only rm -rfs a directory this run created.

B. A lock left by an exited process — fixed. alive() uses process.kill(pid, 0) on every platform (ESRCH → gone, EPERM → alive), so abandoned() returns true from the pid alone; the /proc start-time check is now only what disambiguates a reused pid on Linux, not the liveness test itself. The macOS red test passes here, and the 10-minute fallback is only reached when the pid cannot be asked about at all.

Icon — fixed: package.json magpie.icon is a data:image/png;base64, URI of the copy in packages/gcp-ai/assets/icon.png, read at load time by the auth hook. Nothing points at raw.githubusercontent.com any more.

Smaller things

bun test — 33 pass, 0 fail. bun scripts/check.mjs gcp-ai — ✓ gcp-ai.

@yetone yetone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the round, @Yaka88. Both blockers from last time are fixed, and the icon is too. But the new head has a regression in the named sign-in, and the slug fix doesn't work, so this needs one more pass.

What I ran, at 607e1a2 (merged onto current main 79e383c in a fresh worktree; the merge is clean): bun test in packages/gcp-ai with a sandbox HOME/TMPDIR on macOS, Bun 1.4.2. Then I re-ran last round's repros with test/gcloud.mjs's fake gcloud and stubbed userinfo, Resource Manager and Service Usage. I loaded the package into magpie 0.1.1370's plugin host from its folder (magpie plugin add <path>, sandbox HOME). No real Google credentials or gcloud are on this machine, so nothing was signed in for real and no model was called.

Earlier points, now fixed

  • A. Overwriting an ADC in the user's own CLOUDSDK_CONFIG: fixed. I set up other@'s ADC (refresh_token: "OTHERS-ORIGINAL") and passed it two ways: as cloudsdkConfig with account: me@, and through the CLOUDSDK_CONFIG env. In both cases the file is unchanged when consent is approved as me@ or as a third account.
  • B. Lock left by an exited process: fixed. The child does take(); process.exit(0) and leaves .lock with its pid. A second take now gets the lock in 1 ms. A live holder is still waited on (1.5 s timeout, then "still using this"). The macOS lock test passes.
  • Icon: fixed. magpie.icon is a data URI, byte-identical to assets/icon.png, and the host shows it.
  • #9 (confirmAccount throws), the signal handlers and adc-pending-* in knownAccounts are fixed too. The host lists the row as gcp-ai, @ai-sdk/google, 12 seed models. magpie sends @ai-sdk/google to <base>/models/<id>:streamGenerateContent?alt=sse (pluginFetch), which GEMINI_ROUTE allows, and the ids carry no prefix. So #11 holds for magpie.

Blocking

  1. Every named sign-in now fails, and nothing is saved. A named sign-in is VERTEX_GCLOUD_ACCOUNT, or a row with options.account. The cause is in startLogin: after consent, the new duplicate check hasAdc({ cloudsdkConfig: await adcDirectoryOwnedBy(adcEmail, root) }) runs on the directory this same run just wrote, because for a named sign-in that directory is adcDirectoryFor(account). So the run's own ADC counts as "already signed in", and discard() removes it.
    Repro: empty plugin root, startLogin({ adcRoot, account: "me@example.com" }), consent approved as me@example.com. At c0f2643 the result is { project: "example-project", account: "me@example.com", cloudsdkConfig: "<root>/adc-me-example-com" }. At 607e1a2 it is "me@example.com was already signed in, so no new ADC was added", and the root is empty.
    The check before the window opens already refuses an account filed here. After consent, compare against the registered snapshot, or skip directory itself. Please add a test where a named sign-in succeeds. None of the current ones does, which is how this got through.

  2. The slug collision isn't fixed. The second address is still refused, and a read can go to the other account's ADC. In adcDirectoryOwnedBy, when the readable directory's owner is definitely another account and the hashed one has no ADC yet, the function returns plain, the other account's directory.
    Repro (unnamed sign-ins, magpie's default Add account path): first@, second@ and a.b@x.com are filed fine. Then a-b@x.com → "a-b@x.com was already signed in", and nothing is added. resolveAdcDirectory({ account: "a-b@x.com" }) then returns adc-a-b-x-com, which is a.b@'s ADC, with exists: true. On a definite mismatch, return hashed whether or not it exists. Please add a test for the pair, in both the named and the unnamed path.

  3. With no gcloud installed, the sign-in never ends. This is the README's first requirement, so it is what a new user hits. beginAdcLogin's finish clears the URL timer on spawn ENOENT but never settles url. startLogin awaits login.url before anything else, so the result never settles, even past its own loginTimeout. "gcloud CLI was not found" is never shown, and an adc-pending-* directory is left behind.
    Repro: PATH without gcloud, startLogin({ ..., loginTimeout: 20000 }). After 90 s it is still pending, no window has opened, and adc-pending-CthPDo is on disk. Through the host, magpie plugin login gcp-ai printed the instructions and then sat there until I killed it at 200 s. Settle url with undefined in finish (or race it against done), and remove the staging directory.

  4. The browser test the signed-in account is only read from the Console's own keyed field has no stated timeout. It starts Chrome and then waits up to 1 s plus 5 s inside Bun's 5 s default. Here it failed 4 times out of 4 at 5001 ms. With --timeout 60000 it passes in 8 s. Give it a timeout like the other two browser tests.

Smaller things (not blocking on their own)

  • README: "Linux, macOS and Windows are supported" contradicts the next section's "has not been run on a Windows machine… macOS has likewise not been tried". Please say "implemented, not tried with a real account" there too.
  • lock.mjs: on macOS and Windows, a pid that was reused by an unrelated process now blocks the lock for good. Before, it was freed after 10 minutes. That's rare, but a ceiling past the 10-minute sign-in limit would bound it.
  • The variants send reasoning_effort, which @ai-sdk/google doesn't read in OpenCode (it wants thinkingConfig). magpie reads only the variant names, so this matters for OpenCode only.
  • The Console scrape staying on for every account is still the maintainer's call, as before.

Once 1–4 are fixed, I'll re-run these repros and the host load.

…of the review

- A named sign-in writes `adcDirectoryFor(account)` itself, so the post-consent
  duplicate check now skips the directory this run just wrote instead of
  discarding its own ADC as an existing sign-in.
- `adcDirectoryOwnedBy` returns the hash-suffixed directory as soon as the
  readable one's owner is confirmed to be another account, even before the
  hashed directory exists, so neither `resolveAdcDirectory` nor an unnamed
  sign-in falls back to the first account's ADC.
- `beginAdcLogin` settles `url` with `undefined` in `finish()`, so a missing
  `gcloud` binary rejects through `login.done` at once, removes the staging
  directory, and never hangs awaiting a consent URL.
- `abandoned()` bounds a lock held by a live PID at 15 minutes so a reused PID
  cannot hold the lock forever where `/proc` start times are unavailable.
- The Console keyed-field browser test declares an explicit 40s timeout, and
  the README notes under Requirements that macOS and Windows support is
  implemented but not yet tried with a real account.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@Yaka88

Yaka88 commented Oct 24, 2026

Copy link
Copy Markdown
Author

Thanks again for catching those! All four blocking items and the smaller ones are addressed in 4586c99:

  1. Named sign-in (lib/credentials.mjs) — fixed: the post-consent duplicate check now compares existing !== directory before treating adcDirectoryOwnedBy(adcEmail, root) as a preexisting sign-in, so a named sign-in no longer discards the ADC it just wrote. Added a named sign-in saves its own ADC and refuses a later duplicate.
  2. Slug collision (lib/credentials.mjs) — fixed: adcDirectoryOwnedBy now returns disambiguated(account, root) immediately when plain's owner is confirmed to be another account, whether or not the hashed directory already has an ADC. Added tests covering a.b@x.com and a-b@x.com across both the named and unnamed sign-in paths and resolveAdcDirectory.
  3. Missing gcloud (lib/credentials.mjs) — fixed: beginAdcLogin's finish() settles url with undefined, so ENOENT immediately flows through await login.done, removes the adc-pending-* directory, opens no window, and surfaces "gcloud CLI was not found". Added a test with an empty PATH for both unnamed and named sign-ins.
  4. Browser test timeout (test/browser.test.mjs) — fixed: added { timeout: 40000 } to the signed-in account is only read from the Console's own keyed field.
  5. Lock ceiling & README (lib/lock.mjs, README.md) — added a 15-minute ceiling in abandoned() (with a test verifying an 11-minute lock is kept and a 16-minute lock held by a live PID is reclaimed when /proc is unavailable), and updated the Requirements note in README.md to say macOS and Windows support is implemented but not tried with a real account.

@yetone yetone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 4586c99 (on main df55294).

Confirmed fixed: the four earlier blockers. A sign-in writes only to a directory the plugin made: adcDirectoryToCreate for a named account, or mkdtemp(<config>/gcp-ai/adc-pending-*) for an unnamed one. gcloud's default directory is refused. discard() removes only a directory the plugin created. A pinned cloudsdkConfig with no account is a read-only import. gcloudEnvironment drops CLOUDSDK_CONFIG when the plugin has no directory of its own. I checked this by reading every write path in lib/credentials.mjs and running the suite in a sandbox HOME. Nothing touched the user's CLOUDSDK_CONFIG or ADC.

bun test passes for the package (Bun 1.4.2, macOS).

Blocking: the tests reach the network

I ran the suite with a preload that logs every fetch and every net/tls connect to a non-local host, and watched the test Chrome processes' sockets with lsof. Three different leaks:

  1. test/plugin.test.mjs:277 "a sign-in with an ADC the plugin already filed counts as an overwrite and fails" never replaces globalThis.fetch. startLogin → adcDirectoryOwnedBy → adcOwner → accountOf sends a real GET to https://openidconnect.googleapis.com/v1/userinfo with the fixture's token. Repro: bun test --preload ./log-fetch.mjs test/plugin.test.mjs -t "already filed", with a preload that wraps globalThis.fetch and prints the URL. Fix: stub fetch in this test, as the other sign-in tests do. Better still, make the default fetch in tests throw, so the next one fails loudly.
  2. Leftover work after a timeout. Under load, "a named sign-in saves its own ADC and refuses a later duplicate" and "accounts with the same slug keep separate ADCs after named sign-ins" went past bun's default 5s. Once the stub was restored, their unfinished promises called the real cloudresourcemanager.googleapis.com and userinfo. Each test should await or cancel its own work before it restores fetch, for example in a finally that aborts the login. These tests also need a stated timeout: they start gcloud and Chrome fakes, and 5s is not enough on a busy CI box.
  3. The browser tests' Chrome talks to Google. routeAll intercepts the pages, but Chrome's own background services don't go through it. During bun test test/browser.test.mjs (5 pass), the Chrome started with --user-data-dir under my sandbox opened 88 sockets to 142.250.x, 142.251.x, 172.217.x (*.1e100.net) and 2408:80f1:… on 443 TCP and UDP. Launch the test browser with --disable-background-networking --disable-component-update --disable-sync --no-first-run --disable-features=OptimizationHints,MediaRouter,DialMediaRouteProvider --host-resolver-rules="MAP * ~NOTFOUND, EXCLUDE 127.0.0.1" (the last one is what actually holds), or run the test with a proxy that refuses everything.

Smaller points (not blocking on their own)

  • lib/lock.mjs abandoned(): the new 15-minute ceiling runs before the Linux /proc liveness check, so a live holder on Linux is reclaimed after 15 minutes as well. A sign-in is limited to 10 minutes, so this is probably fine today. But the check order says "age beats liveness", and that is the opposite of what the /proc branch is for. Put the ceiling after it, or say in a comment why age wins.
  • index.mjs settings(): env.VERTEX_GCLOUD_ACCOUNT || auth.accountId || options.account. The environment also outranks the account record for location, profile and billing. The comment above says the account's record beats the environment. Make one of them agree with the other.
  • resolveAdcDirectory and authorizeAdc are exported, but nothing in the plugin calls them. resolveAdcDirectory returns { directory: <user-named dir>, exists: false } for a directory the user named, which goes against the write policy above. Remove them, or make them follow it, so nobody wires them in later.

Not tried: a real Google account, Windows, node --test.

…ove unused exports

- `test/network.mjs` makes `globalThis.fetch` and external `http`/`https`/`net`/
  `tls`/`dns` calls fail by default across the test suite, so an unstubbed
  request fails immediately instead of reaching the network.
- `test/plugin.test.mjs` stubs `fetch` in the already-filed ADC test, sets
  explicit timeouts on every sign-in test, and awaits `login.close()` (which now
  also drains `settled`) in `finally` before restoring `fetch`.
- `test/browser.test.mjs` passes `--host-resolver-rules=MAP * ~NOTFOUND, EXCLUDE 127.0.0.1`
  and a closed loopback proxy to every test Chrome, and `lib/browser.mjs` adds
  `DialMediaRouteProvider` to `--disable-features`.
- `lib/lock.mjs` places the 15-minute age ceiling after the Linux `/proc`
  start-time check so a live holder on Linux outranks lock age.
- `index.mjs` orders `settings()` as `auth > options > env` for every field,
  matching its comment.
- `lib/credentials.mjs` drops the unused `resolveAdcDirectory` and
  `authorizeAdc` exports.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@Yaka88

Yaka88 commented Oct 24, 2026

Copy link
Copy Markdown
Author

All three test network leaks and the smaller points are addressed in 8989b58:

Blocking (test network access)

  1. Unstubbed fetch in tests (test/network.mjs, test/plugin.test.mjs) — added test/network.mjs (imported by test/plugin.test.mjs and test/browser.test.mjs), which replaces the default globalThis.fetch and blocks non-local http/https/net/tls/dns calls so any unstubbed request fails loudly instead of reaching the network. Also stubbed fetch in "a sign-in with an ADC the plugin already filed counts as an overwrite and fails" and in the provider fallback test.
  2. Leftover work after a test / stated timeouts (lib/credentials.mjs, test/plugin.test.mjs) — login.close() now awaits settled as well as stopping the child and window, every sign-in test awaits login?.close() in finally before restoring globalThis.fetch, and all sign-in / fake-gcloud tests declare { timeout: 30000 }.
  3. Browser tests' Chrome background traffic (lib/browser.mjs, test/browser.test.mjs) — added DialMediaRouteProvider to --disable-features, exposed launchArgs on openBrowser/withPage, and passed --host-resolver-rules=MAP * ~NOTFOUND, EXCLUDE 127.0.0.1 plus --proxy-server=http://127.0.0.1:9 / --proxy-bypass-list=<-loopback> to every test Chrome (and added routeAll to the profile-reuse test).

Smaller points

  • lib/lock.mjs abandoned() — moved the 15-minute ceiling after the Linux /proc start-time check so a live holder on Linux outranks lock age.
  • index.mjs settings() — aligned all fields (project, location, billingAccount, profile, account, authMode) to auth > options > env, matching the comment.
  • Unused exports (lib/credentials.mjs) — removed resolveAdcDirectory and authorizeAdc, and updated the tests to call adcDirectoryOwnedBy and adcDirectoryToCreate directly.

@Yaka88

Yaka88 commented Oct 25, 2026

Copy link
Copy Markdown
Author

Hi @yetone, the fixes for the latest review are in 8989b58. When you have time, could you take another look? Thanks for the detailed reviews so far.

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.

2 participants