v2.28.2: notes and Cloud sync stop losing to their own save

apps/desktop/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@zennotes/desktop",
   "productName": "ZenNotes",
-  "version": "2.28.1",
+  "version": "2.28.2",
   "description": "ZenNotes desktop shell",
   "private": true,
   "main": "./out/main/index.js",

apps/desktop/src/main/cloud-auth.ts

@@ -170,8 +170,15 @@ export class CloudAuthManager {
       throw new Error("ZenNotes Cloud returned an invalid sign-in response.");
     }
     if (!(await this.dependencies.setSecret(pending.base_url, payload.token))) {
+      // On Linux this almost always means Chromium settled on its plaintext
+      // key store because it did not recognize the desktop environment, so
+      // point at the override instead of leaving a dead end.
       throw new Error(
-        "ZenNotes could not store the cloud credential securely on this device.",
+        process.platform === "linux"
+          ? "ZenNotes could not store the cloud credential securely on this device. " +
+            "If a Secret Service keyring (such as gnome-keyring) is running, " +
+            "launch ZenNotes with --password-store=gnome-libsecret and sign in again."
+          : "ZenNotes could not store the cloud credential securely on this device.",
       );
     }
 

apps/desktop/src/main/cloud-sync-filesystem.test.ts

@@ -1,13 +1,13 @@
 import { afterEach, describe, expect, it } from 'vitest'
-import { mkdtemp, readFile, rm, writeFile, mkdir } from 'node:fs/promises'
+import { mkdtemp, readdir, readFile, rm, writeFile, mkdir } from 'node:fs/promises'
 import os from 'node:os'
 import path from 'node:path'
 import { createHash } from 'node:crypto'
 import {
-  CloudSyncLocalEditConflictError,
   DesktopCloudSyncRepository,
   DesktopCloudSyncStateStore
 } from './cloud-sync-filesystem'
+import type { CloudSyncChange } from '@zennotes/bridge-contract/cloud-sync'
 import type { CloudSyncTrackedItem } from '@zennotes/shared-domain/cloud-sync-engine'
 
 const roots: string[] = []
@@ -26,6 +26,24 @@ function hash(contents: string): string {
   return createHash('sha256').update(contents).digest('hex')
 }
 
+function upsert(path: string, contents: string): CloudSyncChange {
+  return {
+    sequence: 2,
+    item_id: 'item-remote',
+    type: 'upsert',
+    path,
+    previous_path: null,
+    revision: 2,
+    content: {
+      encoding: 'utf8',
+      data: contents,
+      sha256: hash(contents),
+      byte_length: Buffer.byteLength(contents),
+      media_type: 'text/markdown'
+    }
+  }
+}
+
 function tracked(path: string, contents: string): CloudSyncTrackedItem {
   return {
     item_id: 'item-1',
@@ -140,33 +158,143 @@ describe('DesktopCloudSyncRepository', () => {
     })
   })
 
-  it('does not overwrite a local edit while pulling remote changes', async () => {
+  // The local file is never overwritten, and the incoming version is never
+  // thrown away: it lands beside it. Sync used to throw here instead, which
+  // stopped the whole run and, because the cursor never advanced, stopped
+  // every run after it too (#585 follow-up, reported on Discord).
+  it('keeps both versions when a remote change meets a local edit', async () => {
+    const root = await temporaryRoot()
+    await writeFile(path.join(root, 'note.md'), 'local edit')
+    const repository = new DesktopCloudSyncRepository(root)
+
+    const conflict = await repository.apply(
+      upsert('note.md', 'remote edit'),
+      tracked('note.md', 'old contents')
+    )
+
+    expect(conflict).toEqual({
+      code: 'LOCAL_EDIT_CONFLICT',
+      path: 'note.md',
+      conflict_copy_path: 'note (cloud conflict).md'
+    })
+    expect(await readFile(path.join(root, 'note.md'), 'utf8')).toBe('local edit')
+    expect(await readFile(path.join(root, 'note (cloud conflict).md'), 'utf8')).toBe('remote edit')
+  })
+
+  // What wedged the reporter: the change feed carried a file this device had
+  // never tracked, so sync refused it without ever noticing that the bytes on
+  // disk were already exactly what was being delivered.
+  it('adopts a file that already matches the incoming change', async () => {
+    const root = await temporaryRoot()
+    await mkdir(path.join(root, '.zennotes'), { recursive: true })
+    await writeFile(path.join(root, '.zennotes', 'vault.json'), '{"favorites":[]}')
+    const repository = new DesktopCloudSyncRepository(root)
+
+    const conflict = await repository.apply(
+      upsert('.zennotes/vault.json', '{"favorites":[]}'),
+      undefined
+    )
+
+    expect(conflict).toBeUndefined()
+    expect(await readFile(path.join(root, '.zennotes', 'vault.json'), 'utf8')).toBe(
+      '{"favorites":[]}'
+    )
+    expect(await readdir(path.join(root, '.zennotes'))).toEqual(['vault.json'])
+  })
+
+  it('numbers conflict copies instead of overwriting an earlier one', async () => {
+    const root = await temporaryRoot()
+    await writeFile(path.join(root, 'note.md'), 'local edit')
+    await writeFile(path.join(root, 'note (cloud conflict).md'), 'an earlier conflict')
+    const repository = new DesktopCloudSyncRepository(root)
+
+    const conflict = await repository.apply(upsert('note.md', 'remote edit'), undefined)
+
+    expect(conflict?.conflict_copy_path).toBe('note (cloud conflict 2).md')
+    expect(await readFile(path.join(root, 'note (cloud conflict).md'), 'utf8')).toBe(
+      'an earlier conflict'
+    )
+    expect(await readFile(path.join(root, 'note (cloud conflict 2).md'), 'utf8')).toBe('remote edit')
+  })
+
+  // Settings are a question, not a merge: a numbered copy inside a hidden
+  // folder is not something anyone can act on, so the cloud version waits at
+  // one fixed path and the app asks which side to keep.
+  it('parks conflicting vault settings at one fixed path for the user to answer', async () => {
+    const root = await temporaryRoot()
+    await mkdir(path.join(root, '.zennotes'), { recursive: true })
+    await writeFile(path.join(root, '.zennotes', 'vault.json'), '{"favorites":["a"]}')
+    const repository = new DesktopCloudSyncRepository(root)
+
+    const first = await repository.apply(
+      upsert('.zennotes/vault.json', '{"favorites":["b"]}'),
+      undefined
+    )
+    expect(first).toEqual({
+      code: 'SETTINGS_CONFLICT',
+      path: '.zennotes/vault.json',
+      conflict_copy_path: '.zennotes/vault.cloud-conflict.json'
+    })
+    expect(await repository.pendingConflictPaths()).toEqual(['.zennotes/vault.json'])
+    // The settings in use are still this device's.
+    expect(await readFile(path.join(root, '.zennotes', 'vault.json'), 'utf8')).toBe(
+      '{"favorites":["a"]}'
+    )
+
+    // A newer cloud version replaces the pending one instead of piling up.
+    await repository.apply(upsert('.zennotes/vault.json', '{"favorites":["c"]}'), undefined)
+    expect(
+      await readFile(path.join(root, '.zennotes', 'vault.cloud-conflict.json'), 'utf8')
+    ).toBe('{"favorites":["c"]}')
+    expect((await readdir(path.join(root, '.zennotes'))).sort()).toEqual([
+      'vault.cloud-conflict.json',
+      'vault.json'
+    ])
+  })
+
+  it('keeps a locally edited file that the remote deleted', async () => {
     const root = await temporaryRoot()
     await writeFile(path.join(root, 'note.md'), 'local edit')
     const repository = new DesktopCloudSyncRepository(root)
 
-    await expect(
-      repository.apply(
-        {
-          sequence: 2,
-          item_id: 'item-1',
-          type: 'upsert',
-          path: 'note.md',
-          previous_path: null,
-          revision: 2,
-          content: {
-            encoding: 'utf8',
-            data: 'remote edit',
-            sha256: hash('remote edit'),
-            byte_length: 11,
-            media_type: 'text/markdown'
-          }
-        },
-        tracked('note.md', 'old contents')
-      )
-    ).rejects.toBeInstanceOf(CloudSyncLocalEditConflictError)
+    const conflict = await repository.apply(
+      {
+        sequence: 3,
+        item_id: 'item-1',
+        type: 'delete',
+        path: 'note.md',
+        previous_path: null,
+        revision: 3
+      },
+      tracked('note.md', 'old contents')
+    )
+
+    expect(conflict).toEqual({
+      code: 'LOCAL_EDIT_CONFLICT',
+      path: 'note.md',
+      conflict_copy_path: null
+    })
     expect(await readFile(path.join(root, 'note.md'), 'utf8')).toBe('local edit')
   })
+
+  it('accepts a delete for a file that is already gone locally', async () => {
+    const root = await temporaryRoot()
+    const repository = new DesktopCloudSyncRepository(root)
+
+    const conflict = await repository.apply(
+      {
+        sequence: 4,
+        item_id: 'item-1',
+        type: 'delete',
+        path: 'note.md',
+        previous_path: null,
+        revision: 4
+      },
+      tracked('note.md', 'old contents')
+    )
+
+    expect(conflict).toBeUndefined()
+  })
 })
 
 describe('DesktopCloudSyncStateStore', () => {

apps/desktop/src/main/cloud-sync-filesystem.ts

@@ -1,8 +1,16 @@
 import { createHash, randomUUID } from 'node:crypto'
 import { constants as fsConstants, promises as fs } from 'node:fs'
 import path from 'node:path'
-import type { CloudSyncChange, CloudSyncContent } from '@zennotes/bridge-contract/cloud-sync'
+import type {
+  CloudSyncChange,
+  CloudSyncContent,
+  CloudSyncLocalConflict
+} from '@zennotes/bridge-contract/cloud-sync'
 import {
+  CLOUD_SYNC_SETTINGS_CONFLICT_PATH,
+  CLOUD_SYNC_VAULT_SETTINGS_PATH,
+  cloudSyncConflictCopyPath,
+  isCloudSyncVaultSettingsPath,
   normalizeCloudSyncPath,
   shouldSyncVaultPath,
   shouldTraverseCloudSyncDirectory
@@ -80,21 +88,45 @@ export class DesktopCloudSyncRepository implements CloudSyncRepository {
     return items.sort((left, right) => left.path.localeCompare(right.path))
   }
 
-  async apply(change: CloudSyncChange, previous: CloudSyncTrackedItem | undefined): Promise<void> {
+  async pendingConflictPaths(): Promise<string[]> {
+    return (await exists(this.resolve(CLOUD_SYNC_SETTINGS_CONFLICT_PATH)))
+      ? [CLOUD_SYNC_VAULT_SETTINGS_PATH]
+      : []
+  }
+
+  async apply(
+    change: CloudSyncChange,
+    previous: CloudSyncTrackedItem | undefined
+  ): Promise<CloudSyncLocalConflict | void> {
     const affectedPaths = [change.path, change.previous_path, previous?.path].filter(
       (path): path is string => typeof path === 'string'
     )
     if (affectedPaths.some((path) => !shouldSyncVaultPath(path))) return
 
     if (change.type === 'upsert') {
       if (!change.content) throw new Error(`Upsert change ${change.sequence} did not include content`)
-      await this.assertUnchanged(previous?.path ?? change.path, previous)
+      const atTarget = await this.readIfExists(change.path)
+      // Already byte-for-byte what the change carries. There is nothing to
+      // write and nothing to conflict over, so adopt the file and move on.
+      // Without this, a file both sides already agree on stopped sync dead.
+      if (atTarget && sha256(atTarget) === change.content.sha256) return
+
+      const guardPath = previous?.path ?? change.path
+      const unvouched = await this.firstUnvouchedPath(
+        guardPath === change.path ? [change.path] : [guardPath, change.path],
+        previous
+      )
+      if (unvouched) return await this.keepBoth(change.path, decodeContent(change.content))
+
       await this.write(change.path, decodeContent(change.content))
       return
     }
 
     const previousPath = previous?.path ?? change.previous_path ?? change.path
-    await this.assertUnchanged(previousPath, previous)
+    const unvouched = await this.firstUnvouchedPath([previousPath], previous)
+    // A delete or a move carries no content to park, so keeping the local file
+    // where it is IS the preserved version. The next push re-uploads it.
+    if (unvouched) return localConflict(unvouched, null)
 
     if (change.type === 'delete') {
       await fs.rm(this.resolve(previousPath), { force: true })
@@ -104,9 +136,14 @@ export class DesktopCloudSyncRepository implements CloudSyncRepository {
     const source = this.resolve(previousPath)
     const destination = this.resolve(change.path)
     if (source === destination) return
+    if (!(await exists(source))) {
+      // Nothing here to move. Either the move already landed, or the file is
+      // gone locally and the next scan reconciles it.
+      return
+    }
     await fs.mkdir(path.dirname(destination), { recursive: true })
 
-    if (await exists(destination)) throw new CloudSyncLocalEditConflictError(change.path)
+    if (await exists(destination)) return localConflict(change.path, null)
     await fs.rename(source, destination)
   }
 
@@ -147,23 +184,55 @@ export class DesktopCloudSyncRepository implements CloudSyncRepository {
     return absolutePath
   }
 
-  private async assertUnchanged(
-    relPath: string,
-    previous: CloudSyncTrackedItem | undefined
-  ): Promise<void> {
-    const absolutePath = this.resolve(relPath)
-
+  private async readIfExists(relPath: string): Promise<Buffer | null> {
     try {
-      const bytes = await fs.readFile(absolutePath)
-      if (!previous || sha256(bytes) !== previous.sha256) {
-        throw new CloudSyncLocalEditConflictError(relPath)
-      }
+      return await fs.readFile(this.resolve(relPath))
     } catch (error) {
-      if (isMissingFileError(error) && !previous) return
+      if (isMissingFileError(error)) return null
       throw error
     }
   }
 
+  /**
+   * The first of these paths holding a file sync cannot vouch for, meaning it
+   * is not the exact bytes we last agreed on with the server. A file that is
+   * absent is fine: there is nothing there to lose.
+   */
+  private async firstUnvouchedPath(
+    relPaths: readonly string[],
+    previous: CloudSyncTrackedItem | undefined
+  ): Promise<string | null> {
+    for (const relPath of relPaths) {
+      const bytes = await this.readIfExists(relPath)
+      if (!bytes) continue
+      if (!previous || sha256(bytes) !== previous.sha256) return relPath
+    }
+    return null
+  }
+
+  /** Park the incoming version beside the local file rather than over it. */
+  private async keepBoth(relPath: string, bytes: Buffer): Promise<CloudSyncLocalConflict> {
+    // Settings are answered, not merged: the newest cloud version replaces any
+    // older pending one at a fixed path, and the app asks which side to keep.
+    if (isCloudSyncVaultSettingsPath(relPath)) {
+      await this.write(CLOUD_SYNC_SETTINGS_CONFLICT_PATH, bytes)
+      return {
+        code: 'SETTINGS_CONFLICT',
+        path: relPath,
+        conflict_copy_path: CLOUD_SYNC_SETTINGS_CONFLICT_PATH
+      }
+    }
+    for (let attempt = 1; attempt <= 100; attempt++) {
+      const candidate = cloudSyncConflictCopyPath(relPath, attempt)
+      if (await exists(this.resolve(candidate))) continue
+      await this.write(candidate, bytes)
+      return localConflict(relPath, candidate)
+    }
+    // A hundred conflict copies of one file means something is looping. Keep
+    // the local file and report it rather than filling the vault.
+    return localConflict(relPath, null)
+  }
+
   private async write(relPath: string, bytes: Buffer): Promise<void> {
     const destination = this.resolve(relPath)
     const temporaryPath = `${destination}.${process.pid}.${randomUUID()}.tmp`
@@ -272,6 +341,10 @@ function mediaType(relPath: string, text: boolean): string {
     (text ? 'text/plain' : 'application/octet-stream')
 }
 
+function localConflict(path: string, conflictCopyPath: string | null): CloudSyncLocalConflict {
+  return { code: 'LOCAL_EDIT_CONFLICT', path, conflict_copy_path: conflictCopyPath }
+}
+
 function sha256(bytes: Buffer): string {
   return createHash('sha256').update(bytes).digest('hex')
 }