From 9f458c0aa30c913d2ac838265dc2b8307650d689 Mon Sep 17 00:00:00 2001 From: tianma-if Date: Sat, 22 Aug 2026 19:11:28 +0800 Subject: [PATCH] fix(sync): reconcile merged memo placeholders --- .../src/database-write-optimizations.test.mjs | 26 ++++++++++++++++ apps/api/src/index.ts | 31 +++++++++++++++++-- crates/desktop-sidecar/src/main.rs | 30 ++++++++++++++++-- scripts/test-desktop-sidecar.mjs | 29 ++++++++++++++++- 4 files changed, 111 insertions(+), 5 deletions(-) diff --git a/apps/api/src/database-write-optimizations.test.mjs b/apps/api/src/database-write-optimizations.test.mjs index 2675574c..6c200ae5 100644 --- a/apps/api/src/database-write-optimizations.test.mjs +++ b/apps/api/src/database-write-optimizations.test.mjs @@ -5,6 +5,7 @@ import { acquireMaintenanceLease, createMemoEditSession, fetchEdgeEverApp, + mergeMemosRecord, updateMemoRecord, } from "./index.ts"; @@ -83,6 +84,31 @@ const getSeedMemo = (sqlite) => sqlite.query( ).get(); describe("database write optimizations", () => { + test("returns the completed merge when the same source set is retried", async () => { + const { database, sqlite } = createDatabase(); + const first = getSeedMemo(sqlite); + const secondId = "memo_merge_retry_source"; + sqlite.query( + `INSERT INTO memos (id, workspace_id, notebook_id, title, excerpt, tags_json, created_by, updated_by) + VALUES (?, ?, ?, 'Retry source', 'retry body', '[]', 'user', 'user')`, + ).run(secondId, first.workspace_id, first.notebook_id); + sqlite.query( + `INSERT INTO memo_contents (memo_id, content_json, content_markdown, content_text, content_hash) + VALUES (?, '{"type":"doc","content":[]}', 'retry body', 'retry body', 'retry-hash')`, + ).run(secondId); + + const input = { memoIds: [first.id, secondId], title: "Idempotent merge" }; + const actor = { actorType: "user", actorId: "user_owner" }; + const completed = await mergeMemosRecord(database, first.workspace_id, input, actor, "owner"); + const retried = await mergeMemosRecord(database, first.workspace_id, input, actor, "owner"); + + expect(retried.id).toBe(completed.id); + expect(retried.sourceMemoIds).toEqual(input.memoIds); + expect(sqlite.query("SELECT COUNT(*) AS count FROM memos WHERE is_deleted = 0 AND title = 'Idempotent merge'").get().count).toBe(1); + expect(sqlite.query("SELECT COUNT(*) AS count FROM audit_events WHERE action = 'memo.merge' AND entity_id = ?").get(completed.id).count).toBe(1); + sqlite.close(); + }); + test("reuses a matching live edit session without another insert", async () => { const { database, sqlite } = createDatabase(); const memo = getSeedMemo(sqlite); diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index c76eee89..f497ed03 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -2922,7 +2922,7 @@ const moveMemosRecord = async ( return moveMemosToNotebook(db, workspaceId, memoIds, notebookId, actor, actorLabel); }; -const mergeMemosRecord = async ( +export const mergeMemosRecord = async ( db: D1Database, workspaceId: string, input: { memoIds: string[]; notebookId?: string; title?: string }, @@ -2944,7 +2944,7 @@ const mergeMemosRecord = async ( m.source_memo_ids, m.merge_source_count, m.merged_into_memo_id FROM memos m INNER JOIN memo_contents c ON c.memo_id = m.id - WHERE m.workspace_id = ? AND m.is_deleted = 0 AND m.id IN (${placeholders})` + WHERE m.workspace_id = ? AND m.id IN (${placeholders})` ) .bind(workspaceId, ...uniqueMemoIds) .all(); @@ -2953,6 +2953,33 @@ const mergeMemosRecord = async ( throw new AppError("missing_memos", "One or more memos cannot be merged.", 400); } + const activeRows = rows.results.filter((row) => !row.is_deleted); + if (activeRows.length !== uniqueMemoIds.length) { + // A desktop outbox retry can arrive after the first merge committed but + // before the client acknowledged its local placeholder. When every source + // now points at the same live merge result, return that result instead of + // treating the retry as a new merge. This makes the operation recoverable + // after a lost response without ever creating a second server-side memo. + const mergedTargetIds = new Set( + rows.results + .filter((row) => row.is_deleted && row.merged_into_memo_id) + .map((row) => row.merged_into_memo_id as string), + ); + if (activeRows.length === 0 && mergedTargetIds.size === 1) { + const [mergedTargetId] = mergedTargetIds; + const completedMerge = await getMemoDetail(db, workspaceId, mergedTargetId); + const completedSourceIds = new Set(completedMerge?.sourceMemoIds ?? []); + if ( + completedMerge + && completedMerge.sourceMemoIds.length === uniqueMemoIds.length + && uniqueMemoIds.every((memoId) => completedSourceIds.has(memoId)) + ) { + return completedMerge; + } + } + throw new AppError("missing_memos", "One or more memos cannot be merged.", 400); + } + if (input.notebookId && !(await getNotebook(db, workspaceId, input.notebookId))) { throw new AppError("not_found", "Target notebook not found", 404); } diff --git a/crates/desktop-sidecar/src/main.rs b/crates/desktop-sidecar/src/main.rs index fc41726d..a8466aa3 100644 --- a/crates/desktop-sidecar/src/main.rs +++ b/crates/desktop-sidecar/src/main.rs @@ -1287,9 +1287,35 @@ fn sync_outbox_ack(database: &Connection, params: &Value) -> Result memo.id === first.memo.id)); +const mergeOutbox = (await request("sync.outbox.list", { limit: 200 })).items.find((item) => item.kind === "memo.merge" && item.entityId === merged.memo.id); +assert.ok(mergeOutbox, "memo merge should be queued"); +const remoteMergedMemo = { ...merged.memo, id: "memo_remote_merged" }; +await request("sync.apply", { + changes: [{ entityType: "memo", operation: "upsert", entityId: remoteMergedMemo.id, memo: remoteMergedMemo, notebook: null }], +}); +await request("sync.outbox.ack", { id: mergeOutbox.id, remoteMemo: remoteMergedMemo }); +await assert.rejects(() => request("memo.get", { memoId: merged.memo.id, includeDeleted: true }), /Query returned no rows/); +assert.equal((await request("memo.get", { memoId: first.memo.id, includeDeleted: true })).memo.mergedIntoMemoId, remoteMergedMemo.id); +const mergedResource = (await request("resource.list", { limit: 200 })).resources.find((resource) => resource.id === "resource-from-merge-source"); +assert.equal(mergedResource.memoId, remoteMergedMemo.id, "merge acknowledgement should remap resources before deleting the local memo"); +assert.equal(mergedResource.originalMemoId, first.memo.id, "merge acknowledgement should preserve the resource's source memo"); +assert.ok(!(await request("sync.outbox.list", { limit: 200 })).items.some((item) => item.id === mergeOutbox.id), "merge acknowledgement should clear the outbox item"); await request("template.cache", { template: { @@ -194,7 +221,7 @@ await request("storage.restore", { path: backupWithResource.path }); await assert.rejects(() => request("memo.get", { memoId: afterBackup.memo.id }), /Query returned no rows/); assert.equal(readFileSync(join(stagedResourceDirectory, "stage-test.bin"), "utf8"), "offline attachment snapshot", "restore should recover staged resources"); const outbox = await request("sync.outbox.list", { limit: 100 }); -assert.ok(outbox.items.some((item) => item.kind === "memo.merge")); +assert.ok(!outbox.items.some((item) => item.id === mergeOutbox.id), "a restored backup must not resurrect an acknowledged merge"); const conflictMemo = await request("memo.create", { notebookId: inbox.id, title: "Conflict test", contentMarkdown: "local conflict", tags: [] }); const conflictCandidate = (await request("sync.outbox.list", { limit: 200 })).items.find((item) => item.entityId === conflictMemo.memo.id); assert.ok(conflictCandidate, "conflict test should create an outbox item");