From 3db04e8a41e5902a089982849f76ea81ccf801b0 Mon Sep 17 00:00:00 2001 From: Sewer56 Date: Fri, 27 Feb 2026 19:40:33 +0000 Subject: [PATCH] Fixed: Surface grep/glob partial errors end-to-end Capture per-file traversal and search failures in core grep/glob as structured partial metadata (partial, errors) instead of silently dropping them. Propagate partial state through serdesAI absolute/allowed wrappers using explicit JSON payloads while preserving existing non-partial text behavior. Deduplicate wrapper conversion logic by extracting shared helpers in common::glob and common::grep so absolute and allowed variants stay consistent. --- src/llm-coding-tools-core/src/tools/glob.rs | 58 ++++++++- src/llm-coding-tools-core/src/tools/grep.rs | 114 +++++++++++++++--- .../src/absolute/glob.rs | 38 +++--- .../src/absolute/grep.rs | 46 +++++-- .../src/allowed/glob.rs | 37 +++--- .../src/allowed/grep.rs | 45 +++++-- .../src/common/glob.rs | 39 ++++++ .../src/common/grep.rs | 31 +++++ .../src/common/mod.rs | 2 + 9 files changed, 334 insertions(+), 76 deletions(-) create mode 100644 src/llm-coding-tools-serdesai/src/common/glob.rs create mode 100644 src/llm-coding-tools-serdesai/src/common/grep.rs diff --git a/src/llm-coding-tools-core/src/tools/glob.rs b/src/llm-coding-tools-core/src/tools/glob.rs index 2d083046..8a3f6d05 100644 --- a/src/llm-coding-tools-core/src/tools/glob.rs +++ b/src/llm-coding-tools-core/src/tools/glob.rs @@ -17,6 +17,12 @@ pub struct GlobOutput { /// Whether results were truncated due to limit. #[serde(skip_serializing_if = "std::ops::Not::not")] pub truncated: bool, + /// Whether one or more paths could not be traversed or processed. + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub partial: bool, + /// Per-path traversal errors encountered while collecting matches. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub errors: Vec, } /// Finds files matching a glob pattern in the given directory. @@ -39,6 +45,7 @@ pub fn glob_files( let matcher = Glob::new(pattern)?.compile_matcher(); let mut files_with_mtime: Vec<(String, SystemTime)> = Vec::new(); + let mut errors: Vec = Vec::with_capacity(8); let walker = WalkBuilder::new(&path) .hidden(false) @@ -50,7 +57,10 @@ pub fn glob_files( for entry_result in walker { let entry = match entry_result { Ok(e) => e, - Err(_) => continue, + Err(err) => { + errors.push(format!("walk error: {err}")); + continue; + } }; if let Some(ft) = entry.file_type() { @@ -63,7 +73,14 @@ pub fn glob_files( let rel_path = match entry.path().strip_prefix(&path) { Ok(p) => p.to_string_lossy().into_owned(), - Err(_) => continue, + Err(err) => { + errors.push(format!( + "failed to make '{}' relative to '{}': {err}", + entry.path().display(), + path.display() + )); + continue; + } }; // Normalize Windows backslashes to forward slashes for glob pattern matching @@ -97,7 +114,12 @@ pub fn glob_files( .map(|(path, _)| path) .collect(); - Ok(GlobOutput { files, truncated }) + Ok(GlobOutput { + files, + truncated, + partial: !errors.is_empty(), + errors, + }) } #[cfg(test)] @@ -196,4 +218,34 @@ mod tests { } assert!(result.files.iter().any(|f| f.contains('/'))); } + + #[test] + fn glob_output_serializes_partial_metadata() { + let output = GlobOutput { + files: vec!["src/lib.rs".to_string()], + truncated: false, + partial: true, + errors: vec!["walk error: denied".to_string()], + }; + + let json = serde_json::to_value(&output).unwrap(); + + assert_eq!(json["partial"], true); + assert_eq!(json["errors"][0], "walk error: denied"); + } + + #[test] + fn glob_output_omits_partial_metadata_when_not_partial() { + let output = GlobOutput { + files: vec!["src/lib.rs".to_string()], + truncated: false, + partial: false, + errors: Vec::new(), + }; + + let json = serde_json::to_value(&output).unwrap(); + + assert!(json.get("partial").is_none()); + assert!(json.get("errors").is_none()); + } } diff --git a/src/llm-coding-tools-core/src/tools/grep.rs b/src/llm-coding-tools-core/src/tools/grep.rs index bdd358a7..77a327d6 100644 --- a/src/llm-coding-tools-core/src/tools/grep.rs +++ b/src/llm-coding-tools-core/src/tools/grep.rs @@ -47,6 +47,12 @@ pub struct GrepOutput { pub match_count: usize, /// Whether results were truncated due to limit. pub truncated: bool, + /// Whether one or more files could not be searched. + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub partial: bool, + /// Per-file traversal/search errors encountered while collecting matches. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub errors: Vec, } impl GrepOutput { @@ -86,6 +92,14 @@ impl GrepOutput { let _ = write!(&mut output, "\n(Results truncated at {} matches)", limit); } + if self.partial { + let _ = write!( + &mut output, + "\n(Partial results: {} file error(s) encountered)", + self.errors.len() + ); + } + output } } @@ -116,6 +130,7 @@ pub fn grep_search( .build(); let mut files: Vec = Vec::with_capacity(64); + let mut errors: Vec = Vec::with_capacity(8); let walker = WalkBuilder::new(&path) .hidden(false) @@ -127,7 +142,10 @@ pub fn grep_search( for entry_result in walker { let entry = match entry_result { Ok(e) => e, - Err(_) => continue, + Err(err) => { + errors.push(format!("walk error: {err}")); + continue; + } }; // Skip directories and non-regular files. @@ -149,8 +167,13 @@ pub fn grep_search( } } - let matches = collect_file_matches(&matcher, &mut searcher, entry_path); - if matches.is_empty() { + let search_result = collect_file_matches(&matcher, &mut searcher, entry_path); + + if let Some(error) = search_result.error { + errors.push(error); + } + + if search_result.matches.is_empty() { continue; } @@ -162,7 +185,7 @@ pub fn grep_search( files.push(GrepFileMatches { path: entry_path.to_string_lossy().into_owned(), - matches, + matches: search_result.matches, mtime, }); } @@ -193,30 +216,40 @@ pub fn grep_search( files, match_count, truncated, + partial: !errors.is_empty(), + errors, }) } +struct FileSearchResult { + matches: Vec, + error: Option, +} + #[inline] fn collect_file_matches( matcher: &RegexMatcher, searcher: &mut Searcher, path: &Path, -) -> Vec { +) -> FileSearchResult { let mut matches = Vec::new(); - let _ = searcher.search_path( - matcher, - path, - UTF8(|line_num, line| { - matches.push(GrepLineMatch { - line_num, - line_text: line.trim_end().to_string(), - }); - Ok(true) - }), - ); - - matches + let error = searcher + .search_path( + matcher, + path, + UTF8(|line_num, line| { + matches.push(GrepLineMatch { + line_num, + line_text: line.trim_end().to_string(), + }); + Ok(true) + }), + ) + .err() + .map(|err| format!("failed to search '{}': {err}", path.display())); + + FileSearchResult { matches, error } } #[cfg(test)] @@ -257,4 +290,49 @@ mod tests { assert_eq!(result.files.len(), 1); assert!(result.files[0].path.ends_with(".rs")); } + + #[test] + fn grep_format_includes_partial_marker() { + let output = GrepOutput { + files: Vec::new(), + match_count: 0, + truncated: false, + partial: true, + errors: vec!["walk error: denied".to_string()], + }; + + let formatted = output.format::(10, DEFAULT_MAX_LINE_LENGTH); + + assert!(formatted.contains("Partial results")); + } + + #[test] + fn collect_file_matches_reports_error_for_missing_file() { + let temp = tempdir().unwrap(); + let missing = temp.path().join("missing.txt"); + let matcher = RegexMatcher::new("hello").unwrap(); + let mut searcher = SearcherBuilder::new() + .binary_detection(BinaryDetection::quit(0)) + .build(); + + let result = collect_file_matches(&matcher, &mut searcher, &missing); + + assert!(result.matches.is_empty()); + assert!(result.error.is_some()); + } + + #[test] + fn grep_marks_results_partial_when_walker_reports_error() { + let temp = tempdir().unwrap(); + let missing_root = temp.path().join("missing-root"); + let resolver = AbsolutePathResolver; + + let result = + grep_search(&resolver, "hello", None, missing_root.to_str().unwrap(), 10).unwrap(); + + assert!(result.partial); + assert_eq!(result.match_count, 0); + assert!(!result.truncated); + assert!(!result.errors.is_empty()); + } } diff --git a/src/llm-coding-tools-serdesai/src/absolute/glob.rs b/src/llm-coding-tools-serdesai/src/absolute/glob.rs index e2b5c327..5529ef27 100644 --- a/src/llm-coding-tools-serdesai/src/absolute/glob.rs +++ b/src/llm-coding-tools-serdesai/src/absolute/glob.rs @@ -1,13 +1,14 @@ //! Glob pattern file finding tool using [`AbsolutePathResolver`]. use async_trait::async_trait; +use llm_coding_tools_core::ToolContext; use llm_coding_tools_core::path::AbsolutePathResolver; use llm_coding_tools_core::tool_names; use llm_coding_tools_core::tools::glob_files; -use llm_coding_tools_core::{ToolContext, ToolOutput}; use serde::Deserialize; use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult}; +use crate::common::glob::output_to_return as glob_output_to_return; use crate::convert::to_serdes_result; /// Internal args for JSON deserialization. @@ -60,22 +61,10 @@ impl Tool for GlobTool { let resolver = AbsolutePathResolver; let result = glob_files(&resolver, &args.pattern, &args.path); - // Convert GlobOutput to ToolOutput for consistent error handling - to_serdes_result( - tool_names::GLOB, - result.map(|output| { - let content = if output.files.is_empty() { - "No files found matching the pattern.".to_string() - } else { - output.files.join("\n") - }; - if output.truncated { - ToolOutput::truncated(content) - } else { - ToolOutput::new(content) - } - }), - ) + match result { + Err(e) => to_serdes_result(tool_names::GLOB, Err(e)), + Ok(output) => Ok(glob_output_to_return(output)), + } } } @@ -90,6 +79,7 @@ impl ToolContext for GlobTool { #[cfg(test)] mod tests { use super::*; + use llm_coding_tools_core::tools::GlobOutput; use serde_json::json; use serdes_ai::tools::RunContext; use std::fs::{self, File}; @@ -122,4 +112,18 @@ mod tests { assert!(text.contains("lib.rs")); assert!(text.contains("main.rs")); } + + #[test] + fn partial_glob_output_returns_json_payload() { + let payload = glob_output_to_return(GlobOutput { + files: vec!["src/lib.rs".to_string()], + truncated: false, + partial: true, + errors: vec!["walk error: denied".to_string()], + }); + + let json = payload.as_json().unwrap(); + assert_eq!(json["partial"], true); + assert_eq!(json["errors"][0], "walk error: denied"); + } } diff --git a/src/llm-coding-tools-serdesai/src/absolute/grep.rs b/src/llm-coding-tools-serdesai/src/absolute/grep.rs index bbdbd1cd..bfad6223 100644 --- a/src/llm-coding-tools-serdesai/src/absolute/grep.rs +++ b/src/llm-coding-tools-serdesai/src/absolute/grep.rs @@ -6,10 +6,9 @@ use llm_coding_tools_core::path::AbsolutePathResolver; use llm_coding_tools_core::tool_names; use llm_coding_tools_core::tools::{DEFAULT_MAX_LINE_LENGTH, grep_search}; use serde::Deserialize; -use serdes_ai::tools::{ - RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult, ToolReturn, -}; +use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult}; +use crate::common::grep::output_to_return as grep_output_to_return; use crate::convert::to_serdes_result; const DEFAULT_LIMIT: usize = 100; @@ -115,14 +114,11 @@ impl Tool for GrepTool to_serdes_result(tool_names::GREP, Err(e)), - Ok(grep_output) => { - if grep_output.files.is_empty() { - return Ok(ToolReturn::text("No matches found.")); - } - - let output = grep_output.format::(limit, DEFAULT_MAX_LINE_LENGTH); - Ok(ToolReturn::text(output)) - } + Ok(grep_output) => Ok(grep_output_to_return::( + grep_output, + limit, + DEFAULT_MAX_LINE_LENGTH, + )), } } } @@ -247,6 +243,34 @@ mod tests { assert!(!text.contains("readme.txt")); } + #[tokio::test] + async fn returns_partial_json_when_search_has_errors() { + let dir = TempDir::new().unwrap(); + let missing_path = dir.path().join("missing-root"); + let tool: GrepTool = GrepTool::new(); + + let result = tool + .call( + &mock_ctx(), + json!({ + "pattern": "hello", + "path": missing_path.to_string_lossy() + }), + ) + .await + .unwrap(); + + let payload = result.as_json().unwrap(); + assert_eq!(payload["partial"], true); + assert!(!payload["errors"].as_array().unwrap().is_empty()); + assert!( + payload["content"] + .as_str() + .unwrap() + .contains("Partial results") + ); + } + #[tokio::test] async fn truncates_long_lines_at_max_length() { let dir = TempDir::new().unwrap(); diff --git a/src/llm-coding-tools-serdesai/src/allowed/glob.rs b/src/llm-coding-tools-serdesai/src/allowed/glob.rs index 0ff5a4c6..4c018ce5 100644 --- a/src/llm-coding-tools-serdesai/src/allowed/glob.rs +++ b/src/llm-coding-tools-serdesai/src/allowed/glob.rs @@ -1,13 +1,14 @@ //! Glob pattern file finding tool using [`AllowedPathResolver`]. use async_trait::async_trait; +use llm_coding_tools_core::ToolContext; use llm_coding_tools_core::path::AllowedPathResolver; use llm_coding_tools_core::tool_names; use llm_coding_tools_core::tools::glob_files; -use llm_coding_tools_core::{ToolContext, ToolOutput}; use serde::Deserialize; use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult}; +use crate::common::glob::output_to_return as glob_output_to_return; use crate::convert::to_serdes_result; /// Internal args for JSON deserialization. @@ -66,21 +67,10 @@ impl Tool for GlobTool { .map_err(|e| ToolError::validation_error(tool_names::GLOB, None, e.to_string()))?; let result = glob_files(&self.resolver, &args.pattern, &args.path); - to_serdes_result( - tool_names::GLOB, - result.map(|output| { - let content = if output.files.is_empty() { - "No files found matching the pattern.".to_string() - } else { - output.files.join("\n") - }; - if output.truncated { - ToolOutput::truncated(content) - } else { - ToolOutput::new(content) - } - }), - ) + match result { + Err(e) => to_serdes_result(tool_names::GLOB, Err(e)), + Ok(output) => Ok(glob_output_to_return(output)), + } } } @@ -95,6 +85,7 @@ impl ToolContext for GlobTool { #[cfg(test)] mod tests { use super::*; + use llm_coding_tools_core::tools::GlobOutput; use serde_json::json; use serdes_ai::tools::RunContext; use std::fs::{self, File}; @@ -144,4 +135,18 @@ mod tests { assert!(result.is_err()); } + + #[test] + fn partial_glob_output_returns_json_payload() { + let payload = glob_output_to_return(GlobOutput { + files: vec!["src/lib.rs".to_string()], + truncated: false, + partial: true, + errors: vec!["walk error: denied".to_string()], + }); + + let json = payload.as_json().unwrap(); + assert_eq!(json["partial"], true); + assert_eq!(json["errors"][0], "walk error: denied"); + } } diff --git a/src/llm-coding-tools-serdesai/src/allowed/grep.rs b/src/llm-coding-tools-serdesai/src/allowed/grep.rs index f63becc4..85c91ab9 100644 --- a/src/llm-coding-tools-serdesai/src/allowed/grep.rs +++ b/src/llm-coding-tools-serdesai/src/allowed/grep.rs @@ -6,10 +6,9 @@ use llm_coding_tools_core::path::AllowedPathResolver; use llm_coding_tools_core::tool_names; use llm_coding_tools_core::tools::{DEFAULT_MAX_LINE_LENGTH, grep_search}; use serde::Deserialize; -use serdes_ai::tools::{ - RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult, ToolReturn, -}; +use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult}; +use crate::common::grep::output_to_return as grep_output_to_return; use crate::convert::to_serdes_result; const DEFAULT_LIMIT: usize = 100; @@ -121,14 +120,11 @@ impl Tool for GrepTool to_serdes_result(tool_names::GREP, Err(e)), - Ok(grep_output) => { - if grep_output.files.is_empty() { - return Ok(ToolReturn::text("No matches found.")); - } - - let output = grep_output.format::(limit, DEFAULT_MAX_LINE_LENGTH); - Ok(ToolReturn::text(output)) - } + Ok(grep_output) => Ok(grep_output_to_return::( + grep_output, + limit, + DEFAULT_MAX_LINE_LENGTH, + )), } } } @@ -210,4 +206,31 @@ mod tests { assert!(result.is_err()); } + + #[tokio::test] + async fn returns_partial_json_when_search_has_errors() { + let dir = TempDir::new().unwrap(); + let resolver = AllowedPathResolver::new([dir.path()]).unwrap(); + let tool: GrepTool = GrepTool::new(resolver); + let result = tool + .call( + &mock_ctx(), + json!({ + "pattern": "hello", + "path": "missing-root" + }), + ) + .await + .unwrap(); + + let payload = result.as_json().unwrap(); + assert_eq!(payload["partial"], true); + assert!(!payload["errors"].as_array().unwrap().is_empty()); + assert!( + payload["content"] + .as_str() + .unwrap() + .contains("Partial results") + ); + } } diff --git a/src/llm-coding-tools-serdesai/src/common/glob.rs b/src/llm-coding-tools-serdesai/src/common/glob.rs new file mode 100644 index 00000000..947f95a5 --- /dev/null +++ b/src/llm-coding-tools-serdesai/src/common/glob.rs @@ -0,0 +1,39 @@ +//! Shared helpers for Glob tool implementations. + +use llm_coding_tools_core::tools::GlobOutput; +use serde_json::json; +use serdes_ai::tools::ToolReturn; + +const NO_FILES_FOUND: &str = "No files found matching the pattern."; + +#[inline] +fn output_content(files: &[String]) -> String { + if files.is_empty() { + NO_FILES_FOUND.to_string() + } else { + files.join("\n") + } +} + +#[inline] +pub(crate) fn output_to_return(output: GlobOutput) -> ToolReturn { + let content = output_content(&output.files); + + if output.partial { + return ToolReturn::json(json!({ + "content": content, + "partial": true, + "errors": output.errors, + "truncated": output.truncated, + })); + } + + if output.truncated { + ToolReturn::json(json!({ + "content": content, + "truncated": true, + })) + } else { + ToolReturn::text(content) + } +} diff --git a/src/llm-coding-tools-serdesai/src/common/grep.rs b/src/llm-coding-tools-serdesai/src/common/grep.rs new file mode 100644 index 00000000..224887e7 --- /dev/null +++ b/src/llm-coding-tools-serdesai/src/common/grep.rs @@ -0,0 +1,31 @@ +//! Shared helpers for Grep tool implementations. + +use llm_coding_tools_core::tools::GrepOutput; +use serde_json::json; +use serdes_ai::tools::ToolReturn; + +const NO_MATCHES_FOUND: &str = "No matches found."; + +#[inline] +pub(crate) fn output_to_return( + output: GrepOutput, + limit: usize, + max_line_len: usize, +) -> ToolReturn { + if output.partial { + let content = output.format::(limit, max_line_len); + return ToolReturn::json(json!({ + "content": content, + "partial": true, + "errors": output.errors, + "match_count": output.match_count, + "truncated": output.truncated, + })); + } + + if output.files.is_empty() { + return ToolReturn::text(NO_MATCHES_FOUND); + } + + ToolReturn::text(output.format::(limit, max_line_len)) +} diff --git a/src/llm-coding-tools-serdesai/src/common/mod.rs b/src/llm-coding-tools-serdesai/src/common/mod.rs index 88713424..66a01a13 100644 --- a/src/llm-coding-tools-serdesai/src/common/mod.rs +++ b/src/llm-coding-tools-serdesai/src/common/mod.rs @@ -1,3 +1,5 @@ //! Shared helpers for tool implementations. pub(crate) mod edit; +pub(crate) mod glob; +pub(crate) mod grep;