cmux NEXT: a window closed during the launch restore stays closed

Merged
TE
10d ago
+142-51

Summary

BackgroundAccessoryPolicyTests is red on base on every run, not only on loaded runners. It closes the last window (Close All Windows) as soon as the launch window shows a workspace, and expects the app to have no window and run as a menu bar app. Instead, the same window opens again 56ā€“75 ms later.

Cause: since the first launch shows Home early (#22389), a window can show a workspace while WindowManager.restore() is still running. A window the person closes then is opened again by the rest of the restore:

  • "the most recent window opens": the restore puts back the saved registry, finds no window open, and opens the most recent one;
  • FirstWorkspaceLaunch.land: it reveals the first workspace, which finds its window closed and opens a new one.

Fix: userClosed records windows the person closes while the restore runs, in closedDuringRestore.

  • The restore keeps those windows closed.
  • It skips "the most recent window opens" when any were closed. No window open is then the background mode, not a launch to recover.
  • It skips revealing the first workspace in that case.
  • The set is emptied when the restore finishes, so later closes behave as before.

The launch window too (found by the independent review): closing the launch window during restore used to skip userClosed.

  • Its id is now recorded too, whether it wears the saved front window's id (launch snapshot) or a random one.
  • A random-id launch window's id becomes reconcile's fallback window while the restore runs, and the saved window it would have adopted counts as closed.
  • Windows in the set are marked closed inside the registry step itself (the restored-registry swap and every reconcile), so no sync builds or presents them, not even for a frame.
  • New test aWindowClosedBeforeTheRestoreFinishesStaysClosed: it closes the launch window before the restore finishes, then checks that no window is open and none was presented.

Testing

The red-first test is the existing BackgroundAccessoryPolicyTests (fails at lines 73, 74 and 82: a window is still open and the app is regular after Close All Windows).

No capture: the change is in window lifecycle timing, not in how anything draws. A window closed during the launch restore now stays closed; no screen state changes, and BackgroundAccessoryPolicyTests checks that.

SidebarOneMorphingCardTests and PalettePickerSourcesTests fail in cmux-next swift test here and fail the same way on base (SidebarOneMorphingCard: three shownCards expectations at df9b43f on the same host).

Follow-up fixes after adoption (Codex gpt-6-astra xhigh review)

  • The close fade: fadeOutThenClose now closes at once while the restore runs, or before it has run. A fade could outlive the restore, which then would not know the window was closed and would open it again.
  • Reopen during the restore: both reopenClosedWindow overloads remove the reopened id from closedDuringRestore, so a window the person reopens (Dock click, Reopen Closed Window) before the restore finishes stays open. This replaces the race the earlier "Not in this PR" note described.
  • releaseClosedWindow: a window released with no heir opens again on purpose, so its id leaves closedDuringRestore too.

These three paths have no regression test: the fade doesn't run in tests, and the reopen paths need the same-tree cmux-tui that BackgroundAccessoryPolicyTests skips for in CI.

Not in this PR

  • A launch window the person closes and then reopens during a slow terminal-first launch can be closed again, because closedLaunchWindowID isn't cleared on reopen. This is rare and non-blocking per the review. Follow-up.
  • A saved window that exists only in the cloud keeps its base behavior. It's out of scope here.

The culprit is likely #22389, from the timing, but it wasn't bisected.

Agent session: cosmic-gray

šŸ¤– Generated with Claude Code