[codex] Structure workspace file system errors#3274
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: RPC reads missing error cause
- Added workspaceFileReadCompatibilityDetail() to map each WorkspaceFileSystemError tag to its legacy user-visible description, replacing the broken cause.cause access that produced 'undefined' for the three new error types lacking a cause property.
Or push these changes by commenting:
@cursor push 8dd7e5a105
Preview (8dd7e5a105)
diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts
--- a/apps/server/src/ws.ts
+++ b/apps/server/src/ws.ts
@@ -170,6 +170,23 @@
}
}
+function workspaceFileReadCompatibilityDetail(
+ error: WorkspaceFileSystem.WorkspaceFileSystemError,
+): string {
+ switch (error._tag) {
+ case "WorkspaceFileSystemOperationError":
+ return legacyPlatformFailureDescription(error.cause);
+ case "WorkspaceFilePathEscapeError":
+ return "Workspace file path resolves outside the project root.";
+ case "WorkspacePathNotFileError":
+ return "Workspace path is not a file.";
+ case "WorkspaceBinaryFileError":
+ return "Binary files cannot be previewed as text.";
+ default:
+ return unexpectedCompatibilityError(error);
+ }
+}
+
function projectSetupScriptCompatibilityDetail(
error: ProjectSetupScriptRunner.ProjectSetupScriptRunnerError,
): string {
@@ -1288,7 +1305,7 @@
Effect.mapError((cause) => {
const message = isWorkspacePathOutsideRootError(cause)
? "Workspace file path must stay within the project root."
- : `Failed to read workspace file: ${legacyPlatformFailureDescription(cause.cause)}`;
+ : `Failed to read workspace file: ${workspaceFileReadCompatibilityDetail(cause)}`;
return new ProjectReadFileError({ message, cause });
}),
),You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit a7c5cce. Configure here.
There was a problem hiding this comment.
Effect service review — 1 finding. The refactor splits WorkspaceFileSystemError into four classes, three of which intentionally carry no cause. The sole production consumer (apps/server/src/ws.ts:1291, the projects.readFile RPC handler) still builds its user-facing message from cause.cause, which is now undefined for those errors — surfacing "Failed to read workspace file: undefined" to the UI for not-a-file, binary-file, and symlink-escape cases. See inline comment.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Approved This PR restructures workspace file system error handling to use structured fields instead of generic messages, improving debuggability and security (avoids leaking sensitive data like query contents). No runtime behavior changes - only error reporting format is affected, with backwards compatibility maintained. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Dismissing prior approval to re-evaluate d2e1471
Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit cdfecb3)


Summary
Validation
vp test apps/server/src/workspace/WorkspaceFileSystem.test.tsvp checkvp run typecheckNote
Medium Risk
Changes the wire shape and message text for multiple workspace RPC error paths; clients that relied on old embedded platform strings or query text in messages need to use
failureand optional fields instead.Overview
Workspace file I/O now returns a union of tagged errors (
WorkspaceFileSystemOperationError, path escape, not-a-file, binary) withworkspaceRoot,resolvedPath, and per-stepoperationlabels.readFileusesacquireUseReleaseso file handles close on success, semantic failure, I/O error, or interruption.WebSocket RPC maps workspace/search/browse/file errors through helpers that populate
failurecodes and request context onProjectSearchEntriesError,ProjectListEntriesError,ProjectReadFileError,ProjectWriteFileError, andFilesystemBrowseError. User-facingmessagestrings are derived from cwd/paths only—search queries and file contents are omitted from payloads and messages; rawcauseremains for debugging.Contracts add optional structured fields with custom constructors for new call sites and decode compatibility for legacy message-only errors during rolling upgrades. Tests assert structured RPC shapes and that sensitive tokens do not appear in error text.
Reviewed by Cursor Bugbot for commit d2e1471. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure workspace file system RPC errors with typed, sanitized error classes
WorkspaceFileSystemOperationError,WorkspaceFilePathEscapeError,WorkspacePathNotFileError,WorkspaceBinaryFileError) defined in WorkspaceFileSystem.ts.projectsSearchEntries,projectsListEntries,projectsReadFile,projectsWriteFile, andfilesystemBrowseto populate these structured error types, removing thelegacyPlatformFailureDescriptionhelper.Macroscope summarized d2e1471.