feat (desktop): persist window size and restore macOS fullscreen exits cleanly#3597
feat (desktop): persist window size and restore macOS fullscreen exits cleanly#3597rushilrai wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| window.setAutoHideCursor(false); | ||
| } | ||
|
|
||
| yield* windowState.attach(window); |
There was a problem hiding this comment.
🟡 Medium window/DesktopWindow.ts:292
windowState.attach(window) is called immediately after window creation, before the reveal callback runs window.maximize(). If the window is closed before ready-to-show/did-finish-load fires (e.g. slow or failed renderer startup), the close handler persists window.isMaximized() as false, overwriting the saved maximized state. The next launch then restores a normal-sized window instead of the user's maximized state. Consider calling window.maximize() right after windowState.attach, or deferring windowState.attach until after the maximize is applied.
Also found in 1 other location(s)
apps/desktop/src/window/DesktopWindowState.ts:251
When
resolveDocumentseeswindow.isFullScreen(), it unconditionally writesrestoreMode: "fullscreen-origin"and drops whether the pre-fullscreen window was maximized. If the user enters macOS fullscreen from a maximized window and quits there, the next launch restores a large normal window instead of re-maximizing, so the previous maximize state is silently lost.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindow.ts around line 292:
`windowState.attach(window)` is called immediately after window creation, before the reveal callback runs `window.maximize()`. If the window is closed before `ready-to-show`/`did-finish-load` fires (e.g. slow or failed renderer startup), the close handler persists `window.isMaximized()` as `false`, overwriting the saved `maximized` state. The next launch then restores a normal-sized window instead of the user's maximized state. Consider calling `window.maximize()` right after `windowState.attach`, or deferring `windowState.attach` until after the maximize is applied.
Also found in 1 other location(s):
- apps/desktop/src/window/DesktopWindowState.ts:251 -- When `resolveDocument` sees `window.isFullScreen()`, it unconditionally writes `restoreMode: "fullscreen-origin"` and drops whether the pre-fullscreen window was maximized. If the user enters macOS fullscreen from a maximized window and quits there, the next launch restores a large normal window instead of re-maximizing, so the previous maximize state is silently lost.
| const persist = (document: PersistedWindowStateDocument): Effect.Effect<void> => | ||
| Effect.gen(function* () { | ||
| const directory = path.dirname(windowStatePath); | ||
| const tempPath = `${windowStatePath}.${process.pid}.tmp`; |
There was a problem hiding this comment.
🟡 Medium window/DesktopWindowState.ts:231
persist writes every save through the same fixed temp path `${windowStatePath}.${process.pid}.tmp`. Overlapping saves — e.g. a debounced resize persist racing with an immediate close or maximize persist — clobber each other's temp file, so the final rename can persist a stale document or one writeFileString/rename fails and is silently swallowed by the catch. Consider using a unique temp path per persist call (e.g. appending a random or counter suffix) to prevent concurrent writes from colliding.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindowState.ts around line 231:
`persist` writes every save through the same fixed temp path `` `${windowStatePath}.${process.pid}.tmp` ``. Overlapping saves — e.g. a debounced resize persist racing with an immediate `close` or `maximize` persist — clobber each other's temp file, so the final `rename` can persist a stale document or one `writeFileString`/`rename` fails and is silently swallowed by the `catch`. Consider using a unique temp path per persist call (e.g. appending a random or counter suffix) to prevent concurrent writes from colliding.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1708db1. Configure here.
| if (!isWindowVisibleEnough(sanitizedOrigin, workAreas)) { | ||
| return fallback; | ||
| } | ||
| return { bounds: sanitizedOrigin, restoreMode: "fullscreen-origin" }; |
There was a problem hiding this comment.
Fullscreen restore blocked by normalBounds
Medium Severity
When restoreMode is fullscreen-origin, the load function prematurely rejects persisted window state if normalBounds fail the visibility check. This prevents evaluation of fullscreenOriginBounds, causing the window to reset to default positioning, particularly after display layout changes.
Reviewed by Cursor Bugbot for commit 1708db1. Configure here.
| window.on("unmaximize", persistNow); | ||
| window.on("enter-full-screen", persistNow); | ||
| window.on("leave-full-screen", persistNow); | ||
| window.on("close", persistNow); |
There was a problem hiding this comment.
Fullscreen quit loses origin state
High Severity
On macOS, quitting while in native fullscreen often emits leave-full-screen before close. Both handlers call persistNow, and resolveDocument only writes fullscreen-origin when window.isFullScreen() is still true, so the final save becomes normal/maximized and overwrites the earlier fullscreen snapshot—undermining the PR’s fullscreen exit restore behavior.
Reviewed by Cursor Bugbot for commit 1708db1. Configure here.
| // Re-maximize before showing so the window doesn't flash at normal bounds first. | ||
| if (initialWindowState.restoreMode === "maximized" && !window.isDestroyed()) { | ||
| window.maximize(); | ||
| } |
There was a problem hiding this comment.
Early quit drops maximized mode
Medium Severity
Restored maximized mode is applied only in the first-reveal callback, while attach persists on close from the live window via readRestorableState. If the user quits while the main window is still hidden (e.g. during the connecting splash), isMaximized() is false and the saved file drops back to normal, losing maximized layout on the next launch.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1708db1. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new window state persistence feature with ~300 lines of new logic. Multiple unresolved review comments identify substantive bugs, including a high-severity race condition where quitting in fullscreen loses the origin state. Human review is needed to address these issues and validate the new functionality. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What Changed
This PR enables persistence of window states for the desktop app across relaunches
It adds a
DesktopWindowStateservice (apps/desktop/src/window/DesktopWindowState.ts) that persists the main window's bounds and restore mode towindow-state.jsonand restores them on launch.It restores:
Keeps normal restore bounds separate from the fullscreen-origin visible frame, so standard maximize behaviour stays intact. Missing, corrupt, or off-screen state safely falls back to the centered default.
Implementation notes:
screenis read at a single spot.FileSystemlayer with atomic temp-file writes, a debounced interruptible-fiber save on resize/move, and an immediate save on maximize / fullscreen toggle / close.DesktopWindow.createWindow(initial bounds + maximize-on-reveal),DesktopEnvironment(windowStatePath), and the desktop foundation layer inmain.ts.Why
Right now the desktop app always comes back at the default size, which is frustrating if you keep it in a specific layout.
This change makes relaunch behavior feel much closer to what users expect from native desktop apps.
UI Changes
Desktop window restore behaviour only.
Checklist
Note
Low Risk
Local desktop window UX and JSON persistence only; no auth, network, or shared backend changes, with fallbacks and unit tests for load/sanitize paths.
Overview
Adds
DesktopWindowState, which reads and writeswindow-state.jsonunder the app state directory so the main window reopens with the last size, position, and restore mode instead of fixed defaults.DesktopWindownow loads those bounds when creating the main window, callsattachto save on move/resize (debounced) and on maximize/fullscreen/close, and maximizes before reveal when the saved mode was maximized. Afullscreen-originmode stores the pre-fullscreen frame so relaunch after macOS true fullscreen can reopen on the current desktop without re-entering fullscreen.Missing, invalid, version-mismatched, or off-screen saved state falls back to centered defaults on the primary display.
DesktopEnvironmentexposeswindowStatePath; the service is registered in the desktop foundation layer inmain.ts. Geometry helpers and load behavior are covered by dedicated tests;DesktopWindowtests stub the service to stay filesystem-free.Reviewed by Cursor Bugbot for commit 1708db1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Persist window size and restore maximized state on desktop window creation
DesktopWindowStateservice (DesktopWindowState.ts) that loads and saves window geometry to awindow-state.jsonfile in the app state directory.📊 Macroscope summarized 1708db1. 4 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.