ACP editor tools by jamadeo · Pull Request #7388 · block/goose

jamadeo · 2026-02-20T15:38:55Z

This adds an option to goose-acp, --acp-editor-tools, that replaces goose's standard developer tools with ACP based editor tools. These use ACP's file system and terminals methods to call back to the ACP client to perform file edits and shell commands.

This makes for a nicer experience when using goose over ACP in an editor like Zed.

Before:
https://github.com/user-attachments/assets/c3ee6a05-a4ba-411b-a881-02ae909b6274

After:
https://github.com/user-attachments/assets/247730ee-c83e-454f-b71c-b4b07630305a

Copilot

Pull request overview

This PR adds an --acp-editor-tools flag to goose-acp that replaces goose's standard developer tools with ACP-based editor tools. These tools use the Agent Client Protocol's file system and terminal methods to delegate file operations and shell commands back to the ACP client (like Zed editor), providing a better integrated experience.

Changes:

Introduces ToolCallContext struct to pass session ID, working directory, and tool call request ID through the tool dispatch chain
Refactors all call_tool implementations to accept context instead of individual parameters
Adds new ACP tools module implementing read, write, str_replace, insert, and shell tools using ACP protocol
Refactors text editor functions to separate in-memory logic from file I/O for reusability
Updates extension manager to expose tools unprefixed when only one client is active

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
crates/goose/src/agents/tool_execution.rs	Adds ToolCallContext struct to encapsulate tool call parameters
crates/goose/src/agents/extension_manager.rs	Updates dispatch_tool_call to use ToolCallContext; fixes tool prefixing logic
crates/goose/src/agents/agent.rs	Creates ToolCallContext when dispatching tool calls
crates/goose/src/agents/mcp_client.rs	Updates McpClientTrait call_tool signature to use ToolCallContext
crates/goose/src/agents/platform_extensions/*.rs	Updates all platform extension call_tool implementations
crates/goose-acp/src/tools.rs	New module implementing ACP-based file and shell tools
crates/goose-acp/src/server.rs	Adds acp_editor_tools flag and conditionally adds ACP tools
crates/goose-acp/src/server_factory.rs	Passes acp_editor_tools flag through configuration
crates/goose-mcp/src/developer/text_editor.rs	Refactors replace/insert to extract in-memory logic
crates/goose-cli/src/cli.rs	Adds CLI flag for acp_editor_tools
crates/goose-server/src/routes/agent.rs	Updates tool call dispatch to use ToolCallContext
All test files	Updates test code to construct ToolCallContext

Copilot · 2026-02-20T15:44:05Z

crates/goose-acp/src/server.rs

+        let is_acp_tool = tool_request
+            .and_then(|req| req.tool_call.as_ref().ok())
+            // TODO: something more robust here
+            .is_some_and(|req| {
+                req.name == "read"
+                    || req.name == "write"
+                    || req.name == "str_replace"
+                    || req.name == "insert"
+                    || req.name == "shell"
+            });


Hard-coding tool names for detection is fragile and creates a maintenance burden. Consider adding a method to AcpTools or a marker to identify these tools programmatically, or store the extension name in the tool metadata and check against "acp-tools" instead.

Copilot · 2026-02-20T15:44:06Z

crates/goose-acp/src/tools.rs

+    ($t:ty) => {
+        serde_json::to_value(schema_for!($t))
+            .unwrap()
+            .as_object()
+            .unwrap()
+            .clone()
+    };


The schema macro uses unwrap() which will panic at initialization if the schema conversion fails. While this is unlikely to fail for these simple parameter types, consider handling the error more gracefully or using a static assertion approach to catch schema issues at compile time rather than at runtime initialization.

Suggested change

($t:ty) => {

serde_json::to_value(schema_for!($t))

.unwrap()

.as_object()

.unwrap()

.clone()

};

($t:ty) => {{

let value = match serde_json::to_value(schema_for!($t)) {

Ok(v) => v,

Err(_) => serde_json::Value::Object(JsonObject::new()),

};

match value.as_object() {

Some(map) => map.clone(),

None => JsonObject::new(),

}

}};

Copilot · 2026-02-20T15:44:06Z

crates/goose-acp/src/tools.rs

@@ -0,0 +1,400 @@
+use std::{cell::LazyCell, fmt::Display, future::Future, path::PathBuf};


Replace std::cell::LazyCell with once_cell::sync::Lazy to match the codebase convention. The codebase consistently uses once_cell for lazy static initialization throughout (see usage in goose/src/agents/extension_manager.rs, goose-mcp/src/developer/rmcp_developer.rs, and many other files).

Suggested change

use std::{cell::LazyCell, fmt::Display, future::Future, path::PathBuf};

use std::{fmt::Display, future::Future, path::PathBuf};

use once_cell::sync::Lazy;

Copilot · 2026-02-20T15:44:06Z

crates/goose-acp/src/tools.rs

+        .map_err(|_| {
+            ServiceError::McpError(ErrorData::internal_error("failed to replace", None))


Error information is being discarded when mapping to internal error. The ErrorData from text_editor_replace_inmem contains useful details (like "'old_str' must appear exactly once") that would help users understand what went wrong. Consider propagating the original error message instead of using a generic "failed to replace" message.

Suggested change

.map_err(|_| {

ServiceError::McpError(ErrorData::internal_error("failed to replace", None))

.map_err(|e| {

ServiceError::McpError(ErrorData::internal_error(

&format!("failed to replace: {e}"),

None,

))

Copilot · 2026-02-20T15:44:07Z

crates/goose-acp/src/tools.rs

+            params.position,
+            &params.new_str.as_str(),
+        )
+        .map_err(|_| ServiceError::McpError(ErrorData::internal_error("failed to insert", None)))?;


Error information is being discarded when mapping to internal error. The ErrorData from text_editor_insert_inmem contains useful details (like "Insert line X is beyond the end of the file") that would help users understand what went wrong. Consider propagating the original error message instead of using a generic "failed to insert" message.

Suggested change

.map_err(|_| ServiceError::McpError(ErrorData::internal_error("failed to insert", None)))?;

.map_err(ServiceError::McpError)?;

Copilot · 2026-02-20T15:44:07Z

crates/goose-acp/src/tools.rs

+        params: WriteParams,
+    ) -> Result<CallToolResult, ServiceError> {
+        let path = self.working_dir.join(params.path);
+        let content = read_file(&path, &self.cx, self.session_id.clone()).await?;


The write operation reads the old file content at line 182, but this is unnecessary unless the file already exists. For new files, this will fail. Consider only reading the old content if the file exists, or handle the error case when the file doesn't exist (which is valid for a write operation).

Copilot · 2026-02-20T15:44:07Z

crates/goose-acp/src/tools.rs

+    }
+
+    fn get_info(&self) -> Option<&InitializeResult> {
+        todo!()


This unimplemented method will panic if called. Since this implements the McpClientTrait, get_info might be called by the framework. Either implement it with a proper value or return None if this information isn't available for ACP tools.

Suggested change

todo!()

None

jamadeo added 5 commits February 19, 2026 20:10

ACP tools

722a925

Pass tool call ID into tool context

4a33bc4

Hack around the shell

ba8b7a9

Merge remote-tracking branch 'origin/main' into jackamadeo/acp-tools

53b8115

More ACP messaging

cdf900e

Copilot AI review requested due to automatic review settings February 20, 2026 15:38

Copilot started reviewing on behalf of jamadeo February 20, 2026 15:39 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

lints

d57ad91

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

ACP editor tools#7388

ACP editor tools#7388
jamadeo wants to merge 6 commits intomainfrom
jackamadeo/acp-tools

jamadeo commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-    ($t:ty) => {
-        serde_json::to_value(schema_for!($t))
-            .unwrap()
-            .as_object()
-            .unwrap()
-            .clone()
-    };
+    ($t:ty) => {{
+        let value = match serde_json::to_value(schema_for!($t)) {
+            Ok(v) => v,
+            Err(_) => serde_json::Value::Object(JsonObject::new()),
+        };
+        match value.as_object() {
+            Some(map) => map.clone(),
+            None => JsonObject::new(),
+        }
+    }};

		@@ -0,0 +1,400 @@
		use std::{cell::LazyCell, fmt::Display, future::Future, path::PathBuf};

	use std::{cell::LazyCell, fmt::Display, future::Future, path::PathBuf};
	use std::{fmt::Display, future::Future, path::PathBuf};
	use once_cell::sync::Lazy;

		.map_err(\|_\| {
		ServiceError::McpError(ErrorData::internal_error("failed to replace", None))

	.map_err(\|_\| ServiceError::McpError(ErrorData::internal_error("failed to insert", None)))?;
	.map_err(ServiceError::McpError)?;

Comments

Conversation

jamadeo commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant