diff --git a/crates/tui/src/tools/apply_patch.rs b/crates/tui/src/tools/apply_patch.rs index ba824525..fd37d831 100644 --- a/crates/tui/src/tools/apply_patch.rs +++ b/crates/tui/src/tools/apply_patch.rs @@ -14,7 +14,7 @@ use thiserror::Error; use super::spec::{ ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, - optional_bool, optional_str, optional_u64, required_str, + lsp_diagnostics_for_paths, optional_bool, optional_str, optional_u64, required_str, }; /// Maximum lines of context for fuzzy matching (increased for better tolerance) @@ -216,6 +216,9 @@ impl ToolSpec for ApplyPatchTool { if let Some(changes_value) = input.get("changes") { let (pending, stats) = build_pending_writes_from_changes(changes_value, context)?; apply_pending_writes(&pending)?; + // Resolve absolute paths for LSP diagnostics query. + let abs_paths: Vec = pending.iter().map(|p| p.path.clone()).collect(); + let diag_block = lsp_diagnostics_for_paths(context, &abs_paths).await; let result = PatchResult { success: true, files_applied: stats.stats.files_applied, @@ -228,8 +231,13 @@ impl ToolSpec for ApplyPatchTool { file_summaries: stats.file_summaries.clone(), message: build_summary_message(&stats), }; - return ToolResult::json(&result) - .map_err(|e| ToolError::execution_failed(e.to_string())); + let mut tool_result = ToolResult::json(&result) + .map_err(|e| ToolError::execution_failed(e.to_string()))?; + if !diag_block.is_empty() { + tool_result.content.push('\n'); + tool_result.content.push_str(&diag_block); + } + return Ok(tool_result); } let patch_text = required_str(&input, "patch")?; @@ -265,6 +273,13 @@ impl ToolSpec for ApplyPatchTool { stats.header_path_mismatch = mismatch_note; } apply_pending_writes(&pending)?; + // Resolve absolute paths for LSP diagnostics query. + let abs_paths: Vec = pending + .iter() + .filter(|p| p.content.is_some()) // skip deleted files + .map(|p| p.path.clone()) + .collect(); + let diag_block = lsp_diagnostics_for_paths(context, &abs_paths).await; let result = PatchResult { success: true, files_applied: stats.stats.files_applied, @@ -277,8 +292,13 @@ impl ToolSpec for ApplyPatchTool { file_summaries: stats.file_summaries.clone(), message: build_summary_message(&stats), }; - - ToolResult::json(&result).map_err(|e| ToolError::execution_failed(e.to_string())) + let mut tool_result = + ToolResult::json(&result).map_err(|e| ToolError::execution_failed(e.to_string()))?; + if !diag_block.is_empty() { + tool_result.content.push('\n'); + tool_result.content.push_str(&diag_block); + } + Ok(tool_result) } } diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index 1218a26c..31b62338 100644 --- a/crates/tui/src/tools/file.rs +++ b/crates/tui/src/tools/file.rs @@ -6,7 +6,7 @@ use super::diff_format::make_unified_diff; use super::spec::{ ApprovalRequirement, ToolCapability, ToolContext, ToolError, ToolResult, ToolSpec, - optional_str, required_str, + lsp_diagnostics_for_paths, optional_str, required_str, }; use async_trait::async_trait; use serde_json::{Value, json}; @@ -266,7 +266,15 @@ impl ToolSpec for WriteFileTool { format!("{diff}\n{summary}") }; - Ok(ToolResult::success(body)) + // Append LSP diagnostics for the written file when enabled (#428). + let diag_block = lsp_diagnostics_for_paths(context, &[file_path]).await; + let full_body = if diag_block.is_empty() { + body + } else { + format!("{body}\n{diag_block}") + }; + + Ok(ToolResult::success(full_body)) } } @@ -352,7 +360,15 @@ impl ToolSpec for EditFileTool { format!("{diff}\n{summary}") }; - Ok(ToolResult::success(body)) + // Append LSP diagnostics for the edited file when enabled (#428). + let diag_block = lsp_diagnostics_for_paths(context, &[file_path]).await; + let full_body = if diag_block.is_empty() { + body + } else { + format!("{body}\n{diag_block}") + }; + + Ok(ToolResult::success(full_body)) } } diff --git a/crates/tui/src/tools/spec.rs b/crates/tui/src/tools/spec.rs index 55836a1e..408f407b 100644 --- a/crates/tui/src/tools/spec.rs +++ b/crates/tui/src/tools/spec.rs @@ -7,12 +7,14 @@ //! - `ToolCapability`: Capabilities and requirements of tools use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; use async_trait::async_trait; use serde_json::Value; use tokio_util::sync::CancellationToken; use crate::features::Features; +use crate::lsp::LspManager; use crate::network_policy::NetworkPolicyDecider; use crate::tools::shell::{SharedShellManager, new_shared_shell_manager}; #[allow(unused_imports)] @@ -110,6 +112,11 @@ pub struct ToolContext { /// short-circuit on `None` rather than fall back to a workspace-local /// default. pub memory_path: Option, + /// LSP manager for post-edit diagnostics injection (#428). `None` when + /// LSP is disabled or the context is constructed in a test that does not + /// need diagnostics. Edit tools append a `` block to their + /// result when this is present and the manager is enabled. + pub lsp_manager: Option>, } impl ToolContext { @@ -136,6 +143,7 @@ impl ToolContext { runtime: RuntimeToolServices::default(), cancel_token: None, memory_path: None, + lsp_manager: None, } } @@ -165,6 +173,7 @@ impl ToolContext { runtime: RuntimeToolServices::default(), cancel_token: None, memory_path: None, + lsp_manager: None, } } @@ -194,6 +203,7 @@ impl ToolContext { runtime: RuntimeToolServices::default(), cancel_token: None, memory_path: None, + lsp_manager: None, } } @@ -227,6 +237,15 @@ impl ToolContext { self } + /// Attach an LSP manager so that edit tools can auto-inject diagnostics + /// into their results after a successful file modification (#428). + #[must_use] + #[allow(dead_code)] + pub fn with_lsp_manager(mut self, manager: Arc) -> Self { + self.lsp_manager = Some(manager); + self + } + /// Resolve a path relative to workspace, validating it doesn't escape. /// /// This handles both existing files (using canonicalize) and non-existent files @@ -401,6 +420,35 @@ impl ToolContext { } } +/// Gather LSP diagnostics for `paths` using the manager stored in `context`, +/// and return the rendered `` blocks joined by newlines. +/// +/// Returns an empty string when: +/// - `context.lsp_manager` is `None` +/// - the manager's `enabled` flag is `false` +/// - none of the files produce diagnostics (e.g. all clean, or language unknown) +/// +/// This function is non-blocking by design: every failure mode (missing LSP +/// binary, timeout, unknown language) degrades to an empty string rather than +/// propagating an error to the caller. +pub async fn lsp_diagnostics_for_paths(context: &ToolContext, paths: &[PathBuf]) -> String { + use crate::lsp::render_blocks; + + let manager = match context.lsp_manager.as_ref() { + Some(m) if m.config().enabled => m, + _ => return String::new(), + }; + + let mut blocks = Vec::new(); + for (idx, path) in paths.iter().enumerate() { + if let Some(block) = manager.diagnostics_for(path, idx as u64).await { + blocks.push(block); + } + } + + render_blocks(&blocks) +} + fn normalize_path(path: &Path) -> PathBuf { let mut prefix: Option = None; let mut is_root = false;