Stop main window drifting down on sleep/wake - #6305
Merged
austinywang merged 2 commits intoJun 19, 2026
Merged
austinywang merged 2 commits into
austinywang merged 2 commits into
Conversation
|
@sergej-koscejev is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesWindow Frame Constraining
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 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 |
Contributor
Author
|
The PR is 100 % vibe-coded by Claude Code (Opus 4.8), let me know if that's a problem. I have no experience with Swift whatsoever, I just was annoyed by the bug. Thanks! |
Contributor
Greptile SummaryOverrides
Confidence Score: 5/5Safe to merge — the change is a narrow, well-scoped override of a single AppKit callback with a clear fallback path and full test coverage. The override correctly names and enforces cmux's existing invariant (cmux owns window placement). The pure helper handles all boundary conditions: No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AppKit calls constrainFrameRect] --> B{shouldPreserveFrameDuringConstrain
proposedFrame, visibleFrames}
B -->|intersection >= 60pt both dims| C{Any screen qualifies?}
C -->|Yes| D[Return proposedFrame unchanged]
C -->|No| E[super.constrainFrameRect]
E --> F[AppKit rescues stranded window]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[AppKit calls constrainFrameRect] --> B{shouldPreserveFrameDuringConstrain
proposedFrame, visibleFrames}
B -->|intersection >= 60pt both dims| C{Any screen qualifies?}
C -->|Yes| D[Return proposedFrame unchanged]
C -->|No| E[super.constrainFrameRect]
E --> F[AppKit rescues stranded window]
Reviews (3): Last reviewed commit: "Stop AppKit re-constraining the main win..." | Re-trigger Greptile |
Contributor
Author
Huh? It does no such thing? |
Contributor
Author
Overlap with #2667Heads-up for reviewers: #2667 ("Fix window collapsing to sliver after disconnecting external displays") works in the same window-frame-on-display-reconfiguration machinery, so the two should be reconciled even though they don't conflict textually.
Flagging so whoever lands either PR knows |
sergej-koscejev
changed the title
After the Mac sleeps and wakes, AppKit re-runs its constrain pass over every window. Its default constrainFrameRect does not only clamp off-screen windows back into view — it also repositions windows that are already fully on-screen. CmuxMainWindow is .fullSizeContentView and disables AppKit window restoration (isRestorable = false), re-applying its saved frame only at startup, so nothing re-asserts the frame after wake and the reposition sticks and accumulates each cycle. This test pins the desired behavior: constraining an on-screen frame must leave it untouched (a titlebar-under-menu-bar frame is used as one easy, deterministic on-screen case). It fails today because the inherited NSWindow.constrainFrameRect moves the frame.
After a display/system sleep→wake, AppKit re-runs its constrain pass over every window, and its default constrainFrameRect repositions windows that are already fully on-screen — not just off-screen ones. The move is AppKit-internal: it is not a fixed titlebar-height nudge and is not limited to a titlebar sitting under the menu bar (it also hits e.g. a window in the bottom half of an external display), and it depends on the display arrangement and per-screen menu-bar/safe-area insets. Because cmux owns its own frames, disables AppKit window restoration, and re-applies the saved frame only at startup, nothing re-asserts it after wake, so the reposition sticks and accumulates each cycle. Override CmuxMainWindow.constrainFrameRect to leave an already-reachable frame untouched, deferring to AppKit's default only when the frame would otherwise be stranded off-screen (e.g. a display was disconnected) so a genuinely lost window can still be pulled back into view. cmux already owns and clamps its own placement, so deferring to AppKit's re-constrain here only caused the drift. Adds deterministic, screen-agnostic coverage of the reachability helper.
sergej-koscejev
force-pushed
the
fix-window-shift-lock-constrain
branch
from
June 17, 2026 11:13
db55f55 to
f6cbe00
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jun 26, 2026
austinywang
pushed a commit
that referenced
this pull request
Jun 26, 2026
When the app is in another app and the display sleeps/wakes (or the screen is reconfigured), a maximized or native-fullscreen main window can come back genuinely resized to the ~2/3 default (1000x700) frame. The existing constrainFrameRect override (#6305) only vetoes AppKit's re-constrain of an already-good frame; it cannot undo a real resize applied through another path (native-fullscreen exit, un-zoom revert, or a display-mode resize), so the shrunken frame sticks. A frame change while the app is inactive is never user-driven, so we snapshot each main window's frame on resignActive, arm a flag when a display sleep/wake or screen-parameter change is seen while inactive, and on the next activation restore any window that shrank to a still-reachable earlier frame. Native-fullscreen windows and frames whose display was unplugged are left untouched. Implements CmuxMainWindow.restoredFrameAfterInactiveDisplayTransition, turning the previous commit's regression test green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
austinywang
pushed a commit
that referenced
this pull request
Jun 26, 2026
When a maximized or native-fullscreen cmux window loses focus and the display sleeps/wakes (or is reconfigured) while the user is in another app, the window can come back genuinely resized to the ~2/3 default (1000x700) frame. The existing constrainFrameRect override (#6305) only vetoes AppKit's re-constrain of an already-good frame; it cannot undo a real resize applied through another path (native-fullscreen exit, an un-zoom revert, or a display-mode resize), so the shrunken frame sticks. A frame change while the app is inactive is never user-driven (the user is in another app), so we snapshot each CmuxMainWindow frame on resignActive and, on the next activation, restore any window that shrank to a still-reachable earlier frame. Windows still in native fullscreen and frames whose display was unplugged are left untouched. No flag is needed because an inactive resize is by definition not user-initiated — this also avoids the laptop race where the display wakes exactly as the user returns. Implements CmuxMainWindow.restoredFrameAfterInactiveDisplayTransition, turning the previous commit's regression test green. Refreshes the Swift file-length budget for AppDelegate.swift via scripts/swift_file_length_budget.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
austinywang
pushed a commit
that referenced
this pull request
Jun 26, 2026
When a maximized or native-fullscreen cmux window loses focus and the display sleeps/wakes (or is reconfigured) while the user is in another app, the window can come back genuinely resized to the ~2/3 default (1000x700) frame. The existing constrainFrameRect override (#6305) only vetoes AppKit's re-constrain of an already-good frame; it cannot undo a real resize applied through another path (native-fullscreen exit, an un-zoom revert, or a display-mode resize), so the shrunken frame sticks. New MainWindowFrameRestorer (in CmuxMainWindow.swift) owns the state and decisions: AppDelegate snapshots each main window frame on resignActive and, on the next activation, restores any window that shrank to a still-reachable earlier frame. The restore is gated on a real display transition (display/system sleep or a screen-parameter change) observed while the app was inactive, so a deliberate background resize by a window manager, AppleScript, or macOS window management is left untouched. Arming on the sleep side sets the flag while the user is still away, avoiding the laptop race where the display wakes as the user returns. Native-fullscreen and unplugged-display frames are left alone. Implements CmuxMainWindow.restoredFrameAfterInactiveDisplayTransition, turning the previous commit's regression test green, and adds MainWindowFrameRestorer gating tests. Refreshes the Swift file-length budget for AppDelegate.swift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mxschmitt
added a commit
to mxschmitt/cmux
that referenced
this pull request
Jul 6, 2026
When an external monitor positioned above the built-in display is disconnected, the cmux main window can be left with its body's bottom edge dipping into the built-in display while its titlebar sits far above the only remaining screen — off the top, unreachable. Because the window is non-movable (isMovable=false) and can only be dragged by its titlebar handle (WindowDragHandleView), the user cannot pull it back down. This is the same family as manaflow-ai#2824 / manaflow-ai#2135 / manaflow-ai#1620. The runtime defense against this is CmuxMainWindow.constrainFrameRect (added in manaflow-ai#6305 to stop sleep/wake drift), which vetoes AppKit's corrective re-constrain whenever shouldPreserveFrameDuringConstrain returns true. That predicate only checks for a 60x60pt overlap with ANY screen in both dimensions — it has no requirement that the titlebar / top of the window be reachable. So a window whose bottom 60pt overlaps the built-in display but whose titlebar is hundreds of points above it is preserved unchanged, and AppKit's clamp (which would rescue it) is refused. This test pins the desired behavior: a frame whose titlebar is stranded above the only screen must NOT be preserved. It fails today, reproducing the bug deterministically with synthetic visibleFrames (no display hardware needed).
mxschmitt
added a commit
to mxschmitt/cmux
that referenced
this pull request
Jul 6, 2026
Strengthen the runtime constrain veto so it only preserves a frame whose titlebar (top strip) remains reachable, rather than any frame with a 60x60pt overlap somewhere on screen. The manaflow-ai#6305 override (constrainFrameRect) was added to stop the main window drifting on sleep/wake by refusing AppKit's re-constrain of an already-reachable frame. But its reachability test only required a 60x60 overlap with any screen in both dimensions, with no titlebar requirement. When an external monitor positioned above the built-in display is disconnected, the window can be left with only its lower body overlapping the built-in display while its titlebar sits far above the only remaining screen. The lax predicate counted that as "reachable" and vetoed AppKit's corrective clamp, so the window stayed stranded above the screen — and because the window is non-movable and only draggable by its (now off-screen) titlebar handle, the user could not pull it back down. Require a grabbable slice of the top strip (120x24pt of a 64pt-tall titlebar band) to be on a visible frame, mirroring the restore-path test AppDelegate.shouldPreserveAccessibleFrame. A titlebar-under-menu-bar frame and a fully on-screen frame still qualify (so the sleep/wake drift fix is preserved), but a frame whose titlebar is above every screen now defers to AppKit's clamp, which pulls it back into view.
mxschmitt
added a commit
to mxschmitt/cmux
that referenced
this pull request
Jul 6, 2026
The manaflow-ai#6305 constrainFrameRect override cannot fix a window stranded by a monitor disconnect: cmux main windows set isMovable = false for their custom titlebar drag handling, and AppKit excludes a non-movable NSWindow from its automatic on-screen constraining when displays change. So constrainFrameRect is never invoked on that path, and nothing pulls the window back — when an external monitor positioned above the built-in display is disconnected (or the lid is reopened), the window keeps its titlebar in the now-gone monitor's coordinate space, above every remaining screen and unreachable. Because the only drag affordance is that off-screen titlebar, the user cannot recover it. Add a reactive reconcile: observe didChangeScreenParametersNotification (coalesced with a short settle delay) and, for each main window whose titlebar is no longer reachable, clamp it back onto the display it most overlaps so a grabbable slice of the titlebar returns on-screen. Windows that already fit are left untouched, so displays the reconfiguration did not affect are undisturbed. The decision is a pure, nonisolated reconciledFrameAfterScreenChange that reuses the existing shouldPreserveAccessibleFrame / clampFrame helpers and is unit-tested without live NSScreens (stranded → pulled back, reachable → nil, no displays → nil). A DEBUG cmuxDebugLog records each correction.
mxschmitt
added a commit
to mxschmitt/cmux
that referenced
this pull request
Jul 6, 2026
…red reachability Review follow-ups on the display-reconfiguration reconcile: - Replace DispatchQueue.main.asyncAfter with a cancellable Task + Task.sleep (cmux-architecture bans asyncAfter in new code, and using a delay to let state "settle" is called out specifically). The task is cancelled on each new screen-change event and on teardown, so it can never fire against a half-torn-down app; collapses the notification burst into one pass. - Skip native-fullscreen windows in the reconcile: an NSWindow in a fullscreen Space is owned by AppKit's Space machinery, and calling setFrame on it mid-transition (e.g. its display was just disconnected) fights the fullscreen teardown. - Gate the reconcile on !isApplyingSessionRestore and !isTerminatingApp, like the sibling lifecycle handlers, so it never races the restore path's deliberate setFrame nor persists a frame clamped against transient mid-teardown display geometry. - Extract one shared CmuxMainWindow.isTitlebarReachable predicate used by both the runtime constrain veto and the restore-time clamp (shouldPreserveAccessibleFrame), removing the duplicated 120/64/24 top-strip math that was "kept in sync" by a comment. Retune the thresholds to 60pt width / 16pt height so a window parked at a side edge (60-119pt of titlebar visible) and a window flush to the top of a large-menu-bar / notch display are still preserved — the old 120/24 values would have re-introduced the manaflow-ai#6305 sleep/wake drift for those. Adds regression tests for both. - Drop the debug-only reconcile "source" plumbing (a stored property + method param used only in a #if DEBUG log of a constant string).
Closed
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After the Mac sleeps and wakes (e.g. you lock with
Ctrl+Cmd+Qand come back later, or the display sleeps), the cmux main window shifts downward, and it accumulates across sleep/wake cycles. The lock keystroke itself is not the trigger — the sleep→wake is.Cause
This is AppKit re-constraining the window, not cmux repositioning it. None of cmux's own observers move the window here (
sessionDidResignActiveonly saves a snapshot;didWakeonly restarts the socket listener;applicationDidBecomeActiveonly re-places windows when none is visible).On a display/system sleep→wake, AppKit re-runs its constrain pass (
constrainFrameRect(_:to:)) over every window. The default implementation does not only clamp off-screen windows back into view — it also repositions windows that are already fully on-screen, which is what shows up as the per-cycle creep. The exact reposition is AppKit-internal and depends on the display arrangement and each screen's menu-bar / safe-area insets, so:The main window is
.fullSizeContentView, and cmux disables OS window restoration (isRestorable = false), re-applying its saved frame only at startup. Because nothing re-asserts the saved frame after wake, whatever AppKit's re-constrain produced sticks and accumulates.Fix
Override
CmuxMainWindow.constrainFrameRect(_:to:)to leave an already-reachable frame untouched, only falling back to AppKit's default constraining when the proposed frame would otherwise be stranded off-screen (e.g. a display was disconnected), so a lost window can still be pulled back into view. cmux already owns and clamps its own window placement, so deferring to AppKit's re-constrain here only produced the drift. The reachability decision is factored into a pure, testable helper.Tests (two-commit red/green)
testConstrainPreservesOnScreenFrameOverlappingMenuBarasserts that constraining an on-screen frame leaves it untouched (a titlebar-under-menu-bar frame is used as one easy, deterministic on-screen case). Verified red-by-assertion without the fix (inheritedNSWindow.constrainFrameRectmoves it).shouldPreserveFrameDuringConstrainhelper (inside / menu-bar-overlap / stranded / barely-peeking / no-screens).A third commit corrects code/test comments that had mis-stated the cause. All 6 tests pass green locally on macOS 26.5.1 (M2 Max). Test file is wired into
project.pbxprojand passeslint-pbxproj-test-wiring.sh.Verification
Reproduced and confirmed fixed on the reporter's macOS (26.5.1 Tahoe) via a tagged Debug build and a real sleep/wake cycle — the window now stays put with no cumulative creep.
See also the note below on overlap with #2667.
Summary by CodeRabbit
Bug Fixes
Tests