Skip to content

fix: separate Google and GitHub sign-in options in auth login - #49

Merged
justinhelmer merged 1 commit into
mainfrom
fix/login-provider-picker
Aug 20, 2026
Merged

fix: separate Google and GitHub sign-in options in auth login#49
justinhelmer merged 1 commit into
mainfrom
fix/login-provider-picker

Conversation

@justinhelmer

@justinhelmer justinhelmer commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

First of three PRs from the 2026-08-19 user-testing feedback on CLI sign-in: the picker's sign-in methods become separate options, one per provider.

Stack position: PR 1 of 3 from the 2026-08-19 user-testing feedback, based on main, independent of its siblings: cli#51 (direct-to-provider browser routing — the paired PR that gives this one its end-to-end effect) and cli#53 (terms/privacy notice at account creation). Merging in any order is safe.

Context

cli#45 made auth login the single sign-in (or sign-up) question, but collapsed Google and GitHub into one "GitHub or Google (browser)" entry. User testing on 2026-08-19 flagged that as strange: each method should be its own option, the way auth signup already presents them. This PR separates the picker; the paired sibling PR changes buildBrowserFlowUrls so a provider pick opens that provider's OAuth flow directly — until it lands, a provider pick still opens the OAuth consent URL exactly as the combined entry does today, so this PR alone changes labels and threads intent, not the browser destination.

What this PR does

  • Replaces the combined "GitHub or Google (browser)" picker entry in auth login with two first-class options, Google and GitHub, using the same labels and "opens your browser" hints as auth signup.
  • Threads the chosen provider into the existing BrowserFlowOptions.provider field via oauthLogin(config, true, { provider: method }).
  • Everything else is preserved: the "Sign in (or sign up) with" prompt, Email → the shared email signup/login flow, Device code, API key, the --api-key shortcut, and the --no-browser/non-interactive mapping to the device-code flow.

Verification

  • npm run typecheck — pass
  • npm run lint — pass
  • npm run test — 272/272 pass, 0 fail
  • No test drives the interactive picker today (existing tests bypass it via flags and nothing mocks promptSelect), so no new test was forced for the options list.

🤖 Generated with Claude Code

The combined 'GitHub or Google (browser)' picker entry becomes two first-class options, Google and GitHub, matching auth signup's labels. The chosen provider rides BrowserFlowOptions.provider into the browser OAuth flow.

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

@claude claude Bot 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.

LGTM: clean 1-file, +4/-3 split of the combined browser picker entry into first-class Google/GitHub options that exactly mirrors auth signup, with the provider threaded into the pre-existing BrowserFlowOptions.provider — no blocking issues found.

Independently verified at head da9acc10962cfffa4cf9ffc5cb797daacc43c0a6:

  • Diff: only src/commands/auth/login.ts (+4/-3). The split entries' values/labels/hints/order match auth signup's picker (signup.ts:356-357) exactly; Email / Device code / API key entries and the "Sign in (or sign up) with" prompt are untouched.
  • Merge-order safety: BrowserFlowOptions.provider pre-exists at base (oauth.ts:368) and is only read inside the options.signupEntry branch of buildBrowserFlowUrls (oauth.ts:399-402). Login never sets signupEntry, so this PR alone leaves the login browser URL unchanged — it just threads intent for the sibling PR (#51), and merging in any order is safe, as the PR body says.
  • No stragglers: zero remaining references to the removed 'browser' value or "GitHub or Google" label anywhere in src/ or test/; nothing persists the picked method, no telemetry keyed to it, and no test mocks promptSelect (the no-new-test rationale checks out).
  • CI: 3/3 check runs green at this exact SHA (Test & typecheck on Node 20.x/22.x/24.x).
  • Local checks re-run at head: npm run typecheck (with codegen), npm run lint, npm run test — all pass, 0 failures.

Three nits inline, all explicitly non-blocking: the dropped Recommended hint (login.ts:215-216, appears intentional), the pre-existing "opens your browser" hint under --no-browser (login.ts:215-218, routes to device flow at 234), and the stale pre-existing void flags; (login.ts:241-242).


Generated by Claude Code

Comment thread src/commands/auth/login.ts
Comment thread src/commands/auth/login.ts
Comment thread src/commands/auth/login.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Auto-approved: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.


Generated by Claude Code

@justinhelmer
justinhelmer merged commit 10feec2 into main Aug 20, 2026
7 checks passed
@justinhelmer
justinhelmer deleted the fix/login-provider-picker branch August 20, 2026 01:02
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.

1 participant