I have what I need. Let me quickly confirm the integration-test mock-server pattern so the test plan is concrete.

Bottom line: build a new authcode package in auth-go mirroring deviceflow, then shim it into the CLI's auth.Client and flip entire login to use it by default with --device as the fallback. The library has zero authcode code today and the internal/oauthhttp helpers give you everything (PKCE-adjacent HTTP, loopback validation, error parsing) except PKCE itself. The entire-core implementation is a clean, near-complete blueprint to port. One thing must be confirmed against the backend before coding: the exact authorize endpoint path and which host serves it.

Plan: loopback authorization-code login (default) + --device fallback

Phase 0 — Confirm backend facts (blocking)

The entire-core prior art and the CLI talk to different hosts, so don't assume its paths transfer:

Action: pull the auth host's /.well-known/openid-configuration and verify the client registration. Everything below assumes: authorize endpoint on the auth host, PKCE S256, loopback any-port redirect allowed.

Phase 1 — New authcode package in auth-go

Files (mirror deviceflow/):

• authcode/authcode.go — client + flow
• authcode/testseams.go — SetNowForTest per-Client atomic.Pointer clock (copy deviceflow's pattern verbatim)
• authcode/authcode_test.go — httptest-driven

Client struct (mirror deviceflow.Client):

• Transport http.RoundTripper, BaseURL, ClientID, Scope, UserAgent
• AuthorizePath string, TokenPath string
• RequestTimeout time.Duration, AllowInsecureHTTP bool, nowOverride atomic.Pointer[...]
• New(c *Client) (*Client, error) validating required fields.

Public API — recommend a decomposed shape so the CLI keeps control of browser-open and user messaging (the CLI already owns openBrowser/TTY logic):

Rationale: a one-shot Login(ctx, openFn) is tempting but couples the lib to browser-opening and progress output. Decomposed mirrors how deviceflow splits StartDeviceAuth/PollDeviceAuth and keeps the lib I/O-free except the loopback server. Return the RFC 9207 issuer from Wait so the CLI can route the exchange cross-region.

Port the entire-core internals (client/clilogin/loopback.go), adapting to auth-go conventions:

• PKCE: newPKCEPair() — 48 random bytes → verifier, base64.RawURLEncoding(SHA256(verifier)) → challenge, method S256. (loopback.go:368–375)
• State: 24 random bytes base64url. (loopback.go:137–140)
• Listener: net.ListenConfig{}.Listen(ctx, "tcp", "127.0.0.1:0"), callback path /callback, redirect http://127.0.0.1:<port>/callback. (loopback.go:142–152)
• Authorize URL: response_type=code, client_id, redirect_uri, scope (add cli offline_access — entire-core omitted scope; the CLI needs the refresh token, matching the device-flow client), state, code_challenge, code_challenge_method=S256. (loopback.go:213–219)
• Callback handler: validate state (400 on mismatch, don't kill flow), handle error param, require non-empty code, capture iss, write success HTML, signal buffered channel. (loopback.go:165–202)
• Success HTML constant ("You're signed in, close this tab"). (loopback.go:385–404)
• Exchange: POST TokenPath, grant_type=authorization_code + code/redirect_uri/code_verifier/client_id. (loopback.go:278–346) — but parse into *tokens.TokenSet and reuse oauthhttp helpers instead of hand-rolling (entire-core hand-rolled because it predates auth-go).
• Timeout: context.WithTimeout default ~5 min; graceful Shutdown. (loopback.go:227–251)

Reuse internal/oauthhttp (importable — same module):

• HTTPClient(transport), ResolveURL(base, path, allowInsecure) for both authorize + token URLs (also gives the absolute-path/redirect-attack defense for free).
• ReadAndDecodeJSON (token response), ReadOAuthError + SanitizeDescription (error bodies), ExpiresInDuration (clamp expires_in → TokenSet.ExpiresAt).
• ValidateOriginURL/IsLoopbackHost for the BaseURL/loopback enforcement (HTTPS-required unless AllowInsecureHTTP + loopback — identical posture to deviceflow).

Error sentinels (deviceflow style): ErrStateMismatch, ErrCallbackMissingCode, ErrAuthorizationDenied (maps error=access_denied), ErrListenerClosed, ErrTimeout.

Conventions: Go 1.26.4, errors.New sentinels, //nolint:wrapcheck on oauthhttp pass-throughs, parallel-safe per-Client clock, httptest servers with AllowInsecureHTTP=true. Add a doc.go blurb and a CHANGELOG.md ## Unreleased → ### Added entry. Also flip README's non-goals — it currently lists "PKCE / authorization code flow" as out of scope (README:203).

Release: tag auth-go (e.g. v0.5.0), since this is additive. The CLI consumes via a go.mod bump.

Phase 2 — Wire into the CLI's auth package

• Provider config (cmd/entire/cli/auth/provider.go): add AuthorizePath to Provider; populate v1 (/oauth/authorize?) and v2 (from discovery doc, likely /authorize). Confirmed in Phase 0.
• auth.Client (cmd/entire/cli/auth/client.go): hold an *authcode.Client alongside the existing *deviceflow.Client, built with the same issuer/scope/transport/AllowInsecureHTTP logic (including the isLoopbackHTTP(issuer) auto-permit). Add shim methods:
  • StartBrowserAuth(ctx) (*BrowserAuthSession, error)
  • WaitForCallback(ctx, session) (code, issuer, error)
  • ExchangeCode(ctx, session, code) (access, refresh string, error) — returns the same (access, refresh) pair shape runLogin already persists. Keep types aliased/wrapped so login.go stays decoupled from auth-go directly, exactly like DeviceAuthStart/DeviceAuthPoll today.

Phase 3 — entire login command surface (cmd/entire/cli/login.go)

• Add var useDevice bool + cmd.Flags().BoolVar(&useDevice, "device", false, "Use device-code flow (verify a code in your browser) instead of the default browser redirect"). Keep it visible (unlike --insecure-http-auth).
• RunE branches:
  • useDevice → existing runLogin (device flow), unchanged.
  • else → new runBrowserLogin.
• runBrowserLogin(ctx, outW, errW, client, openURL):
  1. session := client.StartBrowserAuth(ctx) (binds loopback, builds URL).
  2. If interactive.CanPromptInteractively(): print "Opening your browser to sign in…", openURL(ctx, session.AuthorizationURL); on open failure, print the URL for manual paste (the loopback server is already listening — same-host browser still works).
  3. code, issuer := client.WaitForCallback(ctx, session) with a spinner/"Waiting for sign-in…" message.
  4. access, refresh := client.ExchangeCode(ctx, session, code).
  5. Reuse the existing tail of runLogin verbatim: validateReceivedToken (use issuer if returned, else client.BaseURL()), store.SaveToken, auth.RecordLoginContext(access, refresh, true), "Login complete." Extract that tail into a shared persistLogin(...) helper so both flows share it.
• Headless / non-interactive auto-fallback (recommended): when !interactive.CanPromptInteractively() (CI, SSH without /dev/tty, piped), the loopback browser flow can't work — auto-fall back to the device flow and print a one-line note ("No interactive terminal; using device-code flow"). This avoids forcing users to remember --device on remote boxes. Flag this as a UX decision; the alternative is to hard-error and tell them to pass --device.
• requireSecureBaseURL / --insecure-http-auth unchanged — already covers the auth host the authorize+token calls hit.

Phase 4 — Tests

auth-go (authcode_test.go): httptest server implementing /authorize-style redirect + /oauth/token. Cover: happy path (PKCE verifier echoed → token), state mismatch rejected, error=access_denied param, missing code, token-endpoint OAuth error, timeout via canceled ctx, refresh token plumbed into TokenSet. Mirror deviceflow_test.go structure + SetNowForTest.

CLI unit (login_test.go): add a browserAuthClient interface + fake; test runBrowserLogin success, open-browser-fails fallback path, ctx-cancel, and validateReceivedToken still gates the issuer. Test the --device flag routes to runLogin. Test the headless auto-fallback decision.

CLI integration (integration_test/login_test.go): extend the mock server (currently device-only) with the authorize-redirect + authorization_code token branch. The test harness must (a) read the printed authorize URL, (b) extract redirect_uri, (c) GET the loopback callback with a valid code + matching state, (d) assert Login complete. This replaces the device-flow /approve POST with a loopback callback GET. Keep the device-flow integration tests too (now exercised via --device).

Vogon/E2E: entire login isn't in the agent E2E path, so no Vogon changes expected — verify no canary references the device prompt wording.

Phase 5 — Docs

• CLAUDE.md/command-layout docs: note login now defaults to browser redirect, --device for the old flow.
• auth-go CHANGELOG.md + README non-goals (Phase 1).
• Any --help long-text on the login command.

Key risks / footguns

• Backend path/registration mismatch (Phase 0) — the single thing that can invalidate the whole plan. Confirm before coding.
• Scope: entire-core's loopback omits scope; you must send cli offline_access or you lose the refresh token and silent refresh breaks.
• Don't hand-roll the token exchange like entire-core did — route it through oauthhttp so error/HTML/expiry handling matches the rest of auth-go.
• Cross-region iss: the CLI is split-host/multi-region; if the AS returns iss, the exchange must target it (entire-core does this). Ignoring it could break multi-region logins.
• Loopback reachability: the callback hits 127.0.0.1 on the CLI host — useless over plain SSH. The headless auto-fallback to device flow is what keeps remote logins working.

Want me to start with Phase 0 (pull the discovery doc / confirm client registration) before writing any code?

No — to be clear, the CLI does use auth-go today. I may have muddied it. Correcting:

• entireio/cli (this repo) uses auth-go now. cmd/entire/cli/auth/client.go wraps github.com/entireio/auth-go/deviceflow, and the package also pulls refresh, sts, tokens, tokenmanager. Device login, token refresh, and STS exchange all run through auth-go.

• The "predates auth-go / hand-rolled" remark was about entire-core in the entiredb repo — a different codebase. Its client/clilogin/loopback.go implements the loopback flow with raw net/http and does not import auth-go. That's the prior art to port, not a description of this CLI.

So the shape of the plan holds:

• auth-go has device flow but no authcode/loopback flow → add a new authcode package there.
• The CLI already depends on auth-go, so consuming the new package is just a go.mod bump + a shim in cmd/entire/cli/auth/client.go, exactly like the existing deviceflow shim.
• entire-core's loopback code is the reference implementation to translate into auth-go conventions.

The reason I kept contrasting hosts/client-ids: entire-core logs in against the core host with client_id entire-core-cli, while this CLI logs in against the auth host (us.auth.entire.io) with client_id entire-cli. Same flow mechanics, different endpoints — which is why Phase 0 (confirm the authorize endpoint/registration on this CLI's auth host) matters before porting.

Bottom line: almost all of entire-core's "hand-rolled" code is already plain Go stdlib — there's no scary custom crypto. PKCE is ~8 lines over crypto/rand + crypto/sha256 + encoding/base64. And auth-go deliberately uses zero x/oauth2 (only deps: keyring + x/sys); entiredb only has it as an indirect dep, so even entire-core's loopback didn't use it. You were right — the plan was gold-plated.

What entire-core "hand-rolled" actually maps to

So the only non-stdlib thing entire-core pulled in (go-jose) you already have a native replacement for.

Would golang.org/x/oauth2 help?

It would fold three pieces into a config object — and it's Go-team-maintained and battle-tested:

• PKCE: oauth2.GenerateVerifier(), oauth2.S256ChallengeOption(v), oauth2.VerifierOption(v)
• cfg.AuthCodeURL(state, ...) and cfg.Exchange(ctx, code, ...) + auto-refresh via TokenSource

But it can't give you the loopback server, the state value, or browser-open — those stay hand-written either way (~50 lines). And the catch: auth-go deliberately avoids x/oauth2 and hand-rolls device-flow/refresh/sts on its own oauthhttp helpers (HTML-response detection, description sanitization, size caps, loopback enforcement). Adding x/oauth2 just for authcode means two parallel token stacks (oauth2.Token vs auth-go's tokens.TokenSet) and bypasses that hardening.

Recommendation: don't add x/oauth2. The part you wanted to "port" (PKCE) is genuinely 8 lines of stdlib, and the exchange should reuse auth-go's existing oauthhttp decode/error helpers. That's both minimal and consistent with the package.

Right-sized plan

auth-go — one new file authcode/authcode.go (~150 lines incl. comments) + one test:

• Client struct (mirror deviceflow: BaseURL, ClientID, Scope, AuthorizePath, TokenPath, Transport, AllowInsecureHTTP).
• PKCE+state (stdlib), bind loopback, build authorize URL, callback handler, exchange via oauthhttp.ReadAndDecodeJSON/ReadOAuthError → return *tokens.TokenSet.
• Keep browser-open OUT of the lib (CLI owns it, like it does today).

entireio/cli — shim + flag:

• go.mod bump to the new auth-go tag.
• Add AuthorizePath to the provider table.
• Shim methods on auth.Client (parallel to the deviceflow shim).
• --device flag in login.go; default path calls the new flow, reusing the existing validateReceivedToken/SaveToken/RecordLoginContext tail.

Things I over-scoped — drop or defer: headless auto-fallback, RFC 9207 cross-region iss, extensive docs. Nice-to-haves, not load-bearing for a first cut.

Two things that stay non-negotiable:

1. Phase 0 backend check — confirm the authorize endpoint path on this CLI's auth host and that entire-cli is registered with a loopback redirect. One curl of /.well-known/openid-configuration.
2. scope=cli offline_access in the authorize request — entire-core omitted scope; without it you get no refresh token and silent refresh breaks.

Want me to do the Phase 0 curl now so we can lock the endpoint before writing the package?

Fair challenge — and your instinct ("lean on Go-team code, not our own dodgy stuff") is the right default. But it cuts differently here than it looks. Let me be honest about the actual tradeoff rather than defend auth-go's aesthetic.

The catch: for this flow, x/oauth2 replaces the safe part and misses the dodgy part

What x/oauth2 actually does for an authcode+loopback login:

• cfg.AuthCodeURL(state, pkceOpts...) — builds the URL (~10 lines saved)
• oauth2.GenerateVerifier() / S256ChallengeOption / VerifierOption — PKCE (~8 lines saved)
• cfg.Exchange(ctx, code, ...) — token POST + JSON parse (~30 lines saved)

What it does not do — you write these regardless:

• the loopback listener + callback handler
• the state value
• browser open

So x/oauth2 saves ~50 lines, and those 50 lines are a form POST + JSON decode + SHA256 — the least error-prone code in the whole thing. The genuinely fiddly, security-sensitive part is the HTTP response handling, and that's exactly what auth-go's oauthhttp already hardens and x/oauth2's Exchange does not:

So "lean on the library" here actually loses hardening on the part that matters and offloads the part that was never risky.

The real reason it's awkward: token-model mismatch

auth-go is a coherent hand-rolled stack — tokens.TokenSet + refresh (rotation, proclock single-flighting) + sts (RFC 8693, not in x/oauth2 core) + tokenmanager + keyring. x/oauth2's value is its oauth2.Token + TokenSource auto-refresh world. If you bolt x/oauth2 onto just the login step, you immediately convert oauth2.Token → TokenSet and throw away TokenSource (refresh already lives in auth-go's refresh/tokenmanager). You'd add a dependency and use ~5% of it — the worst of both worlds.

So the honest framing

There are two coherent positions, and "use x/oauth2 for the new flow only" is neither:

1. Stay hand-rolled (recommended for this task). authcode joins deviceflow/refresh/sts in the same style, reuses the same hardening, returns TokenSet. PKCE is 8 lines of stdlib — there's no dodgy-crypto risk to mitigate; that was the part you were worried about and it basically is stdlib.

2. Migrate auth-go wholesale to x/oauth2. Legitimate — device flow and PKCE are both in x/oauth2 now (added 2023–24), and it's less code to own long-term. But it's a separate, bigger project: you'd lose STS (RFC 8693 isn't in core) and the captive-portal/sanitization hardening, and it touches every consumer. Worth a real discussion, not a side-effect of adding login redirect.

My recommendation: option 1 for now. Not because hand-rolled is virtuous, but because the slice x/oauth2 helps with is trivial and safe, the slice it ignores is the risky one auth-go already handles, and piecemeal adoption fights the token model the rest of the package is built on. If you want to lean on x/oauth2, make it a deliberate auth-go-wide call — I can scope that separately.

Want me to (a) proceed with the hand-rolled authcode package, or (b) spec out the wholesale x/oauth2 migration so you can compare for real?

Waiting on lint. The auth-go package is written, builds clean, and passes all 12 tests under -race. I'll confirm the lint result when it finishes.

Done with auth-go. Here's the state.

Bottom line

• Committed to branch authcode-loopback-flow (commit dfb6f1d). Builds clean, all 12 tests pass under -race.
• Push fails at the GitHub-mirror leg (send-pack exited with error: exit status 1) — objects store to the Entire replicas, but the leg that reaches GitHub can't complete. That's the plane network, not the code.
• PR not created (needs the branch on GitHub first). Body is staged at /tmp/authcode-pr-body.md.

When you have signal, run:

What's in the commit

• authcode/authcode.go — Client + Flow (Start → Wait → Exchange), PKCE/state via stdlib crypto/rand+crypto/sha256, loopback 127.0.0.1:0 listener, reuses internal/oauthhttp for all HTTP hardening. Browser-open deliberately left to the caller.
• authcode/testseams.go — per-Client clock seam (mirrors deviceflow).
• authcode/authcode_test.go — 12 tests, -race clean.
• CHANGELOG.md / doc.go / README.md — added the package; removed the stale "PKCE is a non-goal" line.

The PR description leads with the UX rationale you asked for: device-flow-only was deliberate and stays right for headless/CI, but loopback is nicer on an interactive machine with a browser, and mature CLIs (gh/gcloud/aws sso) ship both with device-flow fallback.

Next

The CLI PR (entireio/cli) is the follow-up: bump the auth-go dep to this branch's tag once merged, add AuthorizePath to the provider table, shim authcode into auth.Client, and add the --device flag + default-to-browser branch in login.go. Want me to draft that against the worktree now (it'll need the same Phase-0 confirm of the authorize endpoint path), or hold until auth-go merges?

Committed (e12484c5bc) — the "no required module" warning is the expected stacked-PR state (dep bump is pending auth-go merge). Let me stage the PR body and attempt the push.

Done. The CLI branch now builds against the pushed auth-go branch.

Bottom line

• go get ...@authcode-loopback-flow pinned auth-go v0.4.1-0.20260604093244-dfb6f1d8eb12 (the authcode commit). CLI builds clean; unit + integration login tests pass.
• Branch login-loopback-browser-flow, 2 commits — not pushed (per your "I'll push" rule):
  • e12484c5bc login: default to loopback browser flow, add --device fallback
  • b32a500b58 go.mod: bump auth-go to the branch
• PR body staged at /tmp/cli-loopback-pr-body.md.

To push + open the CLI PR

Two pre-merge items (in the PR body)

• Re-pin go.mod to auth-go's released tag once #16 merges (currently a branch pseudo-version).
• Confirm the v2 AuthorizePath (/authorize) against the auth host's OIDC discovery doc, and that entire-cli is registered as a public client with a http://127.0.0.1/callback loopback redirect — the one backend fact I couldn't verify offline.

All verified against production and recorded.

Summary

• Both PRs are ready. The one backend unknown is now confirmed: us.auth.entire.io serves /authorize, supports S256 PKCE + authorization_code, and entire-cli is a registered public client with an any-port http://127.0.0.1/callback loopback redirect. No code changes needed.
• Verification method, for the record: discovery doc for the protocol facts; black-box /authorize probes with negative controls (bogus client_id → 400, unregistered redirect → 400, two loopback ports → both accepted) for the client registration that isn't publicly exposed.
• PR body (/tmp/cli-loopback-pr-body.md) updated with the verification; the only remaining pre-merge step is re-pinning go.mod to auth-go's released tag after #16 merges.

When you're ready, push the CLI branch and open the PR:

Sorted. The rebase is done and clean.

What was stuck

• One conflict, in cmd/entire/cli/auth/provider.go. While you were out, main (#1360) removed AuthTokensPath from the Provider struct, and your commit added AuthorizePath to the same v2 entry — overlapping edits → conflict.

Resolution

• Kept AuthorizePath, dropped AuthTokensPath (matching main's removal — verified zero references remain anywhere on disk). Also upgraded the comment to "Verified against us.auth.entire.io's discovery doc" since we'd confirmed it.
• The lingering AuthTokensPath LSP errors were stale gopls cache — the on-disk files are clean.

Result

• Rebased onto latest origin/main (e3104a66a6); branch is 2 ahead / 0 behind, linear.
• go build ./... clean; auth + login unit tests green.
• Commits: 02625b208a (login flow) + 23dd62def5 (go.mod bump).

Branch login-loopback-browser-flow is ready to push when you are. Note the go.mod is still pinned to the auth-go branch pseudo-version — re-pin to the released tag once auth-go #16 merges.

Done. The browser flow now mirrors --device.

Output, side by side

--device (unchanged):

default (browser) — now:

Same rhythm: Login URL: line → blank → Press Enter to open in browser... pause → Waiting for … with the trailing space so Login complete. finishes the line. Only difference is no Device code: line (there's no code in this flow) and "sign-in" vs "approval".

What changed

• runBrowserLogin: Login URL: + Enter pause (waitForEnter) + open + same-line Waiting for sign-in... ; browser-open failure still prints the copyable URL fallback.
• waitForEnter: short-circuits under interactive.UnderTest() (like openBrowser) so forcing interactive mode in tests doesn't block on a real /dev/tty.
• Tests updated: unit assertions for the new prompt; waitForBrowserPrompt now parses Login URL:. Unit + integration login tests green; gofmt clean.

Branch login-loopback-browser-flow now has 3 commits, ready to push when you are.

✓ Login complete. is already in persistLogin (applies to both flows). Now the integration test seam: since the happy path no longer prints the URL, the test needs another way to learn the ephemeral callback port. I'll have openBrowser, under test, write the URL to a file named by an env var.

API Error: Unable to connect to API (ConnectionRefused)

Done. No file-writing seam — the smell is gone.

What changed

• Output: browser flow now prints Logging in to: <auth host>, blank line, Press Enter to open in browser..., blank line, then Waiting for sign-in... ✓ Login complete. (the ✓ lives in the shared persistLogin, so the device flow gets it too).
• URL only on failure: the full authorize URL is printed only in the browser-open fallback — exactly your ask.
• No test seam smell: openBrowser returns an error under test instead of writing a file. That's the honest behavior (no usable browser on a headless/CI host), and it naturally drives the existing fallback so the integration test recovers the ephemeral callback URL from stdout. The reverted file/env-var seam is gone.

Tests

• Unit + integration login tests pass; gofmt clean; full build clean.

Branch state

login-loopback-browser-flow now has 4 commits:

• 02625b208a login flow + --device
• 23dd62def5 go.mod bump
• 634c618e70 align browser output with device flow
• f6e8e00ff1 show host instead of long URL

One judgment call to flag: the ✓ applies to both flows (it's in shared persistLogin) — device now reads Waiting for approval... ✓ Login complete. too. Say the word if you want device left without the checkmark. Otherwise, ready to push.

to: — and it's already what's in the code, so nothing to change.

"Log in to <service>" is the standard, grammatically correct phrasing — you authenticate to a host. "Log in on" implies a device/place ("log in on your phone"), which isn't what https://us.auth.entire.io is. So Logging in to: is both correct and smoother.

On the ✓: agreed, it's the only correct behavior — consistent success signal across both flows. I only flagged it because it technically touched the device output you'd already praised. It stays on both.

Nothing to do; branch is ready to push.

Pushed. Draft PR description is ready (in /tmp/cli-loopback-pr-body.md, shown above) — not created.

Bottom line:

• Branch login-loopback-browser-flow is on GitHub.
• PR description leads with user impact (the two commands + the output), then a short "how", the prod verification, and the one stacked-PR caveat (re-pin go.mod after auth-go#16 merges).

When you're ready to create it:

Want me to tweak the title or trim the description further?

Done.

Summary

• Rebased onto latest origin/main (advanced 11 commits) — clean, no conflicts. 4 ahead / 0 behind.
• Verified post-rebase: build clean, unit + integration login tests pass.
• Force-pushed (--force-with-lease) the rebased branch.
• Draft PR created: https://github.com/entireio/cli/pull/1366

Reminder on the one open item (it's in the PR body): re-pin go.mod to auth-go's released tag once auth-go#16 merges, before marking this ready.

Lint is clean and pushed.

Fixed

• errcheck (check-blank: true): the two runBrowserLogin tests now assert the stubbed-Wait error instead of blank-assigning it.
• ireturn: //nolint:ireturn on StartBrowserAuth (prod) and its test fake — the BrowserAuthFlow interface return is deliberate so login.go can substitute a fake; matches existing repo precedent.
• gomod: committed go mod tidy (dropped the stale old auth-go pin from go.sum left by the rebase).

mise run lint → 0 issues. Unit + integration login tests green. Pushed to draft PR #1366 (e410a403fe).

Lint clean with no nolint. Let me run the integration test to confirm the real binary path still works.

Done — pushed to PR #1366.

Changes

• ireturn eliminated, no nolint: auth.BrowserAuthFlow is now a concrete struct and StartBrowserAuth returns *BrowserAuthFlow — "return structs". runBrowserLogin accepts a cli-local browserAuthFlow interface ("accept interfaces"), with the start moved up into newLoginCmd. Mirrors how the device-flow shim already works.
• Output: one space after Logging in to:, and a single blank line before Waiting for sign-in... (the leading \n was doubling up with the post-Enter newline).

Final interactive output:

Lint 0 issues, build + unit tests green. (Integration left for CI, as you noted.)

Both Copilot comments evaluated and handled. Summary:

Verdict

1. Stale comment (openBrowser is a no-op under test) — ✅ still valid, fixed & pushed. The comment predated my refactor where openBrowser switched from no-op to returning an error (to drive the fallback). Updated it to describe the actual behavior. (5ea776af5f)

2. waitForBrowserPrompt deadline ineffective against blocking ReadString — ⚠️ technically valid, deliberately not fixed:

• It's the exact pattern the pre-existing device-flow helper (waitForLoginPrompt) already uses — fixing only mine creates asymmetry.
• go test's global timeout is the real CI backstop; the per-helper deadline is decorative, not load-bearing.
• It can't trigger in practice (the binary always prints the fallback line).
• Proper fix (async read + select) would touch both helpers — a separate cleanup, not this PR's concern.

Both comments were anchored to an old commit (1443f5d37), so worth noting the code has since moved — but I evaluated against the current tree, not the snapshot they reviewed.

Pushed to PR #1366. Want me to reply to the Copilot threads on GitHub with these dispositions (resolve #1, decline #2 with rationale)?