Repository navigation
Fix main window refit after display disconnect - #9698
Closed
austinywang wants to merge 5 commits into
Closed
austinywang wants to merge 5 commits into
austinywang wants to merge 5 commits into
Conversation
When a monitor is unplugged, main windows could remain stranded off-screen because constrainFrameRect only runs on AppKit's own schedule. Settings already recovers off-screen frames on open (#5770); mirror that for main windows by listening for didChangeScreenParameters and clamping any window whose frame is no longer grabbable onto a connected display.
Move target-screen selection and frame clamping into MultiMonitorWindowGeometry so main-window recovery no longer depends on SettingsWindowPresenter. Unify off-screen recovery through recoveredFrame for both constrainFrameRect and display-disconnect paths, narrow applyOffscreenRecoveryIfNeeded to CmuxMainWindow, and derive a guaranteed off-screen test frame from connected display bounds.
Use frameRect(forContentRect:styleMask:) so recoveredOffscreenFrame clamps against the true frame minimum (including title bar chrome) instead of treating content-size mins as frame dimensions.
📝 WalkthroughWalkthroughThe PR adds shared multi-monitor frame selection and clamping. Main windows recover after display changes, while settings windows reuse the shared geometry. Regression tests and a DEBUG-only socket method cover recovery behavior. ChangesWindow geometry recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant NSApplication
participant AppDelegate
participant CmuxMainWindow
participant MultiMonitorWindowGeometry
NSApplication->>AppDelegate: didChangeScreenParametersNotification
AppDelegate->>CmuxMainWindow: applyOffscreenRecoveryIfNeeded
CmuxMainWindow->>MultiMonitorWindowGeometry: request recoveredFrame
MultiMonitorWindowGeometry-->>CmuxMainWindow: return clamped frame
CmuxMainWindow-->>AppDelegate: apply frame
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/App/MultiMonitorWindowGeometry.swift`:
- Around line 6-96: Convert MultiMonitorWindowGeometry from an enum with static
APIs into a constructable type that stores screens, fallbackVisibleFrame,
mouseLocation, and inset through its initializer. Make recoveredFrame an
instance method using that stored state, and convert targetVisibleFrame and
clampedFrame to instance or private helper methods as appropriate; retain static
only for private helpers if needed.
In `@Sources/App/SettingsWindowGeometry.swift`:
- Around line 14-17: Update the minimumFrameSize calculation in the Settings
window geometry recovery flow to convert window.contentMinSize into frame
coordinates using NSWindow.frameRect(forContentRect:size:styleMask:) with the
window’s current style mask before comparing it with window.minSize. Use the
converted frame dimensions when enforcing the recovered window’s minimum size.
🪄 Autofix
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: 5feafd37-08af-425d-8a3b-fdbb997652d2
📒 Files selected for processing (7)
Sources/App/CmuxMainWindow.swiftSources/App/MultiMonitorWindowGeometry.swiftSources/App/SettingsWindowGeometry.swiftSources/AppDelegate.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxMainWindowConstrainFrameTests.swiftcmuxTests/SettingsWindowPresenterTests.swift
Comment on lines
+6
to
+96
| enum MultiMonitorWindowGeometry { | ||
| /// Pure selection of the visible-screen frame a window should be clamped | ||
| /// into. When the window's saved frame is off every active screen (e.g. | ||
| /// restored onto a now-disconnected display) it recovers onto the screen | ||
| /// under the cursor, then the main/first screen. Cursor hit-testing uses | ||
| /// each screen's *full* frame: `visibleFrame` excludes the menu bar and | ||
| /// Dock strips, and the cursor sits exactly there when a window is opened | ||
| /// from the menu bar, which would misroute recovery to the main screen. | ||
| static func targetVisibleFrame( | ||
| windowFrame: NSRect, | ||
| screens: [(frame: NSRect, visibleFrame: NSRect)], | ||
| mouseLocation: NSPoint?, | ||
| fallbackVisibleFrame: NSRect? | ||
| ) -> NSRect? { | ||
| guard !screens.isEmpty else { return fallbackVisibleFrame } | ||
|
|
||
| // Prefer the screen the window already overlaps the most so a window | ||
| // that is mostly visible stays where the user put it. | ||
| var bestFrame: NSRect? | ||
| var bestArea: CGFloat = 0 | ||
| for screen in screens { | ||
| let intersection = screen.visibleFrame.intersection(windowFrame) | ||
| let area = intersection.isNull ? 0 : intersection.width * intersection.height | ||
| if area > bestArea { | ||
| bestArea = area | ||
| bestFrame = screen.visibleFrame | ||
| } | ||
| } | ||
| if let bestFrame, bestArea > 0 { | ||
| return bestFrame | ||
| } | ||
|
|
||
| // The window is off every active screen. Recover onto the screen under | ||
| // the cursor when possible so the window appears where the user is looking. | ||
| if let mouseLocation, | ||
| let mouseScreen = screens.first(where: { $0.frame.contains(mouseLocation) }) { | ||
| return mouseScreen.visibleFrame | ||
| } | ||
| return fallbackVisibleFrame ?? screens.first?.visibleFrame | ||
| } | ||
|
|
||
| /// Pure clamp geometry: fit `frame` within `visibleFrame` (honoring `inset` | ||
| /// and a minimum size). | ||
| static func clampedFrame( | ||
| _ frame: NSRect, | ||
| minimumSize: NSSize, | ||
| into visibleFrame: NSRect, | ||
| inset: CGFloat | ||
| ) -> NSRect { | ||
| var result = frame | ||
| let maxVisibleSize = NSSize( | ||
| width: max(minimumSize.width, visibleFrame.width - 2 * inset), | ||
| height: max(minimumSize.height, visibleFrame.height - 2 * inset) | ||
| ) | ||
| result.size.width = min(result.size.width, maxVisibleSize.width) | ||
| result.size.height = min(result.size.height, maxVisibleSize.height) | ||
| let minX = visibleFrame.minX + inset | ||
| let minY = visibleFrame.minY + inset | ||
| let maxX = max(minX, visibleFrame.maxX - inset - result.width) | ||
| let maxY = max(minY, visibleFrame.maxY - inset - result.height) | ||
| result.origin = NSPoint( | ||
| x: min(max(result.origin.x, minX), maxX), | ||
| y: min(max(result.origin.y, minY), maxY) | ||
| ) | ||
| return result | ||
| } | ||
|
|
||
| /// Clamp `frame` onto a connected display using the shared target-screen | ||
| /// selection and clamp geometry. Returns `nil` when no screen is available. | ||
| static func recoveredFrame( | ||
| _ frame: NSRect, | ||
| minimumSize: NSSize, | ||
| screens: [(frame: NSRect, visibleFrame: NSRect)], | ||
| mouseLocation: NSPoint?, | ||
| fallbackVisibleFrame: NSRect?, | ||
| inset: CGFloat | ||
| ) -> NSRect? { | ||
| guard let targetVisibleFrame = targetVisibleFrame( | ||
| windowFrame: frame, | ||
| screens: screens, | ||
| mouseLocation: mouseLocation, | ||
| fallbackVisibleFrame: fallbackVisibleFrame | ||
| ) else { return nil } | ||
| return clampedFrame( | ||
| frame, | ||
| minimumSize: minimumSize, | ||
| into: targetVisibleFrame, | ||
| inset: inset | ||
| ) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the static-only namespace with a constructable geometry owner.
MultiMonitorWindowGeometry contains only static behavior. The Sources/**/*.swift rule prohibits new static-only namespaces. Store the screen snapshot, fallback frame, cursor location, and inset in an initializer. Expose recovery as instance behavior. Keep only private implementation helpers static if needed.
As per coding guidelines, “do not add ... static-only namespaces ... Put state and behavior on a constructable, injectable owning type.”
🤖 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 `@Sources/App/MultiMonitorWindowGeometry.swift` around lines 6 - 96, Convert
MultiMonitorWindowGeometry from an enum with static APIs into a constructable
type that stores screens, fallbackVisibleFrame, mouseLocation, and inset through
its initializer. Make recoveredFrame an instance method using that stored state,
and convert targetVisibleFrame and clampedFrame to instance or private helper
methods as appropriate; retain static only for private helpers if needed.
Source: Coding guidelines
Comment on lines
14
to
17
| let minimumFrameSize = NSSize( | ||
| width: max(window.minSize.width, window.contentMinSize.width), | ||
| height: max(window.minSize.height, window.contentMinSize.height) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'contentMinSize|frameRect\(forContentRect:|minimumFrameSize|clampToVisibleAreaIfNeeded|recoveredFrame' \
Sources cmuxTestsRepository: manaflow-ai/cmux
Length of output: 15408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SettingsWindowGeometry.swift =="
cat -n Sources/App/SettingsWindowGeometry.swift
echo
echo "== SettingsWindowPresenter size setup and clamp call =="
sed -n '340,380p' Sources/App/SettingsWindowPresenter.swift | cat -n
echo
echo "== CmuxMainWindow minimumFrameSize pattern =="
sed -n '440,468p' Sources/App/CmuxMainWindow.swift | cat -n
echo
echo "== MultiMonitorWindowGeometry clamp implementation =="
cat -n Sources/App/MultiMonitorWindowGeometry.swift | sed -n '1,130p'
echo
echo "== Apple NSWindow setFrame documentation reference from local repo/docs? =="
rg -n "setFrame\\(_:display:|contentMinSize|minSize|constrains" Sources/App Sources/PanelsSources/Mobile 2>/dev/null || trueRepository: manaflow-ai/cmux
Length of output: 13172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== SettingsWindowGeometry.swift =="
cat -n Sources/App/SettingsWindowGeometry.swift
echo
echo "== SettingsWindowPresenter size setup and clamp call =="
sed -n '340,380p' Sources/App/SettingsWindowPresenter.swift | cat -n
echo
echo "== CmuxMainWindow minimumFrameSize pattern =="
sed -n '440,468p' Sources/App/CmuxMainWindow.swift | cat -n
echo
echo "== MultiMonitorWindowGeometry clamp implementation =="
cat -n Sources/App/MultiMonitorWindowGeometry.swift | sed -n '1,130p'
echo
echo "== Apple NSWindow setFrame documentation reference from local repo/docs? =="
rg -n "setFrame\\(_:display:|contentMinSize|minSize|constrains" Sources/App Sources/Panels 2>/dev/null || trueRepository: manaflow-ai/cmux
Length of output: 13500
🌐 Web query:
Apple NSWindow setFrame(_:display:) does not enforce contentMinSize
💡 Result:
That is correct. According to Apple's official documentation, the contentMinSize (and contentMaxSize) constraints are explicitly not enforced by the setFrame(:display:) and setFrame(:display:animate:) methods [1][2]. These constraints are intended to limit the user's ability to resize a window interactively [3][4]. Because setFrame(_:display:) is a programmatic way to set the window's frame directly, it bypasses the enforcement of these size constraints [1][2]. If you need to programmatically resize a window while respecting your defined minimum or maximum size constraints, you must manually clamp the values before passing them to the setFrame method, or use other methods if applicable [4].
Citations:
- 1: https://developer.apple.com/documentation/appkit/nswindow/contentminsize
- 2: https://developer.apple.com/documentation/appkit/nswindow/contentmaxsize
- 3: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/WinPanel/Tasks/SizingPlacingWindows.html
- 4: https://chromium.googlesource.com/chromium/src/+/HEAD/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
Convert contentMinSize to frame coordinates before recovery.
NSWindow.setFrame(_:display:) does not enforce contentMinSize, and the current minimum mixes window.minSize (frame coordinates) with window.contentMinSize (content-view coordinates). If recovery shrinks an oversized Settings window, the returned frame can end up smaller than the required content once title-bar and border chrome are included. Compute the frame from window.contentMinSize with NSWindow.frameRect(forContentRect:size:styleMask:) before combining it with window.minSize.
Proposed fix
+ let contentMinimumFrameSize = NSWindow.frameRect(
+ forContentRect: NSRect(origin: .zero, size: window.contentMinSize),
+ styleMask: window.styleMask
+ ).size
let minimumFrameSize = NSSize(
- width: max(window.minSize.width, window.contentMinSize.width),
- height: max(window.minSize.height, window.contentMinSize.height)
+ width: max(window.minSize.width, contentMinimumFrameSize.width),
+ height: max(window.minSize.height, contentMinimumFrameSize.height)
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let minimumFrameSize = NSSize( | |
| width: max(window.minSize.width, window.contentMinSize.width), | |
| height: max(window.minSize.height, window.contentMinSize.height) | |
| ) | |
| let contentMinimumFrameSize = NSWindow.frameRect( | |
| forContentRect: NSRect(origin: .zero, size: window.contentMinSize), | |
| styleMask: window.styleMask | |
| ).size | |
| let minimumFrameSize = NSSize( | |
| width: max(window.minSize.width, contentMinimumFrameSize.width), | |
| height: max(window.minSize.height, contentMinimumFrameSize.height) | |
| ) |
🤖 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 `@Sources/App/SettingsWindowGeometry.swift` around lines 14 - 17, Update the
minimumFrameSize calculation in the Settings window geometry recovery flow to
convert window.contentMinSize into frame coordinates using
NSWindow.frameRect(forContentRect:size:styleMask:) with the window’s current
style mask before comparing it with window.minSize. Use the converted frame
dimensions when enforcing the recovered window’s minimum size.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/TerminalController.swift`:
- Around line 10664-10668: Update the not-found error returned by the window
lookup guard in the relevant TerminalController flow to use a product-neutral
message such as “Main window not found” instead of exposing the internal
CmuxMainWindow type name; preserve the existing error code and window_id data.
- Around line 10660-10689: Move the debug-only
v2DebugWindowRecoverAfterDisplayChange handler out of TerminalController.swift
into the project’s dedicated debug source location, preserving its parameter
validation, window lookup, recovery call, and response payload. Remove the
production implementation and retain only minimal routing glue in
TerminalController.swift if the socket dispatch requires it.
🪄 Autofix
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: f3a65f4f-d2ba-441b-94c7-d05847228d47
📒 Files selected for processing (2)
Sources/TerminalController+DebugMethodNames.swiftSources/TerminalController.swift
Comment on lines
+10660
to
+10689
| private func v2DebugWindowRecoverAfterDisplayChange(params: [String: Any]) -> V2CallResult { | ||
| guard let windowId = v2UUID(params, "window_id") else { | ||
| return .err(code: "invalid_params", message: "Missing or invalid window_id", data: nil) | ||
| } | ||
| guard let window = AppDelegate.shared?.windowForMainWindowId(windowId) as? CmuxMainWindow else { | ||
| return .err( | ||
| code: "not_found", | ||
| message: "No CmuxMainWindow for window_id", | ||
| data: ["window_id": windowId.uuidString] | ||
| ) | ||
| } | ||
|
|
||
| func rectPayload(_ rect: NSRect) -> [String: Double] { | ||
| [ | ||
| "x": Double(rect.origin.x), | ||
| "y": Double(rect.origin.y), | ||
| "width": Double(rect.width), | ||
| "height": Double(rect.height), | ||
| ] | ||
| } | ||
|
|
||
| let before = window.frame | ||
| CmuxMainWindow.applyOffscreenRecoveryIfNeeded(window) | ||
| let after = window.frame | ||
| return .ok([ | ||
| "changed": !before.equalTo(after), | ||
| "before": rectPayload(before), | ||
| "after": rectPayload(after), | ||
| ]) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Isolate the debug handler from the production controller.
v2DebugWindowRecoverAfterDisplayChange is a debugger-only socket seam with no production caller. Move its implementation to a dedicated debug source file or folder. Keep only minimal routing glue in Sources/TerminalController.swift if required.
As per path instructions, genuinely debug-only facilities must be isolated from production Sources/**/*.swift files.
🤖 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 `@Sources/TerminalController.swift` around lines 10660 - 10689, Move the
debug-only v2DebugWindowRecoverAfterDisplayChange handler out of
TerminalController.swift into the project’s dedicated debug source location,
preserving its parameter validation, window lookup, recovery call, and response
payload. Remove the production implementation and retain only minimal routing
glue in TerminalController.swift if the socket dispatch requires it.
Source: Path instructions
Comment on lines
+10664
to
+10668
| guard let window = AppDelegate.shared?.windowForMainWindowId(windowId) as? CmuxMainWindow else { | ||
| return .err( | ||
| code: "not_found", | ||
| message: "No CmuxMainWindow for window_id", | ||
| data: ["window_id": windowId.uuidString] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a neutral not-found message.
The v2 error body exposes the internal type name "No CmuxMainWindow for window_id". Return a product-neutral message such as "Main window not found".
As per coding guidelines, API error bodies must not expose implementation details.
🤖 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 `@Sources/TerminalController.swift` around lines 10664 - 10668, Update the
not-found error returned by the window lookup guard in the relevant
TerminalController flow to use a product-neutral message such as “Main window
not found” instead of exposing the internal CmuxMainWindow type name; preserve
the existing error code and window_id data.
Source: Coding guidelines
austinywang
force-pushed
the
issue-9696-display-disconnect-refit
branch
from
August 6, 2026 06:36
ee3d144 to
8485b81
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/App/MultiMonitorWindowGeometry.swift`:
- Around line 56-65: Update the frame-sizing logic in the visible-frame
adjustment function to cap the effective minimum size at the display’s available
size (visibleFrame minus both insets) before applying result.size. Preserve the
declared minimum when it fits, but ensure result.width and result.height never
exceed the available visible dimensions so the subsequent origin clamping can
keep the entire window visible.
In `@Sources/TerminalController.swift`:
- Line 10701: Update the frame handling around framePayload so a supplied frame
that parses to nil returns the invalid_params error instead of falling back to
window.frame; retain window.frame only when the frame parameter is absent, and
preserve the existing valid-frame behavior.
🪄 Autofix
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: 9f76240e-303e-4380-9107-0ddefef6a890
📒 Files selected for processing (9)
Sources/App/CmuxMainWindow.swiftSources/App/MultiMonitorWindowGeometry.swiftSources/App/SettingsWindowGeometry.swiftSources/AppDelegate.swiftSources/TerminalController+DebugMethodNames.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxMainWindowConstrainFrameTests.swiftcmuxTests/SettingsWindowPresenterTests.swift
Comment on lines
+56
to
+65
| let maxVisibleSize = NSSize( | ||
| width: max(minimumSize.width, visibleFrame.width - 2 * inset), | ||
| height: max(minimumSize.height, visibleFrame.height - 2 * inset) | ||
| ) | ||
| result.size.width = min(result.size.width, maxVisibleSize.width) | ||
| result.size.height = min(result.size.height, maxVisibleSize.height) | ||
| let minX = visibleFrame.minX + inset | ||
| let minY = visibleFrame.minY + inset | ||
| let maxX = max(minX, visibleFrame.maxX - inset - result.width) | ||
| let maxY = max(minY, visibleFrame.maxY - inset - result.height) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clamp the frame to the available visible size.
When minimumSize exceeds visibleFrame - 2 * inset, Line 57 retains a size that cannot fit on the display. Lines 64-65 can move only the origin. The recovered window remains clipped on the right or top edge.
Bound the effective minimum size to the available visible size before setting the frame. Preserve the declared minimum only when the target display can contain it.
🤖 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 `@Sources/App/MultiMonitorWindowGeometry.swift` around lines 56 - 65, Update
the frame-sizing logic in the visible-frame adjustment function to cap the
effective minimum size at the display’s available size (visibleFrame minus both
insets) before applying result.size. Preserve the declared minimum when it fits,
but ensure result.width and result.height never exceed the available visible
dimensions so the subsequent origin clamping can keep the entire window visible.
| ] | ||
| } | ||
|
|
||
| let before = framePayload(params["frame"]) ?? window.frame |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject an invalid supplied frame.
If frame is present but framePayload returns nil, this line silently uses window.frame and returns success. Reject the malformed parameter with invalid_params instead.
Proposed fix
- let before = framePayload(params["frame"]) ?? window.frame
+ let before: NSRect
+ if let rawFrame = params["frame"], !(rawFrame is NSNull) {
+ guard let parsedFrame = framePayload(rawFrame) else {
+ return .err(code: "invalid_params", message: "Invalid frame", data: nil)
+ }
+ before = parsedFrame
+ } else {
+ before = window.frame
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let before = framePayload(params["frame"]) ?? window.frame | |
| let before: NSRect | |
| if let rawFrame = params["frame"], !(rawFrame is NSNull) { | |
| guard let parsedFrame = framePayload(rawFrame) else { | |
| return .err(code: "invalid_params", message: "Invalid frame", data: nil) | |
| } | |
| before = parsedFrame | |
| } else { | |
| before = window.frame | |
| } |
🤖 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 `@Sources/TerminalController.swift` at line 10701, Update the frame handling
around framePayload so a supplied frame that parses to nil returns the
invalid_params error instead of falling back to window.frame; retain
window.frame only when the frame parameter is absent, and preserve the existing
valid-frame behavior.
lawrencecchen
added
the
stale-revisit
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
4 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.
Summary
main; Fix main window recovery when an external display disconnects #7127 was from an external fork, so this PR supersedes it from an in-org branch.MultiMonitorWindowGeometryrecovery/clamp math and uses it for main windows and Settings.NSApplication.didChangeScreenParametersNotificationand fromCmuxMainWindow.constrainFrameRect.debug.window.recover_after_display_changesocket hook for tagged preflight proof.Validation
git diff --check./scripts/lint-pbxproj-test-wiring.sh./scripts/run-e2e.sh cmuxTests/CmuxMainWindowConstrainFrameTests --ref issue-9696-display-disconnect-refit --wait --timeout 240, passed: https://github.com/manaflow-ai/cmux/actions/runs/31077876857, artifact: https://github.com/manaflow-ai/cmux-dev-artifacts/issues/9225./scripts/reload.sh --tag drefit, passed, cloud shim was absent in this worktree./tmp/cmux-debug-drefit.sock: synthetic pre-disconnect frame2560x980 @ 0,0recovered to1512x949 @ 0,0; the actual tagged window matched1512x949 @ 0,0afterward./var/folders/w6/6yv5pn9d57q7nhmjglj8w5wh0000gn/T/cmux-screenshots/issue-9696-after_2026-08-06T06-39-13Z_33C5D689.png.Issues