commit 77d9917f75cf1e9cacfcabecce8dcd8311468525
parent 31aedea53c1694c69a4b2a259c6c849ff47007fe
Author: Michael Camilleri <[email protected]>
Date: Thu, 2 Jul 2026 09:36:13 +0900
Clean up archive asset temp files
This commit gives the archive write path explicit ownership of the
temporary CKAsset files it creates. The previous path left those files
in the system temp directory after CloudKit had consumed them, relying
on OS cleanup even though the archive save has a clear lifecycle
boundary.
Archive record construction now returns a package containing both the
record and its temporary asset URLs. GameArchiver removes those files
only after the CKModifyRecordsOperation succeeds, and logs cleanup
failures without turning a successful archive save into a failed write.
Co-Authored-By: Codex GPT 5.5 <[email protected]>
Diffstat:
3 files changed, 94 insertions(+), 31 deletions(-)
diff --git a/Crossmate/Sync/Archive.swift b/Crossmate/Sync/Archive.swift
@@ -271,11 +271,12 @@ enum Archive {
// MARK: - Record building
- /// Builds the freshly-minted `Archive` record for a snapshot. Write-once
- /// and immutable, so — like `Ping`/`Journal` — there is no system-fields
- /// archive: a re-write of an already-stored archive is a benign conflict the
- /// save path treats as success.
- static func record(from snapshot: Snapshot) throws -> CKRecord {
+ struct RecordPackage {
+ let record: CKRecord
+ let temporaryAssetFileURLs: [URL]
+ }
+
+ static func recordPackage(from snapshot: Snapshot) throws -> RecordPackage {
let zone = zoneID(forOriginalGameID: snapshot.originalGameID)
let recordID = CKRecord.ID(
recordName: recordName(forOriginalGameID: snapshot.originalGameID),
@@ -291,18 +292,24 @@ enum Archive {
}
record["solveSeconds"] = Int64(snapshot.solveSeconds) as CKRecordValue
- record["puzzleSource"] = try asset(for: Data(snapshot.puzzleSource.utf8), ext: "xd")
- record["cells"] = try asset(for: try encodeCells(snapshot.cells), ext: "json")
- record["journals"] = try asset(for: try encodeJournals(snapshot.journal), ext: "json")
- return record
+ let puzzleSource = try asset(for: Data(snapshot.puzzleSource.utf8), ext: "xd")
+ let cells = try asset(for: try encodeCells(snapshot.cells), ext: "json")
+ let journals = try asset(for: try encodeJournals(snapshot.journal), ext: "json")
+ record["puzzleSource"] = puzzleSource.asset
+ record["cells"] = cells.asset
+ record["journals"] = journals.asset
+ return RecordPackage(
+ record: record,
+ temporaryAssetFileURLs: [puzzleSource.url, cells.url, journals.url]
+ )
}
- private static func asset(for data: Data, ext: String) throws -> CKAsset {
+ private static func asset(for data: Data, ext: String) throws -> (asset: CKAsset, url: URL) {
let url = FileManager.default.temporaryDirectory
.appendingPathComponent(UUID().uuidString)
.appendingPathExtension(ext)
try data.write(to: url, options: .atomic)
- return CKAsset(fileURL: url)
+ return (CKAsset(fileURL: url), url)
}
// MARK: - Materialization
diff --git a/Crossmate/Sync/GameArchiver.swift b/Crossmate/Sync/GameArchiver.swift
@@ -140,8 +140,9 @@ final class GameArchiver {
let zoneID = Archive.zoneID(forOriginalGameID: snapshot.originalGameID)
do {
try await ensureZone(zoneID)
- let record = try Archive.record(from: snapshot)
- try await save(record)
+ let package = try Archive.recordPackage(from: snapshot)
+ try await save(package.record)
+ removeTemporaryArchiveFiles(package.temporaryAssetFileURLs)
if markComplete { markArchived(originalGameID: snapshot.originalGameID) }
} catch {
syncMonitor?.recordError("archive game", error)
@@ -152,6 +153,19 @@ final class GameArchiver {
}
}
+ private func removeTemporaryArchiveFiles(_ urls: [URL]) {
+ for url in urls {
+ do {
+ try FileManager.default.removeItem(at: url)
+ } catch {
+ eventLog?.note(
+ "GameArchiver: failed to remove temporary archive asset \(url.lastPathComponent) — \(error)",
+ level: "error"
+ )
+ }
+ }
+ }
+
/// Reads back the archive record from this user's private database — the
/// accumulated, possibly cross-device-converged copy. `nil` when it doesn't
/// exist yet or the database is unreachable.
diff --git a/Tests/Unit/ArchiveTests.swift b/Tests/Unit/ArchiveTests.swift
@@ -94,6 +94,19 @@ struct ArchiveTests {
Dictionary(uniqueKeysWithValues: journals.map { ($0.key, $0.entries) })
}
+ private func withArchiveRecord<T>(
+ from snapshot: Archive.Snapshot,
+ _ body: (CKRecord) throws -> T
+ ) throws -> T {
+ let package = try Archive.recordPackage(from: snapshot)
+ defer {
+ for url in package.temporaryAssetFileURLs {
+ try? FileManager.default.removeItem(at: url)
+ }
+ }
+ return try body(package.record)
+ }
+
// MARK: - Identity
@Test("archiveGameID is deterministic and distinct from the original")
@@ -123,12 +136,11 @@ struct ArchiveTests {
func recordRoundTrip() throws {
let original = UUID()
let snapshot = sampleSnapshot(originalGameID: original)
- let record = try Archive.record(from: snapshot)
-
- #expect(record.recordType == Archive.recordType)
- #expect(record.recordID.zoneID.zoneName == "archive-\(original.uuidString)")
-
- let payload = try #require(Archive.payload(from: record))
+ let payload = try withArchiveRecord(from: snapshot) { record in
+ #expect(record.recordType == Archive.recordType)
+ #expect(record.recordID.zoneID.zoneName == "archive-\(original.uuidString)")
+ return try #require(Archive.payload(from: record))
+ }
#expect(payload.originalGameID == original)
#expect(payload.archiveGameID == Archive.archiveGameID(for: original))
#expect(payload.title == snapshot.title)
@@ -141,6 +153,30 @@ struct ArchiveTests {
#expect(normalized(payload.journal) == normalized(snapshot.journal))
}
+ @Test("record package exposes the temporary CKAsset files it creates")
+ func recordPackageTracksTemporaryAssetFiles() throws {
+ let original = UUID()
+ let package = try Archive.recordPackage(from: sampleSnapshot(originalGameID: original))
+ defer {
+ for url in package.temporaryAssetFileURLs {
+ try? FileManager.default.removeItem(at: url)
+ }
+ }
+
+ #expect(package.temporaryAssetFileURLs.count == 3)
+ #expect(Set(package.temporaryAssetFileURLs).count == 3)
+ for url in package.temporaryAssetFileURLs {
+ #expect(FileManager.default.fileExists(atPath: url.path))
+ }
+
+ let assetURLs = [
+ (package.record["puzzleSource"] as? CKAsset)?.fileURL,
+ (package.record["cells"] as? CKAsset)?.fileURL,
+ (package.record["journals"] as? CKAsset)?.fileURL,
+ ]
+ #expect(Set(assetURLs.compactMap { $0 }) == Set(package.temporaryAssetFileURLs))
+ }
+
// MARK: - Convergence merge
@Test("merging unions peer devices and keeps the local copy of shared keys")
@@ -238,8 +274,9 @@ struct ArchiveTests {
let persistence = makeTestPersistence()
let ctx = persistence.viewContext
let original = UUID()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let payload = try #require(Archive.payload(from: record))
+ let payload = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ try #require(Archive.payload(from: record))
+ }
let game = try #require(Archive.materialize(payload, in: ctx))
#expect(game.id == Archive.archiveGameID(for: original))
@@ -272,8 +309,9 @@ struct ArchiveTests {
let store = makeTestStore(persistence: persistence)
let ctx = persistence.viewContext
let original = UUID()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let payload = try #require(Archive.payload(from: record))
+ let payload = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ try #require(Archive.payload(from: record))
+ }
_ = Archive.materialize(payload, in: ctx)
try ctx.save()
@@ -303,8 +341,9 @@ struct ArchiveTests {
let persistence = makeTestPersistence()
let ctx = persistence.viewContext
let original = UUID()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let payload = try #require(Archive.payload(from: record))
+ let payload = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ try #require(Archive.payload(from: record))
+ }
_ = Archive.materialize(payload, in: ctx)
_ = Archive.materialize(payload, in: ctx)
@@ -336,8 +375,9 @@ struct ArchiveTests {
live.ckRecordName = "game-\(original.uuidString)"
try ctx.save()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let result = engine.applyArchiveRecord(record, in: ctx)
+ let result = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ engine.applyArchiveRecord(record, in: ctx)
+ }
#expect(result == nil)
let req = NSFetchRequest<GameEntity>(entityName: "GameEntity")
@@ -353,8 +393,9 @@ struct ArchiveTests {
let ctx = persistence.viewContext
let original = UUID()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let result = engine.applyArchiveRecord(record, in: ctx)
+ let result = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ engine.applyArchiveRecord(record, in: ctx)
+ }
#expect(result == Archive.archiveGameID(for: original))
}
@@ -376,8 +417,9 @@ struct ArchiveTests {
revoked.ckRecordName = "game-\(original.uuidString)"
try ctx.save()
- let record = try Archive.record(from: sampleSnapshot(originalGameID: original))
- let result = engine.applyArchiveRecord(record, in: ctx)
+ let result = try withArchiveRecord(from: sampleSnapshot(originalGameID: original)) { record in
+ engine.applyArchiveRecord(record, in: ctx)
+ }
#expect(result == Archive.archiveGameID(for: original))
}
}
