From e07b5c3217ddeb1831ac3879698db1830fb3e4ae Mon Sep 17 00:00:00 2001 From: tianma-if Date: Mon, 10 Aug 2026 01:46:31 +0800 Subject: [PATCH] fix(desktop): prevent false autosave conflicts --- apps/web/src/components/EditorPane.tsx | 26 ++++++++++++++- apps/web/src/lib/desktop-sync.test.mjs | 36 ++++++++++++++++++++- apps/web/src/lib/desktop-sync.ts | 45 ++++++++++++++++++++++---- crates/desktop-sidecar/src/main.rs | 6 +++- scripts/test-desktop-sidecar.mjs | 1 + 5 files changed, 105 insertions(+), 9 deletions(-) diff --git a/apps/web/src/components/EditorPane.tsx b/apps/web/src/components/EditorPane.tsx index cfc0e16c..bd819401 100644 --- a/apps/web/src/components/EditorPane.tsx +++ b/apps/web/src/components/EditorPane.tsx @@ -2088,9 +2088,33 @@ const RichEditorPane = ({ useEffect(() => { const handleSyncCompleted = (event: Event) => { - const result = (event as CustomEvent<{ failed?: number; conflicted?: number }>).detail; + const result = (event as CustomEvent<{ + failed?: number; + conflicted?: number; + syncedMemos?: ReadonlyMap; + }>).detail; const memoId = memoRef.current?.id; + const syncedMemo = memoId ? result?.syncedMemos?.get(memoId) : null; + if (syncedMemo && memoRef.current?.id === syncedMemo.id) { + // Keep the live editor document, title, and tags intact while moving + // its concurrency base to the revision just acknowledged by this + // device. This closes the query-refresh window after an autosave. + memoRef.current = { + ...memoRef.current, + revision: syncedMemo.revision, + contentHash: syncedMemo.contentHash, + updatedAt: syncedMemo.updatedAt, + }; + if (editSessionRef.current?.memoId === syncedMemo.id) { + editSessionRef.current = { + ...editSessionRef.current, + baseRevision: syncedMemo.revision, + baseContentHash: syncedMemo.contentHash, + }; + } + } + if (memoId && (result?.conflicted ?? 0) > 0) { void localDb.syncQueue.get(getMemoUpdateQueueId(memoId)).then((item) => { if (!item || item.status !== "conflict" || memoRef.current?.id !== memoId) { diff --git a/apps/web/src/lib/desktop-sync.test.mjs b/apps/web/src/lib/desktop-sync.test.mjs index 430f2f8b..e342138a 100644 --- a/apps/web/src/lib/desktop-sync.test.mjs +++ b/apps/web/src/lib/desktop-sync.test.mjs @@ -1,6 +1,13 @@ import { describe, expect, test } from "bun:test"; -const { isStagedResourceReferenced, mergeMemoIdMappings, orderBootstrapNotebooks, rewriteStagedResource } = await import("./desktop-sync.ts"); +const { + isStagedResourceReferenced, + mergeMemoIdMappings, + mergeSyncedMemos, + orderBootstrapNotebooks, + resolveDesktopMemoSyncBase, + rewriteStagedResource, +} = await import("./desktop-sync.ts"); describe("desktop staged resource sync", () => { test("rewrites placeholders in memo JSON and markdown", () => { @@ -39,6 +46,17 @@ describe("desktop staged resource sync", () => { expect(retained.get("memo_local_1")).toBe("memo_remote_1"); }); + + test("keeps the latest acknowledged memo base across sync phases", () => { + const created = { id: "memo_remote_1", revision: 0 }; + const updated = { id: "memo_remote_1", revision: 1 }; + const retained = mergeSyncedMemos( + new Map([[created.id, created]]), + new Map([[updated.id, updated]]), + ); + + expect(retained.get("memo_remote_1")).toEqual(updated); + }); }); describe("desktop bootstrap sync", () => { @@ -54,3 +72,19 @@ describe("desktop bootstrap sync", () => { ]); }); }); + +describe("desktop memo sync base", () => { + test("repairs a legacy local autosave revision that is ahead of the cloud", () => { + expect(resolveDesktopMemoSyncBase( + { revision: 3, contentHash: "cloud-3" }, + { expectedRevision: 9, expectedContentHash: "local-autosave-9" }, + )).toEqual({ expectedRevision: 3, expectedContentHash: "cloud-3" }); + }); + + test("keeps a genuinely stale base so the server update remains protected", () => { + expect(resolveDesktopMemoSyncBase( + { revision: 9, contentHash: "cloud-9" }, + { expectedRevision: 3, expectedContentHash: "cloud-3" }, + )).toEqual({ expectedRevision: 3, expectedContentHash: "cloud-3" }); + }); +}); diff --git a/apps/web/src/lib/desktop-sync.ts b/apps/web/src/lib/desktop-sync.ts index 645ee017..36906a3e 100644 --- a/apps/web/src/lib/desktop-sync.ts +++ b/apps/web/src/lib/desktop-sync.ts @@ -164,6 +164,24 @@ const applyRemoteNotebook = async (notebook: DesktopRpcResponses["notebook.list" }); }; +export const resolveDesktopMemoSyncBase = ( + current: { revision: number; contentHash: string }, + expected: { expectedRevision: number; expectedContentHash: string }, +) => { + // Older desktop clients advanced the cached cloud revision for every local + // autosave. A base ahead of the actual server is therefore a local counter, + // not evidence of a concurrent remote edit. Rebase that impossible state so + // existing drafts recover automatically. Bases behind the server remain + // conflicts and keep the normal adopt-cloud/copy-draft protection. + if (expected.expectedRevision > current.revision) { + return { + expectedRevision: current.revision, + expectedContentHash: current.contentHash, + }; + } + return expected; +}; + const acknowledge = async ( item: DesktopOutboxItem, remoteMemo?: DesktopRpcResponses["memo.get"]["memo"], @@ -196,11 +214,12 @@ const syncOutboxItem = async (item: DesktopOutboxItem, stagedRewrites: StagedRes const memoId = String(payload.memoId ?? item.entityId); const editSessionResponse = await api.createMemoEditSession(memoId); const editSession = editSessionResponse.editSession; - const expectedBase = { + const queuedBase = { expectedRevision: Number(payload.expectedRevision), expectedContentHash: String(payload.expectedContentHash ?? ""), }; const currentBase = { revision: editSession.baseRevision, contentHash: editSession.baseContentHash }; + const expectedBase = resolveDesktopMemoSyncBase(currentBase, queuedBase); if (!isMemoSyncBaseCurrent(currentBase, expectedBase)) { throw new ApiRequestError( "Note changed before the offline draft could sync.", @@ -210,8 +229,8 @@ const syncOutboxItem = async (item: DesktopOutboxItem, stagedRewrites: StagedRes ); } const data = await api.updateMemo(memoId, { - expectedRevision: Number(payload.expectedRevision), - expectedContentHash: String(payload.expectedContentHash ?? ""), + expectedRevision: expectedBase.expectedRevision, + expectedContentHash: expectedBase.expectedContentHash, editSessionId: editSession.id, title: String(payload.title ?? ""), contentJson: payload.contentJson as TiptapDoc, @@ -354,6 +373,7 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S let conflicted = 0; const response = await request("sync.outbox.list", { limit: 100 }); const memoIdMappings = new Map(); + const syncedMemos = new Map(); for (const item of response.items) { if (onlyKinds && !onlyKinds.has(item.kind)) continue; try { @@ -361,6 +381,9 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S if (item.kind === "memo.create" && result && typeof result === "object" && "id" in result && typeof result.id === "string") { memoIdMappings.set(item.entityId, result.id); } + if (result && typeof result === "object" && "id" in result && typeof result.id === "string" && "contentJson" in result) { + syncedMemos.set(result.id, result as DesktopRpcResponses["memo.get"]["memo"]); + } synced += 1; } catch (error) { const conflict = error instanceof ApiRequestError @@ -370,7 +393,7 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S else failed += 1; } } - return { attempted: onlyKinds ? synced + failed + conflicted : response.items.length, synced, failed, conflicted, memoIdMappings }; + return { attempted: onlyKinds ? synced + failed + conflicted : response.items.length, synced, failed, conflicted, memoIdMappings, syncedMemos }; }; const applyBootstrap = async (page: SyncBootstrapResponse) => { @@ -437,6 +460,7 @@ export type DesktopSyncResult = { failed: number; conflicted: number; memoIdMappings: Map; + syncedMemos: Map; }; export const mergeMemoIdMappings = ( @@ -447,22 +471,30 @@ export const mergeMemoIdMappings = ( return target; }; +export const mergeSyncedMemos = (target: Map, source: ReadonlyMap) => { + for (const [memoId, memo] of source) target.set(memoId, memo); + return target; +}; + let activeSync: Promise | null = null; export const syncDesktopData = () => { if (activeSync) return activeSync; activeSync = (async () => { const memoIdMappings = new Map(); + const syncedMemos = new Map(); if (typeof navigator !== "undefined" && !navigator.onLine) { - return { attempted: 0, synced: 0, failed: 0, conflicted: 0, memoIdMappings }; + return { attempted: 0, synced: 0, failed: 0, conflicted: 0, memoIdMappings, syncedMemos }; } try { const creates = await syncOutbox([], new Set(["memo.create"])); mergeMemoIdMappings(memoIdMappings, creates.memoIdMappings); + mergeSyncedMemos(syncedMemos, creates.syncedMemos); await remapStagedResourceMemoIds(creates.memoIdMappings); const stagedResources = await syncStagedResources(creates.memoIdMappings); const outbox = await syncOutbox(stagedResources.rewrites); mergeMemoIdMappings(memoIdMappings, outbox.memoIdMappings); + mergeSyncedMemos(syncedMemos, outbox.syncedMemos); if (stagedResources.failed === 0 && outbox.failed === 0 && outbox.conflicted === 0 && creates.conflicted === 0) { await removeSyncedStagedResources(stagedResources.stagedIds); } @@ -476,6 +508,7 @@ export const syncDesktopData = () => { failed: creates.failed + outbox.failed + stagedResources.failed, conflicted: creates.conflicted + outbox.conflicted, memoIdMappings, + syncedMemos, }; } catch (error) { lastSyncFailed = true; @@ -483,7 +516,7 @@ export const syncDesktopData = () => { // A create may already have been acknowledged before a later upload or // pull failed. Preserve its id mapping so the UI cannot keep editing an // obsolete temporary id merely because the overall sync was partial. - return { attempted: 0, synced: 0, failed: 1, conflicted: 0, memoIdMappings }; + return { attempted: 0, synced: 0, failed: 1, conflicted: 0, memoIdMappings, syncedMemos }; } })().finally(() => { activeSync = null; }); return activeSync; diff --git a/crates/desktop-sidecar/src/main.rs b/crates/desktop-sidecar/src/main.rs index 42dfd4c9..4a4a0104 100644 --- a/crates/desktop-sidecar/src/main.rs +++ b/crates/desktop-sidecar/src/main.rs @@ -1017,7 +1017,11 @@ fn update_memo(database: &Connection, params: &Value) -> Result { if changed == 0 { return Err(format!("Memo not found or deleted: {memo_id}")); } - tx.execute("UPDATE memo_contents SET content_json = ?2, content_markdown = ?3, content_text = ?4, content_hash = ?5, revision = revision + 1, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE memo_id = ?1", rusqlite::params![memo_id, content_json_text, markdown, text, hash]).map_err(|e| e.to_string())?; + // `revision` is the last acknowledged cloud revision, not a counter for + // local autosaves. Advancing it here makes several saves on one device + // look newer than the cloud and produces a false revision conflict once + // the coalesced outbox item is synced. + tx.execute("UPDATE memo_contents SET content_json = ?2, content_markdown = ?3, content_text = ?4, content_hash = ?5, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE memo_id = ?1", rusqlite::params![memo_id, content_json_text, markdown, text, hash]).map_err(|e| e.to_string())?; tx.commit().map_err(|e| e.to_string())?; enqueue_change( database, diff --git a/scripts/test-desktop-sidecar.mjs b/scripts/test-desktop-sidecar.mjs index ca993cbd..75098fa3 100644 --- a/scripts/test-desktop-sidecar.mjs +++ b/scripts/test-desktop-sidecar.mjs @@ -107,6 +107,7 @@ await request("memo.update", updatePayload("latest autosave")); const coalescedUpdates = (await request("sync.outbox.list", { limit: 200 })).items.filter((item) => item.kind === "memo.update" && item.entityId === coalesced.memo.id); assert.equal(coalescedUpdates.length, 1, "offline autosaves should coalesce into one sidecar outbox item"); assert.equal(coalescedUpdates[0].payload.contentMarkdown, "latest autosave"); +assert.equal((await request("memo.get", { memoId: coalesced.memo.id })).memo.revision, 0, "local autosaves must not advance the acknowledged cloud revision"); const batchNotebook = (await request("notebook.create", { name: "Batch destination" })).notebook; const batchFirst = (await request("memo.create", { notebookId: inbox.id, title: "Batch one", contentMarkdown: "batch one", tags: ["batch-tag"] })).memo;