Sync deletion tombstones: deleted vault docs / API keys resurrect across devices

[vveerrgg]

Summary

A vault document (or API-key vault entry) deleted on one device can resurrect on the others. The storage.sync mirror has no deletion semantics: computeMergeUpdates() is purely additive + last-write-wins, so it never removes a local entry that's absent from an incoming sync payload. Combined with the fact that every device re-pushes its full local cache, a deletion made on device A gets undone the next time device B pushes.

Discovered during the security/correctness audit (follow-up to #1 / #2). Deferred from #2 because — unlike the other deferred items — this is not a contained sync-layer fix. It's entangled with the Nostr relay deletion path (NIP-09), so it needs a deliberate design decision rather than a tombstone map bolted onto storage.

Reproduce

1. Devices A and B, both synced, both holding vault doc notes/todo.md.
2. On A: delete the doc. A removes it from its local vaultDocs cache (deleteDocumentLocal) and publishes a NIP-09 deletion to relays. A's next sync push omits the doc.
3. On B: the storage.onChanged('sync') merge runs. computeMergeUpdates() only adds docs present in the sync payload — it never removes notes/todo.md, which is absent. B still has the doc.
4. B pushes its cache back to storage.sync, re-uploading notes/todo.md (with B's updatedAt).
5. A pulls. A's local no longer has the doc, so the additive merge adds it back. Deletion undone.

Same class of bug for API keys via apikeys.delete.

Root cause (code references)

• src/utilities/sync-manager.js → computeMergeUpdates(): the vaultDocs merge (const localDocs = local.vaultDocs || {} …) and the apiKeyVault merge are both additive — they take the union and pick newer updatedAt, with no path that removes a locally-present entry. No tombstone concept exists.
• src/utilities/sync-manager.js → buildSyncPayload(): serialises the full local vaultDocs / apiKeyVault every push (vaultDoc:<path> entries), so whatever is locally present is always re-advertised.
• src/background.js → vault.delete (~L971) and apikeys.delete (~L1098): publish a NIP-09 deletion event to relays but do not write anything to the storage.sync mirror.
• src/utilities/vault-store.js → deleteDocumentLocal(path) (L72): delete docs[path] from the local cache. Called only from the vault UI (src/vault/vault.js L181/193/231), not from the sync layer.

The entanglement (why this isn't a simple storage fix)

Vault docs have two sync channels with different deletion semantics:

1. Nostr relays (source of truth) — docs are NIP-78 events; deletions are NIP-09 events. A device that re-reads from relays and honours NIP-09 will eventually drop the doc.
2. storage.sync mirror (cache) — a best-effort key/value mirror with additive merge and no deletion concept.

The two fight each other: channel (2) keeps re-propagating a doc that channel (1) has tombstoned. Any fix has to reconcile them, or the storage.sync mirror will keep resurrecting NIP-09-deleted docs.

Design options

Option A — Tombstones in the sync mirror.
Carry deletion records in storage.sync: a deletedDocs / deletedKeys map of { path/id: deletedAt }. On delete, write a tombstone (alongside deleteDocumentLocal). In computeMergeUpdates, remove any local doc whose newest tombstone deletedAt is later than the doc's updatedAt. Prune tombstones older than a TTL (e.g. 90d) to bound growth.

• Pros: self-contained in the sync layer; testable in computeMergeUpdates (pure); independent of relay availability.
• Cons: a second source of deletion truth to keep consistent with NIP-09; tombstone TTL vs. an offline device returning after the TTL could still resurrect (bounded by TTL).

Option B — Make relays the only deletion authority.
Stop syncing vault docs through storage.sync entirely; rely on NIP-78 + NIP-09 over relays for docs/keys, and reserve storage.sync for profile/settings (which have no relay channel).

• Pros: one source of truth; no tombstone bookkeeping; matches where the data actually lives.
• Cons: loses the offline/relay-down resilience the mirror provides for vault content; larger behavioural change; needs the relay-fetch path to reliably honour NIP-09 on every device.

Option C — Hybrid. Keep the mirror for availability but treat NIP-09 as authoritative: when the relay layer processes a NIP-09 deletion, write a sync tombstone so the mirror converges. Essentially A, but tombstones are generated by the relay deletion path rather than the UI action.

Recommendation

Lean Option C (or A as a first step): add tombstones to the sync mirror, written at the same point we publish NIP-09, so both channels agree. It's the smallest change that actually converges, keeps the offline resilience, and the merge logic stays unit-testable in computeMergeUpdates.

Acceptance criteria

• Deleting a vault doc / API key on one device removes it on all synced devices and it does not come back.
• A doc legitimately re-created with the same path after deletion (newer updatedAt than the tombstone) is not suppressed.
• Tombstone storage is bounded (TTL prune) and counted against the sync quota budget (see setSyncRespectingQuota).
• Regression tests in test/sync-merge.test.js covering: delete-propagates, no-resurrection-on-repush, recreate-after-delete wins, tombstone older than doc is ignored.

Affected files

• src/utilities/sync-manager.js (buildSyncPayload, computeMergeUpdates)
• src/utilities/vault-store.js (deleteDocumentLocal, and a new tombstone writer)
• src/background.js (vault.delete, apikeys.delete — emit tombstone when publishing NIP-09)
• test/sync-merge.test.js (new cases)

Context gathered in PR #2.