Skip to content

feat: browser login for Grok Build provider (#7013)#7735

Open
diegosouzapw wants to merge 1 commit into
release/v3.8.49from
feat/7013-grok-build-browser-login
Open

feat: browser login for Grok Build provider (#7013)#7735
diegosouzapw wants to merge 1 commit into
release/v3.8.49from
feat/7013-grok-build-browser-login

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Summary

Grok Build (grok-cli) previously only supported a paste-token / auth.json import flow (flowType: "import_token"). This adds a one-click browser PKCE login, reusing the same auth.x.ai authorize/token endpoints and public grok_id client id already used by the sibling xai-oauth provider (PR #7399), while keeping the existing paste-token path intact as a fallback for headless/remote installs.

  • GROK_BUILD_OAUTH_CONFIG added to src/lib/oauth/constants/oauth.ts, modeled on XAI_OAUTH_CONFIG — same auth.x.ai endpoints + public client id (via the existing resolvePublicCred("grok_id", ...), no new embedded credential — Hard Rule chore(ui): rebrand to OmniRoute #11), Grok-Build-scoped scope, and its own loopback port 56122 (distinct from xai-oauth's 56121) so both flows can run concurrently.
  • New src/lib/oauth/providers/grok-cli-oauth.ts module: buildGrokBuildAuthUrl, exchangeGrokBuildToken, and mapGrokBuildBrowserTokens (mirrors xai-oauth.ts, reuses its decodeXaiIdTokenIdentity helper for id_token metadata).
  • src/lib/oauth/providers/grok-cli.ts: the single grokCli registry entry now carries both flows. flowType flips to authorization_code_pkce (so GET authorize/exchange/poll-callback dispatch correctly), and mapTokens auto-detects the input shape — a browser PKCE exchange response (access_token/refresh_token/...) vs. a pasted-token import ({ accessToken: <JWT or auth.json> }) — so the import-token route action (which calls mapTokens directly, bypassing the flow-type-aware helpers in providers.ts) keeps working unmodified. I investigated whether the registry needed an action-keyed lookup (per the plan's open question) and found no existing precedent for that in providers.ts/providers/index.ts — every entry is a flat single-flow object — so a unified mapTokens dispatch under one "grok-cli" key is the pattern-consistent choice.
  • src/app/api/oauth/[provider]/[action]/route.ts: added "grok-cli" to PKCE_CALLBACK_PROVIDERS. IMPORT_TOKEN_PROVIDERS (paste-token) is untouched — both actions stay available.
  • src/shared/components/OAuthModal.tsx: removed "grok-cli" from IMPORT_TOKEN_ONLY_PROVIDERS (un-hides the Browser Login tab — windsurf/devin-cli untouched, their Phase-2 fix is tracked separately), added it to PKCE_CALLBACK_SERVER_PROVIDERS, and added the redirectUri branch (http://127.0.0.1:56122/callback, merged into the existing xai-oauth branch to stay within the file-size ratchet).
  • src/shared/constants/providers/oauth.ts: updated grok-cli's authHint copy to mention both flows (no new i18n keys needed — same generic tab copy as xai-oauth/codex). Did not regenerate docs/reference/PROVIDER_REFERENCE.md — running the generator pulled in a large amount of unrelated pre-existing provider-catalog drift (265→268 total, several renamed/added providers unrelated to this change); regenerating that file is out of scope here and left for a dedicated doc-sync pass.
  • Gap-fix (Hard Rule chore(ui): rebrand to OmniRoute #11 test coverage): tests/unit/publicCreds.test.ts had no shape assertion for the grok_id embedded default despite it being live in production (GROK_CLI_CONFIG/XAI_OAUTH_CONFIG/now GROK_BUILD_OAUTH_CONFIG). Added one, following the existing gemini_id/windsurf_fb pattern (UUID-shape regex, no literal embedded).

How validated

TDD — new/updated tests written before/alongside the implementation, all passing:

  • tests/unit/oauth-grok-cli-browser.test.ts (new, 12 tests): buildAuthUrl shape, exchangeToken request/response handling + non-OK-response error (no stack leak), unified mapTokens dispatch (browser-shape vs paste-shape, plus the fix(grok-cli): parse expires_at from auth.json and exp from JWT to fix auto-refresh #5775 expiresIn clamp duplicated on the browser path), loopback-port collision guard (56122 vs 56121 vs codex's 1455), and route-level regression checks: GET /authorize reaches the PKCE branch (not the disabled-import_token branch), POST /exchange requires codeVerifier, a failed exchange returns a sanitized 500 (Hard Rule fix(ui): fix Select dropdown dark theme inconsistency #12 — asserted the upstream error body text does NOT leak into the response), and POST /import-token still works (no regression).
  • tests/unit/oauth-modal-grok-cli-browser-login-7013.test.ts (new): source-level regression guard that grok-cli left IMPORT_TOKEN_ONLY_PROVIDERS, windsurf/devin-cli stayed, and grok-cli joined PKCE_CALLBACK_SERVER_PROVIDERS.
  • tests/unit/grok-cli-oauth.test.ts (updated, line-neutral for the one behavioral change): flipped the flowType assertion from import_token to authorization_code_pkce; every paste-token mapTokens test (JWT, auth.json, wrapped shapes, expiry clamps) still passes unmodified, proving the unified dispatch preserves the paste path.
  • tests/unit/oauth-providers-config.test.ts (updated, 2 lines): the registry-conformance test's EXPECTED_CONFIG_BY_PROVIDER["grok-cli"] now points at GROK_BUILD_OAUTH_CONFIG (this test asserts provider.config identity, which legitimately changes with this feature) + added a REQUIRED_FIELDS_BY_PROVIDER["grok-cli"] entry mirroring xai-oauth's.
  • tests/unit/publicCreds.test.ts (gap-fix, +1 test): grok_id shape assertion.

Red→green: ran every new/touched test file individually before and after each implementation step; all 119 tests across the touched OAuth suites pass (oauth-grok-cli-browser, oauth-modal-grok-cli-browser-login-7013, grok-cli-oauth, grok-cli-oauth-test-supported-7610, oauth-providers-config, publicCreds, oauth-keychain-import-only-6041, windsurf-devin-executors, oauth-device-code-error-transparency, xai-oauth-provider). Full test:vitest:ui suite (927 tests, 173 files) also green.

Gates run and green: check-test-discovery.mjs, typecheck:core, typecheck:noimplicit:core (2 pre-existing unrelated errors in combo.ts/usageTracking.ts/cliRuntime.ts, none in touched files), eslint (touched files only), check:complexity-ratchets (both cyclomatic and cognitive within baseline), check:file-size.mjs (OAuthModal.tsx merged the xai-oauth/grok-cli redirect branches to stay within its frozen 998-line baseline instead of growing it), check:cycles, check:docs-sync, check:docs-counts (no regression — provider/OAuth counts unchanged by this PR). check-fabricated-docs.mjs --strict shows the pre-existing 3 unrelated stale claims (tracked in #7728), none of them mine.

What was deferred

  • Live browser round-trip against auth.x.ai cannot be TDD'd — this is the untestable half per Hard Rule fix(ci): add environment for npm token access #18 item 2.
  • docs/reference/PROVIDER_REFERENCE.md regeneration — deferred to avoid pulling in unrelated pre-existing provider-catalog drift into this PR (see note above).

live_check (required before merge — Hard Rule #18 item 2)

Deploy this branch to the VPS (root@192.168.0.15), then:

  1. /deploy-vps-local (or the project's standard branch deploy) onto feat/7013-grok-build-browser-login.
  2. In the dashboard, open the Grok Build (grok-cli) connection card → "Sign in with browser" tab → complete the auth.x.ai login.
  3. Confirm a new provider_connections row is created with accessToken/refreshToken/expiresAt populated (not the paste-token path).
  4. curl -s https://<vps-host>/v1/chat/completions -H "Authorization: Bearer <omniroute-api-key>" -H "Content-Type: application/json" -d '{"model":"grok-build","messages":[{"role":"user","content":"ping"}]}' — confirm a normal completion through cli-chat-proxy.grok.com (proves the inference path is unaffected by the login-flow change).
  5. Regression: confirm the paste-token tab still accepts a manually pasted JWT/auth.json for an existing or new connection.

Refs #7013 (browser login shipped and unit-tested; issue stays open pending the VPS live_check above per Hard Rule #18 — closing here would be premature since the PKCE round-trip itself is not machine-verified in this PR).

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown
Contributor

Thanks for working on a smoother Grok Build login experience. I think the PKCE-specific work in this PR would be safer to consolidate into #7358 rather than merge the two PRs independently.

#7358 already replaces the legacy grok-cli integration with the public grok-build client contract: official device-code OAuth with import fallback, principal-aware token handling and refresh-token rotation, Grok Build session headers, Responses API routing, authenticated model discovery, and the corresponding dashboard flow and tests.

Comparing the patches, five core files overlap:

  • src/app/api/oauth/[provider]/[action]/route.ts
  • src/lib/oauth/constants/oauth.ts
  • src/lib/oauth/providers/grok-cli.ts
  • src/shared/components/OAuthModal.tsx
  • tests/unit/grok-cli-oauth.test.ts

More importantly, the implementations currently select mutually exclusive registry modes: #7358 uses flowType: "device_code", while this PR uses flowType: "authorization_code_pkce". The provider registry currently exposes one flat flow per provider, so merging these independently would not compose the two login methods; whichever lands last would replace the other auth path, with direct conflicts in the provider, modal, and tests.

At the same time, this PR is not a substitute for #7358's executor/model/discovery work, and #7358 does not contain this PR's PKCE UX. My suggestion is therefore to move the unique PKCE pieces from this PR into #7358 (or explicitly rebase this PR on #7358), then make one deliberate decision about auth ownership in that combined context. If both device-code and PKCE should remain available, the provider abstraction should first be extended to support multiple login flows instead of silently replacing the official device flow.

That would preserve the broader grok-build alignment and its existing validation while avoiding two parallel, conflicting implementations of Grok Build authentication.

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