Skip to content

carry of #27257: preserve daemon ownership through teardown - #28093

Closed
teamleaderleo wants to merge 17 commits into
feat-cmux-nextfrom
lane/carry-27257-pinned-teardown
Closed

teamleaderleo wants to merge 17 commits into
feat-cmux-nextfrom
lane/carry-27257-pinned-teardown

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Carries #27257: cmux-next test windows and panels must close after each test, while preserving daemon ownership safely through teardown.

The original PR's latest exact-head review found two P2 teardown hazards in AppTestDaemonOwner:

  • a cancelled endpoint caller could remove its still-running ensure task before reap joined it;
  • an endpoint whose process identity was not verified could still receive a graceful socket shutdown, potentially stopping a replacement daemon.

This carry keeps admitted ensure tasks until teardown joins them, and only performs graceful shutdown for verified process identities. Unverified endpoints use a bounded exit observation and preserve state when ownership remains ambiguous.

Validation: git diff --check passed. Fleet Swift tests were not run because the broker was temporarily rate limited while this carry was prepared.

Original PR: #27257

Agent session: agent session


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


Summary by cubic

Carries #27257: cmux-next tests must close the windows and panels they open after each test, while teardown preserves daemon ownership so a replacement daemon is never accidentally stopped.

An exact-head review of the original found two teardown hazards in AppTestDaemonOwner: a cancelled endpoint caller could drop its still-running ensure task before reap joined it, and an unverified endpoint could receive a graceful socket shutdown that stops a replacement daemon. This carry keeps admitted ensure tasks until teardown joins them, and performs graceful shutdown only for verified process identities. Unverified endpoints get a bounded exit observation and preserve state when ownership is ambiguous.

Behavior changes

  • DaemonConnection.closeAndWait() and DaemonService.shutdownConnectionAndWait() join in-flight startup and reconnect attempts before the owner is reaped.
  • Page Info certificate, site settings, and site data windows close with the retired page, and a late certificate response can no longer present anything.
  • The Mac screen approval prompt closes and drops the last request's panel on settle.

Test harnesses now tear down daemons, windows, palette panels, and cached browser pages even on early failure or cancellation.

Validation: git diff --check passed. Fleet Swift tests were not run because the broker was temporarily rate limited while this carry was prepared.

Written for commit 351edf5. Summary will update on new commits.

View guided diff Turn on auto-fix

teamleaderleo and others added 17 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
Wait for the destination daemon store through its existing sync barrier before inspecting moved tabs. Keep metadata assertions unchanged so a failed rename still fails the test.

Validation: prepush gate and diff check pass. Native Swift syntax is unavailable in this Linux lane; CrossMachine/Stash fleet rerun required. Prior fleet runs 0e700eba and da960326 failed the name assertion before this barrier.
Agent-Session: https://cmux.com/deeplink/agent?id=aid-66d4d9063f5f1bfabf11
Propagate cancellation from the presentation wait, capture its error, and join owned daemon shutdown before rethrowing. Register window and directory cleanup before startup so their lifetime ends in order.

Validation: static prepush and diff checks pass. Swift syntax/native execution unavailable in the Linux lane; fleet regression and cumulative review requested.
Agent-Session: https://cmux.com/deeplink/agent?id=aid-66d4d9063f5f1bfabf11
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
Record verified process identities and use the existing audit-token fallback when a shutdown handshake fails. Preserve state directories until every owned daemon exit is confirmed, and bound moved-tab synchronization and cancellation.

Validation: cmux-next prepush gate and diff check pass; native Swift compilation/runtime unavailable in this Linux lane. Fleet reruns and exact-head review required.
Agent-Session: https://cmux.com/deeplink/agent?id=aid-66d4d9063f5f1bfabf11
@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 351edf5: 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=pr28093 -f ref=lane/carry-27257-pinned-teardown -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 695d1f5; does not block

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Found 7 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpnibhp_j_/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpxpx14fas/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
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
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

Superseded by #28091, a newer carry of #27257 with the same teardown fixes plus the compile repair (head ff12a5d). Closing this one in its favour.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

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

BLOCK

  1. Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/BackgroundAccessoryPolicyTests.swift:93,102 — withLaunch replaces the process-wide AppProcessRoot with an inert AppServices instance when no root existed, then permanently restores that replacement. Since AppProcessRoot has no way to clear or restore nil, subsequent tests in the same process see a stale root and AppServices.background refuses to recognize their services as the process root. A throwing Launch initializer also leaves the newly adopted services installed.

  2. Packages/macOS/CmuxNext/Sources/CmuxNextBrowser/PageInfo/UI/PageInfoController.swift:130-136 — closePanel() only cancels certificateTask; if the page-info controller is retired while its asynchronous loadTask is active, that task remains attached to the controller and can continue updating the retired page. Retirement should cancel and clear all outstanding controller tasks/observation before dropping the panel.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 351edf5b Deployed Oct 30, 2026 by teamleaderleo via namespace autofix #7387
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