Skip to content

mux: non-blocking browser CDP I/O + real headful Chrome default - #7609

Merged
lawrencecchen merged 6 commits into
mainfrom
feat-mux-browser-improvements
Jul 12, 2026
Merged

lawrencecchen merged 6 commits into
mainfrom
feat-mux-browser-improvements

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Two browser-pane improvements on top of the CDP base already on main.

Anti-lag: browser CDP I/O can no longer freeze the terminal

Before: the server processed each connection's commands serially and browser-* handlers did their CDP call inline (up to the call timeout), so one wedged/slow Chrome dammed the socket that also carries terminal input for attach clients. Omnibar/nav actions blocked the TUI event loop the same way.

Now, three invariants:

  • The TUI event loop performs no blocking CDP I/O: nav/back/forward/reload/activate go through the off-loop dispatcher.
  • The server connection loop performs no CDP I/O: each browser surface owns a per-surface worker thread with a bounded queue; browser-* handlers and the resize CDP reconfigure enqueue and ack immediately (accepted, not completed; failures surface via browser-state/status events).
  • Failures localize: two consecutive CDP timeouts mark only that surface Failed("browser is not responding"), once per stall episode, re-armed when a fresh frame clears it. Other panes stay live.

Worker machinery: per-kind latest-wins slots (resize vs nav, no cross-kind clobber), no mutex held across a blocking call, worker reaped on kill and on the pane-missing adoption path, wedged-pane close is fire-and-forget (close_target_detached). Regression test drives a Chrome wedged after bootstrap and asserts a second navigate + a resize + list-workspaces all ack under 500ms on the same socket, plus a non-blocking close.

Real headful Chrome by default

browser.mode defaults to headful, launching the user's real Google Chrome in a visible window with a persistent profile, so signed-in sites treat the pane as a human. --disable-blink-features=AutomationControlled plus a launched-only best-effort UA de-headless (Browser.getVersion cached once, HeadlessChrome→Chrome, setUserAgentOverride before Page.enable, never fails a surface, external browsers untouched) clear the two loudest bot signals. browser.mode: "headless" opts back into a hidden window. Docs cover the Chrome 136 / SingletonLock real-profile caveats and agent-browser ws:// attach (no wss overclaim).

Verification

Built and tested on the AWS M4 Pro builder (macOS 15.7.4) because this dev Mac's macOS 26 SDK can't link ghostty-vt-sys: cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace all green; cargo test -p mux-core --test-threads=64 clean ×3 (does not reintroduce the PTY-exhaustion CI failure just fixed on main). smoke-tui.py passes; smoke-attach.py hits the known themed-prompt false positive that also fails on plain main (CI is its gate).

16 files, all under mux/. No new dependencies.

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Large concurrency and lifecycle changes across browser, mux socket, and attach paths; behavior is heavily regression-tested but wedged-Chrome and queue semantics affect all browser users.

Overview
Browser panes no longer run blocking CDP on the TUI event loop or the mux control socket. Each surface gets a worker thread with a bounded queue: pointer/key input may drop under load, URL nav and resize use latest-wins slots, and back/forward/reload/activate stay FIFO with explicit ok:false when the queue is full. Handlers ack immediately (ok:true means accepted); failures show up via status events and per-surface “browser is not responding” after two CDP timeouts, with recovery when frames return (including attach client title/state broadcast).

Headful Chrome is the default (browser.mode: "headful"): visible window, session-scoped persistent profile, --disable-blink-features=AutomationControlled, and a one-time launched-runtime UA tweak (HeadlessChrome → Chrome). headless opts back in. CDP gains Browser.getVersion, set_user_agent, and fire-and-forget close_target_detached.

The TUI routes omnibar and browser shortcuts through the off-loop browser input dispatcher, surfacing “browser is busy” when the outer queue is full. Docs/config/protocol are updated for mode, queue semantics, browser.discover default false, profile/Chrome 136 caveats, and browser attach streaming over protocol v6.

Reviewed by Cursor Bugbot for commit 20c4fba. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Moves all blocking browser CDP work off the UI/socket path and makes headful Chrome with a persistent profile the default, improving responsiveness and realism. Attach now streams browser panes, and protocol/browser commands return immediate acks with clearer backpressure and recovery.

  • New Features

    • Each browser surface runs CDP on a worker thread with a bounded queue; server and TUI enqueue and ack immediately (ok:true = accepted, not completed). Discrete controls (back/forward/reload/activate) use FIFO and return ok:false when the queue is full; high‑rate pointer/key input may drop to keep the UI non‑blocking.
    • URL navigation uses a latest‑wins slot; resize uses its own latest‑wins slot; wedged‑pane close is fire‑and‑forget via close_target_detached.
    • Headful Chrome is now the default via browser.mode: "headful" with a visible window and persistent profile; launched runtimes add --disable-blink-features=AutomationControlled and a one‑time UA de‑headless (Browser.getVersion, HeadlessChrome → Chrome; external browsers untouched). browser.mode: "headless" opts back in.
    • Protocol adds browser-* input and control commands that enqueue work and return immediate acks; attach-surface streams both PTY and browser panes. Docs cover session‑scoped profiles, Chrome 136 profile lock caveats, browser.discover defaulting to false, and ws:///http:// CDP endpoints (no wss://).
  • Bug Fixes

    • Localized stall handling: two consecutive CDP timeouts mark only that surface failed with “browser is not responding”; a fresh frame clears it, and recovery state/title is broadcast to attached clients.
    • Backpressure is explicit: the TUI surfaces outer‑queue drops (“browser is busy; command dropped”) and worker‑level failures via status events.

Written for commit 20c4fba. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added configurable browser panes browser.mode (headful/headless) with session-scoped profiles and improved Chrome/CDP attachment guidance.
    • Browser navigation/control (navigate/back/forward/reload/activate) now routes through an off-loop queued dispatcher with explicit backpressure handling.
  • Bug Fixes
    • Improved reliability under stalled/blocked CDP calls: non-responsive reporting is throttled and recovers cleanly; shutdown and failed attachments now terminate promptly.
  • Documentation
    • Updated browser pane, configuration, and protocol docs with the new mode, queue semantics, and browser-state/frame streaming details.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@vercel

vercel Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jul 12, 2026 3:50am
cmux-staging Building Building Preview, Comment Jul 12, 2026 3:50am

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds BrowserMode wiring across launch, config, and docs, moves browser surfaces to a bounded worker queue with timeout recovery, routes TUI browser actions through queued commands, and updates mux attachment behavior and protocol/documentation text.

Changes

Browser mode and worker-queue refactor

Layer / File(s) Summary
Chrome launch and CDP contract
mux/crates/mux-cdp/src/chrome.rs, mux/crates/mux-cdp/src/client.rs, mux/crates/mux-cdp/src/lib.rs
BrowserMode drives Chrome argv selection; the CDP client adds browser version lookup, detached target close, and user-agent override support.
Browser mode config wiring
mux/crates/mux-core/src/surface.rs, mux/crates/mux-core/src/lib.rs, mux/crates/mux-tui/src/config.rs
BrowserMode is re-exported, added to SurfaceOptions, parsed from TUI config, and applied to browser surface options.
Per-surface browser worker
mux/crates/mux-core/src/browser.rs
Browser surfaces move to bounded command queues with latest-wins navigation and resize slots, worker-thread execution, stealth user-agent handling, timeout recovery, and queue-driven input/control commands.
Mux attach and worker shutdown
mux/crates/mux-core/src/mux.rs
Browser surface construction now passes mux ownership, and failed pane attachment kills the surface and terminates the worker.
Socket-level queue regression tests
mux/crates/mux-core/tests/browser_runtime.rs
Runtime tests cover accepted-but-wedged navigation, ordered queued back/forward delivery, and queue-full backpressure reporting.
TUI browser command routing
mux/crates/mux-tui/src/app.rs, mux/crates/mux-tui/src/browser_input.rs
TUI browser actions enqueue commands through the dispatcher, and the dispatcher distinguishes disposable input from control commands with status feedback.
Browser docs and protocol
mux/README.md, mux/docs/browser-panes.md, mux/docs/configuration.md, mux/docs/protocol.md
Docs describe browser mode, session-scoped profiles, queue-based browser behavior, endpoint restrictions, and new browser protocol commands.

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

Possibly related PRs

  • manaflow-ai/cmux#7378: Touches the same browser-pane and TUI browser-control areas that this PR extends with queued command handling.

Important

Pre-merge checks failed

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

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux User-Facing Error Privacy ❌ Error browser_input.rs now emits browser command failed: {err} to the UI, and the underlying remote request path bails with raw response errors. Replace interpolated error text with a generic user-facing message and keep the underlying err only in internal logs/telemetry.
Description check ⚠️ Warning The description covers the changes and verification, but it omits required template sections like Demo Video, Review Trigger, and Checklist. Rewrite the PR description using the repo template and add the missing Testing, Demo Video, Review Trigger, and Checklist sections.
✅ Passed checks (23 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 The PR diff only touches Rust/docs; no Swift files or Swift actor-isolation-sensitive changes are introduced.
Cmux Swift Blocking Runtime ✅ Passed The PR diff contains no Swift files, so it doesn't introduce or expand Swift blocking/timing synchronization.
Cmux Browser Automation Off-Main ✅ Passed PASS: The rule targets Swift control-socket code; this PR routes Rust browser commands through per-surface workers and adds regression tests for socket-worker behavior.
Cmux Expensive Synchronous Load ✅ Passed Diff only changes Rust and docs; no Swift files or SwiftUI/main-actor sync-load paths are touched, so the rule is not applicable.
Cmux Cache Substitution Correctness ✅ Passed No Swift/TS/JS production files changed; the diff is Rust and docs only, so the cache-substitution rule doesn’t apply.
Cmux No Hacky Sleeps ✅ Passed PR only changes Rust/docs; no TS/JS/shell/runtime scripts were modified, and the only sleeps found are in test scaffolding, which the rule allows.
Cmux Algorithmic Complexity ✅ Passed New coalescing loops run on fixed-capacity queues (64/512), and attach/adopt uses keyed lookups plus one existing pane_of scan; no new unbounded rescans.
Cmux Swift Concurrency ✅ Passed No Swift files or Swift concurrency changes are in the diff; the PR only touches Rust code and docs.
Cmux Swift @Concurrent ✅ Passed No Swift files were changed in the diff, so the @concurrent review rule is not applicable.
Cmux Swift File And Package Boundaries ✅ Passed No Swift files changed in this PR, so the Swift file/package boundary rule is not implicated.
Cmux Swiftpm Lockfiles ✅ Passed No changed files touch SwiftPM/Xcode/package-resolved triggers; the PR only modifies Rust/docs, so the lockfile rule is not implicated.
Cmux Swift Logging ✅ Passed No Swift files changed, so the Swift logging rule is not applicable to this diff.
Cmux Full Internationalization ✅ Passed Only standalone mux docs/README and Rust config/protocol code changed; no Swift catalogs, web locale files, or localized web content were touched.
Cmux Swiftui State Layout ✅ Passed PR touches only Rust and docs; no SwiftUI views, @Observable/@published, GeometryReader, or render-time state writes were introduced.
Cmux Architecture Rethink ✅ Passed No Swift files are changed in HEAD vs parent, so the Swift architectural-rethink rule is not applicable.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed No Swift files changed in HEAD^..HEAD, so the auxiliary-window close-shortcut rule is not applicable.
Cmux Source Artifacts ✅ Passed Only intentional source/docs files changed; no artifact, temp, cache, build, or log paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PR changes only touch mux Rust/docs files; no production Swift Sources files were modified, so this check is not applicable.
Cmux No Ambient Global State ✅ Passed PR adds only owned config/runtime fields and private file-local helpers; no new file-scope mutable globals or singleton-style state were introduced.
Title check ✅ Passed The title is concise and clearly summarizes the two main changes: non-blocking CDP I/O and headful Chrome default.
✨ 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 feat-mux-browser-improvements

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.

@greptile-apps

greptile-apps Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes browser panes non-blocking and changes launched Chrome to a visible default. The main changes are:

  • Per-surface browser workers handle CDP input, navigation, activation, and resize work.
  • Browser control commands now use bounded queues with immediate acceptance responses.
  • Back, Forward, Reload, and Activate stay in FIFO order instead of sharing the latest navigation slot.
  • Launched Chrome now defaults to headful mode with a persistent session profile.
  • Browser docs and protocol notes describe queue semantics, attach streaming, and Chrome profile caveats.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
cmux-tui/crates/cmux-tui-core/src/browser.rs Adds the per-surface browser worker, FIFO control commands, latest-wins navigation and resize slots, and not-responding state handling.
cmux-tui/crates/cmux-tui/src/browser_input.rs Routes browser commands through the off-loop dispatcher and reports failed discrete control commands back to the app.
cmux-tui/crates/cmux-tui/src/app.rs Moves browser shortcuts, menu actions, omnibar commands, and activation through the browser command dispatcher.
cmux-tui/crates/cmux-tui-cdp/src/chrome.rs Adds browser mode launch arguments and makes headful Chrome the default for launched runtimes.
cmux-tui/crates/cmux-tui-cdp/src/client.rs Adds CDP helpers for browser version lookup, user-agent override, and detached target close.

Reviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment on lines +1188 to +1196
if self.is_dead() {
anyhow::bail!("browser surface is closed");
}
*self.latest_reconfigure.lock().unwrap() = Some(command);
self.wake_worker()
}

fn enqueue_latest_nav(&self, command: BrowserCommand) -> anyhow::Result<()> {
if self.is_dead() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 History Commands Collapse

Back, Forward, Reload, and Activate now use the same latest-wins slot as URL navigation. If a user double-clicks Back or sends Back then Forward while the worker is still blocked in a CDP timeout, the later command overwrites the earlier one while both callers receive success, so relative history actions silently execute fewer steps than requested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in this branch. back/forward/reload/activate no longer share the latest-wins nav slot; they enqueue on the per-surface bounded FIFO command channel, so a queued back can never be overwritten by a later forward. Only Navigate (URL) keeps the latest-wins slot, where just the final destination matters. Ordering between a Navigate and a channel command is preserved because Navigate’s WakeLatest marker holds its position in the FIFO. Added a deterministic regression test (queued_back_and_forward_do_not_collapse_while_worker_is_blocked): it wedges the worker inside a slow Page.navigate, queues back then forward, and asserts both Page.navigateToHistoryEntry calls fire (entry 10 then 12). Verified red→green on the AWS macOS builder: without the fix it fails at back must navigate to entry 10 (got 12); with the fix it passes.

Comment on lines +1181 to +1184
match tx.try_send(command) {
Ok(()) | Err(TrySendError::Full(_)) => Ok(()),
Err(TrySendError::Disconnected(_)) => anyhow::bail!("browser command worker is closed"),
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Full Queue Drops Input

enqueue_input treats a full worker queue as success for every browser input command. When the worker is blocked in a slow CDP call and the queue fills, key presses, inserted text, clicks, or wheel events are discarded even though the socket or TUI caller already received an accepted response.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended bounded backpressure, not changing. The queue is a fixed 64-deep per-surface channel; a full queue only happens when the worker is wedged in a slow CDP call, at which point two timeouts mark the surface Failed("browser is not responding") and the loss is surfaced via browser-state/status. The two alternatives are both worse: an unbounded queue is exactly the unbounded-collection memory-growth the design avoids while a browser hangs, and a blocking send reintroduces the terminal freeze this PR fixes (the socket/TUI thread must never block on CDP I/O). Mouse-moves additionally coalesce. The more serious sibling case — discrete history commands overwriting each other even with one slot free — is the real bug and is fixed in this branch.

Comment on lines +42 to 46
mode: BrowserMode::default(),
user_data_dir: None,
ephemeral: true,
})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Headful Default Breaks Headless Hosts

Chrome::launch now uses BrowserMode::default(), which launches without --headless=new. Existing server or CI installs that create browser panes without browser.mode and without a display can no longer get a DevTools endpoint, so pane creation waits for the launch timeout and then fails instead of using the previous hidden Chrome behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deliberate, documented product default, not changing. Real headful Chrome by default is the headline feature of this PR; mux is a macOS terminal app that runs on the user’s desktop (there is a display), and signed-in sites need a real visible window to treat the pane as human. The documented opt-out is browser.mode: "headless" (covered in docs/configuration.md and docs/browser-panes.md). There is no headless "server/CI install" path that auto-creates browser panes in mux, so the previous hidden-window default is not something we are silently regressing for existing headless hosts; anyone on such a host sets browser.mode: "headless".

Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
Comment thread cmux-tui/crates/cmux-tui/src/browser_input.rs

@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: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
mux/crates/mux-tui/src/app.rs (1)

1475-1522: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor duplication across the three enqueue helpers.

enqueue_active_browser_command, enqueue_browser_command_for_pane, and enqueue_browser_command repeat the same "resolve surface → verify kind() == Browser → enqueue → set/clear status_message" shape, differing only in how the target surface is resolved. Consider extracting a shared enqueue_for(surface_id, surface, kind) helper that the three call into after resolving their surface differently.

♻️ Sketch of a shared helper
+    fn enqueue_for(&mut self, surface_id: SurfaceId, surface: SurfaceHandle, kind: BrowserInputKind) {
+        if surface.kind() != SurfaceKind::Browser {
+            self.status_message = Some("active surface is not a browser".to_string());
+            return;
+        }
+        self.browser_input.enqueue(BrowserInputEvent { surface_id, surface, kind });
+        self.status_message = None;
+    }
🤖 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 `@mux/crates/mux-tui/src/app.rs` around lines 1475 - 1522, The three browser
enqueue helpers in app.rs duplicate the same resolve/validate/enqueue/status
flow. Extract the shared “verify SurfaceKind::Browser, enqueue
BrowserInputEvent, and update status_message” logic into a common helper, then
have enqueue_active_browser_command, enqueue_browser_command_for_pane, and
enqueue_browser_command call it after they each resolve the target surface in
their own way. Preserve the existing error/status strings and keep
browser_surface_for_pane as the pane-specific resolver.
🤖 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 `@mux/crates/mux-core/src/browser.rs`:
- Around line 1186-1198: `enqueue_bounded` is silently discarding full-queue
failures for input that must not be lost, and `key_event`/`insert_text` are
using that path. Keep the bounded drop behavior only for disposable mouse-move
style commands, and route keyboard/text commands through a separate enqueue path
that returns an error when the queue is full. Update the call sites in
`BrowserCommand` handling for `key_event` and `insert_text` so they do not
report success when keystrokes or pasted text are dropped.
- Around line 690-720: The FIFO ordering in start_browser_worker is being broken
because take_latest_worker_commands is drained after every batch, letting
latest-slot navigation commands run ahead of older queued browser controls.
Update the worker loop in start_browser_worker so latest_reconfigure/latest_nav
are only flushed when a BrowserCommand::WakeLatest is processed in its FIFO
position, or otherwise preserve sequencing so Back/Forward/Reload cannot be
overtaken by later Navigate commands. Keep the fix localized to the
batching/draining logic around take_latest_worker_commands and
run_browser_worker_command.
- Around line 803-808: Recovery state is split between worker success handling
in the match on result, store_frame, and the timeout path, so not-responding
recovery can get out of sync. Centralize the recovery reset logic in a shared
helper or introduce a shared recovery epoch used by both the worker command flow
and store_frame, and make sure it clears both consecutive_timeouts and
not_responding_reported together. Update the success path in the worker handler,
the frame-recovery path in store_frame, and the timeout-triggering logic so they
all consult the same recovery state.

In `@mux/docs/configuration.md`:
- Around line 57-58: Update the documentation for browser.capture_scale to match
the runtime validation in the config docs: the accepted range is strictly
greater than 0 and up to 1, not 0.0 through 1.0. Adjust the description near
browser.max_capture_megapixels in configuration.md so it clearly states the
lower bound is exclusive and avoids implying 0.0 is valid.

In `@mux/docs/protocol.md`:
- Around line 113-114: The protocol wording in the browser-queue sentence is
conflating separate browser commands with the `resize-surface` flow. Reword the
sentence around the browser CDP queue behavior so it clearly refers only to
`resize-surface` enqueuing per-surface work and returning `ok:true`, while
keeping browser input, navigation, activation, and reconfigure as separate verbs
handled elsewhere; use the existing browser state/status event language to
describe completion and failure.

---

Outside diff comments:
In `@mux/crates/mux-tui/src/app.rs`:
- Around line 1475-1522: The three browser enqueue helpers in app.rs duplicate
the same resolve/validate/enqueue/status flow. Extract the shared “verify
SurfaceKind::Browser, enqueue BrowserInputEvent, and update status_message”
logic into a common helper, then have enqueue_active_browser_command,
enqueue_browser_command_for_pane, and enqueue_browser_command call it after they
each resolve the target surface in their own way. Preserve the existing
error/status strings and keep browser_surface_for_pane as the pane-specific
resolver.
🪄 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: 378b4707-5321-4bcc-9fda-97a72096d7cd

📥 Commits

Reviewing files that changed from the base of the PR and between a44618f and 487df71.

📒 Files selected for processing (15)
  • mux/README.md
  • mux/crates/mux-cdp/src/chrome.rs
  • mux/crates/mux-cdp/src/client.rs
  • mux/crates/mux-cdp/src/lib.rs
  • mux/crates/mux-core/src/browser.rs
  • mux/crates/mux-core/src/lib.rs
  • mux/crates/mux-core/src/mux.rs
  • mux/crates/mux-core/src/surface.rs
  • mux/crates/mux-core/tests/browser_runtime.rs
  • mux/crates/mux-tui/src/app.rs
  • mux/crates/mux-tui/src/browser_input.rs
  • mux/crates/mux-tui/src/config.rs
  • mux/docs/browser-panes.md
  • mux/docs/configuration.md
  • mux/docs/protocol.md

Comment on lines +690 to +720
fn start_browser_worker(
surface: Arc<Surface>,
rx: Receiver<BrowserCommand>,
latest_reconfigure: Arc<Mutex<Option<BrowserCommand>>>,
latest_nav: Arc<Mutex<Option<BrowserCommand>>>,
mux: Weak<Mux>,
done_tx: Option<Sender<()>>,
) {
let id = surface.id;
let _ =
std::thread::Builder::new().name(format!("browser-surface-{id}-worker")).spawn(move || {
let mut failures = BrowserWorkerErrorState::default();
while let Ok(first) = rx.recv() {
let mut batch = vec![first];
while let Ok(next) = rx.try_recv() {
batch.push(next);
}
coalesce_worker_mouse_moves(&mut batch);
for command in batch {
if matches!(command, BrowserCommand::WakeLatest) {
for command in take_latest_worker_commands(&latest_reconfigure, &latest_nav)
{
run_browser_worker_command(&surface, command, &mux, id, &mut failures);
}
} else {
run_browser_worker_command(&surface, command, &mux, id, &mut failures);
}
}
for command in take_latest_worker_commands(&latest_reconfigure, &latest_nav) {
run_browser_worker_command(&surface, command, &mux, id, &mut failures);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Preserve FIFO ordering when draining latest-slot commands.

Line 718 drains latest_nav/latest_reconfigure after every batch, even before FIFO commands that arrived while the worker was executing the current batch. That lets a later Navigate leapfrog an earlier queued Back/Forward/Reload, breaking the ordering contract for browser actions. Drain latest slots only when a WakeLatest marker reaches its FIFO position, or add sequencing so latest-wins commands cannot bypass older queued controls.

🤖 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 `@mux/crates/mux-core/src/browser.rs` around lines 690 - 720, The FIFO ordering
in start_browser_worker is being broken because take_latest_worker_commands is
drained after every batch, letting latest-slot navigation commands run ahead of
older queued browser controls. Update the worker loop in start_browser_worker so
latest_reconfigure/latest_nav are only flushed when a BrowserCommand::WakeLatest
is processed in its FIFO position, or otherwise preserve sequencing so
Back/Forward/Reload cannot be overtaken by later Navigate commands. Keep the fix
localized to the batching/draining logic around take_latest_worker_commands and
run_browser_worker_command.

Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
Comment on lines +1186 to +1198
// Bounded, in-order delivery for disposable pointer/key input. Input events
// are high-frequency and individually expendable, so under backpressure the
// worker queue drops the newest event rather than blocking or replacing an
// unrelated queued one. Callers are intentionally told `ok` even on drop:
// losing one mouse-move or keystroke frame is not a reported failure.
fn enqueue_bounded(&self, command: BrowserCommand) -> anyhow::Result<()> {
if self.is_dead() {
anyhow::bail!("browser surface is closed");
}
let tx = self.command_sender()?;
match tx.try_send(command) {
Ok(()) | Err(TrySendError::Full(_)) => Ok(()),
Err(TrySendError::Disconnected(_)) => anyhow::bail!("browser command worker is closed"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not silently drop keyboard/text input.

enqueue_bounded returns Ok(()) on a full queue, and key_event/insert_text use that path. Dropping mouse moves is acceptable, but dropping keystrokes or pasted text while reporting success loses user input. Route non-disposable text/key commands through an error-on-full enqueue path.

Proposed direction
+    fn enqueue_required_input(&self, command: BrowserCommand) -> anyhow::Result<()> {
+        if self.is_dead() {
+            anyhow::bail!("browser surface is closed");
+        }
+        let tx = self.command_sender()?;
+        match tx.try_send(command) {
+            Ok(()) => Ok(()),
+            Err(TrySendError::Full(_)) => {
+                anyhow::bail!("browser command queue is full; browser may be unresponsive")
+            }
+            Err(TrySendError::Disconnected(_)) => anyhow::bail!("browser command worker is closed"),
+        }
+    }
+
     pub fn key_event(
         &self,
         event_type: &str,
         key: &str,
@@
-        self.enqueue_bounded(BrowserCommand::Key {
+        self.enqueue_required_input(BrowserCommand::Key {
             event_type: event_type.to_string(),
             key: key.to_string(),
             code: code.to_string(),
@@
     pub fn insert_text(&self, text: &str) -> anyhow::Result<()> {
-        self.enqueue_bounded(BrowserCommand::InsertText(text.to_string()))
+        self.enqueue_required_input(BrowserCommand::InsertText(text.to_string()))
     }

Also applies to: 1336-1365

🤖 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 `@mux/crates/mux-core/src/browser.rs` around lines 1186 - 1198,
`enqueue_bounded` is silently discarding full-queue failures for input that must
not be lost, and `key_event`/`insert_text` are using that path. Keep the bounded
drop behavior only for disposable mouse-move style commands, and route
keyboard/text commands through a separate enqueue path that returns an error
when the queue is full. Update the call sites in `BrowserCommand` handling for
`key_event` and `insert_text` so they do not report success when keystrokes or
pasted text are dropped.

Comment on lines +57 to +58
| `browser.max_capture_megapixels` | number | `2.0` | Maximum browser capture size before downscaling |
| `browser.capture_scale` | number or null | `null` | Fixed capture scale from 0.0 through 1.0 |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Correct the browser.capture_scale lower bound.

The runtime accepts only 0 < scale <= 1, so documenting 0.0 through 1.0 is inaccurate and will mislead users. A literal 0.0 config will be rejected at load time.

♻️ Proposed doc fix
-| `browser.capture_scale` | number or null | `null` | Fixed capture scale from 0.0 through 1.0 |
+| `browser.capture_scale` | number or null | `null` | Fixed capture scale from greater than 0.0 through 1.0 |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| `browser.max_capture_megapixels` | number | `2.0` | Maximum browser capture size before downscaling |
| `browser.capture_scale` | number or null | `null` | Fixed capture scale from 0.0 through 1.0 |
| `browser.max_capture_megapixels` | number | `2.0` | Maximum browser capture size before downscaling |
| `browser.capture_scale` | number or null | `null` | Fixed capture scale from greater than 0.0 through 1.0 |
🤖 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 `@mux/docs/configuration.md` around lines 57 - 58, Update the documentation for
browser.capture_scale to match the runtime validation in the config docs: the
accepted range is strictly greater than 0 and up to 1, not 0.0 through 1.0.
Adjust the description near browser.max_capture_megapixels in configuration.md
so it clearly states the lower bound is exclusive and avoids implying 0.0 is
valid.

Comment thread cmux-tui/docs/protocol.md
@lawrencecchen
lawrencecchen force-pushed the feat-mux-browser-improvements branch 2 times, most recently from 0ff130a to c31b191 Compare July 8, 2026 08:51
Comment thread mux/crates/mux-core/src/browser.rs Outdated
lawrencecchen added a commit that referenced this pull request Jul 8, 2026
#7622)

The change-area detector treats any path not explicitly macos-neutral as
a macOS change, so mux-only PRs resolved macos=true. Combined with the
new linux-preflight staging (#7583), that made the required app-host
Swift tests skip while the routing guard required them, failing 'tests'
and 'ci-status' on every mux-only PR (e.g. #7609).

cmux-mux is a standalone Rust project gated by its own 'mux' workflow and
never affects the macOS app build or app-host tests, so 'mux/' belongs in
is_macos_neutral. Adds test_mux_only_skips_macos.
Two browser-pane improvements on top of the CDP base on main.

Anti-lag: browser CDP commands never block the socket or the TUI event
loop. Each browser surface owns a per-surface worker thread with a
bounded queue; server browser-* handlers and the resize CDP reconfigure
enqueue and ack immediately (accepted, not completed). Discrete history
commands (back/forward/reload/activate) and input go through the bounded
FIFO so a queued Back is never overwritten by a later Forward; only URL
navigation uses the latest-wins slot. Two consecutive CDP timeouts mark
only that surface Failed('browser is not responding'), once per stall
episode, re-armed by a fresh frame; other panes stay live. No mutex held
across a blocking call; worker reaped on kill and the pane-missing path;
wedged-pane close is fire-and-forget.

Real Chrome: browser.mode defaults to headful, launching the user's real
Google Chrome in a visible window with a persistent profile so signed-in
sites treat the pane as a human. --disable-blink-features=
AutomationControlled plus a launched-only best-effort UA de-headless
clear the two loudest bot signals. browser.mode: headless opts back into
a hidden window. Docs cover the Chrome 136 / SingletonLock real-profile
caveats and agent-browser ws:// attach.

Verified on the AWS M4 Pro builder (macOS 15.7.4): fmt, clippy, cargo
test --workspace all green; mux-core --test-threads=64 clean x3. This
Mac's macOS 26 SDK cannot link ghostty-vt-sys locally.
@lawrencecchen
lawrencecchen force-pushed the feat-mux-browser-improvements branch from c31b191 to 8e2d4b9 Compare July 8, 2026 09:28
Comment thread cmux-tui/crates/cmux-tui-core/src/browser.rs
…ovements

# Conflicts:
#	cmux-tui/crates/cmux-tui-cdp/src/chrome.rs
#	cmux-tui/crates/cmux-tui-core/src/browser.rs
#	cmux-tui/crates/cmux-tui/src/app.rs
#	cmux-tui/crates/cmux-tui/src/browser_input.rs
#	cmux-tui/crates/cmux-tui/src/config.rs
#	cmux-tui/docs/browser-panes.md
#	cmux-tui/docs/configuration.md
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
Validation Failed: {"resource":"IssueComment","code":"custom","field":"body","message":"body is too long (maximum is 65536 characters)"} - https://docs.github.com/rest/issues/comments#update-an-issue-comment

Comment thread mux/target/CACHEDIR.TAG Outdated
The merge commit accidentally re-staged the worktree's stale ghostty
checkout over main's submodule bump.
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
Validation Failed: {"resource":"IssueComment","code":"custom","field":"body","message":"body is too long (maximum is 65536 characters)"} - https://docs.github.com/rest/issues/comments#update-an-issue-comment

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
Validation Failed: {"resource":"IssueComment","code":"custom","field":"body","message":"body is too long (maximum is 65536 characters)"} - https://docs.github.com/rest/issues/comments#update-an-issue-comment

Comment thread mux/target/.rustc_info.json Outdated
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
Validation Failed: {"resource":"IssueComment","code":"custom","field":"body","message":"body is too long (maximum is 65536 characters)"} - https://docs.github.com/rest/issues/comments#update-an-issue-comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 20c4fba. Configure here.

self.dead.store(true, Ordering::Release);
self.close_taps();
let _ = self.session.lock().unwrap().take();
self.close_command_sender();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale not-responding latch

Medium Severity

After a browser is not responding episode, not_responding_reported is cleared only when a fresh screencast frame arrives. Successful clear_error (back/forward/reload) or set_url_title (successful navigate) can return the pane to Live without resetting that latch, so later CDP timeouts may no longer promote the surface to failed or broadcast recovery to attach clients.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 20c4fba. Configure here.

@lawrencecchen
lawrencecchen merged commit 1e602aa into main Jul 12, 2026
35 checks passed

This branch was successfully deployed

1 active deployment
Preview – cmux — 20c4fbab Deployed Jul 12, 2026 by vercel[bot]
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.

1 participant