Sign up
View stargazersSign up

fix(desktop): a maximized window reopens on the display it was last used on#25833

open6
main
fix/issue-4841
opened17h agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 43m ago

Problem

The desktop window always reopened on the wrong monitor after it was maximized on another one. While maximized (or fullscreen), the app saves getNormalBounds(), the pre-maximize frame, and on macOS and Windows that frame often still sits on the first monitor. Startup places the window from those bounds and then maximizes it there. The reporter's desktop-settings.json shows exactly that: {"mainWindowBounds":{"x":410,"y":86,…},"mainWindowMaximized":true} for a window maximized on monitor 2.

Reproduced on main (689763a603) in the real Electron app on two 1920×1080 displays (Linux, Xorg, openbox). After a restart, the app maximizes the window itself. If that maximized window is then moved to display 2, the move is never saved, because the normal bounds did not change. The next launch reopens on display 1. A quit while minimized also dropped the maximized flag, because macOS reports a minimized window as not maximized.

Change

apps/desktop/src/window/DesktopWindow.ts:

  • While the window is maximized or fullscreen, if its normal bounds do not fit inside the display the window occupies (the display with the largest overlap), the saved bounds are moved onto that display. The size is kept, shrunk to fit if needed, and the offset is kept and clamped, so startup's existing resolveInitialMainWindowBounds accepts them and maximize() lands on the right display. Bounds that already fit are saved unchanged, as before.
  • The window state is recorded whenever a bounds event fires while the window is not minimized. A write that runs while minimized (the debounce, or the quit flush) uses that recorded state instead of rereading the stale minimized values: macOS reports not maximized plus the old frame, and Windows reports -32000. This keeps both the maximized flag and a move made just before minimizing (#3851's case).

No settings format change and no new setting.

Scope and approval

Fixes pingdotgg/t3code#4841

Triaged via the automated issue flow.

Verification

Focused tests, in apps/desktop/src/window/DesktopWindow.test.ts (28/28 pass; vp run check:changed passes):

  • "persists bounds on the display a maximized window was moved to": three cases. The normal bounds are on display 1, on display 1 with a smaller 1280×720 display 2, or straddling both displays. All three fail on main; the straddle case also failed on an earlier revision of this branch.
  • "keeps the moved display when a maximized window quits minimized": fails on main.
  • "persists a $window window change minimized before its $write": four cases, a normal or maximized window, with an immediate flush or the debounce firing while minimized.
  • "flushes normal bounds when minimized before the debounce completes": the existing #3851 test, still passing. The Windows -32000 position is never written.

Evidence: the real Electron app built at each SHA, a full 3840×1080 root (two displays), with recordings and the saved settings after every step. Before = 689763a603, after = 6fd7ae506e (this branch before the merge of main). Full evidence with recordings, timestamps and per-step settings · observed-outcome matrix · driver scripts. Each sheet shows Before | After on top and Diff0 (threshold 0) | DiffT8 (threshold 8/255) below.

Row (2× 1920×1080 unless noted)Before 689763a603After 6fd7ae506eSheet (Before, After, Diff0, DiffT)
L03: app-restored maximized window moved to display 2, restartmaximized on display 1maximized on display 2L03-sheet
L09: fullscreen on display 2 after an app-restored maximize, restartdisplay 1display 2 (fullscreen is not restored, as before)L09-sheet
L12: maximized on display 2, minimized, quit while minimizednormal on display 2 (maximized flag lost)maximized on display 2L12-sheet
L12b: as L12, minimized 250 ms after the movenormal on display 2maximized on display 2L12b-sheet
L13: normal move, minimized 250 ms later, quit while minimizednormal at the moved positionsame (identical at threshold 0)L13-sheet
L01 / L02 / L04 (display 2 1280×720) / L10: controlsidentical at threshold 0L01 · L02 · L04 · L10

What else changed, beyond window placement: on L12/L12b the restored window is maximized, so the title bar shows the restore glyph and the window fills the display. Nothing else differs in the final stills at threshold 0.

Not verified here:

  • cannot-verify macos: the reporter's platform. On Linux, a window maximized by the window manager reports its maximized bounds as normal bounds, so the reporter's literal steps (move, then maximize) already restore correctly on Linux (L02). The macOS path is covered by the code and the tests.
  • cannot-verify windows.
  • Mixed per-display scale factors.
  • Straddling normal bounds in the real app: the Linux window manager replaces them on maximize. This case is covered by the unit test only.

Model and harness: Claude Opus 5.5 in Claude Code (via T3 Code), with step agents on Claude Opus 5.5 and GPT-6.1 Sol (Codex).

🤖 Generated with Claude Code

Checks

21 successful · 6 skipped · 2 neutral
All checks passed
238347c

Added 6 commits

  • Merge remote-tracking branch 'origin/main' into fix/issue-4841238347c
  • test(desktop): reproduce #4841 maximized window restoring on the wrong display3742be7
  • fix(desktop): a change made just before minimizing is still saved5f78dfa
  • fix(desktop): a maximized window whose normal bounds straddle displays keeps its display6fd7ae5
  • fix(desktop): minimized quit keeps a maximized window's display94f161d
  • fix(desktop): maximized window reopens on the display it was maximized onca11aa4
apps/desktop/src/window/DesktopWindow.tsline 185Open thread
……@@ -148,6 +148,58 @@ function windowFitsWithinDisplay(
182+ }
183+ const source = displayMatching(normalBounds, displays) ?? target;
184+ const width = Math.min(normalBounds.width, target.width);
185+ const height = Math.min(normalBounds.height, target.height);
MmacroscopeappBoton line 18515h ago

🟡 Medium window/DesktopWindow.ts:185

On a display shorter than 620px, this returns a normalBounds.height below the settings minimum; for example, a 600px display produces height 600. normalizeMainWindowBounds then rejects the bounds, causing readPersistableState to return null and persistCurrentBounds to skip saving the new window state, including the maximized flag. Preserve settings-valid bounds when the target display is smaller than the minimum so the state is still persisted.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/desktop/src/window/DesktopWindow.ts around line 185:
2
3On a display shorter than 620px, this returns a `normalBounds.height` below the settings minimum; for example, a 600px display produces height 600. `normalizeMainWindowBounds` then rejects the bounds, causing `readPersistableState` to return `null` and `persistCurrentBounds` to skip saving the new window state, including the maximized flag. Preserve settings-valid bounds when the target display is smaller than the minimum so the state is still persisted.
MmacroscopeappBot15h ago
Synced from GitHub

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a localized desktop window-persistence fix that restores maximized windows to the display where they were last used and adds targeted multi-display/minimization tests without changing schemas or deployment behavior. An unresolved Medium finding identifies a remaining persistence edge case on displays shorter than the configured minimum height.

Needs human approval because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot15h ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between bc8d02ffd51b5004a5b11f553977ac2049d54b8a and 238347c1a248efef9dc15f5d135851e9e2dd394f.

External image Files selected for processing: 2
  • apps/desktop/src/window/DesktopWindow.test.ts
  • apps/desktop/src/window/DesktopWindow.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

Window bounds persistence now tracks normalized bounds and maximized state together. Maximized and fullscreen windows use bounds associated with the display they occupy. Minimized windows reuse the last captured state.

Changes: Desktop window bounds persistence

Layer / File(s)Summary
Normalize bounds for the occupied display
apps/desktop/src/window/DesktopWindow.ts, apps/desktop/src/window/DesktopWindow.test.ts		Display-overlap logic selects a display for maximized or fullscreen windows and normalizes their bounds. Persistence captures those bounds with the maximized state. Tests cover bounds that fit one display or straddle displays.
Preserve captured state while minimized
apps/desktop/src/window/DesktopWindow.ts, apps/desktop/src/window/DesktopWindow.test.ts		The fallback check compares captured bounds and maximized state. Tests cover flush and debounce writes around minimization, including quitting after moving a maximized window to another display.

Priority: External image Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#21590: Both changes update main-window bounds handling in DesktopWindow.ts; this PR also addresses maximized and fullscreen bounds during persistence.
  • pingdotgg/t3code#13125: Both changes modify main-window bounds restoration and fallback behavior.

Suggested reviewers: macroscopeapp

Merge Risk: External image Minimal · up to 23834

The reviewed persistence path retains the moved display and maximized state across restart. No identified issue prevents merging after normal checks.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the primary fix: restoring a maximized desktop window on the display where it was last used.
Description checkPassedThe description includes all required sections, explains the problem and implementation, links the related issue, documents focused verification and evidence, and clearly states unverified platforms a…
Linked Issues checkPassedIssue [#4841] requires restoration of a maximized desktop window on the display where it was last used. DesktopWindow.ts selects the display with the greatest overlap, relocates non-fitting normal b…
Out of Scope Changes checkPassedThe changes remain within desktop window-state persistence for [#4841]. The added tests directly support the persistence behavior. The implementation keeps the existing settings format and adds no unr…
Full details: Description check

Explanation

The description includes all required sections, explains the problem and implementation, links the related issue, documents focused verification and evidence, and clearly states unverified platforms and scenarios.

Full details: Linked Issues check

Explanation

Issue [#4841] requires restoration of a maximized desktop window on the display where it was last used. DesktopWindow.ts selects the display with the greatest overlap, relocates non-fitting normal bounds to that display, preserves size and offset, and clamps the result. It records the last non-minimized bounds and maximized state for delayed and quit writes. The added tests cover display changes, straddling bounds, minimized flushes, debounce writes, and maximized-state retention.

Full details: Out of Scope Changes check

Explanation

The changes remain within desktop window-state persistence for [#4841]. The added tests directly support the persistence behavior. The implementation keeps the existing settings format and adds no unrelated setting or feature.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.