diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md index 87ef1bc..3ad85c1 100644 --- a/IMPROVEMENT_PLAN.md +++ b/IMPROVEMENT_PLAN.md @@ -206,10 +206,10 @@ deserialization) and `tools.rs` (grep/find/cat). There are no tests for: result but the `match` is only used for its pattern — this is confusing. **Actions:** -- [ ] Run `cargo clippy -- -W clippy::all -W clippy::pedantic` and fix findings. -- [ ] Replace the `let _ = match …` with a simple `let separator = "-"` (the +- [x] Run `cargo clippy -- -W clippy::all -W clippy::pedantic` and fix findings. +- [x] Replace the `let _ = match …` with a simple `let separator = "-"` (the match arms all return the same value). -- [ ] Use `write!` instead of `format!` + `push_str` in hot paths to avoid +- [x] Use `write!` instead of `format!` + `push_str` in hot paths to avoid intermediate allocations. --- diff --git a/README.md b/README.md index eb0098f..89bdbc0 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,8 @@ The point: Claude Code's local MCP support is stdio-only. This server speaks str ## Tools +All tools return structured `ToolResponse` objects with metadata (truncation status, error counts, match counts) rather than plain strings. This allows clients to programmatically detect truncation and other conditions. + ### `grep` Regex search across files using parallel directory traversal (`ignore` + `grep-searcher`). @@ -17,17 +19,23 @@ Regex search across files using parallel directory traversal (`ignore` + `grep-s | -------------------- | --------------- | ------- | ----------------------------------------------------------- | | `directory` | `string` | — | required | | `pattern` | `string` | — | required; Rust `regex` flavor — no lookaround/backrefs | -| `before_context` | `int` | `0` | | -| `after_context` | `int` | `0` | | -| `max_results` | `int` | `100` | exact cap (no over-shoot) | +| `output_mode` | `string` | `files_with_matches` | `files_with_matches` (list matching files; fast for broad scans), `content` (matching lines with context), or `count` (per-file match tally) | +| `before_context` | `int` | `0` | lines of context before matches (ignored in `files_with_matches` and `count` modes) | +| `after_context` | `int` | `0` | lines of context after matches (ignored in `files_with_matches` and `count` modes) | +| `max_results` | `int` | `100` | exact cap (no over-shoot); for `files_with_matches`, caps the number of files; for `content`, caps the number of matching lines; for `count`, caps the number of files | | `case_insensitive` | `bool` | `false` | equivalent to `(?i)` prefix in `pattern` | | `include_hidden` | `bool` | `false` | | | `follow_symlinks` | `bool` | `false` | | | `respect_gitignore` | `bool` | `true` | | | `file_extensions` | `string[]` | `[]` | e.g. `["rs", "toml"]`; empty = all | -| `max_bytes` | `int` | ~5 MiB | hard cap on response size; appends `[truncated: byte cap]` | +| `max_bytes` | `int` | ~5 MiB | hard cap on response size | + +**Output modes:** +- **`files_with_matches`** (default): Returns only file paths that contain matches. Each path appears once (on first match), then the file's search stops early — efficient for broad reconnaissance queries. `max_results` caps the number of files. +- **`content`**: Returns matching lines with optional context (before/after). The classic grep output mode, useful when line-level detail is needed. `max_results` caps the number of lines. +- **`count`**: Returns per-file match tallies as `path: N` lines, sorted by path. Useful for understanding distribution of matches across files. -Walker errors and search errors are tallied and reported as a `[notice: N entry errors, M search errors; first: ...]` footer rather than silently dropped. +Walker errors and search errors are tallied and returned in the response metadata rather than silently dropped. ### `find` Find files by regex. @@ -65,10 +73,10 @@ Read file contents with pagination. | ----------- | -------- | ------- | -------------------------------------------------------------------- | | `file_path` | `string` | — | required | | `offset` | `int` | `0` | 0-based line number to start from | -| `max_lines` | `int` | `2000` | appends `[truncated: line cap]` if more lines remain | -| `max_bytes` | `int` | ~5 MiB | appends `[truncated: byte cap]` if hit mid-line (UTF-8-safe cut) | +| `max_lines` | `int` | `2000` | maximum lines to return per call | +| `max_bytes` | `int` | ~5 MiB | hard cap on response size (UTF-8-safe cut at line boundary) | -Use `offset` to page: if the response ends with `[truncated: line cap]`, call again with `offset = previous_offset + max_lines`. +Use `offset` to page through large files: if the response indicates truncation, call again with `offset = previous_offset + max_lines`. The response will include metadata indicating whether the result was truncated and the reason. ## Build & run diff --git a/src/args.rs b/src/args.rs index 06b8fc5..dfd86bd 100644 --- a/src/args.rs +++ b/src/args.rs @@ -58,6 +58,7 @@ fn default_output_mode() -> String { #[derive(Debug, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] +#[allow(clippy::struct_excessive_bools)] pub struct GrepArgs { #[schemars(description = "Directory to search in")] pub directory: String, diff --git a/src/cli.rs b/src/cli.rs index 5328e23..b55fd02 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -17,7 +17,7 @@ pub struct Args { pub(crate) memory_dir: Option, /// Required project root. Every path the tools touch (grep/find - /// directory, cat file_path) is canonicalized and must be within + /// directory, cat `file_path`) is canonicalized and must be within /// this directory; anything outside is rejected. Symlinks in input /// paths are resolved before the check, so a symlink pointing out /// of the project is also rejected. diff --git a/src/error.rs b/src/error.rs index a1ac57c..4bdb45f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,11 +5,13 @@ use thiserror::Error; /// Convert a `tokio::task::JoinError` (from `spawn_blocking`) into an /// `rmcp::ErrorData` with `internal_error` code. Used by every tool handler /// so the `.map_err` boilerplate is a single call. +/// +/// Takes `JoinError` by value so it can be passed directly as +/// `.map_err(join_error)` — `JoinError` is not `Clone`, and the only +/// field we need is accessed via `&self`. +#[allow(clippy::needless_pass_by_value)] pub fn join_error(e: tokio::task::JoinError) -> ErrorData { - ErrorData::internal_error( - "internal_error", - Some(json!({"error": e.to_string()})), - ) + ErrorData::internal_error("internal_error", Some(json!({"error": e.to_string()}))) } /// Convenience alias for tool handler return types. diff --git a/src/limiter.rs b/src/limiter.rs index 8c353de..8dbe17c 100644 --- a/src/limiter.rs +++ b/src/limiter.rs @@ -32,7 +32,7 @@ impl PeerLimiter { /// Convenience: a per-minute rate (capacity = `rate`, refill = `rate`/60s). pub fn per_minute(rate: u32) -> Self { - let cap = rate.max(1) as f64; + let cap = f64::from(rate.max(1)); Self::new(cap, cap / 60.0, 4096) } diff --git a/src/memory.rs b/src/memory.rs index 3ef1416..e870597 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -1,4 +1,5 @@ use crate::error::AppError; +use std::fmt::Write; use std::path::Path; /// Load a memory file from the given directory, or return an index/listing. @@ -18,16 +19,12 @@ pub fn load_memory(dir: &Path, name: Option<&str>) -> Result { // Reject path traversal: name must be a single, non-empty path component. if name.is_empty() || name.contains('/') || name.contains('\\') || name.contains("..") { return Err(AppError::InvalidRequest(format!( - "memory name must be a plain filename, got: {:?}", - name + "memory name must be a plain filename, got: {name:?}" ))); } let path = dir.join(name); if !path.is_file() { - return Err(AppError::NotFound(format!( - "memory not found: {}", - name - ))); + return Err(AppError::NotFound(format!("memory not found: {name}"))); } return Ok(std::fs::read_to_string(&path)?); } @@ -40,7 +37,7 @@ pub fn load_memory(dir: &Path, name: Option<&str>) -> Result { let mut listing = String::from("# Memory dir contents\n\n"); let mut entries: Vec<_> = std::fs::read_dir(dir)? - .filter_map(|e| e.ok()) + .filter_map(std::result::Result::ok) .filter(|e| { e.path() .extension() @@ -48,13 +45,13 @@ pub fn load_memory(dir: &Path, name: Option<&str>) -> Result { .is_some_and(|s| s == "md") }) .collect(); - entries.sort_by_key(|e| e.file_name()); + entries.sort_by_key(std::fs::DirEntry::file_name); if entries.is_empty() { listing.push_str("(no .md files found; configure MEMORY.md or add memory files)\n"); } else { for e in entries { if let Some(name) = e.file_name().to_str() { - listing.push_str(&format!("- {}\n", name)); + let _ = writeln!(listing, "- {name}"); } } listing.push_str( diff --git a/src/reaper.rs b/src/reaper.rs index 4ecd969..5ae3d91 100644 --- a/src/reaper.rs +++ b/src/reaper.rs @@ -47,7 +47,7 @@ pub async fn reap_loop( ticker.tick().await; // first tick fires immediately; skip it loop { tokio::select! { - _ = cancel.cancelled() => return, + () = cancel.cancelled() => return, _ = ticker.tick() => { sweep(&manager, &tracker, idle_timeout).await; } diff --git a/src/scope.rs b/src/scope.rs index 1cad142..0e8b1ce 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -12,10 +12,11 @@ impl Scope { /// Build a scope. The root must exist and be a directory. It is /// canonicalized at construction time so symlinks inside the /// configured path are resolved once. - pub fn new(root: PathBuf) -> Result { - let canon = root.canonicalize().map_err(|e| { - AppError::Internal(format!("--project {}: {}", root.display(), e)) - })?; + pub fn new(root: impl AsRef) -> Result { + let root = root.as_ref(); + let canon = root + .canonicalize() + .map_err(|e| AppError::Internal(format!("--project {}: {}", root.display(), e)))?; if !canon.is_dir() { return Err(AppError::Internal(format!( "--project must be a directory: {}", diff --git a/src/server.rs b/src/server.rs index 1d0e8fe..dd94dcd 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1,3 +1,4 @@ +use std::fmt::Write; use std::path::PathBuf; use rmcp::{ @@ -152,12 +153,13 @@ impl ServerHandler for CodeMcpServer { let mut instructions = String::from( "code-mcp: filesystem search and read tools.\n\n", ); - instructions.push_str(&format!( + let _ = write!( + instructions, "All paths are scoped to the project root: {}. \ Paths outside this directory (or symlinks resolving outside it) are rejected \ with `invalid_params`.\n\n", self.scope.root().display() - )); + ); instructions.push_str( "\ Regex flavor: Rust `regex` crate. No lookaround or backreferences. \ diff --git a/src/tools.rs b/src/tools.rs index 2a84325..3ca2c2d 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -1,7 +1,7 @@ use crate::error::AppError; use grep_regex::{RegexMatcher, RegexMatcherBuilder}; use grep_searcher::{ - BinaryDetection, Searcher, SearcherBuilder, Sink, SinkContext, SinkContextKind, SinkMatch, + BinaryDetection, Searcher, SearcherBuilder, Sink, SinkContext, SinkMatch, }; use ignore::{WalkBuilder, WalkState}; use regex::Regex; @@ -9,6 +9,7 @@ use rmcp::model::CallToolResult; use serde::Serialize; use serde_json::json; use std::collections::HashMap; +use std::fmt::Write; use std::fs::File; use std::io::{self, BufRead, BufReader}; use std::mem; @@ -38,7 +39,7 @@ pub struct ToolResponse { pub content: String, /// Whether the output was truncated due to a size cap. pub truncated: bool, - /// If truncated, the reason (e.g. "byte_cap", "line_cap"). + /// If truncated, the reason (e.g. "`byte_cap`", "`line_cap`"). pub truncation_reason: Option, /// Number of matches found (grep / find). pub match_count: Option, @@ -98,13 +99,16 @@ impl OutputMode { "content" => Ok(Self::Content), "count" => Ok(Self::Count), other => Err(AppError::InvalidRequest(format!( - "unknown output_mode '{}'; expected one of: files_with_matches, content, count", - other + "unknown output_mode '{other}'; expected one of: files_with_matches, content, count" ))), } } } +/// Configuration for the `grep` tool. The boolean fields are independent +/// search/walker toggles; grouping them into enums would obscure the +/// (flat) JSON contract exposed to MCP clients. +#[allow(clippy::struct_excessive_bools)] pub struct GrepOptions { pub before_context: usize, pub after_context: usize, @@ -135,6 +139,7 @@ impl Default for GrepOptions { } } +#[derive(Clone, Copy)] pub struct FindOptions { pub max_results: usize, pub include_hidden: bool, @@ -169,7 +174,7 @@ struct MatchSink<'a> { max_bytes: usize, } -impl<'a> Sink for MatchSink<'a> { +impl Sink for MatchSink<'_> { type Error = io::Error; fn matched( @@ -187,8 +192,7 @@ impl<'a> Sink for MatchSink<'a> { } let line_num = mat.line_number().unwrap_or(0); let line = String::from_utf8_lossy(mat.bytes()); - self.buf - .push_str(&format!("{}:{}: {}", self.path.display(), line_num, line)); + let _ = write!(self.buf, "{}:{}: {}", self.path.display(), line_num, line); if !line.ends_with('\n') { self.buf.push('\n'); } @@ -206,19 +210,17 @@ impl<'a> Sink for MatchSink<'a> { return Ok(false); } // All context kinds use the same separator. - let _ = match ctx.kind() { - SinkContextKind::Before | SinkContextKind::After | SinkContextKind::Other => "-", - }; let separator = "-"; let line_num = ctx.line_number().unwrap_or(0); let line = String::from_utf8_lossy(ctx.bytes()); - self.buf.push_str(&format!( + let _ = write!( + self.buf, "{}{}{} {}", self.path.display(), separator, line_num, line - )); + ); if !line.ends_with('\n') { self.buf.push('\n'); } @@ -239,7 +241,7 @@ struct FileMatchSink<'a> { matched_this_file: bool, } -impl<'a> Sink for FileMatchSink<'a> { +impl Sink for FileMatchSink<'_> { type Error = io::Error; fn matched( @@ -317,15 +319,16 @@ fn record_first(slot: &Mutex>, msg: String) { // grep // --------------------------------------------------------------------------- +#[allow(clippy::needless_pass_by_value)] pub fn grep( directory: &str, pattern: &str, opts: GrepOptions, ) -> Result { match opts.output_mode { - OutputMode::Content => grep_content(directory, pattern, opts), - OutputMode::FilesWithMatches => grep_files(directory, pattern, opts), - OutputMode::Count => grep_count(directory, pattern, opts), + OutputMode::Content => grep_content(directory, pattern, &opts), + OutputMode::FilesWithMatches => grep_files(directory, pattern, &opts), + OutputMode::Count => grep_count(directory, pattern, &opts), } } @@ -348,8 +351,7 @@ fn extension_matches(path: &Path, extensions: &[String]) -> bool { } path.extension() .and_then(|e| e.to_str()) - .map(|e| extensions.iter().any(|w| w == e)) - .unwrap_or(false) + .is_some_and(|e| extensions.iter().any(|w| w == e)) } /// Collect shared error state into the final `ToolResponse` metadata fields. @@ -371,10 +373,11 @@ fn error_metadata( /// `content` mode — the original behaviour: emit matching lines with line /// numbers, streaming through the mpsc pipeline. +#[allow(clippy::too_many_lines)] fn grep_content( directory: &str, pattern: &str, - opts: GrepOptions, + opts: &GrepOptions, ) -> Result { let matcher: RegexMatcher = RegexMatcherBuilder::new() .case_insensitive(opts.case_insensitive) @@ -398,7 +401,7 @@ fn grep_content( let extensions = opts.file_extensions.clone(); - let walker = build_parallel_walker(directory, &opts); + let walker = build_parallel_walker(directory, opts); let (tx, rx) = channel::(); @@ -520,10 +523,11 @@ fn grep_content( /// `files_with_matches` mode — emit the file path on the first match, then /// abort searching that file. `max_results` caps the number of *files*. +#[allow(clippy::too_many_lines)] fn grep_files( directory: &str, pattern: &str, - opts: GrepOptions, + opts: &GrepOptions, ) -> Result { let matcher: RegexMatcher = RegexMatcherBuilder::new() .case_insensitive(opts.case_insensitive) @@ -546,7 +550,7 @@ fn grep_files( let extensions = opts.file_extensions.clone(); - let walker = build_parallel_walker(directory, &opts); + let walker = build_parallel_walker(directory, opts); let (tx, rx) = channel::(); @@ -662,7 +666,7 @@ fn grep_files( fn grep_count( directory: &str, pattern: &str, - opts: GrepOptions, + opts: &GrepOptions, ) -> Result { let matcher: RegexMatcher = RegexMatcherBuilder::new() .case_insensitive(opts.case_insensitive) @@ -687,7 +691,7 @@ fn grep_count( // Shared map: canonical path string → match count. let file_counts: Arc>> = Arc::new(Mutex::new(HashMap::new())); - let walker = build_parallel_walker(directory, &opts); + let walker = build_parallel_walker(directory, opts); walker.run(|| { let entry_errors = Arc::clone(&entry_errors); @@ -729,7 +733,9 @@ fn grep_count( if sink.count > 0 { let key = path.to_string_lossy().into_owned(); - let mut map = file_counts.lock().unwrap_or_else(|e| e.into_inner()); + let mut map = file_counts + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); *map.entry(key).or_insert(0) += sink.count; } @@ -738,7 +744,9 @@ fn grep_count( }); // Sort by path for deterministic output. - let mut counts_map = file_counts.lock().unwrap_or_else(|e| e.into_inner()); + let mut counts_map = file_counts + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let mut entries: Vec<_> = counts_map.drain().collect(); entries.sort_by(|a, b| a.0.cmp(&b.0)); @@ -753,7 +761,7 @@ fn grep_count( let mut output = String::new(); for (path, count) in &entries { - let line = format!("{}: {}\n", path, count); + let line = format!("{path}: {count}\n"); if output.len() + line.len() > max_bytes { break; } @@ -793,7 +801,6 @@ pub fn find( ) -> Result { let re = Regex::new(pattern)?; let max_results = opts.max_results; - let count = Arc::new(AtomicUsize::new(0)); let entry_errors = Arc::new(AtomicUsize::new(0)); let first_error: Arc>> = Arc::new(Mutex::new(None));