diff --git a/package.json b/package.json index 3c6c24a..9276ee6 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "idb": "^8.0.3", "lucide-svelte": "^0.577.0", "markdown-it": "^14.1.1", + "node-diff3": "^3.2.0", "nodemailer": "^8.0.2", "svelte-dnd-action": "^0.9.69", "tiptap-markdown": "^0.9.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 956bb60..56dedea 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -80,6 +80,9 @@ importers: markdown-it: specifier: ^14.1.1 version: 14.1.1 + node-diff3: + specifier: ^3.2.0 + version: 3.2.0 nodemailer: specifier: ^8.0.2 version: 8.0.2 @@ -3004,6 +3007,10 @@ packages: resolution: {integrity: sha512-gBVjCaqDlRUk0EwoPNKzIr9KkS9041G/q31IBShPs1Xz6UTA+EXdZADbzqAJQrpDRq71CIMnOP5VMut3SL0z5Q==} engines: {node: ^18 || ^20 || >= 21} + node-diff3@3.2.0: + resolution: {integrity: sha512-vLh2xJFSyniBLYDEDbXKqD32fQ5vAxmYT4hco8t0EHQ4CQ4BDHhshi7kdvDc6Y1MwGSi1Mhl4unUukPbCayZdw==} + engines: {bun: '>=1.3.0', node: '>=18'} + node-gyp-build@4.8.4: resolution: {integrity: sha512-LA4ZjwlnUblHVgq0oBF3Jl/6h/Nvs5fzBLwdEF4nuxnFdsfajde4WfxtJr3CaiH+F6ewcIB/q4jQ4UzPyid+CQ==} hasBin: true @@ -6780,6 +6787,8 @@ snapshots: node-addon-api@8.6.0: {} + node-diff3@3.2.0: {} + node-gyp-build@4.8.4: {} node-preload@0.2.1: diff --git a/src/lib/server/notes-service.ts b/src/lib/server/notes-service.ts index 87572ca..a864786 100644 --- a/src/lib/server/notes-service.ts +++ b/src/lib/server/notes-service.ts @@ -1,6 +1,6 @@ import type { Db } from './db/index.js'; -import { notes, noteTags, tags, noteCollaborators, noteUserState } from './db/schema.js'; -import { eq, and, desc, like, or, inArray, sql } from 'drizzle-orm'; +import { notes, noteTags, tags, noteCollaborators, noteUserState, noteVersions } from './db/schema.js'; +import { eq, and, desc, like, or, inArray, sql, lte } from 'drizzle-orm'; import { v4 as uuidv4 } from 'uuid'; import { extractTags } from '$lib/utils/tags.js'; import { fetchTagsForNotes, syncNoteTags } from './tags.js'; @@ -8,6 +8,7 @@ import { fetchAttachmentsForNotes } from './attachments.js'; import { fetchCollaboratorsForNotes } from './collaborators.js'; import { canAccessNote } from './api-utils.js'; import { createSnapshot } from './versions-service.js'; +import { mergeContent } from '$lib/utils/content-merge.js'; import type { NoteFilter } from '$lib/types/index.js'; interface NoteRow { @@ -205,6 +206,7 @@ export interface UpdateNoteInput { trashed?: boolean; checklistMode?: boolean; sortOrder?: number; + baseVersion?: number; } /** Per-user fields that go to noteUserState for collaborators */ @@ -232,7 +234,33 @@ export function updateNote(db: Db, userId: number, id: string, input: UpdateNote let hasSharedUpdates = false; if (input.title !== undefined) { sharedUpdates.title = input.title; hasSharedUpdates = true; } - if (input.content !== undefined) { sharedUpdates.content = input.content; hasSharedUpdates = true; } + if (input.content !== undefined) { + // 3-way merge to preserve concurrent edits from other users + if (input.content !== existing.content) { + const baseSnapshot = input.baseVersion !== undefined + ? 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() + : db.select({ content: noteVersions.content }) + .from(noteVersions) + .where(eq(noteVersions.noteId, id)) + .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; + } + hasSharedUpdates = true; + } if (input.color !== undefined) { sharedUpdates.color = input.color; hasSharedUpdates = true; } if (input.checklistMode !== undefined) { sharedUpdates.checklistMode = input.checklistMode; hasSharedUpdates = true; } diff --git a/src/lib/stores/notes.ts b/src/lib/stores/notes.ts index 6975c7f..a6c1c64 100644 --- a/src/lib/stores/notes.ts +++ b/src/lib/stores/notes.ts @@ -142,11 +142,19 @@ export async function createNote(note: NoteCreate): Promise { } export async function updateNote(id: string, updates: NoteUpdate): Promise { + // Read current version for 3-way merge base on the server + let baseVersion: number | undefined; + notes.update((list) => { + const current = list.find((n) => n.id === id); + if (current) baseVersion = current.version; + return list; + }); + try { const res = await fetch(`/api/notes/${id}`, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(updates) + body: JSON.stringify({ ...updates, baseVersion }) }); if (res.ok) { const updated: Note = await res.json(); @@ -173,7 +181,7 @@ export async function updateNote(id: string, updates: NoteUpdate): Promise { const localNotes = await getAllNotes(); const localMap = new Map(localNotes.map((n) => [n.id, n])); + // Build pending-fields map from sync queue to protect unpushed local changes + let pendingFieldsMap = new Map>(); + try { + const queue = await getSyncQueue(); + for (const item of queue) { + if (item.data) { + const fields = pendingFieldsMap.get(item.noteId) || new Set(); + Object.keys(item.data).forEach((k) => fields.add(k)); + pendingFieldsMap.set(item.noteId, fields); + } + } + } catch { + // Queue unavailable — merge without protection + } + for (const remote of remoteNotes) { const local = localMap.get(remote.id); if (local) { - const merged = mergeNotes(local, remote); + const pendingFields = pendingFieldsMap.get(remote.id); + const merged = mergeNotes(local, remote, pendingFields); await putNote(merged); } else { await putNote(remote); diff --git a/src/lib/sync/crdt.ts b/src/lib/sync/crdt.ts index f295df4..554145f 100644 --- a/src/lib/sync/crdt.ts +++ b/src/lib/sync/crdt.ts @@ -1,29 +1,39 @@ import type { Note } from '$lib/types/index.js'; +/** Fields that can be individually merged between local and remote notes */ +const MUTABLE_FIELDS = ['title', 'content', 'color', 'pinned', 'archived', 'trashed', 'checklistMode', 'sortOrder'] as const; + /** - * Last-Write-Wins (LWW) merge strategy per field. - * For a single-user app with multiple devices, this is sufficient. - * Each field is resolved by comparing updatedAt timestamps. + * Last-Write-Wins (LWW) merge strategy with pending-field protection. + * + * When remote is newer, takes remote values for all fields EXCEPT those + * listed in `pendingFields` — these represent locally-queued changes that + * haven't been pushed yet and must be preserved. */ -export function mergeNotes(local: Note, remote: Note): Note { - // If remote is newer overall, take remote +export function mergeNotes(local: Note, remote: Note, pendingFields?: Set): Note { const localTime = new Date(local.updatedAt).getTime(); const remoteTime = new Date(remote.updatedAt).getTime(); - if (remoteTime > localTime) { - return { ...remote }; - } + const remoteWins = remoteTime > localTime || + (remoteTime === localTime && remote.version > local.version); - if (localTime > remoteTime) { + if (!remoteWins) { return { ...local }; } - // Same timestamp - prefer higher version number - if (remote.version > local.version) { + // Remote wins — take remote as base, but preserve pending local fields + if (!pendingFields || pendingFields.size === 0) { return { ...remote }; } - return { ...local }; + const merged = { ...remote }; + for (const field of MUTABLE_FIELDS) { + if (pendingFields.has(field)) { + (merged as unknown as Record)[field] = (local as unknown as Record)[field]; + } + } + + return merged; } /** diff --git a/src/lib/sync/idb.ts b/src/lib/sync/idb.ts index 973fef9..961a1d6 100644 --- a/src/lib/sync/idb.ts +++ b/src/lib/sync/idb.ts @@ -48,6 +48,7 @@ export interface SyncQueueItem { operation: 'create' | 'update' | 'delete'; data?: Partial; timestamp: number; + baseVersion?: number; } let dbPromise: Promise> | null = null; diff --git a/src/lib/sync/server.test.ts b/src/lib/sync/server.test.ts index 56ceb59..4c3d4f0 100644 --- a/src/lib/sync/server.test.ts +++ b/src/lib/sync/server.test.ts @@ -224,21 +224,69 @@ describe('processSyncPush — shared notes', () => { expect(note.trashed).toBe(false); }); - it('should reject changes with older timestamp than server note', async () => { + it('should apply field changes even when timestamp is older (no timestamp gate)', async () => { const t2 = new Date('2024-01-02T00:00:00Z'); seedSharedNote('n1', { updatedAt: t2, createdAt: new Date('2024-01-01T00:00:00Z') }); - // Collaborator's change has a timestamp BEFORE the server's updatedAt + // Collaborator's change has a timestamp BEFORE the server's updatedAt. + // With field-level sync, the title change should still be applied because + // the sync data only contains fields the client explicitly modified. const t1 = new Date('2024-01-01T12:00:00Z').getTime(); await processSyncPush(db, [{ noteId: 'n1', operation: 'update', timestamp: t1, - data: { title: 'Stale edit' } + data: { title: 'Offline edit' } }], COLLAB_ID); const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; - expect(note.title).toBe('Shared Note'); // Unchanged + expect(note.title).toBe('Offline edit'); + // updatedAt should be max of change timestamp and server timestamp + expect(note.updatedAt.getTime()).toBe(t2.getTime()); + }); + + it('should merge checklist content when collaborator pushes concurrent content change', async () => { + const t1 = new Date('2024-01-01T00:00:00Z'); + seedSharedNote('n1', { + updatedAt: t1, + createdAt: t1, + content: '- [ ] Milk\n- [ ] Bread', + checklistMode: true + }); + + // Simulate owner editing content on server (checking Bread) + const t2 = new Date('2024-01-01T12:00:00Z'); + db.update(notes) + .set({ content: '- [ ] Milk\n- [x] Bread', updatedAt: t2, version: 2 }) + .where(eq(notes.id, 'n1')) + .run(); + + // Create a version snapshot for the base content (version 1) + const { noteVersions } = await import('../server/db/schema.js'); + db.insert(noteVersions).values({ + id: 'snap-1', + noteId: 'n1', + version: 1, + title: 'Shared Note', + content: '- [ ] Milk\n- [ ] Bread', + checklistMode: true, + color: 'default', + createdAt: t1 + }).run(); + + // Collaborator was offline since version 1, checked Milk + const t3 = new Date('2024-01-01T13:00:00Z').getTime(); + await processSyncPush(db, [{ + noteId: 'n1', + operation: 'update', + timestamp: t3, + data: { content: '- [x] Milk\n- [ ] Bread' }, + baseVersion: 1 + }], COLLAB_ID); + + const note = db.select().from(notes).where(eq(notes.id, 'n1')).get()!; + // Both changes should be merged: Milk checked by collab, Bread checked by owner + expect(note.content).toBe('- [x] Milk\n- [x] Bread'); }); it('should not allow non-collaborator to update the note', async () => { diff --git a/src/lib/sync/server.ts b/src/lib/sync/server.ts index 9426a75..f21c993 100644 --- a/src/lib/sync/server.ts +++ b/src/lib/sync/server.ts @@ -1,10 +1,37 @@ import type { Db } from '$lib/server/db/index.js'; -import { notes, noteCollaborators, noteUserState, syncLog } from '$lib/server/db/schema.js'; -import { eq, gt, and, or, sql } from 'drizzle-orm'; +import { notes, noteCollaborators, noteUserState, noteVersions, syncLog } from '$lib/server/db/schema.js'; +import { eq, gt, and, or, 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 type { SyncQueueItem } from './idb.js'; + +/** + * Find the base content for 3-way merge from version snapshots. + * Returns the content from the snapshot at or before the given version, + * or the current note content if no snapshot is found. + */ +function getBaseContent(tx: Parameters[0]>[0], noteId: string, baseVersion?: number): string | null { + if (baseVersion !== undefined) { + const snapshot = tx.select({ content: noteVersions.content }) + .from(noteVersions) + .where(and(eq(noteVersions.noteId, noteId), lte(noteVersions.version, baseVersion))) + .orderBy(desc(noteVersions.version)) + .limit(1) + .get(); + if (snapshot) return snapshot.content; + } + // Fallback: get the most recent snapshot before the current version + const latest = tx.select({ content: noteVersions.content }) + .from(noteVersions) + .where(eq(noteVersions.noteId, noteId)) + .orderBy(desc(noteVersions.version)) + .limit(1) + .get(); + return latest?.content ?? null; +} + /** * Process incoming sync changes from client. */ @@ -31,59 +58,70 @@ export async function processSyncPush(db: Db, changes: SyncQueueItem[], userId: if (!isOwner && !isCollaborator) continue; - if (change.timestamp > note.updatedAt.getTime()) { - // Shared fields: title, content, color, checklistMode - const sharedData: Record = { - updatedAt: new Date(change.timestamp), - version: note.version + 1 - }; - if (change.data.title !== undefined) sharedData.title = change.data.title; - if (change.data.content !== undefined) sharedData.content = change.data.content; - if (change.data.color !== undefined) sharedData.color = change.data.color; - if (change.data.checklistMode !== undefined) sharedData.checklistMode = change.data.checklistMode; + // Apply field-level updates unconditionally (no timestamp gate). + // Each sync change only contains the fields the client explicitly modified, + // so applying them won't overwrite other users' changes to different fields. + const sharedData: Record = { + updatedAt: new Date(Math.max(change.timestamp, note.updatedAt.getTime())), + version: note.version + 1 + }; + if (change.data.title !== undefined) sharedData.title = change.data.title; + if (change.data.color !== undefined) sharedData.color = change.data.color; + if (change.data.checklistMode !== undefined) sharedData.checklistMode = change.data.checklistMode; - if (isOwner) { - // Owner: per-user fields go to notes table - if (change.data.pinned !== undefined) sharedData.pinned = change.data.pinned; - if (change.data.archived !== undefined) sharedData.archived = change.data.archived; - if (change.data.trashed !== undefined) { - sharedData.trashed = change.data.trashed; - sharedData.trashedAt = change.data.trashed ? new Date(change.timestamp) : null; - } - if (change.data.sortOrder !== undefined) sharedData.sortOrder = change.data.sortOrder; + // Content: 3-way merge to preserve concurrent edits from other users + 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; } + } - tx.update(notes) - .set(sharedData) - .where(eq(notes.id, change.noteId)) - .run(); + if (isOwner) { + // Owner: per-user fields go to notes table + if (change.data.pinned !== undefined) sharedData.pinned = change.data.pinned; + if (change.data.archived !== undefined) sharedData.archived = change.data.archived; + if (change.data.trashed !== undefined) { + sharedData.trashed = change.data.trashed; + sharedData.trashedAt = change.data.trashed ? new Date(change.timestamp) : null; + } + if (change.data.sortOrder !== undefined) sharedData.sortOrder = change.data.sortOrder; + } - // Collaborator: per-user fields go to noteUserState - if (!isOwner) { - const hasPerUser = change.data.pinned !== undefined || - change.data.archived !== undefined || - change.data.sortOrder !== undefined; - if (hasPerUser) { - const existing = tx.select().from(noteUserState) + tx.update(notes) + .set(sharedData) + .where(eq(notes.id, change.noteId)) + .run(); + + // Collaborator: per-user fields go to noteUserState + if (!isOwner) { + const hasPerUser = change.data.pinned !== undefined || + change.data.archived !== undefined || + change.data.sortOrder !== undefined; + if (hasPerUser) { + const existing = tx.select().from(noteUserState) + .where(and(eq(noteUserState.noteId, change.noteId), eq(noteUserState.userId, userId))) + .get(); + if (existing) { + const updates: Record = {}; + if (change.data.pinned !== undefined) updates.pinned = change.data.pinned; + if (change.data.archived !== undefined) updates.archived = change.data.archived; + if (change.data.sortOrder !== undefined) updates.sortOrder = change.data.sortOrder; + tx.update(noteUserState).set(updates) .where(and(eq(noteUserState.noteId, change.noteId), eq(noteUserState.userId, userId))) - .get(); - if (existing) { - const updates: Record = {}; - if (change.data.pinned !== undefined) updates.pinned = change.data.pinned; - if (change.data.archived !== undefined) updates.archived = change.data.archived; - if (change.data.sortOrder !== undefined) updates.sortOrder = change.data.sortOrder; - tx.update(noteUserState).set(updates) - .where(and(eq(noteUserState.noteId, change.noteId), eq(noteUserState.userId, userId))) - .run(); - } else { - tx.insert(noteUserState).values({ - noteId: change.noteId, - userId, - pinned: change.data.pinned ?? false, - archived: change.data.archived ?? false, - sortOrder: change.data.sortOrder ?? 0 - }).run(); - } + .run(); + } else { + tx.insert(noteUserState).values({ + noteId: change.noteId, + userId, + pinned: change.data.pinned ?? false, + archived: change.data.archived ?? false, + sortOrder: change.data.sortOrder ?? 0 + }).run(); } } } diff --git a/src/lib/sync/sync.test.ts b/src/lib/sync/sync.test.ts index 32cb59e..6e5ecec 100644 --- a/src/lib/sync/sync.test.ts +++ b/src/lib/sync/sync.test.ts @@ -55,6 +55,71 @@ describe('mergeNotes (LWW)', () => { const result = mergeNotes(local, remote); expect(result.title).toBe('Local'); }); + + it('should preserve pending local fields when remote is newer', () => { + const local = makeNote({ + title: 'Local title', + content: 'Local content', + color: 'coral', + updatedAt: new Date('2024-01-01'), + version: 1 + }); + const remote = makeNote({ + title: 'Remote title', + content: 'Remote content', + color: 'default', + updatedAt: new Date('2024-01-02'), + version: 2 + }); + + // Content has pending local changes; title and color do not + const pendingFields = new Set(['content']); + const result = mergeNotes(local, remote, pendingFields); + + // Remote wins overall, but content is preserved from local + expect(result.title).toBe('Remote title'); + expect(result.content).toBe('Local content'); + expect(result.color).toBe('default'); // remote wins for non-pending field + expect(result.version).toBe(2); // metadata from remote + }); + + it('should take remote values for non-pending fields when remote is newer', () => { + const local = makeNote({ + pinned: true, + archived: true, + updatedAt: new Date('2024-01-01') + }); + const remote = makeNote({ + pinned: false, + archived: false, + updatedAt: new Date('2024-01-02') + }); + + const pendingFields = new Set(['pinned']); + const result = mergeNotes(local, remote, pendingFields); + + expect(result.pinned).toBe(true); // pending — preserved + expect(result.archived).toBe(false); // not pending — remote wins + }); + + it('should ignore pendingFields when local is newer', () => { + const local = makeNote({ + title: 'Local', + updatedAt: new Date('2024-01-02'), + version: 2 + }); + const remote = makeNote({ + title: 'Remote', + updatedAt: new Date('2024-01-01'), + version: 1 + }); + + const pendingFields = new Set(['title']); + const result = mergeNotes(local, remote, pendingFields); + + // Local wins entirely since it's newer + expect(result.title).toBe('Local'); + }); }); describe('hasChanges', () => { diff --git a/src/lib/utils/content-merge.test.ts b/src/lib/utils/content-merge.test.ts new file mode 100644 index 0000000..52b4705 --- /dev/null +++ b/src/lib/utils/content-merge.test.ts @@ -0,0 +1,105 @@ +import { describe, it, expect } from 'vitest'; +import { mergeContent, mergeContentTwoWay } from './content-merge.js'; + +describe('mergeContent (3-way)', () => { + it('should return remote when local is unchanged', () => { + const base = '- [ ] Milk\n- [ ] Bread'; + const local = '- [ ] Milk\n- [ ] Bread'; + const remote = '- [x] Milk\n- [ ] Bread'; + + expect(mergeContent(base, local, remote)).toBe('- [x] Milk\n- [ ] Bread'); + }); + + it('should return local when remote is unchanged', () => { + const base = '- [ ] Milk\n- [ ] Bread'; + const local = '- [x] Milk\n- [ ] Bread'; + const remote = '- [ ] Milk\n- [ ] Bread'; + + expect(mergeContent(base, local, remote)).toBe('- [x] Milk\n- [ ] Bread'); + }); + + it('should merge when User A checks an item and User B adds an item', () => { + const base = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs'; + const local = '- [x] Milk\n- [ ] Bread\n- [ ] Eggs'; // User A checks Milk + const remote = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs\n- [ ] Butter'; // User B adds Butter + + const result = mergeContent(base, local, remote); + expect(result).toBe('- [x] Milk\n- [ ] Bread\n- [ ] Eggs\n- [ ] Butter'); + }); + + it('should merge when both users check different items', () => { + const base = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs'; + const local = '- [x] Milk\n- [ ] Bread\n- [ ] Eggs'; // User A checks Milk + const remote = '- [ ] Milk\n- [x] Bread\n- [ ] Eggs'; // User B checks Bread + + const result = mergeContent(base, local, remote); + expect(result).toBe('- [x] Milk\n- [x] Bread\n- [ ] Eggs'); + }); + + it('should LWW (prefer remote) when both users edit the same line', () => { + const base = '- [ ] Milk\n- [ ] Bread'; + const local = '- [ ] Whole Milk\n- [ ] Bread'; // User A renames Milk + const remote = '- [x] Milk\n- [ ] Bread'; // User B checks Milk + + const result = mergeContent(base, local, remote); + // Both changed line 1 — conflict resolved by preferring remote + expect(result).toBe('- [x] Milk\n- [ ] Bread'); + }); + + it('should handle line additions by both sides', () => { + const base = '- [ ] Milk'; + const local = '- [ ] Milk\n- [ ] Bread'; // User A adds Bread at end + const remote = '- [ ] Eggs\n- [ ] Milk'; // User B adds Eggs at beginning + + const result = mergeContent(base, local, remote); + // Both additions should be preserved + expect(result).toContain('- [ ] Milk'); + expect(result).toContain('- [ ] Bread'); + expect(result).toContain('- [ ] Eggs'); + }); + + it('should handle line deletion by one side', () => { + const base = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs'; + const local = '- [ ] Milk\n- [ ] Eggs'; // User A removes Bread + const remote = '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs'; // User B unchanged + + const result = mergeContent(base, local, remote); + expect(result).toBe('- [ ] Milk\n- [ ] Eggs'); + }); + + it('should work with non-checklist markdown (prose paragraphs)', () => { + const base = '# Shopping\n\nBuy groceries today.\n\n## Notes\n\nNothing special.'; + const local = '# Shopping\n\nBuy groceries today.\n\n## Notes\n\nRemember coupons.'; // User A edits notes + const remote = '# Shopping List\n\nBuy groceries today.\n\n## Notes\n\nNothing special.'; // User B edits title + + const result = mergeContent(base, local, remote); + expect(result).toContain('# Shopping List'); + expect(result).toContain('Remember coupons.'); + }); + + it('should handle empty content', () => { + expect(mergeContent('', '', '')).toBe(''); + expect(mergeContent('', 'hello', '')).toBe('hello'); + expect(mergeContent('', '', 'hello')).toBe('hello'); + }); + + it('should return remote when local equals remote', () => { + const content = '- [ ] Milk\n- [ ] Bread'; + expect(mergeContent('old', content, content)).toBe(content); + }); +}); + +describe('mergeContentTwoWay', () => { + it('should return remote when both are identical', () => { + expect(mergeContentTwoWay('hello', 'hello')).toBe('hello'); + }); + + it('should attempt merge 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'); + }); +}); diff --git a/src/lib/utils/content-merge.ts b/src/lib/utils/content-merge.ts new file mode 100644 index 0000000..bedbbad --- /dev/null +++ b/src/lib/utils/content-merge.ts @@ -0,0 +1,88 @@ +import { diff3Merge } from 'node-diff3'; + +/** + * Resolve a conflict region by comparing each line against the base. + * If only one side changed a line, take that change. + * If both sides changed the same line, prefer remote (LWW). + * Handles different-length regions by appending extra lines. + */ +function resolveConflict( + aLines: string[], + oLines: string[], + bLines: string[] +): string[] { + const maxLen = Math.max(aLines.length, bLines.length, oLines.length); + const result: string[] = []; + + for (let i = 0; i < maxLen; i++) { + const a = i < aLines.length ? aLines[i] : undefined; + const o = i < oLines.length ? oLines[i] : undefined; + const b = i < bLines.length ? bLines[i] : undefined; + + const aChanged = a !== o; + const bChanged = b !== o; + + if (aChanged && !bChanged && a !== undefined) { + // Only local changed this line — take local + result.push(a); + } else if (bChanged && !aChanged && b !== undefined) { + // Only remote changed this line — take remote + result.push(b); + } else if (aChanged && bChanged) { + // Both changed — LWW: prefer remote + if (b !== undefined) result.push(b); + // If remote deleted (undefined) but local has it, skip (remote wins) + } else { + // Neither changed, or both are the same as base + if (o !== undefined) result.push(o); + else if (b !== undefined) result.push(b); + else if (a !== undefined) result.push(a); + } + } + + return result; +} + +/** + * 3-way line-level merge for note content. + * Takes a base (common ancestor), local (incoming), and remote (server current). + * For conflict regions, resolves line-by-line: if only one side changed a line, + * take that change. If both changed the same line, prefer remote (LWW). + */ +export function mergeContent(base: string, local: string, remote: string): string { + // If any two are identical, short-circuit + if (local === remote) return remote; + if (local === base) return remote; + if (remote === base) return local; + + const baseLines = base.split('\n'); + const localLines = local.split('\n'); + const remoteLines = remote.split('\n'); + + const regions = diff3Merge(localLines, baseLines, remoteLines); + const merged: string[] = []; + + for (const region of regions) { + if (region.ok) { + merged.push(...region.ok); + } else if (region.conflict) { + merged.push(...resolveConflict( + region.conflict.a, + region.conflict.o, + region.conflict.b + )); + } + } + + return merged.join('\n'); +} + +/** + * 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. + */ +export function mergeContentTwoWay(local: string, remote: string): string { + if (local === remote) return remote; + return mergeContent('', local, remote); +} diff --git a/tests/e2e/sharing.spec.ts b/tests/e2e/sharing.spec.ts index 2b6acb3..a3f9549 100644 --- a/tests/e2e/sharing.spec.ts +++ b/tests/e2e/sharing.spec.ts @@ -124,6 +124,37 @@ test.describe('Note Sharing', () => { expect(syncedNote.content).toBe('Owner sync update'); }); + test('Scenario: Both users\' edits to different fields are preserved after concurrent PATCH', async ({ + authenticatedPage: page, + collabPage + }) => { + // Given a shared checklist note exists + const note = await createSharedNote( + page, + collabPage, + 'Shopping List', + '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs' + ); + + // When the owner checks Milk + await page.request.patch(`/api/notes/${note.id}`, { + data: { content: '- [x] Milk\n- [ ] Bread\n- [ ] Eggs', baseVersion: note.version } + }); + + // And the collaborator adds Butter (based on the same original version) + await collabPage.request.patch(`/api/notes/${note.id}`, { + data: { content: '- [ ] Milk\n- [ ] Bread\n- [ ] Eggs\n- [ ] Butter', baseVersion: note.version } + }); + + // Then both changes are preserved via content merge + const res = await page.request.get(`/api/notes/${note.id}`); + const merged = await res.json(); + expect(merged.content).toContain('- [x] Milk'); + expect(merged.content).toContain('- [ ] Butter'); + expect(merged.content).toContain('- [ ] Bread'); + expect(merged.content).toContain('- [ ] Eggs'); + }); + test('Scenario: Collaborator sees leave action and can leave a shared note', async ({ authenticatedPage: page, collabPage