Repository navigation
Fix Cmd+N window size after last window closes - #3611
Merged
Merged
Conversation
The no-source main-window path currently falls back to a tiny built-in frame instead of the last persisted geometry, so add a regression test that seeds the runtime defaults payload and creates a real main window through AppDelegate. Constraint: Regression policy requires the failing test to land before the fix Confidence: high Scope-risk: narrow Directive: Keep this test behavior-level; do not replace it with source-text assertions Tested: git diff --check Not-tested: XCTest not run locally per repo policy
Cmd+N after the last window closes reaches createMainWindow without a live source window. That path now resolves initial geometry in one AppDelegate-owned order: session snapshot, live source window, persisted last geometry, then a centered 1000x700 fallback clamped to the visible display. Constraint: New main-window creation has multiple entrypoints, so the geometry policy belongs in createMainWindow rather than a menu-only handler Rejected: Add only a SwiftUI defaultSize | main windows are AppKit-created here and the bug also affects Dock/socket/service entrypoints Confidence: high Scope-risk: narrow Directive: Keep new-main-window geometry fallback centralized in AppDelegate.createMainWindow Tested: git diff --check Not-tested: XCTest not run locally per repo policy
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change addresses the Cmd+N new-window issue by introducing a custom ChangesNew-Window Geometry Restoration
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~23 minutes Poem
🚥 Pre-merge checks | ✅ 13✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
The guard budget rejects growth in AppDelegate and AppDelegateShortcutRoutingTests, so move the default-window geometry helper onto the existing CmuxMainWindow type and place the regression in a smaller AppDelegate test file while preserving the same behavior. Constraint: CI enforces per-file Swift line budgets Rejected: Raise the file length budget | this fix can stay within the existing limits Confidence: high Scope-risk: narrow Directive: Keep future AppDelegate window-placement growth out of the large AppDelegate file Tested: git diff --check Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Not-tested: XCTest not run locally per repo policy
Contributor
Greptile SummaryFixes the tiny 460×360 Cmd+N window that appeared after the last window closed by inserting persisted last-window geometry as a third priority in
Confidence Score: 5/5Safe to merge; the change is a well-scoped frame-priority fix with a matching regression test and no correctness issues in the new geometry logic. The frame selection chain is correct in all four branches (session snapshot, source window, persisted geometry, hardcoded default). The private helper resolvedPersistedWindowGeometryFrame lives in AppDelegate.swift alongside the two private methods it calls, so no access-control widening is needed. The clampedFrame helper only operates on already-validated NSRect values from NSScreen.visibleFrame and has a clean guard for zero-size displays. The empty deinit {} in MainWindowHostingView is the correct Swift 6 pattern for NSHostingView subclasses. The test correctly saves and restores UserDefaults in a defer block and asserts frame dimensions within a 1pt tolerance. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[createMainWindow called] --> B{sessionWindowSnapshot?}
B -- yes --> C[resolvedWindowFrame returns restoredFrame]
B -- no --> D{sourceWindow?}
D -- yes --> E[existingFrame from sourceWindow.frame]
D -- no --> F[resolvedPersistedWindowGeometryFrame]
F --> G{persistedGeometry in UserDefaults?}
G -- yes --> H[Decode and map to current display - persistedGeometryFrame]
G -- no --> I[CmuxMainWindow.defaultContentRect 1000x700 clamped to visibleFrame]
C --> J[initialRect = contentRect for restoredFrame, setFrame restoredFrame]
E --> K[initialRect = contentRect for existingFrame, positionNewMainWindow relative to source]
H --> L[initialRect = contentRect for persistedGeometryFrame, setFrame persistedGeometryFrame]
I --> M[initialRect = defaultContentRect, window.center + cascade]
J --> N[Window shown]
K --> N
L --> N
M --> N
N --> O{explicitInitialFrame set?}
O -- yes --> P[setFrame again with display true + DEBUG log]
O -- no --> Q[done]
P --> Q
Reviews (5): Last reviewed commit: "Preserve window geometry fallback order" | Re-trigger Greptile |
The Cmd+N geometry fix should not widen AppDelegate internals just to satisfy tests or a helper placement. Move the persisted geometry resolver back into AppDelegate's implementation file, keep the helper methods private, and expose only a DEBUG test hook for the defaults key so the regression test follows the production key without duplicating the literal. Constraint: Swift file length budget blocks unbounded AppDelegate growth Rejected: Leave the resolver in CmuxMainWindow.swift | forces unrelated AppDelegate helpers to become internal Rejected: Keep the test's duplicated defaults key | can drift from production persistence schema Confidence: high Scope-risk: narrow Tested: git diff --check Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Not-tested: XCTest per repository policy to prefer CI and user instruction forbidding direct xcodebuild
The previous review fix preserved the Swift file budget by compressing the persisted-geometry helper, but that made the window sizing path harder to read. Move MainWindowHostingView beside CmuxMainWindow in the existing main-window source file, then restore the AppDelegate geometry helper to normal Swift formatting while keeping the file under budget. Constraint: Swift file length budget must remain green Rejected: Keep the semicolon-packed helper | non-idiomatic and called out by review Confidence: high Scope-risk: narrow Tested: git diff --check Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Not-tested: XCTest per repository policy to prefer CI and user instruction forbidding direct xcodebuild
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/CmuxMainWindow.swift`:
- Around line 4-26: Add an explicit empty deinit to the MainWindowHostingView
class to satisfy the required_deinit lint rule: inside final class
MainWindowHostingView<Content: View> { ... } add a deinit { } implementation (no
teardown needed) so the class defines a deinitializer while leaving existing
properties and init(coder:) unchanged.
- Around line 63-74: defaultContentRect currently builds a content rect at
origin .zero then clamps the frame but leaves the rect bottom-left positioned,
which is confusing; update CmuxMainWindow.defaultContentRect to explicitly
return a clamped content rect without attempting to center (i.e., compute
frameRect via NSWindow.frameRect(forContentRect: defaultContentSize, styleMask:
styleMask), clamp it with clampedFrame(..., within: visibleFrame), and return
NSWindow.contentRect(forFrameRect: clamped, styleMask: styleMask)), and add a
brief doc comment to defaultContentRect stating it returns an
unpositioned/clamped rect so callers (window.center(), positionNewMainWindow(),
etc.) are responsible for final placement.
In `@Sources/AppDelegate.swift`:
- Around line 6865-6874: The current logic gates sourceWindow and
persisted-geometry fallbacks on sessionWindowSnapshot == nil, so if session
snapshot exists but resolvedWindowFrame(from:) returns nil you skip to default
sizing; change the selection order to explicitly try the snapshot-derived frame
first, then sourceWindow, then persisted geometry, then default. Concretely,
call resolvedWindowFrame(from: sessionWindowSnapshot) (or equivalent) into a
snapshotFrame variable and use it if non-nil; otherwise if let existingFrame
(from sourceWindow) use that; otherwise use restoredFrame ??
resolvedPersistedWindowGeometryFrame(); for whichever frame you pick convert to
content rect via NSWindow.contentRect(forFrameRect: styleMask:) and assign to
initialRect. Ensure persistedGeometryFrame is computed but not conditionally
hidden behind sessionWindowSnapshot == nil so the fallback order is: session
snapshot → sourceWindow → persisted → default.
🪄 Autofix (Beta)
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
Run ID: a886d8d1-a017-4bd1-9381-02dd756ca21c
📒 Files selected for processing (3)
Sources/App/CmuxMainWindow.swiftSources/AppDelegate.swiftcmuxTests/AppDelegateBareSpaceShortcutRoutingTests.swift
CodeRabbit caught that a malformed session snapshot could skip the source-window and persisted-geometry fallbacks. Keep the intended priority explicit: snapshot frame first, then source window, then persisted geometry, then the clamped default. Also add the required empty deinit for MainWindowHostingView and document that defaultContentRect only supplies a clamped, unpositioned content rect. Constraint: Do not edit CI workflow YAML files while updating the PR Rejected: Keep gating source-window fallback on sessionWindowSnapshot == nil | invalid snapshots would jump straight to default sizing Confidence: high Scope-risk: narrow Tested: git diff --check Tested: python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv Not-tested: Local build or XCTest; per repo policy and user instruction, no xcodebuild was run
This was referenced May 9, 2026
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
1 participant
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
Testing
Note
Medium Risk
Changes main-window creation geometry selection to pull from persisted state and clamp to screen bounds, which could affect window placement/sizing across restore and multi-display scenarios.
Overview
Fixes new main windows (e.g.,
Cmd+Nafter the last window closes) opening at a tiny fallback size by reusing the persisted last-window frame when there is no livesourceWindow.Replaces the hardcoded
460x360fallback with a1000x700default content size clamped to the current visible display, and movesMainWindowHostingViewplus sizing helpers intoCmuxMainWindow.Adds a regression test that seeds persisted window geometry in
UserDefaultsand assertscreateMainWindow(sourceWindow: nil)uses that saved frame (via a DEBUG-only hook to access the defaults key).Reviewed by Cursor Bugbot for commit 474fbb3. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the tiny window created by Cmd+N after the last window closes by reusing the last saved window geometry. Falls back to a centered 1000x700 default clamped to the visible display when no geometry is saved.
Bug Fixes
AppDelegate.createMainWindowto cover Cmd+N, Dock, and service entrypoints.Refactors
MainWindowHostingViewintoCmuxMainWindow.AppDelegatewith a DEBUG-only defaults-key hook; restore normal formatting while staying within file-length budgets.Written for commit 474fbb3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Refactor
Tests