Detailed changes
@@ -331,7 +331,7 @@ async fn test_terminal_tool_timeout_kills_handle(cx: &mut TestAppContext) {
"expected tool call update to include terminal content"
);
- let mut task_future: Pin<Box<Fuse<Task<Result<String>>>>> = Box::pin(task.fuse());
+ let mut task_future: Pin<Box<Fuse<Task<Result<String, String>>>>> = Box::pin(task.fuse());
let deadline = std::time::Instant::now() + Duration::from_millis(500);
loop {
@@ -4007,7 +4007,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) {
result.is_err(),
"expected command to be blocked by deny rule"
);
- let err_msg = result.unwrap_err().to_string().to_lowercase();
+ let err_msg = result.unwrap_err().to_lowercase();
assert!(
err_msg.contains("blocked"),
"error should mention the command was blocked"
@@ -4165,7 +4165,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) {
result.is_err(),
"expected command to be blocked by tool-specific deny default"
);
- let err_msg = result.unwrap_err().to_string().to_lowercase();
+ let err_msg = result.unwrap_err().to_lowercase();
assert!(
err_msg.contains("disabled"),
"error should mention the tool is disabled, got: {err_msg}"
@@ -4847,7 +4847,7 @@ async fn test_delete_path_tool_deny_rule_blocks_deletion(cx: &mut TestAppContext
let result = task.await;
assert!(result.is_err(), "expected deletion to be blocked");
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the deletion was blocked"
);
}
@@ -4903,7 +4903,7 @@ async fn test_move_path_tool_denies_if_destination_denied(cx: &mut TestAppContex
"expected move to be blocked due to destination path"
);
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the move was blocked"
);
}
@@ -4959,7 +4959,7 @@ async fn test_move_path_tool_denies_if_source_denied(cx: &mut TestAppContext) {
"expected move to be blocked due to source path"
);
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the move was blocked"
);
}
@@ -5014,7 +5014,7 @@ async fn test_copy_path_tool_deny_rule_blocks_copy(cx: &mut TestAppContext) {
let result = task.await;
assert!(result.is_err(), "expected copy to be blocked");
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the copy was blocked"
);
}
@@ -5074,7 +5074,7 @@ async fn test_save_file_tool_denies_if_any_path_denied(cx: &mut TestAppContext)
"expected save to be blocked due to denied path"
);
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the save was blocked"
);
}
@@ -5120,7 +5120,7 @@ async fn test_save_file_tool_respects_deny_rules(cx: &mut TestAppContext) {
let result = task.await;
assert!(result.is_err(), "expected save to be blocked");
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the save was blocked"
);
}
@@ -5157,10 +5157,15 @@ async fn test_web_search_tool_deny_rule_blocks_search(cx: &mut TestAppContext) {
let result = task.await;
assert!(result.is_err(), "expected search to be blocked");
- assert!(
- result.unwrap_err().to_string().contains("blocked"),
- "error should mention the search was blocked"
- );
+ match result.unwrap_err() {
+ crate::WebSearchToolOutput::Error { error } => {
+ assert!(
+ error.contains("blocked"),
+ "error should mention the search was blocked"
+ );
+ }
+ other => panic!("expected Error variant, got: {other:?}"),
+ }
}
#[gpui::test]
@@ -5332,7 +5337,7 @@ async fn test_fetch_tool_deny_rule_blocks_url(cx: &mut TestAppContext) {
let result = task.await;
assert!(result.is_err(), "expected fetch to be blocked");
assert!(
- result.unwrap_err().to_string().contains("blocked"),
+ result.unwrap_err().contains("blocked"),
"error should mention the fetch was blocked"
);
}
@@ -1,6 +1,5 @@
use super::*;
use agent_settings::AgentSettings;
-use anyhow::Result;
use gpui::{App, SharedString, Task};
use std::future;
use std::sync::atomic::{AtomicBool, Ordering};
@@ -37,7 +36,7 @@ impl AgentTool for EchoTool {
input: Self::Input,
_event_stream: ToolCallEventStream,
_cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
Task::ready(Ok(input.text))
}
}
@@ -78,7 +77,7 @@ impl AgentTool for DelayTool {
input: Self::Input,
_event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>>
+ ) -> Task<Result<String, String>>
where
Self: Sized,
{
@@ -118,14 +117,14 @@ impl AgentTool for ToolRequiringPermission {
_input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
let settings = AgentSettings::get_global(cx);
let decision = decide_permission_from_settings(Self::NAME, &[String::new()], settings);
let authorize = match decision {
ToolPermissionDecision::Allow => None,
ToolPermissionDecision::Deny(reason) => {
- return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
ToolPermissionDecision::Confirm => {
let context = crate::ToolPermissionContext::new(
@@ -138,7 +137,7 @@ impl AgentTool for ToolRequiringPermission {
cx.foreground_executor().spawn(async move {
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
Ok("Allowed".to_string())
})
@@ -173,7 +172,7 @@ impl AgentTool for InfiniteTool {
_input: Self::Input,
_event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
cx.foreground_executor().spawn(async move {
future::pending::<()>().await;
unreachable!()
@@ -225,12 +224,12 @@ impl AgentTool for CancellationAwareTool {
_input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
cx.foreground_executor().spawn(async move {
// Wait for cancellation - this tool does nothing but wait to be cancelled
event_stream.cancelled_by_user().await;
self.was_cancelled.store(true, Ordering::SeqCst);
- anyhow::bail!("Tool cancelled by user");
+ Err("Tool cancelled by user".to_string())
})
}
}
@@ -280,7 +279,7 @@ impl AgentTool for WordListTool {
_input: Self::Input,
_event_stream: ToolCallEventStream,
_cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
Task::ready(Ok("ok".to_string()))
}
}
@@ -2084,32 +2084,28 @@ impl Thread {
let tool_result = tool.run(tool_use.input, tool_event_stream, cx);
log::debug!("Running tool {}", tool_use.name);
Some(cx.foreground_executor().spawn(async move {
- let tool_result = tool_result.await.and_then(|output| {
- if let LanguageModelToolResultContent::Image(_) = &output.llm_output
- && !supports_images
- {
- return Err(anyhow!(
- "Attempted to read an image, but this model doesn't support it.",
- ));
+ let (is_error, output) = match tool_result.await {
+ Ok(mut output) => {
+ if let LanguageModelToolResultContent::Image(_) = &output.llm_output
+ && !supports_images
+ {
+ output = AgentToolOutput::from_error(
+ "Attempted to read an image, but this model doesn't support it.",
+ );
+ (true, output)
+ } else {
+ (false, output)
+ }
}
- Ok(output)
- });
+ Err(output) => (true, output),
+ };
- match tool_result {
- Ok(output) => LanguageModelToolResult {
- tool_use_id: tool_use.id,
- tool_name: tool_use.name,
- is_error: false,
- content: output.llm_output,
- output: Some(output.raw_output),
- },
- Err(error) => LanguageModelToolResult {
- tool_use_id: tool_use.id,
- tool_name: tool_use.name,
- is_error: true,
- content: LanguageModelToolResultContent::Text(Arc::from(error.to_string())),
- output: Some(error.to_string().into()),
- },
+ LanguageModelToolResult {
+ tool_use_id: tool_use.id,
+ tool_name: tool_use.name,
+ is_error,
+ content: output.llm_output,
+ output: Some(output.raw_output),
}
}))
}
@@ -2826,12 +2822,18 @@ where
}
/// Runs the tool with the provided input.
+ ///
+ /// Returns `Result<Self::Output, Self::Output>` rather than `Result<Self::Output, anyhow::Error>`
+ /// because tool errors are sent back to the model as tool results. This means error output must
+ /// be structured and readable by the agent — not an arbitrary `anyhow::Error`. Returning the
+ /// same `Output` type for both success and failure lets tools provide structured data while
+ /// still signaling whether the invocation succeeded or failed.
fn run(
self: Arc<Self>,
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>>;
+ ) -> Task<Result<Self::Output, Self::Output>>;
/// Emits events for a previous execution of the tool.
fn replay(
@@ -2856,6 +2858,17 @@ pub struct AgentToolOutput {
pub raw_output: serde_json::Value,
}
+impl AgentToolOutput {
+ pub fn from_error(message: impl Into<String>) -> Self {
+ let message = message.into();
+ let llm_output = LanguageModelToolResultContent::Text(Arc::from(message.as_str()));
+ Self {
+ raw_output: serde_json::Value::String(message),
+ llm_output,
+ }
+ }
+}
+
pub trait AnyAgentTool {
fn name(&self) -> SharedString;
fn description(&self) -> SharedString;
@@ -2865,12 +2878,13 @@ pub trait AnyAgentTool {
fn supports_provider(&self, _provider: &LanguageModelProviderId) -> bool {
true
}
+ /// See [`AgentTool::run`] for why this returns `Result<AgentToolOutput, AgentToolOutput>`.
fn run(
self: Arc<Self>,
input: serde_json::Value,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<AgentToolOutput>>;
+ ) -> Task<Result<AgentToolOutput, AgentToolOutput>>;
fn replay(
&self,
input: serde_json::Value,
@@ -2916,17 +2930,33 @@ where
input: serde_json::Value,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<AgentToolOutput>> {
+ ) -> Task<Result<AgentToolOutput, AgentToolOutput>> {
cx.spawn(async move |cx| {
- let input = serde_json::from_value(input)?;
- let output = cx
- .update(|cx| self.0.clone().run(input, event_stream, cx))
- .await?;
- let raw_output = serde_json::to_value(&output)?;
- Ok(AgentToolOutput {
- llm_output: output.into(),
- raw_output,
- })
+ let input: T::Input = serde_json::from_value(input).map_err(|e| {
+ AgentToolOutput::from_error(format!("Failed to parse tool input: {e}"))
+ })?;
+ let task = cx.update(|cx| self.0.clone().run(input, event_stream, cx));
+ match task.await {
+ Ok(output) => {
+ let raw_output = serde_json::to_value(&output).map_err(|e| {
+ AgentToolOutput::from_error(format!("Failed to serialize tool output: {e}"))
+ })?;
+ Ok(AgentToolOutput {
+ llm_output: output.into(),
+ raw_output,
+ })
+ }
+ Err(error_output) => {
+ let raw_output = serde_json::to_value(&error_output).unwrap_or_else(|e| {
+ log::error!("Failed to serialize tool error output: {e}");
+ serde_json::Value::Null
+ });
+ Err(AgentToolOutput {
+ llm_output: error_output.into(),
+ raw_output,
+ })
+ }
+ }
})
}
@@ -1,6 +1,6 @@
use crate::{AgentToolOutput, AnyAgentTool, ToolCallEventStream};
use agent_client_protocol::ToolKind;
-use anyhow::{Result, anyhow};
+use anyhow::Result;
use collections::{BTreeMap, HashMap};
use context_server::{ContextServerId, client::NotificationSubscription};
use futures::FutureExt as _;
@@ -332,9 +332,9 @@ impl AnyAgentTool for ContextServerTool {
input: serde_json::Value,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<AgentToolOutput>> {
+ ) -> Task<Result<AgentToolOutput, AgentToolOutput>> {
let Some(server) = self.store.read(cx).get_running_server(&self.server_id) else {
- return Task::ready(Err(anyhow!("Context server not found")));
+ return Task::ready(Err(AgentToolOutput::from_error("Context server not found")));
};
let tool_name = self.tool.name.clone();
let tool_id = mcp_tool_id(&self.server_id.0, &self.tool.name);
@@ -347,10 +347,10 @@ impl AnyAgentTool for ContextServerTool {
);
cx.spawn(async move |_cx| {
- authorize.await?;
+ authorize.await.map_err(|e| AgentToolOutput::from_error(e.to_string()))?;
let Some(protocol) = server.client() else {
- anyhow::bail!("Context server not initialized");
+ return Err(AgentToolOutput::from_error("Context server not initialized"));
};
let arguments = if let serde_json::Value::Object(map) = input {
@@ -374,16 +374,16 @@ impl AnyAgentTool for ContextServerTool {
);
let response = futures::select! {
- response = request.fuse() => response?,
+ response = request.fuse() => response.map_err(|e| AgentToolOutput::from_error(e.to_string()))?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("MCP tool cancelled by user");
+ return Err(AgentToolOutput::from_error("MCP tool cancelled by user"));
}
};
if response.is_error == Some(true) {
let error_message: String =
response.content.iter().filter_map(|c| c.text()).collect();
- anyhow::bail!(error_message);
+ return Err(AgentToolOutput::from_error(error_message));
}
let mut result = String::new();
@@ -5,7 +5,6 @@ use super::tool_permissions::{
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths};
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
-use anyhow::{Context as _, Result, anyhow};
use futures::FutureExt as _;
use gpui::{App, Entity, Task};
use project::Project;
@@ -83,12 +82,12 @@ impl AgentTool for CopyPathTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let settings = AgentSettings::get_global(cx);
let paths = vec![input.source_path.clone(), input.destination_path.clone()];
let decision = decide_permission_for_paths(Self::NAME, &paths, settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
let project = self.project.clone();
@@ -147,7 +146,7 @@ impl AgentTool for CopyPathTool {
};
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let copy_task = project.update(cx, |project, cx| {
@@ -157,12 +156,12 @@ impl AgentTool for CopyPathTool {
{
Some(entity) => match project.find_project_path(&input.destination_path, cx) {
Some(project_path) => Ok(project.copy_entry(entity.id, project_path, cx)),
- None => Err(anyhow!(
+ None => Err(format!(
"Destination path {} was outside the project.",
input.destination_path
)),
},
- None => Err(anyhow!(
+ None => Err(format!(
"Source path {} was not found in the project.",
input.source_path
)),
@@ -172,12 +171,12 @@ impl AgentTool for CopyPathTool {
let result = futures::select! {
result = copy_task.fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Copy cancelled by user");
+ return Err("Copy cancelled by user".to_string());
}
};
- result.with_context(|| {
+ result.map_err(|e| {
format!(
- "Copying {} to {}",
+ "Copying {} to {}: {e}",
input.source_path, input.destination_path
)
})?;
@@ -4,7 +4,6 @@ use super::tool_permissions::{
};
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
-use anyhow::{Context as _, Result, anyhow};
use futures::FutureExt as _;
use gpui::{App, Entity, SharedString, Task};
use project::Project;
@@ -72,12 +71,12 @@ impl AgentTool for CreateDirectoryTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let settings = AgentSettings::get_global(cx);
let decision = decide_permission_for_path(Self::NAME, &input.path, settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
let destination_path: Arc<str> = input.path.as_str().into();
@@ -136,22 +135,22 @@ impl AgentTool for CreateDirectoryTool {
};
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let create_entry = project.update(cx, |project, cx| {
match project.find_project_path(&input.path, cx) {
Some(project_path) => Ok(project.create_entry(project_path, true, cx)),
- None => Err(anyhow!("Path to create was outside the project")),
+ None => Err("Path to create was outside the project".to_string()),
}
})?;
futures::select! {
result = create_entry.fuse() => {
- result.with_context(|| format!("Creating directory {destination_path}"))?;
+ result.map_err(|e| format!("Creating directory {destination_path}: {e}"))?;
}
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Create directory cancelled by user");
+ return Err("Create directory cancelled by user".to_string());
}
}
@@ -6,7 +6,6 @@ use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permi
use action_log::ActionLog;
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
-use anyhow::{Context as _, Result, anyhow};
use futures::{FutureExt as _, SinkExt, StreamExt, channel::mpsc};
use gpui::{App, AppContext, Entity, SharedString, Task};
use project::{Project, ProjectPath};
@@ -75,14 +74,14 @@ impl AgentTool for DeletePathTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let path = input.path;
let settings = AgentSettings::get_global(cx);
let decision = decide_permission_for_path(Self::NAME, &path, settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
let project = self.project.clone();
@@ -140,20 +139,20 @@ impl AgentTool for DeletePathTool {
};
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let (project_path, worktree_snapshot) = project.read_with(cx, |project, cx| {
let project_path = project.find_project_path(&path, cx).ok_or_else(|| {
- anyhow!("Couldn't delete {path} because that path isn't in this project.")
+ format!("Couldn't delete {path} because that path isn't in this project.")
})?;
let worktree = project
.worktree_for_id(project_path.worktree_id, cx)
.ok_or_else(|| {
- anyhow!("Couldn't delete {path} because that path isn't in this project.")
+ format!("Couldn't delete {path} because that path isn't in this project.")
})?;
let worktree_snapshot = worktree.read(cx).snapshot();
- anyhow::Ok((project_path, worktree_snapshot))
+ Result::<_, String>::Ok((project_path, worktree_snapshot))
})?;
let (mut paths_tx, mut paths_rx) = mpsc::channel(256);
@@ -182,7 +181,7 @@ impl AgentTool for DeletePathTool {
let path_result = futures::select! {
path = paths_rx.next().fuse() => path,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Delete cancelled by user");
+ return Err("Delete cancelled by user".to_string());
}
};
let Some(path) = path_result else {
@@ -202,16 +201,16 @@ impl AgentTool for DeletePathTool {
.update(cx, |project, cx| {
project.delete_file(project_path, false, cx)
})
- .with_context(|| {
+ .ok_or_else(|| {
format!("Couldn't delete {path} because that path isn't in this project.")
})?;
futures::select! {
result = deletion_task.fuse() => {
- result.with_context(|| format!("Deleting {path}"))?;
+ result.map_err(|e| format!("Deleting {path}: {e}"))?;
}
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Delete cancelled by user");
+ return Err("Delete cancelled by user".to_string());
}
}
Ok(format!("Deleted {path}"))
@@ -1,6 +1,6 @@
use crate::{AgentTool, ToolCallEventStream};
use agent_client_protocol as acp;
-use anyhow::{Result, anyhow};
+use anyhow::Result;
use futures::FutureExt as _;
use gpui::{App, Entity, Task};
use language::{DiagnosticSeverity, OffsetRangeExt};
@@ -90,11 +90,11 @@ impl AgentTool for DiagnosticsTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, 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",)));
+ return Task::ready(Err(format!("Could not find path {path} in project")));
};
let open_buffer_task = self
@@ -103,9 +103,9 @@ impl AgentTool for DiagnosticsTool {
cx.spawn(async move |cx| {
let buffer = futures::select! {
- result = open_buffer_task.fuse() => result?,
+ result = open_buffer_task.fuse() => result.map_err(|e| e.to_string())?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Diagnostics cancelled by user");
+ return Err("Diagnostics cancelled by user".to_string());
}
};
let mut output = String::new();
@@ -126,7 +126,8 @@ impl AgentTool for DiagnosticsTool {
severity,
range.start.row + 1,
entry.diagnostic.message
- )?;
+ )
+ .ok();
}
if output.is_empty() {
@@ -7,7 +7,7 @@ use crate::{
};
use acp_thread::Diff;
use agent_client_protocol::{self as acp, ToolCallLocation, ToolCallUpdateFields};
-use anyhow::{Context as _, Result, anyhow};
+use anyhow::{Context as _, Result};
use cloud_llm_client::CompletionIntent;
use collections::HashSet;
use futures::{FutureExt as _, StreamExt as _};
@@ -95,29 +95,47 @@ pub enum EditFileMode {
}
#[derive(Debug, Serialize, Deserialize)]
-pub struct EditFileToolOutput {
- #[serde(alias = "original_path")]
- input_path: PathBuf,
- new_text: String,
- old_text: Arc<String>,
- #[serde(default)]
- diff: String,
- #[serde(alias = "raw_output")]
- edit_agent_output: EditAgentOutput,
+#[serde(untagged)]
+pub enum EditFileToolOutput {
+ Success {
+ #[serde(alias = "original_path")]
+ input_path: PathBuf,
+ new_text: String,
+ old_text: Arc<String>,
+ #[serde(default)]
+ diff: String,
+ #[serde(alias = "raw_output")]
+ edit_agent_output: EditAgentOutput,
+ },
+ Error {
+ error: String,
+ },
+}
+
+impl std::fmt::Display for EditFileToolOutput {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ EditFileToolOutput::Success {
+ diff, input_path, ..
+ } => {
+ if diff.is_empty() {
+ write!(f, "No edits were made.")
+ } else {
+ write!(
+ f,
+ "Edited {}:\n\n```diff\n{diff}\n```",
+ input_path.display()
+ )
+ }
+ }
+ EditFileToolOutput::Error { error } => write!(f, "{error}"),
+ }
+ }
}
impl From<EditFileToolOutput> for LanguageModelToolResultContent {
fn from(output: EditFileToolOutput) -> Self {
- if output.diff.is_empty() {
- "No edits were made.".into()
- } else {
- format!(
- "Edited {}:\n\n```diff\n{}\n```",
- output.input_path.display(),
- output.diff
- )
- .into()
- }
+ output.to_string().into()
}
}
@@ -222,16 +240,22 @@ impl AgentTool for EditFileTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let Ok(project) = self
.thread
.read_with(cx, |thread, _cx| thread.project().clone())
else {
- return Task::ready(Err(anyhow!("thread was dropped")));
+ return Task::ready(Err(EditFileToolOutput::Error {
+ error: "thread was dropped".to_string(),
+ }));
};
let project_path = match resolve_path(&input, project.clone(), cx) {
Ok(path) => path,
- Err(err) => return Task::ready(Err(anyhow!(err))),
+ Err(err) => {
+ return Task::ready(Err(EditFileToolOutput::Error {
+ error: err.to_string(),
+ }));
+ }
};
let abs_path = project.read(cx).absolute_path(&project_path, cx);
if let Some(abs_path) = abs_path.clone() {
@@ -242,255 +266,259 @@ impl AgentTool for EditFileTool {
let authorize = self.authorize(&input, &event_stream, cx);
cx.spawn(async move |cx: &mut AsyncApp| {
- authorize.await?;
-
- let (request, model, action_log) = self.thread.update(cx, |thread, cx| {
- let request = thread.build_completion_request(CompletionIntent::ToolResults, cx);
- (request, thread.model().cloned(), thread.action_log().clone())
- })?;
- let request = request?;
- let model = model.context("No language model configured")?;
-
- let edit_format = EditFormat::from_model(model.clone())?;
- let edit_agent = EditAgent::new(
- model,
- project.clone(),
- action_log.clone(),
- self.templates.clone(),
- edit_format,
- );
+ let result: anyhow::Result<EditFileToolOutput> = async {
+ authorize.await?;
- let buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(project_path.clone(), cx)
- })
- .await?;
-
- // Check if the file has been modified since the agent last read it
- if let Some(abs_path) = abs_path.as_ref() {
- let (last_read_mtime, current_mtime, is_dirty, has_save_tool, has_restore_tool) = self.thread.update(cx, |thread, cx| {
- let last_read = thread.file_read_times.get(abs_path).copied();
- let current = buffer.read(cx).file().and_then(|file| file.disk_state().mtime());
- let dirty = buffer.read(cx).is_dirty();
- let has_save = thread.has_tool(SaveFileTool::NAME);
- let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME);
- (last_read, current, dirty, has_save, has_restore)
+ let (request, model, action_log) = self.thread.update(cx, |thread, cx| {
+ let request = thread.build_completion_request(CompletionIntent::ToolResults, cx);
+ (request, thread.model().cloned(), thread.action_log().clone())
})?;
+ let request = request?;
+ let model = model.context("No language model configured")?;
- // Check for unsaved changes first - these indicate modifications we don't know about
- if is_dirty {
- let message = match (has_save_tool, has_restore_tool) {
- (true, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (true, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask the user to manually revert the file, then inform you when it's ok to proceed."
- }
- (false, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask the user to manually save the file, then inform you when it's ok to proceed. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (false, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes, \
- then ask them to save or revert the file manually and inform you when it's ok to proceed."
- }
- };
- anyhow::bail!("{}", message);
- }
+ let edit_format = EditFormat::from_model(model.clone())?;
+ let edit_agent = EditAgent::new(
+ model,
+ project.clone(),
+ action_log.clone(),
+ self.templates.clone(),
+ edit_format,
+ );
- // Check if the file was modified on disk since we last read it
- if let (Some(last_read), Some(current)) = (last_read_mtime, current_mtime) {
- // MTime can be unreliable for comparisons, so our newtype intentionally
- // doesn't support comparing them. If the mtime at all different
- // (which could be because of a modification or because e.g. system clock changed),
- // we pessimistically assume it was modified.
- if current != last_read {
- anyhow::bail!(
- "The file {} has been modified since you last read it. \
- Please read the file again to get the current state before editing it.",
- input.path.display()
- );
- }
- }
- }
+ let buffer = project
+ .update(cx, |project, cx| {
+ project.open_buffer(project_path.clone(), cx)
+ })
+ .await?;
+
+ // Check if the file has been modified since the agent last read it
+ if let Some(abs_path) = abs_path.as_ref() {
+ let (last_read_mtime, current_mtime, is_dirty, has_save_tool, has_restore_tool) = self.thread.update(cx, |thread, cx| {
+ let last_read = thread.file_read_times.get(abs_path).copied();
+ let current = buffer.read(cx).file().and_then(|file| file.disk_state().mtime());
+ let dirty = buffer.read(cx).is_dirty();
+ let has_save = thread.has_tool(SaveFileTool::NAME);
+ let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME);
+ (last_read, current, dirty, has_save, has_restore)
+ })?;
- let diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
- event_stream.update_diff(diff.clone());
- let _finalize_diff = util::defer({
- let diff = diff.downgrade();
- let mut cx = cx.clone();
- move || {
- diff.update(&mut cx, |diff, cx| diff.finalize(cx)).ok();
- }
- });
+ // Check for unsaved changes first - these indicate modifications we don't know about
+ if is_dirty {
+ let message = match (has_save_tool, has_restore_tool) {
+ (true, true) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
+ If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
+ }
+ (true, false) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
+ If they want to discard them, ask the user to manually revert the file, then inform you when it's ok to proceed."
+ }
+ (false, true) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask the user to manually save the file, then inform you when it's ok to proceed. \
+ If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
+ }
+ (false, false) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes, \
+ then ask them to save or revert the file manually and inform you when it's ok to proceed."
+ }
+ };
+ anyhow::bail!("{}", message);
+ }
- let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
- let old_text = cx
- .background_spawn({
- let old_snapshot = old_snapshot.clone();
- async move { Arc::new(old_snapshot.text()) }
- })
- .await;
+ // Check if the file was modified on disk since we last read it
+ if let (Some(last_read), Some(current)) = (last_read_mtime, current_mtime) {
+ // MTime can be unreliable for comparisons, so our newtype intentionally
+ // doesn't support comparing them. If the mtime at all different
+ // (which could be because of a modification or because e.g. system clock changed),
+ // we pessimistically assume it was modified.
+ if current != last_read {
+ anyhow::bail!(
+ "The file {} has been modified since you last read it. \
+ Please read the file again to get the current state before editing it.",
+ input.path.display()
+ );
+ }
+ }
+ }
- let (output, mut events) = if matches!(input.mode, EditFileMode::Edit) {
- edit_agent.edit(
- buffer.clone(),
- input.display_description.clone(),
- &request,
- cx,
- )
- } else {
- edit_agent.overwrite(
- buffer.clone(),
- input.display_description.clone(),
- &request,
- cx,
- )
- };
-
- let mut hallucinated_old_text = false;
- let mut ambiguous_ranges = Vec::new();
- let mut emitted_location = false;
- loop {
- let event = futures::select! {
- event = events.next().fuse() => match event {
- Some(event) => event,
- None => break,
- },
- _ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Edit cancelled by user");
+ let diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
+ event_stream.update_diff(diff.clone());
+ let _finalize_diff = util::defer({
+ let diff = diff.downgrade();
+ let mut cx = cx.clone();
+ move || {
+ diff.update(&mut cx, |diff, cx| diff.finalize(cx)).ok();
}
+ });
+
+ let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+ let old_text = cx
+ .background_spawn({
+ let old_snapshot = old_snapshot.clone();
+ async move { Arc::new(old_snapshot.text()) }
+ })
+ .await;
+
+ let (output, mut events) = if matches!(input.mode, EditFileMode::Edit) {
+ edit_agent.edit(
+ buffer.clone(),
+ input.display_description.clone(),
+ &request,
+ cx,
+ )
+ } else {
+ edit_agent.overwrite(
+ buffer.clone(),
+ input.display_description.clone(),
+ &request,
+ cx,
+ )
};
- match event {
- EditAgentOutputEvent::Edited(range) => {
- if !emitted_location {
- let line = Some(buffer.update(cx, |buffer, _cx| {
- range.start.to_point(&buffer.snapshot()).row
- }));
- if let Some(abs_path) = abs_path.clone() {
- event_stream.update_fields(ToolCallUpdateFields::new().locations(vec![ToolCallLocation::new(abs_path).line(line)]));
+
+ let mut hallucinated_old_text = false;
+ let mut ambiguous_ranges = Vec::new();
+ let mut emitted_location = false;
+ loop {
+ let event = futures::select! {
+ event = events.next().fuse() => match event {
+ Some(event) => event,
+ None => break,
+ },
+ _ = event_stream.cancelled_by_user().fuse() => {
+ anyhow::bail!("Edit cancelled by user");
+ }
+ };
+ match event {
+ EditAgentOutputEvent::Edited(range) => {
+ if !emitted_location {
+ let line = Some(buffer.update(cx, |buffer, _cx| {
+ range.start.to_point(&buffer.snapshot()).row
+ }));
+ if let Some(abs_path) = abs_path.clone() {
+ event_stream.update_fields(ToolCallUpdateFields::new().locations(vec![ToolCallLocation::new(abs_path).line(line)]));
+ }
+ emitted_location = true;
}
- emitted_location = true;
+ },
+ EditAgentOutputEvent::UnresolvedEditRange => hallucinated_old_text = true,
+ EditAgentOutputEvent::AmbiguousEditRange(ranges) => ambiguous_ranges = ranges,
+ EditAgentOutputEvent::ResolvingEditRange(range) => {
+ diff.update(cx, |card, cx| card.reveal_range(range.clone(), cx));
+ // if !emitted_location {
+ // let line = buffer.update(cx, |buffer, _cx| {
+ // range.start.to_point(&buffer.snapshot()).row
+ // }).ok();
+ // if let Some(abs_path) = abs_path.clone() {
+ // event_stream.update_fields(ToolCallUpdateFields {
+ // locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
+ // ..Default::default()
+ // });
+ // }
+ // }
}
- },
- EditAgentOutputEvent::UnresolvedEditRange => hallucinated_old_text = true,
- EditAgentOutputEvent::AmbiguousEditRange(ranges) => ambiguous_ranges = ranges,
- EditAgentOutputEvent::ResolvingEditRange(range) => {
- diff.update(cx, |card, cx| card.reveal_range(range.clone(), cx));
- // if !emitted_location {
- // let line = buffer.update(cx, |buffer, _cx| {
- // range.start.to_point(&buffer.snapshot()).row
- // }).ok();
- // if let Some(abs_path) = abs_path.clone() {
- // event_stream.update_fields(ToolCallUpdateFields {
- // locations: Some(vec![ToolCallLocation { path: abs_path, line }]),
- // ..Default::default()
- // });
- // }
- // }
}
}
- }
-
- let edit_agent_output = output.await?;
- let format_on_save_enabled = buffer.read_with(cx, |buffer, cx| {
- let settings = language_settings::language_settings(
- buffer.language().map(|l| l.name()),
- buffer.file(),
- cx,
- );
- settings.format_on_save != FormatOnSave::Off
- });
-
- if format_on_save_enabled {
- action_log.update(cx, |log, cx| {
- log.buffer_edited(buffer.clone(), cx);
- });
+ let edit_agent_output = output.await?;
- let format_task = project.update(cx, |project, cx| {
- project.format(
- HashSet::from_iter([buffer.clone()]),
- LspFormatTarget::Buffers,
- false, // Don't push to history since the tool did it.
- FormatTrigger::Save,
+ let format_on_save_enabled = buffer.read_with(cx, |buffer, cx| {
+ let settings = language_settings::language_settings(
+ buffer.language().map(|l| l.name()),
+ buffer.file(),
cx,
- )
+ );
+ settings.format_on_save != FormatOnSave::Off
});
- format_task.await.log_err();
- }
- project
- .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
- .await?;
+ if format_on_save_enabled {
+ action_log.update(cx, |log, cx| {
+ log.buffer_edited(buffer.clone(), cx);
+ });
+
+ let format_task = project.update(cx, |project, cx| {
+ project.format(
+ HashSet::from_iter([buffer.clone()]),
+ LspFormatTarget::Buffers,
+ false, // Don't push to history since the tool did it.
+ FormatTrigger::Save,
+ cx,
+ )
+ });
+ format_task.await.log_err();
+ }
- action_log.update(cx, |log, cx| {
- log.buffer_edited(buffer.clone(), cx);
- });
+ project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+ .await?;
- // Update the recorded read time after a successful edit so consecutive edits work
- if let Some(abs_path) = abs_path.as_ref() {
- if let Some(new_mtime) = buffer.read_with(cx, |buffer, _| {
- buffer.file().and_then(|file| file.disk_state().mtime())
- }) {
- self.thread.update(cx, |thread, _| {
- thread.file_read_times.insert(abs_path.to_path_buf(), new_mtime);
- })?;
- }
- }
+ action_log.update(cx, |log, cx| {
+ log.buffer_edited(buffer.clone(), cx);
+ });
- let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
- let (new_text, unified_diff) = cx
- .background_spawn({
- let new_snapshot = new_snapshot.clone();
- let old_text = old_text.clone();
- async move {
- let new_text = new_snapshot.text();
- let diff = language::unified_diff(&old_text, &new_text);
- (new_text, diff)
+ // Update the recorded read time after a successful edit so consecutive edits work
+ if let Some(abs_path) = abs_path.as_ref() {
+ if let Some(new_mtime) = buffer.read_with(cx, |buffer, _| {
+ buffer.file().and_then(|file| file.disk_state().mtime())
+ }) {
+ self.thread.update(cx, |thread, _| {
+ thread.file_read_times.insert(abs_path.to_path_buf(), new_mtime);
+ })?;
}
- })
- .await;
+ }
- let input_path = input.path.display();
- if unified_diff.is_empty() {
- anyhow::ensure!(
- !hallucinated_old_text,
- formatdoc! {"
- Some edits were produced but none of them could be applied.
- Read the relevant sections of {input_path} again so that
- I can perform the requested edits.
- "}
- );
- anyhow::ensure!(
- ambiguous_ranges.is_empty(),
- {
- let line_numbers = ambiguous_ranges
- .iter()
- .map(|range| range.start.to_string())
- .collect::<Vec<_>>()
- .join(", ");
+ let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+ let (new_text, unified_diff) = cx
+ .background_spawn({
+ let new_snapshot = new_snapshot.clone();
+ let old_text = old_text.clone();
+ async move {
+ let new_text = new_snapshot.text();
+ let diff = language::unified_diff(&old_text, &new_text);
+ (new_text, diff)
+ }
+ })
+ .await;
+
+ let input_path = input.path.display();
+ if unified_diff.is_empty() {
+ anyhow::ensure!(
+ !hallucinated_old_text,
formatdoc! {"
- <old_text> matches more than one position in the file (lines: {line_numbers}). Read the
- relevant sections of {input_path} again and extend <old_text> so
- that I can perform the requested edits.
+ Some edits were produced but none of them could be applied.
+ Read the relevant sections of {input_path} again so that
+ I can perform the requested edits.
"}
- }
- );
- }
+ );
+ anyhow::ensure!(
+ ambiguous_ranges.is_empty(),
+ {
+ let line_numbers = ambiguous_ranges
+ .iter()
+ .map(|range| range.start.to_string())
+ .collect::<Vec<_>>()
+ .join(", ");
+ formatdoc! {"
+ <old_text> matches more than one position in the file (lines: {line_numbers}). Read the
+ relevant sections of {input_path} again and extend <old_text> so
+ that I can perform the requested edits.
+ "}
+ }
+ );
+ }
- Ok(EditFileToolOutput {
- input_path: input.path,
- new_text,
- old_text,
- diff: unified_diff,
- edit_agent_output,
- })
+ anyhow::Ok(EditFileToolOutput::Success {
+ input_path: input.path,
+ new_text,
+ old_text,
+ diff: unified_diff,
+ edit_agent_output,
+ })
+ }.await;
+ result
+ .map_err(|e| EditFileToolOutput::Error { error: e.to_string() })
})
}
@@ -501,16 +529,26 @@ impl AgentTool for EditFileTool {
event_stream: ToolCallEventStream,
cx: &mut App,
) -> Result<()> {
- event_stream.update_diff(cx.new(|cx| {
- Diff::finalized(
- output.input_path.to_string_lossy().into_owned(),
- Some(output.old_text.to_string()),
- output.new_text,
- self.language_registry.clone(),
- cx,
- )
- }));
- Ok(())
+ match output {
+ EditFileToolOutput::Success {
+ input_path,
+ old_text,
+ new_text,
+ ..
+ } => {
+ event_stream.update_diff(cx.new(|cx| {
+ Diff::finalized(
+ input_path.to_string_lossy().into_owned(),
+ Some(old_text.to_string()),
+ new_text,
+ self.language_registry.clone(),
+ cx,
+ )
+ }));
+ Ok(())
+ }
+ EditFileToolOutput::Error { .. } => Ok(()),
+ }
}
}
@@ -144,7 +144,7 @@ impl AgentTool for FetchTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let settings = AgentSettings::get_global(cx);
let decision =
decide_permission_from_settings(Self::NAME, std::slice::from_ref(&input.url), settings);
@@ -152,7 +152,7 @@ impl AgentTool for FetchTool {
let authorize = match decision {
ToolPermissionDecision::Allow => None,
ToolPermissionDecision::Deny(reason) => {
- return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
ToolPermissionDecision::Confirm => {
let context =
@@ -177,13 +177,13 @@ impl AgentTool for FetchTool {
cx.foreground_executor().spawn(async move {
let text = futures::select! {
- result = fetch_task.fuse() => result?,
+ result = fetch_task.fuse() => result.map_err(|e| e.to_string())?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Fetch cancelled by user");
+ return Err("Fetch cancelled by user".to_string());
}
};
if text.trim().is_empty() {
- bail!("no textual content found");
+ return Err("no textual content found".to_string());
}
Ok(text)
})
@@ -39,33 +39,48 @@ pub struct FindPathToolInput {
}
#[derive(Debug, Serialize, Deserialize)]
-pub struct FindPathToolOutput {
- offset: usize,
- current_matches_page: Vec<PathBuf>,
- all_matches_len: usize,
+#[serde(untagged)]
+pub enum FindPathToolOutput {
+ Success {
+ offset: usize,
+ current_matches_page: Vec<PathBuf>,
+ all_matches_len: usize,
+ },
+ Error {
+ error: String,
+ },
}
impl From<FindPathToolOutput> for LanguageModelToolResultContent {
fn from(output: FindPathToolOutput) -> Self {
- if output.current_matches_page.is_empty() {
- "No matches found".into()
- } else {
- let mut llm_output = format!("Found {} total matches.", output.all_matches_len);
- if output.all_matches_len > RESULTS_PER_PAGE {
- write!(
- &mut llm_output,
- "\nShowing results {}-{} (provide 'offset' parameter for more results):",
- output.offset + 1,
- output.offset + output.current_matches_page.len()
- )
- .unwrap();
- }
+ match output {
+ FindPathToolOutput::Success {
+ offset,
+ current_matches_page,
+ all_matches_len,
+ } => {
+ if current_matches_page.is_empty() {
+ "No matches found".into()
+ } else {
+ let mut llm_output = format!("Found {} total matches.", all_matches_len);
+ if all_matches_len > RESULTS_PER_PAGE {
+ write!(
+ &mut llm_output,
+ "\nShowing results {}-{} (provide 'offset' parameter for more results):",
+ offset + 1,
+ offset + current_matches_page.len()
+ )
+ .ok();
+ }
- for mat in output.current_matches_page {
- write!(&mut llm_output, "\n{}", mat.display()).unwrap();
- }
+ for mat in current_matches_page {
+ write!(&mut llm_output, "\n{}", mat.display()).ok();
+ }
- llm_output.into()
+ llm_output.into()
+ }
+ }
+ FindPathToolOutput::Error { error } => error.into(),
}
}
}
@@ -109,14 +124,14 @@ impl AgentTool for FindPathTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<FindPathToolOutput>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let search_paths_task = search_paths(&input.glob, self.project.clone(), cx);
cx.background_spawn(async move {
let matches = futures::select! {
- result = search_paths_task.fuse() => result?,
+ result = search_paths_task.fuse() => result.map_err(|e| FindPathToolOutput::Error { error: e.to_string() })?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Path search cancelled by user");
+ return Err(FindPathToolOutput::Error { error: "Path search cancelled by user".to_string() });
}
};
let paginated_matches: &[PathBuf] = &matches[cmp::min(input.offset, matches.len())
@@ -146,7 +161,7 @@ impl AgentTool for FindPathTool {
),
);
- Ok(FindPathToolOutput {
+ Ok(FindPathToolOutput::Success {
offset: input.offset,
current_matches_page: paginated_matches.to_vec(),
all_matches_len: matches.len(),
@@ -1,6 +1,6 @@
use crate::{AgentTool, ToolCallEventStream};
use agent_client_protocol as acp;
-use anyhow::{Result, anyhow};
+use anyhow::Result;
use futures::{FutureExt as _, StreamExt};
use gpui::{App, Entity, SharedString, Task};
use language::{OffsetRangeExt, ParseStatus, Point};
@@ -117,7 +117,7 @@ impl AgentTool for GrepTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
const CONTEXT_LINES: u32 = 2;
const MAX_ANCESTOR_LINES: u32 = 10;
@@ -133,7 +133,7 @@ impl AgentTool for GrepTool {
) {
Ok(matcher) => matcher,
Err(error) => {
- return Task::ready(Err(anyhow!("invalid include glob pattern: {error}")));
+ return Task::ready(Err(format!("invalid include glob pattern: {error}")));
}
};
@@ -148,7 +148,7 @@ impl AgentTool for GrepTool {
match PathMatcher::new(exclude_patterns, path_style) {
Ok(matcher) => matcher,
Err(error) => {
- return Task::ready(Err(anyhow!("invalid exclude pattern: {error}")));
+ return Task::ready(Err(format!("invalid exclude pattern: {error}")));
}
}
};
@@ -165,7 +165,7 @@ impl AgentTool for GrepTool {
None,
) {
Ok(query) => query,
- Err(error) => return Task::ready(Err(error)),
+ Err(error) => return Task::ready(Err(error.to_string())),
};
let results = self
@@ -188,7 +188,7 @@ impl AgentTool for GrepTool {
let search_result = futures::select! {
result = rx.next().fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Search cancelled by user");
+ return Err("Search cancelled by user".to_string());
}
};
let Some(SearchResult::Buffer { buffer, ranges }) = search_result else {
@@ -218,7 +218,7 @@ impl AgentTool for GrepTool {
}
while *parse_status.borrow() != ParseStatus::Idle {
- parse_status.changed().await?;
+ parse_status.changed().await.map_err(|e| e.to_string())?;
}
let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
@@ -280,7 +280,8 @@ impl AgentTool for GrepTool {
}
if !file_header_written {
- writeln!(output, "\n## Matches in {}", path.display())?;
+ writeln!(output, "\n## Matches in {}", path.display())
+ .ok();
file_header_written = true;
}
@@ -288,13 +289,16 @@ impl AgentTool for GrepTool {
output.push_str("\n### ");
for symbol in parent_symbols {
- write!(output, "{} › ", symbol.text)?;
+ write!(output, "{} › ", symbol.text)
+ .ok();
}
if range.start.row == end_row {
- writeln!(output, "L{}", range.start.row + 1)?;
+ writeln!(output, "L{}", range.start.row + 1)
+ .ok();
} else {
- writeln!(output, "L{}-{}", range.start.row + 1, end_row + 1)?;
+ writeln!(output, "L{}-{}", range.start.row + 1, end_row + 1)
+ .ok();
}
output.push_str("```\n");
@@ -304,7 +308,8 @@ impl AgentTool for GrepTool {
if let Some(ancestor_range) = ancestor_range
&& end_row < ancestor_range.end.row {
let remaining_lines = ancestor_range.end.row - end_row;
- writeln!(output, "\n{} lines remaining in ancestor node. Read the file to see all.", remaining_lines)?;
+ writeln!(output, "\n{} lines remaining in ancestor node. Read the file to see all.", remaining_lines)
+ .ok();
}
matches_found += 1;
@@ -149,7 +149,7 @@ impl AgentTool for ListDirectoryTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
// Sometimes models will return these even though we tell it to give a path and not a glob.
// When this happens, just list the root worktree directories.
if matches!(input.path.as_str(), "." | "" | "./" | "*") {
@@ -187,7 +187,7 @@ impl AgentTool for ListDirectoryTool {
canonical_target,
} => (project_path, Some(canonical_target)),
})
- })?;
+ }).map_err(|e| e.to_string())?;
// Check settings exclusions synchronously
project.read_with(cx, |project, cx| {
@@ -236,7 +236,7 @@ impl AgentTool for ListDirectoryTool {
}
anyhow::Ok(())
- })?;
+ }).map_err(|e| e.to_string())?;
if let Some(canonical_target) = &symlink_canonical_target {
let authorize = cx.update(|cx| {
@@ -248,13 +248,13 @@ impl AgentTool for ListDirectoryTool {
cx,
)
});
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let list_path = input.path;
cx.update(|cx| {
Self::build_directory_output(&project, &project_path, &list_path, cx)
- })
+ }).map_err(|e| e.to_string())
})
}
}
@@ -422,7 +422,7 @@ mod tests {
let output = cx
.update(|cx| tool.clone().run(input, ToolCallEventStream::test().0, cx))
.await;
- assert!(output.unwrap_err().to_string().contains("Path not found"));
+ assert!(output.unwrap_err().contains("Path not found"));
// Test trying to list a file instead of directory
let input = ListDirectoryToolInput {
@@ -431,12 +431,7 @@ mod tests {
let output = cx
.update(|cx| tool.run(input, ToolCallEventStream::test().0, cx))
.await;
- assert!(
- output
- .unwrap_err()
- .to_string()
- .contains("is not a directory")
- );
+ assert!(output.unwrap_err().contains("is not a directory"));
}
#[gpui::test]
@@ -528,10 +523,7 @@ mod tests {
.update(|cx| tool.clone().run(input, ToolCallEventStream::test().0, cx))
.await;
assert!(
- output
- .unwrap_err()
- .to_string()
- .contains("file_scan_exclusions"),
+ output.unwrap_err().contains("file_scan_exclusions"),
"Error should mention file_scan_exclusions"
);
@@ -711,12 +703,7 @@ mod tests {
let output = cx
.update(|cx| tool.clone().run(input, ToolCallEventStream::test().0, cx))
.await;
- assert!(
- output
- .unwrap_err()
- .to_string()
- .contains("Cannot list directory"),
- );
+ assert!(output.unwrap_err().contains("Cannot list directory"),);
}
#[gpui::test]
@@ -897,7 +884,7 @@ mod tests {
result.is_err(),
"Expected list_directory to fail on private path"
);
- let error = result.unwrap_err().to_string();
+ let error = result.unwrap_err();
assert!(
error.contains("private"),
"Expected private path validation error, got: {error}"
@@ -5,7 +5,6 @@ use super::tool_permissions::{
use crate::{AgentTool, ToolCallEventStream, ToolPermissionDecision, decide_permission_for_paths};
use agent_client_protocol::ToolKind;
use agent_settings::AgentSettings;
-use anyhow::{Context as _, Result, anyhow};
use futures::FutureExt as _;
use gpui::{App, Entity, SharedString, Task};
use project::Project;
@@ -96,12 +95,12 @@ impl AgentTool for MovePathTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let settings = AgentSettings::get_global(cx);
let paths = vec![input.source_path.clone(), input.destination_path.clone()];
let decision = decide_permission_for_paths(Self::NAME, &paths, settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
let project = self.project.clone();
@@ -160,7 +159,7 @@ impl AgentTool for MovePathTool {
};
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let rename_task = project.update(cx, |project, cx| {
@@ -170,27 +169,24 @@ impl AgentTool for MovePathTool {
{
Some(entity) => match project.find_project_path(&input.destination_path, cx) {
Some(project_path) => Ok(project.rename_entry(entity.id, project_path, cx)),
- None => Err(anyhow!(
+ None => Err(format!(
"Destination path {} was outside the project.",
input.destination_path
)),
},
- None => Err(anyhow!(
+ None => Err(format!(
"Source path {} was not found in the project.",
input.source_path
)),
}
})?;
- let result = futures::select! {
- result = rename_task.fuse() => result,
+ futures::select! {
+ result = rename_task.fuse() => result.map_err(|e| format!("Moving {} to {}: {e}", input.source_path, input.destination_path))?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Move cancelled by user");
+ return Err("Move cancelled by user".to_string());
}
};
- result.with_context(|| {
- format!("Moving {} to {}", input.source_path, input.destination_path)
- })?;
Ok(format!(
"Moved {} to {}",
input.source_path, input.destination_path
@@ -1,7 +1,6 @@
use std::sync::Arc;
use agent_client_protocol as acp;
-use anyhow::Result;
use chrono::{Local, Utc};
use gpui::{App, SharedString, Task};
use schemars::JsonSchema;
@@ -52,7 +51,7 @@ impl AgentTool for NowTool {
input: Self::Input,
_event_stream: ToolCallEventStream,
_cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
let now = match input.timezone {
Timezone::Utc => Utc::now().to_rfc3339(),
Timezone::Local => Local::now().to_rfc3339(),
@@ -4,7 +4,6 @@ use super::tool_permissions::{
};
use crate::AgentTool;
use agent_client_protocol::ToolKind;
-use anyhow::{Context as _, Result};
use futures::FutureExt as _;
use gpui::{App, AppContext as _, Entity, SharedString, Task};
use project::Project;
@@ -65,7 +64,7 @@ impl AgentTool for OpenTool {
input: Self::Input,
event_stream: crate::ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
// If path_or_url turns out to be a path in the project, make it absolute.
let abs_path = to_absolute_path(&input.path_or_url, self.project.clone(), cx);
let initial_title = self.initial_title(Ok(input.clone()), cx);
@@ -114,9 +113,9 @@ impl AgentTool for OpenTool {
};
futures::select! {
- result = authorize.fuse() => result?,
+ result = authorize.fuse() => result.map_err(|e| e.to_string())?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Open cancelled by user");
+ return Err("Open cancelled by user".to_string());
}
}
@@ -126,7 +125,7 @@ impl AgentTool for OpenTool {
Some(path) => open::that(path),
None => open::that(path_or_url),
}
- .context("Failed to open URL or file path")
+ .map_err(|e| format!("Failed to open URL or file path: {e}"))
})
.await?;
@@ -13,6 +13,10 @@ use settings::Settings;
use std::sync::Arc;
use util::markdown::MarkdownCodeBlock;
+fn tool_content_err(e: impl std::fmt::Display) -> LanguageModelToolResultContent {
+ LanguageModelToolResultContent::from(e.to_string())
+}
+
use super::tool_permissions::{
ResolvedProjectPath, authorize_symlink_access, canonicalize_worktree_roots,
resolve_project_path,
@@ -113,7 +117,7 @@ impl AgentTool for ReadFileTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<LanguageModelToolResultContent>> {
+ ) -> Task<Result<LanguageModelToolResultContent, LanguageModelToolResultContent>> {
let project = self.project.clone();
let thread = self.thread.clone();
let action_log = self.action_log.clone();
@@ -132,7 +136,7 @@ impl AgentTool for ReadFileTool {
canonical_target,
} => (project_path, Some(canonical_target)),
})
- })?;
+ }).map_err(tool_content_err)?;
let abs_path = project
.read_with(cx, |project, cx| {
@@ -140,7 +144,7 @@ impl AgentTool for ReadFileTool {
})
.ok_or_else(|| {
anyhow!("Failed to convert {} to absolute path", &input.path)
- })?;
+ }).map_err(tool_content_err)?;
// Check settings exclusions synchronously
project.read_with(cx, |_project, cx| {
@@ -175,7 +179,7 @@ impl AgentTool for ReadFileTool {
}
anyhow::Ok(())
- })?;
+ }).map_err(tool_content_err)?;
if let Some(canonical_target) = &symlink_canonical_target {
let authorize = cx.update(|cx| {
@@ -187,7 +191,7 @@ impl AgentTool for ReadFileTool {
cx,
)
});
- authorize.await?;
+ authorize.await.map_err(tool_content_err)?;
}
let file_path = input.path.clone();
@@ -211,7 +215,7 @@ impl AgentTool for ReadFileTool {
project.open_image(project_path.clone(), cx)
})
})
- .await?;
+ .await.map_err(tool_content_err)?;
let image =
image_entity.read_with(cx, |image_item, _| Arc::clone(&image_item.image));
@@ -219,7 +223,8 @@ impl AgentTool for ReadFileTool {
let language_model_image = cx
.update(|cx| LanguageModelImage::from_image(image, cx))
.await
- .context("processing image")?;
+ .context("processing image")
+ .map_err(tool_content_err)?;
event_stream.update_fields(ToolCallUpdateFields::new().content(vec![
acp::ToolCallContent::Content(acp::Content::new(acp::ContentBlock::Image(
@@ -235,9 +240,9 @@ impl AgentTool for ReadFileTool {
});
let buffer = futures::select! {
- result = open_buffer_task.fuse() => result?,
+ result = open_buffer_task.fuse() => result.map_err(tool_content_err)?,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("File read cancelled by user");
+ return Err(tool_content_err("File read cancelled by user"));
}
};
if buffer.read_with(cx, |buffer, _| {
@@ -246,7 +251,7 @@ impl AgentTool for ReadFileTool {
.as_ref()
.is_none_or(|file| !file.disk_state().exists())
}) {
- anyhow::bail!("{file_path} not found");
+ return Err(tool_content_err(format!("{file_path} not found")));
}
// Record the file read time and mtime
@@ -294,7 +299,7 @@ impl AgentTool for ReadFileTool {
Some(&abs_path.to_string_lossy()),
cx,
)
- .await?;
+ .await.map_err(tool_content_err)?;
action_log.update(cx, |log, cx| {
log.buffer_read(buffer.clone(), cx);
@@ -397,7 +402,7 @@ mod test {
})
.await;
assert_eq!(
- result.unwrap_err().to_string(),
+ error_text(result.unwrap_err()),
"root/nonexistent_file.txt not found"
);
}
@@ -640,6 +645,13 @@ mod test {
assert_eq!(result.unwrap(), "Line 3\n".into());
}
+ fn error_text(content: LanguageModelToolResultContent) -> String {
+ match content {
+ LanguageModelToolResultContent::Text(text) => text.to_string(),
+ other => panic!("Expected text error, got: {other:?}"),
+ }
+ }
+
fn init_test(cx: &mut TestAppContext) {
cx.update(|cx| {
let settings_store = SettingsStore::test(cx);
@@ -1051,10 +1063,7 @@ mod test {
assert!(result.is_err());
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("worktree `private_files` setting"),
+ error_text(result.unwrap_err()).contains("worktree `private_files` setting"),
"Error should mention worktree private_files setting"
);
@@ -1072,10 +1081,7 @@ mod test {
assert!(result.is_err());
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("worktree `file_scan_exclusions` setting"),
+ error_text(result.unwrap_err()).contains("worktree `file_scan_exclusions` setting"),
"Error should mention worktree file_scan_exclusions setting"
);
@@ -1111,10 +1117,7 @@ mod test {
assert!(result.is_err());
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("worktree `private_files` setting"),
+ error_text(result.unwrap_err()).contains("worktree `private_files` setting"),
"Error should mention worktree private_files setting"
);
@@ -1132,10 +1135,7 @@ mod test {
assert!(result.is_err());
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("worktree `file_scan_exclusions` setting"),
+ error_text(result.unwrap_err()).contains("worktree `file_scan_exclusions` setting"),
"Error should mention worktree file_scan_exclusions setting"
);
@@ -1154,10 +1154,7 @@ mod test {
assert!(result.is_err());
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("worktree `private_files` setting"),
+ error_text(result.unwrap_err()).contains("worktree `private_files` setting"),
"Config.toml should be blocked by worktree1's private_files setting"
);
}
@@ -1385,7 +1382,7 @@ mod test {
result.is_err(),
"Expected read_file to fail on private path"
);
- let error = result.unwrap_err().to_string();
+ let error = error_text(result.unwrap_err());
assert!(
error.contains("private_files"),
"Expected private-files validation error, got: {error}"
@@ -5,7 +5,6 @@ use super::tool_permissions::{
};
use agent_client_protocol as acp;
use agent_settings::AgentSettings;
-use anyhow::Result;
use collections::FxHashSet;
use futures::FutureExt as _;
use gpui::{App, Entity, SharedString, Task};
@@ -70,7 +69,7 @@ impl AgentTool for RestoreFileFromDiskTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
let settings = AgentSettings::get_global(cx).clone();
// Check for any immediate deny before spawning async work.
@@ -78,7 +77,7 @@ impl AgentTool for RestoreFileFromDiskTool {
let path_str = path.to_string_lossy();
let decision = decide_permission_for_path(Self::NAME, &path_str, &settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
}
@@ -112,7 +111,7 @@ impl AgentTool for RestoreFileFromDiskTool {
}
}
ToolPermissionDecision::Deny(reason) => {
- return Err(anyhow::anyhow!("{}", reason));
+ return Err(reason);
}
ToolPermissionDecision::Confirm => {
if !symlink_escape {
@@ -159,7 +158,7 @@ impl AgentTool for RestoreFileFromDiskTool {
};
let context = crate::ToolPermissionContext::new(Self::NAME, confirmation_paths);
let authorize = cx.update(|cx| event_stream.authorize(title, context, cx));
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let mut buffers_to_reload: FxHashSet<Entity<Buffer>> = FxHashSet::default();
@@ -221,7 +220,7 @@ impl AgentTool for RestoreFileFromDiskTool {
}
}
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Restore cancelled by user");
+ return Err("Restore cancelled by user".to_string());
}
};
@@ -243,7 +242,7 @@ impl AgentTool for RestoreFileFromDiskTool {
let result = futures::select! {
result = reload_task.fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Restore cancelled by user");
+ return Err("Restore cancelled by user".to_string());
}
};
if let Err(error) = result {
@@ -1,6 +1,5 @@
use agent_client_protocol as acp;
use agent_settings::AgentSettings;
-use anyhow::Result;
use collections::FxHashSet;
use futures::FutureExt as _;
use gpui::{App, Entity, SharedString, Task};
@@ -67,7 +66,7 @@ impl AgentTool for SaveFileTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<String>> {
+ ) -> Task<Result<String, String>> {
let settings = AgentSettings::get_global(cx).clone();
// Check for any immediate deny before spawning async work.
@@ -75,7 +74,7 @@ impl AgentTool for SaveFileTool {
let path_str = path.to_string_lossy();
let decision = decide_permission_for_path(Self::NAME, &path_str, &settings);
if let ToolPermissionDecision::Deny(reason) = decision {
- return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
}
@@ -109,7 +108,7 @@ impl AgentTool for SaveFileTool {
}
}
ToolPermissionDecision::Deny(reason) => {
- return Err(anyhow::anyhow!("{}", reason));
+ return Err(reason);
}
ToolPermissionDecision::Confirm => {
if !symlink_escape {
@@ -154,7 +153,7 @@ impl AgentTool for SaveFileTool {
let context =
crate::ToolPermissionContext::new(Self::NAME, confirmation_paths.clone());
let authorize = cx.update(|cx| event_stream.authorize(title, context, cx));
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let mut buffers_to_save: FxHashSet<Entity<Buffer>> = FxHashSet::default();
@@ -217,7 +216,7 @@ impl AgentTool for SaveFileTool {
}
}
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Save cancelled by user");
+ return Err("Save cancelled by user".to_string());
}
};
@@ -247,7 +246,7 @@ impl AgentTool for SaveFileTool {
let save_result = futures::select! {
result = save_task.fuse() => result,
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Save cancelled by user");
+ return Err("Save cancelled by user".to_string());
}
};
if let Err(error) = save_result {
@@ -118,27 +118,45 @@ struct StreamingEditFileToolPartialInput {
}
#[derive(Debug, Serialize, Deserialize)]
-pub struct StreamingEditFileToolOutput {
- #[serde(alias = "original_path")]
- input_path: PathBuf,
- new_text: String,
- old_text: Arc<String>,
- #[serde(default)]
- diff: String,
+#[serde(untagged)]
+pub enum StreamingEditFileToolOutput {
+ Success {
+ #[serde(alias = "original_path")]
+ input_path: PathBuf,
+ new_text: String,
+ old_text: Arc<String>,
+ #[serde(default)]
+ diff: String,
+ },
+ Error {
+ error: String,
+ },
+}
+
+impl std::fmt::Display for StreamingEditFileToolOutput {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ StreamingEditFileToolOutput::Success {
+ diff, input_path, ..
+ } => {
+ if diff.is_empty() {
+ write!(f, "No edits were made.")
+ } else {
+ write!(
+ f,
+ "Edited {}:\n\n```diff\n{diff}\n```",
+ input_path.display()
+ )
+ }
+ }
+ StreamingEditFileToolOutput::Error { error } => write!(f, "{error}"),
+ }
+ }
}
impl From<StreamingEditFileToolOutput> for LanguageModelToolResultContent {
fn from(output: StreamingEditFileToolOutput) -> Self {
- if output.diff.is_empty() {
- "No edits were made.".into()
- } else {
- format!(
- "Edited {}:\n\n```diff\n{}\n```",
- output.input_path.display(),
- output.diff
- )
- .into()
- }
+ output.to_string().into()
}
}
@@ -253,17 +271,23 @@ impl AgentTool for StreamingEditFileTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let Ok(project) = self
.thread
.read_with(cx, |thread, _cx| thread.project().clone())
else {
- return Task::ready(Err(anyhow!("thread was dropped")));
+ return Task::ready(Err(StreamingEditFileToolOutput::Error {
+ error: "thread was dropped".to_string(),
+ }));
};
let project_path = match resolve_path(&input, project.clone(), cx) {
Ok(path) => path,
- Err(err) => return Task::ready(Err(anyhow!(err))),
+ Err(err) => {
+ return Task::ready(Err(StreamingEditFileToolOutput::Error {
+ error: err.to_string(),
+ }));
+ }
};
let abs_path = project.read(cx).absolute_path(&project_path, cx);
@@ -276,191 +300,195 @@ impl AgentTool for StreamingEditFileTool {
let authorize = self.authorize(&input, &event_stream, cx);
cx.spawn(async move |cx: &mut AsyncApp| {
- authorize.await?;
-
- let buffer = project
- .update(cx, |project, cx| {
- project.open_buffer(project_path.clone(), cx)
- })
- .await?;
+ let result: anyhow::Result<StreamingEditFileToolOutput> = async {
+ authorize.await?;
+
+ let buffer = project
+ .update(cx, |project, cx| {
+ project.open_buffer(project_path.clone(), cx)
+ })
+ .await?;
+
+ if let Some(abs_path) = abs_path.as_ref() {
+ let (last_read_mtime, current_mtime, is_dirty, has_save_tool, has_restore_tool) =
+ self.thread.update(cx, |thread, cx| {
+ let last_read = thread.file_read_times.get(abs_path).copied();
+ let current = buffer
+ .read(cx)
+ .file()
+ .and_then(|file| file.disk_state().mtime());
+ let dirty = buffer.read(cx).is_dirty();
+ let has_save = thread.has_tool(SaveFileTool::NAME);
+ let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME);
+ (last_read, current, dirty, has_save, has_restore)
+ })?;
+
+ if is_dirty {
+ let message = match (has_save_tool, has_restore_tool) {
+ (true, true) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
+ If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
+ }
+ (true, false) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
+ If they want to discard them, ask the user to manually revert the file, then inform you when it's ok to proceed."
+ }
+ (false, true) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
+ If they want to keep them, ask the user to manually save the file, then inform you when it's ok to proceed. \
+ If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
+ }
+ (false, false) => {
+ "This file has unsaved changes. Ask the user whether they want to keep or discard those changes, \
+ then ask them to save or revert the file manually and inform you when it's ok to proceed."
+ }
+ };
+ anyhow::bail!("{}", message);
+ }
- if let Some(abs_path) = abs_path.as_ref() {
- let (last_read_mtime, current_mtime, is_dirty, has_save_tool, has_restore_tool) =
- self.thread.update(cx, |thread, cx| {
- let last_read = thread.file_read_times.get(abs_path).copied();
- let current = buffer
- .read(cx)
- .file()
- .and_then(|file| file.disk_state().mtime());
- let dirty = buffer.read(cx).is_dirty();
- let has_save = thread.has_tool(SaveFileTool::NAME);
- let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME);
- (last_read, current, dirty, has_save, has_restore)
- })?;
-
- if is_dirty {
- let message = match (has_save_tool, has_restore_tool) {
- (true, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (true, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask for confirmation then use the save_file tool to save the file, then retry this edit. \
- If they want to discard them, ask the user to manually revert the file, then inform you when it's ok to proceed."
- }
- (false, true) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes. \
- If they want to keep them, ask the user to manually save the file, then inform you when it's ok to proceed. \
- If they want to discard them, ask for confirmation then use the restore_file_from_disk tool to restore the on-disk contents, then retry this edit."
- }
- (false, false) => {
- "This file has unsaved changes. Ask the user whether they want to keep or discard those changes, \
- then ask them to save or revert the file manually and inform you when it's ok to proceed."
+ if let (Some(last_read), Some(current)) = (last_read_mtime, current_mtime) {
+ if current != last_read {
+ anyhow::bail!(
+ "The file {} has been modified since you last read it. \
+ Please read the file again to get the current state before editing it.",
+ input.path.display()
+ );
}
- };
- anyhow::bail!("{}", message);
- }
-
- if let (Some(last_read), Some(current)) = (last_read_mtime, current_mtime) {
- if current != last_read {
- anyhow::bail!(
- "The file {} has been modified since you last read it. \
- Please read the file again to get the current state before editing it.",
- input.path.display()
- );
}
}
- }
-
- let diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
- event_stream.update_diff(diff.clone());
- let _finalize_diff = util::defer({
- let diff = diff.downgrade();
- let mut cx = cx.clone();
- move || {
- diff.update(&mut cx, |diff, cx| diff.finalize(cx)).ok();
- }
- });
-
- let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
- let old_text = cx
- .background_spawn({
- let old_snapshot = old_snapshot.clone();
- async move { Arc::new(old_snapshot.text()) }
- })
- .await;
- let action_log = self.thread.read_with(cx, |thread, _cx| thread.action_log().clone())?;
+ let diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
+ event_stream.update_diff(diff.clone());
+ let _finalize_diff = util::defer({
+ let diff = diff.downgrade();
+ let mut cx = cx.clone();
+ move || {
+ diff.update(&mut cx, |diff, cx| diff.finalize(cx)).ok();
+ }
+ });
- // Edit the buffer and report edits to the action log as part of the
- // same effect cycle, otherwise the edit will be reported as if the
- // user made it (due to the buffer subscription in action_log).
- match input.mode {
- StreamingEditFileMode::Create | StreamingEditFileMode::Overwrite => {
- action_log.update(cx, |log, cx| {
- log.buffer_created(buffer.clone(), cx);
- });
- let content = input.content.ok_or_else(|| {
- anyhow!("'content' field is required for create and overwrite modes")
- })?;
- cx.update(|cx| {
- buffer.update(cx, |buffer, cx| {
- buffer.edit([(0..buffer.len(), content.as_str())], None, cx);
+ let old_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+ let old_text = cx
+ .background_spawn({
+ let old_snapshot = old_snapshot.clone();
+ async move { Arc::new(old_snapshot.text()) }
+ })
+ .await;
+
+ let action_log = self.thread.read_with(cx, |thread, _cx| thread.action_log().clone())?;
+
+ // Edit the buffer and report edits to the action log as part of the
+ // same effect cycle, otherwise the edit will be reported as if the
+ // user made it (due to the buffer subscription in action_log).
+ match input.mode {
+ StreamingEditFileMode::Create | StreamingEditFileMode::Overwrite => {
+ action_log.update(cx, |log, cx| {
+ log.buffer_created(buffer.clone(), cx);
+ });
+ let content = input.content.ok_or_else(|| {
+ anyhow!("'content' field is required for create and overwrite modes")
+ })?;
+ cx.update(|cx| {
+ buffer.update(cx, |buffer, cx| {
+ buffer.edit([(0..buffer.len(), content.as_str())], None, cx);
+ });
+ action_log.update(cx, |log, cx| {
+ log.buffer_edited(buffer.clone(), cx);
+ });
});
+ }
+ StreamingEditFileMode::Edit => {
action_log.update(cx, |log, cx| {
- log.buffer_edited(buffer.clone(), cx);
+ log.buffer_read(buffer.clone(), cx);
});
- });
+ let edits = input.edits.ok_or_else(|| {
+ anyhow!("'edits' field is required for edit mode")
+ })?;
+ // apply_edits now handles buffer_edited internally in the same effect cycle
+ apply_edits(&buffer, &action_log, &edits, &diff, &event_stream, &abs_path, cx)?;
+ }
}
- StreamingEditFileMode::Edit => {
+
+ let format_on_save_enabled = buffer.read_with(cx, |buffer, cx| {
+ let settings = language_settings::language_settings(
+ buffer.language().map(|l| l.name()),
+ buffer.file(),
+ cx,
+ );
+ settings.format_on_save != FormatOnSave::Off
+ });
+
+ if format_on_save_enabled {
action_log.update(cx, |log, cx| {
- log.buffer_read(buffer.clone(), cx);
+ log.buffer_edited(buffer.clone(), cx);
});
- let edits = input.edits.ok_or_else(|| {
- anyhow!("'edits' field is required for edit mode")
- })?;
- // apply_edits now handles buffer_edited internally in the same effect cycle
- apply_edits(&buffer, &action_log, &edits, &diff, &event_stream, &abs_path, cx)?;
- }
- }
- let format_on_save_enabled = buffer.read_with(cx, |buffer, cx| {
- let settings = language_settings::language_settings(
- buffer.language().map(|l| l.name()),
- buffer.file(),
- cx,
- );
- settings.format_on_save != FormatOnSave::Off
- });
-
- if format_on_save_enabled {
- action_log.update(cx, |log, cx| {
- log.buffer_edited(buffer.clone(), cx);
- });
+ let format_task = project.update(cx, |project, cx| {
+ project.format(
+ HashSet::from_iter([buffer.clone()]),
+ LspFormatTarget::Buffers,
+ false,
+ FormatTrigger::Save,
+ cx,
+ )
+ });
+ futures::select! {
+ result = format_task.fuse() => { result.log_err(); },
+ _ = event_stream.cancelled_by_user().fuse() => {
+ anyhow::bail!("Edit cancelled by user");
+ }
+ };
+ }
- let format_task = project.update(cx, |project, cx| {
- project.format(
- HashSet::from_iter([buffer.clone()]),
- LspFormatTarget::Buffers,
- false,
- FormatTrigger::Save,
- cx,
- )
- });
+ let save_task = project
+ .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx));
futures::select! {
- result = format_task.fuse() => { result.log_err(); },
+ result = save_task.fuse() => { result?; },
_ = event_stream.cancelled_by_user().fuse() => {
anyhow::bail!("Edit cancelled by user");
}
};
- }
-
- let save_task = project
- .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx));
- futures::select! {
- result = save_task.fuse() => { result?; },
- _ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Edit cancelled by user");
- }
- };
-
- action_log.update(cx, |log, cx| {
- log.buffer_edited(buffer.clone(), cx);
- });
- if let Some(abs_path) = abs_path.as_ref() {
- if let Some(new_mtime) = buffer.read_with(cx, |buffer, _| {
- buffer.file().and_then(|file| file.disk_state().mtime())
- }) {
- self.thread.update(cx, |thread, _| {
- thread.file_read_times.insert(abs_path.to_path_buf(), new_mtime);
- })?;
- }
- }
+ action_log.update(cx, |log, cx| {
+ log.buffer_edited(buffer.clone(), cx);
+ });
- let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
- let (new_text, unified_diff) = cx
- .background_spawn({
- let new_snapshot = new_snapshot.clone();
- let old_text = old_text.clone();
- async move {
- let new_text = new_snapshot.text();
- let diff = language::unified_diff(&old_text, &new_text);
- (new_text, diff)
+ if let Some(abs_path) = abs_path.as_ref() {
+ if let Some(new_mtime) = buffer.read_with(cx, |buffer, _| {
+ buffer.file().and_then(|file| file.disk_state().mtime())
+ }) {
+ self.thread.update(cx, |thread, _| {
+ thread.file_read_times.insert(abs_path.to_path_buf(), new_mtime);
+ })?;
}
- })
- .await;
+ }
- let output = StreamingEditFileToolOutput {
- input_path: input.path,
- new_text,
- old_text,
- diff: unified_diff,
- };
+ let new_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+ let (new_text, unified_diff) = cx
+ .background_spawn({
+ let new_snapshot = new_snapshot.clone();
+ let old_text = old_text.clone();
+ async move {
+ let new_text = new_snapshot.text();
+ let diff = language::unified_diff(&old_text, &new_text);
+ (new_text, diff)
+ }
+ })
+ .await;
+
+ let output = StreamingEditFileToolOutput::Success {
+ input_path: input.path,
+ new_text,
+ old_text,
+ diff: unified_diff,
+ };
- Ok(output)
+ Ok(output)
+ }.await;
+ result
+ .map_err(|e| StreamingEditFileToolOutput::Error { error: e.to_string() })
})
}
@@ -471,16 +499,26 @@ impl AgentTool for StreamingEditFileTool {
event_stream: ToolCallEventStream,
cx: &mut App,
) -> Result<()> {
- event_stream.update_diff(cx.new(|cx| {
- Diff::finalized(
- output.input_path.to_string_lossy().into_owned(),
- Some(output.old_text.to_string()),
- output.new_text,
- self.language_registry.clone(),
- cx,
- )
- }));
- Ok(())
+ match output {
+ StreamingEditFileToolOutput::Success {
+ input_path,
+ old_text,
+ new_text,
+ ..
+ } => {
+ event_stream.update_diff(cx.new(|cx| {
+ Diff::finalized(
+ input_path.to_string_lossy().into_owned(),
+ Some(old_text.to_string()),
+ new_text,
+ self.language_registry.clone(),
+ cx,
+ )
+ }));
+ Ok(())
+ }
+ StreamingEditFileToolOutput::Error { .. } => Ok(()),
+ }
}
}
@@ -755,10 +793,11 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
- assert_eq!(output.new_text, "Hello, World!");
- assert!(!output.diff.is_empty());
+ let StreamingEditFileToolOutput::Success { new_text, diff, .. } = result.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "Hello, World!");
+ assert!(!diff.is_empty());
}
#[gpui::test]
@@ -803,10 +842,14 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
- assert_eq!(output.new_text, "new content");
- assert_eq!(*output.old_text, "old content");
+ let StreamingEditFileToolOutput::Success {
+ new_text, old_text, ..
+ } = result.unwrap()
+ else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "new content");
+ assert_eq!(*old_text, "old content");
}
#[gpui::test]
@@ -859,9 +902,10 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
- assert_eq!(output.new_text, "line 1\nmodified line 2\nline 3\n");
+ let StreamingEditFileToolOutput::Success { new_text, .. } = result.unwrap() else {
+ panic!("expected success");
+ };
+ assert_eq!(new_text, "line 1\nmodified line 2\nline 3\n");
}
#[gpui::test]
@@ -920,10 +964,11 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
+ let StreamingEditFileToolOutput::Success { new_text, .. } = result.unwrap() else {
+ panic!("expected success");
+ };
assert_eq!(
- output.new_text,
+ new_text,
"modified line 1\nline 2\nline 3\nline 4\nmodified line 5\n"
);
}
@@ -984,10 +1029,11 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
+ let StreamingEditFileToolOutput::Success { new_text, .. } = result.unwrap() else {
+ panic!("expected success");
+ };
assert_eq!(
- output.new_text,
+ new_text,
"line 1\nmodified line 2\nmodified line 3\nline 4\nline 5\n"
);
}
@@ -1048,10 +1094,11 @@ mod tests {
})
.await;
- assert!(result.is_ok());
- let output = result.unwrap();
+ let StreamingEditFileToolOutput::Success { new_text, .. } = result.unwrap() else {
+ panic!("expected success");
+ };
assert_eq!(
- output.new_text,
+ new_text,
"modified line 1\nline 2\nline 3\nline 4\nmodified line 5\n"
);
}
@@ -1100,10 +1147,10 @@ mod tests {
})
.await;
- assert_eq!(
- result.unwrap_err().to_string(),
- "Can't edit file: path not found"
- );
+ let StreamingEditFileToolOutput::Error { error } = result.unwrap_err() else {
+ panic!("expected error");
+ };
+ assert_eq!(error, "Can't edit file: path not found");
}
#[gpui::test]
@@ -1151,12 +1198,12 @@ mod tests {
})
.await;
- assert!(result.is_err());
+ let StreamingEditFileToolOutput::Error { error } = result.unwrap_err() else {
+ panic!("expected error");
+ };
assert!(
- result
- .unwrap_err()
- .to_string()
- .contains("Could not find matching text")
+ error.contains("Could not find matching text"),
+ "Expected error containing 'Could not find matching text' but got: {error}"
);
}
@@ -1221,11 +1268,12 @@ mod tests {
})
.await;
- let error = result.unwrap_err();
- let error_message = error.to_string();
+ let StreamingEditFileToolOutput::Error { error } = result.unwrap_err() else {
+ panic!("expected error");
+ };
assert!(
- error_message.contains("Conflicting edit ranges detected"),
- "Expected 'Conflicting edit ranges detected' but got: {error_message}"
+ error.contains("Conflicting edit ranges detected"),
+ "Expected 'Conflicting edit ranges detected' but got: {error}"
);
}
@@ -1,6 +1,6 @@
use acp_thread::SUBAGENT_SESSION_ID_META_KEY;
use agent_client_protocol as acp;
-use anyhow::{Result, anyhow};
+use anyhow::Result;
use gpui::{App, SharedString, Task, WeakEntity};
use language_model::LanguageModelToolResultContent;
use schemars::JsonSchema;
@@ -36,16 +36,25 @@ pub struct SubagentToolInput {
}
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
-pub struct SubagentToolOutput {
- pub session_id: acp::SessionId,
- pub output: String,
+#[serde(untagged)]
+pub enum SubagentToolOutput {
+ Success {
+ session_id: acp::SessionId,
+ output: String,
+ },
+ Error {
+ error: String,
+ },
}
impl From<SubagentToolOutput> for LanguageModelToolResultContent {
fn from(output: SubagentToolOutput) -> Self {
- serde_json::to_string(&output)
- .expect("Failed to serialize SubagentToolOutput")
- .into()
+ match output {
+ output @ SubagentToolOutput::Success { .. } => serde_json::to_string(&output)
+ .unwrap_or_else(|e| format!("Failed to serialize subagent output: {e}"))
+ .into(),
+ SubagentToolOutput::Error { error } => error.into(),
+ }
}
}
@@ -89,9 +98,11 @@ impl AgentTool for SubagentTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<SubagentToolOutput>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let Some(parent_thread_entity) = self.parent_thread.upgrade() else {
- return Task::ready(Err(anyhow!("Parent thread no longer exists")));
+ return Task::ready(Err(SubagentToolOutput::Error {
+ error: "Parent thread no longer exists".to_string(),
+ }));
};
let subagent = match self.environment.create_subagent(
@@ -102,7 +113,11 @@ impl AgentTool for SubagentTool {
cx,
) {
Ok(subagent) => subagent,
- Err(err) => return Task::ready(Err(err)),
+ Err(err) => {
+ return Task::ready(Err(SubagentToolOutput::Error {
+ error: err.to_string(),
+ }));
+ }
};
let subagent_session_id = subagent.id();
@@ -115,8 +130,14 @@ impl AgentTool for SubagentTool {
event_stream.update_fields_with_meta(acp::ToolCallUpdateFields::new(), Some(meta));
cx.spawn(async move |cx| {
- let output = subagent.wait_for_output(cx).await?;
- Ok(SubagentToolOutput {
+ let output =
+ subagent
+ .wait_for_output(cx)
+ .await
+ .map_err(|e| SubagentToolOutput::Error {
+ error: e.to_string(),
+ })?;
+ Ok(SubagentToolOutput::Success {
session_id: subagent_session_id,
output,
})
@@ -130,12 +151,17 @@ impl AgentTool for SubagentTool {
event_stream: ToolCallEventStream,
_cx: &mut App,
) -> Result<()> {
- event_stream.subagent_spawned(output.session_id.clone());
- let meta = acp::Meta::from_iter([(
- SUBAGENT_SESSION_ID_META_KEY.into(),
- output.session_id.to_string().into(),
- )]);
- event_stream.update_fields_with_meta(acp::ToolCallUpdateFields::new(), Some(meta));
+ match output {
+ SubagentToolOutput::Success { session_id, .. } => {
+ event_stream.subagent_spawned(session_id.clone());
+ let meta = acp::Meta::from_iter([(
+ SUBAGENT_SESSION_ID_META_KEY.into(),
+ session_id.to_string().into(),
+ )]);
+ event_stream.update_fields_with_meta(acp::ToolCallUpdateFields::new(), Some(meta));
+ }
+ SubagentToolOutput::Error { .. } => {}
+ }
Ok(())
}
}
@@ -88,10 +88,10 @@ impl AgentTool for TerminalTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let working_dir = match working_dir(&input, &self.project, cx) {
Ok(dir) => dir,
- Err(err) => return Task::ready(Err(err)),
+ Err(err) => return Task::ready(Err(err.to_string())),
};
let settings = AgentSettings::get_global(cx);
@@ -104,7 +104,7 @@ impl AgentTool for TerminalTool {
let authorize = match decision {
ToolPermissionDecision::Allow => None,
ToolPermissionDecision::Deny(reason) => {
- return Task::ready(Err(anyhow::anyhow!("{}", reason)));
+ return Task::ready(Err(reason));
}
ToolPermissionDecision::Confirm => {
let context =
@@ -114,7 +114,7 @@ impl AgentTool for TerminalTool {
};
cx.spawn(async move |cx| {
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| e.to_string())?;
}
let terminal = self
@@ -125,9 +125,10 @@ impl AgentTool for TerminalTool {
Some(COMMAND_OUTPUT_LIMIT),
cx,
)
- .await?;
+ .await
+ .map_err(|e| e.to_string())?;
- let terminal_id = terminal.id(cx)?;
+ let terminal_id = terminal.id(cx).map_err(|e| e.to_string())?;
event_stream.update_fields(acp::ToolCallUpdateFields::new().content(vec![
acp::ToolCallContent::Terminal(acp::Terminal::new(terminal_id)),
]));
@@ -136,7 +137,7 @@ impl AgentTool for TerminalTool {
let mut timed_out = false;
let mut user_stopped_via_signal = false;
- let wait_for_exit = terminal.wait_for_exit(cx)?;
+ let wait_for_exit = terminal.wait_for_exit(cx).map_err(|e| e.to_string())?;
match timeout {
Some(timeout) => {
@@ -146,12 +147,12 @@ impl AgentTool for TerminalTool {
_ = wait_for_exit.clone().fuse() => {},
_ = timeout_task.fuse() => {
timed_out = true;
- terminal.kill(cx)?;
+ terminal.kill(cx).map_err(|e| e.to_string())?;
wait_for_exit.await;
}
_ = event_stream.cancelled_by_user().fuse() => {
user_stopped_via_signal = true;
- terminal.kill(cx)?;
+ terminal.kill(cx).map_err(|e| e.to_string())?;
wait_for_exit.await;
}
}
@@ -161,7 +162,7 @@ impl AgentTool for TerminalTool {
_ = wait_for_exit.clone().fuse() => {},
_ = event_stream.cancelled_by_user().fuse() => {
user_stopped_via_signal = true;
- terminal.kill(cx)?;
+ terminal.kill(cx).map_err(|e| e.to_string())?;
wait_for_exit.await;
}
}
@@ -178,7 +179,7 @@ impl AgentTool for TerminalTool {
let user_stopped_via_terminal = terminal.was_stopped_by_user(cx).unwrap_or(false);
let user_stopped = user_stopped_via_signal || user_stopped_via_terminal;
- let output = terminal.current_output(cx)?;
+ let output = terminal.current_output(cx).map_err(|e| e.to_string())?;
Ok(process_content(
output,
@@ -5,7 +5,7 @@ use crate::{
};
use agent_client_protocol as acp;
use agent_settings::AgentSettings;
-use anyhow::{Result, anyhow};
+use anyhow::Result;
use cloud_llm_client::WebSearchResponse;
use futures::FutureExt as _;
use gpui::{App, AppContext, Task};
@@ -29,14 +29,20 @@ pub struct WebSearchToolInput {
}
#[derive(Debug, Serialize, Deserialize)]
-#[serde(transparent)]
-pub struct WebSearchToolOutput(WebSearchResponse);
+#[serde(untagged)]
+pub enum WebSearchToolOutput {
+ Success(WebSearchResponse),
+ Error { error: String },
+}
impl From<WebSearchToolOutput> for LanguageModelToolResultContent {
fn from(value: WebSearchToolOutput) -> Self {
- serde_json::to_string(&value.0)
- .expect("Failed to serialize WebSearchResponse")
- .into()
+ match value {
+ WebSearchToolOutput::Success(response) => serde_json::to_string(&response)
+ .unwrap_or_else(|e| format!("Failed to serialize web search response: {e}"))
+ .into(),
+ WebSearchToolOutput::Error { error } => error.into(),
+ }
}
}
@@ -70,7 +76,7 @@ impl AgentTool for WebSearchTool {
input: Self::Input,
event_stream: ToolCallEventStream,
cx: &mut App,
- ) -> Task<Result<Self::Output>> {
+ ) -> Task<Result<Self::Output, Self::Output>> {
let settings = AgentSettings::get_global(cx);
let decision = decide_permission_from_settings(
Self::NAME,
@@ -81,7 +87,7 @@ impl AgentTool for WebSearchTool {
let authorize = match decision {
ToolPermissionDecision::Allow => None,
ToolPermissionDecision::Deny(reason) => {
- return Task::ready(Err(anyhow!("{}", reason)));
+ return Task::ready(Err(WebSearchToolOutput::Error { error: reason }));
}
ToolPermissionDecision::Confirm => {
let context =
@@ -95,13 +101,15 @@ impl AgentTool for WebSearchTool {
};
let Some(provider) = WebSearchRegistry::read_global(cx).active_provider() else {
- return Task::ready(Err(anyhow!("Web search is not available.")));
+ return Task::ready(Err(WebSearchToolOutput::Error {
+ error: "Web search is not available.".to_string(),
+ }));
};
let search_task = provider.search(input.query, cx);
cx.background_spawn(async move {
if let Some(authorize) = authorize {
- authorize.await?;
+ authorize.await.map_err(|e| WebSearchToolOutput::Error { error: e.to_string() })?;
}
let response = futures::select! {
@@ -111,17 +119,17 @@ impl AgentTool for WebSearchTool {
Err(err) => {
event_stream
.update_fields(acp::ToolCallUpdateFields::new().title("Web Search Failed"));
- return Err(err);
+ return Err(WebSearchToolOutput::Error { error: err.to_string() });
}
}
}
_ = event_stream.cancelled_by_user().fuse() => {
- anyhow::bail!("Web search cancelled by user");
+ return Err(WebSearchToolOutput::Error { error: "Web search cancelled by user".to_string() });
}
};
emit_update(&response, &event_stream);
- Ok(WebSearchToolOutput(response))
+ Ok(WebSearchToolOutput::Success(response))
})
}
@@ -132,7 +140,9 @@ impl AgentTool for WebSearchTool {
event_stream: ToolCallEventStream,
_cx: &mut App,
) -> Result<()> {
- emit_update(&output.0, &event_stream);
+ if let WebSearchToolOutput::Success(response) = &output {
+ emit_update(response, &event_stream);
+ }
Ok(())
}
}
@@ -2052,7 +2052,12 @@ fn run_agent_thread_view_test(
cx.background_executor.allow_parking();
let run_result = cx.foreground_executor.block_test(run_task);
cx.background_executor.forbid_parking();
- run_result.context("ReadFileTool failed")?;
+ run_result.map_err(|e| match e {
+ language_model::LanguageModelToolResultContent::Text(text) => {
+ anyhow::anyhow!("ReadFileTool failed: {text}")
+ }
+ other => anyhow::anyhow!("ReadFileTool failed: {other:?}"),
+ })?;
cx.run_until_parked();