[johnny4young]

Summary

Blob adoption and orphan deletion share the cross-process ownership ledger supplied by #199. An orphan reference check and its deletion cannot interleave with a same-hash adoption and row commit.

Current head: fc98aad11f88f1a5e6b0405bba3fcfab2d2421bf. Stacked on fix/preserve-recovery-generation (#199), currently 27d057ea7405970b78397d100925fa56bbf58c45. Review and merge #199 first; dependency integration and fresh qualification remain required before merging this PR. No merge, release or deployment has been performed.

Ownership contract

• Hold a committed logical token across binary preparation/adoption and row commit for capture, batch, inbox, single/page sync and restore
• Hold the same ownership domain across reference lookup and filesystem deletion for single, chunked and full orphan cleanup
• Finish coordinator transactions before awaiting main-store work. No kernel lock or coordinator transaction spans the asynchronous operation
• Required binary admission and explicit maintenance retry only logical contention or SQLite busy/locked within the existing two-second request window. A deadline reports busy; it never revokes a live or suspended owner
• Text, file-reference and metadata-only adoption skip the required blob lease. A mixed page/batch containing binary content still requires it
• Production stores and file-backed injected facades share the coordinator root. In-memory fixtures use independent per-blob namespaces

Latest repair: opportunistic post-commit sync cleanup

The previous nonbinary-write test used fresh rows, so its cleanup candidate sets were empty. Replacing or deleting an existing binary row could still await the cleanup lease for up to the request window, swallow busy, and return success. Successful writes alone did not establish that this path skipped the wait.

Commit b62cc00 changes three files:

• SyncLocalStore.swift: after a durable single/page upsert or remote deletion, try cleanup admission once. On contention or an acquisition error, keep the bytes for a later maintenance sweep. The internal synchronous acquisition seam enables deterministic attempt-count coverage
• RetentionEngine.swift: share the reference-check/delete implementation between required maintenance and opportunistic cleanup. The caller keeps its acquired token through the fresh reference read and deletion, and releases on completion/error
• BlobOwnershipRaceTests.swift: add old-binary and admission-contract coverage, and assert the original page's summary and persisted preview

This removes the logical-owner retry loop from sync's post-commit path. It does not promise zero I/O latency: a single coordinator attempt can encounter the existing bounded SQLite/lifecycle-fence work. Explicit maintenance and binary admission retain their safety behavior. No detached cleanup task, schema change, public API addition or timeout increase is introduced.

Regression coverage

Existing barrier coverage pauses each cleanup kind after its reference read, checks admission through an independent coordinator, then verifies same-hash committed content remains readable. Cancellation, bounded live-owner busy, legacy-sweep exclusions and coordinator namespace tests remain intact.

The latest tests cover:

• Exactly one synchronous cleanup admission attempt for a real held-owner denial and for an acquisition error; zero attempts for empty candidates. Replacing the helper body with the previous awaited cleanup path would fail the attempt-count assertion
• Single and page text replacement of an old binary row, plus single and page deletion, while an independent store holds the lease
• Persisted replacement content, preview and sync fields read through a second store; page summaries; durable deletions
• Retaining deferred bytes while ownership is unavailable, later collecting actual orphans, and preserving bytes re-adopted under the same hash before maintenance
• Fresh text, file-reference, import, inbox and sync writes with a held owner; binary capture still reports busy

No shared-runner wall-clock threshold is used.

Validation

• Current head was published by a non-force, expected-head update and verified through the remote branch ref
• git diff --check passed; independent focused source review found no ownership/lifetime blocker
• make format, make lint and make test were attempted locally but could not execute because this Linux executor has no Swift/Xcode. No local Swift compile or runtime pass is claimed
• Exact-head native CI run 37551916982 passed: strict swift-format; SwiftLint with 0 violations across 614 files; 1,531 package tests in 237 suites; warning checks; 84.3% package production-source coverage above the unchanged 80% gate; StoreKit; and macOS/iOS app builds
• Native logs confirm the new empty-admission, one-attempt nil/error, four old-binary replacement/deletion cases and same-hash re-adoption tests passed
• Exact-head GitGuardian passed; the non-blocking beta canary was skipped
• Initial follow-up runs stopped at test-line formatter violations. 7ee83c1 and fc98aad wrap those lines without changing behavior; the final exact-head run above passed
• Previous head 9a25adc passed CI with 1,527 package tests and 84.3% coverage; that result does not validate the latest change

Review and rollout limits

Review the token lifetime, lock ordering, conservative error/cancellation paths, and same-hash re-adoption behavior. #199 supplies the process identity, generation lifetime and recovery protocol; this PR consumes that coordination.

Physical iOS/App Group suspension/expiration, foreground resume, restart/reboot and cross-process stress remain release qualifications. All app, extension, CLI and MCP participants must be quiesced, upgraded and restarted; mixed-version ownership is unsupported. Native execution of the combined repair/MCP tree remains a separate integration gate. Passing this PR's CI does not establish those device or combined-tree outcomes. See the store recovery protocol.

Ready for human review of this scoped repair. Physical-device and combined-tree release qualifications above remain open. This description replaces earlier stale timing claims and head-specific validation summaries.

[johnny4young]

Summary (vs fix/preserve-recovery-generation): Every blob-adopting write and every orphan check-then-delete now holds an exclusive cross-process blob claim from #199's ledger. That covers insert, importBatch, inbox ingest, archive restore, sync applyRemoteUpsert/applyRemoteChanges, removeBlobIfOrphaned, removeBlobsIfOrphaned and removeOrphanedBlobs. A claim waits at most 2 s and then throws busy. This closes the "sweeper saw no reference → capture re-adopts the hash → sweeper deletes the bytes" race.

Verdict: the race fix is correct, and I found no nested acquisition. applyRemoteUpsert and applyRemoteChanges release before removeBlobsIfOrphanedAfterCommit, and double release is idempotent. The scope of the claim creates availability regressions, though. Needs changes, or an explicit owner decision.

🟠 1. Text captures are dropped while any long blob operation runs

insert (GRDBClipboardStore.swift, around line 429) takes the exclusive blob claim for every clip, including text, file references and nil content. Only .binary content adopts a blob: insertionRow sets contentBlobHash only in that case. A text insert therefore cannot take part in the race. Meanwhile these operations hold the claim for their whole duration:

• importBatch (all blob writes plus one transaction),
• archive restore (writeBlobs plus restoreRows),
• a CloudKit fetch batch (applyRemoteChanges writes every blob in the batch),
• removeOrphanedBlobs (lists the whole directory).

Any of these can exceed 2 s on a large history or a first sync. ClipIngestionCoordinator.ingest then gets busy, and the copied text is lost. On main none of these serialized captures. Minimal mitigation, which I did not push because it is your call:

let ownership: BlobOwnershipLease? =
    if case .binary? = content { try await acquireBlobOwnership() } else { nil }
defer { ownership?.release() }

The same applies to ingestInboxCapture. Image captures can still hit the 2 s ceiling. Consider a longer wait for captures, or releasing and re-acquiring the claim between import, restore or sync chunks.

🟠 2. One stale blob claim can block all blob work indefinitely

Blob scope conflicts with every other blob claim. A claim left by a process that was killed while holding it is reclaimed only when kill(pid,0) returns ESRCH. On iOS, an app jetsam-killed while suspended mid-sync or mid-restore is a real case, and so is a reboot that hands out the same PID again. In either case the claim stays, and every insert, sync apply, restore and sweep then fails with busy until that PID disappears. With finding 1 that includes all captures. See the matching comment on #199: storing a boot-session UUID and/or the process start time next to the PID would let these claims be reclaimed safely.

🟡 Nits

• Cost per capture: every claim opens a DatabaseQueue on the sidecar (WAL), runs a write transaction to acquire, another to release, and a close that checkpoints. Once text inserts are excluded this only affects images, which is fine.
• removeOrphanedBlobs docs: safety now relies on every participant using the claim. The doc already says mixed-version processes are unqualified. The Mac maintenance step still passes a one-hour cutoff, which remains a good belt-and-braces measure for older CLI or MCP binaries.
• In BlobStore.removeAll, !name.hasPrefix(".") is broader than the old migrationMarker check, which is a good change because it also protects future hidden files.

Validation: there is no Swift toolchain in my environment, so this is code reading only. CI is green.

---

Generated by Claude Code