Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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: 4 additions & 0 deletions GhosttyTabs.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
B9000023A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000022A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift */; };
B9000025A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000026A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift */; };
D0E0F0B0A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */; };
CC000000A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */; };
D0E0F0B2A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */; };
FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */; };
E1000000A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1000001A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift */; };
Expand Down Expand Up @@ -294,6 +295,7 @@
B9000022A1B2C3D4E5F60719 /* CloseWorkspaceCmdDUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloseWorkspaceCmdDUITests.swift; sourceTree = "<group>"; };
B9000026A1B2C3D4E5F60719 /* CloseWindowConfirmDialogUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloseWindowConfirmDialogUITests.swift; sourceTree = "<group>"; };
D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserPaneNavigationKeybindUITests.swift; sourceTree = "<group>"; };
CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GotoSplitCycleUITests.swift; sourceTree = "<group>"; };
D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserOmnibarSuggestionsUITests.swift; sourceTree = "<group>"; };
FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserImportProfilesUITests.swift; sourceTree = "<group>"; };
E1000001A1B2C3D4E5F60718 /* MenuKeyEquivalentRoutingUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuKeyEquivalentRoutingUITests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -570,6 +572,7 @@
C2577001A1B2C3D4E5F60718 /* TerminalCmdClickUITests.swift */,
E6FA9085A1B2C3D4E5F60718 /* WorkspaceDescriptionUITests.swift */,
D0E0F0B1A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift */,
CC000001A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift */,
D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */,
FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */,
C0B4D9B1A1B2C3D4E5F60718 /* UpdatePillUITests.swift */,
Expand Down Expand Up @@ -870,6 +873,7 @@
C2577000A1B2C3D4E5F60718 /* TerminalCmdClickUITests.swift in Sources */,
E6FA9084A1B2C3D4E5F60718 /* WorkspaceDescriptionUITests.swift in Sources */,
D0E0F0B0A1B2C3D4E5F60718 /* BrowserPaneNavigationKeybindUITests.swift in Sources */,
CC000000A1B2C3D4E5F60718 /* GotoSplitCycleUITests.swift in Sources */,
D0E0F0B2A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift in Sources */,
FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */,
C0B4D9B0A1B2C3D4E5F60718 /* UpdatePillUITests.swift in Sources */,
Expand Down
55 changes: 55 additions & 0 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8513,6 +8513,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}
guard let tabManager = self.tabManager else { return }

let layout = env["CMUX_UI_TEST_GOTO_SPLIT_LAYOUT"]?
.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""

if layout == "three_pane_terminal" {
self.setupThreePaneTerminalLayout(tabManager: tabManager)
return
}

let tab = tabManager.addTab()
guard let initialPanelId = tab.focusedPanelId else {
self.writeGotoSplitTestData(["setupError": "Missing initial panel id"])
Expand Down Expand Up @@ -8548,6 +8556,44 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}
}

/// Create a 3-pane terminal-only layout: one horizontal split (right) and one vertical split (down).
/// Used by `CMUX_UI_TEST_GOTO_SPLIT_LAYOUT=three_pane_terminal`.
/// Focus changes are recorded by `recordGotoSplitCycleMoveIfNeeded` in the Ghostty action handler.
private func setupThreePaneTerminalLayout(tabManager: TabManager) {
let tab = tabManager.addTab()
guard let initialPanelId = tab.focusedPanelId else {
writeGotoSplitTestData(["setupError": "Missing initial panel id"])
return
}

// Create horizontal split (right)
guard tabManager.createSplit(
tabId: tab.id, surfaceId: initialPanelId, direction: .right
) != nil else {
writeGotoSplitTestData(["setupError": "Failed to create horizontal split"])
return
}

// Focus back to initial pane, then create vertical split (down)
tab.focusPanel(initialPanelId)
guard tabManager.createSplit(
tabId: tab.id, surfaceId: initialPanelId, direction: .down
) != nil else {
writeGotoSplitTestData(["setupError": "Failed to create vertical split"])
return
}

let allPaneIds = tab.bonsplitController.allPaneIds.map(\.description)
let focusedPaneId = tab.bonsplitController.focusedPaneId?.description ?? ""

writeGotoSplitTestData([
"paneCount": String(allPaneIds.count),
"allPaneIds": allPaneIds.joined(separator: ","),
"focusedPaneId": focusedPaneId,
"setupComplete": "true",
])
}

private func setupBonsplitTabDragUITestIfNeeded() {
guard !didSetupBonsplitTabDragUITest else { return }
didSetupBonsplitTabDragUITest = true
Expand Down Expand Up @@ -9483,6 +9529,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
writeGotoSplitTestData(updates)
}

func recordGotoSplitCycleMoveIfNeeded(forward: Bool) {
guard isGotoSplitUITestRecordingEnabled() else { return }
guard let tabManager, let workspace = tabManager.selectedWorkspace else { return }

var updates = gotoSplitFindStateSnapshot(for: workspace)
updates["lastMoveDirection"] = forward ? "next" : "previous"
writeGotoSplitTestData(updates)
}

private func recordGotoSplitSplitIfNeeded(direction: SplitDirection) {
guard isGotoSplitUITestRecordingEnabled() else { return }
guard let workspace = tabManager?.selectedWorkspace else { return }
Expand Down
21 changes: 16 additions & 5 deletions Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2643,10 +2643,6 @@ class GhosttyApp {

private func focusDirection(from direction: ghostty_action_goto_split_e) -> NavigationDirection? {
switch direction {
// For previous/next, we use left/right as a reasonable default
// Bonsplit doesn't have cycle-based navigation
case GHOSTTY_GOTO_SPLIT_PREVIOUS: return .left
case GHOSTTY_GOTO_SPLIT_NEXT: return .right
case GHOSTTY_GOTO_SPLIT_UP: return .up
case GHOSTTY_GOTO_SPLIT_DOWN: return .down
case GHOSTTY_GOTO_SPLIT_LEFT: return .left
Expand Down Expand Up @@ -2834,9 +2830,24 @@ class GhosttyApp {
}
return true
case GHOSTTY_ACTION_GOTO_SPLIT:
let gotoDirection = action.action.goto_split
// Previous/next use cycle-based navigation through all panes in tree order
if gotoDirection == GHOSTTY_GOTO_SPLIT_PREVIOUS || gotoDirection == GHOSTTY_GOTO_SPLIT_NEXT {
guard let tabId = surfaceView.tabId else { return false }
let forward = gotoDirection == GHOSTTY_GOTO_SPLIT_NEXT
return performOnMain {
guard let tabManager = AppDelegate.shared?.tabManager else { return false }
let result = tabManager.cycleSplitFocus(tabId: tabId, forward: forward)
#if DEBUG
AppDelegate.shared?.recordGotoSplitCycleMoveIfNeeded(forward: forward)
#endif
return result
}
}
// Directional navigation uses spatial positioning
guard let tabId = surfaceView.tabId,
let surfaceId = surfaceView.terminalSurface?.id,
let direction = focusDirection(from: action.action.goto_split) else {
let direction = focusDirection(from: gotoDirection) else {
return false
}
return performOnMain {
Expand Down
7 changes: 7 additions & 0 deletions Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4014,6 +4014,13 @@ class TabManager: ObservableObject {
return true
}

/// Cycle focus to the next or previous pane in tree order, wrapping at the ends.
func cycleSplitFocus(tabId: UUID, forward: Bool) -> Bool {
guard let tab = tabs.first(where: { $0.id == tabId }) else { return false }
tab.cycleFocus(forward: forward)
return true
}

/// Resize split - not directly supported by bonsplit, but we can adjust divider positions
func resizeSplit(tabId: UUID, surfaceId: UUID, direction: ResizeDirection, amount: UInt16) -> Bool {
guard amount > 0,
Expand Down
29 changes: 29 additions & 0 deletions Sources/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10092,6 +10092,35 @@ final class Workspace: Identifiable, ObservableObject {

}

/// Cycle focus to the next or previous pane in tree order, wrapping at the ends.
/// Unlike moveFocus(direction:), this visits all panes regardless of spatial layout.
func cycleFocus(forward: Bool) {
let allPaneIds = bonsplitController.allPaneIds
guard allPaneIds.count > 1,
let currentId = bonsplitController.focusedPaneId,
let currentIndex = allPaneIds.firstIndex(of: currentId) else { return }

// Unfocus the currently-focused panel before navigating.
if let prevPanelId = focusedPanelId, let prev = panels[prevPanelId] {
prev.unfocus()
}

let targetIndex: Int
if forward {
targetIndex = (currentIndex + 1) % allPaneIds.count
} else {
targetIndex = currentIndex == 0 ? allPaneIds.count - 1 : currentIndex - 1
}
bonsplitController.focusPane(allPaneIds[targetIndex])

// Reconcile selection/focus after navigation so AppKit first-responder and
// bonsplit's focused pane stay aligned.
if let paneId = bonsplitController.focusedPaneId,
let tabId = bonsplitController.selectedTab(inPane: paneId)?.id {
applyTabSelection(tabId: tabId, inPane: paneId)
}
}

// MARK: - Surface Navigation

/// Select the next surface in the currently focused pane
Expand Down
204 changes: 204 additions & 0 deletions cmuxUITests/GotoSplitCycleUITests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
import XCTest
import Foundation

/// Tests that goto_split:previous and goto_split:next cycle through ALL panes
/// regardless of split direction (horizontal and vertical), wrapping at the ends.
///
/// Before the fix, goto_split:previous/next were mapped to directional left/right
/// navigation in Bonsplit, which skipped vertically-split panes and did not wrap.
final class GotoSplitCycleUITests: XCTestCase {
private var dataPath = ""

override func setUp() {
super.setUp()
continueAfterFailure = false
dataPath = "/tmp/cmux-ui-test-goto-split-cycle-\(UUID().uuidString).json"
try? FileManager.default.removeItem(atPath: dataPath)
}

// MARK: - Tests

func testGotoSplitNextCyclesAllPanes() {
// Uses Cmd+] which is Ghostty's default keybind for goto_split:next.
let (app, configCleanup) = launchWithThreePaneLayout()
defer { configCleanup() }

XCTAssertTrue(
waitForData(keys: ["setupComplete", "allPaneIds", "focusedPaneId"], timeout: 10.0),
"Expected three-pane setup data to be written"
)

guard let setup = loadData() else {
XCTFail("Missing setup data")
return
}
XCTAssertEqual(setup["paneCount"], "3", "Expected 3 panes")

let allPaneIds = Set(setup["allPaneIds"]!.split(separator: ",").map(String.init))
XCTAssertEqual(allPaneIds.count, 3, "Expected 3 distinct pane IDs")

let startPane = setup["focusedPaneId"]!
XCTAssertTrue(allPaneIds.contains(startPane), "Start pane should be in allPaneIds")

// Send goto_split:next (Cmd+]) 3 times — should visit all panes and wrap.
// Ghostty default keybind: super+]=goto_split:next
var visited = [startPane]
for i in 0..<3 {
app.typeKey("]", modifierFlags: [.command])

XCTAssertTrue(
waitForDataMatch(timeout: 3.0) { data in
guard let focused = data["focusedPaneId"], !focused.isEmpty else { return false }
return focused != visited.last
},
"Focus did not change after goto_split:next #\(i + 1)"
)

guard let data = loadData(), let focused = data["focusedPaneId"] else {
XCTFail("Missing focusedPaneId after goto_split:next #\(i + 1)")
return
}
visited.append(focused)
}

let visitedSet = Set(visited.prefix(3))
XCTAssertEqual(visitedSet, allPaneIds, "goto_split:next should visit all 3 panes")
XCTAssertEqual(visited[3], visited[0], "goto_split:next should wrap back to start")
}

func testGotoSplitPreviousCyclesAllPanes() {
// Uses Cmd+[ which is Ghostty's default keybind for goto_split:previous.
let (app, configCleanup) = launchWithThreePaneLayout()
defer { configCleanup() }

XCTAssertTrue(
waitForData(keys: ["setupComplete", "allPaneIds", "focusedPaneId"], timeout: 10.0),
"Expected three-pane setup data to be written"
)

guard let setup = loadData() else {
XCTFail("Missing setup data")
return
}
XCTAssertEqual(setup["paneCount"], "3", "Expected 3 panes")

let allPaneIds = Set(setup["allPaneIds"]!.split(separator: ",").map(String.init))
XCTAssertEqual(allPaneIds.count, 3, "Expected 3 distinct pane IDs")

let startPane = setup["focusedPaneId"]!

var visited = [startPane]
for i in 0..<3 {
app.typeKey("[", modifierFlags: [.command])

XCTAssertTrue(
waitForDataMatch(timeout: 3.0) { data in
guard let focused = data["focusedPaneId"], !focused.isEmpty else { return false }
return focused != visited.last
},
"Focus did not change after goto_split:previous #\(i + 1)"
)

guard let data = loadData(), let focused = data["focusedPaneId"] else {
XCTFail("Missing focusedPaneId after goto_split:previous #\(i + 1)")
return
}
visited.append(focused)
}

let visitedSet = Set(visited.prefix(3))
XCTAssertEqual(visitedSet, allPaneIds, "goto_split:previous should visit all 3 panes")
XCTAssertEqual(visited[3], visited[0], "goto_split:previous should wrap back to start")
}

// MARK: - Launch Helpers

private func launchWithThreePaneLayout() -> (XCUIApplication, () -> Void) {
let fileManager = FileManager.default
guard let appSupport = fileManager.urls(for: .applicationSupportDirectory, in: .userDomainMask).first else {
XCTFail("Missing Application Support directory")
return (XCUIApplication(), {})
}

let ghosttyDir = appSupport.appendingPathComponent("com.mitchellh.ghostty", isDirectory: true)
let configURL = ghosttyDir.appendingPathComponent("config.ghostty", isDirectory: false)

do {
try fileManager.createDirectory(at: ghosttyDir, withIntermediateDirectories: true)
} catch {
XCTFail("Failed to create Ghostty app support dir: \(error)")
return (XCUIApplication(), {})
}

let originalConfigData = try? Data(contentsOf: configURL)
let cleanup: () -> Void = {
if let originalConfigData {
try? originalConfigData.write(to: configURL, options: .atomic)
} else {
try? fileManager.removeItem(at: configURL)
}
}

let home = fileManager.homeDirectoryForCurrentUser
let configContents = "# cmux goto_split cycle UI test\nworking-directory = \(home.path)\n"

do {
try configContents.write(to: configURL, atomically: true, encoding: .utf8)
} catch {
XCTFail("Failed to write Ghostty config: \(error)")
return (XCUIApplication(), {})
}

let app = XCUIApplication()
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_SETUP"] = "1"
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_PATH"] = dataPath
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_LAYOUT"] = "three_pane_terminal"
app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_USE_GHOSTTY_CONFIG"] = "1"
launchAndEnsureForeground(app)

return (app, cleanup)
}

// MARK: - Data Polling

private func waitForData(keys: [String], timeout: TimeInterval) -> Bool {
waitForCondition(timeout: timeout) {
guard let data = self.loadData() else { return false }
return keys.allSatisfy { data[$0] != nil }
}
}

private func waitForDataMatch(timeout: TimeInterval, predicate: @escaping ([String: String]) -> Bool) -> Bool {
waitForCondition(timeout: timeout) {
guard let data = self.loadData() else { return false }
return predicate(data)
}
}

private func loadData() -> [String: String]? {
guard let data = try? Data(contentsOf: URL(fileURLWithPath: dataPath)) else {
return nil
}
return (try? JSONSerialization.jsonObject(with: data)) as? [String: String]
}

private func waitForCondition(timeout: TimeInterval, predicate: @escaping () -> Bool) -> Bool {
let expectation = XCTNSPredicateExpectation(
predicate: NSPredicate { _, _ in predicate() },
object: nil
)
return XCTWaiter().wait(for: [expectation], timeout: timeout) == .completed
}

private func launchAndEnsureForeground(_ app: XCUIApplication, timeout: TimeInterval = 12.0) {
let options = XCTExpectedFailure.Options()
options.isStrict = false
XCTExpectFailure("App activation may fail on headless CI runners", options: options) {
app.launch()
}

if app.state == .runningForeground { return }
if app.state == .runningBackground { return }
XCTFail("App failed to start. state=\(app.state.rawValue)")
}
Comment on lines +193 to +203
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

Method accepts runningBackground state despite name suggesting foreground.

The method returns successfully when app.state == .runningBackground, which may not actually ensure the app receives keyboard input reliably. Consider either renaming to launchAndEnsureRunning or attempting to activate the app when in background state. This could cause flaky test failures if key events don't reach a background app.

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

In `@cmuxUITests/GotoSplitCycleUITests.swift` around lines 193 - 203, The helper
launchAndEnsureForeground currently returns success when app.state ==
.runningBackground, which contradicts its name and can cause flaky input
failures; update it to either (A) rename the function to launchAndEnsureRunning
(or similar) if background is an acceptable end state, or (B) ensure the app is
actually foreground by calling app.activate() (or another activation attempt)
when app.state == .runningBackground and re-checking app.state before returning,
keeping the existing XCTExpectFailure wrapper around app.launch() and preserving
the failure path that calls XCTFail with state info.

}