fix(root): report operational remove failure codes

CHANGELOG.md

@@ -7,11 +7,13 @@
 - Retry a contended file-lock acquisition when Windows denies access to a lock file whose directory entry is still being torn down, including when the native binding performs the exclusive create. Both the exclusive create and the holder's snapshot read reported that transient `EPERM` as a hard failure, so concurrent `acquireFileLock()` calls failed intermittently on Windows even though the very next attempt would have succeeded. Retries stay bounded, so a genuine permission denial still surfaces as `EPERM` rather than a lock timeout; thanks @Yigtwxx for the fix.
 - Apply `replaceFileAtomic()` and `replaceFileAtomicSync()` modes through pinned temp-file descriptors before rename, and through pinned copy-fallback descriptors, so a post-rename symlink swap cannot redirect `chmod` to an unrelated file while exact modes remain independent of umask; thanks @yetval for reporting this (#86).
 - Bound fallback Windows owner and ACL command execution to 10 seconds per process and report owner-query failures as unverified instead of returning a partial permission classification; thanks @Yigtwxx for the diagnosis.
+- 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`; thanks @Yigtwxx for the fix.
 
 ### Compatibility
 
 - Report access-denied failures from native Windows operations as `EPERM` rather than `EACCES`, matching Node/libuv and the JavaScript fallback. Consumers that matched the previous native-only `EACCES` code should accept `EPERM` as well when supporting older package versions.
 - Add an optional `fchmodSync` operation to the injectable synchronous atomic-replacement filesystem type. Async adapters need no new member because their required `open()` returns a mode-capable `FileHandle`; custom sync adapters that pass `mode` or `preserveExistingMode` now fail before mutation when `fchmodSync` is absent, while adapters that request neither option remain compatible.
+- Classify `not-found`, `not-empty` and `not-removable` as operational rather than policy errors, so `FsSafeError.category` no longer reports routine filesystem outcomes as safety-policy rejections.
 
 ### Features
 

README.md

@@ -506,13 +506,17 @@ Codes are grouped by category:
 ```ts
 if (err instanceof FsSafeError) {
   if (err.category === "policy") {
-    // Unsafe caller input or filesystem state.
+    // Unsafe caller input or filesystem state rejected by a safety policy.
   } else {
-    // Operational problem such as helper startup, timeout, or unverifiable permissions.
+    // Routine filesystem outcome or runtime/environment problem.
   }
 }
 ```
 
+Routine filesystem outcomes such as `not-found`, `not-empty`, and
+`not-removable` are operational; they do not indicate that a filesystem
+boundary policy was violated.
+
 Current `FsSafeErrorCode` values are `already-exists`, `denied-path`, `device-path`, `hardlink`, `helper-failed`, `helper-unavailable`, `invalid-path`, `insecure-permissions`, `not-empty`, `not-file`, `not-found`, `not-owned`, `not-removable`, `outside-workspace`, `path-alias`, `path-mismatch`, `permission-unverified`, `secret-exists`, `symlink`, `timeout`, `too-large`, and `unsupported-platform`.
 
 ## Safety model

docs/errors.md

@@ -50,8 +50,12 @@ destination.
 
 `category` separates caller-policy failures from operational failures:
 
-- `"policy"` — unsafe input or target state, such as `outside-workspace`, `symlink`, `hardlink`, or `too-large`.
-- `"operational"` — environment/runtime failures, such as helper startup, platform support, timeout, or unverifiable permissions.
+- `"policy"` — unsafe input or target state rejected by a safety policy, such as `outside-workspace`, `symlink`, `hardlink`, or `too-large`.
+- `"operational"` — routine filesystem outcomes or environment/runtime failures, such as `not-found`, `not-empty`, `not-removable`, helper startup, platform support, timeout, or unverifiable permissions.
+
+Routine absence or inability to remove a path does not by itself indicate a
+filesystem boundary violation. Branch on the specific code when the distinction
+between those operational outcomes matters.
 
 ## Code union
 

docs/types.md

@@ -163,7 +163,7 @@ type FsSafeErrorCode =
 
 Closed union you switch on. See the [Errors](errors.md) reference for what each one means.
 
-`FsSafeError.category` is `"policy"` for unsafe input/target-state failures and `"operational"` for environment/runtime failures.
+`FsSafeError.category` is `"policy"` for unsafe input or target state rejected by a safety policy and `"operational"` for routine filesystem outcomes or environment/runtime failures. `not-found`, `not-empty`, and `not-removable` are operational.
 
 ## See also
 

src/errors.ts

@@ -29,6 +29,9 @@ export type FsSafeErrorDetails = Readonly<Record<string, unknown>>;
 const OPERATIONAL_CODES: ReadonlySet<FsSafeErrorCode> = new Set([
   "helper-failed",
   "helper-unavailable",
+  "not-empty",
+  "not-found",
+  "not-removable",
   "permission-unverified",
   "timeout",
   "unsupported-platform",

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

@@ -3,7 +3,12 @@ import { chmod, mkdtemp, readdir, readFile, rename, rm, stat, symlink, writeFile
 import os from "node:os";
 import path from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
-import { configureFsSafeNative, FsSafeError, root as openRoot } from "../src/index.js";
+import {
+  categorizeFsSafeError,
+  configureFsSafeNative,
+  FsSafeError,
+  root as openRoot,
+} from "../src/index.js";
 import { openLocalFileSafely, readLocalFileSafely } from "../src/root.js";
 import { __setFsSafeTestHooksForTest } from "../src/test-hooks.js";
 import { expectedFsSafeCode } from "./helpers/security.js";
@@ -26,6 +31,20 @@ afterEach(async () => {
 });
 
 describe("@openclaw/fs-safe", () => {
+  it("classifies routine removal outcomes as operational", () => {
+    for (const code of ["not-found", "not-empty", "not-removable"] as const) {
+      expect(categorizeFsSafeError(code)).toBe("operational");
+      expect(new FsSafeError(code, "remove failed")).toMatchObject({
+        category: "operational",
+        code,
+      });
+    }
+
+    for (const code of ["path-alias", "path-mismatch"] as const) {
+      expect(categorizeFsSafeError(code)).toBe("policy");
+    }
+  });
+
   it.skipIf(skipOnWindows)("reuses a root capability across filesystem operations", async () => {
     const rootPath = await tempRoot("fs-root-object-");
     const root = await openRoot(rootPath);
@@ -396,6 +415,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");