Skip to content

Refactor workspace reads to Effect FileSystem #4050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cursor wants to merge 2 commits into
base: main
Choose a base branch
from
+120 −115
Draft

Refactor workspace reads to Effect FileSystem #4050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51f3403
refactor workspace reads to Effect FileSystem
cursoragent Jul 16, 2026
ea7294b
style workspace filesystem assertions
cursoragent Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 46 additions & 31 deletions apps/server/src/workspace/WorkspaceFileSystem.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as NodeServices from "@effect/platform-node/NodeServices";
import { it, describe, expect } from "@effect/vitest";
import { assert, describe, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as PlatformError from "effect/PlatformError";

import * as ServerConfig from "../config.ts";
import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
Expand Down Expand Up @@ -64,7 +66,7 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
relativePath: "src/index.ts",
});

expect(result).toEqual({
assert.deepStrictEqual(result, {
relativePath: "src/index.ts",
contents: "export const answer = 42;\n",
byteLength: 26,
Expand All @@ -82,7 +84,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
.readFile({ cwd, relativePath: "../escape.md" })
.pipe(Effect.flip);

expect(error.message).toContain(
assert.include(
error.message,
"Workspace file path must be relative to the project root: ../escape.md",
);
}),
Expand All @@ -107,14 +110,14 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
const resolvedWorkspaceRoot = yield* fileSystem.realPath(cwd);
const resolvedPath = yield* fileSystem.realPath(path.join(outsideDir, "secret.txt"));

expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFilePathEscapeError);
expect(error).toMatchObject({
assert.strictEqual(error._tag, "WorkspaceFilePathEscapeError");
assert.deepInclude(error, {
workspaceRoot: cwd,
relativePath: "linked-secret.txt",
resolvedWorkspaceRoot,
resolvedPath,
});
expect("cause" in error).toBe(false);
assert.notProperty(error, "cause");
}),
);

Expand All @@ -131,13 +134,13 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
.pipe(Effect.flip);
const resolvedPath = yield* fileSystem.realPath(path.join(cwd, "src"));

expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspacePathNotFileError);
expect(error).toMatchObject({
assert.strictEqual(error._tag, "WorkspacePathNotFileError");
assert.deepInclude(error, {
workspaceRoot: cwd,
relativePath: "src",
resolvedPath,
});
expect("cause" in error).toBe(false);
assert.notProperty(error, "cause");
}),
);

Expand All @@ -155,14 +158,31 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
.pipe(Effect.flip);
const resolvedPath = yield* fileSystem.realPath(absolutePath);

expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceBinaryFileError);
expect(error).toMatchObject({
assert.strictEqual(error._tag, "WorkspaceBinaryFileError");
assert.deepInclude(error, {
workspaceRoot: cwd,
relativePath: "asset.bin",
resolvedPath,
});
expect("cause" in error).toBe(false);
expect("contents" in error).toBe(false);
assert.notProperty(error, "cause");
assert.notProperty(error, "contents");
}),
);

it.effect("reads at most the preview limit", () =>
Effect.gen(function* () {
const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem;
const cwd = yield* makeTempDir;
yield* writeTextFile(cwd, "large.txt", "a".repeat(1024 * 1024 + 1));

const result = yield* workspaceFileSystem.readFile({
cwd,
relativePath: "large.txt",
});

assert.strictEqual(result.contents.length, 1024 * 1024);
assert.strictEqual(result.byteLength, 1024 * 1024 + 1);
assert.isTrue(result.truncated);
}),
);

Expand All @@ -177,16 +197,16 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
.readFile({ cwd, relativePath: "missing.txt" })
.pipe(Effect.flip);

expect(error).toBeInstanceOf(WorkspaceFileSystem.WorkspaceFileSystemOperationError);
expect(error).toMatchObject({
assert.strictEqual(error._tag, "WorkspaceFileSystemOperationError");
assert.deepInclude(error, {
workspaceRoot: cwd,
relativePath: "missing.txt",
resolvedPath,
operationPath: resolvedPath,
operation: "realpath-target",
});
expect(error.cause).toBeInstanceOf(Error);
expect((error.cause as NodeJS.ErrnoException).code).toBe("ENOENT");
assert.instanceOf(error.cause, PlatformError.PlatformError);
assert.strictEqual((error.cause as PlatformError.PlatformError).reason._tag, "NotFound");
}),
);
});
Expand All @@ -207,8 +227,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
.readFileString(path.join(cwd, "plans/effect-rpc.md"))
.pipe(Effect.orDie);

expect(result).toEqual({ relativePath: "plans/effect-rpc.md" });
expect(saved).toBe("# Plan\n");
assert.deepStrictEqual(result, { relativePath: "plans/effect-rpc.md" });
assert.strictEqual(saved, "# Plan\n");
}),
);

Expand All @@ -220,9 +240,7 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
yield* writeTextFile(cwd, "src/existing.ts", "export {};\n");

const beforeWrite = yield* workspaceEntries.list({ cwd });
expect(beforeWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md")).toBe(
false,
);
assert.isFalse(beforeWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md"));

yield* workspaceFileSystem.writeFile({
cwd,
Expand All @@ -231,10 +249,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
});

const afterWrite = yield* workspaceEntries.list({ cwd });
expect(afterWrite.entries).toEqual(
expect.arrayContaining([expect.objectContaining({ path: "plans/effect-rpc.md" })]),
);
expect(afterWrite.truncated).toBe(false);
assert.isTrue(afterWrite.entries.some((entry) => entry.path === "plans/effect-rpc.md"));
assert.isFalse(afterWrite.truncated);
}),
);

Expand All @@ -253,15 +269,14 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i
})
.pipe(Effect.flip);

expect(error.message).toContain(
assert.include(
error.message,
"Workspace file path must be relative to the project root: ../escape.md",
);

const escapedPath = path.resolve(cwd, "..", "escape.md");
const escapedStat = yield* fileSystem
.stat(escapedPath)
.pipe(Effect.orElseSucceed(() => null));
expect(escapedStat).toBeNull();
const escapedStat = yield* fileSystem.stat(escapedPath).pipe(Effect.option);
assert.isTrue(Option.isNone(escapedStat));
}),
);
});
Expand Down
158 changes: 74 additions & 84 deletions apps/server/src/workspace/WorkspaceFileSystem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @effect-diagnostics nodeBuiltinImport:off
/**
* WorkspaceFileSystem - Effect service contract for workspace file mutations.
*
Expand All @@ -7,7 +6,6 @@
*
* @module WorkspaceFileSystem
*/
import * as NodeFSP from "node:fs/promises";

import type {
ProjectReadFileInput,
Expand All @@ -19,6 +17,7 @@ import * as Context from "effect/Context";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as Schema from "effect/Schema";

Expand Down Expand Up @@ -140,30 +139,32 @@ export const make = Effect.gen(function* () {
relativePath: input.relativePath,
});

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 realWorkspaceRoot = yield* fileSystem.realPath(input.cwd).pipe(
Effect.mapError(
(cause) =>
new WorkspaceFileSystemOperationError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: target.absolutePath,
operationPath: input.cwd,
operation: "realpath-workspace-root",
cause,
}),
),
);
const realTargetPath = yield* fileSystem.realPath(target.absolutePath).pipe(
Effect.mapError(
(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}`) ||
Expand All @@ -178,24 +179,24 @@ export const make = Effect.gen(function* () {
});
}

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) =>
return yield* Effect.scoped(

@macroscopeapp macroscopeapp Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium workspace/WorkspaceFileSystem.ts:182

When the file handle close fails, readFile now throws an untyped defect instead of a WorkspaceFileSystemOperationError with operation close. The old code wrapped handle.close() failures in a typed error; Effect.scoped delegates cleanup to FileSystem.open's finalizer, which runs Effect.orDie(nodeClose(fd)), so a close failure terminates the effect as a defect rather than through the service's advertised error channel. Consider using Effect.acquireRelease (or Effect.acquireUseRelease) with an explicit close error mapping so close failures remain typed errors.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent: 
In file @apps/server/src/workspace/WorkspaceFileSystem.ts around line 182:

When the file handle close fails, `readFile` now throws an untyped defect instead of a `WorkspaceFileSystemOperationError` with operation `close`. The old code wrapped `handle.close()` failures in a typed error; `Effect.scoped` delegates cleanup to `FileSystem.open`'s finalizer, which runs `Effect.orDie(nodeClose(fd))`, so a close failure terminates the effect as a defect rather than through the service's advertised error channel. Consider using `Effect.acquireRelease` (or `Effect.acquireUseRelease`) with an explicit `close` error mapping so close failures remain typed errors.

Effect.gen(function* () {
const handle = yield* fileSystem.open(realTargetPath, { flag: "r" }).pipe(
Effect.mapError(
(cause) =>
new WorkspaceFileSystemOperationError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: realTargetPath,
operationPath: realTargetPath,
operation: "open",
cause,
}),
),
);
const stat = yield* handle.stat.pipe(
Effect.mapError(
(cause) =>
new WorkspaceFileSystemOperationError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
Expand All @@ -204,20 +205,21 @@ export const make = Effect.gen(function* () {
operation: "stat",
cause,
}),
),
);
if (stat.type !== "File") {
return yield* new WorkspacePathNotFileError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: realTargetPath,
});
if (!stat.isFile()) {
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 } = yield* Effect.tryPromise({
try: () => handle.read(buffer, 0, bytesToRead, 0),
catch: (cause) =>
const byteLength = Number(stat.size);
const bytesToRead = Math.min(byteLength, PROJECT_READ_FILE_MAX_BYTES);
const maybeFileBytes = yield* handle.readAlloc(bytesToRead).pipe(
Effect.mapError(
(cause) =>
new WorkspaceFileSystemOperationError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
Expand All @@ -226,36 +228,24 @@ export const make = Effect.gen(function* () {
operation: "read",
cause,
}),
),
);
const fileBytes = Option.getOrElse(maybeFileBytes, () => new Uint8Array());
if (fileBytes.includes(0)) {
return yield* new WorkspaceBinaryFileError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: realTargetPath,
});
const fileBytes = buffer.subarray(0, bytesRead);
if (fileBytes.includes(0)) {
return yield* new WorkspaceBinaryFileError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: realTargetPath,
});
}
}

return {
relativePath: target.relativePath,
contents: new TextDecoder("utf-8").decode(fileBytes),
byteLength: stat.size,
truncated: stat.size > PROJECT_READ_FILE_MAX_BYTES,
};
}),
(handle) =>
Effect.tryPromise({
try: () => handle.close(),
catch: (cause) =>
new WorkspaceFileSystemOperationError({
workspaceRoot: input.cwd,
relativePath: input.relativePath,
resolvedPath: realTargetPath,
operationPath: realTargetPath,
operation: "close",
cause,
}),
}),
return {
relativePath: target.relativePath,
contents: new TextDecoder("utf-8").decode(fileBytes),
byteLength,
truncated: byteLength > PROJECT_READ_FILE_MAX_BYTES,
};
}),
);
});

Expand Down
Loading