refactor(server): name the fs read route prefix

packages/core/src/filesystem.ts

@@ -23,7 +23,7 @@ export const WriteInput = Schema.Struct({
 export type WriteInput = typeof WriteInput.Type
 
 export class NotFoundError extends Schema.TaggedError<NotFoundError>()("FileSystem.NotFoundError", {
-  path: RelativePath,
+  path: Schema.String,
 }) {}
 
 export class DirectoryNotFoundError extends Schema.TaggedError<DirectoryNotFoundError>()(
@@ -139,71 +139,56 @@ const baseLayer = Layer.effect(
           }),
         )
     const resolve = Effect.fnUntraced(function* (input: RelativePath) {
-      if (input.includes("\u0000")) return yield* Effect.fail(new NotFoundError({ path: input }))
+      const missing = new NotFoundError({ path: input })
+      if (input.includes("\u0000")) return yield* missing
       const absolute = path.resolve(location.directory, input)
-      if (!FSUtil.contains(location.directory, absolute)) return yield* Effect.fail(new NotFoundError({ path: input }))
-      const real = yield* fs.realPath(absolute).pipe(
-        Effect.catchReason(
-          "PlatformError",
-          "NotFound",
-          () => Effect.fail(new NotFoundError({ path: input })),
-          (_, error) => Effect.die(error),
-        ),
-      )
-      if (!FSUtil.contains(root, real)) return yield* Effect.fail(new NotFoundError({ path: input }))
-      return { absolute, real, directory: location.directory }
+      if (!FSUtil.contains(location.directory, absolute)) return yield* missing
+      const real = yield* fs.realPath(absolute).pipe(Effect.catch(notFound(missing)))
+      if (!FSUtil.contains(root, real)) return yield* missing
+      return real
     })
     return Service.of({
       find: search.find,
       read: Effect.fn("FileSystem.read")(function* (input) {
-        const target = yield* resolve(input.path)
-        const info = yield* fs.stat(target.real).pipe(
-          Effect.catchReason(
-            "PlatformError",
-            "NotFound",
-            () => Effect.fail(new NotFoundError({ path: input.path })),
-            (_, error) => Effect.die(error),
-          ),
-        )
-        if (info.type !== "File") return yield* Effect.fail(new NotFoundError({ path: input.path }))
+        const real = yield* resolve(input.path)
+        const missing = new NotFoundError({ path: input.path })
+        const info = yield* fs.stat(real).pipe(Effect.catch(notFound(missing)))
+        if (info.type !== "File") return yield* missing
         return {
-          mime: FSUtil.mimeType(target.real),
+          mime: FSUtil.mimeType(real),
           size: Number(info.size),
           mtime: info.mtime,
-          stream: (options) => fs.stream(target.real, options),
+          stream: (options) => fs.stream(real, options),
         }
       }),
       list: Effect.fn("FileSystem.list")(function* (input = {}) {
         // Navigation can leave the cwd without activating another Location.
         const directory = path.resolve(location.directory, input.path ?? ".")
-        const missing = new NotFoundError({ path: RelativePath.make(input.path ?? ".") })
-        const info = yield* fs.stat(directory).pipe(
-          Effect.catchReason(
-            "PlatformError",
-            "NotFound",
-            () => Effect.fail(missing),
-            (_, error) => Effect.die(error),
-          ),
-        )
+        const missing = new NotFoundError({ path: input.path ?? "." })
+        const info = yield* fs.stat(directory).pipe(Effect.catch(notFound(missing)))
         if (info.type !== "Directory") return yield* missing
-        return yield* fs.readDirectoryEntries(directory).pipe(
-          Effect.orDie,
-          Effect.map((items) =>
-            items
-              .flatMap((item) => {
-                if (item.type !== "file" && item.type !== "directory") return []
-                const absolute = path.join(directory, item.name)
-                const relative = path.relative(location.directory, absolute) || "."
-                return [
-                  Entry.make({
-                    path: RelativePath.make(relative + (item.type === "directory" ? path.sep : "")),
-                    type: item.type,
-                  }),
-                ]
-              })
-              .sort((a, b) => (a.type === b.type ? a.path.localeCompare(b.path) : a.type === "directory" ? -1 : 1)),
-          ),
+        const entries = yield* Effect.forEach(
+          yield* fs.readDirectoryEntries(directory).pipe(Effect.orDie),
+          Effect.fnUntraced(function* (item) {
+            const absolute = path.join(directory, item.name)
+            const type =
+              item.type === "symlink"
+                ? yield* fs.stat(absolute).pipe(
+                    Effect.map((target) =>
+                      target.type === "Directory" ? "directory" : target.type === "File" ? "file" : undefined,
+                    ),
+                    Effect.orElseSucceed(() => undefined),
+                  )
+                : item.type
+            if (type !== "file" && type !== "directory") return []
+            const relative = path.relative(location.directory, absolute) || "."
+            return [Entry.make({ path: RelativePath.make(relative + (type === "directory" ? path.sep : "")), type })]
+          }),
+          { concurrency: "unbounded" },
         )
+        return entries
+          .flat()
+          .sort((a, b) => (a.type === b.type ? a.path.localeCompare(b.path) : a.type === "directory" ? -1 : 1))
       }),
       // Unlike read, write reaches outside the location so clients can stage files in the
       // server tmp directory, which the model is already told to prefer and permitted to access.
@@ -221,3 +206,9 @@ export const node = makeLocationNode({
   layer: baseLayer,
   deps: [FSUtil.node, Location.node, FileSystemSearch.node],
 })
+
+// A path through a file (ENOTDIR) or a symlink loop (ELOOP) fails as BadResource, not NotFound.
+function notFound(missing: NotFoundError) {
+  return (error: PlatformError.PlatformError) =>
+    error.reason._tag === "NotFound" || error.reason._tag === "BadResource" ? Effect.fail(missing) : Effect.die(error)
+}

packages/core/test/location-filesystem.test.ts

@@ -32,6 +32,9 @@ const withTmp = <A, E, R>(f: (directory: string) => Effect.Effect<A, E, R>) =>
     (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
   ).pipe(Effect.flatMap((tmp) => f(tmp.path)))
 
+// Windows needs Developer Mode or admin rights to create file symlinks.
+const itWithFileSymlinks = process.platform === "win32" ? it.live.skip : it.live
+
 describe("FileSystem", () => {
   it.live("reads text and binary files", () =>
     withTmp((directory) =>
@@ -68,6 +71,28 @@ describe("FileSystem", () => {
     ),
   )
 
+  itWithFileSymlinks("lists symlinks as the type they point to and omits broken links", () =>
+    withTmp((directory) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(async () => {
+          await fs.mkdir(path.join(directory, "realdir"))
+          await fs.writeFile(path.join(directory, "realfile.txt"), "real")
+          await fs.symlink("realdir", path.join(directory, "linkdir"))
+          await fs.symlink("realfile.txt", path.join(directory, "linkfile"))
+          await fs.symlink("missing", path.join(directory, "broken"))
+        })
+        const filesystem = yield* FileSystem.Service
+        const entries = yield* filesystem.list()
+        expect(entries.map((entry) => ({ path: entry.path, type: entry.type }))).toEqual([
+          { path: RelativePath.make("linkdir" + path.sep), type: "directory" },
+          { path: RelativePath.make("realdir" + path.sep), type: "directory" },
+          { path: RelativePath.make("linkfile"), type: "file" },
+          { path: RelativePath.make("realfile.txt"), type: "file" },
+        ])
+      }).pipe(provide(directory)),
+    ),
+  )
+
   it.live("skips host canonicalization for workspace locations at boot", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
@@ -215,13 +240,26 @@ describe("FileSystem", () => {
         yield* Effect.promise(() => fs.writeFile(path.join(directory, "outside.txt"), "outside"))
         yield* Effect.gen(function* () {
           const filesystem = yield* FileSystem.Service
-          const result = yield* filesystem.read({ path: RelativePath.make("../outside.txt") }).pipe(Effect.exit)
-          expect(Exit.isFailure(result)).toBe(true)
+          const escape = yield* Effect.flip(filesystem.read({ path: RelativePath.make("../outside.txt") }))
+          expect(escape).toBeInstanceOf(FileSystem.NotFoundError)
         }).pipe(provide(current))
       }),
     ),
   )
 
+  it.live("reports paths through a file as not found", () =>
+    withTmp((directory) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(() => fs.writeFile(path.join(directory, "file.txt"), "text"))
+        const filesystem = yield* FileSystem.Service
+        const list = yield* Effect.flip(filesystem.list({ path: "file.txt/child" }))
+        expect(list).toBeInstanceOf(FileSystem.NotFoundError)
+        const read = yield* Effect.flip(filesystem.read({ path: RelativePath.make("file.txt/child") }))
+        expect(read).toBeInstanceOf(FileSystem.NotFoundError)
+      }).pipe(provide(directory)),
+    ),
+  )
+
   it.live("allows listing through an external symlink without allowing file reads", () =>
     withTmp((directory) =>
       Effect.gen(function* () {
@@ -233,12 +271,16 @@ describe("FileSystem", () => {
         yield* Effect.promise(() => fs.symlink(outside, path.join(current, "link"), "junction"))
         yield* Effect.gen(function* () {
           const filesystem = yield* FileSystem.Service
+          const parent = yield* filesystem.list()
+          expect(parent.map((entry) => ({ path: entry.path, type: entry.type }))).toEqual([
+            { path: RelativePath.make("link" + path.sep), type: "directory" },
+          ])
           const entries = yield* filesystem.list({ path: RelativePath.make("link") })
           expect(entries.map((entry) => ({ path: entry.path, type: entry.type }))).toEqual([
             { path: RelativePath.make(path.join("link", "file.txt")), type: "file" },
           ])
-          const result = yield* filesystem.read({ path: RelativePath.make("link/file.txt") }).pipe(Effect.exit)
-          expect(Exit.isFailure(result)).toBe(true)
+          const read = yield* Effect.flip(filesystem.read({ path: RelativePath.make("link/file.txt") }))
+          expect(read).toBeInstanceOf(FileSystem.NotFoundError)
         }).pipe(provide(current))
       }),
     ),

packages/server/src/handlers/fs.ts

@@ -58,7 +58,7 @@ export const FileSystemHandler = HttpApiBuilder.group(Api, "server.fs", (handler
 )
 
 function decodeRequestPath(url: string) {
-  const raw = new URL(url, "http://localhost").pathname.slice(13)
+  const raw = new URL(url, "http://localhost").pathname.slice("/api/fs/read/".length)
   return Effect.try({
     try: () => RelativePath.make(decodeURIComponent(raw)),
     catch: () => new FileNotFoundError({ path: raw, message: `File not found: ${raw}` }),

packages/util/src/fs-util.ts

@@ -59,8 +59,6 @@ export namespace FSUtil {
 
   export const use = serviceUse(Service)
 
-  // Exported so simulation can wrap this layer and override the methods that
-  // bypass the injected FileSystem (readDirectoryEntries, scan, globUp).
   export const layer = Layer.effect(
     Service,
     Effect.gen(function* () {