Keep database-sourced nodes persisted under skipPersistence (closes #2940)

packages/desktop-app/src/lib/services/navigation-service.ts

@@ -137,8 +137,9 @@ export class NavigationService {
           return null;
         }
 
-        // Add to store for future use.
-        // Use type 'database' and skipPersistence since already in backend (or virtual)
+        // Add to store for future use. The 'database' source is what records the
+        // node in persistedNodeIds, so its first edit takes the update path;
+        // skipPersistence only suppresses the write (already in backend, or virtual).
         sharedNodeStore.setNode(
           node,
           { type: 'database', reason: 'fetched-for-link-click' },

packages/desktop-app/src/lib/services/shared-node-store.svelte.ts

@@ -1214,16 +1214,14 @@ export class SharedNodeStore {
   // ========================================================================
 
   /**
-   * Determine persistence behavior from explicit options or legacy source type.
-   *
-   * This helper implements the new explicit persistence API while maintaining
-   * backward compatibility with the legacy `source.type === 'database'` checks.
+   * Determine persistence behavior from explicit options and the source type.
    *
    * Priority (highest to lowest):
    * 1. options.markAsPersistedOnly - Mark as persisted without re-persisting
-   * 2. options.skipPersistence - Skip persistence
+   * 2. options.skipPersistence - Suppress the write; a database source is
+   *    still marked persisted, since the row exists in the backend
    * 3. options.persist - Explicit persistence control
-   * 4. Legacy: source.type === 'database' - Auto-skip persistence
+   * 4. source.type === 'database' - No write, mark persisted
    * 5. Default: Auto-determine based on source type and changes
    *
    * @returns Object with shouldPersist and shouldMarkAsPersisted flags
@@ -1238,9 +1236,12 @@ export class SharedNodeStore {
       return { shouldPersist: false, shouldMarkAsPersisted: true };
     }
 
-    // Priority 2: Skip persistence flag (legacy compatibility)
+    // Priority 2: Skip persistence flag. It suppresses the write only — a
+    // database-sourced node still exists in the backend, so it must still be
+    // tracked as persisted. Otherwise a later update to it takes the create
+    // path and the backend rejects the duplicate insert.
     if (options.skipPersistence) {
-      return { shouldPersist: false, shouldMarkAsPersisted: false };
+      return { shouldPersist: false, shouldMarkAsPersisted: source.type === 'database' };
     }
 
     // Priority 3: Explicit persist option (new API)

packages/desktop-app/src/lib/stores/ai-chats.svelte.ts

@@ -133,11 +133,10 @@ class AiChatsStore {
       // QueryNodeViewer's `handleCreateInstance` already does for its own
       // "+ New" call site) so the viewer's first write, once the tab opens,
       // finds the id already known-persisted. Without this, the id is only
-      // hydrated later via the sync/domain-event echo of this very create,
-      // which sets `skipPersistence: true` and therefore never marks it
-      // persisted — so the viewer's first write (model selection or the
-      // first message) wrongly re-issues a CREATE for an id that already
-      // has a row, colliding with it.
+      // marked persisted once the sync/domain-event echo of this very create
+      // arrives — and if the viewer's first write (model selection or the
+      // first message) races ahead of that echo, it wrongly re-issues a
+      // CREATE for an id that already has a row, colliding with it.
       sharedNodeStore.setNode(created, { type: 'database', reason: 'ai-chat-created' });
       this.state = { ...this.state, chats: [toListItem(created), ...this.state.chats] };
       return created;

packages/desktop-app/src/tests/services/navigated-root-persisted-regression.test.ts

@@ -0,0 +1,83 @@
+/**
+ * Regression coverage for a root node opened by navigation being inserted
+ * into `SharedNodeStore` without persisted tracking.
+ *
+ * `NavigationService.resolveNodeTarget()` fetches a node that is not yet in
+ * the store and inserts it with a `database` source plus `skipPersistence`.
+ * `skipPersistence` used to outrank the database source in
+ * `determinePersistenceBehavior`, so the node was never added to
+ * `persistedNodeIds`. The viewer's `loadChildrenTree()` then skips the
+ * already-cached root, so nothing marked it later either — and the first edit
+ * took the create path, which the backend rejected as a duplicate insert.
+ */
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { getNavigationService } from '$lib/services/navigation-service';
+import { sharedNodeStore } from '$lib/services/shared-node-store.svelte';
+import { backendAdapter } from '$lib/services/backend-adapter';
+import type { Node } from '$lib/types';
+
+const ROOT_ID = 'navigated-root-person';
+
+function personNode(overrides: Partial<Node> = {}): Node {
+  return {
+    id: ROOT_ID,
+    nodeType: 'person',
+    content: '',
+    createdAt: '2026-01-01T00:00:00.000Z',
+    modifiedAt: '2026-01-01T00:00:00.000Z',
+    version: 1,
+    properties: { first_name: 'Ada', last_name: 'Lovelace', email: 'ada@example.com' },
+    ...overrides
+  } as Node;
+}
+
+describe('navigated-to root node — persisted tracking', () => {
+  beforeEach(() => {
+    sharedNodeStore.clearAll();
+  });
+
+  afterEach(() => {
+    sharedNodeStore.clearAll();
+    vi.restoreAllMocks();
+  });
+
+  it('marks the root persisted, so an edit takes the update path', async () => {
+    vi.spyOn(backendAdapter, 'getNode').mockResolvedValue(personNode());
+    const { id, nodeType, content, version, createdAt, modifiedAt, properties } = personNode();
+    vi.spyOn(backendAdapter, 'getChildrenTree').mockResolvedValue({
+      id,
+      nodeType,
+      content,
+      version,
+      createdAt,
+      modifiedAt,
+      properties,
+      children: []
+    });
+    vi.spyOn(backendAdapter, 'getMentioningContainers').mockResolvedValue([]);
+    const createNodeSpy = vi.spyOn(backendAdapter, 'createNode').mockResolvedValue(ROOT_ID);
+    const updateNodeSpy = vi
+      .spyOn(backendAdapter, 'updateNode')
+      .mockImplementation(async (_id, version, update) =>
+        personNode({ ...(update as Partial<Node>), version: version + 1 })
+      );
+
+    // Navigation inserts the root before the viewer loads its tree.
+    const target = await getNavigationService().resolveNodeTarget(ROOT_ID);
+    expect(target?.nodeId).toBe(ROOT_ID);
+    await sharedNodeStore.loadChildrenTree(ROOT_ID);
+
+    expect(sharedNodeStore.isNodePersisted(ROOT_ID)).toBe(true);
+
+    sharedNodeStore.updateNode(
+      ROOT_ID,
+      { properties: { first_name: 'Ada', last_name: 'Byron', email: 'ada@example.com' } },
+      { type: 'viewer', viewerId: 'person-viewer' }
+    );
+
+    await vi.waitFor(() => {
+      expect(updateNodeSpy).toHaveBeenCalledWith(ROOT_ID, 1, expect.anything());
+    });
+    expect(createNodeSpy).not.toHaveBeenCalled();
+  });
+});

packages/desktop-app/src/tests/services/shared-node-store.test.ts

@@ -2410,6 +2410,14 @@ describe('SharedNodeStore', () => {
         // Node should not be marked as persisted
         expect(store.isNodePersisted(testNode.id)).toBe(false);
       });
+
+      it('still marks a database-sourced node persisted when skipping persistence', () => {
+        const testNode = createTestNode('persist-skip-db-1');
+        store.setNode(testNode, { type: 'database', reason: 'test' }, true);
+
+        // skipPersistence suppresses the write only — the row exists in the backend
+        expect(store.isNodePersisted(testNode.id)).toBe(true);
+      });
     });
 
     describe('batchSetNodes', () => {