fix(ios): preserve an active scan archive when starting over

.greptile/config.json

@@ -0,0 +1,3 @@
+{
+  "autoReview": ["open", "push"]
+}

CONTRIBUTING.md

@@ -22,6 +22,8 @@ State what the old code did before describing the new behavior. Link the reviewe
 
 The `size:*` label describes the effective diff. It is a review signal, not a merge gate. Do not set it by hand.
 
+Greptile reviews new commits through [`.greptile/config.json`](.greptile/config.json). The dashboard's open-only trigger left follow-up commits unreviewed. This repository override adds push reviews without changing other repositories' settings. Greptile reads this setting from the PR's source branch, so older branches need the config too. For a branch without it, request a current review with `@greptileai review` after pushing. Use `@greptileai review this draft` for drafts.
+
 ## Checks
 
 | Check | Runs on GitHub when | Local command |

ios/HouseScan/Runtime/KeyframeStore.swift

@@ -51,16 +51,21 @@ final class KeyframeStore {
     /// Bumped by `discardKeyframes`, so a write that started before it doesn't land in the list.
     private var epoch = 0
 
-    /// Makes a new, empty scan folder and deletes every other one: only the current scan is kept
-    /// on the phone. Covers both a start over (the previous scan's folder) and launch (folders a
-    /// quit or crashed run left behind), since both make a new store. The folders to delete are
-    /// listed here, before a newer store can exist, and deleted later off the main actor
-    /// (`ScanFolderCleanup`), so a deletion that runs late can't take a newer scan's folder.
-    init() {
+    /// Makes a new, empty scan folder and deletes the old ones `ScanFolderCleanup` lists: it keeps
+    /// the two most recent completed scans (Saved scans offers them) and any folder whose bundle
+    /// is there but not whole. Covers both a start over (the previous scan's folder) and launch
+    /// (folders a quit or crashed run left behind), since both make a new store.
+    ///
+    /// The folders are listed here, before a newer store can exist, so a deletion that runs late
+    /// can't take a newer scan's folder. They are deleted later off the main actor, once
+    /// `bundleWrites` ends: the scan bundle writes started before this store, of which the
+    /// previous scan's may still be assembling its packet in a folder listed as never packaged.
+    /// Start over doesn't wait for it; only the deletion does. Nil, as at launch, waits for none.
+    init(deletingAfter bundleWrites: Task<Void, Never>? = nil) {
         let scans = Self.scansRoot
         directory = scans.appending(path: UUID().uuidString, directoryHint: .isDirectory)
         try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
-        Self.delete(ScanFolderCleanup(root: scans, keeping: directory.lastPathComponent))
+        Self.delete(ScanFolderCleanup(root: scans, keeping: directory.lastPathComponent), after: bundleWrites)
     }
 
     /// Caches/Scans: one folder per scan. Saved scans lists the completed ones here.
@@ -246,7 +251,8 @@ final class KeyframeStore {
 
     /// Zips the packet folder into `scan.zip`, manifest.json at the zip's root, and removes the
     /// folder. The zip is what "Share scan" offers; the upload sends scene.json alone. It stays on
-    /// the phone until the next scan unless the homeowner shares it.
+    /// the phone, listed in Saved scans, until two newer completed scans push it out
+    /// (`ScanFolderCleanup`).
     ///
     /// Streamed to disk one file at a time: a long walk's JPEGs held in memory twice (the entries
     /// and the archive) is what an in-memory build cost.
@@ -267,11 +273,12 @@ final class KeyframeStore {
         try? files.removeItem(at: folder)
     }
 
-    /// Off the main actor. A write still in flight for a deleted folder fails, because its
-    /// directory is gone, and is dropped like any failed write.
-    nonisolated private static func delete(_ cleanup: ScanFolderCleanup) {
+    /// Off the main actor, after `bundleWrites`. A keyframe write still in flight for a deleted
+    /// folder fails, because its directory is gone, and is dropped like any failed write; the
+    /// bundle lists only keyframes already written.
+    nonisolated private static func delete(_ cleanup: ScanFolderCleanup, after bundleWrites: Task<Void, Never>?) {
         Task.detached(priority: .utility) {
-            for (url, error) in cleanup.run() {
+            for (url, error) in await cleanup.run(after: { await bundleWrites?.value }) {
                 RuntimeLog.engine.error("could not delete old scan \(url.lastPathComponent, privacy: .public): \(String(describing: error), privacy: .public)")
             }
         }

ios/HouseScan/Runtime/ScanEngine.swift

@@ -2220,7 +2220,18 @@ final class ScanEngine {
     /// place, so it is not offered while a write is under way, and writes run one after another:
     /// a retry's write waits for the last one, and a write already superseded is skipped.
     /// `inputs` were captured with the upload's scene (`captureUpload`); this rereads nothing.
+    ///
+    /// Inputs read from an earlier store write into that store's folder, which the current
+    /// store's cleanup may delete; they are refused before they change what this scan offers.
+    /// The callers already drop a capture from before Start over; this keeps that true here.
+    ///
+    /// Start over doesn't wait for a write in flight: the next store's cleanup waits for
+    /// `bundleTask` before deleting the folder, so the write's bundle lands and is kept.
     func saveBundle(_ inputs: PacketInputs?) {
+        if let inputs, inputs.storeDirectory != store.directory {
+            RuntimeLog.engine.error("scan bundle not written: its capture belongs to a scan already started over")
+            return
+        }
         state.shareableScan = nil
         bundleSerial += 1
         let serial = bundleSerial
@@ -2271,7 +2282,11 @@ final class ScanEngine {
         meterTracking = nil
         meterAnchorPresence = MeterAnchorPresence()
         meterPlaneSource = .detectedPlane
-        store = KeyframeStore()
+        // The old scan's bundle may still be packing in the folder the new store's cleanup lists
+        // as never packaged. Start over doesn't wait for it; the cleanup's deletion does, for this
+        // chain head (every write waits for the one before it). `generation` was raised above,
+        // so a queued write for the old scan still skips, and a finished one isn't offered here.
+        store = KeyframeStore(deletingAfter: bundleTask)
         recorder = Self.makeRecorder(store)
         live?.setRecorder(recorder)
         motion.stop()

ios/HouseScanKit/Sources/HouseScanKit/Scene/ScanFolderCleanup.swift

@@ -26,6 +26,13 @@ import Foundation
 /// can exist, and only those are deleted, whenever the deletion gets to run. Listing at deletion
 /// time instead let a cleanup that ran late delete a scan started after it: store A is made,
 /// Start over makes B, and A's cleanup then found B in the listing and deleted the scan in use.
+///
+/// A folder whose bundle is still being built has no `scan.zip` yet, so it is listed as never
+/// packaged. The packet is assembled in the folder first and zipped last, and Start over doesn't
+/// wait for that write, so deleting on the listing alone took the folder from under it and the
+/// scan never reached Saved scans. The app therefore deletes with `run(after:)`, waiting for the
+/// bundle writes that were started before the listing; the recheck in `run` then keeps a folder
+/// whose bundle landed meanwhile.
 public struct ScanFolderCleanup: Sendable {
     /// The bundle Share scan offers. A scan is completed when this file is a whole capture packet
     /// (`PacketArchiveCheck`); the file being there is not enough.
@@ -132,7 +139,8 @@ public struct ScanFolderCleanup: Sendable {
     /// The deletion runs later than the listing, off the main actor, and a folder can change in
     /// between: a bundle write can start in a folder listed as never packaged, or finish in one.
     /// Each entry is read again and deleted only if it is still what it was when listed; one that
-    /// changed is left for the next cleanup. A change after this second read is not caught.
+    /// changed is left for the next cleanup. A change after this second read is not caught, so a
+    /// write still going when this runs can lose its folder: `run(after:)` waits for known writers.
     public func run() -> [(url: URL, error: any Error)] {
         let files = FileManager.default
         return obsolete.compactMap { url in
@@ -146,4 +154,14 @@ public struct ScanFolderCleanup: Sendable {
             }
         }
     }
+
+    /// `run()`, once `wait` returns. The app's `wait` is the scan bundle writes started before
+    /// this cleanup was listed (one chain: each write waits for the one before it), so a folder
+    /// listed while its packet was still being assembled is judged again only after that write
+    /// ends: a bundle that landed keeps the folder, and a write that failed before zipping leaves
+    /// it to be deleted. A `wait` that returns at once, as at launch with no write, is `run()`.
+    public func run(after wait: () async -> Void) async -> [(url: URL, error: any Error)] {
+        await wait()
+        return run()
+    }
 }