feat(auth): close the sign-in browser when the sign-in is done - #767
Conversation
`gflow auth login --browser chrome` asks the user to close Chrome by hand because `RealChromeStrategy` runs it as a bare subprocess with no debugging port, so there is no signal channel. Sibling `notebooklm-py` auto-closes by owning the browser instead. Whether gflow's zero-automation-surface constraint is still load-bearing at sign-in had never been measured. Three arms, human-driven, each on a fresh unauthenticated throwaway profile, $0: bare real Chrome, no stealth flags webdriver=True BLOCKED at /v3/signin/rejected (17.5s) stealth real Chrome + stealth flags webdriver=False PASS, cookie at 59.4s bundled bundled Chromium + same flags webdriver=False PASS, cookie at 276.0s `bare` is the control that makes this conclusive. Without it two passes would have read as "the block no longer fires", which would have justified dropping the mitigations. It fires. The discriminator is not the browser binary — bundled Chromium, the browser KNOWN_ISSUES names as rejected, passed with flags, while real Chrome was blocked without them. A Playwright connection is fine; an advertised one is not. Both passing arms detected the session from the owned context and closed the window themselves, so auto-close is reachable. Also records a doc defect: KNOWN_ISSUES describes RealChromeStrategy as driving Chrome via Playwright `channel="chrome"`, which has never been true — real_chrome.py was born as passive capture at eb0de13 and `git log -S'channel="chrome"' -- src/gflow_cli/auth/` returns zero commits. Not measured: headless (every arm was headed), generation (reCAPTCHA-gated, separate), which flag does the work, and anything beyond N=1 account/machine/IP/day. Refs #480
`gflow auth login` told you to close the Chrome window yourself, because `RealChromeStrategy` launched Chrome as a bare subprocess with no debugging port and so had no way to know when you were finished. It now owns the browser through Playwright and closes it for you. Measured 2026-09-08 (docs/superpowers/spikes/2026-09-08-g12-blocks-webdriver-not-playwright.md), three human-driven arms on fresh profiles: real Chrome, no stealth flags webdriver=True BLOCKED at /v3/signin/rejected, 17.5s real Chrome + stealth flags webdriver=False signed in, closed itself at 59.4s bundled Chromium + same flags webdriver=False signed in, closed itself at 276.0s The discriminator is `navigator.webdriver`, not the browser binary. Google rejects a browser that advertises automation, not an automated browser — so owning the context is fine as long as the advertisement is suppressed. What this changes: - `real_chrome.py` drives Chrome via Playwright `channel="chrome"` by default, polls the Flow session endpoint from the context it owns, and closes when the session is AUTHENTICATED. Not on a cookie name: the cookie can be present while the endpoint still rejects, and `verify_flow_profile` reads the on-disk store, which Chrome batches. The on-disk probe still runs after close, unchanged, as the durability check. - Closing the window yourself is still a first-class way to finish. It routes to the same verification and never raises — three releases of docs told users to do that. - The bare-subprocess flow is retained and runs automatically when no Chrome channel resolves, when the launch fails, or when Google rejects the browser. There is no flag and nothing to choose. `channel="chrome"` resolves only to Google Chrome, while the auth gate deliberately accepts plain Chromium, so without this a Chromium-only Linux user who can onboard today would have been locked out. - `headless=True` routes to the subprocess path: every spike arm was headed, so a headless Playwright sign-in is unmeasured against Google's gate. - `internal_chromium.py` gains the same flags. It had none — which is the exact configuration measured as BLOCKED — and a test asserted the flag must be *absent*. Its emulated 1920x1080 viewport is replaced by a real window: an explicit viewport is emulated independently of the OS window and pushed Google's sign-in form off-screen on scaled displays, with no zoom recovery. - `CHROME_BINARY` no longer makes the Playwright chrome channel look resolvable. Playwright honours a custom binary via `executable_path=`, never via `channel=`, so that path handed Playwright a channel it could not launch. Teardown goes through `close_context_bounded` + `run_teardown_step` in pw -> lease order, so a cancelled login still stops the driver and releases the lease. The two cancellation guards that were fixed twice before are kept for the subprocess path and mirrored for the Playwright path as order assertions. Not verified: no live run of either path (sign-in needs a human at a Google password prompt — no e2e test can cover it, and all 39 e2e tests consume an already authenticated profile); the browser-rejected fallback is exercised by simulation only. Evidence is N=1 — one account, one Windows host, one IP, one Chrome build, one day — and shipping this as the default rather than opt-in is a maintainer decision recorded in the plan, taken against two personas' advice. The follow-up measurements that would close that gap are listed there. Refs #480
Council review of 8172fb0 across five dimensions. Security (D3/D10) and over-engineering (D14) came back green; correctness found one real bug and docs found one blocker. **The bug.** `poll_session_until_authenticated` treated every `PlaywrightError` as "the browser is gone" and stopped polling. But `playwright.async_api.TimeoutError` subclasses `Error`, so a 15 s request timeout, a DNS hiccup or a Wi-Fi reassociation arrived on the same arm. On the owned-browser path that meant gflow closed Chrome out from under a user still on Google's password screen, logged `auth_login_browser_closed_by_user`, and reported exit 8 "No sign-in detected" for a sign-in that had not failed. It now asks the page whether it is actually closed; everything else retries until the deadline. HTTP-level failures never reached this arm at all — they come back as VERIFICATION_ERROR and keep polling — so only transport errors were ever conflated. `test_manual_close_is_not_an_error` had to change with it: it described a browser that raised on every request while insisting it was still open, which is a state Chrome cannot be in. The fake now closes the page, because that is what a closed browser does. **The doc blocker.** `docs/ARCHITECTURE.md` said the auth path bypasses detection with "a JS `add_init_script` that overrides `navigator.webdriver`". That code has never existed in `auth/` — only in the two generation paths. It sat fifteen lines above the paragraph 8172fb0 rewrote, which means that commit fixed one instance of "documentation describing an implementation nobody built" while leaving another untouched in the same file. Also corrected, all same class: - `AuthBrowserRejectedError`'s remediation told users to install Chrome and rerun with `--browser chrome`. The spike disproved exactly that: bundled Chromium signed in fine with the flags, real Chrome was rejected without them. Exit 14 is now reachable only from the internal strategy, so the advice was both wrong and aimed at the one path it cannot help. A test asserted the old string, so the wrong guidance could not be fixed without a test failing to defend it. - `README.md`, `AGENTS.md`, `docs/DEBUGGING.md`, `docs/AUTHENTICATION.md` and two `website/docs/` pages still stated the retired "Google rejects bundled Chromium" claim; two of those website pages contradicted, in an adjacent unedited sentence, what 8172fb0 had just written above them. - `client.py`'s Chrome-not-found hint still offered `CHROME_BINARY`, which no longer helps: Playwright honours a custom binary via `executable_path=`, never `channel=`. - The CHANGELOG attributed the 17.5 s rejection to bundled Chromium. That number is the `bare` arm — real Chrome with the flags removed. Bundled-without-flags was never run, and the entry now says so. **Tests.** `raise_on_close`'s default was unpinned: flipping it left the suite green while turning a failed internal login into a reported success with no `.gflow_account`. Both sides are now pinned, and the guard was A/B-verified by mutation, as was the transient-failure fix and the headless routing guard. 224 passed; ruff, mirror, doc-links, PII, hygiene and council-memory gates green; pyright 86 = 86 against an untouched-checkout control.
`/fx/api/auth/session` is a NextAuth route that can rotate session cookies, and the poll was hitting it every 3 s for the whole login — including while Google held the page for the OAuth callback. Observed live 2026-09-08: the sign-in landed on `labs.google/fx/api/auth/signin?error=OAuthCallback` and then timed out at 600 s. The spike that signed in successfully twice never made this request at all: it read the cookie jar locally over CDP and issued no HTTP during sign-in. Waiting until the page is back on the Flow host restores that property. Also adds a `probe=` discriminator to `auth_login_session_detected`. RealChromeStrategy runs both oracles — the in-context live probe decides when to close, then verify_flow_profile re-checks what landed on disk — and the live run emitted two identical events, leaving "did the on-disk check pass?" unanswerable from the log.
The poll guard added in the previous commit gated on `labs.google` alone. The labs app `location.replace`s a migrated account onto flow.google.com right after the callback returns — server-decided per account, one-way — so the guard would go False on the redirect and never come back, reproducing the exact 600 s timeout it was written to fix, on every migrated account. Both maintainer accounts are migrated, so every live-verify run would have hit this. Neither host is an OAuth handshake host, which is all the guard needs to exclude, so admitting both preserves its intent. A/B control: with the fix the class is 3 passed; gated on labs alone, test_migrated_host_still_polls fails with AuthLoginTimeoutError.
`test_never_logs_a_google_url` parks the page on a secret-bearing Google URL and asserts the poll logs no page URL. With the new host guard the page never left accounts.google.com, so the poll skipped every iteration and the test hung until the 600 s timeout — it was the last test in the file and took the whole suite with it. The page now arrives on the Flow host while the poll is waiting, which is what a real sign-in does. The assertion is unchanged and its intent is stronger: the secret-bearing URL was genuinely visited, and still reaches no log event. 18 passed in 0.32 s, down from a 600 s hang.
Three strategy tests built a page with `goto` and `request.get` mocked but no `url`, so the attribute stayed a bare MagicMock. That is not a str, the poll's new host guard read it as "still mid-OAuth", and the loop spun against a patched `asyncio.sleep` — which records every call, so the run climbed to 14.7 GB before the OS killed it. The doubles now say what the runtime does: the strategy has just navigated to GEMINI_URL, so the page is on the Flow host. 20 passed in 2.11 s.
7805201 to
df9b516
Compare
Live-verified 2026-09-09 — all three required runs passRebased onto Host/profile named per (a) Auto-close fires end to end — PASS
Reproduced three times (84.8 s, 63.7 s, 51.6 s). (b) Manual close verifies normally, never exit 12 — PASSNote the absence of This took four attempts and an instrument to get honestly. Attempts 1–3 all came back exit 0 and all three were not (b): sign-in takes ~50 s, detection fires the moment Flow loads, and auto-close won every race — the logs were shape-identical to (a). Widening (c)
|
…he deadline The host guard `continue`s without touching Playwright, so while the page sits on accounts.google.com nothing ever raises — and `page.is_closed()` was only consulted inside `except PlaywrightError`. A user who abandons a 2FA challenge closes the window on Google's host, which is exactly where that reactive detection cannot fire. Measured before this change: a full run to the deadline with the session endpoint touched 0 times. On the default 600 s timeout that is a ten-minute wait ending in exit 12, for someone who closed the window after thirty seconds. Checking liveness first costs one call per poll and ends the run immediately with `auth_login_browser_closed_by_user`, which routes to the on-disk verify like any other manual close. Introduced by the host guard two commits back, so it never shipped. Found by asking what happens when the two timing layers — how long the user takes, and which host they are on while taking it — interact. Three strategy doubles needed an explicit `is_closed = False`: a bare MagicMock attribute is truthy, so the new check read "already closed" and broke before doing anything under test.
…g CI The passing path returns instantly, so the deadline in this test only matters when the liveness check regresses — and at 600 s that is a ten-minute CI hang rather than a red test. Verified by neutering the check: 5 s gives `1 failed in 5.30s`; 600 s gave a hang that had to be killed by hand.
A third defect, found by asking how the two timing layers interactThe maintainer asked a design question after the live runs: does the browser close on a timeout, or by monitoring the process? The answer is neither — it closes on application state ( But the follow-up — which host is the user on while taking that time — exposed a real bug, now fixed in
|
| arm | result |
|---|---|
| with fix | 1 passed in 0.11s |
| liveness check neutered | 1 failed in 5.30s — AuthLoginTimeoutError |
Gates at 7ee912c0: 122 passed, ruff check / ruff format --check / hygiene / doc-links / website-mirror all green.
Still open, honestly
The 2FA case is verified by unit test and by a direct probe, not by a live run — closing a real window mid-challenge was not re-run against Google. And auth_login_timeout remains 600 s by default, which is a real ceiling for a slow 2FA flow (an authenticator on another device, an SMS that does not arrive, a recovery challenge). It is configurable to 86400 via GFLOW_CLI_AUTH_LOGIN_TIMEOUT, but the default is worth revisiting separately.
The 2FA-abandon case is now live-verified tooThe previous comment listed this as verified by unit test and probe but not by a live run. It is now. Deterministic method, because hand-timing it does not work on this machine: launch
The 15 s total is launch + the deliberate 5 s wait + 2 s detect + the on-disk verify; none of it is latency in the path under test. Why hand-timing failed, which is itself worth recordingTwo attempts to close the window by hand both came back Same lesson as the (b) runs earlier in this PR: when the happy path can outrace the branch you mean to test, drive the race deterministically instead of asking a human to win it, and assert on a path-unique event rather than the exit code both paths share. Timing layers, since this is what prompted the whole investigationThe close is triggered by application state — Two real constraints remain:
|
This branch rewrote `AuthBrowserRejectedError`'s remediation because the 2026-09-08 spike refuted its premise: bundled Chromium signed in normally WITH the stealth flags, while real Chrome WITHOUT them was rejected at /v3/signin/rejected in 17.5 s. The discriminator is `navigator.webdriver`, not the browser binary. The test still asserted the old strings — "--browser chrome" and "GFLOW_CLI_AUTH_BROWSER=chrome" — so it was pinning advice that would send a user to swap browsers over a setting. A test protecting a defect, which is the fourth instance of that pattern on this branch. The assertions are inverted deliberately: the disproved advice must not come back, and the message must name the real discriminator. Missed locally because my runs were scoped to tests/auth/ and tests/test_browser_manager.py; CI runs the whole suite. It also took SonarCloud down with it — the coverage artifact is produced by the test job, so the gate had nothing to analyse.
Council review of the eight unreviewed commits on this branch returned RED, and
the headline finding refutes what the previous commit's comment claimed.
**The host guard never protected the OAuth callback.** NextAuth runs its
callback on the app's OWN origin, so a labs.google host test passes straight
through it. Verified:
_is_on_flow_host("https://labs.google/fx/api/auth/callback/google?state=S&code=C")
-> True
_is_on_flow_host("https://labs.google/fx/api/auth/signin?error=OAuthCallback")
-> True
That second URL is the exact failure the comment cited as observed. Excluding
only accounts.google.com excluded the one phase where the callback is NOT
running. The gate now also excludes NextAuth's own auth routes, and the comment
no longer claims more than it does.
Three further fixes fall out of reusing the existing classifier:
- `urlparse("https://[bad").hostname` raises ValueError. It escaped the helper
into the loop's catch-all `except Exception: break`, which under the
InternalChromium default reports "Browser closed before the Flow editor
sign-in was verified" for a browser that is open.
- The helper was a FOURTH copy of the Flow-host set. `flow_host_kind`
(api/transports/_common.py) is strictly stronger: https required, exact host
rather than substring, ValueError-safe, non-str-safe.
- Its import must be deferred — a module-level one cycles through
profile_store back into gflow_cli.auth. The review asserted there was no
cycle; there is, and the import failed immediately.
Also: `POLL_INTERVAL_SECONDS` governed only ONE of the loop's two sleeps, the
other being a literal 3. That is why widening it during live-verify did not do
what I said it did — the main poll path never changed cadence. Both sites now
use the constant.
Two tests polled with timeout_seconds=30, so a regression would spin 30 s and
~1.1 GB rather than failing fast. Now 5, matching the sibling test.
Docs: three exit-14 remediation sites still prescribed `--browser chrome`,
which the 2026-09-08 spike disproved and which contradicted the test added in
0462c73. The reCAPTCHA-on-generation mention of `--browser chrome` in
DEBUGGING.md is a different surface the spike says nothing about, and stays.
New tests pin the route gate across both cohorts, both auth routes, Google's
host, a malformed URL and a bare mock. Full suite with coverage, as CI runs it:
4146 passed.
1 participant
Summary
gflow auth logintold you to close the Chrome window yourself, becauseRealChromeStrategylaunched Chrome as a bare subprocess with no debugging port and had no completion signal. It now owns the browser through Playwright and closes it for you.Evidence — measured 2026-09-08, three human-driven arms, fresh profiles, $0
navigator.webdriverbareTrue/v3/signin/rejected, 17.5 sstealthFalsebundledFalseThe discriminator is
navigator.webdriver, not the browser binary. Google rejects a browser that advertises automation. Thebarecontrol is what makes this conclusive — without it, two passes would have read as "the G12 block is gone" and justified dropping the mitigations. It fires, in 17.5 s.What changed
channel="chrome") is the default; polls the Flow session endpoint from the owned context and closes onAUTHENTICATED— not on a cookie name, which can be present while the endpoint still rejects.headless,channel_unavailable,launch_failed,browser_rejected. Not optional:channel="chrome"resolves only to Google Chrome while the auth gate deliberately accepts plain Chromium, so without it Chromium-only Linux loses onboarding.internal_chromium.pygains the same flags. It had none — the configuration measured as blocked — and a test asserted the flag must be absent.CHROME_BINARYno longer makes the Playwright chrome channel look resolvable; Playwright honours a custom binary viaexecutable_path=, neverchannel=.Council review (5 predict personas + 4 review dimensions)
Security/auth 🟢 · over-engineering 🟢 · tests 🟢 (verdicts earned by runtime mutation, not assertion). Correctness found a real bug and docs found a blocker; both fixed in
7805201e:playwright.async_api.TimeoutErrorsubclassesError, soexcept PlaywrightError: breaktreated a request timeout as "the user closed it" — closing Chrome mid-sign-in on a network blip and reporting exit 8.docs/ARCHITECTURE.mddescribed anadd_init_scriptauth stealth mechanism that has never existed — the same defect class this PR fixes, in a file it edited.Not verified — why this is a DRAFT
No live run has exercised the shipped path yet. The spike proved a spike script's poll-and-close;
_login_owned_browser, the bounded teardown and the_verify_and_recordhandoff have not run against a real Chrome. One 600 s attempt timed out un-driven — it proved launch, copy and clean teardown (no orphaned Chrome), not detection.Required before this leaves draft:
gflow auth login→ window closes itselfNo e2e test can cover this: all 39 e2e tests consume an already-authenticated profile, and sign-in needs a human at a Google password prompt.
Evidence is N=1 — one account, one Windows host, one IP, one Chrome build, one day, all arms headed. Shipping as default rather than opt-in is a maintainer decision taken against two personas' advice, recorded in the plan with the follow-up measurements owed.
Merge order
After #764 (fixes #763, ready now). Only
CHANGELOG.mdcollides.Test plan
raise_on_closeguardsRefs #480