@sigitcodereview / sigit
Code Commits Issues Pulls Cloud Agent CI/CD
feature/sigit-agent-profile
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 532 lines 18.1 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 user cancelled the turn while the command ran, and it was killed.
145 Cancelled { terminal_id: String, output: Output },
146 /// The terminal started but the client failed a later request. The
147 /// command may have run, so it must not be run again.
148 Failed { terminal_id: String, error: String },
149 }
150
151 impl Route {
152 /// Run `command_str` through the platform shell in `cwd`, embedding the
153 /// terminal in `tool_call_id` when there is one, and wait for it for at
154 /// most `timeout`. The terminal is released before this returns; a client
155 /// keeps showing a released terminal that was embedded in a tool call.
156 pub async fn run(
157 &self,
158 command_str: &str,
159 cwd: &Path,
160 tool_call_id: Option<&str>,
161 timeout: Duration,
162 output_byte_limit: u64,
163 ) -> Run {
164 let session = self.session_id.as_str();
165 let (shell, args) = shell_invocation(command_str);
166 let created = tokio::time::timeout(
167 CLIENT_REQUEST_TIMEOUT,
168 self.terminal
169 .create(session, shell, &args, cwd, output_byte_limit),
170 )
171 .await;
172 let terminal_id = match created {
173 Ok(Ok(id)) => id,
174 Ok(Err(error)) => return Run::NotStarted(error),
175 Err(_) => return Run::NotStarted("terminal/create timed out".to_string()),
176 };
177 let id = terminal_id.as_str();
178
179 if let Some(tool_call_id) = tool_call_id {
180 self.terminal.embed(session, tool_call_id, id).await;
181 }
182
183 let abort = crate::tools::abort_signal(session);
184 let waited = tokio::select! {
185 waited = tokio::time::timeout(timeout, self.terminal.wait_for_exit(session, id)) => {
186 Some(waited)
187 }
188 () = abort.aborted() => None,
189 };
190 let run = match waited {
191 None => {
192 if let Err(error) = self
193 .ask("terminal/kill", self.terminal.kill(session, id))
194 .await
195 {
196 log::warn!("could not kill terminal {id}: {error}");
197 }
198 Run::Cancelled {
199 terminal_id: terminal_id.clone(),
200 output: self.read_output(id).await.unwrap_or_default(),
201 }
202 }
203 Some(Ok(Ok(status))) => match self.read_output(id).await {
204 Ok(output) => Run::Exited {
205 terminal_id: terminal_id.clone(),
206 status,
207 output,
208 },
209 Err(error) => Run::Failed {
210 terminal_id: terminal_id.clone(),
211 error,
212 },
213 },
214 Some(Ok(Err(error))) => Run::Failed {
215 terminal_id: terminal_id.clone(),
216 error: format!("terminal/wait_for_exit failed: {error}"),
217 },
218 Some(Err(_)) => {
219 // Kill before reading, so the output is what the command
220 // printed up to the timeout and nothing more.
221 if let Err(error) = self
222 .ask("terminal/kill", self.terminal.kill(session, id))
223 .await
224 {
225 log::warn!("could not kill terminal {id}: {error}");
226 }
227 Run::TimedOut {
228 terminal_id: terminal_id.clone(),
229 output: self.read_output(id).await.unwrap_or_default(),
230 }
231 }
232 };
233
234 if let Err(error) = self
235 .ask("terminal/release", self.terminal.release(session, id))
236 .await
237 {
238 log::warn!("could not release terminal {id}: {error}");
239 }
240 run
241 }
242
243 async fn read_output(&self, terminal_id: &str) -> Result<Output, String> {
244 self.ask(
245 "terminal/output",
246 self.terminal.output(&self.session_id, terminal_id),
247 )
248 .await
249 }
250
251 /// One client request, bounded by [`CLIENT_REQUEST_TIMEOUT`].
252 async fn ask<T>(
253 &self,
254 method: &str,
255 request: impl std::future::Future<Output = Result<T, String>>,
256 ) -> Result<T, String> {
257 match tokio::time::timeout(CLIENT_REQUEST_TIMEOUT, request).await {
258 Ok(Ok(value)) => Ok(value),
259 Ok(Err(error)) => Err(format!("{method} failed: {error}")),
260 Err(_) => Err(format!("{method} timed out")),
261 }
262 }
263 }
264
265 /// The shell and arguments that run `command_str`, the same shell
266 /// `spawn_shell` uses locally. ACP passes `command` and `args` separately, so
267 /// the whole command line travels as one argument and the client has nothing
268 /// to split or quote.
269 fn shell_invocation(command_str: &str) -> (&'static str, Vec<String>) {
270 #[cfg(unix)]
271 let (shell, flag) = ("sh", "-c");
272 #[cfg(windows)]
273 let (shell, flag) = ("cmd", "/C");
274 (shell, vec![flag.to_string(), command_str.to_string()])
275 }
276
277 #[cfg(test)]
278 mod tests {
279 use super::*;
280 use std::path::PathBuf;
281 use std::sync::Mutex;
282
283 /// What a `terminal/create` asked for: command, args, cwd, byte limit.
284 type Created = (String, Vec<String>, PathBuf, u64);
285
286 /// A client terminal that records what it was asked and answers from a
287 /// script.
288 #[derive(Default)]
289 struct FakeTerminal {
290 refuse_create: bool,
291 /// `None` never answers the wait, which is how a hung command looks.
292 exit: Option<ExitStatus>,
293 output: &'static str,
294 calls: Mutex<Vec<String>>,
295 created: Mutex<Option<Created>>,
296 }
297
298 impl FakeTerminal {
299 fn calls(&self) -> Vec<String> {
300 self.calls.lock().unwrap().clone()
301 }
302 fn record(&self, call: impl Into<String>) {
303 self.calls.lock().unwrap().push(call.into());
304 }
305 }
306
307 #[async_trait]
308 impl ClientTerminal for FakeTerminal {
309 async fn create(
310 &self,
311 session_id: &str,
312 command: &str,
313 args: &[String],
314 cwd: &Path,
315 output_byte_limit: u64,
316 ) -> Result<String, String> {
317 self.record(format!("create {session_id}"));
318 if self.refuse_create {
319 return Err("terminals are disabled".to_string());
320 }
321 *self.created.lock().unwrap() = Some((
322 command.to_string(),
323 args.to_vec(),
324 cwd.to_path_buf(),
325 output_byte_limit,
326 ));
327 Ok("term-1".to_string())
328 }
329
330 async fn embed(&self, _session_id: &str, tool_call_id: &str, terminal_id: &str) {
331 self.record(format!("embed {terminal_id} in {tool_call_id}"));
332 }
333
334 async fn wait_for_exit(
335 &self,
336 _session_id: &str,
337 terminal_id: &str,
338 ) -> Result<ExitStatus, String> {
339 self.record(format!("wait {terminal_id}"));
340 match &self.exit {
341 Some(status) => Ok(status.clone()),
342 None => std::future::pending().await,
343 }
344 }
345
346 async fn output(&self, _session_id: &str, terminal_id: &str) -> Result<Output, String> {
347 self.record(format!("output {terminal_id}"));
348 Ok(Output {
349 output: self.output.to_string(),
350 truncated: false,
351 })
352 }
353
354 async fn kill(&self, _session_id: &str, terminal_id: &str) -> Result<(), String> {
355 self.record(format!("kill {terminal_id}"));
356 Ok(())
357 }
358
359 async fn release(&self, _session_id: &str, terminal_id: &str) -> Result<(), String> {
360 self.record(format!("release {terminal_id}"));
361 Ok(())
362 }
363 }
364
365 fn route(terminal: &Arc<FakeTerminal>) -> Route {
366 Route {
367 terminal: Arc::clone(terminal) as Arc<dyn ClientTerminal>,
368 session_id: "session-1".to_string(),
369 }
370 }
371
372 #[tokio::test]
373 async fn a_command_runs_in_an_embedded_terminal_that_is_released() {
374 let terminal = Arc::new(FakeTerminal {
375 exit: Some(ExitStatus {
376 exit_code: Some(0),
377 signal: None,
378 }),
379 output: "hello\n",
380 ..FakeTerminal::default()
381 });
382 let cwd = std::env::temp_dir();
383
384 let run = route(&terminal)
385 .run(
386 "echo hello",
387 &cwd,
388 Some("call_1"),
389 Duration::from_secs(5),
390 1000,
391 )
392 .await;
393
394 assert_eq!(
395 run,
396 Run::Exited {
397 terminal_id: "term-1".to_string(),
398 status: ExitStatus {
399 exit_code: Some(0),
400 signal: None
401 },
402 output: Output {
403 output: "hello\n".to_string(),
404 truncated: false
405 },
406 }
407 );
408 assert_eq!(
409 terminal.calls(),
410 [
411 "create session-1",
412 "embed term-1 in call_1",
413 "wait term-1",
414 "output term-1",
415 "release term-1",
416 ]
417 );
418 let (command, args, sent_cwd, limit) = terminal.created.lock().unwrap().clone().unwrap();
419 let (shell, expected_args) = shell_invocation("echo hello");
420 assert_eq!(command, shell);
421 assert_eq!(args, expected_args);
422 assert_eq!(args.last().map(String::as_str), Some("echo hello"));
423 assert_eq!(sent_cwd, cwd);
424 assert_eq!(limit, 1000);
425 }
426
427 #[tokio::test]
428 async fn a_cancel_kills_the_terminal_then_reads_what_it_printed() {
429 let terminal = Arc::new(FakeTerminal {
430 exit: None,
431 output: "partial\n",
432 ..FakeTerminal::default()
433 });
434 let route = Route {
435 terminal: Arc::clone(&terminal) as Arc<dyn ClientTerminal>,
436 session_id: "session-cancel".to_string(),
437 };
438 crate::tools::reset_abort("session-cancel");
439 let cwd = std::env::temp_dir();
440
441 let (run, ()) = tokio::join!(
442 route.run("sleep 999", &cwd, None, Duration::from_secs(30), 1000,),
443 async {
444 tokio::time::sleep(Duration::from_millis(50)).await;
445 crate::tools::abort_session_tools("session-cancel");
446 }
447 );
448
449 assert_eq!(
450 run,
451 Run::Cancelled {
452 terminal_id: "term-1".to_string(),
453 output: Output {
454 output: "partial\n".to_string(),
455 truncated: false
456 },
457 }
458 );
459 assert_eq!(
460 terminal.calls(),
461 [
462 "create session-cancel",
463 "wait term-1",
464 "kill term-1",
465 "output term-1",
466 "release term-1",
467 ]
468 );
469 }
470
471 #[tokio::test]
472 async fn a_command_past_the_timeout_is_killed_then_read() {
473 let terminal = Arc::new(FakeTerminal {
474 exit: None,
475 output: "partial\n",
476 ..FakeTerminal::default()
477 });
478
479 let run = route(&terminal)
480 .run(
481 "sleep 999",
482 &std::env::temp_dir(),
483 None,
484 Duration::from_millis(50),
485 1000,
486 )
487 .await;
488
489 assert_eq!(
490 run,
491 Run::TimedOut {
492 terminal_id: "term-1".to_string(),
493 output: Output {
494 output: "partial\n".to_string(),
495 truncated: false
496 },
497 }
498 );
499 // No tool call to embed in, and the kill comes before the read.
500 assert_eq!(
501 terminal.calls(),
502 [
503 "create session-1",
504 "wait term-1",
505 "kill term-1",
506 "output term-1",
507 "release term-1",
508 ]
509 );
510 }
511
512 #[tokio::test]
513 async fn a_terminal_the_client_cannot_create_runs_nothing() {
514 let terminal = Arc::new(FakeTerminal {
515 refuse_create: true,
516 ..FakeTerminal::default()
517 });
518
519 let run = route(&terminal)
520 .run(
521 "true",
522 &std::env::temp_dir(),
523 Some("call_1"),
524 Duration::from_secs(5),
525 1000,
526 )
527 .await;
528
529 assert_eq!(run, Run::NotStarted("terminals are disabled".to_string()));
530 assert_eq!(terminal.calls(), ["create session-1"]);
531 }
532 }