fix(server): name the missing file instead of a generic read failure - #7628
fix(server): name the missing file instead of a generic read failure#7628DraftProducts wants to merge 2 commits into
Conversation
Opening a file that is not in the thread's workspace reports:
Failed to read workspace file 'components/selectors/Foo.vue' in '/repo'.
which reads as an internal error rather than the plain fact that the path is
not there. The server already knows: the read fails with ENOENT and the
structured `failure`/`operation` fields are carried on the error. Only the
message was generic.
Chat file links make this easy to hit. They are built from the text of a
reply, so any path an agent mentions becomes a clickable chip whether or not it
exists under the active workspace root, and `ChatView` always resolves it
against that root. The existing basename lookup only rescues paths with no
separator, so a multi-segment path goes straight to a failed read.
A missing workspace root raises ENOENT through the same call, so
`realpath-workspace-root` is excluded rather than blaming the file for it. Every
other failure keeps its current message; `message` was already honored by the
error constructor and is now expressible on the failure-context type.
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One convention issue: the new "file does not exist" wording is passed to ProjectReadFileError as a prebuilt message string instead of being modeled as a structural attribute and derived inside the error class.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This PR improves error messaging for missing workspace files by detecting ENOENT errors and providing specific error messages. Changes are minimal, additive to the schema, and well-tested with comprehensive test coverage for both the main case and edge cases. You can add or adjust custom eligibility rules. Learn more. |
Review feedback: the not-found wording was handed to ProjectReadFileError as a prebuilt `message`, so the distinction lived in text while `failure` stayed `"operation_failed"`. The `decodedProjectErrorMessage` path exists to decode legacy message-only payloads, not to override messages from new code. Classify instead: `projectFileFailureContext` returns a new `"file_not_found"` member of `ProjectFileFailure`, and `ProjectReadFileError` derives the message from `failure`, `relativePath`, and `cwd`. `message` is gone from `ProjectFileFailureContext`, so callers can no longer inject one. The guard is unchanged in substance: a missing workspace root raises ENOENT through the same operation, so `realpath-workspace-root` still classifies as `operation_failed`. Nothing reads these literals for control flow today.
Dismissing prior approval to re-evaluate 46ccbf9
1 participant
What Changed
projects.readFilenow reportsWorkspace file 'X' does not exist in 'Y'.whenthe read fails with ENOENT, instead of the generic
Failed to read workspace file 'X' in 'Y'.Every other failure keeps its current message.A missing workspace root raises ENOENT through the same call, so
realpath-workspace-rootis excluded rather than blaming the file for it. Theerror constructor already honored a supplied
message; the failure-context typejust did not express it, which is the one line in
packages/contracts.messageis already part of the schema, so nothing changes on the wire.
Tests: two cases in
server.test.ts— a missing file is named, and a missingworkspace root is not relabeled as one (that second test fails without the
realpath-workspace-rootguard).server.test.ts(126),workspace/(56),contracts(257), typecheck, lint, and format pass.Why
The generic message reads as an internal error rather than the plain fact that
the path is not there, which sends people debugging the wrong thing.
Chat file links make this easy to hit. They are built from the text of a reply,
so any path an agent mentions becomes a clickable chip whether or not it exists
under the active workspace root, and
ChatViewalways resolves it against thatroot. The existing basename lookup in
ChatMarkdownonly rescues paths with noseparator, so a multi-segment path goes straight to a failed read and the user
gets the opaque message.
Not addressed: the chip is still offered for a file that is not there. Deciding
whether a link should be verified before it is rendered, or resolved across more
than one workspace root, is a product call rather than an error-message fix.
Checklist
Changes by Claude Opus 5 running in Claude Code.
Note
Low Risk
Small, additive error classification and messaging change with integration tests; no auth or data-path changes.
Overview
projectsReadFilenow classifies filesystem ENOENT (except duringrealpath-workspace-root) asfile_not_foundinstead ofoperation_failed, andProjectReadFileErroruses an explicit message: Workspace file '…' does not exist in '…'.Contracts add the new
file_not_foundfailure literal. Server tests cover a missing relative path and assert a missing workspace root still surfacesoperation_failedwithout the “does not exist” wording.Reviewed by Cursor Bugbot for commit 46ccbf9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Name the missing file in
ProjectReadFileErrorinstead of generic read failureAdds
file_not_foundto theProjectFileFailureunion in project.ts and uses it to produce a message that names the relative path and cwd of the missing file.ENOENTerrors tofile_not_found, except when the operation isrealpath-workspace-root(missing root staysoperation_failed).ProjectFileFailuregains a new literal value; out-of-tree consumers that exhaustively match the union need a case forfile_not_found.📊 Macroscope summarized 46ccbf9. 2 files reviewed, 1 issue evaluated, 1 issue filtered, 0 comments posted
🗂️ Filtered Issues
apps/server/src/ws.ts — 0 comments posted, 1 evaluated, 1 filtered
projectFileFailureContextis shared by bothprojectsReadFileandprojectsWriteFile, so this classifies any write-sideENOENTfrommake-directoryorwrite-fileasfile_not_found. A write target is allowed to be absent (the operation creates it); such anENOENTinstead indicates a failed parent-directory/write operation, for example if a parent is concurrently removed.ProjectWriteFileError.failuretherefore changes fromoperation_failedto the misleadingfile_not_found. Restrict this classification to the read RPC/read operations, or use separate failure mapping for writes. [ Already posted ]