Skip to content

Fix infinite UA-policy restart loop on Google Sheets destinations - #9483

Merged
austinywang merged 1 commit into
manaflow-ai:mainfrom
lintanghui:fix/issue-9462
Aug 4, 2026
Merged

austinywang merged 1 commit into
manaflow-ai:mainfrom
lintanghui:fix/issue-9462

Conversation

@lintanghui

@lintanghui lintanghui commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

On some macOS versions WebKit reports the native identity as "" rather than nil after a restart clears customUserAgent. The policy check customUserAgent != resolvedUserAgent then never converges for destinations resolved to webKitDefault (like docs.google.com), so every policy pass cancels the navigation and reloads — the Sheets pane hangs and the main thread spins at ~100% CPU.

  • Treat an empty reported identity the same as nil when comparing against the resolved policy, so webKitDefault destinations no longer register a mismatch.
  • As a backstop, remember the last main-frame URL we restarted for and allow at most one restart in a row per destination; if the replacement load still reports a mismatch, let it proceed instead of looping.
  • Reworked restartNavigationForBrowserUserAgentPolicyIfNeeded to take the request and main-frame flag directly so the loop guard is testable without constructing a WKNavigationAction.

Added tests covering the empty-identity case and the restart-at-most-once behavior.

Fixes #9462


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes an infinite user‑agent policy restart loop on Google Sheets (docs.google.com) that froze the pane and pegged CPU on some macOS versions. Treats empty identities correctly and limits restarts so navigation proceeds. Fixes #9462.

  • Bug Fixes

    • Treat empty customUserAgent as nil when comparing against webKitDefault.
    • Allow at most one UA-policy restart per main-frame destination; if the replacement still mismatches, proceed.
    • Added tests for the empty-identity case and the single-restart behavior.
  • Refactors

    • Updated restartNavigationForBrowserUserAgentPolicyIfNeeded to take request and targetFrameIsMainFrame directly for easier testing and clearer logic.
    • Track the last restarted URL via an associated object to implement the loop guard.

Written for commit 1f93491. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a17998e-5fd0-47c6-be03-b6c7b163b787

📥 Commits

Reviewing files that changed from the base of the PR and between a2d28ba and 1f93491.

📒 Files selected for processing (4)
  • Sources/Panels/BrowserNavigationDelegate.swift
  • Sources/Panels/BrowserPopupWindowController.swift
  • Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
  • cmuxTests/BrowserUserAgentPolicyWebKitTests.swift

📝 Walkthrough

Walkthrough

The browser user-agent policy now treats an empty custom user agent as the WebKit default and prevents repeated restarts for the same navigation destination. Navigation delegates pass requests and main-frame information directly to the policy. Tests cover Sheets navigation and later restart eligibility.

Changes

Browser user-agent policy

Layer / File(s) Summary
User-agent normalization and restart state
Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
Empty customUserAgent values resolve as the WebKit default. The web view stores the destination of the latest policy restart.
Guarded navigation restart flow
Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift, Sources/Panels/BrowserNavigationDelegate.swift, Sources/Panels/BrowserPopupWindowController.swift, cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
Restart handling accepts a URLRequest and main-frame flag, skips repeated restarts for the same URL, and allows a later navigation to restart again. Callers and tests use the updated flow.

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

Possibly related PRs

Suggested reviewers: azooz2003-bit


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Architecture Rethink ❌ Error The new associated-object URL cache is a mutable side channel for navigation state already owned by the navigation delegates, so the mismatch remains representable and only the Sheets symptom is ca... Move the restart-attempt invariant into one shared navigation coordinator or WebView-owned model, pass it through both delegates, and clear it on explicit navigation completion or cancellation.
Cmux No Ambient Global State ❌ Error The diff adds private var cmuxBrowserUserAgentPolicyRestartedURLKey: UInt8 = 0 at file scope (line 5), a new top-level mutable variable prohibited by the rule. Move restart tracking and its association-key mechanism into a scoped, constructable BrowserUserAgentPolicyState owned by the browser delegate/web view seam and inject that state; remove the file-scope mutable variable.
Description check ⚠️ Warning The description explains the cause and fix but omits the required demo video, review trigger, checklist, and manual testing details. Add the missing template sections, document manual verification, include a demo video or explain its omission, and complete the checklist.
✅ Passed checks (22 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: fixing the infinite user-agent policy restart loop affecting Google Sheets.
Linked Issues check ✅ Passed The changes directly address issue #9462 by preventing repeated Sheets navigation restarts and adding tests for the reported failure modes.
Out of Scope Changes check ✅ Passed All changed files and tests support the user-agent policy restart-loop fix described in issue #9462.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed Production changes remain in @MainActor WKWebView methods and @MainActor navigation delegates; no new models, service protocols, Sendable reference types, or background UI-store access were introdu...
Cmux Swift Blocking Runtime ✅ Passed The production diff adds no semaphore, wait, sleep, delayed dispatch, polling, main-queue sync, or manual lock; it only adds synchronous URL state tracking. Tests are deterministic.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only main-actor WebKit user-agent navigation and tests; it adds no browser socket command, wait, worker route, or change to the policy/router target files.
Cmux Expensive Synchronous Load ✅ Passed The production diff only changes WebKit user-agent policy and URL bookkeeping; it adds no agent-history, transcript, JSON/JSONL, directory, or synchronous file loads on interactive main-actor paths.
Cmux Cache Substitution Correctness ✅ Passed The diff does not replace a persistence/history/undo/snapshot read. The associated URL is transient WKWebView loop-guard state; policy resolution still reads each request and resets on a different...
Cmux No Hacky Sleeps ✅ Passed The diff changes only Swift production code and Swift tests; the rule explicitly scopes TypeScript, JavaScript, shell, and non-Swift runtime scripts.
Cmux Algorithmic Complexity ✅ Passed The production diff adds only constant-time URL equality, optional-string normalization, and associated-object access; it introduces no scalable collection scan, nested loop, sort, filter, or batch...
Cmux Swift Concurrency ✅ Passed The diff adds no Dispatch, Combine, fire-and-forget Task, or new completion-handler async pattern; it only refactors existing synchronous closures at the WebKit decision-handler boundary.
Cmux Swift @Concurrent ✅ Passed The diff adds no async, nonisolated, or @concurrent work. Changed policy methods are synchronous @MainActor functions, so the Swift concurrency rule is not violated.
Cmux Swift Package Boundaries ✅ Passed The diff keeps pure BrowserUserAgentPolicy in CmuxBrowser; changes are WKWebView/Objective-C state and navigation-delegate glue in Sources/Panels, which the rule allows.
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only four Swift source/test files; no Package.swift, Package.resolved, .gitignore, workflow, Xcode project, or dependency files changed.
Cmux Swift Logging ✅ Passed The patch adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data diagnostics; existing NSLog calls are unchanged.
Cmux User-Facing Error Privacy ✅ Passed The production diff changes navigation policy logic only; added vendor/issue references are developer comments, and the new user-agent cases are tests. No user-facing error or recovery text was add...
Cmux Full Internationalization ✅ Passed The diff adds UA policy logic, comments, and tests only; it adds no user-facing production text and changes no localization catalogs, locale files, or web messages.
Cmux Swiftui State Layout ✅ Passed The diff contains no SwiftUI views or state/layout patterns; it changes AppKit/WebKit navigation policy code and tests only.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes UA navigation logic and tests only; it adds no window code. Existing browser popup uses cmux.browser-popup, is registered, and the auxiliary-window lint passes.
Cmux Source Artifacts ✅ Passed All four changed paths are intentional Swift source or test files; the diff adds no logs, caches, scratch directories, build output, screenshots, or other local artifacts.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Production additions contain no test/debug guard or test-shaped member; the refactored restart method has callers in both production delegates, and restart state remains private.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@austinywang
austinywang merged commit f4cd277 into manaflow-ai:main Aug 4, 2026
6 checks passed
austinywang pushed a commit that referenced this pull request Aug 4, 2026
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.
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)

2 participants