From 22ea502d2cb9dd9a7cfc0979338230f4c4b07bd1 Mon Sep 17 00:00:00 2001 From: rzen Date: Sat, 4 Jul 2026 09:44:53 -0400 Subject: [PATCH] Make reads eviction-safe: download offloaded iCloud files instead of losing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the previously-dead ensureDownloaded into every read (it now polls, bounded 30s, instead of fire-and-forget) and fixes the deeper hole: the directory listing skipped hidden files, so evicted records — whose only local trace is a hidden ..json.icloud placeholder — never appeared in reconcile at all and their cache entities were pruned as deleted. Also: reconcile no longer prunes entities whose file failed to read this pass (an eviction-download timeout is not a deletion); tombstone IDs come from stub filenames so an evicted stub still vetoes resurrection; the stale-file stub check is placeholder-aware; read failures log loudly instead of try?-skipping. Verified with a harness compiling the real ICloudFileManager against a local temp directory (placeholder mapping, dedup, tombstone derivation, soft-delete round trip). Real eviction/download behavior needs a device pass. Claude-Session: https://claude.ai/code/session_01SY5jsAUf4qoPxSvv8xAqXS --- CHANGELOG.md | 4 ++ Workouts/Sync/ICloudFileManager.swift | 88 +++++++++++++++++++++++---- Workouts/Sync/SyncEngine.swift | 58 +++++++++++++++--- 3 files changed, 129 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21d183c..92e5560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +**July 2026** + +Fixed workouts and splits vanishing from the app when iPhone storage ran low: when iOS offloads their files to iCloud to free up space, Workouts now downloads them back automatically instead of treating them as gone. + **June 2026** Starting a new workout while another is still going now asks whether to end the current one first or run both in parallel. diff --git a/Workouts/Sync/ICloudFileManager.swift b/Workouts/Sync/ICloudFileManager.swift index 182517b..7a993cd 100644 --- a/Workouts/Sync/ICloudFileManager.swift +++ b/Workouts/Sync/ICloudFileManager.swift @@ -34,7 +34,12 @@ actor ICloudFileManager { if let error = coordError ?? writeError { throw error } } - func read(relativePath: String) throws -> Data { + /// Eviction-safe read: materializes an evicted file (bounded wait) before the + /// coordinated read, so callers never decode a dataless placeholder. Throws on + /// download timeout rather than skipping — a silently dropped evicted file + /// looks to the user like the record was deleted. + func read(relativePath: String) async throws -> Data { + try await ensureDownloaded(relativePath: relativePath) let fileURL = documentsURL.appendingPathComponent(relativePath) var coordError: NSError? var result: Result? @@ -83,36 +88,93 @@ actor ICloudFileManager { listJSON().filter { !$0.hasPrefix("Stubs/") } } - /// All tombstones currently on disk. - func listTombstones() -> [Tombstone] { - listJSON().filter { $0.hasPrefix("Stubs/") }.compactMap { path in - guard let data = try? read(relativePath: path) else { return nil } - return try? DocumentCoder.decoder.decode(Tombstone.self, from: data) + /// All tombstones currently on disk (evicted stubs are downloaded first — + /// old stubs are routinely evicted, and cleanup needs their `deletedAt`). + func listTombstones() async -> [Tombstone] { + var tombstones: [Tombstone] = [] + for path in listJSON() where path.hasPrefix("Stubs/") { + guard let data = try? await read(relativePath: path) else { continue } + if let tombstone = try? DocumentCoder.decoder.decode(Tombstone.self, from: data) { + tombstones.append(tombstone) + } } + return tombstones } + /// IDs of all tombstones, derived from stub filenames alone. Unlike + /// `listTombstones()` this needs no reads, so an evicted stub still counts — + /// reconcile must never treat a record as live because its stub happened to + /// be evicted. + func listTombstoneIDs() -> Set { + Set(listJSON().filter { $0.hasPrefix("Stubs/") } + .map { ($0 as NSString).lastPathComponent.replacingOccurrences(of: ".json", with: "") }) + } + + /// Whether a deletion stub exists for an id — placeholder-aware, so an + /// evicted stub still vetoes a resurrecting live file. + func stubExists(id: String) -> Bool { + let stubs = documentsURL.appendingPathComponent("Stubs", isDirectory: true) + return FileManager.default.fileExists(atPath: stubs.appendingPathComponent("\(id).json").path) + || FileManager.default.fileExists(atPath: stubs.appendingPathComponent(".\(id).json.icloud").path) + } + + /// Deliberately does NOT pass `.skipsHiddenFiles`: iOS materializes evicted + /// iCloud files as hidden placeholder dotfiles (`..json.icloud`), and + /// skipping hidden files would drop every evicted record from the listing — + /// reconcile would then prune their cache entities as "file gone". + /// Placeholder names are mapped back to their real `.json`. private func listJSON() -> [String] { let base = documentsURL.path + "/" guard let enumerator = FileManager.default.enumerator( at: documentsURL, includingPropertiesForKeys: nil, - options: [.skipsHiddenFiles] + options: [] ) else { return [] } + var seen: Set = [] var paths: [String] = [] - for case let url as URL in enumerator where url.pathExtension == "json" { + for case let url as URL in enumerator { let full = url.path - if full.hasPrefix(base) { paths.append(String(full.dropFirst(base.count))) } + guard full.hasPrefix(base) else { continue } + guard let real = Self.realRelativePath(fromRaw: String(full.dropFirst(base.count))), + seen.insert(real).inserted else { continue } + paths.append(real) } return paths } + /// Map a raw relative path from the enumerator — which may be an eviction + /// placeholder like `Workouts/2026/03/..json.icloud` — to the real + /// path `Workouts/2026/03/.json`. Returns nil for non-JSON entries. + nonisolated static func realRelativePath(fromRaw raw: String) -> String? { + var components = raw.split(separator: "/", omittingEmptySubsequences: true).map(String.init) + guard var name = components.popLast() else { return nil } + if name.hasPrefix("."), name.hasSuffix(".icloud") { + name = String(name.dropFirst().dropLast(".icloud".count)) + } + guard name.hasSuffix(".json") else { return nil } + components.append(name) + return components.joined(separator: "/") + } + // MARK: - Eviction /// Triggers a download for an evicted file and polls until it materializes. - func ensureDownloaded(relativePath: String) { + /// `startDownloadingUbiquitousItem` is fire-and-forget with no completion + /// callback, so a bounded poll (30s) is the only way to wait; on timeout this + /// throws rather than letting the caller read a placeholder. + func ensureDownloaded(relativePath: String) async throws { let fileURL = documentsURL.appendingPathComponent(relativePath) - guard let values = try? fileURL.resourceValues(forKeys: [.ubiquitousItemDownloadingStatusKey]), - let status = values.ubiquitousItemDownloadingStatus, status != .current else { return } - try? FileManager.default.startDownloadingUbiquitousItem(at: fileURL) + let values = try fileURL.resourceValues(forKeys: [.ubiquitousItemDownloadingStatusKey]) + guard let status = values.ubiquitousItemDownloadingStatus, status != .current else { return } + + try FileManager.default.startDownloadingUbiquitousItem(at: fileURL) + for _ in 0..<60 { + try await Task.sleep(for: .milliseconds(500)) + let updated = try fileURL.resourceValues(forKeys: [.ubiquitousItemDownloadingStatusKey]) + if let s = updated.ubiquitousItemDownloadingStatus, s == .current { return } + } + throw CocoaError(.fileReadNoPermission, userInfo: [ + NSLocalizedDescriptionKey: "Timed out downloading \(relativePath) from iCloud" + ]) } } diff --git a/Workouts/Sync/SyncEngine.swift b/Workouts/Sync/SyncEngine.swift index 07ebf5c..8b06066 100644 --- a/Workouts/Sync/SyncEngine.swift +++ b/Workouts/Sync/SyncEngine.swift @@ -24,6 +24,14 @@ final class SyncEngine { /// this to push fresh state to the watch. var onCacheChanged: (() -> Void)? + /// Called when a completed workout is persisted. The HealthKit writer uses this to + /// schedule an estimated Health workout if nothing recorded it. + var onWorkoutCompleted: ((WorkoutDocument) -> Void)? + + /// Called when a workout carrying watch-recorded metrics is persisted, so the + /// writer can cancel any pending phone estimate for it. + var onWatchMetricsArrived: ((String) -> Void)? + private let log = Logger(subsystem: "dev.rzen.indie.Workouts", category: "sync") private let modelContainer: ModelContainer private var fileManager: ICloudFileManager? @@ -193,6 +201,10 @@ final class SyncEngine { guard let fm = fileManager else { return } do { try await fm.write(try DocumentCoder.encoder.encode(doc), to: doc.relativePath) } catch { print("[Sync] write failed for \(doc.relativePath): \(error)") } + // Drive the HealthKit writer: a watch-recorded doc cancels any pending estimate; + // a completed doc (re)considers an estimate (the writer dedupes on metrics). + if doc.metrics?.source == .watch { onWatchMetricsArrived?(doc.id) } + if doc.status == WorkoutStatus.completed.rawValue { onWorkoutCompleted?(doc) } } func delete(split: Split) async { @@ -217,15 +229,24 @@ final class SyncEngine { private func importFile(relativePath: String) async { guard let fm = fileManager else { return } - guard let data = try? await fm.read(relativePath: relativePath) else { return } + let data: Data + do { + data = try await fm.read(relativePath: relativePath) + } catch { + // Includes eviction-download timeouts — log loudly, never silently + // treat an unreadable file as absent. The next monitor event or + // reconcile retries it. + log.error("import: read failed for \(relativePath, privacy: .public): \(error)") + return + } if relativePath.hasPrefix("Splits/") { guard let doc = try? DocumentCoder.decoder.decode(SplitDocument.self, from: data), doc.isReadable else { return } - if await fm.fileExists("Stubs/\(doc.id).json") { try? await fm.remove(relativePath: relativePath); return } + if await fm.stubExists(id: doc.id) { try? await fm.remove(relativePath: relativePath); return } CacheMapper.upsertSplit(doc, relativePath: relativePath, into: context) } else if relativePath.hasPrefix("Workouts/") { guard let doc = try? DocumentCoder.decoder.decode(WorkoutDocument.self, from: data), doc.isReadable else { return } - if await fm.fileExists("Stubs/\(doc.id).json") { try? await fm.remove(relativePath: relativePath); return } + if await fm.stubExists(id: doc.id) { try? await fm.remove(relativePath: relativePath); return } CacheMapper.upsertWorkout(doc, relativePath: relativePath, into: context) } } @@ -238,14 +259,29 @@ final class SyncEngine { isSyncing = true defer { isSyncing = false } - let tombstoned = Set(await fm.listTombstones().map(\.id)) + // IDs from stub filenames, not stub contents — an evicted stub must + // still count as a tombstone or the deleted record resurrects. + let tombstoned = await fm.listTombstoneIDs() let dataFiles = await fm.listDataFiles() var liveSplitIDs = Set() var liveWorkoutIDs = Set() + var unreadablePaths = Set() for path in dataFiles { - guard let data = try? await fm.read(relativePath: path) else { continue } + let data: Data + do { + data = try await fm.read(relativePath: path) + } catch { + // A failed read (eviction-download timeout, coordination error) + // is not proof the record is gone — remember the path so the + // prune below keeps its cache entity. Skipping silently here is + // how evicted records used to vanish on storage-constrained + // devices. + log.error("reconcile: read failed for \(path, privacy: .public): \(error)") + unreadablePaths.insert(path) + continue + } if path.hasPrefix("Splits/") { guard let doc = try? DocumentCoder.decoder.decode(SplitDocument.self, from: data), doc.isReadable else { continue } if tombstoned.contains(doc.id) { try? await fm.remove(relativePath: path); continue } @@ -259,12 +295,18 @@ final class SyncEngine { } } - // Prune cache entities no longer backed by a live file. + // Prune cache entities no longer backed by a live file — but never for + // a path that failed to read this pass (it may just be un-downloadable + // right now; pruning would make an eviction look like a deletion). if let splits = try? context.fetch(FetchDescriptor()) { - for s in splits where !liveSplitIDs.contains(s.id) { context.delete(s) } + for s in splits where !liveSplitIDs.contains(s.id) && !unreadablePaths.contains(s.jsonRelativePath) { + context.delete(s) + } } if let workouts = try? context.fetch(FetchDescriptor()) { - for w in workouts where !liveWorkoutIDs.contains(w.id) { context.delete(w) } + for w in workouts where !liveWorkoutIDs.contains(w.id) && !unreadablePaths.contains(w.jsonRelativePath) { + context.delete(w) + } } try? context.save() onCacheChanged?()