feat(lsp): auto-attach diagnostics to edit results (closes #428)
After every successful apply_patch / write_file / edit_file the engine queries LspManager for diagnostics on touched files and appends a <diagnostics file="…"> block to the tool result. - ToolContext gains optional lsp_manager field + with_lsp_manager() builder - lsp_diagnostics_for_paths() helper handles async fan-out and rendering - All three edit tools append the block when non-empty - Gated behind existing [lsp] enabled config flag; silent fallback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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<PathBuf> = 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<PathBuf> = 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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<PathBuf>,
|
||||
/// 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 `<diagnostics>` block to their
|
||||
/// result when this is present and the manager is enabled.
|
||||
pub lsp_manager: Option<Arc<LspManager>>,
|
||||
}
|
||||
|
||||
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<LspManager>) -> 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 `<diagnostics …>` 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<std::ffi::OsString> = None;
|
||||
let mut is_root = false;
|
||||
|
||||
Reference in New Issue
Block a user