From 732b8980e9e0521a1c021943014813e8dbc3bad2 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 22 Sep 2026 23:12:40 -0700 Subject: [PATCH 1/6] test: read the published layout cache after driving split geometry `testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers` compared the cached `workspace.tmuxLayoutSnapshot` against the controller's live tree and read the cache synchronously, one statement after driving geometry. Since a27969a38b the geometry callback publishes that cache through `geometryNotificationScheduler` rather than assigning it in the synchronous `splitTabBar(_:didChangeGeometry:)` body, so both reads saw the previous value -- at the start of a workspace, the one-pane snapshot written at init. The failure reported one pane id against three and frame deltas that are the live equalized geometry (161.33333 = 484/3). Wait for the cache to catch up to the live tree before each read, the way `PaneResizeShortcutTests.expectCachedFramesMatch` already does for the same publication change. The live tree was never wrong: the tree assertions above these reads pass. Co-Authored-By: Claude Opus 5 --- ...pDelegateEqualizeSplitsShortcutTests.swift | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index 0aa94215199d..05f1743f7335 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -492,7 +492,7 @@ final class AppDelegateEqualizeSplitsShortcutTests { } workspace.splitTabBar(workspace.bonsplitController, didChangeGeometry: workspace.bonsplitController.layoutSnapshot()) - guard let seededLayoutSnapshot = workspace.tmuxLayoutSnapshot else { + guard let seededLayoutSnapshot = shortcutRoutingWaitForPublishedLayout(workspace) else { XCTFail("Expected cached layout snapshot after seeding split geometry") return } @@ -527,11 +527,11 @@ final class AppDelegateEqualizeSplitsShortcutTests { XCTAssertEqual(split.dividerPosition, expectedPosition, accuracy: 0.000_1) } - let liveEqualizedLayout = workspace.bonsplitController.layoutSnapshot() - guard let cachedEqualizedLayout = workspace.tmuxLayoutSnapshot else { + guard let cachedEqualizedLayout = shortcutRoutingWaitForPublishedLayout(workspace) else { XCTFail("Expected cached layout snapshot after equalizing split geometry") return } + let liveEqualizedLayout = workspace.bonsplitController.layoutSnapshot() XCTAssertNotEqual( shortcutRoutingPaneFramesById(in: seededLayoutSnapshot), shortcutRoutingPaneFramesById(in: liveEqualizedLayout) @@ -8019,6 +8019,31 @@ final class AppDelegateEqualizeSplitsShortcutTests { Dictionary(uniqueKeysWithValues: snapshot.panes.map { ($0.paneId, $0.frame) }) } + /// The cached snapshot that catches up to the controller's live tree, or + /// the stale cache once the deadline passes so the caller's assertion + /// reports the drift. + /// + /// Since a27969a38b the geometry callback publishes `tmuxLayoutSnapshot` + /// through `geometryNotificationScheduler` instead of assigning it in the + /// synchronous `splitTabBar(_:didChangeGeometry:)` body, so a read taken + /// right after driving geometry still sees the previous cache — at the + /// start of a workspace, the one-pane value written at init. + /// `PaneResizeShortcutTests.expectCachedFramesMatch` waits the same way. + private func shortcutRoutingWaitForPublishedLayout( + _ workspace: Workspace, + timeout: TimeInterval = 3 + ) -> LayoutSnapshot? { + let deadline = Date(timeIntervalSinceNow: timeout) + while Date() < deadline { + let live = workspace.bonsplitController.layoutSnapshot() + if let cached = workspace.tmuxLayoutSnapshot, cached.panes == live.panes { + return cached + } + RunLoop.main.run(mode: .default, before: Date(timeIntervalSinceNow: 0.01)) + } + return workspace.tmuxLayoutSnapshot + } + private func shortcutRoutingAssertPaneFramesMatch( _ lhs: LayoutSnapshot, _ rhs: LayoutSnapshot, From 64da62a8e7588629cf56e272ed223f8394176897 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 03:47:48 -0700 Subject: [PATCH 2/6] test: await the geometry publish instead of pumping the run loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous attempt waited for `tmuxLayoutSnapshot` with `RunLoop.main.run(mode:before:)`, and CI showed it never succeeding: the assertion still compared a 1-pane cache against the live 3-pane tree, only with the line shifted. `splitTabBar(_:didChangeGeometry:)` hands its work to `geometryNotificationScheduler.schedule(zeroDelayPolicy: .yieldOnce)`, which is `Task { await Task.yield(); action() }` on the MainActor executor. A synchronous `@Test` body owns that executor for its whole duration, so the continuation cannot run at all — spinning the run loop does not drain the MainActor's cooperative executor. The cache therefore keeps the one-pane value written at workspace init, and no timeout value could ever have worked. Make the case `async`, wrap it in `AppContextSerialGate.withExclusiveAppContext` (the pattern this file's other async cases use, since async tests in different suites interleave at suspension points and this one swaps `AppDelegate.shared`), and await `Task.yield()` between polls so the scheduled publish can actually run. The wait's failure now names the publish rather than returning a stale snapshot for the caller's assertion to report as a frame mismatch. Co-Authored-By: Claude Opus 5 --- ...pDelegateEqualizeSplitsShortcutTests.swift | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index 05f1743f7335..9bc846fd0307 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -444,7 +444,16 @@ final class AppDelegateEqualizeSplitsShortcutTests { } @Test - func testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers() { + func testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers() async { + await AppContextSerialGate.withExclusiveAppContext { + await self.equalizeSplitsShortcutBalancesWorkspaceDividersBody() + } + } + + /// Body of the above. Split out so the gate wraps exactly one `@MainActor` + /// async closure rather than the whole `@Test` attribute surface. + @MainActor + private func equalizeSplitsShortcutBalancesWorkspaceDividersBody() async { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") return @@ -492,8 +501,8 @@ final class AppDelegateEqualizeSplitsShortcutTests { } workspace.splitTabBar(workspace.bonsplitController, didChangeGeometry: workspace.bonsplitController.layoutSnapshot()) - guard let seededLayoutSnapshot = shortcutRoutingWaitForPublishedLayout(workspace) else { - XCTFail("Expected cached layout snapshot after seeding split geometry") + guard let seededLayoutSnapshot = await shortcutRoutingAwaitPublishedLayout(workspace) else { + XCTFail("tmuxLayoutSnapshot never caught up to the seeded 3-pane tree; the geometry publish Task did not run") return } let expectedEqualizedPositions = shortcutRoutingExpectedEqualizedDividerPositions( @@ -527,8 +536,8 @@ final class AppDelegateEqualizeSplitsShortcutTests { XCTAssertEqual(split.dividerPosition, expectedPosition, accuracy: 0.000_1) } - guard let cachedEqualizedLayout = shortcutRoutingWaitForPublishedLayout(workspace) else { - XCTFail("Expected cached layout snapshot after equalizing split geometry") + guard let cachedEqualizedLayout = await shortcutRoutingAwaitPublishedLayout(workspace) else { + XCTFail("tmuxLayoutSnapshot never caught up to the equalized tree; the geometry publish Task did not run") return } let liveEqualizedLayout = workspace.bonsplitController.layoutSnapshot() @@ -8019,29 +8028,29 @@ final class AppDelegateEqualizeSplitsShortcutTests { Dictionary(uniqueKeysWithValues: snapshot.panes.map { ($0.paneId, $0.frame) }) } - /// The cached snapshot that catches up to the controller's live tree, or - /// the stale cache once the deadline passes so the caller's assertion - /// reports the drift. + /// The published `tmuxLayoutSnapshot` once it catches up to the live tree. /// - /// Since a27969a38b the geometry callback publishes `tmuxLayoutSnapshot` - /// through `geometryNotificationScheduler` instead of assigning it in the - /// synchronous `splitTabBar(_:didChangeGeometry:)` body, so a read taken - /// right after driving geometry still sees the previous cache — at the - /// start of a workspace, the one-pane value written at init. - /// `PaneResizeShortcutTests.expectCachedFramesMatch` waits the same way. - private func shortcutRoutingWaitForPublishedLayout( + /// Since a27969a38b the geometry callback hands its work to + /// `geometryNotificationScheduler.schedule(zeroDelayPolicy: .yieldOnce)`, + /// which is `Task { await Task.yield(); action() }` on the MainActor + /// executor. A synchronous test body owns that executor for its whole + /// duration, so the continuation cannot run and the cache keeps the + /// one-pane value written at workspace init — no amount of + /// `RunLoop.main.run` helps, because spinning the run loop does not let + /// the MainActor's cooperative executor drain queued tasks. The caller + /// must be `async` and this must suspend. + private func shortcutRoutingAwaitPublishedLayout( _ workspace: Workspace, - timeout: TimeInterval = 3 - ) -> LayoutSnapshot? { - let deadline = Date(timeIntervalSinceNow: timeout) - while Date() < deadline { + yields: Int = 200 + ) async -> LayoutSnapshot? { + for _ in 0.. Date: Wed, 23 Sep 2026 04:03:31 -0700 Subject: [PATCH 3/6] test: bound the equalize-splits publish wait by a deadline Independent review flagged three problems with the first pass. The wait used a fixed 200-iteration yield loop. .github/review-bot-rules/test-determinism.md asks for a deadline-bounded poll so the bound constrains the failure path only, and PaneResizeShortcutTests.swift:150 already waits on this exact predicate with AppKitTestEventPump().waitUntil(timeout:). Use the pump. The final frame comparison was a tautology: the wait exited on cached.panes == live.panes and the live snapshot was re-read with no suspension in between, so the assertion could not fail. The second wait now blocks until the cache leaves the seeded geometry, which leaves the comparison able to fail if the publish lands the wrong snapshot. The helper's doc comment claimed RunLoop.main.run can never drain the MainActor executor. That is overstated - the executor enqueues to the main dispatch queue, which is what AppKitTestEventPump.drain() resumes through. The real defect was that the pre-async read had no suspension point at all, so it could only observe the value written at workspace init. Say that instead. Co-Authored-By: Claude Opus 5 --- ...pDelegateEqualizeSplitsShortcutTests.swift | 46 ++++++++++++------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index 9bc846fd0307..759991742036 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -501,7 +501,9 @@ final class AppDelegateEqualizeSplitsShortcutTests { } workspace.splitTabBar(workspace.bonsplitController, didChangeGeometry: workspace.bonsplitController.layoutSnapshot()) - guard let seededLayoutSnapshot = await shortcutRoutingAwaitPublishedLayout(workspace) else { + guard let seededLayoutSnapshot = await shortcutRoutingAwaitPublishedLayout(workspace, until: { + $0.panes == workspace.bonsplitController.layoutSnapshot().panes + }) else { XCTFail("tmuxLayoutSnapshot never caught up to the seeded 3-pane tree; the geometry publish Task did not run") return } @@ -536,8 +538,15 @@ final class AppDelegateEqualizeSplitsShortcutTests { XCTAssertEqual(split.dividerPosition, expectedPosition, accuracy: 0.000_1) } - guard let cachedEqualizedLayout = await shortcutRoutingAwaitPublishedLayout(workspace) else { - XCTFail("tmuxLayoutSnapshot never caught up to the equalized tree; the geometry publish Task did not run") + // Wait for the equalize to be published rather than for the cache to + // match the live tree: waiting on equality would make the frame + // comparison below true by construction. Waiting for the cache to + // leave the seeded geometry keeps that comparison able to fail if the + // publish lands the wrong snapshot. + guard let cachedEqualizedLayout = await shortcutRoutingAwaitPublishedLayout(workspace, until: { + $0.panes != seededLayoutSnapshot.panes + }) else { + XCTFail("tmuxLayoutSnapshot never left the seeded geometry; the geometry publish Task did not run") return } let liveEqualizedLayout = workspace.bonsplitController.layoutSnapshot() @@ -8028,29 +8037,32 @@ final class AppDelegateEqualizeSplitsShortcutTests { Dictionary(uniqueKeysWithValues: snapshot.panes.map { ($0.paneId, $0.frame) }) } - /// The published `tmuxLayoutSnapshot` once it catches up to the live tree. + /// The published `tmuxLayoutSnapshot` once it satisfies `predicate`. /// /// Since a27969a38b the geometry callback hands its work to /// `geometryNotificationScheduler.schedule(zeroDelayPolicy: .yieldOnce)`, /// which is `Task { await Task.yield(); action() }` on the MainActor - /// executor. A synchronous test body owns that executor for its whole - /// duration, so the continuation cannot run and the cache keeps the - /// one-pane value written at workspace init — no amount of - /// `RunLoop.main.run` helps, because spinning the run loop does not let - /// the MainActor's cooperative executor drain queued tasks. The caller - /// must be `async` and this must suspend. + /// executor. The pre-`async` version of this case read + /// `tmuxLayoutSnapshot` with no suspension point after the geometry call, + /// so it could only ever observe the one-pane value written at workspace + /// init. The caller must be `async` and this must suspend. + /// + /// The deadline bounds the failure path only: a publish that lands + /// promptly returns on the first drain. private func shortcutRoutingAwaitPublishedLayout( _ workspace: Workspace, - yields: Int = 200 + timeout: Duration = .seconds(3), + until predicate: (LayoutSnapshot) -> Bool ) async -> LayoutSnapshot? { - for _ in 0.. Date: Wed, 23 Sep 2026 17:48:47 -0700 Subject: [PATCH 4/6] test: give the undo-routing test a key window and the font barrier real turns terminalHostedEditableResponderKeepsLocalUndo passes on Warp and fails on every Blacksmith run (undoCallCount 0): the app host is never active there, so its plain NSWindow never goes key and AppKit does not dispatch the menu key equivalent. The test is about cmux routing ownership, so it now uses KeyStatusTestWindow, as the other key-dependent tests do. testGhosttyAppConfigUpdateWaitsForFontBarrier asserted that no config update had published straight after reloadConfiguration. The publish is asynchronous, so that check passed whether or not the barrier held. It now pumps main-actor turns for 500 ms while font work still holds the barrier. Co-Authored-By: Claude Opus 5.5 --- ...pDelegateEqualizeSplitsShortcutTests.swift | 8 ++++++- cmuxTests/WindowKeyDownReplayGuardTests.swift | 24 ++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index 95059e43ae41..ff8dc83dfe02 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -6374,8 +6374,14 @@ final class AppDelegateEqualizeSplitsShortcutTests { didCommitGhosttyAppConfig, "The app config must not commit before font work finishes" ) + // The reload publishes on later main-actor turns, so a check made + // straight after the call passes whether or not the barrier holds. + // Give it those turns while font work still holds the barrier. + let publishedBeforeFontWork = await AppKitTestEventPump().waitUntil( + timeout: .milliseconds(500) + ) { didUpdateGhosttyAppConfig } XCTAssertFalse( - didUpdateGhosttyAppConfig, + publishedBeforeFontWork, "The app config update itself must wait behind font work" ) XCTAssertGreaterThan(scheduler.delays.count, 2) diff --git a/cmuxTests/WindowKeyDownReplayGuardTests.swift b/cmuxTests/WindowKeyDownReplayGuardTests.swift index cc33ab2ab65c..8df589b4d4ba 100644 --- a/cmuxTests/WindowKeyDownReplayGuardTests.swift +++ b/cmuxTests/WindowKeyDownReplayGuardTests.swift @@ -122,14 +122,13 @@ struct WindowKeyDownReplayGuardTests { return (window, terminal) } - private func makeWindowWithTerminalHostedEditableResponder() - -> (NSWindow, TerminalCommandEquivalentProbeView, EditableUndoProbeTextView) { - let window = NSWindow( - contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), - styleMask: [.titled, .closable], - backing: .buffered, - defer: false - ) + private func makeWindowWithTerminalHostedEditableResponder( + reportsKeyStatus: Bool = false + ) -> (NSWindow, TerminalCommandEquivalentProbeView, EditableUndoProbeTextView) { + let frame = NSRect(x: 0, y: 0, width: 640, height: 420) + let window: NSWindow = reportsKeyStatus + ? KeyStatusTestWindow(contentRect: frame, styleMask: [.titled, .closable], backing: .buffered, defer: false) + : NSWindow(contentRect: frame, styleMask: [.titled, .closable], backing: .buffered, defer: false) // AppKit defaults to isReleasedWhenClosed, so the callers' close() would release a // window ARC still owns and the over-release lands in a later autorelease pool drain. window.isReleasedWhenClosed = false @@ -429,9 +428,12 @@ struct WindowKeyDownReplayGuardTests { _ = NSApplication.shared AppDelegate.installWindowResponderSwizzlesForTesting() - let (window, terminal, textView) = makeWindowWithTerminalHostedEditableResponder() - // This assertion is about cmux routing ownership. AppKit's nil-target - // lookup depends on NSApp.keyWindow, which headless test hosts may lack. + // This assertion is about cmux routing ownership. AppKit only runs a + // window's menu key equivalents while it is key, and the app host is + // never active on Blacksmith runners, so a plain window cannot reach + // the menu there (it can on Warp). AppKit's nil-target lookup depends + // on NSApp.keyWindow too, hence the explicit menu target below. + let (window, terminal, textView) = makeWindowWithTerminalHostedEditableResponder(reportsKeyStatus: true) let previousMenu = installResponderChainUndoMenu(target: textView) defer { NSApp.mainMenu = previousMenu } From 738f1eca28e870d0f1913cc44ae63e172bb6bda6 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 18:42:42 -0700 Subject: [PATCH 5/6] test: prove the reload is parked at the font barrier, not just slow The font-barrier test only showed that no config notification arrived within 500 ms. Record the reload's commit acknowledgement instead: everything up to the barrier runs synchronously and an unblocked reload commits before reloadConfiguration returns, so an uncommitted reload right after the call, and still after the wait, proves the barrier is holding it. The commit then lands once font work releases the barrier. Co-Authored-By: Claude Opus 5.5 --- .../AppDelegateEqualizeSplitsShortcutTests.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index ff8dc83dfe02..fe9d059cb63c 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -6352,7 +6352,9 @@ final class AppDelegateEqualizeSplitsShortcutTests { soft: true, source: "test.fontBarrier", reloadSettingsFromFile: false, - commitCompletion: { _ in didCommitGhosttyAppConfig = true } + commitCompletion: { committed in + didCommitGhosttyAppConfig = committed + } ) // Held, not merely not yet run: the transaction is parked at the // font-work barrier, and giving the main actor turns does not move it. @@ -6372,7 +6374,7 @@ final class AppDelegateEqualizeSplitsShortcutTests { #endif XCTAssertFalse( didCommitGhosttyAppConfig, - "The app config must not commit before font work finishes" + "The reload must be blocked at the font barrier" ) // The reload publishes on later main-actor turns, so a check made // straight after the call passes whether or not the barrier holds. @@ -6384,6 +6386,10 @@ final class AppDelegateEqualizeSplitsShortcutTests { publishedBeforeFontWork, "The app config update itself must wait behind font work" ) + XCTAssertFalse( + didCommitGhosttyAppConfig, + "The reload must stay blocked until font work releases the barrier" + ) XCTAssertGreaterThan(scheduler.delays.count, 2) if scheduler.delays.count > 2 { scheduler.fire(at: 2) @@ -6393,6 +6399,7 @@ final class AppDelegateEqualizeSplitsShortcutTests { // notification is published after its bounded surface fanout, which // runs on later main-actor turns. await waitWhileSuspended(for: [configUpdated], timeout: 5) + XCTAssertTrue(didCommitGhosttyAppConfig) XCTAssertTrue(didUpdateGhosttyAppConfig) } From fffdb7ef90727299be1729ec17af842daa208a63 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 19:06:31 -0700 Subject: [PATCH 6/6] test: check the ordering in the commit callback, not with a timed wait The 500 ms negative poll passed on its deadline. Asserting in commitCompletion that no update was published covers every publish that could happen while the barrier holds, with no clock involved. Co-Authored-By: Claude Opus 5.5 --- ...pDelegateEqualizeSplitsShortcutTests.swift | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift index fe9d059cb63c..839b61dc2cf0 100644 --- a/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift +++ b/cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift @@ -6354,6 +6354,12 @@ final class AppDelegateEqualizeSplitsShortcutTests { reloadSettingsFromFile: false, commitCompletion: { committed in didCommitGhosttyAppConfig = committed + // Anything published while the barrier held would already + // be recorded when the reload finally commits. + XCTAssertFalse( + didUpdateGhosttyAppConfig, + "The app config update itself must wait behind font work" + ) } ) // Held, not merely not yet run: the transaction is parked at the @@ -6376,20 +6382,6 @@ final class AppDelegateEqualizeSplitsShortcutTests { didCommitGhosttyAppConfig, "The reload must be blocked at the font barrier" ) - // The reload publishes on later main-actor turns, so a check made - // straight after the call passes whether or not the barrier holds. - // Give it those turns while font work still holds the barrier. - let publishedBeforeFontWork = await AppKitTestEventPump().waitUntil( - timeout: .milliseconds(500) - ) { didUpdateGhosttyAppConfig } - XCTAssertFalse( - publishedBeforeFontWork, - "The app config update itself must wait behind font work" - ) - XCTAssertFalse( - didCommitGhosttyAppConfig, - "The reload must stay blocked until font work releases the barrier" - ) XCTAssertGreaterThan(scheduler.delays.count, 2) if scheduler.delays.count > 2 { scheduler.fire(at: 2)