diff --git a/Sources/CodexBarCore/TelemetryFolderMigration.swift b/Sources/CodexBarCore/TelemetryFolderMigration.swift index b5cca44..362fc2e 100644 --- a/Sources/CodexBarCore/TelemetryFolderMigration.swift +++ b/Sources/CodexBarCore/TelemetryFolderMigration.swift @@ -27,8 +27,12 @@ public struct TelemetryFolderMigration { guard let stage = Self.stage(of: entry) else { continue } do { let values = try entry.resourceValues(forKeys: [.isRegularFileKey, .isSymbolicLinkKey]) - guard values.isRegularFile == true, values.isSymbolicLink != true else { continue } - if stage == .migrated { + guard values.isRegularFile == true, values.isSymbolicLink != true else { + // removeItem would follow a directory into its contents, so only regular files are ever touched. + if stage != .live { warnings.append("Left \(entry.lastPathComponent) alone in the old telemetry folder: it isn't a regular file") } + continue + } + if stage == .migrated || stage == .mergingDone { try fileManager.removeItem(at: entry) continue } @@ -62,7 +66,7 @@ public struct TelemetryFolderMigration { return true } return entries.contains { entry in - guard let stage = Self.stage(of: entry), stage != .migrated, + guard let stage = Self.stage(of: entry), stage == .live || stage == .merging, let values = try? entry.resourceValues(forKeys: [.isRegularFileKey, .isSymbolicLinkKey]) else { return false } return values.isRegularFile == true && values.isSymbolicLink != true } @@ -116,8 +120,20 @@ public struct TelemetryFolderMigration { try? handle.truncate(atOffset: size) throw error } - let migrated = merging.deletingPathExtension().appendingPathExtension(Self.migratedExtension) - try fileManager.moveItem(at: merging, to: migrated) + // The lines are in the target now, so from here the source must never go back to a name that gets appended again. + let base = merging.deletingPathExtension() + let migrated = base.appendingPathExtension(Self.migratedExtension) + do { + try fileManager.moveItem(at: merging, to: migrated) + } catch { + do { + try fileManager.removeItem(at: merging) + } catch { + try fileManager.moveItem(at: merging, to: base.appendingPathExtension(Self.mergingDoneExtension)) + throw error + } + return + } try fileManager.removeItem(at: migrated) } @@ -134,13 +150,17 @@ public struct TelemetryFolderMigration { private static let mergingExtension = "merging" private static let migratedExtension = "migrated" - private enum Stage { case live, merging, migrated } + private static let mergingDoneExtension = "merging-done" + + /// `merging` and `mergingDone` are both leftovers, but only `merging` still needs its lines appended. + private enum Stage { case live, merging, mergingDone, migrated } private static func stage(of url: URL) -> Stage? { if isTelemetryFile(url) { return .live } guard isTelemetryFile(url.deletingPathExtension()) else { return nil } switch url.pathExtension { case mergingExtension: return .merging + case mergingDoneExtension: return .mergingDone case migratedExtension: return .migrated default: return nil } diff --git a/Tests/CodexBarCoreTests/TelemetryFolderMigrationTests.swift b/Tests/CodexBarCoreTests/TelemetryFolderMigrationTests.swift index fc3f88c..b2f209f 100644 --- a/Tests/CodexBarCoreTests/TelemetryFolderMigrationTests.swift +++ b/Tests/CodexBarCoreTests/TelemetryFolderMigrationTests.swift @@ -15,6 +15,25 @@ private final class UnreadableFileManager: FileManager, @unchecked Sendable { } } +/// Fails the rename that marks an append as done, and optionally the delete that would follow it. +private final class UnmarkableFileManager: FileManager, @unchecked Sendable { + let failsDeletes: Bool + init(failsDeletes: Bool) { + self.failsDeletes = failsDeletes + super.init() + } + override func moveItem(at srcURL: URL, to dstURL: URL) throws { + if dstURL.pathExtension == "migrated" { + throw CocoaError(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: dstURL.path]) + } + try super.moveItem(at: srcURL, to: dstURL) + } + override func removeItem(at URL: URL) throws { + if failsDeletes { throw CocoaError(.fileWriteNoPermission, userInfo: [NSFilePathErrorKey: URL.path]) } + try super.removeItem(at: URL) + } +} + final class TelemetryFolderMigrationTests: XCTestCase { private var home: URL! private var legacy: URL! @@ -179,6 +198,51 @@ final class TelemetryFolderMigrationTests: XCTestCase { XCTAssertFalse(FileManager.default.fileExists(atPath: legacy.path)) } + func testDirectoriesAndLinksNamedLikeLeftoversAreReportedAndNeverRemoved() throws { + let name = "codexbar-metrics-2026-10-11.jsonl" + try write("inside.txt", "keep", in: legacy.appending(path: "\(name).migrated", directoryHint: .isDirectory)) + let outside = home.appending(path: "outside.jsonl") + try Data("secret\n".utf8).write(to: outside) + try FileManager.default.createSymbolicLink(at: legacy.appending(path: "\(name).merging"), withDestinationURL: outside) + let migration = TelemetryFolderMigration(home: home) + + XCTAssertFalse(migration.hasPendingFiles()) + XCTAssertEqual(migration.run().count, 2) + + XCTAssertEqual(try read("inside.txt", in: legacy.appending(path: "\(name).migrated", directoryHint: .isDirectory)), "keep") + XCTAssertNotNil(try? FileManager.default.destinationOfSymbolicLink(atPath: legacy.appending(path: "\(name).merging").path)) + XCTAssertEqual(try String(contentsOf: outside, encoding: .utf8), "secret\n") + XCTAssertFalse(FileManager.default.fileExists(atPath: destination.path)) + } + + func testFailedMigratedRenameAfterAppendFallsBackToDeletingTheSource() throws { + let name = "codexbar-metrics-2026-10-11.jsonl" + try write(name, "new\n", in: destination) + try write(name, "old\n", in: legacy) + + XCTAssertEqual(TelemetryFolderMigration(home: home, fileManager: UnmarkableFileManager(failsDeletes: false)).run(), []) + _ = TelemetryFolderMigration(home: home).run() + + XCTAssertEqual(try read(name, in: destination), "new\nold\n") + XCTAssertFalse(FileManager.default.fileExists(atPath: legacy.path)) + } + + func testSourceThatCantBeMarkedOrDeletedAfterAppendIsNeverAppendedAgain() throws { + let name = "codexbar-metrics-2026-10-11.jsonl" + try write(name, "new\n", in: destination) + try write(name, "old\n", in: legacy) + + let stuck = TelemetryFolderMigration(home: home, fileManager: UnmarkableFileManager(failsDeletes: true)) + XCTAssertEqual(stuck.run().count, 1) + XCTAssertFalse(stuck.hasPendingFiles()) + XCTAssertTrue(FileManager.default.fileExists(atPath: legacy.appending(path: "\(name).merging-done").path)) + + XCTAssertEqual(TelemetryFolderMigration(home: home).run(), []) + + XCTAssertEqual(try read(name, in: destination), "new\nold\n") + XCTAssertFalse(FileManager.default.fileExists(atPath: legacy.path)) + } + func testLeftoverMigratedFileIsOnlyDeleted() throws { let name = "codexbar-metrics-2026-10-11.jsonl" try write(name, "new\n", in: destination)