Fix data-loss and security bugs: chapter id validation, reorder merge semantics, autosave cross-write after drag, listener re-registration, delete flush, failed-load handling

src/main/chapters.ts

@@ -164,11 +164,23 @@ export interface ChapterSnapshot {
   index: number;
 }
 
+// Chapter ids are always slugified lowercase ids; enforcing that shape here
+// keeps renderer-supplied snapshots from escaping the book directory.
+const SAFE_ID = /^[a-z0-9][a-z0-9-]*$/;
+
 // Re-create a previously deleted chapter at its original position, restoring
 // its title, color, and content. The chapter is stored as Markdown so it reads
-// correctly regardless of the format it originally used.
+// correctly regardless of the format it originally used. Idempotent: restoring
+// an id that still exists is a no-op instead of a duplicate.
 export function restoreChapter(bookPath: string, snap: ChapterSnapshot): ChapterEntry {
+  if (!snap || typeof snap.id !== "string" || !SAFE_ID.test(snap.id)) {
+    throw new Error("Invalid chapter id");
+  }
   const meta = loadBook(bookPath);
+  // Already present (e.g. undo clicked twice): return the existing entry
+  // unchanged rather than writing the file again and duplicating the id in
+  // chapterOrder.
+  if (meta.chapters[snap.id]) return meta.chapters[snap.id];
   const file = path.posix.join("chapters", chapterFileName(snap.id));
   fs.mkdirSync(path.join(bookPath, "chapters"), { recursive: true });
   fs.writeFileSync(path.join(bookPath, file), snap.content, "utf-8");
@@ -209,10 +221,25 @@ export function duplicateChapter(
 export function reorderChapters(bookPath: string, newOrder: string[]): void {
   const meta = loadBook(bookPath);
   const valid = new Set(Object.keys(meta.chapters));
-  if (!newOrder.every((id) => valid.has(id))) {
+  if (
+    !Array.isArray(newOrder) ||
+    new Set(newOrder).size !== newOrder.length ||
+    !newOrder.every((id) => valid.has(id))
+  ) {
     throw new Error("Invalid chapter order");
   }
-  meta.chapterOrder = newOrder;
+  // A partial order is allowed (callers may reorder just a subset), but it
+  // must MERGE with the existing sequence rather than replace it: replacing
+  // wholesale used to drop omitted ids from chapterOrder permanently, which
+  // silently hid those chapters from listings and exports.
+  const rest = meta.chapterOrder.filter((id) => !newOrder.includes(id));
+  const merged = [...newOrder, ...rest];
+  // Safety net: any chapter present in metadata but absent from the stored
+  // order (e.g. legacy folio.json drift) is appended rather than lost.
+  for (const id of Object.keys(meta.chapters)) {
+    if (!merged.includes(id)) merged.push(id);
+  }
+  meta.chapterOrder = merged;
   saveMeta(bookPath, meta);
 }
 

src/renderer/book.ts

@@ -739,30 +739,12 @@ function makeSnippet(plain: string, pos: number, len: number): string {
   const welcome = $("welcome");
   const recentList = $("recentList") as HTMLUListElement;
 
-  async function showWelcome() {
+
-    welcome.classList.remove("hidden");
+  // One-time wiring for search UI and the Save button. Previously this
-    chapterView.classList.add("hidden");
+  // block lived inside showWelcome(), re-registering ~15 listeners every
-    fullBookView.classList.add("hidden");
+  // time the welcome screen was shown (N Home trips = N duplicate saves
-    editorPlaceholder.classList.add("hidden");
+  // per click). Elements are static; bind once instead.
-    // Keep the project bar visible so the Home button is a persistent nav
+  function initSearchUI(): void {
-    // control. On Home it is shown disabled/active (you are already here).
-    projectBar.classList.remove("hidden");
-    bookTitleEl.textContent = "";
-    bookTitleEl.classList.add("hidden");
-    bookPathEl.classList.add("hidden");
-    bookPathEl.title = "";
-    $("revealBtn")?.classList.add("hidden");
-    const hb = homeBtn as HTMLButtonElement | null;
-    if (hb) {
-      hb.disabled = true;
-      hb.classList.add("active");
-      hb.setAttribute("aria-current", "page");
-    }
-    currentBook = null;
-    meta = null;
-    ordered = [];
-    selectedId = null;
-    await renderRecents();
   // ---- search across all chapters ----
   const searchBtn = $("searchBtn") as HTMLButtonElement;
   const searchModal = $("searchModal") as HTMLDivElement;
@@ -987,6 +969,32 @@ const tmp = createTiptapEditor(host);
       if (first) first.click();
     }
   });
+  }
+
+  async function showWelcome() {
+    welcome.classList.remove("hidden");
+    chapterView.classList.add("hidden");
+    fullBookView.classList.add("hidden");
+    editorPlaceholder.classList.add("hidden");
+    // Keep the project bar visible so the Home button is a persistent nav
+    // control. On Home it is shown disabled/active (you are already here).
+    projectBar.classList.remove("hidden");
+    bookTitleEl.textContent = "";
+    bookTitleEl.classList.add("hidden");
+    bookPathEl.classList.add("hidden");
+    bookPathEl.title = "";
+    $("revealBtn")?.classList.add("hidden");
+    const hb = homeBtn as HTMLButtonElement | null;
+    if (hb) {
+      hb.disabled = true;
+      hb.classList.add("active");
+      hb.setAttribute("aria-current", "page");
+    }
+    currentBook = null;
+    meta = null;
+    ordered = [];
+    selectedId = null;
+    await renderRecents();
 
   setChapterButtons(false);
     fullBookBtn.classList.remove("active");
@@ -1038,6 +1046,17 @@ const tmp = createTiptapEditor(host);
     }
     setChapterButtons(!!meta);
     renderChapterList();
+    // Preserve the chapter currently being read. Metadata refreshes fire on
+    // every rename/color/reorder, and jumping back to the first chapter
+    // here would yank the editor out from under the user mid-session.
+    const keepId = selectedId && meta?.chapters[selectedId] ? selectedId : null;
+    if (keepId) {
+      const entry = meta!.chapters[keepId];
+      chapterTitleEl.textContent = entry.title || keepId;
+      updateCommentsBadge();
+      await refreshFullBook();
+      return;
+    }
     if (ordered.length) {
       await selectChapter(ordered[0].id);
     } else {
@@ -1232,6 +1251,10 @@ const tmp = createTiptapEditor(host);
     const el = $("undoToast");
     if (!el) return;
     el.classList.remove("show");
+    // Drop the handler so a second click during the hide animation cannot
+    // run the undo twice (restore would duplicate the chapter entry).
+    const btn = $("undoToastBtn") as HTMLButtonElement | null;
+    if (btn) btn.onclick = null;
     setTimeout(() => el.classList.add("hidden"), 200);
   }
   function showUndoToast(msg: string, onUndo: () => void, timeout = 9000): void {
@@ -1243,10 +1266,13 @@ const tmp = createTiptapEditor(host);
     const msgEl = $("undoToastMsg");
     const btn = $("undoToastBtn") as HTMLButtonElement | null;
     if (msgEl) msgEl.textContent = msg;
-    if (btn) btn.onclick = () => {
+    if (btn)
-      hideUndoToast();
+      btn.onclick = () => {
-      onUndo();
+        // Single-fire: detach before running so double-clicks are ignored.
-    };
+        btn.onclick = null;
+        hideUndoToast();
+        onUndo();
+      };
     el.classList.remove("hidden");
     el.classList.add("show");
     if (undoToastTimer) clearTimeout(undoToastTimer);
@@ -1275,39 +1301,53 @@ const tmp = createTiptapEditor(host);
     showToast("Saved");
   }
 
+  // Guards against overlapping selections: rapid clicks (or metadata
+  // refreshes) can interleave loads; only the newest one may commit state.
+  let selectToken = 0;
   async function selectChapter(id: string) {
     clearTimers();
     if (selectedId && dirty) {
       // Auto-save silently so navigating away never loses text and returning to
-      // this chapter shows what was just written.
+      // this chapter shows what was just written. The save targets the
+      // chapter still selected right now, so it happens before reassignment.
       await saveCurrentChapter();
     }
+    const token = ++selectToken;
+    let res: { doc?: never; error?: string } | null = null;
+    try {
+      res = await api.loadChapterDoc(id);
+    } catch {
+      res = null;
+    }
+    // A newer selection superseded this one: drop the stale reply instead of
+    // pairing its document with the wrong chapter id.
+    if (token !== selectToken) return;
+    if (!(res && "doc" in res)) {
+      // Load failed. Keep the current editor contents and selection intact —
+      // seeding an empty document here would let the next autosave overwrite
+      // the real chapter file with near-empty content.
+      const msg = res && "error" in res ? String(res.error) : "The chapter file could not be read.";
+      void alertMessage("Couldn't open chapter", msg);
+      return;
+    }
     selectedId = id;
     renderChapterList();
     updateCommentsBadge();
-    try {
+    const doc = (res as { doc: never }).doc;
-      const res = await api.loadChapterDoc(id);
+    const title = (meta && meta.chapters[id]?.title) || id;
-      const doc =
+    chapterTitleEl.textContent = title;
-        res && "doc" in res
+    setEditorDoc(editor, doc);
-          ? (res as { doc: never }).doc
+    loadedDocJson = JSON.stringify(getEditorDoc(editor));
-          : ({ type: "doc", content: [] } as never);
+    dirty = false;
-      const title = (meta && meta.chapters[id]?.title) || id;
+    saveStateEl.textContent = "Saved";
-      chapterTitleEl.textContent = title;
+    saveStateEl.classList.remove("unsaved");
-      setEditorDoc(editor, doc);
+    wordCountEl.textContent = formatWordCount(editor.getText());
-      loadedDocJson = JSON.stringify(getEditorDoc(editor));
+    editor.commands.focus("end");
-      dirty = false;
+    clearTimers();
-      saveStateEl.textContent = "Saved";
+    fullBookBtn.classList.remove("active");
-      saveStateEl.classList.remove("unsaved");
+    fullBookView.classList.add("hidden");
-      wordCountEl.textContent = formatWordCount(editor.getText());
+    chapterView.classList.remove("hidden");
-      editor.commands.focus("end");
+    editorPlaceholder.classList.add("hidden");
-      clearTimers();
-      fullBookBtn.classList.remove("active");
-      fullBookView.classList.add("hidden");
-      chapterView.classList.remove("hidden");
-      editorPlaceholder.classList.add("hidden");
-    } catch {
-      showPlaceholder();
-    }
   }
 
   // ---- full book view ----
@@ -1623,6 +1663,14 @@ const tmp = createTiptapEditor(host);
     if (!meta || !meta.chapters[id]) return;
     const entry = meta.chapters[id];
     const index = meta.chapterOrder.indexOf(id);
+    // Flush unsaved edits before snapshotting: the undo snapshot reads from
+    // disk, so text typed since the last autosave would otherwise be lost
+    // even through Undo. Also stop a pending autosave timer from firing
+    // while the chapter is being removed.
+    if (selectedId === id) {
+      clearTimers();
+      if (dirty) await saveCurrentChapter();
+    }
     const content = (await api.getChapterContent(id)) as string;
     const res = await api.deleteChapter(id);
     if (res && "error" in res) {
@@ -1841,7 +1889,11 @@ const tmp = createTiptapEditor(host);
         ordered = moveInList(ordered, from, to);
         meta!.chapterOrder = ordered.map((c) => c.id);
         renderChapterList();
-        selectedId = draggingId;
+        // Selecting the dragged row must go through selectChapter: assigning
+        // selectedId directly would pair the id with whatever document is
+        // still in the editor, and the next autosave would write that
+        // document under the dragged chapter's id.
+        void selectChapter(draggingId);
         void persistOrder();
       }
     }
@@ -2301,6 +2353,7 @@ const tmp = createTiptapEditor(host);
   });
 
   setChapterButtons(false);
+  initSearchUI();
   void showWelcome();
 
   (window as any).__folioEditor = editor;