Skip to content

fix(oauth): stabilize macOS embedded login popups - #852

Merged
Neonforge98 merged 2 commits into
developfrom
codex/fix-oauth-popup-crash
Aug 20, 2026
Merged

fix(oauth): stabilize macOS embedded login popups#852
Neonforge98 merged 2 commits into
developfrom
codex/fix-oauth-popup-crash

Conversation

@Neonforge98

@Neonforge98 Neonforge98 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Problem

Closes #845.

On macOS, opening an embedded Claude Code or Codex OAuth surface could close and recreate the native webview in a loop. Commit 61a08e2a3 added document.visibilityState to the host visibility predicate; WKWebView can mark the parent document hidden while a native child webview owns the active login surface. The setup components then interpreted the closed webview as a signal to start a fresh login, repeatedly regenerating PKCE state and driving CPU usage to 75-95%.

Google sign-in also used WebviewWindowBuilder::build() synchronously inside the WebKit createNewPage callback on macOS. That reentrant window construction matches the reported SIGABRT path and could terminate ORG2 when Google requested a popup.

Solution

  • Treat an embedded webview host as hidden only when its container leaves layout, while retaining observer-driven close/reopen behavior for actual KeepAlive host changes.
  • Add a shared OAuth browser auto-start hook that permits one PKCE attempt per browser open, survives React StrictMode effect replay, and leaves retries as an explicit user action.
  • On macOS, hand Claude Google and Codex provider popups to Wry with NewWindowResponse::Allow. Wry reuses the caller WKWebView configuration, preserves window.opener, and avoids constructing a Tauri window from inside createNewPage.
  • Add regression coverage for parent-document visibility and one-attempt-per-open lifecycle behavior.

The resulting invariants are: parent document visibility alone cannot close an active login surface; a mounted browser surface cannot start an unbounded sequence of PKCE attempts; and macOS OAuth popup callbacks do not synchronously build another Tauri window.

Potential risks

  • macOS provider popups are now owned by Wry, so ORG2 no longer observes popup navigation or applies its host-managed delayed close logic on that platform. The real Claude Code Google flow, including authorization completion and credential persistence, succeeded; other providers with different popup behavior were not authenticated end to end.
  • The embedded OAuth webview remains allocated while the app is backgrounded if its host container is still mounted. This preserves the in-progress PKCE flow; measured idle CPU returned to 0-0.1% after closing, and RSS returned below its pre-cycle sample.
  • Linux host-managed popup behavior and existing Windows behavior are unchanged. No storage schema, serialized payload, RPC contract, or historical data migration changes are included, so rollback is a normal code revert.
  • Manual authorization created a persistent Claude Code OAuth credential only in isolated Instance 3. No credential values, account identifiers, or authorization codes are included in this PR or its logs.

Verification

Automated checks:

  • pnpm vitest run src/features/SessionSetup/hooks/useEmbeddedWebview.test.ts src/features/SessionSetup/hooks/useOAuthBrowserAutoStart.test.ts src/features/SessionSetup/hooks/__tests__/useOAuthCapture.test.ts — 22 passed.
  • cargo test --lib oauth::tests --no-fail-fast — 7 passed.
  • pnpm typecheck — passed.
  • ESLint on all changed TypeScript files — passed.
  • pnpm run check:e2e-oauth-guards — passed.
  • rustfmt --edition 2021 --check on both changed OAuth adapters — passed.
  • Commit hook cargo clippy -p org2 — passed.
  • pnpm run tauri:build:fast -- --instance 3 — produced the isolated macOS instance app.

Manual checks on macOS 26.2 (25C56), isolated Instance 3:

  • Claude login remained on one PKCE URL instead of looping; steady CPU was about 0.2% versus 75-95% on the released v1.2.6 build.
  • Opened and closed the real accounts.google.com Claude popup three times without SIGABRT or crash logs.
  • Opened and closed the full Claude OAuth surface three times; each open generated exactly one new PKCE attempt.
  • Google popup CPU was 1.5-2.9%; after closing all OAuth surfaces CPU returned to 0-0.1% and RSS settled at about 165.7 MiB.
  • Codex sign-in and its Google page also loaded without a crash.
  • Performed a clean rebuild of version 1.2.6 from commit 4b6e228ad; the resulting Instance 3 app rendered successfully from the isolated profile.
  • A user completed the real Claude Code Google OAuth flow. The isolated credential store updated with an enabled Claude Code OAuth record, no validation error, zero refresh failures, and no upstream error status; credential values were not inspected or printed.
  • The app remained alive after authorization. A post-login process sample showed about 1.5% CPU, and the recent macOS unified log contained no matching WebKit, SIGABRT, panic, abort, crash, or OAuth error event.

The full frontend and Rust test suites were not run; verification was scoped to the owning OAuth boundaries, full TypeScript checking, clippy, a complete macOS app build, and end-to-end Claude Code OAuth authorization.

Audit

  • Architecture audit: all 10 layers reviewed. The change is confined to control flow, lifecycle ownership, platform popup dispatch, and tests; domain shapes, public wire contracts, initialization, persistence, and serialization remain unchanged.
  • Performance guard: pass. The only timer is a bounded 100 ms one-shot that is cleaned up; observers disconnect on cleanup; three repeated popup and surface cycles showed no CPU loop or retained-memory staircase.
  • No screenshot is attached because there is no layout or styling change; the relevant evidence is native popup creation, process survival, stable PKCE state, successful authorization, and measured lifecycle behavior.

@Neonforge98

Copy link
Copy Markdown
Collaborator Author

The failing Rust job is a shared develop baseline issue, not an OAuth diff regression: git_api::commands has an ambiguous glob re-export for is_transient_error. The same failure appears on unrelated PRs. I opened #853 as a separate single-responsibility blocker fix; once it lands on develop, this PR can rerun the unchanged OAuth diff against the corrected base.

@Neonforge98
Neonforge98 merged commit c6bf7e4 into develop Aug 20, 2026
4 of 5 checks passed
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.

crash: SIGABRT in on_new_window handler when WebKit triggers OAuth/SSO popup (createNewPage)

1 participant