@sigitcodereview / sigit
Code Commits Issues Pulls Cloud Agent CI/CD
main
claude/github-issue-40-0au5ua claude/linked-issue-acp-tui-u4vvpa claude/minor-release-cut-vkugow claude/sigit-acp-local-chat-cx6380 claude/sigit-cloud-agent-expansion-reox0a claude/tool-permission-system claude/zen-feynman-0u78dk development feature/acp-cancel-request feature/acp-session-list-cursor feature/acp-v1-conformance feature/agent-tools-multiedit-glob-todos-remember feature/cargo-workspace feature/hooks feature/load-local-model-explicitly feature/sigit-agent-profile feature/sigit-code-cloud feature/xcode-mcp-server fix/acp-replay-loaded-session-history fix/acp-replay-reasoning-and-attachments fix/dsml-doubled-bar-markers main release/v1.5.10 release/v1.5.6 release/v1.6.1
rs 468 lines 16 KB
Raw
1 //! Running `run_command` in the ACP client's terminal instead of a pipe.
2 //!
3 //! An editor that advertises `terminal` in its `initialize` request can run a
4 //! command itself (`terminal/create`) and show it live in the tool call when
5 //! the agent embeds the terminal as `terminal` content. The user watches the
6 //! output as it arrives and can stop the command from the editor, where a
7 //! command sigit runs itself only shows its output once it has finished.
8 //!
9 //! What `run_command` promises the model stays the same on this path: the
10 //! command runs through the platform shell in the requested directory, it is
11 //! killed (`terminal/kill`) once [`COMMAND_TIMEOUT`](crate::tools) passes, the
12 //! output is capped, and a commit it creates still gets the co-author trailer,
13 //! which `tools.rs` checks on the disk before and after the run. Its stdin is
14 //! not sigit's: the editor owns the terminal, so the JSON-RPC pipe that
15 //! `spawn_shell` has to keep away from a child is not in reach here.
16 //!
17 //! The seam has the same shape as `client_fs`: `main.rs` registers a
18 //! [`ClientTerminal`] from `initialize`, and `tools.rs` asks [`route_for`]
19 //! whether a command should go through it. Only foreground commands are
20 //! routed. Background ones stay local because `command_output` and
21 //! `kill_command` read their pipes, and a command whose directory is outside
22 //! the session's roots stays local because nothing says the editor's machine
23 //! can see it. A client that cannot create the terminal costs a log line and
24 //! the command runs locally; once the terminal exists, a failure is the tool
25 //! call's, since running the command a second time could repeat its effects.
26
27 use std::path::Path;
28 use std::sync::{Arc, RwLock};
29 use std::time::Duration;
30
31 use async_trait::async_trait;
32
33 /// How long the client gets to answer a request that is not the wait for the
34 /// command itself. Creating, reading, killing and releasing a terminal is
35 /// local work for an editor.
36 const CLIENT_REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
37
38 /// How a command in the client's terminal ended.
39 #[derive(Debug, Clone, Default, PartialEq, Eq)]
40 pub struct ExitStatus {
41 pub exit_code: Option<u32>,
42 pub signal: Option<String>,
43 }
44
45 /// What `terminal/output` returned.
46 #[derive(Debug, Clone, Default, PartialEq, Eq)]
47 pub struct Output {
48 pub output: String,
49 /// The client dropped the start of the output to stay under the limit.
50 pub truncated: bool,
51 }
52
53 /// The client's side of ACP's terminal methods. Errors are plain strings that
54 /// end up in the log or the tool result.
55 #[async_trait]
56 pub trait ClientTerminal: Send + Sync {
57 /// `terminal/create`: start `command args…` in `cwd`, returning the
58 /// terminal id.
59 async fn create(
60 &self,
61 session_id: &str,
62 command: &str,
63 args: &[String],
64 cwd: &Path,
65 output_byte_limit: u64,
66 ) -> Result<String, String>;
67 /// Show the terminal in the tool call `tool_call_id` (a `tool_call_update`
68 /// carrying `terminal` content). A notification, so it cannot fail in a
69 /// way worth reporting.
70 async fn embed(&self, session_id: &str, tool_call_id: &str, terminal_id: &str);
71 async fn wait_for_exit(
72 &self,
73 session_id: &str,
74 terminal_id: &str,
75 ) -> Result<ExitStatus, String>;
76 async fn output(&self, session_id: &str, terminal_id: &str) -> Result<Output, String>;
77 async fn kill(&self, session_id: &str, terminal_id: &str) -> Result<(), String>;
78 async fn release(&self, session_id: &str, terminal_id: &str) -> Result<(), String>;
79 }
80
81 /// The connected client's terminal. Capabilities are fixed for the connection,
82 /// and the process serves one connection, so this is set once from
83 /// `initialize`.
84 static CLIENT: RwLock<Option<Arc<dyn ClientTerminal>>> = RwLock::new(None);
85
86 /// Record the client's terminal when it advertised one, and forget any
87 /// earlier one when it did not.
88 pub fn register(terminal: Arc<dyn ClientTerminal>, advertised: bool) {
89 if let Ok(mut guard) = CLIENT.write() {
90 *guard = advertised.then_some(terminal);
91 }
92 }
93
94 /// `SIGIT_CLIENT_TERMINAL=off` keeps `run_command` local even when the client
95 /// offers a terminal, for a client whose terminal misbehaves in a way the
96 /// fallback cannot see.
97 pub fn disabled_by_env() -> bool {
98 std::env::var("SIGIT_CLIENT_TERMINAL")
99 .map(|value| {
100 matches!(
101 value.trim().to_ascii_lowercase().as_str(),
102 "off" | "0" | "false"
103 )
104 })
105 .unwrap_or(false)
106 }
107
108 /// A command's way through the client, for the session that asked.
109 pub struct Route {
110 terminal: Arc<dyn ClientTerminal>,
111 session_id: String,
112 }
113
114 /// The client route for a command run in `cwd`, or `None` when it should run
115 /// locally: no client terminal is registered, no session is live, or `cwd` is
116 /// outside the session's roots.
117 pub fn route_for(session_id: Option<&str>, cwd: &Path) -> Option<Route> {
118 let terminal = CLIENT.read().ok()?.clone()?;
119 let session_id = session_id?;
120 if !cwd.is_absolute() || !crate::client_fs::in_roots(cwd, &crate::workspace::project_dirs()) {
121 return None;
122 }
123 Some(Route {
124 terminal,
125 session_id: session_id.to_string(),
126 })
127 }
128
129 /// How a run through the client went.
130 #[derive(Debug, PartialEq, Eq)]
131 pub enum Run {
132 /// The client could not start the terminal; nothing ran, so the caller
133 /// may run the command itself.
134 NotStarted(String),
135 /// The command ran and exited (on its own, or because the user stopped it
136 /// from the editor).
137 Exited {
138 terminal_id: String,
139 status: ExitStatus,
140 output: Output,
141 },
142 /// The command was still running at the timeout and was killed.
143 TimedOut { terminal_id: String, output: Output },
144 /// The terminal started but the client failed a later request. The
145 /// command may have run, so it must not be run again.
146 Failed { terminal_id: String, error: String },
147 }
148
149 impl Route {
150 /// Run `command_str` through the platform shell in `cwd`, embedding the
151 /// terminal in `tool_call_id` when there is one, and wait for it for at
152 /// most `timeout`. The terminal is released before this returns; a client
153 /// keeps showing a released terminal that was embedded in a tool call.
154 pub async fn run(
155 &self,
156 command_str: &str,
157 cwd: &Path,
158 tool_call_id: Option<&str>,
159 timeout: Duration,
160 output_byte_limit: u64,
161 ) -> Run {
162 let session = self.session_id.as_str();
163 let (shell, args) = shell_invocation(command_str);
164 let created = tokio::time::timeout(
165 CLIENT_REQUEST_TIMEOUT,
166 self.terminal
167 .create(session, shell, &args, cwd, output_byte_limit),
168 )
169 .await;
170 let terminal_id = match created {
171 Ok(Ok(id)) => id,
172 Ok(Err(error)) => return Run::NotStarted(error),
173 Err(_) => return Run::NotStarted("terminal/create timed out".to_string()),
174 };
175 let id = terminal_id.as_str();
176
177 if let Some(tool_call_id) = tool_call_id {
178 self.terminal.embed(session, tool_call_id, id).await;
179 }
180
181 let waited = tokio::time::timeout(timeout, self.terminal.wait_for_exit(session, id)).await;
182 let run = match waited {
183 Ok(Ok(status)) => match self.read_output(id).await {
184 Ok(output) => Run::Exited {
185 terminal_id: terminal_id.clone(),
186 status,
187 output,
188 },
189 Err(error) => Run::Failed {
190 terminal_id: terminal_id.clone(),
191 error,
192 },
193 },
194 Ok(Err(error)) => Run::Failed {
195 terminal_id: terminal_id.clone(),
196 error: format!("terminal/wait_for_exit failed: {error}"),
197 },
198 Err(_) => {
199 // Kill before reading, so the output is what the command
200 // printed up to the timeout and nothing more.
201 if let Err(error) = self
202 .ask("terminal/kill", self.terminal.kill(session, id))
203 .await
204 {
205 log::warn!("could not kill terminal {id}: {error}");
206 }
207 Run::TimedOut {
208 terminal_id: terminal_id.clone(),
209 output: self.read_output(id).await.unwrap_or_default(),
210 }
211 }
212 };
213
214 if let Err(error) = self
215 .ask("terminal/release", self.terminal.release(session, id))
216 .await
217 {
218 log::warn!("could not release terminal {id}: {error}");
219 }
220 run
221 }
222
223 async fn read_output(&self, terminal_id: &str) -> Result<Output, String> {
224 self.ask(
225 "terminal/output",
226 self.terminal.output(&self.session_id, terminal_id),
227 )
228 .await
229 }
230
231 /// One client request, bounded by [`CLIENT_REQUEST_TIMEOUT`].
232 async fn ask<T>(
233 &self,
234 method: &str,
235 request: impl std::future::Future<Output = Result<T, String>>,
236 ) -> Result<T, String> {
237 match tokio::time::timeout(CLIENT_REQUEST_TIMEOUT, request).await {
238 Ok(Ok(value)) => Ok(value),
239 Ok(Err(error)) => Err(format!("{method} failed: {error}")),
240 Err(_) => Err(format!("{method} timed out")),
241 }
242 }
243 }
244
245 /// The shell and arguments that run `command_str`, the same shell
246 /// `spawn_shell` uses locally. ACP passes `command` and `args` separately, so
247 /// the whole command line travels as one argument and the client has nothing
248 /// to split or quote.
249 fn shell_invocation(command_str: &str) -> (&'static str, Vec<String>) {
250 #[cfg(unix)]
251 let (shell, flag) = ("sh", "-c");
252 #[cfg(windows)]
253 let (shell, flag) = ("cmd", "/C");
254 (shell, vec![flag.to_string(), command_str.to_string()])
255 }
256
257 #[cfg(test)]
258 mod tests {
259 use super::*;
260 use std::path::PathBuf;
261 use std::sync::Mutex;
262
263 /// What a `terminal/create` asked for: command, args, cwd, byte limit.
264 type Created = (String, Vec<String>, PathBuf, u64);
265
266 /// A client terminal that records what it was asked and answers from a
267 /// script.
268 #[derive(Default)]
269 struct FakeTerminal {
270 refuse_create: bool,
271 /// `None` never answers the wait, which is how a hung command looks.
272 exit: Option<ExitStatus>,
273 output: &'static str,
274 calls: Mutex<Vec<String>>,
275 created: Mutex<Option<Created>>,
276 }
277
278 impl FakeTerminal {
279 fn calls(&self) -> Vec<String> {
280 self.calls.lock().unwrap().clone()
281 }
282 fn record(&self, call: impl Into<String>) {
283 self.calls.lock().unwrap().push(call.into());
284 }
285 }
286
287 #[async_trait]
288 impl ClientTerminal for FakeTerminal {
289 async fn create(
290 &self,
291 session_id: &str,
292 command: &str,
293 args: &[String],
294 cwd: &Path,
295 output_byte_limit: u64,
296 ) -> Result<String, String> {
297 self.record(format!("create {session_id}"));
298 if self.refuse_create {
299 return Err("terminals are disabled".to_string());
300 }
301 *self.created.lock().unwrap() = Some((
302 command.to_string(),
303 args.to_vec(),
304 cwd.to_path_buf(),
305 output_byte_limit,
306 ));
307 Ok("term-1".to_string())
308 }
309
310 async fn embed(&self, _session_id: &str, tool_call_id: &str, terminal_id: &str) {
311 self.record(format!("embed {terminal_id} in {tool_call_id}"));
312 }
313
314 async fn wait_for_exit(
315 &self,
316 _session_id: &str,
317 terminal_id: &str,
318 ) -> Result<ExitStatus, String> {
319 self.record(format!("wait {terminal_id}"));
320 match &self.exit {
321 Some(status) => Ok(status.clone()),
322 None => std::future::pending().await,
323 }
324 }
325
326 async fn output(&self, _session_id: &str, terminal_id: &str) -> Result<Output, String> {
327 self.record(format!("output {terminal_id}"));
328 Ok(Output {
329 output: self.output.to_string(),
330 truncated: false,
331 })
332 }
333
334 async fn kill(&self, _session_id: &str, terminal_id: &str) -> Result<(), String> {
335 self.record(format!("kill {terminal_id}"));
336 Ok(())
337 }
338
339 async fn release(&self, _session_id: &str, terminal_id: &str) -> Result<(), String> {
340 self.record(format!("release {terminal_id}"));
341 Ok(())
342 }
343 }
344
345 fn route(terminal: &Arc<FakeTerminal>) -> Route {
346 Route {
347 terminal: Arc::clone(terminal) as Arc<dyn ClientTerminal>,
348 session_id: "session-1".to_string(),
349 }
350 }
351
352 #[tokio::test]
353 async fn a_command_runs_in_an_embedded_terminal_that_is_released() {
354 let terminal = Arc::new(FakeTerminal {
355 exit: Some(ExitStatus {
356 exit_code: Some(0),
357 signal: None,
358 }),
359 output: "hello\n",
360 ..FakeTerminal::default()
361 });
362 let cwd = std::env::temp_dir();
363
364 let run = route(&terminal)
365 .run(
366 "echo hello",
367 &cwd,
368 Some("call_1"),
369 Duration::from_secs(5),
370 1000,
371 )
372 .await;
373
374 assert_eq!(
375 run,
376 Run::Exited {
377 terminal_id: "term-1".to_string(),
378 status: ExitStatus {
379 exit_code: Some(0),
380 signal: None
381 },
382 output: Output {
383 output: "hello\n".to_string(),
384 truncated: false
385 },
386 }
387 );
388 assert_eq!(
389 terminal.calls(),
390 [
391 "create session-1",
392 "embed term-1 in call_1",
393 "wait term-1",
394 "output term-1",
395 "release term-1",
396 ]
397 );
398 let (command, args, sent_cwd, limit) = terminal.created.lock().unwrap().clone().unwrap();
399 let (shell, expected_args) = shell_invocation("echo hello");
400 assert_eq!(command, shell);
401 assert_eq!(args, expected_args);
402 assert_eq!(args.last().map(String::as_str), Some("echo hello"));
403 assert_eq!(sent_cwd, cwd);
404 assert_eq!(limit, 1000);
405 }
406
407 #[tokio::test]
408 async fn a_command_past_the_timeout_is_killed_then_read() {
409 let terminal = Arc::new(FakeTerminal {
410 exit: None,
411 output: "partial\n",
412 ..FakeTerminal::default()
413 });
414
415 let run = route(&terminal)
416 .run(
417 "sleep 999",
418 &std::env::temp_dir(),
419 None,
420 Duration::from_millis(50),
421 1000,
422 )
423 .await;
424
425 assert_eq!(
426 run,
427 Run::TimedOut {
428 terminal_id: "term-1".to_string(),
429 output: Output {
430 output: "partial\n".to_string(),
431 truncated: false
432 },
433 }
434 );
435 // No tool call to embed in, and the kill comes before the read.
436 assert_eq!(
437 terminal.calls(),
438 [
439 "create session-1",
440 "wait term-1",
441 "kill term-1",
442 "output term-1",
443 "release term-1",
444 ]
445 );
446 }
447
448 #[tokio::test]
449 async fn a_terminal_the_client_cannot_create_runs_nothing() {
450 let terminal = Arc::new(FakeTerminal {
451 refuse_create: true,
452 ..FakeTerminal::default()
453 });
454
455 let run = route(&terminal)
456 .run(
457 "true",
458 &std::env::temp_dir(),
459 Some("call_1"),
460 Duration::from_secs(5),
461 1000,
462 )
463 .await;
464
465 assert_eq!(run, Run::NotStarted("terminals are disabled".to_string()));
466 assert_eq!(terminal.calls(), ["create session-1"]);
467 }
468 }