diff --git a/crates/goose-acp/src/server.rs b/crates/goose-acp/src/server.rs index 9ebd0aba71d8..82b4e961cddc 100644 --- a/crates/goose-acp/src/server.rs +++ b/crates/goose-acp/src/server.rs @@ -2,6 +2,7 @@ use crate::custom_requests::*; use anyhow::Result; use fs_err as fs; use goose::agents::extension::{Envs, PLATFORM_EXTENSIONS}; +use goose::agents::platform_extensions::DeveloperFileIo; use goose::agents::{Agent, AgentConfig, ExtensionConfig, GoosePlatform, SessionConfig}; use goose::builtin_extension::register_builtin_extensions; use goose::config::base::CONFIG_YAML_NAME; @@ -22,21 +23,22 @@ use goose_acp_macros::custom_methods; use rmcp::model::{CallToolResult, RawContent, ResourceContents, Role}; use sacp::schema::{ AgentCapabilities, AuthMethod, AuthenticateRequest, AuthenticateResponse, BlobResourceContents, - CancelNotification, Content, ContentBlock, ContentChunk, EmbeddedResource, + CancelNotification, ClientCapabilities, Content, ContentBlock, ContentChunk, EmbeddedResource, EmbeddedResourceResource, ImageContent, InitializeRequest, InitializeResponse, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse, McpCapabilities, McpServer, ModelId, ModelInfo, NewSessionRequest, NewSessionResponse, PermissionOption, - PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, - RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, SessionCapabilities, - SessionId, SessionInfo, SessionListCapabilities, SessionModelState, SessionNotification, - SessionUpdate, SetSessionModelRequest, SetSessionModelResponse, StopReason, TextContent, - TextResourceContents, ToolCall, ToolCallContent, ToolCallId, ToolCallLocation, ToolCallStatus, - ToolCallUpdate, ToolCallUpdateFields, ToolKind, + PermissionOptionKind, PromptCapabilities, PromptRequest, PromptResponse, ReadTextFileRequest, + ReadTextFileResponse, RequestPermissionOutcome, RequestPermissionRequest, ResourceLink, + SessionCapabilities, SessionId, SessionInfo, SessionListCapabilities, SessionModelState, + SessionNotification, SessionUpdate, SetSessionModelRequest, SetSessionModelResponse, + StopReason, TextContent, TextResourceContents, ToolCall, ToolCallContent, ToolCallId, + ToolCallLocation, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, + WriteTextFileRequest, WriteTextFileResponse, }; use sacp::{AgentToClient, ByteStreams, Handled, JrConnectionCx, JrMessageHandler, MessageCx}; use std::collections::HashMap; use std::sync::Arc; -use tokio::sync::Mutex; +use tokio::sync::{Mutex, RwLock}; use tokio_util::compat::{TokioAsyncReadCompatExt as _, TokioAsyncWriteCompatExt as _}; use tokio_util::sync::CancellationToken; use tracing::{debug, error, info, warn}; @@ -60,6 +62,7 @@ pub struct GooseAcpAgent { goose_mode: goose::config::GooseMode, disable_session_naming: bool, builtins: Vec, + client_capabilities: RwLock, } fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result { @@ -329,18 +332,92 @@ impl GooseAcpAgent { goose_mode, disable_session_naming, builtins, + client_capabilities: RwLock::new(ClientCapabilities::default()), }) } - async fn create_agent_for_session(&self) -> Arc { - let agent = Agent::with_config(AgentConfig::new( - Arc::clone(&self.session_manager), - Arc::clone(&self.permission_manager), - None, - self.goose_mode, - self.disable_session_naming, - GoosePlatform::GooseCli, - )); + async fn build_developer_file_io( + &self, + session_id: &SessionId, + cx: &JrConnectionCx, + ) -> Option { + let caps = self.client_capabilities.read().await.clone(); + if !caps.fs.read_text_file && !caps.fs.write_text_file { + return None; + } + + let local_io = DeveloperFileIo::default_local(); + let read_file: goose::agents::platform_extensions::ReadFileFn = if caps.fs.read_text_file { + let read_cx = cx.clone(); + let read_session_id = session_id.clone(); + Arc::new(move |path: std::path::PathBuf| { + let read_cx = read_cx.clone(); + let session_id = read_session_id.clone(); + Box::pin(async move { + let response: ReadTextFileResponse = read_cx + .send_request(ReadTextFileRequest::new(session_id, path)) + .block_task() + .await + .map_err(|e| { + std::io::Error::other(format!("ACP read request failed: {e}")) + })?; + Ok(response.content) + }) as goose::agents::platform_extensions::ReadFileFuture + }) + } else { + local_io.read_file.clone() + }; + + let write_file: goose::agents::platform_extensions::WriteFileFn = if caps.fs.write_text_file + { + let write_cx = cx.clone(); + let write_session_id = session_id.clone(); + Arc::new(move |path: std::path::PathBuf, content: String| { + let write_cx = write_cx.clone(); + let session_id = write_session_id.clone(); + Box::pin(async move { + let _: WriteTextFileResponse = write_cx + .send_request(WriteTextFileRequest::new(session_id, path, content)) + .block_task() + .await + .map_err(|e| { + std::io::Error::other(format!("ACP write request failed: {e}")) + })?; + Ok(()) + }) as goose::agents::platform_extensions::WriteFileFuture + }) + } else { + local_io.write_file.clone() + }; + + Some(DeveloperFileIo { + read_file, + read_file_chunk: if caps.fs.read_text_file { + None + } else { + local_io.read_file_chunk.clone() + }, + write_file, + }) + } + + async fn create_agent_for_session( + &self, + session_id: &SessionId, + cx: &JrConnectionCx, + ) -> Arc { + let developer_file_io = self.build_developer_file_io(session_id, cx).await; + let agent = Agent::with_config( + AgentConfig::new( + Arc::clone(&self.session_manager), + Arc::clone(&self.permission_manager), + None, + self.goose_mode, + self.disable_session_naming, + GoosePlatform::GooseCli, + ) + .with_developer_file_io(developer_file_io), + ); let agent = Arc::new(agent); let config_path = self.config_dir.join(CONFIG_YAML_NAME); @@ -662,6 +739,16 @@ impl GooseAcpAgent { args: InitializeRequest, ) -> Result { debug!(?args, "initialize request"); + { + let mut caps = self.client_capabilities.write().await; + *caps = args.client_capabilities.clone(); + } + info!( + fs_read = args.client_capabilities.fs.read_text_file, + fs_write = args.client_capabilities.fs.write_text_file, + terminal = args.client_capabilities.terminal, + "Client capabilities received" + ); let capabilities = AgentCapabilities::new() .load_session(true) @@ -687,6 +774,7 @@ impl GooseAcpAgent { async fn on_new_session( &self, args: NewSessionRequest, + cx: &JrConnectionCx, ) -> Result { debug!(?args, "new session request"); @@ -702,7 +790,8 @@ impl GooseAcpAgent { sacp::Error::internal_error().data(format!("Failed to create session: {}", e)) })?; - let agent = self.create_agent_for_session().await; + let acp_session_id = SessionId::new(goose_session.id.clone()); + let agent = self.create_agent_for_session(&acp_session_id, cx).await; let provider = self .init_provider(&agent, &goose_session) .await @@ -743,7 +832,7 @@ impl GooseAcpAgent { let model_state = build_model_state(&*provider, &provider.get_model_config().model_name).await; - Ok(NewSessionResponse::new(SessionId::new(goose_session.id)).models(model_state)) + Ok(NewSessionResponse::new(acp_session_id).models(model_state)) } async fn init_provider(&self, agent: &Agent, session: &Session) -> Result> { @@ -780,7 +869,7 @@ impl GooseAcpAgent { .data(format!("Failed to load session {}: {}", session_id, e)) })?; - let agent = self.create_agent_for_session().await; + let agent = self.create_agent_for_session(&args.session_id, cx).await; let provider = self .init_provider(&agent, &goose_session) .await @@ -1235,7 +1324,7 @@ impl JrMessageHandler for GooseAcpHandler { .await .if_request( |req: NewSessionRequest, req_cx: JrRequestCx| async { - req_cx.respond(agent.on_new_session(req).await?) + req_cx.respond(agent.on_new_session(req, &cx).await?) }, ) .await diff --git a/crates/goose-acp/tests/common_tests/mod.rs b/crates/goose-acp/tests/common_tests/mod.rs index a2209c2c0466..11827a5c87e9 100644 --- a/crates/goose-acp/tests/common_tests/mod.rs +++ b/crates/goose-acp/tests/common_tests/mod.rs @@ -5,7 +5,8 @@ #[path = "../fixtures/mod.rs"] pub mod fixtures; use fixtures::{ - initialize_agent, Connection, OpenAiFixture, PermissionDecision, Session, TestConnectionConfig, + initialize_agent, server::ClientToAgentConnection, Connection, OpenAiFixture, + PermissionDecision, Session, TestConnectionConfig, }; use fs_err as fs; use goose::config::base::CONFIG_YAML_NAME; @@ -16,8 +17,88 @@ use goose_test_support::{ExpectedSessionId, McpFixture, FAKE_CODE, TEST_MODEL}; use sacp::schema::{ McpServer, McpServerHttp, ModelId, ModelInfo, SessionModelState, ToolCallStatus, }; +use serde_json::json; use std::sync::Arc; +const FS_READ_TEST_CONTENT: &str = "delegated-read-content"; +const FS_WRITE_DELEGATED_CONTENT: &str = "delegated write content"; +const FS_WRITE_LOCAL_CONTENT: &str = "local write content"; + +fn openai_tool_call_fixture( + tool_call_id: &str, + tool_name: &str, + arguments: serde_json::Value, +) -> String { + let args_json = arguments.to_string(); + let chunk_1 = json!({ + "id": "chatcmpl-test", + "object": "chat.completion.chunk", + "created": 1766229303, + "model": "gpt-5-nano", + "choices": [{ + "index": 0, + "delta": { + "role": "assistant", + "content": serde_json::Value::Null, + "tool_calls": [{ + "index": 0, + "id": tool_call_id, + "type": "function", + "function": { + "name": tool_name, + "arguments": "" + } + }] + }, + "finish_reason": serde_json::Value::Null + }] + }); + let chunk_2 = json!({ + "id": "chatcmpl-test", + "object": "chat.completion.chunk", + "created": 1766229303, + "model": "gpt-5-nano", + "choices": [{ + "index": 0, + "delta": { + "tool_calls": [{ + "index": 0, + "function": { + "arguments": args_json + } + }] + }, + "finish_reason": serde_json::Value::Null + }] + }); + let chunk_3 = json!({ + "id": "chatcmpl-test", + "object": "chat.completion.chunk", + "created": 1766229303, + "model": "gpt-5-nano", + "choices": [{ + "index": 0, + "delta": {}, + "finish_reason": "tool_calls" + }] + }); + let chunk_4 = json!({ + "id": "chatcmpl-test", + "object": "chat.completion.chunk", + "created": 1766229303, + "model": "gpt-5-nano", + "choices": [], + "usage": { + "prompt_tokens": 100, + "completion_tokens": 10, + "total_tokens": 110 + } + }); + format!( + "data: {chunk_1}\n\ndata: {chunk_2}\n\ndata: {chunk_3}\n\ndata: {chunk_4}\n\ndata: [DONE]\n" + ) +} + pub async fn run_config_mcp() { let temp_dir = tempfile::tempdir().unwrap(); let expected_session_id = ExpectedSessionId::default(); @@ -326,7 +407,7 @@ pub async fn run_prompt_codemode() { include_str!("../test_data/openai_builtin_execute.txt"), ), ( - r#"Created /tmp/result.txt"#.into(), + r#"Wrote /tmp/result.txt"#.into(), include_str!("../test_data/openai_builtin_final.txt"), ), ], @@ -428,3 +509,155 @@ pub async fn run_prompt_mcp() { assert_eq!(output.text, FAKE_CODE); expected_session_id.assert_matches(&session.session_id().0); } + +pub async fn run_fs_read_delegation_without_permission() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("read-delegation.txt"); + fs::write(&file_path, FS_READ_TEST_CONTENT).unwrap(); + let expected_session_id = ExpectedSessionId::default(); + let prompt = format!( + "Use edit on {} replacing not-present with replacement.", + file_path.display() + ); + let openai = OpenAiFixture::new_dynamic( + vec![ + ( + prompt.clone(), + openai_tool_call_fixture( + "call_read", + "edit", + json!({ + "path": file_path.to_string_lossy(), + "before": "not-present", + "after": "replacement" + }), + ), + ), + ( + FS_READ_TEST_CONTENT.to_string(), + include_str!("../test_data/openai_basic.txt").to_string(), + ), + ], + expected_session_id.clone(), + ) + .await; + let config = TestConnectionConfig { + builtins: vec!["developer".to_string()], + fs_read_text_file: true, + goose_mode: GooseMode::Auto, + ..Default::default() + }; + let mut conn = ClientToAgentConnection::new(config, openai).await; + let (mut session, _) = conn.new_session().await; + expected_session_id.set(session.session_id().0.to_string()); + + let _ = session.prompt(&prompt, PermissionDecision::Cancel).await; + assert_eq!(conn.permission_request_count(), 0); + assert_eq!(conn.read_requests().len(), 1); + assert_eq!(conn.write_requests().len(), 0); +} + +async fn run_fs_write_test( + file_content: &str, + fs_write_enabled: bool, + decision: PermissionDecision, + expected_status: ToolCallStatus, + expect_client_write_calls: usize, + expect_file_exists: bool, +) { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("write-target.txt"); + let expected_session_id = ExpectedSessionId::default(); + let prompt = format!( + "Write {} to {} using write.", + file_content, + file_path.display() + ); + let openai = OpenAiFixture::new_dynamic( + vec![ + ( + prompt.clone(), + openai_tool_call_fixture( + "call_write", + "write", + json!({ + "path": file_path.to_string_lossy(), + "content": file_content + }), + ), + ), + ( + file_path.to_string_lossy().to_string(), + include_str!("../test_data/openai_basic.txt").to_string(), + ), + ], + expected_session_id.clone(), + ) + .await; + let config = TestConnectionConfig { + builtins: vec!["developer".to_string()], + goose_mode: GooseMode::Approve, + fs_write_text_file: fs_write_enabled, + ..Default::default() + }; + let mut conn = ClientToAgentConnection::new(config, openai).await; + let (mut session, _) = conn.new_session().await; + expected_session_id.set(session.session_id().0.to_string()); + + let output = session.prompt(&prompt, decision).await; + assert_eq!(output.tool_status, Some(expected_status)); + assert_eq!(conn.permission_request_count(), 1); + assert_eq!(conn.write_requests().len(), expect_client_write_calls); + assert_eq!(file_path.exists(), expect_file_exists); + if expect_file_exists { + assert_eq!(fs::read_to_string(&file_path).unwrap(), file_content); + } +} + +pub async fn run_fs_write_delegation_with_permission_allowed() { + run_fs_write_test( + FS_WRITE_DELEGATED_CONTENT, + true, + PermissionDecision::AllowOnce, + ToolCallStatus::Completed, + 1, + true, + ) + .await; +} + +pub async fn run_fs_write_delegation_with_permission_rejected() { + run_fs_write_test( + FS_WRITE_DELEGATED_CONTENT, + true, + PermissionDecision::RejectOnce, + ToolCallStatus::Failed, + 0, + false, + ) + .await; +} + +pub async fn run_agent_side_write_when_fs_disabled() { + run_fs_write_test( + FS_WRITE_LOCAL_CONTENT, + false, + PermissionDecision::AllowOnce, + ToolCallStatus::Completed, + 0, + true, + ) + .await; +} + +pub async fn run_agent_side_write_rejected_when_fs_disabled() { + run_fs_write_test( + FS_WRITE_LOCAL_CONTENT, + false, + PermissionDecision::RejectOnce, + ToolCallStatus::Failed, + 0, + false, + ) + .await; +} diff --git a/crates/goose-acp/tests/fixtures/mod.rs b/crates/goose-acp/tests/fixtures/mod.rs index ff9c298311ad..b8051c07cf8f 100644 --- a/crates/goose-acp/tests/fixtures/mod.rs +++ b/crates/goose-acp/tests/fixtures/mod.rs @@ -72,8 +72,8 @@ fn select_option( pub struct OpenAiFixture { _server: MockServer, base_url: String, - exchanges: Vec<(String, &'static str)>, - queue: Arc>>, + exchanges: Vec<(String, String)>, + queue: Arc>>, } impl OpenAiFixture { @@ -82,6 +82,17 @@ impl OpenAiFixture { pub async fn new( exchanges: Vec<(String, &'static str)>, expected_session_id: ExpectedSessionId, + ) -> Self { + let exchanges = exchanges + .into_iter() + .map(|(pattern, response)| (pattern, response.to_string())) + .collect(); + Self::new_dynamic(exchanges, expected_session_id).await + } + + pub async fn new_dynamic( + exchanges: Vec<(String, String)>, + expected_session_id: ExpectedSessionId, ) -> Self { let mock_server = MockServer::start().await; let queue = Arc::new(Mutex::new(VecDeque::from(exchanges.clone()))); @@ -247,6 +258,8 @@ pub struct TestConnectionConfig { pub mcp_servers: Vec, pub builtins: Vec, pub goose_mode: GooseMode, + pub fs_read_text_file: bool, + pub fs_write_text_file: bool, pub data_root: PathBuf, pub provider_factory: Option, } @@ -257,6 +270,8 @@ impl Default for TestConnectionConfig { mcp_servers: Vec::new(), builtins: Vec::new(), goose_mode: GooseMode::Auto, + fs_read_text_file: false, + fs_write_text_file: false, data_root: PathBuf::new(), provider_factory: None, } diff --git a/crates/goose-acp/tests/fixtures/server.rs b/crates/goose-acp/tests/fixtures/server.rs index 1d02eb66251e..48447c5e9374 100644 --- a/crates/goose-acp/tests/fixtures/server.rs +++ b/crates/goose-acp/tests/fixtures/server.rs @@ -5,9 +5,11 @@ use super::{ use async_trait::async_trait; use goose::config::PermissionManager; use sacp::schema::{ - ContentBlock, InitializeRequest, LoadSessionRequest, McpServer, NewSessionRequest, - PromptRequest, ProtocolVersion, RequestPermissionRequest, SessionModelState, - SessionNotification, SessionUpdate, StopReason, TextContent, ToolCallStatus, + ClientCapabilities, ContentBlock, FileSystemCapability, InitializeRequest, LoadSessionRequest, + McpServer, NewSessionRequest, PromptRequest, ProtocolVersion, ReadTextFileRequest, + ReadTextFileResponse, RequestPermissionRequest, SessionModelState, SessionNotification, + SessionUpdate, StopReason, TextContent, ToolCallStatus, WriteTextFileRequest, + WriteTextFileResponse, }; use sacp::{ClientToAgent, JrConnectionCx}; use std::sync::{Arc, Mutex}; @@ -20,6 +22,9 @@ pub struct ClientToAgentConnection { pending_mcp_servers: Vec, updates: Arc>>, permission: Arc>, + permission_requests: Arc>>, + read_requests: Arc>>, + write_requests: Arc>>, notify: Arc, permission_manager: Arc, _openai: super::OpenAiFixture, @@ -39,6 +44,18 @@ impl ClientToAgentConnection { pub fn cx(&self) -> &JrConnectionCx { &self.cx } + + pub fn permission_request_count(&self) -> usize { + self.permission_requests.lock().unwrap().len() + } + + pub fn read_requests(&self) -> Vec { + self.read_requests.lock().unwrap().clone() + } + + pub fn write_requests(&self) -> Vec { + self.write_requests.lock().unwrap().clone() + } } #[async_trait] @@ -66,11 +83,19 @@ impl Connection for ClientToAgentConnection { let updates = Arc::new(Mutex::new(Vec::new())); let notify = Arc::new(Notify::new()); let permission = Arc::new(Mutex::new(PermissionDecision::Cancel)); + let permission_requests = Arc::new(Mutex::new(Vec::new())); + let read_requests = Arc::new(Mutex::new(Vec::new())); + let write_requests = Arc::new(Mutex::new(Vec::new())); + let fs_read_text_file = config.fs_read_text_file; + let fs_write_text_file = config.fs_write_text_file; let cx = { let updates_clone = updates.clone(); let notify_clone = notify.clone(); let permission_clone = permission.clone(); + let permission_requests_clone = permission_requests.clone(); + let read_requests_clone = read_requests.clone(); + let write_requests_clone = write_requests.clone(); let cx_holder: Arc>>> = Arc::new(Mutex::new(None)); @@ -97,9 +122,11 @@ impl Connection for ClientToAgentConnection { .on_receive_request( { let permission = permission_clone.clone(); + let permission_requests = permission_requests_clone.clone(); async move |req: RequestPermissionRequest, request_cx, _connection_cx| { + permission_requests.lock().unwrap().push(req.clone()); let decision = *permission.lock().unwrap(); let response = map_permission_response(&permission_mapping, &req, decision); @@ -108,15 +135,67 @@ impl Connection for ClientToAgentConnection { }, sacp::on_receive_request!(), ) + .on_receive_request( + { + let read_requests = read_requests_clone.clone(); + async move |req: ReadTextFileRequest, request_cx, _connection_cx| { + read_requests.lock().unwrap().push(req.clone()); + let content = std::fs::read_to_string(&req.path).map_err(|e| { + sacp::Error::internal_error().data(format!( + "Failed to read {}: {}", + req.path.display(), + e + )) + })?; + request_cx.respond(ReadTextFileResponse::new(content)) + } + }, + sacp::on_receive_request!(), + ) + .on_receive_request( + { + let write_requests = write_requests_clone.clone(); + async move |req: WriteTextFileRequest, request_cx, _connection_cx| { + write_requests.lock().unwrap().push(req.clone()); + if let Some(parent) = req.path.parent() { + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent).map_err(|e| { + sacp::Error::internal_error().data(format!( + "Failed to create {}: {}", + parent.display(), + e + )) + })?; + } + } + std::fs::write(&req.path, &req.content).map_err(|e| { + sacp::Error::internal_error().data(format!( + "Failed to write {}: {}", + req.path.display(), + e + )) + })?; + request_cx.respond(WriteTextFileResponse::new()) + } + }, + sacp::on_receive_request!(), + ) .connect_to(transport) .unwrap() .run_until({ let cx_holder = cx_holder_clone; move |cx: JrConnectionCx| async move { - cx.send_request(InitializeRequest::new(ProtocolVersion::LATEST)) - .block_task() - .await - .unwrap(); + cx.send_request( + InitializeRequest::new(ProtocolVersion::LATEST) + .client_capabilities( + ClientCapabilities::new().fs(FileSystemCapability::new() + .read_text_file(fs_read_text_file) + .write_text_file(fs_write_text_file)), + ), + ) + .block_task() + .await + .unwrap(); *cx_holder.lock().unwrap() = Some(cx.clone()); let _ = ready_tx.send(()); @@ -141,6 +220,9 @@ impl Connection for ClientToAgentConnection { pending_mcp_servers: config.mcp_servers, updates, permission, + permission_requests, + read_requests, + write_requests, notify, permission_manager, _openai: openai, diff --git a/crates/goose-acp/tests/server_test.rs b/crates/goose-acp/tests/server_test.rs index a143d15356e3..6e4b1ba25255 100644 --- a/crates/goose-acp/tests/server_test.rs +++ b/crates/goose-acp/tests/server_test.rs @@ -2,9 +2,12 @@ mod common_tests; use common_tests::fixtures::run_test; use common_tests::fixtures::server::ClientToAgentConnection; use common_tests::{ - run_config_mcp, run_initialize_without_provider, run_load_model, run_model_list, run_model_set, - run_permission_persistence, run_prompt_basic, run_prompt_codemode, run_prompt_image, - run_prompt_mcp, + run_agent_side_write_rejected_when_fs_disabled, run_agent_side_write_when_fs_disabled, + run_config_mcp, run_fs_read_delegation_without_permission, + run_fs_write_delegation_with_permission_allowed, + run_fs_write_delegation_with_permission_rejected, run_initialize_without_provider, + run_load_model, run_model_list, run_model_set, run_permission_persistence, run_prompt_basic, + run_prompt_codemode, run_prompt_image, run_prompt_mcp, }; #[test] @@ -56,3 +59,28 @@ fn test_prompt_image() { fn test_prompt_mcp() { run_test(async { run_prompt_mcp::().await }); } + +#[test] +fn test_fs_read_delegation_without_permission() { + run_test(async { run_fs_read_delegation_without_permission().await }); +} + +#[test] +fn test_fs_write_delegation_with_permission_allowed() { + run_test(async { run_fs_write_delegation_with_permission_allowed().await }); +} + +#[test] +fn test_fs_write_delegation_with_permission_rejected() { + run_test(async { run_fs_write_delegation_with_permission_rejected().await }); +} + +#[test] +fn test_agent_side_write_when_fs_disabled() { + run_test(async { run_agent_side_write_when_fs_disabled().await }); +} + +#[test] +fn test_agent_side_write_rejected_when_fs_disabled() { + run_test(async { run_agent_side_write_rejected_when_fs_disabled().await }); +} diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index e4134dd79d4a..7450a77a0a0e 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -19,6 +19,7 @@ use crate::agents::extension_manager::{ get_parameter_names, ExtensionManager, ExtensionManagerCapabilities, }; use crate::agents::final_output_tool::{FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_TOOL_NAME}; +use crate::agents::platform_extensions::DeveloperFileIo; use crate::agents::platform_extensions::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; use crate::agents::platform_tools::PLATFORM_MANAGE_SCHEDULE_TOOL_NAME; use crate::agents::prompt_manager::PromptManager; @@ -110,6 +111,7 @@ pub struct AgentConfig { pub goose_mode: GooseMode, pub disable_session_naming: bool, pub goose_platform: GoosePlatform, + pub developer_file_io: Option, } impl AgentConfig { @@ -128,8 +130,14 @@ impl AgentConfig { goose_mode, disable_session_naming, goose_platform, + developer_file_io: None, } } + + pub fn with_developer_file_io(mut self, developer_file_io: Option) -> Self { + self.developer_file_io = developer_file_io; + self + } } /// The main goose Agent @@ -228,6 +236,7 @@ impl Agent { }; let session_manager = Arc::clone(&config.session_manager); let permission_manager = Arc::clone(&config.permission_manager); + let developer_file_io = config.developer_file_io.clone(); Self { provider: provider.clone(), config, @@ -236,6 +245,7 @@ impl Agent { session_manager, goose_platform.to_string(), capabilities, + developer_file_io, )), final_output_tool: Arc::new(Mutex::new(None)), frontend_tools: Mutex::new(HashMap::new()), diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 1ac7437a38d6..cf308cca6bdc 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -35,6 +35,7 @@ use super::types::SharedProvider; use crate::agents::extension::{Envs, ProcessExit}; use crate::agents::extension_malware_check; use crate::agents::mcp_client::{GooseMcpClientCapabilities, McpClient, McpClientTrait}; +use crate::agents::platform_extensions::DeveloperFileIo; use crate::builtin_extension::get_builtin_extension; use crate::config::extensions::name_to_key; use crate::config::search_path::SearchPaths; @@ -482,6 +483,7 @@ impl ExtensionManager { session_manager: Arc, client_name: String, capabilities: ExtensionManagerCapabilities, + developer_file_io: Option, ) -> Self { Self { extensions: Mutex::new(HashMap::new()), @@ -489,6 +491,7 @@ impl ExtensionManager { extension_manager: None, session_manager, session: None, + developer_file_io, }, provider, tools_cache: Mutex::new(None), @@ -506,6 +509,7 @@ impl ExtensionManager { session_manager, "goose-cli".to_string(), ExtensionManagerCapabilities { mcpui: false }, + None, ) } diff --git a/crates/goose/src/agents/platform_extensions/analyze/mod.rs b/crates/goose/src/agents/platform_extensions/analyze/mod.rs index fe7edc23ae9a..163d12d14bdf 100644 --- a/crates/goose/src/agents/platform_extensions/analyze/mod.rs +++ b/crates/goose/src/agents/platform_extensions/analyze/mod.rs @@ -302,6 +302,7 @@ mod tests { extension_manager: None, session_manager: Arc::new(SessionManager::new(std::env::temp_dir())), session: None, + developer_file_io: None, } } diff --git a/crates/goose/src/agents/platform_extensions/developer/edit.rs b/crates/goose/src/agents/platform_extensions/developer/edit.rs index cc19bf0f1e4d..c3b64a0b34de 100644 --- a/crates/goose/src/agents/platform_extensions/developer/edit.rs +++ b/crates/goose/src/agents/platform_extensions/developer/edit.rs @@ -1,11 +1,15 @@ -use std::fs; use std::path::{Path, PathBuf}; use rmcp::model::{CallToolResult, Content}; use schemars::JsonSchema; use serde::Deserialize; +use crate::agents::platform_extensions::{ + DeveloperFileIo, ReadFileChunkFn, ReadFileFn, WriteFileFn, +}; + const NO_MATCH_PREVIEW_LINES: usize = 20; +const DEFAULT_READ_LIMIT_LINES: usize = 500; #[derive(Debug, Deserialize, JsonSchema)] pub struct FileWriteParams { @@ -13,6 +17,13 @@ pub struct FileWriteParams { pub content: String, } +#[derive(Debug, Deserialize, JsonSchema)] +pub struct FileReadParams { + pub path: String, + pub offset: Option, + pub limit: Option, +} + #[derive(Debug, Deserialize, JsonSchema)] pub struct FileEditParams { pub path: String, @@ -20,69 +31,136 @@ pub struct FileEditParams { pub after: String, } -pub struct EditTools; +pub struct EditTools { + read_file: ReadFileFn, + write_file: WriteFileFn, + read_file_chunk: Option, +} impl EditTools { - pub fn new() -> Self { - Self + pub fn with_file_io( + read_file: ReadFileFn, + read_file_chunk: Option, + write_file: WriteFileFn, + ) -> Self { + Self { + read_file, + write_file, + read_file_chunk, + } + } + + pub async fn file_write(&self, params: FileWriteParams) -> CallToolResult { + self.file_write_with_cwd(params, None).await } - pub fn file_write(&self, params: FileWriteParams) -> CallToolResult { - self.file_write_with_cwd(params, None) + pub async fn file_read(&self, params: FileReadParams) -> CallToolResult { + self.file_read_with_cwd(params, None).await } - pub fn file_write_with_cwd( + pub async fn file_read_with_cwd( &self, - params: FileWriteParams, + params: FileReadParams, working_dir: Option<&Path>, ) -> CallToolResult { let path = resolve_path(¶ms.path, working_dir); - - if let Some(parent) = path.parent() { - if !parent.as_os_str().is_empty() && !parent.exists() { - if let Err(error) = fs::create_dir_all(parent) { - return CallToolResult::error(vec![Content::text(format!( - "Failed to create directory {}: {}", - parent.display(), - error - )) - .with_priority(0.0)]); + if params.offset.is_some() || params.limit.is_some() { + let offset = params.offset.unwrap_or(0); + let limit = params.limit.unwrap_or(DEFAULT_READ_LIMIT_LINES); + if limit == 0 { + return CallToolResult::error(vec![Content::text( + "Failed to read: limit must be greater than 0", + ) + .with_priority(0.0)]); + } + if let Some(read_file_chunk) = &self.read_file_chunk { + match read_file_chunk(path, offset, limit).await { + Ok(chunk) => { + return CallToolResult::success(vec![ + Content::text(chunk).with_priority(0.0) + ]); + } + Err(error) => { + return CallToolResult::error(vec![Content::text(format!( + "Failed to read {}: {}", + params.path, error + )) + .with_priority(0.0)]); + } } } + // Fallback for delegated transports (e.g. ACP) that don't support + // offset/limit chunk reads yet: read once, then slice locally. + return match (self.read_file)(path).await { + Ok(content) => CallToolResult::success(vec![Content::text(slice_lines( + &content, offset, limit, + )) + .with_priority(0.0)]), + Err(error) => CallToolResult::error(vec![Content::text(format!( + "Failed to read {}: {}", + params.path, error + )) + .with_priority(0.0)]), + }; } - let is_new = !path.exists(); - - match fs::write(path, ¶ms.content) { - Ok(()) => { - let line_count = params.content.lines().count(); - let action = if is_new { "Created" } else { "Wrote" }; - CallToolResult::success(vec![Content::text(format!( - "{} {} ({} lines)", - action, params.path, line_count - )) - .with_priority(0.0)]) + match (self.read_file)(path).await { + Ok(content) => CallToolResult::success(vec![Content::text(content).with_priority(0.0)]), + Err(error) => { + let error_text = error.to_string(); + let message = if self.read_file_chunk.is_some() + && error_text.contains("exceeds max file size") + { + format!( + "Failed to read {}: {}. Retry with offset and limit to read the file in chunks.", + params.path, error_text + ) + } else { + format!("Failed to read {}: {}", params.path, error_text) + }; + CallToolResult::error(vec![Content::text(message).with_priority(0.0)]) } + } + } + + pub async fn file_write_with_cwd( + &self, + params: FileWriteParams, + working_dir: Option<&Path>, + ) -> CallToolResult { + let FileWriteParams { + path: display_path, + content, + } = params; + let path = resolve_path(&display_path, working_dir); + let line_count = content.lines().count(); + + match (self.write_file)(path, content).await { + Ok(()) => CallToolResult::success(vec![Content::text(format!( + "Wrote {} ({} lines)", + display_path, line_count + )) + .with_priority(0.0)]), Err(error) => CallToolResult::error(vec![Content::text(format!( "Failed to write {}: {}", - params.path, error + display_path, error )) .with_priority(0.0)]), } } - pub fn file_edit(&self, params: FileEditParams) -> CallToolResult { - self.file_edit_with_cwd(params, None) + pub async fn file_edit(&self, params: FileEditParams) -> CallToolResult { + self.file_edit_with_cwd(params, None).await } - pub fn file_edit_with_cwd( + pub async fn file_edit_with_cwd( &self, params: FileEditParams, working_dir: Option<&Path>, ) -> CallToolResult { let path = resolve_path(¶ms.path, working_dir); - let content = match fs::read_to_string(&path) { + let content = match (self.read_file)(path.clone()).await { Ok(c) => c, Err(error) => { return CallToolResult::error(vec![Content::text(format!( @@ -109,7 +187,7 @@ impl EditTools { 1 => { let new_content = content.replacen(¶ms.before, ¶ms.after, 1); - match fs::write(&path, &new_content) { + match (self.write_file)(path, new_content).await { Ok(()) => { let old_lines = params.before.lines().count(); let new_lines = params.after.lines().count(); @@ -155,7 +233,12 @@ impl EditTools { impl Default for EditTools { fn default() -> Self { - Self::new() + let local_io = DeveloperFileIo::default_local(); + Self { + read_file: local_io.read_file, + write_file: local_io.write_file, + read_file_chunk: local_io.read_file_chunk, + } } } @@ -172,6 +255,15 @@ fn resolve_path(path: &str, working_dir: Option<&Path>) -> PathBuf { } } +fn slice_lines(content: &str, offset: usize, limit: usize) -> String { + content + .lines() + .skip(offset) + .take(limit) + .collect::>() + .join("\n") +} + fn count_lines_before(content: &str, byte_pos: usize) -> usize { content .char_indices() @@ -228,8 +320,10 @@ fn build_file_preview(content: &str, max_lines: usize) -> String { #[cfg(test)] mod tests { use super::*; + use crate::agents::platform_extensions::MAX_READ_FILE_BYTES; use rmcp::model::RawContent; use std::fs; + use std::sync::Arc; use tempfile::TempDir; fn setup() -> TempDir { @@ -243,65 +337,233 @@ mod tests { } } - #[test] - fn test_file_write_new() { + #[tokio::test] + async fn test_file_write_new() { let dir = setup(); let path = dir.path().join("new_file.txt"); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_write(FileWriteParams { - path: path.to_string_lossy().to_string(), - content: "Hello, world!\nLine 2".to_string(), - }); + let result = tools + .file_write(FileWriteParams { + path: path.to_string_lossy().to_string(), + content: "Hello, world!\nLine 2".to_string(), + }) + .await; assert!(!result.is_error.unwrap_or(false)); assert!(path.exists()); assert_eq!(fs::read_to_string(&path).unwrap(), "Hello, world!\nLine 2"); } - #[test] - fn test_file_write_overwrite() { + #[tokio::test] + async fn test_file_read_success() { + let dir = setup(); + let path = dir.path().join("read.txt"); + fs::write(&path, "read me").unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: None, + limit: None, + }) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result), "read me"); + } + + #[tokio::test] + async fn test_file_read_with_offset_and_limit() { + let dir = setup(); + let path = dir.path().join("read-chunked.txt"); + fs::write(&path, "line 1\nline 2\nline 3\nline 4\nline 5").unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: Some(1), + limit: Some(2), + }) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result), "line 2\nline 3"); + } + + #[tokio::test] + async fn test_file_read_with_offset_and_limit_falls_back_without_chunk_delegate() { + let read_file: ReadFileFn = + Arc::new(|_path: PathBuf| Box::pin(async { Ok("l1\nl2\nl3\nl4".to_string()) })); + let write_file: WriteFileFn = + Arc::new(|_path: PathBuf, _content: String| Box::pin(async { Ok(()) })); + let tools = EditTools::with_file_io(read_file, None, write_file); + + let result = tools + .file_read(FileReadParams { + path: "ignored.txt".to_string(), + offset: Some(1), + limit: Some(2), + }) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result), "l2\nl3"); + } + + #[tokio::test] + async fn test_file_read_with_zero_limit_errors() { + let dir = setup(); + let path = dir.path().join("read-invalid-limit.txt"); + fs::write(&path, "line 1\nline 2").unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: Some(0), + limit: Some(0), + }) + .await; + + assert!(result.is_error.unwrap_or(false)); + assert!(extract_text(&result).contains("limit must be greater than 0")); + } + + #[tokio::test] + async fn test_file_read_large_requires_chunking() { + let dir = setup(); + let path = dir.path().join("large-read.txt"); + let large = "a".repeat(MAX_READ_FILE_BYTES + 1); + fs::write(&path, &large).unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: None, + limit: None, + }) + .await; + + assert!(result.is_error.unwrap_or(false)); + assert!(extract_text(&result).contains("Retry with offset and limit")); + } + + #[tokio::test] + async fn test_file_read_rejects_oversized_file() { + let dir = setup(); + let path = dir.path().join("large-read.txt"); + let large = "a".repeat(MAX_READ_FILE_BYTES + 1); + fs::write(&path, &large).unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: None, + limit: None, + }) + .await; + + assert!(result.is_error.unwrap_or(false)); + assert!(extract_text(&result).contains("exceeds max file size")); + } + + #[tokio::test] + async fn test_file_read_chunking_allows_large_file() { + let dir = setup(); + let path = dir.path().join("large-read-chunked.txt"); + let lines = vec!["line".repeat(3000); 400]; + fs::write(&path, lines.join("\n")).unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: Some(0), + limit: Some(3), + }) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result).lines().count(), 3); + } + + #[tokio::test] + async fn test_file_read_with_limit_allows_large_file() { + let dir = setup(); + let path = dir.path().join("large-chunk-read.txt"); + let mut content = String::new(); + for _ in 0..220_000 { + content.push_str("line\n"); + } + fs::write(&path, content).unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read(FileReadParams { + path: path.to_string_lossy().to_string(), + offset: Some(0), + limit: Some(3), + }) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result).lines().count(), 3); + } + + #[tokio::test] + async fn test_file_write_overwrite() { let dir = setup(); let path = dir.path().join("existing.txt"); fs::write(&path, "old content").unwrap(); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_write(FileWriteParams { - path: path.to_string_lossy().to_string(), - content: "new content".to_string(), - }); + let result = tools + .file_write(FileWriteParams { + path: path.to_string_lossy().to_string(), + content: "new content".to_string(), + }) + .await; assert!(!result.is_error.unwrap_or(false)); assert_eq!(fs::read_to_string(&path).unwrap(), "new content"); } - #[test] - fn test_file_write_creates_dirs() { + #[tokio::test] + async fn test_file_write_creates_dirs() { let dir = setup(); let path = dir.path().join("a/b/c/file.txt"); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_write(FileWriteParams { - path: path.to_string_lossy().to_string(), - content: "nested".to_string(), - }); + let result = tools + .file_write(FileWriteParams { + path: path.to_string_lossy().to_string(), + content: "nested".to_string(), + }) + .await; assert!(!result.is_error.unwrap_or(false)); assert!(path.exists()); } - #[test] - fn test_file_edit_single_match() { + #[tokio::test] + async fn test_file_edit_single_match() { let dir = setup(); let path = dir.path().join("edit.txt"); fs::write(&path, "fn foo() {\n println!(\"hello\");\n}").unwrap(); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_edit(FileEditParams { - path: path.to_string_lossy().to_string(), - before: "println!(\"hello\");".to_string(), - after: "println!(\"world\");".to_string(), - }); + let result = tools + .file_edit(FileEditParams { + path: path.to_string_lossy().to_string(), + before: "println!(\"hello\");".to_string(), + after: "println!(\"world\");".to_string(), + }) + .await; assert!(!result.is_error.unwrap_or(false)); let content = fs::read_to_string(&path).unwrap(); @@ -309,18 +571,20 @@ mod tests { assert!(!content.contains("println!(\"hello\");")); } - #[test] - fn test_file_edit_no_match() { + #[tokio::test] + async fn test_file_edit_no_match() { let dir = setup(); let path = dir.path().join("edit.txt"); fs::write(&path, "some content").unwrap(); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_edit(FileEditParams { - path: path.to_string_lossy().to_string(), - before: "nonexistent".to_string(), - after: "replacement".to_string(), - }); + let result = tools + .file_edit(FileEditParams { + path: path.to_string_lossy().to_string(), + before: "nonexistent".to_string(), + after: "replacement".to_string(), + }) + .await; assert!(result.is_error.unwrap_or(false)); let text = extract_text(&result); @@ -329,52 +593,58 @@ mod tests { assert!(text.contains("some content")); } - #[test] - fn test_file_edit_multiple_matches() { + #[tokio::test] + async fn test_file_edit_multiple_matches() { let dir = setup(); let path = dir.path().join("edit.txt"); fs::write(&path, "foo\nbar\nfoo\nbaz").unwrap(); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_edit(FileEditParams { - path: path.to_string_lossy().to_string(), - before: "foo".to_string(), - after: "qux".to_string(), - }); + let result = tools + .file_edit(FileEditParams { + path: path.to_string_lossy().to_string(), + before: "foo".to_string(), + after: "qux".to_string(), + }) + .await; assert!(result.is_error.unwrap_or(false)); assert_eq!(fs::read_to_string(&path).unwrap(), "foo\nbar\nfoo\nbaz"); } - #[test] - fn test_file_edit_delete() { + #[tokio::test] + async fn test_file_edit_delete() { let dir = setup(); let path = dir.path().join("edit.txt"); fs::write(&path, "keep\ndelete me\nkeep").unwrap(); - let tools = EditTools::new(); + let tools = EditTools::default(); - let result = tools.file_edit(FileEditParams { - path: path.to_string_lossy().to_string(), - before: "\ndelete me".to_string(), - after: "".to_string(), - }); + let result = tools + .file_edit(FileEditParams { + path: path.to_string_lossy().to_string(), + before: "\ndelete me".to_string(), + after: "".to_string(), + }) + .await; assert!(!result.is_error.unwrap_or(false)); assert_eq!(fs::read_to_string(&path).unwrap(), "keep\nkeep"); } - #[test] - fn test_file_write_resolves_relative_paths_from_working_dir() { + #[tokio::test] + async fn test_file_write_resolves_relative_paths_from_working_dir() { let dir = setup(); - let tools = EditTools::new(); - - let result = tools.file_write_with_cwd( - FileWriteParams { - path: "relative.txt".to_string(), - content: "relative write".to_string(), - }, - Some(dir.path()), - ); + let tools = EditTools::default(); + + let result = tools + .file_write_with_cwd( + FileWriteParams { + path: "relative.txt".to_string(), + content: "relative write".to_string(), + }, + Some(dir.path()), + ) + .await; assert!(!result.is_error.unwrap_or(false)); assert_eq!( @@ -383,20 +653,22 @@ mod tests { ); } - #[test] - fn test_file_edit_resolves_relative_paths_from_working_dir() { + #[tokio::test] + async fn test_file_edit_resolves_relative_paths_from_working_dir() { let dir = setup(); fs::write(dir.path().join("relative-edit.txt"), "before").unwrap(); - let tools = EditTools::new(); - - let result = tools.file_edit_with_cwd( - FileEditParams { - path: "relative-edit.txt".to_string(), - before: "before".to_string(), - after: "after".to_string(), - }, - Some(dir.path()), - ); + let tools = EditTools::default(); + + let result = tools + .file_edit_with_cwd( + FileEditParams { + path: "relative-edit.txt".to_string(), + before: "before".to_string(), + after: "after".to_string(), + }, + Some(dir.path()), + ) + .await; assert!(!result.is_error.unwrap_or(false)); assert_eq!( @@ -404,4 +676,25 @@ mod tests { "after" ); } + + #[tokio::test] + async fn test_file_read_resolves_relative_paths_from_working_dir() { + let dir = setup(); + fs::write(dir.path().join("relative-read.txt"), "relative read").unwrap(); + let tools = EditTools::default(); + + let result = tools + .file_read_with_cwd( + FileReadParams { + path: "relative-read.txt".to_string(), + offset: None, + limit: None, + }, + Some(dir.path()), + ) + .await; + + assert!(!result.is_error.unwrap_or(false)); + assert_eq!(extract_text(&result), "relative read"); + } } diff --git a/crates/goose/src/agents/platform_extensions/developer/mod.rs b/crates/goose/src/agents/platform_extensions/developer/mod.rs index b03827baf001..bf19cdd11155 100644 --- a/crates/goose/src/agents/platform_extensions/developer/mod.rs +++ b/crates/goose/src/agents/platform_extensions/developer/mod.rs @@ -6,7 +6,7 @@ use crate::agents::extension::PlatformExtensionContext; use crate::agents::mcp_client::{Error, McpClientTrait}; use anyhow::Result; use async_trait::async_trait; -use edit::{EditTools, FileEditParams, FileWriteParams}; +use edit::{EditTools, FileEditParams, FileReadParams, FileWriteParams}; use indoc::indoc; use rmcp::model::{ CallToolResult, Content, Implementation, InitializeResult, JsonObject, ListToolsResult, @@ -30,7 +30,7 @@ pub struct DeveloperClient { } impl DeveloperClient { - pub fn new(_context: PlatformExtensionContext) -> Result { + pub fn new(context: PlatformExtensionContext) -> Result { let info = InitializeResult { protocol_version: ProtocolVersion::V_2025_03_26, capabilities: ServerCapabilities { @@ -63,15 +63,29 @@ impl DeveloperClient { For editing software, prefer the flow of using tree to understand the codebase structure and file sizes. When you need to search, prefer rg which correctly respects gitignored - content. Then use cat or sed to gather the context you need, always reading before editing. - Use write and edit to efficiently make changes. Test and verify as appropriate. + content. Use read to gather file context (and use offset/limit for large files), always + reading before editing. Use write and edit to efficiently make changes. Use shell for + command execution and workflows where terminal tools are the right fit. Test and verify + as appropriate. "}.to_string()), }; + let edit_tools = context + .developer_file_io + .clone() + .map(|file_io| { + EditTools::with_file_io( + file_io.read_file, + file_io.read_file_chunk, + file_io.write_file, + ) + }) + .unwrap_or_default(); + Ok(Self { info, shell_tool: Arc::new(ShellTool::new()), - edit_tools: Arc::new(EditTools::new()), + edit_tools: Arc::new(edit_tools), tree_tool: Arc::new(TreeTool::new()), }) } @@ -95,6 +109,19 @@ impl DeveloperClient { fn get_tools() -> Vec { vec![ + Tool::new( + "read".to_string(), + "Read a UTF-8 text file and return its contents. Files over 1MB require offset/limit for chunked reads." + .to_string(), + Self::schema::(), + ) + .annotate(ToolAnnotations { + title: Some("Read".to_string()), + read_only_hint: Some(true), + destructive_hint: Some(false), + idempotent_hint: Some(true), + open_world_hint: Some(false), + }), Tool::new( "write".to_string(), "Create a new file or overwrite an existing file. Creates parent directories if needed.".to_string(), @@ -179,15 +206,31 @@ impl McpClientTrait for DeveloperClient { )) .with_priority(0.0)])), }, + "read" => match Self::parse_args::(arguments) { + Ok(params) => Ok(self + .edit_tools + .file_read_with_cwd(params, working_dir) + .await), + Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( + "Error: {error}" + )) + .with_priority(0.0)])), + }, "write" => match Self::parse_args::(arguments) { - Ok(params) => Ok(self.edit_tools.file_write_with_cwd(params, working_dir)), + Ok(params) => Ok(self + .edit_tools + .file_write_with_cwd(params, working_dir) + .await), Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( "Error: {error}" )) .with_priority(0.0)])), }, "edit" => match Self::parse_args::(arguments) { - Ok(params) => Ok(self.edit_tools.file_edit_with_cwd(params, working_dir)), + Ok(params) => Ok(self + .edit_tools + .file_edit_with_cwd(params, working_dir) + .await), Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( "Error: {error}" )) @@ -227,7 +270,7 @@ mod tests { .map(|t| t.name.to_string()) .collect(); - assert_eq!(names, vec!["write", "edit", "shell", "tree"]); + assert_eq!(names, vec!["read", "write", "edit", "shell", "tree"]); } fn test_context(data_dir: std::path::PathBuf) -> PlatformExtensionContext { @@ -235,6 +278,7 @@ mod tests { extension_manager: None, session_manager: Arc::new(SessionManager::new(data_dir)), session: None, + developer_file_io: None, } } @@ -290,6 +334,178 @@ mod tests { fs::read_to_string(cwd.join("notes.txt")).unwrap(), "updated line" ); + + let read = client + .call_tool( + "session", + "read", + Some(object!({ + "path": "notes.txt" + })), + Some(cwd.to_str().unwrap()), + CancellationToken::new(), + ) + .await + .unwrap(); + assert_eq!(read.is_error, Some(false)); + assert_eq!(first_text(&read), "updated line"); + } + + #[tokio::test] + async fn developer_client_delegates_read_to_context_io() { + use crate::agents::platform_extensions::{DeveloperFileIo, ReadFileFn, WriteFileFn}; + use std::path::PathBuf; + use std::sync::Arc; + + let temp = tempfile::tempdir().unwrap(); + let mut context = test_context(temp.path().join("sessions")); + + let read_file: ReadFileFn = Arc::new(|path: PathBuf| { + Box::pin(async move { + if path.to_string_lossy().ends_with("delegated.txt") { + Ok("delegated content".to_string()) + } else { + Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("not found: {}", path.display()), + )) + } + }) + }); + + let write_file: WriteFileFn = + Arc::new(|_path: PathBuf, _content: String| Box::pin(async move { Ok(()) })); + + context.developer_file_io = Some(DeveloperFileIo { + read_file, + read_file_chunk: None, + write_file, + }); + + let client = DeveloperClient::new(context).unwrap(); + + let result = client + .call_tool( + "session", + "read", + Some(object!({ + "path": "delegated.txt" + })), + None, + CancellationToken::new(), + ) + .await + .unwrap(); + + assert_eq!(result.is_error, Some(false)); + assert_eq!(first_text(&result), "delegated content"); + } + + #[tokio::test] + async fn developer_client_delegates_write_to_context_io() { + use crate::agents::platform_extensions::{DeveloperFileIo, ReadFileFn, WriteFileFn}; + use std::path::PathBuf; + use std::sync::{Arc, Mutex}; + + let temp = tempfile::tempdir().unwrap(); + let mut context = test_context(temp.path().join("sessions")); + + let captured_writes = Arc::new(Mutex::new(Vec::new())); + let writes_clone = captured_writes.clone(); + + let read_file: ReadFileFn = Arc::new(|_path: PathBuf| { + Box::pin(async move { + Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "not found", + )) + }) + }); + + let write_file: WriteFileFn = Arc::new(move |path: PathBuf, content: String| { + let writes = writes_clone.clone(); + Box::pin(async move { + writes.lock().unwrap().push((path, content)); + Ok(()) + }) + }); + + context.developer_file_io = Some(DeveloperFileIo { + read_file, + read_file_chunk: None, + write_file, + }); + + let client = DeveloperClient::new(context).unwrap(); + + let result = client + .call_tool( + "session", + "write", + Some(object!({ + "path": "output.txt", + "content": "delegated write" + })), + None, + CancellationToken::new(), + ) + .await + .unwrap(); + + assert_eq!(result.is_error, Some(false)); + + let writes = captured_writes.lock().unwrap(); + assert_eq!(writes.len(), 1); + assert!(writes[0].0.to_string_lossy().ends_with("output.txt")); + assert_eq!(writes[0].1, "delegated write"); + } + + #[tokio::test] + async fn developer_client_delegates_read_slice_fallback() { + use crate::agents::platform_extensions::{DeveloperFileIo, ReadFileFn, WriteFileFn}; + use std::path::PathBuf; + use std::sync::Arc; + + let temp = tempfile::tempdir().unwrap(); + let mut context = test_context(temp.path().join("sessions")); + + let read_file: ReadFileFn = Arc::new(|_path: PathBuf| { + Box::pin(async move { + // Return 5 lines + Ok("line1\nline2\nline3\nline4\nline5".to_string()) + }) + }); + + let write_file: WriteFileFn = + Arc::new(|_path: PathBuf, _content: String| Box::pin(async move { Ok(()) })); + + // Explicitly set read_file_chunk to None to force fallback + context.developer_file_io = Some(DeveloperFileIo { + read_file, + read_file_chunk: None, + write_file, + }); + + let client = DeveloperClient::new(context).unwrap(); + + // Request offset=1, limit=2 (expect "line2\nline3") + let result = client + .call_tool( + "session", + "read", + Some(object!({ + "path": "test.txt", + "offset": 1, + "limit": 2 + })), + None, + CancellationToken::new(), + ) + .await + .unwrap(); + + assert_eq!(result.is_error, Some(false)); + assert_eq!(first_text(&result), "line2\nline3"); } #[cfg(not(windows))] diff --git a/crates/goose/src/agents/platform_extensions/mod.rs b/crates/goose/src/agents/platform_extensions/mod.rs index 69605746e967..b2678f6b3155 100644 --- a/crates/goose/src/agents/platform_extensions/mod.rs +++ b/crates/goose/src/agents/platform_extensions/mod.rs @@ -10,13 +10,92 @@ pub mod todo; pub mod tom; use std::collections::HashMap; +use std::future::Future; +use std::io; +use std::path::PathBuf; +use std::pin::Pin; +use std::sync::Arc; use crate::agents::mcp_client::McpClientTrait; use crate::session::Session; use once_cell::sync::Lazy; +use tokio::io::AsyncBufReadExt; pub use ext_manager::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE; +pub const MAX_READ_FILE_BYTES: usize = 1024 * 1024; + +pub type ReadFileFuture = Pin> + Send>>; +pub type ReadFileChunkFuture = Pin> + Send>>; +pub type WriteFileFuture = Pin> + Send>>; +pub type ReadFileFn = Arc ReadFileFuture + Send + Sync>; +pub type ReadFileChunkFn = Arc ReadFileChunkFuture + Send + Sync>; +pub type WriteFileFn = Arc WriteFileFuture + Send + Sync>; + +#[derive(Clone)] +pub struct DeveloperFileIo { + pub read_file: ReadFileFn, + pub read_file_chunk: Option, + pub write_file: WriteFileFn, +} + +impl DeveloperFileIo { + pub fn default_local() -> Self { + let read_file: ReadFileFn = Arc::new(|path: PathBuf| { + Box::pin(async move { + let metadata = tokio::fs::metadata(&path).await?; + let file_size = metadata.len(); + if file_size > MAX_READ_FILE_BYTES as u64 { + return Err(io::Error::other(format!( + "{} exceeds max file size of {} bytes (actual: {} bytes)", + path.display(), + MAX_READ_FILE_BYTES, + file_size + ))); + } + tokio::fs::read_to_string(path).await + }) + }); + let read_file_chunk: ReadFileChunkFn = + Arc::new(|path: PathBuf, offset: usize, limit: usize| { + Box::pin(async move { + let file = tokio::fs::File::open(path).await?; + let mut lines = tokio::io::BufReader::new(file).lines(); + let mut line_index = 0usize; + let end = offset.saturating_add(limit); + let mut out = Vec::new(); + + while let Some(line) = lines.next_line().await? { + if line_index >= offset && line_index < end { + out.push(line); + } + line_index = line_index.saturating_add(1); + if line_index >= end { + break; + } + } + + Ok(out.join("\n")) + }) + }); + let write_file: WriteFileFn = Arc::new(|path: PathBuf, content: String| { + Box::pin(async move { + if let Some(parent) = path.parent() { + if !parent.as_os_str().is_empty() { + tokio::fs::create_dir_all(parent).await?; + } + } + tokio::fs::write(path, content).await + }) + }); + Self { + read_file, + read_file_chunk: Some(read_file_chunk), + write_file, + } + } +} + // These are used by integration tests in crates/goose/tests/ #[allow(unused_imports)] pub use ext_manager::MANAGE_EXTENSIONS_TOOL_NAME; @@ -155,6 +234,7 @@ pub struct PlatformExtensionContext { Option>, pub session_manager: std::sync::Arc, pub session: Option>, + pub developer_file_io: Option, } impl PlatformExtensionContext { diff --git a/crates/goose/src/agents/platform_extensions/summon.rs b/crates/goose/src/agents/platform_extensions/summon.rs index cfb5fd6da94d..beafe12b5f14 100644 --- a/crates/goose/src/agents/platform_extensions/summon.rs +++ b/crates/goose/src/agents/platform_extensions/summon.rs @@ -1838,6 +1838,7 @@ mod tests { extension_manager: None, session_manager: Arc::new(crate::session::SessionManager::instance()), session: None, + developer_file_io: None, } } diff --git a/crates/goose/src/agents/prompt_manager.rs b/crates/goose/src/agents/prompt_manager.rs index d3bfc115e1e2..da17ca4cc7fa 100644 --- a/crates/goose/src/agents/prompt_manager.rs +++ b/crates/goose/src/agents/prompt_manager.rs @@ -413,6 +413,7 @@ mod tests { extension_manager: None, session_manager, session: Some(Arc::new(session)), + developer_file_io: None, }; let mut extensions: Vec = PLATFORM_EXTENSIONS diff --git a/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap b/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap index 6012f5361285..2d78d5d02101 100644 --- a/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap +++ b/crates/goose/src/agents/snapshots/goose__agents__prompt_manager__tests__all_platform_extensions.snap @@ -1,5 +1,6 @@ --- source: crates/goose/src/agents/prompt_manager.rs +assertion_line: 442 expression: system_prompt --- You are a general-purpose AI agent called goose, created by Block, the parent company of Square, CashApp, and Tidal. @@ -91,8 +92,10 @@ cost the user money. For editing software, prefer the flow of using tree to understand the codebase structure and file sizes. When you need to search, prefer rg which correctly respects gitignored -content. Then use cat or sed to gather the context you need, always reading before editing. -Use write and edit to efficiently make changes. Test and verify as appropriate. +content. Use read to gather file context (and use offset/limit for large files), always +reading before editing. Use write and edit to efficiently make changes. Use shell for +command execution and workflows where terminal tools are the right fit. Test and verify +as appropriate. ## summon diff --git a/crates/goose/tests/mcp_integration_test.rs b/crates/goose/tests/mcp_integration_test.rs index 3a9aa7319bdd..e433dc340583 100644 --- a/crates/goose/tests/mcp_integration_test.rs +++ b/crates/goose/tests/mcp_integration_test.rs @@ -262,6 +262,7 @@ async fn test_replayed_session( session_manager, GoosePlatform::GooseDesktop.to_string(), ExtensionManagerCapabilities { mcpui: true }, + None, )); #[allow(clippy::redundant_closure_call)] diff --git a/crates/goose/tests/providers.rs b/crates/goose/tests/providers.rs index 2cd9f94110e1..dc92b9e14b33 100644 --- a/crates/goose/tests/providers.rs +++ b/crates/goose/tests/providers.rs @@ -610,6 +610,7 @@ async fn test_provider( session_manager, GoosePlatform::GooseCli.to_string(), ExtensionManagerCapabilities { mcpui: false }, + None, )); extension_manager .add_extension(mcp_extension.clone(), None, None, None)