PR #46306 changed cancellation to wait for tools to complete before returning. This was correct behavior - it allows tools like terminal to capture their output on cancellation. The real issue was that many tools didn't check for cancellation, so they would continue running until they finished. ## The Problem When the user pressed Escape to cancel during a tool operation, tools would continue running because they never checked for the cancellation signal. The thread correctly waited for tools to complete (so terminal could capture output), but tools like edit_file, grep, fetch, etc. would just keep going. ## The Fix Add cancellation handling to all tools using the same pattern as `terminal_tool`: use `select!` to race between the tool's main work and `event_stream.cancelled_by_user()`. When cancelled, tools break out of their loops or return early. ## All Tools Now Cancellation-Aware | Tool | Change | |------|--------| | `edit_file_tool` | Checks cancellation in edit event processing loop | | `terminal_tool` | Already handled cancellation | | `grep_tool` | Checks cancellation in search result iteration loop | | `fetch_tool` | Checks cancellation during HTTP fetch | | `web_search_tool` | Checks cancellation during web search | | `find_path_tool` | Checks cancellation during path search | | `read_file_tool` | Checks cancellation during buffer open | | `copy_path_tool` | Checks cancellation during file copy | | `move_path_tool` | Checks cancellation during file move/rename | | `delete_path_tool` | Checks cancellation during delete operation | | `create_directory_tool` | Checks cancellation during directory creation | | `save_file_tool` | Checks cancellation during buffer open and save | | `restore_file_from_disk_tool` | Checks cancellation during buffer open and reload | | `open_tool` | Checks cancellation during authorization | | `diagnostics_tool` | Checks cancellation during buffer open | | `ContextServerTool` (MCP) | Checks cancellation during external server calls | **Synchronous tools (no async work, return immediately):** - `list_directory_tool` - Reads worktree snapshot synchronously - `now_tool` - Returns current time immediately - `thinking_tool` - Returns immediately ## MCP Tools Automatically Handled MCP tools (user-defined tools via context servers) are now automatically cancellation-aware without any user action. The `ContextServerTool` wrapper races the external server request against `event_stream.cancelled_by_user()`. ## Testing - Added `CancellationAwareTool` test helper that mirrors the cancellation pattern - Updated `test_cancellation_aware_tool_responds_to_cancellation` to properly await the cancel task and verify the tool detected cancellation Release Notes: - Fixed a regression where pressing Escape wouldn't immediately cancel in-progress tool operations
172 lines
5.9 KiB
Rust
172 lines
5.9 KiB
Rust
use crate::{AgentTool, ToolCallEventStream};
|
|
use agent_client_protocol as acp;
|
|
use anyhow::{Result, anyhow};
|
|
use futures::FutureExt as _;
|
|
use gpui::{App, Entity, Task};
|
|
use language::{DiagnosticSeverity, OffsetRangeExt};
|
|
use project::Project;
|
|
use schemars::JsonSchema;
|
|
use serde::{Deserialize, Serialize};
|
|
use std::{fmt::Write, sync::Arc};
|
|
use ui::SharedString;
|
|
use util::markdown::MarkdownInlineCode;
|
|
|
|
/// Get errors and warnings for the project or a specific file.
|
|
///
|
|
/// This tool can be invoked after a series of edits to determine if further edits are necessary, or if the user asks to fix errors or warnings in their codebase.
|
|
///
|
|
/// When a path is provided, shows all diagnostics for that specific file.
|
|
/// When no path is provided, shows a summary of error and warning counts for all files in the project.
|
|
///
|
|
/// <example>
|
|
/// To get diagnostics for a specific file:
|
|
/// {
|
|
/// "path": "src/main.rs"
|
|
/// }
|
|
///
|
|
/// To get a project-wide diagnostic summary:
|
|
/// {}
|
|
/// </example>
|
|
///
|
|
/// <guidelines>
|
|
/// - If you think you can fix a diagnostic, make 1-2 attempts and then give up.
|
|
/// - Don't remove code you've generated just because you can't fix an error. The user can help you fix it.
|
|
/// </guidelines>
|
|
#[derive(Debug, Serialize, Deserialize, JsonSchema)]
|
|
pub struct DiagnosticsToolInput {
|
|
/// The path to get diagnostics for. If not provided, returns a project-wide summary.
|
|
///
|
|
/// This path should never be absolute, and the first component
|
|
/// of the path should always be a root directory in a project.
|
|
///
|
|
/// <example>
|
|
/// If the project has the following root directories:
|
|
///
|
|
/// - lorem
|
|
/// - ipsum
|
|
///
|
|
/// If you wanna access diagnostics for `dolor.txt` in `ipsum`, you should use the path `ipsum/dolor.txt`.
|
|
/// </example>
|
|
pub path: Option<String>,
|
|
}
|
|
|
|
pub struct DiagnosticsTool {
|
|
project: Entity<Project>,
|
|
}
|
|
|
|
impl DiagnosticsTool {
|
|
pub fn new(project: Entity<Project>) -> Self {
|
|
Self { project }
|
|
}
|
|
}
|
|
|
|
impl AgentTool for DiagnosticsTool {
|
|
type Input = DiagnosticsToolInput;
|
|
type Output = String;
|
|
|
|
fn name() -> &'static str {
|
|
"diagnostics"
|
|
}
|
|
|
|
fn kind() -> acp::ToolKind {
|
|
acp::ToolKind::Read
|
|
}
|
|
|
|
fn initial_title(
|
|
&self,
|
|
input: Result<Self::Input, serde_json::Value>,
|
|
_cx: &mut App,
|
|
) -> SharedString {
|
|
if let Some(path) = input.ok().and_then(|input| match input.path {
|
|
Some(path) if !path.is_empty() => Some(path),
|
|
_ => None,
|
|
}) {
|
|
format!("Check diagnostics for {}", MarkdownInlineCode(&path)).into()
|
|
} else {
|
|
"Check project diagnostics".into()
|
|
}
|
|
}
|
|
|
|
fn run(
|
|
self: Arc<Self>,
|
|
input: Self::Input,
|
|
event_stream: ToolCallEventStream,
|
|
cx: &mut App,
|
|
) -> Task<Result<Self::Output>> {
|
|
match input.path {
|
|
Some(path) if !path.is_empty() => {
|
|
let Some(project_path) = self.project.read(cx).find_project_path(&path, cx) else {
|
|
return Task::ready(Err(anyhow!("Could not find path {path} in project",)));
|
|
};
|
|
|
|
let open_buffer_task = self
|
|
.project
|
|
.update(cx, |project, cx| project.open_buffer(project_path, cx));
|
|
|
|
cx.spawn(async move |cx| {
|
|
let buffer = futures::select! {
|
|
result = open_buffer_task.fuse() => result?,
|
|
_ = event_stream.cancelled_by_user().fuse() => {
|
|
anyhow::bail!("Diagnostics cancelled by user");
|
|
}
|
|
};
|
|
let mut output = String::new();
|
|
let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
|
|
|
|
for (_, group) in snapshot.diagnostic_groups(None) {
|
|
let entry = &group.entries[group.primary_ix];
|
|
let range = entry.range.to_point(&snapshot);
|
|
let severity = match entry.diagnostic.severity {
|
|
DiagnosticSeverity::ERROR => "error",
|
|
DiagnosticSeverity::WARNING => "warning",
|
|
_ => continue,
|
|
};
|
|
|
|
writeln!(
|
|
output,
|
|
"{} at line {}: {}",
|
|
severity,
|
|
range.start.row + 1,
|
|
entry.diagnostic.message
|
|
)?;
|
|
}
|
|
|
|
if output.is_empty() {
|
|
Ok("File doesn't have errors or warnings!".to_string())
|
|
} else {
|
|
Ok(output)
|
|
}
|
|
})
|
|
}
|
|
_ => {
|
|
let project = self.project.read(cx);
|
|
let mut output = String::new();
|
|
let mut has_diagnostics = false;
|
|
|
|
for (project_path, _, summary) in project.diagnostic_summaries(true, cx) {
|
|
if summary.error_count > 0 || summary.warning_count > 0 {
|
|
let Some(worktree) = project.worktree_for_id(project_path.worktree_id, cx)
|
|
else {
|
|
continue;
|
|
};
|
|
|
|
has_diagnostics = true;
|
|
output.push_str(&format!(
|
|
"{}: {} error(s), {} warning(s)\n",
|
|
worktree.read(cx).absolutize(&project_path.path).display(),
|
|
summary.error_count,
|
|
summary.warning_count
|
|
));
|
|
}
|
|
}
|
|
|
|
if has_diagnostics {
|
|
Task::ready(Ok(output))
|
|
} else {
|
|
Task::ready(Ok("No errors or warnings found in the project.".into()))
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|