Skip to content

Fix browser UA compatibility without blurring Google Sheets - #9054

Merged
austinywang merged 9 commits into
mainfrom
issue-9052-browser-ua-google-workspace
Jul 28, 2026
Merged

austinywang merged 9 commits into
mainfrom
issue-9052-browser-ua-google-workspace

Conversation

@austinywang

@austinywang austinywang commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • replace the all-or-nothing WKWebView identity toggle with one URL-aware BrowserUserAgentPolicy
  • advertise a Safari-compatible identity for remote sites so Google Workspace and enterprise UA gates recognize the browser
  • keep native embedded WebKit identity for Google Sheets, preserving the sharp 2x canvas path proven by Fix blurred Google Sheets canvas rendering #8697
  • derive the advertised Safari version from the installed Safari bundle, but never advertise below current stable Safari 26.6 so stale local installs cannot fall outside Google’s two-version support window
  • apply the same policy to direct loads, redirects, history traversal, reloads, prewarming, and popup/nested-popup navigation

Reveal geometry is intentionally unchanged: #8697's controlled same-URL experiments disproved it as the Sheets blur cause, while the standalone Safari identity reproduced the 0.5 backing/CSS canvas ratio.

The configurable override requested in #7513 remains a complementary follow-up rather than the default fix: arbitrary sites get correct compatibility behavior without configuration, and a future override can sit above this pure policy.

Closes #9052

Testing

  • Safari support-floor regression pair: test-only 7ff6d68cfe failed on installed 26.4 remaining 26.4; fix 2dfc28ad92 raises it to 26.6 while preserving newer 27.x versions; red CI job: https://github.com/manaflow-ai/cmux/actions/runs/30390021738/job/90379876425; review cleanup c9e336824e removes the optional-collection lint warning; current-head CI: https://github.com/manaflow-ai/cmux/actions/runs/30392519058 (18 jobs green on attempt 2; the isolated rerun cleared one unrelated timing-test flake)
  • review follow-up 0f35c401e6: authorize the exact old navigation before cancellation, preserve automation across a deferred insecure-HTTP prompt, transfer on allow, and cancel on no-start/decline
  • review follow-up 4d9ebc7a60: centralize pane and popup replay behind one strict existing-main-frame path; nil/new-window and subframe targets remain untouched
  • added a red/green Swift Testing regression pair (f74bef3e38 test-only, 0478aed897 implementation)
  • covered Google Workspace and arbitrary enterprise SSO defaults, Google Docs/Slides, Google Sheets and legacy Sheets hosts, local documents, and runtime identity switching on a WKWebView
  • ./scripts/lint-pbxproj-test-wiring.sh
  • ./scripts/check-pbxproj.sh
  • python3 scripts/check-package-resolved-policy.py
  • xcrun swiftc -frontend -parse for all changed Swift files
  • xcrun swiftc -typecheck -warnings-as-errors Sources/Panels/BrowserUserAgentPolicy.swift
  • git diff --check
  • app build and tests intentionally deferred to CI per task constraint; no local xcodebuild was run
  • the repository no longer contains .github/swift-file-length-budget.tsv or scripts/swift_file_length_budget.py (removed upstream in Remove Swift file length budget #8125); no budget TSV was touched, the new files are below 500 lines, and BrowserPanel.swift shrank

Demo Video

Cloud-mac visual verification has not been run yet. The run and full-screen recording evidence for both the Workspace gate and Sheets canvas ratio remain pending before merge readiness is reported.

  • Video URL or attachment: pending

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally (intentionally prohibited for this task; CI + cloud Mac are the authorities)
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed (not needed; no user-facing strings or documented surface changed)
  • I requested bot reviews after my latest commit
  • All code review bot comments are resolved
  • All human review comments are resolved

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

Adds a URL-aware browser user agent policy that uses a Safari‑compatible UA for most sites while keeping WKWebView’s native UA for Google Sheets to prevent blur. Floors the advertised Safari version to current stable when local Safari is older, and restarts top‑level navigations on identity changes so sites see the right UA immediately. Closes #9052.

  • Bug Fixes

    • Added WKWebView helpers (applyBrowserUserAgentPolicy, browserUserAgentPolicyRestartRequest, restartNavigationForBrowserUserAgentPolicyIfNeeded) to swap UA, strip stale User-Agent, and replay main‑frame loads; used in panels, popups, nested popups, prewarmed views.
    • Applied the restart path to direct loads, redirects (including deferred insecure‑HTTP prompts), history traversal, reloads, and prewarming; subframes and new‑window targets are ignored.
    • Preserved automation during deferred UA replays with BrowserAutomationNavigationCoordinator.willReplaceNavigation and .didReplaceNavigation; policy cancellations for the replaced load are ignored until the replacement starts.
    • Derived the Safari‑compatible UA from the installed Safari with a compatibility floor; auxiliary URLSession requests now use BrowserUserAgentPolicy.system.safariCompatibleUserAgent.
    • Added unit and WebKit tests to verify UA restart behavior, reject stale Safari UA, and keep embedded identity for Google Sheets.
    • Isolated the default SharedLiveAgentIndex via a convenience overload to avoid cross‑actor default parameter use.
  • Refactors

    • Simplified Safari version parsing to avoid optionals and handle invalid components.

Written for commit c9e3368. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Improvements
    • Standardized Safari-compatible user-agent enforcement across main loads, prewarmed pages, reloads, and back/forward navigation.
    • Strengthened policy-based navigation restarts, including system-browser checkout and popup/new-window flows, with more reliable tracking of replaced navigations.
    • Preserved native embedded identity for Google Sheets and non-HTTP/local destinations where applicable.
  • New Features
    • Added a browser user-agent policy system to compute and apply the Safari-compatible value consistently.
  • Tests
    • Added/expanded coverage for user-agent selection, WebKit restart-request behavior, and navigation replacement outcomes (commit/cancel).

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The change adds a URL-sensitive Safari-compatible user-agent policy, applies it across browser and popup navigation surfaces, restarts navigations when the identity changes, and transfers automation transactions from replaced navigations to their replacements. It also adds an explicit shared-index workspace availability overload.

Changes

Browser user-agent policy

Layer / File(s) Summary
Policy contract and validation
Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift, Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift, Packages/macOS/CmuxBrowser/Tests/..., cmuxTests/BrowserUserAgentPolicyWebKitTests.swift, cmux.xcodeproj/project.pbxproj
Adds destination-specific Safari-compatible user-agent selection, WebKit policy application and restart requests, plus unit and WebKit coverage.
Browser and popup surface integration
Sources/Panels/BrowserPanel.swift, Sources/Panels/BrowserPopupWindowController.swift, Sources/Panels/BrowserPrewarmedWebViewPool.swift
Applies the policy before loads, history navigation, reloads, popup creation, prewarming, and auxiliary HTTP requests.
Navigation replacement flow
Sources/Panels/BrowserNavigationDelegate.swift, Sources/Panels/BrowserPopupWindowController.swift, Sources/Panels/BrowserPanel.swift
Cancels and restarts main-frame navigations when policy changes, including checkout and popup paths, and forwards replacement navigation identities.
Automation transaction replacement
Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift, Packages/macOS/CmuxBrowser/Tests/.../BrowserAutomationNavigationCoordinatorReplacementTests.swift
Tracks pending replacements, transfers active transactions to replacement navigations, suppresses terminal callbacks for replaced identities, and tests committed and cancelled outcomes.

Workspace availability API

Layer / File(s) Summary
Shared-index resolver overload
Sources/Workspace+ForkAgentConversationAvailability.swift
Adds a panel-only async overload using .shared and removes the default value from the explicit-index resolver.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BrowserNavigationDelegate
  participant WKWebView
  participant BrowserPanel
  participant BrowserAutomationNavigationCoordinator
  BrowserNavigationDelegate->>WKWebView: apply policy and create restart request
  BrowserNavigationDelegate->>WKWebView: cancel original navigation
  BrowserNavigationDelegate->>BrowserPanel: start replacement navigation
  BrowserPanel->>BrowserAutomationNavigationCoordinator: forward replaced and replacement IDs
  BrowserAutomationNavigationCoordinator->>BrowserAutomationNavigationCoordinator: transfer active transaction
Loading

Possibly related PRs


Important

Pre-merge checks failed

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

❌ Failed checks (2 errors, 2 warnings)

Check name Status Explanation Resolution
Cmux Swift Actor Isolation ❌ Error PR adds @MainActor UA helpers but calls them from nonisolated BrowserPanel.makeWebView and createWebViewWith delegates, introducing Swift 6 isolation violations. Mark those entry points @MainActor or hop through MainActor.run/assumeIsolated before calling applyBrowserUserAgentPolicy and related restart helpers.
Cmux No Ambient Global State ❌ Error BrowserUserAgentPolicy adds a new global singleton at line 12 (system), and app code reaches for it directly instead of injecting a policy instance. Remove the static singleton; construct BrowserUserAgentPolicy in BrowserPanel/popup owners and pass it into WKWebView helpers and request handlers.
Out of Scope Changes check ⚠️ Warning The Workspace conversation-availability overload appears unrelated to the UA fix and is out of scope for this PR. Move the Workspace overload change to a separate PR unless it is strictly required for this UA fix, and keep this PR focused on browser identity handling.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (21 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #9052 by using site-aware UA policy for Workspace while preserving native Google Sheets identity.
Cmux Swift Blocking Runtime ✅ Passed PASS: The diff adds UA-policy and state-machine code only; no new production semaphores, locks, sleeps, asyncAfter, main-sync, or polling were introduced. Existing Task.sleep/locks predate the diff.
Cmux Browser Automation Off-Main ✅ Passed Only BrowserUserAgentPolicy.swift changed; no browser.* socket routing, WebKit waits, or mainActor/processV2Command paths were modified.
Cmux Expensive Synchronous Load ✅ Passed No new main-actor/interactive RestorableAgentSessionIndex.load() was added; the changed workspace helper uses SharedLiveAgentIndex.shared, whose reload path runs via Task.detached off-main.
Cmux Cache Substitution Correctness ✅ Passed PASS: the UA-policy change is navigation-only; it doesn’t replace an authoritative persistence/history/snapshot read with a cache, and cold/stale Safari versions are handled.
Cmux No Hacky Sleeps ✅ Passed PR diff contains only Swift sources and pbxproj wiring; no changed JS/TS/shell/runtime files with sleeps, timers, polling, or waits.
Cmux Algorithmic Complexity ✅ Passed No new scalable collection scans or repeated sorts/filters; changes are O(1) state transitions plus tiny bounded URL/version parsing.
Cmux Swift Concurrency ✅ Passed The full diff only changes BrowserUserAgentPolicy.swift, and it adds no DispatchQueue, Combine, completion-handler, or fire-and-forget Task patterns.
Cmux Swift @Concurrent ✅ Passed No changed Swift async work needs @concurrent: the new async methods are @MainActor UI coordination or hop to Task.detached for heavy work.
Cmux Swift Package Boundaries ✅ Passed Reusable policy and navigation coordinator live in CmuxBrowser; app-target changes are WebKit/AppKit glue and composition, which the rule allows.
Cmux Swiftpm Lockfiles ✅ Passed PASS: PR diff only adds Swift source wiring in project.pbxproj; no Package.swift, .gitignore, or Package.resolved changes, and no XCRemoteSwiftPackageReference entries.
Cmux Swift Logging ✅ Passed No added or modified print/debugPrint/dump/NSLog/Logger usages appear in the PR diff; changed files only adjust UA policy and tests.
Cmux User-Facing Error Privacy ✅ Passed PASS: The PR adds UA policy logic and generic browser alerts only; no production user-facing error copy exposes vendor names, raw upstream messages, or secrets.
Cmux Full Internationalization ✅ Passed No new user-facing Swift/web text was added; the diff only introduces developer comments and protocol/config/user-agent tokens, with no localization surface touched.
Cmux Swiftui State Layout ✅ Passed PR diff adds UA-policy/replay logic only; no new SwiftUI views or changed-line GeometryReader/@Observable/@published layout patterns were introduced.
Cmux Architecture Rethink ✅ Passed PASS: the PR adds a pure BrowserUserAgentPolicy and coordinator-owned replacement state; the changed code adds no new timing/polling/observer side-channel hacks.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Popup panel already uses stable cmux.browser-popup and is in cmuxAuxiliaryWindowIdentifiers; this PR only changes UA policy.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/test/config files; no logs, screenshots, recordings, temp dirs, caches, or other artifact paths were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The only changed production file adds a public policy API and private helpers; no #if DEBUG/test-hook seam or testing-only accessor was added.
Title check ✅ Passed The title clearly summarizes the main change: URL-aware browser UA compatibility with Google Sheets preserved.
Description check ✅ Passed The description matches the template with Summary, Testing, Demo Video, Review Trigger, and Checklist sections, though the demo video is still pending.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9052-browser-ua-google-workspace

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Panels/BrowserNavigationDelegate.swift`:
- Around line 449-452: Extend the browser-user-agent lifecycle to handle server
redirects by applying the authoritative destination-based policy in
webView(_:didReceiveServerRedirectForProvisionalNavigation:) before continuing
or claiming a prewarmed view. Update
Sources/Panels/BrowserNavigationDelegate.swift:449-452 for main navigation,
Sources/Panels/BrowserPopupWindowController.swift:689-695 and 714-715 for popup
redirect/proceed paths, and
Sources/Panels/BrowserPrewarmedWebViewPool.swift:50-52 for prewarmed loads,
ensuring the final page URL is used consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 442dfbb9-8a25-4a98-a5ee-a750042213e7

📥 Commits

Reviewing files that changed from the base of the PR and between 877057f and 0478aed.

📒 Files selected for processing (7)
  • Sources/Panels/BrowserNavigationDelegate.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/BrowserPopupWindowController.swift
  • Sources/Panels/BrowserPrewarmedWebViewPool.swift
  • Sources/Panels/BrowserUserAgentPolicy.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/BrowserUserAgentPolicyTests.swift

Comment thread Sources/Panels/BrowserNavigationDelegate.swift Outdated
@cursor

cursor Bot commented Jul 28, 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

Copy link
Copy Markdown
Contributor Author

Review follow-up pushed as 11c6efee86.

Addressed findings:

  • Moved the reusable URL-to-UA policy and pure tests into Packages/macOS/CmuxBrowser; the app target now contains only WKWebView application/replay glue.
  • Covered old-WebKit 302 behavior documented by WebKit 313127@main: when a main-frame destination changes identity policy, cmux cancels and reissues the request after applying the new identity and stripping the stale explicit User-Agent header. The identity comparison prevents replay loops and leaves same-policy redirects untouched.
  • Preserved exact browser-automation lifecycle by transferring the active transaction from the cancelled WKNavigation identity to the replacement; the old cancellation is ignored and the replacement commit completes the same ticket.
  • Marked the shared WKWebView load helper @MainActor, resolving the CI compiler error without changing runtime behavior.
  • Added package coordinator coverage and app-host WebKit coverage for one-shot replay, header stripping, native Sheets identity restoration, and navigation-ID transfer.

Static validation passed: pbxproj normalization, 607-file test wiring audit, workspace package grouping, Package.resolved policy, Swift frontend parse for every changed Swift file, warnings-as-errors typecheck for the extracted policy, and git diff --check. No local xcodebuild or test command was run per the task constraint.

@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review follow-up pushed as 11c6efee86.

Addressed findings:

  • Moved the reusable URL-to-UA policy and pure tests into Packages/macOS/CmuxBrowser; the app target now contains only WKWebView application/replay glue.
    ...

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 4 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@austinywang

Copy link
Copy Markdown
Contributor Author

Current-head CI for 11c6efee86 has been dispatched: https://github.com/manaflow-ai/cmux/actions/runs/30356057686

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== commit and changed files ==='
git show --stat --oneline --decorate --find-renames 11c6efee86
git diff --name-status 11c6efee86^ 11c6efee86

echo '=== relevant source/test files ==='
fd -t f -e swift . | rg '(BrowserUserAgent|BrowserNavigationDelegate|BrowserPanel|BrowserPopup|WebView|Navigation|Transaction)'

echo '=== symbols and call sites ==='
rg -n -C 4 --glob '*.swift' \
  'applyBrowserUserAgentPolicy|BrowserUserAgentPolicy|replay|transfer.*[Tt]ransaction|[Tt]ransfer.*[Nn]avigation|did(Start|Commit|Fail).*Navigation|decidePolicyFor' \
  Sources Packages cmuxTests 2>/dev/null || true

echo '=== outlines ==='
for f in $(fd -t f -e swift . | rg '(BrowserUserAgent|BrowserNavigationDelegate|BrowserPanel|BrowserPopup|WebView|Navigation|Transaction)'); do
  echo "--- $f"
  ast-grep outline "$f" --items all 2>/dev/null || true
done

Length of output: 201375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== follow-up diff ==='
git diff --find-renames --unified=80 11c6efee86^ 11c6efee86 -- \
  Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift \
  Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift \
  Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift \
  Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift \
  Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift \
  Sources/Panels/BrowserNavigationDelegate.swift \
  Sources/Panels/BrowserPopupWindowController.swift \
  cmuxTests/BrowserUserAgentPolicyWebKitTests.swift

echo '=== app replay helper and delegates ==='
for f in \
  Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift \
  Sources/Panels/BrowserNavigationDelegate.swift \
  Sources/Panels/BrowserPopupWindowController.swift \
  cmuxTests/BrowserUserAgentPolicyWebKitTests.swift; do
  echo "--- $f"
  cat -n "$f"
done

echo '=== coordinator implementation and focused tests ==='
sed -n '1,360p' Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift
sed -n '1,460p' Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift

echo '=== all app call sites and transaction callback context ==='
rg -n -C 10 --glob '*.swift' \
  '\b(restartNavigationForUserAgentPolicyIfNeeded|didReplaceNavigation|didInterruptByPolicyChange|didFail\(|didCommit\(|didStart\(|applyBrowserUserAgentPolicy)\b' \
  Sources/Panels cmuxTests

Length of output: 213289


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== BrowserPanel delegate wiring ==='
sed -n '3720,3945p' Sources/Panels/BrowserPanel.swift

echo '=== BrowserPanel navigation dispatch ==='
sed -n '6000,6245p' Sources/Panels/BrowserPanel.swift
sed -n '6470,6630p' Sources/Panels/BrowserPanel.swift

echo '=== every shared load helper call site ==='
rg -n -C 8 --glob '*.swift' '\bbrowserLoadRequest\s*\(' Sources cmuxTests

echo '=== request-navigation callback assignments and active-navigation tracking ==='
rg -n -C 12 --glob '*.swift' \
  '(navigationDelegate\.requestNavigation|requestNavigation\s*=|activeMainFrameNavigation|didReplaceNavigationForUserAgentPolicy)' \
  Sources cmuxTests

echo '=== diffs for all touched files in this follow-up ==='
git diff --name-only 11c6efee86^ 11c6efee86 | while IFS= read -r f; do
  printf '\n--- %s ---\n' "$f"
  git diff --unified=12 11c6efee86^ 11c6efee86 -- "$f"
done

Length of output: 135752


@austinywang I found one automation-lifecycle gap in the replay flow:

restartNavigationForUserAgentPolicyIfNeeded cancels the original navigation at Line 481, then routes the replacement through BrowserPanel.requestNavigation. For an identity-changing redirect to an HTTP URL that requires the insecure-HTTP confirmation, that method defers the replacement until the user responds. In the meantime, WebKit reports cancellation for the old WKNavigation, and didCancelProvisionalNavigation completes the active automation ticket as .cancelled. When the user allows the prompt, didReplaceNavigation is too late because there is no active ticket left.

Please keep the original transaction pending across this deferred policy replay—e.g., transfer/mark the old navigation as an authorized replacement before issuing the cancellation, or suppress its cancellation until the replacement either starts or the prompt is declined. Add a regression test covering an identity-changing redirect that goes through the insecure-HTTP prompt.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Panels/BrowserNavigationDelegate.swift`:
- Around line 464-491: Extract the shared target-frame validation and UA-policy
restart-request derivation from restartNavigationForUserAgentPolicyIfNeeded in
Sources/Panels/BrowserNavigationDelegate.swift:464-491 into one reusable helper,
preserving the main delegate’s strict main-frame guard; keep only
didReplaceNavigationForUserAgentPolicy reporting local to this call site. Update
Sources/Panels/BrowserPopupWindowController.swift:729-748 to use that helper and
remove its duplicated restart logic, ensuring both surfaces share the corrected
targetFrame behavior.

In `@Sources/Panels/BrowserPopupWindowController.swift`:
- Around line 729-748: Update restartNavigationForUserAgentPolicyIfNeeded so it
only handles existing main-frame navigations: require
navigationAction.targetFrame to be non-nil and its isMainFrame value to be true
before applying UA-policy restarts. Allow nil targetFrame actions to return
without cancelling, preserving the createWebViewWith new-window path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 24932ae2-58b2-4dff-a4f7-7c45232e4b2e

📥 Commits

Reviewing files that changed from the base of the PR and between 0478aed and 11c6efe.

📒 Files selected for processing (10)
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift
  • Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift
  • Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift
  • Sources/Panels/BrowserNavigationDelegate.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/BrowserPopupWindowController.swift
  • Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/BrowserUserAgentPolicyWebKitTests.swift

Comment thread Sources/Panels/BrowserNavigationDelegate.swift
Comment thread Sources/Panels/BrowserPopupWindowController.swift
@austinywang

Copy link
Copy Markdown
Contributor Author

Addressed the deferred automation-lifecycle finding from #9054 (comment) in 0f35c401e6.

The exact old WKNavigation is now authorized for replacement before decisionHandler(.cancel). While an insecure-HTTP confirmation defers the replacement, only policy-interruption and cancellation callbacks for that exact old navigation are suppressed. Proceeding transfers the active ticket to the replacement navigation; cancellation, external open, or any other no-start result calls the completion with nil and finishes the original ticket as .cancelled.

The new regression suite covers immediate transfer, an identity-changing redirect whose replacement is deferred across the HTTP prompt while both old cancellation forms arrive, and prompt decline/no-start. The follow-up 4d9ebc7a60 also consolidates pane and popup replay through one strict main-frame-only path.

Static validation passed; no local xcodebuild or tests were run per the task constraint.

@coderabbitai review
@greptile-apps review

@coderabbitai coderabbitai 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.

♻️ Duplicate comments (1)
Sources/Panels/BrowserNavigationDelegate.swift (1)

465-493: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Duplicated UA-policy restart glue persists across surfaces.

restartNavigationForUserAgentPolicyIfNeeded still independently implements the cancel/strip-header/reissue logic that also exists in BrowserPopupWindowController.swift, unchanged from the prior review round. Extract the shared guard/restart-request derivation into a common helper so both surfaces stay in sync (only the automation-ticket reporting closures should remain call-site-specific).

As per coding guidelines, "Do not wire the same behavior separately through multiple surfaces; use one shared action path."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/Panels/BrowserNavigationDelegate.swift` around lines 465 - 493,
Refactor restartNavigationForUserAgentPolicyIfNeeded and the corresponding
BrowserPopupWindowController flow to use one shared helper for the main-frame
guard and browserUserAgentPolicyRestartRequest derivation, while preserving each
caller’s automation-ticket reporting closure. Keep cancellation and reissue
behavior in the shared action path so both surfaces remain synchronized.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@Sources/Panels/BrowserNavigationDelegate.swift`:
- Around line 465-493: Refactor restartNavigationForUserAgentPolicyIfNeeded and
the corresponding BrowserPopupWindowController flow to use one shared helper for
the main-frame guard and browserUserAgentPolicyRestartRequest derivation, while
preserving each caller’s automation-ticket reporting closure. Keep cancellation
and reissue behavior in the shared action path so both surfaces remain
synchronized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 63632cf3-c138-4566-a16c-a28d19505da6

📥 Commits

Reviewing files that changed from the base of the PR and between 11c6efe and 0f35c40.

📒 Files selected for processing (4)
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift
  • Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorReplacementTests.swift
  • Sources/Panels/BrowserNavigationDelegate.swift
  • Sources/Panels/BrowserPanel.swift

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== follow-up commits ==='
git show --stat --oneline --find-renames 0f35c401e6
git show --stat --oneline --find-renames 4d9ebc7a60
echo
echo '=== files changed across both follow-ups ==='
git diff --name-status 0f35c401e6^ 4d9ebc7a60

echo
echo '=== candidate source and test maps ==='
fd -t f -e swift . | rg '(BrowserNavigationDelegate|BrowserPopupWindowController|BrowserAutomationNavigationCoordinator|BrowserUserAgentPolicy|WKWebView|BrowserPanel|Navigation.*Test|UserAgent.*Test)' | while IFS= read -r f; do
  echo "--- $f"
  ast-grep outline "$f" --items all 2>/dev/null || true
done

echo
echo '=== focused diffs ==='
git diff --find-renames --unified=60 0f35c401e6^ 4d9ebc7a60 -- \
  Sources/Panels/BrowserNavigationDelegate.swift \
  Sources/Panels/BrowserPopupWindowController.swift \
  Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift \
  Sources/Panels/BrowserPanel.swift \
  Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift \
  Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift \
  cmuxTests/BrowserUserAgentPolicyWebKitTests.swift

echo
echo '=== replay authorization, completion, and delegate call sites ==='
rg -n -C 12 --glob '*.swift' \
  '\b(authoriz|replace|replacement|restartNavigationForUserAgentPolicyIfNeeded|didReplaceNavigation|didCancelProvisionalNavigation|didFailProvisionalNavigation|didInterrupt|insecure|decisionHandler\s*\(\.cancel|requestNavigation)\b' \
  Sources/Panels Packages/macOS/CmuxBrowser/Sources Packages/macOS/CmuxBrowser/Tests cmuxTests

Length of output: 202130


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== commit file lists ==='
git diff-tree --no-commit-id --name-status -r 0f35c401e6
git diff-tree --no-commit-id --name-status -r 4d9ebc7a60

echo
echo '=== exact diffs ==='
git diff --unified=35 0f35c401e6^ 0f35c401e6
git diff --unified=35 4d9ebc7a60^ 4d9ebc7a60

echo
echo '=== policy replay symbols only ==='
rg -n -C 18 --glob '*.swift' \
  'authorizeNavigationReplacement|completeNavigationReplacement|restartNavigationForUserAgentPolicyIfNeeded|didReplaceNavigationForUserAgentPolicy|didCancelProvisionalNavigation|didCancelNavigationPolicy' \
  Sources/Panels Packages/macOS/CmuxBrowser/Sources Packages/macOS/CmuxBrowser/Tests cmuxTests

echo
echo '=== focused test declarations ==='
rg -n -C 30 --glob '*.swift' \
  'deferred|insecure.*HTTP|replacement.*(cancel|transfer)|policy.*replacement|authorization' \
  Packages/macOS/CmuxBrowser/Tests cmuxTests

echo
echo '=== relevant portions of captured output, if present ==='
rg -n 'BrowserNavigationDelegate.swift|BrowserPopupWindowController.swift|BrowserAutomationNavigationCoordinator.swift|BrowserUserAgentPolicyWebKitTests.swift' \
  /tmp/coderabbit-shell-logs/shell-output-O7GlPV | head -n 120

Length of output: 201871


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== changed files ==='
git diff-tree --no-commit-id --name-only -r 0f35c401e6
git diff-tree --no-commit-id --name-only -r 4d9ebc7a60

echo
echo '=== BrowserNavigationDelegate: declarations, callbacks, and policy replay ==='
cat -n Sources/Panels/BrowserNavigationDelegate.swift | sed -n '1,180p;240,560p'

echo
echo '=== BrowserPopupWindowController: policy replay ==='
cat -n Sources/Panels/BrowserPopupWindowController.swift | sed -n '1,420p'

echo
echo '=== BrowserPanel: wiring of replacement lifecycle ==='
rg -n -C 16 --glob 'BrowserPanel.swift' \
  'willReplaceNavigationForUserAgentPolicy|didReplaceNavigationForUserAgentPolicy|requestNavigation\s*=' \
  Sources/Panels/BrowserPanel.swift

echo
echo '=== BrowserAutomationNavigationCoordinator: replacement state ==='
cat -n Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationNavigationCoordinator.swift | sed -n '1,420p'

echo
echo '=== focused test methods and bodies ==='
rg -n --glob 'BrowserAutomationNavigationCoordinatorTests.swift' '^\s*`@Test`|^\s*func ' \
  Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift
cat -n Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorTests.swift | tail -n 260
rg -n --glob 'BrowserUserAgentPolicyWebKitTests.swift' '^\s*`@Test`|^\s*func ' \
  cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
cat -n cmuxTests/BrowserUserAgentPolicyWebKitTests.swift

Length of output: 91451


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== dedicated replacement coordinator tests ==='
cat -n Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationNavigationCoordinatorReplacementTests.swift

echo
echo '=== BrowserPanel request-navigation implementation and insecure prompt completion ==='
rg -n --glob 'BrowserPanel.swift' \
  'func requestNavigation|func presentInsecureHTTPAlert|onNavigationStarted|BrowserInsecureHTTPNavigationIntent' \
  Sources/Panels/BrowserPanel.swift
cat -n Sources/Panels/BrowserPanel.swift | sed -n '6000,6320p'

echo
echo '=== shared replay helper ==='
cat -n Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift

echo
echo '=== popup navigation delegate and main-frame replay path ==='
rg -n -C 16 --glob 'BrowserPopupWindowController.swift' \
  'final class PopupNavigationDelegate|restartNavigationForUserAgentPolicyIfNeeded|targetFrame.*isMainFrame|requestNavigation\(' \
  Sources/Panels/BrowserPopupWindowController.swift

Length of output: 33433


@austinywang Verified. The follow-up addresses the deferred replay lifecycle gap:

  • The exact old navigation is authorized before decisionHandler(.cancel).
  • Only cancellation and policy-interruption callbacks for that authorized navigation are suppressed while the HTTP confirmation is pending.
  • Proceeding transfers the active automation ticket to the replacement navigation; every no-start path reports nil and resolves the ticket as .cancelled.
  • The dedicated regression tests cover immediate transfer, both old-navigation cancellation forms during deferred HTTP confirmation, and prompt decline.
  • The shared replay helper now limits replay to explicit main-frame navigations, preserving the popup/subframe boundary.

I did not run runtime tests locally; this review is based on targeted source and test inspection. No further issues found.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@austinywang

Copy link
Copy Markdown
Contributor Author

Current-head CI for 4d9ebc7a60 has been dispatched: https://github.com/manaflow-ai/cmux/actions/runs/30358005774

@austinywang

Copy link
Copy Markdown
Contributor Author

CI repair pushed as 7920780e18.

The 4d9ebc7a60 build completed successfully, but the zero-warning gate exposed an unrelated Swift 6 diagnostic at Workspace+ForkAgentConversationAvailability.swift:94: a main-actor .shared value was used as a default argument, whose evaluation is nonisolated. The fix follows the overload pattern already used in that file: the main-actor no-injection overload resolves .shared, while the existing injected overload remains available to tests. No warning-budget TSV was changed.

Fresh current-head CI: https://github.com/manaflow-ai/cmux/actions/runs/30360052501

@cursor

cursor Bot commented Jul 28, 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

Copy link
Copy Markdown
Contributor Author

Safari support-floor follow-up:

Regression pair:

Current-head CI: https://github.com/manaflow-ai/cmux/actions/runs/30391946301

@austinywang

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 4 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR head and recent commits ==='
gh pr view 9054 --repo manaflow-ai/cmux --json headRefOid,commits,statusCheckRollup \
  --jq '{headRefOid, commits: [.commits[] | {oid, messageHeadline}], statusCheckRollup: [.statusCheckRollup[] | {name, status, conclusion}]}'

echo '=== follow-up commit details ==='
git show --stat --oneline --find-renames 7ff6d68cfe
git show --stat --oneline --find-renames 2dfc28ad92

echo '=== current-head changes after prior accepted replay follow-up ==='
git diff --name-status 4d9ebc7a60..HEAD
git diff --find-renames --unified=80 4d9ebc7a60..HEAD -- \
  Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift \
  Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift \
  Sources/Workspace+ForkAgentConversationAvailability.swift

echo '=== policy and test structure ==='
ast-grep outline Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift --items all
ast-grep outline Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift --items all

echo '=== focused source and tests ==='
cat -n Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift
cat -n Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift

echo '=== policy construction and identity consumers ==='
rg -n -C 6 --glob '*.swift' \
  '\bBrowserUserAgentPolicy\b|safariCompatibleUserAgent|customUserAgent\s*\(' \
  Packages Sources cmuxTests

Length of output: 48917


@austinywang Reviewed current head (2dfc28ad92): no actionable issues found.

The compatibility floor correctly uses numeric component comparison, raises stale/invalid candidates to Safari 26.6, preserves newer versions, and retains the Sheets embedded-WebKit exception. The Swift 6 actor-isolation repair also preserves the convenience API while keeping the injectable overload for tests.

I did not run local builds or tests; this is based on focused source and regression-test inspection.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift`:
- Around line 27-29: Update versionComponents(from:) to return [Int] instead of
[Int]?, using an empty array for invalid input. In the candidate-version logic,
replace optional binding with an isEmpty check, assigning non-empty parsed
components and preserving the existing fallback behavior for invalid Safari
versions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 075a4065-e94c-4b63-a98b-c6df8caf113f

📥 Commits

Reviewing files that changed from the base of the PR and between 4d9ebc7 and 2dfc28a.

📒 Files selected for processing (3)
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift
  • Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift
  • Sources/Workspace+ForkAgentConversationAvailability.swift

Comment thread Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift Outdated
@austinywang

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@austinywang

Copy link
Copy Markdown
Contributor Author

Current-head CI is green for c9e336824e: https://github.com/manaflow-ai/cmux/actions/runs/30392519058

All 18 jobs passed on attempt 2, including the Safari support-floor regression and universal Release build. Attempt 1 had one unrelated timing failure in UserDefaultsSettingsStoreNotificationTests; the isolated package-job rerun passed without a code change. Review threads remain fully resolved.

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.

Nightly regression: Google Workspace rejects browser panes ('This browser version is no longer supported') after #8697 UA change

1 participant