History

refactor workspace reads to Effect FileSystem

Cursor Agent committed 3 months ago

Commit 51f34031 parent fdca1542 files · +122 −113Browse files

Unified · Split

2 files
  1. apps/server/src/workspace/WorkspaceFileSystem.test.ts +48 −29
  2. apps/server/src/workspace/WorkspaceFileSystem.ts +74 −84
apps/server/src/workspace/WorkspaceFileSystem.test.ts+48−29
@@ -1,9 +1,11 @@
11 import * as NodeServices from "@effect/platform-node/NodeServices";
2−import { it, describe, expect } from "@effect/vitest";
2+import { assert, describe, it } from "@effect/vitest";
33 import * as Effect from "effect/Effect";
44 import * as FileSystem from "effect/FileSystem";
55 import * as Layer from "effect/Layer";
6+import * as Option from "effect/Option";
67 import * as Path from "effect/Path";
8+import * as PlatformError from "effect/PlatformError";
79
810 import * as ServerConfig from "../config.ts";
911 import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
@@ -64,7 +66,7 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 54 unmodified lines
6466 relativePath: "src/index.ts",
6567 });
6668
67− expect(result).toEqual({
69+ assert.deepStrictEqual(result, {
6870 relativePath: "src/index.ts",
6971 contents: "export const answer = 42;\n",
7072 byteLength: 26,
@@ -82,7 +84,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 11 unmodified lines
8284 .readFile({ cwd, relativePath: "../escape.md" })
8385 .pipe(Effect.flip);
8486
85− expect(error.message).toContain(
87+ assert.include(
88+ error.message,
8689 "Workspace file path must be relative to the project root: ../escape.md",
8790 );
8891 }),
@@ -107,14 +110,14 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 18 unmodified lines
107110 const resolvedWorkspaceRoot = yield* fileSystem.realPath(cwd);
108111 const resolvedPath = yield* fileSystem.realPath(path.join(outsideDir, "secret.txt"));
109112
110− expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFilePathEscapeError);
111− expect(error).toMatchObject({
113+ assert.strictEqual(error._tag, "WorkspaceFilePathEscapeError");
114+ assert.deepInclude(error, {
112115 workspaceRoot: cwd,
113116 relativePath: "linked-secret.txt",
114117 resolvedWorkspaceRoot,
115118 resolvedPath,
116119 });
117− expect("cause" in error).toBe(false);
120+ assert.notProperty(error, "cause");
118121 }),
119122 );
120123
@@ -131,13 +134,13 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 10 unmodified lines
131134 .pipe(Effect.flip);
132135 const resolvedPath = yield* fileSystem.realPath(path.join(cwd, "src"));
133136
134− expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspacePathNotFileError);
135− expect(error).toMatchObject({
137+ assert.strictEqual(error._tag, "WorkspacePathNotFileError");
138+ assert.deepInclude(error, {
136139 workspaceRoot: cwd,
137140 relativePath: "src",
138141 resolvedPath,
139142 });
140− expect("cause" in error).toBe(false);
143+ assert.notProperty(error, "cause");
141144 }),
142145 );
143146
@@ -155,14 +158,31 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 11 unmodified lines
155158 .pipe(Effect.flip);
156159 const resolvedPath = yield* fileSystem.realPath(absolutePath);
157160
158− expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceBinaryFileError);
159− expect(error).toMatchObject({
161+ assert.strictEqual(error._tag, "WorkspaceBinaryFileError");
162+ assert.deepInclude(error, {
160163 workspaceRoot: cwd,
161164 relativePath: "asset.bin",
162165 resolvedPath,
163166 });
164− expect("cause" in error).toBe(false);
165− expect("contents" in error).toBe(false);
167+ assert.notProperty(error, "cause");
168+ assert.notProperty(error, "contents");
169+ }),
170+ );
171+
172+ it.effect("reads at most the preview limit", () =>
173+ Effect.gen(function* () {
174+ const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem;
175+ const cwd = yield* makeTempDir;
176+ yield* writeTextFile(cwd, "large.txt", "a".repeat(1024 * 1024 + 1));
177+
178+ const result = yield* workspaceFileSystem.readFile({
179+ cwd,
180+ relativePath: "large.txt",
181+ });
182+
183+ assert.strictEqual(result.contents.length, 1024 * 1024);
184+ assert.strictEqual(result.byteLength, 1024 * 1024 + 1);
185+ assert.isTrue(result.truncated);
166186 }),
167187 );
168188
@@ -177,16 +197,16 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 8 unmodified lines
177197 .readFile({ cwd, relativePath: "missing.txt" })
178198 .pipe(Effect.flip);
179199
180− expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFileSystemOperationError);
181− expect(error).toMatchObject({
200+ assert.strictEqual(error._tag, "WorkspaceFileSystemOperationError");
201+ assert.deepInclude(error, {
182202 workspaceRoot: cwd,
183203 relativePath: "missing.txt",
184204 resolvedPath,
185205 operationPath: resolvedPath,
186206 operation: "realpath-target",
187207 });
188− expect(error.cause).toBeInstanceOf(Error);
189− expect((error.cause as NodeJS.ErrnoException).code).toBe("ENOENT");
208+ assert.instanceOf(error.cause, PlatformError.PlatformError);
209+ assert.strictEqual((error.cause as PlatformError.PlatformError).reason._tag, "NotFound");
190210 }),
191211 );
192212 });
@@ -207,8 +227,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 14 unmodified lines
207227 .readFileString(path.join(cwd, "plans/effect-rpc.md"))
208228 .pipe(Effect.orDie);
209229
210− expect(result).toEqual({ relativePath: "plans/effect-rpc.md" });
211− expect(saved).toBe("# Plan\n");
230+ assert.deepStrictEqual(result, { relativePath: "plans/effect-rpc.md" });
231+ assert.strictEqual(saved, "# Plan\n");
212232 }),
213233 );
214234
@@ -220,8 +240,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 5 unmodified lines
220240 yield* writeTextFile(cwd, "src/existing.ts", "export {};\n");
221241
222242 const beforeWrite = yield* workspaceEntries.list({ cwd });
223− expect(beforeWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md")).toBe(
224− false,
243+ assert.isFalse(
244+ beforeWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md"),
225245 );
226246
227247 yield* workspaceFileSystem.writeFile({
@@ -231,10 +251,10 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 3 unmodified lines
231251 });
232252
233253 const afterWrite = yield* workspaceEntries.list({ cwd });
234− expect(afterWrite.entries).toEqual(
235− expect.arrayContaining([expect.objectContaining({ path: "plans/effect-rpc.md" })]),
254+ assert.isTrue(
255+ afterWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md"),
236256 );
237− expect(afterWrite.truncated).toBe(false);
257+ assert.isFalse(afterWrite.truncated);
238258 }),
239259 );
240260
@@ -253,15 +273,14 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i · 12 unmodified lines
253273 })
254274 .pipe(Effect.flip);
255275
256− expect(error.message).toContain(
276+ assert.include(
277+ error.message,
257278 "Workspace file path must be relative to the project root: ../escape.md",
258279 );
259280
260281 const escapedPath = path.resolve(cwd, "..", "escape.md");
261− const escapedStat = yield* fileSystem
262− .stat(escapedPath)
263− .pipe(Effect.orElseSucceed(() => null));
264− expect(escapedStat).toBeNull();
282+ const escapedStat = yield* fileSystem.stat(escapedPath).pipe(Effect.option);
283+ assert.isTrue(Option.isNone(escapedStat));
265284 }),
266285 );
267286 });
apps/server/src/workspace/WorkspaceFileSystem.ts+74−84
@@ -1,4 +1,3 @@
1−// @effect-diagnostics nodeBuiltinImport:off
21 /**
32 * WorkspaceFileSystem - Effect service contract for workspace file mutations.
43 *
@@ -7,7 +6,6 @@ · 2 unmodified lines
76 *
87 * @module WorkspaceFileSystem
98 */
10−import * as NodeFSP from "node:fs/promises";
119
1210 import type {
1311 ProjectReadFileInput,
@@ -19,6 +17,7 @@ import * as Context from "effect/Context"; · 5 unmodified lines
1917 import * as Effect from "effect/Effect";
2018 import * as FileSystem from "effect/FileSystem";
2119 import * as Layer from "effect/Layer";
20+import * as Option from "effect/Option";
2221 import * as Path from "effect/Path";
2322 import * as Schema from "effect/Schema";
2423
@@ -140,30 +139,32 @@ export const make = Effect.gen(function* () { · 115 unmodified lines
140139 relativePath: input.relativePath,
141140 });
142141
143− const realWorkspaceRoot = yield* Effect.tryPromise({
144− try: () => NodeFSP.realpath(input.cwd),
145− catch: (cause) =>
146− new WorkspaceFileSystemOperationError({
147− workspaceRoot: input.cwd,
148− relativePath: input.relativePath,
149− resolvedPath: target.absolutePath,
150− operationPath: input.cwd,
151− operation: "realpath-workspace-root",
152− cause,
153− }),
154− });
155− const realTargetPath = yield* Effect.tryPromise({
156− try: () => NodeFSP.realpath(target.absolutePath),
157− catch: (cause) =>
158− new WorkspaceFileSystemOperationError({
159− workspaceRoot: input.cwd,
160− relativePath: input.relativePath,
161− resolvedPath: target.absolutePath,
162− operationPath: target.absolutePath,
163− operation: "realpath-target",
164− cause,
165− }),
166− });
142+ const realWorkspaceRoot = yield* fileSystem.realPath(input.cwd).pipe(
143+ Effect.mapError(
144+ (cause) =>
145+ new WorkspaceFileSystemOperationError({
146+ workspaceRoot: input.cwd,
147+ relativePath: input.relativePath,
148+ resolvedPath: target.absolutePath,
149+ operationPath: input.cwd,
150+ operation: "realpath-workspace-root",
151+ cause,
152+ }),
153+ ),
154+ );
155+ const realTargetPath = yield* fileSystem.realPath(target.absolutePath).pipe(
156+ Effect.mapError(
157+ (cause) =>
158+ new WorkspaceFileSystemOperationError({
159+ workspaceRoot: input.cwd,
160+ relativePath: input.relativePath,
161+ resolvedPath: target.absolutePath,
162+ operationPath: target.absolutePath,
163+ operation: "realpath-target",
164+ cause,
165+ }),
166+ ),
167+ );
167168 const relativeRealPath = path.relative(realWorkspaceRoot, realTargetPath);
168169 if (
169170 relativeRealPath.startsWith(`..${path.sep}`) ||
@@ -178,24 +179,24 @@ export const make = Effect.gen(function* () { · 8 unmodified lines
178179 });
179180 }
180181
181− return yield* Effect.acquireUseRelease(
182− Effect.tryPromise({
183− try: () => NodeFSP.open(realTargetPath, "r"),
184− catch: (cause) =>
185− new WorkspaceFileSystemOperationError({
186− workspaceRoot: input.cwd,
187− relativePath: input.relativePath,
188− resolvedPath: realTargetPath,
189− operationPath: realTargetPath,
190− operation: "open",
191− cause,
192− }),
193− }),
194− (handle) =>
195− Effect.gen(function* () {
196− const stat = yield* Effect.tryPromise({
197− try: () => handle.stat(),
198− catch: (cause) =>
182+ return yield* Effect.scoped(
183+ Effect.gen(function* () {
184+ const handle = yield* fileSystem.open(realTargetPath, { flag: "r" }).pipe(
185+ Effect.mapError(
186+ (cause) =>
187+ new WorkspaceFileSystemOperationError({
188+ workspaceRoot: input.cwd,
189+ relativePath: input.relativePath,
190+ resolvedPath: realTargetPath,
191+ operationPath: realTargetPath,
192+ operation: "open",
193+ cause,
194+ }),
195+ ),
196+ );
197+ const stat = yield* handle.stat.pipe(
198+ Effect.mapError(
199+ (cause) =>
199200 new WorkspaceFileSystemOperationError({
200201 workspaceRoot: input.cwd,
201202 relativePath: input.relativePath,
@@ -204,20 +205,21 @@ export const make = Effect.gen(function* () { · 2 unmodified lines
204205 operation: "stat",
205206 cause,
206207 }),
208+ ),
209+ );
210+ if (stat.type !== "File") {
211+ return yield* new WorkspacePathNotFileError({
212+ workspaceRoot: input.cwd,
213+ relativePath: input.relativePath,
214+ resolvedPath: realTargetPath,
207215 });
208− if (!stat.isFile()) {
209− return yield* new WorkspacePathNotFileError({
210− workspaceRoot: input.cwd,
211− relativePath: input.relativePath,
212− resolvedPath: realTargetPath,
213− });
214− }
216+ }
215217
216− const bytesToRead = Math.min(stat.size, PROJECT_READ_FILE_MAX_BYTES);
217− const buffer = Buffer.alloc(bytesToRead);
218− const { bytesRead } = yield* Effect.tryPromise({
219− try: () => handle.read(buffer, 0, bytesToRead, 0),
220− catch: (cause) =>
218+ const byteLength = Number(stat.size);
219+ const bytesToRead = Math.min(byteLength, PROJECT_READ_FILE_MAX_BYTES);
220+ const maybeFileBytes = yield* handle.readAlloc(bytesToRead).pipe(
221+ Effect.mapError(
222+ (cause) =>
221223 new WorkspaceFileSystemOperationError({
222224 workspaceRoot: input.cwd,
223225 relativePath: input.relativePath,
@@ -226,36 +228,24 @@ export const make = Effect.gen(function* () { · 2 unmodified lines
226228 operation: "read",
227229 cause,
228230 }),
231+ ),
232+ );
233+ const fileBytes = Option.getOrElse(maybeFileBytes, () => new Uint8Array());
234+ if (fileBytes.includes(0)) {
235+ return yield* new WorkspaceBinaryFileError({
236+ workspaceRoot: input.cwd,
237+ relativePath: input.relativePath,
238+ resolvedPath: realTargetPath,
229239 });
230− const fileBytes = buffer.subarray(0, bytesRead);
231− if (fileBytes.includes(0)) {
232− return yield* new WorkspaceBinaryFileError({
233− workspaceRoot: input.cwd,
234− relativePath: input.relativePath,
235− resolvedPath: realTargetPath,
236− });
237− }
240+ }
238241
239− return {
240− relativePath: target.relativePath,
241− contents: new TextDecoder("utf-8").decode(fileBytes),
242− byteLength: stat.size,
243− truncated: stat.size > PROJECT_READ_FILE_MAX_BYTES,
244− };
245− }),
246− (handle) =>
247− Effect.tryPromise({
248− try: () => handle.close(),
249− catch: (cause) =>
250− new WorkspaceFileSystemOperationError({
251− workspaceRoot: input.cwd,
252− relativePath: input.relativePath,
253− resolvedPath: realTargetPath,
254− operationPath: realTargetPath,
255− operation: "close",
256− cause,
257− }),
258− }),
242+ return {
243+ relativePath: target.relativePath,
244+ contents: new TextDecoder("utf-8").decode(fileBytes),
245+ byteLength,
246+ truncated: byteLength > PROJECT_READ_FILE_MAX_BYTES,
247+ };
248+ }),
259249 );
260250 });
261251