acp: Use ACP error types in read_text_file (#38863)

- Map path lookup and internal failures to acp::Error 
- Return INVALID_PARAMS for reads beyond EOF

Release Notes:

- acp: Return more informative error types from `read_text_file` to
agents
This commit is contained in:
Ben Brandt
2025-09-25 11:53:36 +00:00
committed by GitHub
parent a9def8128f
commit f303a461c4
3 changed files with 52 additions and 15 deletions
Generated
+2 -2
View File
@@ -195,9 +195,9 @@ dependencies = [
[[package]]
name = "agent-client-protocol"
version = "0.4.2"
version = "0.4.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "00e33b9f4bd34d342b6f80b7156d3a37a04aeec16313f264001e52d6a9118600"
checksum = "3aaa2bd05a2401887945f8bfd70026e90bc3cf96c62ab9eba2779835bf21dc60"
dependencies = [
"anyhow",
"async-broadcast",
+1 -1
View File
@@ -440,7 +440,7 @@ zlog_settings = { path = "crates/zlog_settings" }
# External crates
#
agent-client-protocol = { version = "0.4.2", features = ["unstable"] }
agent-client-protocol = { version = "0.4.3", features = ["unstable"] }
aho-corasick = "1.1"
alacritty_terminal = "0.25.1-rc1"
any_vec = "0.14"
+49 -12
View File
@@ -1780,20 +1780,26 @@ impl AcpThread {
limit: Option<u32>,
reuse_shared_snapshot: bool,
cx: &mut Context<Self>,
) -> Task<Result<String>> {
) -> Task<Result<String, acp::Error>> {
// Args are 1-based, move to 0-based
let line = line.unwrap_or_default().saturating_sub(1);
let limit = limit.unwrap_or(u32::MAX);
let project = self.project.clone();
let action_log = self.action_log.clone();
cx.spawn(async move |this, cx| {
let load = project.update(cx, |project, cx| {
let path = project
.project_path_for_absolute_path(&path, cx)
.context("invalid path")?;
anyhow::Ok(project.open_buffer(path, cx))
});
let buffer = load??.await?;
let load = project
.update(cx, |project, cx| {
let path = project
.project_path_for_absolute_path(&path, cx)
.ok_or_else(|| {
acp::Error::resource_not_found(Some(path.display().to_string()))
})?;
Ok(project.open_buffer(path, cx))
})
.map_err(|e| acp::Error::internal_error().with_data(e.to_string()))
.flatten()?;
let buffer = load.await?;
let snapshot = if reuse_shared_snapshot {
this.read_with(cx, |this, _| {
@@ -1823,11 +1829,11 @@ impl AcpThread {
let start_position = Point::new(line, 0);
if start_position > max_point {
anyhow::bail!(
return Err(acp::Error::invalid_params().with_data(format!(
"Attempting to read beyond the end of the file, line {}:{}",
max_point.row + 1,
max_point.column
);
)));
}
let start = snapshot.anchor_before(start_position);
@@ -2461,7 +2467,7 @@ mod tests {
assert_eq!(
err.to_string(),
"Attempting to read beyond the end of the file, line 5:0"
"Invalid params: \"Attempting to read beyond the end of the file, line 5:0\""
);
}
@@ -2536,9 +2542,40 @@ mod tests {
assert_eq!(
err.to_string(),
"Attempting to read beyond the end of the file, line 1:0"
"Invalid params: \"Attempting to read beyond the end of the file, line 1:0\""
);
}
#[gpui::test]
async fn test_reading_non_existing_file(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(path!("/tmp"), json!({})).await;
let project = Project::test(fs.clone(), [], cx).await;
project
.update(cx, |project, cx| {
project.find_or_create_worktree(path!("/tmp"), true, cx)
})
.await
.unwrap();
let connection = Rc::new(FakeAgentConnection::new());
let thread = cx
.update(|cx| connection.new_thread(project, Path::new(path!("/tmp")), cx))
.await
.unwrap();
// Out of project file
let err = thread
.update(cx, |thread, cx| {
thread.read_text_file(path!("/foo").into(), None, None, false, cx)
})
.await
.unwrap_err();
assert_eq!(err.code, acp::ErrorCode::RESOURCE_NOT_FOUND.code);
}
#[gpui::test]
async fn test_succeeding_canceled_toolcall(cx: &mut TestAppContext) {