Fix delete-resurrection: atomic tombstone check on sync apply

Catchlight/Database/EncryptedTakeStore.swift

@@ -347,53 +347,78 @@ public final class EncryptedTakeStore: TakeStore {
     // MARK: - TakeStore: Takes
 
     public func upsert(_ take: Take) throws {
+        try queue.sync { try upsertHoldingQueue(take) }
+    }
+
+    public func applyRemote(_ take: Take) throws -> Bool {
         try queue.sync {
-            try exec("BEGIN IMMEDIATE;")
-            var committed = false
-            defer { if !committed { try? exec("ROLLBACK;") } }
-            // Single-Obie invariant under last-write-wins: an incoming Obie
-            // (e.g. applied from another device by sync) demotes any existing
-            // one. Without this, the partial unique index rejected the row with
-            // an opaque writeFailed and the whole upsert rolled back — and the
-            // in-memory store (no index) silently accepted it, so the two
-            // implementations diverged on the same input.
-            if take.isObie {
-                // Demote any existing Obie. The is_obie COLUMN is only an index — a
-                // Take's isObie ALSO lives inside its encrypted PAYLOAD, which
-                // allTakes() decrypts as the source of truth. A column-only UPDATE (the
-                // previous approach) left the demoted Take's payload still isObie=true,
-                // so the timeline read TWO Obies and the newly-set one was invisible
-                // (owner-reported 2026-06-23; pinned by the upsert-demote contract
-                // test, which the column-only path failed). Re-seal each existing Obie
-                // with isObie=false so column AND payload agree. Decoding by PAYLOAD
-                // (not the column) also REPAIRS a store already holding a stray Obie on
-                // the next Obie set. Mirrors InMemoryTakeStore.upsert.
-                let sel = try prepare("SELECT id, payload FROM takes WHERE id != ?1;")
-                defer { sqlite3_finalize(sel) }
-                bindText(sel, 1, take.id.uuidString)
-                for var existing in try collectTakes(sel) where existing.isObie {
-                    existing.isObie = false
-                    // Bump modifiedAt so the demotion SYNCS (2026-07-01). pushOutbound
-                    // selects by `modifiedAt > watermark`; without the bump the demoted
-                    // Take was never re-uploaded, the cloud kept a second isObie=true
-                    // blob, and every later pull hit ConflictResolver's (false,false)
-                    // branch — a phantom "changed on another device" conflict. Matches
-                    // setObie. When SYNC applies a remote Obie this can bump a Take the
-                    // fleet already demoted — one redundant, converging re-upload;
-                    // accepted trade-off for never losing the demotion.
-                    existing.modifiedAt = Date()
-                    try insertOrReplace(existing)
+            // Tombstone check and write INSIDE one queue.sync so a concurrent
+            // delete (main thread) cannot land between them — the pull loop's
+            // own guard reads a pull-start snapshot and cannot see a mid-pull
+            // delete (the delete-resurrection bug, closed 2026-07-23). Deletion
+            // wins ties (`>=`), matching the pull guard: a remote edit STRICTLY
+            // after the deletion still resurrects.
+            let sel = try prepare("SELECT deleted_at FROM tombstones WHERE id = ?1;")
+            defer { sqlite3_finalize(sel) }
+            bindText(sel, 1, take.id.uuidString)
+            if sqlite3_step(sel) == SQLITE_ROW {
+                guard let deletedAt = ISO8601.date(from: columnText(sel, 0)) else {
+                    throw StorageError.corruptRow("tombstone row has unparseable date")
                 }
+                if deletedAt >= take.modifiedAt { return false }
+            }
+            try upsertHoldingQueue(take)
+            return true
+        }
+    }
+
+    /// The upsert body. NOT serialised — callers hold the queue.
+    private func upsertHoldingQueue(_ take: Take) throws {
+        try exec("BEGIN IMMEDIATE;")
+        var committed = false
+        defer { if !committed { try? exec("ROLLBACK;") } }
+        // Single-Obie invariant under last-write-wins: an incoming Obie
+        // (e.g. applied from another device by sync) demotes any existing
+        // one. Without this, the partial unique index rejected the row with
+        // an opaque writeFailed and the whole upsert rolled back — and the
+        // in-memory store (no index) silently accepted it, so the two
+        // implementations diverged on the same input.
+        if take.isObie {
+            // Demote any existing Obie. The is_obie COLUMN is only an index — a
+            // Take's isObie ALSO lives inside its encrypted PAYLOAD, which
+            // allTakes() decrypts as the source of truth. A column-only UPDATE (the
+            // previous approach) left the demoted Take's payload still isObie=true,
+            // so the timeline read TWO Obies and the newly-set one was invisible
+            // (owner-reported 2026-06-23; pinned by the upsert-demote contract
+            // test, which the column-only path failed). Re-seal each existing Obie
+            // with isObie=false so column AND payload agree. Decoding by PAYLOAD
+            // (not the column) also REPAIRS a store already holding a stray Obie on
+            // the next Obie set. Mirrors InMemoryTakeStore.upsert.
+            let sel = try prepare("SELECT id, payload FROM takes WHERE id != ?1;")
+            defer { sqlite3_finalize(sel) }
+            bindText(sel, 1, take.id.uuidString)
+            for var existing in try collectTakes(sel) where existing.isObie {
+                existing.isObie = false
+                // Bump modifiedAt so the demotion SYNCS (2026-07-01). pushOutbound
+                // selects by `modifiedAt > watermark`; without the bump the demoted
+                // Take was never re-uploaded, the cloud kept a second isObie=true
+                // blob, and every later pull hit ConflictResolver's (false,false)
+                // branch — a phantom "changed on another device" conflict. Matches
+                // setObie. When SYNC applies a remote Obie this can bump a Take the
+                // fleet already demoted — one redundant, converging re-upload;
+                // accepted trade-off for never losing the demotion.
+                existing.modifiedAt = Date()
+                try insertOrReplace(existing)
             }
-            try insertOrReplace(take)
-            // Re-creating an item supersedes any pending tombstone for it.
-            let ts = try prepare("DELETE FROM tombstones WHERE id = ?1;")
-            defer { sqlite3_finalize(ts) }
-            bindText(ts, 1, take.id.uuidString)
-            guard sqlite3_step(ts) == SQLITE_DONE else { throw StorageError.writeFailed(lastError()) }
-            try exec("COMMIT;")
-            committed = true
         }
+        try insertOrReplace(take)
+        // Re-creating an item supersedes any pending tombstone for it.
+        let ts = try prepare("DELETE FROM tombstones WHERE id = ?1;")
+        defer { sqlite3_finalize(ts) }
+        bindText(ts, 1, take.id.uuidString)
+        guard sqlite3_step(ts) == SQLITE_DONE else { throw StorageError.writeFailed(lastError()) }
+        try exec("COMMIT;")
+        committed = true
     }
 
     /// Shared by upsert and migration. NOT serialised — callers hold the queue

Catchlight/UI/Views/DailiesView.swift

@@ -1270,8 +1270,16 @@ struct DailiesView: View {
         defer { editDraft = nil; ui.endEditingInPlace() }
         guard var t = editDraft else { return }
         t.removeEmptyTextBlocks()
+        // A tapped-away Take with nothing in it is discarded, NOT saved — and that now
+        // includes a brand-new Obie (owner 2026-07-20): the Obie widget/intent opens a
+        // blank Obie draft, and tapping the background with nothing typed used to persist
+        // an empty pinned Obie. `!t.isObie` was in this test, forcing `isBlank` false for
+        // any Obie; dropping it treats an Obie like any other Take, matching the locked-
+        // capture path (`AppModel.saveLockedCapture`), which already discards a blank Obie.
+        // An EXISTING Obie whose text is cleared is still kept, via the `storedHadContent`
+        // guard below.
         let isBlank = t.plainText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
-            && !t.isTask && t.timeReminder == nil && !t.isObie
+            && !t.isTask && t.timeReminder == nil
             && t.attachments.isEmpty && t.locationReminder == nil
         let storedCopy = try? vm.store.take(id: t.id)
         let storedHadContent = (storedCopy?.plainText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false)

Sources/CatchlightCore/Storage/TakeStore.swift

@@ -73,6 +73,32 @@ public protocol TakeStore: AnyObject {
     /// Remove tombstones once the deletion is durably recorded in the uploaded
     /// cloud manifest (or applied from a remote tombstone).
     func purgeTombstones(ids: [UUID]) throws
+
+    /// Apply a Take arriving FROM SYNC unless a live local tombstone supersedes
+    /// it (`deletedAt >= take.modifiedAt` — deletion wins ties, mirroring the
+    /// pull loop's resurrection guard; a remote edit STRICTLY after the deletion
+    /// still resurrects). Returns false when the tombstone wins and NOTHING was
+    /// written. This exists because the pull loop's own guard reads a tombstone
+    /// snapshot taken at pull-start — a delete landing mid-pull is invisible to
+    /// it, and a plain `upsert` would resurrect the Take and clear the fresh
+    /// tombstone (the delete-resurrection bug, closed 2026-07-23). Stores that
+    /// serialise their operations MUST check-and-write inside one critical
+    /// section so a concurrent `delete` cannot interleave.
+    func applyRemote(_ take: Take) throws -> Bool
+}
+
+public extension TakeStore {
+    /// Default (read-then-write; race-free only where the caller already is —
+    /// fine for synchronous test doubles, NOT for concurrent production stores,
+    /// which must override with an atomic implementation).
+    func applyRemote(_ take: Take) throws -> Bool {
+        if let tomb = try tombstones().first(where: { $0.id == take.id }),
+           tomb.deletedAt >= take.modifiedAt {
+            return false
+        }
+        try upsert(take)
+        return true
+    }
 }
 
 /// In-memory `TakeStore` for tests and previews. Not used in production.
@@ -110,6 +136,16 @@ public final class InMemoryTakeStore: TakeStore {
         tombstoneMap[id] = Tombstone(id: id, deletedAt: Date())
     }
 
+    public func applyRemote(_ take: Take) throws -> Bool {
+        // Mirrors EncryptedTakeStore.applyRemote so the two implementations stay
+        // contract-identical for sync-applied rows.
+        if let tomb = tombstoneMap[take.id], tomb.deletedAt >= take.modifiedAt {
+            return false
+        }
+        try upsert(take)
+        return true
+    }
+
     public func tombstones() throws -> [Tombstone] {
         tombstoneMap.values.sorted { $0.deletedAt < $1.deletedAt }
     }

Sources/CatchlightCore/Sync/SyncEngine.swift

@@ -386,14 +386,17 @@ public final class SyncEngine {
             }
             switch ConflictResolver.decide(local: local, remote: remoteTake, lastSync: lastSync) {
             case .takeRemote(let t):
-                // (Removed 2026-07-16: a TEMP "REMOVE after repro" trap from the deleted-Take-
-                // reappears hunt of 2026-07-10 — that bug was CLOSED as non-reproducible and
-                // PR #128 pulled the traps, but this one was missed. It fired on every sync
-                // apply, so it would have swamped the breadcrumb budget with dead output, and
-                // it logged a Take id fragment — the one thing here that leans toward
-                // identifying content.)
-                try store.upsert(t)
-                report.applied.append(t.id)
+                // RESURRECTION GUARD, part 2 (2026-07-23): the guard above consults
+                // `pendingTombstoneByID`, a snapshot taken at pull-start — a delete
+                // committed MID-PULL is invisible to it, and a plain `upsert` here
+                // would re-create the Take AND clear its fresh tombstone (the
+                // delete-resurrection bug). `applyRemote` re-checks tombstones
+                // atomically with the write (the same store critical section a
+                // concurrent `delete` uses), so the freshest deletion always wins
+                // ties; a remote edit strictly after it still lands.
+                if try store.applyRemote(t) {
+                    report.applied.append(t.id)
+                }
             case .conflict(let l, let r):
                 report.conflicts.append((local: l, remote: r))   // surfaced; UI resolves
             case .keepLocal, .noChange:

Sources/coreverify/main.swift

@@ -287,6 +287,28 @@ do {
         let afterDelete = try storeB.take(id: take.id)
         check("Inbound deletion from another device applied locally", report.deletedLocally == [take.id] && afterDelete == nil)
     }
+    // applyRemote — the sync-apply path must not resurrect a Take over a live
+    // tombstone (delete-resurrection guard, closed 2026-07-23), while a remote
+    // edit strictly after the deletion still lands (edit-wins).
+    do {
+        let store = InMemoryTakeStore()
+        var t = richTake()
+        try store.upsert(t)
+        try store.delete(id: t.id)
+        let deletedAt = try store.tombstones()[0].deletedAt
+
+        t.modifiedAt = deletedAt   // ties go to the deletion
+        let blocked = try store.applyRemote(t) == false
+        let stillGone = try store.take(id: t.id) == nil
+        let tombKept = try store.tombstones().map(\.id) == [t.id]
+        check("applyRemote blocked by live tombstone (no resurrection, tombstone kept)", blocked && stillGone && tombKept)
+
+        t.modifiedAt = deletedAt.addingTimeInterval(1)
+        let applied = try store.applyRemote(t)
+        let restored = try store.take(id: t.id) != nil
+        let tombCleared = try store.tombstones().isEmpty
+        check("applyRemote applies a remote edited after the deletion (edit-wins)", applied && restored && tombCleared)
+    }
     // Conflict detection + surfaced end-to-end without overwrite.
     do {
         let lastSync = Date(timeIntervalSince1970: 1_700_000_000)

Tests/CatchlightCoreTests/SyncEngineEdgeCasesTests.swift

@@ -372,6 +372,48 @@ final class SyncEngineEdgeCasesTests: XCTestCase {
         XCTAssertEqual(try local.take(id: remoteTake.id)?.primaryText, "remote-new")
     }
 
+    // MARK: - Pull: delete lands mid-pull (delete-resurrection race, closed 2026-07-23)
+
+    /// The pull loop snapshots pending tombstones ONCE at pull-start; a user
+    /// delete committed while the loop is mid-flight is invisible to that
+    /// snapshot, and a plain `upsert` on the `.takeRemote` branch resurrected
+    /// the Take and cleared its fresh tombstone. `applyRemote` re-checks
+    /// tombstones atomically with the write, so the deletion must now win and
+    /// survive to propagate on the next push.
+    func testSyncEngine_deleteLandsMidPull_takeStaysDeleted_tombstoneSurvives() throws {
+        let k = makeKeys()
+        let cloud = InMemoryCloudFolder()
+        let t0 = Date(timeIntervalSince1970: 1_700_000_000)
+
+        // Another device pushes the Take at modifiedAt = 200.
+        let remoteStore = InMemoryTakeStore()
+        var remoteTake = TestFixtures.richTake()
+        remoteTake.primaryText = "remote-new"
+        remoteTake.modifiedAt = t0.addingTimeInterval(200)
+        try remoteStore.upsert(remoteTake)
+        try makeEngine(store: remoteStore, cloud: cloud, keys: k, now: { t0.addingTimeInterval(201) }).pushOutbound()
+
+        // Local holds an older, unchanged copy (modifiedAt 100 ≤ watermark 150),
+        // so the pull decides `.takeRemote`. The wrapper lands the user's delete
+        // the moment the loop reads this id — after the pull-start snapshot.
+        let inner = InMemoryTakeStore()
+        var localTake = remoteTake
+        localTake.primaryText = "local-old"
+        localTake.modifiedAt = t0.addingTimeInterval(100)
+        try inner.upsert(localTake)
+        inner.setLastSyncDate(t0.addingTimeInterval(150))
+        let local = MidPullDeleteStore(wrapping: inner, deleting: localTake.id)
+
+        let report = try makeEngine(store: local, cloud: cloud, keys: k,
+                                    now: { t0.addingTimeInterval(300) }).pullInbound()
+
+        XCTAssertTrue(local.deleteFired, "test seam must have exercised the mid-pull delete")
+        XCTAssertTrue(report.applied.isEmpty, "the mid-pull deletion must win over the remote copy")
+        XCTAssertNil(try inner.take(id: localTake.id), "the deleted Take must NOT be resurrected")
+        XCTAssertEqual(try inner.tombstones().map(\.id), [localTake.id],
+                       "the fresh tombstone must survive so the deletion propagates on push")
+    }
+
     // MARK: - Pull: partial-sync quarantine
 
     /// A 5-blob pull where the middle blob is corrupted should quarantine that
@@ -593,3 +635,43 @@ final class SyncEngineEdgeCasesTests: XCTestCase {
         XCTAssertTrue(lock.isStale(now: Date()))
     }
 }
+
+/// Simulates the user deleting a Take MID-PULL: the first time the pull loop
+/// reads the target id — which happens AFTER the pull-start tombstone snapshot —
+/// the delete lands before the read returns. That is exactly the window the
+/// stale snapshot cannot see. Everything else forwards to the wrapped store.
+private final class MidPullDeleteStore: TakeStore {
+    private let wrapped: InMemoryTakeStore
+    private let targetID: UUID
+    private(set) var deleteFired = false
+
+    init(wrapping wrapped: InMemoryTakeStore, deleting targetID: UUID) {
+        self.wrapped = wrapped
+        self.targetID = targetID
+    }
+
+    func take(id: UUID) throws -> Take? {
+        if id == targetID, !deleteFired {
+            deleteFired = true
+            try wrapped.delete(id: id)   // the user's delete lands here
+        }
+        return try wrapped.take(id: id)
+    }
+
+    func upsert(_ take: Take) throws { try wrapped.upsert(take) }
+    func delete(id: UUID) throws { try wrapped.delete(id: id) }
+    func allTakes() throws -> [Take] { try wrapped.allTakes() }
+    func takesModified(since date: Date?) throws -> [Take] { try wrapped.takesModified(since: date) }
+    func search(_ query: String) throws -> [Take] { try wrapped.search(query) }
+    func upsert(_ sequence: CatchlightSequence) throws { try wrapped.upsert(sequence) }
+    func sequence(id: UUID) throws -> CatchlightSequence? { try wrapped.sequence(id: id) }
+    func allSequences() throws -> [CatchlightSequence] { try wrapped.allSequences() }
+    func deleteSequence(id: UUID) throws { try wrapped.deleteSequence(id: id) }
+    func currentObie() throws -> Take? { try wrapped.currentObie() }
+    func setObie(id: UUID, replaceExisting: Bool) throws { try wrapped.setObie(id: id, replaceExisting: replaceExisting) }
+    func lastSyncDate() -> Date? { wrapped.lastSyncDate() }
+    func setLastSyncDate(_ date: Date) { wrapped.setLastSyncDate(date) }
+    func tombstones() throws -> [Tombstone] { try wrapped.tombstones() }
+    func purgeTombstones(ids: [UUID]) throws { try wrapped.purgeTombstones(ids: ids) }
+    func applyRemote(_ take: Take) throws -> Bool { try wrapped.applyRemote(take) }
+}