Repair the remaining app-host failures and the shard-2 host relaunches

[teamleaderleo]

Fixes the app-host failures in shards 1, 2, 4, 5 and 6 of run 35743588302 on fix/app-host-green (head 5a92d7f882), plus the four XCTest-host relaunches in shard 2. Other agents own the remaining failures on that run.

Most of these share one cause. The app-host test process runs headless under xcodebuild test and is not the active app, so AppKit and WebKit withhold state that a test written on a developer's Mac takes for granted: NSApp.keyWindow is nil, a programmatic makeKeyAndOrderFront never makes a window key, NSWindow.occlusionState never carries .visible, and behind XCTest's shielding window WebKit suspends requestAnimationFrame entirely. Shard 4's own log prints the giveaway: windowIsKey: true, appIsActive: false. Four more are stale fixtures that encode a widget tree or a snapshot the product has since changed, and two are real product regressions.

No assertion is weakened anywhere in this PR. Several get stronger, and each is noted below.

Reconciled with the base branch (rebased onto e160e82af3a)

fix/app-host-green moved while this PR was open, and 87a70165b5b (test: activate the app host before waiting for terminal focus) landed a different remedy for the same key-window cause in the same helper, AppDelegate.focusTerminalForTesting. It activates the app host (NSApp.activate(ignoringOtherApps:)) and widens the readiness pump to 10 s, and it names exactly the three tests this PR's own key-window fix covered.

Dropped from this PR: MainWindowKeyStatusPin, and with it every change to cmuxTests/AppDelegateMainWindowTestingSupport.swift — that file is now untouched by this branch. Two reasons, beyond the base's approach already being on the shared branch:

• Activating the host makes isKeyWindow actually true, so the product's focus gates run on real state. The pin forced the answer through a swizzle of NSWindow.isKeyWindow, which leaves everything else the window reports (NSApp.keyWindow, first-responder promotion, key/main notifications) still telling the un-activated truth.
• The pin's swizzle is installed once and never removed, process-wide, for the benefit of one helper — a poor neighbour to the ~hundreds of other cases sharing that app host.

Keeping both would have meant two mechanisms doing one job, which is how a fixed test becomes a flaky one. The shard-1 and shard-2 focus entries below are therefore no longer this PR's fixes; they are 87a70165b5b's, and are kept here only as the record of what was diagnosed.

a682c55a75a (give standalone terminal fixtures a live portal authority) also lands on the same root cause, but in TerminalNotificationDirectInteractionTests, on three shard-4 tests this PR never claimed. It composes with this branch's GhosttySurfaceOverlayTests edits in the same file with no overlap; git merged them cleanly and both are kept.

Everything else in this PR — the four shard-2 host relaunches, the two product regressions, and the stale-fixture and test-bug repairs in shards 2, 4, 5 and 6 — is unaffected and stands as written.

The 4 test-host relaunches in shard 2

Shard 2 launched the XCTest host 5 times — 1 initial plus 4 relaunches (SocketControlServer: Listening appears 5 times; Restarting after unexpected exit, crash, or test timeout 4 times, and its 5th occurrence in the log is the shard's own incomplete app-host test run: verdict quoting that message). Per #13178/#13421 a relaunch is itself a shard failure, so this mattered as much as the assertions.

Mapping each relaunch to the test that was running and never reported a result gives four tests, all in MarkdownPanelTests, each hanging ~5m10s and then killed by XCTest's execution-time allowance:

relaunch	test running	started	host killed
1	testMarkdownRenderBlocksRemoteImagesUntilUserAction	17:13:10	17:18:22
2	testMarkdownRenderHandlesLocalImageSources	17:18:24	17:23:32
3	testMarkdownRenderKeepsVisibleHeadingPositionAfterContentUpdate	17:23:34	17:28:44
4	testMarkdownRenderLoadsSafeDataImage	17:28:46	17:33:54

There is no bystander here: all four are the same gate, and each hung again at the top of a fresh host. cd9f34ff1b rewrote the private helper renderMarkdown(_:in:) to await two animation frames inside callAsyncJavaScript:

window.__cmuxRenderMarkdown(markdown);
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));

App-host tests run behind XCTest's shielding window (Accessibility: Not vending elements because elementWindow(0) is lower than shield(2001)), where WebKit suspends the rendering update and animation-frame callbacks never fire. This repo already documents that, at cmuxTests/BrowserViewportRuntimeTests.swift:404-408. The promise never settles, WebKit never posts the reply, the continuation is never resumed, and there is nothing to cancel.

The correlation is exact: renderMarkdown is the first WebKit interaction after the shell load in those four tests and in no other, and testMarkdownRenderDeniesLocalImageWhenMarkdownPathIsMissing — the one render-family test that does not call it — passed in 0.083 s.

Worth recording, since it is the obvious wrong answer: the Could not signal service com.apple.WebKit.WebContent: 113 lines in the log are not the cause. Those 25 lines cluster at 17:13 and from 17:34 on, with none between 17:18:24 and 17:33:54 — the window holding three of the four hangs. Every fulfillment(of: [loaded], timeout: 5) succeeded, so WebKit loaded and parsed the 268 KB shell in all four cases, and the webContentProcessDidTerminate bursts come from recovery tests driving the coordinator's state machine directly, all of which passed.

Fix, in three parts.

Fail fast. Every WebKit call in MarkdownPanelTests.swift now goes through awaitingWebKitReply(_:timeout:operation:). The awaited value is still the real completion signal and returns the instant the reply arrives; the deadline only decides how a missing reply is reported — an explicit XCTFail naming the operation. The shell-load wait likewise reports MarkdownShellNeverLoaded instead of walking into the next WebKit call. One loud failure costs one test; a hung host costs the shard.

Remove the gate. renderMarkdown now waits on the viewer's own render contract. window.__cmuxRenderMarkdown parses, replaces #content, rewrites image sources and applies the scroll restore before it returns — exactly what MarkdownWebRenderer.Coordinator.pushMarkdown waits for before reporting onMarkdownRendered. No animation frame is involved.

Fix the product bug the rAF await was papering over. restoreMarkdownScrollState re-applied the captured position on its next two animation frames unconditionally, so a scroll that landed in that window — the reader's, a find match, an anchor jump — was yanked back, and a new content update never cancelled the previous one's queued frames. The restore now records the position it wrote and stands down on a scroll event that leaves the viewer somewhere else, while a reflow (which fires no scroll event) still gets the correction the frames exist for. The synchronous pass stays authoritative.

A pre-existing Task.sleep(100ms)-then-assert inside testMarkdownRenderBlocksRemoteImagesUntilUserAction is also gone, replaced by a bounded wait on the real DOM predicate.

Shard 1

plainTerminalTextDoesNotResolveAppShortcutContext() — host dependency. Now fixed on the base branch by 87a70165b5b; this PR no longer changes anything for it. Diagnosis, for the record: 67d6b2a705 added focusTerminalForTesting, whose readiness wait includes window.isKeyWindow. The window comes from AppDelegate.createMainWindow(), so it is a production CmuxMainWindow, and a headless, inactive app never makes one key — the test passed or failed by whether an earlier test in the shard had happened to activate the app. This is the same failure mode 30590ad05f documented and fixed elsewhere with KeyStatusTestWindow, which cannot be used here because CmuxMainWindow is final. This PR originally added a test-target MainWindowKeyStatusPin (a one-time exchange of NSWindow.isKeyWindow consulting a per-instance flag); the base branch instead activates the host, which makes the same state true rather than forcing the answer. The pin is dropped — see Reconciled with the base branch above.

AppDelegateShortcutRoutingTests.testTextBoxSubmitKeepsQueuedRunForStillActiveSurfaceWhenAnotherSurfaceFinishes — host dependency, plus order-dependence on shared static state. The failure shape (sentKeys == [], sentText == []) is only reachable while pasteFilePath's lease.waitUntilApplied() is still pending. That lease drains through beginPreviousContentsCapture, which since 31fd3b9c77 (#8838) re-execs the app binary as a helper process. This was the batch's first .pasteFilePath run, i.e. a cold spawn, on a host that in the same log window took 16.7 s to launch a GPU process. The 5 s waitFor deadline was sized for a warm spawn. The test now waits on the real signal — the fake surface's binding callback, which fires after sentKeys is recorded — with the timeout bounding only the failure path, and calls TextBoxSubmit.debugResetForTesting() at the top the way its sibling already does (e6926fbbda). Stronger: completions == ["finishing"] is now asserted rather than only polled.

Shard 2

keyboardCopyModeKeyClearsTerminalUnread(), workspaceFontSizeShortcutPreservesBackgroundTerminalUnread() — host dependency. Same focusTerminalForTesting key-window gate as above. Also now fixed on the base branch by 87a70165b5b; this PR no longer changes anything for them.

GhosttySurfaceOverlayTests.testPreferredScrollerStyleChangePreservesSystemScrollbarStyle — stale test. The expectation dates from 6d5e284fe3, when scroller presence did not depend on scroller style. fe8872e8a8 (#12918) and 9f29ddfc77 (#12903) introduced TerminalScrollBarPresencePolicy: a legacy scroller is always present, an overlay scroller only with scrollback. The test set scrollerStyle = .legacy by hand and snapshotted contentSize.width before the product re-evaluated presence, capturing the full 360 pt instead of the 345 pt the legacy gutter leaves — so it contradicted its own neighbouring assertions. Stronger: the snapshot is replaced by initialContentWidth - NSScroller.scrollerWidth(for:scrollerStyle: .legacy), a host-independent statement of the invariant.

GhosttySurfaceOverlayTests.testSearchOverlaySurvivesPortalVisibilityToggleDuringWorkspaceSwitchLikeChurn — test bug. setSearchOverlay mounts through a deferred main-actor scheduler (f003c63bae, #9179), and the test used a fixed RunLoop.run(until: +0.05) spin. 9f258dd7c7 converted the sibling test to the class waitUntil helper but missed this one; the three later failures were cascades of the first. Now waits on the same real predicate.

TabManagerCloseCurrentPanelTests.testCloseCurrentPanelClearsNotificationsForClosedSurface — test bug. The workspace had a single surface, and with close-on-last-surface enabled (the default) the shortcut escalates to the window-close path, which a window-less test TabManager cannot perform — so nothing closed and nothing was cleared. The workspace now has a second surface, so the shortcut closes the surface itself, which is what the test is about. Stronger: XCTAssertNil(workspace.panels[initialPanelId]) now asserts the close actually happened, and the confirm-close handler fails the test if it is consulted.

pairingTicketUsesThePublishedV2InstallationIdentity() — test bug / shared-process ordering dependency. The reported error is noRoutes, not the routeUnavailable the test expects, which locates it precisely: createAttachTicket reads the route snapshot before the identity guard, so the route cache was empty. AppDelegate.installMobileHostSettingsObserver() registers a UserDefaults.didChangeNotification observer with object: nil, so any defaults write anywhere in the process schedules MobileHostService.shared.syncToSettings(), which with networking disallowed runs MobileHostIrxRuntime.prepareForStop() → MobileHostPublicStatusCache.removeAll(). The test parked a fixture publication in that process-wide static and then crossed two suspension points; in shard 2 the suite running immediately before is MobileHostServiceSettingsTests, which writes and removes defaults in three parameterised cases. Nothing in the product path changed, and no amount of re-publishing inside the test closes the window. The test now publishes and resolves within a single main-actor turn. Route+identity resolution is extracted into MobileHostService.attachTicketSubject(publishedStatus:routeID:routeKind:target:) — ordinary decomposition with one production caller, no debug naming and no #if DEBUG — which the test drives directly. Separately, and as a real (if latent) product correctness fix, MobileHostPublicStatusCache.publishedStatus() now reads the routes and the v2 identity under one lock; reading snapshot() and currentV2DeviceID() separately could straddle a republish and bind fresh routes to a retired identity. Stronger: the test now also asserts published.routes.contains(route), the selected route kinds, and ticket.macDeviceID, on top of the existing v2 pairing-URL decode.

WorkspacePanelGitBranchTests.testForkAgentWorkspaceLaunchFromPersistentSSHPTYDoesNotReuseParentRelayOrDaemonSlot — test bug. A forked configuration only mints a fresh relay namespace when the control listener can name the socket the new session reconnects through; that comes from the process-wide TerminalController.shared, so the test inherited whatever an earlier test left behind. It now reserves its own startup socket and tears it down. Stronger: XCTAssertEqual(localSocketPath, reservedSocket) pins the new namespace to the reserved socket instead of only asserting it differs from the parent's.

Shard 4

AppDelegateShortcutRoutingTests.testCustomCmdTNewWorkspacePrunesOrphanedContextWithoutLiveWindow — product regression. 6d7b23fbdb changed preferredMainWindowContextForWorkspaceCreation to skip the windowless-context prune, guarding it with (event == nil || eventContext != nil). Its own comment says "Windowless app shortcuts still prune as before", but the condition gates on whether a context resolved, not on whether the event names a window. A responder-chain keyDown with windowNumber == 0 — exactly what a remapped Cmd+T produces — has eventContext == nil, so the stale active context was never pruned and tabManagerFor(windowId:) kept returning it. The prune is now deferred only when the event is addressable and its context failed to resolve, using the existing shortcutEventHasAddressableWindow(_:) predicate. The reindexing-race deferral the guard exists for is preserved; windowless events prune again, as the commit intended.

Shard 5

"Reconnect card and controls stay inside a narrow Cloud split" — stale test. 9bf6cb8c94 (#12609) replaced the AppKit reconnect card — an NSVisualEffectView over an NSStackView of NSTextField/NSButton — with a single SwiftUI hosting view, so overlay.subviews.first as? NSVisualEffectView can never succeed and there is no NSButton to click. The product is correct; the test encoded the old widget tree. The card is now located by an accessibility identifier the overlay sets on it, and Retry is exercised through the same closure the control invokes. Stronger: the narrow-split sweep now runs for both progress states, containment is checked over every AppKit descendant rather than only NSControls, card width is asserted to keep its margin at every pane width and to actually track the pane (190 < 320 ≤ 720) rather than clamping, and a negative case asserts no Retry wiring when the presentation offers none.

browserPanelRetriesDiscardedRestoreAfterConnectionRefused() — host dependency. This one deserves the detail, since the brief asked for a decision. It was synchronising on WKWebView.isLoading, which is a state flag rather than a completion signal. b3755b8523 had already swapped one such wait for another: it stopped waiting 30 s for WebKit's own provisional failure for the reserved loopback port and instead waited 10 s for stopLoading() to clear isLoading, which on these runners it does not do — and the same !panel.webView.isLoading term still gated the surviving 20 s and 30 s waits, so deleting only the 10 s guard would have moved the timeout rather than removed it. None of that is what the case is about: it verifies that a connection-refused discard-restore leaves the pane retryable on the next restore touch, and that bookkeeping is driven entirely by the WKNavigationDelegate terminal callback. It did not need to move lanes. The test now installs a fake navigation source for the panel's web view — the BrowserReloadRecordingWebView pattern BrowserFailedNavigationReloadTests already uses — and reports the refusal through didFailProvisionalNavigation. The panel's restore bookkeeping, error page and retry policy run unchanged. The case now contains no timeouts, no polling and no async. Stronger: four assertions added, including the recorded request URL on both navigations, errorPageLoadCount == 1, and restore_pending == true immediately after the first restore, which the polling version could never observe.

Shard 6

"A late creation preserves newer workspace navigation" — host dependency. 408eddd384 (#13193) gated the late-creation focus steal on window === NSApp.keyWindow, which is nil in this process. That single fact explains all three recorded issues: line 150 fails for both arguments, and line 167 fails only for navigate=false because the guard bails and selection never moves — a cascade, not a regression (the navigate=true path is genuinely suppressed by the cloudWorkspaceSelection.revision guard). The product now asks the window (window.isKeyWindow) rather than the AppKit global, through a named predicate cloudWorkspaceCreationFocusWindow(windowID:) — equivalent in the running app, and the idiom used by 66 other gates in Sources/. The test uses KeyStatusTestWindow and asserts the product's own answer, which still covers context registration, window resolution and key status.

"A surviving Simulator host keeps framebuffer publication active" — host dependency. Frame demand is gated on window.occlusionState.contains(.visible) (aac235180e, #7857), a bit the window server sets only for the active app's on-screen windows. The pane therefore never took frame demand, the emitted transport was dropped, and line 102 passed vacuously because publication was never turned on. Occlusion-gating the framebuffer is the feature, so no product change: the test's host window now pins the on-screen occlusion state, mirroring the KeyStatusTestWindow precedent, and all three expectations exercise the coordinator's per-observer host refcounting.

Checks

Re-run after the rebase onto e160e82af3a: python3 scripts/check-test-determinism.py (0 findings), ./tests/test_ci_pbxproj_test_wiring.sh (18 tests, ok), python3 scripts/ci/validate_test_execution_registry.py (valid, 223 tests), and the five guard-structure suites tests/test_ci_{app_host,quality,release,source_lint}_guard_structure.py and tests/test_ci_workflow_guards_are_wired.py (6 passed). Every changed Swift file also passes swiftc -parse. No .pbxproj change was needed and no file was added, so nothing needs target membership or registry wiring. Per the repo's rules nothing here uses a sleep or a state-flag poll as a synchronization primitive; the only remaining Task.sleep calls are watchdog deadlines on failure paths and poll intervals inside deadline-bounded waits on real predicates. No full local app build was run, so none of these fixes is executed here — CI on this PR is the first execution.

🤖 Generated with Claude Code

---

Summary by cubic

Fixes the remaining app-host failures in shards 1, 2, 4, 5 and 6, including the four shard-2 XCTest-host relaunches. Most trace to one cause: the headless test process is never the active app, so AppKit reports no key window or visible occlusion and WebKit never fires animation frames behind XCTest's shielding window.

Host-dependency fixes

• MarkdownPanelTests WebKit calls now fail at a deadline with a named reason instead of hanging the whole host; renderMarkdown waits on the viewer's own synchronous render contract, and the scroll restore no longer clobbers a scroll that lands after a content update.
• Focus and occlusion gates now ask the window itself (a key-status pin, OnScreenTestWindow, or a named predicate) instead of NSApp.keyWindow or the window server.
• The reconnect-card test finds the SwiftUI card by the overlay's accessibility identifier; the connection-refused restore test reports the failure through a fake navigation source, dropping its timeouts and polling.

Product fixes and remaining test bugs

• Windowless shortcut events prune the orphaned main-window context again; the deferral now applies only to addressable, unresolved events.
• Ticket minting reads routes and the v2 installation identity under one lock, so fresh routes can't bind to a retired identity.
• TabManagerCloseCurrentPanelTests gives the workspace a second surface so the shortcut closes a surface; the fork-relay, search-overlay, and scroller-style tests reserve or wait on deterministic state instead of inheriting ambient test order.
• No assertion is weakened anywhere; several are strengthened.

^{Written for commit e353dd5. Summary will update on new commits.}

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8624eb4b-b1f1-4bb1-a198-4091413e4c3c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}