Repository navigation
fix: keep split zoom when the zoomed pane outlives a tab close - #12853
teamleaderleo merged 3 commits into
Conversation
Closing the selected tab of a zoomed pane currently clears the split zoom even when the pane survives the close, so the window snaps back to the split layout. Cover both halves of the behavior: the zoom must survive a close that leaves tabs behind, and must still end when the close takes the zoomed pane with it. Fails without the fix in the following commit. Issue: manaflow-ai#8363
|
To use Codex here, create a Codex account and connect to github. |
|
All contributors have signed the CLA ✍️ ✅ |
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesSplit zoom tab-close behavior
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Split zoom now remains active when tabs survive and clears when the zoomed pane is removed. The covered change is ready to merge. 🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I have read the CLA Document v2.2 and I hereby sign the CLA |
recordPostCloseState() marked the zoom for clearing whenever the closed tab was the selected tab of the zoomed pane, regardless of how many tabs that pane had. Closing one of several tabs therefore dropped the zoom and snapped the window back to the split, even though the zoomed pane was still there with another tab selected. Mark the zoom for clearing only when the closed tab is the pane's last one, which is the case where the pane itself goes away. Closing the last tab still ends the zoom, so the surviving pane is never left hidden. This covers the closing half of the issue only; opening a tab with Cmd+T still clears the zoom through a separate multi-entrypoint path, so the issue stays open. Issue: manaflow-ai#8363
b5a314a to
65752ee
Compare
# Conflicts: # cmux.xcodeproj/project.pbxproj
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Merged, thank you @kugesh-Rajasekaran! A zoomed pane now stays zoomed when you close one of its tabs. I merged main in and sorted out the project file conflict for you :D |
|
Merge receipt for |
c055747 ci: parse runner expressions in the fork guard, and gate LINUX_RUNNER on fork PRs (manaflow-ai#14192) fc112a8 docs(testing): describe how run-e2e.sh actually picks the runner (manaflow-ai#14620) 0f6edfa ci(owned): keep seeds until the disk is actually short, not at a fixed 6 (manaflow-ai#14621) 1b78097 CI: one owned-pool rescue sweeper instead of a rescue run per CI run (manaflow-ai#14602) c7e79f6 Hold Files tree reloads while its context menu is open (manaflow-ai#14451) dbb24cb ci: read the owned-pool rescue's Actions API through the route App (manaflow-ai#14499) 36b8063 ci: store each app-host product file once in the product archive (manaflow-ai#14601) 0dba677 CI: run CmuxWorkspaces package tests (manaflow-ai#14592) 2244e98 Fix color detection in native tmux mirrors (manaflow-ai#14175) 3d2478e build: keep every built file in a project group so Xcode reuses its build description (manaflow-ai#14486) a1a5ae9 ci(cmux-tui): cache cargo builds in the Rust jobs (manaflow-ai#14613) 560e640 ci(owned): keep 8 seeds per mini and pick seeds by cost, not a 2-commit cap (manaflow-ai#14607) 561d317 cmux-debug-cli: find a publish-hq build in the HQ tag app cache (manaflow-ai#14579) c0aaac7 fix(events): survive receive-timeout reconfiguration churn during replay (manaflow-ai#13888) 3bd994a fix: keep split zoom when the zoomed pane outlives a tab close (manaflow-ai#12853) ca984c7 Session snapshots can send binding actions to ghostty on a surface that is no longer live (manaflow-ai#12623) a47d65b settings: expose local tmux session persistence (manaflow-ai#13210) e3acb25 ci: give an owned Mac's full app rebuild 35 minutes to compile (manaflow-ai#14594) # Conflicts: # .github/workflows/ci-artifact-transport.yml # .github/workflows/ci-cache-receipts.yml # .github/workflows/ci-guards.yml # .github/workflows/ci-macos.yml # .github/workflows/ci-owned-pool-rescue.yml # .github/workflows/ci-web.yml # .github/workflows/ci.yml # .github/workflows/cloud-machine-tests.yml # .github/workflows/cloud-vm-guest-install.yml # .github/workflows/cloud-vm-image-contract.yml # .github/workflows/cloud-vm-image-reachability.yml # .github/workflows/cloudflare-relay.yml # .github/workflows/cmux-skill-contract.yml # .github/workflows/cmux-tui-sdks.yml # .github/workflows/cmux-tui-spec.yml # .github/workflows/cmux-tui.yml # .github/workflows/indexnow-tests.yml # .github/workflows/iroh-v2.yml # .github/workflows/localization-catalog.yml # .github/workflows/r2-upload-tests.yml # .github/workflows/remote-daemon.yml # .github/workflows/repair-nightly-appcast-content-types.yml # .github/workflows/required-checks-drift.yml # .github/workflows/resolve-dispatch-ref.yml # .github/workflows/seed-derived-data.yml # .github/workflows/terminal-hang-diagnostics.yml # .github/workflows/test-e2e.yml # .github/workflows/test-ios.yml # .github/workflows/testbox-broker-guard.yml # .github/workflows/web-validation.yml
…14664) #12853 made a zoomed pane that still has tabs stay zoomed after its selected tab closes (#8363) and added WorkspaceSplitZoomTabCloseTests for it, but the older TabManager test still asserted that the close cleared the zoom, so it fails on main. Keep its focus assertion and expect the new zoom behavior. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
What changed?
Workspace.recordPostCloseState()now marks the split zoom for clearing only when the tab being closed is the last tab in the zoomed pane.Why? Closing a tab inside a zoomed pane dropped the zoom and snapped the window back to the split layout, even though the pane was still there with another tab selected.
The close path is
splitTabBar(_:shouldCloseTab:inPane:)→recordPostCloseState()→postCloseClearSplitZoomTabIds→splitTabBar(_:didCloseTab:fromPane:)→clearSplitZoom(). The mark was recorded from two conditions only:"the closed tab is the selected tab of the zoomed pane" is true for every close in that pane, not just the one that removes the pane.
recordPostCloseState()runs before Bonsplit removes the tab, so the pane's tab count is still authoritative there — addingtabs.count <= 1narrows the mark to the case the clear was meant for.Repro before this change:
Closing the last tab of the zoomed pane still ends the zoom, so the surviving pane is never left hidden behind a zoom pointing at a pane that no longer exists.
Scope note: this is the closing half of #8363. The tab-bar
+button already preserves the zoom (it routes throughdidRequestNewTab, which never clears it), butCmd+Tstill clears it viaTabManager.newSurface(). That opening half touches a different, multi-entrypoint path (~20 call sites deliberately clear the zoom), so per the shared-behavior policy it belongs in its own change rather than being bolted onto this one. Leaving the issue open for it.Testing
Two commits, per the regression-test policy in
AGENTS.md/skills/cmux-testing:test: keep split zoom when a zoomed pane still has tabs— the test alone, expected red.fix: keep split zoom when the zoomed pane outlives a tab close— expected green.New file
cmuxTests/WorkspaceSplitZoomTabCloseTests.swift(Swift Testing,@MainActor,.serialized), wired intocmux.xcodeproj/project.pbxprojwith all four entries:closingSelectedTabKeepsZoomWhilePaneStillHasTabs— split into two panes, add a second tab to pane A, zoom pane A, close the selected tab viaworkspace.closePanel(_:force:)(the realBonsplitController.closeTabpath, not a hand-called delegate), assert the pane still has one tab andzoomedPaneIdis unchanged. This is the assertion that fails without the fix.closingLastTabInZoomedPaneEndsZoom— guards the other direction: closing the pane's only tab still clearszoomedPaneId, and the sibling panel survives.Both drive observable runtime behavior through the executable close path; no source-text or project-metadata assertions.
Verified locally:
What I could not run locally, and why. This machine is macOS 15.7.5 / Xcode 16.2 (Swift 6.0.3), not the pinned Xcode 26.
scripts/setup.sh+ the prebuiltGhosttyKit.xcframeworkfetch both succeed, but./scripts/reload.sh --tag issue-8363fails on currentmainbefore it reaches any of my code, on Swift 6.2-only semantics in packages the app links:(Both are pre-existing on
mainand untouched by this PR — flagging them only to explain the gap, and becauseAGENTS.mdstill describes Xcode 16.2 as a best-effort pathway for the macOS app.)So I have no locally built app and therefore no demo video. The red→green transition across the two commits on CI is the behavioral proof here, and I'll attach the CI evidence as a comment once the checks report. Happy to add a recorded demo if a maintainer can point me at a build lane I can use.
Demo Video
Not attached — see the note above: the pinned Xcode 26 toolchain is unavailable on my machine, so I could not produce a running build to record. The two-commit CI red/green covers the behavior instead.
Review Trigger (Copy/Paste as PR comment)
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes closing a tab in a zoomed pane dropping the zoom, so the window no longer snaps back to the split layout while the pane still has other tabs.
recordPostCloseState()now clears the zoom only when the closed tab is the pane's last one; closing that last tab still ends the zoom as before.Covers the closing half of #8363. The tab-bar
+button already preserved zoom, butCmd+Tstill clears it viaTabManager.newSurface(); that opening half stays open in issue #8363.Testing
WorkspaceSplitZoomTabCloseTestscovering both directions: zoom survives a close that leaves tabs, and ends when the pane's last tab closes.Written for commit 49fd3ed. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests