diff --git a/BitMatchTests/RemoteBackupCoordinatorTests.swift b/BitMatchTests/RemoteBackupCoordinatorTests.swift index 75832120..29edca75 100644 --- a/BitMatchTests/RemoteBackupCoordinatorTests.swift +++ b/BitMatchTests/RemoteBackupCoordinatorTests.swift @@ -63,6 +63,38 @@ struct RemoteBackupCoordinatorTests { } } + @Test func sameSizeLocalFileChangeInvalidatesSavedRemoteManifest() async throws { + let package = FileManager.default.temporaryDirectory.appendingPathComponent("remote-stale-\(UUID())") + let file = package.appendingPathComponent("DCIM/A.ARW") + try FileManager.default.createDirectory(at: file.deletingLastPathComponent(), withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: package) } + try Data("ONE".utf8).write(to: file) + let originalDigest = try await ChecksumEngine.shared.generateChecksum(for: file, type: .sha256) + let fixture = try CoordinatorFixture( + checksum: originalDigest, + fileSize: { url in + let values = try url.resourceValues(forKeys: [.fileSizeKey, .isRegularFileKey]) + guard values.isRegularFile == true, let size = values.fileSize else { + throw RemoteBackupError.localArtifactNotVerified + } + return Int64(size) + }, + sha256: { url in try await ChecksumEngine.shared.generateChecksum(for: url, type: .sha256) } + ) + let item = try #require(try fixture.coordinator.queueRemoteBackup( + for: fixture.card.id, in: fixture.job.id, results: fixture.rows + ).first) + try fixture.bookmarks.save(Data(package.path.utf8), for: item.localArtifactBookmarkReference) + + let validLease = try await fixture.coordinator.resolveLocalArtifact(for: item) + validLease.release() + try Data("TWO".utf8).write(to: file) + #expect(try Data(contentsOf: file).count == 3) + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { + _ = try await fixture.coordinator.resolveLocalArtifact(for: item) + } + } + @Test func resolverScopesAccessUntilItsLeaseIsReleased() async throws { let fixture = try CoordinatorFixture() let item = try #require(try fixture.coordinator.queueRemoteBackup(for: fixture.card.id, in: fixture.job.id, results: fixture.rows).first) @@ -177,7 +209,9 @@ private final class CoordinatorFixture { init( localState: PhotographerLocalState = .locallySafe, checksum: String = String(repeating: "a", count: 64), - fileCount: Int = 1 + fileCount: Int = 1, + fileSize: RemoteBackupCoordinator.FileSize? = nil, + sha256: RemoteBackupCoordinator.SHA256? = nil ) throws { let sourceCard = URL(fileURLWithPath: "/Volumes/CARD") let destinationPackage = URL(fileURLWithPath: "/Volumes/Archive/Job/Card-001") @@ -227,8 +261,8 @@ private final class CoordinatorFixture { bookmarkStore: bookmarks, makeBookmark: { Data($0.path.utf8) }, resolveBookmark: { URL(fileURLWithPath: String(decoding: $0, as: UTF8.self)) }, - fileSize: { _ in 3 }, - sha256: { _ in checksumBox.value }, + fileSize: fileSize ?? { _ in 3 }, + sha256: sha256 ?? { _ in checksumBox.value }, startSecurityScope: { [scopeCounter] _ in scopeCounter.recordStart() }, stopSecurityScope: { [scopeCounter] _ in scopeCounter.recordStop() }, now: { Date(timeIntervalSince1970: 3) } diff --git a/BitMatchTests/RemoteBackupQueueTests.swift b/BitMatchTests/RemoteBackupQueueTests.swift index 743cc340..b405154b 100644 --- a/BitMatchTests/RemoteBackupQueueTests.swift +++ b/BitMatchTests/RemoteBackupQueueTests.swift @@ -35,6 +35,61 @@ struct RemoteBackupQueueTests { #expect(await provider.promoteCallCount == 1) } + @Test func staleLocalArtifactAfterPromotionParksWithoutClaimingRemoteVerification() async throws { + let fixture = try QueueFixture(verificationMode: .sha256) + let provider = FakeRemoteBackupProvider(verificationErrors: [.networkUnavailable]) + let firstQueue = fixture.makeQueue(provider: provider) + try await firstQueue.enqueue(fixture.item) + await firstQueue.run(fixture.item.id) + + #expect(await firstQueue.item(id: fixture.item.id)?.state == .verifying) + #expect(await provider.promoteCallCount == 1) + + let staleQueue = fixture.makeQueue( + provider: provider, + resolver: { _ in throw RemoteBackupError.localArtifactNotVerified }, + now: Date(timeIntervalSince1970: 200) + ) + try await staleQueue.restore() + await staleQueue.run(fixture.item.id) + + let parked = try #require(await staleQueue.item(id: fixture.item.id)) + #expect(parked.state == .paused) + #expect(parked.promotionIntent) + #expect(parked.verificationEvidence == .none) + #expect(parked.errorSummary == RemoteBackupError.localArtifactNotVerified.errorDescription) + #expect(await staleQueue.runnableIDs().isEmpty) + #expect(await fixture.persistence.queueItem(id: fixture.item.id)?.state == .paused) + #expect(await provider.uploadCallCount == 1) + #expect(await provider.promoteCallCount == 1) + + let recoveredQueue = fixture.makeQueue(provider: provider, now: Date(timeIntervalSince1970: 300)) + try await recoveredQueue.restore() + try await recoveredQueue.retry(fixture.item.id) + await recoveredQueue.run(fixture.item.id) + #expect(await recoveredQueue.item(id: fixture.item.id)?.state == .verified) + #expect(await provider.uploadCallCount == 1) + #expect(await provider.promoteCallCount == 1) + } + + @Test func localArtifactChangingAfterPreflightStopsBeforeUpload() async throws { + let fixture = try QueueFixture(verificationMode: .sha256) + let provider = FakeRemoteBackupProvider() + let resolver = StaleAfterPreflightResolver() + let queue = fixture.makeQueue(provider: provider, resolver: { item in + try await resolver.resolve(item) + }) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + #expect(await resolver.callCount == 2) + #expect(await provider.preflightCallCount == 1) + #expect(await provider.uploadCallCount == 0) + #expect(await queue.item(id: fixture.item.id)?.state == .paused) + #expect(await fixture.persistence.queueItem(id: fixture.item.id)?.verificationEvidence == RemoteVerificationEvidence.none) + } + @Test func interruptedUploadResumesOnlyWhenStoredAndServerOffsetsMatch() async throws { let fixture = try QueueFixture(uploadedByteCount: 4) let provider = FakeRemoteBackupProvider(objects: [fixture.item.temporaryRemoteRelativePath: .init(byteCount: 4)]) @@ -450,6 +505,46 @@ struct RemoteBackupQueueTests { #expect(await provider.promoteCallCount == 0) } + @Test func interruptedRunnerCannotPromoteOrRecordRemoteSuccess() async throws { + let fixture = try QueueFixture(verificationMode: .sha256) + let provider = FakeRemoteBackupProvider(blockUpload: true) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + let running = Task { await queue.run(fixture.item.id) } + await provider.waitForUploadStart() + running.cancel() + // Model a provider operation that finishes despite task cancellation. + await provider.resumeUpload() + await running.value + + #expect(await provider.promoteCallCount == 0) + let saved = try #require(await fixture.persistence.queueItem(id: fixture.item.id)) + #expect(saved.state == .paused) + #expect(saved.verificationEvidence == .none) + #expect(saved.errorSummary == RemoteBackupError.interrupted.errorDescription) + #expect(RemoteBackupStatusPresentation.make(state: saved.state, evidence: saved.verificationEvidence).isFullyBackedUp == false) + } + + @Test func concreteProviderFailureWinsWhenWorkerIsAlsoCancelled() async throws { + let fixture = try QueueFixture(verificationMode: .sha256) + let provider = FakeRemoteBackupProvider(uploadError: .verificationFailed, blockUpload: true) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + let running = Task { await queue.run(fixture.item.id) } + await provider.waitForUploadStart() + running.cancel() + await provider.resumeUpload() + await running.value + + let saved = try #require(await fixture.persistence.queueItem(id: fixture.item.id)) + #expect(saved.state == .failed) + #expect(saved.errorSummary == RemoteBackupError.verificationFailed.errorDescription) + #expect(saved.verificationEvidence == .none) + #expect(await provider.promoteCallCount == 0) + } + @Test func pausingDuringPromotionRetainsIntentForSafeRetry() async throws { let fixture = try QueueFixture(verificationMode: .sha256) let provider = FakeRemoteBackupProvider(blockPromotion: true) @@ -598,6 +693,17 @@ private final class QueueFactoryCallCounter: @unchecked Sendable { func record() { lock.withLock { value += 1 } } } +private actor StaleAfterPreflightResolver { + private var calls = 0 + var callCount: Int { calls } + + func resolve(_: RemoteQueueItem) throws -> RemoteBackupArtifactLease { + calls += 1 + guard calls == 1 else { throw RemoteBackupError.localArtifactNotVerified } + return RemoteBackupArtifactLease(url: URL(fileURLWithPath: "/tmp/Card-001.mov"), releaseAccess: {}) + } +} + private struct QueueFixture { let profile: RemoteDestinationProfile let manifest: RemoteManifest diff --git a/Shared/Core/Services/RemoteBackupProvider.swift b/Shared/Core/Services/RemoteBackupProvider.swift index 4c7c5ec3..eae56d2a 100644 --- a/Shared/Core/Services/RemoteBackupProvider.swift +++ b/Shared/Core/Services/RemoteBackupProvider.swift @@ -58,6 +58,7 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { case resumeOffsetMismatch(local: Int64, remote: Int64) case temporaryCleanupFailed case verificationFailed + case interrupted case cancelled var isTransientNetworkFault: Bool { @@ -113,6 +114,8 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { return "The remote temporary upload could not be safely discarded; retry when the destination is reachable." case .verificationFailed: return "Remote backup verification did not produce matching SHA-256 evidence." + case .interrupted: + return "The off-site backup was interrupted before it finished. Retry to continue." case .cancelled: return "The remote backup was cancelled." } diff --git a/Shared/Core/Services/RemoteBackupQueue.swift b/Shared/Core/Services/RemoteBackupQueue.swift index 825eace8..1b5c498d 100644 --- a/Shared/Core/Services/RemoteBackupQueue.swift +++ b/Shared/Core/Services/RemoteBackupQueue.swift @@ -151,7 +151,7 @@ actor RemoteBackupQueue { } func run(_ id: UUID) async { - guard !runningItemIDs.contains(id), isRunnable(id) else { return } + guard !Task.isCancelled, !runningItemIDs.contains(id), isRunnable(id) else { return } let runGeneration = runGenerations[id] ?? 0 runningItemIDs.insert(id) defer { runningItemIDs.remove(id) } @@ -389,6 +389,7 @@ actor RemoteBackupQueue { queuedItem.errorSummary = nil }) else { return } + try Task.checkCancellation() try await provider.upload( local: localURL, toTemporary: item.temporaryRemoteRelativePath, @@ -433,6 +434,7 @@ actor RemoteBackupQueue { provider: any RemoteBackupProvider ) async throws { guard let item = items[itemID], isRunnable(itemID, runGeneration: runGeneration) else { return } + try Task.checkCancellation() try await provider.promoteNoReplace( temporary: item.temporaryRemoteRelativePath, final: item.remoteRelativePath @@ -495,6 +497,7 @@ actor RemoteBackupQueue { return } + try Task.checkCancellation() let evidence = try await provider.verificationEvidence( for: items[itemID]?.remoteRelativePath ?? manifestEntry.relativePath, expectedSHA256: manifestEntry.sha256 @@ -581,10 +584,22 @@ actor RemoteBackupQueue { let item = items[itemID], !item.state.isTerminal, item.state != .paused else { return } - let backupError = (error as? RemoteBackupError) ?? .providerUnavailable + // A cancelled worker is an interruption, not a user cancellation. The + // explicit cancel action persists `.cancelled` and invalidates this run. + // Keep a concrete provider/integrity failure even if cancellation + // arrives while that error is propagating. Only a cancelled worker + // without a more specific failure is an interruption. + let backupError: RemoteBackupError + if let providerError = error as? RemoteBackupError { + backupError = providerError + } else if error is CancellationError || Task.isCancelled { + backupError = .interrupted + } else { + backupError = .providerUnavailable + } do { - if backupError.isTransientNetworkFault { + if backupError.isTransientNetworkFault && !Task.isCancelled { let retryCount = item.retryCount + 1 if retryCount > Self.maxRetryCount { _ = try await transition(itemID: itemID) { queuedItem in @@ -606,7 +621,7 @@ actor RemoteBackupQueue { } } } else { - _ = try await transition(itemID: itemID) { queuedItem in + _ = try await transition(itemID: itemID, allowingTaskCancellation: true) { queuedItem in // Keep the promotion-intent marker only for recoverable // verification/promotion interruptions. Invalid persisted // linkage, a digest mismatch, conflict, or resume @@ -614,6 +629,10 @@ actor RemoteBackupQueue { if item.state != .verifying || clearsPromotionIntent(backupError) { queuedItem.state = backupError.failClosedState queuedItem.promotionIntent = false + } else { + // Preserve recovery of an owned final, but stop + // automatic retries until local evidence is valid. + queuedItem.state = .paused } queuedItem.nextAttemptAt = nil queuedItem.errorSummary = backupError.errorDescription @@ -645,12 +664,14 @@ actor RemoteBackupQueue { private func transition( itemID: UUID, allowingCancellation: Bool = false, + allowingTaskCancellation: Bool = false, _ change: (inout RemoteQueueItem) -> Void ) async throws -> Bool { guard var updatedItem = deferredWrites[itemID]?.item ?? items[itemID], allowingCancellation || !cancellingItemIDs.contains(itemID) else { return false } + if !allowingCancellation && !allowingTaskCancellation { try Task.checkCancellation() } change(&updatedItem) updatedItem.updatedAt = now() let generation = nextMutationGeneration(for: itemID)