Repository navigation
Fix wedged browser automation recovery - #8094
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBrowser automation now tracks WebView readiness, probes callback liveness, recovers stalled surfaces, distinguishes JavaScript and screenshot timeouts, preserves automation scripts across replacements, and applies expanded CLI request timeouts. ChangesBrowser automation recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR adds recovery for wedged browser automation. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (17): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/BrowserAutomationSnapshotResult.swift`:
- Around line 3-7: Update the BrowserAutomationSnapshotResult enum declaration
to explicitly conform to Sendable, preserving its existing cases and payload
types.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 7115-7116: Guard the pending preparation callbacks with didFinish
before starting work: in Sources/Panels/BrowserPanel.swift lines 7115-7116,
check didFinish before invoking operation(captureWebView, false, finish);
likewise in Sources/Panels/BrowserScreenshotSnapshotter.swift lines 340-341,
check didFinish before invoking operation(finish). Preserve the existing timeout
completion behavior while preventing callbacks from launching captures after
completion.
In `@Sources/TerminalController`+BrowserAutomationRecovery.swift:
- Around line 57-80: The recovery Task created inside socketAwaitCallback must
be tied to the socket wait’s cancellation lifecycle so it cannot continue after
the 2.5-second timeout. Update the surrounding recovery operation to retain and
cancel the Task when socketAwaitCallback times out, or use a structured worker
whose cancellation propagates to the watchdog, while preserving the existing
finish(result) behavior.
- Around line 34-49: Update V2JavaScriptResult to expose a typed
JavaScript-timeout outcome, then change v2RecoverTimedOutBrowserJavaScript to
pattern-match that structured case instead of comparing message text. Preserve
the existing recovery return for timeout results and return all other result
cases unchanged.
🪄 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
Run ID: c2e73bd9-9d9b-43ca-b5a7-00fb9b2ad51d
📒 Files selected for processing (14)
CLI/cmux.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationRecoveryOutcome.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationWatchdog.swiftPackages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationWatchdogTests.swiftResources/Localizable.xcstringsSources/Panels/BrowserAutomationProbeChannel.swiftSources/Panels/BrowserAutomationSnapshotResult.swiftSources/Panels/BrowserPanel+AutomationRecovery.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserScreenshotPipeline.swiftSources/Panels/BrowserScreenshotSnapshotter.swiftSources/TerminalController+BrowserAutomationRecovery.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxproj
…ehydration-hang # Conflicts: # cmux.xcodeproj/project.pbxproj
Contributor
Author
|
Review follow-up for the earlier CodeRabbit pre-merge summary on the watchdog timing shape:
The concrete typed-timeout, late-callback, callback-channel, state-preservation, and task-lifecycle findings are all fixed on current HEAD cf69bd1. |
There was a problem hiding this comment.
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
`@Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationWatchdog.swift`:
- Around line 48-72: Add a focused watchdog test covering two different
observedInstanceID values racing through recoverIfUnresponsive. Block the first
performRecovery/recover call, start the second instance, assert the old
instance’s followers resolve with .superseded and verify the old leader’s
recover invocation behavior, then release the race and assert both calls
complete with the expected outcomes. Anchor the test to recoverIfUnresponsive
and the existing watchdog test utilities.
In `@Sources/TerminalController`+BrowserAutomationRecovery.swift:
- Around line 44-50: Update the .timedOut branch in
v2BrowserAutomationMessageAfterLivenessCheck to construct the JavaScript-timeout
message with String(localized:defaultValue:) rather than a bare literal,
matching the sibling messages. Add the corresponding localization key and
translations for all 20 supported locales in Localizable.xcstrings.
🪄 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
Run ID: 1e838b70-36b4-48a3-b096-8e74acac01a9
📒 Files selected for processing (14)
CLI/cmux.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationProbeSignal.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationWatchdog.swiftPackages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationWatchdogTests.swiftResources/Localizable.xcstringsSources/Panels/BrowserAutomationProbeChannel.swiftSources/Panels/BrowserAutomationSnapshotResult.swiftSources/Panels/BrowserJavaScriptEvaluationResult.swiftSources/Panels/BrowserPanel+AutomationRecovery.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserScreenshotSnapshotter.swiftSources/TerminalController+BrowserAutomationRecovery.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- Resources/Localizable.xcstrings
👮 Files not reviewed due to content moderation or server errors (2)
- Sources/TerminalController.swift
- CLI/cmux.swift
There was a problem hiding this comment.
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/Tests/CmuxBrowserTests/Control/BrowserAutomationWatchdogTests.swift`:
- Around line 247-258: Update the test around firstFollower and the subsequent
recovery attempt to await an explicit signal that firstFollower has joined the
in-flight recovery, rather than using Task.yield() or scheduler timing. Add or
reuse a synchronization mechanism exposed by the watchdog/coalescing flow, and
only start the superseding recovery after that signal so the later wait cannot
hang.
🪄 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
Run ID: df8b038d-240a-4279-b134-536966afc802
📒 Files selected for processing (3)
Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationWatchdogTests.swiftResources/Localizable.xcstringsSources/TerminalController+BrowserAutomationRecovery.swift
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
There was a problem hiding this comment.
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
`@Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationDocumentReadiness.swift`:
- Around line 20-23: Remove the production-only init(onWaiterRegistered:) seam
from BrowserAutomationDocumentReadiness and update cancellationReleasesWaiter to
observe registration through `@testable` import by widening the necessary internal
state or behavior from private. If a callback remains unavoidable, move it into
a dedicated test-support extension/file rather than the main production type.
In
`@Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationDocumentReadinessOutcome.swift`:
- Around line 2-11: Mark the pure value enum
BrowserAutomationDocumentReadinessOutcome as nonisolated while preserving its
existing Sendable and Equatable conformances and cases, so it can be consumed
from both actor-isolated and nonisolated code.
🪄 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
Run ID: d9b1220e-d52f-4a9b-8c87-ada4b88a8d08
📒 Files selected for processing (8)
CLI/cmux.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationDocumentReadiness.swiftPackages/macOS/CmuxBrowser/Sources/CmuxBrowser/Control/BrowserAutomationDocumentReadinessOutcome.swiftPackages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/Control/BrowserAutomationDocumentReadinessTests.swiftResources/Localizable.xcstringsSources/Panels/BrowserPanel+AutomationRecovery.swiftSources/Panels/BrowserPanel.swiftSources/TerminalController.swift
…ehydration-hang # Conflicts: # Sources/Panels/BrowserPanel.swift
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
3 of 6 tasks
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jul 16, 2026
* test: cover browser automation watchdog recovery * fix: recover unresponsive browser automation * fix: cover all browser automation callback channels * refactor: isolate browser watchdog signal * fix: preserve browser automation recovery state * test: cover shared browser recovery * fix: share browser automation recovery * test: cover superseded browser recovery * fix: preserve page-world automation state * fix: await recovered browser document readiness * test: preserve browser consent during recovery * fix: preserve consent across browser recovery * fix: preserve interactive prompts during browser recovery * test: cover browser recovery lifecycle races * fix: guard browser automation recovery lifecycle * test: remove browser readiness registration hook * fix: preserve browser recovery timeout headroom * test: cover browser omnibar draft isolation * fix: isolate browser omnibar drafts by panel * ci: gate browser panel identity regression (cherry picked from commit f008967)
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@MainActorbrowser-automation watchdog that probes the same WebKit callback channel after an operation deadline and only replaces the exact unresponsiveWKWebViewinstanceBrowserPanel's existing state-preserving WebView replacement path so URL, navigation history, profile/data store, zoom, render state, and developer-tools intent remain under one lifecycle ownercmux browser screenshotenough socket response headroom to return the recovery diagnosis instead of timing out firstRoot cause
cmux recovered a WebKit process only when
webViewWebContentProcessDidTerminatefired. WebKit can instead stop completing JavaScript/snapshot callbacks without delivering that termination signal, leaving automation requests to hit their socket deadlines forever while the surface remains installed. The screenshot command's 15-second client deadline also raced the app's 15-second capture deadline, which is why the reporter saw exit 124 before cmux could classify the failure.Tests
2d86a5409f) adds the deterministic failing watchdog regression tests087d82f2b8) implements the watchdog and recovery patharch -arm64 swift testinPackages/macOS/CmuxBrowser: 144 tests across 22 suites passedThe exact multi-hour WebKit wedge is nondeterministic and is not simulated. Coverage deterministically verifies that a live callback preserves the current process, a missing callback recovers exactly once, and a WebView superseded during probing is not replaced again.
Localization
The three new user-facing recovery/timeout messages are present and non-empty in every locale currently in
Localizable.xcstrings(20 locales, including English and Japanese).Closes #8054
Issue: #8054
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Recover wedged browser automation by probing WebKit liveness and replacing only the stuck
WKWebView. Fixes #8054 with guards so we don’t resurrect closed panels or interrupt active visual capture; recovery is clear, pending prompts stay intact, and omnibar drafts are scoped per panel.@MainActorwatchdog that races JS and screenshot liveness against a deadline with outcomes: responsive/recovered/superseded/cancelled; concurrent probes share one result.WKUserScripts, keep page‑world hooks stable, and wait for the first document commit before resuming.automationTimedOut; Terminal reports recovery; CLI headroom is 30s post‑action and 20s otherwise.Written for commit b39ad9d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests