Keep the main window floor on the animating setFrame path - #15368
Conversation
setFrame(_:display:animate:) does not route through setFrame(_:display:), so it could size the main window below the 400 pt layout floor. The content could not shrink with it, overflowed the window, and a split pane laid out past the bottom edge ended at 0x0. Fixes #15347 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 42 seconds. 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)
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 |
|
All contributors have signed the CLA ✍️ ✅ |
|
Dogfood build of cmux DEV pr-15368-781dd465.app The link opens this exact commit in the cmux dev menu bar app. The build starts on each push and the page waits until it is ready; a newer push replaces it. It signs in against production, so Cloud or backend changes still need a tagged build with a development backend. |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CI failure attributionCI passes on Written by |
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
|
|
Merge receipt for |
1028a08 test: isolate background workspace git probe fixture (manaflow-ai#15388) 41ad40d fix: keep the terminal area when the window is too narrow for the side panels (manaflow-ai#15369) 2890f0b Roll the Base create back when the owner network resolve fails (manaflow-ai#15358) 7b0a15f Keep agent- and script-opened workspaces and panes in the background (manaflow-ai#15281) 4f14fa3 ci: move CLI regressions to CLI product tests and rebalance the seven app-host shards (manaflow-ai#15177) 906926a ci: dogfood builds are opt-in with the dev-build label (manaflow-ai#15380) 2f6716c PR media: classify app changes by CI's build inputs; a reuse error is no refusal (manaflow-ai#15386) bc28bc4 Release the Base generation when a create is refused for credits (manaflow-ai#15343) 6760c93 iOS: Add Computer never disturbs the active Mac (manaflow-ai#15102) 0f2d3d3 Show Claude sessions that stop on an API error instead of leaving them Running (manaflow-ai#15232) 20ef7c9 Keep the main window floor on the animating setFrame path (manaflow-ai#15368) b4f5dc5 ci: move UI runs pinned to Blacksmith macOS 26 onto owned Macs (manaflow-ai#15383) e02c385 PR media: compile once when CI's build cannot load, and say why a tour skipped (manaflow-ai#15378) ebd1f4f fix(iroh-v2): commit delivery accounting only after the frame is sent (manaflow-ai#15344) # Conflicts: # .github/workflows/ci-guards.yml # .github/workflows/ci-macos.yml # .github/workflows/ci.yml # .github/workflows/pr-media.yml # .github/workflows/test-e2e.yml
Summary
Split down, split up, then resize the window to 800x200: one of the three stacked panes ended at 0x0 (#15347).
The main window has a 400 pt minimum height, and
CmuxMainWindowenforces it by overridingsetFrame(_:display:).setFrame(_:display:animate:)is a separate entry point that does not route through that override, so display placement, UI-test placement and the debug resize verb could size the window below the floor. The workspace content cannot shrink past its own 400 pt minimum, so it overflowed the window: the pane area stayed 372 pt tall inside a 200 pt window, and the bottom pane was laid out entirely below the window edge, where the terminal portal collapses it to 0x0.The fix is to apply the same size policy (cap to the display union, raise to the floor, leave full screen alone) on the animating path too. The window now stops at 800x400, the pane area fits, and each of the three panes keeps a real height.
Pane-level clamping was the other option. bonsplit already keeps both children of every split nonzero within its container; the zero came from the container being taller than the window, so no pane minimum could fix it. Keeping the window at or above its content minimum removes the whole class of overflow, not just this split shape.
Testing
MainWindowSelfSizingTests.testAnimatingSetFrameRaisesUndersizedFrameToMinimumContentSize: an 800x200setFrame(_:display:animate:)must keep the width, raise the height to the floor and keep the top edge.dogfood/fuzz/regressions/issue-15347-stacked-panes-short-window.json, soscripts/fuzz regressions --app ...replays it.pane-view-degenerateat step 3; this branch passes (frames below).cmuxTests/MainWindowSelfSizingTests(withAppDelegateFullScreenFrameRestoreTestsandMainWindowZoomPlacementTests) in the app-host changed-suites job on this head: passed. The one red guard test,test_ci_owned_spm_scratch, comes from ci: bound the SwiftPM scratch holder and cache scratch sizes #15366 on main and is unrelated to this change.Changelog
Fixed: Resizing the main window from a script or a display change can no longer shrink it below its minimum height and leave a split pane with no size
Demo Video
The fuzzer repro (split down, split up, resize to 800x200) replayed on a build Mac against a DEV build of main and one of this branch, with a frame after each step.
Main, after the resize: the window is 200 pt tall, the pane area still lays out at its 400 pt minimum and overflows, and the third pane is off the bottom edge at 0x0 (the replay reports
pane-view-degenerate).This branch, after the same resize: the window stops at 800x400 and all three panes keep a real height (the replay passes).
Before the resize, for reference (this branch):
🤖 Generated with Claude Code