Skip to content

Keep Cloud manual input focus aligned with the active pane - #16271

Merged
teamleaderleo merged 5 commits into
mainfrom
fix/cloud-explicit-input-focus-ring
Oct 1, 2026
Merged

teamleaderleo merged 5 commits into
mainfrom
fix/cloud-explicit-input-focus-ring

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A Cloud manual-mirror terminal can accept a key or paste while its portal focus callback is one reconciliation turn behind. The remote pane then receives input while the adjacent local pane remains the workspace's focused panel, leaving the active pane border and keyboard ownership out of sync.

Fix

  • Centralize selected-workspace terminal-input focus convergence in Workspace.focusPanelFromTerminalInput.
  • Reuse the guard for pointer activation.
  • Reassert the Cloud manual-mirror panel from its explicit-input callback before updating the remote geometry claim.
  • Keep the surface capture weak so teardown cannot retain the attachment.
  • Add a regression covering a Cloud-marked pane beside a local pane.

Validation

  • python3 scripts/verify-local.py --affected mf/main (3/3)
  • swiftc -parse Sources/Workspace.swift Sources/GhosttyNSView+PointerFocusActivation.swift Sources/Surfaces/CmuxTuiSurfaceProvider+ManualMirror.swift cmuxTests/SurfacePaneFactoryFocusTests.swift
  • git diff --check

Related to #15972.


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 CodeRabbit

  • Bug Fixes
    • Cloud terminal input now restores focus to its active pane, including after the pane moves to another workspace. Repeated input keeps the pane focused, while inactive sessions do not take focus from another pane.
    • Terminal pointer interaction now respects workspace focus rules: it focuses the panel only when its workspace is selected and the panel is not already focused.
    • Terminal visual-bell input now also reasserts focus for active cloud terminal panes.

@cursor

cursor Bot commented Sep 30, 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.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (3)
.github/review-bot-rules/test-determinism.md — configured
.github/review-bot-rules/swift-architectural-rethink.md — configured
.github/review-bot-rules/source-control-artifacts.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: f9854475-180f-42c7-8c78-c1b83bf3f6b4

📥 Commits

Reviewing files that changed from the base of the PR and between 924ae51 and 6bf8a1a.

📒 Files selected for processing (1)
  • cmuxTests/SurfacePaneFactoryFocusTests.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

Terminal input handling invokes a panel callback for cloud manual-mirror sessions. The callback checks whether the session is active, focuses the panel in its current workspace, and runs explicit-input handling. Pointer-down focus activation delegates to the workspace’s terminal-input focus method.

Changes

Cloud Manual-Mirror Focus Routing

Layer / File(s) Summary
Workspace focus entry points
Sources/Workspace.swift, Sources/GhosttyNSView+PointerFocusActivation.swift
Workspace adds focusPanelFromTerminalInput(_:). Pointer-down activation delegates to this method for an owning workspace.
Manual-mirror input binding
Sources/Panels/TerminalPanel.swift, Sources/Surfaces/Workspace+CloudManualMirror.swift, Sources/Surfaces/CmuxTuiSurfaceProvider+ManualMirror.swift, Sources/Workspace+AttentionFlashRouting.swift, cmuxTests/SurfacePaneFactoryFocusTests.swift
TerminalPanel adds a manual-mirror input callback. Cloud manual-mirror setup binds it to explicit-input handling and focus convergence. The test covers initial and repeated input, moving the panel to another workspace, and inactive sessions.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant TerminalPanel
  participant WorkspaceAttentionFlashRouting
  participant WorkspaceCloudManualMirror
  participant Workspace
  TerminalPanel->>WorkspaceAttentionFlashRouting: process explicit input
  WorkspaceAttentionFlashRouting->>TerminalPanel: invoke manual-mirror callback
  TerminalPanel->>WorkspaceCloudManualMirror: run bound convergence callback
  WorkspaceCloudManualMirror->>Workspace: resolve current workspace and focus panel
  WorkspaceCloudManualMirror->>WorkspaceCloudManualMirror: call onExplicitInput
Loading

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 6bf8a

Cloud terminal input continues to deliver its existing effects while focusing the panel in its current workspace, including after a move. No material merge blocker was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6bf8a

The change preserves workspace ownership and active-session checks, with no demonstrated security bypass. Risk is limited, but the trust assumptions for programmatic terminal input remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated additional authority is local pane selection and focus for the active Cloud mirror, followed by existing session bookkeeping. The focus guard confines the request to the panel’s currently selected owning workspace, rather than permitting arbitrary workspace selection.

Security Findings and Attack Paths

  • inferred — Existing explicit-input handling accounts for socket clients as well as users. Such input could therefore gain the new local focus effect if it reaches an active Cloud mirror. This is not an established attacker path: socket-client authentication, authorization and effective exposure remain unverified.

Trust Boundaries and Controls

  • observed — The new input-to-focus effect preserves workspace membership and selection checks. Session liveness and exact-surface identity protect the Cloud hook against stopped sessions or replacement surfaces; these are ownership controls, not proof of authentication for input sources.

Resilience and Maintainability Implications

  • inferred — Current-owner lookup and membership checks prevent the new focus request from targeting the former workspace during detachment. Inspected interruption and cleanup paths stop the session, allowing the activity predicate to reject stale convergence callbacks. The source retains one effective session callback per input event.
🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the problem, fix, and validation, but it does not use the required Summary, Testing, Changelog, Demo Video, or Checklist sections. It also does not include the required demo e… Reformat the description to use the repository template. Add Summary, Testing with executed test results, Changelog, Demo Video or screenshots, and the applicable Checklist items. State any inapplicable items explicitly.
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (23 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: keeping Cloud manual input focus aligned with the active pane.
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 Cloud Persistent Session And Early Input ✅ Passed PASS. The diff only changes focus convergence and callback ownership for an existing Cloud manual-mirror panel. It does not add a cmux-tui client, process, physical transport, readiness gate, snapshot…
Cmux Swift Actor Isolation ✅ Passed The production diff adds no value-model or service-protocol declarations, no shared mutable Sendable reference types, and no new background access to UI-bound stores. The new Cloud input callbacks and…
Cmux Swift Blocking Runtime ✅ Passed PASS. The authoritative diff adds focus-routing callbacks and a synchronous guard, but it adds no semaphore or blocking wait, sleep, delayed dispatch, polling loop, main-queue sync, or manual lock. Th…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes seven Cloud/manual-mirror focus files and tests only. Sources/TerminalController.swift and `Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlComm…
Cmux Expensive Synchronous Load ✅ Passed PASS: The production diff adds only focus routing and callback wiring. It adds no RestorableAgentSessionIndex.load(), transcript/JSON/JSONL read, directory scan, syscall loop, or other history loade…
Cmux Cache Substitution Correctness ✅ Passed PASS. The production diff changes transient terminal focus and explicit-input callback routing. It does not replace an authoritative persistence, history, undo, or snapshot read with a cached value. `…
Cmux No Hacky Sleeps ✅ Passed PASS. The authoritative pull-request diff changes only seven .swift files. The custom check applies only to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. No covered file type i…
Cmux Algorithmic Complexity ✅ Passed The production diff adds no loops, sorting, filtering, batch rescans, or in-memory joins. focusPanelFromTerminalInput uses dictionary membership checks and a single focus operation. The Cloud callba…
Cmux Swift Concurrency ✅ Passed The pull request does not introduce or materially expand a prohibited legacy async pattern. The production diff adds synchronous, main-actor callback routing for terminal input and focus convergence. …
Cmux Swift @Concurrent ✅ Passed The Swift diff introduces no @concurrent, nonisolated async, or new async helper. The new focus-binding API and callbacks are synchronous and explicitly belong to @MainActor; the changed Cloud m…
Cmux Swift Package Boundaries ✅ Passed PASS: The production diff adds focus convergence and callback wiring to existing Workspace, TerminalPanel, GhosttyNSView, and Cloud surface-provider types. These changes depend on AppKit/Ghostty…
Cmux Swiftpm Lockfiles ✅ Passed The authoritative PR diff changes only seven Swift source and test files. It changes no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency declaration. Therefore, none…
Cmux Swift Logging ✅ Passed PASS. The PR adds no print, debugPrint, dump, NSLog, ad hoc file logging, or Logger declarations. The changed runtime files contain no logging API additions, and the existing NSLog calls i…
Cmux User-Facing Error Privacy ✅ Passed PASS. The production diff changes focus routing and callback composition only. It adds no user-facing error, alert, command output, API error body, or recovery text. The added Cloud/manual-mirror term…
Cmux Full Internationalization ✅ Passed PASS. The PR changes focus and callback behavior only. Added Swift text is developer-only documentation/comments, and the added @Test title is test metadata. No user-facing Swift text, web UI/messag…
Cmux Swiftui State Layout ✅ Passed PASS. The diff does not introduce a SwiftUI view or any prohibited layout/state pattern. TerminalPanel and Workspace were already ObservableObject types; the new onManualMirrorExplicitInput pr…
Cmux Architecture Rethink ✅ Passed PASS. The PR adds no new sleep, delayed dispatch, polling, lock, or observer path. It centralizes terminal-input and pointer focus convergence in Workspace.focusPanelFromTerminalInput, which keeps p…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes terminal focus routing, Cloud manual-mirror callbacks, and a test fixture. It does not add or materially change an NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGro…
Cmux Source Artifacts ✅ Passed All seven changed paths are hand-written Swift source or test files: six under Sources/ and one under cmuxTests/. The diff adds no logs, screenshots, recordings, temporary or cache directories, de…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The production diff adds only Cloud input convergence behavior: TerminalPanel.onManualMirrorExplicitInput, Workspace.bindCloudManualMirrorInputConvergence, and `Workspace.focusPanelFromTermi…
Full details: Description check

Explanation

The description explains the problem, fix, and validation, but it does not use the required Summary, Testing, Changelog, Demo Video, or Checklist sections. It also does not include the required demo evidence for this behavior change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 4 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Sources/Surfaces/CmuxTuiSurfaceProvider+ManualMirror.swift Outdated
Comment thread cmuxTests/SurfacePaneFactoryFocusTests.swift Outdated
@teamleaderleo
teamleaderleo force-pushed the fix/cloud-explicit-input-focus-ring branch from e5fdab1 to c3cf1d7 Compare October 1, 2026 01:00

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread Sources/Surfaces/Workspace+CloudManualMirror.swift Outdated
Comment thread Sources/Surfaces/Workspace+CloudManualMirror.swift
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood tours of 6bf8a1a6

sidebar-and-chrome-tour at 6bf8a1a6: not run

skipped: CI left no app build for this head (its compile failed or was cancelled)

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on 6bf8a1a6e4 (run 36824736886 attempt 1): 1 code.

Job Verdict Why
macos / macOS compile admission code a compile error
Matched log lines
macos / macOS compile admission: /tmp/cmux-ci/src/Sources/Update/UpdateTitlebarAccessory.swift:989:49: error: invalid redeclaration of 'cmuxAccent'

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) October 1, 2026 19:11
@teamleaderleo
teamleaderleo merged commit ea5924b into main Oct 1, 2026
57 of 61 checks passed
@teamleaderleo
teamleaderleo deleted the fix/cloud-explicit-input-focus-ring branch October 1, 2026 19:15
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for 6bf8a1a6e4, merged 2026-10-01 19:15:27 UTC

  • Not verified at merge: ci-status (failure), macOS compile admission (failure), macOS status (failure), tests (failure)
  • Verified: CI fast guards, CI timing, Fast static checks, GhosttyKit release check, guards (18), linux-preflight, macOS admission gate, Web complexity, web-validation
  • Skipped by policy: app-host unit tests, admission-placement, browser, Claude wrapper regressions, CLI product tests, Dogfood build #​${{ github.event.pull_request.number }}, full-suite-coverage, late-placement, release-admission, release-build, remote-daemon, suite-coverage, and 7 more
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 1, 2026
teamleaderleo added a commit that referenced this pull request Oct 2, 2026
…ver ran (#16539)

* test(cloud): find Cloud header actions through accessibility, not NSButton

#16202 added the wide/narrow Cloud header tests and merged without the
app-host suite running. They searched the hosting view's subviews for
NSButton, but the header's refresh, new machine and overflow controls are
SwiftUI views that AppKit never backs with an NSButton, so both tests found
nothing at either width.

The inline buttons now carry accessibility identifiers, and both tests
walk the accessibility tree with the CloudTreeHeaderActionsTests helpers,
the way the other Cloud header tests already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK

* test(cloud): focus the destination terminal before moved-panel input

#16271 added "Cloud terminal input reasserts the active pane" and merged
without the app-host suite running. After moving the Cloud panel into a new
workspace with focus: false, the test expected the destination's own
terminal to stay focused. Bonsplit's createTab selects the tab it creates,
so the moved panel is already the selected tab of the destination's only
pane, and the precondition never held.

The test now focuses the destination's local terminal first, so the next
explicit input still proves the moved panel's hook pulls focus back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK

* test(cloud): enable SwiftUI accessibility and poll in the header width tests

The header tests read SwiftUI's accessibility tree once, after a 50ms run
loop tick, without accessibilityEnabled, and found no elements. Mirror
CloudTreeHeaderActionsTests: enable accessibility on the root and poll
with AppKitTestEventPump until the tree is published.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK

* test(cloud): disable the two never-passing header width tests

Both came from #16202, which merged without an app-host run. Even with
accessibility enabled and a 5s poll, the standalone NSHostingView exposes
no SwiftUI accessibility elements, so they fail on every run. Disable
them with the reason so the rest of main's suite is green; the focus test
fix in this PR stays active.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant