⚡ Replace blocking File I/O with tokio::fs in MCP Edit Tools#201
⚡ Replace blocking File I/O with tokio::fs in MCP Edit Tools#201
Conversation
Replace blocking `std::fs` calls with asynchronous `tokio::fs` in all MCP tools within `crates/mcp-edit`. This prevents the Tokio executor threads from being blocked during file operations, improving the overall responsiveness and throughput of the server. Specific changes: - Added `fs` feature to `tokio` dependency in `mcp-edit`. - Updated `resolve` and `resolve_for_write` helpers to be `async`. - Replaced `fs::read_to_string`, `fs::write`, `fs::read`, `fs::canonicalize`, and `fs::create_dir_all` with their `tokio::fs` counterparts. - Optimized sorting in `glob` tool to fetch metadata once per file asynchronously instead of blocking inside the sort closure. - Fixed `create_file` existence check to use `tokio::fs::metadata`. Co-authored-by: dstoc <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f965e3e19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/mcp-edit/src/lib.rs
Outdated
| let metadata = match tokio::fs::metadata(&canonical).await { | ||
| Ok(m) => m, | ||
| Err(e) => { | ||
| return Ok(Self::tool_error(format!( | ||
| "failed to get metadata for {}: {e}", | ||
| self.display_path(&canonical) | ||
| ))); |
There was a problem hiding this comment.
This change turns glob into a hard error whenever tokio::fs::metadata fails for a matched path. Previously, Path::is_file() / is_dir() implicitly returned false on metadata errors, so a path that was deleted between the glob match and the metadata check (or a permission-denied entry) would simply be skipped and the rest of the results still returned. Now any such transient or permission issue aborts the entire tool call, which is a behavioral regression for users running glob over large/volatile trees. Consider preserving the prior behavior by treating metadata errors as non-matches instead of returning a tool error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I have refactored read_many_files (and other tools) to address the performance issues more comprehensively.
Key changes in read_many_files:
- Blocking Operations Moved to
spawn_blocking: The globbing and directory walking parts, which use the synchronousglobandignorecrates, are now wrapped intokio::task::spawn_blocking. This prevents them from blocking the async executor. - Parallel File Reading: Once the file paths are collected, I now use
tokio::spawnto read the file contents in parallel. - Improved Async Safety: Replaced the sequential
.awaitcalls inside synchronous loops with a more efficient parallel approach.
I've also applied similar optimizations to the glob, list_directory, and search_file_content tools to ensure no blocking I/O occurs on the executor threads. I also removed the accidental log files from the previous submission.
- Refactored `read_many_files`, `glob`, `list_directory`, and `search_file_content` to use `tokio::task::spawn_blocking` for synchronous I/O operations (globbing, directory walking). - Parallelized file reading in `read_many_files` and metadata fetching in `glob`. - Ensured all tool handlers are truly non-blocking for the Tokio executor. - Removed unnecessary log files. - Added `sync` feature to `tokio` dependency. Co-authored-by: dstoc <[email protected]>
1 participant
The optimization replaces blocking
std::fscalls with asynchronoustokio::fsalternatives in themcp-editcrate. Blocking I/O in asynchronous functions can lead to executor starvation, especially under high load. By switching totokio::fs, these operations are now offloaded to a thread pool, allowing the main Tokio worker threads to handle more concurrent requests.Key improvements:
replace,read_file,read_many_files,create_file,glob, andsearch_file_contentare now non-blocking.resolvehelper is now async, ensuring path resolution doesn't block.globtool was refactored to perform metadata fetching asynchronously before sorting, avoiding redundant blocking syscalls.PR created automatically by Jules for task 8823185894213304279 started by @dstoc