[codex] Structure workspace file system errors

apps/server/src/server.test.ts

@@ -4443,6 +4443,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
       const outsideFile = path.join(outsideDir, "outside.txt");
       yield* fs.writeFileString(outsideFile, "outside\n");
       yield* fs.symlink(outsideFile, path.join(workspaceDir, "linked-outside.txt"));
+      const resolvedOutsideFile = yield* fs.realPath(outsideFile);
 
       yield* buildAppUnderTest();
 
@@ -4485,7 +4486,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => {
       assertTrue(results.read._tag === "Failure");
       assert.equal(
         results.read.failure.message,
-        "Failed to read workspace file: Workspace file path resolves outside the project root.",
+        `Failed to read workspace file: Workspace file 'linked-outside.txt' resolves outside workspace root '${workspaceDir}': ${resolvedOutsideFile}`,
       );
       assertTrue(results.browse._tag === "Failure");
       assert.equal(

apps/server/src/workspace/WorkspaceFileSystem.test.ts

@@ -104,14 +104,89 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
         const error = yield* workspaceFileSystem
           .readFile({ cwd, relativePath: "linked-secret.txt" })
           .pipe(Effect.flip);
+        const resolvedWorkspaceRoot = yield* fileSystem.realPath(cwd);
+        const resolvedPath = yield* fileSystem.realPath(path.join(outsideDir, "secret.txt"));
 
-        expect(error.message).toBe(
-          `Workspace file operation 'workspaceFileSystem.readFile' failed for 'linked-secret.txt' in '${cwd}'.`,
-        );
+        expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFilePathEscapeError);
+        expect(error).toMatchObject({
+          workspaceRoot: cwd,
+          relativePath: "linked-secret.txt",
+          resolvedWorkspaceRoot,
+          resolvedPath,
+        });
+        expect("cause" in error).toBe(false);
+      }),
+    );
+
+    it.effect("rejects directories without manufacturing an I/O cause", () =>
+      Effect.gen(function* () {
+        const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem;
+        const fileSystem = yield* FileSystem.FileSystem;
+        const path = yield* Path.Path;
+        const cwd = yield* makeTempDir;
+        yield* fileSystem.makeDirectory(path.join(cwd, "src"));
+
+        const error = yield* workspaceFileSystem
+          .readFile({ cwd, relativePath: "src" })
+          .pipe(Effect.flip);
+        const resolvedPath = yield* fileSystem.realPath(path.join(cwd, "src"));
+
+        expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspacePathNotFileError);
+        expect(error).toMatchObject({
+          workspaceRoot: cwd,
+          relativePath: "src",
+          resolvedPath,
+        });
+        expect("cause" in error).toBe(false);
+      }),
+    );
+
+    it.effect("rejects binary files without leaking their contents into the error", () =>
+      Effect.gen(function* () {
+        const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem;
+        const fileSystem = yield* FileSystem.FileSystem;
+        const path = yield* Path.Path;
+        const cwd = yield* makeTempDir;
+        const absolutePath = path.join(cwd, "asset.bin");
+        yield* fileSystem.writeFile(absolutePath, Uint8Array.from([0x61, 0, 0x62]));
+
+        const error = yield* workspaceFileSystem
+          .readFile({ cwd, relativePath: "asset.bin" })
+          .pipe(Effect.flip);
+        const resolvedPath = yield* fileSystem.realPath(absolutePath);
+
+        expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceBinaryFileError);
+        expect(error).toMatchObject({
+          workspaceRoot: cwd,
+          relativePath: "asset.bin",
+          resolvedPath,
+        });
+        expect("cause" in error).toBe(false);
+        expect("contents" in error).toBe(false);
+      }),
+    );
+
+    it.effect("preserves the real cause and path for I/O failures", () =>
+      Effect.gen(function* () {
+        const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem;
+        const path = yield* Path.Path;
+        const cwd = yield* makeTempDir;
+        const resolvedPath = path.join(cwd, "missing.txt");
+
+        const error = yield* workspaceFileSystem
+          .readFile({ cwd, relativePath: "missing.txt" })
+          .pipe(Effect.flip);
+
+        expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFileSystemOperationError);
+        expect(error).toMatchObject({
+          workspaceRoot: cwd,
+          relativePath: "missing.txt",
+          resolvedPath,
+          operationPath: resolvedPath,
+          operation: "realpath-target",
+        });
         expect(error.cause).toBeInstanceOf(Error);
-        expect((error.cause as Error).message).toBe(
-          "Workspace file path resolves outside the project root.",
-        );
+        expect((error.cause as NodeJS.ErrnoException).code).toBe("ENOENT");
       }),
     );
   });

apps/server/src/workspace/WorkspaceFileSystem.ts

@@ -27,25 +27,79 @@ import * as WorkspacePaths from "./WorkspacePaths.ts";
 
 const PROJECT_READ_FILE_MAX_BYTES = 1024 * 1024;
 
-export class WorkspaceFileSystemError extends Schema.TaggedErrorClass<WorkspaceFileSystemError>()(
-  "WorkspaceFileSystemError",
+export class WorkspaceFileSystemOperationError extends Schema.TaggedErrorClass<WorkspaceFileSystemOperationError>()(
+  "WorkspaceFileSystemOperationError",
   {
-    cwd: Schema.String,
-    relativePath: Schema.optional(Schema.String),
+    workspaceRoot: Schema.String,
+    relativePath: Schema.String,
+    resolvedPath: Schema.String,
+    operationPath: Schema.String,
     operation: Schema.Literals([
-      "workspaceFileSystem.readFile",
-      "workspaceFileSystem.makeDirectory",
-      "workspaceFileSystem.writeFile",
+      "realpath-workspace-root",
+      "realpath-target",
+      "open",
+      "stat",
+      "read",
+      "close",
+      "make-directory",
+      "write-file",
     ]),
     cause: Schema.Defect(),
   },
 ) {
   override get message(): string {
-    const target = this.relativePath ? `'${this.relativePath}' in '${this.cwd}'` : `'${this.cwd}'`;
-    return `Workspace file operation '${this.operation}' failed for ${target}.`;
+    return `Workspace file operation '${this.operation}' failed at '${this.operationPath}' for resolved path '${this.resolvedPath}' (requested as '${this.relativePath}' in '${this.workspaceRoot}').`;
   }
 }
 
+export class WorkspaceFilePathEscapeError extends Schema.TaggedErrorClass<WorkspaceFilePathEscapeError>()(
+  "WorkspaceFilePathEscapeError",
+  {
+    workspaceRoot: Schema.String,
+    relativePath: Schema.String,
+    resolvedWorkspaceRoot: Schema.String,
+    resolvedPath: Schema.String,
+  },
+) {
+  override get message(): string {
+    return `Workspace file '${this.relativePath}' resolves outside workspace root '${this.workspaceRoot}': ${this.resolvedPath}`;
+  }
+}
+
+export class WorkspacePathNotFileError extends Schema.TaggedErrorClass<WorkspacePathNotFileError>()(
+  "WorkspacePathNotFileError",
+  {
+    workspaceRoot: Schema.String,
+    relativePath: Schema.String,
+    resolvedPath: Schema.String,
+  },
+) {
+  override get message(): string {
+    return `Workspace path '${this.relativePath}' in '${this.workspaceRoot}' is not a file: ${this.resolvedPath}`;
+  }
+}
+
+export class WorkspaceBinaryFileError extends Schema.TaggedErrorClass<WorkspaceBinaryFileError>()(
+  "WorkspaceBinaryFileError",
+  {
+    workspaceRoot: Schema.String,
+    relativePath: Schema.String,
+    resolvedPath: Schema.String,
+  },
+) {
+  override get message(): string {
+    return `Workspace file '${this.relativePath}' in '${this.workspaceRoot}' is binary and cannot be previewed as text.`;
+  }
+}
+
+export const WorkspaceFileSystemError = Schema.Union([
+  WorkspaceFileSystemOperationError,
+  WorkspaceFilePathEscapeError,
+  WorkspacePathNotFileError,
+  WorkspaceBinaryFileError,
+]);
+export type WorkspaceFileSystemError = typeof WorkspaceFileSystemError.Type;
+
 /** Service tag for workspace file operations. */
 export class WorkspaceFileSystem extends Context.Service<
   WorkspaceFileSystem,
@@ -86,53 +140,123 @@ export const make = Effect.gen(function* () {
       relativePath: input.relativePath,
     });
 
-    return yield* Effect.tryPromise({
-      try: async () => {
-        const [realWorkspaceRoot, realTargetPath] = await Promise.all([
-          NodeFSP.realpath(input.cwd),
-          NodeFSP.realpath(target.absolutePath),
-        ]);
-        const relativeRealPath = path.relative(realWorkspaceRoot, realTargetPath);
-        if (
-          relativeRealPath.startsWith(`..${path.sep}`) ||
-          relativeRealPath === ".." ||
-          path.isAbsolute(relativeRealPath)
-        ) {
-          throw new Error("Workspace file path resolves outside the project root.");
-        }
-
-        const handle = await NodeFSP.open(realTargetPath, "r");
-        try {
-          const stat = await handle.stat();
+    const realWorkspaceRoot = yield* Effect.tryPromise({
+      try: () => NodeFSP.realpath(input.cwd),
+      catch: (cause) =>
+        new WorkspaceFileSystemOperationError({
+          workspaceRoot: input.cwd,
+          relativePath: input.relativePath,
+          resolvedPath: target.absolutePath,
+          operationPath: input.cwd,
+          operation: "realpath-workspace-root",
+          cause,
+        }),
+    });
+    const realTargetPath = yield* Effect.tryPromise({
+      try: () => NodeFSP.realpath(target.absolutePath),
+      catch: (cause) =>
+        new WorkspaceFileSystemOperationError({
+          workspaceRoot: input.cwd,
+          relativePath: input.relativePath,
+          resolvedPath: target.absolutePath,
+          operationPath: target.absolutePath,
+          operation: "realpath-target",
+          cause,
+        }),
+    });
+    const relativeRealPath = path.relative(realWorkspaceRoot, realTargetPath);
+    if (
+      relativeRealPath.startsWith(`..${path.sep}`) ||
+      relativeRealPath === ".." ||
+      path.isAbsolute(relativeRealPath)
+    ) {
+      return yield* new WorkspaceFilePathEscapeError({
+        workspaceRoot: input.cwd,
+        relativePath: input.relativePath,
+        resolvedWorkspaceRoot: realWorkspaceRoot,
+        resolvedPath: realTargetPath,
+      });
+    }
+
+    return yield* Effect.acquireUseRelease(
+      Effect.tryPromise({
+        try: () => NodeFSP.open(realTargetPath, "r"),
+        catch: (cause) =>
+          new WorkspaceFileSystemOperationError({
+            workspaceRoot: input.cwd,
+            relativePath: input.relativePath,
+            resolvedPath: realTargetPath,
+            operationPath: realTargetPath,
+            operation: "open",
+            cause,
+          }),
+      }),
+      (handle) =>
+        Effect.gen(function* () {
+          const stat = yield* Effect.tryPromise({
+            try: () => handle.stat(),
+            catch: (cause) =>
+              new WorkspaceFileSystemOperationError({
+                workspaceRoot: input.cwd,
+                relativePath: input.relativePath,
+                resolvedPath: realTargetPath,
+                operationPath: realTargetPath,
+                operation: "stat",
+                cause,
+              }),
+          });
           if (!stat.isFile()) {
-            throw new Error("Workspace path is not a file.");
+            return yield* new WorkspacePathNotFileError({
+              workspaceRoot: input.cwd,
+              relativePath: input.relativePath,
+              resolvedPath: realTargetPath,
+            });
           }
+
           const bytesToRead = Math.min(stat.size, PROJECT_READ_FILE_MAX_BYTES);
           const buffer = Buffer.alloc(bytesToRead);
-          const { bytesRead } = await handle.read(buffer, 0, bytesToRead, 0);
+          const { bytesRead } = yield* Effect.tryPromise({
+            try: () => handle.read(buffer, 0, bytesToRead, 0),
+            catch: (cause) =>
+              new WorkspaceFileSystemOperationError({
+                workspaceRoot: input.cwd,
+                relativePath: input.relativePath,
+                resolvedPath: realTargetPath,
+                operationPath: realTargetPath,
+                operation: "read",
+                cause,
+              }),
+          });
           const fileBytes = buffer.subarray(0, bytesRead);
           if (fileBytes.includes(0)) {
-            throw new Error("Binary files cannot be previewed as text.");
+            return yield* new WorkspaceBinaryFileError({
+              workspaceRoot: input.cwd,
+              relativePath: input.relativePath,
+              resolvedPath: realTargetPath,
+            });
           }
-          const contents = new TextDecoder("utf-8").decode(fileBytes);
+
           return {
             relativePath: target.relativePath,
-            contents,
+            contents: new TextDecoder("utf-8").decode(fileBytes),
             byteLength: stat.size,
             truncated: stat.size > PROJECT_READ_FILE_MAX_BYTES,
           };
-        } finally {
-          await handle.close();
-        }
-      },
-      catch: (cause) =>
-        new WorkspaceFileSystemError({
-          cwd: input.cwd,
-          relativePath: input.relativePath,
-          operation: "workspaceFileSystem.readFile",
-          cause,
         }),
-    });
+      (handle) =>
+        Effect.tryPromise({
+          try: () => handle.close(),
+          catch: (cause) =>
+            new WorkspaceFileSystemOperationError({
+              workspaceRoot: input.cwd,
+              relativePath: input.relativePath,
+              resolvedPath: realTargetPath,
+              operationPath: realTargetPath,
+              operation: "close",
+              cause,
+            }),
+        }),
+    );
   });
 
   const writeFile: WorkspaceFileSystem["Service"]["writeFile"] = Effect.fn(
@@ -146,21 +270,25 @@ export const make = Effect.gen(function* () {
     yield* fileSystem.makeDirectory(path.dirname(target.absolutePath), { recursive: true }).pipe(
       Effect.mapError(
         (cause) =>
-          new WorkspaceFileSystemError({
-            cwd: input.cwd,
+          new WorkspaceFileSystemOperationError({
+            workspaceRoot: input.cwd,
             relativePath: input.relativePath,
-            operation: "workspaceFileSystem.makeDirectory",
+            resolvedPath: target.absolutePath,
+            operationPath: path.dirname(target.absolutePath),
+            operation: "make-directory",
             cause,
           }),
       ),
     );
     yield* fileSystem.writeFileString(target.absolutePath, input.contents).pipe(
       Effect.mapError(
         (cause) =>
-          new WorkspaceFileSystemError({
-            cwd: input.cwd,
+          new WorkspaceFileSystemOperationError({
+            workspaceRoot: input.cwd,
             relativePath: input.relativePath,
-            operation: "workspaceFileSystem.writeFile",
+            resolvedPath: target.absolutePath,
+            operationPath: target.absolutePath,
+            operation: "write-file",
             cause,
           }),
       ),