@sigitcodereview / sigit / commits / d9e1ff4

Read and write files through the ACP client when it offers to (#174)

A client that advertises fs.readTextFile and fs.writeTextFile serves those requests from its buffers. sigit always used the disk, so read_file missed unsaved changes and edit_file wrote underneath a buffer the user might have modified. initialize now keeps the client's fs capabilities and registers the connection with a new seam module, client_fs, shaped like the one mcp::call_tool gives MCP. read_file, create_file, edit_file and multi_edit go through the client for paths inside the session roots. Each tool is split into a parse step and a render/apply step that the disk path and the client path share, so the result text is the same. The two capabilities are independent, and the disk is the fallback: the TUI and headless modes, paths outside the roots, and any request the client fails or leaves unanswered for 30 seconds. SIGIT_CLIENT_FS=off turns the routing off. Closes #147 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Seto Elkahfi committed Oct 5, 2026 at 21:02 UTC d9e1ff4dac5bad74d2d2dafc94c2ef4f252ff9ea
6 files changed +1319 -192
.agents/AGENTS.md
+19 -1
@@ -299,6 +299,23 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b
299 every root. MCP is deliberately not on that list — `mcp::init` runs once at startup, before
300 any session exists, so a second root's `.sigit/mcp.toml` has nobody to tell. (The servers a
301 client names in `mcpServers` are a different thing and are per session; see `src/mcp.rs`.)
302 +- **`src/client_fs.rs`** — file reads and writes through the ACP client. A client that
303 + advertises `fs.readTextFile` / `fs.writeTextFile` in `initialize` serves `fs/read_text_file`
304 + and `fs/write_text_file` from its buffers, so `read_file` sees unsaved changes and
305 + `edit_file` lands in the open buffer instead of underneath it. `tools.rs` has no access to the
306 + connection, so this is a seam like the one `mcp::call_tool` gives MCP: `handle_initialize`
307 + registers a `ClientFileSystem` (`AcpClientFs` in `main.rs`, holding the connection), and
308 + `execute_tool_impl` asks `route_for` before it dispatches `read_file`, `create_file`,
309 + `edit_file` or `multi_edit`. Each of those tools is split into a parse step and a pure
310 + render/apply step (`ReadFileCall`, `CreateFileCall`, `EditCall`) that the disk path and the
311 + client path share, so the model gets the same result text either way. The two capabilities are
312 + independent: a client that only reads still gets its edits written to disk. The disk is the
313 + fallback throughout: nothing is registered in the TUI or headless modes, a path outside the
314 + session's roots is not routed, and a request the client fails or leaves unanswered for 30
315 + seconds is retried on disk with a warning in the log. The client is only ever asked from
316 + inside a spawned prompt turn, never from `handle_initialize` itself (deadlock, same as
317 + permission requests). Existence checks and `create_file`'s parent directories still use the
318 + disk. `SIGIT_CLIENT_FS=off` turns the routing off. Covered by `tests/acp_client_fs.rs`.
319 - **`src/chat.rs`** — the Unix-only ratatui TUI. Loading-spinner phase then chat; uses
320 `tokio::select!` to multiplex terminal events with streaming tokens.
321 - **`src/headless.rs`** — non-interactive `sigit run` execution for scripts, CI, and Factory
@@ -363,7 +380,8 @@ verbosity with `RUST_LOG`.
380 `OPENAI_BASE_URL` / `OPENAI_API_KEY` (provider override), `SIGIT_API_URL` (account API base,
381 default `https://sigit.si`), `SIGIT_CLOUD_URL`, `SIGIT_CONFIG_DIR` (default `~/.config/sigit`),
382 `SIGIT_MODEL`, `SIGIT_MAX_TOOL_ROUNDS` (1 to 500, default 24; the tool-round cap of a
366 -headless run or an ACP prompt turn), `SIGIT_MCP` (`off` disables MCP), `SIGIT_MCP_SMBCLOUD` (`off` drops the baked-in
383 +headless run or an ACP prompt turn), `SIGIT_MCP` (`off` disables MCP), `SIGIT_CLIENT_FS` (`off` keeps the file tools on disk even when the
384 +ACP client offers `fs/read_text_file` / `fs/write_text_file`), `SIGIT_MCP_SMBCLOUD` (`off` drops the baked-in
385 smbCloud CLI server), `SIGIT_MCP_OFFICIAL` (`off` drops the baked-in
386 server), `SIGIT_PERMISSIONS` (`allow`/`ask`/`deny` — overrides the default permission mode for
387 mutating tools; the escape hatch for clients without permission-request support),
src/client_fs.rs new
+350
@@ -0,0 +1,350 @@
1 +//! Reading and writing files through the ACP client instead of the disk.
2 +//!
3 +//! An editor that advertises `fs.readTextFile` / `fs.writeTextFile` in its
4 +//! `initialize` request will serve `fs/read_text_file` and `fs/write_text_file`
5 +//! from its own buffers. Going through it is what lets `read_file` see changes
6 +//! the user has not saved yet, and lets `edit_file` land in the open buffer
7 +//! rather than underneath it, where the editor would have to reconcile the
8 +//! file on disk with a buffer the user may have modified.
9 +//!
10 +//! The tool code in `tools.rs` has no access to the connection, so this module
11 +//! is the seam, the same shape as the one `mcp::call_tool` gives MCP: `main.rs`
12 +//! registers a [`ClientFileSystem`] once the client has said what it supports,
13 +//! and the file tools ask [`route_for`] whether a path should go through it.
14 +//!
15 +//! The disk stays the fallback everywhere. Nothing is registered in the TUI or
16 +//! headless modes, a path outside the session's roots is not the editor's to
17 +//! serve, and a request the client fails or never answers is retried on disk,
18 +//! so a client with a partial or broken implementation costs a log line rather
19 +//! than the tool call.
20 +
21 +use std::path::{Path, PathBuf};
22 +use std::sync::{Arc, RwLock};
23 +use std::time::Duration;
24 +
25 +use async_trait::async_trait;
26 +
27 +/// How long the client gets to answer one file request before the disk is used
28 +/// instead. A buffer read or write is local work for an editor; a client that
29 +/// takes longer than this has most likely dropped the request.
30 +const CLIENT_FS_TIMEOUT: Duration = Duration::from_secs(30);
31 +
32 +/// The client's side of ACP's file-system methods. Errors are plain strings:
33 +/// they only ever reach the log, since the caller falls back to the disk.
34 +#[async_trait]
35 +pub trait ClientFileSystem: Send + Sync {
36 + async fn read_text_file(&self, session_id: &str, path: &Path) -> Result<String, String>;
37 + async fn write_text_file(
38 + &self,
39 + session_id: &str,
40 + path: &Path,
41 + content: &str,
42 + ) -> Result<(), String>;
43 +}
44 +
45 +#[derive(Clone)]
46 +struct Registered {
47 + fs: Arc<dyn ClientFileSystem>,
48 + read: bool,
49 + write: bool,
50 +}
51 +
52 +/// The connected client's file system, with the two capabilities it
53 +/// advertised. Capabilities are fixed for the connection, and the process
54 +/// serves one connection, so this is set once from `initialize`.
55 +static CLIENT: RwLock<Option<Registered>> = RwLock::new(None);
56 +
57 +/// Record what the client offered in `clientCapabilities.fs`. A client that
58 +/// offered neither method leaves the disk as the only path.
59 +pub fn register(fs: Arc<dyn ClientFileSystem>, read: bool, write: bool) {
60 + let registered = (read || write).then_some(Registered { fs, read, write });
61 + if let Ok(mut guard) = CLIENT.write() {
62 + *guard = registered;
63 + }
64 +}
65 +
66 +/// `SIGIT_CLIENT_FS=off` keeps every file tool on the disk, for a client whose
67 +/// file-system methods misbehave in a way the fallback cannot see.
68 +pub fn disabled_by_env() -> bool {
69 + std::env::var("SIGIT_CLIENT_FS")
70 + .map(|value| {
71 + matches!(
72 + value.trim().to_ascii_lowercase().as_str(),
73 + "off" | "0" | "false"
74 + )
75 + })
76 + .unwrap_or(false)
77 +}
78 +
79 +/// One file's way through the client, for the session that asked.
80 +pub struct Route {
81 + fs: Arc<dyn ClientFileSystem>,
82 + session_id: String,
83 + read: bool,
84 + write: bool,
85 +}
86 +
87 +/// The client route for `path`, or `None` when the tool should use the disk:
88 +/// no client file system is registered, no session is live, or the path is
89 +/// outside the session's roots. `path` must be absolute, which is also what
90 +/// the protocol requires of the paths it carries.
91 +pub fn route_for(session_id: Option<&str>, path: &Path) -> Option<Route> {
92 + let registered = CLIENT.read().ok()?.clone()?;
93 + let session_id = session_id?;
94 + if !path.is_absolute() || !in_roots(path, &crate::workspace::project_dirs()) {
95 + return None;
96 + }
97 + Some(Route {
98 + fs: registered.fs,
99 + session_id: session_id.to_string(),
100 + read: registered.read,
101 + write: registered.write,
102 + })
103 +}
104 +
105 +impl Route {
106 + /// The file's text: the client's copy when it can read, the disk's when it
107 + /// cannot or the request failed.
108 + pub async fn read(&self, path: &Path) -> std::io::Result<String> {
109 + if self.read {
110 + let request = self.fs.read_text_file(&self.session_id, path);
111 + match tokio::time::timeout(CLIENT_FS_TIMEOUT, request).await {
112 + Ok(Ok(content)) => return Ok(content),
113 + Ok(Err(error)) => log::warn!(
114 + "fs/read_text_file failed for {}: {error}; reading from disk",
115 + path.display()
116 + ),
117 + Err(_) => log::warn!(
118 + "fs/read_text_file timed out for {}; reading from disk",
119 + path.display()
120 + ),
121 + }
122 + }
123 + std::fs::read_to_string(path)
124 + }
125 +
126 + /// Write the file's text through the client when it can write, and to the
127 + /// disk when it cannot or the request failed.
128 + pub async fn write(&self, path: &Path, content: &str) -> std::io::Result<()> {
129 + if self.write {
130 + let request = self.fs.write_text_file(&self.session_id, path, content);
131 + match tokio::time::timeout(CLIENT_FS_TIMEOUT, request).await {
132 + Ok(Ok(())) => return Ok(()),
133 + Ok(Err(error)) => log::warn!(
134 + "fs/write_text_file failed for {}: {error}; writing to disk",
135 + path.display()
136 + ),
137 + Err(_) => log::warn!(
138 + "fs/write_text_file timed out for {}; writing to disk",
139 + path.display()
140 + ),
141 + }
142 + }
143 + std::fs::write(path, content)
144 + }
145 +}
146 +
147 +/// Whether `path` sits inside one of `roots`.
148 +///
149 +/// Both sides are resolved first, so a root reached through a symlink (macOS
150 +/// spells `/tmp` as `/private/tmp`) still contains its files, and a `..` that
151 +/// climbs out of a root does not pass as being inside it.
152 +fn in_roots(path: &Path, roots: &[PathBuf]) -> bool {
153 + let path = resolve(path);
154 + roots
155 + .iter()
156 + .any(|root| path.starts_with(crate::workspace::canonical_key(root)))
157 +}
158 +
159 +/// `path` with its deepest existing ancestor canonicalized. A file that is
160 +/// about to be created cannot be canonicalized itself, but its directory can.
161 +fn resolve(path: &Path) -> PathBuf {
162 + let normalized = crate::workspace::normalize(path);
163 + let mut missing: Vec<&std::ffi::OsStr> = Vec::new();
164 + let mut existing = normalized.as_path();
165 + loop {
166 + if let Ok(canonical) = existing.canonicalize() {
167 + return missing
168 + .iter()
169 + .rev()
170 + .fold(canonical, |joined, part| joined.join(part));
171 + }
172 + match (existing.parent(), existing.file_name()) {
173 + (Some(parent), Some(name)) => {
174 + missing.push(name);
175 + existing = parent;
176 + }
177 + _ => return normalized,
178 + }
179 + }
180 +}
181 +
182 +#[cfg(test)]
183 +mod tests {
184 + use super::*;
185 +
186 + /// A client holding one buffer's text, which can be told to fail.
187 + struct FakeClient {
188 + buffer: &'static str,
189 + fail: bool,
190 + written: std::sync::Mutex<Vec<(String, PathBuf, String)>>,
191 + }
192 +
193 + impl FakeClient {
194 + fn new(buffer: &'static str, fail: bool) -> Arc<Self> {
195 + Arc::new(Self {
196 + buffer,
197 + fail,
198 + written: std::sync::Mutex::default(),
199 + })
200 + }
201 + }
202 +
203 + #[async_trait]
204 + impl ClientFileSystem for FakeClient {
205 + async fn read_text_file(&self, _session_id: &str, _path: &Path) -> Result<String, String> {
206 + if self.fail {
207 + return Err("no such buffer".to_string());
208 + }
209 + Ok(self.buffer.to_string())
210 + }
211 +
212 + async fn write_text_file(
213 + &self,
214 + session_id: &str,
215 + path: &Path,
216 + content: &str,
217 + ) -> Result<(), String> {
218 + if self.fail {
219 + return Err("read-only buffer".to_string());
220 + }
221 + self.written.lock().unwrap().push((
222 + session_id.to_string(),
223 + path.to_path_buf(),
224 + content.to_string(),
225 + ));
226 + Ok(())
227 + }
228 + }
229 +
230 + fn route(client: &Arc<FakeClient>, read: bool, write: bool) -> Route {
231 + Route {
232 + fs: Arc::clone(client) as Arc<dyn ClientFileSystem>,
233 + session_id: "session-1".to_string(),
234 + read,
235 + write,
236 + }
237 + }
238 +
239 + #[tokio::test]
240 + async fn reads_and_writes_go_to_the_client_when_it_offers_them() {
241 + let dir = scratch("client");
242 + let file = dir.join("a.txt");
243 + std::fs::write(&file, "on disk").unwrap();
244 + let client = FakeClient::new("in the buffer", false);
245 + let route = route(&client, true, true);
246 +
247 + assert_eq!(route.read(&file).await.unwrap(), "in the buffer");
248 + route.write(&file, "edited").await.unwrap();
249 +
250 + // The client owns the write, so the disk is left for it to update.
251 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "on disk");
252 + assert_eq!(
253 + *client.written.lock().unwrap(),
254 + vec![("session-1".to_string(), file.clone(), "edited".to_string())]
255 + );
256 +
257 + std::fs::remove_dir_all(&dir).ok();
258 + }
259 +
260 + #[tokio::test]
261 + async fn a_capability_the_client_did_not_offer_stays_on_disk() {
262 + let dir = scratch("partial");
263 + let file = dir.join("a.txt");
264 + std::fs::write(&file, "on disk").unwrap();
265 + let client = FakeClient::new("in the buffer", false);
266 + let route = route(&client, false, false);
267 +
268 + assert_eq!(route.read(&file).await.unwrap(), "on disk");
269 + route.write(&file, "edited").await.unwrap();
270 +
271 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "edited");
272 + assert!(client.written.lock().unwrap().is_empty());
273 +
274 + std::fs::remove_dir_all(&dir).ok();
275 + }
276 +
277 + #[tokio::test]
278 + async fn a_request_the_client_fails_falls_back_to_disk() {
279 + let dir = scratch("failing");
280 + let file = dir.join("a.txt");
281 + std::fs::write(&file, "on disk").unwrap();
282 + let client = FakeClient::new("in the buffer", true);
283 + let route = route(&client, true, true);
284 +
285 + assert_eq!(route.read(&file).await.unwrap(), "on disk");
286 + route.write(&file, "edited").await.unwrap();
287 +
288 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "edited");
289 +
290 + std::fs::remove_dir_all(&dir).ok();
291 + }
292 +
293 + fn scratch(name: &str) -> PathBuf {
294 + let dir =
295 + std::env::temp_dir().join(format!("sigit-client-fs-{name}-{}", uuid::Uuid::new_v4()));
296 + std::fs::create_dir_all(&dir).unwrap();
297 + dir
298 + }
299 +
300 + #[test]
301 + fn a_file_inside_a_root_is_in_roots() {
302 + let root = scratch("inside");
303 + std::fs::write(root.join("a.txt"), "a").unwrap();
304 +
305 + assert!(in_roots(&root.join("a.txt"), std::slice::from_ref(&root)));
306 + // Not created yet, so only its directory can be resolved.
307 + assert!(in_roots(
308 + &root.join("new").join("b.txt"),
309 + std::slice::from_ref(&root)
310 + ));
311 +
312 + std::fs::remove_dir_all(&root).ok();
313 + }
314 +
315 + #[test]
316 + fn a_file_outside_every_root_is_not() {
317 + let root = scratch("root");
318 + let other = scratch("other");
319 + std::fs::write(other.join("a.txt"), "a").unwrap();
320 +
321 + assert!(!in_roots(&other.join("a.txt"), std::slice::from_ref(&root)));
322 + // `..` out of the root names the sibling, however the path starts.
323 + let climbing = root
324 + .join("..")
325 + .join(other.file_name().unwrap())
326 + .join("a.txt");
327 + assert!(!in_roots(&climbing, std::slice::from_ref(&root)));
328 + assert!(!in_roots(&root.join("a.txt"), &[]));
329 +
330 + std::fs::remove_dir_all(&root).ok();
331 + std::fs::remove_dir_all(&other).ok();
332 + }
333 +
334 + #[cfg(unix)]
335 + #[test]
336 + fn a_symlink_out_of_the_root_is_not_inside_it() {
337 + let root = scratch("link-root");
338 + let outside = scratch("link-outside");
339 + std::fs::write(outside.join("secret.txt"), "s").unwrap();
340 + std::os::unix::fs::symlink(&outside, root.join("link")).unwrap();
341 +
342 + assert!(!in_roots(
343 + &root.join("link").join("secret.txt"),
344 + std::slice::from_ref(&root)
345 + ));
346 +
347 + std::fs::remove_dir_all(&root).ok();
348 + std::fs::remove_dir_all(&outside).ok();
349 + }
350 +}
src/main.rs
+73 -10
@@ -32,6 +32,7 @@ mod account;
32 mod backend;
33 mod browser_auth;
34 mod chat;
35 +mod client_fs;
36 mod commands;
37 mod credentials;
38 mod frontmatter;
@@ -77,13 +78,14 @@ use agent_client_protocol::schema::v1::{
78 ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse,
79 McpCapabilities, McpServer, Meta, NewSessionRequest, NewSessionResponse, PermissionOption,
80 PermissionOptionKind, Plan, PlanEntry, PlanEntryPriority, PlanEntryStatus, PromptCapabilities,
80 - PromptRequest, PromptResponse, RequestPermissionOutcome, RequestPermissionRequest,
81 - SessionAdditionalDirectoriesCapabilities, SessionCapabilities, SessionConfigOption,
82 - SessionConfigOptionCategory, SessionConfigSelectOption, SessionConfigValueId,
83 - SessionForkCapabilities, SessionId, SessionInfo, SessionListCapabilities, SessionNotification,
84 - SessionUpdate, SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, StopReason,
85 - ToolCall, ToolCallContent, ToolCallLocation, ToolCallStatus, ToolCallUpdate,
86 - ToolCallUpdateFields, ToolKind, UnstructuredCommandInput,
81 + PromptRequest, PromptResponse, ReadTextFileRequest, RequestPermissionOutcome,
82 + RequestPermissionRequest, SessionAdditionalDirectoriesCapabilities, SessionCapabilities,
83 + SessionConfigOption, SessionConfigOptionCategory, SessionConfigSelectOption,
84 + SessionConfigValueId, SessionForkCapabilities, SessionId, SessionInfo, SessionListCapabilities,
85 + SessionNotification, SessionUpdate, SetSessionConfigOptionRequest,
86 + SetSessionConfigOptionResponse, StopReason, ToolCall, ToolCallContent, ToolCallLocation,
87 + ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, UnstructuredCommandInput,
88 + WriteTextFileRequest,
89 };
90 use agent_client_protocol::{Agent, ByteStreams, Client, ConnectionTo, Responder};
91 use onde::inference::{ChatEngine, GgufModelConfig};
@@ -878,6 +880,46 @@ fn initialize_meta() -> Meta {
880 meta
881 }
882
883 +/// The editor's file system, reached over the ACP connection. Registered with
884 +/// `client_fs` at `initialize` when the client advertises `fs` capabilities.
885 +struct AcpClientFs {
886 + cx: ConnectionTo<Client>,
887 +}
888 +
889 +#[async_trait::async_trait]
890 +impl client_fs::ClientFileSystem for AcpClientFs {
891 + async fn read_text_file(
892 + &self,
893 + session_id: &str,
894 + path: &std::path::Path,
895 + ) -> Result<String, String> {
896 + self.cx
897 + .send_request(ReadTextFileRequest::new(session_id.to_string(), path))
898 + .block_task()
899 + .await
900 + .map(|response| response.content)
901 + .map_err(|error| error.to_string())
902 + }
903 +
904 + async fn write_text_file(
905 + &self,
906 + session_id: &str,
907 + path: &std::path::Path,
908 + content: &str,
909 + ) -> Result<(), String> {
910 + self.cx
911 + .send_request(WriteTextFileRequest::new(
912 + session_id.to_string(),
913 + path,
914 + content,
915 + ))
916 + .block_task()
917 + .await
918 + .map(|_| ())
919 + .map_err(|error| error.to_string())
920 + }
921 +}
922 +
923 /// What one ACP session owns, kept per session id.
924 ///
925 /// An editor runs a single sigit process for every thread it has open, but the
@@ -1659,10 +1701,31 @@ fn validate_session_roots(
1701 impl SiGitAgent {
1702 async fn handle_initialize(
1703 &self,
1662 - _req: InitializeRequest,
1704 + cx: &ConnectionTo<Client>,
1705 + req: InitializeRequest,
1706 ) -> agent_client_protocol::Result<InitializeResponse> {
1707 log::info!("initialize");
1708
1709 + // A client that serves files from its buffers gets the file tools'
1710 + // reads and writes (see `client_fs`). Only the connection is kept
1711 + // here: asking the client for anything from this handler would
1712 + // deadlock, and the tools ask from inside a spawned prompt turn.
1713 + let fs = &req.client_capabilities.fs;
1714 + if client_fs::disabled_by_env() {
1715 + log::info!("client file system: off (SIGIT_CLIENT_FS)");
1716 + } else {
1717 + log::info!(
1718 + "client file system: read={}, write={}",
1719 + fs.read_text_file,
1720 + fs.write_text_file
1721 + );
1722 + client_fs::register(
1723 + Arc::new(AcpClientFs { cx: cx.clone() }),
1724 + fs.read_text_file,
1725 + fs.write_text_file,
1726 + );
1727 + }
1728 +
1729 // Agent-handled auth method. We don't use `AuthMethod::Terminal`: editors
1730 // like Zed advertise terminal-auth capability but don't actually spawn the
1731 // login terminal for *custom* ACP agents, so the button is a silent no-op.
@@ -4781,8 +4844,8 @@ async fn run_acp_server(auto_load_local_model: bool) -> anyhow::Result<()> {
4844 .on_receive_request(
4845 {
4846 let state = Arc::clone(&state);
4784 - async move |req: InitializeRequest, responder, _cx: ConnectionTo<Client>| {
4785 - handle_response(responder, state.handle_initialize(req).await)
4847 + async move |req: InitializeRequest, responder, cx: ConnectionTo<Client>| {
4848 + handle_response(responder, state.handle_initialize(&cx, req).await)
4849 }
4850 },
4851 agent_client_protocol::on_receive_request!(),
src/tools.rs
+327 -180
@@ -462,6 +462,12 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {
462 }
463
464 async fn execute_tool_impl(name: &str, arguments: &str) -> String {
465 + // An editor that serves files from its buffers gets the file tools' reads
466 + // and writes for paths in the session's roots (see `client_fs`).
467 + if let Some(route) = client_file_route(name, arguments) {
468 + return exec_file_tool_via_client(name, arguments, &route).await;
469 + }
470 +
471 match name {
472 TASK_TOOL_NAME => exec_task(arguments).await,
473 WEB_SEARCH_TOOL_NAME => exec_web_search(arguments).await,
@@ -996,65 +1002,85 @@ fn absolute_path_string(path: &Path) -> String {
1002
1003 // ── read_file ────────────────────────────────────────────────────────────────
1004
999 -fn exec_read_file(arguments: &str) -> String {
1000 - let args: Value = match serde_json::from_str(arguments) {
1001 - Ok(v) => v,
1002 - Err(err) => return format!("Error: failed to parse arguments: {err}"),
1003 - };
1004 -
1005 - let path_str = match args.get("path").and_then(Value::as_str) {
1006 - Some(p) => p,
1007 - None => return "Error: missing required parameter \"path\"".to_string(),
1008 - };
1005 +/// A parsed `read_file` call whose path is known to name a file on disk.
1006 +struct ReadFileCall {
1007 + path: PathBuf,
1008 + start_line: Option<usize>,
1009 + end_line: Option<usize>,
1010 +}
1011
1010 - let start_line = args
1011 - .get("start_line")
1012 - .and_then(Value::as_u64)
1013 - .map(|n| n as usize);
1014 - let end_line = args
1015 - .get("end_line")
1016 - .and_then(Value::as_u64)
1017 - .map(|n| n as usize);
1012 +impl ReadFileCall {
1013 + fn parse(arguments: &str) -> Result<Self, String> {
1014 + let args: Value = serde_json::from_str(arguments)
1015 + .map_err(|err| format!("Error: failed to parse arguments: {err}"))?;
1016
1019 - let path = Path::new(path_str);
1020 - let absolute_path = absolute_path(path);
1021 - let absolute_path_str = absolute_path.display().to_string();
1017 + let path_str = args
1018 + .get("path")
1019 + .and_then(Value::as_str)
1020 + .ok_or_else(|| "Error: missing required parameter \"path\"".to_string())?;
1021 +
1022 + let start_line = args
1023 + .get("start_line")
1024 + .and_then(Value::as_u64)
1025 + .map(|n| n as usize);
1026 + let end_line = args
1027 + .get("end_line")
1028 + .and_then(Value::as_u64)
1029 + .map(|n| n as usize);
1030 +
1031 + let path = absolute_path(Path::new(path_str));
1032 + let absolute_path_str = path.display().to_string();
1033 +
1034 + if !path.exists() {
1035 + return Err(format!("Error: path does not exist: {absolute_path_str}"));
1036 + }
1037
1023 - if !absolute_path.exists() {
1024 - return format!("Error: path does not exist: {absolute_path_str}");
1025 - }
1038 + if !path.is_file() {
1039 + return Err(format!("Error: path is not a file: {absolute_path_str}"));
1040 + }
1041
1027 - if !absolute_path.is_file() {
1028 - return format!("Error: path is not a file: {absolute_path_str}");
1042 + Ok(Self {
1043 + path,
1044 + start_line,
1045 + end_line,
1046 + })
1047 }
1048
1031 - match fs::read_to_string(&absolute_path) {
1032 - Ok(contents) => {
1033 - if start_line.is_some() || end_line.is_some() {
1034 - let lines: Vec<&str> = contents.lines().collect();
1035 - let total = lines.len();
1036 - let start = start_line.unwrap_or(1).max(1);
1037 - let end = end_line.unwrap_or(total).min(total);
1049 + /// The tool result for the file's `contents`, wherever they were read from.
1050 + fn render(&self, contents: String) -> String {
1051 + let absolute_path_str = self.path.display().to_string();
1052 + if self.start_line.is_some() || self.end_line.is_some() {
1053 + let lines: Vec<&str> = contents.lines().collect();
1054 + let total = lines.len();
1055 + let start = self.start_line.unwrap_or(1).max(1);
1056 + let end = self.end_line.unwrap_or(total).min(total);
1057
1039 - if start > total {
1040 - return format!(
1041 - "Error: start_line {start} is beyond end of file ({total} lines)"
1042 - );
1043 - }
1044 -
1045 - let selected: Vec<&str> = lines[(start - 1)..end].to_vec();
1046 - let range_text = selected.join("\n");
1047 - format!("Lines {start}-{end} of {total} in {absolute_path_str}:\n{range_text}")
1048 - } else if contents.len() > READ_FILE_CHAR_LIMIT {
1049 - let truncated: String = contents.chars().take(READ_FILE_CHAR_LIMIT).collect();
1050 - format!(
1051 - "{truncated}\n\n--- truncated (showing {READ_FILE_CHAR_LIMIT} of {} characters) ---",
1052 - contents.len()
1053 - )
1054 - } else {
1055 - contents
1058 + if start > total {
1059 + return format!("Error: start_line {start} is beyond end of file ({total} lines)");
1060 }
1061 +
1062 + let selected: Vec<&str> = lines[(start - 1)..end].to_vec();
1063 + let range_text = selected.join("\n");
1064 + format!("Lines {start}-{end} of {total} in {absolute_path_str}:\n{range_text}")
1065 + } else if contents.len() > READ_FILE_CHAR_LIMIT {
1066 + let truncated: String = contents.chars().take(READ_FILE_CHAR_LIMIT).collect();
1067 + format!(
1068 + "{truncated}\n\n--- truncated (showing {READ_FILE_CHAR_LIMIT} of {} characters) ---",
1069 + contents.len()
1070 + )
1071 + } else {
1072 + contents
1073 }
1074 + }
1075 +}
1076 +
1077 +fn exec_read_file(arguments: &str) -> String {
1078 + let call = match ReadFileCall::parse(arguments) {
1079 + Ok(call) => call,
1080 + Err(error) => return error,
1081 + };
1082 + match fs::read_to_string(&call.path) {
1083 + Ok(contents) => call.render(contents),
1084 Err(err) => format!("Error: could not read file: {err}"),
1085 }
1086 }
@@ -1404,46 +1430,68 @@ fn exec_create_directory(arguments: &str) -> String {
1430 }
1431 }
1432
1407 -/// fails if file exists so the LLM is forced to use `edit_file` for modifications.
1408 -fn exec_create_file(arguments: &str) -> String {
1409 - let args: Value = match serde_json::from_str(arguments) {
1410 - Ok(v) => v,
1411 - Err(err) => return format!("Error: failed to parse arguments: {err}"),
1412 - };
1433 +/// A parsed `create_file` call, ready to write: the path is free and its
1434 +/// parent directories exist.
1435 +struct CreateFileCall {
1436 + path: PathBuf,
1437 + content: String,
1438 +}
1439
1414 - let path_str = match args.get("path").and_then(Value::as_str) {
1415 - Some(p) => p,
1416 - None => return "Error: missing required parameter \"path\"".to_string(),
1417 - };
1440 +impl CreateFileCall {
1441 + fn parse(arguments: &str) -> Result<Self, String> {
1442 + let args: Value = serde_json::from_str(arguments)
1443 + .map_err(|err| format!("Error: failed to parse arguments: {err}"))?;
1444
1419 - let content = match args.get("content").and_then(Value::as_str) {
1420 - Some(c) => c,
1421 - None => return "Error: missing required parameter \"content\"".to_string(),
1422 - };
1445 + let path_str = args
1446 + .get("path")
1447 + .and_then(Value::as_str)
1448 + .ok_or_else(|| "Error: missing required parameter \"path\"".to_string())?;
1449
1424 - let path = Path::new(path_str);
1425 - let absolute_path = absolute_path(path);
1426 - let absolute_path_str = absolute_path.display().to_string();
1450 + let content = args
1451 + .get("content")
1452 + .and_then(Value::as_str)
1453 + .ok_or_else(|| "Error: missing required parameter \"content\"".to_string())?;
1454
1428 - if absolute_path.exists() {
1429 - return format!(
1430 - "Error: file already exists: {absolute_path_str} — use edit_file to modify existing files"
1431 - );
1455 + let path = absolute_path(Path::new(path_str));
1456 +
1457 + if path.exists() {
1458 + return Err(format!(
1459 + "Error: file already exists: {} — use edit_file to modify existing files",
1460 + path.display()
1461 + ));
1462 + }
1463 +
1464 + if let Some(parent) = path.parent()
1465 + && !parent.as_os_str().is_empty()
1466 + && !parent.exists()
1467 + && let Err(err) = fs::create_dir_all(parent)
1468 + {
1469 + return Err(format!("Error: could not create parent directories: {err}"));
1470 + }
1471 +
1472 + Ok(Self {
1473 + path,
1474 + content: content.to_string(),
1475 + })
1476 }
1477
1434 - if let Some(parent) = absolute_path.parent()
1435 - && !parent.as_os_str().is_empty()
1436 - && !parent.exists()
1437 - && let Err(err) = fs::create_dir_all(parent)
1438 - {
1439 - return format!("Error: could not create parent directories: {err}");
1478 + fn written(&self) -> String {
1479 + format!(
1480 + "Created file: {} ({} bytes)",
1481 + self.path.display(),
1482 + self.content.len()
1483 + )
1484 }
1485 +}
1486
1442 - match fs::write(&absolute_path, content) {
1443 - Ok(()) => format!(
1444 - "Created file: {absolute_path_str} ({} bytes)",
1445 - content.len()
1446 - ),
1487 +/// fails if file exists so the LLM is forced to use `edit_file` for modifications.
1488 +fn exec_create_file(arguments: &str) -> String {
1489 + let call = match CreateFileCall::parse(arguments) {
1490 + Ok(call) => call,
1491 + Err(error) => return error,
1492 + };
1493 + match fs::write(&call.path, &call.content) {
1494 + Ok(()) => call.written(),
1495 Err(err) => format!("Error: could not write file: {err}"),
1496 }
1497 }
@@ -1510,136 +1558,235 @@ fn nearest_line_hint(contents: &str, old_text: &str) -> String {
1558 String::new()
1559 }
1560
1513 -fn exec_edit_file(arguments: &str) -> String {
1514 - let args: Value = match serde_json::from_str(arguments) {
1515 - Ok(v) => v,
1516 - Err(err) => return format!("Error: failed to parse arguments: {err}"),
1517 - };
1561 +/// What an edit call does to the file's text.
1562 +enum EditOps {
1563 + /// `edit_file`: one replacement.
1564 + Single {
1565 + old_text: String,
1566 + new_text: String,
1567 + replace_all: bool,
1568 + },
1569 + /// `multi_edit`: the raw edit objects, checked one at a time as they are
1570 + /// applied so a malformed one is reported by its position.
1571 + Batch(Vec<Value>),
1572 +}
1573
1519 - let path_str = match args.get("path").and_then(Value::as_str) {
1520 - Some(p) => p,
1521 - None => return "Error: missing required parameter \"path\"".to_string(),
1522 - };
1574 +/// A parsed `edit_file` or `multi_edit` call whose path is known to name a
1575 +/// file on disk.
1576 +struct EditCall {
1577 + path: PathBuf,
1578 + ops: EditOps,
1579 +}
1580
1524 - let old_text = match args.get("old_text").and_then(Value::as_str) {
1525 - Some(t) => t,
1526 - None => return "Error: missing required parameter \"old_text\"".to_string(),
1527 - };
1581 +impl EditCall {
1582 + fn parse(tool: &str, arguments: &str) -> Result<Self, String> {
1583 + let args: Value = serde_json::from_str(arguments)
1584 + .map_err(|err| format!("Error: failed to parse arguments: {err}"))?;
1585
1529 - let new_text = match args.get("new_text").and_then(Value::as_str) {
1530 - Some(t) => t,
1531 - None => return "Error: missing required parameter \"new_text\"".to_string(),
1532 - };
1586 + let path_str = args
1587 + .get("path")
1588 + .and_then(Value::as_str)
1589 + .ok_or_else(|| "Error: missing required parameter \"path\"".to_string())?;
1590
1534 - let replace_all = args
1535 - .get("replace_all")
1536 - .and_then(Value::as_bool)
1537 - .unwrap_or(false);
1591 + let ops = if tool == "multi_edit" {
1592 + match args.get("edits").and_then(Value::as_array) {
1593 + Some(edits) if !edits.is_empty() => EditOps::Batch(edits.clone()),
1594 + Some(_) => {
1595 + return Err("Error: \"edits\" must contain at least one edit".to_string());
1596 + }
1597 + None => return Err("Error: missing required parameter \"edits\"".to_string()),
1598 + }
1599 + } else {
1600 + let text = |name: &str| {
1601 + args.get(name)
1602 + .and_then(Value::as_str)
1603 + .map(str::to_string)
1604 + .ok_or_else(|| format!("Error: missing required parameter \"{name}\""))
1605 + };
1606 + EditOps::Single {
1607 + old_text: text("old_text")?,
1608 + new_text: text("new_text")?,
1609 + replace_all: args
1610 + .get("replace_all")
1611 + .and_then(Value::as_bool)
1612 + .unwrap_or(false),
1613 + }
1614 + };
1615
1539 - let path = Path::new(path_str);
1540 - let absolute_path = absolute_path(path);
1541 - let absolute_path_str = absolute_path.display().to_string();
1616 + let path = absolute_path(Path::new(path_str));
1617 + let absolute_path_str = path.display().to_string();
1618
1543 - if !absolute_path.exists() {
1544 - return format!(
1545 - "Error: file does not exist: {absolute_path_str} — use create_file for new files"
1546 - );
1619 + if !path.exists() {
1620 + return Err(format!(
1621 + "Error: file does not exist: {absolute_path_str} — use create_file for new files"
1622 + ));
1623 + }
1624 +
1625 + if !path.is_file() {
1626 + return Err(format!("Error: path is not a file: {absolute_path_str}"));
1627 + }
1628 +
1629 + Ok(Self { path, ops })
1630 + }
1631 +
1632 + /// The file's text after the edits, or the tool result explaining which
1633 + /// one did not match. A batch is all-or-nothing: each edit is applied to
1634 + /// the result of the previous one and a failure leaves nothing to write.
1635 + fn apply(&self, contents: &str) -> Result<String, String> {
1636 + let absolute_path_str = self.path.display().to_string();
1637 + match &self.ops {
1638 + EditOps::Single {
1639 + old_text,
1640 + new_text,
1641 + replace_all,
1642 + } => apply_edit(contents, old_text, new_text, *replace_all)
1643 + .map_err(|why| format!("Error: {why} (in {absolute_path_str})")),
1644 + EditOps::Batch(edits) => {
1645 + let mut working = contents.to_string();
1646 + for (idx, edit) in edits.iter().enumerate() {
1647 + let old_text =
1648 + edit.get("old_text")
1649 + .and_then(Value::as_str)
1650 + .ok_or_else(|| {
1651 + format!("Error: edit #{} is missing \"old_text\"", idx + 1)
1652 + })?;
1653 + let new_text =
1654 + edit.get("new_text")
1655 + .and_then(Value::as_str)
1656 + .ok_or_else(|| {
1657 + format!("Error: edit #{} is missing \"new_text\"", idx + 1)
1658 + })?;
1659 + let replace_all = edit
1660 + .get("replace_all")
1661 + .and_then(Value::as_bool)
1662 + .unwrap_or(false);
1663 +
1664 + working =
1665 + apply_edit(&working, old_text, new_text, replace_all).map_err(|why| {
1666 + format!(
1667 + "Error: edit #{} failed: {why}. No changes were written to {absolute_path_str}.",
1668 + idx + 1
1669 + )
1670 + })?;
1671 + }
1672 + Ok(working)
1673 + }
1674 + }
1675 }
1676
1549 - if !absolute_path.is_file() {
1550 - return format!("Error: path is not a file: {absolute_path_str}");
1677 + fn written(&self, updated: &str) -> String {
1678 + let absolute_path_str = self.path.display().to_string();
1679 + match &self.ops {
1680 + EditOps::Single { .. } => format!(
1681 + "Edited file: {absolute_path_str} ({} bytes written)",
1682 + updated.len()
1683 + ),
1684 + EditOps::Batch(edits) => format!(
1685 + "Applied {} edits to {absolute_path_str} ({} bytes written)",
1686 + edits.len(),
1687 + updated.len()
1688 + ),
1689 + }
1690 }
1691 +}
1692 +
1693 +/// `edit_file` and `multi_edit` against the disk.
1694 +fn exec_edit(tool: &str, arguments: &str) -> String {
1695 + let call = match EditCall::parse(tool, arguments) {
1696 + Ok(call) => call,
1697 + Err(error) => return error,
1698 + };
1699
1553 - let contents = match fs::read_to_string(&absolute_path) {
1700 + let contents = match fs::read_to_string(&call.path) {
1701 Ok(c) => c,
1702 Err(err) => return format!("Error: could not read file: {err}"),
1703 };
1704
1558 - let updated = match apply_edit(&contents, old_text, new_text, replace_all) {
1705 + let updated = match call.apply(&contents) {
1706 Ok(updated) => updated,
1560 - Err(why) => return format!("Error: {why} (in {absolute_path_str})"),
1707 + Err(error) => return error,
1708 };
1709
1563 - match fs::write(&absolute_path, &updated) {
1564 - Ok(()) => format!(
1565 - "Edited file: {absolute_path_str} ({} bytes written)",
1566 - updated.len()
1567 - ),
1710 + match fs::write(&call.path, &updated) {
1711 + Ok(()) => call.written(&updated),
1712 Err(err) => format!("Error: could not write file: {err}"),
1713 }
1714 }
1715
1716 +fn exec_edit_file(arguments: &str) -> String {
1717 + exec_edit("edit_file", arguments)
1718 +}
1719 +
1720 /// Apply a batch of edits to one file atomically: each edit is applied to the
1721 /// result of the previous one, and the file is only written if *every* edit
1722 /// matches. A failure leaves the file untouched.
1723 fn exec_multi_edit(arguments: &str) -> String {
1576 - let args: Value = match serde_json::from_str(arguments) {
1577 - Ok(v) => v,
1578 - Err(err) => return format!("Error: failed to parse arguments: {err}"),
1579 - };
1724 + exec_edit("multi_edit", arguments)
1725 +}
1726
1581 - let path_str = match args.get("path").and_then(Value::as_str) {
1582 - Some(p) => p,
1583 - None => return "Error: missing required parameter \"path\"".to_string(),
1584 - };
1727 +// ── File tools through the ACP client ────────────────────────────────────────
1728
1586 - let edits = match args.get("edits").and_then(Value::as_array) {
1587 - Some(e) if !e.is_empty() => e,
1588 - Some(_) => return "Error: \"edits\" must contain at least one edit".to_string(),
1589 - None => return "Error: missing required parameter \"edits\"".to_string(),
1590 - };
1729 +/// The file tools whose reads and writes can go through the editor.
1730 +const CLIENT_FILE_TOOLS: &[&str] = &["read_file", "create_file", "edit_file", "multi_edit"];
1731
1592 - let path = Path::new(path_str);
1593 - let absolute_path = absolute_path(path);
1594 - let absolute_path_str = absolute_path.display().to_string();
1595 -
1596 - if !absolute_path.exists() {
1597 - return format!(
1598 - "Error: file does not exist: {absolute_path_str} — use create_file for new files"
1599 - );
1600 - }
1601 -
1602 - if !absolute_path.is_file() {
1603 - return format!("Error: path is not a file: {absolute_path_str}");
1732 +/// The route through the client for a file tool call, when the path it names
1733 +/// should take one (see [`crate::client_fs::route_for`]).
1734 +fn client_file_route(name: &str, arguments: &str) -> Option<crate::client_fs::Route> {
1735 + if !CLIENT_FILE_TOOLS.contains(&name) {
1736 + return None;
1737 }
1738 + let args: Value = serde_json::from_str(arguments).ok()?;
1739 + let path = absolute_path(Path::new(args.get("path")?.as_str()?));
1740 + crate::client_fs::route_for(active_session().as_deref(), &path)
1741 +}
1742
1606 - let mut working = match fs::read_to_string(&absolute_path) {
1607 - Ok(c) => c,
1608 - Err(err) => return format!("Error: could not read file: {err}"),
1609 - };
1610 -
1611 - for (idx, edit) in edits.iter().enumerate() {
1612 - let old_text = match edit.get("old_text").and_then(Value::as_str) {
1613 - Some(t) => t,
1614 - None => return format!("Error: edit #{} is missing \"old_text\"", idx + 1),
1615 - };
1616 - let new_text = match edit.get("new_text").and_then(Value::as_str) {
1617 - Some(t) => t,
1618 - None => return format!("Error: edit #{} is missing \"new_text\"", idx + 1),
1619 - };
1620 - let replace_all = edit
1621 - .get("replace_all")
1622 - .and_then(Value::as_bool)
1623 - .unwrap_or(false);
1624 -
1625 - match apply_edit(&working, old_text, new_text, replace_all) {
1626 - Ok(updated) => working = updated,
1627 - Err(why) => {
1628 - return format!(
1629 - "Error: edit #{} failed: {why}. No changes were written to {absolute_path_str}.",
1630 - idx + 1
1631 - );
1743 +/// A file tool with its reads and writes sent to the client. Everything else
1744 +/// about the call (argument checks, edit matching, the result text) is the
1745 +/// disk version's, so the model cannot tell the two apart.
1746 +async fn exec_file_tool_via_client(
1747 + name: &str,
1748 + arguments: &str,
1749 + route: &crate::client_fs::Route,
1750 +) -> String {
1751 + match name {
1752 + "read_file" => {
1753 + let call = match ReadFileCall::parse(arguments) {
1754 + Ok(call) => call,
1755 + Err(error) => return error,
1756 + };
1757 + match route.read(&call.path).await {
1758 + Ok(contents) => call.render(contents),
1759 + Err(err) => format!("Error: could not read file: {err}"),
1760 + }
1761 + }
1762 + "create_file" => {
1763 + let call = match CreateFileCall::parse(arguments) {
1764 + Ok(call) => call,
1765 + Err(error) => return error,
1766 + };
1767 + match route.write(&call.path, &call.content).await {
1768 + Ok(()) => call.written(),
1769 + Err(err) => format!("Error: could not write file: {err}"),
1770 + }
1771 + }
1772 + _ => {
1773 + let call = match EditCall::parse(name, arguments) {
1774 + Ok(call) => call,
1775 + Err(error) => return error,
1776 + };
1777 + let contents = match route.read(&call.path).await {
1778 + Ok(c) => c,
1779 + Err(err) => return format!("Error: could not read file: {err}"),
1780 + };
1781 + let updated = match call.apply(&contents) {
1782 + Ok(updated) => updated,
1783 + Err(error) => return error,
1784 + };
1785 + match route.write(&call.path, &updated).await {
1786 + Ok(()) => call.written(&updated),
1787 + Err(err) => format!("Error: could not write file: {err}"),
1788 }
1789 }
1634 - }
1635 -
1636 - match fs::write(&absolute_path, &working) {
1637 - Ok(()) => format!(
1638 - "Applied {} edits to {absolute_path_str} ({} bytes written)",
1639 - edits.len(),
1640 - working.len()
1641 - ),
1642 - Err(err) => format!("Error: could not write file: {err}"),
1790 }
1791 }
1792
src/workspace.rs
+1 -1
@@ -95,7 +95,7 @@ pub fn canonical_key(path: &Path) -> PathBuf {
95
96 /// Absolute + lexically normalized: `.` dropped, `..` folded into the previous
97 /// segment. Purely textual, so it never touches the filesystem.
98 -fn normalize(path: &Path) -> PathBuf {
98 +pub(crate) fn normalize(path: &Path) -> PathBuf {
99 let absolute = if path.is_absolute() {
100 path.to_path_buf()
101 } else {
tests/acp_client_fs.rs new
+549
@@ -0,0 +1,549 @@
1 +//! The file tools go through the ACP client when it offers to serve files.
2 +//!
3 +//! An editor that advertises `fs.readTextFile` / `fs.writeTextFile` holds the
4 +//! truth about an open file: the buffer, which may have changes the disk does
5 +//! not. These tests spawn `sigit` in ACP mode against a scripted endpoint and
6 +//! play that editor: they answer `fs/read_text_file` with text that differs
7 +//! from the file on disk and record `fs/write_text_file` without touching the
8 +//! disk, so each assertion can tell which of the two the tool used.
9 +
10 +use std::collections::VecDeque;
11 +use std::io::{BufRead, BufReader, Read, Write};
12 +use std::net::TcpListener;
13 +use std::path::{Path, PathBuf};
14 +use std::process::{Child, ChildStdin, Command, Stdio};
15 +use std::sync::mpsc::{Receiver, channel};
16 +use std::sync::{Arc, Mutex};
17 +use std::time::{Duration, Instant};
18 +
19 +use serde_json::{Value, json};
20 +
21 +const TIMEOUT: Duration = Duration::from_secs(60);
22 +
23 +// ── Scripted OpenAI-compatible endpoint ─────────────────────────────────────
24 +
25 +fn sse_body(events: &[Value]) -> String {
26 + let mut body = String::new();
27 + for event in events {
28 + body.push_str("data: ");
29 + body.push_str(&event.to_string());
30 + body.push_str("\n\n");
31 + }
32 + body.push_str("data: [DONE]\n\n");
33 + body
34 +}
35 +
36 +fn sse_tool_call(id: &str, name: &str, arguments: Value) -> String {
37 + sse_body(&[json!({
38 + "choices": [{"delta": {"tool_calls": [{
39 + "index": 0,
40 + "id": id,
41 + "function": {"name": name, "arguments": arguments.to_string()},
42 + }]}}]
43 + })])
44 +}
45 +
46 +fn sse_text(text: &str) -> String {
47 + sse_body(&[json!({"choices": [{"delta": {"content": text}}]})])
48 +}
49 +
50 +/// Serves one scripted SSE response per request and records each request body.
51 +struct FakeEndpoint {
52 + port: u16,
53 + requests: Arc<Mutex<Vec<Value>>>,
54 +}
55 +
56 +impl FakeEndpoint {
57 + /// The result the model was given for the tool call `id`.
58 + fn tool_result(&self, id: &str) -> String {
59 + let requests = self.requests.lock().unwrap();
60 + requests
61 + .iter()
62 + .flat_map(|request| request["messages"].as_array().cloned().unwrap_or_default())
63 + .find(|message| message["role"] == "tool" && message["tool_call_id"] == id)
64 + .and_then(|message| message["content"].as_str().map(str::to_string))
65 + .unwrap_or_else(|| panic!("no tool result for {id} reached the endpoint"))
66 + }
67 +}
68 +
69 +fn start_fake_endpoint(responses: Vec<String>) -> FakeEndpoint {
70 + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fake endpoint");
71 + let port = listener.local_addr().unwrap().port();
72 + let requests: Arc<Mutex<Vec<Value>>> = Arc::default();
73 + let recorded = Arc::clone(&requests);
74 + let queue = Mutex::new(VecDeque::from(responses));
75 +
76 + std::thread::spawn(move || {
77 + for stream in listener.incoming() {
78 + let Ok(mut stream) = stream else { continue };
79 + let mut reader = BufReader::new(match stream.try_clone() {
80 + Ok(clone) => clone,
81 + Err(_) => continue,
82 + });
83 + let mut content_length = 0usize;
84 + loop {
85 + let mut line = String::new();
86 + if reader.read_line(&mut line).unwrap_or(0) == 0 {
87 + break;
88 + }
89 + let line = line.trim();
90 + if line.is_empty() {
91 + break;
92 + }
93 + if let Some(length) = line.to_ascii_lowercase().strip_prefix("content-length:") {
94 + content_length = length.trim().parse().unwrap_or(0);
95 + }
96 + }
97 + let mut body = vec![0u8; content_length];
98 + if reader.read_exact(&mut body).is_err() {
99 + continue;
100 + }
101 + if let Ok(request) = serde_json::from_slice::<Value>(&body) {
102 + recorded.lock().unwrap().push(request);
103 + }
104 + let payload = queue
105 + .lock()
106 + .unwrap()
107 + .pop_front()
108 + .unwrap_or_else(|| sse_text("out of scripted responses"));
109 + let response = format!(
110 + "HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\n\
111 + content-length: {}\r\nconnection: close\r\n\r\n{}",
112 + payload.len(),
113 + payload
114 + );
115 + let _ = stream.write_all(response.as_bytes());
116 + let _ = stream.flush();
117 + }
118 + });
119 +
120 + FakeEndpoint { port, requests }
121 +}
122 +
123 +// ── ACP client over the binary's stdio ──────────────────────────────────────
124 +
125 +/// What the editor's buffers hold, and what it does with a file request.
126 +#[derive(Default)]
127 +struct Editor {
128 + /// Buffer text by file name. A read of anything else is an error, the way
129 + /// an editor answers for a file it cannot open.
130 + buffers: Vec<(&'static str, &'static str)>,
131 + /// Every `fs/*` request the agent sent, in order.
132 + requests: Vec<Value>,
133 +}
134 +
135 +impl Editor {
136 + fn requests_for(&self, method: &str) -> Vec<&Value> {
137 + self.requests
138 + .iter()
139 + .filter(|request| request["method"] == method)
140 + .collect()
141 + }
142 +}
143 +
144 +struct AgentUnderTest {
145 + child: Child,
146 + stdin: ChildStdin,
147 + incoming: Receiver<Value>,
148 + next_id: u64,
149 +}
150 +
151 +fn spawn_agent(port: u16, config_dir: &Path, client_fs: Option<&str>) -> AgentUnderTest {
152 + let mut command = Command::new(env!("CARGO_BIN_EXE_sigit"));
153 + command
154 + .env("OPENAI_BASE_URL", format!("http://127.0.0.1:{port}"))
155 + .env("OPENAI_API_KEY", "test-key")
156 + .env("SIGIT_MODEL", "scripted-model")
157 + .env("SIGIT_CONFIG_DIR", config_dir)
158 + .env("SIGIT_MCP", "off")
159 + .env("SIGIT_PERMISSIONS", "allow")
160 + .env_remove("SIGIT_LOCAL_INFERENCE")
161 + .stdin(Stdio::piped())
162 + .stdout(Stdio::piped())
163 + .stderr(Stdio::null());
164 + match client_fs {
165 + None => command.env_remove("SIGIT_CLIENT_FS"),
166 + Some(value) => command.env("SIGIT_CLIENT_FS", value),
167 + };
168 + let mut child = command.spawn().expect("spawn sigit in ACP mode");
169 +
170 + let stdout = child.stdout.take().unwrap();
171 + let (message_tx, incoming) = channel();
172 + std::thread::spawn(move || {
173 + for line in BufReader::new(stdout).lines() {
174 + let Ok(line) = line else { break };
175 + if let Ok(message) = serde_json::from_str::<Value>(&line)
176 + && message_tx.send(message).is_err()
177 + {
178 + break;
179 + }
180 + }
181 + });
182 +
183 + let stdin = child.stdin.take().unwrap();
184 + AgentUnderTest {
185 + child,
186 + stdin,
187 + incoming,
188 + next_id: 0,
189 + }
190 +}
191 +
192 +impl AgentUnderTest {
193 + fn send(&mut self, message: Value) {
194 + let mut line = message.to_string();
195 + line.push('\n');
196 + self.stdin.write_all(line.as_bytes()).expect("write stdin");
197 + self.stdin.flush().expect("flush stdin");
198 + }
199 +
200 + fn request(&mut self, method: &str, params: Value) -> u64 {
201 + self.next_id += 1;
202 + let id = self.next_id;
203 + self.send(json!({"jsonrpc": "2.0", "id": id, "method": method, "params": params}));
204 + id
205 + }
206 +
207 + /// Wait for the response to request `id`, serving the agent's `fs/*`
208 + /// requests from `editor` on the way.
209 + fn wait_for_response(&mut self, id: u64, editor: &mut Editor) -> Value {
210 + let deadline = Instant::now() + TIMEOUT;
211 + loop {
212 + let remaining = deadline.saturating_duration_since(Instant::now());
213 + let Ok(message) = self.incoming.recv_timeout(remaining) else {
214 + panic!("timed out waiting for the response to request {id}");
215 + };
216 + if message["id"] == id && message.get("method").is_none() {
217 + assert!(
218 + message.get("error").is_none(),
219 + "request {id} failed: {message}"
220 + );
221 + return message;
222 + }
223 + let Some(method) = message["method"].as_str() else {
224 + continue;
225 + };
226 + if !method.starts_with("fs/") {
227 + continue;
228 + }
229 + editor.requests.push(message.clone());
230 + let request_id = message["id"].clone();
231 + let path = message["params"]["path"].as_str().unwrap_or_default();
232 + let name = Path::new(path)
233 + .file_name()
234 + .and_then(|name| name.to_str())
235 + .unwrap_or_default();
236 + let buffer = editor.buffers.iter().find(|(file, _)| *file == name);
237 + let reply = match (method, buffer) {
238 + ("fs/read_text_file", Some((_, text))) => {
239 + json!({"jsonrpc": "2.0", "id": request_id, "result": {"content": text}})
240 + }
241 + ("fs/read_text_file", None) => json!({
242 + "jsonrpc": "2.0",
243 + "id": request_id,
244 + "error": {"code": -32002, "message": "no such buffer"},
245 + }),
246 + // A write is recorded and deliberately not applied to the disk.
247 + _ => json!({"jsonrpc": "2.0", "id": request_id, "result": {}}),
248 + };
249 + self.send(reply);
250 + }
251 + }
252 +
253 + /// `initialize` with the given `fs` capabilities, then a session in `cwd`.
254 + fn open_session(&mut self, fs: Value, cwd: &Path) -> String {
255 + let mut editor = Editor::default();
256 + let id = self.request(
257 + "initialize",
258 + json!({"protocolVersion": 1, "clientCapabilities": {"fs": fs}}),
259 + );
260 + self.wait_for_response(id, &mut editor);
261 +
262 + let id = self.request("session/new", json!({"cwd": cwd, "mcpServers": []}));
263 + self.wait_for_response(id, &mut editor)["result"]["sessionId"]
264 + .as_str()
265 + .expect("session id")
266 + .to_string()
267 + }
268 +
269 + fn prompt(&mut self, session_id: &str, editor: &mut Editor) {
270 + let id = self.request(
271 + "session/prompt",
272 + json!({"sessionId": session_id, "prompt": [{"type": "text", "text": "go"}]}),
273 + );
274 + self.wait_for_response(id, editor);
275 + }
276 +}
277 +
278 +impl Drop for AgentUnderTest {
279 + fn drop(&mut self) {
280 + let _ = self.child.kill();
281 + let _ = self.child.wait();
282 + }
283 +}
284 +
285 +/// A scratch project: `config/` for the agent and `work/` as the session root.
286 +struct Scratch {
287 + root: PathBuf,
288 + config: PathBuf,
289 + work: PathBuf,
290 +}
291 +
292 +impl Scratch {
293 + fn new(name: &str) -> Self {
294 + let root = std::env::temp_dir().join(format!("sigit_acp_fs_{name}_{}", std::process::id()));
295 + let _ = std::fs::remove_dir_all(&root);
296 + let config = root.join("config");
297 + let work = root.join("work");
298 + std::fs::create_dir_all(&config).unwrap();
299 + std::fs::create_dir_all(&work).unwrap();
300 + Self { root, config, work }
301 + }
302 +}
303 +
304 +impl Drop for Scratch {
305 + fn drop(&mut self) {
306 + std::fs::remove_dir_all(&self.root).ok();
307 + }
308 +}
309 +
310 +const BOTH: fn() -> Value = || json!({"readTextFile": true, "writeTextFile": true});
311 +
312 +// ── The tests ───────────────────────────────────────────────────────────────
313 +
314 +#[test]
315 +fn read_file_sees_the_unsaved_buffer() {
316 + let scratch = Scratch::new("read");
317 + let file = scratch.work.join("notes.txt");
318 + std::fs::write(&file, "saved text").unwrap();
319 +
320 + let endpoint = start_fake_endpoint(vec![
321 + // A relative path: the request to the client still has to be absolute.
322 + sse_tool_call("call_1", "read_file", json!({"path": "notes.txt"})),
323 + sse_text("done"),
324 + ]);
325 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
326 + let session_id = agent.open_session(BOTH(), &scratch.work);
327 +
328 + let mut editor = Editor {
329 + buffers: vec![("notes.txt", "unsaved text")],
330 + ..Editor::default()
331 + };
332 + agent.prompt(&session_id, &mut editor);
333 +
334 + assert_eq!(endpoint.tool_result("call_1"), "unsaved text");
335 +
336 + let reads = editor.requests_for("fs/read_text_file");
337 + assert_eq!(reads.len(), 1, "requests: {:?}", editor.requests);
338 + assert_eq!(reads[0]["params"]["sessionId"], session_id.as_str());
339 + let sent = Path::new(reads[0]["params"]["path"].as_str().unwrap());
340 + assert!(sent.is_absolute(), "path sent to the client: {sent:?}");
341 + assert!(sent.ends_with("notes.txt"));
342 +}
343 +
344 +#[test]
345 +fn edit_file_edits_the_buffer_and_writes_through_the_client() {
346 + let scratch = Scratch::new("edit");
347 + let file = scratch.work.join("main.rs");
348 + std::fs::write(&file, "fn saved() {}\n").unwrap();
349 +
350 + let endpoint = start_fake_endpoint(vec![
351 + // `unsaved` exists only in the buffer, so the edit can only match
352 + // if the tool read the client's copy.
353 + sse_tool_call(
354 + "call_1",
355 + "edit_file",
356 + json!({"path": file, "old_text": "unsaved", "new_text": "edited"}),
357 + ),
358 + sse_tool_call(
359 + "call_2",
360 + "multi_edit",
361 + json!({"path": file, "edits": [
362 + {"old_text": "fn ", "new_text": "pub fn "},
363 + {"old_text": "{}", "new_text": "{ }"},
364 + ]}),
365 + ),
366 + sse_text("done"),
367 + ]);
368 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
369 + let session_id = agent.open_session(BOTH(), &scratch.work);
370 +
371 + let mut editor = Editor {
372 + buffers: vec![("main.rs", "fn unsaved() {}\n")],
373 + ..Editor::default()
374 + };
375 + agent.prompt(&session_id, &mut editor);
376 +
377 + let first = endpoint.tool_result("call_1");
378 + assert!(first.starts_with("Edited file:"), "edit_file said: {first}");
379 + let second = endpoint.tool_result("call_2");
380 + assert!(
381 + second.starts_with("Applied 2 edits"),
382 + "multi_edit said: {second}"
383 + );
384 +
385 + let writes = editor.requests_for("fs/write_text_file");
386 + assert_eq!(writes.len(), 2, "requests: {:?}", editor.requests);
387 + assert_eq!(writes[0]["params"]["sessionId"], session_id.as_str());
388 + assert_eq!(writes[0]["params"]["content"], "fn edited() {}\n");
389 + assert_eq!(writes[1]["params"]["content"], "pub fn unsaved() { }\n");
390 +
391 + // The editor owns the write; sigit must not also have written the disk.
392 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "fn saved() {}\n");
393 +}
394 +
395 +#[test]
396 +fn create_file_writes_through_the_client() {
397 + let scratch = Scratch::new("create");
398 + let file = scratch.work.join("src").join("new.txt");
399 +
400 + let endpoint = start_fake_endpoint(vec![
401 + sse_tool_call(
402 + "call_1",
403 + "create_file",
404 + json!({"path": file, "content": "hello"}),
405 + ),
406 + sse_text("done"),
407 + ]);
408 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
409 + let session_id = agent.open_session(BOTH(), &scratch.work);
410 +
411 + let mut editor = Editor::default();
412 + agent.prompt(&session_id, &mut editor);
413 +
414 + let result = endpoint.tool_result("call_1");
415 + assert!(
416 + result.starts_with("Created file:"),
417 + "create_file said: {result}"
418 + );
419 + let writes = editor.requests_for("fs/write_text_file");
420 + assert_eq!(writes.len(), 1, "requests: {:?}", editor.requests);
421 + assert_eq!(writes[0]["params"]["content"], "hello");
422 + assert!(!file.exists(), "sigit wrote the file itself");
423 +}
424 +
425 +#[test]
426 +fn a_client_without_fs_capabilities_gets_no_fs_requests() {
427 + let scratch = Scratch::new("nocaps");
428 + let file = scratch.work.join("notes.txt");
429 + std::fs::write(&file, "saved text").unwrap();
430 +
431 + let endpoint = start_fake_endpoint(vec![
432 + sse_tool_call("call_1", "read_file", json!({"path": file})),
433 + sse_tool_call(
434 + "call_2",
435 + "edit_file",
436 + json!({"path": file, "old_text": "saved", "new_text": "edited"}),
437 + ),
438 + sse_text("done"),
439 + ]);
440 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
441 + let session_id = agent.open_session(json!({}), &scratch.work);
442 +
443 + let mut editor = Editor {
444 + buffers: vec![("notes.txt", "unsaved text")],
445 + ..Editor::default()
446 + };
447 + agent.prompt(&session_id, &mut editor);
448 +
449 + assert_eq!(endpoint.tool_result("call_1"), "saved text");
450 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);
451 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "edited text");
452 +}
453 +
454 +#[test]
455 +fn only_the_advertised_half_goes_through_the_client() {
456 + let scratch = Scratch::new("readonly");
457 + let file = scratch.work.join("notes.txt");
458 + std::fs::write(&file, "saved text").unwrap();
459 +
460 + let endpoint = start_fake_endpoint(vec![
461 + sse_tool_call(
462 + "call_1",
463 + "edit_file",
464 + json!({"path": file, "old_text": "unsaved", "new_text": "edited"}),
465 + ),
466 + sse_text("done"),
467 + ]);
468 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
469 + let session_id = agent.open_session(json!({"readTextFile": true}), &scratch.work);
470 +
471 + let mut editor = Editor {
472 + buffers: vec![("notes.txt", "unsaved text")],
473 + ..Editor::default()
474 + };
475 + agent.prompt(&session_id, &mut editor);
476 +
477 + assert_eq!(editor.requests_for("fs/read_text_file").len(), 1);
478 + assert!(editor.requests_for("fs/write_text_file").is_empty());
479 + assert_eq!(std::fs::read_to_string(&file).unwrap(), "edited text");
480 +}
481 +
482 +#[test]
483 +fn a_path_outside_the_session_roots_stays_on_disk() {
484 + let scratch = Scratch::new("outside");
485 + let outside = scratch.root.join("elsewhere");
486 + std::fs::create_dir_all(&outside).unwrap();
487 + let file = outside.join("notes.txt");
488 + std::fs::write(&file, "saved text").unwrap();
489 +
490 + let endpoint = start_fake_endpoint(vec![
491 + sse_tool_call("call_1", "read_file", json!({"path": file})),
492 + sse_text("done"),
493 + ]);
494 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
495 + let session_id = agent.open_session(BOTH(), &scratch.work);
496 +
497 + let mut editor = Editor {
498 + buffers: vec![("notes.txt", "unsaved text")],
499 + ..Editor::default()
500 + };
501 + agent.prompt(&session_id, &mut editor);
502 +
503 + assert_eq!(endpoint.tool_result("call_1"), "saved text");
504 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);
505 +}
506 +
507 +#[test]
508 +fn a_read_the_client_refuses_falls_back_to_disk() {
509 + let scratch = Scratch::new("fallback");
510 + let file = scratch.work.join("notes.txt");
511 + std::fs::write(&file, "saved text").unwrap();
512 +
513 + let endpoint = start_fake_endpoint(vec![
514 + sse_tool_call("call_1", "read_file", json!({"path": file})),
515 + sse_text("done"),
516 + ]);
517 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);
518 + let session_id = agent.open_session(BOTH(), &scratch.work);
519 +
520 + // No buffers: the editor answers the read with an error.
521 + let mut editor = Editor::default();
522 + agent.prompt(&session_id, &mut editor);
523 +
524 + assert_eq!(editor.requests_for("fs/read_text_file").len(), 1);
525 + assert_eq!(endpoint.tool_result("call_1"), "saved text");
526 +}
527 +
528 +#[test]
529 +fn sigit_client_fs_off_keeps_the_tools_on_disk() {
530 + let scratch = Scratch::new("off");
531 + let file = scratch.work.join("notes.txt");
532 + std::fs::write(&file, "saved text").unwrap();
533 +
534 + let endpoint = start_fake_endpoint(vec![
535 + sse_tool_call("call_1", "read_file", json!({"path": file})),
536 + sse_text("done"),
537 + ]);
538 + let mut agent = spawn_agent(endpoint.port, &scratch.config, Some("off"));
539 + let session_id = agent.open_session(BOTH(), &scratch.work);
540 +
541 + let mut editor = Editor {
542 + buffers: vec![("notes.txt", "unsaved text")],
543 + ..Editor::default()
544 + };
545 + agent.prompt(&session_id, &mut editor);
546 +
547 + assert_eq!(endpoint.tool_result("call_1"), "saved text");
548 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);
549 +}