-
Notifications
You must be signed in to change notification settings - Fork 0
MES-709: Upgrade mesa-dev to new API
#32
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.
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
MES-709: Upgrade mesa-dev to new API
#32
Conversation
Update all API call sites to use the new hierarchical client pattern (client.org().repos().at().content()), new Content/DirEntry enums from mesa_dev::low_level::content, and string-based error wrapping since the error type is now generic per endpoint.
Mesa DescriptionThis PR upgrades the
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performed full review of 6fc3cda...9c2fd82
Analysis
-
The migration to mesa-dev 1.8 removes structured error context that was previously used to distinguish between authentication, throttling, and missing data issues, which may negatively impact observability and errno mapping.
-
With the new dependency on the low-level content API, filesystem semantics (file/directory/symlink distinctions) must be preserved with absolute precision to prevent abstraction leakage through the FUSE bridge.
-
The extensive plumbing changes required to adapt to the new builder- and resource-based SDK models may introduce subtle compatibility issues between the OrgFs/RepoFs layers and the new response models.
-
Converting SDK errors to bare strings simplifies trait bounds but removes valuable context that higher layers previously utilized, creating a trade-off that warrants further evaluation.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 2 comments | Edit Agent Settings • Read Docs
src/fs/mescloud/repo.rs
Outdated
| let kind = match &content { | ||
| mesa_dev::models::Content::File { .. } => DirEntryType::RegularFile, | ||
| mesa_dev::models::Content::Dir { .. } => DirEntryType::Directory, | ||
| Content::File(_) | Content::Symlink(_) => DirEntryType::RegularFile, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Content::Symlink is being surfaced as a DirEntryType::RegularFile (and below as a FileAttr::RegularFile). From the FUSE side this means lstat/readdir will never report an entry as a symlink, so the kernel will happily open/read it instead of issuing a readlink, and all downstream tooling will think the repo tree contains plain files. The new content API distinguishes File vs Symlink so we should preserve that by returning DirEntryType::Symlink and a FileAttr::Symlink { size } for links (and wiring up readlink/ENOSYS separately). Otherwise every symlink in a repo is misrepresented and will behave as an opaque regular file.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: mesa-dot-dev/gitfs#32
File: src/fs/mescloud/repo.rs#L133
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
`Content::Symlink` is being surfaced as a `DirEntryType::RegularFile` (and below as a `FileAttr::RegularFile`). From the FUSE side this means `lstat`/`readdir` will never report an entry as a symlink, so the kernel will happily `open`/`read` it instead of issuing a `readlink`, and all downstream tooling will think the repo tree contains plain files. The new content API distinguishes `File` vs `Symlink` so we should preserve that by returning `DirEntryType::Symlink` and a `FileAttr::Symlink { size }` for links (and wiring up `readlink`/`ENOSYS` separately). Otherwise every symlink in a repo is misrepresented and will behave as an opaque regular file.
| mesa_dev::models::Content::File { content, .. } => content, | ||
| mesa_dev::models::Content::Dir { .. } => return Err(ReadError::NotAFile), | ||
| Content::File(f) => f.content.unwrap_or_default(), | ||
| Content::Symlink(s) => s.content.unwrap_or_default(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read() now base64‑decodes Content::Symlink and returns the underlying string just like a file. That isn’t valid FUSE semantics: reading a symlink should either be rejected (EINVAL/ENOSYS) or follow the link via a dedicated readlink implementation. With the current code a cat of a symlink feeds the encoded target instead of the file it points at, and there is no way for the kernel to discover the link target via readlink. Once symlinks are surfaced as DirEntryType::Symlink, this branch should return ReadError::NotAFile (or implement a proper readlink) rather than pretending the link is file content.
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: mesa-dot-dev/gitfs#32
File: src/fs/mescloud/repo.rs#L306
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
`read()` now base64‑decodes `Content::Symlink` and returns the underlying string just like a file. That isn’t valid FUSE semantics: reading a symlink should either be rejected (`EINVAL`/`ENOSYS`) or follow the link via a dedicated `readlink` implementation. With the current code a `cat` of a symlink feeds the encoded target instead of the file it points at, and there is no way for the kernel to discover the link target via `readlink`. Once symlinks are surfaced as `DirEntryType::Symlink`, this branch should return `ReadError::NotAFile` (or implement a proper `readlink`) rather than pretending the link is file content.
No description provided.