fix(root): report documented remove() failure codes

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - Suffix Windows reserved basenames with `_` in `sanitizeUntrustedFileName()` while preserving case and extensions on every platform, including dollar names and superscript COM/LPT variants; thanks @SebTardif (#67).
 
+### Security and Correctness
+
+- Report the documented `remove()` failure codes instead of collapsing every failure to `path-alias`. A missing target now throws `not-found`, a non-empty directory throws `not-empty`, and any other filesystem failure throws `not-removable`, while directory identity drift keeps reporting `path-mismatch`. The errno classification covers only the deletion syscalls, so a parent-directory guard failure still fails closed with `path-alias` rather than implying a removal was attempted.
+
 ## 0.5.1 - 2026-08-01
 
 ### Security and Correctness

src/root-errors.ts

@@ -1,5 +1,7 @@
 import { FsSafeError } from "./errors.js";
-import { hasNodeErrorCode } from "./path.js";
+import { hasNodeErrorCode, isNodeError, isNotFoundPathError } from "./path.js";
+
+const REMOVE_NOT_EMPTY_CODES = new Set(["ENOTEMPTY", "EEXIST"]);
 
 export function isAlreadyExistsError(error: unknown): boolean {
   return hasNodeErrorCode(error, "EEXIST") || /File exists|EEXIST/i.test(String(error));
@@ -22,3 +24,32 @@ export function normalizePinnedPathError(error: unknown): Error {
     cause: error instanceof Error ? error : undefined,
   });
 }
+
+export function normalizeRemoveGuardError(error: unknown): Error {
+  if (error instanceof FsSafeError) {
+    return error;
+  }
+  if (isNotFoundPathError(error)) {
+    return new FsSafeError("not-found", "file not found", {
+      cause: error instanceof Error ? error : undefined,
+    });
+  }
+  return normalizePinnedPathError(error);
+}
+
+export function normalizeRemovePathError(error: unknown): Error {
+  if (error instanceof FsSafeError) {
+    return error;
+  }
+  if (!isNodeError(error) || typeof error.code !== "string") {
+    return normalizePinnedPathError(error);
+  }
+  const cause = error instanceof Error ? error : undefined;
+  if (isNotFoundPathError(error)) {
+    return new FsSafeError("not-found", "file not found", { cause });
+  }
+  if (REMOVE_NOT_EMPTY_CODES.has(error.code)) {
+    return new FsSafeError("not-empty", "directory is not empty", { cause });
+  }
+  return new FsSafeError("not-removable", "path could not be removed", { cause });
+}

src/root-impl.ts

@@ -47,6 +47,8 @@ import {
   isAlreadyExistsError,
   normalizePinnedPathError,
   normalizePinnedWriteError,
+  normalizeRemoveGuardError,
+  normalizeRemovePathError,
 } from "./root-errors.js";
 import { getFsSafeTestHooks } from "./test-hooks.js";
 import { stringifyJsonDocument } from "./json-stringify.js";
@@ -1324,11 +1326,24 @@ async function resolvePinnedRootPathInRoot(
   };
 }
 
+async function prepareRemoveGuard(targetPath: string) {
+  try {
+    const guard = await createAsyncDirectoryGuard(path.dirname(targetPath));
+    await getFsSafeTestHooks()?.beforeRootFallbackMutation?.("remove", targetPath);
+    await assertAsyncDirectoryGuard(guard);
+    return guard;
+  } catch (error) {
+    throw normalizeRemoveGuardError(error);
+  }
+}
+
 async function removePathFallback(resolved: { resolved: string }): Promise<void> {
-  const guard = await createAsyncDirectoryGuard(path.dirname(resolved.resolved));
-  await getFsSafeTestHooks()?.beforeRootFallbackMutation?.("remove", resolved.resolved);
-  await assertAsyncDirectoryGuard(guard);
-  await ((await fs.lstat(resolved.resolved)).isDirectory() ? fs.rmdir(resolved.resolved) : fs.rm(resolved.resolved));
+  const guard = await prepareRemoveGuard(resolved.resolved);
+  try {
+    await ((await fs.lstat(resolved.resolved)).isDirectory() ? fs.rmdir(resolved.resolved) : fs.rm(resolved.resolved));
+  } catch (error) {
+    throw normalizeRemovePathError(error);
+  }
   await assertAsyncDirectoryGuard(guard).catch(() => undefined);
 }
 

test/edge-coverage.test.ts

@@ -16,11 +16,15 @@ import {
   safePathSegmentHashed,
 } from "../src/install-path.js";
 import { replaceDirectoryAtomic } from "../src/replace-directory.js";
+import { root as openRoot } from "../src/root.js";
 import {
   isAlreadyExistsError,
   normalizePinnedPathError,
   normalizePinnedWriteError,
+  normalizeRemoveGuardError,
+  normalizeRemovePathError,
 } from "../src/root-errors.js";
+import { __setFsSafeTestHooksForTest } from "../src/test-hooks.js";
 import { movePathToTrash } from "../src/trash.js";
 
 const tempDirs = new Set<string>();
@@ -60,6 +64,60 @@ describe("root error helpers", () => {
     });
     expect(normalizePinnedPathError("raw string")).toMatchObject({ code: "path-alias" });
   });
+
+  it("maps remove syscall failures without swallowing boundary errors", () => {
+    const drift = new FsSafeError("path-mismatch", "directory changed during operation");
+    expect(normalizeRemovePathError(drift)).toBe(drift);
+
+    const mappings = [
+      ["ENOENT", "not-found"],
+      ["ENOTDIR", "not-found"],
+      ["ENOTEMPTY", "not-empty"],
+      ["EEXIST", "not-empty"],
+      ["EACCES", "not-removable"],
+      ["EBUSY", "not-removable"],
+    ] as const;
+    for (const [errno, code] of mappings) {
+      const error = Object.assign(new Error(errno), { code: errno });
+      expect(normalizeRemovePathError(error)).toMatchObject({ code });
+    }
+
+    expect(normalizeRemovePathError(new Error("raw"))).toMatchObject({ code: "path-alias" });
+    expect(normalizeRemovePathError("raw string")).toMatchObject({ code: "path-alias" });
+  });
+
+  it("keeps guard-stage failures fail-closed instead of reporting a removal outcome", () => {
+    const drift = new FsSafeError("path-mismatch", "directory changed during operation");
+    expect(normalizeRemoveGuardError(drift)).toBe(drift);
+
+    for (const errno of ["ENOENT", "ENOTDIR"]) {
+      const error = Object.assign(new Error(errno), { code: errno });
+      expect(normalizeRemoveGuardError(error), errno).toMatchObject({ code: "not-found" });
+    }
+    for (const errno of ["ELOOP", "EACCES", "EBUSY", "ENOTEMPTY", "EEXIST"]) {
+      const error = Object.assign(new Error(errno), { code: errno });
+      expect(normalizeRemoveGuardError(error), errno).toMatchObject({ code: "path-alias" });
+    }
+    expect(normalizeRemoveGuardError(new Error("raw"))).toMatchObject({ code: "path-alias" });
+  });
+
+  it("does not map a guard-stage filesystem failure to a removal code", async () => {
+    const rootPath = await tempRoot("fs-safe-remove-guard-");
+    const scoped = await openRoot(rootPath);
+    await scoped.write("target.txt", "keep");
+
+    __setFsSafeTestHooksForTest({
+      beforeRootFallbackMutation() {
+        throw Object.assign(new Error("ELOOP"), { code: "ELOOP" });
+      },
+    });
+    try {
+      await expect(scoped.remove("target.txt")).rejects.toMatchObject({ code: "path-alias" });
+    } finally {
+      __setFsSafeTestHooksForTest(undefined);
+    }
+    await expect(fs.readFile(path.join(rootPath, "target.txt"), "utf8")).resolves.toBe("keep");
+  });
 });
 
 describe("directory replacement and file store boundary helpers", () => {

test/fs-safe.test.ts

@@ -396,6 +396,22 @@ describe("@openclaw/fs-safe", () => {
     });
   });
 
+  it("reports documented failure codes when remove cannot unlink the target", async () => {
+    const rootPath = await tempRoot("fs-safe-remove-codes-");
+    const root = await openRoot(rootPath);
+    await root.mkdir("full/child");
+
+    await expect(root.remove("missing.txt")).rejects.toMatchObject({
+      code: expectedFsSafeCode("not-found"),
+    });
+    await expect(root.remove("missing-dir/missing.txt")).rejects.toMatchObject({
+      code: expectedFsSafeCode("not-found"),
+    });
+    await expect(root.remove("full")).rejects.toMatchObject({
+      code: expectedFsSafeCode("not-empty"),
+    });
+  });
+
   it("opens a file handle for fast reads when kernel fd path validation is available", async () => {
     const root = await openRoot(await tempRoot("fs-safe-open-"));
     await root.write("file.txt", "fast");