@sigitcodereview / sigit / commits / 4f7cb68

Run run_command in the ACP client's terminal

When the editor advertises the terminal capability, a foreground run_command whose directory is inside the session's roots now runs through terminal/create. The terminal is embedded in the tool call as terminal content, so the user sees the output while the command runs and can stop it from the editor. Before, sigit ran every command itself and only reported the output once it ended. run_command keeps its guarantees on this path. The 120 second timeout ends in terminal/kill, the output is capped, and the co-author trailer is still amended into a new commit by checking HEAD on disk before and after (CommitWatch, now shared with the local path). The result text has the same shape as a local run. Background commands stay local, since command_output and kill_command read their pipes, and so does a command whose directory is outside the session's roots. If the client cannot create the terminal, nothing has run and the command runs locally. A failure after that is reported as the tool's error rather than retried, because the command may already have had its effects. SIGIT_CLIENT_TERMINAL=off turns the routing off. Closes #146 Co-Authored-By: siGit Code v1.6.4-acp <noreply@sigit.si> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Seto Elkahfi committed Oct 8, 2026 at 07:13 UTC 4f7cb689fb2a9ba503a5ae49ad21e64f3e8c4dec

6 files changed +1456 -95

.agents/AGENTS.md

+19 -1

@@ -342,6 +342,23 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

342 inside a spawned prompt turn, never from `handle_initialize` itself (deadlock, same as

343 permission requests). Existence checks and `create_file`'s parent directories still use the

344 disk. `SIGIT_CLIENT_FS=off` turns the routing off. Covered by `tests/acp_client_fs.rs`.

345 +- **`src/client_terminal.rs`** — `run_command` in the ACP client's terminal. A client that

346 + advertises `terminal` in `initialize` runs a foreground command itself (`terminal/create`, with

347 + the platform shell as `command` and the whole command line as one argument), and

348 + `exec_run_command_via_client` embeds the terminal in the tool call as `terminal` content, so the

349 + user watches the output live and can stop the command from the editor. The seam is

350 + `client_fs`'s: `handle_initialize` registers `AcpClientTerminal`, and `execute_tool_impl` asks

351 + `client_terminal_route` before the blocking path. Embedding needs the tool call's id, which is

352 + why `execute_tool_with_change` takes one. What `run_command` promises stays put: the 120 s

353 + timeout ends in `terminal/kill`, the output is capped, and `CommitWatch` still amends the

354 + co-author trailer, checking HEAD on disk before and after. Background commands stay local

355 + (`command_output` and `kill_command` read their pipes), and so does a command whose directory

356 + is outside the session's roots, since nothing says the editor's machine can see it. A

357 + `terminal/create` the client fails means nothing ran, so the command runs locally; a failure

358 + after that is reported as the tool's error instead, because running it again could repeat its

359 + effects. The finished card keeps the terminal and drops the result text, which still goes out

360 + as `raw_output`. `SIGIT_CLIENT_TERMINAL=off` turns the routing off. Covered by

361 + `tests/acp_client_terminal.rs`.

362 - **`src/chat.rs`** — the Unix-only ratatui TUI. Loading-spinner phase then chat; uses

363 `tokio::select!` to multiplex terminal events with streaming tokens.

364 - **`src/headless.rs`** — non-interactive `sigit run` execution for scripts, CI, and Factory

@@ -411,7 +428,8 @@ verbosity with `RUST_LOG`.

428 default `https://sigit.si`), `SIGIT_CLOUD_URL`, `SIGIT_CONFIG_DIR` (default `~/.config/sigit`),

429 `SIGIT_MODEL`, `SIGIT_MAX_TOOL_ROUNDS` (1 to 500, default 24; the tool-round cap of a

430 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

414 -ACP client offers `fs/read_text_file` / `fs/write_text_file`), `SIGIT_MCP_SMBCLOUD` (`off` drops the baked-in

431 +ACP client offers `fs/read_text_file` / `fs/write_text_file`), `SIGIT_CLIENT_TERMINAL` (`off` runs

432 +`run_command` locally even when the ACP client offers a terminal), `SIGIT_MCP_SMBCLOUD` (`off` drops the baked-in

433 smbCloud CLI server), `SIGIT_MCP_OFFICIAL` (`off` drops the baked-in

434 server), `SIGIT_PERMISSIONS` (`allow`/`ask`/`deny` — overrides the default permission mode for

435 mutating tools; the escape hatch for clients without permission-request support),

src/client_fs.rs

+3 -2

@@ -148,8 +148,9 @@ impl Route {

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 {

151 +/// climbs out of a root does not pass as being inside it. `client_terminal`

152 +/// uses the same test for a command's directory.

153 +pub(crate) fn in_roots(path: &Path, roots: &[PathBuf]) -> bool {

154 let path = resolve(path);

155 roots

156 .iter()

src/client_terminal.rs new

+468

@@ -0,0 +1,468 @@

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 +}

src/main.rs

+180 -25

@@ -33,6 +33,7 @@ mod backend;

33 mod browser_auth;

34 mod chat;

35 mod client_fs;

36 +mod client_terminal;

37 mod commands;

38 mod credentials;

39 mod frontmatter;

@@ -74,21 +75,23 @@ use agent_client_protocol::schema::v1::{

75 AgentAuthCapabilities, AgentCapabilities, AuthMethod, AuthMethodAgent, AuthenticateRequest,

76 AuthenticateResponse, AvailableCommand, AvailableCommandInput, AvailableCommandsUpdate,

77 CancelNotification, CloseSessionRequest, CloseSessionResponse, ConfigOptionUpdate,

77 - ContentBlock, ContentChunk, CurrentModeUpdate, Diff, EmbeddedResourceResource,

78 - ForkSessionRequest, ForkSessionResponse, Implementation, InitializeRequest, InitializeResponse,

79 - ListSessionsRequest, ListSessionsResponse, LoadSessionRequest, LoadSessionResponse,

80 - LogoutCapabilities, LogoutRequest, LogoutResponse, McpCapabilities, McpServer, Meta,

81 - NewSessionRequest, NewSessionResponse, PermissionOption, PermissionOptionKind, Plan, PlanEntry,

82 - PlanEntryPriority, PlanEntryStatus, PromptCapabilities, PromptRequest, PromptResponse,

83 - ReadTextFileRequest, RequestPermissionOutcome, RequestPermissionRequest, ResumeSessionRequest,

84 - ResumeSessionResponse, SessionAdditionalDirectoriesCapabilities, SessionCapabilities,

85 - SessionCloseCapabilities, SessionConfigOption, SessionConfigOptionCategory,

86 - SessionConfigSelectOption, SessionConfigValueId, SessionForkCapabilities, SessionId,

87 - SessionInfo, SessionListCapabilities, SessionMode, SessionModeState, SessionNotification,

88 - SessionResumeCapabilities, SessionUpdate, SetSessionConfigOptionRequest,

89 - SetSessionConfigOptionResponse, SetSessionModeRequest, SetSessionModeResponse, StopReason,

90 - ToolCall, ToolCallContent, ToolCallLocation, ToolCallStatus, ToolCallUpdate,

91 - ToolCallUpdateFields, ToolKind, UnstructuredCommandInput, WriteTextFileRequest,

78 + ContentBlock, ContentChunk, CreateTerminalRequest, CurrentModeUpdate, Diff,

79 + EmbeddedResourceResource, ForkSessionRequest, ForkSessionResponse, Implementation,

80 + InitializeRequest, InitializeResponse, KillTerminalRequest, ListSessionsRequest,

81 + ListSessionsResponse, LoadSessionRequest, LoadSessionResponse, LogoutCapabilities,

82 + LogoutRequest, LogoutResponse, McpCapabilities, McpServer, Meta, NewSessionRequest,

83 + NewSessionResponse, PermissionOption, PermissionOptionKind, Plan, PlanEntry, PlanEntryPriority,

84 + PlanEntryStatus, PromptCapabilities, PromptRequest, PromptResponse, ReadTextFileRequest,

85 + ReleaseTerminalRequest, RequestPermissionOutcome, RequestPermissionRequest,

86 + ResumeSessionRequest, ResumeSessionResponse, SessionAdditionalDirectoriesCapabilities,

87 + SessionCapabilities, SessionCloseCapabilities, SessionConfigOption,

88 + SessionConfigOptionCategory, SessionConfigSelectOption, SessionConfigValueId,

89 + SessionForkCapabilities, SessionId, SessionInfo, SessionListCapabilities, SessionMode,

90 + SessionModeState, SessionNotification, SessionResumeCapabilities, SessionUpdate,

91 + SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest,

92 + SetSessionModeResponse, StopReason, Terminal, TerminalOutputRequest, ToolCall, ToolCallContent,

93 + ToolCallLocation, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind,

94 + UnstructuredCommandInput, WaitForTerminalExitRequest, WriteTextFileRequest,

95 };

96 use agent_client_protocol::{Agent, ByteStreams, Client, ConnectionTo, Responder};

97 use onde::inference::{ChatEngine, GgufModelConfig};

@@ -522,9 +525,20 @@ fn file_change_content(change: &tools::FileChange) -> ToolCallContent {

525 .into()

526 }

527

525 -/// The content of a finished tool call's card: the diff of the file it wrote,

526 -/// if it wrote one, then its result text.

527 -fn tool_result_content(output: &str, change: Option<&tools::FileChange>) -> Vec<ToolCallContent> {

528 +/// The content of a finished tool call's card: the client terminal the

529 +/// command ran in, when it ran in one, which already shows its output (the

530 +/// full result text still travels as `raw_output`); otherwise the diff of the

531 +/// file it wrote, if it wrote one, then its result text.

532 +fn tool_result_content(

533 + output: &str,

534 + change: Option<&tools::FileChange>,

535 + terminal: Option<&str>,

536 +) -> Vec<ToolCallContent> {

537 + if let Some(terminal_id) = terminal {

538 + return vec![ToolCallContent::Terminal(Terminal::new(

539 + terminal_id.to_string(),

540 + ))];

541 + }

542 change

543 .map(file_change_content)

544 .into_iter()

@@ -954,6 +968,113 @@ impl client_fs::ClientFileSystem for AcpClientFs {

968 }

969 }

970

971 +/// The editor's terminal, reached over the ACP connection. Registered with

972 +/// `client_terminal` at `initialize` when the client advertises `terminal`.

973 +struct AcpClientTerminal {

974 + cx: ConnectionTo<Client>,

975 +}

976 +

977 +#[async_trait::async_trait]

978 +impl client_terminal::ClientTerminal for AcpClientTerminal {

979 + async fn create(

980 + &self,

981 + session_id: &str,

982 + command: &str,

983 + args: &[String],

984 + cwd: &std::path::Path,

985 + output_byte_limit: u64,

986 + ) -> Result<String, String> {

987 + self.cx

988 + .send_request(

989 + CreateTerminalRequest::new(session_id.to_string(), command)

990 + .args(args.to_vec())

991 + .cwd(cwd.to_path_buf())

992 + .output_byte_limit(output_byte_limit),

993 + )

994 + .block_task()

995 + .await

996 + .map(|response| response.terminal_id.to_string())

997 + .map_err(|error| error.to_string())

998 + }

999 +

1000 + async fn embed(&self, session_id: &str, tool_call_id: &str, terminal_id: &str) {

1001 + let update = SessionUpdate::ToolCallUpdate(ToolCallUpdate::new(

1002 + tool_call_id.to_string(),

1003 + ToolCallUpdateFields::new().content(vec![ToolCallContent::Terminal(Terminal::new(

1004 + terminal_id.to_string(),

1005 + ))]),

1006 + ));

1007 + if let Err(error) = self

1008 + .cx

1009 + .send_notification(SessionNotification::new(session_id.to_string(), update))

1010 + {

1011 + log::warn!("could not show terminal {terminal_id} in {tool_call_id}: {error}");

1012 + }

1013 + }

1014 +

1015 + async fn wait_for_exit(

1016 + &self,

1017 + session_id: &str,

1018 + terminal_id: &str,

1019 + ) -> Result<client_terminal::ExitStatus, String> {

1020 + self.cx

1021 + .send_request(WaitForTerminalExitRequest::new(

1022 + session_id.to_string(),

1023 + terminal_id.to_string(),

1024 + ))

1025 + .block_task()

1026 + .await

1027 + .map(|response| client_terminal::ExitStatus {

1028 + exit_code: response.exit_status.exit_code,

1029 + signal: response.exit_status.signal,

1030 + })

1031 + .map_err(|error| error.to_string())

1032 + }

1033 +

1034 + async fn output(

1035 + &self,

1036 + session_id: &str,

1037 + terminal_id: &str,

1038 + ) -> Result<client_terminal::Output, String> {

1039 + self.cx

1040 + .send_request(TerminalOutputRequest::new(

1041 + session_id.to_string(),

1042 + terminal_id.to_string(),

1043 + ))

1044 + .block_task()

1045 + .await

1046 + .map(|response| client_terminal::Output {

1047 + output: response.output,

1048 + truncated: response.truncated,

1049 + })

1050 + .map_err(|error| error.to_string())

1051 + }

1052 +

1053 + async fn kill(&self, session_id: &str, terminal_id: &str) -> Result<(), String> {

1054 + self.cx

1055 + .send_request(KillTerminalRequest::new(

1056 + session_id.to_string(),

1057 + terminal_id.to_string(),

1058 + ))

1059 + .block_task()

1060 + .await

1061 + .map(|_| ())

1062 + .map_err(|error| error.to_string())

1063 + }

1064 +

1065 + async fn release(&self, session_id: &str, terminal_id: &str) -> Result<(), String> {

1066 + self.cx

1067 + .send_request(ReleaseTerminalRequest::new(

1068 + session_id.to_string(),

1069 + terminal_id.to_string(),

1070 + ))

1071 + .block_task()

1072 + .await

1073 + .map(|_| ())

1074 + .map_err(|error| error.to_string())

1075 + }

1076 +}

1077 +

1078 /// Which request is bringing a saved session back (see `restore_session`).

1079 #[derive(Clone, Copy, PartialEq, Eq)]

1080 enum RestoreKind {

@@ -1893,6 +2014,16 @@ impl SiGitAgent {

2014 );

2015 }

2016

2017 + // Likewise a client with a terminal runs foreground commands in it,

2018 + // so the user sees them live (see `client_terminal`).

2019 + let terminal = req.client_capabilities.terminal;

2020 + if client_terminal::disabled_by_env() {

2021 + log::info!("client terminal: off (SIGIT_CLIENT_TERMINAL)");

2022 + } else {

2023 + log::info!("client terminal: {terminal}");

2024 + client_terminal::register(Arc::new(AcpClientTerminal { cx: cx.clone() }), terminal);

2025 + }

2026 +

2027 // Agent-handled auth method. We don't use `AuthMethod::Terminal`: editors

2028 // like Zed advertise terminal-auth capability but don't actually spawn the

2029 // login terminal for *custom* ACP agents, so the button is a silent no-op.

@@ -3223,6 +3354,7 @@ impl SiGitAgent {

3354 // Permission gate: read-only tools pass straight through; a

3355 // mutating tool consults policy and may ask the client.

3356 let mut change = None;

3357 + let mut terminal = None;

3358 let output = match decision {

3359 None => format!(

3360 "The tool `{}` was not executed again because the model repeated \

@@ -3231,9 +3363,14 @@ impl SiGitAgent {

3363 ),

3364 Some(decision) => match decision {

3365 permissions::Decision::Allow => {

3234 - let outcome =

3235 - tools::execute_tool_with_change(&tc.name, &tc.arguments).await;

3366 + let outcome = tools::execute_tool_with_change(

3367 + &tc.name,

3368 + &tc.arguments,

3369 + Some(&tc.id),

3370 + )

3371 + .await;

3372 change = outcome.change;

3373 + terminal = outcome.terminal;

3374 outcome.output

3375 }

3376 permissions::Decision::Deny(reason) => {

@@ -3294,10 +3431,14 @@ impl SiGitAgent {

3431 )

3432 .ok();

3433 }

3297 - let outcome =

3298 - tools::execute_tool_with_change(&tc.name, &tc.arguments)

3299 - .await;

3434 + let outcome = tools::execute_tool_with_change(

3435 + &tc.name,

3436 + &tc.arguments,

3437 + Some(&tc.id),

3438 + )

3439 + .await;

3440 change = outcome.change;

3441 + terminal = outcome.terminal;

3442 outcome.output

3443 }

3444 PermissionVerdict::Denied(reason) => {

@@ -3361,7 +3502,11 @@ impl SiGitAgent {

3502 tc.id.clone(),

3503 ToolCallUpdateFields::new()

3504 .status(ToolCallStatus::Completed)

3364 - .content(tool_result_content(&output, change.as_ref()))

3505 + .content(tool_result_content(

3506 + &output,

3507 + change.as_ref(),

3508 + terminal.as_deref(),

3509 + ))

3510 .raw_output(serde_json::Value::String(output.clone())),

3511 )),

3512 )

@@ -7033,6 +7178,7 @@ mod tests {

7178 let content = serde_json::to_value(tool_result_content(

7179 "Edited file: /repo/src/lib.rs (4 bytes written)",

7180 Some(&change),

7181 + None,

7182 ))

7183 .unwrap();

7184 assert_eq!(

@@ -7056,7 +7202,16 @@ mod tests {

7202 assert!(diff.get("oldText").is_none_or(serde_json::Value::is_null));

7203

7204 // Every other tool keeps the result text alone.

7059 - let plain = serde_json::to_value(tool_result_content("ok", None)).unwrap();

7205 + // A command that ran in the client's terminal keeps the terminal,

7206 + // which already shows its output.

7207 + let ran = serde_json::to_value(tool_result_content("Exit code 0:\nhi", None, Some("t-1")))

7208 + .unwrap();

7209 + assert_eq!(

7210 + ran,

7211 + serde_json::json!([{ "type": "terminal", "terminalId": "t-1" }])

7212 + );

7213 +

7214 + let plain = serde_json::to_value(tool_result_content("ok", None, None)).unwrap();

7215 assert_eq!(plain.as_array().unwrap().len(), 1);

7216 assert_eq!(plain[0]["type"], "content");

7217 }

src/tools.rs

+282 -67

@@ -454,6 +454,10 @@ pub struct FileChange {

454 pub struct ToolOutcome {

455 pub output: String,

456 pub change: Option<FileChange>,

457 + /// The client terminal `run_command` ran in, when it ran in one (see

458 + /// `client_terminal`). Already embedded in the tool call; the finished

459 + /// card keeps showing it.

460 + pub terminal: Option<String>,

461 }

462

463 impl From<String> for ToolOutcome {

@@ -461,16 +465,23 @@ impl From<String> for ToolOutcome {

465 Self {

466 output,

467 change: None,

468 + terminal: None,

469 }

470 }

471 }

472

473 pub async fn execute_tool(name: &str, arguments: &str) -> String {

469 - execute_tool_with_change(name, arguments).await.output

474 + execute_tool_with_change(name, arguments, None).await.output

475 }

476

472 -/// [`execute_tool`], keeping the file change for the surfaces that render it.

473 -pub async fn execute_tool_with_change(name: &str, arguments: &str) -> ToolOutcome {

477 +/// [`execute_tool`], keeping the file change and terminal for the surfaces

478 +/// that render them. `tool_call_id` is the call as the client knows it, so a

479 +/// client terminal can be shown in that call's card while it runs.

480 +pub async fn execute_tool_with_change(

481 + name: &str,

482 + arguments: &str,

483 + tool_call_id: Option<&str>,

484 +) -> ToolOutcome {

485 // Settings are read once per call (not once per hook phase) so a

486 // deployment without any [hooks] configured pays one cheap TOML parse

487 // per tool call rather than two.

@@ -483,7 +494,7 @@ pub async fn execute_tool_with_change(name: &str, arguments: &str) -> ToolOutcom

494 crate::hooks::run_pre_tool_use_hooks(&hook_settings, name, arguments, cwd);

495 }

496

486 - let result = execute_tool_impl(name, arguments).await;

497 + let result = execute_tool_impl(name, arguments, tool_call_id).await;

498

499 if let Some(cwd) = &cwd

500 && !hook_settings.post_tool_use.is_empty()

@@ -494,12 +505,19 @@ pub async fn execute_tool_with_change(name: &str, arguments: &str) -> ToolOutcom

505 result

506 }

507

497 -async fn execute_tool_impl(name: &str, arguments: &str) -> ToolOutcome {

508 +async fn execute_tool_impl(name: &str, arguments: &str, tool_call_id: Option<&str>) -> ToolOutcome {

509 // An editor that serves files from its buffers gets the file tools' reads

510 // and writes for paths in the session's roots (see `client_fs`).

511 if let Some(route) = client_file_route(name, arguments) {

512 return exec_file_tool_via_client(name, arguments, &route).await;

513 }

514 + // An editor with a terminal runs foreground commands in it, where the

515 + // user sees them live (see `client_terminal`).

516 + if name == "run_command"

517 + && let Some((call, route)) = client_terminal_route(arguments)

518 + {

519 + return exec_run_command_via_client(arguments, &call, &route, tool_call_id).await;

520 + }

521

522 match name {

523 TASK_TOOL_NAME => exec_task(arguments).await.into(),

@@ -1522,6 +1540,7 @@ impl CreateFileCall {

1540 old_text: None,

1541 new_text: self.content,

1542 }),

1543 + terminal: None,

1544 }

1545 }

1546 }

@@ -1741,6 +1760,7 @@ impl EditCall {

1760 old_text: Some(contents),

1761 new_text: updated,

1762 }),

1763 + terminal: None,

1764 }

1765 }

1766 }

@@ -2506,37 +2526,125 @@ fn ensure_commit_co_author(cwd: &Path) -> Option<String> {

2526 }

2527 }

2528

2529 +/// A parsed `run_command` call whose working directory exists.

2530 +struct RunCommandCall {

2531 + command: String,

2532 + cwd: PathBuf,

2533 + background: bool,

2534 +}

2535 +

2536 +impl RunCommandCall {

2537 + fn parse(arguments: &str) -> Result<Self, String> {

2538 + let args: Value = serde_json::from_str(arguments)

2539 + .map_err(|err| format!("Error: failed to parse arguments: {err}"))?;

2540 +

2541 + let command = args

2542 + .get("command")

2543 + .and_then(Value::as_str)

2544 + .ok_or_else(|| "Error: missing required parameter \"command\"".to_string())?;

2545 +

2546 + let default_cwd = std::env::var("HOME").unwrap_or_else(|_| ".".to_string());

2547 + let cwd = args

2548 + .get("cwd")

2549 + .and_then(Value::as_str)

2550 + .unwrap_or(&default_cwd);

2551 + let cwd = absolute_path(Path::new(cwd));

2552 +

2553 + if !cwd.exists() {

2554 + return Err(format!(

2555 + "Error: working directory does not exist: {}",

2556 + cwd.display()

2557 + ));

2558 + }

2559 +

2560 + Ok(Self {

2561 + command: command.to_string(),

2562 + cwd,

2563 + background: args

2564 + .get("run_in_background")

2565 + .and_then(Value::as_bool)

2566 + .unwrap_or(false),

2567 + })

2568 + }

2569 +}

2570 +

2571 +/// Co-author attribution for a foreground command: where HEAD was before a

2572 +/// command that looks like it may commit, so a new commit can be detected

2573 +/// afterwards. The string check is only a cheap trigger — a false positive

2574 +/// costs one `git rev-parse` and nothing else. Background tasks skip it:

2575 +/// their commits finish after the tool returns, when there is nothing to

2576 +/// amend from.

2577 +struct CommitWatch {

2578 + cwd: PathBuf,

2579 + head_before: Option<String>,

2580 +}

2581 +

2582 +impl CommitWatch {

2583 + fn start(command_str: &str, cwd: &Path) -> Option<Self> {

2584 + (command_str.contains("git") && command_str.contains("commit")).then(|| Self {

2585 + cwd: cwd.to_path_buf(),

2586 + head_before: git_head(cwd),

2587 + })

2588 + }

2589 +

2590 + /// A new commit appeared under the command: make sure it carries the

2591 + /// siGit co-author trailer (see `ensure_commit_co_author`), and say so at

2592 + /// the end of `output` when it had to be amended in.

2593 + fn finish(self, output: &mut String) {

2594 + let head_after = git_head(&self.cwd);

2595 + if head_after.is_some()

2596 + && head_after != self.head_before

2597 + && let Some(note) = ensure_commit_co_author(&self.cwd)

2598 + {

2599 + if !output.is_empty() && !output.ends_with('\n') {

2600 + output.push('\n');

2601 + }

2602 + output.push_str(&note);

2603 + }

2604 + }

2605 +}

2606 +

2607 +/// The tool result for a finished foreground command, from its combined

2608 +/// output. `exit_code` is `None` when the command did not exit on its own:

2609 +/// killed by `signal` when the client says which, otherwise reported as

2610 +/// exit code -1.

2611 +fn command_result(exit_code: Option<i64>, signal: Option<&str>, combined: String) -> String {

2612 + let truncated = if combined.len() > COMMAND_OUTPUT_LIMIT {

2613 + let truncated_str = &combined[..COMMAND_OUTPUT_LIMIT];

2614 + format!("{truncated_str}\n\n… (output truncated at {COMMAND_OUTPUT_LIMIT} bytes)")

2615 + } else {

2616 + combined

2617 + };

2618 +

2619 + let ended = match (exit_code, signal) {

2620 + (Some(code), _) => format!("exit code {code}"),

2621 + (None, Some(signal)) => format!("killed by signal {signal}"),

2622 + (None, None) => "exit code -1".to_string(),

2623 + };

2624 + if exit_code == Some(0) {

2625 + if truncated.is_empty() {

2626 + format!("Command succeeded ({ended}) with no output.")

2627 + } else {

2628 + format!("Exit code 0:\n{truncated}")

2629 + }

2630 + } else {

2631 + format!("Command failed ({ended}):\n{truncated}")

2632 + }

2633 +}

2634 +

2635 /// runs via `sh -c` / `cmd /C`; killed after COMMAND_TIMEOUT unless

2636 /// `run_in_background` is set, in which case the child is registered as a

2637 /// background task and polled with `command_output` / stopped with

2638 /// `kill_command`.

2639 fn exec_run_command(arguments: &str, owner: Option<&str>) -> String {

2514 - let args: Value = match serde_json::from_str(arguments) {

2515 - Ok(v) => v,

2516 - Err(err) => return format!("Error: failed to parse arguments: {err}"),

2517 - };

2518 -

2519 - let command_str = match args.get("command").and_then(Value::as_str) {

2520 - Some(c) => c,

2521 - None => return "Error: missing required parameter \"command\"".to_string(),

2640 + let call = match RunCommandCall::parse(arguments) {

2641 + Ok(call) => call,

2642 + Err(error) => return error,

2643 };

2523 -

2524 - let default_cwd = std::env::var("HOME").unwrap_or_else(|_| ".".to_string());

2525 - let cwd = args

2526 - .get("cwd")

2527 - .and_then(Value::as_str)

2528 - .unwrap_or(&default_cwd);

2529 - let cwd_path = absolute_path(Path::new(cwd));

2644 + let command_str = call.command.as_str();

2645 + let cwd_path = call.cwd;

2646 let cwd_str = cwd_path.display().to_string();

2531 -

2532 - if !cwd_path.exists() {

2533 - return format!("Error: working directory does not exist: {cwd_str}");

2534 - }

2535 -

2536 - let run_in_background = args

2537 - .get("run_in_background")

2538 - .and_then(Value::as_bool)

2539 - .unwrap_or(false);

2647 + let run_in_background = call.background;

2648

2649 log::info!("run_command: `{command_str}` in `{cwd_str}` (background: {run_in_background})");

2650

@@ -2544,18 +2652,7 @@ fn exec_run_command(arguments: &str, owner: Option<&str>) -> String {

2652 return start_background_task(command_str, &cwd_path, owner.map(str::to_string));

2653 }

2654

2547 - // Co-author attribution: note where HEAD is before a command that looks

2548 - // like it may commit, so a new commit can be detected afterwards. The

2549 - // string check is only a cheap trigger — a false positive costs one

2550 - // `git rev-parse` and nothing else. Background tasks skip the gate: their

2551 - // commits finish after the tool returns, when there is nothing to amend

2552 - // from.

2553 - let may_commit = command_str.contains("git") && command_str.contains("commit");

2554 - let head_before = if may_commit {

2555 - git_head(&cwd_path)

2556 - } else {

2557 - None

2558 - };

2655 + let commit_watch = CommitWatch::start(command_str, &cwd_path);

2656

2657 let mut child = match spawn_shell(command_str, &cwd_path) {

2658 Ok(c) => c,

@@ -2615,41 +2712,123 @@ fn exec_run_command(arguments: &str, owner: Option<&str>) -> String {

2712 }

2713 }

2714

2618 - let exit_code = status.code().unwrap_or(-1);

2715 let mut combined = String::new();

2716 combined.push_str(&String::from_utf8_lossy(&stdout_bytes));

2717 combined.push_str(&String::from_utf8_lossy(&stderr_bytes));

2718

2623 - // A new commit appeared under this command: make sure it carries the

2624 - // siGit co-author trailer (see `ensure_commit_co_author`).

2625 - if may_commit {

2626 - let head_after = git_head(&cwd_path);

2627 - if head_after.is_some()

2628 - && head_after != head_before

2629 - && let Some(note) = ensure_commit_co_author(&cwd_path)

2630 - {

2631 - if !combined.is_empty() && !combined.ends_with('\n') {

2632 - combined.push('\n');

2633 - }

2634 - combined.push_str(&note);

2635 - }

2719 + if let Some(watch) = commit_watch {

2720 + watch.finish(&mut combined);

2721 }

2722

2638 - let truncated = if combined.len() > COMMAND_OUTPUT_LIMIT {

2639 - let truncated_str = &combined[..COMMAND_OUTPUT_LIMIT];

2640 - format!("{truncated_str}\n\n… (output truncated at {COMMAND_OUTPUT_LIMIT} bytes)")

2641 - } else {

2642 - combined

2643 - };

2723 + command_result(status.code().map(i64::from), None, combined)

2724 +}

2725

2645 - if status.success() {

2646 - if truncated.is_empty() {

2647 - format!("Command succeeded (exit code {exit_code}) with no output.")

2648 - } else {

2649 - format!("Exit code {exit_code}:\n{truncated}")

2726 +/// The client terminal route for a `run_command` call, when it should take

2727 +/// one: a foreground command whose directory is in the session's roots, with

2728 +/// a client terminal registered (see [`crate::client_terminal::route_for`]).

2729 +fn client_terminal_route(

2730 + arguments: &str,

2731 +) -> Option<(RunCommandCall, crate::client_terminal::Route)> {

2732 + let call = RunCommandCall::parse(arguments).ok()?;

2733 + if call.background {

2734 + return None;

2735 + }

2736 + let route = crate::client_terminal::route_for(active_session().as_deref(), &call.cwd)?;

2737 + Some((call, route))

2738 +}

2739 +

2740 +/// A foreground `run_command` in the client's terminal, embedded in the tool

2741 +/// call `tool_call_id`. The result text has the local path's shape, so the

2742 +/// model cannot tell the two apart; a client that cannot start the terminal

2743 +/// gets the command run locally instead.

2744 +async fn exec_run_command_via_client(

2745 + arguments: &str,

2746 + call: &RunCommandCall,

2747 + route: &crate::client_terminal::Route,

2748 + tool_call_id: Option<&str>,

2749 +) -> ToolOutcome {

2750 + use crate::client_terminal::Run;

2751 +

2752 + log::info!(

2753 + "run_command: `{}` in `{}` (client terminal)",

2754 + call.command,

2755 + call.cwd.display()

2756 + );

2757 + let commit_watch = CommitWatch::start(&call.command, &call.cwd);

2758 +

2759 + let run = route

2760 + .run(

2761 + &call.command,

2762 + &call.cwd,

2763 + tool_call_id,

2764 + COMMAND_TIMEOUT,

2765 + COMMAND_OUTPUT_LIMIT as u64,

2766 + )

2767 + .await;

2768 +

2769 + let (output, terminal_id) = match run {

2770 + Run::NotStarted(error) => {

2771 + log::warn!("client terminal could not start the command: {error}; running it locally");

2772 + let arguments = arguments.to_owned();

2773 + let owner = active_session();

2774 + return tokio::task::spawn_blocking(move || {

2775 + exec_run_command(&arguments, owner.as_deref())

2776 + })

2777 + .await

2778 + .unwrap_or_else(|err| format!("Error: run_command task failed: {err}"))

2779 + .into();

2780 + }

2781 + Run::Exited {

2782 + terminal_id,

2783 + status,

2784 + output,

2785 + } => {

2786 + let mut combined = client_output_text(output);

2787 + if let Some(watch) = commit_watch {

2788 + watch.finish(&mut combined);

2789 + }

2790 + let result = command_result(

2791 + status.exit_code.map(i64::from),

2792 + status.signal.as_deref(),

2793 + combined,

2794 + );

2795 + (result, terminal_id)

2796 + }

2797 + Run::TimedOut {

2798 + terminal_id,

2799 + output,

2800 + } => {

2801 + let mut result = format!(

2802 + "Error: command timed out after {} seconds and was killed.",

2803 + COMMAND_TIMEOUT.as_secs()

2804 + );

2805 + let combined = client_output_text(output);

2806 + if !combined.is_empty() {

2807 + result.push_str("\nOutput before it was killed:\n");

2808 + result.push_str(&combined);

2809 + }

2810 + (result, terminal_id)

2811 }

2812 + Run::Failed { terminal_id, error } => (format!("Error: {error}"), terminal_id),

2813 + };

2814 +

2815 + ToolOutcome {

2816 + output,

2817 + change: None,

2818 + terminal: Some(terminal_id),

2819 + }

2820 +}

2821 +

2822 +/// The text of a client terminal's output, saying so when the client dropped

2823 +/// its start to stay under the byte limit.

2824 +fn client_output_text(output: crate::client_terminal::Output) -> String {

2825 + if output.truncated {

2826 + format!(

2827 + "… (earlier output dropped to stay under {COMMAND_OUTPUT_LIMIT} bytes)\n{}",

2828 + output.output

2829 + )

2830 } else {

2652 - format!("Command failed (exit code {exit_code}):\n{truncated}")

2831 + output.output

2832 }

2833 }

2834

@@ -3504,6 +3683,42 @@ mod tests {

3683 let _ = fs::remove_dir_all(&dir);

3684 }

3685

3686 + #[test]

3687 + fn command_results_read_the_same_wherever_the_command_ran() {

3688 + assert_eq!(

3689 + command_result(Some(0), None, "hi\n".to_string()),

3690 + "Exit code 0:\nhi\n"

3691 + );

3692 + assert_eq!(

3693 + command_result(Some(0), None, String::new()),

3694 + "Command succeeded (exit code 0) with no output."

3695 + );

3696 + assert_eq!(

3697 + command_result(Some(2), None, "boom\n".to_string()),

3698 + "Command failed (exit code 2):\nboom\n"

3699 + );

3700 + // Stopped from the editor: the client names the signal.

3701 + assert_eq!(

3702 + command_result(None, Some("SIGTERM"), "partial\n".to_string()),

3703 + "Command failed (killed by signal SIGTERM):\npartial\n"

3704 + );

3705 + // A local process killed by a signal has no code to report.

3706 + assert_eq!(

3707 + command_result(None, None, String::new()),

3708 + "Command failed (exit code -1):\n"

3709 + );

3710 +

3711 + let dropped = client_output_text(crate::client_terminal::Output {

3712 + output: "tail\n".to_string(),

3713 + truncated: true,

3714 + });

3715 + assert!(

3716 + dropped.starts_with("… (earlier output dropped"),

3717 + "{dropped}"

3718 + );

3719 + assert!(dropped.ends_with("\ntail\n"), "{dropped}");

3720 + }

3721 +

3722 #[test]

3723 fn written_files_report_their_change() {

3724 let dir = std::env::temp_dir().join("sigit_test_file_change");

tests/acp_client_terminal.rs new

+504

@@ -0,0 +1,504 @@

1 +//! `run_command` runs in the ACP client's terminal when it offers one.

2 +//!

3 +//! An editor that advertises `terminal` can run a command itself and show it

4 +//! live in the tool call. These tests spawn `sigit` in ACP mode against a

5 +//! scripted endpoint and play that editor: it answers `terminal/*` with

6 +//! output the command could not have printed locally, so each assertion can

7 +//! tell where the command ran.

8 +

9 +use std::collections::VecDeque;

10 +use std::io::{BufRead, BufReader, Read, Write};

11 +use std::net::TcpListener;

12 +use std::path::{Path, PathBuf};

13 +use std::process::{Child, ChildStdin, Command, Stdio};

14 +use std::sync::mpsc::{Receiver, channel};

15 +use std::sync::{Arc, Mutex};

16 +use std::time::{Duration, Instant};

17 +

18 +use serde_json::{Value, json};

19 +

20 +const TIMEOUT: Duration = Duration::from_secs(60);

21 +

22 +// ── Scripted OpenAI-compatible endpoint ─────────────────────────────────────

23 +

24 +fn sse_body(events: &[Value]) -> String {

25 + let mut body = String::new();

26 + for event in events {

27 + body.push_str("data: ");

28 + body.push_str(&event.to_string());

29 + body.push_str("\n\n");

30 + }

31 + body.push_str("data: [DONE]\n\n");

32 + body

33 +}

34 +

35 +fn sse_tool_call(id: &str, name: &str, arguments: Value) -> String {

36 + sse_body(&[json!({

37 + "choices": [{"delta": {"tool_calls": [{

38 + "index": 0,

39 + "id": id,

40 + "function": {"name": name, "arguments": arguments.to_string()},

41 + }]}}]

42 + })])

43 +}

44 +

45 +fn sse_text(text: &str) -> String {

46 + sse_body(&[json!({"choices": [{"delta": {"content": text}}]})])

47 +}

48 +

49 +/// Serves one scripted SSE response per request and records each request body.

50 +struct FakeEndpoint {

51 + port: u16,

52 + requests: Arc<Mutex<Vec<Value>>>,

53 +}

54 +

55 +impl FakeEndpoint {

56 + /// The result the model was given for the tool call `id`.

57 + fn tool_result(&self, id: &str) -> String {

58 + let requests = self.requests.lock().unwrap();

59 + requests

60 + .iter()

61 + .flat_map(|request| request["messages"].as_array().cloned().unwrap_or_default())

62 + .find(|message| message["role"] == "tool" && message["tool_call_id"] == id)

63 + .and_then(|message| message["content"].as_str().map(str::to_string))

64 + .unwrap_or_else(|| panic!("no tool result for {id} reached the endpoint"))

65 + }

66 +}

67 +

68 +fn start_fake_endpoint(responses: Vec<String>) -> FakeEndpoint {

69 + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fake endpoint");

70 + let port = listener.local_addr().unwrap().port();

71 + let requests: Arc<Mutex<Vec<Value>>> = Arc::default();

72 + let recorded = Arc::clone(&requests);

73 + let queue = Mutex::new(VecDeque::from(responses));

74 +

75 + std::thread::spawn(move || {

76 + for stream in listener.incoming() {

77 + let Ok(mut stream) = stream else { continue };

78 + let mut reader = BufReader::new(match stream.try_clone() {

79 + Ok(clone) => clone,

80 + Err(_) => continue,

81 + });

82 + let mut content_length = 0usize;

83 + loop {

84 + let mut line = String::new();

85 + if reader.read_line(&mut line).unwrap_or(0) == 0 {

86 + break;

87 + }

88 + let line = line.trim();

89 + if line.is_empty() {

90 + break;

91 + }

92 + if let Some(length) = line.to_ascii_lowercase().strip_prefix("content-length:") {

93 + content_length = length.trim().parse().unwrap_or(0);

94 + }

95 + }

96 + let mut body = vec![0u8; content_length];

97 + if reader.read_exact(&mut body).is_err() {

98 + continue;

99 + }

100 + if let Ok(request) = serde_json::from_slice::<Value>(&body) {

101 + recorded.lock().unwrap().push(request);

102 + }

103 + let payload = queue

104 + .lock()

105 + .unwrap()

106 + .pop_front()

107 + .unwrap_or_else(|| sse_text("out of scripted responses"));

108 + let response = format!(

109 + "HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\n\

110 + content-length: {}\r\nconnection: close\r\n\r\n{}",

111 + payload.len(),

112 + payload

113 + );

114 + let _ = stream.write_all(response.as_bytes());

115 + let _ = stream.flush();

116 + }

117 + });

118 +

119 + FakeEndpoint { port, requests }

120 +}

121 +

122 +// ── ACP client over the binary's stdio ──────────────────────────────────────

123 +

124 +/// What the editor's terminal does, and what it was asked.

125 +#[derive(Default)]

126 +struct Editor {

127 + /// Answer `terminal/create` with an error, the way a client with

128 + /// terminals turned off would.

129 + refuse_create: bool,

130 + /// Every `terminal/*` request the agent sent, in order.

131 + requests: Vec<Value>,

132 + /// Every `session/update` the agent sent, in order.

133 + updates: Vec<Value>,

134 +}

135 +

136 +impl Editor {

137 + fn methods(&self) -> Vec<&str> {

138 + self.requests

139 + .iter()

140 + .filter_map(|request| request["method"].as_str())

141 + .collect()

142 + }

143 +

144 + fn request(&self, method: &str) -> &Value {

145 + self.requests

146 + .iter()

147 + .find(|request| request["method"] == method)

148 + .unwrap_or_else(|| panic!("no {method} request in {:?}", self.requests))

149 + }

150 +

151 + /// The `tool_call_update`s for one tool call, in order.

152 + fn call_updates(&self, tool_call_id: &str) -> Vec<&Value> {

153 + self.updates

154 + .iter()

155 + .map(|update| &update["params"]["update"])

156 + .filter(|update| {

157 + update["sessionUpdate"] == "tool_call_update"

158 + && update["toolCallId"] == tool_call_id

159 + })

160 + .collect()

161 + }

162 +}

163 +

164 +/// What the editor's terminal printed, which no local run could.

165 +const EDITOR_OUTPUT: &str = "printed in the editor\n";

166 +

167 +struct AgentUnderTest {

168 + child: Child,

169 + stdin: ChildStdin,

170 + incoming: Receiver<Value>,

171 + next_id: u64,

172 +}

173 +

174 +fn spawn_agent(port: u16, config_dir: &Path, client_terminal: Option<&str>) -> AgentUnderTest {

175 + let mut command = Command::new(env!("CARGO_BIN_EXE_sigit"));

176 + command

177 + .env("OPENAI_BASE_URL", format!("http://127.0.0.1:{port}"))

178 + .env("OPENAI_API_KEY", "test-key")

179 + .env("SIGIT_MODEL", "scripted-model")

180 + .env("SIGIT_CONFIG_DIR", config_dir)

181 + .env("SIGIT_MCP", "off")

182 + .env("SIGIT_PERMISSIONS", "allow")

183 + .env_remove("SIGIT_LOCAL_INFERENCE")

184 + .stdin(Stdio::piped())

185 + .stdout(Stdio::piped())

186 + .stderr(Stdio::null());

187 + match client_terminal {

188 + None => command.env_remove("SIGIT_CLIENT_TERMINAL"),

189 + Some(value) => command.env("SIGIT_CLIENT_TERMINAL", value),

190 + };

191 + let mut child = command.spawn().expect("spawn sigit in ACP mode");

192 +

193 + let stdout = child.stdout.take().unwrap();

194 + let (message_tx, incoming) = channel();

195 + std::thread::spawn(move || {

196 + for line in BufReader::new(stdout).lines() {

197 + let Ok(line) = line else { break };

198 + if let Ok(message) = serde_json::from_str::<Value>(&line)

199 + && message_tx.send(message).is_err()

200 + {

201 + break;

202 + }

203 + }

204 + });

205 +

206 + let stdin = child.stdin.take().unwrap();

207 + AgentUnderTest {

208 + child,

209 + stdin,

210 + incoming,

211 + next_id: 0,

212 + }

213 +}

214 +

215 +impl AgentUnderTest {

216 + fn send(&mut self, message: Value) {

217 + let mut line = message.to_string();

218 + line.push('\n');

219 + self.stdin.write_all(line.as_bytes()).expect("write stdin");

220 + self.stdin.flush().expect("flush stdin");

221 + }

222 +

223 + fn request(&mut self, method: &str, params: Value) -> u64 {

224 + self.next_id += 1;

225 + let id = self.next_id;

226 + self.send(json!({"jsonrpc": "2.0", "id": id, "method": method, "params": params}));

227 + id

228 + }

229 +

230 + /// Wait for the response to request `id`, serving the agent's

231 + /// `terminal/*` requests from `editor` on the way.

232 + fn wait_for_response(&mut self, id: u64, editor: &mut Editor) -> Value {

233 + let deadline = Instant::now() + TIMEOUT;

234 + loop {

235 + let remaining = deadline.saturating_duration_since(Instant::now());

236 + let Ok(message) = self.incoming.recv_timeout(remaining) else {

237 + panic!("timed out waiting for the response to request {id}");

238 + };

239 + if message["id"] == id && message.get("method").is_none() {

240 + assert!(

241 + message.get("error").is_none(),

242 + "request {id} failed: {message}"

243 + );

244 + return message;

245 + }

246 + let Some(method) = message["method"].as_str() else {

247 + continue;

248 + };

249 + if method == "session/update" {

250 + editor.updates.push(message.clone());

251 + continue;

252 + }

253 + if !method.starts_with("terminal/") {

254 + continue;

255 + }

256 + editor.requests.push(message.clone());

257 + let request_id = message["id"].clone();

258 + let result = match method {

259 + "terminal/create" if editor.refuse_create => {

260 + self.send(json!({

261 + "jsonrpc": "2.0",

262 + "id": request_id,

263 + "error": {"code": -32603, "message": "terminals are off"},

264 + }));

265 + continue;

266 + }

267 + "terminal/create" => json!({"terminalId": "term-1"}),

268 + "terminal/wait_for_exit" => json!({"exitCode": 0}),

269 + "terminal/output" => json!({

270 + "output": EDITOR_OUTPUT,

271 + "truncated": false,

272 + "exitStatus": {"exitCode": 0},

273 + }),

274 + _ => json!({}),

275 + };

276 + self.send(json!({"jsonrpc": "2.0", "id": request_id, "result": result}));

277 + }

278 + }

279 +

280 + /// `initialize` with or without the `terminal` capability, then a session

281 + /// in `cwd`.

282 + fn open_session(&mut self, terminal: bool, cwd: &Path) -> String {

283 + let mut editor = Editor::default();

284 + let id = self.request(

285 + "initialize",

286 + json!({"protocolVersion": 1, "clientCapabilities": {"terminal": terminal}}),

287 + );

288 + self.wait_for_response(id, &mut editor);

289 +

290 + let id = self.request("session/new", json!({"cwd": cwd, "mcpServers": []}));

291 + self.wait_for_response(id, &mut editor)["result"]["sessionId"]

292 + .as_str()

293 + .expect("session id")

294 + .to_string()

295 + }

296 +

297 + fn prompt(&mut self, session_id: &str, editor: &mut Editor) {

298 + let id = self.request(

299 + "session/prompt",

300 + json!({"sessionId": session_id, "prompt": [{"type": "text", "text": "go"}]}),

301 + );

302 + self.wait_for_response(id, editor);

303 + }

304 +}

305 +

306 +impl Drop for AgentUnderTest {

307 + fn drop(&mut self) {

308 + let _ = self.child.kill();

309 + let _ = self.child.wait();

310 + }

311 +}

312 +

313 +/// A scratch project: `config/` for the agent and `work/` as the session root.

314 +struct Scratch {

315 + root: PathBuf,

316 + config: PathBuf,

317 + work: PathBuf,

318 +}

319 +

320 +impl Scratch {

321 + fn new(name: &str) -> Self {

322 + let root =

323 + std::env::temp_dir().join(format!("sigit_acp_terminal_{name}_{}", std::process::id()));

324 + let _ = std::fs::remove_dir_all(&root);

325 + let config = root.join("config");

326 + let work = root.join("work");

327 + std::fs::create_dir_all(&config).unwrap();

328 + std::fs::create_dir_all(&work).unwrap();

329 + Self { root, config, work }

330 + }

331 +}

332 +

333 +impl Drop for Scratch {

334 + fn drop(&mut self) {

335 + std::fs::remove_dir_all(&self.root).ok();

336 + }

337 +}

338 +

339 +/// A `run_command` call printing a marker, in `cwd`. Quote-free so the same

340 +/// command line works under `sh -c` and `cmd /C`.

341 +fn echo_call(id: &str, cwd: &Path, background: bool) -> String {

342 + sse_tool_call(

343 + id,

344 + "run_command",

345 + json!({"command": "echo ran-locally", "cwd": cwd, "run_in_background": background}),

346 + )

347 +}

348 +

349 +// ── The tests ───────────────────────────────────────────────────────────────

350 +

351 +#[test]

352 +fn run_command_runs_in_the_client_terminal_and_shows_it() {

353 + let scratch = Scratch::new("terminal");

354 + let endpoint = start_fake_endpoint(vec![

355 + echo_call("call_1", &scratch.work, false),

356 + sse_text("done"),

357 + ]);

358 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

359 + let session_id = agent.open_session(true, &scratch.work);

360 +

361 + let mut editor = Editor::default();

362 + agent.prompt(&session_id, &mut editor);

363 +

364 + assert_eq!(

365 + endpoint.tool_result("call_1"),

366 + format!("Exit code 0:\n{EDITOR_OUTPUT}")

367 + );

368 + assert_eq!(

369 + editor.methods(),

370 + [

371 + "terminal/create",

372 + "terminal/wait_for_exit",

373 + "terminal/output",

374 + "terminal/release",

375 + ]

376 + );

377 +

378 + let create = &editor.request("terminal/create")["params"];

379 + assert_eq!(create["sessionId"], session_id.as_str());

380 + #[cfg(unix)]

381 + assert_eq!(create["command"], "sh");

382 + #[cfg(windows)]

383 + assert_eq!(create["command"], "cmd");

384 + assert_eq!(create["args"][1], "echo ran-locally");

385 + let cwd = PathBuf::from(create["cwd"].as_str().unwrap());

386 + assert_eq!(

387 + cwd.canonicalize().unwrap(),

388 + scratch.work.canonicalize().unwrap()

389 + );

390 + for method in [

391 + "terminal/wait_for_exit",

392 + "terminal/output",

393 + "terminal/release",

394 + ] {

395 + assert_eq!(editor.request(method)["params"]["terminalId"], "term-1");

396 + }

397 +

398 + // The terminal is in the card while the command runs and after it ends.

399 + let terminal = json!([{"type": "terminal", "terminalId": "term-1"}]);

400 + let updates = editor.call_updates("call_1");

401 + let shown = updates

402 + .iter()

403 + .position(|update| update["content"] == terminal && update["status"].is_null())

404 + .unwrap_or_else(|| panic!("terminal never embedded: {updates:?}"));

405 + let finished = updates

406 + .iter()

407 + .position(|update| update["status"] == "completed")

408 + .expect("call_1 never completed");

409 + assert!(shown < finished, "embedded after completion: {updates:?}");

410 + assert_eq!(updates[finished]["content"], terminal);

411 +}

412 +

413 +#[test]

414 +fn a_client_without_a_terminal_gets_the_command_run_locally() {

415 + let scratch = Scratch::new("noterminal");

416 + let endpoint = start_fake_endpoint(vec![

417 + echo_call("call_1", &scratch.work, false),

418 + sse_text("done"),

419 + ]);

420 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

421 + let session_id = agent.open_session(false, &scratch.work);

422 +

423 + let mut editor = Editor::default();

424 + agent.prompt(&session_id, &mut editor);

425 +

426 + let result = endpoint.tool_result("call_1");

427 + assert!(result.contains("ran-locally"), "run_command said: {result}");

428 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);

429 +}

430 +

431 +#[test]

432 +fn a_background_command_stays_local() {

433 + let scratch = Scratch::new("background");

434 + let endpoint = start_fake_endpoint(vec![

435 + echo_call("call_1", &scratch.work, true),

436 + sse_text("done"),

437 + ]);

438 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

439 + let session_id = agent.open_session(true, &scratch.work);

440 +

441 + let mut editor = Editor::default();

442 + agent.prompt(&session_id, &mut editor);

443 +

444 + let result = endpoint.tool_result("call_1");

445 + assert!(result.contains("task"), "run_command said: {result}");

446 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);

447 +}

448 +

449 +#[test]

450 +fn a_directory_outside_the_session_roots_stays_local() {

451 + let scratch = Scratch::new("outside");

452 + let outside = scratch.root.join("elsewhere");

453 + std::fs::create_dir_all(&outside).unwrap();

454 + let endpoint =

455 + start_fake_endpoint(vec![echo_call("call_1", &outside, false), sse_text("done")]);

456 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

457 + let session_id = agent.open_session(true, &scratch.work);

458 +

459 + let mut editor = Editor::default();

460 + agent.prompt(&session_id, &mut editor);

461 +

462 + let result = endpoint.tool_result("call_1");

463 + assert!(result.contains("ran-locally"), "run_command said: {result}");

464 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);

465 +}

466 +

467 +#[test]

468 +fn a_terminal_the_client_refuses_falls_back_to_running_locally() {

469 + let scratch = Scratch::new("refused");

470 + let endpoint = start_fake_endpoint(vec![

471 + echo_call("call_1", &scratch.work, false),

472 + sse_text("done"),

473 + ]);

474 + let mut agent = spawn_agent(endpoint.port, &scratch.config, None);

475 + let session_id = agent.open_session(true, &scratch.work);

476 +

477 + let mut editor = Editor {

478 + refuse_create: true,

479 + ..Editor::default()

480 + };

481 + agent.prompt(&session_id, &mut editor);

482 +

483 + let result = endpoint.tool_result("call_1");

484 + assert!(result.contains("ran-locally"), "run_command said: {result}");

485 + assert_eq!(editor.methods(), ["terminal/create"]);

486 +}

487 +

488 +#[test]

489 +fn sigit_client_terminal_off_keeps_commands_local() {

490 + let scratch = Scratch::new("off");

491 + let endpoint = start_fake_endpoint(vec![

492 + echo_call("call_1", &scratch.work, false),

493 + sse_text("done"),

494 + ]);

495 + let mut agent = spawn_agent(endpoint.port, &scratch.config, Some("off"));

496 + let session_id = agent.open_session(true, &scratch.work);

497 +

498 + let mut editor = Editor::default();

499 + agent.prompt(&session_id, &mut editor);

500 +

501 + let result = endpoint.tool_result("call_1");

502 + assert!(result.contains("ran-locally"), "run_command said: {result}");

503 + assert!(editor.requests.is_empty(), "{:?}", editor.requests);

504 +}