[mcp-server] Expose fuzzy file search in MCP #2677
Conversation
dylan-hurd-oai
added
the
codex-rust-review
github-actions
bot
added
codex-rust-review-in-progress
and removed
codex-rust-review
|
Summary
Review
If not already in the PR body, suggest adding a brief motivation (e.g., expose server-side fuzzy search to power clients’ quick file navigation) and, if applicable, a short note on expected performance/limits. |
github-actions
bot
added
codex-rust-review-completed
and removed
codex-rust-review-in-progress
labels
![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 suggestions.
Reply with @codex fix comments to fix any unresolved comments.
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, or 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 fix this CI failure" or "@codex address that feedback".
| async fn fuzzy_file_search(&self, request_id: RequestId, params: FuzzyFileSearchParams) { | ||
| let FuzzyFileSearchParams { query, roots } = params; | ||
|
|
||
| // Execute fuzzy search across provided roots using codex-file-search. | ||
| use codex_file_search as file_search; | ||
| use std::collections::HashSet; | ||
| use std::num::NonZero; | ||
| use std::path::PathBuf; | ||
|
|
||
| let mut files: Vec<String> = Vec::new(); | ||
| let mut seen: HashSet<String> = HashSet::new(); | ||
| #[expect(clippy::expect_used)] | ||
| let limit_per_root = NonZero::new(50).expect("50 is a valid non-zero usize"); | ||
| #[expect(clippy::expect_used)] | ||
| let threads = NonZero::new(2).expect("2 is a valid non-zero usize"); | ||
| let compute_indices = false; | ||
|
|
||
| for root in roots { | ||
| let search_dir = PathBuf::from(&root); | ||
| // TODO: offer a way to cancel outdated requests in the interface | ||
| let cancel_flag = std::sync::Arc::new(AtomicBool::new(false)); | ||
|
|
||
| // because message_processor runs each request in a separate thread, we don't need to spawn a new thread | ||
| match file_search::run( | ||
| &query, | ||
| limit_per_root, | ||
| &search_dir, | ||
| Vec::new(), | ||
| threads, | ||
| cancel_flag, | ||
| compute_indices, | ||
| ) { | ||
| Ok(res) => { | ||
| for m in res.matches { | ||
| // Join root + relative match path for clarity across multiple roots. | ||
| let full_path = PathBuf::from(&root).join(m.path); | ||
| if let Some(s) = full_path.to_str() | ||
| && seen.insert(s.to_string()) | ||
| { | ||
| files.push(s.to_string()); | ||
| } | ||
| } | ||
| } | ||
| Err(e) => { | ||
| tracing::warn!("fuzzy-file-search root '{}' failed: {}", root, e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let response = FuzzyFileSearchResponse { files }; | ||
| self.outgoing.send_response(request_id, response).await; |
There was a problem hiding this comment.
[P1] Avoid blocking processor during fuzzy search
In fuzzy_file_search (mcp-server/src/codex_message_processor.rs lines 644‑695) the code calls file_search::run directly with a comment claiming each request runs in its own thread. However MessageProcessor::process_request handles requests sequentially on one Tokio task, so this synchronous filesystem traversal can block the entire processor, preventing other JSON‑RPC messages from being serviced and making the server unresponsive during large searches.
Useful? React with 👍 / 👎.
dylan-hurd-oai
force-pushed
the
dh--mcp-file-search
branch
from
e410104 to
23d5c75
Compare
dylan-hurd-oai
force-pushed
the
dh--mcp-file-search
branch
from
23d5c75 to
872cc13
Compare
| .map(std::num::NonZero::get) | ||
| .unwrap_or(1); | ||
| let threads = cores.min(MAX_THREADS); | ||
| let threads_per_root = (threads / roots.len()).max(1); |
There was a problem hiding this comment.
| let threads_per_root = (threads / roots.len()).max(1); | |
| let threads_per_root = (threads / roots.len()).min(1); |
right?
There was a problem hiding this comment.
I think we want max here! It's primarily to catch the case where roots.len() > threads, and make sure threads_per_root != 0
There was a problem hiding this comment.
You want 5.min(1) -> 5 and 0.min(1) -> 1 right?
Isn't 5.max(1) -> 1 and 0.max(1) -> 0?
There was a problem hiding this comment.
Other way around! Here's a repl to make sure we're on the same page:
Summary
Expose a simple fuzzy file search implementation for mcp clients to work with
Testing