Skip to content

cmux-next: close batch B2 overlay and harness windows - #27258

Merged
teamleaderleo merged 50 commits into
feat-cmux-nextfrom
lane/window-leak-suites-B2-codex
Nov 1, 2026
Merged

teamleaderleo merged 50 commits into
feat-cmux-nextfrom
lane/window-leak-suites-B2-codex

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Batch B2 closes the remaining overlay-host and app-harness windows that outlived their tests. This follows Leo's instruction: "But why do you guys not just clean up after yourselves, or at least do your best to?"

A closing window now tears down an overlay host created during the close, and the Menu Key Routing, Multi Window Input Routing, and related harnesses close the app-owned windows they create. This branch is stacked after the merged overlay-host fix.

No capture: no visible change; these changes only close hidden panels and test windows after their owner closes.

metric before after delta harness or run link
Windows open at process exit 251 pending pending five-step fleet census harness

Agent session: agent session

Carry: merged with feat-cmux-next, review BLOCK fixed (head 2e607e3)

The earlier lane went quiet, so this was carried to the finish.

  • Merged current feat-cmux-next. Since B2 was opened, base landed most of this cleanup itself: cmux-next: enforce window cleanup trait coverage #27236 put .closesItsWindows on these suites and closes their windows (closeAfterTest, putAway, tearDown), and carry of #27257: close batch B1 test windows #28091 (B1) landed the daemon-owner teardown. Every conflict (including DaemonConnection.swift and DaemonPrestart.swift) takes base. B2 copies that duplicated base were removed: a second shutdownConnectionAndWait, second close() helpers and duplicate imports would otherwise not compile. What B2 still adds over base: AppHarnessWindows (closes the app-harness windows and page panels an AppServices test opened), the cross-machine harness teardown (both daemon owners reaped, windows and palette closed, state removed only once reaped), the palette Settings test closing its panel, and .closesItsWindows on MoveToNewWindowTests, StashTests, PageFullscreenWholeScreenTests and PaletteSettingWriteTests.
  • Dropped 2153d45 and 141a41b (reverted). They were added for B2's intermittent BackgroundAccessoryPolicyTests.aWindowClosedBeforeTheRestoreFinishesStaysClosed red. fix(windows): a window closed during launch stays closed when the restore's workspace is reported late #31155 found and fixed the cause on base: the restore's workspace being reported late. fix(windows): a window closed during launch stays closed when the restore's workspace is reported late #31155 replaced closedDuringRestore with RestoreClosedWindows. Keeping its design, nothing here changes WindowManager or WindowHandlers.
  • Review BLOCK at 141a41b (noteUserClose recorded the close before the window's delegate decided, so a cancelled incognito confirmation during the restore left the window remembered as closed and torn down). That code is gone. Base records a close only in windowWillClose, after AppKit carries it out. New test BackgroundAccessoryPolicyTests.aCloseTheWindowRefusesDuringTheRestoreKeepsItOpen keeps it that way: during the restore, Close All Windows meets a window that refuses its close, as a cancelled confirmation does. The window must stay open, restoreClosed must stay empty, and the restore must end with an open window.

Testing

cmux-ci run --class light --script scripts/ci/package-test-lane.sh --ref <sha> --arg suite --arg Packages/macOS/CmuxNext --arg <suites>

  • Red, probe 4bec788 (a throwaway commit on top of the head that records the close before the delegate decides, as 141a41b did), step e47601e9: aCloseTheWindowRefusesDuringTheRestoreKeepsItOpen failed at BackgroundAccessoryPolicyTests.swift:160, 161, 167: the refused close was in restoreClosed, was remembered as the closed launch window, and the restore ended with no open window.
  • Green, head 2e607e3, step 5c785f05: BackgroundAccessoryPolicyTests (all 4 tests, including the new one and aWindowClosedBeforeTheRestoreFinishesStaysClosed), CrossMachineTabMoveTests, MenuKeyRoutingTests, MoveToNewWindowTests, MultiWindowInputRoutingTests, PageFullscreenWholeScreenTests, PaletteSettingWriteTests, PasswordImportConflictTests and StashTests passed.
  • WindowCleanupCoverageTests failed on NewTabHoldCancellationTests.swift:5 and AAProfileMenuGalleryTests.swift:15. This PR does not touch those files, and on base they lack .closesItsWindows the same way, so it is a base red.

Observed / Not run

  • Observed: the fleet runs above; prepush-gate.py reports no new violations.
  • Not run: RestoreClosedWindowsTests (the step filter named the wrong target, so it matched nothing; this PR does not change it); a full CmuxNextAppTests run; the five-step window census (the table above stays pending).

Changelog

none

🤖 Generated with Claude Code

Agent session: agent session

teamleaderleo and others added 8 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
Leo (2026-10-28): "But why do you guys not just clean up after yourselves, or at least do your best to?"

- WindowOverlayHost.tearDown closes its overlay panel when its window closes (it only ordered it
  out, so every closed window left an open OverlayHostPanel in the app's window list).
- Test windows close instead of ordering out (isReleasedWhenClosed = false first); harnesses that
  never closed their window now do.
- Hover card, tooltip and palette windows the code under test opened close in teardown.
- App harness tests (withApp, the multi-view fixture) close the windows their app opened,
  including the page chrome's Page Info and Extensions bubbles.
- Each fixed suite carries .closesItsWindows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
…host; app harness releases its pages

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

A toast or overlay presented while a window closes (a toast's dismiss that offers the next one)
reached a new host for the closed window and opened a new panel nothing closed. The torn-down host
answers for the window until the close is over and never attaches its panel again.

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

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

A window controller's teardown (run from the close) could make the window's first overlay host;
its close observer came after the close notification, so its panel stayed open. The window
manager tears that host down after the controller's teardown. The palette test ends the search
field's editing before it closes the palette.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
…es what the app opened for its windows

Leo (2026-10-28): "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
…down a host made during the close

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-85ca59f74f02d81734c3
@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 2e607e3: 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=pr27258 -f ref=lane/window-leak-suites-B2-codex -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Web bundle key inputs (+1 more) is not red on the base a971618; do not merge until green; see lane-rules

Default changes

No defaults change at 2e607e39a997.

Shipped in:

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

Copy link
Copy Markdown
Collaborator Author

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex-review-box/gpt-6-astra high, lane-place): BLOCK at f4a00b0

BLOCK

Exact head: f4a00b0a85ae4d18914ff7bb8c74b0ea43b3f59a.

  1. [P2] B1 profile-menu tests leave browser popup panels alive. BrowserProfileNewTabMenuTests.swift:55 closes only the palette. These tests create browser tabs but retain the original PinnedPageTests/PasswordManagerRouteTests harnesses, whose cleanup closes shell windows without releasing cached browsers. Bound action handlers retain AppServices; therefore cached chrome—and its Page Info/Extensions panels—survives. The new retirement cleanup never runs for those entries. Release the test app’s cached pages, as AppHarnessWindows.withApp does.

  2. [P2] Cleanup still misses failed setup and cancellation. PageFullscreenWholeScreenTests.swift:53 installs teardown only after the throwing asynchronous fixture initializer returns. That initializer opens its window before a throwing sleep; cancellation bypasses both fixture.close() and the new panel sweep. B1 has the same gap at BrowserProfileNewTabMenuTests.swift:54: a timeout waiting for palette rows occurs before the panel is captured for cleanup. Register cleanup immediately upon acquisition and unwind partially initialized fixtures. The suite-end leak checker is not per-test teardown.

Validation limits: all 47 changed files match the diff’s head blob hashes. Teardown ordering, late-overlay guards, and window ownership were reviewed statically. No macOS runtime suites ran; this box is Linux without Swift. Runtime suites and the five-step full census remain pending fleet execution. No crash or incomplete run is excused as base-red. No files were modified or merged.

@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/tmp_apux53a/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpxullppw9/summary-fail.json (live):/summary-fail.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
imported web bundle inputs are not routed:/imported web bundle inputs are not routed: View Logs
test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov
ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)/
test_every_part_of_bun_run_check_runs_in_the_warn_branch (main.WebviewsWarnSplitCov
ersCheck.test_every_part_of_bun_run_check_runs_in_the_warn_branch)
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-review-box/gpt-6-astra high, lane-place): BLOCK at f8bbe6b

BLOCK

Reviewed head f8bbe6bd784897980db4d2c2405a3576d4ce7dfb against base 6ce9c0a7e37d803e788f7ac58b892ce4d09cd490.

  1. [P2] B1 remains incomplete: BrowserProfileNewTabMenuTests.swift:132. The third test, theProfileBadgeMenuOpensItsTabInTheRightClickedTabsPane, never registers Self.close. It opens browser tabs, but PasswordManagerRouteTests.withApp only closes shell windows. Cached browser entries and their popup panels remain unretired. Register cleanup at this closure’s entry too.

  2. [P2] Browser cleanup assumes the tested close succeeds: DaemonClosedBrowserPageTests.swift:25. The defer releases only tabs[0]. If closeTab throws or either subsequent wait fails, tabs[1] can retain its page and panels. Teardown should release both entries independently of the assertions.

  3. [P2] Background cleanup is skipped on early failure: BackgroundAccessoryPolicyTests.swift:75. The new status-item cleanup runs only through success-path shutDown() calls. For example, the requirement at line 95 can throw after closing the last window entered background mode. Register unconditional teardown so that failure also removes the status item.

The first two profile tests now cover the throwing palette wait. B2’s fullscreen cleanup precedes async initialization, and both routing factories cover partial construction; their window sweep filters by suite and test ownership.

Validation was static: all 47 changed files match the diff’s head blob hashes; none are omitted. No macOS build or runtime suites ran in this Linux box. Fleet suites and the complete census remain pending; readiness and absence of runtime regressions are unverified. Crashed or incomplete runs provide neither passing evidence nor proof of base failures.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex-review-box/gpt-6-astra high, lane-place): SHIP at 89f803e

SHIP

@teamleaderleo teamleaderleo removed the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026

@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 89f803e: an independent review passed this exact head (review box review-27258-89f8 receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

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

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): BLOCK at 141a41b

  • [WindowHandlers.swift:57] noteUserClose runs before WindowTargeting.close, whose delegate may reject the close asynchronously for an incognito window. If the user cancels that confirmation during restore, the ID remains in closedDuringRestore; restore then marks the still-open window closed and tears it down. Record the intent only after an accepted close, or exclude rejected confirmations.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Orphaned: no activity for 19 h, and no lane holds a lease on it. The orphan sweep could not merge feat-cmux-next in: conflicts in Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Connection/DaemonConnection.swift. Listed for a lane to resolve.

teamleaderleo and others added 4 commits November 1, 2026 23:04
… it already covers B2

Base (#27236, #28091, #31155) already closes the windows these suites open, stops daemon
owners, and keeps a window closed during the restore. Conflicts take base; B2 keeps only
what base lacks: the app-harness window close (AppHarnessWindows), the cross-machine and
palette teardown, and the trait on four suites that open windows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-1a293c23a59e24572f4b
… open

Review of #27258 (BLOCK at 141a41b): noteUserClose recorded the close before the
window's delegate decided, so a cancelled incognito confirmation during the restore left
the window remembered as closed and the restore tore it down. That code is gone (base's
#31155 records the close in windowWillClose, after AppKit carries it out); this test keeps
it that way.

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

cursor Bot commented Nov 1, 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 added risk:low Risk tier (docs/ci/risk-tiers.md) and removed risk:normal Risk tier (docs/ci/risk-tiers.md) labels Nov 1, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex-review-box/gpt-6-astra high, lane-place): SHIP at 2e607e3

SHIP at 2e607e3

Visual verdict: no images; Leo tries the build

Follow-ups: none

@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 2e607e3: an independent review passed this exact head (review box review-27258-2e60 receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@teamleaderleo
teamleaderleo merged commit f5523b5 into feat-cmux-next Nov 1, 2026
163 of 167 checks passed
@teamleaderleo
teamleaderleo deleted the lane/window-leak-suites-B2-codex branch November 1, 2026 23:50

This branch was previously deployed

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant