Avoid unnecessary synchronous dirty-workspace scans on quit

Merged
AU
1mo ago
+320-16

Summary

⌘Q now skips the synchronous Ghostty dirty-workspace scan when the selected quit policy cannot use that result. The scan still runs for an unconfirmed stable user quit in dirtyOnly mode, so quit confirmation behavior is unchanged. Session persistence, bounded scrollback snapshotting, and owned-agent cleanup remain on their existing paths.

The scan can reach ghostty_surface_needs_confirm_quit, which takes the renderer mutex implicated in the quit-hang family. The new policy gate avoids taking that path for unconditional, disabled, already-confirmed, development, session-end, and update-relaunch termination requests. The existing deferred termination for socket-driven Cmd-Q from #10788 remains in place.

Fixes #18620

Investigation and residual risk

  • #10788 was already addressed by deferring termination from the main-queue socket handler; this change does not alter that scheduling boundary.
  • #6758 is an OS pasteboard promised-data stall during NSApplication.terminate; the existing termination watchdog still bounds that path, and this PR does not remove pasteboard or other termination observers.
  • dirtyOnly quits still perform the precise dirty-workspace scan when its result controls the warning. Full scrollback snapshotting and owned-agent termination also remain synchronous cleanup steps, so those paths can still add quit latency.

Validation

  • git diff --check origin/main...HEAD — passed.
  • python3 scripts/verify-local.py --only swift-syntax --swift-changed origin/main — passed; 3 changed Swift files parsed.
  • python3 scripts/localization_defaults.py — passed; 0 mismatches.
  • bash tests/test_ci_pbxproj_test_wiring.sh — passed.
  • Focused CI on 1ce0e60e17853560f976c9f5a58aec1b8935a99c: QuitConfirmationAlertPresenterTests/dirtyWorkspaceScanRunsOnlyForUnconfirmedUserDirtyOnlyQuits() passed, 1 test in 1 suite: https://github.com/manaflow-ai/cmux/actions/runs/37840834033
  • The local ./scripts/test-unit.sh ... build-for-testing attempt was rejected by the repository admission guard; no native Xcode build was run in this checkout.
  • verify-local.py --only feature-flags timed out/interrupted; that check is unrelated to the changed quit policy.

Changelog

Fixed: Skip unnecessary synchronous dirty-workspace scans during quit.


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


Summary by cubic

Fixes the ⌘Q quit slowness from #18620 by skipping the synchronous dirty-workspace scan on quit paths that cannot use its result, bounding the quit scrollback snapshot with Ghostty's native tail formatter, and keeping sudo broker shutdown off the termination critical path. Quit confirmation behavior is unchanged.

  • Gate the scan behind an unconfirmed stable dirtyOnly user quit, avoiding the Ghostty close-risk query and its renderer mutex for unconditional, disabled, dev, already-confirmed, session-end, and update-relaunch terminations. Startup breadcrumbs record whether the scan ran.
  • Stop the sudo broker in the background on termination instead of joining its recovery tasks inline, so persistence and AppKit termination do not wait for the join.

Scrollback capture

  • Snapshot only the newest scrollback rows with readBoundedScreenTailVT and a row/byte budget instead of formatting the full history and trimming it.
  • Keep the internal clipboard export as a fallback for runtimes that reject the bounded request, with test stubs and unit coverage for both paths.

Process census

  • Bound the fresh quit process census at 750 ms, falling back to the cached shared index; exact PID and ownership validation still apply to the cached data.

[!NOTE]
Medium Risk
Changes quit and session-snapshot timing (bounded process census fallback, async sudo stop) and scrollback capture path; confirmation semantics are narrowed but dirtyOnly user quits are unchanged.

Overview
Reduces quit latency by avoiding work on termination paths that do not need it, while keeping dirtyOnly quit confirmation behavior for unconfirmed stable user quits.

Quit confirmation: The synchronous dirty-workspace scan (which can hit Ghostty’s close-risk query and renderer mutex) now runs only when shouldEvaluateQuitConfirmationDirtyWorkspaces says it matters—unconfirmed stable user quit in dirtyOnly mode. Cmd+Q and applicationShouldTerminate skip the scan for always/never modes, dev builds, already-confirmed quits, and session/update terminations; breadcrumbs record whether the scan ran.

Terminate-later cleanup: Sudo broker shutdown is removed from the blocking owned-cleanup task and moved to stopInBackgroundForTermination() so slow broker joins do not delay session persistence. Fresh process census during quit uses loadFreshWithDeadline (750ms) with fallback to cached resume indexes when the scan times out.

Scrollback snapshots: Session persistence prefers Ghostty’s readBoundedScreenTailVT with row/byte caps instead of exporting full VT history and trimming; the clipboard export path remains as fallback.

Reviewed by Cursor Bugbot for commit 4ce04df3da4394948935a118ad358f15d3913add. Bugbot is set up for automated code reviews on this repo. Configure here.