mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-20 11:46:49 +00:00
acp_thread: Share v2 notice definitions (#65241)
Share SDK v2 notice definitions across the existing live session owner and notice UI. One explicit v1 adapter feeds the same insertion path used by direct v2 values; production ACP remains v1. - Preserve all notice fields, opaque future/custom severity strings, plain text, nested notice metadata, and absent versus empty optional values. - Keep monotonically assigned local IDs, duplicate live events, independent/idempotent dismissal, and the existing notice update/notify lifecycle. - Keep notices out of transcript/history/export, title, conversation metadata, activity/error state, and permissions. - Keep the existing severity presentation, unknown-severity fallback, controls, mouse dismissal, and keyboard focus restoration unchanged. - Preserve the current v1 beta/capability advertisement. V2 notices do not require client capability advertisement, but this slice adds no v2 wire handler or backend. ## Review guide - [explicit v1 adapter](7bed6876a0/crates/acp_thread/src/notices.rs (L4)) and [adapter fixture](7bed6876a0/crates/acp_thread/src/notices.rs (L23)): direct field mapping without a production serde bridge. - [shared live-only insertion](7bed6876a0/crates/acp_thread/src/acp_thread.rs (L3817)) and [live-state and independent-dismissal regression](7bed6876a0/crates/acp_thread/src/acp_thread.rs (L18441)): one existing live-only owner and event path for both versions. - [existing notice renderer](7bed6876a0/crates/agent_ui/src/ui/session_notice.rs (L13)) and [rendered mouse/keyboard dismissal regression](7bed6876a0/crates/agent_ui/src/conversation_view.rs (L5337)): mixed v1/direct-v2 values, complete metadata retention, actual mouse/keyboard dismissal, and focus restoration. - [SDK session-isolation/reload regression](7bed6876a0/crates/agent_servers/src/acp.rs (L5416)): existing v1 traffic remains scoped to the correct session and notices are not restored on reload. Release Notes: - N/A
This commit is contained in:
Ben Brandt
GitHub
parent
e7526b378a
commit
f87356210a
5 changed files with 127 additions and 29 deletions
|
|
@ -5,6 +5,7 @@ mod connection;
|
|||
pub mod content;
|
||||
mod diff;
|
||||
mod mention;
|
||||
pub mod notices;
|
||||
pub mod prompt_capabilities;
|
||||
mod submission;
|
||||
mod terminal;
|
||||
|
|
@ -3368,7 +3369,7 @@ pub struct AcpThread {
|
|||
provisional_title: Option<SharedString>,
|
||||
entries: Vec<AgentThreadEntry>,
|
||||
// Notices stay with the live session, but never enter conversation history or exports.
|
||||
notices: Vec<(usize, acp_v1::Notice)>,
|
||||
notices: Vec<(usize, acp_v2::Notice)>,
|
||||
next_notice_id: usize,
|
||||
elicitations: ElicitationStore,
|
||||
permission_requests: IndexMap<PermissionRequestId, PermissionRequest>,
|
||||
|
|
@ -3810,10 +3811,18 @@ impl AcpThread {
|
|||
&self.entries
|
||||
}
|
||||
|
||||
pub fn notices(&self) -> &[(usize, acp_v1::Notice)] {
|
||||
pub fn notices(&self) -> &[(usize, acp_v2::Notice)] {
|
||||
&self.notices
|
||||
}
|
||||
|
||||
pub fn push_notice(&mut self, notice: acp_v2::Notice, cx: &mut Context<Self>) {
|
||||
let notice_id = self.next_notice_id;
|
||||
self.next_notice_id += 1;
|
||||
self.notices.push((notice_id, notice));
|
||||
cx.emit(AcpThreadEvent::NoticesUpdated);
|
||||
cx.notify();
|
||||
}
|
||||
|
||||
pub fn dismiss_notice(&mut self, notice_id: usize, cx: &mut Context<Self>) {
|
||||
let previous_count = self.notices.len();
|
||||
self.notices.retain(|(id, _)| *id != notice_id);
|
||||
|
|
@ -4287,11 +4296,7 @@ impl AcpThread {
|
|||
self.update_plan(plan, cx).map_err(acp_v1::Error::from)?;
|
||||
}
|
||||
acp_v1::SessionUpdate::Notice(notice) => {
|
||||
let notice_id = self.next_notice_id;
|
||||
self.next_notice_id += 1;
|
||||
self.notices.push((notice_id, notice));
|
||||
cx.emit(AcpThreadEvent::NoticesUpdated);
|
||||
cx.notify();
|
||||
self.push_notice(notices::from_v1(notice).map_err(acp_v1::Error::from)?, cx);
|
||||
}
|
||||
acp_v1::SessionUpdate::SessionInfoUpdate(info_update) => {
|
||||
self.update_session_info(session_info_update_from_v1(info_update), cx);
|
||||
|
|
@ -18463,29 +18468,32 @@ mod tests {
|
|||
});
|
||||
|
||||
let warning =
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Warning, "MCP server unavailable")
|
||||
acp_v2::Notice::new(acp_v2::NoticeSeverity::Warning, "MCP server unavailable")
|
||||
.description("Continuing without it.");
|
||||
let notices = vec![
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Info, "Using default configuration"),
|
||||
acp_v2::Notice::new(acp_v2::NoticeSeverity::Info, "Using default configuration"),
|
||||
warning.clone(),
|
||||
warning.clone(),
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Error, "Optional integration failed"),
|
||||
acp_v1::Notice::new(
|
||||
acp_v1::NoticeSeverity::Other("critical".into()),
|
||||
acp_v2::Notice::new(acp_v2::NoticeSeverity::Error, "Optional integration failed")
|
||||
.description("")
|
||||
.meta(acp_v2::Meta::new()),
|
||||
acp_v2::Notice::new(
|
||||
acp_v2::NoticeSeverity::Other("critical".into()),
|
||||
"Future severity",
|
||||
),
|
||||
acp_v1::Notice::new(
|
||||
acp_v1::NoticeSeverity::Other("_custom".into()),
|
||||
acp_v2::Notice::new(
|
||||
acp_v2::NoticeSeverity::Other("_custom".into()),
|
||||
"**Plain text**, not Markdown",
|
||||
)
|
||||
.meta(acp_v1::Meta::from_iter([("source".into(), "test".into())])),
|
||||
.meta(acp_v2::Meta::from_iter([(
|
||||
"extension".into(),
|
||||
serde_json::json!({"nested": [null, true, {"value": "retained"}]}),
|
||||
)])),
|
||||
];
|
||||
|
||||
thread.update(cx, |thread, cx| {
|
||||
for notice in ¬ices {
|
||||
thread
|
||||
.handle_session_update(acp_v1::SessionUpdate::Notice(notice.clone()), cx)
|
||||
.expect("notice should be accepted");
|
||||
thread.push_notice(notice.clone(), cx);
|
||||
}
|
||||
assert_eq!(
|
||||
thread.notices(),
|
||||
|
|
@ -18523,9 +18531,7 @@ mod tests {
|
|||
thread.dismiss_notice(notice_id, cx);
|
||||
}
|
||||
assert!(thread.notices().is_empty());
|
||||
thread
|
||||
.handle_session_update(acp_v1::SessionUpdate::Notice(warning.clone()), cx)
|
||||
.expect("a repeated notice is a new live event");
|
||||
thread.push_notice(warning.clone(), cx);
|
||||
thread.dismiss_notice(1, cx);
|
||||
assert_eq!(thread.notices(), &[(notices.len(), warning)]);
|
||||
assert!(thread.entries().is_empty());
|
||||
|
|
|
|||
78
crates/acp_thread/src/notices.rs
Normal file
78
crates/acp_thread/src/notices.rs
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
use agent_client_protocol::schema::{v1 as acp_v1, v2 as acp_v2};
|
||||
use anyhow::{Result, bail};
|
||||
|
||||
pub fn from_v1(notice: acp_v1::Notice) -> Result<acp_v2::Notice> {
|
||||
let severity = match notice.severity {
|
||||
acp_v1::NoticeSeverity::Info => acp_v2::NoticeSeverity::Info,
|
||||
acp_v1::NoticeSeverity::Warning => acp_v2::NoticeSeverity::Warning,
|
||||
acp_v1::NoticeSeverity::Error => acp_v2::NoticeSeverity::Error,
|
||||
acp_v1::NoticeSeverity::Other(severity) => acp_v2::NoticeSeverity::Other(severity),
|
||||
_ => bail!("Unsupported legacy notice severity"),
|
||||
};
|
||||
Ok(acp_v2::Notice::new(severity, notice.title)
|
||||
.description(notice.description)
|
||||
.meta(notice.meta))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn legacy_notices_preserve_fields_and_unknown_severities() -> Result<()> {
|
||||
for (legacy, expected) in [
|
||||
(
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Info, "Using default configuration"),
|
||||
json!({"severity": "info", "title": "Using default configuration"}),
|
||||
),
|
||||
(
|
||||
serde_json::from_value(json!({
|
||||
"severity": "warning",
|
||||
"title": "MCP server unavailable",
|
||||
"description": null,
|
||||
"_meta": null
|
||||
}))?,
|
||||
json!({"severity": "warning", "title": "MCP server unavailable"}),
|
||||
),
|
||||
(
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Error, "Optional integration failed")
|
||||
.description("")
|
||||
.meta(acp_v1::Meta::new()),
|
||||
json!({
|
||||
"severity": "error",
|
||||
"title": "Optional integration failed",
|
||||
"description": "",
|
||||
"_meta": {}
|
||||
}),
|
||||
),
|
||||
(
|
||||
acp_v1::Notice::new(
|
||||
acp_v1::NoticeSeverity::Other("critical".into()),
|
||||
"Future severity",
|
||||
)
|
||||
.description("**Plain text**, not Markdown.\nWork continues.")
|
||||
.meta(acp_v1::Meta::from_iter([(
|
||||
"extension".into(),
|
||||
json!({"nested": [null, true, {"value": "retained"}]}),
|
||||
)])),
|
||||
json!({
|
||||
"severity": "critical",
|
||||
"title": "Future severity",
|
||||
"description": "**Plain text**, not Markdown.\nWork continues.",
|
||||
"_meta": {"extension": {"nested": [null, true, {"value": "retained"}]}}
|
||||
}),
|
||||
),
|
||||
(
|
||||
acp_v1::Notice::new(
|
||||
acp_v1::NoticeSeverity::Other("_custom/β".into()),
|
||||
"Custom severity",
|
||||
),
|
||||
json!({"severity": "_custom/β", "title": "Custom severity"}),
|
||||
),
|
||||
] {
|
||||
assert_eq!(serde_json::to_value(from_v1(legacy)?)?, expected);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
@ -5585,7 +5585,7 @@ exit 7
|
|||
assert_eq!(notice.title, "A current advisory");
|
||||
assert_eq!(
|
||||
notice.severity,
|
||||
acp::NoticeSeverity::Other("_custom".into())
|
||||
acp_v2::NoticeSeverity::Other("_custom".into())
|
||||
);
|
||||
assert_eq!(notice.description, None);
|
||||
assert_eq!(notice.meta, None);
|
||||
|
|
|
|||
|
|
@ -5349,12 +5349,26 @@ pub(crate) mod tests {
|
|||
let notice =
|
||||
acp_v1::Notice::new(acp_v1::NoticeSeverity::Error, "Optional integration failed")
|
||||
.description("**Plain text**, not Markdown. Work continues.");
|
||||
let shared_notice = acp_thread::notices::from_v1(notice.clone()).expect("shared notice");
|
||||
let custom_notice = acp_v2::Notice::new(
|
||||
acp_v2::NoticeSeverity::Other("_custom/β".into()),
|
||||
"**Plain text**, not Markdown",
|
||||
)
|
||||
.description("")
|
||||
.meta(acp_v2::Meta::from_iter([(
|
||||
"extension".into(),
|
||||
json!({"nested": [null, true, {"value": "retained"}]}),
|
||||
)]));
|
||||
thread.update(cx, |thread, cx| {
|
||||
for _ in 0..2 {
|
||||
thread
|
||||
.handle_session_update(acp_v1::SessionUpdate::Notice(notice.clone()), cx)
|
||||
.expect("notice should be accepted");
|
||||
}
|
||||
thread
|
||||
.handle_session_update(acp_v1::SessionUpdate::Notice(notice), cx)
|
||||
.expect("notice should be accepted");
|
||||
thread.push_notice(custom_notice.clone(), cx);
|
||||
assert_eq!(
|
||||
thread.notices(),
|
||||
&[(0, shared_notice), (1, custom_notice.clone())]
|
||||
);
|
||||
assert!(thread.to_markdown(cx).is_empty());
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
|
|
@ -5370,7 +5384,7 @@ pub(crate) mod tests {
|
|||
cx.simulate_click(dismiss.center(), gpui::Modifiers::default());
|
||||
cx.run_until_parked();
|
||||
thread.read_with(cx, |thread, _| {
|
||||
assert_eq!(thread.notices(), &[(1, notice)]);
|
||||
assert_eq!(thread.notices(), &[(1, custom_notice)]);
|
||||
});
|
||||
assert!(cx.debug_bounds("dismiss-session-notice-0").is_none());
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use agent_client_protocol::schema::v1 as acp;
|
||||
use agent_client_protocol::schema::v2 as acp;
|
||||
use component::{Component, ComponentScope, example_group_with_title, single_example};
|
||||
use gpui::{AnyElement, App, ClickEvent, ElementId, Stateful, Window, px};
|
||||
use ui::{Callout, Color, IconButton, IconName, IconSize, Severity, Tooltip, prelude::*};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue