From e79ab7521e96c88c655286962ff68a65c4f6b11a Mon Sep 17 00:00:00 2001 From: "Khokon M." <50947615+khokonm@users.noreply.github.com> Date: Thu, 25 Jun 2026 15:41:39 +0600 Subject: [PATCH] fix(sync): requester never shown approval prompt + backfill initialized flag for legacy devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After #11, the existing origin device had no per-chain initialized flag (it was paired before the flag existed), so it was treated as needsTransfer=true and auto-requested too — causing the brand-new requesting device to be shown an approve/deny prompt for its own join. Fixes: - One-time legacy backfill in startSync: a device that already holds notes is grandfathered as initialized (it's the chain's source of truth) so it stops auto-requesting. Empty devices still request normally. Marker-gated. - Hard guards regardless of flag state: a device that still needs an initial transfer ignores transfer-requested (nothing to send), never approves its own request, and never requests a transfer from itself (client + server reject). --- server/src/index.ts | 6 ++++++ src/utils/sync.ts | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/server/src/index.ts b/server/src/index.ts index 1963753..58805f2 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -790,6 +790,12 @@ function handleMessage(client: Client, msg: Record, ip: string) return } + // A device can never transfer from itself. + if (targetDeviceId === deviceId) { + client.ws.send(JSON.stringify({ type: 'error', message: 'Cannot request a transfer from this same device' })) + return + } + // Verify target device exists in this room const targetDevice = getDevice(roomId, targetDeviceId) if (!targetDevice) { diff --git a/src/utils/sync.ts b/src/utils/sync.ts index 59e9099..e098f91 100644 --- a/src/utils/sync.ts +++ b/src/utils/sync.ts @@ -52,6 +52,8 @@ const SYNC_TOMBSTONES_KEY = 'notes-sync-tombstones' // transfer?" — note count and the server-side cursor are both unreliable // proxies (a fresh device can have a seed note; the origin's cursor is 0). const SYNC_INIT_ROOM_KEY = 'notes-sync-initialized-room' +// One-time marker so the legacy backfill below runs at most once per device. +const SYNC_INIT_MIGRATED_KEY = 'notes-sync-init-migrated-v1' // How long to keep tombstones around. A device that's been offline longer // than this and then re-broadcasts an old note will be allowed to resurrect @@ -211,6 +213,25 @@ function needsInitialTransfer(): boolean { return !isInitializedForRoom(getRoomId()) } +/** + * One-time backfill for devices that were paired BEFORE the per-chain + * initialized flag existed (they have no flag yet). A device that already holds + * notes is the source of truth for its chain and must never demand a transfer — + * mark it initialized. A device with no notes is genuinely empty and is left to + * request a transfer as normal. Runs at most once (gated by a marker that is + * set unconditionally), so a device that legitimately needs a transfer is never + * grandfathered later just because the user typed a note before approving. + */ +function migrateInitFlagIfNeeded(): void { + if (localStorage.getItem(SYNC_INIT_MIGRATED_KEY)) return + localStorage.setItem(SYNC_INIT_MIGRATED_KEY, '1') + const roomId = getRoomId() + if (!roomId) return + if (isInitializedForRoom(roomId)) return + const hasNotes = (callbacks?.getNotes()?.length ?? 0) > 0 + if (hasNotes) markInitializedForRoom(roomId) +} + // ── Tombstones ──────────────────────────────────────────────────────────── // // When a note is deleted we record { noteId -> deletedAt }. Any incoming @@ -688,6 +709,14 @@ async function handleServerMessage(msg: Record): Promise const requesterName = msg.requesterName as string const requesterId = msg.requesterId as string + // Guard: a device that itself still needs an initial transfer has no + // notes to give, so it must never be asked to approve one. And we never + // approve our own request. Without these guards a brand-new device could + // be shown an approve/deny prompt for its own join — nonsensical. + if (needsInitialTransfer() || requesterId === getDeviceId()) { + break + } + updateState({ pendingTransfer: { transferId, requesterId, requesterName }, }) @@ -1202,6 +1231,10 @@ export function startSync(cbs: SyncCallbacks): void { stopSync() if (!isSyncEnabled()) return callbacks = cbs + // Grandfather pre-existing devices (paired before the initialized flag) so a + // device that already holds the notes isn't wrongly treated as needing a + // transfer (which made it auto-request and prompt the brand-new device). + migrateInitFlagIfNeeded() updateState({ enabled: true, status: 'connecting' }) loadSyncCode().then(() => connect()).catch(() => {}) } @@ -1284,6 +1317,8 @@ export async function denyTransfer(transferId: number): Promise { * Used by the new-device onboarding picker. */ export async function requestTransferFromDevice(targetDeviceId: string): Promise { + // Never request a transfer from ourselves. + if (targetDeviceId === getDeviceId()) return if (ws?.readyState !== WebSocket.OPEN) { updateState({ error: 'Not connected to the sync server. Try again in a moment.' }) return