Skip to content

Use current Safari identity for Google Sheets - #9536

Merged
austinywang merged 14 commits into
mainfrom
issue-9462-google-sheets-browser-pane-cpu-hang
Aug 4, 2026
Merged

austinywang merged 14 commits into
mainfrom
issue-9462-google-sheets-browser-pane-cpu-hang

Conversation

@austinywang

@austinywang austinywang commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #9462

Root cause

#9054 gave Google Sheets a unique .webKitDefault identity to preserve an older sharp-canvas behavior. The #9462 CPU loop happened because WebKit can expose that native identity as either nil or an empty string. Raw comparison treated those equivalent values as different, so every replacement navigation synchronously requested another replacement before first paint.

#9482 correctly made that comparison semantic and idempotent. #9483 added separate same-URL retry state that could allow a mismatched identity to proceed. This PR removes the #9483-only state and keeps the nil/empty normalization.

Once navigation could finish, a second regression became visible: the embedded WebKit UA omits the Version/... Safari/... tokens. Google Sheets therefore displayed its unsupported-browser banner even though the document loaded. Sheets was also the only HTTP(S) destination that could switch away from the current Safari-compatible identity.

Fix

  • use the current Safari-compatible identity for every HTTP(S) destination, including all Google Sheets URL variants
  • remove the .webKitDefault web resolution entirely, so a remote navigation cannot downgrade into the unsupported embedded UA
  • preserve the Safari 26.6 compatibility floor while retaining any newer installed Safari version
  • retain semantic nil/empty normalization when non-web destinations clear a custom identity

Every web URL now resolves to one target identity. The first real identity change can restart once; every repeated evaluation sees the already-applied target and becomes a no-op. Redirect chains cannot toggle between Sheets and non-Sheets identities.

Regression coverage

  • policy tests require docs.google.com/spreadsheets, sheets.google.com, and spreadsheets.google.com to resolve to the current Safari-compatible identity
  • WebKit tests require native, empty, and stale Sheets identities to converge to current Safari and require a second evaluation to return no replacement
  • the final regression pair is fe19c02230 followed by c7374d1715
  • Swift frontend parsing passed for all changed Swift files
  • ./scripts/lint-pbxproj-test-wiring.sh
  • ./scripts/check-pbxproj.sh
  • python3 scripts/check-package-resolved-policy.py
  • git diff --check

Runtime validation

Tagged app: issue-9462-google-sheets-browser-pane-cpu-hang

Tested the reported Sheet in the embedded browser on the pushed head:

  • navigator.userAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/26.6 Safari/605.1.15
  • document.readyState: complete
  • unsupported-browser text present: false
  • visible canvas: 2614 x 2616 backing pixels for 1307 x 1308 CSS pixels, exactly 2x on both axes
  • main-process CPU settled to 0.4% instead of the reported 100% spin

Build succeeded on the pushed head with:

CMUX_SKIP_ZIG_BUILD=1 /Users/austinwang/manaflow/cmuxterm-hq/scripts/reload-cloud.sh --builder fleet --pool-tag tag:macbuilder --tag issue-9462-google-sheets-browser-pane-cpu-hang --launch

No user-facing strings changed. Localization files were audited and are unaffected.

austinywang and others added 7 commits August 3, 2026 16:42
# Conflicts:
#	Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
Restore the original navigation-action API and remove the regression test that encoded the same-URL stale-identity fallback. Keep PR #9482 nil/empty normalization as the sole convergence rule.
# Conflicts:
#	cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR removes the webKitDefault case from BrowserUserAgentPolicyResolution and its handling logic. All HTTP/HTTPS destinations, including Google Sheets, now receive a custom Safari-compatible user agent. Navigation restart call sites now pass the full WKNavigationAction. Tests are updated to match.

Changes

Unified Safari-compatible user agent policy

Layer / File(s) Summary
Policy resolution and enum contract
Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift, Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicyResolution.swift, Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift
Removes the webKitDefault enum case and Sheets host/path detection; resolution now always returns .custom(safariCompatibleUserAgent) for HTTP/HTTPS URLs. Tests expect the custom identity instead of the WebKit default.
Navigation restart wiring
Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift, Sources/Panels/BrowserNavigationDelegate.swift, Sources/Panels/BrowserPopupWindowController.swift
Removes the .webKitDefault switch branch. Restart method now accepts a WKNavigationAction and derives request and main-frame status internally; call sites updated accordingly.
Test coverage updates
cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
Adds tests for stale custom identity replacement on Sheets and rejection of empty custom identities. Removes tests treating empty user agents as embedded Sheets identities and the stale-identity regression test.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BrowserNavigationDelegate
  participant BrowserPopupWindowController
  participant WKWebViewExtension as WKWebView+BrowserUserAgentPolicy
  BrowserNavigationDelegate->>WKWebViewExtension: restartNavigationForBrowserUserAgentPolicyIfNeeded(navigationAction)
  BrowserPopupWindowController->>WKWebViewExtension: restartNavigationForBrowserUserAgentPolicyIfNeeded(navigationAction)
  WKWebViewExtension->>WKWebViewExtension: derive request and isMainFrame from navigationAction
  WKWebViewExtension->>WKWebViewExtension: apply Safari-compatible custom user agent
Loading

Possibly related issues

  • manaflow-ai/cmux-dev-artifacts#8164: Modifies and tests the same WebKit browser user-agent policy behavior, including custom identity handling and navigation restarts.
  • manaflow-ai/cmux-dev-artifacts#8174: Addresses the same test failure by removing webKitDefault and applying a unified custom Safari-compatible user agent strategy.
  • manaflow-ai/cmux-dev-artifacts#8291: Removes WebKit default identity handling and applies custom user agents to navigations, matching the same test file updates.
  • manaflow-ai/cmux-dev-artifacts#8036: Updates BrowserUserAgentPolicyWebKitTests.swift with the same new test expectations for Sheets and stale identity scenarios.
  • manaflow-ai/cmux-dev-artifacts#8072: Modifies the same test suite, BrowserUserAgentPolicyWebKitTests.swift.

Possibly related PRs

  • manaflow-ai/cmux#8697: Modifies Google Sheets/WKWebView user-agent handling, including customUserAgent identity behavior and related regression tests.
  • manaflow-ai/cmux#9054: Introduced the browser user-agent policy and WKWebView navigation-restart code that this PR refines, changing Sheets from native WebKit identity to Safari-compatible custom identity.
  • manaflow-ai/cmux#9483: Modifies the same files (BrowserNavigationDelegate.swift, BrowserPopupWindowController.swift, WKWebView+BrowserUserAgentPolicy.swift, tests) with an opposite approach that preserves webKitDefault.
🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided, so the required summary, testing details, demo information, review trigger, and checklist are missing. Add a description that completes the repository template, including the change summary, testing performed, demo video information, review trigger, and checklist status.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (23 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed All production Swift changes maintain proper Swift 6 actor isolation. Sendable value types are correctly marked, MainActor-constrained methods remain properly isolated, and no shared mutable refere...
Cmux Swift Blocking Runtime ✅ Passed No blocking or timing-based synchronization patterns (semaphores, blocking waits, sleeps, locks, polling) introduced. All methods use @MainActor for thread safety via structured concurrency.
Cmux Browser Automation Off-Main ✅ Passed This PR modifies internal navigation policy helpers (@MainActor extension functions and navigation delegate callbacks), not browser socket automation commands. The rule applies to browser.* socket...
Cmux Expensive Synchronous Load ✅ Passed The production Swift diff only changes user-agent policy and navigation restart plumbing; it adds no agent-history loader, filesystem scan, transcript/JSON parsing, or synchronous load.
Cmux Cache Substitution Correctness ✅ Passed The diff only changes transient WebKit navigation user-agent resolution and restart handling; it does not replace an authoritative read in persistence, history, undo, or snapshot code.
Cmux No Hacky Sleeps ✅ Passed The PR contains only Swift file changes. The check applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. Swift timing is covered by the separate "cmux Swift blocking runtim...
Cmux Algorithmic Complexity ✅ Passed The production diff removes URL host/path classification and adds no scalable collection scan; the changed policy and restart plumbing use constant-size control flow. Existing version loops are unc...
Cmux Swift Concurrency ✅ Passed Changes remove .webKitDefault case and simplify browser user agent policy. WebKit callback parameters are required API boundaries. No DispatchQueue, Combine, fire-and-forget Tasks, or new complet...
Cmux Swift @Concurrent ✅ Passed All modified Swift functions are synchronous and properly marked with @MainActor. No async/await functions without @concurrent annotations, no @concurrent on synchronous functions, and no heavy asy...
Cmux Swift Package Boundaries ✅ Passed BrowserUserAgentPolicy and its resolution enum remain in the CmuxBrowser SwiftPM target with package tests; app changes are limited to WKWebView/WebKit integration glue.
Cmux Swiftpm Lockfiles ✅ Passed This PR makes no changes to Package.resolved, .gitignore, Package.swift, Xcode projects, or workflows; it only modifies Swift source and test files, so the SwiftPM lockfile policy check does not ap...
Cmux Swift Logging ✅ Passed The PR diff changes only user-agent policy, navigation calls, and tests; it adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data logs.
Cmux User-Facing Error Privacy ✅ Passed All changes are internal (enum cases, function logic, test code, comments); no user-facing error messages, alerts, or localized strings were added or modified.
Cmux Full Internationalization ✅ Passed The diff changes user-agent policy/control flow and tests only; it adds no user-facing text and changes no string catalogs, locale files, or web messages.
Cmux Swiftui State Layout ✅ Passed PASS: The PR changes only browser user-agent policy, AppKit/WebKit delegates, and tests; added lines contain no SwiftUI state, layout, lazy-row store, or render-time mutation patterns.
Cmux Architecture Rethink ✅ Passed PR removes architectural anti-patterns (side-channel associated objects, restart-once-per-URL workaround) from #9483. Keeps only small local empty-string normalization with clear invariant and just...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only browser user-agent policy and navigation call signatures; it adds no standalone window or shortcut ownership code, and the auxiliary-window lint passes for 35 identifiers.
Cmux Source Artifacts ✅ Passed All seven changed paths are tracked Swift source or test files. The diff adds no files, directories, binaries, logs, caches, or build artifacts.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR removes complexity from browser user-agent policy code without introducing test/debug seams in production source files. No #if DEBUG blocks, test-named members, or visibility-widened accesso...
Cmux No Ambient Global State ✅ Passed The production diff adds no top-level functions, mutable state, namespace types, or singleton. It only changes existing policy logic and call arguments; existing system is unchanged.
Title check ✅ Passed The title clearly describes the primary user-agent change for Google Sheets.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9462-google-sheets-browser-pane-cpu-hang

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@austinywang austinywang changed the title Fully revert #9483 browser identity fallback Use current Safari identity for Google Sheets Aug 4, 2026
@austinywang
austinywang merged commit e48438c into main Aug 4, 2026
6 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.

Google Sheets embedded browser pane hangs forever, pegs cmux main thread at ~100% CPU (regression after #8697/#9054)

1 participant