[lawrencecchen]

Two Move to New Window actions from one window opened both new windows at the same frame, so the second one hid the first completely. The app then paused the hidden window's terminal, and after a restart the session showed one window where the person had two (v2.2.6-opt gate journey mw-restart-three-windows: both windows at [438, 242, 1100, 720]).

MovedWindowPlacement placed the new window from the source frame alone (beside it, else cascaded 28 pt), so every move from the same window got the same answer. frame(beside:on:avoiding:) now takes the frames of the windows already open. From a spot a window already takes (same top-left), it steps on by the cascade step, down and right, as AppKit's cascadeTopLeft(from:) does. A step that would leave the screen goes back to the screen's top-left only when that spot is free; otherwise it steps on, partly off screen. A zoomed or near-full source has no free spot on its screen, and a window partly off screen is better than one hidden under another (review finding on dc8712d). WindowManager.openMovedWindow passes every controller's window frame, including a window a move just opened that is not ordered in yet. Chrome offsets each new window from the last one in the same way (WindowSizer, the GetSavedWindowBounds cascade).

Ported from cp/fix-moved-window-cascade (perf train 321: c49bca9, 27a3b5d, d899ecc), plus the zoomed-source fix found in review. On feat the bug is present: openMovedWindow called frame(beside:on:) with no other windows. The App change is net zero lines.

Testing

• MoveToNewWindowTests.twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther runs the real action (moveWorkspaceToNewWindow) twice from one window on a same-tree daemon and checks that no two windows share a frame origin.
• MoveToNewWindowTests.threeMovesFromOneWindowEachGetASpotOfTheirOwn calls the placement three times from a zoomed source, a near-full source and the gate's 1100x720 source, and checks that no two windows share a top-left.

Fleet jobs (package-test-lane.sh suite Packages/macOS/CmuxNext MoveToNewWindowTests):

• Compile (Linux, CmuxNext + app + cmux-tui, recipe v2) at the head 1079922: 22fd224f6c49ba07065294e7, ok. Its log shows the changed files compiled.
• Red, test commit 191c268 alone on feat: e5203dcf8f07b53eb1f87a3f on aws-m4pro-3. FAILED as expected: twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther at MoveToNewWindowTests.swift:162, (origins.count → 2) == (frames.count → 3); the other 7 tests passed.
• Red, zoomed-source test at 7b9cf7e: 0095c9ba236049b6ffc95432 on aws-m4pro-1. FAILED as expected at MoveToNewWindowTests.swift:190: zoomed source (spots.count → 2) == (open.count → 4), near-full source (3) == (4); the 1100x720 case and the other 8 tests passed.
• Green, head 1079922: ec225222ef8848ba784d7bac on aws-m4pro-6. PASSED: "Test run with 9 tests in 1 suite passed", both new tests included (the zoomed test with its 3 cases).

Observed / Not run

• Observed: the compile, both reds and the green above. A Python model of the placement (review) puts every window at its own top-left for zoomed, near-full and 1100x720 sources, and in 20,000 random cases.
• Not run: the live mw-restart-three-windows journey on a feat build.

Changelog

Fixed: Moving two workspaces to new windows from the same window no longer opens the second window exactly on top of the first

Proof

No capture: window placement only; the regression tests check the frames.

[lawrencecchen]

Review (claude-review-subagent): FIX at dc8712d

Read-only review of cba94d5..dc8712d (3 files). Nothing compiled or run on the review machine.

Correctness

1. Packages/macOS/CmuxNext/Sources/CmuxNextLayout/Geometry/MovedWindowPlacement.swift:52 (the wrap): the fix does not work for a zoomed or near-full-screen source window. The second (or third) move still opens exactly on the first.
   When the source has no room for any of the four cascades, frame(beside:on:) returns source + (28, -28). The wrap target is (screen.minX + 28, screen.maxY - h - 28). When the source fills the visible frame, that wrap target is the same spot as the first moved window. And when w > screen.width - 56 or h > screen.height - 56, a step from the wrap target leaves the screen again and wraps back to the same spot. So the loop goes around on one taken spot until occupied.count runs out, and then returns that taken spot.
   I ran the same arithmetic in a Python model (visible frame 1512x944, 28 pt step, 12 pt gap):

   • Source zoomed to the visible frame (0,0,1512,944): move 1 gives (28,-28), and move 2 gives (28,-28) again. Same frame, which is the reported bug.
   • Source (20,20,1480,900): move 1 gives (48,-8), move 2 gives (28,16), and move 3 gives (28,16) again. Same frame.
   • The gate case (a 1100x720 window) is fixed: (438,242), (466,214), (494,186).
     A window zoomed to fill the screen is a common setup, so the doc promise ("never exactly on a window already open") and the PR intent do not hold there. A suggested repair: when the wrapped spot equals the current candidate, or is itself taken, step on from it anyway (for example, move the wrap column by k*step, or offset from the taken frame without the screen check). A frame that hangs 28 pt off the screen is better than one that hides a window completely. Also add a pure unit test next to theNewWindowOpensBesideTheOldOne that places three windows for source == screen and for a near-full source, and expects three different origins. That test needs no app or daemon, and it fails at this head.

2. The regression test does not cover the failing shape. twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther uses the default 1100x720 window (WindowController.swift:94), so the wrap path never runs. The test itself is sound for what it checks:

   • On the base it fails: both moves get one frame from the source alone, so 3 frames give 2 origins.
   • On the head it should pass, because every controller's frame is passed now, including the one not ordered in yet.
   • The controllers list drops closed windows (WindowManager.swift:482), so closed windows do not count.
   • The source stays non-whole after move 1, because the launch workspace and second remain in it.

Checked and OK

• Call site (WindowManager+Membership.swift:341). It passes the source's own frame and windows on other screens.
  • The first candidate is never at the source's top-left, because every branch of frame(beside:on:) offsets it.
  • Frames on other screens are in global coordinates, so they cannot match a spot on this screen. They are harmless, but the occupied doc comment ("on that screen") does not match what the caller passes.
  • In the whole-window branch, the source moves to a spot that avoids the other windows. Its own old frame in the list is harmless.
• Off-screen results. When the source is too big, frame(beside:on:) can return a frame partly off the screen. The base did the same, so this PR does not add that behavior.
• Loop bound. The loop runs at most n steps over n frames, and the work is O(n^2) on a few windows. Nothing new blocks the action's turn.
• Repository rules.
  • The red test is committed before the fix (191c268).
  • The PR has risk:normal, a Changelog line and an Observed / Not run section.
  • No user-facing strings, animations or CI gates are added.

Evidence state

• Linux compile b18ceaac17593aa5a5eb4117: passed, as the PR reports. I did not re-check it.
• Mac red e5203dcf8f07b53eb1f87a3f and green ffe8cc7a650ef16e5211586b: both were still queued when I checked (no worker free). Neither has a result at this head yet.

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager+Membership.swift">

<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager+Membership.swift:341">
P2: The occupancy check in `MovedWindowPlacement.frame(beside:on:avoiding:)` matches on top edge (`maxY`), not origin. An open window with the same origin but a different height is not treated as occupied, so the new window can land on its exact origin. Compare `minY` instead of `maxY`, or check both edges, so the placement matches the origin the regression test asserts.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The occupancy check in MovedWindowPlacement.frame(beside:on:avoiding:) matches on top edge (maxY), not origin. An open window with the same origin but a different height is not treated as occupied, so the new window can land on its exact origin. Compare minY instead of maxY, or check both edges, so the placement matches the origin the regression test asserts.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager+Membership.swift, line 341:

<comment>The occupancy check in `MovedWindowPlacement.frame(beside:on:avoiding:)` matches on top edge (`maxY`), not origin. An open window with the same origin but a different height is not treated as occupied, so the new window can land on its exact origin. Compare `minY` instead of `maxY`, or check both edges, so the placement matches the origin the regression test asserts.</comment>

<file context>
@@ -332,13 +332,13 @@ extension WindowManager {
     func openMovedWindow(_ ids: [String], from origin: MoveOrigin, select: Bool) {
         let source = origin.window.flatMap(controller(for:))
-        let frame = source?.window.map { MovedWindowPlacement().frame(beside: $0.frame, on: ($0.screen ?? NSScreen.main)?.visibleFrame ?? .infinite) }
+        let frame = source?.window.map { MovedWindowPlacement().frame(beside: $0.frame, on: ($0.screen ?? NSScreen.main)?.visibleFrame ?? .infinite, avoiding: controllers.compactMap { $0.window?.frame }) }
         let whole = source.map { Set(ids) == Set(registry.members(of: $0.state.id)) } == true
         if whole, let source, let frame { source.window?.setFrame(frame, display: true); recordSaver.stateDidChange(source.state) }
</file context>

[lawrencecchen]

Review (claude-review-subagent-2): SHIP at 1079922

Scope: the whole PR, git diff cba94d55a6a4 107992259937 (5 commits, 3 files). I read the code only. I compiled nothing and ran no Swift. I modeled the placement arithmetic in Python.

Correctness: no blocking defect found.

• MovedWindowPlacement.swift:50-66 frame(beside:on:avoiding:). The loop always ends on a free spot. A wrap is taken only when taken(wrapped) is false, so the next turn breaks at once. A wrap therefore always ends the loop, and every other move is a strict +28/-28 diagonal step, so no spot repeats. The n checked spots are distinct taken spots (28 pt apart, so one occupied frame cannot match two of them with the 1 pt tolerance). By pigeonhole they use all n occupied frames, so the last candidate, which is not checked, is free. A wrap cannot pick a spot that a later step in the same call lands on, because a wrap ends the call. In a later call, a step can reach the wrapped spot W. W is then taken, the loop steps on, and the count still holds. Screens not at (0,0) are handled: W uses screen.minX and screen.maxY, and .infinite contains every finite candidate. AppKit y grows up: the step is (+28, -28), which is down and right, and the top-left is compared as (minX, maxY).
• Python model (/work/tmp/perfx-a11f/model.py) on a 1512x944 screen:
  • Zoomed source (0,0,1512,944): red code gives (28,-28) three times. Head gives (28,-28), (56,-56), (84,-84).
  • Near-full source (20,20,1480,900): red code gives (48,-8), (28,16), (28,16). Head gives (48,-8), (28,16), (76,-36).
  • The gate's 1100x720 source: same result at red and head.
  • Fuzz test: 20,000 random screens (also at negative origins and not at origin 0,0), source sizes (full, near-full, larger than the screen, partly off screen), extra windows, and 1 to 12 moves each. It never returned a taken spot.
• WindowManager+Membership.swift:341. The call site passes the frames of all controllers, and this includes the source. frame(beside:on:) never returns the source frame, so the whole-window branch is correct. A window just opened is in controllers (makeController appends to it before presentation), and openWindow calls setFrame(frame) after makeController, so the restore clamp WindowPlacementFallback.visible does not move it back. All entry points (WindowMembershipHandlers.swift:16, WorkspaceStructureHandlers.swift:195,202, WindowManager+Incognito.swift:210) go through openMovedWindow, and this includes the batch loop at :202.
• Tests: threeMovesFromOneWindowEachGetASpotOfTheirOwn fails at 7b9cf7e for the first two arguments and passes at the head for all three (model above). twoMovesFromOneWindowOpenTwoWindowsThatDoNotCoverEachOther must fail at 191c268: there, openMovedWindow called frame(beside:on:) with the same source frame two times, so it got the same frame two times.

Not blocking. Do these before ready-to-land:

1. The PR body is out of date. It names dc8712d as the head. It describes the old wrap ("back to the screen's top-left when it would leave the screen ... stops after as many steps as there are windows"). It does not name the new test or its red job 0095c9ba236049b6ffc95432. The red result and the green result both say "RESULT PENDING". CLAUDE.md requires the body to give the failure on the base and the pass on the head. Write the job results in the body when they are available.
2. No Mac test result exists yet. At review time, e5203dcf8f07b53eb1f87a3f was running on aws-m4pro-3, and 0095c9ba236049b6ffc95432 and 68c7f8ac091d60cc7f27c878 were queued. This SHIP rests on code reading, the Python model and the Linux compile 22fd224f6c49ba07065294e7. A red result caused by the PR on the green job removes this SHIP.

Notes (no change required):

• MovedWindowPlacement.swift:63: the wrapped spot can be a few points from a window that is open. For the near-full source, (28,16) is 8 pt right of the source (20,20) and its top is 4 pt lower. The top-left is different, so the stated intent holds, but the source window is almost fully covered. A wider "taken" tolerance (for example, less than one step) would prevent this. That is a design choice, not a defect.
• MoveToNewWindowTests.swift:185: the pure geometry test is in a suite with .enabled(if: SameTreeDaemonBinary.url != nil). Where the daemon binary is missing, this test is skipped, but it does not need the daemon.

[github-actions]

cmux status _{(one comment, edited in place; each line is updated by the job that owns it)}

Review 1079922: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr30011 -f ref=perfx/moved-window-cascade -f platform=macos -f backend_mode=local

Not verified: macOS jobs did not run: compile and app tests were skipped on 1079922599; ci-status is green without them.

CI passes on 1079922599 (run 41266362714 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

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; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

Shipped in:

• cmux NEXT nightly 1.0.0-nightly.4130052256501 (28aa638015)

[blacksmith-sh]

Found 8 test failures on Blacksmith runners:

Failures

Test	View Logs
/tmp/tmp2sj12okf/summary-missing.json (live):/summary-missing.json (live):	View Logs
/tmp/tmpx5o0cvvu/summary-fail.json (live):/summary-fail.json (live):	View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/ [new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows	View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames	View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/ [tabs] debug.tab_switch timed 0 of 40 switches	View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames	View Logs
convention violations found (ERROR lines above)./ convention violations found (ERROR lines above).	View Logs
test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)/ test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)	View Logs

^{Need help on this PR? Tag @codesmith-bot with what you need.}