diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 6dff2216d31c..e7100c6717f8 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -1141,8 +1141,17 @@ struct CMUXCLI { let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace") let (titleOpt, rem1) = parseOption(rem0, name: "--title") let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) - let trailingTitle = rem1.dropFirst(rem1.first == "--" ? 1 : 0).joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) - let title = titleOpt ?? (trailingTitle.isEmpty ? nil : trailingTitle) + let trailing = try positionalArguments(rem1, command: "new-page") + let title: String? + if let titleOpt { + guard trailing.isEmpty else { + throw CLIError(message: "new-page: unexpected arguments: \(trailing.joined(separator: " "))") + } + title = titleOpt + } else { + let trailingTitle = trailing.joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) + title = trailingTitle.isEmpty ? nil : trailingTitle + } var params: [String: Any] = [:] let wsId = try normalizeWorkspaceHandle(workspaceArg, client: client, allowCurrent: true) if let wsId { params["workspace_id"] = wsId } @@ -1155,8 +1164,17 @@ struct CMUXCLI { let (pageOpt, rem1) = parseOption(rem0, name: "--page") let (titleOpt, rem2) = parseOption(rem1, name: "--title") let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) - let trailingTitle = rem2.dropFirst(rem2.first == "--" ? 1 : 0).joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) - let title = titleOpt ?? (trailingTitle.isEmpty ? nil : trailingTitle) + let trailing = try positionalArguments(rem2, command: "duplicate-page") + let title: String? + if let titleOpt { + guard trailing.isEmpty else { + throw CLIError(message: "duplicate-page: unexpected arguments: \(trailing.joined(separator: " "))") + } + title = titleOpt + } else { + let trailingTitle = trailing.joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) + title = trailingTitle.isEmpty ? nil : trailingTitle + } var params: [String: Any] = [:] let wsId = try normalizeWorkspaceHandle(workspaceArg, client: client, allowCurrent: true) if let wsId { params["workspace_id"] = wsId } @@ -1397,8 +1415,22 @@ struct CMUXCLI { printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["workspace"])) case "close-page": - let workspaceArg = workspaceFromArgsOrEnv(commandArgs, windowOverride: windowId) - let pageRaw = optionValue(commandArgs, name: "--page") ?? commandArgs.first + let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace") + let (pageOpt, rem1) = parseOption(rem0, name: "--page") + let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) + let trailing = try positionalArguments(rem1, command: "close-page") + let pageRaw: String? + if let pageOpt { + guard trailing.isEmpty else { + throw CLIError(message: "close-page: unexpected arguments: \(trailing.joined(separator: " "))") + } + pageRaw = pageOpt + } else { + guard trailing.count <= 1 else { + throw CLIError(message: "close-page: unexpected arguments: \(trailing.dropFirst().joined(separator: " "))") + } + pageRaw = trailing.first + } var params: [String: Any] = [:] let wsId = try normalizeWorkspaceHandle(workspaceArg, client: client, allowCurrent: true) if let wsId { params["workspace_id"] = wsId } @@ -1460,14 +1492,15 @@ struct CMUXCLI { let wsId = try normalizeWorkspaceHandle(workspaceArg, client: client, allowCurrent: true) if let wsId { params["workspace_id"] = wsId } let payload = try client.sendV2(method: "page.current", params: params) - printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["page", "workspace"])) + let fallbackText = formatHandle(payload, kind: "page", idFormat: idFormat) + ?? v2OKSummary(payload, idFormat: idFormat, kinds: ["page", "workspace"]) + printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: fallbackText) case "rename-page": let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace") let (pageOpt, rem1) = parseOption(rem0, name: "--page") let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) - let titleArgs = rem1.dropFirst(rem1.first == "--" ? 1 : 0) - let title = titleArgs.joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) + let title = try trailingTextArgument(rem1, command: "rename-page") ?? "" guard !title.isEmpty else { throw CLIError(message: "rename-page requires a title") } @@ -5493,6 +5526,21 @@ struct CMUXCLI { args.contains(name) } + private func positionalArguments(_ args: [String], command: String) throws -> [String] { + let positional = args.first == "--" ? Array(args.dropFirst()) : args + if args.first != "--", + let unknown = positional.first(where: { $0.hasPrefix("--") }) { + throw CLIError(message: "\(command): unknown flag '\(unknown)'") + } + return positional + } + + private func trailingTextArgument(_ args: [String], command: String) throws -> String? { + let positional = try positionalArguments(args, command: command) + let text = positional.joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines) + return text.isEmpty ? nil : text + } + private func replaceToken(_ args: [String], from: String, to: String) -> [String] { args.map { $0 == from ? to : $0 } } diff --git a/Resources/Info.plist b/Resources/Info.plist index c2badb5db6e2..6e3d5891465a 100644 --- a/Resources/Info.plist +++ b/Resources/Info.plist @@ -95,6 +95,16 @@ public.data + + UTTypeIdentifier + com.cmux.titlebar-page-reorder + UTTypeDescription + cmux Titlebar Page Reorder + UTTypeConformsTo + + public.data + + NSAppTransportSecurity diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 5e0b625066c1..fc4a193015af 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -625,10 +625,6 @@ enum WorkspaceShortcutMapper { return nil } - static func pageIndex(forOptionDigit digit: Int, pageCount: Int) -> Int? { - workspaceIndex(forCommandDigit: digit, workspaceCount: pageCount) - } - static func optionDigitForPage(at index: Int, pageCount: Int) -> Int? { commandDigitForWorkspace(at: index, workspaceCount: pageCount) } @@ -6852,35 +6848,64 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .newPage)) { - _ = tabManager?.selectedWorkspace?.newPage(select: true) + let createdPage = tabManager?.selectedWorkspace?.newPage(select: true) +#if DEBUG + dlog( + "shortcut.action name=newPage handled=\(createdPage == nil ? 0 : 1) " + + "\(debugShortcutRouteSnapshot(event: event)) " + + "page=\(createdPage?.id.uuidString.prefix(5) ?? "nil")" + ) +#endif return true } if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .renamePage)) { - guard let workspace = tabManager?.selectedWorkspace, - let pageId = workspace.activePage?.id else { + guard let pageId = tabManager?.selectedWorkspace?.activePage?.id else { +#if DEBUG + dlog("shortcut.action name=renamePage handled=0 \(debugShortcutRouteSnapshot(event: event))") +#endif return false } - workspace.promptRenamePage(pageId: pageId) + tabManager?.selectedWorkspace?.promptRenamePage(pageId: pageId) +#if DEBUG + dlog( + "shortcut.action name=renamePage handled=1 " + + "\(debugShortcutRouteSnapshot(event: event)) page=\(pageId.uuidString.prefix(5))" + ) +#endif return true } if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .closePage)) { - guard let workspace = tabManager?.selectedWorkspace, - let pageId = workspace.activePage?.id else { + guard let pageId = tabManager?.selectedWorkspace?.activePage?.id else { +#if DEBUG + dlog("shortcut.action name=closePage handled=0 \(debugShortcutRouteSnapshot(event: event))") +#endif return false } - workspace.closePage(pageId) + tabManager?.selectedWorkspace?.closePage(pageId) +#if DEBUG + dlog( + "shortcut.action name=closePage handled=1 " + + "\(debugShortcutRouteSnapshot(event: event)) page=\(pageId.uuidString.prefix(5))" + ) +#endif return true } if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .nextPage)) { tabManager?.selectedWorkspace?.selectNextPage() +#if DEBUG + dlog("shortcut.action name=nextPage handled=1 \(debugShortcutRouteSnapshot(event: event))") +#endif return true } if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .previousPage)) { tabManager?.selectedWorkspace?.selectPreviousPage() +#if DEBUG + dlog("shortcut.action name=previousPage handled=1 \(debugShortcutRouteSnapshot(event: event))") +#endif return true } @@ -6902,6 +6927,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } else { tabManager?.selectedWorkspace?.selectLastPage() } +#if DEBUG + let targetText = pageIndex.map(String.init) ?? "last" + dlog( + "shortcut.action name=\(action.rawValue) handled=1 " + + "\(debugShortcutRouteSnapshot(event: event)) target=\(targetText)" + ) +#endif return true } } diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 2e0862820972..79c0274b2dc7 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -1315,8 +1315,9 @@ struct ContentView: View { @State private var hoveredTitlebarPageId: UUID? @State private var draggedTitlebarPageId: UUID? @State private var titlebarPageDropIndicator: TitlebarPageDropIndicator? + @State private var titlebarPageWidths: [UUID: CGFloat] = [:] @StateObject private var titlebarPageDragAutoScrollController = TitlebarPageDragAutoScrollController() - @StateObject private var titlebarPageShortcutHintMonitor = ShortcutHintModifierMonitor(requiredModifierFlags: [.option]) + @StateObject private var titlebarPageShortcutHintMonitor = ShortcutHintModifierMonitor(requiredModifierFlags: [.command]) @State private var titlebarPageDragMonitorTask: Task? @State private var sidebarDraggedTabId: UUID? @State private var titlebarTextUpdateCoalescer = NotificationBurstCoalescer(delay: 1.0 / 30.0) @@ -1529,6 +1530,10 @@ struct ContentView: View { static let workspaceShouldPin = "workspace.shouldPin" static let workspaceHasPullRequests = "workspace.hasPullRequests" static let workspaceHasSplits = "workspace.hasSplits" + static let workspaceHasMultiplePages = "workspace.hasMultiplePages" + static let pageCanClose = "page.canClose" + static let pageCanMoveLeft = "page.canMoveLeft" + static let pageCanMoveRight = "page.canMoveRight" static let hasFocusedPanel = "panel.hasFocus" static let panelName = "panel.name" @@ -2136,12 +2141,27 @@ struct ContentView: View { private func startTitlebarPageDragMonitor() { titlebarPageDragMonitorTask?.cancel() +#if DEBUG + dlog("titlebar.page.dragMonitor.start page=\(draggedTitlebarPageId?.uuidString.prefix(5) ?? "nil")") +#endif titlebarPageDragMonitorTask = Task { @MainActor in while !Task.isCancelled { try? await Task.sleep(nanoseconds: 50_000_000) if NSEvent.pressedMouseButtons != 0 { +#if DEBUG + dlog( + "titlebar.page.dragMonitor.update page=\(draggedTitlebarPageId?.uuidString.prefix(5) ?? "nil") " + + "indicator=\(debugTitlebarPageDropIndicator(titlebarPageDropIndicator))" + ) +#endif continue } +#if DEBUG + dlog( + "titlebar.page.dragMonitor.clear page=\(draggedTitlebarPageId?.uuidString.prefix(5) ?? "nil") " + + "indicator=\(debugTitlebarPageDropIndicator(titlebarPageDropIndicator))" + ) +#endif draggedTitlebarPageId = nil titlebarPageDropIndicator = nil titlebarPageDragAutoScrollController.stop() @@ -2151,10 +2171,19 @@ struct ContentView: View { } private func stopTitlebarPageDragMonitor() { +#if DEBUG + dlog("titlebar.page.dragMonitor.stop page=\(draggedTitlebarPageId?.uuidString.prefix(5) ?? "nil")") +#endif titlebarPageDragMonitorTask?.cancel() titlebarPageDragMonitorTask = nil } + private func debugTitlebarPageDropIndicator(_ indicator: TitlebarPageDropIndicator?) -> String { + guard let indicator else { return "nil" } + let pageText = indicator.pageId.map { String($0.uuidString.prefix(5)) } ?? "end" + return "\(pageText):\(indicator.edge == .leading ? "leading" : "trailing")" + } + private func titlebarPageStrip(workspace: Workspace) -> some View { ScrollViewReader { proxy in HStack(spacing: 6) { @@ -2175,7 +2204,8 @@ struct ContentView: View { workspace: workspace, draggedPageId: $draggedTitlebarPageId, dragAutoScrollController: titlebarPageDragAutoScrollController, - dropIndicator: $titlebarPageDropIndicator + dropIndicator: $titlebarPageDropIndicator, + targetPageWidth: nil ) ) .overlay(alignment: .leading) { @@ -2186,6 +2216,10 @@ struct ContentView: View { } } .padding(.trailing, 4) + .onPreferenceChange(TitlebarPageWidthPreferenceKey.self) { widths in + let livePageIds = Set(workspace.pages.map(\.id)) + titlebarPageWidths = widths.filter { livePageIds.contains($0.key) } + } } .scrollClipDisabled() .background( @@ -2210,6 +2244,7 @@ struct ContentView: View { } .buttonStyle(.plain) .help(String(localized: "workspace.page.new.tooltip", defaultValue: "New Page")) + .accessibilityLabel(String(localized: "workspace.page.new.tooltip", defaultValue: "New Page")) .accessibilityIdentifier("titlebarPageNewButton") .transition(.opacity) } @@ -2302,6 +2337,7 @@ struct ContentView: View { } .buttonStyle(.plain) .disabled(!canClose) + .accessibilityLabel(String(localized: "workspace.page.context.close", defaultValue: "Close Page")) .opacity(showCloseButton ? 1 : 0) .allowsHitTesting(showCloseButton) .accessibilityIdentifier(titlebarPageCloseButtonAccessibilityIdentifier(pageId: page.id)) @@ -2310,6 +2346,11 @@ struct ContentView: View { .animation(.easeInOut(duration: 0.12), value: showCloseButton) .animation(.easeInOut(duration: 0.12), value: showsShortcutHint) } + .background( + GeometryReader { proxy in + Color.clear.preference(key: TitlebarPageWidthPreferenceKey.self, value: [page.id: proxy.size.width]) + } + ) .contentShape(Rectangle()) .opacity(isDragged ? 0.55 : 1) .onHover { hovering in @@ -2338,7 +2379,8 @@ struct ContentView: View { workspace: workspace, draggedPageId: $draggedTitlebarPageId, dragAutoScrollController: titlebarPageDragAutoScrollController, - dropIndicator: $titlebarPageDropIndicator + dropIndicator: $titlebarPageDropIndicator, + targetPageWidth: titlebarPageWidths[page.id] ) ) .overlay(alignment: .leading) { @@ -4424,6 +4466,17 @@ struct ContentView: View { snapshot.setString(CommandPaletteContextKeys.workspaceName, workspaceDisplayName(workspace)) snapshot.setBool(CommandPaletteContextKeys.workspaceHasCustomName, workspace.customTitle != nil) snapshot.setBool(CommandPaletteContextKeys.workspaceShouldPin, !workspace.isPinned) + snapshot.setBool(CommandPaletteContextKeys.workspaceHasMultiplePages, workspace.pages.count > 1) + if let activePage = workspace.activePage { + snapshot.setBool(CommandPaletteContextKeys.pageCanClose, workspace.canClosePage(activePage.id)) + } + if let activePageIndex = workspace.activePageIndex { + snapshot.setBool(CommandPaletteContextKeys.pageCanMoveLeft, activePageIndex > 0) + snapshot.setBool( + CommandPaletteContextKeys.pageCanMoveRight, + activePageIndex < workspace.pages.count - 1 + ) + } snapshot.setBool( CommandPaletteContextKeys.workspaceHasPullRequests, !workspace.sidebarPullRequestsInDisplayOrder().isEmpty @@ -4760,7 +4813,10 @@ struct ContentView: View { title: constant(String(localized: "command.closePage.title", defaultValue: "Close Page")), subtitle: workspaceSubtitle, keywords: ["close", "page", "workspace"], - when: { $0.bool(CommandPaletteContextKeys.hasWorkspace) } + when: { + $0.bool(CommandPaletteContextKeys.hasWorkspace) + && $0.bool(CommandPaletteContextKeys.pageCanClose) + } ) ) contributions.append( @@ -4769,7 +4825,10 @@ struct ContentView: View { title: constant(String(localized: "command.closeOtherPages.title", defaultValue: "Close Other Pages")), subtitle: workspaceSubtitle, keywords: ["close", "other", "pages", "workspace"], - when: { $0.bool(CommandPaletteContextKeys.hasWorkspace) } + when: { + $0.bool(CommandPaletteContextKeys.hasWorkspace) + && $0.bool(CommandPaletteContextKeys.workspaceHasMultiplePages) + } ) ) contributions.append( @@ -4796,7 +4855,10 @@ struct ContentView: View { title: constant(String(localized: "command.movePageLeft.title", defaultValue: "Move Page Left")), subtitle: constant(String(localized: "command.movePageLeft.subtitle", defaultValue: "Page Navigation")), keywords: ["move", "page", "left", "reorder"], - when: { $0.bool(CommandPaletteContextKeys.hasWorkspace) } + when: { + $0.bool(CommandPaletteContextKeys.hasWorkspace) + && $0.bool(CommandPaletteContextKeys.pageCanMoveLeft) + } ) ) contributions.append( @@ -4805,7 +4867,10 @@ struct ContentView: View { title: constant(String(localized: "command.movePageRight.title", defaultValue: "Move Page Right")), subtitle: constant(String(localized: "command.movePageRight.subtitle", defaultValue: "Page Navigation")), keywords: ["move", "page", "right", "reorder"], - when: { $0.bool(CommandPaletteContextKeys.hasWorkspace) } + when: { + $0.bool(CommandPaletteContextKeys.hasWorkspace) + && $0.bool(CommandPaletteContextKeys.pageCanMoveRight) + } ) ) @@ -8283,7 +8348,7 @@ private struct SidebarExternalDropDelegate: DropDelegate { private final class ShortcutHintModifierMonitor: ObservableObject { @Published private(set) var isModifierPressed = false - private let requiredModifierFlags: NSEvent.ModifierFlags + private let requiredModifierFlagsProvider: () -> NSEvent.ModifierFlags private weak var hostWindow: NSWindow? private var hostWindowDidBecomeKeyObserver: NSObjectProtocol? private var hostWindowDidResignKeyObserver: NSObjectProtocol? @@ -8292,8 +8357,11 @@ private final class ShortcutHintModifierMonitor: ObservableObject { private var appResignObserver: NSObjectProtocol? private var pendingShowWorkItem: DispatchWorkItem? - init(requiredModifierFlags: NSEvent.ModifierFlags = [.command]) { - self.requiredModifierFlags = requiredModifierFlags + init( + requiredModifierFlags: NSEvent.ModifierFlags = [.command], + requiredModifierFlagsProvider: (() -> NSEvent.ModifierFlags)? = nil + ) { + self.requiredModifierFlagsProvider = requiredModifierFlagsProvider ?? { requiredModifierFlags } } func setHostWindow(_ window: NSWindow?) { @@ -8395,7 +8463,7 @@ private final class ShortcutHintModifierMonitor: ObservableObject { hostWindowIsKey: hostWindow?.isKeyWindow ?? false, eventWindowNumber: eventWindow?.windowNumber, keyWindowNumber: NSApp.keyWindow?.windowNumber, - requiredModifierFlags: requiredModifierFlags + requiredModifierFlags: requiredModifierFlagsProvider() ) else { cancelPendingHintShow(resetVisible: true) return @@ -8417,7 +8485,7 @@ private final class ShortcutHintModifierMonitor: ObservableObject { hostWindowIsKey: self.hostWindow?.isKeyWindow ?? false, eventWindowNumber: nil, keyWindowNumber: NSApp.keyWindow?.windowNumber, - requiredModifierFlags: self.requiredModifierFlags + requiredModifierFlags: self.requiredModifierFlagsProvider() ) else { return } self.isModifierPressed = true } @@ -9069,7 +9137,7 @@ private struct SidebarHelpMenuButton: View { @State private var isPopoverPresented = false private var sendFeedbackShortcutHint: String { - decodeShortcut( + StoredShortcut.decode( from: sendFeedbackShortcutData, fallback: KeyboardShortcutSettings.Action.sendFeedback.defaultShortcut ).displayString @@ -9241,13 +9309,6 @@ private struct SidebarHelpMenuButton: View { } } - private func decodeShortcut(from data: Data, fallback: StoredShortcut) -> StoredShortcut { - guard !data.isEmpty, - let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else { - return fallback - } - return shortcut - } } private struct SidebarFooterIconButtonStyle: ButtonStyle { @@ -11271,6 +11332,14 @@ private final class TitlebarPageDragAutoScrollController: ObservableObject { } } +private struct TitlebarPageWidthPreferenceKey: PreferenceKey { + static var defaultValue: [UUID: CGFloat] = [:] + + static func reduce(value: inout [UUID: CGFloat], nextValue: () -> [UUID: CGFloat]) { + value.merge(nextValue(), uniquingKeysWith: { _, new in new }) + } +} + private enum TitlebarPageDragPayload { static let typeIdentifier = "com.cmux.titlebar-page-reorder" static let dropContentType = UTType(exportedAs: typeIdentifier) @@ -11294,19 +11363,36 @@ private struct TitlebarPageDropDelegate: DropDelegate { @Binding var draggedPageId: UUID? let dragAutoScrollController: TitlebarPageDragAutoScrollController @Binding var dropIndicator: TitlebarPageDropIndicator? + let targetPageWidth: CGFloat? private let targetWidthEstimate: CGFloat = 80 func validateDrop(info: DropInfo) -> Bool { - info.hasItemsConforming(to: [TitlebarPageDragPayload.typeIdentifier]) && draggedPageId != nil + let valid = info.hasItemsConforming(to: [TitlebarPageDragPayload.typeIdentifier]) && draggedPageId != nil +#if DEBUG + dlog( + "titlebar.page.validateDrop target=\(targetPageId?.uuidString.prefix(5) ?? "end") " + + "valid=\(valid ? 1 : 0)" + ) +#endif + return valid } func dropEntered(info: DropInfo) { dragAutoScrollController.updateFromDragLocation() updateDropIndicator(for: info) +#if DEBUG + dlog( + "titlebar.page.dropEntered target=\(targetPageId?.uuidString.prefix(5) ?? "end") " + + "indicator=\(debugIndicator(dropIndicator))" + ) +#endif } func dropExited(info: DropInfo) { +#if DEBUG + dlog("titlebar.page.dropExited target=\(targetPageId?.uuidString.prefix(5) ?? "end")") +#endif if dropIndicator?.pageId == targetPageId { dropIndicator = nil } @@ -11315,6 +11401,12 @@ private struct TitlebarPageDropDelegate: DropDelegate { func dropUpdated(info: DropInfo) -> DropProposal? { dragAutoScrollController.updateFromDragLocation() updateDropIndicator(for: info) +#if DEBUG + dlog( + "titlebar.page.dropUpdated target=\(targetPageId?.uuidString.prefix(5) ?? "end") " + + "indicator=\(debugIndicator(dropIndicator)) width=\(effectiveTargetWidth())" + ) +#endif return DropProposal(operation: .move) } @@ -11324,6 +11416,12 @@ private struct TitlebarPageDropDelegate: DropDelegate { dropIndicator = nil dragAutoScrollController.stop() } +#if DEBUG + dlog( + "titlebar.page.drop target=\(targetPageId?.uuidString.prefix(5) ?? "end") " + + "indicator=\(debugIndicator(dropIndicator))" + ) +#endif guard let draggedPageId else { return false } let pageIds = workspace.pages.map(\.id) guard let targetIndex = TitlebarPageDropPlanner.targetIndex( @@ -11332,8 +11430,17 @@ private struct TitlebarPageDropDelegate: DropDelegate { indicator: dropIndicator, pageIds: pageIds ) else { +#if DEBUG + dlog("titlebar.page.drop.abort reason=noTargetIndex") +#endif return false } +#if DEBUG + dlog( + "titlebar.page.drop.commit page=\(draggedPageId.uuidString.prefix(5)) " + + "target=\(targetIndex)" + ) +#endif return workspace.movePage(pageId: draggedPageId, toIndex: targetIndex) } @@ -11343,9 +11450,20 @@ private struct TitlebarPageDropDelegate: DropDelegate { targetPageId: targetPageId, pageIds: workspace.pages.map(\.id), pointerX: targetPageId == nil ? nil : info.location.x, - targetWidth: targetPageId == nil ? nil : targetWidthEstimate + targetWidth: targetPageId == nil ? nil : effectiveTargetWidth() ) } + + private func effectiveTargetWidth() -> CGFloat { + guard let targetPageWidth, targetPageWidth > 0 else { return targetWidthEstimate } + return targetPageWidth + } + + private func debugIndicator(_ indicator: TitlebarPageDropIndicator?) -> String { + guard let indicator else { return "nil" } + let pageText = indicator.pageId.map { String($0.uuidString.prefix(5)) } ?? "end" + return "\(pageText):\(indicator.edge == .leading ? "leading" : "trailing")" + } } struct SidebarDropIndicator { diff --git a/Sources/KeyboardShortcutSettings.swift b/Sources/KeyboardShortcutSettings.swift index a07bc2c86ba0..8603b54a77b7 100644 --- a/Sources/KeyboardShortcutSettings.swift +++ b/Sources/KeyboardShortcutSettings.swift @@ -193,27 +193,27 @@ enum KeyboardShortcutSettings { case .closePage: return StoredShortcut(key: "w", command: true, shift: false, option: true, control: false) case .nextPage: - return StoredShortcut(key: "]", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "]", command: true, shift: false, option: true, control: false) case .previousPage: - return StoredShortcut(key: "[", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "[", command: true, shift: false, option: true, control: false) case .selectPage1: - return StoredShortcut(key: "1", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "1", command: true, shift: false, option: true, control: false) case .selectPage2: - return StoredShortcut(key: "2", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "2", command: true, shift: false, option: true, control: false) case .selectPage3: - return StoredShortcut(key: "3", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "3", command: true, shift: false, option: true, control: false) case .selectPage4: - return StoredShortcut(key: "4", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "4", command: true, shift: false, option: true, control: false) case .selectPage5: - return StoredShortcut(key: "5", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "5", command: true, shift: false, option: true, control: false) case .selectPage6: - return StoredShortcut(key: "6", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "6", command: true, shift: false, option: true, control: false) case .selectPage7: - return StoredShortcut(key: "7", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "7", command: true, shift: false, option: true, control: false) case .selectPage8: - return StoredShortcut(key: "8", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "8", command: true, shift: false, option: true, control: false) case .selectLastPage: - return StoredShortcut(key: "9", command: false, shift: false, option: true, control: false) + return StoredShortcut(key: "9", command: true, shift: false, option: true, control: false) case .focusLeft: return StoredShortcut(key: "←", command: true, shift: false, option: true, control: false) case .focusRight: @@ -436,6 +436,14 @@ struct StoredShortcut: Codable, Equatable { } } + static func decode(from data: Data, fallback: StoredShortcut) -> StoredShortcut { + guard !data.isEmpty, + let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else { + return fallback + } + return shortcut + } + static func from(event: NSEvent) -> StoredShortcut? { guard let key = storedKey(from: event) else { return nil } diff --git a/Sources/NotificationsPage.swift b/Sources/NotificationsPage.swift index 370477f08c5d..2cbad17d826e 100644 --- a/Sources/NotificationsPage.swift +++ b/Sources/NotificationsPage.swift @@ -131,7 +131,7 @@ struct NotificationsPage: View { } private var jumpToUnreadShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: jumpToUnreadShortcutData, fallback: KeyboardShortcutSettings.Action.jumpToUnread.defaultShortcut ) @@ -141,14 +141,6 @@ struct NotificationsPage: View { notificationStore.notifications.contains(where: { !$0.isRead }) } - private func decodeShortcut(from data: Data, fallback: StoredShortcut) -> StoredShortcut { - guard !data.isEmpty, - let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else { - return fallback - } - return shortcut - } - private func tabTitle(for tabId: UUID) -> String? { AppDelegate.shared?.tabTitle(for: tabId) ?? tabManager.tabs.first(where: { $0.id == tabId })?.title } diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 901aefae6b21..7907574cfa7b 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -2651,8 +2651,8 @@ class TerminalController { let surfaces: [[String: Any]] = pane.panelIds.enumerated().compactMap { surfaceIndex, panelId in guard let panel = panelSnapshotsById[panelId] else { return nil } var item: [String: Any] = [ - "id": NSNull(), - "ref": NSNull(), + "id": panel.id.uuidString, + "ref": v2Ref(kind: .surface, uuid: panel.id), "index": surfaceIndex, "type": panel.type.rawValue, "title": panel.customTitle ?? panel.title ?? "", @@ -2672,21 +2672,38 @@ class TerminalController { } let focused = surfaces.contains { ($0["focused"] as? Bool) == true } + let storedPaneSurfaces = Self.filteredStoredPaneSurfaces( + panelIds: pane.panelIds, + selectedPanelId: pane.selectedPanelId, + presentPanelIds: Set(panelSnapshotsById.keys) + ) + let surfaceIds = storedPaneSurfaces.panelIds.map(\.uuidString) + let surfaceRefs = storedPaneSurfaces.panelIds.map { v2Ref(kind: .surface, uuid: $0) } return [ "id": NSNull(), "ref": NSNull(), "index": paneIndex, "focused": focused, - "surface_ids": [], - "surface_refs": [], - "selected_surface_id": NSNull(), - "selected_surface_ref": NSNull(), + "surface_ids": surfaceIds, + "surface_refs": surfaceRefs, + "selected_surface_id": v2OrNull(storedPaneSurfaces.selectedPanelId?.uuidString), + "selected_surface_ref": v2Ref(kind: .surface, uuid: storedPaneSurfaces.selectedPanelId), "surface_count": surfaces.count, "surfaces": surfaces ] } } + static func filteredStoredPaneSurfaces( + panelIds: [UUID], + selectedPanelId: UUID?, + presentPanelIds: Set + ) -> (panelIds: [UUID], selectedPanelId: UUID?) { + let filteredPanelIds = panelIds.filter { presentPanelIds.contains($0) } + let filteredSelectedPanelId = selectedPanelId.flatMap { filteredPanelIds.contains($0) ? $0 : nil } + return (filteredPanelIds, filteredSelectedPanelId) + } + private func v2TreePaneSnapshots(in layout: SessionWorkspaceLayoutSnapshot) -> [SessionPaneLayoutSnapshot] { switch layout { case .pane(let pane): @@ -2917,11 +2934,6 @@ class TerminalController { return tm } } - if let pageId = v2UUID(params, "page_id") { - if let tm = v2MainSync({ self.v2LocatePage(pageId)?.tabManager }) { - return tm - } - } return tabManager } @@ -3584,7 +3596,11 @@ class TerminalController { } let title = v2String(params, "title") - let select = v2FocusAllowed(requested: v2Bool(params, "select") ?? true) + let parsedSelect = v2Bool(params, "select") + if params["select"] != nil && parsedSelect == nil { + return .err(code: "invalid_params", message: "Missing or invalid select", data: nil) + } + let select = v2FocusAllowed(requested: parsedSelect ?? false) var result: V2CallResult = .err(code: "not_found", message: "Workspace not found", data: nil) v2MainSync { @@ -3604,35 +3620,34 @@ class TerminalController { } private func v2PageDuplicate(params: [String: Any]) -> V2CallResult { - guard let tabManager = v2ResolveTabManager(params: params) else { - return .err(code: "unavailable", message: "TabManager not available", data: nil) - } - let requestedPageId = v2UUID(params, "page_id") let title = v2String(params, "title") - let select = v2FocusAllowed(requested: v2Bool(params, "select") ?? true) + let parsedSelect = v2Bool(params, "select") + if params["select"] != nil && parsedSelect == nil { + return .err(code: "invalid_params", message: "Missing or invalid select", data: nil) + } + let select = v2FocusAllowed(requested: parsedSelect ?? false) var result: V2CallResult = .err(code: "not_found", message: "Page not found", data: nil) v2MainSync { - guard let workspace = v2ResolveWorkspace(params: params, tabManager: tabManager) else { return } - let sourcePageId = requestedPageId.flatMap { candidate in - workspace.pageIndex(pageId: candidate) != nil ? candidate : nil - } ?? workspace.activePageId - - guard workspace.pageIndex(pageId: sourcePageId) != nil else { - result = .err(code: "not_found", message: "Page not found", data: [ - "page_id": v2OrNull(requestedPageId?.uuidString), - "page_ref": v2Ref(kind: .page, uuid: requestedPageId) - ]) + let resolved: (tabManager: TabManager, workspace: Workspace) + let resolution = v2ResolveWorkspaceForPage(params: params, pageId: requestedPageId) + switch resolution { + case .resolved(let tabManager, let workspace): + resolved = (tabManager, workspace) + case .pageNotFound, .pageNotFoundInScopedWorkspace, .windowNotFound, .workspaceNotFound, .surfaceNotFound, .tabNotFound, .noActiveWorkspace, .invalidParams: + result = v2PageResolutionResult(resolution, params: params, pageId: requestedPageId) return } + let sourcePageId = requestedPageId ?? resolved.workspace.activePageId + if select { - v2MaybeFocusWindow(for: tabManager) - v2MaybeSelectWorkspace(tabManager, workspace: workspace) + v2MaybeFocusWindow(for: resolved.tabManager) + v2MaybeSelectWorkspace(resolved.tabManager, workspace: resolved.workspace) } - guard let page = workspace.duplicatePage( + guard let page = resolved.workspace.duplicatePage( sourcePageId: sourcePageId, select: select, title: title @@ -3644,7 +3659,7 @@ class TerminalController { return } - result = .ok(v2PageResultPayload(tabManager: tabManager, workspace: workspace, pageId: page.id)) + result = .ok(v2PageResultPayload(tabManager: resolved.tabManager, workspace: resolved.workspace, pageId: page.id)) } return result @@ -3699,41 +3714,57 @@ class TerminalController { return .err(code: "invalid_params", message: "Missing or invalid page_id", data: nil) } - let force = v2Bool(params, "force") ?? true + let parsedForce = v2Bool(params, "force") + if params["force"] != nil && parsedForce == nil { + return .err(code: "invalid_params", message: "Missing or invalid force", data: nil) + } + let force = parsedForce ?? false + let pageData: [String: Any] = [ + "page_id": pageId.uuidString, + "page_ref": v2Ref(kind: .page, uuid: pageId) + ] + if !force { + return .err( + code: "interactive_not_allowed", + message: "page.close requires force=true because socket commands cannot show confirmation UI", + data: pageData + ) + } + var result: V2CallResult = .err(code: "not_found", message: "Page not found", data: [ "page_id": pageId.uuidString, "page_ref": v2Ref(kind: .page, uuid: pageId) ]) v2MainSync { - let located: (windowId: UUID?, tabManager: TabManager, workspace: Workspace, pageIndex: Int)? - if let explicitTabManager = v2ResolveTabManager(params: params), - let workspace = v2ResolveWorkspace(params: params, tabManager: explicitTabManager), - let pageIndex = workspace.pageIndex(pageId: pageId) { - located = (v2ResolveWindowId(tabManager: explicitTabManager), explicitTabManager, workspace, pageIndex) - } else { - let resolved = v2LocatePage(pageId) - located = resolved.map { ($0.windowId, $0.tabManager, $0.workspace, $0.pageIndex) } + let resolved: (tabManager: TabManager, workspace: Workspace) + let resolution = v2ResolveWorkspaceForPage(params: params, pageId: pageId) + switch resolution { + case .resolved(let tabManager, let workspace): + resolved = (tabManager, workspace) + case .pageNotFound, .pageNotFoundInScopedWorkspace, .windowNotFound, .workspaceNotFound, .surfaceNotFound, .tabNotFound, .noActiveWorkspace, .invalidParams: + result = v2PageResolutionResult(resolution, params: params, pageId: pageId) + return } - guard let located else { return } - guard located.workspace.canClosePage(pageId) else { + let windowId = v2ResolveWindowId(tabManager: resolved.tabManager) + guard resolved.workspace.canClosePage(pageId) else { result = .err(code: "invalid_state", message: "Cannot close the last page in a workspace", data: [ "page_id": pageId.uuidString, "page_ref": v2Ref(kind: .page, uuid: pageId) ]) return } - located.workspace.closePage(pageId, skipConfirmation: force) + resolved.workspace.closePage(pageId, skipConfirmation: true) result = .ok([ - "window_id": v2OrNull(located.windowId?.uuidString), - "window_ref": v2Ref(kind: .window, uuid: located.windowId), - "workspace_id": located.workspace.id.uuidString, - "workspace_ref": v2Ref(kind: .workspace, uuid: located.workspace.id), + "window_id": v2OrNull(windowId?.uuidString), + "window_ref": v2Ref(kind: .window, uuid: windowId), + "workspace_id": resolved.workspace.id.uuidString, + "workspace_ref": v2Ref(kind: .workspace, uuid: resolved.workspace.id), "page_id": pageId.uuidString, "page_ref": v2Ref(kind: .page, uuid: pageId), - "selected_page_id": located.workspace.activePageId.uuidString, - "selected_page_ref": v2Ref(kind: .page, uuid: located.workspace.activePageId) + "selected_page_id": resolved.workspace.activePageId.uuidString, + "selected_page_ref": v2Ref(kind: .page, uuid: resolved.workspace.activePageId) ]) } @@ -3741,12 +3772,6 @@ class TerminalController { } private func v2PageReorder(params: [String: Any]) -> V2CallResult { - guard let tabManager = v2ResolveTabManager(params: params) else { - return .err(code: "unavailable", message: "TabManager not available", data: nil) - } - guard let workspace = v2ResolveWorkspace(params: params, tabManager: tabManager) else { - return .err(code: "not_found", message: "Workspace not found", data: nil) - } guard let pageId = v2UUID(params, "page_id") else { return .err(code: "invalid_params", message: "Missing or invalid page_id", data: nil) } @@ -3763,39 +3788,77 @@ class TerminalController { ) } - var moved = false - var newIndex: Int? + var result: V2CallResult = .err(code: "not_found", message: "Page not found", data: [ + "page_id": pageId.uuidString, + "page_ref": v2Ref(kind: .page, uuid: pageId) + ]) v2MainSync { - guard workspace.pageIndex(pageId: pageId) != nil else { return } + let resolved: (tabManager: TabManager, workspace: Workspace) + let resolution = v2ResolveWorkspaceForPage(params: params, pageId: pageId) + switch resolution { + case .resolved(let tabManager, let workspace): + resolved = (tabManager, workspace) + case .pageNotFound, .pageNotFoundInScopedWorkspace, .windowNotFound, .workspaceNotFound, .surfaceNotFound, .tabNotFound, .noActiveWorkspace, .invalidParams: + result = v2PageResolutionResult(resolution, params: params, pageId: pageId) + return + } + + guard let sourceIndex = resolved.workspace.pageIndex(pageId: pageId) else { return } + let destinationIndex: Int if let index { - moved = workspace.movePage(pageId: pageId, toIndex: index) - } else if let beforeId, let beforeIndex = workspace.pageIndex(pageId: beforeId) { - moved = workspace.movePage(pageId: pageId, toIndex: beforeIndex) - } else if let afterId, let afterIndex = workspace.pageIndex(pageId: afterId) { - moved = workspace.movePage(pageId: pageId, toIndex: afterIndex) + destinationIndex = index + } else if let beforeId { + guard let beforeIndex = resolved.workspace.pageIndex(pageId: beforeId) else { + result = .err(code: "not_found", message: "Anchor page not found", data: [ + "page_id": beforeId.uuidString, + "page_ref": v2Ref(kind: .page, uuid: beforeId) + ]) + return + } + if beforeId == pageId { + destinationIndex = sourceIndex + } else if sourceIndex < beforeIndex { + destinationIndex = beforeIndex - 1 + } else { + destinationIndex = beforeIndex + } + } else if let afterId { + guard let afterIndex = resolved.workspace.pageIndex(pageId: afterId) else { + result = .err(code: "not_found", message: "Anchor page not found", data: [ + "page_id": afterId.uuidString, + "page_ref": v2Ref(kind: .page, uuid: afterId) + ]) + return + } + if afterId == pageId { + destinationIndex = sourceIndex + } else if sourceIndex < afterIndex { + destinationIndex = afterIndex + } else { + destinationIndex = afterIndex + 1 + } + } else { + result = .err(code: "invalid_params", message: "Missing reorder target", data: nil) + return } - newIndex = workspace.pageIndex(pageId: pageId) - } - guard moved else { - return .err(code: "not_found", message: "Page not found", data: [ - "page_id": pageId.uuidString, - "page_ref": v2Ref(kind: .page, uuid: pageId) - ]) - } + guard resolved.workspace.movePage(pageId: pageId, toIndex: destinationIndex) else { + result = .err( + code: "invalid_state", + message: "Cannot reorder the only page in a workspace", + data: v2ScopedPageLookupData(params: params, pageId: pageId) + ) + return + } - var payload = v2PageResultPayload(tabManager: tabManager, workspace: workspace, pageId: pageId) - payload["index"] = v2OrNull(newIndex) - return .ok(payload) + var payload = v2PageResultPayload(tabManager: resolved.tabManager, workspace: resolved.workspace, pageId: pageId) + payload["index"] = v2OrNull(resolved.workspace.pageIndex(pageId: pageId)) + result = .ok(payload) + } + return result } private func v2PageRename(params: [String: Any]) -> V2CallResult { - guard let tabManager = v2ResolveTabManager(params: params) else { - return .err(code: "unavailable", message: "TabManager not available", data: nil) - } - guard let workspace = v2ResolveWorkspace(params: params, tabManager: tabManager) else { - return .err(code: "not_found", message: "Workspace not found", data: nil) - } guard let pageId = v2UUID(params, "page_id") else { return .err(code: "invalid_params", message: "Missing or invalid page_id", data: nil) } @@ -3803,20 +3866,27 @@ class TerminalController { return .err(code: "invalid_params", message: "Missing or invalid title", data: nil) } - var found = false + var result: V2CallResult = .err(code: "not_found", message: "Page not found", data: [ + "page_id": pageId.uuidString, + "page_ref": v2Ref(kind: .page, uuid: pageId) + ]) v2MainSync { - guard workspace.pageIndex(pageId: pageId) != nil else { return } - workspace.setPageTitle(pageId: pageId, title: title) - found = true - } + let resolved: (tabManager: TabManager, workspace: Workspace) + let resolution = v2ResolveWorkspaceForPage(params: params, pageId: pageId) + switch resolution { + case .resolved(let tabManager, let workspace): + resolved = (tabManager, workspace) + case .pageNotFound, .pageNotFoundInScopedWorkspace, .windowNotFound, .workspaceNotFound, .surfaceNotFound, .tabNotFound, .noActiveWorkspace, .invalidParams: + result = v2PageResolutionResult(resolution, params: params, pageId: pageId) + return + } - guard found else { - return .err(code: "not_found", message: "Page not found", data: [ - "page_id": pageId.uuidString, - "page_ref": v2Ref(kind: .page, uuid: pageId) - ]) + guard resolved.workspace.pageIndex(pageId: pageId) != nil else { return } + resolved.workspace.setPageTitle(pageId: pageId, title: title) + result = .ok(v2PageResultPayload(tabManager: resolved.tabManager, workspace: resolved.workspace, pageId: pageId)) } - return .ok(v2PageResultPayload(tabManager: tabManager, workspace: workspace, pageId: pageId)) + + return result } private func v2PageNext(params: [String: Any]) -> V2CallResult { @@ -4140,9 +4210,6 @@ class TerminalController { if let wsId = v2UUID(params, "workspace_id") { return tabManager.tabs.first(where: { $0.id == wsId }) } - if let pageId = v2UUID(params, "page_id") { - return tabManager.tabs.first(where: { $0.pageIndex(pageId: pageId) != nil }) - } if let surfaceId = v2UUID(params, "surface_id") ?? v2UUID(params, "tab_id") { return tabManager.tabs.first(where: { $0.panels[surfaceId] != nil }) } @@ -4150,6 +4217,212 @@ class TerminalController { return tabManager.tabs.first(where: { $0.id == wsId }) } + private enum V2PageWorkspaceResolution { + case resolved(tabManager: TabManager, workspace: Workspace) + case pageNotFound + case pageNotFoundInScopedWorkspace + case windowNotFound + case workspaceNotFound + case surfaceNotFound + case tabNotFound + case noActiveWorkspace + case invalidParams + } + + private func v2HasExplicitPageResolutionScope(params: [String: Any]) -> Bool { + params["workspace_id"] != nil + || params["window_id"] != nil + || params["surface_id"] != nil + || params["tab_id"] != nil + } + + private func v2ScopedWorkspaceLookupData(params: [String: Any]) -> [String: Any]? { + var data: [String: Any] = [:] + + if params["window_id"] != nil { + let windowId = v2UUID(params, "window_id") + data["window_id"] = v2OrNull(windowId?.uuidString) + data["window_ref"] = v2Ref(kind: .window, uuid: windowId) + } + if params["workspace_id"] != nil { + let workspaceId = v2UUID(params, "workspace_id") + data["workspace_id"] = v2OrNull(workspaceId?.uuidString) + data["workspace_ref"] = v2Ref(kind: .workspace, uuid: workspaceId) + } + if params["surface_id"] != nil { + let surfaceId = v2UUID(params, "surface_id") + data["surface_id"] = v2OrNull(surfaceId?.uuidString) + data["surface_ref"] = v2Ref(kind: .surface, uuid: surfaceId) + } + if params["tab_id"] != nil { + let tabId = v2UUID(params, "tab_id") + data["tab_id"] = v2OrNull(tabId?.uuidString) + data["tab_ref"] = v2TabRef(uuid: tabId) + } + + return data.isEmpty ? nil : data + } + + private func v2ScopedPageLookupData(params: [String: Any], pageId: UUID?) -> [String: Any] { + var data = v2ScopedWorkspaceLookupData(params: params) ?? [:] + data["page_id"] = v2OrNull(pageId?.uuidString) + data["page_ref"] = v2Ref(kind: .page, uuid: pageId) + return data + } + + private func v2PageResolutionInvalidParamsResult(params: [String: Any], pageId: UUID?) -> V2CallResult { + if params["window_id"] != nil && v2UUID(params, "window_id") == nil { + return .err(code: "invalid_params", message: "Missing or invalid window_id", data: nil) + } + if params["workspace_id"] != nil && v2UUID(params, "workspace_id") == nil { + return .err(code: "invalid_params", message: "Missing or invalid workspace_id", data: nil) + } + if params["surface_id"] != nil && v2UUID(params, "surface_id") == nil { + return .err(code: "invalid_params", message: "Missing or invalid surface_id", data: nil) + } + if params["tab_id"] != nil && v2UUID(params, "tab_id") == nil { + return .err(code: "invalid_params", message: "Missing or invalid tab_id", data: nil) + } + if params["page_id"] != nil && pageId == nil { + return .err(code: "invalid_params", message: "Missing or invalid page_id", data: nil) + } + return .err(code: "invalid_params", message: "Invalid page routing parameters", data: nil) + } + + private func v2PageResolutionResult( + _ resolution: V2PageWorkspaceResolution, + params: [String: Any], + pageId: UUID? + ) -> V2CallResult { + switch resolution { + case .resolved: + return .err(code: "internal_error", message: "Unexpected page resolution state", data: nil) + case .pageNotFound: + return .err(code: "not_found", message: "Page not found", data: [ + "page_id": v2OrNull(pageId?.uuidString), + "page_ref": v2Ref(kind: .page, uuid: pageId) + ]) + case .pageNotFoundInScopedWorkspace: + return .err(code: "not_found", message: "Page not found", data: v2ScopedPageLookupData(params: params, pageId: pageId)) + case .windowNotFound: + let windowId = v2UUID(params, "window_id") + return .err(code: "not_found", message: "Window not found", data: [ + "window_id": v2OrNull(windowId?.uuidString), + "window_ref": v2Ref(kind: .window, uuid: windowId) + ]) + case .workspaceNotFound: + return .err(code: "not_found", message: "Workspace not found", data: v2ScopedWorkspaceLookupData(params: params)) + case .surfaceNotFound: + return .err(code: "not_found", message: "Surface not found", data: v2ScopedWorkspaceLookupData(params: params)) + case .tabNotFound: + return .err(code: "not_found", message: "Tab not found", data: v2ScopedWorkspaceLookupData(params: params)) + case .noActiveWorkspace: + return .err(code: "not_found", message: "Workspace not found", data: nil) + case .invalidParams: + return v2PageResolutionInvalidParamsResult(params: params, pageId: pageId) + } + } + + private func v2ResolveWorkspaceForPage( + params: [String: Any], + pageId: UUID? + ) -> V2PageWorkspaceResolution { + let hasExplicitScope = v2HasExplicitPageResolutionScope(params: params) + let isWindowOnlyScope = + params["window_id"] != nil + && params["workspace_id"] == nil + && params["surface_id"] == nil + && params["tab_id"] == nil + if params["window_id"] != nil && v2UUID(params, "window_id") == nil { + return .invalidParams + } + if params["workspace_id"] != nil && v2UUID(params, "workspace_id") == nil { + return .invalidParams + } + if params["surface_id"] != nil && v2UUID(params, "surface_id") == nil { + return .invalidParams + } + if params["tab_id"] != nil && v2UUID(params, "tab_id") == nil { + return .invalidParams + } + if params["page_id"] != nil && pageId == nil { + return .invalidParams + } + + let routedTabManager: TabManager? + if let windowId = v2UUID(params, "window_id") { + guard let tabManager = v2MainSync({ AppDelegate.shared?.tabManagerFor(windowId: windowId) }) else { + return .windowNotFound + } + routedTabManager = tabManager + } else if let workspaceId = v2UUID(params, "workspace_id") { + guard let tabManager = v2MainSync({ AppDelegate.shared?.tabManagerFor(tabId: workspaceId) }) else { + return .workspaceNotFound + } + routedTabManager = tabManager + } else if let surfaceId = v2UUID(params, "surface_id") { + guard let tabManager = v2MainSync({ AppDelegate.shared?.locateSurface(surfaceId: surfaceId)?.tabManager }) else { + return .surfaceNotFound + } + routedTabManager = tabManager + } else if let tabId = v2UUID(params, "tab_id") { + guard let tabManager = v2MainSync({ AppDelegate.shared?.locateSurface(surfaceId: tabId)?.tabManager }) else { + return .tabNotFound + } + routedTabManager = tabManager + } else { + routedTabManager = tabManager + } + + if let routedTabManager, + let workspace = v2ResolveWorkspace(params: params, tabManager: routedTabManager) { + if let pageId { + if workspace.pageIndex(pageId: pageId) != nil { + return .resolved(tabManager: routedTabManager, workspace: workspace) + } + if isWindowOnlyScope, + let scopedWorkspace = routedTabManager.tabs.first(where: { $0.pageIndex(pageId: pageId) != nil }) { + return .resolved(tabManager: routedTabManager, workspace: scopedWorkspace) + } + if hasExplicitScope { + return .pageNotFoundInScopedWorkspace + } + } else { + return .resolved(tabManager: routedTabManager, workspace: workspace) + } + } else if let routedTabManager, + let pageId, + isWindowOnlyScope, + let scopedWorkspace = routedTabManager.tabs.first(where: { $0.pageIndex(pageId: pageId) != nil }) { + return .resolved(tabManager: routedTabManager, workspace: scopedWorkspace) + } + + if let pageId, + !hasExplicitScope, + let located = v2LocatePage(pageId) { + return .resolved(tabManager: located.tabManager, workspace: located.workspace) + } + + if hasExplicitScope { + if params["workspace_id"] != nil { + return .workspaceNotFound + } + if params["surface_id"] != nil { + return .surfaceNotFound + } + if params["tab_id"] != nil { + return .tabNotFound + } + return .noActiveWorkspace + } + + if pageId != nil { + return .pageNotFound + } + + return .noActiveWorkspace + } + private func v2SurfaceList(params: [String: Any]) -> V2CallResult { guard let tabManager = v2ResolveTabManager(params: params) else { return .err(code: "unavailable", message: "TabManager not available", data: nil) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index f3d841d0288e..7e002b59611a 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -112,7 +112,7 @@ extension Workspace { if page.id == activePageId { state = activePageSnapshot } else if let storedState = storedPageStates[page.id] { - state = storedState.sessionState + state = exportedPageStateSnapshot(storedState.sessionState, includeScrollback: includeScrollback) } else { state = emptyPageSessionStateSnapshot(currentDirectory: currentDirectory) } @@ -165,6 +165,7 @@ extension Workspace { pageModels.contains(where: { $0.id == candidate }) ? candidate : nil } ?? pageModels.first?.id ?? activePageId + teardownStoredPageStates() storedPageStates.removeAll(keepingCapacity: false) pages = pageModels activePageId = restoredActivePageId @@ -246,6 +247,30 @@ extension Workspace { ) } + private func exportedPageStateSnapshot( + _ snapshot: SessionWorkspacePageStateSnapshot, + includeScrollback: Bool + ) -> SessionWorkspacePageStateSnapshot { + guard !includeScrollback else { return snapshot } + + var strippedSnapshot = snapshot + strippedSnapshot.panels = snapshot.panels.map { panel in + var strippedPanel = panel + if var terminal = strippedPanel.terminal { + terminal.scrollback = nil + strippedPanel.terminal = terminal + } + return strippedPanel + } + return strippedSnapshot + } + + private func teardownStoredPageStates() { + for storedState in storedPageStates.values { + teardownStoredPageState(storedState) + } + } + private func restoreSessionPageState(_ snapshot: SessionWorkspacePageStateSnapshot) { restoredTerminalScrollbackByPanelId.removeAll(keepingCapacity: false) metadataBlocks = [:] @@ -1396,7 +1421,8 @@ final class Workspace: Identifiable, ObservableObject { if pageId == activePageId { return currentPageSessionStateSnapshot(includeScrollback: includeScrollback) } - return storedPageStates[pageId]?.sessionState + guard let storedState = storedPageStates[pageId] else { return nil } + return exportedPageStateSnapshot(storedState.sessionState, includeScrollback: includeScrollback) } func pageStructureSummary(pageId: UUID) -> (paneCount: Int, surfaceCount: Int)? { @@ -1414,7 +1440,7 @@ final class Workspace: Identifiable, ObservableObject { } @discardableResult - func newPage(select: Bool = true) -> WorkspacePage { + func newPage(select: Bool = false) -> WorkspacePage { let page = WorkspacePage( id: UUID(), title: defaultPageTitle(number: nextAutoPageNumber) @@ -1436,7 +1462,7 @@ final class Workspace: Identifiable, ObservableObject { @discardableResult func duplicatePage( sourcePageId: UUID, - select: Bool = true, + select: Bool = false, title: String? = nil ) -> WorkspacePage? { guard let sourceIndex = pageIndex(pageId: sourcePageId), @@ -1459,7 +1485,7 @@ final class Workspace: Identifiable, ObservableObject { selectPage(page.id) } - return page + return pages.first(where: { $0.id == page.id }) } func selectPage(_ pageId: UUID) { @@ -1557,6 +1583,7 @@ final class Workspace: Identifiable, ObservableObject { if let replacementPageId { restoreStoredPage(replacementPageId) activePageId = replacementPageId + requestBackgroundTerminalSurfaceStartIfNeeded() } } else { if let storedState = storedPageStates.removeValue(forKey: pageId) { @@ -1726,13 +1753,31 @@ final class Workspace: Identifiable, ObservableObject { runtimeState: nil ) - if let runtimeState = storedState.runtimeState { + if let runtimeState = storedState.runtimeState, + Self.canRestoreRuntimeLayout(runtimeState.layout, detachedSurfaceIds: Set(runtimeState.detachedSurfaces.keys)) { restoreRuntimePageState(runtimeState) } else { + teardownDetachedSurfaces(storedState.runtimeState?.detachedSurfaces) restoreSessionPageState(storedState.sessionState) } } + static func canRestoreRuntimeLayout( + _ layout: SessionWorkspaceLayoutSnapshot, + detachedSurfaceIds: Set + ) -> Bool { + Set(runtimeLayoutPanelIds(in: layout)).isSubset(of: detachedSurfaceIds) + } + + private static func runtimeLayoutPanelIds(in layout: SessionWorkspaceLayoutSnapshot) -> [UUID] { + switch layout { + case .pane(let pane): + return pane.panelIds + case .split(let split): + return runtimeLayoutPanelIds(in: split.first) + runtimeLayoutPanelIds(in: split.second) + } + } + private func restoreRuntimePageState(_ runtimeState: StoredPageState.RuntimeState) { currentDirectory = runtimeState.currentDirectory statusEntries = runtimeState.statusEntries @@ -1815,8 +1860,12 @@ final class Workspace: Identifiable, ObservableObject { } private func teardownStoredPageState(_ storedState: StoredPageState) { - guard let runtimeState = storedState.runtimeState else { return } - for transfer in runtimeState.detachedSurfaces.values { + teardownDetachedSurfaces(storedState.runtimeState?.detachedSurfaces) + } + + private func teardownDetachedSurfaces(_ detachedSurfaces: [UUID: DetachedSurfaceTransfer]?) { + guard let detachedSurfaces else { return } + for transfer in detachedSurfaces.values { transfer.panel.close() } } @@ -2955,6 +3004,9 @@ final class Workspace: Identifiable, ObservableObject { /// Called before the workspace is removed from TabManager to ensure child /// processes receive SIGHUP even if ARC deallocation is delayed. func teardownAllPanels() { + teardownStoredPageStates() + storedPageStates.removeAll(keepingCapacity: false) + let panelEntries = Array(panels) for (panelId, panel) in panelEntries { panelSubscriptions.removeValue(forKey: panelId) diff --git a/Sources/WorkspaceContentView.swift b/Sources/WorkspaceContentView.swift index e8c087ac9c4d..03d3240bb246 100644 --- a/Sources/WorkspaceContentView.swift +++ b/Sources/WorkspaceContentView.swift @@ -336,19 +336,11 @@ struct EmptyPanelView: View { } private var newSurfaceShortcut: StoredShortcut { - decodeShortcut(from: newSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.newSurface.defaultShortcut) + StoredShortcut.decode(from: newSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.newSurface.defaultShortcut) } private var openBrowserShortcut: StoredShortcut { - decodeShortcut(from: openBrowserShortcutData, fallback: KeyboardShortcutSettings.Action.openBrowser.defaultShortcut) - } - - private func decodeShortcut(from data: Data, fallback: StoredShortcut) -> StoredShortcut { - guard !data.isEmpty, - let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else { - return fallback - } - return shortcut + StoredShortcut.decode(from: openBrowserShortcutData, fallback: KeyboardShortcutSettings.Action.openBrowser.defaultShortcut) } @ViewBuilder diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index 18bb18ab6ade..10e0ff860867 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -42,6 +42,15 @@ struct cmuxApp: App { @AppStorage(KeyboardShortcutSettings.Action.closePage.defaultsKey) private var closePageShortcutData = Data() @AppStorage(KeyboardShortcutSettings.Action.nextPage.defaultsKey) private var nextPageShortcutData = Data() @AppStorage(KeyboardShortcutSettings.Action.previousPage.defaultsKey) private var previousPageShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage1.defaultsKey) private var selectPage1ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage2.defaultsKey) private var selectPage2ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage3.defaultsKey) private var selectPage3ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage4.defaultsKey) private var selectPage4ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage5.defaultsKey) private var selectPage5ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage6.defaultsKey) private var selectPage6ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage7.defaultsKey) private var selectPage7ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectPage8.defaultsKey) private var selectPage8ShortcutData = Data() + @AppStorage(KeyboardShortcutSettings.Action.selectLastPage.defaultsKey) private var selectLastPageShortcutData = Data() @NSApplicationDelegateAdaptor(AppDelegate.self) private var appDelegate init() { @@ -775,137 +784,137 @@ struct cmuxApp: App { } private var splitRightMenuShortcut: StoredShortcut { - decodeShortcut(from: splitRightShortcutData, fallback: KeyboardShortcutSettings.Action.splitRight.defaultShortcut) + StoredShortcut.decode(from: splitRightShortcutData, fallback: KeyboardShortcutSettings.Action.splitRight.defaultShortcut) } private var toggleSidebarMenuShortcut: StoredShortcut { - decodeShortcut(from: toggleSidebarShortcutData, fallback: KeyboardShortcutSettings.Action.toggleSidebar.defaultShortcut) + StoredShortcut.decode(from: toggleSidebarShortcutData, fallback: KeyboardShortcutSettings.Action.toggleSidebar.defaultShortcut) } private var newWorkspaceMenuShortcut: StoredShortcut { - decodeShortcut(from: newWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.newTab.defaultShortcut) + StoredShortcut.decode(from: newWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.newTab.defaultShortcut) } private var newWindowMenuShortcut: StoredShortcut { - decodeShortcut(from: newWindowShortcutData, fallback: KeyboardShortcutSettings.Action.newWindow.defaultShortcut) + StoredShortcut.decode(from: newWindowShortcutData, fallback: KeyboardShortcutSettings.Action.newWindow.defaultShortcut) } private var openFolderMenuShortcut: StoredShortcut { - decodeShortcut(from: openFolderShortcutData, fallback: KeyboardShortcutSettings.Action.openFolder.defaultShortcut) + StoredShortcut.decode(from: openFolderShortcutData, fallback: KeyboardShortcutSettings.Action.openFolder.defaultShortcut) } private var showNotificationsMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: showNotificationsShortcutData, fallback: KeyboardShortcutSettings.Action.showNotifications.defaultShortcut ) } private var jumpToUnreadMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: jumpToUnreadShortcutData, fallback: KeyboardShortcutSettings.Action.jumpToUnread.defaultShortcut ) } private var nextSurfaceMenuShortcut: StoredShortcut { - decodeShortcut(from: nextSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.nextSurface.defaultShortcut) + StoredShortcut.decode(from: nextSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.nextSurface.defaultShortcut) } private var prevSurfaceMenuShortcut: StoredShortcut { - decodeShortcut(from: prevSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.prevSurface.defaultShortcut) + StoredShortcut.decode(from: prevSurfaceShortcutData, fallback: KeyboardShortcutSettings.Action.prevSurface.defaultShortcut) } private var nextWorkspaceMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: nextWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.nextSidebarTab.defaultShortcut ) } private var prevWorkspaceMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: prevWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.prevSidebarTab.defaultShortcut ) } private var splitDownMenuShortcut: StoredShortcut { - decodeShortcut(from: splitDownShortcutData, fallback: KeyboardShortcutSettings.Action.splitDown.defaultShortcut) + StoredShortcut.decode(from: splitDownShortcutData, fallback: KeyboardShortcutSettings.Action.splitDown.defaultShortcut) } private var toggleBrowserDeveloperToolsMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: toggleBrowserDeveloperToolsShortcutData, fallback: KeyboardShortcutSettings.Action.toggleBrowserDeveloperTools.defaultShortcut ) } private var showBrowserJavaScriptConsoleMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: showBrowserJavaScriptConsoleShortcutData, fallback: KeyboardShortcutSettings.Action.showBrowserJavaScriptConsole.defaultShortcut ) } private var splitBrowserRightMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: splitBrowserRightShortcutData, fallback: KeyboardShortcutSettings.Action.splitBrowserRight.defaultShortcut ) } private var splitBrowserDownMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: splitBrowserDownShortcutData, fallback: KeyboardShortcutSettings.Action.splitBrowserDown.defaultShortcut ) } private var renameWorkspaceMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: renameWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.renameWorkspace.defaultShortcut ) } private var closeWorkspaceMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: closeWorkspaceShortcutData, fallback: KeyboardShortcutSettings.Action.closeWorkspace.defaultShortcut ) } private var newPageMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: newPageShortcutData, fallback: KeyboardShortcutSettings.Action.newPage.defaultShortcut ) } private var renamePageMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: renamePageShortcutData, fallback: KeyboardShortcutSettings.Action.renamePage.defaultShortcut ) } private var closePageMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: closePageShortcutData, fallback: KeyboardShortcutSettings.Action.closePage.defaultShortcut ) } private var nextPageMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: nextPageShortcutData, fallback: KeyboardShortcutSettings.Action.nextPage.defaultShortcut ) } private var previousPageMenuShortcut: StoredShortcut { - decodeShortcut( + StoredShortcut.decode( from: previousPageShortcutData, fallback: KeyboardShortcutSettings.Action.previousPage.defaultShortcut ) @@ -921,14 +930,6 @@ struct cmuxApp: App { ) ?? tabManager } - private func decodeShortcut(from data: Data, fallback: StoredShortcut) -> StoredShortcut { - guard !data.isEmpty, - let shortcut = try? JSONDecoder().decode(StoredShortcut.self, from: data) else { - return fallback - } - return shortcut - } - private func notificationMenuItemTitle(for notification: TerminalNotification) -> String { let tabTitle = appDelegate.tabTitle(for: notification.tabId) return MenuBarNotificationLineFormatter.menuTitle(notification: notification, tabTitle: tabTitle) @@ -969,27 +970,54 @@ struct cmuxApp: App { private func pageSelectionMenuShortcut(index: Int, pageCount: Int) -> StoredShortcut? { switch index { case 0: - return KeyboardShortcutSettings.shortcut(for: .selectPage1) + return StoredShortcut.decode( + from: selectPage1ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage1.defaultShortcut + ) case 1: - return KeyboardShortcutSettings.shortcut(for: .selectPage2) + return StoredShortcut.decode( + from: selectPage2ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage2.defaultShortcut + ) case 2: - return KeyboardShortcutSettings.shortcut(for: .selectPage3) + return StoredShortcut.decode( + from: selectPage3ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage3.defaultShortcut + ) case 3: - return KeyboardShortcutSettings.shortcut(for: .selectPage4) + return StoredShortcut.decode( + from: selectPage4ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage4.defaultShortcut + ) case 4: - return KeyboardShortcutSettings.shortcut(for: .selectPage5) + return StoredShortcut.decode( + from: selectPage5ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage5.defaultShortcut + ) case 5: - return KeyboardShortcutSettings.shortcut(for: .selectPage6) + return StoredShortcut.decode( + from: selectPage6ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage6.defaultShortcut + ) case 6: - return KeyboardShortcutSettings.shortcut(for: .selectPage7) + return StoredShortcut.decode( + from: selectPage7ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage7.defaultShortcut + ) case 7: - return KeyboardShortcutSettings.shortcut(for: .selectPage8) + return StoredShortcut.decode( + from: selectPage8ShortcutData, + fallback: KeyboardShortcutSettings.Action.selectPage8.defaultShortcut + ) default: break } if index == pageCount - 1 { - return KeyboardShortcutSettings.shortcut(for: .selectLastPage) + return StoredShortcut.decode( + from: selectLastPageShortcutData, + fallback: KeyboardShortcutSettings.Action.selectLastPage.defaultShortcut + ) } return nil } diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 9832f13d9af7..f903ed5d7715 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -899,7 +899,7 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { } } - func testOptionDigitPageShortcutFallsBackByKeyCodeOnSymbolFirstLayouts() { + func testCommandOptionDigitPageShortcutFallsBackByKeyCodeOnSymbolFirstLayouts() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") return @@ -922,14 +922,14 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { withTemporaryShortcut(action: .selectPage1) { // Symbol-first layouts (for example AZERTY) can report "&" for the ANSI 1 key. - // Option+1 page selection should still match via keyCode fallback. + // Cmd+Option+1 page selection should still match via keyCode fallback. guard let event = makeKeyDownEvent( key: "&", - modifiers: [.option], + modifiers: [.command, .option], keyCode: 18, // kVK_ANSI_1 windowNumber: window.windowNumber ) else { - XCTFail("Failed to construct Option+1 event on ANSI 1 key") + XCTFail("Failed to construct Cmd+Option+1 event on ANSI 1 key") return } @@ -944,7 +944,7 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertNotEqual(workspace.activePage?.id, selectedBeforeShortcut) } - func testOption9SelectsLastPageInEventWindowWhenActiveManagerIsStale() { + func testCommandOption9SelectsLastPageInEventWindowWhenActiveManagerIsStale() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") return @@ -981,11 +981,11 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { guard let event = makeKeyDownEvent( key: "9", - modifiers: [.option], + modifiers: [.command, .option], keyCode: 25, // kVK_ANSI_9 windowNumber: secondWindow.windowNumber ) else { - XCTFail("Failed to construct Option+9 event") + XCTFail("Failed to construct Cmd+Option+9 event") return } @@ -998,12 +998,12 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertEqual( firstWorkspace.activePage?.id, firstWorkspaceFirstPageId, - "Option+9 must not select a page in the stale active window" + "Cmd+Option+9 must not select a page in the stale active window" ) XCTAssertEqual( secondWorkspace.activePage?.id, secondWorkspaceLastPage.id, - "Option+9 should select the last page in the event window" + "Cmd+Option+9 should select the last page in the event window" ) XCTAssertTrue(appDelegate.tabManager === secondManager, "Shortcut routing should retarget active manager to event window") } @@ -1054,7 +1054,7 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertNotEqual(workspace.activePage?.id, firstPageId) } - func testOptionRightBracketPageShortcutFallsBackByKeyCodeOnNonUSLayouts() { + func testCommandOptionRightBracketPageShortcutFallsBackByKeyCodeOnNonUSLayouts() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") return @@ -1076,14 +1076,14 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { withTemporaryShortcut(action: .nextPage) { // Some non-US layouts can report unrelated symbols for the ANSI ] key. - // Option+] should still work via keyCode fallback. + // Cmd+Option+] should still work via keyCode fallback. guard let event = makeKeyDownEvent( key: "*", - modifiers: [.option], + modifiers: [.command, .option], keyCode: 30, // kVK_ANSI_RightBracket windowNumber: window.windowNumber ) else { - XCTFail("Failed to construct Option+] event on ANSI ] key") + XCTFail("Failed to construct Cmd+Option+] event on ANSI ] key") return } @@ -1342,6 +1342,12 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { workspace.setPageTitle(pageId: firstPageId, title: "Agents") XCTAssertNotNil(workspace.newTerminalSurface(inPane: firstPaneId, focus: false)) + let expectedAgentSurfaceIds = workspace.bonsplitController + .tabs(inPane: firstPaneId) + .compactMap { workspace.panelIdFromSurfaceId($0.id)?.uuidString } + let expectedAgentSelectedSurfaceId = workspace.bonsplitController + .selectedTab(inPane: firstPaneId) + .flatMap { workspace.panelIdFromSurfaceId($0.id)?.uuidString } let secondPage = workspace.newPage(select: true) workspace.setPageTitle(pageId: secondPage.id, title: "Editor") @@ -1374,10 +1380,20 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { let agentsPage = pagesByTitle["Agents"] let editorPage = pagesByTitle["Editor"] + let agentsPane = (agentsPage?["panes"] as? [[String: Any]])?.first + let agentsSurfaces = agentsPane?["surfaces"] as? [[String: Any]] XCTAssertEqual(agentsPage?["selected"] as? Bool, false) XCTAssertEqual(editorPage?["selected"] as? Bool, true) XCTAssertEqual((agentsPage?["panes"] as? [[String: Any]])?.count, 1) XCTAssertEqual((editorPage?["panes"] as? [[String: Any]])?.count, 1) + XCTAssertEqual(agentsPane?["surface_ids"] as? [String], expectedAgentSurfaceIds) + XCTAssertEqual(agentsPane?["selected_surface_id"] as? String, expectedAgentSelectedSurfaceId) + XCTAssertEqual(agentsSurfaces?.compactMap { $0["id"] as? String }, expectedAgentSurfaceIds) + XCTAssertEqual((agentsPane?["surface_refs"] as? [String])?.count, expectedAgentSurfaceIds.count) + XCTAssertEqual(agentsSurfaces?.compactMap { $0["ref"] as? String }.count, expectedAgentSurfaceIds.count) + XCTAssertEqual(agentsPane?["selected_surface_ref"] as? String, agentsSurfaces?.first(where: { + ($0["id"] as? String) == expectedAgentSelectedSurfaceId + })?["ref"] as? String) let agentsSurfaceCount = ((agentsPage?["panes"] as? [[String: Any]])?.first?["surfaces"] as? [[String: Any]])?.count let editorSurfaceCount = ((editorPage?["panes"] as? [[String: Any]])?.first?["surfaces"] as? [[String: Any]])?.count @@ -1388,6 +1404,387 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertEqual(mirroredSurfaceCount, editorSurfaceCount) } + func testV2PageReorderBeforeAndAfterUseFinalPositionSemantics() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: windowId) + } + + guard let manager = appDelegate.tabManagerFor(windowId: windowId), + let workspace = manager.selectedWorkspace, + let firstPageId = workspace.activePage?.id else { + XCTFail("Expected test window and workspace") + return + } + + workspace.setPageTitle(pageId: firstPageId, title: "A") + let secondPage = workspace.newPage(select: false) + workspace.setPageTitle(pageId: secondPage.id, title: "B") + let thirdPage = workspace.newPage(select: false) + workspace.setPageTitle(pageId: thirdPage.id, title: "C") + let fourthPage = workspace.newPage(select: false) + workspace.setPageTitle(pageId: fourthPage.id, title: "D") + TerminalController.shared.setActiveTabManager(manager) + + let beforeResult = v2Result( + method: "page.reorder", + params: [ + "workspace_id": workspace.id.uuidString, + "page_id": firstPageId.uuidString, + "before_page_id": fourthPage.id.uuidString + ] + ) + XCTAssertEqual(workspace.pages.map(\.title), ["B", "C", "A", "D"]) + XCTAssertEqual(beforeResult["page_index"] as? Int, 2) + XCTAssertEqual(workspace.activePageId, firstPageId) + + let afterResult = v2Result( + method: "page.reorder", + params: [ + "workspace_id": workspace.id.uuidString, + "page_id": fourthPage.id.uuidString, + "after_page_id": secondPage.id.uuidString + ] + ) + XCTAssertEqual(workspace.pages.map(\.title), ["B", "D", "C", "A"]) + XCTAssertEqual(afterResult["page_index"] as? Int, 1) + XCTAssertEqual(workspace.activePageId, firstPageId) + } + + func testV2PageReorderSinglePageReturnsInvalidState() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: windowId) + } + + guard let manager = appDelegate.tabManagerFor(windowId: windowId), + let workspace = manager.selectedWorkspace, + let pageId = workspace.activePage?.id else { + XCTFail("Expected test window and workspace") + return + } + + TerminalController.shared.setActiveTabManager(manager) + + let response = v2Response( + method: "page.reorder", + params: [ + "workspace_id": workspace.id.uuidString, + "page_id": pageId.uuidString, + "index": 0 + ] + ) + XCTAssertEqual(response["ok"] as? Bool, false) + XCTAssertEqual((response["error"] as? [String: Any])?["code"] as? String, "invalid_state") + } + + func testV2PageCloseRequiresForceAndHonorsWorkspaceScope() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let firstWindowId = appDelegate.createMainWindow() + let secondWindowId = appDelegate.createMainWindow() + + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: firstWindowId) + closeWindow(withId: secondWindowId) + } + + guard let firstManager = appDelegate.tabManagerFor(windowId: firstWindowId), + let secondManager = appDelegate.tabManagerFor(windowId: secondWindowId), + let firstWorkspace = firstManager.selectedWorkspace, + let secondWorkspace = secondManager.selectedWorkspace, + let pageId = firstWorkspace.activePage?.id else { + XCTFail("Expected both window contexts to exist") + return + } + + _ = firstWorkspace.newPage(select: false) + TerminalController.shared.setActiveTabManager(firstManager) + + let noForceResponse = v2Response( + method: "page.close", + params: [ + "workspace_id": firstWorkspace.id.uuidString, + "page_id": pageId.uuidString + ] + ) + XCTAssertEqual(noForceResponse["ok"] as? Bool, false) + XCTAssertEqual((noForceResponse["error"] as? [String: Any])?["code"] as? String, "interactive_not_allowed") + XCTAssertEqual(firstWorkspace.pages.count, 2) + + let wrongScopeResponse = v2Response( + method: "page.close", + params: [ + "workspace_id": secondWorkspace.id.uuidString, + "page_id": pageId.uuidString, + "force": true + ] + ) + XCTAssertEqual(wrongScopeResponse["ok"] as? Bool, false) + XCTAssertEqual((wrongScopeResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual(firstWorkspace.pages.count, 2) + + let closeResult = v2Result( + method: "page.close", + params: [ + "workspace_id": firstWorkspace.id.uuidString, + "page_id": pageId.uuidString, + "force": true + ] + ) + XCTAssertEqual(closeResult["page_id"] as? String, pageId.uuidString) + XCTAssertEqual(firstWorkspace.pages.count, 1) + } + + func testV2PageMethodsRejectMalformedBooleanParams() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: windowId) + } + + guard let manager = appDelegate.tabManagerFor(windowId: windowId), + let workspace = manager.selectedWorkspace, + let pageId = workspace.activePage?.id else { + XCTFail("Expected window and workspace") + return + } + + TerminalController.shared.setActiveTabManager(manager) + + let malformedCreateResponse = v2Response( + method: "page.create", + params: ["select": "nope"] + ) + XCTAssertEqual(malformedCreateResponse["ok"] as? Bool, false) + XCTAssertEqual((malformedCreateResponse["error"] as? [String: Any])?["code"] as? String, "invalid_params") + + let malformedDuplicateResponse = v2Response( + method: "page.duplicate", + params: [ + "workspace_id": workspace.id.uuidString, + "page_id": pageId.uuidString, + "select": "nope" + ] + ) + XCTAssertEqual(malformedDuplicateResponse["ok"] as? Bool, false) + XCTAssertEqual((malformedDuplicateResponse["error"] as? [String: Any])?["code"] as? String, "invalid_params") + + let malformedCloseResponse = v2Response( + method: "page.close", + params: [ + "workspace_id": workspace.id.uuidString, + "page_id": pageId.uuidString, + "force": "tru" + ] + ) + XCTAssertEqual(malformedCloseResponse["ok"] as? Bool, false) + XCTAssertEqual((malformedCloseResponse["error"] as? [String: Any])?["code"] as? String, "invalid_params") + } + + func testV2PageMethodsRejectMalformedScopedIDsAndHonorWindowScope() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let firstWindowId = appDelegate.createMainWindow() + let secondWindowId = appDelegate.createMainWindow() + + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: firstWindowId) + closeWindow(withId: secondWindowId) + } + + guard let firstManager = appDelegate.tabManagerFor(windowId: firstWindowId), + let secondManager = appDelegate.tabManagerFor(windowId: secondWindowId), + let firstWorkspace = firstManager.selectedWorkspace, + let secondWorkspace = secondManager.selectedWorkspace, + let pageId = firstWorkspace.activePage?.id else { + XCTFail("Expected both window contexts to exist") + return + } + + firstWorkspace.setPageTitle(pageId: pageId, title: "Agents") + let secondWorkspaceInitialTitle = secondWorkspace.pages.first?.title + TerminalController.shared.setActiveTabManager(secondManager) + + let malformedWorkspaceResponse = v2Response( + method: "page.duplicate", + params: ["workspace_id": "not-a-uuid"] + ) + XCTAssertEqual(malformedWorkspaceResponse["ok"] as? Bool, false) + XCTAssertEqual((malformedWorkspaceResponse["error"] as? [String: Any])?["code"] as? String, "invalid_params") + + let malformedPageResponse = v2Response( + method: "page.duplicate", + params: ["page_id": "not-a-uuid"] + ) + XCTAssertEqual(malformedPageResponse["ok"] as? Bool, false) + XCTAssertEqual((malformedPageResponse["error"] as? [String: Any])?["code"] as? String, "invalid_params") + + let crossWindowRenameResponse = v2Response( + method: "page.rename", + params: [ + "window_id": secondWindowId.uuidString, + "page_id": pageId.uuidString, + "title": "Renamed From Wrong Window" + ] + ) + XCTAssertEqual(crossWindowRenameResponse["ok"] as? Bool, false) + XCTAssertEqual((crossWindowRenameResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual(firstWorkspace.pages.first?.title, "Agents") + XCTAssertEqual(secondWorkspace.pages.first?.title, secondWorkspaceInitialTitle) + } + + func testV2PageMethodsReportSpecificMissingScopeErrors() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: windowId) + } + + guard let manager = appDelegate.tabManagerFor(windowId: windowId), + let workspace = manager.selectedWorkspace, + let pageId = workspace.activePage?.id else { + XCTFail("Expected window and workspace") + return + } + + TerminalController.shared.setActiveTabManager(manager) + + let missingWindowResponse = v2Response( + method: "page.rename", + params: [ + "window_id": UUID().uuidString, + "page_id": pageId.uuidString, + "title": "Missing Window" + ] + ) + XCTAssertEqual(missingWindowResponse["ok"] as? Bool, false) + XCTAssertEqual((missingWindowResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual((missingWindowResponse["error"] as? [String: Any])?["message"] as? String, "Window not found") + + let missingSurfaceResponse = v2Response( + method: "page.rename", + params: [ + "surface_id": UUID().uuidString, + "page_id": pageId.uuidString, + "title": "Missing Surface" + ] + ) + XCTAssertEqual(missingSurfaceResponse["ok"] as? Bool, false) + XCTAssertEqual((missingSurfaceResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual((missingSurfaceResponse["error"] as? [String: Any])?["message"] as? String, "Surface not found") + + let missingTabResponse = v2Response( + method: "page.rename", + params: [ + "tab_id": UUID().uuidString, + "page_id": pageId.uuidString, + "title": "Missing Tab" + ] + ) + XCTAssertEqual(missingTabResponse["ok"] as? Bool, false) + XCTAssertEqual((missingTabResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual((missingTabResponse["error"] as? [String: Any])?["message"] as? String, "Tab not found") + + TerminalController.shared.setActiveTabManager(nil) + let noActiveWorkspaceResponse = v2Response(method: "page.duplicate", params: [:]) + XCTAssertEqual(noActiveWorkspaceResponse["ok"] as? Bool, false) + XCTAssertEqual((noActiveWorkspaceResponse["error"] as? [String: Any])?["code"] as? String, "not_found") + XCTAssertEqual((noActiveWorkspaceResponse["error"] as? [String: Any])?["message"] as? String, "Workspace not found") + } + + func testV2PageMethodsResolveWindowScopedPageAcrossWorkspaceTabs() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { + TerminalController.shared.setActiveTabManager(nil) + closeWindow(withId: windowId) + } + + guard let manager = appDelegate.tabManagerFor(windowId: windowId), + let firstWorkspace = manager.selectedWorkspace else { + XCTFail("Expected window and selected workspace") + return + } + + let secondWorkspace = manager.addWorkspace(select: true) + let pageId = secondWorkspace.activePageId + secondWorkspace.setPageTitle(pageId: pageId, title: "Window Scoped") + manager.selectWorkspace(firstWorkspace) + TerminalController.shared.setActiveTabManager(manager) + + let result = v2Result( + method: "page.rename", + params: [ + "window_id": windowId.uuidString, + "page_id": pageId.uuidString, + "title": "Renamed In Other Workspace" + ] + ) + + XCTAssertEqual(result["workspace_id"] as? String, secondWorkspace.id.uuidString) + XCTAssertEqual(secondWorkspace.pageTitle(pageId: pageId), "Renamed In Other Workspace") + XCTAssertEqual(manager.selectedWorkspace?.id, firstWorkspace.id) + } + + func testFilteredStoredPaneSurfacesDropsMissingPanelReferences() { + let firstPanelId = UUID() + let missingPanelId = UUID() + let selectedPanelId = UUID() + + let filtered = TerminalController.filteredStoredPaneSurfaces( + panelIds: [firstPanelId, missingPanelId, selectedPanelId], + selectedPanelId: selectedPanelId, + presentPanelIds: [firstPanelId, selectedPanelId] + ) + + XCTAssertEqual(filtered.panelIds, [firstPanelId, selectedPanelId]) + XCTAssertEqual(filtered.selectedPanelId, selectedPanelId) + + let selectedMissing = TerminalController.filteredStoredPaneSurfaces( + panelIds: [firstPanelId, missingPanelId], + selectedPanelId: missingPanelId, + presentPanelIds: [firstPanelId] + ) + + XCTAssertEqual(selectedMissing.panelIds, [firstPanelId]) + XCTAssertNil(selectedMissing.selectedPanelId) + } + func testCmdShiftNonDigitKeySymbolDoesNotMatchShiftedDigitShortcut() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") @@ -2742,7 +3139,7 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { #endif } - private func v2Result( + private func v2Response( method: String, params: [String: Any] = [:], file: StaticString = #filePath, @@ -2798,10 +3195,20 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { return [:] } + return response + } + + private func v2Result( + method: String, + params: [String: Any] = [:], + file: StaticString = #filePath, + line: UInt = #line + ) -> [String: Any] { + let response = v2Response(method: method, params: params, file: file, line: line) let isOK = (response["ok"] as? Bool) == true - XCTAssertTrue(isOK, "Expected successful v2 response: \(responseString)", file: file, line: line) + XCTAssertTrue(isOK, "Expected successful v2 response: \(response)", file: file, line: line) guard let result = response["result"] as? [String: Any] else { - XCTFail("Expected result payload in response: \(responseString)", file: file, line: line) + XCTFail("Expected result payload in response: \(response)", file: file, line: line) return [:] } return result diff --git a/cmuxTests/WorkspaceContentViewVisibilityTests.swift b/cmuxTests/WorkspaceContentViewVisibilityTests.swift index 14e136847876..a02bdbeb211c 100644 --- a/cmuxTests/WorkspaceContentViewVisibilityTests.swift +++ b/cmuxTests/WorkspaceContentViewVisibilityTests.swift @@ -128,4 +128,27 @@ final class WorkspacePageLifecycleTests: XCTestCase { "Returning to the second page should reuse its parked live panel instead of rebuilding a new one" ) } + + func testRuntimeRestoreRequiresEveryPanelReferencedByLayout() { + let firstPanelId = UUID() + let secondPanelId = UUID() + let thirdPanelId = UUID() + let layout = SessionWorkspaceLayoutSnapshot.split( + SessionSplitLayoutSnapshot( + orientation: .vertical, + dividerPosition: 0.5, + first: .pane(SessionPaneLayoutSnapshot(panelIds: [firstPanelId, secondPanelId], selectedPanelId: secondPanelId)), + second: .pane(SessionPaneLayoutSnapshot(panelIds: [thirdPanelId], selectedPanelId: thirdPanelId)) + ) + ) + + XCTAssertFalse( + Workspace.canRestoreRuntimeLayout(layout, detachedSurfaceIds: Set([firstPanelId, secondPanelId])), + "Runtime restore should fall back when any panel in the layout is missing from detached surfaces" + ) + XCTAssertTrue( + Workspace.canRestoreRuntimeLayout(layout, detachedSurfaceIds: Set([firstPanelId, secondPanelId, thirdPanelId])), + "Runtime restore should proceed only when every layout panel is available" + ) + } } diff --git a/docs/workspace-pages-spec.md b/docs/workspace-pages-spec.md index 709e2251c993..e07970b497fb 100644 --- a/docs/workspace-pages-spec.md +++ b/docs/workspace-pages-spec.md @@ -63,7 +63,7 @@ Implemented on this branch: 5. Page close-button visibility follows the active/hover rules in the titlebar strip. 6. Page context menus support create, duplicate, rename, close, close others, move left, and move right. 7. Page switching detaches inactive Ghostty and WKWebView-backed panels from the live hierarchy instead of killing PTYs or browser state. -8. Holding Option reveals direct-select page shortcut badges in the titlebar strip, using the existing shortcut-hint pattern. +8. Holding `Command` reveals page shortcut badges in the titlebar strip, using the existing shortcut-hint pattern and showing the current direct-select bindings. 9. Customizable page shortcuts exist in `KeyboardShortcutSettings`, and the default bindings are wired through app-level shortcut handling. 10. `Cmd+Shift+P` exposes page create, duplicate, rename, close, close others, next/previous, move left/right, and direct page selection commands. 11. The app menu exposes page create, duplicate, rename, close, close others, move left/right, next/previous, and direct page selection actions. @@ -79,7 +79,7 @@ Implemented on this branch: Not implemented yet: 1. The deeper model refactor where each page owns its own `bonsplitController` and live panel map directly. -2. CI execution and stabilization for the new page UI automation and external page API regressions still needs to be wired and kept green on this branch. +2. CI execution and stabilization for the new page UI automation and `tests_v2` external page API regressions still needs to be wired and kept green on this branch. ## Titlebar UX @@ -94,7 +94,7 @@ V1 strip rules: 5. A page `+` control sits at the far right of the fake titlebar lane, outside the scrollable page list. 6. Right click on a page opens its context menu. 7. Empty titlebar space remains draggable. -8. Holding Option should reveal the direct-select shortcut labels for visible pages, using the existing shortcut-hint pattern instead of adding permanent chrome. +8. Holding `Command` should reveal the shortcut labels for visible pages instead of adding permanent chrome. 9. The page `+` control is only visible while hovering the fake titlebar. The current titlebar folder icon goes away in V1. `Open Folder` remains available through existing menu, command palette, and shortcut paths. @@ -189,7 +189,7 @@ Tooltips and hints: 1. Hovering a page should show the full page title when truncated. 2. Hovering the `+` affordance should show `New Page` plus its effective shortcut. -3. Holding Option should show page-index shortcut hints in the strip, following the same “hold modifier to reveal hints” idea already used elsewhere in cmux. +3. Holding `Command` should show page-index shortcut hints in the strip, following the same “hold modifier to reveal hints” idea already used elsewhere in cmux. ## Page Behavior @@ -250,10 +250,10 @@ Default shortcuts: 1. `Command+Option+N`: new page. 2. `Command+Option+R`: rename page. 3. `Command+Option+W`: close page. -4. `Option+1` through `Option+8`: select page by index. -5. `Option+9`: select the last page. -6. `Option+]`: next page. -7. `Option+[`: previous page. +4. `Command+Option+1` through `Command+Option+8`: select page by index. +5. `Command+Option+9`: select the last page. +6. `Command+Option+]`: next page. +7. `Command+Option+[`: previous page. All page shortcuts must be first-class `KeyboardShortcutSettings` actions so they appear in Settings and can be customized. @@ -261,7 +261,7 @@ The same actions should also appear in the command palette and the app menu. Implementation note: -Direct page selection should route by physical digit intent, not by text produced after Option modifies the character, so `Option+digit` keeps working across keyboard layouts. +Direct page selection should route by physical digit intent, not by text produced after Option modifies the character, so `Command+Option+digit` keeps working across keyboard layouts. ## Cmd+Shift+P Commands @@ -292,11 +292,12 @@ Command-palette behavior: Right-clicking a page should expose: 1. `New Page` -2. `Rename Page…` -3. `Move Left` -4. `Move Right` -5. `Close Page` -6. `Close Other Pages` +2. `Duplicate Page` +3. `Rename Page…` +4. `Move Left` +5. `Move Right` +6. `Close Page` +7. `Close Other Pages` Current branch status: @@ -385,9 +386,10 @@ Implemented v2 API surface: Identity and targeting: -1. `system.identify` includes `focused.page_id`, `focused.page_ref`, `focused.page_index`, and `focused.page_title`. +1. `system.identify` includes `page_id`, `page_ref`, `page_index`, and `page_title` inside the `focused` payload. 2. Short refs support `page:`. 3. Commands that target panes or surfaces without an explicit page should resolve against the currently selected page in the targeted workspace. +4. Socket clients must pass `force=true` to `page.close` because the transport cannot show confirmation UI. The CLI `close-page` command supplies that automatically. Implemented CLI surface: @@ -416,7 +418,7 @@ The first implementation should feel complete if all of this is true: 1. A workspace can hold multiple pages with independent pane/tab layouts. 2. The titlebar strip replaces the folder icon area and is usable with mouse only. -3. `Option+1..9` works by default and is customizable in Settings. +3. `Command+Option+1..9` works by default and is customizable in Settings. 4. Right click works on page items without breaking window dragging or terminal focus. 5. Active-page close button visibility matches the rules above. 6. Inactive pages unmount from the live UI so only the active page's terminal and browser views stay mounted. @@ -429,7 +431,7 @@ The first implementation should feel complete if all of this is true: Current branch status: 1. The V1 acceptance list is implemented. -2. The remaining work is follow-on coverage and the deeper per-page controller refactor described above. +2. Dedicated UI automation and the `tests_v2` page parity regression exist, but CI stabilization still needs follow-up alongside the deeper per-page controller refactor described above. ## Test Expectations @@ -437,7 +439,7 @@ Once implementation starts, add coverage for: 1. titlebar hit testing, page item interaction, and empty-space drag behavior 2. page switching preserving per-page Bonsplit state -3. `Option+1..9` routing, including `9 -> last` +3. `Command+Option+1..9` routing, including `9 -> last` 4. custom shortcut overrides for page actions 5. `Cmd+Shift+P` page commands and rename flow 6. page context menu actions @@ -448,6 +450,6 @@ Once implementation starts, add coverage for: Current branch status: -1. Unit coverage now exists for page persistence round-trips and page shortcut routing, including `Option+9 -> last page`, `Option+]`, `Cmd+Option+N`, and symbol-first layout fallback for page shortcuts. +1. Unit coverage now exists for page persistence round-trips and page shortcut routing, including `Command+Option+9 -> last page`, `Command+Option+]`, `Command+Option+N`, and symbol-first layout fallback for page shortcuts. 2. Unit coverage also exists for duplicate-page structure preservation and active-page close-neighbor selection. -3. UI and end-to-end coverage for titlebar hit testing, drag behavior, and page lifecycle still needs to be added. +3. Dedicated UI automation and `tests_v2` parity coverage now exist for titlebar interaction and external page commands, but CI stabilization still needs follow-up. diff --git a/tests_v2/test_page_cli_socket_parity.py b/tests_v2/test_page_cli_socket_parity.py index 4d53bd3c0d5a..4f3fbde38a8e 100644 --- a/tests_v2/test_page_cli_socket_parity.py +++ b/tests_v2/test_page_cli_socket_parity.py @@ -13,7 +13,25 @@ from cmux import cmux, cmuxError -SOCKET_PATH = os.environ.get("CMUX_SOCKET", "/tmp/cmux-debug.sock") +def _resolve_socket_path() -> str | None: + explicit = os.environ.get("CMUX_SOCKET") + if explicit: + return explicit + + tag = os.environ.get("CMUX_TAG") + if tag: + return f"/tmp/cmux-debug-{tag}.sock" + + return None + + +SOCKET_PATH: str | None = _resolve_socket_path() + + +def _require_socket_path() -> str: + if SOCKET_PATH is None: + raise cmuxError("Set CMUX_SOCKET or CMUX_TAG before running tests_v2 page parity against a tagged cmux build") + return SOCKET_PATH def _must(cond: bool, msg: str) -> None: @@ -40,12 +58,13 @@ def _find_cli_binary() -> str: def _run_cli(cli: str, args: List[str], json_output: bool) -> str: + socket_path = _require_socket_path() env = dict(os.environ) env.pop("CMUX_WORKSPACE_ID", None) env.pop("CMUX_SURFACE_ID", None) env.pop("CMUX_TAB_ID", None) - cmd = [cli, "--socket", SOCKET_PATH] + cmd = [cli, "--socket", socket_path] if json_output: cmd.append("--json") cmd.extend(args) @@ -83,12 +102,13 @@ def _workspace_node(tree: Dict, workspace_id: str) -> Dict: def main() -> int: cli = _find_cli_binary() + socket_path = _require_socket_path() help_text = _run_cli(cli, ["list-pages", "--help"], json_output=False) _must("page:" in help_text, "list-pages --help should mention page: refs") _must("current-page" in help_text, "list-pages --help should mention related page commands") - with cmux(SOCKET_PATH) as c: + with cmux(socket_path) as c: created = c._call("workspace.create", {}) or {} workspace_id = str(created.get("workspace_id") or "") _must(bool(workspace_id), f"workspace.create returned no workspace_id: {created}") @@ -123,8 +143,8 @@ def main() -> int: listed = c._call("page.list", {"workspace_id": workspace_id}) or {} titles, selected_titles = _page_titles_and_selected(listed) _must(titles == ["agents", "editor"], f"page.list returned unexpected titles after create: {listed}") - _must(selected_titles == ["editor"], f"page.list should report editor selected after create: {listed}") - _must(str(listed.get("page_id") or "") == second_page_id, f"page.list should mirror active page: {listed}") + _must(selected_titles == ["agents"], f"page.list should keep the current page selected after create: {listed}") + _must(str(listed.get("page_id") or "") == first_page_id, f"page.list should mirror the unchanged active page: {listed}") selected = _run_cli_json( cli, @@ -164,8 +184,8 @@ def main() -> int: f"system.tree page order did not match reorder result: {workspace}", ) _must( - str(workspace.get("selected_page_id") or "") == duplicate_page_id, - f"system.tree selected page did not mirror active duplicated page: {workspace}", + str(workspace.get("selected_page_id") or "") == first_page_id, + f"system.tree should keep the previously selected page active after duplicate/reorder: {workspace}", ) last_page = c._call("page.last", {"workspace_id": workspace_id}) or {} @@ -176,6 +196,8 @@ def main() -> int: str(current_cli.get("page_id") or "") == second_page_id, f"current-page CLI should agree with page.last: {current_cli}", ) + current_cli_text = _run_cli(cli, ["current-page", "--workspace", workspace_id], json_output=False).strip() + _must(current_cli_text == second_page_ref, f"current-page text output should be the page ref: {current_cli_text!r}") closed = _run_cli_json( cli, @@ -183,15 +205,15 @@ def main() -> int: ) _must(str(closed.get("page_id") or "") == duplicate_page_id, f"close-page closed wrong page: {closed}") _must( - str(closed.get("selected_page_id") or "") == first_page_id, - f"close-page should select the nearest surviving neighbor after closing the leftmost active page: {closed}", + str(closed.get("selected_page_id") or "") == second_page_id, + f"close-page should preserve the selected page when closing an inactive page: {closed}", ) final_list = _run_cli_json(cli, ["list-pages", "--workspace", workspace_id]) final_titles, final_selected = _page_titles_and_selected(final_list) _must(final_titles == ["agents", "editor"], f"list-pages should reflect closed duplicate page: {final_list}") - _must(final_selected == ["agents"], f"list-pages should report agents selected after close: {final_list}") - _must(str(final_list.get("page_id") or "") == first_page_id, f"list-pages active page mismatch after close: {final_list}") + _must(final_selected == ["editor"], f"list-pages should keep editor selected after closing an inactive page: {final_list}") + _must(str(final_list.get("page_id") or "") == second_page_id, f"list-pages active page mismatch after close: {final_list}") _must( second_page_ref.startswith("page:"), f"new-page should return a page ref handle: {created_page}",