From 5299f3bb07478403367c11fe915573b58857b0a4 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:01:50 -0800 Subject: [PATCH 01/12] fix(trash): verify fresh file identity A URL retained from a scan can cache its resource identifier. Read the current identifier from a fresh URL before allowing the move. --- Radix/Services/SystemIntegration.swift | 3 +- RadixCoreTests/SystemIntegrationTests.swift | 48 +++++++++++++-------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/Radix/Services/SystemIntegration.swift b/Radix/Services/SystemIntegration.swift index a3203a1..c42078f 100644 --- a/Radix/Services/SystemIntegration.swift +++ b/Radix/Services/SystemIntegration.swift @@ -515,8 +515,9 @@ enum SystemIntegration { } private nonisolated static func currentResourceIdentity(for url: URL) -> Result { + let currentURL = URL(filePath: url.path) do { - let values = try url.resourceValues(forKeys: [.fileResourceIdentifierKey]) + let values = try currentURL.resourceValues(forKeys: [.fileResourceIdentifierKey]) guard let identifierData = values.fileResourceIdentifier as? Data else { return .failure(.metadataUnavailable(String(localized: "file resource identifier unavailable", comment: "Reason the selected file identity could not be verified."))) } diff --git a/RadixCoreTests/SystemIntegrationTests.swift b/RadixCoreTests/SystemIntegrationTests.swift index d41359e..7f01877 100644 --- a/RadixCoreTests/SystemIntegrationTests.swift +++ b/RadixCoreTests/SystemIntegrationTests.swift @@ -374,26 +374,34 @@ struct SystemIntegrationTests { } } - @Test - func testIdentityBoundTrashMoveDoesNotMutateReplacementAtScannedPath() throws { - let node = trashTestNode() - var operationOrder: [String] = [] + @Test(arguments: [false, true]) + func testIdentityBoundTrashMoveUsesCurrentResourceIdentity(replaced: Bool) throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let itemURL = fixture.appending(path: "marked.txt") + try Data([1]).write(to: itemURL) + let scannedURLs = try FileManager.default.contentsOfDirectory( + at: fixture, + includingPropertiesForKeys: [.fileResourceIdentifierKey] + ) + let scannedURL = try #require(scannedURLs.first) + let scannedIdentity = try #require(ScanMetadataLoader().metadata(for: scannedURL).fileIdentity) + try #require(!scannedIdentity.isFileSystemIdentity) + let node = trashTestNode(url: scannedURL, fileIdentity: scannedIdentity) + if replaced { + try FileManager.default.moveItem(at: itemURL, to: fixture.appending(path: "original.txt")) + try Data([2]).write(to: itemURL) + } + var trashedURLs: [URL] = [] let result = try SystemIntegration.moveToTrash( node, - identityVerifier: { verifiedNode in - operationOrder.append("verify:\(verifiedNode.id)") - // Models the path having been replaced after the user confirmed - // the request but before the consolidated mutation begins. - return .mismatch - }, - trashItem: { _ in - operationOrder.append("trash") - } + identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { trashedURLs.append($0) } ) - #expect(result == .mismatch) - #expect(operationOrder == ["verify:\(node.id)"]) + #expect(result == (replaced ? .mismatch : .matches)) + #expect(trashedURLs == (replaced ? [] : [scannedURL])) } @Test @@ -416,9 +424,11 @@ struct SystemIntegrationTests { #expect(operationOrder == ["verify", "trash:\(node.url.path)"]) } - private func trashTestNode() -> FileNodeRecord { - let url = URL(filePath: "/tmp/radix-trash-test.txt", directoryHint: .notDirectory) - return FileNodeRecord( + private func trashTestNode( + url: URL = URL(filePath: "/tmp/radix-trash-test.txt", directoryHint: .notDirectory), + fileIdentity: FileIdentity = FileIdentity(device: 1, inode: 2) + ) -> FileNodeRecord { + FileNodeRecord( id: url.path, url: url, name: url.lastPathComponent, @@ -428,7 +438,7 @@ struct SystemIntegrationTests { logicalSize: 1, descendantFileCount: 1, lastModified: nil, - fileIdentity: FileIdentity(device: 1, inode: 2), + fileIdentity: fileIdentity, isPackage: false, isAccessible: true, isSelfAccessible: true, From ef2bd57ba873f0ce6a3c59d15ef2a4a05e0230c3 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:01:59 -0800 Subject: [PATCH 02/12] feat(discard-pile): share marks across scans Keep the cleanup list available when the active scan changes. Review one verified batch across locations and keep marks that did not move. Refs #108 --- Radix/ContentView.swift | 16 +- .../DiscardPile/DiscardPileReviewSheet.swift | 93 ++- Radix/Features/Sidebar/SidebarView.swift | 7 +- Radix/Localizable.xcstrings | 329 ++++++++++ Radix/Models/FileNodeActions.swift | 11 - Radix/Services/AppSystemActions.swift | 2 +- Radix/Services/ScanCoordinator.swift | 28 +- Radix/ViewModels/AppModel.swift | 526 +++++++-------- .../AppPresentationCoordinator.swift | 5 + .../SidebarScanCacheController.swift | 40 +- Radix/ViewModels/TrashFlowController.swift | 292 ++++++--- RadixCoreTests/AppModelDependencyTests.swift | 166 +++-- .../AppModelFolderRefreshTests.swift | 6 +- RadixCoreTests/ScanCoordinatorTests.swift | 604 +++++++++++++++++- RadixCoreTests/ScanModelTests.swift | 11 - .../SidebarScanCacheControllerTests.swift | 52 ++ RadixCoreTests/TrashFlowControllerTests.swift | 138 +++- 17 files changed, 1855 insertions(+), 471 deletions(-) diff --git a/Radix/ContentView.swift b/Radix/ContentView.swift index 020881a..72ded81 100644 --- a/Radix/ContentView.swift +++ b/Radix/ContentView.swift @@ -188,6 +188,8 @@ struct ContentView: View { case .discardPileReview: DiscardPileReviewSheet( snapshot: discardPileSnapshot, + canMoveToTrash: appModel.canMoveDiscardPileToTrash, + isPreparingTrash: appModel.isPreparingDiscardPileTrash, actions: DiscardPileReviewActions( removeNodes: { nodeIDs in appModel.removeDiscardPileNodes(ids: nodeIDs) @@ -199,13 +201,23 @@ struct ContentView: View { appModel.dismissDiscardPileReview() }, moveToTrash: { - if appModel.requestMoveDiscardPileToTrash() { - appModel.dismissDiscardPileReview() + Task { @MainActor in + if await appModel.requestMoveDiscardPileToTrash() { + appModel.dismissDiscardPileReview() + } } } ) ) .environmentObject(appModel.workspaceTour) + case .discardPileTrashConfirmation: + if let selection = appModel.pendingTrashSelection { + DiscardPileTrashConfirmationSheet( + selection: selection, + onCancel: { appModel.cancelPendingTrash() }, + onConfirm: { appModel.confirmMovePendingSelectionToTrash() } + ) + } case .importPreview: if let preview = appModel.pendingImportPreview { ImportSnapshotPreviewSheet( diff --git a/Radix/Features/DiscardPile/DiscardPileReviewSheet.swift b/Radix/Features/DiscardPile/DiscardPileReviewSheet.swift index 5fc8c68..ddb88c6 100644 --- a/Radix/Features/DiscardPile/DiscardPileReviewSheet.swift +++ b/Radix/Features/DiscardPile/DiscardPileReviewSheet.swift @@ -8,21 +8,29 @@ struct DiscardPileReviewActions { } struct DiscardPileReviewSheet: View { - @Environment(\.isReadOnlyMode) private var isReadOnlyMode @EnvironmentObject private var tour: WorkspaceTourController private let rows: [DiscardPileReviewRow] private let rowIDs: Set private let summary: DiscardPileSummary + private let canMoveToTrash: Bool + private let isPreparingTrash: Bool private let actions: DiscardPileReviewActions @State private var selection: Set = [] @State private var isConfirmingClear = false - init(snapshot: DiscardPileSnapshot, actions: DiscardPileReviewActions) { + init( + snapshot: DiscardPileSnapshot, + canMoveToTrash: Bool, + isPreparingTrash: Bool, + actions: DiscardPileReviewActions + ) { let rows = snapshot.nodes.map(DiscardPileReviewRow.init) self.rows = rows self.rowIDs = Set(rows.map(\.id)) self.summary = snapshot.summary + self.canMoveToTrash = canMoveToTrash + self.isPreparingTrash = isPreparingTrash self.actions = actions } @@ -53,7 +61,7 @@ struct DiscardPileReviewSheet: View { Button("Clear All…") { isConfirmingClear = true } - .disabled(rows.isEmpty) + .disabled(summary.isEmpty) Spacer() @@ -62,11 +70,17 @@ struct DiscardPileReviewSheet: View { } .keyboardShortcut(.cancelAction) - Button(moveButtonTitle, role: .destructive) { + if isPreparingTrash { + ProgressView() + .controlSize(.small) + .accessibilityLabel(Text("Checking Marked Items")) + } + + Button("Move to Trash…") { actions.moveToTrash() } .keyboardShortcut(showsTourGuidance ? nil : .defaultAction) - .disabled(rows.isEmpty || isReadOnlyMode) + .disabled(!canMoveToTrash) } } .padding(20) @@ -122,7 +136,7 @@ struct DiscardPileReviewSheet: View { Divider() HStack(spacing: 4) { - Text(summaryText) + Text(summary.markedItemsText) if selectedRowCount > 0 { Text(verbatim: "•") @@ -186,11 +200,6 @@ struct DiscardPileReviewSheet: View { .padding(.vertical, 4) } - private var summaryText: String { - let size = RadixFormatters.size(summary.totalAllocatedSize) - return String(localized: "\(summary.itemCount) items • \(size)", comment: "Discard Pile status showing the marked item count and total allocated size. The item count controls pluralization.") - } - private var showsTourGuidance: Bool { tour.step == .removeMark || tour.step == .finished } @@ -210,9 +219,67 @@ struct DiscardPileReviewSheet: View { selection.subtract(removedIDs) actions.removeNodes(removedIDs) } +} + +struct DiscardPileTrashConfirmationSheet: View { + let selection: TrashFlowController.PendingTrashSelection + let onCancel: () -> Void + let onConfirm: () -> Void - private var moveButtonTitle: String { - String(localized: "Move \(summary.itemCount) Items to Trash", comment: "Destructive action for moving marked items to the Trash.") + var body: some View { + let hasItemsToMove = !selection.nodes.isEmpty + let title: LocalizedStringKey = hasItemsToMove ? "Move to Trash?" : "No Items Can Be Moved" + let cancelTitle: LocalizedStringKey = hasItemsToMove ? "Cancel" : "Close" + + VStack(alignment: .leading, spacing: 14) { + Text(title) + .font(.title3.weight(.semibold)) + .accessibilityAddTraits(.isHeader) + + List { + if hasItemsToMove { + Section("Items to Move") { + ForEach(selection.nodes) { node in + Text(verbatim: node.url.path) + .fixedSize(horizontal: false, vertical: true) + } + } + } + + if let unavailable = selection.discardPileBatch?.unavailable, !unavailable.isEmpty { + Section("Items That Will Stay Marked") { + ForEach(unavailable, id: \.node.id) { item in + VStack(alignment: .leading, spacing: 3) { + Text(verbatim: item.node.url.path) + Text(verbatim: item.message) + .font(.caption) + .foregroundStyle(.secondary) + } + .fixedSize(horizontal: false, vertical: true) + } + } + } + } + .listStyle(.inset) + .textSelection(.enabled) + + HStack { + Spacer() + + Button(cancelTitle, role: .cancel, action: onCancel) + .keyboardShortcut(.cancelAction) + + if hasItemsToMove { + Button( + String(localized: "Move \(selection.nodes.count) Items to Trash", comment: "Destructive action for moving marked items to the Trash."), + role: .destructive, + action: onConfirm + ) + } + } + } + .padding(20) + .frame(width: 640, height: 360) } } diff --git a/Radix/Features/Sidebar/SidebarView.swift b/Radix/Features/Sidebar/SidebarView.swift index 5805f66..e0a2c26 100644 --- a/Radix/Features/Sidebar/SidebarView.swift +++ b/Radix/Features/Sidebar/SidebarView.swift @@ -61,13 +61,13 @@ struct SidebarView: View { } .listStyle(.sidebar) - if scanState.snapshot != nil { + if scanState.snapshot != nil || !discardPileSummary.isEmpty { Divider() DiscardPileSidebarButton( summary: discardPileSummary, isDropHintActive: discardPileDragIsActive && !isReadOnlyMode, - allowsDrop: !isReadOnlyMode, + allowsDrop: scanState.snapshot != nil && !isReadOnlyMode, isDropTargeted: $discardPileDropIsTargeted, addDroppedPayloads: addDroppedPayloadsToDiscardPile ) { @@ -263,8 +263,7 @@ private struct DiscardPileSidebarButton: View { return String(localized: "No items marked", comment: "Discard Pile subtitle when it is empty.") } - let size = RadixFormatters.size(summary.totalAllocatedSize) - return String(localized: "\(summary.itemCount) items • \(size)", comment: "Discard Pile subtitle showing the marked item count and total allocated size. The item count controls pluralization.") + return summary.markedItemsText } } diff --git a/Radix/Localizable.xcstrings b/Radix/Localizable.xcstrings index f6c4ab7..ae11199 100644 --- a/Radix/Localizable.xcstrings +++ b/Radix/Localizable.xcstrings @@ -5320,6 +5320,53 @@ } } }, + "Checking Marked Items" : { + "comment" : "Accessibility label while Radix checks which Discard Pile items can move to Trash.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Markierte Objekte werden überprüft" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Checking Marked Items" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Comprobando los elementos marcados" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Vérification des éléments marqués" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Verifica degli elementi contrassegnati" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Проверка отмеченных объектов" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "正在检查已标记的项目" + } + } + } + }, "Checking for changes in %@" : { "comment" : "Progress title while Radix prepares an incremental rescan.", "localizations" : { @@ -7247,6 +7294,53 @@ } } }, + "Close" : { + "comment" : "Closes the Discard Pile confirmation when no items can be moved.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Schließen" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Close" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Cerrar" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Fermer" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Chiudi" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Закрыть" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "关闭" + } + } + } + }, "Cloud Storage" : { "comment" : "Fallback title for a cloud-storage confirmation dialog.", "localizations" : { @@ -13516,6 +13610,100 @@ } } }, + "Items to Move" : { + "comment" : "Section listing the exact items that can be moved to the Trash.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Zu verschiebende Elemente" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Items to Move" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elementos que se moverán" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Éléments à déplacer" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elementi da spostare" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Объекты для перемещения" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "要移动的项目" + } + } + } + }, + "Items That Will Stay Marked" : { + "comment" : "Section for items that will remain marked in the Discard Pile because they cannot be moved.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Weiterhin markierte Elemente" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Items That Will Stay Marked" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elementos que seguirán marcados" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Éléments qui resteront marqués" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elementi che resteranno contrassegnati" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Объекты, которые останутся отмеченными" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "将保留标记的项目" + } + } + } + }, "Items present in both scans whose tracked size changed" : { "localizations" : { "de" : { @@ -15439,6 +15627,53 @@ } } }, + "Move to Trash…" : { + "comment" : "Opens the Discard Pile confirmation where the user can inspect the Trash batch.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "In den Papierkorb bewegen…" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Move to Trash…" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Mover a la papelera…" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Placer dans la corbeille…" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Sposta nel Cestino…" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Переместить в Корзину…" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "移到废纸篓…" + } + } + } + }, "Moved" : { "comment" : "Comparison change type for an item whose location changed.", "localizations" : { @@ -16103,6 +16338,53 @@ } } }, + "No Items Can Be Moved" : { + "comment" : "Title of the Discard Pile confirmation when every marked item is unavailable.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Keine Elemente können verschoben werden" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "No Items Can Be Moved" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "No se puede mover ningún elemento" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Aucun élément ne peut être déplacé" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nessun elemento può essere spostato" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не удаётся переместить ни одного объекта" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "没有可移动的项目" + } + } + } + }, "No Items Marked" : { "localizations" : { "de" : { @@ -18931,6 +19213,53 @@ } } }, + "Radix can’t verify this mark. Scan the item and mark it again." : { + "comment" : "Reason a Discard Pile item stays marked when its original identity cannot be verified.", + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix kann diese Markierung nicht überprüfen. Scannen Sie das Objekt und markieren Sie es erneut." + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix can’t verify this mark. Scan the item and mark it again." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix no puede verificar esta marca. Analiza el elemento y vuelve a marcarlo." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix ne peut pas vérifier ce marquage. Analysez l’élément et marquez-le à nouveau." + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix non può verificare questo contrassegno. Scansiona l’elemento e contrassegnalo di nuovo." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix не может проверить эту отметку. Просканируйте объект и отметьте его снова." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radix 无法验证此标记。请扫描该项目并重新标记。" + } + } + } + }, "Radix could not create an FSEvents history stream." : { "comment" : "Error shown when an FSEvents history stream cannot be created.", "localizations" : { diff --git a/Radix/Models/FileNodeActions.swift b/Radix/Models/FileNodeActions.swift index e3b96b7..fe7216b 100644 --- a/Radix/Models/FileNodeActions.swift +++ b/Radix/Models/FileNodeActions.swift @@ -156,17 +156,6 @@ nonisolated struct FileNodeActionAvailability: Equatable, Sendable { } } -nonisolated enum ScanPostTrashAction: Equatable { - case clearActiveScan - case removeFromActiveScan - case none - - static func afterRemovingNode(activeTargetID: String?, removedNodeID: String) -> ScanPostTrashAction { - guard let activeTargetID else { return .none } - return activeTargetID == removedNodeID ? .clearActiveScan : .removeFromActiveScan - } -} - extension FileNodeRecord { nonisolated var supportsMoveToTrash: Bool { supportsMoveToTrash(trashSafetyPolicy: .live()) diff --git a/Radix/Services/AppSystemActions.swift b/Radix/Services/AppSystemActions.swift index 7d2ccff..f608f0e 100644 --- a/Radix/Services/AppSystemActions.swift +++ b/Radix/Services/AppSystemActions.swift @@ -31,7 +31,7 @@ struct AppSystemActions { var presentImportScanPanel: () -> URL? var presentComparisonSnapshotPanel: () async -> URL? var fileExists: (URL) -> Bool - var verifyTrashIdentity: (FileNodeRecord) -> TrashIdentityVerificationResult + var verifyTrashIdentity: @Sendable (FileNodeRecord) -> TrashIdentityVerificationResult var isExistingDirectory: (URL) -> Bool var preferredSmartTargetIDs: () -> [String] var mountedVolumeEvents: () -> AnyPublisher diff --git a/Radix/Services/ScanCoordinator.swift b/Radix/Services/ScanCoordinator.swift index de980c6..7807e3e 100644 --- a/Radix/Services/ScanCoordinator.swift +++ b/Radix/Services/ScanCoordinator.swift @@ -227,6 +227,13 @@ final class ScanCoordinator: ObservableObject { isScanOperationInProgress && folderRescanState?.isBackground != true } + var isFullScanInProgress: Bool { + guard isForegroundScanOperationInProgress else { return false } + if snapshot == nil { return true } + guard case .manualFolder = folderRescanState?.origin else { return false } + return progress.executionMode == .fullFallback(.sharedAllocationTopologyChanged) + } + var canRescan: Bool { selectedTarget != nil && !isForegroundScanOperationInProgress } @@ -564,22 +571,25 @@ final class ScanCoordinator: ObservableObject { } @discardableResult - func removeNodesFromCurrentSnapshot(ids nodeIDs: [FileNodeRecord.ID]) async -> Bool { - guard let initialSnapshot = snapshot else { return false } - let removalNodeIDs = initialSnapshot.treeStore.topLevelNodeIDs(from: nodeIDs) - guard !removalNodeIDs.isEmpty else { return false } + func removeNodesFromCurrentSnapshot( + _ nodes: [FileNodeRecord], + matching matchesMovedNode: (FileNodeRecord, FileNodeRecord) -> Bool = { $0.fileIdentity == $1.fileIdentity } + ) async -> Bool { + guard !nodes.isEmpty else { return false } let removalContextID = snapshotContextID do { while snapshotContextID == removalContextID { try Task.checkCancellation() guard let currentSnapshot = snapshot else { return false } - let currentRemovalNodeIDs = currentSnapshot.treeStore.topLevelNodeIDs( - from: removalNodeIDs - ) - if currentRemovalNodeIDs.isEmpty { - return true + let treeStore = currentSnapshot.treeStore + let matchingIDs = nodes.compactMap { moved -> FileNodeRecord.ID? in + guard let node = treeStore.node(id: moved.id), + matchesMovedNode(moved, node) else { return nil } + return node.id } + let currentRemovalNodeIDs = treeStore.topLevelNodeIDs(from: matchingIDs) + guard !currentRemovalNodeIDs.isEmpty else { return false } if let expandingNodeID, currentRemovalNodeIDs.contains(where: { diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index 43055c6..385231b 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -16,43 +16,22 @@ struct ExportConfirmationState: Identifiable, Equatable, Sendable { let archiveURL: URL } -struct DiscardPileState: Equatable, Sendable { - let nodeIDs: [FileNodeRecord.ID] - let snapshotID: UUID? - - init( - nodeIDs: [FileNodeRecord.ID] = [], - snapshotID: UUID? = nil - ) { - self.nodeIDs = nodeIDs - self.snapshotID = nodeIDs.isEmpty ? nil : snapshotID - } - - var isEmpty: Bool { - nodeIDs.isEmpty - } -} - struct DiscardPileSummary: Equatable, Sendable { let itemCount: Int let totalAllocatedSize: Int64 - init(itemCount: Int, totalAllocatedSize: Int64) { - self.itemCount = itemCount - self.totalAllocatedSize = totalAllocatedSize + init(nodes: [FileNodeRecord]) { + self.itemCount = nodes.count + self.totalAllocatedSize = nodes.reduce(into: Int64(0)) { total, node in + total = ScanIntegerMath.addingClamped(total, node.allocatedSize) + } } - var isEmpty: Bool { - itemCount == 0 - } + var isEmpty: Bool { itemCount == 0 } - init(nodes: [FileNodeRecord]) { - self.init( - itemCount: nodes.count, - totalAllocatedSize: nodes.reduce(into: Int64(0)) { total, node in - total += node.allocatedSize - } - ) + var markedItemsText: String { + let size = RadixFormatters.size(totalAllocatedSize) + return String(localized: "\(itemCount) items • \(size)", comment: "Discard Pile status showing the marked item count and total allocated size. The item count controls pluralization.") } } @@ -204,7 +183,10 @@ final class AppModel: ObservableObject { func setReadOnlyMode(_ isEnabled: Bool) { guard canChangeReadOnlyMode, isEnabled != isReadOnlyMode, trashFlow.setReadOnlyMode(isEnabled) else { return } - if isEnabled { cancelDeferredDiscardPileAdd() } + if isEnabled { + cancelDeferredDiscardPileAdd() + cancelDiscardPilePreparation() + } dependencies.preferences.saveReadOnlyMode(isEnabled) } @@ -247,6 +229,9 @@ final class AppModel: ObservableObject { set { trashFlow.replaceDiscardPile(newValue) } } + private var projectedDiscardPileIDs: Set = [] + private var discardPilePreparation: (id: UUID, task: Task)? + private var optimisticTrashVisibility: TrashFlowController.OptimisticTrashVisibilityState { trashFlow.optimisticTrashVisibility } @@ -429,10 +414,16 @@ final class AppModel: ObservableObject { self.objectWillChange.send() self.fileTransferReconciliation.resumeIfPossible() } - trashFlow.onDiscardPileHistoryReplay = { [weak self] state in + trashFlow.$discardPile + .sink { [weak self] pile in + guard let self else { return } + projectedDiscardPileIDs = pile.matchingNodeIDs(in: scanCoordinator.snapshot) + } + .store(in: &cancellables) + trashFlow.onDiscardPileHistoryReplay = { [weak self] _ in guard let self, let fileTreeStore = self.scanCoordinator.fileTreeStore else { return } self.reconcileNavigationForHiddenNodes( - hiddenNodeIDs: Set(state.nodeIDs), + hiddenNodeIDs: self.workspaceHiddenNodeIDs, fileTreeStore: fileTreeStore ) } @@ -467,6 +458,7 @@ final class AppModel: ObservableObject { } func cleanup() { + cancelDiscardPilePreparation() fileTransferReconciliation.cleanup() folderRefreshController?.update(snapshot: nil, focusedNodeID: nil, canRefresh: false) folderRefreshController = nil @@ -506,6 +498,7 @@ final class AppModel: ObservableObject { } func suspendMainWindowActivity() { + cancelDiscardPilePreparation() fileTransferReconciliation.suspend() isWorkspaceActive = false updateAutomaticFolderRefresh() @@ -614,11 +607,15 @@ final class AppModel: ObservableObject { } var discardPileSnapshot: DiscardPileSnapshot { - let nodes = resolvedDiscardPileNodes() - return DiscardPileSnapshot( - nodes: nodes, - summary: DiscardPileSummary(nodes: nodes) - ) + let nodes = discardPile.nodes + return DiscardPileSnapshot(nodes: nodes, summary: DiscardPileSummary(nodes: nodes)) + } + + var isPreparingDiscardPileTrash: Bool { discardPilePreparation != nil } + + var canMoveDiscardPileToTrash: Bool { + !discardPile.isEmpty && !isReadOnlyMode && scanCoordinator.snapshotSource.allowsFileMutation + && canUseWorkspaceCommands && !trashFlow.isMovingFiles && !isPreparingDiscardPileTrash } var workspaceHiddenNodeIDs: Set { @@ -626,8 +623,7 @@ final class AppModel: ObservableObject { } var discardPileRootNodeIDs: Set { - guard discardPile.snapshotID == scanCoordinator.snapshot?.id else { return [] } - return Set(discardPile.nodeIDs) + projectedDiscardPileIDs } var movingToTrashRootNodeIDs: Set { @@ -722,6 +718,7 @@ final class AppModel: ObservableObject { sidebarTargetID: sidebarModel.activeTargetID, visualization: scanVisualizationMode )) + discardPile = DiscardPileState() workspaceTour.start(snapshotID: nil, isReady: false) } @@ -761,6 +758,7 @@ final class AppModel: ObservableObject { } func dismissDiscardPileReview() { + cancelDiscardPilePreparation() workspaceTour.reviewClosed() showsDiscardPileReview = false } @@ -773,6 +771,8 @@ final class AppModel: ObservableObject { dismissOnboarding() case .discardPileReview: dismissDiscardPileReview() + case .discardPileTrashConfirmation: + cancelPendingTrash() case .importPreview: cancelImportPreview() case .comparisonSetup: @@ -837,12 +837,19 @@ final class AppModel: ObservableObject { } private func synchronizeTrashConfirmationPresentation() { - if pendingTrashSelection != nil { - presentationCoordinator.present(.dialog(.trashConfirmation)) + if let selection = pendingTrashSelection { + if selection.discardPileBatch != nil { + presentationCoordinator.present(.sheet(.discardPileTrashConfirmation)) + } else { + presentationCoordinator.present(.dialog(.trashConfirmation)) + } } else { resumeDeferredArchiveImport( presentationCoordinator.cancel(kind: .trashConfirmation) ) + resumeDeferredArchiveImport( + presentationCoordinator.cancel(kind: .discardPileTrashConfirmation) + ) } } @@ -2337,23 +2344,8 @@ final class AppModel: ObservableObject { @discardableResult func requestMoveNodesToTrash(_ nodes: [FileNodeRecord]) -> Bool { - requestMoveNodesToTrash(nodes, allowingHiddenNodes: false) - } - - @discardableResult - private func requestMoveNodesToTrash( - _ nodes: [FileNodeRecord], - allowingHiddenNodes: Bool - ) -> Bool { do { - let nodes = try validatedNodesForMutation( - nodes, - allowingHiddenNodes: allowingHiddenNodes - ) - try requestTrashMove( - for: nodes, - allowingHiddenNodes: allowingHiddenNodes - ) + try requestTrashMove(for: validatedNodesForMutation(nodes)) return true } catch { presentError(error) @@ -2361,16 +2353,12 @@ final class AppModel: ObservableObject { } } - private func requestTrashMove( - for nodes: [FileNodeRecord], - allowingHiddenNodes: Bool = false - ) throws { + private func requestTrashMove(for nodes: [FileNodeRecord]) throws { try trashFlow.stageTrashRequest( for: nodes, activeTarget: scanCoordinator.selectedTarget, trashSafetyPolicy: scanCoordinator.trashSafetyPolicy, - fileTreeStore: scanCoordinator.fileTreeStore, - allowingHiddenNodes: allowingHiddenNodes + fileTreeStore: scanCoordinator.fileTreeStore ) } @@ -2507,39 +2495,83 @@ final class AppModel: ObservableObject { } @discardableResult - func requestMoveDiscardPileToTrash() -> Bool { - reconcileDiscardPile() - return requestMoveNodesToTrash( - topLevelTrashNodes(from: resolvedDiscardPileNodes()), - allowingHiddenNodes: true - ) + func requestMoveDiscardPileToTrash() async -> Bool { + guard !isPreparingDiscardPileTrash else { return false } + do { + try validateSnapshotAllowsMutation() + guard canUseWorkspaceCommands, !trashFlow.isMovingFiles else { throw FileActionError.unsupported } + + let pile = discardPile + let nodes = pile.nodes + let snapshotID = scanCoordinator.snapshot?.id + let policy = scanCoordinator.trashSafetyPolicy + let verifyIdentity = dependencies.systemActions.verifyTrashIdentity + let id = UUID() + let task = Task.detached(priority: .userInitiated) { + try TrashFlowController.prepareDiscardPileTrashRequest( + nodes: nodes, trashSafetyPolicy: policy, verifyIdentity: verifyIdentity + ) + } + discardPilePreparation = (id, task) + objectWillChange.send() + defer { + if discardPilePreparation?.id == id { + discardPilePreparation = nil + objectWillChange.send() + } + } + + let selection = try await withTaskCancellationHandler { + try await task.value + } onCancel: { + task.cancel() + } + guard !Task.isCancelled, discardPilePreparation?.id == id, + discardPile == pile, scanCoordinator.snapshot?.id == snapshotID, + canUseWorkspaceCommands, !trashFlow.isMovingFiles, + pendingTrashSelection == nil, pendingCloudFileAction == nil else { return false } + try validateSnapshotAllowsMutation() + pendingTrashSelection = selection + return true + } catch is CancellationError { + return false + } catch { + presentError(error) + return false + } + } + + private func cancelDiscardPilePreparation() { + guard let preparation = discardPilePreparation else { return } + preparation.task.cancel() + discardPilePreparation = nil + objectWillChange.send() } func confirmMovePendingSelectionToTrash() { - let allowsHiddenNodes = pendingTrashSelection?.allowsHiddenNodes == true - let nodes = pendingTrashSelection?.nodes ?? [] - guard !nodes.isEmpty else { return } + guard let selection = pendingTrashSelection, !selection.nodes.isEmpty else { return } pendingTrashSelection = nil - if !allowsHiddenNodes { - do { - try validateMutationDoesNotIncludeHiddenNodes(nodes) - } catch { - presentError(error) + do { + try validateSnapshotAllowsMutation() + if selection.discardPileBatch == nil { + try validateMutationDoesNotIncludeHiddenNodes(selection.nodes) + } + + if let cloudImpact = cloudStorageImpact(of: selection.nodes) { + pendingCloudFileAction = PendingCloudFileAction( + kind: .moveToTrash, + nodes: selection.nodes, + cloudImpact: cloudImpact, + discardPileBatch: selection.discardPileBatch + ) return } - } - if let cloudImpact = cloudStorageImpact(of: nodes) { - pendingCloudFileAction = PendingCloudFileAction( - kind: .moveToTrash(allowsHiddenNodes: allowsHiddenNodes), - nodes: nodes, - cloudImpact: cloudImpact - ) - return + performConfirmedTrashMove(selection.nodes, discardPileBatch: selection.discardPileBatch) + } catch { + presentError(error) } - - performConfirmedTrashMove(nodes) } func confirmPendingCloudFileAction() { @@ -2551,12 +2583,15 @@ final class AppModel: ObservableObject { case .addToDiscardPile: let nodes = try validatedNodesForDiscardPile(action.nodes) try commitDiscardPileAddition(topLevelTrashNodes(from: nodes)) - case .moveToTrash(let allowsHiddenNodes): - let nodes = try validatedNodesForMutation( - action.nodes, - allowingHiddenNodes: allowsHiddenNodes - ) - performConfirmedTrashMove(nodes) + case .moveToTrash: + let nodes: [FileNodeRecord] + if action.discardPileBatch == nil { + nodes = try validatedNodesForMutation(action.nodes) + } else { + try validateSnapshotAllowsMutation() + nodes = action.nodes + } + performConfirmedTrashMove(nodes, discardPileBatch: action.discardPileBatch) } } catch { presentError(error) @@ -2569,16 +2604,11 @@ final class AppModel: ObservableObject { private func commitDiscardPileAddition(_ nodes: [FileNodeRecord]) throws { try validateSnapshotAllowsMutation() - guard let snapshot = scanCoordinator.snapshot, - let fileTreeStore = scanCoordinator.fileTreeStore else { + guard let snapshot = scanCoordinator.snapshot else { throw FileActionError.unsupported } let previousIDs = Set(discardPile.nodeIDs) - addDiscardPileNodes( - nodes, - snapshot: snapshot, - fileTreeStore: fileTreeStore - ) + addDiscardPileNodes(nodes, snapshot: snapshot) workspaceTour.didAddMarks( Set(discardPile.nodeIDs).subtracting(previousIDs), snapshotID: snapshot.id @@ -2614,9 +2644,13 @@ final class AppModel: ObservableObject { return containsCloudStorage ? .containsCloudStorage : nil } - private func performConfirmedTrashMove(_ nodes: [FileNodeRecord]) { - let originalSnapshotID = scanCoordinator.snapshot?.id - let statsFileTreeStore = scanCoordinator.fileTreeStore + private func performConfirmedTrashMove( + _ nodes: [FileNodeRecord], + discardPileBatch: TrashFlowController.DiscardPileTrashBatch? + ) { + let originalSnapshot = scanCoordinator.snapshot + let originalSnapshotID = originalSnapshot?.id + let statsFileTreeStore = originalSnapshot?.treeStore trashFlow.startConfirmedMove( nodes, @@ -2640,8 +2674,8 @@ final class AppModel: ObservableObject { requestedNodes: requested, movedNodes: moved, actionError: actionError, - originalSnapshotID: originalSnapshotID, - statsFileTreeStore: statsFileTreeStore, + originalSnapshot: originalSnapshot, + discardPileBatch: discardPileBatch, wasCancelled: wasCancelled ) } @@ -2652,22 +2686,17 @@ final class AppModel: ObservableObject { requestedNodes: [FileNodeRecord], movedNodes: [FileNodeRecord], actionError: Error?, - originalSnapshotID: UUID?, - statsFileTreeStore: FileTreeStore?, + originalSnapshot: ScanSnapshot?, + discardPileBatch: TrashFlowController.DiscardPileTrashBatch?, wasCancelled: Bool = false ) { + let originalSnapshotID = originalSnapshot?.id if !movedNodes.isEmpty { - if scanCoordinator.snapshot?.id == originalSnapshotID { - trashFlow.invalidateDiscardPileHistory() - } - if discardPile.snapshotID == originalSnapshotID { - removeMovedNodesFromDiscardPile(movedNodes, fileTreeStore: statsFileTreeStore) - } - recordTrashMove(movedNodes, fileTreeStore: statsFileTreeStore) + trashFlow.invalidateDiscardPileHistory() sidebarScanCacheController.clearCache() - if shouldApplyPostTrashSnapshotUpdate(originalSnapshotID: originalSnapshotID) { - handleMovedToTrash(movedNodes) - } + reconcileScanAfterTrash(movedNodes, originalSnapshotID: originalSnapshotID) + trashFlow.removeMovedDiscardPileNodes(movedNodes, batch: discardPileBatch, from: originalSnapshot) + recordTrashMove(movedNodes, fileTreeStore: originalSnapshot?.treeStore) refreshAvailableTargets() } @@ -2687,59 +2716,118 @@ final class AppModel: ObservableObject { } } - private func shouldApplyPostTrashSnapshotUpdate(originalSnapshotID: UUID?) -> Bool { - guard let originalSnapshotID else { return true } - return scanCoordinator.snapshot?.id == originalSnapshotID - } + private func reconcileScanAfterTrash(_ movedNodes: [FileNodeRecord], originalSnapshotID: UUID?) { + guard let target = scanCoordinator.selectedTarget else { return } + let snapshot = scanCoordinator.snapshot + let isFullScanInProgress = scanCoordinator.isFullScanInProgress + guard snapshot?.source.allowsFileMutation ?? isFullScanInProgress else { return } + + let rootMayHaveMoved = movedNodes.contains { + $0.id == target.id || ($0.isDirectory && Self.path(target.id, isWithin: $0.id)) + } + if rootMayHaveMoved { + let rootStillValid = snapshot.map { + dependencies.systemActions.verifyTrashIdentity($0.root) == .matches + } ?? dependencies.systemActions.fileExists(target.url) + if !rootStillValid { + clearScanAfterTrash(target) + return + } + } - private func handleMovedToTrash(_ nodes: [FileNodeRecord]) { - var shouldClearActiveScan = false - var removalNodeIDs: [FileNodeRecord.ID] = [] - var fallbackFocusID: FileNodeRecord.ID? + if isFullScanInProgress, + rootMayHaveMoved || movedNodes.contains(where: { Self.path($0.id, isWithin: target.id) }) { + if restartCurrentScanAfterTrash(target, snapshot: snapshot) { return } + scanCoordinator.stopScan() + } - for node in nodes { - switch ScanPostTrashAction.afterRemovingNode(activeTargetID: scanCoordinator.selectedTarget?.id, removedNodeID: node.id) { - case .clearActiveScan: - shouldClearActiveScan = true - case .removeFromActiveScan: - removalNodeIDs.append(node.id) - fallbackFocusID = fallbackFocusID ?? postTrashFocusFallbackID(for: node) - case .none: - break + guard let snapshot else { return } + let tree = snapshot.treeStore + let matching = { [self] (moved: FileNodeRecord, current: FileNodeRecord) in + matchesMovedNode(moved, current: current, sameSnapshot: originalSnapshotID == snapshot.id) + } + var summarized: [FileNodeRecord] = [] + let matched = movedNodes.compactMap { moved -> FileNodeRecord? in + if let current = tree.node(id: moved.id) { + return matching(moved, current) ? current : nil } + if let ancestor = summarizedAncestor(of: moved.id, in: tree) { summarized.append(ancestor) } + return nil + } + if matched.contains(where: { $0.id == target.id }) { + clearScanAfterTrash(target) + return } - if shouldClearActiveScan { - trashFlow.cancelPostTrashSnapshotRemoval() - scanCoordinator.clearScan() - navigationModel.reset() - sidebarModel.setActiveTargetID(nil) - sidebarScanCacheController.clearDisplayedSnapshot() - } else if !removalNodeIDs.isEmpty { - enqueuePostTrashSnapshotRemoval( - nodeIDs: removalNodeIDs, - fallbackFocusID: fallbackFocusID - ) + if !summarized.isEmpty { + sidebarScanCacheController.invalidateDisplayedNodes(summarized, in: snapshot) + if restartCurrentScanAfterTrash(target, snapshot: snapshot) { return } } + guard !matched.isEmpty else { return } + + sidebarScanCacheController.invalidateDisplayedNodes(matched, in: snapshot) + let fallbackFocusID = matched.lazy.compactMap { self.postTrashFocusFallbackID(for: $0) }.first + trashFlow.hideTrashNodesDuringMove( + matched, + snapshotID: snapshot.id, + activeSnapshotID: snapshot.id, + activeFileTreeStore: tree + ) + enqueuePostTrashSnapshotRemoval( + movedNodes: movedNodes, + hiddenIDs: Set(matched.map(\.id)), + sameSnapshot: originalSnapshotID == snapshot.id, + fallbackFocusID: fallbackFocusID + ) } - func cancelPendingTrash() { - pendingTrashSelection = nil + private func matchesMovedNode( + _ moved: FileNodeRecord, + current: FileNodeRecord, + sameSnapshot: Bool + ) -> Bool { + if moved.fileIdentity == current.fileIdentity { + return moved.fileIdentity != nil || sameSnapshot + } + guard let movedIdentity = moved.fileIdentity, let currentIdentity = current.fileIdentity, + movedIdentity.isFileSystemIdentity != currentIdentity.isFileSystemIdentity else { return false } + return dependencies.systemActions.verifyTrashIdentity(current) == .missingCurrentItem } - func reconcileDiscardPile() { - guard !discardPile.isEmpty else { return } - guard let snapshot = scanCoordinator.snapshot, - let fileTreeStore = scanCoordinator.fileTreeStore else { - discardPile = DiscardPileState() - return + private func summarizedAncestor(of path: String, in tree: FileTreeStore) -> FileNodeRecord? { + var parent = path + while let separator = parent.lastIndex(of: "/"), parent != "/" { + parent = separator == parent.startIndex ? "/" : String(parent[.. Bool { + guard sidebarModel.activeTargetID == target.id, + deferredWorkBoxes[.scanStart] == nil, deferredWorkBoxes[.sidebarSelection] == nil else { return false } + let options = snapshot?.scanOptions ?? scanOptions(for: target) + sidebarScanCacheController.prepareForScanStart(target: target, options: options) + scanCoordinator.startScan(target, options: options) + return true + } + + private func clearScanAfterTrash(_ target: ScanTarget) { + trashFlow.cancelPostTrashSnapshotRemoval() + scanCoordinator.clearScan() + navigationModel.reset() + if sidebarModel.activeTargetID == target.id { + sidebarModel.setActiveTargetID(nil) } + sidebarScanCacheController.clearDisplayedSnapshot() + } - reconcileDiscardPile(snapshotID: snapshot.id, fileTreeStore: fileTreeStore) + private static func path(_ path: String, isWithin root: String) -> Bool { + root == "/" || path == root || path.hasPrefix(root + "/") + } + + func cancelPendingTrash() { + pendingTrashSelection = nil } private func postTrashFocusFallbackID(for node: FileNodeRecord) -> FileNodeRecord.ID? { @@ -2752,14 +2840,19 @@ final class AppModel: ObservableObject { } private func enqueuePostTrashSnapshotRemoval( - nodeIDs: [FileNodeRecord.ID], + movedNodes: [FileNodeRecord], + hiddenIDs: Set, + sameSnapshot: Bool, fallbackFocusID: FileNodeRecord.ID? ) { let snapshotID = scanCoordinator.snapshot?.id trashFlow.enqueuePostTrashSnapshotRemoval { [weak self, scanCoordinator] in guard scanCoordinator.snapshot?.id == snapshotID else { return } - let didRemove = await scanCoordinator.removeNodesFromCurrentSnapshot(ids: nodeIDs) + let didRemove = await scanCoordinator.removeNodesFromCurrentSnapshot(movedNodes) { [weak self] moved, current in + self?.matchesMovedNode(moved, current: current, sameSnapshot: sameSnapshot) == true + } guard !Task.isCancelled, let self else { return } + trashFlow.unhideTrashNodes(hiddenIDs, snapshotID: snapshotID) if didRemove, let fallbackFocusID, @@ -2878,15 +2971,10 @@ final class AppModel: ObservableObject { return node } - private func validatedNodesForMutation( - _ nodes: [FileNodeRecord], - allowingHiddenNodes: Bool = false - ) throws -> [FileNodeRecord] { + private func validatedNodesForMutation(_ nodes: [FileNodeRecord]) throws -> [FileNodeRecord] { try validateSnapshotAllowsMutation() let nodes = try validatedNodes(nodes, requiresLivePath: true) - if !allowingHiddenNodes { - try validateMutationDoesNotIncludeHiddenNodes(nodes) - } + try validateMutationDoesNotIncludeHiddenNodes(nodes) return nodes } @@ -2965,8 +3053,8 @@ final class AppModel: ObservableObject { guard let snapshotID else { return [] } var nodeIDs = Set() - if discardPile.snapshotID == snapshotID { - nodeIDs.formUnion(discardPile.nodeIDs) + if scanCoordinator.snapshot?.id == snapshotID { + nodeIDs.formUnion(projectedDiscardPileIDs) } if optimisticTrashVisibility.snapshotID == snapshotID { nodeIDs.formUnion(optimisticTrashVisibility.nodeIDs) @@ -2976,36 +3064,20 @@ final class AppModel: ObservableObject { private func addDiscardPileNodes( _ nodes: [FileNodeRecord], - snapshot: ScanSnapshot, - fileTreeStore: FileTreeStore + snapshot: ScanSnapshot ) { - guard !nodes.isEmpty else { return } - - let queuedIDs = (discardPile.snapshotID == snapshot.id ? discardPile.nodeIDs : []) + nodes.map(\.id) - let deduplicatedIDs = deduplicatedDiscardPileIDs(queuedIDs, fileTreeStore: fileTreeStore) + let updatedPile = trashFlow.discardPileAdding(nodes, in: snapshot) reconcileNavigationForHiddenNodes( - hiddenNodeIDs: hiddenNodeIDs(for: snapshot.id).union(deduplicatedIDs), - fileTreeStore: fileTreeStore, + hiddenNodeIDs: hiddenNodeIDs(for: snapshot.id).union(updatedPile.matchingNodeIDs(in: snapshot)), + fileTreeStore: snapshot.treeStore, preservingSelection: navigationModel.selectedNodeIDs.count == 1 ) trashFlow.changeDiscardPile( - DiscardPileState(nodeIDs: deduplicatedIDs, snapshotID: snapshot.id), + updatedPile, actionName: String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion.") ) } - private func deduplicatedDiscardPileIDs( - _ nodeIDs: [FileNodeRecord.ID], - fileTreeStore: FileTreeStore - ) -> [FileNodeRecord.ID] { - fileTreeStore.topLevelNodeIDs(from: nodeIDs) - } - - private func resolvedDiscardPileNodes() -> [FileNodeRecord] { - guard let fileTreeStore = scanCoordinator.fileTreeStore else { return [] } - return discardPile.nodeIDs.compactMap { fileTreeStore.node(id: $0) } - } - private func reconcileNavigationForHiddenNodes( hiddenNodeIDs: Set, fileTreeStore: FileTreeStore, @@ -3050,43 +3122,6 @@ final class AppModel: ObservableObject { return fileTreeStore.rootID } - private func removeMovedNodesFromDiscardPile( - _ movedNodes: [FileNodeRecord], - fileTreeStore: FileTreeStore? - ) { - guard !discardPile.isEmpty, !movedNodes.isEmpty else { return } - - let movedIDs = Set(movedNodes.map(\.id)) - let movedNodeSet = fileTreeStore?.preparedNodeSet(for: movedIDs) - let remainingIDs = discardPile.nodeIDs.filter { queuedID in - guard !movedIDs.contains(queuedID) else { return false } - guard let fileTreeStore, let movedNodeSet else { return true } - return !fileTreeStore.isNodeOrDescendant(queuedID, of: movedNodeSet) - } - guard remainingIDs != discardPile.nodeIDs else { return } - discardPile = DiscardPileState( - nodeIDs: remainingIDs, - snapshotID: discardPile.snapshotID - ) - } - - private func syncDiscardPile(with snapshot: ScanSnapshot?) { - trashFlow.synchronizeDiscardPileContext( - snapshotID: snapshot?.id, - treeContentID: snapshot?.treeStore.contentID - ) - guard !discardPile.isEmpty else { return } - guard let snapshot else { - discardPile = DiscardPileState() - return - } - guard discardPile.snapshotID == snapshot.id else { - discardPile = DiscardPileState() - return - } - reconcileDiscardPile(snapshotID: snapshot.id, fileTreeStore: snapshot.treeStore) - } - private func syncOptimisticTrashVisibility(with snapshot: ScanSnapshot?) { guard optimisticTrashVisibility.snapshotID != nil else { return } guard let snapshot, @@ -3095,27 +3130,11 @@ final class AppModel: ObservableObject { return } - let nodeIDs = optimisticTrashVisibility.nodeIDs.filter { snapshot.treeStore.node(id: $0) != nil } - guard nodeIDs != optimisticTrashVisibility.nodeIDs else { return } - trashFlow.replaceOptimisticTrashVisibility( - nodeIDs: nodeIDs, - snapshotID: snapshot.id - ) - } - - private func reconcileDiscardPile( - snapshotID: UUID, - fileTreeStore: FileTreeStore - ) { - let reconciledIDs = deduplicatedDiscardPileIDs( - discardPile.nodeIDs.filter { fileTreeStore.node(id: $0) != nil }, - fileTreeStore: fileTreeStore - ) - guard reconciledIDs != discardPile.nodeIDs else { return } - discardPile = DiscardPileState( - nodeIDs: reconciledIDs, - snapshotID: snapshotID - ) + let retained = optimisticTrashVisibility.nodes.filter { id, previous in + guard let current = snapshot.treeStore.node(id: id) else { return false } + return DiscardPileState.Mark.matchesIdentity(of: previous, with: current) + } + trashFlow.replaceOptimisticTrashVisibility(nodes: retained, snapshotID: snapshot.id) } private func isDirectoryURL(_ url: URL) -> Bool { @@ -3123,6 +3142,8 @@ final class AppModel: ObservableObject { } private func prepareForScan(_ target: ScanTarget) { + cancelDiscardPilePreparation() + let isTourTarget = tourSession.contains(target) lastErrorMessage = nil scanComparison = nil navigationModel.reset() @@ -3130,11 +3151,10 @@ final class AppModel: ObservableObject { pendingImportPreview = nil pendingTrashSelection = nil pendingCloudFileAction = nil - discardPile = DiscardPileState() clearOptimisticTrashVisibility() - sidebarModel.setActiveTargetID(tourSession.contains(target) ? nil : target.id) + sidebarModel.setActiveTargetID(isTourTarget ? nil : target.id) - if !tourSession.contains(target) { + if !isTourTarget { registerRecentTarget(target) refreshAvailableTargets() } @@ -3159,6 +3179,7 @@ final class AppModel: ObservableObject { } private func prepareForImportedSnapshot() { + cancelDiscardPilePreparation() lastErrorMessage = nil scanComparison = nil navigationModel.reset() @@ -3166,7 +3187,6 @@ final class AppModel: ObservableObject { pendingImportPreview = nil pendingTrashSelection = nil pendingCloudFileAction = nil - discardPile = DiscardPileState() clearOptimisticTrashVisibility() sidebarModel.setActiveTargetID(nil) quickLookController.closePreview() @@ -3278,7 +3298,7 @@ final class AppModel: ObservableObject { quickLookController.closePreview() refreshDiskFreeSpaceCapacity(for: snapshot) syncOptimisticTrashVisibility(with: snapshot) - syncDiscardPile(with: snapshot) + projectedDiscardPileIDs = discardPile.matchingNodeIDs(in: snapshot) fileTransferReconciliation.displayedSnapshotDidChange() if let snapshotID = snapshot?.id, snapshotID == deferredNavigationContextSnapshotID { diff --git a/Radix/ViewModels/AppPresentationCoordinator.swift b/Radix/ViewModels/AppPresentationCoordinator.swift index 8af64bf..c31353f 100644 --- a/Radix/ViewModels/AppPresentationCoordinator.swift +++ b/Radix/ViewModels/AppPresentationCoordinator.swift @@ -14,6 +14,7 @@ final class AppPresentationCoordinator: ObservableObject { case onboarding case goToPath(GoToPathRequest) case discardPileReview + case discardPileTrashConfirmation case importPreview(URL) case comparisonSetup(UUID) @@ -25,6 +26,8 @@ final class AppPresentationCoordinator: ObservableObject { "onboarding" case .discardPileReview: "discard-pile-review" + case .discardPileTrashConfirmation: + "discard-pile-trash-confirmation" case .importPreview(let url): "import-preview-\(url.absoluteString)" case .comparisonSetup(let id): @@ -37,6 +40,7 @@ final class AppPresentationCoordinator: ObservableObject { case .goToPath: .goToPath case .onboarding: .onboarding case .discardPileReview: .discardPileReview + case .discardPileTrashConfirmation: .discardPileTrashConfirmation case .importPreview: .importPreview case .comparisonSetup: .comparisonSetup } @@ -78,6 +82,7 @@ final class AppPresentationCoordinator: ObservableObject { case goToPath case onboarding case discardPileReview + case discardPileTrashConfirmation case importPreview case comparisonSetup case error diff --git a/Radix/ViewModels/SidebarScanCacheController.swift b/Radix/ViewModels/SidebarScanCacheController.swift index 225df25..064cac1 100644 --- a/Radix/ViewModels/SidebarScanCacheController.swift +++ b/Radix/ViewModels/SidebarScanCacheController.swift @@ -176,8 +176,10 @@ final class SidebarScanCacheController { private let completedScanCache: CompletedScanCache private var activeScanCacheKey: ScanCacheKey? private var displayedScanCacheKey: ScanCacheKey? + private var movedDisplayedNodes: (snapshotID: UUID, nodes: [FileNodeRecord])? private var sidebarScopeTask: Task? private var sidebarScopeID: UUID? + private var cacheGeneration: UInt64 = 0 private var transferGeneration: UInt64 = 0 private var transferValidationByKey: [ScanCacheKey: FileTransferValidation] = [:] // Expansion can replace a scoped child's backing. Its original parent @@ -199,7 +201,7 @@ final class SidebarScanCacheController { func resetTransientState() { cancelPendingSidebarTargetRestore() activeScanCacheKey = nil - displayedScanCacheKey = nil + clearDisplayedSnapshot() } func cancelPendingSidebarTargetRestore() { @@ -214,9 +216,32 @@ final class SidebarScanCacheController { func clearDisplayedSnapshot() { displayedScanCacheKey = nil + movedDisplayedNodes = nil + } + + func invalidateDisplayedNodes(_ nodes: [FileNodeRecord], in snapshot: ScanSnapshot) { + let previous = movedDisplayedNodes?.snapshotID == snapshot.id + ? movedDisplayedNodes?.nodes.filter { Self.contains($0, in: snapshot) } ?? [] + : [] + movedDisplayedNodes = (snapshot.id, previous + nodes) + } + + private func isReusable(_ snapshot: ScanSnapshot) -> Bool { + guard let moved = movedDisplayedNodes else { return true } + if moved.snapshotID == snapshot.id && moved.nodes.contains(where: { Self.contains($0, in: snapshot) }) { + return false + } + movedDisplayedNodes = nil + return true + } + + private static func contains(_ node: FileNodeRecord, in snapshot: ScanSnapshot) -> Bool { + guard let current = snapshot.treeStore.node(id: node.id) else { return false } + return current.fileIdentity == node.fileIdentity } func clearCache() { + cacheGeneration &+= 1 completedScanCache.removeAll() transferValidationByKey.removeAll() containingKeyByScope.removeAll() @@ -384,7 +409,7 @@ final class SidebarScanCacheController { let key = ScanCacheKey(target: target, options: options) activeScanCacheKey = key containingKeyByScope.removeValue(forKey: key) - displayedScanCacheKey = nil + clearDisplayedSnapshot() } func currentScanExclusionRootPath(currentSnapshot: ScanSnapshot?) -> String? { @@ -403,7 +428,9 @@ final class SidebarScanCacheController { return } - completedScanCache.store(snapshot, for: cacheKey) + if isReusable(snapshot) { + completedScanCache.store(snapshot, for: cacheKey) + } displayedScanCacheKey = cacheKey } @@ -418,6 +445,7 @@ final class SidebarScanCacheController { startScan: @escaping ScanStart ) -> Bool { let cacheKey = ScanCacheKey(target: target, options: options) + let currentSnapshot = currentSnapshot.flatMap { isReusable($0) ? $0 : nil } if let currentSnapshot, currentSnapshot.target.id == target.id, displayedScanCacheKey == cacheKey { @@ -526,6 +554,7 @@ final class SidebarScanCacheController { let canonicalKey = containingKeyByScope[initialContainingKey] ?? initialContainingKey let canonicalVersion = completedScanCache.snapshot(for: canonicalKey).map(ScanCacheSnapshotVersion.init) let scopeID = UUID() + let scopeCacheGeneration = cacheGeneration sidebarScopeID = scopeID sidebarScopeTask = Task { @MainActor [weak self, snapshotTransformService] in do { @@ -544,6 +573,11 @@ final class SidebarScanCacheController { clearSidebarScope(scopeID) return } + guard cacheGeneration == scopeCacheGeneration else { + clearSidebarScope(scopeID) + startScan(target) + return + } let canonicalParent = completedScanCache.snapshot(for: canonicalKey) let changedCanonicalParent = canonicalParent.flatMap { latest in canonicalVersion != nil && ScanCacheSnapshotVersion(latest) != canonicalVersion ? latest : nil diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index 62031ba..f0809c3 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -1,43 +1,96 @@ import Combine import Foundation +struct DiscardPileState: Equatable, Sendable { + struct Mark: Equatable, Sendable { + let node: FileNodeRecord + let unidentifiedTreeID: UUID? + + init(node: FileNodeRecord, treeID: UUID? = nil) { + self.node = node + unidentifiedTreeID = node.fileIdentity == nil ? treeID : nil + } + + func matches(_ current: FileNodeRecord, treeID: UUID) -> Bool { + guard node.id == current.id else { return false } + if node.fileIdentity == nil { + return unidentifiedTreeID == treeID && current.fileIdentity == nil + } + return Self.matchesIdentity(of: node, with: current) + } + + static func matchesIdentity(of original: FileNodeRecord, with current: FileNodeRecord) -> Bool { + guard original.id == current.id else { return false } + if original.fileIdentity == current.fileIdentity { return true } + guard !original.isSymbolicLink, !current.isSymbolicLink, + let first = original.fileIdentity, let second = current.fileIdentity, + first.isFileSystemIdentity != second.isFileSystemIdentity else { return false } + return SystemIntegration.verifyTrashIdentity(original) == .matches + && SystemIntegration.verifyTrashIdentity(current) == .matches + && SystemIntegration.verifyTrashIdentity(original) == .matches + } + } + + let marks: [Mark] + + init(marks: [Mark] = []) { + self.marks = marks + } + + var nodes: [FileNodeRecord] { marks.map(\.node) } + var nodeIDs: [FileNodeRecord.ID] { marks.map(\.node.id) } + var isEmpty: Bool { marks.isEmpty } + + func matchingNodeIDs(in snapshot: ScanSnapshot?) -> Set { + guard let snapshot, snapshot.isComplete, snapshot.source.allowsFileMutation else { return [] } + let tree = snapshot.treeStore + return Set(marks.compactMap { mark in + guard let current = tree.node(id: mark.node.id), + mark.matches(current, treeID: tree.contentID) else { return nil } + return current.id + }) + } +} + @MainActor final class TrashFlowController { struct OptimisticTrashVisibilityState: Equatable, Sendable { - let nodeIDs: Set + let nodes: [FileNodeRecord.ID: FileNodeRecord] let snapshotID: UUID? - init( - nodeIDs: Set = [], - snapshotID: UUID? = nil - ) { - self.nodeIDs = nodeIDs.isEmpty ? [] : nodeIDs - self.snapshotID = nodeIDs.isEmpty ? nil : snapshotID + init(nodes: [FileNodeRecord.ID: FileNodeRecord] = [:], snapshotID: UUID? = nil) { + self.nodes = nodes + self.snapshotID = nodes.isEmpty ? nil : snapshotID } - } - struct PendingTrashSelection { - let nodes: [FileNodeRecord] - let allowsHiddenNodes: Bool + var nodeIDs: Set { Set(nodes.keys) } + } - init( - nodes: [FileNodeRecord], - allowsHiddenNodes: Bool = false - ) { - self.nodes = nodes - self.allowsHiddenNodes = allowsHiddenNodes + nonisolated struct DiscardPileTrashBatch: Sendable { + struct Unavailable: Sendable { + let node: FileNodeRecord + let message: String } + + let unavailable: [Unavailable] + let coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] + } + + nonisolated struct PendingTrashSelection: Sendable { + let nodes: [FileNodeRecord] + var discardPileBatch: DiscardPileTrashBatch? = nil } struct PendingCloudFileAction { enum Kind: Equatable { case addToDiscardPile - case moveToTrash(allowsHiddenNodes: Bool) + case moveToTrash } let kind: Kind let nodes: [FileNodeRecord] let cloudImpact: CloudStorageLocation.Impact + var discardPileBatch: DiscardPileTrashBatch? = nil } var onChange: (() -> Void)? @@ -57,8 +110,6 @@ final class TrashFlowController { let discardPileUndoManager = UndoManager() var onDiscardPileHistoryReplay: ((DiscardPileState) -> Void)? - private var discardPileHistorySnapshotID: UUID? - private var discardPileHistoryTreeContentID: UUID? private var historyNotifications = Set() private(set) var optimisticTrashVisibility = OptimisticTrashVisibilityState() @@ -92,7 +143,6 @@ final class TrashFlowController { self.pendingTrashSelection = pendingTrashSelection self.pendingCloudFileAction = pendingCloudFileAction self.discardPile = discardPile - discardPileHistorySnapshotID = discardPile.snapshotID discardPileUndoManager.groupsByEvent = false discardPileUndoManager.levelsOfUndo = 50 NotificationCenter.default.publisher( @@ -125,13 +175,10 @@ final class TrashFlowController { @discardableResult func replaceOptimisticTrashVisibility( - nodeIDs: Set, + nodes: [FileNodeRecord.ID: FileNodeRecord], snapshotID: UUID? ) -> Bool { - let updatedState = OptimisticTrashVisibilityState( - nodeIDs: nodeIDs, - snapshotID: snapshotID - ) + let updatedState = OptimisticTrashVisibilityState(nodes: nodes, snapshotID: snapshotID) guard updatedState != optimisticTrashVisibility else { return false } optimisticTrashVisibility = updatedState notifyChanged() @@ -140,39 +187,80 @@ final class TrashFlowController { @discardableResult func clearOptimisticTrashVisibility() -> Bool { - replaceOptimisticTrashVisibility(nodeIDs: [], snapshotID: nil) + replaceOptimisticTrashVisibility(nodes: [:], snapshotID: nil) } func stageTrashRequest( for nodes: [FileNodeRecord], activeTarget: ScanTarget?, trashSafetyPolicy: TrashSafetyPolicy, - fileTreeStore: FileTreeStore?, - allowingHiddenNodes: Bool = false + fileTreeStore: FileTreeStore? ) throws { guard !isReadOnlyMode else { throw FileActionError.readOnlyMode } - guard nodes.allSatisfy({ node in - node.supportsMoveToTrash( - activeTarget: activeTarget, - trashSafetyPolicy: trashSafetyPolicy - ) + guard nodes.allSatisfy({ + $0.supportsMoveToTrash(activeTarget: activeTarget, trashSafetyPolicy: trashSafetyPolicy) }) else { throw FileActionError.unsupported } let trashNodes = Self.topLevelTrashNodes(from: nodes, fileTreeStore: fileTreeStore) - pendingTrashSelection = PendingTrashSelection( - nodes: trashNodes, - allowsHiddenNodes: allowingHiddenNodes - ) + pendingTrashSelection = PendingTrashSelection(nodes: trashNodes) } - func synchronizeDiscardPileContext(snapshotID: UUID?, treeContentID: UUID?) { - guard discardPileHistorySnapshotID != snapshotID || - discardPileHistoryTreeContentID != treeContentID else { return } - discardPileHistorySnapshotID = snapshotID - discardPileHistoryTreeContentID = treeContentID - invalidateDiscardPileHistory() + nonisolated static func prepareDiscardPileTrashRequest( + nodes: [FileNodeRecord], + trashSafetyPolicy: TrashSafetyPolicy, + verifyIdentity: (FileNodeRecord) -> TrashIdentityVerificationResult + ) throws -> PendingTrashSelection { + guard !nodes.isEmpty else { throw FileActionError.noSelection } + + var unavailable: [DiscardPileTrashBatch.Unavailable] = [] + var eligible: [FileNodeRecord] = [] + for node in nodes { + try Task.checkCancellation() + let message: String? + if !node.supportsMoveToTrash(trashSafetyPolicy: trashSafetyPolicy) { + message = FileActionError.unsupported.localizedDescription + } else { + let result = node.fileIdentity == nil ? .missingScannedIdentity : verifyIdentity(node) + switch result { + case .mismatch, .missingScannedIdentity: + message = String(localized: "Radix can’t verify this mark. Scan the item and mark it again.") + default: + message = Self.fileActionError(for: result, node: node)?.localizedDescription + } + } + if let message { + unavailable.append(.init(node: node, message: message)) + } else { + eligible.append(node) + } + } + + var foldersByPath: [String: FileNodeRecord.ID] = [:] + for folder in eligible where folder.isDirectory && !folder.isSymbolicLink { + try Task.checkCancellation() + let path = folder.url.resolvingSymlinksInPath().path + if foldersByPath[path] == nil { foldersByPath[path] = folder.id } + } + var coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] = [:] + let operations = try eligible.filter { node in + try Task.checkCancellation() + guard !foldersByPath.isEmpty else { return true } + var parent = node.url.deletingLastPathComponent().resolvingSymlinksInPath().path + var coveringID: FileNodeRecord.ID? + while let separator = parent.lastIndex(of: "/"), parent != "/" { + if let ancestor = foldersByPath[parent] { coveringID = ancestor } + parent = String(parent[.. DiscardPileState { + let tree = snapshot.treeStore + var updated = discardPile.marks + var offsets = Dictionary(uniqueKeysWithValues: updated.enumerated().map { ($0.element.node.id, $0.offset) }) + for node in nodes { + let mark = DiscardPileState.Mark(node: node, treeID: tree.contentID) + guard let current = tree.node(id: node.id), mark.matches(current, treeID: tree.contentID) else { continue } + if let offset = offsets[node.id] { + if !updated[offset].matches(node, treeID: tree.contentID) { updated[offset] = mark } + } else { + offsets[node.id] = updated.count + updated.append(mark) + } + } + + let incomingIDs = Set(nodes.map(\.id)) + let matchingIDs = DiscardPileState(marks: updated).matchingNodeIDs(in: snapshot) + return DiscardPileState(marks: updated.filter { mark in + guard matchingIDs.contains(mark.node.id) else { return true } + var parent = tree.parent(of: mark.node.id) + while let ancestor = parent { + if matchingIDs.contains(ancestor.id), + let offset = offsets[ancestor.id], + ancestor.isDirectory, !ancestor.isSymbolicLink, + incomingIDs.contains(ancestor.id) || incomingIDs.contains(mark.node.id), + updated[offset].node.fileIdentity != nil || mark.node.fileIdentity == nil { + return false + } + parent = tree.parent(of: ancestor.id) + } + return true + }) + } + + func removeMovedDiscardPileNodes( + _ movedNodes: [FileNodeRecord], + batch: DiscardPileTrashBatch?, + from originalSnapshot: ScanSnapshot? + ) { + guard !discardPile.isEmpty else { return } + let removed = movedNodes + movedNodes.flatMap { batch?.coveredMarks[$0.id] ?? [] } + let removedByID = Dictionary(removed.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) + let tree = originalSnapshot?.treeStore + let movedFolders = movedNodes.filter { $0.isDirectory && !$0.isSymbolicLink } + let matchedFolders = movedFolders.filter { folder in + tree?.node(id: folder.id)?.fileIdentity == folder.fileIdentity + } + let removedSubtrees = tree?.preparedNodeSet(for: Set(matchedFolders.map(\.id))) + + let remaining = discardPile.marks.filter { mark in + if let node = removedByID[mark.node.id] { + return mark.node.fileIdentity != node.fileIdentity + || (node.fileIdentity == nil && mark.unidentifiedTreeID != tree?.contentID) + } + guard batch == nil, let tree, let removedSubtrees, + tree.isNodeOrDescendant(mark.node.id, of: removedSubtrees), + let original = tree.node(id: mark.node.id) else { return true } + return !mark.matches(original, treeID: tree.contentID) + } + if remaining != discardPile.marks { replaceDiscardPile(DiscardPileState(marks: remaining)) } + } + func changeDiscardPile(_ state: DiscardPileState, actionName: String) { guard state != discardPile else { return } - if discardPileHistorySnapshotID == nil { - discardPileHistorySnapshotID = state.snapshotID ?? discardPile.snapshotID - } - guard let snapshotID = discardPileHistorySnapshotID, - state.isEmpty || state.snapshotID == snapshotID else { return } let previous = discardPile let isReplaying = discardPileUndoManager.isUndoing || discardPileUndoManager.isRedoing if !isReplaying { discardPileUndoManager.beginUndoGrouping() } discardPileUndoManager.registerUndo(withTarget: self) { controller in - guard controller.discardPileHistorySnapshotID == snapshotID else { return } controller.changeDiscardPile(previous, actionName: actionName) } discardPileUndoManager.setActionName(actionName) @@ -209,9 +353,9 @@ final class TrashFlowController { func removeDiscardPileNodes(ids nodeIDs: Set) { guard !nodeIDs.isEmpty else { return } - let remainingIDs = discardPile.nodeIDs.filter { !nodeIDs.contains($0) } + let remaining = discardPile.marks.filter { !nodeIDs.contains($0.node.id) } changeDiscardPile( - DiscardPileState(nodeIDs: remainingIDs, snapshotID: discardPile.snapshotID), + DiscardPileState(marks: remaining), actionName: String(localized: "Remove from Discard Pile", comment: "Action for unmarking items that will no longer be included in the Discard Pile.") ) } @@ -232,7 +376,7 @@ final class TrashFlowController { return fileTreeStore.topLevelNodeIDs(from: nodes.map(\.id)).compactMap { nodesByID[$0] } } - static func fileActionError( + nonisolated static func fileActionError( for result: TrashIdentityVerificationResult, node: FileNodeRecord ) -> Error? { @@ -263,17 +407,21 @@ final class TrashFlowController { return false } - let nodeIDs = Set(activeFileTreeStore.topLevelNodeIDs(from: nodes.map(\.id))) + let matched = nodes.compactMap { node -> FileNodeRecord? in + guard let current = activeFileTreeStore.node(id: node.id), + DiscardPileState.Mark.matchesIdentity(of: node, with: current) else { return nil } + return current + } + let nodeIDs = Set(activeFileTreeStore.topLevelNodeIDs(from: matched.map(\.id))) guard !nodeIDs.isEmpty else { return false } - let existingIDs = optimisticTrashVisibility.snapshotID == snapshotID - ? optimisticTrashVisibility.nodeIDs - : [] - let hiddenIDs = existingIDs.union(nodeIDs) - replaceOptimisticTrashVisibility( - nodeIDs: hiddenIDs, - snapshotID: snapshotID - ) + var hiddenNodes = optimisticTrashVisibility.snapshotID == snapshotID + ? optimisticTrashVisibility.nodes + : [:] + for node in matched where nodeIDs.contains(node.id) { + hiddenNodes[node.id] = node + } + replaceOptimisticTrashVisibility(nodes: hiddenNodes, snapshotID: snapshotID) return true } @@ -282,22 +430,16 @@ final class TrashFlowController { movedNodes: [FileNodeRecord], snapshotID: UUID? ) { - guard let snapshotID, - optimisticTrashVisibility.snapshotID == snapshotID else { - return - } - let movedNodeIDs = Set(movedNodes.map(\.id)) - let unmovedNodeIDs = Set( - requestedNodes - .map(\.id) - .filter { !movedNodeIDs.contains($0) } - ) - guard !unmovedNodeIDs.isEmpty else { return } + let unmovedNodeIDs = Set(requestedNodes.map(\.id)).subtracting(movedNodeIDs) + unhideTrashNodes(unmovedNodeIDs, snapshotID: snapshotID) + } - let hiddenIDs = optimisticTrashVisibility.nodeIDs.subtracting(unmovedNodeIDs) + func unhideTrashNodes(_ nodeIDs: Set, snapshotID: UUID?) { + guard !nodeIDs.isEmpty, let snapshotID, + optimisticTrashVisibility.snapshotID == snapshotID else { return } replaceOptimisticTrashVisibility( - nodeIDs: hiddenIDs, + nodes: optimisticTrashVisibility.nodes.filter { !nodeIDs.contains($0.key) }, snapshotID: snapshotID ) } diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index c23eb19..1c1bc94 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -1,6 +1,7 @@ import AppKit import Combine import Foundation +import os import Testing @testable import RadixCore @@ -156,7 +157,7 @@ struct AppModelDependencyTests { } @Test - func testReadOnlyModeBlocksCleanupPreservesPileAndAllowsBrowsing() { + func testReadOnlyModeBlocksCleanupPreservesPileAndAllowsBrowsing() async { let recorder = AppModelActionRecorder() var actions = AppSystemActions.inert actions.fileExists = { _ in true } @@ -181,7 +182,7 @@ struct AppModelDependencyTests { model.requestMoveSelectedToTrash() #expect(model.pendingTrashSelection != nil) model.trashFlow.pendingCloudFileAction = .init( - kind: .moveToTrash(allowsHiddenNodes: false), nodes: [file], cloudImpact: .storedInCloud) + kind: .moveToTrash, nodes: [file], cloudImpact: .storedInCloud) model.setReadOnlyMode(true) #expect(model.pendingTrashSelection == nil) @@ -191,7 +192,7 @@ struct AppModelDependencyTests { #expect(!model.addNodesToDiscardPile([file])) #expect(!model.addNodeIDsToDiscardPile([file.id], snapshotID: snapshot.id)) #expect(!model.requestMoveNodesToTrash([file])) - #expect(!model.requestMoveDiscardPileToTrash()) + #expect(!(await model.requestMoveDiscardPileToTrash())) model.requestMoveSelectedToTrash() model.requestMovePrimarySelectionToTrash() #expect(model.pendingTrashSelection == nil) @@ -1232,6 +1233,7 @@ struct AppModelDependencyTests { func testOptimisticTrashVisibilityMutationsPublishThroughAppModel() { let model = AppModel(dependencies: makeDependencies()) let snapshotID = UUID() + let file = makeTestFileNode(id: "/selection/file.txt", name: "file.txt") var publicationCount = 0 let cancellable = model.objectWillChange.sink { publicationCount += 1 @@ -1240,14 +1242,14 @@ struct AppModelDependencyTests { #expect( model.trashFlow.replaceOptimisticTrashVisibility( - nodeIDs: ["/selection/file.txt"], + nodes: [file.id: file], snapshotID: snapshotID )) #expect(publicationCount == 1) #expect( !(model.trashFlow.replaceOptimisticTrashVisibility( - nodeIDs: ["/selection/file.txt"], + nodes: [file.id: file], snapshotID: snapshotID ))) #expect(publicationCount == 1) @@ -1303,18 +1305,18 @@ struct AppModelDependencyTests { } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let oldFile = makeTestFileNode(id: "/selection/file.txt", name: "file.txt", size: 40) + let oldFile = makeTestFileNode(id: "/selection/file.txt", name: "file.txt", size: 40, fileIdentity: FileIdentity(device: 1, inode: 1)) let oldRoot = makeTestDirectoryNode(id: "/selection", name: "selection", children: [oldFile]) let oldStore = FileTreeStore(root: oldRoot, childrenByID: [oldRoot.id: [oldFile]]) let oldSnapshot = makeTestSnapshot(root: oldRoot, store: oldStore) model.scanState.replaceCurrentSnapshot(oldSnapshot) model.navigation.reconcileAfterSnapshotApplied(oldSnapshot) model.addNodesToDiscardPile([oldFile]) - #expect(model.requestMoveDiscardPileToTrash()) + #expect(await model.requestMoveDiscardPileToTrash()) model.confirmMovePendingSelectionToTrash() try await probe.waitUntilStarted() - let newFile = makeTestFileNode(id: oldFile.id, name: oldFile.name, size: 80) + let newFile = makeTestFileNode(id: oldFile.id, name: oldFile.name, size: 80, fileIdentity: FileIdentity(device: 1, inode: 2)) let newRoot = makeTestDirectoryNode(id: "/selection", name: "selection", children: [newFile]) let newStore = FileTreeStore(root: newRoot, childrenByID: [newRoot.id: [newFile]]) let newSnapshot = makeTestSnapshot(root: newRoot, store: newStore) @@ -1327,7 +1329,6 @@ struct AppModelDependencyTests { try await waitUntil("old async trash completion recorded", timeout: 2) { model.usageStats.bytesMovedToTrash == oldFile.allocatedSize } - #expect(model.discardPile.snapshotID == newSnapshot.id) #expect(model.discardPile.nodeIDs == [newFile.id]) #expect(model.discardPileSnapshot.summary.totalAllocatedSize == newFile.allocatedSize) } @@ -1346,15 +1347,15 @@ struct AppModelDependencyTests { } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let first = makeTestFileNode(id: "/selection/first.bin", name: "first.bin", size: 40) - let second = makeTestFileNode(id: "/selection/second.bin", name: "second.bin", size: 60) + let first = makeTestFileNode(id: "/selection/first.bin", name: "first.bin", size: 40, fileIdentity: FileIdentity(device: 1, inode: 1)) + let second = makeTestFileNode(id: "/selection/second.bin", name: "second.bin", size: 60, fileIdentity: FileIdentity(device: 1, inode: 2)) let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [first, second]) let store = FileTreeStore(root: root, childrenByID: [root.id: [first, second]]) let snapshot = makeTestSnapshot(root: root, store: store) model.scanState.replaceCurrentSnapshot(snapshot) model.navigation.reconcileAfterSnapshotApplied(snapshot) model.addNodesToDiscardPile([first, second]) - #expect(model.requestMoveDiscardPileToTrash()) + #expect(await model.requestMoveDiscardPileToTrash()) model.confirmMovePendingSelectionToTrash() try await probe.waitUntilStarted() @@ -1619,7 +1620,7 @@ struct AppModelDependencyTests { #expect(recorder.movedToTrashURLs.isEmpty) #expect(model.pendingTrashSelection == nil) - #expect(model.pendingCloudFileAction?.kind == .moveToTrash(allowsHiddenNodes: false)) + #expect(model.pendingCloudFileAction?.kind == .moveToTrash) #expect(model.pendingCloudFileAction?.cloudImpact == .storedInCloud) model.confirmPendingCloudFileAction() @@ -1664,7 +1665,6 @@ struct AppModelDependencyTests { } #expect(model.scanState.snapshot?.treeStore.node(id: queued.id) != nil) #expect(model.discardPile.nodeIDs == [queued.id]) - #expect(model.discardPile.snapshotID == snapshot.id) } @Test @@ -1705,7 +1705,6 @@ struct AppModelDependencyTests { model.scanState.snapshot?.treeStore.node(id: visible.id) == nil } #expect(model.discardPile.nodeIDs == [firstQueued.id, secondQueued.id]) - #expect(model.discardPile.snapshotID == snapshot.id) } @Test @@ -1905,7 +1904,6 @@ struct AppModelDependencyTests { model.removeDiscardPileNodes(ids: [first.id, third.id]) #expect(model.discardPile.nodeIDs == [second.id]) - #expect(model.discardPile.snapshotID == snapshot.id) #expect(publicationCount == 1) } @@ -1963,24 +1961,22 @@ struct AppModelDependencyTests { } @Test - func testEmptyDiscardPileHistoryClearsForNewScanAndChangedTree() throws { + func testDiscardPileHistorySurvivesSnapshotChanges() throws { let model = AppModel(dependencies: makeDependencies()) let file = installSelection(on: model) #expect(model.addNodesToDiscardPile([file])) model.clearDiscardPile() - #expect(model.canUndoDiscardPileChange) let existing = try #require(model.scanState.snapshot) - let changed = try #require(existing.removingNode(id: file.id)) - #expect(changed.id == existing.id) - model.scanState.replaceCurrentSnapshot(changed) - #expect(!(model.discardPileUndoManager.canUndo)) - #expect(!(model.discardPileUndoManager.canRedo)) - let replacement = installSelection(on: model) - #expect(model.addNodesToDiscardPile([replacement])) - model.clearDiscardPile() + model.scanState.replaceCurrentSnapshot(existing.removingNode(id: file.id)) #expect(model.canUndoDiscardPileChange) + model.undoDiscardPileChange() + #expect(model.discardPileSnapshot.nodes == [file]) + #expect(model.workspaceHiddenNodeIDs.isEmpty) + installSelection(on: model) - #expect(!(model.discardPileUndoManager.canUndo)) + #expect(model.canRedoDiscardPileChange) + model.redoDiscardPileChange() + #expect(model.discardPile.isEmpty) } @Test @@ -2027,27 +2023,58 @@ struct AppModelDependencyTests { } @Test - func testDiscardPileAddDefersLivePathValidationUntilTrashRequest() { - var fileExistsCallCount = 0 + func testDiscardPileAddDefersLivePathValidationUntilTrashRequest() async { + let verified = OSAllocatedUnfairLock(initialState: [FileNodeRecord]()) var actions = AppSystemActions.inert - actions.fileExists = { _ in - fileExistsCallCount += 1 - return false + actions.verifyTrashIdentity = { node in + verified.withLock { $0.append(node) } + return .missingCurrentItem } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let file = installSelection(on: model) + let file = installSelection( + on: model, + file: makeTestFileNode(id: "/selection/file.txt", name: "file.txt", fileIdentity: FileIdentity(device: 1, inode: 1)) + ) - let didAdd = model.addNodesToDiscardPile([file]) + #expect(model.addNodesToDiscardPile([file])) + #expect(verified.withLock { $0.isEmpty }) - #expect(didAdd) - #expect(model.discardPile.nodeIDs == [file.id]) - #expect(fileExistsCallCount == 0) + #expect(await model.requestMoveDiscardPileToTrash()) + #expect(verified.withLock { $0 } == [file]) + #expect(model.pendingTrashSelection?.nodes.isEmpty == true) + #expect(model.pendingTrashSelection?.discardPileBatch?.unavailable.map(\.node) == [file]) + #expect(model.discardPileSnapshot.nodes == [file]) + } + + @Test + func testDismissingDiscardPileReviewCancelsPreparation() async throws { + let verifierStarted = OSAllocatedUnfairLock(initialState: false) + let verifierGate = DispatchSemaphore(value: 0) + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { _ in + verifierStarted.withLock { $0 = true } + _ = verifierGate.wait(timeout: .now() + 5) + return .matches + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + let file = installSelection( + on: model, + file: makeTestFileNode(id: "/selection/file.txt", name: "file.txt", fileIdentity: FileIdentity(device: 1, inode: 1)) + ) + try #require(model.addNodesToDiscardPile([file])) + model.presentDiscardPileReview() + let preparation = Task { await model.requestMoveDiscardPileToTrash() } + defer { verifierGate.signal() } - let didRequestTrash = model.requestMoveDiscardPileToTrash() + try await waitUntil("discard pile verifier starts") { verifierStarted.withLock { $0 } } + #expect(model.isPreparingDiscardPileTrash) + model.dismissDiscardPileReview() + verifierGate.signal() - #expect(!(didRequestTrash)) - #expect(fileExistsCallCount == 1) - #expect(model.discardPile.nodeIDs == [file.id]) + #expect(!(await preparation.value)) + #expect(!model.isPreparingDiscardPileTrash) + #expect(model.pendingTrashSelection == nil) + #expect(model.discardPileSnapshot.nodes == [file]) } @Test @@ -2370,7 +2397,7 @@ struct AppModelDependencyTests { } @Test - func testDiscardPileClearsWhenActiveSnapshotIsReplaced() { + func testDiscardPileReviewSurvivesActiveSnapshotReplacement() { var actions = AppSystemActions.inert actions.fileExists = { _ in true } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -2389,7 +2416,7 @@ struct AppModelDependencyTests { let secondSnapshot = makeTestSnapshot(root: secondRoot, store: secondStore) model.scanState.replaceCurrentSnapshot(secondSnapshot) - #expect(model.discardPile.isEmpty) + #expect(model.discardPileSnapshot.nodes == [firstFile]) #expect(model.workspaceHiddenNodeIDs.isEmpty) } @@ -2404,7 +2431,7 @@ struct AppModelDependencyTests { } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let child = makeTestFileNode(id: "/selection/folder/child.txt", name: "child.txt") - let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child]) + let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child], fileIdentity: FileIdentity(device: 1, inode: 1)) let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder]) let store = FileTreeStore( root: root, @@ -2418,7 +2445,7 @@ struct AppModelDependencyTests { model.addNodesToDiscardPile([child]) model.addNodesToDiscardPile([folder]) - let didRequestTrash = model.requestMoveDiscardPileToTrash() + let didRequestTrash = await model.requestMoveDiscardPileToTrash() #expect(didRequestTrash) #expect(model.pendingTrashSelection?.nodes.map(\.id) == [folder.id]) @@ -2434,7 +2461,50 @@ struct AppModelDependencyTests { } @Test - func testDiscardPileReconcilesUnavailableQueuedIDsOut() { + func testDiscardPilePreviewsFrozenCrossLocationBatchAndKeepsFailedSuffix() async throws { + let first = makeTestFileNode( + id: "/first/file", name: "file", size: 10, fileIdentity: FileIdentity(device: 1, inode: 1) + ) + let failed = makeTestFileNode( + id: "/second/failed", name: "failed", size: 20, fileIdentity: FileIdentity(device: 1, inode: 2) + ) + let unattempted = makeTestFileNode( + id: "/second/unattempted", name: "unattempted", size: 30, fileIdentity: FileIdentity(device: 1, inode: 3) + ) + var attempted: [FileNodeRecord] = [] + var actions = AppSystemActions.inert + actions.moveToTrash = { node in + attempted.append(node) + return node == failed ? .mismatch : .matches + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + + func display(_ rootID: String, nodes: [FileNodeRecord]) { + let root = makeTestDirectoryNode(id: rootID, name: rootID, children: nodes) + let tree = FileTreeStore(root: root, childrenByID: [root.id: nodes]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(root: root, store: tree)) + } + + display("/first", nodes: [first]) + try #require(model.addNodesToDiscardPile([first])) + display("/second", nodes: [failed, unattempted]) + try #require(model.addNodesToDiscardPile([failed, unattempted])) + #expect(model.discardPileSnapshot.nodes == [first, failed, unattempted]) + #expect(model.workspaceHiddenNodeIDs == [failed.id, unattempted.id]) + + try #require(await model.requestMoveDiscardPileToTrash()) + #expect(model.pendingTrashSelection?.nodes == [first, failed, unattempted]) + #expect(attempted.isEmpty) + + model.confirmMovePendingSelectionToTrash() + try await waitUntil("cross-location Trash reports its failed item") { model.lastErrorMessage != nil } + #expect(attempted == [first, failed]) + #expect(model.discardPileSnapshot.nodes == [failed, unattempted]) + #expect(model.usageStats.bytesMovedToTrash == first.allocatedSize) + } + + @Test + func testDiscardPileKeepsMarksMissingFromCurrentTree() { var actions = AppSystemActions.inert actions.fileExists = { _ in true } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -2463,7 +2533,7 @@ struct AppModelDependencyTests { ) model.scanState.replaceCurrentSnapshot(updatedSnapshot) - #expect(model.discardPile.nodeIDs == [first.id]) + #expect(model.discardPileSnapshot.nodes == [first, second]) } @Test @@ -3535,7 +3605,7 @@ struct AppModelDependencyTests { model.clearSelection() case .hidden: model.trashFlow.replaceDiscardPile( - DiscardPileState(nodeIDs: [node.id], snapshotID: current.id) + DiscardPileState(marks: [.init(node: node, treeID: current.treeStore.contentID)]) ) case .removed: let removed = try #require(current.removingNode(id: node.id)) diff --git a/RadixCoreTests/AppModelFolderRefreshTests.swift b/RadixCoreTests/AppModelFolderRefreshTests.swift index 732a244..1c0592a 100644 --- a/RadixCoreTests/AppModelFolderRefreshTests.swift +++ b/RadixCoreTests/AppModelFolderRefreshTests.swift @@ -52,9 +52,10 @@ struct AppModelFolderRefreshTests { func nativeFileDragPausesAutomaticRefreshAndPublicationUntilSessionsEnd() async throws { let fixture = AppFolderRefreshFixture() var actions = AppSystemActions.inert + let scannedTree = fixture.snapshot.treeStore actions.verifyTrashIdentity = { node in guard let scannedIdentity = node.fileIdentity else { return .missingScannedIdentity } - guard let currentIdentity = fixture.snapshot.treeStore.node(id: node.id)?.fileIdentity else { + guard let currentIdentity = scannedTree.node(id: node.id)?.fileIdentity else { return .missingCurrentItem } return scannedIdentity == currentIdentity ? .matches : .mismatch @@ -285,7 +286,8 @@ struct AppModelFolderRefreshTests { #expect(model.navigation.focusedNodeID == fixture.downloads.id) #expect(model.navigation.selectedNodeID == fixture.keptDownload.id) #expect(model.discardPile.nodeIDs == [fixture.markedDownload.id]) - #expect(model.discardPile.snapshotID == fixture.snapshot.id) + #expect(model.workspaceHiddenNodeIDs.contains(fixture.markedDownload.id)) + #expect(model.scanState.snapshot?.treeStore.node(id: fixture.document.id) == fixture.document) #expect(model.scanState.scanCompletionNotice == nil) #expect(harness.scanner.requests.count == 1) diff --git a/RadixCoreTests/ScanCoordinatorTests.swift b/RadixCoreTests/ScanCoordinatorTests.swift index cb06ab1..261560d 100644 --- a/RadixCoreTests/ScanCoordinatorTests.swift +++ b/RadixCoreTests/ScanCoordinatorTests.swift @@ -1386,7 +1386,7 @@ struct ScanCoordinatorTests { store: store )) - let didRemove = await coordinator.removeNodesFromCurrentSnapshot(ids: [first.id, second.id]) + let didRemove = await coordinator.removeNodesFromCurrentSnapshot([first, second]) let batches = await transformService.recordedRemovingNodeIDBatches() #expect(didRemove) @@ -1414,10 +1414,10 @@ struct ScanCoordinatorTests { )) let firstRemoval = Task { @MainActor in - await coordinator.removeNodesFromCurrentSnapshot(ids: [first.id]) + await coordinator.removeNodesFromCurrentSnapshot([first]) } try await transformService.waitUntilPaused() - let didRemoveSecond = await coordinator.removeNodesFromCurrentSnapshot(ids: [second.id]) + let didRemoveSecond = await coordinator.removeNodesFromCurrentSnapshot([second]) await transformService.resume() let didRemoveFirst = await firstRemoval.value let batches = await transformService.recordedRemovalBatches() @@ -1430,6 +1430,62 @@ struct ScanCoordinatorTests { #expect(coordinator.snapshot?.treeStore.node(id: retained.id) != nil) } + @Test + func testConfirmedTrashRemovalRetriesAfterRescanChangesIdentityEncoding() async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let target = ScanTarget(url: fixture) + let folder = ScanTarget(url: fixture.appending(path: "folder", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: folder.url, withIntermediateDirectories: true) + let file = folder.url.appending(path: "moved.dat") + try Data([1]).write(to: file) + let loader = ScanMetadataLoader() + let inode = try loader.fileSystemIdentity(at: file) + let resource = try #require(loader.metadata(for: file).fileIdentity) + try #require(inode.isFileSystemIdentity != resource.isFileSystemIdentity) + let originalFile = makeTestFileNode(id: file.path, name: "moved.dat", fileIdentity: inode) + let alternateFile = makeTestFileNode(id: file.path, name: "moved.dat", fileIdentity: resource) + try #require(SystemIntegration.verifyTrashIdentity(originalFile) == .matches) + try #require(SystemIntegration.verifyTrashIdentity(alternateFile) == .matches) + let folderRoot = makeTestDirectoryNode(id: folder.id, name: folder.displayName, children: [originalFile]) + let root = makeTestDirectoryNode(id: target.id, name: target.displayName, children: [folderRoot]) + let tree = FileTreeStore(root: root, childrenByID: [root.id: [folderRoot], folder.id: [originalFile]]) + let baseline = makeCoordinatorSnapshot(target: target, root: root, store: tree, scanOptions: ScanOptions()) + let alternateRoot = makeTestDirectoryNode(id: folder.id, name: folder.displayName, children: [alternateFile]) + let alternateScan = makeCoordinatorSnapshot( + target: folder, root: alternateRoot, + store: FileTreeStore(root: alternateRoot, childrenByID: [folder.id: [alternateFile]]) + ) + + let service = ControlledScanService() + let transformer = PausingSnapshotTransformService(pausedRemovalID: originalFile.id) + defer { Task { await transformer.resume() } } + let coordinator = ScanCoordinator(scanService: service, snapshotTransformService: transformer) + coordinator.restoreCompletedSnapshot(baseline) + try #require(coordinator.rescanFolder(id: folder.id)) + + try FileManager.default.moveItem(at: file, to: fixture.appending(path: "moved-fixture")) + let removal = Task { @MainActor in + await coordinator.removeNodesFromCurrentSnapshot([originalFile]) { moved, current in + moved.fileIdentity == current.fileIdentity + || SystemIntegration.verifyTrashIdentity(current) == .missingCurrentItem + } + } + try await transformer.waitUntilPaused() + + service.yield(.finished(alternateScan), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("subtree rescan publishes stale alternate file", timeout: 5) { + coordinator.snapshot?.treeStore.node(id: file.path)?.fileIdentity == resource + } + + await transformer.resume() + let didRemove = await removal.value + #expect(didRemove) + try #require(!FileManager.default.fileExists(atPath: file.path)) + #expect(coordinator.snapshot?.treeStore.node(id: file.path) == nil) + } + @Test func testPausedRemovalCannotOverwriteSameIDExternalSnapshotReplacement() async throws { let removed = makeTestFileNode(id: "/root/removed.dat", name: "removed.dat", size: 10) @@ -1447,7 +1503,7 @@ struct ScanCoordinatorTests { coordinator.replaceCurrentSnapshot(originalSnapshot) let removal = Task { @MainActor in - await coordinator.removeNodesFromCurrentSnapshot(ids: [removed.id]) + await coordinator.removeNodesFromCurrentSnapshot([removed]) } try await transformService.waitUntilPaused() @@ -1515,7 +1571,7 @@ struct ScanCoordinatorTests { scanService.finish(scanIndex: 0) try await transformService.waitUntilPaused() - let didRemove = await coordinator.removeNodesFromCurrentSnapshot(ids: [removed.id]) + let didRemove = await coordinator.removeNodesFromCurrentSnapshot([removed]) await transformService.resume() try await waitUntil("expansion retry") { expansionResult != nil @@ -1667,6 +1723,509 @@ struct ScanCoordinatorTests { #expect(model.sidebar.activeTargetID == firstTarget.id) } + @Test(arguments: [false, true]) + func testAppModelRestoresDiscardPileWhenReturningToSidebarTarget(rescansOnReturn: Bool) async throws { + let service = ControlledScanService() + let firstTarget = makeCoordinatorTarget("/app/sidebar/first") + let secondTarget = makeCoordinatorTarget("/app/sidebar/second") + let model = AppModel( + dependencies: makeCoordinatorAppDependencies( + scanService: service, + systemActions: makeCoordinatorSidebarActions(targets: [firstTarget, secondTarget]) + ), + completedScanCacheMaxTotalNodeCount: rescansOnReturn ? 1 : 100 + ) + let kept = makeTestFileNode( + id: firstTarget.id + "/kept.txt", name: "kept.txt", fileIdentity: FileIdentity(device: 1, inode: 1) + ) + let replaced = makeTestFileNode( + id: firstTarget.id + "/replaced.txt", name: "replaced.txt", fileIdentity: FileIdentity(device: 1, inode: 2) + ) + let replacement = makeTestFileNode( + id: replaced.id, name: replaced.name, fileIdentity: FileIdentity(device: 1, inode: 3) + ) + func firstSnapshot(with files: [FileNodeRecord]) -> ScanSnapshot { + let root = makeTestDirectoryNode(id: firstTarget.id, name: firstTarget.displayName, children: files) + let store = FileTreeStore(root: root, childrenByID: [root.id: files]) + return makeCoordinatorSnapshot(target: firstTarget, root: root, store: store) + } + + model.selectSidebarTarget(id: firstTarget.id) + try await waitUntil("first sidebar scan request") { service.requests.count == 1 } + service.yield(.finished(firstSnapshot(with: [kept, replaced])), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("first sidebar scan finished") { model.scanState.snapshot?.target == firstTarget } + try #require(model.addNodesToDiscardPile([kept, replaced])) + + model.selectSidebarTarget(id: secondTarget.id) + try await waitUntil("second sidebar scan request") { service.requests.count == 2 } + service.yield(.finished(makeCoordinatorSnapshot(target: secondTarget)), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("second sidebar scan finished") { model.scanState.snapshot?.target == secondTarget } + #expect(model.discardPileSnapshot.nodes == [kept, replaced]) + #expect(model.workspaceHiddenNodeIDs.isEmpty) + + model.selectSidebarTarget(id: firstTarget.id) + if rescansOnReturn { + try await waitUntil("first sidebar target rescanned") { service.requests.count == 3 } + service.yield(.finished(firstSnapshot(with: [kept, replacement])), scanIndex: 2) + service.finish(scanIndex: 2) + } + try await waitUntil("first sidebar target restored") { model.scanState.snapshot?.target == firstTarget } + + let expectedIDs = rescansOnReturn ? [kept.id] : [kept.id, replaced.id] + #expect(model.discardPileSnapshot.nodes == [kept, replaced]) + #expect(model.workspaceHiddenNodeIDs == Set(expectedIDs)) + } + + @Test(arguments: [false, true]) + func testMovedSummarizedAncestorClearsDisplayedDescendant(recreated: Bool) async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let parent = ScanTarget(url: fixture.appending(path: "parent", directoryHint: .isDirectory)) + let cacheURL = parent.url.appending(path: "intermediate/cache", directoryHint: .isDirectory) + let child = ScanTarget(url: cacheURL.appending(path: "child", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: child.url, withIntermediateDirectories: true) + let fileURL = child.url.appending(path: "file") + try Data([1]).write(to: fileURL) + let loader = ScanMetadataLoader() + let summarized = FileNodeRecord( + id: cacheURL.path, url: cacheURL, name: "cache", isDirectory: true, + isSymbolicLink: false, allocatedSize: 20, logicalSize: 20, descendantFileCount: 1, lastModified: nil, + fileIdentity: try loader.fileSystemIdentity(at: cacheURL), + isPackage: false, isAccessible: true, isSelfAccessible: true, isSynthetic: false, isAutoSummarized: true + ) + let intermediate = makeTestDirectoryNode( + id: cacheURL.deletingLastPathComponent().path, name: "intermediate", children: [summarized] + ) + let parentRoot = makeTestDirectoryNode(id: parent.id, name: parent.displayName, children: [intermediate]) + let parentStore = FileTreeStore(root: parentRoot, childrenByID: [parent.id: [intermediate], intermediate.id: [summarized]]) + let parentSnapshot = makeCoordinatorSnapshot(target: parent, root: parentRoot, store: parentStore) + let file = makeTestFileNode(id: fileURL.path, name: "file", fileIdentity: try loader.fileSystemIdentity(at: fileURL)) + let childRoot = makeTestDirectoryNode( + id: child.id, name: child.displayName, children: [file], fileIdentity: try loader.fileSystemIdentity(at: child.url) + ) + let childStore = FileTreeStore(root: childRoot, childrenByID: [child.id: [file]]) + let childSnapshot = makeCoordinatorSnapshot(target: child, root: childRoot, store: childStore) + let service = ControlledScanService() + + let move = PausedFixtureTrashMove(destination: fixture.appending(path: "moved-directory")) + var actions = makeCoordinatorSidebarActions(targets: [parent, child]) + actions.fileExists = { FileManager.default.fileExists(atPath: $0.path) } + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + actions.moveToTrash = { node in + let result = try await move.perform(node) + if result == .matches, recreated { + try FileManager.default.createDirectory(at: child.url, withIntermediateDirectories: true) + } + return result + } + let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) + defer { + move.cancel() + model.cleanup() + } + model.selectSidebarTarget(id: parent.id) + try await waitUntil("parent scan starts", timeout: 5) { service.requests.count == 1 } + service.yield(.finished(parentSnapshot), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("parent scan publishes", timeout: 5) { model.scanState.snapshot?.id == parentSnapshot.id } + try #require(model.requestMoveNodesToTrash([summarized])) + model.confirmMovePendingSelectionToTrash() + try await move.waitUntilStarted() + + model.selectSidebarTarget(id: child.id) + try await waitUntil("child scan starts", timeout: 5) { service.requests.count == 2 } + service.yield(.finished(childSnapshot), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("child scan publishes", timeout: 5) { model.scanState.snapshot?.id == childSnapshot.id } + + move.finish() + try await waitUntil("move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try #require(FileManager.default.fileExists(atPath: child.id) == recreated) + if recreated { + let replacementIdentity = try loader.fileSystemIdentity(at: child.url) + try #require(replacementIdentity != childRoot.fileIdentity) + } + #expect(model.scanState.snapshot == nil) + } + + @Test + func testContainedNavigationCanUseUpdatedTreeAfterTrash() async throws { + let service = ControlledScanService() + let parent = makeCoordinatorTarget("/review-again/cache/parent") + let child = makeCoordinatorTarget(parent.id + "/child") + let initial = makeCoordinatorHomeSnapshot(target: parent, downloadsTarget: child, rootName: "parent") + let removed = try #require(initial.treeStore.node(id: parent.id + "/notes.txt")) + var actions = makeCoordinatorSidebarActions(targets: [parent, child]) + actions.fileExists = { _ in true } + actions.moveToTrash = { _ in .matches } + let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) + defer { model.cleanup() } + + model.selectSidebarTarget(id: parent.id) + try await waitUntil("parent scan starts", timeout: 5) { service.requests.count == 1 } + service.yield(.finished(initial), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("parent scan publishes", timeout: 5) { model.scanState.snapshot?.id == initial.id } + + try #require(model.requestMoveNodesToTrash([removed])) + model.confirmMovePendingSelectionToTrash() + try await waitUntil("tree no longer contains moved node", timeout: 5) { + model.scanState.snapshot?.id == initial.id && model.scanState.fileTreeStore?.node(id: removed.id) == nil + } + + model.selectSidebarTarget(id: child.id) + try await waitUntil("child display or fresh scan", timeout: 5) { + model.scanState.snapshot?.target == child || service.requests.count > 1 + } + #expect(service.requests.count == 1) + #expect(model.scanState.snapshot?.target == child) + } + + @Test + func testTrashCompletingBeforeOverlappingFullScanPublishes() async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let parent = ScanTarget(url: fixture.appending(path: "parent", directoryHint: .isDirectory)) + let child = ScanTarget(url: parent.url.appending(path: "child", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: child.url, withIntermediateDirectories: true) + let fileURL = child.url.appending(path: "marked.txt") + try Data([1]).write(to: fileURL) + let identity = try ScanMetadataLoader().fileSystemIdentity(at: fileURL) + let marked = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: identity) + let childRoot = makeTestDirectoryNode(id: child.id, name: child.displayName, children: [marked]) + let childStore = FileTreeStore(root: childRoot, childrenByID: [child.id: [marked]]) + let childSnapshot = makeCoordinatorSnapshot(target: child, root: childRoot, store: childStore) + let parentRoot = makeTestDirectoryNode(id: parent.id, name: parent.displayName, children: [childRoot]) + let parentStore = FileTreeStore(root: parentRoot, childrenByID: [parent.id: [childRoot], child.id: [marked]]) + let parentSnapshot = makeCoordinatorSnapshot(target: parent, root: parentRoot, store: parentStore) + let service = ControlledScanService() + + let move = PausedFixtureTrashMove(destination: fixture.appending(path: "moved-file")) + var actions = makeCoordinatorSidebarActions(targets: [parent, child]) + actions.fileExists = { FileManager.default.fileExists(atPath: $0.path) } + actions.moveToTrash = move.perform + let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) + defer { + move.cancel() + model.cleanup() + } + model.selectSidebarTarget(id: child.id) + try await waitUntil("child scan starts", timeout: 5) { service.requests.count == 1 } + service.yield(.finished(childSnapshot), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("child scan publishes", timeout: 5) { model.scanState.snapshot?.id == childSnapshot.id } + try #require(model.addNodesToDiscardPile([marked])) + try #require(await model.requestMoveDiscardPileToTrash()) + model.confirmMovePendingSelectionToTrash() + try await move.waitUntilStarted() + + model.selectSidebarTarget(id: parent.id) + try await waitUntil("parent scan reads file before move finishes", timeout: 5) { service.requests.count == 2 } + try #require(model.scanState.snapshot == nil) + move.finish() + try await waitUntil("move finishes before scan publishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try #require(!FileManager.default.fileExists(atPath: fileURL.path)) + + service.yield(.finished(parentSnapshot), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("parent scan publishes or restarts", timeout: 5) { + model.scanState.snapshot?.target == parent || service.requests.count > 2 + } + if service.requests.count == 3 { + let refreshed = try #require(parentSnapshot.removingNode(id: marked.id)) + service.yield(.finished(refreshed), scanIndex: 2) + service.finish(scanIndex: 2) + } + try await waitUntil("parent scan is displayed", timeout: 5) { + model.scanState.snapshot?.target == parent + } + #expect(model.scanState.snapshot?.treeStore.node(id: marked.id) == nil) + } + + @Test + func testTrashRestartsInFlightFullFallbackWithoutRepublishingMovedFile() async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let target = ScanTarget(url: fixture) + let child = ScanTarget(url: fixture.appending(path: "Changed", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: child.url, withIntermediateDirectories: true) + let fileURL = fixture.appending(path: "marked.txt") + try Data([1]).write(to: fileURL) + let identity = try ScanMetadataLoader().fileSystemIdentity(at: fileURL) + let moved = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: identity) + let keptURL = fixture.appending(path: "kept.txt") + try Data([2]).write(to: keptURL) + let kept = makeTestFileNode( + id: keptURL.path, name: "kept.txt", fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: keptURL) + ) + let cloneIdentity = CloneIdentity(device: 1, cloneID: 42) + let clone = makeTestFileNode( + id: child.id + "/shared.dat", name: "shared.dat", size: 20, + cloneIdentity: cloneIdentity, mayShareDataBlocks: true + ) + let siblingClone = makeTestFileNode( + id: target.id + "/shared.dat", name: "shared.dat", size: 0, unduplicatedAllocatedSize: 20, + cloneIdentity: cloneIdentity, mayShareDataBlocks: true + ) + let childRoot = makeTestDirectoryNode(id: child.id, name: child.displayName, children: [clone]) + let root = makeTestDirectoryNode(id: target.id, name: target.displayName, children: [childRoot, siblingClone, moved, kept]) + let tree = FileTreeStore(root: root, childrenByID: [root.id: [childRoot, siblingClone, moved, kept], childRoot.id: [clone]]) + let options = ScanOptions() + let baseline = makeCoordinatorSnapshot(target: target, root: root, store: tree, scanOptions: options) + let staleFullScan = makeCoordinatorSnapshot(target: target, root: root, store: tree, scanOptions: options) + let service = ControlledScanService() + let move = PausedFixtureTrashMove(destination: fixture.appending(path: "moved-file")) + var actions = makeCoordinatorSidebarActions(targets: [target]) + actions.fileExists = { FileManager.default.fileExists(atPath: $0.path) } + actions.moveToTrash = move.perform + let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) + defer { + move.cancel() + model.cleanup() + } + model.selectSidebarTarget(id: target.id) + try await waitUntil("initial scan starts", timeout: 5) { service.requests.count == 1 } + service.yield(.finished(baseline), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("initial scan publishes", timeout: 5) { model.scanState.snapshot?.id == baseline.id } + try #require(model.addNodesToDiscardPile([kept])) + try #require(model.requestMoveNodesToTrash([moved])) + model.confirmMovePendingSelectionToTrash() + try await move.waitUntilStarted() + try #require(model.scanState.rescanFolder(id: child.id)) + let changed = makeTestFileNode( + id: clone.id, name: clone.name, size: 40, cloneIdentity: cloneIdentity, mayShareDataBlocks: true + ) + let changedRoot = makeTestDirectoryNode(id: child.id, name: child.displayName, children: [changed]) + let replacement = makeCoordinatorSnapshot( + target: child, root: changedRoot, store: FileTreeStore(root: changedRoot, childrenByID: [child.id: [changed]]) + ) + service.yield(.finished(replacement), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("manual rescan falls back to a full scan", timeout: 5) { + service.requests.count == 3 && model.scanState.progress.executionMode == .fullFallback(.sharedAllocationTopologyChanged) + } + try #require(model.scanState.snapshot?.id == baseline.id) + + move.finish() + try await waitUntil("move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try #require(!FileManager.default.fileExists(atPath: moved.id)) + service.yield(.finished(staleFullScan), scanIndex: 2) + service.finish(scanIndex: 2) + try await waitUntil("full scan finishes or restarts", timeout: 5) { + !model.scanState.isScanOperationInProgress || service.requests.count > 3 + } + if service.requests.count == 4 { + let refreshed = try #require(staleFullScan.removingNode(id: moved.id)) + service.yield(.finished(refreshed), scanIndex: 3) + service.finish(scanIndex: 3) + } + try await waitUntil("updated scan publishes", timeout: 5) { model.scanState.phase == .displaying } + #expect(model.scanState.snapshot?.treeStore.node(id: moved.id) == nil) + #expect(model.discardPileSnapshot.nodes.map(\.id) == [kept.id]) + } + + @Test + func testCompletedTrashUpdatesOverlappingSidebarView() async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let parent = ScanTarget(url: fixture.appending(path: "parent", directoryHint: .isDirectory)) + let scope = ScanTarget(url: parent.url.appending(path: "scope", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: scope.url, withIntermediateDirectories: true) + let fileURL = scope.url.appending(path: "marked.txt") + try Data([1]).write(to: fileURL) + let identity = try ScanMetadataLoader().fileSystemIdentity(at: fileURL) + let marked = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: identity) + let scopedRoot = makeTestDirectoryNode(id: scope.id, name: scope.displayName, children: [marked]) + let root = makeTestDirectoryNode(id: parent.id, name: parent.displayName, children: [scopedRoot]) + let store = FileTreeStore(root: root, childrenByID: [root.id: [scopedRoot], scopedRoot.id: [marked]]) + let initial = makeCoordinatorSnapshot(target: parent, root: root, store: store) + let service = ControlledScanService() + + let move = PausedFixtureTrashMove(destination: fixture.appending(path: "moved-file")) + var actions = makeCoordinatorSidebarActions(targets: [parent, scope]) + actions.fileExists = { FileManager.default.fileExists(atPath: $0.path) } + actions.moveToTrash = move.perform + let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) + defer { + move.cancel() + model.cleanup() + } + + model.selectSidebarTarget(id: parent.id) + try await waitUntil("parent scan request", timeout: 5) { service.requests.count == 1 } + service.yield(.finished(initial), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("parent finishes", timeout: 5) { model.scanState.snapshot?.id == initial.id } + + model.selectSidebarTarget(id: scope.id) + try await waitUntil("scope displays", timeout: 5) { model.scanState.snapshot?.target == scope } + try #require(model.addNodesToDiscardPile([marked])) + + model.selectSidebarTarget(id: parent.id) + try await waitUntil("parent displays", timeout: 5) { model.scanState.snapshot?.id == initial.id } + try #require(model.addNodesToDiscardPile([marked])) + try #require(await model.requestMoveDiscardPileToTrash()) + model.confirmMovePendingSelectionToTrash() + try await move.waitUntilStarted() + + model.selectSidebarTarget(id: scope.id) + try await waitUntil("scope displays again", timeout: 5) { model.scanState.snapshot?.target == scope } + try #require(model.discardPileSnapshot.nodes.map(\.id) == [marked.id]) + try #require(model.workspaceHiddenNodeIDs.contains(marked.id)) + + move.finish() + try await waitUntil("confirmed move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try #require(!FileManager.default.fileExists(atPath: fileURL.path)) + try #require(model.discardPileSnapshot.nodes.isEmpty) + + if model.scanState.fileTreeStore?.node(id: marked.id) != nil { + #expect(model.workspaceHiddenNodeIDs.contains(marked.id)) + } + try await waitUntil("moved file is removed from the overlapping scan", timeout: 5) { + model.scanState.fileTreeStore?.node(id: marked.id) == nil + } + } + + @Test + func testRestoredMarkRejectsReplacementAfterFolderRefresh() async throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let target = ScanTarget(url: fixture.appending(path: "first", directoryHint: .isDirectory)) + let other = ScanTarget(url: fixture.appending(path: "second", directoryHint: .isDirectory)) + let folder = ScanTarget(url: target.url.appending(path: "Folder", directoryHint: .isDirectory)) + try FileManager.default.createDirectory(at: folder.url, withIntermediateDirectories: true) + let fileURL = folder.url.appending(path: "marked.txt") + try Data([1]).write(to: fileURL) + let oldIdentity = try ScanMetadataLoader().fileSystemIdentity(at: fileURL) + + let service = ControlledScanService() + var actions = makeCoordinatorSidebarActions(targets: [target, other]) + actions.fileExists = { FileManager.default.fileExists(atPath: $0.path) } + actions.verifyTrashIdentity = SystemIntegration.verifyTrashIdentity + var preferences = AppPreferences.defaults + preferences.didCompleteOnboarding = true + let model = AppModel(dependencies: makeCoordinatorAppDependencies( + preferences: TestAppPreferencesStore(preferences: preferences), scanService: service, systemActions: actions + )) + defer { model.cleanup() } + let marked = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: oldIdentity) + let initialFolder = makeTestDirectoryNode(id: folder.id, name: folder.displayName, children: [marked]) + let root = makeTestDirectoryNode(id: target.id, name: target.displayName, children: [initialFolder]) + let initial = makeCoordinatorSnapshot( + target: target, root: root, + store: FileTreeStore(root: root, childrenByID: [root.id: [initialFolder], initialFolder.id: [marked]]), + scanOptions: ScanOptions() + ) + model.selectSidebarTarget(id: target.id) + try await waitUntil("initial scan request") { service.requests.count == 1 } + service.yield(.finished(initial), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("initial scan finishes") { model.scanState.snapshot?.id == initial.id } + try #require(model.addNodesToDiscardPile([marked])) + + model.selectSidebarTarget(id: other.id) + try await waitUntil("other scan request") { service.requests.count == 2 } + service.yield(.finished(makeCoordinatorSnapshot(target: other)), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("other scan finishes") { model.scanState.snapshot?.target == other } + + try FileManager.default.moveItem(at: fileURL, to: fixture.appending(path: "old-file")) + try Data([2]).write(to: fileURL) + let newIdentity = try ScanMetadataLoader().fileSystemIdentity(at: fileURL) + try #require(oldIdentity != newIdentity) + + model.selectSidebarTarget(id: target.id) + try await waitUntil("initial snapshot restores from cache") { model.scanState.snapshot?.id == initial.id } + try #require(model.discardPileSnapshot.nodes.map(\.fileIdentity) == [oldIdentity]) + try #require(model.scanState.refreshFolderInBackground(id: folder.id)) + let replacement = makeTestFileNode(id: fileURL.path, name: marked.name, fileIdentity: newIdentity) + let replacementFolder = makeTestDirectoryNode(id: folder.id, name: folder.displayName, children: [replacement]) + let replacementScan = makeCoordinatorSnapshot( + target: folder, root: replacementFolder, + store: FileTreeStore(root: replacementFolder, childrenByID: [replacementFolder.id: [replacement]]) + ) + service.yield(.finished(replacementScan), scanIndex: 2) + service.finish(scanIndex: 2) + try await waitUntil("replacement folder refresh publishes", timeout: 5) { + model.scanState.snapshot?.treeStore.node(id: marked.id)?.fileIdentity == newIdentity + } + try #require(model.scanState.snapshot?.id == initial.id) + #expect(model.discardPileSnapshot.nodes == [marked]) + #expect(model.workspaceHiddenNodeIDs.isEmpty) + #expect(await model.requestMoveDiscardPileToTrash()) + #expect(model.pendingTrashSelection?.nodes.isEmpty == true) + #expect(model.pendingTrashSelection?.discardPileBatch?.unavailable.map(\.node) == [marked]) + } + + @Test + func testTemporarilySummarizedFileRestoresWhenVisibleAgain() async throws { + let service = ControlledScanService() + let target = makeCoordinatorTarget("/review/summary/first") + let other = makeCoordinatorTarget("/review/summary/second") + let model = AppModel( + dependencies: makeCoordinatorAppDependencies( + scanService: service, systemActions: makeCoordinatorSidebarActions(targets: [target, other]) + ), + completedScanCacheMaxTotalNodeCount: 1 + ) + defer { model.cleanup() } + let summarized = makeCoordinatorSummarizedDirectoryNode(id: target.id + "/parent/cache", name: "cache", size: 300) + let parent = makeTestDirectoryNode(id: target.id + "/parent", name: "parent", children: [summarized]) + let root = makeTestDirectoryNode(id: target.id, name: target.displayName, children: [parent]) + let collapsed = FileTreeStore(root: root, childrenByID: [root.id: [parent], parent.id: [summarized]]) + let marked = makeTestFileNode( + id: summarized.id + "/marked.txt", name: "marked.txt", size: 300, + fileIdentity: FileIdentity(device: 1, inode: 10) + ) + let expanded = makeTestDirectoryNode(id: summarized.id, name: summarized.name, children: [marked]) + let subtree = makeCoordinatorSnapshot( + target: makeCoordinatorTarget(summarized.id), root: expanded, + store: FileTreeStore(root: expanded, childrenByID: [expanded.id: [marked]]) + ) + + model.selectSidebarTarget(id: target.id) + try await waitUntil("initial summary scan request") { service.requests.count == 1 } + service.yield(.finished(makeCoordinatorSnapshot(target: target, root: root, store: collapsed)), scanIndex: 0) + service.finish(scanIndex: 0) + try await waitUntil("initial summary scan finishes") { model.scanState.snapshot?.target == target } + model.expandSummarizedNode(summarized) {} + try await waitUntil("initial expansion request") { service.requests.count == 2 } + service.yield(.finished(subtree), scanIndex: 1) + service.finish(scanIndex: 1) + try await waitUntil("marked file visible after initial expansion") { + model.scanState.fileTreeStore?.node(id: marked.id) != nil + } + try #require(model.addNodesToDiscardPile([marked])) + + model.selectSidebarTarget(id: other.id) + try await waitUntil("other scan request") { service.requests.count == 3 } + service.yield(.finished(makeCoordinatorSnapshot(target: other)), scanIndex: 2) + service.finish(scanIndex: 2) + try await waitUntil("other scan finishes") { model.scanState.snapshot?.target == other } + + model.selectSidebarTarget(id: target.id) + try await waitUntil("first target scans again after cache eviction") { service.requests.count == 4 } + service.yield(.finished(makeCoordinatorSnapshot(target: target, root: root, store: collapsed)), scanIndex: 3) + service.finish(scanIndex: 3) + try await waitUntil("fresh summary scan finishes") { model.scanState.snapshot?.target == target } + try #require(model.scanState.fileTreeStore?.node(id: summarized.id)?.isAutoSummarized == true) + try #require(model.scanState.fileTreeStore?.node(id: marked.id) == nil) + #expect(model.discardPileSnapshot.nodes == [marked]) + #expect(!model.workspaceHiddenNodeIDs.contains(marked.id)) + + model.expandSummarizedNode(summarized) {} + try await waitUntil("second expansion request") { service.requests.count == 5 } + service.yield(.finished(subtree), scanIndex: 4) + service.finish(scanIndex: 4) + try await waitUntil("same file appears after second expansion") { + model.scanState.fileTreeStore?.node(id: marked.id)?.fileIdentity == marked.fileIdentity + } + #expect(model.discardPileSnapshot.nodes.map(\.id) == [marked.id]) + } + @Test func testAppModelKeepsSmallCachedSidebarTargetsBeyondTwoScans() async throws { let service = ControlledScanService() @@ -3007,6 +3566,41 @@ private actor PausingSnapshotTransformService: ScanSnapshotTransforming { } } +@MainActor +private final class PausedFixtureTrashMove { + private let destination: URL + private var continuation: CheckedContinuation? + + init(destination: URL) { + self.destination = destination + } + + func perform(_ node: FileNodeRecord) async throws -> TrashIdentityVerificationResult { + let proceed = await withCheckedContinuation { continuation = $0 } + guard proceed else { throw CancellationError() } + + let verification = SystemIntegration.verifyTrashIdentity(node) + if verification == .matches { + try FileManager.default.moveItem(at: node.url, to: destination) + } + return verification + } + + func waitUntilStarted() async throws { + try await waitUntil("move is in flight", timeout: 5) { self.continuation != nil } + } + + func finish() { + continuation?.resume(returning: true) + continuation = nil + } + + func cancel() { + continuation?.resume(returning: false) + continuation = nil + } +} + private final class ControlledScanService: ScanEventStreaming, @unchecked Sendable { private typealias Continuation = AsyncThrowingStream.Continuation diff --git a/RadixCoreTests/ScanModelTests.swift b/RadixCoreTests/ScanModelTests.swift index 127a96c..ac7d84d 100644 --- a/RadixCoreTests/ScanModelTests.swift +++ b/RadixCoreTests/ScanModelTests.swift @@ -616,17 +616,6 @@ struct ScanModelTests { snapshot.scoped(to: ScanTarget(url: URL(filePath: "/root/Missing", directoryHint: .isDirectory))) == nil) } - @Test - func testPostTrashActionMatchesCurrentSelectionPolicy() { - #expect( - ScanPostTrashAction.afterRemovingNode(activeTargetID: "/scan/root", removedNodeID: "/scan/root") - == .clearActiveScan) - #expect( - ScanPostTrashAction.afterRemovingNode(activeTargetID: "/scan/root", removedNodeID: "/scan/root/file.txt") - == .removeFromActiveScan) - #expect(ScanPostTrashAction.afterRemovingNode(activeTargetID: nil, removedNodeID: "/scan/root") == .none) - } - @Test func testSnapshotReplacingNodeDeduplicatesWarningsByContent() throws { let child = makeNode(id: "/root/folder", isDirectory: true, isSynthetic: false, isAccessible: true) diff --git a/RadixCoreTests/SidebarScanCacheControllerTests.swift b/RadixCoreTests/SidebarScanCacheControllerTests.swift index 4e571d7..63d9130 100644 --- a/RadixCoreTests/SidebarScanCacheControllerTests.swift +++ b/RadixCoreTests/SidebarScanCacheControllerTests.swift @@ -846,6 +846,58 @@ struct SidebarScanCacheControllerTests { #expect(recorder.startedTargets.isEmpty) } + @Test + func testScopeRequiresFreshScanWhileDisplayedTreeStillContainsMovedFile() throws { + let controller = SidebarScanCacheController(maxTotalNodeCount: 100) + let tree = makeParentAndChildSnapshot() + let options = ScanOptions() + controller.prepareForScanStart(target: tree.snapshot.target, options: options) + controller.handleCompletedScanSnapshot(tree.snapshot) + let removed = try #require(tree.snapshot.treeStore.node(id: "/cache/root/child/file.txt")) + controller.clearCache() + controller.invalidateDisplayedNodes([removed], in: tree.snapshot) + + let requiresScan = controller.applyCachedOrContainedSidebarTarget( + tree.childTarget, + options: options, + currentSnapshot: tree.snapshot, + isTargetActive: { _ in true }, + cancelDeferredScanStart: {}, + restoreSnapshot: { _, _ in Issue.record("A stale tree cannot be restored") }, + startScan: { _ in Issue.record("The caller starts the fresh scan") } + ) + #expect(requiresScan) + } + + @Test + func testScopePendingWhenCacheIsInvalidatedStartsFreshScan() async throws { + let transform = PausingCacheScopeTransform() + let controller = SidebarScanCacheController(maxTotalNodeCount: 100, snapshotTransformService: transform) + let tree = makeParentAndChildSnapshot() + let options = ScanOptions() + controller.prepareForScanStart(target: tree.snapshot.target, options: options) + controller.handleCompletedScanSnapshot(tree.snapshot) + let recorder = SidebarScanCacheRecorder() + controller.applyCachedOrContainedSidebarTarget( + tree.childTarget, + options: options, + currentSnapshot: tree.snapshot, + isTargetActive: { _ in true }, + cancelDeferredScanStart: {}, + restoreSnapshot: { snapshot, _ in recorder.restoredSnapshots.append(snapshot) }, + startScan: { target in recorder.startedTargets.append(target) } + ) + + try await waitUntil("cache scope transform paused", timeout: 5) { await transform.isPaused } + controller.clearCache() + await transform.resume() + try await waitUntil("sidebar target resolves", timeout: 5) { + !recorder.restoredSnapshots.isEmpty || !recorder.startedTargets.isEmpty + } + #expect(recorder.restoredSnapshots.isEmpty) + #expect(recorder.startedTargets == [tree.childTarget]) + } + @Test func testPendingGrandchildScopeUsesCanonicalParentAfterChildCacheIsRemoved() async throws { let firstFile = makeTestFileNode(id: "/cache/root/child/grandchild/first.txt", name: "first.txt") diff --git a/RadixCoreTests/TrashFlowControllerTests.swift b/RadixCoreTests/TrashFlowControllerTests.swift index 3996485..81d80ce 100644 --- a/RadixCoreTests/TrashFlowControllerTests.swift +++ b/RadixCoreTests/TrashFlowControllerTests.swift @@ -5,35 +5,118 @@ import Testing @MainActor struct TrashFlowControllerTests { + @Test(arguments: [false, true]) + func testDiscardPileProjectsAcrossEnumeratorsOnlyForTheOriginalFile(replaced: Bool) throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let fileURL = fixture.appending(path: "marked.txt") + try Data([1]).write(to: fileURL) + let loader = ScanMetadataLoader() + let bulkResult = try BulkDirectoryEnumerator.directoryEntries( + at: fixture, includeHiddenFiles: false, metadataLoader: loader, cancellationCheck: {} + ) + let bulk = try #require(bulkResult) + let nativeIdentity = try #require(bulk.entries.first?.metadata?.fileIdentity) + if replaced { + try FileManager.default.moveItem(at: fileURL, to: fixture.appending(path: "original.txt")) + try Data([2]).write(to: fileURL) + } + let metadata = try loader.metadata(for: fileURL) + let foundationIdentity = try #require(metadata.fileIdentity) + let nativeNode = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: nativeIdentity) + let foundationNode = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: foundationIdentity) + #expect(SystemIntegration.verifyTrashIdentity(nativeNode) == (replaced ? .mismatch : .matches)) + #expect(SystemIntegration.verifyTrashIdentity(foundationNode) == .matches) + try #require(nativeIdentity != foundationIdentity) + + func snapshot(_ node: FileNodeRecord) -> ScanSnapshot { + let root = makeTestDirectoryNode(id: fixture.path, name: "fixture", children: [node]) + return makeTestSnapshot(root: root, store: FileTreeStore(root: root, childrenByID: [root.id: [node]])) + } + let initial = snapshot(nativeNode) + let controller = TrashFlowController() + controller.changeDiscardPile(controller.discardPileAdding([nativeNode], in: initial), actionName: "Add") + let refreshed = snapshot(foundationNode) + + #expect(controller.discardPile.matchingNodeIDs(in: refreshed) == (replaced ? [] : [nativeNode.id])) + #expect(controller.discardPile.nodes == [nativeNode]) + let preview = try TrashFlowController.prepareDiscardPileTrashRequest( + nodes: controller.discardPile.nodes, trashSafetyPolicy: .live(), + verifyIdentity: SystemIntegration.verifyTrashIdentity + ) + #expect(preview.nodes == (replaced ? [] : [nativeNode])) + #expect(preview.discardPileBatch?.unavailable.map(\.node) == (replaced ? [nativeNode] : [])) + } + + @Test + func testDiscardPileBatchCollapsesVerifiedDescendantsAndRetainsUnavailableMarks() throws { + let identity = FileIdentity(device: 1, inode: 1) + let folder = makeTestDirectoryNode(id: "/scope/folder", name: "folder", children: [], fileIdentity: identity) + let child = makeTestFileNode( + id: "/scope/folder/deep/file", name: "file", fileIdentity: FileIdentity(device: 1, inode: 2) + ) + let unavailable = makeTestFileNode( + id: "/scope/folder/deep/missing", name: "missing", fileIdentity: FileIdentity(device: 1, inode: 3) + ) + let sibling = makeTestFileNode( + id: "/scope/folder-other/file", name: "file", fileIdentity: FileIdentity(device: 1, inode: 4) + ) + let controller = TrashFlowController( + discardPile: DiscardPileState(marks: [child, folder, unavailable, sibling].map { .init(node: $0) }) + ) + + let preview = try TrashFlowController.prepareDiscardPileTrashRequest( + nodes: controller.discardPile.nodes, trashSafetyPolicy: .live() + ) { + $0 == unavailable ? .missingCurrentItem : .matches + } + #expect(preview.nodes == [folder, sibling]) + #expect(preview.discardPileBatch?.unavailable.map(\.node) == [unavailable]) + controller.removeMovedDiscardPileNodes([folder], batch: preview.discardPileBatch, from: nil) + #expect(controller.discardPile.nodes == [unavailable, sibling]) + } + @Test func testDiscardPileRemovalAndClearRestoreExactOrderedBatches() { - let snapshotID = UUID() - let original = DiscardPileState(nodeIDs: ["first", "second", "third"], snapshotID: snapshotID) + let original = makePile(["first", "second", "third"]) let controller = TrashFlowController(discardPile: original) - controller.removeDiscardPileNodes(ids: ["first", "third"]) - #expect(controller.discardPile.nodeIDs == ["second"]) + controller.removeDiscardPileNodes(ids: ["/first", "/third"]) + #expect(controller.discardPile == makePile(["second"])) controller.discardPileUndoManager.undo() #expect(controller.discardPile == original) controller.discardPileUndoManager.redo() - #expect(controller.discardPile.nodeIDs == ["second"]) + #expect(controller.discardPile == makePile(["second"])) controller.clearDiscardPile() #expect(controller.discardPile.isEmpty) - #expect(controller.discardPile.snapshotID == nil) controller.discardPileUndoManager.undo() - #expect(controller.discardPile == DiscardPileState(nodeIDs: ["second"], snapshotID: snapshotID)) + #expect(controller.discardPile == makePile(["second"])) controller.discardPileUndoManager.redo() #expect(controller.discardPile.isEmpty) } + @Test + func testCompletedDirectTrashMovePrunesOnlyMovedItems() { + let child = makeTestFileNode(id: "/selection/folder/child", name: "child") + let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child]) + let kept = makeTestFileNode(id: "/selection/kept", name: "kept") + let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder, kept]) + let store = FileTreeStore(root: root, childrenByID: [root.id: [folder, kept], folder.id: [child]]) + let snapshot = makeTestSnapshot(root: root, store: store) + let controller = TrashFlowController() + controller.changeDiscardPile(controller.discardPileAdding([child, kept], in: snapshot), actionName: "Add") + + controller.removeMovedDiscardPileNodes([folder], batch: nil, from: snapshot) + + #expect(controller.discardPile.nodeIDs == [kept.id]) + } + @Test func testNoOpEditsKeepRedoAndNewEditReplacesIt() { - let snapshotID = UUID() let controller = TrashFlowController() - controller.synchronizeDiscardPileContext(snapshotID: snapshotID, treeContentID: UUID()) - let first = DiscardPileState(nodeIDs: ["first"], snapshotID: snapshotID) + let first = makePile(["first"]) controller.changeDiscardPile(first, actionName: "Add") controller.changeDiscardPile(first, actionName: "Duplicate") - controller.removeDiscardPileNodes(ids: ["unknown"]) + controller.removeDiscardPileNodes(ids: ["/unknown"]) controller.discardPileUndoManager.undo() #expect(controller.discardPile.isEmpty) #expect(!(controller.discardPileUndoManager.canUndo)) @@ -41,9 +124,7 @@ struct TrashFlowControllerTests { controller.clearDiscardPile() controller.removeDiscardPileNodes(ids: []) #expect(controller.discardPileUndoManager.canRedo) - controller.changeDiscardPile( - DiscardPileState(nodeIDs: ["second"], snapshotID: snapshotID), actionName: "Add" - ) + controller.changeDiscardPile(makePile(["second"]), actionName: "Add") #expect(!(controller.discardPileUndoManager.canRedo)) controller.discardPileUndoManager.undo() #expect(controller.discardPile.isEmpty) @@ -51,14 +132,9 @@ struct TrashFlowControllerTests { @Test func testDiscardPileHistoryIsBoundedToFiftyUserEdits() { - let snapshotID = UUID() let controller = TrashFlowController() - controller.synchronizeDiscardPileContext(snapshotID: snapshotID, treeContentID: UUID()) for count in 1...60 { - controller.changeDiscardPile( - DiscardPileState(nodeIDs: (1...count).map(String.init), snapshotID: snapshotID), - actionName: "Add" - ) + controller.changeDiscardPile(makePile((1...count).map(String.init)), actionName: "Add") } var undoCount = 0 while controller.discardPileUndoManager.canUndo { @@ -72,27 +148,21 @@ struct TrashFlowControllerTests { } @Test - func testProgrammaticAndEmptyPileContextReplacementsInvalidateHistory() { - let snapshotID = UUID() - let contentID = UUID() - let original = DiscardPileState(nodeIDs: ["first"], snapshotID: snapshotID) + func testProgrammaticPileReplacementInvalidatesHistory() { let controller = TrashFlowController() - controller.synchronizeDiscardPileContext(snapshotID: snapshotID, treeContentID: contentID) - controller.changeDiscardPile(original, actionName: "Add") - controller.clearDiscardPile() - controller.synchronizeDiscardPileContext(snapshotID: snapshotID, treeContentID: contentID) - #expect(controller.discardPileUndoManager.canUndo) - controller.synchronizeDiscardPileContext(snapshotID: UUID(), treeContentID: UUID()) - #expect(!(controller.discardPileUndoManager.canUndo)) - #expect(!(controller.discardPileUndoManager.canRedo)) - controller.synchronizeDiscardPileContext(snapshotID: snapshotID, treeContentID: contentID) - controller.changeDiscardPile(original, actionName: "Add") + controller.changeDiscardPile(makePile(["first"]), actionName: "Add") controller.discardPileUndoManager.undo() #expect(controller.discardPileUndoManager.canRedo) controller.replaceDiscardPile(DiscardPileState()) #expect(!(controller.discardPileUndoManager.canRedo)) } + private func makePile(_ names: [String]) -> DiscardPileState { + DiscardPileState(marks: names.map { + .init(node: makeTestFileNode(id: "/" + $0, name: $0)) + }) + } + @Test func testReadOnlyModeRejectsDirectTrashRequestsAndConfirmedMoves() throws { let controller = TrashFlowController() From ce78390ee605d12942b6caf48a86e365f5852304 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 22:02:00 -0800 Subject: [PATCH 03/12] fix(discard-pile): keep trash batches stable Collapse alternate paths to one directory entry while keeping distinct hard links and symlinks. Preserve preparation across live scan updates and reuse the scan projection when adding marks. --- Radix/ViewModels/AppModel.swift | 20 +++---- Radix/ViewModels/TrashFlowController.swift | 60 ++++++++++++------- RadixCoreTests/AppModelDependencyTests.swift | 17 ++++-- RadixCoreTests/TrashFlowControllerTests.swift | 46 +++++++++++++- 4 files changed, 103 insertions(+), 40 deletions(-) diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index 385231b..50a3d7a 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -414,12 +414,10 @@ final class AppModel: ObservableObject { self.objectWillChange.send() self.fileTransferReconciliation.resumeIfPossible() } - trashFlow.$discardPile - .sink { [weak self] pile in - guard let self else { return } - projectedDiscardPileIDs = pile.matchingNodeIDs(in: scanCoordinator.snapshot) - } - .store(in: &cancellables) + trashFlow.onDiscardPileChange = { [weak self] pile, matchingNodeIDs in + guard let self else { return } + projectedDiscardPileIDs = matchingNodeIDs ?? pile.matchingNodeIDs(in: scanCoordinator.snapshot) + } trashFlow.onDiscardPileHistoryReplay = { [weak self] _ in guard let self, let fileTreeStore = self.scanCoordinator.fileTreeStore else { return } self.reconcileNavigationForHiddenNodes( @@ -2503,7 +2501,6 @@ final class AppModel: ObservableObject { let pile = discardPile let nodes = pile.nodes - let snapshotID = scanCoordinator.snapshot?.id let policy = scanCoordinator.trashSafetyPolicy let verifyIdentity = dependencies.systemActions.verifyTrashIdentity let id = UUID() @@ -2527,7 +2524,7 @@ final class AppModel: ObservableObject { task.cancel() } guard !Task.isCancelled, discardPilePreparation?.id == id, - discardPile == pile, scanCoordinator.snapshot?.id == snapshotID, + discardPile == pile, canUseWorkspaceCommands, !trashFlow.isMovingFiles, pendingTrashSelection == nil, pendingCloudFileAction == nil else { return false } try validateSnapshotAllowsMutation() @@ -3066,15 +3063,16 @@ final class AppModel: ObservableObject { _ nodes: [FileNodeRecord], snapshot: ScanSnapshot ) { - let updatedPile = trashFlow.discardPileAdding(nodes, in: snapshot) + let (updatedPile, matchingNodeIDs) = trashFlow.discardPileAdding(nodes, in: snapshot) reconcileNavigationForHiddenNodes( - hiddenNodeIDs: hiddenNodeIDs(for: snapshot.id).union(updatedPile.matchingNodeIDs(in: snapshot)), + hiddenNodeIDs: hiddenNodeIDs(for: snapshot.id).union(matchingNodeIDs), fileTreeStore: snapshot.treeStore, preservingSelection: navigationModel.selectedNodeIDs.count == 1 ) trashFlow.changeDiscardPile( updatedPile, - actionName: String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion.") + actionName: String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion."), + matchingNodeIDs: matchingNodeIDs ) } diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index f0809c3..fb5dd45 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -106,6 +106,7 @@ final class TrashFlowController { @Published private(set) var discardPile: DiscardPileState { didSet { notifyChanged() } } + var onDiscardPileChange: ((DiscardPileState, Set?) -> Void)? let discardPileUndoManager = UndoManager() var onDiscardPileHistoryReplay: ((DiscardPileState) -> Void)? @@ -215,7 +216,7 @@ final class TrashFlowController { guard !nodes.isEmpty else { throw FileActionError.noSelection } var unavailable: [DiscardPileTrashBatch.Unavailable] = [] - var eligible: [FileNodeRecord] = [] + var eligible: [(node: FileNodeRecord, path: String)] = [] for node in nodes { try Task.checkCancellation() let message: String? @@ -233,29 +234,26 @@ final class TrashFlowController { if let message { unavailable.append(.init(node: node, message: message)) } else { - eligible.append(node) + let parent = node.url.deletingLastPathComponent().resolvingSymlinksInPath() + eligible.append((node, parent.appending(path: node.url.lastPathComponent).path)) } } - var foldersByPath: [String: FileNodeRecord.ID] = [:] - for folder in eligible where folder.isDirectory && !folder.isSymbolicLink { - try Task.checkCancellation() - let path = folder.url.resolvingSymlinksInPath().path - if foldersByPath[path] == nil { foldersByPath[path] = folder.id } - } + let entriesByPath = Dictionary(eligible.map { ($0.path, $0.node) }, uniquingKeysWith: { first, _ in first }) var coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] = [:] - let operations = try eligible.filter { node in + let operations = try eligible.compactMap { node, path -> FileNodeRecord? in try Task.checkCancellation() - guard !foldersByPath.isEmpty else { return true } - var parent = node.url.deletingLastPathComponent().resolvingSymlinksInPath().path - var coveringID: FileNodeRecord.ID? - while let separator = parent.lastIndex(of: "/"), parent != "/" { - if let ancestor = foldersByPath[parent] { coveringID = ancestor } + var coveringID = entriesByPath[path]?.id ?? node.id + var parent = path + while let separator = parent.lastIndex(of: "/"), separator != parent.startIndex { parent = String(parent[.. DiscardPileState { + func discardPileAdding( + _ nodes: [FileNodeRecord], + in snapshot: ScanSnapshot + ) -> (pile: DiscardPileState, matchingNodeIDs: Set) { let tree = snapshot.treeStore var updated = discardPile.marks var offsets = Dictionary(uniqueKeysWithValues: updated.enumerated().map { ($0.element.node.id, $0.offset) }) @@ -292,7 +293,8 @@ final class TrashFlowController { let incomingIDs = Set(nodes.map(\.id)) let matchingIDs = DiscardPileState(marks: updated).matchingNodeIDs(in: snapshot) - return DiscardPileState(marks: updated.filter { mark in + var retainedIDs = matchingIDs + let retainedMarks = updated.filter { mark in guard matchingIDs.contains(mark.node.id) else { return true } var parent = tree.parent(of: mark.node.id) while let ancestor = parent { @@ -301,12 +303,14 @@ final class TrashFlowController { ancestor.isDirectory, !ancestor.isSymbolicLink, incomingIDs.contains(ancestor.id) || incomingIDs.contains(mark.node.id), updated[offset].node.fileIdentity != nil || mark.node.fileIdentity == nil { + retainedIDs.remove(mark.node.id) return false } parent = tree.parent(of: ancestor.id) } return true - }) + } + return (DiscardPileState(marks: retainedMarks), retainedIDs) } func removeMovedDiscardPileNodes( @@ -337,7 +341,11 @@ final class TrashFlowController { if remaining != discardPile.marks { replaceDiscardPile(DiscardPileState(marks: remaining)) } } - func changeDiscardPile(_ state: DiscardPileState, actionName: String) { + func changeDiscardPile( + _ state: DiscardPileState, + actionName: String, + matchingNodeIDs: Set? = nil + ) { guard state != discardPile else { return } let previous = discardPile let isReplaying = discardPileUndoManager.isUndoing || discardPileUndoManager.isRedoing @@ -347,10 +355,18 @@ final class TrashFlowController { } discardPileUndoManager.setActionName(actionName) if !isReplaying { discardPileUndoManager.endUndoGrouping() } - discardPile = state + publishDiscardPile(state, matchingNodeIDs: matchingNodeIDs) if isReplaying { onDiscardPileHistoryReplay?(state) } } + private func publishDiscardPile( + _ state: DiscardPileState, + matchingNodeIDs: Set? = nil + ) { + onDiscardPileChange?(state, matchingNodeIDs) + discardPile = state + } + func removeDiscardPileNodes(ids nodeIDs: Set) { guard !nodeIDs.isEmpty else { return } let remaining = discardPile.marks.filter { !nodeIDs.contains($0.node.id) } diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index 1c1bc94..3eb9d41 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -2046,8 +2046,8 @@ struct AppModelDependencyTests { #expect(model.discardPileSnapshot.nodes == [file]) } - @Test - func testDismissingDiscardPileReviewCancelsPreparation() async throws { + @Test(arguments: [false, true]) + func testDiscardPilePreparationSurvivesScanPublicationAndCancelsOnDismissal(dismissed: Bool) async throws { let verifierStarted = OSAllocatedUnfairLock(initialState: false) let verifierGate = DispatchSemaphore(value: 0) var actions = AppSystemActions.inert @@ -2068,12 +2068,19 @@ struct AppModelDependencyTests { try await waitUntil("discard pile verifier starts") { verifierStarted.withLock { $0 } } #expect(model.isPreparingDiscardPileTrash) - model.dismissDiscardPileReview() + if dismissed { + model.dismissDiscardPileReview() + } else { + let original = try #require(model.scanState.snapshot) + let refreshed = makeTestSnapshot(root: original.root, store: original.treeStore) + try #require(original.id != refreshed.id) + model.scanState.restoreCompletedSnapshot(refreshed) + } verifierGate.signal() - #expect(!(await preparation.value)) + #expect(await preparation.value == !dismissed) #expect(!model.isPreparingDiscardPileTrash) - #expect(model.pendingTrashSelection == nil) + #expect(model.pendingTrashSelection?.nodes == (dismissed ? nil : [file])) #expect(model.discardPileSnapshot.nodes == [file]) } diff --git a/RadixCoreTests/TrashFlowControllerTests.swift b/RadixCoreTests/TrashFlowControllerTests.swift index 81d80ce..040e0b0 100644 --- a/RadixCoreTests/TrashFlowControllerTests.swift +++ b/RadixCoreTests/TrashFlowControllerTests.swift @@ -35,7 +35,7 @@ struct TrashFlowControllerTests { } let initial = snapshot(nativeNode) let controller = TrashFlowController() - controller.changeDiscardPile(controller.discardPileAdding([nativeNode], in: initial), actionName: "Add") + controller.changeDiscardPile(controller.discardPileAdding([nativeNode], in: initial).pile, actionName: "Add") let refreshed = snapshot(foundationNode) #expect(controller.discardPile.matchingNodeIDs(in: refreshed) == (replaced ? [] : [nativeNode.id])) @@ -48,6 +48,48 @@ struct TrashFlowControllerTests { #expect(preview.discardPileBatch?.unavailable.map(\.node) == (replaced ? [nativeNode] : [])) } + @Test + func testDiscardPileBatchDeduplicatesAliasesAndKeepsDistinctDirectoryEntries() throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let folder = fixture.appending(path: "folder", directoryHint: .isDirectory) + let aliasFolder = fixture.appending(path: "alias", directoryHint: .isDirectory) + try manager.createDirectory(at: folder, withIntermediateDirectories: false) + try manager.createSymbolicLink(at: aliasFolder, withDestinationURL: folder) + let file = folder.appending(path: "marked.txt") + let hardLink = folder.appending(path: "hard-link.txt") + let symlink = folder.appending(path: "symlink.txt") + try Data([1]).write(to: file) + try manager.linkItem(at: file, to: hardLink) + try manager.createSymbolicLink(at: symlink, withDestinationURL: file) + + let loader = ScanMetadataLoader() + func node(at url: URL, isSymbolicLink: Bool = false) throws -> FileNodeRecord { + makeTestFileNode( + id: url.path, + name: url.lastPathComponent, + fileIdentity: try loader.fileSystemIdentity(at: url), + isSymbolicLink: isSymbolicLink + ) + } + let original = try node(at: file) + let alias = try node(at: aliasFolder.appending(path: file.lastPathComponent)) + let distinctEntries = try [node(at: hardLink), node(at: symlink, isSymbolicLink: true)] + try #require(original.fileIdentity == alias.fileIdentity) + try #require(original.fileIdentity == distinctEntries[0].fileIdentity) + let marks = [original, alias] + distinctEntries + let controller = TrashFlowController(discardPile: DiscardPileState(marks: marks.map { .init(node: $0) })) + + let preview = try TrashFlowController.prepareDiscardPileTrashRequest( + nodes: marks, trashSafetyPolicy: .live(), verifyIdentity: SystemIntegration.verifyTrashIdentity + ) + #expect(preview.nodes == [original] + distinctEntries) + #expect(preview.discardPileBatch?.unavailable.isEmpty == true) + controller.removeMovedDiscardPileNodes([original], batch: preview.discardPileBatch, from: nil) + #expect(controller.discardPile.nodes == distinctEntries) + } + @Test func testDiscardPileBatchCollapsesVerifiedDescendantsAndRetainsUnavailableMarks() throws { let identity = FileIdentity(device: 1, inode: 1) @@ -103,7 +145,7 @@ struct TrashFlowControllerTests { let store = FileTreeStore(root: root, childrenByID: [root.id: [folder, kept], folder.id: [child]]) let snapshot = makeTestSnapshot(root: root, store: store) let controller = TrashFlowController() - controller.changeDiscardPile(controller.discardPileAdding([child, kept], in: snapshot), actionName: "Add") + controller.changeDiscardPile(controller.discardPileAdding([child, kept], in: snapshot).pile, actionName: "Add") controller.removeMovedDiscardPileNodes([folder], batch: nil, from: snapshot) From 1a7d9bcda9dfd3bbb2c8f3cb86a4e543c9143da6 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 22:35:30 -0800 Subject: [PATCH 04/12] fix(discard-pile): reconcile alternate trash paths Use the canonical directory entry for grouping and cloud warnings. Apply each successful move to all the marked paths it covered. --- Radix/ViewModels/AppModel.swift | 34 ++++---- Radix/ViewModels/TrashFlowController.swift | 55 ++++++++----- RadixCoreTests/AppModelDependencyTests.swift | 80 +++++++++++++++++++ RadixCoreTests/TrashFlowControllerTests.swift | 41 ++++++++-- 4 files changed, 168 insertions(+), 42 deletions(-) diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index 50a3d7a..c6b4279 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -2420,7 +2420,7 @@ final class AppModel: ObservableObject { throw FileActionError.unsupported } let discardNodes = topLevelTrashNodes(from: nodes) - if let cloudImpact = cloudStorageImpact(of: discardNodes) { + if let cloudImpact = cloudStorageImpact(of: discardNodes.map(\.url)) { pendingCloudFileAction = PendingCloudFileAction( kind: .addToDiscardPile, nodes: discardNodes, @@ -2555,7 +2555,8 @@ final class AppModel: ObservableObject { try validateMutationDoesNotIncludeHiddenNodes(selection.nodes) } - if let cloudImpact = cloudStorageImpact(of: selection.nodes) { + let urls = selection.nodes.map(\.url) + (selection.discardPileBatch?.resolvedOperationURLs ?? []) + if let cloudImpact = cloudStorageImpact(of: urls) { pendingCloudFileAction = PendingCloudFileAction( kind: .moveToTrash, nodes: selection.nodes, @@ -2613,7 +2614,7 @@ final class AppModel: ObservableObject { } private func cloudStorageImpact( - of nodes: [FileNodeRecord] + of urls: [URL] ) -> CloudStorageLocation.Impact? { var containsCloudStorage = false var existenceByRootPath: [String: Bool] = [:] @@ -2625,9 +2626,9 @@ final class AppModel: ObservableObject { existenceByRootPath[root.path] = rootExists return rootExists } - for node in nodes { + for url in urls { switch CloudStorageLocation.impact( - of: node.url, + of: url, cloudRootExists: cloudRootExists ) { case .storedInCloud: @@ -2654,7 +2655,7 @@ final class AppModel: ObservableObject { moveToTrash: dependencies.systemActions.moveToTrash, beginMove: { let didHide = trashFlow.hideTrashNodesDuringMove( - nodes, + discardPileBatch?.affectedNodes(for: nodes) ?? nodes, snapshotID: originalSnapshotID, activeSnapshotID: scanCoordinator.snapshot?.id, activeFileTreeStore: statsFileTreeStore @@ -2688,29 +2689,26 @@ final class AppModel: ObservableObject { wasCancelled: Bool = false ) { let originalSnapshotID = originalSnapshot?.id + let affectedMovedNodes = discardPileBatch?.affectedNodes(for: movedNodes) ?? movedNodes if !movedNodes.isEmpty { trashFlow.invalidateDiscardPileHistory() sidebarScanCacheController.clearCache() - reconcileScanAfterTrash(movedNodes, originalSnapshotID: originalSnapshotID) - trashFlow.removeMovedDiscardPileNodes(movedNodes, batch: discardPileBatch, from: originalSnapshot) + reconcileScanAfterTrash(affectedMovedNodes, originalSnapshotID: originalSnapshotID) + trashFlow.removeMovedDiscardPileNodes( + affectedMovedNodes, from: originalSnapshot, includeSnapshotDescendants: discardPileBatch == nil + ) recordTrashMove(movedNodes, fileTreeStore: originalSnapshot?.treeStore) refreshAvailableTargets() } - if let actionError { - trashFlow.unhideTrashNodesAfterFailedMove( - requestedNodes: requestedNodes, - movedNodes: movedNodes, - snapshotID: originalSnapshotID - ) - presentError(actionError) - } else if wasCancelled { + if actionError != nil || wasCancelled { trashFlow.unhideTrashNodesAfterFailedMove( - requestedNodes: requestedNodes, - movedNodes: movedNodes, + requestedNodes: discardPileBatch?.affectedNodes(for: requestedNodes) ?? requestedNodes, + movedNodes: affectedMovedNodes, snapshotID: originalSnapshotID ) } + if let actionError { presentError(actionError) } } private func reconcileScanAfterTrash(_ movedNodes: [FileNodeRecord], originalSnapshotID: UUID?) { diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index fb5dd45..76f2a6e 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -73,7 +73,12 @@ final class TrashFlowController { } let unavailable: [Unavailable] + let resolvedOperationURLs: [URL] let coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] + + func affectedNodes(for operations: [FileNodeRecord]) -> [FileNodeRecord] { + operations + operations.flatMap { coveredMarks[$0.id] ?? [] } + } } nonisolated struct PendingTrashSelection: Sendable { @@ -216,7 +221,7 @@ final class TrashFlowController { guard !nodes.isEmpty else { throw FileActionError.noSelection } var unavailable: [DiscardPileTrashBatch.Unavailable] = [] - var eligible: [(node: FileNodeRecord, path: String)] = [] + var eligible: [(node: FileNodeRecord, resolvedURL: URL)] = [] for node in nodes { try Task.checkCancellation() let message: String? @@ -234,15 +239,20 @@ final class TrashFlowController { if let message { unavailable.append(.init(node: node, message: message)) } else { - let parent = node.url.deletingLastPathComponent().resolvingSymlinksInPath() - eligible.append((node, parent.appending(path: node.url.lastPathComponent).path)) + let url = URL(filePath: node.url.path) + // Canonical paths recover the on-disk spelling without following + // a final symlink or treating distinct hard links as one entry. + let path = (try? url.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath) ?? url.path + eligible.append((node, URL(filePath: path))) } } - let entriesByPath = Dictionary(eligible.map { ($0.path, $0.node) }, uniquingKeysWith: { first, _ in first }) + let entriesByPath = Dictionary(eligible.map { ($0.resolvedURL.path, $0.node) }, uniquingKeysWith: { first, _ in first }) + var operations: [(node: FileNodeRecord, resolvedURL: URL)] = [] var coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] = [:] - let operations = try eligible.compactMap { node, path -> FileNodeRecord? in + for (node, resolvedURL) in eligible { try Task.checkCancellation() + let path = resolvedURL.path var coveringID = entriesByPath[path]?.id ?? node.id var parent = path while let separator = parent.lastIndex(of: "/"), separator != parent.startIndex { @@ -251,13 +261,17 @@ final class TrashFlowController { coveringID = ancestor.id } } - guard coveringID != node.id else { return node } - coveredMarks[coveringID, default: []].append(node) - return nil + if coveringID == node.id { + operations.append((node, resolvedURL)) + } else { + coveredMarks[coveringID, default: []].append(node) + } } return PendingTrashSelection( - nodes: operations, - discardPileBatch: DiscardPileTrashBatch(unavailable: unavailable, coveredMarks: coveredMarks) + nodes: operations.map(\.node), + discardPileBatch: DiscardPileTrashBatch( + unavailable: unavailable, resolvedOperationURLs: operations.map(\.resolvedURL), coveredMarks: coveredMarks + ) ) } @@ -315,25 +329,28 @@ final class TrashFlowController { func removeMovedDiscardPileNodes( _ movedNodes: [FileNodeRecord], - batch: DiscardPileTrashBatch?, - from originalSnapshot: ScanSnapshot? + from originalSnapshot: ScanSnapshot?, + includeSnapshotDescendants: Bool ) { guard !discardPile.isEmpty else { return } - let removed = movedNodes + movedNodes.flatMap { batch?.coveredMarks[$0.id] ?? [] } - let removedByID = Dictionary(removed.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) + let removedByID = Dictionary(movedNodes.map { ($0.id, $0) }, uniquingKeysWith: { first, _ in first }) let tree = originalSnapshot?.treeStore - let movedFolders = movedNodes.filter { $0.isDirectory && !$0.isSymbolicLink } - let matchedFolders = movedFolders.filter { folder in - tree?.node(id: folder.id)?.fileIdentity == folder.fileIdentity + let removedSubtrees: PreparedFileTreeNodeSet? = if includeSnapshotDescendants, let tree { + tree.preparedNodeSet(for: Set(movedNodes.compactMap { node in + guard node.isDirectory, !node.isSymbolicLink, + let current = tree.node(id: node.id), current.fileIdentity == node.fileIdentity else { return nil } + return node.id + })) + } else { + nil } - let removedSubtrees = tree?.preparedNodeSet(for: Set(matchedFolders.map(\.id))) let remaining = discardPile.marks.filter { mark in if let node = removedByID[mark.node.id] { return mark.node.fileIdentity != node.fileIdentity || (node.fileIdentity == nil && mark.unidentifiedTreeID != tree?.contentID) } - guard batch == nil, let tree, let removedSubtrees, + guard let tree, let removedSubtrees, tree.isNodeOrDescendant(mark.node.id, of: removedSubtrees), let original = tree.node(id: mark.node.id) else { return true } return !mark.matches(original, treeID: tree.contentID) diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index 3eb9d41..b75ac92 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -1632,6 +1632,35 @@ struct AppModelDependencyTests { #expect(model.pendingCloudFileAction == nil) } + @Test + func testDiscardPileTrashWarnsWhenDataVolumePathContainsCloudStorage() async throws { + let home = FileManager.default.homeDirectoryForCurrentUser + let library = home.appending(path: "Library", directoryHint: .isDirectory) + let dataLibrary = URL(filePath: "/System/Volumes/Data" + library.path, directoryHint: .isDirectory) + let cloudRoot = library.appending(path: "CloudStorage", directoryHint: .isDirectory) + let node = makeTestDirectoryNode( + id: dataLibrary.path, name: "Library", children: [], + fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: dataLibrary) + ) + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + // The volume alias and Library are real; no installed cloud provider is needed. + actions.fileExists = { $0.path == cloudRoot.path } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + defer { model.cleanup() } + let root = makeTestDirectoryNode(id: dataLibrary.deletingLastPathComponent().path, name: "Home", children: [node]) + let store = FileTreeStore(root: root, childrenByID: [root.id: [node]]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(root: root, store: store)) + try #require(model.addNodesToDiscardPile([node])) + try #require(await model.requestMoveDiscardPileToTrash()) + + model.confirmMovePendingSelectionToTrash() + + #expect(model.pendingCloudFileAction?.kind == .moveToTrash) + #expect(model.pendingCloudFileAction?.cloudImpact == .containsCloudStorage) + #expect(model.pendingCloudFileAction?.nodes == [node]) + } + @Test func testMovingVisibleNodeToTrashDoesNotMoveDiscardPileNodes() async throws { let recorder = AppModelActionRecorder() @@ -2510,6 +2539,57 @@ struct AppModelDependencyTests { #expect(model.usageStats.bytesMovedToTrash == first.allocatedSize) } + @Test + func testDiscardPileTrashRemovesTheCoveredPathFromTheDisplayedScan() async throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let folder = fixture.appending(path: "folder", directoryHint: .isDirectory) + let alias = fixture.appending(path: "alias", directoryHint: .isDirectory) + try manager.createDirectory(at: folder, withIntermediateDirectories: false) + try manager.createSymbolicLink(at: alias, withDestinationURL: folder) + let file = folder.appending(path: "marked.txt") + let moved = fixture.appending(path: "moved.txt") + try Data([1]).write(to: file) + + var attempted: [URL] = [] + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + actions.moveToTrash = { node in + attempted.append(node.url) + try manager.moveItem(at: node.url, to: moved) + return .matches + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + defer { model.cleanup() } + + func display(_ parent: URL) throws -> FileNodeRecord { + let url = parent.appending(path: file.lastPathComponent) + let node = makeTestFileNode(id: url.path, name: url.lastPathComponent, + fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: url)) + let root = makeTestDirectoryNode(id: parent.path, name: parent.lastPathComponent, children: [node]) + let tree = FileTreeStore(root: root, childrenByID: [root.id: [node]]) + let target = ScanTarget(id: root.id, url: root.url, displayName: root.name, kind: .folder) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(target: target, root: root, store: tree)) + return node + } + + let original = try display(folder) + try #require(model.addNodesToDiscardPile([original])) + let covered = try display(alias) + try #require(model.addNodesToDiscardPile([covered])) + try #require(await model.requestMoveDiscardPileToTrash()) + try #require(model.pendingTrashSelection?.nodes == [original]) + + model.confirmMovePendingSelectionToTrash() + try await waitUntil("trash and scan reconciliation finish") { !model.trashFlow.isMovingFiles } + + #expect(attempted == [original.url]) + #expect(!manager.fileExists(atPath: covered.id)) + #expect(model.discardPile.isEmpty) + #expect(model.scanState.fileTreeStore?.node(id: covered.id) == nil) + } + @Test func testDiscardPileKeepsMarksMissingFromCurrentTree() { var actions = AppSystemActions.inert diff --git a/RadixCoreTests/TrashFlowControllerTests.swift b/RadixCoreTests/TrashFlowControllerTests.swift index 040e0b0..16a934b 100644 --- a/RadixCoreTests/TrashFlowControllerTests.swift +++ b/RadixCoreTests/TrashFlowControllerTests.swift @@ -85,11 +85,41 @@ struct TrashFlowControllerTests { nodes: marks, trashSafetyPolicy: .live(), verifyIdentity: SystemIntegration.verifyTrashIdentity ) #expect(preview.nodes == [original] + distinctEntries) - #expect(preview.discardPileBatch?.unavailable.isEmpty == true) - controller.removeMovedDiscardPileNodes([original], batch: preview.discardPileBatch, from: nil) + let batch = try #require(preview.discardPileBatch) + #expect(batch.unavailable.isEmpty) + controller.removeMovedDiscardPileNodes(batch.affectedNodes(for: [original]), from: nil, includeSnapshotDescendants: false) #expect(controller.discardPile.nodes == distinctEntries) } + @Test + func testDiscardPileBatchOnlyMovesOneEntryAfterCaseOnlyRename() throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let originalURL = fixture.appending(path: "Marked.txt") + let renamedURL = fixture.appending(path: "marked.txt") + let intermediateURL = fixture.appending(path: "rename-tmp") + let loader = ScanMetadataLoader() + try Data([1]).write(to: originalURL) + let original = makeTestFileNode( + id: originalURL.path, name: originalURL.lastPathComponent, fileIdentity: try loader.fileSystemIdentity(at: originalURL) + ) + try manager.moveItem(at: originalURL, to: intermediateURL) + try manager.moveItem(at: intermediateURL, to: renamedURL) + let renamed = makeTestFileNode( + id: renamedURL.path, name: renamedURL.lastPathComponent, fileIdentity: try loader.fileSystemIdentity(at: renamedURL) + ) + try #require(original.fileIdentity == renamed.fileIdentity) + let originalStillResolves = manager.fileExists(atPath: originalURL.path) + + let preview = try TrashFlowController.prepareDiscardPileTrashRequest( + nodes: [original, renamed], trashSafetyPolicy: .live(), verifyIdentity: SystemIntegration.verifyTrashIdentity + ) + + #expect(preview.nodes == [originalStillResolves ? original : renamed]) + #expect(preview.discardPileBatch?.unavailable.map(\.node) == (originalStillResolves ? [] : [original])) + } + @Test func testDiscardPileBatchCollapsesVerifiedDescendantsAndRetainsUnavailableMarks() throws { let identity = FileIdentity(device: 1, inode: 1) @@ -113,8 +143,9 @@ struct TrashFlowControllerTests { $0 == unavailable ? .missingCurrentItem : .matches } #expect(preview.nodes == [folder, sibling]) - #expect(preview.discardPileBatch?.unavailable.map(\.node) == [unavailable]) - controller.removeMovedDiscardPileNodes([folder], batch: preview.discardPileBatch, from: nil) + let batch = try #require(preview.discardPileBatch) + #expect(batch.unavailable.map(\.node) == [unavailable]) + controller.removeMovedDiscardPileNodes(batch.affectedNodes(for: [folder]), from: nil, includeSnapshotDescendants: false) #expect(controller.discardPile.nodes == [unavailable, sibling]) } @@ -147,7 +178,7 @@ struct TrashFlowControllerTests { let controller = TrashFlowController() controller.changeDiscardPile(controller.discardPileAdding([child, kept], in: snapshot).pile, actionName: "Add") - controller.removeMovedDiscardPileNodes([folder], batch: nil, from: snapshot) + controller.removeMovedDiscardPileNodes([folder], from: snapshot, includeSnapshotDescendants: true) #expect(controller.discardPile.nodeIDs == [kept.id]) } From 1232adffdb1914ba904ec7897027cc5431bb9374 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 22:55:06 -0800 Subject: [PATCH 05/12] fix(discard-pile): refresh scans after aliased moves Reconcile the displayed scan against the resolved path of each successful move so renamed rows and moved scan roots disappear. --- Radix/Models/TrashSafetyPolicy.swift | 15 +++ Radix/ViewModels/AppModel.swift | 80 +++++++++--- Radix/ViewModels/TrashFlowController.swift | 6 +- RadixCoreTests/AppModelDependencyTests.swift | 128 +++++++++++++++++-- 4 files changed, 199 insertions(+), 30 deletions(-) diff --git a/Radix/Models/TrashSafetyPolicy.swift b/Radix/Models/TrashSafetyPolicy.swift index aab8ffc..8f61970 100644 --- a/Radix/Models/TrashSafetyPolicy.swift +++ b/Radix/Models/TrashSafetyPolicy.swift @@ -69,6 +69,9 @@ nonisolated struct TrashSafetyPolicy: Sendable { private nonisolated static let defaultFirmlinkRootPaths = Set( defaultFirmlinkEntries.map { standardizedPath(forPath: $0.visiblePath) } ) + private nonisolated static let startupVolumePathPairs = defaultFirmlinkEntries.compactMap { entry in + dataVolumePath(forRelativePath: entry.dataRelativePath).map { (visible: entry.visiblePath, data: $0) } + } private let protectedRootPaths: Set @@ -131,6 +134,18 @@ nonisolated struct TrashSafetyPolicy: Sendable { defaultFirmlinkRootPaths.contains(standardizedPath(for: url)) } + nonisolated static func alternateStartupVolumePath(for path: String) -> String? { + for (visible, data) in startupVolumePathPairs { + if path == visible || path.hasPrefix(visible + "/") { + return data + path.dropFirst(visible.count) + } + if path == data || path.hasPrefix(data + "/") { + return visible + path.dropFirst(data.count) + } + } + return nil + } + nonisolated static func blockReason(for url: URL) -> TrashSafetyBlockReason? { live().blockReason(for: url) } diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index c6b4279..1eccdee 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -2555,7 +2555,8 @@ final class AppModel: ObservableObject { try validateMutationDoesNotIncludeHiddenNodes(selection.nodes) } - let urls = selection.nodes.map(\.url) + (selection.discardPileBatch?.resolvedOperationURLs ?? []) + let resolvedURLs = selection.discardPileBatch?.resolvedURLsByOperationID ?? [:] + let urls = selection.nodes.map(\.url) + Array(resolvedURLs.values) if let cloudImpact = cloudStorageImpact(of: urls) { pendingCloudFileAction = PendingCloudFileAction( kind: .moveToTrash, @@ -2693,7 +2694,11 @@ final class AppModel: ObservableObject { if !movedNodes.isEmpty { trashFlow.invalidateDiscardPileHistory() sidebarScanCacheController.clearCache() - reconcileScanAfterTrash(affectedMovedNodes, originalSnapshotID: originalSnapshotID) + reconcileScanAfterTrash( + affectedMovedNodes, + resolvedURLs: discardPileBatch?.resolvedURLsByOperationID ?? [:], + originalSnapshotID: originalSnapshotID + ) trashFlow.removeMovedDiscardPileNodes( affectedMovedNodes, from: originalSnapshot, includeSnapshotDescendants: discardPileBatch == nil ) @@ -2711,14 +2716,34 @@ final class AppModel: ObservableObject { if let actionError { presentError(actionError) } } - private func reconcileScanAfterTrash(_ movedNodes: [FileNodeRecord], originalSnapshotID: UUID?) { + private func reconcileScanAfterTrash( + _ movedNodes: [FileNodeRecord], + resolvedURLs: [FileNodeRecord.ID: URL], + originalSnapshotID: UUID? + ) { guard let target = scanCoordinator.selectedTarget else { return } let snapshot = scanCoordinator.snapshot let isFullScanInProgress = scanCoordinator.isFullScanInProgress guard snapshot?.source.allowsFileMutation ?? isFullScanInProgress else { return } - let rootMayHaveMoved = movedNodes.contains { - $0.id == target.id || ($0.isDirectory && Self.path(target.id, isWithin: $0.id)) + let resolvedTargetPath = resolvedURLs.isEmpty ? target.id : Self.resolvedScanPath(target.url) + let alternatePaths = movedNodes.flatMap { node -> [(id: String, node: FileNodeRecord)] in + guard let resolvedPath = resolvedURLs[node.id]?.path else { return [] } + var paths = [resolvedPath] + if let alternate = TrashSafetyPolicy.alternateStartupVolumePath(for: resolvedPath) { + paths.append(alternate) + } + if resolvedTargetPath != target.id, Self.path(resolvedPath, isWithin: resolvedTargetPath) { + paths.append(target.id + resolvedPath.dropFirst(resolvedTargetPath.count)) + } + return paths.filter { $0 != node.id }.map { (id: $0, node: node) } + } + let paths = movedNodes.map { (id: $0.id, node: $0) } + alternatePaths + let targetPaths = [target.id, resolvedTargetPath] + let rootMayHaveMoved = paths.contains { id, node in + targetPaths.contains { targetPath in + id == targetPath || (node.isDirectory && !node.isSymbolicLink && Self.path(targetPath, isWithin: id)) + } } if rootMayHaveMoved { let rootStillValid = snapshot.map { @@ -2731,22 +2756,28 @@ final class AppModel: ObservableObject { } if isFullScanInProgress, - rootMayHaveMoved || movedNodes.contains(where: { Self.path($0.id, isWithin: target.id) }) { + rootMayHaveMoved || paths.contains(where: { Self.path($0.id, isWithin: target.id) }) { if restartCurrentScanAfterTrash(target, snapshot: snapshot) { return } scanCoordinator.stopScan() } guard let snapshot else { return } let tree = snapshot.treeStore - let matching = { [self] (moved: FileNodeRecord, current: FileNodeRecord) in - matchesMovedNode(moved, current: current, sameSnapshot: originalSnapshotID == snapshot.id) - } + let sameSnapshot = originalSnapshotID == snapshot.id var summarized: [FileNodeRecord] = [] - let matched = movedNodes.compactMap { moved -> FileNodeRecord? in - if let current = tree.node(id: moved.id) { - return matching(moved, current) ? current : nil + var alternateMatches: [FileNodeRecord] = [] + var matchedIDs: Set = [] + let matched = paths.compactMap { id, moved -> FileNodeRecord? in + if let current = tree.node(id: id) { + let isAlternatePath = id != moved.id + guard !matchedIDs.contains(id), matchesMovedNode( + moved, current: current, sameSnapshot: sameSnapshot, requiresMissingPath: isAlternatePath + ) else { return nil } + if isAlternatePath { alternateMatches.append(current) } + matchedIDs.insert(id) + return current } - if let ancestor = summarizedAncestor(of: moved.id, in: tree) { summarized.append(ancestor) } + if let ancestor = summarizedAncestor(of: id, in: tree) { summarized.append(ancestor) } return nil } if matched.contains(where: { $0.id == target.id }) { @@ -2769,20 +2800,35 @@ final class AppModel: ObservableObject { activeFileTreeStore: tree ) enqueuePostTrashSnapshotRemoval( - movedNodes: movedNodes, + movedNodes: movedNodes + alternateMatches, hiddenIDs: Set(matched.map(\.id)), - sameSnapshot: originalSnapshotID == snapshot.id, + sameSnapshot: sameSnapshot, fallbackFocusID: fallbackFocusID ) } + private static func resolvedScanPath(_ url: URL) -> String { + let path = url.path + var ancestor = URL(filePath: path) + // The move may have removed the scan root or one of its parents. + while ancestor.path != "/" { + if let resolved = try? ancestor.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath { + return resolved + path.dropFirst(ancestor.path.count) + } + ancestor.deleteLastPathComponent() + } + return path + } + private func matchesMovedNode( _ moved: FileNodeRecord, current: FileNodeRecord, - sameSnapshot: Bool + sameSnapshot: Bool, + requiresMissingPath: Bool = false ) -> Bool { if moved.fileIdentity == current.fileIdentity { - return moved.fileIdentity != nil || sameSnapshot + guard moved.fileIdentity != nil || sameSnapshot else { return false } + return !requiresMissingPath || dependencies.systemActions.verifyTrashIdentity(current) == .missingCurrentItem } guard let movedIdentity = moved.fileIdentity, let currentIdentity = current.fileIdentity, movedIdentity.isFileSystemIdentity != currentIdentity.isFileSystemIdentity else { return false } diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index 76f2a6e..d8ed3b5 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -73,7 +73,7 @@ final class TrashFlowController { } let unavailable: [Unavailable] - let resolvedOperationURLs: [URL] + let resolvedURLsByOperationID: [FileNodeRecord.ID: URL] let coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] func affectedNodes(for operations: [FileNodeRecord]) -> [FileNodeRecord] { @@ -270,7 +270,9 @@ final class TrashFlowController { return PendingTrashSelection( nodes: operations.map(\.node), discardPileBatch: DiscardPileTrashBatch( - unavailable: unavailable, resolvedOperationURLs: operations.map(\.resolvedURL), coveredMarks: coveredMarks + unavailable: unavailable, + resolvedURLsByOperationID: Dictionary(uniqueKeysWithValues: operations.map { ($0.node.id, $0.resolvedURL) }), + coveredMarks: coveredMarks ) ) } diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index b75ac92..6a5e4be 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -2539,18 +2539,22 @@ struct AppModelDependencyTests { #expect(model.usageStats.bytesMovedToTrash == first.allocatedSize) } - @Test - func testDiscardPileTrashRemovesTheCoveredPathFromTheDisplayedScan() async throws { + @Test(arguments: [false, true]) + func testDiscardPileTrashRemovesAliasedRowAndKeepsHardLink(markAlias: Bool) async throws { let fixture = try makeTemporaryDirectory() let manager = FileManager.default defer { try? manager.removeItem(at: fixture) } - let folder = fixture.appending(path: "folder", directoryHint: .isDirectory) - let alias = fixture.appending(path: "alias", directoryHint: .isDirectory) + let canonicalPath = try #require(fixture.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath) + let parent = URL(filePath: canonicalPath, directoryHint: .isDirectory) + let alternateParent = URL(filePath: "/System/Volumes/Data" + canonicalPath, directoryHint: .isDirectory) + let folder = parent.appending(path: "folder", directoryHint: .isDirectory) + let alias = alternateParent.appending(path: "folder", directoryHint: .isDirectory) try manager.createDirectory(at: folder, withIntermediateDirectories: false) - try manager.createSymbolicLink(at: alias, withDestinationURL: folder) let file = folder.appending(path: "marked.txt") + let link = folder.appending(path: "keep.txt") let moved = fixture.appending(path: "moved.txt") try Data([1]).write(to: file) + try manager.linkItem(at: file, to: link) var attempted: [URL] = [] var actions = AppSystemActions.inert @@ -2564,20 +2568,22 @@ struct AppModelDependencyTests { defer { model.cleanup() } func display(_ parent: URL) throws -> FileNodeRecord { - let url = parent.appending(path: file.lastPathComponent) - let node = makeTestFileNode(id: url.path, name: url.lastPathComponent, + let nodes = try [file, link].map { entry in + let url = parent.appending(path: entry.lastPathComponent) + return makeTestFileNode(id: url.path, name: url.lastPathComponent, fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: url)) - let root = makeTestDirectoryNode(id: parent.path, name: parent.lastPathComponent, children: [node]) - let tree = FileTreeStore(root: root, childrenByID: [root.id: [node]]) + } + let root = makeTestDirectoryNode(id: parent.path, name: parent.lastPathComponent, children: nodes) + let tree = FileTreeStore(root: root, childrenByID: [root.id: nodes]) let target = ScanTarget(id: root.id, url: root.url, displayName: root.name, kind: .folder) model.scanState.restoreCompletedSnapshot(makeTestSnapshot(target: target, root: root, store: tree)) - return node + return nodes[0] } let original = try display(folder) try #require(model.addNodesToDiscardPile([original])) let covered = try display(alias) - try #require(model.addNodesToDiscardPile([covered])) + if markAlias { try #require(model.addNodesToDiscardPile([covered])) } try #require(await model.requestMoveDiscardPileToTrash()) try #require(model.pendingTrashSelection?.nodes == [original]) @@ -2588,6 +2594,106 @@ struct AppModelDependencyTests { #expect(!manager.fileExists(atPath: covered.id)) #expect(model.discardPile.isEmpty) #expect(model.scanState.fileTreeStore?.node(id: covered.id) == nil) + #expect(model.scanState.fileTreeStore?.node(id: alias.appending(path: link.lastPathComponent).path) != nil) + } + + @Test + func testDiscardPileTrashRemovesRowAfterCaseOnlyRename() async throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let target = ScanTarget(url: fixture) + let originalURL = target.url.appending(path: "Marked.txt") + let renamedURL = target.url.appending(path: "marked.txt") + let stagingURL = target.url.appending(path: "rename-tmp") + let movedURL = target.url.appending(path: "moved.txt") + try Data([1]).write(to: originalURL) + guard manager.fileExists(atPath: renamedURL.path) else { return } + + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + actions.moveToTrash = { node in + try SystemIntegration.moveToTrash( + node, identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { try manager.moveItem(at: $0, to: movedURL) } + ) + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + defer { model.cleanup() } + + func scan(for url: URL) async throws -> FileNodeRecord { + model.scanState.startScan(target, options: ScanOptions()) + try await waitUntil("live scan contains \(url.lastPathComponent)") { + model.scanState.snapshot?.isComplete == true && model.scanState.fileTreeStore?.node(id: url.path) != nil + } + return try #require(model.scanState.fileTreeStore?.node(id: url.path)) + } + + let original = try await scan(for: originalURL) + try #require(model.addNodesToDiscardPile([original])) + try manager.moveItem(at: originalURL, to: stagingURL) + try manager.moveItem(at: stagingURL, to: renamedURL) + let renamed = try await scan(for: renamedURL) + try #require(await model.requestMoveDiscardPileToTrash()) + try #require(model.pendingTrashSelection?.nodes == [original]) + + model.confirmMovePendingSelectionToTrash() + try await waitUntil("trash and scan reconciliation finish") { !model.trashFlow.isMovingFiles } + + #expect(manager.fileExists(atPath: movedURL.path)) + #expect(model.discardPile.isEmpty) + #expect(model.scanState.fileTreeStore?.node(id: renamed.id) == nil) + } + + @Test(arguments: [false, true]) + func testDiscardPileTrashClosesScanOfMovedFolderUnderOtherVolumePath(markDataPath: Bool) async throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let canonicalPath = try #require(fixture.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath) + let visibleParent = URL(filePath: canonicalPath, directoryHint: .isDirectory) + let dataParent = URL(filePath: "/System/Volumes/Data" + canonicalPath, directoryHint: .isDirectory) + let markParent = markDataPath ? dataParent : visibleParent + let displayedParent = markDataPath ? visibleParent : dataParent + let folder = visibleParent.appending(path: "folder", directoryHint: .isDirectory) + let moved = visibleParent.appending(path: "moved", directoryHint: .isDirectory) + try manager.createDirectory(at: folder, withIntermediateDirectories: false) + + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + actions.moveToTrash = { node in + try SystemIntegration.moveToTrash( + node, identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { try manager.moveItem(at: $0, to: moved) } + ) + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + defer { model.cleanup() } + + func display(_ root: FileNodeRecord, children: [FileNodeRecord]) { + let target = ScanTarget(id: root.id, url: root.url, displayName: root.name, kind: .folder) + let tree = FileTreeStore(root: root, childrenByID: [root.id: children]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(target: target, root: root, store: tree)) + } + + func folderNode(in parent: URL) throws -> FileNodeRecord { + let url = parent.appending(path: folder.lastPathComponent, directoryHint: .isDirectory) + return makeTestDirectoryNode(id: url.path, name: url.lastPathComponent, children: [], + fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: url)) + } + let marked = try folderNode(in: markParent) + display(makeTestDirectoryNode(id: markParent.path, name: markParent.lastPathComponent, children: [marked]), children: [marked]) + try #require(model.addNodesToDiscardPile([marked])) + let displayed = try folderNode(in: displayedParent) + display(displayed, children: []) + try #require(await model.requestMoveDiscardPileToTrash()) + + model.confirmMovePendingSelectionToTrash() + try await waitUntil("trash and scan reconciliation finish") { !model.trashFlow.isMovingFiles } + + #expect(manager.fileExists(atPath: moved.path)) + #expect(model.discardPile.isEmpty) + #expect(model.scanState.snapshot == nil) } @Test From e819477403312dda710692c1f7f96fbdf3706988 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:02:02 -0800 Subject: [PATCH 06/12] fix(discard-pile): match marks without filesystem reads Capture the device alongside comparable Foundation identities during scanning. Use recorded identities for mark projection while keeping the full resource identifier for verification before Trash. --- Radix/Services/ScanArchiveModels.swift | 2 +- Radix/Services/ScanMetadataLoader.swift | 36 +++++++++++++++---- Radix/ViewModels/TrashFlowController.swift | 8 +++-- RadixCoreTests/ScanMetadataLoaderTests.swift | 20 +++++++++++ RadixCoreTests/ScanResultFingerprint.swift | 2 +- RadixCoreTests/TrashFlowControllerTests.swift | 4 ++- 6 files changed, 59 insertions(+), 13 deletions(-) diff --git a/Radix/Services/ScanArchiveModels.swift b/Radix/Services/ScanArchiveModels.swift index 9ec78b1..8b5c359 100644 --- a/Radix/Services/ScanArchiveModels.swift +++ b/Radix/Services/ScanArchiveModels.swift @@ -509,7 +509,7 @@ nonisolated struct ScanArchiveFileIdentity: Codable, Sendable { init(_ identity: FileIdentity) { switch identity { - case .resourceIdentifier(let data): + case .resourceIdentifier(let data, _): self.kind = .resourceIdentifier self.resourceIdentifier = data.base64EncodedString() self.device = nil diff --git a/Radix/Services/ScanMetadataLoader.swift b/Radix/Services/ScanMetadataLoader.swift index 86728bf..b5cff7a 100644 --- a/Radix/Services/ScanMetadataLoader.swift +++ b/Radix/Services/ScanMetadataLoader.swift @@ -533,7 +533,9 @@ nonisolated struct ScanMetadataLoader: Sendable { fileIdentity = fileSystemInfo.identity ?? fileIdentity linkCount = values.linkCount.map(UInt64.init) ?? fileSystemInfo.linkCount } - fileIdentity = fileIdentity?.preservingVolumeIdentity(from: resourceIdentity) + fileIdentity = fileIdentity? + .preservingVolumeIdentity(from: resourceIdentity) + .preservingScanDevice(from: status?.fileIdentity) let cloneMetadata = !isDirectory && !isSymbolicLink ? cloneMappingCapabilityCache.cloneMetadata(for: url) : (identity: nil, mayShareDataBlocks: false) @@ -764,7 +766,7 @@ nonisolated struct CloneIdentity: Hashable, Sendable { } nonisolated enum FileIdentity: Hashable, Sendable { - case resourceIdentifier(Data) + case resourceIdentifier(Data, scanDevice: UInt64? = nil) case fileSystem(device: UInt64, inode: UInt64, volumeToken: UInt64? = nil) nonisolated init(device: UInt64, inode: UInt64, volumeToken: UInt64? = nil) { @@ -797,11 +799,10 @@ nonisolated enum FileIdentity: Hashable, Sendable { return nil } - /// Supplementary archive-comparison metadata must not split native and - /// Foundation claims for the same hard link during scan accounting. + /// Supplementary metadata must not change identity equality or hashing. static func == (lhs: Self, rhs: Self) -> Bool { switch (lhs, rhs) { - case let (.resourceIdentifier(lhs), .resourceIdentifier(rhs)): + case let (.resourceIdentifier(lhs, _), .resourceIdentifier(rhs, _)): return lhs == rhs case let (.fileSystem(lhsDevice, lhsInode, _), .fileSystem(rhsDevice, rhsInode, _)): return lhsDevice == rhsDevice && lhsInode == rhsInode @@ -812,7 +813,7 @@ nonisolated enum FileIdentity: Hashable, Sendable { func hash(into hasher: inout Hasher) { switch self { - case .resourceIdentifier(let data): + case .resourceIdentifier(let data, _): hasher.combine(0) hasher.combine(data) case .fileSystem(let device, let inode, _): @@ -834,7 +835,7 @@ nonisolated enum FileIdentity: Hashable, Sendable { switch self { case .fileSystem(_, let inode, let volumeToken): return volumeToken.map { DarwinIdentity(fileID: inode, volumeToken: $0) } - case .resourceIdentifier(let data): + case .resourceIdentifier(let data, _): guard data.count == MemoryLayout.size * 2 else { return nil } return data.withUnsafeBytes { bytes in DarwinIdentity( @@ -847,6 +848,27 @@ nonisolated enum FileIdentity: Hashable, Sendable { } } + var scanFileSystemIdentity: Self? { + switch self { + case .fileSystem: + self + case .resourceIdentifier(_, let device): + if let device, let identity = darwinIdentity { + .fileSystem(device: device, inode: identity.fileID) + } else { + nil + } + } + } + + func preservingScanDevice(from statusIdentity: FileIdentity?) -> Self { + // Prefetched metadata and lstat may observe different files at the same path. + guard case .resourceIdentifier(let data, _) = self, + case .fileSystem(let device, let inode, _) = statusIdentity, + darwinIdentity?.fileID == inode else { return self } + return .resourceIdentifier(data, scanDevice: device) + } + func preservingVolumeIdentity(from resourceIdentity: FileIdentity?) -> Self { guard case .fileSystem(let device, let inode, _) = self, let identity = resourceIdentity?.darwinIdentity, diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index d8ed3b5..c326d57 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -25,9 +25,11 @@ struct DiscardPileState: Equatable, Sendable { guard !original.isSymbolicLink, !current.isSymbolicLink, let first = original.fileIdentity, let second = current.fileIdentity, first.isFileSystemIdentity != second.isFileSystemIdentity else { return false } - return SystemIntegration.verifyTrashIdentity(original) == .matches - && SystemIntegration.verifyTrashIdentity(current) == .matches - && SystemIntegration.verifyTrashIdentity(original) == .matches + if let firstDarwin = first.darwinIdentity, let secondDarwin = second.darwinIdentity { + return firstDarwin == secondDarwin + } + guard let firstNative = first.scanFileSystemIdentity else { return false } + return firstNative == second.scanFileSystemIdentity } } diff --git a/RadixCoreTests/ScanMetadataLoaderTests.swift b/RadixCoreTests/ScanMetadataLoaderTests.swift index da43db6..0297db4 100644 --- a/RadixCoreTests/ScanMetadataLoaderTests.swift +++ b/RadixCoreTests/ScanMetadataLoaderTests.swift @@ -111,6 +111,26 @@ struct ScanMetadataLoaderTests { #expect(try loader.metadata(for: directory).fileIdentity != original) } + @Test + func testPrefetchedFileMetadataDoesNotAdoptReplacementIdentity() throws { + let root = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: root) } + let file = root.appending(path: "file.bin") + try Data([1]).write(to: file) + let values = try file.resourceValues(forKeys: ScanMetadataLoader.scanResourceKeys) + let original = FileIdentity(resourceIdentifier: try #require(values.fileResourceIdentifier as? Data)) + try manager.moveItem(at: file, to: root.appending(path: "original.bin")) + try Data([2]).write(to: file) + + let metadata = ScanMetadataLoader().metadata(for: file, prefetchedResourceValues: values) + let node = makeTestFileNode(id: file.path, name: file.lastPathComponent, fileIdentity: metadata.fileIdentity) + + #expect(metadata.fileIdentity == original) + #expect(metadata.fileIdentity?.scanFileSystemIdentity == nil) + #expect(SystemIntegration.verifyTrashIdentity(node) == .mismatch) + } + @Test func testFileSystemIdentityValidationUsesTheDedicatedProvider() throws { let url = URL(filePath: "/virtual/directory", directoryHint: .isDirectory) diff --git a/RadixCoreTests/ScanResultFingerprint.swift b/RadixCoreTests/ScanResultFingerprint.swift index 9566194..c77ccec 100644 --- a/RadixCoreTests/ScanResultFingerprint.swift +++ b/RadixCoreTests/ScanResultFingerprint.swift @@ -31,7 +31,7 @@ func scanResultFingerprint(_ store: FileTreeStore) -> String { if let fileIdentity = node.fileIdentity { fingerprint.append(true) switch fileIdentity { - case .resourceIdentifier(let data): + case .resourceIdentifier(let data, _): fingerprint.append(UInt64(0)) fingerprint.append(data) case .fileSystem(let device, let inode, _): diff --git a/RadixCoreTests/TrashFlowControllerTests.swift b/RadixCoreTests/TrashFlowControllerTests.swift index 16a934b..0d5c736 100644 --- a/RadixCoreTests/TrashFlowControllerTests.swift +++ b/RadixCoreTests/TrashFlowControllerTests.swift @@ -27,7 +27,6 @@ struct TrashFlowControllerTests { let foundationNode = makeTestFileNode(id: fileURL.path, name: "marked.txt", fileIdentity: foundationIdentity) #expect(SystemIntegration.verifyTrashIdentity(nativeNode) == (replaced ? .mismatch : .matches)) #expect(SystemIntegration.verifyTrashIdentity(foundationNode) == .matches) - try #require(nativeIdentity != foundationIdentity) func snapshot(_ node: FileNodeRecord) -> ScanSnapshot { let root = makeTestDirectoryNode(id: fixture.path, name: "fixture", children: [node]) @@ -46,6 +45,9 @@ struct TrashFlowControllerTests { ) #expect(preview.nodes == (replaced ? [] : [nativeNode])) #expect(preview.discardPileBatch?.unavailable.map(\.node) == (replaced ? [nativeNode] : [])) + + try FileManager.default.moveItem(at: fileURL, to: fixture.appending(path: "after-scan.txt")) + #expect(controller.discardPile.matchingNodeIDs(in: refreshed) == (replaced ? [] : [nativeNode.id])) } @Test From 1f08e28b0a401f0f09c2fc441f2b91db9da8f13f Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:32:51 -0800 Subject: [PATCH 07/12] fix(trash): verify discard pile moves A replacement can reuse a scanned inode, and a child can leave a marked directory before it moves. Verify the scanned item before Trash and keep covered marks until the move confirms their destination. Reject stale mark requests and keep the browser and sidebar cache in sync when scan rows change. --- Radix/Localizable.xcstrings | 16 +- Radix/Services/AppSystemActions.swift | 14 +- Radix/Services/BulkDirectoryEnumerator.swift | 16 +- Radix/Services/ScanArchiveModels.swift | 6 +- Radix/Services/ScanComparisonService.swift | 2 +- Radix/Services/ScanMetadataLoader.swift | 150 ++++++++++---- Radix/Services/SystemIntegration.swift | 34 +-- Radix/ViewModels/AppModel.swift | 72 ++++--- .../SidebarScanCacheController.swift | 8 +- Radix/ViewModels/TrashFlowController.swift | 124 ++++++++--- RadixCoreTests/AppModelDependencyTests.swift | 195 +++++++++++++++--- RadixCoreTests/ScanCoordinatorTests.swift | 18 +- RadixCoreTests/ScanEngineTests.swift | 1 + RadixCoreTests/ScanMetadataLoaderTests.swift | 8 +- RadixCoreTests/ScanResultFingerprint.swift | 4 +- .../SidebarScanCacheControllerTests.swift | 46 +++++ RadixCoreTests/SystemIntegrationTests.swift | 62 ++++++ RadixCoreTests/TrashFlowControllerTests.swift | 28 +-- 18 files changed, 596 insertions(+), 208 deletions(-) diff --git a/Radix/Localizable.xcstrings b/Radix/Localizable.xcstrings index ae11199..f002270 100644 --- a/Radix/Localizable.xcstrings +++ b/Radix/Localizable.xcstrings @@ -27642,43 +27642,43 @@ } } }, - "The item at %@ changed since this scan. Rescan before moving it to Trash." : { + "The item at %@ changed since this scan. Rescan and select it again." : { "comment" : "Safety error shown when a selected item changed after scanning.", "localizations" : { "de" : { "stringUnit" : { "state" : "translated", - "value" : "Das Element unter %@ wurde seit diesem Scan geändert. Scannen Sie es erneut, bevor Sie es in den Papierkorb bewegen." + "value" : "Das Element unter %@ wurde seit diesem Scan geändert. Führen Sie den Scan erneut aus und wählen Sie das Element erneut aus." } }, "es" : { "stringUnit" : { "state" : "translated", - "value" : "El elemento de %@ cambió desde este análisis. Vuelve a analizarlo antes de moverlo a la papelera." + "value" : "El elemento de %@ cambió desde este análisis. Vuelve a escanearlo y selecciónalo de nuevo." } }, "fr" : { "stringUnit" : { "state" : "translated", - "value" : "L’élément situé à %@ a changé depuis cette analyse. Réanalysez-le avant de le placer dans la corbeille." + "value" : "L’élément situé à %@ a changé depuis cette analyse. Relancez l’analyse, puis sélectionnez-le à nouveau." } }, "it" : { "stringUnit" : { "state" : "translated", - "value" : "L'elemento in %@ è cambiato dopo questa scansione. Esegui nuovamente la scansione prima di spostarlo nel Cestino." + "value" : "L'elemento in %@ è cambiato dopo questa scansione. Ripeti la scansione e selezionalo di nuovo." } }, "ru" : { "stringUnit" : { "state" : "translated", - "value" : "Объект по адресу %@ изменился после сканирования. Выполните повторное сканирование перед перемещением в Корзину." + "value" : "Объект по адресу %@ изменился после сканирования. Выполните сканирование ещё раз и снова выберите объект." } }, "zh-Hans" : { "stringUnit" : { "state" : "translated", - "value" : "%@ 处的项目自本次扫描后已更改。移到废纸篓前请重新扫描。" + "value" : "%@ 处的项目自本次扫描后已更改。请重新扫描并再次选择该项目。" } } } @@ -30732,4 +30732,4 @@ } }, "version" : "1.1" -} \ No newline at end of file +} diff --git a/Radix/Services/AppSystemActions.swift b/Radix/Services/AppSystemActions.swift index f608f0e..0ff6565 100644 --- a/Radix/Services/AppSystemActions.swift +++ b/Radix/Services/AppSystemActions.swift @@ -10,6 +10,16 @@ nonisolated enum TrashIdentityVerificationResult: Equatable, Sendable { case metadataUnavailable(String) } +nonisolated struct TrashMoveOutcome: Sendable { + let verification: TrashIdentityVerificationResult + let destination: URL? + + init(_ verification: TrashIdentityVerificationResult, destination: URL? = nil) { + self.verification = verification + self.destination = destination + } +} + @MainActor struct AppSystemActions { var open: (URL) throws -> Void @@ -18,7 +28,7 @@ struct AppSystemActions { var revealMany: ([URL]) -> Void var copyPath: (URL) throws -> Void var copyPaths: ([URL]) throws -> Void - var moveToTrash: @MainActor (FileNodeRecord) async throws -> TrashIdentityVerificationResult + var moveToTrash: @MainActor (FileNodeRecord) async throws -> TrashMoveOutcome var validateQuickLookSelection: ([FileNodeRecord], ScanSnapshotSource) async throws -> Void var prepareAndOpenFullDiskAccessSettings: () -> Bool var fullDiskAccessStatus: @MainActor () async -> FullDiskAccessStatus @@ -130,7 +140,7 @@ struct AppSystemActions { revealMany: { _ in }, copyPath: { _ in }, copyPaths: { _ in }, - moveToTrash: { _ in .matches }, + moveToTrash: { _ in TrashMoveOutcome(.matches) }, validateQuickLookSelection: { _, _ in }, prepareAndOpenFullDiskAccessSettings: { true }, fullDiskAccessStatus: { .unknown }, diff --git a/Radix/Services/BulkDirectoryEnumerator.swift b/Radix/Services/BulkDirectoryEnumerator.swift index a5cbe70..1fafdab 100644 --- a/Radix/Services/BulkDirectoryEnumerator.swift +++ b/Radix/Services/BulkDirectoryEnumerator.swift @@ -467,7 +467,7 @@ nonisolated enum BulkDirectoryEnumerator { } private static var requestedCommonAttributes: attrgroup_t { - requiredCommonAttributes | attrgroup_t(ATTR_CMN_FNDRINFO) + requiredCommonAttributes | attrgroup_t(ATTR_CMN_CRTIME | ATTR_CMN_FNDRINFO) } private static var requiredCommonAttributes: attrgroup_t { @@ -521,8 +521,7 @@ nonisolated enum BulkDirectoryEnumerator { let dataAllocatedSize: Int64 let modificationTime: timespec let isReadable: Bool - let device: UInt64 - let inode: UInt64 + let fileIdentity: FileIdentity let linkCount: UInt64 let cloneIdentity: CloneIdentity? let mayShareDataBlocks: Bool @@ -657,6 +656,7 @@ nonisolated enum BulkDirectoryEnumerator { ), let deviceID: dev_t = cursor.read(), let objectType: fsobj_type_t = cursor.read(), + let creationTime: timespec = cursor.read(), let modificationTime: timespec = cursor.read() else { return nil } @@ -748,6 +748,9 @@ nonisolated enum BulkDirectoryEnumerator { } else { cloneIdentity = nil } + let birthTime = returned.commonattr & attrgroup_t(ATTR_CMN_CRTIME) != 0 + ? FileIdentity.birthTime(from: creationTime) + : nil let metadata = ParsedEntryMetadata( isSymbolicLink: isSymbolicLink, logicalSize: max(Int64(logicalSize), 0), @@ -755,8 +758,9 @@ nonisolated enum BulkDirectoryEnumerator { dataAllocatedSize: max(Int64(dataAllocatedSize), 0), modificationTime: modificationTime, isReadable: (userAccess & UInt32(R_OK)) != 0, - device: UInt64(truncatingIfNeeded: deviceID), - inode: fileID, + fileIdentity: FileIdentity( + device: UInt64(truncatingIfNeeded: deviceID), inode: fileID, birthTime: birthTime + ), linkCount: isDirectory ? 1 : max(UInt64(fileLinkCount), 1), cloneIdentity: cloneIdentity, mayShareDataBlocks: !isDirectory && !isSymbolicLink && @@ -816,7 +820,7 @@ nonisolated enum BulkDirectoryEnumerator { lastModified: lastModified, isReadable: metadata.isReadable, volumeCapacity: nil, - fileIdentity: FileIdentity(device: metadata.device, inode: metadata.inode), + fileIdentity: metadata.fileIdentity, linkCount: metadata.linkCount, cloneIdentity: metadata.cloneIdentity, mayShareDataBlocks: metadata.mayShareDataBlocks diff --git a/Radix/Services/ScanArchiveModels.swift b/Radix/Services/ScanArchiveModels.swift index 8b5c359..d2a8fb9 100644 --- a/Radix/Services/ScanArchiveModels.swift +++ b/Radix/Services/ScanArchiveModels.swift @@ -509,18 +509,18 @@ nonisolated struct ScanArchiveFileIdentity: Codable, Sendable { init(_ identity: FileIdentity) { switch identity { - case .resourceIdentifier(let data, _): + case .resourceIdentifier(let data, _, _): self.kind = .resourceIdentifier self.resourceIdentifier = data.base64EncodedString() self.device = nil self.inode = nil self.volumeToken = nil - case .fileSystem(let device, let inode, let volumeToken): + case .fileSystem(let device, let inode, _, _): self.kind = .fileSystem self.resourceIdentifier = nil self.device = device self.inode = inode - self.volumeToken = volumeToken + self.volumeToken = identity.darwinIdentity?.volumeToken } } diff --git a/Radix/Services/ScanComparisonService.swift b/Radix/Services/ScanComparisonService.swift index 62e40a3..c01ee61 100644 --- a/Radix/Services/ScanComparisonService.swift +++ b/Radix/Services/ScanComparisonService.swift @@ -1061,7 +1061,7 @@ nonisolated struct ScanComparisonService: Sendable { } // Bulk records borrow a preserved token only for their own device; // an APFS startup scan may span multiple volumes. - if case .fileSystem(let device, let inode, _) = identity { + if case .fileSystem(let device, let inode, _, _) = identity { guard !context.ambiguousDevices.contains(device) else { return nil } if let volumeToken = context.volumeTokens[device] { return .darwin(FileIdentity.DarwinIdentity(fileID: inode, volumeToken: volumeToken)) diff --git a/Radix/Services/ScanMetadataLoader.swift b/Radix/Services/ScanMetadataLoader.swift index b5cff7a..0233a5e 100644 --- a/Radix/Services/ScanMetadataLoader.swift +++ b/Radix/Services/ScanMetadataLoader.swift @@ -269,6 +269,7 @@ nonisolated struct ScanMetadataLoader: Sendable { .totalFileAllocatedSizeKey, .fileSizeKey, .totalFileSizeKey, + .creationDateKey, .contentModificationDateKey, .isReadableKey, .linkCountKey, @@ -282,6 +283,7 @@ nonisolated struct ScanMetadataLoader: Sendable { .totalFileAllocatedSizeKey, .fileSizeKey, .totalFileSizeKey, + .creationDateKey, .contentModificationDateKey, .isReadableKey, .linkCountKey, @@ -297,6 +299,7 @@ nonisolated struct ScanMetadataLoader: Sendable { .totalFileAllocatedSizeKey, .fileSizeKey, .totalFileSizeKey, + .creationDateKey, .isReadableKey, .linkCountKey, .fileResourceIdentifierKey @@ -509,7 +512,9 @@ nonisolated struct ScanMetadataLoader: Sendable { max(allocatedSize, 0) ) let isReadable = values.isReadable ?? false - let resourceIdentity = Self.fileIdentity(from: values.fileResourceIdentifier) + let resourceIdentity = Self.fileIdentity( + from: values.fileResourceIdentifier, birthTime: values.creationDate + ) var fileIdentity = resourceIdentity var linkCount = values.linkCount.map(UInt64.init) ?? 1 if isSymbolicLink && loadsSymbolicLinkFileSystemInfo { @@ -534,7 +539,7 @@ nonisolated struct ScanMetadataLoader: Sendable { linkCount = values.linkCount.map(UInt64.init) ?? fileSystemInfo.linkCount } fileIdentity = fileIdentity? - .preservingVolumeIdentity(from: resourceIdentity) + .preservingResourceIdentity(from: resourceIdentity, isDirectory: isDirectory && !isSymbolicLink) .preservingScanDevice(from: status?.fileIdentity) let cloneMetadata = !isDirectory && !isSymbolicLink ? cloneMappingCapabilityCache.cloneMetadata(for: url) @@ -704,10 +709,11 @@ nonisolated struct ScanMetadataLoader: Sendable { } private nonisolated static func fileIdentity( - from resourceIdentifier: (any NSCopying & NSSecureCoding & NSObjectProtocol)? + from resourceIdentifier: (any NSCopying & NSSecureCoding & NSObjectProtocol)?, + birthTime: Date? ) -> FileIdentity? { guard let identifierData = resourceIdentifier as? Data else { return nil } - return FileIdentity(resourceIdentifier: identifierData) + return FileIdentity(resourceIdentifier: identifierData, birthTime: birthTime) } } @@ -766,23 +772,65 @@ nonisolated struct CloneIdentity: Hashable, Sendable { } nonisolated enum FileIdentity: Hashable, Sendable { - case resourceIdentifier(Data, scanDevice: UInt64? = nil) - case fileSystem(device: UInt64, inode: UInt64, volumeToken: UInt64? = nil) + enum NativeResourceEvidence: Sendable { + case volumeToken(UInt64) + case directoryIdentifier(Data) + } - nonisolated init(device: UInt64, inode: UInt64, volumeToken: UInt64? = nil) { - self = .fileSystem(device: device, inode: inode, volumeToken: volumeToken) + case resourceIdentifier(Data, scanDevice: UInt64? = nil, birthTime: Date? = nil) + case fileSystem(device: UInt64, inode: UInt64, resourceEvidence: NativeResourceEvidence? = nil, birthTime: Date? = nil) + + nonisolated init(device: UInt64, inode: UInt64, volumeToken: UInt64? = nil, birthTime: Date? = nil) { + self = .fileSystem( + device: device, inode: inode, + resourceEvidence: volumeToken.map(NativeResourceEvidence.volumeToken), birthTime: birthTime + ) } nonisolated init(fileSystemStatus status: stat) { // dev_t is signed; preserve negative device IDs without trapping. self.init( device: UInt64(truncatingIfNeeded: status.st_dev), - inode: UInt64(status.st_ino) + inode: UInt64(status.st_ino), + birthTime: Self.birthTime(from: status.st_birthtimespec) ) } - nonisolated init(resourceIdentifier: Data) { - self = .resourceIdentifier(resourceIdentifier) + nonisolated init(resourceIdentifier: Data, birthTime: Date? = nil) { + self = .resourceIdentifier(resourceIdentifier, birthTime: birthTime) + } + + static func birthTime(from value: timespec) -> Date? { + // Darwin returns a zero timespec when the filesystem lacks birth time. + guard value.tv_sec != 0 || value.tv_nsec != 0 else { return nil } + // Use Foundation's reference epoch so equivalent resource dates compare exactly. + return Date( + timeIntervalSinceReferenceDate: Double(value.tv_sec) - Date.timeIntervalBetween1970AndReferenceDate + + Double(value.tv_nsec) / 1_000_000_000 + ) + } + + var birthTime: Date? { + switch self { + case .resourceIdentifier(_, _, let birthTime), .fileSystem(_, _, _, let birthTime): + birthTime + } + } + + var trashResourceIdentifier: Data? { + switch self { + case .resourceIdentifier(let data, _, _), .fileSystem(_, _, .directoryIdentifier(let data), _): + data + case .fileSystem: + nil + } + } + + func hasConflictingTrashEvidence(with other: Self) -> Bool { + if let identifier = trashResourceIdentifier, let otherIdentifier = other.trashResourceIdentifier, + identifier != otherIdentifier { return true } + guard let birthTime, let otherBirthTime = other.birthTime else { return false } + return birthTime != otherBirthTime } nonisolated var isFileSystemIdentity: Bool { @@ -793,7 +841,7 @@ nonisolated enum FileIdentity: Hashable, Sendable { } nonisolated var fileSystemDeviceID: UInt64? { - if case .fileSystem(let device, _, _) = self { + if case .fileSystem(let device, _, _, _) = self { return device } return nil @@ -802,9 +850,9 @@ nonisolated enum FileIdentity: Hashable, Sendable { /// Supplementary metadata must not change identity equality or hashing. static func == (lhs: Self, rhs: Self) -> Bool { switch (lhs, rhs) { - case let (.resourceIdentifier(lhs, _), .resourceIdentifier(rhs, _)): + case let (.resourceIdentifier(lhs, _, _), .resourceIdentifier(rhs, _, _)): return lhs == rhs - case let (.fileSystem(lhsDevice, lhsInode, _), .fileSystem(rhsDevice, rhsInode, _)): + case let (.fileSystem(lhsDevice, lhsInode, _, _), .fileSystem(rhsDevice, rhsInode, _, _)): return lhsDevice == rhsDevice && lhsInode == rhsInode default: return false @@ -813,10 +861,10 @@ nonisolated enum FileIdentity: Hashable, Sendable { func hash(into hasher: inout Hasher) { switch self { - case .resourceIdentifier(let data, _): + case .resourceIdentifier(let data, _, _): hasher.combine(0) hasher.combine(data) - case .fileSystem(let device, let inode, _): + case .fileSystem(let device, let inode, _, _): hasher.combine(1) hasher.combine(device) hasher.combine(inode) @@ -833,18 +881,27 @@ nonisolated enum FileIdentity: Hashable, Sendable { /// their opaque identity. Native identities can preserve the same token. var darwinIdentity: DarwinIdentity? { switch self { - case .fileSystem(_, let inode, let volumeToken): - return volumeToken.map { DarwinIdentity(fileID: inode, volumeToken: $0) } - case .resourceIdentifier(let data, _): - guard data.count == MemoryLayout.size * 2 else { return nil } - return data.withUnsafeBytes { bytes in - DarwinIdentity( - fileID: UInt64(littleEndian: bytes.loadUnaligned(as: UInt64.self)), - volumeToken: UInt64(littleEndian: bytes.loadUnaligned( - fromByteOffset: MemoryLayout.size, as: UInt64.self - )) - ) - } + case .fileSystem(_, let inode, .volumeToken(let token), _): + return DarwinIdentity(fileID: inode, volumeToken: token) + case .fileSystem(_, let inode, .directoryIdentifier(let data), _): + guard let identity = Self.decodeDarwinIdentity(data), identity.fileID == inode else { return nil } + return identity + case .fileSystem: + return nil + case .resourceIdentifier(let data, _, _): + return Self.decodeDarwinIdentity(data) + } + } + + private static func decodeDarwinIdentity(_ data: Data) -> DarwinIdentity? { + guard data.count == MemoryLayout.size * 2 else { return nil } + return data.withUnsafeBytes { bytes in + DarwinIdentity( + fileID: UInt64(littleEndian: bytes.loadUnaligned(as: UInt64.self)), + volumeToken: UInt64(littleEndian: bytes.loadUnaligned( + fromByteOffset: MemoryLayout.size, as: UInt64.self + )) + ) } } @@ -852,9 +909,9 @@ nonisolated enum FileIdentity: Hashable, Sendable { switch self { case .fileSystem: self - case .resourceIdentifier(_, let device): + case .resourceIdentifier(_, let device, let birthTime): if let device, let identity = darwinIdentity { - .fileSystem(device: device, inode: identity.fileID) + .fileSystem(device: device, inode: identity.fileID, birthTime: birthTime) } else { nil } @@ -863,16 +920,27 @@ nonisolated enum FileIdentity: Hashable, Sendable { func preservingScanDevice(from statusIdentity: FileIdentity?) -> Self { // Prefetched metadata and lstat may observe different files at the same path. - guard case .resourceIdentifier(let data, _) = self, - case .fileSystem(let device, let inode, _) = statusIdentity, - darwinIdentity?.fileID == inode else { return self } - return .resourceIdentifier(data, scanDevice: device) - } - - func preservingVolumeIdentity(from resourceIdentity: FileIdentity?) -> Self { - guard case .fileSystem(let device, let inode, _) = self, - let identity = resourceIdentity?.darwinIdentity, - identity.fileID == inode else { return self } - return .fileSystem(device: device, inode: inode, volumeToken: identity.volumeToken) + guard case .resourceIdentifier(let data, _, let birthTime) = self, + let statusIdentity, + case .fileSystem(let device, let inode, _, _) = statusIdentity, + darwinIdentity?.fileID == inode, + !hasConflictingTrashEvidence(with: statusIdentity) else { return self } + return .resourceIdentifier(data, scanDevice: device, birthTime: birthTime) + } + + func preservingResourceIdentity(from resourceIdentity: FileIdentity?, isDirectory: Bool) -> Self { + guard case .fileSystem(let device, let inode, _, let birthTime) = self, + let resourceIdentity else { return self } + let evidence: NativeResourceEvidence + if isDirectory, let identifier = resourceIdentity.trashResourceIdentifier { + // Some filesystems recycle stat's inode before reusing the resource identifier. + evidence = .directoryIdentifier(identifier) + } else if let identity = resourceIdentity.darwinIdentity, identity.fileID == inode, + !hasConflictingTrashEvidence(with: resourceIdentity) { + evidence = .volumeToken(identity.volumeToken) + } else { + return self + } + return .fileSystem(device: device, inode: inode, resourceEvidence: evidence, birthTime: birthTime) } } diff --git a/Radix/Services/SystemIntegration.swift b/Radix/Services/SystemIntegration.swift index c42078f..0725c0c 100644 --- a/Radix/Services/SystemIntegration.swift +++ b/Radix/Services/SystemIntegration.swift @@ -444,15 +444,16 @@ enum SystemIntegration { /// practical window while preserving native per-volume Trash behavior. nonisolated static func moveToTrash( _ node: FileNodeRecord - ) throws -> TrashIdentityVerificationResult { - try moveToTrash( + ) throws -> TrashMoveOutcome { + var resultingItemURL: NSURL? + let verification = try moveToTrash( node, identityVerifier: verifyTrashIdentity, trashItem: { url in - var resultingItemURL: NSURL? try FileManager.default.trashItem(at: url, resultingItemURL: &resultingItemURL) } ) + return TrashMoveOutcome(verification, destination: resultingItemURL as URL?) } nonisolated static func moveToTrash( @@ -471,23 +472,30 @@ enum SystemIntegration { } nonisolated static func verifyTrashIdentity(_ node: FileNodeRecord) -> TrashIdentityVerificationResult { + verifyTrashIdentity(node, at: node.url) + } + + nonisolated static func verifyTrashIdentity(_ node: FileNodeRecord, at url: URL) -> TrashIdentityVerificationResult { guard !node.isSynthetic else { return .matches } - guard let scannedIdentity = node.fileIdentity else { + guard let scannedIdentity = node.fileIdentity, let birthTime = scannedIdentity.birthTime else { return .missingScannedIdentity } - switch currentFileSystemIdentity(for: node.url) { - case .success(let currentIdentity): - if node.isSymbolicLink || scannedIdentity.isFileSystemIdentity { - return currentIdentity == scannedIdentity ? .matches : .mismatch + let requiresNativeIdentity = node.isSymbolicLink || scannedIdentity.isFileSystemIdentity + switch currentFileSystemIdentity(for: url) { + case .success(let identity): + if requiresNativeIdentity, identity != scannedIdentity || identity.birthTime != birthTime { + return .mismatch } case .failure(let error): return error.verificationResult } - switch currentResourceIdentity(for: node.url) { - case .success(let currentIdentity): - return currentIdentity == scannedIdentity ? .matches : .mismatch + guard let resourceIdentifier = scannedIdentity.trashResourceIdentifier else { return .matches } + switch currentResourceIdentity(for: url) { + case .success(let identity): + return identity.trashResourceIdentifier == resourceIdentifier && + (requiresNativeIdentity || identity.birthTime == birthTime) ? .matches : .mismatch case .failure(let error): return error.verificationResult } @@ -517,11 +525,11 @@ enum SystemIntegration { private nonisolated static func currentResourceIdentity(for url: URL) -> Result { let currentURL = URL(filePath: url.path) do { - let values = try currentURL.resourceValues(forKeys: [.fileResourceIdentifierKey]) + let values = try currentURL.resourceValues(forKeys: [.fileResourceIdentifierKey, .creationDateKey]) guard let identifierData = values.fileResourceIdentifier as? Data else { return .failure(.metadataUnavailable(String(localized: "file resource identifier unavailable", comment: "Reason the selected file identity could not be verified."))) } - return .success(FileIdentity(resourceIdentifier: identifierData)) + return .success(FileIdentity(resourceIdentifier: identifierData, birthTime: values.creationDate)) } catch { let nsError = error as NSError if nsError.domain == NSCocoaErrorDomain && nsError.code == NSFileNoSuchFileError { diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index 1eccdee..bbfef0c 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -71,7 +71,7 @@ nonisolated enum FileActionError: LocalizedError, Sendable { case .unavailable(let path): return String(localized: "The item at \(path) is no longer available.", comment: "Error shown when a selected item has disappeared.") case .changedSinceScan(let path): - return String(localized: "The item at \(path) changed since this scan. Rescan before moving it to Trash.", comment: "Safety error shown when a selected item changed after scanning.") + return String(localized: "The item at \(path) changed since this scan. Rescan and select it again.", comment: "Safety error shown when a selected item changed after scanning.") case .missingScannedIdentity(let path): return String(localized: "Radix could not verify the scanned identity for \(path). Rescan before moving it to Trash.", comment: "Safety error shown when a scanned file identity cannot be verified.") case .currentIdentityUnavailable(let path, let reason): @@ -2410,15 +2410,7 @@ final class AppModel: ObservableObject { @discardableResult func addNodesToDiscardPile(_ nodes: [FileNodeRecord]) -> Bool { do { - let nodes = try validatedNodesForDiscardPile(nodes) - guard nodes.allSatisfy({ node in - node.supportsMoveToTrash( - activeTarget: scanCoordinator.selectedTarget, - trashSafetyPolicy: scanCoordinator.trashSafetyPolicy - ) - }) else { - throw FileActionError.unsupported - } + try validateDiscardPileAddition(nodes) let discardNodes = topLevelTrashNodes(from: nodes) if let cloudImpact = cloudStorageImpact(of: discardNodes.map(\.url)) { pendingCloudFileAction = PendingCloudFileAction( @@ -2580,8 +2572,8 @@ final class AppModel: ObservableObject { do { switch action.kind { case .addToDiscardPile: - let nodes = try validatedNodesForDiscardPile(action.nodes) - try commitDiscardPileAddition(topLevelTrashNodes(from: nodes)) + try validateDiscardPileAddition(action.nodes) + try commitDiscardPileAddition(topLevelTrashNodes(from: action.nodes)) case .moveToTrash: let nodes: [FileNodeRecord] if action.discardPileBatch == nil { @@ -2653,10 +2645,11 @@ final class AppModel: ObservableObject { trashFlow.startConfirmedMove( nodes, + discardPileBatch: discardPileBatch, moveToTrash: dependencies.systemActions.moveToTrash, beginMove: { let didHide = trashFlow.hideTrashNodesDuringMove( - discardPileBatch?.affectedNodes(for: nodes) ?? nodes, + discardPileBatch?.plannedNodes(for: nodes) ?? nodes, snapshotID: originalSnapshotID, activeSnapshotID: scanCoordinator.snapshot?.id, activeFileTreeStore: statsFileTreeStore @@ -2668,14 +2661,13 @@ final class AppModel: ObservableObject { ) } }, - onFinish: { [weak self] requested, moved, actionError, wasCancelled in + onFinish: { [weak self] requested, moved, actionError, _ in self?.finishConfirmedTrashMove( requestedNodes: requested, - movedNodes: moved, + moved: moved, actionError: actionError, originalSnapshot: originalSnapshot, - discardPileBatch: discardPileBatch, - wasCancelled: wasCancelled + discardPileBatch: discardPileBatch ) } ) @@ -2683,15 +2675,14 @@ final class AppModel: ObservableObject { private func finishConfirmedTrashMove( requestedNodes: [FileNodeRecord], - movedNodes: [FileNodeRecord], + moved: TrashFlowController.CompletedTrashMoves, actionError: Error?, originalSnapshot: ScanSnapshot?, - discardPileBatch: TrashFlowController.DiscardPileTrashBatch?, - wasCancelled: Bool = false + discardPileBatch: TrashFlowController.DiscardPileTrashBatch? ) { let originalSnapshotID = originalSnapshot?.id - let affectedMovedNodes = discardPileBatch?.affectedNodes(for: movedNodes) ?? movedNodes - if !movedNodes.isEmpty { + let affectedMovedNodes = moved.affectedNodes + if !moved.operations.isEmpty { trashFlow.invalidateDiscardPileHistory() sidebarScanCacheController.clearCache() reconcileScanAfterTrash( @@ -2702,17 +2693,15 @@ final class AppModel: ObservableObject { trashFlow.removeMovedDiscardPileNodes( affectedMovedNodes, from: originalSnapshot, includeSnapshotDescendants: discardPileBatch == nil ) - recordTrashMove(movedNodes, fileTreeStore: originalSnapshot?.treeStore) + recordTrashMove(moved.operations, fileTreeStore: originalSnapshot?.treeStore) refreshAvailableTargets() } - if actionError != nil || wasCancelled { - trashFlow.unhideTrashNodesAfterFailedMove( - requestedNodes: discardPileBatch?.affectedNodes(for: requestedNodes) ?? requestedNodes, - movedNodes: affectedMovedNodes, - snapshotID: originalSnapshotID - ) - } + trashFlow.unhideUnmovedTrashNodes( + requestedNodes: discardPileBatch?.plannedNodes(for: requestedNodes) ?? requestedNodes, + movedNodes: affectedMovedNodes, + snapshotID: originalSnapshotID + ) if let actionError { presentError(actionError) } } @@ -2826,6 +2815,8 @@ final class AppModel: ObservableObject { sameSnapshot: Bool, requiresMissingPath: Bool = false ) -> Bool { + if let movedIdentity = moved.fileIdentity, let currentIdentity = current.fileIdentity, + movedIdentity.hasConflictingTrashEvidence(with: currentIdentity) { return false } if moved.fileIdentity == current.fileIdentity { guard moved.fileIdentity != nil || sameSnapshot else { return false } return !requiresMissingPath || dependencies.systemActions.verifyTrashIdentity(current) == .missingCurrentItem @@ -3028,9 +3019,20 @@ final class AppModel: ObservableObject { return try validatedNodes(visibleNodes, requiresLivePath: true) } - private func validatedNodesForDiscardPile(_ nodes: [FileNodeRecord]) throws -> [FileNodeRecord] { + private func validateDiscardPileAddition(_ nodes: [FileNodeRecord]) throws { try validateSnapshotAllowsMutation() - return try validatedNodes(nodes, requiresLivePath: false) + guard !nodes.isEmpty else { throw FileActionError.noSelection } + guard let tree = scanCoordinator.fileTreeStore else { throw FileActionError.unsupported } + for node in nodes { + guard node.supportsMoveToTrash( + activeTarget: scanCoordinator.selectedTarget, + trashSafetyPolicy: scanCoordinator.trashSafetyPolicy + ) else { throw FileActionError.unsupported } + guard let current = tree.node(id: node.id), + DiscardPileState.Mark.matchesIdentity(of: node, with: current) else { + throw FileActionError.changedSinceScan(path: node.url.path) + } + } } private func validateLivePathAction(_ node: FileNodeRecord) throws { @@ -3340,7 +3342,11 @@ final class AppModel: ObservableObject { quickLookController.closePreview() refreshDiskFreeSpaceCapacity(for: snapshot) syncOptimisticTrashVisibility(with: snapshot) - projectedDiscardPileIDs = discardPile.matchingNodeIDs(in: snapshot) + let matchingNodeIDs = discardPile.matchingNodeIDs(in: snapshot) + if matchingNodeIDs != projectedDiscardPileIDs { + objectWillChange.send() + projectedDiscardPileIDs = matchingNodeIDs + } fileTransferReconciliation.displayedSnapshotDidChange() if let snapshotID = snapshot?.id, snapshotID == deferredNavigationContextSnapshotID { diff --git a/Radix/ViewModels/SidebarScanCacheController.swift b/Radix/ViewModels/SidebarScanCacheController.swift index 064cac1..e989e9e 100644 --- a/Radix/ViewModels/SidebarScanCacheController.swift +++ b/Radix/ViewModels/SidebarScanCacheController.swift @@ -221,23 +221,23 @@ final class SidebarScanCacheController { func invalidateDisplayedNodes(_ nodes: [FileNodeRecord], in snapshot: ScanSnapshot) { let previous = movedDisplayedNodes?.snapshotID == snapshot.id - ? movedDisplayedNodes?.nodes.filter { Self.contains($0, in: snapshot) } ?? [] + ? movedDisplayedNodes?.nodes.filter { Self.mayContain($0, in: snapshot) } ?? [] : [] movedDisplayedNodes = (snapshot.id, previous + nodes) } private func isReusable(_ snapshot: ScanSnapshot) -> Bool { guard let moved = movedDisplayedNodes else { return true } - if moved.snapshotID == snapshot.id && moved.nodes.contains(where: { Self.contains($0, in: snapshot) }) { + if moved.snapshotID == snapshot.id && moved.nodes.contains(where: { Self.mayContain($0, in: snapshot) }) { return false } movedDisplayedNodes = nil return true } - private static func contains(_ node: FileNodeRecord, in snapshot: ScanSnapshot) -> Bool { + private static func mayContain(_ node: FileNodeRecord, in snapshot: ScanSnapshot) -> Bool { guard let current = snapshot.treeStore.node(id: node.id) else { return false } - return current.fileIdentity == node.fileIdentity + return DiscardPileState.Mark.compareIdentity(of: node, with: current) != .different } func clearCache() { diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index c326d57..b45713a 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -3,6 +3,12 @@ import Foundation struct DiscardPileState: Equatable, Sendable { struct Mark: Equatable, Sendable { + enum IdentityComparison { + case same + case different + case unknown + } + let node: FileNodeRecord let unidentifiedTreeID: UUID? @@ -11,6 +17,12 @@ struct DiscardPileState: Equatable, Sendable { unidentifiedTreeID = node.fileIdentity == nil ? treeID : nil } + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.node == rhs.node && lhs.unidentifiedTreeID == rhs.unidentifiedTreeID + && lhs.node.fileIdentity?.birthTime == rhs.node.fileIdentity?.birthTime + && lhs.node.fileIdentity?.trashResourceIdentifier == rhs.node.fileIdentity?.trashResourceIdentifier + } + func matches(_ current: FileNodeRecord, treeID: UUID) -> Bool { guard node.id == current.id else { return false } if node.fileIdentity == nil { @@ -20,16 +32,28 @@ struct DiscardPileState: Equatable, Sendable { } static func matchesIdentity(of original: FileNodeRecord, with current: FileNodeRecord) -> Bool { - guard original.id == current.id else { return false } - if original.fileIdentity == current.fileIdentity { return true } + compareIdentity(of: original, with: current) == .same + } + + static func compareIdentity(of original: FileNodeRecord, with current: FileNodeRecord) -> IdentityComparison { + guard original.id == current.id else { return .different } + guard let first = original.fileIdentity, let second = current.fileIdentity else { + return original.fileIdentity == current.fileIdentity ? .same : .different + } + guard !first.hasConflictingTrashEvidence(with: second) else { return .different } + if first == second { return .same } guard !original.isSymbolicLink, !current.isSymbolicLink, - let first = original.fileIdentity, let second = current.fileIdentity, - first.isFileSystemIdentity != second.isFileSystemIdentity else { return false } + first.isFileSystemIdentity != second.isFileSystemIdentity else { return .different } + if let identifier = first.trashResourceIdentifier, identifier == second.trashResourceIdentifier { + return .same + } if let firstDarwin = first.darwinIdentity, let secondDarwin = second.darwinIdentity { - return firstDarwin == secondDarwin + return firstDarwin == secondDarwin ? .same : .different + } + if let firstNative = first.scanFileSystemIdentity, let secondNative = second.scanFileSystemIdentity { + return firstNative == secondNative ? .same : .different } - guard let firstNative = first.scanFileSystemIdentity else { return false } - return firstNative == second.scanFileSystemIdentity + return .unknown } } @@ -74,15 +98,47 @@ final class TrashFlowController { let message: String } + struct CoveredMark: Sendable { + let node: FileNodeRecord + let relativePath: String + } + let unavailable: [Unavailable] let resolvedURLsByOperationID: [FileNodeRecord.ID: URL] - let coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] + let coveredMarks: [FileNodeRecord.ID: [CoveredMark]] + + func plannedNodes(for operations: [FileNodeRecord]) -> [FileNodeRecord] { + operations + operations.flatMap { coveredMarks[$0.id]?.map(\.node) ?? [] } + } - func affectedNodes(for operations: [FileNodeRecord]) -> [FileNodeRecord] { - operations + operations.flatMap { coveredMarks[$0.id] ?? [] } + func verifyCoveredMarks(for operation: FileNodeRecord, at destination: URL) -> [FileNodeRecord] { + guard let marks = coveredMarks[operation.id] else { return [] } + let canonicalDestination = try? destination.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath + return marks.compactMap { mark in + let movedURL: URL + if mark.relativePath.isEmpty { + let original = SystemIntegration.verifyTrashIdentity(mark.node) + guard original == .missingCurrentItem || original == .mismatch else { return nil } + movedURL = destination + } else { + movedURL = destination.appending(path: mark.relativePath) + // A replaced intermediate directory could redirect this path outside Trash. + guard let canonicalDestination, + let path = try? movedURL.resourceValues(forKeys: [.canonicalPathKey]).canonicalPath, + path.hasPrefix(canonicalDestination + "/") else { return nil } + } + return SystemIntegration.verifyTrashIdentity(mark.node, at: movedURL) == .matches ? mark.node : nil + } } } + struct CompletedTrashMoves { + var operations: [FileNodeRecord] = [] + var coveredMarks: [FileNodeRecord] = [] + + var affectedNodes: [FileNodeRecord] { operations + coveredMarks } + } + nonisolated struct PendingTrashSelection: Sendable { let nodes: [FileNodeRecord] var discardPileBatch: DiscardPileTrashBatch? = nil @@ -251,22 +307,23 @@ final class TrashFlowController { let entriesByPath = Dictionary(eligible.map { ($0.resolvedURL.path, $0.node) }, uniquingKeysWith: { first, _ in first }) var operations: [(node: FileNodeRecord, resolvedURL: URL)] = [] - var coveredMarks: [FileNodeRecord.ID: [FileNodeRecord]] = [:] + var coveredMarks: [FileNodeRecord.ID: [DiscardPileTrashBatch.CoveredMark]] = [:] for (node, resolvedURL) in eligible { try Task.checkCancellation() let path = resolvedURL.path - var coveringID = entriesByPath[path]?.id ?? node.id + var covering = (id: entriesByPath[path]?.id ?? node.id, path: path) var parent = path while let separator = parent.lastIndex(of: "/"), separator != parent.startIndex { parent = String(parent[.. (pile: DiscardPileState, matchingNodeIDs: Set) { let tree = snapshot.treeStore var updated = discardPile.marks var offsets = Dictionary(uniqueKeysWithValues: updated.enumerated().map { ($0.element.node.id, $0.offset) }) - for node in nodes { + for node in validatedNodes { let mark = DiscardPileState.Mark(node: node, treeID: tree.contentID) - guard let current = tree.node(id: node.id), mark.matches(current, treeID: tree.contentID) else { continue } if let offset = offsets[node.id] { if !updated[offset].matches(node, treeID: tree.contentID) { updated[offset] = mark } } else { @@ -309,7 +365,7 @@ final class TrashFlowController { } } - let incomingIDs = Set(nodes.map(\.id)) + let incomingIDs = Set(validatedNodes.map(\.id)) let matchingIDs = DiscardPileState(marks: updated).matchingNodeIDs(in: snapshot) var retainedIDs = matchingIDs let retainedMarks = updated.filter { mark in @@ -342,7 +398,8 @@ final class TrashFlowController { let removedSubtrees: PreparedFileTreeNodeSet? = if includeSnapshotDescendants, let tree { tree.preparedNodeSet(for: Set(movedNodes.compactMap { node in guard node.isDirectory, !node.isSymbolicLink, - let current = tree.node(id: node.id), current.fileIdentity == node.fileIdentity else { return nil } + let current = tree.node(id: node.id), + DiscardPileState.Mark.matchesIdentity(of: node, with: current) else { return nil } return node.id })) } else { @@ -351,7 +408,7 @@ final class TrashFlowController { let remaining = discardPile.marks.filter { mark in if let node = removedByID[mark.node.id] { - return mark.node.fileIdentity != node.fileIdentity + return !DiscardPileState.Mark.matchesIdentity(of: mark.node, with: node) || (node.fileIdentity == nil && mark.unidentifiedTreeID != tree?.contentID) } guard let tree, let removedSubtrees, @@ -462,7 +519,7 @@ final class TrashFlowController { return true } - func unhideTrashNodesAfterFailedMove( + func unhideUnmovedTrashNodes( requestedNodes: [FileNodeRecord], movedNodes: [FileNodeRecord], snapshotID: UUID? @@ -483,28 +540,35 @@ final class TrashFlowController { func startConfirmedMove( _ nodes: [FileNodeRecord], - moveToTrash: @escaping @MainActor (FileNodeRecord) async throws -> TrashIdentityVerificationResult, + discardPileBatch: DiscardPileTrashBatch? = nil, + moveToTrash: @escaping @MainActor (FileNodeRecord) async throws -> TrashMoveOutcome, beginMove: () -> Void, - onFinish: @escaping @MainActor (_ requested: [FileNodeRecord], _ moved: [FileNodeRecord], _ actionError: Error?, _ wasCancelled: Bool) -> Void + onFinish: @escaping @MainActor (_ requested: [FileNodeRecord], _ moved: CompletedTrashMoves, _ actionError: Error?, _ wasCancelled: Bool) -> Void ) { guard !isReadOnlyMode else { - onFinish(nodes, [], FileActionError.readOnlyMode, false) + onFinish(nodes, CompletedTrashMoves(), FileActionError.readOnlyMode, false) return } let requestID = UUID() confirmedTrashMoveTasks[requestID] = Task { [weak self] in - var movedNodes: [FileNodeRecord] = [] + var moved = CompletedTrashMoves() var actionError: Error? var wasCancelled = false for node in nodes { do { try Task.checkCancellation() - let verificationResult = try await moveToTrash(node) - if let identityError = Self.fileActionError(for: verificationResult, node: node) { + let outcome = try await moveToTrash(node) + if let identityError = Self.fileActionError(for: outcome.verification, node: node) { actionError = identityError break } - movedNodes.append(node) + moved.operations.append(node) + if let discardPileBatch, discardPileBatch.coveredMarks[node.id] != nil, let destination = outcome.destination { + let covered = await Task.detached(priority: .userInitiated) { + discardPileBatch.verifyCoveredMarks(for: node, at: destination) + }.value + moved.coveredMarks.append(contentsOf: covered) + } } catch is CancellationError { wasCancelled = true break @@ -515,7 +579,7 @@ final class TrashFlowController { } self?.confirmedTrashMoveTasks.removeValue(forKey: requestID) - onFinish(nodes, movedNodes, actionError, wasCancelled) + onFinish(nodes, moved, actionError, wasCancelled) } beginMove() } diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index 6a5e4be..cfdbe00 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -102,9 +102,9 @@ struct AppModelDependencyTests { } model.trashFlow.startConfirmedMove([file], moveToTrash: { _ in await withCheckedContinuation { continuation = $0 } - return .matches + return TrashMoveOutcome(.matches) }, beginMove: {}, onFinish: { _, moved, error, wasCancelled in - #expect(moved == [file]) + #expect(moved.operations == [file]) #expect(error == nil) #expect(!wasCancelled) didFinish = true @@ -163,7 +163,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { node in recorder.movedToTrashURLs.append(node.url) - return .matches + return TrashMoveOutcome(.matches) } actions.open = { recorder.openedURLs.append($0) } actions.reveal = { recorder.revealedURLs.append($0) } @@ -335,7 +335,7 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.moveToTrash = { _ in trashCalls += 1 - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) defer { model.cleanup() } @@ -1141,7 +1141,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } actions.defaultTargets = { recorder.defaultTargetsCallCount += 1 @@ -1169,7 +1169,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { node in await probe.move(node.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let file = installSelection(on: model) @@ -1266,7 +1266,7 @@ struct AppModelDependencyTests { actions.moveToTrash = { node in movedIDs.append(node.id) await probe.move(node.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let first = makeTestFileNode(id: "/selection/first.bin", name: "first.bin", size: 40) @@ -1301,7 +1301,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { node in await probe.move(node.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -1343,7 +1343,7 @@ struct AppModelDependencyTests { await probe.move(node.url) try Task.checkCancellation() } - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -1397,7 +1397,7 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeDependencies( @@ -1426,7 +1426,7 @@ struct AppModelDependencyTests { @Test(arguments: [ ( TrashIdentityVerificationResult.mismatch, - "The item at /selection/file.txt changed since this scan. Rescan before moving it to Trash." + "The item at /selection/file.txt changed since this scan. Rescan and select it again." ), ( .missingScannedIdentity, @@ -1443,7 +1443,7 @@ struct AppModelDependencyTests { fileIdentity: result == .missingScannedIdentity ? nil : FileIdentity(device: 1, inode: 2) ) var actions = AppSystemActions.inert - actions.moveToTrash = { _ in result } + actions.moveToTrash = { _ in TrashMoveOutcome(result) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) installSelection(on: model, file: file) @@ -1475,9 +1475,9 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.moveToTrash = { node in verifiedNodeIDs.append(node.id) - guard node.id != second.id else { return .mismatch } + guard node.id != second.id else { return TrashMoveOutcome(.mismatch) } recorder.movedToTrashURLs.append(node.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [first, second]) @@ -1497,7 +1497,7 @@ struct AppModelDependencyTests { #expect(recorder.movedToTrashURLs == [first.url]) #expect( model.lastErrorMessage - == "The item at \(second.url.path) changed since this scan. Rescan before moving it to Trash.") + == "The item at \(second.url.path) changed since this scan. Rescan and select it again.") try await waitUntil("partially successful trash batch reconciled", timeout: 2) { model.scanState.snapshot?.treeStore.node(id: first.id) == nil } @@ -1512,7 +1512,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let protectedRoot = makeTestDirectoryNode( @@ -1556,14 +1556,15 @@ struct AppModelDependencyTests { #expect(model.pendingTrashSelection?.nodes.map(\.id) == [folder.id]) } - @Test - func testAddingResidentCloudFileToDiscardPileRequiresConfirmation() { + @Test(arguments: [false, true]) + func testAddingResidentCloudFileToDiscardPileRequiresConfirmation(replaced: Bool) { var actions = AppSystemActions.inert actions.fileExists = { _ in true } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let cloudFile = makeTestFileNode( id: "/Users/alex/Library/CloudStorage/Dropbox/file.bin", - name: "file.bin" + name: "file.bin", + fileIdentity: FileIdentity(device: 1, inode: 1) ) let root = makeTestDirectoryNode( id: "/Users/alex/Library/CloudStorage/Dropbox", @@ -1583,10 +1584,18 @@ struct AppModelDependencyTests { #expect(model.pendingCloudFileAction?.nodes.map(\.id) == [cloudFile.id]) #expect(model.pendingCloudFileAction?.cloudImpact == .storedInCloud) + if replaced { + let replacement = makeTestFileNode( + id: cloudFile.id, name: cloudFile.name, fileIdentity: FileIdentity(device: 1, inode: 2) + ) + let refreshedStore = FileTreeStore(root: root, childrenByID: [root.id: [replacement]]) + model.scanState.replaceCurrentSnapshot(makeTestSnapshot(root: root, store: refreshedStore)) + } model.confirmPendingCloudFileAction() - #expect(model.discardPile.nodeIDs == [cloudFile.id]) + #expect(model.discardPile.nodeIDs == (replaced ? [] : [cloudFile.id])) #expect(model.pendingCloudFileAction == nil) + #expect(model.lastErrorMessage == (replaced ? FileActionError.changedSinceScan(path: cloudFile.id).localizedDescription : nil)) } @Test @@ -1596,7 +1605,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let cloudFile = makeTestFileNode( @@ -1668,7 +1677,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let queued = makeTestFileNode(id: "/selection/queued.txt", name: "queued.txt", size: 40) @@ -1703,7 +1712,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let firstQueued = makeTestFileNode(id: "/selection/firstQueued.txt", name: "firstQueued.txt", size: 40) @@ -1743,7 +1752,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let queued = makeTestFileNode(id: "/selection/folder/queued.txt", name: "queued.txt", size: 40) @@ -1777,7 +1786,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let queued = makeTestFileNode(id: "/selection/folder/queued.txt", name: "queued.txt", size: 40) @@ -1812,7 +1821,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let queued = makeTestFileNode(id: "/selection/folder/queued.txt", name: "queued.txt", size: 40) @@ -1847,7 +1856,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let queued = makeTestFileNode(id: "/selection/queued.txt", name: "queued.txt", size: 40) @@ -1945,7 +1954,7 @@ struct AppModelDependencyTests { } actions.moveToTrash = { _ in Issue.record("Replaying marks must not move files") - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let file = installSelection(on: model) @@ -2012,7 +2021,7 @@ struct AppModelDependencyTests { func testConfirmedTrashInvalidatesHistoryEvenWhenDiscardPileIsEmpty() async throws { var actions = AppSystemActions.inert actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let file = installSelection(on: model) model.scanState.selectedTarget = model.scanState.snapshot?.target @@ -2164,6 +2173,29 @@ struct AppModelDependencyTests { #expect(model.lastErrorMessage == "This item does not support that action.") } + @Test(arguments: [false, true]) + func testDiscardPileRejectsStaleChildBeforeReducingSelection(replaced: Bool) { + let model = AppModel(dependencies: makeDependencies()) + defer { model.cleanup() } + let stale = makeTestFileNode( + id: "/selection/folder/child.txt", name: "child.txt", fileIdentity: FileIdentity(device: 1, inode: 1) + ) + let replacement = makeTestFileNode( + id: stale.id, name: stale.name, fileIdentity: FileIdentity(device: 1, inode: 2) + ) + let children = replaced ? [replacement] : [] + let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: children) + let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder]) + let store = FileTreeStore(root: root, childrenByID: [root.id: [folder], folder.id: children]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(root: root, store: store)) + + let didAdd = model.addNodesToDiscardPile([folder, stale]) + + #expect(!didAdd) + #expect(model.discardPile.isEmpty) + #expect(model.lastErrorMessage == FileActionError.changedSinceScan(path: stale.id).localizedDescription) + } + @Test func testDiscardPileRejectsUnsupportedNode() { var actions = AppSystemActions.inert @@ -2403,6 +2435,50 @@ struct AppModelDependencyTests { #expect(observedFocusID == root.id) } + @Test + func testSnapshotReplacementPublishesChangedDiscardPileVisibility() throws { + let model = AppModel(dependencies: makeDependencies()) + defer { model.cleanup() } + model.showFreeSpaceInDiskMaps = true + let snapshotID = UUID() + let capacity = VolumeCapacitySnapshot(totalCapacity: 10_000, availableCapacity: 9_000) + let marked = makeTestFileNode( + id: "/volume/marked.txt", name: "marked.txt", + fileIdentity: FileIdentity(device: 1, inode: 1, birthTime: Date(timeIntervalSinceReferenceDate: 1)) + ) + let replacement = makeTestFileNode( + id: marked.id, name: marked.name, + fileIdentity: FileIdentity(device: 1, inode: 1, birthTime: Date(timeIntervalSinceReferenceDate: 2)) + ) + func snapshot(containing node: FileNodeRecord) -> ScanSnapshot { + let root = makeTestDirectoryNode(id: "/volume", name: "Volume", children: [node]) + return ScanSnapshot( + id: snapshotID, + target: ScanTarget(url: root.url, kind: .volume), + treeStore: FileTreeStore(root: root, childrenByID: [root.id: [node]]), + startedAt: Date(), + finishedAt: Date(), + scanWarnings: [], + isComplete: true, + volumeCapacity: capacity + ) + } + model.scanState.restoreCompletedSnapshot(snapshot(containing: marked)) + try #require(model.addNodesToDiscardPile([marked])) + try #require(model.workspaceHiddenNodeIDs.contains(marked.id)) + + var didPublish = false + let cancellable = model.objectWillChange.sink { didPublish = true } + defer { cancellable.cancel() } + model.scanState.replaceCurrentSnapshot(snapshot(containing: replacement)) + + #expect(didPublish) + #expect(!model.workspaceHiddenNodeIDs.contains(marked.id)) + + #expect(model.addNodesToDiscardPile([replacement])) + #expect(model.workspaceHiddenNodeIDs.contains(replacement.id)) + } + @Test func testDeferredDiscardPileAddMovesFocusedSelectionToVisibleAncestor() async throws { var actions = AppSystemActions.inert @@ -2463,7 +2539,7 @@ struct AppModelDependencyTests { actions.fileExists = { _ in true } actions.moveToTrash = { recorder.movedToTrashURLs.append($0.url) - return .matches + return TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) let child = makeTestFileNode(id: "/selection/folder/child.txt", name: "child.txt") @@ -2511,7 +2587,7 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.moveToTrash = { node in attempted.append(node) - return node == failed ? .mismatch : .matches + return TrashMoveOutcome(node == failed ? .mismatch : .matches) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -2539,6 +2615,55 @@ struct AppModelDependencyTests { #expect(model.usageStats.bytesMovedToTrash == first.allocatedSize) } + @Test(arguments: [false, true]) + func testDiscardPileKeepsUnconfirmedCoveredMarks(hasDestination: Bool) async throws { + let fixture = try makeTemporaryDirectory() + let manager = FileManager.default + defer { try? manager.removeItem(at: fixture) } + let parentURL = fixture.appending(path: "parent", directoryHint: .isDirectory) + let destination = fixture.appending(path: "trash-parent", directoryHint: .isDirectory) + let rescuedURL = fixture.appending(path: "rescued.txt") + try manager.createDirectory(at: parentURL, withIntermediateDirectories: false) + let children = try ["leaves.txt", "stays.txt"].map { name in + let url = parentURL.appending(path: name) + try Data([1]).write(to: url) + return makeTestFileNode(id: url.path, name: name, fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: url)) + } + let parent = makeTestDirectoryNode( + id: parentURL.path, name: parentURL.lastPathComponent, children: children, + fileIdentity: try ScanMetadataLoader().fileSystemIdentity(at: parentURL) + ) + var actions = AppSystemActions.inert + actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } + actions.moveToTrash = { node in + let verification = try SystemIntegration.moveToTrash( + node, identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { try manager.moveItem(at: $0, to: destination) } + ) + return TrashMoveOutcome(verification, destination: hasDestination ? destination : nil) + } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + defer { model.cleanup() } + + let childStore = FileTreeStore(root: parent, childrenByID: [parent.id: children]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(root: parent, store: childStore)) + try #require(model.addNodesToDiscardPile(children)) + let root = makeTestDirectoryNode(id: fixture.path, name: fixture.lastPathComponent, children: [parent]) + let parentStore = FileTreeStore(root: root, childrenByID: [root.id: [parent]]) + model.scanState.restoreCompletedSnapshot(makeTestSnapshot(root: root, store: parentStore)) + try #require(model.addNodesToDiscardPile([parent])) + try #require(await model.requestMoveDiscardPileToTrash()) + try #require(model.pendingTrashSelection?.nodes == [parent]) + + try manager.moveItem(at: children[0].url, to: rescuedURL) + model.confirmMovePendingSelectionToTrash() + try await waitUntil("trash and scan reconciliation finish") { !model.trashFlow.isMovingFiles } + + #expect(manager.fileExists(atPath: rescuedURL.path)) + #expect(manager.fileExists(atPath: destination.appending(path: children[1].name).path)) + #expect(model.discardPile.nodes == (hasDestination ? [children[0]] : children)) + } + @Test(arguments: [false, true]) func testDiscardPileTrashRemovesAliasedRowAndKeepsHardLink(markAlias: Bool) async throws { let fixture = try makeTemporaryDirectory() @@ -2562,7 +2687,7 @@ struct AppModelDependencyTests { actions.moveToTrash = { node in attempted.append(node.url) try manager.moveItem(at: node.url, to: moved) - return .matches + return TrashMoveOutcome(.matches, destination: moved) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) defer { model.cleanup() } @@ -2613,10 +2738,11 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } actions.moveToTrash = { node in - try SystemIntegration.moveToTrash( + let verification = try SystemIntegration.moveToTrash( node, identityVerifier: SystemIntegration.verifyTrashIdentity, trashItem: { try manager.moveItem(at: $0, to: movedURL) } ) + return TrashMoveOutcome(verification, destination: movedURL) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) defer { model.cleanup() } @@ -2662,10 +2788,11 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } actions.moveToTrash = { node in - try SystemIntegration.moveToTrash( + let verification = try SystemIntegration.moveToTrash( node, identityVerifier: SystemIntegration.verifyTrashIdentity, trashItem: { try manager.moveItem(at: $0, to: moved) } ) + return TrashMoveOutcome(verification, destination: moved) } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) defer { model.cleanup() } diff --git a/RadixCoreTests/ScanCoordinatorTests.swift b/RadixCoreTests/ScanCoordinatorTests.swift index 261560d..48721de 100644 --- a/RadixCoreTests/ScanCoordinatorTests.swift +++ b/RadixCoreTests/ScanCoordinatorTests.swift @@ -1815,7 +1815,7 @@ struct ScanCoordinatorTests { actions.verifyTrashIdentity = { SystemIntegration.verifyTrashIdentity($0) } actions.moveToTrash = { node in let result = try await move.perform(node) - if result == .matches, recreated { + if result.verification == .matches, recreated { try FileManager.default.createDirectory(at: child.url, withIntermediateDirectories: true) } return result @@ -1859,7 +1859,7 @@ struct ScanCoordinatorTests { let removed = try #require(initial.treeStore.node(id: parent.id + "/notes.txt")) var actions = makeCoordinatorSidebarActions(targets: [parent, child]) actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel(dependencies: makeCoordinatorAppDependencies(scanService: service, systemActions: actions)) defer { model.cleanup() } @@ -2633,7 +2633,7 @@ struct ScanCoordinatorTests { let service = ControlledScanService() var actions = AppSystemActions.inert actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeCoordinatorAppDependencies( scanService: service, @@ -2676,7 +2676,7 @@ struct ScanCoordinatorTests { let service = ControlledScanService() var actions = AppSystemActions.inert actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeCoordinatorAppDependencies( scanService: service, @@ -2714,7 +2714,7 @@ struct ScanCoordinatorTests { let service = ControlledScanService() var actions = AppSystemActions.inert actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeCoordinatorAppDependencies( scanService: service, @@ -2748,7 +2748,7 @@ struct ScanCoordinatorTests { let service = ControlledScanService() var actions = AppSystemActions.inert actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeCoordinatorAppDependencies( scanService: service, @@ -2783,7 +2783,7 @@ struct ScanCoordinatorTests { let secondTarget = makeCoordinatorTarget("/app/sidebar/stale-second") var actions = makeCoordinatorSidebarActions(targets: [firstTarget, secondTarget]) actions.fileExists = { _ in true } - actions.moveToTrash = { _ in .matches } + actions.moveToTrash = { _ in TrashMoveOutcome(.matches) } let model = AppModel( dependencies: makeCoordinatorAppDependencies( scanService: service, @@ -3575,7 +3575,7 @@ private final class PausedFixtureTrashMove { self.destination = destination } - func perform(_ node: FileNodeRecord) async throws -> TrashIdentityVerificationResult { + func perform(_ node: FileNodeRecord) async throws -> TrashMoveOutcome { let proceed = await withCheckedContinuation { continuation = $0 } guard proceed else { throw CancellationError() } @@ -3583,7 +3583,7 @@ private final class PausedFixtureTrashMove { if verification == .matches { try FileManager.default.moveItem(at: node.url, to: destination) } - return verification + return TrashMoveOutcome(verification, destination: destination) } func waitUntilStarted() async throws { diff --git a/RadixCoreTests/ScanEngineTests.swift b/RadixCoreTests/ScanEngineTests.swift index 656e37d..dd42814 100644 --- a/RadixCoreTests/ScanEngineTests.swift +++ b/RadixCoreTests/ScanEngineTests.swift @@ -1276,6 +1276,7 @@ struct ScanEngineTests { var returned = attribute_set_t() returned.commonattr = .max returned.fileattr = .max + returned.commonattr &= ~attrgroup_t(ATTR_CMN_CRTIME) #expect(BulkDirectoryEnumerator.hasRequiredMetadataAttributes(returned, objectType: VREG.rawValue)) diff --git a/RadixCoreTests/ScanMetadataLoaderTests.swift b/RadixCoreTests/ScanMetadataLoaderTests.swift index 0297db4..d686293 100644 --- a/RadixCoreTests/ScanMetadataLoaderTests.swift +++ b/RadixCoreTests/ScanMetadataLoaderTests.swift @@ -612,14 +612,14 @@ struct ScanMetadataLoaderTests { let data = [UInt64(42).littleEndian, UInt64(123).littleEndian].withUnsafeBytes { Data($0) } let resource = FileIdentity(resourceIdentifier: data) #expect( - native.preservingVolumeIdentity(from: resource).darwinIdentity + native.preservingResourceIdentity(from: resource, isDirectory: false).darwinIdentity == FileIdentity.DarwinIdentity(fileID: 42, volumeToken: 123)) - #expect(FileIdentity(device: 7, inode: 43).preservingVolumeIdentity(from: resource).darwinIdentity == nil) - #expect(native.preservingVolumeIdentity(from: nil).darwinIdentity == nil) + #expect(FileIdentity(device: 7, inode: 43).preservingResourceIdentity(from: resource, isDirectory: false).darwinIdentity == nil) + #expect(native.preservingResourceIdentity(from: nil, isDirectory: false).darwinIdentity == nil) for length in [0, 8, 15, 17, 32] { let unfamiliar = FileIdentity(resourceIdentifier: Data(repeating: 0, count: length)) #expect(unfamiliar.darwinIdentity == nil) - #expect(native.preservingVolumeIdentity(from: unfamiliar).darwinIdentity == nil) + #expect(native.preservingResourceIdentity(from: unfamiliar, isDirectory: false).darwinIdentity == nil) } } diff --git a/RadixCoreTests/ScanResultFingerprint.swift b/RadixCoreTests/ScanResultFingerprint.swift index c77ccec..752bd96 100644 --- a/RadixCoreTests/ScanResultFingerprint.swift +++ b/RadixCoreTests/ScanResultFingerprint.swift @@ -31,10 +31,10 @@ func scanResultFingerprint(_ store: FileTreeStore) -> String { if let fileIdentity = node.fileIdentity { fingerprint.append(true) switch fileIdentity { - case .resourceIdentifier(let data, _): + case .resourceIdentifier(let data, _, _): fingerprint.append(UInt64(0)) fingerprint.append(data) - case .fileSystem(let device, let inode, _): + case .fileSystem(let device, let inode, _, _): fingerprint.append(UInt64(1)) fingerprint.append(device) fingerprint.append(inode) diff --git a/RadixCoreTests/SidebarScanCacheControllerTests.swift b/RadixCoreTests/SidebarScanCacheControllerTests.swift index 63d9130..d429e62 100644 --- a/RadixCoreTests/SidebarScanCacheControllerTests.swift +++ b/RadixCoreTests/SidebarScanCacheControllerTests.swift @@ -869,6 +869,52 @@ struct SidebarScanCacheControllerTests { #expect(requiresScan) } + @Test(arguments: [1, 2, nil] as [UInt64?]) + func testSidebarReuseRejectsMovedRowsAcrossIdentityFormats(resourceInode: UInt64?) { + let controller = SidebarScanCacheController(maxTotalNodeCount: 100) + let options = ScanOptions() + let snapshotID = UUID() + let moved = makeTestFileNode( + id: "/cache/root/file.txt", name: "file.txt", + fileIdentity: FileIdentity(device: 1, inode: 1, volumeToken: 3) + ) + let resourceData = resourceInode.map { inode in + [inode.littleEndian, UInt64(3).littleEndian].withUnsafeBytes { Data($0) } + } ?? Data([1]) + let current = makeTestFileNode( + id: moved.id, name: moved.name, fileIdentity: FileIdentity(resourceIdentifier: resourceData) + ) + func snapshot(containing node: FileNodeRecord) -> ScanSnapshot { + let root = makeTestDirectoryNode(id: "/cache/root", name: "root", children: [node]) + return ScanSnapshot( + id: snapshotID, + target: ScanTarget(url: root.url), + treeStore: FileTreeStore(root: root, childrenByID: [root.id: [node]]), + startedAt: Date(), + finishedAt: Date(), + scanWarnings: [], + isComplete: true + ) + } + let original = snapshot(containing: moved) + controller.prepareForScanStart(target: original.target, options: options) + controller.handleCompletedScanSnapshot(original) + controller.clearCache() + controller.invalidateDisplayedNodes([moved], in: original) + + let requiresScan = controller.applyCachedOrContainedSidebarTarget( + original.target, + options: options, + currentSnapshot: snapshot(containing: current), + isTargetActive: { _ in true }, + cancelDeferredScanStart: {}, + restoreSnapshot: { _, _ in Issue.record("An exact current snapshot does not need restoration") }, + startScan: { _ in Issue.record("The caller starts the fresh scan") } + ) + + #expect(requiresScan == (resourceInode != 2)) + } + @Test func testScopePendingWhenCacheIsInvalidatedStartsFreshScan() async throws { let transform = PausingCacheScopeTransform() diff --git a/RadixCoreTests/SystemIntegrationTests.swift b/RadixCoreTests/SystemIntegrationTests.swift index 7f01877..1c97370 100644 --- a/RadixCoreTests/SystemIntegrationTests.swift +++ b/RadixCoreTests/SystemIntegrationTests.swift @@ -404,6 +404,68 @@ struct SystemIntegrationTests { #expect(trashedURLs == (replaced ? [] : [scannedURL])) } + @Test(arguments: [false, true]) + func testIdentityBoundTrashMoveRejectsReusedIdentity(useNativeIdentity: Bool) throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let itemURL = fixture.appending(path: "marked.txt") + try Data([1]).write(to: itemURL) + let loader = ScanMetadataLoader() + func currentIdentity() throws -> FileIdentity { + if useNativeIdentity { + return try loader.fileSystemIdentity(at: itemURL) + } + let freshURL = URL(filePath: itemURL.path) + return try #require(loader.metadata(for: freshURL).fileIdentity) + } + let scannedIdentity = try currentIdentity() + #expect(scannedIdentity.isFileSystemIdentity == useNativeIdentity) + let birthTime = try #require(scannedIdentity.birthTime) + let node = trashTestNode(url: itemURL, fileIdentity: scannedIdentity) + + // Reproduce a recycled identity without depending on allocator behavior. + var replacementURL = itemURL + var values = URLResourceValues() + values.creationDate = birthTime.addingTimeInterval(-60) + try replacementURL.setResourceValues(values) + #expect(try currentIdentity() == scannedIdentity) + + let result = try SystemIntegration.moveToTrash( + node, + identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { _ in Issue.record("Moved an item with a different birth time") } + ) + + #expect(result == .mismatch) + } + + @Test + func testIdentityBoundTrashMoveRejectsReusedNativeDirectoryIdentity() throws { + let fixture = try makeTemporaryDirectory() + defer { try? FileManager.default.removeItem(at: fixture) } + let directory = fixture.appending(path: "marked", directoryHint: .isDirectory) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: false) + let loader = ScanMetadataLoader() + let originalIdentity = try #require(loader.metadata(for: directory).fileIdentity) + let originalResourceIdentifier = try #require(originalIdentity.trashResourceIdentifier) + try FileManager.default.moveItem(at: directory, to: fixture.appending(path: "original")) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: false) + + // Keep the original opaque ID and simulate a recycled stat inode and birth time. + let replacementIdentity = try loader.fileSystemIdentity(at: directory) + let scannedIdentity = replacementIdentity.preservingResourceIdentity( + from: FileIdentity(resourceIdentifier: originalResourceIdentifier), isDirectory: true + ) + let node = makeTestDirectoryNode(id: directory.path, name: "marked", children: [], fileIdentity: scannedIdentity) + let result = try SystemIntegration.moveToTrash( + node, + identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { _ in Issue.record("Moved a directory with a different resource identifier") } + ) + + #expect(result == .mismatch) + } + @Test func testIdentityBoundTrashMoveVerifiesImmediatelyBeforeNativeMutation() throws { let node = trashTestNode() diff --git a/RadixCoreTests/TrashFlowControllerTests.swift b/RadixCoreTests/TrashFlowControllerTests.swift index 0d5c736..f02eee8 100644 --- a/RadixCoreTests/TrashFlowControllerTests.swift +++ b/RadixCoreTests/TrashFlowControllerTests.swift @@ -81,16 +81,13 @@ struct TrashFlowControllerTests { try #require(original.fileIdentity == alias.fileIdentity) try #require(original.fileIdentity == distinctEntries[0].fileIdentity) let marks = [original, alias] + distinctEntries - let controller = TrashFlowController(discardPile: DiscardPileState(marks: marks.map { .init(node: $0) })) - let preview = try TrashFlowController.prepareDiscardPileTrashRequest( nodes: marks, trashSafetyPolicy: .live(), verifyIdentity: SystemIntegration.verifyTrashIdentity ) #expect(preview.nodes == [original] + distinctEntries) let batch = try #require(preview.discardPileBatch) #expect(batch.unavailable.isEmpty) - controller.removeMovedDiscardPileNodes(batch.affectedNodes(for: [original]), from: nil, includeSnapshotDescendants: false) - #expect(controller.discardPile.nodes == distinctEntries) + #expect(batch.plannedNodes(for: [original]) == [original, alias]) } @Test @@ -135,20 +132,15 @@ struct TrashFlowControllerTests { let sibling = makeTestFileNode( id: "/scope/folder-other/file", name: "file", fileIdentity: FileIdentity(device: 1, inode: 4) ) - let controller = TrashFlowController( - discardPile: DiscardPileState(marks: [child, folder, unavailable, sibling].map { .init(node: $0) }) - ) - let preview = try TrashFlowController.prepareDiscardPileTrashRequest( - nodes: controller.discardPile.nodes, trashSafetyPolicy: .live() + nodes: [child, folder, unavailable, sibling], trashSafetyPolicy: .live() ) { $0 == unavailable ? .missingCurrentItem : .matches } #expect(preview.nodes == [folder, sibling]) let batch = try #require(preview.discardPileBatch) #expect(batch.unavailable.map(\.node) == [unavailable]) - controller.removeMovedDiscardPileNodes(batch.affectedNodes(for: [folder]), from: nil, includeSnapshotDescendants: false) - #expect(controller.discardPile.nodes == [unavailable, sibling]) + #expect(batch.plannedNodes(for: [folder]) == [folder, child]) } @Test @@ -251,11 +243,11 @@ struct TrashFlowControllerTests { var didFinish = false controller.startConfirmedMove([node], moveToTrash: { _ in Issue.record("Read-only mode must prevent filesystem moves") - return .matches + return TrashMoveOutcome(.matches) }, beginMove: { Issue.record("Rejected moves must not hide scan results") }, onFinish: { _, moved, error, wasCancelled in - #expect(moved.isEmpty) + #expect(moved.operations.isEmpty) #expect(!wasCancelled) if case .readOnlyMode? = error as? FileActionError {} else { Issue.record("Expected the read-only mode error") @@ -274,7 +266,7 @@ struct TrashFlowControllerTests { var didFinish = false controller.startConfirmedMove([node], moveToTrash: { _ in await withCheckedContinuation { continuation = $0 } - return .matches + return TrashMoveOutcome(.matches) }, beginMove: {}, onFinish: { _, _, _, _ in didFinish = true }) #expect(!controller.setReadOnlyMode(true)) try await waitUntil("move starts") { continuation != nil } @@ -301,12 +293,12 @@ struct TrashFlowControllerTests { moveToTrash: { node in attemptedIDs.append(node.id) await withCheckedContinuation { continuations.append($0) } - return .matches + return TrashMoveOutcome(.matches) }, beginMove: {}, onFinish: { requested, moved, error, wasCancelled in #expect(requested == [first, second]) - #expect(moved == [first]) + #expect(moved.operations == [first]) #expect(error == nil) #expect(wasCancelled) finishedCount += 1 @@ -330,11 +322,11 @@ struct TrashFlowControllerTests { [node], moveToTrash: { _ in Issue.record("A cancelled batch must not start a filesystem move") - return .matches + return TrashMoveOutcome(.matches) }, beginMove: { controller.cancelConfirmedTrashMoves() }, onFinish: { _, moved, error, wasCancelled in - #expect(moved.isEmpty) + #expect(moved.operations.isEmpty) #expect(error == nil) #expect(wasCancelled) didFinish = true From cdd9674acff21d72416c304988b61b3f8a05fdb2 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:50:41 -0800 Subject: [PATCH 08/12] test(trash): reproduce stale prefetched identity --- RadixCoreTests/ScanMetadataLoaderTests.swift | 54 +++++++++++++++++--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/RadixCoreTests/ScanMetadataLoaderTests.swift b/RadixCoreTests/ScanMetadataLoaderTests.swift index d686293..52e52c5 100644 --- a/RadixCoreTests/ScanMetadataLoaderTests.swift +++ b/RadixCoreTests/ScanMetadataLoaderTests.swift @@ -111,24 +111,62 @@ struct ScanMetadataLoaderTests { #expect(try loader.metadata(for: directory).fileIdentity != original) } - @Test - func testPrefetchedFileMetadataDoesNotAdoptReplacementIdentity() throws { + enum PrefetchedEntryKind: CaseIterable { + case file, hardLink, symbolicLink + } + + @Test(arguments: PrefetchedEntryKind.allCases) + func testPrefetchedFileMetadataDoesNotAdoptReplacementIdentity(kind: PrefetchedEntryKind) throws { let root = try makeTemporaryDirectory() let manager = FileManager.default defer { try? manager.removeItem(at: root) } let file = root.appending(path: "file.bin") - try Data([1]).write(to: file) + let originalFile = root.appending(path: "original.bin") + let target = root.appending(path: "target.bin") + let loader = ScanMetadataLoader() + func node(for metadata: NodeMetadata) -> FileNodeRecord { + makeTestFileNode( + id: file.path, name: file.lastPathComponent, fileIdentity: metadata.fileIdentity, + isSymbolicLink: metadata.isSymbolicLink + ) + } + if kind == .symbolicLink { + try manager.createSymbolicLink(at: file, withDestinationURL: target) + } else { + try Data([1]).write(to: file) + if kind == .hardLink { + try manager.linkItem(at: file, to: root.appending(path: "alias.bin")) + } + } let values = try file.resourceValues(forKeys: ScanMetadataLoader.scanResourceKeys) let original = FileIdentity(resourceIdentifier: try #require(values.fileResourceIdentifier as? Data)) - try manager.moveItem(at: file, to: root.appending(path: "original.bin")) - try Data([2]).write(to: file) + if kind == .hardLink { + try #require(values.linkCount == 2) + } + #expect(SystemIntegration.verifyTrashIdentity(node(for: loader.metadata(for: file, prefetchedResourceValues: values))) == .matches) + if kind != .file { + let keysWithoutBirthTime = ScanMetadataLoader.scanResourceKeys.subtracting([.creationDateKey]) + let valuesWithoutBirthTime = try URL(filePath: file.path).resourceValues(forKeys: keysWithoutBirthTime) + try #require(valuesWithoutBirthTime.creationDate == nil) + let metadataWithoutBirthTime = loader.metadata(for: file, prefetchedResourceValues: valuesWithoutBirthTime) + #expect(SystemIntegration.verifyTrashIdentity(node(for: metadataWithoutBirthTime)) == .missingScannedIdentity) + } + try manager.moveItem(at: file, to: originalFile) + if kind == .symbolicLink { + try manager.createSymbolicLink(at: file, withDestinationURL: target) + } else { + try Data([2, 3]).write(to: file) + } - let metadata = ScanMetadataLoader().metadata(for: file, prefetchedResourceValues: values) - let node = makeTestFileNode(id: file.path, name: file.lastPathComponent, fileIdentity: metadata.fileIdentity) + let metadata = loader.metadata(for: file, prefetchedResourceValues: values) #expect(metadata.fileIdentity == original) #expect(metadata.fileIdentity?.scanFileSystemIdentity == nil) - #expect(SystemIntegration.verifyTrashIdentity(node) == .mismatch) + #expect(try SystemIntegration.moveToTrash( + node(for: metadata), + identityVerifier: SystemIntegration.verifyTrashIdentity, + trashItem: { _ in Issue.record("Moved the replacement") } + ) == .mismatch) } @Test From 468aeeafb7198272a8cd1fd29e72cbbe35e67383 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:50:53 -0800 Subject: [PATCH 09/12] fix(trash): preserve prefetched file identity --- Radix/Services/ScanMetadataLoader.swift | 32 ++++++++++---------- RadixCoreTests/ScanMetadataLoaderTests.swift | 2 +- RadixCoreTests/SystemIntegrationTests.swift | 3 +- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/Radix/Services/ScanMetadataLoader.swift b/Radix/Services/ScanMetadataLoader.swift index 0233a5e..78623a8 100644 --- a/Radix/Services/ScanMetadataLoader.swift +++ b/Radix/Services/ScanMetadataLoader.swift @@ -774,7 +774,7 @@ nonisolated struct CloneIdentity: Hashable, Sendable { nonisolated enum FileIdentity: Hashable, Sendable { enum NativeResourceEvidence: Sendable { case volumeToken(UInt64) - case directoryIdentifier(Data) + case identifier(Data) } case resourceIdentifier(Data, scanDevice: UInt64? = nil, birthTime: Date? = nil) @@ -819,7 +819,7 @@ nonisolated enum FileIdentity: Hashable, Sendable { var trashResourceIdentifier: Data? { switch self { - case .resourceIdentifier(let data, _, _), .fileSystem(_, _, .directoryIdentifier(let data), _): + case .resourceIdentifier(let data, _, _), .fileSystem(_, _, .identifier(let data), _): data case .fileSystem: nil @@ -883,7 +883,7 @@ nonisolated enum FileIdentity: Hashable, Sendable { switch self { case .fileSystem(_, let inode, .volumeToken(let token), _): return DarwinIdentity(fileID: inode, volumeToken: token) - case .fileSystem(_, let inode, .directoryIdentifier(let data), _): + case .fileSystem(_, let inode, .identifier(let data), _): guard let identity = Self.decodeDarwinIdentity(data), identity.fileID == inode else { return nil } return identity case .fileSystem: @@ -929,18 +929,18 @@ nonisolated enum FileIdentity: Hashable, Sendable { } func preservingResourceIdentity(from resourceIdentity: FileIdentity?, isDirectory: Bool) -> Self { - guard case .fileSystem(let device, let inode, _, let birthTime) = self, - let resourceIdentity else { return self } - let evidence: NativeResourceEvidence - if isDirectory, let identifier = resourceIdentity.trashResourceIdentifier { - // Some filesystems recycle stat's inode before reusing the resource identifier. - evidence = .directoryIdentifier(identifier) - } else if let identity = resourceIdentity.darwinIdentity, identity.fileID == inode, - !hasConflictingTrashEvidence(with: resourceIdentity) { - evidence = .volumeToken(identity.volumeToken) - } else { - return self - } - return .fileSystem(device: device, inode: inode, resourceEvidence: evidence, birthTime: birthTime) + guard case .fileSystem(let device, let inode, let evidence, _) = self else { return self } + // A later stat can provide an accounting key, but Trash needs evidence from the prefetched entry. + guard let resourceIdentity, let identifier = resourceIdentity.trashResourceIdentifier else { + return .fileSystem(device: device, inode: inode, resourceEvidence: evidence) + } + // On some volumes a directory's resource ID and stat inode have separate namespaces. + if !isDirectory, resourceIdentity.darwinIdentity?.fileID != inode || + hasConflictingTrashEvidence(with: resourceIdentity) { + return resourceIdentity + } + return .fileSystem( + device: device, inode: inode, resourceEvidence: .identifier(identifier), birthTime: resourceIdentity.birthTime + ) } } diff --git a/RadixCoreTests/ScanMetadataLoaderTests.swift b/RadixCoreTests/ScanMetadataLoaderTests.swift index 52e52c5..aae641a 100644 --- a/RadixCoreTests/ScanMetadataLoaderTests.swift +++ b/RadixCoreTests/ScanMetadataLoaderTests.swift @@ -652,7 +652,7 @@ struct ScanMetadataLoaderTests { #expect( native.preservingResourceIdentity(from: resource, isDirectory: false).darwinIdentity == FileIdentity.DarwinIdentity(fileID: 42, volumeToken: 123)) - #expect(FileIdentity(device: 7, inode: 43).preservingResourceIdentity(from: resource, isDirectory: false).darwinIdentity == nil) + #expect(FileIdentity(device: 7, inode: 43).preservingResourceIdentity(from: resource, isDirectory: false) == resource) #expect(native.preservingResourceIdentity(from: nil, isDirectory: false).darwinIdentity == nil) for length in [0, 8, 15, 17, 32] { let unfamiliar = FileIdentity(resourceIdentifier: Data(repeating: 0, count: length)) diff --git a/RadixCoreTests/SystemIntegrationTests.swift b/RadixCoreTests/SystemIntegrationTests.swift index 1c97370..d3f16ce 100644 --- a/RadixCoreTests/SystemIntegrationTests.swift +++ b/RadixCoreTests/SystemIntegrationTests.swift @@ -454,7 +454,8 @@ struct SystemIntegrationTests { // Keep the original opaque ID and simulate a recycled stat inode and birth time. let replacementIdentity = try loader.fileSystemIdentity(at: directory) let scannedIdentity = replacementIdentity.preservingResourceIdentity( - from: FileIdentity(resourceIdentifier: originalResourceIdentifier), isDirectory: true + from: FileIdentity(resourceIdentifier: originalResourceIdentifier, birthTime: replacementIdentity.birthTime), + isDirectory: true ) let node = makeTestDirectoryNode(id: directory.path, name: "marked", children: [], fileIdentity: scannedIdentity) let result = try SystemIntegration.moveToTrash( From 4af63ca5d2ccb21b4d4ad4d7f0597348faca5a83 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:50:53 -0800 Subject: [PATCH 10/12] test(ci): remove scan scheduling assumptions --- RadixCoreTests/AppModelDependencyTests.swift | 24 +++--- RadixCoreTests/ScanCoordinatorTests.swift | 86 ++++++++++--------- .../SidebarScanCacheControllerTests.swift | 4 +- 3 files changed, 58 insertions(+), 56 deletions(-) diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index cfdbe00..22bbd40 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -2091,7 +2091,7 @@ struct AppModelDependencyTests { var actions = AppSystemActions.inert actions.verifyTrashIdentity = { _ in verifierStarted.withLock { $0 = true } - _ = verifierGate.wait(timeout: .now() + 5) + verifierGate.wait() return .matches } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -2102,19 +2102,19 @@ struct AppModelDependencyTests { try #require(model.addNodesToDiscardPile([file])) model.presentDiscardPileReview() let preparation = Task { await model.requestMoveDiscardPileToTrash() } - defer { verifierGate.signal() } - try await waitUntil("discard pile verifier starts") { verifierStarted.withLock { $0 } } - #expect(model.isPreparingDiscardPileTrash) - if dismissed { - model.dismissDiscardPileReview() - } else { - let original = try #require(model.scanState.snapshot) - let refreshed = makeTestSnapshot(root: original.root, store: original.treeStore) - try #require(original.id != refreshed.id) - model.scanState.restoreCompletedSnapshot(refreshed) + do { + defer { verifierGate.signal() } + try await waitUntil("discard pile verifier starts") { verifierStarted.withLock { $0 } } + try #require(model.isPreparingDiscardPileTrash) + if dismissed { + model.dismissDiscardPileReview() + } else { + let original = try #require(model.scanState.snapshot) + let refreshed = makeTestSnapshot(root: original.root, store: original.treeStore) + model.scanState.restoreCompletedSnapshot(refreshed) + } } - verifierGate.signal() #expect(await preparation.value == !dismissed) #expect(!model.isPreparingDiscardPileTrash) diff --git a/RadixCoreTests/ScanCoordinatorTests.swift b/RadixCoreTests/ScanCoordinatorTests.swift index 48721de..948ec76 100644 --- a/RadixCoreTests/ScanCoordinatorTests.swift +++ b/RadixCoreTests/ScanCoordinatorTests.swift @@ -13,17 +13,19 @@ struct ScanCoordinatorTests { defer { coordinator.stopScan() } let target = makeCoordinatorTarget("/scan/presentation") let folderTarget = makeCoordinatorTarget(target.id + "/Downloads") - coordinator.restoreCompletedSnapshot(makeCoordinatorHomeSnapshot( - target: target, downloadsTarget: folderTarget, rootName: "Home", scanOptions: ScanOptions() - )) - let replacement = makeCoordinatorSnapshot(target: folderTarget, scanOptions: ScanOptions()) - try #require(coordinator.rescanFolder(id: folderTarget.id)) - let manualTask = try #require(coordinator.scanTask) - service.yield(.finished(replacement), scanIndex: 0) + let options = ScanOptions() + let baseline = makeCoordinatorHomeSnapshot( + target: target, downloadsTarget: folderTarget, rootName: "Home", scanOptions: options + ) + coordinator.startScan(target, options: options, baseline: baseline, isRescan: true) + let initialTask = try #require(coordinator.scanTask) + let fallback = IncrementalRescanFallbackReason.changedScanOptions + service.yield(.executionMode(.fullFallback(fallback)), scanIndex: 0) + service.yield(.finished(baseline), scanIndex: 0) service.finish(scanIndex: 0) - await manualTask.value - let notice = try #require(coordinator.scanCompletionNotice) - #expect(notice == .folderUpdated(name: "Downloads")) + await initialTask.value + let notice = ScanCompletionNotice.fullFallback(fallback) + try #require(coordinator.scanCompletionNotice == notice) let metrics = makeCoordinatorMetrics(path: "/previous/progress", filesVisited: 7) coordinator.scanMetrics = metrics coordinator.progress.executionMode = .incremental @@ -37,7 +39,7 @@ struct ScanCoordinatorTests { #expect(coordinator.progress.executionMode == .incremental) let task = try #require(coordinator.scanTask) service.yield(.progress(makeCoordinatorMetrics(path: "/background/progress", filesVisited: 2)), scanIndex: 1) - service.yield(.finished(replacement), scanIndex: 1) + service.yield(.finished(makeCoordinatorSnapshot(target: folderTarget, scanOptions: options)), scanIndex: 1) service.finish(scanIndex: 1) await task.value #expect(coordinator.scanCompletionNotice == notice) @@ -1475,7 +1477,7 @@ struct ScanCoordinatorTests { service.yield(.finished(alternateScan), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("subtree rescan publishes stale alternate file", timeout: 5) { + try await waitUntil("subtree rescan publishes stale alternate file") { coordinator.snapshot?.treeStore.node(id: file.path)?.fileIdentity == resource } @@ -1826,22 +1828,22 @@ struct ScanCoordinatorTests { model.cleanup() } model.selectSidebarTarget(id: parent.id) - try await waitUntil("parent scan starts", timeout: 5) { service.requests.count == 1 } + try await waitUntil("parent scan starts") { service.requests.count == 1 } service.yield(.finished(parentSnapshot), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("parent scan publishes", timeout: 5) { model.scanState.snapshot?.id == parentSnapshot.id } + try await waitUntil("parent scan publishes") { model.scanState.snapshot?.id == parentSnapshot.id } try #require(model.requestMoveNodesToTrash([summarized])) model.confirmMovePendingSelectionToTrash() try await move.waitUntilStarted() model.selectSidebarTarget(id: child.id) - try await waitUntil("child scan starts", timeout: 5) { service.requests.count == 2 } + try await waitUntil("child scan starts") { service.requests.count == 2 } service.yield(.finished(childSnapshot), scanIndex: 1) service.finish(scanIndex: 1) - try await waitUntil("child scan publishes", timeout: 5) { model.scanState.snapshot?.id == childSnapshot.id } + try await waitUntil("child scan publishes") { model.scanState.snapshot?.id == childSnapshot.id } move.finish() - try await waitUntil("move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try await waitUntil("move finishes") { model.usageStats.bytesMovedToTrash > 0 } try #require(FileManager.default.fileExists(atPath: child.id) == recreated) if recreated { let replacementIdentity = try loader.fileSystemIdentity(at: child.url) @@ -1864,19 +1866,19 @@ struct ScanCoordinatorTests { defer { model.cleanup() } model.selectSidebarTarget(id: parent.id) - try await waitUntil("parent scan starts", timeout: 5) { service.requests.count == 1 } + try await waitUntil("parent scan starts") { service.requests.count == 1 } service.yield(.finished(initial), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("parent scan publishes", timeout: 5) { model.scanState.snapshot?.id == initial.id } + try await waitUntil("parent scan publishes") { model.scanState.snapshot?.id == initial.id } try #require(model.requestMoveNodesToTrash([removed])) model.confirmMovePendingSelectionToTrash() - try await waitUntil("tree no longer contains moved node", timeout: 5) { + try await waitUntil("tree no longer contains moved node") { model.scanState.snapshot?.id == initial.id && model.scanState.fileTreeStore?.node(id: removed.id) == nil } model.selectSidebarTarget(id: child.id) - try await waitUntil("child display or fresh scan", timeout: 5) { + try await waitUntil("child display or fresh scan") { model.scanState.snapshot?.target == child || service.requests.count > 1 } #expect(service.requests.count == 1) @@ -1912,25 +1914,25 @@ struct ScanCoordinatorTests { model.cleanup() } model.selectSidebarTarget(id: child.id) - try await waitUntil("child scan starts", timeout: 5) { service.requests.count == 1 } + try await waitUntil("child scan starts") { service.requests.count == 1 } service.yield(.finished(childSnapshot), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("child scan publishes", timeout: 5) { model.scanState.snapshot?.id == childSnapshot.id } + try await waitUntil("child scan publishes") { model.scanState.snapshot?.id == childSnapshot.id } try #require(model.addNodesToDiscardPile([marked])) try #require(await model.requestMoveDiscardPileToTrash()) model.confirmMovePendingSelectionToTrash() try await move.waitUntilStarted() model.selectSidebarTarget(id: parent.id) - try await waitUntil("parent scan reads file before move finishes", timeout: 5) { service.requests.count == 2 } + try await waitUntil("parent scan reads file before move finishes") { service.requests.count == 2 } try #require(model.scanState.snapshot == nil) move.finish() - try await waitUntil("move finishes before scan publishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try await waitUntil("move finishes before scan publishes") { model.usageStats.bytesMovedToTrash > 0 } try #require(!FileManager.default.fileExists(atPath: fileURL.path)) service.yield(.finished(parentSnapshot), scanIndex: 1) service.finish(scanIndex: 1) - try await waitUntil("parent scan publishes or restarts", timeout: 5) { + try await waitUntil("parent scan publishes or restarts") { model.scanState.snapshot?.target == parent || service.requests.count > 2 } if service.requests.count == 3 { @@ -1938,7 +1940,7 @@ struct ScanCoordinatorTests { service.yield(.finished(refreshed), scanIndex: 2) service.finish(scanIndex: 2) } - try await waitUntil("parent scan is displayed", timeout: 5) { + try await waitUntil("parent scan is displayed") { model.scanState.snapshot?.target == parent } #expect(model.scanState.snapshot?.treeStore.node(id: marked.id) == nil) @@ -1986,10 +1988,10 @@ struct ScanCoordinatorTests { model.cleanup() } model.selectSidebarTarget(id: target.id) - try await waitUntil("initial scan starts", timeout: 5) { service.requests.count == 1 } + try await waitUntil("initial scan starts") { service.requests.count == 1 } service.yield(.finished(baseline), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("initial scan publishes", timeout: 5) { model.scanState.snapshot?.id == baseline.id } + try await waitUntil("initial scan publishes") { model.scanState.snapshot?.id == baseline.id } try #require(model.addNodesToDiscardPile([kept])) try #require(model.requestMoveNodesToTrash([moved])) model.confirmMovePendingSelectionToTrash() @@ -2004,17 +2006,17 @@ struct ScanCoordinatorTests { ) service.yield(.finished(replacement), scanIndex: 1) service.finish(scanIndex: 1) - try await waitUntil("manual rescan falls back to a full scan", timeout: 5) { + try await waitUntil("manual rescan falls back to a full scan") { service.requests.count == 3 && model.scanState.progress.executionMode == .fullFallback(.sharedAllocationTopologyChanged) } try #require(model.scanState.snapshot?.id == baseline.id) move.finish() - try await waitUntil("move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try await waitUntil("move finishes") { model.usageStats.bytesMovedToTrash > 0 } try #require(!FileManager.default.fileExists(atPath: moved.id)) service.yield(.finished(staleFullScan), scanIndex: 2) service.finish(scanIndex: 2) - try await waitUntil("full scan finishes or restarts", timeout: 5) { + try await waitUntil("full scan finishes or restarts") { !model.scanState.isScanOperationInProgress || service.requests.count > 3 } if service.requests.count == 4 { @@ -2022,7 +2024,7 @@ struct ScanCoordinatorTests { service.yield(.finished(refreshed), scanIndex: 3) service.finish(scanIndex: 3) } - try await waitUntil("updated scan publishes", timeout: 5) { model.scanState.phase == .displaying } + try await waitUntil("updated scan publishes") { model.scanState.phase == .displaying } #expect(model.scanState.snapshot?.treeStore.node(id: moved.id) == nil) #expect(model.discardPileSnapshot.nodes.map(\.id) == [kept.id]) } @@ -2055,36 +2057,36 @@ struct ScanCoordinatorTests { } model.selectSidebarTarget(id: parent.id) - try await waitUntil("parent scan request", timeout: 5) { service.requests.count == 1 } + try await waitUntil("parent scan request") { service.requests.count == 1 } service.yield(.finished(initial), scanIndex: 0) service.finish(scanIndex: 0) - try await waitUntil("parent finishes", timeout: 5) { model.scanState.snapshot?.id == initial.id } + try await waitUntil("parent finishes") { model.scanState.snapshot?.id == initial.id } model.selectSidebarTarget(id: scope.id) - try await waitUntil("scope displays", timeout: 5) { model.scanState.snapshot?.target == scope } + try await waitUntil("scope displays") { model.scanState.snapshot?.target == scope } try #require(model.addNodesToDiscardPile([marked])) model.selectSidebarTarget(id: parent.id) - try await waitUntil("parent displays", timeout: 5) { model.scanState.snapshot?.id == initial.id } + try await waitUntil("parent displays") { model.scanState.snapshot?.id == initial.id } try #require(model.addNodesToDiscardPile([marked])) try #require(await model.requestMoveDiscardPileToTrash()) model.confirmMovePendingSelectionToTrash() try await move.waitUntilStarted() model.selectSidebarTarget(id: scope.id) - try await waitUntil("scope displays again", timeout: 5) { model.scanState.snapshot?.target == scope } + try await waitUntil("scope displays again") { model.scanState.snapshot?.target == scope } try #require(model.discardPileSnapshot.nodes.map(\.id) == [marked.id]) try #require(model.workspaceHiddenNodeIDs.contains(marked.id)) move.finish() - try await waitUntil("confirmed move finishes", timeout: 5) { model.usageStats.bytesMovedToTrash > 0 } + try await waitUntil("confirmed move finishes") { model.usageStats.bytesMovedToTrash > 0 } try #require(!FileManager.default.fileExists(atPath: fileURL.path)) try #require(model.discardPileSnapshot.nodes.isEmpty) if model.scanState.fileTreeStore?.node(id: marked.id) != nil { #expect(model.workspaceHiddenNodeIDs.contains(marked.id)) } - try await waitUntil("moved file is removed from the overlapping scan", timeout: 5) { + try await waitUntil("moved file is removed from the overlapping scan") { model.scanState.fileTreeStore?.node(id: marked.id) == nil } } @@ -2149,7 +2151,7 @@ struct ScanCoordinatorTests { ) service.yield(.finished(replacementScan), scanIndex: 2) service.finish(scanIndex: 2) - try await waitUntil("replacement folder refresh publishes", timeout: 5) { + try await waitUntil("replacement folder refresh publishes") { model.scanState.snapshot?.treeStore.node(id: marked.id)?.fileIdentity == newIdentity } try #require(model.scanState.snapshot?.id == initial.id) @@ -3587,7 +3589,7 @@ private final class PausedFixtureTrashMove { } func waitUntilStarted() async throws { - try await waitUntil("move is in flight", timeout: 5) { self.continuation != nil } + try await waitUntil("move is in flight") { self.continuation != nil } } func finish() { diff --git a/RadixCoreTests/SidebarScanCacheControllerTests.swift b/RadixCoreTests/SidebarScanCacheControllerTests.swift index d429e62..8f9318b 100644 --- a/RadixCoreTests/SidebarScanCacheControllerTests.swift +++ b/RadixCoreTests/SidebarScanCacheControllerTests.swift @@ -934,10 +934,10 @@ struct SidebarScanCacheControllerTests { startScan: { target in recorder.startedTargets.append(target) } ) - try await waitUntil("cache scope transform paused", timeout: 5) { await transform.isPaused } + try await waitUntil("cache scope transform paused") { await transform.isPaused } controller.clearCache() await transform.resume() - try await waitUntil("sidebar target resolves", timeout: 5) { + try await waitUntil("sidebar target resolves") { !recorder.restoredSnapshots.isEmpty || !recorder.startedTargets.isEmpty } #expect(recorder.restoredSnapshots.isEmpty) From 5314eb1a3037849e98c5bd3577134147c6bcafcf Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Mon, 9 Nov 2026 00:14:53 -0800 Subject: [PATCH 11/12] test(scan): reproduce discarded Trash evidence --- RadixCoreTests/ScanCoordinatorTests.swift | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/RadixCoreTests/ScanCoordinatorTests.swift b/RadixCoreTests/ScanCoordinatorTests.swift index 948ec76..1c804a7 100644 --- a/RadixCoreTests/ScanCoordinatorTests.swift +++ b/RadixCoreTests/ScanCoordinatorTests.swift @@ -960,6 +960,45 @@ struct ScanCoordinatorTests { #expect(coordinator.scanCompletionNotice == nil) } + @Test + func testBackgroundRefreshRevealsReplacementWithReusedFileIdentity() async throws { + let service = ControlledScanService() + let coordinator = ScanCoordinator(scanService: service) + let target = makeCoordinatorTarget("/scan/reused-identity") + let birthTime = Date(timeIntervalSinceReferenceDate: 100) + let fileID = target.id + "/marked.txt" + func snapshot(resourceIdentifier: Data) -> ScanSnapshot { + let file = makeTestFileNode( + id: fileID, name: "marked.txt", + fileIdentity: .fileSystem( + device: 1, inode: 2, resourceEvidence: .identifier(resourceIdentifier), birthTime: birthTime + ) + ) + let root = makeTestDirectoryNode(id: target.id, name: target.displayName, children: [file]) + return makeCoordinatorSnapshot( + target: target, root: root, + store: FileTreeStore(root: root, childrenByID: [root.id: [file]]), + scanOptions: ScanOptions() + ) + } + let baseline = snapshot(resourceIdentifier: Data([1])) + let marked = try #require(baseline.treeStore.node(id: fileID)) + let pile = DiscardPileState(marks: [.init(node: marked, treeID: baseline.treeStore.contentID)]) + coordinator.restoreCompletedSnapshot(baseline) + try #require(pile.matchingNodeIDs(in: coordinator.snapshot) == [fileID]) + + try #require(coordinator.refreshFolderInBackground(id: target.id)) + let refresh = try #require(coordinator.scanTask) + let replacement = snapshot(resourceIdentifier: Data([2])) + service.yield(.finished(replacement), scanIndex: 0) + service.finish(scanIndex: 0) + await refresh.value + + let displayedFile = coordinator.snapshot?.treeStore.node(id: fileID) + #expect(displayedFile?.fileIdentity?.trashResourceIdentifier == Data([2])) + #expect(pile.matchingNodeIDs(in: coordinator.snapshot).isEmpty) + } + @Test func testUnchangedAutomaticVolumeSubtreeDoesNotQueryCapacityOrRebuildTree() async throws { let service = ControlledScanService() From 609462d1b63d3c1ccb7d72013672c0c595a64ae7 Mon Sep 17 00:00:00 2001 From: Colin Kim <121141519+colinvkim@users.noreply.github.com> Date: Mon, 9 Nov 2026 00:14:53 -0800 Subject: [PATCH 12/12] fix(scan): publish refreshed Trash evidence --- Radix/Services/ScanCoordinator.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Radix/Services/ScanCoordinator.swift b/Radix/Services/ScanCoordinator.swift index 7807e3e..2e270fd 100644 --- a/Radix/Services/ScanCoordinator.swift +++ b/Radix/Services/ScanCoordinator.swift @@ -834,7 +834,12 @@ final class ScanCoordinator: ObservableObject { var visitedCount = 0 while let id = pending.popLast() { try Task.checkCancellation() - guard baseline.treeStore.node(id: id) == replacement.treeStore.node(id: id) else { + // File identity equality ignores evidence used to verify Trash actions. + guard let oldNode = baseline.treeStore.node(id: id), + let newNode = replacement.treeStore.node(id: id), + oldNode == newNode, + oldNode.fileIdentity?.birthTime == newNode.fileIdentity?.birthTime, + oldNode.fileIdentity?.trashResourceIdentifier == newNode.fileIdentity?.trashResourceIdentifier else { return false } let oldChildren = baseline.treeStore.childIDs(of: id)