From f303a461c4a839b6dbd515dec0c3bc1dd4e88881 Mon Sep 17 00:00:00 2001
From: Ben Brandt <benjamin.j.brandt@gmail.com>
Date: Thu, 25 Sep 2025 13:53:36 +0200
Subject: [PATCH] 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
---
 Cargo.lock                          |  4 +-
 Cargo.toml                          |  2 +-
 crates/acp_thread/src/acp_thread.rs | 61 +++++++++++++++++++++++------
 3 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index f131e9ad52ff746650a633a8584042c8018370de..f31813e1fe6fe723bf4945846af80cc46d83ed65 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -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",
diff --git a/Cargo.toml b/Cargo.toml
index 4d51d0577051280305330a6481b0138027dd0b24..8beed19d7c9355cdc47e456a2801d5882acb18af 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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"
diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs
index e6e5e2ec7bbc577d448af4d1296a69f902c28064..185eed3cd7a89c8c0dad17979e34c7ffc5684e08 100644
--- a/crates/acp_thread/src/acp_thread.rs
+++ b/crates/acp_thread/src/acp_thread.rs
@@ -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) {