fix: many-pane workspace wedges the main thread in SwiftUI focus navigation (#10311)

AP
5d ago
+173-0

Fixes the hang reported in #10311: a many-pane workspace pins the main thread and the window never becomes visible, on both session restore and workspace switch.

The hang

Every pane is wrapped in its own NSHostingController by bonsplit's SinglePaneWrapper, so a many-pane workspace stacks ~15 nested NSHostingViews. AppKit rebuilds the default key view loop on every subview insertion, and SwiftUI answers each nested host's acceptsFirstResponder by running a fresh full FocusNavigationSequence over the subtree beneath it. That work multiplies across the nesting levels.

Both failing paths converge on one trigger; only the outer frames differ (makeKeyAndOrderFront vs. the display cycle):

AppKitPlatformViewHost.viewDidMoveToSuperview()
  → -[NSView _setDefaultKeyViewLoop]              ← the trigger, both paths
    → -[NSView _recursiveSetDefaultKeyViewLoop] ×3
      → NSHostingView._recursiveSetDefaultKeyViewLoop()
        → FocusBridge.updateDefaultKeyViewLoop()
          → FocusNavigationSequence.next()
            → NSHostingView.acceptsFirstResponder.getter
              → FocusBridge.hostAcceptsFirstResponder.getter
                → FocusNavigator.firstNavigableItem.getter
                  → FocusNavigationSequence.next()      … ×15

Worth stating precisely: this is not infinite recursion. Both hang samples show the cycle repeating exactly 15 times at a stack depth of only ~220–240 frames — bounded nesting with exponential work per level. The 5 MB sample files come from the call graph being wide, not deep. A 21-pane workspace stalls the main thread for 9s+; 1–3 panes are instant.

The first commit sets window.autorecalculatesKeyViewLoop = false in createMainWindow, before contentView is assigned so it lands before any pane mounts. autorecalculatesKeyViewLoop is the documented switch for exactly this recalculation, and it is currently never set anywhere in the tree.

This should be safe because cmux never reads that loop: there are no uses of nextKeyView, selectNextKeyView or canBecomeKeyView anywhere in the repository. Tab belongs to the shell, and pane focus is driven by cmux's own focus system.

The self-perpetuating relaunch

The second commit addresses a separable problem from the same report. A workspace that wedges the main thread during restore is self-perpetuating: selectedWorkspaceIndex still points at it, so force-quit and relaunch walks straight back in, indefinitely, with no user-facing route out.

A watchdog cannot fix this by aborting the restore — a wedged main thread will not run anything scheduled on it, so any main-thread recovery path is dead by construction. Instead SessionRestoreGuard records that a restore started and clears that record only once the main thread has proven it is still servicing work afterwards. Reaching the end of completeSessionRestoreOperation is not sufficient, since the restored panes mount later during layout — which is exactly where the stall happens — so the clear is deferred behind a liveness delay. If the main thread is stuck, that block never runs and the marker survives to the next launch, which then comes up clean.

Coming up clean is itself destructive without care: registerMainWindow saves a snapshot unconditionally, so an empty session would overwrite the good one within seconds. That is the data loss described in #8267. The guard therefore copies the snapshot to session-<bundle>-recovered.json before skipping restore. A graceful quit clears the marker immediately, so quitting inside the liveness window does not cost restore on the next launch.

Tests cover three behaviours through the real file-backed path: a completed restore leaves nothing behind, an unfinished restore is detected exactly once, and the recovered copy still holds the original workspaces after the primary snapshot has been overwritten.

Verification status

Please treat this as needing maintainer verification.

What is verified: the Swift side of the app compiles cleanly with these changes applied on top of 5734a451c (the commit before 04ff18eea introduced the file that currently breaks main). The three commits cherry-pick onto that base without conflict.

What is not verified: the runtime behaviour. The new tests have not been executed, and the hang itself has not been confirmed fixed on a live many-pane workspace.

Two corrections worth stating plainly, since an earlier revision of this description overstated things:

  • I previously claimed AppDelegate.swift type-checked cleanly against the real module graph. That was wrong. On main the build dies on TerminalController+MobileSurfaces.swift before this file finishes type-checking, so a compile-batch line was not evidence of success. Building from a base that actually compiles caught a genuine bug in this change — a call to a nonexistent UpdateLogStore.shared — fixed in the third commit.
  • The one inference behind the first commit that I still cannot confirm by running: that -[NSView _setDefaultKeyViewLoop] is gated by autorecalculatesKeyViewLoop. That is the documented purpose of the flag and both hang stacks are consistent with it, but a runtime check on a many-pane workspace would be worth doing before merging.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added protection against interrupted session restores.
    • Preserves affected session snapshots for recovery.
    • Automatically skips problematic restores on the next launch.
  • Bug Fixes
    • Improved startup reliability after crashes or unexpected termination during session restoration.
  • Tests
    • Added coverage for restore completion, interruption handling, cleanup, and snapshot preservation.