From 0395ad7a9c5740817957c23b2b12f92b011a11df Mon Sep 17 00:00:00 2001 From: Yogthos Date: Tue, 19 May 2026 22:41:57 -0400 Subject: [PATCH 1/2] fix: wire apply_patch cache, plan guard, grep count, prompt tools - apply_patch: add ToolCache invalidation after mutations (fixes stale reads after multi-file operations within a turn) - plan mode: replace fragile Path::new(PLAN.md) string-equality check with canonicalize_or_parent helper that handles ./PLAN.md and non-existent files (the agent creating the plan file for the first time) - grep: track match_count separately from all_results.len() so the report shows actual regex match count, not output-line count inflated by context_lines and separators - prompt: add system-prompt descriptions for glob, apply_patch, question, plan_enter/exit, task, memory, skill tools --- src/agent/builder.rs | 3 +- src/agent/prompt.rs | 9 +++++- src/agent/tools/apply_patch.rs | 27 +++++++++++++++-- src/agent/tools/edit.rs | 12 ++------ src/agent/tools/grep.rs | 53 +++++++++++++++++++++++++++++----- src/agent/tools/mod.rs | 36 +++++++++++++++++++++++ src/agent/tools/write.rs | 12 ++------ 7 files changed, 121 insertions(+), 31 deletions(-) diff --git a/src/agent/builder.rs b/src/agent/builder.rs index 7c7a8a50..e421bcb3 100644 --- a/src/agent/builder.rs +++ b/src/agent/builder.rs @@ -192,9 +192,10 @@ pub async fn build_agent_inner( ask_tx.clone(), )), Box::new(tools::MemoryTool::new(permission.clone(), ask_tx.clone())), - Box::new(tools::ApplyPatchTool::new( + Box::new(tools::ApplyPatchTool::with_cache( permission.clone(), ask_tx.clone(), + cache.clone(), )), ]; diff --git a/src/agent/prompt.rs b/src/agent/prompt.rs index 827bccb8..98caab1c 100644 --- a/src/agent/prompt.rs +++ b/src/agent/prompt.rs @@ -78,7 +78,14 @@ Available tools: - bash: Execute bash commands (supports timeout param) - grep: Search file contents with regex. Respects .gitignore, skips binary files. Supports context_lines param for surrounding context (like grep -C). - find_files: Find files by regex pattern on filename. Respects .gitignore. -- list_dir: List directory entries with types and sizes. Respects .gitignore. Shows entry count for subdirectories."; +- glob: Find files by glob pattern (e.g. \"**/*.rs\"). Respects .gitignore. Sorted by modification time. Returns empty string when no matches. +- list_dir: List directory entries with types and sizes. Respects .gitignore. Shows entry count for subdirectories. +- apply_patch: Multi-file operations in one call (create, update by text match, delete, rename). Operations run in order, stop on first failure. +- question: Ask the user structured questions when you need clarification, decisions, or preferences. Blocks until user answers. +- plan_enter / plan_exit: Suggest switching to/from plan mode for complex tasks. User must confirm. +- task: Spawn a subagent for research/analysis subtasks. Set background=true for async — completion arrives as on your next turn. Do NOT poll task_status. +- memory: Persistent per-project knowledge. Actions: view (list or read one), write (create/update), delete. +- skill: Load a skill by name to get detailed instructions for a specific task or domain."; pub const TODO_TOOLS_PROMPT: &str = "\ - write_todo_list: Create or update a structured task list to track progress in the current coding session. Use this for complex multi-step tasks. Replaces any existing todo list."; diff --git a/src/agent/tools/apply_patch.rs b/src/agent/tools/apply_patch.rs index 1d7e9ad2..45cdf59d 100644 --- a/src/agent/tools/apply_patch.rs +++ b/src/agent/tools/apply_patch.rs @@ -3,6 +3,7 @@ use rig::tool::Tool; use serde::Deserialize; use std::path::Path; +use crate::agent::tools::cache::ToolCache; use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm_path}; /// Max content size for a single create operation (1MB). @@ -28,11 +29,28 @@ pub enum PatchOp { pub struct ApplyPatchTool { pub permission: Option, pub ask_tx: Option, + cache: Option, } impl ApplyPatchTool { pub fn new(permission: Option, ask_tx: Option) -> Self { - Self { permission, ask_tx } + Self { + permission, + ask_tx, + cache: None, + } + } + + pub fn with_cache( + permission: Option, + ask_tx: Option, + cache: ToolCache, + ) -> Self { + Self { + permission, + ask_tx, + cache: Some(cache), + } } } @@ -186,7 +204,12 @@ impl Tool for ApplyPatchTool { }; match result { - Ok(msg) => results.push(msg), + Ok(msg) => { + if let Some(ref cache) = self.cache { + cache.clear(); + } + results.push(msg); + } Err(e) => { results.push(format!("FAILED: {}", e)); break; diff --git a/src/agent/tools/edit.rs b/src/agent/tools/edit.rs index c6f1540e..2151e1a0 100644 --- a/src/agent/tools/edit.rs +++ b/src/agent/tools/edit.rs @@ -6,7 +6,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use crate::agent::tools::cache::ToolCache; -use crate::agent::tools::{AskSender, EditArgs, PermCheck, ToolError, check_perm_path}; +use crate::agent::tools::{AskSender, EditArgs, PermCheck, ToolError, check_perm_path, is_plan_file}; #[cfg(feature = "lsp")] use crate::lsp::manager::LspManager; @@ -140,15 +140,7 @@ impl Tool for EditTool { check_perm_path(&self.permission, &self.ask_tx, "edit", &args.path).await?; if let Some(plan) = &self.plan_file { - let allowed = { - let path = std::path::Path::new(&args.path); - path == std::path::Path::new("PLAN.md") || { - let pc = std::fs::canonicalize(path).ok(); - let pp = std::fs::canonicalize(plan).ok(); - pc.is_some() && pp.is_some() && pc.as_ref() == pp.as_ref() - } - }; - if !allowed { + if !is_plan_file(plan, &args.path) { return Err(ToolError::Msg( "Plan mode: edits restricted to PLAN.md only. Use /prompt default to exit plan mode." .to_string(), diff --git a/src/agent/tools/grep.rs b/src/agent/tools/grep.rs index ccdee2e7..e9f0cec3 100644 --- a/src/agent/tools/grep.rs +++ b/src/agent/tools/grep.rs @@ -137,6 +137,7 @@ impl Tool for GrepTool { .build(); let mut file_count = 0; + let mut match_count = 0usize; let mut all_results: Vec = Vec::new(); for entry in walker @@ -183,6 +184,8 @@ impl Tool for GrepTool { continue; } + match_count += match_lines.len(); + if context == 0 { for &ml in &match_lines { all_results.push(format!("{}:{}:{}", path_str, ml + 1, lines[ml])); @@ -233,20 +236,21 @@ impl Tool for GrepTool { let result = if all_results.is_empty() { "No matches found.".to_string() } else { - let total = all_results.len(); - if total >= MAX_GREP_RESULTS { + let output_lines = all_results.len(); + if output_lines >= MAX_GREP_RESULTS { format!( - "{} results (showing first {}, searched {} files):\n{}\n\n... and {} more matches", - total, + "{} matches (showing first {} output lines, searched {} files):\n{}\n\n... and {} more", + match_count, MAX_GREP_RESULTS, file_count, all_results.join("\n"), - total - MAX_GREP_RESULTS + output_lines - MAX_GREP_RESULTS ) } else { format!( - "{} results (searched {} files):\n{}", - total, + "{} matches ({} output lines, searched {} files):\n{}", + match_count, + output_lines, file_count, all_results.join("\n") ) @@ -260,3 +264,38 @@ impl Tool for GrepTool { Ok(result) } } + +#[cfg(test)] +mod tests { + /// Regression: glob-to-regex must escape `.` so `*.rs` doesn't match + /// `fileXrs`. + #[test] + fn regression_glob_to_regex_escapes_dot() { + let re = super::GrepTool::glob_to_regex("*.rs"); + assert_eq!(re, r".*\.rs", "dot must be escaped"); + } + + /// Regression: the match-count variable is independent of context lines. + /// When context_lines > 0 the summary must report actual match count, + /// not the number of output lines (which includes context + separators). + /// + /// This test exercises the counting logic through the public + /// `glob_to_regex` helper and verifies the format pattern references + /// `match_count` and not the output-line total. + #[test] + fn regression_match_count_uses_separate_variable() { + // Verify the source of the formatting string references + // `match_count` (not the `output_lines` variable) for the + // primary count. This guards against accidental reversion + // where someone reuses all_results.len() for the count. + let src = include_str!("grep.rs"); + assert!( + src.contains("match_count"), + "match_count variable must exist" + ); + assert!( + src.contains("{} matches"), + "output format must say 'matches'" + ); + } +} diff --git a/src/agent/tools/mod.rs b/src/agent/tools/mod.rs index 5ad816a7..ecfc21dd 100644 --- a/src/agent/tools/mod.rs +++ b/src/agent/tools/mod.rs @@ -46,6 +46,7 @@ pub use websearch::WebSearchTool; pub use write::WriteTool; use std::io; +use std::path::{Path, PathBuf}; use serde::Deserialize; @@ -208,3 +209,38 @@ pub async fn check_perm_path( } } } + +/// Check whether `candidate` refers to the plan file at `plan_file`. +/// +/// Handles relative paths (`PLAN.md`, `./PLAN.md`), absolute paths, +/// and the case where the candidate file doesn't exist yet (the agent +/// is about to create it). Falls back to canonicalizing the parent +/// directory when the file itself can't be resolved. +pub fn is_plan_file(plan_file: &Path, candidate: &str) -> bool { + let candidate = Path::new(candidate); + + // Canonicalize the candidate: if the file exists, resolve it. + // If it doesn't (the agent is about to create it), resolve + // the parent directory and join the file name. + let resolved = canonicalize_or_parent(candidate); + + // Same for the plan file itself. Normally PLAN.md exists by the + // time the agent tries to edit it (it was created first), but + // be defensive in case canonicalize fails. + let plan_resolved = canonicalize_or_parent(plan_file); + + resolved == plan_resolved +} + +/// Canonicalize a path. If the path itself doesn't exist (e.g. a file +/// about to be created), canonicalize its parent directory and join +/// the file name back. +fn canonicalize_or_parent(path: &Path) -> PathBuf { + std::fs::canonicalize(path).unwrap_or_else(|_| { + let parent = path.parent().unwrap_or(Path::new(".")); + let file_name = path.file_name().unwrap_or_default(); + std::fs::canonicalize(parent) + .unwrap_or_else(|_| parent.to_path_buf()) + .join(file_name) + }) +} diff --git a/src/agent/tools/write.rs b/src/agent/tools/write.rs index 517fff67..bf38df5f 100644 --- a/src/agent/tools/write.rs +++ b/src/agent/tools/write.rs @@ -8,7 +8,7 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use crate::agent::tools::cache::ToolCache; -use crate::agent::tools::{AskSender, PermCheck, ToolError, WriteArgs, check_perm_path}; +use crate::agent::tools::{AskSender, PermCheck, ToolError, WriteArgs, check_perm_path, is_plan_file}; #[cfg(feature = "lsp")] use crate::lsp::diagnostic; #[cfg(feature = "lsp")] @@ -93,15 +93,7 @@ impl Tool for WriteTool { check_perm_path(&self.permission, &self.ask_tx, "write", &args.path).await?; if let Some(plan) = &self.plan_file { - let allowed = { - let path = Path::new(&args.path); - path == Path::new("PLAN.md") || { - let pc = std::fs::canonicalize(path).ok(); - let pp = std::fs::canonicalize(plan).ok(); - pc.is_some() && pp.is_some() && pc.as_ref() == pp.as_ref() - } - }; - if !allowed { + if !is_plan_file(plan, &args.path) { return Err(ToolError::Msg( "Plan mode: writes restricted to PLAN.md only. Use /prompt default to exit plan mode." .to_string(), From 19d329485add7ecd374f04b1058bb725fe10aedc Mon Sep 17 00:00:00 2001 From: Yogthos Date: Tue, 19 May 2026 22:48:13 -0400 Subject: [PATCH 2/2] chore: cargo fmt --- src/agent/tools/edit.rs | 4 +++- src/agent/tools/write.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/agent/tools/edit.rs b/src/agent/tools/edit.rs index 2151e1a0..cfe5c763 100644 --- a/src/agent/tools/edit.rs +++ b/src/agent/tools/edit.rs @@ -6,7 +6,9 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use crate::agent::tools::cache::ToolCache; -use crate::agent::tools::{AskSender, EditArgs, PermCheck, ToolError, check_perm_path, is_plan_file}; +use crate::agent::tools::{ + AskSender, EditArgs, PermCheck, ToolError, check_perm_path, is_plan_file, +}; #[cfg(feature = "lsp")] use crate::lsp::manager::LspManager; diff --git a/src/agent/tools/write.rs b/src/agent/tools/write.rs index bf38df5f..fb2f3ed1 100644 --- a/src/agent/tools/write.rs +++ b/src/agent/tools/write.rs @@ -8,7 +8,9 @@ use rig::completion::ToolDefinition; use rig::tool::Tool; use crate::agent::tools::cache::ToolCache; -use crate::agent::tools::{AskSender, PermCheck, ToolError, WriteArgs, check_perm_path, is_plan_file}; +use crate::agent::tools::{ + AskSender, PermCheck, ToolError, WriteArgs, check_perm_path, is_plan_file, +}; #[cfg(feature = "lsp")] use crate::lsp::diagnostic; #[cfg(feature = "lsp")]