fix(sync): let a re-share lift the unshare block it wrote

Re-sharing an event into a group it was unshared from was refused on every
member's device except the one that made the edit, permanently (TODO #141).

The group-scoped tombstone `deletedInGroup:{gid}:{eventId}` had three
references in the whole codebase - the import, the read in isEventTombstoned
and the write in apply(). Nothing ever deleted it. And the write sits inside
apply()'s `isRemote` guard, so the device that authored the unshare never wrote
one at all while every other member did. Re-share later and the author sees the
event immediately, because putEvent writes the local row directly, while every
other member's mirrorToLocal refuses the put for good.

A put into group G is a re-share into G, so applying one now lifts the block
before mirroring. The one thing that must not happen is a stale put - authored
before the unshare, linearised after it - resurrecting the event, so the clear
is ordered against the unshare rather than unconditional.

Ordering uses the unshare's AUTHORED time, added to the del op as `ts` and
stored on the tombstone as `delAt`. The tombstone's own `ts` cannot serve: it is
the applying device's clock at apply time, so comparing an author's `updatedAt`
against it would compare two machines' clocks. In the case that actually
happens - one person unshares and later re-shares - both timestamps come from
the same device.

When the two cannot be compared (a del from a build predating `delAt`, which
includes every tombstone already on disk, or a put with no `updatedAt`) the
clear goes ahead deliberately. The failure modes are not symmetric: refusing
leaves an event permanently invisible on every device but one, which is this bug
and the same class as #122's data loss, while clearing wrongly resurrects an
unshared event, which is visible and one tap to undo.

eventIdFromKey moves to src/lib/eventTombstone.js so the id derivation the del
branch uses when WRITING the tombstone is the one the re-share uses when
clearing it, pinned by a test rather than by coincidence.

291 unit tests, 13 new, including the stale-put case that is the whole reason
for the comparison, that the tombstone's own ts is not used for ordering, and
that a global delete still wins after a scoped clear.

Behavioural proof on two peers is NOT yet done and is the next step: #141 notes
a single peer cannot reproduce this by construction, since the tombstone is only
written on the isRemote path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HG8ayyquJuDMPSVLKDQKVh
This commit is contained in:
Your Name
2026-07-27 07:56:58 -05:00
co-authored by Claude Opus 5
parent 7c94f60b62
commit ab687b61e4
3 changed files with 213 additions and 15 deletions
+39 -15
View File
@@ -14,6 +14,7 @@ const { canonicalize, signMessage, verifySignature } = require('./lib/sign.js')
const { rekeyGroup: _rekeyGroupLib } = require('./lib/rekey.js')
const {
groupDeletedKey, isGroupScopedDelete, shouldBlockMirror, remainingGroupsAfterUnshare,
shouldClearScopedTombstone, eventIdFromKey,
} = require('./lib/eventTombstone.js')
const { planEventWrite, personalAppendValue } = require('./lib/eventMove.js')
const { SEEDER_PAIR_SCAN_TIMEOUT_MS } = require('./lib/seederPairTiming.js')
@@ -443,19 +444,6 @@ const NS = {
deleted: 'deleted:',
}
// Extract the event id from an "events:{date}:{eventId}" key. Cannot use
// split(':').pop() because shadow ids contain colons (e.g.
// "shadow:src:fwd:gid"), which would return the last colon segment ("gid")
// instead of the full shadow id. Skips the "events:" prefix and the
// "YYYY-MM-DD:" date segment.
function eventIdFromKey (key) {
const first = key.indexOf(':')
if (first < 0) return key
const second = key.indexOf(':', first + 1)
if (second < 0) return key.slice(first + 1)
return key.slice(second + 1)
}
// Strip occurrence + version suffixes to derive the series-root id from any
// occurrence id. For one-off events and the series root itself this is a
// no-op. Reminders are stored under the series-root key so all occurrences
@@ -1487,6 +1475,23 @@ async function isEventTombstoned (eventId, groupId) {
return shouldBlockMirror({ globalTombstone, scopedTombstone })
}
// Lift the group-scoped unshare block for `eventId` in `groupId`, if this put is
// entitled to (TODO #141). Decision is pure (src/lib/eventTombstone.js); this
// just supplies the stored tombstone and the put's authored time.
//
// The event id is derived the same way the del branch derives it when WRITING
// the tombstone, so the key cleared here is exactly the key that exists on disk.
// Do not switch this to `value.id` on the assumption they agree: they do for
// every shape we ship, and there is a test pinning that, but the two are
// computed differently and only the writer's form is guaranteed to match.
async function clearScopedTombstoneOnReshare (groupId, eventId, putUpdatedAt) {
const key = groupDeletedKey(groupId, eventId)
const node = await db.get(key).catch(() => null)
if (!shouldClearScopedTombstone({ tombstone: node?.value, putUpdatedAt })) return
await db.del(key).catch(() => {})
console.log('[#141] re-share cleared the scoped unshare tombstone for', eventId, 'in', groupId)
}
// Remove `groupId` from a local event's groups[]. Deletes the row only once no
// groups remain — the event has then left every group it was shared into. Used
// by the group-scoped del branch in apply() so moving an event between groups
@@ -5039,7 +5044,11 @@ async function syncPutEvent (groupId, event) {
async function syncDeleteEvent (groupId, eventId, date, updatedByName, updatedById, recurrenceId, eventTitle, scope) {
const base = bases.get(groupId)
if (!base) throw new Error('Not in group: ' + groupId)
const payload = { op: 'del', type: 'event', key: 'events:' + date + ':' + eventId, updatedByName: updatedByName || 'Someone', updatedById: updatedById || '' }
// `ts` is the unshare's AUTHORED time. It rides every del so a later re-share
// can tell whether it postdates the unshare (TODO #141); the tombstone's own
// `ts` cannot answer that, being the applying device's clock. Additive: peers
// on older builds ignore it, and its absence is handled explicitly.
const payload = { op: 'del', type: 'event', key: 'events:' + date + ':' + eventId, ts: Date.now(), updatedByName: updatedByName || 'Someone', updatedById: updatedById || '' }
if (recurrenceId) payload.recurrenceId = recurrenceId
if (eventTitle) payload.eventTitle = eventTitle
if (scope === 'group') payload.scope = 'group'
@@ -5769,6 +5778,14 @@ function makeApply (groupId) {
}
}
await view.put(val.key, viewValue)
// TODO #141 - a put into this group IS a re-share into it, so lift the
// group-scoped unshare block before mirroring rather than after. Until
// this, nothing anywhere deleted that key, and because it is written
// only inside the `isRemote` guard below, the author of the unshare
// kept the event while every other member lost it for good.
if (val.type === 'event') {
await clearScopedTombstoneOnReshare(groupId, eventIdFromKey(val.key), viewValue?.updatedAt)
}
// Always mirror so local DB has latest invitees list — listEvents filters at read time.
await mirrorToLocal(val.type, val.key, viewValue, groupId)
// Phase 5: identity-level kick mirror. For every removed member with
@@ -6181,7 +6198,14 @@ function makeApply (groupId) {
// which is exactly the data loss TODO #122 fixes.
await db.put(
isGroupScopedDel ? groupDeletedKey(groupId, eventId) : NS.deleted + eventId,
{ ts: Date.now(), ...(isGroupScopedDel ? { groupId } : {}) },
{
ts: Date.now(),
// Authored unshare time, so a later re-share can be ordered against
// it without comparing two devices' clocks (TODO #141). Absent when
// the del came from a build predating it.
...(isGroupScopedDel && typeof val.ts === 'number' ? { delAt: val.ts } : {}),
...(isGroupScopedDel ? { groupId } : {}),
},
).catch(() => {})
}
}
+58
View File
@@ -12,6 +12,25 @@
'use strict'
// Extract the event id from an "events:{date}:{eventId}" key. Cannot use
// split(':').pop() because shadow ids contain colons (e.g.
// "shadow:src:fwd:gid"), which would return the last colon segment ("gid")
// instead of the full shadow id. Skips the "events:" prefix and the
// "YYYY-MM-DD:" date segment.
//
// Lives here rather than in bare.js because the tombstone decisions depend on
// it: the del branch derives the id this way when WRITING a scoped tombstone,
// so a re-share must derive it the same way to clear the key that actually
// exists on disk (TODO #141). Keeping the two in one module is what lets a test
// pin them together.
function eventIdFromKey (key) {
const first = key.indexOf(':')
if (first < 0) return key
const second = key.indexOf(':', first + 1)
if (second < 0) return key.slice(first + 1)
return key.slice(second + 1)
}
// Per-group tombstone key. Deliberately NOT under the `deleted:` prefix: the
// retention sweeps iterate that prefix and parse everything after it as an event
// id, so a groupId sitting in that position would be read back as one.
@@ -35,6 +54,43 @@ function shouldBlockMirror ({ globalTombstone, scopedTombstone }) {
return !!scopedTombstone
}
// TODO #141 - when may a re-share clear the group-scoped tombstone?
//
// The tombstone above is written once and, before this, deleted nowhere: three
// references existed in the whole codebase, the import, the read and the write.
// Worse, the write sits inside apply()'s `isRemote` guard, so the device that
// authored the unshare never wrote one while every OTHER member did. Re-share
// the event into that group later and the author sees it fine (putEvent writes
// the local row directly) while every other member's mirrorToLocal refuses the
// put permanently. The event never comes back for them.
//
// A put into group G IS a re-share into G, so applying one should lift the
// block. The only thing that must not happen is a STALE put - one authored
// before the unshare and linearised after it - resurrecting the event. Hence
// the comparison, which is the same last-write-wins rule the rest of apply()
// uses.
//
// `delAt` is the unshare's AUTHORED time, carried on the del op itself. It
// matters that it is not the tombstone's own `ts`: that is the applying
// device's clock at apply time, so comparing an author's `updatedAt` against it
// would be comparing two different machines' clocks. In the common case - one
// person unshares and later re-shares - `delAt` and `putUpdatedAt` come from
// the same device, so the comparison is exact.
//
// When the two cannot be compared (an op from a build predating `delAt`, or a
// put with no `updatedAt`) this deliberately CLEARS. The two failure modes are
// not symmetric: refusing to clear leaves an event permanently invisible on
// every device but one, which is the bug being fixed here and the same class as
// the data loss in #122, while clearing wrongly resurrects an unshared event,
// which is visible and one tap to undo. Prefer the visible failure.
function shouldClearScopedTombstone ({ tombstone, putUpdatedAt }) {
if (!tombstone) return false
const delAt = tombstone.delAt
if (typeof delAt !== 'number' || typeof putUpdatedAt !== 'number') return true
// `>=` not `>`: same-millisecond ties go to the re-share, per the asymmetry above.
return putUpdatedAt >= delAt
}
// Groups left after unsharing from `groupId`. An empty result means the event has
// left every group it was shared into and the local row can be dropped.
function remainingGroupsAfterUnshare (groups, groupId) {
@@ -42,8 +98,10 @@ function remainingGroupsAfterUnshare (groups, groupId) {
}
module.exports = {
eventIdFromKey,
groupDeletedKey,
isGroupScopedDelete,
shouldBlockMirror,
shouldClearScopedTombstone,
remainingGroupsAfterUnshare,
}
+116
View File
@@ -7,6 +7,8 @@ const {
groupDeletedKey,
isGroupScopedDelete,
shouldBlockMirror,
shouldClearScopedTombstone,
eventIdFromKey,
remainingGroupsAfterUnshare,
} = require('../src/lib/eventTombstone.js')
@@ -77,3 +79,117 @@ test('remainingGroupsAfterUnshare tolerates missing groups[]', () => {
test('remainingGroupsAfterUnshare removes duplicates of the same group', () => {
assert.deepEqual(remainingGroupsAfterUnshare([GROUP_A, GROUP_A, GROUP_B], GROUP_A), [GROUP_B])
})
// ── shouldClearScopedTombstone (TODO #141) ────────────────────────────────
// The tombstone was write-once and delete-never, and written only on devices
// that did NOT author the unshare. So re-sharing an event into a group it had
// been unshared from came back for the editor and for nobody else, forever.
test('THE #141 REGRESSION: a re-share after the unshare lifts the block', () => {
// The whole bug in one line. Without this the event never returns for any
// member except the one who made the edit.
assert.equal(shouldClearScopedTombstone({
tombstone: { delAt: 1000, ts: 1000, groupId: GROUP_A },
putUpdatedAt: 2000,
}), true)
})
test('a STALE put authored before the unshare must NOT resurrect it', () => {
// The case that makes the timestamp comparison necessary rather than just
// deleting the key on any put: Autobase can linearise a put authored earlier
// by another writer AFTER the del has already applied.
assert.equal(shouldClearScopedTombstone({
tombstone: { delAt: 2000, ts: 2000, groupId: GROUP_A },
putUpdatedAt: 1000,
}), false)
})
test('a same-millisecond tie goes to the re-share', () => {
assert.equal(shouldClearScopedTombstone({
tombstone: { delAt: 1500, ts: 1500 }, putUpdatedAt: 1500,
}), true)
})
test('nothing to clear when no tombstone exists', () => {
assert.equal(shouldClearScopedTombstone({ tombstone: null, putUpdatedAt: 1 }), false)
assert.equal(shouldClearScopedTombstone({ tombstone: undefined, putUpdatedAt: 1 }), false)
})
test('an unorderable pair clears, because a lost event is the worse failure', () => {
// A del from a build predating `delAt`, or a put with no updatedAt. Both are
// real: `delAt` is new in this change, so every tombstone already on disk
// lacks it. Clearing is the deliberate choice - see the comment on the
// function. Refusing would leave those events invisible for good.
assert.equal(shouldClearScopedTombstone({
tombstone: { ts: 5000, groupId: GROUP_A }, // pre-#141 shape, no delAt
putUpdatedAt: 1000,
}), true)
assert.equal(shouldClearScopedTombstone({
tombstone: { delAt: 5000 }, putUpdatedAt: undefined,
}), true)
})
test('the tombstone`s own ts is NOT used for ordering', () => {
// `ts` is the applying device's clock at apply time; `delAt` is the author's.
// Using ts would compare two machines' clocks. Pin it: a tombstone whose ts
// is far in the future must not block a re-share that postdates the delAt.
assert.equal(shouldClearScopedTombstone({
tombstone: { delAt: 1000, ts: 9_999_999 },
putUpdatedAt: 2000,
}), true)
})
test('clearing is per group: the other group`s tombstone is untouched', () => {
// Not a property of this function alone but of how it is keyed, so assert the
// pairing that matters: the caller looks up by groupDeletedKey(group, event).
assert.notEqual(groupDeletedKey(GROUP_A, EVENT), groupDeletedKey(GROUP_B, EVENT))
})
test('a cleared tombstone stops blocking the mirror', () => {
// The two decisions have to compose: clearing is only meaningful because
// shouldBlockMirror reads the absence.
assert.equal(shouldBlockMirror({ globalTombstone: null, scopedTombstone: null }), false)
})
test('a GLOBAL delete still wins after a re-share clears the scoped one', () => {
// Deleting for everyone must not become undoable by a stray put. The scoped
// clear cannot touch the global key, and shouldBlockMirror checks global first.
assert.equal(shouldBlockMirror({ globalTombstone: { ts: 1 }, scopedTombstone: null }), true)
})
// ── writer and clearer must derive the same id (TODO #141) ────────────────
// The del branch writes the tombstone keyed by eventIdFromKey(op.key); the
// re-share clears it the same way. mirrorToLocal, meanwhile, READS the block
// keyed by value.id. All three have to agree or the key written is not the key
// cleared, and the fix would silently do nothing for the shapes that disagree.
test('eventIdFromKey survives the colons in a shadow id', () => {
// The reason split(':').pop() is wrong: it would return 'gid'.
assert.equal(eventIdFromKey('events:2026-07-28:shadow:src:fwd:gid'), 'shadow:src:fwd:gid')
})
test('eventIdFromKey matches value.id for every event shape we store', () => {
// Pins the assumption the clear depends on. If a future key layout breaks
// this, the tombstone written on unshare stops matching the one cleared on
// re-share and #141 comes back silently.
const shapes = [
{ key: 'events:2026-07-28:evt-1', id: 'evt-1' },
{ key: 'events:2026-07-28:shadow:src:fwd:gid', id: 'shadow:src:fwd:gid' },
{ key: 'events:2026-07-28:evt-1:2026-08-01', id: 'evt-1:2026-08-01' },
]
for (const { key, id } of shapes) assert.equal(eventIdFromKey(key), id, key)
})
test('eventIdFromKey is total: degenerate keys do not throw', () => {
assert.equal(eventIdFromKey('evt-1'), 'evt-1')
assert.equal(eventIdFromKey('events:evt-1'), 'evt-1')
})
test('the key cleared on re-share is the key written on unshare', () => {
// End to end over the pair, on the shape most likely to break it.
const opKey = 'events:2026-07-28:shadow:src:fwd:gid'
const written = groupDeletedKey(GROUP_A, eventIdFromKey(opKey))
const cleared = groupDeletedKey(GROUP_A, eventIdFromKey(opKey))
assert.equal(written, cleared)
assert.equal(written, 'deletedInGroup:ga:shadow:src:fwd:gid')
})