diff --git a/crates/acp_thread/src/terminal.rs b/crates/acp_thread/src/terminal.rs index 9c10ebc9392..dd79565c53b 100644 --- a/crates/acp_thread/src/terminal.rs +++ b/crates/acp_thread/src/terminal.rs @@ -420,7 +420,7 @@ pub struct Terminal { } enum TerminalExecution { - Process(Shared>), + Process(Shared>), Display(DisplayTerminalState), } @@ -468,6 +468,29 @@ pub struct DisplayTerminalOutput { pub meta: Option, } +/// Process snapshots are separate from agent-reported display output and wire response envelopes. +#[derive(Clone, Debug, PartialEq)] +pub struct ProcessOutput { + pub output: String, + pub truncated: bool, + pub exit_status: Option, +} + +impl ProcessOutput { + pub fn new(output: impl Into, truncated: bool) -> Self { + Self { + output: output.into(), + truncated, + exit_status: None, + } + } + + pub fn exit_status(mut self, status: acp_v2::TerminalExitStatus) -> Self { + self.exit_status = Some(status); + self + } +} + pub struct TerminalOutput { pub ended_at: Instant, pub exit_status: acp_v2::TerminalExitStatus, @@ -521,16 +544,12 @@ impl Terminal { execution: TerminalExecution::Process( cx.spawn(async move |this, cx| { let exit_status = command_task.await.map(Self::transform_exit_status); - let exit_status = acp_v1::TerminalExitStatus::new() + let exit_status = acp_v2::TerminalExitStatus::new() .exit_code(exit_status.as_ref().map(|status| status.0)) .signal(exit_status.and_then(|status| status.1)); this.update(cx, |this, cx| { - this.cache_output( - Self::exit_status_from_v1(exit_status.clone()), - Instant::now(), - cx, - ); + this.cache_output(exit_status.clone(), Instant::now(), cx); this.terminal.update(cx, |terminal, _cx| { terminal.release_pty_resources(); }); @@ -598,7 +617,7 @@ impl Terminal { matches!(self.execution, TerminalExecution::Process(_)) } - pub fn wait_for_exit(&self) -> Result>> { + pub fn wait_for_exit(&self) -> Result>> { match &self.execution { TerminalExecution::Process(task) => Ok(task.clone()), TerminalExecution::Display(_) => { @@ -631,22 +650,17 @@ impl Terminal { self.user_stopped.load(Ordering::SeqCst) } - pub fn current_output(&self, cx: &App) -> acp_v1::TerminalOutputResponse { + pub fn current_output(&self, cx: &App) -> ProcessOutput { if let Some(output) = self.output.as_ref() { - acp_v1::TerminalOutputResponse::new( + ProcessOutput::new( output.content.clone(), output.original_content_len > output.content.len(), ) - .exit_status( - acp_v1::TerminalExitStatus::new() - .exit_code(output.exit_status.exit_code) - .signal(output.exit_status.signal.clone()) - .meta(output.exit_status.meta.clone()), - ) + .exit_status(output.exit_status.clone()) } else { let (current_content, original_len) = self.truncated_output(cx); let truncated = current_content.len() < original_len; - acp_v1::TerminalOutputResponse::new(current_content, truncated) + ProcessOutput::new(current_content, truncated) } } diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 4e02992ecd2..11e36c78e88 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -31,7 +31,8 @@ pub use tools::*; use acp_thread::{ AcpThread, AgentModelId, AgentModelSelector, AgentSessionInfo, AgentSessionList, - AgentSessionListRequest, AgentSessionListResponse, ClientUserMessageId, TokenUsageRatio, + AgentSessionListRequest, AgentSessionListResponse, ClientUserMessageId, ProcessOutput, + TokenUsageRatio, }; use agent_client_protocol::schema::v1 as acp_v1; use agent_client_protocol::schema::v2 as acp_v2; @@ -3700,12 +3701,12 @@ impl TerminalHandle for AcpTerminalHandle { Ok(self.terminal.read_with(cx, |term, _cx| term.id().clone())) } - fn wait_for_exit(&self, cx: &AsyncApp) -> Result>> { + fn wait_for_exit(&self, cx: &AsyncApp) -> Result>> { self.terminal .read_with(cx, |term, _cx| term.wait_for_exit()) } - fn current_output(&self, cx: &AsyncApp) -> Result { + fn current_output(&self, cx: &AsyncApp) -> Result { Ok(self .terminal .read_with(cx, |term, cx| term.current_output(cx))) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 9fe0e172b86..f1bc028d524 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -1,7 +1,7 @@ use super::*; use acp_thread::{ AgentConnection, AgentModelGroupName, AgentModelId, AgentModelList, AgentThreadEntry, - ClientUserMessageId, PermissionOptions, ThreadStatus, + ClientUserMessageId, PermissionOptions, ProcessOutput, ThreadStatus, }; use agent_client_protocol::schema::MaybeUndefined; use agent_client_protocol::schema::v1 as acp; @@ -76,8 +76,8 @@ pub(crate) struct FakeTerminalHandle { killed: Arc, stopped_by_user: Arc, exit_sender: std::cell::RefCell>>, - wait_for_exit: Shared>, - output: acp::TerminalOutputResponse, + wait_for_exit: Shared>, + output: ProcessOutput, id: acp_v2::TerminalId, } @@ -92,7 +92,7 @@ impl FakeTerminalHandle { .spawn(async move |_cx| { // Wait for the exit signal (sent when kill() is called) let _ = exit_receiver.await; - acp::TerminalExitStatus::new() + acp_v2::TerminalExitStatus::new() }) .shared(); @@ -101,7 +101,7 @@ impl FakeTerminalHandle { stopped_by_user, exit_sender: std::cell::RefCell::new(Some(exit_sender)), wait_for_exit, - output: acp::TerminalOutputResponse::new("partial output".to_string(), false), + output: ProcessOutput::new("partial output".to_string(), false), id: acp_v2::TerminalId::new("fake_terminal".to_string()), } } @@ -112,7 +112,7 @@ impl FakeTerminalHandle { let (exit_sender, _exit_receiver) = futures::channel::oneshot::channel(); let wait_for_exit = cx - .spawn(async move |_cx| acp::TerminalExitStatus::new().exit_code(exit_code)) + .spawn(async move |_cx| acp_v2::TerminalExitStatus::new().exit_code(exit_code)) .shared(); Self { @@ -120,12 +120,12 @@ impl FakeTerminalHandle { stopped_by_user, exit_sender: std::cell::RefCell::new(Some(exit_sender)), wait_for_exit, - output: acp::TerminalOutputResponse::new("command output".to_string(), false), + output: ProcessOutput::new("command output".to_string(), false), id: acp_v2::TerminalId::new("fake_terminal".to_string()), } } - pub(crate) fn with_output(mut self, output: acp::TerminalOutputResponse) -> Self { + pub(crate) fn with_output(mut self, output: ProcessOutput) -> Self { self.output = output; self } @@ -150,11 +150,11 @@ impl crate::TerminalHandle for FakeTerminalHandle { Ok(self.id.clone()) } - fn current_output(&self, _cx: &AsyncApp) -> Result { + fn current_output(&self, _cx: &AsyncApp) -> Result { Ok(self.output.clone()) } - fn wait_for_exit(&self, _cx: &AsyncApp) -> Result>> { + fn wait_for_exit(&self, _cx: &AsyncApp) -> Result>> { Ok(self.wait_for_exit.clone()) } diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 9b383117ba5..9ef463fdc75 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -7,7 +7,7 @@ use crate::{ SystemPromptTemplate, Template, Templates, TerminalTool, ToolPermissionDecision, WebSearchTool, WriteFileTool, decide_permission_from_settings, }; -use acp_thread::{AgentModelId, ClientUserMessageId, MentionUri}; +use acp_thread::{AgentModelId, ClientUserMessageId, MentionUri, ProcessOutput}; use action_log::ActionLog; use agent_settings::UserAgentsMd; @@ -761,8 +761,8 @@ pub enum AgentMessageContent { pub trait TerminalHandle { fn id(&self, cx: &AsyncApp) -> Result; - fn current_output(&self, cx: &AsyncApp) -> Result; - fn wait_for_exit(&self, cx: &AsyncApp) -> Result>>; + fn current_output(&self, cx: &AsyncApp) -> Result; + fn wait_for_exit(&self, cx: &AsyncApp) -> Result>>; fn kill(&self, cx: &AsyncApp) -> Result<()>; fn was_stopped_by_user(&self, cx: &AsyncApp) -> Result; } diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 253075d5bdc..61c7693e043 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -1,4 +1,4 @@ -use agent_client_protocol::schema::v1 as acp; +use acp_thread::ProcessOutput; use agent_client_protocol::schema::v2 as acp_v2; use anyhow::Result; use futures::FutureExt as _; @@ -1246,7 +1246,7 @@ fn wsl_interop_blocked(content: &str) -> bool { } fn process_content( - output: acp::TerminalOutputResponse, + output: ProcessOutput, command: &str, timed_out: bool, user_stopped: bool, @@ -1567,7 +1567,7 @@ mod tests { #[test] fn test_process_content_user_stopped() { - let output = acp::TerminalOutputResponse::new("partial output".to_string(), false); + let output = ProcessOutput::new("partial output".to_string(), false); let result = process_content( output, @@ -1803,8 +1803,8 @@ mod tests { #[test] fn test_process_content_filters_success_output_for_model() { - let output = acp::TerminalOutputResponse::new("one\ntwo\nthree\nfour".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("one\ntwo\nthree\nfour".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let result = process_content( output, @@ -1822,8 +1822,8 @@ mod tests { #[test] fn test_process_content_filters_failure_output_for_model() { - let output = acp::TerminalOutputResponse::new("one\ntwo\nthree".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(1)); + let output = ProcessOutput::new("one\ntwo\nthree".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(1)); let result = process_content( output, @@ -1844,7 +1844,7 @@ mod tests { #[test] fn test_process_content_filters_timeout_output_for_model() { - let output = acp::TerminalOutputResponse::new("one\ntwo\nthree".to_string(), false); + let output = ProcessOutput::new("one\ntwo\nthree".to_string(), false); let result = process_content( output, @@ -1865,7 +1865,7 @@ mod tests { #[test] fn test_process_content_filters_user_stopped_output_for_model() { - let output = acp::TerminalOutputResponse::new("one\ntwo\nthree".to_string(), false); + let output = ProcessOutput::new("one\ntwo\nthree".to_string(), false); let result = process_content( output, @@ -1887,8 +1887,8 @@ mod tests { #[test] fn test_process_content_selected_output_has_no_explanatory_note() { - let output = acp::TerminalOutputResponse::new("one\ntwo\nthree".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("one\ntwo\nthree".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let result = process_content( output, @@ -1908,7 +1908,7 @@ mod tests { #[test] fn test_process_content_user_stopped_empty_output() { - let output = acp::TerminalOutputResponse::new("".to_string(), false); + let output = ProcessOutput::new("".to_string(), false); let result = process_content( output, @@ -1932,7 +1932,7 @@ mod tests { #[test] fn test_process_content_timed_out() { - let output = acp::TerminalOutputResponse::new("build output here".to_string(), false); + let output = ProcessOutput::new("build output here".to_string(), false); let result = process_content( output, @@ -1956,7 +1956,7 @@ mod tests { #[test] fn test_process_content_timed_out_with_empty_output() { - let output = acp::TerminalOutputResponse::new("".to_string(), false); + let output = ProcessOutput::new("".to_string(), false); let result = process_content( output, @@ -1980,8 +1980,8 @@ mod tests { #[test] fn test_process_content_with_success() { - let output = acp::TerminalOutputResponse::new("success output".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("success output".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let result = process_content( output, @@ -2005,8 +2005,8 @@ mod tests { #[test] fn test_process_content_with_success_empty_output() { - let output = acp::TerminalOutputResponse::new("".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let result = process_content( output, @@ -2025,8 +2025,8 @@ mod tests { #[test] fn test_process_content_with_error_exit() { - let output = acp::TerminalOutputResponse::new("error output".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(1)); + let output = ProcessOutput::new("error output".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(1)); let result = process_content( output, @@ -2050,8 +2050,8 @@ mod tests { #[test] fn test_process_content_with_error_exit_empty_output() { - let output = acp::TerminalOutputResponse::new("".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(1)); + let output = ProcessOutput::new("".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(1)); let result = process_content( output, @@ -2070,7 +2070,7 @@ mod tests { #[test] fn test_process_content_unexpected_termination() { - let output = acp::TerminalOutputResponse::new("some output".to_string(), false); + let output = ProcessOutput::new("some output".to_string(), false); let result = process_content( output, @@ -2094,7 +2094,7 @@ mod tests { #[test] fn test_process_content_unexpected_termination_empty_output() { - let output = acp::TerminalOutputResponse::new("".to_string(), false); + let output = ProcessOutput::new("".to_string(), false); let result = process_content( output, @@ -2400,9 +2400,8 @@ mod tests { fs.insert_tree("/root", serde_json::json!({})).await; let project = project::Project::test(fs, ["/root".as_ref()], cx).await; - let output = - acp::TerminalOutputResponse::new("one\ntwo\nthree\nfour\nfive".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("one\ntwo\nthree\nfour\nfive".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let environment = std::rc::Rc::new(cx.update(|cx| { crate::tests::FakeThreadEnvironment::default().with_terminal( crate::tests::FakeTerminalHandle::new_with_immediate_exit(cx, 0) @@ -2460,8 +2459,8 @@ mod tests { fs.insert_tree("/root", serde_json::json!({})).await; let project = project::Project::test(fs, ["/root".as_ref()], cx).await; - let output = acp::TerminalOutputResponse::new("command output".to_string(), false) - .exit_status(acp::TerminalExitStatus::new().exit_code(0)); + let output = ProcessOutput::new("command output".to_string(), false) + .exit_status(acp_v2::TerminalExitStatus::new().exit_code(0)); let environment = std::rc::Rc::new(cx.update(|cx| { crate::tests::FakeThreadEnvironment::default().with_terminal( crate::tests::FakeTerminalHandle::new_with_immediate_exit(cx, 0) diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 4898b5cb771..d43a76860a8 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -2754,6 +2754,48 @@ mod tests { use super::*; use settings::Settings as _; + #[test] + fn test_process_terminal_responses_preserve_v1_payloads() { + use acp_thread::ProcessOutput; + use serde_json::json; + + let pending = process_output_to_v1(ProcessOutput::new("partial\n", false)); + assert_eq!( + serde_json::to_value(pending).expect("pending output response"), + json!({"output": "partial\n", "truncated": false}), + ); + let unknown_exit = process_output_to_v1( + ProcessOutput::new("", false).exit_status(acp_v2::TerminalExitStatus::new()), + ); + assert_eq!( + serde_json::to_value(unknown_exit).expect("unknown exit response"), + json!({"output": "", "truncated": false, "exitStatus": {}}), + ); + + let meta = + acp_v2::Meta::from_iter([("custom".into(), json!({"values": [null, 7, "keep"]}))]); + let status = acp_v2::TerminalExitStatus::new() + .exit_code(u32::MAX) + .signal("SIG_CUSTOM") + .meta(meta); + let expected_status = json!({ + "exitCode": u32::MAX, + "signal": "SIG_CUSTOM", + "_meta": {"custom": {"values": [null, 7, "keep"]}}, + }); + let output = + process_output_to_v1(ProcessOutput::new("héllo\n", true).exit_status(status.clone())); + assert_eq!( + serde_json::to_value(output).expect("completed output response"), + json!({"output": "héllo\n", "truncated": true, "exitStatus": expected_status}), + ); + let wait = acp::WaitForTerminalExitResponse::new(terminal_exit_status_to_v1(status)); + assert_eq!( + serde_json::to_value(wait).expect("wait response"), + expected_status, + ); + } + #[test] fn test_v1_permission_conversion_preserves_options_and_arc_ids() { for (legacy_kind, kind) in [ @@ -7965,12 +8007,24 @@ fn handle_terminal_output( .flatten_acp() { Ok(output) => { - responder.respond(output).log_err(); + responder.respond(process_output_to_v1(output)).log_err(); } Err(e) => respond_err(responder, e), } } +fn process_output_to_v1(output: acp_thread::ProcessOutput) -> acp::TerminalOutputResponse { + acp::TerminalOutputResponse::new(output.output, output.truncated) + .exit_status(output.exit_status.map(terminal_exit_status_to_v1)) +} + +fn terminal_exit_status_to_v1(status: acp_v2::TerminalExitStatus) -> acp::TerminalExitStatus { + acp::TerminalExitStatus::new() + .exit_code(status.exit_code) + .signal(status.signal) + .meta(status.meta) +} + fn handle_wait_for_terminal_exit( args: acp::WaitForTerminalExitRequest, responder: Responder, @@ -7999,7 +8053,12 @@ fn handle_wait_for_terminal_exit( }) .await; - respond_result(responder, result.map(acp::WaitForTerminalExitResponse::new)); + respond_result( + responder, + result.map(|status| { + acp::WaitForTerminalExitResponse::new(terminal_exit_status_to_v1(status)) + }), + ); }) .detach(); }