Prevent sync data loss from incomplete tracked files (#725)

Co-authored-by: Zhizhen He <hezhizhen.yi@gmail.com>

Cargo.lock

@@ -11667,6 +11667,7 @@ dependencies = [
  "serde_path_to_error",
  "serde_yaml",
  "sha1 0.11.0",
+ "tempfile",
  "thiserror 2.0.20",
  "tokio",
  "ts-rs",

apps/yaak-client/commands/createEnvironment.tsx

@@ -6,11 +6,12 @@ import { showDialog } from "../lib/dialog";
 import { jotaiStore } from "../lib/jotai";
 import { setWorkspaceSearchParams } from "../lib/setWorkspaceSearchParams";
 
-export const createSubEnvironmentAndActivate = createFastMutation<
-  string | null,
-  unknown,
-  Environment | null
->({
+/**
+ * Prompt for and create a sub-environment of the given base environment, resolving with the new
+ * environment's ID (or null if the dialog was dismissed). This does not change the workspace's
+ * active environment.
+ */
+export const createSubEnvironment = createFastMutation<string | null, unknown, Environment | null>({
   mutationKey: ["create_environment"],
   mutationFn: async (baseEnvironment) => {
     if (baseEnvironment == null) {
@@ -41,6 +42,17 @@ export const createSubEnvironmentAndActivate = createFastMutation<
       });
     });
   },
+});
+
+/** Same as {@link createSubEnvironment}, but also makes the new environment the active one. */
+export const createSubEnvironmentAndActivate = createFastMutation<
+  string | null,
+  unknown,
+  Environment | null
+>({
+  mutationKey: ["create_environment_and_activate"],
+  mutationFn: (baseEnvironment) => createSubEnvironment.mutateAsync(baseEnvironment),
+  disableToastError: true, // Already shown by createSubEnvironment
   onSuccess: async (environmentId) => {
     if (environmentId == null) {
       return; // Was not created

apps/yaak-client/components/EnvironmentEditDialog.tsx

@@ -4,8 +4,16 @@ import type { TreeHandle, TreeNode, TreeProps } from "@yaakapp-internal/ui";
 import { Banner, Icon, InlineCode, SplitLayout, Tree } from "@yaakapp-internal/ui";
 import { atom, useAtomValue } from "jotai";
 import { atomFamily } from "jotai-family";
-import { useCallback, useLayoutEffect, useRef, useState } from "react";
-import { createSubEnvironmentAndActivate } from "../commands/createEnvironment";
+import {
+  createContext,
+  useCallback,
+  useContext,
+  useEffect,
+  useLayoutEffect,
+  useRef,
+  useState,
+} from "react";
+import { createSubEnvironment } from "../commands/createEnvironment";
 import { activeWorkspaceAtom, activeWorkspaceIdAtom } from "../hooks/useActiveWorkspace";
 import {
   environmentsBreakdownAtom,
@@ -40,6 +48,9 @@ interface Props {
 
 type TreeModel = Environment | Workspace;
 
+/** Lets tree row slots create a sub-environment and select it in the editor. */
+const CreateSubEnvironmentContext = createContext<() => Promise<void>>(async () => {});
+
 export function EnvironmentEditDialog({ initialEnvironmentId, setRef }: Props) {
   const { allEnvironments, baseEnvironment, baseEnvironments } = useEnvironmentsBreakdown();
   const [selectedEnvironmentId, setSelectedEnvironmentId] = useState<string | null>(
@@ -115,7 +126,7 @@ function EnvironmentEditDialogSidebar({
   const activeWorkspaceId = useAtomValue(activeWorkspaceIdAtom) ?? "";
   const treeId = `environment.${activeWorkspaceId}.sidebar`;
   const treeRef = useRef<TreeHandle>(null);
-  const { baseEnvironment, baseEnvironments } = useEnvironmentsBreakdown();
+  const { allEnvironments, baseEnvironment, baseEnvironments } = useEnvironmentsBreakdown();
 
   // oxlint-disable-next-line react-hooks/exhaustive-deps -- none
   useLayoutEffect(() => {
@@ -124,6 +135,31 @@ function EnvironmentEditDialogSidebar({
     treeRef.current?.focus();
   }, []);
 
+  // A just-created environment to select once it reaches the model store. Selecting it before
+  // then would show a "failed to find" error in the editor, and the tree would drop the
+  // selection of an item it doesn't have yet.
+  const [pendingSelectionId, setPendingSelectionId] = useState<string | null>(null);
+  const tree = useAtomValue(treeAtom);
+  useEffect(() => {
+    if (pendingSelectionId == null) return;
+    if (!allEnvironments.some((e) => e.id === pendingSelectionId)) return;
+
+    setPendingSelectionId(null);
+    setSelectedEnvironmentId(pendingSelectionId);
+
+    // The tree leaves out sub-environments while there are multiple base environments
+    const node = tree == null ? null : findTreeNode(tree, pendingSelectionId);
+    if (node == null) return;
+    if (node.parent != null) {
+      // Expand the parent so the new item is visible to be selected
+      const parentId = node.parent.item.id;
+      jotaiStore.set(collapsedFamily(treeId), (prev) => ({ ...prev, [parentId]: false }));
+    }
+    // Wait a frame so the tree's own pending selection fixups, which may have been scheduled
+    // before the new item arrived, don't reset this selection
+    requestAnimationFrame(() => treeRef.current?.selectItem(pendingSelectionId));
+  }, [allEnvironments, pendingSelectionId, setSelectedEnvironmentId, tree, treeId]);
+
   const handleDeleteEnvironment = useCallback(
     async (environment: Environment) => {
       await deleteModelWithConfirm(environment);
@@ -134,6 +170,14 @@ function EnvironmentEditDialogSidebar({
     [baseEnvironment?.id, selectedEnvironmentId, setSelectedEnvironmentId],
   );
 
+  const handleCreateSubEnvironment = useCallback(async () => {
+    const { baseEnvironment } = jotaiStore.get(environmentsBreakdownAtom);
+    if (baseEnvironment == null) return;
+    const id = await createSubEnvironment.mutateAsync(baseEnvironment);
+    // Show the new environment in the editor without changing the workspace's active environment
+    if (id != null) setPendingSelectionId(id);
+  }, []);
+
   const treeHasFocus = useCallback(() => treeRef.current?.hasFocus() ?? false, []);
 
   const getSelectedTreeModels = useCallback(
@@ -195,13 +239,14 @@ function EnvironmentEditDialogSidebar({
       const addEnvironmentItem: DropdownItem = {
         label: "Create Sub Environment",
         leftSlot: <Icon icon="plus" />,
-        onSelect: async () => {
-          await createSubEnvironment();
-        },
+        onSelect: handleCreateSubEnvironment,
       };
+      // Sub-environments aren't shown while there are multiple base environments, same as the
+      // plus button on the base environment row
+      const canCreateSubEnvironment = baseEnvironments.length <= 1;
 
       if (environment == null || environment.model !== "environment") {
-        return [addEnvironmentItem];
+        return canCreateSubEnvironment ? [addEnvironmentItem] : [];
       }
 
       const singleEnvironment = items.length === 1;
@@ -257,7 +302,7 @@ function EnvironmentEditDialogSidebar({
       ];
 
       // Add sub environment to base environment
-      if (isBaseEnvironment(environment) && singleEnvironment) {
+      if (isBaseEnvironment(environment) && singleEnvironment && canCreateSubEnvironment) {
         menuItems.push({ type: "separator" });
         menuItems.push(addEnvironmentItem);
       }
@@ -266,6 +311,7 @@ function EnvironmentEditDialogSidebar({
     },
     [
       baseEnvironments.length,
+      handleCreateSubEnvironment,
       handleDeleteEnvironment,
       handleDuplicateSelected,
       handleRenameSelected,
@@ -317,34 +363,37 @@ function EnvironmentEditDialogSidebar({
   );
 
   const renderContextMenuFn = useCallback<NonNullable<TreeProps<TreeModel>["renderContextMenu"]>>(
-    ({ items, position, onClose }) => (
-      <ContextMenu items={items as DropdownItem[]} triggerPosition={position} onClose={onClose} />
-    ),
+    ({ items, position, onClose }) =>
+      // Right-clicking empty space has nothing to offer while there are multiple base environments
+      items.length === 0 ? null : (
+        <ContextMenu items={items as DropdownItem[]} triggerPosition={position} onClose={onClose} />
+      ),
     [],
   );
 
-  const tree = useAtomValue(treeAtom);
   return (
     <aside className="x-theme-sidebar h-full w-full min-w-0 grid overflow-y-auto border-r border-border-subtle ">
       {tree != null && (
-        <div className="pt-2">
-          <Tree
-            ref={treeRef}
-            treeId={treeId}
-            collapsedAtom={collapsedFamily(treeId)}
-            className="px-2 pb-10"
-            root={tree}
-            getContextMenu={getContextMenu}
-            renderContextMenu={renderContextMenuFn}
-            onDragEnd={handleDragEnd}
-            getItemKey={(i) => `${i.id}::${i.name}`}
-            ItemLeftSlotInner={ItemLeftSlotInner}
-            ItemRightSlot={ItemRightSlot}
-            ItemInner={ItemInner}
-            onActivate={handleActivate}
-            getEditOptions={getEditOptions}
-          />
-        </div>
+        <CreateSubEnvironmentContext.Provider value={handleCreateSubEnvironment}>
+          <div className="pt-2">
+            <Tree
+              ref={treeRef}
+              treeId={treeId}
+              collapsedAtom={collapsedFamily(treeId)}
+              className="px-2 pb-10"
+              root={tree}
+              getContextMenu={getContextMenu}
+              renderContextMenu={renderContextMenuFn}
+              onDragEnd={handleDragEnd}
+              getItemKey={(i) => `${i.id}::${i.name}`}
+              ItemLeftSlotInner={ItemLeftSlotInner}
+              ItemRightSlot={ItemRightSlot}
+              ItemInner={ItemInner}
+              onActivate={handleActivate}
+              getEditOptions={getEditOptions}
+            />
+          </div>
+        </CreateSubEnvironmentContext.Provider>
       )}
     </aside>
   );
@@ -384,6 +433,15 @@ const treeAtom = atom<TreeNode<TreeModel> | null>((get) => {
   return root;
 });
 
+function findTreeNode(node: TreeNode<TreeModel>, id: string): TreeNode<TreeModel> | null {
+  if (node.item.id === id) return node;
+  for (const child of node.children ?? []) {
+    const found = findTreeNode(child, id);
+    if (found != null) return found;
+  }
+  return null;
+}
+
 function ItemLeftSlotInner({ item }: { item: TreeModel }) {
   const { baseEnvironments } = useEnvironmentsBreakdown();
   return baseEnvironments.length > 1 ? (
@@ -395,6 +453,7 @@ function ItemLeftSlotInner({ item }: { item: TreeModel }) {
 
 function ItemRightSlot({ item }: { item: TreeModel }) {
   const { baseEnvironments } = useEnvironmentsBreakdown();
+  const handleCreateSubEnvironment = useContext(CreateSubEnvironmentContext);
   return (
     <>
       {item.model === "environment" && baseEnvironments.length <= 1 && isBaseEnvironment(item) && (
@@ -405,7 +464,7 @@ function ItemRightSlot({ item }: { item: TreeModel }) {
           icon="plus_circle"
           className="opacity-50 hover:opacity-100"
           title="Add Sub-Environment"
-          onClick={createSubEnvironment}
+          onClick={handleCreateSubEnvironment}
         />
       )}
     </>
@@ -425,13 +484,6 @@ function ItemInner({ item }: { item: TreeModel }) {
   );
 }
 
-async function createSubEnvironment() {
-  const { baseEnvironment } = jotaiStore.get(environmentsBreakdownAtom);
-  if (baseEnvironment == null) return;
-  const id = await createSubEnvironmentAndActivate.mutateAsync(baseEnvironment);
-  return id;
-}
-
 function getEditOptions(item: TreeModel) {
   const options: ReturnType<NonNullable<TreeProps<TreeModel>["getEditOptions"]>> = {
     defaultValue: item.name,

apps/yaak-client/components/core/BulkPairEditor.test.ts

@@ -1,5 +1,10 @@
 import { describe, expect, test } from "vite-plus/test";
-import { parseBulkPairLine } from "./BulkPairEditor";
+import {
+  formatBulkPairLine,
+  formatBulkPairs,
+  parseBulkPairLine,
+  parseBulkPairs,
+} from "./BulkPairEditor";
 
 describe("parseBulkPairLine", () => {
   test("parses colon-space pairs as name and value", () => {
@@ -33,4 +38,130 @@ describe("parseBulkPairLine", () => {
       value: "bar\nbaz",
     });
   });
+
+  test("parses commented-out pairs as disabled", () => {
+    expect(parseBulkPairLine("# foo: bar")).toMatchObject({
+      enabled: false,
+      name: "foo",
+      value: "bar",
+    });
+  });
+
+  test("allows any whitespace after the comment marker", () => {
+    expect(parseBulkPairLine("#   foo: bar")).toMatchObject({
+      enabled: false,
+      name: "foo",
+      value: "bar",
+    });
+    expect(parseBulkPairLine("#\tfoo: bar")).toMatchObject({
+      enabled: false,
+      name: "foo",
+      value: "bar",
+    });
+  });
+
+  test("treats a # without whitespace after it as part of the name", () => {
+    expect(parseBulkPairLine("#foo: bar")).toMatchObject({
+      enabled: true,
+      name: "#foo",
+      value: "bar",
+    });
+  });
+
+  test("parses commented-out pairs with an empty value as disabled", () => {
+    expect(parseBulkPairLine("# token:")).toMatchObject({
+      enabled: false,
+      name: "token",
+      value: "",
+    });
+    expect(parseBulkPairLine("# token: ")).toMatchObject({
+      enabled: false,
+      name: "token",
+      value: "",
+    });
+  });
+
+  test("drops comments that are not pairs", () => {
+    expect(parseBulkPairLine("# just a comment")).toBeNull();
+    expect(parseBulkPairLine("# see http://example.com")).toBeNull();
+    expect(parseBulkPairLine("#")).toBeNull();
+  });
+
+  test("leaves values that start with # alone", () => {
+    expect(parseBulkPairLine("color: #fff")).toMatchObject({
+      enabled: true,
+      name: "color",
+      value: "#fff",
+    });
+  });
+});
+
+describe("formatBulkPairLine", () => {
+  test("formats enabled pairs as name and value", () => {
+    expect(formatBulkPairLine({ enabled: true, name: "foo", value: "bar" })).toBe("foo: bar");
+  });
+
+  test("treats pairs without an enabled flag as enabled", () => {
+    expect(formatBulkPairLine({ name: "foo", value: "bar" })).toBe("foo: bar");
+  });
+
+  test("comments out disabled pairs", () => {
+    expect(formatBulkPairLine({ enabled: false, name: "foo", value: "bar" })).toBe("# foo: bar");
+  });
+
+  test("escapes newlines in values", () => {
+    expect(formatBulkPairLine({ enabled: false, name: "foo", value: "a\nb" })).toBe("# foo: a\\nb");
+  });
+});
+
+describe("bulk pair round trip", () => {
+  test("preserves order and enabled state", () => {
+    const pairs = [
+      { enabled: true, name: "host", value: "example.com" },
+      { enabled: false, name: "token", value: "old" },
+      { enabled: true, name: "color", value: "#fff" },
+      { enabled: false, name: "multi", value: "a\nb" },
+      { enabled: true, name: "token", value: "new" },
+    ];
+
+    const text = formatBulkPairs(pairs);
+    expect(text).toBe(
+      ["host: example.com", "# token: old", "color: #fff", "# multi: a\\nb", "token: new"].join(
+        "\n",
+      ),
+    );
+
+    const parsed = parseBulkPairs(text);
+    expect(parsed.map(({ enabled, name, value }) => ({ enabled, name, value }))).toEqual(pairs);
+  });
+
+  test("preserves enabled pairs whose name starts with #", () => {
+    const pairs = [{ enabled: true, name: "#foo", value: "bar" }];
+    const text = formatBulkPairs(pairs);
+    expect(text).toBe("#foo: bar");
+    const parsed = parseBulkPairs(text);
+    expect(parsed.map(({ enabled, name, value }) => ({ enabled, name, value }))).toEqual(pairs);
+  });
+
+  test("preserves disabled pairs whose name starts with #", () => {
+    const pairs = [{ enabled: false, name: "#foo", value: "bar" }];
+    const text = formatBulkPairs(pairs);
+    expect(text).toBe("# #foo: bar");
+    const parsed = parseBulkPairs(text);
+    expect(parsed.map(({ enabled, name, value }) => ({ enabled, name, value }))).toEqual(pairs);
+  });
+
+  test("preserves disabled pairs with empty values", () => {
+    const pairs = [{ enabled: false, name: "token", value: "" }];
+    const parsed = parseBulkPairs(formatBulkPairs(pairs));
+    expect(parsed.map(({ enabled, name, value }) => ({ enabled, name, value }))).toEqual(pairs);
+  });
+
+  test("drops free-text comments and blank lines between pairs", () => {
+    const parsed = parseBulkPairs("# Auth\nfoo: bar\n\n# baz: qux\n# see the docs");
+    expect(parsed.map(({ enabled, name, value }) => ({ enabled, name, value }))).toEqual([
+      { enabled: true, name: "foo", value: "bar" },
+      { enabled: false, name: "baz", value: "qux" },
+    ]);
+  });
 });

apps/yaak-client/components/core/BulkPairEditor.tsx

@@ -14,20 +14,11 @@ export function BulkPairEditor({
   forcedEnvironmentId,
   stateKey,
 }: Props) {
-  const pairsText = useMemo(() => {
-    return pairs
-      .filter((p) => !(p.name.trim() === "" && p.value.trim() === ""))
-      .map(formatBulkPairLine)
-      .join("\n");
-  }, [pairs]);
+  const pairsText = useMemo(() => formatBulkPairs(pairs), [pairs]);
 
   const handleChange = useCallback(
     (text: string) => {
-      const pairs = text
-        .split("\n")
-        .filter((l: string) => l.trim())
-        .map(parseBulkPairLine);
-      onChange(pairs);
+      onChange(parseBulkPairs(text));
     },
     [onChange],
   );
@@ -47,13 +38,57 @@ export function BulkPairEditor({
   );
 }
 
-export function formatBulkPairLine(pair: Pair) {
-  const value = pair.value.replaceAll("\n", "\\n");
-  return `${pair.name}: ${value}`;
+export function formatBulkPairs(pairs: Pair[]): string {
+  return pairs
+    .filter((p) => !(p.name.trim() === "" && p.value.trim() === ""))
+    .map(formatBulkPairLine)
+    .join("\n");
 }
 
-export function parseBulkPairLine(line: string): PairWithId {
-  const [, name, value] = line.match(/^([^:]+):\s+(.*)$/) ?? [];
+export function parseBulkPairs(text: string): PairWithId[] {
+  return text
+    .split("\n")
+    .filter((l: string) => l.trim())
+    .map(parseBulkPairLine)
+    .filter((p) => p != null);
+}
+
+/**
+ * Format a pair as a `name: value` line. Disabled pairs are commented out dotenv-style, as
+ * `# name: value`, so the enabled state survives a round trip through {@link parseBulkPairLine}.
+ */
+export function formatBulkPairLine(pair: Pair) {
+  const value = pair.value.replaceAll("\n", "\\n");
+  const line = `${pair.name}: ${value}`;
+  return pair.enabled === false ? `# ${line}` : line;
+}
+
+const PAIR_REGEX = /^([^:]+):\s+(.*)$/;
+// Commented-out pairs may also end at the colon, so a hand-typed `# name:` isn't dropped
+const COMMENTED_PAIR_REGEX = /^([^:]+):(?:\s+(.*))?$/;
+// A # only marks a comment when followed by whitespace (or nothing), so `#foo: bar` stays an
+// enabled pair named `#foo`
+const COMMENT_PREFIX_REGEX = /^\s*#(?:\s+|$)/;
+
+/**
+ * Parse a `name: value` line into an enabled pair. A line starting with `# ` is a disabled pair
+ * if the rest of it parses as `name: value`, otherwise it's a free-text comment and `null` is
+ * returned so it can be dropped.
+ */
+export function parseBulkPairLine(line: string): PairWithId | null {
+  const commentPrefix = line.match(COMMENT_PREFIX_REGEX);
+  if (commentPrefix != null) {
+    const uncommented = line.slice(commentPrefix[0].length);
+    const match = uncommented.match(COMMENTED_PAIR_REGEX);
+    if (match == null) return null;
+    return { ...pairFromMatch(uncommented, match), enabled: false };
+  }
+
+  return pairFromMatch(line, line.match(PAIR_REGEX));
+}
+
+function pairFromMatch(line: string, match: RegExpMatchArray | null): PairWithId {
+  const [, name, value] = match ?? [];
   return {
     enabled: true,
     name: (name ?? line).trim(),

apps/yaak-client/components/core/Editor/Editor.tsx

@@ -442,7 +442,21 @@ function EditorInner({
           ? EditorState.fromJSON(cachedJsonState, config, stateFields)
           : EditorState.create(config);
 
-        const view = new EditorView({ state, parent: container });
+        // PERF: Built detached and attached in a microtask, rather than passing `parent` and
+        // letting the constructor attach it first.
+        //
+        // The constructor sets `contenteditable` on the content element and then reads
+        // `document.fonts.ready`. Both force the browser to flush style and layout when the
+        // element is already in the document, and whatever the previous editor attached has
+        // just dirtied it again — so mounting n editors in one commit costs n forced layouts.
+        // The environment editor mounts two per variable row (~60 of them), which is where its
+        // half-second open went.
+        //
+        // Building detached moves that work off the live document, and deferring the attach to
+        // a microtask means every editor in the commit is built before any of them is inserted:
+        // one forced layout for the batch instead of one each. The microtask still runs before
+        // paint, so nothing is visible in the meantime.
+        const view = new EditorView({ state });
 
         // For large documents, the parser may parse the max number of lines and fail to add
         // things like fold markers because of it.
@@ -450,12 +464,26 @@ function EditorInner({
         forceParsing(view, 9e6, 100);
 
         cm.current = { view, languageCompartment };
-        if (autoFocus) {
-          view.focus();
-        }
-        if (autoSelect) {
-          view.dispatch({ selection: { anchor: 0, head: view.state.doc.length } });
-        }
+        queueMicrotask(() => {
+          // The editor was torn down (or replaced) before we got here
+          if (cm.current?.view !== view) return;
+
+          // Runs after the surrounding try/catch has returned, so it needs its own
+          try {
+            container.appendChild(view.dom);
+            // Everything the constructor measured, it measured detached
+            view.requestMeasure();
+
+            if (autoFocus) {
+              view.focus();
+            }
+            if (autoSelect) {
+              view.dispatch({ selection: { anchor: 0, head: view.state.doc.length } });
+            }
+          } catch (e) {
+            console.log("Failed to attach Codemirror", e);
+          }
+        });
         setRef?.(view);
       } catch (e) {
         console.log("Failed to initialize Codemirror", e);

apps/yaak-client/components/responseViewers/PdfViewer.tsx

@@ -5,13 +5,11 @@ import type { PDFDocumentProxy } from "pdfjs-dist";
 import { useMemo, useRef, useState } from "react";
 import { Document, Page, pdfjs } from "react-pdf";
 import { useContainerSize } from "@yaakapp-internal/ui";
+import pdfWorkerUrl from "./pdfWorker?worker&url";
 
 // Document can start loading during render, so configure its worker synchronously.
 // Vite bundles the installed PDF.js worker as a local asset, including offline builds.
-pdfjs.GlobalWorkerOptions.workerSrc = new URL(
-  "pdfjs-dist/build/pdf.worker.min.mjs",
-  import.meta.url,
-).toString();
+pdfjs.GlobalWorkerOptions.workerSrc = new URL(pdfWorkerUrl, import.meta.url).toString();
 
 interface Props {
   /** A URL for the body the host already stored. */

apps/yaak-client/components/responseViewers/pdfWorker.ts

@@ -0,0 +1,3 @@
+// The worker has its own globals, so it needs the polyfills before PDF.js evaluates
+import "../../polyfills";
+import "pdfjs-dist/build/pdf.worker.min.mjs";

apps/yaak-client/index.html

@@ -31,6 +31,7 @@
     <div id="cm-portal" class="cm-portal"></div>
     <div id="react-portal"></div>
     <div id="radix-portal" class="cm-portal"></div>
+    <script type="module" src="/polyfills.ts"></script>
     <script type="module" src="/theme.ts"></script>
     <script type="module" src="/font-size.ts"></script>
     <script type="module" src="/font.ts"></script>

apps/yaak-client/polyfills.test.ts

@@ -0,0 +1,167 @@
+import { describe, expect, test, vi } from "vite-plus/test";
+import { getOrInsert, getOrInsertComputed, readableStreamValues, sumPrecise } from "./polyfills";
+
+describe("getOrInsert", () => {
+  test("returns the existing value without replacing it", () => {
+    const map = new Map([["a", 1]]);
+    expect(getOrInsert.call(map, "a", 2)).toBe(1);
+    expect(map.get("a")).toBe(1);
+  });
+
+  test("inserts and returns the value for a missing key", () => {
+    const map = new Map<string, number>();
+    expect(getOrInsert.call(map, "a", 2)).toBe(2);
+    expect(map.get("a")).toBe(2);
+  });
+
+  test("works on a WeakMap", () => {
+    const key = {};
+    const map = new WeakMap<object, number>();
+    expect(getOrInsert.call(map, key, 1)).toBe(1);
+    expect(getOrInsert.call(map, key, 2)).toBe(1);
+  });
+});
+
+describe("getOrInsertComputed", () => {
+  test("returns the existing value without calling the callback", () => {
+    const map = new Map([["a", 1]]);
+    const callback = vi.fn(() => 2);
+    expect(getOrInsertComputed.call(map, "a", callback)).toBe(1);
+    expect(callback).not.toHaveBeenCalled();
+  });
+
+  test("calls the callback with the key and inserts its result", () => {
+    const map = new Map<string, string>();
+    expect(getOrInsertComputed.call(map, "a", (key) => `${String(key)}!`)).toBe("a!");
+    expect(map.get("a")).toBe("a!");
+  });
+
+  test("stores what the callback returns even if the callback set the key", () => {
+    const map = new Map<string, number>();
+    const result = getOrInsertComputed.call(map, "a", () => {
+      map.set("a", 1);
+      return 2;
+    });
+    expect(result).toBe(2);
+    expect(map.get("a")).toBe(2);
+  });
+
+  test("canonicalizes -0 to +0", () => {
+    const map = new Map<number, number>();
+    getOrInsertComputed.call(map, -0, (key) => {
+      expect(Object.is(key, 0)).toBe(true);
+      return 1;
+    });
+    expect(Object.is([...map.keys()][0], 0)).toBe(true);
+  });
+
+  test("throws when the callback is not a function, even for an existing key", () => {
+    const map = new Map([["a", 1]]);
+    // @ts-expect-error testing a non-callable callback
+    expect(() => getOrInsertComputed.call(map, "a", 1)).toThrow(TypeError);
+  });
+
+  test("is installed on Map and WeakMap", () => {
+    const map = new Map<string, number[]>();
+    map.getOrInsertComputed("a", () => []).push(1);
+    map.getOrInsertComputed("a", () => []).push(2);
+    expect(map.get("a")).toEqual([1, 2]);
+
+    const key = {};
+    const weakMap = new WeakMap<object, number>();
+    expect(weakMap.getOrInsertComputed(key, () => 1)).toBe(1);
+    expect(weakMap.getOrInsert(key, 2)).toBe(1);
+  });
+});
+
+describe("sumPrecise", () => {
+  test("sums without intermediate rounding", () => {
+    expect(sumPrecise([1e20, 0.1, -1e20])).toBe(0.1);
+    expect(sumPrecise([0.1, 0.2, 0.3])).toBe(0.6);
+    expect(sumPrecise(Array.from({ length: 10 }, () => 0.1))).toBe(1);
+  });
+
+  test("rounds half to even on the final step", () => {
+    expect(sumPrecise([1, 1e-16, 1e-16])).toBe(1.0000000000000002);
+    expect(sumPrecise([1, 2 ** -53])).toBe(1);
+  });
+
+  test("handles zeroes, infinities and NaN like the spec", () => {
+    expect(Object.is(sumPrecise([]), -0)).toBe(true);
+    expect(Object.is(sumPrecise([-0, -0]), -0)).toBe(true);
+    expect(Object.is(sumPrecise([-0, 0]), 0)).toBe(true);
+    expect(Object.is(sumPrecise([1, -1]), 0)).toBe(true);
+    expect(sumPrecise([1, Infinity])).toBe(Infinity);
+    expect(sumPrecise([1, -Infinity])).toBe(-Infinity);
+    expect(sumPrecise([Infinity, -Infinity])).toBeNaN();
+    expect(sumPrecise([1, NaN])).toBeNaN();
+  });
+
+  test("accepts any iterable and rejects non-numbers", () => {
+    expect(sumPrecise(new Set([1, 2, 3]))).toBe(6);
+    // @ts-expect-error testing a non-number item
+    expect(() => sumPrecise([1, "2"])).toThrow(TypeError);
+  });
+
+  test("is installed on Math", () => {
+    expect(Math.sumPrecise([1, 2, 3])).toBe(6);
+  });
+});
+
+describe("readableStreamValues", () => {
+  const values = readableStreamValues<number>;
+
+  function streamOf(chunks: number[], onCancel?: () => void) {
+    return new ReadableStream<number>({
+      start(controller) {
+        for (const chunk of chunks) controller.enqueue(chunk);
+        controller.close();
+      },
+      cancel: onCancel,
+    });
+  }
+
+  test("yields every chunk and releases the lock", async () => {
+    const stream = streamOf([1, 2, 3]);
+    const seen: number[] = [];
+    for await (const chunk of values.call(stream)) seen.push(chunk);
+    expect(seen).toEqual([1, 2, 3]);
+    expect(stream.locked).toBe(false);
+  });
+
+  test("cancels the stream when the loop exits early", async () => {
+    const onCancel = vi.fn();
+    const stream = new ReadableStream<number>({
+      pull(controller) {
+        controller.enqueue(1);
+      },
+      cancel: onCancel,
+    });
+    for await (const _ of values.call(stream)) break;
+    expect(onCancel).toHaveBeenCalled();
+    expect(stream.locked).toBe(false);
+  });
+
+  test("leaves the stream open with preventCancel", async () => {
+    const onCancel = vi.fn();
+    const stream = streamOf([1, 2], onCancel);
+    for await (const _ of values.call(stream, { preventCancel: true })) break;
+    expect(onCancel).not.toHaveBeenCalled();
+    expect(stream.locked).toBe(false);
+  });
+
+  test("releases the lock and rethrows when cancelling rejects", async () => {
+    const stream = new ReadableStream<number>({
+      pull(controller) {
+        controller.enqueue(1);
+      },
+      cancel() {
+        throw new Error("cancel failed");
+      },
+    });
+    await expect(async () => {
+      for await (const _ of values.call(stream)) break;
+    }).rejects.toThrow("cancel failed");
+    expect(stream.locked).toBe(false);
+  });
+});

apps/yaak-client/polyfills.ts

@@ -0,0 +1,172 @@
+/**
+ * Built-ins that PDF.js's modern build calls unconditionally but the system WebKit on
+ * older macOS lacks: the upsert methods and `Math.sumPrecise` arrived in Safari 26.2,
+ * and async iteration of `ReadableStream` in Safari 27. Without them the PDF viewer
+ * throws as soon as it renders. Loaded first by `index.html`, and by `pdfWorker.ts`
+ * because the worker has its own globals.
+ *
+ * Each is defined only when missing, so newer engines keep their native versions.
+ */
+
+type KeyedCollection<K, V> = {
+  has(key: K): boolean;
+  get(key: K): V | undefined;
+  set(key: K, value: V): unknown;
+};
+
+/** https://tc39.es/proposal-upsert/#sec-map.prototype.getOrInsert */
+export function getOrInsert<K, V>(this: KeyedCollection<K, V>, key: K, value: V): V {
+  if (this.has(key)) return this.get(key) as V;
+  this.set(key, value);
+  return value;
+}
+
+/** https://tc39.es/proposal-upsert/#sec-map.prototype.getOrInsertComputed */
+export function getOrInsertComputed<K, V>(
+  this: KeyedCollection<K, V>,
+  key: K,
+  callbackfn: (key: K) => V,
+): V {
+  if (typeof callbackfn !== "function") {
+    throw new TypeError("getOrInsertComputed callback is not a function");
+  }
+  if (this.has(key)) return this.get(key) as V;
+  // Map keys are canonicalized, so -0 is stored and passed on as +0
+  const k = (Object.is(key, -0) ? 0 : key) as K;
+  const value = callbackfn(k);
+  // The spec sets after the callback even if the callback inserted the key itself
+  this.set(k, value);
+  return value;
+}
+
+/**
+ * https://tc39.es/proposal-math-sum/
+ *
+ * Shewchuk's exact summation with round-half-even on the final step (the algorithm
+ * behind Python's `math.fsum`). A finite input set whose running sum overflows
+ * partway through is not handled exactly, which PDF.js's byte lengths and widths
+ * never come near.
+ */
+export function sumPrecise(items: Iterable<number>): number {
+  const partials: number[] = [];
+  let allNegativeZero = true;
+  let sawNaN = false;
+  let sawPositiveInfinity = false;
+  let sawNegativeInfinity = false;
+
+  for (const item of items) {
+    if (typeof item !== "number") {
+      throw new TypeError("Math.sumPrecise requires an iterable of numbers");
+    }
+    if (!Object.is(item, -0)) allNegativeZero = false;
+    if (Number.isNaN(item)) {
+      sawNaN = true;
+    } else if (item === Infinity) {
+      sawPositiveInfinity = true;
+    } else if (item === -Infinity) {
+      sawNegativeInfinity = true;
+    } else {
+      let x = item;
+      let i = 0;
+      for (let y of partials) {
+        if (Math.abs(x) < Math.abs(y)) [x, y] = [y, x];
+        const hi = x + y;
+        const lo = y - (hi - x);
+        if (lo !== 0) partials[i++] = lo;
+        x = hi;
+      }
+      partials.length = i;
+      partials.push(x);
+    }
+  }
+
+  if (sawNaN || (sawPositiveInfinity && sawNegativeInfinity)) return NaN;
+  if (sawPositiveInfinity) return Infinity;
+  if (sawNegativeInfinity) return -Infinity;
+  if (allNegativeZero) return -0;
+
+  let n = partials.length;
+  let hi = 0;
+  let lo = 0;
+  if (n > 0) {
+    hi = partials[--n] as number;
+    while (n > 0) {
+      const x = hi;
+      const y = partials[--n] as number;
+      hi = x + y;
+      lo = y - (hi - x);
+      if (lo !== 0) break;
+    }
+    const next = partials[n - 1];
+    if (n > 0 && next != null && ((lo < 0 && next < 0) || (lo > 0 && next > 0))) {
+      const y = lo * 2;
+      const x = hi + y;
+      if (y === x - hi) hi = x;
+    }
+  }
+  return hi === 0 ? 0 : hi;
+}
+
+/** https://streams.spec.whatwg.org/#rs-asynciterator */
+export async function* readableStreamValues<R>(
+  this: ReadableStream<R>,
+  { preventCancel = false }: { preventCancel?: boolean } = {},
+): AsyncGenerator<R, undefined, undefined> {
+  const reader = this.getReader();
+  let finished = false;
+  try {
+    while (true) {
+      const { done, value } = await reader.read();
+      if (done) {
+        finished = true;
+        return;
+      }
+      yield value;
+    }
+  } catch (err) {
+    finished = true;
+    throw err;
+  } finally {
+    // Leaving the loop early (break, return, throw in the body) cancels the stream. The
+    // lock is released even if cancelling rejects, which then propagates as in the spec.
+    try {
+      if (!finished && !preventCancel) await reader.cancel();
+    } finally {
+      reader.releaseLock();
+    }
+  }
+}
+
+function define(target: object, key: PropertyKey, value: unknown) {
+  if (key in target) return;
+  Object.defineProperty(target, key, { value, writable: true, configurable: true });
+}
+
+define(Map.prototype, "getOrInsert", getOrInsert);
+define(Map.prototype, "getOrInsertComputed", getOrInsertComputed);
+define(WeakMap.prototype, "getOrInsert", getOrInsert);
+define(WeakMap.prototype, "getOrInsertComputed", getOrInsertComputed);
+define(Math, "sumPrecise", sumPrecise);
+if (typeof ReadableStream !== "undefined") {
+  define(ReadableStream.prototype, "values", readableStreamValues);
+  define(
+    ReadableStream.prototype,
+    Symbol.asyncIterator,
+    Reflect.get(ReadableStream.prototype, "values"),
+  );
+}
+
+// TypeScript's lib does not describe these yet
+declare global {
+  interface Map<K, V> {
+    getOrInsert(key: K, value: V): V;
+    getOrInsertComputed(key: K, callbackfn: (key: K) => V): V;
+  }
+  interface WeakMap<K extends WeakKey, V> {
+    getOrInsert(key: K, value: V): V;
+    getOrInsertComputed(key: K, callbackfn: (key: K) => V): V;
+  }
+  interface Math {
+    sumPrecise(items: Iterable<number>): number;
+  }
+}

crates-tauri/yaak-app-client/src/sync_ext.rs

@@ -29,7 +29,7 @@ pub(crate) async fn cmd_sync_calculate<R: Runtime>(
     let db = app_handle.db();
     let version = app_handle.package_info().version.to_string();
     let db_candidates = get_db_candidates(&db, &version, workspace_id, sync_dir)?;
-    let fs_candidates = get_fs_candidates(sync_dir)?
+    let fs_candidates = get_fs_candidates(sync_dir, &db_candidates)?
         .into_iter()
         // Only keep items in the same workspace
         .filter(|fs| fs.model.workspace_id() == workspace_id)
@@ -39,7 +39,7 @@ pub(crate) async fn cmd_sync_calculate<R: Runtime>(
 
 pub(crate) async fn cmd_sync_calculate_fs(dir: &Path) -> Result<Vec<SyncOp>> {
     let db_candidates = Vec::new();
-    let fs_candidates = get_fs_candidates(dir)?;
+    let fs_candidates = get_fs_candidates(dir, &db_candidates)?;
     Ok(compute_sync_ops(db_candidates, fs_candidates))
 }
 

crates/yaak-git/src/status.rs

@@ -135,7 +135,7 @@ pub fn git_status(dir: &Path) -> crate::error::Result<GitStatusSummary> {
                     let obj = entry.to_object(&repo)?;
                     let content = obj.as_blob().unwrap().content();
                     let name = Path::new(entry.name().unwrap_or_default());
-                    SyncModel::from_bytes(content.into(), name)?.map(|m| m.0)
+                    SyncModel::from_bytes(content.into(), name).ok().flatten().map(|m| m.0)
                 }
                 Err(_) => None,
             },
@@ -143,7 +143,7 @@ pub fn git_status(dir: &Path) -> crate::error::Result<GitStatusSummary> {
 
         let next = {
             let full_path = repo.workdir().unwrap().join(rela_path.clone());
-            SyncModel::from_file(full_path.as_path())?.map(|m| m.0)
+            SyncModel::from_file(full_path.as_path()).ok().flatten().map(|m| m.0)
         };
 
         entries.push(GitStatusEntry {

crates/yaak-http/src/types.rs

@@ -181,7 +181,11 @@ fn strip_query_params(url: &str, names: &[&str]) -> String {
 }
 
 fn build_url(r: &HttpRequest) -> String {
-    let (url_string, params) = apply_path_placeholders(&ensure_proto(&r.url), &r.url_parameters);
+    // Trim stray whitespace: a leading space breaks scheme
+    // detection in `ensure_proto` (IdnaError); a trailing space becomes `/path%20` -> 404.
+    let trimmed_url = r.url.trim();
+    let (url_string, params) =
+        apply_path_placeholders(&ensure_proto(trimmed_url), &r.url_parameters);
     let mut url = append_query_params(
         &url_string,
         params
@@ -817,6 +821,64 @@ mod tests {
         assert_eq!(result, "https://example.com/api?foo=bar#section");
     }
 
+    #[test]
+    fn test_build_url_trims_leading_whitespace() {
+        // A leading space must not break scheme detection (it used to produce
+        // `http:// https://...`, which fails to parse with an IdnaError).
+        let r = HttpRequest {
+            url: " https://example.com/api".to_string(),
+            url_parameters: vec![],
+            ..Default::default()
+        };
+
+        let result = build_url(&r);
+        assert_eq!(result, "https://example.com/api");
+    }
+
+    #[test]
+    fn test_build_url_trims_trailing_whitespace() {
+        let r = HttpRequest {
+            url: "https://example.com/api ".to_string(),
+            url_parameters: vec![],
+            ..Default::default()
+        };
+
+        let result = build_url(&r);
+        assert_eq!(result, "https://example.com/api");
+    }
+
+    #[test]
+    fn test_build_url_trims_whitespace_with_params() {
+        // A trailing space must not end up percent-encoded in the path once query
+        // parameters are appended (e.g. `/api%20?foo=bar`, which 404s).
+        let r = HttpRequest {
+            url: " https://example.com/api ".to_string(),
+            url_parameters: vec![HttpUrlParameter {
+                enabled: true,
+                name: "foo".to_string(),
+                value: "bar".to_string(),
+                id: None,
+            }],
+            ..Default::default()
+        };
+
+        let result = build_url(&r);
+        assert_eq!(result, "https://example.com/api?foo=bar");
+    }
+
+    #[test]
+    fn test_build_url_trims_whitespace_without_scheme() {
+        let r = HttpRequest {
+            url: "  example.com/api\t".to_string(),
+            url_parameters: vec![],
+            ..Default::default()
+        };
+
+        let result = build_url(&r);
+        // ensure_proto defaults to http:// for regular domains
+        assert_eq!(result, "http://example.com/api");
+    }
+
     #[test]
     fn test_build_url_with_existing_query_and_fragment() {
         let r = HttpRequest {

crates/yaak-sync/Cargo.toml

@@ -18,3 +18,6 @@ tokio = { workspace = true, features = ["fs", "sync", "macros", "rt"] }
 ts-rs = { workspace = true, features = ["chrono-impl", "serde-json-impl"] }
 yaak-models = { workspace = true }
 serde_path_to_error = "0.1.20"
+
+[dev-dependencies]
+tempfile = "3"

crates/yaak-sync/src/models.rs

@@ -1,7 +1,7 @@
 use crate::error::Error::UnknownModel;
 use crate::error::Result;
 use chrono::NaiveDateTime;
-use log::{debug, warn};
+use log::debug;
 use serde::{Deserialize, Deserializer, Serialize};
 use serde_yaml::{Mapping, Value};
 use sha1::{Digest, Sha1};
@@ -98,11 +98,13 @@ fn migrate_environment(obj: &mut Mapping) {
 }
 
 impl SyncModel {
+    /// Returns `Ok(None)` for files that don't look like Yaak models, and an error for ones that
+    /// do but can't be parsed.
     pub fn from_bytes(content: Vec<u8>, file_path: &Path) -> Result<Option<(SyncModel, String)>> {
         let mut hasher = Sha1::new();
         hasher.update(&content);
         let checksum = hex::encode(hasher.finalize());
-        let content_str = String::from_utf8(content.clone()).unwrap_or_default();
+        let content_str = String::from_utf8(content).unwrap_or_default();
 
         // Check for some strings that will be in a model file for sure. If these strings
         // don't exist, then it's probably not a Yaak file.
@@ -111,34 +113,19 @@ impl SyncModel {
         }
 
         let ext = file_path.extension().unwrap_or_default();
-        if ext == "yml" || ext == "yaml" {
-            Ok(match serde_yaml::from_str::<SyncModel>(&content_str) {
-                Ok(m) => Some((m, checksum)),
-                Err(e) => {
-                    warn!("Error parsing {:?} {:?}", file_path.file_name(), e);
-                    None
-                }
-            })
+        let model = if ext == "yml" || ext == "yaml" {
+            serde_yaml::from_str::<SyncModel>(&content_str)?
         } else if ext == "json" {
-            Ok(match serde_json::from_str::<SyncModel>(&content_str) {
-                Ok(m) => Some((m, checksum)),
-                Err(e) => {
-                    warn!("Error parsing {:?} {:?}", file_path.file_name(), e);
-                    None
-                }
-            })
+            serde_json::from_str::<SyncModel>(&content_str)?
         } else {
-            Ok(None)
-        }
+            return Ok(None);
+        };
+
+        Ok(Some((model, checksum)))
     }
 
     pub fn from_file(file_path: &Path) -> Result<Option<(SyncModel, String)>> {
-        let content = match fs::read(file_path) {
-            Ok(c) => c,
-            Err(_) => return Ok(None),
-        };
-
-        Self::from_bytes(content, file_path)
+        Self::from_bytes(fs::read(file_path)?, file_path)
     }
 
     pub fn to_file_contents(&self, rel_path: &Path) -> Result<(Vec<u8>, String)> {

crates/yaak-sync/src/sync.rs

@@ -1,4 +1,4 @@
-use crate::error::Result;
+use crate::error::{Error, Result};
 use crate::models::SyncModel;
 use chrono::Utc;
 use log::{info, warn};
@@ -7,8 +7,9 @@ use std::collections::HashMap;
 use std::fmt::{Display, Formatter};
 use std::fs;
 use std::fs::File;
-use std::io::Write;
+use std::io::{self, Write};
 use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicUsize, Ordering};
 use ts_rs::TS;
 use yaak_models::blob_manager::BlobManager;
 use yaak_models::client_db::{ClientDb, WriteDb};
@@ -178,33 +179,77 @@ pub fn get_db_candidates(
     Ok(candidates)
 }
 
-pub fn get_fs_candidates(dir: &Path) -> Result<Vec<FsCandidate>> {
+/// Read sync files, validating paths tracked by existing database models.
+///
+/// Workspace sync callers must pass all candidates from [`get_db_candidates`] for
+/// the same workspace and directory. An empty slice is for import discovery
+/// without database sync state; it does not validate tracked model identities.
+pub fn get_fs_candidates(dir: &Path, db_candidates: &[DbCandidate]) -> Result<Vec<FsCandidate>> {
     // Ensure the root directory exists
     fs::create_dir_all(dir)?;
 
+    let tracked_paths: HashMap<PathBuf, (&SyncModel, &SyncState)> = db_candidates
+        .iter()
+        .filter_map(|candidate| match candidate {
+            DbCandidate::Added(_) | DbCandidate::Deleted(_) => None,
+            DbCandidate::Modified(model, state) | DbCandidate::Unmodified(model, state) => {
+                Some((PathBuf::from(&state.rel_path), (model, state)))
+            }
+        })
+        .collect();
+
     let mut candidates = Vec::new();
     let entries = fs::read_dir(dir)?;
     for dir_entry in entries {
-        let dir_entry = match dir_entry {
-            Ok(v) => v,
-            Err(_) => continue,
-        };
+        let dir_entry = dir_entry?;
+        let path = dir_entry.path();
+        let rel_path = PathBuf::from(dir_entry.file_name());
+        let tracked = tracked_paths.get(&rel_path);
+        let invalid =
+            |reason: String| Err(Error::InvalidSyncFile(format!("{}: {reason}", path.display())));
 
         if !dir_entry.file_type()?.is_file() {
+            if tracked.is_some() {
+                return invalid("expected a regular sync file".into());
+            }
             continue;
         };
 
-        let path = dir_entry.path();
-        let (model, checksum) = match SyncModel::from_file(&path) {
-            Ok(Some(m)) => m,
-            Ok(None) => continue,
-            Err(e) => {
-                warn!("Failed to parse sync file {e}");
-                return Err(e);
+        // A present tracked file must not disappear from the candidates: that
+        // would turn a read/parse failure into a database deletion.
+        let (model, checksum) = match (SyncModel::from_file(&path), tracked) {
+            (Ok(Some(m)), _) => m,
+            (Ok(None), None) => continue,
+            (Err(e), None) => {
+                warn!("Skipping invalid sync file {}: {e}", path.display());
+                continue;
             }
+            (Ok(None), Some(_)) => return invalid("file does not contain a Yaak model".into()),
+            (Err(e), Some(_)) => return invalid(e.to_string()),
         };
 
-        let rel_path = Path::new(&dir_entry.file_name()).to_path_buf();
+        if let Some((previous, state)) = tracked {
+            if model.id() != state.model_id {
+                return invalid(format!(
+                    "expected model ID {}, found {}",
+                    state.model_id,
+                    model.id()
+                ));
+            }
+            // A wrong/defaulted workspace ID would be filtered out by the caller,
+            // making this tracked model look deleted even if its ID is intact.
+            if model.workspace_id() != state.workspace_id {
+                return invalid(format!(
+                    "expected workspace ID {}, found {}",
+                    state.workspace_id,
+                    model.workspace_id()
+                ));
+            }
+            if std::mem::discriminant(&model) != std::mem::discriminant(*previous) {
+                return invalid("model type does not match the tracked model".into());
+            }
+        }
+
         candidates.push(FsCandidate { rel_path, model, checksum })
     }
 
@@ -391,8 +436,7 @@ pub fn apply_fs_sync_ops(
                 let rel_path = derive_model_filename(&model);
                 let abs_path = sync_dir.join(rel_path.clone());
                 let (content, checksum) = model.to_file_contents(&rel_path)?;
-                let mut f = File::create(&abs_path)?;
-                f.write_all(&content)?;
+                write_sync_file(&abs_path, |file| file.write_all(&content))?;
                 SyncStateOp::Create { model_id: model.id(), checksum, rel_path }
             }
             SyncOp::FsUpdate { model, state } => {
@@ -400,8 +444,7 @@ pub fn apply_fs_sync_ops(
                 let rel_path = Path::new(&state.rel_path);
                 let abs_path = Path::new(&state.sync_dir).join(&rel_path);
                 let (content, checksum) = model.to_file_contents(&rel_path)?;
-                let mut f = File::create(&abs_path)?;
-                f.write_all(&content)?;
+                write_sync_file(&abs_path, |file| file.write_all(&content))?;
                 SyncStateOp::Update {
                     state: state.to_owned(),
                     checksum,
@@ -447,6 +490,39 @@ pub fn apply_fs_sync_ops(
     Ok(pending)
 }
 
+/// Publish a complete file without exposing partial writes to sync readers.
+///
+/// The temporary file's extension keeps it out of sync candidates. A crash may
+/// leave it behind, but it is never read as a model.
+fn write_sync_file(path: &Path, write: impl FnOnce(&mut File) -> io::Result<()>) -> Result<()> {
+    static COUNTER: AtomicUsize = AtomicUsize::new(0);
+
+    // Opening the existing file for writing keeps read-only files from being replaced
+    let permissions = match fs::OpenOptions::new().write(true).open(path) {
+        Ok(file) => Some(file.metadata()?.permissions()),
+        Err(error) if error.kind() == io::ErrorKind::NotFound => None,
+        Err(error) => return Err(error.into()),
+    };
+    let tmp_path = path.with_file_name(format!(
+        ".{}.{}-{}.tmp",
+        path.file_name().unwrap_or_default().to_string_lossy(),
+        std::process::id(),
+        COUNTER.fetch_add(1, Ordering::Relaxed),
+    ));
+    let result = File::create_new(&tmp_path).and_then(|mut file| {
+        if let Some(permissions) = permissions {
+            file.set_permissions(permissions)?;
+        }
+        write(&mut file)?;
+        drop(file);
+        fs::rename(&tmp_path, path)
+    });
+    if result.is_err() {
+        let _ = fs::remove_file(&tmp_path);
+    }
+    Ok(result?)
+}
+
 impl PendingDbSyncOps {
     /// Upserts are collected per model type and written in one batch so
     /// foreign keys are satisfied.
@@ -603,3 +679,90 @@ fn delete_model(db: &WriteDb, blobs: &BlobManager, model: &SyncModel) -> Result<
     };
     Ok(())
 }
+
+#[cfg(test)]
+mod write_tests {
+    use super::*;
+
+    #[test]
+    fn failed_partial_write_keeps_previous_file() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("request.yaml");
+        fs::write(&path, b"complete previous contents").unwrap();
+        let result = write_sync_file(&path, |file| {
+            file.write_all(b"partial new contents")?;
+            Err(io::Error::new(io::ErrorKind::WriteZero, "simulated write failure"))
+        });
+        assert!(
+            matches!(result, Err(Error::IoError(error)) if error.kind() == io::ErrorKind::WriteZero)
+        );
+        assert_eq!(fs::read(&path).unwrap(), b"complete previous contents");
+        assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 1);
+    }
+
+    #[test]
+    fn readers_ignore_the_in_progress_file() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("request.yaml");
+        let request = HttpRequest {
+            id: "rq_write_test".into(),
+            workspace_id: "wk_write_test".into(),
+            name: "Previous request".into(),
+            ..Default::default()
+        };
+        let model = SyncModel::HttpRequest(request.clone());
+        let (old_contents, checksum) = model.to_file_contents(&path).unwrap();
+        fs::write(&path, &old_contents).unwrap();
+        let db_candidates = vec![DbCandidate::Unmodified(
+            model,
+            SyncState {
+                model_id: request.id.clone(),
+                workspace_id: request.workspace_id.clone(),
+                rel_path: "request.yaml".into(),
+                checksum,
+                ..Default::default()
+            },
+        )];
+        let model = SyncModel::HttpRequest(HttpRequest { name: "New request".into(), ..request });
+        let (new_contents, _) = model.to_file_contents(&path).unwrap();
+        let split = new_contents.len() / 2;
+        write_sync_file(&path, |file| {
+            file.write_all(&new_contents[..split])?;
+            assert_eq!(fs::read(&path).unwrap(), old_contents);
+            let fs_candidates = get_fs_candidates(dir.path(), &db_candidates).unwrap();
+            assert_eq!(fs_candidates.len(), 1);
+            assert!(compute_sync_ops(db_candidates.clone(), fs_candidates).is_empty());
+            file.write_all(&new_contents[split..])
+        })
+        .unwrap();
+        assert_eq!(fs::read(&path).unwrap(), new_contents);
+        assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 1);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn writes_preserve_existing_permissions_and_new_file_defaults() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("request.yaml");
+        fs::write(&path, b"old contents").unwrap();
+        fs::set_permissions(&path, fs::Permissions::from_mode(0o640)).unwrap();
+        write_sync_file(&path, |file| {
+            // Keep unpublished data at least as private as the original file.
+            assert_eq!(file.metadata()?.permissions().mode() & 0o777 & !0o640, 0);
+            file.write_all(b"new contents")
+        })
+        .unwrap();
+        assert_eq!(fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o640);
+
+        let reference = dir.path().join("reference.yaml");
+        File::create(&reference).unwrap();
+        let new_path = dir.path().join("new.yaml");
+        write_sync_file(&new_path, |file| file.write_all(b"new contents")).unwrap();
+        assert_eq!(
+            fs::metadata(&new_path).unwrap().permissions().mode() & 0o777,
+            fs::metadata(&reference).unwrap().permissions().mode() & 0o777
+        );
+    }
+}

crates/yaak-sync/tests/sync_files.rs

@@ -0,0 +1,217 @@
+use std::fs;
+use std::path::PathBuf;
+use tempfile::TempDir;
+use yaak_models::blob_manager::BlobManager;
+use yaak_models::models::{HttpRequest, SyncState, Workspace};
+use yaak_models::query_manager::QueryManager;
+use yaak_sync::error::{Error, Result};
+use yaak_sync::models::SyncModel;
+use yaak_sync::sync::{
+    SyncOp, apply_db_sync_ops, apply_fs_sync_ops, apply_sync_state_ops, compute_sync_ops,
+    get_db_candidates, get_fs_candidates,
+};
+
+struct Fixture {
+    db: QueryManager,
+    blobs: BlobManager,
+    dir: TempDir,
+    workspace_id: String,
+    request: HttpRequest,
+    request_path: PathBuf,
+}
+
+impl Fixture {
+    fn new(request_filename: &str) -> Self {
+        let (db, blobs, _events) = yaak_models::init_in_memory().unwrap();
+        let dir = tempfile::tempdir().unwrap();
+        let workspace = Workspace { id: "wk_sync_test".into(), ..Default::default() };
+        let request = HttpRequest {
+            id: "rq_sync_test".into(),
+            workspace_id: workspace.id.clone(),
+            name: "Keep this request".into(),
+            ..Default::default()
+        };
+        let request_path = dir.path().join(request_filename);
+        let fixture = Self {
+            db,
+            blobs,
+            dir,
+            workspace_id: workspace.id.clone(),
+            request: request.clone(),
+            request_path,
+        };
+        for (model, path) in [
+            (SyncModel::Workspace(workspace), fixture.dir.path().join("workspace.yaml")),
+            (SyncModel::HttpRequest(request), fixture.request_path.clone()),
+        ] {
+            let (contents, _) = model.to_file_contents(&path).unwrap();
+            fs::write(path, contents).unwrap();
+        }
+        let fs_candidates = get_fs_candidates(fixture.dir.path(), &[]).unwrap();
+        fixture.apply(compute_sync_ops(Vec::new(), fs_candidates));
+        assert!(fixture.calculate().unwrap().is_empty());
+        fixture
+    }
+
+    fn calculate(&self) -> Result<Vec<SyncOp>> {
+        let db_candidates =
+            get_db_candidates(&self.db.connect(), "test", &self.workspace_id, self.dir.path())?;
+        let fs_candidates = get_fs_candidates(self.dir.path(), &db_candidates)?
+            .into_iter()
+            .filter(|candidate| candidate.model.workspace_id() == self.workspace_id)
+            .collect();
+        Ok(compute_sync_ops(db_candidates, fs_candidates))
+    }
+
+    fn apply(&self, ops: Vec<SyncOp>) {
+        let pending = apply_fs_sync_ops(&self.workspace_id, self.dir.path(), ops).unwrap();
+        let workspace = &self.workspace_id;
+        let dir = self.dir.path();
+        self.db
+            .with_tx::<_, Error>(|tx| {
+                let states = apply_db_sync_ops(tx, &self.blobs, workspace, dir, pending)?;
+                apply_sync_state_ops(tx, workspace, dir, states)
+            })
+            .unwrap();
+    }
+
+    fn states(&self) -> Vec<SyncState> {
+        let mut states = self
+            .db
+            .connect()
+            .list_sync_states_for_workspace(&self.workspace_id, self.dir.path())
+            .unwrap();
+        states.sort_by(|a, b| a.id.cmp(&b.id));
+        states
+    }
+
+    fn db_request(&self) -> HttpRequest {
+        self.db.connect().get_http_request(&self.request.id).unwrap()
+    }
+
+    fn edit(&self, edit: impl FnOnce(&mut serde_json::Value)) {
+        let mut value = serde_json::to_value(SyncModel::HttpRequest(self.request.clone())).unwrap();
+        edit(&mut value);
+        let content = if self.request_path.extension().unwrap() == "json" {
+            serde_json::to_string(&value).unwrap()
+        } else {
+            serde_yaml::to_string(&value).unwrap()
+        };
+        fs::write(&self.request_path, content).unwrap();
+    }
+
+    fn assert_invalid_file(&self) -> String {
+        let before_request = self.db_request();
+        let before_states = serde_json::to_value(self.states()).unwrap();
+        let error = self.calculate().unwrap_err();
+        assert!(matches!(&error, Error::InvalidSyncFile(_)), "{error:?}");
+        assert!(error.to_string().contains(self.request_path.to_str().unwrap()));
+        assert_eq!(self.db_request(), before_request);
+        assert_eq!(serde_json::to_value(self.states()).unwrap(), before_states);
+        error.to_string()
+    }
+}
+
+#[test]
+fn tracked_model_type_changes_abort_sync() {
+    let fixture = Fixture::new("request.yaml");
+    fixture.edit(|value| {
+        value["model"] = "websocket_request".into();
+    });
+    fixture.assert_invalid_file();
+}
+
+#[test]
+fn invalid_tracked_files_abort_without_changing_data() {
+    for (filename, contents) in [
+        ("request.yaml", b"model: http_request\nid: [broken\n".as_slice()),
+        ("request.json", b"{\"model\":\"http_request\",\"id\":".as_slice()),
+        ("custom-request.yml", b"name: half-written\n".as_slice()),
+        ("custom-request.yml", b"model: http_request\n".as_slice()),
+        ("custom-request.yml", [0xff, 0xfe].as_slice()),
+    ] {
+        let fixture = Fixture::new(filename);
+        fs::write(&fixture.request_path, contents).unwrap();
+        fixture.assert_invalid_file();
+        assert_eq!(fs::read(&fixture.request_path).unwrap(), contents);
+    }
+}
+
+#[test]
+fn tracked_file_with_git_conflict_markers_aborts_sync() {
+    let fixture = Fixture::new("request.yaml");
+    let contents = fs::read_to_string(&fixture.request_path).unwrap().replace(
+        "name: Keep this request\n",
+        "<<<<<<< HEAD\nname: Mine\n=======\nname: Theirs\n>>>>>>> origin/main\n",
+    );
+    fs::write(&fixture.request_path, contents).unwrap();
+    let error = fixture.assert_invalid_file();
+    assert!(error.contains("line "), "{error}");
+}
+
+#[test]
+fn empty_tracked_file_aborts_sync() {
+    let fixture = Fixture::new("request.yaml");
+    fs::write(&fixture.request_path, "").unwrap();
+    fixture.assert_invalid_file();
+}
+
+#[cfg(unix)]
+#[test]
+fn unreadable_tracked_file_aborts_sync() {
+    use std::os::unix::fs::PermissionsExt;
+    let fixture = Fixture::new("request.yaml");
+    fs::set_permissions(&fixture.request_path, fs::Permissions::from_mode(0o000)).unwrap();
+    // Root bypasses ordinary POSIX permission checks.
+    if fs::read(&fixture.request_path).is_ok() {
+        return;
+    }
+    let error = fixture.assert_invalid_file();
+    assert!(error.contains("Permission denied"), "{error}");
+}
+
+#[test]
+fn mismatched_identities_abort_sync_for_yaml_and_json() {
+    for filename in ["request.yaml", "request.json"] {
+        for field in ["id", "workspaceId"] {
+            let fixture = Fixture::new(filename);
+            fixture.edit(|value| value[field] = "different".into());
+            fixture.assert_invalid_file();
+        }
+    }
+}
+
+#[test]
+fn tracked_path_replaced_by_a_directory_aborts_sync() {
+    let fixture = Fixture::new("request.yaml");
+    fs::remove_file(&fixture.request_path).unwrap();
+    fs::create_dir(&fixture.request_path).unwrap();
+    fixture.assert_invalid_file();
+}
+
+#[test]
+fn deleting_a_tracked_file_still_deletes_the_request() {
+    let fixture = Fixture::new("request.yaml");
+    fs::remove_file(&fixture.request_path).unwrap();
+    let ops = fixture.calculate().unwrap();
+    assert!(
+        matches!(ops.as_slice(), [SyncOp::DbDelete { model, .. }] if model.id() == fixture.request.id)
+    );
+    fixture.apply(ops);
+    assert!(fixture.db.connect().get_http_request(&fixture.request.id).is_err());
+}
+
+#[test]
+fn unrelated_files_are_still_ignored() {
+    let fixture = Fixture::new("request.yaml");
+    for (name, contents) in [
+        ("README.md", "model: http_request\nid: [broken\n"),
+        ("config.yaml", "model: unrelated\nid: example\n"),
+        ("unrelated.json", "{\"model\":\"config\",\"id\":"),
+        ("empty.yaml", ""),
+    ] {
+        fs::write(fixture.dir.path().join(name), contents).unwrap();
+    }
+    fs::create_dir(fixture.dir.path().join("other-files")).unwrap();
+    assert!(fixture.calculate().unwrap().is_empty());
+}

plugins/template-function-xml/package.json

@@ -8,7 +8,8 @@
   "types": "src/index.ts",
   "scripts": {
     "build": "yaakcli build",
-    "dev": "yaakcli dev"
+    "dev": "yaakcli dev",
+    "test": "vp test --run tests"
   },
   "dependencies": {
     "@xmldom/xmldom": "^0.9.12",

plugins/template-function-xml/src/index.ts

@@ -77,11 +77,19 @@ export function filterXPath(
     return String(items);
   }
   if (!Array.isArray(items) || result === "first") {
-    return items[0] != null ? String(items[0].firstChild ?? "") : "";
+    return items[0] != null ? nodeText(items[0]) : "";
   }
   if (result === "join") {
-    return items.map((item) => String(item.firstChild ?? "")).join(join ?? "");
+    return items.map(nodeText).join(join ?? "");
   }
   // Not sure what cases this happens in (?)
   return String(items);
 }
+
+function nodeText(node: Node): string {
+  // Attribute, text and CDATA nodes have no children, so their value is on the node itself
+  if (xpath.isAttribute(node) || xpath.isTextNode(node) || xpath.isCDATASection(node)) {
+    return node.nodeValue ?? "";
+  }
+  return String(node.firstChild ?? "");
+}

plugins/template-function-xml/tests/xpath.test.ts

@@ -0,0 +1,36 @@
+import type { Context } from "@yaakapp/api";
+import { describe, expect, it } from "vite-plus/test";
+import { filterXPath, plugin } from "../src";
+
+const USERS =
+  '<users><user id="42"><name>Alice</name></user><user id="7"><name>Bob</name></user></users>';
+
+describe("xml.xpath", () => {
+  const xpathFunction = plugin.templateFunctions?.find((f) => f.name === "xml.xpath");
+
+  const select = async (query: string, result = "first", join = ", ") =>
+    await xpathFunction?.onRender({} as Context, {
+      values: { input: USERS, query, result, join },
+      purpose: "send",
+    });
+
+  it("returns the text of a selected element", async () => {
+    expect(await select("//user/name")).toBe("Alice");
+  });
+
+  it("returns the value of a selected attribute", async () => {
+    expect(await select("//user/@id")).toBe("42");
+  });
+
+  it("joins the values of selected attributes", async () => {
+    expect(await select("//user/@id", "join")).toBe("42, 7");
+  });
+
+  it("returns the value of a selected text node", async () => {
+    expect(await select("//user/name/text()")).toBe("Alice");
+  });
+
+  it("returns the value of a selected CDATA section", () => {
+    expect(filterXPath("<a><![CDATA[x<y]]></a>", "/a/text()", "first", null)).toBe("x<y");
+  });
+});

tests/pdfViewer.test.ts

@@ -93,9 +93,13 @@ test.each(["http://tauri.localhost", "tauri://localhost"])(
     const pdfjsDir = path.dirname(reactPdfRequire.resolve("pdfjs-dist/package.json"));
     const installed = JSON.parse(await readFile(path.join(pdfjsDir, "package.json"), "utf8"));
     expect(version).toBe(installed.version);
-    expect(Buffer.from(worker.source)).toEqual(
-      await readFile(path.join(pdfjsDir, "build/pdf.worker.min.mjs")),
-    );
+    // The worker is bundled with the polyfills older WebKit needs, ahead of PDF.js itself.
+    const source = Buffer.from(worker.source).toString("utf8");
+    const escapedVersion = installed.version.replaceAll(".", "\\.");
+    expect(source).toMatch(new RegExp(`["'\`]${escapedVersion}["'\`]`));
+    const polyfillAt = source.indexOf("getOrInsertComputed callback is not a function");
+    expect(polyfillAt).toBeGreaterThanOrEqual(0);
+    expect(polyfillAt).toBeLessThan(source.indexOf("pdfjs_internal_editor_"));
   },
   30_000,
 );