Skip to content

Fix tab drag reorder to middle positions - #9398

Merged
azooz2003-bit merged 9 commits into
mainfrom
fix-tab-reorder-middle
Aug 3, 2026
Merged

azooz2003-bit merged 9 commits into
mainfrom
fix-tab-reorder-middle

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Aug 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Testing

  • Remote aws-m4pro-4: swift test --filter TabDropDelegateSamePaneFallbackTests, 2 tests passed.

Dependency

Issues

  • Related: cmux tab drag reorder middle-index bug reported in chat.

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

Fixes tab drag reorder for middle drops by using bonsplit’s manual drop target so tabs insert at the intended index. Updates vendor/bonsplit to the latest merge fixes and adds UI tests that verify both later-to-middle and earlier-to-middle reorders.

  • Bug Fixes

    • Use bonsplit manual drop target for same-pane drops so middle drops go to the correct index.
    • Preserve cross-pane same-process fallback and prevent stale native drop from overriding manual reorder.
  • Tests

    • Add UI tests for later-to-middle and earlier-to-middle reorders in a four-tab setup.
    • Add CMUX_UI_TEST_BONSPLIT_FOUR_TAB_SETUP and expose all tab titles and panel IDs for assertions.

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

Review in cubic

Summary by CodeRabbit

  • Chores
    • Updated an underlying component to a newer revision.
    • No user-facing features, behavior, or interface changes are expected.
    • Existing application functionality remains unchanged.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

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

Use the following commands to manage reviews:

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

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The vendor/bonsplit submodule reference changes from commit e3837a787968d8235106d12418b653e8d1e2619c to commit 9b3e2ed6864c10b2fcc08b3b6970742de229dbbd.

Changes

Bonsplit submodule update

Layer / File(s) Summary
Update bonsplit reference
vendor/bonsplit
The submodule pointer now references commit 9b3e2ed6864c10b2fcc08b3b6970742de229dbbd.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: lawrencecchen, austinywang


Important

Pre-merge checks failed

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

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Cmux User-Facing Error Privacy ❓ Inconclusive The diff is only a gitlink update, but the target bonsplit revision is unavailable locally, so changed vendor behavior cannot be inspected. Provide the bonsplit target revision contents or an expanded vendor diff to verify user-facing text.
✅ Passed checks (24 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 changes only the bonsplit gitlink; the referenced 4b5→9b3 revision contains only a test-file change, so no production Swift actor-isolation issue is introduced.
Cmux Swift Blocking Runtime ✅ Passed The commit changes only the vendor/bonsplit gitlink. It adds no Swift files or blocking, sleep, polling, sync, or lock primitives to the parent diff.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only the vendor/bonsplit gitlink. TerminalController.swift, ControlCommandExecutionPolicy.swift, and the browser rule are unchanged.
Cmux Expensive Synchronous Load ✅ Passed The PR diff changes only the vendor/bonsplit gitlink and no cmux Swift files; available bonsplit changes cover tab-drop reordering and tests, with no expensive agent-history loads.
Cmux Cache Substitution Correctness ✅ Passed The commit changes only the vendor/bonsplit gitlink; it contains no Swift, TypeScript, or JavaScript cache substitution in a persistence, history, undo, or snapshot path.
Cmux No Hacky Sleeps ✅ Passed The PR changes only the vendor/bonsplit gitlink; it adds no TypeScript, JavaScript, shell, or build/runtime script delay, so this check is not applicable.
Cmux Algorithmic Complexity ✅ Passed The PR changes only the bonsplit gitlink. The inspected reorder path uses bounded-count linear firstIndex/map scans, with no nested scans, batch rescans, sorting, or filtering hot path.
Cmux Swift Concurrency ✅ Passed The diff changes only the third-party vendor/bonsplit gitlink; no cmux-owned Swift files or Swift implementation changes are present.
Cmux Swift @Concurrent ✅ Passed The bonsplit pointer's only Swift diff updates synchronous test assertions; it adds no async, nonisolated, @concurrent, or actor-isolation changes.
Cmux Swift Package Boundaries ✅ Passed The commit changes only the vendor/bonsplit gitlink; no Swift production files or app-target logic are added, so the package-boundary rule is not triggered.
Cmux Swiftpm Lockfiles ✅ Passed The commit changes only the vendored third-party submodule gitlink vendor/bonsplit; no cmux-owned Package.swift, Package.resolved, Xcode project, or .gitignore changes require lockfile updates.
Cmux Swift Logging ✅ Passed The PR changes only the bonsplit pointer; its Swift range adds no prohibited logging. The only dlog change is formatting under #if DEBUG, and existing NSLog remains debug-only.
Cmux Full Internationalization ✅ Passed The parent diff only updates the bonsplit pointer; the referenced bonsplit delta changes one XCTest file only (4 insertions, 4 deletions) and adds no user-facing text or localization data.
Cmux Swiftui State Layout ✅ Passed The commit changes only the vendor/bonsplit gitlink (1 insertion and 1 deletion); it introduces no SwiftUI source or state/layout code.
Cmux Architecture Rethink ✅ Passed The diff only updates the bonsplit pointer; the referenced commit changes two tests to use XCTUnwrap. No production architecture, timing, ownership, or side-channel changes are present.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR diff changes only the vendor/bonsplit gitlink; it contains no Swift files or cmux-owned window code, so the auxiliary-window shortcut rule is not applicable.
Cmux Source Artifacts ✅ Passed The only changed path is the declared vendor/bonsplit submodule gitlink (mode 160000), and the PR gives a deliberate dependency and tab-reorder reason.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR diff changes only the vendor/bonsplit gitlink; it adds no Swift file or member under a production Sources/ path.
Cmux No Ambient Global State ✅ Passed The PR diff contains only the vendor/bonsplit gitlink update; no Swift files or production declarations changed, so this ambient-global-state check is not applicable.
Title check ✅ Passed The title clearly describes the primary fix for tab drag reordering to middle positions.
Description check ✅ Passed The description explains the change, dependency condition, and testing, but it omits the template checklist and demo video.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-tab-reorder-middle

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.

@cursor

cursor Bot commented Aug 3, 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 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 `@vendor/bonsplit`:
- Line 1: Update the vendor/bonsplit gitlink so its pinned commit is an ancestor
of origin/main, waiting for bonsplit PR `#197` to merge before selecting the
commit. Alternatively, point the dependency at a branch or tag that contains the
required changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a6d88acf-f422-411d-b193-94d40d04da57

📥 Commits

Reviewing files that changed from the base of the PR and between e7f5226 and 9720d92.

📒 Files selected for processing (1)
  • vendor/bonsplit

Comment thread vendor/bonsplit Outdated

@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 `@vendor/bonsplit`:
- Line 1: Do not merge this change while bonsplit PR `#197` remains open. After it
merges into main, repin the vendor/bonsplit gitlink to the resulting main
commit, or verify the current pinned commit is an ancestor of origin/main using
the provided validation flow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aed8adbf-2675-4758-a214-d7b1fe6b8988

📥 Commits

Reviewing files that changed from the base of the PR and between 9720d92 and ac35b62.

📒 Files selected for processing (1)
  • vendor/bonsplit

Comment thread vendor/bonsplit Outdated
@@ -1 +1 @@
Subproject commit e3837a787968d8235106d12418b653e8d1e2619c
Subproject commit 9b3e2ed6864c10b2fcc08b3b6970742de229dbbd

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Wait for bonsplit PR #197 to merge before pinning this gitlink.

As of August 3, 2026, PR #197 is still open and targets main. The main...9b3e2ed comparison contains 11 commits across 3 files, so this pointer still selects unmerged upstream history. (github.com)

Do not merge this cmux change yet. After PR #197 merges, repin vendor/bonsplit to the resulting main commit, or verify that this commit is an ancestor of origin/main.

#!/usr/bin/env bash
set -euo pipefail

path="vendor/bonsplit"
expected="$(git ls-tree HEAD -- "$path" | awk '{print $3}')"

git submodule update --init --checkout -- "$path"
git -C "$path" fetch --no-tags origin main
main_head="$(git -C "$path" rev-parse FETCH_HEAD)"
git -C "$path" merge-base --is-ancestor "$expected" "$main_head"

gh api repos/manaflow-ai/bonsplit/pulls/197 \
  | jq -e '.state == "closed" and .merged_at != null and .base.ref == "main"'
🤖 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 `@vendor/bonsplit` at line 1, Do not merge this change while bonsplit PR `#197`
remains open. After it merges into main, repin the vendor/bonsplit gitlink to
the resulting main commit, or verify the current pinned commit is an ancestor of
origin/main using the provided validation flow.

@azooz2003-bit
azooz2003-bit merged commit 2141de5 into main Aug 3, 2026
6 checks passed
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