diff --git a/server/src/index.ts b/server/src/index.ts index 9074072..1963753 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -16,7 +16,7 @@ * * WebSocket protocol (JSON messages): * Client → Server: - * { type: 'join', roomId, token, deviceId, deviceName, hasData } + * { type: 'join', roomId, token, deviceId, deviceName, needsTransfer } * { type: 'push-op', payload (base64) } * { type: 'ack', cursor } * { type: 'request-transfer' } @@ -641,18 +641,17 @@ function handleMessage(client: Client, msg: Record, ip: string) const room = getRoom(roomId) room.add(client) - // A device "needs a transfer" only when it has NO notes of its own yet and - // there is another device in the chain to copy from. The client reports - // whether it already holds local notes via `hasData` — the server can't - // see the (E2E-encrypted) data itself. Relying on `cursor === 0` alone was - // wrong: the device that generated the sync code owns all the notes but - // hasn't pushed any ops, so its cursor is still 0. That misclassified the - // notes-holding device as a transfer requester, so both devices showed - // "Choose a source device" and the approval prompt never appeared anywhere. - const hasData = msg.hasData === true + // Whether this device needs an initial transfer is the CLIENT's own + // declaration (`needsTransfer`), driven by persisted intent: it generated + // the code (origin → false) or entered an existing code and hasn't yet + // pulled the notes (→ true). The server can't infer this — the data is + // E2E-encrypted, note count is unreliable (fresh devices can have a seed + // note), and `cursor === 0` is true for the origin too. We only honor the + // request when there's actually another device to copy from. + const clientNeedsTransfer = msg.needsTransfer === true const allDevices = getDevices(roomId) const otherDevices = allDevices.filter(d => d.deviceId !== deviceId) - const isNewDevice = !hasData && device.cursor === 0 && otherDevices.length > 0 + const isNewDevice = clientNeedsTransfer && otherDevices.length > 0 const maxSeq = getMaxSeq(roomId) const hasPendingOps = device.cursor < maxSeq diff --git a/src/components/SettingsPanel.tsx b/src/components/SettingsPanel.tsx index 6bd6e7c..16c65e7 100644 --- a/src/components/SettingsPanel.tsx +++ b/src/components/SettingsPanel.tsx @@ -1108,7 +1108,8 @@ function SyncPage({ syncState, onTriggerSync, onSyncEnabled, onSyncDisabled }: S try { const result = await generateSyncCode() setGeneratedCode(result.syncCode) - enableSync(result.syncCode, result.roomId, result.token) + // We created this chain — we are the source of truth, no transfer needed. + enableSync(result.syncCode, result.roomId, result.token, { initialized: true }) setEnabled(true) onSyncEnabled() } catch (e) { @@ -1124,7 +1125,9 @@ function SyncPage({ syncState, onTriggerSync, onSyncEnabled, onSyncDisabled }: S setError(null) try { const result = await validateSyncCode(syncCodeInput) - enableSync(syncCodeInput, result.roomId, result.token) + // We're joining an existing chain — we must pull the notes from an + // existing device before we're a full member. + enableSync(syncCodeInput, result.roomId, result.token, { initialized: false }) setEnabled(true) setMode(null) setSyncCodeInput('') diff --git a/src/utils/sync.ts b/src/utils/sync.ts index c7d9194..59e9099 100644 --- a/src/utils/sync.ts +++ b/src/utils/sync.ts @@ -44,6 +44,14 @@ const SYNC_CURSOR_KEY = 'notes-sync-cursor' const SYNC_QUEUE_KEY = 'notes-sync-queue' const SYNC_LAST_KEY = 'notes-sync-last' const SYNC_TOMBSTONES_KEY = 'notes-sync-tombstones' +// Records the roomId this device has completed its initial bootstrap for. A +// device is "initialized" for a chain when it either GENERATED the code (it is +// the origin / source of truth) or finished RECEIVING an initial transfer. Any +// other state means it still needs to pull the existing notes before it can be +// treated as a full member. This is the authoritative signal for "do I need a +// 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' // 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 @@ -184,6 +192,25 @@ function setCursor(cursor: number): void { localStorage.setItem(SYNC_CURSOR_KEY, String(cursor)) } +// ── Initial-transfer bootstrap state ─────────────────────────────────────── + +function getRoomId(): string | null { + return localStorage.getItem(SYNC_ROOM_KEY) +} + +function isInitializedForRoom(roomId: string | null): boolean { + return !!roomId && localStorage.getItem(SYNC_INIT_ROOM_KEY) === roomId +} + +function markInitializedForRoom(roomId: string | null): void { + if (roomId) localStorage.setItem(SYNC_INIT_ROOM_KEY, roomId) +} + +/** True when this device still needs an initial transfer for the active chain. */ +function needsInitialTransfer(): boolean { + return !isInitializedForRoom(getRoomId()) +} + // ── Tombstones ──────────────────────────────────────────────────────────── // // When a note is deleted we record { noteId -> deletedAt }. Any incoming @@ -435,14 +462,14 @@ async function refreshJoinToken(): Promise<{ roomId: string; token: string } | n return getJoinToken() } -// Build the `join` payload. `hasData` tells the server whether this device -// already holds notes locally. The server can't inspect our (E2E-encrypted) -// data, so without this hint it inferred "new device" purely from cursor === 0 -// — which wrongly flagged the device that *generated* the sync code (and owns -// all the notes, but hasn't pushed any ops yet, so cursor is still 0) as a -// device needing a transfer. That left both devices stuck on "Choose a source -// device" with neither ever showing the approval prompt. A device that has -// local notes is a source/approver, never a transfer requester. +// Build the `join` payload. `needsTransfer` is the device's own declaration of +// whether it still needs an initial bootstrap for this chain — set from +// persisted intent (generated the code, or finished a transfer), NOT inferred +// from note count or the server cursor. Both of those were unreliable: a fresh +// joining device often has a seed/empty note (so "has data" was true and it was +// wrongly marked established → it never asked to sync), and the origin's cursor +// is 0 (so it was wrongly marked new). The server trusts this declaration to +// decide the device's role. function buildJoinPayload(roomId: string, token: string): string { return JSON.stringify({ type: 'join', @@ -450,7 +477,7 @@ function buildJoinPayload(roomId: string, token: string): string { token, deviceId: getDeviceId(), deviceName: getDeviceName(), - hasData: (callbacks?.getNotes()?.length ?? 0) > 0, + needsTransfer: !isInitializedForRoom(roomId), }) } @@ -523,13 +550,29 @@ function scheduleReconnect(): void { }, 5000) } +/** + * If this device still needs its initial copy of the notes and an existing + * device is online, automatically ask that device for a transfer. This is what + * makes the approve/deny prompt appear on the existing device without the user + * having to manually pick a source on the new one. Self-guards so it never + * fires while a transfer is already pending/in progress. + */ +function maybeAutoRequestTransfer(): void { + if (!needsInitialTransfer()) return + if (currentState.status === 'transferring') return + if (currentState.pendingTransfer) return + if (currentState.awaitingDeviceId) return + if (ws?.readyState !== WebSocket.OPEN) return + const onlineOther = currentState.devices.find(d => !d.isSelf && d.online) + if (onlineOther) void requestTransferFromDevice(onlineOther.deviceId) +} + async function handleServerMessage(msg: Record): Promise { const syncCode = getSyncCode() if (!syncCode) return switch (msg.type) { case 'welcome': { - const needsTransfer = msg.needsTransfer as boolean const cursor = msg.cursor as number const selfId = (msg.selfDeviceId as string) || getDeviceId() const rawDevices = (msg.devices as Array<{ @@ -543,6 +586,13 @@ async function handleServerMessage(msg: Record): Promise isSelf: d.deviceId === selfId, })) + // Authoritative "do we still need a transfer?" is our own persisted + // bootstrap state — not the server's hint (which only reflects whether + // other devices exist). We may still need a transfer even when the server + // can't tell, and we must NOT think we're done just because we happen to + // hold a stray local note. + const needsTransfer = needsInitialTransfer() + updateState({ status: needsTransfer ? 'awaiting-source' : 'idle', deviceCount: msg.deviceCount as number, @@ -565,9 +615,12 @@ async function handleServerMessage(msg: Record): Promise // every note on every reconnect was the main source of duplicate // "conflict copy" notes. await flushQueue() + } else { + // New device — automatically request the initial copy from an online + // device so its approval prompt fires immediately. The picker remains + // available if the user wants to choose a specific source instead. + maybeAutoRequestTransfer() } - // New devices wait for the user to pick a source device explicitly - // (handled by UI calling requestTransferFromDevice). break } @@ -693,7 +746,10 @@ async function handleServerMessage(msg: Record): Promise // If this was the last chunk, mark transfer complete if (chunkIndex + 1 >= totalChunks) { ws?.send(JSON.stringify({ type: 'transfer-complete', transferId })) - updateState({ status: 'idle', transferProgress: undefined }) + // We now hold the chain's notes — record this so we never ask for + // another initial transfer (and join as a full member from now on). + markInitializedForRoom(getRoomId()) + updateState({ status: 'idle', transferProgress: undefined, needsTransfer: false }) // Any local edits made during the transfer were already pushed // live via syncPushSingle (ws was open). Just flush anything that // happened to land in the queue. @@ -748,6 +804,10 @@ async function handleServerMessage(msg: Record): Promise if (currentState.awaitingDeviceId === joinedId && ws?.readyState === WebSocket.OPEN) { ws.send(JSON.stringify({ type: 'request-transfer-from', targetDeviceId: joinedId })) updateState({ status: 'transferring', error: null, awaitingDeviceId: null, awaitingDeviceName: null }) + } else { + // A device came online and we still need our initial copy — ask it now + // so its approval prompt appears without manual picking. + maybeAutoRequestTransfer() } break } @@ -1086,12 +1146,33 @@ export async function loadSyncPassword(): Promise { return loadSyncCode() } -export function enableSync(syncCode: string, roomId?: string, token?: string): void { +/** + * Enable sync for a chain. + * + * `opts.initialized` declares whether this device already holds the chain's + * notes and therefore does NOT need an initial transfer: + * - true → this device GENERATED the code (it is the origin / source). + * - false → this device ENTERED an existing code (it must pull the notes + * from an existing device before it's a full member). + */ +export function enableSync( + syncCode: string, + roomId?: string, + token?: string, + opts?: { initialized?: boolean }, +): void { localStorage.setItem(SYNC_ENABLED_KEY, '1') cachedSyncCode = normalizeSyncCode(syncCode) secureSet(SYNC_CODE_KEY, cachedSyncCode).catch(() => {}) if (roomId) localStorage.setItem(SYNC_ROOM_KEY, roomId) if (token) localStorage.setItem(SYNC_TOKEN_KEY, token) + if (opts?.initialized && roomId) { + markInitializedForRoom(roomId) + } else if (opts?.initialized === false && roomId && !isInitializedForRoom(roomId)) { + // Joining a chain we haven't bootstrapped — make sure any stale flag from a + // different room doesn't suppress the initial transfer. + localStorage.removeItem(SYNC_INIT_ROOM_KEY) + } } export function disableSync(): void { @@ -1105,6 +1186,7 @@ export function disableSync(): void { localStorage.removeItem(SYNC_QUEUE_KEY) localStorage.removeItem(SYNC_LAST_KEY) localStorage.removeItem(SYNC_TOMBSTONES_KEY) + localStorage.removeItem(SYNC_INIT_ROOM_KEY) cachedSyncCode = null stopSync() }