Skip to content

cmux-next tests: batch B suites close the windows they open (1 of 2), popups close their panels - #27239

Merged
teamleaderleo merged 4 commits into
feat-cmux-nextfrom
window-leak-suites-B1
Oct 29, 2026
Merged

teamleaderleo merged 4 commits into
feat-cmux-nextfrom
window-leak-suites-B1

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Leo: "But why do you guys not just clean up after yourselves, or at least do your best to?" (on Swift suites that leave windows open for the suites after them)

This is the second of three. It follows #27172 (overlay host panels close with their window).

What leaked and why

The #27003 census at its head (fleet steps wleak-before-1/3/5) named 19 suites in this batch. Together they left 35 windows open at process exit. There were three causes:

  • Teardowns that only order a window out. defer { window.orderOut(nil) } hides a window, but it stays open in NSApp.windows. Affected: AgentInteropAccessibility (2), FullScreenTitlebarPageMode (2), the Home suites' shell windows, HibernationWakeCover, HostPressureBannerToast and others. Fixed with close().
  • Product popups that keep their panel between opens and only order it out. Page Info and the Extensions bubble (NativePopupHost) stayed open after their page closed: BrowserToolbarPressTarget 8, AgentPageStaysInWindow 2, DaemonClosedBrowserPage 2. The fix is the new NativePopupHost.closePanel(). BrowserPageRetirement (the page close path) now closes both bubbles for good. The host pressure popover closes its panel when its indicator leaves its window. The Mac screen approval prompt closes and drops its panel on hide. That panel held the prompt, so it stayed for good, and the next request makes a fresh one.
  • Panels the code under test opens that the test never closed. These are the palette (BrowserProfileNewTabMenu, DebugPaletteCapture) and Home's Chief Settings popover (HomeOpenFocusesComposer; Home makes it up front and only ever hides it). The tests now close them through the product's own API in teardown.

Each suite now carries @Suite(.closesItsWindows), so a new leak fails that suite by name. No test is skipped and no assertion is changed.

No capture: no visible change. The popups look and open the same; only their hidden panels now close when their owner goes away.

Performance

Windows these 19 suites left open at process exit, from the fleet test census. Before is the #27003 head (steps wleak-before-1/3/5); after is step ef4421a6 at this tree, same filters.

metric before after delta harness
windows left open at exit, 19 suites 35 1 -34 (-97%) package-test-lane.sh suite, steps wleak-before-1/3/5 / ef4421a6
BrowserToolbarPressTargetTests 8 0 -8 same
HomeOpenFocusesComposerTests 2 1 -1 same (that run was cut short; see below)

Known, not ours

  • BackgroundAccessoryPolicyTests.aWindowClosedBeforeTheRestoreFinishesStaysClosed fails on the slower perf minis with the same two expectations and messages at base. The base runs that show it are steps fd82dc62 and 0d61f186, both pinned to the same worker; the test passes on faster workers at both base and head. It also leaves no window open now.
  • HomeOpenFocusesComposerTests: the test process sometimes ends in the middle of openingAConversationFromTheList… (the run-loop stop its own doc comment describes). The harness window is then still open because its defer never ran. This happens at base too (wleak-before-1 and the batch base run). Runs that finish pass and leave 0 windows.

Risk

The product changes are small and only on close paths: a page's popup panels close when the page retires; the host pressure popover's panel closes when it leaves its window; the approval prompt's panel closes on hide. Each popup reopens with a fresh or reopened panel on its next show.

Agent session: agent session

🤖 Generated with Claude Code


Summary by cubic

Closes the windows these 19 cmux-next test suites leave open at process exit (35 open at exit down to 1), and makes the popup panels that caused most of the leak close for good when their owner goes away.

What leaked and why

  • Teardowns that only ordered a window out hid it but left it in NSApp.windows; they now call close().
  • Page Info, the Extensions bubble and the host pressure popover kept their panels between opens and only ordered them out; the new NativePopupHost.closePanel() now closes them when their page retires, when the indicator leaves its window, and when the Mac screen approval prompt hides (the prompt also drops its panel).
  • Panels the tests themselves opened, the palette and Home's Chief Settings popover, are now closed in teardown through the product's own APIs.

Every suite carries @Suite(.closesItsWindows), so a new leak fails that suite by name. No test is skipped and no assertion changes.

Written for commit 7d6bea5. Summary will update on new commits.

View guided diff Turn on auto-fix

teamleaderleo and others added 3 commits October 28, 2026 22:13
Leo (2026-10-28), on Swift suites that leave windows open for the suites
after them: "But why do you guys not just clean up after yourselves, or at
least do your best to?"

Each suite in this batch now closes every window its tests open, in the
test's own teardown, and carries `.closesItsWindows` so a new leak fails by
name:
- teardowns that only ordered a window out close it;
- tests that make browser pages release them (or close the chrome's popup
  panels), and tests that open the palette close its panel;
- background mode's menu bar item goes when the launch test shuts down.

Product code never closed three kinds of panel, only ordered them out, so
they stayed in the app's window list while anything held their owner:
- a window's overlay host panel, when the window closed (WindowOverlayHost);
- a browser page's Page Info and Extensions panels, when its tab closed
  (BrowserEntry.close now calls BrowserChromeView.closePopupPanels);
- the Mac screen approval prompt's panel, whose content holds the prompt
  (now closed and dropped when the prompt hides; the next request makes one).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
…heir panels

Leo (2026-10-28): "But why do you guys not just clean up after yourselves,
or at least do your best to?"

- WindowOverlayHost keeps a closed window's torn-down host registered, so a
  late `host(for:)` for that window returns it instead of making a fresh
  panel nothing closes; a window shown again gets a new host, and
  `existingHost` still answers nil once torn down.
- NativePopupHost.closePanel closes a popup's panel for good; Page Info,
  the Extensions bubble and the host pressure popover (when its indicator
  leaves its window) use it.
- DebugPaletteCaptureTests and HomeOpenFocusesComposerTests drop the trait
  for now: the first leaves only AppKit's own TUINSWindow, the second the
  Chief Settings popover's panel, which its owner never closes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
…pKit-only suites keep the trait

Leo: "But why do you guys not just clean up after yourselves, or at least do your best to?"

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

cursor Bot commented Oct 29, 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.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 7d6bea5: 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=pr27239 -f ref=window-leak-suites-B1 -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on e219b3a to compare; do not merge until green; see lane-rules

@teamleaderleo

teamleaderleo commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 29, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Found 8 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpibw0cwww/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmpnl7to0au/summary-missing.json (live):/summary-missing.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra, high): SHIP at 2a4ec40

Independent Codex review of the full diff at this head, including the moved popup close in BrowserPageRetirement.close(). The reviewer found no blockers. BackgroundAccessoryPolicyTests.aWindowClosedBeforeTheRestoreFinishesStaysClosed fails the same way at base on the same worker.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

This PR conflicts with feat-cmux-next. Please resolve it with a normal merge commit (no force push) and get a fresh review at the new head. The window-leak series is the fix for the cmux-next swift test hang, where leaked windows starve the main thread and runs get cut short at 1500 s. Four UI PRs are blocked on it: #24073, #24295, #25978 and #26558. #27462 has merged, so a cut-short run now names its unfinished tests.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

This change continues in #27257, which carries the same batch B1 window cleanup plus the repairs found in review (page-info certificate, site-settings and site-data windows now close with their tab). I'll close this PR with a link once #27257 lands; until then, review and land through #27257.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

For awareness, from an audit against the aiforui course's popover rules (a popover inside another layer closes only itself; overflow fades at the edge, never a hard clip). This PR touches the same popup hosts, so these are yours to take or reject:

  1. NativePopupHost.swift:139-142: a click inside a child panel dismisses the outer popup host as well. Expected: an outside click, Escape or a pick closes only the innermost layer. A popover opened from inside another must leave its container open.
  2. WebPopupHost.swift:177-183: the native host has no edge mask where scrolling content is clipped. Expected: the clipped edge fades, matching the web ui-scroll-edge-fade that is in cmux NEXT: draggable Session Info cards and shared edge treatments #26720, and is never cut hard.

The audit is leaving both files alone while this PR is open, to avoid conflicts.

🤖 Generated with Claude Code

Resolve BrowserToolbarPressTargetTests: keep base's NSApplication.shared init and the PR's .closesItsWindows trait.

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

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at 7d6bea5

No blocking findings.

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmux-bot approves 7d6bea5: an independent review passed this exact head (next-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@teamleaderleo
teamleaderleo merged commit 1674720 into feat-cmux-next Oct 29, 2026
157 of 167 checks passed
@teamleaderleo
teamleaderleo deleted the window-leak-suites-B1 branch October 29, 2026 23:36
teamleaderleo added a commit that referenced this pull request Oct 29, 2026
Base now has #27239 squashed. Conflicts keep this branch's cleanup: PageInfoController.closePanel retires and closes its windows, BackgroundAccessoryPolicyTests stops through AppTestDaemonOwner, DaemonClosedBrowserPageTests releases both tabs, and BrowserProfileNewTabMenuTests / DebugPaletteCaptureTests keep their own panel close (base's second close is dropped).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-0cf32b32a03b27dcdf4d
@github-actions

Copy link
Copy Markdown
Contributor

Shipped in:

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 7d6bea5f Deployed Oct 29, 2026 by teamleaderleo via wait for same-key tree publisher #78729
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