diff --git a/BitMatch-iPad/Views/ModularContentView.swift b/BitMatch-iPad/Views/ModularContentView.swift index 7a566508..5b9eda2b 100644 --- a/BitMatch-iPad/Views/ModularContentView.swift +++ b/BitMatch-iPad/Views/ModularContentView.swift @@ -327,7 +327,7 @@ struct SettingsSheetView: View { } Section { - Text("Every destination is checked against your card before BitMatch calls it verified. This sets how thoroughly that check runs.") + Text("This controls how BitMatch checks a copy. Quick checks file sizes without verifying contents.") .font(.footnote) .foregroundColor(.secondary) } header: { diff --git a/BitMatch-iPadTests/QueueAccessoryPresentationTests.swift b/BitMatch-iPadTests/QueueAccessoryPresentationTests.swift index 62e09498..69d01714 100644 --- a/BitMatch-iPadTests/QueueAccessoryPresentationTests.swift +++ b/BitMatch-iPadTests/QueueAccessoryPresentationTests.swift @@ -96,12 +96,13 @@ struct QueueAccessoryPresentationTests { #expect(afterRun.tone == .warning) } - @Test func progressAboveOneIsClamped() { + @Test func runningProgressAboveOneCannotClaimACompleteQueue() { let result = QueueAccessoryPresentation.make(queue: queue([ row("A001", state: .copying(progress: 140), progress: 1.4) ])) - #expect(result.detail == "1 of 1 · 100%") - #expect(result.progressFraction == 1) + #expect(result.detail == "1 of 1 · 99%") + #expect(result.progressFraction == 0.99) + #expect(result.tone == .progress) } @Test func sameRecordRealarmsAfterLeavingAndReenteringAProblemState() { diff --git a/BitMatch/Core/Services/CameraCardDetectionService.swift b/BitMatch/Core/Services/CameraCardDetectionService.swift index b46936e3..bf192558 100644 --- a/BitMatch/Core/Services/CameraCardDetectionService.swift +++ b/BitMatch/Core/Services/CameraCardDetectionService.swift @@ -181,6 +181,10 @@ final class CameraCardDetectionService: ObservableObject { // detection started before this mount. unmountedPaths.remove(event.volume.path) volumeGenerations[event.volume.path, default: 0] &+= 1 + // A missed unmount event can leave the previous card at this + // path. Retire its row before the new detection; path-based + // deduplication must not keep the old camera label. + await removeDetectedCamera(at: event.volume) startDetection(for: event.volume) case .unmounted: diff --git a/BitMatch/Core/Services/MacCameraAutoSourceController.swift b/BitMatch/Core/Services/MacCameraAutoSourceController.swift index b182526f..c8e9fe57 100644 --- a/BitMatch/Core/Services/MacCameraAutoSourceController.swift +++ b/BitMatch/Core/Services/MacCameraAutoSourceController.swift @@ -45,6 +45,15 @@ final class MacCameraAutoSourceController: ObservableObject { guard let shared, shared.reportSettings.enableAutoCameraDetection, volumeAccess?.needsDriveAccess != true, !shared.isOperationInProgress, !shared.showsOutcomeSummary else { return } + // The executor can become terminal before its journal record settles. + // Finish is not safe to display yet, but a new card must not silently + // replace the source until the operator starts a new transfer. + if !shared.lastOperationWasCompare { + switch shared.operationState { + case .completed, .failed, .cancelled: return + default: break + } + } let shouldSelect = AutomaticSourceSelectionPolicy.shouldSelect( automaticSelectionEnabled: shared.reportSettings.autoPopulateSource, hasExistingSource: shared.sourceURL != nil, diff --git a/BitMatch/Core/Services/VolumeMonitorService.swift b/BitMatch/Core/Services/VolumeMonitorService.swift index 7d0e0827..8ba90cac 100644 --- a/BitMatch/Core/Services/VolumeMonitorService.swift +++ b/BitMatch/Core/Services/VolumeMonitorService.swift @@ -5,6 +5,7 @@ import BitMatchEngine #if os(macOS) import DiskArbitration +import Darwin #endif /// Finds camera cards and backup drives as they mount. Published state and @@ -30,6 +31,55 @@ final class VolumeMonitorService: ObservableObject { #if os(macOS) private var volumesDispatchSource: DispatchSourceFileSystemObject? #endif + /// A path may be reused by a different card when the unmount and mount + /// happen between directory notifications. Keep the mounted filesystem's + /// best available identity, rather than the path, beside each row. + private var publishedMountIdentities: [String: String] = [:] + private var pendingMountIdentities: [String: String] = [:] + private var pendingAnalysisRequests: [String: UInt64] = [:] + private var nextAnalysisRequest: UInt64 = 0 + + struct MountedVolume: Sendable { + let url: URL + let identity: String? + + var path: String { url.standardizedFileURL.resolvingSymlinksInPath().path } + } + + struct Reconciliation: Sendable { + let stalePaths: Set + let toAnalyze: [MountedVolume] + } + + /// Pure decision used by the live monitor and deterministic event-gap + /// tests. A pending scan does not make an old published identity current. + nonisolated static func reconciliation( + mounted: [MountedVolume], published: [String: String], + pending: [String: String], knownPaths: Set, forceAnalysis: Bool = false + ) -> Reconciliation { + let current = Dictionary(mounted.compactMap { volume in + volume.identity.map { (volume.path, $0) } + }, uniquingKeysWith: { _, newer in newer }) + let stale = Set(knownPaths.union(published.keys).filter { path in + guard let previous = published[path] else { return current[path] == nil } + return current[path] != previous + }) + let toAnalyze = mounted.filter { volume in + guard let identity = volume.identity else { return false } + return (forceAnalysis || !knownPaths.contains(volume.path) || stale.contains(volume.path)) + && (forceAnalysis || pending[volume.path] != identity) + } + return Reconciliation(stalePaths: stale, toAnalyze: toAnalyze) + } + + nonisolated static func acceptsAnalysis(_ analyzed: MountedVolume, mounted: [MountedVolume]) -> Bool { + guard let identity = analyzed.identity else { return false } + return mounted.contains { $0.path == analyzed.path && $0.identity == identity } + } + + nonisolated static func acceptsAnalysisRequest(_ request: UInt64, current: UInt64?) -> Bool { + current == request + } struct DetectedVolume: Identifiable, Equatable, Sendable { let id = UUID() @@ -228,26 +278,40 @@ final class VolumeMonitorService: ObservableObject { vlog("⏭️ Skipping system volume: \(volumePath)") return } - analyzeInBackground([volumePath], facts: facts) + analyzeInBackground( + [MountedVolume(url: volumePath, identity: Self.mountIdentity(for: volumePath))], + facts: facts, forceAnalysis: true + ) } #endif // MARK: - Volume Analysis /// Reads each volume off the main actor and adds what it finds. - private func analyzeInBackground(_ volumes: [URL], facts: DiskFacts? = nil) { + private func analyzeInBackground( + _ volumes: [MountedVolume], facts: DiskFacts? = nil, forceAnalysis: Bool = false + ) { #if os(macOS) + let fresh = volumes.compactMap { volume -> (MountedVolume, UInt64)? in + guard let identity = volume.identity else { return nil } + guard forceAnalysis || pendingMountIdentities[volume.path] != identity else { return nil } + nextAnalysisRequest &+= 1 + pendingMountIdentities[volume.path] = identity + pendingAnalysisRequests[volume.path] = nextAnalysisRequest + return (volume, nextAnalysisRequest) + } Task.detached(priority: .utility) { [weak self] in - for volumeURL in volumes { + for (volume, request) in fresh { + let volumeURL = volume.url Self.vlog("🔬 Analyzing volume at: \(volumeURL.path)") let connected = VolumeAnalysis.connectedVolume(at: volumeURL, facts: facts) - if let connected { await self?.updateConnectedVolume(connected) } - if let detected = VolumeAnalysis.analyzeVolume(at: volumeURL, facts: facts, cameraInfo: connected?.cameraName) { + let detected = VolumeAnalysis.analyzeVolume(at: volumeURL, facts: facts, cameraInfo: connected?.cameraName) + if let detected { Self.vlog("✅ Volume detected as \(detected.type): \(detected.displayName)") - await self?.addDetectedVolume(detected) } else { Self.vlog("❌ Volume not recognized as camera card or backup drive") } + await self?.publishAnalysis(connected: connected, detected: detected, for: volume, request: request) } } #else @@ -255,41 +319,72 @@ final class VolumeMonitorService: ObservableObject { #endif } - private func checkForVolumeChanges() { + private func checkForVolumeChanges(forceAnalysis: Bool = false) { vlog("🔍 Checking for volume changes...") guard let currentVolumes = Self.mountedVolumes() else { return } - let currentVolumePaths = Set(currentVolumes.map { $0.standardizedFileURL.resolvingSymlinksInPath().path }) - - connectedVolumes.removeAll { !currentVolumePaths.contains($0.url.path) } - - // Remove cards and drives that are gone. - for card in availableCameraCards where !currentVolumePaths.contains(card.url.path) { - vlog("📤 Camera card removed: \(card.displayName)") - availableCameraCards.removeAll { $0.id == card.id } - } - for drive in availableBackupDrives where !currentVolumePaths.contains(drive.url.path) { - vlog("📤 Backup drive removed: \(drive.displayName)") - availableBackupDrives.removeAll { $0.id == drive.id } - } - - // Analyze only volumes not already known (read here, on the main - // actor, where the lists change). - // A volume counts as known once either list has it, so a card the - // drive list hides is not analyzed again on every change. + let mounted = currentVolumes.map { MountedVolume(url: $0, identity: Self.mountIdentity(for: $0)) } + let currentIdentities = Dictionary(mounted.compactMap { volume in + volume.identity.map { (volume.path, $0) } + }, uniquingKeysWith: { _, newer in newer }) + pendingMountIdentities = pendingMountIdentities.filter { currentIdentities[$0.key] == $0.value } + pendingAnalysisRequests = pendingAnalysisRequests.filter { pendingMountIdentities[$0.key] != nil } let knownPaths = Set(connectedVolumes.map { $0.url.path }) .union((availableCameraCards + availableBackupDrives).map { $0.url.path }) - let newVolumes = currentVolumes.filter { !knownPaths.contains($0.path) } - for volumeURL in newVolumes { - vlog("🔍 New volume detected: \(volumeURL.lastPathComponent)") + let plan = Self.reconciliation( + mounted: mounted, published: publishedMountIdentities, + pending: pendingMountIdentities, knownPaths: knownPaths, + forceAnalysis: forceAnalysis + ) + if !plan.stalePaths.isEmpty { + connectedVolumes.removeAll { plan.stalePaths.contains($0.url.path) } + availableCameraCards.removeAll { plan.stalePaths.contains($0.url.path) } + availableBackupDrives.removeAll { plan.stalePaths.contains($0.url.path) } + for path in plan.stalePaths { publishedMountIdentities.removeValue(forKey: path) } } - analyzeInBackground(newVolumes) + for volume in plan.toAnalyze { + vlog("🔍 New or replaced volume detected: \(volume.url.lastPathComponent)") + } + analyzeInBackground(plan.toAnalyze, forceAnalysis: forceAnalysis) } private func scanExistingVolumes() { vlog("🔍 Scanning existing volumes...") - guard let volumes = Self.mountedVolumes() else { return } - vlog("📂 Found \(volumes.count) volumes: \(volumes.map { $0.lastPathComponent })") - analyzeInBackground(volumes) + checkForVolumeChanges() + } + + /// UUID plus the mounted filesystem's device number catches ordinary + /// replacements at the same path. The device number is only a best-effort + /// discovery fallback when macOS provides no UUID; it can be reused after + /// a fast remount and never authorizes a transfer by itself. + nonisolated static func mountIdentity(for url: URL) -> String? { + #if os(macOS) + var current = url + current.removeAllCachedResourceValues() + guard let mountRoot = (try? current.resourceValues(forKeys: [.volumeURLKey]))?.allValues[.volumeURLKey] as? URL, + mountRoot.standardizedFileURL.resolvingSymlinksInPath().path + == current.standardizedFileURL.resolvingSymlinksInPath().path else { return nil } + return filesystemIdentity(for: current) + #else + return nil + #endif + } + + /// A selected source may be a folder inside a card, not the mount root. + nonisolated static func selectionIdentity(for url: URL) -> String? { + filesystemIdentity(for: url) + } + + private nonisolated static func filesystemIdentity(for url: URL) -> String? { + #if os(macOS) + var current = url + current.removeAllCachedResourceValues() + var info = stat() + guard stat(current.path, &info) == 0 else { return nil } + let uuid = (try? current.resourceValues(forKeys: [.volumeUUIDStringKey]))?.volumeUUIDString ?? "" + return "\(UInt64(info.st_dev)):\(uuid)" + #else + return nil + #endif } private nonisolated static func mountedVolumes() -> [URL]? { @@ -306,8 +401,32 @@ final class VolumeMonitorService: ObservableObject { // MARK: - Volume Management + private func publishAnalysis( + connected: ConnectedDrivesPresentation.Volume?, detected: DetectedVolume?, + for mount: MountedVolume, request: UInt64 + ) { + defer { + if Self.acceptsAnalysisRequest(request, current: pendingAnalysisRequests[mount.path]) { + pendingMountIdentities.removeValue(forKey: mount.path) + pendingAnalysisRequests.removeValue(forKey: mount.path) + } + } + guard Self.acceptsAnalysisRequest(request, current: pendingAnalysisRequests[mount.path]), + let identity = mount.identity, + let live = Self.mountedVolumes()?.map({ MountedVolume(url: $0, identity: Self.mountIdentity(for: $0)) }), + Self.acceptsAnalysis(mount, mounted: live) else { return } + if publishedMountIdentities[mount.path] != nil, + publishedMountIdentities[mount.path] != identity { + connectedVolumes.removeAll { $0.url.path == mount.path } + availableCameraCards.removeAll { $0.url.path == mount.path } + availableBackupDrives.removeAll { $0.url.path == mount.path } + } + if let connected { updateConnectedVolume(connected) } + if let detected { addDetectedVolume(detected) } + if connected != nil || detected != nil { publishedMountIdentities[mount.path] = identity } + } + private func updateConnectedVolume(_ volume: ConnectedDrivesPresentation.Volume) { - guard Self.mountedVolumes()?.contains(where: { $0.standardizedFileURL.resolvingSymlinksInPath() == volume.url }) == true else { return } if let index = connectedVolumes.firstIndex(where: { $0.url == volume.url }) { connectedVolumes[index] = volume } else { @@ -318,12 +437,16 @@ final class VolumeMonitorService: ObservableObject { private func addDetectedVolume(_ volume: DetectedVolume) { switch volume.type { case .cameraCard: - if !availableCameraCards.contains(where: { $0.devicePath == volume.devicePath }) { + if let index = availableCameraCards.firstIndex(where: { $0.url.path == volume.url.path }) { + availableCameraCards[index] = volume + } else if !availableCameraCards.contains(where: { $0.devicePath == volume.devicePath }) { availableCameraCards.append(volume) vlog("📷 Camera card detected: \(volume.displayName) (\(volume.capacityFormatted))") } case .backupDrive: - if !availableBackupDrives.contains(where: { $0.devicePath == volume.devicePath }) { + if let index = availableBackupDrives.firstIndex(where: { $0.url.path == volume.url.path }) { + availableBackupDrives[index] = volume + } else if !availableBackupDrives.contains(where: { $0.devicePath == volume.devicePath }) { availableBackupDrives.append(volume) vlog("💾 Backup drive detected: \(volume.displayName) (\(volume.capacityFormatted))") } @@ -339,12 +462,30 @@ final class VolumeMonitorService: ObservableObject { // MARK: - Public Methods func refreshVolumes() { - scanExistingVolumes() + // The user just granted/recovered access or asked to rescan. A known + // mount may have been classified only partially while access was + // missing, so re-analyze even when its identity did not change. + checkForVolumeChanges(forceAnalysis: true) } func forceAnalyzeVolume(at url: URL) { vlog("🔧 Force analyzing volume: \(url)") - analyzeInBackground([url]) + analyzeInBackground( + [MountedVolume(url: url, identity: Self.mountIdentity(for: url))], forceAnalysis: true + ) + } + + /// Auto-queue may only act on a row while its published mount is still + /// the mounted filesystem, even if a change notification is in flight. + func isCurrentDiscoveredVolume(_ row: ConnectedDrivesPresentation.Row) -> Bool { + var current = row.url + current.removeAllCachedResourceValues() + let path = current.standardizedFileURL.resolvingSymlinksInPath().path + guard let published = publishedMountIdentities[path], let rowID = row.volumeID, + let liveID = (try? current.resourceValues(forKeys: [.volumeUUIDStringKey]))?.volumeUUIDString else { + return false + } + return rowID == liveID && Self.mountIdentity(for: current) == published } } diff --git a/BitMatch/Core/ViewModels/MacVolumeAccessModel.swift b/BitMatch/Core/ViewModels/MacVolumeAccessModel.swift index 8eb44da7..457a4b08 100644 --- a/BitMatch/Core/ViewModels/MacVolumeAccessModel.swift +++ b/BitMatch/Core/ViewModels/MacVolumeAccessModel.swift @@ -63,6 +63,10 @@ final class MacVolumeAccessModel: ObservableObject { /// it, so rediscovery never undoes a deliberate removal. private var dismissedDestinationPaths = Set() private var autoQueuedVolumeIDs = Set() + private var selectedSourceMountIdentity: String? + /// Tests replace this with deterministic mount identities; the app reads + /// the mounted filesystem, not just the path under /Volumes. + var sourceMountIdentity: (URL) -> String? = VolumeMonitorService.selectionIdentity /// Volume facts for `BackupTargetPolicy`. Tests supply their own, since /// their drives are not mounted. var volumeFacts: (URL) -> BackupTargetPolicy.VolumeFacts? = BackupTargetPolicy.VolumeFacts.read @@ -107,6 +111,12 @@ final class MacVolumeAccessModel: ObservableObject { /// Last-used backups follow the shared selection. `$destinationURLs` /// publishes the new value before it is stored, so the sink uses it. private func observeSelection(of shared: SharedAppCoordinator) { + shared.$sourceURL + .sink { [weak self] url in + guard let self else { return } + self.selectedSourceMountIdentity = url.flatMap { self.sourceMountIdentity($0) } + } + .store(in: &cancellables) shared.$destinationURLs.dropFirst() .sink { [weak self] urls in guard self?.shared?.isReplayingQueuedTransfer != true else { return } @@ -167,11 +177,30 @@ final class MacVolumeAccessModel: ObservableObject { volumeMonitor.$connectedVolumes .receive(on: DispatchQueue.main) - .sink { [weak self] volumes in self?.autoQueueNewCards(from: volumes) } + .sink { [weak self] volumes in self?.handleConnectedVolumesUpdate(volumes) } .store(in: &cancellables) } + + /// A card at the same mount path is not necessarily the selected card. + /// Clear the selection if its mounted filesystem changed while discovery + /// was catching up; a transfer must never silently switch sources. + private func invalidateReplacedSource() { + guard let sourceURL, let selectedSourceMountIdentity else { return } + guard sourceMountIdentity(sourceURL) == selectedSourceMountIdentity else { + SharedLogger.warning("Selected source volume changed while mounted; clearing selection", category: .transfer) + shared?.sourceURL = nil + return + } + } + + /// Internal so a missed event gap can be driven without a real mount. + func handleConnectedVolumesUpdate(_ volumes: [ConnectedDrivesPresentation.Volume]) { + invalidateReplacedSource() + autoQueueNewCards(from: volumes) + } private func handleCameraCardsUpdate(_ cards: [VolumeMonitorService.DetectedVolume]) { + invalidateReplacedSource() // Check if current source is still available if let sourceURL = sourceURL { let sourceStillExists = cards.contains { $0.url.path == sourceURL.path } @@ -198,6 +227,7 @@ final class MacVolumeAccessModel: ObservableObject { ) for row in candidates { guard let volumeID = row.volumeID else { continue } + guard volumeMonitor.isCurrentDiscoveredVolume(row) else { continue } do { try shared.enqueueAutomaticallyDetectedCard(source: row.url) autoQueuedVolumeIDs.insert(volumeID) diff --git a/BitMatch/Views/PreferencesWindow.swift b/BitMatch/Views/PreferencesWindow.swift index 12bf0d09..67704a03 100644 --- a/BitMatch/Views/PreferencesWindow.swift +++ b/BitMatch/Views/PreferencesWindow.swift @@ -360,12 +360,7 @@ struct PreferencesWindow: View { } private var verificationExplanation: String { - switch coordinator.verificationMode { - case .standard: "Standard reads the card once and checks every drive." - case .quick: "Quick compares file sizes without checking file contents." - case .thorough: "Thorough also re-reads the card for verification." - case .paranoid: "Paranoid also compares every byte." - } + coordinator.verificationMode.description } } diff --git a/BitMatchTests/CameraDetectionRaceTests.swift b/BitMatchTests/CameraDetectionRaceTests.swift index a77c08c1..e2101b99 100644 --- a/BitMatchTests/CameraDetectionRaceTests.swift +++ b/BitMatchTests/CameraDetectionRaceTests.swift @@ -271,6 +271,39 @@ final class CameraDetectionRaceTests: XCTestCase { XCTAssertEqual(cards.first?.model, "M2") } + /// If the unmount notification is missed but a new mount arrives at the + /// same path, the published camera must be replaced, not path-deduped. + func testMountReplacementWithoutUnmountEventUpdatesCard() async throws { + let gate = Gate() + let vol = URL(fileURLWithPath: "/Volumes/RACE-MISSED-UNMOUNT") + let service = await makeService(gate: gate, volumes: []) + defer { + gate.resumeAll(returning: nil) + Task { @MainActor [service] in service.stopMonitoring() } + } + await MainActor.run { service.startMonitoring() } + + await mount(service, volume: vol) + let firstStarted = await waitUntil(timeout: .seconds(5)) { gate.callCount == 1 } + XCTAssertTrue(firstStarted) + gate.resumeNext(returning: Self.card(for: vol, model: "Old")) + let firstSettled = await waitUntil(timeout: .seconds(5)) { gate.settledCount == 1 } + XCTAssertTrue(firstSettled) + let first = await cardInfo(of: service) + XCTAssertEqual(first.first?.model, "Old") + + // There is no unmount event here: only a new mount at the same URL. + await mount(service, volume: vol) + let nextStarted = await waitUntil(timeout: .seconds(5)) { gate.callCount == 2 } + XCTAssertTrue(nextStarted) + gate.resumeNext(returning: Self.card(for: vol, model: "New")) + let nextSettled = await waitUntil(timeout: .seconds(5)) { gate.settledCount == 2 } + XCTAssertTrue(nextSettled) + let replacement = await cardInfo(of: service) + XCTAssertEqual(replacement.count, 1) + XCTAssertEqual(replacement.first?.model, "New") + } + /// A rescan while monitoring is stopped must not spawn a scan at all: /// no detection starts and no card publishes. func testRescanWhileStoppedDoesNothing() async throws { diff --git a/BitMatchTests/CancelledOutcomeTests.swift b/BitMatchTests/CancelledOutcomeTests.swift index 33160057..b739a211 100644 --- a/BitMatchTests/CancelledOutcomeTests.swift +++ b/BitMatchTests/CancelledOutcomeTests.swift @@ -10,7 +10,14 @@ import BitMatchEngine @MainActor struct CancelledOutcomeTests { private func makeCoordinator() -> SharedAppCoordinator { - SharedAppCoordinator(platformManager: MacOSPlatformManager.shared, defaults: .isolatedWorkflowDefaults()) + // These presentation tests cancel a coordinator without starting a transfer. + // Give each one private file operations so it cannot cancel another test's + // active run on MacOSPlatformManager.shared's process-wide pipeline. + let operations = RecordingFileOperations() + return SharedAppCoordinator( + platformManager: RecordingPlatformManager(fileOperations: operations), + defaults: .isolatedWorkflowDefaults() + ) } private func makeRow(path: String) -> ResultRow { @@ -70,9 +77,7 @@ struct CancelledOutcomeTests { @Test func masterReportIsNotCoveredByPreviousTransferOutcome() { #if os(macOS) let coordinator = makeCoordinator() - coordinator.operationState = .completed( - OperationCompletionInfo(success: false, message: "Not verified") - ) + coordinator.operationState = .failed #expect(coordinator.showsOutcomeSummary) #expect(coordinator.currentMode == .copyAndVerify) @@ -82,9 +87,7 @@ struct CancelledOutcomeTests { #expect(coordinator.showsOutcomeSummary) #expect(coordinator.currentMode == .masterReport) #expect(!(coordinator.currentMode == .copyAndVerify && coordinator.showsOutcomeSummary)) - #expect(coordinator.operationState == .completed( - OperationCompletionInfo(success: false, message: "Not verified") - )) + #expect(coordinator.operationState == .failed) coordinator.switchMode(to: .copyAndVerify) #expect(coordinator.currentMode == .copyAndVerify && coordinator.showsOutcomeSummary) @@ -93,4 +96,32 @@ struct CancelledOutcomeTests { #endif } + @Test func terminalScreenWaitsForDurableJournalSettlement() { + let completed = OperationState.completed(.init(success: true, message: "Verified")) + let shows = { (state: OperationState, recordState: LocalTransferState?, running: Bool) in + SharedAppCoordinator.shouldShowOutcomeSummary( + state: state, recordState: recordState, + isOperationInProgress: running, hasJournalPersistenceError: false, + lastOperationWasCompare: false + ) + } + + #expect(!shows(completed, .running, true)) + #expect(!shows(completed, nil, false)) + #expect(shows(completed, .completed, true)) + #expect(shows(completed, .issues, false)) + #expect(!shows(.failed, .running, true)) + #expect(!shows(.failed, .running, false)) + #expect(!shows(.cancelled, .running, false)) + #expect(SharedAppCoordinator.shouldShowOutcomeSummary( + state: .failed, recordState: .running, isOperationInProgress: false, + hasJournalPersistenceError: true, lastOperationWasCompare: false + )) + #expect(!SharedAppCoordinator.shouldShowOutcomeSummary( + state: completed, recordState: .completed, + isOperationInProgress: false, hasJournalPersistenceError: false, + lastOperationWasCompare: true + )) + } + } diff --git a/BitMatchTests/CardSafetyStateTests.swift b/BitMatchTests/CardSafetyStateTests.swift index 30dad05b..0fdc63c9 100644 --- a/BitMatchTests/CardSafetyStateTests.swift +++ b/BitMatchTests/CardSafetyStateTests.swift @@ -29,8 +29,8 @@ struct CardSafetyStateTests { } @Test func preparingAndActiveWorkStayBlue() { - let active: [CardSafetyState] = [.preparing, .copying(progress: nil), .verifying(progress: nil)] + let active: [CardSafetyState] = [.preparing, .copying(progress: nil), .verifying(progress: nil), .finishing(progress: nil)] #expect(active.allSatisfy { $0.tint == .blue }) - #expect(active.map(\.title) == ["Preparing", "Copying", "Verifying"]) + #expect(active.map(\.title) == ["Preparing", "Copying", "Verifying", "Finishing"]) } } diff --git a/BitMatchTests/LocalTransferJournalTests.swift b/BitMatchTests/LocalTransferJournalTests.swift index 1290ed7d..d3f6ea0f 100644 --- a/BitMatchTests/LocalTransferJournalTests.swift +++ b/BitMatchTests/LocalTransferJournalTests.swift @@ -67,6 +67,134 @@ struct LocalTransferJournalTests { #expect(journal.matchingVerifiedRecord(sourceFingerprint: "incomplete") == nil) } + @Test func fingerprintMatchRejectsIncompleteRecordedPlan() throws { + let f = try fixture() + defer { try? FileManager.default.removeItem(at: f.root) } + let journal = LocalTransferJournal(fileURL: f.journal) + let id = try journal.enqueue( + sourceURL: f.source, destinationURLs: [f.destination], verificationMode: .standard, + cameraSettings: CameraLabelSettings(), reportSettings: ReportPrefs() + ) + try journal.markRunning(id: id) + try journal.recordPlan(id: id, fileCounts: [2]) + try journal.finish( + id: id, + results: [ResultRow( + path: "clip.mov", status: ResultOutcome.verified.statusText, size: 1, + checksum: "abc", destination: "Backup", + destinationPath: f.destination.appendingPathComponent("clip.mov").path + )], + summary: "Done", hadIssues: false, sourceFingerprint: "short-plan" + ) + + #expect(journal.records.first?.state == .issues) + #expect(journal.records.first?.summary == "Transfer results are incomplete. Do not erase the card.") + #expect(journal.matchingVerifiedRecord(sourceFingerprint: "short-plan") == nil) + + let duplicateID = try journal.enqueue( + sourceURL: f.source, destinationURLs: [f.destination], verificationMode: .standard, + cameraSettings: CameraLabelSettings(), reportSettings: ReportPrefs() + ) + try journal.markRunning(id: duplicateID) + try journal.recordPlan(id: duplicateID, fileCounts: [2]) + let row = ResultRow( + path: "clip.mov", status: ResultOutcome.verified.statusText, size: 1, + checksum: "abc", destination: "Backup", + destinationPath: f.destination.appendingPathComponent("clip.mov").path + ) + try journal.finish( + id: duplicateID, results: [row, row], summary: "Done", + hadIssues: false, sourceFingerprint: "duplicate-plan" + ) + #expect(journal.records.first?.state == .issues) + #expect(journal.matchingVerifiedRecord(sourceFingerprint: "duplicate-plan") == nil) + } + + @Test func reviewedExclusionDoesNotBecomeAnIncompletePlanAlarm() throws { + let f = try fixture() + defer { try? FileManager.default.removeItem(at: f.root) } + let journal = LocalTransferJournal(fileURL: f.journal) + let id = try journal.enqueue( + sourceURL: f.source, destinationURLs: [f.destination], verificationMode: .standard, + cameraSettings: CameraLabelSettings(), reportSettings: ReportPrefs() + ) + try journal.markRunning(id: id) + try journal.recordPlan(id: id, fileCounts: [1]) + let selected = ResultRow( + path: "clip.mov", status: ResultOutcome.verified.statusText, size: 1, + checksum: "abc", destination: "Backup", + destinationPath: f.destination.appendingPathComponent("clip.mov").path + ) + let excluded = ResultRow( + path: "._clip.mov", status: ResultOutcome.excludedAppleDouble.statusText, size: 1, + checksum: nil, destination: "Backup" + ) + try journal.finish( + id: id, results: [selected, excluded], + summary: "Selected files verified; AppleDouble excluded.", hadIssues: true + ) + + #expect(journal.records.first?.state == .issues) + #expect(journal.records.first?.summary == "Selected files verified; AppleDouble excluded.") + } + + @Test func standardTransferCannotFinishWithCopiedButUnverifiedResults() throws { + let f = try fixture() + defer { try? FileManager.default.removeItem(at: f.root) } + let journal = LocalTransferJournal(fileURL: f.journal) + let id = try journal.enqueue( + sourceURL: f.source, destinationURLs: [f.destination], verificationMode: .standard, + cameraSettings: CameraLabelSettings(), reportSettings: ReportPrefs() + ) + try journal.markRunning(id: id) + try journal.recordPlan(id: id, fileCounts: [1]) + try journal.finish( + id: id, + results: [ResultRow( + path: "clip.mov", status: ResultOutcome.copiedUnverified.statusText, size: 1, + checksum: nil, destination: "Backup", + destinationPath: f.destination.appendingPathComponent("clip.mov").path + )], + summary: "All files verified.", hadIssues: false + ) + + #expect(journal.records.first?.state == .issues) + #expect(journal.records.first?.summary == "Some selected files were not verified. Do not erase the card.") + } + + @Test func quickReportFailureRemainsAnIssueAfterReload() throws { + let f = try fixture() + defer { try? FileManager.default.removeItem(at: f.root) } + do { + let journal = LocalTransferJournal(fileURL: f.journal) + let id = try journal.enqueue( + sourceURL: f.source, destinationURLs: [f.destination], verificationMode: .quick, + cameraSettings: CameraLabelSettings(), reportSettings: ReportPrefs() + ) + try journal.markRunning(id: id) + try journal.recordPlan(id: id, fileCounts: [1]) + try journal.finish( + id: id, + results: [ResultRow( + path: "clip.mov", status: ResultOutcome.copiedUnverified.statusText, size: 1, + checksum: nil, destination: "Backup", + destinationPath: f.destination.appendingPathComponent("clip.mov").path + )], + summary: "All selected files copied, but the report could not be saved.", + hadIssues: true, quickOnlyUnverified: false + ) + + let saved = try #require(journal.records.first) + #expect(saved.state == .issues) + #expect(saved.quickOnlyUnverified == false) + #expect(TransferLibraryPresentation.safetyState(for: saved) == .needsAttention) + } + + let restored = try #require(LocalTransferJournal(fileURL: f.journal).records.first) + #expect(restored.quickOnlyUnverified == false) + #expect(TransferLibraryPresentation.safetyState(for: restored) == .needsAttention) + } + @Test func oldHistoryJSONDecodesWithoutNewSourceAndDiskFields() throws { let f = try fixture() defer { try? FileManager.default.removeItem(at: f.root) } @@ -82,6 +210,7 @@ struct LocalTransferJournalTests { object["source"] = oldSource object.removeValue(forKey: "sourceFingerprint") object.removeValue(forKey: "independentDestinationCount") + object.removeValue(forKey: "quickOnlyUnverified") let decoded = try JSONDecoder().decode( LocalTransferRecord.self, @@ -92,6 +221,7 @@ struct LocalTransferJournalTests { #expect(decoded.plannedFileCounts == nil) #expect(decoded.sourceFingerprint == nil) #expect(decoded.independentDestinationCount == nil) + #expect(decoded.quickOnlyUnverified == nil) } @Test func recoveryExportsKeepLineageAndCountsWithoutAccessTokens() throws { diff --git a/BitMatchTests/LocalTransferQueueIntegrationTests.swift b/BitMatchTests/LocalTransferQueueIntegrationTests.swift index fddb29ae..7faba6c3 100644 --- a/BitMatchTests/LocalTransferQueueIntegrationTests.swift +++ b/BitMatchTests/LocalTransferQueueIntegrationTests.swift @@ -1532,6 +1532,7 @@ final class LocalTransferQueueIntegrationTests: XCTestCase { XCTAssertTrue(ended) XCTAssertNil(coordinator.queuePausedRecordID) XCTAssertEqual(coordinator.queuePresentation.tally.copiedNotVerified, 2) + XCTAssertTrue(journal.records.allSatisfy { $0.quickOnlyUnverified == true }) XCTAssertTrue(coordinator.standaloneAttentionRecordIDsSinceLaunch.isEmpty) } @@ -1562,11 +1563,18 @@ private final class QuickQueueRecordingOperations: FileOperationsService, @unche progressCallback: @escaping ProgressCallback, onFileResult: FileResultCallback? ) async throws -> FileOperation { let sourceFile = sourceURL.appendingPathComponent("clip.mov") - let rows = destinationURLs.map { destination in - FileOperationResult( + let sourceBytes = try Data(contentsOf: sourceFile) + let rows = try destinationURLs.map { destination in + let outputRoot = try SafetyValidator.resolvedDestinationRootChecked( + source: sourceURL, destination: destination, settings: settings + ) + try FileManager.default.createDirectory(at: outputRoot, withIntermediateDirectories: true) + let outputFile = outputRoot.appendingPathComponent("clip.mov") + try sourceBytes.write(to: outputFile) + return FileOperationResult( sourceURL: sourceFile, - destinationURL: destination.appendingPathComponent("clip.mov"), - success: true, error: nil, fileSize: 3, + destinationURL: outputFile, + success: true, error: nil, fileSize: Int64(sourceBytes.count), verificationResult: nil, processingTime: 0 ) } diff --git a/BitMatchTests/MacCameraAutoSourceTests.swift b/BitMatchTests/MacCameraAutoSourceTests.swift index a2c1b666..954d94ce 100644 --- a/BitMatchTests/MacCameraAutoSourceTests.swift +++ b/BitMatchTests/MacCameraAutoSourceTests.swift @@ -55,17 +55,22 @@ struct MacCameraAutoSourceTests { #expect(shared.sourceURL == nil) } - /// Plant: remove the `showsOutcomeSummary` guard in `cardDetected`; a - /// newly inserted card replaces the finished run while Finish is shown. - @Test func finishScreenLocksAutomaticSourceSelection() throws { + /// The executor can become terminal before the journal outcome is ready + /// for Finish. A newly inserted card must wait for New transfer. + @Test func terminalOperationLocksAutomaticSourceBeforeJournalSettles() throws { let (controller, shared, folders) = try makeController() defer { folders.cleanup() } shared.reportSettings.enableAutoCameraDetection = true shared.reportSettings.autoPopulateSource = true shared.operationState = .completed(.init(success: true, message: "Verified")) + #expect(!shared.showsOutcomeSummary) controller.cardDetected(at: folders.source) #expect(shared.sourceURL == nil) + + shared.resetForNewOperation() + controller.cardDetected(at: folders.source) + #expect(shared.sourceURL == folders.source) } } diff --git a/BitMatchTests/QueueSessionPresentationTests.swift b/BitMatchTests/QueueSessionPresentationTests.swift index f48cd950..bbab76d7 100644 --- a/BitMatchTests/QueueSessionPresentationTests.swift +++ b/BitMatchTests/QueueSessionPresentationTests.swift @@ -53,6 +53,28 @@ struct QueueSessionPresentationTests { #expect(verifying.rows.first?.oneLineStatus().contains("Verifying 40%") == true) } + @Test func fileWorkAtOneHundredPercentIsStillFinishingWhileRecordRuns() throws { + let fixture = try QueuePresentationFixture() + defer { fixture.cleanup() } + let running = try fixture.record(name: "A001", state: .running, at: 1) + let progress = OperationProgress( + overallProgress: 1, currentFile: nil, filesProcessed: 2, totalFiles: 2, + currentStage: .completed, speed: nil, elapsedTime: nil, averageSpeed: nil, + peakSpeed: nil, bytesProcessed: nil, totalBytes: nil, stageProgress: 1 + ) + + let queue = QueueSessionPresentation.make( + records: [running], sessionIDs: [running.id], progress: progress, + mountedSourceIDs: [] + ) + let row = try #require(queue.rows.first) + #expect(row.safetyState == .finishing(progress: 99)) + #expect(row.progressFraction == 0.99) + #expect(row.oneLineStatus().contains("Finishing 99%")) + #expect(queue.headerTitle == "Finishing A001") + #expect(!row.safetyState.canEject) + } + /// Plant: filter `projectID != nil` records out of the session. @Test func projectRecordUsesTheSameRunningAndFinishedSafetyRows() throws { let fixture = try QueuePresentationFixture() @@ -105,7 +127,10 @@ struct QueueSessionPresentationTests { let fixture = try QueuePresentationFixture() defer { fixture.cleanup() } let safe = try fixture.record(name: "A001", state: .completed, outcome: .verified, at: 1) - let quick = try fixture.record(name: "A002", state: .issues, mode: .quick, outcome: .copiedUnverified, at: 2) + let quick = try fixture.record( + name: "A002", state: .issues, mode: .quick, outcome: .copiedUnverified, + quickOnlyUnverified: true, at: 2 + ) let attention = try fixture.record(name: "A003", state: .issues, outcome: .failed, summary: "3 files failed on Shuttle B", at: 3) let failed = try fixture.record(name: "A004", state: .failed, summary: "A004 is not connected", at: 4) let interrupted = try fixture.record(name: "A005", state: .interrupted, summary: "Cancelled", at: 5) @@ -244,7 +269,8 @@ struct QueueSessionPresentationTests { defer { fixture.cleanup() } let safe = try fixture.record(name: "A001", state: .completed, outcome: .verified, at: 1) let quick = try fixture.record( - name: "A002", state: .issues, mode: .quick, outcome: .copiedUnverified, at: 2 + name: "A002", state: .issues, mode: .quick, outcome: .copiedUnverified, + quickOnlyUnverified: true, at: 2 ) let attention = try fixture.record(name: "A003", state: .issues, outcome: .failed, at: 3) let records = [safe, quick, attention] @@ -405,6 +431,7 @@ private final class QueuePresentationFixture { state: LocalTransferState, mode: VerificationMode = .standard, outcome: ResultOutcome? = nil, + quickOnlyUnverified: Bool? = nil, size: Int64 = 64, summary: String = "Ready", wasReused: Bool = false, @@ -422,6 +449,7 @@ private final class QueuePresentationFixture { ) record.state = state record.summary = summary + record.quickOnlyUnverified = quickOnlyUnverified if let outcome { record.results = [ResultRow( path: source.appendingPathComponent("clip.mov").path, diff --git a/BitMatchTests/ReportThumbnailTests.swift b/BitMatchTests/ReportThumbnailTests.swift index 8dbe49ce..56541b2a 100644 --- a/BitMatchTests/ReportThumbnailTests.swift +++ b/BitMatchTests/ReportThumbnailTests.swift @@ -206,6 +206,13 @@ struct ReportThumbnailTests { @Test func transferDetailsPreviewsAndSelectionStayConsistentAcrossModes() async throws { try await FileOperationsTestLock.shared.run { let fm = FileManager.default + // The matrix exercises the real copy engine, but owns its pipeline. + // Parallel cancellation tests must not cancel this run through the + // Mac platform manager's process-wide pipeline. + let platform = RecordingPlatformManager(fileOperations: TransferPipeline( + fileSystem: MacOSPlatformManager.shared.fileSystem, + checksum: MacOSPlatformManager.shared.checksum + )) for mode in VerificationMode.allCases { for exclude in [false, true] { let root = fm.temporaryDirectory.appendingPathComponent("bitmatch-combined-\(UUID())") @@ -228,7 +235,7 @@ struct ReportThumbnailTests { if exclude { settings.excludedAppleDoublePaths = try AppleDoubleSelection.review(source: source) } let prefs = ReportPrefs(verificationMode: mode, includeThumbnails: true, notes: "Card 01 · 日本語 & ", operatorName: "Alex & Sam") - let executor = CopyVerifyExecutor(platformManager: MacOSPlatformManager.shared, + let executor = CopyVerifyExecutor(platformManager: platform, timingService: OperationTimingService(), errorService: ErrorReportingService(), stateService: OperationStateService(), backgroundTaskService: IOSBackgroundTaskService.shared) var terminal: OperationCompletionInfo? diff --git a/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift b/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift index 47379508..69143291 100644 --- a/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift +++ b/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift @@ -1,3 +1,4 @@ +import CryptoKit import Foundation import Testing @testable import BitMatch @@ -167,6 +168,30 @@ struct SFTPRemoteBackupProviderContractTests { #expect(capabilities.missingRequirements(for: .sha256) == [.verificationEvidence]) } + @Test func sha256ModeDownloadsAndHashesRemoteBytesWithoutReadingTheCard() async throws { + #if os(macOS) + let bytes = Data("remote clip bytes".utf8) + let digest = SHA256.hash(data: bytes).map { String(format: "%02x", $0) }.joined() + let download = SFTPDownloadRecorder(bytes: bytes) + let provider = SFTPRemoteBackupProvider( + profile: try fixtureProfile(), credential: .sshAgent, + run: { command in try await download.run(command) } + ) + let remoteFile = try RemoteRelativePath(components: ["Jobs", "clip.mov"]) + + let evidence = try await provider.verificationEvidence(for: remoteFile, expectedSHA256: digest) + #expect(evidence == .readBackSHA256(digest)) + #expect(await download.commands().count == 1) + #expect(await download.downloadedPath().map { !FileManager.default.fileExists(atPath: $0) } == true) + + await #expect(throws: RemoteBackupError.verificationFailed) { + try await provider.verificationEvidence(for: remoteFile, expectedSHA256: String(repeating: "0", count: 64)) + } + #expect(await download.commands().count == 2) + #expect(await download.downloadedPath().map { !FileManager.default.fileExists(atPath: $0) } == true) + #endif + } + @Test func uploadOnlyStillRequiresNoReplacePromotion() async throws { let capabilities = RemoteProviderCapabilities( supportsNoReplacePromotion: false, @@ -291,4 +316,30 @@ private actor OpenSSHCommandRecorder { func commands() -> [OpenSSHCommand] { recorded } } + +private actor SFTPDownloadRecorder { + private let bytes: Data + private var recorded: [OpenSSHCommand] = [] + private var path: String? + + init(bytes: Data) { self.bytes = bytes } + + func run(_ command: OpenSSHCommand) throws -> OpenSSHCommandResult { + recorded.append(command) + guard command.executable == "/usr/bin/sftp", + let batch = command.standardInput.flatMap({ String(data: $0, encoding: .utf8) }) else { + throw RemoteBackupError.providerUnavailable + } + let fields = batch.components(separatedBy: "\"") + guard fields.count >= 4, fields[0] == "get ", fields[1] == "Backups/Jobs/clip.mov" else { + throw RemoteBackupError.providerUnavailable + } + path = fields[3] + try bytes.write(to: URL(fileURLWithPath: fields[3])) + return .init(status: 0, stdout: Data(), stderr: Data()) + } + + func commands() -> [OpenSSHCommand] { recorded } + func downloadedPath() -> String? { path } +} #endif diff --git a/BitMatchTests/TransferLibraryPresentationTests.swift b/BitMatchTests/TransferLibraryPresentationTests.swift index 01e8e921..e6947985 100644 --- a/BitMatchTests/TransferLibraryPresentationTests.swift +++ b/BitMatchTests/TransferLibraryPresentationTests.swift @@ -22,6 +22,7 @@ struct TransferLibraryPresentationTests { @Test func completedQuickTransferIsNotGreen() { var record = PresentationTestSupport.record(state: .completed, verificationMode: .quick) record.results = [PresentationTestSupport.row(.copiedUnverified, destination: record.destinations[0].url)] + record.quickOnlyUnverified = true let label = TransferLibraryPresentation.stateLabel(for: record) #expect(label.tint != .green) #expect(label.title == "Copied, not verified") @@ -30,6 +31,7 @@ struct TransferLibraryPresentationTests { @Test func journalQuickResultUsesCopiedNotVerifiedWords() { var record = PresentationTestSupport.record(state: .issues, verificationMode: .quick) record.results = [PresentationTestSupport.row(.copiedUnverified, destination: record.destinations[0].url)] + record.quickOnlyUnverified = true let label = TransferLibraryPresentation.stateLabel(for: record) #expect(label.title == "Copied, not verified") @@ -37,6 +39,21 @@ struct TransferLibraryPresentationTests { #expect(label.accessibilityLabel == "Copied, not verified: size check only") } + @Test func quickReportFailureDoesNotLookLikeAnOrdinarySizeOnlyCopy() { + var record = PresentationTestSupport.record(state: .issues, verificationMode: .quick) + record.results = [PresentationTestSupport.row(.copiedUnverified, destination: record.destinations[0].url)] + record.summary = "The report could not be saved." + record.quickOnlyUnverified = false + + #expect(TransferLibraryPresentation.safetyState(for: record) == .needsAttention) + #expect(TransferLibraryPresentation.stateLabel(for: record).title == "Needs attention") + + // A pre-upgrade Quick record has no reliable distinction between a + // clean size-only copy and another failed step, so remain conservative. + record.quickOnlyUnverified = nil + #expect(TransferLibraryPresentation.safetyState(for: record) == .needsAttention) + } + @Test func quickRecordCannotBeSafeEvenWhenRowsClaimVerified() { var record = PresentationTestSupport.record(state: .completed, verificationMode: .quick) record.results = [PresentationTestSupport.row(.verified, destination: record.destinations[0].url)] @@ -67,6 +84,61 @@ struct TransferLibraryPresentationTests { #expect(label.tint != .green) } + @Test func completedRecordWithMissingPlannedResultIsNotSafe() { + var record = PresentationTestSupport.record(state: .completed) + record.plannedFileCounts = [2] + record.results = [PresentationTestSupport.row(.verified, destination: record.destinations[0].url)] + + #expect(TransferLibraryPresentation.safetyState(for: record) == .needsAttention) + #expect(TransferLibraryPresentation.stateLabel(for: record).tint != .green) + + // Old journals have no recorded plan; preserve their existing rule. + record.plannedFileCounts = nil + #expect(TransferLibraryPresentation.safetyState(for: record) == .safeToErase) + } + + @Test func duplicateVerifiedPathCannotHideMissingPlannedFile() { + var record = PresentationTestSupport.record(state: .completed) + record.plannedFileCounts = [2] + let row = PresentationTestSupport.row(.verified, destination: record.destinations[0].url) + record.results = [row, row] + + #expect(TransferLibraryPresentation.safetyState(for: record) == .needsAttention) + } + + @Test @MainActor func savedInterruptionKeepsItsHistoryWordingOnFinish() throws { + var record = PresentationTestSupport.record(state: .interrupted) + record.summary = "Transfer interrupted unexpectedly; partial results retained." + let state = try #require(TransferOutcomePresentation.recordedState(for: record)) + let finish = TransferOutcomePresentation.make( + state: state, rows: [], destinations: record.destinations.map(\.url), + hasErrors: false, hasCriticalErrors: false, errorCount: 0, warningCount: 0, + duration: nil, verificationMode: record.verificationMode, + canRetry: true, canExport: true, sourceName: record.title, + completionReason: record.summary + ) + + #expect(finish.safetyState == TransferLibraryPresentation.safetyState(for: record)) + #expect(finish.finishTitle.contains("Transfer interrupted")) + #expect(finish.visibleVerdictText.contains("interrupted unexpectedly")) + #expect(finish.visibleVerdictText.contains("Do not erase")) + + record.state = .cancelled + record.summary = "Cancelled by user." + #expect(TransferOutcomePresentation.recordedState(for: record) == .cancelled) + let cancelled = TransferOutcomePresentation.make( + state: try #require(TransferOutcomePresentation.recordedState(for: record)), + rows: [], destinations: record.destinations.map(\.url), + hasErrors: false, hasCriticalErrors: false, errorCount: 0, warningCount: 0, + duration: nil, verificationMode: record.verificationMode, + canRetry: true, canExport: true, sourceName: record.title, + completionReason: record.summary + ) + #expect(cancelled.visibleVerdictText.contains("Cancelled by user")) + record.state = .failed + #expect(TransferOutcomePresentation.recordedState(for: record) == .failed) + } + @Test func completedHistoryRequiresEveryPlannedSelectedFileOncePerDestination() { var record = PresentationTestSupport.record(state: .completed) let destination = record.destinations[0].url diff --git a/BitMatchTests/TransferOptionsPresentationTests.swift b/BitMatchTests/TransferOptionsPresentationTests.swift index 44f6d889..c46a2718 100644 --- a/BitMatchTests/TransferOptionsPresentationTests.swift +++ b/BitMatchTests/TransferOptionsPresentationTests.swift @@ -70,13 +70,17 @@ struct TransferOptionsPresentationTests { @Test func compareSubsetNotesOnlyTheMode() { let standard = TransferOptionsPresentation.make( - verificationMode: .standard, generateASCMHL: nil, makeReport: nil, cameraLabel: nil + verificationMode: .standard, generateASCMHL: nil, makeReport: nil, cameraLabel: nil, + isCompare: true ) #expect(standard.advancedNote.isEmpty) + #expect(standard.verificationDetail == CompareCheckPlan.make(for: .standard).summary) let paranoid = TransferOptionsPresentation.make( - verificationMode: .paranoid, generateASCMHL: nil, makeReport: nil, cameraLabel: nil + verificationMode: .paranoid, generateASCMHL: nil, makeReport: nil, cameraLabel: nil, + isCompare: true ) #expect(paranoid.advancedNote == "Paranoid mode") + #expect(paranoid.verificationDetail == CompareCheckPlan.make(for: .paranoid).summary) } // Every platform now writes a PDF (`ReportPDFRenderer`), so the label names it everywhere. diff --git a/BitMatchTests/TransferProgressPresentationTests.swift b/BitMatchTests/TransferProgressPresentationTests.swift index bfe1bda9..29d3bde0 100644 --- a/BitMatchTests/TransferProgressPresentationTests.swift +++ b/BitMatchTests/TransferProgressPresentationTests.swift @@ -84,6 +84,17 @@ struct TransferProgressPresentationTests { #expect(p.countText == "3 of 8") } + @Test func finishedFileWorkDoesNotShowCompletedTransferBeforeJournalSettles() { + let p = make(state: .inProgress, progress: progress(stage: .completed, overall: 1)) + + #expect(p.phase == .finishing) + #expect(p.title == "Finishing") + #expect(p.fraction == 0.99) + #expect(p.percentText == "99%") + #expect(p.accessibilityValue.contains("Finishing, 99%")) + #expect(p.tone != .attention) + } + /// Plant: in `TransferProgressPresentation.make`, pass /// `canCancel: true` to `ProgressControls`. @Test func cancelUnavailableWhenIdle() { diff --git a/BitMatchTests/VolumeMonitorReconciliationTests.swift b/BitMatchTests/VolumeMonitorReconciliationTests.swift new file mode 100644 index 00000000..2b9ae361 --- /dev/null +++ b/BitMatchTests/VolumeMonitorReconciliationTests.swift @@ -0,0 +1,130 @@ +import Foundation +import Testing +@testable import BitMatch + +#if os(macOS) +@MainActor +struct VolumeMonitorReconciliationTests { + private let path = URL(fileURLWithPath: "/Volumes/REUSED-CARD") + + private func mount(_ identity: String?) -> VolumeMonitorService.MountedVolume { + .init(url: path, identity: identity) + } + + @Test func missedUnmountAndReplacementAtSamePathIsRediscovered() { + let old = mount("old-device:old-uuid") + let replacement = mount("new-device:new-uuid") + let plan = VolumeMonitorService.reconciliation( + mounted: [replacement], published: [old.path: old.identity!], + pending: [:], knownPaths: [old.path] + ) + + #expect(plan.stalePaths == [old.path]) + #expect(plan.toAnalyze.map(\.identity) == [replacement.identity]) + #expect(!VolumeMonitorService.acceptsAnalysis(old, mounted: [replacement])) + #expect(VolumeMonitorService.acceptsAnalysis(replacement, mounted: [replacement])) + } + + @Test func repeatedEventForSameMountDoesNotDuplicateAnalysis() { + let card = mount("same-device:same-uuid") + let published = VolumeMonitorService.reconciliation( + mounted: [card], published: [card.path: card.identity!], + pending: [:], knownPaths: [card.path] + ) + #expect(published.stalePaths.isEmpty) + #expect(published.toAnalyze.isEmpty) + + let pending = VolumeMonitorService.reconciliation( + mounted: [card], published: [:], + pending: [card.path: card.identity!], knownPaths: [] + ) + #expect(pending.toAnalyze.isEmpty) + } + + @Test func explicitRefreshRetriesKnownMountAfterAccessChanges() { + let card = mount("same-device:same-uuid") + let normal = VolumeMonitorService.reconciliation( + mounted: [card], published: [card.path: card.identity!], + pending: [:], knownPaths: [card.path] + ) + #expect(normal.toAnalyze.isEmpty) + + let refresh = VolumeMonitorService.reconciliation( + mounted: [card], published: [card.path: card.identity!], + pending: [:], knownPaths: [card.path], forceAnalysis: true + ) + #expect(refresh.stalePaths.isEmpty) + #expect(refresh.toAnalyze.map(\.identity) == [card.identity]) + + // An access grant can race an analysis started before the grant. + // Refresh must start a replacement and retire the old request even + // though the filesystem identity is unchanged. + let pendingRefresh = VolumeMonitorService.reconciliation( + mounted: [card], published: [card.path: card.identity!], + pending: [card.path: card.identity!], knownPaths: [card.path], + forceAnalysis: true + ) + #expect(pendingRefresh.toAnalyze.map(\.identity) == [card.identity]) + #expect(!VolumeMonitorService.acceptsAnalysisRequest(1, current: 2)) + #expect(VolumeMonitorService.acceptsAnalysisRequest(2, current: 2)) + } + + @Test func missingIdentityDropsStaleCardWithoutPublishingAnother() { + let old = mount("old-device:old-uuid") + let unresolved = mount(nil) + let plan = VolumeMonitorService.reconciliation( + mounted: [unresolved], published: [old.path: old.identity!], + pending: [:], knownPaths: [old.path] + ) + #expect(plan.stalePaths == [old.path]) + #expect(plan.toAnalyze.isEmpty) + #expect(!VolumeMonitorService.acceptsAnalysis(old, mounted: [unresolved])) + } + + @Test func disappearedMountClearsPublishedIdentityAfterItsRowWasRemoved() { + let old = mount("old-device:old-uuid") + let plan = VolumeMonitorService.reconciliation( + mounted: [], published: [old.path: old.identity!], + pending: [:], knownPaths: [] + ) + #expect(plan.stalePaths == [old.path]) + #expect(plan.toAnalyze.isEmpty) + } + + @Test func cardWithoutVolumeUUIDCannotEnterAutomaticQueue() { + // A device number alone is not a durable card identity. Discovery may + // show the card, but automatic queueing requires a volume UUID. + let volume = ConnectedDrivesPresentation.Volume( + name: "Card", url: path, totalBytes: 64, freeBytes: 32, + isRemovable: true, isInternal: false, volumeID: nil, cameraName: "Camera" + ) + let rows = ConnectedDrivesPresentation.make( + volumes: [volume], sourceURL: nil, destinationURLs: [] + ) + #expect(AutoQueuePolicy.candidates( + eligibleRows: rows, seenVolumeIDs: [], activeDestinationVolumeIDs: [] + ).isEmpty) + } + + @Test func replacedSelectedSourceIsClearedButSameCardIsKept() { + let shared = SharedAppCoordinator( + platformManager: RecordingPlatformManager(fileOperations: RecordingFileOperations()), + projectStore: InMemoryPhotographerJobStore(), + defaults: .isolatedWorkflowDefaults() + ) + let model = MacVolumeAccessModel(shared: shared, enableVolumeMonitoring: false) + var liveIdentity = "old-device:old-uuid" + model.sourceMountIdentity = { _ in liveIdentity } + shared.sourceURL = path + + model.handleConnectedVolumesUpdate([]) + #expect(shared.sourceURL == path) + + // Neither an unmount event nor a path disappearance is delivered: + // the next snapshot already has a different card at the same path. + liveIdentity = "new-device:new-uuid" + model.handleConnectedVolumesUpdate([]) + #expect(shared.sourceURL == nil) + } +} +#endif diff --git a/Packages/BitMatchEngine/Sources/BitMatchEngine/Models/Verification.swift b/Packages/BitMatchEngine/Sources/BitMatchEngine/Models/Verification.swift index 55232810..6c720c5e 100644 --- a/Packages/BitMatchEngine/Sources/BitMatchEngine/Models/Verification.swift +++ b/Packages/BitMatchEngine/Sources/BitMatchEngine/Models/Verification.swift @@ -218,13 +218,23 @@ public enum VerificationMode: String, CaseIterable, Identifiable, Codable, Senda case paranoid = "Paranoid" public var id: String { self.rawValue } + + /// Compact copy-mode wording for setup, where the mode name is already shown. + public var shortDescription: String { + switch self { + case .quick: return "File sizes only; contents not verified" + case .standard: return "SHA-256 backup readback; existing files recheck the source" + case .thorough: return "Separate card re-read; SHA-256 and MD5" + case .paranoid: return "Separate card re-read; byte comparison and SHA-256" + } + } public var description: String { switch self { - case .quick: return "Quick checks file sizes only; file contents are not checksum-verified." - case .standard: return "Standard reads the card once and checks every drive." - case .thorough: return "Thorough also re-reads the card for verification." - case .paranoid: return "Paranoid also compares every byte." + case .quick: return "Quick checks file sizes, not contents. It never marks the card safe to erase." + case .standard: return "Standard reads each backup back with SHA-256. New copies use the source hash from copying; existing files are checked against a fresh source read." + case .thorough: return "Thorough separately re-reads the card and checks SHA-256 and MD5 on every backup." + case .paranoid: return "Paranoid separately re-reads the card and compares every byte with each backup, plus SHA-256." } } diff --git a/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferJournal.swift b/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferJournal.swift index 9c290a78..3e9ee2f2 100644 --- a/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferJournal.swift +++ b/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferJournal.swift @@ -127,6 +127,10 @@ public struct LocalTransferRecord: Identifiable, Codable, Sendable { public let retryOf: UUID? /// Selected files per destination, from the engine manifest (exclusions omitted). public var plannedFileCounts: [Int]? + /// True only when Quick's size-only check was the sole reason the attempt + /// could not be verified. False when reporting, coverage, or another part + /// of the attempt also failed; nil for journals written before this field. + public var quickOnlyUnverified: Bool? = nil /// Physical drives represented by `destinations`. Nil for older records /// whose disk independence was never assessed. public var independentDestinationCount: Int? @@ -156,7 +160,7 @@ public struct LocalTransferRecord: Identifiable, Codable, Sendable { case id, createdAt, source, destinations, verificationMode, cameraSettings, reportSettings case generateASCMHL, projectID, projectCardID, state, startedAt, endedAt case copyDurationSeconds, verifyDurationSeconds, performanceTelemetry, summary, results, sourceFingerprint - case independentDestinationCount, retryOf, plannedFileCounts + case independentDestinationCount, retryOf, plannedFileCounts, quickOnlyUnverified } public init(from decoder: Decoder) throws { @@ -189,6 +193,7 @@ public struct LocalTransferRecord: Identifiable, Codable, Sendable { independentDestinationCount = try c.decodeIfPresent(Int.self, forKey: .independentDestinationCount) retryOf = try c.decodeIfPresent(UUID.self, forKey: .retryOf) plannedFileCounts = try c.decodeIfPresent([Int].self, forKey: .plannedFileCounts) + quickOnlyUnverified = try c.decodeIfPresent(Bool.self, forKey: .quickOnlyUnverified) } public var issueCount: Int { results.filter { !$0.isSuccessStatus }.count } } @@ -605,6 +610,7 @@ public final class TransferJournal: Sendable { results: [ResultRow], summary: String, hadIssues: Bool, + quickOnlyUnverified: Bool? = nil, copyDurationSeconds: TimeInterval? = nil, verifyDurationSeconds: TimeInterval? = nil, performanceTelemetry: TransferPerformanceTelemetry? = nil, @@ -628,17 +634,34 @@ public final class TransferJournal: Sendable { ) } record.sourceFingerprint = sourceFingerprint - record.state = hadIssues || results.isEmpty || results.contains(where: { !$0.isSuccessStatus }) ? .issues : .completed + record.quickOnlyUnverified = record.verificationMode == .quick ? quickOnlyUnverified : nil + // A current transfer records the selected manifest size before + // copying. A shorter or duplicated result set is incomplete even + // if every retained row says it verified. Legacy nil-plan records + // keep their historical rule. + let incompletePlan = !Self.hasCompletePlannedResults(record) + // A complete plan can still contain copied-but-unverified rows. + // Only Quick mode intentionally accepts that outcome. + let missingVerification = record.verificationMode != .quick && results.contains { row in + row.isSuccessStatus && !row.isVerifiedStatus + } + record.state = hadIssues || results.isEmpty || incompletePlan || missingVerification + || results.contains(where: { !$0.isSuccessStatus }) ? .issues : .completed + let truthfulSummary = incompletePlan + ? "Transfer results are incomplete. Do not erase the card." + : missingVerification && !hadIssues + ? "Some selected files were not verified. Do not erase the card." + : summary // A Quick-mode record must always say its contents were not // verified, whatever `summary` says. In the normal flow `summary` // already carries that note (see `TransferCompletion.verdict`), // so only add it once, never twice. if record.verificationMode == .quick { let note = TransferCompletion.quickModeNote - record.summary = summary.contains(note) ? summary : "\(summary) \(note)" + record.summary = truthfulSummary.contains(note) ? truthfulSummary : "\(truthfulSummary) \(note)" record.state = .issues } else { - record.summary = summary + record.summary = truthfulSummary } record.endedAt = Date() } @@ -668,22 +691,43 @@ public final class TransferJournal: Sendable { return PathContainment.isWithin(path, root: destination.url.path) } } - guard groups.allSatisfy({ !$0.isEmpty }), + guard groups.allSatisfy({ !$0.isEmpty && Set($0.map(\.path)).count == $0.count }), groups.reduce(0, { $0 + $1.count }) == record.results.count, let expectedPaths = groups.first.map({ Set($0.map(\.path)) }), !expectedPaths.isEmpty else { return false } - if let planned = record.plannedFileCounts { - guard planned.count == groups.count, - zip(groups, planned).allSatisfy({ rows, count in - let selected = rows.filter { - ResultOutcome(statusText: $0.status) != .excludedAppleDouble - } - return selected.count == count && Set(selected.map(\.path)).count == count + if let plan = record.plannedFileCounts { + guard plan.count == groups.count, + zip(plan, groups).allSatisfy({ expected, rows in + expected > 0 && Set(rows.map(\.path)).count == expected }) else { return false } } return groups.allSatisfy { Set($0.map(\.path)) == expectedPaths } } + /// Match the recorded selected-file count for each destination. Reviewed + /// AppleDouble exclusions are retained as evidence but are not selected + /// files, so they do not make a complete selected-file plan look short. + private static func hasCompletePlannedResults(_ record: LocalTransferRecord) -> Bool { + guard let plan = record.plannedFileCounts else { return true } + guard plan.count == record.destinations.count else { return false } + let selected = record.results.filter { + ResultOutcome(statusText: $0.status) != .excludedAppleDouble + } + let groups = record.destinations.map { destination in + selected.filter { row in + guard let path = row.destinationPath else { return false } + return PathContainment.isWithin(path, root: destination.url.path) + } + } + guard groups.reduce(0, { $0 + $1.count }) == selected.count, + zip(plan, groups).allSatisfy({ expected, rows in + expected >= 0 && rows.count == expected + && Set(rows.map(\.path)).count == expected + }) else { return false } + guard let expectedPaths = groups.first.map({ Set($0.map(\.path)) }) else { return false } + return groups.allSatisfy { Set($0.map(\.path)) == expectedPaths } + } + public func interrupt(id: UUID, summary: String, results: [ResultRow]? = nil) throws { try update(id: id) { record in guard record.state == .running else { throw LocalTransferJournalError.invalidState } diff --git a/Packages/BitMatchEngine/Tests/BitMatchEngineTests/BufferSizeBenchTests.swift b/Packages/BitMatchEngine/Tests/BitMatchEngineTests/BufferSizeBenchTests.swift index aead2b66..024a0859 100644 --- a/Packages/BitMatchEngine/Tests/BitMatchEngineTests/BufferSizeBenchTests.swift +++ b/Packages/BitMatchEngine/Tests/BitMatchEngineTests/BufferSizeBenchTests.swift @@ -5,6 +5,9 @@ import XCTest /// Opt-in storage benchmark. Run with: /// `BITMATCH_BENCH=1 swift test --filter BufferSizeBenchTests` +/// Source writes and timed reads request F_NOCACHE, including the first run. +/// This requests a cache bypass but cannot prove a physical-device read; +/// confirm that separately with the selected drive's I/O counters. final class BufferSizeBenchTests: XCTestCase { private let totalBytes: Int64 = 2 * 1_024 * 1_024 * 1_024 private let fileBytes: Int64 = 256 * 1_024 * 1_024 @@ -56,6 +59,7 @@ final class BufferSizeBenchTests: XCTestCase { } print("\nF_NOCACHE buffer benchmark — 2 GiB in 8 × 256 MiB files, \(runs) runs") + print("Cache bypass requested for source creation, hash reads, and copy I/O; physical-device reads are not independently confirmed. Check drive read counters before using these numbers for a storage decision.") print("Buffer | Hash MB/s median (IQR) | Copy MB/s median (IQR)") print("------ | ---------------------- | ----------------------") for row in rows { @@ -88,6 +92,12 @@ final class BufferSizeBenchTests: XCTestCase { let descriptor = Darwin.open(url.path, O_WRONLY | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR) try requireDescriptor(descriptor, operation: "create", path: url.path) defer { Darwin.close(descriptor) } + // fsync alone flushes the file; it does not evict the data just written. + // Avoid warming the source in the filesystem cache before the first + // timed read, just as the timed source reads avoid that cache. + guard Darwin.fcntl(descriptor, F_NOCACHE, 1) != -1 else { + throw posixError("F_NOCACHE", url.path) + } var remaining = fileBytes while remaining > 0 { let count = min(Int64(block.count), remaining) diff --git a/Shared/Core/Models/CompletionVerdictPresentation.swift b/Shared/Core/Models/CompletionVerdictPresentation.swift index b17aace3..cafd8c9c 100644 --- a/Shared/Core/Models/CompletionVerdictPresentation.swift +++ b/Shared/Core/Models/CompletionVerdictPresentation.swift @@ -57,7 +57,7 @@ struct CompletionVerdictPresentation: Equatable, Sendable { symbol: safetyState.symbol, sourceGuidance: "Do not erase \(card). Nothing here has been confirmed safe." ) - case .waiting, .preparing, .copying, .verifying, .interrupted: + case .waiting, .preparing, .copying, .verifying, .finishing, .interrupted: preconditionFailure("A completion verdict must produce a finish state") } } diff --git a/Shared/Core/Models/QueueAccessoryPresentation.swift b/Shared/Core/Models/QueueAccessoryPresentation.swift index 7984614b..0f995b84 100644 --- a/Shared/Core/Models/QueueAccessoryPresentation.swift +++ b/Shared/Core/Models/QueueAccessoryPresentation.swift @@ -76,10 +76,10 @@ struct QueueAccessoryPresentation: Equatable, Sendable { if let running, let index = queue.rows.firstIndex(where: { $0.id == running.id }) { let completed = queue.rows.filter(\.isFinished).count - let cardProgress = min(max(running.progressFraction ?? 0, 0), 1) + let cardProgress = min(max(running.progressFraction ?? 0, 0), 0.99) let overall = min( max((Double(completed) + cardProgress) / Double(queue.rows.count), 0), - 1 + 0.99 ) var details = ["\(index + 1) of \(queue.rows.count)", percent(overall)] if let timeRemaining { details.append("\(timeRemaining) left") } diff --git a/Shared/Core/Models/QueueSessionPresentation.swift b/Shared/Core/Models/QueueSessionPresentation.swift index 52a07ce3..7e7ab2de 100644 --- a/Shared/Core/Models/QueueSessionPresentation.swift +++ b/Shared/Core/Models/QueueSessionPresentation.swift @@ -47,7 +47,7 @@ struct QueueSessionRow: Identifiable, Equatable, Sendable { func oneLineStatus(timeRemaining: String? = nil) -> String { let prefix = compactWaitingText if isRunning { - let progress = progressFraction.map { " \(Int((min(max($0, 0), 1) * 100).rounded(.down)))%" } ?? "" + let progress = progressFraction.map { " \(Int((min(max($0, 0), 0.99) * 100).rounded(.down)))%" } ?? "" let measuredTime = timeRemaining == TransferProgressPresentation.estimatingTimeLeft ? nil : timeRemaining @@ -56,6 +56,7 @@ struct QueueSessionRow: Identifiable, Equatable, Sendable { switch safetyState { case .copying: phase = "Copying" case .verifying: phase = "Verifying" + case .finishing: phase = "Finishing" case .preparing: phase = "Preparing" default: phase = safetyState.title } @@ -149,7 +150,7 @@ struct QueueSessionPresentation: Equatable, Sendable { case .failed: tally.failed += 1 case .interrupted: tally.interrupted += 1 case .waiting: tally.notStarted += 1 - case .preparing, .copying, .verifying: break + case .preparing, .copying, .verifying, .finishing: break } } let running = rows.first { $0.safetyState.isActiveQueueState } @@ -213,10 +214,16 @@ struct QueueSessionPresentation: Equatable, Sendable { ) -> QueueSessionRow { let state: CardSafetyState if record.state == .running { - let percent = progress.map { Int((min(max($0.stageProgress ?? $0.overallProgress, 0), 1) * 100).rounded(.down)) } + // A completed file-work stage is not a finished transfer. MHL, + // report writing and journal settlement can still fail or stop. + let activeFraction = progress.map { + min(max($0.stageProgress ?? $0.overallProgress, 0), 0.99) + } + let percent = activeFraction.map { Int(($0 * 100).rounded(.down)) } switch progress?.currentStage { case .copying: state = .copying(progress: percent) - case .verifying, .generating, .completed: state = .verifying(progress: percent) + case .verifying: state = .verifying(progress: percent) + case .generating, .completed: state = .finishing(progress: percent) default: state = .preparing } } else { @@ -253,7 +260,9 @@ struct QueueSessionPresentation: Equatable, Sendable { evidence: evidence, destinations: destinationSummary(destinationNames), safetyState: state, - progressFraction: record.state == .running ? progress?.stageProgress ?? progress?.overallProgress : nil, + progressFraction: record.state == .running ? progress.map { + min(max($0.stageProgress ?? $0.overallProgress, 0), 0.99) + } : nil, action: action, cause: cause, copySummary: TransferOutcomePresentation.makeCopySummary( @@ -288,7 +297,7 @@ struct QueueSessionPresentation: Equatable, Sendable { operationState = .failed case .interrupted: operationState = .cancelled - case .waiting, .preparing, .copying, .verifying: + case .waiting, .preparing, .copying, .verifying, .finishing: return nil } let uniqueFiles = Dictionary(grouping: record.results, by: \.path).values.compactMap(\.first) @@ -366,7 +375,7 @@ struct QueueSessionPresentation: Equatable, Sendable { extension CardSafetyState { var isActiveQueueState: Bool { switch self { - case .preparing, .copying, .verifying: true + case .preparing, .copying, .verifying, .finishing: true default: false } } diff --git a/Shared/Core/Models/ResultPresentation.swift b/Shared/Core/Models/ResultPresentation.swift index a45a4de1..a6366229 100644 --- a/Shared/Core/Models/ResultPresentation.swift +++ b/Shared/Core/Models/ResultPresentation.swift @@ -24,6 +24,9 @@ enum CardSafetyState: Equatable, Sendable { case preparing case copying(progress: Int?) case verifying(progress: Int?) + /// File work has drained, but evidence/reporting and journal settlement + /// are still in progress. Never a completed verdict. + case finishing(progress: Int?) case safeToErase case copiedNotVerified case needsAttention @@ -31,7 +34,7 @@ enum CardSafetyState: Equatable, Sendable { case interrupted static let invariantSamples: [Self] = [ - .waiting, .preparing, .copying(progress: 42), .verifying(progress: 31), + .waiting, .preparing, .copying(progress: 42), .verifying(progress: 31), .finishing(progress: 99), .safeToErase, .copiedNotVerified, .needsAttention, .failed, .interrupted, ] @@ -41,6 +44,7 @@ enum CardSafetyState: Equatable, Sendable { case .preparing: "Preparing" case .copying(let progress): Self.progressTitle("Copying", progress: progress) case .verifying(let progress): Self.progressTitle("Verifying", progress: progress) + case .finishing(let progress): Self.progressTitle("Finishing", progress: progress) case .safeToErase: "Safe to erase" case .copiedNotVerified: "Copied, not verified" case .needsAttention: "Needs attention" @@ -55,6 +59,7 @@ enum CardSafetyState: Equatable, Sendable { case .preparing: "hourglass" case .copying: "doc.on.doc" case .verifying: "checkmark.shield" + case .finishing: "hourglass" case .safeToErase: "checkmark.circle.fill" case .copiedNotVerified: "doc.on.doc" case .needsAttention: "exclamationmark.triangle.fill" @@ -66,7 +71,7 @@ enum CardSafetyState: Equatable, Sendable { var tint: CardSafetyTint { switch self { case .waiting: .gray - case .preparing, .copying, .verifying: .blue + case .preparing, .copying, .verifying, .finishing: .blue case .safeToErase: .green // Amber, like a Quick compare (THESIS decision, 2026-09-25); its // symbol and words keep it apart from Needs attention. @@ -86,7 +91,7 @@ enum CardSafetyState: Equatable, Sendable { switch self { case .copiedNotVerified, .needsAttention, .failed, .interrupted: return "Do not erase the card." - case .waiting, .preparing, .copying, .verifying, .safeToErase: + case .waiting, .preparing, .copying, .verifying, .finishing, .safeToErase: return nil } } @@ -99,6 +104,7 @@ enum CardSafetyState: Equatable, Sendable { case .preparing: return "Preparing \(card)" case .copying: return "Copying \(card)" case .verifying: return "Verifying \(card)" + case .finishing: return "Finishing \(card)" case .safeToErase: return "\(cardStart) is safe to erase" case .copiedNotVerified: return "\(cardStart) copied, not verified" case .needsAttention: return "\(cardStart) needs attention" @@ -108,7 +114,7 @@ enum CardSafetyState: Equatable, Sendable { } static func make(state: OperationState, verdict: CompletionVerdict, progress: Double? = nil) -> Self { - let percent = progress.map { Int((min(max($0, 0), 1) * 100).rounded(.down)) } + let percent = progress.map { Int((min(max($0, 0), 0.99) * 100).rounded(.down)) } switch state { case .idle, .notStarted: return .waiting case .inProgress, .resuming: return .preparing diff --git a/Shared/Core/Models/TransferFinishNotice.swift b/Shared/Core/Models/TransferFinishNotice.swift index 3007ffe5..183cf615 100644 --- a/Shared/Core/Models/TransferFinishNotice.swift +++ b/Shared/Core/Models/TransferFinishNotice.swift @@ -57,7 +57,7 @@ struct TransferFinishNotice: Equatable, Sendable { return .init(title: "\(card) failed", body: "Do not erase the card.", kind: .attention) case .interrupted: return .init(title: "\(card) was interrupted", body: "Do not erase the card.", kind: .attention) - case .waiting, .preparing, .copying, .verifying: + case .waiting, .preparing, .copying, .verifying, .finishing: return nil } } diff --git a/Shared/Core/Models/TransferLibraryPresentation.swift b/Shared/Core/Models/TransferLibraryPresentation.swift index f0c8602f..d65415a9 100644 --- a/Shared/Core/Models/TransferLibraryPresentation.swift +++ b/Shared/Core/Models/TransferLibraryPresentation.swift @@ -165,6 +165,11 @@ enum TransferLibraryPresentation { case .completed, .issues: guard !record.results.isEmpty else { return .needsAttention } if record.verificationMode == .quick { + // Quick journals are always `.issues`: that state alone does + // not distinguish a clean size-only copy from a failed report. + // Older records have no explicit verdict, so keep them out of + // the benign copied-only state rather than hide a real issue. + guard record.quickOnlyUnverified == true else { return .needsAttention } if hasCompleteEvidence(record, where: { ResultOutcome(statusText: $0.status) == .copiedUnverified }) { @@ -209,7 +214,10 @@ enum TransferLibraryPresentation { destinations: record.destinations.map(\.url) ) guard summaries.count == record.destinations.count, - summaries.allSatisfy({ !$0.rows.isEmpty && $0.rows.allSatisfy(accepts) }), + summaries.allSatisfy({ summary in + !summary.rows.isEmpty && summary.rows.allSatisfy(accepts) + && Set(summary.rows.map(\.path)).count == summary.rows.count + }), let expectedPaths = summaries.first.map({ Set($0.rows.map(\.path)) }), !expectedPaths.isEmpty else { return false } if record.plannedFileCounts != nil, savedResultsCoverSelectedPlan(record) != true { return false } diff --git a/Shared/Core/Models/TransferOptionsPresentation.swift b/Shared/Core/Models/TransferOptionsPresentation.swift index 6e539cc9..b7488beb 100644 --- a/Shared/Core/Models/TransferOptionsPresentation.swift +++ b/Shared/Core/Models/TransferOptionsPresentation.swift @@ -30,6 +30,7 @@ struct TransferOptionsPresentation: Equatable, Sendable { makeReport: Bool?, cameraLabel: String?, includesVerificationNote: Bool = true, + isCompare: Bool = false, writesPDF: Bool = TransferOptionsPresentation.platformWritesPDF ) -> TransferOptionsPresentation { var notes: [String] = [] @@ -49,7 +50,9 @@ struct TransferOptionsPresentation: Equatable, Sendable { let enabled = ascMHLEnabled(for: verificationMode) return TransferOptionsPresentation( advancedNote: notes.joined(separator: " · "), - verificationDetail: verificationMode.description, + verificationDetail: isCompare + ? CompareCheckPlan.make(for: verificationMode).summary + : verificationMode.description, ascMHLEnabled: enabled, ascMHLFootnote: ascMHLFootnote(for: verificationMode), reportToggleTitle: reportToggleTitle(writesPDF: writesPDF) diff --git a/Shared/Core/Models/TransferOutcomePresentation.swift b/Shared/Core/Models/TransferOutcomePresentation.swift index cc4bff68..f7df52ea 100644 --- a/Shared/Core/Models/TransferOutcomePresentation.swift +++ b/Shared/Core/Models/TransferOutcomePresentation.swift @@ -143,6 +143,7 @@ struct TransferOutcomePresentation: Equatable, Sendable { case .preparing: "is preparing" case .copying: "is copying" case .verifying: "is verifying" + case .finishing: "is finishing" } } @@ -383,7 +384,7 @@ struct TransferOutcomePresentation: Equatable, Sendable { case .safeToErase, .copiedNotVerified, .needsAttention: if case .completed = state { return "Completed in \(text)" } return "Ended after \(text)" - case .waiting, .preparing, .copying, .verifying: + case .waiting, .preparing, .copying, .verifying, .finishing: return "Elapsed \(text)" } } @@ -519,6 +520,7 @@ struct TransferOutcomePresentation: Equatable, Sendable { if warningCount > 0 { return warningCount == 1 ? "1 warning" : "\(warningCount) warnings" } return completionDetail case .failed: return recordedReason?.isEmpty == false ? recordedReason : completionDetail + case .interrupted: return recordedReason?.isEmpty == false ? recordedReason : nil default: return nil } } @@ -542,6 +544,7 @@ struct TransferOutcomePresentation: Equatable, Sendable { case .preparing: verdict = "preparing" case .copying: verdict = "copying" case .verifying: verdict = "verifying" + case .finishing: verdict = "finishing" case .safeToErase: verdict = "safe to erase" case .copiedNotVerified: verdict = "copied, not verified (size check only)" case .needsAttention: verdict = reason.map { "needs attention: \($0)" } ?? "needs attention" @@ -591,10 +594,13 @@ struct TransferOutcomePresentation: Equatable, Sendable { let separator = failure.last.map { ".!?".contains($0) } == true ? " " : ". " return "\(failure)\(separator)Do not erase the card." case .interrupted: - // The interrupted headline carries the card warning itself, since - // `bannerGuidance` is suppressed for this state (no duplication). - return "The transfer stopped before every destination was verified. Do not erase the card." - case .waiting, .preparing, .copying, .verifying: + // Saved history distinguishes an explicit cancel from an + // unexpected interruption. Carry that reason into Finish too. + let interruption = reason ?? "The transfer stopped before every destination was verified." + if interruption.localizedCaseInsensitiveContains("do not erase") { return interruption } + let separator = interruption.last.map { ".!?".contains($0) } == true ? " " : ". " + return "\(interruption)\(separator)Do not erase the card." + case .waiting, .preparing, .copying, .verifying, .finishing: return fallback } } diff --git a/Shared/Core/Models/TransferProgressPresentation.swift b/Shared/Core/Models/TransferProgressPresentation.swift index f0d18ca1..35df1a00 100644 --- a/Shared/Core/Models/TransferProgressPresentation.swift +++ b/Shared/Core/Models/TransferProgressPresentation.swift @@ -117,9 +117,9 @@ struct TransferProgressPresentation: Equatable, Sendable { /// "A001 to 2 backups", or why the run is paused. let detail: String? let tone: ProgressTone - /// One definition everywhere: the engine's (copied + verified) / - /// (files × stages). Nil before the engine reports, so the bar is - /// indeterminate rather than a false 0%. + /// The engine's (copied + verified) / (files × stages), capped below 100% + /// while the journal still owns an active attempt. Nil before the engine + /// reports, so the bar is indeterminate rather than a false 0%. let fraction: Double? let percentText: String? /// "120 of 400" (file copies across all backups). The phase and heading @@ -188,7 +188,9 @@ struct TransferProgressPresentation: Equatable, Sendable { ) -> Self { let phase = Self.phase(state: state, stage: progress?.currentStage) let isPaused = phase == .paused - let fraction = progress.map { min(1, max(0, $0.overallProgress)) } + // The engine can finish file work before ASC MHL, report writing and + // the saved verdict settle. A running transfer must never show 100%. + let fraction = progress.map { min(isRunning ? 0.99 : 1, max(0, $0.overallProgress)) } let controls = ProgressControls( canPause: state.canPause, canResume: state.canResume, diff --git a/Shared/Core/Services/LocalTransferJournal.swift b/Shared/Core/Services/LocalTransferJournal.swift index f7ae69d2..494f22f3 100644 --- a/Shared/Core/Services/LocalTransferJournal.swift +++ b/Shared/Core/Services/LocalTransferJournal.swift @@ -162,6 +162,7 @@ final class LocalTransferJournal: ObservableObject { results: [ResultRow], summary: String, hadIssues: Bool, + quickOnlyUnverified: Bool? = nil, copyDurationSeconds: TimeInterval? = nil, verifyDurationSeconds: TimeInterval? = nil, performanceTelemetry: TransferPerformanceTelemetry? = nil, @@ -173,6 +174,7 @@ final class LocalTransferJournal: ObservableObject { results: results, summary: summary, hadIssues: hadIssues, + quickOnlyUnverified: quickOnlyUnverified, copyDurationSeconds: copyDurationSeconds, verifyDurationSeconds: verifyDurationSeconds, performanceTelemetry: performanceTelemetry, diff --git a/Shared/Core/Services/SharedAppCoordinator.swift b/Shared/Core/Services/SharedAppCoordinator.swift index 7f105d1a..e0bbba28 100644 --- a/Shared/Core/Services/SharedAppCoordinator.swift +++ b/Shared/Core/Services/SharedAppCoordinator.swift @@ -971,9 +971,37 @@ class SharedAppCoordinator: ObservableObject { /// A compare shows its outcome inside the Compare screen; it is never /// the transfer outcome summary. var showsOutcomeSummary: Bool { + Self.shouldShowOutcomeSummary( + state: operationState, + recordState: outcomeRecord?.state, + isOperationInProgress: isOperationInProgress, + hasJournalPersistenceError: transferJournal.persistenceError != nil, + lastOperationWasCompare: lastOperationWasCompare + ) + } + + static func shouldShowOutcomeSummary( + state: OperationState, + recordState: LocalTransferState?, + isOperationInProgress: Bool, + hasJournalPersistenceError: Bool, + lastOperationWasCompare: Bool + ) -> Bool { guard !lastOperationWasCompare else { return false } - switch operationState { - case .completed, .failed, .cancelled: + switch state { + case .completed: + // The executor can finish before the durable journal settles. + // Keep showing the active queue row until History has a terminal + // record; otherwise a verified-looking Finish can flash early. + guard let recordState else { return false } + return recordState != .queued && recordState != .running + case .failed, .cancelled: + // Error handling also writes its journal outcome asynchronously. + // Only a confirmed journal write failure may fall back to the + // live failure state after the operation stops. + if recordState == .running { + return state == .failed && !isOperationInProgress && hasJournalPersistenceError + } return true default: return false @@ -2044,6 +2072,7 @@ class SharedAppCoordinator: ObservableObject { results: runResults.rows, summary: info.message, hadIssues: !info.success, + quickOnlyUnverified: info.copiedNotVerified, copyDurationSeconds: durations.copySeconds, verifyDurationSeconds: durations.verifySeconds, performanceTelemetry: copyVerifyExecutor.completedPerformanceTelemetry, diff --git a/Shared/Views/ReportView.swift b/Shared/Views/ReportView.swift index d03ba42d..fcc5e87e 100644 --- a/Shared/Views/ReportView.swift +++ b/Shared/Views/ReportView.swift @@ -945,7 +945,7 @@ struct ReportView: View { return "The card is not safe to erase because the transfer failed." case .interrupted: return "The card is not safe to erase because the transfer was interrupted." - case .waiting, .preparing, .copying, .verifying: + case .waiting, .preparing, .copying, .verifying, .finishing: return "The card is not safe to erase because verification did not finish." case .safeToErase: return "Every file on every destination was verified." diff --git a/Shared/Views/Setup/SetupLocationsView.swift b/Shared/Views/Setup/SetupLocationsView.swift index 8689c5d8..84772241 100644 --- a/Shared/Views/Setup/SetupLocationsView.swift +++ b/Shared/Views/Setup/SetupLocationsView.swift @@ -293,12 +293,7 @@ struct SetupLocationsView: View { } private var shortVerificationDetail: String { - switch verificationMode { - case .quick: "File sizes only; contents are not checksum-verified" - case .standard: "Reads the card once and checks every drive" - case .thorough: "Also re-reads the card for verification" - case .paranoid: "Also compares every byte" - } + verificationMode.shortDescription } private var destinationsAccessibilityLabel: String { diff --git a/Shared/Views/Setup/TransferOptionsSection.swift b/Shared/Views/Setup/TransferOptionsSection.swift index c5509637..c202d7b0 100644 --- a/Shared/Views/Setup/TransferOptionsSection.swift +++ b/Shared/Views/Setup/TransferOptionsSection.swift @@ -44,7 +44,8 @@ struct TransferOptionsSection: View { generateASCMHL: generateASCMHL?.wrappedValue, makeReport: makeReport?.wrappedValue, cameraLabel: cameraLabel, - includesVerificationNote: showsVerificationPicker + includesVerificationNote: showsVerificationPicker, + isCompare: showsVerificationPicker ) } diff --git a/Shared/Views/TransferPresentationAdapters.swift b/Shared/Views/TransferPresentationAdapters.swift index 5cae9e28..69ecc1f1 100644 --- a/Shared/Views/TransferPresentationAdapters.swift +++ b/Shared/Views/TransferPresentationAdapters.swift @@ -1,4 +1,5 @@ import Foundation +import BitMatchEngine extension TransferProgressPresentation { /// The one coordinator adapter used by every platform's running row. @@ -41,18 +42,30 @@ extension TransferOutcomePresentation { static func make(coordinator: SharedAppCoordinator) -> Self { let record = coordinator.outcomeRecord let rows = record?.results ?? coordinator.results - let recordedIssueCount = rows.filter { !$0.isSuccessStatus }.count + let recordedIssueCount = rows.filter { + !$0.isSuccessStatus && ResultOutcome(statusText: $0.status) != .excludedAppleDouble + }.count let duration: TimeInterval? = record.flatMap { record in guard let started = record.startedAt, let ended = record.endedAt else { return nil } return ended.timeIntervalSince(started) } let isFinished = record.map { $0.state != .queued && $0.state != .running } ?? false + let journalWriteFailed = record?.state == .running + && coordinator.transferJournal.persistenceError != nil + && !coordinator.isOperationInProgress + && coordinator.operationState == .failed + let displayedState = journalWriteFailed + ? OperationState.failed + : record.flatMap { recordedState(for: $0) } ?? coordinator.operationState return make( - state: coordinator.operationState, + // The journal is the durable terminal outcome. An unexpected + // interruption may leave the live state `.failed`, but its saved + // record is `.interrupted`; Finish and History must agree. + state: displayedState, rows: rows, destinations: record?.destinations.map(\.url) ?? [], hasErrors: record == nil ? coordinator.hasErrors : recordedIssueCount > 0, - hasCriticalErrors: coordinator.hasCriticalErrors, + hasCriticalErrors: record == nil && coordinator.hasCriticalErrors, errorCount: record == nil ? coordinator.errorCount : recordedIssueCount, warningCount: coordinator.warningCount, duration: duration, @@ -68,4 +81,26 @@ extension TransferOutcomePresentation { independentDestinationCount: record?.independentDestinationCount ) } + + /// Translate a saved attempt into presentation state without treating an + /// interrupted process as a user-requested cancellation in the journal. + @MainActor + static func recordedState(for record: LocalTransferRecord) -> OperationState? { + switch TransferLibraryPresentation.safetyState(for: record) { + case .safeToErase: + return .completed(.init(success: true, message: record.summary)) + case .copiedNotVerified: + return .completed(.init(success: false, message: record.summary, copiedNotVerified: true)) + case .needsAttention: + return .completed(.init(success: false, message: record.summary)) + case .failed: + return .failed + case .interrupted: + return .cancelled + case .waiting, .preparing, .copying, .verifying, .finishing: + // Even a direct caller bypassing `showsOutcomeSummary` must not + // turn an active journal row into a verified finish. + return .inProgress + } + } } diff --git a/docs/TODO.md b/docs/TODO.md index 955925d4..999f0d28 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -12,14 +12,16 @@ This is the order to **investigate or start a bounded slice**, not a promise to The two active reports come first as **casework**, but neither is a ready-to-ship fix: [#23](https://github.com/BitmatchApp/Bitmatch/issues/23) needs the reporter's version/phase or a reproduced memory path, and [#10](https://github.com/BitmatchApp/Bitmatch/issues/10) needs decisive diagnostics or a physical reproduction. Synthetic passes alone cannot close either report. +Unreleased branch progress: terminal/History presentation and Mac card-event reconciliation have bounded implementations; an SFTP contract test checks downloaded remote bytes, and the buffer benchmark requests uncached writes. Setup, History and report wording have landed, while the broader assurance audit, physical measurements and signed-app walkthrough remain open below. + | Rank | Build or decision work | Usefulness | Difficulty | Blast radius | Shippability | | ---: | --- | --- | --- | --- | --- | -| 1 | Terminal progress and outcome parity | High | Low–medium | Medium | Quick | -| 2 | Verification coverage and wording audit; defer any default change until benchmarking | High | Medium | Medium | Scoped | +| 1 | Terminal progress and outcome parity (branch implementation; release check open) | High | Low–medium | Medium | Quick | +| 2 | Verification coverage and wording audit (setup, History and reports aligned; mode/fault audit open) | High | Medium | Medium | Scoped | | 3 | Honest claims when physical drive identity is unknown | High | Medium | Medium | Scoped | -| 4 | Distinguish local-source, local-backup and SFTP assurance | High | Low–medium | Low–medium | Quick | +| 4 | Distinguish local-source, local-backup and SFTP assurance (labels shipped; interruption/stale-evidence tests open) | High | Low–medium | Low–medium | Quick | | 5 | Expand the MHL interoperability corpus | High | Medium | Low | Scoped | -| 6 | Reproduce missed Mac card events; add a backstop only if needed | Medium–high | Low | Low | Quick | +| 6 | Reconcile missed Mac card events (branch implementation; physical check open) | Medium–high | Low | Low | Quick | | 7 | Eliminate the exFAT/FAT publication race | Very high | High | High | Long; protected safety work | | 8 | Mark failed readback output unmistakably on disk | High | High | High | After publication ownership | | 9 | Mac menu bar companion, first Remote phase | High | Medium | Medium | Scoped | @@ -59,7 +61,7 @@ These eight supporting tasks are not competing features. Do them when their evid | Next gate | Supporting work | Usefulness | Difficulty | Blast radius | Shippability | | ---: | --- | --- | --- | --- | --- | -| 1 | Make the buffer benchmark's first read representative | High | Low | Low | Quick | +| 1 | Confirm the buffer benchmark's first read reaches physical storage (uncached-write request added) | High | Low | Low | Quick | | 2 | Run the end-to-end two-drive physical benchmark | High | Medium | Low | Hardware-dependent | | 3 | Remove permission prompts from automated tests | Medium–high | Medium | Low | Scoped | | 4 | Keep release walkthroughs and synthetic/physical results separate | High | Ongoing | Low | Every relevant release | @@ -88,9 +90,9 @@ The priority buckets are not a strict implementation sequence: investigate P0 re - [ ] **Make failed readback output unmistakable on disk.** BitMatch publishes a copied file at its normal destination name before readback; the existing corruption regression confirms that the bad file remains there even though the app verdict correctly fails closed. Investigate marking or moving only an attempt-owned, identity-checked file that fails verification. A read error does not prove the bytes are corrupt, and even bad bytes may be worth preserving. **Acceptance:** injected mismatch, read error, external replacement, interrupted marking and marking failure never overwrite, delete or relabel another writer's file; the real on-disk path and uncertainty appear in History/reports and recovery; source media stays untouched. -- [ ] **Verification coverage and default-policy audit.** Standard is the fresh-install default; a previously saved mode stays selected. Map Quick, Standard, Thorough and Paranoid to copy-stream source hashing, independent source rereads, destination readback, byte comparison, source-change checks, ASC MHL/report evidence and the actual safe-to-erase verdict. Include a one-time transient source misread and a repeatably wrong read: a second read can catch differing bytes, but cannot prove the original camera bytes or detect every stable fault. Treat the recommended default as a product choice informed by safety, user workflows and measured time; it may move up or down the assurance ladder, but every verdict must name the checks actually performed and the selected-file scope. Audit the existing wording now rather than waiting for a benchmark. **Acceptance:** setup, completion, History, PDF/CSV/JSON, README and Guide state the same mode-specific limits; Quick and intentionally excluded-file transfers stay unsafe; injected source and destination faults produce honest outcomes. Any default change has explicit behavior for fresh versus saved preferences, release notes and Mac/iPad/iPhone validation. +- [ ] **Verification coverage and default-policy audit.** Standard is the fresh-install default; a previously saved mode stays selected. Setup, History and PDF/CSV/JSON wording have been aligned with recorded checks, and Compare has its own check-plan wording. Still map Quick, Standard, Thorough and Paranoid to copy-stream source hashing, independent source rereads, destination readback, byte comparison, source-change checks, ASC MHL/report evidence and the actual safe-to-erase verdict. Include a one-time transient source misread and a repeatably wrong read: a second read can catch differing bytes, but cannot prove the original camera bytes or detect every stable fault. Treat the recommended default as a product choice informed by safety, user workflows and measured time; it may move up or down the assurance ladder, but every verdict must name the checks actually performed and the selected-file scope. **Acceptance:** finish-screen, README and Guide mode claims need a final check against the engine and fault-injection tests; Quick and intentionally excluded-file transfers stay unsafe; injected source and destination faults produce honest outcomes. Any default change has explicit behavior for fresh versus saved preferences, release notes and Mac/iPad/iPhone validation. -- [ ] **Separate local-source and remote-destination assurance.** A local source can be independently re-read before a later SFTP upload; the current Mac SFTP queue uploads an already verified local backup, not the card. Its SHA-256 readback downloads the remote file, while upload-only remains **Uploaded · Unverified**. Review screen, report and diagnostic labels so a local source reread, local backup verification, remote upload and remote readback are distinct facts. Do not infer source assurance from a remote readback or deny source assurance merely because a later destination is remote. **Acceptance:** tests cover both SFTP modes, interruption and stale local evidence; no remote result becomes a whole-card safe verdict without the required local checks. Apply the same evidence separation to any future cloud provider, with Mac-only SFTP scope explicit on iPad/iPhone. +- [ ] **Separate local-source and remote-destination assurance.** A local source can be independently re-read before a later SFTP upload; the current Mac SFTP queue uploads an already verified local backup, not the card. Its SHA-256 readback downloads the remote file, while upload-only remains **Uploaded · Unverified**. The screen and report labels landed in #26; this branch adds a contract test that checks the downloaded bytes are hashed and a mismatch fails, without reading the card. Keep local source reread, local backup verification, remote upload and remote readback as distinct facts in diagnostics and future changes. Do not infer source assurance from a remote readback or deny source assurance merely because a later destination is remote. **Acceptance:** tests still need interruption and stale local evidence; no remote result becomes a whole-card safe verdict without the required local checks. Apply the same evidence separation to any future cloud provider, with Mac-only SFTP scope explicit on iPad/iPhone. - [ ] **Storage-aware verification scheduling.** Replace the CPU-derived default with scheduling informed by physical storage identity and measured behavior. Avoid competing large sequential reads on one HDD; permit useful concurrency across independent SSD/NVMe devices. Account for shared devices and buses only where reliably identifiable. Preserve diagnostic worker-count overrides and benchmark actual throughput and memory use on physical hardware, including the two-drive test below. **Acceptance:** independent destination readback remains complete, cancellation still stops the run, and unknown topology gets a conservative policy rather than invented independence. @@ -110,7 +112,7 @@ The priority buckets are not a strict implementation sequence: investigate P0 re - [ ] **One destination out of space.** Investigate pausing only that destination while the others finish, then revalidate space and identity before continuing. Today the failed destination is reported while others can finish. **Acceptance:** partial work remains visibly incomplete, cancellation works, and the overall verdict cannot become safe until every required backup verifies. -- [ ] **Finish terminal progress/outcome parity.** The current engine change withholds `.completed` and 100% when selected rows fail. Check every UI, accessibility, History and future remote presentation; a percentage alone is never a success verdict. **Acceptance:** synthetic source/destination disconnects during copy and verification, failed destinations and cancellation leave terminal stage, text, history and authoritative results in agreement; no incomplete transfer is presented as completed. +- [x] **Finish local terminal progress/outcome parity.** The engine withholds `.completed` and 100% when selected rows fail. This branch adds an active Finishing state while MHL/report/journal evidence settles, keeps cancelled and unexpectedly interrupted wording distinct, and refuses a safe History/Finish verdict for missing, duplicated or copied-but-unverified selected results. Mac/iPhone/iPad shared presentation and focused cancellation/terminal tests cover this local slice; a percentage alone is never a success verdict. Future remote presentation and the signed-app fault walkthrough remain separate release gates. - [ ] **Opt-in crash reports.** Investigate Apple's MetricKit crash/hang diagnostics, off by default, with no third-party service or automatic sending. Existing local diagnostic exports already record phases, cancellations, verification breadcrumbs and memory use; they cannot establish why a process disappeared. @@ -159,7 +161,7 @@ Remote scope excludes screen sharing, arbitrary filesystem browsing, remote shel - [ ] **Organize and rename.** Extend existing project naming and folder recipes with remaining naming-pattern and organize-by-date workflows. Preserve original card contents and test destination collisions before adding new options. - [ ] **Honest byte-weighted progress.** The engine currently counts finished file/stage units, and the queue bar averages cards, so a tiny sidecar or small card can move the percentage as much as a huge clip or card. Track measurable copy and required readback bytes per destination, including source rereads and ASC MHL destination reads when enabled; show an explicit finishing stage where remaining report work has no reliable byte total. Keep the main UI quiet and reserve 100% for the authoritative result. **Acceptance:** synthetic tiny-plus-huge files/cards, two destinations with different speeds, reuse/retry, optional MHL, failure and cancellation show monotonic, plausible progress without an early completed state. - [ ] **Speed for each drive.** Show which destination is slow instead of only an average; share measured-throughput instrumentation with the storage-scheduling work, without requiring a scheduler change to display it. Put per-destination copy/readback progress and measured throughput in expanded details, not another default setup control; distinguish a slow write from slow verification. -- [ ] **Recover missed Mac card events.** BitMatch already has mount notifications and manual rescan, but a missed-unmount/reinsert case is worth testing. First reproduce it with deterministic event gaps and card identity changes; add a low-rate reconciliation backstop only if the current monitor misses it. **Acceptance:** a reinserted card is discoverable without a restart, a different card at the same mount path is never mistaken for the old one, transient mount flaps do not duplicate cards, and stale discovery cannot authorize or auto-start a transfer. Mac-only discovery must ignore system and unselected network volumes. +- [x] **Reconcile missed Mac card events.** Mount notifications and manual rescan now reconcile the mounted filesystem's identity with published cards and drives. Deterministic event-gap tests cover unmount/reinsert, same-path replacement, stale asynchronous analysis and source selection; stale discovery cannot authorize auto-queue. This is a bounded event-triggered backstop, not a new polling service. A real removable-card walkthrough remains in the release gate below. - [ ] **Slow-cable hint.** On Mac, show a quiet hint when reliable connection information identifies a USB 2 bottleneck. Do not promise a speed multiplier without measurements. - [ ] **Time estimate before Start.** Use card size and recent measured speeds; keep “estimating” when evidence is insufficient. - [ ] **Phone notification with no account.** Optional completion/failure push through an open service such as ntfy. Make disclosure of card names/results to that service explicit. @@ -177,14 +179,14 @@ These remain ongoing gates, not optional features. Use disposable data and retai - [ ] **Physical hardware reports.** Ask users to fill in the [hardware report template](HARDWARE_REPORT_TEMPLATE.md) for their cameras, readers, drives and connections. Follow the [hardware testing procedure](HARDWARE_TESTING.md); disk-image tests do not establish cable, dock or controller reliability. - [ ] **End-to-end two-drive benchmark.** Start with a representative disposable card containing large clips and small sidecars, one source and two physical destinations on known connections, through `RealCopyBenchTests` and the signed app. Compare Standard and Thorough first; expand verification modes, APFS/exFAT/FAT and drive combinations only when evidence warrants it. Record total time to the final verdict plus source read, hash, destination write, destination readback, MHL and report time; preserve copy/verify overlap, per-destination throughput, peak memory, CPU use, and warm/cold-cache conditions rather than summing overlapping stages. Repeat enough runs to expose variance and keep failed runs. Feed results into the default-tier and storage-scheduling decisions. Remove only the owned `bench-card` fixture from the T9 after confirming its identity. Mac measurements do not stand in for later iPhone/iPad device checks. - [ ] **VoiceOver pass on real devices.** Preserve the remaining accessibility checks in the [thesis](THESIS.md). -- [ ] **Release walkthroughs.** For behavior changes, run relevant engine and Mac tests, build iPhone/iPad, and exercise the signed app's affected workflow. On Mac, check that selected removable and NAS roots request needed access before Start where possible; if access lapses mid-copy, report an interruption clearly rather than silently stalling. Do not probe unselected volumes. Keep a focused regression for root `.fseventsd` appearing or disappearing during enumeration while nested user files remain included; static root exclusions are already tested. Record physical-device, simulator and synthetic coverage separately; do not close user reports on a synthetic pass alone. +- [ ] **Release walkthroughs.** For behavior changes, run relevant engine and Mac tests, build iPhone/iPad, and exercise the signed app's affected workflow, including a real removable-card unmount/reinsert and narrow-screen assurance wording. On Mac, check that selected removable and NAS roots request needed access before Start where possible; if access lapses mid-copy, report an interruption clearly rather than silently stalling. Do not probe unselected volumes. `CardSourceTests` covers root `.fseventsd` appearing and disappearing during enumeration while nested user files remain included; the physical-card check is still open. Record physical-device, simulator and synthetic coverage separately; do not close user reports on a synthetic pass alone. - [ ] **iPhone/iPad distribution.** TestFlight, then the App Store, so users need not build from source. - [ ] **Real screenshots.** Replace demo queue imagery with real-card workflows when available, after removing private production details. ## Developer chores - [ ] **Tests without permission prompts.** Investigate drive-access prompts that stall Mac tests; prefer owned temporary fixtures and scoped test configuration without weakening release entitlements. The documented DerivedData-outside-Desktop workaround addresses a separate loader stall. -- [ ] **Buffer benchmark first run.** Avoid measuring freshly written files still in cache; evict/rewrite before timing or discard a warm-up run, and verify that the measured read actually reaches storage. +- [ ] **Buffer benchmark first run.** Source creation and timed reads now request `F_NOCACHE`, including the first run. That request does not prove physical I/O: confirm drive read counters or cold/warm behavior on the selected hardware before using the numbers for a buffer or verification-policy decision. - [x] **Reverse flight on Edit.** Mac Edit carries the queued card back to setup, respecting Reduce Motion. Compact mobile Edit dismisses the queue sheet. Settings, operator and notes remain snapshotted. ## Out of scope diff --git a/docs/VERIFICATION_ASSURANCE_AUDIT.md b/docs/VERIFICATION_ASSURANCE_AUDIT.md index 05f1da01..9d1ec0f6 100644 --- a/docs/VERIFICATION_ASSURANCE_AUDIT.md +++ b/docs/VERIFICATION_ASSURANCE_AUDIT.md @@ -42,7 +42,7 @@ The source audit found two presentation/report issues addressed in this batch: README and Guide now explain Standard's read scope and the limits of repeated source reads. This does **not** complete the TODO acceptance criterion for all surfaces. Continue on Mac with: -- Setup has both `VerificationMode.description` and a separate short mode description in `SetupLocationsView`. Align them without obscuring warnings or crowding narrow screens. +- Setup now takes its compact description from `VerificationMode`, and Mac/iPhone/iPad settings use the same full description. Compare uses its own check plan for mode details rather than copy-mode wording. The iPhone/iPad introduction no longer implies that Quick checks file contents. Review the compact wording on narrow devices during the next signed-app walkthrough. - Outcome and History show mode/digest labels, but do not yet communicate all mode-specific source-read limits. - PDF/CSV/JSON primarily record method/algorithm. Add explicit scope and limits without implying that an enabled check completed in an interrupted attempt. Preserve older evidence compatibility and selected-file exclusions. - `EvidenceReader.verificationMode` currently infers modes from legacy method/algorithm strings. An MD5 algorithm alone is not proof of a Thorough source reread. Plan explicit recorded-mode/scope metadata and conservative legacy presentation rather than inventing source assurance.