fix(sync): merge concurrent note content edits

Create version snapshots before mutating note content so later concurrent
saves can find a base version. Reuse the content merge helper for direct
updates and sync pushes, with tests covering fallback and queued-edit
merge behavior.

Co-authored-by: Codex <codex@openai.com>

src/lib/server/notes-service.test.ts

@@ -263,6 +263,31 @@ describe('updateNote', () => {
 		const updated = updateNote(db, OWNER_ID, 'n1', { title: 'Now with #newtag' });
 		expect(updated!.tags).toContain('newtag');
 	});
+
+	it('should merge concurrent content edits using the snapshot created by the first update', () => {
+		seedNote('n1', {
+			title: 'Shopping',
+			content: '- [ ] Milk\n- [ ] Bread',
+			checklistMode: true,
+			version: 1
+		});
+		shareNote('n1');
+
+		const ownerUpdate = updateNote(db, OWNER_ID, 'n1', {
+			content: '- [ ] Milk\n- [x] Bread',
+			baseVersion: 1
+		});
+		expect(ownerUpdate!.version).toBe(2);
+
+		const collabUpdate = updateNote(db, COLLAB_ID, 'n1', {
+			content: '- [x] Milk\n- [ ] Bread',
+			baseVersion: 1
+		});
+
+		expect(collabUpdate!.content).toBe('- [x] Milk\n- [x] Bread');
+		const snapshots = db.select().from(noteVersions).where(eq(noteVersions.noteId, 'n1')).all();
+		expect(snapshots.some((snapshot) => snapshot.version === 1)).toBe(true);
+	});
 });
 
 describe('deleteNote', () => {

src/lib/server/notes-service.ts

@@ -9,7 +9,7 @@ import { fetchCollaboratorsForNotes } from './collaborators.js';
 import { fetchSharesForNotes } from './shares-service.js';
 import { canAccessNote } from './api-utils.js';
 import { createSnapshot } from './versions-service.js';
-import { mergeContent } from '$lib/utils/content-merge.js';
+import { mergeContentUpdate } from '$lib/utils/content-merge.js';
 import type { NoteFilter } from '$lib/types/index.js';
 
 interface NoteRow {
@@ -215,6 +215,19 @@ export interface UpdateNoteInput {
 /** Per-user fields that go to noteUserState for collaborators */
 const PER_USER_FIELDS = ['pinned', 'archived', 'sortOrder'] as const;
 
+function getBaseContent(db: Db, noteId: string, baseVersion: number | undefined): string | null {
+	if (baseVersion === undefined) return null;
+
+	const snapshot = db.select({ content: noteVersions.content })
+		.from(noteVersions)
+		.where(and(eq(noteVersions.noteId, noteId), lte(noteVersions.version, baseVersion)))
+		.orderBy(desc(noteVersions.version))
+		.limit(1)
+		.get();
+
+	return snapshot?.content ?? null;
+}
+
 export function updateNote(db: Db, userId: number, id: string, input: UpdateNoteInput) {
 	const { canAccess, isOwner } = canAccessNote(db, id, userId);
 	if (!canAccess) return null;
@@ -238,29 +251,18 @@ export function updateNote(db: Db, userId: number, id: string, input: UpdateNote
 
 	if (input.title !== undefined) { sharedUpdates.title = input.title; hasSharedUpdates = true; }
 	if (input.content !== undefined) {
-		// 3-way merge to preserve concurrent edits from other users.
-		// Only attempt merge when there's a concurrent edit: the client's
-		// baseVersion is behind the current version, meaning someone else
-		// saved between the client's load and this save.
+		// Preserve concurrent edits at line/block granularity when the client
+		// saved from an older note version.
 		const hasConcurrentEdit = input.baseVersion !== undefined
 			&& input.baseVersion < existing.version;
 
-		if (hasConcurrentEdit && input.content !== existing.content) {
-			const baseSnapshot = db.select({ content: noteVersions.content })
-				.from(noteVersions)
-				.where(and(eq(noteVersions.noteId, id), lte(noteVersions.version, input.baseVersion!)))
-				.orderBy(desc(noteVersions.version))
-				.limit(1)
-				.get();
-
-			if (baseSnapshot && baseSnapshot.content !== existing.content) {
-				sharedUpdates.content = mergeContent(baseSnapshot.content, input.content, existing.content);
-			} else {
-				sharedUpdates.content = input.content;
-			}
-		} else {
-			sharedUpdates.content = input.content;
-		}
+		sharedUpdates.content = hasConcurrentEdit
+			? mergeContentUpdate({
+				baseContent: getBaseContent(db, id, input.baseVersion),
+				incomingContent: input.content,
+				currentContent: existing.content
+			})
+			: input.content;
 		hasSharedUpdates = true;
 	}
 	if (input.color !== undefined) { sharedUpdates.color = input.color; hasSharedUpdates = true; }
@@ -278,6 +280,22 @@ export function updateNote(db: Db, userId: number, id: string, input: UpdateNote
 		if (input.sortOrder !== undefined) { sharedUpdates.sortOrder = input.sortOrder; hasSharedUpdates = true; }
 	}
 
+	const hasActualContentChange =
+		(input.title !== undefined && input.title !== existing.title) ||
+		(sharedUpdates.content !== undefined && sharedUpdates.content !== existing.content) ||
+		(input.color !== undefined && input.color !== existing.color) ||
+		(input.checklistMode !== undefined && input.checklistMode !== existing.checklistMode);
+
+	if (hasActualContentChange) {
+		createSnapshot(db, id, {
+			version: existing.version,
+			title: existing.title,
+			content: existing.content,
+			checklistMode: existing.checklistMode,
+			color: existing.color
+		});
+	}
+
 	if (hasSharedUpdates || isOwner) {
 		db.update(notes).set(sharedUpdates).where(eq(notes.id, id)).run();
 	}
@@ -324,23 +342,6 @@ export function updateNote(db: Db, userId: number, id: string, input: UpdateNote
 		}
 	}
 
-	// Create a version snapshot only when at least one content field actually changed
-	const hasActualContentChange =
-		(input.title !== undefined && input.title !== existing.title) ||
-		(input.content !== undefined && input.content !== existing.content) ||
-		(input.color !== undefined && input.color !== existing.color) ||
-		(input.checklistMode !== undefined && input.checklistMode !== existing.checklistMode);
-
-	if (hasActualContentChange) {
-		createSnapshot(db, id, {
-			version: existing.version,
-			title: existing.title,
-			content: existing.content,
-			checklistMode: existing.checklistMode,
-			color: existing.color
-		});
-	}
-
 	return getNote(db, userId, id);
 }
 

src/lib/server/versions-service.ts

@@ -7,12 +7,15 @@ import type { NoteVersion, NoteVersionSummary, NoteColor } from '$lib/types/inde
 const MAX_VERSIONS_PER_NOTE = 50;
 const CONTENT_PREVIEW_LENGTH = 80;
 
+type Transaction = Parameters<Parameters<Db['transaction']>[0]>[0];
+type SnapshotDb = Db | Transaction;
+
 /**
  * Create a snapshot of a note's current state.
  * Only called when content actually changed. Prunes snapshots beyond MAX_VERSIONS_PER_NOTE.
  */
 export function createSnapshot(
-db: Db,
+db: SnapshotDb,
 noteId: string,
 snapshot: {
 version: number;

src/lib/sync/server.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, beforeEach } from 'vitest';
 import { createTestDb } from '../server/db/test-helpers.js';
-import { notes, noteCollaborators, noteUserState, users, syncLog } from '../server/db/schema.js';
+import { notes, noteCollaborators, noteUserState, users, syncLog, noteVersions } from '../server/db/schema.js';
 import { eq, and } from 'drizzle-orm';
 import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3';
 import type * as schema from '../server/db/schema.js';
@@ -289,6 +289,64 @@ describe('processSyncPush — shared notes', () => {
 		expect(note.content).toBe('- [x] Milk\n- [x] Bread');
 	});
 
+	it('should create a base snapshot during sync so later queued edits can merge', async () => {
+		const t1 = new Date('2024-01-01T00:00:00Z');
+		seedSharedNote('n1', {
+			updatedAt: t1,
+			createdAt: t1,
+			content: '- [ ] Milk\n- [ ] Bread',
+			checklistMode: true,
+			version: 1
+		});
+
+		await processSyncPush(db, [{
+			noteId: 'n1',
+			operation: 'update',
+			timestamp: t1.getTime() + 1000,
+			data: { content: '- [ ] Milk\n- [x] Bread' },
+			baseVersion: 1
+		}], OWNER_ID);
+
+		await processSyncPush(db, [{
+			noteId: 'n1',
+			operation: 'update',
+			timestamp: t1.getTime() + 2000,
+			data: { content: '- [x] Milk\n- [ ] Bread' },
+			baseVersion: 1
+		}], COLLAB_ID);
+
+		const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!;
+		expect(note.content).toBe('- [x] Milk\n- [x] Bread');
+
+		const snapshots = db.select().from(noteVersions).where(eq(noteVersions.noteId, 'n1')).all();
+		expect(snapshots.some((snapshot) => snapshot.version === 1)).toBe(true);
+	});
+
+	it('should retain at most 50 snapshots created by sync updates', async () => {
+		const createdAt = new Date('2024-01-01T00:00:00Z');
+		seedSharedNote('n1', {
+			updatedAt: createdAt,
+			createdAt,
+			content: 'version 1',
+			version: 1
+		});
+
+		for (let version = 2; version <= 52; version++) {
+			await processSyncPush(db, [{
+				noteId: 'n1',
+				operation: 'update',
+				timestamp: createdAt.getTime() + version,
+				data: { content: `version ${version}` },
+				baseVersion: version - 1
+			}], OWNER_ID);
+		}
+
+		const snapshots = db.select().from(noteVersions).where(eq(noteVersions.noteId, 'n1')).all();
+		expect(snapshots).toHaveLength(50);
+		expect(snapshots.some((snapshot) => snapshot.version === 1)).toBe(false);
+		expect(snapshots.some((snapshot) => snapshot.version === 51)).toBe(true);
+	});
+
 	it('should not allow non-collaborator to update the note', async () => {
 		const t1 = new Date('2024-01-01T00:00:00Z');
 		// Create note without sharing with user 3

src/lib/sync/server.ts

@@ -1,10 +1,10 @@
 import type { Db } from '$lib/server/db/index.js';
 import { notes, noteCollaborators, noteUserState, noteVersions, syncLog } from '$lib/server/db/schema.js';
-import { eq, gt, and, or, sql, desc, lte } from 'drizzle-orm';
+import { eq, gt, and, sql, desc, lte } from 'drizzle-orm';
 import { extractTags } from '$lib/utils/tags.js';
 import { syncNoteTags } from '$lib/server/tags.js';
-import { canAccessNote } from '$lib/server/api-utils.js';
-import { mergeContent } from '$lib/utils/content-merge.js';
+import { createSnapshot } from '$lib/server/versions-service.js';
+import { mergeContentUpdate } from '$lib/utils/content-merge.js';
 import type { SyncQueueItem } from './idb.js';
 
 /**
@@ -69,16 +69,17 @@ export async function processSyncPush(db: Db, changes: SyncQueueItem[], userId:
 						if (change.data.color !== undefined) sharedData.color = change.data.color;
 						if (change.data.checklistMode !== undefined) sharedData.checklistMode = change.data.checklistMode;
 
-						// Content: 3-way merge to preserve concurrent edits from other users
+						// Content: merge only when the client saved from an older note version.
 						if (change.data.content !== undefined && change.data.content !== note.content) {
-							const baseContent = getBaseContent(tx, change.noteId, change.baseVersion);
-							if (baseContent !== null && baseContent !== note.content) {
-								// 3-way merge: base, incoming (local), server current (remote)
-								sharedData.content = mergeContent(baseContent, change.data.content, note.content);
-							} else {
-								// No base or base matches server — take incoming content
-								sharedData.content = change.data.content;
-							}
+							const hasConcurrentEdit = change.baseVersion !== undefined
+								&& change.baseVersion < note.version;
+							sharedData.content = hasConcurrentEdit
+								? mergeContentUpdate({
+									baseContent: getBaseContent(tx, change.noteId, change.baseVersion),
+									incomingContent: change.data.content,
+									currentContent: note.content
+								})
+								: change.data.content;
 						}
 
 						if (isOwner) {
@@ -92,6 +93,22 @@ export async function processSyncPush(db: Db, changes: SyncQueueItem[], userId:
 							if (change.data.sortOrder !== undefined) sharedData.sortOrder = change.data.sortOrder;
 						}
 
+						const hasActualContentChange =
+							(change.data.title !== undefined && change.data.title !== note.title) ||
+							(sharedData.content !== undefined && sharedData.content !== note.content) ||
+							(change.data.color !== undefined && change.data.color !== note.color) ||
+							(change.data.checklistMode !== undefined && change.data.checklistMode !== note.checklistMode);
+
+						if (hasActualContentChange) {
+							createSnapshot(tx, change.noteId, {
+								version: note.version,
+								title: note.title,
+								content: note.content,
+								checklistMode: note.checklistMode,
+								color: note.color
+							});
+						}
+
 						tx.update(notes)
 							.set(sharedData)
 							.where(eq(notes.id, change.noteId))

src/lib/utils/content-merge.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from 'vitest';
-import { mergeContent, mergeContentTwoWay } from './content-merge.js';
+import { mergeContent, mergeContentTwoWay, mergeContentUpdate } from './content-merge.js';
 
 describe('mergeContent (3-way)', () => {
 	it('should return remote when local is unchanged', () => {
@@ -94,12 +94,37 @@ describe('mergeContentTwoWay', () => {
 		expect(mergeContentTwoWay('hello', 'hello')).toBe('hello');
 	});
 
-	it('should attempt merge when contents differ', () => {
+	it('should preserve the current server document when contents differ', () => {
 		const local = '- [x] Milk\n- [ ] Bread';
 		const remote = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs';
 
-		const result = mergeContentTwoWay(local, remote);
-		// Without a base, conflict resolution prefers remote
-		expect(result).toContain('- [ ] Bread');
+		expect(mergeContentTwoWay(local, remote)).toBe(remote);
+	});
+
+	it('should not resurrect content deleted from the current document', () => {
+		expect(mergeContentTwoWay('deleted content', '')).toBe('');
+	});
+});
+
+
+describe('mergeContentUpdate', () => {
+	it('should merge an incoming update against current content when a base is available', () => {
+		const result = mergeContentUpdate({
+			baseContent: '- [ ] Milk\n- [ ] Bread',
+			incomingContent: '- [x] Milk\n- [ ] Bread',
+			currentContent: '- [ ] Milk\n- [x] Bread'
+		});
+
+		expect(result).toBe('- [x] Milk\n- [x] Bread');
+	});
+
+	it('should preserve current content when the base is missing', () => {
+		const result = mergeContentUpdate({
+			baseContent: null,
+			incomingContent: '- [x] Milk',
+			currentContent: '- [ ] Bread'
+		});
+
+		expect(result).toBe('- [ ] Bread');
 	});
 });

src/lib/utils/content-merge.ts

@@ -79,10 +79,31 @@ export function mergeContent(base: string, local: string, remote: string): strin
 
 /**
  * 2-way merge fallback when no base is available.
- * Uses an empty string as the base, so all lines from both sides appear
- * as additions. For conflicts (same region), prefers remote.
+ * Without a common ancestor, edits and deletions cannot be distinguished from
+ * concurrent changes. Preserve the current server document rather than
+ * fabricating content by combining two complete snapshots.
  */
 export function mergeContentTwoWay(local: string, remote: string): string {
-	if (local === remote) return remote;
-	return mergeContent('', local, remote);
+	return remote;
+}
+
+/**
+ * Merge an incoming content snapshot into the current server content.
+ * Uses 3-way merge when a base is available; otherwise falls back to a
+ * conservative 2-way merge instead of replacing the whole document.
+ */
+export function mergeContentUpdate(input: {
+	baseContent: string | null;
+	incomingContent: string;
+	currentContent: string;
+}): string {
+	const { baseContent, incomingContent, currentContent } = input;
+
+	if (incomingContent === currentContent) return currentContent;
+	if (baseContent === incomingContent) return currentContent;
+	if (baseContent === currentContent) return incomingContent;
+
+	return baseContent === null
+		? mergeContentTwoWay(incomingContent, currentContent)
+		: mergeContent(baseContent, incomingContent, currentContent);
 }