From 5a2a63e2bb80d4adf374752a1bb31a9570caff19 Mon Sep 17 00:00:00 2001 From: Frederic Cabassut Date: Sun, 22 Mar 2026 18:51:14 +0100 Subject: [PATCH] fix: prevent 3-way merge from reverting sequential single-user edits The server's 3-way content merge was incorrectly triggering on sequential edits by the same user. When a user checked then unchecked a checklist item, the merge saw local === base (both unchecked) and kept the server's checked state, silently discarding the uncheck. Now the merge only runs when there's an actual concurrent edit (baseVersion < existing.version), meaning another user saved between the client's load and this save. For single-user sequential edits, the client's content is used directly. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/lib/server/notes-service.ts | 29 ++++++++++++++--------------- tests/e2e/checklist.spec.ts | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/lib/server/notes-service.ts b/src/lib/server/notes-service.ts index b610c57..3fc8633 100644 --- a/src/lib/server/notes-service.ts +++ b/src/lib/server/notes-service.ts @@ -238,21 +238,20 @@ 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 - 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(); + // 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. + 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); diff --git a/tests/e2e/checklist.spec.ts b/tests/e2e/checklist.spec.ts index 47ead8e..0f4c9a5 100644 --- a/tests/e2e/checklist.spec.ts +++ b/tests/e2e/checklist.spec.ts @@ -60,6 +60,31 @@ test.describe('Checklist', () => { await expect(page.getByTestId('checklist-done-checkbox').first()).toBeChecked(); }); + test('Scenario: Sequential check and uncheck both persist correctly', async ({ authenticatedPage: page }) => { + // Given a checklist note with an item "Buy milk" exists + await createChecklistNote(page, 'Tasks2', ['Buy milk']); + + // When the user checks the item and closes + await noteCard(page, 'Tasks2').click(); + await page.getByTestId('checklist-checkbox').first().click(); + await expect(page.getByTestId('checklist-toggle-done')).toContainText('1 done'); + await page.getByTestId('close-editor-btn').click(); + + // Then the checked state persists + await noteCard(page, 'Tasks2').click(); + await expect(page.getByTestId('checklist-done-checkbox').first()).toBeChecked(); + + // When the user unchecks the item from the done section and closes + await page.getByTestId('checklist-done-checkbox').first().click(); + await expect(page.getByTestId('checklist-checkbox')).toHaveCount(1); + await page.getByTestId('close-editor-btn').click(); + + // Then the unchecked state also persists (not reverted by 3-way merge) + await noteCard(page, 'Tasks2').click(); + await expect(page.getByTestId('checklist-checkbox')).toHaveCount(1); + await expect(page.getByTestId('checklist-input').first()).toHaveValue('Buy milk'); + }); + test('Scenario: Enter key adds a new checklist item', async ({ authenticatedPage: page }) => { // Given a checklist with one item await page.getByTestId('new-note-btn').click();