answer missing reads from the cache and resolve mkdir/rmdir paths

2026-06-17 04:35:53 +00:00 · 2026-06-15 15:55:50 +02:00
parent c22ecb5fef
commit 448c6eea2c
5 changed files with 328 additions and 37 deletions
--- a/packages/shim/src/fs/promises-mutations.test.js
+++ b/packages/shim/src/fs/promises-mutations.test.js
@@ -0,0 +1,140 @@
+import { describe, it, expect, vi, afterEach } from "vitest";
+import { createFsPromises } from "./promises.js";
+import { registerPathResolver, _reset } from "./transforms.js";
+import { isRecentLocalOp } from "./echo-guard.js";
+
+function makeDeps() {
+  const store = new Map();
+
+  const metadataCache = {
+    has: (p) => store.has(p),
+    get: (p) => (store.has(p) ? store.get(p) : null),
+    set: (p, m) => store.set(p, m),
+    delete: (p) => store.delete(p),
+    toStat: (p) =>
+      store.has(p)
+        ? {
+            type: store.get(p).type,
+            isDirectory: () => store.get(p).type === "directory",
+            isFile: () => store.get(p).type === "file",
+          }
+        : null,
+    readdir: () => [],
+  };
+
+  const contentCache = {
+    get: () => null,
+    set: vi.fn(),
+    delete: vi.fn(),
+    invalidate: vi.fn(),
+  };
+
+  const transport = {
+    mkdir: vi.fn(async () => {}),
+    rmdir: vi.fn(async () => {}),
+    stat: vi.fn(async () => ({ type: "file", size: 1 })),
+    readFile: vi.fn(async () => {
+      throw new Error("transport.readFile should not be called");
+    }),
+  };
+
+  return { metadataCache, contentCache, transport, store };
+}
+
+describe("promises directory mutations honor path resolvers", () => {
+  afterEach(() => _reset());
+
+  it("mkdir uses the resolved path for cache, echo-guard, and transport", async () => {
+    registerPathResolver(
+      (p) => p === "logical/dir",
+      () => "physical/dir",
+    );
+
+    const deps = makeDeps();
+    const fs = createFsPromises(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+
+    await fs.mkdir("logical/dir", { recursive: true });
+
+    expect(deps.store.get("physical/dir")).toEqual({ type: "directory" });
+    expect(deps.store.has("logical/dir")).toBe(false);
+    expect(deps.transport.mkdir).toHaveBeenCalledWith("physical/dir", true);
+    expect(isRecentLocalOp("physical/dir")).toBe(true);
+    expect(isRecentLocalOp("logical/dir")).toBe(false);
+  });
+
+  it("rmdir uses the resolved path for cache, echo-guard, and transport", async () => {
+    registerPathResolver(
+      (p) => p === "logical/dir",
+      () => "physical/dir",
+    );
+
+    const deps = makeDeps();
+    const fs = createFsPromises(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+    deps.store.set("physical/dir", { type: "directory" });
+
+    await fs.rmdir("logical/dir");
+
+    expect(deps.store.has("physical/dir")).toBe(false);
+    expect(deps.transport.rmdir).toHaveBeenCalledWith("physical/dir");
+    expect(isRecentLocalOp("physical/dir")).toBe(true);
+  });
+});
+
+describe("promises readFile existence", () => {
+  afterEach(() => _reset());
+
+  it("answers ENOENT from the cache for a missing non-redirected path, no transport", async () => {
+    const deps = makeDeps();
+    const fs = createFsPromises(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+
+    await expect(
+      fs.readFile("/.obsidian/backlink.json", "utf8"),
+    ).rejects.toThrow(/ENOENT/);
+    expect(deps.transport.readFile).not.toHaveBeenCalled();
+  });
+
+  it("falls back to the original path for a redirected miss", async () => {
+    registerPathResolver(
+      (p) => p === ".obsidian/workspace.json",
+      () => ".obsidian/workspace.Work.json",
+    );
+
+    const deps = makeDeps();
+    deps.transport.readFile = vi.fn(async (p) => {
+      if (p === ".obsidian/workspace.Work.json") {
+        const e = new Error("ENOENT");
+        e.code = "ENOENT";
+        throw e;
+      }
+
+      return "BASE";
+    });
+
+    const fs = createFsPromises(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+
+    // Returns the base content after the redirect target 404s: the fallback fired.
+    await expect(fs.readFile("/.obsidian/workspace.json", "utf8")).resolves.toBe(
+      "BASE",
+    );
+    expect(deps.transport.readFile).toHaveBeenCalledWith(
+      ".obsidian/workspace.Work.json",
+      "utf8",
+    );
+  });
+});
--- a/packages/shim/src/fs/promises.js
+++ b/packages/shim/src/fs/promises.js
@@ -4,6 +4,7 @@ import {
  applyReadTransform,
  applyWriteTransform,
  resolvePath,
+  resolvePathInfo,
 } from "./transforms.js";
 import { hasVirtualFile, getVirtualFile } from "./virtual-files.js";
 import { realpathSync } from "./realpath.js";
@@ -53,7 +54,7 @@ export function createFsPromises(metadataCache, contentCache, transport) {
      }

      const wantText = encoding === "utf8" || encoding === "utf-8";
-      const resolved = resolvePath(path);
+      const { resolved, redirected } = resolvePathInfo(path);

      // Virtual plugin source overrides any cache/transport version.
      if (hasVirtualFile(resolved)) {
@@ -86,8 +87,9 @@ export function createFsPromises(metadataCache, contentCache, transport) {
          throw e;
        }

-        if (!meta && resolved && resolved === path) {
-          // Throw ENOENT only when not redirected; redirected paths fall through to the transport's fallback.
+        if (!meta && !redirected) {
+          // The metadata cache holds every existing path (populated at bootstrap, kept current by the watcher).
+          // A cache miss on a non-redirected path is genuinely absent. Redirected paths fall through to the transport.
          const e = new Error(
            `ENOENT: no such file or directory, open '${path}'`,
          );
@@ -102,7 +104,7 @@ export function createFsPromises(metadataCache, contentCache, transport) {
        try {
          result = await transport.readFile(resolved, encoding);
        } catch (e) {
-          if (resolved !== path && e.code === "ENOENT") {
+          if (redirected && e.code === "ENOENT") {
            result = await transport.readFile(path, encoding);
          } else {
            throw e;
@@ -207,16 +209,20 @@ export function createFsPromises(metadataCache, contentCache, transport) {
      const recursive =
        typeof options === "object" ? !!options.recursive : !!options;

-      markLocalOp(path);
-      metadataCache.set(path, { type: "directory" });
+      const resolved = resolvePath(path);

-      await transport.mkdir(path, recursive);
+      markLocalOp(resolved);
+      metadataCache.set(resolved, { type: "directory" });
+
+      await transport.mkdir(resolved, recursive);
    },

    async rmdir(path) {
-      markLocalOp(path);
-      metadataCache.delete(path);
-      await transport.rmdir(path);
+      const resolved = resolvePath(path);
+
+      markLocalOp(resolved);
+      metadataCache.delete(resolved);
+      await transport.rmdir(resolved);
    },

    async rm(path, options) {
--- a/packages/shim/src/fs/sync-mutations.test.js
+++ b/packages/shim/src/fs/sync-mutations.test.js
@@ -1,6 +1,7 @@
-import { describe, it, expect, vi } from "vitest";
+import { describe, it, expect, vi, afterEach } from "vitest";
 import { createFsSync } from "./sync.js";
-import { resolvePath } from "./transforms.js";
+import { resolvePath, registerPathResolver, _reset } from "./transforms.js";
+import { isRecentLocalOp } from "./echo-guard.js";

 function makeDeps() {
  const store = new Map();
@@ -43,6 +44,9 @@ function makeDeps() {
    appendFile: vi.fn(async () => {}),
    utimes: vi.fn(async () => {}),
    stat: vi.fn(async () => ({ type: "file", size: 1 })),
+    readFileSync: vi.fn(() => {
+      throw new Error("transport.readFileSync should not be called");
+    }),
  };

  return { metadataCache, contentCache, transport, store };
@@ -51,7 +55,11 @@ function makeDeps() {
 describe("sync fs mutations", () => {
  it("lstatSync mirrors statSync", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
    deps.store.set(resolvePath("dir"), { type: "directory" });

    expect(fs.lstatSync("dir").isDirectory()).toBe(true);
@@ -59,7 +67,11 @@ describe("sync fs mutations", () => {

  it("mkdirSync updates the cache and fires the transport", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );

    fs.mkdirSync("newdir", { recursive: true });

@@ -69,7 +81,11 @@ describe("sync fs mutations", () => {

  it("rmSync deletes from the cache and fires the transport", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
    const key = resolvePath("gone.md");
    deps.store.set(key, { type: "file" });

@@ -81,7 +97,11 @@ describe("sync fs mutations", () => {

  it("renameSync moves cache metadata and fires the transport", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
    const from = resolvePath("a.md");
    const to = resolvePath("b.md");
    deps.store.set(from, { type: "file", size: 2 });
@@ -95,7 +115,11 @@ describe("sync fs mutations", () => {

  it("copyFileSync optimistically mirrors source metadata and fires the transport", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
    const srcKey = resolvePath("src.md");
    const destKey = resolvePath("dest.md");
    deps.store.set(srcKey, { type: "file", size: 9 });
@@ -108,7 +132,11 @@ describe("sync fs mutations", () => {

  it("utimesSync sets mtime and fires the transport", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
    const key = resolvePath("note.md");
    deps.store.set(key, { type: "file", mtime: 0 });

@@ -117,12 +145,101 @@ describe("sync fs mutations", () => {
    expect(deps.store.get(key).mtime).toBe(222);
    expect(deps.transport.utimes).toHaveBeenCalled();
  });
+});
+
+describe("directory mutations honor path resolvers", () => {
+  afterEach(() => _reset());
+
+  it("mkdirSync uses the resolved path for cache, echo-guard, and transport", () => {
+    registerPathResolver(
+      (p) => p === "logical/dir",
+      () => "physical/dir",
+    );

-  it("chmodSync is a no-op that does not throw", () => {
    const deps = makeDeps();
-    const fs = createFsSync(deps.metadataCache, deps.contentCache, deps.transport);
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );

-    expect(() => fs.chmodSync("note.md", 0o644)).not.toThrow();
-    expect(fs.chmodSync("note.md", 0o644)).toBeUndefined();
+    fs.mkdirSync("logical/dir", { recursive: true });
+
+    expect(deps.store.get("physical/dir")).toEqual({ type: "directory" });
+    expect(deps.store.has("logical/dir")).toBe(false);
+    expect(deps.transport.mkdir).toHaveBeenCalledWith("physical/dir", true);
+    expect(isRecentLocalOp("physical/dir")).toBe(true);
+    expect(isRecentLocalOp("logical/dir")).toBe(false);
+  });
+
+  it("rmdirSync uses the resolved path for cache, echo-guard, and transport", () => {
+    registerPathResolver(
+      (p) => p === "logical/dir",
+      () => "physical/dir",
+    );
+
+    const deps = makeDeps();
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+    deps.store.set("physical/dir", { type: "directory" });
+
+    fs.rmdirSync("logical/dir");
+
+    expect(deps.store.has("physical/dir")).toBe(false);
+    expect(deps.transport.rmdir).toHaveBeenCalledWith("physical/dir");
+    expect(isRecentLocalOp("physical/dir")).toBe(true);
+  });
+});
+
+describe("readFileSync existence", () => {
+  afterEach(() => _reset());
+
+  it("answers ENOENT from the cache for a missing non-redirected path, no transport", () => {
+    const deps = makeDeps();
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+
+    // Leading slash: normalize strips it, so resolved !== the raw argument.
+    expect(() => fs.readFileSync("/.obsidian/backlink.json", "utf8")).toThrow(
+      /ENOENT/,
+    );
+    expect(deps.transport.readFileSync).not.toHaveBeenCalled();
+  });
+
+  it("falls back to the original path for a redirected miss", () => {
+    registerPathResolver(
+      (p) => p === ".obsidian/workspace.json",
+      () => ".obsidian/workspace.Work.json",
+    );
+
+    const deps = makeDeps();
+    deps.transport.readFileSync = vi.fn((p) => {
+      if (p === ".obsidian/workspace.Work.json") {
+        const e = new Error("ENOENT");
+        e.code = "ENOENT";
+        throw e;
+      }
+
+      return "BASE";
+    });
+
+    const fs = createFsSync(
+      deps.metadataCache,
+      deps.contentCache,
+      deps.transport,
+    );
+
+    // Returns the base content after the redirect target 404s: the fallback fired.
+    expect(fs.readFileSync("/.obsidian/workspace.json", "utf8")).toBe("BASE");
+    expect(deps.transport.readFileSync).toHaveBeenCalledWith(
+      ".obsidian/workspace.Work.json",
+      "utf8",
+    );
  });
 });
--- a/packages/shim/src/fs/sync.js
+++ b/packages/shim/src/fs/sync.js
@@ -4,6 +4,7 @@ import {
  applyReadTransform,
  applyWriteTransform,
  resolvePath,
+  resolvePathInfo,
 } from "./transforms.js";
 import { hasVirtualFile, getVirtualFile } from "./virtual-files.js";

@@ -69,7 +70,7 @@ export function createFsSync(metadataCache, contentCache, transport) {
      }

      const wantText = encoding === "utf8" || encoding === "utf-8";
-      const resolved = resolvePath(path);
+      const { resolved, redirected } = resolvePathInfo(path);

      // Virtual plugin source overrides any cache or transport version.
      if (hasVirtualFile(resolved)) {
@@ -108,13 +109,22 @@ export function createFsSync(metadataCache, contentCache, transport) {
        result = contentCache.get(resolved);
      }

+      // The metadata cache is kept fresh by the filewatcher and a miss here genuinely means the file is absent.
+      // Redirected paths fall through to the transport, so we can't trust the cache for them, but non-redirected misses are definitive.
+      if (result === null && !meta && !redirected) {
+        const e = new Error(
+          `ENOENT: no such file or directory, open '${path}'`,
+        );
+        e.code = "ENOENT";
+        throw e;
+      }
+
      if (result === null) {
-        // ENOENT fallback: if the resolved path doesn't exist, try the original.
-        // Covers per-name workspace files that haven't been saved yet.
+        // A resolver can map a path onto a physical target that does not exist yet, so a redirected miss retries the original path before failing.
        try {
          result = transport.readFileSync(resolved, encoding);
        } catch (e) {
-          if (resolved !== path && e.code === "ENOENT") {
+          if (redirected && e.code === "ENOENT") {
            console.warn(
              "[shim:fs] readFileSync cache miss, using sync XHR:",
              path,
@@ -206,20 +216,32 @@ export function createFsSync(metadataCache, contentCache, transport) {
      const recursive =
        typeof options === "object" ? !!options.recursive : !!options;

-      markLocalOp(path);
-      metadataCache.set(path, { type: "directory" });
+      const resolved = resolvePath(path);

-      transport.mkdir(path, recursive).catch((e) => {
-        console.error("[shim:fs] mkdirSync background create failed:", path, e);
+      markLocalOp(resolved);
+      metadataCache.set(resolved, { type: "directory" });
+
+      transport.mkdir(resolved, recursive).catch((e) => {
+        console.error(
+          "[shim:fs] mkdirSync background create failed:",
+          resolved,
+          e,
+        );
      });
    },

    rmdirSync(path) {
-      markLocalOp(path);
-      metadataCache.delete(path);
+      const resolved = resolvePath(path);

-      transport.rmdir(path).catch((e) => {
-        console.error("[shim:fs] rmdirSync background remove failed:", path, e);
+      markLocalOp(resolved);
+      metadataCache.delete(resolved);
+
+      transport.rmdir(resolved).catch((e) => {
+        console.error(
+          "[shim:fs] rmdirSync background remove failed:",
+          resolved,
+          e,
+        );
      });
    },

--- a/packages/shim/src/fs/transforms.js
+++ b/packages/shim/src/fs/transforms.js
@@ -12,7 +12,9 @@ export function registerPathResolver(matcher, resolver) {
  pathResolvers.push({ matcher, resolver });
 }

-export function resolvePath(path) {
+// resolved is the physical path.
+// redirected is true when a path resolver sent the request to a different path.
+export function resolvePathInfo(path) {
  const norm = normalize(path);

  for (const { matcher, resolver } of pathResolvers) {
@@ -21,13 +23,17 @@ export function resolvePath(path) {
        const resolved = resolver(norm);

        if (typeof resolved === "string" && resolved.length > 0) {
-          return resolved;
+          return { resolved, redirected: true };
        }
      }
    } catch {}
  }

-  return norm;
+  return { resolved: norm, redirected: false };
+}
+
+export function resolvePath(path) {
+  return resolvePathInfo(path).resolved;
 }

 // --- Read transforms ---