Skip to content

cmux NEXT: a window closed during the launch restore stays closed - #23695

Merged
teamleaderleo merged 13 commits into
feat-cmux-nextfrom
browser-import-2/bg-accessory-close
Oct 26, 2026
Merged

teamleaderleo merged 13 commits into
feat-cmux-nextfrom
browser-import-2/bg-accessory-close

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

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).

  • Red on base 967cc2b: step 03438c61a24ee14018ba6dbe.
  • Red with only the first part of the fix (keeping closed windows closed and skipping the reopen rule): step bc7733b86f5b70510b632c5f. That located the second reopen, FirstWorkspaceLaunch.land.
  • Green at f403f9c (before rebase): step 740eef3f80576b19a09aa272, aws-m4pro-9. BackgroundAccessoryPolicy, FirstLaunch, FreshLaunchSidebar, RestorePreviousLaunch and WindowCloseNeighbor all passed.
  • Red on the current tip df9b43f: step 97389b20b0d9006af7d0585f, aws-m4pro-5, the same three lines.
  • Green at the rebased head 85912fd: step 35f4f3b433808e5bc5a3ae25, aws-m4pro-2. All 7 suites passed: the 5 above plus LaunchFirstFrameReveal and WindowFocusRestore.

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

teamleaderleo and others added 2 commits October 21, 2026 13:08
The restore puts back the saved registry, where the window is open, and
then opens the front window when none is open. A Close All Windows that
landed before the restore finished came back 60-75 ms later, so the app
never went to background mode (BackgroundAccessoryPolicyTests red on base).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d18-98cd-7a42-ac32-a70f43729c15&host=cosmic-gray
@teamleaderleo teamleaderleo added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 21, 2026
@teamleaderleo

teamleaderleo commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review c38061c: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr23695 -f ref=browser-import-2/bg-accessory-close -f platform=macos -f backend_mode=local

Passes: CI passes on c38061cd00.

CI passes on c38061cd00 (run 40297927815 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

web and size checks: green

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-f47am created for this PR by the cmux coordinator.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (independent subagent): SHIP at 85912fd

  • State: restored and restoring are set together before the first await. Before restore starts, only the unregistered launch window exists, and its close never reaches userClosed. The defer empties the set after land.
  • Regressions:
    • Normal launch, or a previous launch that left every window closed: the set is empty, so behavior is unchanged.
    • Registry closes (programmaticCloses, terminating) return before userClosed, so they are never recorded.
    • Incognito windows aren't in the saved document, so they aren't affected.
  • Concurrency: the package runs on the main actor by default. The loop walks a copy of the set while it updates the registry.
  • Non-blocking:
    • When the person closes one of several windows during the restore, land is skipped. It only selects the workspace created for an empty tree.
    • Already the case before this PR: closing the launch window while it is still unregistered isn't recorded.

@blacksmith-sh

This comment has been minimized.

The launch window often wears the saved front window's id, and its close
returned before userClosed, so the restore opened that window again and
still landed the first workspace. Record it in closedDuringRestore too,
and test a close before the restore finishes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d18-98cd-7a42-ac32-a70f43729c15&host=cosmic-gray
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next crash-class rules / cmux-next crash-class rules is red on 8 of 9 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

…estore

Closing a launch window with a fresh id left reconcile registering the
workspaces under another new id, so a window opened anyway. Keep the closed
launch id while restore runs as reconcile's fallback window, and treat the
saved window it would have adopted as closed too. The test's temp dir goes
in a defer again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d18-98cd-7a42-ac32-a70f43729c15&host=cosmic-gray
@cursor

cursor Bot commented Oct 22, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

…y step itself

Closing it after reconcile let the reconcile's sync build and present it for
a moment (activating the app out of background mode). The restored-registry
swap and every reconcile now mark those windows closed before any sync, and
the test checks nothing was presented.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15d18-98cd-7a42-ac32-a70f43729c15&host=cosmic-gray
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (independent subagent): SHIP at dc89c30

Fourth pass, after three HOLDs, each fixed on this branch:

  • the launch window close skipped userClosed;
  • a random-id launch window was replaced by a fresh reconcile window;
  • closed windows were presented for a moment before restore closed them.

At this head, keepClosedDuringRestore runs inside the registry swap and every reconcile, so no sync opens a closed window. The state resets in the defer. Nothing changes outside restore.

Not blocking: a window closed and then reopened (Dock) before restore finishes is closed again; this is noted in the body.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol): BLOCK at dc89c30

Blocking findings:

  • [P2] Packages/macOS/CmuxNext/Sources/CmuxNextApp/Windows/WindowManager.swift:293-294: an explicit reopen is undone. While restore is suspended, close a registered window, then invoke Show cmux (or Reopen Closed) before restore finishes. userClosed records the id, but neither reopenClosedWindow overload in WindowManager+Membership.swift:258-286 removes it. The reopen sets isOpen = true and presents the window; the next membership reconciliation, including restore's final reconciliation, calls keepClosedDuringRestore and sets it false again, so sync closes the window the person just requested. Remove the suppression when an explicit reopen succeeds, and cover close → reopen → finish restore. The PR body acknowledges this case, but it is a regression of the existing reopen action.

Non-blocking findings:

  • Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift:91-96: the new test can pass without restore completing. The wait checks restored && !registry.isLaunching, but silently returns on timeout; the following requirement checks only restored, which is set at the start of restore, before its awaits. If restore remains suspended after that assignment with no controllers, every remaining assertion passes. Require the actual completion predicate after the wait. The fixture also starts with an empty state directory, so it does not exercise saved-window identity adoption or the restored-registry replacement.

Checked: all three changed files against merge-base 01f3767, surrounding launch/adoption, registry persistence/reconciliation, close/reopen action paths, and test assertions; small static source checks only, no builds or Swift suites. Final head recheck matched the SHA above.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (independent subagent): SHIP at c4ada02

The head is a clean merge of feat-cmux-next (b5a78a8) into the reviewed head dc89c30. The base merge brings in the base fixes the earlier swift-test run lacked (the tooltip offender and the cut-short run were not this PR's).

  • This PR's change: WindowManager.swift, WindowManager+Membership.swift and BackgroundAccessoryPolicyTests.swift are byte-identical to the reviewed head.
  • Nearby base changes: base changed drag-target code in WindowController+MultiView.swift and palette labels in WindowTargetSource.swift. Neither touches restore, the closed-during-restore set, registry reconcile or the close path. No base line mentions restore(), reconcileMembership, setRestored, adoptLaunchWindow, userClosed or windowWillClose.
  • The test: every symbol it uses still resolves at this head.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next guards / tooling-tests is red on 16 of 16 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next guards / nightly-guards / workflow-guard-tests / release-notary is red on 15 of 16 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Orphaned: no activity for 14 h, and no lane holds a lease on it. Adopted by the orphan sweep: merged feat-cmux-next in at e913f95. A later pass queues it for the merge hub if it is reviewed, small and red only on base reds.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Checks job structure is red on 13 of 14 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › No new SwiftUI (macOS UI is AppKit) is red on 13 of 14 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

…at once

A fade-out defers windowWillClose, so the close could land after the restore
cleared closedDuringRestore, and the restore would reopen or reveal into it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-3e2afd09725c1ccba8ee
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

high-signal review link / link is red on 16 of 18 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

Reopen Closed Window cleared nothing, so the restore's next reconcile closed
the reopened window again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-3e2afd09725c1ccba8ee
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next fast gates / post cmux-next/fast-gates is red on 16 of 20 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra xhigh): SHIP at f3bf599

Four rounds. R1–R3 found the close-fade, reopen and releaseClosed paths, and all three are fixed. R4 shipped it with one non-blocking note: closedLaunchWindowID isn't cleared on reopen. That's rare and listed in the body as a follow-up. The reds at this head don't come from this PR: NativeTooltipGuard flags MacScreen and Design files this PR doesn't touch, and the BrowserTabMirror, ActionBatch, hibernation and screen-replace timeouts differ from run to run.

@teamleaderleo
teamleaderleo merged commit 36ce49d into feat-cmux-next Oct 26, 2026
132 of 136 checks passed
@teamleaderleo
teamleaderleo deleted the browser-import-2/bg-accessory-close branch October 26, 2026 11:14
@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

This branch was previously deployed

1 inactive deployment
ci-routing-agent — f3bf5991 Deployed Oct 26, 2026 by teamleaderleo via namespace autofix #3315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant