agent: Decouple process output from ACP v1 (#65518)

Decouple local process output and completion from ACP v1 wire responses,
without changing terminal behavior.

- Add a small application-owned [ProcessOutput
snapshot](6799112199/crates/acp_thread/src/terminal.rs (L471))
containing text, truncation and optional v2 exit status. Keep the cached
UI output and agent-reported display updates separate.
- Return v2 exit status from local process waits, and migrate native
handles, terminal-tool consumption and existing test fakes mechanically.
- Convert snapshots/status back to v1 only in the external client
response adapter, retaining text, truncation, exit code, signal and exit
metadata.

Release Notes:

- N/A
This commit is contained in:
Ben Brandt 2026-10-12 20:19:21 +00:00 • committed by GitHub
parent c66312b200
commit ff52936d53
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 137 additions and 64 deletions

View file

@ -420,7 +420,7 @@ pub struct Terminal {
}
enum TerminalExecution {
Process(Shared<Task<acp_v1::TerminalExitStatus>>),
Process(Shared<Task<acp_v2::TerminalExitStatus>>),
Display(DisplayTerminalState),
}
@ -468,6 +468,29 @@ pub struct DisplayTerminalOutput {
pub meta: Option<acp_v2::Meta>,
}
/// 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<acp_v2::TerminalExitStatus>,
}
impl ProcessOutput {
pub fn new(output: impl Into<String>, 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<Shared<Task<acp_v1::TerminalExitStatus>>> {
pub fn wait_for_exit(&self) -> Result<Shared<Task<acp_v2::TerminalExitStatus>>> {
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)
}
}

View file

@ -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<Shared<Task<acp_v1::TerminalExitStatus>>> {
fn wait_for_exit(&self, cx: &AsyncApp) -> Result<Shared<Task<acp_v2::TerminalExitStatus>>> {
self.terminal
.read_with(cx, |term, _cx| term.wait_for_exit())
}
fn current_output(&self, cx: &AsyncApp) -> Result<acp_v1::TerminalOutputResponse> {
fn current_output(&self, cx: &AsyncApp) -> Result<ProcessOutput> {
Ok(self
.terminal
.read_with(cx, |term, cx| term.current_output(cx)))

View file

@ -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<AtomicBool>,
stopped_by_user: Arc<AtomicBool>,
exit_sender: std::cell::RefCell<Option<futures::channel::oneshot::Sender<()>>>,
wait_for_exit: Shared<Task<acp::TerminalExitStatus>>,
output: acp::TerminalOutputResponse,
wait_for_exit: Shared<Task<acp_v2::TerminalExitStatus>>,
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<acp::TerminalOutputResponse> {
fn current_output(&self, _cx: &AsyncApp) -> Result<ProcessOutput> {
Ok(self.output.clone())
}
fn wait_for_exit(&self, _cx: &AsyncApp) -> Result<Shared<Task<acp::TerminalExitStatus>>> {
fn wait_for_exit(&self, _cx: &AsyncApp) -> Result<Shared<Task<acp_v2::TerminalExitStatus>>> {
Ok(self.wait_for_exit.clone())
}

View file

@ -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<acp_v2::TerminalId>;
fn current_output(&self, cx: &AsyncApp) -> Result<acp::TerminalOutputResponse>;
fn wait_for_exit(&self, cx: &AsyncApp) -> Result<Shared<Task<acp::TerminalExitStatus>>>;
fn current_output(&self, cx: &AsyncApp) -> Result<ProcessOutput>;
fn wait_for_exit(&self, cx: &AsyncApp) -> Result<Shared<Task<acp_v2::TerminalExitStatus>>>;
fn kill(&self, cx: &AsyncApp) -> Result<()>;
fn was_stopped_by_user(&self, cx: &AsyncApp) -> Result<bool>;
}

View file

@ -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)

View file

@ -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<acp::WaitForTerminalExitResponse>,
@ -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();
}