Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3058,6 +3058,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {

func applyWindowBackgroundIfActive() {
guard let window else { return }
// Always maintain the surface-level background to prevent blank panes.
// This is safe even when window-level background is skipped (idempotent).
applySurfaceBackground()
let appDelegate = AppDelegate.shared
let owningManager = tabId.flatMap { appDelegate?.tabManagerFor(tabId: $0) }
let owningSelectedTabId = owningManager?.selectedTabId
Expand All @@ -3070,7 +3073,6 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
) else {
return
}
applySurfaceBackground()
let color = effectiveBackgroundColor()
if cmuxShouldUseClearWindowBackground(for: color.alphaComponent) {
window.backgroundColor = cmuxTransparentWindowBaseColor()
Expand Down
7 changes: 6 additions & 1 deletion Sources/TerminalNotificationStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,12 @@ final class TerminalNotificationStore: ObservableObject {
}

if WorkspaceAutoReorderSettings.isEnabled() {
AppDelegate.shared?.tabManager?.moveTabToTop(tabId)
// Defer tab reordering to the next run loop iteration to avoid
// cascading SwiftUI re-renders (tabs + notifications @Published
// changes in the same frame) that can blank the active pane (#914).
DispatchQueue.main.async {
AppDelegate.shared?.tabManager?.moveTabToTop(tabId)
}
}

let notification = TerminalNotification(
Expand Down
77 changes: 77 additions & 0 deletions cmuxTests/GhosttyConfigTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,83 @@ final class WindowBackgroundSelectionGateTests: XCTestCase {
}
}

final class WindowBackgroundSurfaceBackgroundGuaranteeTests: XCTestCase {
func testShouldApplyRejectsWhenOwningSelectionDiffersButSurfaceMustStayVisible() {
let surfaceTabId = UUID()
let differentSelectedTab = UUID()

let rejected = !GhosttyNSView.shouldApplyWindowBackground(
surfaceTabId: surfaceTabId,
owningManagerExists: true,
owningSelectedTabId: differentSelectedTab,
activeSelectedTabId: surfaceTabId
)
XCTAssertTrue(rejected, "Guard should reject when owning selection differs — this is the #914 trigger condition")
}

func testShouldApplyRejectsWhenActiveSelectionDiffersWithNoOwningManager() {
let surfaceTabId = UUID()
let differentActiveTab = UUID()

let rejected = !GhosttyNSView.shouldApplyWindowBackground(
surfaceTabId: surfaceTabId,
owningManagerExists: false,
owningSelectedTabId: nil,
activeSelectedTabId: differentActiveTab
)
XCTAssertTrue(rejected, "Guard should reject when active selection differs — surface background must still be maintained")
}
}

final class NotificationTabReorderDeferralTests: XCTestCase {
func testMoveTabToTopIsDeferredInsideAddNotification() throws {
let projectRoot = findProjectRoot()
let storeURL = projectRoot.appendingPathComponent("Sources/TerminalNotificationStore.swift")
let source = try String(contentsOf: storeURL, encoding: .utf8)
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This test asserts source-code strings instead of runtime behavior, violating the repository test policy and creating brittle regression coverage.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmuxTests/GhosttyConfigTests.swift, line 807:

<comment>This test asserts source-code strings instead of runtime behavior, violating the repository test policy and creating brittle regression coverage.</comment>

<file context>
@@ -792,42 +792,76 @@ final class WindowBackgroundSurfaceBackgroundGuaranteeTests: XCTestCase {
+    func testMoveTabToTopIsDeferredInsideAddNotification() throws {
+        let projectRoot = findProjectRoot()
+        let storeURL = projectRoot.appendingPathComponent("Sources/TerminalNotificationStore.swift")
+        let source = try String(contentsOf: storeURL, encoding: .utf8)
 
-        // Capture the tab count before adding a notification.
</file context>
Fix with Cubic


guard let methodStart = source.range(of: "func addNotification(tabId:") else {
XCTFail("Failed to locate addNotification(tabId:) in TerminalNotificationStore.swift")
return
}

let searchRange = methodStart.upperBound..<source.endIndex
let methodEnd = source.range(of: "\n func ", range: searchRange)?.lowerBound ?? source.endIndex
let methodBody = String(source[methodStart.lowerBound..<methodEnd])

XCTAssertTrue(
methodBody.contains("DispatchQueue.main.async"),
"addNotification must defer moveTabToTop via DispatchQueue.main.async (#914)."
)
XCTAssertTrue(
methodBody.contains("moveTabToTop"),
"addNotification must call moveTabToTop for auto-reorder behavior."
)

if let reorderStart = methodBody.range(of: "WorkspaceAutoReorderSettings.isEnabled()") {
let reorderSection = String(methodBody[reorderStart.lowerBound..<methodBody.endIndex])
if let asyncStart = reorderSection.range(of: "DispatchQueue.main.async") {
let beforeAsync = String(reorderSection[reorderSection.startIndex..<asyncStart.lowerBound])
XCTAssertFalse(
beforeAsync.contains("moveTabToTop"),
"moveTabToTop must be inside the DispatchQueue.main.async block, not before it (#914)."
)
}
}
}

private func findProjectRoot() -> URL {
var dir = URL(fileURLWithPath: #file).deletingLastPathComponent().deletingLastPathComponent()
for _ in 0..<10 {
let marker = dir.appendingPathComponent("GhosttyTabs.xcodeproj")
if FileManager.default.fileExists(atPath: marker.path) {
return dir
}
dir = dir.deletingLastPathComponent()
}
return URL(fileURLWithPath: FileManager.default.currentDirectoryPath)
}
Comment on lines +780 to +826
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Test does not verify the claimed deferral behavior.

The test claims to validate that moveTabToTop() is NOT called synchronously during addNotification(), but it doesn't actually verify this:

  1. No observable state is captured before/after the synchronous call to detect if moveTabToTop fired early
  2. The expectation just proves DispatchQueue.main.async executes, which is always true
  3. There's no mock or spy to verify call ordering

The verbose comments (lines 797-806, 810-813, 822-828) describe the intent, but the actual test body doesn't enforce it. Per learnings, tests should verify observable runtime behavior through executable paths.

Consider either:

  • Mocking AppDelegate.shared?.tabManager to capture and verify call timing
  • Using an observable side effect (e.g., a counter or flag) that changes when moveTabToTop is called, then asserting it hasn't changed immediately after addNotification() returns but has changed after the expectation completes

Based on learnings: "Tests must verify observable runtime behavior through executable paths (unit/integration/e2e/CLI), not implementation shape".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/GhosttyConfigTests.swift` around lines 795 - 831, The test
TestMoveTabToTopIsNotCalledSynchronouslyDuringAddNotification doesn't actually
assert that moveTabToTop was deferred; update
NotificationTabReorderDeferralTests.testMoveTabToTopIsNotCalledSynchronouslyDuringAddNotification
to observe a concrete side-effect: install a spy or observable flag on the tab
manager (e.g., stub AppDelegate.shared?.tabManager or inject a test TabManager)
that flips a boolean or increments a counter when moveTabToTop() is called,
capture that value before calling
TerminalNotificationStore.addNotification(...), assert it is unchanged
immediately after addNotification returns, then schedule the existing
DispatchQueue.main.async expectation and assert the flag/counter changed after
the expectation completes; reference TerminalNotificationStore.addNotification
and moveTabToTop to locate the code under test.

}

final class NotificationBurstCoalescerTests: XCTestCase {
func testSignalsInSameBurstFlushOnce() {
let coalescer = NotificationBurstCoalescer(delay: 0.01)
Expand Down