fix(sync,chat,notes): delete resurrection, maxSteps persistence, chat quick-settings, notes DnD

changelogs/v2.5.5.md

@@ -0,0 +1,17 @@
+# v2.5.5
+
+## Added
+
+- **Chat settings, without leaving the chat.** A new gear icon in the chat toolbar opens a compact popover to change the model, max tool-call steps, temperature, and subagents on the spot — no round-trip to Settings just to bump the step limit. The model picker uses the app's dropdown with a **Refresh models** action (re-queries your endpoint) and a **Custom model…** option for typing any model id the endpoint doesn't list. A "More settings…" link jumps to the full AI & Chat settings when you need endpoints, API keys, or MCP.
+- **The notes folder tree now remembers which folders are expanded.** Collapse/expand state is saved per project and restored when you switch views, change projects, or reload — no more re-opening the same folders every time you return to Notes.
+
+## Changed
+
+- **Subagents is now a single global preference** (in Settings → AI & Chat, and in the new in-chat gear popover) instead of a per-conversation toggle. Turn it on once and every chat uses the dispatch → research/write architecture. The old per-thread "Subagents" pill in the chat toolbar has been replaced by the gear popover.
+- **The "Move to root" drop zone is now pinned to the bottom of the notes panel** while dragging, instead of appearing inline between folders — so it's always in the same, easy-to-reach spot no matter how far you've scrolled.
+
+## Fixed
+
+- **Deleting a note no longer briefly "comes back" during sync.** When you deleted a note on desktop and then synced again before the other device (e.g. your phone) had picked up the deletion, the peer's still-stale copy of the note could be re-applied and resurrect the note — until the other device finally synced the deletion and it disappeared again. The desktop deletes rows physically (leaving no local tombstone), so the sync engine had nothing to compare an incoming older edit against and let it win. The engine now leaves a tombstone marker whenever a hard-deleted row is drained, so a stale edit from a peer can no longer bring a deleted note back. Deletions now stick immediately and both devices converge on the delete.
+- **Changing the chat tool-call limit ("Max steps") now sticks.** Setting a new limit (including "∞") in Settings would appear to work, then quietly revert to 30 partway through a conversation. The setting was being dropped from the backend settings cache, so the next time the app refreshed its config — which happens automatically after any turn that writes with a tool — the limit reset to the default. The full AI settings (max steps, temperature, context limit, AI-enabled) are now preserved end to end, and the config refresh no longer overwrites richer locally-saved settings with a partial cache.
+- **The "Move to root" notes drop zone no longer lingers after dragging a note or folder onto another project.** Because the dragged item moves out of the current project (and its row unmounts) the moment you drop it on another project, the drag-end that clears the drop zone never fired — so the zone stayed on screen until the next click. Drag state is now cleared reliably via a document-level listener, so the zone disappears as soon as you let go.

electron/lib/config-cache.test.ts

@@ -14,7 +14,70 @@
  *   2. getEmbeddingsSettingsCached() must run without throwing when electron
  *      is unavailable — returning a plain object (possibly empty).
  */
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import fs from "fs";
+import os from "os";
+import path from "path";
+
+// Point config-cache's userData resolver at a temp dir so the round-trip tests
+// below never touch the developer's real ai-settings-cache.json.
+let tmpDir: string;
+vi.mock("../runtime/port-discovery", () => ({
+  findUserDataDir: () => tmpDir,
+}));
+
+describe("config-cache AI settings round-trip", () => {
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "cairn-cfg-"));
+  });
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it("persists maxSteps (and other behavioural fields) for the 'ai' config", async () => {
+    const { saveCachedConfig, getCachedConfig } = await import("./config-cache");
+    saveCachedConfig("ai", {
+      provider: "openai",
+      baseUrl: "https://api.openai.com",
+      model: "gpt-4o",
+      apiKey: "sk-x",
+      maxSteps: 1000,
+      temperature: 0.5,
+      contextLimit: 200000,
+      aiEnabled: true,
+      subagentsEnabled: true,
+    });
+    const ai = getCachedConfig().aiConfig;
+    // Regression: maxSteps used to be silently dropped here, which reset the
+    // chat tool-call limit to the default (30) on the next hydrate.
+    expect(ai?.maxSteps).toBe(1000);
+    expect(ai?.temperature).toBe(0.5);
+    expect(ai?.contextLimit).toBe(200000);
+    expect(ai?.aiEnabled).toBe(true);
+    expect(ai?.subagentsEnabled).toBe(true);
+    expect(ai?.model).toBe("gpt-4o");
+  });
+
+  it("persists subagentsEnabled (global subagents preference)", async () => {
+    const { saveCachedConfig, getCachedConfig } = await import("./config-cache");
+    saveCachedConfig("ai", { subagentsEnabled: true });
+    expect(getCachedConfig().aiConfig?.subagentsEnabled).toBe(true);
+    // A later connection-only save must not clobber it.
+    saveCachedConfig("ai", { model: "gpt-4o" });
+    expect(getCachedConfig().aiConfig?.subagentsEnabled).toBe(true);
+  });
+
+  it("a later connection-only save preserves an existing maxSteps", async () => {
+    const { saveCachedConfig, getCachedConfig } = await import("./config-cache");
+    saveCachedConfig("ai", { maxSteps: 500 });
+    // A subsequent save that only carries connection fields (e.g. from the
+    // provider picker) must not wipe the previously-stored maxSteps.
+    saveCachedConfig("ai", { model: "gpt-4o-mini" });
+    const ai = getCachedConfig().aiConfig;
+    expect(ai?.maxSteps).toBe(500);
+    expect(ai?.model).toBe("gpt-4o-mini");
+  });
+});
 
 describe("config-cache without Electron", () => {
   it("imports without throwing the electron install error", async () => {

electron/lib/config-cache.ts

@@ -15,6 +15,15 @@ export interface CachedConfig {
     baseUrl?: string;
     model?: string;
     apiKey?: string;
+    // Persist the FULL renderer AIConfig, not just connection fields. Dropping
+    // these here caused hydrateFromElectron() to reset them to DEFAULT_AI_CONFIG
+    // after every write-tool turn — e.g. the chat tool-call limit (maxSteps)
+    // silently "reverted to 30" mid-conversation.
+    maxSteps?: number;
+    temperature?: number;
+    contextLimit?: number;
+    aiEnabled?: boolean;
+    subagentsEnabled?: boolean;
   };
   agentConfig?: {
     baseUrl?: string;
@@ -80,6 +89,13 @@ export function saveCachedConfig(type: "ai" | "agent" | "embeddings" | "theme" |
         baseUrl: typeof configRecord.baseUrl === "string" ? configRecord.baseUrl : current.aiConfig?.baseUrl,
         model: typeof configRecord.model === "string" ? configRecord.model : current.aiConfig?.model,
         apiKey: typeof configRecord.apiKey === "string" ? configRecord.apiKey : current.aiConfig?.apiKey,
+        // Preserve behavioural fields too — omitting maxSteps here made the chat
+        // tool-call limit reset to the default on the next hydrate.
+        maxSteps: typeof configRecord.maxSteps === "number" ? configRecord.maxSteps : current.aiConfig?.maxSteps,
+        temperature: typeof configRecord.temperature === "number" ? configRecord.temperature : current.aiConfig?.temperature,
+        contextLimit: typeof configRecord.contextLimit === "number" ? configRecord.contextLimit : current.aiConfig?.contextLimit,
+        aiEnabled: typeof configRecord.aiEnabled === "boolean" ? configRecord.aiEnabled : current.aiConfig?.aiEnabled,
+        subagentsEnabled: typeof configRecord.subagentsEnabled === "boolean" ? configRecord.subagentsEnabled : current.aiConfig?.subagentsEnabled,
       };
     } else if (type === "agent" && configRecord) {
       current.agentConfig = {

shared/sync/engine.test.ts

@@ -749,6 +749,35 @@ describe("sync engine — trigger→drain capture of real queries.ts writes", ()
       expect((B.db.prepare("SELECT COUNT(*) c FROM notes WHERE id='n1' AND deleted_at IS NULL").get() as { c: number }).c).toBe(0);
     });
 
+    it("a stale peer put drained AFTER a local hard-delete does not resurrect the row", () => {
+      // Real-world timing bug: desktop deletes a note (physical DELETE, no local
+      // tombstone), publishes the delete, then syncs AGAIN and reads the phone's
+      // still-stale oplog (a put@lowerHLC published before the phone saw the
+      // delete). With no local tombstone the staleness guard has nothing to
+      // compare against, so the older put would resurrect the note. The
+      // drainPending tombstone-shell fix keeps the delete sticky.
+      const clkA = clockFrom(12_150_000);
+      const clkB = clockFrom(12_150_000);
+      const A = makeDevice("A", clkA.now);
+      const B = makeDevice("B", clkB.now);
+      seedNoteOnBoth(A, B);
+      // Phone publishes its current (live) state — a put with the seed's HLC.
+      writeOplogFile(dir, "B", B.engine.exportOplog());
+      expect((B.db.prepare("SELECT COUNT(*) c FROM notes WHERE id='n1' AND deleted_at IS NULL").get() as { c: number }).c).toBe(1);
+
+      // Desktop deletes and republishes (higher HLC).
+      clkA.advance(100);
+      q.deleteNote(A.db, "n1");
+      A.engine.drainPending();
+      writeOplogFile(dir, "A", A.engine.exportOplog());
+      expect((A.db.prepare("SELECT COUNT(*) c FROM notes WHERE id='n1' AND deleted_at IS NULL").get() as { c: number }).c).toBe(0);
+
+      // Desktop syncs again while the phone is still offline: it reconciles the
+      // phone's stale put. The note must STAY deleted on desktop.
+      A.engine.applyRemote(readPeerOplogs(dir, "A"));
+      expect((A.db.prepare("SELECT COUNT(*) c FROM notes WHERE id='n1' AND deleted_at IS NULL").get() as { c: number }).c).toBe(0);
+    });
+
     it("a concurrent higher-HLC peer edit wins over an earlier hard-delete (row comes back on both)", () => {
       // Desktop hard-deletes n1, but the phone had a LATER edit it hadn't synced
       // yet. LWW: the higher-HLC edit wins, so the note is restored — and both

shared/sync/engine.ts

@@ -188,14 +188,35 @@ export class SyncEngine {
       if (p.seq > maxSeq) maxSeq = p.seq;
     }
 
+    // A hard-deleted row leaves no tombstone, so we may need to insert a
+    // tombstone SHELL (placeholder FK columns) to keep the staleness guard armed
+    // against a later stale peer 'put'. Shells legitimately violate FKs, so
+    // disable enforcement around the transaction (the pragma is a no-op inside
+    // one) exactly as applyRemote does. Restored in `finally`.
+    const fkRow = this.db.prepare("PRAGMA foreign_keys").get() as { foreign_keys?: number } | undefined;
+    const fkWasOn = fkRow?.foreign_keys === 1;
+    if (fkWasOn) this.db.prepare("PRAGMA foreign_keys = OFF").run();
+
     const run = this.db.transaction(() => {
       this.setSuppress(true); // our hlc/deleted_at writes below must not re-stage
       for (const { entity, entity_id, op } of latest.values()) {
         const stamp = this.hlc.send();
         const row = readRow(this.db, entity, entity_id);
         if (op === "delete" || !row) {
           // Row gone (hard delete) or explicitly tombstoned.
-          this.db.prepare(`UPDATE ${entity} SET deleted_at = COALESCE(deleted_at, ?), hlc = ? WHERE id = ?`).run(nowIso(), stamp, entity_id);
+          const upd = this.db
+            .prepare(`UPDATE ${entity} SET deleted_at = COALESCE(deleted_at, ?), hlc = ? WHERE id = ?`)
+            .run(nowIso(), stamp, entity_id) as { changes?: number };
+          // Desktop's q.deleteNote does a physical `DELETE FROM notes`, so by the
+          // time we drain the staged 'delete' the row is already gone and the
+          // UPDATE above matches zero rows — leaving NO local tombstone. Without a
+          // tombstone, a later peer 'put' with an OLDER HLC hits reconcileOne with
+          // local=undefined (localHlc=null), bypasses the staleness guard, and
+          // RESURRECTS the deleted row. Insert a tombstone shell (same as the
+          // delete-arrives-before-insert case) so the staleness guard trips and
+          // the stale put stays dead. FK columns on the shell are placeholders;
+          // the row is a tombstone and never read as live data.
+          if ((upd.changes ?? 0) === 0) this.insertTombstoneShell(entity, entity_id, stamp);
           this.appendOplog({ hlc: stamp, origin: this.deviceId, entity, entity_id, op: "delete", payload: null });
         } else {
           this.db.prepare(`UPDATE ${entity} SET hlc = ? WHERE id = ?`).run(stamp, entity_id);
@@ -218,7 +239,11 @@ export class SyncEngine {
       this.setSuppress(false);
       this.persistHlc();
     });
-    run();
+    try {
+      run();
+    } finally {
+      if (fkWasOn) this.db.prepare("PRAGMA foreign_keys = ON").run();
+    }
     return latest.size;
   }