Skip to content

base red: four window-opening suites close their windows (.closesItsWindows) - #29544

Merged
teamleaderleo merged 4 commits into
feat-cmux-nextfrom
drain-ci-swift-test-closes-windows
Nov 1, 2026
Merged

teamleaderleo merged 4 commits into
feat-cmux-nextfrom
drain-ci-swift-test-closes-windows

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

The desk's ask: "Add .closesItsWindows to the @suite of PaletteLargeTextTests, SidebarFoldKeyboardTests, WorkspaceTabHoverCardContrastTests and PageWebViewFocusViewChangeTests. Make sure each suite really closes what it opens; if one leaks on purpose, fix the leak instead."

Summary

This PR fixes a base red. WindowCleanupCoverageTests.everySuiteThatOpensAWindowClosesWhatItLeaves fails on feat-cmux-next. It names four suites, all landed 2026-10-31, that open windows without .closesItsWindows:

The failure is in run 41219252754, job 124604477110 (#28605, which changes no Swift, so its Swift suites are the base's). The test failed at WindowCleanupCoverageTests.swift:33 with those four offenders.

Changes

  • Each suite gets .closesItsWindows (and import CmuxNextTestSupport).
  • PaletteLargeTextTests: it really leaked. PaletteController.hide() only orders the panel out, so each test left its palette panels open. The test now uses the target's closeForTest() (hide, then close the panel), as the other palette suites do. Its host windows use closeAfterTest().
  • The other three: their windows are closed in a defer, so a failed #require no longer leaves one open. WorkspaceTabHoverCardContrastTests resolves its theme before it makes the window, so a throw there leaves nothing open.

Why it matters: the cut-short swift test

The cmux-next swift test broad pass is cut short at its 1500 s whole-run limit (exit 143). That is main-thread starvation in the one shared test process, from windows piling up. In run 41219252754:

  • Throughput fell from about 1,000 to 2,500 tests a minute (16:45 to 16:51) to about 100 to 400, and about 45 at the end.
  • From 17:00 the log has 15 to 45 s gaps, and unrelated suites report "passed after 270 to 1440 s".
  • At 1500 s the hang sample shows a 7.4 GB process footprint. 2043 of 2452 main-thread samples are in CA::Transaction::flush, then NSDisplayCycleFlush, then -[NSWindow layoutIfNeeded], deep in sidebar view layout (SidebarIconButton.layout).
  • 32 tests started and never finished.

This is a partial fix. It stops these four suites from adding windows, and the trait now fails any of them by name if it leaves one. The structural fix is #29490 (carry of #26809): it runs the main-thread-heavy suites in their own processes and shards the broad pass. Until #29490 lands, this PR's own swift test may still be cut short at 1500 s.

Testing

  • python3 scripts/cmux-next/prepush-gate.py: no new violations.
  • Not run locally: no Swift build or test on this host. This PR's cmux-next swift test job is the check: WindowCleanupCoverageTests should pass, and each changed suite should report no window leak.

Changelog

none

Agent session: agent session

🤖 Generated with Claude Code


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


Summary by cubic

Makes the four window-opening test suites on feat-cmux-next declare .closesItsWindows and actually close what they open, fixing the WindowCleanupCoverageTests failure.

  • PaletteLargeTextTests genuinely leaked: hide() only orders the palette panel out, so it now uses closeForTest() like the other palette suites.
  • The other three closed their windows in a defer, but a failed #require could leave one open; they now close via closeAfterTest(). WorkspaceTabHoverCardContrastTests also resolves its theme before making the window.

This is also a partial fix for the cmux-next swift test run being cut short at its 1500 s limit, since the window pile-up starved the main thread.

Written for commit c0e697e. Summary will update on new commits.

View guided diff Turn on auto-fix

…esItsWindows

WindowCleanupCoverageTests.everySuiteThatOpensAWindowClosesWhatItLeaves fails
on base: PaletteLargeTextTests, SidebarFoldKeyboardTests,
WorkspaceTabHoverCardContrastTests and PageWebViewFocusViewChangeTests open
windows without the trait. PaletteLargeTextTests also only ordered its
palette panels out (hide()), never closed them; it now closes them with
closeForTest(). The other three close their windows in a defer, so a failed
#require no longer leaves one open.

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

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 7f07196: 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=pr29544 -f ref=drain-ci-swift-test-closes-windows -f platform=macos -f backend_mode=local

Shipped in:

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@blacksmith-sh

blacksmith-sh Bot commented Oct 31, 2026

Copy link
Copy Markdown

Found 3 test failures on Blacksmith runners:

Failures

Test View Logs
tab: tabGroup.addTab is not a top-level row/tab: tabGroup.addTab is not a top-level row View Logs
tab: tabGroup.create is not a top-level row/tab: tabGroup.create is not a top-level row View Logs
tab: tabGroup.removeTab is not a top-level row/
tab: tabGroup.removeTab is not a top-level row
View Logs

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

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

Copy link
Copy Markdown
Collaborator Author

Review (codex, router-review): SHIP at 7f07196

Reviewer: codex gpt-6.1-sol xhigh, fast tier.

Follow-ups: 1. Each lands as an issue in this repo that links this receipt; the lane that opened the PR fixes it forward.

SHIP at 7f07196

Visual verdict: no images; Leo tries the build

No correctness findings.

Follow-ups:

  • [P2] [follow-up] No after screenshots were provided for visual comparison.

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

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (next-review patch-id; carried): SHIP at 02b0112

The independent SHIP at 7f07196 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (18e06a4266dd4b6a) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@teamleaderleo teamleaderleo removed the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@github-actions github-actions Bot added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 20:43 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 20:43 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 20:44 Inactive
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (next-review patch-id; carried): SHIP at 700dc73

The independent SHIP at 7f07196 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (18e06a4266dd4b6a) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@teamleaderleo teamleaderleo removed the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@github-actions github-actions Bot added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 22:48 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 22:48 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 22:52 Inactive
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (next-review patch-id; carried): SHIP at c0e697e

The independent SHIP at 7f07196 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (18e06a4266dd4b6a) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@teamleaderleo
teamleaderleo merged commit 0f07db8 into feat-cmux-next Nov 1, 2026
197 of 210 checks passed
@teamleaderleo
teamleaderleo deleted the drain-ci-swift-test-closes-windows branch November 1, 2026 00:12

This branch was previously deployed

1 inactive deployment
ci-routing-agent — c0e697ea Deployed Oct 31, 2026 by cmux-bot[bot] via wait for same-key tree publisher #91894
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