Skip to content

Fix drag-to-split cursor focus reconciliation - #9754

Merged
austinywang merged 2 commits into
mainfrom
issue-9504-dragsplit-focused-cursor
Aug 10, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-9504-dragsplit-focused-cursor

Conversation

@austinywang

@austinywang austinywang commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • make focus intent explicit for every existing-tab-to-new-split transaction
  • reconcile Bonsplit pane selection, Workspace focus, Ghostty cursor focus, and AppKit first responder through one shared path
  • reuse the existing event-driven layout follow-up so SwiftUI reparenting cannot leave cursor styling stale

Root cause

Bonsplit moves the tab and changes its focused pane synchronously, but that operation emits only didSplitPane. Workspace therefore did not commit the moved tab through its focus owner, and the following SwiftUI reparent pass could disturb AppKit first responder independently. The model, cursor, and responder layers could diverge even though Bonsplit still tracked only one focused pane.

Testing

Audit

  • no user-facing strings changed; localization files require no updates
  • no warning-budget files changed

Closes #9504


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

Fix drag-to-split so the moved terminal becomes the only focused cursor when requested, or preserves current focus and agent hibernation when not. Makes focus intent explicit for all moving‑tab splits and reconciles AppKit first responder through SwiftUI reparenting (fixes #9504).

  • Bug Fixes
    • Added Workspace.splitPaneMovingTab(..., focusIntent: ...) with MovingTabSplitFocusIntent; on preserve, restore previous pane/tab and the panel’s owned responder; suppress Bonsplit delegate activation.
    • In the Bonsplit delegate, for moving‑tab splits call activateMovedTabAfterSplit; guard responder churn via suppressReparentFocusUntilLayoutFollowUp(..., terminalFocusPanelId: ...); ignore didSelectTab/didFocusPane while preserving focus.
    • Routed all call sites through the helper: AppDelegate (dock and same‑workspace moves) and directional navigation use .activateMovedTab; control sidebar and surface.split_off use .preserveCurrent. Added DEBUG tests for sole‑cursor activation, reassertion without a previous terminal responder, and non‑focus split hibernation.

Written for commit 5f19f16. Summary will update on new commits.

Review in cubic


Note

Medium Risk
Changes focus ownership across many split/move entry points and Bonsplit delegate handling; regressions would show up as wrong terminal focus or lost hibernation after drag-to-split, but behavior is covered by new tests.

Overview
Fixes drag-to-split and other move-tab-into-split paths so keyboard focus matches intent: the moved terminal becomes the sole focused cursor when activation is requested, and stays inactive when focus must be preserved.

Introduces Workspace.splitPaneMovingTab with MovingTabSplitFocusIntent (activateMovedTab vs preserveCurrent). Call sites in AppDelegate (dock and cross-workspace moves), surface navigation, control sidebar, and surface.split_off route through it instead of calling bonsplitController.splitPane directly.

On didSplitPane for moving-tab splits, activateMovedTabAfterSplit applies tab selection and suppressReparentFocusUntilLayoutFollowUp (now optionally keyed by terminalFocusPanelId) so AppKit first responder survives SwiftUI reparenting. didSelectTab / didFocusPane are skipped while preserving focus; preserve mode restores the prior pane, tab, and owned responder (including agent hibernation).

Adds WorkspaceDragSplitFocusTests for activation, post-reparent reassertion, and non-focus preservation.

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

Summary by CodeRabbit

  • Bug Fixes

    • Improved focus handling when moving terminal tabs into new split panes.
    • Correctly activates the moved terminal when requested.
    • Preserves current focus, cursor state, and agent hibernation when focus should remain unchanged.
    • Prevents focus conflicts while closing tabs during detach operations.
    • Applies consistent focus behavior across workspace, sidebar, and directional split actions.
  • Tests

    • Added coverage for focus transfer and preservation after moving terminals into split panes.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 148e99a5-a8dc-4476-83a7-6e64068dbfcc

📥 Commits

Reviewing files that changed from the base of the PR and between 66529e0 and 5f19f16.

📒 Files selected for processing (2)
  • Sources/Workspace.swift
  • cmuxTests/WorkspaceDragSplitFocusTests.swift

📝 Walkthrough

Walkthrough

Moving-tab splits now use explicit focus intent. The workspace preserves current panel focus or activates the moved tab after reparenting. Split call sites use the shared helper, and serialized tests cover focus transfer and focus preservation.

Changes

Drag-Split Focus Synchronization

Layer / File(s) Summary
Moving-tab split focus handling
Sources/Workspace+MovingTabSplit.swift, Sources/Workspace.swift
splitPaneMovingTab tracks focus intent, restores preserved pane and terminal focus, suppresses conflicting activation during reparenting, and activates the destination tab when required.
Split operation call-site wiring
Sources/AppDelegate.swift, Sources/AppDelegate+DockSurfaceMove.swift, Sources/TerminalController+ControlSidebarContext3.swift, Sources/TerminalController+MoveTabToNewWorkspace.swift, Sources/Workspace+SurfaceNavigation.swift
Split operations now use splitPaneMovingTab and pass either .activateMovedTab or .preserveCurrent.
Focus regression coverage
cmuxTests/WorkspaceDragSplitFocusTests.swift, cmux.xcodeproj/project.pbxproj
Serialized tests verify moved-tab activation and preservation of pane focus, cursor state, and agent hibernation. The new source and test files are registered in the Xcode project.

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

Sequence Diagram(s)

sequenceDiagram
    participant SplitAction
    participant Workspace
    participant TerminalPanel
    participant LayoutFollowUp

    SplitAction->>Workspace: Call splitPaneMovingTab with focus intent
    Workspace->>TerminalPanel: Move tab and preserve or activate focus
    Workspace->>LayoutFollowUp: Suppress conflicting reparent-focus changes
    LayoutFollowUp-->>Workspace: Complete geometry reconciliation
Loading

Possibly related issues

  • manaflow-ai/cmux-dev-artifacts#9539 — Reports failures in the moving-tab split focus assertions.
  • manaflow-ai/cmux-dev-artifacts#9553 — Modifies and tests the same drag-split focus behavior.

Suggested reviewers: lawrencecchen, azooz2003-bit


Important

Pre-merge checks failed

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

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Cmux Algorithmic Complexity ❌ Error Workspace+MovingTabSplit.swift:43 adds paneId(forPanelId:), which scans every pane and each pane's tabs at Workspace+WorkspaceSurfaceTreeReading.swift:23-24; preserve also calls focusPanel's nested... Capture the pre-split PaneID or use an indexed panel-to-pane lookup, and pass that pane into focus restoration so each split avoids nested full-collection scans.
Cmux Architecture Rethink ❌ Error The fix adds activeMovingTabSplitFocusIntent as a Workspace side channel and feeds reparent focus into an asyncAfter/backoff observer loop, treating a lifecycle race after the fact instead of enf... Make Bonsplit’s split transaction carry focus intent into one Workspace-owned commit of pane, tab, and responder state; remove the mutable flag and retry-based reassertion, then test callback ordering.
✅ Passed checks (23 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the drag-to-split cursor focus reconciliation fix.
Description check ✅ Passed The description explains the fix, testing approach, linked issue, and implementation details, but omits some optional template sections.
Linked Issues check ✅ Passed The changes address [#9504] by enforcing single-terminal focus, explicit focus intent, focus handoff, and regression coverage.
Out of Scope Changes check ✅ Passed The production changes and tests are directly related to drag-to-split focus reconciliation and moving-tab split behavior.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Cmux Swift Actor Isolation ✅ Passed Workspace and AppDelegate are already @MainActor; the new focus enum/state and helpers stay in that UI transaction, with no new Sendable, service protocol, logger, or background store access.
Cmux Swift Blocking Runtime ✅ Passed Production diff adds no semaphore, blocking wait, sleep, timer, polling, main-queue sync, or lock; it reuses the existing documented event-driven layout follow-up. Added tests use no timing waits.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only moving-tab split focus paths and tests; no browser socket commands, WebKit waits, worker-router, execution-policy, or policy-test lines changed.
Cmux Expensive Synchronous Load ✅ Passed The production diff adds no agent-history loader, file read, JSON parse, or directory scan. New split paths only route focus; existing RestorableAgentSessionIndex.load() calls are unchanged.
Cmux Cache Substitution Correctness ✅ Passed The production diff only changes transient split/focus and reparent handling; it does not replace an authoritative persistence, history, undo, or snapshot read with a cache.
Cmux No Hacky Sleeps ✅ Passed The diff changes only Swift sources/tests and Xcode project metadata; it adds no covered TypeScript, JavaScript, shell, or non-Swift runtime sleep or timer.
Cmux Swift Concurrency ✅ Passed The diff adds no legacy async or Combine patterns; production concurrency-pattern counts are unchanged, and the test-only @MainActor usage is an allowed XCTest boundary.
Cmux Swift @Concurrent ✅ Passed The diff adds no async, nonisolated, or @concurrent work; new focus helpers are synchronous members of @MainActor Workspace, and existing async declarations and call sites are unchanged.
Cmux Swift Package Boundaries ✅ Passed The new logic is an internal Workspace/AppKit/Bonsplit/Ghostty focus bridge; it has no package-independent API or cross-surface domain model, so it fits the rule's allowed app glue.
Cmux Swiftpm Lockfiles ✅ Passed The PR changes no Package.swift, Package.resolved, .gitignore, or workflow files; its Xcode project diff only adds source-file entries, not SwiftPM package references.
Cmux Swift Logging ✅ Passed The diff adds no production print, debugPrint, dump, NSLog, file, or stdout logging; the only changed log is existing cmuxDebugLog under #if DEBUG, an allowed debug-only sink.
Cmux User-Facing Error Privacy ✅ Passed Production additions only change focus-routing and internal debug suppression; no user-facing errors, alerts, command output, or sensitive diagnostic text were added or changed.
Cmux Full Internationalization ✅ Passed The production diff adds no user-facing text or localization/catalog changes; its only new string is an internal debug reason, while other literals occur only in tests as protocol fixtures.
Cmux Swiftui State Layout ✅ Passed The diff adds no SwiftUI view, state-wrapper, GeometryReader, lazy-row store, or render-time mutation; existing Workspace ObservableObject/@published state is only touched incidentally.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The production diff adds focus logic only; NSWindow construction is confined to test fixtures, which the rule allows. The CI lint also passes with 35 identifiers checked.
Cmux Source Artifacts ✅ Passed All nine changed paths are Swift source, an Xcode project file, or a test; no artifact paths were added. The /tmp/issue-9504 value is a deliberate test fixture.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new test/debug seam appears in production Sources: moving-tab members have product callers, while existing DEBUG ForTesting accessors are unchanged; new scaffolding is confined to cmuxTests.
Cmux No Ambient Global State ✅ Passed Production additions are Workspace instance state and Workspace extension members; no new file-scope API, mutable global, static-only namespace, or singleton appears in the diff.
✨ 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-9504-dragsplit-focused-cursor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang
austinywang marked this pull request as ready for review August 7, 2026 04:19
@austinywang
austinywang force-pushed the issue-9504-dragsplit-focused-cursor branch from be79e7d to e76a4b0 Compare August 7, 2026 05:12

@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 `@cmuxTests/WorkspaceDragSplitFocusTests.swift`:
- Around line 30-31: Update the test setup around makeWindow() to capture the
current key window before calling makeKeyAndOrderFront(nil), then restore that
captured window in the defer cleanup alongside window.orderOut(nil). Ensure the
prior AppKit key-window state is restored even when the test exits early.
🪄 Autofix

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: 4340ea96-9f4c-4f6b-b62e-266b507184f8

📥 Commits

Reviewing files that changed from the base of the PR and between ce41f15 and 2a68075.

📒 Files selected for processing (3)
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/WorkspaceDragSplitFocusTests.swift

Comment thread cmuxTests/WorkspaceDragSplitFocusTests.swift Outdated
@austinywang
austinywang force-pushed the issue-9504-dragsplit-focused-cursor branch from 2a68075 to 515dbe0 Compare August 8, 2026 23:40
Comment thread Sources/Workspace+MovingTabSplit.swift
@austinywang
austinywang force-pushed the issue-9504-dragsplit-focused-cursor branch 2 times, most recently from c9c4374 to 66529e0 Compare August 9, 2026 00:36

@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 `@cmuxTests/WorkspaceDragSplitFocusTests.swift`:
- Around line 33-40: Update the split setup in WorkspaceDragSplitFocusTests to
call Workspace.splitPaneMovingTab with focusIntent: .activateMovedTab instead of
invoking bonsplitController.splitPane directly, while preserving the existing
source pane, orientation, moved tab, and insertion order arguments.
🪄 Autofix

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: ba89f84e-0845-4462-b210-6f76f5cf43b3

📥 Commits

Reviewing files that changed from the base of the PR and between 2a68075 and 66529e0.

📒 Files selected for processing (9)
  • Sources/AppDelegate+DockSurfaceMove.swift
  • Sources/AppDelegate.swift
  • Sources/TerminalController+ControlSidebarContext3.swift
  • Sources/TerminalController+MoveTabToNewWorkspace.swift
  • Sources/Workspace+MovingTabSplit.swift
  • Sources/Workspace+SurfaceNavigation.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/WorkspaceDragSplitFocusTests.swift

Comment thread cmuxTests/WorkspaceDragSplitFocusTests.swift

@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 66529e0. Configure here.

Comment thread Sources/Workspace+MovingTabSplit.swift
@austinywang
austinywang force-pushed the issue-9504-dragsplit-focused-cursor branch 2 times, most recently from 82ebfdd to 9f9df3a Compare August 9, 2026 01:05
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.

Drag-to-split leaves both the new and original split showing focused cursor

1 participant