From 99fcd3c17f5e858cab0b4e36c8cd0066ceb7e6b9 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 16:32:22 -0700 Subject: [PATCH 01/22] Improve file explorer search navigation --- Sources/ContentView.swift | 41 +++++++++--- Sources/FileExplorerKeyboardShortcuts.swift | 13 +++- Sources/FileExplorerStore.swift | 9 ++- Sources/FileExplorerView.swift | 21 +++--- Sources/Panels/FilePreviewPanel.swift | 64 ++++++++++++++++++ Sources/RightSidebarPanelView.swift | 2 +- Sources/RightSidebarToolPanel.swift | 36 +++++++--- .../FileExplorerShortcutSettingsTests.swift | 2 +- cmuxTests/FileExplorerStoreTests.swift | 67 ++++++++++++++++--- .../FilePreviewTextEditorTextKitTests.swift | 30 +++++++++ cmuxTests/FileSearchRipgrepParserTests.swift | 2 +- ...ddenRightSidebarContentMountingTests.swift | 2 +- cmuxTests/WindowAndDragTests.swift | 2 +- 13 files changed, 241 insertions(+), 50 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index ac3cff00d88b..9b4f2e5a8df5 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -1921,8 +1921,8 @@ struct ContentView: View { onResumeSession: { entry in resumeSession(entry: entry) }, - onOpenFilePreview: { filePath in - openFilePreviewFromSidebar(filePath: filePath) + onOpenFilePreview: { filePath, lineNumber, columnNumber in + openFilePreviewFromSidebar(filePath: filePath, lineNumber: lineNumber, columnNumber: columnNumber) }, onOpenAsPane: { mode in openRightSidebarToolPane(mode) @@ -2302,7 +2302,7 @@ struct ContentView: View { _ = workspace.openOrFocusRightSidebarToolSurface(inPane: paneId, mode: mode, focus: true) } - private func openFilePreviewFromSidebar(filePath: String) { + private func openFilePreviewFromSidebar(filePath: String, lineNumber: Int?, columnNumber: Int?) { guard let workspace = tabManager.selectedWorkspace else { return } guard let paneId = workspace.bonsplitController.focusedPaneId ?? workspace.bonsplitController.allPaneIds.first else { return @@ -2314,24 +2314,45 @@ struct ContentView: View { guard let workspace else { return } do { let localURL = try await fileExplorerStore.materializeRemoteFileForPreview(path: filePath) - _ = workspace.openFileSurfaces( + let panels = workspace.openFilePreviewSurfaces( inPane: paneId, filePaths: [localURL.path], focus: true, reuseExisting: true ) + Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) } catch { NSSound.beep() } } return } - _ = workspace.openFileSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) + if lineNumber != nil { + let panels = workspace.openFilePreviewSurfaces( + inPane: paneId, + filePaths: [filePath], + focus: true, + reuseExisting: true + ) + Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) + } else { + _ = workspace.openFileSurfaces( + inPane: paneId, + filePaths: [filePath], + focus: true, + reuseExisting: true + ) + } + } + + @MainActor + private static func applyFilePreviewNavigation( + lineNumber: Int?, + columnNumber: Int?, + to panel: FilePreviewPanel? + ) { + guard let panel, let lineNumber else { return } + panel.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber ?? 1) } private func syncFileExplorerDirectory() { diff --git a/Sources/FileExplorerKeyboardShortcuts.swift b/Sources/FileExplorerKeyboardShortcuts.swift index 628ee0b53062..1e936cad031b 100644 --- a/Sources/FileExplorerKeyboardShortcuts.swift +++ b/Sources/FileExplorerKeyboardShortcuts.swift @@ -2,9 +2,16 @@ import AppKit import CmuxSettings import CmuxWorkspaces +typealias FileExplorerPreviewOpenHandler = (_ path: String, _ lineNumber: Int?, _ columnNumber: Int?) -> Void + /// Perform the configured action for opening a local file from the file explorer. @MainActor -func performFileExplorerFileOpen(path: String, onOpenFilePreview: (String) -> Void) { +func performFileExplorerFileOpen( + path: String, + lineNumber: Int? = nil, + columnNumber: Int? = nil, + onOpenFilePreview: FileExplorerPreviewOpenHandler +) { let action = FileExplorerDoubleClickActionSettings.resolvedAction() let hasPreferredEditor = PreferredEditorSettingsStore(defaults: .standard).resolvedCommand != nil switch FileExplorerDoubleClickActionSettings.fileActivation( @@ -12,7 +19,7 @@ func performFileExplorerFileOpen(path: String, onOpenFilePreview: (String) -> Vo hasPreferredEditorCommand: hasPreferredEditor ) { case .preview: - onOpenFilePreview(path) + onOpenFilePreview(path, lineNumber, columnNumber) case .defaultEditor: FileExternalOpenAction.openDefault(fileURL: URL(fileURLWithPath: path)) case .preferredEditor: @@ -41,7 +48,7 @@ extension FileExplorerPanelView.Coordinator { } guard store.provider is LocalFileExplorerProvider else { - onOpenFilePreview(node.path) + onOpenFilePreview(node.path, nil, nil) return } performFileExplorerFileOpen(path: node.path, onOpenFilePreview: onOpenFilePreview) diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 2422da8d8fe8..8956a4bd72a4 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -752,8 +752,8 @@ final class FileExplorerStore: ObservableObject { /// Whether hidden files are shown. Set from FileExplorerState externally. var showHiddenFiles: Bool = false - /// Watches the root directory for filesystem changes (local only). - private var directoryWatcher: FileWatcher? + /// Watches the root directory recursively for filesystem changes (local only). + private var directoryWatcher: RecursivePathWatcher? private var directoryWatchTask: Task? private var directoryWatchPath: String? @@ -893,8 +893,7 @@ final class FileExplorerStore: ObservableObject { if provider is LocalFileExplorerProvider, !rootPath.isEmpty { guard directoryWatchPath != rootPath || directoryWatcher == nil else { return } stopDirectoryWatcher() - // Preserve the previous 0.3s coalescing as a leading-edge throttle. - let watcher = FileWatcher(path: rootPath, throttle: .milliseconds(300)) + guard let watcher = RecursivePathWatcher(paths: [rootPath]) else { return } directoryWatcher = watcher directoryWatchPath = rootPath let events = watcher.events @@ -911,7 +910,7 @@ final class FileExplorerStore: ObservableObject { } /// Cancels the directory-watch consumer and drops the watcher; the watcher's - /// deinit cancels its `DispatchSource`s synchronously. + /// deinit tears down its filesystem stream synchronously. private func stopDirectoryWatcher() { directoryWatchTask?.cancel() directoryWatchTask = nil diff --git a/Sources/FileExplorerView.swift b/Sources/FileExplorerView.swift index 0c7c017ae904..cfa7e08e0b55 100644 --- a/Sources/FileExplorerView.swift +++ b/Sources/FileExplorerView.swift @@ -37,7 +37,7 @@ enum FileExplorerPanelPlacement: Equatable { struct FileExplorerPanelView: NSViewRepresentable { @ObservedObject var store: FileExplorerStore @ObservedObject var state: FileExplorerState - let onOpenFilePreview: (String) -> Void + let onOpenFilePreview: FileExplorerPreviewOpenHandler var presentation: FileExplorerPanelPresentation = .files var placement: FileExplorerPanelPlacement = .rightSidebar var onFocus: (() -> Void)? @@ -86,7 +86,7 @@ struct FileExplorerPanelView: NSViewRepresentable { final class Coordinator: NSObject, NSOutlineViewDataSource, NSOutlineViewDelegate, NSMenuDelegate { var store: FileExplorerStore var state: FileExplorerState - var onOpenFilePreview: (String) -> Void + var onOpenFilePreview: FileExplorerPreviewOpenHandler var placement: FileExplorerPanelPlacement var onFocus: (() -> Void)? var onContainerChange: ((FileExplorerContainerView?) -> Void)? @@ -100,7 +100,7 @@ struct FileExplorerPanelView: NSViewRepresentable { init( store: FileExplorerStore, state: FileExplorerState, - onOpenFilePreview: @escaping (String) -> Void, + onOpenFilePreview: @escaping FileExplorerPreviewOpenHandler, placement: FileExplorerPanelPlacement = .rightSidebar, onFocus: (() -> Void)? = nil, onContainerChange: ((FileExplorerContainerView?) -> Void)? = nil @@ -1461,14 +1461,19 @@ final class FileExplorerContainerView: NSView { fileprivate func openSelectedSearchResult() { let row = searchResultsView.selectedRow guard row >= 0, row < searchSnapshot.results.count else { return } - let path = searchSnapshot.results[row].path + let result = searchSnapshot.results[row] // Editor/preferred-editor actions operate on local file paths via // NSWorkspace; for non-local providers fall back to the cmux preview. guard coordinator.store.provider is LocalFileExplorerProvider else { - coordinator.onOpenFilePreview(path) + coordinator.onOpenFilePreview(result.path, result.lineNumber, result.columnNumber) return } - performFileExplorerFileOpen(path: path, onOpenFilePreview: coordinator.onOpenFilePreview) + performFileExplorerFileOpen( + path: result.path, + lineNumber: result.lineNumber, + columnNumber: result.columnNumber, + onOpenFilePreview: coordinator.onOpenFilePreview + ) } @objc private func openSelectedSearchResultFromTable(_ sender: NSTableView) { @@ -1477,7 +1482,7 @@ final class FileExplorerContainerView: NSView { @objc private func contextMenuOpenSearchResultInCmux(_ sender: NSMenuItem) { guard let result = searchResult(forMenuItem: sender) else { return } - coordinator.onOpenFilePreview(result.path) + coordinator.onOpenFilePreview(result.path, result.lineNumber, result.columnNumber) } @objc private func contextMenuOpenSearchResultExternally(_ sender: NSMenuItem) { @@ -1653,4 +1658,4 @@ extension FileExplorerContainerView: NSSearchFieldDelegate, NSTableViewDataSourc copyRelativePathItem.representedObject = NSNumber(value: row) menu.addItem(copyRelativePathItem) } -} \ No newline at end of file +} diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index 3bc754a2cd90..23c710fb2d8e 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1000,6 +1000,8 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan private var saveGeneration = 0 private var activeSaveGeneration: Int? private weak var textView: NSTextView? + private var hasLoadedTextContent = false + private var pendingTextNavigation: (lineNumber: Int, columnNumber: Int)? private let focusCoordinator: FilePreviewFocusCoordinator private let textLoader: @Sendable (URL) async -> FilePreviewTextLoader.Result @@ -1054,6 +1056,7 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan func attachTextView(_ textView: NSTextView) { self.textView = textView focusCoordinator.register(root: textView, primaryResponder: textView, intent: .textEditor) + applyPendingTextNavigationIfReady() } func handleDroppedFileURLsAsText(_ urls: [URL]) -> Bool { @@ -1070,6 +1073,16 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan focusCoordinator.fulfillPendingFocusIfNeeded() } + func navigateToTextPosition(lineNumber: Int, columnNumber: Int) { + pendingTextNavigation = ( + lineNumber: max(1, lineNumber), + columnNumber: max(1, columnNumber) + ) + focusCoordinator.notePreferredIntent(.textEditor) + _ = restoreFocusIntent(.filePreview(.textEditor)) + applyPendingTextNavigationIfReady() + } + func attachPDFPreview(root: NSView, primaryResponder: NSView) { attachPreviewFocus(root: root, primaryResponder: primaryResponder, intent: .pdfCanvas) } @@ -1187,6 +1200,7 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan guard previewMode == .text else { return Task {} } + hasLoadedTextContent = false textLoadGeneration += 1 let generation = textLoadGeneration let fileURL = fileURL @@ -1208,6 +1222,7 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan switch result { case .unavailable: guard replacingDirtyContent || !isDirty else { + hasLoadedTextContent = true isFileUnavailable = true return } @@ -1215,12 +1230,16 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan originalTextContent = "" isDirty = false isFileUnavailable = true + hasLoadedTextContent = true + pendingTextNavigation = nil return case .loaded(let content, let encoding): if !replacingDirtyContent && isDirty { originalTextContent = content textEncoding = encoding isFileUnavailable = false + hasLoadedTextContent = true + applyPendingTextNavigationIfReady() return } textContent = content @@ -1228,6 +1247,8 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan textEncoding = encoding isDirty = false isFileUnavailable = false + hasLoadedTextContent = true + applyPendingTextNavigationIfReady() } } @@ -1280,6 +1301,49 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan return .quickLook } } + + private func applyPendingTextNavigationIfReady() { + guard previewMode == .text, + hasLoadedTextContent, + let textView, + let pendingTextNavigation else { + return + } + + if textView.string != textContent { + textView.string = textContent + } + let location = Self.textLocation( + lineNumber: pendingTextNavigation.lineNumber, + columnNumber: pendingTextNavigation.columnNumber, + in: textView.string + ) + let textLength = (textView.string as NSString).length + let range = NSRange(location: min(location, textLength), length: 0) + textView.setSelectedRange(range) + textView.scrollRangeToVisible(range) + _ = textView.window?.makeFirstResponder(textView) + self.pendingTextNavigation = nil + } + + private static func textLocation(lineNumber: Int, columnNumber: Int, in text: String) -> Int { + var index = text.startIndex + var currentLine = 1 + while currentLine < lineNumber, index < text.endIndex { + let character = text[index] + index = text.index(after: index) + if character.isNewline { + currentLine += 1 + } + } + + var currentColumn = 1 + while currentColumn < columnNumber, index < text.endIndex, !text[index].isNewline { + index = text.index(after: index) + currentColumn += 1 + } + return index.utf16Offset(in: text) + } } struct FilePreviewPanelView: View { diff --git a/Sources/RightSidebarPanelView.swift b/Sources/RightSidebarPanelView.swift index b934346a4051..c983e64df6bb 100644 --- a/Sources/RightSidebarPanelView.swift +++ b/Sources/RightSidebarPanelView.swift @@ -123,7 +123,7 @@ struct RightSidebarPanelView: View { let titlebarHeight: CGFloat let workspaceId: UUID? let onResumeSession: ((SessionEntry) -> Void)? - let onOpenFilePreview: (String) -> Void + let onOpenFilePreview: FileExplorerPreviewOpenHandler let onOpenAsPane: (RightSidebarMode) -> Void let onClose: () -> Void diff --git a/Sources/RightSidebarToolPanel.swift b/Sources/RightSidebarToolPanel.swift index 0089a1ec35d6..bd7e296a04d3 100644 --- a/Sources/RightSidebarToolPanel.swift +++ b/Sources/RightSidebarToolPanel.swift @@ -86,7 +86,7 @@ final class RightSidebarToolPanel: Panel, ObservableObject { } } - func openFilePreview(_ filePath: String) { + func openFilePreview(_ filePath: String, lineNumber: Int?, columnNumber: Int?) { guard let workspace, let paneId = workspace.bonsplitController.focusedPaneId ?? workspace.bonsplitController.allPaneIds.first else { return @@ -97,24 +97,44 @@ final class RightSidebarToolPanel: Panel, ObservableObject { guard let workspace, let store else { return } do { let localURL = try await store.materializeRemoteFileForPreview(path: filePath) - _ = workspace.openFileSurfaces( + let panels = workspace.openFilePreviewSurfaces( inPane: paneId, filePaths: [localURL.path], focus: true, reuseExisting: true ) + Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) } catch { NSSound.beep() } } return } - _ = workspace.openFileSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) + if lineNumber != nil { + let panels = workspace.openFilePreviewSurfaces( + inPane: paneId, + filePaths: [filePath], + focus: true, + reuseExisting: true + ) + Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) + } else { + _ = workspace.openFileSurfaces( + inPane: paneId, + filePaths: [filePath], + focus: true, + reuseExisting: true + ) + } + } + + private static func applyFilePreviewNavigation( + lineNumber: Int?, + columnNumber: Int?, + to panel: FilePreviewPanel? + ) { + guard let panel, let lineNumber else { return } + panel.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber ?? 1) } var isFocusedInWorkspace: Bool { diff --git a/cmuxTests/FileExplorerShortcutSettingsTests.swift b/cmuxTests/FileExplorerShortcutSettingsTests.swift index ea57eb91fa8a..b7cf477f13b0 100644 --- a/cmuxTests/FileExplorerShortcutSettingsTests.swift +++ b/cmuxTests/FileExplorerShortcutSettingsTests.swift @@ -365,7 +365,7 @@ private final class ShortcutNoopFileSearchController: FileSearchControlling { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _ in } + onOpenFilePreview: { _, _, _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index cfba78a3e5f1..9da6d80fd4cd 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -186,6 +186,43 @@ struct FileExplorerStoreTests { #expect(!(store.rootNodes[1].isDirectory)) } + @Test + func testLocalFileTreeRefreshesExpandedFolderAfterNestedFileCreate() async throws { + let rootURL = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-file-explorer-refresh-\(UUID().uuidString)", isDirectory: true) + let sourceURL = rootURL.appendingPathComponent("Sources", isDirectory: true) + try FileManager.default.createDirectory(at: sourceURL, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: rootURL) } + try "initial\n".write( + to: sourceURL.appendingPathComponent("Initial.swift"), + atomically: true, + encoding: .utf8 + ) + + let store = FileExplorerStore() + store.applyWorkspaceRoot(.local(workspaceId: UUID(), path: rootURL.path)) + try await waitFor("root directory loaded") { + store.rootNodes.map(\.name) == ["Sources"] + } + + let sourceNode = try #require(store.rootNodes.first { $0.name == "Sources" }) + store.expand(node: sourceNode) + try await waitFor("expanded source directory loaded") { + store.rootNodes.first { $0.name == "Sources" }?.children?.map(\.name) == ["Initial.swift"] + } + + try "created\n".write( + to: sourceURL.appendingPathComponent("Created.swift"), + atomically: true, + encoding: .utf8 + ) + + try await waitFor("nested file create refreshed expanded directory") { + let childNames = store.rootNodes.first { $0.name == "Sources" }?.children?.map(\.name) ?? [] + return childNames == ["Created.swift", "Initial.swift"] + } + } + @Test func testDisplayRootPathUsesTilde() { let provider = MockFileExplorerProvider(homePath: "/home/user") @@ -800,7 +837,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _ in } + onOpenFilePreview: { _, _, _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, @@ -839,12 +876,12 @@ struct FileSearchControllerTests { let store = FileExplorerStore() let state = FileExplorerState() let searchController = SpyFileSearchController() - var openedPaths: [String] = [] + var openedRequests: [(path: String, lineNumber: Int?, columnNumber: Int?)] = [] let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { path in - openedPaths.append(path) + onOpenFilePreview: { path, lineNumber, columnNumber in + openedRequests.append((path, lineNumber, columnNumber)) } ) let container = FileExplorerContainerView( @@ -861,7 +898,7 @@ struct FileSearchControllerTests { KeyboardShortcutSettings.setShortcut(.unbound, for: .fileExplorerOpenSelectionFinderAlias) let searchField = try #require(Self.findSearchField(in: container)) - let result = Self.searchResult(relativePath: "selected.txt") + let result = Self.searchResult(relativePath: "selected.txt", lineNumber: 42, columnNumber: 14) searchController.publish(FileSearchSnapshot( query: "needle", results: [result], @@ -876,7 +913,11 @@ struct FileSearchControllerTests { ) #expect(handled) - #expect(openedPaths == [result.path]) + let openedRequest = try #require(openedRequests.first) + #expect(openedRequests.count == 1) + #expect(openedRequest.path == result.path) + #expect(openedRequest.lineNumber == 42) + #expect(openedRequest.columnNumber == 14) } } @@ -888,7 +929,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _ in } + onOpenFilePreview: { _, _, _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, @@ -945,7 +986,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _ in } + onOpenFilePreview: { _, _, _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, @@ -1099,12 +1140,16 @@ struct FileSearchControllerTests { #expect(!(message.contains("%@"))) } - private static func searchResult(relativePath: String) -> FileSearchResult { + private static func searchResult( + relativePath: String, + lineNumber: Int = 1, + columnNumber: Int = 1 + ) -> FileSearchResult { FileSearchResult( path: "/tmp/cmux-find-content-revision-test/\(relativePath)", relativePath: relativePath, - lineNumber: 1, - columnNumber: 1, + lineNumber: lineNumber, + columnNumber: columnNumber, preview: "needle" ) } diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index 014425886b34..ede8746e4049 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -45,4 +45,34 @@ struct FilePreviewTextEditorTextKitTests { #expect(textView.layoutManager != nil) #expect(textView.layoutManager?.allowsNonContiguousLayout == true) } + + @Test("search navigation selects the requested loaded line and column") + func searchNavigationSelectsRequestedLoadedLineAndColumn() async throws { + let content = """ + first + second + abcd needle + """ + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-file-preview-navigation-\(UUID().uuidString)") + .appendingPathExtension("swift") + try content.write(to: url, atomically: true, encoding: .utf8) + defer { try? FileManager.default.removeItem(at: url) } + + let panel = FilePreviewPanel( + workspaceId: UUID(), + filePath: url.path, + textLoader: { _ in .loaded(content, .utf8) } + ) + defer { panel.close() } + + panel.navigateToTextPosition(lineNumber: 3, columnNumber: 6) + await panel.loadTextContent().value + + let textView = SavingTextView.makeFilePreviewTextView() + textView.string = panel.textContent + panel.attachTextView(textView) + + #expect(textView.selectedRange().location == (content as NSString).range(of: "needle").location) + } } diff --git a/cmuxTests/FileSearchRipgrepParserTests.swift b/cmuxTests/FileSearchRipgrepParserTests.swift index ed136ad32f33..5ef4840b136a 100644 --- a/cmuxTests/FileSearchRipgrepParserTests.swift +++ b/cmuxTests/FileSearchRipgrepParserTests.swift @@ -196,7 +196,7 @@ struct FileSearchWorkspaceScopeTests { let store = FileExplorerStore() let state = FileExplorerState() let searchController = SpyFileSearchController() - let coordinator = FileExplorerPanelView.Coordinator(store: store, state: state, onOpenFilePreview: { _ in }) + let coordinator = FileExplorerPanelView.Coordinator(store: store, state: state, onOpenFilePreview: { _, _, _ in }) let container = FileExplorerContainerView(coordinator: coordinator, presentation: .find, searchController: searchController) let rootPath = "/tmp/cmux-find-same-directory-test" diff --git a/cmuxTests/HiddenRightSidebarContentMountingTests.swift b/cmuxTests/HiddenRightSidebarContentMountingTests.swift index b75ba9ea12ef..c2c6d95dd48c 100644 --- a/cmuxTests/HiddenRightSidebarContentMountingTests.swift +++ b/cmuxTests/HiddenRightSidebarContentMountingTests.swift @@ -69,7 +69,7 @@ struct HiddenRightSidebarContentMountingTests { titlebarHeight: 36, workspaceId: nil, onResumeSession: nil, - onOpenFilePreview: { _ in }, + onOpenFilePreview: { _, _, _ in }, onOpenAsPane: { _ in }, onClose: {} ) diff --git a/cmuxTests/WindowAndDragTests.swift b/cmuxTests/WindowAndDragTests.swift index 79144b18322e..399fbe02efa2 100644 --- a/cmuxTests/WindowAndDragTests.swift +++ b/cmuxTests/WindowAndDragTests.swift @@ -1838,7 +1838,7 @@ final class WindowDragHandleHitTests: XCTestCase { titlebarHeight: 36, workspaceId: nil, onResumeSession: nil, - onOpenFilePreview: { _ in }, + onOpenFilePreview: { _, _, _ in }, onOpenAsPane: { _ in }, onClose: {} ) From f980df9ec1fd918f1a5250eb0a277a82ace87ed2 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 16:35:27 -0700 Subject: [PATCH 02/22] Refresh Swift file length budget --- .github/swift-file-length-budget.tsv | 42 +++++++++++++--------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 7b3ae3f3ab43..416167d54b79 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -2,37 +2,37 @@ # Format: max_linesrelative path # Reduce counts as files shrink. CI fails if tracked files exceed this budget. 34629 CLI/cmux.swift -17827 Sources/AppDelegate.swift -16128 Sources/ContentView.swift +17826 Sources/AppDelegate.swift +16130 Sources/ContentView.swift 13832 Sources/TerminalController.swift -12878 Sources/Workspace.swift +12838 Sources/Workspace.swift 12237 Sources/GhosttyTerminalView.swift 12144 cmuxTests/AppDelegateShortcutRoutingTests.swift -11578 Sources/Panels/BrowserPanel.swift +11567 Sources/Panels/BrowserPanel.swift 9497 cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift 8016 CLI/cmux_open.swift -7986 Sources/Panels/BrowserPanelView.swift +7952 Sources/Panels/BrowserPanelView.swift 7366 cmuxTests/WorkspaceUnitTests.swift 7218 cmuxTests/WorkspaceRemoteConnectionTests.swift -6916 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift +6876 Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift 6317 cmuxTests/SessionPersistenceTests.swift 6217 cmuxTests/GhosttyConfigTests.swift 6183 Sources/TabManager.swift -6084 Sources/TextBoxInput.swift +6077 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -4487 Sources/Panels/FilePreviewPanel.swift +4551 Sources/Panels/FilePreviewPanel.swift 4478 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4187 Sources/BrowserWindowPortal.swift 3934 Sources/Feed/FeedPanelView.swift 3926 cmuxTests/TabManagerUnitTests.swift 3896 cmuxTests/WindowAndDragTests.swift -3745 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift 3699 cmuxTests/CLIGenericHookPersistenceTests.swift +3688 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift 3397 Sources/CmuxConfig.swift 3364 cmuxTests/TabManagerSessionSnapshotTests.swift -3058 Sources/Update/UpdateTitlebarAccessory.swift +3053 Sources/Update/UpdateTitlebarAccessory.swift 2876 cmuxTests/CMUXOpenCommandTests.swift 2875 Sources/SessionIndexView.swift 2606 Sources/KeyboardShortcutSettings.swift @@ -47,23 +47,23 @@ 2091 cmuxTests/ShortcutAndCommandPaletteTests.swift 2078 Sources/SessionPersistence.swift 1952 Sources/KeyboardShortcutSettingsFileStore.swift -1949 Sources/Panels/BrowserWebAuthnSupport.swift 1945 Sources/RestorableAgentSession.swift 1900 cmuxTests/NotificationAndMenuBarTests.swift +1866 Sources/Panels/BrowserWebAuthnSupport.swift 1810 Sources/SessionIndexStore.swift 1748 Sources/WindowDragHandleView.swift 1732 cmuxTests/WorkspacePullRequestSidebarTests.swift 1687 cmuxTests/MarkdownPanelTests.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift -1656 Sources/FileExplorerView.swift +1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1452 Sources/RemoteTmuxControlConnection.swift -1447 Sources/FileExplorerStore.swift +1446 Sources/FileExplorerStore.swift +1433 Sources/RemoteTmuxControlConnection.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift 1373 cmuxTests/AppDelegateIssue2907RoutingTests.swift @@ -74,11 +74,11 @@ 1285 cmuxUITests/SidebarHelpMenuUITests.swift 1277 cmuxTests/RestorableAgentSessionIndexTests.swift 1258 Sources/Feed/FeedCoordinator.swift +1249 cmuxTests/FileExplorerStoreTests.swift 1209 Packages/macOS/CmuxCommandPalette/Tests/CmuxCommandPaletteTests/CommandPaletteSearchEngineTests.swift -1204 cmuxTests/FileExplorerStoreTests.swift 1197 cmuxTests/CodexAppServerSessionTests.swift -1169 Sources/RemoteTmuxController.swift 1166 Sources/VaultAgentProcessScanner.swift +1164 Sources/RemoteTmuxController.swift 1161 cmuxTests/SidebarOrderingTests.swift 1144 cmuxTests/PiVaultAgentPersistenceTests.swift 1121 cmuxTests/AgentHibernationTests.swift @@ -101,16 +101,16 @@ 905 Sources/CmuxSSHURLRequest.swift 899 Sources/Panels/MarkdownWebRenderer.swift 885 Sources/Panels/TerminalPanel.swift -878 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift 877 Packages/Shared/CmuxAgentChat/Tests/CmuxAgentChatTests/ChatConversationStoreTests.swift +877 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift 871 cmuxTests/ClaudeHookSurfaceResolutionSwiftTests.swift 868 Sources/Panels/BrowserScreenshotSnapshotter.swift 859 Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Workspace/ControlCommandCoordinator+Workspace.swift 847 cmuxTests/AgentSessionAutoResumeSettingsTests.swift 845 cmuxTests/SSHStartupSignalLifecycleTests.swift 830 Sources/TaskManagerTypes.swift -825 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift 824 Sources/MainWindowFocusController.swift +813 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift 810 Packages/macOS/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift 803 Packages/iOS/CmuxMobilePairedMac/Sources/CmuxMobilePairedMac/MobilePairedMacStore.swift 802 Sources/WorkspaceContentView.swift @@ -127,7 +127,7 @@ 752 cmuxUITests/CloseWorkspaceCmdDUITests.swift 739 cmuxTests/CLICodexHookTimeoutRegressionTests.swift 738 Packages/macOS/CMUXProjectModel/Sources/CMUXProjectModel/XcodeProjectAdapter.swift -725 Sources/Panels/BrowserPopupWindowController.swift +724 Sources/Panels/BrowserPopupWindowController.swift 722 Packages/Shared/CmuxAgentChat/Sources/CmuxAgentChat/Store/ChatConversationStore.swift 718 Packages/Shared/CmuxAuthRuntime/Sources/CmuxAuthRuntime/Coordinator/AuthCoordinator.swift 716 Sources/TaskManagerSnapshot.swift @@ -185,7 +185,6 @@ 572 Sources/Feed/FeedTextEditorDebugWindowController.swift 568 Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/MobileTerminalRenderGrid.swift 567 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/ConfigDiscovery/GhosttyConfigDiscovery.swift -562 Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swift 562 cmuxTests/AgentExecutableResolverTests.swift 561 cmuxTests/GhosttyConfigPathResolverTests.swift 560 cmuxTests/CLISSHPTYResizeInputTests.swift @@ -222,11 +221,10 @@ 509 Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizerAdditionalPolicies.swift 507 Sources/TerminalControllerTopSupport.swift 506 Sources/App/MainWindowVisibilityController.swift -505 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift 505 cmuxUITests/DisplayResolutionRegressionUITests.swift 504 cmuxTests/TerminalNotificationSocketActionTests.swift 503 Sources/Settings/ConfigSource.swift 502 Sources/CmuxEventPublishing.swift 502 Sources/RemoteTmuxSessionMirror.swift -501 Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift +500 Packages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs.swift 500 Sources/KeyboardShortcutRecorder.swift From 6324690982cac1acc27078dd47f4ff878cb08056 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 17:01:27 -0700 Subject: [PATCH 03/22] Address file preview navigation review feedback --- .github/swift-file-length-budget.tsv | 4 +- Sources/ContentView.swift | 14 +--- Sources/Panels/FilePreviewPanel.swift | 26 +++++-- Sources/RightSidebarToolPanel.swift | 13 +--- .../FilePreviewTextEditorTextKitTests.swift | 70 +++++++++++++++++++ 5 files changed, 95 insertions(+), 32 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index ed22d8e6475e..efb329c31f1a 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -3,7 +3,7 @@ # Reduce counts as files shrink. CI fails if tracked files exceed this budget. 34629 CLI/cmux.swift 17854 Sources/AppDelegate.swift -16130 Sources/ContentView.swift +16120 Sources/ContentView.swift 13913 Sources/TerminalController.swift 12838 Sources/Workspace.swift 12348 cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -21,7 +21,7 @@ 6077 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -4551 Sources/Panels/FilePreviewPanel.swift +4563 Sources/Panels/FilePreviewPanel.swift 4478 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4187 Sources/BrowserWindowPortal.swift diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 9b4f2e5a8df5..56725e758b1c 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -2320,7 +2320,7 @@ struct ContentView: View { focus: true, reuseExisting: true ) - Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) + panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) } catch { NSSound.beep() } @@ -2334,7 +2334,7 @@ struct ContentView: View { focus: true, reuseExisting: true ) - Self.applyFilePreviewNavigation(lineNumber: lineNumber, columnNumber: columnNumber, to: panels.first) + panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) } else { _ = workspace.openFileSurfaces( inPane: paneId, @@ -2345,16 +2345,6 @@ struct ContentView: View { } } - @MainActor - private static func applyFilePreviewNavigation( - lineNumber: Int?, - columnNumber: Int?, - to panel: FilePreviewPanel? - ) { - guard let panel, let lineNumber else { return } - panel.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber ?? 1) - } - private func syncFileExplorerDirectory() { guard let selectedId = tabManager.selectedTabId, let tab = tabManager.tabs.first(where: { $0.id == selectedId }) else { diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index 23c710fb2d8e..bb2a88b7f4a4 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1073,10 +1073,11 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan focusCoordinator.fulfillPendingFocusIfNeeded() } - func navigateToTextPosition(lineNumber: Int, columnNumber: Int) { + func navigateToTextPosition(lineNumber: Int?, columnNumber: Int?) { + guard let lineNumber else { return } pendingTextNavigation = ( lineNumber: max(1, lineNumber), - columnNumber: max(1, columnNumber) + columnNumber: max(1, columnNumber ?? 1) ) focusCoordinator.notePreferredIntent(.textEditor) _ = restoreFocusIntent(.filePreview(.textEditor)) @@ -1224,6 +1225,7 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan guard replacingDirtyContent || !isDirty else { hasLoadedTextContent = true isFileUnavailable = true + pendingTextNavigation = nil return } textContent = "" @@ -1337,12 +1339,22 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan } } - var currentColumn = 1 - while currentColumn < columnNumber, index < text.endIndex, !text[index].isNewline { - index = text.index(after: index) - currentColumn += 1 + let utf8 = text.utf8 + guard var utf8Index = index.samePosition(in: utf8) else { + return index.utf16Offset(in: text) + } + + for _ in 1.. FilePreviewTextLoader.Result { + result + } } From a5a0b8585ee3ef1857be77481eaced6cbd59e072 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 17:15:16 -0700 Subject: [PATCH 04/22] Clear stale navigation for dirty preview buffers --- .github/swift-file-length-budget.tsv | 2 +- Sources/Panels/FilePreviewPanel.swift | 7 +++- .../FilePreviewTextEditorTextKitTests.swift | 32 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index efb329c31f1a..a764d39350d4 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -21,7 +21,7 @@ 6077 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -4563 Sources/Panels/FilePreviewPanel.swift +4568 Sources/Panels/FilePreviewPanel.swift 4478 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4187 Sources/BrowserWindowPortal.swift diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index bb2a88b7f4a4..93040a02578d 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1237,11 +1237,16 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan return case .loaded(let content, let encoding): if !replacingDirtyContent && isDirty { + let canApplyPendingNavigation = textContent == content originalTextContent = content textEncoding = encoding isFileUnavailable = false hasLoadedTextContent = true - applyPendingTextNavigationIfReady() + if canApplyPendingNavigation { + applyPendingTextNavigationIfReady() + } else { + pendingTextNavigation = nil + } return } textContent = content diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index 4b5ec7ea12de..6daafd8e34d7 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -133,6 +133,38 @@ struct FilePreviewTextEditorTextKitTests { #expect(textView.selectedRange().location == 0) } + + @Test("loaded dirty buffer clears pending search navigation when content differs") + func loadedDirtyBufferClearsPendingSearchNavigationWhenContentDiffers() async throws { + let onDiskContent = "first\nneedle\n" + let loader = PreviewTextLoaderStub(result: .loaded(onDiskContent, .utf8)) + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-file-preview-dirty-loaded-navigation-\(UUID().uuidString)") + .appendingPathExtension("swift") + try onDiskContent.write(to: url, atomically: true, encoding: .utf8) + defer { try? FileManager.default.removeItem(at: url) } + + let panel = FilePreviewPanel( + workspaceId: UUID(), + filePath: url.path, + textLoader: loader.load + ) + defer { panel.close() } + + await panel.loadTextContent().value + panel.updateTextContent("dirty\nfirst\nneedle\n") + #expect(panel.isDirty) + + panel.navigateToTextPosition(lineNumber: 2, columnNumber: 1) + await panel.loadTextContent(replacingDirtyContent: false).value + + let textView = SavingTextView.makeFilePreviewTextView() + textView.string = panel.textContent + textView.setSelectedRange(NSRange(location: 0, length: 0)) + panel.attachTextView(textView) + + #expect(textView.selectedRange().location == 0) + } } private final class PreviewTextLoaderStub: @unchecked Sendable { From 4aa8871a83446a1bd3f7f0ef9735d8db6e964a17 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 21:41:26 -0700 Subject: [PATCH 05/22] Fix file preview navigation test enum calls --- cmuxTests/FilePreviewTextEditorTextKitTests.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index 6daafd8e34d7..a5563b468787 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -62,7 +62,7 @@ struct FilePreviewTextEditorTextKitTests { let panel = FilePreviewPanel( workspaceId: UUID(), filePath: url.path, - textLoader: { _ in .loaded(content, .utf8) } + textLoader: { _ in .loaded(content: content, encoding: .utf8) } ) defer { panel.close() } @@ -88,7 +88,7 @@ struct FilePreviewTextEditorTextKitTests { let panel = FilePreviewPanel( workspaceId: UUID(), filePath: url.path, - textLoader: { _ in .loaded(content, .utf8) } + textLoader: { _ in .loaded(content: content, encoding: .utf8) } ) defer { panel.close() } @@ -104,7 +104,7 @@ struct FilePreviewTextEditorTextKitTests { @Test("unavailable dirty load clears pending search navigation") func unavailableDirtyLoadClearsPendingSearchNavigation() async throws { - let loader = PreviewTextLoaderStub(result: .loaded("clean\n", .utf8)) + let loader = PreviewTextLoaderStub(result: .loaded(content: "clean\n", encoding: .utf8)) let url = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-file-preview-unavailable-navigation-\(UUID().uuidString)") .appendingPathExtension("swift") @@ -123,7 +123,7 @@ struct FilePreviewTextEditorTextKitTests { #expect(panel.isDirty) panel.navigateToTextPosition(lineNumber: 1, columnNumber: 7) - loader.result = .unavailable + loader.result = FilePreviewTextLoader.Result.unavailable await panel.loadTextContent(replacingDirtyContent: false).value let textView = SavingTextView.makeFilePreviewTextView() @@ -137,7 +137,7 @@ struct FilePreviewTextEditorTextKitTests { @Test("loaded dirty buffer clears pending search navigation when content differs") func loadedDirtyBufferClearsPendingSearchNavigationWhenContentDiffers() async throws { let onDiskContent = "first\nneedle\n" - let loader = PreviewTextLoaderStub(result: .loaded(onDiskContent, .utf8)) + let loader = PreviewTextLoaderStub(result: .loaded(content: onDiskContent, encoding: .utf8)) let url = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-file-preview-dirty-loaded-navigation-\(UUID().uuidString)") .appendingPathExtension("swift") From 3c0b30e5ae1d2baea8e2403b4685b8b433510e22 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 22:27:22 -0700 Subject: [PATCH 06/22] Address file explorer preview review followups --- .github/swift-file-length-budget.tsv | 4 +- .../FileWatch/FileSystemEventStream.swift | 12 ++++- .../FileWatch/RecursivePathWatcher.swift | 9 ++++ Sources/ContentView.swift | 31 +++++------ Sources/FileExplorerStore.swift | 25 ++++++++- .../FilePreviewWorkspaceOpenSupport.swift | 24 +++++++++ Sources/RightSidebarToolPanel.swift | 31 +++++------ .../FilePreviewTextEditorTextKitTests.swift | 53 +++++++++++++++++++ 8 files changed, 147 insertions(+), 42 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 8cb569ca67cc..b58ef026f4ea 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -3,7 +3,7 @@ # Reduce counts as files shrink. CI fails if tracked files exceed this budget. 34629 CLI/cmux.swift 17854 Sources/AppDelegate.swift -16487 Sources/ContentView.swift +16480 Sources/ContentView.swift 13913 Sources/TerminalController.swift 12838 Sources/Workspace.swift 12348 cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -62,7 +62,7 @@ 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1446 Sources/FileExplorerStore.swift +1469 Sources/FileExplorerStore.swift 1433 Sources/RemoteTmuxControlConnection.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift index 08fde56a3b44..22078a2e1533 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift @@ -55,12 +55,19 @@ final class FileSystemEventStream: @unchecked Sendable { /// /// - Parameters: /// - paths: The files and directories to watch. Must be non-empty. + /// - excludedPaths: Descendant paths ignored by the stream. Use this for + /// high-churn subtrees that should not wake downstream reloads. /// - latency: The FSEvents coalescing latency in seconds. /// - onEvent: A non-blocking sink invoked on the shared queue for each /// coalesced batch of filesystem events. /// - Returns: `nil` if `paths` is empty or the underlying `FSEventStream` /// could not be created or started. - init?(paths: [String], latency: TimeInterval, onEvent: @escaping @Sendable () -> Void) { + init?( + paths: [String], + excludedPaths: [String] = [], + latency: TimeInterval, + onEvent: @escaping @Sendable () -> Void + ) { guard !paths.isEmpty else { return nil } self.onEvent = onEvent self.stream = nil @@ -85,6 +92,9 @@ final class FileSystemEventStream: @unchecked Sendable { return nil } self.stream = stream + if !excludedPaths.isEmpty { + _ = FSEventStreamSetExclusionPaths(stream, excludedPaths as CFArray) + } FSEventStreamSetDispatchQueue(stream, Self.queue) guard FSEventStreamStart(stream) else { stop() diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift index 3cc657ed3e37..7edc0e461740 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift @@ -38,6 +38,9 @@ public actor RecursivePathWatcher { /// recreating an equivalent watcher. public nonisolated let watchedPaths: [String] + /// Descendant paths excluded by the underlying `FSEventStream`. + public nonisolated let excludedPaths: [String] + /// Stream of coalesced change events. Yields one element per throttle window /// in which at least one filesystem event affected a watched path. Finishes /// when ``stop()`` is called or the watcher is deallocated. @@ -63,6 +66,8 @@ public actor RecursivePathWatcher { /// /// - Parameters: /// - paths: The files and directories to watch. Must be non-empty. + /// - excludedPaths: Descendant paths to suppress at the `FSEventStream` + /// level before they reach the watcher's throttle. /// - clock: The clock driving the coalescing throttle. Defaults to /// ``SystemFileWatchClock``. /// - Returns: `nil` if `paths` is empty or the underlying `FSEventStream` @@ -70,10 +75,12 @@ public actor RecursivePathWatcher { /// listening. public init?( paths: [String], + excludedPaths: [String] = [], clock: any FileWatchClock = SystemFileWatchClock() ) { guard !paths.isEmpty else { return nil } self.watchedPaths = paths + self.excludedPaths = excludedPaths self.clock = clock let (events, eventsContinuation) = AsyncStream.makeStream() self.events = events @@ -86,6 +93,7 @@ public actor RecursivePathWatcher { // actor mid-init. guard let eventStream = FileSystemEventStream( paths: paths, + excludedPaths: excludedPaths, latency: Self.streamLatency, onEvent: { rawContinuation.yield(()) } ) else { @@ -112,6 +120,7 @@ public actor RecursivePathWatcher { /// with an injected clock and no real filesystem dependency. init(testThrottleClock clock: any FileWatchClock) { self.watchedPaths = [] + self.excludedPaths = [] self.clock = clock let (events, eventsContinuation) = AsyncStream.makeStream() self.events = events diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 7b227f3c3faa..c896f1621519 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -2314,35 +2314,28 @@ struct ContentView: View { guard let workspace else { return } do { let localURL = try await fileExplorerStore.materializeRemoteFileForPreview(path: filePath) - let panels = workspace.openFilePreviewSurfaces( + workspace.openFileSurfacesNavigatingTextPosition( inPane: paneId, - filePaths: [localURL.path], + filePath: localURL.path, + lineNumber: lineNumber, + columnNumber: columnNumber, focus: true, reuseExisting: true ) - panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) } catch { NSSound.beep() } } return } - if lineNumber != nil { - let panels = workspace.openFilePreviewSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) - panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) - } else { - _ = workspace.openFileSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) - } + workspace.openFileSurfacesNavigatingTextPosition( + inPane: paneId, + filePath: filePath, + lineNumber: lineNumber, + columnNumber: columnNumber, + focus: true, + reuseExisting: true + ) } private func syncFileExplorerDirectory() { diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 8956a4bd72a4..97456778f5cb 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -739,6 +739,19 @@ enum FileExplorerSelectionRestoration { /// because NSOutlineView data source/delegate methods are called on the main thread /// but are not annotated @MainActor. final class FileExplorerStore: ObservableObject { + private static let recursiveWatcherIgnoredDirectoryNames: Set = [ + ".build", + ".git", + ".next", + ".swiftpm", + ".turbo", + "DerivedData", + "build", + "dist", + "node_modules", + "target", + ] + @Published var rootPath: String = "" @Published var rootNodes: [FileExplorerNode] = [] @Published private(set) var isRootLoading: Bool = false @@ -893,7 +906,10 @@ final class FileExplorerStore: ObservableObject { if provider is LocalFileExplorerProvider, !rootPath.isEmpty { guard directoryWatchPath != rootPath || directoryWatcher == nil else { return } stopDirectoryWatcher() - guard let watcher = RecursivePathWatcher(paths: [rootPath]) else { return } + guard let watcher = RecursivePathWatcher( + paths: [rootPath], + excludedPaths: Self.recursiveWatcherExcludedPaths(under: rootPath) + ) else { return } directoryWatcher = watcher directoryWatchPath = rootPath let events = watcher.events @@ -1282,6 +1298,13 @@ final class FileExplorerStore: ObservableObject { return trimmed } + private static func recursiveWatcherExcludedPaths(under rootPath: String) -> [String] { + let rootURL = URL(fileURLWithPath: rootPath).standardizedFileURL + return recursiveWatcherIgnoredDirectoryNames + .map { rootURL.appendingPathComponent($0, isDirectory: true).path } + .sorted() + } + private static func remotePreviewCacheURL(displayTarget: String, remotePath: String) -> URL { let cacheRoot = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-remote-file-previews", isDirectory: true) diff --git a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift index 1eada6ab4dd1..b7a128ec3629 100644 --- a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift +++ b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift @@ -2,6 +2,30 @@ import Bonsplit import Foundation extension Workspace { + @discardableResult + func openFileSurfacesNavigatingTextPosition( + inPane paneId: PaneID, + filePath: String, + lineNumber: Int?, + columnNumber: Int?, + focus: Bool? = nil, + reuseExisting: Bool = false + ) -> [any Panel] { + let openedPanels = openFileSurfaces( + inPane: paneId, + filePaths: [filePath], + focus: focus, + reuseExisting: reuseExisting + ) + for panel in openedPanels { + (panel as? FilePreviewPanel)?.navigateToTextPosition( + lineNumber: lineNumber, + columnNumber: columnNumber + ) + } + return openedPanels + } + @discardableResult func openFileSurfaces( inPane paneId: PaneID, diff --git a/Sources/RightSidebarToolPanel.swift b/Sources/RightSidebarToolPanel.swift index eaa7da04e49d..b6dfc8204385 100644 --- a/Sources/RightSidebarToolPanel.swift +++ b/Sources/RightSidebarToolPanel.swift @@ -97,35 +97,28 @@ final class RightSidebarToolPanel: Panel, ObservableObject { guard let workspace, let store else { return } do { let localURL = try await store.materializeRemoteFileForPreview(path: filePath) - let panels = workspace.openFilePreviewSurfaces( + workspace.openFileSurfacesNavigatingTextPosition( inPane: paneId, - filePaths: [localURL.path], + filePath: localURL.path, + lineNumber: lineNumber, + columnNumber: columnNumber, focus: true, reuseExisting: true ) - panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) } catch { NSSound.beep() } } return } - if lineNumber != nil { - let panels = workspace.openFilePreviewSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) - panels.first?.navigateToTextPosition(lineNumber: lineNumber, columnNumber: columnNumber) - } else { - _ = workspace.openFileSurfaces( - inPane: paneId, - filePaths: [filePath], - focus: true, - reuseExisting: true - ) - } + workspace.openFileSurfacesNavigatingTextPosition( + inPane: paneId, + filePath: filePath, + lineNumber: lineNumber, + columnNumber: columnNumber, + focus: true, + reuseExisting: true + ) } var isFocusedInWorkspace: Bool { diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index a5563b468787..b1958e3862a2 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -102,6 +102,59 @@ struct FilePreviewTextEditorTextKitTests { #expect(textView.selectedRange().location == (content as NSString).range(of: "needle").location) } + @Test("search navigation preserves markdown surface routing") + func searchNavigationPreservesMarkdownSurfaceRouting() throws { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString) + .appendingPathExtension("md") + try "# Title\n\nneedle\n".write(to: url, atomically: true, encoding: .utf8) + defer { try? FileManager.default.removeItem(at: url) } + + let manager = TabManager() + let workspace = manager.addWorkspace(select: true, eagerLoadTerminal: false) + defer { workspace.teardownAllPanels() } + let pane = try #require(workspace.bonsplitController.allPaneIds.first) + + let openedPanels = workspace.openFileSurfacesNavigatingTextPosition( + inPane: pane, + filePath: url.path, + lineNumber: 3, + columnNumber: 1, + focus: false, + reuseExisting: true + ) + + let markdownPanel = try #require(openedPanels.first as? MarkdownPanel) + #expect(markdownPanel.filePath == url.path) + #expect((openedPanels.first as? FilePreviewPanel) == nil) + } + + @Test("search navigation preserves Xcode project surface routing") + func searchNavigationPreservesXcodeProjectSurfaceRouting() throws { + let projectURL = FileManager.default.temporaryDirectory + .appendingPathComponent("\(UUID().uuidString).xcodeproj", isDirectory: true) + try FileManager.default.createDirectory(at: projectURL, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: projectURL) } + + let manager = TabManager() + let workspace = manager.addWorkspace(select: true, eagerLoadTerminal: false) + defer { workspace.teardownAllPanels() } + let pane = try #require(workspace.bonsplitController.allPaneIds.first) + + let openedPanels = workspace.openFileSurfacesNavigatingTextPosition( + inPane: pane, + filePath: projectURL.path, + lineNumber: 1, + columnNumber: 1, + focus: false, + reuseExisting: true + ) + + let projectPanel = try #require(openedPanels.first as? ProjectPanel) + #expect(projectPanel.projectURL.path == projectURL.path) + #expect((openedPanels.first as? FilePreviewPanel) == nil) + } + @Test("unavailable dirty load clears pending search navigation") func unavailableDirtyLoadClearsPendingSearchNavigation() async throws { let loader = PreviewTextLoaderStub(result: .loaded(content: "clean\n", encoding: .utf8)) From f2323e5bf754361a7a94f860f89871b723efde87 Mon Sep 17 00:00:00 2001 From: cmux Date: Fri, 26 Jun 2026 22:41:00 -0700 Subject: [PATCH 07/22] Address file watcher and dirty navigation review --- .github/swift-file-length-budget.tsv | 4 +-- .../FileWatch/FileSystemEventStream.swift | 9 +++++-- .../FileWatch/RecursivePathWatcher.swift | 7 ++++-- .../FileWatch/RecursivePathWatcherTests.swift | 15 +++++++++++ Sources/FileExplorerStore.swift | 25 ++++++++----------- Sources/Panels/FilePreviewPanel.swift | 9 +++++-- .../FilePreviewTextEditorTextKitTests.swift | 12 ++++----- 7 files changed, 52 insertions(+), 29 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index b58ef026f4ea..80332e01ff88 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -21,7 +21,7 @@ 6077 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -4568 Sources/Panels/FilePreviewPanel.swift +4573 Sources/Panels/FilePreviewPanel.swift 4478 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4187 Sources/BrowserWindowPortal.swift @@ -62,7 +62,7 @@ 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1469 Sources/FileExplorerStore.swift +1466 Sources/FileExplorerStore.swift 1433 Sources/RemoteTmuxControlConnection.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift index 22078a2e1533..38c451ab0481 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift @@ -27,6 +27,7 @@ import Foundation /// after `FSEventStreamInvalidate`. No callback ever touches a freed instance, /// so a separately retained context box is unnecessary. final class FileSystemEventStream: @unchecked Sendable { + private static let maximumExclusionPathCount = 8 private static let queueSpecificKey = DispatchSpecificKey() private static let queue: DispatchQueue = { let queue = DispatchQueue(label: "com.cmux.recursive-path-watcher", qos: .utility) @@ -71,6 +72,7 @@ final class FileSystemEventStream: @unchecked Sendable { guard !paths.isEmpty else { return nil } self.onEvent = onEvent self.stream = nil + let effectiveExcludedPaths = Array(excludedPaths.prefix(Self.maximumExclusionPathCount)) var context = FSEventStreamContext( version: 0, @@ -92,8 +94,11 @@ final class FileSystemEventStream: @unchecked Sendable { return nil } self.stream = stream - if !excludedPaths.isEmpty { - _ = FSEventStreamSetExclusionPaths(stream, excludedPaths as CFArray) + if !effectiveExcludedPaths.isEmpty { + guard FSEventStreamSetExclusionPaths(stream, effectiveExcludedPaths as CFArray) else { + stop() + return nil + } } FSEventStreamSetDispatchQueue(stream, Self.queue) guard FSEventStreamStart(stream) else { diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift index 7edc0e461740..e8c0718d0f0a 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift @@ -32,6 +32,8 @@ import Foundation /// and applies the throttle. The pump's lifetime is the raw stream's: ``stop()`` /// and `deinit` finish it. public actor RecursivePathWatcher { + private static let maximumFSEventExclusionPathCount = 8 + /// The paths this watcher observes, as passed to ``init(paths:clock:)``. /// /// Exposed so callers can compare against a freshly resolved set and skip @@ -79,8 +81,9 @@ public actor RecursivePathWatcher { clock: any FileWatchClock = SystemFileWatchClock() ) { guard !paths.isEmpty else { return nil } + let effectiveExcludedPaths = Array(excludedPaths.prefix(Self.maximumFSEventExclusionPathCount)) self.watchedPaths = paths - self.excludedPaths = excludedPaths + self.excludedPaths = effectiveExcludedPaths self.clock = clock let (events, eventsContinuation) = AsyncStream.makeStream() self.events = events @@ -93,7 +96,7 @@ public actor RecursivePathWatcher { // actor mid-init. guard let eventStream = FileSystemEventStream( paths: paths, - excludedPaths: excludedPaths, + excludedPaths: effectiveExcludedPaths, latency: Self.streamLatency, onEvent: { rawContinuation.yield(()) } ) else { diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileWatch/RecursivePathWatcherTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileWatch/RecursivePathWatcherTests.swift index 6c5a05ae0f10..58ca1b26ec66 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileWatch/RecursivePathWatcherTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/FileWatch/RecursivePathWatcherTests.swift @@ -54,6 +54,21 @@ private actor GateClock: FileWatchClock { await watcher?.stop() } + @Test func exclusionPathsAreLimitedAndCanPointAtMissingSubtrees() async { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-file-watch-exclusions-\(UUID().uuidString)", isDirectory: true) + try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let exclusions = (0..<10).map { + directory.appendingPathComponent("ignored-\($0)", isDirectory: true).path + } + + let watcher = RecursivePathWatcher(paths: [directory.path], excludedPaths: exclusions) + #expect(watcher != nil) + #expect(watcher?.excludedPaths == Array(exclusions.prefix(8))) + await watcher?.stop() + } + /// A burst of events inside one throttle window coalesces into a single /// yield, a fresh event re-arms the throttle, and `stop()` finishes the /// stream. This is the leading-edge behavior the watcher provides: react once diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 97456778f5cb..3a564e85e710 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -739,19 +739,6 @@ enum FileExplorerSelectionRestoration { /// because NSOutlineView data source/delegate methods are called on the main thread /// but are not annotated @MainActor. final class FileExplorerStore: ObservableObject { - private static let recursiveWatcherIgnoredDirectoryNames: Set = [ - ".build", - ".git", - ".next", - ".swiftpm", - ".turbo", - "DerivedData", - "build", - "dist", - "node_modules", - "target", - ] - @Published var rootPath: String = "" @Published var rootNodes: [FileExplorerNode] = [] @Published private(set) var isRootLoading: Bool = false @@ -1300,7 +1287,17 @@ final class FileExplorerStore: ObservableObject { private static func recursiveWatcherExcludedPaths(under rootPath: String) -> [String] { let rootURL = URL(fileURLWithPath: rootPath).standardizedFileURL - return recursiveWatcherIgnoredDirectoryNames + let ignoredDirectoryNames = [ + ".build", + ".git", + ".next", + ".swiftpm", + "DerivedData", + "build", + "dist", + "node_modules", + ] + return ignoredDirectoryNames .map { rootURL.appendingPathComponent($0, isDirectory: true).path } .sorted() } diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index 93040a02578d..a9d43202f5e5 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1073,15 +1073,20 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan focusCoordinator.fulfillPendingFocusIfNeeded() } - func navigateToTextPosition(lineNumber: Int?, columnNumber: Int?) { - guard let lineNumber else { return } + @discardableResult + func navigateToTextPosition(lineNumber: Int?, columnNumber: Int?) -> Task? { + guard let lineNumber else { return nil } pendingTextNavigation = ( lineNumber: max(1, lineNumber), columnNumber: max(1, columnNumber ?? 1) ) focusCoordinator.notePreferredIntent(.textEditor) _ = restoreFocusIntent(.filePreview(.textEditor)) + if isDirty { + return loadTextContent(replacingDirtyContent: false) + } applyPendingTextNavigationIfReady() + return nil } func attachPDFPreview(root: NSView, primaryResponder: NSView) { diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index b1958e3862a2..1b0472c6084b 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -175,15 +175,14 @@ struct FilePreviewTextEditorTextKitTests { panel.updateTextContent("dirty needle\n") #expect(panel.isDirty) - panel.navigateToTextPosition(lineNumber: 1, columnNumber: 7) - loader.result = FilePreviewTextLoader.Result.unavailable - await panel.loadTextContent(replacingDirtyContent: false).value - let textView = SavingTextView.makeFilePreviewTextView() textView.string = panel.textContent textView.setSelectedRange(NSRange(location: 0, length: 0)) panel.attachTextView(textView) + loader.result = FilePreviewTextLoader.Result.unavailable + await panel.navigateToTextPosition(lineNumber: 1, columnNumber: 7)?.value + #expect(textView.selectedRange().location == 0) } @@ -208,14 +207,13 @@ struct FilePreviewTextEditorTextKitTests { panel.updateTextContent("dirty\nfirst\nneedle\n") #expect(panel.isDirty) - panel.navigateToTextPosition(lineNumber: 2, columnNumber: 1) - await panel.loadTextContent(replacingDirtyContent: false).value - let textView = SavingTextView.makeFilePreviewTextView() textView.string = panel.textContent textView.setSelectedRange(NSRange(location: 0, length: 0)) panel.attachTextView(textView) + await panel.navigateToTextPosition(lineNumber: 2, columnNumber: 1)?.value + #expect(textView.selectedRange().location == 0) } } From 059d64554f273dc56caf7abbc6e55f0f124f380c Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 16:52:18 -0700 Subject: [PATCH 08/22] Coalesce file-explorer git status refreshes under watcher churn The recursive directory watcher delivers a coalesced event roughly once per throttle window, but a sustained filesystem storm (build output, log writes, nested high-churn dirs not covered by the top-level-only FSEvents exclusion list) still drove one `refreshGitStatus()` per window. Each fetch spawned an uncancelled `git status` process, so on a large repo where the fetch outlasts the window those processes piled up unbounded. Guard `refreshGitStatus()` so at most one `git status` runs at a time and coalesce concurrent requests into a single trailing re-run. `reload()` already supersedes its own prior load via `cancelAllLoads()`, so the watcher loop now has bounded backpressure on both work items. Co-Authored-By: Claude Opus 4.8 --- Sources/FileExplorerStore.swift | 47 +++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 3a564e85e710..72fcda979d67 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -757,6 +757,18 @@ final class FileExplorerStore: ObservableObject { private var directoryWatchTask: Task? private var directoryWatchPath: String? + /// Whether a `git status` fetch started by ``refreshGitStatus()`` is still + /// running. The recursive watcher can request a refresh once per throttle + /// window during a sustained filesystem storm; without this guard each + /// request would spawn another uncancelled `git status` process, letting + /// slow fetches on a large repo pile up. Only ever read/written on the main + /// actor. + private var isGitStatusRefreshInFlight = false + /// Set when a refresh is requested while one is already in flight, so exactly + /// one follow-up runs after the current fetch finishes (coalescing a burst of + /// requests into a single trailing refresh rather than N concurrent ones). + private var gitStatusRefreshRequestedWhileInFlight = false + /// Paths that are logically expanded (persisted across provider changes) private(set) var expandedPaths: Set = [] @@ -850,8 +862,20 @@ final class FileExplorerStore: ObservableObject { func refreshGitStatus() { guard !rootPath.isEmpty else { gitStatusByPath = [:] + gitStatusRefreshRequestedWhileInFlight = false + return + } + // Coalesce overlapping refreshes. The recursive directory watcher can + // request a refresh once per throttle window during a sustained + // filesystem storm, and each fetch spawns an external `git status` + // process. Running at most one at a time ā€” and re-running once if a + // request arrived mid-flight ā€” bounds this to a single process instead + // of letting slow fetches on a large repo pile up. + if isGitStatusRefreshInFlight { + gitStatusRefreshRequestedWhileInFlight = true return } + isGitStatusRefreshInFlight = true let path = rootPath if let sshProvider = provider as? SSHFileExplorerProvider { let dest = sshProvider.destination @@ -864,19 +888,32 @@ final class FileExplorerStore: ObservableObject { identityFile: identity, sshOptions: opts ) DispatchQueue.main.async { [weak self] in - self?.gitStatusByPath = status + self?.applyGitStatusResult(status) } } } else { DispatchQueue.global(qos: .utility).async { let status = GitStatusProvider.fetchStatus(directory: path) DispatchQueue.main.async { [weak self] in - self?.gitStatusByPath = status + self?.applyGitStatusResult(status) } } } } + /// Applies a completed `git status` result and, if a refresh was requested + /// while the fetch was in flight, runs exactly one coalesced follow-up so the + /// final state reflects the latest filesystem change without spawning + /// overlapping `git status` processes. + private func applyGitStatusResult(_ status: [String: GitFileStatus]) { + gitStatusByPath = status + isGitStatusRefreshInFlight = false + if gitStatusRefreshRequestedWhileInFlight { + gitStatusRefreshRequestedWhileInFlight = false + refreshGitStatus() + } + } + func materializeRemoteFileForPreview(path: String) async throws -> URL { guard let sshProvider = provider as? SSHFileExplorerProvider else { throw FileExplorerError.providerUnavailable @@ -900,6 +937,12 @@ final class FileExplorerStore: ObservableObject { directoryWatcher = watcher directoryWatchPath = rootPath let events = watcher.events + // `events` is already leading-edge throttled by RecursivePathWatcher + // (one element per window even during a sustained storm), so this + // loop runs at a bounded rate rather than per raw filesystem event. + // `reload()` supersedes its own prior load via `cancelAllLoads()`, + // and `refreshGitStatus()` coalesces to a single in-flight process, + // so neither work item accumulates under churn. directoryWatchTask = Task { @MainActor [weak self] in for await _ in events { guard let self else { break } From 40a7b287351facfd95890d48e96fb19342dae739 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 17:14:46 -0700 Subject: [PATCH 09/22] Watch .git metadata so explorer badges refresh after commit/stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main recursive watcher excludes .git (and build/dependency dirs) to avoid reload churn, but that also suppressed the events behind metadata- only git operations ā€” commit, add, reset ā€” which change `git status` output without touching the working tree. Badges could stay stale until an unrelated working-tree event happened. Add a second, low-churn watcher over `.git` (excluding the high-churn `objects`/`logs` subtrees) that triggers only `refreshGitStatus()`, never a tree `reload()`, since `.git` writes don't change the visible file tree. No-op when `.git` is absent or a worktree/submodule link file. Refresh the Swift file length budget for the added watcher. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- Sources/FileExplorerStore.swift | 45 ++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 06f789c03450..988b3fdb7c44 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -59,10 +59,10 @@ 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift +1550 Sources/FileExplorerStore.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1466 Sources/FileExplorerStore.swift 1433 Sources/RemoteTmuxControlConnection.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 72fcda979d67..2b28fa847bdd 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -757,6 +757,14 @@ final class FileExplorerStore: ObservableObject { private var directoryWatchTask: Task? private var directoryWatchPath: String? + /// Watches the repository's `.git` metadata so status badges refresh after + /// metadata-only changes (`git commit`, `git add`, `git reset`) that never + /// touch the working tree ā€” and so are excluded from the main tree watcher. + /// Kept separate because those events must refresh git status without + /// triggering a tree `reload()`. + private var gitStateWatcher: RecursivePathWatcher? + private var gitStateWatchTask: Task? + /// Whether a `git status` fetch started by ``refreshGitStatus()`` is still /// running. The recursive watcher can request a refresh once per throttle /// window during a sustained filesystem storm; without this guard each @@ -950,18 +958,51 @@ final class FileExplorerStore: ObservableObject { self.refreshGitStatus() } } + startGitStateWatcher(under: rootPath) } else { stopDirectoryWatcher() } } - /// Cancels the directory-watch consumer and drops the watcher; the watcher's - /// deinit tears down its filesystem stream synchronously. + /// Watches the repository's `.git` directory (minus the high-churn + /// `objects`/`logs` subtrees) so status badges refresh after metadata-only + /// changes the main tree watcher intentionally excludes. Only refreshes git + /// status ā€” `.git` writes don't change the visible file tree, so no + /// `reload()`. No-op when `.git` is absent or is a worktree/submodule link + /// file rather than a directory. + private func startGitStateWatcher(under rootPath: String) { + let gitDirectory = URL(fileURLWithPath: rootPath, isDirectory: true) + .appendingPathComponent(".git", isDirectory: true) + var isDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: gitDirectory.path, isDirectory: &isDirectory), + isDirectory.boolValue else { return } + guard let watcher = RecursivePathWatcher( + paths: [gitDirectory.path], + excludedPaths: [ + gitDirectory.appendingPathComponent("objects", isDirectory: true).path, + gitDirectory.appendingPathComponent("logs", isDirectory: true).path, + ] + ) else { return } + gitStateWatcher = watcher + let events = watcher.events + gitStateWatchTask = Task { @MainActor [weak self] in + for await _ in events { + guard let self else { break } + self.refreshGitStatus() + } + } + } + + /// Cancels the directory-watch consumers and drops the watchers; each + /// watcher's deinit tears down its filesystem stream synchronously. private func stopDirectoryWatcher() { directoryWatchTask?.cancel() directoryWatchTask = nil directoryWatcher = nil directoryWatchPath = nil + gitStateWatchTask?.cancel() + gitStateWatchTask = nil + gitStateWatcher = nil } private func setProvider(_ newProvider: FileExplorerProvider?, reloadIfAvailable: Bool = true) { From 06b9303ad60909e490a6a3039fe1214090d59392 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 17:31:08 -0700 Subject: [PATCH 10/22] Scope git status coalescing to the explorer root/provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-flight coalescing gate was store-global, so on a workspace switch A -> B while A's `git status` was slow or hung: B's refresh only set the trailing flag (blocked until A returned), and A's stale result was then applied to the now-current B before the trailing refresh ran. Key each fetch to a context generation that bumps on every root/provider change. `invalidateGitStatusRefresh()` (called from setRootPath and setProvider) releases the in-flight gate so the new context's fetch starts immediately, and a completion whose generation no longer matches is dropped ā€” so a slow/hung fetch for a previous workspace can neither block nor overwrite the active one. Same-root watcher churn stays coalesced to a single in-flight process plus one trailing refresh. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- Sources/FileExplorerStore.swift | 41 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 988b3fdb7c44..dc9faa44bd4c 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -58,8 +58,8 @@ 1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift +1573 Sources/FileExplorerStore.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift -1550 Sources/FileExplorerStore.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 2b28fa847bdd..00793581af8b 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -765,12 +765,18 @@ final class FileExplorerStore: ObservableObject { private var gitStateWatcher: RecursivePathWatcher? private var gitStateWatchTask: Task? - /// Whether a `git status` fetch started by ``refreshGitStatus()`` is still - /// running. The recursive watcher can request a refresh once per throttle - /// window during a sustained filesystem storm; without this guard each - /// request would spawn another uncancelled `git status` process, letting - /// slow fetches on a large repo pile up. Only ever read/written on the main + /// Bumped whenever the explorer root or provider changes + /// (``invalidateGitStatusRefresh()``). A `git status` fetch captures this at + /// launch; a result whose generation no longer matches is dropped, so a + /// slow/hung fetch for a previous workspace can neither overwrite the current + /// workspace's status nor block its refresh. Only read/written on the main /// actor. + private var gitStatusContextGeneration = 0 + /// Whether a `git status` fetch is in flight for the current context. The + /// recursive watcher can request a refresh once per throttle window during a + /// sustained filesystem storm; without this guard each request would spawn + /// another uncancelled `git status` process, letting slow fetches on a large + /// repo pile up. Only read/written on the main actor. private var isGitStatusRefreshInFlight = false /// Set when a refresh is requested while one is already in flight, so exactly /// one follow-up runs after the current fetch finishes (coalescing a burst of @@ -862,6 +868,7 @@ final class FileExplorerStore: ObservableObject { pendingDescendIntoFirstChildPath = nil } rootPath = path + invalidateGitStatusRefresh() reload() refreshGitStatus() updateDirectoryWatcher() @@ -885,6 +892,7 @@ final class FileExplorerStore: ObservableObject { } isGitStatusRefreshInFlight = true let path = rootPath + let generation = gitStatusContextGeneration if let sshProvider = provider as? SSHFileExplorerProvider { let dest = sshProvider.destination let port = sshProvider.port @@ -896,14 +904,14 @@ final class FileExplorerStore: ObservableObject { identityFile: identity, sshOptions: opts ) DispatchQueue.main.async { [weak self] in - self?.applyGitStatusResult(status) + self?.applyGitStatusResult(status, generation: generation) } } } else { DispatchQueue.global(qos: .utility).async { let status = GitStatusProvider.fetchStatus(directory: path) DispatchQueue.main.async { [weak self] in - self?.applyGitStatusResult(status) + self?.applyGitStatusResult(status, generation: generation) } } } @@ -912,8 +920,11 @@ final class FileExplorerStore: ObservableObject { /// Applies a completed `git status` result and, if a refresh was requested /// while the fetch was in flight, runs exactly one coalesced follow-up so the /// final state reflects the latest filesystem change without spawning - /// overlapping `git status` processes. - private func applyGitStatusResult(_ status: [String: GitFileStatus]) { + /// overlapping `git status` processes. Results from a fetch superseded by a + /// root/provider change (stale `generation`) are dropped without touching the + /// newer fetch's in-flight state. + private func applyGitStatusResult(_ status: [String: GitFileStatus], generation: Int) { + guard generation == gitStatusContextGeneration else { return } gitStatusByPath = status isGitStatusRefreshInFlight = false if gitStatusRefreshRequestedWhileInFlight { @@ -922,6 +933,17 @@ final class FileExplorerStore: ObservableObject { } } + /// Invalidates any in-flight `git status` fetch when the explorer root or + /// provider changes: the in-flight result is dropped (its generation no + /// longer matches) and the in-flight gate is released so a fetch for the new + /// context can start immediately instead of waiting behind a slow or hung + /// fetch for the previous one. + private func invalidateGitStatusRefresh() { + gitStatusContextGeneration &+= 1 + isGitStatusRefreshInFlight = false + gitStatusRefreshRequestedWhileInFlight = false + } + func materializeRemoteFileForPreview(path: String) async throws -> URL { guard let sshProvider = provider as? SSHFileExplorerProvider else { throw FileExplorerError.providerUnavailable @@ -1010,6 +1032,7 @@ final class FileExplorerStore: ObservableObject { NSLog("[FileExplorer] setProvider: \(type(of: newProvider).self) available=\(newProvider?.isAvailable ?? false)") #endif provider = newProvider + invalidateGitStatusRefresh() // Re-expand previously expanded nodes if provider becomes available if reloadIfAvailable, newProvider?.isAvailable == true { reload() From c4862775ddfb23b25ec73d02fdfd118cd3d20150 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 17:43:25 -0700 Subject: [PATCH 11/22] Resolve .git gitdir pointer for worktree/submodule status watching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startGitStateWatcher only handled `.git` as a directory, so in Git worktrees and submodules ā€” where `.git` is a file containing a `gitdir:` pointer to an external metadata directory ā€” metadata-only operations (commit, add, reset) updated that external gitdir and the explorer's status badges never refreshed. Resolve the `gitdir:` pointer (absolute or relative to the root) to the authoritative metadata directory and watch that, so worktrees and submodules get the same badge-refresh behavior as plain checkouts. Add a deterministic unit test for the resolver covering plain `.git` dirs, absolute and relative gitdir pointers, missing repos, and dangling pointers. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 +- Sources/FileExplorerStore.swift | 64 +++++++++++++++++++++----- cmuxTests/FileExplorerStoreTests.swift | 52 +++++++++++++++++++++ 3 files changed, 106 insertions(+), 14 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index dc9faa44bd4c..ec401d44650a 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -57,8 +57,8 @@ 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift 1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift +1613 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift -1573 Sources/FileExplorerStore.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift @@ -69,12 +69,12 @@ 1373 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift 1360 Sources/Feed/FeedButtonStyleDebugWindowController.swift +1301 cmuxTests/FileExplorerStoreTests.swift 1295 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Config/GhosttyConfig.swift 1295 cmuxTests/MobileHostAuthorizationTests.swift 1287 cmuxTests/RestorableAgentSessionIndexTests.swift 1285 cmuxUITests/SidebarHelpMenuUITests.swift 1258 Sources/Feed/FeedCoordinator.swift -1249 cmuxTests/FileExplorerStoreTests.swift 1209 Packages/macOS/CmuxCommandPalette/Tests/CmuxCommandPaletteTests/CommandPaletteSearchEngineTests.swift 1205 Sources/RemoteTmuxController.swift 1197 cmuxTests/CodexAppServerSessionTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 00793581af8b..addac0f2831b 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -986,23 +986,20 @@ final class FileExplorerStore: ObservableObject { } } - /// Watches the repository's `.git` directory (minus the high-churn + /// Watches the repository's Git metadata directory (minus the high-churn /// `objects`/`logs` subtrees) so status badges refresh after metadata-only /// changes the main tree watcher intentionally excludes. Only refreshes git - /// status ā€” `.git` writes don't change the visible file tree, so no - /// `reload()`. No-op when `.git` is absent or is a worktree/submodule link - /// file rather than a directory. + /// status ā€” Git metadata writes don't change the visible file tree, so no + /// `reload()`. Resolves the `gitdir:` pointer when `.git` is a file (the + /// worktree/submodule layout); no-op when there is no usable `.git` entry. private func startGitStateWatcher(under rootPath: String) { - let gitDirectory = URL(fileURLWithPath: rootPath, isDirectory: true) - .appendingPathComponent(".git", isDirectory: true) - var isDirectory: ObjCBool = false - guard FileManager.default.fileExists(atPath: gitDirectory.path, isDirectory: &isDirectory), - isDirectory.boolValue else { return } + guard let gitMetadataDirectory = Self.gitMetadataDirectory(under: rootPath) else { return } + let metadata = gitMetadataDirectory as NSString guard let watcher = RecursivePathWatcher( - paths: [gitDirectory.path], + paths: [gitMetadataDirectory], excludedPaths: [ - gitDirectory.appendingPathComponent("objects", isDirectory: true).path, - gitDirectory.appendingPathComponent("logs", isDirectory: true).path, + metadata.appendingPathComponent("objects"), + metadata.appendingPathComponent("logs"), ] ) else { return } gitStateWatcher = watcher @@ -1015,6 +1012,49 @@ final class FileExplorerStore: ObservableObject { } } + /// Resolves the directory holding the repository's Git metadata for + /// `rootPath`. When `.git` is a directory it is the metadata directory; when + /// `.git` is a file (the layout Git uses for worktrees and submodules, where + /// `git add`/`commit`/`reset` update an external gitdir) its `gitdir:` + /// pointer is followed and resolved ā€” relative targets against `rootPath`. + /// Returns nil when there is no `.git` entry or the resolved target is not a + /// directory. + static func gitMetadataDirectory(under rootPath: String) -> String? { + let gitPath = (rootPath as NSString).appendingPathComponent(".git") + var isDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: gitPath, isDirectory: &isDirectory) else { + return nil + } + if isDirectory.boolValue { + return gitPath + } + guard let contents = try? String(contentsOfFile: gitPath, encoding: .utf8) else { + return nil + } + let gitdirPrefix = "gitdir:" + let pointerLine = contents + .split(whereSeparator: { $0.isNewline }) + .lazy + .map { String($0).trimmingCharacters(in: .whitespacesAndNewlines) } + .first { $0.hasPrefix(gitdirPrefix) } + guard let pointerLine else { return nil } + let target = String(pointerLine.dropFirst(gitdirPrefix.count)) + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !target.isEmpty else { return nil } + let absoluteTarget = (target as NSString).isAbsolutePath + ? target + : (rootPath as NSString).appendingPathComponent(target) + // Resolve `.`/`..` lexically without touching symlinks so the watched + // path stays consistent with `rootPath`'s own representation. + let resolved = URL(fileURLWithPath: absoluteTarget).standardizedFileURL.path + var resolvedIsDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: resolved, isDirectory: &resolvedIsDirectory), + resolvedIsDirectory.boolValue else { + return nil + } + return resolved + } + /// Cancels the directory-watch consumers and drops the watchers; each /// watcher's deinit tears down its filesystem stream synchronously. private func stopDirectoryWatcher() { diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 9da6d80fd4cd..1c194e367e77 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -652,6 +652,58 @@ struct FileExplorerStoreTests { store.expand(node: node) #expect(!(store.isExpanded(node))) } + + @Test("gitMetadataDirectory resolves .git dirs, worktree/submodule gitdir files, and missing repos") + func testGitMetadataDirectoryResolution() throws { + let fileManager = FileManager.default + let workspaceBase = fileManager.temporaryDirectory + .appendingPathComponent("cmux-git-metadata-\(UUID().uuidString)", isDirectory: true) + try fileManager.createDirectory(at: workspaceBase, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: workspaceBase) } + // Resolve /var -> /private/var up front so expected paths match the + // store's lexical standardization regardless of temp-dir symlinks. + let workspace = workspaceBase.resolvingSymlinksInPath() + + func makeDirectory(_ name: String) throws -> URL { + let url = workspace.appendingPathComponent(name, isDirectory: true) + try fileManager.createDirectory(at: url, withIntermediateDirectories: true) + return url + } + func writeGitFile(_ contents: String, in root: URL) throws { + try contents.write( + to: root.appendingPathComponent(".git"), + atomically: true, + encoding: .utf8 + ) + } + + // No `.git` entry ā†’ nil. + let plainRoot = try makeDirectory("plain") + #expect(FileExplorerStore.gitMetadataDirectory(under: plainRoot.path) == nil) + + // `.git` directory ā†’ the directory itself. + let repoRoot = try makeDirectory("repo") + let repoGitDir = repoRoot.appendingPathComponent(".git", isDirectory: true) + try fileManager.createDirectory(at: repoGitDir, withIntermediateDirectories: true) + #expect(FileExplorerStore.gitMetadataDirectory(under: repoRoot.path) == repoGitDir.path) + + // Worktree layout: `.git` file with an absolute `gitdir:` pointer. + let worktreeRoot = try makeDirectory("worktree") + let externalGitDir = try makeDirectory("main/.git/worktrees/wt") + try writeGitFile("gitdir: \(externalGitDir.path)\n", in: worktreeRoot) + #expect(FileExplorerStore.gitMetadataDirectory(under: worktreeRoot.path) == externalGitDir.path) + + // Submodule layout: `.git` file with a relative `gitdir:` pointer. + let submoduleRoot = try makeDirectory("submodule") + let submoduleGitDir = try makeDirectory("shared-modules") + try writeGitFile("gitdir: ../shared-modules\n", in: submoduleRoot) + #expect(FileExplorerStore.gitMetadataDirectory(under: submoduleRoot.path) == submoduleGitDir.path) + + // `gitdir:` pointing at a non-existent directory ā†’ nil. + let danglingRoot = try makeDirectory("dangling") + try writeGitFile("gitdir: \(workspace.appendingPathComponent("missing").path)\n", in: danglingRoot) + #expect(FileExplorerStore.gitMetadataDirectory(under: danglingRoot.path) == nil) + } } @MainActor From f9591a1cc7443a4e52290dc3c622969d6aa15ec4 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 17:49:14 -0700 Subject: [PATCH 12/22] Validate resolved gitdir is a real Git metadata directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.git` file's `gitdir:` pointer (and a `.git` symlink) is controlled by repository contents, so a checked-out workspace could aim it at `/`, a home directory, or another large/sensitive tree and make cmux watch that recursively and re-run `git status` on unrelated churn ā€” a trust-boundary issue. Before watching, require the resolved candidate to have the Git metadata shape: a `HEAD` file plus either `config` (main checkout or submodule) or `commondir` (linked worktree). This also rejects a `.git` symlink that resolves to a non-Git directory. Extend the resolver test with bare-`.git` and arbitrary-target rejection cases. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 +- Sources/FileExplorerStore.swift | 80 +++++++++++++++++--------- cmuxTests/FileExplorerStoreTests.swift | 43 ++++++++++---- 3 files changed, 88 insertions(+), 39 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index ec401d44650a..a1d241c1b4ab 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -57,7 +57,7 @@ 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift 1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift -1613 Sources/FileExplorerStore.swift +1641 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift @@ -69,7 +69,7 @@ 1373 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift 1360 Sources/Feed/FeedButtonStyleDebugWindowController.swift -1301 cmuxTests/FileExplorerStoreTests.swift +1322 cmuxTests/FileExplorerStoreTests.swift 1295 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Config/GhosttyConfig.swift 1295 cmuxTests/MobileHostAuthorizationTests.swift 1287 cmuxTests/RestorableAgentSessionIndexTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index addac0f2831b..263213251bb7 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -1017,42 +1017,70 @@ final class FileExplorerStore: ObservableObject { /// `.git` is a file (the layout Git uses for worktrees and submodules, where /// `git add`/`commit`/`reset` update an external gitdir) its `gitdir:` /// pointer is followed and resolved ā€” relative targets against `rootPath`. - /// Returns nil when there is no `.git` entry or the resolved target is not a - /// directory. + /// + /// The resolved candidate is validated to actually look like Git metadata + /// before being returned: a `.git` file's pointer (and a `.git` symlink) is + /// repository-controlled, so without this check a checked-out workspace could + /// aim `gitdir:` at `/`, a home directory, or another large/sensitive tree + /// and make cmux watch it recursively. Returns nil when there is no `.git` + /// entry, the resolved target is not a directory, or it lacks the Git + /// metadata shape. static func gitMetadataDirectory(under rootPath: String) -> String? { let gitPath = (rootPath as NSString).appendingPathComponent(".git") var isDirectory: ObjCBool = false guard FileManager.default.fileExists(atPath: gitPath, isDirectory: &isDirectory) else { return nil } + + let candidate: String if isDirectory.boolValue { - return gitPath - } - guard let contents = try? String(contentsOfFile: gitPath, encoding: .utf8) else { + candidate = gitPath + } else { + guard let contents = try? String(contentsOfFile: gitPath, encoding: .utf8) else { + return nil + } + let gitdirPrefix = "gitdir:" + let pointerLine = contents + .split(whereSeparator: { $0.isNewline }) + .lazy + .map { String($0).trimmingCharacters(in: .whitespacesAndNewlines) } + .first { $0.hasPrefix(gitdirPrefix) } + guard let pointerLine else { return nil } + let target = String(pointerLine.dropFirst(gitdirPrefix.count)) + .trimmingCharacters(in: .whitespacesAndNewlines) + guard !target.isEmpty else { return nil } + let absoluteTarget = (target as NSString).isAbsolutePath + ? target + : (rootPath as NSString).appendingPathComponent(target) + // Resolve `.`/`..` lexically without touching symlinks so the watched + // path stays consistent with `rootPath`'s own representation. + candidate = URL(fileURLWithPath: absoluteTarget).standardizedFileURL.path + } + + var candidateIsDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: candidate, isDirectory: &candidateIsDirectory), + candidateIsDirectory.boolValue, + Self.isPlausibleGitMetadataDirectory(candidate) else { return nil } - let gitdirPrefix = "gitdir:" - let pointerLine = contents - .split(whereSeparator: { $0.isNewline }) - .lazy - .map { String($0).trimmingCharacters(in: .whitespacesAndNewlines) } - .first { $0.hasPrefix(gitdirPrefix) } - guard let pointerLine else { return nil } - let target = String(pointerLine.dropFirst(gitdirPrefix.count)) - .trimmingCharacters(in: .whitespacesAndNewlines) - guard !target.isEmpty else { return nil } - let absoluteTarget = (target as NSString).isAbsolutePath - ? target - : (rootPath as NSString).appendingPathComponent(target) - // Resolve `.`/`..` lexically without touching symlinks so the watched - // path stays consistent with `rootPath`'s own representation. - let resolved = URL(fileURLWithPath: absoluteTarget).standardizedFileURL.path - var resolvedIsDirectory: ObjCBool = false - guard FileManager.default.fileExists(atPath: resolved, isDirectory: &resolvedIsDirectory), - resolvedIsDirectory.boolValue else { - return nil + return candidate + } + + /// Whether `path` has the shape of a Git metadata directory: a `HEAD` file + /// plus either a `config` (main checkout or submodule) or a `commondir` + /// (linked worktree). Used to reject repository-controlled `gitdir:` pointers + /// or `.git` symlinks that aim at arbitrary, non-Git directories. + private static func isPlausibleGitMetadataDirectory(_ path: String) -> Bool { + let directory = path as NSString + func containsFile(_ name: String) -> Bool { + var isDirectory: ObjCBool = false + return FileManager.default.fileExists( + atPath: directory.appendingPathComponent(name), + isDirectory: &isDirectory + ) && !isDirectory.boolValue } - return resolved + guard containsFile("HEAD") else { return false } + return containsFile("config") || containsFile("commondir") } /// Cancels the directory-watch consumers and drops the watchers; each diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 1c194e367e77..ba1ab7b23ae1 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -653,7 +653,7 @@ struct FileExplorerStoreTests { #expect(!(store.isExpanded(node))) } - @Test("gitMetadataDirectory resolves .git dirs, worktree/submodule gitdir files, and missing repos") + @Test("gitMetadataDirectory resolves plausible .git dirs and worktree/submodule gitdir files, rejecting others") func testGitMetadataDirectoryResolution() throws { let fileManager = FileManager.default let workspaceBase = fileManager.temporaryDirectory @@ -669,39 +669,60 @@ struct FileExplorerStoreTests { try fileManager.createDirectory(at: url, withIntermediateDirectories: true) return url } - func writeGitFile(_ contents: String, in root: URL) throws { - try contents.write( - to: root.appendingPathComponent(".git"), - atomically: true, - encoding: .utf8 - ) + func write(_ contents: String, to url: URL) throws { + try contents.write(to: url, atomically: true, encoding: .utf8) + } + // Gives `directory` the minimal Git metadata shape the resolver requires. + func populateGitMetadata(_ directory: URL, marker: String) throws { + try write("ref: refs/heads/main\n", to: directory.appendingPathComponent("HEAD")) + try write("", to: directory.appendingPathComponent(marker)) + } + func writeGitPointer(_ contents: String, in root: URL) throws { + try write(contents, to: root.appendingPathComponent(".git")) } // No `.git` entry ā†’ nil. let plainRoot = try makeDirectory("plain") #expect(FileExplorerStore.gitMetadataDirectory(under: plainRoot.path) == nil) - // `.git` directory ā†’ the directory itself. + // Plausible `.git` directory ā†’ the directory itself. let repoRoot = try makeDirectory("repo") let repoGitDir = repoRoot.appendingPathComponent(".git", isDirectory: true) try fileManager.createDirectory(at: repoGitDir, withIntermediateDirectories: true) + try populateGitMetadata(repoGitDir, marker: "config") #expect(FileExplorerStore.gitMetadataDirectory(under: repoRoot.path) == repoGitDir.path) + // `.git` directory without the Git metadata shape ā†’ nil. + let bareRoot = try makeDirectory("bare") + try fileManager.createDirectory( + at: bareRoot.appendingPathComponent(".git", isDirectory: true), + withIntermediateDirectories: true + ) + #expect(FileExplorerStore.gitMetadataDirectory(under: bareRoot.path) == nil) + // Worktree layout: `.git` file with an absolute `gitdir:` pointer. let worktreeRoot = try makeDirectory("worktree") let externalGitDir = try makeDirectory("main/.git/worktrees/wt") - try writeGitFile("gitdir: \(externalGitDir.path)\n", in: worktreeRoot) + try populateGitMetadata(externalGitDir, marker: "commondir") + try writeGitPointer("gitdir: \(externalGitDir.path)\n", in: worktreeRoot) #expect(FileExplorerStore.gitMetadataDirectory(under: worktreeRoot.path) == externalGitDir.path) // Submodule layout: `.git` file with a relative `gitdir:` pointer. let submoduleRoot = try makeDirectory("submodule") let submoduleGitDir = try makeDirectory("shared-modules") - try writeGitFile("gitdir: ../shared-modules\n", in: submoduleRoot) + try populateGitMetadata(submoduleGitDir, marker: "config") + try writeGitPointer("gitdir: ../shared-modules\n", in: submoduleRoot) #expect(FileExplorerStore.gitMetadataDirectory(under: submoduleRoot.path) == submoduleGitDir.path) + // `gitdir:` aimed at a real but non-Git directory ā†’ nil (trust boundary). + let untrustedRoot = try makeDirectory("untrusted") + let arbitraryDir = try makeDirectory("arbitrary-secrets") + try writeGitPointer("gitdir: \(arbitraryDir.path)\n", in: untrustedRoot) + #expect(FileExplorerStore.gitMetadataDirectory(under: untrustedRoot.path) == nil) + // `gitdir:` pointing at a non-existent directory ā†’ nil. let danglingRoot = try makeDirectory("dangling") - try writeGitFile("gitdir: \(workspace.appendingPathComponent("missing").path)\n", in: danglingRoot) + try writeGitPointer("gitdir: \(workspace.appendingPathComponent("missing").path)\n", in: danglingRoot) #expect(FileExplorerStore.gitMetadataDirectory(under: danglingRoot.path) == nil) } } From a571ecc54c618d2e017ff4bf2da008ea805383ff Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 17:55:11 -0700 Subject: [PATCH 13/22] Bound .git pointer file reads to avoid workspace-controlled hangs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gitMetadataDirectory read a non-directory `.git` entry with String(contentsOfFile:), an unbounded synchronous read on the main actor during watcher setup. A malformed or malicious workspace with a huge `.git` regular file could hang the UI or exhaust memory just by being opened in the explorer. Read at most 64 KiB via a bounded FileHandle read ā€” a real `gitdir:` pointer is a single short line ā€” and bail if the pointer isn't found in that prefix. Add a regression test that a valid pointer buried past the cap is not parsed. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 ++-- Sources/FileExplorerStore.swift | 20 +++++++++++++++++++- cmuxTests/FileExplorerStoreTests.swift | 9 +++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index a1d241c1b4ab..ec1c0c7b10c0 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -56,8 +56,8 @@ 1687 cmuxTests/MarkdownPanelTests.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift 1661 Sources/FileExplorerView.swift +1659 Sources/FileExplorerStore.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift -1641 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1547 cmuxTests/TerminalControllerSocketSecurityTests.swift @@ -69,7 +69,7 @@ 1373 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift 1360 Sources/Feed/FeedButtonStyleDebugWindowController.swift -1322 cmuxTests/FileExplorerStoreTests.swift +1331 cmuxTests/FileExplorerStoreTests.swift 1295 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Config/GhosttyConfig.swift 1295 cmuxTests/MobileHostAuthorizationTests.swift 1287 cmuxTests/RestorableAgentSessionIndexTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 263213251bb7..e950bfd0ecd3 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -1012,6 +1012,24 @@ final class FileExplorerStore: ObservableObject { } } + /// Maximum bytes read from a non-directory `.git` entry. A real `gitdir:` + /// pointer is tiny (`gitdir: `); the cap stops a huge + /// workspace-controlled `.git` regular file from hanging the UI or + /// exhausting memory during watcher setup on the main actor. + private static let maximumGitPointerFileBytes = 64 * 1024 + + /// Reads up to ``maximumGitPointerFileBytes`` from the `.git` pointer file at + /// `path`, so a malformed or malicious oversized file can't be loaded whole. + private static func readGitPointerFile(at path: String) -> String? { + guard let handle = try? FileHandle(forReadingFrom: URL(fileURLWithPath: path)) else { + return nil + } + let data = try? handle.read(upToCount: maximumGitPointerFileBytes) + try? handle.close() + guard let data else { return nil } + return String(data: data, encoding: .utf8) + } + /// Resolves the directory holding the repository's Git metadata for /// `rootPath`. When `.git` is a directory it is the metadata directory; when /// `.git` is a file (the layout Git uses for worktrees and submodules, where @@ -1036,7 +1054,7 @@ final class FileExplorerStore: ObservableObject { if isDirectory.boolValue { candidate = gitPath } else { - guard let contents = try? String(contentsOfFile: gitPath, encoding: .utf8) else { + guard let contents = Self.readGitPointerFile(at: gitPath) else { return nil } let gitdirPrefix = "gitdir:" diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index ba1ab7b23ae1..987e35b18e0d 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -724,6 +724,15 @@ struct FileExplorerStoreTests { let danglingRoot = try makeDirectory("dangling") try writeGitPointer("gitdir: \(workspace.appendingPathComponent("missing").path)\n", in: danglingRoot) #expect(FileExplorerStore.gitMetadataDirectory(under: danglingRoot.path) == nil) + + // Oversized `.git` file: a valid pointer buried past the read cap is not + // parsed, so the read stays bounded ā†’ nil. + let oversizedRoot = try makeDirectory("oversized") + let oversizedTargetDir = try makeDirectory("oversized-gitdir") + try populateGitMetadata(oversizedTargetDir, marker: "config") + let padding = String(repeating: "#\n", count: 40_000) // > 64 KiB + try writeGitPointer(padding + "gitdir: \(oversizedTargetDir.path)\n", in: oversizedRoot) + #expect(FileExplorerStore.gitMetadataDirectory(under: oversizedRoot.path) == nil) } } From f582bf3834df05a306810337673bd00ec55b4e8e Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Mon, 29 Jun 2026 18:04:09 -0700 Subject: [PATCH 14/22] Install git-state watcher when a repo appears after folder open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main tree watcher excludes `.git`, so when a non-git folder becomes a repository while open (git init, or adding a worktree/submodule) the `.git` creation isn't observed and no git-state watcher was installed ā€” badges stayed empty until a root reset. Re-check on the next working-tree event: once `.git` resolves, install the git-state watcher so metadata changes refresh badges from then on. Cheap (a single stat per coalesced event, only while no repo is present) and self-disabling once installed. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- Sources/FileExplorerStore.swift | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index ec1c0c7b10c0..9edaa15602a2 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -55,8 +55,8 @@ 1732 cmuxTests/WorkspacePullRequestSidebarTests.swift 1687 cmuxTests/MarkdownPanelTests.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift +1671 Sources/FileExplorerStore.swift 1661 Sources/FileExplorerView.swift -1659 Sources/FileExplorerStore.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index e950bfd0ecd3..2b96041a7625 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -978,6 +978,7 @@ final class FileExplorerStore: ObservableObject { guard let self else { break } self.reload() self.refreshGitStatus() + self.installGitStateWatcherIfNeeded() } } startGitStateWatcher(under: rootPath) @@ -1012,6 +1013,17 @@ final class FileExplorerStore: ObservableObject { } } + /// Installs the git-state watcher if a repository appeared after the folder + /// was opened (e.g. `git init`, or adding a worktree/submodule). The main + /// tree watcher excludes `.git`, so its creation isn't observed directly; + /// instead this re-checks on the next working-tree event and starts watching + /// metadata once `.git` exists. A no-op once the watcher is installed or + /// while the folder still isn't a repository. + private func installGitStateWatcherIfNeeded() { + guard gitStateWatcher == nil, let watchPath = directoryWatchPath else { return } + startGitStateWatcher(under: watchPath) + } + /// Maximum bytes read from a non-directory `.git` entry. A real `gitdir:` /// pointer is tiny (`gitdir: `); the cap stops a huge /// workspace-controlled `.git` regular file from hanging the UI or From 666882cd1c77e3127fc0d185752f3fa27abd3dcf Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Wed, 1 Jul 2026 19:28:53 -0700 Subject: [PATCH 15/22] Bundle file-preview open target into a single tuple parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FileExplorerPreviewOpenHandler took three positional arguments (path, lineNumber, columnNumber). Every site that merely constructs the handler ā€” including several pre-existing XCTest suites unrelated to this feature ā€” had to change its closure arity, and the two Int? line/column arguments were positionally swappable. Collapse the handler to a single labeled-tuple parameter (path:lineNumber:columnNumber:). Handler-constructing closures that ignore the value revert to `{ _ in }`, matching origin/main, so the unrelated XCTest files (FileSearchRipgrepParserTests, WindowAndDragTests) are no longer touched by this PR. Co-Authored-By: Claude Opus 4.8 --- Sources/ContentView.swift | 4 ++-- Sources/FileExplorerKeyboardShortcuts.swift | 6 +++--- Sources/FileExplorerView.swift | 4 ++-- Sources/RightSidebarToolPanel.swift | 5 ++++- cmuxTests/FileExplorerShortcutSettingsTests.swift | 2 +- cmuxTests/FileExplorerStoreTests.swift | 10 +++++----- cmuxTests/FileSearchRipgrepParserTests.swift | 2 +- cmuxTests/HiddenRightSidebarContentMountingTests.swift | 2 +- cmuxTests/WindowAndDragTests.swift | 2 +- 9 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 44ab18377473..6ce5da262a90 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -1920,8 +1920,8 @@ struct ContentView: View { onResumeSession: { entry in resumeSession(entry: entry) }, - onOpenFilePreview: { filePath, lineNumber, columnNumber in - openFilePreviewFromSidebar(filePath: filePath, lineNumber: lineNumber, columnNumber: columnNumber) + onOpenFilePreview: { request in + openFilePreviewFromSidebar(filePath: request.path, lineNumber: request.lineNumber, columnNumber: request.columnNumber) }, onOpenAsPane: { mode in openRightSidebarToolPane(mode) diff --git a/Sources/FileExplorerKeyboardShortcuts.swift b/Sources/FileExplorerKeyboardShortcuts.swift index 1e936cad031b..f9250261c015 100644 --- a/Sources/FileExplorerKeyboardShortcuts.swift +++ b/Sources/FileExplorerKeyboardShortcuts.swift @@ -2,7 +2,7 @@ import AppKit import CmuxSettings import CmuxWorkspaces -typealias FileExplorerPreviewOpenHandler = (_ path: String, _ lineNumber: Int?, _ columnNumber: Int?) -> Void +typealias FileExplorerPreviewOpenHandler = ((path: String, lineNumber: Int?, columnNumber: Int?)) -> Void /// Perform the configured action for opening a local file from the file explorer. @MainActor @@ -19,7 +19,7 @@ func performFileExplorerFileOpen( hasPreferredEditorCommand: hasPreferredEditor ) { case .preview: - onOpenFilePreview(path, lineNumber, columnNumber) + onOpenFilePreview((path: path, lineNumber: lineNumber, columnNumber: columnNumber)) case .defaultEditor: FileExternalOpenAction.openDefault(fileURL: URL(fileURLWithPath: path)) case .preferredEditor: @@ -48,7 +48,7 @@ extension FileExplorerPanelView.Coordinator { } guard store.provider is LocalFileExplorerProvider else { - onOpenFilePreview(node.path, nil, nil) + onOpenFilePreview((path: node.path, lineNumber: nil, columnNumber: nil)) return } performFileExplorerFileOpen(path: node.path, onOpenFilePreview: onOpenFilePreview) diff --git a/Sources/FileExplorerView.swift b/Sources/FileExplorerView.swift index cfa7e08e0b55..8f6173c54e67 100644 --- a/Sources/FileExplorerView.swift +++ b/Sources/FileExplorerView.swift @@ -1465,7 +1465,7 @@ final class FileExplorerContainerView: NSView { // Editor/preferred-editor actions operate on local file paths via // NSWorkspace; for non-local providers fall back to the cmux preview. guard coordinator.store.provider is LocalFileExplorerProvider else { - coordinator.onOpenFilePreview(result.path, result.lineNumber, result.columnNumber) + coordinator.onOpenFilePreview((path: result.path, lineNumber: result.lineNumber, columnNumber: result.columnNumber)) return } performFileExplorerFileOpen( @@ -1482,7 +1482,7 @@ final class FileExplorerContainerView: NSView { @objc private func contextMenuOpenSearchResultInCmux(_ sender: NSMenuItem) { guard let result = searchResult(forMenuItem: sender) else { return } - coordinator.onOpenFilePreview(result.path, result.lineNumber, result.columnNumber) + coordinator.onOpenFilePreview((path: result.path, lineNumber: result.lineNumber, columnNumber: result.columnNumber)) } @objc private func contextMenuOpenSearchResultExternally(_ sender: NSMenuItem) { diff --git a/Sources/RightSidebarToolPanel.swift b/Sources/RightSidebarToolPanel.swift index b6dfc8204385..f164d7671b9c 100644 --- a/Sources/RightSidebarToolPanel.swift +++ b/Sources/RightSidebarToolPanel.swift @@ -86,7 +86,10 @@ final class RightSidebarToolPanel: Panel, ObservableObject { } } - func openFilePreview(_ filePath: String, lineNumber: Int?, columnNumber: Int?) { + func openFilePreview(_ request: (path: String, lineNumber: Int?, columnNumber: Int?)) { + let filePath = request.path + let lineNumber = request.lineNumber + let columnNumber = request.columnNumber guard let workspace, let paneId = workspace.bonsplitController.focusedPaneId ?? workspace.bonsplitController.allPaneIds.first else { return diff --git a/cmuxTests/FileExplorerShortcutSettingsTests.swift b/cmuxTests/FileExplorerShortcutSettingsTests.swift index b7cf477f13b0..ea57eb91fa8a 100644 --- a/cmuxTests/FileExplorerShortcutSettingsTests.swift +++ b/cmuxTests/FileExplorerShortcutSettingsTests.swift @@ -365,7 +365,7 @@ private final class ShortcutNoopFileSearchController: FileSearchControlling { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _, _, _ in } + onOpenFilePreview: { _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 987e35b18e0d..a270a3f0a90f 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -919,7 +919,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _, _, _ in } + onOpenFilePreview: { _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, @@ -962,8 +962,8 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { path, lineNumber, columnNumber in - openedRequests.append((path, lineNumber, columnNumber)) + onOpenFilePreview: { request in + openedRequests.append(request) } ) let container = FileExplorerContainerView( @@ -1011,7 +1011,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _, _, _ in } + onOpenFilePreview: { _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, @@ -1068,7 +1068,7 @@ struct FileSearchControllerTests { let coordinator = FileExplorerPanelView.Coordinator( store: store, state: state, - onOpenFilePreview: { _, _, _ in } + onOpenFilePreview: { _ in } ) let container = FileExplorerContainerView( coordinator: coordinator, diff --git a/cmuxTests/FileSearchRipgrepParserTests.swift b/cmuxTests/FileSearchRipgrepParserTests.swift index 5ef4840b136a..ed136ad32f33 100644 --- a/cmuxTests/FileSearchRipgrepParserTests.swift +++ b/cmuxTests/FileSearchRipgrepParserTests.swift @@ -196,7 +196,7 @@ struct FileSearchWorkspaceScopeTests { let store = FileExplorerStore() let state = FileExplorerState() let searchController = SpyFileSearchController() - let coordinator = FileExplorerPanelView.Coordinator(store: store, state: state, onOpenFilePreview: { _, _, _ in }) + let coordinator = FileExplorerPanelView.Coordinator(store: store, state: state, onOpenFilePreview: { _ in }) let container = FileExplorerContainerView(coordinator: coordinator, presentation: .find, searchController: searchController) let rootPath = "/tmp/cmux-find-same-directory-test" diff --git a/cmuxTests/HiddenRightSidebarContentMountingTests.swift b/cmuxTests/HiddenRightSidebarContentMountingTests.swift index 23b43e1312af..5cfd21724214 100644 --- a/cmuxTests/HiddenRightSidebarContentMountingTests.swift +++ b/cmuxTests/HiddenRightSidebarContentMountingTests.swift @@ -71,7 +71,7 @@ struct HiddenRightSidebarContentMountingTests { windowAppearance: .rightSidebarPanelViewTestDefault, workspaceId: nil, onResumeSession: nil, - onOpenFilePreview: { _, _, _ in }, + onOpenFilePreview: { _ in }, onOpenAsPane: { _ in }, onClose: {} ) diff --git a/cmuxTests/WindowAndDragTests.swift b/cmuxTests/WindowAndDragTests.swift index c0d8394da569..8bf7212d82f8 100644 --- a/cmuxTests/WindowAndDragTests.swift +++ b/cmuxTests/WindowAndDragTests.swift @@ -1838,7 +1838,7 @@ final class WindowDragHandleHitTests: XCTestCase { titlebarHeight: 36, windowAppearance: .rightSidebarPanelViewTestDefault, workspaceId: nil, onResumeSession: nil, - onOpenFilePreview: { _, _, _ in }, + onOpenFilePreview: { _ in }, onOpenAsPane: { _ in }, onClose: {} ) From 8d3d56d9bac6392b306be4a976bfdf64ead6ff5c Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Wed, 1 Jul 2026 22:44:00 -0700 Subject: [PATCH 16/22] Harden .git pointer reads and document navigation/exclusion invariants Address review findings on the file-explorer git-watcher and file-preview navigation changes: - Reject non-regular `.git` entries before opening. A repository-controlled `.git` can be a FIFO/socket/device; `FileHandle(forReadingFrom:)` blocks on open() for a FIFO reader (waits for a writer), hanging watcher setup on the main actor before the byte cap applies. Gate on `stat`+S_IFREG, which follows symlinks (matching the existing `fileExists` probe) and never blocks. Add a timeout-raced regression test that fails cleanly rather than wedging CI. - Single-source the FSEvents exclusion-path cap: RecursivePathWatcher now references FileSystemEventStream.maximumExclusionPathCount instead of a duplicate constant, so the two truncation sites can never drift. - Document that CRLF (`\r\n`) is one Swift `Character` for which `isNewline` is true once, so the file-preview line counter advances once per CRLF. Add a pure-function regression test covering CRLF and LF offsets. - Document that text-position navigation applies only to plain-text previews; Markdown/Xcode-project surfaces intentionally ignore it. Refresh the Swift file-length budget for the documented files. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 6 +-- .../FileWatch/FileSystemEventStream.swift | 5 ++- .../FileWatch/RecursivePathWatcher.swift | 4 +- Sources/FileExplorerStore.swift | 15 +++++++ Sources/Panels/FilePreviewPanel.swift | 7 ++- .../FilePreviewWorkspaceOpenSupport.swift | 6 +++ cmuxTests/FileExplorerStoreTests.swift | 45 +++++++++++++++++++ .../FilePreviewTextEditorTextKitTests.swift | 22 +++++++++ 8 files changed, 102 insertions(+), 8 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 7bff0023aaee..b9d72cd28ff2 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -21,7 +21,7 @@ 6077 Sources/TextBoxInput.swift 5915 cmuxTests/TerminalAndGhosttyTests.swift 5573 cmuxTests/BrowserConfigTests.swift -4573 Sources/Panels/FilePreviewPanel.swift +4578 Sources/Panels/FilePreviewPanel.swift 4483 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4264 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift @@ -54,8 +54,8 @@ 1748 Sources/WindowDragHandleView.swift 1732 cmuxTests/WorkspacePullRequestSidebarTests.swift 1687 cmuxTests/MarkdownPanelTests.swift +1686 Sources/FileExplorerStore.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift -1671 Sources/FileExplorerStore.swift 1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1649 cmuxTests/TerminalControllerSocketSecurityTests.swift @@ -67,9 +67,9 @@ 1420 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift +1376 cmuxTests/FileExplorerStoreTests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift 1360 Sources/Feed/FeedButtonStyleDebugWindowController.swift -1331 cmuxTests/FileExplorerStoreTests.swift 1295 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Config/GhosttyConfig.swift 1295 cmuxTests/MobileHostAuthorizationTests.swift 1287 cmuxTests/RestorableAgentSessionIndexTests.swift diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift index 38c451ab0481..2a808e8e6de1 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/FileSystemEventStream.swift @@ -27,7 +27,10 @@ import Foundation /// after `FSEventStreamInvalidate`. No callback ever touches a freed instance, /// so a separately retained context box is unnecessary. final class FileSystemEventStream: @unchecked Sendable { - private static let maximumExclusionPathCount = 8 + /// Upper bound on FSEvents exclusion paths. `FSEventStreamSetExclusionPaths` + /// accepts at most 8 entries and silently ignores the rest, so this stream and + /// its ``RecursivePathWatcher`` owner truncate to this single shared limit. + static let maximumExclusionPathCount = 8 private static let queueSpecificKey = DispatchSpecificKey() private static let queue: DispatchQueue = { let queue = DispatchQueue(label: "com.cmux.recursive-path-watcher", qos: .utility) diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift index e8c0718d0f0a..58afdf7740ce 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/FileWatch/RecursivePathWatcher.swift @@ -32,8 +32,6 @@ import Foundation /// and applies the throttle. The pump's lifetime is the raw stream's: ``stop()`` /// and `deinit` finish it. public actor RecursivePathWatcher { - private static let maximumFSEventExclusionPathCount = 8 - /// The paths this watcher observes, as passed to ``init(paths:clock:)``. /// /// Exposed so callers can compare against a freshly resolved set and skip @@ -81,7 +79,7 @@ public actor RecursivePathWatcher { clock: any FileWatchClock = SystemFileWatchClock() ) { guard !paths.isEmpty else { return nil } - let effectiveExcludedPaths = Array(excludedPaths.prefix(Self.maximumFSEventExclusionPathCount)) + let effectiveExcludedPaths = Array(excludedPaths.prefix(FileSystemEventStream.maximumExclusionPathCount)) self.watchedPaths = paths self.excludedPaths = effectiveExcludedPaths self.clock = clock diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 2b96041a7625..201d1363b8a2 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -1032,7 +1032,22 @@ final class FileExplorerStore: ObservableObject { /// Reads up to ``maximumGitPointerFileBytes`` from the `.git` pointer file at /// `path`, so a malformed or malicious oversized file can't be loaded whole. + /// + /// Only a regular file holds a real `gitdir:` pointer. A repository-controlled + /// `.git` entry can be a FIFO, socket, or device, and opening one for reading + /// can block indefinitely on `open()` (a reader blocks until a FIFO writer + /// appears), hanging watcher setup on the main actor before the byte cap above + /// can ever apply. `stat` follows symlinks ā€” matching the + /// `fileExists(atPath:isDirectory:)` probe in ``gitMetadataDirectory(under:)``, + /// so a legitimate `.git` symlink to a regular pointer file still resolves ā€” + /// and never blocks, so reject anything that is not a regular file up front. private static func readGitPointerFile(at path: String) -> String? { + var info = stat() + guard stat(path, &info) == 0, + (info.st_mode & mode_t(S_IFMT)) == mode_t(S_IFREG) + else { + return nil + } guard let handle = try? FileHandle(forReadingFrom: URL(fileURLWithPath: path)) else { return nil } diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index a9d43202f5e5..c5428979a10e 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1338,9 +1338,14 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan self.pendingTextNavigation = nil } - private static func textLocation(lineNumber: Int, columnNumber: Int, in text: String) -> Int { + static func textLocation(lineNumber: Int, columnNumber: Int, in text: String) -> Int { var index = text.startIndex var currentLine = 1 + // Iterate by `Character` (grapheme cluster), not by Unicode scalar or byte: + // a `\r\n` (CRLF) sequence is a single `Character` whose `isNewline` is true + // exactly once, and `index(after:)` steps over both code units together. So a + // CRLF line ending advances `currentLine` once ā€” never twice ā€” keeping line + // numbers correct for Windows-style files. while currentLine < lineNumber, index < text.endIndex { let character = text[index] index = text.index(after: index) diff --git a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift index b7a128ec3629..e00ca3dc524c 100644 --- a/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift +++ b/Sources/Panels/FilePreviewWorkspaceOpenSupport.swift @@ -17,6 +17,12 @@ extension Workspace { focus: focus, reuseExisting: reuseExisting ) + // Line/column navigation applies only to plain-text file previews. The same + // path may instead route to a Markdown render surface or an Xcode project + // surface (see `openFileSurfaces`), neither of which has a selectable text + // position, so for those the navigation request is intentionally a no-op. + // Routing is covered by `searchNavigationPreservesMarkdownSurfaceRouting` and + // `searchNavigationPreservesXcodeProjectSurfaceRouting`. for panel in openedPanels { (panel as? FilePreviewPanel)?.navigateToTextPosition( lineNumber: lineNumber, diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index a270a3f0a90f..7deeead6db70 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -116,6 +116,15 @@ private final class DeferredListFileExplorerProvider: FileExplorerProvider { // MARK: - Store Tests +/// Result of the timed `gitMetadataDirectory` probe in +/// `testGitMetadataDirectoryRejectsFIFOGitEntry`. The `timedOut` case lets a regression +/// that reintroduces the blocking `open()` fail the test cleanly instead of wedging the +/// whole test process on a FIFO that never gets a writer. +private enum GitMetadataProbeOutcome: Sendable { + case completed(String?) + case timedOut +} + /// The store's `@Published` state is driven by unstructured `Task { ... }` calls that /// hop to `@MainActor`. Pinning the test class to `@MainActor` keeps observations on /// the same actor as the mutations, so reads see a consistent snapshot. @@ -734,6 +743,42 @@ struct FileExplorerStoreTests { try writeGitPointer(padding + "gitdir: \(oversizedTargetDir.path)\n", in: oversizedRoot) #expect(FileExplorerStore.gitMetadataDirectory(under: oversizedRoot.path) == nil) } + + @Test("gitMetadataDirectory rejects a FIFO .git entry without blocking on open()") + func testGitMetadataDirectoryRejectsFIFOGitEntry() async throws { + let fileManager = FileManager.default + let root = fileManager.temporaryDirectory + .appendingPathComponent("cmux-git-fifo-\(UUID().uuidString)", isDirectory: true) + try fileManager.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: root) } + + // A repository-controlled `.git` can be a FIFO. `fileExists` reports it as a + // non-directory, so without the regular-file guard the resolver would open it + // with `FileHandle(forReadingFrom:)`, which blocks on open() until a writer + // appears ā€” hanging watcher setup on the main actor. The guard must reject it. + let gitPath = root.appendingPathComponent(".git").path + try #require(mkfifo(gitPath, 0o600) == 0) + + // Race resolution against a timeout so a regression fails cleanly here rather + // than blocking forever on the FIFO's open(). + let outcome = await withTaskGroup(of: GitMetadataProbeOutcome.self) { group in + group.addTask { .completed(FileExplorerStore.gitMetadataDirectory(under: root.path)) } + group.addTask { + try? await Task.sleep(nanoseconds: 5_000_000_000) + return .timedOut + } + let first = await group.next() ?? .timedOut + group.cancelAll() + return first + } + + switch outcome { + case .completed(let metadata): + #expect(metadata == nil, "A FIFO .git entry must resolve to nil, not be read") + case .timedOut: + Issue.record("gitMetadataDirectory blocked on a FIFO .git entry ā€” regular-file guard missing") + } + } } @MainActor diff --git a/cmuxTests/FilePreviewTextEditorTextKitTests.swift b/cmuxTests/FilePreviewTextEditorTextKitTests.swift index 1b0472c6084b..386426476d54 100644 --- a/cmuxTests/FilePreviewTextEditorTextKitTests.swift +++ b/cmuxTests/FilePreviewTextEditorTextKitTests.swift @@ -76,6 +76,28 @@ struct FilePreviewTextEditorTextKitTests { #expect(textView.selectedRange().location == (content as NSString).range(of: "needle").location) } + @Test("line scan counts a CRLF line ending as a single line break") + func textLocationCountsCRLFAsSingleLineBreak() { + // `\r\n` is one Swift `Character` (grapheme cluster) whose `isNewline` is true + // exactly once, so a CRLF line ending must advance the line counter once, not + // twice. If it were miscounted as two breaks, line 3 would resolve into + // "second" and the offset would be wrong. Pure-function regression guard for + // the file-preview line counter, independent of NSTextView line-ending handling. + let crlf = "first\r\nsecond\r\nabcd needle" + #expect( + FilePreviewPanel.textLocation(lineNumber: 3, columnNumber: 6, in: crlf) + == (crlf as NSString).range(of: "needle").location + ) + + // The same document with LF endings resolves through the identical code path, + // confirming there is no CRLF-specific branch ā€” only the grapheme-cluster step. + let lf = "first\nsecond\nabcd needle" + #expect( + FilePreviewPanel.textLocation(lineNumber: 3, columnNumber: 1, in: lf) + == (lf as NSString).range(of: "abcd").location + ) + } + @Test("search navigation treats columns as UTF-8 byte offsets") func searchNavigationUsesUTF8ByteColumnOffset() async throws { let content = "first\nšŸ” needle\n" From ecfd61e44b6bd029440cb32aafc92dbca356a761 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 2 Jul 2026 02:21:15 -0700 Subject: [PATCH 17/22] Install git-state watcher via a .git-creation bootstrap watcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main recursive tree watcher excludes `.git`, so `git init` in a folder opened before it was a repository writes only under the excluded `.git` and produces no main-watcher event. The git-state watcher's only recovery path, `installGitStateWatcherIfNeeded()`, runs solely from that main-watcher loop, so status badges stayed empty until an unrelated working-tree change fired. Add a short-lived bootstrap watcher on the same root that does NOT exclude `.git`, so `.git`'s creation is observed as an ordinary descendant event. Its handler installs the git-state watcher and then tears the bootstrap watcher down, so it never observes steady-state `.git` churn. It keeps every other high-churn exclusion (node_modules, build, etc.) so it stays cheap while it waits on a folder that may never become a repository. The regression test is deterministic and pure ā€” it asserts the bootstrap watcher's exclusions equal the main watcher's minus `.git` (a regression to reusing the main exclusions would silently reinstate the stale-badge bug). A behavioral test would require live FSEvents timing; the pure-function guard covers the load-bearing mechanism and cannot compile without the fix, so test and fix land together. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 +- Sources/FileExplorerStore.swift | 77 +++++++++++++++++++++++++- cmuxTests/FileExplorerStoreTests.swift | 27 +++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 115a68fe5047..6771a0c5815b 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -58,16 +58,16 @@ 1661 Sources/FileExplorerView.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1649 cmuxTests/TerminalControllerSocketSecurityTests.swift +1631 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift -1556 Sources/FileExplorerStore.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift 1433 Sources/RemoteTmuxControlConnection.swift 1420 cmuxTests/AppDelegateIssue2907RoutingTests.swift +1403 cmuxTests/FileExplorerStoreTests.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift -1376 cmuxTests/FileExplorerStoreTests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift 1360 Sources/Feed/FeedButtonStyleDebugWindowController.swift 1295 Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Config/GhosttyConfig.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 52ca08cd4565..a18263282131 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -765,6 +765,17 @@ final class FileExplorerStore: ObservableObject { private var gitStateWatcher: RecursivePathWatcher? private var gitStateWatchTask: Task? + /// Short-lived bootstrap watcher used only while the opened folder is not yet + /// a Git repository. The main tree watcher excludes `.git`, so `git init` ā€” + /// which writes only under `.git` ā€” produces no main-watcher event, and the + /// git-state watcher would otherwise never be installed until an unrelated + /// working-tree change happened to fire. This watches the same root *without* + /// excluding `.git`, so `.git`'s creation is observed as an ordinary + /// descendant event; it installs the git-state watcher and then tears itself + /// down, so it never observes steady-state `.git` churn. + private var gitCreationWatcher: RecursivePathWatcher? + private var gitCreationWatchTask: Task? + /// Bumped whenever the explorer root or provider changes /// (``invalidateGitStatusRefresh()``). A `git status` fetch captures this at /// launch; a result whose generation no longer matches is dropped, so a @@ -990,6 +1001,7 @@ final class FileExplorerStore: ObservableObject { } } startGitStateWatcher(under: rootPath) + installGitCreationWatcherIfNeeded(under: rootPath) } else { stopDirectoryWatcher() } @@ -1030,6 +1042,47 @@ final class FileExplorerStore: ObservableObject { private func installGitStateWatcherIfNeeded() { guard gitStateWatcher == nil, let watchPath = directoryWatchPath else { return } startGitStateWatcher(under: watchPath) + // Once the repository exists the git-state watcher owns `.git`, so the + // bootstrap creation watcher (whichever code path installed the metadata + // watcher) has done its job and must stop before it observes churn. + if gitStateWatcher != nil { + stopGitCreationWatcher() + } + } + + /// Installs the ``gitCreationWatcher`` bootstrap watcher when the opened + /// folder is not yet a repository, so a later `git init` is observed even + /// though the main tree watcher excludes `.git`. Unlike the main watcher this + /// one does *not* exclude `.git`, so `.git`'s creation reaches its throttle; + /// its handler installs the git-state watcher and then tears the bootstrap + /// watcher down. A no-op once the repository exists (the git-state watcher is + /// already installed) or once a bootstrap watcher is already running. + private func installGitCreationWatcherIfNeeded(under rootPath: String) { + guard gitStateWatcher == nil, gitCreationWatcher == nil else { return } + guard let watcher = RecursivePathWatcher( + paths: [rootPath], + excludedPaths: Self.gitCreationWatcherExcludedPaths(under: rootPath) + ) else { return } + gitCreationWatcher = watcher + let events = watcher.events + gitCreationWatchTask = Task { @MainActor [weak self] in + for await _ in events { + guard let self else { break } + self.installGitStateWatcherIfNeeded() + // `installGitStateWatcherIfNeeded()` stops this watcher once the + // metadata watcher is installed; break so the loop ends promptly + // even before the finished stream drains. + if self.gitStateWatcher != nil { break } + } + } + } + + /// Cancels the bootstrap creation-watch consumer and drops the watcher; its + /// deinit tears down the filesystem stream synchronously. Idempotent. + private func stopGitCreationWatcher() { + gitCreationWatchTask?.cancel() + gitCreationWatchTask = nil + gitCreationWatcher = nil } /// Maximum bytes read from a non-directory `.git` entry. A real `gitdir:` @@ -1146,6 +1199,7 @@ final class FileExplorerStore: ObservableObject { gitStateWatchTask?.cancel() gitStateWatchTask = nil gitStateWatcher = nil + stopGitCreationWatcher() } private func setProvider(_ newProvider: FileExplorerProvider?, reloadIfAvailable: Bool = true) { @@ -1513,7 +1567,7 @@ final class FileExplorerStore: ObservableObject { return trimmed } - private static func recursiveWatcherExcludedPaths(under rootPath: String) -> [String] { + static func recursiveWatcherExcludedPaths(under rootPath: String) -> [String] { let rootURL = URL(fileURLWithPath: rootPath).standardizedFileURL let ignoredDirectoryNames = [ ".build", @@ -1530,6 +1584,26 @@ final class FileExplorerStore: ObservableObject { .sorted() } + /// The root-level `.git` path as the exclusion list represents it, so the two + /// stay in lockstep (same `standardizedFileURL` construction) and the filter + /// in ``gitCreationWatcherExcludedPaths(under:)`` matches exactly. + static func rootLevelGitPath(under rootPath: String) -> String { + URL(fileURLWithPath: rootPath) + .standardizedFileURL + .appendingPathComponent(".git", isDirectory: true) + .path + } + + /// Exclusions for the ``gitCreationWatcher`` bootstrap watcher: the same + /// high-churn directories the main tree watcher ignores, *except* `.git` + /// itself ā€” whose creation is precisely the event this watcher exists to + /// observe. Keeping the other exclusions means it stays cheap while it waits + /// for a folder that may never become a repository. + static func gitCreationWatcherExcludedPaths(under rootPath: String) -> [String] { + let gitPath = rootLevelGitPath(under: rootPath) + return recursiveWatcherExcludedPaths(under: rootPath).filter { $0 != gitPath } + } + private static func remotePreviewCacheURL(displayTarget: String, remotePath: String) -> URL { let cacheRoot = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-remote-file-previews", isDirectory: true) @@ -1552,5 +1626,6 @@ final class FileExplorerStore: ObservableObject { deinit { cancelRemoteHomeResolution() directoryWatchTask?.cancel() + gitCreationWatchTask?.cancel() } } diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 7deeead6db70..399237df0e22 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -779,6 +779,33 @@ struct FileExplorerStoreTests { Issue.record("gitMetadataDirectory blocked on a FIFO .git entry ā€” regular-file guard missing") } } + + @Test("git-creation bootstrap watcher excludes churn dirs but never .git itself") + func testGitCreationWatcherExclusionsOmitDotGit() { + // Any absolute path works ā€” these are pure string builders. Both helpers + // apply the same `standardizedFileURL` normalization, so the `.git` entry + // compares equal without needing the directory to exist on disk. + let root = "/tmp/cmux-git-creation-\(UUID().uuidString)" + let mainExclusions = FileExplorerStore.recursiveWatcherExcludedPaths(under: root) + let creationExclusions = FileExplorerStore.gitCreationWatcherExcludedPaths(under: root) + let gitPath = FileExplorerStore.rootLevelGitPath(under: root) + + // Premise: the main tree watcher excludes `.git`. That exclusion is exactly + // why `git init` (which writes only under `.git`) produces no main-watcher + // event, so the git-state watcher would never install on its own. + #expect(mainExclusions.contains(gitPath)) + + // The bootstrap watcher must NOT exclude `.git`, or it could never observe + // `.git`'s creation ā€” reintroducing the "git init leaves status badges + // stale until an unrelated tree change" regression. If someone reuses + // `recursiveWatcherExcludedPaths` here (which excludes `.git`), this fails. + #expect(!creationExclusions.contains(gitPath)) + + // It keeps every *other* high-churn exclusion so it stays cheap while it + // waits on a folder that may never become a repository. + #expect(Set(creationExclusions) == Set(mainExclusions).subtracting([gitPath])) + #expect(creationExclusions.contains { $0.hasSuffix("/node_modules") }) + } } @MainActor From be8a5a4db8a0382c3735c731b8e80761e7c4aaab Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 2 Jul 2026 02:32:37 -0700 Subject: [PATCH 18/22] Clear pending text-position jump on a nil-line file open `navigateToTextPosition` queues a `pendingTextNavigation` jump that is applied once the text content finishes loading. A nil line number returned early without touching that queued state, so this sequence left a stale jump armed: a search-result open queues a jump for line N while the text is still loading, then the same file is reopened normally (no line target, `reuseExisting: true`) before the load completes. The nil-line open did not cancel the older jump, so when the load finished the reused panel scrolled to the stale search location instead of staying put. Treat a nil line as an explicit no-navigation request and clear `pendingTextNavigation` before returning. Both sidebar open paths (`openFilePreviewFromSidebar`, `RightSidebarToolPanel.openFilePreview`) take an optional line number and reuse the existing preview, so the nil path is reachable in normal use. Adds a deterministic regression test: with no text view attached the queued jump cannot be consumed, so the test asserts a line-carrying open sets the pending jump and a subsequent nil-line open clears it, without depending on async load timing. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- Sources/Panels/FilePreviewPanel.swift | 17 ++++++++++- .../FilePreviewReviewFeedbackTests.swift | 28 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 6771a0c5815b..b31f325a1817 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -21,7 +21,7 @@ 5915 cmuxTests/TerminalAndGhosttyTests.swift 5809 Sources/TextBoxInput.swift 5573 cmuxTests/BrowserConfigTests.swift -4578 Sources/Panels/FilePreviewPanel.swift +4593 Sources/Panels/FilePreviewPanel.swift 4483 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4257 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index c5428979a10e..3ec296851408 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1075,7 +1075,15 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan @discardableResult func navigateToTextPosition(lineNumber: Int?, columnNumber: Int?) -> Task? { - guard let lineNumber else { return nil } + // A nil line is an explicit "open without navigating" request (an ordinary file + // open, as opposed to a search-result open that carries a target line). It must + // supersede any jump still queued from an earlier search-result open whose text + // load has not resolved yet; otherwise that stale jump fires when the load + // completes and scrolls the reused panel to the old search location. + guard let lineNumber else { + pendingTextNavigation = nil + return nil + } pendingTextNavigation = ( lineNumber: max(1, lineNumber), columnNumber: max(1, columnNumber ?? 1) @@ -1089,6 +1097,13 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan return nil } + /// Whether a text-position jump is still queued (set by a line-carrying + /// `navigateToTextPosition` and consumed once the text content loads). Exposed for + /// tests that assert a nil-line open clears an earlier search-result jump. + var hasPendingTextNavigationForTesting: Bool { + pendingTextNavigation != nil + } + func attachPDFPreview(root: NSView, primaryResponder: NSView) { attachPreviewFocus(root: root, primaryResponder: primaryResponder, intent: .pdfCanvas) } diff --git a/cmuxTests/FilePreviewReviewFeedbackTests.swift b/cmuxTests/FilePreviewReviewFeedbackTests.swift index 2e0bcfe85761..884214b1c5da 100644 --- a/cmuxTests/FilePreviewReviewFeedbackTests.swift +++ b/cmuxTests/FilePreviewReviewFeedbackTests.swift @@ -365,6 +365,34 @@ final class FilePreviewReviewFeedbackTests: XCTestCase { XCTAssertEqual(workspace.bonsplitController.tabs(inPane: targetPane).count, startingTargetTabs + 1) } + // Regression: a search-result open queues a text-position jump; if the same file is + // then reopened normally (no line target) before the text finishes loading, the stale + // jump must be discarded rather than firing when the load completes and scrolling the + // reused panel to the old search location. + func testNilLineOpenClearsPendingSearchNavigation() throws { + let url = try temporaryTextFile(contents: "l1\nl2\nl3\nl4\nl5\n", encoding: .utf8) + defer { try? FileManager.default.removeItem(at: url) } + + let panel = FilePreviewPanel(workspaceId: UUID(), filePath: url.path) + defer { panel.close() } + + // Queue a search-result jump. With no text view attached the jump cannot be + // applied yet, so it stays pending exactly as it would while the file is loading. + panel.navigateToTextPosition(lineNumber: 3, columnNumber: 1) + XCTAssertTrue( + panel.hasPendingTextNavigationForTesting, + "A line-carrying open should queue a pending text-position jump" + ) + + // Reopening without a line target is an explicit no-navigation request and must + // discard the queued jump. + panel.navigateToTextPosition(lineNumber: nil, columnNumber: nil) + XCTAssertFalse( + panel.hasPendingTextNavigationForTesting, + "A nil-line open must clear a previously queued search-result jump" + ) + } + private func temporaryTextFile(contents: String, encoding: String.Encoding) throws -> URL { let url = FileManager.default.temporaryDirectory .appendingPathComponent(UUID().uuidString) From 550df11cfbae5782ad12923b090b8032cb7b7d85 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 2 Jul 2026 02:50:32 -0700 Subject: [PATCH 19/22] Refresh git status when the git-state watcher installs post-open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The git-state watcher started by `startGitStateWatcher` only calls `refreshGitStatus()` on *subsequent* `.git` events. For a repository that appears after the folder is opened (`git init`), the bootstrap creation watcher installs the git-state watcher only after `.git` already exists ā€” its initial metadata writes are finished, and no further `.git` event is guaranteed. Status badges therefore stayed empty until some unrelated working-tree or metadata change happened to fire the watcher. Trigger an eager `refreshGitStatus()` the moment `installGitStateWatcherIfNeeded()` successfully installs the watcher, mirroring the eager refresh the already-a-repo path gets from `setRootPath`. Both watcher entrypoints (main tree watcher and bootstrap creation watcher) share this path; `refreshGitStatus()` coalesces, so the main-watcher entrypoint's redundant call (it refreshes immediately before) collapses into a single `git status` process. Adds a deterministic regression test that drives the shared install path against a freshly `git init`-ed fixture and asserts the install kicks off a status refresh. `refreshGitStatus()` sets its in-flight flag synchronously before dispatching, and the MainActor test never yields before asserting, so the check is race-free. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 +-- Sources/FileExplorerStore.swift | 28 ++++++++++++++++ cmuxTests/FileExplorerStoreTests.swift | 44 ++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index b31f325a1817..5a209989e38a 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -56,16 +56,16 @@ 1687 cmuxTests/MarkdownPanelTests.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift 1661 Sources/FileExplorerView.swift +1659 Sources/FileExplorerStore.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1649 cmuxTests/TerminalControllerSocketSecurityTests.swift -1631 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift +1447 cmuxTests/FileExplorerStoreTests.swift 1433 Sources/RemoteTmuxControlConnection.swift 1420 cmuxTests/AppDelegateIssue2907RoutingTests.swift -1403 cmuxTests/FileExplorerStoreTests.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift 1380 cmuxUITests/MenuKeyEquivalentRoutingUITests.swift 1363 Sources/CMUXInstalledExtensionSidebarHostView.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index a18263282131..50b6ef52faeb 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -1047,6 +1047,15 @@ final class FileExplorerStore: ObservableObject { // watcher) has done its job and must stop before it observes churn. if gitStateWatcher != nil { stopGitCreationWatcher() + // The git-state watcher only refreshes on *subsequent* `.git` changes. + // A repository that appeared after the folder was opened (`git init`) + // has already finished writing its initial metadata by the time this + // installs, so without an eager refresh the badges would stay empty + // until some unrelated change fired. The already-a-repo path gets the + // same eager refresh from `setRootPath`; `refreshGitStatus()` coalesces, + // so the redundant call from the main-watcher entrypoint (which just + // refreshed) collapses into a single `git status` process. + refreshGitStatus() } } @@ -1218,6 +1227,25 @@ final class FileExplorerStore: ObservableObject { func setProviderForTesting(_ newProvider: FileExplorerProvider?, reloadIfAvailable: Bool = true) { setProvider(newProvider, reloadIfAvailable: reloadIfAvailable) } + + /// Drives the shared "a repository appeared after the folder was opened" path: + /// points the store at `rootPath` as the watched directory (without starting the + /// FSEvents watchers) and runs the metadata-watcher install path exactly as the + /// bootstrap creation watcher would. Returns whether the git-state watcher was + /// installed, so tests can assert the install also kicks off a status refresh. + @discardableResult + func simulateGitRepositoryAppearedForTesting(rootPath: String) -> Bool { + self.rootPath = rootPath + directoryWatchPath = rootPath + installGitStateWatcherIfNeeded() + return gitStateWatcher != nil + } + + var isGitStatusRefreshInFlightForTesting: Bool { isGitStatusRefreshInFlight } + + func stopWatchersForTesting() { + stopDirectoryWatcher() + } #endif func reload() { diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 399237df0e22..1134f551ecb6 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -806,6 +806,50 @@ struct FileExplorerStoreTests { #expect(Set(creationExclusions) == Set(mainExclusions).subtracting([gitPath])) #expect(creationExclusions.contains { $0.hasSuffix("/node_modules") }) } + + // A folder can be opened before it becomes a Git repository; when `git init` + // later creates `.git`, the bootstrap watcher installs the git-state watcher. + // That watcher only refreshes on *subsequent* `.git` changes, so installing it + // must itself kick off a status refresh ā€” otherwise the badges stay empty until + // an unrelated change fires (the exact git-init-after-open regression). + @Test(.enabled(if: FileExplorerStoreTests.hasGit(), "git is required to create a repository fixture")) + func testGitStateWatcherInstallTriggersStatusRefresh() throws { + let repo = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-git-refresh-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: repo, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: repo) } + try Self.runGit(["init"], in: repo) + + let store = FileExplorerStore() + store.setProviderForTesting(LocalFileExplorerProvider(), reloadIfAvailable: false) + defer { store.stopWatchersForTesting() } + + // No refresh has been requested yet, so any in-flight refresh observed after + // the install must have been triggered by the install path itself. + #expect(!store.isGitStatusRefreshInFlightForTesting) + + let installed = store.simulateGitRepositoryAppearedForTesting(rootPath: repo.path) + #expect(installed, "git-state watcher should install once .git exists") + #expect( + store.isGitStatusRefreshInFlightForTesting, + "installing the git-state watcher for a repo that appeared after open must eagerly refresh git status" + ) + } + + private nonisolated static func hasGit() -> Bool { + FileManager.default.isExecutableFile(atPath: "/usr/bin/git") + } + + private nonisolated static func runGit(_ arguments: [String], in directory: URL) throws { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/git") + process.arguments = arguments + process.currentDirectoryURL = directory + process.standardOutput = FileHandle.nullDevice + process.standardError = FileHandle.nullDevice + try process.run() + process.waitUntilExit() + } } @MainActor From 486cad33a327973e7df78b8ad41841aa0fd9ad30 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 2 Jul 2026 03:09:44 -0700 Subject: [PATCH 20/22] Make FIFO .git regression test fail cleanly instead of hanging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FIFO probe raced gitMetadataDirectory() against a timeout inside a withTaskGroup. Structured concurrency will not let the group scope return until every child finishes, and cancelAll() cannot interrupt a synchronous open(), so if the blocking-open regression returned, the probe child would wedge the group forever ā€” the test would hang CI instead of reaching the .timedOut assertion, leaving the main-actor-hang regression without a clean red signal. Run the probe on a detached OS thread raced against a timeout through a single-resume checked continuation. On timeout the continuation settles and the test records the failure while the wedged thread is simply abandoned (parked in open(), leaked, harmless ā€” the test has already failed and the process is torn down). The passing path is unchanged: the regular-file guard rejects the FIFO without opening, so the probe resumes .completed(nil) at once. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 2 +- cmuxTests/FileExplorerStoreTests.swift | 47 ++++++++++++++++++++------ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index 5a209989e38a..bcbfac00e20e 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -63,7 +63,7 @@ 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1447 cmuxTests/FileExplorerStoreTests.swift +1474 cmuxTests/FileExplorerStoreTests.swift 1433 Sources/RemoteTmuxControlConnection.swift 1420 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index 1134f551ecb6..ed1362f09564 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -125,6 +125,28 @@ private enum GitMetadataProbeOutcome: Sendable { case timedOut } +/// Guards the single legal `resume` of a checked continuation shared by a detached +/// probe thread and a timeout that race to settle it. Whichever fires first wins; the +/// loser's `resume` is dropped. `@unchecked Sendable` because `settled` is only ever +/// read or written under `lock`, and the stored continuation is resumed exactly once. +private final class SingleResume: @unchecked Sendable { + private let lock = NSLock() + private var settled = false + private let continuation: CheckedContinuation + + init(_ continuation: CheckedContinuation) { + self.continuation = continuation + } + + func resume(_ value: Value) { + lock.lock() + defer { lock.unlock() } + guard !settled else { return } + settled = true + continuation.resume(returning: value) + } +} + /// The store's `@Published` state is driven by unstructured `Task { ... }` calls that /// hop to `@MainActor`. Pinning the test class to `@MainActor` keeps observations on /// the same actor as the mutations, so reads see a consistent snapshot. @@ -759,17 +781,22 @@ struct FileExplorerStoreTests { let gitPath = root.appendingPathComponent(".git").path try #require(mkfifo(gitPath, 0o600) == 0) - // Race resolution against a timeout so a regression fails cleanly here rather - // than blocking forever on the FIFO's open(). - let outcome = await withTaskGroup(of: GitMetadataProbeOutcome.self) { group in - group.addTask { .completed(FileExplorerStore.gitMetadataDirectory(under: root.path)) } - group.addTask { - try? await Task.sleep(nanoseconds: 5_000_000_000) - return .timedOut + // Race the resolver against a timeout so a regression fails cleanly here rather + // than blocking forever on the FIFO's open(). The probe runs on a *detached* + // thread, not a structured child: `withTaskGroup` cannot return from its scope + // until every child finishes, and `cancelAll()` cannot interrupt a synchronous + // `open()`, so a regressed blocking open would wedge the group (and CI) instead + // of reaching the `.timedOut` assertion below. A detached thread can simply be + // abandoned ā€” on timeout it stays parked in `open()` and is leaked, which is + // harmless because the test has already failed and the process is torn down. + let outcome: GitMetadataProbeOutcome = await withCheckedContinuation { continuation in + let race = SingleResume(continuation) + Thread.detachNewThread { + race.resume(.completed(FileExplorerStore.gitMetadataDirectory(under: root.path))) + } + DispatchQueue.global().asyncAfter(deadline: .now() + 5) { + race.resume(.timedOut) } - let first = await group.next() ?? .timedOut - group.cancelAll() - return first } switch outcome { From 0b8b9e650a2a577d43393496ee0386bb715355e2 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Thu, 2 Jul 2026 03:27:36 -0700 Subject: [PATCH 21/22] Reach git-state watcher internals via @testable, not DEBUG test seams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FIFO-hang regression test drove FileExplorerStore through three DEBUG-only accessors (simulateGitRepositoryAppearedForTesting, isGitStatusRefreshInFlightForTesting, stopWatchersForTesting). Per the Aziz test/debug-seam policy, test-only accessors do not belong in production Sources/. Remove them and widen the five members the test drives (directoryWatchPath, gitStateWatcher, isGitStatusRefreshInFlight, installGitStateWatcherIfNeeded, stopDirectoryWatcher) from private to internal, reaching them from the test via the existing @testable import ā€” the same resolution applied in #6452. The test now points the store at the repo (rootPath/directoryWatchPath) and calls installGitStateWatcherIfNeeded() directly, asserting the git-state watcher installs and eagerly kicks off a status refresh. Behavior is unchanged; the grandfathered setProviderForTesting stays. Co-Authored-By: Claude Opus 4.8 --- .github/swift-file-length-budget.tsv | 4 ++-- Sources/FileExplorerStore.swift | 29 +++++--------------------- cmuxTests/FileExplorerStoreTests.swift | 21 ++++++++++++------- 3 files changed, 21 insertions(+), 33 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index bcbfac00e20e..e49d0c6ff301 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -56,14 +56,14 @@ 1687 cmuxTests/MarkdownPanelTests.swift 1680 cmuxUITests/BrowserPaneNavigationKeybindUITests.swift 1661 Sources/FileExplorerView.swift -1659 Sources/FileExplorerStore.swift 1652 cmuxTests/CMUXCLIErrorOutputRegressionTests.swift 1649 cmuxTests/TerminalControllerSocketSecurityTests.swift +1640 Sources/FileExplorerStore.swift 1581 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift 1560 cmuxTests/TextBoxMentionCompletionTests.swift 1500 cmuxUITests/MultiWindowNotificationsUITests.swift 1499 cmuxTests/OmnibarAndToolsTests.swift -1474 cmuxTests/FileExplorerStoreTests.swift +1481 cmuxTests/FileExplorerStoreTests.swift 1433 Sources/RemoteTmuxControlConnection.swift 1420 cmuxTests/AppDelegateIssue2907RoutingTests.swift 1384 cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift diff --git a/Sources/FileExplorerStore.swift b/Sources/FileExplorerStore.swift index 50b6ef52faeb..b5f2bd22e5af 100644 --- a/Sources/FileExplorerStore.swift +++ b/Sources/FileExplorerStore.swift @@ -755,14 +755,14 @@ final class FileExplorerStore: ObservableObject { /// Watches the root directory recursively for filesystem changes (local only). private var directoryWatcher: RecursivePathWatcher? private var directoryWatchTask: Task? - private var directoryWatchPath: String? + var directoryWatchPath: String? /// Watches the repository's `.git` metadata so status badges refresh after /// metadata-only changes (`git commit`, `git add`, `git reset`) that never /// touch the working tree ā€” and so are excluded from the main tree watcher. /// Kept separate because those events must refresh git status without /// triggering a tree `reload()`. - private var gitStateWatcher: RecursivePathWatcher? + var gitStateWatcher: RecursivePathWatcher? private var gitStateWatchTask: Task? /// Short-lived bootstrap watcher used only while the opened folder is not yet @@ -788,7 +788,7 @@ final class FileExplorerStore: ObservableObject { /// sustained filesystem storm; without this guard each request would spawn /// another uncancelled `git status` process, letting slow fetches on a large /// repo pile up. Only read/written on the main actor. - private var isGitStatusRefreshInFlight = false + var isGitStatusRefreshInFlight = false /// Set when a refresh is requested while one is already in flight, so exactly /// one follow-up runs after the current fetch finishes (coalescing a burst of /// requests into a single trailing refresh rather than N concurrent ones). @@ -1039,7 +1039,7 @@ final class FileExplorerStore: ObservableObject { /// instead this re-checks on the next working-tree event and starts watching /// metadata once `.git` exists. A no-op once the watcher is installed or /// while the folder still isn't a repository. - private func installGitStateWatcherIfNeeded() { + func installGitStateWatcherIfNeeded() { guard gitStateWatcher == nil, let watchPath = directoryWatchPath else { return } startGitStateWatcher(under: watchPath) // Once the repository exists the git-state watcher owns `.git`, so the @@ -1200,7 +1200,7 @@ final class FileExplorerStore: ObservableObject { /// Cancels the directory-watch consumers and drops the watchers; each /// watcher's deinit tears down its filesystem stream synchronously. - private func stopDirectoryWatcher() { + func stopDirectoryWatcher() { directoryWatchTask?.cancel() directoryWatchTask = nil directoryWatcher = nil @@ -1227,25 +1227,6 @@ final class FileExplorerStore: ObservableObject { func setProviderForTesting(_ newProvider: FileExplorerProvider?, reloadIfAvailable: Bool = true) { setProvider(newProvider, reloadIfAvailable: reloadIfAvailable) } - - /// Drives the shared "a repository appeared after the folder was opened" path: - /// points the store at `rootPath` as the watched directory (without starting the - /// FSEvents watchers) and runs the metadata-watcher install path exactly as the - /// bootstrap creation watcher would. Returns whether the git-state watcher was - /// installed, so tests can assert the install also kicks off a status refresh. - @discardableResult - func simulateGitRepositoryAppearedForTesting(rootPath: String) -> Bool { - self.rootPath = rootPath - directoryWatchPath = rootPath - installGitStateWatcherIfNeeded() - return gitStateWatcher != nil - } - - var isGitStatusRefreshInFlightForTesting: Bool { isGitStatusRefreshInFlight } - - func stopWatchersForTesting() { - stopDirectoryWatcher() - } #endif func reload() { diff --git a/cmuxTests/FileExplorerStoreTests.swift b/cmuxTests/FileExplorerStoreTests.swift index ed1362f09564..4baefcc9a496 100644 --- a/cmuxTests/FileExplorerStoreTests.swift +++ b/cmuxTests/FileExplorerStoreTests.swift @@ -848,17 +848,24 @@ struct FileExplorerStoreTests { try Self.runGit(["init"], in: repo) let store = FileExplorerStore() - store.setProviderForTesting(LocalFileExplorerProvider(), reloadIfAvailable: false) - defer { store.stopWatchersForTesting() } + defer { store.stopDirectoryWatcher() } // No refresh has been requested yet, so any in-flight refresh observed after // the install must have been triggered by the install path itself. - #expect(!store.isGitStatusRefreshInFlightForTesting) - - let installed = store.simulateGitRepositoryAppearedForTesting(rootPath: repo.path) - #expect(installed, "git-state watcher should install once .git exists") + #expect(!store.isGitStatusRefreshInFlight) + + // Drive the shared "a repository appeared after the folder was opened" path + // directly (reaching internal state via `@testable import`): point the store + // at the repo as its watched directory without starting real FSEvents + // watchers, then run the metadata-watcher install path exactly as the + // bootstrap creation watcher would once `.git` shows up. + store.rootPath = repo.path + store.directoryWatchPath = repo.path + store.installGitStateWatcherIfNeeded() + + #expect(store.gitStateWatcher != nil, "git-state watcher should install once .git exists") #expect( - store.isGitStatusRefreshInFlightForTesting, + store.isGitStatusRefreshInFlight, "installing the git-state watcher for a repo that appeared after open must eagerly refresh git status" ) } From 99f168cd1279222b6048b3cacb7a7fa9cc3e9459 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Fri, 3 Jul 2026 16:45:05 -0700 Subject: [PATCH 22/22] Fix file preview text attach merge conflict --- .github/swift-file-length-budget.tsv | 2 +- Sources/Panels/FilePreviewPanel.swift | 8 +------- Sources/Panels/FilePreviewTextEditor.swift | 1 + 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index e00c446b18b2..0a44233d5d5e 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -21,7 +21,7 @@ 5915 cmuxTests/TerminalAndGhosttyTests.swift 5809 Sources/TextBoxInput.swift 5573 cmuxTests/BrowserConfigTests.swift -4593 Sources/Panels/FilePreviewPanel.swift +4587 Sources/Panels/FilePreviewPanel.swift 4483 Sources/cmuxApp.swift 4367 cmuxTests/BrowserPanelTests.swift 4239 Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift diff --git a/Sources/Panels/FilePreviewPanel.swift b/Sources/Panels/FilePreviewPanel.swift index c783484af32e..22560609bac5 100644 --- a/Sources/Panels/FilePreviewPanel.swift +++ b/Sources/Panels/FilePreviewPanel.swift @@ -1053,12 +1053,6 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan focusFlashToken += 1 } - func attachTextView(_ textView: NSTextView) { - self.textView = textView - focusCoordinator.register(root: textView, primaryResponder: textView, intent: .textEditor) - applyPendingTextNavigationIfReady() - } - func handleDroppedFileURLsAsText(_ urls: [URL]) -> Bool { guard previewMode == .text, let textView else { return false } let text = TerminalImageTransferPlanner.insertedText(forFileURLs: urls) @@ -1329,7 +1323,7 @@ final class FilePreviewPanel: Panel, ObservableObject, FilePreviewTextEditingPan } } - private func applyPendingTextNavigationIfReady() { + func applyPendingTextNavigationIfReady() { guard previewMode == .text, hasLoadedTextContent, let textView, diff --git a/Sources/Panels/FilePreviewTextEditor.swift b/Sources/Panels/FilePreviewTextEditor.swift index f24f6ffa0c01..5f5e43cfc7a2 100644 --- a/Sources/Panels/FilePreviewTextEditor.swift +++ b/Sources/Panels/FilePreviewTextEditor.swift @@ -429,6 +429,7 @@ extension FilePreviewPanel { func attachTextView(_ textView: NSTextView) { self.textView = textView focusCoordinator.register(root: textView, primaryResponder: textView, intent: .textEditor) + applyPendingTextNavigationIfReady() } @discardableResult